chore(agent): phase roadmap from TODO.md — 67_llm_retry + 68_search_tool

This commit is contained in:
2026-09-01 20:15:08 -04:00
parent 66419bf652
commit cddc84c7db
10 changed files with 419 additions and 0 deletions
@@ -0,0 +1,51 @@
# Phase 67 — LLM Retry with Live "Trying Again" Feedback
**Source:** `TODO.md` L3 — "Add a .env configurable retry in case the LLM server fails to respond. Allow 3 retries by default, with 5 seconds between each retry. Update the user interface to show 'communication interrupted, trying again' or something like that if the LLM server stops communicating."
**Story:** n/a (TODO-derived — owner roadmap confirmation 2026-09-01)
**Context:**
- `app/rag/llm.py` — `LLMClient.chat_stream` is the single streaming surface (deflected turns + every agent round); `LLMError` is the typed failure the API layer already turns into an SSE `error` frame (`app/api/chat.py` — `except LLMError` around the piece loop; `EmbeddingError` around the pre-stream `llm.embed_one(request.message)`).
- `app/rag/agent.py` — `run_agent` issues one `chat_stream` per round (plus a final `tools=None` call at the round cap); the phase-48 teardown binds each stream and closes it in a `finally`.
- `app/config.py` — the `BOR_*` settings block (LLM section: `llm_base_url`, `llm_chat_model`, …); `agent_max_rounds` shows the house pattern for a tunable with a startup validator.
- `app/schemas.py` — the SSE event family (`ChatThinkingEvent`, `ChatToolEvent`, `ChatDoneEvent`, `ChatErrorEvent`).
- `frontend/assets/app.js` — `runTurn`'s `readSSE` callback is the event state machine; the `tool` branch is the house pattern for a server-driven STATUS change (`#send-status` + typing-indicator `aria-label`, no new bubble, `clearTurnTimeout()` because a frame arrived). `tests/e2e/test_agent_document_tools.py` (L236+) records every `#send-status` value during a turn for assertions.
- `tests/e2e/mock_llm.py` — deterministic marker-driven OpenAI-compatible stand-in (chat + embeddings), run as a uvicorn subprocess by `tests/e2e/conftest.py`; the marker flow is discriminated statelessly from the request.
- **Not in scope (owner-locked A1):** the one-shot `LLMClient.chat()` path (document summaries, KB overview) and the sync probe (`check_models`) — those are admin/import paths with their own fail-fast behavior (phase 41) and no live user to notify.
## Objective
When the aipi endpoint dies mid-turn, the app retries the LLM request automatically — `.env`-tunable, **3 retries / 5 s delay by default** — and the UI tells the user what is happening ("Communication interrupted — retrying (n of N)…") instead of the turn dead-ending in an error banner. A retry only ever restarts a request that has **not yet streamed a single output frame** to the client (locked A2), so no answer token is ever duplicated.
## Dependencies
- `66_history_auto_save_copy` (todo, preceding — no functional dependency; ordering by number)
## Tasks
1. `01_config_and_retry_primitive.md` — `BOR_LLM_RETRIES` / `BOR_LLM_RETRY_DELAY` settings + the `RetryPiece` + `chat_stream_retried()` primitive in `app/rag/llm.py`.
2. `02_chat_endpoint_retry.md` — the `retry` SSE event, the embedding retry loop, and the deflected-stream retry in `app/api/chat.py`.
3. `03_agent_round_retry.md` — per-round retries inside `run_agent` (loop rounds + final no-tools call).
4. `04_frontend_retry_status.md` — the `retry` branch in `runTurn`'s SSE handler: the live "retrying (n of N)…" status.
5. `05_e2e_and_commit.md` — `mock_llm.py` failure injection, `tests/e2e/test_llm_retry.py`, regressions, commit.
## Testing & Quality
- Unit: `tests/unit/test_config.py` (new vars, defaults, validators), `tests/unit/test_llm_client.py` (`chat_stream_retried` semantics — retry only before the first piece, `RetryPiece` ordering, exhaustion, `retries=0`), `tests/unit/test_agent.py` (per-round retry), `tests/unit/test_frontend_tool_states.py` pattern (JS pins for the `retry` branch).
- Integration: `tests/integration/test_chat_api.py` — SSE frame ordering (embed-fail → `retry` frame → completed turn; embed-exhausted → `retry` frames + terminal `error` frame; mid-stream failure AFTER a delta → no retry, `error` frame).
- E2E (mandatory, house rule): `tests/e2e/test_llm_retry.py`, run in isolation (mock LLM with deterministic failure injection; `BOR_LLM_RETRY_DELAY=0` on the test server so the suite stays fast).
- Coverage: **>90%** on `app/` (validate.sh gate).
## Completion Criteria
- [ ] `BOR_LLM_RETRIES` (default 3) and `BOR_LLM_RETRY_DELAY` (default 5 s) are honored end to end and documented in `.env.example`.
- [ ] A dead-then-recovered endpoint: the turn completes with a normal answer and the UI showed the "retrying" status while waiting; a dead endpoint: after N attempts the existing terminal error banner appears.
- [ ] A stream failure after the first output frame still terminates with the `error` event — no retry, no duplicated tokens.
- [ ] `uv run pytest` green; coverage TOTAL >90%; `uv run ruff check . && uv run pyright` clean.
- [ ] `uv run pytest tests/e2e/test_llm_retry.py -v --no-cov` green in isolation (DB up).
- [ ] Regression E2E suites green in isolation: `test_chat_rag.py`, `test_agent_document_tools.py`, `test_stop_generation.py`, `test_retry_answer.py`.
- [ ] One `--no-gpg-sign` commit; phase dir moved to `.agent/phases/complete/`.
## Locked decisions
- **Owner-locked (2026-09-01, roadmap confirmation, A1):** scope is the **chat turn only** — question embedding + the answer stream (deflected path and every agent round). `LLMClient.chat()` (summaries, KB overview) and `check_models` (sync probe) are untouched.
- **Owner-locked (2026-09-01, roadmap confirmation, A2):** a retry restarts the LLM request **only if no output frame has been streamed to the client yet** for that request (no thinking/tool/delta emitted). Once tokens are flowing, the failure stays terminal (the existing `error` frame) — a partial answer is never redone.
- **Owner-locked (2026-09-01, roadmap confirmation, A3):** env names `BOR_LLM_RETRIES` (int, default **3**) and `BOR_LLM_RETRY_DELAY` (seconds, default **5**) — a flat delay between attempts, no exponential backoff (the TODO specifies a fixed 5 s).
- **Owner-locked (2026-09-01, roadmap confirmation, A4):** UI copy — `#send-status` reads `Communication interrupted — retrying (n of N)…` (n = current attempt, N = the configured retry count) on the existing status line; no new banner, no bubble.
## Commit
```bash
git add -A .agent/ app/ tests/ frontend/ && git commit --no-gpg-sign -m "feat(rag): retry a failed LLM request before the first token lands — BOR_LLM_RETRIES/BOR_LLM_RETRY_DELAY with a live 'retrying' status"
```