diff --git a/.agents/phases/todo/185_sync_defer_content_fast_path/00_phase.md b/.agents/phases/complete/185_sync_defer_content_fast_path/00_phase.md similarity index 100% rename from .agents/phases/todo/185_sync_defer_content_fast_path/00_phase.md rename to .agents/phases/complete/185_sync_defer_content_fast_path/00_phase.md diff --git a/.agents/phases/todo/185_sync_defer_content_fast_path/01_fast_path_statement_probe.md b/.agents/phases/complete/185_sync_defer_content_fast_path/01_fast_path_statement_probe.md similarity index 100% rename from .agents/phases/todo/185_sync_defer_content_fast_path/01_fast_path_statement_probe.md rename to .agents/phases/complete/185_sync_defer_content_fast_path/01_fast_path_statement_probe.md diff --git a/.agents/phases/todo/185_sync_defer_content_fast_path/02_defer_content.md b/.agents/phases/complete/185_sync_defer_content_fast_path/02_defer_content.md similarity index 100% rename from .agents/phases/todo/185_sync_defer_content_fast_path/02_defer_content.md rename to .agents/phases/complete/185_sync_defer_content_fast_path/02_defer_content.md diff --git a/.agents/phases/todo/185_sync_defer_content_fast_path/03_e2e_deferred_content.md b/.agents/phases/complete/185_sync_defer_content_fast_path/03_e2e_deferred_content.md similarity index 100% rename from .agents/phases/todo/185_sync_defer_content_fast_path/03_e2e_deferred_content.md rename to .agents/phases/complete/185_sync_defer_content_fast_path/03_e2e_deferred_content.md diff --git a/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__00_phase.a1.err b/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__00_phase.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__00_phase.a1.md b/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__00_phase.a1.md new file mode 100644 index 0000000..1d35b39 --- /dev/null +++ b/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__00_phase.a1.md @@ -0,0 +1,20 @@ +All gates green. Report: + +**Implemented / verified (phase 185)** +- `_doc_row_stmt` is the ONE deferral helper for all four fast-path row loads; only `Document.content` is deferred, no `raiseload`/`noload` (LOCKED A1/A2) — pinned by `tests/unit/test_importer_defer_content.py`. +- `tests/integration/test_import_fast_path_columns.py` (5), `tests/e2e/test_fast_path_deferred_content.py` (3, isolation) green; added 2 constraint pins to `test_import_session_retention.py` (summary-NULL backfill survives the cadence; both prune guards cost the same statement budget). +- **Fixed 6 pre-existing E2E failures** (all reproduce at HEAD with this phase's importer reverted): stale pins from phase 168 `Settings` nav link (`test_nav_rename_sources`), phase 182 `verbatim.js` script count (`test_steering`), phase 175 `/api/chats?limit=…` pagination breaking `endswith` counting (`test_navbar_refresh`), phase-167 lazy-drill race in the breadcrumb read (`test_modal_back_button`), nav-length tab budget (`test_suggestion_chips`). +- **Fixed a genuine flake** in `test_source_chip_quality` (passed/failed across identical runs): `ORDER BY embedding <=> q` is not a total order, so equal-cosine fixtures get a different RRF rank each run. Pinned it as an explicit tie-group (`_assert_ranked` / `_assert_related_pair`) instead of an exact order. + +**Tests / lint (all 100% green)** +- `uv run pytest --cov=app --cov-report=term-missing` → **4519 passed**, `app/rag/importer.py` **99%**, `TOTAL` **99%** (>90%). +- `uv run ruff check .` → All checks passed; `uv run pyright` → 0 errors. +- Full E2E sweep, one pytest process per file (the README's sanctioned loop) → **167/167 files OK, 0 FAIL** (re-run after the fixes). + +**Completion criteria**: helper shared by 4 loads ✅ · column-list + 1-SELECT + backfill pins ✅ · `test_importer_unchanged_fast_path.py` 24 passed, zero expectation edits ✅ · full suite + coverage + lint/types ✅ · phase E2E in isolation ✅ · docs accurate ✅ · commit left to the harness (no git commands run). + +**Documentation**: README + AGENTS.md already carry the phase-185 rule; full pass found and fixed no doc inconsistencies (env vars: all 85 `Settings` fields documented in `.env.example` + README, no ghosts; every `app/… tests/… scripts/…` path referenced in both files exists). + +**Deviations**: (1) the ranking non-determinism behind the chip-quality flake is real in `app/rag/retriever.py` — a SQL tie-break would cost pgvector index usage, so I kept the fix test-side; a follow-up phase should make vector-candidate ranking a total order. (2) Two tracked E2E screenshots were regenerated by the sweep (artifacts, left modified). + +**Next pending phase**: `186_git_log_streaming_dates`. diff --git a/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__00_phase.a1.md.status b/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__00_phase.a1.md.status new file mode 100644 index 0000000..9766475 --- /dev/null +++ b/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__00_phase.a1.md.status @@ -0,0 +1 @@ +ok diff --git a/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__00_phase.a1.validate b/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__00_phase.a1.validate new file mode 100644 index 0000000..ffe0103 --- /dev/null +++ b/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__00_phase.a1.validate @@ -0,0 +1,143 @@ +........................................................................ [ 1%] +........................................................................ [ 3%] +........................................................................ [ 4%] +........................................................................ [ 6%] +........................................................................ [ 7%] +........................................................................ [ 9%] +........................................................................ [ 11%] +........................................................................ [ 12%] +........................................................................ [ 14%] +........................................................................ [ 15%] +........................................................................ [ 17%] +........................................................................ [ 19%] +........................................................................ [ 20%] +........................................................................ [ 22%] +........................................................................ [ 23%] +........................................................................ [ 25%] +........................................................................ [ 27%] +........................................................................ [ 28%] +........................................................................ [ 30%] +........................................................................ [ 31%] +........................................................................ [ 33%] +........................................................................ [ 35%] +........................................................................ [ 36%] +........................................................................ [ 38%] +........................................................................ [ 39%] +........................................................................ [ 41%] +........................................................................ [ 43%] +........................................................................ [ 44%] +........................................................................ [ 46%] +........................................................................ [ 47%] +........................................................................ [ 49%] +........................................................................ [ 50%] +........................................................................ [ 52%] +........................................................................ [ 54%] +........................................................................ [ 55%] +........................................................................ [ 57%] +........................................................................ [ 58%] +........................................................................ [ 60%] +........................................................................ [ 62%] +........................................................................ [ 63%] +........................................................................ [ 65%] +........................................................................ [ 66%] +........................................................................ [ 68%] +........................................................................ [ 70%] +........................................................................ [ 71%] +........................................................................ [ 73%] +........................................................................ [ 74%] +........................................................................ [ 76%] +........................................................................ [ 78%] +........................................................................ [ 79%] +........................................................................ [ 81%] +........................................................................ [ 82%] +........................................................................ [ 84%] +........................................................................ [ 86%] +........................................................................ [ 87%] +........................................................................ [ 89%] +........................................................................ [ 90%] +........................................................................ [ 92%] +........................................................................ [ 94%] +........................................................................ [ 95%] +........................................................................ [ 97%] +........................................................................ [ 98%] +....................................................... [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +-------------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/admin_settings.py 114 0 100% +app/api/auth.py 52 0 100% +app/api/chat.py 292 1 99% +app/api/chat_images.py 52 0 100% +app/api/chats.py 111 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 99 0 100% +app/api/docs.py 239 1 99% +app/api/git_sources.py 259 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 204 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 318 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/compression.py 59 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 43 0 100% +app/core/errors.py 5 0 100% +app/core/http_cache.py 22 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/response_budget.py 63 0 100% +app/core/security_headers.py 20 0 100% +app/core/settings_catalog.py 18 0 100% +app/core/settings_migrate.py 32 0 100% +app/core/settings_overlay.py 111 0 100% +app/core/theming.py 40 0 100% +app/core/tokens.py 44 0 100% +app/db.py 22 0 100% +app/main.py 105 0 100% +app/models.py 141 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 494 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 864 3 99% +app/rag/llm.py 253 1 99% +app/rag/md_images.py 63 0 100% +app/rag/overview.py 78 0 100% +app/rag/pdf.py 192 0 100% +app/rag/prompts.py 162 0 100% +app/rag/retriever.py 252 3 99% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 58 0 100% +app/rag/svg_text.py 111 0 100% +app/rag/verbatim.py 160 0 100% +app/schemas.py 549 0 100% +-------------------------------------------------- +TOTAL 6872 16 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/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__01_fast_path_statement_probe.a1.err b/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__01_fast_path_statement_probe.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__01_fast_path_statement_probe.a1.md b/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__01_fast_path_statement_probe.a1.md new file mode 100644 index 0000000..79d422e --- /dev/null +++ b/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__01_fast_path_statement_probe.a1.md @@ -0,0 +1,18 @@ +**Task 01 complete — probe written, green, and it pins the defect.** + +**Implemented** +- `tests/integration/test_import_fast_path_columns.py` (new, test-only; no `app/` change): fixture KB of 4 × ~8 KB `.md` files seeded through the real `import_sources`, then a SECOND unchanged `import_sources` captured via a `sqlalchemy.event` `before_cursor_execute` listener (phase-183 technique, removed in teardown) with `(statement, params)` so every claim is attributable per file/PK. +- Helpers `doc_selects()` / `loads_content()` (projection-only, `\bdocuments\.content\b` token so `content_hash` can never match — stated in the comment), plus `row_loads()` (the `(source, path)` fast-path shape), `pk_followups()` (the deferred-column lazy-load shape) and a matcher self-test. +- `test_unchanged_run_hydrates_content_pre_fix`: 4 row loads, **all naming `documents.content`**, 0 PK follow-ups, +2 per-source image post-passes pinned apart; carries `# task 02 flips this`. +- `test_unchanged_file_is_one_select`: exactly **1** documents SELECT per summary-bearing file, 0 follow-ups — explicitly marked MUST-STAY-GREEN post-fix (the anti-lazy-load-storm guard). +- `test_summary_null_backfill_reads_stored_content`: staged `summary=NULL` row (stale `is_summary` chunk stamped) → backfilled from the stored markdown (the `lite` call receives `documents.content` verbatim), chunk replaced; `EXPECTED_EXTRA_DOC_SELECTS_FOR_BACKFILL = 0` → 1 with the arithmetic in the comment. +- D4 reference test: unchanged-run counts (`unchanged=4`, `summary_backfilled=1`, `summaries=0`, `chunks=0`, `changed_paths=[]`) + verbatim log words (`import: unchanged` ×4, `import: summary-backfill` ×1, no `import: summary source=`/`date-refreshed`) and the intact KB. +- Module docstring records the baseline (17,407 rows · avg 15,217 chars · 252.6 MB), the mechanism (`_index_file` whole-entity load + `expire_on_commit=False`), the captured statement verbatim, the post-fix lazy-load shape measured via an ad-hoc `defer(Document.content)`, and the verdict **CONFIRMED — the fast path SELECTs `content` for every file inspected**. + +**Gates** — `uv run pytest tests/integration/test_import_fast_path_columns.py -v` → 5 passed; `uv run pytest --cov=app --cov-report=term-missing` → **4511 passed, 0 failed, exit 0**, `app/` coverage **99%** (importer 99%); `uv run ruff check .` → All checks passed; `uv run pyright` → 0 errors. + +**Docs** — no doc impact (test-only; the README/AGENTS sentences belong to tasks 02/03 per the phase plan). + +**Notable decisions** — the two per-source image post-pass SELECTs (`_enrich_pending_images`, `_backfill_image_contexts`) still load `content` and are deliberately *out of scope* (once per source, not per file), so the flip target is the `row_loads()` subset, not "every documents SELECT" — documented so task 02 flips cleanly. Three earlier `test_sync_api.py`/`test_chat_api.py` errors were self-inflicted: I ran probe tests concurrently against the shared Postgres; the serial re-run is fully green, no pre-existing defects. + +**Next pending task:** `.agents/phases/todo/185_sync_defer_content_fast_path/02_defer_content.md` (wire the shared `_doc_row_stmt`/`defer(Document.content)` helper into the four fast-path row loads, flip the probe). diff --git a/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__01_fast_path_statement_probe.a1.md.status b/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__01_fast_path_statement_probe.a1.md.status new file mode 100644 index 0000000..9766475 --- /dev/null +++ b/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__01_fast_path_statement_probe.a1.md.status @@ -0,0 +1 @@ +ok diff --git a/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__01_fast_path_statement_probe.a1.validate b/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__01_fast_path_statement_probe.a1.validate new file mode 100644 index 0000000..83c766f --- /dev/null +++ b/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__01_fast_path_statement_probe.a1.validate @@ -0,0 +1,143 @@ +........................................................................ [ 1%] +........................................................................ [ 3%] +........................................................................ [ 4%] +........................................................................ [ 6%] +........................................................................ [ 7%] +........................................................................ [ 9%] +........................................................................ [ 11%] +........................................................................ [ 12%] +........................................................................ [ 14%] +........................................................................ [ 15%] +........................................................................ [ 17%] +........................................................................ [ 19%] +........................................................................ [ 20%] +........................................................................ [ 22%] +........................................................................ [ 23%] +........................................................................ [ 25%] +........................................................................ [ 27%] +........................................................................ [ 28%] +........................................................................ [ 30%] +........................................................................ [ 31%] +........................................................................ [ 33%] +........................................................................ [ 35%] +........................................................................ [ 36%] +........................................................................ [ 38%] +........................................................................ [ 39%] +........................................................................ [ 41%] +........................................................................ [ 43%] +........................................................................ [ 44%] +........................................................................ [ 46%] +........................................................................ [ 47%] +........................................................................ [ 49%] +........................................................................ [ 51%] +........................................................................ [ 52%] +........................................................................ [ 54%] +........................................................................ [ 55%] +........................................................................ [ 57%] +........................................................................ [ 59%] +........................................................................ [ 60%] +........................................................................ [ 62%] +........................................................................ [ 63%] +........................................................................ [ 65%] +........................................................................ [ 67%] +........................................................................ [ 68%] +........................................................................ [ 70%] +........................................................................ [ 71%] +........................................................................ [ 73%] +........................................................................ [ 75%] +........................................................................ [ 76%] +........................................................................ [ 78%] +........................................................................ [ 79%] +........................................................................ [ 81%] +........................................................................ [ 82%] +........................................................................ [ 84%] +........................................................................ [ 86%] +........................................................................ [ 87%] +........................................................................ [ 89%] +........................................................................ [ 90%] +........................................................................ [ 92%] +........................................................................ [ 94%] +........................................................................ [ 95%] +........................................................................ [ 97%] +........................................................................ [ 98%] +............................................... [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +-------------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/admin_settings.py 114 0 100% +app/api/auth.py 52 0 100% +app/api/chat.py 292 1 99% +app/api/chat_images.py 52 0 100% +app/api/chats.py 111 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 99 0 100% +app/api/docs.py 239 1 99% +app/api/git_sources.py 259 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 204 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 318 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/compression.py 59 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 43 0 100% +app/core/errors.py 5 0 100% +app/core/http_cache.py 22 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/response_budget.py 63 0 100% +app/core/security_headers.py 20 0 100% +app/core/settings_catalog.py 18 0 100% +app/core/settings_migrate.py 32 0 100% +app/core/settings_overlay.py 111 0 100% +app/core/theming.py 40 0 100% +app/core/tokens.py 44 0 100% +app/db.py 22 0 100% +app/main.py 105 0 100% +app/models.py 141 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 494 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 862 3 99% +app/rag/llm.py 253 1 99% +app/rag/md_images.py 63 0 100% +app/rag/overview.py 78 0 100% +app/rag/pdf.py 192 0 100% +app/rag/prompts.py 162 0 100% +app/rag/retriever.py 252 3 99% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 58 0 100% +app/rag/svg_text.py 111 0 100% +app/rag/verbatim.py 160 0 100% +app/schemas.py 549 0 100% +-------------------------------------------------- +TOTAL 6870 16 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/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__02_defer_content.a1.err b/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__02_defer_content.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__02_defer_content.a1.md b/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__02_defer_content.a1.md new file mode 100644 index 0000000..564c1e6 --- /dev/null +++ b/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__02_defer_content.a1.md @@ -0,0 +1,15 @@ +**Implemented (phase 185 task 02)** +- `app/rag/importer.py`: new module-level `_doc_row_stmt(source, rel)` (next to `_file_hash`) returning `select(Document).options(defer(Document.content)).where(source, path)`; `Select` + `defer` imports added. Only `content` is deferred — no `load_only`, no `raiseload`/`noload` (LOCKED A1/A2). +- All four fast-path row loads now share it: `_index_file` L1557, `_index_pdf_file` L2018, `_index_one_embedded_image` L2777 (`image_doc`), `_index_image_file` L3777. Surrounding logic untouched. +- Fifth-site note: the only other `select(Document)`s are the two per-source image post-passes (L3336/L3345, `_backfill_image_contexts` L3495) and `_prune`'s phase-184 `load_only` light row (L4076) — NOT fast-path loads, left as-is by D1. +- Docstrings: module phase-147 paragraph, `_index_file`, `_unchanged_doc` step-3 comment (the backfill is the ONE read that pulls deferred `content` back — one SELECT, one time). +- Probe flipped in `tests/integration/test_import_fast_path_columns.py`: `EXPECTED_EXTRA_DOC_SELECTS_FOR_BACKFILL = 1`, `test_unchanged_run_hydrates_content_pre_fix` → `test_unchanged_run_defers_content_post_fix` (no `documents.content` in the fast-path SELECT, exactly 1 PK follow-up run-wide), docstring verdict **CONFIRMED → FIXED** with the post-fix captured statement (verified verbatim against a live capture) and the 6→7 statement arithmetic. +- New `tests/unit/test_importer_defer_content.py` (the D1 unit pins): `content` is the only column missing from the compiled projection vs the full mapper set; predicate stays `(source, path)`; all four functions call the helper and none builds its own `select(Document)`; exactly 4 call sites; AST guard that no `raiseload`/`noload` is called/imported; real-session read-back of the deferred column. + +**Gates** — `uv run pytest` → **4517 passed, 0 failed**; `uv run pytest --cov=app --cov-report=term-missing` → **app/ 99%** (importer.py 99%), >90% met; `uv run ruff check .` → All checks passed; `uv run pyright` → 0 errors. `test_importer_unchanged_fast_path.py` green with **zero edited expectations** (no harness change was needed — those tests use the real Postgres session, so the lazy load works as-is). Extra regression runs: `test_sync_changed_only.py` (3), `test_document_viewer.py` + `test_document_summaries.py` (12), `test_sync_session_reset.py` (4) all pass. + +**Docs** — `README.md`: fast-path row lookup is metadata-only + a new "A full walk reads row metadata, not the corpus (phase 185)" bullet under *Sync from the UI*. `AGENTS.md`: new operating rule 15 (share `_doc_row_stmt`, never a fifth whole-entity load, never `load_only`/`raiseload`/`noload`, pinned by the two test files). + +**Deviations** — none functional; the phase's D1 "unit pin" landed in a new file so the phase-147 matrix stayed byte-untouched. + +**Next pending task:** `.agents/phases/todo/185_sync_defer_content_fast_path/03_e2e_deferred_content.md`. diff --git a/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__02_defer_content.a1.md.status b/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__02_defer_content.a1.md.status new file mode 100644 index 0000000..9766475 --- /dev/null +++ b/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__02_defer_content.a1.md.status @@ -0,0 +1 @@ +ok diff --git a/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__02_defer_content.a1.validate b/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__02_defer_content.a1.validate new file mode 100644 index 0000000..9be08e5 --- /dev/null +++ b/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__02_defer_content.a1.validate @@ -0,0 +1,143 @@ +........................................................................ [ 1%] +........................................................................ [ 3%] +........................................................................ [ 4%] +........................................................................ [ 6%] +........................................................................ [ 7%] +........................................................................ [ 9%] +........................................................................ [ 11%] +........................................................................ [ 12%] +........................................................................ [ 14%] +........................................................................ [ 15%] +........................................................................ [ 17%] +........................................................................ [ 19%] +........................................................................ [ 20%] +........................................................................ [ 22%] +........................................................................ [ 23%] +........................................................................ [ 25%] +........................................................................ [ 27%] +........................................................................ [ 28%] +........................................................................ [ 30%] +........................................................................ [ 31%] +........................................................................ [ 33%] +........................................................................ [ 35%] +........................................................................ [ 36%] +........................................................................ [ 38%] +........................................................................ [ 39%] +........................................................................ [ 41%] +........................................................................ [ 43%] +........................................................................ [ 44%] +........................................................................ [ 46%] +........................................................................ [ 47%] +........................................................................ [ 49%] +........................................................................ [ 51%] +........................................................................ [ 52%] +........................................................................ [ 54%] +........................................................................ [ 55%] +........................................................................ [ 57%] +........................................................................ [ 58%] +........................................................................ [ 60%] +........................................................................ [ 62%] +........................................................................ [ 63%] +........................................................................ [ 65%] +........................................................................ [ 66%] +........................................................................ [ 68%] +........................................................................ [ 70%] +........................................................................ [ 71%] +........................................................................ [ 73%] +........................................................................ [ 74%] +........................................................................ [ 76%] +........................................................................ [ 78%] +........................................................................ [ 79%] +........................................................................ [ 81%] +........................................................................ [ 82%] +........................................................................ [ 84%] +........................................................................ [ 86%] +........................................................................ [ 87%] +........................................................................ [ 89%] +........................................................................ [ 90%] +........................................................................ [ 92%] +........................................................................ [ 94%] +........................................................................ [ 95%] +........................................................................ [ 97%] +........................................................................ [ 98%] +..................................................... [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +-------------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/admin_settings.py 114 0 100% +app/api/auth.py 52 0 100% +app/api/chat.py 292 1 99% +app/api/chat_images.py 52 0 100% +app/api/chats.py 111 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 99 0 100% +app/api/docs.py 239 1 99% +app/api/git_sources.py 259 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 204 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 318 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/compression.py 59 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 43 0 100% +app/core/errors.py 5 0 100% +app/core/http_cache.py 22 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/response_budget.py 63 0 100% +app/core/security_headers.py 20 0 100% +app/core/settings_catalog.py 18 0 100% +app/core/settings_migrate.py 32 0 100% +app/core/settings_overlay.py 111 0 100% +app/core/theming.py 40 0 100% +app/core/tokens.py 44 0 100% +app/db.py 22 0 100% +app/main.py 105 0 100% +app/models.py 141 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 494 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 864 3 99% +app/rag/llm.py 253 1 99% +app/rag/md_images.py 63 0 100% +app/rag/overview.py 78 0 100% +app/rag/pdf.py 192 0 100% +app/rag/prompts.py 162 0 100% +app/rag/retriever.py 252 3 99% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 58 0 100% +app/rag/svg_text.py 111 0 100% +app/rag/verbatim.py 160 0 100% +app/schemas.py 549 0 100% +-------------------------------------------------- +TOTAL 6872 16 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/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__03_e2e_deferred_content.a1.err b/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__03_e2e_deferred_content.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__03_e2e_deferred_content.a1.md b/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__03_e2e_deferred_content.a1.md new file mode 100644 index 0000000..2d13467 --- /dev/null +++ b/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__03_e2e_deferred_content.a1.md @@ -0,0 +1,17 @@ +**Task 03 complete — phase 185's dedicated Playwright suite is green in isolation and the whole project is 100% green.** + +**Implemented** +- `tests/e2e/test_fast_path_deferred_content.py` (new, mock-only, dedicated module app on port 8202, dedicated `BOR_SOURCES_DIR`, per-test truncate before/after + settled-sync guard, fixture tree re-seeded to original bytes each test so single-test re-runs stay honest): **T1** hand-nulled `documents.summary` + deleted stale `is_summary` chunk → Sources-page re-sync settles `0 added · 2 unchanged` (`summaries`/`dates_updated` stay 0), summary comes back byte-identical from the STORED content through the one deferred-column lazy read, exactly one `is_summary` chunk, viewer shows it; **T2** untouched re-sync then viewer renders the FULL stored text (head/tail/middle sentinels + every stored line, whitespace-collapsed); **T3** rewritten file → `0 added · 1 updated · 1 unchanged`, stored content equals new bytes, chunk count moved, neighbour hashes byte-still, viewer shows only the new text, third sync all-`unchanged`. Docstring: incident, A1/A2 deferral, test→observable mapping, isolation command. +- Docs: README phase-185 operations note was already landed (task 02); extended the README and AGENTS (rule 15) pin lists to cite the new E2E suite; full review of both files surfaced no other inconsistencies (no test asserts doc contents). + +**Gates (exact)** +- `uv run pytest tests/e2e/test_fast_path_deferred_content.py -v --no-cov` → **3 passed** (in isolation) +- `uv run pytest tests/e2e/test_document_dates.py -v --no-cov` → **6 passed**; `…test_sync_changed_only.py -v --no-cov` → **3 passed** (in isolation) +- `uv run pytest --cov=app --cov-report=term-missing` → **4517 passed, 0 failed**, `app/` coverage **99%** (importer 99%) — >90% met +- `uv run ruff check .` → All checks passed; `uv run pyright` → **0 errors** + +**Docs** — `README.md` + `AGENTS.md`: phase-185 pin lists now include the E2E suite; no other doc impact. + +**Notable** — Commit + phase-dir move left to the harness per pipeline rules (project per-phase-commit instruction overridden). Two transient E2E failures during development were my own fixture bugs (missing source registration per test), not app defects; the shared Postgres was left clean. + +**Next pending task:** none in `todo/185_sync_defer_content_fast_path/` — the phase completes with this task (harness commit + move outstanding). diff --git a/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__03_e2e_deferred_content.a1.md.status b/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__03_e2e_deferred_content.a1.md.status new file mode 100644 index 0000000..9766475 --- /dev/null +++ b/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__03_e2e_deferred_content.a1.md.status @@ -0,0 +1 @@ +ok diff --git a/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__03_e2e_deferred_content.a1.validate b/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__03_e2e_deferred_content.a1.validate new file mode 100644 index 0000000..9be08e5 --- /dev/null +++ b/.agents/reports/185_sync_defer_content_fast_path/185_sync_defer_content_fast_path__03_e2e_deferred_content.a1.validate @@ -0,0 +1,143 @@ +........................................................................ [ 1%] +........................................................................ [ 3%] +........................................................................ [ 4%] +........................................................................ [ 6%] +........................................................................ [ 7%] +........................................................................ [ 9%] +........................................................................ [ 11%] +........................................................................ [ 12%] +........................................................................ [ 14%] +........................................................................ [ 15%] +........................................................................ [ 17%] +........................................................................ [ 19%] +........................................................................ [ 20%] +........................................................................ [ 22%] +........................................................................ [ 23%] +........................................................................ [ 25%] +........................................................................ [ 27%] +........................................................................ [ 28%] +........................................................................ [ 30%] +........................................................................ [ 31%] +........................................................................ [ 33%] +........................................................................ [ 35%] +........................................................................ [ 36%] +........................................................................ [ 38%] +........................................................................ [ 39%] +........................................................................ [ 41%] +........................................................................ [ 43%] +........................................................................ [ 44%] +........................................................................ [ 46%] +........................................................................ [ 47%] +........................................................................ [ 49%] +........................................................................ [ 51%] +........................................................................ [ 52%] +........................................................................ [ 54%] +........................................................................ [ 55%] +........................................................................ [ 57%] +........................................................................ [ 58%] +........................................................................ [ 60%] +........................................................................ [ 62%] +........................................................................ [ 63%] +........................................................................ [ 65%] +........................................................................ [ 66%] +........................................................................ [ 68%] +........................................................................ [ 70%] +........................................................................ [ 71%] +........................................................................ [ 73%] +........................................................................ [ 74%] +........................................................................ [ 76%] +........................................................................ [ 78%] +........................................................................ [ 79%] +........................................................................ [ 81%] +........................................................................ [ 82%] +........................................................................ [ 84%] +........................................................................ [ 86%] +........................................................................ [ 87%] +........................................................................ [ 89%] +........................................................................ [ 90%] +........................................................................ [ 92%] +........................................................................ [ 94%] +........................................................................ [ 95%] +........................................................................ [ 97%] +........................................................................ [ 98%] +..................................................... [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +-------------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/admin_settings.py 114 0 100% +app/api/auth.py 52 0 100% +app/api/chat.py 292 1 99% +app/api/chat_images.py 52 0 100% +app/api/chats.py 111 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 99 0 100% +app/api/docs.py 239 1 99% +app/api/git_sources.py 259 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 204 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 318 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/compression.py 59 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 43 0 100% +app/core/errors.py 5 0 100% +app/core/http_cache.py 22 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/response_budget.py 63 0 100% +app/core/security_headers.py 20 0 100% +app/core/settings_catalog.py 18 0 100% +app/core/settings_migrate.py 32 0 100% +app/core/settings_overlay.py 111 0 100% +app/core/theming.py 40 0 100% +app/core/tokens.py 44 0 100% +app/db.py 22 0 100% +app/main.py 105 0 100% +app/models.py 141 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 494 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 864 3 99% +app/rag/llm.py 253 1 99% +app/rag/md_images.py 63 0 100% +app/rag/overview.py 78 0 100% +app/rag/pdf.py 192 0 100% +app/rag/prompts.py 162 0 100% +app/rag/retriever.py 252 3 99% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 58 0 100% +app/rag/svg_text.py 111 0 100% +app/rag/verbatim.py 160 0 100% +app/schemas.py 549 0 100% +-------------------------------------------------- +TOTAL 6872 16 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/166_pre_answer_window_1280.png b/.agents/screenshots/166_pre_answer_window_1280.png index 350bd53..0ad1f10 100644 Binary files a/.agents/screenshots/166_pre_answer_window_1280.png and b/.agents/screenshots/166_pre_answer_window_1280.png differ diff --git a/.agents/screenshots/doc_modal_scrollbar_1280.png b/.agents/screenshots/doc_modal_scrollbar_1280.png index 88bff57..e691e36 100644 Binary files a/.agents/screenshots/doc_modal_scrollbar_1280.png and b/.agents/screenshots/doc_modal_scrollbar_1280.png differ diff --git a/AGENTS.md b/AGENTS.md index e6113e0..59dd0e4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -172,6 +172,31 @@ `tests/e2e/test_sync_session_reset.py` (the five-document prune at batch `2` through the Sources page: both folders cleared, the survivors' chunks intact, zero orphan chunk rows). +15. **The unchanged fast path reads metadata, never markdown:** the four + row lookups that share it (`_index_file`, `_index_pdf_file`, + `_index_one_embedded_image`, `_index_image_file`) all go through the ONE + statement `_doc_row_stmt` (`app/rag/importer.py`), which defers + `Document.content` and NOTHING ELSE (phase 185, LOCKED A1) — the row + load is the only place the fast path gets its row, and hydrating + `content` for every inspected file is what put ~252.6 MB of markdown in + front of a run that re-indexed 217 of ~27k files. Do not add a fifth + whole-entity `select(Document).where(Document.source == …, Document.path + == …)`, do not swap the `defer` for a `load_only` allowlist (a future + column would silently become lazy and resurface as an N+1), and do not + add `raiseload`/`noload` (LOCKED A2): `_unchanged_doc`'s phase-118 + summary-NULL backfill genuinely reads `doc.content` and must pay one + per-row, one-time SELECT instead of raising. Pinned by + `tests/integration/test_import_fast_path_columns.py` (the captured + fast-path `SELECT` names no `documents.content`, exactly one `SELECT` + per unchanged file with a summary, exactly one PK follow-up for the + backfill), `tests/unit/test_importer_defer_content.py` (`content` is + the only deferred column, the four loads share the helper, no + `raiseload`/`noload` in the importer) and + `tests/e2e/test_fast_path_deferred_content.py` (the owner-visible + parity: the summary-NULL backfill survives the deferral with exactly + one `is_summary` chunk, an unchanged doc's viewer shows the full + stored text, a changed file re-indexes and the fast path still fires + after). ## Quick reference ```bash diff --git a/README.md b/README.md index f613f3e..1fd8878 100644 --- a/README.md +++ b/README.md @@ -429,7 +429,12 @@ idempotent and delta-based: each file's RAW BYTES are hashed (sha256, streamed) and checked against the last index's stamp *before* any extraction or vision work — an unchanged file costs one hash + one row lookup (no re-extract, no vision, no re-embed), while a changed file -re-indexes fully. The one exception is a PDF row that still claims a +re-indexes fully. That row lookup reads **metadata only** (phase 185: +`Document.content` is deferred), so a full-resync walk's cost is +metadata-bound instead of corpus-bound; the rare unchanged file whose +`summary` is still NULL re-reads its stored content — one query, one time, +by design — because that backfill genuinely needs the text. The one +exception is a PDF row that still claims a page strip: it is re-gated from its current bytes (no vision, no LLM — see "The strip repairs itself" below). A refresh after a normal editing session takes seconds — pictorial PDFs included (their expensive @@ -683,6 +688,32 @@ only) runs the whole source refresh in one click: `0`-cadence run of the same fixture is byte-identical to the `3`-cadence one (per-path chunk counts, `content`, `summary`, both hashes, the counts line). +- **A full walk reads row metadata, not the corpus** (phase 185). The four + row lookups the unchanged fast path shares (`_index_file` and the PDF / + PDF-embedded-image / standalone-image branches) go through ONE statement, + `_doc_row_stmt`, which defers the single column the fast path never + reads — `Document.content`. The 2026-10-15 sync inspected ~27k files and + re-indexed 217, yet hydrated all 17,407 rows whole (avg 15,217 chars = + **252.6 MB of markdown**, held for the whole run by `expire_on_commit=False` + and phase 184's stamp); now an inspected row carries file_hash, summary, + dates, page count — and no markdown. Exactly ONE column is deferred, and + there is deliberately **no `raiseload`/`noload`**: the phase-118 + summary-NULL backfill genuinely reads the stored content, so that one row + pays one extra `SELECT documents.content … WHERE id = …` (per-row, + one-time, already rare) instead of crashing. Counts, log words, the D1 + manual-date lock, the phase-163 strip re-gate and the backfill result are + byte-identical — pinned by + `tests/integration/test_import_fast_path_columns.py` (the captured + fast-path `SELECT` names no `documents.content`, one `SELECT` per + unchanged file with a summary, exactly one lazy follow-up for the + backfill), `tests/unit/test_importer_defer_content.py` (`content` is + the only deferred column, all four loads share the helper, no + `raiseload`/`noload` anywhere in the importer) and, through the UI, + `tests/e2e/test_fast_path_deferred_content.py` (a hand-nulled summary + backfills to its byte-identical text from the Sources-page re-sync with + exactly one `is_summary` chunk, an unchanged doc's viewer still shows + the full stored text, and a rewritten file re-indexes to the new text + with the next sync all-`unchanged` again). - **Cancel**: while a run is in flight, a **Cancel** button appears next to the sync button; clicking it posts `POST /api/sync/cancel` (admin only) and registers a cooperative cancel (`202`) — the run diff --git a/app/rag/importer.py b/app/rag/importer.py index 2ed4363..946e5ac 100644 --- a/app/rag/importer.py +++ b/app/rag/importer.py @@ -84,6 +84,14 @@ pre-phase on the first post-upgrade sync (one full pass), then stamp and go fast. The previous re-burn costs (a garbage PDF's fallback vision re-issued each sync, a non-deterministic vision model flipping a pictorial PDF's content digest) are gone: the file bytes decide. +Phase 185 (task 02): the fast path's ``(source, path)`` row comes from the +shared :func:`_doc_row_stmt`, which defers ``Document.content`` — the ONE +column the fast path never reads and the fat one of the row (252.6 MB of +markdown over 17,407 rows on the incident box) — so a full walk's memory +scales with row METADATA, not with the corpus. Exactly one column is +deferred (no ``load_only`` allowlist, no ``raiseload``/``noload``): the +phase-118 summary-NULL backfill genuinely reads ``doc.content`` and pays +ONE lazy SELECT for that one row, by design. Standalone images (phase 122, LOCKED A3): with the ``images`` toggle (``BOR_IMAGES``) ON, the walk also admits the image extension set @@ -345,8 +353,8 @@ from pathlib import Path, PurePosixPath from typing import Any, Protocol import pymupdf as fitz # PyMuPDF (phase 126, locked A1); `fitz` = the stable API name -from sqlalchemy import select -from sqlalchemy.orm import Session, load_only +from sqlalchemy import Select, select +from sqlalchemy.orm import Session, defer, load_only from app.config import Settings from app.db import SessionLocal @@ -705,6 +713,37 @@ def _file_hash(path: Path) -> str: return h.hexdigest() +def _doc_row_stmt(source: str, rel: str) -> Select[tuple[Document]]: + """The importer's single Document-row statement (phase 185). + + Used by the FOUR loads that share the phase-147 unchanged fast path — + :func:`_index_file`, :func:`_index_pdf_file`, + :func:`_index_one_embedded_image` and :func:`_index_image_file` — so + the deferral below cannot drift between them. + + ``defer(Document.content)`` — the unchanged fast path (phase 147) reads + file_hash/is_image/page_count/summary/dates, never the stored markdown, + and ``app/db.py`` builds sessions with ``expire_on_commit=False``, so + hydrating ``content`` for EVERY inspected file is what took the + 2026-10-15 run to ~250 MB on a 17,407-document KB (avg 15,217 + chars/row): the walk inspected ~27k files and re-indexed 217. + + ONE column is deferred on purpose (LOCKED A1): a ``load_only`` + allowlist would silently defer every column a later phase adds, and the + deferred read would then surface as an unexplained N+1. No + ``raiseload``/``noload`` either (LOCKED A2): the phase-118 summary-NULL + backfill (``_unchanged_doc`` step 3) genuinely reads ``doc.content`` and + must keep working — it costs one extra SELECT for that one row, a + per-row, one-time, already-rare cost, accepted instead of a rule that + would crash the branch where it should query. + """ + return ( + select(Document) + .options(defer(Document.content)) + .where(Document.source == source, Document.path == rel) + ) + + async def import_sources( sources: list[Path], llm: Embedder, @@ -1194,7 +1233,10 @@ async def _unchanged_doc( # branch paths — the next sync is then fast either way). doc.file_hash = file_digest # 3 — the phase-118 (A2) summary-NULL backfill, from the stored - # content (see the docstring — never a re-read/re-extract). + # content (see the docstring — never a re-read/re-extract). Phase 185: + # this is the ONE fast-path read that pulls the deferred ``content`` + # back — one SELECT, one time, by design (LOCKED A2: no raiseload, the + # branch must query rather than crash). if doc.summary is None: await _store_summary( session, doc=doc, source=source, rel=rel, content=doc.content, @@ -1457,8 +1499,12 @@ async def _index_file( Phase 147 (task 02): the UNCHANGED FAST PATH. BEFORE the image/text/PDF dispatch, the file's RAW source bytes are hashed (:func:`_file_hash` — streamed), the ``(source, path)`` row is - looked up, and the D2 mtime fallback for *raw_date* runs (once, - here — the branches receive the already-computed date) — a + looked up through the shared :func:`_doc_row_stmt` (phase 185: with + ``Document.content`` DEFERRED — the fast path reads row METADATA only, + so a full walk no longer hydrates the KB's markdown; the phase-118 + summary-NULL backfill is the one read that pulls the text back, one + SELECT for that row), and the D2 mtime fallback for *raw_date* runs + (once, here — the branches receive the already-computed date) — a ``doc.file_hash == file_digest`` match returns through the shared unchanged path (:func:`_unchanged_doc`) with ZERO file read beyond the streamed hash, ZERO PDF extraction, ZERO vision call, and @@ -1508,7 +1554,7 @@ async def _index_file( # ``read_text``/``read_bytes``, no ``extract_pdf_text``, no # fallback/pictorial vision, no re-chunk/re-embed/re-summary. file_digest = _file_hash(full_path) - doc = session.scalar(select(Document).where(Document.source == source, Document.path == rel)) + doc = session.scalar(_doc_row_stmt(source, rel)) if raw_date is None: # D2 fallback: no source date in the map → the file's mtime # (one stat). Read before the unchanged early-return — the @@ -1969,7 +2015,7 @@ async def _index_pdf_file( else hashlib.sha256(b"").hexdigest() ) title = _extract_pdf_title(full_path, fallback=full_path.stem) - doc = session.scalar(select(Document).where(Document.source == source, Document.path == rel)) + doc = session.scalar(_doc_row_stmt(source, rel)) if doc is not None and doc.content_hash == digest: # Phase 147: the shared unchanged path — a pre-phase NULL-hash # row, or a bytes-changed/content-same PDF (the content digest @@ -2728,9 +2774,7 @@ async def _index_one_embedded_image( return image_rel = f"{rel}#page={img.page}#img={img.index}{ext}" digest = hashlib.sha256(img.data).hexdigest() - image_doc = session.scalar( - select(Document).where(Document.source == source, Document.path == image_rel) - ) + image_doc = session.scalar(_doc_row_stmt(source, image_rel)) if image_doc is not None and image_doc.content_hash == digest: # Unchanged embedded image (byte digest) — no re-describe, no # re-embed (the standalone image's unchanged rule). @@ -3730,7 +3774,7 @@ async def _index_image_file( # byte change. data = full_path.read_bytes() digest = hashlib.sha256(data).hexdigest() - doc = session.scalar(select(Document).where(Document.source == source, Document.path == rel)) + doc = session.scalar(_doc_row_stmt(source, rel)) if doc is not None and doc.is_image and doc.content_hash == digest: # Phase 147: the shared unchanged path — a pre-phase NULL-hash diff --git a/tests/e2e/test_fast_path_deferred_content.py b/tests/e2e/test_fast_path_deferred_content.py new file mode 100644 index 0000000..d60d772 --- /dev/null +++ b/tests/e2e/test_fast_path_deferred_content.py @@ -0,0 +1,760 @@ +"""Phase 185 E2E (Playwright, mock-only): the deferred column still shows up +where the owner reads it. + +Source: owner incident 2026-10-15 (the same run phases 183/184 fixed). The +unchanged fast path looked each file up by a WHOLE-ENTITY ``select(Document)`` +— ``documents.content`` included — and ``app/db.py`` builds sessions with +``expire_on_commit=False``, so the 2026-10-15 sync inspected ~27k files and +held ~252.6 MB of markdown (17,407 rows × avg 15,217 chars) resident to +discover that 217 files had changed. + +The fix (``app/rag/importer.py``): the four fast-path row loads share the ONE +statement ``_doc_row_stmt``, which defers ``Document.content`` — exactly ONE +column (LOCKED A1: no ``load_only`` allowlist, so a future column can never +silently become lazy) and deliberately NO ``raiseload``/``noload`` (LOCKED +A2: the phase-118 summary-NULL backfill genuinely reads the stored markdown +and must pay one per-row, one-time SELECT instead of crashing). The +statement-level proof — the captured fast-path ``SELECT`` names no +``documents.content``, one ``SELECT`` per unchanged file with a summary, one +lazy follow-up for the backfill — lives in +``tests/integration/test_import_fast_path_columns.py``. What a browser can +see is what THIS module pins: the deferral is invisible to the owner. + +Run in isolation (DB must be up: ``podman compose up -d db``): + + uv run pytest tests/e2e/test_fast_path_deferred_content.py -v --no-cov + +MOCK-ONLY suite: ``E2E_REAL_LLM=1`` is skipped — the backfilled summary is +compared BYTE-IDENTICAL against the summary the first import stored, which +is a property of the deterministic mock summarizer; against the real endpoint +the comparison would be a measurement, not a pin. + +One module app on a dedicated port (the conftest session app owns 8123 in a +combined run), booting against the mock LLM with ``BOR_GIT_SOURCES`` forced +empty (an operator's local ``.env`` must never leak a real source into a +run) and its own ``BOR_SOURCES_DIR`` / ``BOR_PDF_PAGE_DIR`` under +``tmp_path_factory``. The fixture KB is ONE plain (non-git) directory named +:data:`SOURCE_NAME` (``kind='local'`` rows index under ``Path(path).name``, +so the source label is stable whatever temp root pytest picked) holding TWO +``.md`` files; both carry a sentinel phrase on their FIRST content line and +another on their LAST line, so "the viewer shows the FULL stored text" is +provable at both ends of a file. + +Isolation: the shared E2E Postgres is truncated BEFORE **and** AFTER every +test (``chunks, documents, query_log, kb_overview, git_sources, sources_meta, +folder_summaries``), after waiting for the app to hold no running sync (the +run state lives in the app's memory — a leaked run would keep importing into +the next test's truncated KB). The fixture TREE is re-seeded to its original +bytes at the top of every test too (test 3 rewrites one file — a lone re-run +of this module must still see ``2 added``, not ``0 added · 2 unchanged``), +and each test registers its own source row. + +Test → observable mapping (Playwright Mapping Rule): +1. ``test_summary_null_backfill_survives_the_deferral`` — with one row's + ``documents.summary`` nulled AND its stale ``is_summary`` chunk deleted + directly (the phase-118 staging, extended so the chunk's REAPPEARANCE + proves the backfill ran), the Sources-page re-sync still settles + ``0 added · 2 unchanged`` (a backfill is not an ``updated``/``added``), + the document page shows a non-empty summary identical to the one run 1 + stored — produced from the STORED content through the deferred-column + lazy read — and exactly ONE ``is_summary`` chunk exists for the doc + (no duplicate). +2. ``test_unchanged_document_viewer_shows_full_stored_text`` — a second + sync of untouched files settles ``0 added · 2 unchanged`` and the + document view then renders the FULL stored text: the head AND the tail + sentinel both present, every stored line present, no truncation, no + empty body. The row lookup the deferral thinned is the same lookup the + fast path answers with; the viewer reads its row separately and still + gets every byte. +3. ``test_changed_file_reindexes_under_the_deferral`` — one file is + rewritten on disk (new head/tail sentinels, more sections, different + length); the re-sync settles ``0 added · 1 updated · 1 unchanged``, the + viewer shows the NEW text with NO stale fragment, the stored content + equals the new file bytes, the content-chunk count moved, the + untouched neighbour row did not, and a THIRD sync reports all + ``unchanged`` — the fast path still fires after a deferral-mediated + write. +""" +from __future__ import annotations + +import os +import re +import subprocess +import sys +import time +from collections.abc import Iterator +from datetime import datetime +from pathlib import Path +from typing import Any + +import httpx +import pytest +from playwright.sync_api import Page, expect +from sqlalchemy import delete, func, select, text + +from app.db import SessionLocal +from app.models import Chunk, Document +from e2e.auth_helpers import login +from e2e.conftest import ( + ADMIN_PASSWORD, + MOCK_PORT, + SESSION_SECRET, + USE_REAL_LLM, + _wait_http, +) + +REPO = Path(__file__).resolve().parents[2] + +#: Dedicated module port (the house env-overridable pattern): the conftest +#: session app owns 8123 in a combined run. +APP_PORT = int(os.environ.get("E2E_APP_PORT_DEFERCONTENT", "8202")) +APP_URL = f"http://127.0.0.1:{APP_PORT}" + +SOURCES_URL = "/sources.html" + +#: The fixture directory's basename — the ``source`` the importer records for +#: a ``kind='local'`` row (``Path(path).name``), stable whatever temp root +#: this run got. +SOURCE_NAME = "defer-fast-path" + +ALPHA_PATH = "notes/alpha.md" +BETA_PATH = "notes/beta.md" +N_FILES = 2 + +#: Head/tail sentinels: the FIRST content line and the LAST line of each +#: fixture file, so "the viewer shows the FULL stored text" is proven at +#: both ends (test 2), "no stale fragment" is proven at both ends (test 3). +ALPHA_HEAD = "RESE-DEFER-ALPHA-HEAD-3c81" +ALPHA_TAIL = "RESE-DEFER-ALPHA-TAIL-7e52" +ALPHA_MIDDLE = "RESE-DEFER-ALPHA-MIDDLE-0f64" +BETA_HEAD = "RESE-DEFER-BETA-HEAD-5b19" +BETA_TAIL = "RESE-DEFER-BETA-TAIL-8a64" +#: What test 3 rewrites alpha.md into (and what must have replaced every +#: ALPHA_* marker in the KB and in the viewer). +REVISED_HEAD = "RESE-DEFER-ALPHA2-HEAD-e208" +REVISED_TAIL = "RESE-DEFER-ALPHA2-TAIL-41cb" + +#: Test 1 stages THIS row (phase-118's summary-NULL backfill case). +BACKFILL_PATH = BETA_PATH + +#: "Synced HH:MM" — the local-time last-result label (sources.js's +#: fmtSyncTime), any hour/minute (the test_sync_button.py pattern). +SYNCED_LABEL = re.compile(r"Synced \d{1,2}:\d{2}$") + +#: A two-file mock sync (walk + embeds + summary + KB overview) lands in +#: seconds; this is an order of magnitude of head-room with no client-side +#: hard timeout (the test_sync_session_reset.py budget). +SETTLE_TIMEOUT_MS = 90_000 + + +# --------------------------------------------------------------------------- +# The fixture source +# --------------------------------------------------------------------------- + + +def _alpha_text() -> str: + """The ORIGINAL bytes of ``notes/alpha.md`` (test 2's full-text subject, + test 3's rewrite subject). Two H2 sections — a section is the md chunker's + unit, so the rewrite's extra sections move the chunk count.""" + return ( + "# Deferred content alpha\n" + "\n" + f"Head marker {ALPHA_HEAD} opens this file.\n" + "\n" + f"Middle marker {ALPHA_MIDDLE} sits between the sections.\n" + "\n" + "## Alpha body\n" + "\n" + "Alpha prose that the unchanged fast path must never need to read: " + "the walk wants the row metadata, not these paragraphs. It keeps " + "growing so the file is unmistakably a document and not a stub.\n" + "\n" + "## Alpha notes\n" + "\n" + "More alpha prose so the content chunk count is more than one. Each " + "section is its own chunk, which is what test 3 counts when it " + "rewrites this file under the deferral.\n" + "\n" + f"Tail marker {ALPHA_TAIL} closes this file.\n" + ) + + +def _revised_alpha_text() -> str: + """The rewrite: NEW head/tail sentinels, two extra sections, clearly + different length — every old marker is gone from the bytes.""" + return ( + "# Deferred content alpha revised\n" + "\n" + f"Head marker {REVISED_HEAD} opens the revised file.\n" + "\n" + "## Alpha body\n" + "\n" + "The revised alpha prose. The deferral changed which columns the " + "unchanged fast path reads; a CHANGED file must still re-index its " + "full text, so this paragraph and its siblings must land in the KB " + "verbatim, and every marker of the original file must be gone.\n" + "\n" + "## Alpha notes\n" + "\n" + "Revised notes prose, longer than before on purpose so the stored " + "length and the viewer both prove the write path carried the new " + "text through the same session that read the row with content " + "deferred.\n" + "\n" + "## Alpha appendix one\n" + "\n" + "A brand new section that did not exist before the rewrite, added " + "so the content chunk count of this document must change when the " + "re-indexed file is re-chunked.\n" + "\n" + "## Alpha appendix two\n" + "\n" + "A second brand new section, because the point of the phase is " + "that NOTHING about what a run does changed — only which columns " + "travel on an unchanged row lookup.\n" + "\n" + f"Tail marker {REVISED_TAIL} closes the revised file.\n" + ) + + +def _beta_text() -> str: + """``notes/beta.md`` — test 1's backfill subject. Ordinary prose with + its own head/tail sentinels.""" + return ( + "# Deferred content beta\n" + "\n" + f"Head marker {BETA_HEAD} opens this file.\n" + "\n" + "## Beta body\n" + "\n" + "Beta is the document whose stored summary test 1 nulls directly, " + "so the Sources-page re-sync must backfill it from the stored " + "content — the ONE fast-path read that pulls the deferred column " + "back, one SELECT, one time, by design.\n" + "\n" + f"Tail marker {BETA_TAIL} closes this file.\n" + ) + + +def _seed_source(root: Path) -> Path: + """(Re-)write BOTH fixture files under ``root / SOURCE_NAME``. + + Idempotent on purpose: every test calls it first, so the tree is at its + ORIGINAL bytes whatever an earlier test (test 3!) left behind — a lone + re-run of any single test still sees a fresh-KB ``2 added``. + """ + base = root / SOURCE_NAME + files = {ALPHA_PATH: _alpha_text(), BETA_PATH: _beta_text()} + for rel, body in files.items(): + target = base / rel + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(body, encoding="utf-8") + assert not (base / ".git").exists() # a plain directory — no git anywhere + return base + + +@pytest.fixture(scope="module") +def source_dir(tmp_path_factory: pytest.TempPathFactory) -> Path: + """The fixture tree the module app walks (the app server runs on the same + host, so the path is visible to it).""" + return _seed_source(tmp_path_factory.mktemp("bor_fast_path_deferred")) + + +# --------------------------------------------------------------------------- +# The module app +# --------------------------------------------------------------------------- + + +@pytest.fixture(scope="module") +def app_server( + mock_llm: int, source_dir: Path, tmp_path_factory: pytest.TempPathFactory +) -> Iterator[str]: + """The real app under test — per-module env: the mock LLM, the + mock-calibrated gate (the conftest convention), NO git fallback, and a + dedicated ``BOR_SOURCES_DIR`` so no run writes into the operator's. + + The phase's subject needs no knob: the ``defer(Document.content)`` fast + path is the code default (LOCKED A1/A2) — this app is the app every + other suite runs with, walked over a fixture small enough to read. + """ + if USE_REAL_LLM: + pytest.skip( + "mock-only suite — the byte-identical backfilled summary is a " + "property of the deterministic mock (E2E_REAL_LLM unsupported)" + ) + assert mock_llm and source_dir + scratch = tmp_path_factory.mktemp("bor_fast_path_deferred_app") + env = dict(os.environ) + env.pop("DEBUGPY", None) + env["BOR_ENVIRONMENT"] = "e2e" + env["BOR_STATIC_DIR"] = str(REPO / "frontend") + env["BOR_LLM_BASE_URL"] = f"http://127.0.0.1:{MOCK_PORT}/v1" + # Mock-calibrated gate (the conftest pattern): both floors must stay <= + # the threshold or the app refuses to boot (app/config.py). + env["BOR_RELEVANCE_THRESHOLD"] = "0.30" + env["BOR_LEXICAL_SUPPORT_FLOOR"] = "0.15" + env["BOR_SOURCE_USEFULNESS_FLOOR"] = "0.15" + env.setdefault( + "BOR_DATABASE_URL", + "postgresql+psycopg://reese:reese@localhost:5432/brain_of_reese", + ) + # Phase 16: admin auth must be set or create_app() refuses to boot. + env["BOR_ADMIN_PASSWORD"] = ADMIN_PASSWORD + env["BOR_SESSION_SECRET"] = SESSION_SECRET + # No env-fallback rows: the registry each test writes through the + # authenticated API is the only source list. + env["BOR_GIT_SOURCES"] = "" + env["BOR_SOURCES_DIR"] = str(scratch / "checkouts") + env["BOR_PDF_PAGE_DIR"] = str(scratch / "pdf-pages") + 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=env, + ) + 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 + + +# --------------------------------------------------------------------------- +# Isolation +# --------------------------------------------------------------------------- + + +def _truncate_all() -> None: + """Fresh registry + KB (the E2E isolation pattern, the phase-184 list: + ``sources_meta`` and ``folder_summaries`` reset with the KB).""" + with SessionLocal() as db: + db.execute( + text( + "TRUNCATE chunks, documents, query_log, kb_overview, " + "git_sources, sources_meta, folder_summaries" + ) + ) + db.commit() # without the commit the TRUNCATE rolls back + + +def _wait_sync_settled(base_url: str, timeout_s: float = 180.0) -> None: + """No run may leak across a truncate: wait for a non-running status with + an own admin session (the endpoint is admin-only). Usually a no-op — the + tests settle before they end.""" + deadline = time.monotonic() + timeout_s + while time.monotonic() < deadline: + with httpx.Client(base_url=base_url, timeout=5.0) as client: + client.post("/api/login", json={"password": ADMIN_PASSWORD}) + 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(autouse=True) +def _clean(app_url: str, db_ready: None) -> Iterator[None]: + """Per-test KB isolation: the shared E2E Postgres is truncated before AND + after every test, the app holding no running sync at both boundaries.""" + _wait_sync_settled(app_url) + _truncate_all() + yield + _wait_sync_settled(app_url) + _truncate_all() + + +# --------------------------------------------------------------------------- +# Helpers: the UI side +# --------------------------------------------------------------------------- + + +def _add_local_source(page: Page, base_url: str, path: Path) -> None: + """Register the fixture directory through the authenticated API (the + phase-49 shape — the admin cookie rides ``page.request``).""" + r = page.request.post( + f"{base_url}/api/git-sources", + data={"kind": "local", "path": str(path)}, + ) + assert r.status == 201, f"expected 201 for {path}: {r.status} {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 and this process runs on the same host).""" + started = body.get("started_at") + if not started: + return 0.0 + try: + return datetime.fromisoformat(str(started)).timestamp() + except ValueError: + return 0.0 + + +def _sync_from_sources_page(page: Page, base_url: str) -> dict[str, Any]: + """Click the Sources page **Sync sources** button and wait for THIS run's + terminal state (the module app is shared, so a terminal counts only after + a ``running`` tick or when its ``started_at`` post-dates the click). + + Settles the UI on success: the button re-enabled, ``Synced HH:MM``, no + error banner (the test_sync_session_reset.py helper).""" + btn = page.locator("#sync-btn") + expect(btn).to_be_visible(timeout=30_000) + expect(btn).to_be_enabled() + clicked_at = time.time() + btn.click() + expect(btn).to_be_disabled() + expect(page.locator("#sync-label")).to_have_text("Syncing…") + + deadline = time.monotonic() + SETTLE_TIMEOUT_MS / 1000 + saw_running = False + body: dict[str, Any] = {} + while time.monotonic() < deadline: + r = page.request.get(f"{base_url}/api/sync/status") + assert r.status == 200, r.text + body = r.json() + state = body["state"] + if state == "running": + saw_running = True + elif state in ("success", "failed", "cancelled") and ( + saw_running or _started_at(body) >= clicked_at - 1.0 + ): + break + time.sleep(0.2) + else: + raise AssertionError(f"sync never reached a terminal state: {body}") + + assert body["state"] == "success", f"sync failed: {body}" + assert body["error"] is None, body + expect(btn).to_be_enabled(timeout=15_000) + expect(page.locator("#sync-label")).to_have_text(SYNCED_LABEL, timeout=15_000) + expect(page.locator("#sync-error-banner")).to_be_hidden() + return body + + +def _expect_counts_line(page: Page, expected: str) -> None: + """The counts line — the owner-visible sync report on the single sync + text surface (phase 150; sources.js fmtSyncResult; the zero "updated" + part is omitted).""" + expect(page.locator("#sync-progress-text")).to_have_text(expected, timeout=15_000) + + +def _goto_sources(page: Page, base_url: str) -> None: + """Back to the Sources page (the sync button's home) after a viewer + detour — the session cookie rides the navigation.""" + page.goto(base_url + SOURCES_URL) + expect(page.locator("#sync-btn")).to_be_visible(timeout=30_000) + expect(page.locator("#sync-btn")).to_be_enabled() + + +def _open_document_page(page: Page, base_url: str, path: str) -> None: + """Open the standalone viewer for one fixture document (the phase-10 + surface; the session cookie from ``login`` rides the navigation).""" + page.goto( + f"{base_url}/document.html?source={SOURCE_NAME}" + f"&path={path.replace('/', '%2F')}" + ) + expect(page.locator("#doc-content .doc-md")).to_have_count(1, timeout=30_000) + + +# --------------------------------------------------------------------------- +# Helpers: the DB side +# --------------------------------------------------------------------------- + + +def _summary_map() -> dict[str, str | None]: + """``path`` → ``documents.summary`` for both fixture rows.""" + with SessionLocal() as db: + rows = db.execute( + select(Document.path, Document.summary).where( + Document.source == SOURCE_NAME + ) + ).all() + return {str(path): (None if summary is None else str(summary)) for path, summary in rows} + + +def _summary_chunk_contents(path: str) -> list[str]: + """The ``is_summary`` chunk contents of one document — the duplicate + check (phase 30: at most one per doc at a time).""" + with SessionLocal() as db: + return [ + str(content) + for (content,) in db.execute( + select(Chunk.content) + .join(Document, Document.id == Chunk.document_id) + .where( + Document.source == SOURCE_NAME, + Document.path == path, + Chunk.is_summary.is_(True), + ) + ).all() + ] + + +def _content_chunk_counts() -> dict[str, int]: + """``path`` → content-chunk count per fixture doc (the ``position >= 0`` + rows — the ``is_summary`` position −1 chunk stays out).""" + with SessionLocal() as db: + rows = db.execute( + select(Document.path, func.count(Chunk.id)) + .join(Chunk, Chunk.document_id == Document.id) + .where( + Document.source == SOURCE_NAME, + Chunk.position >= 0, + ) + .group_by(Document.path) + ).all() + return {str(path): int(count) for path, count in rows} + + +def _stored_content(path: str) -> str: + with SessionLocal() as db: + content = db.scalar( + select(Document.content).where( + Document.source == SOURCE_NAME, Document.path == path + ) + ) + assert content is not None, f"{path} is not in the KB" + return str(content) + + +def _row_stamp(path: str) -> tuple[Any, Any]: + """``(content_hash, file_hash)`` of one fixture row — the neighbour's + byte-still proof (a re-index would re-stamp both).""" + with SessionLocal() as db: + row = db.execute( + select(Document.content_hash, Document.file_hash).where( + Document.source == SOURCE_NAME, Document.path == path + ) + ).one() + return row[0], row[1] + + +def _stage_null_summary(path: str) -> None: + """The phase-118 backfill case, staged directly (the way phase 118's + tests null the column), extended for this phase: the stale + ``is_summary`` CHUNK goes too, so the chunk's REAPPEARANCE after the + re-sync proves the backfill ran rather than merely survived.""" + with SessionLocal() as db: + doc_id = db.scalar( + select(Document.id).where( + Document.source == SOURCE_NAME, Document.path == path + ) + ) + assert doc_id is not None, f"{path} is not in the KB" + db.execute(delete(Chunk).where(Chunk.document_id == doc_id, Chunk.is_summary.is_(True))) + db.execute( + text( + "UPDATE documents SET summary = NULL " + "WHERE source = :s AND path = :p" + ), + {"s": SOURCE_NAME, "p": path}, + ) + db.commit() + # The stage landed — otherwise every later assertion would pass vacuously. + assert _summary_map()[path] is None + assert _summary_chunk_contents(path) == [] + + +# --------------------------------------------------------------------------- +# 1. The summary-NULL backfill survives the deferral +# --------------------------------------------------------------------------- + + +def test_summary_null_backfill_survives_the_deferral( + page: Page, app_url: str, source_dir: Path, db_ready: None +) -> None: + """Run 1 indexes both docs and summarizes them (phase 118 A2: markdown + docs are summarized too). One row is then staged by hand — + ``documents.summary = NULL`` AND its ``is_summary`` chunk deleted — and + the Sources-page re-sync runs WITH the deferral in place: + + * the run reports the doc as ``unchanged``, not ``added``/``updated`` + (the settle line and the terminal detail both say so — the backfill + must not masquerade as a re-index, and ``dates_updated``/``summaries`` + stay 0); + * the document page shows a NON-EMPTY summary again, byte-identical to + the one run 1 stored — produced from the STORED content through the + one lazy read the deferral leaves on this branch (LOCKED A2); + * exactly ONE ``is_summary`` chunk exists for the doc (it was recreated, + not duplicated).""" + page.set_default_timeout(30_000) + _seed_source(source_dir.parent) # idempotent — the tree starts at its original bytes + login(page, app_url, next=SOURCES_URL) + _add_local_source(page, app_url, source_dir) + + # --- run 1: everything is new, both docs get a summary ---------------- + body1 = _sync_from_sources_page(page, app_url) + _expect_counts_line(page, f"{N_FILES} added") + assert body1["detail"]["added"] == N_FILES, body1["detail"] + originals = _summary_map() + assert set(originals) == {ALPHA_PATH, BACKFILL_PATH}, originals + stored_summary = originals[BACKFILL_PATH] + assert stored_summary, "run 1 must have summarized the markdown doc (phase 118 A2)" + + # --- stage the phase-118 case directly -------------------------------- + _stage_null_summary(BACKFILL_PATH) + + # --- run 2: the unchanged re-sync backfills from the STORED content ---- + body2 = _sync_from_sources_page(page, app_url) + _expect_counts_line(page, f"0 added · {N_FILES} unchanged") + detail = body2["detail"] + assert detail["added"] == 0 and detail["updated"] == 0, detail + assert detail["unchanged"] == N_FILES, detail + assert detail["errors"] == 0, detail + # The counts stay honest: a backfill is neither a re-summary of changed + # content (``summaries``) nor a date refresh (``dates_updated``). + assert detail["summaries"] == 0, detail + assert detail["dates_updated"] == 0, detail + assert detail["pruned"] == 0, detail + + # --- the DB side: the summary is back, byte-identical, one chunk ------ + after = _summary_map() + assert after[BACKFILL_PATH] == stored_summary, ( + "the backfilled summary must equal what run 1 stored from the same " + "content (the deterministic mock makes byte-equality a pin)" + ) + chunks = _summary_chunk_contents(BACKFILL_PATH) + assert len(chunks) == 1, f"exactly one is_summary chunk, got {len(chunks)}" + assert chunks[0] == stored_summary + + # --- the owner's surface: the document page shows the summary --------- + _open_document_page(page, app_url, BACKFILL_PATH) + expect(page.locator("#doc-title")).to_have_text("Deferred content beta") + summary_text = page.locator("#doc-content .doc-summary-text") + expect(summary_text).to_have_count(1) + expect(summary_text).not_to_be_empty() + expect(summary_text).to_have_text(stored_summary) + # The deferral did not thin the doc for the viewer either. + expect(page.locator("#doc-content")).to_contain_text(BETA_HEAD) + expect(page.locator("#doc-content")).to_contain_text(BETA_TAIL) + + +# --------------------------------------------------------------------------- +# 2. An unchanged document's viewer still shows the FULL stored text +# --------------------------------------------------------------------------- + + +def test_unchanged_document_viewer_shows_full_stored_text( + page: Page, app_url: str, source_dir: Path, db_ready: None +) -> None: + """Run 1 indexes, run 2 changes NOTHING (untouched bytes, so every file + walks the deferred-row fast path) — and the document view then renders + the FULL stored text: the sentinel at the START and the sentinel at the + END are both present, every stored line is present, no truncation, no + empty body. The deferral must not have made the viewer's source row + thinner.""" + page.set_default_timeout(30_000) + _seed_source(source_dir.parent) + login(page, app_url, next=SOURCES_URL) + _add_local_source(page, app_url, source_dir) + + _sync_from_sources_page(page, app_url) + _expect_counts_line(page, f"{N_FILES} added") + _sync_from_sources_page(page, app_url) + _expect_counts_line(page, f"0 added · {N_FILES} unchanged") + + stored = _stored_content(ALPHA_PATH) + assert stored == _alpha_text(), "the stored content is the fixture's original bytes" + + _open_document_page(page, app_url, ALPHA_PATH) + expect(page.locator("#doc-title")).to_have_text("Deferred content alpha") + body = page.locator("#doc-content .doc-md") + expect(body).not_to_be_empty() + + # Both ENDS of the file, in the rendered view (the task's pin): a viewer + # fed by a truncated row would be missing one of them. + expect(body).to_contain_text(ALPHA_HEAD) + expect(body).to_contain_text(ALPHA_TAIL) + expect(body).to_contain_text(ALPHA_MIDDLE) + + # The strong form: EVERY stored line renders (headings render without + # their leading '#', the fixture carries no list/inline markup, so the + # comparison is line-for-line on whitespace-collapsed text — a viewer + # fed by a thinner row, or truncated anywhere in the middle, drops one + # of these). + rendered = " ".join(body.inner_text().split()) + for line in stored.splitlines(): + if not line.strip(): + continue + expected = " ".join(line.lstrip("#").split()) + assert expected in rendered, f"stored line not rendered: {line!r}" + + +# --------------------------------------------------------------------------- +# 3. A changed file re-indexes under the deferral +# --------------------------------------------------------------------------- + + +def test_changed_file_reindexes_under_the_deferral( + page: Page, app_url: str, source_dir: Path, db_ready: None +) -> None: + """One file is rewritten on disk (new head/tail sentinels, two extra + sections, clearly different length). The re-sync runs with the row read + deferred and must behave exactly as before the phase: ``1 updated`` with + the neighbour ``unchanged``, the viewer shows the NEW text with NO stale + fragment, the stored content equals the new bytes, the content-chunk + count moved, and the untouched neighbour's hashes did not. A THIRD sync + then reports everything ``unchanged`` — the fast path still fires after + a deferral-mediated write.""" + page.set_default_timeout(30_000) + _seed_source(source_dir.parent) + login(page, app_url, next=SOURCES_URL) + _add_local_source(page, app_url, source_dir) + + _sync_from_sources_page(page, app_url) + _expect_counts_line(page, f"{N_FILES} added") + chunks_before = _content_chunk_counts() + stamps_before = _row_stamp(BETA_PATH) + assert set(chunks_before) == {ALPHA_PATH, BETA_PATH}, chunks_before + + # --- rewrite alpha on disk (new sentinels, longer, more sections) ----- + (source_dir / ALPHA_PATH).write_text(_revised_alpha_text(), encoding="utf-8") + + body = _sync_from_sources_page(page, app_url) + _expect_counts_line(page, f"0 added · 1 updated · {N_FILES - 1} unchanged") + detail = body["detail"] + assert detail["added"] == 0 and detail["updated"] == 1, detail + assert detail["unchanged"] == N_FILES - 1, detail + assert detail["errors"] == 0, detail + + # The stored content is the NEW file bytes — the write path carried the + # full text through the same session whose row read deferred it. + assert _stored_content(ALPHA_PATH) == _revised_alpha_text() + + # The content-chunk count moved (the rewrite added two whole sections). + chunks_after = _content_chunk_counts() + assert chunks_after[ALPHA_PATH] != chunks_before[ALPHA_PATH], ( + f"chunk count must move on a re-index: {chunks_before} -> {chunks_after}" + ) + assert chunks_after[ALPHA_PATH] > 0 and chunks_before[ALPHA_PATH] > 0 + # The untouched neighbour row did not move (both hashes byte-still). + assert _row_stamp(BETA_PATH) == stamps_before + + # The viewer shows the NEW text and NO stale fragment. + _open_document_page(page, app_url, ALPHA_PATH) + expect(page.locator("#doc-title")).to_have_text("Deferred content alpha revised") + rendered = page.locator("#doc-content .doc-md") + expect(rendered).to_contain_text(REVISED_HEAD) + expect(rendered).to_contain_text(REVISED_TAIL) + expect(rendered).not_to_contain_text(ALPHA_HEAD) + expect(rendered).not_to_contain_text(ALPHA_TAIL) + expect(rendered).not_to_contain_text(ALPHA_MIDDLE) + + # --- run 3: the fast path still fires after the deferral-mediated write + _goto_sources(page, app_url) + body3 = _sync_from_sources_page(page, app_url) + _expect_counts_line(page, f"0 added · {N_FILES} unchanged") + assert body3["detail"]["updated"] == 0, body3["detail"] + assert body3["detail"]["unchanged"] == N_FILES, body3["detail"] diff --git a/tests/e2e/test_modal_back_button.py b/tests/e2e/test_modal_back_button.py index e4c6ecc..e2dfec1 100644 --- a/tests/e2e/test_modal_back_button.py +++ b/tests/e2e/test_modal_back_button.py @@ -99,6 +99,7 @@ Test → observable mapping (Playwright Mapping Rule): from __future__ import annotations import asyncio +import time from pathlib import Path from threading import Thread from typing import Any @@ -229,13 +230,29 @@ def _assert_crumb(page: Page, links: list[str], current: str) -> None: ``.kb-crumb-link`` per ancestor (in order — the first is always the ``Knowledge base`` jump-to-top segment), then the last segment as the ``.kb-crumb-current`` span with ``aria-current="page"``. - Hidden = the TOP level (no drill).""" + Hidden = the TOP level (no drill). + + The chain is POLLED, not read once: phase 167 made a drill LAZY (the + level and its breadcrumb render when ``/api/docs/tree/node`` resolves), + so the single read that sufficed while the whole tree shipped at boot + now races the fetch and intermittently returns the level the user came + FROM. + """ + expected = [*links, current] expect(page.locator("#kb-crumb")).to_be_visible() - texts = page.eval_on_selector_all( - "#kb-crumb :is(.kb-crumb-link, .kb-crumb-current)", - "(els) => els.map((el) => el.textContent)", - ) - assert texts == [*links, current], texts + + def _crumb_texts() -> list[str]: + return page.eval_on_selector_all( + "#kb-crumb :is(.kb-crumb-link, .kb-crumb-current)", + "(els) => els.map((el) => el.textContent)", + ) + + deadline = time.monotonic() + 30.0 + texts = _crumb_texts() + while texts != expected and time.monotonic() < deadline: + page.wait_for_timeout(50) + texts = _crumb_texts() + assert texts == expected, texts cur = page.locator("#kb-crumb .kb-crumb-current") expect(cur).to_have_count(1) expect(cur).to_have_attribute("aria-current", "page") diff --git a/tests/e2e/test_nav_rename_sources.py b/tests/e2e/test_nav_rename_sources.py index 4df355f..a866172 100644 --- a/tests/e2e/test_nav_rename_sources.py +++ b/tests/e2e/test_nav_rename_sources.py @@ -112,13 +112,14 @@ CURRENT_LINK: dict[str, str | None] = { #: The four primary nav links, in their physical DOM order — the labels #: after the phase-48 swap (ids/hrefs unchanged). NAV_LABELS = ("Chat", "RAG", "Sources", "Tuning") -#: The FIFTH–SEVENTH a.nav-link in every page header (phase 53 +#: The FIFTH–EIGHTH a.nav-link in every page header (phase 53 #: saved-chat history + phase 79 task 06 access tokens + phase 91 -#: theme tab — all ship-hidden, revealed by header.js for admins). +#: theme tab + phase 168 admin settings panel — all ship-hidden, +#: revealed by header.js for admins). #: The DOM enumeration below therefore always sees them (pre-existing; #: the list matches the real nav — the phase-34 one-bar contract ships #: the SAME nav, incl. the Tokens and Theme links, on every page). -NAV_TAIL = ("History", "Tokens", "Theme") +NAV_TAIL = ("History", "Tokens", "Theme", "Settings") #: The login.js script — route pattern for the redirect suppression. LOGIN_JS_ROUTE = re.compile(r"/assets/login\.js(\?.*)?$") diff --git a/tests/e2e/test_navbar_refresh.py b/tests/e2e/test_navbar_refresh.py index 28320da..2ac20f5 100644 --- a/tests/e2e/test_navbar_refresh.py +++ b/tests/e2e/test_navbar_refresh.py @@ -71,6 +71,7 @@ from collections.abc import Callable from pathlib import Path from threading import Thread from typing import Any +from urllib.parse import urlsplit import httpx from playwright.sync_api import Locator, Page, Request, expect @@ -510,7 +511,23 @@ def test_all_four_data_views_refetch_on_reshow( log = _get_logger(page) def count(endpoint: str) -> int: - return sum(1 for u in log if u.endswith(endpoint)) + """Requests to ONE endpoint: exact path, plus every query pair the + pin names. Phase 175 paginated the History list + (``/api/chats?limit=25&offset=0``, ``history.js``), so the plain + ``endswith`` this suite used to count with stopped seeing the + view's own fetch altogether — the mount baseline never arrived and + the loop timed out on the FIRST view.""" + path, _, query = endpoint.partition("?") + wanted = [pair for pair in query.split("&") if pair] + total = 0 + for url in log: + parts = urlsplit(url) + if parts.path != path: + continue + params = parts.query.split("&") if parts.query else [] + if all(pair in params for pair in wanted): + total += 1 + return total for nav_sel, view_path, endpoint in DATA_VIEWS: # First show (the mount) — wait until the mount's own load diff --git a/tests/e2e/test_source_chip_quality.py b/tests/e2e/test_source_chip_quality.py index 3953953..1204a2f 100644 --- a/tests/e2e/test_source_chip_quality.py +++ b/tests/e2e/test_source_chip_quality.py @@ -114,6 +114,46 @@ def _ask(page: Page, message: str) -> None: page.click("#send-btn") +def _assert_ranked(recorded: str, groups: list[list[str]]) -> None: + """Compare a ``query_log.sources`` string against an expected rank list + whose trailing entries are TIE-GROUPS (order-free sets). + + WHY the groups exist: the E2E mock embedder builds real token-overlap + vectors, so several tiny fixture documents end up on EXACTLY the same + fused score. ``fuse`` then sorts by ``(-score, -cosine, path, + position)``… but that only orders chunks whose SCORES differ; between + two documents tied in both lists the vector rank itself comes out of + ``ORDER BY embedding <=> q``, which is not a total order, so Postgres + (a parallel seq scan handing its partial sorts back) may return the + tied pair the other way round between two runs of the SAME fixture. + That was this suite's flake. The rank of every block, and the exact + SET inside it, are still pinned — what the story pins is that the + record carries the FULL retrieval (LOCKED A3), not the luck of a tie. + """ + docs = [d.strip() for d in recorded.split(",")] + expected = [d for group in groups for d in group] + assert len(docs) == len(expected), f"recorded {docs}, expected {expected}" + at = 0 + for group in groups: + window = docs[at : at + len(group)] + assert sorted(window) == sorted(group), ( + f"rank block {at + 1}..{at + len(group)}: got {window}, " + f"expected the set {sorted(group)}" + ) + at += len(group) + + +def _assert_related_pair(links: object, titles: set[str]) -> None: + """The two related-doc links carry EXACTLY these two documents, in + either order (the same tie the record pins as a group — see + :func:`_assert_ranked`).""" + found = { + links.nth(i).get_attribute("title") # pyright: ignore[reportAttributeAccessIssue] + for i in range(2) + } + assert found == titles, f"related pair is {found}, expected {titles}" + + def test_single_source_question_shows_exactly_one_citation_chip( page: Page, app_url: str, mock_llm: int, db_ready: None ) -> None: @@ -179,15 +219,18 @@ def test_single_source_question_shows_exactly_one_citation_chip( # (best eff 0.016277) now out-ranks ``kubernetes.md`` (0.016036); # re-pinned against the post-phase walk (the record's CONTENT — # suggested + related + read, deduped — is LOCKED A3, untouched). - assert row_log.sources == ( - "docs/homelab/ssh/ssh_aliases.txt, " - "docs/homelab/container_gitlab/gitlab-compose.yaml, " - "docs/homelab/scripts/uptime_probe.py, " - "docs/homelab/backups.md, " - "docs/homelab/kubernetes.md, " - "docs/homelab/quadlet/compose.container, " - "docs/homelab/networking/static-dns.json" - ), row_log.sources + _assert_ranked( + row_log.sources, + [ + ["docs/homelab/ssh/ssh_aliases.txt"], + ["docs/homelab/container_gitlab/gitlab-compose.yaml"], + ["docs/homelab/scripts/uptime_probe.py"], + # The tied pair this record used to pin in an arbitrary order. + ["docs/homelab/backups.md", "docs/homelab/kubernetes.md"], + ["docs/homelab/quadlet/compose.container"], + ["docs/homelab/networking/static-dns.json"], + ], + ) def test_deflected_question_shows_zero_citation_chips( @@ -225,8 +268,9 @@ def test_deflected_question_shows_zero_citation_chips( # record, but a deflected turn cites nothing, so they never render # as chips. expect(links).to_have_count(2) - expect(links.nth(0)).to_contain_text("backups.md") - expect(links.nth(1)).to_contain_text("kubernetes.md") + # The pair, as a SET: the two are tied in the fused score (see + # :func:`_assert_ranked`), so only their presence is a contract. + _assert_related_pair(links, {"docs/homelab/backups.md", "docs/homelab/kubernetes.md"}) # ZERO citation chips under the bubble — the weak hits are scored # docs, not citations (the phase-112/113 contract on the wire). @@ -243,12 +287,16 @@ def test_deflected_question_shows_zero_citation_chips( assert row_log.deflected is True assert 0.0 < row_log.top_score < get_settings().relevance_threshold assert row_log.fts_hits == 0 - assert row_log.sources == ( - "docs/deployments/new-service.md, " - "docs/homelab/ssh/ssh_aliases.txt, " - "docs/homelab/quadlet/lan.network, " - "docs/homelab/scripts/uptime_probe.py, " - "docs/homelab/quadlet/compose.container, " - "docs/homelab/backups.md, " - "docs/homelab/kubernetes.md" - ), row_log.sources + _assert_ranked( + row_log.sources, + [ + ["docs/deployments/new-service.md"], + ["docs/homelab/ssh/ssh_aliases.txt"], + ["docs/homelab/quadlet/lan.network"], + ["docs/homelab/scripts/uptime_probe.py"], + ["docs/homelab/quadlet/compose.container"], + # The related row IS this pair, and the row's order is the same + # tie — pinned as a set below (``_assert_related_pair``). + ["docs/homelab/backups.md", "docs/homelab/kubernetes.md"], + ], + ) diff --git a/tests/e2e/test_steering.py b/tests/e2e/test_steering.py index 3606c8e..01540ff 100644 --- a/tests/e2e/test_steering.py +++ b/tests/e2e/test_steering.py @@ -48,11 +48,13 @@ NOTE = "STEEER-MARKER be concise" XSS_NOTE = "" #: The shell (index.html — served for BOTH / and /tuning.html since #: phase 76 task 01, when the Tuning view folded into it) ships exactly -#: FIVE classic/module script tags: the phase-39 brand.js classic layer -#: + markdown.js + the chat module (app.js) + the shell router -#: (router.js, which lazy-imports the tuning.js view module) + the -#: phase-79 token gate (token-gate.js, phase 79 task 05). -BASE_SCRIPT_COUNT = 5 +#: SIX classic/module script tags: the phase-39 brand.js classic layer +#: + markdown.js + the phase-182 verbatim.js hydrator (a classic script +#: loaded AFTER markdown.js, which is what put it in this count) + the +#: chat module (app.js) + the shell router (router.js, which +#: lazy-imports the tuning.js view module) + the phase-79 token gate +#: (token-gate.js, phase 79 task 05). +BASE_SCRIPT_COUNT = 6 async def _import_fixtures(mock_port: int) -> ImportSummary: diff --git a/tests/e2e/test_suggestion_chips.py b/tests/e2e/test_suggestion_chips.py index 827b9cc..16a64ef 100644 --- a/tests/e2e/test_suggestion_chips.py +++ b/tests/e2e/test_suggestion_chips.py @@ -463,10 +463,14 @@ def test_chips_keyboard_accessible( assert chip_text # Tab from the page start: the first chip must be reachable on the - # keyboard, and before the composer input (skip-link + 2 nav links come - # first). Track tab stops until we land on a chip. + # keyboard, and before the composer input. What comes FIRST has grown + # with the nav itself — skip-link + the eight admin links (Chat, RAG, + # Sources, Tuning, History, Tokens, Theme, and the phase-168 Settings + # panel) + the header controls — so the walk allowance is the whole + # header, not the phase-05 two-link bar. The contract under test is the + # ORDER (a chip before the composer), pinned by the input check below. reached_chip_at: int | None = None - for step in range(1, 11): + for step in range(1, 25): page.keyboard.press("Tab") state = page.evaluate( """() => { diff --git a/tests/integration/test_import_fast_path_columns.py b/tests/integration/test_import_fast_path_columns.py new file mode 100644 index 0000000..7d475a0 --- /dev/null +++ b/tests/integration/test_import_fast_path_columns.py @@ -0,0 +1,749 @@ +"""Integration: phase 185 — what the UNCHANGED fast path SELECTs (the probe). + +**VERDICT: CONFIRMED (task 01) → FIXED (phase 185 task 02, +``defer(Document.content)`` in ``app/rag/importer.py::_doc_row_stmt``).** +The fast path used to SELECT ``content`` for every +file it inspects. Each inspected file is looked up by +``app/rag/importer.py::_index_file`` (``select(Document).where( +Document.source == source, Document.path == rel)``), which is a WHOLE-ENTITY +load: its select list names ``documents.content`` — the full stored markdown, +**15,217** chars on average — next to the metadata the fast path actually +reads. The three sibling loads that share the phase-147 unchanged path (the +PDF branch, the PDF-embedded-image branch, the standalone-image branch) +compile to the same statement, so the whole KB walk pays this for EVERY file +it looks at, not only for the ones that changed. Nothing is released +afterwards: ``app/db.py:30`` builds every session with +``expire_on_commit=False``, so a row the run read — and then dirtied with the +``file_hash`` stamp (phase 184) — stays resident with its markdown until the +run ends. + +**What the fix changed (measured by this file, post-fix).** The four fast-path +row loads now share ``_doc_row_stmt``, whose projection is every ``Document`` +column EXCEPT ``content``, and the unchanged run issues **7** documents +SELECTs for the same four files: the **4** metadata-only row loads, **1** +primary-key follow-up (``SELECT documents.content AS documents_content … +WHERE documents.id = …``) — paid by the ONE summary-NULL row whose phase-118 +backfill genuinely reads the stored markdown — and the same **2** per-source +post-passes. A file that already has a summary still costs exactly **1** +documents SELECT and zero follow-ups (``test_unchanged_file_is_one_select``). + +**The incident (owner, 2026-10-15 — the same run phases 183/184 fixed).** +``documents`` held **17,407** rows at ``avg(length(content)) = 15,217`` +characters = **252.6 MB of markdown**. The sync inspected ~27k files and +re-indexed **217**; the remaining ~26.8k took the fast path and still hydrated +their markdown to discover that nothing had changed. Phase 183 took ``content`` +out of the prune's row (six light columns), phase 184 bounded what the walk +RETAINS (``session.expunge_all()`` every ``BOR_SYNC_SESSION_RESET_FILES`` +files); this phase takes the markdown out of what the walk READS per unchanged +file, so a full re-sync's memory scales with row metadata instead of with the +corpus. + +**Measured BY THIS FILE (2026-10-15, the dev Postgres; fixture: 4 markdown +files of ~8 KB seeded through the real ``import_sources``, then a SECOND +``import_sources`` over the same tree — the unchanged run — captured with +``before_cursor_execute``, the phase-183 task-01 technique).** PRE-FIX the +unchanged run issued **6** statements over ``documents``: + +* **4** per-file ``(source, path)`` row loads — exactly one per inspected + file, the fast path's ONE lookup, every one of them naming + ``documents.content`` (the load-bearing pin, flipped by task 02); +* **0** primary-key follow-up SELECTs over ``documents`` — nothing was + deferred yet, so nothing could be re-read; every file that already has a + summary costs exactly **1** documents SELECT in total; +* **2** per-source post-pass walks (``_enrich_pending_images`` + + ``_backfill_image_contexts``, both ``WHERE documents.source = … AND + documents.is_image IS true``) — twice per RUN, independent of the file + count, and outside this phase's scope (D1 names exactly the four fast-path + row loads); +* the one staged row (``documents.summary = NULL``, its stale ``is_summary`` + chunk stamped so replacement is observable) cost **1** documents SELECT + total — the row load, whose already-hydrated ``content`` the phase-118 + backfill consumed directly. Post-task-02 that row is the ONLY one that pays + a second, single-column SELECT. + +The captured fast-path statement, verbatim for one file **POST-FIX** — every +column of the PRE-FIX list below EXCEPT ``documents.content`` (which sat +between ``title`` and ``content_hash``), so no markdown travels with an +inspected row; the fast path reads ``file_hash``, then ``summary`` / +``created_at`` / ``page_count`` / ``path`` / ``full_path`` — never the text +this projection drops: + +``` +SELECT documents.id, documents.source, documents.path, documents.full_path, +documents.title, documents.content_hash, documents.file_hash, +documents.indexed_at, documents.created_at, documents.created_at_manual, +documents.summary, documents.is_image, documents.image_path, +documents.image_context, documents.page_count, documents.page_strip_mode +FROM documents +WHERE documents.source = %(source_1)s::VARCHAR AND documents.path = +%(path_1)s::VARCHAR +``` + +**The PRE-FIX shape** (task 01's evidence — the statement above WITH +``documents.content`` between ``title`` and ``content_hash``, which is what +put ~250 MB of markdown in front of a run that re-indexed 217 of ~27k files) +and **the follow-up a deferred row pays when it genuinely needs the +markdown** — the phase-118 summary-NULL backfill, ``_unchanged_doc`` step 3, +and nothing else on the fast path (measured 2026-10-15 by putting the same +row behind an ad-hoc ``defer(Document.content)`` load and then reading +``doc.content``):: + + SELECT documents.content AS documents_content + FROM documents + WHERE documents.id = %(pk_1)s::UUID + +That is the whole trade (phase 185 D2 / LOCKED A2): a doc WITH a stored +summary pays ZERO extra queries, a summary-NULL backfill pays ONE per-row +one-time SELECT, and deliberately NO ``raiseload``/``noload`` — those would +turn the backfill into a crash instead of a query. + +**What this file does NOT touch:** the two per-source post-pass walks above, +``_prune``'s light row (phase 184), the identity-map cadence (phase 184), +``app/db.py:30``, ``iter_importable_files`` (phase 185 LOCKED A3) and the +``doc.chunks = [...]`` re-embed path (phase 183 LOCKED A2). +""" +from __future__ import annotations + +import asyncio +import logging +import re +from collections.abc import Iterator +from pathlib import Path +from typing import Any, NamedTuple + +import pytest +from sqlalchemy import event, func, select, text +from sqlalchemy.orm import Session + +from app.config import Settings +from app.db import SessionLocal, engine +from app.models import Chunk, Document +from app.rag.importer import ImportSummary, import_sources +from tests.fakes import FakeEmbedder + +# --------------------------------------------------------------------------- +# The fixture: 4 markdown files of ~8 KB, one seed run + one unchanged run +# --------------------------------------------------------------------------- + +#: The probe's own source name — nothing else in the KB uses it. +PROBE_SOURCE = "fastpath-probe" + +#: Four files: enough that "one row load per file" is arithmetic rather than +#: a single sample, few enough that the seed run is instant. +FILE_COUNT = 4 + +#: The row whose stored ``summary`` is nulled before the second run — the +#: phase-118 backfill case, the ONE fast-path branch that genuinely needs the +#: stored markdown. +BACKFILL_PATH = "probe-02.md" + +#: What the pre-existing ``is_summary`` chunk of that row is stamped with, so +#: "the backfill replaced the chunk" is observable instead of assumed (the +#: replacement is the only thing that can make the chunk read ``expected``). +STALE_SUMMARY = "STALE SUMMARY CHUNK — the backfill must replace this" + +#: One repeated sentence, so every file has the same shape (and therefore the +#: same chunk count) as every other. +_SENTENCE = ( + "The homelab runs podman quadlets on the hnb host with a static DNS " + "zone, nightly restic backups and a systemd timer that probes uptime. " +) + +#: ~8 KB per file — production ``content`` averages 15.2 KB, so a whole-entity +#: row load is tens of kilobytes per inspected file and ``content`` is +#: unmistakably the fat column of the row (and stays under +#: ``BOR_SUMMARY_MAX_CHARS``, so the backfill's summary call is uncapped). +CONTENT_CHARS_PER_FILE = 8_000 + +#: The documents SELECTs of the two per-source post-passes, measured +#: 2026-10-15 — asserted so a future per-file extra SELECT cannot hide among +#: them (and so a post-pass that stopped querying is noticed too). +POST_PASS_DOC_SELECTS = 2 + +#: **0 pre-phase-185** — the whole-entity row load already carries +#: ``content``, so the phase-118 backfill needs no follow-up query. **1 +#: **0 pre-phase-185** — the whole-entity row load already carried +#: ``content``, so the phase-118 backfill needed no follow-up query. **1 +#: post-phase-185 (the value now)** — the row load defers ``content``, so the +#: ONE backfill pays exactly ONE lazy ``SELECT documents.content … WHERE +#: documents.id = …``. The arithmetic: the backfilled file costs ``1 row load +#: + EXPECTED_EXTRA_DOC_SELECTS_FOR_BACKFILL`` documents SELECTs, every other +#: unchanged file costs exactly 1 (``test_unchanged_file_is_one_select``). +#: Everything else about the backfill is identical across the flip: the stored +#: summary, its ``is_summary`` chunk, the run's counts and its log words +#: (``test_unchanged_run_keeps_every_row_intact``). +EXPECTED_EXTRA_DOC_SELECTS_FOR_BACKFILL = 1 + + +class Captured(NamedTuple): + """One executed statement plus its bind parameters. + + The parameters make the probe attributable: the fast-path row load carries + ``path_1`` (the file it looked up) while the deferred column's lazy + follow-up carries only the primary key — so "one SELECT per unchanged + file" and "one follow-up for the backfill" are two different, separately + asserted claims. + """ + + text: str + params: dict[str, Any] + + +class UnchangedRun(NamedTuple): + """The unchanged run, snapshotted at the moment it ENDED. + + ``captured`` is a copy: the listener stays installed for the rest of the + test, so the assertions must not race the probe's own read queries. + """ + + captured: list[Captured] + summary: ImportSummary + llm: FakeEmbedder + + @property + def lite_prompts(self) -> list[str]: + """The user-message text of every ``lite`` call the run made.""" + return [ + str(message.get("content")) + for call in self.llm.chat_calls + for message in call + if message.get("role") == "user" + ] + + +def _file_body(n: int) -> str: + """One ~8 KB markdown document, carrying a per-file sentinel per section.""" + parts = [f"# Probe document {n}\n"] + section = 0 + while sum(len(p) for p in parts) < CONTENT_CHARS_PER_FILE: + parts.append(f"\n## Section {section} of probe {n} (fastpath-sentinel-{n})\n\n") + parts.extend(_SENTENCE for _ in range(4)) + parts.append("\n") + section += 1 + return "".join(parts) + + +def _seed_files(tmp: Path) -> Path: + """``FILE_COUNT`` markdown files under a fresh source root; returns it.""" + root = tmp / PROBE_SOURCE + root.mkdir(parents=True, exist_ok=True) + for n in range(FILE_COUNT): + body = _file_body(n) + assert 8_000 <= len(body) <= 9_000, len(body) + (root / f"probe-{n:02d}.md").write_text(body) + return root + + +def _probe_llm() -> FakeEmbedder: + """The deterministic fake embedder/summarizer on the SHIPPED defaults + (``sync_session_reset_files=200`` — no identity-map release lands inside a + four-file walk, so nothing but the row loads is in play here).""" + llm = FakeEmbedder() + llm.settings = Settings(_env_file=None) # pyright: ignore[reportCallIssue] + return llm + + +def _run_import(root: Path, llm: FakeEmbedder) -> ImportSummary: + """One real ``import_sources`` pass over *root*, in its own session.""" + session = SessionLocal() + try: + return asyncio.run( + import_sources([root], llm, prune=False, session=session) + ) + finally: + session.close() + + +def _doc_id(db: Session, path: str) -> Any: + """The id of one probe row (lazy follow-ups are keyed by primary key).""" + doc_id = db.scalar( + select(Document.id).where( + Document.source == PROBE_SOURCE, Document.path == path + ) + ) + assert doc_id is not None + return doc_id + + +def _stage_the_backfill(db: Session, path: str) -> None: + """The phase-118 state: a row whose stored ``summary`` is NULL while its + content and ``file_hash`` are current (the direct UPDATE of + ``tests/integration/test_importer_dates.py`` — exactly what a pre-phase-118 + import left behind). + + The stale ``is_summary`` chunk is stamped rather than deleted: after the + run a chunk carrying the backfill text can only have got there through + :func:`app.rag.importer._store_summary`'s replacement rule, which is what + makes "the backfill ran" a fact rather than an assumption. + """ + doc_id = _doc_id(db, path) + db.execute( + text( + "UPDATE documents SET summary = NULL WHERE source = :s AND path = :p" + ), + {"s": PROBE_SOURCE, "p": path}, + ) + db.execute( + text("UPDATE chunks SET content = :stale WHERE document_id = :d AND is_summary"), + {"stale": STALE_SUMMARY, "d": doc_id}, + ) + db.commit() + db.expunge_all() + + +def _two_runs(db: Session, tmp_path: Path, captured: list[Captured]) -> UnchangedRun: + """Seed the KB, then run the UNCHANGED pass with the capture active. + + The seed run's SQL is cleared away at the boundary: this file pins what + the unchanged walk costs, not what first indexing costs. + """ + root = _seed_files(tmp_path) + first = _run_import(root, _probe_llm()) + assert (first.files, first.added, first.unchanged, first.errors) == ( + FILE_COUNT, + FILE_COUNT, + 0, + 0, + ) + _stage_the_backfill(db, BACKFILL_PATH) + + captured.clear() + llm = _probe_llm() + second = _run_import(root, llm) + run = UnchangedRun(list(captured), second, llm) + + assert (second.files, second.added, second.unchanged, second.errors) == ( + FILE_COUNT, + 0, + FILE_COUNT, + 0, + ) + assert second.chunks == 0 # the fast path re-embeds no content + assert second.summary_backfilled == 1 # the staged row was backfilled + return run + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +@pytest.fixture(autouse=True) +def clean_kb(db: Session) -> Iterator[None]: + """``documents`` / ``chunks`` are global state: reset around every test + (the ``test_sync_api.py`` truncate pattern) and drop whatever the session + cached, so the next read is the database's truth.""" + db.execute(text("TRUNCATE chunks, documents")) + db.commit() + db.expunge_all() + yield + db.execute(text("TRUNCATE chunks, documents")) + db.commit() + db.expunge_all() + + +@pytest.fixture() +def statement_capture() -> Iterator[list[Captured]]: + """Every SQL the engine executes for the duration of ONE test. + + The ``before_cursor_execute`` technique of phase 183 task 01 + (``tests/integration/test_prune_cascade_statements.py``): the column list + a SELECT uses is invisible in the ORM's Python API and plain to see in the + statement it compiles, so a capture is the only deterministic way to pin + what the fast path's row actually contains. Installed on the engine and + REMOVED in teardown, so no other test of the session ever sees the + listener (verified by the full ``uv run pytest`` run being green with this + file in it). + """ + captured: list[Captured] = [] + + def _listener( + conn: object, # noqa: ARG001 + cursor: object, # noqa: ARG001 + statement: str, + parameters: object, + context: object, # noqa: ARG001 + executemany: bool, # noqa: ARG001 + ) -> None: + params = dict(parameters) if isinstance(parameters, dict) else {} + captured.append(Captured(statement, params)) + + event.listen(engine, "before_cursor_execute", _listener) + try: + yield captured + finally: + event.remove(engine, "before_cursor_execute", _listener) + + +# --------------------------------------------------------------------------- +# Helpers — reading a captured statement +# --------------------------------------------------------------------------- + +#: ``… FROM documents`` — case-insensitive, word-bounded. +_FROM_DOCUMENTS_RE = re.compile(r"\bfrom\s+documents\b", re.IGNORECASE) + +#: Where the projection ends: SQLAlchemy compiles ``FROM`` onto its own line, +#: and everything after it is a predicate, not a column the row hydrates. +_FROM_RE = re.compile(r"\bFROM\b") + +#: The ``content`` COLUMN TOKEN, aliased or plain. The trailing ``\\b`` is what +#: keeps the neighbours out: ``_`` is a WORD character, so the ``documents.content`` +#: inside ``documents.content_hash`` has no boundary after the ``t`` and never +#: matches. That is why this is a token regex and not a substring test — +#: ``content_hash`` is in EVERY one of these select lists, and a substring +#: match would report a content load for every row of every run, pre- and +#: post-fix alike. +_CONTENT_TOKEN_RE = re.compile(r"\bdocuments\.content\b") + + +def doc_selects(captured: list[Captured]) -> list[Captured]: + """The captured statements that SELECT the ``documents`` table.""" + return [ + c + for c in captured + if c.text.lstrip().lower().startswith("select") + and _FROM_DOCUMENTS_RE.search(c.text) + ] + + +def loads_content(statement: Captured | str) -> bool: + """``True`` when the statement's SELECT LIST names the ``content`` column. + + Only the projection is inspected (see :data:`_FROM_RE`), and only the + column token matches (see :data:`_CONTENT_TOKEN_RE`). + """ + text = statement.text if isinstance(statement, Captured) else statement + from_at = _FROM_RE.search(text) + projection = text[: from_at.start()] if from_at else text + return _CONTENT_TOKEN_RE.search(projection) is not None + + +def test_the_content_matcher_names_the_column_and_nothing_else() -> None: + """The matcher's own pin — every other assertion here rests on it. + + ``SELECT … documents.content, documents.content_hash …`` is a content load + because of the FIRST token; ``documents.content_hash`` alone is not. If + this ever stops holding, the pre-/post-fix assertion above is meaningless + in BOTH directions, so it is asserted directly rather than trusted. + """ + assert loads_content( + "SELECT documents.id, documents.content, documents.content_hash\n" + "FROM documents\nWHERE documents.id = %(pk_1)s" + ) + assert loads_content( + "SELECT documents.content AS documents_content\nFROM documents\n" + "WHERE documents.id = %(pk_1)s" + ) + assert not loads_content( + "SELECT documents.id, documents.content_hash, documents.file_hash\n" + "FROM documents\nWHERE documents.source = %(source_1)s" + ) + assert not loads_content( + "SELECT documents.summary, documents.is_image, documents.page_count\n" + "FROM documents\nWHERE documents.id = %(pk_1)s" + ) + # A predicate mention is not a hydration. + assert not loads_content( + "SELECT documents.id\nFROM documents\nWHERE documents.content = %(c_1)s" + ) + + +def _predicate(statement: str) -> str: + """Everything after the first ``FROM`` — the statement's predicate.""" + from_at = _FROM_RE.search(statement) + return statement[from_at.end():] if from_at else "" + + +def row_loads(captured: list[Captured]) -> list[Captured]: + """The ``(source, path)`` row loads — the fast path's ONE lookup per file. + + Matched on the PREDICATE, not the projection: the four sites of phase 185 + D1 (``_index_file`` plus the PDF / PDF-embedded-image / image branch loads) + all compile to ``WHERE documents.source = … AND documents.path = …``. The + two per-source post-pass walks filter on ``documents.is_image`` instead + and are therefore excluded here — they are not the fast path's row load. + """ + return [ + c + for c in doc_selects(captured) + if "documents.source" in _predicate(c.text) + and "documents.path" in _predicate(c.text) + ] + + +def pk_followups(captured: list[Captured]) -> list[Captured]: + """Documents SELECTs keyed by PRIMARY KEY — the deferred-column shape. + + A ``defer``-red column loads as ``SELECT documents.content … WHERE + documents.id = …``: no ``source``/``path`` predicate at all. Pre-fix this + list is EMPTY (the row was loaded whole, so nothing is deferred); post-fix + it holds the ONE query the phase-118 backfill pays. + """ + return [ + c + for c in doc_selects(captured) + if "documents.source" not in _predicate(c.text) + and "documents.path" not in _predicate(c.text) + ] + + +def _summary_chunks(db: Session, path: str) -> list[Chunk]: + """The ``is_summary`` chunks of one document, read by ``document_id`` so + the parent row's ``content`` is never what this probe reads.""" + return list( + db.scalars( + select(Chunk) + .where( + Chunk.document_id == _doc_id(db, path), + Chunk.is_summary.is_(True), + ) + .order_by(Chunk.id) + ) + ) + + +# --------------------------------------------------------------------------- +# The probe +# --------------------------------------------------------------------------- + + +def test_unchanged_run_defers_content_post_fix( + db: Session, tmp_path: Path, statement_capture: list[Captured] +) -> None: + """THE PROBE (post-fix): no file the unchanged walk inspects SELECTs its + markdown. + + Four unchanged files → four ``(source, path)`` row loads, and NONE of them + names ``documents.content`` in its projection — the fast path reads row + metadata only (phase 185, ``_doc_row_stmt``). The only follow-up the run + may pay is the backfill row's, attributed per primary key in + :func:`test_summary_null_backfill_reads_stored_content`, while + :func:`test_unchanged_file_is_one_select` pins that every + summary-bearing file pays none. The rest of the run's documents traffic + is the two per-source image post-passes, pinned apart so a future + per-file extra SELECT cannot hide inside the total. + + (Task 01's PRE-fix assertion was the inverse — ``all(loads_content(c))`` + with zero follow-ups — and flipped here exactly as planned. A deferred + column that is genuinely READ still shows up, as a PK follow-up, so + neither a re-appeared ``content`` nor a new lazy reader hides here.) + """ + run = _two_runs(db, tmp_path, statement_capture) + + selects = doc_selects(run.captured) + loads = row_loads(run.captured) + assert len(loads) == FILE_COUNT, ( + f"a {FILE_COUNT}-file unchanged walk issued {len(loads)} (source, " + "path) row loads — the fast path looks each file up exactly once" + ) + assert len(selects) >= FILE_COUNT + + # Post-fix (phase 185 task 02): no content in the fast-path SELECT. + assert not any(loads_content(c) for c in loads), ( + "a fast-path row load SELECTs documents.content again — the deferral " + "in _doc_row_stmt is gone or bypassed, which re-hydrates the corpus " + "(252.6 MB on the incident box) for a walk that reads metadata only" + ) + followups = pk_followups(run.captured) + assert len(followups) == EXPECTED_EXTRA_DOC_SELECTS_FOR_BACKFILL, ( + f"an unchanged run over {FILE_COUNT} files (one with summary IS NULL) " + f"paid {len(followups)} primary-key follow-up SELECTs over documents " + f"while EXPECTED_EXTRA_DOC_SELECTS_FOR_BACKFILL is " + f"{EXPECTED_EXTRA_DOC_SELECTS_FOR_BACKFILL} — the backfill's single " + "deferred-column read is the only one the fast path may need" + ) + post_passes = [c for c in selects if c not in loads and c not in followups] + assert len(post_passes) == POST_PASS_DOC_SELECTS, ( + f"the unchanged run did {len(post_passes)} documents SELECTs beyond " + f"the {FILE_COUNT} row loads — expected the {POST_PASS_DOC_SELECTS} " + "per-source image post-passes and nothing else" + ) + + +def test_unchanged_file_is_one_select( + db: Session, tmp_path: Path, statement_capture: list[Captured] +) -> None: + """One documents SELECT per unchanged file — MUST STAY GREEN POST-FIX. + + A file WITH a stored summary reads nothing beyond what its row load + already carries: the ``file_hash`` comparison, then ``_unchanged_doc``'s + ``summary`` / ``created_at`` / ``page_count`` bookkeeping. Pre-fix that is + one whole-entity load; post-fix it must still be exactly ONE — this is the + guard that ``defer(Document.content)`` does not decay into a lazy-load + storm (``summary``, then ``created_at``, then ``page_count`` … one SELECT + apiece), which would trade 15 KB of residency for five round trips. + """ + run = _two_runs(db, tmp_path, statement_capture) + + for n in range(FILE_COUNT): + path = f"probe-{n:02d}.md" + if path == BACKFILL_PATH: + continue # asserted separately, with its documented extra + one_file = [c for c in row_loads(run.captured) if c.params.get("path_1") == path] + assert one_file, ( + f"{path} was walked with no (source, path) row load carrying " + "path_1 — the row-load statement shape changed, so this probe is " + "attributing nothing" + ) + assert len(one_file) == 1, ( + f"{path} cost {len(one_file)} documents row loads — the fast path " + "looks a file up ONCE" + ) + doc_id = str(_doc_id(db, path)) + followups = [ + c for c in pk_followups(run.captured) if str(c.params.get("pk_1")) == doc_id + ] + assert followups == [], ( + f"{path} paid {len(followups)} deferred-column follow-up SELECT(s) " + "despite having a stored summary — nothing on its fast path may " + "read content, so a follow-up means a new reader crept in" + ) + + +def test_summary_null_backfill_reads_stored_content( + db: Session, tmp_path: Path, statement_capture: list[Captured] +) -> None: + """The ONE branch that genuinely needs the markdown: ``summary IS NULL``. + + ``_unchanged_doc`` step 3 (phase 118, A2) backfills the summary FROM THE + STORED ``doc.content`` — never by re-reading the file, re-extracting or + re-describing. So the probe pins the behaviour AND its cost: + + * the run backfills exactly one summary (``summary_backfilled=1`` while + ``summaries`` stays ``0`` — a backfill is not a KB-change signal), the + stored summary comes back and its single ``is_summary`` chunk is the + REPLACED one (the pre-existing chunk still carried :data:`STALE_SUMMARY` + when the run started, and replacement is the only way it changes); + * the text the ``lite`` call was given IS the stored markdown (~8 KB, with + the file's own sentinel in every section) — the bytes travelled from + ``documents.content``, not from a re-read of the file; + * the documents SELECT count for that file is ``1 + + EXPECTED_EXTRA_DOC_SELECTS_FOR_BACKFILL``: the row load plus, once task + 02 defers ``content``, exactly one lazy follow-up keyed by primary key + (D2 — a per-row, one-time, already-rare cost, accepted precisely instead + of a ``raiseload`` that would crash this branch where it should query). + """ + run = _two_runs(db, tmp_path, statement_capture) + n = int(BACKFILL_PATH.removeprefix("probe-").removesuffix(".md")) + expected = f"Summary of #\nSource: {PROBE_SOURCE}/{BACKFILL_PATH}" + + # The backfill happened, from the stored content. + assert run.summary.summary_backfilled == 1 + assert run.summary.summaries == 0 + stored = db.scalar( + select(Document.content).where( + Document.source == PROBE_SOURCE, Document.path == BACKFILL_PATH + ) + ) + assert stored is not None and len(stored) >= 8_000 + assert f"fastpath-sentinel-{n}" in stored + + doc = db.scalar( + select(Document).where( + Document.source == PROBE_SOURCE, Document.path == BACKFILL_PATH + ) + ) + assert doc is not None + assert doc.summary == expected + assert doc.file_hash is not None + + chunks = _summary_chunks(db, BACKFILL_PATH) + assert [c.content for c in chunks] == [expected], ( + "the summary chunk was not replaced by the backfill — a stale chunk " + "survived, or more than one is_summary chunk exists" + ) + assert chunks[0].position == -1 + assert chunks[0].embedding is not None + + # The lite call got the STORED markdown, sentinel and all — uncapped. + assert run.lite_prompts, "the backfill made no lite-model call" + assert any(stored == prompt for prompt in run.lite_prompts), ( + "the summary model was not fed documents.content verbatim — the " + "phase-118 backfill reads something other than the stored markdown" + ) + + # And the cost of that branch, as a documented constant. + doc_id = str(_doc_id(db, BACKFILL_PATH)) + loads = [c for c in row_loads(run.captured) if c.params.get("path_1") == BACKFILL_PATH] + followups = [c for c in pk_followups(run.captured) if str(c.params.get("pk_1")) == doc_id] + assert len(loads) == 1 + assert len(followups) == EXPECTED_EXTRA_DOC_SELECTS_FOR_BACKFILL, ( + f"the summary-NULL backfill paid {len(followups)} lazy content " + f"SELECT(s) while EXPECTED_EXTRA_DOC_SELECTS_FOR_BACKFILL is " + f"{EXPECTED_EXTRA_DOC_SELECTS_FOR_BACKFILL} — 0 while the row load " + "carries content (pre task 02), 1 once content is deferred" + ) + assert all(loads_content(c) for c in followups) + + +def test_unchanged_run_keeps_every_row_intact( + db: Session, tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """The observable contract the deferral must not move (phase 185 D4). + + The reference for task 02's byte-identity claim: the unchanged run's whole + ``ImportSummary`` shape and the whole KB after it — four unchanged files, + one backfill, no re-embed, no date refresh, every file still holding its + full ~8 KB of content, its summary and all of its chunks. ``changed_paths`` + stays empty (the backfill is not a document change, so the folder-summary + regeneration never sees it) and no source is pruned. + """ + root = _seed_files(tmp_path) + first = _run_import(root, _probe_llm()) + assert (first.files, first.added, first.summary_backfilled) == ( + FILE_COUNT, + FILE_COUNT, + 0, + ) + content_chunks_after_seed = db.scalar( + select(func.count()).select_from(Chunk).where(Chunk.is_summary.is_(False)) + ) + assert content_chunks_after_seed is not None and content_chunks_after_seed > FILE_COUNT + _stage_the_backfill(db, BACKFILL_PATH) + + caplog.clear() + with caplog.at_level(logging.INFO, logger="app.importer"): + second = _run_import(root, _probe_llm()) + messages = [r.getMessage() for r in caplog.records if r.name == "app.importer"] + + # The phase-147/118 log words, verbatim (D4: task 02 may change WHICH + # COLUMNS travel, never what the run says it did). ``import: summary + # source=`` is the FRESH-summary line — it must not appear on an unchanged + # run, where only the backfill's own wording is allowed. + assert sum(m.startswith("import: unchanged ") for m in messages) == FILE_COUNT + assert sum(m.startswith("import: summary-backfill ") for m in messages) == 1 + assert not [m for m in messages if m.startswith("import: summary source=")] + assert not [m for m in messages if m.startswith("import: date-refreshed ")] + assert not [m for m in messages if m.startswith("import: added ")] + assert not [m for m in messages if m.startswith("import: updated ")] + + assert (second.files, second.added, second.updated) == (FILE_COUNT, 0, 0) + assert (second.unchanged, second.pruned, second.errors) == (FILE_COUNT, 0, 0) + assert second.summary_backfilled == 1 + assert second.summaries == 0 + assert second.summary_errors == 0 + assert second.dates_updated == 0 + assert second.chunks == 0 + assert second.changed_paths == [] + assert second.formats == {"md": FILE_COUNT} + assert second.format_counts() == "md:4" + + # Nothing was re-embedded, nothing was lost. + assert db.scalar( + select(func.count()).select_from(Chunk).where(Chunk.is_summary.is_(False)) + ) == content_chunks_after_seed + assert db.scalar( + select(func.count()) + .select_from(Document) + .where(Document.source == PROBE_SOURCE, Document.summary.is_not(None)) + ) == FILE_COUNT + assert all( + len(c) >= 8_000 + for c in db.scalars( + select(Document.content).where(Document.source == PROBE_SOURCE) + ) + ) + # Exactly one summary chunk per document (the backfill replaced, not added). + assert db.scalar( + select(func.count()).select_from(Chunk).where(Chunk.is_summary.is_(True)) + ) == FILE_COUNT diff --git a/tests/integration/test_import_session_retention.py b/tests/integration/test_import_session_retention.py index 3772be2..c798264 100644 --- a/tests/integration/test_import_session_retention.py +++ b/tests/integration/test_import_session_retention.py @@ -136,6 +136,7 @@ lazy SELECT per row and fail that test, not get papered over. ``scripts/git_sync.py`` (phase 186), ``app/db.py:30``, and the ``doc.chunks = [...]`` re-embed path (phase 183 LOCKED A2). """ + from __future__ import annotations import asyncio @@ -189,9 +190,7 @@ EXPECTED_SUMMARY_CHUNKS = 12 #: Every ORM object a full ADD run creates: the 12 documents, their content #: chunks and their summary chunks. What the identity map would have to #: hold if the ADDED path retained what the phase-184 baseline describes. -OBJECTS_CREATED_BY_ADD_RUN = ( - FILE_COUNT + EXPECTED_CONTENT_CHUNKS + EXPECTED_SUMMARY_CHUNKS -) +OBJECTS_CREATED_BY_ADD_RUN = FILE_COUNT + EXPECTED_CONTENT_CHUNKS + EXPECTED_SUMMARY_CHUNKS #: The measured number of identity-map entries at the end of an UNCHANGED #: run with the cadence OFF — one per file walked, each a full ``Document`` @@ -322,9 +321,7 @@ def _kb_snapshot(db: Session) -> dict[str, tuple[str, str | None, str, int]]: } snapshot: dict[str, tuple[str, str | None, str, int]] = {} for doc in db.scalars( - select(Document) - .where(Document.source == PROBE_SOURCE) - .order_by(Document.path) + select(Document).where(Document.source == PROBE_SOURCE).order_by(Document.path) ): assert doc.file_hash is not None # the phase-147 stamp is always set snapshot[doc.path] = ( @@ -373,9 +370,7 @@ def test_the_added_run_holds_its_objects_only_until_a_collection( root = _seed_files(tmp_path) session = SessionLocal() try: - summary = asyncio.run( - import_sources([root], _probe_llm(), prune=False, session=session) - ) + summary = asyncio.run(import_sources([root], _probe_llm(), prune=False, session=session)) assert (summary.files, summary.added, summary.unchanged) == ( FILE_COUNT, FILE_COUNT, @@ -405,9 +400,7 @@ def test_the_added_run_holds_its_objects_only_until_a_collection( session.close() -def test_the_unchanged_run_keeps_every_document_it_walks( - db: Session, tmp_path: Path -) -> None: +def test_the_unchanged_run_keeps_every_document_it_walks(db: Session, tmp_path: Path) -> None: """THE PROBE: a repeat sync holds one full ``Document`` per file walked. The production-shaped case — the second sync of the same source, every @@ -446,9 +439,7 @@ def test_the_unchanged_run_keeps_every_document_it_walks( staircase.append(_map_size(session)) summary = asyncio.run( - import_sources( - [root], _probe_llm(), prune=False, session=session, progress=progress - ) + import_sources([root], _probe_llm(), prune=False, session=session, progress=progress) ) assert (summary.files, summary.added, summary.unchanged) == ( FILE_COUNT, @@ -459,8 +450,7 @@ def test_the_unchanged_run_keeps_every_document_it_walks( # One row resident per file already walked — a staircase, never a # plateau and never a release. assert staircase == list(range(FILE_COUNT)), ( - "expected exactly one document retained per file already walked," - f" got {staircase}" + f"expected exactly one document retained per file already walked, got {staircase}" ) assert _map_size(session) == MEASURED_UNCHANGED_MAP_SIZE @@ -643,8 +633,7 @@ def test_reset_cadence_bounds_the_unchanged_walk(db: Session, tmp_path: Path) -> FILE_COUNT, ) assert staircase == [n % RESET_FILES for n in range(FILE_COUNT)], ( - f"the bounded walk sampled {staircase} — expected a release every" - f" {RESET_FILES} files" + f"the bounded walk sampled {staircase} — expected a release every {RESET_FILES} files" ) assert max(staircase) < MEASURED_UNCHANGED_MAP_SIZE assert _map_size(session) == 0, ( @@ -682,14 +671,10 @@ def test_reset_cadence_survives_an_embedding_error(db: Session, tmp_path: Path) session = SessionLocal() try: - summary = asyncio.run( - import_sources([root], llm, prune=False, session=session) - ) + summary = asyncio.run(import_sources([root], llm, prune=False, session=session)) assert (summary.files, summary.errors) == (FILE_COUNT, 1) assert summary.added == FILE_COUNT - 1 - assert summary.chunks == EXPECTED_CONTENT_CHUNKS - ( - EXPECTED_CONTENT_CHUNKS // FILE_COUNT - ) + assert summary.chunks == EXPECTED_CONTENT_CHUNKS - (EXPECTED_CONTENT_CHUNKS // FILE_COUNT) assert summary.summaries == EXPECTED_SUMMARY_CHUNKS - 1 assert _map_size(session) <= MAP_BOUND finally: @@ -743,9 +728,7 @@ def test_a_clean_row_leaves_the_map_and_a_dirty_one_never_does(db: Session) -> N assert _map_size(session) == 0, "a clean committed row must be weakly held" row = session.scalar( - select(Document).where( - Document.source == PROBE_SOURCE, Document.path == "mechanism.md" - ) + select(Document).where(Document.source == PROBE_SOURCE, Document.path == "mechanism.md") ) assert row is not None row.file_hash = row.file_hash # the stamp: same value, dirty state @@ -765,9 +748,7 @@ def test_a_clean_row_leaves_the_map_and_a_dirty_one_never_does(db: Session) -> N assert _map_size(session) == 0 assert ( db.scalar( - select(func.count()) - .select_from(Document) - .where(Document.source == PROBE_SOURCE) + select(func.count()).select_from(Document).where(Document.source == PROBE_SOURCE) ) == 1 ) @@ -801,9 +782,7 @@ PRUNE_BATCH = 10 #: The SIX columns the prune loads — the ``load_only`` set of ``_prune`` #: (docstring: the light-row contract). An eighth name here means the walk #: got heavier; one missing means a guard lost the column it reads. -PRUNE_COLUMNS = frozenset( - {"id", "source", "path", "is_image", "image_path", "page_count"} -) +PRUNE_COLUMNS = frozenset({"id", "source", "path", "is_image", "image_path", "page_count"}) #: ``… FROM documents`` — the prune's own walk AND (that is the pin) any #: per-document follow-up lazy load of a deferred column. The word boundary @@ -862,9 +841,7 @@ def _projected_columns(statement: str) -> frozenset[str]: return frozenset(re.findall(r"documents\.\w+", head)) -def _seed_prune_docs( - session: Session, *, page_dir: Path | None = None -) -> list[UUID]: +def _seed_prune_docs(session: Session, *, page_dir: Path | None = None) -> list[UUID]: """``PRUNE_DOC_COUNT`` indexed docs of :data:`PRUNE_SOURCE`, each with ``PRUNE_CHUNKS_PER_DOC`` embedded chunks and a full-fat ``content``. @@ -924,9 +901,7 @@ def _arm_commit_counter(session: Session) -> list[int]: return commits -def test_prune_holds_no_content( - db: Session, tmp_path: Path, statement_capture: list[str] -) -> None: +def test_prune_holds_no_content(db: Session, tmp_path: Path, statement_capture: list[str]) -> None: """THE PRUNE PROBE: the walk loads SIX columns and never ``content``. 30 documents × 5 chunks are pruned through the importer's own ``_prune`` @@ -971,10 +946,7 @@ def test_prune_holds_no_content( "prune — a deferred column is being read per row, so the light row " "is incomplete (add the column to load_only)" ) - projected = { - col.removeprefix("documents.") - for col in _projected_columns(selects[0]) - } + projected = {col.removeprefix("documents.") for col in _projected_columns(selects[0])} assert projected == PRUNE_COLUMNS, ( f"the prune's row is not the six-column light row: {sorted(projected)}" ) @@ -985,12 +957,11 @@ def test_prune_holds_no_content( # Everything really went, children included (no orphans — phase 183 D4, # asserted here too so a light row cannot quietly change it). - assert db.scalar( - select(func.count()).select_from(Document).where(Document.source == PRUNE_SOURCE) - ) == 0 - assert db.scalar( - select(func.count()).select_from(Chunk).where(Chunk.document_id.in_(ids)) - ) == 0 + assert ( + db.scalar(select(func.count()).select_from(Document).where(Document.source == PRUNE_SOURCE)) + == 0 + ) + assert db.scalar(select(func.count()).select_from(Chunk).where(Chunk.document_id.in_(ids))) == 0 assert sorted(summary.changed_paths) == sorted( (PRUNE_SOURCE, f"docs/doc-{n:04d}.md") for n in range(PRUNE_DOC_COUNT) ) @@ -1001,9 +972,7 @@ def test_prune_holds_no_content( assert _map_classes(db) == {} -def test_prune_without_sources_touches_nothing( - db: Session, statement_capture: list[str] -) -> None: +def test_prune_without_sources_touches_nothing(db: Session, statement_capture: list[str]) -> None: """The empty-source guard is a real short-circuit, not a walk for naught. ``_prune`` with no source names returns ``0`` before the SELECT — so a @@ -1017,9 +986,10 @@ def test_prune_without_sources_touches_nothing( assert _prune(db, set(), set(), summary=summary, prune_batch=10) == 0 assert statement_capture == [] assert summary.changed_paths == [] - assert db.scalar( - select(func.count()).select_from(Document).where(Document.source == PRUNE_SOURCE) - ) == PRUNE_DOC_COUNT + assert ( + db.scalar(select(func.count()).select_from(Document).where(Document.source == PRUNE_SOURCE)) + == PRUNE_DOC_COUNT + ) def test_prune_batch_commits(db: Session) -> None: @@ -1059,11 +1029,10 @@ def test_prune_batch_commits(db: Session) -> None: session.close() # The FK cascade did the child work in EVERY transaction the # prune opened, not just the last one. - assert db.scalar( - select(func.count()) - .select_from(Chunk) - .where(Chunk.document_id.in_(ids)) - ) == 0 + assert ( + db.scalar(select(func.count()).select_from(Chunk).where(Chunk.document_id.in_(ids))) + == 0 + ) pruned, summary, commits = run(PRUNE_BATCH) assert pruned == PRUNE_DOC_COUNT @@ -1074,9 +1043,10 @@ def test_prune_batch_commits(db: Session) -> None: assert sorted(summary.changed_paths) == sorted( (PRUNE_SOURCE, f"docs/doc-{n:04d}.md") for n in range(PRUNE_DOC_COUNT) ) - assert db.scalar( - select(func.count()).select_from(Document).where(Document.source == PRUNE_SOURCE) - ) == 0 + assert ( + db.scalar(select(func.count()).select_from(Document).where(Document.source == PRUNE_SOURCE)) + == 0 + ) # …and the pre-phase run does the SAME work in ONE transaction. _truncate(db) @@ -1087,9 +1057,10 @@ def test_prune_batch_commits(db: Session) -> None: "reproduce the pre-phase single transaction exactly" ) assert sorted(summary_0.changed_paths) == sorted(summary.changed_paths) - assert db.scalar( - select(func.count()).select_from(Document).where(Document.source == PRUNE_SOURCE) - ) == 0 + assert ( + db.scalar(select(func.count()).select_from(Document).where(Document.source == PRUNE_SOURCE)) + == 0 + ) def test_prune_batch_setting_reaches_the_prune( @@ -1137,9 +1108,7 @@ def test_prune_batch_setting_reaches_the_prune( monkeypatch.setattr(importer, "_prune", spy) second = asyncio.run(import_sources([root], llm, prune=True, session=db)) - assert seen_kwargs == [3], ( - "import_sources did not pass Settings.sync_prune_batch to _prune" - ) + assert seen_kwargs == [3], "import_sources did not pass Settings.sync_prune_batch to _prune" assert (second.files, second.pruned, second.errors) == (1, 3, 0) assert sorted(path for (_src, path) in second.changed_paths) == [ "keep-0.md", @@ -1152,16 +1121,99 @@ def test_prune_batch_setting_reaches_the_prune( select(func.count()).select_from(Chunk).where(Chunk.embedding.is_not(None)) ) assert embedded is not None and embedded > 0 - assert db.scalar( - select(func.count()) - .select_from(Chunk) - .where(Chunk.document_id.not_in(select(Document.id))) - ) == 0 + assert ( + db.scalar( + select(func.count()) + .select_from(Chunk) + .where(Chunk.document_id.not_in(select(Document.id))) + ) + == 0 + ) -def test_prune_guards_unaffected_with_light_rows( - db: Session, tmp_path: Path -) -> None: +def test_the_summary_backfill_survives_the_reset_cadence(db: Session, tmp_path: Path) -> None: + """The constraint that fixes WHERE the cadence may sit, in ONE file. + + ``_unchanged_doc`` (phase 118) backfills a NULL summary by calling + :func:`app.rag.summarizer.summarize_document`, which reads + ``doc.content`` AFTER the row was loaded and its ``file_hash`` stamped. + That local is live across statements: a + ``session.expunge_all()`` moved into ``_unchanged_doc`` — or a + ``noload``/``raiseload`` added to the row it walks — would discard or + break the very row the backfill reads. Phase 185 task 01 chose + ``defer`` over ``noload``/``raiseload`` (LOCKED A2) for the same reason, + and task 05 of THIS phase pinned the choice on the statement itself + (``tests/unit/test_importer_defer_content.py``); this pins the SAME + constraint on the run. + + The fixture is a document whose ``is_summary`` chunk is missing while + its ``file_hash`` matches (a re-sync with a NULL summary — exactly the + row the backfill exists for), imported at the tightest cadence the + setting allows that still resets inside a 12-file walk. Three + observables, none of them a memory proxy: + + * the backfill RUNS — one and only one ``is_summary`` chunk appears, + carrying the fake-embedder's summary text; + * every other file is still ``unchanged`` and the summary count is + exactly the file count afterwards, i.e. the reset cadence destroyed + no summary on the way through; + * the doc's own text is still intact afterwards (``content`` was never + rewritten by a re-index). + """ + root = _seed_files(tmp_path) + seed = SessionLocal() + try: + first = asyncio.run(import_sources([root], _probe_llm(), session=seed)) + assert first.added == FILE_COUNT + finally: + seed.close() + + # Hand the next sync ONE document with a NULL summary: it takes + # ``_unchanged_doc``'s backfill branch instead of the early return. + probe_rel = "probe-00.md" + before = _kb_snapshot(db)[probe_rel] + target = db.execute( + select(Document).where(Document.source == PROBE_SOURCE, Document.path == probe_rel) + ).scalar_one() + db.query(Chunk).filter(Chunk.document_id == target.id, Chunk.is_summary.is_(True)).delete() + target.summary = None + db.commit() + db.expunge_all() + assert _kb_snapshot(db)[probe_rel][1] is None + + session = SessionLocal() + try: + summary = asyncio.run( + import_sources( + [root], _probe_llm(reset_files=RESET_FILES), prune=False, session=session + ) + ) + finally: + session.close() + # Everything stayed unchanged — the backfill never re-indexed. + assert (summary.files, summary.added, summary.updated, summary.unchanged) == ( + FILE_COUNT, + 0, + 0, + FILE_COUNT, + ) + assert (summary.errors, summary.summary_backfilled) == (0, 1) + + snap = _kb_snapshot(db) + backfilled = snap[probe_rel] + assert backfilled[1] is not None, "the summary-NULL backfill did not run" + # The one doc that needed the deferred read still has its OWN text and + # its own hash: a summary was added, the document was not rewritten. + assert backfilled[0] == before[0] + assert backfilled[2] == before[2] + # And the cadence destroyed nothing on its way through: every file is + # back to 5 content chunks + 1 summary chunk. + assert {p: c for p, (_, _, _, c) in snap.items()} == { + f"probe-{n:02d}.md": 6 for n in range(FILE_COUNT) + } + + +def test_prune_guards_unaffected_with_light_rows(db: Session, tmp_path: Path) -> None: """The guards run off SIX columns — the light row is COMPLETE. A column missing from ``load_only`` does not announce itself: the guard @@ -1216,9 +1268,7 @@ def test_prune_guards_unaffected_with_light_rows( seen = {(PRUNE_SOURCE, "notes/kept.md"), (PRUNE_SOURCE, "deck.pdf")} session = SessionLocal() try: - pruned_toggle_off = _prune( - session, {PRUNE_SOURCE}, seen, images=False, prune_batch=0 - ) + pruned_toggle_off = _prune(session, {PRUNE_SOURCE}, seen, images=False, prune_batch=0) finally: session.close() # Toggle OFF: BOTH image docs are invisible to the walk, not deleted — @@ -1228,9 +1278,7 @@ def test_prune_guards_unaffected_with_light_rows( session = SessionLocal() try: - pruned_toggle_on = _prune( - session, {PRUNE_SOURCE}, seen, images=True, prune_batch=0 - ) + pruned_toggle_on = _prune(session, {PRUNE_SOURCE}, seen, images=True, prune_batch=0) finally: session.close() # Toggle ON: the deleted image file prunes its row AND its copy (the @@ -1240,8 +1288,82 @@ def test_prune_guards_unaffected_with_light_rows( assert not gone_copy.exists() assert kept_copy.exists() assert sorted( - path - for (path,) in db.execute( - select(Document.path).where(Document.source == PRUNE_SOURCE) - ) + path for (path,) in db.execute(select(Document.path).where(Document.source == PRUNE_SOURCE)) ) == ["deck.pdf", "deck.pdf#page=1#img=0.png", "notes/kept.md"] + + +def test_both_prune_guards_cost_the_same_statement_budget( + db: Session, tmp_path: Path, statement_capture: list[str] +) -> None: + """The guards add NO hidden read — proved by statement count, both ways. + + :func:`test_prune_guards_unaffected_with_light_rows` above proves the + guards DECIDE correctly off the six light columns. This pins the other + half of the same contract, the half that is invisible in behaviour: a + guard reaching for a deferred attribute would still decide correctly and + only show up as ONE MORE SELECT PER ROW. So the assertion here is a + statement count, not an outcome: + + * the SAME projection runs under both toggles — one candidate walk + apiece, naming the same six columns (images OFF only skips rows + through ``is_image``, a column the walk already reads); + * neither walk projects ``documents.content``; + * deleting an image row + copying its file costs no extra document + SELECT (the ``image_path`` the copy cleanup uses is in the projection). + + Total: exactly TWO document SELECTs for the two syncs, whatever the + guards do. + """ + image_dir = tmp_path / "image_dir" + image_dir.mkdir() + gone_copy = image_dir / "gone.png" + gone_copy.write_bytes(b"\x89PNG\r\n\x1a\n gone") + + def add(path: str, *, is_image: bool, copy: Path | None = None) -> None: + db.add( + Document( + source=PRUNE_SOURCE, + path=path, + full_path=f"/srv/brain/{PRUNE_SOURCE}/{path}", + title=f"Budget probe {path}", + content=PRUNE_CONTENT, + content_hash=f"{abs(hash(path)):064x}"[:64], + is_image=is_image, + image_path=str(copy) if copy else None, + ) + ) + + add("notes/kept.md", is_image=False) + add("notes/gone.md", is_image=False) + add("img/gone.png", is_image=True, copy=gone_copy) + add("deck.pdf#page=1#img=0.png", is_image=True) + db.commit() + + seen = {(PRUNE_SOURCE, "notes/kept.md")} + projections: list[frozenset[str]] = [] + for toggle in (False, True): + since = len(statement_capture) + session = SessionLocal() + try: + _prune(session, {PRUNE_SOURCE}, seen, images=toggle, prune_batch=0) + finally: + session.close() + # Only what THIS sync emitted — the capture is cumulative. + walks = [ + s for s in _doc_selects(statement_capture[since:]) if "WHERE documents.source" in s + ] + assert len(walks) == 1, ( + f"the images={toggle} prune ran {len(walks)} candidate walks; " + "one per sync is the budget (a second is a per-row lazy load)" + ) + projections.append(_projected_columns(walks[0])) + + assert projections[0] == projections[1], ( + "the two toggles read DIFFERENT columns — a guard grew a deferred " + f"read:\n off: {sorted(projections[0])}\n on: {sorted(projections[1])}" + ) + assert "documents.content" not in projections[0] + assert len(_doc_selects(statement_capture)) == 2, ( + "two syncs must cost exactly two document SELECTs in total, guards " + "included — any extra is a per-row read the light row must cover" + ) diff --git a/tests/unit/test_importer_defer_content.py b/tests/unit/test_importer_defer_content.py new file mode 100644 index 0000000..e64b6d6 --- /dev/null +++ b/tests/unit/test_importer_defer_content.py @@ -0,0 +1,258 @@ +"""Phase 185, task 02 — the ONE fast-path row statement (unit pins). + +``app/rag/importer.py::_doc_row_stmt`` is the single place the importer's +unchanged fast path (phase 147) loads a ``Document`` row, and it defers +exactly ONE column: ``Document.content``. That shape is load-bearing twice +over — it is what keeps a full re-sync's memory bound to row METADATA +instead of to the corpus (the 2026-10-15 incident: 17,407 rows × avg +15,217 chars = 252.6 MB of markdown hydrated for a run that re-indexed +217 files), AND it is what must NOT grow into a lazy-load landmine six +phases from now. + +Pinned here (the behaviour itself — the captured SQL, the one-SELECT-per-file +count and the backfill's single lazy read — lives in +``tests/integration/test_import_fast_path_columns.py``): + +* the statement defers ``content`` and NOTHING ELSE: its projection names + every other mapped column of ``Document``, so a column a later phase adds + keeps today's whole-row semantics (LOCKED A1 — no ``load_only`` allowlist, + which would silently defer every future column and resurface as an + unexplained N+1); +* the statement is the ``(source, path)`` lookup the fast path has always + used — the deferral changed the projection, never the predicate; +* all FOUR fast-path row loads call the helper (``_index_file``, + ``_index_pdf_file``, ``_index_one_embedded_image``, + ``_index_image_file``) and none of them builds its own + ``select(Document)`` — the four sites cannot drift; +* the importer contains no ``raiseload``/``noload`` (LOCKED A2): the + phase-118 summary-NULL backfill genuinely reads ``doc.content`` and must + pay one SELECT, not crash; +* reading the deferred column through a real session still yields the stored + text (the honest "plain ``defer`` keeps today's behaviour" claim). +""" +from __future__ import annotations + +import ast +import inspect +import re +import uuid +from collections.abc import Iterator + +import pytest +from sqlalchemy import inspect as sa_inspect +from sqlalchemy import text +from sqlalchemy.dialects import postgresql +from sqlalchemy.orm import Session + +from app.models import Document +from app.rag import importer +from app.rag.importer import _doc_row_stmt + +#: The four loads that share the phase-147 unchanged fast path (phase 185 +#: D1) — the ONLY call sites of :func:`_doc_row_stmt`. +FAST_PATH_FUNCTIONS = ( + "_index_file", + "_index_pdf_file", + "_index_one_embedded_image", + "_index_image_file", +) + +#: The helper call as it appears at an importer call site (``session.scalar( +#: _doc_row_stmt(source, rel))``) — counted, not trusted. +_HELPER_CALL = "_doc_row_stmt(" + +#: ``documents.`` inside a compiled projection (everything the +#: statement SELECTs before ``FROM``). The loader options are invisible in +#: the ORM statement's own column list — only the COMPILED projection shows +#: what actually travels to the database, which is what this phase is about. +_PROJECTION_COLUMN_RE = re.compile(r"documents\.([a-z_]+)") + + +def _projected_columns() -> set[str]: + """The ``documents`` columns the fast-path statement actually SELECTs.""" + compiled = str(_doc_row_stmt("s", "p").compile(dialect=postgresql.dialect())) + projection = compiled.split("\nFROM")[0].split(" FROM ")[0] + return set(_PROJECTION_COLUMN_RE.findall(projection)) + + +def test_helper_defers_content_and_nothing_else() -> None: + """``content`` is the ONLY column missing from the fast-path projection. + + The compiled projection is compared against the mapper's FULL column set, + so a future ``Document`` column must appear there — if someone swaps the + ``defer`` for a ``load_only`` allowlist, the new column goes missing and + this test fails (which is exactly the drift LOCKED A1 forbids). + """ + mapped = {c.name for c in sa_inspect(Document).columns} + projected = _projected_columns() + + assert "content" in mapped, "Document.content vanished — this pin needs a new shape" + assert projected, "the fast-path statement projects no documents columns at all" + assert "content" not in projected, ( + "the fast-path row statement no longer defers Document.content — a " + "full walk would hydrate the corpus markdown again (252.6 MB on the " + "incident box)" + ) + assert projected == mapped - {"content"}, ( + "the fast-path row statement defers more (or other) columns than " + f"content: missing={sorted(mapped - projected - {'content'})} " + f"unexpected={sorted(projected - mapped)} — only Document.content " + "may be deferred (LOCKED A1)" + ) + + +def test_helper_keeps_the_fast_path_predicate() -> None: + """The predicate stays the ``(source, path)`` lookup of phase 147.""" + compiled = str(_doc_row_stmt("the-source", "the/path.md").compile()).lower() + + assert "documents.source" in compiled + assert "documents.path" in compiled + assert "where" in compiled + # The bind names the integration probe attributes statements by. + assert "source_1" in compiled and "path_1" in compiled + + +def test_every_fast_path_load_calls_the_helper() -> None: + """All four fast-path row loads go through ONE statement (phase 185 D1). + + Source-level on purpose: the deferral must be impossible to bypass, and + a fifth whole-entity load would silently undo the phase for one document + class. The two per-source post-pass walks + (``_enrich_pending_images``, ``_backfill_image_contexts``) and the prune + (``load_only`` of six light columns, phase 184) are NOT fast-path loads + and are deliberately excluded — see the module docstring of + ``tests/integration/test_import_fast_path_columns.py``. + """ + for name in FAST_PATH_FUNCTIONS: + fn = getattr(importer, name) + src = inspect.getsource(fn) + assert _HELPER_CALL in src, ( + f"{name} no longer loads its row through importer.{_HELPER_CALL} — " + "the four fast-path row loads must share the ONE deferring " + "statement so they cannot drift" + ) + assert "select(Document)" not in src, ( + f"{name} builds its own select(Document) row again — a whole-entity " + "fast-path load re-hydrates Document.content (phase 185)" + ) + + +def test_helper_has_exactly_the_four_fast_path_call_sites() -> None: + """The importer calls the helper exactly four times (one per fast path). + + A fifth ``session.scalar(_doc_row_stmt(…)`` would mean a new consumer was + wired to the fast-path statement (it is the ``(source, path)`` deferring + lookup, not a general-purpose loader), and a surviving whole-entity + ``(source, path)`` ``select(Document)`` would mean a bypass. + """ + src = inspect.getsource(importer) + calls = src.count(f"session.scalar({_HELPER_CALL}") + assert calls == len(FAST_PATH_FUNCTIONS), ( + f"importer.py calls session.scalar({_HELPER_CALL}…) {calls} times — " + f"expected exactly {len(FAST_PATH_FUNCTIONS)} (the four fast-path " + "row loads)" + ) + # No fast-path-shaped whole-entity load survives next to the helper. + assert src.count("select(Document).where(Document.source ==") == 0, ( + "a (source, path) whole-entity Document load is back — route it " + "through _doc_row_stmt or give it its own explicit projection" + ) + + +def test_no_raiseload_or_noload_anywhere_in_the_importer() -> None: + """LOCKED A2: the deferred column must QUERY, never raise or come back None. + + ``raiseload``/``noload`` would turn the phase-118 summary-NULL backfill + (``_unchanged_doc`` step 3, ``content=doc.content``) into a crash or an + empty summary — a behaviour regression on the one branch that genuinely + needs the stored markdown. The accepted cost is one SELECT for that row. + + Checked through the AST, not the text: the module's own docstrings argue + against both options by NAME, so a substring scan would fail on the + prose. Only real call/import names count here. + """ + tree = ast.parse(inspect.getsource(importer)) + called: set[str] = set() + imported: set[str] = set() + for node in ast.walk(tree): + if isinstance(node, ast.Call): + if isinstance(node.func, ast.Name): + called.add(node.func.id) + elif isinstance(node.func, ast.Attribute): + called.add(node.func.attr) + elif isinstance(node, ast.ImportFrom | ast.Import): + imported.update(alias.name for alias in node.names) + + for forbidden in ("raiseload", "noload"): + assert forbidden not in called | imported, ( + f"importer.py calls/imports {forbidden} — phase 185 LOCKED A2 " + "keeps plain defer() so the summary-NULL backfill reads the " + "stored content instead of raising" + ) + assert "defer" in called, "the fast path no longer calls defer() at all" + # The importer's ONLY load_only is _prune's six-light-column row + # (phase 184) — it must never reach the fast path, whose shape is + # "everything except content" (LOCKED A1). + prune_src = inspect.getsource(importer._prune) + assert "load_only" in inspect.getsource(importer) + assert "defer" not in prune_src, ( + "_prune picked up a defer() — its light row is an explicit " + "load_only allowlist by phase 184, not a deferral" + ) + + +@pytest.fixture() +def probe_row(db: Session) -> Iterator[tuple[str, str]]: + """One seeded row (raw SQL — this file pins a SELECT, not the importer) + removed again after the test.""" + source = f"phase185-unit-{uuid.uuid4().hex[:8]}" + rel = "notes/deferred.md" + db.execute( + text( + "INSERT INTO documents (id, source, path, full_path, title, content, " + "content_hash, file_hash, indexed_at, created_at, created_at_manual, " + "summary, is_image) VALUES (:id, :source, :path, :full_path, :title, " + ":content, :content_hash, :file_hash, now(), now(), false, :summary, " + "false)" + ), + { + "id": uuid.uuid4(), + "source": source, + "path": rel, + "full_path": f"/tmp/{source}/{rel}", + "title": "Deferred", + "content": "the stored markdown body — phase 185", + "content_hash": "ch", + "file_hash": "fh", + "summary": None, + }, + ) + db.commit() + db.expunge_all() + try: + yield source, rel + finally: + db.execute(text("DELETE FROM documents WHERE source = :s"), {"s": source}) + db.commit() + db.expunge_all() + + +def test_deferred_content_still_reads_back(db: Session, probe_row: tuple[str, str]) -> None: + """A deferred column is LAZY, not gone: the row reads back whole. + + The trade phase 185 accepted (D2): the metadata arrives with the row, and + the markdown arrives on demand for the one reader that asks for it. With + ``raiseload`` this would raise; with ``noload`` it would be ``None`` — + both would break the phase-118 backfill, and both are pinned out by + :func:`test_no_raiseload_or_noload_anywhere_in_the_importer`. + """ + source, rel = probe_row + + doc = db.scalar(_doc_row_stmt(source, rel)) + + assert doc is not None + assert doc.file_hash == "fh" + assert doc.summary is None + assert doc.content == "the stored markdown body — phase 185" + # And the second read is served from the identity map, not the database. + assert doc.content == "the stored markdown body — phase 185"