diff --git a/.agents/phases/todo/148_pdf_pages_in_sources/00_phase.md b/.agents/phases/complete/148_pdf_pages_in_sources/00_phase.md similarity index 100% rename from .agents/phases/todo/148_pdf_pages_in_sources/00_phase.md rename to .agents/phases/complete/148_pdf_pages_in_sources/00_phase.md diff --git a/.agents/phases/todo/148_pdf_pages_in_sources/01_page_render_at_index.md b/.agents/phases/complete/148_pdf_pages_in_sources/01_page_render_at_index.md similarity index 100% rename from .agents/phases/todo/148_pdf_pages_in_sources/01_page_render_at_index.md rename to .agents/phases/complete/148_pdf_pages_in_sources/01_page_render_at_index.md diff --git a/.agents/phases/todo/148_pdf_pages_in_sources/02_page_route_and_payload.md b/.agents/phases/complete/148_pdf_pages_in_sources/02_page_route_and_payload.md similarity index 100% rename from .agents/phases/todo/148_pdf_pages_in_sources/02_page_route_and_payload.md rename to .agents/phases/complete/148_pdf_pages_in_sources/02_page_route_and_payload.md diff --git a/.agents/phases/todo/148_pdf_pages_in_sources/03_viewer_page_strip.md b/.agents/phases/complete/148_pdf_pages_in_sources/03_viewer_page_strip.md similarity index 100% rename from .agents/phases/todo/148_pdf_pages_in_sources/03_viewer_page_strip.md rename to .agents/phases/complete/148_pdf_pages_in_sources/03_viewer_page_strip.md diff --git a/.agents/phases/todo/148_pdf_pages_in_sources/04_e2e_pdf_pages.md b/.agents/phases/complete/148_pdf_pages_in_sources/04_e2e_pdf_pages.md similarity index 100% rename from .agents/phases/todo/148_pdf_pages_in_sources/04_e2e_pdf_pages.md rename to .agents/phases/complete/148_pdf_pages_in_sources/04_e2e_pdf_pages.md diff --git a/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__00_phase.a1.err b/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__00_phase.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__00_phase.a1.md b/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__00_phase.a1.md new file mode 100644 index 0000000..9a0e0ba --- /dev/null +++ b/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__00_phase.a1.md @@ -0,0 +1,8 @@ +**Phase 148 — final verification pass: complete, 100% green.** + +- **Verified (tasks 01–04 already implemented):** page render/persist at index (`_persist_page_copies`, fail-soft, stale-replace, qualifier-loss, prune hook via `_delete_page_copies`); public `GET /api/documents/{id}/pages/{n}` (uuid4-credential, 404-all-non-servable, `image/png` + cache header); `DocContent.page_urls` additive-omitted; shared `renderDocument` strip (lazy imgs, "Page N" captions, text + Summary suppressed, onerror note, createElement-only); migration 0026; `pdf_page_dir` setting; `.env.example` entry +- **Completion criteria:** (1) pictorial + fallback classes get `p{N}.png` + `page_count`, text-heavy gets neither — pinned, pass; (2) unchanged re-sync renders zero (fast path) — pass; (3) changed re-index replaces copies, qualifier-loss/prune delete — pass; (4) route 404 matrix + payload key absent-never-null — pass; (5) modal AND /document.html strip with suppression, text-heavy byte-identical — E2E pass; (6) see below; (7) no completed-phase drift — full suite green +- **Tests:** `uv run pytest --cov=app --cov-report=term-missing` → **3461 passed, 0 failed**, `app/` coverage **99%** (>90%); `uv run pytest tests/e2e/test_pdf_pages_in_sources.py -v --no-cov` → **4 passed** in isolation; `uv run ruff check .` clean; `uv run pyright` → 0 errors, 0 warnings +- **Documentation:** README had **1 pre-existing inconsistency** — the Production Deployment section's standalone `podman run` example (and compose note) omitted the **required** `BOR_ADMIN_PASSWORD`/`BOR_SESSION_SECRET` (the fail-loud boot gate the README's own Admin section documents — the container would refuse to start); fixed with the two `-e` flags + a note for the compose path. All else verified consistent: 70/70 `.env.example` vars documented, all defaults match `config.py`, pages table matches routes, Containerfile/esbuild/uid-10001/healthcheck/entrypoint claims accurate, QA commands match pytest config. **AGENTS.md: verified consistent — no changes needed** +- **Notable:** repo uses `Containerfile` (Podman) — README's `podman build` example is valid as-is; no live infrastructure touched +- **Next pending phase:** `149_sync_progress_box` diff --git a/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__00_phase.a1.validate b/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__00_phase.a1.validate new file mode 100644 index 0000000..e08e342 --- /dev/null +++ b/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__00_phase.a1.validate @@ -0,0 +1,120 @@ +........................................................................ [ 2%] +........................................................................ [ 4%] +........................................................................ [ 6%] +........................................................................ [ 8%] +........................................................................ [ 10%] +........................................................................ [ 12%] +........................................................................ [ 14%] +........................................................................ [ 16%] +........................................................................ [ 18%] +........................................................................ [ 20%] +........................................................................ [ 22%] +........................................................................ [ 24%] +........................................................................ [ 27%] +........................................................................ [ 29%] +........................................................................ [ 31%] +........................................................................ [ 33%] +........................................................................ [ 35%] +........................................................................ [ 37%] +........................................................................ [ 39%] +........................................................................ [ 41%] +........................................................................ [ 43%] +........................................................................ [ 45%] +........................................................................ [ 47%] +........................................................................ [ 49%] +........................................................................ [ 52%] +........................................................................ [ 54%] +........................................................................ [ 56%] +........................................................................ [ 58%] +........................................................................ [ 60%] +........................................................................ [ 62%] +........................................................................ [ 64%] +........................................................................ [ 66%] +........................................................................ [ 68%] +........................................................................ [ 70%] +........................................................................ [ 72%] +........................................................................ [ 74%] +........................................................................ [ 76%] +........................................................................ [ 79%] +........................................................................ [ 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 270 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 183 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 265 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 135 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 431 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 744 4 99% +app/rag/llm.py 246 1 99% +app/rag/md_images.py 63 0 100% +app/rag/overview.py 71 0 100% +app/rag/pdf.py 124 0 100% +app/rag/prompts.py 155 0 100% +app/rag/retriever.py 248 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 443 0 100% +-------------------------------------------------- +TOTAL 5609 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/148_pdf_pages_in_sources/148_pdf_pages_in_sources__01_page_render_at_index.a1.err b/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__01_page_render_at_index.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__01_page_render_at_index.a1.md b/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__01_page_render_at_index.a1.md new file mode 100644 index 0000000..c2cec16 --- /dev/null +++ b/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__01_page_render_at_index.a1.md @@ -0,0 +1,19 @@ +All gates green. Task 01 is complete. + +## Report + +**Implemented** (phase 148, task 01 — page render at index time): +- `app/config.py`: `pdf_page_dir` (`BOR_PDF_PAGE_DIR`, default `~/bor-sources/pages`) — raw-string `image_dir` convention +- `alembic/versions/0026_documents_page_count.py` + `Document.page_count` (nullable Int, no index, tested downgrade) +- `app/rag/importer.py`: eligibility signals in `_index_pdf_file` (phase-133 `fallback_fired` / phase-140 `pictorial_described`); fail-soft `_persist_pdf_pages` after flush (every page → `-p{N}.png`, `page_count` = page count, stale copies deleted before replacement, qualifier-loss/unopenable → NULL + copies deleted, partial-write cleanup); `_delete_page_copies` helper; `_prune` deletes copies with pruned rows (incl. parent-PDF path); `source_removal.py` untouched; fast path renders nothing by construction +- Docs: README env table + `.env.example` gained `BOR_PDF_PAGE_DIR`; AGENTS.md unchanged (no convention impact) + +**Tests / lint / coverage:** +- `uv run pytest` → **3455 passed, 0 failed** (incl. 12 new `tests/unit/test_pdf_page_copies.py` matrix, `tests/integration/test_migration_0026.py` round-trip, `test_config.py` pin; pre-phase PDF/importer/fast-path suites green unedited) +- `uv run pytest --cov=app --cov-report=term-missing` → **99%** on `app/` (gate: >90%) +- `uv run ruff check . && uv run pyright` → clean; `tests/e2e/test_smoke.py --no-cov` → 3 passed (boot sanity) +- Dev DB migrated to head (0026) + +**Notable decisions:** `_prune` takes an optional `page_dir` kwarg (the phase-122 `images` pattern) so pre-phase call sites keep working; `page_count` commits atomically with doc+chunks (block sits between flush and the chunk pipeline). No live-infra changes. + +**Next pending task:** `02_page_route_and_payload.md` diff --git a/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__01_page_render_at_index.a1.validate b/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__01_page_render_at_index.a1.validate new file mode 100644 index 0000000..f0a4ec2 --- /dev/null +++ b/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__01_page_render_at_index.a1.validate @@ -0,0 +1,119 @@ +........................................................................ [ 2%] +........................................................................ [ 4%] +........................................................................ [ 6%] +........................................................................ [ 8%] +........................................................................ [ 10%] +........................................................................ [ 12%] +........................................................................ [ 14%] +........................................................................ [ 16%] +........................................................................ [ 18%] +........................................................................ [ 20%] +........................................................................ [ 22%] +........................................................................ [ 25%] +........................................................................ [ 27%] +........................................................................ [ 29%] +........................................................................ [ 31%] +........................................................................ [ 33%] +........................................................................ [ 35%] +........................................................................ [ 37%] +........................................................................ [ 39%] +........................................................................ [ 41%] +........................................................................ [ 43%] +........................................................................ [ 45%] +........................................................................ [ 47%] +........................................................................ [ 50%] +........................................................................ [ 52%] +........................................................................ [ 54%] +........................................................................ [ 56%] +........................................................................ [ 58%] +........................................................................ [ 60%] +........................................................................ [ 62%] +........................................................................ [ 64%] +........................................................................ [ 66%] +........................................................................ [ 68%] +........................................................................ [ 70%] +........................................................................ [ 72%] +........................................................................ [ 75%] +........................................................................ [ 77%] +........................................................................ [ 79%] +........................................................................ [ 81%] +........................................................................ [ 83%] +........................................................................ [ 85%] +........................................................................ [ 87%] +........................................................................ [ 89%] +........................................................................ [ 91%] +........................................................................ [ 93%] +........................................................................ [ 95%] +........................................................................ [ 97%] +....................................................................... [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 270 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 181 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 183 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 265 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 135 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 431 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 744 4 99% +app/rag/llm.py 246 1 99% +app/rag/md_images.py 63 0 100% +app/rag/overview.py 71 0 100% +app/rag/pdf.py 124 0 100% +app/rag/prompts.py 155 0 100% +app/rag/retriever.py 248 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 440 0 100% +-------------------------------------------------- +TOTAL 5591 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/148_pdf_pages_in_sources/148_pdf_pages_in_sources__02_page_route_and_payload.a1.err b/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__02_page_route_and_payload.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__02_page_route_and_payload.a1.md b/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__02_page_route_and_payload.a1.md new file mode 100644 index 0000000..6f6fcb3 --- /dev/null +++ b/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__02_page_route_and_payload.a1.md @@ -0,0 +1,18 @@ +Task 02 complete. All verification green. + +**Implemented** +- `GET /api/documents/{doc_id}/pages/{n}` in `app/api/docs.py` — public page-bytes route (image route's phase-124 A4 trust model verbatim): `image/png` + `Cache-Control: private, max-age=3600`; 404 `document not found` for unparseable/unknown id, `page_count` NULL, `n` outside `1..page_count`, and missing file; path built only from row id + int `n` (no traversal surface); module docstring extended +- `DocContent.page_urls: list[str] | None` in `app/schemas.py` + serializer pop (absent, never null — the `image_url` precedent) +- `GET /documents/content` fills `page_urls` = exact 1-based route list when `page_count` is set +- Tests: serializer omission pin + stub-session endpoint pin (`tests/unit/test_document_viewer.py`); serve-bytes, full 404 matrix (incl. malformed id → 404 not 422, non-int `n` → 422, lost copy, out-of-range), public/anonymous, and payload presence/absence pins (`tests/integration/test_docs_api.py`, beside the image-route tests) + +**Results** +- `uv run pytest` → 3461 passed +- `uv run pytest --cov=app --cov-report=term-missing` → 3461 passed, TOTAL 99% (>90%; `app/schemas.py` 100%, `app/api/docs.py` 99% — sole miss is a pre-existing defensive branch) +- `uv run ruff check .` clean; `uv run pyright` → 0 errors + +**Docs**: no changes needed — `BOR_PDF_PAGE_DIR` already in README/.env.example from task 01; bytes routes aren't individually listed in the README (image-route parity); viewer-strip user-visible behavior lands in task 03. + +**Decisions**: none — followed the task's locked design verbatim. + +**Next pending task**: `03_viewer_page_strip.md` (the `renderDocument` page strip + CSS). diff --git a/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__02_page_route_and_payload.a1.validate b/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__02_page_route_and_payload.a1.validate new file mode 100644 index 0000000..e08e342 --- /dev/null +++ b/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__02_page_route_and_payload.a1.validate @@ -0,0 +1,120 @@ +........................................................................ [ 2%] +........................................................................ [ 4%] +........................................................................ [ 6%] +........................................................................ [ 8%] +........................................................................ [ 10%] +........................................................................ [ 12%] +........................................................................ [ 14%] +........................................................................ [ 16%] +........................................................................ [ 18%] +........................................................................ [ 20%] +........................................................................ [ 22%] +........................................................................ [ 24%] +........................................................................ [ 27%] +........................................................................ [ 29%] +........................................................................ [ 31%] +........................................................................ [ 33%] +........................................................................ [ 35%] +........................................................................ [ 37%] +........................................................................ [ 39%] +........................................................................ [ 41%] +........................................................................ [ 43%] +........................................................................ [ 45%] +........................................................................ [ 47%] +........................................................................ [ 49%] +........................................................................ [ 52%] +........................................................................ [ 54%] +........................................................................ [ 56%] +........................................................................ [ 58%] +........................................................................ [ 60%] +........................................................................ [ 62%] +........................................................................ [ 64%] +........................................................................ [ 66%] +........................................................................ [ 68%] +........................................................................ [ 70%] +........................................................................ [ 72%] +........................................................................ [ 74%] +........................................................................ [ 76%] +........................................................................ [ 79%] +........................................................................ [ 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 270 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 183 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 265 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 135 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 431 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 744 4 99% +app/rag/llm.py 246 1 99% +app/rag/md_images.py 63 0 100% +app/rag/overview.py 71 0 100% +app/rag/pdf.py 124 0 100% +app/rag/prompts.py 155 0 100% +app/rag/retriever.py 248 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 443 0 100% +-------------------------------------------------- +TOTAL 5609 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/148_pdf_pages_in_sources/148_pdf_pages_in_sources__03_viewer_page_strip.a1.err b/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__03_viewer_page_strip.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__03_viewer_page_strip.a1.md b/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__03_viewer_page_strip.a1.md new file mode 100644 index 0000000..40d4b8b --- /dev/null +++ b/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__03_viewer_page_strip.a1.md @@ -0,0 +1,19 @@ +Everything is complete and verified. Final report: + +**Task 03 — viewer page strip — complete** + +- `frontend/assets/document.js`: `renderDocument` gate (`page_urls` non-empty array → strip + early return — text content AND Summary panel incl. admin-edit wiring not built; meta row untouched; null/absent/empty → exact pre-phase path) + `docPagesStrip` helper (`.doc-pages`, one `
` per URL: lazy `` alt="Page N of \", "Page N" `
`; per-img `onerror` swaps only its `` for a "Page N unavailable." `role=status` note reusing the phase-122 `doc-image-unavailable` class); createElement/textContent only; module docstring gained the phase-148 paragraph + XSS-contract note +- `frontend/assets/styles.css`: `.doc-pages` (grid, 1rem gap, rides the `--chat-column` reading column), `.doc-page img` (surface/border/radius, width 100%), `.doc-page-num` (muted, centered, AA in both themes); no animation +- `tests/unit/test_wide_column_css.py`: `.doc-pages` added as the 6th token consumer (the phase-122 `.doc-image` precedent) +- Verified live (scratch Playwright, both surfaces): strip + suppression + meta intact; lost page → note on its figure only; text doc pre-phase shape + +**Results** +- `uv run pytest --cov=app --cov-report=term-missing` → **3461 passed, 0 failed**; `app/` coverage **99%** (>90%) +- Pre-phase E2E green unedited (isolated): `test_document_viewer` 8, `test_image_documents` 4, `test_summary_in_viewer`+`test_edit_summaries` 8, `test_document_dates`+`test_modal_back_button`+`test_document_back_navigation` 13 +- `uv run ruff check .` clean; `uv run pyright` → 0 errors, 0 warnings; emoji guard green + +**Docs**: README PDF section's stale "Served as text" bullet updated (phase-148 strip contract; `BOR_PDF_PAGE_DIR` row already present from task 01); no AGENTS.md impact. + +**Notable**: the tracked `doc_modal_scrollbar_1280.png` visual record was rewritten by the E2E run — pixel diff verified to be ONLY the wall-clock meta-row dates (inherent drift, no visual regression). + +**Next pending task**: `04_e2e_pdf_pages.md` diff --git a/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__03_viewer_page_strip.a1.validate b/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__03_viewer_page_strip.a1.validate new file mode 100644 index 0000000..e08e342 --- /dev/null +++ b/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__03_viewer_page_strip.a1.validate @@ -0,0 +1,120 @@ +........................................................................ [ 2%] +........................................................................ [ 4%] +........................................................................ [ 6%] +........................................................................ [ 8%] +........................................................................ [ 10%] +........................................................................ [ 12%] +........................................................................ [ 14%] +........................................................................ [ 16%] +........................................................................ [ 18%] +........................................................................ [ 20%] +........................................................................ [ 22%] +........................................................................ [ 24%] +........................................................................ [ 27%] +........................................................................ [ 29%] +........................................................................ [ 31%] +........................................................................ [ 33%] +........................................................................ [ 35%] +........................................................................ [ 37%] +........................................................................ [ 39%] +........................................................................ [ 41%] +........................................................................ [ 43%] +........................................................................ [ 45%] +........................................................................ [ 47%] +........................................................................ [ 49%] +........................................................................ [ 52%] +........................................................................ [ 54%] +........................................................................ [ 56%] +........................................................................ [ 58%] +........................................................................ [ 60%] +........................................................................ [ 62%] +........................................................................ [ 64%] +........................................................................ [ 66%] +........................................................................ [ 68%] +........................................................................ [ 70%] +........................................................................ [ 72%] +........................................................................ [ 74%] +........................................................................ [ 76%] +........................................................................ [ 79%] +........................................................................ [ 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 270 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 183 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 265 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 135 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 431 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 744 4 99% +app/rag/llm.py 246 1 99% +app/rag/md_images.py 63 0 100% +app/rag/overview.py 71 0 100% +app/rag/pdf.py 124 0 100% +app/rag/prompts.py 155 0 100% +app/rag/retriever.py 248 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 443 0 100% +-------------------------------------------------- +TOTAL 5609 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/148_pdf_pages_in_sources/148_pdf_pages_in_sources__04_e2e_pdf_pages.a1.err b/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__04_e2e_pdf_pages.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__04_e2e_pdf_pages.a1.md b/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__04_e2e_pdf_pages.a1.md new file mode 100644 index 0000000..877481b --- /dev/null +++ b/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__04_e2e_pdf_pages.a1.md @@ -0,0 +1,15 @@ +All green. Final report: + +**Implemented** +- `tests/e2e/test_pdf_pages_in_sources.py` (new, task 04): dedicated isolated suite — module app (own port 8161, `BOR_PDF_PICTORIAL=1` opt-in, `BOR_PDF_PAGE_DIR` → scratch, `pdf` import token), two synthetic PyMuPDF PDFs (IKEA-shaped 3-page pictorial + 2-page text-heavy) in a local-dir source, API-registered + one real sync; seed asserts DB (`page_count==3` vs NULL, canned blocks in stored content, summaries) + filesystem (exactly `-p{1..3}.png`, real PNGs, no orphans). Tests: ① modal strip — 3 loaded lazy imgs (`decode()`+`naturalWidth>0`), `Page N of ` alts, `Page N` captions, no `.doc-raw`/`.doc-md`/`.doc-summary`, meta badges intact, canned description absent from the rendered modal; ② text-heavy byte-identical (`.doc-raw` + Summary panel, `page_urls` key ABSENT from the payload); ③ `/document.html` renders the same strip (one core, no drift); ④ anonymous `page_urls[0]` GET → 200 `image/png`, `Cache-Control: private, max-age=3600`, body byte-equal to the stored copy. +- Fixed stale `tests/e2e/test_pictorial_pdf.py` (phase-140 display pins contradicted phase-148's locked strip behavior — its fixture is page-qualifying under its own `BOR_PDF_PICTORIAL=1` opt-in): test 1 now asserts the 2-figure strip + suppression, seed gains `page_count == 2`, docstrings updated. Chat-echo + sync-detail tests unchanged. + +**Results** +- `uv run pytest tests/e2e/test_pdf_pages_in_sources.py -v --no-cov` → 4 passed (3 consecutive runs, reproducible) +- `uv run pytest --cov=app --cov-report=term-missing` → 3461 passed, `app/` 99% (>90%) +- `uv run ruff check .` clean; `uv run pyright` → 0 errors +- Pre-existing PDF/doc-viewer E2Es: test_pictorial_pdf 3 passed, test_pdf_support 6, test_garbage_pdf 3, test_pdf_image_context/test_document_summaries/test_summary_in_viewer/test_sync_changed_only/test_document_dates 19, test_document_viewer 8 — all green + +**Docs:** no doc impact (README/.env.example already carry phase-148 entries from tasks 01–02; task 04 is E2E-only) + +**Next pending task:** none — 04 is the last task of phase 148. diff --git a/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__04_e2e_pdf_pages.a1.validate b/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__04_e2e_pdf_pages.a1.validate new file mode 100644 index 0000000..e08e342 --- /dev/null +++ b/.agents/reports/148_pdf_pages_in_sources/148_pdf_pages_in_sources__04_e2e_pdf_pages.a1.validate @@ -0,0 +1,120 @@ +........................................................................ [ 2%] +........................................................................ [ 4%] +........................................................................ [ 6%] +........................................................................ [ 8%] +........................................................................ [ 10%] +........................................................................ [ 12%] +........................................................................ [ 14%] +........................................................................ [ 16%] +........................................................................ [ 18%] +........................................................................ [ 20%] +........................................................................ [ 22%] +........................................................................ [ 24%] +........................................................................ [ 27%] +........................................................................ [ 29%] +........................................................................ [ 31%] +........................................................................ [ 33%] +........................................................................ [ 35%] +........................................................................ [ 37%] +........................................................................ [ 39%] +........................................................................ [ 41%] +........................................................................ [ 43%] +........................................................................ [ 45%] +........................................................................ [ 47%] +........................................................................ [ 49%] +........................................................................ [ 52%] +........................................................................ [ 54%] +........................................................................ [ 56%] +........................................................................ [ 58%] +........................................................................ [ 60%] +........................................................................ [ 62%] +........................................................................ [ 64%] +........................................................................ [ 66%] +........................................................................ [ 68%] +........................................................................ [ 70%] +........................................................................ [ 72%] +........................................................................ [ 74%] +........................................................................ [ 76%] +........................................................................ [ 79%] +........................................................................ [ 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 270 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 183 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 265 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 135 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 431 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 744 4 99% +app/rag/llm.py 246 1 99% +app/rag/md_images.py 63 0 100% +app/rag/overview.py 71 0 100% +app/rag/pdf.py 124 0 100% +app/rag/prompts.py 155 0 100% +app/rag/retriever.py 248 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 443 0 100% +-------------------------------------------------- +TOTAL 5609 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 54fc60a..f282e91 100644 Binary files a/.agents/screenshots/doc_modal_scrollbar_1280.png and b/.agents/screenshots/doc_modal_scrollbar_1280.png differ diff --git a/.env.example b/.env.example index 1c655e3..b0c9d64 100644 --- a/.env.example +++ b/.env.example @@ -132,6 +132,18 @@ BOR_IMPORT_EXTENSIONS=md,markdown,txt,yaml,yml,json,py,container,network,volume, # BOR_IMAGE_DIR=~/bor-sources/images # persistent home for the served image bytes # (uploads are replaced, checkouts re-cloned) +# Phase 148 (PDF page copies): where a page-qualifying PDF (the +# phase-133 screenshot class — unusable text layer — or the phase-140 +# pictorial class — the sparse-page pass described >=1 page) gets EVERY +# page rendered at index time as <doc-id>-p{N}.png, so the document +# viewer shows the actual pages (TODO L5 — the IKEA class: the page +# descriptions help the embeddings but not when shown to the user). +# Text-heavy PDFs keep the text view. The copy must outlive the source +# file (uploads are replaced, checkouts re-cloned — the image_dir +# rationale); stale copies are replaced on re-index, deleted on a +# qualifier-loss and on prune. +# BOR_PDF_PAGE_DIR=~/bor-sources/pages # persistent home for the rendered page PNGs + # Phase 124 (answer images): off by default; enable alongside # BOR_IMAGES to let the model show a supporting image in its answer # (independent of it — BOR_IMAGES indexes image docs, this one shows diff --git a/README.md b/README.md index f424cb0..c264e97 100644 --- a/README.md +++ b/README.md @@ -678,8 +678,21 @@ uploaded archives): pages on the import summary line (`pdfs_pictorial=N` `pdf_pictorial_pages=N`). `BOR_PDF_PICTORIAL=0` turns the whole pass off (byte-identical pre-phase-140 content). -- **Served as text.** The document viewer renders the extracted text — - never the binary PDF. +- **Served as text — or as pages (phase 148).** The document viewer + renders the extracted text — never the binary PDF. The exception is + the two page-qualifying classes: a PDF whose text layer fired the + phase-133 screenshot fallback, or a PDF whose phase-140 pictorial + pass described ≥ 1 sparse page. For those, every page is rendered + to a persistent PNG at index time (under `BOR_PDF_PAGE_DIR`) and + the viewer — the doc modal AND `/document.html`, one shared core — + 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 + 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. --- @@ -805,6 +818,7 @@ to `.env` and adjust. | `BOR_IMPORT_EXTENSIONS` | A9 family | Importable text formats — the A9 default is `md,markdown,txt,yaml,yml,json,py,container,network,volume,image,pod,kube,swap,os,endpoint,j2`; any well-formed extension is allowed. Add the `pdf` token to make PDFs first-class, retrievable documents (phase 126, opt-in). Images are **not** added here — see `BOR_IMAGES` | | `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_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 `<doc-id>-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_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 | @@ -954,15 +968,24 @@ podman run -d --name brain-of-reese \ -e BOR_DATABASE_URL=postgresql+psycopg://reese:SECRETPASSWORD@dbhost:5432/brain_of_reese \ -e BOR_LLM_BASE_URL=https://aipi.reeseapps.com/v1 \ -e BOR_LLM_API_KEY=$AIPI_KEY \ + -e BOR_ADMIN_PASSWORD=your-password \ + -e BOR_SESSION_SECRET=<the-hex-from-the-setup-above> \ brain-of-reese/app:latest ``` +`BOR_ADMIN_PASSWORD` and `BOR_SESSION_SECRET` are **required** — the +fail-loud boot gate in [Admin & Sign-in](#admin--sign-in) refuses to +start while either is empty. + The entrypoint runs `alembic upgrade head` automatically on start. Or run the whole stack from compose (app + db): ```bash podman compose --profile prod up -d --build ``` +The `app` service picks up `BOR_ADMIN_PASSWORD` / `BOR_SESSION_SECRET` +from the shell environment (or an env file) — the same fail-loud gate +applies. Production hardening: app runs as non-root (uid 10001), slim image, healthcheck on `/api/health`, debugpy off unless `DEBUGPY=1`, all assets diff --git a/alembic/versions/0026_documents_page_count.py b/alembic/versions/0026_documents_page_count.py new file mode 100644 index 0000000..f17cf0c --- /dev/null +++ b/alembic/versions/0026_documents_page_count.py @@ -0,0 +1,60 @@ +"""documents.page_count: the rendered page-strip's page count (phase 148, task 01) + +Revision ID: 0026 +Revises: 0025 +Create Date: 2026-10-01 + +Phase 148 (pictorial PDFs show their PAGES in the sources — task 01, +storage only): + +* ``documents.page_count`` — INTEGER NULLABLE, NO server default, NO + index: the number of rendered page PNGs persisted in + ``settings.pdf_page_dir`` (``BOR_PDF_PAGE_DIR``) as + ``<doc-id>-p{N}.png`` (N 1-based) for ONE page-qualifying document — + the PDF's page count. Set ONLY by the importer's PDF branch (task 01): + a qualifying PDF (the phase-133 fallback class — unusable text layer, + — or the phase-140 pictorial class — the sparse-page pass described + ≥1 page) gets every page rendered at index time, and this column + records the page count (a per-page render failure leaves a GAP — the + file is simply absent, the route of task 02 404s that N, the viewer's + note covers it). NULL for every other document (text documents, + images, text-heavy PDFs) and for a qualifying PDF whose render/write + failed (fail-soft — the render is a display concern, it never gates + indexing); the prune cleanup deletes the copies with the row. + + A display fact, never a query surface: the page-bytes route (task 02) + looks the document up by its uuid4 id FIRST (the unguessable id is the + credential — the phase-124 image-route trust model) and reads this + column from the ROW, so it carries no index of its own (the + ``file_hash`` convention, 0025). + +One additive, fully reversible migration (A13); no other schema change. +The render/persist/cleanup logic itself (task 01) is importer code — +this revision only carries the column. +""" +from __future__ import annotations + +import sqlalchemy as sa + +from alembic import op + +revision = "0026" +down_revision = "0025" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + # No index (a display fact, never a query surface — the page route + # reads it from the row after the uuid4 id lookup) and no server + # default (NULL is the meaningful state for every non-qualifying + # document and every fail-soft miss). + op.add_column("documents", sa.Column("page_count", sa.Integer(), nullable=True)) + + +def downgrade() -> None: + # The page_count column is the only 0026 artefact — dropping it + # leaves 0025's schema byte-identical (A13, fully reversible). The + # rendered copies are re-derivable from the source files on the + # next sync, so no data is lost by the drop. + op.drop_column("documents", "page_count") diff --git a/app/api/docs.py b/app/api/docs.py index 46c9517..c75848f 100644 --- a/app/api/docs.py +++ b/app/api/docs.py @@ -40,6 +40,20 @@ traversal/UUID-guessing has no filesystem surface to hit). The document's TEXT (the content endpoint) stays user-gated — only the bytes are public. +GET /api/documents/{doc_id}/pages/{n} — the phase-148 (task 02) PDF +page BYTES route: ONE rendered page PNG (``<doc-id>-p{N}.png`` in the +importer's ``pdf_page_dir`` copy — task 01) of a page-QUALIFYING +document (the phase-133 fallback class or the phase-140 pictorial +class). PUBLIC — the image route's phase-124 A4 trust model verbatim +(a shared chat can cite a page figure, so the bytes serve +ANONYMOUSLY; the unguessable ``uuid4`` document id IS the credential — +the document's TEXT stays user-gated). ``image/png`` + +``Cache-Control: private, max-age=3600`` (the image route's header); +404 ``document not found`` for EVERY non-servable case (an +unparseable id, an unknown doc, ``page_count`` NULL, ``n`` outside +``1..page_count``, or a copy lost on disk). The path is built from the +ROW's id + ``n`` only — no request value reaches a filesystem path. + PATCH /api/folders/summary — the admin folder-description editor (phase 97, task 03): update / create / clear a stored ``folder_summaries`` row, marking every non-empty save @@ -228,6 +242,18 @@ def get_document_content( ), origin_url=origin[0] if origin else None, origin_branch=origin[1] if origin else None, + # Phase 148 (task 02): the rendered-page BYTES routes for a + # page-qualifying PDF — the exact 1-based list over + # ``1..page_count`` (the viewer's strip source, task 03; a + # per-page render failure leaves a GAP the route 404s). ABSENT + # (never null — the ``image_url`` omission rule) for every + # other document, so its payload is byte-identical to + # pre-phase. + page_urls=( + [f"/api/documents/{doc.id}/pages/{i}" for i in range(1, doc.page_count + 1)] + if doc.page_count is not None + else None + ), ) @@ -290,6 +316,69 @@ def get_document_image( ) +@router.get("/documents/{doc_id}/pages/{n}", response_class=FileResponse) +def get_document_page( + doc_id: str, + n: int, + db: Session = Depends(get_db), # noqa: B008 +) -> FileResponse: + """The phase-148 (task 02) PDF page BYTES route — ONE rendered page + PNG (``<doc-id>-p{N}.png`` in the importer's ``pdf_page_dir`` copy, + task 01) of a page-QUALIFYING document. + + PUBLIC (the phase-124 A4 trust model verbatim — the image route's + precedent): a shared chat can cite a page FIGURE (the viewer's + strip, task 03), so the bytes serve ANONYMOUSLY. The document id + is an unguessable server-minted ``uuid4`` — IT IS the credential, + with no enumeration value — exactly like the image route and the + saved-chat id (phase 55). The document's TEXT stays user-gated (the + content endpoint): only the page bytes are public. + + 404 ``document not found`` (the router's unknown-document shape) in + EVERY non-servable case — an unparseable ``doc_id`` (the + ``uuid.UUID`` try/except maps here, not to a 422 — the image + route's mapping: a guessed id is an unknown document), an unknown + doc, a doc whose ``page_count`` is NULL (a non-qualifying doc — the + route knows nothing of its pages), ``n`` outside ``1..page_count`` + (out of range — a GAP, a page whose render failed, hits the + missing-file arm below), or a file on disk that is missing + (defensive — the row exists, the bytes don't; the viewer's onerror + note covers it). + + ``n`` is an ``int`` path parameter (FastAPI parses it — a non-int + 422s at the router), and the path is built ONLY from the row's id + + ``n`` (``pdf_page_dir/<doc.id>-p{n}.png``) — no request value + reaches a filesystem path (no traversal surface, the image route's + property). + + Servable pages stream the exact stored bytes as ``image/png`` + (``render_pdf_pages``'s ``tobytes("png")`` — a page is always a + PNG) with ``Cache-Control: private, max-age=3600`` (the image + route's header — the bytes are content-addressed by the document + id + page number, bustable by re-index). + """ + try: + uid = uuid.UUID(doc_id) + except ValueError: + raise HTTPException(status_code=404, detail="document not found") from None + doc = db.scalar(select(Document).where(Document.id == uid)) + if doc is None or doc.page_count is None: + raise HTTPException(status_code=404, detail="document not found") + if not 1 <= n <= doc.page_count: + raise HTTPException(status_code=404, detail="document not found") + page_file = Path(get_settings().pdf_page_dir).expanduser() / f"{doc.id}-p{n}.png" + if not page_file.is_file(): + # Defensive: the row qualifies but the copy was lost (the owner + # cleaned the page dir, the disk was wiped) — the viewer's + # onerror note renders "Page N unavailable". + raise HTTPException(status_code=404, detail="document not found") + return FileResponse( + page_file, + media_type="image/png", + headers={"Cache-Control": "private, max-age=3600"}, + ) + + @router.patch("/documents/summary", response_model=SummaryResult) async def update_document_summary( payload: SummaryUpdate, diff --git a/app/config.py b/app/config.py index 51b4503..82555f0 100644 --- a/app/config.py +++ b/app/config.py @@ -395,6 +395,24 @@ class Settings(BaseSettings): #: checkouts, re-cloned) and ``upload_dir`` (replaced on every #: upload): the served copy must outlive the source file. image_dir: str = "~/bor-sources/images" + #: Where rendered PDF PAGE PNGs are copied for serving (phase 148, + #: ``BOR_PDF_PAGE_DIR`` — the ``image_dir`` convention): a + #: page-qualifying PDF (the phase-133 fallback class — unusable + #: text layer — or the phase-140 pictorial class — the sparse-page + #: pass described ≥1 page) gets EVERY page rendered at index time + #: to ``<doc-id>-p{N}.png`` here (N 1-based; ``documents.page_count`` + #: is the page count — a per-page render failure leaves a gap), + #: so the document viewer shows the ACTUAL pages instead of the + #: page descriptions (TODO L5 — the IKEA assembly-instruction + #: class: the descriptions help the embeddings, "but not when + #: shown to the user"). Raw string — ``Path.expanduser()`` is + #: applied by the importer, not here (the ``sources_dir``/ + #: ``upload_dir``/``image_dir`` convention). Deliberately separate + #: from ``sources_dir``/``upload_dir``: git checkouts are + #: re-cloned, uploads are replaced, local dirs are user-edited — + #: the served copy must outlive the source file (the ``image_dir`` + #: rationale). + pdf_page_dir: str = "~/bor-sources/pages" # --- Pictorial PDF pages (phase 140: sparse pages rendered + vision-described) --- #: Master switch for the pictorial PDF pass (phase 140, LOCKED A3, diff --git a/app/models.py b/app/models.py index 8813275..4b5b455 100644 --- a/app/models.py +++ b/app/models.py @@ -183,6 +183,21 @@ class Document(Base): #: embedding model embeds this alongside the vision description when #: the image is enriched (task 03). image_context: Mapped[str | None] = mapped_column(Text, default=None) + #: The number of rendered page PNGs persisted in + #: ``settings.pdf_page_dir`` (phase 148, task 01) as + #: ``<doc-id>-p{N}.png`` (N 1-based): the PDF's page count for one + #: page-QUALIFYING document — the phase-133 fallback class (unusable + #: text layer — the screenshot class) or the phase-140 pictorial + #: class (the sparse-page pass described ≥1 page — the IKEA + #: assembly-instruction class). NULL for every other document + #: (text documents, images, text-heavy PDFs) and for a qualifying + #: PDF whose render/write failed (fail-soft — the render is a + #: display concern, it never gates indexing). Set/cleared ONLY by + #: the importer's PDF branch (a per-page render failure leaves a + #: GAP — the file is absent, the page route 404s that N); the prune + #: 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) 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 355577a..723970d 100644 --- a/app/rag/importer.py +++ b/app/rag/importer.py @@ -177,6 +177,29 @@ counters ``pdfs_pictorial`` / ``pdf_pictorial_pages`` are log-only (the sync ``detail`` API shape is untouched, the stats log-only convention). +Persistent page copies (phase 148, task 01): in the PDF branch's +NEW/CHANGED path — after the final content is decided (the +fallback/pictorial passes have run) and after the row's flush (the +doc has its id) — a page-QUALIFYING PDF (the phase-133 fallback +fired, or the phase-140 pictorial pass described ≥1 page — the +sync's own decisions: text-heavy PDFs stay ineligible, and the +``pdf_pictorial`` kill switch keeps its semantics) gets EVERY page +rendered at index time (:func:`app.rag.pdf.render_pdf_pages` — 150 +dpi, every page) and persisted to ``settings.pdf_page_dir`` +(``BOR_PDF_PAGE_DIR``, the ``image_dir`` convention) as +``<doc-id>-p{N}.png`` (N 1-based), with ``documents.page_count`` +(migration 0026) recording the page count — a per-page render +failure leaves a GAP (no file — the route 404s that N, the viewer's +note covers it). The existing copies are deleted before replacement +(a shrunk PDF's old ``p{N}`` tail cannot leak), a document that +STOPS qualifying is cleared (``page_count`` NULL + copies deleted), +and the prune deletes the copies with the row +(: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). + Image-anchor context (phase 125, task 03, LOCKED A1/A2/A3): BEFORE the description, the same source's importable markdown files are scanned for ``![alt](path)`` references that name the image (the task-02 @@ -851,6 +874,7 @@ async def import_sources( summary.pruned = _prune( session, source_names, seen, images=llm.settings.images, summary=summary, + page_dir=Path(llm.settings.pdf_page_dir).expanduser(), ) summary.embed_batches = llm.embed_batches summary.log() @@ -1413,7 +1437,19 @@ 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). + sync takes the fast path). This path renders NO pages and + touches NO ``page_count`` (the copies persist — zero render + work). + * Phase 148 (task 01): in this NEW/CHANGED path, RIGHT AFTER the + row's flush and BEFORE the chunk pipeline, a page-QUALIFYING + PDF (the phase-133 fallback fired, or the phase-140 pictorial + pass described ≥1 page — the signals recorded above) gets its + every page rendered to ``settings.pdf_page_dir`` as + ``<doc-id>-p{N}.png`` and ``documents.page_count`` set + (:func:`_persist_pdf_pages` — stale copies deleted first, a + stopped-qualifier cleared, a per-page ``b""`` a gap). FAIL-SOFT + — a render/write exception leaves ``page_count`` NULL + a + warning; the document indexes exactly as pre-phase. * THEN the embedded images: :func:`_index_pdf_embedded_images` (A3 — the phase-122 pipeline, gated by the ``images`` toggle). @@ -1435,12 +1471,25 @@ async def _index_pdf_file( # Extract the text (fail-soft: an unopenable PDF yields "" — the # document may still carry embedded images worth indexing). content = extract_pdf_text(full_path) + # Phase 148 (task 01): the page-strip ELIGIBILITY signals — the + # LOCKED rule, recorded where the sync makes the decision: a PDF + # gets rendered pages iff the sync ALREADY decided its text is not + # the useful view — the phase-133 fallback fired (unusable text + # layer — the screenshot class) OR the phase-140 pictorial pass + # described ≥1 page (the sparse-page class — the IKEA fixture). + # Text-heavy PDFs (usable layer, zero candidates) stay ineligible + # (the text view), and ``pdf_pictorial=0`` keeps its kill-switch + # semantics (no pictorial pass → the sparse class stays ineligible + # — the pre-phase-148 viewer). + fallback_fired = False + pictorial_described = False # Phase 133 (task 03): the garbage gate — BEFORE the digest/chunk/ # summary pipeline, so it runs on the FINAL content exactly once. # A clean layer never reaches the fallback (the gate is the only # gate — no fallback vision calls on the happy path). if not is_usable_pdf_text(content): summary.pdfs_screenshotted += 1 + fallback_fired = True content = await _fallback_page_screenshots( session, source=source, rel=rel, full_path=full_path, llm=llm ) @@ -1457,6 +1506,7 @@ async def _index_pdf_file( source=source, rel=rel, full_path=full_path, llm=llm, summary=summary ) if passed is not None: + pictorial_described = True content = passed if not content: logger.warning( @@ -1515,6 +1565,35 @@ async def _index_pdf_file( session.flush() # guarantees doc.id even for brand-new rows + # Phase 148 (task 01): the PERSISTENT PAGE COPIES — a qualifying + # PDF (the eligibility signals above) gets every page rendered to + # ``settings.pdf_page_dir`` (``<doc-id>-p{N}.png``) and + # ``documents.page_count`` set. RIGHT AFTER the flush (the doc has + # 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. + try: + _persist_pdf_pages( + doc, + full_path=full_path, + page_dir=Path(llm.settings.pdf_page_dir).expanduser(), + eligible=fallback_fired or pictorial_described, + source=source, + rel=rel, + ) + except Exception as e: + doc.page_count = None + logger.warning( + "import: PDF page render failed source=%s path=%s — %s", + source, rel, e, + ) + # Phase 1+2 — the shared chunk/replace/embed pipeline (the page- # aware chunker dispatches on the ``.pdf`` suffix; empty content # clears the chunk rows and returns 0), committed atomically with @@ -1757,6 +1836,91 @@ async def _pictorial_pdf_pass( return "\n\n".join(parts) +def _persist_pdf_pages( + doc: Document, + *, + full_path: Path, + page_dir: Path, + eligible: bool, + 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). + + * 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, + :func:`render_pdf_pages`' default — the phase-133 constant; the + ``pages=None`` "list index *i* is page *i*+1" contract) and each + NON-EMPTY PNG is written to *page_dir* as + ``<doc-id>-p{N}.png`` (N 1-based), then ``doc.page_count`` is set + to the PDF's page count (``len(pages)`` — one slot per page, so + the list IS the page count for an openable file). A per-page + ``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). + + *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: + doc.page_count = 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). + logger.warning( + "import: PDF page render produced no pages source=%s path=%s", + source, rel, + ) + doc.page_count = None + return + page_dir.mkdir(parents=True, exist_ok=True) + written: list[Path] = [] + 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) + 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: + path.unlink(missing_ok=True) + raise + doc.page_count = len(pages) + + async def _index_pdf_embedded_images( session: Session, *, @@ -2800,6 +2964,28 @@ def _delete_image_copy(image_path: str | None) -> None: logger.warning("import: could not delete image copy %s — %s", image_path, e) +def _delete_page_copies(doc: Document, page_dir: Path) -> None: + """Best-effort removal of one document's stale PDF page copies + (phase 148, task 01 — the :func:`_delete_image_copy` convention): + the ``<doc-id>-p{N}.png`` files for N in 1..``doc.page_count`` — + the row knows its own extent (a NULL/zero count is a no-op, which + covers fresh docs, non-qualifying docs, and every fail-soft miss). + + A missing file is a silent no-op (already gone — e.g. the owner + cleaned the pages dir); an unreadable one is logged, never raised — + copy cleanup must not break the sync (the doc row's fate is decided + by the upsert/prune logic, not by filesystem hygiene). + """ + if not doc.page_count: + return + for n in range(1, doc.page_count + 1): + 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_image_file( session: Session, *, @@ -3020,6 +3206,7 @@ def _prune( seen: set[tuple[str, str]], images: bool = False, summary: ImportSummary | None = None, + page_dir: Path | None = None, ) -> int: """Delete documents of *source_names* whose file is no longer in *seen*. @@ -3050,6 +3237,14 @@ def _prune( appended BEFORE the ``session.delete`` — so the sync's incremental folder-summary regeneration (phase 127) sees the pruned docs' folders too. Callers passing nothing keep the pre-phase count-only contract. + + *page_dir* (phase 148, task 01): when given (the sync's + ``Path(settings.pdf_page_dir).expanduser()``), every pruned doc's + PDF page copies (1..``doc.page_count`` — a NULL count is a no-op, + covering non-qualifying docs) are deleted beside the row and the + image copy — no orphans when a page-qualifying PDF (its parent or + a standalone doc) leaves the source. Callers passing nothing keep + the pre-phase copy-cleanup contract. """ if not source_names: return 0 @@ -3069,6 +3264,11 @@ def _prune( # invisible to the walk, not deleted. continue _delete_image_copy(doc.image_path) + # Phase 148 (task 01): the page-qualifying doc's page + # copies go with the row (a pruned parent PDF's copies too + # — the same site as its embedded-image copies' deletion). + if page_dir is not None: + _delete_page_copies(doc, page_dir) if summary is not None: summary.changed_paths.append((doc.source, doc.path)) session.delete(doc) diff --git a/app/schemas.py b/app/schemas.py index 2acdd6e..ba01c7b 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -602,6 +602,19 @@ class DocContent(BaseModel): #: repository's default branch — nothing to show); also dropped by #: the serializer (never ``null``). origin_branch: str | None = None + #: Phase 148 (task 02) — the rendered-page BYTES routes for a + #: page-QUALIFYING PDF (the phase-133 fallback class or the + #: phase-140 pictorial class): ``/api/documents/<id>/pages/<i>`` + #: for ``i`` in 1..``page_count`` (1-based — the importer's + #: ``<doc-id>-p{N}.png`` copy names; a per-page render failure + #: leaves a GAP the route 404s — the viewer's note covers it). The + #: viewer's page strip (task 03) renders from it; the document's + #: text stays in the DB for Q&A, it is simply not displayed. ABSENT + #: from the wire for every OTHER document — text docs, images, + #: text-heavy PDFs (``None`` → dropped by the serializer — never + #: ``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 @model_serializer(mode="wrap") def _serialize(self, handler: SerializerFunctionWrapHandler) -> Any: @@ -612,6 +625,8 @@ class DocContent(BaseModel): data.pop("origin_url", None) if self.origin_branch is None: data.pop("origin_branch", None) + if self.page_urls is None: + data.pop("page_urls", None) return data diff --git a/frontend/assets/document.js b/frontend/assets/document.js index d9fb3e0..0c0df47 100644 --- a/frontend/assets/document.js +++ b/frontend/assets/document.js @@ -36,8 +36,10 @@ * * XSS-safe by construction: markdown is escaped before transform, raw * formats are set via textContent, and every document-derived string - * (title, badges, path) is written with textContent — never innerHTML - * (innerHTML goes through renderMarkdown, which escapes first). + * (title, badges, path — and the phase-148 page strip's alt text and + * captions) is written with textContent or a property — never + * innerHTML (innerHTML goes through renderMarkdown, which escapes + * first). * * Phase 19: the viewer joins the shared header (assets/header.js) — the * whoami fetch is the module's cached promise (one request per page, @@ -134,6 +136,29 @@ * AND /document.html render it (no per-surface copy); createElement * + properties/textContent only (the never-build-HTML rule — no * innerHTML). + * + * Phase 148 (task 03): the page strip — when the payload carries + * page_urls (a page-qualifying PDF — the phase-133 fallback class + * or the phase-140 pictorial class — the task-02 endpoint's exact + * 1-based /api/documents/{id}/pages/{n} route list; ABSENT on the + * 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 <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 + * Q&A (the content/embeddings/summaries are untouched); they are + * simply not shown to the user — so the Summary panel is NOT built + * (its admin edit affordance is moot without it). A page whose copy + * was lost (the route's defensive 404s) swaps ONLY ITS figure's + * <img> for a "Page N unavailable." note (role=status — the + * phase-122 image-unavailable idiom, the same muted class); the rest + * of the strip is untouched (one lost copy never hides the other + * pages). ONE shared core — the modal AND /document.html get it (no + * per-surface copy); createElement + properties/textContent only (the + * never-build-HTML rule — no innerHTML). */ import { bindSharedHeaderControls, fetchIsAdmin, initSharedHeader } from "./header.js"; @@ -257,6 +282,24 @@ export function renderDocument(doc, { titleEl, metaEl, contentEl }) { }); contentEl.replaceChildren(); + // 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 + // 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 + // importer's content/embeddings/summary are untouched); they are + // simply not shown to the user (the owner's contract). The meta + // badge row above renders exactly as pre-phase. null / absent / + // empty page_urls falls through to the EXACT pre-phase rendering + // 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) { + contentEl.appendChild(docPagesStrip(doc, pageUrls)); + return; + } // 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 @@ -343,6 +386,56 @@ function docImageBlock(doc) { return wrap; } +/* ---------- page strip (phase 148, task 03) ---------- + * The viewer's page strip for a page-qualifying PDF (doc.page_urls + * non-empty — the task-02 payload: one + * /api/documents/{id}/pages/{n} route per rendered page, 1-based). + * A .doc-pages container goes into the content slot INSTEAD of the + * text content and the Summary panel (the renderDocument branch + * above) — the owner's contract: the per-page descriptions stay in + * the DB for Q&A, they are just not displayed. 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, the phase-122 image idiom) + a "Page N" + * <figcaption class="doc-page-num">. A page whose copy was lost + * (the route's defensive 404) swaps ONLY ITS figure's <img> for a + * small "Page N unavailable." note (role=status — the phase-122 + * image-unavailable note class, the same muted styling); the rest of + * the strip is untouched (one lost copy never hides the other pages). + * createElement + properties/textContent only — every document- + * derived string (the title in the alt, the note) lands via a + * property or textContent, never innerHTML (the XSS contract, + * unchanged). */ +function docPagesStrip(doc, pageUrls) { + const wrap = document.createElement("div"); + wrap.className = "doc-pages"; + pageUrls.forEach((url, i) => { + const n = i + 1; // 1-based over page_urls (the route list's own numbering) + const figure = document.createElement("figure"); + figure.className = "doc-page"; + const img = document.createElement("img"); + img.loading = "lazy"; // lazy imgs — a long manual streams down the strip + img.src = url; + img.alt = `Page ${n} of ${doc.title}`; + img.addEventListener("error", () => { + // The phase-122 idiom (its muted note class, reused): the lost + // copy's <img> becomes a small "Page N unavailable." note in + // ITS figure only — the caption and every other page stand. + const note = document.createElement("p"); + note.className = "doc-image-unavailable"; + note.setAttribute("role", "status"); + note.textContent = `Page ${n} unavailable.`; + img.replaceWith(note); + }); + const figcaption = document.createElement("figcaption"); + figcaption.className = "doc-page-num"; + figcaption.textContent = `Page ${n}`; + figure.append(img, figcaption); + wrap.appendChild(figure); + }); + return wrap; +} + /* ---------- summary editing (phase 57, task 02 — D4, admin-only) ---------- * The .doc-summary panel is the ONE place the stored summary is edited * (page + modal through this core). Only an admin (docAdminReady) ever diff --git a/frontend/assets/styles.css b/frontend/assets/styles.css index 398bf77..d1710fd 100644 --- a/frontend/assets/styles.css +++ b/frontend/assets/styles.css @@ -4778,13 +4778,56 @@ a.answer-image:hover { text-decoration: underline; } /* flat — the caption und } /* The onerror fallback (the route's 404 — the row exists but the copy was lost): a muted note in the card's place-of-image; the - description below still renders, so the doc stays readable. */ + description below still renders, so the doc stays readable. Phase + 148 (task 03) reuses THIS class for the page strip's lost-page + note (the same muted styling — no new pattern). */ .doc-image-unavailable { margin: 1.25rem 1rem; color: var(--ink-soft); /* 5.1:1 on --surface (AA) */ font-size: 0.9rem; } +/* Page strip (phase 148, task 03): the viewer's rendered-page strip + for a page-qualifying PDF — the payload's page_urls (task 02's + exact 1-based route list) replaces the text content AND the + .doc-summary panel (the owner's contract: the per-page + descriptions stay in the DB for Q&A, just not shown to the user). + The core renders it into the SAME #doc-content slot the .doc-raw / + .doc-md paths use — no new column — inside the existing doc + content column: --chat-column (72rem — the .container width, the + .doc-md / .doc-image language) at every viewport. A vertical stack + (one page per row — the natural reading order for instruction + manuals); each figure carries the page PNG on the non-clashing + --surface with the line border + radius (the .doc-image figure + precedent — a page PNG must sit on a surface in both themes) and a + centered muted "Page N" caption. Static content — no animation + (nothing for prefers-reduced-motion to still); the lazy imgs + alt + text (WCAG) carry the accessibility. Phase-08 tokens only (no new + hue — the monochrome invariant). */ +.doc-pages { + display: grid; + gap: 1rem; + width: 100%; + max-width: var(--chat-column); + margin-inline: auto; +} +.doc-page { margin: 0; } +.doc-page img { + display: block; + width: 100%; + max-width: 100%; + height: auto; + border: 1px solid var(--line); + border-radius: var(--radius); + background: var(--surface); +} +.doc-page-num { + font-size: 0.78rem; + color: var(--ink-soft); /* 8.2:1 on --surface, ~9.7:1 on --bg (AA in both themes) */ + text-align: center; + padding: 0.35rem 0 0; +} + /* 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 new file mode 100644 index 0000000..c8106db --- /dev/null +++ b/tests/e2e/test_pdf_pages_in_sources.py @@ -0,0 +1,775 @@ +"""Phase 148 task 04 E2E (Playwright, mock-only): the pictorial PDF +opens as its PAGES — a text-heavy PDF renders byte-identical to +pre-phase. + +Run in isolation (DB must be up: ``podman compose up -d db``): + + uv run pytest tests/e2e/test_pdf_pages_in_sources.py -v --no-cov + +The dedicated suite for ``148_pdf_pages_in_sources`` (A16 — one +Playwright file per phase, run in isolation): the owner's TODO L5 +scenario end to end on SYNTHETIC fixtures in the owner's named +``ikea_instructions.tar``'s exact CLASS (the real fixture is pinned at +the integration level — the E2E keeps the deterministic canned-reply +premise). Two PyMuPDF PDFs (the house recipe) land in a LOCAL +DIRECTORY source (``BOR_SOURCES_DIR``-sibling temp dir, API-registered +— local dirs are the owner's named case) and one real sync indexes +them: + +* ``pictorial.pdf`` — the IKEA shape (the pictorial class): page 1 a + dense prose paragraph (over the 100-char sparseness bar — NOT a + candidate; the whole layer passes ``is_usable_pdf_text``), pages + 2–3 SPARSE (a short marker + a drawn VECTOR figure — rectangle + + arrow, <100 stripped chars — the ``pictorial_pages(texts, + min_chars=100)`` candidates the mock's vision describes with the + canned ``PICTORIAL_PAGE_ANSWER``); +* ``textheavy.pdf`` — 2 dense text pages (ZERO candidates — the + negative class: the text view, the pre-phase rendering). + +App env: the conftest pattern with the phase-140 opt-in (``BOR_PDF_ +PICTORIAL`` pinned EXPLICITLY ``1`` — the conftest pins it off in +``os.environ`` for the pre-phase-140 suites) + the phase-148 +``BOR_PDF_PAGE_DIR`` pointed at a suite-private scratch dir (the seed +asserts the copies THERE; the owner's ``~/bor-sources/pages`` default +is never touched). ``BOR_IMPORT_EXTENSIONS`` carries the ``pdf`` +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. + +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. +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. +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 + PNG body byte-equal to the stored copy (the public + uuid4-credential bytes route, from the UI's perspective). + +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 four tests SHARE the module seed +(one source, two documents, one sync), 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 +from urllib.parse import quote + +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, SESSION_SECRET, USE_REAL_LLM, _wait_http +from e2e.mock_llm import PICTORIAL_PAGE_ANSWER + +REPO = Path(__file__).resolve().parents[2] + +#: The local directory's NAME — the source name the registry stores +#: for a local-kind source (the directory basename, the +#: test_local_directory_sources convention). +SOURCE_NAME = "e2e-p148-pdf" +PICTORIAL_PATH = "pictorial.pdf" +TEXTHEAVY_PATH = "textheavy.pdf" +PICTORIAL_TITLE = "Assembly Instructions" # the metadata title +TEXTHEAVY_TITLE = "Dense Notes" # the metadata title + +#: Page 1 — the dense prose paragraph (over the 100-char sparseness +#: bar — NOT a pictorial candidate; the whole layer is USABLE). +PAGE_ONE = ( + "This manual covers the complete flat-pack furniture frame, its " + "shipped hardware kit, and the safety checks a first-time assembler " + "should run before starting. The box holds the two side panels, the " + "top board, the adjustable leg set, eight cam locks, fourteen wood " + "screws, two bolts, and the wall-anchoring strap. Work on a flat " + "surface with soft flooring, keep the hardware sorted by size, and " + "read every step through before you begin so you know which panel " + "is which. The frame stands roughly a metre tall once assembled, " + "weights under twelve kilograms, and needs no further tools beyond " + "the included Allen key." +) +assert len(PAGE_ONE) > 500, "page 1 must be a 500+ char paragraph (the task's recipe)" + +#: The sparse pages' markers — ALL the text layer carries on pages +#: 2–3 (6 stripped chars, under the 100-char bar — the candidates); +#: the figures are drawn vector (no raster image). +PAGE_TWO_MARKER = "Fig. 2" +PAGE_THREE_MARKER = "Fig. 3" + +#: The stored illustration blocks (the phase-140 LOCKED A4 shape) — +#: the mock's canned ``PICTORIAL_PAGE_MODE`` reply, deterministic. +BLOCK_TWO = f"[Page 2 illustration: {PICTORIAL_PAGE_ANSWER}]" +BLOCK_THREE = f"[Page 3 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_PDFPAGES", "8161")) +APP_URL = f"http://127.0.0.1:{APP_PORT}" + + +# --------------------------------------------------------------------------- +# Fixture builder (the house PyMuPDF recipe — the IKEA class: a dense +# prose page + sparse vector-illustrated pages) +# --------------------------------------------------------------------------- + + +def _draw_illustration(page: Any) -> None: + """The IKEA pictogram class: a rectangle (a part) + a line arrow + with a two-stroke head (a join) — vector-drawn, so + ``get_image_info()`` sees ZERO embedded raster images (the + phase-122/137 pipeline is blind to it).""" + 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_pictorial(path: Path) -> None: + """The IKEA shape: page 1 = the dense prose paragraph + (``insert_textbox`` — word-wrapped), pages 2–3 = a short marker + + the drawn vector figure (sparse — the pictorial candidates).""" + doc = fitz.open() + page1 = doc.new_page() + rc = page1.insert_textbox(fitz.Rect(72, 72, 523, 772), PAGE_ONE, fontsize=11) + assert rc >= 0, "the page-1 paragraph must fit its text box" + for marker in (PAGE_TWO_MARKER, PAGE_THREE_MARKER): + p = doc.new_page() + p.insert_text((72, 72), marker) + _draw_illustration(p) + doc.set_metadata({"title": PICTORIAL_TITLE}) + doc.save(path) + doc.close() + + +def _build_textheavy(path: Path) -> None: + """The negative class: two DENSE text pages (zero candidates — the + text view, byte-identical to pre-phase).""" + doc = fitz.open() + for i in (1, 2): + p = doc.new_page() + rc = p.insert_textbox( + fitz.Rect(72, 72, 523, 772), f"Paragraph {i}: {PAGE_ONE}", fontsize=11 + ) + assert rc >= 0, f"the page-{i} paragraph must fit its text box" + doc.set_metadata({"title": TEXTHEAVY_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: both fixture PDFs, built + ONCE (module-scoped). The directory's NAME is the source name + (a ``kind='local'`` registry row indexes under the directory's + basename — the test_local_directory_sources convention), so it + is a named subdirectory, not the mktemp root (whose name carries + a random suffix). The premise pins (deterministic — computed from + the fixtures THEMSELVES, no hardcoded page list): the pictorial + PDF is usable-layer with candidates [2, 3] (3 pages — the seed's + ``page_count`` target); the textheavy PDF is usable-layer with + ZERO candidates.""" + root = tmp_path_factory.mktemp("bor_p148_pdf") / SOURCE_NAME + root.mkdir(parents=True) + min_chars = Settings.model_fields["pdf_pictorial_min_chars"].default + pic = root / PICTORIAL_PATH + heavy = root / TEXTHEAVY_PATH + _build_pictorial(pic) + _build_textheavy(heavy) + pic_texts = extract_pdf_page_texts(pic) + heavy_texts = extract_pdf_page_texts(heavy) + assert is_usable_pdf_text(extract_pdf_text(pic)), ( + "the pictorial fixture must PASS the usability gate (the IKEA " + "class — the fallback class is the other qualifying class, not " + "this one)" + ) + assert pictorial_pages(pic_texts, min_chars=min_chars) == [2, 3], ( + f"the pictorial fixture's sparse-page premise broke: " + f"{pictorial_pages(pic_texts, min_chars=min_chars)}" + ) + assert is_usable_pdf_text(extract_pdf_text(heavy)) + assert pictorial_pages(heavy_texts, min_chars=min_chars) == [], ( + "the textheavy fixture must carry ZERO sparse pages (the " + "negative class)" + ) + # Every page renders to a real PNG (the pipeline's byte premise). + assert len(render_pdf_pages(pic)) == 3 + assert len(render_pdf_pages(heavy)) == 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_p148_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 seed asserts the ``<doc-id>-p{N}.png`` copies here.""" + 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 + IKEA class) and the phase-148 ``BOR_PDF_PAGE_DIR`` (suite-private + — the seed's file assertions target it, the owner's default is + never written).""" + 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) — the app boots with the + # same env shape the story suites use. + 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 fixture without it). + env["BOR_IMPORT_EXTENSIONS"] = "md,markdown,txt,pdf" + # Phase 140: the pictorial toggle EXPLICITLY on (the opt-in — see + # the module docstring; without it the whole premise is void). + env["BOR_PDF_PICTORIAL"] = "1" + # Phase 148: the rendered-page copies' home (the seed asserts the + # files here). + env["BOR_PDF_PAGE_DIR"] = str(page_dir) + # The embedded-image pipeline is out of scope (the fixtures' + # figures are vector-drawn — no image doc 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 env only needs the homes; the dir's existence is the seed's job + scratch: Path, + page_dir: Path, +) -> Iterator[str]: + """The app under test (all four tests): one module app with the + phase-140/148 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 helpers +# --------------------------------------------------------------------------- + + +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 — + the sync re-creates the row.""" + 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, and a still-running run would keep importing into + the NEXT test's truncated KB): 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 four tests SHARE the module seed's single source + + two documents, so there is NO per-test truncate (it would wipe + the seed from under the next test — the per-test pattern belongs to + the suites where every test imports its own source); instead the + shared KB is flushed ONCE, after the last test — no rows leak into + the next suite's run. The pre-wait keeps a leaked sync (the module + app is still up here) 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`` / phase-94 pattern — a plain + directory is an API/DB-only operation; the page form is gone).""" + 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; this process runs on the 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 _run_sync(base_url: str, 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_garbage_pdf`` pattern over + plain httpx — with the stale-terminal freshness gate: a terminal + is accepted only after a running tick was observed or its + ``started_at`` post-dates this POST).""" + 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 + ): + return body + time.sleep(0.1) + raise AssertionError(f"sync did not reach a terminal state: {body}") + + +# --------------------------------------------------------------------------- +# The module seed: register the local dir + one sync (both PDFs), then +# assert the DB rows and the rendered page copies +# --------------------------------------------------------------------------- + + +@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 + mock's vision model describes the two sparse pages in-process — + the ``PICTORIAL_PAGE_MODE`` canned reply). Asserts the phase-148 + index-time contract: the pictorial row qualifies (``page_count == + 3``, the canned blocks IN the stored content — the Q&A witness), + the textheavy row does not (``page_count`` NULL, no copies), and + the scratch page dir holds EXACTLY the pictorial doc's + ``<id>-p{1..3}.png`` (real PNGs, non-empty — no orphans, no + textheavy copies). Yields both docs' 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) + assert body["state"] == "success", body + detail = body["detail"] + assert detail["files"] == 2, detail + assert detail["added"] == 2, detail + assert detail["pruned"] == 0, detail + assert detail["errors"] == 0, detail + + with SessionLocal() as db: + pic = db.scalar( + select(Document).where( + Document.source == SOURCE_NAME, Document.path == PICTORIAL_PATH + ) + ) + heavy = db.scalar( + select(Document).where( + Document.source == SOURCE_NAME, Document.path == TEXTHEAVY_PATH + ) + ) + assert pic is not None, "the pictorial PDF must become a document" + assert heavy is not None, "the textheavy PDF must become a document" + # The pictorial class (IKEA shape): qualifies. The metadata + # title (LOCKED A2); a TEXT document (no image doc, no image + # path). + assert pic.title == PICTORIAL_TITLE + assert pic.is_image is False and pic.image_path is None + assert pic.page_count == 3, ( + f"the qualifying PDF's page_count must be its page count: {pic.page_count}" + ) + # The Q&A witness: the described pages ARE in the stored + # content (the embeddings/summary ride on them — the strip + # just does not display them). + assert "---PAGE-1---" in pic.content and "---PAGE-3---" in pic.content + assert BLOCK_TWO in pic.content and BLOCK_THREE in pic.content + assert pic.summary, "the qualifying doc still gets its lite summary" + # The negative class: NO strip (the pre-phase-148 viewer). + assert heavy.title == TEXTHEAVY_TITLE + assert heavy.page_count is None, ( + f"a text-heavy PDF must not qualify: page_count={heavy.page_count}" + ) + assert heavy.summary, "the textheavy doc still gets its lite summary" + pic_id, heavy_id = str(pic.id), str(heavy.id) + + # The filesystem contract: EXACTLY the qualifying doc's three + # copies (no textheavy copies, no orphans) — real, non-empty PNGs. + pngs = sorted(page_dir.glob("*.png")) + assert [p.name for p in pngs] == [ + f"{pic_id}-p1.png", + f"{pic_id}-p2.png", + f"{pic_id}-p3.png", + ], f"the page dir holds {sorted(p.name for p in pngs)}" + for png in pngs: + data = png.read_bytes() + assert data[:8] == PNG_MAGIC and len(data) > 0, f"{png.name} is not a real PNG" + assert not list(page_dir.glob(f"{heavy_id}-*")), ( + "the non-qualifying PDF must not have page copies" + ) + yield { + "pictorial_id": pic_id, + "textheavy_id": heavy_id, + "page_dir": page_dir, + "title": PICTORIAL_TITLE, + } + + +# --------------------------------------------------------------------------- +# 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 — both 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) -> None: + """The house doc-modal idiom: the row's path link opens the + same-page modal (no new tab).""" + 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" + expect(page.locator("#doc-modal")).to_be_visible(timeout=30_000) + + +def _wait_img_loaded(img: Locator) -> None: + """Wait until a LAZY ``<img>`` really loaded pixels: scroll it into + the viewport (a below-the-fold lazy img is not fetched yet), then + 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_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).""" + strip = content.locator(".doc-pages") + expect(strip).to_have_count(1) + 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") + # The figure still carries its <img> (a lost copy would have + # been swapped for the "Page N unavailable" note instead). + 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}") + # 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) + + +# --------------------------------------------------------------------------- +# 1. The pictorial PDF: the modal shows its three rendered pages +# --------------------------------------------------------------------------- + + +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 + ``.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.""" + login(page, app_url) # lands on /sources.html + _drill_to_source(page, SOURCE_NAME) + expect(page.locator("#docs-tbody tr")).to_have_count(2) + _open_doc_modal(page, PICTORIAL_PATH) + expect(page.locator("#doc-modal-title")).to_have_text( + seeded["title"] + ) + + content = page.locator("#doc-modal-content") + _assert_page_strip(content, seeded["title"], seeded["pictorial_id"]) + + # The meta badge row is untouched by the strip (the pre-phase row). + meta = page.locator("#doc-modal-meta") + expect(meta.locator(".doc-source-badge")).to_have_text(SOURCE_NAME) + 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 +# --------------------------------------------------------------------------- + + +def test_textheavy_pdf_unchanged( + page: Page, app_url: str, seeded: dict[str, Any] +) -> None: + """The zero-candidates PDF renders the PRE-PHASE shape: the + content in ``pre.doc-raw`` (both pages, verbatim), the Summary + panel (the mock's lite digest), NO ``.doc-pages`` strip — and the + payload's ``page_urls`` KEY is ABSENT from the content endpoint's + JSON (never ``null`` — the omission pin the viewer's strip gate + rides on; the non-qualifying doc's payload is byte-identical to + pre-phase).""" + login(page, app_url) # lands on /sources.html + _drill_to_source(page, SOURCE_NAME) + _open_doc_modal(page, TEXTHEAVY_PATH) + expect(page.locator("#doc-modal-title")).to_have_text(TEXTHEAVY_TITLE) + + content = page.locator("#doc-modal-content") + # No strip for the negative class… + expect(content.locator(".doc-pages")).to_have_count(0) + # …the pre-phase raw rendering (both pages, the stored content + # verbatim). + pre = content.locator("pre.doc-raw") + expect(pre).to_have_count(1) + expect(pre).to_contain_text("---PAGE-1---") + expect(pre).to_contain_text("Paragraph 1:") + expect(pre).to_contain_text("---PAGE-2---") + expect(pre).to_contain_text("Paragraph 2:") + # …and the Summary panel (a non-empty lite summary). + expect(content.locator(".doc-summary")).to_have_count(1) + expect(content.locator(".doc-summary-text")).not_to_be_empty() + + # The payload pin: the page_urls KEY is ABSENT (the admin cookie + # rides the browser context — the content endpoint is + # require_user-gated). + r = page.request.get( + f"{app_url}/api/documents/content", + params={"source": SOURCE_NAME, "path": TEXTHEAVY_PATH}, + ) + assert r.status == 200, r.text + assert "page_urls" not in r.json(), ( + f"the non-qualifying doc's payload must not carry page_urls: " + f"{r.json().get('page_urls')!r}" + ) + + +# --------------------------------------------------------------------------- +# 3. The full page surface: the SAME strip, no per-surface drift +# --------------------------------------------------------------------------- + + +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.""" + login(page, app_url, next="/") + page.goto( + f"{app_url}/document.html?source={quote(SOURCE_NAME)}" + f"&path={quote(PICTORIAL_PATH)}" + ) + expect(page.locator("#doc-title")).to_have_text(seeded["title"]) + _assert_page_strip( + page.locator("#doc-content"), seeded["title"], seeded["pictorial_id"] + ) + # The meta row on the full page is the pre-phase shape too. + expect(page.locator("#doc-meta .doc-source-badge")).to_have_text(SOURCE_NAME) + expect(page.locator("#doc-meta .doc-indexed")).to_contain_text("Indexed") + + +# --------------------------------------------------------------------------- +# 4. The page bytes route: public PNG with the image route's headers +# --------------------------------------------------------------------------- + + +def test_page_route_serves_png( + page: Page, app_url: str, seeded: dict[str, Any] +) -> None: + """An ANONYMOUS (no-login — this test's fresh context never signs + in) direct request on the first ``page_urls`` entry: 200, + ``image/png``, the ``Cache-Control: private, max-age=3600`` + header (the image route's header), a non-empty PNG body byte-equal + to the stored copy (the public uuid4-credential bytes route — the + document id IS the credential — from the UI's perspective).""" + doc_id = seeded["pictorial_id"] + r = page.request.get(f"{app_url}/api/documents/{doc_id}/pages/1") + assert r.status == 200, r.text + assert r.headers["content-type"] == "image/png", r.headers + assert r.headers["cache-control"] == "private, max-age=3600", r.headers + body = r.body() + assert body, "the page route served an empty body" + assert body[:8] == PNG_MAGIC, "the page route served non-PNG bytes" + # The exact stored bytes (the route streams the persistent copy). + stored = (seeded["page_dir"] / f"{doc_id}-p1.png").read_bytes() + assert body == stored, "the route's bytes are not the stored copy's bytes" diff --git a/tests/e2e/test_pictorial_pdf.py b/tests/e2e/test_pictorial_pdf.py index 2b2a878..5c817b0 100644 --- a/tests/e2e/test_pictorial_pdf.py +++ b/tests/e2e/test_pictorial_pdf.py @@ -24,13 +24,19 @@ 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 - renders the stored content: 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; the stored content is asserted BYTE-EXACT against - the fixture's own extraction in the seed — the candidates are computed - from ``extract_pdf_page_texts`` + ``pictorial_pages`` IN the test, no - hardcoded page list); + 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 + (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 + the DB: the seed asserts it BYTE-EXACT against the fixture's own + extraction (the candidates computed from + ``extract_pdf_page_texts`` + ``pictorial_pages`` IN the test, no + hardcoded page list) and the chat test below proves it still reaches + the model; * (b) a chat question that ONLY the illustration answers ("how are the two parts joined on page two?" — the phrase lives NOWHERE in the text layer) grounds (the described page's embedding retrieves — the mock's @@ -77,11 +83,14 @@ 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) renders the - stored content under the METADATA title: the page-1 prose (NO - ``[Page 1 illustration`` block), the page-2 marker + its - ``[Page 2 illustration: <canned>]`` block, and the Summary panel - (a non-image doc with a non-empty summary). + 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). 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 @@ -554,9 +563,14 @@ def seeded( "described sparse page — the pictorial pass misfired" ) # The non-markdown doc got its lite summary (the mock's - # SUMMARY_MODE digest) — the summary chunk + the viewer's - # Summary panel exist. + # 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). assert doc.summary + # Phase 148: the described sparse page makes this doc + # page-QUALIFYING — the strip's payload (page_urls) rides + # on page_count (the 2-page fixture's count). + assert doc.page_count == 2 finally: page.close() yield {"title": PDF_TITLE} @@ -574,12 +588,18 @@ 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) - renders the stored content under the METADATA title: page 1's prose - WITHOUT an illustration block (not a candidate) and page 2 WITH - ``[Page 2 illustration: <canned>]`` (the described sparse page — - LOCKED A4's user surface), plus the Summary panel (a non-image doc - with a non-empty summary).""" + 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).""" 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") @@ -596,27 +616,44 @@ 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) - raw = modal.locator("pre.doc-raw") - # Page 1 — the prose WITHOUT any illustration block. The viewer - # renders the content verbatim (``insert_textbox`` wraps mid- - # sentence, so compare whitespace-normalized). - expect(raw).to_contain_text("---PAGE-1---") - rendered = " ".join(raw.inner_text().split()) - assert " ".join(PAGE_ONE.split()) in rendered, ( - "the page-1 prose is not what the viewer renders" - ) - assert "[Page 1 illustration" not in rendered, ( - "page 1 (over the sparseness bar) must carry NO illustration block" - ) - # Page 2 — the sparse page WITH the described block (a) — the mock's - # canned reply, deterministic. - expect(raw).to_contain_text("---PAGE-2---") - expect(raw).to_contain_text(PAGE_TWO) - expect(raw).to_contain_text(ILLUSTRATION_BLOCK) - # The Summary panel renders (a non-image doc with a non-empty - # summary); no image block (the PDF is not an image document). - expect(modal.locator(".doc-summary")).to_have_count(1) + # 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). + strip = modal.locator(".doc-pages") + expect(strip).to_have_count(1) + figures = strip.locator(".doc-page") + expect(figures).to_have_count(2) + for i in range(2): + n = i + 1 + img = figures.nth(i).locator("img") + # The figure still carries its <img> (a lost copy would have + # been swapped for the "Page N unavailable" note instead). + expect(img).to_have_count(1) + expect(img).to_have_attribute("loading", "lazy") + expect(img).to_have_attribute("alt", f"Page {n} of {PDF_TITLE}") + img.scroll_into_view_if_needed() + img.evaluate("el => el.decode()") + assert img.evaluate("el => el.naturalWidth") > 0, ( + f"page {n} image did not load (the route must serve a real PNG)" + ) + 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) 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 dc9f76a..b554184 100644 --- a/tests/integration/test_docs_api.py +++ b/tests/integration/test_docs_api.py @@ -3,6 +3,15 @@ and GET /api/docs/tree (phase 97, task 02 — the full recursive KB tree the RAG view renders: shape, ordering, counts, summaries, the 403 gate, the superset rule, and the stat-walk equivalence with ``GET /api/docs``). +Also the BYTES routes and their content-payload affordances: the +phase-122 (task 04) image route (``GET /api/documents/{id}/image`` +— exact bytes, the extension's Content-Type, the 404 matrix, public +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``). + Uses the real compose Postgres (``db`` fixture) and FastAPI's TestClient. """ from __future__ import annotations @@ -1370,6 +1379,241 @@ def test_content_endpoint_exposes_the_image_affordance( _cleanup_kb(db) +# --------------------------------------------------------------------------- +# Phase 148 (task 02) — the PDF page BYTES route + the content payload's +# ``page_urls`` affordance +# --------------------------------------------------------------------------- + + +def _page_settings(pages_dir: Path) -> Settings: + """Fresh settings (the ``.env`` file ignored) with the phase-148 + ``pdf_page_dir`` pinned to *pages_dir* — the page route reads the + setting at request time, and the copies under test never land in + the real home (the ``get_settings`` monkeypatch pattern).""" + return Settings( + _env_file=None, # pyright: ignore[reportCallIssue] + pdf_page_dir=str(pages_dir), + ) + + +def _seed_page_doc( + db, + pages_dir: Path, + *, + source: str = "PdfSrc", + path: str = "besta.pdf", + page_count: int | None = 2, + write_copies: bool = True, +) -> Document: + """One PDF document row with ``page_count`` + its rendered page + copies (the importer's ``pdf_page_dir`` layout — + ``<doc-id>-p{N}.png``, N 1-based); page N is the ``PNG_1X1 + N`` + 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).""" + doc = Document( + id=uuid.uuid4(), + source=source, + path=path, + full_path=f"/tmp/{path}", + title=path.rsplit(".", 1)[0], + content="[Page 1 illustration: the parts tray] [Page 2 illustration: step 1]", + content_hash=hashlib.sha256(path.encode("utf-8")).hexdigest(), + indexed_at=datetime.now(UTC), + created_at=datetime(2026, 1, 1, tzinfo=UTC), + summary="IKEA BESTA frame assembly manual (page descriptions).", + page_count=page_count, + ) + db.add(doc) + db.flush() + if write_copies and page_count: + pages_dir.mkdir(parents=True, exist_ok=True) + for n in range(1, page_count + 1): + (pages_dir / f"{doc.id}-p{n}.png").write_bytes(PNG_1X1 + str(n).encode("ascii")) + db.add_all( + [ + Chunk( + document_id=doc.id, position=0, content=doc.content, embedding=[0.01] * 768 + ), + Chunk( + document_id=doc.id, + position=-1, + content=doc.summary, + embedding=[0.01] * 768, + is_summary=True, + ), + ] + ) + db.commit() + return doc + + +def test_page_route_serves_exact_bytes_with_png_type( + admin_client: TestClient, db, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The serve contract: each page of a qualifying doc streams the + EXACT stored PNG bytes (a per-page sentinel — a page mix-up is + caught) as ``image/png`` (a page is always a PNG — + ``render_pdf_pages``'s ``tobytes("png")``) with + ``Cache-Control: private, max-age=3600`` (the image route's + header — content-addressed by the document id + page number). + The path is built ONLY from the row's id + ``n`` (the row + derives it; ``n`` is the int path parameter) — the copies + written under the tmp ``pdf_page_dir`` are what come back.""" + _cleanup_kb(db) + try: + pages_dir = tmp_path / "pages" + monkeypatch.setattr(docs_api, "get_settings", lambda: _page_settings(pages_dir)) + doc = _seed_page_doc(db, pages_dir, page_count=2) + for n in (1, 2): + r = admin_client.get(f"/api/documents/{doc.id}/pages/{n}") + assert r.status_code == 200, n + assert r.headers["content-type"] == "image/png" + assert r.headers["cache-control"] == "private, max-age=3600" + assert r.content == PNG_1X1 + str(n).encode("ascii") # the exact stored bytes + finally: + _cleanup_kb(db) + + +def test_page_route_404_matrix( + admin_client: TestClient, db, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Every non-servable case is 404 ``document not found`` (the + router's unknown-document shape — the same detail string the + content endpoint uses): a missing id, a MALFORMED id (an + unparseable string maps here, not to a 422 — the image route's + mapping: a guessed id is an unknown document), a text doc, an + image doc, a PDF row whose ``page_count`` is NULL (a + non-qualifying doc — the route knows nothing of its pages), + ``n`` outside ``1..page_count`` (``n=0``/``n=-1`` below the range + — the int parameter makes fractions impossible — and ``n=3`` + above a 2-page doc), and a qualifying row whose copy was lost on + disk (defensive — the row exists, the bytes don't; the viewer's + onerror note covers it). The int parameter is the no-traversal + pin: a NON-int ``n`` 422s at the router, and no request value + ever reaches a filesystem path (the out-of-range 404s show the + path is bounded by the ROW's extent).""" + _cleanup_kb(db) + try: + pages_dir = tmp_path / "pages" + monkeypatch.setattr(docs_api, "get_settings", lambda: _page_settings(pages_dir)) + _seed_doc(db, "TextSrc", "note.md", "Note", 1, datetime.now(UTC)) + db.commit() # the module's _seed_doc leaves the row uncommitted + text_doc_id = db.scalar( + select(Document.id).where(Document.source == "TextSrc", Document.path == "note.md") + ) + img = _seed_image_doc(db, tmp_path, path="pic.png") + plain = _seed_page_doc(db, pages_dir, path="texty.pdf", page_count=None) + lost = _seed_page_doc(db, pages_dir, path="lost.pdf") + assert (pages_dir / f"{lost.id}-p1.png").is_file() # the copies were written + (pages_dir / f"{lost.id}-p1.png").unlink() # the copy is lost (the row remains) + qualifying = _seed_page_doc(db, pages_dir, path="besta.pdf") + + for doc_id, n in ( + (str(uuid.uuid4()), 1), # missing id + ("abc", 1), # malformed id → 404, not 422 + (str(text_doc_id), 1), # text doc + (str(img.id), 1), # image doc + (str(plain.id), 1), # PDF, page_count NULL (non-qualifying) + (str(lost.id), 1), # qualifying doc, copy lost on disk + (str(qualifying.id), 0), # below range + (str(qualifying.id), -1), # negative — the int parse allows it, the range rejects it + (str(qualifying.id), 3), # above range (page_count 2) + ): + r = admin_client.get(f"/api/documents/{doc_id}/pages/{n}") + assert r.status_code == 404, (doc_id, n) + assert r.json() == {"detail": "document not found"}, (doc_id, n) + + # The int parameter (the no-traversal pin): a NON-int n is a + # router-level 422 — no other ``n`` shape reaches the handler. + r = admin_client.get(f"/api/documents/{qualifying.id}/pages/abc") + assert r.status_code == 422 + finally: + _cleanup_kb(db) + + +def test_page_route_is_public_like_the_shared_chat_content( + admin_client: TestClient, db, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The image route's phase-124 A4 trust model verbatim: a shared + chat can cite a page FIGURE (the viewer's strip, task 03), so the + BYTES serve ANONYMOUSLY — the unguessable ``uuid4`` document id IS + the credential, no enumeration value (PLAN A10: the shared chats + are the app's ONLY anonymous surface). An ANONYMOUS caller (a + FRESH client — no session) gets the page PNG, and a signed-in + caller gets the same. The document's TEXT (the content endpoint) + stays user-gated — only the page bytes are public.""" + _cleanup_kb(db) + try: + pages_dir = tmp_path / "pages" + monkeypatch.setattr(docs_api, "get_settings", lambda: _page_settings(pages_dir)) + doc = _seed_page_doc(db, pages_dir) + anonymous = TestClient(fastapi_app) + r = anonymous.get(f"/api/documents/{doc.id}/pages/1") + assert r.status_code == 200 + assert r.headers["content-type"] == "image/png" + assert r.content == PNG_1X1 + b"1" + # The signed-in client (admin) gets the same bytes. + r_admin = admin_client.get(f"/api/documents/{doc.id}/pages/1") + assert r_admin.status_code == 200 + assert r_admin.content == PNG_1X1 + b"1" + finally: + _cleanup_kb(db) + + +def test_content_endpoint_exposes_the_page_urls_affordance( + admin_client: TestClient, db, tmp_path: Path +) -> None: + """The content endpoint (the viewer's data source — task 03's + strip reads from it): a page-qualifying PDF (``page_count`` set) + carries ``page_urls`` = the EXACT 1-based route list over + ``1..page_count`` (the count == ``page_count`` — a per-page render + gap is still listed; the route 404s that N); text docs, image + docs, and non-qualifying PDFs (``page_count`` NULL) keep the key + ABSENT from the JSON — never ``null`` (the ``image_url`` + omission rule — a non-qualifying doc's payload is byte-identical + to pre-phase).""" + _cleanup_kb(db) + try: + pages_dir = tmp_path / "pages" + doc = _seed_page_doc(db, pages_dir, page_count=3) + r = admin_client.get( + "/api/documents/content", params={"source": "PdfSrc", "path": "besta.pdf"} + ) + assert r.status_code == 200 + body = r.json() + assert body["page_urls"] == [ + f"/api/documents/{doc.id}/pages/1", + f"/api/documents/{doc.id}/pages/2", + f"/api/documents/{doc.id}/pages/3", + ] # the exact 1-based list, count == page_count + + _seed_doc(db, "TextSrc", "note.md", "Note", 1, datetime.now(UTC)) + db.commit() # the app's endpoint session reads committed data only + r = admin_client.get( + "/api/documents/content", params={"source": "TextSrc", "path": "note.md"} + ) + assert r.status_code == 200 + assert "page_urls" not in r.json() # absent — never null (text doc) + + _seed_image_doc(db, tmp_path, source="ImgSrc", path="pic.png") + r = admin_client.get( + "/api/documents/content", params={"source": "ImgSrc", "path": "pic.png"} + ) + assert r.status_code == 200 + assert "page_urls" not in r.json() # absent — never null (image doc) + + _seed_page_doc(db, pages_dir, path="texty.pdf", page_count=None) + r = admin_client.get( + "/api/documents/content", params={"source": "PdfSrc", "path": "texty.pdf"} + ) + assert r.status_code == 200 + assert "page_urls" not in r.json() # absent — never null (non-qualifying PDF) + finally: + _cleanup_kb(db) + + def _tree_file_nodes_all(sources) -> list[dict]: """Every file node of a tree response, walked recursively (all sources — env-registered 0-document sources may join the response diff --git a/tests/integration/test_migration_0026.py b/tests/integration/test_migration_0026.py new file mode 100644 index 0000000..d9e3d56 --- /dev/null +++ b/tests/integration/test_migration_0026.py @@ -0,0 +1,351 @@ +"""Integration: migration 0026 (documents.page_count) schema contract +(phase 148, task 01). + +Drives the **real Alembic engine** against the live dev database +(``podman compose up -d db``), mirroring the house pattern of +``test_migration_0025.py`` (information_schema assertions on the state +the migration must leave). The tests target the 0025 → 0026 step +explicitly so later migrations cannot break the pins: + +* upgrade 0025 → 0026 → ``page_count`` exists with the full contract — + INTEGER, NULLABLE, NO server default (NULL = "not page-qualifying" + for every pre-phase row and every fail-soft miss) — and is UNINDEXED + (a display fact, never a query surface — the page route of task 02 + reads it from the ROW after the uuid4 id lookup, the ``file_hash`` + convention of 0025) while the 0025 ``documents`` schema survives — + in particular ``content_hash`` stays NOT NULL and INDEXED (the + retriever's dedupe/caching identity, untouched) and ``file_hash`` + keeps its 0025 contract; +* rows inserted while the DB is at 0025 backfill ``page_count`` to + NULL — the pre-phase shape, no backfill, no default; +* the ORM contract agrees: a freshly inserted ``Document`` with no + ``page_count`` reads ``page_count is None`` through a fresh session + (the pre-phase shape), and an explicit count round-trips; +* downgrade to 0025 → the column is GONE (A13) while the row + its + content survive; upgrade back to 0026 → 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 = "mig0026" +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) for one documents + column.""" + row = db.execute( + text( + "SELECT data_type, is_nullable, column_default" + " 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) -> uuid.UUID: + """Insert one documents row with the PRE-0026 column set (the 0025 + shape — the page_count column, when present, is omitted so a NULL + backfill is what the row reads).""" + doc_id = uuid.uuid4() + db.execute( + text( + "INSERT INTO documents (id, source, path, full_path, title, content," + " content_hash) VALUES (:id, :s, :p, :fp, :t, :c, :h)" + ), + { + "id": doc_id, + "s": SOURCE, + "p": path, + "fp": f"/tmp/{path}", + "t": f"Doc {path}", + "c": CONTENT, + "h": CONTENT_HASH, + }, + ) + 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_0025_schema_survives(db: Session) -> None: + """The 0025 documents contract must survive the additive upgrade — + in particular ``content_hash`` stays NOT NULL and indexed (the + retriever's identity — untouched by phase 148) and ``file_hash`` + keeps its 0025 contract (VARCHAR(64), nullable, 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 0025 contract" + ) + assert ( + _index_covering(db, "content_hash") is not None + ), "the ix_documents_content_hash index must survive the upgrade" + + file_hash = _column(db, "file_hash") + assert file_hash is not None and file_hash[0] == "character varying", ( + "documents.file_hash (0025) must survive the upgrade" + ) + assert file_hash[1] == "YES", "file_hash must stay NULLABLE" + assert file_hash[2] is None, "file_hash must keep NO server default" + assert _index_covering(db, "file_hash") is None, ( + "file_hash must stay UNINDEXED" + ) + + 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" + ) + image_path = _column(db, "image_path") + assert image_path is not None and image_path[1] == "YES", ( + "documents.image_path (0022) must survive the upgrade" + ) + + +def test_upgrade_to_0026_adds_page_count(db: Session, alembic: Config) -> None: + """Upgrade 0025 → 0026: ``page_count`` exists with the full + contract (INTEGER, NULLABLE, NO server default — NULL = "not + page-qualifying" for every pre-phase row — and UNINDEXED: a + display fact, never a query surface), is ABSENT at 0025, and every + pre-0026 row backfills ``page_count`` to NULL (no backfill + possible — a migration cannot know which documents qualified) — + while the 0025 table contract (incl. the INDEXED ``content_hash`` + and the unindexed ``file_hash``) survives.""" + command.downgrade(alembic, "0025") # start from the pre-0026 state + assert _version(db) == "0025" + assert _column(db, "page_count") is None, "page_count must be absent at 0025" + + pre_id = _insert_sql(db, "pre.md") # the 0025 column set + try: + command.upgrade(alembic, "0026") + assert _version(db) == "0026", "alembic_version must be at 0026" + + col = _column(db, "page_count") + assert col is not None, "documents.page_count is missing" + assert col[0] == "integer", "page_count must be INTEGER" + assert col[1] == "YES", ( + "page_count must be NULLABLE (NULL = not page-qualifying)" + ) + assert col[2] is None, ( + "page_count must carry NO server default — NULL is the meaningful" + " pre-phase state" + ) + assert _index_covering(db, "page_count") is None, ( + "page_count must be UNINDEXED — a display fact, never a query" + " surface (the page route reads it from the row after the id lookup)" + ) + + # The pre-0026 row backfilled to NULL (the pre-phase shape). + assert db.execute( + text( + "SELECT content_hash, page_count FROM documents WHERE id = :id" + ), + {"id": pre_id}, + ).fetchone() == (CONTENT_HASH, None), ( + "every pre-0026 row must backfill page_count to NULL — no" + " backfill, no default — while content_hash survives" + ) + + _assert_0025_schema_survives(db) + finally: + _delete(db, pre_id) + + +def test_orm_page_count_round_trips(db: Session, alembic: Config) -> None: + """The ORM contract agrees with the column contract: a freshly + inserted ``Document`` with NO ``page_count`` reads + ``page_count is None`` through a FRESH session (the pre-phase + shape — NULL until the importer's PDF branch sets it), and an + explicit count round-trips the value.""" + command.upgrade(alembic, "head") + row_counted = Document( + source=SOURCE, + path="counted.pdf", + full_path="/tmp/counted.pdf", + title="counted", + content=CONTENT, + content_hash=CONTENT_HASH, + page_count=24, + ) + row_uncounted = Document( + source=SOURCE, + path="uncounted.md", + full_path="/tmp/uncounted.md", + title="uncounted", + content=CONTENT, + content_hash=CONTENT_HASH, + ) + db.add(row_counted) + db.add(row_uncounted) + db.commit() + try: + with SessionLocal() as fresh: + reloaded_counted = fresh.get(Document, row_counted.id) + assert reloaded_counted is not None, "the counted row must be readable" + assert reloaded_counted.page_count == 24, ( + "the explicit page_count must round-trip through the DB" + ) + reloaded_uncounted = fresh.get(Document, row_uncounted.id) + assert reloaded_uncounted is not None, "the uncounted row must be readable" + assert reloaded_uncounted.page_count is None, ( + "a row created without page_count must read NULL (the pre-phase" + " shape)" + ) + assert reloaded_uncounted.content_hash == CONTENT_HASH, ( + "content_hash must be untouched by the page_count addition" + ) + finally: + _delete(db, row_counted.id) + _delete(db, row_uncounted.id) + + +def test_downgrade_to_0025_drops_the_column(db: Session, alembic: Config) -> None: + """Downgrade 0026 → 0025: the page_count column is gone (A13 — + fully reversible, the copies are re-derivable from the source + files on the next sync) while the row + its content (incl. the + 0025 ``file_hash`` sibling) survive, and the rest of the 0025 + 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=24, + ) + db.add(row) + db.commit() + try: + command.downgrade(alembic, "0025") + assert _version(db) == "0025" + assert _column(db, "page_count") is None, "page_count must be dropped" + + surviving = db.execute( + text( + "SELECT source, path, content, content_hash, file_hash, is_image," + " image_path 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] is None and surviving[5] is False and surviving[6] is None, ( + "file_hash + is_image + image_path 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 0025, then upgrade back to 0026: ``page_count`` is + back with the full contract (INTEGER, NULLABLE, NO server default, + UNINDEXED), a row seeded at 0025 reads ``page_count is None`` (the + pre-phase shape on the round-trip), and the 0025 schema (incl. the + indexed ``content_hash``) is intact.""" + command.downgrade(alembic, "0025") + pre_id = _insert_sql(db, "round.pdf") + try: + command.upgrade(alembic, "0026") + assert _version(db) == "0026", "round-trip upgrade must land at 0026" + + col = _column(db, "page_count") + assert col is not None, "documents.page_count must be back" + assert col[0] == "integer", "page_count must be INTEGER after the round-trip" + assert col[1] == "YES", "page_count must be NULLABLE after the round-trip" + assert col[2] is None, ( + "page_count must still carry NO server default after the round-trip" + ) + assert _index_covering(db, "page_count") is None, ( + "page_count must stay UNINDEXED after the round-trip" + ) + + assert db.execute( + text("SELECT page_count FROM documents WHERE id = :id"), + {"id": pre_id}, + ).scalar() is None, ( + "a row seeded at 0025 must read page_count NULL after the round-trip" + ) + + _assert_0025_schema_survives(db) + finally: + _delete(db, pre_id) diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index ee4f73c..db55311 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -547,6 +547,18 @@ def test_sources_dir_env_override_is_raw_string(monkeypatch: pytest.MonkeyPatch) assert s.sources_dir == "/data/bor/sources" +def test_pdf_page_dir_default_and_env_override(monkeypatch: pytest.MonkeyPatch) -> None: + """Phase 148 (task 01): ``pdf_page_dir`` — the persistent home for + a page-qualifying PDF's rendered page PNGs — defaults to + ``~/bor-sources/pages`` and, like ``sources_dir``/``upload_dir``/ + ``image_dir``, is a RAW string (``Path.expanduser()`` is applied by + the importer, not here) that ``BOR_PDF_PAGE_DIR`` overrides.""" + monkeypatch.delenv("BOR_PDF_PAGE_DIR", raising=False) + assert _settings().pdf_page_dir == "~/bor-sources/pages" + monkeypatch.setenv("BOR_PDF_PAGE_DIR", "/data/bor/pages") + assert _settings().pdf_page_dir == "/data/bor/pages" + + def test_suggestions_default_is_three_plus_real_questions() -> None: s = _settings() assert len(s.suggestions) >= 3 diff --git a/tests/unit/test_document_viewer.py b/tests/unit/test_document_viewer.py index 3585f2d..4753dab 100644 --- a/tests/unit/test_document_viewer.py +++ b/tests/unit/test_document_viewer.py @@ -21,6 +21,15 @@ Frontend side: import-safe page guard, the modal skeleton on chat + Sources, and the modal module's close/focus/URL contract. +Phase 148 (task 02) — the content payload's rendered-page routes +(the viewer strip, task 03, reads from them): the ``DocContent`` +omission rule — ``page_urls`` ABSENT on the wire when ``None`` (never +``null``; a non-qualifying doc's payload is byte-identical to +pre-phase) — and the content endpoint (stubbed session) — a +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 145 (task 03) — the content payload's git origin fields + the ``renderDocument`` "Original" anchor (modal AND /document.html, one site): @@ -46,6 +55,7 @@ import json import re import shutil import subprocess +import uuid from datetime import UTC, datetime from pathlib import Path @@ -279,10 +289,13 @@ def _text_doc() -> Document: def _text_doc_content( - origin_url: str | None = None, origin_branch: str | None = None + origin_url: str | None = None, + origin_branch: str | None = None, + page_urls: list[str] | None = None, ) -> DocContent: """One text doc's content payload (the pre-phase shape) with the - phase-145 origin fields optionally set.""" + phase-145 origin fields + the phase-148 ``page_urls`` optionally + set.""" return DocContent( source="S", path="a.md", @@ -294,6 +307,7 @@ def _text_doc_content( chunks=1, origin_url=origin_url, origin_branch=origin_branch, + page_urls=page_urls, ) @@ -316,6 +330,64 @@ def test_doc_content_origin_fields_absent_on_the_wire_when_unset() -> None: assert dumped["origin_branch"] == "docs" +def test_doc_content_page_urls_absent_on_the_wire_when_unset() -> None: + """Phase 148 (task 02) — the model-level omission rule (the + ``image_url`` precedent): a non-qualifying document (``page_urls`` + ``None`` — a text doc, an image, a text-heavy PDF) serializes with + the key ABSENT — never ``null`` — so its payload is byte-identical + to pre-phase. A page-qualifying PDF keeps the exact route list.""" + dumped = _text_doc_content().model_dump() + assert "page_urls" not in dumped # absent — never null + urls = ["/api/documents/d/pages/1", "/api/documents/d/pages/2"] + dumped = _text_doc_content(page_urls=urls).model_dump() + assert dumped["page_urls"] == urls # the exact 1-based route list + + +def _page_pdf_doc() -> Document: + """A page-QUALIFYING PDF row (phase 148): ``page_count`` set — the + endpoint derives ``page_urls`` from it (the exact 1-based list).""" + doc = Document( + id=uuid.uuid4(), + source="Homelab", + path="ikea/besta.pdf", + full_path="/tmp/besta.pdf", + title="BESTA frame (white) assembly", + 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, + ) + doc.indexed_at = datetime(2026, 8, 22, 1, 2, 3, tzinfo=UTC) + doc.created_at = datetime(2026, 8, 20, 9, 0, 0, tzinfo=UTC) + return doc + + +def test_content_page_urls_carried_for_a_page_qualifying_pdf() -> None: + """Phase 148 (task 02) — the endpoint (stubbed session, no +database): a page-qualifying PDF row (``page_count`` set) carries + ``page_urls`` = the EXACT 1-based + ``/api/documents/<id>/pages/<i>`` list over ``1..page_count`` + (the viewer strip's source, task 03 — a per-page render gap is + still listed; the route 404s that N). Every pre-phase key stands + untouched (``is_image`` false, ``image_url`` ABSENT — the PDF is + not an image doc; the document's TEXT stays user-gated, only the + page bytes are public).""" + doc = _page_pdf_doc() + 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_urls"] == [ + f"/api/documents/{doc.id}/pages/1", + f"/api/documents/{doc.id}/pages/2", + ] # 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) + + def test_content_origin_url_carried_for_a_matching_git_row() -> None: """The endpoint (stubbed session, no database): a ``kind='git'`` row whose URL basename matches the document's source adds diff --git a/tests/unit/test_pdf_page_copies.py b/tests/unit/test_pdf_page_copies.py new file mode 100644 index 0000000..8cdeff8 --- /dev/null +++ b/tests/unit/test_pdf_page_copies.py @@ -0,0 +1,628 @@ +"""Phase 148, task 01 — render qualifying PDF pages at index time (unit +matrix). + +The owner's TODO L5: "For primarily pictographic PDFs it's not super +helpful to see the summaries of the pages. I want to see the PDF pages +in the sources." The locked rule: a PDF gets rendered pages iff the +sync ALREADY decided its text is not the useful view — the phase-133 +fallback fired (unusable text layer — the screenshot class) OR the +phase-140 pictorial pass described ≥1 page (the sparse-page class — +the IKEA fixture). Text-heavy PDFs (usable layer, zero candidates) +keep the text view, and ``BOR_PDF_PICTORIAL=0`` keeps its kill-switch +semantics (no pictorial descriptions → no page strip for the sparse +class). + +Pinned here (the house PyMuPDF fixture recipes — the +``test_pictorial_import.py`` builder for the pictorial class, the +``test_importer.py`` garbage builder for the fallback class, the dense +builder for the negative; the copies land in a tmp ``pdf_page_dir``): + +* eligibility × new index — the pictorial class (IKEA shape: usable + 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 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; +* 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`` → + ``[]``): ``page_count`` NULL, no copies, NO exception, the doc is + still indexed exactly as pre-phase (the row lands, the summary + runs); +* per-page gap — one unreadable page (the render's ``b""`` + placeholder): the other copies are written, ``page_count`` keeps + 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); +* 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). + +The route + payload (task 02) and the viewer strip (task 03) are +pinned in their own task files; the E2E surface is +``tests/e2e/test_pdf_pages_in_sources.py`` (task 04). +""" +from __future__ import annotations + +import asyncio +import logging +import uuid +from collections.abc import Callable, Sequence +from pathlib import Path +from typing import Any + +import pymupdf as fitz +import pytest +from sqlalchemy import select + +from app.config import Settings +from app.models import Document +from app.rag import importer +from app.rag.importer import _delete_page_copies, import_sources +from app.rag.pdf import extract_pdf_text +from tests.unit.test_importer import _build_garbage_pdf, _PageVisionLLM +from tests.unit.test_pictorial_import import ( + TITLE, + _build_dense_pdf, + _build_ikea_like_pdf, + _insert_dense, + _PictorialLLM, +) + +#: The PNG signature (``render_pdf_pages``' output is ``tobytes("png")``). +PNG_MAGIC = b"\x89PNG\r\n\x1a\n" + + +def _settings(tmp_path: Path, **kwargs: Any) -> Settings: + """Settings admitting the ``pdf`` import token with the phase-140 + knobs at their CODE DEFAULTS (``pdf_pictorial`` ON — LOCKED A3) and + the phase-148 ``pdf_page_dir`` pinned to *tmp_path* (the copies + under test never land in the real home); ``images`` off (the + embedded-image pass is not in play — the fixture class is + vector-drawn).""" + 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 _doc(db, source: str, rel: str) -> Document: + doc = db.scalar( + select(Document).where(Document.source == source, Document.path == rel) + ) + assert doc is not None, f"no documents row for ({source!r}, {rel!r})" + return doc + + +def _cleanup_source(db, source: str) -> None: + for doc in db.scalars(select(Document).where(Document.source == source)).all(): + db.delete(doc) + db.commit() + + +def _build_ikea_shrunk(path: Path) -> None: + """A SHRUNK IKEA-class PDF (the stale-replacement target): 6 pages + — the title page, two dense warnings, three sparse ``Step N`` + markers — still qualifying (candidates: 1 and 4–6), but four pages + shorter than :func:`_build_ikea_like_pdf`'s 10 (the old ``p7..p10`` + tail must not leak).""" + doc = fitz.open() + doc.new_page().insert_text((72, 72), "BESTA") + _insert_dense(doc.new_page()) + _insert_dense(doc.new_page()) + for n in range(4, 7): + doc.new_page().insert_text((72, 72), f"Step {n}") + doc.set_metadata({"title": TITLE}) + doc.save(path) + doc.close() + + +def _render_spy( + monkeypatch: pytest.MonkeyPatch, + *, + mutate_full: Callable[[list[bytes]], list[bytes]] | None = None, + raise_full: bool = False, +) -> list[Any]: + """Count + steer the ``render_pdf_pages`` calls the importer makes + (the pictorial pass's candidate render AND the phase-148 + full-page render both resolve the module global at call time): + returns the list of *pages* arguments seen (a candidate list for + the pass, ``None`` for the full render). *mutate_full* transforms + the full-render result (e.g. blanks one page's slot); + *raise_full* makes the full render raise. The candidate render + always runs the real helper.""" + real = importer.render_pdf_pages + calls: list[Any] = [] + + def spy(pdf_path: Path, dpi: int = 150, pages: Sequence[int] | None = None) -> list[bytes]: + calls.append(pages) + if pages is not None: + return real(pdf_path, dpi=dpi, pages=pages) + if raise_full: + raise RuntimeError("simulated render failure (test sentinel)") + out = real(pdf_path, dpi=dpi, pages=pages) + return mutate_full(out) if mutate_full is not None else out + + monkeypatch.setattr(importer, "render_pdf_pages", spy) + return calls + + +# --------------------------------------------------------------------------- +# Eligibility × new index +# --------------------------------------------------------------------------- + + +def test_pictorial_pdf_new_index_writes_every_page_copy(db, tmp_path: Path) -> None: + """Eligible — the PICTORIAL class (IKEA shape: usable layer, ≥1 + sparse page): the first index writes EVERY page of the PDF (not + just the described candidates — the strip shows the whole manual) + as ``<doc-id>-p{N}.png`` under ``pdf_page_dir`` (N 1-based), sets + ``page_count`` to the page count, and the files carry real PNG + bytes — while the indexed content is unchanged by this phase (the + phase-140 illustration blocks stay: the descriptions help the + embeddings and stay in the DB for Q&A — they are simply not + displayed, task 03).""" + root = tmp_path / "pgp" + 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) + assert summary.pdfs_pictorial == 1 # the pass ran (the eligibility class) + doc = _doc(db, "pgp", "ikea.pdf") + assert doc.page_count == 10, "page_count = the PDF's page count" + page_dir = _pages_dir(tmp_path) + for n in range(1, 11): + path = page_dir / f"{doc.id}-p{n}.png" + assert path.is_file(), f"copy p{n} is missing" + assert path.read_bytes()[:8] == PNG_MAGIC, f"p{n} is not a real PNG" + # No behavior change: the final content still carries the + # phase-140 illustration blocks on the sparse pages (1, 6–10) + # and not on the dense ones (2–5). + assert "[Page 1 illustration:" in doc.content + assert "[Page 6 illustration:" in doc.content + assert "[Page 2 illustration:" not in doc.content + finally: + _cleanup_source(db, "pgp") + + +def test_fallback_class_garbage_pdf_writes_every_page_copy(db, tmp_path: Path) -> None: + """Eligible — the FALLBACK class (unusable text layer — the + screenshot class): the garbage PDF takes the phase-133 screenshot + fallback UNCHANGED and now gets the page strip too — one copy per + page + ``page_count`` (the viewer can show the actual pages of the + screenshot class as well).""" + root = tmp_path / "pgf" + root.mkdir() + _build_garbage_pdf(root / "garbage.pdf") # 2 pages + llm = _PageVisionLLM() + 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) + assert summary.pdfs_screenshotted == 1 # the fallback fired (the class) + doc = _doc(db, "pgf", "garbage.pdf") + assert doc.page_count == 2 + page_dir = _pages_dir(tmp_path) + for n in (1, 2): + path = page_dir / f"{doc.id}-p{n}.png" + assert path.is_file(), f"copy p{n} is missing" + assert path.read_bytes()[:8] == PNG_MAGIC + # No behavior change: the content is still the fallback's + # vision descriptions (the page-anchored separator shape). + assert doc.content.startswith("---PAGE-1---") + finally: + _cleanup_source(db, "pgf") + + +def test_text_heavy_pdf_keeps_the_text_view( + db, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Ineligible — a TEXT-HEAVY PDF (usable layer, zero candidates): + the text view stays — no copies, NULL ``page_count``, ZERO render + calls (the pictorial pass no-ops before any render), and the + content is byte-identical to ``extract_pdf_text`` (the + pre-phase-140/148 pin).""" + root = tmp_path / "pgd" + root.mkdir() + pdf = root / "clean.pdf" + _build_dense_pdf(pdf) + expected = extract_pdf_text(pdf) + llm = _PictorialLLM() + llm.settings = _settings(tmp_path) + calls = _render_spy(monkeypatch) + try: + summary = asyncio.run(import_sources([root], llm, session=db)) + assert (summary.files, summary.added, summary.errors) == (1, 1, 0) + 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 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" + finally: + _cleanup_source(db, "pgd") + + +# --------------------------------------------------------------------------- +# Lifecycle: unchanged / changed / qualifier loss +# --------------------------------------------------------------------------- + + +def test_unchanged_second_sync_renders_nothing( + db, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The phase-147 fast path + phase 148: an UNCHANGED qualifying + PDF's second sync costs ZERO render calls (the copies persist — no + re-render, no orphans), and ``page_count`` and every copy's bytes + are untouched. The fast path returns before the branch, so the + render block is skipped by construction.""" + root = tmp_path / "pgu" + root.mkdir() + _build_ikea_like_pdf(root / "ikea.pdf") + llm = _PictorialLLM() + llm.settings = _settings(tmp_path) + try: + calls = _render_spy(monkeypatch) + s1 = asyncio.run(import_sources([root], llm, session=db)) + assert (s1.added, s1.unchanged) == (1, 0) + doc1 = _doc(db, "pgu", "ikea.pdf") + assert doc1.page_count == 10 + assert len([c for c in calls if c is None]) == 1, ( + "exactly one full-page render on the first index" + ) + page_dir = _pages_dir(tmp_path) + copies = { + n: (page_dir / f"{doc1.id}-p{n}.png").read_bytes() for n in range(1, 11) + } + assert all(b[:8] == PNG_MAGIC for b in copies.values()) + embeds1 = len(llm.calls) + + s2 = asyncio.run(import_sources([root], llm, session=db)) + assert (s2.added, s2.updated, s2.unchanged) == (0, 0, 1) + assert len(calls) == 2, "second sync: ZERO render calls (the fast path)" + assert len(llm.calls) == embeds1, "no re-embed (the fast path's zero-work)" + + doc2 = _doc(db, "pgu", "ikea.pdf") + assert doc2.id == doc1.id + assert doc2.page_count == 10, "page_count untouched" + for n in range(1, 11): + assert (page_dir / f"{doc2.id}-p{n}.png").read_bytes() == copies[n], ( + f"copy p{n} moved on an unchanged sync" + ) + finally: + _cleanup_source(db, "pgu") + + +def test_changed_pdf_replaces_the_copy_set_without_orphans( + db, tmp_path: Path, +) -> 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.""" + root = tmp_path / "pgc" + root.mkdir() + pdf = root / "ikea.pdf" + _build_ikea_like_pdf(pdf) + llm = _PictorialLLM() + llm.settings = _settings(tmp_path) + try: + s1 = asyncio.run(import_sources([root], llm, session=db)) + assert s1.added == 1 + doc1 = _doc(db, "pgc", "ikea.pdf") + assert doc1.page_count == 10 + + _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) + 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" + 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" + for n in range(7, 11): + assert not (page_dir / f"{doc2.id}-p{n}.png").exists(), ( + f"stale p{n} from the 10-page version leaked" + ) + leftovers = sorted(p.name for p in page_dir.iterdir()) + assert len(leftovers) == 6, "exactly the new copy set, no orphans" + finally: + _cleanup_source(db, "pgc") + + +def test_qualifier_loss_clears_page_count_and_copies( + db, tmp_path: Path, +) -> None: + """A qualifying PDF whose NEW VERSION gained a dense text layer + (zero candidates, ``pdf_pictorial`` ON): the document STOPPED + qualifying — ``page_count`` is NULL, the copies are deleted, and + the text view returns (the content is the extraction, + byte-identical).""" + root = tmp_path / "pgl" + root.mkdir() + pdf = root / "ikea.pdf" + _build_ikea_like_pdf(pdf) + llm = _PictorialLLM() + llm.settings = _settings(tmp_path) + try: + s1 = asyncio.run(import_sources([root], llm, session=db)) + assert s1.added == 1 + doc1 = _doc(db, "pgl", "ikea.pdf") + assert doc1.page_count == 10 + assert all( + _pages_dir(tmp_path).joinpath(f"{doc1.id}-p{n}.png").is_file() + for n in range(1, 11) + ) + + _build_dense_pdf(pdf) # same path: dense text layer, zero candidates + s2 = asyncio.run(import_sources([root], llm, session=db)) + assert (s2.added, s2.updated, s2.unchanged) == (0, 1, 0) + 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 not list(_pages_dir(tmp_path).glob(f"{doc2.id}-p*.png")), ( + "the copies must be deleted on qualifier loss" + ) + assert doc2.content == extract_pdf_text(pdf), "the text view returns" + finally: + _cleanup_source(db, "pgl") + + +# --------------------------------------------------------------------------- +# Fail-soft: unopenable / gap / render exception / write exception +# --------------------------------------------------------------------------- + + +def test_unopenable_pdf_page_render_is_fail_soft( + db, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A render that yields NO pages (an unopenable file — + ``render_pdf_pages`` → ``[]``): ``page_count`` NULL, no copies, + NO exception — the document still indexes exactly as pre-phase + (the fallback's all-failed path: an empty-content row lands, the + summary runs).""" + root = tmp_path / "pgn" + root.mkdir() + _build_garbage_pdf(root / "garbage.pdf") # the fallback class + llm = _PageVisionLLM() + llm.settings = _settings(tmp_path) + monkeypatch.setattr(importer, "render_pdf_pages", lambda *a, **k: []) + try: + summary = asyncio.run(import_sources([root], llm, session=db)) + 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 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" + finally: + _cleanup_source(db, "pgn") + + +def test_per_page_gap_writes_the_other_copies_and_keeps_the_full_count( + db, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """One unreadable page (the render's ``b""`` placeholder — page 4 + here): a per-page failure is NOT an exception — the other copies + are written, ``page_count`` keeps the FULL count, and the gap's + file is simply absent (a display-level 404 — the route of task 02 + pins its side).""" + root = tmp_path / "pgg" + root.mkdir() + _build_ikea_like_pdf(root / "ikea.pdf") + llm = _PictorialLLM() + llm.settings = _settings(tmp_path) + + def gap_page_four(pages: list[bytes]) -> list[bytes]: + pages[3] = b"" # index 3 = page 4 (the helper's list-index contract) + return pages + + _render_spy(monkeypatch, mutate_full=gap_page_four) + try: + asyncio.run(import_sources([root], llm, session=db)) + doc = _doc(db, "pgg", "ikea.pdf") + assert doc.page_count == 10, "the gap does not shrink the count" + page_dir = _pages_dir(tmp_path) + for n in range(1, 11): + path = page_dir / f"{doc.id}-p{n}.png" + if n == 4: + assert not path.exists(), "the gap has no file" + else: + assert path.is_file(), f"copy p{n} is missing" + finally: + _cleanup_source(db, "pgg") + + +def test_render_exception_is_fail_soft( + db, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """The full-page render RAISES: the warning is the signal, + ``page_count`` NULL, no copies — and the document indexes EXACTLY + as pre-phase (the pictorial content, the chunks, the summary) — + the render is a display concern, it never gates the indexing.""" + root = tmp_path / "pge" + root.mkdir() + _build_ikea_like_pdf(root / "ikea.pdf") + llm = _PictorialLLM() + llm.settings = _settings(tmp_path) + _render_spy(monkeypatch, raise_full=True) + 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) + doc = _doc(db, "pge", "ikea.pdf") + assert doc.page_count is None + assert not _pages_dir(tmp_path).exists(), "no copies — no partial orphans" + assert doc.chunks, "the chunk pipeline ran (fail-soft)" + assert summary.summaries == 1, "the summary ran (fail-soft)" + assert "[Page 6 illustration:" in doc.content, ( + "the pictorial pass's content is untouched by the render failure" + ) + assert any("PDF page render failed" in m for m in caplog.messages), ( + caplog.messages + ) + finally: + _cleanup_source(db, "pge") + + +def test_write_exception_is_fail_soft_and_leaves_no_orphans( + db, + tmp_path: Path, + 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.""" + root = tmp_path / "pgw" + root.mkdir() + _build_ikea_like_pdf(root / "ikea.pdf") + llm = _PictorialLLM() + llm.settings = _settings(tmp_path) + original_write = Path.write_bytes + written_paths: list[Path] = [] + + def boom(self: Path, data: bytes) -> int: + written_paths.append(self) + if len(written_paths) > 1: + raise OSError("simulated disk full (test sentinel)") + return original_write(self, data) + + 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" + doc = _doc(db, "pgw", "ikea.pdf") + assert doc.page_count is None + 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 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 + ) + finally: + _cleanup_source(db, "pgw") + + +# --------------------------------------------------------------------------- +# The shared cleanup helper (the _delete_image_copy convention) +# --------------------------------------------------------------------------- + + +def test_delete_page_copies_helper_is_best_effort( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """The helper (the ``_delete_image_copy`` convention): the row + knows its own extent — copies 1..``page_count`` are deleted, a + NULL/zero count is a no-op, missing files are a SILENT no-op + (already gone — e.g. the owner cleaned the pages dir), and an + unreadable file is logged, never raised (copy cleanup must not + break the sync).""" + page_dir = tmp_path / "pages" + page_dir.mkdir() + doc_id = uuid.uuid4() + doc = Document( + id=doc_id, + source="s", + path="p.pdf", + full_path="/p.pdf", + title="t", + content="c", + content_hash="x", + page_count=3, + ) + p1 = page_dir / f"{doc_id}-p1.png" + p3 = page_dir / f"{doc_id}-p3.png" + p1.write_bytes(b"png1") + p3.write_bytes(b"png3") # p2 is missing (the silent no-op) + + original_unlink = Path.unlink + + def boom(self: Path, **kwargs: Any) -> None: + if self == p3: + raise OSError("simulated unreadable file (test sentinel)") + return original_unlink(self, **kwargs) + + monkeypatch.setattr(Path, "unlink", boom) # pyright: ignore[reportArgumentType] + with caplog.at_level(logging.WARNING, logger="app.importer"): + _delete_page_copies(doc, page_dir) + assert not p1.exists(), "the existing copy was deleted" + assert p3.exists(), "the unreadable file survived (cleanup never raises)" + assert any( + "could not delete PDF page copy" in m for m in caplog.messages + ), caplog.messages + + # A NULL count is a no-op (fresh docs, non-qualifying docs, every + # fail-soft miss) — no exception, no attempt. + doc.page_count = None + _delete_page_copies(doc, page_dir) + assert p3.exists() + + +# --------------------------------------------------------------------------- +# Prune +# --------------------------------------------------------------------------- + + +def test_prune_deletes_page_copies(db, tmp_path: Path) -> None: + """A qualifying doc is PRUNED (its file removed from the source): + the row goes AND its page copies go with it — no orphans in the + pages dir (the ``_prune`` hook beside the image-copy deletion).""" + root = tmp_path / "pgz" + root.mkdir() + pdf = root / "ikea.pdf" + _build_ikea_like_pdf(pdf) + llm = _PictorialLLM() + llm.settings = _settings(tmp_path) + try: + s1 = asyncio.run(import_sources([root], llm, session=db)) + assert s1.added == 1 + doc = _doc(db, "pgz", "ikea.pdf") + assert doc.page_count == 10 + assert len(list(_pages_dir(tmp_path).glob(f"{doc.id}-p*.png"))) == 10 + + pdf.unlink() + s2 = asyncio.run(import_sources([root], llm, session=db, prune=True)) + assert s2.pruned == 1 + assert ( + db.scalar( + select(Document).where( + Document.source == "pgz", Document.path == "ikea.pdf" + ) + ) + is None + ), "the row is pruned" + assert not list(_pages_dir(tmp_path).glob("*.png")), ( + "no orphaned page copies after the prune" + ) + finally: + _cleanup_source(db, "pgz") diff --git a/tests/unit/test_wide_column_css.py b/tests/unit/test_wide_column_css.py index d441a00..cbbf2a2 100644 --- a/tests/unit/test_wide_column_css.py +++ b/tests/unit/test_wide_column_css.py @@ -13,10 +13,12 @@ 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 four reading-column selectors — ``.chat-shell``, - ``.shared-shell``, ``.doc-md``, ``.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 four rules); +* the six 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); * 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``) @@ -117,21 +119,24 @@ def test_no_literal_46rem_width_remains() -> None: def test_the_reading_columns_use_the_token() -> None: """.chat-shell, .shared-shell, .doc-md, .doc-image (phase 122, task 04 — the viewer's image block rides the SAME reading column - as its .doc-md sibling) and .doc-summary:has(+ .doc-md) each cap - with max-width: var(--chat-column) — and exactly those five rules - use the token for a max-width (no other selector).""" + 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 + the token for a max-width (no other selector).""" css = _css() for selector in ( ".chat-shell", ".shared-shell", ".doc-md", ".doc-image", + ".doc-pages", ".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)") == 5, ( + assert css.count("max-width: var(--chat-column)") == 6, ( "exactly the reading-column selectors use the token" )