phase: 163_pdf_strip_additive_selfheal
All gates verified. Final report: **Phase 163 — final verification pass (all 5 tasks already complete; verification-only run)** - Verified D1–D4 in code: `page_strip_mode` column + migration 0027, `DocContent` absent-never-null rule keyed off `page_count`; `_persist_pdf_pages` render → atomic tmp+`os.replace` writes → stale-tail-last + `strip_class` stamp; `_regate_pdf_strip` on both unchanged paths (mirror gate, heal, repair, fail-soft, one INFO line only when acting); viewer mode branch + Text/Pages toggle (Text default, `aria-pressed`, no innerHTML, lazy imgs). - `uv run pytest --cov=app --cov-report=term-missing` → **3796 tests, exit 0, coverage 99%** (>90%) - `uv run pytest tests/e2e/test_pdf_strip_additive_selfheal.py -v --no-cov` → **5 passed** in isolation - `test_pdf_pages_in_sources + test_pictorial_pdf + test_garbage_pdf` → 10 passed; `test_document_viewer(_context) + test_image_documents` → 20 passed - `uv run ruff check .` clean; `uv run pyright` → 0 errors; alembic **0027 downgrade→upgrade CLI round-trip clean** (+ integration pins) - Completion criteria: additive view w/ toggle ✅ · primary DOM byte-identity ✅ · write-first order/mid-write-failure/atomic pins ✅ · heal + repair E2E ✅ · zero vision/LLM on fast path (mock-captured) ✅ · non-qualifying payload byte-identity ✅ · docs pass ✅ - **Documentation**: README.md + AGENTS.md reviewed in full — the phase's updates are accurate; all config-table defaults cross-checked against `app/config.py` (no drift), every page route, script, and E2E command referenced exists, every `.env.example` var documented — **verified consistent, no changes needed**. - Notable: live `Letumdei` repair is owner-runtime post-deploy per D5 (next production Sync self-heals; no live changes made); the modified `.agents/screenshots/doc_modal_scrollbar_1280.png` is an E2E-regenerated artifact left for the pipeline commit. - **Next pending phase:** `164_chat_error_honesty`
This commit is contained in:
@@ -206,6 +206,16 @@ def get_document_content(
|
||||
basename matches). ``None`` — a non-git source, no row, or an
|
||||
ambiguous basename — keeps the payload byte-identical to pre-phase
|
||||
(both fields are dropped by the serializer, never ``null``).
|
||||
|
||||
Phase 163 (task 01): a page-qualifying PDF's payload additionally
|
||||
carries ``page_strip_mode`` — ``"primary"`` (the phase-133 fallback
|
||||
class: the strip IS the view) or ``"additive"`` (the phase-140
|
||||
pictorial class: the text + Summary view with the strip behind a
|
||||
Text/Pages toggle). Both strip keys are built off ``page_count`` and
|
||||
dropped when it is NULL, so every non-qualifying payload stays
|
||||
byte-identical to pre-phase (and a pre-phase row, whose mode column
|
||||
is NULL, keeps the strip-only DOM the viewer assumes for an absent
|
||||
mode).
|
||||
"""
|
||||
row = db.execute(
|
||||
select(Document, func.count(Chunk.id).label("chunks"))
|
||||
@@ -254,6 +264,16 @@ def get_document_content(
|
||||
if doc.page_count is not None
|
||||
else None
|
||||
),
|
||||
# Phase 163 (task 01): the strip CLASS (``"primary"`` = the
|
||||
# fallback class, the strip IS the view; ``"additive"`` = the
|
||||
# pictorial class, text + a Text/Pages toggle). The whole strip
|
||||
# block keys off ``page_count`` — a row with a stray mode but no
|
||||
# strip serializes NEITHER key — and a pre-phase row (mode NULL)
|
||||
# serializes the key ABSENT (never null), which the viewer reads
|
||||
# as ``"primary"`` (task 04).
|
||||
page_strip_mode=(
|
||||
doc.page_strip_mode if doc.page_count is not None else None
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -198,6 +198,24 @@ class Document(Base):
|
||||
#: cleanup deletes the copies with the row. No index: a display
|
||||
#: fact, never a query surface (the ``file_hash`` convention, 0025).
|
||||
page_count: Mapped[int | None] = mapped_column(Integer, default=None)
|
||||
#: The STRIP CLASS behind a non-NULL :py:attr:`page_count` (phase 163,
|
||||
#: LOCKED A1/A3) — the signal the viewer branches on so a text-bearing
|
||||
#: book never loses its text:
|
||||
#:
|
||||
#: * ``"primary"`` — the phase-133 FALLBACK class (unusable text layer:
|
||||
#: the screenshots ARE the content): the strip IS the view, exactly
|
||||
#: as phase 148 shipped it;
|
||||
#: * ``"additive"`` — the phase-140 PICTORIAL class (usable text layer,
|
||||
#: ≥1 sparse page described): the text view + the Summary panel, with
|
||||
#: the strip one Text/Pages toggle away (Text is the default).
|
||||
#:
|
||||
#: NULL = no strip (every non-qualifying document) — and a pre-phase-163
|
||||
#: row keeps NULL while its ``page_count`` stands, which the wire treats
|
||||
#: as ``"primary"`` (today's DOM) until the fast-path re-gate stamps it.
|
||||
#: Stamped/cleared EXACTLY with ``page_count``: written by the importer's
|
||||
#: persist (task 02) and the unchanged-path re-gate (task 03) only. Like
|
||||
#: ``page_count`` it is a display fact, never a query surface — no index.
|
||||
page_strip_mode: Mapped[str | None] = mapped_column(String(16), default=None)
|
||||
|
||||
chunks: Mapped[list[Chunk]] = relationship(
|
||||
back_populates="document", cascade="all, delete-orphan"
|
||||
|
||||
+409
-71
@@ -190,15 +190,37 @@ dpi, every page) and persisted to ``settings.pdf_page_dir``
|
||||
``<doc-id>-p{N}.png`` (N 1-based), with ``documents.page_count``
|
||||
(migration 0026) recording the page count — a per-page render
|
||||
failure leaves a GAP (no file — the route 404s that N, the viewer's
|
||||
note covers it). The existing copies are deleted before replacement
|
||||
(a shrunk PDF's old ``p{N}`` tail cannot leak), a document that
|
||||
STOPS qualifying is cleared (``page_count`` NULL + copies deleted),
|
||||
and the prune deletes the copies with the row
|
||||
note covers it). WRITE-FIRST (phase 163, task 02, LOCKED A4): the
|
||||
fresh render's bytes land ATOMICALLY per file (``<name>.png.tmp`` +
|
||||
``os.replace``) BEFORE any old copy is removed — only the old-extent
|
||||
files the fresh render did NOT write (a shrunk PDF's ``p{N}`` tail)
|
||||
go, so a rollback or crash can never restore a stale ``page_count``
|
||||
over ZERO files. The row is stamped with the strip's CLASS beside the
|
||||
count (``documents.page_strip_mode``, migration 0027 — ``primary`` for
|
||||
the fallback class, ``additive`` for the pictorial class, phase 163);
|
||||
a document that STOPS qualifying is cleared (both columns NULL +
|
||||
copies deleted — deletion-first is correct where there is no
|
||||
replacement set), and the prune deletes the copies with the row
|
||||
(:func:`_delete_page_copies`). FAIL-SOFT: a render or write failure
|
||||
leaves ``page_count`` NULL and logs a warning — the document indexes
|
||||
EXACTLY as pre-phase (the render is a display concern, it never
|
||||
gates the indexing). The UNCHANGED fast path renders nothing (the
|
||||
copies persist — zero render work, zero orphans).
|
||||
leaves ``page_count``/``page_strip_mode`` NULL and logs a warning —
|
||||
the document indexes EXACTLY as pre-phase (the render is a display
|
||||
concern, it never gates the indexing).
|
||||
|
||||
PDF strip re-gate (phase 163, task 03, LOCKED A5): the UNCHANGED paths
|
||||
never assume "the copies persist" any more — a row that still claims a
|
||||
strip (``page_count`` set, a ``.pdf`` path) is re-gated
|
||||
deterministically from the CURRENT bytes + CURRENT settings
|
||||
(:func:`_regate_pdf_strip`, run at the tail of
|
||||
:func:`_unchanged_doc`): the class is re-stamped (``primary`` for the
|
||||
unusable-layer fallback class, ``additive`` for the sparse-page
|
||||
class), any missing ``<doc-id>-p{N}.png`` is RE-RENDERED through the
|
||||
same write-first atomic writer, and a strip that no longer qualifies
|
||||
(lowered ``pdf_pictorial_min_chars``, the kill switch off) is cleared
|
||||
with its copies. ZERO vision/LLM calls (the render is pure CPU, PLAN
|
||||
A21-safe), one greppable INFO line only when the re-gate ACTS, and
|
||||
fail-soft on every exception — the loss state (an external wipe, a
|
||||
pre-phase delete-first rollback) cannot survive one sync. Every other
|
||||
file's fast path still performs ZERO PDF parsing (the guard).
|
||||
|
||||
Image-anchor context (phase 125, task 03, LOCKED A1/A2/A3): BEFORE the
|
||||
description, the same source's importable markdown files are scanned for
|
||||
@@ -298,6 +320,7 @@ from __future__ import annotations
|
||||
|
||||
import hashlib
|
||||
import logging
|
||||
import os
|
||||
import posixpath
|
||||
import re
|
||||
import uuid
|
||||
@@ -1021,6 +1044,7 @@ async def _unchanged_doc(
|
||||
summary: ImportSummary,
|
||||
raw_date: datetime,
|
||||
file_digest: str,
|
||||
full_path: Path | None = None,
|
||||
) -> None:
|
||||
"""Phase 147 (task 02): the ONE shared unchanged path — the three
|
||||
branches' near-identical unchanged blocks, one code site, reached
|
||||
@@ -1064,7 +1088,22 @@ async def _unchanged_doc(
|
||||
the date block (the stamp set ``file_hash`` and the date did
|
||||
not change): quiet bookkeeping — no count, no log (the
|
||||
``unchanged`` line already fired; a date-change run already
|
||||
committed in step 5).
|
||||
committed in step 5);
|
||||
7. the phase-163 (task 03) PDF strip RE-GATE — a row that still
|
||||
claims a strip (``page_count`` set) whose path ends in ``.pdf``
|
||||
goes through :func:`_regate_pdf_strip`: the strip is re-derived
|
||||
from the CURRENT bytes + CURRENT settings (deterministic, ZERO
|
||||
LLM/vision calls) — a missing/changed ``page_strip_mode`` is
|
||||
re-stamped, copies that vanished from disk are RE-RENDERED, a
|
||||
strip that no longer qualifies is cleared — and whatever it
|
||||
wrote commits in this helper's transaction. Unchanged files
|
||||
re-gate the PDF strip deterministically (phase 163): the
|
||||
pre-phase "the copies persist" assumption is gone, so a wipe of
|
||||
``pdf_page_dir`` or a pre-phase delete-first rollback cannot
|
||||
outlive one sync. Every other file (``page_count`` NULL) pays
|
||||
ZERO extra read — the guard keeps the fast path's contract, and
|
||||
BOTH exits (the D1 manual-date return and the normal tail) run
|
||||
the re-gate.
|
||||
"""
|
||||
summary.unchanged += 1
|
||||
logger.info("import: unchanged source=%s path=%s", source, rel)
|
||||
@@ -1078,10 +1117,32 @@ async def _unchanged_doc(
|
||||
session, doc=doc, source=source, rel=rel, content=doc.content,
|
||||
llm=llm, summary=summary, backfill=True,
|
||||
)
|
||||
|
||||
def _regate_strip() -> None:
|
||||
"""Step 7 (phase 163, task 03): the GUARDED strip re-gate — only
|
||||
a row that carries a strip (``page_count`` set) on a ``.pdf``
|
||||
path reaches :func:`_regate_pdf_strip`, so every other file's
|
||||
fast path stays byte-identical (zero extra read, zero PDF
|
||||
parse). The re-gate never raises (fail-soft inside); its writes
|
||||
ride this helper's transaction."""
|
||||
if doc.page_count is None or not str(doc.path).lower().endswith(".pdf"):
|
||||
return
|
||||
_regate_pdf_strip(
|
||||
session,
|
||||
doc,
|
||||
full_path=full_path if full_path is not None else Path(doc.full_path),
|
||||
llm=llm,
|
||||
summary=summary,
|
||||
)
|
||||
if session.is_modified(doc):
|
||||
session.commit()
|
||||
|
||||
# 4 — the phase-106 (D1) manual-date lock: no date write; the
|
||||
# stamp, if dirty, is the only write (committed — see step 2's
|
||||
# "next sync takes the fast path").
|
||||
# "next sync takes the fast path") — plus whatever the phase-163
|
||||
# re-gate repaired.
|
||||
if doc.created_at_manual:
|
||||
_regate_strip()
|
||||
if session.is_modified(doc):
|
||||
session.commit()
|
||||
return
|
||||
@@ -1103,6 +1164,165 @@ async def _unchanged_doc(
|
||||
# 6 — the stamp's commit (the date did not move): quiet
|
||||
# bookkeeping — the ``unchanged`` line already fired.
|
||||
session.commit()
|
||||
# 7 — the phase-163 (task 03) strip re-gate (see the local helper).
|
||||
_regate_strip()
|
||||
|
||||
|
||||
def _regate_pdf_strip(
|
||||
session: Session,
|
||||
doc: Document,
|
||||
*,
|
||||
full_path: Path,
|
||||
llm: Embedder,
|
||||
summary: ImportSummary,
|
||||
) -> None:
|
||||
"""Phase 163 (task 03, LOCKED A5): the DETERMINISTIC re-gate of a
|
||||
PDF's display strip — run from the unchanged paths
|
||||
(:func:`_unchanged_doc`) whenever a row still claims a strip
|
||||
(``page_count`` set), so a strip can never die silently.
|
||||
|
||||
WHY (the 2026-10-07 live state, owner report): the ``Letumdei`` row
|
||||
carried ``page_count = 36`` over ZERO files — the pre-phase-163
|
||||
:func:`_persist_pdf_pages` deleted the old copies BEFORE rendering,
|
||||
OUTSIDE the DB transaction, so a later per-file rollback (the
|
||||
import loop's ``EmbeddingError`` handler) restored the stale count
|
||||
over an empty directory; an external wipe of ``pdf_page_dir`` lands
|
||||
exactly the same state. The phase-147 fast path believed "the
|
||||
copies persist" and never re-rendered, so every
|
||||
``GET /api/documents/<id>/pages/N`` 404'd and the viewer showed
|
||||
"Page N unavailable." on all 36 pages — on every sync, forever.
|
||||
|
||||
WHAT IT DOES — the changed-path gate mirrored exactly, from the
|
||||
CURRENT file bytes + CURRENT settings, with ZERO vision/LLM calls
|
||||
(:func:`app.rag.pdf.render_pdf_pages` is pure CPU — PLAN A21-safe,
|
||||
no model, no budget):
|
||||
|
||||
* ``not is_usable_pdf_text(extract_pdf_text(full_path))`` → the
|
||||
phase-133 fallback class → mode ``"primary"`` (the strip IS the
|
||||
view — the same doc as today);
|
||||
* else ``settings.pdf_pictorial`` ON and
|
||||
:func:`app.rag.pdf.pictorial_pages` NON-EMPTY over
|
||||
:func:`app.rag.pdf.extract_pdf_page_texts` → the phase-140
|
||||
pictorial class → mode ``"additive"`` (the text view + the strip
|
||||
behind a toggle). The candidates are the existing gate's
|
||||
VERBATIM (text-carrying pages under ``min_chars``; a page with no
|
||||
text has no key in the map and is not a candidate) — the re-gate
|
||||
adds NO new eligibility semantics;
|
||||
* else the strip no longer qualifies (the owner lowered
|
||||
``pdf_pictorial_min_chars``, the kill switch went off, the layer
|
||||
gained substance) → the REPAIR arm: the copies go
|
||||
(``_delete_page_copies`` reads ``doc.page_count``, so it runs
|
||||
while the count still describes the extent) and BOTH columns are
|
||||
cleared.
|
||||
|
||||
For a re-gated-ELIGIBLE doc the copies are VERIFIED: any missing
|
||||
``<doc-id>-p{N}.png`` in 1..``page_count`` re-renders EVERY page
|
||||
through the changed path's own write-first atomic writer
|
||||
(:func:`_write_page_copies` — tmp + ``os.replace``), then the
|
||||
stale tail beyond the new count goes and ``page_count`` follows the
|
||||
fresh render's length (a drift names both counts in a WARNING — on
|
||||
unchanged bytes a drift is pathological). A fresh render producing
|
||||
NO pages (unopenable right after the fast path hashed the file)
|
||||
leaves the row exactly as it was + a WARNING: the re-gate never
|
||||
clears a strip on a transient.
|
||||
|
||||
The mode re-stamp is SILENT when it only fills a NULL (a pre-phase
|
||||
row learning its class is beneficial bookkeeping — it rides the
|
||||
same INFO line only when real work followed); a change of an
|
||||
ALREADY-SET mode (the kill switch, a class flip) says so
|
||||
(``mode <old>-><new>``). The ONE greppable INFO line fires only
|
||||
when the re-gate ACTS — never per unchanged doc.
|
||||
|
||||
FAIL-SOFT: the whole body is wrapped — any exception is a WARNING
|
||||
and the sync continues (the strip is a display concern). A repair's
|
||||
DB writes ride the caller's transaction, so an exception raised
|
||||
after them rolls back with the file like any other per-file
|
||||
failure. *summary* keeps the unchanged path's signature shape (and
|
||||
a future counter site); the re-gate books nothing there — its stats
|
||||
are log-only (the phase-133/140 convention).
|
||||
"""
|
||||
source, rel = doc.source, doc.path
|
||||
try:
|
||||
page_dir = Path(llm.settings.pdf_page_dir).expanduser()
|
||||
# The gate, mirrored from the changed path (the phase-133
|
||||
# precedence over the phase-140 pass, the kill switch included).
|
||||
if not is_usable_pdf_text(extract_pdf_text(full_path)):
|
||||
strip_class: str | None = "fallback"
|
||||
elif llm.settings.pdf_pictorial and pictorial_pages(
|
||||
extract_pdf_page_texts(full_path),
|
||||
min_chars=llm.settings.pdf_pictorial_min_chars,
|
||||
):
|
||||
strip_class = "pictorial"
|
||||
else:
|
||||
strip_class = None
|
||||
if strip_class is None:
|
||||
# The REPAIR arm — the strip no longer qualifies: delete
|
||||
# while the count still names the extent, then clear both
|
||||
# columns (the same clearing the changed path does for a
|
||||
# doc that stopped qualifying).
|
||||
_delete_page_copies(doc, page_dir)
|
||||
doc.page_count = None
|
||||
doc.page_strip_mode = None
|
||||
logger.info(
|
||||
"import: PDF strip re-gate source=%s path=%s — cleared stale strip",
|
||||
source, rel,
|
||||
)
|
||||
return
|
||||
mode = "primary" if strip_class == "fallback" else "additive"
|
||||
old_mode = doc.page_strip_mode
|
||||
old_count = doc.page_count or 0
|
||||
actions: list[str] = []
|
||||
missing = [
|
||||
n
|
||||
for n in range(1, old_count + 1)
|
||||
if not (page_dir / f"{doc.id}-p{n}.png").is_file()
|
||||
]
|
||||
if missing:
|
||||
pages = render_pdf_pages(full_path)
|
||||
if not pages or not any(pages):
|
||||
# Unopenable (or every page a placeholder) — right
|
||||
# after the fast path hashed the file this is
|
||||
# pathological: KEEP the row as-is (never clear a strip
|
||||
# on a transient) and let the next sync try again.
|
||||
logger.warning(
|
||||
"import: PDF strip re-gate source=%s path=%s — re-render "
|
||||
"produced no pages (state kept)",
|
||||
source, rel,
|
||||
)
|
||||
return
|
||||
page_dir.mkdir(parents=True, exist_ok=True)
|
||||
written_ns = _write_page_copies(doc, page_dir, pages)
|
||||
_delete_stale_page_tail(
|
||||
doc_id=doc.id,
|
||||
page_dir=page_dir,
|
||||
old_extent=old_count,
|
||||
written_ns=written_ns,
|
||||
)
|
||||
doc.page_count = len(pages)
|
||||
actions.append(f"healed {len(missing)} pages")
|
||||
if doc.page_count != old_count:
|
||||
logger.warning(
|
||||
"import: PDF strip re-gate source=%s path=%s — page_count "
|
||||
"drift %s->%s on unchanged bytes",
|
||||
source, rel, old_count, doc.page_count,
|
||||
)
|
||||
if doc.page_strip_mode != mode:
|
||||
# NULL -> stamped stays SILENT (it rides the line only when
|
||||
# work followed); a change of an already-set mode is news.
|
||||
doc.page_strip_mode = mode
|
||||
if old_mode is not None:
|
||||
actions.append(f"mode {old_mode}->{mode}")
|
||||
if actions:
|
||||
logger.info(
|
||||
"import: PDF strip re-gate source=%s path=%s — %s",
|
||||
source, rel, " | ".join(actions),
|
||||
)
|
||||
except Exception as e:
|
||||
# Fail-soft: the strip never breaks a sync.
|
||||
logger.warning(
|
||||
"import: PDF strip re-gate failed source=%s path=%s — %s",
|
||||
source, rel, e,
|
||||
)
|
||||
|
||||
|
||||
async def _index_file(
|
||||
@@ -1186,7 +1406,7 @@ async def _index_file(
|
||||
# helper, nothing else does.
|
||||
await _unchanged_doc(
|
||||
session, doc, source=source, rel=rel, llm=llm, summary=summary,
|
||||
raw_date=raw_date, file_digest=file_digest,
|
||||
raw_date=raw_date, file_digest=file_digest, full_path=full_path,
|
||||
)
|
||||
return
|
||||
# Phase 122 (task 02): the image branch FIRST. Only reachable while
|
||||
@@ -1223,7 +1443,7 @@ async def _index_file(
|
||||
# file hash, so the NEXT sync takes the fast path above.
|
||||
await _unchanged_doc(
|
||||
session, doc, source=source, rel=rel, llm=llm, summary=summary,
|
||||
raw_date=raw_date, file_digest=file_digest,
|
||||
raw_date=raw_date, file_digest=file_digest, full_path=full_path,
|
||||
)
|
||||
return
|
||||
|
||||
@@ -1506,19 +1726,24 @@ async def _index_pdf_file(
|
||||
(:func:`_unchanged_doc`) — no re-extract, no re-embed, no image
|
||||
pass, the backfill/manual-date/date-refresh semantics in one
|
||||
code site — and the helper stamps the new file hash (the next
|
||||
sync takes the fast path). This path renders NO pages and
|
||||
touches NO ``page_count`` (the copies persist — zero render
|
||||
work).
|
||||
sync takes the fast path). This path renders NO pages through
|
||||
THIS branch, but a row that carries a strip is re-gated by the
|
||||
helper's phase-163 tail (:func:`_regate_pdf_strip` — the class
|
||||
re-stamped, missing copies healed, a stale strip cleared; zero
|
||||
vision/LLM work).
|
||||
* Phase 148 (task 01): in this NEW/CHANGED path, RIGHT AFTER the
|
||||
row's flush and BEFORE the chunk pipeline, a page-QUALIFYING
|
||||
PDF (the phase-133 fallback fired, or the phase-140 pictorial
|
||||
pass described ≥1 page — the signals recorded above) gets its
|
||||
every page rendered to ``settings.pdf_page_dir`` as
|
||||
``<doc-id>-p{N}.png`` and ``documents.page_count`` set
|
||||
(:func:`_persist_pdf_pages` — stale copies deleted first, a
|
||||
stopped-qualifier cleared, a per-page ``b""`` a gap). FAIL-SOFT
|
||||
— a render/write exception leaves ``page_count`` NULL + a
|
||||
warning; the document indexes exactly as pre-phase.
|
||||
``<doc-id>-p{N}.png`` and ``documents.page_count`` +
|
||||
``documents.page_strip_mode`` set
|
||||
(:func:`_persist_pdf_pages` — WRITE-FIRST (phase 163, LOCKED
|
||||
A4): the new copies land atomically per file BEFORE any old copy
|
||||
is removed, a stopped-qualifier cleared, a per-page ``b""`` a
|
||||
gap). FAIL-SOFT — a render/write exception leaves
|
||||
``page_count``/``page_strip_mode`` NULL + a warning; the
|
||||
document indexes exactly as pre-phase.
|
||||
* THEN the embedded images: :func:`_index_pdf_embedded_images`
|
||||
(A3 — the phase-122 pipeline, gated by the ``images`` toggle).
|
||||
|
||||
@@ -1631,7 +1856,7 @@ async def _index_pdf_file(
|
||||
# fast path).
|
||||
await _unchanged_doc(
|
||||
session, doc, source=source, rel=rel, llm=llm, summary=summary,
|
||||
raw_date=raw_date, file_digest=file_digest,
|
||||
raw_date=raw_date, file_digest=file_digest, full_path=full_path,
|
||||
)
|
||||
return
|
||||
|
||||
@@ -1673,23 +1898,34 @@ async def _index_pdf_file(
|
||||
# its id — the copy names need it) and BEFORE the chunk pipeline
|
||||
# (the count commits ATOMICALLY with the doc + chunks). Runs ONLY
|
||||
# in this new/changed path: the phase-147 fast path returned
|
||||
# before the branch (an unchanged PDF re-renders NOTHING — the
|
||||
# copies persist) and the content-digest hit above took the shared
|
||||
# unchanged path (no page work there either). FAIL-SOFT: the
|
||||
# render is a DISPLAY concern — any exception leaves
|
||||
# ``page_count`` NULL, the warning is the signal, and the document
|
||||
# proceeds to chunk/embed/summary EXACTLY as pre-phase.
|
||||
# before the branch and the content-digest hit above took the
|
||||
# shared unchanged path — neither renders here; the phase-163
|
||||
# task-03 re-gate (:func:`_regate_pdf_strip`, run by
|
||||
# :func:`_unchanged_doc`) owns the repair of an unchanged doc's
|
||||
# strip. FAIL-SOFT: the render is a DISPLAY
|
||||
# concern — any exception leaves ``page_count``/``page_strip_mode``
|
||||
# NULL, the warning is the signal, and the document proceeds to
|
||||
# chunk/embed/summary EXACTLY as pre-phase.
|
||||
# Phase 163 (task 02): the eligibility decision carried as a CLASS
|
||||
# (LOCKED A7 — the truth table is the phase-148 rule verbatim: a
|
||||
# non-None class ⇔ fallback_fired or pictorial_described). The
|
||||
# class decides the stamped `page_strip_mode` ("fallback" →
|
||||
# "primary", "pictorial" → "additive"), nothing else.
|
||||
strip_class = (
|
||||
"fallback" if fallback_fired else ("pictorial" if pictorial_described else None)
|
||||
)
|
||||
try:
|
||||
_persist_pdf_pages(
|
||||
doc,
|
||||
full_path=full_path,
|
||||
page_dir=Path(llm.settings.pdf_page_dir).expanduser(),
|
||||
eligible=fallback_fired or pictorial_described,
|
||||
strip_class=strip_class,
|
||||
source=source,
|
||||
rel=rel,
|
||||
)
|
||||
except Exception as e:
|
||||
doc.page_count = None
|
||||
doc.page_strip_mode = None
|
||||
logger.warning(
|
||||
"import: PDF page render failed source=%s path=%s — %s",
|
||||
source, rel, e,
|
||||
@@ -1991,84 +2227,186 @@ def _persist_pdf_pages(
|
||||
*,
|
||||
full_path: Path,
|
||||
page_dir: Path,
|
||||
eligible: bool,
|
||||
strip_class: str | None,
|
||||
source: str,
|
||||
rel: str,
|
||||
) -> None:
|
||||
"""Phase 148 (task 01): the persistent page copies of one NEW/CHANGED
|
||||
PDF — the display strip's bytes (task 02's public route serves them).
|
||||
"""Phase 148 (task 01) + phase 163 (task 02): the persistent page
|
||||
copies of one NEW/CHANGED PDF — the display strip's bytes (task 02's
|
||||
public route serves them) — written WRITE-FIRST and stamped with the
|
||||
strip's CLASS.
|
||||
|
||||
* ELIGIBLE (the phase-133 fallback fired, or the phase-140
|
||||
pictorial pass described ≥1 page — the sync's own decisions, see
|
||||
the caller's signals): EVERY page of the PDF is rendered (150 dpi,
|
||||
*strip_class* is the sync's eligibility decision carried as a CLASS
|
||||
(phase 163, LOCKED A7): ``"fallback"`` (the phase-133 fallback fired
|
||||
— the screenshots ARE the content → stamped mode ``"primary"``),
|
||||
``"pictorial"`` (the phase-140 pass described ≥1 sparse page →
|
||||
stamped mode ``"additive"``), or ``None`` (no strip). The truth
|
||||
table is the phase-148 eligibility rule VERBATIM: a non-None class
|
||||
⇔ the old ``fallback_fired or pictorial_described`` — this phase
|
||||
changes only what the class DOES (the stamp), never who qualifies.
|
||||
|
||||
* ELIGIBLE: EVERY page of the PDF is rendered (150 dpi,
|
||||
:func:`render_pdf_pages`' default — the phase-133 constant; the
|
||||
``pages=None`` "list index *i* is page *i*+1" contract) and each
|
||||
NON-EMPTY PNG is written to *page_dir* as
|
||||
``<doc-id>-p{N}.png`` (N 1-based), then ``doc.page_count`` is set
|
||||
to the PDF's page count (``len(pages)`` — one slot per page, so
|
||||
the list IS the page count for an openable file). A per-page
|
||||
NON-EMPTY PNG lands in *page_dir* as ``<doc-id>-p{N}.png``
|
||||
(N 1-based) through an ATOMIC per-file write — the shared writer
|
||||
:func:`_write_page_copies` puts the bytes on ``<name>.png.tmp``
|
||||
first and ``os.replace`` moves them onto the final name, so a
|
||||
concurrent reader of the route NEVER sees a torn PNG (the phase
|
||||
163 task-03 re-gate heals a strip through THE SAME writer) — and
|
||||
the success tail stamps ``doc.page_count = len(pages)``
|
||||
(one slot per page — the list IS the page count for an openable
|
||||
file) and ``doc.page_strip_mode`` from the class. A per-page
|
||||
``b""`` (an unreadable page) is a GAP, not an error: no file is
|
||||
written for it (the route 404s that N, the viewer's note covers
|
||||
it), and the count still covers the full range.
|
||||
* the STALE-REPLACEMENT convention (phase 122's image copies): the
|
||||
doc's EXISTING copies (1..``doc.page_count`` — the row knows its
|
||||
own extent, read BEFORE any change) are deleted FIRST, so a
|
||||
changed PDF's old page set (e.g. 30 pages → 24) cannot leak
|
||||
``p25..p30``; a fresh doc's NULL count is a no-op.
|
||||
* NOT eligible, or an unopenable file (:func:`render_pdf_pages` →
|
||||
``[]``), or a render with ZERO non-empty pages: ``page_count`` is
|
||||
None + the existing copies deleted — a document that STOPPED
|
||||
qualifying loses its strip (e.g. a new version gained a real text
|
||||
layer).
|
||||
* FAIL-SOFT: a write failure removes the copies THIS CALL wrote
|
||||
(no partial-write orphans) and re-raises — the caller
|
||||
(:func:`_index_pdf_file`) catches, leaves ``page_count`` NULL,
|
||||
logs the warning, and the document indexes exactly as pre-phase
|
||||
(the render is a display concern — it never gates indexing).
|
||||
* WRITE-FIRST (phase 163, LOCKED A4 — the 2026-10-07 live loss): the
|
||||
OLD copies are deleted LAST, never first. The pre-phase order
|
||||
(blanket delete → render → write) ran OUTSIDE the DB transaction,
|
||||
so any later per-file rollback (the import loop's
|
||||
``EmbeddingError`` handler) — or a crash between delete and write —
|
||||
restored the stale ``page_count`` over ZERO files and the strip
|
||||
404ed forever. Now only the old-extent files (1..the count read
|
||||
BEFORE any change) that the fresh render did NOT write — a shrunk
|
||||
PDF's ``p{N}`` tail, a new gap — are removed, and only once every
|
||||
replacement byte is durably in place; a file about to be
|
||||
overwritten is never deleted at all.
|
||||
* NOT eligible (``strip_class is None``), an unopenable file
|
||||
(:func:`render_pdf_pages` → ``[]``), or a render with ZERO
|
||||
non-empty pages: the existing copies are deleted (the helper reads
|
||||
``doc.page_count``, so it runs BEFORE the nulling) and BOTH
|
||||
``page_count`` and ``page_strip_mode`` are cleared — the
|
||||
deletion-first order is correct HERE, there is no replacement set
|
||||
to protect; a document that STOPPED qualifying loses its strip
|
||||
(e.g. a new version gained a real text layer).
|
||||
* FAIL-SOFT: a write failure removes this call's ``.tmp`` files and
|
||||
the final-name files it CREATED — but NEVER a file it OVERWROTE
|
||||
(the replaced copy keeps standing as the durable new bytes; a
|
||||
failed call may change a copy's bytes, it may never remove a
|
||||
copy) — and re-raises: the caller (:func:`_index_pdf_file`)
|
||||
catches, leaves ``page_count`` AND ``page_strip_mode`` NULL, logs
|
||||
the warning, and the document indexes exactly as pre-phase (the
|
||||
render is a display concern — it never gates indexing).
|
||||
|
||||
*page_dir* is ``Path(settings.pdf_page_dir).expanduser()`` (the
|
||||
``image_dir`` convention — raw string, expanduser'd by the
|
||||
importer); the directory is created on demand, only when a page is
|
||||
actually written (an ineligible doc never creates it).
|
||||
"""
|
||||
# 1 — the stale-replacement: this doc's EXISTING copies go first —
|
||||
# a changed PDF's old page set must not leak, and a doc that
|
||||
# stopped qualifying loses its strip (a fresh doc's NULL count is
|
||||
# a no-op; missing files are a silent no-op).
|
||||
_delete_page_copies(doc, page_dir)
|
||||
if not eligible:
|
||||
if strip_class is None:
|
||||
# No strip — the DELETION-first order is correct here (there is
|
||||
# no replacement set to protect); the helper reads
|
||||
# doc.page_count, so it runs BEFORE the nulling (a fresh doc's
|
||||
# NULL count is a no-op; missing files are a silent no-op).
|
||||
_delete_page_copies(doc, page_dir)
|
||||
doc.page_count = None
|
||||
doc.page_strip_mode = None
|
||||
return
|
||||
pages = render_pdf_pages(full_path)
|
||||
if not pages or not any(pages):
|
||||
# Unopenable file (the render's ``[]``) or every page a
|
||||
# placeholder: no strip — fail-soft, the index is untouched
|
||||
# (the render helper already logged the per-page failures).
|
||||
# placeholder: no strip and NO replacement set — the existing
|
||||
# copies go with the cleared row (helper BEFORE the nulling),
|
||||
# fail-soft, the index is untouched (the render helper already
|
||||
# logged the per-page failures).
|
||||
logger.warning(
|
||||
"import: PDF page render produced no pages source=%s path=%s",
|
||||
source, rel,
|
||||
)
|
||||
_delete_page_copies(doc, page_dir)
|
||||
doc.page_count = None
|
||||
doc.page_strip_mode = None
|
||||
return
|
||||
# The row's OWN old extent, read BEFORE any change — the stale-tail
|
||||
# delete below walks it (the write-first contract never deletes a
|
||||
# file outside this extent, and never before its replacement is
|
||||
# durable).
|
||||
old_extent = doc.page_count or 0
|
||||
# 1 — WRITE FIRST, atomically per file (the shared writer, phase
|
||||
# 163 task 02 — the phase-163 task-03 re-gate heals through it).
|
||||
written_ns = _write_page_copies(doc, page_dir, pages)
|
||||
# 2 — the stale tail goes LAST.
|
||||
_delete_stale_page_tail(
|
||||
doc_id=doc.id,
|
||||
page_dir=page_dir,
|
||||
old_extent=old_extent,
|
||||
written_ns=written_ns,
|
||||
)
|
||||
doc.page_count = len(pages)
|
||||
doc.page_strip_mode = "primary" if strip_class == "fallback" else "additive"
|
||||
|
||||
|
||||
def _write_page_copies(doc: Document, page_dir: Path, pages: Sequence[bytes]) -> set[int]:
|
||||
"""Phase 163 (task 02, LOCKED A4; extracted for the task-03 re-gate,
|
||||
which heals a strip through the SAME writer): write every NON-EMPTY
|
||||
PNG of *pages* to ``page_dir/<doc-id>-p{N}.png`` (N 1-based) and
|
||||
return the set of page numbers written.
|
||||
|
||||
* ATOMIC per file — the bytes land on ``<name>.png.tmp`` and
|
||||
``os.replace`` moves them onto the final name, so a concurrent
|
||||
reader of the ``/pages/{n}`` route never sees a torn PNG.
|
||||
* A per-page ``b""`` (the render's failure placeholder) is the GAP:
|
||||
no file, no entry in the returned set.
|
||||
* The directory is created on demand (a doc that never writes never
|
||||
creates it).
|
||||
* A failure leaves NO orphan: this call's ``.tmp`` files and the
|
||||
finals it CREATED are removed — an OVERWRITTEN final keeps
|
||||
standing (deleting it would re-open the 2026-10-07 hole with one
|
||||
page instead of the whole strip) — and the exception re-raises so
|
||||
the caller keeps its fail-soft contract.
|
||||
"""
|
||||
page_dir.mkdir(parents=True, exist_ok=True)
|
||||
written: list[Path] = []
|
||||
written_tmps: list[Path] = []
|
||||
created_finals: list[Path] = [] # finals that did NOT exist before
|
||||
written_ns: set[int] = set()
|
||||
try:
|
||||
for n, png in enumerate(pages, start=1):
|
||||
if not png:
|
||||
# A per-page render failure: the GAP (no file — the
|
||||
# route 404s that N, the viewer's note covers it).
|
||||
continue
|
||||
path = page_dir / f"{doc.id}-p{n}.png"
|
||||
path.write_bytes(png)
|
||||
written.append(path)
|
||||
final = page_dir / f"{doc.id}-p{n}.png"
|
||||
tmp = page_dir / f"{doc.id}-p{n}.png.tmp"
|
||||
# Track the tmp BEFORE the write: a failure mid-write may
|
||||
# leave it on disk, and the cleanup must find it either way.
|
||||
written_tmps.append(tmp)
|
||||
existed = final.exists()
|
||||
tmp.write_bytes(png)
|
||||
os.replace(tmp, final) # atomic per file — no torn reads
|
||||
written_ns.add(n)
|
||||
if not existed:
|
||||
created_finals.append(final)
|
||||
except Exception:
|
||||
# A partial write leaves no orphan: the copies this call wrote
|
||||
# go, then the re-raise hands the failure to the caller's
|
||||
# fail-soft handler (page_count NULL + the warning).
|
||||
for path in written:
|
||||
# A partial write leaves no orphan: this call's tmp files and
|
||||
# the finals it CREATED go (an OVERWRITTEN final keeps standing
|
||||
# — deleting it would re-open the 2026-10-07 hole with one page
|
||||
# instead of the whole strip), then the re-raise hands the
|
||||
# failure to the caller's fail-soft handler (page_count +
|
||||
# page_strip_mode NULL + the warning).
|
||||
for path in [*written_tmps, *created_finals]:
|
||||
path.unlink(missing_ok=True)
|
||||
raise
|
||||
doc.page_count = len(pages)
|
||||
return written_ns
|
||||
|
||||
|
||||
def _delete_stale_page_tail(
|
||||
*, doc_id: uuid.UUID, page_dir: Path, old_extent: int, written_ns: set[int]
|
||||
) -> None:
|
||||
"""Phase 163 (task 02, LOCKED A4 — the LAST step of the write-first
|
||||
order): remove the files of the row's OLD extent (1..*old_extent*)
|
||||
the fresh render did NOT write — a shrunk PDF's ``p{N}`` tail, a new
|
||||
gap. Runs only once every replacement byte is durable, never touches
|
||||
a file outside the old extent, and never touches a file that was just
|
||||
overwritten. Best-effort like :func:`_delete_page_copies`: an
|
||||
unreadable file is logged, never raised (the sync survives)."""
|
||||
for n in range(1, old_extent + 1):
|
||||
if n in written_ns:
|
||||
continue
|
||||
path = page_dir / f"{doc_id}-p{n}.png"
|
||||
try:
|
||||
path.unlink(missing_ok=True)
|
||||
except OSError as e:
|
||||
logger.warning("import: could not delete PDF page copy %s — %s", path, e)
|
||||
|
||||
|
||||
async def _index_pdf_embedded_images(
|
||||
@@ -3271,7 +3609,7 @@ async def _index_image_file(
|
||||
# ``_store_summary`` backfill re-stores it verbatim.
|
||||
await _unchanged_doc(
|
||||
session, doc, source=source, rel=rel, llm=llm, summary=summary,
|
||||
raw_date=raw_date, file_digest=file_digest,
|
||||
raw_date=raw_date, file_digest=file_digest, full_path=full_path,
|
||||
)
|
||||
return
|
||||
|
||||
|
||||
@@ -636,6 +636,19 @@ class DocContent(BaseModel):
|
||||
#: ``null``, the :attr:`image_url` omission precedent), so a
|
||||
#: non-qualifying doc's payload is byte-identical to pre-phase.
|
||||
page_urls: list[str] | None = None
|
||||
#: Phase 163 (task 01, LOCKED A2/A3) — the STRIP CLASS behind
|
||||
#: :attr:`page_urls`: ``"primary"`` = the phase-133 fallback class
|
||||
#: (an unusable text layer — the screenshots ARE the content, so the
|
||||
#: strip IS the view, exactly as phase 148 shipped it); ``"additive"``
|
||||
#: = the phase-140 pictorial class (a usable text layer with ≥1 sparse
|
||||
#: page described — the text view + the Summary panel, the strip one
|
||||
#: Text/Pages toggle away). ABSENT from the wire whenever there is no
|
||||
#: strip (``None`` → dropped by the serializer — never ``null``, the
|
||||
#: :attr:`image_url` omission rule), which is ALSO how a pre-phase row
|
||||
#: (``page_count`` set, the column NULL) serializes: the viewer treats
|
||||
#: an absent mode as ``"primary"`` (task 04), so every existing row
|
||||
#: keeps today's DOM until the fast-path re-gate stamps it.
|
||||
page_strip_mode: str | None = None
|
||||
|
||||
@model_serializer(mode="wrap")
|
||||
def _serialize(self, handler: SerializerFunctionWrapHandler) -> Any:
|
||||
@@ -648,6 +661,11 @@ class DocContent(BaseModel):
|
||||
data.pop("origin_branch", None)
|
||||
if self.page_urls is None:
|
||||
data.pop("page_urls", None)
|
||||
# Phase 163 (task 01): the strip class rides the strip — ABSENT
|
||||
# (never null) with no strip, so a non-qualifying doc's payload is
|
||||
# byte-identical to pre-phase.
|
||||
if self.page_strip_mode is None:
|
||||
data.pop("page_strip_mode", None)
|
||||
return data
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user