phase: 152_sync_progress_image_pass
All criteria verified. The CLI path is hook-less (only `app/api/sync.py` passes the hook), and the byte-identity is pinned by `test_on_image_progress_none_run_is_byte_identical`. ## Final Report — Phase 152 final verification pass - **Verified (no new code needed):** `SyncStatus.images_done/images_total` + reset/clear rules in `app/api/sync.py`; `on_image_progress` threaded through the importer's three PDF vision spans with the continuous per-file counter; the `_image_hook` closure; `sources.js` `· image n/m` label part (box + title + aria). - **C1** status climbs 1→N + UI shows `· image n/m`: ✅ unit 3/15→15/15, integration 1/15→7/15, E2E 1/4→4/4 (title/aria ride the full string) - **C2** never frozen: ✅ E2E asserts ≥3 distinct positions, starting at 1, non-decreasing, ≤ total, reaching it - **C3** cleared at overview/summaries + all terminals; per-file reset via `_hook`; no stale position for image-less files: ✅ unit pins + E2E settle (image part gone in box/title/aria, endpoint 0/0) - **C4** `/api/sync/status` additive-only: ✅ full-dict pin passes (all pre-existing keys unchanged + the two new 0/0 keys) - **C5** hook-less caller byte-identical: ✅ CLI passes no hook; `test_on_image_progress_none_run_is_byte_identical` green - **C6** `uv run pytest`: **3526 passed, exit 0** (95s); dedicated E2E in isolation: **2 passed** (95s); `uv run ruff check . && uv run pyright`: **clean**; coverage: **99% on `app/` (>90% gate)** - Regression: all five neighbouring E2E suites green in isolation (progress_box 8, chunk_progress 2, label_mobile 4, pdf_support 6, image_documents 4) - **C7** atomic commit: left to the harness (no git index/history touched per rules) - **Docs:** README.md — sync-label paragraph already updated this phase, accurately; all `.env.example` vars documented; routes/commands verified — **no changes needed**. AGENTS.md — all references (PLAN, user_stories, test_smoke.py, .env.example, gpg config, .gitignore) verified — **no changes needed** - **Defects found in prior work:** none (one prior env-leak defect was already fixed inside this phase's task 04, conftest) - No live-infrastructure touched; no ntfy used. **Next pending phase:** none — `todo/` held only this phase
This commit is contained in:
+180
-25
@@ -285,10 +285,14 @@ log-only (the sync ``detail`` shape is untouched — the stats log-only
|
||||
convention, phase 133).
|
||||
|
||||
``import_sources`` accepts an optional per-file ``progress`` callback
|
||||
(phase 64, task 01) reporting the file being processed right now, and
|
||||
an optional ``on_chunk_progress`` callback (phase 129, task 02)
|
||||
(phase 64, task 01) reporting the file being processed right now, an
|
||||
optional ``on_chunk_progress`` callback (phase 129, task 02)
|
||||
reporting the in-flight file's embedding-chunk position at batch
|
||||
granularity (see the function docstring).
|
||||
granularity, and an optional ``on_image_progress`` callback (phase
|
||||
152, task 02) reporting the in-flight PDF's vision-image position —
|
||||
immediately before each of the PDF's vision describe calls, with the
|
||||
image's 1-based position in the file's ONE continuous vision counter
|
||||
and the span's count (see the function docstring).
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
@@ -663,6 +667,7 @@ async def import_sources(
|
||||
session: Session | None = None,
|
||||
progress: Callable[[str, str, int, int], None] | None = None,
|
||||
on_chunk_progress: Callable[[int, int], None] | None = None,
|
||||
on_image_progress: Callable[[int, int], None] | None = None,
|
||||
ignore_by_root: dict[str, list[str]] | None = None,
|
||||
include_hidden_by_root: dict[str, bool] | None = None,
|
||||
doc_dates_by_root: dict[str, dict[str, datetime]] | None = None,
|
||||
@@ -708,9 +713,41 @@ async def import_sources(
|
||||
``1/1``, which IS its only chunk): the document's summary-chunk
|
||||
embed (:func:`_store_summary`) NEVER receives the hook, so the
|
||||
counter counts content chunks only (no ``1/1`` noise after the
|
||||
real progress ends), and the post-index enrichment pass / the
|
||||
PDF's embedded-image docs (separate documents, not the walked
|
||||
file) do not report either.
|
||||
real progress ends), and the post-index passes (the phase-125
|
||||
enrichment, the phase-137 document-context backfill) do not report
|
||||
either — the PDF's vision describe calls report through the
|
||||
separate ``on_image_progress`` hook instead (phase 152, below).
|
||||
|
||||
``on_image_progress`` (phase 152, task 02) is an optional
|
||||
vision-image-position hook for the PDF file currently being
|
||||
indexed: while the PDF's vision spans run — the phase-133
|
||||
page-screenshot fallback, the phase-140 pictorial sparse-page
|
||||
pass, and the phase-126 embedded-image pass — it is called
|
||||
immediately before each of their vision describe calls with
|
||||
``(position_1based, total)``: the 1-based position of the image
|
||||
about to be described and the count for that span — the extracted
|
||||
image count (post-dedupe, post-blank-gate, phase 151: the work
|
||||
actually about to happen) for the embedded pass, the candidate-
|
||||
page count for the pictorial pass, the rendered-page count for the
|
||||
screenshot fallback. The spans of ONE file are reported as ONE
|
||||
continuous counter: each span is called with the count already
|
||||
reported for this file (*base*) and returns the number of images
|
||||
it reported, and the caller adds them — the pass that runs second
|
||||
starts where the first ended, so the position climbs across the
|
||||
file's whole vision span instead of restarting (the fallback and
|
||||
the pictorial pass are mutually exclusive — the phase-140
|
||||
precedence over the garbage gate — and both precede the embedded
|
||||
pass, which runs after the document commit). A skipped span (the
|
||||
``images`` toggle off, zero candidates, a clean text layer) reports
|
||||
nothing and advances the counter not at all. Standalone image
|
||||
files get NO sub-position (phase 152 A4 — one image per file,
|
||||
``1/1`` per file would be noise; the file count already covers
|
||||
them), and the hook carries counts only, never the image identity
|
||||
(A2 — the per-file ``progress`` hook names the file). Same "only
|
||||
when provided" rule as ``progress`` and ``on_chunk_progress``:
|
||||
``None`` (every existing caller — the CLI, the chat path) is a
|
||||
zero-cost guarded no-op, and the run is byte-identical to a
|
||||
hook-less run.
|
||||
|
||||
``ignore_by_root`` (phase 89, A1/A2) maps ``str(root)`` — the root path
|
||||
string exactly as passed in *sources* — to that source's RAW ignore-path
|
||||
@@ -861,7 +898,7 @@ async def import_sources(
|
||||
await _index_file(
|
||||
session, source=source, rel=rel, full_path=path, llm=llm,
|
||||
summary=summary, raw_date=dates_map.get(rel),
|
||||
on_chunk=on_chunk_progress,
|
||||
on_chunk=on_chunk_progress, on_image=on_image_progress,
|
||||
)
|
||||
except EmbeddingError as e:
|
||||
# A pathological file (e.g. content the embedding endpoint
|
||||
@@ -1078,6 +1115,7 @@ async def _index_file(
|
||||
summary: ImportSummary,
|
||||
raw_date: datetime | None = None,
|
||||
on_chunk: Callable[[int, int], None] | None = None,
|
||||
on_image: Callable[[int, int], None] | None = None,
|
||||
) -> None:
|
||||
"""Upsert one file: doc row + chunk rows + embeddings, one transaction.
|
||||
|
||||
@@ -1112,6 +1150,15 @@ async def _index_file(
|
||||
:func:`_index_pdf_file` (its content embed is the primary target).
|
||||
``None`` (the default) keeps every branch byte-identical.
|
||||
|
||||
``on_image`` (phase 152, task 02) is the in-flight file's
|
||||
vision-image-position hook (see :func:`import_sources`) — passed
|
||||
through to the PDF branch only (:func:`_index_pdf_file` — its
|
||||
three vision spans share ONE continuous counter per file): the
|
||||
image branch receives NO sub-position (phase 152 A4 — one image
|
||||
per file, ``1/1`` would be noise; the file count already covers
|
||||
it) and the text branch has no vision span. ``None`` (the
|
||||
default) keeps every branch byte-identical.
|
||||
|
||||
Image files (phase 122, toggle on) delegate to
|
||||
:func:`_index_image_file` — the binary path (bytes digest,
|
||||
persistent copy, ``content`` = the vision description) — BEFORE
|
||||
@@ -1165,7 +1212,7 @@ async def _index_file(
|
||||
return await _index_pdf_file(
|
||||
session, source=source, rel=rel, full_path=full_path, llm=llm,
|
||||
summary=summary, raw_date=raw_date, file_digest=file_digest,
|
||||
on_chunk=on_chunk,
|
||||
on_chunk=on_chunk, on_image=on_image,
|
||||
)
|
||||
content = full_path.read_text(encoding="utf-8", errors="replace").replace("\x00", "")
|
||||
digest = hashlib.sha256(content.encode("utf-8")).hexdigest()
|
||||
@@ -1401,6 +1448,7 @@ async def _index_pdf_file(
|
||||
file_digest: str,
|
||||
raw_date: datetime,
|
||||
on_chunk: Callable[[int, int], None] | None = None,
|
||||
on_image: Callable[[int, int], None] | None = None,
|
||||
) -> None:
|
||||
"""The phase-126 PDF branch of :func:`_index_file` — a PDF is
|
||||
indexed from its EXTRACTED TEXT (LOCKED A1/A2/A4), one transaction
|
||||
@@ -1488,6 +1536,26 @@ async def _index_pdf_file(
|
||||
documents — their ``1/1`` positions would be noise after the
|
||||
file's real progress ends, the A6 exclusion). ``None`` keeps the
|
||||
branch byte-identical.
|
||||
|
||||
``on_image`` (phase 152, task 02) is the in-flight file's
|
||||
vision-image-position hook (see :func:`import_sources`): it is
|
||||
forwarded to the three vision spans — the phase-133 fallback
|
||||
(:func:`_fallback_page_screenshots`), the phase-140 pictorial pass
|
||||
(:func:`_pictorial_pdf_pass`), and the embedded-image pass
|
||||
(:func:`_index_pdf_embedded_images`) — each firing it immediately
|
||||
before its vision describe calls. The spans of this file share
|
||||
ONE continuous counter: the local *images_reported* starts at 0,
|
||||
each span is called with the count reported so far as its *base*
|
||||
(the pass that runs second starts where the first ended), and the
|
||||
returned reported count is added — the position climbs across the
|
||||
file's whole vision span instead of restarting. The fallback and
|
||||
the pictorial pass are mutually exclusive (the phase-140
|
||||
precedence over the garbage gate — an unusable layer takes the
|
||||
fallback, a usable one the pass), both run BEFORE the document
|
||||
commit, and the embedded pass runs after it; a skipped span
|
||||
(toggle off, zero candidates, a clean text layer) reports nothing
|
||||
and advances the counter not at all. ``None`` keeps the branch
|
||||
byte-identical.
|
||||
"""
|
||||
# Extract the text (fail-soft: an unopenable PDF yields "" — the
|
||||
# document may still carry embedded images worth indexing).
|
||||
@@ -1504,6 +1572,14 @@ async def _index_pdf_file(
|
||||
# — the pre-phase-148 viewer).
|
||||
fallback_fired = False
|
||||
pictorial_described = False
|
||||
# Phase 152 (task 02): the file's ONE continuous vision-image
|
||||
# counter — 0 at the top of the file (the per-file pairing rule:
|
||||
# each file's spans start fresh; the caller's status fields reset
|
||||
# at the file boundary). Each span below is called with the count
|
||||
# reported so far as its *base* and returns the number of images
|
||||
# it reported, which is added — the pass that runs second starts
|
||||
# where the first ended. Stays 0 while the hook is None.
|
||||
images_reported = 0
|
||||
# Phase 133 (task 03): the garbage gate — BEFORE the digest/chunk/
|
||||
# summary pipeline, so it runs on the FINAL content exactly once.
|
||||
# A clean layer never reaches the fallback (the gate is the only
|
||||
@@ -1511,9 +1587,11 @@ async def _index_pdf_file(
|
||||
if not is_usable_pdf_text(content):
|
||||
summary.pdfs_screenshotted += 1
|
||||
fallback_fired = True
|
||||
content = await _fallback_page_screenshots(
|
||||
session, source=source, rel=rel, full_path=full_path, llm=llm
|
||||
content, reported = await _fallback_page_screenshots(
|
||||
session, source=source, rel=rel, full_path=full_path, llm=llm,
|
||||
on_image=on_image, base=images_reported,
|
||||
)
|
||||
images_reported += reported
|
||||
elif llm.settings.pdf_pictorial:
|
||||
# Phase 140 (task 03, LOCKED A1–A4): the pictorial pass —
|
||||
# USABLE layers only (the precedence is locked: the phase-133
|
||||
@@ -1523,9 +1601,11 @@ async def _index_pdf_file(
|
||||
# pre-phase-140 path); otherwise the FINAL content carries the
|
||||
# described candidates' illustration blocks (A4: the digest
|
||||
# below runs over it, the phase-126/133 convention).
|
||||
passed = await _pictorial_pdf_pass(
|
||||
source=source, rel=rel, full_path=full_path, llm=llm, summary=summary
|
||||
passed, reported = await _pictorial_pdf_pass(
|
||||
source=source, rel=rel, full_path=full_path, llm=llm, summary=summary,
|
||||
on_image=on_image, base=images_reported,
|
||||
)
|
||||
images_reported += reported
|
||||
if passed is not None:
|
||||
pictorial_described = True
|
||||
content = passed
|
||||
@@ -1650,6 +1730,7 @@ async def _index_pdf_file(
|
||||
await _index_pdf_embedded_images(
|
||||
session, source=source, rel=rel, full_path=full_path,
|
||||
llm=llm, summary=summary, raw_date=raw_date,
|
||||
on_image=on_image, base=images_reported,
|
||||
)
|
||||
|
||||
|
||||
@@ -1660,7 +1741,9 @@ async def _fallback_page_screenshots(
|
||||
rel: str,
|
||||
full_path: Path,
|
||||
llm: Embedder,
|
||||
) -> str:
|
||||
on_image: Callable[[int, int], None] | None = None,
|
||||
base: int = 0,
|
||||
) -> tuple[str, int]:
|
||||
"""The phase-133 (task 03) page-screenshot fallback — the owner's
|
||||
specified path ("screenshotting the pages and reading each one as
|
||||
an image") for a PDF whose text layer failed
|
||||
@@ -1700,6 +1783,18 @@ async def _fallback_page_screenshots(
|
||||
unchanged row's content digest (the row stays put until the
|
||||
bytes move; the doc is still correct either way).
|
||||
|
||||
* phase 152 (task 02): the ``on_image`` hook (the file's
|
||||
vision-image position, see :func:`import_sources`) is called
|
||||
immediately before EACH rendered page's vision call with
|
||||
``(base + i + 1, base + len(pages))`` — the page's 1-based slot
|
||||
in the rendered-page list and the span's total (the rendered-
|
||||
page count), at the START of the page's slow call, never after.
|
||||
The return is the ``(content, reported)`` pair — *reported* the
|
||||
number of images this span reported (0 while the hook is
|
||||
``None``), the count the caller adds to the file's continuous
|
||||
counter. A hook-less run is byte-identical (every call site is
|
||||
guarded).
|
||||
|
||||
*session* is part of the signature for symmetry with the other
|
||||
per-file index helpers (the fallback itself needs no DB access —
|
||||
the caller's upsert pipeline owns the row).
|
||||
@@ -1711,14 +1806,22 @@ async def _fallback_page_screenshots(
|
||||
source, rel, len(pages),
|
||||
)
|
||||
if not pages:
|
||||
return ""
|
||||
return "", 0
|
||||
blocks: list[str] = []
|
||||
got_any = False
|
||||
reported = 0
|
||||
for i, png in enumerate(pages):
|
||||
page = i + 1
|
||||
if not png:
|
||||
blocks.append(f"page {page} — description unavailable")
|
||||
continue
|
||||
# Phase 152 (task 02): the file's vision-image position —
|
||||
# this page's 1-based slot and the span's total (the rendered-
|
||||
# page count), at the START of the page's slow call, never
|
||||
# after.
|
||||
if on_image is not None:
|
||||
on_image(base + i + 1, base + len(pages))
|
||||
reported += 1
|
||||
desc = await describe_image(llm, data=png, mime="image/png")
|
||||
if desc is None or not desc.strip():
|
||||
blocks.append(f"page {page} — description unavailable")
|
||||
@@ -1726,10 +1829,13 @@ async def _fallback_page_screenshots(
|
||||
blocks.append(desc.strip())
|
||||
got_any = True
|
||||
if not got_any:
|
||||
return ""
|
||||
return "\n\n".join(
|
||||
f"{PAGE_SEPARATOR.format(page=page)}\n{block}"
|
||||
for page, block in enumerate(blocks, start=1)
|
||||
return "", reported
|
||||
return (
|
||||
"\n\n".join(
|
||||
f"{PAGE_SEPARATOR.format(page=page)}\n{block}"
|
||||
for page, block in enumerate(blocks, start=1)
|
||||
),
|
||||
reported,
|
||||
)
|
||||
|
||||
|
||||
@@ -1740,7 +1846,9 @@ async def _pictorial_pdf_pass(
|
||||
full_path: Path,
|
||||
llm: Embedder,
|
||||
summary: ImportSummary,
|
||||
) -> str | None:
|
||||
on_image: Callable[[int, int], None] | None = None,
|
||||
base: int = 0,
|
||||
) -> tuple[str | None, int]:
|
||||
"""The phase-140 (task 03) pictorial pass (LOCKED A1–A4) — the
|
||||
IKEA assembly-instruction class (TODO L5): a PDF whose text layer
|
||||
is USABLE but SPARSE (mostly pictorial pages — 4–73 chars of
|
||||
@@ -1791,6 +1899,19 @@ async def _pictorial_pdf_pass(
|
||||
(+the described page count), plus the ONE loud log line (PLAN
|
||||
§9): ``pdf: pictorial pass source=… path=… — N candidate pages,
|
||||
M described (min_chars=…)``.
|
||||
* phase 152 (task 02): the ``on_image`` hook (the file's
|
||||
vision-image position, see :func:`import_sources`) is called
|
||||
immediately before EACH candidate's vision call with
|
||||
``(base + i + 1, base + len(candidates))`` — the candidate's
|
||||
1-based slot in the deterministic gate's list and the span's
|
||||
total (the candidate-page count), at the START of the page's
|
||||
slow call, never after (an unreadable page — the zero-byte
|
||||
render placeholder — burns no vision call and reports nothing).
|
||||
The return is the ``(content-or-None, reported)`` pair —
|
||||
*reported* the number of images this span reported (0 while the
|
||||
hook is ``None`` or there are zero candidates), the count the
|
||||
caller adds to the file's continuous counter. A hook-less run
|
||||
is byte-identical (every call site is guarded).
|
||||
"""
|
||||
settings = llm.settings
|
||||
page_texts = extract_pdf_page_texts(full_path)
|
||||
@@ -1798,17 +1919,18 @@ async def _pictorial_pdf_pass(
|
||||
page_texts, min_chars=settings.pdf_pictorial_min_chars
|
||||
)
|
||||
if not candidates:
|
||||
return None
|
||||
return None, 0
|
||||
# ONE render for the whole candidate set (the phase-140 selector —
|
||||
# a 500-page PDF with 30 sparse pages renders 30 PNGs, not 500);
|
||||
# the result is aligned with *candidates* (ascending, one slot
|
||||
# each — a zero-byte placeholder never shifts the positions).
|
||||
pngs = render_pdf_pages(full_path, pages=candidates)
|
||||
descriptions: dict[int, str] = {}
|
||||
reported = 0
|
||||
# strict: the render's result is aligned with the candidate list by
|
||||
# contract (one slot per named page, ascending) — a length drift is
|
||||
# a parser regression, fail loud.
|
||||
for page, png in zip(candidates, pngs, strict=True):
|
||||
for i, (page, png) in enumerate(zip(candidates, pngs, strict=True)):
|
||||
if not png:
|
||||
# The phase-133 zero-byte placeholder: an unreadable page.
|
||||
logger.warning(
|
||||
@@ -1817,6 +1939,13 @@ async def _pictorial_pdf_pass(
|
||||
page, source, rel,
|
||||
)
|
||||
continue
|
||||
# Phase 152 (task 02): the file's vision-image position —
|
||||
# this candidate's 1-based slot and the span's total (the
|
||||
# candidate-page count), at the START of the page's slow
|
||||
# call, never after.
|
||||
if on_image is not None:
|
||||
on_image(base + i + 1, base + len(candidates))
|
||||
reported += 1
|
||||
desc = await describe_pdf_page(
|
||||
llm,
|
||||
data=png,
|
||||
@@ -1854,7 +1983,7 @@ async def _pictorial_pdf_pass(
|
||||
if page in descriptions:
|
||||
block += f"\n\n[Page {page} illustration: {descriptions[page]}]"
|
||||
parts.append(f"{PAGE_SEPARATOR.format(page=page)}\n{block}")
|
||||
return "\n\n".join(parts)
|
||||
return "\n\n".join(parts), reported
|
||||
|
||||
|
||||
def _persist_pdf_pages(
|
||||
@@ -1951,7 +2080,9 @@ async def _index_pdf_embedded_images(
|
||||
llm: Embedder,
|
||||
summary: ImportSummary,
|
||||
raw_date: datetime,
|
||||
) -> None:
|
||||
on_image: Callable[[int, int], None] | None = None,
|
||||
base: int = 0,
|
||||
) -> int:
|
||||
"""The phase-126 (A3) embedded-image pass — one call per (re)indexed
|
||||
PDF, after its document commit.
|
||||
|
||||
@@ -2034,6 +2165,19 @@ async def _index_pdf_embedded_images(
|
||||
layer: origin-line-only blocks, fail-soft like the rest of the
|
||||
parser.
|
||||
|
||||
* phase 152 (task 02): the ``on_image`` hook (the file's
|
||||
vision-image position, see :func:`import_sources`) is called at
|
||||
the START of EACH image's pipeline step — before the slow
|
||||
:func:`_describe_or_skip` seam inside
|
||||
:func:`_index_one_embedded_image` — with
|
||||
``(base + i + 1, base + len(images))``: the image's 1-based slot
|
||||
in the pass's list (post-dedupe, post-blank-gate, phase 151:
|
||||
the work actually about to happen) and the span's total. Returns
|
||||
the number of images reported (0 while the hook is ``None`` or
|
||||
the toggle is off) — the count the caller adds to the file's
|
||||
continuous counter. A hook-less run is byte-identical (every
|
||||
call site is guarded).
|
||||
|
||||
``raw_date`` is the PDF's resolved source date (the pass runs only
|
||||
for (re)indexed PDFs, so it is always concrete) — the image docs
|
||||
inherit the PDF's date (they are part of it).
|
||||
@@ -2048,7 +2192,7 @@ async def _index_pdf_embedded_images(
|
||||
"source=%s path=%s",
|
||||
source, rel,
|
||||
)
|
||||
return
|
||||
return 0
|
||||
# Phase 137 (task 02/03): the document context's raw material —
|
||||
# the SHARED probe (task 01's per-page text map extracted ONCE per
|
||||
# PDF — the pass reads it per image — plus the phase-133 garbage
|
||||
@@ -2069,12 +2213,23 @@ async def _index_pdf_embedded_images(
|
||||
)
|
||||
summary.pdf_images_deduped += stats.duplicates
|
||||
summary.pdf_images_blank_skipped += stats.blank_skipped
|
||||
for img in images:
|
||||
reported = 0
|
||||
for i, img in enumerate(images):
|
||||
# Phase 152 (task 02): the file's vision-image position —
|
||||
# this image's 1-based slot in the pass's list (post-dedupe,
|
||||
# post-blank-gate: the work actually about to happen) and the
|
||||
# span's total, at the START of the image's pipeline step
|
||||
# (the slow :func:`_describe_or_skip` seam inside
|
||||
# :func:`_index_one_embedded_image`), never after.
|
||||
if on_image is not None:
|
||||
on_image(base + i + 1, base + len(images))
|
||||
reported += 1
|
||||
await _index_one_embedded_image(
|
||||
session, img, source=source, rel=rel, full_path=full_path,
|
||||
llm=llm, summary=summary, raw_date=raw_date,
|
||||
page_texts=page_texts, text_usable=text_usable,
|
||||
)
|
||||
return reported
|
||||
|
||||
|
||||
async def _index_one_embedded_image(
|
||||
|
||||
Reference in New Issue
Block a user