From 88293ed02f2a8038687569143fba903fd0e6e842 Mon Sep 17 00:00:00 2001 From: ducoterra Date: Wed, 2 Sep 2026 10:44:05 -0400 Subject: [PATCH] =?UTF-8?q?feat(rag):=20retry=20a=20failed=20LLM=20request?= =?UTF-8?q?=20before=20the=20first=20token=20lands=20=E2=80=94=20BOR=5FLLM?= =?UTF-8?q?=5FRETRIES/BOR=5FLLM=5FRETRY=5FDELAY=20with=20a=20live=20'retry?= =?UTF-8?q?ing'=20status?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../66_history_auto_save_copy/00_phase.md | 0 .../02_tests_and_commit.md | 0 .../01_config_and_retry_primitive.md | 0 .../67_llm_retry/02_chat_endpoint_retry.md | 0 .../67_llm_retry/03_agent_round_retry.md | 0 .../67_llm_retry/04_frontend_retry_status.md | 0 .../67_llm_retry/05_e2e_and_commit.md | 0 ...66_history_auto_save_copy__00_phase.a1.err | 0 .../66_history_auto_save_copy__00_phase.a1.md | 12 + ...story_auto_save_copy__00_phase.a1.validate | 72 +++ ..._auto_save_copy__02_tests_and_commit.a1.md | 16 + ...save_copy__02_tests_and_commit.a1.validate | 72 +++ .../67_llm_retry__00_phase.a1.err | 0 ...etry__01_config_and_retry_primitive.a1.err | 0 ...retry__01_config_and_retry_primitive.a1.md | 17 + ..._01_config_and_retry_primitive.a1.validate | 72 +++ ...7_llm_retry__02_chat_endpoint_retry.a1.err | 0 ...67_llm_retry__02_chat_endpoint_retry.a1.md | 10 + ..._retry__02_chat_endpoint_retry.a1.validate | 72 +++ .../67_llm_retry__03_agent_round_retry.a1.err | 0 .../67_llm_retry__03_agent_round_retry.a1.md | 19 + ...lm_retry__03_agent_round_retry.a1.validate | 72 +++ ...llm_retry__04_frontend_retry_status.a1.err | 0 ..._llm_retry__04_frontend_retry_status.a1.md | 19 + ...etry__04_frontend_retry_status.a1.validate | 72 +++ .../67_llm_retry__05_e2e_and_commit.a1.err | 0 .../67_llm_retry__05_e2e_and_commit.a1.md | 11 + ...7_llm_retry__05_e2e_and_commit.a1.validate | 72 +++ .env.example | 2 + app/api/chat.py | 127 ++++- app/config.py | 25 + app/rag/agent.py | 76 ++- app/rag/llm.py | 94 ++++ app/schemas.py | 24 + frontend/assets/app.js | 39 ++ tests/e2e/conftest.py | 9 + tests/e2e/mock_llm.py | 125 ++++- tests/e2e/test_llm_retry.py | 506 ++++++++++++++++++ tests/integration/test_chat_api.py | 206 ++++++- tests/unit/test_agent.py | 314 ++++++++++- tests/unit/test_config.py | 35 ++ tests/unit/test_frontend_feedback.py | 53 ++ tests/unit/test_llm_client.py | 274 ++++++++++ tests/unit/test_sse_events.py | 27 +- 44 files changed, 2488 insertions(+), 56 deletions(-) rename .agent/phases/{todo => complete}/66_history_auto_save_copy/00_phase.md (100%) rename .agent/phases/{todo => complete}/66_history_auto_save_copy/02_tests_and_commit.md (100%) rename .agent/phases/{todo => complete}/67_llm_retry/01_config_and_retry_primitive.md (100%) rename .agent/phases/{todo => complete}/67_llm_retry/02_chat_endpoint_retry.md (100%) rename .agent/phases/{todo => complete}/67_llm_retry/03_agent_round_retry.md (100%) rename .agent/phases/{todo => complete}/67_llm_retry/04_frontend_retry_status.md (100%) rename .agent/phases/{todo => complete}/67_llm_retry/05_e2e_and_commit.md (100%) create mode 100644 .agent/reports/66_history_auto_save_copy/66_history_auto_save_copy__00_phase.a1.err create mode 100644 .agent/reports/66_history_auto_save_copy/66_history_auto_save_copy__00_phase.a1.md create mode 100644 .agent/reports/66_history_auto_save_copy/66_history_auto_save_copy__00_phase.a1.validate create mode 100644 .agent/reports/66_history_auto_save_copy/66_history_auto_save_copy__02_tests_and_commit.a1.md create mode 100644 .agent/reports/66_history_auto_save_copy/66_history_auto_save_copy__02_tests_and_commit.a1.validate create mode 100644 .agent/reports/67_llm_retry/67_llm_retry__00_phase.a1.err create mode 100644 .agent/reports/67_llm_retry/67_llm_retry__01_config_and_retry_primitive.a1.err create mode 100644 .agent/reports/67_llm_retry/67_llm_retry__01_config_and_retry_primitive.a1.md create mode 100644 .agent/reports/67_llm_retry/67_llm_retry__01_config_and_retry_primitive.a1.validate create mode 100644 .agent/reports/67_llm_retry/67_llm_retry__02_chat_endpoint_retry.a1.err create mode 100644 .agent/reports/67_llm_retry/67_llm_retry__02_chat_endpoint_retry.a1.md create mode 100644 .agent/reports/67_llm_retry/67_llm_retry__02_chat_endpoint_retry.a1.validate create mode 100644 .agent/reports/67_llm_retry/67_llm_retry__03_agent_round_retry.a1.err create mode 100644 .agent/reports/67_llm_retry/67_llm_retry__03_agent_round_retry.a1.md create mode 100644 .agent/reports/67_llm_retry/67_llm_retry__03_agent_round_retry.a1.validate create mode 100644 .agent/reports/67_llm_retry/67_llm_retry__04_frontend_retry_status.a1.err create mode 100644 .agent/reports/67_llm_retry/67_llm_retry__04_frontend_retry_status.a1.md create mode 100644 .agent/reports/67_llm_retry/67_llm_retry__04_frontend_retry_status.a1.validate create mode 100644 .agent/reports/67_llm_retry/67_llm_retry__05_e2e_and_commit.a1.err create mode 100644 .agent/reports/67_llm_retry/67_llm_retry__05_e2e_and_commit.a1.md create mode 100644 .agent/reports/67_llm_retry/67_llm_retry__05_e2e_and_commit.a1.validate create mode 100644 tests/e2e/test_llm_retry.py diff --git a/.agent/phases/todo/66_history_auto_save_copy/00_phase.md b/.agent/phases/complete/66_history_auto_save_copy/00_phase.md similarity index 100% rename from .agent/phases/todo/66_history_auto_save_copy/00_phase.md rename to .agent/phases/complete/66_history_auto_save_copy/00_phase.md diff --git a/.agent/phases/todo/66_history_auto_save_copy/02_tests_and_commit.md b/.agent/phases/complete/66_history_auto_save_copy/02_tests_and_commit.md similarity index 100% rename from .agent/phases/todo/66_history_auto_save_copy/02_tests_and_commit.md rename to .agent/phases/complete/66_history_auto_save_copy/02_tests_and_commit.md diff --git a/.agent/phases/todo/67_llm_retry/01_config_and_retry_primitive.md b/.agent/phases/complete/67_llm_retry/01_config_and_retry_primitive.md similarity index 100% rename from .agent/phases/todo/67_llm_retry/01_config_and_retry_primitive.md rename to .agent/phases/complete/67_llm_retry/01_config_and_retry_primitive.md diff --git a/.agent/phases/todo/67_llm_retry/02_chat_endpoint_retry.md b/.agent/phases/complete/67_llm_retry/02_chat_endpoint_retry.md similarity index 100% rename from .agent/phases/todo/67_llm_retry/02_chat_endpoint_retry.md rename to .agent/phases/complete/67_llm_retry/02_chat_endpoint_retry.md diff --git a/.agent/phases/todo/67_llm_retry/03_agent_round_retry.md b/.agent/phases/complete/67_llm_retry/03_agent_round_retry.md similarity index 100% rename from .agent/phases/todo/67_llm_retry/03_agent_round_retry.md rename to .agent/phases/complete/67_llm_retry/03_agent_round_retry.md diff --git a/.agent/phases/todo/67_llm_retry/04_frontend_retry_status.md b/.agent/phases/complete/67_llm_retry/04_frontend_retry_status.md similarity index 100% rename from .agent/phases/todo/67_llm_retry/04_frontend_retry_status.md rename to .agent/phases/complete/67_llm_retry/04_frontend_retry_status.md diff --git a/.agent/phases/todo/67_llm_retry/05_e2e_and_commit.md b/.agent/phases/complete/67_llm_retry/05_e2e_and_commit.md similarity index 100% rename from .agent/phases/todo/67_llm_retry/05_e2e_and_commit.md rename to .agent/phases/complete/67_llm_retry/05_e2e_and_commit.md diff --git a/.agent/reports/66_history_auto_save_copy/66_history_auto_save_copy__00_phase.a1.err b/.agent/reports/66_history_auto_save_copy/66_history_auto_save_copy__00_phase.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agent/reports/66_history_auto_save_copy/66_history_auto_save_copy__00_phase.a1.md b/.agent/reports/66_history_auto_save_copy/66_history_auto_save_copy__00_phase.a1.md new file mode 100644 index 0000000..048bc86 --- /dev/null +++ b/.agent/reports/66_history_auto_save_copy/66_history_auto_save_copy__00_phase.a1.md @@ -0,0 +1,12 @@ +All criteria verified independently — no fixes were needed. Final report: + +**Phase 66 final verification pass — all green** + +- Verified (no code changes required): `frontend/history.html` — the 3 retired Save-button strings are gone; the 3 locked (A3) auto-save strings present byte-exact (meta L6, page-sub L107, empty row L162); `

Saved chats

` + anonymous gate copy byte-identical in `f04ddbe`'s diff. +- `tests/e2e/test_history_copy.py` run in isolation asserts the rendered admin page: gate hidden, table visible, 0-row empty-row + page-sub + meta equal locked copy, whole body matches no "press(ed) save"/"save button" pattern, `#save-chat-btn` absent. +- Tests: `uv run pytest tests/e2e/test_history_copy.py -v --no-cov` → 1 passed · regressions in isolation: `test_chat_history.py` → 5 passed, `test_stale_saved_chats.py` → 3 passed (DB up, healthy). +- `uv run pytest --cov=app --cov-report=term-missing` → 1254 passed, TOTAL **99%** (>90%). +- `uv run ruff check . && uv run pyright` → "All checks passed!" / "0 errors, 0 warnings". +- Commit criterion: `f04ddbe` exists on `main` with the locked message, staged `.agent/ frontend/ tests/`, no gpgsig (i.e. `--no-gpg-sign`); task files 01+02 now in `.agent/phases/complete/66_history_auto_save_copy/` (the 02 move sits in the working tree for the harness to finalize — left untouched per rules). +- Deviations: none. +- Next pending phase: `67_llm_retry` (`01_config_and_retry_primitive.md`). diff --git a/.agent/reports/66_history_auto_save_copy/66_history_auto_save_copy__00_phase.a1.validate b/.agent/reports/66_history_auto_save_copy/66_history_auto_save_copy__00_phase.a1.validate new file mode 100644 index 0000000..7bdd241 --- /dev/null +++ b/.agent/reports/66_history_auto_save_copy/66_history_auto_save_copy__00_phase.a1.validate @@ -0,0 +1,72 @@ +........................................................................ [ 5%] +........................................................................ [ 11%] +........................................................................ [ 17%] +........................................................................ [ 22%] +........................................................................ [ 28%] +........................................................................ [ 34%] +........................................................................ [ 40%] +........................................................................ [ 45%] +........................................................................ [ 51%] +........................................................................ [ 57%] +........................................................................ [ 63%] +........................................................................ [ 68%] +........................................................................ [ 74%] +........................................................................ [ 80%] +........................................................................ [ 86%] +........................................................................ [ 91%] +........................................................................ [ 97%] +.............................. [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +----------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 22 0 100% +app/api/chat.py 131 0 100% +app/api/chats.py 110 0 100% +app/api/config.py 7 0 100% +app/api/doc_drafts.py 93 0 100% +app/api/docs.py 50 0 100% +app/api/git_sources.py 181 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 8 0 100% +app/api/sync.py 101 0 100% +app/config.py 112 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 20 0 100% +app/core/caching.py 108 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/logging.py 13 0 100% +app/db.py 21 0 100% +app/main.py 52 0 100% +app/models.py 86 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 78 0 100% +app/rag/archive_upload.py 128 0 100% +app/rag/chunker.py 206 4 98% +app/rag/git_sources.py 14 0 100% +app/rag/importer.py 180 3 98% +app/rag/llm.py 189 0 100% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 65 0 100% +app/rag/retriever.py 94 3 97% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 24 0 100% +app/schemas.py 201 0 100% +----------------------------------------------- +TOTAL 2529 12 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +validation OK diff --git a/.agent/reports/66_history_auto_save_copy/66_history_auto_save_copy__02_tests_and_commit.a1.md b/.agent/reports/66_history_auto_save_copy/66_history_auto_save_copy__02_tests_and_commit.a1.md new file mode 100644 index 0000000..c044adb --- /dev/null +++ b/.agent/reports/66_history_auto_save_copy/66_history_auto_save_copy__02_tests_and_commit.a1.md @@ -0,0 +1,16 @@ +Task 02 complete — all gates green, committed. + +**Implemented** +- `tests/unit/test_history_copy.py` (NEW, house pattern): negative pins (3 retired "press Save" strings GONE), positive pins (3 locked A3 strings, each exactly once), state-language survivors present (h1, gate title/sub). +- `tests/e2e/test_history_copy.py` (NEW): admin-logged-in empty History tab — gate hidden, `#history-empty-row` revealed with the locked text, `.page-sub` + meta description equal to locked A3 copy, body text matches neither `/press(ed)?\s+save/i` nor `/save\s+button/i` with `

Saved chats` pinned, `#save-chat-btn` absent; `_reset_db` TRUNCATE extended with `saved_chats`. +- Fixed one pyright error in the new E2E (`text_content()` is `str | None`). + +**Verification (all green)** +- `uv run pytest tests/e2e/test_history_copy.py -v --no-cov` → 1 passed (isolation, DB up) +- `uv run pytest tests/e2e/test_chat_history.py -v --no-cov` → 5 passed · `test_stale_saved_chats.py` → 3 passed (both isolation) +- `uv run pytest --cov=app --cov-report=term-missing` → 1254 passed, TOTAL 99% (>90%) +- `uv run ruff check . && uv run pyright` → clean + +**Commit:** `f04ddbe` on `main`, `fix(web): history tab copy — …`, staged `.agent/ frontend/ tests/` only, `--no-gpg-sign`; tree clean. + +**Deviations:** none. Next pending: `67_llm_retry/01_config_and_retry_primitive.md`. diff --git a/.agent/reports/66_history_auto_save_copy/66_history_auto_save_copy__02_tests_and_commit.a1.validate b/.agent/reports/66_history_auto_save_copy/66_history_auto_save_copy__02_tests_and_commit.a1.validate new file mode 100644 index 0000000..7bdd241 --- /dev/null +++ b/.agent/reports/66_history_auto_save_copy/66_history_auto_save_copy__02_tests_and_commit.a1.validate @@ -0,0 +1,72 @@ +........................................................................ [ 5%] +........................................................................ [ 11%] +........................................................................ [ 17%] +........................................................................ [ 22%] +........................................................................ [ 28%] +........................................................................ [ 34%] +........................................................................ [ 40%] +........................................................................ [ 45%] +........................................................................ [ 51%] +........................................................................ [ 57%] +........................................................................ [ 63%] +........................................................................ [ 68%] +........................................................................ [ 74%] +........................................................................ [ 80%] +........................................................................ [ 86%] +........................................................................ [ 91%] +........................................................................ [ 97%] +.............................. [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +----------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 22 0 100% +app/api/chat.py 131 0 100% +app/api/chats.py 110 0 100% +app/api/config.py 7 0 100% +app/api/doc_drafts.py 93 0 100% +app/api/docs.py 50 0 100% +app/api/git_sources.py 181 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 8 0 100% +app/api/sync.py 101 0 100% +app/config.py 112 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 20 0 100% +app/core/caching.py 108 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/logging.py 13 0 100% +app/db.py 21 0 100% +app/main.py 52 0 100% +app/models.py 86 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 78 0 100% +app/rag/archive_upload.py 128 0 100% +app/rag/chunker.py 206 4 98% +app/rag/git_sources.py 14 0 100% +app/rag/importer.py 180 3 98% +app/rag/llm.py 189 0 100% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 65 0 100% +app/rag/retriever.py 94 3 97% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 24 0 100% +app/schemas.py 201 0 100% +----------------------------------------------- +TOTAL 2529 12 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +validation OK diff --git a/.agent/reports/67_llm_retry/67_llm_retry__00_phase.a1.err b/.agent/reports/67_llm_retry/67_llm_retry__00_phase.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agent/reports/67_llm_retry/67_llm_retry__01_config_and_retry_primitive.a1.err b/.agent/reports/67_llm_retry/67_llm_retry__01_config_and_retry_primitive.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agent/reports/67_llm_retry/67_llm_retry__01_config_and_retry_primitive.a1.md b/.agent/reports/67_llm_retry/67_llm_retry__01_config_and_retry_primitive.a1.md new file mode 100644 index 0000000..52c6169 --- /dev/null +++ b/.agent/reports/67_llm_retry/67_llm_retry__01_config_and_retry_primitive.a1.md @@ -0,0 +1,17 @@ +**Task 01 complete — retry settings + `chat_stream_retried` primitive** + +Implemented: +- `app/config.py`: `llm_retries: int = 3` / `llm_retry_delay: float = 5.0` in the LLM block + fail-loud `>= 0` startup validators (house `agent_max_rounds` pattern) +- `.env.example`: both `BOR_LLM_*` commented defaults next to `BOR_LLM_CHAT_MODEL` +- `app/rag/llm.py`: frozen `RetryPiece(attempt, max_attempts)` + `chat_stream_retried()` — retries only before the first piece (A2), `RetryPiece` always precedes its sleep, byte-identical restart, per-attempt `finally: await stream.aclose()` (phase-48 teardown), `retries=0` = single attempt +- 9 new unit tests in `test_llm_client.py` (incl. A2 pin, exhaustion, `retries=0`, `delay=0`, abandon mid-attempt and mid-sleep) + 4 in `test_config.py` + +Results: +- `uv run pytest tests/unit/test_llm_client.py tests/unit/test_config.py -v` → 92 passed +- `uv run pytest --cov=app` → 1266 passed, TOTAL **99%** (>90%); config.py & llm.py 100% +- `uv run ruff check .` → clean; `uv run pyright` → 0 errors +- Diff is purely additive — plain `chat_stream` untouched + +Notable: task text's "attempt that is about to be tried" conflicts with its own test pins (`RetryPiece(1, N)` after failure 1); I followed the pins (attempt = failed attempt = retry in flight), documented in the dataclass, consistent with task 02's `ChatRetryEvent(attempt=piece.attempt)`. + +Next pending task: `.agent/phases/todo/67_llm_retry/02_chat_endpoint_retry.md` diff --git a/.agent/reports/67_llm_retry/67_llm_retry__01_config_and_retry_primitive.a1.validate b/.agent/reports/67_llm_retry/67_llm_retry__01_config_and_retry_primitive.a1.validate new file mode 100644 index 0000000..2e698d0 --- /dev/null +++ b/.agent/reports/67_llm_retry/67_llm_retry__01_config_and_retry_primitive.a1.validate @@ -0,0 +1,72 @@ +........................................................................ [ 5%] +........................................................................ [ 11%] +........................................................................ [ 17%] +........................................................................ [ 22%] +........................................................................ [ 28%] +........................................................................ [ 34%] +........................................................................ [ 39%] +........................................................................ [ 45%] +........................................................................ [ 51%] +........................................................................ [ 56%] +........................................................................ [ 62%] +........................................................................ [ 68%] +........................................................................ [ 73%] +........................................................................ [ 79%] +........................................................................ [ 85%] +........................................................................ [ 90%] +........................................................................ [ 96%] +.......................................... [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +----------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 22 0 100% +app/api/chat.py 131 0 100% +app/api/chats.py 110 0 100% +app/api/config.py 7 0 100% +app/api/doc_drafts.py 93 0 100% +app/api/docs.py 50 0 100% +app/api/git_sources.py 181 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 8 0 100% +app/api/sync.py 101 0 100% +app/config.py 126 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 20 0 100% +app/core/caching.py 108 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/logging.py 13 0 100% +app/db.py 21 0 100% +app/main.py 52 0 100% +app/models.py 86 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 78 0 100% +app/rag/archive_upload.py 128 0 100% +app/rag/chunker.py 206 4 98% +app/rag/git_sources.py 14 0 100% +app/rag/importer.py 180 3 98% +app/rag/llm.py 213 0 100% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 65 0 100% +app/rag/retriever.py 94 3 97% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 24 0 100% +app/schemas.py 201 0 100% +----------------------------------------------- +TOTAL 2567 12 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +validation OK diff --git a/.agent/reports/67_llm_retry/67_llm_retry__02_chat_endpoint_retry.a1.err b/.agent/reports/67_llm_retry/67_llm_retry__02_chat_endpoint_retry.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agent/reports/67_llm_retry/67_llm_retry__02_chat_endpoint_retry.a1.md b/.agent/reports/67_llm_retry/67_llm_retry__02_chat_endpoint_retry.a1.md new file mode 100644 index 0000000..124cac0 --- /dev/null +++ b/.agent/reports/67_llm_retry/67_llm_retry__02_chat_endpoint_retry.a1.md @@ -0,0 +1,10 @@ +**Task 02 complete — all gates green.** + +- `ChatRetryEvent` SSE schema (`type/attempt/max_attempts`), documented as "attempt about to be tried, 1-based" (locked A2/A4) +- `app/api/chat.py`: embedding retry loop over `llm_retries + 1` attempts (warning + `retry` frame + flat delay; existing terminal error frame unchanged), deflected stream through `chat_stream_retried(tools=None)`, `RetryPiece` branch in the shared piece loop, `retries=N` appended to the per-turn log line, docstring updated +- **Defect fix (task 01):** `RetryPiece.attempt` carried the *failed* attempt, contradicting task 01's own spec, the endpoint's embed convention (`attempt+1`), and the task-05 E2E pins ("retrying (2 of 4)…" / "(4 of 4)…"). Changed `chat_stream_retried` to yield `RetryPiece(attempt + 1, …)` + updated its 3 unit pins +- Tests: 5 new integration scenarios (embed recover, embed exhaustion, deflected-stream retry, post-token terminal, `retries=0` byte-identical) + log-line asserts; updated 2 existing embed-failure tests; wire-shape pins added to `test_sse_events.py` +- **Deviation (Work 6):** the JS branch-order pin would fail until task 04 implements the branch (house pattern: pins land with the implementation, cf. phase 43) — locked the wire contract early in `test_sse_events.py` instead; task 04 adds the JS pin per its Work 3 +- `uv run pytest` → 1273 passed; `--cov=app` TOTAL 99% (chat.py/llm.py/schemas.py 100%); `ruff check .` + `pyright` clean; `.agent/validate.sh` → OK +- E2E smoke `tests/e2e/test_chat_rag.py` → 3 passed (deflected path now routes through the retry wrapper) +- Next pending task: `.agent/phases/todo/67_llm_retry/03_agent_round_retry.md` diff --git a/.agent/reports/67_llm_retry/67_llm_retry__02_chat_endpoint_retry.a1.validate b/.agent/reports/67_llm_retry/67_llm_retry__02_chat_endpoint_retry.a1.validate new file mode 100644 index 0000000..93049d4 --- /dev/null +++ b/.agent/reports/67_llm_retry/67_llm_retry__02_chat_endpoint_retry.a1.validate @@ -0,0 +1,72 @@ +........................................................................ [ 5%] +........................................................................ [ 11%] +........................................................................ [ 16%] +........................................................................ [ 22%] +........................................................................ [ 28%] +........................................................................ [ 33%] +........................................................................ [ 39%] +........................................................................ [ 45%] +........................................................................ [ 50%] +........................................................................ [ 56%] +........................................................................ [ 62%] +........................................................................ [ 67%] +........................................................................ [ 73%] +........................................................................ [ 79%] +........................................................................ [ 84%] +........................................................................ [ 90%] +........................................................................ [ 96%] +................................................. [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +----------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 22 0 100% +app/api/chat.py 147 0 100% +app/api/chats.py 110 0 100% +app/api/config.py 7 0 100% +app/api/doc_drafts.py 93 0 100% +app/api/docs.py 50 0 100% +app/api/git_sources.py 181 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 8 0 100% +app/api/sync.py 101 0 100% +app/config.py 126 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 20 0 100% +app/core/caching.py 108 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/logging.py 13 0 100% +app/db.py 21 0 100% +app/main.py 52 0 100% +app/models.py 86 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 78 0 100% +app/rag/archive_upload.py 128 0 100% +app/rag/chunker.py 206 4 98% +app/rag/git_sources.py 14 0 100% +app/rag/importer.py 180 3 98% +app/rag/llm.py 213 0 100% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 65 0 100% +app/rag/retriever.py 94 3 97% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 24 0 100% +app/schemas.py 205 0 100% +----------------------------------------------- +TOTAL 2587 12 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +validation OK diff --git a/.agent/reports/67_llm_retry/67_llm_retry__03_agent_round_retry.a1.err b/.agent/reports/67_llm_retry/67_llm_retry__03_agent_round_retry.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agent/reports/67_llm_retry/67_llm_retry__03_agent_round_retry.a1.md b/.agent/reports/67_llm_retry/67_llm_retry__03_agent_round_retry.a1.md new file mode 100644 index 0000000..d5c613c --- /dev/null +++ b/.agent/reports/67_llm_retry/67_llm_retry__03_agent_round_retry.a1.md @@ -0,0 +1,19 @@ +Task 03 is complete and fully green. + +## Report + +**Implemented** +- `app/rag/agent.py` — `run_agent` now routes every model request (each tool round **and** the forced final `tools=None` call) through `chat_stream_retried(llm, …, retries=settings.llm_retries, delay=settings.llm_retry_delay)`; phase-48 bind + `try/finally: await stream.aclose()` kept around the new generator (its own `finally` closes the in-flight inner `chat_stream` on consumer abandon); return type widened to include `RetryPiece`; module docstring (new loop-contract point 7) and `run_agent` docstring updated per the task. +- `tests/unit/test_agent.py` — new `FailingLLM` scripted fake (`(pieces, error)` attempts, records requests + stream teardown) and 6 pins: round retried before first piece (`RetryPiece` before tool call, byte-identical restart, one `tool_calls` log line, flat delay awaited); mid-stream drop terminal (A2 — `LLMError` out, holder untouched, no retry/sleep); forced final no-tools call retried (`tools=None` on both attempts); `llm_retries=0` = one plain attempt; consumer abandon mid-retry-sleep leaks nothing (inner stream closed, retry never starts); retries invisible to the round cap (`round=1/2`, `round=2/2`, cap warning). + +**Tests / lint / coverage** +- `uv run pytest tests/unit/test_agent.py -v` → **28 passed** (22 existing pins untouched + 6 new) +- `uv run pytest --cov=app --cov-report=term-missing` → **1279 passed**, TOTAL **99%** (>90%) +- `uv run ruff check .` → clean; `uv run pyright` → **0 errors** +- `uv run pytest tests/e2e/test_agent_document_tools.py -v --no-cov` → **4 passed** (grounded-path E2E regression) + +**Notes** +- Initial full-suite run had 2 failures in `test_import_docs_git.py` — purely environmental (Postgres down); fixed with `podman compose up -d db`, no code involved. +- No deviations; commit deferred to task 05 per phase plan. + +**Next pending task:** `.agent/phases/todo/67_llm_retry/04_frontend_retry_status.md` diff --git a/.agent/reports/67_llm_retry/67_llm_retry__03_agent_round_retry.a1.validate b/.agent/reports/67_llm_retry/67_llm_retry__03_agent_round_retry.a1.validate new file mode 100644 index 0000000..55ad73e --- /dev/null +++ b/.agent/reports/67_llm_retry/67_llm_retry__03_agent_round_retry.a1.validate @@ -0,0 +1,72 @@ +........................................................................ [ 5%] +........................................................................ [ 11%] +........................................................................ [ 16%] +........................................................................ [ 22%] +........................................................................ [ 28%] +........................................................................ [ 33%] +........................................................................ [ 39%] +........................................................................ [ 45%] +........................................................................ [ 50%] +........................................................................ [ 56%] +........................................................................ [ 61%] +........................................................................ [ 67%] +........................................................................ [ 73%] +........................................................................ [ 78%] +........................................................................ [ 84%] +........................................................................ [ 90%] +........................................................................ [ 95%] +....................................................... [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +----------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 22 0 100% +app/api/chat.py 147 0 100% +app/api/chats.py 110 0 100% +app/api/config.py 7 0 100% +app/api/doc_drafts.py 93 0 100% +app/api/docs.py 50 0 100% +app/api/git_sources.py 181 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 8 0 100% +app/api/sync.py 101 0 100% +app/config.py 126 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 20 0 100% +app/core/caching.py 108 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/logging.py 13 0 100% +app/db.py 21 0 100% +app/main.py 52 0 100% +app/models.py 86 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 78 0 100% +app/rag/archive_upload.py 128 0 100% +app/rag/chunker.py 206 4 98% +app/rag/git_sources.py 14 0 100% +app/rag/importer.py 180 3 98% +app/rag/llm.py 213 0 100% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 65 0 100% +app/rag/retriever.py 94 3 97% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 24 0 100% +app/schemas.py 205 0 100% +----------------------------------------------- +TOTAL 2587 12 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +validation OK diff --git a/.agent/reports/67_llm_retry/67_llm_retry__04_frontend_retry_status.a1.err b/.agent/reports/67_llm_retry/67_llm_retry__04_frontend_retry_status.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agent/reports/67_llm_retry/67_llm_retry__04_frontend_retry_status.a1.md b/.agent/reports/67_llm_retry/67_llm_retry__04_frontend_retry_status.a1.md new file mode 100644 index 0000000..fcf4c93 --- /dev/null +++ b/.agent/reports/67_llm_retry/67_llm_retry__04_frontend_retry_status.a1.md @@ -0,0 +1,19 @@ +All green. Task 04 is complete. + +## Report + +**Implemented** +- `frontend/assets/app.js` — new first-class `retry` branch in `runTurn`'s `readSSE` handler, between the `tool` and `delta` branches: `clearTurnTimeout()` + locked A4 copy `Communication interrupted — retrying (n of N)…` on the existing `#send-status` live region + typing-indicator `aria-label`; gate covers both `thinking` and `streaming`; no new DOM/bubble/banner/state change (purely additive). +- `frontend/assets/app.js` header doc — `retry` frame added to the SSE-event inventory (house convention, quotes the A4 copy). +- `tests/unit/test_frontend_feedback.py` — two new pins (task 02's contract, which task 02 left unimplemented): branch-order (`tool` < `retry` < `delta`), `clearTurnTimeout()`, exact copy literal, no `addMessage`/`appendToolLine`/`showErrorBanner`/`setUiState` in the branch, header inventory. Negative-checked: pin fails with the branch removed. + +**Tests / lint / coverage** +- `uv run pytest --cov=app` → exit 0, all unit + integration green +- Coverage TOTAL **99%** (app/), gate >90% holds (no `app/` change this task) +- `uv run ruff check . && uv run pyright` → all checks passed / 0 errors +- `node --check frontend/assets/app.js` → syntax OK (JS behavior is E2E-gated in task 05) + +**Decisions** +- Pin landed in `test_frontend_feedback.py` (the file task 02's work-6 named first); no behavior change to existing frame types. + +**Next pending task:** `67_llm_retry/05_e2e_and_commit.md` (mock-LLM failure injection, `tests/e2e/test_llm_retry.py`, regressions, commit). diff --git a/.agent/reports/67_llm_retry/67_llm_retry__04_frontend_retry_status.a1.validate b/.agent/reports/67_llm_retry/67_llm_retry__04_frontend_retry_status.a1.validate new file mode 100644 index 0000000..069fe22 --- /dev/null +++ b/.agent/reports/67_llm_retry/67_llm_retry__04_frontend_retry_status.a1.validate @@ -0,0 +1,72 @@ +........................................................................ [ 5%] +........................................................................ [ 11%] +........................................................................ [ 16%] +........................................................................ [ 22%] +........................................................................ [ 28%] +........................................................................ [ 33%] +........................................................................ [ 39%] +........................................................................ [ 44%] +........................................................................ [ 50%] +........................................................................ [ 56%] +........................................................................ [ 61%] +........................................................................ [ 67%] +........................................................................ [ 73%] +........................................................................ [ 78%] +........................................................................ [ 84%] +........................................................................ [ 89%] +........................................................................ [ 95%] +......................................................... [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +----------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 22 0 100% +app/api/chat.py 147 0 100% +app/api/chats.py 110 0 100% +app/api/config.py 7 0 100% +app/api/doc_drafts.py 93 0 100% +app/api/docs.py 50 0 100% +app/api/git_sources.py 181 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 8 0 100% +app/api/sync.py 101 0 100% +app/config.py 126 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 20 0 100% +app/core/caching.py 108 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/logging.py 13 0 100% +app/db.py 21 0 100% +app/main.py 52 0 100% +app/models.py 86 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 78 0 100% +app/rag/archive_upload.py 128 0 100% +app/rag/chunker.py 206 4 98% +app/rag/git_sources.py 14 0 100% +app/rag/importer.py 180 3 98% +app/rag/llm.py 213 0 100% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 65 0 100% +app/rag/retriever.py 94 3 97% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 24 0 100% +app/schemas.py 205 0 100% +----------------------------------------------- +TOTAL 2587 12 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +validation OK diff --git a/.agent/reports/67_llm_retry/67_llm_retry__05_e2e_and_commit.a1.err b/.agent/reports/67_llm_retry/67_llm_retry__05_e2e_and_commit.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agent/reports/67_llm_retry/67_llm_retry__05_e2e_and_commit.a1.md b/.agent/reports/67_llm_retry/67_llm_retry__05_e2e_and_commit.a1.md new file mode 100644 index 0000000..75b2f85 --- /dev/null +++ b/.agent/reports/67_llm_retry/67_llm_retry__05_e2e_and_commit.a1.md @@ -0,0 +1,11 @@ +All green and committed. Final report: + +**Phase 67 task 05 — complete** (`46c0718`) + +- `tests/e2e/mock_llm.py`: deterministic failure injection — `fail then answer` (2 dead app-level attempts → answer), `always fail` (all 500s), `embed fail once` (first embeddings POST 500s); module counters reset per trigger phrase, documented in the marker list +- `tests/e2e/conftest.py`: `BOR_LLM_RETRY_DELAY=0` on the e2e server + `BOR_LLM_RETRIES` forced to the real default (3, phase-61 leak-guard pattern) +- `tests/e2e/test_llm_retry.py`: 4 tests — dead-then-recovered deflected (statuses "(2 of 4)…"/"(3 of 4)…", retry frames before first delta, deflected settle, no banner), dead-then-recovered grounded (agent round retried, sources intact), embedding retry visible to UI, exhaustion (last retrying status "(4 of 4)…", `role=alert` banner with existing copy, send re-enabled, no bubble) +- Notable: the mock counts failures in **app-level attempts, not POSTs** — the openai SDK's default (max_retries=2) re-POSTs a dead stream twice, so a literal "first 2 POSTs fail" would be absorbed internally and no app retry would ever occur; deflected test seeds one fixture doc (`kubernetes.md`) because the marker's `fail`/`answer` tokens FTS-hit the 13-doc corpus and would flip the gate to HIGH (verified via `plan_turn`) +- Results: `uv run pytest tests/e2e/test_llm_retry.py -v --no-cov` → 4 passed (16.8s); regressions in isolation → chat_rag 3, agent_document_tools 4, stop_generation 3, retry_answer 4 — all passed; `uv run pytest --cov=app --cov-report=term-missing` → 1281 passed, TOTAL **99%**; `uv run ruff check .` clean; `uv run pyright` 0 errors +- One `--no-gpg-sign` commit (code + tasks 01–04 dir moves + phase-66 leftover move, phase-66 pattern); the 05 task file stays in `todo/` for the harness to move +- Next pending: `.agent/phases/todo/68_search_tool/` diff --git a/.agent/reports/67_llm_retry/67_llm_retry__05_e2e_and_commit.a1.validate b/.agent/reports/67_llm_retry/67_llm_retry__05_e2e_and_commit.a1.validate new file mode 100644 index 0000000..069fe22 --- /dev/null +++ b/.agent/reports/67_llm_retry/67_llm_retry__05_e2e_and_commit.a1.validate @@ -0,0 +1,72 @@ +........................................................................ [ 5%] +........................................................................ [ 11%] +........................................................................ [ 16%] +........................................................................ [ 22%] +........................................................................ [ 28%] +........................................................................ [ 33%] +........................................................................ [ 39%] +........................................................................ [ 44%] +........................................................................ [ 50%] +........................................................................ [ 56%] +........................................................................ [ 61%] +........................................................................ [ 67%] +........................................................................ [ 73%] +........................................................................ [ 78%] +........................................................................ [ 84%] +........................................................................ [ 89%] +........................................................................ [ 95%] +......................................................... [100%] +=============================== warnings summary =============================== +.venv/lib/python3.13/site-packages/fastapi/testclient.py:1 + /var/home/ducoterra/Projects/Personal/brain_of_reese/.venv/lib/python3.13/site-packages/fastapi/testclient.py:1: StarletteDeprecationWarning: Using `httpx` with `starlette.testclient` is deprecated; install `httpx2` instead. + from starlette.testclient import TestClient as TestClient # noqa + +-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html +================================ tests coverage ================================ +_______________ coverage: platform linux, python 3.13.13-final-0 _______________ + +Name Stmts Miss Cover +----------------------------------------------- +app/__init__.py 1 0 100% +app/api/__init__.py 0 0 100% +app/api/auth.py 22 0 100% +app/api/chat.py 147 0 100% +app/api/chats.py 110 0 100% +app/api/config.py 7 0 100% +app/api/doc_drafts.py 93 0 100% +app/api/docs.py 50 0 100% +app/api/git_sources.py 181 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 8 0 100% +app/api/sync.py 101 0 100% +app/config.py 126 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 20 0 100% +app/core/caching.py 108 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/logging.py 13 0 100% +app/db.py 21 0 100% +app/main.py 52 0 100% +app/models.py 86 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 78 0 100% +app/rag/archive_upload.py 128 0 100% +app/rag/chunker.py 206 4 98% +app/rag/git_sources.py 14 0 100% +app/rag/importer.py 180 3 98% +app/rag/llm.py 213 0 100% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 65 0 100% +app/rag/retriever.py 94 3 97% +app/rag/sources_meta.py 16 0 100% +app/rag/suggestions.py 27 0 100% +app/rag/summarizer.py 24 0 100% +app/schemas.py 205 0 100% +----------------------------------------------- +TOTAL 2587 12 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +validation OK diff --git a/.env.example b/.env.example index 4088536..8bcd900 100644 --- a/.env.example +++ b/.env.example @@ -18,6 +18,8 @@ BOR_DATABASE_URL=postgresql+psycopg://reese:reese@localhost:5432/brain_of_reese BOR_LLM_BASE_URL=https://aipi.reeseapps.com/v1 BOR_LLM_API_KEY= # falls back to $AIPI_KEY, then "not-needed" BOR_LLM_CHAT_MODEL=turbo +# 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) BOR_LLM_EMBED_MODEL=embed BOR_LLM_SUMMARY_MODEL=lite # one-shot completions: document summaries (phase 30), KB overview (phase 31) BOR_EMBEDDING_DIM=768 # verified 2026-08 via scripts/llm_probe.py diff --git a/app/api/chat.py b/app/api/chat.py index 1fc5805..6921b99 100644 --- a/app/api/chat.py +++ b/app/api/chat.py @@ -68,9 +68,27 @@ byte-identical to the pre-phase path (A8):** the LOW prompt never carries tools, and with ``agent_max_rounds`` at **0** ``run_agent`` makes exactly one ``tools=None`` request, reproducing the pre-phase behavior (the kill switch). + +LLM retries (phase 67, ``BOR_LLM_RETRIES`` / ``BOR_LLM_RETRY_DELAY``, +owner-locked 2026-09-01): when the aipi endpoint dies before a request +has streamed its first output frame (locked A2), the request is +restarted — up to ``llm_retries`` times (default 3), a flat +``llm_retry_delay`` (default 5 s) between attempts. Every restart is +announced with an SSE ``retry`` frame (``{"type": "retry", +"attempt": n, "max_attempts": N}`` — the attempt about to be tried, +1-based, ahead of the pre-retry wait) so the UI can show the transient +"Communication interrupted — retrying (n of N)…" status (locked A4); +exhaustion and any failure after the first frame keep the existing +terminal ``error`` frames. The pre-stream question embedding retries on +the same budget, and the deflected answer stream goes through +``chat_stream_retried`` (the agent loop retries per round — task 03). +The per-turn log line records ``retries=N`` after ``total_ms=N`` (0 +when nothing was retried — the field is uniform across all turn +shapes). """ from __future__ import annotations +import asyncio import json import logging import time @@ -91,8 +109,10 @@ from app.rag.llm import ( EmbeddingError, LLMClient, LLMError, + RetryPiece, # phase 67: one LLM request restart (an SSE retry frame) StreamPiece, # type of the answer pieces streamed by the agent loop ToolCallPiece, # phase 37: one model-requested tool call + chat_stream_retried, # phase 67: the retry-before-first-piece primitive ) from app.rag.overview import load_kb_overview from app.rag.prompts import build_deflect_prompt, build_high_prompt @@ -102,6 +122,7 @@ from app.schemas import ( ChatDoneEvent, ChatErrorEvent, ChatRequest, + ChatRetryEvent, ChatThinkingEvent, ChatToolEvent, SourceRef, @@ -246,34 +267,66 @@ async def chat( # consumer went away before any terminal frame; it must not # yield (GeneratorExit handling). settled = False + retries_used = 0 # phase 67: LLM requests restarted this turn (log line) try: + settings = get_settings() + # 1. Embed the question. + # Phase 67: a dead embeddings endpoint is retried before any + # frame has left the server — up to ``llm_retries`` restarts, + # a flat ``llm_retry_delay`` between attempts, one SSE + # ``retry`` frame per restart (the UI shows the transient + # "retrying" status, not an error — locked A4). The final + # failure keeps the EXISTING terminal ``error`` frame (the + # copy reads correctly after N tries); ``llm_retries=0`` is + # byte-identical to the pre-phase-67 single attempt. t0 = time.monotonic() - try: - question_vec = await llm.embed_one(request.message) - except EmbeddingError as e: - embed_ms = int((time.monotonic() - t0) * 1000) - total_ms = int((time.monotonic() - started) * 1000) - logger.error( - "chat: question=%r embed_ms=%d total_ms=%d — embedding failed: %s", - request.message, - embed_ms, - total_ms, - e, - ) - settled = True # terminal: the error frame settles the turn - yield sse_event( - ChatErrorEvent( - detail="I couldn't reach the embedding model — please try again." - ).model_dump() - ) - return + max_attempts = settings.llm_retries + 1 + attempt = 1 + while True: + try: + question_vec = await llm.embed_one(request.message) + break + except EmbeddingError as e: + embed_ms = int((time.monotonic() - t0) * 1000) + if attempt >= max_attempts: + total_ms = int((time.monotonic() - started) * 1000) + logger.error( + "chat: question=%r embed_ms=%d total_ms=%d — embedding failed: %s", + request.message, + embed_ms, + total_ms, + e, + ) + settled = True # terminal: the error frame settles the turn + yield sse_event( + ChatErrorEvent( + detail="I couldn't reach the embedding model — please try again." + ).model_dump() + ) + return + logger.warning( + "chat: question=%r embedding failed (attempt %d/%d) — " + "retrying in %.1fs: %s", + request.message, + attempt, + max_attempts, + settings.llm_retry_delay, + e, + ) + retries_used += 1 + yield sse_event( + ChatRetryEvent( + attempt=attempt + 1, max_attempts=max_attempts + ).model_dump() + ) + await asyncio.sleep(settings.llm_retry_delay) + attempt += 1 embed_ms = int((time.monotonic() - t0) * 1000) # 2. Retrieve top-K chunks, load the owner's steering notes # (phase 15), then the honesty gate (A8) picks the HIGH # (grounded) or LOW (deflected) prompt + context. - settings = get_settings() try: steering_notes = load_steering_notes(db) # KB overview (phase 31): one indexed PK lookup per turn — @@ -313,9 +366,21 @@ async def chat( # ``agent_max_rounds=0`` ``run_agent`` is a single # ``tools=None`` request anyway (the kill switch). holder = AgentHolder() - answer_stream: AsyncIterator[StreamPiece | ToolCallPiece] + answer_stream: AsyncIterator[StreamPiece | ToolCallPiece | RetryPiece] if plan.deflected: - answer_stream = llm.chat_stream(messages) + # Phase 67: the deflected stream goes through the retry + # primitive — a dead endpoint is restarted (SSE ``retry`` + # frames) only before its first piece (locked A2); the + # grounded path stays a plain ``run_agent`` call (task 03 + # makes IT retry internally) — its ``RetryPiece``s flow + # through the shared piece loop below. + answer_stream = chat_stream_retried( + llm, + messages, + tools=None, + retries=settings.llm_retries, + delay=settings.llm_retry_delay, + ) else: answer_stream = run_agent( llm, @@ -328,7 +393,7 @@ async def chat( ) thinking_chars = 0 try: - async for piece in answer_stream: # StreamPiece | ToolCallPiece + async for piece in answer_stream: # StreamPiece | ToolCallPiece | RetryPiece if isinstance(piece, ToolCallPiece): # Phase 37 (PLAN §4 extension): one SSE ``tool`` # frame per model-requested call; ``argument`` is @@ -345,6 +410,19 @@ async def chat( ).model_dump() ) continue + if isinstance(piece, RetryPiece): + # Phase 67: the answer stream was restarted before + # its first piece (locked A2) — a transient status + # frame, never an error. No other state changes: + # the thinking/clock/timeout handling is the + # client's job. + retries_used += 1 + yield sse_event( + ChatRetryEvent( + attempt=piece.attempt, max_attempts=piece.max_attempts + ).model_dump() + ) + continue if piece.kind == "thinking": thinking_chars += len(piece.text) if settings.stream_thinking: @@ -419,7 +497,7 @@ async def chat( logger.info( "question=%r embed_ms=%d top_score=%.3f fts_hits=%d summary_hits=%d tuning=%d " "kb_chars=%d threshold=%.2f deflected=%s sources=%r thinking_chars=%d " - "tool_calls=%d total_ms=%d", + "tool_calls=%d total_ms=%d retries=%d", request.message, embed_ms, plan.top_score, @@ -433,6 +511,7 @@ async def chat( thinking_chars, holder.tool_calls, total_ms, + retries_used, ) settled = True # terminal: the done frame settles the turn yield sse_event( diff --git a/app/config.py b/app/config.py index 6576073..7c530a0 100644 --- a/app/config.py +++ b/app/config.py @@ -73,6 +73,13 @@ class Settings(BaseSettings): #: pieces are still counted for the per-turn log line but never #: emitted — the answer stream itself is unchanged. stream_thinking: bool = True + #: 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_retries: int = 3 + #: Flat seconds to wait between attempts (phase 67, + #: ``BOR_LLM_RETRY_DELAY``); the TODO-locked 5 s, no backoff. + llm_retry_delay: float = 5.0 # --- RAG tuning --- embedding_dim: int = 768 # verified against aipi /v1 (embed model) @@ -261,6 +268,24 @@ class Settings(BaseSettings): raise ValueError("agent_max_rounds must be >= 0 (0 = no tools)") return v + @field_validator("llm_retries") + @classmethod + def _llm_retries_non_negative(cls, v: int) -> int: + """``0`` is the no-retry kill switch (pre-phase-67 behavior) — a + negative value is a typo (the ``agent_max_rounds`` pattern).""" + if v < 0: + raise ValueError("llm_retries must be >= 0 (0 = no retries)") + return v + + @field_validator("llm_retry_delay") + @classmethod + def _llm_retry_delay_non_negative(cls, v: float) -> float: + """A negative delay is a typo — fail loud at startup (the + ``agent_max_rounds`` pattern).""" + if v < 0: + raise ValueError("llm_retry_delay must be >= 0 (seconds)") + return v + @field_validator("upload_max_mb") @classmethod def _upload_max_mb_positive(cls, v: int) -> int: diff --git a/app/rag/agent.py b/app/rag/agent.py index 6d45f84..f1518f5 100644 --- a/app/rag/agent.py +++ b/app/rag/agent.py @@ -43,16 +43,27 @@ task 04): the assistant tool-call message + the tool result (refusals included), consumes one round, and the model is called again. At the round cap — ``max_rounds = settings.agent_max_rounds`` (``BOR_AGENT_MAX_ROUNDS``, - default 10) — the loop forces one final ``chat_stream(messages, - tools=None)`` and returns: the cap is the **only** forced exit - (besides "the stream carried no calls"), and it bounds pathological - rejected-call streams. + default 10) — the loop forces one final retried no-tools request + (``chat_stream_retried`` with ``tools=None``) and returns: the cap is + the **only** forced exit (besides "the stream carried no calls"), and + it bounds pathological rejected-call streams. 5. A rare stream that carries both content and a tool call keeps the content (it was already emitted) **and** still runs the tool. 6. *holder* (an :class:`AgentHolder`) records the read documents and the number of executed tool calls (re-lists included); the API layer (task 04) reads it after the stream to extend ``done.sources`` / ``query_log.sources`` and the per-turn log line (``tool_calls=N``). +7. Retries (phase 67, owner-locked A2): every model request — each tool + round and the forced final ``tools=None`` call — goes through + ``chat_stream_retried``: a round that dies before its first piece is + restarted with the SAME messages (up to ``settings.llm_retries`` + restarts, a flat ``settings.llm_retry_delay`` between attempts, each + preceded by a :class:`app.rag.llm.RetryPiece` the API layer turns into + an SSE ``retry`` frame); a round that already streamed a piece fails + the turn as before (no partial answer is ever redone). Retries are + invisible to the round cap: a round that needed a retry still consumes + exactly one round. With ``settings.llm_retries=0`` every request is a + single plain attempt (the pre-phase-67 path). The DB accessors (:func:`list_catalog`, :func:`find_document`) are module-level functions so unit tests can monkeypatch them without a @@ -71,7 +82,13 @@ from sqlalchemy.orm import Session from app.config import Settings from app.models import Document -from app.rag.llm import LLMClient, StreamPiece, ToolCallPiece +from app.rag.llm import ( + LLMClient, + RetryPiece, + StreamPiece, + ToolCallPiece, + chat_stream_retried, +) logger = logging.getLogger("app.agent") @@ -224,13 +241,20 @@ async def run_agent( seed_docs: Sequence[Document], settings: Settings, holder: AgentHolder, -) -> AsyncIterator[StreamPiece | ToolCallPiece]: +) -> AsyncIterator[StreamPiece | ToolCallPiece | RetryPiece]: """Run the grounded-turn tool loop, yielding every stream piece. - Every piece (``thinking`` / ``content`` / tool calls) is yielded as it - arrives; the API layer (task 04) turns tool-call pieces into SSE - ``tool`` events. After the loop finishes, *holder* carries the read - documents and the executed tool-call count (re-lists included). + Every piece (``thinking`` / ``content`` / tool calls / + :class:`RetryPiece`) is yielded as it arrives; the API layer (task 04) + turns tool-call pieces into SSE ``tool`` events and retry pieces into + SSE ``retry`` events. After the loop finishes, *holder* carries the + read documents and the executed tool-call count (re-lists included). + + Retries (phase 67, owner-locked A2): every model request goes through + :func:`chat_stream_retried` — a failed round is retried **before** its + first piece (same messages, ``settings.llm_retries`` restarts, a flat + ``settings.llm_retry_delay``); a round that already streamed pieces + fails the turn as before. ``seed_docs`` are the documents the retrieval already put in context (they shape the *system_prompt* the caller built); re-reading one of @@ -252,10 +276,22 @@ async def run_agent( calls: list[ToolCallPiece] = [] # Phase 48: bind the round's stream so a consumer abandon # (GeneratorExit into the yield below) tears down the in-flight - # model stream deterministically — not GC-dependent. Awaiting - # ``aclose()`` in the ``finally`` is safe because it does not - # yield; on a fully consumed round it is a quiet no-op. - stream = llm.chat_stream(cast("list[dict[str, str]]", messages), tools=tools) + # model stream deterministically — not GC-dependent. Phase 67: + # the round goes through the retry primitive — a failure before + # the first piece restarts the request (locked A2) after a + # RetryPiece; closing the OUTER generator propagates GeneratorExit + # into ``chat_stream_retried``, whose own ``finally`` closes the + # in-flight inner ``chat_stream``, so teardown stays deterministic + # on consumer abandon. Awaiting ``aclose()`` in the ``finally`` is + # safe because it does not yield; on a fully consumed round it is + # a quiet no-op. + stream = chat_stream_retried( + llm, + cast("list[dict[str, str]]", messages), + tools=tools, + retries=settings.llm_retries, + delay=settings.llm_retry_delay, + ) try: async for piece in stream: if isinstance(piece, ToolCallPiece): @@ -300,8 +336,16 @@ async def run_agent( ) # Phase 48: the forced final answer gets the same explicit # teardown as the loop rounds (consumer abandon mid-final - # answer must still close the model's stream). - final = llm.chat_stream(cast("list[dict[str, str]]", messages), tools=None) + # answer must still close the model's stream). Phase 67: the + # forced call retries under the same locked-A2 rule as the + # loop rounds. + final = chat_stream_retried( + llm, + cast("list[dict[str, str]]", messages), + tools=None, + retries=settings.llm_retries, + delay=settings.llm_retry_delay, + ) try: async for piece in final: yield piece diff --git a/app/rag/llm.py b/app/rag/llm.py index afe26c8..4f21476 100644 --- a/app/rag/llm.py +++ b/app/rag/llm.py @@ -18,6 +18,7 @@ vectors that pgvector rejects. """ from __future__ import annotations +import asyncio import json import logging from collections.abc import AsyncGenerator @@ -82,6 +83,25 @@ class ToolCallPiece: arguments: dict[str, Any] +@dataclass(frozen=True) +class RetryPiece: + """One LLM request retry that is about to start (phase 67, locked A2). + + ``attempt`` is the 1-based number of the attempt that is about to be + tried — the one AFTER the attempt that just failed (a first-attempt + failure carries ``attempt=2``, so the API's SSE ``retry`` frame reads + "retrying (2 of N)" — the same convention the endpoint's embedding + retry loop uses, phase 67 task 02); ``max_attempts`` is the total + attempt budget (``llm_retries + 1``). One piece per wait: the API + layer turns it into an SSE ``retry`` frame, and it always precedes + the pre-retry sleep so the frame reaches the client before the wait + starts. + """ + + attempt: int + max_attempts: int + + @dataclass class _ToolCallSlot: """Mutable accumulator for one streamed tool call (phase 37, private). @@ -442,6 +462,80 @@ class LLMClient: await stream.close() +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]: + """Stream a chat turn, retrying a dead endpoint (phase 67). + + Wraps :meth:`LLMClient.chat_stream` with the retry-before-first-piece + rule (owner-locked A2): a request is restarted **only** while no output + piece (thinking/tool/delta) has been yielded for it. Once pieces have + flowed, an :class:`LLMError` is re-raised unchanged — a partial answer + is never redone, and the API layer's terminal ``error`` frame applies. + This primitive is the ONLY place that rule lives (the chat endpoint + and the agent loop both build on it). + + Up to *retries* restarts after the initial attempt (``retries + 1`` + attempts total; ``retries=0`` is exactly one attempt with no + :class:`RetryPiece` — the pre-phase-67 kill-switch path). Each restart + is preceded by one :class:`RetryPiece` (``attempt`` = the 1-based + number of the attempt about to be tried — the failed attempt + 1 — + and ``max_attempts`` = ``retries + 1``) and a flat + ``asyncio.sleep(delay)`` — the TODO-locked fixed interval, no + backoff. The ``RetryPiece`` always precedes its sleep: the API frame + must reach the client before the wait starts. + + The request is restarted byte-identical: ``chat_stream`` is stateless, + so every attempt is opened with the SAME *messages*/*tools*. + + Teardown (phase 48, extended): every attempt's stream is explicitly + closed in a ``finally`` — normal exhaustion, a terminal + :class:`LLMError`, and a consumer abandon (``GeneratorExit`` mid-attempt + or during the pre-retry sleep) all run it, so an abandoned turn never + leaves the endpoint's stream open. + """ + max_attempts = retries + 1 + for attempt in range(1, max_attempts + 1): + emitted = False + stream = llm.chat_stream(messages, tools=tools) + try: + async for piece in stream: + emitted = True + yield piece + except LLMError as e: + if emitted: + # Locked A2: tokens already flowed — the failure is + # terminal, never redo a partial answer. + raise + if attempt >= max_attempts: + # Retries exhausted — the API layer turns this into the + # terminal error frame. + raise + logger.warning( + "llm stream failed before the first piece (attempt %d/%d) — " + "retrying in %.1fs: %s", + attempt, + max_attempts, + delay, + e, + ) + yield RetryPiece(attempt + 1, max_attempts) + await asyncio.sleep(delay) + else: + # A fully consumed attempt — the turn is done; the loop must + # NOT open another stream for an attempt that never failed. + return + finally: + # Phase 48 teardown for THIS attempt's stream (a no-op once the + # stream already ended; the real close on a consumer abandon). + await stream.aclose() + + async def check_models(llm: LLMClient) -> None: """Verify the models a sync needs (embed + summary) before any expensive work; raise ModelUnavailableError naming the model. diff --git a/app/schemas.py b/app/schemas.py index 12bb6c1..7ac1ab4 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -108,6 +108,30 @@ class ChatErrorEvent(BaseModel): detail: str +class ChatRetryEvent(BaseModel): + """SSE retry event: an LLM request is restarted before the first token + (phase 67, owner-locked 2026-09-01). + + Sibling of :class:`ChatErrorEvent`, but transient — the client shows a + live status on the existing ``#send-status`` line (locked A4: + "Communication interrupted — retrying (n of N)…") and the send button + stays the Stop control; it never flips the state machine to error. It + is only ever sent when the failed attempt had NOT streamed a single + output frame yet (locked A2: no thinking/tool/delta emitted) — once + tokens are flowing, a failure is terminal (the ``error`` frame) and + this event cannot appear. + + ``attempt`` is the 1-based number of the attempt the endpoint is about + to try next (what the endpoint sends — the first failure of a + 4-attempt budget carries ``attempt=2``); ``max_attempts`` is the total + attempt budget (``llm_retries + 1``). + """ + + type: Literal["retry"] = "retry" + attempt: int + max_attempts: int + + class DocSummary(BaseModel): """One indexed document as shown on the Sources page / API.""" diff --git a/frontend/assets/app.js b/frontend/assets/app.js index 82cc04b..796a839 100644 --- a/frontend/assets/app.js +++ b/frontend/assets/app.js @@ -75,6 +75,24 @@ * optional `tools: [{name, argument}]` array next to `thinking` and * restore re-renders the lines (phase 14 convention). * + * LLM retry status (phase 67, TODO.md L3): if the endpoint dies BEFORE + * the first frame of an LLM request lands, the server restarts that + * request (up to BOR_LLM_RETRIES retries, BOR_LLM_RETRY_DELAY seconds + * apart — a flat delay, no backoff, owner-locked A3) and streams one + * `retry` SSE frame per wait. The handler treats it with the same + * status pattern as the `tool` frames: the 120s guard clears (a frame + * arrived) and the EXISTING #send-status live region + the + * typing-indicator aria-label read the owner-locked copy + * "Communication interrupted — retrying (n of N)…" (n = the attempt + * about to be tried, N = the configured total). Nothing else changes — + * no bubble, no tool line, no banner, no UI-state change: it is a + * transient status that the next thinking/tool/delta frame replaces + * through its own branch. A retry arrives only before the request's + * first piece (locked A2), so it never races a partial answer; the + * status gate still covers BOTH live states (thinking AND streaming) + * because a LATER agent round may restart while an earlier round + * already emitted content (the rare content+tool-call stream). + * * Steering notes (phase 15) let the owner tune how Brain answers: a * "Tune" button under every completed brain bubble (deflected included) * opens an inline form → POST /api/steering → the note is stored in @@ -1927,6 +1945,27 @@ async function runTurn(text, { reask = false } = {}) { } appendToolLine(wrap, name, argument); // No page scroll (phase 42): tool lines never yank the viewport. + } else if (ev.type === "retry") { + // Phase 67 (owner-locked A4): the server restarted the LLM + // request before ANY frame of it reached the client (locked + // A2) — say exactly what is happening on the existing status + // line. Transient status only: no bubble, no tool line, no + // banner, no UI-state change — the next thinking/tool/delta + // frame replaces it through its own branch. The gate covers + // BOTH live states: a LATER agent round may restart while the + // UI already streams (a content+tool-call stream from an + // earlier round). + 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); + } } else if (ev.type === "delta") { acc += ev.text || ""; if (uiState === UI_STATE.thinking) setUiState(UI_STATE.streaming); diff --git a/tests/e2e/conftest.py b/tests/e2e/conftest.py index 788c6e5..0491360 100644 --- a/tests/e2e/conftest.py +++ b/tests/e2e/conftest.py @@ -98,6 +98,15 @@ def app_server(mock_llm: int) -> Iterator[str]: # The production default stays 0.62 (re-tuned against the real # `embed` model's 0.41–0.84 cosine range, PLAN A8). env["BOR_RELEVANCE_THRESHOLD"] = "0.30" + # Phase 67 (LLM retry): the e2e pins the retry MECHANISM with instant + # waits (BOR_LLM_RETRY_DELAY=0 — the 5 s default is unit-pinned via + # tests/unit/test_config.py). BOR_LLM_RETRIES is forced to the code + # default (derived from the class field, never drifts from + # app/config.py) so the exhaustion test relies on the REAL budget and + # an operator's local (gitignored) .env cannot leak a different one + # into the app under test (the phase-61 leak-guard pattern). + env["BOR_LLM_RETRY_DELAY"] = "0" + env["BOR_LLM_RETRIES"] = str(_Settings.model_fields["llm_retries"].default) env.setdefault("BOR_DATABASE_URL", "postgresql+psycopg://reese:reese@localhost:5432/brain_of_reese") # Phase 16: admin auth must be set or create_app() refuses to boot. env["BOR_ADMIN_PASSWORD"] = ADMIN_PASSWORD diff --git a/tests/e2e/mock_llm.py b/tests/e2e/mock_llm.py index 2fe99c7..79fa3e9 100644 --- a/tests/e2e/mock_llm.py +++ b/tests/e2e/mock_llm.py @@ -112,6 +112,36 @@ Implements just enough of the aipi surface: answer; the E2E asks it against an on-topic fixture (HIGH gate) and asserts non-deflection. + Failure injection (phase 67, LLM retry, TODO.md L3) — deterministic + dead-endpoint behavior for the retry E2E suite (``tests/e2e/ +test_llm_retry.py``). The mock is single-conversation per e2e server, so + the sequences are driven by module-level counters that reset per + trigger phrase after the success they guard (a second question with + the same trigger re-drives the sequence from zero): + - user message containing ``fail then answer`` (``RETRY_TRIGGER``): + the first ``RETRY_DEAD_ATTEMPTS`` (2) app-level streaming attempts + respond 500 (JSON body, like a dead proxy) and the third streams + the normal composed answer — 2 = 1 original attempt + 1 retry under + the default ``BOR_LLM_RETRIES=3``, so a suite exercises a real + retry without waiting for the 4-attempt exhaustion. Counted in + APP-LEVEL attempts, not raw HTTP POSTs: while the endpoint stays + dead, the openai SDK's default policy (max_retries=2 — the app's + ``LLMClient`` keeps it) re-POSTs a 500'd streaming request twice + before surfacing the error, so each dead attempt costs exactly 3 + POSTs (``_HTTPS_PER_DEAD_ATTEMPT``). + - user message containing ``always fail`` + (``ALWAYS_FAIL_TRIGGER``): EVERY streaming chat/completions request + responds 500 — the retry-budget exhaustion path (the terminal + error banner in the UI). + - embeddings request whose input contains ``embed fail once`` + (``EMBED_FAIL_TRIGGER``): the FIRST such request responds 500, the + next returns the normal bag-of-words vector — the endpoint's + pre-stream embedding retry loop. Raw httpx on the client side (no + SDK-level retries), so one POST per attempt: the counter is per + POST here, unlike the chat counter above. + Non-streaming requests (document summaries, KB overview) never 500 — + the retry scope is the chat turn only (owner-locked A1). + ``max_tokens`` is honored deterministically (token ≈ whitespace word), like a real endpoint: an answer longer than the cap is truncated. This is what makes the phase-11 truncation regression observable. @@ -129,7 +159,7 @@ import uuid from typing import Any from fastapi import FastAPI -from fastapi.responses import StreamingResponse +from fastapi.responses import JSONResponse, StreamingResponse app = FastAPI() @@ -265,6 +295,76 @@ TABLE_ANSWER = ( "| value-one | value-two | value-three | value-four | value-five |" ) +# --------------------------------------------------------------------------- +# Phase 67 (LLM retry, TODO.md L3): deterministic failure injection +# --------------------------------------------------------------------------- + +#: A user message containing this substring (case-insensitive) gets +#: ``RETRY_DEAD_ATTEMPTS`` dead streaming attempts (500, JSON body) before +#: the normal composed answer streams — 1 original attempt + 1 retry under +#: the default ``BOR_LLM_RETRIES=3`` (see the module docstring). +RETRY_TRIGGER = "fail then answer" +#: App-level attempts the endpoint stays dead for before the answer. +RETRY_DEAD_ATTEMPTS = 2 + +#: A user message containing this substring (case-insensitive) makes +#: EVERY streaming chat/completions request respond 500 — the +#: retry-budget exhaustion path (the terminal error banner in the UI). +ALWAYS_FAIL_TRIGGER = "always fail" + +#: An embeddings request whose input contains this substring +#: (case-insensitive) 500s on its FIRST POST; the next returns the normal +#: bag-of-words vector — the endpoint's pre-stream embedding retry loop. +EMBED_FAIL_TRIGGER = "embed fail once" + +#: One DEAD app-level chat attempt costs exactly this many HTTP POSTs +#: while the endpoint stays down: the openai SDK's default policy +#: (max_retries=2 — the app's ``LLMClient`` keeps it) re-POSTs a 500'd +#: streaming request twice before surfacing the error to +#: ``chat_stream_retried``. The failure counters below therefore count +#: app-level attempts (groups of this size), not raw POSTs — the visible +#: sequence (one SSE ``retry`` frame after each dead attempt, the answer +#: on the third) stays deterministic regardless of the SDK's internal +#: backoff pacing. +_HTTPS_PER_DEAD_ATTEMPT = 3 + +#: Module-level failure counters — the mock is single-conversation per +#: e2e server. Keyed by trigger phrase (reset per trigger): the number +#: of matching POSTs served so far. Each sequence resets after the +#: success it guards, so a second question carrying the same trigger +#: re-drives the failure sequence from zero. +_fail_posts: dict[str, int] = {} + + +def _llm_500(why: str) -> JSONResponse: + """A dead-proxy 500 with a JSON error body (phase 67 injection).""" + return JSONResponse( + status_code=500, + content={ + "error": { + "message": f"upstream connection reset ({why})", + "type": "proxy_error", + } + }, + ) + + +def _bump_fail(key: str) -> int: + n = _fail_posts.get(key, 0) + 1 + _fail_posts[key] = n + return n + + +def _chat_dead(key: str, dead_attempts: int) -> bool: + """Bump *key*'s counter; True while the endpoint stays dead. + + Counted in app-level attempts (see ``_HTTPS_PER_DEAD_ATTEMPT``): the + first ``dead_attempts * _HTTPS_PER_DEAD_ATTEMPT`` POSTs 500 and the + next attempt's first POST streams (the caller resets the counter on + the success). + """ + return _bump_fail(key) <= dead_attempts * _HTTPS_PER_DEAD_ATTEMPT + #: The agent's ``read_document`` tool-result prefix (app.rag.agent #: ``_execute_tool``): ``"Document :\n"``. @@ -656,11 +756,22 @@ def models() -> dict[str, Any]: @app.post("/v1/embeddings") -def embeddings(body: dict[str, Any]) -> dict[str, Any]: +def embeddings(body: dict[str, Any]) -> Any: # dict, or a 500 (phase 67) raw = body.get("input") if isinstance(raw, str): raw = [raw] inputs: list[Any] = list(raw) if isinstance(raw, list) else [] + # Phase 67 (embedding retry): the first embeddings request whose + # input carries the marker 500s; the next returns the normal + # bag-of-words vector (see the module docstring). Raw httpx on the + # client side — no SDK-level retries — so one POST per app attempt: + # the counter is per POST here (unlike the chat counter below). + joined = " ".join(str(t) for t in inputs if isinstance(t, str)).lower() + if EMBED_FAIL_TRIGGER in joined: + n = _bump_fail(EMBED_FAIL_TRIGGER) + if n == 1: + return _llm_500(EMBED_FAIL_TRIGGER) + _fail_posts[EMBED_FAIL_TRIGGER] = 0 # the vector went out — restart data = [ {"object": "embedding", "index": i, "embedding": embed_text(t)} for i, t in enumerate(inputs) @@ -827,6 +938,16 @@ def chat_completions(body: dict[str, Any]) -> Any: # the flow handles streaming requests; a non-streaming marker request # (never issued by the app) falls through to the regular answer. if body.get("stream"): + # Phase 67 (LLM retry): deterministic failure injection — see + # the module docstring. Checked before the marker tool flow: the + # injection markers never combine with the tool-flow markers in + # any suite, and a dead endpoint answers nothing (no flow). + if ALWAYS_FAIL_TRIGGER in user_lower: + return _llm_500(ALWAYS_FAIL_TRIGGER) + if RETRY_TRIGGER in user_lower: + if _chat_dead(RETRY_TRIGGER, RETRY_DEAD_ATTEMPTS): + return _llm_500(RETRY_TRIGGER) + _fail_posts[RETRY_TRIGGER] = 0 # the answer streamed — restart flow = _tool_flow(body) if flow is not None: if flow[0] == "list": diff --git a/tests/e2e/test_llm_retry.py b/tests/e2e/test_llm_retry.py new file mode 100644 index 0000000..c8036d9 --- /dev/null +++ b/tests/e2e/test_llm_retry.py @@ -0,0 +1,506 @@ +"""Phase 67 E2E (Playwright): 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' … if the LLM server stops communicating." +(TODO-derived phase — no story file.) +Run in isolation (DB must be up: ``podman compose up -d db``): + + uv run pytest tests/e2e/test_llm_retry.py -v --no-cov + +MOCK-ONLY suite: ``E2E_REAL_LLM=1`` is not supported — the retry gate is +the deterministic failure injection in ``tests/e2e/mock_llm.py``: + +* ``fail then answer`` (``RETRY_TRIGGER``): the first 2 app-level + streaming attempts 500 (JSON body, like a dead proxy) and the third + streams the normal composed answer — 1 original attempt + 1 retry + under the default ``BOR_LLM_RETRIES=3``. (Each dead app attempt costs + 3 HTTP POSTs — the openai SDK's default 1 + 2 internal retries — so + the mock counts attempts, not POSTs; see the mock docstring.) +* ``always fail`` (``ALWAYS_FAIL_TRIGGER``): every streaming request + 500s — the retry-budget exhaustion path. +* ``embed fail once`` (``EMBED_FAIL_TRIGGER``): the first embeddings + request 500s, the next returns the normal vector — the endpoint's + pre-stream embedding retry loop. + +The e2e app server boots with ``BOR_LLM_RETRY_DELAY=0`` (conftest) so +the retry waits are instant — the suite pins the MECHANISM; the 5 s +default is unit-pinned via ``tests/unit/test_config.py``. +``BOR_LLM_RETRIES`` is forced to its real default (3, conftest) — the +exhaustion test relies on the real budget: 4 attempts total, so the +last ``retry`` frame reads "(4 of 4)". + +KB seed (phase-37 direct-seed pattern): ONE fixture document +(``homelab/kubernetes.md``), one chunk carrying the mock's own +bag-of-words embedding. The marker questions were verified against this +exact seed (``plan_turn``, E2E threshold 0.30): + +* the "sourdough" questions (DEFLECT_Q / EXHAUST_Q) share no FTS token + with the document (cosine 0.000, fts 0) → LOW → the DEFLECTED path + (``chat_stream_retried`` directly, no agent round); +* the "kubernetes cluster" questions (GROUNDED_Q / EMBED_Q) FTS-hit the + document (cosine 0.171, fts 1) → HIGH → the GROUNDED path (the agent + loop's per-round retry). + +Test → source mapping: +1. ``test_dead_then_recovered_deflected`` — LOW turn: the recorded + ``#send-status`` values contain "Communication interrupted — + retrying (2 of 4)…" and "(3 of 4)…" (the owner-locked A4 copy), the + wire carries the two ``retry`` frames ahead of the first delta, the + deflected answer settles, and no error banner appears. +2. ``test_dead_then_recovered_grounded`` — HIGH turn: the same status + + wire assertions; the grounded answer completes with the source chip + (the agent round retried, the turn is intact). +3. ``test_embedding_retry_completes`` — ``embed fail once``: the + pre-stream embedding retry is visible to the UI (a ``retry`` status + before any answer frame) and the turn completes normally. +4. ``test_exhaustion_lands_on_the_error_banner`` — ``always fail``: + after the 4th dead attempt the EXISTING terminal error banner + appears (role=alert, the "dropped the connection" copy), the last + retrying status is the highest attempt — "(4 of 4)…" — and the send + button re-enables (the banner path settles the state machine). +""" +from __future__ import annotations + +import hashlib +import json +import re +import time +from datetime import UTC, datetime +from pathlib import Path + +from playwright.sync_api import Page, expect +from sqlalchemy import select, text +from sqlalchemy.orm import Session + +from app.db import SessionLocal +from app.models import Chunk, Document, QueryLog +from tests.e2e.mock_llm import embed_text + +REPO = Path(__file__).resolve().parents[2] + +# -------------------------------------------------------------------------- +# Seed + questions (see the module docstring for the gate verification) +# -------------------------------------------------------------------------- + +SEED_SOURCE = "docs" +SEED_PATH = "homelab/kubernetes.md" +SEED_SP = f"{SEED_SOURCE}/{SEED_PATH}" +KUB_CONTENT = (REPO / "tests" / "fixtures" / "docs" / SEED_PATH).read_text() + +#: LOW turn (deflected path) + the retry trigger: no FTS overlap with the +#: seeded document, cosine 0.000 → the honesty gate deflects. +DEFLECT_Q = "How do I bake sourdough bread? fail then answer" +#: HIGH turn (grounded path) + the retry trigger: FTS-hits the seeded +#: document → the agent loop runs (and its single round is retried). +GROUNDED_Q = "How is my Kubernetes cluster set up? fail then answer" +#: HIGH turn + the embedding trigger: the pre-stream embedding 500s once. +EMBED_Q = "How is my Kubernetes cluster set up? embed fail once" +#: LOW turn + the exhaustion trigger: every streaming request 500s. +EXHAUST_Q = "How do I bake sourdough bread? always fail" + +#: The conftest forces BOR_LLM_RETRIES to its default (3) → 4 attempts +#: total; the attempt math in the assertions is fixed by that budget. +MAX_ATTEMPTS = 4 + + +def _retry_status(attempt: int) -> str: + """The owner-locked A4 copy for *attempt* (1-based) of MAX_ATTEMPTS.""" + return f"Communication interrupted — retrying ({attempt} of {MAX_ATTEMPTS})…" + + +MOCK_ANSWER_MARKER = "Deterministic mock answer for E2E" +DEFLECT_PHRASE = r"haven't done anything like that" +ERROR_COPY = "The chat model dropped the connection — try again?" + +# -------------------------------------------------------------------------- +# DB seeding (TRUNCATE-then-seed, cf. test_agent_document_tools.py) +# -------------------------------------------------------------------------- + + +def _seed(db: Session) -> None: + """The single fixture document (see the module docstring).""" + md = Document( + source=SEED_SOURCE, + path=SEED_PATH, + full_path=f"/tmp/{SEED_PATH}", + title="Kubernetes", + content=KUB_CONTENT, + content_hash=hashlib.sha256(KUB_CONTENT.encode()).hexdigest(), + indexed_at=datetime.now(UTC), + ) + db.add(md) + db.flush() + # One chunk carrying the mock's own embedding → genuine token overlap + # for the grounded questions (the FTS path carries them to HIGH). + db.add( + Chunk( + document_id=md.id, + position=0, + content=KUB_CONTENT, + embedding=embed_text(KUB_CONTENT), + ) + ) + + +def _reset_db() -> None: + """Truncate the KB (plus the prompt-shaping tables), then re-seed. + + ``steering_notes`` / ``kb_overview`` are truncated too, so the + prompts are exactly ```` + ```` (+ ```` + on HIGH) regardless of leftovers from other suites — byte-stable + prompts, byte-stable answers. + """ + with SessionLocal() as db: + db.execute( + text("TRUNCATE chunks, documents, query_log, steering_notes, kb_overview") + ) + db.commit() + _seed(db) + db.commit() + + +def _last_query_log() -> QueryLog: + with SessionLocal() as db: + rows = db.scalars(select(QueryLog)).all() + assert len(rows) == 1, f"expected exactly one query_log row, got {len(rows)}" + return rows[0] + + +# -------------------------------------------------------------------------- +# Page hooks (the #send-status recorder + SSE capture, the phase-37 +# pattern from test_agent_document_tools.py) +# -------------------------------------------------------------------------- + +#: Records every value #send-status takes during the turn (a +#: MutationObserver on the element), so the in-flight status sequence — +#: including the transient phase-67 "retrying" states — is captured +#: deterministically (no polling race). +STATUS_RECORDER = """ +() => { + if (window.__statusesInstalled) return; + window.__statusesInstalled = true; + window.__statuses = []; + const el = document.querySelector('#send-status'); + if (!el) return; + const rec = (v) => { + const l = window.__statuses; + if (!l.length || l[l.length - 1] !== v) l.push(v); + }; + rec(el.textContent); + new MutationObserver(() => rec(el.textContent)).observe(el, { + childList: true, + subtree: true, + }); +} +""" + +#: Captures the raw SSE ``data:`` payloads of the /api/chat stream +#: (a response clone read in the background) — wire-level assertions +#: for the ``retry`` frames, independent of the UI rendering. +SSE_HOOK = """ +() => { + if (window.__sseInstalled) return; + window.__sseInstalled = true; + window.__sseFrames = []; + const origFetch = window.fetch; + window.fetch = async function (...args) { + const res = await origFetch.apply(this, args); + try { + const url = typeof args[0] === 'string' ? args[0] : args[0].url; + if (url.includes('/api/chat')) { + res.clone().text().then((bodyText) => { + for (const block of bodyText.split('\\n\\n')) { + const line = block.trim(); + if (line.startsWith('data: ')) { + window.__sseFrames.push(line.slice(6)); + } + } + }); + } + } catch (e) { /* non-clonable responses: ignored */ } + return res; + }; +} +""" + + +def _install_page_hooks(page: Page) -> None: + """Install both hooks on the loaded page (post-goto, pre-submit). + + The fetch wrapper only needs to be in place before the turn's + ``fetch("/api/chat")`` call; the observer needs the rendered + ``#send-status``. (``add_init_script`` would not do — it binds to + the NEXT navigation, and the story page is navigated exactly once.) + """ + page.evaluate(SSE_HOOK) + page.evaluate(STATUS_RECORDER) + + +def _frames(page: Page, terminal: str = "done") -> list[dict]: + """The captured SSE frames, once the *terminal* frame lands. + + The hook reads ``res.clone().text()`` in a background promise that + resolves right after the stream closes — poll briefly until the + terminal (``done``, or ``error`` for the exhaustion test) lands. + """ + deadline = time.monotonic() + 30.0 + while True: + raw = page.evaluate("() => window.__sseFrames || []") + parsed = [json.loads(line) for line in raw if line] + if any(f.get("type") == terminal for f in parsed): + return parsed + if time.monotonic() > deadline: + raise AssertionError( + f"SSE hook captured no `{terminal}` frame (frames so far: " + f"{len(parsed)}) — hook install failed?" + ) + time.sleep(0.05) + + +def _retry_frames(frames: list[dict]) -> list[dict]: + return [f for f in frames if f.get("type") == "retry"] + + +def _assert_retries_before_first_delta(frames: list[dict], retries: list[dict]) -> None: + """The wire contract (locked A2): every retry frame precedes the + first answer delta — a retry can only restart a request that never + streamed a frame.""" + assert retries, "no retry frames on the wire" + first_delta = next(i for i, f in enumerate(frames) if f.get("type") == "delta") + assert all( + i < first_delta for i, f in enumerate(frames) if f.get("type") == "retry" + ) + + +def _assert_no_error_frames(frames: list[dict]) -> None: + assert not [f for f in frames if f.get("type") == "error"] + + +def _submit(page: Page, question: str) -> None: + page.fill("#message-input", question) + page.click("#send-btn") + # The user bubble lands synchronously with the submit handler. + expect(page.locator(".msg.user .bubble").last).to_contain_text(question) + + +def _wait_settled(page: Page) -> None: + """The turn is complete: answer text in the bubble, button recovered. + + Phase 48: the label assertion carries the settle wait with an + explicit timeout — the in-flight button is the enabled Stop control + (never disabled), so ``to_be_enabled`` no longer blocks until the + turn settles, and Playwright expect's default (5s) does not inherit + the page default.""" + expect(page.locator(".msg.brain .bubble").last).not_to_have_text("", timeout=30_000) + expect(page.locator("#send-btn")).to_be_enabled(timeout=30_000) + expect(page.locator("#send-label")).to_have_text("Send", timeout=30_000) + + +def _assert_no_error_banner(page: Page) -> None: + """A retried turn settles through the normal done path — never the + red role=alert error banner (the KB-offline banner is a separate, + health-driven state the db_ready fixture keeps away).""" + banner = page.locator("#kb-banner") + expect(banner).to_be_hidden() + expect(banner).not_to_have_attribute("role", "alert") + expect(banner).not_to_have_class(re.compile(r"is-error")) + + +# -------------------------------------------------------------------------- +# 1. Dead-then-recovered, DEFLECTED path (LOW turn): the status line +# shows the live "retrying" copy and the answer still completes +# -------------------------------------------------------------------------- + + +def test_dead_then_recovered_deflected( + page: Page, app_url: str, mock_llm: int, db_ready: None +) -> None: + _reset_db() + page.set_default_timeout(30_000) + page.goto(app_url) + _install_page_hooks(page) + + _submit(page, DEFLECT_Q) + _wait_settled(page) + + # The live status (owner-locked A4 copy) was recorded for BOTH + # restarts: attempt 2 (after the original attempt died) and attempt + # 3 (after the first retry died) — attempt 4 never needed to start. + statuses = page.evaluate("() => window.__statuses") + assert _retry_status(2) in statuses, statuses + assert _retry_status(3) in statuses, statuses + + # Wire: exactly the two retry frames (attempt = the attempt about to + # be tried, 1-based; max_attempts = the forced-default budget of 4), + # both ahead of the first delta, and no error frame anywhere. + frames = _frames(page) + retries = _retry_frames(frames) + assert retries == [ + {"type": "retry", "attempt": 2, "max_attempts": MAX_ATTEMPTS}, + {"type": "retry", "attempt": 3, "max_attempts": MAX_ATTEMPTS}, + ], retries + _assert_retries_before_first_delta(frames, retries) + _assert_no_error_frames(frames) + done = next(f for f in frames if f.get("type") == "done") + assert done["deflected"] is True + + # The deflected answer settled normally — no error banner. + last = page.locator(".msg.brain").last + expect(last).to_have_class(re.compile(r"is-deflected")) + expect(last.locator(".bubble")).to_contain_text( + re.compile(DEFLECT_PHRASE, re.IGNORECASE) + ) + _assert_no_error_banner(page) + + row = _last_query_log() + assert row.question == DEFLECT_Q + assert row.deflected is True + + +# -------------------------------------------------------------------------- +# 2. Dead-then-recovered, GROUNDED path (HIGH turn): the agent round +# retried and the answer completes with its sources +# -------------------------------------------------------------------------- + + +def test_dead_then_recovered_grounded( + page: Page, app_url: str, mock_llm: int, db_ready: None +) -> None: + _reset_db() + page.set_default_timeout(30_000) + page.goto(app_url) + _install_page_hooks(page) + + _submit(page, GROUNDED_Q) + _wait_settled(page) + + # Same status sequence as the deflected path — the per-round retry + # (agent loop, task 03) surfaces through the same status line. + statuses = page.evaluate("() => window.__statuses") + assert _retry_status(2) in statuses, statuses + assert _retry_status(3) in statuses, statuses + + frames = _frames(page) + retries = _retry_frames(frames) + assert retries == [ + {"type": "retry", "attempt": 2, "max_attempts": MAX_ATTEMPTS}, + {"type": "retry", "attempt": 3, "max_attempts": MAX_ATTEMPTS}, + ], retries + _assert_retries_before_first_delta(frames, retries) + _assert_no_error_frames(frames) + done = next(f for f in frames if f.get("type") == "done") + assert done["deflected"] is False + assert [(s["source"], s["path"]) for s in done["sources"]] == [ + (SEED_SOURCE, SEED_PATH) + ] + + # The grounded answer completed with the source chip — the agent + # round retried and the turn is intact. + bubble = page.locator(".msg.brain .bubble").last + expect(bubble).to_contain_text(GROUNDED_Q) + expect(bubble).to_contain_text(MOCK_ANSWER_MARKER) + chips = page.locator(".msg.brain .source-chip") + expect(chips).to_have_count(1) + expect(chips.first).to_contain_text(SEED_SP) + _assert_no_error_banner(page) + + row = _last_query_log() + assert row.question == GROUNDED_Q + assert row.deflected is False + assert row.sources == SEED_SP + + +# -------------------------------------------------------------------------- +# 3. Embedding retry: the pre-stream embedding loop is visible to the +# UI (a retry status before any answer) and the turn completes +# -------------------------------------------------------------------------- + + +def test_embedding_retry_completes( + page: Page, app_url: str, mock_llm: int, db_ready: None +) -> None: + _reset_db() + page.set_default_timeout(30_000) + page.goto(app_url) + _install_page_hooks(page) + + _submit(page, EMBED_Q) + _wait_settled(page) + + # The embedding step runs BEFORE the honesty gate and the answer + # stream, so its single retry is the ONLY retry frame of the turn — + # and the UI saw it as the live status (no answer frame had landed). + statuses = page.evaluate("() => window.__statuses") + assert _retry_status(2) in statuses, statuses + + frames = _frames(page) + retries = _retry_frames(frames) + assert retries == [ + {"type": "retry", "attempt": 2, "max_attempts": MAX_ATTEMPTS}, + ], retries + _assert_retries_before_first_delta(frames, retries) + _assert_no_error_frames(frames) + done = next(f for f in frames if f.get("type") == "done") + assert done["deflected"] is False + + # The turn completed normally with the grounded answer + chip. + bubble = page.locator(".msg.brain .bubble").last + expect(bubble).to_contain_text(MOCK_ANSWER_MARKER) + chips = page.locator(".msg.brain .source-chip") + expect(chips).to_have_count(1) + expect(chips.first).to_contain_text(SEED_SP) + _assert_no_error_banner(page) + + +# -------------------------------------------------------------------------- +# 4. Exhaustion: a dead endpoint burns the whole budget, then the +# EXISTING terminal error banner lands and the composer recovers +# -------------------------------------------------------------------------- + + +def test_exhaustion_lands_on_the_error_banner( + page: Page, app_url: str, mock_llm: int, db_ready: None +) -> None: + _reset_db() + page.set_default_timeout(30_000) + page.goto(app_url) + _install_page_hooks(page) + + _submit(page, EXHAUST_Q) + + # After the 4th dead attempt the turn dies: the existing terminal + # error banner (role=alert) with the existing copy, and the send + # button re-enabled (the banner path settles the state machine). + expect(page.locator("#kb-banner")).to_have_attribute( + "role", "alert", timeout=60_000 + ) + expect(page.locator("#kb-banner")).to_contain_text(ERROR_COPY) + expect(page.locator("#send-btn")).to_be_enabled(timeout=30_000) + expect(page.locator("#send-label")).to_have_text("Send", timeout=30_000) + + # The live status climbed the whole budget: the LAST retrying status + # is the highest attempt — "(4 of 4)…" (no attempt 5 exists). + statuses = page.evaluate("() => window.__statuses") + retry_statuses = [s for s in statuses if "retrying" in s] + assert retry_statuses, statuses + assert retry_statuses[-1] == _retry_status(MAX_ATTEMPTS), statuses + + # Wire: the three retry frames (attempts 2, 3, 4 of 4), then the + # terminal error frame as the LAST event — no done, no delta. + frames = _frames(page, terminal="error") + retries = _retry_frames(frames) + assert retries == [ + {"type": "retry", "attempt": a, "max_attempts": MAX_ATTEMPTS} + for a in (2, 3, 4) + ], retries + assert frames[-1]["type"] == "error" + assert ERROR_COPY in frames[-1]["detail"] + assert not [f for f in frames if f.get("type") == "done"] + assert not [f for f in frames if f.get("type") == "delta"] + + # No answer bubble was ever rendered (no frame ever streamed a + # token) — the user bubble is the only message in the DOM. + expect(page.locator("#messages > .msg.brain")).to_have_count(0) + expect(page.locator(".msg.user .bubble")).to_have_count(1) diff --git a/tests/integration/test_chat_api.py b/tests/integration/test_chat_api.py index 54c5c68..e542bde 100644 --- a/tests/integration/test_chat_api.py +++ b/tests/integration/test_chat_api.py @@ -61,6 +61,8 @@ class FakeRagLLM: stream_error: Exception | None = None, fail_mid_stream: bool = False, tool_script: list[list[StreamPiece | ToolCallPiece]] | None = None, + embed_fail_count: int = 0, + stream_fail_count: int = 0, ) -> None: self.settings = Settings(_env_file=None) # pyright: ignore[reportCallIssue] self.embed_batches = 0 @@ -69,6 +71,14 @@ class FakeRagLLM: self.embed_error = embed_error self.stream_error = stream_error self.fail_mid_stream = fail_mid_stream + #: Phase 67: the first N ``embed_one`` calls raise an + #: ``EmbeddingError`` (then succeed) — a dead-then-recovered + #: embeddings endpoint for the retry loop. + self.embed_fail_count = embed_fail_count + #: Phase 67: the first N ``chat_stream`` requests die with an + #: ``LLMError`` BEFORE any piece (then succeed) — a dead-then- + #: recovered answer endpoint for the pre-first-piece retry rule. + self.stream_fail_count = stream_fail_count self.question_embeds: list[str] = [] self.seen_messages: list[list[dict[str, str]]] = [] #: Every request's ``tools`` value (phase 37) — ``None`` is the @@ -100,6 +110,10 @@ class FakeRagLLM: async def embed_one(self, text: str) -> list[float]: if self.embed_error is not None: raise self.embed_error + if self.embed_fail_count > 0: + self.embed_fail_count -= 1 + self.question_embeds.append(text) + raise EmbeddingError("simulated embeddings endpoint failure") self.question_embeds.append(text) return _token_vec(text) @@ -118,6 +132,9 @@ class FakeRagLLM: self.seen_tools.append(tools) if self.stream_error is not None: raise self.stream_error + if self.stream_fail_count > 0: + self.stream_fail_count -= 1 + raise LLMError("simulated pre-piece endpoint failure") if tools is not None and self.tool_script: for piece in self.tool_script.pop(0): yield piece @@ -388,17 +405,33 @@ def test_chat_empty_kb_streams_empty_sources(client, db) -> None: assert row.sources == "" -def test_chat_embed_failure_yields_error_event(client, db, seeded_kb: FakeRagLLM) -> None: +def test_chat_embed_failure_yields_error_event( + client, db, seeded_kb: FakeRagLLM, monkeypatch: pytest.MonkeyPatch +) -> None: + """Phase 67: a dead embeddings endpoint retries on the configured + budget — one ``retry`` frame per restart (the attempt about to be + tried, 1-based) — and settles on the existing terminal error frame; + no query_log row. Zero delay keeps the exhaustion path fast.""" broken = FakeRagLLM(embed_error=EmbeddingError("embeddings endpoint down")) + live = get_settings() + monkeypatch.setattr( + chat_api, "get_settings", lambda: _retry_settings(live) + ) fastapi_app.dependency_overrides[chat_api.get_llm] = lambda: broken try: _, _, frames = _stream_chat(client, QUESTION) finally: fastapi_app.dependency_overrides.clear() - assert len(frames) == 1 - assert frames[0]["type"] == "error" - assert "embedding" in frames[0]["detail"] + retries = live.llm_retries + assert [f["type"] for f in frames] == ["retry"] * retries + ["error"] + assert [f["attempt"] for f in frames if f["type"] == "retry"] == list( + range(2, retries + 2) + ) + assert all( + f["max_attempts"] == retries + 1 for f in frames if f["type"] == "retry" + ) + assert "embedding" in frames[-1]["detail"] assert db.scalars(select(QueryLog)).all() == [] @@ -416,11 +449,19 @@ def test_chat_mid_stream_failure_yields_error_after_partial_deltas(client, db) - assert db.scalars(select(QueryLog)).all() == [] -def test_error_event_matches_contract_shape(client, db, seeded_kb) -> None: +def test_error_event_matches_contract_shape( + client, db, seeded_kb, monkeypatch: pytest.MonkeyPatch +) -> None: """The SSE error event (PLAN §4) is exactly ``{type, detail}`` — the client's loading-feedback state machine (phase 06) keys off this shape - to flip to the error state and re-enable the send button.""" + to flip to the error state and re-enable the send button. + ``llm_retries=0`` keeps this a single-attempt turn: the contract under + test is the error frame itself, not the phase-67 retry loop.""" broken = FakeRagLLM(embed_error=EmbeddingError("embeddings endpoint down")) + live = get_settings() + monkeypatch.setattr( + chat_api, "get_settings", lambda: _retry_settings(live, llm_retries=0) + ) fastapi_app.dependency_overrides[chat_api.get_llm] = lambda: broken try: _, _, frames = _stream_chat(client, QUESTION) @@ -718,3 +759,156 @@ def test_tool_execution_db_failure_yields_error_event( assert frames[0]["name"] == "list_documents" assert "offline mid-question" in frames[1]["detail"] assert db.scalars(select(QueryLog)).all() == [] # no row for a failed turn + + +# ---------- phase 67: LLM retries before the first token ---------- + + +def _retry_settings(live: Settings, **overrides: Any) -> Settings: + """Settings for the retry tests: the live (mock-calibrated) threshold + plus the phase-67 knobs, with a ZERO delay so the suite never sleeps. + (The 5 s default is unit-pinned in ``tests/unit/test_config.py``.)""" + kwargs: dict[str, Any] = { + "relevance_threshold": live.relevance_threshold, + "llm_retry_delay": 0.0, + } + kwargs.update(overrides) + return Settings(_env_file=None, **kwargs) # pyright: ignore[reportCallIssue] + + +def test_embed_failure_retries_then_turn_completes( + client, + db, + seeded_kb: FakeRagLLM, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """A dead-then-recovered embeddings endpoint: one SSE ``retry`` frame + (the attempt about to be tried, 1-based) ahead of the normal answer + frames; the turn completes and the per-turn log line counts the + retry (``retries=1``).""" + flaky = FakeRagLLM(embed_fail_count=1) + live = get_settings() + monkeypatch.setattr( + chat_api, "get_settings", lambda: _retry_settings(live, llm_retries=1) + ) + fastapi_app.dependency_overrides[chat_api.get_llm] = lambda: flaky + try: + caplog.set_level(logging.INFO, logger="app.chat") + _, _, frames = _stream_chat(client, QUESTION) + finally: + fastapi_app.dependency_overrides.clear() + + assert frames[0] == {"type": "retry", "attempt": 2, "max_attempts": 2} + assert not any(f["type"] == "error" for f in frames) + deltas = [f for f in frames if f["type"] == "delta"] + assert len(deltas) >= 2 + assert "".join(d["text"] for d in deltas) == flaky.answer + assert frames[-1]["type"] == "done" + lines = [r.getMessage() for r in caplog.records if "question=" in r.getMessage()] + assert lines and "retries=1" in lines[-1] + + +def test_embed_failure_exhausts_retries_then_terminal_error( + client, db, seeded_kb: FakeRagLLM, monkeypatch: pytest.MonkeyPatch +) -> None: + """A dead embeddings endpoint (``llm_retries=2`` → 3 attempts): one + ``retry`` frame per restart (attempts 2 and 3 of 3), then the + EXISTING terminal error frame — the copy is unchanged, no query_log + row.""" + dead = FakeRagLLM(embed_fail_count=99) # every attempt fails + live = get_settings() + monkeypatch.setattr( + chat_api, "get_settings", lambda: _retry_settings(live, llm_retries=2) + ) + fastapi_app.dependency_overrides[chat_api.get_llm] = lambda: dead + try: + _, _, frames = _stream_chat(client, QUESTION) + finally: + fastapi_app.dependency_overrides.clear() + + assert [f["type"] for f in frames] == ["retry", "retry", "error"] + assert [f["attempt"] for f in frames if f["type"] == "retry"] == [2, 3] + assert all(f["max_attempts"] == 3 for f in frames if f["type"] == "retry") + assert "embedding" in frames[-1]["detail"] + assert db.scalars(select(QueryLog)).all() == [] + + +def test_deflected_stream_retries_before_the_first_piece( + client, + db, + seeded_kb: FakeRagLLM, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """Deflected answer stream: the first attempt dies before any piece, + the restart streams — a ``retry`` frame ahead of the deltas, the + request restarted with the same messages (no tools key), and the + per-turn log line counts the retry.""" + flaky = FakeRagLLM(stream_fail_count=1) + live = get_settings() + monkeypatch.setattr( + chat_api, "get_settings", lambda: _retry_settings(live, llm_retries=2) + ) + fastapi_app.dependency_overrides[chat_api.get_llm] = lambda: flaky + try: + caplog.set_level(logging.INFO, logger="app.chat") + _, _, frames = _stream_chat(client, OFF_TOPIC) + finally: + fastapi_app.dependency_overrides.clear() + + assert frames[0] == {"type": "retry", "attempt": 2, "max_attempts": 3} + rest = frames[1:] + assert all(f["type"] in ("delta", "done") for f in rest) + assert "".join(f["text"] for f in rest if f["type"] == "delta") == flaky.answer + assert rest[-1]["type"] == "done" and rest[-1]["deflected"] is True + assert len(flaky.seen_messages) == 2 # the request was restarted + assert flaky.seen_tools == [None, None] # …byte-identical (no tools key) + lines = [r.getMessage() for r in caplog.records if "question=" in r.getMessage()] + assert lines and "retries=1" in lines[-1] + + +def test_deflected_stream_failure_after_first_frame_is_terminal( + client, db, seeded_kb: FakeRagLLM, monkeypatch: pytest.MonkeyPatch +) -> None: + """Locked A2: a stream failure AFTER the first output frame is + terminal — no ``retry`` frame, the existing error copy, no row (a + partial answer is never redone).""" + broken = FakeRagLLM(fail_mid_stream=True) + live = get_settings() + monkeypatch.setattr( + chat_api, "get_settings", lambda: _retry_settings(live, llm_retries=3) + ) + fastapi_app.dependency_overrides[chat_api.get_llm] = lambda: broken + try: + _, _, frames = _stream_chat(client, OFF_TOPIC) + finally: + fastapi_app.dependency_overrides.clear() + + assert [f["type"] for f in frames] == ["delta", "error"] + assert not any(f["type"] == "retry" for f in frames) + assert "dropped the connection" in frames[1]["detail"] + assert db.scalars(select(QueryLog)).all() == [] + + +def test_zero_retries_keep_the_pre_phase_wire_shape( + client, db, seeded_kb: FakeRagLLM, monkeypatch: pytest.MonkeyPatch +) -> None: + """The ``BOR_LLM_RETRIES=0`` kill switch: one attempt, the existing + terminal error frame, no ``retry`` frames — the pre-phase-67 + byte-identical wire shape.""" + broken = FakeRagLLM(embed_error=EmbeddingError("embeddings endpoint down")) + live = get_settings() + monkeypatch.setattr( + chat_api, "get_settings", lambda: _retry_settings(live, llm_retries=0) + ) + fastapi_app.dependency_overrides[chat_api.get_llm] = lambda: broken + try: + _, _, frames = _stream_chat(client, QUESTION) + finally: + fastapi_app.dependency_overrides.clear() + + assert len(frames) == 1 + assert frames[0]["type"] == "error" + assert "embedding" in frames[0]["detail"] + assert not any(f["type"] == "retry" for f in frames) diff --git a/tests/unit/test_agent.py b/tests/unit/test_agent.py index de712b1..031796d 100644 --- a/tests/unit/test_agent.py +++ b/tests/unit/test_agent.py @@ -8,15 +8,21 @@ removed the per-tool budgets, the assistant/tool message history), the kill switch (``agent_max_rounds=0`` single-call path), the round cap forcing a final no-tools answer (an always-calling stream and an always-rejected stream), re-lists and multi-reads executing without -budgets, dedupe, unknown tool / missing args / unknown path, and the -```` prompt section (HIGH only). +budgets, dedupe, unknown tool / missing args / unknown path, the +```` prompt section (HIGH only), and the phase-67 per-round +retries (a dead-then-recovered round restarts before its first piece +with a ``RetryPiece``; a mid-stream drop stays terminal — locked A2; +the forced final no-tools call retries too; ``llm_retries=0`` is one +plain attempt; retries are invisible to the round cap; consumer abandon +mid-retry-sleep leaks nothing). """ from __future__ import annotations import asyncio import json +import logging import uuid -from collections.abc import AsyncIterator +from collections.abc import AsyncGenerator, AsyncIterator from copy import deepcopy from typing import Any, cast @@ -31,7 +37,7 @@ from app.rag.agent import ( AgentHolder, run_agent, ) -from app.rag.llm import LLMClient, StreamPiece, ToolCallPiece +from app.rag.llm import LLMClient, LLMError, RetryPiece, StreamPiece, ToolCallPiece from app.rag.prompts import TOOLS_SECTION, _base, build_deflect_prompt, build_high_prompt @@ -73,12 +79,12 @@ class ScriptedLLM: async def _run( - llm: ScriptedLLM, + llm: ScriptedLLM | FailingLLM, holder: AgentHolder, settings: Settings, seed_docs: list[Document] | None = None, -) -> list[StreamPiece | ToolCallPiece]: - out: list[StreamPiece | ToolCallPiece] = [] +) -> list[StreamPiece | ToolCallPiece | RetryPiece]: + out: list[StreamPiece | ToolCallPiece | RetryPiece] = [] async for piece in run_agent( cast("LLMClient", llm), cast("Session", None), @@ -541,6 +547,300 @@ def test_read_document_missing_arguments_refused( assert llm.requests[1][1] == AGENT_TOOLS +# ---------- retries inside the agent loop (phase 67, locked A2) ---------- + + +class FailingLLM: + """A scripted fake whose Nth ``chat_stream`` call yields pieces and + then raises (phase 67): ``attempts`` is a list of ``(pieces, error)`` + — an error after zero pieces = "the endpoint died before the first + token"; after some pieces = a mid-stream drop. Records every + request's messages/tools and the indices of the attempts whose stream + teardown ran (``closed``).""" + + def __init__( + self, + attempts: list[tuple[list[StreamPiece | ToolCallPiece], Exception | None]], + ) -> None: + self.attempts = list(attempts) + self.requests: list[tuple[list[dict[str, Any]], list[dict[str, Any]] | None]] = [] + #: Indices of attempts whose stream teardown has run. + self.closed: list[int] = [] + + def chat_stream( + self, + messages: list[dict[str, str]], + tools: list[dict[str, Any]] | None = None, + ) -> AsyncIterator[StreamPiece | ToolCallPiece]: + index = len(self.requests) + pieces, error = ( + self.attempts[index] + if index < len(self.attempts) + else ([], LLMError("script exhausted")) + ) + self.requests.append( + (deepcopy(messages), deepcopy(tools) if tools is not None else None) + ) + return self._attempt(index, pieces, error) + + async def _attempt( + self, + index: int, + pieces: list[StreamPiece | ToolCallPiece], + error: Exception | None, + ) -> AsyncIterator[StreamPiece | ToolCallPiece]: + try: + for piece in pieces: + yield piece + if error is not None: + raise error + finally: + self.closed.append(index) + + +def _record_sleeps(monkeypatch: pytest.MonkeyPatch) -> list[float]: + """Monkeypatch ``asyncio.sleep`` (what ``chat_stream_retried`` awaits + for the flat pre-retry delay) and record every awaited delay.""" + sleeps: list[float] = [] + + async def fake_sleep(seconds: float) -> None: + sleeps.append(seconds) + + monkeypatch.setattr(asyncio, "sleep", fake_sleep) + return sleeps + + +def test_round_retried_before_first_piece( + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """A tool round that dies before its first piece is restarted with the + same messages: the stream carries a RetryPiece BEFORE the tool call, + the tool executes, the final answer streams, and the per-call log line + is still emitted exactly once (retries are invisible to the loop).""" + monkeypatch.setattr(agent, "list_catalog", lambda db: [("S", "a.md", "A")]) + holder = AgentHolder() + llm = FailingLLM( + [ + ([], LLMError("connection refused")), + ([ToolCallPiece(id="call_1", name="list_documents", arguments={})], None), + ([StreamPiece("content", "Done!")], None), + ] + ) + sleeps = _record_sleeps(monkeypatch) + with caplog.at_level(logging.INFO, logger="app.agent"): + pieces = asyncio.run( + _run(llm, holder, _settings(agent_max_rounds=2, llm_retry_delay=2.5)) + ) + assert pieces == [ + RetryPiece(2, 4), # default llm_retries=3 → 4 attempts + ToolCallPiece(id="call_1", name="list_documents", arguments={}), + StreamPiece("content", "Done!"), + ] + assert holder.tool_calls == 1 + assert holder.read_docs == [] + # The restart is byte-identical: same messages, same tools offered. + assert len(llm.requests) == 3 + assert llm.requests[0] == llm.requests[1] + assert llm.requests[0][1] == AGENT_TOOLS + assert llm.requests[2][1] == AGENT_TOOLS # the answer round still offered + # The flat delay was awaited exactly once, before the retry. + assert sleeps == [2.5] + tool_logs = [r for r in caplog.records if r.getMessage().startswith("agent tool=")] + assert len(tool_logs) == 1 # the retry did not re-run the tool or log + assert tool_logs[0].getMessage() == "agent tool=list_documents args={} round=1/2" + + +def test_round_failure_after_first_piece_is_terminal( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Locked A2: a round that already streamed a piece fails the turn — + the LLMError propagates out of ``run_agent``, no RetryPiece, no + sleep, no second request, and the holder is untouched (the tool + never ran).""" + monkeypatch.setattr(agent, "list_catalog", lambda db: []) + holder = AgentHolder() + llm = FailingLLM( + [([StreamPiece("content", "partial ")], LLMError("mid-stream drop"))] + ) + sleeps = _record_sleeps(monkeypatch) + + async def drain() -> list[StreamPiece | ToolCallPiece | RetryPiece]: + out: list[StreamPiece | ToolCallPiece | RetryPiece] = [] + with pytest.raises(LLMError, match="mid-stream drop"): + async for piece in run_agent( + cast("LLMClient", llm), + cast("Session", None), + system_prompt="SYSTEM_PROMPT", + user_message="QUESTION", + seed_docs=[], + settings=_settings(), + holder=holder, + ): + out.append(piece) + return out + + out = asyncio.run(drain()) + assert out == [StreamPiece("content", "partial ")] # no RetryPiece + assert len(llm.requests) == 1 # no retry + assert sleeps == [] + assert holder.read_docs == [] and holder.tool_calls == 0 + + +def test_forced_final_no_tools_call_is_retried( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The forced final request (round cap reached) goes through the same + retry rule: a failure before its first piece yields a RetryPiece and + restarts with ``tools=None``; the answer from the retry streams.""" + monkeypatch.setattr(agent, "list_catalog", lambda db: [("S", "a.md", "A")]) + holder = AgentHolder() + llm = FailingLLM( + [ + ([ToolCallPiece(id="call_1", name="list_documents", arguments={})], None), + ([ToolCallPiece(id="call_2", name="list_documents", arguments={})], None), + ([], LLMError("down at the cap")), + ([StreamPiece("content", "forced answer")], None), + ] + ) + pieces = asyncio.run(_run(llm, holder, _settings(agent_max_rounds=2))) + assert [type(p) for p in pieces] == [ + ToolCallPiece, + ToolCallPiece, + RetryPiece, + StreamPiece, + ] + assert pieces[2] == RetryPiece(2, 4) + assert pieces[3] == StreamPiece("content", "forced answer") + assert len(llm.requests) == 4 # 2 tool rounds + the final + its retry + # The forced final (and its retry) carry no tools, whatever is left. + assert llm.requests[2][1] is None + assert llm.requests[3][1] is None + # …and the restart is byte-identical. + assert llm.requests[2][0] == llm.requests[3][0] + assert holder.tool_calls == 2 + + +def test_zero_retries_is_one_plain_attempt( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The kill-switch path (``llm_retries=0``): a dead round raises + immediately — one request, no RetryPiece, no sleep (pre-phase-67 + behavior).""" + monkeypatch.setattr(agent, "list_catalog", lambda db: []) + holder = AgentHolder() + llm = FailingLLM([([], LLMError("connection refused"))]) + sleeps = _record_sleeps(monkeypatch) + + async def drain() -> list[StreamPiece | ToolCallPiece | RetryPiece]: + out: list[StreamPiece | ToolCallPiece | RetryPiece] = [] + with pytest.raises(LLMError, match="connection refused"): + async for piece in run_agent( + cast("LLMClient", llm), + cast("Session", None), + system_prompt="SYSTEM_PROMPT", + user_message="QUESTION", + seed_docs=[], + settings=_settings(llm_retries=0), + holder=holder, + ): + out.append(piece) + return out + + out = asyncio.run(drain()) + assert out == [] # nothing streamed, no RetryPiece + assert len(llm.requests) == 1 + assert sleeps == [] + assert holder.read_docs == [] and holder.tool_calls == 0 + + +def test_abandon_mid_retry_sleep_leaks_nothing( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Consumer abandon while a retried round is parked in the pre-retry + sleep (client disconnect): the driving task is cancelled cleanly, the + production teardown ``aclose()`` on ``run_agent`` does not raise, the + inner attempt's stream was torn down, and the retry never starts.""" + entered = asyncio.Event() + + async def parking_sleep(seconds: float) -> None: + entered.set() + await asyncio.Event().wait() # park until the abandon arrives + + monkeypatch.setattr(asyncio, "sleep", parking_sleep) + monkeypatch.setattr(agent, "list_catalog", lambda db: []) + holder = AgentHolder() + llm = FailingLLM( + [([], LLMError("endpoint down")), ([StreamPiece("content", "never")], None)] + ) + + async def run() -> None: + gen = run_agent( + cast("LLMClient", llm), + cast("Session", None), + system_prompt="SYSTEM_PROMPT", + user_message="QUESTION", + seed_docs=[], + settings=_settings(), + holder=holder, + ) + + async def consumer() -> list[StreamPiece | ToolCallPiece | RetryPiece]: + return [p async for p in gen] + + task = asyncio.ensure_future(consumer()) + await entered.wait() # the round's retry is parked in the sleep + assert not task.done() + task.cancel() # client disconnect: the driving task is cancelled + with pytest.raises(asyncio.CancelledError): + await task + # Production teardown (phase 48 pattern): must not raise. ``run_agent`` + # is an async generator despite its AsyncIterator annotation. + await cast( + "AsyncGenerator[StreamPiece | ToolCallPiece | RetryPiece, None]", gen + ).aclose() + + asyncio.run(run()) + assert len(llm.requests) == 1 # the retry never started + assert llm.closed == [0] # attempt 1's inner stream was torn down + assert holder.read_docs == [] and holder.tool_calls == 0 + + +def test_retries_are_invisible_to_the_round_cap( + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """A failing-then-succeeding round consumes ONE round: with a cap of + 2, the retried first round and the second tool round fill the cap — + the forced final follows the SECOND call, and the log lines read + round=1/2 and round=2/2.""" + monkeypatch.setattr(agent, "list_catalog", lambda db: [("S", "a.md", "A")]) + holder = AgentHolder() + llm = FailingLLM( + [ + ([], LLMError("down")), + ([ToolCallPiece(id="call_1", name="list_documents", arguments={})], None), + ([ToolCallPiece(id="call_2", name="list_documents", arguments={})], None), + ([StreamPiece("content", "forced answer")], None), + ] + ) + with caplog.at_level(logging.INFO, logger="app.agent"): + pieces = asyncio.run(_run(llm, holder, _settings(agent_max_rounds=2))) + assert [type(p) for p in pieces] == [ + RetryPiece, + ToolCallPiece, + ToolCallPiece, + StreamPiece, + ] + assert len(llm.requests) == 4 # 2 (round 1 + its retry) + 1 + the forced final + assert llm.requests[3][1] is None # the forced final, after round 2 + assert holder.tool_calls == 2 + msgs = [r.getMessage() for r in caplog.records] + assert "agent tool=list_documents args={} round=1/2" in msgs + assert "agent tool=list_documents args={} round=2/2" in msgs + assert any("round cap reached (rounds=2)" in m for m in msgs) + + # ---------- prompts: section (HIGH only) ---------- diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 3e9bca1..bfb39fc 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -112,6 +112,41 @@ def test_max_output_tokens_env_override(monkeypatch) -> None: assert s.max_output_tokens == 1234 +def test_llm_retry_defaults(monkeypatch: pytest.MonkeyPatch) -> None: + """Phase 67: a failed LLM request is retried by default — 3 retries + with a flat 5 s delay (the TODO-locked values, no backoff).""" + monkeypatch.delenv("BOR_LLM_RETRIES", raising=False) + monkeypatch.delenv("BOR_LLM_RETRY_DELAY", raising=False) + s = _settings() + assert s.llm_retries == 3 + assert s.llm_retry_delay == 5.0 + + +def test_llm_retry_env_overrides(monkeypatch: pytest.MonkeyPatch) -> None: + """``BOR_LLM_RETRIES`` / ``BOR_LLM_RETRY_DELAY`` override the defaults; + ``0`` retries is the no-retry kill switch (pre-phase-67 behavior).""" + monkeypatch.setenv("BOR_LLM_RETRIES", "0") + monkeypatch.setenv("BOR_LLM_RETRY_DELAY", "1.5") + s = _settings() + assert s.llm_retries == 0 + assert s.llm_retry_delay == 1.5 + + +def test_llm_retries_rejects_negative(monkeypatch: pytest.MonkeyPatch) -> None: + """``0`` is the kill switch — a negative value is a typo, so the + validator fails loudly at startup (the ``agent_max_rounds`` pattern).""" + monkeypatch.setenv("BOR_LLM_RETRIES", "-1") + with pytest.raises(ValidationError, match="llm_retries"): + _settings() + + +def test_llm_retry_delay_rejects_negative(monkeypatch: pytest.MonkeyPatch) -> None: + """A negative delay is a typo — fail loudly at startup.""" + monkeypatch.setenv("BOR_LLM_RETRY_DELAY", "-0.5") + with pytest.raises(ValidationError, match="llm_retry_delay"): + _settings() + + def test_agent_max_rounds_default_and_env_override(monkeypatch: pytest.MonkeyPatch) -> None: """Phase 45: the per-tool budgets are gone — ``BOR_AGENT_MAX_ROUNDS`` (default 10) is the single agent-loop knob; ``0`` is the no-tools diff --git a/tests/unit/test_frontend_feedback.py b/tests/unit/test_frontend_feedback.py index 5897e2a..8902f95 100644 --- a/tests/unit/test_frontend_feedback.py +++ b/tests/unit/test_frontend_feedback.py @@ -605,3 +605,56 @@ def test_index_messages_comment_documents_the_meta_actions() -> None: assert "every visitor" in comment.lower() or "everyone" in comment.lower(), ( "Retry is documented as available to all visitors" ) + + +# ---------- llm retry status (phase 67, task 04) ---------- + + +def test_retry_frame_is_a_first_class_branch_between_tool_and_delta() -> None: + """Phase 67 (owner-locked A4, task 02's contract): a `retry` SSE + frame (the server restarted the LLM request before its first piece + — locked A2) is a first-class branch in runTurn's handler, ordered + BETWEEN the `tool` and `delta` branches. It clears the 120s guard + (a frame arrived), resolves n/N from the frame, and writes the + owner-locked copy literal onto the EXISTING channels only — the + #send-status live region + the typing-indicator aria-label. No DOM + of its own: no addMessage, no appendToolLine, no error banner, no + UI-state change. The gate covers BOTH live states: a later agent + round may restart while the UI already streams.""" + js = _js() + tool_idx = js.find('ev.type === "tool"') + retry_idx = js.find('ev.type === "retry"') + delta_idx = js.find('ev.type === "delta"') + assert -1 < tool_idx < retry_idx < delta_idx, ( + "the turn handler must branch on retry frames, between tool and delta" + ) + assert js.count('ev.type === "retry"') == 1, "exactly one retry branch" + branch = js[retry_idx:delta_idx] + assert "clearTurnTimeout()" in branch, "a frame arrived — the 120s guard clears" + assert "Number(ev.attempt)" in branch, "n = the attempt about to be tried" + assert "Number(ev.max_attempts)" in branch, "N = the configured total" + assert ( + "`Communication interrupted — retrying (${attempt} of ${max})…`" in branch + ), "the owner-locked copy literal (A4)" + assert "sendStatus.textContent = retryStatus" in branch, ( + "the existing #send-status live region carries the status" + ) + assert "#typing-indicator .bubble" in branch, ("the typing-indicator is reused") + assert 'setAttribute("aria-label", retryStatus)' in branch + assert "UI_STATE.thinking" in branch and "UI_STATE.streaming" in branch, ( + "the gate covers BOTH live states (a later round may restart mid-stream)" + ) + for forbidden in ("addMessage", "appendToolLine", "showErrorBanner", "setUiState"): + assert forbidden not in branch, f"transient status only — no {forbidden}" + + +def test_header_inventory_documents_the_retry_frame() -> None: + """The app.js file-header doc comment inventories every SSE frame + type (house convention); phase 67 adds the `retry` frame there, with + the owner-locked copy quoted, so the A4 literal has exactly two + homes: the header doc and the handler branch.""" + js = _js() + header = js[: js.find("import {")] + assert "`retry`" in header, "the header must list the retry frame" + assert "Communication interrupted — retrying" in header + assert header.count("Communication interrupted — retrying") == 1 diff --git a/tests/unit/test_llm_client.py b/tests/unit/test_llm_client.py index af914c0..218cd77 100644 --- a/tests/unit/test_llm_client.py +++ b/tests/unit/test_llm_client.py @@ -10,6 +10,7 @@ from __future__ import annotations import asyncio import json +from collections.abc import AsyncGenerator from types import SimpleNamespace from typing import Any, cast @@ -21,8 +22,10 @@ from app.rag.llm import ( EmbeddingError, LLMClient, LLMError, + RetryPiece, StreamPiece, ToolCallPiece, + chat_stream_retried, ) @@ -800,3 +803,274 @@ def test_chat_whitespace_only_content_raises_llm_error() -> None: llm, _ = _make_chat_client(_FakeCompletion(" \n\t ")) with pytest.raises(LLMError, match="empty content"): asyncio.run(llm.chat([{"role": "user", "content": "q"}])) + + +# ---------- chat_stream_retried (phase 67, task 01) ---------- + +_RETRY_MSGS: list[dict[str, str]] = [{"role": "user", "content": "q"}] + + +class _ScriptedClient(LLMClient): + """An LLMClient whose ``chat_stream`` is scripted per attempt — no + endpoint. ``attempts`` scripts the Nth call: ``(pieces, error)`` — the + stream yields *pieces*, then raises *error* if not None (an error after + zero pieces = "the endpoint died before the first token"; after some + pieces = a mid-stream drop). Records every request's messages/tools + and every attempt's stream teardown (the phase-48 close analog).""" + + def __init__( + self, attempts: list[tuple[list[StreamPiece | ToolCallPiece], Exception | None]] + ) -> None: + super().__init__(_settings()) + self.attempts = list(attempts) + self.request_args: list[ + tuple[list[dict[str, str]], list[dict[str, Any]] | None] + ] = [] + #: Indices of attempts whose stream teardown has run. + self.closed: list[int] = [] + + def chat_stream( + self, + messages: list[dict[str, str]], + tools: list[dict[str, Any]] | None = None, + ) -> AsyncGenerator[StreamPiece | ToolCallPiece, None]: + index = len(self.request_args) + pieces, error = ( + self.attempts[index] + if index < len(self.attempts) + else ([], LLMError("script exhausted")) + ) + self.request_args.append( + (list(messages), list(tools) if tools is not None else None) + ) + return self._attempt(index, pieces, error) + + async def _attempt( + self, index: int, pieces: list[StreamPiece | ToolCallPiece], + error: Exception | None, + ) -> AsyncGenerator[StreamPiece | ToolCallPiece, None]: + try: + for piece in pieces: + yield piece + if error is not None: + raise error + finally: + self.closed.append(index) + + +def _record_sleeps(monkeypatch: pytest.MonkeyPatch) -> list[float]: + """Monkeypatch ``asyncio.sleep`` (what ``chat_stream_retried`` awaits + for the flat pre-retry delay) and record every awaited delay.""" + sleeps: list[float] = [] + + async def fake_sleep(seconds: float) -> None: + sleeps.append(seconds) + + monkeypatch.setattr(asyncio, "sleep", fake_sleep) + return sleeps + + +def _collect_retried( + client: _ScriptedClient, + messages: list[dict[str, str]], + *, + tools: list[dict[str, Any]] | None = None, + retries: int, + delay: float, +) -> list[StreamPiece | ToolCallPiece | RetryPiece]: + async def run() -> list[StreamPiece | ToolCallPiece | RetryPiece]: + return [ + p + async for p in chat_stream_retried( + client, messages, tools=tools, retries=retries, delay=delay + ) + ] + + return asyncio.run(run()) + + +def test_retried_retries_a_dead_attempt_before_the_first_piece( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Failure on attempt 1, success on attempt 2 → [RetryPiece(2, N)] + (the attempt about to be tried, 1-based) then the answer pieces; the + request is restarted byte-identical and the flat delay is awaited + exactly once.""" + answer: list[StreamPiece | ToolCallPiece] = [ + StreamPiece("content", "A "), + StreamPiece("content", "B"), + ] + client = _ScriptedClient([([], LLMError("connection refused")), (answer, None)]) + sleeps = _record_sleeps(monkeypatch) + pieces = _collect_retried(client, _RETRY_MSGS, retries=3, delay=2.5) + assert pieces == [RetryPiece(2, 4), *answer] + assert len(client.request_args) == 2 + # The restart is byte-identical: same messages, same (absent) tools. + assert client.request_args[0] == client.request_args[1] + assert client.request_args[0][1] is None + assert sleeps == [2.5] + + +def test_retried_exhaustion_yields_all_retries_then_raises( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """retries=2 → 3 attempts; each pre-first-piece failure yields a + RetryPiece naming the attempt about to be tried (attempts 2 and 3 of + 3), the final failure raises the terminal LLMError, and no sleep + follows the last attempt.""" + client = _ScriptedClient( + [([], LLMError("down 1")), ([], LLMError("down 2")), ([], LLMError("down 3"))] + ) + sleeps = _record_sleeps(monkeypatch) + + async def run() -> list[RetryPiece]: + out: list[RetryPiece] = [] + with pytest.raises(LLMError, match="down 3"): + async for p in chat_stream_retried( + client, _RETRY_MSGS, retries=2, delay=0.5 + ): + assert isinstance(p, RetryPiece) + out.append(p) + return out + + out = asyncio.run(run()) + assert out == [RetryPiece(2, 3), RetryPiece(3, 3)] + assert len(client.request_args) == 3 + assert sleeps == [0.5, 0.5] + + +def test_retried_failure_after_first_piece_is_terminal( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Locked A2: a piece has already flowed → the LLMError is re-raised + immediately — no RetryPiece, no sleep, no second call (a partial + answer is never redone).""" + client = _ScriptedClient( + [([StreamPiece("content", "partial ")], LLMError("mid-stream drop"))], + ) + sleeps = _record_sleeps(monkeypatch) + + async def run() -> list[StreamPiece | ToolCallPiece | RetryPiece]: + out: list[StreamPiece | ToolCallPiece | RetryPiece] = [] + with pytest.raises(LLMError, match="mid-stream drop"): + async for p in chat_stream_retried( + client, _RETRY_MSGS, retries=3, delay=5.0 + ): + out.append(p) + return out + + out = asyncio.run(run()) + assert out == [StreamPiece("content", "partial ")] + assert not any(isinstance(p, RetryPiece) for p in out) + assert len(client.request_args) == 1 + assert sleeps == [] + + +def test_retried_zero_retries_is_one_attempt_no_retry_pieces( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The kill-switch path (retries=0): exactly one attempt, the error + propagates, no RetryPiece, no sleep — the pre-phase-67 behavior.""" + client = _ScriptedClient([([], LLMError("connection refused"))]) + sleeps = _record_sleeps(monkeypatch) + with pytest.raises(LLMError, match="connection refused"): + _collect_retried(client, _RETRY_MSGS, retries=0, delay=5.0) + assert len(client.request_args) == 1 + assert sleeps == [] + + +def test_retried_healthy_stream_is_untouched( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """No failure → exactly one attempt, every piece kind (thinking / tool + call / content) passes through unchanged, no RetryPiece, no sleep — + a healthy turn is byte-identical to the plain chat_stream.""" + answer = [ + StreamPiece("thinking", "hmm"), + ToolCallPiece(id="call_1", name="list_documents", arguments={}), + StreamPiece("content", "Talos."), + ] + client = _ScriptedClient([(answer, None)]) + sleeps = _record_sleeps(monkeypatch) + tools = [ + { + "type": "function", + "function": {"name": "list_documents", "parameters": {}}, + } + ] + pieces = _collect_retried( + client, _RETRY_MSGS, tools=tools, retries=3, delay=5.0 + ) + assert pieces == answer + assert client.request_args == [(_RETRY_MSGS, tools)] + assert sleeps == [] + + +def test_retried_zero_delay_still_notifies(monkeypatch: pytest.MonkeyPatch) -> None: + """The e2e fast path (BOR_LLM_RETRY_DELAY=0): the RetryPiece is still + emitted and the (zero) sleep is still awaited.""" + client = _ScriptedClient( + [([], LLMError("down")), ([StreamPiece("content", "ok")], None)] + ) + sleeps = _record_sleeps(monkeypatch) + pieces = _collect_retried(client, _RETRY_MSGS, retries=1, delay=0) + assert pieces == [RetryPiece(2, 2), StreamPiece("content", "ok")] + assert sleeps == [0] + + +def test_retried_abandon_mid_attempt_closes_the_attempt_stream() -> None: + """Consumer abandon at a mid-attempt piece (the stop-generation path, + phase 48): no exception leaks and the attempt's stream is torn down + through the wrapper's explicit close.""" + client = _ScriptedClient( + [([StreamPiece("content", "A "), StreamPiece("content", "B ")], None)] + ) + + async def run() -> None: + gen = chat_stream_retried(client, _RETRY_MSGS, retries=3, delay=1.0) + first = await gen.__anext__() + assert first == StreamPiece("content", "A ") + await gen.aclose() # the consumer stops after the first piece + + asyncio.run(run()) + assert client.closed == [0] # attempt 1's stream was closed + assert len(client.request_args) == 1 # no second attempt + + +def test_retried_abandon_during_retry_sleep_leaks_nothing( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Consumer abandon while the generator is parked in the pre-retry + sleep (client disconnect): the driving task is cancelled cleanly, + the phase-48 teardown ``aclose()`` on the generator does not raise, + and the retry never starts.""" + entered = asyncio.Event() + + async def parking_sleep(seconds: float) -> None: + entered.set() + await asyncio.Event().wait() # park until the abandon arrives + + monkeypatch.setattr(asyncio, "sleep", parking_sleep) + client = _ScriptedClient( + [([], LLMError("endpoint down")), ([StreamPiece("content", "never")], None)] + ) + + async def run() -> None: + gen = chat_stream_retried(client, _RETRY_MSGS, retries=3, delay=1.5) + + async def consumer() -> list[StreamPiece | ToolCallPiece | RetryPiece]: + return [p async for p in gen] + + task = asyncio.ensure_future(consumer()) + await entered.wait() # the generator is inside the pre-retry sleep + assert not task.done() + task.cancel() # client disconnect: the driving task is cancelled + with pytest.raises(asyncio.CancelledError): + await task + # Production teardown (phase 48 pattern): the endpoint's finally + # closes the stream generator — must not raise. + await gen.aclose() + + asyncio.run(run()) + assert len(client.request_args) == 1 # the retry never started + assert client.closed == [0] # attempt 1's stream was torn down diff --git a/tests/unit/test_sse_events.py b/tests/unit/test_sse_events.py index 1a4aa67..2acdf65 100644 --- a/tests/unit/test_sse_events.py +++ b/tests/unit/test_sse_events.py @@ -4,7 +4,12 @@ from __future__ import annotations import json from app.api.chat import sse_event -from app.schemas import ChatErrorEvent, ChatThinkingEvent, ChatToolEvent +from app.schemas import ( + ChatErrorEvent, + ChatRetryEvent, + ChatThinkingEvent, + ChatToolEvent, +) def _payload(frame: str) -> dict: @@ -100,3 +105,23 @@ def test_tool_event_shape_is_type_name_argument_only() -> None: dumped = ChatToolEvent(name="read_document", argument="S/p.md").model_dump() assert set(dumped.keys()) == {"type", "name", "argument"} assert dumped["type"] == "tool" # default — call sites never spell it out + + +def test_retry_frame_serializes_exactly() -> None: + """Phase 67: the ``retry`` frame is exactly ``{type: "retry", + attempt: int, max_attempts: int}`` — a transient status the client's + readSSE handler branches on ("Communication interrupted — retrying + (n of N)…"), never an error. ``attempt`` is the attempt the server is + about to try next (1-based); the contract the frontend branch and the + E2E suite key off, locked here ahead of the JS implementation.""" + frame = sse_event(ChatRetryEvent(attempt=2, max_attempts=4).model_dump()) + assert frame == 'data: {"type": "retry", "attempt": 2, "max_attempts": 4}\n\n' + assert _payload(frame) == {"type": "retry", "attempt": 2, "max_attempts": 4} + + +def test_retry_event_shape_is_type_attempt_max_attempts_only() -> None: + dumped = ChatRetryEvent(attempt=2, max_attempts=4).model_dump() + assert set(dumped.keys()) == {"type", "attempt", "max_attempts"} + assert dumped["type"] == "retry" # default — call sites never spell it out + assert isinstance(dumped["attempt"], int) + assert isinstance(dumped["max_attempts"], int)