feat(rag): feed whole matched documents to the LLM — no context truncation (A7 revised)

2026-08-24 23:37:44 -04:00
parent d7a4064616
commit 1e6ae360e0
16 changed files with 923 additions and 60 deletions
@@ -0,0 +1,138 @@
+# Phase 24 — Whole-Document Context: a matched document is never truncated
+
+**Source:** `TODO.md` L3–L4 — *"Documents are truncated for some reason?
+This should never happen"* + *"When the LLM matches a chunk it should get
+the entire document placed in its context so it can see the whole thing
+before answering the question"*
+**Story:** `.agent/user_stories/whole-document-context.md` (created by
+task 03)
+**Context:** `app/rag/retriever.py::select_documents` (the one and only
+place document content is cut — the 24k budget), `app/api/chat.py::plan_turn`
+(passes the budget on both HIGH and LOW paths), `app/config.py`
+(`max_context_chars`), `app/rag/prompts.py` (the shared `[…truncated…]`
+marker — still owned by the steering section), `tests/unit/test_retriever.py`
+(pins the current cap), `tests/e2e/mock_llm.py` + `tests/e2e/test_chat_rag.py`
+(E2E patterns), `.env.example` + `README.md` (document the knob).
+
+## Verified diagnosis (2026-08-24, this conversion — not a guess)
+1. **The importer stores the whole file** — `app/rag/importer.py:199–228`
+   reads each file into `documents.content` in full (sha256 over the whole
+   content). No truncation at import time.
+2. **Chunking never touches the LLM context** — the 2000-char target /
+   1200-char hard cap only shapes `chunks` rows (retrieval + embeddings);
+   the chat prompt is built from `documents.content`.
+3. **The viewer serves raw content** — `GET /api/documents/content`
+   (`app/api/docs.py`) returns `doc.content` unchanged; no truncation there
+   either. So the owner's "truncated for some reason" is *not* a separate
+   import/viewer bug.
+4. **The one and only truncation point is `select_documents()`**
+   (`app/rag/retriever.py:252–289`): the top-2 documents' combined text is
+   capped at `BOR_MAX_CONTEXT_CHARS` (default **24 000**) and the
+   lowest-ranked overflowing document is truncated in place with
+   `[…truncated…]`. `plan_turn()` (`app/api/chat.py`) passes the budget on
+   both the HIGH (grounded) and LOW (deflected) paths. The marker the owner
+   saw in answers comes from here.
+5. **The cap is pinned + documented** — `tests/unit/test_retriever.py`
+   (`test_combined_content_capped_with_truncation_marker`,
+   `test_single_doc_over_budget_is_truncated_to_budget`,
+   `test_under_budget_no_truncation`); the knob is in `.env.example`
+   (`BOR_MAX_CONTEXT_CHARS=24000`) and `README.md:377`.
+
+## Objective
+When hybrid retrieval matches a chunk, the LLM sees the **entire** parent
+document — the 24k context budget (and `BOR_MAX_CONTEXT_CHARS`) is removed
+from the document path, and a dedicated story E2E proves deterministically
+that a >24k document — and the *second* document of a >24k pair (the exact
+case the old budget cut) — reaches the model whole.
+
+## Owner-confirmed (2026-08-24, roadmap D1–D5)
+1. **D1 — no cap at all (revises LOCKED A7):** `select_documents` returns
+   the full top-N document texts, always. The `max_context_chars` setting
+   and `BOR_MAX_CONTEXT_CHARS` env var are removed. If a future KB ever
+   makes the prompt too large for the model, the existing `LLMError` → SSE
+   `error` path surfaces it loudly — no silent partial context. The
+   emergency-valve variant (raised cap + warning log) was **explicitly
+   rejected**.
+2. **D2 — `top_n_docs = 2` unchanged** (the TODO is about truncation, not
+   about how many documents).
+3. **D3 — no viewer/import changes** — both already serve full content
+   (verified diagnosis above).
+4. **D4 — E2E evidence via a deterministic mock tail-echo** (repo pattern,
+   cf. the phase-15 tuning-note echo); the big documents are seeded
+   directly in the DB inside the E2E test — `tests/fixtures/docs/` must
+   not grow, because other suites pin `summary.added == 8`.
+5. **D5 — no `query_log` schema change** (no new columns, no migration).
+
+## Dependencies
+- `03_story_chat_rag` (complete) — the RAG turn + `plan_turn` this phase
+  modifies.
+- `09_story_retrieval_quality` (complete) — hybrid retrieval +
+  `select_documents` (A7) whose cap this phase revises.
+- `15_steering_notes` (complete) — still owns `[…truncated…]` +
+  `BOR_STEERING_MAX_CHARS` (shared marker; unchanged).
+
+## Tasks
+1. `01_remove_context_cap.md` — remove the 24k budget from
+   `select_documents`, `plan_turn`, `config`, `.env.example`, `README`;
+   rewrite the unit tests to pin *no* truncation.
+2. `02_whole_doc_e2e_suite.md` — mock tail-echo trigger +
+   `tests/e2e/test_whole_document_context.py` (whole >24k doc, whole second
+   doc of a >24k pair, small-doc regression).
+3. `03_story_docs_plan_commit.md` — story file, PLAN.md A7/§6/§12 revision
+   (owner permission 2026-08-24), full validation, one `--no-gpg-sign`
+   commit, phase move.
+
+## Locked decisions
+- **A7 revision (owner permission 2026-08-24, D1):** A7's context clause
+  becomes *"feed the **full text of top-N=2 documents** (deduped)"* — the
+  "capped at 24k chars" clause is **removed**; matched parent documents are
+  **never truncated**. The revision note lands in PLAN.md via task 03
+  (phase-17/19 precedent — recorded, not silently deviated).
+- **Steering unchanged** — `BOR_STEERING_MAX_CHARS` (8 000) still caps the
+  `<tuning>` section with the same `[…truncated…]` marker (phase-15
+  behavior byte-identical).
+- **A16 honored** — dedicated Playwright story suite run in isolation;
+  unit + integration green; `app/` coverage >90%.
+
+## Testing & Quality
+- **Unit:** `tests/unit/test_retriever.py` — `select_documents` returns
+  byte-identical full content well past the old 24k budget, no marker;
+  ranking / dedup / n-cap tests unchanged.
+- **Integration:** `uv run pytest tests/integration` green — existing
+  `test_chat_api.py` prompt tests exercise `plan_turn` through the new
+  signature (no integration test pins the cap — verified).
+- **Coverage:** >90% on `app/` held
+  (`uv run pytest --cov=app --cov-report=term-missing`).
+- **E2E (new, isolated):** `tests/e2e/test_whole_document_context.py` —
+  the tail sentinel of a 30k-char document (and of the *second* document of
+  a >24k pair) appears in the rendered answer; `[…truncated…]` never
+  appears; the small-document grounded path is unchanged.
+- **Lint/types:** `uv run ruff check . && uv run pyright` clean.
+
+## Completion Criteria
+- [ ] `select_documents` has no budget parameter and never truncates;
+      `TRUNCATION_MARKER` remains for the steering section only.
+- [ ] `BOR_MAX_CONTEXT_CHARS` gone from `app/config.py`, `.env.example`,
+      and `README.md`.
+- [ ] `uv run pytest` green; `uv run pytest --cov=app
+      --cov-report=term-missing` ≥ today's number.
+- [ ] `uv run pytest tests/e2e/test_whole_document_context.py -v --no-cov`
+      green in isolation.
+- [ ] `uv run ruff check . && uv run pyright` clean.
+- [ ] `.agent/user_stories/whole-document-context.md` exists; PLAN.md
+      carries the A7 revision + §6 bullet + §12 row 24 (owner permission
+      2026-08-24).
+- [ ] One `--no-gpg-sign` commit (below);
+      `.agent/phases/todo/24_whole_document_context/` moved to
+      `.agent/phases/complete/`.
+
+## Commit (task 03 — after the phase dir has moved to `complete/`)
+```bash
+git add -f .agent/phases/complete/24_whole_document_context/ .agent/user_stories/whole-document-context.md .agent/PLAN.md
+git add -A .agent/phases/todo/24_whole_document_context/ app/ README.md .env.example tests/
+git commit --no-gpg-sign -m "feat(rag): feed whole matched documents to the LLM — no context truncation (A7 revised)"
+```
+
+(The *conversion* commit — this phase dir force-added under `todo/` plus
+the cleared `TODO.md` — lands separately when the roadmap is written, per
+the phase-20–23 precedent, commit `824914c`.)