phase: 133_pdf_screenshot_fallback
All verification is complete — everything is green. No repo-side changes were needed in this pass (the phase's work in the working tree is correct and complete).
## Phase 133 final verification report
- **Verified (already implemented in working tree):** `is_usable_pdf_text` gate (0.30 XML-ish / 0.05 non-printable / empty), `render_pdf_pages`, `_fallback_page_screenshots` + `pdfs_screenshotted` counter in `app/rag/{pdf,importer}.py`; regression pin, unit/integration/E2E suites.
- **Completion criteria:**
1. Real guide clean through real pipeline — ✅ `test_pdf_hnb_guide_regression` + E2E preview/viewer checks (0 XML-ish lines, first-page text present).
2. Garbage PDF → vision fallback end to end — ✅ unit + E2E (stored content = canned per-page descriptions, garbage not stored, sync `detail` shape unchanged).
3. Clean PDF never burns a fallback vision call — ✅ unit + E2E (byte-faithful content).
4. Gates — ✅ `uv run pytest --cov=app --cov-report=term-missing`: **3129 passed, app/ 99%** (>90%); `uv run pytest tests/e2e/test_garbage_pdf.py -v --no-cov`: **3 passed** in isolation.
5. No behavior change to completed phases — ✅ entire suite green (all prior-phase tests included); `ruff check .` + `pyright` clean.
6. Production stale row — post-deploy re-sync verification (live infra, no repo work; no live changes made — deploy of `main` is the owner's step).
- **Documentation:** README.md reviewed in full — every env default cross-checked against `app/config.py`, page routes against `app/main.py`, scripts/Containerfile/compose flags, `.env.example`; the phase-133 PDF bullet added in the working tree is accurate. AGENTS.md verified consistent (all referenced files/commands exist). **No changes needed.**
- **Next pending phase:** `134_sync_cancel`.
This commit is contained in:
+297
-16
@@ -12,6 +12,7 @@ from __future__ import annotations
|
||||
import asyncio
|
||||
import hashlib
|
||||
import logging
|
||||
import tarfile
|
||||
from collections.abc import Callable, Sequence
|
||||
from datetime import timedelta
|
||||
from pathlib import Path
|
||||
@@ -29,12 +30,17 @@ from app.rag import pdf as pdf_mod
|
||||
from app.rag.importer import import_sources
|
||||
from app.rag.llm import EmbeddingError, LLMError
|
||||
from app.rag.pdf import (
|
||||
_XMLISH_LINE_RE,
|
||||
NONPRINT_CHAR_FRACTION,
|
||||
PAGE_SEPARATOR,
|
||||
XMLISH_LINE_FRACTION,
|
||||
PdfImage,
|
||||
_mime_from_image_info,
|
||||
_page_image_entries,
|
||||
extract_pdf_images,
|
||||
extract_pdf_text,
|
||||
is_usable_pdf_text,
|
||||
render_pdf_pages,
|
||||
)
|
||||
from tests.fakes import FakeEmbedder
|
||||
|
||||
@@ -475,13 +481,20 @@ def test_pdf_text_change_updates_doc_and_keeps_unchanged_images(
|
||||
_cleanup_source(db, "pdfchg")
|
||||
|
||||
|
||||
def test_pdf_without_text_still_becomes_a_document(
|
||||
def test_pdf_without_text_takes_the_screenshot_fallback(
|
||||
db, tmp_path: Path, caplog: pytest.LogCaptureFixture
|
||||
) -> None:
|
||||
"""An image-only PDF (no extractable text) still becomes a document
|
||||
— with empty content (the empty byte string's digest), no content
|
||||
chunks, and a warning (the signal). The summary path still runs
|
||||
(best-effort on the empty content)."""
|
||||
"""Phase 133 (task 03): an image-only PDF (an EMPTY text layer — the
|
||||
gate's other target) takes the page-screenshot fallback: the single
|
||||
page is rendered to a PNG and vision-described (the mock's canned
|
||||
:data:`DESCRIPTION`), and the doc lands with the ``---PAGE-N---``-
|
||||
anchored description as content (chunked + summarized as any other
|
||||
PDF). The phase-126 empty-content behavior is retired for layers
|
||||
that CAN be screenshotted — it survives only for an unopenable
|
||||
file (``test_corrupt_pdf_file_becomes_an_empty_document``), where
|
||||
the render fails too. The loud fallback warning fires; the phase-
|
||||
126 "no extractable text" warning does NOT (the doc is not empty).
|
||||
"""
|
||||
root = tmp_path / "pdfempty"
|
||||
root.mkdir()
|
||||
_build_pdf(root / "pics.pdf", [("", [_png_bytes(40, 30)])])
|
||||
@@ -490,41 +503,61 @@ def test_pdf_without_text_still_becomes_a_document(
|
||||
with caplog.at_level(logging.INFO, logger="app.importer"):
|
||||
summary = asyncio.run(import_sources([root], llm, session=db))
|
||||
assert (summary.files, summary.added) == (1, 1)
|
||||
assert summary.pdfs_screenshotted == 1
|
||||
doc = _doc(db, "pdfempty", "pics.pdf")
|
||||
assert doc.content == ""
|
||||
assert doc.content_hash == hashlib.sha256(b"").hexdigest()
|
||||
assert doc.content == f"{PAGE_SEPARATOR.format(page=1)}\n{DESCRIPTION}"
|
||||
assert doc.content_hash == hashlib.sha256(doc.content.encode("utf-8")).hexdigest()
|
||||
chunks = db.scalars(
|
||||
select(Chunk).where(Chunk.document_id == doc.id).order_by(Chunk.position)
|
||||
).all()
|
||||
assert [c for c in chunks if not c.is_summary] == []
|
||||
assert doc.summary is not None # best-effort on the empty content
|
||||
content_chunks = [c for c in chunks if not c.is_summary]
|
||||
assert [c.content for c in content_chunks] == [f"Page 1:\n{DESCRIPTION}"]
|
||||
assert doc.summary is not None
|
||||
# The loud fallback warning (the FINDING's signature) fired —
|
||||
# and the phase-126 "no extractable text" warning did NOT (the
|
||||
# doc is not empty — the screenshots ARE the content).
|
||||
warnings = [
|
||||
r
|
||||
for r in caplog.records
|
||||
if "no extractable text" in r.getMessage()
|
||||
r for r in caplog.records if "text layer unusable" in r.getMessage()
|
||||
]
|
||||
assert len(warnings) == 1 and warnings[0].levelno == logging.WARNING
|
||||
assert "falling back to 1 page screenshots" in warnings[0].getMessage()
|
||||
assert not [
|
||||
r for r in caplog.records if "no extractable text" in r.getMessage()
|
||||
]
|
||||
finally:
|
||||
_cleanup_source(db, "pdfempty")
|
||||
|
||||
|
||||
def test_corrupt_pdf_file_becomes_an_empty_document(db, tmp_path: Path) -> None:
|
||||
def test_corrupt_pdf_file_becomes_an_empty_document(
|
||||
db, tmp_path: Path, caplog: pytest.LogCaptureFixture
|
||||
) -> None:
|
||||
"""A .pdf file that is not a PDF at all: the parser's fail-soft
|
||||
contract (no text, no images) — the document lands with empty
|
||||
content and no chunks, and the sync stays green."""
|
||||
contract (no text, no images) AND the phase-133 fallback's
|
||||
render-less branch (``render_pdf_pages`` → ``[]`` → the helper's
|
||||
``""``) — the document lands with empty content (the empty byte
|
||||
string's digest) and no chunks, the fallback is still counted
|
||||
(the gate fired, the render produced nothing), and the phase-126
|
||||
"no extractable text" warning fires — the doc really is empty."""
|
||||
root = tmp_path / "pdfbad"
|
||||
root.mkdir()
|
||||
(root / "broken.pdf").write_bytes(b"this is not a pdf at all")
|
||||
llm = _pdf_llm(tmp_path)
|
||||
try:
|
||||
summary = asyncio.run(import_sources([root], llm, session=db))
|
||||
with caplog.at_level(logging.INFO, logger="app.importer"):
|
||||
summary = asyncio.run(import_sources([root], llm, session=db))
|
||||
assert (summary.files, summary.added, summary.errors) == (1, 1, 0)
|
||||
assert summary.pdfs_screenshotted == 1 # the gate fired
|
||||
doc = _doc(db, "pdfbad", "broken.pdf")
|
||||
assert doc.content == "" and doc.title == "broken"
|
||||
assert doc.content_hash == hashlib.sha256(b"").hexdigest()
|
||||
chunks = db.scalars(
|
||||
select(Chunk).where(Chunk.document_id == doc.id).order_by(Chunk.position)
|
||||
).all()
|
||||
assert [c for c in chunks if not c.is_summary] == []
|
||||
warnings = [
|
||||
r for r in caplog.records if "no extractable text" in r.getMessage()
|
||||
]
|
||||
assert len(warnings) == 1 and warnings[0].levelno == logging.WARNING
|
||||
finally:
|
||||
_cleanup_source(db, "pdfbad")
|
||||
|
||||
@@ -1074,3 +1107,251 @@ def test_page_image_entries_falls_back_on_old_pymupdf() -> None:
|
||||
)
|
||||
entries = _page_image_entries(page)
|
||||
assert entries == [{"xref": 7, "width": 40, "height": 30}]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# is_usable_pdf_text (phase 133 task 02 — the deterministic garbage gate)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
#: The owner's untracked fixture (the stable reference — the same
|
||||
#: provenance as the phase-133 task-01 integration regression pin:
|
||||
#: the tarball, never the extracted PDF, is what the test starts
|
||||
#: from, and it never touches the repo).
|
||||
TARBALL = Path(__file__).resolve().parents[2] / "test_sources" / "hnb_2026_guide.tar.gz"
|
||||
|
||||
#: The inner file's name (it keeps its name through extraction — the
|
||||
#: task-01 convention).
|
||||
PDF_NAME = "HBI-2026-Guide-Year-Round_260704_131221.pdf"
|
||||
|
||||
|
||||
@pytest.fixture(scope="module")
|
||||
def pre126_garbage_text(tmp_path_factory: pytest.TempPathFactory) -> str:
|
||||
"""THE REAL GARBAGE (the task's airtight pin, module-scoped):
|
||||
the owner's exact PDF read with the PRE-PHASE-126 recipe —
|
||||
``read_text(encoding="utf-8", errors="replace")`` +
|
||||
``.replace("\\x00", "")`` — the 46%-XML-ish, U+FFFD-heavy soup
|
||||
that was chunked and embedded before the parser shipped (the
|
||||
FINDING: 138,170 non-empty lines, ~46% XML-ish, the owner's
|
||||
``<illustrator:StartupProfile>`` preview). The tarball's single
|
||||
member is extracted into a temp dir (the task-01 integration
|
||||
pin's pattern, shared — the fixture stays out of the repo)."""
|
||||
if not TARBALL.is_file():
|
||||
pytest.skip(
|
||||
"test_sources/hnb_2026_guide.tar.gz is missing — the "
|
||||
"untracked owner fixture (2.6 MB, one 48-page Illustrator PDF)"
|
||||
)
|
||||
root = tmp_path_factory.mktemp("pre126")
|
||||
with tarfile.open(TARBALL, "r:gz") as tar:
|
||||
tar.extractall(root, filter="data")
|
||||
member = root / PDF_NAME
|
||||
return member.read_text(encoding="utf-8", errors="replace").replace("\x00", "")
|
||||
|
||||
|
||||
def test_gate_clean_text_with_page_separators_is_usable() -> None:
|
||||
"""Real prose + the phase-126 ``---PAGE-N---`` separators: the
|
||||
happy path the gate must NEVER block (a clean PDF never burns a
|
||||
vision call — the gate is the only gate, and the happy path stays
|
||||
byte-identical to phase 126)."""
|
||||
clean = (
|
||||
f"{PAGE_SEPARATOR.format(page=1)}\n2026 U.S. Benefits Guide\n"
|
||||
"\n"
|
||||
"Covered employees receive comprehensive medical, dental, and\n"
|
||||
"vision benefits under this plan document.\n"
|
||||
"\n"
|
||||
f"{PAGE_SEPARATOR.format(page=2)}\nPremiums are deducted from\n"
|
||||
"each paycheck; the plan year runs January 1 through\n"
|
||||
"December 31."
|
||||
)
|
||||
assert is_usable_pdf_text(clean) is True
|
||||
|
||||
|
||||
def test_gate_empty_text_layer_is_unusable() -> None:
|
||||
"""The empty text layer — a scanned/image-only PDF has nothing to
|
||||
read — is the fallback's other target."""
|
||||
assert is_usable_pdf_text("") is False
|
||||
assert is_usable_pdf_text(" \n\t\n \r\n") is False
|
||||
|
||||
|
||||
def test_gate_xml_heavy_layer_is_unusable() -> None:
|
||||
"""The raw-XMP / binary-soup signature: ≥ 30% of the non-empty
|
||||
lines look like tags (``<tag>``, ``</tag>``, ``<tag attr=`` — the
|
||||
regex covers them all, including the PDF-structure ``<</Key…``
|
||||
noise)."""
|
||||
tags = [f"<xmp:Prop{i}>value</xmp:Prop{i}>" for i in range(30)]
|
||||
prose = [f"Line {i} of ordinary prose." for i in range(70)]
|
||||
assert is_usable_pdf_text("\n".join(tags + prose)) is False # 30/100
|
||||
|
||||
|
||||
def test_gate_borderline_document_about_xml_stays_usable() -> None:
|
||||
"""ASSUMPTION A8's edge from below: a real document ABOUT XML
|
||||
whose tag-like lines stay UNDER the 0.30 bar (29%) is usable —
|
||||
the documented false-positive cost is paid only at/above the bar."""
|
||||
tags = [f"<example:{i}>text</example:{i}>" for i in range(29)]
|
||||
prose = [
|
||||
f"Paragraph {i} explains the markup in plain words."
|
||||
for i in range(71)
|
||||
]
|
||||
assert is_usable_pdf_text("\n".join(tags + prose)) is True # 29/100
|
||||
|
||||
|
||||
def test_gate_at_and_above_the_xml_bar_is_unusable() -> None:
|
||||
"""The threshold's edges from above: EXACTLY 30% tag-like lines is
|
||||
already unusable (the bar is ≥, not >), and 31% is too — a document
|
||||
ABOUT XML that reaches the bar pays the documented ASSUMPTION A8
|
||||
cost (the fallback still extracts it correctly via vision)."""
|
||||
for n in (30, 31):
|
||||
tags = [f"<example:{i}>text</example:{i}>" for i in range(n)]
|
||||
prose = [
|
||||
f"Paragraph {i} of ordinary explanation."
|
||||
for i in range(100 - n)
|
||||
]
|
||||
assert is_usable_pdf_text("\n".join(tags + prose)) is False
|
||||
|
||||
|
||||
def test_gate_replacement_char_soup_is_unusable() -> None:
|
||||
"""The SECOND signal, alone: a ≥ 5% U+FFFD load (the
|
||||
``errors="replace"`` binary-soup signal) is unusable even with few
|
||||
tag-like lines — the XML-ish gate does not mask it, and this soup
|
||||
has none (the two failure signals fire independently)."""
|
||||
soup = "Benefits summary paragraph. " * 24 + "\ufffd" * 60 # ~7.9%
|
||||
assert is_usable_pdf_text(soup) is False
|
||||
|
||||
|
||||
def test_gate_structural_newlines_and_tabs_are_printable() -> None:
|
||||
"""The documented exclusion: the ``\\n``/``\\t``/``\\r`` the
|
||||
extractor legitimately emits are NOT non-printable — a
|
||||
newline/tab/indent-heavy document is printable (counting them
|
||||
would flag every prose document)."""
|
||||
text = "\n\t".join(f"Indented line {i} with a tab." for i in range(20))
|
||||
assert is_usable_pdf_text(text) is True
|
||||
|
||||
|
||||
def test_gate_real_pre126_garbage_is_unusable(
|
||||
pre126_garbage_text: str,
|
||||
) -> None:
|
||||
"""THE REAL GARBAGE (the airtight pin): the owner's exact file,
|
||||
read with the pre-126 recipe (the module fixture) — the
|
||||
reproduced 46%-XML-ish, U+FFFD-heavy soup — must be flagged
|
||||
unusable. Both signals fire on the REAL string (the FINDING's
|
||||
evidence, pinned against the named thresholds: if the owner ever
|
||||
re-cuts the fixture, the drift shows up here — the pin has teeth,
|
||||
the same contract as the task-01 suite)."""
|
||||
assert is_usable_pdf_text(pre126_garbage_text) is False
|
||||
# The owner's exact preview line is in the reproduced soup.
|
||||
assert "<illustrator:StartupProfile>" in pre126_garbage_text
|
||||
lines = [line for line in pre126_garbage_text.splitlines() if line.strip()]
|
||||
xml_frac = sum(1 for line in lines if _XMLISH_LINE_RE.search(line)) / len(lines)
|
||||
nonprint_frac = sum(
|
||||
1
|
||||
for ch in pre126_garbage_text
|
||||
if ch == "\ufffd" or (ord(ch) < 32 and ch not in "\t\n\r")
|
||||
) / len(pre126_garbage_text)
|
||||
# Both signals independently clear their named bars on the real
|
||||
# string — neither signal can silently die.
|
||||
assert xml_frac >= XMLISH_LINE_FRACTION
|
||||
assert nonprint_frac >= NONPRINT_CHAR_FRACTION
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# render_pdf_pages (phase 133 task 03 — the fallback's visual input)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _two_page_text_pdf(path: Path) -> Path:
|
||||
"""A 2-page temp PDF with EXPLICIT letter pages (612x792pt —
|
||||
deterministic render sizes: 1275x1650 px at 150 DPI, 612x792 at
|
||||
72 DPI) and page-specific text (the render must be page-specific,
|
||||
so the pages' PNG bytes must differ)."""
|
||||
doc = fitz.open()
|
||||
doc.new_page(width=612, height=792).insert_text((72, 72), "First page marker text.")
|
||||
doc.new_page(width=612, height=792).insert_text((72, 72), "Second page marker text.")
|
||||
doc.save(path)
|
||||
doc.close()
|
||||
return path
|
||||
|
||||
|
||||
def test_render_pdf_pages_returns_one_png_per_page_in_order(
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
"""A 2-page PDF → 2 PNGs in PAGE ORDER (list index i is page i+1),
|
||||
each a genuine decodable PNG at the default 150-DPI render (612x792pt
|
||||
letter → 1275x1650 px), and page-specific (page 1's bytes != page
|
||||
2's — different text on the page)."""
|
||||
path = _two_page_text_pdf(tmp_path / "two.pdf")
|
||||
pages = render_pdf_pages(path)
|
||||
assert len(pages) == 2
|
||||
for png in pages:
|
||||
assert png.startswith(PNG_MAGIC)
|
||||
pix = fitz.Pixmap(png)
|
||||
assert (pix.width, pix.height) == (1275, 1650)
|
||||
assert pages[0] != pages[1] # the render is page-specific
|
||||
|
||||
|
||||
def test_render_pdf_pages_honors_the_dpi_argument(tmp_path: Path) -> None:
|
||||
"""ASSUMPTION A8's fixed 150 DPI is the DEFAULT; an explicit *dpi*
|
||||
changes the render (72 DPI → the page's natural 612x792 px)."""
|
||||
path = _two_page_text_pdf(tmp_path / "two.pdf")
|
||||
pix = fitz.Pixmap(render_pdf_pages(path, dpi=72)[0])
|
||||
assert (pix.width, pix.height) == (612, 792)
|
||||
|
||||
|
||||
def test_render_pdf_pages_unopenable_file_returns_empty_list(
|
||||
tmp_path: Path, caplog: pytest.LogCaptureFixture,
|
||||
) -> None:
|
||||
"""A file that cannot be OPENED yields ``[]`` (the caller's existing
|
||||
empty-content path — the doc lands with the no-extractable-text
|
||||
behavior, unchanged) + a warning (the fail-soft style)."""
|
||||
bad = tmp_path / "bad.pdf"
|
||||
bad.write_bytes(b"this is not a pdf at all")
|
||||
with caplog.at_level(logging.WARNING, logger="app.rag.pdf"):
|
||||
assert render_pdf_pages(bad) == []
|
||||
assert render_pdf_pages(tmp_path / "missing.pdf") == []
|
||||
warnings = [
|
||||
r for r in caplog.records if "could not open" in r.getMessage()
|
||||
]
|
||||
assert len(warnings) == 2 and all(w.levelno == logging.WARNING for w in warnings)
|
||||
|
||||
|
||||
def test_render_pdf_pages_zero_page_doc_returns_empty_list(
|
||||
monkeypatch: pytest.MonkeyPatch, tmp_path: Path,
|
||||
) -> None:
|
||||
"""A zero-page PDF (openable, no pages — this PyMuPDF refuses to
|
||||
SAVE such a file, so it is stubbed) yields ``[]`` — the loop runs
|
||||
zero times, nothing renders."""
|
||||
path = tmp_path / "zero.pdf"
|
||||
path.write_bytes(b"%PDF-1.4 fake")
|
||||
monkeypatch.setattr(pdf_mod.fitz, "open", lambda _p: _FakeDoc([]))
|
||||
assert render_pdf_pages(path) == []
|
||||
|
||||
|
||||
def test_render_pdf_pages_broken_page_is_zero_bytes_at_its_index(
|
||||
monkeypatch: pytest.MonkeyPatch, tmp_path: Path,
|
||||
) -> None:
|
||||
"""A page whose get_pixmap raises → ``b""`` at ITS index (the
|
||||
caller's placeholder slot — the page index NEVER shifts, so
|
||||
``---PAGE-N---`` stays truthful); the good pages keep their PNGs
|
||||
at their own indices, and the failure is logged (the
|
||||
:func:`extract_pdf_text` fail-soft style)."""
|
||||
path = tmp_path / "fake.pdf"
|
||||
path.write_bytes(b"%PDF-1.4 fake")
|
||||
|
||||
class _RenderPage:
|
||||
def __init__(self, data: bytes | None) -> None:
|
||||
self._data = data
|
||||
|
||||
def get_pixmap(self, dpi: int) -> Any:
|
||||
if self._data is None:
|
||||
raise RuntimeError("boom: page render failed")
|
||||
return SimpleNamespace(tobytes=lambda _fmt: self._data)
|
||||
|
||||
doc = _FakeDoc(
|
||||
[
|
||||
_RenderPage(b"\x89PNG-fake-page-1"),
|
||||
_RenderPage(None),
|
||||
_RenderPage(b"\x89PNG-fake-page-3"),
|
||||
]
|
||||
)
|
||||
monkeypatch.setattr(pdf_mod.fitz, "open", lambda _p: doc)
|
||||
assert render_pdf_pages(path) == [
|
||||
b"\x89PNG-fake-page-1", b"", b"\x89PNG-fake-page-3",
|
||||
]
|
||||
|
||||
Reference in New Issue
Block a user