brain-of-reese/.agents/phases/complete/96_oneshot_resilience/01_chat_retry.md

Task 01 — chat() retries an empty one-shot reply under the house retry policy

Phase: 96_oneshot_resilience · Story: n/a (incident-driven resilience — phase 67's owner-locked A1 left the one-shot path out of scope; this task closes that gap per owner request 2026-09-11).

Objective

LLMClient.chat() (the one-shot surface used by document summaries, the KB overview, folder summaries, and the sync probe) retries an empty-content reply up to BOR_LLM_RETRIES times with BOR_LLM_RETRY_DELAY between attempts, logs each retry, and only then raises — so a transient "the model answered but said nothing" reply (the 2026-09-11 incident: content="", finish_reason="length", the whole budget spent in reasoning_content) no longer loses a summary on its first attempt.

Work

1. app/rag/llm.py — LLMClient:
   • Extract chat()'s single-attempt body verbatim into a private _chat_once(self, messages, model) -> str: the existing try/except transport wrap (→ LLMError with the sanitized base URL), the choiceless check (…returned no choices), and the empty-content check (content None or not content.strip()). _chat_once must also surface the empty reply's finish_reason to its caller for the log line — e.g. raise a small internal signal or return it alongside; keep the public error messages exactly as they are today for the no-retry cases.
   • Rewrite chat() around it: N = 1 + self.settings.llm_retries total attempts. Attempt 1 → on the empty-content failure only: while attempts remain, logger.warning(…) with the model name, the empty reply's finish_reason, and attempt {n} of {N} (PLAN §9 ample logging — this line is the greppable record of the incident class), then await asyncio.sleep(self.settings.llm_retry_delay) (flat delay — the phase-67 convention), then the next attempt. Every other LLMError (transport, no-choices) propagates immediately — no app-level retry (the openai SDK's own max_retries=2 already re-POSTs wire-level failures; an app-level transport retry would stack on top of it).
   • Exhaustion: raise LLMError with the updated message f"…returned empty content on all {N} attempts — refusing to store a silent summary" (same base-URL sanitization as today). When llm_retries == 0, raise the current message verbatim (…returned empty content — refusing to store a silent summary) — the kill-switch must be byte-identical to pre-phase-96 behavior (house byte-identical convention).
   • Update chat()'s docstring: the phase-30 contract ("a silent empty summary must never be stored") stands; add the retry policy (D1/D2/D3 of 00_phase.md): which failures retry (empty content only), the knobs, the flat delay, the exhaustion message.
   • check_models (the "ping" probe) needs no change — it benefits automatically.
2. .env.example — the BOR_LLM_RETRIES / BOR_LLM_RETRY_DELAY comments gain that they now cover "the chat-turn stream (phase 67) and one-shot summary calls (phase 96)". No new settings, no app/config.py change (the validators for both knobs already exist from phase 67).

• ASSUMPTION: the retry loop lives inside chat() itself (one place, every consumer protected) — NOT in each caller; the generators' per-folder fail-soft semantics are untouched (they still catch the post-exhaustion LLMError exactly as today).
• ASSUMPTION: finish_reason availability — the empty reply is already parsed (resp.choices[0]); reading .finish_reason off it is free. If a provider omits it, log finish_reason=None (the line must never crash on the diagnostic path).

Testing & Quality

• Unit — tests/unit/test_llm_client.py (extend the existing _FakeCompletion/_make_chat_client harness; the fake completions object can be scripted to yield a sequence of replies per create() call):
  • empty-then-success: 2 attempts, exactly 1 sleep of llm_retry_delay, returns the second reply's trimmed content; the WARNING fired once (caplog).
  • all-empty with llm_retries=3 (default): 4 attempts, 3 sleeps, LLMError matching all 4 attempts; with a custom llm_retries=1: 2 attempts, message names 2.
  • first-attempt success: exactly 1 create() call, zero sleeps, return value byte-identical to today's behavior (the happy path is untouched).
  • choiceless reply: 1 attempt, no retry, the existing no choices error.
  • transport failure: 1 attempt, no retry, the existing wrapped error.
  • llm_retries=0: empty reply → 1 attempt, zero sleeps, the legacy message byte-identical (assert the exact string).
  • sleep value: record asyncio.sleep calls (monkeypatch) — flat llm_retry_delay each time, never a growing backoff.
  • update the two existing pins (test_chat_missing_content_raises_llm_error, test_chat_whitespace_only_content_raises_llm_error, ~L989–1000) to the new contract — either pin the exhaustion path with a low-retry setting or set llm_retries=0 there to keep the legacy-message assertion; both None and whitespace-only content must still refuse.
• Coverage: >90% on the modified app/rag/llm.py lines (uv run pytest --cov=app --cov-report=term-missing).
• No integration/E2E in this task (tasks 03/04 cover the paths end to end).

Completion Criteria

• uv run pytest tests/unit/test_llm_client.py -v green with the new pins above
• full suite green, coverage >90%, uv run ruff check . && uv run pyright clean
• the streaming path (chat_stream, chat_stream_retried) is byte-identical — no diff outside chat()/_chat_once in app/rag/llm.py
• .env.example comments updated; no new settings
• no behavior change in completed work (phase 67's retry suite green)