phase: 137_pdf_image_context
All verification complete — everything is 100% green. Final report: **Phase 137 final verification pass — all tasks already complete, all criteria verified** - **Full suite:** `uv run pytest --cov=app --cov-report=term-missing` → 3272 passed, exit 0; `app/` coverage **99%** (>90% ✓) - **Phase E2E in isolation:** `uv run pytest tests/e2e/test_pdf_image_context.py -v --no-cov` → **4 passed** (forward context, display line, standalone parity, both-blocks markdown case) - **Pre-existing suites in isolation:** `test_pdf_support.py` + `test_image_anchor_context.py` + `test_image_documents.py` → **14 passed**; ripple-edited `test_garbage_pdf.py` → **3 passed** - **Lint/types:** `uv run ruff check .` → all passed; `uv run pyright` → 0 errors **Completion criteria:** (1) forward-path context in vision msg + stored `image_context`/summary — ✓ (unit builder/seam + integration fake-LLM + E2E); (2) both blocks document-first + standalone describe byte-identical — ✓ (combiner + standalone pins); (3) backfill with context + zero-vision idempotent re-sync + parent-gone origin-only — ✓ (6 integration tests); (4) suggestion block leads with `From <source>/<pdf> (page N)` — ✓ (renderer units + E2E); (5) all gates green — ✓ above; (6) no behavior change in phases 122/125/126 + sync `detail` shape untouched — ✓ (verified in code: new counter is log-only); (7) ruff+pyright — ✓; (8) commit — left to the harness per pipeline rules (working tree untouched by me). **Documentation:** README.md audited in full — the new phase-137 bullet verified against code (6 000-char cap, log-only `image_context_backfilled=N`, `<document>`-body locator placement), all env tables/defaults cross-checked vs `app/config.py` and `.env.example` — consistent, no changes needed. AGENTS.md: every rule/command verified — consistent, no changes needed. **Notable deviations (pre-existing, documented in task-05 report, forced by design):** `test_pdf_support.py`/`test_garbage_pdf.py` got minimal ripple edits (the "UNEDITED" pin was unsatisfiable once describe calls carry context by design); the unit `test_image_anchor_context.py` CONTEXT_PREFIX pin generalized per ASSUMPTION A5; one extra mock line (`_seed_context_line` accepts `From …`). **Next pending phase:** none — `todo/` holds only this phase.
This commit is contained in:
+117
-40
@@ -445,6 +445,22 @@ def build_kb_section(overview: str, max_chars: int | None = None) -> str:
|
||||
#: (graceful degradation, A4).
|
||||
_IMAGE_CTX_HEADER_RE = re.compile(r"^Referenced in (?P<ref>.+):$")
|
||||
|
||||
#: Phase 137 (task 04, ASSUMPTION A6) — the header line of the PDF
|
||||
#: provenance block of a PDF-embedded image doc's stored
|
||||
#: ``image_context`` (the exact shape the importer's document-block
|
||||
#: writer emits, phase 137 tasks 02/03 — written FIRST, before any
|
||||
#: markdown reference blocks, ASSUMPTION A2):
|
||||
#: ``Extracted from <source>/<pdf path> (page <N>):`` — ``loc`` is the
|
||||
#: ``<source>/<pdf path>`` locator (``.+`` greedy to the LAST
|
||||
#: `` (page N):`` — the pdf path may itself contain spaces); the line
|
||||
#: FOLLOWING the header is page-text body (or the next block's header)
|
||||
#: and is never part of the clause — the header line alone IS the
|
||||
#: clause (no bullet is required or expected). A look-alike line inside
|
||||
#: the page-text body emits a clause the same way — accepted: the
|
||||
#: stored value is the context the importer wrote, not free model text
|
||||
#: (the ``Referenced in`` twin IS guarded by its bullet requirement).
|
||||
_PDF_CTX_HEADER_RE = re.compile(r"^Extracted from (?P<loc>.+) \(page (?P<page>\d+)\):$")
|
||||
|
||||
#: The ``- Alt text: "..."`` bullet of the same block. The greedy alt
|
||||
#: field (the block writer does not escape quotes) is captured between
|
||||
#: the opening quote (the pattern's literal) and the CLOSING quote
|
||||
@@ -469,48 +485,96 @@ def _image_ctx_clause(ref: str, alt: str | None) -> str:
|
||||
|
||||
def _extract_context_line(image_context: str) -> str | None:
|
||||
"""The ONE-LINE rendering of an enriched image doc's stored
|
||||
``image_context`` (phase 125, task 05, LOCKED A4) — or ``None``
|
||||
when nothing in the value parses as a reference block (graceful
|
||||
degradation: the ``<document>`` block then shows only the
|
||||
description, exactly pre-phase — the value never raises into the
|
||||
prompt build).
|
||||
``image_context`` (phase 125, task 05, LOCKED A4; phase 137, task
|
||||
04, ASSUMPTION A6) — or ``None`` when nothing in the value parses
|
||||
as a reference block OR a provenance block (graceful degradation:
|
||||
the ``<document>`` block then shows only the description, exactly
|
||||
pre-phase — the value never raises into the prompt build).
|
||||
|
||||
The stored value (tasks 03/04) is one block per referring markdown
|
||||
file, blank-line joined::
|
||||
The stored value is blank-line-joined blocks of two kinds:
|
||||
|
||||
Referenced in <source>/<markdown_path>:
|
||||
- Alt text: "<alt_text>"
|
||||
Context: <context paragraphs — may span many lines>
|
||||
- Alt text: "<a second reference's alt>"
|
||||
* the phase-137 document (provenance) block (tasks 02/03) — written
|
||||
FIRST for a PDF-embedded image (ASSUMPTION A2: provenance before
|
||||
purpose); ONE header line, no bullet::
|
||||
|
||||
The result keeps only what tells the LLM WHY the image exists in
|
||||
the documentation — one clause per referring file (its identity +
|
||||
the FIRST reference's alt), after a single ``Referenced in ``
|
||||
prefix, ``; ``-joined::
|
||||
Extracted from <source>/<pdf path> (page <N>):
|
||||
<page text — never rendered: it already reached the vision
|
||||
model at describe time>
|
||||
|
||||
Referenced in docs/a.md: "alt a"; docs/b.md
|
||||
→ the clause ``From <source>/<pdf path> (page <N>)`` — the
|
||||
header line alone (the page-text body is skipped as context
|
||||
body; a spurious ``Extracted from … (page N):`` line inside the
|
||||
page text emits a clause the same way — it is the stored context
|
||||
the importer wrote, not free model text, accepted).
|
||||
|
||||
The ``Context:`` body is never rendered (the surrounding paragraphs
|
||||
already reached the vision model at enrichment time, LOCKED A3 —
|
||||
the line is the locator + purpose, not a duplicate of the
|
||||
paragraphs), so the line stays short enough not to bloat the
|
||||
prompt. The markdown doc's ``IMAGE_CONTEXT:`` pending record
|
||||
(task 04) carries no ``Referenced in`` header at all and parses to
|
||||
``None`` — and it rides a TEXT doc, which :func:`_document_body`
|
||||
excludes from the line via the ``is_image`` guard anyway.
|
||||
* the phase-125 reference block (tasks 03/04) — one per referring
|
||||
markdown file; the line FOLLOWING the header is always its first
|
||||
``- Alt text: "..."`` bullet (a look-alike header without its
|
||||
bullet is spurious and dropped)::
|
||||
|
||||
Referenced in <source>/<markdown_path>:
|
||||
- Alt text: "<alt_text>"
|
||||
Context: <context paragraphs — may span many lines>
|
||||
- Alt text: "<a second reference's alt>"
|
||||
|
||||
→ the clause ``Referenced in <ref>: "<alt>"`` (identity + the
|
||||
FIRST reference's alt; the ``Context:`` body is never rendered —
|
||||
LOCKED A3, the paragraphs already reached the vision model at
|
||||
enrichment time, so the line stays short enough not to bloat
|
||||
the prompt).
|
||||
|
||||
Clauses join with ``"; "`` IN STORED ORDER:
|
||||
|
||||
* BOTH kinds (the phase-137 combined shape — document block first,
|
||||
then the markdown blocks) render each clause with its own
|
||||
prefix::
|
||||
|
||||
From <source>/<pdf> (page N); Referenced in docs/a.md: "alt"
|
||||
|
||||
* a MARKDOWN-ONLY value keeps the phase-125 shape byte-identically
|
||||
(the regression pin — ONE ``Referenced in `` prefix for the
|
||||
joined referenced clauses)::
|
||||
|
||||
Referenced in docs/a.md: "alt a"; docs/b.md
|
||||
|
||||
A value with NO parsable header of either kind parses to ``None``
|
||||
(pre-phase rows, unparseable text). The markdown doc's
|
||||
``IMAGE_CONTEXT:`` pending record (phase 125, task 04) carries no
|
||||
parsable header of either kind and parses to ``None`` — and it
|
||||
rides a TEXT doc, which :func:`_document_body` excludes from the
|
||||
line via the ``is_image`` guard anyway.
|
||||
"""
|
||||
clauses: list[str] = []
|
||||
clauses: list[tuple[str, str]] = []
|
||||
# (kind, text), in stored order: a ``"pdf"`` clause's text already
|
||||
# carries its ``From …`` prefix; a ``"ref"`` clause's text is the
|
||||
# BARE ``<ref>: "<alt>"`` core — the prefix is applied at the join,
|
||||
# below (a markdown-only value keeps ONE prefix, byte-identically).
|
||||
ref: str | None = None # the current block's "source/markdown_path"
|
||||
alt: str | None = None # the current block's first bullet's alt text
|
||||
expecting_alt = False # the line after a header must be its bullet
|
||||
expecting_alt = False # the line after a "Referenced in" header must be its bullet
|
||||
for line in image_context.splitlines():
|
||||
header = _IMAGE_CTX_HEADER_RE.match(line)
|
||||
if header is not None:
|
||||
if ref is not None and not expecting_alt:
|
||||
clauses.append(_image_ctx_clause(ref, alt))
|
||||
clauses.append(("ref", _image_ctx_clause(ref, alt)))
|
||||
ref, alt = header.group("ref"), None
|
||||
expecting_alt = True
|
||||
continue
|
||||
pdf_header = _PDF_CTX_HEADER_RE.match(line)
|
||||
if pdf_header is not None:
|
||||
# A provenance header requires NO following bullet — the
|
||||
# header line alone is the clause (the next line is
|
||||
# page-text body or the next header; neither changes it).
|
||||
# A pending reference block's bullet never came → dropped
|
||||
# by the same spurious-header rule; a complete one is
|
||||
# kept, in stored order.
|
||||
if ref is not None and not expecting_alt:
|
||||
clauses.append(("ref", _image_ctx_clause(ref, alt)))
|
||||
clauses.append(
|
||||
("pdf", f"From {pdf_header.group('loc')} (page {pdf_header.group('page')})")
|
||||
)
|
||||
ref, alt, expecting_alt = None, None, False
|
||||
continue
|
||||
alt_match = _IMAGE_CTX_ALT_RE.match(line) if ref is not None else None
|
||||
if expecting_alt:
|
||||
if alt_match is None:
|
||||
@@ -521,16 +585,27 @@ def _extract_context_line(image_context: str) -> str | None:
|
||||
alt = alt_match.group("alt")
|
||||
expecting_alt = False
|
||||
continue
|
||||
# Any other line is context body or a second bullet of the
|
||||
# same block — the first reference's alt is already captured,
|
||||
# so the one-line rendering is unaffected (no branch needed:
|
||||
# with a live block and no pending bullet, ``alt`` is never
|
||||
# ``None``).
|
||||
# Any other line is context body — a provenance block's page
|
||||
# text, a reference block's Context: paragraphs, or a second
|
||||
# bullet of the same block — and the one-line rendering is
|
||||
# unaffected (no branch needed: with a live block and no
|
||||
# pending bullet, ``alt`` is never ``None``).
|
||||
if ref is not None and not expecting_alt:
|
||||
clauses.append(_image_ctx_clause(ref, alt))
|
||||
clauses.append(("ref", _image_ctx_clause(ref, alt)))
|
||||
if not clauses:
|
||||
return None
|
||||
return "Referenced in " + "; ".join(clauses)
|
||||
if all(kind == "ref" for kind, _ in clauses):
|
||||
# Markdown-only: the phase-125 shape, byte-identically (the
|
||||
# regression pin) — ONE ``Referenced in `` prefix for the
|
||||
# joined clauses.
|
||||
return "Referenced in " + "; ".join(text for _, text in clauses)
|
||||
# A provenance clause is present: every clause carries its own
|
||||
# prefix (the ``pdf`` ones already do; the referenced clauses gain
|
||||
# theirs) — the stored order (document block first, ASSUMPTION A2)
|
||||
# is the render order.
|
||||
return "; ".join(
|
||||
text if kind == "pdf" else f"Referenced in {text}" for kind, text in clauses
|
||||
)
|
||||
|
||||
|
||||
def _image_context_line(doc: Document) -> str | None:
|
||||
@@ -559,12 +634,14 @@ def _document_body(doc: Document) -> str:
|
||||
or under the cap rides whole, unmarked (nothing was cut). No LLM
|
||||
call at chat time — the preview is deterministic.
|
||||
|
||||
Phase 125 (task 05, LOCKED A4): an ENRICHED image doc (``is_image``
|
||||
with a non-NULL ``image_context`` that parses — see
|
||||
:func:`_image_context_line`) leads its body with the ONE short
|
||||
context line — ``Referenced in <source>/<path>: "<alt>"`` — before
|
||||
the description, so the LLM knows WHY the image exists in the
|
||||
documentation, not only what it shows. Every other doc is
|
||||
Phase 125 (task 05, LOCKED A4; phase 137, task 04, A6): an ENRICHED
|
||||
image doc (``is_image`` with a non-NULL ``image_context`` that
|
||||
parses — see :func:`_image_context_line`) leads its body with the
|
||||
ONE short context line — ``Referenced in <source>/<path>: "<alt>"``
|
||||
(the phase-125 reference shape) or ``From <source>/<pdf> (page N)``
|
||||
(the phase-137 provenance shape) — before the description, so the
|
||||
LLM knows WHY the image exists in the documentation, not only what
|
||||
it shows. Every other doc is
|
||||
byte-identical to the pre-phase body: every text doc (its
|
||||
``image_context`` may carry the task-04 pending record, which never
|
||||
renders) and every non-enriched image doc (NULL ``image_context``
|
||||
|
||||
Reference in New Issue
Block a user