diff --git a/.agents/phases/todo/163_pdf_strip_additive_selfheal/00_phase.md b/.agents/phases/complete/163_pdf_strip_additive_selfheal/00_phase.md similarity index 100% rename from .agents/phases/todo/163_pdf_strip_additive_selfheal/00_phase.md rename to .agents/phases/complete/163_pdf_strip_additive_selfheal/00_phase.md diff --git a/.agents/phases/todo/163_pdf_strip_additive_selfheal/01_schema_wire_strip_mode.md b/.agents/phases/complete/163_pdf_strip_additive_selfheal/01_schema_wire_strip_mode.md similarity index 100% rename from .agents/phases/todo/163_pdf_strip_additive_selfheal/01_schema_wire_strip_mode.md rename to .agents/phases/complete/163_pdf_strip_additive_selfheal/01_schema_wire_strip_mode.md diff --git a/.agents/phases/todo/163_pdf_strip_additive_selfheal/02_persist_write_first.md b/.agents/phases/complete/163_pdf_strip_additive_selfheal/02_persist_write_first.md similarity index 100% rename from .agents/phases/todo/163_pdf_strip_additive_selfheal/02_persist_write_first.md rename to .agents/phases/complete/163_pdf_strip_additive_selfheal/02_persist_write_first.md diff --git a/.agents/phases/todo/163_pdf_strip_additive_selfheal/03_fast_path_regate.md b/.agents/phases/complete/163_pdf_strip_additive_selfheal/03_fast_path_regate.md similarity index 100% rename from .agents/phases/todo/163_pdf_strip_additive_selfheal/03_fast_path_regate.md rename to .agents/phases/complete/163_pdf_strip_additive_selfheal/03_fast_path_regate.md diff --git a/.agents/phases/todo/163_pdf_strip_additive_selfheal/04_viewer_additive_toggle.md b/.agents/phases/complete/163_pdf_strip_additive_selfheal/04_viewer_additive_toggle.md similarity index 100% rename from .agents/phases/todo/163_pdf_strip_additive_selfheal/04_viewer_additive_toggle.md rename to .agents/phases/complete/163_pdf_strip_additive_selfheal/04_viewer_additive_toggle.md diff --git a/.agents/phases/todo/163_pdf_strip_additive_selfheal/05_e2e_selfheal_pins.md b/.agents/phases/complete/163_pdf_strip_additive_selfheal/05_e2e_selfheal_pins.md similarity index 100% rename from .agents/phases/todo/163_pdf_strip_additive_selfheal/05_e2e_selfheal_pins.md rename to .agents/phases/complete/163_pdf_strip_additive_selfheal/05_e2e_selfheal_pins.md diff --git a/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__00_phase.a1.err b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__00_phase.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__00_phase.a1.md b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__00_phase.a1.md new file mode 100644 index 0000000..06c8d37 --- /dev/null +++ b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__00_phase.a1.md @@ -0,0 +1,13 @@ +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` diff --git a/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__00_phase.a1.md.status b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__00_phase.a1.md.status new file mode 100644 index 0000000..9766475 --- /dev/null +++ b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__00_phase.a1.md.status @@ -0,0 +1 @@ +ok diff --git a/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__00_phase.a1.validate b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__00_phase.a1.validate new file mode 100644 index 0000000..481cc35 --- /dev/null +++ b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__00_phase.a1.validate @@ -0,0 +1,124 @@ +........................................................................ [ 1%] +........................................................................ [ 3%] +........................................................................ [ 5%] +........................................................................ [ 7%] +........................................................................ [ 9%] +........................................................................ [ 11%] +........................................................................ [ 13%] +........................................................................ [ 15%] +........................................................................ [ 17%] +........................................................................ [ 18%] +........................................................................ [ 20%] +........................................................................ [ 22%] +........................................................................ [ 24%] +........................................................................ [ 26%] +........................................................................ [ 28%] +........................................................................ [ 30%] +........................................................................ [ 32%] +........................................................................ [ 34%] +........................................................................ [ 36%] +........................................................................ [ 37%] +........................................................................ [ 39%] +........................................................................ [ 41%] +........................................................................ [ 43%] +........................................................................ [ 45%] +........................................................................ [ 47%] +........................................................................ [ 49%] +........................................................................ [ 51%] +........................................................................ [ 53%] +........................................................................ [ 55%] +........................................................................ [ 56%] +........................................................................ [ 58%] +........................................................................ [ 60%] +........................................................................ [ 62%] +........................................................................ [ 64%] +........................................................................ [ 66%] +........................................................................ [ 68%] +........................................................................ [ 70%] +........................................................................ [ 72%] +........................................................................ [ 73%] +........................................................................ [ 75%] +........................................................................ [ 77%] +........................................................................ [ 79%] +........................................................................ [ 81%] +........................................................................ [ 83%] +........................................................................ [ 85%] +........................................................................ [ 87%] +........................................................................ [ 89%] +........................................................................ [ 91%] +........................................................................ [ 92%] +........................................................................ [ 94%] +........................................................................ [ 96%] +........................................................................ [ 98%] +.................................................... [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +-------------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 52 0 100% +app/api/chat.py 277 1 99% +app/api/chat_images.py 50 0 100% +app/api/chats.py 110 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 99 0 100% +app/api/docs.py 196 1 99% +app/api/git_sources.py 259 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 204 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 266 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 45 0 100% +app/core/caching.py 124 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/errors.py 5 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/security_headers.py 20 0 100% +app/core/theming.py 38 0 100% +app/core/tokens.py 44 0 100% +app/db.py 22 0 100% +app/main.py 68 0 100% +app/models.py 136 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 454 1 99% +app/rag/archive_upload.py 134 0 100% +app/rag/chunker.py 220 4 98% +app/rag/doc_dates.py 18 0 100% +app/rag/folder_summaries.py 134 0 100% +app/rag/git_sources.py 77 0 100% +app/rag/importer.py 841 4 99% +app/rag/llm.py 246 1 99% +app/rag/md_images.py 63 0 100% +app/rag/overview.py 78 0 100% +app/rag/pdf.py 192 0 100% +app/rag/prompts.py 156 0 100% +app/rag/retriever.py 252 3 99% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 58 0 100% +app/schemas.py 473 0 100% +-------------------------------------------------- +TOTAL 5869 17 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +WARNING: there is a new pyright version available (v1.1.411 -> v1.1.414). +Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest` + +validation OK diff --git a/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__01_schema_wire_strip_mode.a1.err b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__01_schema_wire_strip_mode.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__01_schema_wire_strip_mode.a1.md b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__01_schema_wire_strip_mode.a1.md new file mode 100644 index 0000000..e5c4224 --- /dev/null +++ b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__01_schema_wire_strip_mode.a1.md @@ -0,0 +1,21 @@ +All green. Final state: + +**Implemented (task 01 — phase 163 D1, LOCKED A2/A3)** +- `app/models.py`: `Document.page_strip_mode` (VARCHAR(16), nullable, no server default, unindexed) right after `page_count`, docstringed as the strip CLASS (`primary` = fallback class, `additive` = pictorial class, NULL = no strip). +- `alembic/versions/0027_documents_page_strip_mode.py`: additive add/drop migration mirroring 0026; `upgrade head → downgrade -1 → upgrade head` round-trips clean (DB now at `0027 (head)`). +- `app/schemas.py`: `DocContent.page_strip_mode` + the `image_url`-style omission in the existing `model_serializer` (ABSENT, never `null`). +- `app/api/docs.py`: content payload passes `page_strip_mode=doc.page_strip_mode if doc.page_count is not None else None` — the whole strip block keys off `page_count`, so a stray mode never travels alone; `/pages/{n}` untouched; endpoint docstring + README "Which kind of strip it is (phase 163)" section updated. No AGENTS.md change (its viewer bullets are the final pass's). +- Tests: `tests/integration/test_migration_0027.py` (new, house pattern: contract + pre-phase `page_count=36`/mode-NULL backfill + ORM round-trip both classes + tested downgrade + round-trip), `tests/unit/test_models.py` (column contract + ORM defaults), `tests/unit/test_document_viewer.py` (model omission rule; endpoint: stamped primary/additive, pre-phase mode-NULL row → key absent with `page_urls` kept, stray mode + `page_count` NULL → neither key, exact non-qualifying key-set), `tests/unit/test_pdf_page_copies.py` (importer leaves the class unstamped until task 02), `tests/integration/test_docs_api.py` (`_seed_page_doc` gains `page_strip_mode`; real-DB class pins + never-alone + exact key set). + +**Gates** +- `uv run pytest` → **3772 passed**; `uv run pytest --cov=app --cov-report=term-missing` → **3772 passed, app TOTAL 99%** (`app/schemas.py` 100%). +- `uv run ruff check .` → All checks passed; `uv run pyright` → 0 errors. +- `uv run alembic upgrade head && downgrade -1 && upgrade head` → clean, `current` = `0027 (head)`. +- E2E regression (isolation, `--no-cov`): `test_pdf_pages_in_sources.py` 4 passed, `test_pictorial_pdf.py`+`test_garbage_pdf.py` 6 passed, `test_smoke.py` 3 passed. + +**Notes / decisions** +- Non-qualifying payloads stay byte-identical (both strip keys dropped when `page_count` is NULL); pre-phase rows serialize the class ABSENT and the viewer will read that as `primary` (task 04) — pinned, no data backfill. +- Column carries no CHECK constraint (purely additive per A13); the class values are enforced by the writers (tasks 02/03). +- No live-infrastructure changes needed for this task. + +**Next pending task:** `163_pdf_strip_additive_selfheal/02_persist_write_first.md`. diff --git a/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__01_schema_wire_strip_mode.a1.md.status b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__01_schema_wire_strip_mode.a1.md.status new file mode 100644 index 0000000..9766475 --- /dev/null +++ b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__01_schema_wire_strip_mode.a1.md.status @@ -0,0 +1 @@ +ok diff --git a/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__01_schema_wire_strip_mode.a1.validate b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__01_schema_wire_strip_mode.a1.validate new file mode 100644 index 0000000..9a8ee95 --- /dev/null +++ b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__01_schema_wire_strip_mode.a1.validate @@ -0,0 +1,124 @@ +........................................................................ [ 1%] +........................................................................ [ 3%] +........................................................................ [ 5%] +........................................................................ [ 7%] +........................................................................ [ 9%] +........................................................................ [ 11%] +........................................................................ [ 13%] +........................................................................ [ 15%] +........................................................................ [ 17%] +........................................................................ [ 19%] +........................................................................ [ 20%] +........................................................................ [ 22%] +........................................................................ [ 24%] +........................................................................ [ 26%] +........................................................................ [ 28%] +........................................................................ [ 30%] +........................................................................ [ 32%] +........................................................................ [ 34%] +........................................................................ [ 36%] +........................................................................ [ 38%] +........................................................................ [ 40%] +........................................................................ [ 41%] +........................................................................ [ 43%] +........................................................................ [ 45%] +........................................................................ [ 47%] +........................................................................ [ 49%] +........................................................................ [ 51%] +........................................................................ [ 53%] +........................................................................ [ 55%] +........................................................................ [ 57%] +........................................................................ [ 59%] +........................................................................ [ 61%] +........................................................................ [ 62%] +........................................................................ [ 64%] +........................................................................ [ 66%] +........................................................................ [ 68%] +........................................................................ [ 70%] +........................................................................ [ 72%] +........................................................................ [ 74%] +........................................................................ [ 76%] +........................................................................ [ 78%] +........................................................................ [ 80%] +........................................................................ [ 82%] +........................................................................ [ 83%] +........................................................................ [ 85%] +........................................................................ [ 87%] +........................................................................ [ 89%] +........................................................................ [ 91%] +........................................................................ [ 93%] +........................................................................ [ 95%] +........................................................................ [ 97%] +........................................................................ [ 99%] +............................ [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +-------------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 52 0 100% +app/api/chat.py 277 1 99% +app/api/chat_images.py 50 0 100% +app/api/chats.py 110 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 99 0 100% +app/api/docs.py 196 1 99% +app/api/git_sources.py 259 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 204 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 266 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 45 0 100% +app/core/caching.py 124 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/errors.py 5 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/security_headers.py 20 0 100% +app/core/theming.py 38 0 100% +app/core/tokens.py 44 0 100% +app/db.py 22 0 100% +app/main.py 68 0 100% +app/models.py 136 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 454 1 99% +app/rag/archive_upload.py 134 0 100% +app/rag/chunker.py 220 4 98% +app/rag/doc_dates.py 18 0 100% +app/rag/folder_summaries.py 134 0 100% +app/rag/git_sources.py 77 0 100% +app/rag/importer.py 764 4 99% +app/rag/llm.py 246 1 99% +app/rag/md_images.py 63 0 100% +app/rag/overview.py 78 0 100% +app/rag/pdf.py 192 0 100% +app/rag/prompts.py 156 0 100% +app/rag/retriever.py 252 3 99% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 58 0 100% +app/schemas.py 473 0 100% +-------------------------------------------------- +TOTAL 5792 17 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +WARNING: there is a new pyright version available (v1.1.411 -> v1.1.414). +Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest` + +validation OK diff --git a/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__02_persist_write_first.a1.err b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__02_persist_write_first.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__02_persist_write_first.a1.md b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__02_persist_write_first.a1.md new file mode 100644 index 0000000..1497282 --- /dev/null +++ b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__02_persist_write_first.a1.md @@ -0,0 +1,13 @@ +**Task 02 complete — `_persist_pdf_pages` write-first atomic persist + `strip_class` stamp (D2, A4/A7).** + +- `app/rag/importer.py`: `_persist_pdf_pages` rewritten — render → per-file atomic write (`.png.tmp` + `os.replace`, never a direct final-path write) → stale-tail delete LAST (only old-extent files not rewritten; a file about to be overwritten is never deleted); failure path unlinks this call's `.tmp`s and files it **created** (an overwritten copy keeps standing — deleting it would re-open the live-loss hole); `strip_class="fallback"|"pictorial"|None` replaces `eligible`, stamping `page_strip_mode` `primary`/`additive` exactly with `page_count`; the None arm and no-pages guard delete-then-null (helper reads the count); module + function docstrings carry the write-first rationale and the verbatim phase-148 truth table. +- Call site (`_index_pdf_file`): `strip_class = "fallback" if fallback_fired else ("pictorial" if pictorial_described else None)`; fail-soft handler nulls `page_strip_mode` too; comments updated. +- `tests/unit/test_pdf_page_copies.py`: re-cut task-01 pin → additive stamp; fallback→`primary`, ineligible/qualifier-loss/no-pages→NULL pins; new pins — order (event log: all `os.replace`s strictly before the first stale `unlink`, new bytes win), mid-write failure (prior 10-file set intact, zero `.tmp` orphans, count/mode NULL, doc indexed), atomic tmp+replace writer (every write to `.png.tmp`, `dst`/`src` pairing), best-effort stale-tail OSError branch; writer-exception test re-cut for tmp semantics. + +Gates: `uv run pytest --cov=app --cov-report=term-missing` → **3776 passed, app coverage 99%** (importer 99%; remaining misses pre-existing); `uv run ruff check . && uv run pyright` → clean; E2E `test_pdf_pages_in_sources` (4), `test_pictorial_pdf` (3), `test_garbage_pdf` (3) all pass in isolation. + +Docs: no further doc impact — README's phase-163 strip-class section (task 01) already covers the user-visible contract; deletion ordering is internal. + +Notable decision: the partial-write cleanup keeps an **overwritten** final (keeps only newly-created files) — strictly satisfies "a failed call never leaves more files than before, and never removes a live copy" (LOCKED A4). + +Next pending task: `163_pdf_strip_additive_selfheal/03_fast_path_regate.md`. diff --git a/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__02_persist_write_first.a1.md.status b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__02_persist_write_first.a1.md.status new file mode 100644 index 0000000..9766475 --- /dev/null +++ b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__02_persist_write_first.a1.md.status @@ -0,0 +1 @@ +ok diff --git a/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__02_persist_write_first.a1.validate b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__02_persist_write_first.a1.validate new file mode 100644 index 0000000..97bdfb6 --- /dev/null +++ b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__02_persist_write_first.a1.validate @@ -0,0 +1,124 @@ +........................................................................ [ 1%] +........................................................................ [ 3%] +........................................................................ [ 5%] +........................................................................ [ 7%] +........................................................................ [ 9%] +........................................................................ [ 11%] +........................................................................ [ 13%] +........................................................................ [ 15%] +........................................................................ [ 17%] +........................................................................ [ 19%] +........................................................................ [ 20%] +........................................................................ [ 22%] +........................................................................ [ 24%] +........................................................................ [ 26%] +........................................................................ [ 28%] +........................................................................ [ 30%] +........................................................................ [ 32%] +........................................................................ [ 34%] +........................................................................ [ 36%] +........................................................................ [ 38%] +........................................................................ [ 40%] +........................................................................ [ 41%] +........................................................................ [ 43%] +........................................................................ [ 45%] +........................................................................ [ 47%] +........................................................................ [ 49%] +........................................................................ [ 51%] +........................................................................ [ 53%] +........................................................................ [ 55%] +........................................................................ [ 57%] +........................................................................ [ 59%] +........................................................................ [ 61%] +........................................................................ [ 62%] +........................................................................ [ 64%] +........................................................................ [ 66%] +........................................................................ [ 68%] +........................................................................ [ 70%] +........................................................................ [ 72%] +........................................................................ [ 74%] +........................................................................ [ 76%] +........................................................................ [ 78%] +........................................................................ [ 80%] +........................................................................ [ 81%] +........................................................................ [ 83%] +........................................................................ [ 85%] +........................................................................ [ 87%] +........................................................................ [ 89%] +........................................................................ [ 91%] +........................................................................ [ 93%] +........................................................................ [ 95%] +........................................................................ [ 97%] +........................................................................ [ 99%] +................................ [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +-------------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 52 0 100% +app/api/chat.py 277 1 99% +app/api/chat_images.py 50 0 100% +app/api/chats.py 110 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 99 0 100% +app/api/docs.py 196 1 99% +app/api/git_sources.py 259 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 204 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 266 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 45 0 100% +app/core/caching.py 124 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/errors.py 5 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/security_headers.py 20 0 100% +app/core/theming.py 38 0 100% +app/core/tokens.py 44 0 100% +app/db.py 22 0 100% +app/main.py 68 0 100% +app/models.py 136 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 454 1 99% +app/rag/archive_upload.py 134 0 100% +app/rag/chunker.py 220 4 98% +app/rag/doc_dates.py 18 0 100% +app/rag/folder_summaries.py 134 0 100% +app/rag/git_sources.py 77 0 100% +app/rag/importer.py 788 4 99% +app/rag/llm.py 246 1 99% +app/rag/md_images.py 63 0 100% +app/rag/overview.py 78 0 100% +app/rag/pdf.py 192 0 100% +app/rag/prompts.py 156 0 100% +app/rag/retriever.py 252 3 99% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 58 0 100% +app/schemas.py 473 0 100% +-------------------------------------------------- +TOTAL 5816 17 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +WARNING: there is a new pyright version available (v1.1.411 -> v1.1.414). +Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest` + +validation OK diff --git a/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__03_fast_path_regate.a1.err b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__03_fast_path_regate.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__03_fast_path_regate.a1.md b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__03_fast_path_regate.a1.md new file mode 100644 index 0000000..2273c45 --- /dev/null +++ b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__03_fast_path_regate.a1.md @@ -0,0 +1,10 @@ +**Task 03 complete — all green.** + +- **`app/rag/importer.py`**: new `_regate_pdf_strip(session, doc, *, full_path, llm, summary)` — mirrors the changed-path gate verbatim from current bytes + settings (phase-133 `is_usable_pdf_text` → phase-140 `pictorial_pages` + `pdf_pictorial` kill switch), **zero LLM/vision calls**; heals any missing `-p{N}.png` (full re-render), stamps `page_strip_mode` (silent NULL→stamp, `mode old->new` on a set mode), clears a strip that no longer qualifies + deletes its copies, one greppable INFO only when it **acts**, whole body fail-soft (`re-gate failed` / `re-render produced no pages (state kept)` / `page_count drift a->b` warnings). +- `_unchanged_doc` gained `full_path: Path | None = None`; the **guarded** re-gate (`page_count` set + `.pdf`) runs at **both** exits (incl. the D1 manual-date return) and commits what it wrote; all 4 call sites (fast path, text/PDF/image content-digest hits) pass their `full_path`. Docstrings: the phase-147 "the copies persist" assumption is replaced (module doc + `_unchanged_doc` step 7 + `_index_pdf_file`). +- De-duplicated task 02's writer: extracted `_write_page_copies` (tmp + `os.replace`, orphan cleanup) and `_delete_stale_page_tail`; `_persist_pdf_pages` and the heal now share them (behavior unchanged). +- **Tests**: 11 new pins in `tests/unit/test_importer_unchanged_fast_path.py` — heal (pictorial, live NULL-mode), fallback heal (`primary`), healthy no-op (zero writes, zero re-gate log), mode re-stamp, repair arm, kill-switch clear, render-raise fail-soft, empty-render keeps row, count drift, content-digest-hit path, non-strip zero-parse guard (spy). +- `uv run pytest tests/unit/test_importer_unchanged_fast_path.py tests/unit/test_pdf_page_copies.py --no-cov` → exit 0 (24 + 15 tests); `uv run pytest --cov=app` → **3787 passed**, `app/rag/importer.py` **99%** (only pre-existing misses: 883, 3506-3507, 3793), TOTAL 99%; `uv run ruff check .` → clean; `uv run pyright` → **0 errors**; PDF E2E files pass in isolation (`test_pdf_pages_in_sources`, `test_pictorial_pdf`, `test_garbage_pdf`, `test_sync_changed_only`, exit 0). +- **Docs**: README — new "The strip repairs itself (phase 163)" viewer bullet, the "Updating the documents" fast-path qualifier, and the `BOR_PDF_PAGE_DIR` env-row note (dedup avoided). **AGENTS.md: no change** (per the phase overview its viewer bullet lands in the phase's final pass). +- **Notable**: (1) running all of `tests/e2e` in one process fails on a **pre-existing** design artifact — `tests/e2e/test_extensionless_import.py:70` mutates `os.environ["BOR_IMPORT_EXTENSIONS"]` at import, so other suites' 13-doc seed drops to 5; unaffected by this task, and the project's gate is per-file isolation (flagging for the owner/final pass). (2) Restored `.agents/screenshots/doc_modal_scrollbar_1280.png` (a tracked artifact my E2E runs overwrote) from HEAD so the phase commit stays clean. +- **Next pending task**: `163_pdf_strip_additive_selfheal/04_viewer_additive_toggle.md`. diff --git a/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__03_fast_path_regate.a1.md.status b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__03_fast_path_regate.a1.md.status new file mode 100644 index 0000000..9766475 --- /dev/null +++ b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__03_fast_path_regate.a1.md.status @@ -0,0 +1 @@ +ok diff --git a/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__03_fast_path_regate.a1.validate b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__03_fast_path_regate.a1.validate new file mode 100644 index 0000000..9940158 --- /dev/null +++ b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__03_fast_path_regate.a1.validate @@ -0,0 +1,124 @@ +........................................................................ [ 1%] +........................................................................ [ 3%] +........................................................................ [ 5%] +........................................................................ [ 7%] +........................................................................ [ 9%] +........................................................................ [ 11%] +........................................................................ [ 13%] +........................................................................ [ 15%] +........................................................................ [ 17%] +........................................................................ [ 19%] +........................................................................ [ 20%] +........................................................................ [ 22%] +........................................................................ [ 24%] +........................................................................ [ 26%] +........................................................................ [ 28%] +........................................................................ [ 30%] +........................................................................ [ 32%] +........................................................................ [ 34%] +........................................................................ [ 36%] +........................................................................ [ 38%] +........................................................................ [ 39%] +........................................................................ [ 41%] +........................................................................ [ 43%] +........................................................................ [ 45%] +........................................................................ [ 47%] +........................................................................ [ 49%] +........................................................................ [ 51%] +........................................................................ [ 53%] +........................................................................ [ 55%] +........................................................................ [ 57%] +........................................................................ [ 58%] +........................................................................ [ 60%] +........................................................................ [ 62%] +........................................................................ [ 64%] +........................................................................ [ 66%] +........................................................................ [ 68%] +........................................................................ [ 70%] +........................................................................ [ 72%] +........................................................................ [ 74%] +........................................................................ [ 76%] +........................................................................ [ 77%] +........................................................................ [ 79%] +........................................................................ [ 81%] +........................................................................ [ 83%] +........................................................................ [ 85%] +........................................................................ [ 87%] +........................................................................ [ 89%] +........................................................................ [ 91%] +........................................................................ [ 93%] +........................................................................ [ 95%] +........................................................................ [ 96%] +........................................................................ [ 98%] +........................................... [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +-------------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 52 0 100% +app/api/chat.py 277 1 99% +app/api/chat_images.py 50 0 100% +app/api/chats.py 110 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 99 0 100% +app/api/docs.py 196 1 99% +app/api/git_sources.py 259 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 204 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 266 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 45 0 100% +app/core/caching.py 124 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/errors.py 5 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/security_headers.py 20 0 100% +app/core/theming.py 38 0 100% +app/core/tokens.py 44 0 100% +app/db.py 22 0 100% +app/main.py 68 0 100% +app/models.py 136 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 454 1 99% +app/rag/archive_upload.py 134 0 100% +app/rag/chunker.py 220 4 98% +app/rag/doc_dates.py 18 0 100% +app/rag/folder_summaries.py 134 0 100% +app/rag/git_sources.py 77 0 100% +app/rag/importer.py 841 4 99% +app/rag/llm.py 246 1 99% +app/rag/md_images.py 63 0 100% +app/rag/overview.py 78 0 100% +app/rag/pdf.py 192 0 100% +app/rag/prompts.py 156 0 100% +app/rag/retriever.py 252 3 99% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 58 0 100% +app/schemas.py 473 0 100% +-------------------------------------------------- +TOTAL 5869 17 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +WARNING: there is a new pyright version available (v1.1.411 -> v1.1.414). +Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest` + +validation OK diff --git a/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__04_viewer_additive_toggle.a1.err b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__04_viewer_additive_toggle.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__04_viewer_additive_toggle.a1.md b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__04_viewer_additive_toggle.a1.md new file mode 100644 index 0000000..304c2ac --- /dev/null +++ b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__04_viewer_additive_toggle.a1.md @@ -0,0 +1,18 @@ +**Task 04 complete — report** + +**Implemented** +- `frontend/assets/document.js`: `renderDocument` now branches on `doc.page_strip_mode` — absent/`"primary"` keeps the phase-148 strip-only branch verbatim (byte-identical); `"additive"` renders the text doc (Summary panel + content) inside a `.doc-views` wrapper with a `Text`/`Pages` toggle (`role=group`, `aria-pressed`, Text default per LOCKED A6); strip mounts once, hidden via CSS `data-view` swap (lazy imgs never fetch while hidden, no refetch on flip-flop); never-build-HTML rule kept (module still has exactly one `innerHTML`); module docstring gained the phase-163 paragraph and re-scoped the phase-148 "INSTEAD of" sentence to the primary class. +- `frontend/assets/styles.css`: `.doc-view-toggle` ghost-pill pair (house `.auth-link`/`.share-chat-btn` idiom, chat-column cap, ≥44px targets, global 3px focus ring, pressed = brand fill + weight 700 — text + style, never color alone) + `.doc-views[data-view]` show/hide rules. +- New `tests/unit/test_viewer_additive_toggle.py`: 9 pins (source-contract + node behavioral runs of the real core: additive shape, primary/absent byte-identity, text-doc untouched, flip semantics with strip-node persistence, lazy imgs, XSS, CSS contract). +- Re-cut stale/newly-affected pins: `test_summary_edit_ui.py` (slot mount), `test_wide_column_css.py` (7th reading-column selector), E2E additive-view contracts in `test_pdf_pages_in_sources.py` and `test_pictorial_pdf.py` (text-first default, Pages flip reveals loaded strip, description rides the shown text). + +**Gates** +- `uv run pytest` → **3796 passed**, 0 failed; coverage `app/` **99%** (>90%). +- `uv run ruff check .` → clean; `uv run pyright` → 0 errors. +- E2E in isolation: `test_pdf_pages_in_sources.py` 4✓, `test_pictorial_pdf.py` 3✓, `test_garbage_pdf.py` 3✓, plus `test_pdf_support.py` 6✓, `test_document_viewer.py` 8✓, `test_summary_in_viewer.py` 4✓. + +**Docs**: README — phase-148 viewer bullet scope-fixed to "primary class only" (the phase-163 bullets from tasks 01–03 already describe the toggle); AGENTS.md — no doc impact (viewer bullet note is reserved for the phase final pass). + +**Decisions**: additive E2E witnesses flipped from "descriptions not displayed" to "descriptions ride the shown text" (intended consequence of LOCKED A1/A6); toggle lives in a `.doc-views` wrapper only on additive docs — all other DOM untouched. + +**Next pending task**: `163_pdf_strip_additive_selfheal/05_e2e_selfheal_pins.md`. diff --git a/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__04_viewer_additive_toggle.a1.md.status b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__04_viewer_additive_toggle.a1.md.status new file mode 100644 index 0000000..9766475 --- /dev/null +++ b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__04_viewer_additive_toggle.a1.md.status @@ -0,0 +1 @@ +ok diff --git a/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__04_viewer_additive_toggle.a1.validate b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__04_viewer_additive_toggle.a1.validate new file mode 100644 index 0000000..481cc35 --- /dev/null +++ b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__04_viewer_additive_toggle.a1.validate @@ -0,0 +1,124 @@ +........................................................................ [ 1%] +........................................................................ [ 3%] +........................................................................ [ 5%] +........................................................................ [ 7%] +........................................................................ [ 9%] +........................................................................ [ 11%] +........................................................................ [ 13%] +........................................................................ [ 15%] +........................................................................ [ 17%] +........................................................................ [ 18%] +........................................................................ [ 20%] +........................................................................ [ 22%] +........................................................................ [ 24%] +........................................................................ [ 26%] +........................................................................ [ 28%] +........................................................................ [ 30%] +........................................................................ [ 32%] +........................................................................ [ 34%] +........................................................................ [ 36%] +........................................................................ [ 37%] +........................................................................ [ 39%] +........................................................................ [ 41%] +........................................................................ [ 43%] +........................................................................ [ 45%] +........................................................................ [ 47%] +........................................................................ [ 49%] +........................................................................ [ 51%] +........................................................................ [ 53%] +........................................................................ [ 55%] +........................................................................ [ 56%] +........................................................................ [ 58%] +........................................................................ [ 60%] +........................................................................ [ 62%] +........................................................................ [ 64%] +........................................................................ [ 66%] +........................................................................ [ 68%] +........................................................................ [ 70%] +........................................................................ [ 72%] +........................................................................ [ 73%] +........................................................................ [ 75%] +........................................................................ [ 77%] +........................................................................ [ 79%] +........................................................................ [ 81%] +........................................................................ [ 83%] +........................................................................ [ 85%] +........................................................................ [ 87%] +........................................................................ [ 89%] +........................................................................ [ 91%] +........................................................................ [ 92%] +........................................................................ [ 94%] +........................................................................ [ 96%] +........................................................................ [ 98%] +.................................................... [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +-------------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 52 0 100% +app/api/chat.py 277 1 99% +app/api/chat_images.py 50 0 100% +app/api/chats.py 110 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 99 0 100% +app/api/docs.py 196 1 99% +app/api/git_sources.py 259 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 204 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 266 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 45 0 100% +app/core/caching.py 124 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/errors.py 5 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/security_headers.py 20 0 100% +app/core/theming.py 38 0 100% +app/core/tokens.py 44 0 100% +app/db.py 22 0 100% +app/main.py 68 0 100% +app/models.py 136 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 454 1 99% +app/rag/archive_upload.py 134 0 100% +app/rag/chunker.py 220 4 98% +app/rag/doc_dates.py 18 0 100% +app/rag/folder_summaries.py 134 0 100% +app/rag/git_sources.py 77 0 100% +app/rag/importer.py 841 4 99% +app/rag/llm.py 246 1 99% +app/rag/md_images.py 63 0 100% +app/rag/overview.py 78 0 100% +app/rag/pdf.py 192 0 100% +app/rag/prompts.py 156 0 100% +app/rag/retriever.py 252 3 99% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 58 0 100% +app/schemas.py 473 0 100% +-------------------------------------------------- +TOTAL 5869 17 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +WARNING: there is a new pyright version available (v1.1.411 -> v1.1.414). +Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest` + +validation OK diff --git a/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__05_e2e_selfheal_pins.a1.err b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__05_e2e_selfheal_pins.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__05_e2e_selfheal_pins.a1.md b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__05_e2e_selfheal_pins.a1.md new file mode 100644 index 0000000..98adf62 --- /dev/null +++ b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__05_e2e_selfheal_pins.a1.md @@ -0,0 +1,16 @@ +**Task 05 complete — report** + +**Implemented** +- New `tests/e2e/test_pdf_strip_additive_selfheal.py` (5 tests, phase-163 A16 suite): fixture `strip_fixture` = `manual.pdf` (3 dense pages + 1 sparse map page → additive), `scanned.pdf` (XML-garbage layer → primary), `plain.pdf` (no strip); asserts 1 additive view (payload stamp, Text default, Pages flip loads all 4 imgs, flip-back keeps the mounted strip node — dataset-probed, zero refetch); 2 primary/plain DOM byte-identity incl. the key-omission payload pins; 3 heal (wipe copies → route 404 → one unchanged-byte sync → files restored, route 200, viewer complete); 4 repair (direct-DB stale `page_count`/mode + fake copies → cleared + deleted); 5 zero-LLM (mock `/v1/e2e/captured`: index burned both vision passes, heal/repair captured zero content calls — the sync prelude's fixed `ping` health probe is the one documented exemption). +- Docs pass: README verified in full (viewer + importer + env-table claims cross-checked against code; defaults script-audited vs `Settings` — no drift); AGENTS.md rule 5 gained the phase-163 viewer-class note. + +**Gates** (all run this session, DB up) +- `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 / test_document_viewer / test_document_viewer_context / test_image_documents` → all exit 0, unmodified. +- `uv run pytest --cov=app --cov-report=term-missing` → **3796 passed**, coverage **99%** (>90%); `uv run ruff check .` clean; `uv run pyright` 0 errors; alembic **0027 downgrade→upgrade round-trip clean**. + +**Docs**: README — no further edits needed (phase bullets from tasks 01–03 verified accurate); AGENTS.md — viewer-class note added. + +**Decisions**: heal/repair zero-call pins exempt only `check_models`' `ping` (every sync pays it, predates the phase); anything else fails the pin. Pre-existing `.agents/screenshots/doc_modal_scrollbar_1280.png` modification (earlier E2E run) left for the pipeline commit. + +**Next pending task**: `164_chat_error_honesty/01_caps_long_answer_chain.md`. diff --git a/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__05_e2e_selfheal_pins.a1.md.status b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__05_e2e_selfheal_pins.a1.md.status new file mode 100644 index 0000000..9766475 --- /dev/null +++ b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__05_e2e_selfheal_pins.a1.md.status @@ -0,0 +1 @@ +ok diff --git a/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__05_e2e_selfheal_pins.a1.validate b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__05_e2e_selfheal_pins.a1.validate new file mode 100644 index 0000000..481cc35 --- /dev/null +++ b/.agents/reports/163_pdf_strip_additive_selfheal/163_pdf_strip_additive_selfheal__05_e2e_selfheal_pins.a1.validate @@ -0,0 +1,124 @@ +........................................................................ [ 1%] +........................................................................ [ 3%] +........................................................................ [ 5%] +........................................................................ [ 7%] +........................................................................ [ 9%] +........................................................................ [ 11%] +........................................................................ [ 13%] +........................................................................ [ 15%] +........................................................................ [ 17%] +........................................................................ [ 18%] +........................................................................ [ 20%] +........................................................................ [ 22%] +........................................................................ [ 24%] +........................................................................ [ 26%] +........................................................................ [ 28%] +........................................................................ [ 30%] +........................................................................ [ 32%] +........................................................................ [ 34%] +........................................................................ [ 36%] +........................................................................ [ 37%] +........................................................................ [ 39%] +........................................................................ [ 41%] +........................................................................ [ 43%] +........................................................................ [ 45%] +........................................................................ [ 47%] +........................................................................ [ 49%] +........................................................................ [ 51%] +........................................................................ [ 53%] +........................................................................ [ 55%] +........................................................................ [ 56%] +........................................................................ [ 58%] +........................................................................ [ 60%] +........................................................................ [ 62%] +........................................................................ [ 64%] +........................................................................ [ 66%] +........................................................................ [ 68%] +........................................................................ [ 70%] +........................................................................ [ 72%] +........................................................................ [ 73%] +........................................................................ [ 75%] +........................................................................ [ 77%] +........................................................................ [ 79%] +........................................................................ [ 81%] +........................................................................ [ 83%] +........................................................................ [ 85%] +........................................................................ [ 87%] +........................................................................ [ 89%] +........................................................................ [ 91%] +........................................................................ [ 92%] +........................................................................ [ 94%] +........................................................................ [ 96%] +........................................................................ [ 98%] +.................................................... [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +-------------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 52 0 100% +app/api/chat.py 277 1 99% +app/api/chat_images.py 50 0 100% +app/api/chats.py 110 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 99 0 100% +app/api/docs.py 196 1 99% +app/api/git_sources.py 259 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 204 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 266 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 45 0 100% +app/core/caching.py 124 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/errors.py 5 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/security_headers.py 20 0 100% +app/core/theming.py 38 0 100% +app/core/tokens.py 44 0 100% +app/db.py 22 0 100% +app/main.py 68 0 100% +app/models.py 136 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 454 1 99% +app/rag/archive_upload.py 134 0 100% +app/rag/chunker.py 220 4 98% +app/rag/doc_dates.py 18 0 100% +app/rag/folder_summaries.py 134 0 100% +app/rag/git_sources.py 77 0 100% +app/rag/importer.py 841 4 99% +app/rag/llm.py 246 1 99% +app/rag/md_images.py 63 0 100% +app/rag/overview.py 78 0 100% +app/rag/pdf.py 192 0 100% +app/rag/prompts.py 156 0 100% +app/rag/retriever.py 252 3 99% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 58 0 100% +app/schemas.py 473 0 100% +-------------------------------------------------- +TOTAL 5869 17 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +WARNING: there is a new pyright version available (v1.1.411 -> v1.1.414). +Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest` + +validation OK diff --git a/.agents/screenshots/doc_modal_scrollbar_1280.png b/.agents/screenshots/doc_modal_scrollbar_1280.png index a719bc1..d438404 100644 Binary files a/.agents/screenshots/doc_modal_scrollbar_1280.png and b/.agents/screenshots/doc_modal_scrollbar_1280.png differ diff --git a/AGENTS.md b/AGENTS.md index 14e2f27..c46e2fb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -21,7 +21,11 @@ panels — `max-height: min(70vh, 64rem)` — with a sticky header row; a clipped action label shows an ellipsis hint (`.cell-clip`) with the full text kept reachable via the control's aria-label - (phase 156)) and + (phase 156); the document viewer's page strip is CLASS-scoped — + the primary class shows the strip in place of the text, the + additive class never hides the text: Text/Pages toggle group + (`aria-pressed`, Text default) with the strip behind it + (phase 163)) and meets WCAG 2.1 AA basics (semantic landmarks, labels, contrast ≥4.5:1, focus-visible, aria-live for the stream). diff --git a/README.md b/README.md index a900e0a..6564ccb 100644 --- a/README.md +++ b/README.md @@ -239,9 +239,11 @@ idempotent and delta-based: each file's RAW BYTES are hashed (sha256, streamed) and checked against the last index's stamp *before* any extraction or vision work — an unchanged file costs one hash + one row lookup (no re-extract, no vision, no re-embed), while a changed file -re-indexes fully. A refresh after a normal editing session takes -seconds — pictorial PDFs included (their expensive first-index vision -burns only once per file version): +re-indexes fully. The one exception is a PDF row that still claims a +page strip: it is re-gated from its current bytes (no vision, no LLM — +see "The strip repairs itself" below). A refresh after a normal editing +session takes seconds — pictorial PDFs included (their expensive +first-index vision burns only once per file version): ```bash # After editing/adding/removing notes: @@ -844,11 +846,42 @@ uploaded archives): shows the PAGES: a vertical strip of lazy page images with "Page N" captions, in place of the text content and the Summary panel (the per-page descriptions stay in the DB for Q&A — the - embeddings and summary are untouched, just not displayed). A lost + embeddings and summary are untouched, just not displayed). Since + phase 163 this strip-only shape is the PRIMARY class only — the + pictorial class keeps its text behind a Text/Pages toggle, per the + next bullet. A lost copy shows a "Page N unavailable" note for that page only. Text- heavy PDFs (usable layer, zero pictorial candidates — and the whole sparse class while `BOR_PDF_PICTORIAL=0`) keep the text view, byte-identical to pre-phase. +- **Which kind of strip it is (phase 163).** The two qualifying classes + are not the same problem: the screenshot class has no usable text to + show, while a book with one sparse page (an illustration map page in a + 36-page text) does. The document row therefore records the strip's + CLASS beside the page count (`documents.page_strip_mode` — + `primary` = the screenshots ARE the content, so the strip is the view; + `additive` = a usable text layer, so the text + Summary view stays and + the pages sit behind a Text/Pages toggle), and the document-content + payload carries it as `page_strip_mode`. The key is ABSENT — never + `null` — when there is no strip, which is also how a row written before + this phase serializes; the viewer treats an absent class as `primary`, + so every existing document renders exactly as before until the next + sync stamps it (the sync's self-heal pass, phase 163). +- **The strip repairs itself (phase 163).** An unchanged PDF is not + assumed to still have its page images on disk. On every sync, a row + that claims a strip is re-gated deterministically from the current + file bytes and the current settings — the same gate the changed path + uses (`BOR_PDF_PICTORIAL` and `BOR_PDF_PICTORIAL_MIN_CHARS` + included), with **zero vision or LLM calls** (rendering is CPU-only): + the class is re-stamped, any `-p{N}.png` missing from + `BOR_PDF_PAGE_DIR` is re-rendered (write-before-delete, atomically per + file), and a strip that no longer qualifies — the kill switch turned + off, the sparse-page bar lowered past it — is cleared along with its + copies. So a wiped pages directory, or a copy set lost to an + interrupted sync, repairs itself on the next sync instead of leaving + "Page N unavailable" forever. It logs one `PDF strip re-gate …` line + only when it actually acts, and a failure there never fails the sync. + PDFs without a strip pay nothing extra (no extra read at all). --- @@ -975,7 +1008,7 @@ to `.env` and adjust. | `BOR_PDF_PICTORIAL` | `1` | Master switch for the pictorial PDF pass (phase 140, on by default): a usable-text PDF's sparse pages (stripped text under `BOR_PDF_PICTORIAL_MIN_CHARS`) are rendered + vision-described into the indexed content (`[Page N illustration: …]`); `0` ⇒ content byte-identical to pre-phase-140 (no render, no vision call) | | `BOR_PDF_PICTORIAL_MIN_CHARS` | `100` | Sparseness bar for the pictorial pass (phase 140): a page whose STRIPPED text is STRICTLY shorter than this many chars is a candidate (a page with exactly this many is not); `0` = every page (A21); a negative value fails startup loudly | | `BOR_PDF_IMAGE_SKIP_BLANK` | `1` | Skip blank/decorative images embedded in PDFs (phase 151, on by default): near-white, nearly-uniform images (the table-shading "blank white" underlays) are dropped from the embedded-image pass — no row, no copy, no vision call; `0` ⇒ every extracted image is indexed again (pre-phase behavior; dedupe + soft-mask compositing unaffected) | -| `BOR_PDF_PAGE_DIR` | `~/bor-sources/pages` | Persistent home for the rendered PDF page PNGs (phase 148): a page-qualifying PDF (the phase-133 screenshot class or the phase-140 pictorial class) gets every page rendered at index time as `-p{N}.png` here, so the document viewer shows the actual pages (the descriptions stay embedded for Q&A, just not displayed); text-heavy PDFs keep the text view | +| `BOR_PDF_PAGE_DIR` | `~/bor-sources/pages` | Persistent home for the rendered PDF page PNGs (phase 148): a page-qualifying PDF (the phase-133 screenshot class or the phase-140 pictorial class) gets every page rendered at index time as `-p{N}.png` here, so the document viewer shows the actual pages (the descriptions stay embedded for Q&A, just not displayed); text-heavy PDFs keep the text view. Clearing the directory is self-healing — the next sync re-gates any strip that still claims pages and re-renders what is missing, with zero vision calls (phase 163) | | `BOR_GIT_SOURCES` | — (empty) | CSV of git repo URLs — **fallback while the admin Git sources page's list is empty** | | `BOR_SOURCES_DIR` | `~/bor-sources` | Where git repos are cloned/pulled | | `BOR_UPLOAD_DIR` | `~/bor-sources/uploads` | Where uploaded source archives are unpacked | diff --git a/alembic/versions/0027_documents_page_strip_mode.py b/alembic/versions/0027_documents_page_strip_mode.py new file mode 100644 index 0000000..6791d06 --- /dev/null +++ b/alembic/versions/0027_documents_page_strip_mode.py @@ -0,0 +1,68 @@ +"""documents.page_strip_mode: the strip CLASS behind page_count (phase 163, task 01) + +Revision ID: 0027 +Revises: 0026 +Create Date: 2026-10-08 + +Phase 163 (the PDF page strip is ADDITIVE for a text-bearing book — +task 01, storage only): + +* ``documents.page_strip_mode`` — VARCHAR(16) NULLABLE, NO server + default, NO index: the CLASS of the page strip a non-NULL + ``page_count`` describes, so the viewer knows whether the strip + REPLACES the text or sits one toggle behind it. + + ``"primary"`` — the phase-133 FALLBACK class (an unusable text layer: + the screenshots ARE the content), i.e. exactly the pre-phase view: the + strip is the document's view and no text is shown. ``"additive"`` — + the phase-140 PICTORIAL class (a usable text layer with ≥1 sparse page + described): the document keeps its text view and its Summary panel, + and the strip is one Text/Pages toggle away. + + NULL means NO strip (every non-qualifying document — text documents, + images, text-heavy PDFs — and every qualifying PDF whose render/write + failed). A row written before this phase keeps NULL while its + ``page_count`` stands; the wire omits the key in that case and the + viewer treats an absent mode as ``"primary"`` (today's DOM), so + existing rows render exactly as before until the fast-path re-gate + (task 03) stamps them. Stamped and cleared EXACTLY with + ``page_count`` — the importer's persist (task 02) and the unchanged- + path re-gate (task 03) are the only writers. + + A display fact, never a query surface, like the 0026 column it + accompanies: the content endpoint reads it from the ROW after the + ``(source, path)`` lookup, so it carries no index of its own (the + ``file_hash`` convention, 0025). + +One additive, fully reversible migration (A13); no other schema change. +The ordering of the copies on disk (task 02) and the re-gate itself +(task 03) are importer code — this revision only carries the column. +""" +from __future__ import annotations + +import sqlalchemy as sa + +from alembic import op + +revision = "0027" +down_revision = "0026" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + # No index (a display fact, never a query surface — the content + # endpoint reads it from the row) and no server default (NULL is the + # meaningful state for every non-qualifying document, every fail-soft + # miss, and every pre-phase row). + op.add_column( + "documents", sa.Column("page_strip_mode", sa.String(length=16), nullable=True) + ) + + +def downgrade() -> None: + # The page_strip_mode column is the only 0027 artefact — dropping it + # leaves 0026's schema byte-identical (A13, fully reversible). The + # class is re-derivable from the source file on the next sync (the + # deterministic re-gate of task 03), so no data is lost by the drop. + op.drop_column("documents", "page_strip_mode") diff --git a/app/api/docs.py b/app/api/docs.py index c75848f..ad3da4f 100644 --- a/app/api/docs.py +++ b/app/api/docs.py @@ -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 + ), ) diff --git a/app/models.py b/app/models.py index 4b5b455..0d099d1 100644 --- a/app/models.py +++ b/app/models.py @@ -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" diff --git a/app/rag/importer.py b/app/rag/importer.py index ce39042..07e88f6 100644 --- a/app/rag/importer.py +++ b/app/rag/importer.py @@ -190,15 +190,37 @@ dpi, every page) and persisted to ``settings.pdf_page_dir`` ``-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 (``.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 ``-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//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 + ``-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 ->``). 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 - ``-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. + ``-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 - ``-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 ``-p{N}.png`` + (N 1-based) through an ATOMIC per-file write — the shared writer + :func:`_write_page_copies` puts the bytes on ``.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/-p{N}.png`` (N 1-based) and + return the set of page numbers written. + + * ATOMIC per file — the bytes land on ``.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 diff --git a/app/schemas.py b/app/schemas.py index 0331f63..e98395e 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -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 diff --git a/frontend/assets/document.js b/frontend/assets/document.js index efd6e89..e5d5663 100644 --- a/frontend/assets/document.js +++ b/frontend/assets/document.js @@ -144,8 +144,9 @@ * wire for every other document, so they render the EXACT pre-phase * DOM on BOTH surfaces — the load-bearing byte-identity property), * #doc-content renders a .doc-pages strip INSTEAD of the text - * content AND the .doc-summary panel: one
- * per URL — a lazy (alt = "Page N of ", the WCAG alt + * content AND the .doc-summary panel FOR THE PRIMARY CLASS ONLY + * (phase 163 — the absent/`primary` mode; see the next paragraph): + * one <figure class="doc-page"> per URL — a lazy <img> (alt = "Page N of <title>", the WCAG alt * contract — the title as the fallback context) + a "Page N" * <figcaption> — while the meta badge row renders exactly as today. * The owner's contract: the per-page descriptions stay in the DB for @@ -160,6 +161,26 @@ * per-surface copy); createElement + properties/textContent only (the * never-build-HTML rule — no innerHTML). * + * Phase 163 (task 04, LOCKED A1/A2/A6): the strip's CLASS decides the + * view. page_strip_mode === "additive" (the pictorial class — a USABLE + * text layer, so the pages are a convenience, not the content) keeps + * the TEXT doc EXACTLY as a non-qualifying doc renders it (Summary + * panel + content — the pre-148 branch reused), and the strip sits one + * Text/Pages toggle away: a .doc-view-toggle group (role=group, + * aria-label="Document view", two <button aria-pressed>) above a + * .doc-views wrapper holding the text view + the strip; Text is + * pressed by DEFAULT (A6 — never a sparse-ratio flip), Pages flips the + * wrapper's data-view (CSS shows the strip / hides the text — the + * strip node stays MOUNTED, its lazy imgs never refetch on flip-flop). + * The text is NEVER hidden for this class. The absent/"primary" mode + * (the fallback class — the screenshots ARE the content, and every + * pre-163 stamped row) keeps the strip-only DOM above byte-identically + * (the tolerant read: only the exact "additive" string switches on the + * toggle); falsy page_urls stay untouched. The wrapper exists ONLY on + * additive docs — every other document's DOM is unchanged. Toggle + * labels are fixed strings (no doc-derived text); createElement + + * properties/textContent only. + * * Phase 153 (task 02): the meta row's "Copy path" control — the * read-tool `source/path` form in the clipboard with one click * (ASSUMPTION A1 — the exact `source/path` the read tool, the @@ -361,7 +382,9 @@ export function renderDocument(doc, { titleEl, metaEl, contentEl }) { // Phase 148 (task 03): the page strip — the payload's page_urls // (task 02's endpoint: the exact 1-based route list for a // page-qualifying PDF; ABSENT on the wire for every other - // document) is the ONE branch that replaces the content slot: the + // document) is the ONE branch that replaces the content slot (phase + // 163: the PRIMARY class only — an "additive" doc falls through to + // the text rendering with the strip behind a toggle, D4/A1): the // markdown/.doc-raw content block AND the labeled Summary panel // (including its admin edit wiring — moot without the panel) are // NOT built. The per-page descriptions stay in the DB for Q&A (the @@ -372,16 +395,40 @@ export function renderDocument(doc, { titleEl, metaEl, contentEl }) { // below (byte-identical for every other doc — the load-bearing // property). const pageUrls = Array.isArray(doc.page_urls) ? doc.page_urls : null; - if (pageUrls && pageUrls.length > 0) { + const stripEligible = pageUrls !== null && pageUrls.length > 0; + // Phase 163 (task 04, LOCKED A2): the strip's CLASS picks the view. + // Only the exact "additive" string switches on the toggle — absent + // (pre-163 rows, and the omission rule's non-qualifying docs) or + // "primary" read as primary (the tolerant-reading house rule), so + // the fallback class keeps the strip-only DOM byte-identically. + const additive = stripEligible && doc.page_strip_mode === "additive"; + if (stripEligible && !additive) { contentEl.appendChild(docPagesStrip(doc, pageUrls)); return; } + // Phase 163 (task 04, LOCKED A1/A6): an ADDITIVE doc renders the + // text doc EXACTLY as a non-qualifying one does (Summary panel + + // content below, reused unchanged) INSIDE a .doc-views wrapper — + // the Text/Pages toggle above it flips the wrapper's data-view + // (CSS-driven show/hide, never a re-render); the strip mounts once + // AFTER the text view and stays mounted across flips (lazy imgs, + // display:none while hidden, never fetch). Every non-additive doc + // keeps slot === contentEl — its DOM is byte-identical to pre-phase. + let views = null; + let slot = contentEl; + if (additive) { + views = document.createElement("div"); + views.className = "doc-views"; + views.setAttribute("data-view", "text"); // LOCKED A6: Text is the default view + contentEl.append(docViewToggle(views), views); + slot = views; + } // Phase 122 (task 04): an image document — the persistent bytes // render as the <img> block FIRST; the description (= doc.content) // follows in the normal content slot below (the plain-content path // — the last branch in this function). if (doc.is_image) { - contentEl.appendChild(docImageBlock(doc)); + slot.appendChild(docImageBlock(doc)); } // Phase 36: the summary panel — labeled section ABOVE the original // content, on BOTH surfaces (page + modal) through this one core. @@ -405,7 +452,7 @@ export function renderDocument(doc, { titleEl, metaEl, contentEl }) { body.className = "doc-summary-text"; body.textContent = doc.summary; // text node — XSS contract unchanged section.append(title, body); - contentEl.appendChild(section); + slot.appendChild(section); // Phase 57 (task 02, D4): the section above is the phase-36 shape // for EVERYONE — only an authenticated admin (docAdminReady(), the // module-cached whoami promise) then gains the header row + Edit @@ -419,13 +466,59 @@ export function renderDocument(doc, { titleEl, metaEl, contentEl }) { const wrap = document.createElement("div"); wrap.className = "doc-md"; wrap.innerHTML = renderMarkdown(doc.content); // escape-first: XSS-safe - contentEl.appendChild(wrap); + slot.appendChild(wrap); } else { const pre = document.createElement("pre"); pre.className = "doc-raw"; pre.textContent = doc.content; // text node: never parsed as HTML - contentEl.appendChild(pre); + slot.appendChild(pre); } + // Phase 163 (task 04): the additive strip mounts once, AFTER the + // text view, inside the wrapper — hidden by [data-view="text"] + // (the lazy imgs never fetch until Pages is chosen; the node + // persists across flips — no refetch). + if (views !== null) { + views.appendChild(docPagesStrip(doc, pageUrls)); + } +} + +/* ---------- view toggle (phase 163, task 04 — LOCKED A1/A6) ---------- + * The Text/Pages toggle of an ADDITIVE strip doc: one + * <div class="doc-view-toggle" role="group" aria-label="Document + * view"> with two <button type="button" class="doc-view-btn" + * aria-pressed>. Text is pressed initially (A6 — no sparse-ratio + * default flip). A click mutates ONLY the two aria-pressed states and + * the .doc-views wrapper's data-view attribute — the CSS shows/hides + * the text view and the strip (never a re-render, never a refetch). + * The labels are FIXED strings (no doc-derived text inside the + * toggle); createElement + properties/textContent only (the + * never-build-HTML rule — no innerHTML). */ +function docViewToggle(viewsEl) { + const group = document.createElement("div"); + group.className = "doc-view-toggle"; + group.setAttribute("role", "group"); + group.setAttribute("aria-label", "Document view"); + const textBtn = viewToggleButton("Text"); + const pagesBtn = viewToggleButton("Pages"); + const select = (view) => { + viewsEl.setAttribute("data-view", view); + textBtn.setAttribute("aria-pressed", view === "text" ? "true" : "false"); + pagesBtn.setAttribute("aria-pressed", view === "pages" ? "true" : "false"); + }; + textBtn.setAttribute("aria-pressed", "true"); // LOCKED A6: Text pressed by default + pagesBtn.setAttribute("aria-pressed", "false"); + textBtn.addEventListener("click", () => select("text")); + pagesBtn.addEventListener("click", () => select("pages")); + group.append(textBtn, pagesBtn); + return group; +} + +function viewToggleButton(label) { + const btn = document.createElement("button"); + btn.type = "button"; + btn.className = "doc-view-btn"; + btn.textContent = label; // fixed string — never doc-derived + return btn; } /* ---------- image block (phase 122, task 04) ---------- diff --git a/frontend/assets/styles.css b/frontend/assets/styles.css index 6dc3d60..3a3c78f 100644 --- a/frontend/assets/styles.css +++ b/frontend/assets/styles.css @@ -5311,6 +5311,59 @@ a.answer-image:hover { text-decoration: underline; } /* flat — the caption und padding: 0.35rem 0 0; } +/* Additive strip view (phase 163, task 04): the pictorial-class PDF + — a USABLE text layer — keeps the text doc (Summary panel + + content) with the page strip one Text/Pages toggle away (LOCKED + A1/A6). The toggle is the house ghost-pill idiom (the .auth-link + family): the two pills sit adjacent in the labeled group, ink-soft + on the transparent pill (AA on --bg/--surface), hover the + brand-soft/brand-ink pair; the PRESSED button is the brand fill + + heavier weight — text + style, never color alone (B5) — with --bg + ink on --brand (5.2:1, the .share-chat-btn pair). ≥44px targets + (§7.1); :focus-visible rides the global 3px ring. The view swap is + CSS-driven off the .doc-views wrapper's data-view: the strip is + display:none by default (its lazy imgs never fetch while hidden) + and shows on [data-view="pages"], which hides the text view instead + — the nodes stay mounted across flips (no re-render, no refetch). + Phase-08 tokens only (no new hue — the monochrome invariant). */ +.doc-view-toggle { + display: flex; + gap: 0.4rem; + width: 100%; + max-width: var(--chat-column); + margin-inline: auto; + margin-bottom: 0.75rem; +} +.doc-view-btn { + display: inline-flex; + align-items: center; + justify-content: center; + min-height: 44px; + padding: 0.5rem 0.9rem; + border-radius: 999px; + border: 1px solid var(--line); + background: transparent; + color: var(--ink-soft); + font: inherit; + font-weight: 600; + font-size: 0.95rem; + white-space: nowrap; + cursor: pointer; +} +.doc-view-btn:hover { background: var(--brand-soft); color: var(--brand-ink); } +.doc-view-btn[aria-pressed="true"] { + background: var(--brand); + border-color: var(--brand); + color: var(--bg); /* dark ink on brand: 5.2:1 (AA) */ + font-weight: 700; /* pressed = weight + fill, never color alone (B5) */ +} +.doc-views .doc-pages { display: none; } /* Text default — hidden lazy imgs never fetch */ +.doc-views[data-view="pages"] .doc-summary, +.doc-views[data-view="pages"] .doc-md, +.doc-views[data-view="pages"] .doc-raw, +.doc-views[data-view="pages"] .doc-image { display: none; } +.doc-views[data-view="pages"] .doc-pages { display: grid; } + /* Designed not-found state (no emoji — plain SVG mark, phase 08 rule). */ .doc-not-found { width: 100%; diff --git a/tests/e2e/test_pdf_pages_in_sources.py b/tests/e2e/test_pdf_pages_in_sources.py index c8106db..2a46867 100644 --- a/tests/e2e/test_pdf_pages_in_sources.py +++ b/tests/e2e/test_pdf_pages_in_sources.py @@ -36,33 +36,38 @@ token (the phase-126 convention — the owner's local ``.env`` may lack it, and without it the walk is blind to the fixture). MOCK-ONLY suite: ``E2E_REAL_LLM=1`` is not supported — the described -blocks (and the seed's DB witness that the descriptions ARE indexed -but NOT displayed) are exact only against the mock's deterministic -``PICTORIAL_PAGE_MODE`` canned reply. +blocks (and the seed's DB witness that the descriptions ARE indexed) +are exact only against the mock's deterministic +``PICTORIAL_PAGE_MODE`` canned reply. (Phase 163 re-cut: the viewer +assertions moved from the phase-148 suppression contract to the +ADDITIVE contract — the pictorial class shows its text INCLUDING the +page descriptions, with the strip behind the Text/Pages toggle.) Test → observable mapping (Playwright Mapping Rule): 1. ``test_pictorial_pdf_shows_pages_in_modal`` — the seed (module) asserts the DB + filesystem first: the pictorial row has ``page_count == 3`` and the scratch page dir holds EXACTLY ``<id>-p{1..3}.png`` (real PNG magic, non-empty — the textheavy - doc's copies do not exist); then the Sources-tree doc modal shows - the ``.doc-pages`` strip: exactly three ``.doc-page`` figures, - each a LOADED lazy ``<img>`` (``complete && naturalWidth > 0`` — - the route served a real PNG, not a 404) with the - ``Page N of <title>`` alt, the ``Page N`` caption, the NO - ``.doc-raw`` / NO ``.doc-md`` / NO ``.doc-summary`` suppression - contract (the mock's page descriptions sit in the DB content but - nowhere in the rendered modal), the meta badge row (source / - Created / Indexed) untouched. + doc's copies do not exist); then the Sources-tree doc modal opens + the ADDITIVE view (phase 163 — a usable text layer: TEXT first, + the strip one toggle away): the Text/Pages group with ``Text`` + pressed, the Summary panel + content visible (the mock's page + descriptions RIDE the shown text — LOCKED A1/A6), the + ``.doc-pages`` strip mounted but hidden; pressing ``Pages`` shows + exactly three ``.doc-page`` figures, each a LOADED lazy ``<img>`` + (``complete && naturalWidth > 0`` — the route served a real PNG, + not a 404) with the ``Page N of <title>`` alt and the ``Page N`` + caption while the text view is hidden (mounted for the flip- + back), the meta badge row (source / Created / Indexed) untouched. 2. ``test_textheavy_pdf_unchanged`` — the textheavy modal renders the PRE-PHASE shape: the content in ``pre.doc-raw`` (both pages), the Summary panel (the mock's lite digest), NO ``.doc-pages`` — and the content payload's ``page_urls`` KEY is ABSENT (never ``null`` — the omission pin the viewer's strip gate rides on). -3. ``test_full_page_surface`` — the same strip on the dedicated - ``/document.html`` page (admin-logged-in, the phase-79 gate - satisfied): one ``renderDocument`` core — no per-surface drift — - same figures, same suppression. +3. ``test_full_page_surface`` — the same additive view + strip on the + dedicated ``/document.html`` page (admin-logged-in, the phase-79 + gate satisfied): one ``renderDocument`` core — no per-surface + drift — same toggle, same figures, same hidden-while-Text shape. 4. ``test_page_route_serves_png`` — an ANONYMOUS (no-login) direct request on ``/api/documents/<id>/pages/1``: 200, ``image/png``, the ``Cache-Control: private, max-age=3600`` header, a non-empty @@ -597,15 +602,25 @@ def _wait_img_loaded(img: Locator) -> None: def _assert_page_strip( content: Locator, title: str, doc_id: str, page_count: int = 3 ) -> None: - """The phase-148 strip contract inside a content container (the - ONE ``renderDocument`` core — modal and full page, no - per-surface drift): a ``.doc-pages`` strip with exactly - *page_count* ``.doc-page`` figures; each figure's lazy ``<img>`` - is LOADED (the route served a real PNG) with the ``Page N of - <title>`` alt and the exact 1-based route src, and the - figcaption reads ``Page N``; the text content block AND the - Summary panel are SUPPRESSED (the descriptions stay in the DB, - not displayed).""" + """The strip contract inside a content container (the ONE + ``renderDocument`` core — modal and full page, no per-surface + drift): a ``.doc-pages`` strip with exactly *page_count* + ``.doc-page`` figures; each figure's lazy ``<img>`` is LOADED (the + route served a real PNG) with the ``Page N of <title>`` alt and + the exact 1-based route src, and the figcaption reads ``Page N``. + + Phase 163 (task 04) re-cut: this fixture is the PICTORIAL class — + an ADDITIVE doc (``page_strip_mode == "additive"``) that opens on + its TEXT view, so the strip is revealed by pressing the toggle's + ``Pages`` button first (a primary/absent doc has no toggle — the + strip already IS the content). The text content block and the + Summary panel are HIDDEN while the strip shows (they stay mounted + for the flip back — LOCKED A1); a primary-class doc has no such + nodes at all, which the same ``not_to_be_visible`` assertion + covers.""" + toggle = content.locator(".doc-view-toggle") + if toggle.count() > 0: + toggle.get_by_role("button", name="Pages").click() strip = content.locator(".doc-pages") expect(strip).to_have_count(1) figures = strip.locator(".doc-page") @@ -624,12 +639,12 @@ def _assert_page_strip( ) _wait_img_loaded(img) expect(fig.locator("figcaption.doc-page-num")).to_have_text(f"Page {n}") - # The suppression contract (the owner's TODO L5): no text content - # block, no Summary panel — the meta badge row is asserted by the - # caller (it renders exactly as today). - expect(content.locator("pre.doc-raw")).to_have_count(0) - expect(content.locator(".doc-md")).to_have_count(0) - expect(content.locator(".doc-summary")).to_have_count(0) + # While the strip shows, the text view is not (phase 163: hidden, + # not unmounted — the additive flip-back; a primary doc has no + # such nodes at all). The meta badge row is asserted by the caller. + expect(content.locator("pre.doc-raw")).not_to_be_visible() + expect(content.locator(".doc-md")).not_to_be_visible() + expect(content.locator(".doc-summary")).not_to_be_visible() # --------------------------------------------------------------------------- @@ -640,15 +655,18 @@ def _assert_page_strip( def test_pictorial_pdf_shows_pages_in_modal( page: Page, app_url: str, seeded: dict[str, Any] ) -> None: - """The IKEA-shaped PDF opens in the doc modal as a strip of its - REAL pages (the owner's L5 ask): ``.doc-pages`` with exactly three + """The IKEA-shaped PDF opens in the doc modal as an ADDITIVE doc + (phase 163 — a usable text layer: the pictorial class): the TEXT + view first (Summary panel + content visible, the toggle's ``Text`` + pressed — LOCKED A6, the mock's page descriptions ride the shown + text), with the strip of its REAL pages (the owner's L5 ask) one + ``Pages`` click away: ``.doc-pages`` with exactly three ``.doc-page`` figures — each a loaded lazy ``<img>`` (naturalWidth > 0 proves the full pipeline: sync → render → persist → route → viewer) with the ``Page N of <title>`` alt and the ``Page N`` - caption — and the suppression contract: NO text content - (``.doc-raw``), NO Summary panel (the mock's page descriptions are - in the DB content, nowhere in the rendered modal); the meta badge - row (source / Created / Indexed) renders as today.""" + caption — while the text view is hidden (mounted for the + flip-back); the meta badge row (source / Created / Indexed) + renders as today.""" login(page, app_url) # lands on /sources.html _drill_to_source(page, SOURCE_NAME) expect(page.locator("#docs-tbody tr")).to_have_count(2) @@ -658,6 +676,29 @@ def test_pictorial_pdf_shows_pages_in_modal( ) content = page.locator("#doc-modal-content") + # Phase 163 (task 04, LOCKED A1/A6): the pictorial class is an + # ADDITIVE doc — it opens on its TEXT view: Summary panel + + # content visible, the toggle's Text pressed, the strip mounted + # but hidden (its lazy imgs never fetched while hidden). + toggle = content.locator(".doc-view-toggle") + expect(toggle).to_have_count(1) + expect(toggle.get_by_role("button", name="Text")).to_have_attribute( + "aria-pressed", "true" + ) + expect(toggle.get_by_role("button", name="Pages")).to_have_attribute( + "aria-pressed", "false" + ) + expect(content.locator("pre.doc-raw")).to_be_visible() + expect(content.locator(".doc-summary")).to_be_visible() + expect(content.locator(".doc-pages")).to_be_hidden() + # The text view carries the indexed content — the phase-140 page + # descriptions ride the SHOWN text for an additive doc (the text + # is never hidden: A1). + assert PICTORIAL_PAGE_ANSWER in content.inner_text(), ( + "the additive text view shows the indexed content including " + "the described sparse pages" + ) + # …and the strip is one Pages click away (the helper flips). _assert_page_strip(content, seeded["title"], seeded["pictorial_id"]) # The meta badge row is untouched by the strip (the pre-phase row). @@ -666,13 +707,6 @@ def test_pictorial_pdf_shows_pages_in_modal( expect(meta.locator(".doc-created")).to_contain_text("Created") expect(meta.locator(".doc-indexed")).to_contain_text("Indexed") - # The suppression witness: the mock's canned page descriptions sit - # in the DB (the seed asserted it) but are NOT displayed. - assert PICTORIAL_PAGE_ANSWER not in content.inner_text(), ( - "the described pages' text is in the DB for Q&A — it must not " - "be shown to the user (the owner's contract)" - ) - # --------------------------------------------------------------------------- # 2. The text-heavy PDF: byte-identical to pre-phase @@ -730,10 +764,11 @@ def test_textheavy_pdf_unchanged( def test_full_page_surface(page: Page, app_url: str, seeded: dict[str, Any]) -> None: """The dedicated ``/document.html`` page (admin-logged-in — the - phase-79 gate satisfied) renders the SAME strip through the ONE - shared ``renderDocument`` core: the same three loaded figures with - captions/alt texts, the same suppression (no text content, no - Summary panel), the meta row as today.""" + phase-79 gate satisfied) renders the SAME additive view through + the ONE shared ``renderDocument`` core (no per-surface drift): the + text view first, the strip one ``Pages`` click away with the same + three loaded figures + captions/alt texts and the text view + hidden while it shows, the meta row as today.""" login(page, app_url, next="/") page.goto( f"{app_url}/document.html?source={quote(SOURCE_NAME)}" diff --git a/tests/e2e/test_pdf_strip_additive_selfheal.py b/tests/e2e/test_pdf_strip_additive_selfheal.py new file mode 100644 index 0000000..d46c7aa --- /dev/null +++ b/tests/e2e/test_pdf_strip_additive_selfheal.py @@ -0,0 +1,1084 @@ +"""Phase 163 task 05 E2E (Playwright, mock-only): the additive PDF +strip, the byte-identical primary/plain classes, and the fast-path +self-heal — the ``Letumdei`` failure mode, reproduced and made +impossible. + +Run in isolation (DB must be up: ``podman compose up -d db``): + + uv run pytest tests/e2e/test_pdf_strip_additive_selfheal.py -v --no-cov + +The dedicated suite for ``163_pdf_strip_additive_selfheal`` (A16 — one +Playwright file per phase, run in isolation). Pre-fix evidence +(2026-10-07, brain.reeseapps.com): ``Letumdei_ Heirs to the Imperium.pdf`` +— 36 ``page_urls`` in the payload, ALL 36 page routes 404, 114 389 +chars of genuine extracted text with exactly ONE sparse map page +(the pictorial class) — the viewer showed "Page N unavailable." on +every page with the text hidden behind the strip. Two stacked defects: +the pictorial class rendered the strip INSTEAD of its text (fixed by +the additive view, LOCKED A1/A6), and the copies were deleted before +the replacements were written, outside the DB transaction, so a later +rollback (or an external wipe) left the row over zero files with the +phase-147 fast path never re-rendering (fixed by write-first persist ++ the deterministic re-gate, LOCKED A4/A5). One local-directory source +(``strip_fixture``, the API-registered ``kind=local`` pattern) carries +three synthetic PyMuPDF PDFs — the live shape in miniature: + +* ``manual.pdf`` — the PICTORIAL class (the Letumdei shape): 3 dense + prose pages + ONE sparse map-style page (<100 stripped chars, a + vector-drawn figure) → usable text layer, one gate candidate → + ``page_strip_mode == "additive"``, ``page_count == 4``; +* ``scanned.pdf`` — the FALLBACK class (the phase-133 shape): two + pages whose ONLY text is the owner's XML-garbage line repeated 20× → + unusable layer → ``page_strip_mode == "primary"``, ``page_count == 2``; +* ``plain.pdf`` — clean dense text, ZERO sparse pages → no strip at + all (``page_count`` NULL, both strip keys absent from the payload). + +App env: the conftest pattern with the phase-140 opt-in (``BOR_PDF_ +PICTORIAL`` pinned EXPLICITLY ``1``) + the phase-148 ``BOR_PDF_PAGE_DIR`` +pointed at a suite-private scratch dir (the heal deletes and re-creates +files THERE — the owner's ``~/bor-sources/pages`` default is never +touched) + ``BOR_IMPORT_EXTENSIONS`` carrying the ``pdf`` token (the +phase-126 convention). The page-directory homes are module-scoped so +the heal/repair tests mutate the exact dir the module app reads. + +MOCK-ONLY suite: ``E2E_REAL_LLM=1`` is not supported for the +zero-LLM-call pin (assert 5 reads the mock's ``/v1/e2e/captured`` +request log — the phase-123 house pattern for "ZERO chat calls"). + +Test → observable mapping (Playwright Mapping Rule): +1. ``test_additive_view`` — the ``manual.pdf`` modal: the content + payload stamps the class (``page_strip_mode == "additive"`` beside + the 4 ``page_urls``), the TEXT view opens by default (Summary panel + + content visible — the map page's canned description RIDES the + shown text, LOCKED A1; the toggle's ``Text`` pressed, LOCKED A6) + with the strip mounted but hidden; pressing ``Pages`` shows the + 4-figure strip and EVERY ``<img>`` loads (naturalWidth > 0 — the + route serves; no "unavailable" note anywhere); pressing ``Text`` + back shows the text, hides the strip, and the strip NODE persists + (the same DOM element — probed by a dataset marker — with the same + 4 loaded imgs: no re-render, no refetch). +2. ``test_primary_and_plain_doms`` — the ``scanned.pdf`` modal keeps + the phase-148 strip-only DOM byte-identically (payload + ``page_strip_mode == "primary"``; NO ``.doc-view-toggle``, NO + ``.doc-summary``, NO content block; 2 loaded ``.doc-page`` figures + with the ``Page N`` captions); the ``plain.pdf`` modal is the plain + text doc (raw content + Summary panel, ZERO ``.doc-pages`` / + ``.doc-view-toggle`` nodes) and its payload carries NEITHER + ``page_urls`` NOR ``page_strip_mode`` (the omission rule — the + non-qualifying payload is byte-identical to pre-phase). +3. ``test_heal_restores_deleted_copies`` — the live-loss state + reproduced: delete ALL ``pdf_page_dir/<manual-id>-p*.png`` (the row + still claims 4) → a page route 404s → run a sync over UNCHANGED + bytes (the fast path) → the four copies exist again (real PNGs), + the route serves 200 with PNG bytes, the row keeps ``page_count`` + 4 / mode ``additive``, the payload still carries ``page_urls`` + + ``additive``, and the viewer's Pages view loads every img — the + exact D5 promise (a wipe cannot outlive one sync). +4. ``test_repair_clears_stale_strip`` — the stale-eligible state NO + current index produces: direct-DB seed sets ``plain.pdf``'s row to + ``page_count`` 2 / mode ``additive`` and writes two stale copies on + disk → sync (still unchanged bytes) → both columns NULL (read + through the fixture session), the stale copies GONE, and the modal + shows the plain text doc (the strip is finished). +5. ``test_zero_llm_calls_on_heal_and_repair`` — the mock's request + log: the INITIAL index burned the vision passes it owes (≥1 + ``PICTORIAL_PAGE_MODE`` + ≥1 ``IMAGE_DESCRIPTION_MODE`` request — + the capture works and the premise is real), while the heal sync and + the repair sync each captured ZERO content/vision chat requests — + the re-gate is deterministic (render is CPU, LOCKED A5). The one + exempted request is the sync prelude's fixed ``ping`` model-health + probe (``app.rag.llm.check_models`` — every sync ever run pays it, + unchanged by this phase); anything else in the capture is a + violation. + +House pattern: one module app over the shared E2E Postgres (its own +port, module env — the conftest session app is never started in this +isolated run, so no port clash). The five tests SHARE the module seed +(one source, three documents, one sync) and deliberately MUTATE shared +state (the page dir, then one row) in file order, so the shared +registry + KB is truncated ONCE, after the last test (a +module-granularity autouse — a per-test truncate would wipe the seed +from under its siblings), with a no-running-sync wait so a leaked run +cannot race it. +""" +from __future__ import annotations + +import json +import os +import subprocess +import sys +import time +from collections.abc import Iterator +from datetime import datetime +from pathlib import Path +from typing import Any + +import httpx +import pymupdf as fitz # PyMuPDF — the synthetic-fixture builder (the house recipe) +import pytest +from playwright.sync_api import Locator, Page, expect +from sqlalchemy import select, text + +from app.config import Settings +from app.db import SessionLocal +from app.models import Document +from app.rag.pdf import ( + extract_pdf_page_texts, + extract_pdf_text, + is_usable_pdf_text, + pictorial_pages, + render_pdf_pages, +) +from e2e.auth_helpers import login +from e2e.conftest import ( + ADMIN_PASSWORD, + MOCK_PORT, + SESSION_SECRET, + USE_REAL_LLM, + _wait_http, +) +from e2e.mock_llm import IMAGE_DESCRIPTION_ANSWER, PICTORIAL_PAGE_ANSWER + +REPO = Path(__file__).resolve().parents[2] + +#: The local directory's NAME — the source name the registry stores for +#: a ``kind='local'`` source (the directory basename, the +#: test_local_directory_sources convention). The task's fixture name. +SOURCE_NAME = "strip_fixture" +MANUAL_PATH = "manual.pdf" +SCANNED_PATH = "scanned.pdf" +PLAIN_PATH = "plain.pdf" +MANUAL_TITLE = "Campaign Manual" # the metadata title (the Letumdei stand-in) +SCANNED_TITLE = "Scanned Plates" # the metadata title +PLAIN_TITLE = "Plain Notes" # the metadata title + +#: The dense prose paragraph (500+ chars — over the 100-char sparseness +#: bar, so text pages are never gate candidates; the +#: test_pdf_pages_in_sources recipe). +DENSE_PARAGRAPH = ( + "This manual covers the complete campaign, its muster rules, and the " + "supply checks a first-time commander should run before marching. The " + "train holds the two supply wagons, the field forge, the water cart, " + "eight cam locks for the tent pegs, fourteen spare stakes, two spare " + "axle bolts, and the signal-horn case. Work from a flat ridge with dry " + "ground, keep the squads sorted by banner, and read every dispatch " + "through before you begin so you know which standard is which. The " + "column marches roughly ten leagues a day once supplied, weighs under " + "twelve tonnes, and needs no further provision beyond the local levy." +) +assert len(DENSE_PARAGRAPH) > 500, "the dense paragraph must be 500+ chars" + +#: The sparse map page's marker — ALL page 4's text layer carries (21 +#: stripped chars, under the 100-char bar — the ONE gate candidate; the +#: figure is vector-drawn, exactly like the live page-36 map). +MAP_MARKER = "Map of the Imperium" + +#: The phase-133 garbage line (the owner's exact shape) — 100% of the +#: scanned pages' text looks like tags, so ``is_usable_pdf_text`` fails +#: deterministically (the fallback class). +GARBAGE_LINE = "<illustrator:StartupProfile>Web</illustrator:StartupProfile>" +GARBAGE_LINES_PER_PAGE = 20 +N_SCANNED_PAGES = 2 + +#: The stored map-page block (the phase-140 LOCKED A4 shape) — the +#: mock's canned ``PICTORIAL_PAGE_MODE`` reply, deterministic. +MAP_BLOCK = f"[Page 4 illustration: {PICTORIAL_PAGE_ANSWER}]" + +#: The PNG signature (the route/seed byte assertions' magic number). +PNG_MAGIC = b"\x89PNG\r\n\x1a\n" + +#: Phase 79 (task 04, full inventory): the conftest session app owns +#: its port in a combined run — this module app binds its own port +#: instead (a same-port second uvicorn dies on bind and would drive the +#: wrong server). Env-overridable. +APP_PORT = int(os.environ.get("E2E_APP_PORT_STRIPHEAL", "8177")) +APP_URL = f"http://127.0.0.1:{APP_PORT}" +MOCK_URL = f"http://127.0.0.1:{MOCK_PORT}" + +#: The suite's LLM-request ledger (assert 5): every sync stores the +#: mock's chat-completions capture (``/v1/e2e/captured`` — the phase-123 +#: house pattern) taken AROUND its own run under its name, so the +#: heal/repair syncs' ZERO-call pins read from the log the app itself +#: produced, not from a spy that never saw the real request. +LLM_CAPTURES: dict[str, list[dict[str, Any]]] = {} + + +# --------------------------------------------------------------------------- +# Fixture builders (the house PyMuPDF recipe — the three strip classes) +# --------------------------------------------------------------------------- + + +def _draw_map_figure(page: Any) -> None: + """The vector map figure: a rectangle (the province) + a road line + with a two-stroke head — drawn, not embedded, so the text layer + stays the ONLY text and the embedded-image pipeline sees nothing.""" + page.draw_rect(fitz.Rect(100, 120, 300, 220)) + page.draw_line(fitz.Point(320, 170), fitz.Point(470, 170)) + page.draw_line(fitz.Point(470, 170), fitz.Point(450, 158)) + page.draw_line(fitz.Point(470, 170), fitz.Point(450, 182)) + + +def _build_manual(path: Path) -> None: + """The Letumdei shape in MINATURE (the pictorial class): three dense + prose pages + ONE sparse map-style page. Usable text layer with + exactly one gate candidate (page 4) → the ADDITIVE strip class.""" + doc = fitz.open() + for i in range(1, 4): + p = doc.new_page() + rc = p.insert_textbox( + fitz.Rect(72, 72, 523, 772), f"Book {i}. {DENSE_PARAGRAPH}", fontsize=11 + ) + assert rc >= 0, f"the page-{i} paragraph must fit its text box" + p4 = doc.new_page() + p4.insert_text((72, 72), MAP_MARKER) + _draw_map_figure(p4) + doc.set_metadata({"title": MANUAL_TITLE}) + doc.save(path) + doc.close() + + +def _build_scanned(path: Path) -> None: + """The phase-133 fallback class: two pages whose ONLY text is the + XML-garbage line ×20 — the gate fails, so the page SCREENSHOTS are + the content (the PRIMARY strip class).""" + doc = fitz.open() + for page_num in range(1, N_SCANNED_PAGES + 1): + page = doc.new_page() + for i in range(GARBAGE_LINES_PER_PAGE): + page.insert_text((72, 72 + i * 14), f"{GARBAGE_LINE} p{page_num} l{i}") + doc.set_metadata({"title": SCANNED_TITLE}) + doc.save(path) + doc.close() + + +def _build_plain(path: Path) -> None: + """The negative class: two dense text pages, ZERO sparse candidates + — no strip ever (until the repair test seeds a stale one).""" + doc = fitz.open() + for i in (1, 2): + p = doc.new_page() + rc = p.insert_textbox( + fitz.Rect(72, 72, 523, 772), f"Note {i}. {DENSE_PARAGRAPH}", fontsize=11 + ) + assert rc >= 0, f"the page-{i} paragraph must fit its text box" + doc.set_metadata({"title": PLAIN_TITLE}) + doc.save(path) + doc.close() + + +@pytest.fixture(scope="module") +def pdfs_dir(tmp_path_factory: pytest.TempPathFactory) -> Path: + """The local directory source's root: the three fixture PDFs, built + ONCE (module-scoped). The premise pins (deterministic — computed + from the fixtures THEMSELVES, no hardcoded page lists): ``manual`` + is usable-layer with candidates ``[4]`` (4 pages — the seed's + ``page_count`` target, the Letumdei shape), ``scanned`` is + UNUSABLE-layer (the fallback class), ``plain`` is usable-layer with + ZERO candidates (no strip). Every page renders to a real PNG (the + pipeline's byte premise).""" + root = tmp_path_factory.mktemp("bor_p163_strip") / SOURCE_NAME + root.mkdir(parents=True) + min_chars = Settings.model_fields["pdf_pictorial_min_chars"].default + manual = root / MANUAL_PATH + scanned = root / SCANNED_PATH + plain = root / PLAIN_PATH + _build_manual(manual) + _build_scanned(scanned) + _build_plain(plain) + manual_texts = extract_pdf_page_texts(manual) + assert is_usable_pdf_text(extract_pdf_text(manual)), ( + "the manual fixture must PASS the usability gate (the pictorial class)" + ) + assert pictorial_pages(manual_texts, min_chars=min_chars) == [4], ( + "the manual fixture's sparse-page premise broke: candidates=" + f"{pictorial_pages(manual_texts, min_chars=min_chars)}" + ) + assert not is_usable_pdf_text(extract_pdf_text(scanned)), ( + "the scanned fixture's text layer must FAIL the usability gate " + "(the fallback class)" + ) + assert is_usable_pdf_text(extract_pdf_text(plain)) + assert pictorial_pages( + extract_pdf_page_texts(plain), min_chars=min_chars + ) == [], "the plain fixture must carry ZERO sparse pages (the negative class)" + assert len(render_pdf_pages(manual)) == 4 + assert len(render_pdf_pages(scanned)) == N_SCANNED_PAGES + assert len(render_pdf_pages(plain)) == 2 + return root + + +@pytest.fixture(scope="module") +def scratch(tmp_path_factory: pytest.TempPathFactory) -> Path: + """The suite-private app homes (pages dir, upload dir, checkouts, + images) — disposable with the module.""" + return tmp_path_factory.mktemp("bor_p163_scratch") + + +@pytest.fixture(scope="module") +def page_dir(scratch: Path) -> Path: + """The app's ``pdf_page_dir`` (``BOR_PDF_PAGE_DIR``) for this suite + — the heal test DELETES the manual doc's copies here and the re-gate + re-creates them; the owner's ``~/bor-sources/pages`` default is + never touched.""" + return scratch / "pages" + + +# --------------------------------------------------------------------------- +# Module app (the conftest env pattern + the phase-140/148 knobs) +# --------------------------------------------------------------------------- + + +def _app_env(mock_llm: int, scratch: Path, page_dir: Path) -> dict[str, str]: + """The conftest app env (leak guards included) with the phase-140 + opt-in (``BOR_PDF_PICTORIAL`` EXPLICITLY on — the conftest pins it + off in ``os.environ`` for the pre-phase suites; this suite IS the + pictorial class + its kill-switch-mirroring re-gate) and the + phase-148 ``BOR_PDF_PAGE_DIR`` (suite-private — the heal mutates + this dir, and the app under test reads the same one).""" + env = dict(os.environ) + env.pop("DEBUGPY", None) + env["BOR_ENVIRONMENT"] = "e2e" + env["BOR_STATIC_DIR"] = str(REPO / "frontend") + env["BOR_LLM_BASE_URL"] = ( + "https://aipi.reeseapps.com/v1" + if USE_REAL_LLM + else f"http://127.0.0.1:{mock_llm}/v1" + ) + # Mock-calibrated gates (conftest pattern). + env["BOR_RELEVANCE_THRESHOLD"] = "0.30" + env["BOR_LEXICAL_SUPPORT_FLOOR"] = "0.15" + env["BOR_SOURCE_USEFULNESS_FLOOR"] = "0.15" + # Phase 67: instant retry waits + the code-default budget. + env["BOR_LLM_RETRY_DELAY"] = "0" + env["BOR_LLM_RETRIES"] = str(Settings.model_fields["llm_retries"].default) + env.setdefault( + "BOR_DATABASE_URL", + "postgresql+psycopg://reese:reese@localhost:5432/brain_of_reese", + ) + env["BOR_ADMIN_PASSWORD"] = ADMIN_PASSWORD + env["BOR_SESSION_SECRET"] = SESSION_SECRET + # Leak guards (conftest pattern — the owner's local gitignored + # .env cannot leak the scope). + env["BOR_GIT_SOURCES"] = "" + env["BOR_DOCS_REPO"] = "" + env["BOR_SUGGESTIONS"] = json.dumps(Settings.model_fields["suggestions"].default) + env["BOR_INPUT_PLACEHOLDER"] = Settings.model_fields["input_placeholder"].default + env["BOR_FOOTER_TEXT"] = Settings.model_fields["footer_text"].default + # Phase 126: the import scope WITH the ``pdf`` token (the walk is + # blind to the fixtures without it). + env["BOR_IMPORT_EXTENSIONS"] = "md,markdown,txt,pdf" + # Phase 140: the pictorial toggle EXPLICITLY on (the opt-in — + # without it the whole premise is void). + env["BOR_PDF_PICTORIAL"] = "1" + # Phase 148: the rendered-page copies' home (the heal's delete/ + # re-render target). + env["BOR_PDF_PAGE_DIR"] = str(page_dir) + # The embedded-image pipeline is out of scope (the figures are + # vector-drawn — no image docs either way). + env["BOR_IMAGES"] = "false" + env["BOR_UPLOAD_DIR"] = str(scratch / "uploads") + env["BOR_SOURCES_DIR"] = str(scratch / "checkouts") + env["BOR_IMAGE_DIR"] = str(scratch / "images") + return env + + +@pytest.fixture(scope="module") +def app_server( + mock_llm: int, + pdfs_dir: Path, # noqa: ARG001 — the dir's existence is the seed's job + scratch: Path, + page_dir: Path, +) -> Iterator[str]: + """The app under test (all five tests): one module app with the + phase-140/148/163 env (see :func:`_app_env`).""" + proc = subprocess.Popen( + [ + sys.executable, + "-m", + "uvicorn", + "app.main:app", + "--host", + "127.0.0.1", + "--port", + str(APP_PORT), + "--log-level", + "warning", + ], + cwd=REPO, + env=_app_env(mock_llm, scratch, page_dir), + ) + try: + _wait_http(f"{APP_URL}/api/health") + yield APP_URL + finally: + proc.terminate() + try: + proc.wait(timeout=10) + except subprocess.TimeoutExpired: + proc.kill() + + +@pytest.fixture(scope="module") +def app_url(app_server: str) -> str: + return app_server + + +# --------------------------------------------------------------------------- +# DB + sync + LLM-capture helpers (the house patterns) +# --------------------------------------------------------------------------- + + +def _truncate_all() -> None: + """Fresh KB + registry (the E2E isolation pattern): the suites + share one Postgres, and a leftover row would corrupt the counts. + ``sources_meta`` (the KB generation counter) resets with the KB.""" + with SessionLocal() as db: + db.execute( + text( + "TRUNCATE chunks, documents, query_log, steering_notes, " + "kb_overview, git_sources, folder_summaries, sources_meta" + ) + ) + db.commit() # without the commit the TRUNCATE rolls back (the house pattern) + + +def _health_db(app_url: str) -> bool: + try: + return httpx.get(f"{app_url}/api/health", timeout=5).json()["db"] == "up" + except Exception: # noqa: BLE001 — unreachable is the skip case + return False + + +def _wait_sync_settled(base_url: str, timeout_s: float = 60.0) -> None: + """No background sync may leak across tests (the run state lives in + the app's memory): wait for the app's sync status to be non-running + BEFORE the truncate (the phase-96 pattern).""" + with httpx.Client(base_url=base_url, timeout=5.0) as client: + r = client.post("/api/login", json={"password": ADMIN_PASSWORD}) + assert r.status_code == 204, r.text + deadline = time.monotonic() + timeout_s + while time.monotonic() < deadline: + r = client.get("/api/sync/status") + assert r.status_code == 200, r.text + if r.json()["state"] != "running": + return + time.sleep(0.2) + raise AssertionError(f"a sync was still running at a test boundary: {base_url}") + + +@pytest.fixture(scope="module", autouse=True) +def _clean_kb(app_url: str) -> Iterator[None]: + """Module-end KB cleanup (the phase-96 pattern, module granularity): + the five tests SHARE the module seed's one source + three documents + and MUTATE shared state (the page dir, then one row) in file order, + so there is NO per-test truncate; the shared KB is flushed ONCE, + after the last test. The pre-wait keeps a leaked sync from racing + the truncate.""" + yield + _wait_sync_settled(app_url) + _truncate_all() + + +def _register_source(base_url: str, path: Path) -> None: + """Register the local directory through the authenticated API (the + ``test_local_directory_sources`` pattern — a plain directory is an + API/DB-only operation).""" + with httpx.Client(base_url=base_url, timeout=30.0) as client: + r = client.post("/api/login", json={"password": ADMIN_PASSWORD}) + assert r.status_code == 204, r.text + r = client.post("/api/git-sources", json={"kind": "local", "path": str(path)}) + assert r.status_code == 201, r.text + + +def _started_at(body: dict[str, Any]) -> float: + """The status body's ``started_at`` as a wall-clock epoch (the + server stamps it in UTC; same host, same clock).""" + started = body.get("started_at") + if not started: + return 0.0 + try: + return datetime.fromisoformat(str(started)).timestamp() + except ValueError: + return 0.0 + + +def _capture_reset() -> None: + """Clear the mock's chat-completions capture (the phase-123 house + endpoint) so the NEXT sync's LLM requests are exactly what the + capture holds afterwards. The MOCK-ONLY pin is skipped under + ``E2E_REAL_LLM=1`` (no capture to read — assert 5 skips too).""" + if USE_REAL_LLM: + return + r = httpx.post(f"{MOCK_URL}/v1/e2e/captured/reset", timeout=5.0) + assert r.status_code == 200, r.text + + +def _capture_read(name: str) -> list[dict[str, Any]]: + """Store the mock's current capture in the suite ledger under + *name* (see :data:`LLM_CAPTURES`).""" + if USE_REAL_LLM: + LLM_CAPTURES[name] = [] + return [] + r = httpx.get(f"{MOCK_URL}/v1/e2e/captured", timeout=5.0) + assert r.status_code == 200, r.text + captured = r.json() + LLM_CAPTURES[name] = captured + return captured + + +def _run_sync( + base_url: str, capture_name: str | None = None, timeout_s: float = 120.0 +) -> dict[str, Any]: + """Login + ``POST /api/sync`` + poll the status endpoint until THIS + run reaches a terminal state (the ``test_pdf_pages_in_sources`` + pattern — with the stale-terminal freshness gate). When + *capture_name* is given, the mock's request capture is reset before + the POST and stored under that name once the run settles — the + sync's ENTIRE LLM footprint, bracketed (assert 5's lens).""" + _capture_reset() + with httpx.Client(base_url=base_url, timeout=30.0) as client: + r = client.post("/api/login", json={"password": ADMIN_PASSWORD}) + assert r.status_code == 204, r.text + t0 = time.time() + r = client.post("/api/sync") + assert r.status_code == 202, r.text + saw_running = False + deadline = time.monotonic() + timeout_s + body: dict[str, Any] = {} + while time.monotonic() < deadline: + r = client.get("/api/sync/status") + assert r.status_code == 200, r.text + body = r.json() + state = body["state"] + if state == "running": + saw_running = True + elif state in ("success", "failed") and ( + saw_running + or _started_at(body) >= t0 - 1.0 # same-host clock; 1 s tolerance + ): + if capture_name is not None: + _capture_read(capture_name) + return body + time.sleep(0.1) + if capture_name is not None: + _capture_read(capture_name) + raise AssertionError(f"sync did not reach a terminal state: {body}") + + +def _doc_row(path: str) -> Document: + """The fixture session's view of one source row (the DB-lens the + heal/repair asserts read through — ``expire_on_commit`` off means + the values are the row's, freshly queried per call).""" + with SessionLocal() as db: + doc = db.scalar( + select(Document).where( + Document.source == SOURCE_NAME, Document.path == path + ) + ) + assert doc is not None, f"the {path} row must exist" + db.expunge(doc) + return doc + + +def _payload(app_url: str, path: str) -> dict[str, Any]: + """The document-content payload (the surface the viewer renders + from) as a dict — the ``page_urls`` / ``page_strip_mode`` key + presence rules are asserted on THIS JSON, not the DB.""" + with httpx.Client(base_url=app_url, timeout=30.0) as client: + r = client.post("/api/login", json={"password": ADMIN_PASSWORD}) + assert r.status_code == 204, r.text + r = client.get("/api/documents/content", params={"source": SOURCE_NAME, "path": path}) + assert r.status_code == 200, r.text + return r.json() + + +# --------------------------------------------------------------------------- +# The module seed: register the local dir + one sync (all three PDFs), +# then assert the three-class DB + filesystem state +# --------------------------------------------------------------------------- + + +@pytest.fixture(scope="module") +def seeded(app_url: str, pdfs_dir: Path, page_dir: Path) -> Iterator[dict[str, Any]]: + """Register the local directory source and run ONE real sync (the + index pass, its capture stored as ``LLM_CAPTURES["index"]``). + Asserts the post-index three-class contract: ``manual`` is the + pictorial class (``page_count`` 4, mode ``additive``, the canned + map block IN the content — the Letumdei shape), ``scanned`` the + fallback class (``page_count`` 2, mode ``primary``, the canned + screenshot descriptions, the garbage NOT stored), ``plain`` + unqualified (both columns NULL) — and the page dir holds EXACTLY + the two qualifying docs' six copies (real PNGs, no plain copies, no + orphans). Yields the ids + the page dir.""" + if not _health_db(app_url): + pytest.skip("Postgres not reachable — run `podman compose up -d db` first") + + _truncate_all() + _register_source(app_url, pdfs_dir) + body = _run_sync(app_url, capture_name="index") + assert body["state"] == "success", body + detail = body["detail"] + assert detail["files"] == 3, detail + assert detail["added"] == 3, detail + assert detail["pruned"] == 0, detail + assert detail["errors"] == 0, detail + + manual = _doc_row(MANUAL_PATH) + assert manual.title == MANUAL_TITLE + assert manual.page_count == 4, ( + f"the pictorial-class row must carry its page count: {manual.page_count}" + ) + assert manual.page_strip_mode == "additive", ( + "the pictorial class (usable layer + sparse candidate) must stamp " + f"additive: {manual.page_strip_mode!r}" + ) + # The phase-140 descriptions ride the stored content (Q&A parity) — + # the ONE map block is the live Letumdei shape in miniature. + assert "---PAGE-1---" in manual.content and "---PAGE-4---" in manual.content + assert MAP_BLOCK in manual.content, ( + "the map page's canned description must be in the stored content" + ) + assert manual.summary + + scanned = _doc_row(SCANNED_PATH) + assert scanned.page_count == N_SCANNED_PAGES + assert scanned.page_strip_mode == "primary", ( + "the fallback class (unusable layer) must stamp primary: " + f"{scanned.page_strip_mode!r}" + ) + assert scanned.content.startswith("---PAGE-1---") + assert IMAGE_DESCRIPTION_ANSWER in scanned.content, ( + "the fallback's content must be the canned page-screenshot descriptions" + ) + assert GARBAGE_LINE not in scanned.content, ( + "the garbage text layer must NOT be the stored content" + ) + + plain = _doc_row(PLAIN_PATH) + assert plain.page_count is None, "a zero-candidate PDF must not qualify" + assert plain.page_strip_mode is None + + manual_id, scanned_id, plain_id = str(manual.id), str(scanned.id), str(plain.id) + + # The filesystem contract: EXACTLY the two qualifying docs' copies + # (no plain copies, no orphans) — real, non-empty PNGs. + expected = sorted( + [f"{manual_id}-p{n}.png" for n in range(1, 5)] + + [f"{scanned_id}-p{n}.png" for n in range(1, N_SCANNED_PAGES + 1)] + ) + pngs = sorted(p.name for p in page_dir.glob("*.png")) + assert pngs == expected, f"the page dir holds {pngs}, expected {expected}" + for name in pngs: + data = (page_dir / name).read_bytes() + assert data[:8] == PNG_MAGIC and data, f"{name} is not a real PNG" + assert not list(page_dir.glob(f"{plain_id}-*")), ( + "the non-qualifying PDF must not have page copies" + ) + yield { + "manual_id": manual_id, + "scanned_id": scanned_id, + "plain_id": plain_id, + "page_dir": page_dir, + } + + +# --------------------------------------------------------------------------- +# Shared UI helpers +# --------------------------------------------------------------------------- + + +def _drill_to_source(page: Page, name: str) -> None: + """Sources tree: top level → the source (the file table then holds + its direct files — all three fixtures sit at the directory root).""" + page.locator("#folders-tbody .folder-link").first.wait_for(state="visible") + page.click(f'#folders-tbody a.folder-link:text-is("{name}")') + expect(page.locator("#docs-tbody tr").first).to_be_visible(timeout=30_000) + + +def _open_doc_modal(page: Page, doc_path: str) -> Locator: + """The house doc-modal idiom: the row's path link opens the + same-page modal (no new tab). Returns the content slot locator.""" + before = len(page.context.pages) + page.locator(f'#docs-tbody a.doc-link:text-is("{doc_path}")').click() + assert len(page.context.pages) == before, "the row link must not open a new tab" + modal = page.locator("#doc-modal") + expect(modal).to_be_visible(timeout=30_000) + return page.locator("#doc-modal-content") + + +def _wait_img_loaded(img: Locator) -> None: + """Wait until a LAZY ``<img>`` really loaded pixels: scroll it into + the viewport, await the browser's own ``decode()`` promise (it + rejects on a failed fetch — a 404 route), and assert + ``naturalWidth > 0`` (a real PNG, not a zero-size error image).""" + img.scroll_into_view_if_needed() + img.evaluate("el => el.decode()") + natural = img.evaluate("el => el.naturalWidth") + assert natural > 0, f"the page image did not load (naturalWidth={natural})" + + +def _assert_figures(strip: Locator, title: str, doc_id: str, page_count: int) -> None: + """The phase-148 figure contract inside a (visible) strip: exactly + *page_count* ``.doc-page`` figures, each with a LOADED lazy + ``<img>`` (the route served a real PNG — not the "Page N + unavailable" note), the ``Page N of <title>`` alt, the exact 1-based + route src, and the ``Page N`` caption.""" + figures = strip.locator(".doc-page") + expect(figures).to_have_count(page_count) + for i in range(page_count): + n = i + 1 + fig = figures.nth(i) + img = fig.locator("img") + expect(img).to_have_count(1) + expect(img).to_have_attribute("loading", "lazy") + expect(img).to_have_attribute("alt", f"Page {n} of {title}") + expect(img).to_have_attribute("src", f"/api/documents/{doc_id}/pages/{n}") + _wait_img_loaded(img) + expect(fig.locator("figcaption.doc-page-num")).to_have_text(f"Page {n}") + + +# --------------------------------------------------------------------------- +# 1. The additive view: text first, the strip one toggle away, no +# refetch on flip-flop +# --------------------------------------------------------------------------- + + +def test_additive_view(page: Page, app_url: str, seeded: dict[str, Any]) -> None: + """The ``manual.pdf`` modal — the ADDITIVE class (LOCKED A1/A6): + the content payload stamps the class (``page_strip_mode == + "additive"`` beside the 4 ``page_urls``); the TEXT view opens by + default — Summary panel + content visible, the toggle's ``Text`` + ``aria-pressed="true"``, the strip mounted but hidden, the map + page's canned description RIDING the shown text (the text is never + hidden for the pictorial class); pressing ``Pages`` shows the + 4-figure strip with EVERY img loaded (naturalWidth > 0 — the route + serves; no "unavailable" note anywhere) and hides the text view; + pressing ``Text`` back shows the text, hides the strip, and the + strip node is the SAME mounted DOM element (a dataset probe survives + the flip — no re-render) with its 4 imgs still loaded (no + refetch).""" + # The wire pin: the class is stamped next to the page_urls. + payload = _payload(app_url, MANUAL_PATH) + assert payload.get("page_strip_mode") == "additive", payload + assert payload.get("page_urls") == [ + f"/api/documents/{seeded['manual_id']}/pages/{n}" for n in range(1, 5) + ], payload + + login(page, app_url) # lands on /sources.html + _drill_to_source(page, SOURCE_NAME) + expect(page.locator("#docs-tbody tr")).to_have_count(3) + content = _open_doc_modal(page, MANUAL_PATH) + expect(page.locator("#doc-modal-title")).to_have_text(MANUAL_TITLE) + + # Default view: TEXT (LOCKED A6) — Summary panel + content visible, + # the strip mounted but hidden (its lazy imgs never fetched yet). + toggle = content.locator(".doc-view-toggle") + expect(toggle).to_have_count(1) + expect(toggle.get_by_role("button", name="Text")).to_have_attribute( + "aria-pressed", "true" + ) + expect(toggle.get_by_role("button", name="Pages")).to_have_attribute( + "aria-pressed", "false" + ) + expect(content.locator("pre.doc-raw")).to_be_visible() + expect(content.locator(".doc-summary")).to_be_visible() + expect(content.locator(".doc-pages")).to_be_hidden() + assert MAP_MARKER in content.inner_text(), "the text view shows the book's text" + assert PICTORIAL_PAGE_ANSWER in content.inner_text(), ( + "the additive text view shows the indexed content INCLUDING the " + "map page's description (LOCKED A1 — the text is never hidden)" + ) + + # Pages: the strip shows, every copy loads, the text view hides. + toggle.get_by_role("button", name="Pages").click() + expect(content.locator(".doc-pages")).to_be_visible() + _assert_figures(content.locator(".doc-pages"), MANUAL_TITLE, seeded["manual_id"], 4) + assert "unavailable" not in content.inner_text(), ( + "no page may show the 'Page N unavailable' note once the strip shows" + ) + expect(content.locator("pre.doc-raw")).not_to_be_visible() + expect(content.locator(".doc-summary")).not_to_be_visible() + # Tag the strip's FIRST img so the flip-back can prove the node is + # the same DOM element (a re-render would build a fresh one). + first_img = content.locator(".doc-pages .doc-page img").first + first_img.evaluate("el => { el.dataset.p163probe = 'mounted'; }") + + # Text: the reversal — text visible, strip hidden, the strip node + # STILL MOUNTED with its 4 loaded imgs (zero refetch). + toggle.get_by_role("button", name="Text").click() + expect(toggle.get_by_role("button", name="Text")).to_have_attribute( + "aria-pressed", "true" + ) + expect(content.locator("pre.doc-raw")).to_be_visible() + expect(content.locator(".doc-pages")).to_be_hidden() + expect(content.locator(".doc-pages .doc-page img")).to_have_count(4) + assert ( + content.locator(".doc-pages .doc-page img").first.evaluate( + "el => el.dataset.p163probe" + ) + == "mounted" + ), "flipping back must reuse the mounted strip (no re-render, no refetch)" + assert content.locator(".doc-pages .doc-page img").first.evaluate( + "el => el.complete && el.naturalWidth > 0" + ), "the mounted imgs stay decoded across the flip (no reset)" + + +# --------------------------------------------------------------------------- +# 2. The primary class (byte-identical strip-only DOM) + the plain doc +# --------------------------------------------------------------------------- + + +def test_primary_and_plain_doms(page: Page, app_url: str, seeded: dict[str, Any]) -> None: + """The two untouched shapes, pinned side by side. ``scanned.pdf`` + (the fallback class, payload ``page_strip_mode == "primary"``) + keeps the phase-148 strip-only DOM byte-identically: NO + ``.doc-view-toggle``, NO ``.doc-summary``, NO content block — just + the 2-figure strip, every img loaded with the ``Page N`` captions. + ``plain.pdf`` (no strip) renders the plain text doc — the raw + content + the Summary panel, ZERO ``.doc-pages`` / + ``.doc-view-toggle`` nodes — and its payload carries NEITHER + ``page_urls`` NOR ``page_strip_mode`` (the ABSENT-never-null + omission rule: the non-qualifying payload is byte-identical to + pre-phase).""" + # The wire pins: primary stamped; both keys absent for plain. + payload = _payload(app_url, SCANNED_PATH) + assert payload.get("page_strip_mode") == "primary", payload + assert payload.get("page_urls") == [ + f"/api/documents/{seeded['scanned_id']}/pages/{n}" + for n in range(1, N_SCANNED_PAGES + 1) + ], payload + plain_payload = _payload(app_url, PLAIN_PATH) + assert "page_urls" not in plain_payload, plain_payload + assert "page_strip_mode" not in plain_payload, plain_payload + + login(page, app_url) # lands on /sources.html + _drill_to_source(page, SOURCE_NAME) + + # The primary class: the strip IS the view — no toggle, no Summary + # panel, no content block (the phase-148 DOM, byte-for-byte). + content = _open_doc_modal(page, SCANNED_PATH) + expect(page.locator("#doc-modal-title")).to_have_text(SCANNED_TITLE) + expect(content.locator(".doc-view-toggle")).to_have_count(0) + expect(content.locator(".doc-summary")).to_have_count(0) + expect(content.locator("pre.doc-raw")).to_have_count(0) + strip = content.locator(".doc-pages") + expect(strip).to_have_count(1) + expect(strip).to_be_visible() + _assert_figures(strip, SCANNED_TITLE, seeded["scanned_id"], N_SCANNED_PAGES) + + # The plain doc: the pre-phase text rendering — no strip artifacts. + page.goto(f"{app_url}/sources.html") + _drill_to_source(page, SOURCE_NAME) + content = _open_doc_modal(page, PLAIN_PATH) + expect(page.locator("#doc-modal-title")).to_have_text(PLAIN_TITLE) + expect(content.locator(".doc-pages")).to_have_count(0) + expect(content.locator(".doc-view-toggle")).to_have_count(0) + expect(content.locator("pre.doc-raw")).to_have_count(1) + expect(content.locator("pre.doc-raw")).to_contain_text("---PAGE-1---") + expect(content.locator("pre.doc-raw")).to_contain_text("Note 2.") + expect(content.locator(".doc-summary")).to_have_count(1) + + +# --------------------------------------------------------------------------- +# 3. The heal: copies deleted under a stamped page_count → one +# unchanged-byte sync restores everything +# --------------------------------------------------------------------------- + + +def test_heal_restores_deleted_copies( + page: Page, app_url: str, seeded: dict[str, Any] +) -> None: + """The live ``Letumdei`` failure mode, reproduced and healed: delete + ALL ``pdf_page_dir/<manual-id>-p*.png`` (the row still claims 4) → + a page route 404s (the exact live evidence) → run a sync over + UNCHANGED bytes (the phase-147 fast path) → the four copies exist + again (real PNGs), the route serves 200 with PNG bytes, the row + keeps ``page_count`` 4 / mode ``additive``, the payload still + carries ``page_urls`` + ``additive``, and the viewer's Pages view + loads EVERY img — the disk-level loss the owner saw cannot outlive + one sync (LOCKED A5, the D5 promise).""" + manual_id = seeded["manual_id"] + page_dir: Path = seeded["page_dir"] + copies = sorted(page_dir.glob(f"{manual_id}-p*.png")) + assert len(copies) == 4, f"the seed must leave 4 copies: {[p.name for p in copies]}" + for p in copies: + p.unlink() + assert not list(page_dir.glob(f"{manual_id}-p*.png")), "the wipe must be complete" + # The live-loss state on the wire: the row claims 4, the route 404s. + r = page.request.get(f"{app_url}/api/documents/{manual_id}/pages/1") + assert r.status == 404, f"the wiped copy must 404 before the heal: {r.status}" + + # One sync over unchanged bytes — the fast path + the re-gate. The + # run's whole LLM footprint is captured (assert 5 consumes it). + body = _run_sync(app_url, capture_name="heal") + assert body["state"] == "success", body + detail = body["detail"] + assert detail["unchanged"] == 3, detail + assert detail["added"] == 0, detail + assert detail["errors"] == 0, detail + + # The disk: the four copies exist again — real PNGs. + restored = sorted(page_dir.glob(f"{manual_id}-p*.png")) + assert [p.name for p in restored] == [ + f"{manual_id}-p{n}.png" for n in range(1, 5) + ], f"the heal left {[p.name for p in restored]}" + for p in restored: + data = p.read_bytes() + assert data[:8] == PNG_MAGIC and data, f"{p.name} is not a real PNG after heal" + + # The route serves again; the payload keeps the strip + the class. + r = page.request.get(f"{app_url}/api/documents/{manual_id}/pages/1") + assert r.status == 200, f"the healed copy must serve: {r.status}" + assert r.headers["content-type"] == "image/png", r.headers + assert r.body()[:8] == PNG_MAGIC, "the healed route served non-PNG bytes" + payload = _payload(app_url, MANUAL_PATH) + assert payload.get("page_strip_mode") == "additive", payload + assert len(payload.get("page_urls", [])) == 4, payload + + # The row (the fixture session's view): count + class intact. + row = _doc_row(MANUAL_PATH) + assert row.page_count == 4 + assert row.page_strip_mode == "additive" + + # The viewer, complete: every page image loads through the route. + login(page, app_url) + _drill_to_source(page, SOURCE_NAME) + content = _open_doc_modal(page, MANUAL_PATH) + content.locator(".doc-view-toggle").get_by_role("button", name="Pages").click() + expect(content.locator(".doc-pages")).to_be_visible() + _assert_figures(content.locator(".doc-pages"), MANUAL_TITLE, manual_id, 4) + assert "unavailable" not in content.inner_text(), ( + "after the heal the viewer must show zero 'Page N unavailable' notes" + ) + + +# --------------------------------------------------------------------------- +# 4. The repair arm: a stale page_count on a now-ineligible doc is +# cleared, with its copies +# --------------------------------------------------------------------------- + + +def test_repair_clears_stale_strip( + page: Page, app_url: str, seeded: dict[str, Any] +) -> None: + """The stale-eligible state NO current index produces, seeded + direct-DB: ``plain.pdf``'s row gets ``page_count`` 2 + mode + ``additive`` and two stale copies land on disk (the strip-shape a + pre-phase bug or a lowered ``pdf_pictorial_min_chars`` leaves + behind). One sync (the bytes stay unchanged) → the REPAIR arm: both + columns NULL again (read through the fixture session), the stale + copies GONE, and the modal shows the plain text doc — the strip is + finished. The other docs' strips are untouched.""" + plain_id = seeded["plain_id"] + page_dir: Path = seeded["page_dir"] + # The seed: the row claims a strip its current gate would never set. + with SessionLocal() as db: + doc = db.scalar( + select(Document).where( + Document.source == SOURCE_NAME, Document.path == PLAIN_PATH + ) + ) + assert doc is not None + doc.page_count = 2 + doc.page_strip_mode = "additive" + db.commit() + stale = [page_dir / f"{plain_id}-p{n}.png" for n in (1, 2)] + for p in stale: + p.write_bytes(b"STALE-PRE-PHASE-COPY") # not even a real PNG + + body = _run_sync(app_url, capture_name="repair") + assert body["state"] == "success", body + detail = body["detail"] + assert detail["unchanged"] == 3, detail + assert detail["errors"] == 0, detail + + # The row (the fixture session's view): both columns cleared. + row = _doc_row(PLAIN_PATH) + assert row.page_count is None, ( + f"the repair arm must clear a stale page_count: {row.page_count}" + ) + assert row.page_strip_mode is None, ( + f"the repair arm must clear the mode with the count: {row.page_strip_mode}" + ) + # The stale copies are gone; the other docs' copies untouched. + assert not list(page_dir.glob(f"{plain_id}-*")), ( + "the repair arm must delete the stale copies" + ) + assert len(list(page_dir.glob(f"{seeded['manual_id']}-p*.png"))) == 4, ( + "the repair of one doc must not touch another doc's copies" + ) + assert len(list(page_dir.glob(f"{seeded['scanned_id']}-p*.png"))) == 2 + + # The wire + the viewer: the strip is gone, the plain text doc is + # back (the same payload shape as test 2 pinned). + assert "page_urls" not in _payload(app_url, PLAIN_PATH) + login(page, app_url) + _drill_to_source(page, SOURCE_NAME) + content = _open_doc_modal(page, PLAIN_PATH) + expect(content.locator(".doc-pages")).to_have_count(0) + expect(content.locator(".doc-view-toggle")).to_have_count(0) + expect(content.locator("pre.doc-raw")).to_be_visible() + + +# --------------------------------------------------------------------------- +# 5. Zero LLM calls on the fast-path repair (LOCKED A5) +# --------------------------------------------------------------------------- + + +def test_zero_llm_calls_on_heal_and_repair(seeded: dict[str, Any]) -> None: + """The mock's request log (the house capture pattern): the INITIAL + index burned the vision passes it owes — at least one + ``PICTORIAL_PAGE_MODE`` request (the map page) and at least one + ``IMAGE_DESCRIPTION_MODE`` request (the fallback screenshots), the + witnesses that the capture really sees the app's calls — while the + HEAL sync and the REPAIR sync each captured EXACTLY ZERO + chat-completions requests. The re-gate mirrors the gate + deterministically from the current bytes + settings: rendering is + CPU-only, so the self-heal burns zero LLM/vision calls (LOCKED A5).""" + if USE_REAL_LLM: + pytest.skip("the zero-call pin reads the mock's request capture") + index = LLM_CAPTURES.get("index") + heal = LLM_CAPTURES.get("heal") + repair = LLM_CAPTURES.get("repair") + assert index is not None and heal is not None and repair is not None, ( + "the suite ledger must hold all three syncs' captures — run the tests in order" + ) + dumped = json.dumps(index) + assert "PICTORIAL_PAGE_MODE" in dumped, ( + "the initial index must have burned the pictorial vision pass — " + "without this witness the zero-call pin below is vacuous" + ) + assert "IMAGE_DESCRIPTION_MODE" in dumped, ( + "the initial index must have burned the fallback screenshot pass" + ) + heal_content = [b for b in heal if not _is_health_probe(b)] + repair_content = [b for b in repair if not _is_health_probe(b)] + assert heal_content == [], f"the heal sync burned LLM calls: {_summarize(heal_content)}" + assert repair_content == [], ( + f"the repair sync burned LLM calls: {_summarize(repair_content)}" + ) + + +def _is_health_probe(body: dict[str, Any]) -> bool: + """The sync prelude's fixed model-health ping (``app.rag.llm + .check_models`` — one non-streaming ``ping`` chat every sync ever + run pays, unchanged by phase 163). Exempted from the zero-call + pins; NOTHING else may appear in a heal/repair capture.""" + msgs = body.get("messages") or [] + return ( + not body.get("stream") + and len(msgs) == 1 + and msgs[0].get("content") == "ping" + ) + + +def _summarize(captured: list[dict[str, Any]]) -> str: + """A compact one-liner per captured request (its system-prompt + marker) for the failure messages — keeps a violation readable + without dumping whole prompt bodies.""" + out = [] + for body in captured: + msgs = body.get("messages", []) + first = str(msgs[0].get("content", ""))[:80] if msgs else "?" + out.append(f"chat(stream={bool(body.get('stream'))}): {first}") + return " | ".join(out) diff --git a/tests/e2e/test_pictorial_pdf.py b/tests/e2e/test_pictorial_pdf.py index 185f04f..653dacf 100644 --- a/tests/e2e/test_pictorial_pdf.py +++ b/tests/e2e/test_pictorial_pdf.py @@ -24,11 +24,14 @@ format — the upload gate admits it) and arriving through the REAL admin upload flow → the RAG page's Sync button: * (a) the document appears on the Sources page and the document viewer - shows the PDF's ACTUAL pages (phase 148 — a described sparse page - makes the doc page-QUALIFYING: the page strip REPLACES the text - content AND the Summary panel, the owner's TODO L5 contract): two - ``.doc-page`` figures with loaded lazy images + ``Page N`` captions, - NO ``.doc-raw``, NO ``.doc-summary``; the described content itself + opens the PDF in its ADDITIVE view (phase 163 — a described sparse + page makes the doc page-QUALIFYING, and a usable text layer makes + the strip ADDITIVE: the text stays, the pages sit behind a + Text/Pages toggle — LOCKED A1/A6): the Text view first (``.doc-raw`` + + ``.doc-summary`` visible, ``Text`` pressed), the strip one + ``Pages`` click away — two ``.doc-page`` figures with loaded lazy + images + ``Page N`` captions, the text view hidden while it shows; + the described content itself (page 1's prose WITHOUT an illustration block, page 2 WITH ``[Page 2 illustration: <the mock's canned PICTORIAL_PAGE_ANSWER>]`` — the task-02 ``PICTORIAL_PAGE_MODE`` branch, deterministic) stays in @@ -83,14 +86,17 @@ Test → observable mapping (Playwright Mapping Rule): Sources page reads 1 doc / 3 chunks (2 page-anchored content chunks + the summary chunk — no image doc), the drill-down table lists the PDF as a plain text row (LOCKED A4 — a text document, no thumbnail), and - the same-page document viewer (the row's path link) shows the - PHASE-148 PAGE STRIP under the METADATA title (the fixture is - page-qualifying — the pictorial pass described its sparse page): - exactly two ``.doc-page`` figures, each a LOADED lazy ``<img>`` - (naturalWidth > 0) with the ``Page N of <title>`` alt + the - ``Page N`` caption, NO ``.doc-raw`` / NO ``.doc-summary`` (the - page descriptions stay in the DB for Q&A, not displayed — the - owner's contract). + the same-page document viewer (the row's path link) opens the + PHASE-163 ADDITIVE VIEW under the METADATA title (the fixture is + page-qualifying with a usable text layer — the pictorial pass + described its sparse page): the TEXT view first — ``.doc-raw`` + + ``.doc-summary`` visible with the ``[Page 2 illustration: …]`` + block among them, the toggle's ``Text`` pressed, the strip mounted + but hidden; pressing ``Pages`` shows exactly two ``.doc-page`` + figures, each a LOADED lazy ``<img>`` (naturalWidth > 0) with the + ``Page N of <title>`` alt + the ``Page N`` caption while the text + view hides (mounted for the flip-back — the text is never + destroyed). 2. ``test_chat_question_grounds_and_echoes_the_illustration`` — the raw SSE over the scripted seeded-read question (httpx — the ``done`` event is the wire lens the task pins; the UI flow's premise is the @@ -578,8 +584,8 @@ def seeded( ) # The non-markdown doc got its lite summary (the mock's # SUMMARY_MODE digest) — the summary chunk exists (the - # viewer's Summary panel is suppressed by the phase-148 - # strip — the summary itself stays in the DB). + # additive view shows the Summary panel; the summary + # itself stays in the DB regardless). assert doc.summary # Phase 148: the described sparse page makes this doc # page-QUALIFYING — the strip's payload (page_urls) rides @@ -602,18 +608,19 @@ def test_sources_page_and_viewer_show_the_described_page( content chunks + the phase-30 summary chunk — no image doc; the vector figure is invisible to the embedded-image pipeline). The drill-down table lists the PDF as a plain text row (LOCKED A4), and - the same-page document viewer (the row's path link, phase 26) shows - the PHASE-148 PAGE STRIP under the METADATA title: the fixture is - page-qualifying (the pictorial pass described its sparse page), so - the strip REPLACES the text content AND the Summary panel — two - ``.doc-page`` figures, each a loaded lazy ``<img>`` with the - ``Page N of <title>`` alt + the ``Page N`` caption, NO - ``.doc-raw``, NO ``.doc-summary``. The described content (page 1's - prose WITHOUT an illustration block, page 2 WITH - ``[Page 2 illustration: <canned>]`` — LOCKED A4) stays in the DB - for Q&A: the seed asserted it byte-exact, the chat test proves it - reaches the model — it is simply not displayed (the owner's - contract).""" + the same-page document viewer (the row's path link, phase 26) + opens the PHASE-163 ADDITIVE VIEW (LOCKED A1/A6): the TEXT view + first — Summary panel + ``.doc-raw`` visible, the Text/Pages + toggle's ``Text`` pressed — with the ``[Page 2 illustration: + <canned>]`` block riding the SHOWN text (the text is never hidden + for the pictorial class). Pressing ``Pages`` reveals the strip — + two ``.doc-page`` figures, each a loaded lazy ``<img>`` with the + ``Page N of <title>`` alt + the ``Page N`` caption — while the + text view is hidden (mounted, not removed: flipping back shows it + with zero refetch). The described content (page 1's prose WITHOUT + an illustration block, page 2 WITH the block — LOCKED A4) also + stays in the DB for Q&A: the seed asserted it byte-exact, the chat + test proves it reaches the model.""" login(page, app_url) # lands on /sources.html expect(page.locator("#stat-docs")).to_have_text("1") expect(page.locator("#stat-chunks")).to_have_text("3") @@ -630,13 +637,31 @@ def test_sources_page_and_viewer_show_the_described_page( expect(modal).to_be_visible(timeout=30_000) expect(page.locator("#doc-modal-title")).to_have_text(PDF_TITLE) - # Phase 148: the page-qualifying fixture shows its ACTUAL pages — - # a .doc-pages strip with exactly two .doc-page figures (the - # 2-page fixture), each a LOADED lazy <img> (naturalWidth > 0 — - # the route served a real PNG) with the "Page N of <title>" alt + - # the "Page N" caption. The text content AND the Summary panel are - # SUPPRESSED (the owner's contract — the descriptions stay in the - # DB for Q&A, not displayed). + # Phase 163 (task 04, LOCKED A1/A6): the page-qualifying fixture + # with a usable text layer is an ADDITIVE doc — it opens on its + # TEXT view: Summary panel + content visible, Text pressed, the + # strip mounted but hidden (its lazy imgs never fetched). + toggle = modal.locator(".doc-view-toggle") + expect(toggle).to_have_count(1) + expect(toggle.get_by_role("button", name="Text")).to_have_attribute( + "aria-pressed", "true" + ) + expect(toggle.get_by_role("button", name="Pages")).to_have_attribute( + "aria-pressed", "false" + ) + expect(modal.locator("pre.doc-raw")).to_be_visible() + expect(modal.locator(".doc-summary")).to_be_visible() + expect(modal.locator(".doc-pages")).to_be_hidden() + # The canned page description RIDES the shown text (phase-140 + # blocks are part of the content — the additive view never hides + # the text). The node stays in the DOM, so a visibility-scoped + # inner_text() check would miss it — use the DOM text. + assert PICTORIAL_PAGE_ANSWER in modal.evaluate("el => el.textContent"), ( + "the additive text view shows the indexed content including " + "the described sparse page" + ) + # …and the strip is one Pages click away (phase 148's ask): + toggle.get_by_role("button", name="Pages").click() strip = modal.locator(".doc-pages") expect(strip).to_have_count(1) figures = strip.locator(".doc-page") @@ -657,17 +682,12 @@ def test_sources_page_and_viewer_show_the_described_page( expect(figures.nth(i).locator("figcaption.doc-page-num")).to_have_text( f"Page {n}" ) - # The suppression contract: no text content block, no Summary - # panel, no image block (the PDF is not an image document). - expect(modal.locator("pre.doc-raw")).to_have_count(0) - expect(modal.locator(".doc-summary")).to_have_count(0) + # While the strip shows, the text view is HIDDEN — mounted for + # the flip back (LOCKED A1), never removed. No image block (the + # PDF is not an image document). + expect(modal.locator("pre.doc-raw")).not_to_be_visible() + expect(modal.locator(".doc-summary")).not_to_be_visible() expect(modal.locator(".doc-image-img")).to_have_count(0) - # The canned description is in the DB (the seed asserted it) - # but nowhere in the rendered modal. - assert PICTORIAL_PAGE_ANSWER not in modal.inner_text(), ( - "the described page's text is in the DB for Q&A — it must not " - "be shown to the user (the owner's contract)" - ) # --------------------------------------------------------------------------- diff --git a/tests/integration/test_docs_api.py b/tests/integration/test_docs_api.py index b554184..22ddacb 100644 --- a/tests/integration/test_docs_api.py +++ b/tests/integration/test_docs_api.py @@ -10,7 +10,10 @@ per phase 124 A4) and the phase-148 (task 02) PDF page route (``GET /api/documents/{id}/pages/{n}`` — the same trust model and 404 posture for a page-qualifying document's rendered page PNGs) + the content endpoint's ``image_url`` / ``page_urls`` keys (present for the -qualifying row, ABSENT otherwise — never ``null``). +qualifying row, ABSENT otherwise — never ``null``), plus the phase-163 +(task 01) ``page_strip_mode`` class signal (``"primary"`` /``"additive"`` +for a stamped strip, ABSENT for a pre-phase row and for every +non-qualifying document). Uses the real compose Postgres (``db`` fixture) and FastAPI's TestClient. """ @@ -1381,7 +1384,8 @@ def test_content_endpoint_exposes_the_image_affordance( # --------------------------------------------------------------------------- # Phase 148 (task 02) — the PDF page BYTES route + the content payload's -# ``page_urls`` affordance +# ``page_urls`` affordance (phase 163 task 01 adds the ``page_strip_mode`` +# class signal beside it) # --------------------------------------------------------------------------- @@ -1404,6 +1408,7 @@ def _seed_page_doc( path: str = "besta.pdf", page_count: int | None = 2, write_copies: bool = True, + page_strip_mode: str | None = None, ) -> Document: """One PDF document row with ``page_count`` + its rendered page copies (the importer's ``pdf_page_dir`` layout — @@ -1411,7 +1416,8 @@ def _seed_page_doc( sentinel bytes, so a page mix-up between the two copies is caught. ``page_count=None`` is a non-qualifying PDF (no copies); ``write_copies=False`` leaves a qualifying row without its files - (the lost-copy corner).""" + (the lost-copy corner); ``page_strip_mode`` (phase 163) stamps the + strip CLASS — ``None`` is the pre-phase row.""" doc = Document( id=uuid.uuid4(), source=source, @@ -1424,6 +1430,7 @@ def _seed_page_doc( created_at=datetime(2026, 1, 1, tzinfo=UTC), summary="IKEA BESTA frame assembly manual (page descriptions).", page_count=page_count, + page_strip_mode=page_strip_mode, ) db.add(doc) db.flush() @@ -1610,6 +1617,79 @@ def test_content_endpoint_exposes_the_page_urls_affordance( ) assert r.status_code == 200 assert "page_urls" not in r.json() # absent — never null (non-qualifying PDF) + # Phase 163 (task 01): the class key follows the same rule — a + # pre-phase row (mode NULL) omits it while keeping page_urls. + assert "page_strip_mode" not in r.json() + finally: + _cleanup_kb(db) + + +@pytest.mark.parametrize("mode", ["primary", "additive"]) +def test_content_endpoint_carries_the_strip_class( + admin_client: TestClient, db, tmp_path: Path, mode: str +) -> None: + """Phase 163 (task 01, LOCKED A2/A3) — the real-DB round-trip of the + strip CLASS: a stamped row (``page_strip_mode`` = ``"primary"`` — the + phase-133 fallback class, the strip IS the view — or ``"additive"`` — + the phase-140 pictorial class, text + a Text/Pages toggle) serves the + class VERBATIM beside the untouched ``page_urls`` list (the viewer's + branch signal, task 04); every other key stands where phase 148 left + it.""" + _cleanup_kb(db) + try: + pages_dir = tmp_path / "pages" + _seed_page_doc(db, pages_dir, page_count=3, page_strip_mode=mode) + r = admin_client.get( + "/api/documents/content", params={"source": "PdfSrc", "path": "besta.pdf"} + ) + assert r.status_code == 200 + body = r.json() + assert body["page_strip_mode"] == mode + assert len(body["page_urls"]) == 3 # the strip rides along, unchanged + finally: + _cleanup_kb(db) + + +def test_content_strip_class_never_travels_without_the_strip( + admin_client: TestClient, db, tmp_path: Path +) -> None: + """Phase 163 (task 01) — the two strip keys are ONE block keyed off + ``page_count``: a row with a stray ``page_strip_mode`` and no strip + (``page_count`` NULL — the corner a partial repair could leave) + serializes NEITHER key, byte-identically to a plain non-qualifying + document (the viewer must never be handed a class for pages that do + not exist).""" + _cleanup_kb(db) + try: + pages_dir = tmp_path / "pages" + _seed_page_doc( + db, + pages_dir, + path="stray.pdf", + page_count=None, + write_copies=False, + page_strip_mode="additive", + ) + r = admin_client.get( + "/api/documents/content", params={"source": "PdfSrc", "path": "stray.pdf"} + ) + assert r.status_code == 200 + body = r.json() + assert "page_strip_mode" not in body + assert "page_urls" not in body + # The exact pre-phase key set (the byte-identity pin). + assert set(body) == { + "source", + "path", + "title", + "format", + "summary", + "created_at", + "content", + "indexed_at", + "chunks", + "is_image", + } finally: _cleanup_kb(db) diff --git a/tests/integration/test_migration_0027.py b/tests/integration/test_migration_0027.py new file mode 100644 index 0000000..bc2a786 --- /dev/null +++ b/tests/integration/test_migration_0027.py @@ -0,0 +1,370 @@ +"""Integration: migration 0027 (documents.page_strip_mode) schema contract +(phase 163, task 01). + +Drives the **real Alembic engine** against the live dev database +(``podman compose up -d db``), mirroring the house pattern of +``test_migration_0026.py`` (information_schema assertions on the state +the migration must leave). The tests target the 0026 → 0027 step +explicitly so later migrations cannot break the pins: + +* upgrade 0026 → 0027 → ``page_strip_mode`` exists with the full + contract — VARCHAR(16), NULLABLE, NO server default (NULL = "no strip" + for every pre-phase row, every non-qualifying document and every + fail-soft miss) and UNINDEXED (a display fact, never a query surface — + the content endpoint reads it from the ROW, the ``file_hash`` / + ``page_count`` convention) — while the 0026 ``documents`` schema + survives, in particular ``page_count`` keeping its 0026 contract and + ``content_hash`` staying NOT NULL and INDEXED; +* rows inserted while the DB is at 0026 backfill ``page_strip_mode`` to + NULL — including a row whose ``page_count`` IS set (the pre-phase strip + with no stamped class: the wire omits the key and the viewer keeps + today's "primary" DOM until the task-03 re-gate stamps it); +* the ORM contract agrees: a freshly inserted ``Document`` with no mode + reads ``page_strip_mode is None`` through a fresh session, and each of + the two classes (``"primary"`` / ``"additive"``) round-trips verbatim; +* downgrade to 0026 → the column is GONE (A13) while the row + its + content + its ``page_count`` survive; upgrade back to 0027 → the column + is back (round-trip). + +The ``alembic`` fixture guarantees the DB ends at head even if a test +fails or the process is interrupted. +""" +from __future__ import annotations + +import hashlib +import uuid +from collections.abc import Iterator +from typing import Any + +import pytest +from alembic.config import Config +from sqlalchemy import text +from sqlalchemy.orm import Session + +from alembic import command +from app.db import SessionLocal, db_available +from app.models import Document + +SOURCE = "mig0027" +CONTENT = "content here" +CONTENT_HASH = hashlib.sha256(CONTENT.encode()).hexdigest() + + +@pytest.fixture() +def alembic(db: Session) -> Iterator[Config]: + """Real Alembic config bound to the dev DB (URL from app settings). + + Starts at head (repairs an interrupted earlier run); teardown + upgrades to head no matter what happened, so the dev DB is never + left below head. + """ + if not db_available(): + pytest.skip("Postgres not reachable — run `podman compose up -d db` first") + cfg = Config() # no alembic.ini file — env.py gets the URL from app config + cfg.set_main_option("script_location", "alembic") + command.upgrade(cfg, "head") + try: + yield cfg + finally: + # Release the test session's open transaction BEFORE the repair + # DDL: an idle-in-transaction SELECT holds an ACCESS SHARE lock + # on ``documents``, which would deadlock the repair's + # ``ALTER TABLE`` forever. + db.rollback() + command.upgrade(cfg, "head") + + +def _version(db: Session) -> str | None: + return db.execute(text("SELECT version_num FROM alembic_version")).scalar() + + +def _column(db: Session, column: str) -> tuple[Any, ...] | None: + """(data_type, is_nullable, column_default, character_maximum_length) + for one documents column.""" + row = db.execute( + text( + "SELECT data_type, is_nullable, column_default, character_maximum_length" + " FROM information_schema.columns" + " WHERE table_name = 'documents' AND column_name = :c" + ), + {"c": column}, + ).fetchone() + return tuple(row) if row is not None else None + + +def _index_covering(db: Session, column: str) -> str | None: + """The name of any documents index whose definition mentions the + column (None = UNINDEXED).""" + return db.execute( + text( + "SELECT indexname FROM pg_indexes" + " WHERE tablename = 'documents' AND indexdef ILIKE :like" + ), + {"like": f"%{column}%"}, + ).scalar() + + +def _insert_sql(db: Session, path: str, page_count: int | None = None) -> uuid.UUID: + """Insert one documents row with the PRE-0027 column set (the 0026 + shape — the mode column, when present, is omitted so a NULL backfill + is what the row reads; ``page_count`` is 0026's own column and may be + set to model a pre-phase strip).""" + doc_id = uuid.uuid4() + db.execute( + text( + "INSERT INTO documents (id, source, path, full_path, title, content," + " content_hash, page_count)" + " VALUES (:id, :s, :p, :fp, :t, :c, :h, :pc)" + ), + { + "id": doc_id, + "s": SOURCE, + "p": path, + "fp": f"/tmp/{path}", + "t": f"Doc {path}", + "c": CONTENT, + "h": CONTENT_HASH, + "pc": page_count, + }, + ) + db.commit() + return doc_id + + +def _delete(db: Session, doc_id: uuid.UUID) -> None: + db.execute(text("DELETE FROM documents WHERE id = :id"), {"id": doc_id}) + db.commit() + + +def _assert_0026_schema_survives(db: Session) -> None: + """The 0026 documents contract must survive the additive upgrade — + ``page_count`` keeps its INTEGER / NULLABLE / NO-default / UNINDEXED + shape (the sibling column this one rides) and ``content_hash`` stays + NOT NULL and indexed (the retriever's identity).""" + page_count = _column(db, "page_count") + assert page_count is not None, "documents.page_count (0026) must survive the upgrade" + assert page_count[0] == "integer", "page_count must stay INTEGER" + assert page_count[1] == "YES", "page_count must stay NULLABLE" + assert page_count[2] is None, "page_count must keep NO server default" + assert _index_covering(db, "page_count") is None, "page_count must stay UNINDEXED" + + content = _column(db, "content_hash") + assert content is not None and content[0] == "character varying" and content[1] == "NO", ( + "documents.content_hash (0001) must keep its contract" + ) + assert ( + _index_covering(db, "content_hash") is not None + ), "the ix_documents_content_hash index must survive the upgrade" + + indexed = _column(db, "indexed_at") + assert indexed is not None and indexed[0] == "timestamp with time zone", ( + "documents.indexed_at (0001) must survive the upgrade" + ) + is_image = _column(db, "is_image") + assert is_image is not None and is_image[0] == "boolean" and is_image[1] == "NO", ( + "documents.is_image (0022) must survive the upgrade" + ) + + +def test_upgrade_to_0027_adds_page_strip_mode(db: Session, alembic: Config) -> None: + """Upgrade 0026 → 0027: ``page_strip_mode`` exists with the full + contract (VARCHAR(16), NULLABLE, NO server default — NULL = "no + strip" — and UNINDEXED: a display fact, never a query surface), is + ABSENT at 0026, and every pre-0027 row backfills it to NULL — + including the row whose ``page_count`` IS set (the pre-phase strip + with no stamped class), while the 0026 table contract (the + ``page_count`` sibling, the indexed ``content_hash``) survives. + """ + command.downgrade(alembic, "0026") # start from the pre-0027 state + assert _version(db) == "0026" + assert _column(db, "page_strip_mode") is None, "page_strip_mode must be absent at 0026" + + plain_id = _insert_sql(db, "plain.pdf") # the 0026 column set, no strip + pre_strip_id = _insert_sql(db, "prephase.pdf", page_count=36) # a live pre-phase strip + + try: + command.upgrade(alembic, "0027") + assert _version(db) == "0027", "alembic_version must be at 0027" + + col = _column(db, "page_strip_mode") + assert col is not None, "documents.page_strip_mode is missing" + assert col[0] == "character varying", "page_strip_mode must be VARCHAR" + assert col[3] == 16, "page_strip_mode must be VARCHAR(16) — the two class names" + assert col[1] == "YES", ( + "page_strip_mode must be NULLABLE (NULL = no strip, and the pre-phase shape)" + ) + assert col[2] is None, ( + "page_strip_mode must carry NO server default — NULL is the meaningful" + " pre-phase state" + ) + assert _index_covering(db, "page_strip_mode") is None, ( + "page_strip_mode must be UNINDEXED — a display fact, never a query" + " surface (the content endpoint reads it from the row)" + ) + + # Every pre-0027 row backfills to NULL — no backfill, no default. + assert ( + db.execute( + text("SELECT page_count, page_strip_mode FROM documents WHERE id = :id"), + {"id": plain_id}, + ).fetchone() + == (None, None) + ), "a pre-0027 row without a strip must read both columns NULL" + assert ( + db.execute( + text("SELECT page_count, page_strip_mode FROM documents WHERE id = :id"), + {"id": pre_strip_id}, + ).fetchone() + == (36, None) + ), ( + "a pre-phase strip (page_count set) must backfill page_strip_mode to" + " NULL — the wire omits the key and the viewer keeps today's DOM" + ) + + _assert_0026_schema_survives(db) + finally: + _delete(db, plain_id) + _delete(db, pre_strip_id) + + +@pytest.mark.parametrize("mode", ["primary", "additive"]) +def test_orm_page_strip_mode_round_trips(db: Session, alembic: Config, mode: str) -> None: + """The ORM contract agrees with the column contract: each class + (``"primary"`` = the fallback class, ``"additive"`` = the pictorial + class) round-trips VERBATIM through a fresh session, and a row + created with no mode reads ``page_strip_mode is None`` (the + pre-phase shape — NULL until the importer stamps it).""" + command.upgrade(alembic, "head") + stamped = Document( + source=SOURCE, + path="stamped.pdf", + full_path="/tmp/stamped.pdf", + title="stamped", + content=CONTENT, + content_hash=CONTENT_HASH, + page_count=36, + page_strip_mode=mode, + ) + unstamped = Document( + source=SOURCE, + path="unstamped.md", + full_path="/tmp/unstamped.md", + title="unstamped", + content=CONTENT, + content_hash=CONTENT_HASH, + ) + db.add(stamped) + db.add(unstamped) + db.commit() + try: + with SessionLocal() as fresh: + reloaded = fresh.get(Document, stamped.id) + assert reloaded is not None, "the stamped row must be readable" + assert reloaded.page_strip_mode == mode, ( + f"the {mode!r} class must round-trip through the DB verbatim" + ) + assert reloaded.page_count == 36, "the sibling page_count must round-trip too" + reloaded_unstamped = fresh.get(Document, unstamped.id) + assert reloaded_unstamped is not None, "the unstamped row must be readable" + assert reloaded_unstamped.page_strip_mode is None, ( + "a row created without a mode must read NULL (the pre-phase shape)" + ) + assert reloaded_unstamped.content_hash == CONTENT_HASH, ( + "content_hash must be untouched by the page_strip_mode addition" + ) + finally: + _delete(db, stamped.id) + _delete(db, unstamped.id) + + +def test_downgrade_to_0026_drops_the_column(db: Session, alembic: Config) -> None: + """Downgrade 0027 → 0026: the page_strip_mode column is gone (A13 — + fully reversible, the class is re-derivable from the source file by + the deterministic re-gate) while the row + its content + its 0026 + ``page_count`` survive, and the rest of the table contract is intact.""" + command.upgrade(alembic, "head") + row = Document( + source=SOURCE, + path="down.pdf", + full_path="/tmp/down.pdf", + title="down", + content=CONTENT, + content_hash=CONTENT_HASH, + page_count=36, + page_strip_mode="additive", + ) + db.add(row) + db.commit() + try: + command.downgrade(alembic, "0026") + assert _version(db) == "0026" + assert _column(db, "page_strip_mode") is None, "page_strip_mode must be dropped" + + surviving = db.execute( + text( + "SELECT source, path, content, content_hash, page_count, is_image" + " FROM documents WHERE id = :id" + ), + {"id": row.id}, + ).fetchone() + assert surviving is not None and surviving[0] == SOURCE, ( + "the row must survive the column drop" + ) + assert surviving[1] == "down.pdf" and surviving[2] == CONTENT, ( + "path + content must survive the drop" + ) + assert surviving[3] == CONTENT_HASH, ( + "content_hash (the retriever's identity) must survive the drop" + ) + assert surviving[4] == 36, ( + "page_count (the 0026 sibling) must survive the drop — the strip" + " itself is untouched by this migration's reversal" + ) + assert surviving[5] is False, "is_image must survive the drop" + assert _index_covering(db, "content_hash") is not None, ( + "the content_hash index must survive the downgrade" + ) + finally: + _delete(db, row.id) + # Repair: the fixture teardown re-upgrades to head. + + +def test_upgrade_round_trip_restores_the_column(db: Session, alembic: Config) -> None: + """Downgrade to 0026, then upgrade back to 0027: ``page_strip_mode`` + is back with the full contract (VARCHAR(16), NULLABLE, NO server + default, UNINDEXED), a row seeded at 0026 — strip included — reads + the mode as NULL (the pre-phase shape on the round-trip), and the + 0026 schema (the ``page_count`` sibling, the indexed + ``content_hash``) is intact.""" + command.downgrade(alembic, "0026") + pre_id = _insert_sql(db, "round.pdf", page_count=36) + try: + command.upgrade(alembic, "0027") + assert _version(db) == "0027", "round-trip upgrade must land at 0027" + + col = _column(db, "page_strip_mode") + assert col is not None, "documents.page_strip_mode must be back" + assert col[0] == "character varying", "page_strip_mode must be VARCHAR after the round-trip" + assert col[3] == 16, "page_strip_mode must be VARCHAR(16) after the round-trip" + assert col[1] == "YES", "page_strip_mode must be NULLABLE after the round-trip" + assert col[2] is None, ( + "page_strip_mode must still carry NO server default after the round-trip" + ) + assert _index_covering(db, "page_strip_mode") is None, ( + "page_strip_mode must stay UNINDEXED after the round-trip" + ) + + assert ( + db.execute( + text("SELECT page_count, page_strip_mode FROM documents WHERE id = :id"), + {"id": pre_id}, + ).fetchone() + == (36, None) + ), ( + "a row seeded at 0026 must read page_strip_mode NULL after the" + " round-trip while its page_count stands" + ) + + _assert_0026_schema_survives(db) + finally: + _delete(db, pre_id) diff --git a/tests/unit/test_document_viewer.py b/tests/unit/test_document_viewer.py index 5af5cf4..df64a31 100644 --- a/tests/unit/test_document_viewer.py +++ b/tests/unit/test_document_viewer.py @@ -30,6 +30,17 @@ page-qualifying PDF row (``page_count`` set) adds ``page_urls`` = the exact 1-based ``/api/documents/<id>/pages/<i>`` list over ``1..page_count``. +Phase 163 (task 01) — the content payload's STRIP CLASS signal +(``page_strip_mode``: ``"primary"`` = the fallback class, the strip IS +the view; ``"additive"`` = the pictorial class, text + a Text/Pages +toggle): the model-level omission rule (ABSENT when ``None``, never +``null``), and the endpoint (stubbed session) — the key rides +``page_urls`` for a qualifying row, a PRE-PHASE row (``page_count`` set, +mode NULL — the exact live ``Letumdei`` state) serializes it ABSENT while +keeping ``page_urls``, and the whole strip block keys off +``page_count``, so a row with a stray mode and no strip carries neither +key (every non-qualifying payload byte-identical to pre-phase). + Phase 145 (task 03) — the content payload's git origin fields + the ``renderDocument`` "Original" anchor (modal AND /document.html, one site): @@ -316,10 +327,11 @@ def _text_doc_content( origin_url: str | None = None, origin_branch: str | None = None, page_urls: list[str] | None = None, + page_strip_mode: str | None = None, ) -> DocContent: """One text doc's content payload (the pre-phase shape) with the - phase-145 origin fields + the phase-148 ``page_urls`` optionally - set.""" + phase-145 origin fields + the phase-148 ``page_urls`` + the + phase-163 ``page_strip_mode`` optionally set.""" return DocContent( source="S", path="a.md", @@ -332,6 +344,7 @@ def _text_doc_content( origin_url=origin_url, origin_branch=origin_branch, page_urls=page_urls, + page_strip_mode=page_strip_mode, ) @@ -367,9 +380,29 @@ def test_doc_content_page_urls_absent_on_the_wire_when_unset() -> None: assert dumped["page_urls"] == urls # the exact 1-based route list -def _page_pdf_doc() -> Document: +def test_doc_content_page_strip_mode_absent_on_the_wire_when_unset() -> None: + """Phase 163 (task 01, LOCKED A2) — the model-level omission rule + (the ``image_url`` / ``page_urls`` precedent): no strip class + (``page_strip_mode`` ``None`` — every non-qualifying doc AND every + pre-phase row) serializes the key ABSENT — never ``null`` — so those + payloads are byte-identical to pre-phase. A stamped row carries the + class verbatim, beside ``page_urls``.""" + dumped = _text_doc_content().model_dump() + assert "page_strip_mode" not in dumped # absent — never null + for mode in ("primary", "additive"): + dumped = _text_doc_content( + page_urls=["/api/documents/d/pages/1"], page_strip_mode=mode + ).model_dump() + assert dumped["page_strip_mode"] == mode # the class verbatim + + +def _page_pdf_doc( + page_count: int | None = 2, page_strip_mode: str | None = None +) -> Document: """A page-QUALIFYING PDF row (phase 148): ``page_count`` set — the - endpoint derives ``page_urls`` from it (the exact 1-based list).""" + endpoint derives ``page_urls`` from it (the exact 1-based list). + ``page_strip_mode`` (phase 163) is the strip CLASS; ``None`` is the + PRE-PHASE row shape (the column NULL while the strip stands).""" doc = Document( id=uuid.uuid4(), source="Homelab", @@ -379,7 +412,8 @@ def _page_pdf_doc() -> Document: content="[Page 1 illustration: the parts tray]…", content_hash="e" * 64, is_image=False, # phase 122: the stub session never applies defaults - page_count=2, + page_count=page_count, + page_strip_mode=page_strip_mode, ) doc.indexed_at = datetime(2026, 8, 22, 1, 2, 3, tzinfo=UTC) doc.created_at = datetime(2026, 8, 20, 9, 0, 0, tzinfo=UTC) @@ -410,6 +444,79 @@ database): a page-qualifying PDF row (``page_count`` set) carries ] # the exact 1-based list, count == page_count assert body["is_image"] is False assert "image_url" not in body # absent — never null (not an image doc) + # Phase 163 (task 01): the pre-phase row (mode NULL) keeps the strip + # and serializes the class ABSENT — the viewer treats that as + # "primary", i.e. today's DOM. + assert "page_strip_mode" not in body + + +@pytest.mark.parametrize("mode", ["primary", "additive"]) +def test_content_page_strip_mode_carried_for_a_stamped_strip(mode: str) -> None: + """Phase 163 (task 01, LOCKED A2/A3) — the endpoint (stubbed +session, no +database): a page-qualifying PDF whose row carries the class + (``"primary"`` = the fallback class, ``"additive"`` = the pictorial + class) carries ``page_strip_mode`` VERBATIM next to ``page_urls`` — + the signal the viewer branches on so an additive strip never hides + the text (task 04). Nothing else in the payload moves.""" + doc = _page_pdf_doc(page_strip_mode=mode) + with _client_with_row((doc, 4)) as client: + r = client.get( + "/api/documents/content", + params={"source": "Homelab", "path": "ikea/besta.pdf"}, + ) + assert r.status_code == 200 + body = r.json() + assert body["page_strip_mode"] == mode # the class verbatim + assert body["page_urls"] == [ + f"/api/documents/{doc.id}/pages/1", + f"/api/documents/{doc.id}/pages/2", + ] # the strip rides along, unchanged + + +def test_content_strip_block_keys_off_page_count_never_mode_alone() -> None: + """Phase 163 (task 01) — the class never appears alone: the whole + strip block keys off ``page_count``, so a row whose column somehow + holds a mode while ``page_count`` is NULL serializes NEITHER + ``page_strip_mode`` nor ``page_urls`` — the payload is byte-identical + to a plain non-qualifying document (there is no strip to show, and + the viewer must never be told a class for pages that do not exist).""" + doc = _page_pdf_doc(page_count=None, page_strip_mode="additive") + with _client_with_row((doc, 1)) as client: + r = client.get( + "/api/documents/content", + params={"source": "Homelab", "path": "ikea/besta.pdf"}, + ) + assert r.status_code == 200 + body = r.json() + assert "page_strip_mode" not in body # absent — the strip block is off + assert "page_urls" not in body # absent — never null + + +def test_content_non_qualifying_payload_key_set_is_byte_identical() -> None: + """Phase 163 (task 01) — the byte-identity pin: a non-qualifying PDF + row (``page_count`` NULL, mode NULL) carries EXACTLY the pre-phase + key set — neither strip key joins it (``is_image`` always present, + ``image_url`` / the origin keys / both strip keys absent).""" + doc = _page_pdf_doc(page_count=None) + with _client_with_row((doc, 1)) as client: + r = client.get( + "/api/documents/content", + params={"source": "Homelab", "path": "ikea/besta.pdf"}, + ) + assert r.status_code == 200 + assert set(r.json()) == { + "source", + "path", + "title", + "format", + "summary", + "created_at", + "content", + "indexed_at", + "chunks", + "is_image", + } def test_content_origin_url_carried_for_a_matching_git_row() -> None: diff --git a/tests/unit/test_importer_unchanged_fast_path.py b/tests/unit/test_importer_unchanged_fast_path.py index 532c5be..1a9fda5 100644 --- a/tests/unit/test_importer_unchanged_fast_path.py +++ b/tests/unit/test_importer_unchanged_fast_path.py @@ -31,6 +31,16 @@ the second sync): * the streamed hash is transparent (a >1 MiB file hashes identically to ``hashlib.sha256(bytes).hexdigest()``). +Phase 163 (task 03) extends this file with the PDF strip RE-GATE the +unchanged paths now run (``_regate_pdf_strip``) — the pre-phase "the +copies persist" assumption is what broke in production (a row claiming +``page_count = 36`` over ZERO files, every page 404ing forever): the +re-gate re-derives the strip from the current bytes + settings (ZERO +vision/LLM calls), heals missing copies, re-stamps a changed class, +clears a strip that no longer qualifies (kill switch included), stays +fail-soft, logs only when it ACTS, and costs a non-strip file zero PDF +parsing (spy-pinned). + The E2E surface (the idempotent second sync through the real UI + mock LLM) is pinned in ``tests/e2e/test_sync_changed_only.py`` (task 03). @@ -43,6 +53,7 @@ import logging import os from datetime import UTC, datetime, timedelta from pathlib import Path +from typing import Any import pymupdf as fitz import pytest @@ -50,6 +61,7 @@ from sqlalchemy import select from app.config import Settings from app.models import Document +from app.rag import importer from app.rag.importer import _file_hash, import_sources from tests.fakes import FakeEmbedder from tests.unit.test_importer import ( @@ -60,6 +72,7 @@ from tests.unit.test_importer import ( _VisionLLM, ) from tests.unit.test_pictorial_import import ( + _build_dense_pdf, _build_ikea_like_pdf, _PictorialLLM, _settings, @@ -454,6 +467,520 @@ def test_pre_phase_null_file_hash_image_runs_branch_and_stamps( _cleanup_source(db, root.name) +# ---------- phase 163 (task 03): the deterministic PDF strip re-gate ---------- +# +# The live defect (owner report, 2026-10-07): the `Letumdei` row carried +# `page_count = 36` over ZERO files on disk (the pre-phase-163 persist +# deleted the old copies BEFORE rendering, outside the DB transaction — +# a later per-file rollback restored the stale count over an empty +# directory; an external wipe of `pdf_page_dir` lands the same state). +# The phase-147 fast path believed "the copies persist" and never +# re-rendered, so the strip 404'd forever. `_regate_pdf_strip` now runs +# at the tail of `_unchanged_doc`: the changed-path gate mirrored from +# the current bytes + settings (ZERO vision/LLM calls), missing copies +# healed, a stale strip cleared. + +#: The PNG signature (a healed copy is a real render, not a placeholder). +PNG_MAGIC = b"\x89PNG\r\n\x1a\n" + + +def _regate_settings(tmp_path: Path, **kwargs: Any) -> Settings: + """Settings for the re-gate matrix: the ``pdf`` token admitted, the + phase-140 knobs at their CODE DEFAULTS (``pdf_pictorial`` ON), and + the strip's home ``pdf_page_dir`` pinned under *tmp_path* (never the + real home).""" + kwargs.setdefault("_env_file", None) + kwargs.setdefault("import_extensions", "md,txt,pdf") + kwargs.setdefault("images", False) + kwargs.setdefault("image_dir", str(tmp_path / "images")) + kwargs.setdefault("pdf_page_dir", str(tmp_path / "pages")) + return Settings(**kwargs) # pyright: ignore[reportCallIssue] + + +def _pages_dir(tmp_path: Path) -> Path: + return tmp_path / "pages" + + +def _strip_copies(doc: Document, page_dir: Path, count: int) -> list[Path]: + return [page_dir / f"{doc.id}-p{n}.png" for n in range(1, count + 1)] + + +def test_regate_heals_deleted_copies_of_a_pictorial_strip( + db, tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """THE live-loss reproduction (heal arm): a pictorial-class PDF is + indexed (``page_count`` set, copies written), its copies are deleted + out from under the row (the exact live state — mode NULL on a + pre-phase row), and an UNCHANGED re-sync restores every file with + real PNG bytes, stamps the mode ``additive``, keeps ``page_count`` + (no drift), burns ZERO llm/vision calls, and logs the one greppable + INFO line. The NULL-mode stamp is SILENT (no ``mode`` segment — it + rides the line only when work followed).""" + root = tmp_path / "rgheal" + root.mkdir() + _build_ikea_like_pdf(root / "ikea.pdf") + llm = _PictorialLLM() + llm.settings = _regate_settings(tmp_path) + try: + asyncio.run(import_sources([root], llm, session=db)) + doc = _doc(db, "rgheal", "ikea.pdf") + assert (doc.page_count, doc.page_strip_mode) == (10, "additive") + page_dir = _pages_dir(tmp_path) + doc.page_strip_mode = None # the pre-phase row (the live shape) + db.commit() + for path in _strip_copies(doc, page_dir, 10): + path.unlink() + assert not list(page_dir.glob(f"{doc.id}-*.png")) + embeds1, chats1, vision1 = len(llm.calls), len(llm.chat_calls), llm.pictorial_calls + + with caplog.at_level(logging.INFO, logger="app.importer"): + second = asyncio.run(import_sources([root], llm, session=db)) + + assert (second.added, second.updated, second.unchanged) == (0, 0, 1) + # ZERO llm work of any kind (the re-gate is deterministic, A5). + assert len(llm.calls) == embeds1, "the re-gate never re-embeds" + assert len(llm.chat_calls) == chats1, "the re-gate never chats" + assert llm.pictorial_calls == vision1, "the re-gate issues ZERO vision calls" + + db.expire_all() + doc2 = _doc(db, "rgheal", "ikea.pdf") + assert doc2.page_count == 10, "no drift — the count is untouched" + assert doc2.page_strip_mode == "additive", "the mode is stamped" + for path in _strip_copies(doc2, page_dir, 10): + assert path.is_file(), f"copy {path.name} was not healed" + assert path.read_bytes()[:8] == PNG_MAGIC, f"{path.name} is not a PNG" + assert not list(page_dir.glob("*.tmp")), "no .tmp orphan from the heal" + messages = [r.getMessage() for r in caplog.records] + assert ( + "import: PDF strip re-gate source=rgheal path=ikea.pdf — " + "healed 10 pages" + ) in messages, messages + assert not any("mode " in m for m in messages if "re-gate" in m), ( + "a NULL->stamp mode re-stamp is silent-beneficial " + str(messages) + ) + finally: + _cleanup_source(db, "rgheal") + + +def test_regate_heals_a_fallback_strip_as_primary( + db, tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """Heal arm, the FALLBACK class (phase 133's unusable text layer): + the copies of a garbage-layer PDF's strip are restored on an + unchanged re-sync and the mode lands ``primary`` (the strip IS the + view) — and the phase-133 fallback vision does NOT re-fire (the + class is re-derived from the bytes, no vision call).""" + root = tmp_path / "rgfall" + root.mkdir() + _build_garbage_pdf(root / "garbage.pdf") # 2 pages + llm = _PageVisionLLM() + llm.settings = _regate_settings(tmp_path) + try: + asyncio.run(import_sources([root], llm, session=db)) + doc = _doc(db, "rgfall", "garbage.pdf") + assert (doc.page_count, doc.page_strip_mode) == (2, "primary") + page_dir = _pages_dir(tmp_path) + for path in _strip_copies(doc, page_dir, 2): + path.unlink() + vision1, chats1, embeds1 = llm.vision_calls, len(llm.chat_calls), len(llm.calls) + + with caplog.at_level(logging.INFO, logger="app.importer"): + second = asyncio.run(import_sources([root], llm, session=db)) + + assert (second.added, second.updated, second.unchanged) == (0, 0, 1) + assert second.pdfs_screenshotted == 0, "the fallback gate did not re-run" + assert llm.vision_calls == vision1, "ZERO vision calls on the re-gate" + assert len(llm.chat_calls) == chats1 and len(llm.calls) == embeds1 + + db.expire_all() + doc2 = _doc(db, "rgfall", "garbage.pdf") + assert doc2.page_strip_mode == "primary", "the unusable layer is primary" + assert doc2.page_count == 2 + for path in _strip_copies(doc2, page_dir, 2): + assert path.is_file() and path.read_bytes()[:8] == PNG_MAGIC + assert any( + "healed 2 pages" in m and "re-gate" in m for m in caplog.messages + ), caplog.messages + finally: + _cleanup_source(db, "rgfall") + + +def test_regate_noop_when_the_strip_is_healthy( + db, tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """No-op arm (the IKEA-class doc whose copies all stand): an + unchanged re-sync writes NOTHING (every copy's mtime and bytes are + untouched), renders NOTHING, and logs NO re-gate line — the INFO is + never per-unchanged-doc — while the pre-existing ``import: + unchanged`` line still fires.""" + root = tmp_path / "rgnoop" + root.mkdir() + _build_ikea_like_pdf(root / "ikea.pdf") + llm = _PictorialLLM() + llm.settings = _regate_settings(tmp_path) + try: + asyncio.run(import_sources([root], llm, session=db)) + doc = _doc(db, "rgnoop", "ikea.pdf") + page_dir = _pages_dir(tmp_path) + before = { + p: (p.stat().st_mtime_ns, p.read_bytes()) + for p in _strip_copies(doc, page_dir, 10) + } + assert len(before) == 10 + + with caplog.at_level(logging.INFO, logger="app.importer"): + second = asyncio.run(import_sources([root], llm, session=db)) + + assert (second.added, second.updated, second.unchanged) == (0, 0, 1) + db.expire_all() + doc2 = _doc(db, "rgnoop", "ikea.pdf") + assert (doc2.page_count, doc2.page_strip_mode) == (10, "additive") + after = { + p: (p.stat().st_mtime_ns, p.read_bytes()) + for p in _strip_copies(doc2, page_dir, 10) + } + assert after == before, "a healthy strip is not written at all" + messages = [r.getMessage() for r in caplog.records] + assert any(m.startswith("import: unchanged ") for m in messages), messages + assert not any("re-gate" in m for m in messages), ( + f"the re-gate logged without acting: {messages}" + ) + finally: + _cleanup_source(db, "rgnoop") + + +def test_regate_restamps_a_changed_mode( + db, tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """Mode re-stamp (the class flip): a row whose stamped mode no + longer matches the gate's class (here a pictorial-class row carrying + ``primary`` — a legacy/mis-stamped row) is re-stamped to + ``additive``, and because the mode was ALREADY SET the change is + reported (``mode primary->additive``) — the only thing the re-gate + did, so that is the whole INFO line (no copies were missing, none + were written).""" + root = tmp_path / "rgmode" + root.mkdir() + _build_ikea_like_pdf(root / "ikea.pdf") + llm = _PictorialLLM() + llm.settings = _regate_settings(tmp_path) + try: + asyncio.run(import_sources([root], llm, session=db)) + doc = _doc(db, "rgmode", "ikea.pdf") + assert doc.page_strip_mode == "additive" + doc.page_strip_mode = "primary" # the mis-stamp the re-gate fixes + db.commit() + page_dir = _pages_dir(tmp_path) + before = { + p: p.stat().st_mtime_ns for p in _strip_copies(doc, page_dir, 10) + } + + with caplog.at_level(logging.INFO, logger="app.importer"): + second = asyncio.run(import_sources([root], llm, session=db)) + + assert (second.added, second.unchanged) == (0, 1) + db.expire_all() + doc2 = _doc(db, "rgmode", "ikea.pdf") + assert doc2.page_strip_mode == "additive", "the class is re-stamped" + assert doc2.page_count == 10 + assert { + p: p.stat().st_mtime_ns for p in _strip_copies(doc2, page_dir, 10) + } == before, "a mode fix writes no files" + assert ( + "import: PDF strip re-gate source=rgmode path=ikea.pdf — " + "mode primary->additive" + ) in [r.getMessage() for r in caplog.records], caplog.messages + finally: + _cleanup_source(db, "rgmode") + + +def test_regate_clears_a_strip_that_no_longer_qualifies( + db, tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """REPAIR arm: a row claims a strip (``page_count`` set, copies on + disk) on a PDF whose GATE now says ineligible (a clean, dense text + layer — zero sparse pages). The unchanged re-sync clears BOTH + columns and deletes the copies (the strip must not linger over a + document the viewer shows as text) + the ``cleared stale strip`` + INFO line, with zero llm calls.""" + root = tmp_path / "rgclear" + root.mkdir() + pdf = root / "clean.pdf" + _build_dense_pdf(pdf) # usable layer, zero candidates — never qualifies + llm = _PictorialLLM() + llm.settings = _regate_settings(tmp_path) + try: + asyncio.run(import_sources([root], llm, session=db)) + doc = _doc(db, "rgclear", "clean.pdf") + assert (doc.page_count, doc.page_strip_mode) == (None, None) + # The stale state the re-gate must repair (a strip stamped by an + # older gate/settings, copies still on disk). + page_dir = _pages_dir(tmp_path) + page_dir.mkdir(parents=True, exist_ok=True) + for path in _strip_copies(doc, page_dir, 2): + path.write_bytes(b"STALE-COPY") + doc.page_count = 2 + doc.page_strip_mode = "additive" + db.commit() + embeds1, chats1 = len(llm.calls), len(llm.chat_calls) + + with caplog.at_level(logging.INFO, logger="app.importer"): + second = asyncio.run(import_sources([root], llm, session=db)) + + assert (second.added, second.updated, second.unchanged) == (0, 0, 1) + assert len(llm.calls) == embeds1 and len(llm.chat_calls) == chats1 + + db.expire_all() + doc2 = _doc(db, "rgclear", "clean.pdf") + assert doc2.page_count is None, "the stale strip is cleared" + assert doc2.page_strip_mode is None, "the class is cleared with it" + assert not list(page_dir.glob(f"{doc2.id}-p*.png")), "the copies are deleted" + assert ( + "import: PDF strip re-gate source=rgclear path=clean.pdf — " + "cleared stale strip" + ) in [r.getMessage() for r in caplog.records], caplog.messages + finally: + _cleanup_source(db, "rgclear") + + +def test_regate_kill_switch_clears_the_pictorial_strip( + db, tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """REPAIR arm via the kill switch (``pdf_pictorial=False``): the + changed-path semantics mirrored — with the pass off a sparse-page + PDF is NOT eligible, so its existing strip is cleared with its + copies on the next (unchanged) sync. The bytes never moved: the + setting alone decided.""" + root = tmp_path / "rgkill" + root.mkdir() + _build_ikea_like_pdf(root / "ikea.pdf") + llm = _PictorialLLM() + llm.settings = _regate_settings(tmp_path) + try: + asyncio.run(import_sources([root], llm, session=db)) + doc = _doc(db, "rgkill", "ikea.pdf") + assert (doc.page_count, doc.page_strip_mode) == (10, "additive") + page_dir = _pages_dir(tmp_path) + assert len(list(page_dir.glob(f"{doc.id}-p*.png"))) == 10 + vision1 = llm.pictorial_calls + + llm.settings = _regate_settings(tmp_path, pdf_pictorial=False) + with caplog.at_level(logging.INFO, logger="app.importer"): + second = asyncio.run(import_sources([root], llm, session=db)) + + assert (second.added, second.updated, second.unchanged) == (0, 0, 1) + assert llm.pictorial_calls == vision1, "ZERO vision calls (the pass is off)" + db.expire_all() + doc2 = _doc(db, "rgkill", "ikea.pdf") + assert doc2.page_count is None and doc2.page_strip_mode is None + assert not list(page_dir.glob(f"{doc2.id}-p*.png")), "the copies went too" + assert any( + "cleared stale strip" in m for m in caplog.messages + ), caplog.messages + finally: + _cleanup_source(db, "rgkill") + + +def test_regate_render_exception_is_fail_soft( + db, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """Fail-soft: the heal's render RAISES — the sync completes + (``unchanged``, zero errors), the row keeps its prior state + (``page_count`` + mode untouched — the re-gate never clears a strip + on a failure), no copies are touched, and the WARNING is the only + signal.""" + root = tmp_path / "rgboom" + root.mkdir() + _build_ikea_like_pdf(root / "ikea.pdf") + llm = _PictorialLLM() + llm.settings = _regate_settings(tmp_path) + try: + asyncio.run(import_sources([root], llm, session=db)) + doc = _doc(db, "rgboom", "ikea.pdf") + page_dir = _pages_dir(tmp_path) + for path in _strip_copies(doc, page_dir, 10): + path.unlink() + + def boom(*args: object, **kwargs: object) -> list[bytes]: + raise RuntimeError("simulated render failure (test sentinel)") + + monkeypatch.setattr(importer, "render_pdf_pages", boom) + with caplog.at_level(logging.WARNING, logger="app.importer"): + second = asyncio.run(import_sources([root], llm, session=db)) + + assert (second.added, second.updated, second.unchanged, second.errors) == ( + 0, 0, 1, 0, + ) + db.expire_all() + doc2 = _doc(db, "rgboom", "ikea.pdf") + assert (doc2.page_count, doc2.page_strip_mode) == (10, "additive"), ( + "the row keeps its pre-re-gate state" + ) + assert not list(page_dir.glob(f"{doc.id}-p*.png")), "no copies were touched" + assert any( + "PDF strip re-gate failed" in m and "test sentinel" in m + for m in caplog.messages + ), caplog.messages + finally: + _cleanup_source(db, "rgboom") + + +def test_regate_empty_render_keeps_the_row( + db, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """Fail-soft on the no-pages render (unopenable right after the fast + path hashed the file — pathological): the row is kept EXACTLY as it + was (never cleared on a transient), nothing is written, and the + WARNING names the no-pages state.""" + root = tmp_path / "rgempty" + root.mkdir() + _build_ikea_like_pdf(root / "ikea.pdf") + llm = _PictorialLLM() + llm.settings = _regate_settings(tmp_path) + try: + asyncio.run(import_sources([root], llm, session=db)) + doc = _doc(db, "rgempty", "ikea.pdf") + page_dir = _pages_dir(tmp_path) + for path in _strip_copies(doc, page_dir, 10): + path.unlink() + monkeypatch.setattr(importer, "render_pdf_pages", lambda *a, **k: []) + + with caplog.at_level(logging.WARNING, logger="app.importer"): + second = asyncio.run(import_sources([root], llm, session=db)) + + assert (second.added, second.unchanged, second.errors) == (0, 1, 0) + db.expire_all() + doc2 = _doc(db, "rgempty", "ikea.pdf") + assert (doc2.page_count, doc2.page_strip_mode) == (10, "additive") + assert not page_dir.exists() or not list(page_dir.glob(f"{doc.id}-*")) + assert any( + "re-render produced no pages (state kept)" in m + for m in caplog.messages + ), caplog.messages + finally: + _cleanup_source(db, "rgempty") + + +def test_regate_reports_a_page_count_drift_on_the_heal( + db, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """Drift (pathological on unchanged bytes, so it is LOUD): the heal + renders 6 pages for a row that claimed 10 — ``page_count`` follows + the fresh render, the ``p7..p10`` tail beyond it is deleted, and a + WARNING names both counts beside the ``healed`` INFO line.""" + root = tmp_path / "rgdrift" + root.mkdir() + _build_ikea_like_pdf(root / "ikea.pdf") + llm = _PictorialLLM() + llm.settings = _regate_settings(tmp_path) + try: + asyncio.run(import_sources([root], llm, session=db)) + doc = _doc(db, "rgdrift", "ikea.pdf") + page_dir = _pages_dir(tmp_path) + for path in _strip_copies(doc, page_dir, 10): + path.unlink() + monkeypatch.setattr( + importer, + "render_pdf_pages", + lambda *a, **k: [PNG_MAGIC + f"{i}".encode() for i in range(6)], + ) + + with caplog.at_level(logging.INFO, logger="app.importer"): + second = asyncio.run(import_sources([root], llm, session=db)) + + assert (second.added, second.unchanged) == (0, 1) + db.expire_all() + doc2 = _doc(db, "rgdrift", "ikea.pdf") + assert doc2.page_count == 6, "the count follows the fresh render" + for n in range(1, 7): + assert (page_dir / f"{doc2.id}-p{n}.png").is_file() + for n in range(7, 11): + assert not (page_dir / f"{doc2.id}-p{n}.png").exists(), f"p{n} leaked" + messages = [r.getMessage() for r in caplog.records] + assert any("page_count drift 10->6" in m for m in messages), messages + assert any("healed 10 pages" in m for m in messages), messages + finally: + monkeypatch.undo() # pyright: ignore[reportUnknownMemberType] + _cleanup_source(db, "rgdrift") + + +def test_regate_fires_on_the_content_digest_hit_path( + db, tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """The SECOND unchanged path (the PDF branch's content-digest hit — + a pre-phase row with ``file_hash`` NULL): the branch re-runs its own + gate (the byte-change's honest cost) and lands unchanged, and the + shared helper's tail still heals the strip deleted out from under + the row — the re-gate is not fast-path-only.""" + root = tmp_path / "rgbranch" + root.mkdir() + _build_ikea_like_pdf(root / "ikea.pdf") + llm = _PictorialLLM() + llm.settings = _regate_settings(tmp_path) + try: + asyncio.run(import_sources([root], llm, session=db)) + doc = _doc(db, "rgbranch", "ikea.pdf") + doc.file_hash = None # the pre-phase row: the fast path cannot fire + db.commit() + page_dir = _pages_dir(tmp_path) + for path in _strip_copies(doc, page_dir, 10): + path.unlink() + + with caplog.at_level(logging.INFO, logger="app.importer"): + second = asyncio.run(import_sources([root], llm, session=db)) + + assert (second.added, second.updated, second.unchanged) == (0, 0, 1) + db.expire_all() + doc2 = _doc(db, "rgbranch", "ikea.pdf") + assert doc2.file_hash is not None, "the helper stamped the hash" + for path in _strip_copies(doc2, page_dir, 10): + assert path.is_file(), f"copy {path.name} was not healed" + assert any("healed 10 pages" in m for m in caplog.messages), caplog.messages + finally: + _cleanup_source(db, "rgbranch") + + +def test_regate_guard_costs_a_non_strip_file_nothing( + db, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The GUARD (A5): a non-PDF unchanged file and a PDF whose + ``page_count`` is NULL reach the tail of ``_unchanged_doc`` and + ``extract_pdf_text`` is NEVER called for either — every non-strip + file's fast path keeps its zero-extra-read contract (one sync of + each class, then the spy covers the second sync only).""" + root = tmp_path / "rgguard" + root.mkdir() + (root / "note.md").write_text("# Note\n\nplain text body\n") + _build_dense_pdf(root / "clean.pdf") # usable layer, no strip + llm = _PictorialLLM() + llm.settings = _regate_settings(tmp_path) + seen: list[str] = [] + real_extract = importer.extract_pdf_text + + def spy(path: Path) -> str: + seen.append(str(path)) + return real_extract(path) + + try: + asyncio.run(import_sources([root], llm, session=db)) + monkeypatch.setattr(importer, "extract_pdf_text", spy) + second = asyncio.run(import_sources([root], llm, session=db)) + assert (second.added, second.updated, second.unchanged) == (0, 0, 2) + assert seen == [], f"the guard let a PDF parse through: {seen}" + finally: + monkeypatch.undo() # pyright: ignore[reportUnknownMemberType] + _cleanup_source(db, "rgguard") + + # ---------- the preserved semantics on the fast path -------------------------- diff --git a/tests/unit/test_models.py b/tests/unit/test_models.py index 9ca8591..aceec1d 100644 --- a/tests/unit/test_models.py +++ b/tests/unit/test_models.py @@ -293,6 +293,82 @@ def test_git_source_python_default_include_hidden_false() -> None: assert col.default.arg is False, "the default must resolve to False" +def test_documents_page_strip_mode_column_contract() -> None: + """Phase 163 (task 01): ``documents.page_strip_mode`` — the CLASS of + the page strip a non-NULL ``page_count`` describes (``"primary"`` = + the phase-133 fallback class, the strip IS the view; ``"additive"`` + = the phase-140 pictorial class, text + a Text/Pages toggle): + VARCHAR(16), NULLABLE (NULL = no strip — and the pre-phase shape for + a row whose strip stands), NO server default, and NO index (the + content endpoint reads it from the ROW — a display fact, never a + query surface, the ``file_hash`` / ``page_count`` convention). + ``page_count`` (0026) stays exactly as phase 148 left it.""" + from sqlalchemy import String as SAString + + documents = Base.metadata.tables["documents"] + assert "page_strip_mode" in documents.c, ( + "documents must have the page_strip_mode column (phase 163)" + ) + col = documents.c["page_strip_mode"] + assert col.nullable is True, ( + "page_strip_mode must be NULLABLE — NULL means no strip (and the" + " pre-phase row whose strip stands)" + ) + assert isinstance(col.type, SAString), "page_strip_mode must be a String column" + assert col.type.length == 16, ( + "page_strip_mode must be String(16) — the two class names, bounded" + ) + assert col.server_default is None, ( + "page_strip_mode must carry NO server default — NULL is the" + " meaningful pre-phase state" + ) + assert not col.index, ( + "page_strip_mode must be UNINDEXED — the content endpoint reads it" + " from the row, no query filters on it" + ) + covering = [ + idx + for idx in documents.indexes + if any(c.name == "page_strip_mode" for c in idx.columns) + ] + assert not covering, "no table-level index may cover page_strip_mode" + + # page_count is untouched: still the nullable, unindexed sibling. + page_count = documents.c["page_count"] + assert page_count.nullable is True, "page_count must stay NULLABLE (0026)" + assert not page_count.index, "page_count must stay UNINDEXED (0026)" + + +def test_document_page_strip_mode_orm_defaults_to_none() -> None: + """A freshly constructed ``Document`` (no DB) exposes + ``page_strip_mode``: an omitted kwarg and an explicit ``None`` both + read NULL (no strip — the shape every non-qualifying document keeps, + and the shape a pre-phase-163 row keeps WHILE its ``page_count`` + stands, which the wire serializes ABSENT), and each class name + round-trips verbatim as a plain string (the real DB round-trip is + pinned in ``test_migration_0027.py``).""" + from app.models import Document + + base = dict( + source="s", + path="p.pdf", + full_path="/tmp/s/p.pdf", + title="p", + content="c", + content_hash="1" * 64, + page_count=36, + ) + for mode in ("primary", "additive"): + assert Document(**base, page_strip_mode=mode).page_strip_mode == mode + + assert Document(**base, page_strip_mode=None).page_strip_mode is None + omitted = Document(**base) + assert omitted.page_strip_mode is None, ( + "an omitted page_strip_mode must read NULL while page_count stands" + " (the pre-phase strip the viewer keeps showing as 'primary')" + ) + + def test_doc_drafts_column_contract() -> None: """Phase 59: the editable triple (title/path/body) + status + timestamps are NOT NULL; ``branch`` / ``commit_sha`` are NULL diff --git a/tests/unit/test_pdf_page_copies.py b/tests/unit/test_pdf_page_copies.py index 8cdeff8..4717acf 100644 --- a/tests/unit/test_pdf_page_copies.py +++ b/tests/unit/test_pdf_page_copies.py @@ -21,14 +21,20 @@ builder for the negative; the copies land in a tmp ``pdf_page_dir``): layer, ≥1 sparse page) and the fallback class (unusable layer) get ``<doc-id>-p{N}.png`` for EVERY page (not just the described candidates — the strip shows the whole manual) + ``page_count`` = - the page count + real PNG bytes; a text-heavy PDF gets neither - (no copies, NULL count, ZERO render calls, byte-identical content); + the page count + real PNG bytes + the phase-163 CLASS stamp + (``page_strip_mode`` ``additive`` / ``primary``); a text-heavy PDF + gets neither (no copies, NULL count, NULL mode, ZERO render calls, + byte-identical content); * the phase-147 fast path — an unchanged qualifying PDF's second sync re-renders NOTHING (zero render calls — the copies persist, ``page_count`` and the bytes untouched); -* stale replacement — a changed (shrunk) PDF's old copy set is - replaced: the new set covers the new pages, the old ``p{N}`` tail - is GONE (no orphans), ``page_count`` follows the new count; +* WRITE-FIRST replacement (phase 163, task 02, LOCKED A4) — a changed + (shrunk) PDF's new copies land atomically (tmp + ``os.replace``, + pinned) BEFORE any old copy is removed; the old ``p{N}`` tail is + GONE only after the replacement bytes are durable (no orphans), + ``page_count`` follows the new count; a MID-WRITE failure leaves the + prior copy set intact (the rollback-orphan state — the row over + zero files — is structurally unrepresentable); * qualifier loss — a qualifying PDF whose new version gained a dense text layer loses its strip: ``page_count`` NULL + copies deleted; * unopenable — a render that yields no pages (``render_pdf_pages`` → @@ -40,9 +46,10 @@ builder for the negative; the copies land in a tmp ``pdf_page_dir``): the FULL count (the gap is a display-level 404 — task 02 pins the route's side); * fail-soft — the render raising (warning is the signal, ``page_count`` - NULL, no copies, the doc indexed fully — chunks + summary) and the - WRITER raising (the copies this call wrote are removed — no partial - orphans, ``page_count`` NULL, the doc indexed fully); + + ``page_strip_mode`` NULL, no copies, the doc indexed fully — chunks + + summary) and the WRITER raising (this call's ``.tmp`` files and the + finals it CREATED are removed — no partial orphans, ``page_count`` + + ``page_strip_mode`` NULL, the doc indexed fully); * prune — a qualifying doc's file removed from the source: the row goes and its page copies go with it (no orphans in the tmp dir). @@ -54,6 +61,7 @@ from __future__ import annotations import asyncio import logging +import os import uuid from collections.abc import Callable, Sequence from pathlib import Path @@ -161,6 +169,35 @@ def _render_spy( return calls +# --------------------------------------------------------------------------- +# Phase 163 (task 01) — the strip-class column beside page_count +# --------------------------------------------------------------------------- + + +def test_pictorial_index_stamps_the_additive_class(db, tmp_path: Path) -> None: + """Phase 163 (task 02, re-cut of the task-01 pin) — the STAMP: a + qualifying PDF indexed through the PICTORIAL class gets + ``page_strip_mode == "additive"`` beside ``page_count`` (the text + + toggle view); the strip's copies themselves are unchanged by the + phase (task 01's shape, mode stamped exactly with the count). + """ + root = tmp_path / "pgm" + root.mkdir() + _build_ikea_like_pdf(root / "ikea.pdf") + llm = _PictorialLLM() + llm.settings = _settings(tmp_path) + try: + summary = asyncio.run(import_sources([root], llm, session=db)) + assert (summary.files, summary.added, summary.errors) == (1, 1, 0) + doc = _doc(db, "pgm", "ikea.pdf") + assert doc.page_count == 10, "the strip itself is unchanged by task 02" + assert doc.page_strip_mode == "additive", ( + "the pictorial class stamps the additive mode (LOCKED A1/A3)" + ) + finally: + _cleanup_source(db, "pgm") + + # --------------------------------------------------------------------------- # Eligibility × new index # --------------------------------------------------------------------------- @@ -219,6 +256,10 @@ def test_fallback_class_garbage_pdf_writes_every_page_copy(db, tmp_path: Path) - assert summary.pdfs_screenshotted == 1 # the fallback fired (the class) doc = _doc(db, "pgf", "garbage.pdf") assert doc.page_count == 2 + assert doc.page_strip_mode == "primary", ( + "the fallback class stamps the primary mode — the strip IS the " + "view, byte-identical to phase 148 otherwise (LOCKED A1)" + ) page_dir = _pages_dir(tmp_path) for n in (1, 2): path = page_dir / f"{doc.id}-p{n}.png" @@ -253,6 +294,7 @@ def test_text_heavy_pdf_keeps_the_text_view( assert summary.pdfs_pictorial == 0 # zero candidates — the pass no-ops doc = _doc(db, "pgd", "clean.pdf") assert doc.page_count is None, "a text-heavy PDF keeps the text view" + assert doc.page_strip_mode is None, "no strip — no class either" assert not _pages_dir(tmp_path).exists(), "no copies — the dir is never created" assert calls == [], "zero render calls (no fallback, zero candidates)" assert doc.content == expected, "byte-identical to the extraction" @@ -315,9 +357,10 @@ def test_changed_pdf_replaces_the_copy_set_without_orphans( ) -> None: """A CHANGED (shrunk, 10 → 6 pages) qualifying PDF: the re-index replaces the copy set — the new set covers p1..p6, the old - version's ``p7..p10`` tail is GONE (the stale-replacement runs - before the write), the doc keeps its id, and ``page_count`` - follows the new page count.""" + version's ``p7..p10`` tail is GONE (the stale-tail delete runs + LAST, after the write-first loop — phase 163, task 02), the doc + keeps its id, ``page_count`` follows the new page count, and + ``page_strip_mode`` is (re-)stamped.""" root = tmp_path / "pgc" root.mkdir() pdf = root / "ikea.pdf" @@ -336,6 +379,7 @@ def test_changed_pdf_replaces_the_copy_set_without_orphans( doc2 = _doc(db, "pgc", "ikea.pdf") assert doc2.id == doc1.id, "the doc keeps its id (the copy path is stable)" assert doc2.page_count == 6, "page_count follows the new page count" + assert doc2.page_strip_mode == "additive", "the class is (re-)stamped" page_dir = _pages_dir(tmp_path) for n in range(1, 7): assert (page_dir / f"{doc2.id}-p{n}.png").is_file(), f"p{n} is missing" @@ -379,6 +423,7 @@ def test_qualifier_loss_clears_page_count_and_copies( doc2 = _doc(db, "pgl", "ikea.pdf") assert doc2.id == doc1.id assert doc2.page_count is None, "a doc that stopped qualifying loses its strip" + assert doc2.page_strip_mode is None, "the class is cleared with the count" assert not list(_pages_dir(tmp_path).glob(f"{doc2.id}-p*.png")), ( "the copies must be deleted on qualifier loss" ) @@ -411,6 +456,7 @@ def test_unopenable_pdf_page_render_is_fail_soft( assert (summary.files, summary.added, summary.errors) == (1, 1, 0) doc = _doc(db, "pgn", "garbage.pdf") assert doc.page_count is None, "no pages rendered — no strip" + assert doc.page_strip_mode is None, "the class is cleared with the count" assert not _pages_dir(tmp_path).exists(), "no copies — the dir is never created" assert doc.content == "", "the fallback's all-failed render lands empty content" assert summary.summaries == 1, "the document is indexed exactly as pre-phase" @@ -493,10 +539,11 @@ def test_write_exception_is_fail_soft_and_leaves_no_orphans( monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture, ) -> None: - """The PNG WRITER raises (a full disk) after the FIRST copy - landed: the copies this call wrote are removed (no partial - orphans), ``page_count`` NULL, the warning is the signal, and the - document indexes exactly as pre-phase.""" + """The PNG WRITER raises (a full disk) after the FIRST atomic copy + landed: this call's ``.tmp`` files and the final-name files it + CREATED are removed (no partial orphans — phase 163, task 02), + ``page_count`` AND ``page_strip_mode`` NULL, the warning is the + signal, and the document indexes exactly as pre-phase.""" root = tmp_path / "pgw" root.mkdir() _build_ikea_like_pdf(root / "ikea.pdf") @@ -507,22 +554,29 @@ def test_write_exception_is_fail_soft_and_leaves_no_orphans( def boom(self: Path, data: bytes) -> int: written_paths.append(self) + n = original_write(self, data) # the tmp file really lands first if len(written_paths) > 1: raise OSError("simulated disk full (test sentinel)") - return original_write(self, data) + return n monkeypatch.setattr(Path, "write_bytes", boom) # pyright: ignore[reportArgumentType] try: with caplog.at_level(logging.WARNING, logger="app.importer"): summary = asyncio.run(import_sources([root], llm, session=db)) assert (summary.files, summary.added, summary.errors) == (1, 1, 0) - assert len(written_paths) == 2, "p1 landed, p2's write failed" + assert len(written_paths) == 2, "p1's tmp landed + replaced, p2's write failed" + assert all(p.name.endswith(".png.tmp") for p in written_paths), ( + "the writer only ever touches .tmp paths (atomic replace)" + ) doc = _doc(db, "pgw", "ikea.pdf") assert doc.page_count is None + assert doc.page_strip_mode is None, "the class is cleared with the count" assert not list(_pages_dir(tmp_path).glob(f"{doc.id}-p*.png")), ( "the partial write was cleaned up — no orphans" ) - assert not written_paths[0].exists(), "the landed p1 copy was removed" + assert not list(_pages_dir(tmp_path).glob("*.tmp")), ( + "the failed call's .tmp file was cleaned up too" + ) assert summary.summaries == 1, "the document is indexed exactly as pre-phase" assert any("PDF page render failed" in m for m in caplog.messages), ( caplog.messages @@ -531,6 +585,246 @@ def test_write_exception_is_fail_soft_and_leaves_no_orphans( _cleanup_source(db, "pgw") +# --------------------------------------------------------------------------- +# Phase 163 (task 02) — WRITE-FIRST, atomic per file (LOCKED A4) +# --------------------------------------------------------------------------- + + +def test_write_first_new_bytes_are_durable_before_any_stale_delete( + db, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """THE order pin (LOCKED A4 — the 2026-10-07 live loss): re-rendering + a CHANGED (shrunk, 10 → 6) PDF writes every replacement page FIRST + (tmp + ``os.replace``) and only then deletes the stale tail — the + event log shows ALL replaces of the fresh bytes before the FIRST + unlink of an old-extent file, the overwritten pages keep the NEW + bytes, the shrunk tail (p7..p10) is gone, and nothing else is + deleted.""" + root = tmp_path / "pgord" + root.mkdir() + pdf = root / "ikea.pdf" + _build_ikea_like_pdf(pdf) + llm = _PictorialLLM() + llm.settings = _settings(tmp_path) + asyncio.run(import_sources([root], llm, session=db)) + doc1 = _doc(db, "pgord", "ikea.pdf") + page_dir = _pages_dir(tmp_path) + sentinel = b"OLD-COPY-SENTINEL" + for n in range(1, 11): # make the old copies unmistakably distinguishable + (page_dir / f"{doc1.id}-p{n}.png").write_bytes(sentinel) + + events: list[tuple[str, str]] = [] + real_replace = os.replace + + def replace_spy(src, dst, *args, **kwargs): # pyright: ignore[reportUnknownParameterType] + events.append(("replace", Path(os.fspath(dst)).name)) + return real_replace(src, dst, *args, **kwargs) + + real_unlink = Path.unlink + + def unlink_spy(self: Path, **kwargs: Any) -> None: + if self.parent == page_dir and self.name.startswith(str(doc1.id)): + events.append(("unlink", self.name)) + return real_unlink(self, **kwargs) + + monkeypatch.setattr(importer.os, "replace", replace_spy) + monkeypatch.setattr(Path, "unlink", unlink_spy) # pyright: ignore[reportArgumentType] + try: + _build_ikea_shrunk(pdf) # same path, 6 pages — still qualifying + s2 = asyncio.run(import_sources([root], llm, session=db)) + assert (s2.added, s2.updated, s2.unchanged) == (0, 1, 0) + finally: + monkeypatch.undo() # pyright: ignore[reportUnknownMemberType] + _cleanup_source(db, "pgord") + replaces = [name for kind, name in events if kind == "replace"] + unlinks = [name for kind, name in events if kind == "unlink"] + assert len(replaces) == 6, "the 6 fresh pages are written via os.replace" + assert set(unlinks) == {f"{doc1.id}-p{n}.png" for n in range(7, 11)}, ( + "the stale-tail delete touches ONLY the old extent the render did " + "not rewrite — a file about to be overwritten is never deleted" + ) + first_unlink = next(i for i, e in enumerate(events) if e[0] == "unlink") + last_replace = max(i for i, e in enumerate(events) if e[0] == "replace") + assert last_replace < first_unlink, ( + f"write-first violated — event order: {events}" + ) + for n in range(1, 7): + blob = (page_dir / f"{doc1.id}-p{n}.png").read_bytes() + assert blob != sentinel and blob[:8] == PNG_MAGIC, ( + f"p{n} keeps the NEW rendered bytes" + ) + for n in range(7, 11): + assert not (page_dir / f"{doc1.id}-p{n}.png").exists(), f"stale p{n} leaked" + assert not list(page_dir.glob("*.tmp")), "no .tmp orphans" + + +def test_mid_write_failure_keeps_the_prior_copies_intact( + db, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """THE failure pin (LOCKED A4): the writer raises on the SECOND + page's tmp write of a changed re-render — the prior copy set stays + on disk INTACT (the rollback-orphan state — the row over zero + files — is structurally unrepresentable): every one of the 10 prior + files still exists (p1 with the new bytes already atomically + replaced before the failure, p2..p10 byte-for-byte the prior + copies), zero ``.tmp`` orphans from this call, and the caller's + fail-soft fires (``page_count``/``page_strip_mode`` NULL + the + warning, the doc indexed fully).""" + root = tmp_path / "pgfail" + root.mkdir() + pdf = root / "ikea.pdf" + _build_ikea_like_pdf(pdf) + llm = _PictorialLLM() + llm.settings = _settings(tmp_path) + asyncio.run(import_sources([root], llm, session=db)) + doc1 = _doc(db, "pgfail", "ikea.pdf") + page_dir = _pages_dir(tmp_path) + prior = { + n: (page_dir / f"{doc1.id}-p{n}.png").read_bytes() for n in range(1, 11) + } + # Tag the re-render's bytes so the new p1 is recognizable, then + # blow up the SECOND tmp write (page 2). + _render_spy( + monkeypatch, + mutate_full=lambda pages: [ + PNG_MAGIC + f"NEW-{i}".encode() for i in range(len(pages)) + ], + ) + original_write = Path.write_bytes + tmp_writes: list[Path] = [] + + def boom(self: Path, data: bytes) -> int: + if self.suffix == ".tmp": + tmp_writes.append(self) + if len(tmp_writes) > 1: + raise OSError("simulated disk full (test sentinel)") + return original_write(self, data) + + monkeypatch.setattr(Path, "write_bytes", boom) # pyright: ignore[reportArgumentType] + _build_ikea_shrunk(pdf) + try: + with caplog.at_level(logging.WARNING, logger="app.importer"): + s2 = asyncio.run(import_sources([root], llm, session=db)) + finally: + monkeypatch.undo() # pyright: ignore[reportUnknownMemberType] + assert (s2.added, s2.updated, s2.unchanged) == (0, 1, 0) + assert len(tmp_writes) == 2, "p1's tmp wrote, p2's tmp write raised" + doc2 = _doc(db, "pgfail", "ikea.pdf") + assert doc2.page_count is None, "the caller's fail-soft nulled the count" + assert doc2.page_strip_mode is None, "the caller's fail-soft nulled the class" + assert any("PDF page render failed" in m for m in caplog.messages), caplog.messages + # The prior set is INTACT: exactly the 10 names, none lost, none + # orphaned on top. + assert {p.name for p in page_dir.iterdir()} == { + f"{doc1.id}-p{n}.png" for n in range(1, 11) + }, "files on disk exactly the prior set" + assert ( + page_dir / f"{doc1.id}-p1.png" + ).read_bytes() == PNG_MAGIC + b"NEW-0", ( + "p1's replacement bytes were durable before the failure (atomic " + "replace) and were NOT removed by the cleanup" + ) + for n in range(2, 11): + assert ( + page_dir / f"{doc1.id}-p{n}.png" + ).read_bytes() == prior[n], f"prior copy p{n} was disturbed" + assert not list(page_dir.glob("*.tmp")), "zero .tmp orphans from this call" + assert doc2.chunks, "the document is indexed fully (fail-soft)" + + +def test_page_copies_are_written_atomically_tmp_then_replace( + db, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """THE atomic pin (LOCKED A4): the writer NEVER writes a final + ``<doc-id>-p{N}.png`` path directly — every page's bytes land on + ``<name>.png.tmp`` and ``os.replace`` moves them onto the final + name in order (a reader opening the final path never sees truncated + bytes), and no ``.tmp`` survives the successful call.""" + root = tmp_path / "pgatom" + root.mkdir() + _build_ikea_like_pdf(root / "ikea.pdf") + llm = _PictorialLLM() + llm.settings = _settings(tmp_path) + written: list[Path] = [] + original_write = Path.write_bytes + + def write_spy(self: Path, data: bytes) -> int: + written.append(self) + return original_write(self, data) + + replaced: list[tuple[str, str]] = [] + real_replace = os.replace + + def replace_spy(src, dst, *args, **kwargs): # pyright: ignore[reportUnknownParameterType] + replaced.append((os.fspath(src), os.fspath(dst))) + return real_replace(src, dst, *args, **kwargs) + + monkeypatch.setattr(Path, "write_bytes", write_spy) # pyright: ignore[reportArgumentType] + monkeypatch.setattr(importer.os, "replace", replace_spy) + try: + asyncio.run(import_sources([root], llm, session=db)) + finally: + monkeypatch.undo() # pyright: ignore[reportUnknownMemberType] + doc = _doc(db, "pgatom", "ikea.pdf") + page_dir = _pages_dir(tmp_path) + assert len(written) == 10 and len(replaced) == 10 + assert all(p.name.endswith(".png.tmp") for p in written), ( + "no final path is ever written directly" + ) + for n, (src, dst) in enumerate(replaced, start=1): + assert dst == str(page_dir / f"{doc.id}-p{n}.png") + assert src == f"{dst}.tmp", "the tmp name is <final>.tmp" + assert not list(page_dir.glob("*.tmp")), "no .tmp survives the success path" + + +def test_stale_tail_delete_failure_is_best_effort( + db, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """An unreadable STALE-TAIL copy (the write-first tail pass, the + ``_delete_page_copies`` convention): the failure is logged, never + raised — the sync completes, the row is stamped with the fresh + count, and the reachable tail files still go.""" + root = tmp_path / "pgtail" + root.mkdir() + pdf = root / "ikea.pdf" + _build_ikea_like_pdf(pdf) + llm = _PictorialLLM() + llm.settings = _settings(tmp_path) + asyncio.run(import_sources([root], llm, session=db)) + doc1 = _doc(db, "pgtail", "ikea.pdf") + page_dir = _pages_dir(tmp_path) + p10 = page_dir / f"{doc1.id}-p10.png" + real_unlink = Path.unlink + + def boom(self: Path, **kwargs: Any) -> None: + if self == p10: + raise OSError("simulated unreadable file (test sentinel)") + return real_unlink(self, **kwargs) + + monkeypatch.setattr(Path, "unlink", boom) # pyright: ignore[reportArgumentType] + _build_ikea_shrunk(pdf) + try: + with caplog.at_level(logging.WARNING, logger="app.importer"): + s2 = asyncio.run(import_sources([root], llm, session=db)) + finally: + monkeypatch.undo() # pyright: ignore[reportUnknownMemberType] + assert (s2.added, s2.updated, s2.unchanged) == (0, 1, 0) + doc2 = _doc(db, "pgtail", "ikea.pdf") + assert doc2.page_count == 6 and doc2.page_strip_mode == "additive" + assert p10.exists(), "the unreadable tail file survived (never raised)" + for n in range(7, 10): + assert not (page_dir / f"{doc1.id}-p{n}.png").exists(), f"p{n} still leaked" + assert any( + "could not delete PDF page copy" in m for m in caplog.messages + ), caplog.messages + + # --------------------------------------------------------------------------- # The shared cleanup helper (the _delete_image_copy convention) # --------------------------------------------------------------------------- diff --git a/tests/unit/test_summary_edit_ui.py b/tests/unit/test_summary_edit_ui.py index bda1341..9de15f7 100644 --- a/tests/unit/test_summary_edit_ui.py +++ b/tests/unit/test_summary_edit_ui.py @@ -117,7 +117,10 @@ def test_gate_runs_after_the_phase36_base_construction() -> None: label = js.find('title.textContent = "Summary"') text_node = js.find('body.className = "doc-summary-text"') append = js.find("section.append(title, body)") - mount = js.find("contentEl.appendChild(section)") + # Phase 163 (task 04): the mount rides the `slot` variable — + # slot === contentEl for every non-additive doc (the byte-identity + # property), views only for the additive strip class. + mount = js.find("slot.appendChild(section)") gate = js.find("void docAdminReady().then(", mount) wiring = js.find("if (admin) wireSummaryEdit(section, doc);") assert 0 < base < label < text_node < append < mount < gate < wiring, ( diff --git a/tests/unit/test_viewer_additive_toggle.py b/tests/unit/test_viewer_additive_toggle.py new file mode 100644 index 0000000..ca519a0 --- /dev/null +++ b/tests/unit/test_viewer_additive_toggle.py @@ -0,0 +1,431 @@ +"""Unit: the additive PDF strip view (phase 163, task 04 — D4, LOCKED A1/A6). + +Frontend source-contract pins + node behavioral runs (the +``test_document_viewer.py`` house pattern) over ``document.js``'s +``renderDocument`` — the ONE shared core (Sources modal AND +/document.html): + +* the MODE branch — ``page_urls`` + ``page_strip_mode === "additive"`` + falls through to the text-doc rendering (Summary panel + content, + the pre-148 branch reused) wrapped in a ``.doc-views`` container + with the ``.doc-view-toggle`` group above it (Text pressed by + default, LOCKED A6); ``page_urls`` with the mode ABSENT or + ``"primary"`` keeps the phase-148 strip-only branch VERBATIM (append + the strip to ``contentEl`` + return — no toggle, no wrapper, no + Summary panel); falsy ``page_urls`` is untouched; +* the toggle semantics — ``role="group"`` + ``aria-label="Document + view"``, two ``<button type="button">`` labeled ``Text`` / ``Pages`` + (fixed strings, no doc-derived text), ``aria-pressed`` true/false + initial state, and a click mutating ONLY the two ``aria-pressed`` + states + the wrapper's ``data-view`` attribute (CSS-driven show/ + hide — the strip node stays mounted, no re-render, no refetch); +* the never-build-HTML rule — createElement + properties/textContent + only in the new code path (the module's ONE innerHTML stays the + escape-first markdown render); the strip imgs keep + ``loading = "lazy"`` (the hidden strip never fetches); +* the ``.doc-view-toggle`` CSS — the house ghost-pill idiom, the + pressed state as text + style (never color alone, B5), ≥44px + targets, and the ``data-view`` show/hide contract. +""" +from __future__ import annotations + +import json +import re +import shutil +import subprocess +from pathlib import Path + +import pytest + +FRONTEND = Path(__file__).resolve().parents[2] / "frontend" +DOCUMENT_JS = FRONTEND / "assets" / "document.js" +STYLES_CSS = FRONTEND / "assets" / "styles.css" + +HAVE_NODE = shutil.which("node") is not None + + +def _read(path: Path) -> str: + return path.read_text(encoding="utf-8") + + +def _fn(js: str, name: str) -> str: + """The source of a top-level function via balanced-brace counting + (the ``test_document_viewer.py`` pattern).""" + start = js.find(f"function {name}(") + assert start != -1, f"{name}() must exist in document.js" + i = js.find("(", start) + depth = 0 + close = i + while i < len(js): + if js[i] == "(": + depth += 1 + elif js[i] == ")": + depth -= 1 + if depth == 0: + close = i + break + i += 1 + brace = js.find("{", close) + depth = 0 + for j in range(brace, len(js)): + if js[j] == "{": + depth += 1 + elif js[j] == "}": + depth -= 1 + if depth == 0: + return js[start : j + 1] + raise AssertionError(f"unbalanced braces in {name}()") + + +def _code(js: str) -> str: + """The source with comments stripped (the never-build-HTML pins).""" + return re.sub(r"//.*?$|/\*.*?\*/", "", js, flags=re.S | re.M) + + +def _run_node(script: str) -> str: + proc = subprocess.run( + ["node", "-e", script], capture_output=True, text=True, timeout=60 + ) + assert proc.returncode == 0, f"node failed: {proc.stderr}" + return proc.stdout + + +# --------------------------------------------------------------------------- +# Source pins — the renderDocument mode branch +# --------------------------------------------------------------------------- + + +def test_render_document_branches_on_strip_mode() -> None: + """The mode branch (LOCKED A2 — the tolerant read): the additive + flag keys on the EXACT ``doc.page_strip_mode === "additive"`` + string (absent / "primary" / anything else ⇒ primary), and the + primary/absent strip branch stays the phase-148 shape — append + ``docPagesStrip`` to ``contentEl`` and RETURN — evaluated BEFORE + any wrapper/toggle construction.""" + render = _fn(_read(DOCUMENT_JS), "renderDocument") + assert 'doc.page_strip_mode === "additive"' in render, ( + "only the exact additive string switches on the toggle" + ) + gate_i = render.find('const additive = stripEligible && doc.page_strip_mode === "additive";') + primary_i = render.find("if (stripEligible && !additive) {") + strip_i = render.find("contentEl.appendChild(docPagesStrip(doc, pageUrls));") + return_i = render.find("return;", strip_i) + wrapper_i = render.find('views.className = "doc-views";') + assert -1 < gate_i < primary_i < strip_i < return_i < wrapper_i, ( + "the primary/absent branch (strip into contentEl + return) precedes " + "the additive wrapper — its DOM stays the phase-148 shape verbatim" + ) + # The wrapper + toggle: ONE .doc-views container, the toggle BEFORE + # it, both into contentEl; the text view lands in the wrapper. + append_i = render.find("contentEl.append(docViewToggle(views), views);") + assert wrapper_i < append_i, "the wrapper is built, then appended after its toggle" + assert 'views.setAttribute("data-view", "text");' in render, ( + "LOCKED A6 — Text is the default view" + ) + # The strip mounts ONCE inside the wrapper, after the text view — + # never rebuilt on a flip (no docPagesStrip call outside the two + # branches). + assert render.count("docPagesStrip(doc, pageUrls)") == 2 + mount_i = render.rfind("views.appendChild(docPagesStrip(doc, pageUrls));") + assert mount_i > render.find('slot.appendChild(pre);') or mount_i > render.rfind( + "slot.appendChild(wrap);" + ), "the strip mounts AFTER the text view inside the wrapper" + # The non-additive path is untouched: slot === contentEl (the + # pre-148 appends keep their exact shape through the slot var). + assert "let slot = contentEl;" in render + + +def test_primary_absent_branch_builds_no_toggle() -> None: + """The primary/absent branch is toggle-free by construction: the + strip-branch block (from the ``pageUrls`` read to its ``return``) + contains NO ``doc-view-toggle`` / ``doc-views`` construction — the + strip-only DOM stays byte-identical to pre-phase-163.""" + render = _fn(_read(DOCUMENT_JS), "renderDocument") + primary_start = render.find("if (stripEligible && !additive)") + branch = render[render.find("const pageUrls =") : render.find("return;", primary_start)] + assert "doc-view-toggle" not in branch + assert "doc-views" not in branch + assert branch.count("appendChild") == 1, ( + "the primary branch is exactly the phase-148 append + return" + ) + + +def test_toggle_never_builds_html_and_labels_are_fixed() -> None: + """The never-build-HTML rule: the toggle builders use + createElement + properties/textContent only — no innerHTML / + outerHTML — and the labels are the FIXED strings ``Text`` / + ``Pages`` (no doc-derived text inside the toggle). The module's + ONE innerHTML (the escape-first markdown render) survives.""" + js = _read(DOCUMENT_JS) + toggle = _fn(js, "docViewToggle") + btn = _fn(js, "viewToggleButton") + for body in (toggle, btn): + code = _code(body) + assert "innerHTML" not in code and "outerHTML" not in code + assert 'document.createElement("div")' in toggle + assert 'group.setAttribute("role", "group")' in toggle + assert 'group.setAttribute("aria-label", "Document view")' in toggle + assert 'document.createElement("button")' in btn + assert 'btn.type = "button"' in btn + assert 'btn.className = "doc-view-btn"' in btn + assert 'viewToggleButton("Text")' in toggle + assert 'viewToggleButton("Pages")' in toggle + # The module-wide XSS contract: exactly one innerHTML, the + # renderMarkdown line. + whole = _code(js) + assert whole.count("innerHTML") == 1 + assert "outerHTML" not in whole + + +def test_strip_imgs_stay_lazy() -> None: + """The phase-148 lazy-img pin re-runs green: the strip builder is + UNMODIFIED — every page ``<img>`` keeps ``loading = "lazy"`` (the + additive default view never fetches the strip).""" + strip = _fn(_read(DOCUMENT_JS), "docPagesStrip") + assert 'img.loading = "lazy";' in strip + + +def test_view_toggle_css() -> None: + """styles.css: the ``.doc-view-toggle`` group (the chat-column + reading width, the pill pair adjacent) + the ``.doc-view-btn`` + ghost pill (≥44px target, ink-soft text — AA) + the PRESSED state + (``[aria-pressed="true"]`` — brand fill + font-weight 700: text + + style, never color alone, B5) + the CSS-driven view swap + (``.doc-views .doc-pages`` hidden by default; ``[data-view= + "pages"]`` hides the Summary panel / .doc-md / .doc-raw / + .doc-image and shows the strip as the grid). Provenance comment + above the block (house style).""" + css = _read(STYLES_CSS) + toggle_i = css.find(".doc-view-toggle {") + assert toggle_i != -1, "styles.css must style .doc-view-toggle" + comment_start = css.rfind("/*", 0, toggle_i) + assert -1 < comment_start < toggle_i and "phase 163" in css[comment_start:toggle_i] + btn_i = css.find(".doc-view-btn {") + pressed_i = css.find('.doc-view-btn[aria-pressed="true"]') + assert toggle_i < btn_i < pressed_i + btn_rule = css[btn_i : css.find("}", btn_i)] + for decl in ("min-height: 44px", "color: var(--ink-soft)", "border: 1px solid var(--line)"): + assert decl in btn_rule + pressed_rule = css[pressed_i : css.find("}", pressed_i)] + assert "background: var(--brand)" in pressed_rule + assert "font-weight: 700" in pressed_rule, ( + "the pressed state is text weight + fill — never color alone (B5)" + ) + # The CSS-driven swap: hidden strip by default; pages hides the + # text view and shows the strip. + assert '.doc-views .doc-pages { display: none; }' in css + for sel in (".doc-summary", ".doc-md", ".doc-raw", ".doc-image"): + assert f'.doc-views[data-view="pages"] {sel}' in css + assert '.doc-views[data-view="pages"] .doc-pages { display: grid; }' in css + + +# --------------------------------------------------------------------------- +# Node behavioral runs — the real renderDocument over a minimal fake DOM +# --------------------------------------------------------------------------- + +_FAKE_DOM = """ +function makeEl(tag) { + return { + tag, className: "", textContent: "", attrs: {}, children: [], handlers: {}, + setAttribute(k, v) { this.attrs[k] = String(v); }, + getAttribute(k) { return k in this.attrs ? this.attrs[k] : null; }, + appendChild(c) { this.children.push(c); return c; }, + append(...cs) { this.children.push(...cs); }, + replaceChildren(...cs) { this.children = cs; }, + addEventListener(ev, fn) { this.handlers[ev] = fn; }, + click() { this.handlers.click(); }, + }; +} +globalThis.document = { createElement: (t) => makeEl(t) }; +function fmtDate(x) { return "DATE:" + x; } +function metaBadge(cls, text, title) { + const s = makeEl("span"); + s.className = cls; + s.textContent = text; + if (title !== undefined) s.setAttribute("title", title); + return s; +} +function copyPathButton() { return makeEl("button"); } +function originAnchor() { return makeEl("a"); } +async function docAdminReady() { return false; } +function wireDateEdit() {} +function wireSummaryEdit() {} +function renderMarkdown(s) { return s; } +function docImageBlock() { return makeEl("div"); } +function describe(el) { + return { + tag: el.tag, + className: el.className, + attrs: el.attrs, + text: el.textContent, + children: el.children.map(describe), + }; +} +""" + + +def _render_script() -> str: + js = _read(DOCUMENT_JS) + parts = [ + _fn(js, n) + for n in ("renderDocument", "docViewToggle", "viewToggleButton", "docPagesStrip") + ] + return _FAKE_DOM + "\n".join(parts) + + +@pytest.mark.skipif(not HAVE_NODE, reason="node not available") +def test_additive_doc_renders_text_views_and_toggle() -> None: + """Behavioral (node, fake DOM): an ADDITIVE doc renders the TEXT + doc exactly as a non-qualifying one — Summary panel + content — + inside ``.doc-views`` (data-view="text"), with the toggle group + FIRST in the content slot: role=group, aria-label="Document + view", the two ``doc-view-btn`` buttons labeled Text/Pages, Text + aria-pressed="true" (LOCKED A6), and the strip MOUNTED inside the + wrapper (figures + lazy imgs) after the text view.""" + out = _run_node( + _render_script() + + """ +const doc = { + title: "Letumdei", path: "book.pdf", source: "Library", format: "pdf", + summary: "A book.", content: "BODY TEXT", created_at: "c", indexed_at: "i", + chunks: 9, page_urls: ["/api/documents/x/pages/1", "/api/documents/x/pages/2"], + page_strip_mode: "additive", +}; +const contentEl = makeEl("div"); +renderDocument(doc, { titleEl: makeEl("h1"), metaEl: makeEl("div"), contentEl }); +console.log(JSON.stringify(describe(contentEl))); +""" + ) + content = json.loads(out) + assert [c["className"] for c in content["children"]] == [ + "doc-view-toggle", + "doc-views", + ], "the toggle group sits above the .doc-views wrapper" + toggle, views = content["children"] + assert toggle["attrs"]["role"] == "group" + assert toggle["attrs"]["aria-label"] == "Document view" + labels = [c["text"] for c in toggle["children"]] + assert labels == ["Text", "Pages"] + assert [c["attrs"]["aria-pressed"] for c in toggle["children"]] == ["true", "false"], ( + "Text pressed by default (LOCKED A6)" + ) + for c in toggle["children"]: + assert c["tag"] == "button" + assert views["attrs"]["data-view"] == "text" + assert [c["className"] for c in views["children"]] == [ + "doc-summary", + "doc-raw", + "doc-pages", + ], "additive = Summary panel + text content + the mounted strip" + # pre.textContent — a text node, never parsed as HTML + assert views["children"][1]["text"] == "BODY TEXT" + strip = views["children"][2] + assert len(strip["children"]) == 2, "one figure per page_urls entry" + + +@pytest.mark.skipif(not HAVE_NODE, reason="node not available") +def test_primary_and_absent_modes_keep_the_strip_only_dom() -> None: + """Behavioral: mode "primary" AND an absent mode (the pre-163 + stamped row — the live Letumdei pre-sync shape) keep the phase-148 + DOM byte-identically: the content slot holds EXACTLY the + ``.doc-pages`` strip — no wrapper, no toggle, no Summary panel, + no text block.""" + out = _run_node( + _render_script() + + """ +const results = []; +for (const mode of ["primary", undefined]) { + const doc = { + title: "Manual", path: "m.pdf", source: "Library", format: "pdf", + summary: "A manual.", content: "SHOULD NOT SHOW", created_at: "c", + indexed_at: "i", chunks: 4, + page_urls: ["/api/documents/y/pages/1"], + }; + if (mode !== undefined) doc.page_strip_mode = mode; + const contentEl = makeEl("div"); + renderDocument(doc, { titleEl: makeEl("h1"), metaEl: makeEl("div"), contentEl }); + results.push(describe(contentEl)); +} +console.log(JSON.stringify(results)); +""" + ) + primary, absent = json.loads(out) + for content in (primary, absent): + assert len(content["children"]) == 1 + assert content["children"][0]["className"] == "doc-pages" + classes = json.dumps(content) + assert "doc-view-toggle" not in classes + assert "doc-views" not in classes + assert "doc-summary" not in classes + assert "doc-raw" not in classes, "the strip replaces the text block (primary class)" + + +@pytest.mark.skipif(not HAVE_NODE, reason="node not available") +def test_text_doc_dom_is_untouched() -> None: + """Behavioral: a doc with no page_urls is UNTOUCHED — Summary + panel + content appended straight to the content slot, no + wrapper, no toggle (the byte-identity property for every + non-PDF).""" + out = _run_node( + _render_script() + + """ +const doc = { + title: "Note", path: "n.pdf", source: "Library", format: "pdf", + summary: "S.", content: "TEXT", created_at: "c", indexed_at: "i", chunks: 1, +}; +const contentEl = makeEl("div"); +renderDocument(doc, { titleEl: makeEl("h1"), metaEl: makeEl("div"), contentEl }); +console.log(JSON.stringify(describe(contentEl))); +""" + ) + content = json.loads(out) + assert [c["className"] for c in content["children"]] == ["doc-summary", "doc-raw"] + + +@pytest.mark.skipif(not HAVE_NODE, reason="node not available") +def test_toggle_flips_view_without_rerender_or_refetch() -> None: + """Behavioral: the toggle handler mutates ONLY the two + aria-pressed states + the wrapper's data-view. Clicking Pages + flips data-view to "pages"; clicking back to Text reverses; the + strip node STAYS MOUNTED the whole time (same object, no + re-append — the flip-flop fetches nothing) and the wrapper's + children never change count.""" + out = _run_node( + _render_script() + + """ +const doc = { + title: "Book", path: "b.pdf", source: "Library", format: "pdf", + summary: "S.", content: "TEXT", created_at: "c", indexed_at: "i", chunks: 2, + page_urls: ["/api/documents/z/pages/1", "/api/documents/z/pages/2"], + page_strip_mode: "additive", +}; +const contentEl = makeEl("div"); +renderDocument(doc, { titleEl: makeEl("h1"), metaEl: makeEl("div"), contentEl }); +const toggle = contentEl.children[0]; +const views = contentEl.children[1]; +const [textBtn, pagesBtn] = toggle.children; +const stripBefore = views.children[2]; +const countBefore = views.children.length; +pagesBtn.click(); +const afterPages = { + view: views.getAttribute("data-view"), + pressed: toggle.children.map((b) => b.getAttribute("aria-pressed")), +}; +textBtn.click(); +const afterText = { + view: views.getAttribute("data-view"), + pressed: toggle.children.map((b) => b.getAttribute("aria-pressed")), + stripSame: views.children[2] === stripBefore, + countSame: views.children.length === countBefore, +}; +console.log(JSON.stringify({ afterPages, afterText })); +""" + ) + res = json.loads(out) + assert res["afterPages"] == {"view": "pages", "pressed": ["false", "true"]} + assert res["afterText"] == { + "view": "text", + "pressed": ["true", "false"], + "stripSame": True, + "countSame": True, + }, "the strip node persists across flips — no re-render, no refetch" diff --git a/tests/unit/test_wide_column_css.py b/tests/unit/test_wide_column_css.py index cbbf2a2..557b256 100644 --- a/tests/unit/test_wide_column_css.py +++ b/tests/unit/test_wide_column_css.py @@ -13,12 +13,13 @@ without a browser: * NO ``@media (min-width: 1500px)`` block anywhere in the file — the phase-58 wide-desktop doubling is deleted in full (the token is 72rem at every viewport); -* the six reading-column selectors — ``.chat-shell``, +* the reading-column selectors — ``.chat-shell``, ``.shared-shell``, ``.doc-md``, ``.doc-image`` (phase 122, task 04), - ``.doc-pages`` (phase 148, task 03 — the page strip), and - ``.doc-summary:has(+ .doc-md)`` — each capped with - ``max-width: var(--chat-column)`` and NOTHING else in the file uses - the token for a max-width (exactly six rules); + ``.doc-pages`` (phase 148, task 03 — the page strip), + ``.doc-view-toggle`` (phase 163, task 04 — the Text/Pages toggle + above the additive views), and ``.doc-summary:has(+ .doc-md)`` — + each capped with ``max-width: var(--chat-column)`` and NOTHING else + in the file uses the token for a max-width (exactly seven rules); * the flipped negative pin — ZERO literal ``max-width: 46rem`` rules remain (the phase-27/59/91 form-column caps are retired): the form shells (``.tuning-shell``, ``.theme-shell``, ``.doc-edit-shell``) @@ -121,8 +122,10 @@ def test_the_reading_columns_use_the_token() -> None: task 04 — the viewer's image block rides the SAME reading column as its .doc-md sibling), .doc-pages (phase 148, task 03 — the viewer's page strip rides the SAME reading column in the shared - #doc-content slot) and .doc-summary:has(+ .doc-md) each cap with - max-width: var(--chat-column) — and exactly those six rules use + #doc-content slot), .doc-view-toggle (phase 163, task 04 — the + Text/Pages toggle sits above the additive views, aligned with + them) and .doc-summary:has(+ .doc-md) each cap with + max-width: var(--chat-column) — and exactly those seven rules use the token for a max-width (no other selector).""" css = _css() for selector in ( @@ -131,12 +134,13 @@ def test_the_reading_columns_use_the_token() -> None: ".doc-md", ".doc-image", ".doc-pages", + ".doc-view-toggle", ".doc-summary:has(+ .doc-md)", ): assert "max-width: var(--chat-column)" in _rule_block(css, selector), ( f"{selector} must cap with max-width: var(--chat-column)" ) - assert css.count("max-width: var(--chat-column)") == 6, ( + assert css.count("max-width: var(--chat-column)") == 7, ( "exactly the reading-column selectors use the token" )