From 986e704eb17b50e0a51d7def09dff0c6376bebb8 Mon Sep 17 00:00:00 2001 From: ducoterra Date: Mon, 24 Aug 2026 18:53:24 -0400 Subject: [PATCH] =?UTF-8?q?chore(agent):=20add=20phase=2024=20from=20TODO.?= =?UTF-8?q?md=20=E2=80=94=20whole-document=20context=20(no=20truncation,?= =?UTF-8?q?=20A7=20revised);=20clear=20TODO.md?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../24_whole_document_context/00_phase.md | 138 ++++++++++++++++++ .../01_remove_context_cap.md | 62 ++++++++ .../02_whole_doc_e2e_suite.md | 84 +++++++++++ .../03_story_docs_plan_commit.md | 76 ++++++++++ TODO.md | 4 +- 5 files changed, 361 insertions(+), 3 deletions(-) create mode 100644 .agent/phases/todo/24_whole_document_context/00_phase.md create mode 100644 .agent/phases/todo/24_whole_document_context/01_remove_context_cap.md create mode 100644 .agent/phases/todo/24_whole_document_context/02_whole_doc_e2e_suite.md create mode 100644 .agent/phases/todo/24_whole_document_context/03_story_docs_plan_commit.md diff --git a/.agent/phases/todo/24_whole_document_context/00_phase.md b/.agent/phases/todo/24_whole_document_context/00_phase.md new file mode 100644 index 0000000..6a8eaaa --- /dev/null +++ b/.agent/phases/todo/24_whole_document_context/00_phase.md @@ -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 + `` 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`.) diff --git a/.agent/phases/todo/24_whole_document_context/01_remove_context_cap.md b/.agent/phases/todo/24_whole_document_context/01_remove_context_cap.md new file mode 100644 index 0000000..93cbd4a --- /dev/null +++ b/.agent/phases/todo/24_whole_document_context/01_remove_context_cap.md @@ -0,0 +1,62 @@ +# Task 01 — Remove the 24k document-context budget + +**Phase:** `24_whole_document_context` · **Source:** `TODO.md:3–4` — *"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) + +## Objective +`select_documents` returns the full text of the top-N parent documents, +always — the budget parameter, the `max_context_chars` setting, the env var, +and the documentation lines are removed, and the unit tests pin the new +no-truncation contract (owner-confirmed D1: no cap at all — the +emergency-valve variant was explicitly rejected). + +## Work +1. `app/rag/retriever.py` — `select_documents(chunks, n=None)`: drop the + `max_chars` parameter and the whole `remaining`/truncation loop; return + the deduped top-N documents with their `content` byte-identical. Update + the module docstring (remove the "capped at ``BOR_MAX_CONTEXT_CHARS``" + sentence; state the A7 revision — whole documents, never truncated, + owner permission 2026-08-24) and the `select_documents` docstring. + `TRUNCATION_MARKER` **stays** exported — `app/rag/prompts.py` + (steering section, phase 15) still imports it. +2. `app/api/chat.py` — `plan_turn`: both `select_documents(…)` call sites + (HIGH and LOW paths) drop the `max_chars=settings.max_context_chars` + argument. No other turn-flow change (deflection, steering, `query_log`, + SSE all unchanged). +3. `app/config.py` — delete `max_context_chars: int = 24_000` and its + comment. (Any leftover `BOR_MAX_CONTEXT_CHARS=…` in an operator's + gitignored `.env` is silently ignored via `extra="ignore"` — no + migration, no startup check needed.) +4. `.env.example` — delete the `BOR_MAX_CONTEXT_CHARS=24000` line. +5. `README.md` — delete the settings-table row + `| BOR_MAX_CONTEXT_CHARS | 24000 | cap on total document text sent to the LLM |` + (line ~377). Leave the steering-notes `[…truncated…]` mention (~line 178) + alone — that budget still exists. +6. `tests/unit/test_retriever.py` — drop the `max_chars=…` argument from + every `select_documents` call; replace the three cap tests + (`test_combined_content_capped_with_truncation_marker`, + `test_single_doc_over_budget_is_truncated_to_budget`, + `test_under_budget_no_truncation`) with + `test_content_never_truncated_even_past_old_budget`: two documents of + 20 000 + 15 000 chars (35 000 combined — past the old 24 000 cap) come + back with content **byte-identical** (assert equality against the + originals) and `TRUNCATION_MARKER` absent from both. Keep the module + docstring accurate ("ordering and dedup — no context cap, A7 revised"). + +## Testing & Quality +- Unit: `uv run pytest tests/unit/test_retriever.py -v` green — ranking, + dedup, n-cap, and the new no-truncation test. +- Integration: `uv run pytest tests/integration` green — the existing + `test_chat_api.py` prompt tests exercise `plan_turn` through the new + signature (no integration test pins the cap — verified during conversion). +- Coverage: `uv run pytest --cov=app --cov-report=term-missing` ≥ today's + number (the >90% gate). + +## Completion Criteria +- [ ] `rg "max_context" app/ tests/ .env.example README.md` → **no matches** + (`steering_max_chars` is a different setting and must remain). +- [ ] `uv run pytest tests/unit/test_retriever.py -v` green. +- [ ] `uv run pytest` green; `uv run pytest --cov=app + --cov-report=term-missing` ≥ today's number. +- [ ] No behavior change outside the context budget — deflection, steering, + viewer, and import behave byte-identically to before. diff --git a/.agent/phases/todo/24_whole_document_context/02_whole_doc_e2e_suite.md b/.agent/phases/todo/24_whole_document_context/02_whole_doc_e2e_suite.md new file mode 100644 index 0000000..6e74c43 --- /dev/null +++ b/.agent/phases/todo/24_whole_document_context/02_whole_doc_e2e_suite.md @@ -0,0 +1,84 @@ +# Task 02 — Story E2E: whole documents reach the LLM (mock tail-echo) + +**Phase:** `24_whole_document_context` · **Source:** `TODO.md:3–4` — *"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"* (this task makes the no-truncation contract provable end-to-end) +**Story:** `.agent/user_stories/whole-document-context.md` (created by task 03) + +## Objective +`tests/e2e/test_whole_document_context.py` proves, with the deterministic +mock, that a matched document **larger than the old 24k cap** — and the +**second** document of a pair whose combined size exceeds it (the exact case +the old budget cut) — reach the LLM whole. + +## Work +1. `tests/e2e/mock_llm.py` — add one user-message trigger, same pattern as + `LONG_ANSWER_TRIGGER` and the phase-15 tuning-note echo: + - `END_OF_NOTES_TRIGGER = "show the end of your notes"` — verified + 2026-08-24: no existing E2E question or fixture file contains the + phrase, so every other suite is unaffected. + - In `compose_answer`, after the `DEFLECT_MODE` check and before the + generic branch: when the trigger is present in the user message, the + answer quotes the **tail of the context** — e.g. + `f"…and the very end of my notes reads: “{_context(body)[-160:]}” (Deterministic mock answer for E2E.)"`. + Byte-stable across runs, so a sentinel placed at the *end* of a + document appears in the rendered answer **iff the whole document was + in the prompt**. (The tail includes the closing `` — + harmless for `to_contain_text` sentinel assertions.) + - Keep the mock docstring's trigger list updated. +2. `tests/e2e/test_whole_document_context.py` — new story suite. Reuse the + `page`, `app_url`, `mock_llm`, `db_ready` fixtures and the + TRUNCATE-then-seed pattern from `tests/e2e/test_chat_rag.py` / + `tests/integration/test_document_content.py`. **Seed the big documents + directly via SQLAlchemy** (a `documents` row + 2–3 `chunks` rows, each + chunk with `embedding = embed_text(chunk_text)` imported from + `tests.e2e.mock_llm` — the same deterministic bag-of-words vector the + mock computes, so the question's live mock embedding genuinely overlaps). + **Do not add fixture files** — `tests/fixtures/docs/` must stay at its + current 8 files; other suites pin `summary.added == 8`. + - Content helper: a ~30 000-char document = repeated + "gitlab install playbook" body (tokens shared with the question → + hybrid hit) whose **last line is a unique sentinel**, e.g. + `WHOLE-DOC-TAIL-GITLAB-`. + - `test_whole_document_over_old_cap_reaches_llm` — seed one 30 000-char + document (past the old 24 000 cap); ask + `"Show the end of your notes about the gitlab install playbook, please."`; + assert the rendered brain bubble contains the tail sentinel **and** + `"Deterministic mock answer for E2E"`, contains **no** + `[…truncated…]`, the document's `.source-chip` renders, and the + `query_log` row has `deflected == False`. + - `test_second_document_of_over_cap_pair_reaches_llm` — seed two + documents of ~16 000 chars each (32 000 combined — under the old + budget the lower-ranked one was truncated in place). Doc 1's first + chunk carries the question's key tokens ("gitlab install playbook"); + doc 2's first chunk carries a weaker overlap ("playbook", "notes") so + doc 1 ranks first and doc 2 is the **last block inside + ``** — i.e. the tail-echo surfaces doc 2's sentinel. Assert + doc 2's tail sentinel in the answer. If the ranking ever flips locally, + strengthen doc 1's token overlap (repeat the question phrase in its + first chunk) until two consecutive isolated runs are stable — do not + weaken the assertion. + - `test_small_document_path_unchanged` — regression: re-import the + standard fixtures via the real importer (the `test_chat_rag.py` + `_import_fixtures` pattern), ask the usual on-topic question + (`"How is my Kubernetes cluster set up?"`) → grounded answer + + `kubernetes.md` chip, no `[…truncated…]` (the under-24k path is + byte-identical to before). + - Header comment: story path + run-in-isolation command + (`uv run pytest tests/e2e/test_whole_document_context.py -v --no-cov`). + +## Testing & Quality +- E2E: `uv run pytest tests/e2e/test_whole_document_context.py -v --no-cov` + green **in isolation** (DB up: `podman compose up -d db`). +- E2E regressions: `uv run pytest tests/e2e/test_chat_rag.py + tests/e2e/test_honest_deflection.py -v --no-cov` still green (the mock + change is additive — the trigger phrase appears in no existing question). +- Coverage: the >90% `app/` gate is unaffected (tests + mock only) — re-run + `uv run pytest --cov=app --cov-report=term-missing` to prove it. +- Lint/types: `uv run ruff check . && uv run pyright` clean. + +## Completion Criteria +- [ ] `uv run pytest tests/e2e/test_whole_document_context.py -v --no-cov` + — all three tests green in isolation. +- [ ] `uv run pytest tests/e2e/test_chat_rag.py + tests/e2e/test_honest_deflection.py -v --no-cov` green. +- [ ] `uv run ruff check . && uv run pyright` clean. +- [ ] `tests/fixtures/docs/` unchanged (still 8 files). diff --git a/.agent/phases/todo/24_whole_document_context/03_story_docs_plan_commit.md b/.agent/phases/todo/24_whole_document_context/03_story_docs_plan_commit.md new file mode 100644 index 0000000..1b1932c --- /dev/null +++ b/.agent/phases/todo/24_whole_document_context/03_story_docs_plan_commit.md @@ -0,0 +1,76 @@ +# Task 03 — Story file, PLAN.md revision, validation, commit + +**Phase:** `24_whole_document_context` · **Source:** `TODO.md:3–4` — *"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 this task) + +## Objective +Record the user story and the A7 revision (owner permission 2026-08-24), +run the full validation gate, land one atomic commit, and move the phase +to `complete/`. + +## Work +1. `.agent/user_stories/whole-document-context.md` — story file in house + style (cf. `.agent/user_stories/sources-midstream.md`): + - Header: `**Phase:** 24_whole_document_context · **E2E:** + tests/e2e/test_whole_document_context.py`. + - Bug report section: the two TODO.md items (L3–L4), verbatim. + - Narrative: *Given* a question whose chunk matches a document, + *when* the turn assembles context, *then* the LLM receives the + **entire** parent document — never a `[…truncated…]`-cut version — + so the answer is grounded in the whole note. + - Owner-confirmed (2026-08-24, roadmap D1–D5) — the five decisions from + the phase overview, verbatim. + - Acceptance criteria: no budget parameter in `select_documents`; + `BOR_MAX_CONTEXT_CHARS` gone from settings/env/README; the story E2E's + three tests green in isolation; steering-note truncation (phase 15) + unchanged; unit+integration green, coverage >90%, one + `--no-gpg-sign` commit. + - Playwright Mapping Rule: the three tests of task 02, one line each. +2. `.agent/PLAN.md` revisions (phase-17/19 precedent — record the owner + permission, never deviate silently): + - §2, A7 row: replace *"feed the **full text of top-N=2 documents** + (deduped, capped at 24k chars)"* with *"feed the **full text of + top-N=2 documents** (deduped)"* and append the rationale addendum: + "A7 revised 2026-08-24 — matched documents never truncated (owner: + 'this should never happen'; emergency-valve variant rejected)". + - §2, revision-note block under the anchors table: add + **A7 revision (phase 24, owner permission 2026-08-24):** the + `[…truncated…]` cap on document context is removed — + `select_documents` always returns the full top-N texts; + `BOR_MAX_CONTEXT_CHARS` is gone. The steering section + (`BOR_STEERING_MAX_CHARS`, phase 15) keeps its budget and the shared + marker. + - §6, retrieval bullet: *"top 2 → full content, concatenated, truncated + to `BOR_MAX_CONTEXT_CHARS` (24k) with a `[…truncated…]` marker"* → + *"top 2 → full content, concatenated — **never truncated** (A7 + revised, phase 24, owner permission 2026-08-24)"*. + - §12 roadmap table: add row 24 — + `| 24 | 24_whole_document_context.md | whole-document-context.md | tests/e2e/test_whole_document_context.py |` + plus a footnote: "Row 24 added 2026-08-24 with owner permission — + A7's 24k context cap removed (documents are never truncated)". +3. Final validation (AGENTS.md §9 — all must be green before the commit): + - `uv run pytest` (unit + integration) + - `uv run pytest --cov=app --cov-report=term-missing` (>90% gate) + - `uv run pytest tests/e2e/test_whole_document_context.py -v --no-cov` + (in isolation) + - `uv run ruff check . && uv run pyright` +4. Commit + phase move (AGENTS.md §8; use the commit block in + `00_phase.md` — it assumes the move happens first): + `mv .agent/phases/todo/24_whole_document_context + .agent/phases/complete/`, then the two `git add` lines + one + `--no-gpg-sign` commit. + +## Testing & Quality +- All AGENTS.md §9 gates: unit + integration green, `app/` coverage >90%, + story E2E green in isolation, lint + types clean. + +## Completion Criteria +- [ ] `.agent/user_stories/whole-document-context.md` exists (story, + D1–D5, acceptance criteria, Playwright mapping). +- [ ] `.agent/PLAN.md` carries the A7 row + revision note (§2), the §6 + bullet, and the §12 row 24 + footnote — every one marked + "owner permission 2026-08-24". +- [ ] All validation commands green (work step 3). +- [ ] One `--no-gpg-sign` commit; + `.agent/phases/complete/24_whole_document_context/` exists and + `.agent/phases/todo/24_whole_document_context/` is gone. diff --git a/TODO.md b/TODO.md index e7f79b6..6554401 100644 --- a/TODO.md +++ b/TODO.md @@ -1,5 +1,3 @@ # TODO -All items from this file were converted to the phased-execution roadmap -on 2026-08-24 — see `.agent/phases/todo/` (phases 20–23). Each phase -file carries a `Source:` line citing the original item and line. +Phased on 2026-08-24 — items now live in `.agent/phases/todo/24_whole_document_context/` (phase 24, owner-confirmed: no context truncation, A7 revised).