diff --git a/.agents/phases/todo/167_api_payload_diet/00_phase.md b/.agents/phases/todo/167_api_payload_diet/00_phase.md index 9d0e8b4..a468f87 100644 --- a/.agents/phases/todo/167_api_payload_diet/00_phase.md +++ b/.agents/phases/todo/167_api_payload_diet/00_phase.md @@ -2,7 +2,7 @@ **Source:** `TODO.md` L5 — "GET 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). +**Context:** `app/api/docs.py` (`GET /api/docs/tree` L919, the pure builder `build_kb_tree` L780 / `_level_children` L671 / `_folder_counts` L626 / `_subtree_max` L760, `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. @@ -27,7 +27,7 @@ Exact-path skip (never a prefix — `/api/chat-images` must still compress). Sta **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)`. +- `GET /api/docs/tree` gains `depth: int = Query(1, ge=1, le=8)`. **The default is what breaks the existing pins:** the full consumer list is exactly nine files (`grep -rl "docs/tree" tests app` → `tests/integration/test_docs_api.py`, `tests/integration/test_docs_api_dates.py`, `tests/unit/test_kb_tree_builder.py`, `tests/unit/test_kb_tree_ui.py`, `tests/unit/test_frontend_router.py`, `tests/e2e/test_kb_tree.py`, `tests/e2e/test_navbar_refresh.py`, `tests/e2e/test_document_dates.py`, `tests/e2e/test_git_source_dates.py`), and the TWO DATE SUITES walk straight through the tree with no depth param (`test_docs_api_dates.py` reaches `a` → `a/b` → deeper; `test_git_source_dates.py::_find_file` recurses by design) — they assert phase-106 D9 dates, not payload shape, so they migrate by requesting `?depth=8` explicitly with every assertion kept (task 02 item 9; the E2E one is named in task 06 item 4). - `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. @@ -57,8 +57,8 @@ It counts `http.response.body` chunk bytes as they pass (zero buffering, SSE-saf ## 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). +- Integration: depth + truncated + totals + node-route matrix (200/404/403), the migrated `tests/integration/test_docs_api_dates.py` (explicit `?depth=8`, every D9 date assertion intact), 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_git_source_dates.py` (its `_find_file` recurses — it needs the explicit `?depth=8`), `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 diff --git a/.agents/phases/todo/167_api_payload_diet/02_tree_depth_and_totals.md b/.agents/phases/todo/167_api_payload_diet/02_tree_depth_and_totals.md index 534a20a..307e8f4 100644 --- a/.agents/phases/todo/167_api_payload_diet/02_tree_depth_and_totals.md +++ b/.agents/phases/todo/167_api_payload_diet/02_tree_depth_and_totals.md @@ -15,6 +15,10 @@ The pure builder can stop at a depth budget: a node whose children were not buil 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). +9. Migrate the TWO date suites that consume the tree with no depth param (they break on the new default — the phase changes the default, so it owns the repair; every assertion stays, only the URL gains `?depth=8`): + - `tests/integration/test_docs_api_dates.py` (~L157): `admin_client.get("/api/docs/tree")` → `"/api/docs/tree?depth=8"`, keeping the whole `a` → `a/b` → deeper D9 walk (deep date beats shallow at every level) and the 0-document-source `updated_at is None` case; add a one-line comment naming phase 167 (the default is the view's first paint, `depth=8` is the whole tree). + - `tests/e2e/test_git_source_dates.py` (~L419): the `page.request.get(f"{app_url}/api/docs/tree")` call gains `?depth=8` so `_find_file` (which recurses by design) still reaches `old/…` and `recent/…`; the folder `updated_at` date assertions stay verbatim. + - Add the truncation-proof counterpart so the guarantee is tested BOTH ways: at the DEFAULT depth, a truncated folder node's `updated_at` still equals the deep max (this is exactly what the task-02 `max_dates` map buys — the date contract survives the payload diet), pinned in `test_docs_api_dates.py` and in `tests/unit/test_kb_tree_builder.py`. ## Testing & Quality - Unit + integration above; `uv run pytest tests/unit/test_kb_tree_builder.py tests/integration/test_docs_api.py -v` green. diff --git a/.agents/phases/todo/167_api_payload_diet/06_e2e_payload_diet.md b/.agents/phases/todo/167_api_payload_diet/06_e2e_payload_diet.md index baa939c..0c02408 100644 --- a/.agents/phases/todo/167_api_payload_diet/06_e2e_payload_diet.md +++ b/.agents/phases/todo/167_api_payload_diet/06_e2e_payload_diet.md @@ -17,7 +17,7 @@ An isolated Playwright suite proves the owner-visible outcome — a small first - **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. +4. `tests/e2e/test_kb_tree_nav.py`, `test_document_dates.py`, `test_git_source_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 condition assumed the full tree loaded (e.g. a test that expects a deep folder's rows before drilling — add the drill step; a test that walks the tree JSON — give it `?depth=8`, the repair task 02 item 9 defines), 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);