fix(read_file): warn when PDF pages yield no text (scanned-image coverage gap)
anydoc converts the PDF text layer only and emits no image placeholders or page markers, so a mostly-scanned PDF extracts 'successfully' into section headers with empty bodies — silent data loss the model cannot detect. Count per-page text via poppler pdftotext and prepend an EXTRACTION COVERAGE WARNING naming the empty pages and the recovery path (pdftoppm + vision_analyze, or the ocr-and-documents skill). Found on a 311-page HOA resale package where 198 scanned pages (CC&Rs, Bylaws, Articles, insurance certs) vanished without a trace.
This commit is contained in:
parent
72eda946be
commit
89c14aeb9e
|
|
@ -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**:
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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": {
|
||||
|
|
|
|||
|
|
@ -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 <first> -l <last> '{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:
|
||||
|
|
|
|||
Loading…
Reference in New Issue