Files
brain-of-reese/tests/unit/test_importer_defer_content.py
T
ducoterra 632fb88e43
Build and Push Containers / build-and-push-app (push) Successful in 1m41s
Build and Push Containers / build-and-push-db (push) Successful in 11s
phase: 185_sync_defer_content_fast_path
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`.
2026-10-16 04:54:01 -04:00

259 lines
11 KiB
Python
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
"""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.<column>`` 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"