From 3a0fc3db0508c78c77ef4ec8f87cff6231e6b5bb Mon Sep 17 00:00:00 2001 From: ducoterra Date: Thu, 24 Sep 2026 18:50:36 -0400 Subject: [PATCH] phase: 120_failed_turn_retry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All verification complete. Final report: **Phase 120 — Failed-turn retry: verification pass (all 3 tasks were done; final verification + 1 regression fix)** **Verified:** `ChatMessage.failed`/`error` (≤500, `extra="forbid"` intact); `finalizeFailedTurn` funnel on the 3 failure paths (catch-else, stream-drop guard, zero-frame fallback) with `failed: true` + capped detail + `markLastRetryable`; `appendFailedNote` restore branch (Save-as-doc/Tune excluded); `showErrorBanner`/`retryLastTurn` byte-pinned untouched; only the three paths persist `failed: true` (grep + unit pin); no test asserts the old broken behavior. **Defect found & fixed (rule 7):** a real navigate-away mid-turn let the browser's teardown fetch rejection (TypeError, not AbortError) leak into the failed funnel, persisting a phantom failed brain record — `test_sources_midstream_bug.py::test_no_orphan_brain_message_when_navigated_before_first_token` failed (2 `.msg` after reload) and violated the phase-20 navigate-away convention. Fixed: turn-scoped `leftThePage` flag (set unconditionally on `pagehide`, reset in `runTurn`) skips the funnel in the catch-else branch; pinned by new unit test `test_navigate_away_is_not_a_failed_turn`. No phase-overview/PLAN/todo/complete files touched; no commits made. **Gates (exact):** - `uv run pytest` → 2577 passed - `uv run pytest --cov=app --cov-report=term-missing` → TOTAL 4271 stmts, 99% (>90%) - `uv run pytest tests/e2e/test_failed_turn_retry.py -v --no-cov` → 4 passed (isolated) - `uv run ruff check . && uv run pyright` → clean (0 errors) - Regression E2E, isolated: `test_sources_midstream_bug.py` 6/6 (was 5/6); `test_llm_retry`/`test_tool_scaffolding_guardrails`/`test_stop_generation`/`test_navbar_refresh` 17/17 **Completion criteria:** (1) network error → banner + in-bubble Retry, re-ask without re-typing ✅ (E2E A); (2) refresh restores failed bubble + working Retry, no "new chat" ✅ (E2E C); (3) stopped/successful turns byte-identical ✅ (negative E2E, stop suite, byte-identity units); (4) pytest/coverage/lint/types ✅; (5) commit + phase move — left to the harness per pass rules. **Notable:** deviation = the regression fix above (a navigation is not a failed turn; phase-20 partial-persist convention restored). Next pending phase: `121_git_source_tokens`. --- .../120_failed_turn_retry/00_phase.md | 0 .../01_persist_failed_turn.md | 0 .../02_restore_failed_turn.md | 0 .../03_failed_turn_tests.md | 0 .../120_failed_turn_retry__00_phase.a1.err | 0 .../120_failed_turn_retry__00_phase.a1.md | 18 + ...20_failed_turn_retry__00_phase.a1.validate | 104 ++++ ..._turn_retry__01_persist_failed_turn.a1.err | 0 ...d_turn_retry__01_persist_failed_turn.a1.md | 17 + ..._retry__01_persist_failed_turn.a1.validate | 104 ++++ ..._turn_retry__02_restore_failed_turn.a1.err | 0 ...d_turn_retry__02_restore_failed_turn.a1.md | 21 + ..._retry__02_restore_failed_turn.a1.validate | 104 ++++ ...ed_turn_retry__03_failed_turn_tests.a1.err | 0 ...led_turn_retry__03_failed_turn_tests.a1.md | 16 + ...rn_retry__03_failed_turn_tests.a1.validate | 104 ++++ app/schemas.py | 21 +- frontend/assets/app.js | 208 ++++++- frontend/assets/styles.css | 33 + tests/e2e/mock_llm.py | 112 ++++ tests/e2e/test_failed_turn_retry.py | 566 ++++++++++++++++++ tests/e2e/test_llm_retry.py | 37 +- tests/e2e/test_navbar_refresh.py | 2 +- tests/e2e/test_tool_scaffolding_guardrails.py | 45 +- tests/integration/test_chats_api.py | 123 +++- tests/unit/test_chat_message_failed.py | 196 ++++++ tests/unit/test_frontend_failed_turn.py | 468 +++++++++++++++ tests/unit/test_save_as_doc_button.py | 13 +- tests/unit/test_schemas.py | 14 + 29 files changed, 2298 insertions(+), 28 deletions(-) rename .agents/phases/{todo => complete}/120_failed_turn_retry/00_phase.md (100%) rename .agents/phases/{todo => complete}/120_failed_turn_retry/01_persist_failed_turn.md (100%) rename .agents/phases/{todo => complete}/120_failed_turn_retry/02_restore_failed_turn.md (100%) rename .agents/phases/{todo => complete}/120_failed_turn_retry/03_failed_turn_tests.md (100%) create mode 100644 .agents/reports/120_failed_turn_retry/120_failed_turn_retry__00_phase.a1.err create mode 100644 .agents/reports/120_failed_turn_retry/120_failed_turn_retry__00_phase.a1.md create mode 100644 .agents/reports/120_failed_turn_retry/120_failed_turn_retry__00_phase.a1.validate create mode 100644 .agents/reports/120_failed_turn_retry/120_failed_turn_retry__01_persist_failed_turn.a1.err create mode 100644 .agents/reports/120_failed_turn_retry/120_failed_turn_retry__01_persist_failed_turn.a1.md create mode 100644 .agents/reports/120_failed_turn_retry/120_failed_turn_retry__01_persist_failed_turn.a1.validate create mode 100644 .agents/reports/120_failed_turn_retry/120_failed_turn_retry__02_restore_failed_turn.a1.err create mode 100644 .agents/reports/120_failed_turn_retry/120_failed_turn_retry__02_restore_failed_turn.a1.md create mode 100644 .agents/reports/120_failed_turn_retry/120_failed_turn_retry__02_restore_failed_turn.a1.validate create mode 100644 .agents/reports/120_failed_turn_retry/120_failed_turn_retry__03_failed_turn_tests.a1.err create mode 100644 .agents/reports/120_failed_turn_retry/120_failed_turn_retry__03_failed_turn_tests.a1.md create mode 100644 .agents/reports/120_failed_turn_retry/120_failed_turn_retry__03_failed_turn_tests.a1.validate create mode 100644 tests/e2e/test_failed_turn_retry.py create mode 100644 tests/unit/test_chat_message_failed.py create mode 100644 tests/unit/test_frontend_failed_turn.py diff --git a/.agents/phases/todo/120_failed_turn_retry/00_phase.md b/.agents/phases/complete/120_failed_turn_retry/00_phase.md similarity index 100% rename from .agents/phases/todo/120_failed_turn_retry/00_phase.md rename to .agents/phases/complete/120_failed_turn_retry/00_phase.md diff --git a/.agents/phases/todo/120_failed_turn_retry/01_persist_failed_turn.md b/.agents/phases/complete/120_failed_turn_retry/01_persist_failed_turn.md similarity index 100% rename from .agents/phases/todo/120_failed_turn_retry/01_persist_failed_turn.md rename to .agents/phases/complete/120_failed_turn_retry/01_persist_failed_turn.md diff --git a/.agents/phases/todo/120_failed_turn_retry/02_restore_failed_turn.md b/.agents/phases/complete/120_failed_turn_retry/02_restore_failed_turn.md similarity index 100% rename from .agents/phases/todo/120_failed_turn_retry/02_restore_failed_turn.md rename to .agents/phases/complete/120_failed_turn_retry/02_restore_failed_turn.md diff --git a/.agents/phases/todo/120_failed_turn_retry/03_failed_turn_tests.md b/.agents/phases/complete/120_failed_turn_retry/03_failed_turn_tests.md similarity index 100% rename from .agents/phases/todo/120_failed_turn_retry/03_failed_turn_tests.md rename to .agents/phases/complete/120_failed_turn_retry/03_failed_turn_tests.md diff --git a/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__00_phase.a1.err b/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__00_phase.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__00_phase.a1.md b/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__00_phase.a1.md new file mode 100644 index 0000000..3abf188 --- /dev/null +++ b/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__00_phase.a1.md @@ -0,0 +1,18 @@ +All verification complete. Final report: + +**Phase 120 — Failed-turn retry: verification pass (all 3 tasks were done; final verification + 1 regression fix)** + +**Verified:** `ChatMessage.failed`/`error` (≤500, `extra="forbid"` intact); `finalizeFailedTurn` funnel on the 3 failure paths (catch-else, stream-drop guard, zero-frame fallback) with `failed: true` + capped detail + `markLastRetryable`; `appendFailedNote` restore branch (Save-as-doc/Tune excluded); `showErrorBanner`/`retryLastTurn` byte-pinned untouched; only the three paths persist `failed: true` (grep + unit pin); no test asserts the old broken behavior. + +**Defect found & fixed (rule 7):** a real navigate-away mid-turn let the browser's teardown fetch rejection (TypeError, not AbortError) leak into the failed funnel, persisting a phantom failed brain record — `test_sources_midstream_bug.py::test_no_orphan_brain_message_when_navigated_before_first_token` failed (2 `.msg` after reload) and violated the phase-20 navigate-away convention. Fixed: turn-scoped `leftThePage` flag (set unconditionally on `pagehide`, reset in `runTurn`) skips the funnel in the catch-else branch; pinned by new unit test `test_navigate_away_is_not_a_failed_turn`. No phase-overview/PLAN/todo/complete files touched; no commits made. + +**Gates (exact):** +- `uv run pytest` → 2577 passed +- `uv run pytest --cov=app --cov-report=term-missing` → TOTAL 4271 stmts, 99% (>90%) +- `uv run pytest tests/e2e/test_failed_turn_retry.py -v --no-cov` → 4 passed (isolated) +- `uv run ruff check . && uv run pyright` → clean (0 errors) +- Regression E2E, isolated: `test_sources_midstream_bug.py` 6/6 (was 5/6); `test_llm_retry`/`test_tool_scaffolding_guardrails`/`test_stop_generation`/`test_navbar_refresh` 17/17 + +**Completion criteria:** (1) network error → banner + in-bubble Retry, re-ask without re-typing ✅ (E2E A); (2) refresh restores failed bubble + working Retry, no "new chat" ✅ (E2E C); (3) stopped/successful turns byte-identical ✅ (negative E2E, stop suite, byte-identity units); (4) pytest/coverage/lint/types ✅; (5) commit + phase move — left to the harness per pass rules. + +**Notable:** deviation = the regression fix above (a navigation is not a failed turn; phase-20 partial-persist convention restored). Next pending phase: `121_git_source_tokens`. diff --git a/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__00_phase.a1.validate b/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__00_phase.a1.validate new file mode 100644 index 0000000..518eb2b --- /dev/null +++ b/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__00_phase.a1.validate @@ -0,0 +1,104 @@ +........................................................................ [ 2%] +........................................................................ [ 5%] +........................................................................ [ 8%] +........................................................................ [ 11%] +........................................................................ [ 13%] +........................................................................ [ 16%] +........................................................................ [ 19%] +........................................................................ [ 22%] +........................................................................ [ 25%] +........................................................................ [ 27%] +........................................................................ [ 30%] +........................................................................ [ 33%] +........................................................................ [ 36%] +........................................................................ [ 39%] +........................................................................ [ 41%] +........................................................................ [ 44%] +........................................................................ [ 47%] +........................................................................ [ 50%] +........................................................................ [ 53%] +........................................................................ [ 55%] +........................................................................ [ 58%] +........................................................................ [ 61%] +........................................................................ [ 64%] +........................................................................ [ 67%] +........................................................................ [ 69%] +........................................................................ [ 72%] +........................................................................ [ 75%] +........................................................................ [ 78%] +........................................................................ [ 81%] +........................................................................ [ 83%] +........................................................................ [ 86%] +........................................................................ [ 89%] +........................................................................ [ 92%] +........................................................................ [ 94%] +........................................................................ [ 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 52 0 100% +app/api/chat.py 226 1 99% +app/api/chats.py 110 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 99 0 100% +app/api/docs.py 156 1 99% +app/api/git_sources.py 232 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 139 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 231 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 45 0 100% +app/core/caching.py 124 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/errors.py 5 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/security_headers.py 20 0 100% +app/core/theming.py 38 0 100% +app/core/tokens.py 44 0 100% +app/db.py 22 0 100% +app/main.py 66 0 100% +app/models.py 128 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 347 1 99% +app/rag/archive_upload.py 134 0 100% +app/rag/chunker.py 206 4 98% +app/rag/doc_dates.py 18 0 100% +app/rag/folder_summaries.py 123 0 100% +app/rag/git_sources.py 14 0 100% +app/rag/importer.py 219 3 99% +app/rag/llm.py 244 1 99% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 102 0 100% +app/rag/retriever.py 242 3 99% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +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 332 0 100% +-------------------------------------------------- +TOTAL 4271 16 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +WARNING: there is a new pyright version available (v1.1.411 -> v1.1.414). +Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest` + +validation OK diff --git a/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__01_persist_failed_turn.a1.err b/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__01_persist_failed_turn.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__01_persist_failed_turn.a1.md b/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__01_persist_failed_turn.a1.md new file mode 100644 index 0000000..75126c0 --- /dev/null +++ b/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__01_persist_failed_turn.a1.md @@ -0,0 +1,17 @@ +**Task 01 complete — persist the failed turn: schema fields + live error paths** + +Implemented: +- `app/schemas.py`: `ChatMessage.failed: bool|None` + `error: str|None (max_length=500)` beside `stopped`, docstring extended (phase-48 precedent, locked A1); `extra="forbid"` intact. +- `frontend/assets/app.js`: `FAILED_TURN_TEXT` constant; `appendFailedNote(wrap, detail)` (mirror of `appendStoppedNote`, detail via textContent); `finalizeFailedTurn()` funnel (partial → close block/calls + note + persist `failed:true`; zero-frames → FAILED_TURN_TEXT bubble; both set `lastBrainWrap` + `markLastRetryable()`, detail trimmed/capped 500). Wired into: error-catch `else` (before `setUiState`), stream-drop guard, and the zero-frame fallback (now `failed:true` + "The model answered with nothing." note). +- `frontend/assets/styles.css`: `.failed-note` — stopped-note family in `--err-ink` (~9:1 on `--surface`, wraps, non-interactive). +- `showErrorBanner`/`retryLastTurn`/`markLastRetryable`/stop/done paths byte-untouched (verified via diff); `failed: true` persisted at exactly the three failed paths (grep). +- Ripple fixes for the new response keys (exact-shape pins): `tests/unit/test_schemas.py` fixtures, `tests/integration/test_chats_api.py` (`_expect` + `FULL_BRAIN`), `tests/e2e/test_navbar_refresh.py` allowed key-set. + +Results: +- `uv run pytest` → 2547 passed; `uv run pytest --cov=app --cov-report=term-missing` → TOTAL **99%** (>90%) +- `uv run ruff check . && uv run pyright` → clean (0 errors) +- Adjacent E2E sanity (isolated): `test_retry_answer.py` 4/4, `test_stop_generation.py` 3/3 — stopped/success turns unchanged. + +Decisions: no test files authored (task 03 owns them, per the task file); schema verified via ad-hoc checks. No commit made (harness owns the atomic commit). + +Next pending task: `.agents/phases/todo/120_failed_turn_retry/02_restore_failed_turn.md`. diff --git a/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__01_persist_failed_turn.a1.validate b/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__01_persist_failed_turn.a1.validate new file mode 100644 index 0000000..e40ed56 --- /dev/null +++ b/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__01_persist_failed_turn.a1.validate @@ -0,0 +1,104 @@ +........................................................................ [ 2%] +........................................................................ [ 5%] +........................................................................ [ 8%] +........................................................................ [ 11%] +........................................................................ [ 14%] +........................................................................ [ 16%] +........................................................................ [ 19%] +........................................................................ [ 22%] +........................................................................ [ 25%] +........................................................................ [ 28%] +........................................................................ [ 31%] +........................................................................ [ 33%] +........................................................................ [ 36%] +........................................................................ [ 39%] +........................................................................ [ 42%] +........................................................................ [ 45%] +........................................................................ [ 48%] +........................................................................ [ 50%] +........................................................................ [ 53%] +........................................................................ [ 56%] +........................................................................ [ 59%] +........................................................................ [ 62%] +........................................................................ [ 65%] +........................................................................ [ 67%] +........................................................................ [ 70%] +........................................................................ [ 73%] +........................................................................ [ 76%] +........................................................................ [ 79%] +........................................................................ [ 81%] +........................................................................ [ 84%] +........................................................................ [ 87%] +........................................................................ [ 90%] +........................................................................ [ 93%] +........................................................................ [ 96%] +........................................................................ [ 98%] +........................... [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 52 0 100% +app/api/chat.py 226 1 99% +app/api/chats.py 110 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 99 0 100% +app/api/docs.py 156 1 99% +app/api/git_sources.py 232 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 139 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 231 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 45 0 100% +app/core/caching.py 124 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/errors.py 5 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/security_headers.py 20 0 100% +app/core/theming.py 38 0 100% +app/core/tokens.py 44 0 100% +app/db.py 22 0 100% +app/main.py 66 0 100% +app/models.py 128 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 347 1 99% +app/rag/archive_upload.py 134 0 100% +app/rag/chunker.py 206 4 98% +app/rag/doc_dates.py 18 0 100% +app/rag/folder_summaries.py 123 0 100% +app/rag/git_sources.py 14 0 100% +app/rag/importer.py 219 3 99% +app/rag/llm.py 244 1 99% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 102 0 100% +app/rag/retriever.py 242 3 99% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +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 332 0 100% +-------------------------------------------------- +TOTAL 4271 16 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +WARNING: there is a new pyright version available (v1.1.411 -> v1.1.414). +Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest` + +validation OK diff --git a/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__02_restore_failed_turn.a1.err b/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__02_restore_failed_turn.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__02_restore_failed_turn.a1.md b/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__02_restore_failed_turn.a1.md new file mode 100644 index 0000000..2ecc8b6 --- /dev/null +++ b/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__02_restore_failed_turn.a1.md @@ -0,0 +1,21 @@ +All green. My task-02 hunks are cleanly scoped (lines 76, 85–109 of the diff); the rest are task 01's prior work already in the tree. + +**Task 02 — Restore the failed turn: COMPLETE** + +**Implemented** (`frontend/assets/app.js` → `renderStoredMessage`): +- Failed-branch restore: `m.failed` record renders its `text`, then `appendFailedNote(wrap, m.error)` — note omitted entirely when `m.error` is null (record's `text` already carries the detail) +- Save-as-doc gate extended: `if (!m.stopped && !m.failed) appendSaveAsDocButton(wrap, m.text);` +- Tune gate added: `if (!m.failed) appendTuneButton(wrap);` (a note, not an answer; stopped/successful records byte-identical — they keep Tune) +- No other restore change: existing `lastBrainWrap = wrap` + `markLastRetryable()` already land the Retry on the last restored brain bubble (the failed one when it ends the chat); `retryLastTurn` unchanged; mid-conversation failed bubble gets no Retry (last-bubble-only rule intact) +- Shared page: confirmed `shared.js` `renderSharedMessage` never reads `failed`/`error` → failed records render text-only, no note/Retry, no code change (per task) +- Updated `bor.chat.v1` record-shape comment (+ `failed?, error?`) and the phase-59 line-pin in `tests/unit/test_save_as_doc_button.py` + +**Tests / lint / coverage** +- `uv run pytest` → 2547 passed +- `uv run ruff check . && uv run pyright` → clean (0 errors, 0 warnings) +- `uv run pytest --cov=app` → TOTAL 99% (>90% gate; `schemas.py` 100%) +- `bash .agents/validate.sh` → validation OK + +**Decisions:** the task's "No Tune button for failed records either" implemented as a `!m.failed`-only gate (the restore call site didn't previously gate Tune on `m.stopped`, so stopped behavior is untouched per the byte-identical criterion). Unit/E2E tests for this task are task 03's per the phase plan. + +**Next pending task:** `.agents/phases/todo/120_failed_turn_retry/03_failed_turn_tests.md` diff --git a/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__02_restore_failed_turn.a1.validate b/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__02_restore_failed_turn.a1.validate new file mode 100644 index 0000000..e40ed56 --- /dev/null +++ b/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__02_restore_failed_turn.a1.validate @@ -0,0 +1,104 @@ +........................................................................ [ 2%] +........................................................................ [ 5%] +........................................................................ [ 8%] +........................................................................ [ 11%] +........................................................................ [ 14%] +........................................................................ [ 16%] +........................................................................ [ 19%] +........................................................................ [ 22%] +........................................................................ [ 25%] +........................................................................ [ 28%] +........................................................................ [ 31%] +........................................................................ [ 33%] +........................................................................ [ 36%] +........................................................................ [ 39%] +........................................................................ [ 42%] +........................................................................ [ 45%] +........................................................................ [ 48%] +........................................................................ [ 50%] +........................................................................ [ 53%] +........................................................................ [ 56%] +........................................................................ [ 59%] +........................................................................ [ 62%] +........................................................................ [ 65%] +........................................................................ [ 67%] +........................................................................ [ 70%] +........................................................................ [ 73%] +........................................................................ [ 76%] +........................................................................ [ 79%] +........................................................................ [ 81%] +........................................................................ [ 84%] +........................................................................ [ 87%] +........................................................................ [ 90%] +........................................................................ [ 93%] +........................................................................ [ 96%] +........................................................................ [ 98%] +........................... [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 52 0 100% +app/api/chat.py 226 1 99% +app/api/chats.py 110 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 99 0 100% +app/api/docs.py 156 1 99% +app/api/git_sources.py 232 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 139 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 231 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 45 0 100% +app/core/caching.py 124 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/errors.py 5 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/security_headers.py 20 0 100% +app/core/theming.py 38 0 100% +app/core/tokens.py 44 0 100% +app/db.py 22 0 100% +app/main.py 66 0 100% +app/models.py 128 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 347 1 99% +app/rag/archive_upload.py 134 0 100% +app/rag/chunker.py 206 4 98% +app/rag/doc_dates.py 18 0 100% +app/rag/folder_summaries.py 123 0 100% +app/rag/git_sources.py 14 0 100% +app/rag/importer.py 219 3 99% +app/rag/llm.py 244 1 99% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 102 0 100% +app/rag/retriever.py 242 3 99% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +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 332 0 100% +-------------------------------------------------- +TOTAL 4271 16 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +WARNING: there is a new pyright version available (v1.1.411 -> v1.1.414). +Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest` + +validation OK diff --git a/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__03_failed_turn_tests.a1.err b/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__03_failed_turn_tests.a1.err new file mode 100644 index 0000000..e69de29 diff --git a/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__03_failed_turn_tests.a1.md b/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__03_failed_turn_tests.a1.md new file mode 100644 index 0000000..7f78474 --- /dev/null +++ b/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__03_failed_turn_tests.a1.md @@ -0,0 +1,16 @@ +All gates green. Final report: + +**Phase 120 task 03 — Failed-turn tests: COMPLETE** + +**Implemented** +- `tests/unit/test_chat_message_failed.py` (new, 10 tests): `failed`/`error` acceptance, 501-char `error` 422 (500 passes), unknown-key 422 (`extra="forbid"` intact), pre-phase records byte-identical on existing keys +- `tests/unit/test_frontend_failed_turn.py` (new, 14 tests): all three live failure paths route through `finalizeFailedTurn` (persist `failed: true`, end `markLastRetryable`), `appendFailedNote` dedup + textContent-only, `FAILED_TURN_TEXT` distinct from `EMPTY_ANSWER_FALLBACK`, `showErrorBanner` + `retryLastTurn` **byte-pinned**, restore branch (note + Save-as-doc/Tune exclusion), `.failed-note` CSS +- `tests/integration/test_chats_api.py` (extended, +4): failed record POST/GET/PUT round-trips byte-identical; `error`>500 → 422 (POST + PUT, nothing stored); shared chat with failed record serves unchanged public shape +- `tests/e2e/test_failed_turn_retry.py` (new, 4 tests) + two new mock injections (`fail first turn`, `partial then fail`): A zero-frame error → banner Retry + failed bubble → re-ask → grounded answer, bubble gone; B partial+error → text kept + note + in-bubble Retry → redo-in-place; C refresh restores failed bubble with working Retry; negative: stopped turn restores "Stopped" note, never a failed note +- Defect fixes: `test_llm_retry.py` (exhaustion) + `test_tool_scaffolding_guardrails.py` (terminal) asserted the old broken behavior (no bubble on error) — updated to the phase-120 contract + +**Results**: `uv run pytest` 2576 passed · `uv run pytest --cov=app --cov-report=term-missing` TOTAL 99% (>90%) · `uv run ruff check . && uv run pyright` clean · `tests/e2e/test_failed_turn_retry.py -v --no-cov` 4/4 in isolation (×2) · validate.sh OK + +**Decisions**: mock fails exactly one turn (4 attempts = forced default budget) then answers, so Retry succeeds; B/C use grounded questions so re-asks assert `MOCK_ANSWER_MARKER`; E2E waits for the idle "Send" label before reading localStorage (done-frame settle — the marker is visible a few frames before `done`). + +**Next pending**: none in phase 120 (all 3 tasks done); next phase dir: `todo/121_git_source_tokens`. diff --git a/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__03_failed_turn_tests.a1.validate b/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__03_failed_turn_tests.a1.validate new file mode 100644 index 0000000..9bf2d8b --- /dev/null +++ b/.agents/reports/120_failed_turn_retry/120_failed_turn_retry__03_failed_turn_tests.a1.validate @@ -0,0 +1,104 @@ +........................................................................ [ 2%] +........................................................................ [ 5%] +........................................................................ [ 8%] +........................................................................ [ 11%] +........................................................................ [ 13%] +........................................................................ [ 16%] +........................................................................ [ 19%] +........................................................................ [ 22%] +........................................................................ [ 25%] +........................................................................ [ 27%] +........................................................................ [ 30%] +........................................................................ [ 33%] +........................................................................ [ 36%] +........................................................................ [ 39%] +........................................................................ [ 41%] +........................................................................ [ 44%] +........................................................................ [ 47%] +........................................................................ [ 50%] +........................................................................ [ 53%] +........................................................................ [ 55%] +........................................................................ [ 58%] +........................................................................ [ 61%] +........................................................................ [ 64%] +........................................................................ [ 67%] +........................................................................ [ 69%] +........................................................................ [ 72%] +........................................................................ [ 75%] +........................................................................ [ 78%] +........................................................................ [ 81%] +........................................................................ [ 83%] +........................................................................ [ 86%] +........................................................................ [ 89%] +........................................................................ [ 92%] +........................................................................ [ 95%] +........................................................................ [ 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 52 0 100% +app/api/chat.py 226 1 99% +app/api/chats.py 110 0 100% +app/api/config.py 13 0 100% +app/api/doc_drafts.py 99 0 100% +app/api/docs.py 156 1 99% +app/api/git_sources.py 232 0 100% +app/api/health.py 10 0 100% +app/api/steering.py 42 0 100% +app/api/suggestions.py 33 0 100% +app/api/sync.py 139 0 100% +app/api/tokens.py 40 0 100% +app/api/ui_settings.py 55 0 100% +app/config.py 231 0 100% +app/core/__init__.py 0 0 100% +app/core/auth.py 45 0 100% +app/core/caching.py 124 0 100% +app/core/debugging.py 29 2 93% +app/core/docs_push.py 39 0 100% +app/core/errors.py 5 0 100% +app/core/logging.py 13 0 100% +app/core/rate_limit.py 44 0 100% +app/core/security_headers.py 20 0 100% +app/core/theming.py 38 0 100% +app/core/tokens.py 44 0 100% +app/db.py 22 0 100% +app/main.py 66 0 100% +app/models.py 128 0 100% +app/rag/__init__.py 0 0 100% +app/rag/agent.py 347 1 99% +app/rag/archive_upload.py 134 0 100% +app/rag/chunker.py 206 4 98% +app/rag/doc_dates.py 18 0 100% +app/rag/folder_summaries.py 123 0 100% +app/rag/git_sources.py 14 0 100% +app/rag/importer.py 219 3 99% +app/rag/llm.py 244 1 99% +app/rag/overview.py 71 0 100% +app/rag/prompts.py 102 0 100% +app/rag/retriever.py 242 3 99% +app/rag/scaffolding.py 55 0 100% +app/rag/source_removal.py 41 0 100% +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 332 0 100% +-------------------------------------------------- +TOTAL 4271 16 99% +coverage gate: app/ 99% (>90%) OK +All checks passed! +0 errors, 0 warnings, 0 informations +WARNING: there is a new pyright version available (v1.1.411 -> v1.1.414). +Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest` + +validation OK diff --git a/app/schemas.py b/app/schemas.py index 1412f2b..26591e6 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -744,8 +744,8 @@ class ChatMessage(BaseModel): (phase 14) — the stored ``messages`` payload of a saved chat (phase 50). ``{who, text, sources?, related?, deflected?, suggestions?, - thinking?, tools?, stopped?}`` — raw text, never HTML, so a saved - chat restores pixel-identical through the existing + thinking?, tools?, stopped?, failed?, error?}`` — raw text, never + HTML, so a saved chat restores pixel-identical through the existing ``renderStoredMessage`` path. ``extra="forbid"`` rejects unknown keys (a corrupted or HTML-shaped payload, e.g. a stray ````-ish extra key) at the boundary with a @@ -774,6 +774,19 @@ class ChatMessage(BaseModel): shape; ``suggestions`` ≤ 50 chips of ≤ 200 chars; ``tools`` ≤ 50 — one entry per tool call, the round cap is 10). Only value bounds were added — the accepted/rejected KEYS are unchanged. + + Phase 120 (task 01, locked A1): a FAILED turn (network error, SSE + ``error`` frame, stream drop — the client-side failure paths) stores + ``{who: "brain", text: , failed: true, + error: }`` — the phase-48 ``stopped`` precedent: a marker + + the capped detail on the brain record itself, no separate error + table, no new API (``retryLastTurn``'s pop-the-last-brain-record + logic works on a failed record UNCHANGED — the question's user + record immediately precedes it). ``error`` is the persisted banner + detail, capped at 500 (the phase-83 value-bounds style). No + serializer change: ``None`` values flow as absent/None exactly like + ``stopped`` today — the phase-50 byte-identical round-trip contract + covers the new keys automatically. """ model_config = ConfigDict(extra="forbid") @@ -787,6 +800,10 @@ class ChatMessage(BaseModel): thinking: str | None = Field(default=None, max_length=32_000) tools: list[ToolCall] | None = Field(default=None, max_length=50) stopped: bool | None = None + # Phase 120 (task 01, locked A1): the failed-turn marker + the + # persisted error detail (the phase-48 ``stopped`` precedent). + failed: bool | None = None + error: str | None = Field(default=None, max_length=500) class SavedChatCreate(BaseModel): diff --git a/frontend/assets/app.js b/frontend/assets/app.js index bc97db2..bafdca2 100644 --- a/frontend/assets/app.js +++ b/frontend/assets/app.js @@ -364,6 +364,15 @@ const ERROR_HINT = "If this persists, check the LLM is reachable."; exhausted max_tokens — phase 17) still renders a bubble, and this exact text is what gets persisted: what the user saw is what is stored. */ const EMPTY_ANSWER_FALLBACK = "Hmm — that came back empty. Ask me again?"; +/* Phase 120 (TODO.md L3–4, locked A1): the fixed bubble text of a FAILED + turn that streamed ZERO frames (network error, pre-stream HTTP error) — + a short honest "my answer didn't make it" line. NOT the + EMPTY_ANSWER_FALLBACK: that constant is the zero-frame-but-COMPLETED + case's answer text (the stream settled; a failed turn didn't). The real + detail rides the in-bubble .failed-note (appendFailedNote) and the + persisted `error` key (capped at 500 — the ChatMessage bound). */ +const FAILED_TURN_TEXT = + "My answer didn't make it — the connection dropped. Use Retry to ask again."; /* Calm, don't remove: smooth scrolling is the one motion JS controls. */ const reducedMotion = @@ -504,6 +513,48 @@ function appendStoppedNote(wrap) { meta.appendChild(note); } +/* Phase 120 (TODO.md L3–4): the "Failed" note in the meta row of a failed + * brain bubble — the mirror of appendStoppedNote for the phase-120 failed + * turn (network error, SSE error frame, stream drop). The live failure + * paths (finalizeFailedTurn / the zero-frame fallback) and the phase-14 + * restore path (a record with `m.failed`) share this helper, so a restored + * bubble reads exactly like the failed one. The banner keeps its + * role="alert" summary; this in-bubble note is the refresh-surviving copy + * (on restore it renders from the persisted `error` detail). Reuses the + * .msg-meta row the way appendStoppedNote does (role=list → the span joins + * as a listitem so ARIA stays valid); the triangle glyph is aria-hidden + * decoration — the "Failed" label + the detail text carry the accessible + * meaning (text + color, never color alone — B5). `detail` goes through + * textContent (no HTML from the error string, ever). */ +const FAILED_ICON = + ''; + +function appendFailedNote(wrap, detail) { + const body = wrap?.querySelector?.(".msg-body"); + if (!body) return; + let meta = body.querySelector(".msg-meta"); + if (!meta) { + meta = document.createElement("div"); + meta.className = "msg-meta"; + body.appendChild(meta); + } + if (meta.querySelector(".failed-note")) return; // one per bubble + const note = document.createElement("span"); + note.className = "failed-note"; + if (meta.getAttribute("role") === "list") note.setAttribute("role", "listitem"); + note.innerHTML = FAILED_ICON; + const label = document.createElement("span"); + label.textContent = "Failed"; + note.appendChild(label); + if (detail) { + const d = document.createElement("span"); + d.className = "failed-detail"; + d.textContent = detail; // the error string is NEVER innerHTML + note.appendChild(d); + } + meta.appendChild(note); +} + /* Phase 49 (owner-locked 2026-08-29, TODO.md L4): the rendered wrap of * the CURRENT last brain record. Set wherever a brain bubble becomes the * latest persisted answer (the `done` branch, the empty-answer fallback, @@ -1182,6 +1233,19 @@ let toolLineWrap = null; // the live wrap the clock suffixes (null when stopped) let acc = ""; // accumulated answer text this turn let thinkingAcc = ""; // accumulated thinking text (persisted with the turn) let persistedOnLeave = false; // pagehide partial-persist at most once +/* Phase 120 verification (navigate-away regression): a pagehide has + * fired. A REAL departure tears the document down and the browser + * cancels the in-flight fetch — that teardown rejection (a TypeError, + * NOT an AbortError) must not be read as a FAILED turn by the error + * catch's else branch: the phase-20 convention is that a thinking-only + * navigate-away persists NOTHING brain-side (the question is already + * saved on send — the user can re-ask) and a partial navigate-away + * persists the pagehide partial, not a failed record. Set + * unconditionally on pagehide — a merely-hidden tab in some browsers + * also fires it, but there the stream keeps arriving (no rejection + * follows), so the flag stays inert — and reset per turn like the + * other turn locals. */ +let leftThePage = false; /* Phase 73 (task 02, TODO.md L3): the pagehide partial is CORRELATED with * the turn's settle. `leavePartialIndex` is the index in `conversation` * of the brain record the `pagehide` handler pushed for THIS turn (else @@ -1546,7 +1610,8 @@ function appendMaybeTry(wrap, suggestions) { * bor.chat.v1 → { v: 1, chatId: string | null, * messages: [{ who: "user"|"brain", text, * sources?, deflected?, suggestions?, - * thinking?, tools?, stopped? }] } + * thinking?, tools?, stopped?, + * failed?, error? }] } * * Only RAW TEXT is stored — restore re-renders it through the escape-first * markdown renderer, so no HTML is ever persisted. Save points: the user @@ -1680,12 +1745,28 @@ function renderStoredMessage(m) { appendMaybeTry(wrap, m.suggestions); } appendSources(wrap, m.sources); - appendTuneButton(wrap); // restored brain answers are tunable too + // Phase 120: a failed turn (m.failed) is a note, not an answer — no + // Tune (the "note, not an answer" scope the m.stopped exclusion + // below shares; the live failure paths add no Tune either). Stopped + // partials keep their button — byte-identical to pre-phase-120. + if (!m.failed) appendTuneButton(wrap); // restored brain answers are tunable too // Phase 59: the RAW persisted markdown (m.text — HTML is never - // persisted). A stopped partial (m.stopped) is a note, not an answer - // — no button (the live stop path adds none either). - if (!m.stopped) appendSaveAsDocButton(wrap, m.text); + // persisted). A stopped partial (m.stopped) or a failed turn + // (m.failed) is a note, not an answer — no button (the live + // stop/failure paths add none either). + if (!m.stopped && !m.failed) appendSaveAsDocButton(wrap, m.text); if (m.stopped) appendStoppedNote(wrap); // phase 48: the stop marker restores + // Phase 120: the failed marker restores — the in-bubble error note + // re-renders from the persisted `error` detail (the note is absent + // when it is null: the record's `text` already carries the detail). + // No other restore change: the lastBrainWrap assignment below + the + // restore loop's markLastRetryable() already land the Retry button + // on the LAST restored brain bubble — the failed one when it ends + // the conversation — so the refreshed page is the same retryable + // state the live error was (retryLastTurn re-asks the question that + // precedes the failed record, unchanged; a mid-conversation failed + // bubble gets no button — the phase-49 last-bubble-only rule). + if (m.failed && m.error) appendFailedNote(wrap, m.error); // Phase 113 (task 02): the related tier restores with the bubble // (LAST — after the meta-row claimers, exactly like the live done // path). Pre-phase records carry no `related` → appendRelated @@ -2333,6 +2414,67 @@ async function handleSend(e) { await runTurn(text, { reask: false }); } +/* Phase 120 (TODO.md L3–4, locked A1): the single funnel for every + * NON-STOP, NON-ABORT turn failure — the SSE error-frame catch's else + * branch (network error, pre-stream HTTP error, the `error` frame's + * throw) and the stream-drop guard (frames arrived, no `done`). A failed + * turn persists as a BRAIN record marked `failed: true` + the capped + * error detail (the phase-48 `stopped` precedent — no separate error + * table, no new API): the question's user record immediately precedes it, + * so retryLastTurn's pop-the-last-brain-record-then-re-ask logic works + * UNCHANGED, and lastBrainWrap is set BEFORE the caller's + * setUiState(UI_STATE.error, …) — so showErrorBanner's EXISTING + * `opts.retryable && lastBrainWrap` condition finally reveals the + * phase-111 banner Retry on a network error (showErrorBanner itself is + * byte-unchanged). + * + * Two shapes: a partial `wrap` exists (a frame streamed) — close the + * thinking block + tool calls (the stop-finalize pattern), keep the + * streamed answer text, add the in-bubble error note; or zero frames — + * a new FAILED_TURN_TEXT bubble. Either way the record rides + * rememberBrainTurn (localStorage + the phase-55 auto-save) and the + * in-bubble Retry button lands on the failed bubble (markLastRetryable). + * `detail` is trimmed + capped at 500 before persistence (the schema's + * ChatMessage.error bound is the backstop; the banner keeps the original + * untrimmed string). NOT this funnel: the stop path (its own branch), + * the 300s abort (the `aborted` branch), and the zero-frame-but-completed + * case (the stream settled — runTurn's inline fallback branch keeps its + * EMPTY_ANSWER_FALLBACK bubble and only gains the failed marker). */ +function finalizeFailedTurn(detail, { acc, thinking, tools, wrap, leavePartialIndex }) { + const error = (detail || "").trim().slice(0, 500); + if (wrap) { + // Partial streamed: keep the answer text the user saw, settle the + // block + calls closed (the stop-finalize pattern), mark it failed. + closeThinkingBlock(wrap); + closeToolCalls(wrap); + appendFailedNote(wrap, error); + rememberBrainTurn( + acc, + { + thinking: thinking || undefined, + tools: tools.length ? tools : undefined, + failed: true, + error: error || undefined, + }, + leavePartialIndex + ); + lastBrainWrap = wrap; + } else { + // Zero frames (network error, pre-stream HTTP error): a new failed + // bubble with the fixed honest line — the real detail rides the note + // + the persisted `error` key. + const fwrap = addMessage("brain", FAILED_TURN_TEXT); + appendFailedNote(fwrap, error); + rememberBrainTurn( + FAILED_TURN_TEXT, + { failed: true, error: error || undefined }, + leavePartialIndex + ); + lastBrainWrap = fwrap; + } + markLastRetryable(); // the in-bubble Retry lands on the failed bubble +} + /* Phase 49 (owner-locked 2026-08-29, TODO.md L4): the chat turn — * extracted from handleSend so the retry redo can re-run a question * without re-adding it. `reask` skips (a) the user-bubble append and @@ -2367,6 +2509,7 @@ async function runTurn(text, { reask = false } = {}) { acc = ""; thinkingAcc = ""; persistedOnLeave = false; + leftThePage = false; leavePartialIndex = -1; // Phase 48: a fresh abort owner per turn (cleared in the finally); the // stop flag resets with the rest of the turn locals. @@ -2654,20 +2797,40 @@ async function runTurn(text, { reask = false } = {}) { // idle with a half bubble. The zero-frame case falls through to the // existing empty-answer fallback below. if (!sawDone && !aborted && (acc || thinkingAcc)) { - setUiState( - UI_STATE.error, - "The stream ended before my answer finished — try again?" - ); + // Phase 120 (TODO.md L3–4): a half-answer is a FAILED answer — the + // partial persists as failed (its text + the in-bubble error note + + // a working Retry) BEFORE the error state, so Refresh restores what + // the user saw + a Retry (no bare question, no "new chat"). + const detail = "The stream ended before my answer finished — try again?"; + finalizeFailedTurn(detail, { + acc, + thinking: thinkingAcc, + tools: toolAcc, + wrap, + leavePartialIndex, + }); + setUiState(UI_STATE.error, detail); } if (!aborted && !wrap) { const fallback = EMPTY_ANSWER_FALLBACK; + // Phase 120 (TODO.md L3–4, task 01 ASSUMPTION): the stream + // COMPLETED with ZERO frames — it is a failed turn too. The bubble + // text stays EMPTY_ANSWER_FALLBACK (a meaningful record text); the + // failed marker + error note make it persist + restore as a failure + // with a working Retry (the pre-phase record persisted with no + // marker — inconsistent with the refresh case this phase fixes). + const nothing = "The model answered with nothing."; const fwrap = addMessage("brain", fallback); appendTuneButton(fwrap); appendSaveAsDocButton(fwrap, fallback); // phase 59: parity with the done path + appendFailedNote(fwrap, nothing); // Phase 73: correlated like the other settles — unreachable when a // pagehide partial exists (that needs acc, which means a wrap), but // passed so EVERY settle write goes through the same correlation. - rememberBrainTurn(fallback, {}, leavePartialIndex); // persist what the user actually saw + rememberBrainTurn(fallback, { + failed: true, + error: nothing, + }, leavePartialIndex); // persist what the user actually saw lastBrainWrap = fwrap; markLastRetryable(); // phase 49: the fallback bubble is retryable too } @@ -2708,6 +2871,30 @@ async function runTurn(text, { reask = false } = {}) { err instanceof Error && err.message ? err.message : "Something went wrong on my side."; + // Phase 120 (TODO.md L3–4, locked A1): persist the failed turn + // BEFORE the error state — the funnel sets lastBrainWrap, so the + // existing `opts.retryable && lastBrainWrap` condition in + // setUiState → showErrorBanner now reveals the phase-111 banner + // Retry on a network error (the button's precondition finally + // holds; showErrorBanner itself is byte-unchanged). + // Phase 120 verification (navigate-away regression): a REAL + // departure (pagehide fired) tears the document down and the + // browser cancels the in-flight fetch — that teardown rejection + // is NOT a failed turn. The phase-20 convention stands: a + // thinking-only navigate-away persists NOTHING brain-side, and a + // partial navigate-away persists the pagehide partial (above) as + // a plain record — never a failed one. A real network error + // while the page is still alive (no pagehide) takes the funnel + // as before. + if (!leftThePage) { + finalizeFailedTurn(detail, { + acc, + thinking: thinkingAcc, + tools: toolAcc, + wrap, + leavePartialIndex, + }); + } // Phase 114 (TODO L6): the SSE error frame's optional hint flows to // the banner (setUiState → showErrorBanner's opts.hint); the // phase-111 Retry button rides along on the same turn-error path. @@ -2776,6 +2963,7 @@ staleRegenBtn?.addEventListener("click", regenerateStaleChat); * C1). A REAL navigation (the page actually unloads) never runs a settle, * so the partial stays persisted exactly as before. */ window.addEventListener("pagehide", () => { + leftThePage = true; // the dying fetch's teardown rejection is not a failed turn if (persistedOnLeave) return; if (uiState !== UI_STATE.thinking && uiState !== UI_STATE.streaming) return; diff --git a/frontend/assets/styles.css b/frontend/assets/styles.css index 8916007..46e2e42 100644 --- a/frontend/assets/styles.css +++ b/frontend/assets/styles.css @@ -898,6 +898,39 @@ details.thinking .thinking-text ul { margin: 0 0 0.5rem; } } .stopped-note svg { width: 10px; height: 10px; display: block; fill: currentColor; } +/* Phase 120 (TODO.md L3–4): the "Failed" note in a failed brain + bubble's meta row — the .stopped-note family in the theme's error + color (text + color, never color alone — B5: the "Failed" label + carries the meaning, the triangle glyph is aria-hidden decoration in + the JS). --err-ink on the --surface bubble ≈9.0:1 (the same value as + --brand-ink, computed 9.0:1 on --surface above) — well past 4.5:1 + (WCAG 2.1 AA); the monochrome theme grays it automatically (token, + not literal — phase-92 zero-literal). Non-interactive — no hover, no + focus (pointer-events: none), the stopped note's way. The detail + span WRAPS (the stopped note's nowrap fits a one-word label; a 500- + char error detail must not blow out the 46rem chat column) — + overflow-wrap: anywhere keeps long unbroken details inside the row. */ +.failed-note { + display: inline-flex; + align-items: flex-start; + gap: 0.3rem; + max-width: 100%; + color: var(--err-ink); + font-size: 0.75rem; + font-weight: 600; + pointer-events: none; +} +.failed-note svg { + width: 10px; + height: 10px; + margin-top: 0.15rem; + display: block; + flex: none; + fill: none; + stroke: currentColor; +} +.failed-detail { font-weight: 400; overflow-wrap: anywhere; } + /* Inline tuning form under the bubble: labeled textarea + Save/Cancel (both ≥44px). Save = brand button (dark ink 5.2:1), Cancel = ghost. */ .tune-form { diff --git a/tests/e2e/mock_llm.py b/tests/e2e/mock_llm.py index 81320b5..82d68f1 100644 --- a/tests/e2e/mock_llm.py +++ b/tests/e2e/mock_llm.py @@ -521,6 +521,36 @@ test_llm_retry.py``). The mock is single-conversation per e2e server, so strings are this suite's own folder names, so no other E2E can hit them (they seed different trees). + Failure injection (phase 120, failed-turn retry, TODO.md L3–4) — + deterministic failures for the failed-turn E2E suite + (``tests/e2e/test_failed_turn_retry.py``), reusing the phase-67 + counter machinery: + - user message containing ``fail first turn`` + (``FAIL_FIRST_TURN_TRIGGER``): EVERY app-level attempt of the FIRST + turn responds 500 — the full forced-default budget (conftest pins + ``BOR_LLM_RETRIES`` to the code default 3 → 4 app-level attempts = + 12 POSTs, ``FAIL_FIRST_TURN_DEAD_ATTEMPTS`` × + ``_HTTPS_PER_DEAD_ATTEMPT``) — the retry-budget exhaustion with + ZERO frames (the terminal ``error`` before any delta) — and the + SECOND turn's first request streams the normal composed answer (the + banner/in-bubble Retry's re-ask). The sequence resets after that + success, so a second question carrying the trigger re-drives the + failure from zero (the phase-67 convention). + - user message containing ``partial then fail`` + (``PARTIAL_FAIL_TRIGGER``): the FIRST matching streaming request + sends ``PARTIAL_FAIL_TEXT`` in normal 12-char ``delta.content`` + chunks and then the generator RAISES — a genuine mid-stream + connection reset (the body ends without ``finish_reason`` / + ``[DONE]``; a mid-stream failure never re-POSTs — the body was + already flowing). The app's ``chat_stream_retried`` re-raises the + wrapped ``LLMError`` (a piece already emitted — the locked + retry-before-first-frame rule) and the chat endpoint settles the + turn with the terminal SSE ``error`` frame AFTER the partial + deltas (the partial-then-error wire: the partial bubble keeps its + text + the error note + the Retry). The SECOND request streams the + normal composed answer (the retry's re-ask) and resets the + sequence. + ``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. @@ -750,6 +780,46 @@ ALWAYS_FAIL_TRIGGER = "always fail" #: bag-of-words vector — the endpoint's pre-stream embedding retry loop. EMBED_FAIL_TRIGGER = "embed fail once" +# --------------------------------------------------------------------------- +# Phase 120 (failed-turn retry, TODO.md L3–4): deterministic failure +# injections for the failed-turn E2E suite (tests/e2e/ +# test_failed_turn_retry.py) — see the module docstring +# --------------------------------------------------------------------------- + +#: A user message containing this substring (case-insensitive) dies for +#: the ENTIRE first turn — every app-level attempt 500s (the +#: retry-budget exhaustion, the zero-frame terminal ``error``) — and +#: streams the normal composed answer from the SECOND turn on (the +#: banner/in-bubble Retry's re-ask, phase 111/120). The dead window is +#: exactly ONE turn: ``FAIL_FIRST_TURN_DEAD_ATTEMPTS`` = the e2e +#: forced-default budget (conftest pins ``BOR_LLM_RETRIES`` to the code +#: default 3 → 4 app-level attempts = 12 POSTs at +#: ``_HTTPS_PER_DEAD_ATTEMPT``). Each sequence resets after the success +#: it guards, so a second question carrying the trigger re-drives the +#: failure from zero (the phase-67 ``_fail_posts`` convention). +FAIL_FIRST_TURN_TRIGGER = "fail first turn" +#: One full turn under the forced-default budget (3 retries → 4 +#: attempts): the whole first turn dies, the second turn's first +#: attempt streams (the Retry's re-ask). +FAIL_FIRST_TURN_DEAD_ATTEMPTS = 4 + +#: A user message containing this substring (case-insensitive) streams +#: ``PARTIAL_FAIL_TEXT`` as ordinary ``delta.content`` chunks on its +#: FIRST matching request and then the mock's generator RAISES (a +#: mid-stream connection reset — the SSE body ends without +#: ``finish_reason``/``[DONE]``; a mid-stream failure never re-POSTs). +#: The app's ``chat_stream_retried`` re-raises the wrapped ``LLMError`` +#: (a piece already emitted — the locked retry-before-first-frame rule) +#: and the chat endpoint settles with the terminal SSE ``error`` frame +#: AFTER the partial deltas. The SECOND request streams the normal +#: composed answer (the retry's re-ask) and resets the sequence. +PARTIAL_FAIL_TRIGGER = "partial then fail" +#: Matching requests that partial-then-die (one POST — see above). +PARTIAL_FAIL_DEAD_REQUESTS = 1 +#: The deterministic partial answer (byte-stable — the E2E asserts the +#: bubble KEEPS exactly this text, with the error note appended). +PARTIAL_FAIL_TEXT = "Here is the start of the answer that never finished." + # --------------------------------------------------------------------------- # Phase 71 (tool-scaffolding guardrails, 2026-09-03 incident): # deterministic raw-markup flows — see the module docstring @@ -987,6 +1057,32 @@ def _chat_dead(key: str, dead_attempts: int) -> bool: return _bump_fail(key) <= dead_attempts * _HTTPS_PER_DEAD_ATTEMPT +def _partial_fail_stream() -> Any: + """The phase-120 partial-then-fail stream (``PARTIAL_FAIL_TRIGGER``): + ``PARTIAL_FAIL_TEXT`` in 12-char ``delta.content`` chunks (the + mock's default cadence — NO ``finish_reason``, NO ``[DONE]``), then + a RAISE that ends the SSE body abruptly (a connection reset + mid-stream — the client SDK sees a truncated body, the app wraps it + in ``LLMError`` after the pieces already emitted, and the chat + endpoint's terminal ``error`` frame lands AFTER the partial deltas + on the client's stream).""" + model = "turbo" + chunk_id = f"chatcmpl-{uuid.uuid4()}" + for piece in re.findall(r".{1,12}", PARTIAL_FAIL_TEXT, re.S): + payload = { + "id": chunk_id, + "object": "chat.completion.chunk", + "created": int(time.time()), + "model": model, + "choices": [ + {"index": 0, "delta": {"content": piece}, "finish_reason": None} + ], + } + yield f"data: {json_dumps(payload)}\n\n" + time.sleep(0.02) + raise RuntimeError("e2e mid-stream failure (phase 120 partial-then-fail)") + + #: The agent's ``read`` tool-result prefix (app.rag.agent #: ``_execute_tool``): ``"Document :\n"``. _READ_RESULT_PREFIX = "Document " @@ -2471,6 +2567,22 @@ def chat_completions(body: dict[str, Any]) -> Any: if _chat_dead(RETRY_TRIGGER, RETRY_DEAD_ATTEMPTS): return _llm_500(RETRY_TRIGGER) _fail_posts[RETRY_TRIGGER] = 0 # the answer streamed — restart + # Phase 120 (failed-turn retry): the failed-turn suite's two + # injections — the whole-first-turn death (the zero-frame + # exhaustion whose Retry's re-ask the mock then answers) and the + # partial-then-die (the partial deltas + terminal error frame). + if FAIL_FIRST_TURN_TRIGGER in user_lower: + if _chat_dead(FAIL_FIRST_TURN_TRIGGER, FAIL_FIRST_TURN_DEAD_ATTEMPTS): + return _llm_500(FAIL_FIRST_TURN_TRIGGER) + _fail_posts[FAIL_FIRST_TURN_TRIGGER] = 0 # the answer streamed — restart + if PARTIAL_FAIL_TRIGGER in user_lower: + if _bump_fail(PARTIAL_FAIL_TRIGGER) <= PARTIAL_FAIL_DEAD_REQUESTS: + return StreamingResponse( + _partial_fail_stream(), + media_type="text/event-stream", + headers={"Cache-Control": "no-cache", "X-Accel-Buffering": "no"}, + ) + _fail_posts[PARTIAL_FAIL_TRIGGER] = 0 # the answer streamed — restart # Phase 71 (tool-scaffolding guardrails): the deterministic raw- # markup flow — checked BEFORE the search/tool marker flows (the # trigger is independent of the ```` marker, so both diff --git a/tests/e2e/test_failed_turn_retry.py b/tests/e2e/test_failed_turn_retry.py new file mode 100644 index 0000000..1f3c25b --- /dev/null +++ b/tests/e2e/test_failed_turn_retry.py @@ -0,0 +1,566 @@ +"""Phase 120 E2E (Playwright): failed-turn retry — network errors and +refresh survive a failed turn. + +Source: ``TODO.md`` L3–4 — "Retry doesn't seem to work on network +error" + "Refreshing the page after an error shows only the chat +message you sent and no options to retry the message …". +Run in isolation (DB must be up: ``podman compose up -d db``): + + uv run pytest tests/e2e/test_failed_turn_retry.py -v --no-cov + +MOCK-ONLY suite: ``E2E_REAL_LLM=1`` is not supported — the failure +shapes are the deterministic injections in ``tests/e2e/mock_llm.py`` +(the phase-67 counter machinery, the phase-120 triggers): + +* ``fail first turn`` (``FAIL_FIRST_TURN_TRIGGER``): the WHOLE first + turn dies — every app-level attempt of the forced-default budget + (conftest pins ``BOR_LLM_RETRIES`` to the code default 3 → 4 + attempts) 500s, ZERO frames — the retry-budget exhaustion + (``tests/e2e/test_llm_retry.py``'s ``always fail`` past its budget) — + and the second turn's first request streams the normal answer (the + Retry's re-ask). +* ``partial then fail`` (``PARTIAL_FAIL_TRIGGER``): the first request + streams ``PARTIAL_FAIL_TEXT`` in ordinary delta chunks, then the + mock's generator RAISES — a genuine mid-stream connection reset: the + app's ``chat_stream_retried`` re-raises (a piece already emitted — + the locked retry-before-first-frame rule) and the chat endpoint + settles with the terminal SSE ``error`` frame AFTER the partial + deltas. The second request streams the normal answer. + +The e2e app server boots with ``BOR_LLM_RETRY_DELAY=0`` (conftest) so +the dead attempts are instant; ``BOR_LLM_RETRIES`` is forced to the +real default (3) so the exhaustion turn burns the REAL budget — 4 +attempts, the last ``retry`` frame reads "(4 of 4)". + +KB seed (the ``test_llm_retry.py`` direct-seed pattern): ONE fixture +document (``homelab/kubernetes.md``) — the "How is my Kubernetes +cluster set up?" questions FTS-hit it → HIGH → the grounded path (the +re-asked turn streams the grounded ``MOCK_ANSWER_MARKER`` answer). + +Test → contract mapping (locked A1: a failed turn persists as a brain +record with the ``failed`` marker + the capped ``error`` detail; the +banner Retry stays as-is — ``lastBrainWrap`` simply EXISTS on the error +paths now): + +1. ``test_network_error_shows_banner_retry_and_failed_bubble`` — + A (zero-frame network error): the banner (role=alert) is visible + WITH the phase-111 Retry button (its ``lastBrainWrap`` precondition + finally holds), a failed bubble with the fixed honest line + the + in-bubble error note (the detail) + the in-bubble Retry exists, the + record persists ``failed: true`` + ``error`` in localStorage, and + the wire is retry×3 + terminal error with NO delta/done. Clicking + the BANNER Retry re-asks WITHOUT re-typing (no second user + bubble), the mock now answers, the grounded answer streams in + place, and the failed bubble is gone (redo-in-place). +2. ``test_partial_then_error_keeps_partial_with_note_and_retry`` — + B (SSE error frame after partial deltas): the partial bubble KEEPS + its streamed text, gains the in-bubble error note (the detail) + + the in-bubble Retry, the record persists the RAW partial + + ``failed: true`` + ``error``, and the wire is deltas-then-terminal- + error with no done. Clicking the IN-BUBBLE Retry re-asks in place + (the failed record is replaced by the fresh grounded answer — no + re-typing, no failed note). +3. ``test_failed_turn_survives_a_refresh`` — C (the refresh case): + after a zero-frame failure, ``page.reload()`` restores the question + + the failed bubble (error detail visible) WITH a working Retry + button on it — no "new chat" required — and clicking it re-asks + (the grounded answer replaces the failed record). +4. ``test_stopped_turn_is_not_a_failed_turn`` — negative (phase 48 + unchanged): a user-stopped turn restores with the "Stopped" note + and NOT a failed note — the two markers are mutually exclusive by + construction (the stop path never touches the failed funnel). +""" +from __future__ import annotations + +import hashlib +import json +import re +import time +from collections.abc import Iterator +from datetime import UTC, datetime +from pathlib import Path + +import pytest +from playwright.sync_api import Page, expect +from sqlalchemy import text +from sqlalchemy.orm import Session + +from app.db import SessionLocal +from app.models import Chunk, Document +from e2e.auth_helpers import login +from tests.e2e.mock_llm import PARTIAL_FAIL_TEXT, embed_text + +REPO = Path(__file__).resolve().parents[2] + +# -------------------------------------------------------------------------- +# Seed + questions (see the module docstring for the gate notes) +# -------------------------------------------------------------------------- + +SEED_SOURCE = "docs" +SEED_PATH = "homelab/kubernetes.md" +KUB_CONTENT = (REPO / "tests" / "fixtures" / "docs" / SEED_PATH).read_text() + +#: A (zero-frame network error) + C (refresh) — HIGH turn (FTS-hits the +#: seed) so the re-asked turn streams the GROUNDED mock answer. +FAIL_Q = "How is my Kubernetes cluster set up? fail first turn" +#: B (partial deltas + the terminal error frame) — HIGH turn, same +#: re-ask contract. +PARTIAL_Q = "How is my Kubernetes cluster set up? partial then fail" +#: Negative: the mock's ~8 s long answer (12 chars / 0.02 s) — the +#: comfortable stop window (the phase-48 suite's phrasing). +STOP_Q = "How is my Kubernetes cluster set up? write a long answer" + +#: The app's terminal LLM-failure copy (app/api/chat.py's LLMError +#: frame) — the error detail the note/banner/record all carry. +ERROR_COPY = "The chat model dropped the connection — try again?" +MOCK_ANSWER_MARKER = "Deterministic mock answer for E2E" + +#: The conftest forces BOR_LLM_RETRIES to its default (3) → 4 attempts +#: total; the retry-frame math in the assertions is fixed by that. +MAX_ATTEMPTS = 4 + +STORAGE_KEY = "bor.chat.v1" + + +def _js_const(name: str) -> str: + """A frontend string constant, read from app.js — the E2E asserts + against the SAME text the page renders (no JS/Python drift).""" + js = (REPO / "frontend" / "assets" / "app.js").read_text(encoding="utf-8") + m = re.search(rf'const {name}\s*=\s*\n?\s*"([^"]*)"', js) + assert m, f"const {name} not found in app.js" + return m.group(1) + + +FAILED_TURN_TEXT = _js_const("FAILED_TURN_TEXT") + + +# -------------------------------------------------------------------------- +# DB seeding (TRUNCATE-then-seed, cf. test_llm_retry.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 byte-stable regardless of leftovers from other suites. + """ + with SessionLocal() as db: + db.execute( + text("TRUNCATE chunks, documents, query_log, steering_notes, kb_overview") + ) + db.commit() + _seed(db) + db.commit() + + +@pytest.fixture(autouse=True) +def clean_chats() -> Iterator[None]: + """The send auto-saves a ``saved_chats`` row per turn (phase 55) — + truncate around every test so the suite starts from (and leaves) + an empty deployment (the phase-80/103 isolation pattern).""" + with SessionLocal() as db: + db.execute(text("TRUNCATE saved_chats")) + db.commit() + yield + # The auto-save is fire-and-forget (the browser never awaits the + # PUT): a short margin lets the last turn's PUT land server-side + # before the TRUNCATE, so the teardown never 500s an in-flight + # upsert (the "Could not refresh instance" race). This runs AFTER + # the page closes (LIFO finalization), so the PUT is either done + # or aborted — never in flight. + time.sleep(0.75) + with SessionLocal() as db: + db.execute(text("TRUNCATE saved_chats")) + db.commit() + + +# -------------------------------------------------------------------------- +# Page hooks (the SSE capture — the phase-37/67 pattern from +# test_llm_retry.py) + localStorage reads +# -------------------------------------------------------------------------- + +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_sse_hook(page: Page) -> None: + page.evaluate(SSE_HOOK) + + +def _frames(page: Page, terminal: str = "done") -> list[dict]: + """The captured SSE frames of the CURRENT turn, once the *terminal* + frame lands (``error`` for the failure scenarios).""" + 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 _stored(page: Page) -> dict: + """The persisted ``bor.chat.v1`` conversation (localStorage — the + phase-14 store the restore reads on refresh).""" + raw = page.evaluate(f"() => localStorage.getItem('{STORAGE_KEY}')") + assert raw is not None, "the conversation key must exist in localStorage" + return json.loads(raw) + + +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 _assert_failed_bubble( + page: Page, *, expected_text: str, note_detail: str +) -> None: + """The failed-bubble contract shared by the scenarios: ONE brain + bubble with *expected_text*, the in-bubble error note (the "Failed" + label + *note_detail*), and the in-bubble Retry button + (``markLastRetryable`` on the failed bubble — the last brain + wrap).""" + brain = page.locator(".msg.brain") + expect(brain).to_have_count(1) + expect(brain.locator(".bubble")).to_have_text(expected_text) + note = brain.locator(".failed-note") + expect(note).to_have_count(1) + expect(note).to_contain_text("Failed") + expect(note).to_contain_text(note_detail) + expect(brain.locator(".retry-btn")).to_have_count(1) + # A note, not an answer: no Save-as-doc button on the failed bubble. + expect(brain.locator(".save-as-doc-btn")).to_have_count(0) + + +def _assert_settled_composer(page: Page) -> None: + """The failed turn settles to idle: the Send button recovered + (never a zombified Stop — the §7.4 never-stale contract).""" + 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_redo_succeeded(page: Page) -> None: + """After a Retry re-ask: exactly question + the fresh GROUNDED + answer (the mock now answers) — the failed record is REPLACED in + place (no second user bubble, no failed note, no failed marker on + the new record).""" + expect(page.locator(".msg.user .bubble")).to_have_count(1) + expect(page.locator(".msg.brain")).to_have_count(1) + bubble = page.locator(".msg.brain .bubble").last + expect(bubble).to_contain_text(MOCK_ANSWER_MARKER, timeout=30_000) + expect(page.locator(".failed-note")).to_have_count(0) + # The turn has FULLY settled: the ``done`` frame is processed (the + # answer record persisted through rememberBrainTurn — synchronously + # in the frame handler) BEFORE the finally's idle state flips the + # label to Send. The label is the settle signal, so the local + # storage read below is race-free (the marker can be visible a few + # frames before ``done`` — the last deltas carry it). + expect(page.locator("#send-label")).to_have_text("Send", timeout=30_000) + stored = _stored(page) + assert [m["who"] for m in stored["messages"]] == ["user", "brain"] + last = stored["messages"][-1] + assert last.get("failed") in (None, False), "the fresh answer is not failed" + assert MOCK_ANSWER_MARKER in last["text"] + # The fresh grounded record's text is the rendered answer (no + # failed-marker leftovers in the conversation). + assert FAILED_TURN_TEXT not in last["text"] + + +# -------------------------------------------------------------------------- +# A — zero-frame network error: the banner WITH a working Retry + the +# failed bubble; clicking the banner Retry re-asks without re-typing +# -------------------------------------------------------------------------- + + +def test_network_error_shows_banner_retry_and_failed_bubble( + page: Page, app_url: str, mock_llm: int, db_ready: None +) -> None: + _reset_db() + page.set_default_timeout(30_000) + login(page, app_url, next="/") + _install_sse_hook(page) + + _submit(page, FAIL_Q) + + # After the 4th dead attempt (the forced-default budget) the turn + # dies with ZERO frames: the existing terminal error banner + # (role=alert, the "dropped the connection" copy) — NOW with its + # Retry button revealed (the phase-111 condition + # ``opts.retryable && lastBrainWrap`` finally holds: the failed + # bubble's wrap exists before the error state). + banner = page.locator("#kb-banner") + expect(banner).to_have_attribute("role", "alert", timeout=60_000) + expect(banner).to_contain_text(ERROR_COPY) + expect(page.locator("#banner-retry")).to_be_visible(timeout=10_000) + _assert_settled_composer(page) + + # The zero-frame failed bubble: the fixed honest line (NOT the raw + # detail — that rides the note), the in-bubble error note carrying + # the detail, and the in-bubble Retry. + _assert_failed_bubble(page, expected_text=FAILED_TURN_TEXT, note_detail=ERROR_COPY) + + # Persisted: the question's user record + the failed brain record — + # ``failed: true`` + the capped detail (the phase-48 ``stopped`` + # precedent in localStorage; the phase-55 auto-save rides the same + # call, so the server-side saved chat carries it too). + stored = _stored(page) + assert [m["who"] for m in stored["messages"]] == ["user", "brain"] + assert stored["messages"][0]["text"] == FAIL_Q + failed = stored["messages"][-1] + assert failed["text"] == FAILED_TURN_TEXT + assert failed["failed"] is True + assert failed["error"] == ERROR_COPY + + # Wire: the three retry frames (attempts 2–4 of 4 — the real + # budget), then the terminal error frame LAST — no delta, no done + # (ZERO frames reached the client). + frames = _frames(page, terminal="error") + assert [f for f in frames if f["type"] == "retry"] == [ + {"type": "retry", "attempt": a, "max_attempts": MAX_ATTEMPTS} + for a in (2, 3, 4) + ], frames + assert frames[-1]["type"] == "error" + assert ERROR_COPY in frames[-1]["detail"] + assert not [f for f in frames if f.get("type") in ("done", "delta")], frames + + # The BANNER Retry: re-asks WITHOUT re-typing (no second user + # bubble) — the mock now answers (the sequence resets after its + # guarded success) and the grounded answer streams in place of the + # failed bubble (redo-in-place — the failed record is popped). + page.click("#banner-retry") + _assert_redo_succeeded(page) + + +# -------------------------------------------------------------------------- +# B — SSE error frame after partial deltas: the partial keeps its text +# + the error note + the in-bubble Retry; clicking it re-asks in +# place (the failed record is replaced by the fresh answer) +# -------------------------------------------------------------------------- + + +def test_partial_then_error_keeps_partial_with_note_and_retry( + page: Page, app_url: str, mock_llm: int, db_ready: None +) -> None: + _reset_db() + page.set_default_timeout(30_000) + login(page, app_url, next="/") + _install_sse_hook(page) + + _submit(page, PARTIAL_Q) + + # The partial streams, then the mock's generator dies mid-stream: + # the app settles the turn with the terminal error banner AFTER the + # partial deltas — and the partial bubble KEEPS its text. + banner = page.locator("#kb-banner") + expect(banner).to_have_attribute("role", "alert", timeout=60_000) + expect(banner).to_contain_text(ERROR_COPY) + _assert_settled_composer(page) + + # The failed partial: the streamed text kept verbatim + the + # in-bubble error note (the detail) + the in-bubble Retry. + _assert_failed_bubble( + page, expected_text=PARTIAL_FAIL_TEXT, note_detail=ERROR_COPY + ) + + # Persisted: the RAW partial (what the user saw is what is stored — + # the phase-17/20 convention, now with the failed marker + detail). + stored = _stored(page) + assert [m["who"] for m in stored["messages"]] == ["user", "brain"] + failed = stored["messages"][-1] + assert failed["text"] == PARTIAL_FAIL_TEXT + assert failed["failed"] is True + assert failed["error"] == ERROR_COPY + + # Wire: the delta frames (the partial, in order) precede the + # terminal error frame — no done (the stream never settled), no + # retry (a piece already emitted — the locked retry-before-first- + # frame rule means the failure is terminal, never redone). + frames = _frames(page, terminal="error") + deltas = [f for f in frames if f["type"] == "delta"] + assert deltas, "no delta frames before the error" + assert "".join(d["text"] for d in deltas) == PARTIAL_FAIL_TEXT + assert not [f for f in frames if f.get("type") == "retry"], frames + assert frames[-1]["type"] == "error" + assert not [f for f in frames if f.get("type") == "done"], frames + + # The IN-BUBBLE Retry: redo-in-place — the failed partial record is + # popped + re-asked (no re-typing), and the fresh grounded answer + # (the mock now answers — the sequence resets) replaces it. + page.click(".msg.brain .retry-btn") + _assert_redo_succeeded(page) + + +# -------------------------------------------------------------------------- +# C — the refresh case: reload restores the failed bubble WITH a +# working Retry; clicking it re-asks (no "new chat" required) +# -------------------------------------------------------------------------- + + +def test_failed_turn_survives_a_refresh( + page: Page, app_url: str, mock_llm: int, db_ready: None +) -> None: + _reset_db() + page.set_default_timeout(30_000) + login(page, app_url, next="/") + _install_sse_hook(page) + + _submit(page, FAIL_Q) + + # The zero-frame failure lands (same shape as scenario A). + banner = page.locator("#kb-banner") + expect(banner).to_have_attribute("role", "alert", timeout=60_000) + expect(banner).to_contain_text(ERROR_COPY) + expect(page.locator("#banner-retry")).to_be_visible(timeout=10_000) + _assert_failed_bubble( + page, expected_text=FAILED_TURN_TEXT, note_detail=ERROR_COPY + ) + + # The refresh: the page restores from localStorage (the failed + # record persisted by the funnel) — the question + the failed + # bubble with its error detail, and the Retry button on the failed + # bubble (the restore loop's lastBrainWrap + markLastRetryable land + # it on the LAST restored brain bubble — the failed one). + page.reload() + expect(page.locator("#messages > .msg")).to_have_count(2) + expect(page.locator(".msg.user .bubble")).to_have_text(FAIL_Q) + _assert_failed_bubble( + page, expected_text=FAILED_TURN_TEXT, note_detail=ERROR_COPY + ) + # The error state itself does NOT restore (the banner is a live- + # turn state) — the recovery affordance is the in-bubble Retry. + expect(page.locator("#kb-banner")).to_be_hidden() + _assert_settled_composer(page) + + # The restored record still carries the marker (the restore is + # lossless: text + detail + marker). + stored = _stored(page) + assert [m["who"] for m in stored["messages"]] == ["user", "brain"] + assert stored["messages"][-1]["failed"] is True + assert stored["messages"][-1]["error"] == ERROR_COPY + + # The restored Retry WORKS: clicking it re-asks the question that + # precedes the failed record (retryLastTurn, unchanged) — the + # grounded answer streams and replaces the failed record. + page.click(".msg.brain .retry-btn") + _assert_redo_succeeded(page) + + +# -------------------------------------------------------------------------- +# Negative — a STOPPED turn (phase 48) is NOT a failed turn: the stop +# path restores with the "Stopped" note and never the failed note +# (mutually exclusive by construction — the stop branch is the catch's +# own, the failed funnel is the catch's else) +# -------------------------------------------------------------------------- + + +def test_stopped_turn_is_not_a_failed_turn( + page: Page, app_url: str, mock_llm: int, db_ready: None +) -> None: + _reset_db() + page.set_default_timeout(30_000) + login(page, app_url, next="/") + + # The ~8 s long answer — a comfortable stop window (phase 48). + _submit(page, STOP_Q) + answer = page.locator(".msg.brain .bubble:not(.typing)") + answer.wait_for(state="visible", timeout=30_000) + partial = "" + deadline = time.monotonic() + 15 + while time.monotonic() < deadline: + partial = answer.inner_text() + if len(partial.split()) >= 8: + break + time.sleep(0.05) + assert len(partial.split()) >= 8, "no answer deltas before the stop" + + expect(page.locator("#send-label")).to_have_text("Stop") + page.click("#send-btn") # the in-flight button IS the Stop control + + # Settled to idle: no error banner (the stop path never shows one), + # the partial kept + the "Stopped" note — and NO failed note. + expect(page.locator("#send-label")).to_have_text("Send", timeout=5_000) + expect(page.locator("#kb-banner")).to_be_hidden() + expect(page.locator(".msg.brain .stopped-note")).to_have_count(1) + expect(page.locator(".msg.brain .stopped-note")).to_contain_text("Stopped") + expect(page.locator(".msg.brain .failed-note")).to_have_count(0) + stopped_text = answer.inner_text() + assert stopped_text.strip() + + # Persisted: the stopped marker, NOT the failed marker (the two are + # mutually exclusive by construction). + stored = _stored(page) + last = stored["messages"][-1] + assert last["stopped"] is True + assert last.get("failed") in (None, False), "a stopped turn is not a failed turn" + assert "error" not in last or last["error"] is None + + # The refresh: the stopped partial restores with the "Stopped" + # note and NOT the failed note — phase 48's restore is unchanged. + page.reload() + expect(page.locator("#messages > .msg")).to_have_count(2) + expect(page.locator(".msg.user .bubble")).to_have_text(STOP_Q) + expect(page.locator(".msg.brain .bubble:not(.typing)")).to_have_text( + stopped_text + ) + expect(page.locator(".msg.brain .stopped-note")).to_have_count(1) + expect(page.locator(".msg.brain .failed-note")).to_have_count(0) + expect(page.locator("#kb-banner")).to_be_hidden() + stored = _stored(page) + assert stored["messages"][-1]["stopped"] is True + assert stored["messages"][-1].get("failed") in (None, False) diff --git a/tests/e2e/test_llm_retry.py b/tests/e2e/test_llm_retry.py index 64d670d..e6de9e1 100644 --- a/tests/e2e/test_llm_retry.py +++ b/tests/e2e/test_llm_retry.py @@ -60,6 +60,11 @@ Test → source mapping: 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). + Phase 120 (locked A1): the zero-frame turn ALSO renders the + retryable failed bubble (the fixed honest line + the in-bubble + error note with the detail + the in-bubble Retry) and reveals the + banner Retry (its ``lastBrainWrap`` precondition finally holds on + the error path) — a network error is retryable. """ from __future__ import annotations @@ -115,6 +120,21 @@ 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?" + +def _js_const(name: str) -> str: + """A frontend string constant, read from app.js (no JS/Python + drift — the E2E asserts against the SAME text the page renders).""" + js = (REPO / "frontend" / "assets" / "app.js").read_text(encoding="utf-8") + m = re.search(rf'const {name}\s*=\s*\n?\s*"([^"]*)"', js) + assert m, f"const {name} not found in app.js" + return m.group(1) + + +#: Phase 120: the fixed bubble text of a ZERO-frame failed turn (the +#: exhaustion case below) — read from app.js so this suite's assertion +#: tracks the page's constant. +FAILED_TURN_TEXT = _js_const("FAILED_TURN_TEXT") + # -------------------------------------------------------------------------- # DB seeding (TRUNCATE-then-seed, cf. test_agent_document_tools.py) # -------------------------------------------------------------------------- @@ -501,7 +521,18 @@ def test_exhaustion_lands_on_the_error_banner( 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) + # Phase 120 (locked A1): the zero-frame turn is no longer a bare + # question — it persists as a FAILED brain record (the phase-48 + # ``stopped`` precedent) and renders the retryable error bubble: + # the fixed honest line (no frame ever streamed a token, so the + # bubble is NOT the empty-answer fallback), the in-bubble error + # note carrying the detail, the in-bubble Retry, and the banner + # Retry revealed too (its ``lastBrainWrap`` precondition finally + # holds on the error path — a network error is retryable). + brain = page.locator("#messages > .msg.brain") + expect(brain).to_have_count(1) + expect(brain.locator(".bubble")).to_have_text(FAILED_TURN_TEXT) + expect(brain.locator(".failed-note")).to_contain_text(ERROR_COPY) + expect(brain.locator(".retry-btn")).to_have_count(1) + expect(page.locator("#banner-retry")).to_be_visible() expect(page.locator(".msg.user .bubble")).to_have_count(1) diff --git a/tests/e2e/test_navbar_refresh.py b/tests/e2e/test_navbar_refresh.py index ee679d3..cb11667 100644 --- a/tests/e2e/test_navbar_refresh.py +++ b/tests/e2e/test_navbar_refresh.py @@ -628,7 +628,7 @@ def test_api_created_chats_carry_the_bor_chat_v1_shape( # brain records (accepted by the ChatMessage schema — its earlier # absence 422'd the done-time auto-save, phase-118 verification fix). allowed = {"who", "text", "sources", "related", "deflected", "suggestions", - "thinking", "tools", "stopped"} + "thinking", "tools", "stopped", "failed", "error"} assert all({"who", "text"} <= set(m) <= allowed for m in body["messages"]) # The History row renders it (the Open link's text IS the title). page.click("#nav-history") diff --git a/tests/e2e/test_tool_scaffolding_guardrails.py b/tests/e2e/test_tool_scaffolding_guardrails.py index 4fab32f..ef35123 100644 --- a/tests/e2e/test_tool_scaffolding_guardrails.py +++ b/tests/e2e/test_tool_scaffolding_guardrails.py @@ -48,10 +48,13 @@ Test → phase mapping (Playwright Mapping Rule): 2. ``test_terminal_scaffolding_lands_on_dedicated_error_app_stays_usable`` — the terminal case: the existing error state renders with the dedicated copy ("The model returned a malformed reply — please try - again."), no raw tokens in the DOM, no answer bubble, no ``done`` - and NO ``query_log`` row (the existing terminal-error semantics) — - and the app stays usable: a follow-up plain question in the same - session gets a normal deflected answer and the banner clears. + again."), no raw tokens in the DOM, no answer bubble (phase 120: + the zero-frame turn renders the FAILED note bubble — the fixed + honest line + the error note + the Retry, NOT an answer), no + ``done`` and NO ``query_log`` row (the existing terminal-error + semantics) — and the app stays usable: a follow-up plain question + in the same session gets a normal deflected answer and the banner + clears. 3. ``test_plain_turn_never_recovers_and_streams_byte_clean`` — no false positive: a plain deflected question streams its first-request answer byte-clean (the concatenated delta text is @@ -64,6 +67,7 @@ from __future__ import annotations import json import re import time +from pathlib import Path from playwright.sync_api import Page, expect from sqlalchemy import select, text @@ -73,6 +77,24 @@ from app.models import QueryLog from e2e.auth_helpers import login from tests.e2e.mock_llm import SCAFFOLD_RECOVERY_ANSWER, compose_answer +REPO = Path(__file__).resolve().parents[2] + + +def _js_const(name: str) -> str: + """A frontend string constant, read from app.js (no JS/Python + drift — the E2E asserts against the SAME text the page renders).""" + js = (REPO / "frontend" / "assets" / "app.js").read_text(encoding="utf-8") + m = re.search(rf'const {name}\s*=\s*\n?\s*"([^"]*)"', js) + assert m, f"const {name} not found in app.js" + return m.group(1) + + +#: Phase 120: the fixed bubble text of a ZERO-frame failed turn (the +#: terminal scaffolding case below) — read from app.js so this suite's +#: assertion tracks the page's constant. +FAILED_TURN_TEXT = _js_const("FAILED_TURN_TEXT") + + # -------------------------------------------------------------------------- # Questions + the mock's deterministic expectations # -------------------------------------------------------------------------- @@ -364,9 +386,20 @@ def test_terminal_scaffolding_lands_on_dedicated_error_app_stays_usable( expect(page.locator("#send-btn")).to_be_enabled(timeout=30_000) expect(page.locator("#send-label")).to_have_text("Send", timeout=30_000) - # No answer bubble was ever rendered (every streamed frame was + # No ANSWER bubble was ever rendered (every streamed frame was # stripped server-side) and no raw token is anywhere in the DOM. - expect(page.locator("#messages > .msg.brain")).to_have_count(0) + # Phase 120 (locked A1): the zero-frame turn persists as a FAILED + # brain record (the phase-48 ``stopped`` precedent) and renders the + # retryable error bubble — the fixed honest line (NOT an answer), + # the in-bubble error note carrying the dedicated copy, the + # in-bubble Retry, and the banner Retry revealed too (its + # ``lastBrainWrap`` precondition finally holds on the error path). + brain = page.locator("#messages > .msg.brain") + expect(brain).to_have_count(1) + expect(brain.locator(".bubble")).to_have_text(FAILED_TURN_TEXT) + expect(brain.locator(".failed-note")).to_contain_text(MALFORMED_ERROR_COPY) + expect(brain.locator(".retry-btn")).to_have_count(1) + expect(page.locator("#banner-retry")).to_be_visible() _assert_no_raw_tokens(page) # Wire: the terminal error frame is LAST — no done, and NO delta diff --git a/tests/integration/test_chats_api.py b/tests/integration/test_chats_api.py index f19bc89..348f374 100644 --- a/tests/integration/test_chats_api.py +++ b/tests/integration/test_chats_api.py @@ -11,7 +11,8 @@ unchanged: the auto-title convention (first user message, whitespace-collapsed, 120-char cap + the no-user-message fallback), the list order (``updated_at desc, id desc``), the full-payload round-trip (a ``bor.chat.v1``-shaped brain record carrying -``sources``/``thinking``/``tools``/``stopped`` survives losslessly), +``sources``/``thinking``/``tools``/``stopped``/``failed``/``error`` +survives losslessly), the PUT upsert semantics (replacement + title-keep + title-set + ``updated_at`` bump), the delete 404/204, and (phase 53, task 03) the sources-version stamp + ``stale`` flag: create and re-Save stamp the @@ -103,6 +104,12 @@ FULL_BRAIN: dict[str, Any] = { }, ], "stopped": False, + # Phase 120 (task 01): the failed-turn marker + the persisted error + # detail — the phase-48 ``stopped`` precedent. A FULL brain record + # carries the keys (``failed: False`` = the marker is explicit, not + # absent; the round-trip stays byte-identical through them). + "failed": False, + "error": None, } OUT_KEYS = { @@ -191,6 +198,8 @@ def _expect(records: list[dict[str, Any]]) -> list[dict[str, Any]]: "thinking": m.get("thinking"), "tools": m.get("tools"), "stopped": m.get("stopped"), + "failed": m.get("failed"), + "error": m.get("error"), } for m in records ] @@ -1185,3 +1194,115 @@ def test_put_rejects_oversized_message_and_leaves_row_unchanged( assert got.json()["messages"] == _expect(_simple_conversation()) assert got.json()["message_count"] == 2 # original count, not the rejected 1 + +# ---------- failed-turn records (phase 120, task 01 — locked A1: a +# failed chat turn persists as a BRAIN record with the `failed` marker +# + the capped `error` detail, the phase-48 `stopped` precedent — no +# separate error table, no new API) ---------- + +#: The failed record shape exactly as the client persists it (the +#: zero-frame network-error case — ``finalizeFailedTurn``'s +#: FAILED_TURN_TEXT bubble + the terminal error detail). +FAILED_BRAIN: dict[str, Any] = { + "who": "brain", + "text": "My answer didn't make it — the connection dropped. Use Retry to ask again.", + "failed": True, + "error": "The chat model dropped the connection — try again?", +} + + +def test_create_round_trips_failed_record_byte_identical( + admin_client: TestClient, +) -> None: + """``POST /api/chats`` with a failed brain record returns it + byte-identically (the phase-50 contract through the new keys), + ``GET`` survives the trip to Postgres and back, and a ``PUT`` + re-Save round-trips it too (the re-Save upsert keeps the marker + + detail — the auto-save rides exactly this path).""" + records = [_user(FIRST_QUESTION), FAILED_BRAIN] + + r = admin_client.post("/api/chats", json={"messages": records}) + assert r.status_code == 201 + body = r.json() + assert body["messages"] == _expect(records) + assert body["messages"][1]["failed"] is True + assert body["messages"][1]["error"] == FAILED_BRAIN["error"] + + got = admin_client.get(f"/api/chats/{body['id']}") + assert got.status_code == 200 + assert got.json()["messages"] == _expect(records) + + r2 = admin_client.put( + f"/api/chats/{body['id']}", json={"messages": records} + ) + assert r2.status_code == 200 + assert r2.json()["messages"] == _expect(records) + + +def test_post_rejects_error_over_500_and_stores_nothing( + client: TestClient, admin_client: TestClient +) -> None: + """The phase-83 style value bound on the new key: a 501-char + ``error`` 422s at the boundary (``ChatMessage.error`` + ``max_length=500``) and NOTHING is stored — the hostile detail + string never lands in the JSONB.""" + baseline = admin_client.get("/api/chats").json()["chats"] + + bad = dict(FAILED_BRAIN) + bad["error"] = "e" * 501 + r = client.post( + "/api/chats", + json={"messages": [_user("hi"), bad]}, + ) + assert r.status_code == 422 + + assert admin_client.get("/api/chats").json()["chats"] == baseline + + +def test_put_rejects_error_over_500_and_leaves_row_unchanged( + client: TestClient, admin_client: TestClient +) -> None: + """The re-Save path is gated by the SAME bound: a 501-char + ``error`` 422s and the row keeps its original payload + byte-for-byte.""" + created = client.post( + "/api/chats", json={"messages": _simple_conversation()} + ).json() + + bad = dict(FAILED_BRAIN) + bad["error"] = "e" * 501 + r = client.put( + f"/api/chats/{created['id']}", + json={"messages": [bad]}, + ) + assert r.status_code == 422 + + got = admin_client.get(f"/api/chats/{created['id']}") + assert got.status_code == 200 + assert got.json()["messages"] == _expect(_simple_conversation()) + + +def test_shared_chat_with_failed_record_serves_public_shape( + admin_client: TestClient, +) -> None: + """The phase-51 public read is UNCHANGED by the failed record: + ``GET /api/shared/`` still serves exactly the public shape + (``title`` + ``messages`` — no id, no timestamps, no token) and the + failed record rides the snapshot verbatim (the shared page renders + its ``text`` as-is — no note, no Retry, read-only by design).""" + records = [_user(FIRST_QUESTION), FAILED_BRAIN] + created = admin_client.post( + "/api/chats", + json={"title": EXPLICIT_TITLE, "messages": records, "share": True}, + ).json() + + anon = TestClient(fastapi_app) # fresh jar: truly anonymous + r = anon.get(f"/api{created['share_url']}") + assert r.status_code == 200 + body = r.json() + assert set(body) == SHARED_OUT_KEYS # title + messages — the public shape + assert body["title"] == EXPLICIT_TITLE + assert body["messages"] == _expect(records) + assert body["messages"][1]["failed"] is True + assert body["messages"][1]["error"] == FAILED_BRAIN["error"] + diff --git a/tests/unit/test_chat_message_failed.py b/tests/unit/test_chat_message_failed.py new file mode 100644 index 0000000..fcdda58 --- /dev/null +++ b/tests/unit/test_chat_message_failed.py @@ -0,0 +1,196 @@ +"""Unit: the failed-turn schema boundary (phase 120, task 03). + +Phase 120 (task 01, locked A1) added two OPTIONAL keys to +``ChatMessage`` (``app/schemas.py``) — ``failed`` (a bool marker) and +``error`` (the persisted error detail, capped at 500) — the phase-48 +``stopped`` precedent: a FAILED chat turn (network error, SSE ``error`` +frame, stream drop) persists as a BRAIN record +``{who: "brain", text: , failed: true, error: +}`` — no separate error table, no new API (``retryLastTurn``'s +pop-the-last-brain-record logic works on a failed record UNCHANGED). + +This module pins the boundary the phase plan names: + +* a failed record (``failed: true`` + ``error``) validates and + round-trips losslessly; +* ``error`` of 501 chars 422s at the boundary (the phase-83 + value-bounds style; exactly 500 passes); +* an unknown key still 422s (``extra="forbid"`` intact — the new keys + are DECLARED, they did not loosen the boundary); +* a record WITHOUT the new keys validates, serializes with the new keys + as explicit nulls, and — on every pre-phase key — is byte-identical + to the pre-phase-120 stored shape (the phase-50 contract: the server + stores ``model_dump()`` without ``exclude_none``, so a pre-phase + round-trip is untouched apart from the two added nulls). + +House convention: pure schema tests (no DB, no client) — the API-level +round-trip pins live in ``tests/integration/test_chats_api.py``. +""" +from __future__ import annotations + +import pytest +from pydantic import ValidationError + +from app.schemas import ChatMessage + +#: The phase-120 failed record shape (locked A1) — the zero-frame +#: network-error case exactly as ``finalizeFailedTurn`` persists it. +FAILED_RECORD: dict = { + "who": "brain", + "text": "My answer didn't make it — the connection dropped. Use Retry to ask again.", + "failed": True, + "error": "The chat model dropped the connection — try again?", +} + + +# ---------- acceptance: the failed record validates + round-trips ---------- + + +def test_failed_record_validates() -> None: + """A failed brain record (``failed: true`` + the capped ``error`` + detail) crosses the boundary and the fields survive the trip.""" + msg = ChatMessage.model_validate(FAILED_RECORD) + assert msg.who == "brain" + assert msg.text == FAILED_RECORD["text"] + assert msg.failed is True + assert msg.error == FAILED_RECORD["error"] + + +def test_failed_record_round_trips_losslessly() -> None: + """The stored shape (``model_dump`` — plain, no ``exclude_none``, + the phase-50 storage convention) keeps the marker + detail and + fills the remaining optional keys with explicit nulls (the + restore path is null-safe).""" + dumped = ChatMessage.model_validate(FAILED_RECORD).model_dump() + assert dumped["failed"] is True + assert dumped["error"] == FAILED_RECORD["error"] + for key in ("sources", "related", "deflected", "suggestions", "thinking", "tools", "stopped"): + assert dumped[key] is None, f"{key} must be an explicit null, got {dumped[key]!r}" + # Re-validate the stored shape — the round-trip is lossless. + assert ChatMessage.model_validate(dumped).model_dump() == dumped + + +def test_failed_false_is_an_explicit_marker() -> None: + """``failed: false`` is a legal value (a full ``bor.chat.v1`` brain + record carries the key explicitly — the integration FULL_BRAIN + round-trip relies on it); it must not be coerced to absent.""" + msg = ChatMessage.model_validate( + {"who": "brain", "text": "hi", "failed": False, "error": None} + ) + assert msg.failed is False + assert msg.error is None + assert msg.model_dump()["failed"] is False + + +# ---------- the value bounds: error ≤ 500 (phase-83 style) ---------- + + +def test_error_at_500_chars_passes() -> None: + """The bound is inclusive: exactly 500 chars validate.""" + msg = ChatMessage.model_validate( + {"who": "brain", "text": "hi", "failed": True, "error": "e" * 500} + ) + assert len(msg.error or "") == 500 + + +def test_error_over_500_chars_is_rejected() -> None: + """501 chars 422s at the boundary (the API surfaces this as a 422 — + the hostile detail string is capped at the schema, the + ``finalizeFailedTurn`` 500-char slice is the UI-side first cut).""" + with pytest.raises(ValidationError): + ChatMessage.model_validate( + {"who": "brain", "text": "hi", "failed": True, "error": "e" * 501} + ) + + +# ---------- the boundary stays strict: extra="forbid" intact ---------- + + +def test_unknown_key_is_still_rejected() -> None: + """``extra="forbid"`` was NOT loosened by the new keys: a stray + key still rejects at the boundary (the corrupted / HTML-shaped + payload defense, unchanged).""" + with pytest.raises(ValidationError): + ChatMessage.model_validate({"who": "brain", "text": "hi", "foo": 1}) + + +def test_unknown_key_is_rejected_even_with_the_new_keys_present() -> None: + """A record carrying the new keys AND an unknown key still + rejects — the new declarations did not widen the accepted key set + beyond ``failed``/``error``.""" + with pytest.raises(ValidationError): + ChatMessage.model_validate( + {"who": "brain", "text": "hi", "failed": True, "error": "x", "foo": 1} + ) + + +# ---------- backward compatibility: the pre-phase-120 shape ---------- + + +def test_record_without_the_new_keys_validates_with_none() -> None: + """A pre-phase record (no ``failed``/``error`` keys) still + validates; both new fields default to ``None`` (they round-trip as + nulls — absent/None, exactly like ``stopped`` today).""" + msg = ChatMessage.model_validate({"who": "brain", "text": "hi"}) + assert msg.failed is None + assert msg.error is None + dumped = msg.model_dump() + assert dumped["failed"] is None + assert dumped["error"] is None + + +def test_pre_phase_record_round_trips_byte_identical_on_existing_keys() -> None: + """The phase-50 contract through the new schema: a pre-phase-120 + STORED record (every phase-50 key present, explicit nulls where an + optional key does not apply) validates, and re-serializes + byte-identically on EVERY pre-phase key — the only diff is the two + added keys as explicit nulls. Old saved chats and shared links + therefore render/restore exactly as before.""" + pre_phase: dict = { + "who": "brain", + "text": "Your k3s cluster runs on three nodes — you've got this.", + "sources": [{"source": "Homelab", "path": "kubernetes.md", "title": "Kubernetes"}], + "related": [{"source": "Homelab", "path": "traefik.md", "title": "Traefik"}], + "deflected": False, + "suggestions": ["What ports does Traefik expose?"], + "thinking": "scratchpad", + "tools": [ + { + "name": "read", + "argument": "Homelab/kubernetes.md", + "truncated": False, + "chars_shown": None, + "chars_total": None, + } + ], + "stopped": None, + } + dumped = ChatMessage.model_validate(pre_phase).model_dump() + # Every pre-phase key survives byte-identical… + for key, value in pre_phase.items(): + assert dumped[key] == value, f"pre-phase key {key!r} changed: {dumped[key]!r}" + # …and the ONLY additions are the two new keys as explicit nulls. + assert set(dumped) == set(pre_phase) | {"failed", "error"} + assert dumped["failed"] is None + assert dumped["error"] is None + # The stored shape re-validates (round-trip through the DB JSONB). + assert ChatMessage.model_validate(dumped).model_dump() == dumped + + +def test_minimal_user_record_unchanged() -> None: + """A user record (no brain metadata at all) is untouched by the + phase: it validates and carries the new keys as nulls only.""" + dumped = ChatMessage.model_validate({"who": "user", "text": "hi"}).model_dump() + assert dumped == { + "who": "user", + "text": "hi", + "sources": None, + "related": None, + "deflected": None, + "suggestions": None, + "thinking": None, + "tools": None, + "stopped": None, + "failed": None, + "error": None, + } diff --git a/tests/unit/test_frontend_failed_turn.py b/tests/unit/test_frontend_failed_turn.py new file mode 100644 index 0000000..5466649 --- /dev/null +++ b/tests/unit/test_frontend_failed_turn.py @@ -0,0 +1,468 @@ +"""Unit: the failed-turn frontend contract (phase 120, task 03). + +Pins the phase-120 client contract as source assertions (the house +``test_frontend_*`` style — no browser): the three live failure paths +route through the single ``finalizeFailedTurn`` funnel (persist +``failed: true`` + the capped detail, end retryable), the zero-frame +fallback bubble persists the marker too, ``appendFailedNote`` mirrors +the stopped note (one per bubble, textContent-only detail), +``FAILED_TURN_TEXT`` is a distinct constant (NOT +``EMPTY_ANSWER_FALLBACK``), the restore branch renders the failed note +and excludes the Save-as-doc / Tune buttons, and — the phase's explicit +"NOT touched" contract — ``showErrorBanner`` and ``retryLastTurn`` are +byte-unchanged (their full sources are pinned below). +""" +from __future__ import annotations + +import re +from pathlib import Path + +FRONTEND = Path(__file__).resolve().parents[2] / "frontend" +APP_JS = FRONTEND / "assets" / "app.js" +STYLES_CSS = FRONTEND / "assets" / "styles.css" + + +def _js() -> str: + return APP_JS.read_text(encoding="utf-8") + + +def _css() -> str: + return STYLES_CSS.read_text(encoding="utf-8") + + +def _find_body_brace(js: str, start: int) -> int: + """The index of the REAL opening brace of a function at *start* — + the first ``{`` OUTSIDE the parameter list (paren depth 0), so + empty object defaults (``opts = {}``) and destructured parameters + (``{ acc, thinking }``) are skipped (the phase-111 banner-test + convention, extended).""" + i = start + paren = 0 + while i < len(js): + c = js[i] + if c == "(": + paren += 1 + elif c == ")": + paren -= 1 + elif c == "{" and paren == 0: + return i + i += 1 + return -1 + + +def _fn_source(js: str, name: str) -> str: + """The full source of ``function name(…){…}`` (signature + body).""" + start = js.index(f"function {name}(") + brace = _find_body_brace(js, start) + depth = 0 + i = brace + while True: + c = js[i] + if c == "{": + depth += 1 + elif c == "}": + depth -= 1 + if depth == 0: + break + i += 1 + return js[start : i + 1] + + +def _fn_body(js: str, name: str) -> str: + """Just the body of ``function name(…){…}`` (between the braces).""" + src = _fn_source(js, name) + start = js.index(f"function {name}(") + brace = _find_body_brace(js, start) + rel = brace - start + return src[rel + 1 : len(src) - 1] + + +# --------------------------------------------------------------------------- +# appendFailedNote — the in-bubble error note (the stopped-note mirror) +# --------------------------------------------------------------------------- + + +def test_append_failed_note_exists_and_guards_dedup() -> None: + """``appendFailedNote(wrap, detail)`` exists and mirrors + ``appendStoppedNote``: it reuses the ``.msg-meta`` row and adds at + most ONE ``.failed-note`` per bubble (the duplicate guard).""" + js = _js() + body = _fn_body(js, "appendFailedNote") + assert '.msg-body' in body, "the note must land in the bubble's .msg-body" + assert ".msg-meta" in body, "the note rides the existing .msg-meta row" + assert '.failed-note' in body + assert ( + 'if (meta.querySelector(".failed-note")) return;' in body + ), "one .failed-note per bubble (the duplicate guard, the stopped-note way)" + assert 'note.className = "failed-note";' in body + + +def test_append_failed_note_is_text_and_color_never_color_alone() -> None: + """The note is TEXT + color (B5): the "Failed" label is set through + ``textContent``, the icon is aria-hidden decoration, and the detail + goes through ``textContent`` too — the error string is NEVER + innerHTML (no HTML from an error, ever).""" + js = _js() + body = _fn_body(js, "appendFailedNote") + assert 'label.textContent = "Failed";' in body + assert 'd.className = "failed-detail"' in body + assert "d.textContent = detail" in body, "the detail is textContent, never innerHTML" + # The icon itself is the only innerHTML — the static SVG constant, + # aria-hidden decoration (the accessible meaning is the label + + # detail text — B5: text + color, never color alone). + assert "note.innerHTML = FAILED_ICON;" in body + m_icon = re.search(r"const FAILED_ICON\s*=\s*\n?\s*'([^']*)'", js) + assert m_icon, "const FAILED_ICON must exist" + assert 'aria-hidden="true"' in m_icon.group(1), "the icon is decoration" + + +def test_failed_turn_text_is_a_distinct_constant() -> None: + """``FAILED_TURN_TEXT`` is its OWN string literal — NOT + ``EMPTY_ANSWER_FALLBACK`` (that constant stays the + zero-frame-but-COMPLETED case's answer text) and a short honest + "my answer didn't make it" line (not the answer text, not the raw + detail).""" + js = _js() + m_fail = re.search(r'const FAILED_TURN_TEXT\s*=\s*\n?\s*"([^"]*)"', js) + assert m_fail, "const FAILED_TURN_TEXT must be a string literal" + failed_text = m_fail.group(1) + assert failed_text, "FAILED_TURN_TEXT must be non-empty" + + m_empty = re.search(r'const EMPTY_ANSWER_FALLBACK\s*=\s*\n?\s*"([^"]*)"', js) + assert m_empty, "const EMPTY_ANSWER_FALLBACK must still be a string literal" + assert failed_text != m_empty.group(1), ( + "FAILED_TURN_TEXT must be DISTINCT from EMPTY_ANSWER_FALLBACK" + ) + # The zero-frame branch uses the constant, not a copy of the + # fallback. + assert 'addMessage("brain", FAILED_TURN_TEXT);' in js + + +# --------------------------------------------------------------------------- +# finalizeFailedTurn — the single funnel for the live failure paths +# --------------------------------------------------------------------------- + + +def test_finalize_failed_turn_persists_failed_in_both_shapes() -> None: + """The funnel persists ``failed: true`` in BOTH shapes (partial + wrap + the zero-frame bubble) with the capped detail, and the + partial keeps the streamed text (``acc``).""" + js = _js() + body = _fn_body(js, "finalizeFailedTurn") + # Both branches persist the marker… + assert body.count("failed: true") == 2, ( + "both funnel shapes must persist failed: true" + ) + # …the detail trimmed + capped at 500 before persistence (the + # schema's ChatMessage.error bound is the backstop)… + assert '(detail || "").trim().slice(0, 500)' in body + # …the zero-frame shape creates the FAILED_TURN_TEXT bubble… + assert 'addMessage("brain", FAILED_TURN_TEXT);' in body + assert "appendFailedNote(fwrap, error);" in body + # …and the partial shape settles the block + calls closed (the + # stop-finalize pattern) and adds the note. + assert "closeThinkingBlock(wrap);" in body + assert "closeToolCalls(wrap);" in body + assert "appendFailedNote(wrap, error);" in body + # Both shapes land the record through rememberBrainTurn (local + # storage + the phase-55 auto-save ride) and set lastBrainWrap + # BEFORE the caller's setUiState(error, …). + assert body.count("rememberBrainTurn(") == 2 + assert body.count("lastBrainWrap =") == 2 + + +def test_finalize_failed_turn_ends_retryable() -> None: + """The funnel ENDS with ``markLastRetryable()`` (the last statement + — a trailing comment is fine) — the in-bubble Retry button lands + on the failed bubble (the last brain wrap).""" + js = _js() + body = _fn_body(js, "finalizeFailedTurn").rstrip() + last_line = body.splitlines()[-1].strip() + assert last_line.startswith("markLastRetryable();"), ( + "finalizeFailedTurn must end with the markLastRetryable() call" + ) + + +def test_error_catch_else_routes_through_the_funnel() -> None: + """The error catch's ``else`` branch (non-abort, non-stop — network + error, pre-stream HTTP error, the SSE ``error`` frame's throw) + calls ``finalizeFailedTurn`` BEFORE ``setUiState(UI_STATE.error, …)`` + — the funnel sets lastBrainWrap, so the banner's EXISTING + ``opts.retryable && lastBrainWrap`` condition reveals the Retry.""" + js = _js() + # The stop branch ends where the plain else begins. + stop_branch = js.index('} else if (stoppedByUser || err?.name === "AbortError") {') + else_branch = js.index("} else {", stop_branch) + finally_branch = js.index("} finally {", else_branch) + else_body = js[else_branch:finally_branch] + assert "finalizeFailedTurn(detail, {" in else_body + # The persistence (the funnel) precedes the error state — the + # banner's Retry precondition is set before the banner shows. + assert else_body.index("finalizeFailedTurn(detail, {") < else_body.index( + "setUiState(UI_STATE.error, detail," + ) + + +def test_navigate_away_is_not_a_failed_turn() -> None: + """A REAL departure is not a failed turn (phase-120 verification + fix): the pagehide handler sets a turn-scoped ``leftThePage`` flag + (module scope, like ``persistedOnLeave``), and the error catch's + ``else`` branch skips the failed funnel when it is set — the + browser's teardown rejection of the cancelled in-flight fetch (a + TypeError, NOT an AbortError) must not persist a failed brain + record: the phase-20 convention stands (thinking-only + navigate-away persists nothing brain-side; a partial navigate-away + persists the pagehide partial as a plain record). The flag is set + unconditionally on pagehide (a merely-hidden tab in some browsers + also fires it — the stream keeps arriving, so no rejection + follows and it stays inert there) and reset per turn at the top of + ``runTurn`` with the other turn locals.""" + js = _js() + # Module-scope declaration (column 0), exactly once. + assert re.search(r"^let leftThePage = false", js, re.M), ( + "leftThePage must be a module-scope flag (the pagehide handler " + "reads it), like persistedOnLeave" + ) + assert js.count("let leftThePage = false;") == 1 + + # Set as the FIRST statement of the pagehide handler — before the + # persistedOnLeave early return (a thinking-only navigate-away + # returns early, but the flag must be set for the funnel skip). + ph = js.index('window.addEventListener("pagehide", () => {') + ph_end = js.index("\n});", ph) + ph_body = js[ph:ph_end] + idx_flag = ph_body.find("leftThePage = true;") + idx_return = ph_body.find("if (persistedOnLeave) return;") + assert 0 <= idx_flag < idx_return, ( + "leftThePage must be set BEFORE the pagehide early returns" + ) + + # Reset per turn at the top of runTurn (the persistedOnLeave group). + turn = js.index("async function runTurn") + abort_idx = js.index("turnAbort = new AbortController()", turn) + turn_top = js[turn:abort_idx] + assert "leftThePage = false;" in turn_top, ( + "leftThePage must be reset per turn at the top of the turn handler" + ) + assert turn_top.index("persistedOnLeave = false;") < turn_top.index( + "leftThePage = false;" + ) + + # The catch's else branch (non-abort, non-stop) skips the funnel + # when the flag is set — the funnel call itself stays intact (the + # real-network-error path, no pagehide). + stop_branch = js.index('} else if (stoppedByUser || err?.name === "AbortError") {') + else_branch = js.index("} else {", stop_branch) + finally_branch = js.index("} finally {", else_branch) + else_body = js[else_branch:finally_branch] + idx_guard = else_body.find("if (!leftThePage) {") + idx_funnel = else_body.find("finalizeFailedTurn(detail, {") + assert 0 <= idx_guard < idx_funnel, ( + "the failed funnel must be skipped after a real page departure " + "(the teardown rejection is not a failed turn)" + ) + + +def test_stream_drop_guard_routes_through_the_funnel() -> None: + """The stream-drop guard (frames arrived, no ``done`` — the + connection died mid-turn) routes through the SAME funnel: the + half-answer persists as failed (its text + the error note + a + working Retry) BEFORE the error state.""" + js = _js() + guard = js.index("if (!sawDone && !aborted && (acc || thinkingAcc)) {") + zero_frame = js.index("if (!aborted && !wrap) {", guard) + guard_body = js[guard:zero_frame] + assert "finalizeFailedTurn(detail, {" in guard_body + assert guard_body.index("finalizeFailedTurn(detail, {") < guard_body.index( + "setUiState(UI_STATE.error, detail);" + ) + + +def test_zero_frame_fallback_persists_failed_marker() -> None: + """The zero-frame-but-COMPLETED fallback bubble (the stream settled + with no events) is a failed turn too (task 01 ASSUMPTION): the + bubble text stays ``EMPTY_ANSWER_FALLBACK`` (a meaningful record + text) but the record gains ``failed: true`` + the error note, and + the bubble ends retryable.""" + js = _js() + zero_frame = js.index("if (!aborted && !wrap) {") + catch = js.index("} catch (err) {", zero_frame) + block = js[zero_frame:catch] + assert "const fallback = EMPTY_ANSWER_FALLBACK;" in block, ( + "the bubble text stays the EMPTY_ANSWER_FALLBACK answer text" + ) + assert "appendFailedNote(fwrap, nothing);" in block + assert "failed: true," in block + assert "error: nothing," in block + assert "markLastRetryable();" in block + + +def test_only_the_three_failed_paths_persist_failed() -> None: + """No call site OUTSIDE the three failed paths persists + ``failed: true`` (task 01 completion criterion, grep-level): + exactly three CODE sites — two in ``finalizeFailedTurn`` (the + partial + zero-frame shapes) and one in the zero-frame-but- + completed fallback — the done, stop, and restore paths never set + the marker (they read it, or don't touch it).""" + js = _js() + fn = _fn_body(js, "finalizeFailedTurn") + zero_frame = js.index("if (!aborted && !wrap) {") + catch = js.index("} catch (err) {", zero_frame) + fallback_block = js[zero_frame:catch] + fn_start = js.index("function finalizeFailedTurn(") + fn_src = _fn_source(js, "finalizeFailedTurn") + code_outside = js[:fn_start] + js[fn_start + len(fn_src) :] + code_outside = code_outside.replace(fallback_block, "") + # Comments may mention the marker; code must not (the file's + # block-comment lines start with * or /* after stripping). + code_lines = [ + line + for line in code_outside.splitlines() + if not line.strip().startswith(("//", "*", "/*")) + ] + assert "failed: true" not in "\n".join(code_lines), ( + "only the three failed paths may persist failed: true" + ) + assert fn.count("failed: true") == 2 + assert fallback_block.count("failed: true") == 1 + + +# --------------------------------------------------------------------------- +# restore — a failed record renders as an error bubble with the note +# --------------------------------------------------------------------------- + + +def test_restore_renders_the_failed_note() -> None: + """The restore branch re-renders the in-bubble error note from the + persisted ``error`` detail — and only when it is present (a record + whose ``error`` is null has the detail in its ``text`` already).""" + js = _js() + body = _fn_body(js, "renderStoredMessage") + assert "if (m.failed && m.error) appendFailedNote(wrap, m.error);" in body, ( + "the failed note restores from the persisted error detail" + ) + + +def test_restore_excludes_save_as_doc_and_tune_for_failed() -> None: + """A failed turn is a NOTE, not an answer: the restore excludes + both the Save-as-doc button (the ``m.stopped`` exclusion extended + with ``!m.failed``) and the Tune button (``!m.failed``). Stopped + and successful records keep their buttons — the pre-phase-120 + behavior for them is byte-identical.""" + js = _js() + body = _fn_body(js, "renderStoredMessage") + assert "if (!m.stopped && !m.failed) appendSaveAsDocButton(wrap, m.text);" in body + assert "if (!m.failed) appendTuneButton(wrap);" in body + + +# --------------------------------------------------------------------------- +# NOT touched — the phase's explicit contract (byte-pinned sources) +# --------------------------------------------------------------------------- + +#: The FULL source of ``showErrorBanner`` as of phase 120 — the +#: phase-111 button + the phase-114 hint, byte-unchanged by this phase +#: (the phase makes ``lastBrainWrap`` EXIST on the error paths instead +#: of changing the condition). A diff here is a contract violation. +PINNED_SHOW_ERROR_BANNER = """function showErrorBanner(detail, opts = {}) { + banner.hidden = false; + banner.classList.add("is-error"); + banner.setAttribute("role", "alert"); + // Phase 114 (TODO L6): a frame-carried hint (the "question too long" + // case — reachability is fine, only the length is the problem) replaces + // the default reachability hint when present. + bannerText.textContent = detail + ? `${detail} ${opts.hint ?? ERROR_HINT}` + : (opts.hint ?? ERROR_HINT); + // Phase 111 (task 01): reveal the banner Retry button only for failed + // chat turns (opts.retryable) AND when a retryable bubble exists. + if (opts.retryable) { + const btn = document.querySelector("#banner-retry"); + if (btn && lastBrainWrap) { + btn.hidden = false; + // Bind click once per reveal — the old listener is removed after + // the first click, so re-binding on every reveal is safe. + btn.addEventListener("click", () => retryLastTurn(lastBrainWrap)); + } + } +}""" + + +def test_show_error_banner_is_byte_unchanged() -> None: + """``showErrorBanner`` is byte-unchanged by phase 120 (the + "NOT touched" contract): its full source must match the pin — + the ``opts.retryable && lastBrainWrap`` condition, the phase-114 + hint merge, and the once-per-reveal binding included.""" + assert _fn_source(_js(), "showErrorBanner") == PINNED_SHOW_ERROR_BANNER + + +#: The FULL source of ``retryLastTurn`` as of phase 120 — the phase-49 +#: redo-in-place (pop the last brain record, re-ask the preceding +#: question). It works on a failed record UNCHANGED (locked A1): the +#: question's user record immediately precedes the failed record. +PINNED_RETRY_LAST_TURN = """function retryLastTurn(wrap) { + if (uiState === UI_STATE.thinking || uiState === UI_STATE.streaming) return; + if (wrap !== lastBrainWrap) return; // stale click — the button moved on + let lastIdx = -1; + for (let i = conversation.length - 1; i >= 0; i -= 1) { + if (conversation[i].who === "brain") { + lastIdx = i; + break; + } + } + if (lastIdx === -1) return; + // Invariant: every brain record follows its user record — the + // question to re-ask is the record immediately before the popped one. + const prev = conversation[lastIdx - 1]; + if (!prev || prev.who !== "user") return; + const text = prev.text; + conversation.splice(lastIdx, 1); // redo in place: the old answer is gone + // Save BEFORE the rerun: what the user saw — the removed answer — is + // what is stored from this point on (the question stays, the replaced + // answer never comes back). + saveConversation(); + wrap.remove(); + lastBrainWrap = null; + // Re-ask without re-adding: the reask turn skips the user append and + // persistence save point 1 (the question is already in both). + // Phase 53: the promise is returned (the Regenerate await above); + // runTurn never rejects — a failure surfaces as the error banner. + return runTurn(text, { reask: true }); +}""" + + +def test_retry_last_turn_is_byte_unchanged() -> None: + """``retryLastTurn`` is byte-unchanged by phase 120 (locked A1 — + the redo-in-place is REUSED, not extended): the full source must + match the pin — the pop-the-last-brain-record + re-ask logic, the + in-flight guard, and the stale-click guard included.""" + assert _fn_source(_js(), "retryLastTurn") == PINNED_RETRY_LAST_TURN + + +# --------------------------------------------------------------------------- +# CSS — the in-bubble error line (the .stopped-note family, error color) +# --------------------------------------------------------------------------- + + +def test_failed_note_css_uses_the_error_token() -> None: + """``.failed-note`` exists in styles.css, colored by the theme's + error TOKEN (``--err-ink`` — the monochrome theme grays it + automatically; the contrast floor is the stopped-note family's) — + never a literal color (the phase-92 zero-literal convention).""" + css = _css() + m = re.search(r"\.failed-note \{([\s\S]*?)\n\}", css) + assert m, "styles.css must define .failed-note" + body = m.group(1) + assert "color: var(--err-ink);" in body, ( + ".failed-note must use the theme's error token" + ) + assert "pointer-events: none;" in body, "the note is non-interactive (the stopped-note way)" + + +def test_failed_detail_wraps_long_details() -> None: + """The detail span WRAPS (a 500-char error detail must not blow + out the 46rem chat column — the stopped note's nowrap fits a + one-word label, not a detail).""" + css = _css() + m = re.search(r"\.failed-detail \{([\s\S]*?)\n\}", css) + assert m, "styles.css must define .failed-detail" + assert "overflow-wrap: anywhere;" in m.group(1) diff --git a/tests/unit/test_save_as_doc_button.py b/tests/unit/test_save_as_doc_button.py index 9d7eb23..7895a56 100644 --- a/tests/unit/test_save_as_doc_button.py +++ b/tests/unit/test_save_as_doc_button.py @@ -172,9 +172,10 @@ def test_app_js_call_sites_pass_the_raw_markdown() -> None: the rendered HTML): the live `done` branch (exactly the string rememberBrainTurn stores, so a reload offers the identical draft), the empty-answer fallback bubble (parity with the done path), and - the restore path (m.text). A stopped partial is a note, not an - answer — the restore gates on !m.stopped; the live stop path and - the pagehide partial never call the helper at all.""" + the restore path (m.text). A stopped partial — or a failed turn + (phase 120: a note, not an answer either) — is excluded: the + restore gates on !m.stopped && !m.failed; the live stop/failure + paths and the pagehide partial never call the helper at all.""" js = _text(APP_JS) assert 'appendSaveAsDocButton(wrap, finalText || acc || "…");' in js, ( "the live done branch must pass the raw persisted text" @@ -182,8 +183,10 @@ def test_app_js_call_sites_pass_the_raw_markdown() -> None: assert "appendSaveAsDocButton(fwrap, fallback);" in js, ( "the empty-answer fallback bubble must get the button too" ) - assert "if (!m.stopped) appendSaveAsDocButton(wrap, m.text);" in js, ( - "the restore path must pass m.text and skip stopped records" + assert ( + "if (!m.stopped && !m.failed) appendSaveAsDocButton(wrap, m.text);" in js + ), ( + "the restore path must pass m.text and skip stopped + failed records" ) # The live call sits next to the Tune button (same meta row scope). tune_idx = js.find("appendTuneButton(wrap); // every completed brain bubble is tunable") diff --git a/tests/unit/test_schemas.py b/tests/unit/test_schemas.py index af5a31b..ef0b170 100644 --- a/tests/unit/test_schemas.py +++ b/tests/unit/test_schemas.py @@ -339,6 +339,8 @@ def test_minimal_message_still_validates() -> None: msg.thinking, msg.tools, msg.stopped, + msg.failed, + msg.error, ) == ( None, None, @@ -347,6 +349,8 @@ def test_minimal_message_still_validates() -> None: None, None, None, + None, + None, ) @@ -373,6 +377,8 @@ def test_realistic_bor_chat_v1_payload_round_trips() -> None: "thinking": None, "tools": None, "stopped": None, + "failed": None, + "error": None, }, { "who": "brain", @@ -411,6 +417,8 @@ def test_realistic_bor_chat_v1_payload_round_trips() -> None: }, ], "stopped": None, + "failed": None, + "error": None, }, { "who": "user", @@ -422,6 +430,8 @@ def test_realistic_bor_chat_v1_payload_round_trips() -> None: "thinking": None, "tools": None, "stopped": None, + "failed": None, + "error": None, }, { "who": "brain", @@ -433,6 +443,8 @@ def test_realistic_bor_chat_v1_payload_round_trips() -> None: "thinking": None, "tools": None, "stopped": True, # the owner stopped the generation mid-answer + "failed": None, + "error": None, }, ] @@ -459,6 +471,8 @@ def test_realistic_payload_round_trips_through_update_model() -> None: "thinking": "scratchpad", "tools": [_tool_call()], "stopped": None, + "failed": None, + "error": None, } payload = SavedChatUpdate.model_validate({"messages": [msg]}) assert payload.model_dump()["messages"] == [msg]