diff --git a/skills/productivity/ocr-and-documents/SKILL.md b/skills/productivity/ocr-and-documents/SKILL.md index 7f6e7bf2c54e3..5d7278fa07a78 100644 --- a/skills/productivity/ocr-and-documents/SKILL.md +++ b/skills/productivity/ocr-and-documents/SKILL.md @@ -18,6 +18,8 @@ For PPTX: see the `powerpoint` skill (full create/read/edit support). For PDF manipulation (merge, split, forms, watermarks, creation): see the `pdf` skill. This skill covers **text extraction from PDFs and scanned documents**. +> **Coming from a `read_file` EXTRACTION COVERAGE WARNING?** `read_file` auto-converts local PDFs but reads the text layer only; the warning footer lists the pages that yielded no text (scanned images). For a handful of pages, render + vision is fastest: `pdftoppm -jpeg -r 150 -f N -l N file.pdf /tmp/page` then `vision_analyze` each image. For bulk OCR of many pages, use marker-pdf below (Step 2). + ## Step 1: Remote URL Available? If the document has a URL, **always try `web_extract` first**: diff --git a/skills/productivity/pdf/SKILL.md b/skills/productivity/pdf/SKILL.md index 23d9730874122..cf55b9e3a6242 100644 --- a/skills/productivity/pdf/SKILL.md +++ b/skills/productivity/pdf/SKILL.md @@ -158,6 +158,7 @@ Read [forms.md](forms.md) first — it distinguishes fillable (AcroForm) PDFs fr ## Pitfalls +- `read_file` auto-converts PDFs (via the optional anydoc converter) but reads the **text layer only**. A mostly-scanned PDF converts "successfully" into section headers with empty bodies; when that happens read_file appends an `EXTRACTION COVERAGE WARNING` footer listing the pages that yielded no text. Recover those pages with `pdftoppm -jpeg -r 150 -f N -l N file.pdf /tmp/page` + `vision_analyze`, or bulk-OCR via the `ocr-and-documents` skill. - `page.extract_text()` returns `None` on image-only pages — guard with `or ""` and fall back to OCR. - pypdf preserves encryption flags: reading an encrypted PDF requires `PdfReader(path, password=...)` before pages are accessible. - reportlab coordinates are bottom-left origin, points (1/72″) — not top-left. diff --git a/tests/tools/test_read_extract.py b/tests/tools/test_read_extract.py index 038d073bb3e8f..1c4b4fe3f405d 100644 --- a/tests/tools/test_read_extract.py +++ b/tests/tools/test_read_extract.py @@ -485,5 +485,110 @@ class TestReadFileToolIntegration(unittest.TestCase): self.assertIn("Report body", res["content"]) +# --------------------------------------------------------------------------- +# Scanned-PDF coverage warning +# --------------------------------------------------------------------------- + +class TestPdfCoverageNote(unittest.TestCase): + """The coverage footer flags PDFs whose pages yielded no text.""" + + def _note_with_counts(self, counts): + from tools import read_extract + with mock.patch.object(read_extract, "_pdf_page_char_counts", + return_value=counts): + return read_extract._pdf_coverage_note("/x/doc.pdf") + + def test_mostly_scanned_pdf_warns_with_page_ranges(self): + # 3 text pages then 6 empty ones (scanned) — well past the ratio. + 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("vision_analyze", note) # recovery path is named + self.assertIn("ocr-and-documents", note) + + def test_full_text_pdf_is_silent(self): + self.assertEqual(self._note_with_counts([500] * 20), "") + + def test_one_blank_page_is_tolerated(self): + # A single separator/blank page in a text PDF should not warn. + self.assertEqual(self._note_with_counts([500, 0, 500, 500]), "") + + def test_small_share_below_ratio_and_absolute_is_silent(self): + # 3 empty of 40 (7.5% < 20%, and < absolute threshold of 10). + counts = [400] * 37 + [0, 0, 0] + self.assertEqual(self._note_with_counts(counts), "") + + def test_large_absolute_count_warns_even_below_ratio(self): + # 12 empty of 100 (12% < 20% ratio) still warns: 12 lost pages + # is real data loss regardless of document size. + counts = [400] * 88 + [0] * 12 + note = self._note_with_counts(counts) + self.assertIn("12 of 100 pages", note) + + def test_undeterminable_counts_are_silent(self): + self.assertEqual(self._note_with_counts(None), "") + self.assertEqual(self._note_with_counts([0]), "") # single page + + def test_page_ranges_compact(self): + from tools.read_extract import _page_ranges + self.assertEqual(_page_ranges([2, 3, 4, 7, 9, 10]), "2-4, 7, 9-10") + self.assertEqual(_page_ranges([5]), "5") + + def test_page_char_counts_missing_pdftotext(self): + from tools import read_extract + with mock.patch.object(read_extract.shutil, "which", return_value=None): + self.assertIsNone(read_extract._pdf_page_char_counts("/x/doc.pdf")) + + def test_page_char_counts_parses_formfeeds(self): + from tools import read_extract + fake = mock.Mock(returncode=0, stdout=b"alpha beta\fgamma\f\f") + with mock.patch.object(read_extract.shutil, "which", + return_value="/usr/bin/pdftotext"), \ + mock.patch.object(read_extract.subprocess, "run", + return_value=fake): + counts = read_extract._pdf_page_char_counts("/x/doc.pdf") + # Trailing empty segment after the final \f is dropped; the real + # empty page between the two \f markers is preserved. + self.assertEqual(counts, [len("alpha beta"), len("gamma"), 0]) + + def test_extract_anydoc_prepends_note_for_pdf(self): + """The warning leads the extracted text for .pdf inputs (a trailing + footer would land on a page the model may never fetch).""" + from tools import read_extract + fake_mod = mock.Mock() + fake_mod.to_markdown.return_value = "# Title\n\nBody" + with tempfile.NamedTemporaryFile(suffix=".pdf", delete=False) as fh: + fh.write(b"%PDF-1.4 fake") + p = fh.name + try: + with mock.patch.object(read_extract, "_anydoc", + return_value=fake_mod), \ + mock.patch.object(read_extract, "_pdf_coverage_note", + return_value="[EXTRACTION COVERAGE WARNING: test]\n"): + text = read_extract._extract_anydoc(p) + finally: + os.unlink(p) + self.assertTrue(text.startswith("[EXTRACTION COVERAGE WARNING")) + self.assertIn("# Title", text) + + def test_extract_anydoc_no_note_for_non_pdf(self): + from tools import read_extract + fake_mod = mock.Mock() + fake_mod.to_markdown.return_value = "converted" + with tempfile.NamedTemporaryFile(suffix=".rtf", delete=False) as fh: + fh.write(b"{\\rtf1 fake}") + p = fh.name + try: + with mock.patch.object(read_extract, "_anydoc", + return_value=fake_mod), \ + mock.patch.object(read_extract, "_pdf_coverage_note") as note: + text = read_extract._extract_anydoc(p) + finally: + os.unlink(p) + note.assert_not_called() + self.assertEqual(text, "converted\n") + + if __name__ == "__main__": unittest.main() diff --git a/tools/file_tools.py b/tools/file_tools.py index 35782cdf7fd13..65a5c57990b44 100644 --- a/tools/file_tools.py +++ b/tools/file_tools.py @@ -2418,7 +2418,7 @@ def _check_file_reqs(): READ_FILE_SCHEMA = { "name": "read_file", - "description": "Read a text file with line numbers and pagination. Use this instead of cat/head/tail in terminal. Output format: 'LINE_NUM|CONTENT'. Suggests similar filenames if not found. Use offset and limit for large files. Reads exceeding ~100K characters are truncated on a line boundary and return a next_offset; continue with offset to read the rest. Jupyter notebooks (.ipynb), Word documents (.docx), and Excel workbooks (.xlsx) are auto-extracted to readable text; PDF, legacy Office (.doc/.ppt/.xls), OpenDocument, RTF, and EPUB convert too when the optional anydoc converter is available (auto-installed on first use where installs are permitted). NOTE: Cannot read images or other binary files — use vision_analyze for images.", + "description": "Read a text file with line numbers and pagination. Use this instead of cat/head/tail in terminal. Output format: 'LINE_NUM|CONTENT'. Suggests similar filenames if not found. Use offset and limit for large files. Reads exceeding ~100K characters are truncated on a line boundary and return a next_offset; continue with offset to read the rest. Jupyter notebooks (.ipynb), Word documents (.docx), and Excel workbooks (.xlsx) are auto-extracted to readable text; PDF, legacy Office (.doc/.ppt/.xls), OpenDocument, RTF, and EPUB convert too when the optional anydoc converter is available (auto-installed on first use where installs are permitted). PDF conversion reads the text layer only: scanned/image pages yield no text, and when many pages come back empty the output ends with an EXTRACTION COVERAGE WARNING listing the affected pages — follow its instructions (render pages with pdftoppm and inspect via vision_analyze, or OCR) instead of treating the extraction as complete. NOTE: Cannot read images or other binary files — use vision_analyze for images.", "parameters": { "type": "object", "properties": { diff --git a/tools/read_extract.py b/tools/read_extract.py index 0ec42a17308db..8f55528905233 100644 --- a/tools/read_extract.py +++ b/tools/read_extract.py @@ -16,6 +16,8 @@ import importlib import json import os import posixpath +import shutil +import subprocess import threading import time import zipfile @@ -145,7 +147,96 @@ def _extract_anydoc(path: str) -> str: raise ExtractionError(f"{type(exc).__name__}: {exc}") from exc if not isinstance(text, str) or not text.strip(): raise ExtractionError("Document contains no extractable text") - return text.rstrip("\n") + "\n" + text = text.rstrip("\n") + "\n" + if Path(path).suffix.lower() == ".pdf": + note = _pdf_coverage_note(path) + if note: + # Prepend: read_file paginates the extraction, so a footer on a + # long document would sit on a page the model may never fetch. + text = note + text + return text + + +# ── Scanned-PDF coverage detection ────────────────────────────────── +# +# anydoc (like every text-layer extractor) returns nothing for scanned +# image pages and emits no image placeholders or page markers, so a +# mostly-scanned PDF converts "successfully" into a few headers with +# empty bodies — silent data loss the model cannot detect. Count per-page +# text via poppler's pdftotext (form-feed page separators) and append a +# loud footer when a meaningful share of pages yielded no text. + +# A page with fewer extracted characters than this is considered empty. +PDF_EMPTY_PAGE_CHARS = 20 +# Warn when at least this many pages are empty AND they exceed the ratio, +# or when the absolute count alone is overwhelming. +PDF_COVERAGE_MIN_EMPTY = 2 +PDF_COVERAGE_MIN_RATIO = 0.2 +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.""" + if shutil.which("pdftotext") is None: + return None + try: + proc = subprocess.run( + ["pdftotext", path, "-"], + capture_output=True, + timeout=PDF_PAGE_SCAN_TIMEOUT, + ) + except (OSError, subprocess.SubprocessError): + return None + if proc.returncode != 0: + return None + 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 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'.""" + 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) + + +def _pdf_coverage_note(path: str) -> str: + """A warning footer when many PDF pages produced no text, else ''.""" + counts = _pdf_page_char_counts(path) + if not counts or len(counts) < 2: + return "" + 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: + return "" + if ( + len(empty) / total < PDF_COVERAGE_MIN_RATIO + and len(empty) < PDF_COVERAGE_ABSOLUTE_EMPTY + ): + return "" + 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"`pdftoppm -jpeg -r 150 -f -l '{path}' /tmp/page` " + "and inspect each image with the vision_analyze tool, or use the " + "ocr-and-documents skill (marker-pdf) for bulk OCR.]\n" + ) def _source_text(source) -> str: