feat(read_extract): label each unreadable PDF gap with its preceding section text
The coverage warning listed bare page ranges, which tells the agent WHERE the gaps are but not WHAT they contain — its only options were guessing or OCRing everything. Each gap is now labeled with the last text extracted before it (usually a section divider page), so the agent can decide which gaps it actually needs and render/OCR only those. Gap list capped at 20 entries with a summary line for pathological alternating documents.
This commit is contained in:
@@ -601,9 +601,12 @@ class TestPdfCoverageNote(unittest.TestCase):
|
||||
"""The coverage footer flags PDFs whose pages yielded no text."""
|
||||
|
||||
def _note_with_counts(self, counts):
|
||||
"""Drive _pdf_coverage_note with synthetic per-page texts whose
|
||||
stripped lengths equal ``counts``."""
|
||||
from tools import read_extract
|
||||
with mock.patch.object(read_extract, "_pdf_page_char_counts",
|
||||
return_value=counts):
|
||||
texts = None if counts is None else ["x" * n for n in counts]
|
||||
with mock.patch.object(read_extract, "_pdf_page_texts",
|
||||
return_value=texts):
|
||||
return read_extract._pdf_coverage_note("/x/doc.pdf")
|
||||
|
||||
def test_mostly_scanned_pdf_warns_with_page_ranges(self):
|
||||
@@ -611,9 +614,48 @@ class TestPdfCoverageNote(unittest.TestCase):
|
||||
note = self._note_with_counts([900, 800, 700, 0, 0, 3, 0, 0, 0])
|
||||
self.assertIn("EXTRACTION COVERAGE WARNING", note)
|
||||
self.assertIn("6 of 9 pages", note)
|
||||
self.assertIn("4-9", note) # contiguous empty range
|
||||
self.assertIn("pages 4-9", note) # contiguous empty gap
|
||||
self.assertIn("(6 pages)", note) # gap size stated
|
||||
self.assertIn("vision_analyze", note) # recovery path is named
|
||||
self.assertIn("ocr-and-documents", note)
|
||||
self.assertIn("do NOT OCR or render everything", note)
|
||||
|
||||
def test_gap_labels_carry_preceding_section_text(self):
|
||||
"""Each gap is labeled with the last text page before it (usually
|
||||
a section divider), so the agent can pick which gaps to read."""
|
||||
from tools import read_extract
|
||||
texts = (
|
||||
["Section One: Bylaws of the Corporation"] + [""] * 5
|
||||
+ ["Section Two: Budget details here"] + [""] * 4
|
||||
)
|
||||
with mock.patch.object(read_extract, "_pdf_page_texts",
|
||||
return_value=texts):
|
||||
note = read_extract._pdf_coverage_note("/x/doc.pdf")
|
||||
self.assertIn(
|
||||
'pages 2-6 (5 pages) — after "Section One: Bylaws of the Corporation" (p1)',
|
||||
note,
|
||||
)
|
||||
self.assertIn(
|
||||
'pages 8-11 (4 pages) — after "Section Two: Budget details here" (p7)',
|
||||
note,
|
||||
)
|
||||
|
||||
def test_gap_map_caps_pathological_alternation(self):
|
||||
"""Hundreds of alternating text/scan pages must not balloon the
|
||||
warning — gaps beyond the cap collapse to one summary line."""
|
||||
from tools import read_extract
|
||||
texts = []
|
||||
for i in range(60): # 60 gaps of 1 page each
|
||||
texts.extend([f"Divider page number {i} with enough text", ""])
|
||||
with mock.patch.object(read_extract, "_pdf_page_texts",
|
||||
return_value=texts):
|
||||
note = read_extract._pdf_coverage_note("/x/doc.pdf")
|
||||
gap_lines = [ln for ln in note.splitlines() if ln.startswith(" ")]
|
||||
self.assertEqual(
|
||||
len(gap_lines), read_extract.PDF_GAP_MAP_MAX_ENTRIES + 1
|
||||
)
|
||||
self.assertIn("more gaps", gap_lines[-1])
|
||||
self.assertIn("(40 pages)", gap_lines[-1])
|
||||
|
||||
def test_full_text_pdf_is_silent(self):
|
||||
self.assertEqual(self._note_with_counts([500] * 20), "")
|
||||
|
||||
+63
-15
@@ -214,8 +214,8 @@ PDF_COVERAGE_ABSOLUTE_EMPTY = 10
|
||||
PDF_PAGE_SCAN_TIMEOUT = 20.0
|
||||
|
||||
|
||||
def _pdf_page_char_counts(path: str) -> Optional[list[int]]:
|
||||
"""Per-page extracted-text char counts, or None when undeterminable."""
|
||||
def _pdf_page_texts(path: str) -> Optional[list[str]]:
|
||||
"""Per-page extracted text, or None when undeterminable."""
|
||||
if shutil.which("pdftotext") is None:
|
||||
return None
|
||||
try:
|
||||
@@ -231,23 +231,66 @@ def _pdf_page_char_counts(path: str) -> Optional[list[int]]:
|
||||
pages = proc.stdout.decode("utf-8", errors="replace").split("\f")
|
||||
if pages and not pages[-1].strip():
|
||||
pages.pop() # trailing form-feed artifact
|
||||
if not pages:
|
||||
return pages or None
|
||||
|
||||
|
||||
def _pdf_page_char_counts(path: str) -> Optional[list[int]]:
|
||||
"""Per-page extracted-text char counts, or None when undeterminable."""
|
||||
pages = _pdf_page_texts(path)
|
||||
if pages is None:
|
||||
return None
|
||||
return [len(page.strip()) for page in pages]
|
||||
|
||||
|
||||
def _page_ranges(pages: list[int]) -> str:
|
||||
"""Compact 1-based range list, e.g. '2-29, 33-35, 42'."""
|
||||
parts = [f"{a}-{b}" if a != b else str(a) for a, b in _group_ranges(pages)]
|
||||
if len(parts) > 12:
|
||||
parts = parts[:12] + ["…"]
|
||||
return ", ".join(parts)
|
||||
|
||||
|
||||
def _group_ranges(pages: list[int]) -> list[list[int]]:
|
||||
"""Group sorted 1-based page numbers into [start, end] runs."""
|
||||
ranges: list[list[int]] = []
|
||||
for p in pages:
|
||||
if ranges and p == ranges[-1][1] + 1:
|
||||
ranges[-1][1] = p
|
||||
else:
|
||||
ranges.append([p, p])
|
||||
parts = [f"{a}-{b}" if a != b else str(a) for a, b in ranges]
|
||||
if len(parts) > 12:
|
||||
parts = parts[:12] + ["…"]
|
||||
return ", ".join(parts)
|
||||
return ranges
|
||||
|
||||
|
||||
# Cap the per-gap breakdown so a pathological PDF (hundreds of alternating
|
||||
# text/scan pages) cannot balloon the warning. Ranges beyond the cap are
|
||||
# summarized in one line.
|
||||
PDF_GAP_MAP_MAX_ENTRIES = 20
|
||||
_GAP_CONTEXT_CHARS = 60
|
||||
|
||||
|
||||
def _gap_map(counts: list[int], texts: list[str], empty: list[int]) -> str:
|
||||
"""Per-gap breakdown: each empty range labeled with the last text seen
|
||||
before it (usually a section divider/header page), so the agent can
|
||||
decide WHICH gaps it actually needs to read instead of OCRing all of
|
||||
them."""
|
||||
ranges = _group_ranges(empty)
|
||||
lines: list[str] = []
|
||||
for a, b in ranges[:PDF_GAP_MAP_MAX_ENTRIES]:
|
||||
label = ""
|
||||
# Walk back to the nearest preceding page with text.
|
||||
for prev in range(a - 2, -1, -1):
|
||||
if counts[prev] >= PDF_EMPTY_PAGE_CHARS:
|
||||
snippet = " ".join(texts[prev].split())[:_GAP_CONTEXT_CHARS]
|
||||
label = f' — after "{snippet}" (p{prev + 1})'
|
||||
break
|
||||
span = f"page {a}" if a == b else f"pages {a}-{b}"
|
||||
n = b - a + 1
|
||||
lines.append(f" {span} ({n} page{'s' if n != 1 else ''}){label}")
|
||||
if len(ranges) > PDF_GAP_MAP_MAX_ENTRIES:
|
||||
rest = ranges[PDF_GAP_MAP_MAX_ENTRIES:]
|
||||
rest_pages = sum(b - a + 1 for a, b in rest)
|
||||
lines.append(f" … {len(rest)} more gaps ({rest_pages} pages)")
|
||||
return "\n".join(lines)
|
||||
|
||||
|
||||
def _pdf_coverage_note(path: str, display_path: Optional[str] = None) -> str:
|
||||
@@ -257,9 +300,10 @@ def _pdf_coverage_note(path: str, display_path: Optional[str] = None) -> str:
|
||||
for backend-transferred bytes); ``display_path`` is the path shown in
|
||||
the recovery command — the one the agent's terminal can actually see.
|
||||
"""
|
||||
counts = _pdf_page_char_counts(path)
|
||||
if not counts or len(counts) < 2:
|
||||
texts = _pdf_page_texts(path)
|
||||
if not texts or len(texts) < 2:
|
||||
return ""
|
||||
counts = [len(page.strip()) for page in texts]
|
||||
empty = [i + 1 for i, n in enumerate(counts) if n < PDF_EMPTY_PAGE_CHARS]
|
||||
total = len(counts)
|
||||
if len(empty) < PDF_COVERAGE_MIN_EMPTY:
|
||||
@@ -272,14 +316,18 @@ def _pdf_coverage_note(path: str, display_path: Optional[str] = None) -> str:
|
||||
shown = display_path or path
|
||||
return (
|
||||
"[EXTRACTION COVERAGE WARNING: "
|
||||
f"{len(empty)} of {total} pages in this PDF yielded no text "
|
||||
f"(pages {_page_ranges(empty)}). Those pages are likely scanned "
|
||||
"images (or blank) — their content is MISSING from the extracted "
|
||||
"text below, even where section headers appear with empty bodies. "
|
||||
"To read them: render pages to images with "
|
||||
f"{len(empty)} of {total} pages in this PDF yielded no text. "
|
||||
"Those pages are likely scanned images (or blank) — their content "
|
||||
"is MISSING from the extracted text below, even where section "
|
||||
"headers appear with empty bodies. Unreadable gaps, each labeled "
|
||||
"with the last text extracted before it:\n"
|
||||
f"{_gap_map(counts, texts, empty)}\n"
|
||||
"Decide which gaps you actually need — do NOT OCR or render "
|
||||
"everything. For the gaps that matter, render just that range with "
|
||||
f"`pdftoppm -jpeg -r 150 -f <first> -l <last> '{shown}' /tmp/page` "
|
||||
"and inspect each image with the vision_analyze tool, or use the "
|
||||
"ocr-and-documents skill (marker-pdf) for bulk OCR.]\n"
|
||||
"ocr-and-documents skill (marker-pdf) for bulk OCR of large "
|
||||
"ranges.]\n"
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -30,13 +30,16 @@ Extraction works with remote terminal backends (Docker, Modal, SSH): the file's
|
||||
|
||||
PDF conversion reads the **text layer only**. Pages that are scanned images — common in legal documents, resale packages, signed contracts, faxes — contain no text layer and silently convert to nothing. The telltale signature is section headers with empty bodies.
|
||||
|
||||
When a meaningful share of pages yields no text (over 20% of the document, or 10+ pages absolute), `read_file` prepends a warning to the extraction:
|
||||
When a meaningful share of pages yields no text (over 20% of the document, or 10+ pages absolute), `read_file` prepends a warning to the extraction. Each unreadable gap is labeled with the last text extracted before it — usually a section divider — so the agent can target only the gaps it actually needs instead of OCRing the whole document:
|
||||
|
||||
```
|
||||
[EXTRACTION COVERAGE WARNING: 198 of 311 pages in this PDF yielded no
|
||||
text (pages 2-29, 33-35, 42-77, 79-85, 92-213, 224, 226). Those pages
|
||||
are likely scanned images (or blank) — their content is MISSING from
|
||||
the extracted text below ...]
|
||||
text. ... Unreadable gaps, each labeled with the last text extracted
|
||||
before it:
|
||||
pages 42-77 (36 pages) — after "Antigua Maintenance Corp Bylaws" (p41)
|
||||
pages 92-213 (122 pages) — after "... Covenants, Codes and Regulations" (p91)
|
||||
page 224 (1 page) — after "... Insurance Declaration Pages" (p223)
|
||||
Decide which gaps you actually need — do NOT OCR or render everything. ...]
|
||||
```
|
||||
|
||||
The warning lists the exact page ranges and the recovery paths:
|
||||
|
||||
Reference in New Issue
Block a user