feat(rag): retry a failed LLM request before the first token lands — BOR_LLM_RETRIES/BOR_LLM_RETRY_DELAY with a live 'retrying' status
This commit is contained in:
@@ -1,45 +0,0 @@
|
||||
# Phase 66 — History Tab Copy: Auto-Save, No Save Button
|
||||
|
||||
**Source:** `TODO.md` L4 — "Update the history tab - the text still reads like there's a save button. There isn't anymore, every chat is saved."
|
||||
**Story:** n/a (TODO-derived — owner roadmap confirmation 2026-09-01)
|
||||
**Context:** Verified copy inventory (audited 2026-09-01, line numbers as of this writing):
|
||||
- `frontend/history.html` L6 — `<meta name="description" content="Saved chats — every conversation you saved, one click back.">` ("you saved" implies the manual action).
|
||||
- `frontend/history.html` L106–109 — the `.page-sub`: "Every conversation you pressed <strong>Save</strong> on — newest activity first. Click a title to return to that chat." — the primary offender: it names a Save button that has not existed since phase 55 (owner-locked A2, 2026-08-31: every conversation auto-saves via `app.js` `persistConversation`; pinned by `test_save_share_ux.py::test_anonymous_auto_save`).
|
||||
- `frontend/history.html` L163 — the empty-table row (inside `#history-empty-row`, `hidden` until `frontend/assets/history.js` reveals it on a 0-row fetch): "No saved chats yet — finish a conversation and press <strong>Save</strong> in the chat."
|
||||
- STAYS AS-IS (verified accurate — "saved" as a STATE, not a button): the `<h1>Saved chats</h1>` (L105), the anonymous gate copy (L123–128: "Sign in to view your saved chats" / "Saved conversations are admin-only…"), the table caption, and the `history.js` aria-labels ("Delete saved chat: …", "Unshare saved chat: …").
|
||||
- No existing test pins any of the three offending strings (verified: "pressed Save" / "press Save" / "every conversation you saved" appear nowhere in `tests/`).
|
||||
- `.agent/` is tracked and committed (AGENTS.md rule 8) — the phase commit stages `.agent/` + `frontend/` + `tests/`.
|
||||
|
||||
## Objective
|
||||
Every user-visible string on the History tab describes the app as it works TODAY: conversations save themselves automatically — there is no Save button to press. The exact replacement copy is locked below.
|
||||
|
||||
## Dependencies
|
||||
- `65_bottom_chat_actions` (todo, preceding — no functional dependency; ordering by number)
|
||||
|
||||
## Tasks
|
||||
1. `01_history_page_copy.md` — rewrite the three strings in `frontend/history.html` (meta description, page-sub, empty row) to the locked auto-save copy.
|
||||
2. `02_tests_and_commit.md` — unit text pins + dedicated E2E `tests/e2e/test_history_copy.py` (run in isolation) + regression pass + one atomic commit.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit pins (house style, `tests/unit/test_stale_ui_copy.py` pattern — read `frontend/history.html` as text): the old literals are GONE, the locked replacements are present.
|
||||
- E2E (mandatory, house rule): `tests/e2e/test_history_copy.py`, run in isolation against the shared conftest server (DB up) — an admin sees the ACTUAL page (gate hidden, table rendered): the rendered page-sub and empty-row texts match the locked copy, and no rendered page text reads like a Save button exists.
|
||||
- Coverage: **>90%** on `app/` (validate.sh gate — no `app/` change, the gate stays green at its current level).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `frontend/history.html` contains none of: "you pressed <strong>Save</strong>", "press <strong>Save</strong>", "every conversation you saved" — and carries the three locked (A3) strings.
|
||||
- [ ] The rendered History page (admin, empty table) shows the locked page-sub and empty-row text; the `<h1>Saved chats</h1>` and the gate copy are untouched.
|
||||
- [ ] `uv run pytest` green; coverage TOTAL >90%; `uv run ruff check . && uv run pyright` clean.
|
||||
- [ ] `uv run pytest tests/e2e/test_history_copy.py -v --no-cov` green in isolation (DB up).
|
||||
- [ ] Regression E2E suites green in isolation: `test_chat_history.py`, `test_stale_saved_chats.py`.
|
||||
- [ ] One `--no-gpg-sign` commit; phase dir moved to `.agent/phases/complete/`.
|
||||
|
||||
## Locked decisions
|
||||
- **Owner-locked (2026-09-01, roadmap confirmation, A3):** exact replacement copy —
|
||||
- meta description (`history.html` L6): `Saved chats — every conversation is saved automatically, one click back.`
|
||||
- page-sub (`history.html` L106–109): `Every conversation is saved automatically — newest activity first. Click a title to return to that chat.` (the `<strong>Save</strong>` emphasis is retired with the button — no inline emphasis in the replacement)
|
||||
- empty row (`history.html` L163): `No saved chats yet — start a conversation and it will be saved automatically.`
|
||||
|
||||
## Commit
|
||||
```bash
|
||||
git add .agent/ frontend/ tests/ && git commit --no-gpg-sign -m "fix(web): history tab copy — every chat saves automatically, there is no Save button"
|
||||
```
|
||||
@@ -1,41 +0,0 @@
|
||||
# Task 02 — Unit pins + dedicated E2E + regression pass + atomic commit
|
||||
|
||||
**Phase:** `66_history_auto_save_copy` · **Source:** `TODO.md:4` — "Update the history tab - the text still reads like there's a save button. There isn't anymore, every chat is saved."
|
||||
**Story:** n/a (TODO-derived)
|
||||
|
||||
## Objective
|
||||
The new auto-save copy is pinned at two layers so it can never silently regress — unit text pins (no browser, house style) and one dedicated Playwright suite asserting what a signed-in admin actually SEES on the History tab — then one atomic commit.
|
||||
|
||||
## Work
|
||||
1. `tests/unit/test_history_copy.py` (NEW — house pattern `tests/unit/test_stale_ui_copy.py`: read `frontend/history.html` as text, assert substrings):
|
||||
- Negative pins (GONE): `"you pressed <strong>Save</strong>"`, `"press <strong>Save</strong> in the chat"`, `"every conversation you saved"`.
|
||||
- Positive pins (PRESENT, exactly once each): the three locked (A3) strings — `Saved chats — every conversation is saved automatically, one click back.`, `Every conversation is saved automatically — newest activity first. Click a title to return to that chat.`, `No saved chats yet — start a conversation and it will be saved automatically.`
|
||||
- State-language survivors (PRESENT, proving no over-deletion): `<h1>Saved chats</h1>`, `Sign in to view your saved chats`, `Saved conversations are admin-only`.
|
||||
2. `tests/e2e/test_history_copy.py` (NEW — one phase, one file, run in isolation, DB up; shared conftest server, default env):
|
||||
- Setup (house patterns from `tests/e2e/test_chat_history.py`): `_reset_db` with the TRUNCATE extended to include `saved_chats` (so the table is genuinely empty), `login(page, app_url, next="/history.html")` from `tests/e2e/auth_helpers.py`.
|
||||
- Admin view of an empty History tab:
|
||||
- the anonymous gate is hidden (`#history-gate` hidden — the login worked);
|
||||
- `#history-empty-row` is revealed (visible) and its textContent == the locked (A3) empty-row string;
|
||||
- the `.page-sub` innerText == the locked (A3) page-sub string (whitespace-normalized compare);
|
||||
- `document.querySelector('meta[name="description"]').content` == the locked (A3) meta string;
|
||||
- the no-button contract: `document.body.innerText` matches NEITHER `/press(ed)?\s+save/i` NOR `/save\s+button/i` — while the `<h1>` still reads "Saved chats" (asserted present, so the scan cannot pass by deleting the heading);
|
||||
- no `#save-chat-btn` anywhere (phase 55 A2 — the control is gone from the app, not just the copy).
|
||||
- Determinism: all assertions are settled-state (static HTML + the one 0-row fetch); Playwright `expect` retries ride out the table load.
|
||||
3. Regression E2E (each in isolation, DB up): `test_chat_history.py` (the row actions + restore flow on this page), `test_stale_saved_chats.py` (the stale pills + regenerate flow).
|
||||
4. One atomic commit (`.agent/` tracked — AGENTS.md rule 8):
|
||||
```bash
|
||||
git add .agent/ frontend/ tests/ && git commit --no-gpg-sign -m "fix(web): history tab copy — every chat saves automatically, there is no Save button"
|
||||
```
|
||||
Commit body: the verification read-through outcome from task 01 (the three replacements; h1/gate/history.js untouched) — one line each.
|
||||
|
||||
## Testing & Quality
|
||||
- `uv run pytest` green (unit + integration); coverage TOTAL **>90%** (validate.sh gate — no `app/` change).
|
||||
- `uv run pytest tests/e2e/test_history_copy.py -v --no-cov` green in isolation (DB up) — run it after the unit pins pass.
|
||||
- `uv run ruff check . && uv run pyright` clean (the new test files included).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `tests/unit/test_history_copy.py` + `tests/e2e/test_history_copy.py` exist and are green (E2E in isolation, DB up).
|
||||
- [ ] The two regression E2E suites are green in isolation.
|
||||
- [ ] Full suite green, coverage >90%, lint + types clean.
|
||||
- [ ] Exactly one new commit on `main`, staging `.agent/ frontend/ tests/` only, `--no-gpg-sign`.
|
||||
- [ ] Phase dir moved to `.agent/phases/complete/66_history_auto_save_copy/` (`.agent/` stays tracked and committed).
|
||||
@@ -1,44 +0,0 @@
|
||||
# Task 01 — Retry Settings + the `chat_stream_retried` Primitive
|
||||
|
||||
**Phase:** `67_llm_retry` · **Source:** `TODO.md:3` — "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)
|
||||
|
||||
## Objective
|
||||
The two `.env` knobs (`BOR_LLM_RETRIES=3`, `BOR_LLM_RETRY_DELAY=5`) and one shared streaming primitive — `chat_stream_retried()` in `app/rag/llm.py` — that the chat endpoint (task 02) and the agent loop (task 03) both build on. The primitive is the ONLY place the retry-before-first-piece rule (locked A2) lives.
|
||||
|
||||
## Work
|
||||
1. `app/config.py` — in the `--- LLM ---` settings block (next to `llm_chat_model` / `stream_thinking`), add:
|
||||
- `llm_retries: int = 3` — comment: retries of a failed LLM request when the endpoint stops responding (phase 67, `BOR_LLM_RETRIES`); `0` = no retries (the turn fails on the first error, pre-phase-67 behavior).
|
||||
- `llm_retry_delay: float = 5.0` — comment: flat seconds to wait between attempts (phase 67, `BOR_LLM_RETRY_DELAY`); the TODO-locked 5 s, no backoff.
|
||||
- Startup validators (house pattern: `agent_max_rounds` rejects negatives at startup): `llm_retries >= 0`, `llm_retry_delay >= 0`.
|
||||
2. `.env.example` — in the LLM section, add the two commented defaults next to `BOR_LLM_CHAT_MODEL`:
|
||||
- `# BOR_LLM_RETRIES=3 # retry a dead LLM request before the first token lands (phase 67); 0 = off`
|
||||
- `# BOR_LLM_RETRY_DELAY=5 # seconds between LLM retries (phase 67)`
|
||||
3. `app/rag/llm.py` — add, next to the other piece dataclasses:
|
||||
- `RetryPiece` — frozen dataclass, fields `attempt: int` (1-based attempt number that is about to be tried), `max_attempts: int` (total attempts = `llm_retries + 1`). One per wait; the API layer turns it into an SSE `retry` frame.
|
||||
- `async def chat_stream_retried(llm: LLMClient, messages: list[dict[str, str]], *, tools: list[dict[str, Any]] | None = None, retries: int = 0, delay: float = 0.0) -> AsyncGenerator[StreamPiece | ToolCallPiece | RetryPiece, None]`:
|
||||
- Loop `attempt` over `range(1, retries + 2)` (i.e. `retries + 1` attempts).
|
||||
- Each attempt: open `stream = llm.chat_stream(cast(...), tools=tools)`; track `emitted = False`.
|
||||
- `async for piece in stream`: mark `emitted = True`, `yield piece`.
|
||||
- On `LLMError`: if `emitted` → **re-raise unchanged** (terminal — locked A2: tokens already flowed, never redo a partial). Else if this was the final attempt → re-raise (exhausted). Else: `logger.warning("llm stream failed before the first piece (attempt %d/%d) — retrying in %.1fs: %s", ...)`, `yield RetryPiece(attempt, retries + 1)`, `await asyncio.sleep(delay)`, and start the next attempt with the SAME `messages`/`tools` (the request is restarted byte-identical — `chat_stream` is stateless).
|
||||
- `finally: await stream.aclose()` per attempt (keeps phase 48's deterministic teardown for every attempt's stream, including a consumer abandon during the sleep or a mid-attempt GeneratorExit).
|
||||
- `retries=0` → exactly one attempt, never a `RetryPiece` (the pre-phase-67 path, the kill-switch).
|
||||
- Docstring: state the A2 rule explicitly and that `RetryPiece` always precedes its sleep (the API frame must reach the client before the wait starts).
|
||||
4. Unit tests — `tests/unit/test_llm_client.py` (append; a fake `LLMClient` whose `chat_stream` is scripted):
|
||||
- failure on attempt 1, success on attempt 2 → pieces = `[RetryPiece(1, N), *answer pieces]`, `chat_stream` called twice, sleep awaited with the delay (monkeypatch `asyncio.sleep` and record calls).
|
||||
- failure on every attempt with `retries=2` → `RetryPiece(1, 3)`, `RetryPiece(2, 3)` then `LLMError` raised; 3 calls total.
|
||||
- failure AFTER the first piece → `LLMError` raised immediately, no `RetryPiece`, sleep never awaited, no second call (the A2 pin).
|
||||
- `retries=0` → one call, error propagates, no `RetryPiece`.
|
||||
- `delay=0` → sleep called with 0 (the e2e fast-path).
|
||||
- a consumer abandon (close the outer generator) mid-sleep and mid-attempt → no exception leaks, inner stream `aclose()` awaited.
|
||||
5. `tests/unit/test_config.py` — defaults (`3` / `5.0`), `BOR_LLM_RETRIES=0` and `BOR_LLM_RETRY_DELAY=1.5` honored, negative values rejected at startup.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit/integration: as listed in Work (4).
|
||||
- Coverage: **>90%** on this task's new/modified code.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `Settings()` defaults: `llm_retries == 3`, `llm_retry_delay == 5.0`; `.env.example` documents both.
|
||||
- [ ] `chat_stream_retried` passes all unit pins, including the A2 no-retry-after-first-piece rule.
|
||||
- [ ] `uv run pytest tests/unit/test_llm_client.py tests/unit/test_config.py -v` green; `uv run ruff check . && uv run pyright` clean.
|
||||
- [ ] no behavior change in completed work (the plain `llm.chat_stream` is untouched — the new function is additive).
|
||||
@@ -1,40 +0,0 @@
|
||||
# Task 02 — The `retry` SSE Event + Endpoint-Level Retries (Embedding, Deflected Stream)
|
||||
|
||||
**Phase:** `67_llm_retry` · **Source:** `TODO.md:3` — "…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)
|
||||
|
||||
## Objective
|
||||
`POST /api/chat` survives a dead endpoint: the pre-stream embedding and the deflected-turn stream retry (with SSE `retry` frames the UI can render live), and a `RetryPiece` from any answer stream becomes a `retry` SSE frame. Exhaustion and post-token failures keep today's terminal `error` frames.
|
||||
|
||||
## Work
|
||||
1. `app/schemas.py` — add `ChatRetryEvent` next to `ChatErrorEvent`:
|
||||
- `type: Literal["retry"] = "retry"`, `attempt: int` (the attempt number that just failed / the retry currently in flight — document which the endpoint sends: the attempt being tried next, 1-based), `max_attempts: int` (`llm_retries + 1`).
|
||||
- Docstring: sibling of the other SSE events; signals "the LLM request was restarted before any token landed" (locked A2); the client shows a transient status, not an error.
|
||||
2. `app/api/chat.py` — the embedding step (currently: `try: question_vec = await llm.embed_one(...) except EmbeddingError → error frame`):
|
||||
- Replace with an explicit attempt loop over `settings.llm_retries + 1` attempts (`attempt` 1-based):
|
||||
- on `EmbeddingError` before the final attempt: `logger.warning("chat: question=%r embedding failed (attempt %d/%d) — retrying in %.1fs", ...)`, `yield sse_event(ChatRetryEvent(attempt=attempt + 1, max_attempts=settings.llm_retries + 1).model_dump())`, `await asyncio.sleep(settings.llm_retry_delay)`.
|
||||
- on the final failure: the EXISTING terminal `error` frame ("I couldn't reach the embedding model — please try again.") unchanged — retries are exhausted, the copy stays (it reads correctly after N tries).
|
||||
- `llm_retries=0` → byte-identical to today (single attempt, same frame on failure).
|
||||
- track the turn-level retry count (`retries_used`) for the per-turn log line (step 4).
|
||||
3. `app/api/chat.py` — the answer stream:
|
||||
- deflected path: `answer_stream = llm.chat_stream(messages)` → `chat_stream_retried(llm, messages, tools=None, retries=settings.llm_retries, delay=settings.llm_retry_delay)`.
|
||||
- grounded path: unchanged call to `run_agent(...)` (task 03 makes IT retry internally); the shared piece loop below handles its `RetryPiece`s.
|
||||
- piece loop: add the `RetryPiece` branch (alongside the `ToolCallPiece` branch): `yield sse_event(ChatRetryEvent(attempt=piece.attempt, max_attempts=piece.max_attempts).model_dump())` and fold the attempt into `retries_used` (count each `RetryPiece`). No other state changes (the thinking/clock/timeout handling is the client's job).
|
||||
4. `app/api/chat.py` — per-turn log line (PLAN §9): append a `retries=N` field (0 when nothing retried — the grounded/deflected/deflected-empty shapes all carry it, so the line shape is uniform; update the log-format comments and any test that pins the line shape).
|
||||
5. Integration tests — `tests/integration/test_chat_api.py` (fake LLM client injected via the existing dependency override pattern):
|
||||
- embedding fails once then succeeds → frames: `retry` (attempt 2) then the normal `delta`/`done` sequence; the answer completes.
|
||||
- embedding fails on every attempt (`llm_retries=2`) → `retry` frames (attempts 2, 3) then the terminal `error` frame; the detail is the existing embedding copy.
|
||||
- deflected stream: first attempt `LLMError` before any piece, second attempt streams → `retry` frame then `delta` frames + `done`.
|
||||
- deflected stream: `LLMError` AFTER one delta frame → no `retry` frame, the existing terminal `error` frame ("The chat model dropped the connection — try again?").
|
||||
- `llm_retries=0` → today's behavior (one attempt, error frame, no `retry` frames).
|
||||
- per-turn log line carries `retries=N` (assert on the captured log record).
|
||||
6. `tests/unit/test_frontend_tool_states.py` pattern — `tests/unit/test_frontend_feedback.py` (or the sibling JS-pin file the executor finds for SSE-branch ordering): pin that `ev.type === "retry"` is a first-class branch in `runTurn`'s handler (task 04 implements it; the pin lands with this task's contract so the shape is locked early). If a dedicated branch-order pin file already exists, extend it instead of creating one.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit/integration: as listed in Work (5, 6).
|
||||
- Coverage: **>90%** on this task's new/modified code.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] A simulated dead-then-recovered endpoint returns a completed turn with a `retry` frame in between; a dead endpoint returns the existing terminal error frame after `llm_retries + 1` attempts.
|
||||
- [ ] `uv run pytest tests/integration/test_chat_api.py -v` green; `uv run ruff check . && uv run pyright` clean.
|
||||
- [ ] no behavior change when `BOR_LLM_RETRIES=0` (pre-phase-67 wire shape, byte-identical frames).
|
||||
@@ -1,31 +0,0 @@
|
||||
# Task 03 — Per-Round Retries Inside the Agent Loop
|
||||
|
||||
**Phase:** `67_llm_retry` · **Source:** `TODO.md:3` — "Add a .env configurable retry in case the LLM server fails to respond. Allow 3 retries by default, with 5 seconds between each retry."
|
||||
**Story:** n/a (TODO-derived — owner roadmap confirmation 2026-09-01)
|
||||
|
||||
## Objective
|
||||
A grounded turn survives a dead endpoint in the MIDDLE of the agent loop: every model request `run_agent` makes (each tool round and the forced final no-tools call) goes through `chat_stream_retried`, so a round that dies before its first piece is restarted with the same messages — while the round-cap, tool-counting, and phase-48 teardown semantics stay exactly as they are.
|
||||
|
||||
## Work
|
||||
1. `app/rag/agent.py` — `run_agent`:
|
||||
- loop round: `stream = llm.chat_stream(cast("list[dict[str, str]]", messages), tools=tools)` → `stream = chat_stream_retried(llm, cast(...), tools=tools, retries=settings.llm_retries, delay=settings.llm_retry_delay)`. Keep the phase-48 binding + `try/finally: await stream.aclose()` shape AROUND the new generator — closing the outer generator propagates `GeneratorExit` into `chat_stream_retried`, whose own `finally` closes the in-flight inner `chat_stream` (task 01). The inner stream's teardown therefore still happens deterministically on consumer abandon.
|
||||
- forced final no-tools call: same substitution with `tools=None`.
|
||||
- the `async for piece in stream` loop already yields every piece — `RetryPiece` values flow through to the API layer unchanged (no filtering); `calls` still collects only `ToolCallPiece`s.
|
||||
- `retries=0` (or `agent_max_rounds=0`'s single `tools=None` request) → identical to today: one plain attempt.
|
||||
- update the module docstring + the `run_agent` docstring: a failed round is retried before its first piece (phase 67, locked A2); a round that already streamed pieces fails the turn as before.
|
||||
2. Unit tests — `tests/unit/test_agent.py` (scripted fake `LLMClient.chat_stream`):
|
||||
- round 1 dies before any piece, round 1 retry succeeds with a `list_documents` tool call → pieces include a `RetryPiece` BEFORE the tool call; the tool executes; the final answer streams; `holder.tool_calls == 1`; `tool_calls` log line still emitted once.
|
||||
- a round dies AFTER a content piece → `LLMError` propagates out of `run_agent`, no retry (A2), the holder is untouched.
|
||||
- the forced final no-tools call (round cap reached) dies before its first piece → retried; the answer from the retry streams.
|
||||
- `settings.llm_retries=0` → no `RetryPiece` ever; a dead round raises immediately (pre-phase-67 behavior).
|
||||
- consumer abandon (close `run_agent`) while a retried round is mid-sleep → no leaked exception, inner stream closed.
|
||||
- round-cap counting is unaffected by retries: a failing-then-succeeding round consumes ONE round (retries are invisible to the cap).
|
||||
|
||||
## Testing & Quality
|
||||
- Unit/integration: as listed in Work (2).
|
||||
- Coverage: **>90%** on this task's new/modified code.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] A grounded turn whose agent loop's endpoint dies-then-recovers completes with the tool flow intact and a `RetryPiece` in the stream.
|
||||
- [ ] `uv run pytest tests/unit/test_agent.py -v` green; `uv run ruff check . && uv run pyright` clean.
|
||||
- [ ] the round cap, `holder` accounting, and teardown semantics are unchanged (existing `tests/unit/test_agent.py` pins still green).
|
||||
@@ -1,39 +0,0 @@
|
||||
# Task 04 — The Live "Communication interrupted — retrying" Status
|
||||
|
||||
**Phase:** `67_llm_retry` · **Source:** `TODO.md:3` — "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)
|
||||
|
||||
## Objective
|
||||
When a `retry` SSE frame arrives, the composer status line tells the user exactly what is happening — `Communication interrupted — retrying (n of N)…` — using the house status pattern (the `tool` frames' treatment: `#send-status` + typing-indicator `aria-label`, no new bubble, turn-timeout reset). No CSS change.
|
||||
|
||||
## Work
|
||||
1. `frontend/assets/app.js` — `runTurn`'s `readSSE` callback, add a first-class branch (between the `tool` and `delta` branches — the branch-order unit pin from task 02 expects it):
|
||||
```js
|
||||
} else if (ev.type === "retry") {
|
||||
clearTurnTimeout(); // the stream is alive — the server is restarting the LLM request
|
||||
const attempt = Number(ev.attempt) || 1;
|
||||
const max = Number(ev.max_attempts) || 1;
|
||||
const retryStatus =
|
||||
`Communication interrupted — retrying (${attempt} of ${max})…`;
|
||||
if (uiState === UI_STATE.thinking || uiState === UI_STATE.streaming) {
|
||||
sendStatus.textContent = retryStatus;
|
||||
document
|
||||
.querySelector("#typing-indicator .bubble")
|
||||
?.setAttribute("aria-label", retryStatus);
|
||||
}
|
||||
}
|
||||
```
|
||||
- The status gate covers BOTH live states: a retry arrives only before the current request's first piece (server rule A2), but the UI may already be `streaming` when a LATER agent round restarts after an earlier round emitted content (the rare content+tool-call stream).
|
||||
- NO new bubble, NO tool line, NO banner — it is a transient status; the next `thinking`/`tool`/`delta` frame replaces it via the existing branches.
|
||||
2. `frontend/assets/app.js` — the file-header doc comment: add the `retry` frame to the SSE-event inventory (the header documents every frame type; keep the house convention).
|
||||
3. JS unit pin (task 02's contract) — the pin file from task 02 now asserts: the branch exists between `tool` and `delta`; it calls `clearTurnTimeout()`; the locked copy literal `Communication interrupted — retrying (${attempt} of ${max})…` appears; no `addMessage`/`appendToolLine` in the branch.
|
||||
4. A11y (PLAN §7 / AGENTS.md rule 5): the status line already announces through the existing `#send-status` live-region and the typing-indicator `aria-label` — verify the branch reuses both (no new DOM, no new region). Contrast/focus unaffected (no new element).
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: the JS pin (Work 3).
|
||||
- Coverage: **>90%** on `app/` (no `app/` change this task — the gate holds at its current level).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] The branch-order + copy + no-DOM pins are green.
|
||||
- [ ] `uv run ruff check . && uv run pyright` clean (no JS check — house style: JS is E2E-gated in task 05).
|
||||
- [ ] no behavior change for existing frame types (the branch is purely additive).
|
||||
@@ -1,38 +0,0 @@
|
||||
# Task 05 — Failure-Injection Mock, E2E Suite, Regressions, Commit
|
||||
|
||||
**Phase:** `67_llm_retry` · **Source:** `TODO.md:3` — the full item (end-to-end proof: `.env` retry + the live UI feedback).
|
||||
**Story:** n/a (TODO-derived — owner roadmap confirmation 2026-09-01)
|
||||
|
||||
## Objective
|
||||
Prove the whole loop in a browser: a dead-then-recovered LLM endpoint shows the "retrying" status mid-turn and the answer still completes; a dead endpoint exhausts the retries and lands on the existing error banner. One dedicated Playwright suite, green in isolation, plus the regression pass and the phase commit.
|
||||
|
||||
## Work
|
||||
1. `tests/e2e/mock_llm.py` — deterministic failure injection (module-level counters, reset per trigger phrase — the mock is single-conversation per e2e server):
|
||||
- `RETRY_TRIGGER = "fail then answer"` — user message containing it: the first **2** streaming `chat/completions` requests respond `500` (JSON body, like a dead proxy); the 3rd streams the normal composed answer. (2 = 1 original attempt + 1 retry under the default `BOR_LLM_RETRIES=3`, so the e2e exercises a real retry without waiting for exhaustion.)
|
||||
- `ALWAYS_FAIL_TRIGGER = "always fail"` — user message containing it: every streaming `chat/completions` request responds `500` (exhaustion path).
|
||||
- `EMBED_FAIL_TRIGGER = "embed fail once"` — the first `embeddings` request responds `500`; the next returns the normal bag-of-words vector (covers the embedding retry loop in task 02).
|
||||
- Document all three in the module docstring's marker list (house convention).
|
||||
2. `tests/e2e/conftest.py` (or the `app_server` env block that sets `BOR_LLM_BASE_URL`) — add `BOR_LLM_RETRY_DELAY=0` to the e2e server env so retry waits are instant (the e2e pins the MECHANISM; the 5 s default is unit-pinned via config). `BOR_LLM_RETRIES` stays at its default (3) — the exhaustion test relies on the real default.
|
||||
3. `tests/e2e/test_llm_retry.py` — the dedicated suite (reuse the `#send-status` value-recording `add_init_script` pattern from `tests/e2e/test_agent_document_tools.py` L236+):
|
||||
- **dead-then-recovered (deflected path):** import one fixture doc; ask an unrelated question containing `fail then answer` (LOW turn → deflected stream retry): the recorded status values contain `Communication interrupted — retrying (2 of 4)…`; the turn settles with a deflected answer bubble, NO error banner.
|
||||
- **dead-then-recovered (grounded path):** ask a KB question containing `fail then answer` (HIGH turn → agent round retries): same status assertion; the answer completes (sources present).
|
||||
- **embedding retry:** an `embed fail once` question completes with a normal answer and a `retry` status recorded (the embedding loop is visible to the UI).
|
||||
- **exhaustion:** an `always fail` question → the error banner (`role="alert"`) appears with the existing copy ("The chat model dropped the connection — try again?"); the last recorded status is the highest attempt (`… retrying (4 of 4)…`); the send button is re-enabled (the banner path settles the state machine).
|
||||
- **zero-retry kill switch is unit-pinned only** (no e2e server variant needed).
|
||||
4. Regression pass (each in isolation, DB up): `uv run pytest tests/e2e/test_chat_rag.py -v --no-cov`, `test_agent_document_tools.py`, `test_stop_generation.py`, `test_retry_answer.py` — all green (the turn state machine, tool flow, stop, and redo must be untouched).
|
||||
5. Full gate: `uv run pytest --cov=app --cov-report=term-missing` (TOTAL >90%), `uv run ruff check . && uv run pyright`.
|
||||
6. Commit (AGENTS.md rule 8 — one atomic phase commit; stage the phase's code + its dir move):
|
||||
```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"
|
||||
```
|
||||
Then move the phase dir to `.agent/phases/complete/67_llm_retry/` and include the move in the SAME commit (house convention: commit first, move + amend the tree, one atomic commit — follow exactly what phase 66's commit did).
|
||||
|
||||
## Testing & Quality
|
||||
- E2E: as listed (Work 3–4).
|
||||
- Coverage: **>90%** on `app/` (validate.sh gate).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `uv run pytest tests/e2e/test_llm_retry.py -v --no-cov` green in isolation (DB up).
|
||||
- [ ] The four regression suites green in isolation.
|
||||
- [ ] `uv run pytest --cov=app` TOTAL >90%; `uv run ruff check . && uv run pyright` clean.
|
||||
- [ ] One `--no-gpg-sign` Conventional Commits commit; phase dir in `.agent/phases/complete/`.
|
||||
Reference in New Issue
Block a user