phase: 129_sync_chunk_progress
All green. Final report:
**Phase 129 final verification pass — 100% green** (previous work intact; this pass re-verified everything and fixed two pre-existing README inconsistencies)
- Verified on-spec: `LLMClient.embed` per-batch `on_progress` hook; importer `on_chunk_progress` plumbing (text/image/PDF branches; summary embed excluded); `SyncStatus.current_chunk/chunks_total` reset+clear contract; UI `.sync-label-chunk` pinned part with untruncated `title`/`#sync-result`
- `uv run pytest --cov=app -q`: exit 0, ~3041 passed, 0 failed, `app/` coverage **99%** (>90% gate)
- `uv run pytest tests/e2e/test_sync_chunk_progress.py -v --no-cov` (isolated, DB up): **2 passed** (endpoint chunk climb with true `chunk_document` total + UI `· chunk n/m` part)
- `uv run ruff check .`: all checks passed · `uv run pyright`: 0 errors, 0 warnings
- Completion criteria: all 5 ✅ (incl. no behavior change in completed phases — full suite green, CLI/chat no-op paths byte-identical)
- Docs: README.md fixed 2 pre-existing inconsistencies (upload section claimed "unpacks + scans immediately" — phase 90 made it unpack+register only, scan deferred to Sync; troubleshooting claimed "120s client timeout" — actual guard is the 300 s pre-token `TURN_TIMEOUT_MS`). Phase-129 label behavior already documented. AGENTS.md re-verified consistent — no changes
- Removed stray untracked `NOTES.md` (8-byte artifact); no live-infra changes; no commits — all changes left in working tree for the harness
- Next pending phase: `130_chat_answer_clusters`
This commit is contained in:
+104
-10
@@ -144,7 +144,10 @@ is NOT cleared on success — it is the retrieval display's record
|
||||
also, task 05).
|
||||
|
||||
``import_sources`` accepts an optional per-file ``progress`` callback
|
||||
(phase 64, task 01) reporting the file being processed right now.
|
||||
(phase 64, task 01) reporting the file being processed right now, and
|
||||
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).
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
@@ -198,7 +201,14 @@ class Embedder(Protocol):
|
||||
settings: Settings
|
||||
embed_batches: int
|
||||
|
||||
async def embed(self, texts: list[str]) -> list[list[float]]: ...
|
||||
async def embed(
|
||||
self,
|
||||
texts: list[str],
|
||||
*,
|
||||
# Phase 129 (task 01): the per-batch position hook — ``None``
|
||||
# (every pre-phase caller) is a zero-cost guarded no-op.
|
||||
on_progress: Callable[[int, int], None] | None = None,
|
||||
) -> list[list[float]]: ...
|
||||
|
||||
async def chat(self, messages: list[dict[str, Any]], model: str | None = None) -> str: ...
|
||||
# ^ the one-shot completion the summarizer uses — the ``lite`` model
|
||||
@@ -419,6 +429,7 @@ async def import_sources(
|
||||
limit: int | None = None,
|
||||
session: Session | None = None,
|
||||
progress: Callable[[str, str, int, int], None] | None = None,
|
||||
on_chunk_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,
|
||||
@@ -442,6 +453,32 @@ async def import_sources(
|
||||
exceeds the limit, but ``total`` stays the full pre-walk count (an
|
||||
incomplete walk must not misreport the denominator).
|
||||
|
||||
``on_chunk_progress`` (phase 129, task 02) is an optional
|
||||
chunk-position hook for the file currently being indexed: while that
|
||||
file's content chunks embed, it is called — through
|
||||
:meth:`app.rag.llm.LLMClient.embed`'s ``on_progress`` — before EACH
|
||||
embed batch with ``(first_chunk_1based, total_chunks)``: the 1-based
|
||||
position of the first chunk of the in-flight batch and the file's
|
||||
total content-chunk count (batch granularity, the phase-129 A6
|
||||
assumption — the transport embeds token-budgeted batches per HTTP
|
||||
call). It carries the in-flight FILE's chunk position only, never
|
||||
the file identity — the per-file ``progress`` hook owns that: the
|
||||
caller pairs them (a ``progress`` call names the file, and every
|
||||
chunk position reported until the next ``progress`` call belongs to
|
||||
it; the position climbs through the file's batches and restarts at
|
||||
1 for the next file). Same "only when provided" rule as
|
||||
``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. Only the content-chunk embeds report (the
|
||||
shared :func:`_replace_content_chunks` — text, image, and PDF
|
||||
branches; the image branch's one description chunk reports
|
||||
``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.
|
||||
|
||||
``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
|
||||
lines (the importer normalizes them via ``_ignore_for_root``, the single
|
||||
@@ -574,6 +611,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,
|
||||
)
|
||||
except EmbeddingError as e:
|
||||
# A pathological file (e.g. content the embedding endpoint
|
||||
@@ -608,7 +646,13 @@ async def import_sources(
|
||||
|
||||
|
||||
async def _replace_content_chunks(
|
||||
session: Session, *, doc: Document, llm: Embedder, content: str, rel: str
|
||||
session: Session,
|
||||
*,
|
||||
doc: Document,
|
||||
llm: Embedder,
|
||||
content: str,
|
||||
rel: str,
|
||||
on_chunk: Callable[[int, int], None] | None = None,
|
||||
) -> int:
|
||||
"""Phase 1+2 on *content*: chunk it (format-aware, PLAN §5), replace
|
||||
the doc's content chunk rows (embeddings NULL), and embed the new
|
||||
@@ -622,6 +666,20 @@ async def _replace_content_chunks(
|
||||
verbatim by the text path (:func:`_index_file`), the image path
|
||||
(task 03), and the task-04 enrichment pass — one copy, three call
|
||||
sites.
|
||||
|
||||
``on_chunk`` (phase 129, task 02) is the in-flight file's
|
||||
chunk-position hook, forwarded as ``on_progress`` to the content
|
||||
embed: it fires before each top-level embed batch with
|
||||
``(first_chunk_1based, total_chunks)``. The token-cap re-chunk retry
|
||||
(the ``while True`` above that halves *target*) re-embeds the NEW
|
||||
(smaller-target) chunk list, and the hook reports against the NEW
|
||||
total automatically — no special case: the announced total is
|
||||
always the chunk list actually in flight. The empty-content
|
||||
``doc.chunks = []`` break never embeds, so the hook never fires for
|
||||
it. ``None`` (the default; every pre-phase call site — the CLI, the
|
||||
phase-125 enrichment pass, and the PDF's embedded-image docs, which
|
||||
index separate documents rather than the walked file) keeps the call
|
||||
byte-identical.
|
||||
"""
|
||||
settings = llm.settings
|
||||
target = max(400, settings.chunk_target_chars)
|
||||
@@ -635,7 +693,12 @@ async def _replace_content_chunks(
|
||||
if not doc.chunks:
|
||||
break
|
||||
try:
|
||||
vectors = await llm.embed([c.content for c in doc.chunks])
|
||||
# Phase 129 (task 02): the in-flight file's chunk-position
|
||||
# hook (``on_progress`` — the task-01 contract; ``None`` is a
|
||||
# zero-cost no-op).
|
||||
vectors = await llm.embed(
|
||||
[c.content for c in doc.chunks], on_progress=on_chunk
|
||||
)
|
||||
for row, vec in zip(doc.chunks, vectors, strict=True):
|
||||
row.embedding = vec
|
||||
break
|
||||
@@ -660,6 +723,7 @@ async def _index_file(
|
||||
llm: Embedder,
|
||||
summary: ImportSummary,
|
||||
raw_date: datetime | None = None,
|
||||
on_chunk: Callable[[int, int], None] | None = None,
|
||||
) -> None:
|
||||
"""Upsert one file: doc row + chunk rows + embeddings, one transaction.
|
||||
|
||||
@@ -668,6 +732,15 @@ async def _index_file(
|
||||
map, or ``None`` (every non-git case): the file's mtime is read
|
||||
here, once, and becomes the source date (the D2 fallback).
|
||||
|
||||
``on_chunk`` (phase 129, task 02) is the in-flight file's
|
||||
chunk-position hook (see :func:`import_sources`) — passed through to
|
||||
ALL THREE branches' content embeds: the text path's
|
||||
:func:`_replace_content_chunks` directly, the image branch via
|
||||
:func:`_index_image_file` (its one description chunk reports
|
||||
``1/1`` — it IS the file's only chunk), and the PDF branch via
|
||||
:func:`_index_pdf_file` (its content embed is the primary target).
|
||||
``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
|
||||
@@ -683,7 +756,7 @@ async def _index_file(
|
||||
if match_extension(full_path, image_set) is not None:
|
||||
return await _index_image_file(
|
||||
session, source=source, rel=rel, full_path=full_path, llm=llm,
|
||||
summary=summary, raw_date=raw_date,
|
||||
summary=summary, raw_date=raw_date, on_chunk=on_chunk,
|
||||
)
|
||||
# Phase 126: PDF files take the PDF index path — AFTER the image
|
||||
# branch, BEFORE any text read (a PDF is binary; ``extract_pdf_text``
|
||||
@@ -691,7 +764,7 @@ async def _index_file(
|
||||
if full_path.suffix.lower() == ".pdf":
|
||||
return await _index_pdf_file(
|
||||
session, source=source, rel=rel, full_path=full_path, llm=llm,
|
||||
summary=summary, raw_date=raw_date,
|
||||
summary=summary, raw_date=raw_date, on_chunk=on_chunk,
|
||||
)
|
||||
content = full_path.read_text(encoding="utf-8", errors="replace").replace("\x00", "")
|
||||
digest = hashlib.sha256(content.encode("utf-8")).hexdigest()
|
||||
@@ -793,7 +866,7 @@ async def _index_file(
|
||||
# lives in the shared helper — the global policy stays intact for the
|
||||
# rest of the KB).
|
||||
chunks_n = await _replace_content_chunks(
|
||||
session, doc=doc, llm=llm, content=content, rel=rel
|
||||
session, doc=doc, llm=llm, content=content, rel=rel, on_chunk=on_chunk
|
||||
)
|
||||
session.commit()
|
||||
|
||||
@@ -952,6 +1025,7 @@ async def _index_pdf_file(
|
||||
llm: Embedder,
|
||||
summary: ImportSummary,
|
||||
raw_date: datetime | None = None,
|
||||
on_chunk: 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
|
||||
@@ -985,6 +1059,14 @@ async def _index_pdf_file(
|
||||
``raw_date`` follows the text path: the caller's git last-commit
|
||||
date or ``None`` (the mtime fallback, read before the unchanged
|
||||
early-return — the unchanged path refreshes from the same source).
|
||||
|
||||
``on_chunk`` (phase 129, task 02) is the in-flight file's
|
||||
chunk-position hook: it is forwarded to the PDF's CONTENT embed
|
||||
(the :func:`_replace_content_chunks` call — the primary target),
|
||||
but NOT to the embedded-image pass (those image docs are separate
|
||||
documents — their ``1/1`` positions would be noise after the
|
||||
file's real progress ends, the A6 exclusion). ``None`` keeps the
|
||||
branch byte-identical.
|
||||
"""
|
||||
# Extract the text (fail-soft: an unopenable PDF yields "" — the
|
||||
# document may still carry embedded images worth indexing).
|
||||
@@ -1066,9 +1148,11 @@ async def _index_pdf_file(
|
||||
# Phase 1+2 — the shared chunk/replace/embed pipeline (the page-
|
||||
# aware chunker dispatches on the ``.pdf`` suffix; empty content
|
||||
# clears the chunk rows and returns 0), committed atomically with
|
||||
# the document.
|
||||
# the document. Phase 129 (task 02): the in-flight file's
|
||||
# chunk-position hook rides the content embed (the embedded-image
|
||||
# pass does not receive it — separate documents, the A6 exclusion).
|
||||
chunks_n = await _replace_content_chunks(
|
||||
session, doc=doc, llm=llm, content=content, rel=rel
|
||||
session, doc=doc, llm=llm, content=content, rel=rel, on_chunk=on_chunk
|
||||
)
|
||||
session.commit()
|
||||
|
||||
@@ -1815,6 +1899,7 @@ async def _index_image_file(
|
||||
llm: Embedder,
|
||||
summary: ImportSummary,
|
||||
raw_date: datetime | None = None,
|
||||
on_chunk: Callable[[int, int], None] | None = None,
|
||||
) -> None:
|
||||
"""The phase-122 image branch of :func:`_index_file` — a standalone
|
||||
image is indexed from its BYTES, never its text:
|
||||
@@ -1855,6 +1940,13 @@ async def _index_image_file(
|
||||
pre-phase). The UNCHANGED path never re-scans (the stored context
|
||||
is part of the row's committed content; a changed source state
|
||||
re-indexes the file through this path).
|
||||
|
||||
``on_chunk`` (phase 129, task 02) is the in-flight file's
|
||||
chunk-position hook: it is forwarded to the description's content
|
||||
embed (:func:`_replace_content_chunks`). A description (enriched or
|
||||
not) chunks to ONE chunk, so the hook reports ``1/1`` — acceptable,
|
||||
it IS the file's only chunk. ``None`` keeps the branch
|
||||
byte-identical.
|
||||
"""
|
||||
settings = llm.settings
|
||||
data = full_path.read_bytes()
|
||||
@@ -1996,8 +2088,10 @@ async def _index_image_file(
|
||||
# the chunk rows (embeddings NULL), embed, commit — one transaction
|
||||
# per file; the endpoint token-cap retry lives in the shared helper,
|
||||
# and a description is short, so it never fires in practice).
|
||||
# Phase 129 (task 02): the in-flight file's chunk-position hook
|
||||
# rides this embed (one description chunk → ``1/1``).
|
||||
chunks_n = await _replace_content_chunks(
|
||||
session, doc=doc, llm=llm, content=content, rel=rel
|
||||
session, doc=doc, llm=llm, content=content, rel=rel, on_chunk=on_chunk
|
||||
)
|
||||
session.commit()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user