From 7c2bc87f8153907061a649e58e7f65fcde5ce1ea Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sat, 8 Aug 2026 05:33:27 -0700 Subject: [PATCH] feat(read_extract): label each unreadable PDF gap with its preceding section text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- tests/tools/test_read_extract.py | 48 +++++++++++- tools/read_extract.py | 78 +++++++++++++++---- .../features/document-extraction.md | 11 ++- 3 files changed, 115 insertions(+), 22 deletions(-) diff --git a/tests/tools/test_read_extract.py b/tests/tools/test_read_extract.py index 4384cc5af2..d96de13ae0 100644 --- a/tests/tools/test_read_extract.py +++ b/tests/tools/test_read_extract.py @@ -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), "") diff --git a/tools/read_extract.py b/tools/read_extract.py index 6c2f299379..df09c3def2 100644 --- a/tools/read_extract.py +++ b/tools/read_extract.py @@ -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 -l '{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" ) diff --git a/website/docs/user-guide/features/document-extraction.md b/website/docs/user-guide/features/document-extraction.md index 3d5f1eba0f..e3872a8e70 100644 --- a/website/docs/user-guide/features/document-extraction.md +++ b/website/docs/user-guide/features/document-extraction.md @@ -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: