Files
ducoterra a49be80b8e
Build and Push Containers / build-and-push-app (push) Successful in 1m34s
Build and Push Containers / build-and-push-db (push) Successful in 10s
phase: 96_oneshot_resilience
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`.
2026-09-11 13:16:20 -04:00

17 KiB
Raw Permalink Blame History

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, content None/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 left chat() out ("admin/import paths with their own fail-fast behavior").
  • chat() consumers (all fail-soft per call, all protected once chat() retries): app/rag/summarizer.py L113 (document summaries — failure leaves documents.summary NULL + a summary_errors count), app/rag/overview.py L182 (KB overview — failure keeps the old row; its _overview_row_exists gate already self-heals a missing row on the next unchanged walk), app/rag/folder_summaries.py L267 via summarize_folder → generate_folder_summaries (per-folder fail-soft; no self-heal — the gap this phase fills), app/rag/llm.py L641 check_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 by LLMClient) 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) and app/api/sync.py _run_sync() (L276–281) — the folder-summary gate, one copy per path: changed = summary.added + summary.updated > 0; script path folders_due = changed or _folder_summaries_table_empty() (--limit skips entirely); API path if changed or folder_summary_table_empty(fs_db). folder_summary_table_empty (app/rag/folder_summaries.py) is the bounded LIMIT 1 probe.
  • 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, SimpleNamespace completions); existing pins test_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 .calls counter (the generator's unit pattern).
  • tests/integration/test_sync_folder_summaries.py + tests/integration/test_import_docs_overview.py — the change-gate integration pattern (fake LLMClient keyed on the mode marker; zero-call assertions on unchanged re-runs).
  • tests/e2e/mock_llm.py — compose_answer's FOLDER_SUMMARY_MODE branch (L1504) returns the deterministic one-liner Fixture folder summary for <folder>. from the user message's Folder: … header (imported FOLDER_HEADER_PREFIX — the mock can never drift from it); phase 67's failure-injection machinery: module-level _fail_posts counters + _bump_fail, keyed by trigger phrase, reset after the success they guard.
  • tests/e2e/test_ls_tree_drilldown.py (phase 94) — the scripted ls drill-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/sync pattern; 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 to chat() is a deliberate extension of that policy, not a deviation (the lock scoped what phase 67 shipped, it does not prohibit chat() from honoring the same knobs). .env.example comments 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 (content None or whitespace — the exact incident failure class). A choiceless reply still raises immediately (no retry), and transport failures keep today's behavior (the SDK's max_retries=2 already 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_retries empty attempts, chat() raises the same LLMError type with an updated message naming the attempts (…returned empty content on all {N} attempts — refusing to store a silent summary); with llm_retries=0 the 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_summaries gains only_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 runs only_missing=True — only the missing candidates are generated, existing rows stay byte-identical (text AND updated_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); no documents.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_exists gate 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-content LLMError only, while attempts remain: logger.warning (PLAN §9: model, finish_reason of the empty reply when available, attempt n of N) + await asyncio.sleep(settings.llm_retry_delay) (flat, phase 67) + retry via _chat_once. Any other LLMError propagates immediately.
  • N = 1 + settings.llm_retries total 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_reason for the log line (the incident's signature was finish_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_models benefits 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 with group_by_folder.
  • generate_folder_summaries(db, llm, *, skip=False, only_missing=False) — only_missing=True restricts the sorted candidate iteration to the keys from missing_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_empty is deleted in task 03 (both call sites replaced — the function becomes dead code; its unit tests move to missing_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 becomes folder_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 normal Fixture 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_llm conftest fixtures; admin login via tests/e2e/auth_helpers.py; sync via POST /api/sync per test_sync_button.py; the test server boots with BOR_LLM_RETRY_DELAY=0 per the phase-67 pattern; default BOR_LLM_RETRIES=3):
  1. Seed a temp local-dir source with three ≥ 2-doc folders: e2e_empty_once/, e2e_empty_always/, normal/.
  2. Sync #1 → a scripted ls <source> drill-down turn (the phase-94 echo pattern) asserts: the e2e_empty_once/ line carries : Fixture folder summary for <src>/e2e_empty_once. (the retry recovered the row — without task 01 it would be absent), the normal/ line does too, and the e2e_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).
  3. Delete the normal stored row directly (simulating a historical failure) → Sync #2 (KB unchanged) → the normal row is back with its deterministic text; the e2e_empty_once row's updated_at is unchanged (targeted fill — no full regeneration); e2e_empty_always remains absent (the gap-fill attempts it, exhausts, stays absent, sync still succeeds).

Tasks

  1. 01_chat_retry.md — chat() empty-content retry under BOR_LLM_RETRIES/BOR_LLM_RETRY_DELAY + logging + unit pins
  2. 02_gapfill_generator.md — missing_folder_summaries + generate_folder_summaries(only_missing=…) + unit pins
  3. 03_sync_gates.md — the gap gate in scripts/import_docs.py + app/api/sync.py (replacing the table-empty probe) + integration pins
  4. 04_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 + retries attempts + 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 — record asyncio.sleep); tests/unit/test_folder_summaries.py (missing_folder_summaries shapes: 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=False unchanged).
  • Integration: tests/integration/test_sync_folder_summaries.py + the test_import_docs_overview.py pattern (both sync paths: unchanged + gap → targeted fill of exactly the missing rows; unchanged + no gap → zero FOLDER_SUMMARY_MODE calls; changed → full regeneration as today; --limit skips).
  • 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 to BOR_LLM_RETRIES times with BOR_LLM_RETRY_DELAY between attempts, one WARNING per retry (model + finish_reason + attempt count), and succeeds when a later attempt returns content (unit-pinned)
  • an all-empty chat() raises LLMError naming the attempts after exactly 1 + BOR_LLM_RETRIES attempts; BOR_LLM_RETRIES=0 reproduces 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.py and POST /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 lite folder-summary calls (the phase-94 zero-burn invariant holds)
  • uv run pytest tests/e2e/test_oneshot_llm_retry.py -v --no-cov green in isolation: the e2e_empty_once folder summary is recovered by the retry (visible in the ls drill-down), e2e_empty_always stays 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 pytest green, coverage >90%, uv run ruff check . && uv run pyright clean
  • 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"