All checks complete. Final report: **Phase 96 (one-shot resilience) — final verification pass, all green** (all 4 task files already in `complete/`; verified the working-tree implementation against the design) - `LLMClient.chat()` empty-content retry (D1–D3) via `_chat_once` + `_EmptyContentError` (carries `finish_reason`), under `BOR_LLM_RETRIES`/`BOR_LLM_RETRY_DELAY` — verified in diff - `missing_folder_summaries()` + `generate_folder_summaries(only_missing=…)` — verified; `folder_summary_table_empty` deleted, both sync gates switched to the gap probe - `.env.example` comments updated (chat-turn stream + one-shot summary calls) **Test / lint / coverage results** - `uv run pytest --cov=app --cov-report=term-missing` → **1988 passed**, coverage **99%** (gate >90%) - `uv run pytest tests/e2e/test_oneshot_llm_retry.py -v --no-cov` → **2 passed** (isolated) - Regressions, each isolated: `test_ls_tree_drilldown` 3 passed, `test_sync_button` 3 passed, `test_local_directory_sources` 3 passed, `test_llm_retry` 4 passed - `uv run ruff check . && uv run pyright` → clean (0 errors) **Completion criteria:** retry-then-recover unit-pinned ✓ · exhaustion + `BOR_LLM_RETRIES=0` byte-identical ✓ · streaming path untouched ✓ · gap-fill both sync paths, other rows byte-identical incl. `updated_at` ✓ · no-gap zero-burn ✓ · phase E2E green ✓ · regression E2Es green ✓ · full suite + >90% + lint/types ✓ · no completed-phase behavior change (full suite green) ✓. Commit left to the harness per executor rules (working tree, 16 files). **Deviations:** none. **Next pending phase:** `97_kb_tree_catalog`.
17 KiB
Phase 96 — One-shot LLM resilience: retry empty replies, self-heal missing folder summaries
Source: 2026-09-11 incident (owner chat) — the then-lite model intermittently returned empty completions: its internal monologue (the wire's reasoning_content field) consumed the entire max_tokens=2048 budget, so the reply arrived as content="" + finish_reason="length". Measured 2 empties in 90 folder-summary calls on the deploy source sync (≈2–7% per call in follow-up probes), losing two stored rows: deploy/Deployments (283 docs) and deploy/Deployments/stackexpected/website/app (7 docs). The app handled it exactly as designed (LLMClient.chat refuses to store a silent summary; generate_folder_summaries is per-folder fail-soft) — but nothing retried, and nothing ever filled the hole: the folder-summary gate (changed KB or empty table) skips regeneration on unchanged re-syncs, so missing rows persist until a KB change. The owner replaced the lite model server-side (it no longer emits reasoning_content — verified live: 16/16 clean, no reasoning_content field); this phase removes the remaining app-side exposure so ANY future transient empty reply from ANY one-shot consumer cannot silently lose data.
Story: n/a (incident-driven resilience — one Playwright file per phase, run in isolation, A16).
Context:
app/rag/llm.py—LLMClient.chat()(~L335–378) is the one-shot surface: transport failure →LLMError(wrapped), choiceless reply →LLMError,contentNone/blank →LLMError("…returned empty content — refusing to store a silent summary"). No retry. Phase 67's retry machinery (BOR_LLM_RETRIES=3 /BOR_LLM_RETRY_DELAY=5 s, flat;chat_stream_retried,RetryPiece) covers the streaming chat-turn path only — phase 67's owner-locked A1 explicitly scoped it to the chat turn and leftchat()out ("admin/import paths with their own fail-fast behavior").chat()consumers (all fail-soft per call, all protected oncechat()retries):app/rag/summarizer.pyL113 (document summaries — failure leavesdocuments.summaryNULL + asummary_errorscount),app/rag/overview.pyL182 (KB overview — failure keeps the old row; its_overview_row_existsgate already self-heals a missing row on the next unchanged walk),app/rag/folder_summaries.pyL267 viasummarize_folder→generate_folder_summaries(per-folder fail-soft; no self-heal — the gap this phase fills),app/rag/llm.pyL641check_models(the sync-time "ping" probe).app/config.py—llm_retries: int = 3,llm_retry_delay: float = 5(phase 67; validators already enforce the house kill-switch pattern: 0 disables, negative fails startup).- The openai SDK's own
max_retries=2(kept byLLMClient) already re-POSTs wire-level 500s — the gap is the semantic failure: the endpoint is healthy, answers, and says nothing. scripts/import_docs.py_run()(~L335–360) andapp/api/sync.py_run_sync()(L276–281) — the folder-summary gate, one copy per path:changed = summary.added + summary.updated > 0; script pathfolders_due = changed or _folder_summaries_table_empty()(--limitskips entirely); API pathif changed or folder_summary_table_empty(fs_db).folder_summary_table_empty(app/rag/folder_summaries.py) is the boundedLIMIT 1probe.app/rag/folder_summaries.py—group_by_folder(the one recursive-subtree concept),MIN_DOCS_PER_FOLDER = 2,generate_folder_summaries(sorted candidate iteration, per-folder fail-soft upsert, then a prune pass deleting rows whose folder lost ≥ 2 docs; flushes only — the sync path owns the transaction).tests/unit/test_llm_client.py— the fake-client harness (_FakeCompletion,_make_chat_client,SimpleNamespacecompletions); existing pinstest_chat_missing_content_raises_llm_error/test_chat_whitespace_only_content_raises_llm_error(~L989–1000) match"empty content".tests/unit/test_folder_summaries.py— the duck-typed fake LLM with a.callscounter (the generator's unit pattern).tests/integration/test_sync_folder_summaries.py+tests/integration/test_import_docs_overview.py— the change-gate integration pattern (fakeLLMClientkeyed on the mode marker; zero-call assertions on unchanged re-runs).tests/e2e/mock_llm.py—compose_answer'sFOLDER_SUMMARY_MODEbranch (L1504) returns the deterministic one-linerFixture folder summary for <folder>.from the user message'sFolder: …header (importedFOLDER_HEADER_PREFIX— the mock can never drift from it); phase 67's failure-injection machinery: module-level_fail_postscounters +_bump_fail, keyed by trigger phrase, reset after the success they guard.tests/e2e/test_ls_tree_drilldown.py(phase 94) — the scriptedlsdrill-down turn pattern (the mock echoes the tool result into its grounded answer — the E2E's only lens on the LLM context) and the temp-local-source seed +POST /api/syncpattern;tests/e2e/test_llm_retry.py(phase 67) — the test-server env-override pattern (BOR_LLM_RETRY_DELAY=0).
Objective
A transient empty one-shot LLM reply can no longer silently lose data: LLMClient.chat() retries an empty-content reply under the existing BOR_LLM_RETRIES/BOR_LLM_RETRY_DELAY policy (logged per attempt) before raising, and an unchanged-KB sync detects folder-summary gaps (candidate folders with no stored row) and fills only the missing rows — so a failed folder summary self-heals on the next sync instead of persisting until a KB change.
Dependencies
67_llm_retry(complete) — the retry policy knobs (BOR_LLM_RETRIES/BOR_LLM_RETRY_DELAY), the logging/kill-switch conventions, and the mock's failure-injection pattern.94_ls_tree_drilldown(complete) —folder_summaries,generate_folder_summaries, the change-gate, the E2E drill-down + sync patterns.
Decisions recorded here (no PLAN.md change needed — no new technology, no new env vars, no new SSE events)
- D1 — knob reuse: the one-shot path honors the existing
BOR_LLM_RETRIES(default 3) /BOR_LLM_RETRY_DELAY(default 5 s) — no new settings. Phase 67's owner-locked A3 named these the house LLM-retry policy; extending them tochat()is a deliberate extension of that policy, not a deviation (the lock scoped what phase 67 shipped, it does not prohibitchat()from honoring the same knobs)..env.examplecomments are updated to say so. (If the owner prefers a separate one-shot knob, task 01 is a one-line change.) - D2 — retry condition:
chat()retries empty content only (contentNone or whitespace — the exact incident failure class). A choiceless reply still raises immediately (no retry), and transport failures keep today's behavior (the SDK'smax_retries=2already covers wire-level 500s; an app-level transport retry would double-stack the SDK policy and multiply the 300 s timeout). - D3 — exhaustion contract: after
1 + llm_retriesempty attempts,chat()raises the sameLLMErrortype with an updated message naming the attempts (…returned empty content on all {N} attempts — refusing to store a silent summary); withllm_retries=0the message stays byte-identical to today's (the feature is fully off — phase 67 kill-switch convention). All consumers' fail-soft behavior is untouched (document summary → NULL + count, overview → old row, folder → skip + old row, probe → sync fails loudly as today). - D4 — targeted gap-fill:
generate_folder_summariesgainsonly_missing=False. The changed-KB path is unchanged (full regeneration + prune). The unchanged-KB gate changes from "table empty?" to "any candidate row missing?" (subsumes the table-empty first-run case exactly); when a gap exists it runsonly_missing=True— only the missing candidates are generated, existing rows stay byte-identical (text ANDupdated_at), and the prune pass still runs (a no-op on an unchanged KB, the invariant kept).--limit(script only) still skips generation entirely. - Non-goals: the streaming path (
chat_stream/chat_stream_retried) is untouched (phase 67 territory); nodocuments.summary-NULL catch-up (the retry eliminates the failure class — expected per-doc loss p⁴; the importer's unchanged-skip would be a separate, larger change); no KB-overview catch-up (its_overview_row_existsgate already self-heals); no UI/SSE/sync-status surface change (folder stats stay log-only, phase 94 contract); no aipi/model-side change (owner already done).
Design (shared by all tasks — the executor reads this, not the chat)
The one-shot retry (task 01)
app/rag/llm.py LLMClient:
- Extract the single-attempt body of
chat()into a private_chat_once(messages, model) -> str(the existing try/except transport wrap + choiceless check + empty-content check, verbatim behavior).chat()becomes: attempt 1 via_chat_once; on the empty-contentLLMErroronly, while attempts remain:logger.warning(PLAN §9: model,finish_reasonof the empty reply when available,attempt n of N) +await asyncio.sleep(settings.llm_retry_delay)(flat, phase 67) + retry via_chat_once. Any otherLLMErrorpropagates immediately. N = 1 + settings.llm_retriestotal attempts. Exhaustion → the D3 message.llm_retries=0→ one attempt, the byte-identical legacy message.- The empty-content check must capture the reply's
finish_reasonfor the log line (the incident's signature wasfinish_reason="length"— it makes the warning greppable and self-explaining). chat()docstring: the phase-30 "a silent empty summary must never be stored" contract stays, extended with the retry policy (D1/D2/D3)..env.example: the two phase-67 comments gain "(chat-turn stream and one-shot summary calls)".check_modelsbenefits automatically (no change).
The gap detector + targeted fill (task 02)
app/rag/folder_summaries.py:
missing_folder_summaries(db) -> list[tuple[str, str]]— the candidate folders (the generator's exact candidate computation: one catalog query ordered by(source, path),group_by_folder, subtrees with ≥MIN_DOCS_PER_FOLDER) that have no stored row, sorted by(source, folder_path). The gate probe and the fill both use this one function — one concept, as withgroup_by_folder.generate_folder_summaries(db, llm, *, skip=False, only_missing=False)—only_missing=Truerestricts the sorted candidate iteration to the keys frommissing_folder_summaries(db)(computed from the SAME catalog pass — no second full query inside the generator: derive the missing set from the candidates already grouped). Upsert, fail-soft, and the prune pass are otherwise unchanged; stats dict shape unchanged (the caller logs the mode).folder_summary_table_emptyis deleted in task 03 (both call sites replaced — the function becomes dead code; its unit tests move tomissing_folder_summaries).
The sync gates (task 03)
Both paths, identical shape (the phase-94 "one gate, two call sites" convention):
- Changed KB →
generate_folder_summaries(session, llm)— full regeneration (today's behavior, byte-identical). - Unchanged KB →
missing = missing_folder_summaries(session); non-empty →generate_folder_summaries(session, llm, only_missing=True)+ a log line naming the gap-fill (script: the summary line token becomesfolder_summaries=<g>/<f>/<p> (gap-fill); API:sync: folder_summaries gap-fill stats=…); empty → today's skip log. --limit(script) → unchanged full skip. Transaction convention unchanged (generator flushes, the path commits).
The E2E (task 04)
tests/e2e/mock_llm.py — the FOLDER_SUMMARY_MODE branch gains the incident-shape injection, keyed by the folder LABEL (the seeded KB's folder names carry the trigger — deterministic, no env, no request-order state beyond the phase-67 per-key counter, reset after the success it guards):
- label ends with
/e2e_empty_once→ the FIRST non-stream POST for that label returns the incident envelope: same OpenAI shape,message.content="",finish_reason="length"; later POSTs return the normalFixture folder summary for <folder>.line. - label ends with
/e2e_empty_always→ every non-stream POST for that label returns the empty envelope.tests/e2e/test_oneshot_llm_retry.py(new;app_server+mock_llmconftest fixtures; admin login viatests/e2e/auth_helpers.py; sync viaPOST /api/syncpertest_sync_button.py; the test server boots withBOR_LLM_RETRY_DELAY=0per the phase-67 pattern; defaultBOR_LLM_RETRIES=3):
- Seed a temp local-dir source with three ≥ 2-doc folders:
e2e_empty_once/,e2e_empty_always/,normal/. - Sync #1 → a scripted
ls <source>drill-down turn (the phase-94 echo pattern) asserts: thee2e_empty_once/line carries: Fixture folder summary for <src>/e2e_empty_once.(the retry recovered the row — without task 01 it would be absent), thenormal/line does too, and thee2e_empty_always/line has NO: …suffix (exhausted → fail-soft → absent) while the sync still reports success (the folder-stats failure never flips the run, phase 94). - Delete the
normalstored row directly (simulating a historical failure) → Sync #2 (KB unchanged) → thenormalrow is back with its deterministic text; thee2e_empty_oncerow'supdated_atis unchanged (targeted fill — no full regeneration);e2e_empty_alwaysremains absent (the gap-fill attempts it, exhausts, stays absent, sync still succeeds).
Tasks
01_chat_retry.md—chat()empty-content retry underBOR_LLM_RETRIES/BOR_LLM_RETRY_DELAY+ logging + unit pins02_gapfill_generator.md—missing_folder_summaries+generate_folder_summaries(only_missing=…)+ unit pins03_sync_gates.md— the gap gate inscripts/import_docs.py+app/api/sync.py(replacing the table-empty probe) + integration pins04_e2e_oneshot_retry.md— the mock's incident-shape injection +tests/e2e/test_oneshot_llm_retry.py+ regressions + commit
Testing & Quality
- Unit:
tests/unit/test_llm_client.py(retry matrix: empty-then-success = 2 attempts + 1 sleep; all-empty =1 + retriesattempts + the exhaustion message; first-success = 1 attempt, 0 sleeps, byte-identical return; choiceless reply = no retry, immediate error; transport error = no app-level retry;llm_retries=0= legacy byte-identical message; sleep value honored — recordasyncio.sleep);tests/unit/test_folder_summaries.py(missing_folder_summariesshapes: fresh table → all candidates, full table → empty, one row deleted → that folder;only_missing=True→ only missing keys generated, existing rows byte-identical incl.updated_at, zero LLM calls when there is no gap, prune still runs;only_missing=Falseunchanged). - Integration:
tests/integration/test_sync_folder_summaries.py+ thetest_import_docs_overview.pypattern (both sync paths: unchanged + gap → targeted fill of exactly the missing rows; unchanged + no gap → zeroFOLDER_SUMMARY_MODEcalls; changed → full regeneration as today;--limitskips). - E2E (mandatory, A16):
tests/e2e/test_oneshot_llm_retry.py, in isolation, deterministic mock,BOR_LLM_RETRY_DELAY=0. - Coverage: >90% on
app/(uv run pytest --cov=app --cov-report=term-missing). - Lint/types:
uv run ruff check . && uv run pyright.
Completion Criteria
- a one-shot
chat()call whose first reply is empty content (None or whitespace) is retried up toBOR_LLM_RETRIEStimes withBOR_LLM_RETRY_DELAYbetween attempts, oneWARNINGper retry (model + finish_reason + attempt count), and succeeds when a later attempt returns content (unit-pinned) - an all-empty
chat()raisesLLMErrornaming the attempts after exactly1 + BOR_LLM_RETRIESattempts;BOR_LLM_RETRIES=0reproduces today's single-attempt behavior and message byte-identically - the streaming path is untouched:
test_llm_retry.py(phase 67) green in isolation - an unchanged-KB sync with a missing folder-summary row regenerates exactly that row (both
scripts/import_docs.pyandPOST /api/sync), leaves every other row byte-identical (incl.updated_at), and logs the gap-fill (integration-pinned) - an unchanged-KB sync with no gap burns zero
litefolder-summary calls (the phase-94 zero-burn invariant holds) uv run pytest tests/e2e/test_oneshot_llm_retry.py -v --no-covgreen in isolation: thee2e_empty_oncefolder summary is recovered by the retry (visible in thelsdrill-down),e2e_empty_alwaysstays absent with the sync green, and the deleted-row gap self-heals on the next unchanged sync- regression E2E green in isolation:
test_ls_tree_drilldown.py,test_sync_button.py,test_local_directory_sources.py,test_llm_retry.py uv run pytestgreen, coverage >90%,uv run ruff check . && uv run pyrightclean- no behavior change in completed phases; one atomic Conventional Commit,
--no-gpg-sign
Commit
git add -A .agents/ app/ scripts/ tests/ && git commit --no-gpg-sign -m "fix(rag): retry empty one-shot LLM replies and self-heal missing folder summaries on unchanged syncs"