docs(phases): convert TODO L3-L5 into phases 165-167 (prose wrap, no empty answer box, API payload diet)
Three owner-reported defects become executable phases; TODO.md is cleared
once the roadmap is written (Protocol B append, PLAN.md/AGENTS.md untouched).
- 165_prose_word_wrap (TODO L3): image descriptions and .txt bodies render as
a wrapping .doc-prose block instead of <pre class="doc-raw"> (page + modal via
the one renderDocument core). Code and code-ish formats keep horizontal scroll
- phase 116 locked A1 survives; extracted PDF text stays raw and the deferral
is recorded as an owner call, not an oversight.
- 166_hide_empty_answer_bubble (TODO L4): .msg .bubble:empty { display: none }.
CSS-only, so the phase-158/161 "reasoning container inserts before the bubble"
anchor and its unit pins stay untouched; the box returns at the first delta,
and the shared/restore empty-answer corners take the same path.
- 167_api_payload_diet (TODO L5): selective pure-ASGI gzip (SSE /api/chat skipped
by exact path), a depth-limited KB tree with server-side totals + truncated
nodes whose recursive updated_at comes from a new aggregate map, a
GET /api/docs/tree/node drill route, content-hash weak ETag/304 on the catalogue
reads (NOT sources_meta - bumping it would mark saved chats stale), the lazy
client drill, and a /api/* byte-budget log line plus scripts/api_payload_audit.py
whose measured numbers land in AUDIT.md.
Phase 97's "one fetch, drill client-side" contract is deliberately superseded
(locked A3); phase 116 A1, phase 158/161 anchors and A19/A20 middleware order are
explicitly preserved.
LLM-Generated: true
This commit is contained in:
@@ -0,0 +1,63 @@
|
||||
# Phase 165 — Prose word wrap: image descriptions and plain-text docs stop scrolling horizontally
|
||||
|
||||
**Source:** `TODO.md` L3 — "Need word wrap for image explanations and other long text rather than scrolling horizontally"
|
||||
**Story:** n/a (owner-directed UI defect)
|
||||
**Context:** `frontend/assets/document.js::renderDocument` (the ONE viewer core, imported by `frontend/assets/document-modal.js:73`), `frontend/assets/styles.css` (`.doc-raw` ~L5218, `.doc-md` ~L4929), phases 10 (viewer), 36 (summary panel), 116 (themed raw scrollbar, **locked A1: code never wraps**), 122 task 04 (image docs: `content` = `summary` = the vision description), 148/163 (page strip + the Text/Pages toggle).
|
||||
|
||||
## Objective
|
||||
An image document's description — and a plain-text (`.txt`) document's body — render as **wrapping prose** inside the 72rem reading column on both viewer surfaces (the `/document.html` page and the in-page modal), so a long sentence is read by scrolling vertically like every other text surface in the app. Code and code-ish content keep their horizontal scroller exactly as phase 116's locked A1 decided.
|
||||
|
||||
## Why it scrolls today (traced 2026-10-08)
|
||||
`renderDocument` has two content branches: `format ∈ {md, markdown}` → `<div class="doc-md">` (wraps, centered 72rem column), **everything else** → `<pre class="doc-raw">`, and `.doc-raw` is `white-space: pre` + `overflow-x: auto`. An image document's `format` is its extension (`png`, `webp`, …) and its `content` is the model's prose description (phase 122 task 03), so the description lands in the `pre` and each generated line becomes one unbreakable line → horizontal scrollbar. A `.txt` document is the same shape.
|
||||
|
||||
## Design (shared by all tasks — the executor reads this, not the chat)
|
||||
|
||||
**D1 — the prose set is one named constant (task 01, LOCKED A1).** `frontend/assets/document.js`:
|
||||
```js
|
||||
const PROSE_FORMATS = new Set(["txt", "text"]);
|
||||
function isProseDoc(doc) { return doc.is_image === true || PROSE_FORMATS.has(doc.format); }
|
||||
```
|
||||
`renderDocument`'s content branch becomes three-way: markdown → `.doc-md` (untouched) → prose → `<div class="doc-prose">` (NEW) → everything else → `<pre class="doc-raw">` (untouched). `is_image` wins regardless of format (an image doc is prose by construction, phase 122 A3). `doc.format` values that stay in `.doc-raw`: `py`, `yaml`, `yml`, `json`, `j2`, the quadlet family, `pdf`, and every unknown/extension-less format.
|
||||
|
||||
**D2 — `.doc-prose` is `.doc-md`'s column with prose rhythm (task 01, LOCKED A2).** `frontend/assets/styles.css`, adjacent to `.doc-raw`:
|
||||
`width: 100%; max-width: var(--chat-column); margin-inline: auto; background: var(--surface); border: 1px solid var(--line); border-radius: var(--radius); box-shadow: var(--shadow); padding: 1.5rem 1.75rem; color: var(--ink); font-family: inherit; white-space: pre-wrap; overflow-wrap: anywhere; line-height: 1.55;`
|
||||
The content stays a **text node** (`textContent`, never `innerHTML` — the XSS contract is untouched; the description is model prose with literal newlines, so `pre-wrap` keeps paragraph breaks without parsing markdown). Tokens only (the `:root`-drift test parses built-ins from this file — zero literals). No animation (nothing for `prefers-reduced-motion` to still).
|
||||
|
||||
**D3 — the code contract stays locked and gets pinned (task 02, LOCKED A3).** Phase 116's locked A1 ("keep horizontal scroll (NO line-wrap) for the code content") survives verbatim: `.doc-raw`, `.bubble pre`, `.md-table-wrap` are byte-identical, and phase 116 task 01's themed scrollbar rules (`.doc-modal .doc-raw`) keep applying to what still scrolls. The unit test asserts the exact prose set, so a future format cannot drift into prose by accident.
|
||||
|
||||
**D4 — verification (task 03).** Dedicated Playwright suite `tests/e2e/test_prose_word_wrap.py` (isolated run, deterministic mock LLM, the seeding idiom of `tests/e2e/test_image_documents.py` / `tests/e2e/test_document_viewer.py`): for a seeded image doc AND a seeded `.txt` doc with a > 200-char single line, assert in **both** surfaces (page + modal) that the prose block (a) contains the full text, (b) has `scrollWidth <= clientWidth + 1` (no horizontal overflow — the TODO's literal ask), (c) is taller than one line (proof it wrapped), and that a seeded `.py`/`yaml` doc STILL reports `scrollWidth > clientWidth` (the locked code path).
|
||||
|
||||
**Deferred, recorded, not silently dropped:** the same `pre`-scroll applies to **extracted PDF text** (`format == "pdf"` → `.doc-raw`, e.g. the IKEA-manual corpus). Wrapping it means touching `pre.doc-raw` assertions in three completed suites (`test_pdf_pages_in_sources.py`, `test_pdf_strip_additive_selfheal.py`, `test_garbage_pdf.py`) and the phase-163 strip contract — an owner call, one-line change once made (`PROSE_FORMATS.add("pdf")` + those pins). This phase deliberately does NOT change PDF rendering (no behavior change in completed phases without owner sign-off).
|
||||
|
||||
## Dependencies
|
||||
- `122_image_documents` (complete) — image docs: `content` = `summary` = vision description, `is_image` on the content payload, `imageSummaryIsContent` suppression.
|
||||
- `116_modal_scrollbar_theme` (complete) — **locked A1** (code keeps horizontal scroll) and the themed scrollbar this phase must leave working.
|
||||
- `163_pdf_strip_additive_selfheal` (complete) — the `.doc-views` wrapper + Text/Pages toggle: the prose block mounts inside the same `slot` the `.doc-raw` path used, so `[data-view="pages"]` hiding must keep working (add `.doc-prose` to the hide list — task 01, it is the additive/PDF case's text view).
|
||||
|
||||
## Tasks
|
||||
1. `01_doc_prose_block.md` — the prose-set constant + `renderDocument`'s third branch + `.doc-prose` CSS (page + modal, strip hide list) + unit pins.
|
||||
2. `02_raw_scroll_pins.md` — pin the untouched code scrollers (`.doc-raw`, `.bubble pre`, `.md-table-wrap`, phase-116 scrollbar), pin the exact prose set + the XSS/textContent rule.
|
||||
3. `03_e2e_prose_wrap.md` — `tests/e2e/test_prose_word_wrap.py` + the full gates + the docs pass + the commit.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit (extend, following the house CSS/JS source-assertion style of `tests/unit/test_image_documents.py`, `tests/unit/test_viewer_additive_toggle.py`, `tests/unit/test_document_viewer.py`): `isProseDoc` truth table (image × format, txt/text, py/yaml/json/j2/quadlet/pdf/unknown/extension-less); `renderDocument` output shape for each branch (`className`, `textContent`, no `innerHTML`); the `.doc-prose` rule contains `pre-wrap` + `overflow-wrap: anywhere` + `var(--chat-column)` and zero color literals; the `.doc-raw` rule still contains `white-space: pre` + `overflow-x: auto`.
|
||||
- E2E: `tests/e2e/test_prose_word_wrap.py` green **in isolation** (`uv run pytest tests/e2e/test_prose_word_wrap.py -v --no-cov`); `tests/e2e/test_image_documents.py`, `test_document_viewer.py`, `test_pdf_pages_in_sources.py`, `test_pdf_strip_additive_selfheal.py`, `test_edit_summaries.py`, `test_summary_in_viewer.py` stay green (PDF/raw paths unchanged).
|
||||
- Coverage: **>90%** on `app/` (this phase is frontend+CSS only; the gate must not move).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] An image document's description renders in `.doc-prose`, wraps at the reading column, and shows the FULL description (no clamp, no horizontal scrollbar) on both `/document.html` and the modal.
|
||||
- [ ] A `.txt` document wraps the same way; `py`/`yaml`/`json`/`j2`/quadlet/`pdf`/unknown formats still render in `.doc-raw` with the horizontal scroller and the themed modal scrollbar.
|
||||
- [ ] No `innerHTML` for document content anywhere in the new path; contrast pairs are the existing AA token pairs.
|
||||
- [ ] `uv run pytest` green; coverage >90%; `uv run ruff check . && uv run pyright` clean; the phase E2E green in isolation.
|
||||
- [ ] One `--no-gpg-sign` Conventional Commit; the phase dir moves to `complete/`.
|
||||
|
||||
## Locked decisions
|
||||
- **A1 — the prose set is exactly `{is_image: true} ∪ {format ∈ txt, text}`** (owner-confirmed 2026-10-07: "image descriptions + plain-text prose"); code and code-ish formats keep scrolling.
|
||||
- **A2 — `.doc-prose` is a text-node `pre-wrap` block in the 72rem column** (no markdown parsing of a description — a description is prose, not markup; the phase-122 rule).
|
||||
- **A3 — phase 116's locked A1 is NOT superseded**: code content keeps `white-space: pre` + `overflow-x: auto`.
|
||||
- **A4 — extracted PDF text stays in `.doc-raw` for now** (see "Deferred" above) — an owner call, not an executor choice.
|
||||
|
||||
## Commit
|
||||
```bash
|
||||
git add frontend/ tests/ .agents/phases/ && git commit --no-gpg-sign -m "fix(viewer): wrap image descriptions and plain-text docs as prose instead of scrolling horizontally"
|
||||
```
|
||||
@@ -0,0 +1,38 @@
|
||||
# Task 01 — The prose content block: `PROSE_FORMATS`, `renderDocument`'s third branch, `.doc-prose` (phase 165 D1/D2)
|
||||
|
||||
**Phase:** `165_prose_word_wrap` · **Source:** `TODO.md:3` — "Need word wrap for image explanations and other long text rather than scrolling horizontally"
|
||||
**Story:** n/a
|
||||
|
||||
## Objective
|
||||
The viewer renders an image document's description and a `.txt` document's body as a wrapping prose block (`.doc-prose`) instead of `<pre class="doc-raw">`, on both the `/document.html` page and the in-page modal, with the markdown and raw branches untouched.
|
||||
|
||||
## Work
|
||||
1. `frontend/assets/document.js` (module scope, above `renderDocument`) — the ONE prose rule, phase 165 D1:
|
||||
```js
|
||||
const PROSE_FORMATS = new Set(["txt", "text"]);
|
||||
function isProseDoc(doc) {
|
||||
return doc.is_image === true || PROSE_FORMATS.has(doc.format);
|
||||
}
|
||||
```
|
||||
Docstring: the image arm is unconditional (phase 122 A3 — an image doc's `content` IS the vision description, prose by construction, whatever its extension); `format` comes from the server's `doc_format()` (lowercased suffix). Name the TODO defect in the comment ("the description used to land in the `pre` and each generated line became one unbreakable line").
|
||||
2. `frontend/assets/document.js` — `renderDocument`'s content branch (today: `if (doc.format === "md" || doc.format === "markdown") { … .doc-md … } else { … .doc-raw … }`): make it three-way and put the prose arm **between** them —
|
||||
- markdown → the existing `.doc-md` div, byte-identical;
|
||||
- `isProseDoc(doc)` → `<div class="doc-prose">` with `textContent = doc.content` (a **text node**: the XSS contract is unchanged; `pre-wrap` carries the newlines, no markdown parsing — locked A2);
|
||||
- else → the existing `<pre class="doc-raw">`, byte-identical.
|
||||
The prose block appends into the SAME `slot` variable the raw branch uses, so the phase-122 image block (`docImageBlock`) stays FIRST, the phase-36 summary panel (when not suppressed by `imageSummaryIsContent`) stays between them, and the phase-163 additive strip (`docPagesStrip`) still mounts last inside `.doc-views`.
|
||||
3. `frontend/assets/styles.css` — new `.doc-prose` rule immediately after `.doc-raw` (~L5229), phase 165 D2, exact property set:
|
||||
`width: 100%; max-width: var(--chat-column); margin-inline: auto; background: var(--surface); border: 1px solid var(--line); border-radius: var(--radius); box-shadow: var(--shadow); padding: 1.5rem 1.75rem; color: var(--ink); white-space: pre-wrap; overflow-wrap: anywhere; line-height: 1.55;`
|
||||
(`--ink` on `--surface` is the documented 14.5:1 pair; **zero color literals** — the `:root`-drift unit test parses built-ins from this file.) Comment block cites: TODO L3, the `.doc-md` column precedent, `.doc-raw`'s `pre` as the defect, locked A1/A2, "static content — no animation".
|
||||
4. `frontend/assets/styles.css:5361-5364` — add `.doc-views[data-view="pages"] .doc-prose` to the existing hide list (the four-selector rule that hides the text view behind the page strip). Defensive today (image/`txt` docs are never page-qualifying), load-bearing if the owner later adds `pdf` to `PROSE_FORMATS` — say so in the comment.
|
||||
5. `tests/unit/test_image_documents.py` (extend; reuse its `renderDocument` harness) — the image-doc content branch now yields `className === "doc-prose"`, `textContent === description`, and NOT a `doc-raw` sibling; the summary-suppression case (`summary === content`) is unchanged.
|
||||
6. `tests/unit/test_document_viewer.py` (extend) — the format table: `txt` / `text` → `doc-prose`; `md`/`markdown` → `doc-md` (innerHTML through `renderMarkdown`, unchanged); `py`, `yaml`, `json`, `j2`, `conf`, `pdf`, `""`, an unknown suffix → `doc-raw` (proves locked A1/A3 unchanged); the never-`innerHTML` pin for the prose arm (`assert "innerHTML" not in prose_branch`).
|
||||
7. `tests/unit/test_viewer_additive_toggle.py` (extend) — the `[data-view="pages"]` hide-list test enumerates `.doc-summary`, `.doc-md`, `.doc-raw`, `.doc-image`: add `.doc-prose` to the expected selectors and assert the rule stays ONE grouped declaration.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: items 5–7 + the CSS pins (`.doc-prose` contains `pre-wrap`, `overflow-wrap: anywhere`, `var(--chat-column)`; `.doc-raw` still contains `white-space: pre` and `overflow-x: auto`).
|
||||
- Coverage: **>90%** on `app/` unchanged (no `app/` change in this task).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `isProseDoc` is the only place the prose rule lives (grep-verified: no second `txt` literal in the content branch).
|
||||
- [ ] Image docs and `.txt` docs render `.doc-prose` on page + modal (both call `renderDocument`); every other format's markup is byte-identical to pre-task.
|
||||
- [ ] No `innerHTML` on document content in the new path; full `uv run pytest` green.
|
||||
@@ -0,0 +1,26 @@
|
||||
# Task 02 — Pin the untouched scrollers: code keeps horizontal scroll (phase 165 D3, LOCKED A1/A3)
|
||||
|
||||
**Phase:** `165_prose_word_wrap` · **Source:** `TODO.md:3` — the "other long text" half of the item, bounded by phase 116's locked A1 ("keep horizontal scroll (NO line-wrap) for the code content … wrapping breaks code readability")
|
||||
**Story:** n/a
|
||||
|
||||
## Objective
|
||||
The word-wrap fix is provably scoped: every surface that must keep scrolling horizontally still declares it, the prose set is pinned as data, and the chat/table scrollers are asserted byte-identical — so "we fixed wrapping" can never later mean "we wrapped code" or "we wrapped wide tables" by accident.
|
||||
|
||||
## Work
|
||||
1. New `tests/unit/test_prose_word_wrap.py` (the house CSS/JS source-assertion style — read `tests/unit/test_markdown_tables.py` and `tests/unit/test_date_editor.py` first; they parse `frontend/assets/styles.css` and the JS files as text):
|
||||
- **the prose set is exactly `{txt, text}` + `is_image`**: assert the `PROSE_FORMATS` literal contains exactly `"txt"`, `"text"` (a set-membership assertion over the source line, so adding a format is a deliberate, test-editing act); assert `isProseDoc` reads `doc.is_image` first;
|
||||
- **code scrollers present and unchanged**: `.doc-raw {` block still contains `white-space: pre` and `overflow-x: auto`; `.bubble pre` still contains `overflow-x: auto` (no `white-space` added); `.md-table-wrap { overflow-x: auto; }` and `.md-table { … min-width: max-content … }` still match the phase-44 declaration;
|
||||
- **phase 116 A1 stays locked**: the four `.doc-modal .doc-raw` scrollbar rules (`scrollbar-width`, `scrollbar-color`, `::-webkit-scrollbar` ×3) are all still present and still scoped to `.doc-modal`;
|
||||
- **`.doc-prose` never reaches for the raw scroller**: the `.doc-prose` block contains no `overflow-x` and no `white-space: pre;` (only `pre-wrap`).
|
||||
2. `tests/unit/test_prose_word_wrap.py` — the contrast/AA pin for the new block: `.doc-prose`'s `color` is `var(--ink)` and its background `var(--surface)` (the documented 14.5:1 pair), and the block contains no hex/`rgb(` literal (the phase-92 zero-literal invariant).
|
||||
3. Repo hygiene pass: `grep -rn "doc-raw" frontend/` — confirm the only remaining writers of `doc-raw` are the raw branch of `renderDocument` and the phase-116 CSS; if `frontend/doc-edit.html` / `doc-edit.js` / `shared.*` render document content, verify they are NOT a second content renderer that needs the same rule (they are not expected to be — record the finding in the test's docstring).
|
||||
4. ASSUMPTION: PDF-extracted text (`format == "pdf"`) is deliberately OUT of the prose set — it stays in `.doc-raw` because three completed suites pin `pre.doc-raw` for PDFs and the phase-163 strip contract; the deferral is recorded in `00_phase.md` ("Deferred") and in this test's docstring, so it is an owner decision on record, not an oversight.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: the new file (all assertions are static-source assertions — no app dependency); run the phase-116/44/163-owning unit files to prove nothing moved: `uv run pytest tests/unit -k "additive_toggle or markdown_tables or date_editor or image_documents or document_viewer" -v`.
|
||||
- Coverage: **>90%** on `app/` unchanged.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `uv run pytest tests/unit/test_prose_word_wrap.py -v` green.
|
||||
- [ ] The pin set proves: code/raw/tables keep `overflow-x: auto`; `.doc-prose` has none; the prose set is `{txt, text}` + image.
|
||||
- [ ] Full `uv run pytest` green; ruff + pyright clean; no `app/` change.
|
||||
@@ -0,0 +1,34 @@
|
||||
# Task 03 — The wrapping proof: `tests/e2e/test_prose_word_wrap.py`, migrating the two suites that pinned `pre.doc-raw` for prose, final gates (phase 165 D4)
|
||||
|
||||
**Phase:** `165_prose_word_wrap` · **Source:** `TODO.md:3` — "Need word wrap for image explanations and other long text rather than scrolling horizontally"
|
||||
**Story:** n/a
|
||||
|
||||
## Objective
|
||||
A dedicated, isolated Playwright suite proves the TODO's literal claim (no horizontal scrollbar, full text visible, both surfaces, both prose classes) and the two completed suites whose fixtures ARE prose are migrated to the new contract — while the code path that keeps scrolling is still proven to scroll.
|
||||
|
||||
## Work
|
||||
1. New `tests/e2e/test_prose_word_wrap.py` — module-scoped app fixtures + `_reset_db`/seed idiom copied from `tests/e2e/test_document_viewer.py` (read it first; `_seed_long_line_doc` shows the direct-seed pattern) and the image seeding of `tests/e2e/test_image_documents.py`:
|
||||
- **seed**: one image doc (`is_image`, `content` = `summary` = a ~900-char single-paragraph vision description with `image_path` + a served `image_url`) and one `notes/prose-long.txt` doc whose content is a single ~900-char line plus a second paragraph (proves BOTH intra-line wrap and paragraph breaks); plus one `notes/code-long.py` doc with one ~900-char line (the negative control).
|
||||
- **assert, per prose doc, on BOTH surfaces** (`/document.html?source=…&path=…` and the same-page modal via the Sources row / chip click):
|
||||
- `#doc-content .doc-prose` (and `#doc-modal-content .doc-prose`) count == 1 and `to_have_text(<full description>)` — nothing clamped, nothing truncated;
|
||||
- `el.scrollWidth <= el.clientWidth + 1` — **the TODO's literal defect: no horizontal overflow**;
|
||||
- `el.getBoundingClientRect().height > 2.2 × line-height` — the text really wrapped;
|
||||
- the computed `white-space === "pre-wrap"` and the block's max-width resolves to the 72rem column (never wider than the container);
|
||||
- the image doc: the `<img class="doc-image-img">` still renders ABOVE the prose block (phase 122 ordering) and the summary panel stays suppressed (`summary === content`).
|
||||
- **negative control**: `notes/code-long.py` renders `pre.doc-raw` with `scrollWidth > clientWidth` (phase 116 A1 alive).
|
||||
- `--no-cov` isolation run per AGENTS.md rule 9.
|
||||
2. `tests/e2e/test_document_viewer.py` — MIGRATE the phase-116 scrollbar proof: `_seed_long_line_doc()` seeds `notes/long-line.txt`, which is now prose. Change the seed to `notes/long-line.json` (same long line, a code-ish format), keep every assertion (`pre.doc-raw`, real `scrollWidth > clientWidth`, the themed `scrollbar-width`/`scrollbar-color` reading, the screenshots) and update the docstring + the file's outline lines 27/42 so they say the raw block is a CODE doc and that prose wraps (cross-reference phase 165). Do NOT weaken the overflow assertion.
|
||||
3. `tests/e2e/test_image_documents.py:390,410` — MIGRATE the viewer assertion: the image doc's description now lives in `.doc-prose` (keep `to_have_text(seeded["description"])`), and the docstring line that says "the plain-content path (`pre.doc-raw`)" becomes the prose path; the rest of the suite (thumbnail, alt, chips, figure) is untouched.
|
||||
4. `tests/unit/test_image_documents.py:1042` — update the comment/assert pair that names "the existing plain-content path — doc-raw via textContent" to the prose path (task 01 already added the shape assertion; here just remove the stale claim).
|
||||
5. Docs pass + gates: `README.md` — the document-viewer section gains one sentence (prose formats wrap, code keeps the horizontal scroller; the phase-116 themed scrollbar applies to what still scrolls). `AGENTS.md` — verify the §7 UI list needs no change (the 72rem reading column already covers this). Then the full gate set: `uv run pytest`, `uv run pytest --cov=app --cov-report=term-missing` (> 90%), `uv run ruff check . && uv run pyright`, the phase E2E in isolation, plus the regression set that owns the surfaces touched: `tests/e2e/test_document_viewer.py`, `tests/e2e/test_image_documents.py`, `tests/e2e/test_pdf_pages_in_sources.py`, `tests/e2e/test_pdf_strip_additive_selfheal.py`, `tests/e2e/test_edit_summaries.py`, `tests/e2e/test_summary_in_viewer.py`, `tests/e2e/test_chat_answer_clusters.py` (each in isolation).
|
||||
6. Move `.agents/phases/todo/165_prose_word_wrap/` → `.agents/phases/complete/` and commit with the `00_phase.md` message (AGENTS.md rule 8, `--no-gpg-sign`).
|
||||
|
||||
## Testing & Quality
|
||||
- E2E: the new suite green in isolation + the seven regression suites green in isolation (the migration in items 2–3 must keep each suite's ORIGINAL claim true, just on the right element).
|
||||
- Coverage: **>90%** on `app/` (no `app/` change; the gate must not move).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `uv run pytest tests/e2e/test_prose_word_wrap.py -v --no-cov` green — image description and `.txt` body wrap with zero horizontal overflow on page + modal, full text present.
|
||||
- [ ] The `code-long.py` control still overflows horizontally (locked A1 pinned end-to-end, not just in CSS).
|
||||
- [ ] `test_document_viewer.py` and `test_image_documents.py` pass with their phase-116/122 claims intact on the new elements — no assertion deleted, only re-pointed.
|
||||
- [ ] `uv run pytest` green, coverage > 90%, ruff + pyright clean, one atomic `--no-gpg-sign` commit, phase dir in `complete/`.
|
||||
@@ -0,0 +1,57 @@
|
||||
# Phase 166 — No empty answer box before the first token
|
||||
|
||||
**Source:** `TODO.md` L4 — "There's an empty box where the LLM response will go that shows up before the LLM responds. It looks ugly." (the owner attached `image.png`: mid-turn the chat shows `Thinking` ▸, `Tool calls (2)` ▸, an expanded `Thinking` run with text, and BELOW it a bordered, padded, empty rounded box ≈ 40px tall)
|
||||
**Story:** n/a (owner-directed UI defect)
|
||||
**Context:** `frontend/assets/styles.css` (`.bubble` ~L501, `.bubble typing`, `.msg-body` flex gap ~L489), `frontend/assets/app.js` (`addMessage` L1386, `addTyping` L1433, `ensureReasoningWrap` L1610-1625, the `if (!wrap) wrap = addMessage("brain", "")` sites at ~4063 thinking / ~4164 tool / ~4269 first delta, `finalizeFailedTurn` ~3841, the restore path ~2864), `frontend/assets/shared.js:113` (the same `.msg/.msg-body/.bubble` structure), phases 06 (loading feedback), 17/117/158/161 (the per-run timeline that inserts BEFORE `.bubble`), 120/164 (failed-turn bubbles).
|
||||
|
||||
## Objective
|
||||
While a turn is in flight, nothing paints an empty answer bubble: the box appears **at the first answer token**, and every visible-feedback affordance the app already owns (the typing indicator, the reasoning container with its live status, `#send-status`, the elapsed clock) keeps working exactly as before. Same rule covers the shared page's empty persisted answer, which renders the same empty box today.
|
||||
|
||||
## Why the box exists (traced 2026-10-08)
|
||||
The first `thinking`/`tool` frame of a turn calls `addMessage("brain", "")` (`app.js:4063` / `:4164`) — an EMPTY bubble is created on purpose, because `ensureReasoningWrap` uses it as the DOM anchor (`body.insertBefore(cont, body.querySelector(".bubble"))`). `.bubble` carries `padding: 0.7rem 1rem` + `border` + `background` + `box-shadow`, so an empty one paints a box; the `.msg-body` flex gap adds the visible separation. `renderMarkdown("")` returns `""` (verified), so the restore path produces the same empty element — and the shared page (`shared.js:113`) too.
|
||||
|
||||
## Design (shared by all tasks — the executor reads this, not the chat)
|
||||
|
||||
**D1 — hide it in CSS, do not restructure the JS (task 01, LOCKED A1).** `frontend/assets/styles.css`, in the Messages block right after `.bubble p`:
|
||||
```css
|
||||
.msg .bubble:empty { display: none; }
|
||||
```
|
||||
`:empty` matches only an element with NO child nodes, which is exactly what `addMessage("brain", "")` produces (`<div class="bubble"></div>`); the first delta writes `innerHTML` and the box returns on the same frame. `display: none` also drops the bubble from the `.msg-body` flex layout, so the gap collapses (no orphan whitespace). The typing indicator is `.bubble typing` WITH three `<span>` children, so it is never `:empty` and its animation is untouched. Scoped to `.msg .bubble` — the viewer's `.doc-*` blocks and the composer are unaffected.
|
||||
|
||||
**D2 — the JS anchor stays (task 02).** Creating the empty wrap and anchoring the reasoning container before it is the phase-158/161 contract, pinned by `tests/unit/test_frontend_tool_states.py:61`, `tests/unit/test_thinking_timeline.py:475,522`, `tests/unit/test_frontend_edit_message.py:971,1072,1177`. Changing it (create-the-bubble-at-first-delta) would re-own those anchors for zero user-visible gain over D1 — so `app.js` is **not** modified by this phase; task 02's job is to prove the feedback contract holds with the box hidden.
|
||||
|
||||
**D3 — §7.4 feedback contract, verified not assumed (task 02).** With the box hidden, a live turn still shows, in order: `#typing-indicator` (role=status, `TYPING_LABEL()`, elapsed-seconds suffix from `thinkingClock`) → the open `details.reasoning` container with its live status span ("Thinking…" / "Calling <name>…" / "N steps") → the answer bubble at the first delta. Nothing else in the app depends on the empty bubble being painted: `scrollReveal` measures the `.msg` wrap (not the bubble), the phase-161 window pins measure `.reasoning-body`, the phase-18 follow-bottom logic is user-intent-gated, and `#messages` remains the single `aria-live="polite"` region (an `aria-live` announcement fires when content lands — at which point the bubble is visible again).
|
||||
|
||||
**D4 — verification (task 03).** Dedicated suite `tests/e2e/test_no_empty_box.py`: stub `POST /api/chat` with a controllable SSE stream (`thinking` → `tool` → a pause → `delta`), assert `page.locator(".msg.brain .bubble").first` is `not_to_be_visible()` (or has zero box) during the pre-delta window while `#typing-indicator` / `details.reasoning[open]` / `#send-label` ARE in their expected states, and that after the first `delta` the same locator becomes visible and holds the token. Plus a restore case (a saved chat whose last brain record has empty text renders no empty box) and the shared-page case.
|
||||
|
||||
## Dependencies
|
||||
- `161_reasoning_bubble` + `158_thinking_timeline` (complete) — the container-before-the-bubble anchor this phase keeps, and the unit pins that must stay green untouched.
|
||||
- `06_story_loading_feedback` + `109_turn_progress_loader` + `120_failed_turn_retry` (complete) — the visible-feedback states that must not regress when the box stops painting.
|
||||
- `130_chat_answer_clusters` (complete) — `.msg-meta` rows are siblings of `.bubble`, never children: hiding an empty bubble never hides a chip/action row (pin it).
|
||||
|
||||
## Tasks
|
||||
1. `01_hide_empty_bubble.md` — the CSS rule + its provenance comment + the unit pins (styles.css source assertions + the typing-indicator non-emptiness pin).
|
||||
2. `02_feedback_contract_check.md` — prove §7.4 endures: the existing state-machine pins stay green, the shared/restore empty-answer corners are covered, and nothing measures the hidden box.
|
||||
3. `03_e2e_no_empty_box.md` — `tests/e2e/test_no_empty_box.py` + the regression set + the full gates + the commit.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: new `tests/unit/test_no_empty_answer_bubble.py` (the house "read the source file, assert on the rules" style — see `tests/unit/test_thinking_scroll.py`): the `.msg .bubble:empty { display: none; }` declaration exists exactly once and sits in the Messages block; `.bubble` itself keeps `padding`/`border`/`background`/`box-shadow` (the rule hides, it does not restyle); `addTyping`'s markup still contains the three `<span>` dots (⇒ never `:empty`); `app.js` still contains the three `addMessage("brain", "")` anchors (D2 — the rule is CSS-only).
|
||||
- E2E: `tests/e2e/test_no_empty_box.py` green **in isolation**; the owning regression set green in isolation: `test_thinking_timeline.py`, `test_turn_progress_loader.py`, `test_chat_history.py`, `test_share_chat.py`, `test_save_share_ux.py`, `test_failed_turn_retry.py`, `test_chat_answer_clusters.py`, `test_chat_error_honesty.py`.
|
||||
- Coverage: **>90%** on `app/` (no `app/` change; the gate must not move).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] Mid-turn (thinking and/or tool frames, zero deltas) NO empty answer box paints — the reasoning container and the typing indicator are the only brain-side boxes.
|
||||
- [ ] The answer bubble appears with the first delta, in the same DOM position, with the source chips / actions / figures below it exactly as phase 130 arranged them.
|
||||
- [ ] A restored or shared answer whose persisted text is empty paints no empty box either; every non-empty bubble is byte-identical.
|
||||
- [ ] No `app.js` change; the phase-158/161 anchor unit pins pass untouched; `uv run pytest` green, coverage > 90%, ruff + pyright clean.
|
||||
- [ ] One `--no-gpg-sign` Conventional Commit; the phase dir moves to `complete/`.
|
||||
|
||||
## Locked decisions
|
||||
- **A1 — CSS-only suppression** (`.msg .bubble:empty { display: none }`); the JS continues to create the empty bubble as the reasoning anchor (owner-confirmed 2026-10-07: "hide until the first token", not "restructure the anchor").
|
||||
- **A2 — nothing replaces the box**: no skeleton, no shimmer — the existing loading feedback (typing indicator → reasoning container → status label) is the turn's visible state (PLAN §7.4), owner-confirmed 2026-10-07.
|
||||
- **A3 — the rule extends to the shared page and the restore path** (same `.msg .bubble` structure; an empty persisted answer is the same defect), owner-confirmed 2026-10-07.
|
||||
|
||||
## Commit
|
||||
```bash
|
||||
git add frontend/ tests/ .agents/phases/ && git commit --no-gpg-sign -m "fix(chat): stop painting an empty answer bubble before the first token"
|
||||
```
|
||||
@@ -0,0 +1,44 @@
|
||||
# Task 01 — `.msg .bubble:empty { display: none }`: the empty answer bubble stops painting (phase 166 D1)
|
||||
|
||||
**Phase:** `166_hide_empty_answer_bubble` · **Source:** `TODO.md:4` — "There's an empty box where the LLM response will go that shows up before the LLM responds. It looks ugly." (attached `image.png`: Thinking ▸ / Tool calls (2) / an expanded Thinking run, then an empty bordered box)
|
||||
**Story:** n/a
|
||||
|
||||
## Objective
|
||||
One CSS rule suppresses the empty answer bubble during a turn (and for an empty persisted answer), with the box returning the instant the first token lands — no JS change, no restyle of a non-empty bubble, no change to any loading affordance.
|
||||
|
||||
## Work
|
||||
1. `frontend/assets/styles.css` — in the `---------- Messages ----------` block, directly after `.bubble p { margin: 0.2rem 0; }` (~L509), add:
|
||||
```css
|
||||
/* Phase 166 (TODO L4): the answer bubble an in-flight turn creates is
|
||||
EMPTY by construction — app.js:4063/:4164 call addMessage("brain","")
|
||||
so ensureReasoningWrap has an anchor to insert the per-run timeline
|
||||
BEFORE. With .bubble's own padding + border + background + shadow an
|
||||
empty one paints a ~40px box, which is the "empty box before the LLM
|
||||
responds" the owner reported. :empty matches only a child-less
|
||||
element — exactly innerHTML === "" (renderMarkdown("") is "", so the
|
||||
restore and the shared page take the same path) — and the first
|
||||
delta writes content, so the box appears AT the first token.
|
||||
display:none also drops the flex item, so .msg-body's gap collapses.
|
||||
The typing indicator is never :empty (three dot <span>s) — its
|
||||
animation is untouched. Scoped to .msg .bubble: the viewer's .doc-*
|
||||
blocks and the composer never match. */
|
||||
.msg .bubble:empty { display: none; }
|
||||
```
|
||||
No new tokens, no animation (nothing for `prefers-reduced-motion` to still).
|
||||
2. `tests/unit/test_no_empty_answer_bubble.py` (new — static source-assertion style, copy the harness of `tests/unit/test_thinking_scroll.py` / `tests/unit/test_markdown_tables.py`):
|
||||
- the declaration appears **exactly once**, spelled `.msg .bubble:empty { display: none; }`, and sits after `.bubble {` and before `.bubble pre {` (provenance ordering, the `test_date_editor.py:411` `css.find(...)` idiom);
|
||||
- the `.bubble {` rule still declares `padding`, `border-radius`, `background`, `border`, `box-shadow` (the fix HIDES, it never restyles);
|
||||
- `.bubble typing` / the typing-dot rules are still present and unchanged (the indicator stays visible);
|
||||
- `frontend/assets/app.js` still contains all three `addMessage("brain", "")` call sites and `ensureReasoningWrap` still does `body.insertBefore(cont, body.querySelector(".bubble"))` (D2: the anchor survives — this is the pin that stops a future "cleanup" from moving the fix into JS and breaking the timeline);
|
||||
- `frontend/assets/markdown.js` — `renderMarkdown("")` and `renderMarkdown(" ")` both return `""` (execute the file the way the existing markdown unit tests do), which is WHY the restore/shared paths are covered by the same rule;
|
||||
- no CDN / no `url(http` introduced (the AGENTS.md rule-6 idiom these files already assert).
|
||||
3. `frontend/assets/app.js` — **no change** (explicitly). If the executor believes a JS change is needed, stop and flag it: LOCKED A1 (the reasoning-container anchor and the phase-158/161 pins are the reason this is CSS-only).
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: the new file; then `uv run pytest tests/unit -k "thinking_timeline or tool_states or edit_message or frontend" -v` — the phase-158/161/159 anchor pins pass untouched.
|
||||
- Coverage: **>90%** on `app/` unchanged (no `app/` change).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] The rule exists exactly once, scoped, with the provenance comment naming TODO L4, `image.png`, and the anchor it preserves.
|
||||
- [ ] `addTyping()`'s bubble is provably non-empty (unit-pinned) so the loading state never disappears.
|
||||
- [ ] `git diff` for this task touches `frontend/assets/styles.css` + one new unit test file only.
|
||||
@@ -0,0 +1,32 @@
|
||||
# Task 02 — §7.4 audit: the turn is never feedback-less with the box hidden (phase 166 D3)
|
||||
|
||||
**Phase:** `166_hide_empty_answer_bubble` · **Source:** `TODO.md:4` — hiding the empty box must not remove the owner's only mid-turn signal; PLAN §7.4 ("never stale": every state has a defined UI) and AGENTS.md rule 10
|
||||
**Story:** n/a
|
||||
|
||||
## Objective
|
||||
Prove — with tests, not prose — that every mid-turn state still has a visible affordance with the empty bubble suppressed, that no code path measures the hidden box, and that the restore and shared-page corners render no empty box either.
|
||||
|
||||
## Work
|
||||
1. `tests/unit/test_no_empty_answer_bubble.py` (extend) — the **feedback inventory** pins, each naming the state it guarantees:
|
||||
- `addTyping()` markup contains `role="status"`, `aria-label="${TYPING_LABEL()}"` and the three `<span></span>` dots (the pre-thinking state);
|
||||
- `thinkingClock` still writes the `Ns` `.typing-elapsed` suffix into `#typing-indicator .bubble` (the long-tool-call state) — and note in the comment that this bubble is never `:empty`, so the clock keeps painting;
|
||||
- `setReasoningStatus("Thinking…")` / `"Calling "` / the settle `"N steps"` literals are still wired at their three call sites (the thinking/tool state), and `ensureReasoningWrap` sets `cont.open = true` (the container is visible while live);
|
||||
- `#send-status` / `#send-label` state copy is untouched (the composer-side signal), and `#messages` keeps `aria-live="polite"` (`frontend/index.html:167`) — the ONE conversation live region; an announcement fires when content lands, at which point the bubble is visible again (comment this reasoning).
|
||||
2. `tests/unit/test_no_empty_answer_bubble.py` (extend) — the **"nothing measures the hidden box"** audit, executed as source assertions with the finding recorded per site:
|
||||
- `scrollReveal(wrap)` receives the `.msg` WRAP (not the bubble) — pin the call site `if (scroll) scrollReveal(wrap);`;
|
||||
- the phase-161 window pins operate on `.reasoning-body` (`isReasoningPinned` / `scrollReasoningToBottom`) — pin that neither takes a `.bubble`;
|
||||
- `.bubble` is only ever read for CONTENT (`bubble.textContent`, `bubble.innerHTML`, `bubble.querySelector(".msg-image")`, `bubble.classList`) in `app.js`/`shared.js` — grep-assert there is no `getBoundingClientRect`/`offsetHeight`/`clientHeight` on a `.bubble` selector;
|
||||
- `.msg-meta` (chips, actions, figures, `.failed-note`, `.stopped-note`) are appended to `.msg-body` — pin one append site each so a future refactor cannot move them INTO the bubble (where hiding an empty one would hide them).
|
||||
3. `tests/unit/test_no_empty_answer_bubble.py` (extend) — the **restore / shared corners**:
|
||||
- `app.js` restore renders brain bubbles through `addMessage("brain", renderMarkdown(m.text), true)` — with `m.text === ""` the bubble is child-less ⇒ `:empty` ⇒ hidden (assert the call shape, plus the `renderMarkdown("")` fact already pinned in task 01);
|
||||
- `shared.js` builds the identical `<div class="bubble">${html}</div>` structure (its line ~113), so the same `.msg .bubble:empty` scope covers the shared page — assert the rule's scope prefix `.msg ` matches both (the shared page uses the same `.msg`/`.msg-body` classes).
|
||||
4. Findings note: append a short `AUDIT NOTE:` block to the test module docstring listing the four mid-turn states and the affordance that carries each after this phase (idle/typing → typing dots + clock; thinking/tool → reasoning container + status span; first token → the bubble appears; terminal → answer/deflection/stopped/failed copy). This is the record the next agent reads instead of re-deriving it.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: the extended file + `uv run pytest tests/unit -k "frontend or reasoning or thinking or tool" -v` green; no `app/` change, coverage gate unmoved.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] Every mid-turn state has a pinned, visible affordance with the empty bubble suppressed (the four-state inventory is in the test docstring).
|
||||
- [ ] No code path measures a `.bubble` box (grep-pinned), so `display:none` cannot shift a scroll or a pin.
|
||||
- [ ] The restore and shared paths are covered by the same scope (pinned), with no `shared.js` change needed.
|
||||
- [ ] Full `uv run pytest` green.
|
||||
@@ -0,0 +1,34 @@
|
||||
# Task 03 — `tests/e2e/test_no_empty_box.py`: the mid-turn proof, a paced mock turn, and the final gates (phase 166 D4)
|
||||
|
||||
**Phase:** `166_hide_empty_answer_bubble` · **Source:** `TODO.md:4` — "There's an empty box where the LLM response will go that shows up before the LLM responds. It looks ugly."
|
||||
**Story:** n/a
|
||||
|
||||
## Objective
|
||||
An isolated Playwright suite proves the defect is gone with a race-free timeline: during the pre-delta window (thinking frames + a tool call, no answer token) the brain message paints NO answer box, the reasoning container and the typing/loading affordances DO paint, and the box appears exactly with the first delta. Plus the deterministic mock turn that makes that window long enough to assert, the regression set, and the phase gates.
|
||||
|
||||
## Work
|
||||
1. `tests/e2e/mock_llm.py` — a new dedicated paced trigger (the `TURN_PROGRESS_TRIGGER` precedent, its block documents the convention): `EMPTY_BUBBLE_TRIGGER = "paint the pre-answer window"` (verify the phrase appears in NO existing E2E question and NO fixture file — the phase-109 suite's disjointness check, grep before committing), streaming only, with baked-in delays so the windows are ≥ 1 s:
|
||||
- request 1 (tools offered, no tool results): `TURN_*`-style local constants (defined next to the trigger, never inline magic): a ~1.2 s pre-delay, then 3 `reasoning_content` chunks at ~0.4 s (≈ 1.2 s of thinking with **no** `delta.content`), then one synthetic `ls` tool call (`call_0`, `finish_reason: "tool_calls"`) — i.e. request 1 NEVER emits a content delta;
|
||||
- request 2 (a `tool`-role result present): a ~2.0 s gap, then 3 short `delta.content` chunks (0.05 s apart) + `finish_reason: "stop"`;
|
||||
- the module docstring's trigger inventory gains the entry; every other suite's classification is untouched (stateless discrimination from the messages, like the phase-109 flow).
|
||||
2. New `tests/e2e/test_no_empty_box.py` (module-scoped `app_server`/`app_url` + `_reset_db(mock_llm, seed=True)` + `login()` copied from `tests/e2e/test_reasoning_bubble.py`, which is the closest live-stream suite — read its `_submit`/`_wait_answer` helpers first):
|
||||
- **the defect window**: submit `EMPTY_BUBBLE_TRIGGER` question; while the thinking/tool frames stream (`expect(page.locator("details.reasoning")).to_be_visible()` first, as the sync point) assert on `.msg.brain .bubble`: `be_not_visible()` AND a zero-height box (`evaluate("el => el.getBoundingClientRect().height") === 0`) — the box does not paint; assert `details.reasoning[open]` is visible, its status span carries the live literal (`Thinking…` / `Calling `), and `#send-label` reads the in-flight state;
|
||||
- **the settle**: after `done`, the SAME locator is visible, `height > 0`, and contains the answer text; the container folds (`details.reasoning:not([open])`), the chips/actions rows are present below the bubble (phase 130 ordering survives);
|
||||
- **no-thinking control**: a plain answer question (no thinking, no tools) shows `#typing-indicator` before the settle (the typing bubble has dots ⇒ visible) and no empty box at any point — assert `#typing-indicator .bubble` height > 0 while in flight;
|
||||
- **restore**: save the answered chat, reload `/?chat=<id>` — the answered turn renders its bubble normally (non-empty), and NO `.bubble:empty` exists in the DOM (`page.locator(".msg .bubble:empty").count() === 0`);
|
||||
- **shared page**: share the chat, open `/shared/<token>` — the same zero-`:empty` assertion (LOCKED A3);
|
||||
- screenshot of the mid-turn state (the owner's `image.png` before/after record) into the suite's screenshot dir the way `test_document_viewer.py` does.
|
||||
3. Regression run (each **in isolation**, `--no-cov`): `tests/e2e/test_thinking_timeline.py`, `test_reasoning_bubble.py`, `test_turn_progress_loader.py`, `test_chat_history.py`, `test_share_chat.py`, `test_save_share_ux.py`, `test_failed_turn_retry.py`, `test_chat_answer_clusters.py`, `test_chat_error_honesty.py`, `test_markdown_tables.py` (the `.bubble pre` scroller), `test_wide_desktop_column.py` (the chat column metrics).
|
||||
4. Docs + gates: `README.md` — one sentence in the chat-turn/loading section (the answer bubble appears at the first token; mid-turn feedback is the typing indicator + the reasoning container). Then `uv run pytest`, `uv run pytest --cov=app --cov-report=term-missing` (> 90%), `uv run ruff check . && uv run pyright`, the phase E2E in isolation.
|
||||
5. Move `.agents/phases/todo/166_hide_empty_answer_bubble/` → `.agents/phases/complete/` and commit with the `00_phase.md` message (`--no-gpg-sign`).
|
||||
|
||||
## Testing & Quality
|
||||
- E2E: the new suite green in isolation (its windows are ≥ 1 s by construction — no `sleep`-style flake), plus the ten regression suites above.
|
||||
- Mock: the new trigger changes NO other suite (grep-proven disjoint phrase; the full E2E smoke `tests/e2e/test_smoke.py` green).
|
||||
- Coverage: **>90%** on `app/` (mock + tests only here; no `app/` change in the phase).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] Mid-turn: `height === 0` / not visible on `.msg.brain .bubble` while thinking + tool frames stream; the reasoning container, the status literal, and the composer state are all visible (screenshotted).
|
||||
- [ ] The box appears with the first delta and holds the answer; chips/actions/figures keep their positions below it.
|
||||
- [ ] Zero `.bubble:empty` elements after a restore and on the shared page.
|
||||
- [ ] `uv run pytest` green, coverage > 90%, ruff + pyright clean, all ten regression suites green in isolation, one atomic `--no-gpg-sign` commit, phase dir in `complete/`.
|
||||
@@ -0,0 +1,82 @@
|
||||
# Phase 167 — API payload diet: gzip, a lazily-drilled KB tree, revalidation, and a measured byte budget
|
||||
|
||||
**Source:** `TODO.md` L5 — "GET <https://brain.reeseapps.com/api/docs/tree> loads 2 MB in production which causes long loading times. Fix that any any other endpoints that could load a ton of data."
|
||||
**Story:** n/a (owner-directed performance defect)
|
||||
**Context:** `app/api/docs.py` (`GET /api/docs/tree` L919, the pure builder `build_kb_tree` L781 / `_level_children` L671 / `_folder_counts` L626 / `_subtree_max` L746, `GET /api/docs` L129), `app/schemas.py` (`KbTree` L576, `KbTreeFolder` L501, `KbTreeSource` L539, `KbTreeFile` L447 + the `_drop_image_fields` omission precedent), `app/main.py` (middleware stack: `SessionMiddleware` → routers → `configure_caching` → `SecurityHeadersMiddleware` outermost), `app/core/caching.py` (the `/api/*` pass-through contract), `app/core/logging.py` (§9 log style), `frontend/assets/sources.js` (`loadTree` L1625, `renderLevel` L1888, `treeStats` L1703, `folderExistsIn` L1667, `resetVanishedLocation` L1646), `app/models.py` (`SourcesMeta` — deliberately NOT the ETag source, see A3), phases 94/97/98/99/106/122 (the tree contract), A15 (SSE), A19 (caching), A20 (security headers).
|
||||
|
||||
## Objective
|
||||
`GET /api/docs/tree` stops shipping 2 MB: the first load carries what the RAG view actually renders (sources + one level, with server-computed totals), deeper levels load on drill, every response the client already has revalidates to a 304, and **every** non-SSE response ships gzipped (~8-12× on this JSON). Then the same lens is pointed at the rest of the API with a byte-budget log line and a measured audit, so "any other endpoints that could load a ton of data" becomes a recorded finding with fixes for the offenders instead of a guess.
|
||||
|
||||
## Why it is 2 MB (traced 2026-10-08)
|
||||
`list_kb_tree` returns the FULL recursive tree in one body (the deliberate phase-97 "the RAG view drills CLIENT-side, zero per-level requests" contract): every indexed document as a file node (`path`, `title`, `chunks`, `created_at`, `indexed_at`, + `is_image`/`image_url`/`summary` on image nodes) AND **every stored `folder_summaries` row inlined on its folder/source node**, with no compression anywhere in the stack (the app has no GZip middleware — `app/main.py` adds only Session / caching / security-headers; nothing sets `Content-Encoding`). At the live corpus (≈1 k documents, hundreds of folders with AI descriptions, image docs whose `summary` is a full vision description) that is ~2 MB of uncompressed JSON per Sources-page load, and the browser re-downloads it after every sync and on every shell boot.
|
||||
|
||||
## Design (shared by all tasks — the executor reads this, not the chat)
|
||||
|
||||
**D1 — selective gzip, SSE never touched (task 01, LOCKED A1).** New `app/core/compression.py`, a **pure-ASGI** middleware (`BaseHTTPMiddleware` is forbidden here — it buffers):
|
||||
```python
|
||||
class SelectiveGZipMiddleware:
|
||||
def __init__(self, app, *, minimum_size: int, skip_paths: tuple[str, ...] = ("/api/chat",)) -> None:
|
||||
self.app = app
|
||||
self.gzip = GZipMiddleware(app, minimum_size=minimum_size) # starlette, reused, not forked
|
||||
async def __call__(self, scope, receive, send):
|
||||
if scope["type"] != "http" or scope["path"] in self.skip_paths:
|
||||
await self.app(scope, receive, send); return
|
||||
await self.gzip(scope, receive, send)
|
||||
```
|
||||
Exact-path skip (never a prefix — `/api/chat-images` must still compress). Starlette's `GZipMiddleware` already: requires `Accept-Encoding: gzip`, respects `minimum_size`, skips a response that already sets `content-encoding`, appends `Vary: Accept-Encoding`, and streams with `compressobj` + a final flush. `app/main.py`: new `Settings.compress_min_bytes: int = 860` (`BOR_COMPRESS_MIN_BYTES`, `0` = the feature OFF (skip the middleware entirely), negative → fail startup loudly — the `agent_max_rounds` pattern, PLAN §13), wired by `app.add_middleware(...)` **before** the `SecurityHeadersMiddleware` line so SecurityHeaders stays outermost (A20) and gzip sits outside the caching layer (an already-rewritten HTML page compresses; `/api/*` still passes the caching middleware byte-identically). No production nginx change is required; if nginx also gzips, it will not double-encode an already-encoded body.
|
||||
|
||||
**D2 — a depth-limited tree + server totals (task 02, LOCKED A2).** `app/api/docs.py`:
|
||||
- `build_kb_tree(..., depth: int | None = None)` and `_level_children(..., depth)` — `None` = unlimited (the legacy behavior every existing unit test keeps, byte-identical); a positive budget stops the recursion. A folder node whose own children were NOT built carries `truncated=True`, `children=[]`, its recursive `documents` from `counts[sub]` (unchanged) and — the correctness trap — its `updated_at` from a NEW recursive max-date map computed in `_folder_counts` (because `_subtree_max(children)` over an empty child list would be `None`, which would silently break the phase-106 D9 column). Its `summary` still rides the node (the level row shows a subfolder's description).
|
||||
- `GET /api/docs/tree` gains `depth: int = Query(1, ge=1, le=8)`.
|
||||
- `KbTree` gains **`totals: KbTreeTotals`** (`documents`, `chunks`, `last_indexed: str | None`) computed in the endpoint from the SAME catalogue rows (no extra query) — `sources.js::treeStats()` cannot sum a truncated tree client-side (it walks children) and the three stat cards must stay honest.
|
||||
- `truncated: bool = False` on `KbTreeFolder`/`KbTreeSource` with a `@model_serializer(mode="wrap")` omission when `False` (the `_drop_image_fields` precedent): an untruncated node's wire shape stays byte-identical, and only the depth-limited nodes add the key.
|
||||
|
||||
**D3 — the subtree route + revalidation (task 03, LOCKED A3/A4).**
|
||||
- `GET /api/docs/tree/node?source=<name>&path=<folder>&depth=1` (admin-gated like `/api/docs/tree`, `path=""` = the source root): one node in the `KbTreeFolder`/`KbTreeSource` shape (its direct children + `truncated` flags), 404 `{"detail": "unknown node"}` for an unregistered+unindexed source or a folder that fails the phase-94 existence rule. The RAG view's drill target; the catalogue query is the same one `GET /api/docs/tree` runs (documented: one query per drill, no N+1).
|
||||
- Revalidation, both tree routes (and `GET /api/docs`): build → serialize once (`model_dump_json()` / json) → `ETag: W/"<sha256(body)[:32]>"` + `Cache-Control: private, must-revalidate`; `If-None-Match` match (weak/strong and `*` tolerant) → `Response(304)` with the same two headers. Helpers live in a new `app/core/http_cache.py` (`weak_etag(body)`, `etag_matches(header, tag)`) so the pattern is one function, not a copy. **Why not `sources_meta.version`** (LOCKED A3): bumping that generation marks every saved chat `stale` (phase 53) — the wrong tool — and the admin PATCHes (`/api/documents/summary`, `/api/documents/date`, `/api/folders/summary`) change the tree WITHOUT bumping it, so a version-derived tag would serve stale trees; a content hash is exact, needs no migration, and the tree is built anyway. This is a deliberate, recorded extension of A19 (which governs HTML pages + `/assets/*` + the caching pass-through) — `/api/*` gains a validator on two catalogue routes only; the SSE stream and every mutating route are untouched.
|
||||
|
||||
**D4 — the client drills lazily (task 04).** `frontend/assets/sources.js`: `loadTree()` → `fetch("/api/docs/tree?depth=1")` (still ONE first load), a `levelCache` (Map keyed `source + "/" + folder`), and a drill that fetches `GET /api/docs/tree/node?…` **only** when the target node reports `truncated === true` and is not cached — merged into the cached node before `renderLevel()`. Never-stale (§7.4): the drill's in-flight state disables the row's link + shows one "Loading…" row (the `.kb-*` empty/loading idiom), a failure shows a `role="alert"` line with a Retry affordance and leaves the current level rendered, and the race token (`loadSeq`) extends to the node fetch (a stale drill response may never paint). `folderExistsIn`/`resetVanishedLocation` stop walking a full tree: the node route's 404 IS the "the folder vanished" signal → reset to the top level. `treeStats()` reads `kbTree.totals` (fallback to the existing walk when `totals` is absent — the defensive no-crash path).
|
||||
|
||||
**D5 — the byte budget and the audit (task 05).** New `app/core/response_budget.py`, pure ASGI, one log line per `/api/*` response in the §9 single-line style:
|
||||
`api method=GET path=/api/docs/tree status=200 bytes=204511 enc=gzip duration_ms=41`
|
||||
It counts `http.response.body` chunk bytes as they pass (zero buffering, SSE-safe: `duration_ms` for a stream is the stream's life, documented), logs at INFO when `bytes >= settings.log_large_response_bytes` (`BOR_LOG_LARGE_RESPONSE_BYTES`, default 65536, `0` = log every `/api/*` response, negative fails startup) and at DEBUG otherwise. `GET /api/chat` gets a `stream=1` variant of the line. New `scripts/api_payload_audit.py` (`python -m scripts.api_payload_audit --base-url … --user … --password …`): logs in, hits the candidate endpoints (`/api/docs/tree` at depth 1 and unlimited, `/api/docs`, `/api/documents/content` for the largest doc, `/api/chats`, `/api/chats/{id}` for the largest chat, `/api/git-sources`, `/api/suggestions`, `/api/sync/status`, `/api/tokens`, `/api/config`), and prints uncompressed AND gzip bytes + duration per endpoint with a stated budget (256 KB uncompressed = "look at it", 64 KB = "fine"). Findings land in this directory as `AUDIT.md` (endpoint, before/after bytes, the remedy applied or the accepted-why) — the TODO's "any other endpoints" answered with numbers, not vibes. The audit runs against the local 1,000-doc live-replica snapshot (the `restore-test-db` skill) so the numbers are the production order of magnitude, not the fixture KB's.
|
||||
|
||||
**D6 — verification (task 06).** New `tests/e2e/test_kb_tree_lazy_load.py`: the Sources page's FIRST `/api/docs/tree` request carries `depth=1` + `totals` and no deep children (route-level byte assertion + `via response.json()` node walk), drilling two levels deep issues the node fetch (exactly once per level, cached the second time), the stat cards match `totals` at every level, a 404 node (folder pruned under us — a `page.route` stub) resets to the top level with no stale breadcrumb, `Accept-Encoding: gzip` yields `Content-Encoding: gzip` + a `Vary` header, and the second visit revalidates to a 304 (`page.route` observes the 304 + the body unchanged from cache). Plus the migrated pins: `tests/unit/test_kb_tree_ui.py` (asserts exactly ONE `fetch("/api/docs/tree")` occurrence), `tests/unit/test_frontend_router.py:591`, `tests/e2e/test_navbar_refresh.py:115` (the `#nav-sources` → `/api/docs/tree` expectation), and the phase-97/99 integration shape tests.
|
||||
|
||||
## Dependencies
|
||||
- `97_kb_tree_catalog` + `99_kb_tree_table_and_back_nav` (complete) — the one-fetch/drill-client-side contract this phase **deliberately supersedes** (owner-confirmed 2026-10-07, TODO L5); `106_document_dates` (D9: a truncated node's `updated_at` must stay the recursive max — D2's trap), `98_sync_summary_visibility` (D3 `summary_pending` on every node, including truncated ones), `122_image_documents` (task 04's image map, which stays a bounded select), `156_table_ux_polish` (the bounded scroll panel + sticky header the drill re-renders into), `82_security_headers` / A19 / A20 (the middleware order this phase must not disturb), A15 (SSE).
|
||||
- No migration: nothing in the DB changes.
|
||||
|
||||
## Tasks
|
||||
1. `01_gzip_middleware.md` — `app/core/compression.py` + `Settings.compress_min_bytes` + wiring + the compression/SSE/passthrough tests.
|
||||
2. `02_tree_depth_and_totals.md` — `KbTreeTotals`, the `truncated` omission serializer, `build_kb_tree(depth=…)`, the recursive max-date map, `depth` on `/api/docs/tree`, builder + integration tests.
|
||||
3. `03_tree_node_endpoint_etag.md` — `app/core/http_cache.py`, `GET /api/docs/tree/node`, ETag/304 on the two tree routes + `/api/docs`, integration matrix.
|
||||
4. `04_client_lazy_drill.md` — `sources.js` level cache + per-level fetch + §7.4 drill states + `totals` stat cards + the migrated unit pins.
|
||||
5. `05_response_budget_audit.md` — `app/core/response_budget.py` + `scripts/api_payload_audit.py` + `AUDIT.md` with the measured numbers + the remedies for the offenders (and the accepted-why for the rest) + README/env docs.
|
||||
6. `06_e2e_payload_diet.md` — `tests/e2e/test_kb_tree_lazy_load.py`, the migrated E2E pins, the full gate set, the commit.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: the builder depth/truncation/max-date matrix (`tests/unit/test_kb_tree_builder.py`), the schema omission wire-shape pins, `tests/unit/test_caching.py` extended (the `/api/*` pass-through survives gzip coexistence), config validator pins, `tests/unit/test_kb_tree_ui.py` extended for the drill (cache, single fetch per level, race token, loading/alert states, `totals` stat cards).
|
||||
- Integration: depth + truncated + totals + node-route matrix (200/404/403), the ETag matrix (no header → 200 + `ETag`; match → 304 no body; mismatch → 200; `*`; weak/strong equivalence; a folder-summary PATCH changes the tag), gzip (`Content-Encoding: gzip`, `Vary`, under-`minimum_size` identity, `Accept-Encoding: identity` identity, the SSE stream NOT compressed and arriving frame-by-frame), and the response-budget log line (caplog: INFO over the threshold, DEBUG under, `stream=1` for `/api/chat`).
|
||||
- E2E: `tests/e2e/test_kb_tree_lazy_load.py` green **in isolation**; the owning regression set green in isolation: `test_kb_tree.py` (migrated), `test_kb_tree_nav.py`, `test_navbar_refresh.py`, `test_document_dates.py`, `test_sync_summary_visibility.py`, `test_table_ux_polish.py`, `test_sources_midstream_bug.py`, `test_ls_tree_drilldown.py` (the agent's `ls` view must stay byte-identical), `test_smoke.py` (the no-CDN integration test stays green).
|
||||
- Coverage **>90%** on `app/` (the new `compression.py` / `http_cache.py` / `response_budget.py` are small and fully pinned).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `GET /api/docs/tree` (default `depth=1`) + `Content-Encoding: gzip` transfers < 40 KB on the audited live-replica corpus (measured by `scripts/api_payload_audit.py`, recorded in `AUDIT.md`) — from ~2 MB — and a repeat load answers **304** with an empty body.
|
||||
- [ ] Drilling two folders deep renders correct rows (counts, dates, descriptions, thumbnails, pending markers) with exactly one node fetch per uncached level; a vanished folder resets honestly.
|
||||
- [ ] The stat cards are right at every level (server `totals`), and the tree's per-node `updated_at` is still the recursive max even when the node's children are truncated.
|
||||
- [ ] `POST /api/chat` streams uncompressed and frame-by-frame (unit + E2E proof); HTML pages still serve `no-cache` with the `?v=` rewrite; every response still carries the A20 headers.
|
||||
- [ ] `AUDIT.md` lists every candidate endpoint with before/after bytes and its verdict; offenders over the stated budget are fixed or explicitly accepted with a reason.
|
||||
- [ ] `uv run pytest` green, coverage > 90%, `uv run ruff check . && uv run pyright` clean, the phase E2E green in isolation, one `--no-gpg-sign` commit, phase dir in `complete/`.
|
||||
|
||||
## Locked decisions
|
||||
- **A1 — gzip is selective and pure-ASGI**: `POST /api/chat` (SSE, A15) is skipped by EXACT path; no `BaseHTTPMiddleware`; `BOR_COMPRESS_MIN_BYTES` (`0` = off, negative = fail loudly) — owner-confirmed 2026-10-07 ("gzip + ETag + lazy drill").
|
||||
- **A2 — `depth` (default 1, max 8) + `totals` on the tree, `truncated` omitted when false**: the depth-limited response keeps every per-node field the view renders (counts, `updated_at`, `summary`, `summary_pending`, image keys) and only omits un-rendered DEPTH; the recursive max-date map is mandatory so phase 106 D9 does not silently regress — owner-confirmed 2026-10-07.
|
||||
- **A3 — the phase-97 "ONE fetch, drill client-side" contract is superseded** by one initial load + one fetch per drilled level (the deliberate cost of a small first payload; the drill UX, breadcrumb, and scroll-panel behavior stay as phases 97/99/156 built them) — owner-confirmed 2026-10-07.
|
||||
- **A4 — the ETag is a weak hash of the response body, NOT `sources_meta.version`**: bumping the generation would mark every saved chat `stale` (phase 53), and the admin summary/date/folder PATCHes mutate the tree without bumping it. Recorded as a scoped extension of A19 (validators on two read-only catalogue routes; SSE and mutations untouched) — owner-confirmed 2026-10-07.
|
||||
- **A5 — the byte budget is a log line + a script, not a hard 413**: no endpoint starts rejecting large payloads this phase (the app's own documents are legitimately large); the audit's 256 KB "look at it" threshold drives follow-up work, recorded in `AUDIT.md`.
|
||||
|
||||
## Commit
|
||||
```bash
|
||||
git add app/ frontend/ scripts/ tests/ README.md .env.example .agents/phases/ && git commit --no-gpg-sign -m "perf(api): gzip non-SSE responses, a depth-limited KB tree with per-level drill and ETag revalidation, and a measured response byte budget"
|
||||
```
|
||||
@@ -0,0 +1,33 @@
|
||||
# Task 01 — Selective gzip: `app/core/compression.py`, `BOR_COMPRESS_MIN_BYTES`, SSE untouched (phase 167 D1, LOCKED A1)
|
||||
|
||||
**Phase:** `167_api_payload_diet` · **Source:** `TODO.md:5` — "GET <https://brain.reeseapps.com/api/docs/tree> loads 2 MB in production which causes long loading times."
|
||||
**Story:** n/a
|
||||
|
||||
## Objective
|
||||
Every HTTP response except the `POST /api/chat` SSE stream compresses with gzip, with a config kill switch, the existing middleware order and contracts (A19 caching pass-through, A20 security headers outermost) untouched, and a test that proves the chat stream still arrives frame-by-frame and uncompressed.
|
||||
|
||||
## Work
|
||||
1. New `app/core/compression.py` — the middleware of phase 167 D1, **pure ASGI** (never `BaseHTTPMiddleware`, which buffers and would break streaming responses):
|
||||
- `class SelectiveGZipMiddleware` holding `self.app` and a composed `starlette.middleware.gzip.GZipMiddleware(self.app, minimum_size=minimum_size)`; `__call__` delegates to the gzip path unless `scope["type"] != "http"` or `scope["path"] in skip_paths` (EXACT match — `/api/chat-images` must still compress), in which case it calls `self.app` directly;
|
||||
- `DEFAULT_SKIP_PATHS = ("/api/chat",)` module constant, docstring names A15 (SSE) and the buffering reason; do NOT re-implement compression math — Starlette's `GZipMiddleware` owns `Accept-Encoding` negotiation, `minimum_size`, the already-`content-encoding` skip, the `Vary: Accept-Encoding` append, and the `compressobj` streaming + final flush;
|
||||
- module docstring: the TODO's 2 MB defect, the ~8-12× JSON expectation, the SSE exclusion, and "nginx may also gzip; an already-encoded body is never double-encoded".
|
||||
2. `app/config.py` — `compress_min_bytes: int = 860` (`BOR_COMPRESS_MIN_BYTES`) beside the other ints, with the `agent_max_rounds` validator pattern (`_compress_min_bytes_non_negative`: negative → `ValueError("compress_min_bytes must be >= 0 (0 = compression disabled)")`) and the docstring stating `0` = the feature is OFF (no middleware), `> 0` = the Starlette minimum-size floor (860 = the classic nginx default, below which compression can inflate).
|
||||
3. `app/main.py` — wire it, added **before** the `app.add_middleware(SecurityHeadersMiddleware)` line so SecurityHeaders stays outermost (A20, the existing comment says "last add_middleware"): construct `SelectiveGZipMiddleware` only when `settings.compress_min_bytes > 0` (the `0` = OFF path adds nothing at all), with a comment naming TODO L5, the SSE skip, and the position (outside `configure_caching`, inside SecurityHeaders → the rewritten HTML pages compress too, `/api/*` still passes the caching layer byte-identically before compression).
|
||||
4. `.env.example` — `BOR_COMPRESS_MIN_BYTES=860` with the three-value comment (0 off, negative refuses startup, the 860 rationale).
|
||||
5. New `tests/unit/test_compression.py`:
|
||||
- a tiny app with a ~50 KB JSON route + a ~50 byte route, wrapped in the middleware: `Accept-Encoding: gzip` → `Content-Encoding: gzip` + a body that `gzip.decompress`es to the original + `Vary` mentions `accept-encoding`; `Accept-Encoding: identity` → no `Content-Encoding`; the small route → identity (under the floor); a response that already set `content-encoding` → untouched;
|
||||
- `skip_paths`: a `/api/chat`-named route in the test app is NOT compressed and streams incrementally (assert the received chunks arrive before the response completes — the `list(stream)` idiom the existing SSE tests use), while a `/api/chat-images/x` route IS compressed (the exact-match pin);
|
||||
- `scope["type"] == "lifespan"`/`"websocket"` pass-through;
|
||||
- config: `compress_min_bytes = 0` → `create_app` adds no gzip layer (assert via `app.user_middleware`'s classes), `-1` → the validator raises naming the field.
|
||||
6. Integration pin `tests/integration/test_docs_api.py` (or the middleware-owning integration file — locate via `SecurityHeadersMiddleware` in `tests/`): the real app's `GET /api/health`-class small response stays identity; a large `/api/docs/tree` (seed ≥ 50 docs) returns `Content-Encoding: gzip` and decompresses to the documented JSON; the A20 headers (`CSP`, `X-Content-Type-Options`, `X-Frame-Options`/`frame-ancestors`) are STILL present on a gzipped response (the outer-layer invariant); the HTML page contract (A19: `no-cache`, `?v=` rewrite, no validators) survives decompression.
|
||||
7. ASSUMPTION: `Settings.compress_min_bytes` (name + default 860) — the TODO named no knob; the house kill-switch convention (`0` off, negative loud) is what makes it owner-controllable without a code change.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit + integration as above; `uv run pytest tests/integration/test_chat_api.py -v` (or the suite that owns the SSE contract) stays green — the stream is not compressed and not buffered.
|
||||
- Coverage: **>90%** on `app/core/compression.py` (every branch listed above is pinned).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `curl -H 'Accept-Encoding: gzip' …/api/docs/tree | wc -c` beats the identity size by an order of magnitude on any corpus above the floor; `Vary: Accept-Encoding` is present.
|
||||
- [ ] `POST /api/chat` responses carry NO `Content-Encoding` and stream frame-by-frame (test-proven), and `/api/chat-images` still compresses.
|
||||
- [ ] `BOR_COMPRESS_MIN_BYTES=0` removes the middleware entirely; `-1` refuses startup naming the field.
|
||||
- [ ] A20 headers, A19 page/asset caching, and the `/api/*` pass-through are unchanged; `uv run pytest` green; ruff + pyright clean.
|
||||
@@ -0,0 +1,27 @@
|
||||
# Task 02 — Depth-limited tree + server totals: `truncated`, the recursive max-date map, `KbTreeTotals` (phase 167 D2, LOCKED A2)
|
||||
|
||||
**Phase:** `167_api_payload_diet` · **Source:** `TODO.md:5` — "GET …/api/docs/tree loads 2 MB in production which causes long loading times."
|
||||
**Story:** n/a
|
||||
|
||||
## Objective
|
||||
The pure builder can stop at a depth budget: a node whose children were not built says so (`truncated: true`) while keeping every field the Sources view renders (recursive count, recursive `updated_at`, its own `summary`, `summary_pending`, the image affordance), the response gains the `totals` the stat cards need, and `GET /api/docs/tree` defaults to `depth=1`.
|
||||
|
||||
## Work
|
||||
1. `app/schemas.py` — `KbTreeTotals(BaseModel)`: `documents: int = Field(ge=0)`, `chunks: int = Field(ge=0)`, `last_indexed: str | None = None` (docstring: the catalogue-wide totals the RAG view's three stat cards read, because a depth-limited tree cannot be summed client-side — the phase-97 `treeStats()` walk needed the whole tree).
|
||||
2. `app/schemas.py` — `KbTree` gains `totals: KbTreeTotals = KbTreeTotals()` (docstring: additive top-level key, TODO L5) and `KbTreeFolder` / `KbTreeSource` gain `truncated: bool = False` with an OMIT-WHEN-FALSE serializer. Follow the file's existing omission idiom (`_drop_image_fields` + `@model_serializer(mode="wrap")`): add the `truncated` key only when `True`, so an untruncated node's wire shape is byte-identical to today (every existing wire pin stays green). `KbTreeFile` never truncates (a file has no children).
|
||||
3. `app/api/docs.py::_folder_counts` — extend the return to `(folders, counts, max_dates)` where `max_dates[folder]` is the folder's RECURSIVE MAX document `created_at` (the same ISO-8601 strings, so the lexicographic max is the chronological one — the phase-106 D9 rule), computed from the rows in the same pass. Rationale in the docstring: a truncated node has NO built children, so `_subtree_max(children)` would read `None` and silently lose the D9 column; the aggregate map is the truncation-proof source. Keep `_subtree_max` for the expanded path UNCHANGED and add a unit test asserting the two agree for every expanded folder (one concept, two derivations, cross-pinned).
|
||||
4. `app/api/docs.py::_level_children` — add `depth: int | None`; per subfolder: when `depth is None` or `depth > 1`, build the node as today with the recursion at `depth - 1`; when the budget is exhausted, build `KbTreeFolder(path=sub, documents=counts[sub], updated_at=max_dates[sub], summary=summaries.get((source, sub)), summary_pending=<unchanged rule>, children=[], truncated=True)`. Update the docstring with the depth rule + the truncated-node field set (the level row renders a subfolder's name, count, date and description WITHOUT its contents).
|
||||
5. `app/api/docs.py::build_kb_tree` — thread `depth` (`None` = unlimited: every existing unit call and every existing expectation is byte-identical); `_source_node` passes the budget down and never marks the SOURCE node itself truncated at `depth ≥ 1` (the source's direct children are always built — the top level of the RAG view is source → level 1).
|
||||
6. `app/api/docs.py::list_kb_tree` — `depth: int = Query(1, ge=1, le=8)` (docstring: 1 = sources + one level, the view's first paint; 8 = the practical KB ceiling; the endpoint computes `totals` from the catalogue rows it already fetched: `documents = len(rows)`, `chunks = sum(chunks)`, `last_indexed = max(indexed_at)` → `None` for an empty KB) and passes it to the builder. `GET /api/docs` and the agent's `ls` path are untouched (pin it: `app/rag/agent.py` imports are unchanged).
|
||||
7. `tests/unit/test_kb_tree_builder.py` (extend): depth `None` vs `1` vs `2` vs deep on the multi-level fixture — the truncated node's exact field set (count recursive, `updated_at` = the deep max, summary present, `summary_pending` present, `children == []`, `truncated is True`), the totals math incl. the empty KB (`last_indexed is None`), the `_subtree_max`/`max_dates` cross-check on every expanded folder, and the phase-94/98 invariants (existence rule, `missing_folder_summaries` cross-check) at each depth.
|
||||
8. `tests/integration/test_docs_api.py` (extend): `GET /api/docs/tree` default → depth 1 (deep children absent, `truncated` present on a subfolder that has them, `totals` correct vs `GET /api/docs`), `?depth=3` → deeper, `?depth=0` / `?depth=9` / `?depth=abc` → 422, anonymous → 403 unchanged, and the wire-shape pin: an UNtruncated node's JSON keys are exactly the pre-phase set (no `truncated` key).
|
||||
|
||||
## Testing & Quality
|
||||
- Unit + integration above; `uv run pytest tests/unit/test_kb_tree_builder.py tests/integration/test_docs_api.py -v` green.
|
||||
- Coverage **>90%** on the modified `app/api/docs.py` / `app/schemas.py` lines.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] Default `/api/docs/tree` returns sources + ONE level with correct `totals`, and every node that has unloaded children carries `truncated: true`.
|
||||
- [ ] A truncated node still carries its recursive count, its recursive `updated_at`, its `summary`, and `summary_pending` — phase 98/106 semantics intact at every depth (unit + integration).
|
||||
- [ ] `depth=None` (builder default) reproduces today's tree byte-for-byte; untruncated nodes' wire shape is unchanged.
|
||||
- [ ] `uv run pytest` green; coverage > 90%; ruff + pyright clean.
|
||||
@@ -0,0 +1,35 @@
|
||||
# Task 03 — The subtree route and revalidation: `GET /api/docs/tree/node`, `app/core/http_cache.py`, ETag/304 (phase 167 D3, LOCKED A3/A4)
|
||||
|
||||
**Phase:** `167_api_payload_diet` · **Source:** `TODO.md:5` — "Fix that any any other endpoints that could load a ton of data."
|
||||
**Story:** n/a
|
||||
|
||||
## Objective
|
||||
One small admin-gated route serves a single tree node's direct children (the drill's fetch target), and the catalogue read routes (`/api/docs/tree`, `/api/docs/tree/node`, `/api/docs`) carry a content-exact weak `ETag` + `Cache-Control: private, must-revalidate` so a repeat load costs a 304 with no body — without touching `sources_meta` (which would mark saved chats stale) or the SSE/mutating surface.
|
||||
|
||||
## Work
|
||||
1. New `app/core/http_cache.py` (small, total, no I/O):
|
||||
- `weak_etag(body: str | bytes) -> str` → `W/"<sha256 hexdigest[:32]>"` (docstring: content-exact by construction; 32 hex chars is the file's truncation convention and collisions are not a correctness risk for a revalidated cache — a false match only ever costs one stale repaint on a 2 MB namespace, and the tag is over the EXACT served bytes);
|
||||
- `etag_matches(header: str | None, tag: str) -> bool` → tolerant of `*`, a comma-separated list, and weak/strong equivalence (`W/"x"` matches `"x"` both directions), whitespace-tolerant, `None`/empty → `False`;
|
||||
- `revalidate_headers(etag: str) -> dict[str, str]` → `{"ETag": etag, "Cache-Control": "private, must-revalidate"}` (ONE place for the pair; docstring names A19's scope — HTML pages stay `no-cache` with no validators, `/assets/*` stay immutable, this is the read-only API arm).
|
||||
2. `app/api/docs.py` — `GET /api/docs/tree/node`:
|
||||
- signature: `source: str = Query(...)`, `path: str = Query("")` (source-relative; `""` = the source root node), `depth: int = Query(1, ge=1, le=8)`, `db=Depends(get_db)`, `_admin=Depends(require_admin)` (`403 {"detail": "admin only"}` — the same gate as `/api/docs/tree`), `request: Request`;
|
||||
- composition: the SAME catalogue query as `list_kb_tree` (one query, no N+1 — the file's docstring paragraph says so), the same `folder_summaries` + image-docs bounded selects, then build the single node through the pure helpers: resolve the source (registry name or indexed source), `_folder_counts(rows)` for that source, `_level_children(source, path, folders, counts, rows, summaries, images, depth=depth)`; the node is a `KbTreeSource` when `path == ""` else a `KbTreeFolder` with `documents=counts[path]`, `updated_at=max_dates[path]`, `summary`, `summary_pending` (the phase-98 D3 rule), `truncated` per the depth budget;
|
||||
- 404 `{"detail": "unknown node"}` when: the source is neither registered nor indexed, OR the folder fails the phase-94 existence rule (`path` not in `folders` and `path != ""`). Never a filesystem path built from `path` (docstring: the row-only rule, the phase-10 traversal argument reused);
|
||||
- `response_model=None` on the decorator with the union return annotated, and the body returned as a raw `Response` (below) so the serializer is not run twice.
|
||||
3. `app/api/docs.py` — ETag wiring on the three read routes (`/api/docs/tree`, `/api/docs/tree/node`, `/api/docs`): build the model → `body = model.model_dump_json()` (for `/api/docs`: `DocList.model_dump_json()`) → `etag = weak_etag(body)` → `if etag_matches(request.headers.get("if-none-match"), etag): return Response(status_code=304, headers=revalidate_headers(etag))` → else `Response(content=body, media_type="application/json", headers=revalidate_headers(etag))`. Docstring per route: the client is `fetch` (a 304 is served from its cache transparently), and `must-revalidate` guarantees a sync or an admin edit is seen on the next load — the tag changes because the body changes. Keep `response_model=KbTree`/`DocList` on the decorators for the OpenAPI schema (a returned `Response` short-circuits serialization — note that in the docstring).
|
||||
- ASSUMPTION: `GET /api/documents/content` is NOT given a validator this phase (one document's body, on demand, already compressed; the audit in task 05 decides whether it needs more) — recorded so the scope is explicit.
|
||||
4. `tests/integration/test_docs_api.py` (extend):
|
||||
- the node route: 200 for a source root (`path=""`, the `KbTreeSource` shape), 200 for a nested folder (correct `documents`/`updated_at`/`summary`/`summary_pending`/`children` at `depth=1` with `truncated` on the deeper subfolders), `?depth=2` deeper, 404 `unknown node` for an unknown source / a nonexistent folder / a `path` that is a FILE's path, 403 anonymous, and a cross-check that the node's children equal the same level of the unlimited `/api/docs/tree?depth=8` response (ONE concept, two routes, pinned equal);
|
||||
- ETag matrix on all three routes: first GET → 200 + `ETag` + `Cache-Control: private, must-revalidate`; re-send with `If-None-Match` → 304 + empty body + both headers; a wrong tag → 200; a `W/`-vs-strong mismatch pair → 304; `*` → 304; after `PATCH /api/folders/summary` (and after `PATCH /api/documents/date`) the tag CHANGES and the next GET is a 200 with the new tree (the A4 proof — a version-derived tag would have failed exactly here);
|
||||
- gzip coexistence: a gzipped 200 still carries the `ETag` (the header survives the compression layer) and a conditional request returns a body-less 304.
|
||||
5. `tests/unit/test_http_cache.py` (new): `weak_etag` stability/length/prefix (`W/"…"`), bytes-vs-str equality, `etag_matches` for `None`, empty, `*`, single, comma-list, weak↔strong both directions, whitespace, and a tag that merely shares a prefix → `False`.
|
||||
|
||||
## Testing & Quality
|
||||
- Integration + unit above; `uv run pytest tests/integration/test_docs_api.py tests/unit/test_http_cache.py -v` green; `tests/integration/test_caching.py` stays green (the `/api/*` pass-through and the HTML rewrite are untouched by a route-level header).
|
||||
- Coverage **>90%** on `app/core/http_cache.py` and the new route lines.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `GET /api/docs/tree/node?source=X&path=Y` returns exactly that folder's direct children with the same per-node fields the full tree produces (equality pinned against `depth=8`).
|
||||
- [ ] A repeat request with `If-None-Match` answers **304, no body**; a folder-summary or document-date edit changes the tag and forces the next 200.
|
||||
- [ ] Anonymous gets `403 {"detail":"admin only"}`; an unknown source/folder gets `404 {"detail":"unknown node"}`; nothing builds a filesystem path from `path`.
|
||||
- [ ] `uv run pytest` green; coverage > 90%; ruff + pyright clean.
|
||||
@@ -0,0 +1,35 @@
|
||||
# Task 04 — The client drills lazily: level cache, per-level fetch, §7.4 drill states, `totals` stat cards (phase 167 D4, LOCKED A3)
|
||||
|
||||
**Phase:** `167_api_payload_diet` · **Source:** `TODO.md:5` — "GET …/api/docs/tree loads 2 MB in production which causes long loading times."
|
||||
**Story:** n/a
|
||||
|
||||
## Objective
|
||||
The RAG view loads one small tree, fetches a folder's contents the first time it is opened, caches it, keeps the breadcrumb/scroll-panel/editing behavior identical, and never shows a stale or half-updated level — the UX cost of A3 paid in full so the user cannot tell the tree is no longer one payload.
|
||||
|
||||
## Work
|
||||
1. `frontend/assets/sources.js::loadTree()` (~L1625) — `fetch("/api/docs/tree?depth=1")`; on success: store the tree, CLEAR `levelCache` (a refresh invalidates every drilled level — the sync/refresh call sites at ~L1258, ~L1386, ~L2192, ~L2213 keep working unchanged), then `resetVanishedLocation()` + `renderLevel()` as today. Keep the `loadSeq` race token and the `renderEmpty()` failure path.
|
||||
2. `frontend/assets/sources.js` — new module state + the drill:
|
||||
- `const levelCache = new Map();` keyed `source + "/" + folder` (a node's `path`), populated ONLY by a node fetch;
|
||||
- `function needsLevelFetch(node) { return node.truncated === true && !levelCache.has(key); }`;
|
||||
- `enterFolder(source, folder)` (the folder-row click path, today a pure `current = …; renderLevel()`): when the target needs a fetch → set the in-flight state (`drillPending = {source, folder}`, the row link gets `aria-busy="true"` + `aria-disabled="true"` so a double click cannot start a second fetch), `await fetch("/api/docs/tree/node?source=…&path=…")`, merge the result (`Object.assign(targetNode, node)` + `levelCache.set(key, true)`), clear the flags, then `current = …; renderLevel()`. A stale response (token changed) never paints (the existing `my === loadSeq` convention extended to the drill).
|
||||
- In-flight rendering: render the level the user is LEAVING unchanged and show one loading row in the folders panel using the file's existing status/empty idiom (a `role="status"` `.kb-level-note`-style line "Loading <folder>…" — reuse an existing class if one fits, otherwise add ONE `.kb-level-loading` rule to `styles.css` with tokens only, no animation beyond the existing house spinner if one exists);
|
||||
- failure (non-OK or a network throw): `role="alert"` line "Couldn't load <folder> — Retry." with a real `<button type="button">Retry</button>` that re-runs the same fetch, and the CURRENT level stays rendered (never blank, never a stale breadcrumb — PLAN §7.4 "every failure path re-enables its controls").
|
||||
3. `frontend/assets/sources.js` — the existence rules stop walking a full tree:
|
||||
- `folderExistsIn(src, folderPath)` (~L1667) is replaced by `folderKnownIn(src, folderPath)`: true when the path's whole ancestor chain is present in what the client holds (the loaded tree + cached levels), so `resetVanishedLocation()` can still answer after a re-fetch WITHOUT assuming a full tree; when a drill fetch answers `404` the folder is gone → `current = { source: null, folder: null }` + `renderLevel()` (the honest reset, the phase-97 "never stale" contract carried over), and the failing `levelCache` entry is not cached;
|
||||
- `currentLevelNode()` (~L1680) keeps walking, but through cached nodes (unchanged semantics for loaded levels).
|
||||
4. `frontend/assets/sources.js::treeStats()` (~L1703) — read `kbTree.totals` (`documents`, `chunks`, `last_indexed`) when present, and keep the existing recursive walk ONLY as the defensive fallback when `totals` is absent (a tree served by a pre-phase process); the three stat cards (`statDocs`, `statChunks`, `statLast`) render identically at every level (pin the exact formatting call — `fmtDate(st.last)` — unchanged).
|
||||
5. `frontend/assets/sources.js` docstring (the module header, ~L55-103) — rewrite the "ONE fetch of GET /api/docs/tree returns the whole tree" paragraph for the depth-1 + per-level model, citing TODO L5 and phase 167, and keep the phase-79 race-token, phase-90 upload and phase-98 pending narratives accurate.
|
||||
6. `tests/unit/test_kb_tree_ui.py` (MIGRATE — it currently asserts `js.count('fetch("/api/docs/tree")') == 1` at ~L250 and the "ONE fetch" contract at ~L9/L27): the fetch assertion becomes `fetch("/api/docs/tree?depth=1")` exactly once + `fetch("/api/docs/tree/node?` exactly once, and the "drills client-side" docstring assertions become the level-cache assertions: `levelCache` declared once, cleared inside `loadTree`, consulted by `needsLevelFetch`, written only after a 200; the 404 → reset branch exists; `drillPending` guards a second click; the Retry button is a real `<button>`; `treeStats` prefers `totals`. Preserve every OTHER phase-97/99 pin (breadcrumb, sticky header, scroll panel, editor reset, empty state).
|
||||
7. `tests/unit/test_frontend_router.py:591` — the shell-route pin (`'fetch("/api/docs/tree")' in body`) moves to the `?depth=1` URL, keeping its original intent (the Sources view loads the catalogue from the tree endpoint, not the flat one).
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: the migrated + new pins above; run the owning set for collateral: `uv run pytest tests/unit -k "kb_tree or frontend_router or table_ux or source" -v`.
|
||||
- No `app/` change → coverage gate unmoved (> 90% on `app/` stays green).
|
||||
- Frontend house rules hold: `createElement` + `textContent` only (no HTML strings), `aria-busy`/`aria-disabled` on the in-flight link, `role="status"`/`role="alert"` for the two states, `:focus-visible` on the new Retry button via the global rule, tokens-only CSS if a new class is added.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] First paint = ONE `depth=1` request; the second visit to a folder is free (cache hit, zero network — unit-pinned).
|
||||
- [ ] A drilled level shows correct rows (counts, dates, descriptions, pending markers, image thumbnails) inside the phase-156 bounded panel with the sticky header intact.
|
||||
- [ ] In-flight = `aria-busy` + one status line; failure = `role="alert"` + working Retry with the current level still rendered; a 404 resets to the top level with no stale breadcrumb; a stale response never paints.
|
||||
- [ ] The three stat cards match the server `totals` at every level, and the pre-phase fallback path exists.
|
||||
- [ ] Full `uv run pytest` green; ruff + pyright clean.
|
||||
@@ -0,0 +1,36 @@
|
||||
# Task 05 — The byte budget: `/api/*` response-size log line + `scripts/api_payload_audit.py` + `AUDIT.md` (phase 167 D5, LOCKED A5)
|
||||
|
||||
**Phase:** `167_api_payload_diet` · **Source:** `TODO.md:5` — "Fix that **any any other endpoints that could load a ton of data**."
|
||||
**Story:** n/a
|
||||
|
||||
## Objective
|
||||
The app can answer "which endpoint is heavy, right now, in production?" from its own logs, and the owner's "any other endpoints" gets a measured, written answer: every candidate endpoint's uncompressed and gzipped size against a stated budget, with each offender either fixed or explicitly accepted.
|
||||
|
||||
## Work
|
||||
1. New `app/core/response_budget.py` — **pure ASGI** (no buffering, SSE-safe):
|
||||
- wraps `send`, counting the bytes of every `http.response.body` message's `body` chunk, and `receive`-independent timing (`time.monotonic()` at entry → at `more_body=False` completion);
|
||||
- logs ONE §9-style line per `/api/*` response on the `http.response.body` completion (for a stream that is when the stream ends — say so in the docstring):
|
||||
`api method=GET path=/api/docs/tree status=200 bytes=204511 enc=gzip duration_ms=41`
|
||||
(`enc` = the response's `content-encoding` or `identity`; add ` stream=1` for `media_type == "text/event-stream"` responses, where `bytes` is the streamed total and `duration_ms` is the stream's life);
|
||||
- level: INFO when `bytes >= settings.log_large_response_bytes`, DEBUG otherwise; non-`/api/` paths and non-HTTP scopes pass through silently; an exception in the wrapped app still logs (finally-style, `bytes` as counted) and re-raises unchanged;
|
||||
- `304`s log `bytes=0` (the win this phase bought is visible in the log, not hidden).
|
||||
2. `app/config.py` — `log_large_response_bytes: int = 65536` (`BOR_LOG_LARGE_RESPONSE_BYTES`) with the `agent_max_rounds` validator (negative → `ValueError` naming the field; `0` = log EVERY `/api/*` response at INFO). `.env.example` entry with the three-value comment.
|
||||
3. `app/main.py` — `app.add_middleware(ResponseBudgetMiddleware)` **before** the gzip `add_middleware` call, so the stack is SecurityHeaders (outermost) → gzip → response-budget → caching → session → routes. Consequence to state in the comment: `bytes=` is the bytes the ROUTE produced (uncompressed) while `enc=` reports what went on the wire — the two together answer "how fat is it, and how fat did the client see it".
|
||||
4. New `scripts/api_payload_audit.py` (`python -m scripts.api_payload_audit`), stdlib + `httpx` only, `app.core.debugging` conditional import preserved if it touches boot (AGENTS.md rule 7 — it does not import the app factory, so just follow the other scripts' shape, see `scripts/eval_retrieval.py`):
|
||||
- args: `--base-url` (default `http://localhost:8000`), `--username`/`--password` (or `BOR_ADMIN_*` env reads, never hardcoded), `--budget` (default `262144`), `--path-tree` etc. only if needed;
|
||||
- logs in (the real `/api/login` session cookie), then for each candidate: `GET /api/docs/tree?depth=1`, `/api/docs/tree?depth=8`, `/api/docs/tree/node?source=<first>&path=<first folder>`, `GET /api/docs`, `GET /api/documents/content` for the LARGEST document (ask `/api/docs` then pick by chunks) — its `content` length is the size, `GET /api/chats`, `GET /api/chats/{id}` for the largest chat, `/api/git-sources`, `/api/suggestions`, `/api/sync/status`, `/api/tokens`, `/api/config`, `/api/whoami` — measuring BOTH `identity` and `gzip` request variants (explicit `Accept-Encoding`) via `response.content` length + wall time, plus one conditional re-request for the ETag routes (expect 304/0 bytes);
|
||||
- prints a table (endpoint, identity bytes, gzip bytes, ratio, ms, verdict) where the verdict is `ok` / `LOOK (> budget)` / `304`, exits 0 (it is a report, not a gate) — and prints the corpus it measured (document count from `/api/docs`) so numbers are interpretable.
|
||||
5. `.agents/phases/todo/167_api_payload_diet/AUDIT.md` (new, committed with the phase) — the measured record: how it was run (the local DB restored to the **1,000-document live-replica snapshot** via the `restore-test-db` skill, so the order of magnitude matches production), the full table before (uncompressed, no middleware) and after (depth-1 + gzip + ETag), and one verdict line per endpoint: **fixed** (tree: 2 MB → X KB, second load 304) / **accepted with why** (`/api/documents/content` — one document on demand, compressed; `/api/chats/{id}` — the owner opening one saved chat; `/api/sync/status` — tiny but polled, so the log line is what matters) / **follow-up** (anything still over `--budget`, with the proposed remedy named). Do not hand-write numbers: paste the script's output.
|
||||
6. Remedies in scope if the audit shows an offender (apply, don't just note): add the task-03 ETag pair to any other read route that is large and re-fetched (e.g. `/api/git-sources` if it grows past the budget); raise nothing, reject nothing (LOCKED A5).
|
||||
7. `README.md` — an "Operational notes" subsection: the log line format + how to read it (`grep ' :: api ' `), `BOR_LOG_LARGE_RESPONSE_BYTES`, running the audit script against a restored replica, and the deploy note that the app gzips itself (so nginx `gzip on` is optional, and `proxy_set_header Accept-Encoding` passthrough is fine).
|
||||
|
||||
## Testing & Quality
|
||||
- Unit (`tests/unit/test_response_budget.py`, new): the line's exact shape for a large 200 (INFO), a small 200 (DEBUG), a 304 (`bytes=0`), a non-`/api/` path (nothing logged), a streaming response (`stream=1`, bytes = sum of chunks, logs on completion), the `enc=gzip` vs `enc=identity` reading, an app exception (logged + re-raised), and the config validator (`0` → INFO for everything, negative → refuses startup).
|
||||
- Script smoke: `uv run python -m scripts.api_payload_audit --base-url http://localhost:8000` against a running dev app (document count + at least the tree rows printed; the script must fail loudly with a clear message when the app is unreachable or the login is rejected).
|
||||
- Coverage **>90%** on `app/core/response_budget.py`.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] One parseable `api method=… path=… status=… bytes=… enc=… duration_ms=…` line per `/api/*` response at the configured level, with the SSE stream logged once at stream end and never buffered.
|
||||
- [ ] `AUDIT.md` exists in this phase directory with real numbers (before/after), the corpus it measured, and a verdict per candidate endpoint.
|
||||
- [ ] Every endpoint over the stated budget is either fixed in this phase or carries a named follow-up with its reason.
|
||||
- [ ] README documents the log line, the env var, the audit script, and the self-gzip deploy note; `uv run pytest` green; ruff + pyright clean.
|
||||
@@ -0,0 +1,39 @@
|
||||
# Task 06 — `tests/e2e/test_kb_tree_lazy_load.py`, the migrated pins, the full gates (phase 167 D6)
|
||||
|
||||
**Phase:** `167_api_payload_diet` · **Source:** `TODO.md:5` — "GET <https://brain.reeseapps.com/api/docs/tree> loads 2 MB in production which causes long loading times. Fix that any any other endpoints that could load a ton of data."
|
||||
**Story:** n/a
|
||||
|
||||
## Objective
|
||||
An isolated Playwright suite proves the owner-visible outcome — a small first Sources load, correct content when drilling, no refetch, an honest reset when a folder disappears, gzip on the wire, a 304 on the second visit — and every pin the superseded phase-97 contract owned (D3/A3) is migrated rather than deleted. Then the phase gates close it out.
|
||||
|
||||
## Work
|
||||
1. New `tests/e2e/test_kb_tree_lazy_load.py` (module-scoped `app_server`/`app_url`; seed a multi-level KB directly through the DB the way `tests/e2e/test_kb_tree.py` does — ≥ 3 sources, a 3-deep folder chain, one folder with a stored `folder_summaries` row, one pending folder, and one image doc if `BOR_IMAGES=true` is cheap in this suite's env; read `tests/e2e/test_kb_tree.py` and `tests/e2e/test_kb_tree_nav.py` for the seeding + drill helpers first):
|
||||
- **the first load is small**: collect the network (`page.on("response")`) for the Sources view boot → exactly one `/api/docs/tree?depth=1` request; its JSON has `totals` with the expected `documents`/`chunks`, its top-level source nodes carry `documents` counts that include the deep files (recursive), and NO node deeper than level 1 exists while level-1 subfolders report `truncated === true` (assert on the parsed body, not on bytes);
|
||||
- **the stat cards** match `totals` at the top level AND after drilling two levels (the numbers never change — they are catalogue-wide);
|
||||
- **the drill**: click into a level-1 folder → exactly one `/api/docs/tree/node?source=…&path=…` request; rows render (subfolders sorted, then files in catalog order) with the stored description in the level block, the pending marker on the pending folder, and the phase-156 sticky header/scroll panel intact; click into a deeper folder → a second node fetch; go BACK to the first folder → **zero** new requests (cache);
|
||||
- **the revalidation**: reload the view → the browser re-sends `If-None-Match` (observe the request header) and the response is `304` with an empty body while the table still renders (the `page.route`/`expect_response` idiom);
|
||||
- **gzip**: assert the tree response's `Content-Encoding` is `gzip` and `Vary` includes `accept-encoding` (Chromium sends `Accept-Encoding: gzip, deflate, br` by default — pin what actually arrives, and if the harness negotiates identity, assert the server side in the integration suite instead and say so in the test docstring);
|
||||
- **the vanished folder** (§7.4): a `page.route` stub that answers the node fetch with `404 {"detail":"unknown node"}` → the alert line + Retry button appear, the level the user was on stays rendered; click a parent crumb → the level is fine; then let the real route answer and Retry → the level loads;
|
||||
- **in-flight**: a `page.route` handler that delays the node response → the clicked link is `aria-busy`/`aria-disabled` and the status line shows, and a second click does NOT issue a second request.
|
||||
2. `tests/e2e/test_kb_tree.py` (MIGRATE, do not delete): the suite's "ONE fetch of `/api/docs/tree`, drill is client-side" assertions become the depth-1 + per-level assertions (the file's outline comment block first, then each assertion); every OTHER claim it owns (breadcrumb text, back navigation, table columns, dates, summary editor round-trip, pending markers, empty state, the anonymous gate) stays, re-aimed at the level the user is on. Each edit is a re-point, never a removal — state that in the commit body.
|
||||
3. `tests/e2e/test_navbar_refresh.py:115` — the `("#nav-sources", "/sources.html", "/api/docs/tree")` expectation matches the new URL (keep the substring `/api/docs/tree` if the assertion is a substring test — verify, then pin precisely); re-run that suite.
|
||||
4. `tests/e2e/test_kb_tree_nav.py`, `test_document_dates.py`, `test_sync_summary_visibility.py`, `test_table_ux_polish.py`, `test_sources_midstream_bug.py`, `test_ls_tree_drilldown.py` (the agent's `ls` must be untouched) — run in isolation, repair ONLY where a `pre`-condition assumed the full tree loaded (e.g. a test that expects a deep folder's rows before drilling — add the drill step), never by weakening an assertion.
|
||||
5. Final gates + docs, in one pass:
|
||||
- `uv run pytest` (unit + integration) green;
|
||||
- `uv run pytest --cov=app --cov-report=term-missing` → `app/` > 90% (report the number in the commit body);
|
||||
- `uv run ruff check . && uv run pyright` clean;
|
||||
- `uv run pytest tests/e2e/test_kb_tree_lazy_load.py -v --no-cov` green in isolation + the six regression suites in isolation + `tests/e2e/test_smoke.py`;
|
||||
- `uv run python -m scripts.eval_retrieval --from-file tests/fixtures/retrieval_battery.txt` unchanged (retrieval is untouched — record it);
|
||||
- `AUDIT.md` (task 05) reflects the FINAL state after these fixes; `README.md` + `AGENTS.md` reviewed in full once and every inconsistency from this phase fixed (the tree endpoint table, the env var list, the deploy note about self-gzip);
|
||||
- UI Structure Check (AGENTS.md rule 5) for the drill: the level stays inside the 72rem container, the bounded scroll panel + sticky header survive a lazy render, the loading/alert lines are `role="status"`/`role="alert"`, the Retry button meets 44px + `:focus-visible`, contrast pairs are the existing AA tokens.
|
||||
6. Move `.agents/phases/todo/167_api_payload_diet/` → `.agents/phases/complete/` (with `AUDIT.md`) and commit with the `00_phase.md` message, `--no-gpg-sign`, one atomic commit (AGENTS.md rule 8).
|
||||
|
||||
## Testing & Quality
|
||||
- E2E: the new suite green in isolation + the six migrated/regression suites + `test_smoke.py` in isolation.
|
||||
- Coverage **>90%** on `app/` including `compression.py`, `http_cache.py`, `response_budget.py` and the new route/builder lines.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] Boot = one `depth=1` request whose body has `totals` and no deep children; drill = one node fetch per uncached level, zero on a cache hit; reload = 304.
|
||||
- [ ] Every level renders the same data the pre-phase full tree rendered (counts, dates, descriptions, pending markers, thumbnails) — proven by the migrated `test_kb_tree.py` claims, none dropped.
|
||||
- [ ] Failure paths are honest and recoverable (alert + Retry, level preserved, vanished folder resets the breadcrumb), and the agent's `ls` output is unchanged.
|
||||
- [ ] The measured numbers are in `AUDIT.md`, the coverage number is in the commit body, `uv run pytest` green, ruff + pyright clean, one atomic `--no-gpg-sign` commit, phase dir in `complete/`.
|
||||
Reference in New Issue
Block a user