Everything is verified green. Final report: ## Phase 140 final verification pass — report - Verified all 4 tasks already complete (code in working tree, task files in `complete/`): `pictorial_pages` gate + 2 settings (`BOR_PDF_PICTORIAL`, `BOR_PDF_PICTORIAL_MIN_CHARS` + validator), `PICTORIAL_PAGE_MODE`/`describe_pdf_page` seam + `render_pdf_pages` page selector + mock branch, `_pictorial_pdf_pass` in `_index_pdf_file` with log-only counters, dedicated E2E. No code changes needed. - Test results (exact commands): - `uv run pytest --cov=app --cov-report=term-missing` → **3320 passed, 11 skipped, 0 failed; TOTAL 99%** (>90% gate) - `uv run pytest tests/e2e/test_pictorial_pdf.py -v --no-cov` → **3 passed in isolation** (Playwright, mock-only) - Pre-existing suites unedited: `test_pdf_support.py` 6 passed; `test_garbage_pdf.py` 2 passed + 1 skipped (owner fixture); `test_pdf_image_context.py` 4 passed - `uv run ruff check .` → All checks passed; `uv run pyright` → 0 errors - Completion criteria: 1. IKEA tar import repro — pinned by 2 integration tests; **skipped loudly here** (`test_sources/ikea_instructions.tar` absent on this machine — verified repo-wide; same house convention as completed phase-133 HNB tests; runs real on the owner's machine). Equivalent logic fully covered by unit pins on synthetic IKEA-class (vector-drawn, zero-raster) PDFs. 2. Chat grounded on described page — **green** (E2E test b: phrase-only-in-illustration question, PDF in `done.sources`, answer echoes the block) 3. Clean-text no-op (zero vision calls) + garbage precedence pin — **green** (`test_zero_candidates_is_a_noop` unit; `test_garbage_pdf_with_pictorial_on_takes_the_fallback_not_the_pass` integration, verified alone) 4. `BOR_PDF_PICTORIAL=0` byte-identical — **green** (unit pin on synthetic PDF; IKEA variant runs on owner's machine) 5. Full gate + E2E isolation + pre-existing suites — **green** (above) 6. ruff + pyright — **clean** 7. Commit/phase-move — delegated to harness per run rules (left in working tree: 16 modified + 8 new files, uncommitted) - Deviations: none beyond the expected owner-fixture skips (11, all pre-existing fixture-missing skips, including 2 IKEA). - Next pending phase: **none** — `todo/` holds only 140; 139 is already complete.
451 lines
17 KiB
Python
451 lines
17 KiB
Python
"""Phase 140, task 02 — the ``PICTORIAL_PAGE_MODE`` vision seam + the
|
|
``render_pdf_pages`` page selector (unit).
|
|
|
|
Pins the task-02 contracts:
|
|
|
|
* ``PICTORIAL_PAGE_MODE`` / ``PICTORIAL_INSTRUCTION`` /
|
|
``PICTORIAL_PROMPT`` — the marker convention next to
|
|
``IMAGE_DESCRIPTION_MODE`` (marker first, the deterministic E2E mock
|
|
LLM keys on it, PLAN §6) with the LOCKED wording (phase 140 design:
|
|
describe in detail what the page shows — the assembly steps, part
|
|
names, arrows, tool icons, before/after states — ESPECIALLY the
|
|
information the extracted text does not contain).
|
|
* ``DESCRIBE_PROMPT`` byte-identity (regression pin — the phase 122/126/
|
|
137 suites pass unedited; the pictorial pass has its OWN marker, it
|
|
never re-wraps the image prompt).
|
|
* :func:`describe_pdf_page` — the same one-chat-call multimodal
|
|
contract as :func:`describe_image`: a capturing fake LLM pins the
|
|
message shape (the text part starts with ``PICTORIAL_PROMPT``, then
|
|
the page block — the page's own text as context, the
|
|
``"(none)`` case; the ``image_url`` part is the data URL from the
|
|
PNG bytes + mime), the CHAT (vision) model (never ``lite``), the
|
|
phase-30 cap, and ``None`` on an empty reply or a client error
|
|
(fail-soft — one dead page never stalls a sync).
|
|
* :func:`render_pdf_pages`' optional 1-based *pages* selector — the
|
|
subset's count + ascending order + PNG magic bytes (a 3-page
|
|
fixture, the phase-126/133 recipe), ``None`` = every page (the
|
|
phase-133 caller stays byte-identical), an unreadable/out-of-range
|
|
named page → the zero-byte placeholder at its position, an
|
|
unopenable file → ``[]``.
|
|
"""
|
|
from __future__ import annotations
|
|
|
|
import asyncio
|
|
import base64
|
|
import logging
|
|
from pathlib import Path
|
|
from types import SimpleNamespace
|
|
from typing import Any
|
|
|
|
import pymupdf as fitz
|
|
import pytest
|
|
|
|
from app.config import Settings
|
|
from app.rag import pdf as pdf_mod
|
|
from app.rag.llm import LLMError
|
|
from app.rag.pdf import render_pdf_pages
|
|
from app.rag.summarizer import (
|
|
DESCRIBE_PROMPT,
|
|
IMAGE_DESCRIPTION_MODE,
|
|
PICTORIAL_INSTRUCTION,
|
|
PICTORIAL_PAGE_MODE,
|
|
PICTORIAL_PROMPT,
|
|
describe_pdf_page,
|
|
)
|
|
|
|
PNG_MAGIC = b"\x89PNG"
|
|
|
|
#: A tiny non-empty PNG-ish payload for the describe-call's ``data``
|
|
#: (the seam is content-agnostic — it never parses the image; a
|
|
#: well-formed magic keeps the data-URL assertion honest).
|
|
PNG_1X1 = base64.b64decode(
|
|
"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAAC0lEQVR4nGP4DwQACfsD/fteaysAAAAASUVORK5CYII="
|
|
)
|
|
|
|
|
|
def _settings(**kwargs: Any) -> Settings:
|
|
"""Build Settings without reading a .env file (deterministic tests)."""
|
|
kwargs.setdefault("_env_file", None)
|
|
return Settings(**kwargs) # pyright: ignore[reportCallIssue] (kwarg exists at runtime)
|
|
|
|
|
|
class _VisionMock:
|
|
"""Duck-typed chat-model mock for :func:`describe_pdf_page` (task
|
|
02): records the message + model and returns a canned reply (or
|
|
raises it). Satisfies the summarizer's ``SummaryLLM`` protocol.
|
|
"""
|
|
|
|
def __init__(
|
|
self, reply: str | Exception = "Two parts joined with an arrow."
|
|
) -> None:
|
|
self.settings = _settings()
|
|
self.reply = reply
|
|
self.messages: list[Any] = []
|
|
self.model: str | None = None
|
|
|
|
async def chat(
|
|
self, messages: list[dict[str, Any]], model: str | None = None
|
|
) -> str:
|
|
self.messages = list(messages)
|
|
self.model = model
|
|
if isinstance(self.reply, Exception):
|
|
raise self.reply
|
|
return self.reply
|
|
|
|
|
|
def _message(mock: _VisionMock) -> list[dict[str, Any]]:
|
|
"""The one user message's multimodal ``content`` list (asserted to
|
|
be a list of two parts by the shape test)."""
|
|
assert len(mock.messages) == 1
|
|
message = mock.messages[0]
|
|
assert message["role"] == "user"
|
|
content = message["content"]
|
|
assert isinstance(content, list)
|
|
return content
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# The marker + the locked instruction (the mock's key, PLAN §6)
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def test_pictorial_prompt_carries_the_mode_marker_and_locked_wording() -> None:
|
|
"""The ``PICTORIAL_PAGE_MODE`` marker heads the pictorial prompt
|
|
(the ``SUMMARY_MODE``/``IMAGE_DESCRIPTION_MODE`` convention — the
|
|
deterministic E2E mock LLM keys on it), and the LOCKED wording's
|
|
load-bearing fragments are in the instruction: a re-wrap of the
|
|
prompt cannot silently re-route the mock, and the "describe what
|
|
the text does NOT contain" mandate stays in the constant."""
|
|
assert PICTORIAL_PAGE_MODE == "PICTORIAL_PAGE_MODE"
|
|
assert PICTORIAL_PROMPT.startswith(PICTORIAL_PAGE_MODE + ": ")
|
|
assert (
|
|
PICTORIAL_PROMPT == PICTORIAL_PAGE_MODE + ": " + PICTORIAL_INSTRUCTION
|
|
)
|
|
for fragment in (
|
|
"assembly steps",
|
|
"part names",
|
|
"arrows",
|
|
"tool icons",
|
|
"before/after states",
|
|
"ESPECIALLY",
|
|
"extracted text",
|
|
"plain text",
|
|
):
|
|
assert fragment in PICTORIAL_PROMPT
|
|
# The markers are disjoint fixed app constants (neither contains
|
|
# the other) — the mock's two vision branches cannot shadow each
|
|
# other regardless of branch order.
|
|
assert IMAGE_DESCRIPTION_MODE not in PICTORIAL_PROMPT
|
|
assert PICTORIAL_PAGE_MODE not in DESCRIBE_PROMPT
|
|
|
|
|
|
def test_describe_prompt_byte_identity_regression_pin() -> None:
|
|
"""REGRESSION PIN: the phase-122 ``DESCRIBE_PROMPT`` is untouched
|
|
by the pictorial seam (the phase 122/126/137 suites pass unedited —
|
|
the image marker's mock branch serves the exact pre-phase-140
|
|
prompt). The full value is pinned, not just its shape: any
|
|
re-wrap of the locked instruction fails here, not in the story
|
|
suites."""
|
|
assert DESCRIBE_PROMPT == (
|
|
"IMAGE_DESCRIPTION_MODE: Describe this image faithfully, in "
|
|
"plain text, for a search index. State what is depicted, "
|
|
"transcribe any visible text, labels, or titles, describe the "
|
|
"structure of any diagram, table, or layout, and call out the "
|
|
"most salient details. Write 2-4 sentences of substance. Do not "
|
|
"use markdown. Do not invent anything that is not visible in "
|
|
"the image. Your description is the ONLY text that will ever be "
|
|
"retrieved for this image — it must carry the image's full "
|
|
"meaning."
|
|
)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# describe_pdf_page — message shape, model, cap, fail-soft
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def test_describe_pdf_page_message_shape_multimodal_and_chat_model() -> None:
|
|
"""ONE chat-model call (LOCKED A2 — the vision ``llm_chat_model``,
|
|
NOT the ``lite`` summary model) with the OpenAI-compatible
|
|
multimodal user message: the text part is exactly
|
|
``PICTORIAL_PROMPT + "\\n\\nPage {page} of the PDF. Text extracted
|
|
from this page:\\n" + page_text.strip()`` (the page's OWN text as
|
|
context, so the model describes what the text does NOT carry) and
|
|
the ``image_url`` part is the data URL ``data:<mime>;base64,<png
|
|
bytes>`` — no system prompt, no tools."""
|
|
mock = _VisionMock("Two parts joined with an arrow.")
|
|
result = asyncio.run(
|
|
describe_pdf_page(
|
|
mock,
|
|
data=PNG_1X1,
|
|
mime="image/png",
|
|
page=7,
|
|
page_text="Illustration 7",
|
|
)
|
|
)
|
|
assert result == "Two parts joined with an arrow."
|
|
content = _message(mock)
|
|
assert len(content) == 2
|
|
assert content[0] == {
|
|
"type": "text",
|
|
"text": (
|
|
PICTORIAL_PROMPT
|
|
+ "\n\nPage 7 of the PDF. Text extracted from this page:\n"
|
|
+ "Illustration 7"
|
|
),
|
|
}
|
|
b64 = base64.b64encode(PNG_1X1).decode("ascii")
|
|
assert content[1] == {
|
|
"type": "image_url",
|
|
"image_url": {"url": f"data:image/png;base64,{b64}"},
|
|
}
|
|
# LOCKED A2: the CHAT (vision) model — never the ``lite`` summary
|
|
# model (it is not assumed vision-capable).
|
|
assert mock.model == mock.settings.llm_chat_model
|
|
assert mock.model != mock.settings.llm_summary_model
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
("page_text", "suffix"),
|
|
[
|
|
(None, "(none)"),
|
|
("", "(none)"),
|
|
(" \t\n ", "(none)"),
|
|
(" Illustration 7 ", "Illustration 7"),
|
|
],
|
|
)
|
|
def test_describe_pdf_page_page_block_none_and_strip_cases(
|
|
page_text: str | None, suffix: str,
|
|
) -> None:
|
|
"""The page block's tail is ``page_text.strip() or "(none)"`` — a
|
|
page with no extracted text (``None``/empty/blank) sends the
|
|
explicit ``(none)`` sentinel (the vision model knows there is
|
|
nothing to restate), and a text-carrying page sends its STRIPPED
|
|
text (the leading/trailing whitespace of the extraction never
|
|
reaches the model)."""
|
|
mock = _VisionMock("ok")
|
|
asyncio.run(
|
|
describe_pdf_page(
|
|
mock, data=PNG_1X1, mime="image/png", page=3, page_text=page_text
|
|
)
|
|
)
|
|
content = _message(mock)
|
|
assert content[0]["text"] == (
|
|
PICTORIAL_PROMPT
|
|
+ f"\n\nPage 3 of the PDF. Text extracted from this page:\n{suffix}"
|
|
)
|
|
|
|
|
|
def test_describe_pdf_page_strips_and_caps_at_summary_max_chars() -> None:
|
|
"""The reply is whitespace-stripped and cut exactly at
|
|
``summary_max_chars`` (the phase-30 cap — the same uniform ceiling
|
|
as :func:`describe_image`). The explicit ``settings`` parameter
|
|
wins; ``settings=None`` falls back to the LLM's own settings."""
|
|
long_reply = "x" * 500
|
|
mock = _VisionMock(long_reply)
|
|
capped = asyncio.run(
|
|
describe_pdf_page(
|
|
mock,
|
|
data=PNG_1X1,
|
|
mime="image/png",
|
|
page=2,
|
|
page_text=None,
|
|
settings=_settings(summary_max_chars=100),
|
|
)
|
|
)
|
|
assert capped == "x" * 100
|
|
mock2 = _VisionMock(" " + "y" * 50 + "\n")
|
|
mock2.settings = _settings(summary_max_chars=42)
|
|
result = asyncio.run(
|
|
describe_pdf_page(mock2, data=PNG_1X1, mime="image/png", page=2, page_text=None)
|
|
)
|
|
assert result == ("y" * 42)
|
|
|
|
|
|
def test_describe_pdf_page_returns_none_on_failure_classes(
|
|
caplog: pytest.LogCaptureFixture,
|
|
) -> None:
|
|
"""Fail-soft (LOCKED A2 — one dead page never stalls a sync):
|
|
``None`` on a client error (with a warning log naming the page —
|
|
the house one-shot style) and on an empty or whitespace-only reply
|
|
(the house one-shot empty-content policy)."""
|
|
failing = _VisionMock(LLMError("simulated vision-model failure (test sentinel)"))
|
|
with caplog.at_level(logging.WARNING, logger="app.summarizer"):
|
|
assert (
|
|
asyncio.run(
|
|
describe_pdf_page(
|
|
failing, data=PNG_1X1, mime="image/png", page=9, page_text=None
|
|
)
|
|
)
|
|
is None
|
|
)
|
|
warnings = [
|
|
r
|
|
for r in caplog.records
|
|
if r.levelno == logging.WARNING and "pictorial page description" in r.getMessage()
|
|
]
|
|
assert len(warnings) == 1 and "page=9" in warnings[0].getMessage()
|
|
for empty in ("", " \n"):
|
|
blank = _VisionMock(empty)
|
|
assert (
|
|
asyncio.run(
|
|
describe_pdf_page(
|
|
blank, data=PNG_1X1, mime="image/png", page=9, page_text=None
|
|
)
|
|
)
|
|
is None
|
|
)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# render_pdf_pages — the optional 1-based page selector
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def _three_page_text_pdf(path: Path) -> Path:
|
|
"""A 3-page temp PDF with EXPLICIT letter pages (612x792pt —
|
|
deterministic render sizes) and page-specific text (the render must
|
|
be page-specific, so the pages' PNG bytes must differ — the
|
|
phase-126/133 recipe, extended to three pages for the selector's
|
|
count + order assertions)."""
|
|
doc = fitz.open()
|
|
for marker in ("Alpha page text.", "Beta page text.", "Gamma page text."):
|
|
doc.new_page(width=612, height=792).insert_text((72, 72), marker)
|
|
doc.save(path)
|
|
doc.close()
|
|
return path
|
|
|
|
|
|
def test_render_selector_none_renders_every_page(tmp_path: Path) -> None:
|
|
"""``pages=None`` (the default) = every page in page order — the
|
|
phase-133 caller's result, byte-identical: a 3-page PDF → 3 PNGs,
|
|
each a genuine decodable PNG, page-specific (the bytes differ)."""
|
|
path = _three_page_text_pdf(tmp_path / "three.pdf")
|
|
pages = render_pdf_pages(path)
|
|
assert len(pages) == 3
|
|
for png in pages:
|
|
assert png.startswith(PNG_MAGIC)
|
|
pix = fitz.Pixmap(png)
|
|
assert (pix.width, pix.height) == (1275, 1650) # 150-DPI letter
|
|
assert pages[0] != pages[1] != pages[2] and pages[0] != pages[2]
|
|
|
|
|
|
def test_render_selector_subset_count_ascending_order_and_pngs(
|
|
tmp_path: Path,
|
|
) -> None:
|
|
"""A named subset renders ONLY the named pages (3-page PDF,
|
|
``pages=[3, 1]`` → 2 PNGs, not 3), in ASCENDING order regardless of
|
|
the input order (list index *i* is the *i*-th smallest NAMED page —
|
|
index 0 is page 1, index 1 is page 3), each a genuine PNG whose
|
|
bytes equal the corresponding page's render in the full
|
|
(``pages=None``) pass — the selector changes the set, never the
|
|
per-page output."""
|
|
path = _three_page_text_pdf(tmp_path / "three.pdf")
|
|
full = render_pdf_pages(path)
|
|
selected = render_pdf_pages(path, pages=[3, 1])
|
|
assert len(selected) == 2
|
|
for png in selected:
|
|
assert png.startswith(PNG_MAGIC)
|
|
pix = fitz.Pixmap(png)
|
|
assert (pix.width, pix.height) == (1275, 1650) # 150-DPI letter
|
|
assert selected[0] == full[0] # the smallest named page (1) first
|
|
assert selected[1] == full[2] # page 3 second, page 2 rendered by no one
|
|
single = render_pdf_pages(path, pages=[2])
|
|
assert single == [full[1]]
|
|
|
|
|
|
def test_render_selector_out_of_range_named_page_is_zero_bytes(
|
|
tmp_path: Path, caplog: pytest.LogCaptureFixture,
|
|
) -> None:
|
|
"""A named page outside the document's page range yields ``b""`` at
|
|
ITS position (the caller's placeholder — the result's order never
|
|
shifts, so it stays aligned with the requested pages) + a warning;
|
|
the in-range page keeps its PNG."""
|
|
path = _three_page_text_pdf(tmp_path / "three.pdf")
|
|
full = render_pdf_pages(path)
|
|
with caplog.at_level(logging.WARNING, logger="app.rag.pdf"):
|
|
selected = render_pdf_pages(path, pages=[2, 99])
|
|
assert selected == [full[1], b""]
|
|
warnings = [
|
|
r
|
|
for r in caplog.records
|
|
if r.levelno == logging.WARNING and "could not render page 99" in r.getMessage()
|
|
]
|
|
assert len(warnings) == 1
|
|
|
|
|
|
def test_render_selector_empty_list_renders_nothing(tmp_path: Path) -> None:
|
|
"""``pages=[]`` (no candidates) renders nothing — the zero-
|
|
candidates no-op the importer relies on (a clean-text PDF's path
|
|
never burns a render or a vision call)."""
|
|
path = _three_page_text_pdf(tmp_path / "three.pdf")
|
|
assert render_pdf_pages(path, pages=[]) == []
|
|
|
|
|
|
def test_render_selector_unopenable_file_returns_empty(
|
|
tmp_path: Path,
|
|
) -> None:
|
|
"""A file that cannot be OPENED yields ``[]`` even with a selector
|
|
(the caller's existing empty-content path — unchanged by the
|
|
phase-140 parameter)."""
|
|
bad = tmp_path / "bad.pdf"
|
|
bad.write_bytes(b"this is not a pdf at all")
|
|
assert render_pdf_pages(bad, pages=[1, 2]) == []
|
|
assert render_pdf_pages(tmp_path / "missing.pdf", pages=[1]) == []
|
|
|
|
|
|
def test_render_selector_broken_named_page_is_zero_bytes(
|
|
monkeypatch: pytest.MonkeyPatch, tmp_path: Path,
|
|
) -> None:
|
|
"""A named page whose ``get_pixmap`` raises → ``b""`` at ITS
|
|
position (the caller's placeholder slot — the page index NEVER
|
|
shifts, so the result stays aligned with the requested pages); the
|
|
good named pages keep their PNGs, 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)
|
|
|
|
class _FakeDoc:
|
|
def __init__(self, pages: list[Any]) -> None:
|
|
self._pages = pages
|
|
|
|
def __len__(self) -> int:
|
|
return len(self._pages)
|
|
|
|
def __getitem__(self, index: int) -> Any:
|
|
return self._pages[index]
|
|
|
|
def __enter__(self) -> _FakeDoc: # noqa: F821 (nested class, future annotations)
|
|
return self
|
|
|
|
def __exit__(self, *args: object) -> bool:
|
|
return False
|
|
|
|
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)
|
|
# pages=[3, 1, 2] renders ascending 1, 2, 3 — the broken page 2
|
|
# holds ITS position (index 1), not the tail.
|
|
assert render_pdf_pages(path, pages=[3, 1, 2]) == [
|
|
b"\x89PNG-fake-page-1",
|
|
b"",
|
|
b"\x89PNG-fake-page-3",
|
|
]
|
|
# A repeated page number renders ONCE (the selector is a set of
|
|
# pages, ascending).
|
|
assert render_pdf_pages(path, pages=[1, 1]) == [b"\x89PNG-fake-page-1"]
|