Compare commits
2
Commits
a5b63f83ad
...
3a0fc3db05
| Author | SHA1 | Date | |
|---|---|---|---|
 ducoterra
|
3a0fc3db05 | ||
|
ducoterra
|
0ff1f8c4d6 |
@@ -0,0 +1,51 @@
|
||||
# Phase 120 — 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, forcing the user to click 'new chat' or be stuck."
|
||||
**Story:** n/a (bug-fix follow-up; extends the phase-49/53 redo-in-place retry, phase-67 LLM retry, and phase-111 banner Retry assets).
|
||||
**Context:** `frontend/assets/app.js` — `showErrorBanner(detail, opts)` (L2210) reveals `#banner-retry` only when `opts.retryable && lastBrainWrap` (L2222); `retryLastTurn(wrap)` (L2276) pops the LAST brain record and re-asks the user question immediately before it (the invariant `every brain record follows its user record`); `rememberBrainTurn(rawText, meta, replaceIndex)` (L2108) pushes/replaces the brain record in `conversation` + `saveConversation()` + `persistConversation()` (the phase-55 auto-save rides the same call). `lastBrainWrap` is assigned only on three paths: the `done` settle (L2609), the zero-frame fallback bubble (L2671), and the user-stop finalize (L2699) — **never on a turn error**. The error catch (the `else` branch at ~L2690) calls `setUiState(UI_STATE.error, detail, { hint })` with NO brain record persisted, whether or not a partial `wrap` exists. The stream-drop guard (~L2651, `!sawDone && !aborted && (acc || thinkingAcc)`) also lands in the error state with nothing persisted. Restore: `renderStoredMessage(m)` (L1642) renders `m.stopped` via `appendStoppedNote` (L486); the restore loop sets `lastBrainWrap` on the last restored brain bubble (L1694) and calls `markLastRetryable()` (L1708, L560 — removes all `.retry-btn`, re-adds on the LAST `.brain-wrap`). `app/schemas.py` — `ChatMessage` (L742) is `extra="forbid"` with fields `who`, `text` (≤32 000), `sources?`, `related?`, `deflected?`, `suggestions?`, `thinking?` (≤32 000), `tools?`, `stopped?: bool | None` (L786); `SavedChatCreate/Update` messages are non-empty, ≤200 (phase 83). `tests/e2e/mock_llm.py` + `tests/e2e/test_llm_retry.py` hold the existing LLM-failure mock pattern for the E2E.
|
||||
|
||||
## Objective
|
||||
A failed chat turn — network error (zero frames), SSE `error` frame, or mid-stream drop — leaves a **retryable error state** both live (the banner Retry and an in-bubble Retry both work) and after a page refresh (the failed turn restores as an error bubble with a working Retry button). No failed turn strands the user with a bare question and no recovery.
|
||||
|
||||
## Dependencies
|
||||
- `119_name_signal_read_chips` (complete) — pipeline predecessor (execution order) only.
|
||||
- Code dependencies (all complete): phase 49/53 `retryLastTurn` redo-in-place, phase 111 `#banner-retry`, phase 48 `stopped` persistence + `appendStoppedNote` pattern, phase 55 auto-save riding `rememberBrainTurn`.
|
||||
|
||||
## Design (shared by all tasks — the executor reads this, not the chat)
|
||||
- **Failed record (task 01, server side):** two new OPTIONAL fields on `ChatMessage`, the phase-48 `stopped` precedent (L786): `failed: bool | None = None` and `error: str | None = Field(default=None, max_length=500)` (the persisted error detail; 500 caps a hostile detail string in the phase-83 style). `extra="forbid"` stays — the keys are now declared, unknown keys still 422. No change in `app/api/chats.py` logic (the schema flows through `SavedChatCreate/Update`); the shared-chat shape (`SharedChatOut.messages`) carries failed records verbatim (text renders as-is on the shared page — no change needed there).
|
||||
- **Failed turn = a brain record (LOCKED A1):** a failed turn persists `{ who: "brain", text: <detail or fallback>, failed: true, error: <detail> }` via `rememberBrainTurn` — so it lands in localStorage AND the server-side saved chat through the existing phase-55 auto-save ride. There is no separate error table and no new API: `retryLastTurn`'s pop-the-last-brain-record-then-re-ask-the-preceding-question logic works on a failed record UNCHANGED (the invariant holds — the question's user record immediately precedes it).
|
||||
- **Live error paths (task 01, frontend):** the error catch's `else` branch (non-abort, non-stop) and the stream-drop guard BOTH funnel into one new helper `finalizeFailedTurn(detail, { acc, thinking, tools })`:
|
||||
- **Partial exists** (`wrap` with streamed text): close the thinking block + tool calls (the stop-finalize pattern), `appendFailedNote(wrap, detail)` (new, mirrors `appendStoppedNote` L486 — an in-bubble error line with the detail), persist via `rememberBrainTurn(acc, { thinking, tools, failed: true, error: detail }, leavePartialIndex)`, `lastBrainWrap = wrap`.
|
||||
- **No wrap** (network error, zero frames): create a brain bubble with a fixed fallback text (a short honest "my answer didn't make it" line — NOT the `EMPTY_ANSWER_FALLBACK` answer text; the `appendFailedNote` carries the real detail), persist the same record shape, `lastBrainWrap = fwrap`.
|
||||
- Then `markLastRetryable()` — the in-bubble Retry button appears, and `showErrorBanner`'s existing `opts.retryable && lastBrainWrap` condition (L2222) now holds on a turn error, so the phase-111 banner Retry appears too — **no change to `showErrorBanner`** (it binds `() => retryLastTurn(lastBrainWrap)` at reveal; `lastBrainWrap` is set before `setUiState(UI_STATE.error, …)` runs).
|
||||
- The zero-frame-but-stream-completed case keeps its existing fallback bubble (L2663–2673) — now ALSO marked `failed: true` + error note (it is a failed turn; the bubble text stays `EMPTY_ANSWER_FALLBACK` so the record keeps a meaningful `text`).
|
||||
- **Restore (task 02):** `renderStoredMessage(m)` gains the failed branch — a `m.failed` record renders as a brain bubble (the persisted `text`), gets `appendFailedNote(wrap, m.error)`, and gets NO Save-as-doc button (a note, not an answer — the `m.stopped` exclusion at L1687 precedent: `if (!m.stopped && !m.failed) appendSaveAsDocButton(…)`). No other restore change is required: the restore loop's `lastBrainWrap = wrap` (L1694) + `markLastRetryable()` (L1708) already target the last `.brain-wrap`, which is now the failed bubble → the in-bubble Retry button renders on refresh. `retryLastTurn` needs no change (the failed record is the last brain record; its preceding user record is the question).
|
||||
- **Interaction with `stopped`:** a turn is either stopped (user engaged, partial kept, `stopped: true`) or failed (`failed: true`) — mutually exclusive by construction (the stop path is the catch's `stoppedByUser`/`AbortError` branch, which this phase does not touch).
|
||||
- **NOT touched:** `retryLastTurn` itself, the stop path, the done path, the server save/restore API logic (schema fields only), the shared page rendering, and every non-chat `showErrorBanner` caller.
|
||||
|
||||
## Tasks
|
||||
1. `01_persist_failed_turn.md` — `ChatMessage.failed`/`error` fields + the live error paths persist a failed brain record with a rendered error bubble (banner Retry works on network errors).
|
||||
2. `02_restore_failed_turn.md` — restore renders a `failed` record as an error bubble with a working Retry button (the refresh case).
|
||||
3. `03_failed_turn_tests.md` — unit + integration + isolated E2E `test_failed_turn_retry.py`.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: `tests/unit/test_chat_message_failed.py` (new, task 03) — `ChatMessage` accepts `failed`/`error`, `error` >500 chars 422s, unknown keys still 422, omitted keys round-trip `None`; `tests/unit/test_frontend_failed_turn.py` (new, task 03) — house-style source assertions: the error catch + stream-drop guard route through the failed-turn finalize (persist `failed: true`, call `markLastRetryable`), `appendFailedNote` exists and mirrors the stopped-note structure, the restore branch renders the note and excludes Save-as-doc, `showErrorBanner` is byte-unchanged (the `lastBrainWrap` condition untouched).
|
||||
- Integration: `tests/integration/test_chats_api.py` (extend) — `POST`/`PUT /api/chats` with a `failed: true` + `error` record round-trips byte-identically (the phase-50 contract); a shared chat carrying a failed record still serves (public shape unchanged).
|
||||
- E2E: `tests/e2e/test_failed_turn_retry.py` (new, task 03) — run in isolation per AGENTS.md §4. Scenarios (the `mock_llm.py` failure pattern from `test_llm_retry.py`): (A) network-class failure (zero frames) → banner with a visible Retry → click re-asks without re-typing; (B) SSE `error` frame after partial deltas → partial bubble keeps its text + error note + Retry → click re-asks; (C) reload the page after a failed turn → the failed bubble restores with a working Retry button → click re-asks.
|
||||
- Coverage: **>90%** on `app/` (validate.sh gate).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] A network-error turn shows a Retry (banner and/or in-bubble); clicking it re-asks the last question without re-typing.
|
||||
- [ ] Reloading the page after a failed turn shows the failed bubble (with the error detail) and a working Retry — no "new chat" required.
|
||||
- [ ] Stopped turns (phase 48) and successful turns behave byte-identically to before.
|
||||
- [ ] `uv run pytest` green; `uv run pytest --cov=app --cov-report=term-missing` TOTAL >90%; `uv run ruff check . && uv run pyright` clean.
|
||||
- [ ] One `--no-gpg-sign` commit; phase dir moved to `.agents/phases/complete/` by the pipeline gate.
|
||||
|
||||
## Locked decisions
|
||||
- **A1 — a failed turn persists as a brain record with a `failed` marker (+ capped `error` detail), the phase-48 `stopped` precedent; no separate error table, no new API, `retryLastTurn` reused unchanged (owner-confirmed 2026-09-24, roadmap confirmation).**
|
||||
- **Banner Retry stays as-is** — the phase-111 `opts.retryable && lastBrainWrap` condition is kept; this phase makes `lastBrainWrap` exist on the error paths so the existing button finally appears (owner-confirmed: same mechanism, no `showErrorBanner` change).
|
||||
|
||||
## Commit
|
||||
```bash
|
||||
git add app/ frontend/ tests/ .agents/phases/ && git commit --no-gpg-sign -m "fix(chat): persist failed turns so retry works on network errors and survives a refresh"
|
||||
```
|
||||
@@ -0,0 +1,37 @@
|
||||
# Task 01 — Persist the failed turn: schema fields + live error paths
|
||||
|
||||
**Phase:** `120_failed_turn_retry` · **Source:** `TODO.md:3–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, forcing the user to click 'new chat' or be stuck."
|
||||
|
||||
## Objective
|
||||
Every failed chat turn (network error, SSE `error` frame, stream drop) persists a `failed: true` brain record and renders a retryable error bubble live — so the phase-111 banner Retry finally appears on network errors (its `lastBrainWrap` precondition now holds).
|
||||
|
||||
## Work
|
||||
1. `app/schemas.py` — `ChatMessage` (L742, `extra="forbid"`): add, next to `stopped` (L786),
|
||||
```python
|
||||
failed: bool | None = None
|
||||
error: str | None = Field(default=None, max_length=500)
|
||||
```
|
||||
Extend the docstring: the phase-48 `stopped` precedent — a FAILED turn (network/SSE error/stream drop) stores `{who: "brain", text: <detail or fallback>, failed: true, error: <detail>}`; `error` is the persisted banner detail, capped at 500 (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).
|
||||
2. `frontend/assets/app.js`:
|
||||
- New `appendFailedNote(wrap, detail)` — mirror of `appendStoppedNote` (L486): one `.failed-note` per bubble (guard query), a visible error line inside the brain bubble carrying `detail` (the banner keeps its role="alert" summary; the note is the in-bubble, refresh-surviving copy).
|
||||
- New `finalizeFailedTurn(detail, { acc, thinking, tools, wrap, leavePartialIndex })` — the single funnel for every non-stop, non-abort turn failure:
|
||||
- `wrap` exists (partial streamed): `closeThinkingBlock(wrap)` + `closeToolCalls(wrap)` (the stop-finalize pattern, ~L2685), `appendFailedNote(wrap, detail)`, `rememberBrainTurn(acc, { thinking: thinking || undefined, tools: tools.length ? tools : undefined, failed: true, error: detail }, leavePartialIndex)`, `lastBrainWrap = wrap`.
|
||||
- no `wrap` (network error, zero frames): `fwrap = addMessage("brain", FAILED_TURN_TEXT)` where `FAILED_TURN_TEXT` is a new short honest constant ("My answer didn't make it — the connection dropped. Use Retry to ask again.") — NOT `EMPTY_ANSWER_FALLBACK` (that constant stays for the zero-frame-but-completed case); `appendFailedNote(fwrap, detail)`, `rememberBrainTurn(FAILED_TURN_TEXT, { failed: true, error: detail }, leavePartialIndex)`, `lastBrainWrap = fwrap`.
|
||||
- end with `markLastRetryable()`.
|
||||
- `detail` is the trimmed error string, truncated to 500 chars before persistence (the schema cap is the backstop).
|
||||
- The error catch's `else` branch (~L2690, currently `setUiState(UI_STATE.error, detail, { hint })` with no persistence): call `finalizeFailedTurn(detail, {…})` BEFORE `setUiState(UI_STATE.error, detail, err.hint ? { hint: err.hint } : {})` (the banner stays — now with its Retry revealed because `lastBrainWrap` is set).
|
||||
- The stream-drop guard (~L2651, `!sawDone && !aborted && (acc || thinkingAcc)` → currently a bare `setUiState(UI_STATE.error, "The stream ended before my answer finished — try again?")`): route through the same `finalizeFailedTurn` with that detail (the partial persists as failed — a half-answer is a failed answer, and Refresh must restore what the user saw + a Retry).
|
||||
- The zero-frame-but-completed fallback bubble (L2663–2673): add `failed: true` + `error: "The model answered with nothing."` to its `rememberBrainTurn` call and `appendFailedNote(fwrap, …)` — the bubble text stays `EMPTY_ANSWER_FALLBACK`.
|
||||
- Do NOT touch: `showErrorBanner` (L2210), `retryLastTurn` (L2276), the stop branch, the done settle (L2609), `markLastRetryable` (L560).
|
||||
3. `frontend/assets/styles.css` — `.failed-note`: the in-bubble error line treatment (the `.stopped-note` family, error-colored per the current theme's error token — contrast ≥4.5:1, PLAN §7).
|
||||
4. ASSUMPTION: the zero-frame-but-completed case (stream returns, no events, no throw) is also marked failed — it is a failed turn, and its record previously persisted with no marker (inconsistent with the refresh case this phase fixes).
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: `tests/unit/test_chat_message_failed.py` (shipped with task 03's test task — this task ships the code): `ChatMessage` accepts `failed: true` + `error`; `error` >500 chars → 422; an unknown key still → 422; a record without the new keys is byte-identical to before. `tests/unit/test_frontend_failed_turn.py` (task 03): the catch `else` branch + stream-drop guard + zero-frame fallback all route through the failed finalize (persist `failed: true`, call `markLastRetryable`); `appendFailedNote` exists; `showErrorBanner` and `retryLastTurn` sources are untouched (byte-pinned).
|
||||
- Coverage: **>90%** on `app/` for the schema change (the frontend JS is pinned by source-assertion unit tests).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `ChatMessage` round-trips `failed`/`error` (unit tests green).
|
||||
- [ ] A zero-frame network error (E2E scenario A, task 03) shows the banner WITH a visible Retry button and a failed bubble with the error detail.
|
||||
- [ ] No call site outside the three failed paths persists `failed: true` (grep).
|
||||
- [ ] `uv run pytest` green; `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -0,0 +1,25 @@
|
||||
# Task 02 — Restore the failed turn: refresh keeps the Retry
|
||||
|
||||
**Phase:** `120_failed_turn_retry` · **Source:** `TODO.md:4` — "Refreshing the page after an error shows only the chat message you sent and no options to retry the message, forcing the user to click 'new chat' or be stuck."
|
||||
|
||||
## Objective
|
||||
A `failed` record restores as an error bubble (persisted text + error note) that carries a working Retry button — the refreshed page is the same retryable state the live error was, and Retry re-asks the question through the unchanged `retryLastTurn`.
|
||||
|
||||
## Work
|
||||
1. `frontend/assets/app.js` — `renderStoredMessage(m)` (L1642): add the failed branch, mirroring the `m.stopped` handling (L1685–1688):
|
||||
- a `m.failed` brain record renders its `text` (the persisted detail or fallback line) as the bubble content, then `appendFailedNote(wrap, m.error)` (the note is absent when `m.error` is null — the record's `text` already carries it), and
|
||||
- `if (!m.stopped && !m.failed) appendSaveAsDocButton(wrap, m.text);` — a failed turn is a note, not an answer (the stopped exclusion precedent at L1687).
|
||||
- No Tune button for failed records either (a note, not an answer — same scope as the `m.stopped` exclusion around L584–588 if the restore call site applies it there).
|
||||
- No other restore change: the restore loop already sets `lastBrainWrap = wrap` on the last restored brain bubble (L1694) and calls `markLastRetryable()` (L1708), which removes every `.retry-btn` and re-adds it on the LAST `.brain-wrap` — the failed bubble. `retryLastTurn` works unchanged: the failed record is the last brain record, the user record immediately before it is the question (the invariant holds by construction — task 01 persists the brain record right after the user record), so the redo-in-place pops the failed record and re-asks.
|
||||
2. `frontend/assets/app.js` — the shared-chat restore (`frontend/assets/shared.js` / the shared page): failed records render their `text` as a plain brain bubble (no note, no Retry — the shared view is read-only and text-only by design; no change beyond confirming the `renderStoredMessage`-equivalent there does not choke on the unknown-looking `failed`/`error` keys — it renders `text` only).
|
||||
3. ASSUMPTION: a failed bubble restored at the END of the conversation gets the Retry; a failed bubble in the MIDDLE of a longer conversation does not (the phase-49 last-bubble-only rule, unchanged).
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: `tests/unit/test_frontend_failed_turn.py` (task 03): the restore branch renders the failed note, excludes Save-as-doc (and Tune, where applicable), and the restore path is the only place `m.failed` is read for rendering; the shared page renders failed records text-only.
|
||||
- E2E: scenario C of `tests/e2e/test_failed_turn_retry.py` (task 03) pins this task end-to-end.
|
||||
- Coverage: n/a (frontend) — the validate.sh `app/` gate must stay green.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] Reload after a failed turn (E2E scenario C): the failed bubble shows with its error detail and a Retry button; clicking Retry re-asks the preceding question without re-typing and the failed record is replaced by the new answer.
|
||||
- [ ] A mid-conversation failed record restores with NO Retry button (last-bubble-only rule intact).
|
||||
- [ ] `uv run pytest` green; `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -0,0 +1,34 @@
|
||||
# Task 03 — Failed-turn tests: unit + integration + isolated E2E
|
||||
|
||||
**Phase:** `120_failed_turn_retry` · **Source:** `TODO.md:3–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 …"
|
||||
|
||||
## Objective
|
||||
Pin the whole failed-turn contract: the schema boundary, the save/restore round-trip, the three live failure paths, and the two user scenarios (retry on network error; retry after refresh) as an isolated Playwright suite.
|
||||
|
||||
## Work
|
||||
1. `tests/unit/test_chat_message_failed.py` (new):
|
||||
- `ChatMessage` accepts `{who: "brain", text: "…", failed: true, error: "detail"}`; `error` of 501 chars → 422; an unknown key (e.g. `"foo": 1`) → 422 (`extra="forbid"` intact); a record WITHOUT the new keys serializes byte-identically to a pre-phase record (the phase-50 contract).
|
||||
2. `tests/unit/test_frontend_failed_turn.py` (new) — house-style source assertions (the phase-111 `test_frontend_banner_retry.py` pattern):
|
||||
- the error catch `else` branch, the stream-drop guard, and the zero-frame fallback bubble all persist `failed: true` (grep the three sites for the `failed: true` persist) and each funnel path ends with `markLastRetryable`;
|
||||
- `appendFailedNote` exists and guards against duplicates (one `.failed-note` per bubble);
|
||||
- `FAILED_TURN_TEXT` is a distinct constant (not `EMPTY_ANSWER_FALLBACK`);
|
||||
- `showErrorBanner` and `retryLastTurn` are byte-unchanged (pin their source — the phase's explicit "NOT touched" contract);
|
||||
- the restore branch renders `m.failed` (note + Save-as-doc exclusion).
|
||||
3. `tests/integration/test_chats_api.py` (extend):
|
||||
- `POST /api/chats` with a brain record `{text, failed: true, error: "…"}` returns it byte-identically; `PUT` re-Save round-trips it; `error` >500 chars → 422;
|
||||
- a shared chat (the phase-51 `share` path) carrying a failed record still serves `GET /api/shared/{token}` (public shape — `title` + `messages` — unchanged).
|
||||
4. `tests/e2e/test_failed_turn_retry.py` (new — ONE file, run in isolation per AGENTS.md §4: `uv run pytest tests/e2e/test_failed_turn_retry.py -v --no-cov`), reusing `tests/e2e/mock_llm.py`'s failure pattern from `tests/e2e/test_llm_retry.py`:
|
||||
- **A — network error:** mock the chat endpoint to fail with ZERO frames (connection reset / immediate close — the same failure `test_llm_retry.py` exercises past its retry budget, or a hard 500 if the mock supports it) → assert: the banner is visible with a working Retry button AND a failed bubble with the error detail exists → click banner Retry → the question is re-asked (mock now succeeds) → a grounded answer renders and the failed bubble is gone.
|
||||
- **B — SSE error frame with partial:** mock streams some `delta` frames then an `error` frame → assert: the partial bubble KEEPS its streamed text + shows the error note + carries the in-bubble Retry → click it → re-asked in place (redo-in-place: the failed record is replaced by the new answer).
|
||||
- **C — refresh:** fail a turn (as in A) → `page.reload()` → assert: the question + the failed bubble restore (error detail visible) + the Retry button is present on the failed bubble → click it → re-asked → answer renders.
|
||||
- Negative: a STOPPED turn (user Stop) still restores with the "Answer stopped." note and NOT a failed note (phase 48 unchanged).
|
||||
5. Run the full gate: `uv run pytest` (unit + integration), `uv run pytest --cov=app --cov-report=term-missing` (TOTAL >90%), the isolated E2E file, `uv run ruff check . && uv run pyright`.
|
||||
|
||||
## Testing & Quality
|
||||
- This task IS the phase's test suite (see Work).
|
||||
- Coverage: **>90%** on `app/` — the only `app/` code in this phase is the `ChatMessage` schema (100% by the unit cases).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] All four test artifacts exist and pass; the isolated E2E file passes standalone.
|
||||
- [ ] `uv run pytest --cov=app` TOTAL >90%; lint + types clean.
|
||||
- [ ] No test asserts the old (broken) behavior — grep for any assertion that a network error shows NO Retry (must not exist).
|
||||
@@ -0,0 +1,48 @@
|
||||
# Phase 121 — Private git sources: a token that never reaches the UI or the API
|
||||
|
||||
**Source:** `TODO.md` L5 — "Need a way to add private repos without exposing the token in the UI (like when adding an https repo `https://myuser:ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx@github.com/myuser/my-private-repo.git`)"
|
||||
**Story:** n/a (feature request; extends the phase-28/35/38 git/local sources and phase-89 per-source settings assets).
|
||||
**Context:** `app/models.py:245` — `GitSource`: `url: Text UNIQUE NOT NULL`, `kind` ("git"/"local"), `path`, `ignore_paths` (JSONB), `include_hidden`, `added_at` — no token column. `app/schemas.py` — `GitSourceIn` (L561: `kind`, `url` min 1/max 500, `path`, `ignore_paths`, `include_hidden`; `_trim_url` before-validator L594), `GitSourceOut` (L605: `url: str`), `GitSourceRow` (L626: `url`), `GitSourcePatchIn` (L651). `app/api/git_sources.py` — `URL_RE = ^(?:https?://|ssh://|git@)` (L195; a prefix match, so `user:token@` URLs pass); POST validates the prefix (L344–345) and duplicates via `GitSource.url == url` (L349); GET list / GET single return `row.url` RAW (L235, L250, L297, L423) — an embedded token is echoed to any browser (the leak); the local-source upload endpoint (L432). `app/api/sync.py:296` — `clone_or_pull(row.url, sources_root / repo_name(row.url))`; `scripts/git_sync.py` — `clone_or_pull`, `repo_name`; `scripts/import_docs.py::_resolve_sources` — the CLI's second clone caller (both consume `app.rag.git_sources.effective_sources`, L27). `frontend/assets/git-sources.js` — add form `#git-source-url` (L241; submit `body: (url) => ({ url })` L945); every display site renders `s.url` raw (list cell L395–406 incl. the `title` attr, delete row L533, edit modal L697, ignore-list context L763/L830). `alembic/` — migrations.
|
||||
|
||||
## Objective
|
||||
Private repos are added with a bare URL plus an optional MASKED token field. The token lives in a dedicated DB column, is injected only into the clone URL at sync/clone time, and is absent from every API response and UI surface — including legacy rows that already embed the token in `url` (those are sanitized on output but keep working).
|
||||
|
||||
## Dependencies
|
||||
- `120_failed_turn_retry` (todo) — pipeline predecessor (execution order) only; no code dependency.
|
||||
- Code dependencies (all complete): phase 35/38 `GitSource` kinds + `effective_sources`, phase 89 per-source settings (the PATCH-field precedent), phase 28 `clone_or_pull`/`repo_name`.
|
||||
|
||||
## Design (shared by all tasks — the executor reads this, not the chat)
|
||||
- **Storage (task 01, LOCKED A2):** `GitSource.token` — `Text NULL` (NULL = public/no credential; **plaintext by necessity** — the repo must remain cloneable, so the raw credential must be recoverable; the Postgres DB is the trusted store and is never served to the UI; there is deliberately no external secrets backend). `GitSourceIn.token: str | None = None` (max 500, trimmed); `GitSourcePatchIn.token: str | None = None` — PATCH semantics: **absent/None = no change, non-empty = replace, empty string = clear** (the UI offers replace; clear exists for API completeness). `GitSourceOut`/`GitSourceRow` gain NO token field — no response shape ever carries it (LOCKED A2).
|
||||
- **Normalization (task 02):** on POST (and PATCH when a new url/token arrives), the server normalizes: if the incoming URL contains userinfo (`user:pass@host`, only for `https?://` URLs — ssh/`git@` carry no userinfo), the **userinfo is stripped** for storage and the embedded credential is moved into `token` — UNLESS the caller also sent an explicit `token` field, which WINS (explicit beats embedded). Pasting the old-style `https://user:ghp_…@github.com/x/y.git` URL still works and ends up token-column-clean. The duplicate check (L349) runs on the NORMALIZED bare URL, so the same repo with a different token is still the same source (409, not a second row).
|
||||
- **Clone-time credential (task 02):** `clone_url_for(row) -> str` in `app/rag/git_sources.py` (next to `effective_sources`): `row.url` unchanged when `token` is NULL; otherwise inject `https://x-access-token:<token>@<host>/<path>` (https rows only — a token on a non-https row is a no-op with a warning log). `repo_name` keeps operating on the bare `row.url`. Callers switch from `row.url` to `clone_url_for(row)`: `app/api/sync.py:296` and `scripts/import_docs.py::_resolve_sources` (both already import from `app.rag.git_sources`).
|
||||
- **Output sanitization (task 02, LOCKED A2):** every API surface that returns a git URL runs it through `sanitize_url(url)` (new, in `app/rag/git_sources.py`): strips the userinfo component (`https://…@host/…` → `https://host/…`), leaves ssh/`git@`/local paths untouched. Applied to `GitSourceOut.url` / `GitSourceRow.url` construction (GET list L235/L297, GET single, the `BOR_GIT_SOURCES` env fallback rows L250 — env rows can embed tokens too) and to any sync-status field echoing a repo URL (grep for `url=` in the sync responses). Belt-and-braces for legacy embedded-token rows whose credential is NOT in the `token` column: their DB value is untouched (the clone still authenticates from the stored URL) but no API/UI output ever shows the credential.
|
||||
- **UI (task 03):** the add form gains a second field — a masked `<input type="password" id="git-source-token">`, optional, labelled "Token (private repos)" with a visible "optional" hint; submit sends `{ url, token }` (token omitted when blank). The edit modal mirrors it with placeholder "leave blank to keep the current token" (blank → omit from PATCH = no change). Every display site keeps rendering `s.url` — now bare by server sanitization, so list cells, `title` attributes, the delete row, and the ignore-list context become token-free with no per-site change. No new CSS beyond reusing the existing form-field styles (the theme's input treatment).
|
||||
- **NOT touched:** local-kind sources (no URL credential), the `BOR_GIT_SOURCES` env parsing (its rows are sanitized on OUTPUT only), the upload endpoint, sync scheduling, and the Sources page layout.
|
||||
|
||||
## Tasks
|
||||
1. `01_token_storage.md` — migration + `GitSource.token` + input schemas (`GitSourceIn`/`GitSourcePatchIn`); no token in any output shape.
|
||||
2. `02_clone_url_and_sanitization.md` — URL/token normalization on write, `clone_url_for` at clone time, `sanitize_url` on every output.
|
||||
3. `03_ui_token_field.md` — masked token field in the add form + edit modal; display stays `s.url` (now bare).
|
||||
4. `04_token_tests.md` — unit + integration + isolated E2E `test_git_source_tokens.py`.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: `tests/unit/test_git_source_token.py` (new, task 04) — `sanitize_url` (https userinfo stripped, ssh/git@/local untouched, no-userinfo unchanged), `clone_url_for` (NULL token → bare URL; token → injected; non-https token → bare + no crash), normalization (embedded token moved to the column when no explicit token; explicit token wins; duplicate on bare URL).
|
||||
- Integration: `tests/integration/test_git_sources_api.py` (extend) — POST with `token` → GET list/single responses contain the token NOWHERE (assert on the raw JSON text) and show the bare URL; POST with an old-style embedded-token URL → stored bare + token column populated, responses clean; PATCH token replace/clear semantics; the sync flow builds the clone URL with the injected token (mock `clone_or_pull`).
|
||||
- E2E: `tests/e2e/test_git_source_tokens.py` (new, task 04) — isolated run per AGENTS.md §4: add a private repo through the Sources UI (bare URL + token) → the list row shows the bare URL, the token is absent from the page text, the `title` attribute, and `GET /api/git-sources` JSON; edit the row (blank token) → no 4xx, token kept.
|
||||
- Coverage: **>90%** on `app/` (validate.sh gate).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] A private repo added via the UI (or a pasted embedded-token URL) syncs/clones fine, and its token appears in NO API response, NO page text, and NO attribute.
|
||||
- [ ] Legacy embedded-token rows (pre-phase) still clone, and their API/UI output is token-free.
|
||||
- [ ] Public repos and local sources behave byte-identically to before.
|
||||
- [ ] `uv run pytest` green; `uv run pytest --cov=app --cov-report=term-missing` TOTAL >90%; `uv run ruff check . && uv run pyright` clean.
|
||||
- [ ] One `--no-gpg-sign` commit; phase dir moved to `.agents/phases/complete/` by the pipeline gate.
|
||||
|
||||
## Locked decisions
|
||||
- **A2 — the token is stored PLAINTEXT in a dedicated `GitSource.token` column (cloneability requires the raw credential; no external secrets backend), is NEVER returned by any API shape, and legacy embedded-token URLs are sanitized on output while keeping their stored value for clones (owner-confirmed 2026-09-24, roadmap confirmation).**
|
||||
- **A6 — pasting an old-style embedded-token URL is accepted and normalized (userinfo → `token` column); an explicit `token` field wins over an embedded one (owner-confirmed: same confirmation — the proposed design).**
|
||||
|
||||
## Commit
|
||||
```bash
|
||||
git add app/ alembic/ frontend/ scripts/ tests/ .agents/phases/ && git commit --no-gpg-sign -m "feat(sources): add private git repos with a masked token that never reaches the UI or API"
|
||||
```
|
||||
@@ -0,0 +1,38 @@
|
||||
# Task 01 — Token storage: model, migration, input schemas
|
||||
|
||||
**Phase:** `121_git_source_tokens` · **Source:** `TODO.md:5` — "Need a way to add private repos without exposing the token in the UI (like when adding an https repo `https://myuser:ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx@github.com/myuser/my-private-repo.git`)"
|
||||
|
||||
## Objective
|
||||
The `git_sources` table can hold a per-row token, and the add/patch input shapes can carry one — while NO output shape (`GitSourceOut`, `GitSourceRow`, list, single, env-fallback rows) ever can.
|
||||
|
||||
## Work
|
||||
1. `app/models.py` — `GitSource` (L245): add
|
||||
```python
|
||||
#: Private-repo credential (phase 121, LOCKED A2): the PAT the owner
|
||||
#: types into the masked Sources-page field. NULL = public repo (or a
|
||||
#: legacy row whose credential is still embedded in ``url``). Stored
|
||||
#: plaintext BY NECESSITY — the repo must remain cloneable, so the
|
||||
#: raw credential must be recoverable at sync time; the DB is the
|
||||
#: trusted store and is never served to the UI. Injected into the
|
||||
#: clone URL ONLY at clone time
|
||||
#: (:func:`app.rag.git_sources.clone_url_for`); NEVER returned by
|
||||
#: any API shape (the output models gain no token field).
|
||||
token: Mapped[str | None] = mapped_column(Text, default=None)
|
||||
```
|
||||
(docstring first, then the column — the phase-89/105 field-docstring house style).
|
||||
2. `alembic/versions/` — new revision (head of the current chain): `op.add_column("git_sources", sa.Column("token", sa.Text(), nullable=True))` + downgrade `op.drop_column`. Follow the existing migration file conventions (check the latest revision for the revision/down_revision pattern).
|
||||
3. `app/schemas.py`:
|
||||
- `GitSourceIn` (L561): add `token: str | None = Field(default=None, max_length=500)` with a `before`-mode trim validator (the `_trim_url` L594 precedent) — plus the docstring note: masked input from the Sources page; absent/None = no credential.
|
||||
- `GitSourcePatchIn` (L651): add `token: str | None = Field(default=None, max_length=500)` — docstring the PATCH tri-state: **absent/None = no change, non-empty = replace, empty string = clear**.
|
||||
- `GitSourceOut` (L605) / `GitSourceRow` (L626): add NO field; extend their docstrings with the explicit "no token — never a response field (phase 121)" note so the omission is a documented contract, not an accident.
|
||||
4. No endpoint changes in this task (acceptance of `token` and normalization are task 02) — the extra field on the input models is inert until then (Pydantic would currently just pass it through unused; task 02 consumes it).
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: `tests/unit/test_git_source_token.py` (task 04 extends): `GitSourceIn`/`GitSourcePatchIn` accept/trim `token`; `GitSourceOut`/`GitSourceRow` reject a `token` key (they are output models built from rows — assert constructing them with a token kwarg raises).
|
||||
- Integration: `tests/integration/test_git_sources_api.py` (task 04) — after `alembic upgrade head`, `git_sources.token` exists (a `SELECT` sanity check in the existing test fixtures).
|
||||
- Coverage: **>90%** on `app/` for the touched modules.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `uv run alembic upgrade head` applies the new revision on a clean DB and the downgrade removes the column.
|
||||
- [ ] The ORM round-trips a `token` value; output models have no token field (grep + unit assertion).
|
||||
- [ ] `uv run pytest` green; `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -0,0 +1,29 @@
|
||||
# Task 02 — Clone-time credential + output sanitization
|
||||
|
||||
**Phase:** `121_git_source_tokens` · **Source:** `TODO.md:5` — "Need a way to add private repos without exposing the token in the UI …"
|
||||
|
||||
## Objective
|
||||
The token is injected into the clone URL only at clone time, old-style embedded-token URLs are normalized into the column on write, and every URL that leaves the API is token-free — including legacy rows and env-fallback rows.
|
||||
|
||||
## Work
|
||||
1. `app/rag/git_sources.py` — add two pure helpers (unit-testable, no DB):
|
||||
- `sanitize_url(url: str) -> str` — strip the userinfo component of `https?://` URLs (`https://user:pass@host/path` → `https://host/path`); leave `ssh://`, `git@`, and local paths untouched; idempotent. Use a small regex (`^(https?://)([^/@]+)@` → `\1`) — never a URL parser that re-serializes (byte-identical output for clean URLs is a requirement: the phase-50/35 contract is that stored URLs surface verbatim when they carry no credential).
|
||||
- `clone_url_for(row) -> str` — `row.url` when `row.token` is falsy; for an `https://` row with a token, inject `https://x-access-token:<token>@<host>/<path>` (replace any existing userinfo with the column credential); for a non-https row with a token, log a warning and return `row.url` unchanged (a token cannot authenticate ssh — the owner must use a deploy key/agent there).
|
||||
- Also `normalize_credential(url, token) -> (bare_url, effective_token)` — the write-path normalizer: if `url` (https only) contains userinfo, strip it → bare URL, and the embedded credential becomes `effective_token` UNLESS `token` is non-None (explicit wins, LOCKED A6). Returns the input untouched for clean URLs.
|
||||
2. `app/api/git_sources.py`:
|
||||
- POST git source (L342–355): run `normalize_credential(payload.url, payload.token)`; store the BARE url + `effective_token`; the duplicate check (L349) runs on the bare URL.
|
||||
- PATCH (the url/token branch): when `payload.url` or `payload.token` is present, re-normalize the (current or new) pair with the same rules — PATCH token tri-state from task 01 (None = no change, `""` = clear → store NULL, non-empty = replace).
|
||||
- Every output construction runs `sanitize_url` on the URL before it enters the response: the DB-row list path (L235/L297), the GET single path (L423), and the `BOR_GIT_SOURCES` env-fallback rows (L250 — an env URL can embed a token; the ENV VALUE itself is untouched, only the response is masked). Grep the router for any other `url=` response field (including sync-status echoes — `app/api/sync.py` responses that surface a repo URL get the same treatment) and sanitize those too.
|
||||
3. `app/api/sync.py` (L296) + `scripts/import_docs.py::_resolve_sources` — swap `row.url` → `clone_url_for(row)` at the clone call site (`clone_or_pull(clone_url_for(row), sources_root / repo_name(row.url))` — `repo_name` stays on the bare URL so the checkout directory name is credential-free).
|
||||
4. ASSUMPTION: `x-access-token` as the injected userinfo username (GitHub-agnostic — any git host that accepts `https://user:token@` treats the first component opaquely; `oauth2:` is also common, but `x-access-token` works on GitHub and GitLab and reads as non-identifying).
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: `tests/unit/test_git_source_token.py` (task 04 finalizes) — `sanitize_url` (strip/no-op/idempotent/ssh/git@/local), `clone_url_for` (NULL token; token injected; non-https token no-op), `normalize_credential` (embedded→column, explicit wins, clean URL untouched).
|
||||
- Integration: `tests/integration/test_git_sources_api.py` (task 04) — POST embedded-token URL → row.url bare + row.token populated; GET list JSON (raw text) contains the token NOWHERE; sync with a token row (mock `clone_or_pull`) receives the injected URL and a credential-free checkout path.
|
||||
- Coverage: **>90%** on the touched modules.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] No token string in ANY API response for a token-bearing row (integration assertion on raw JSON text).
|
||||
- [ ] A legacy row (token embedded in the stored `url`, `token` NULL) still produces the ORIGINAL stored URL at clone time (the credential keeps working) but its API output is masked.
|
||||
- [ ] `repo_name` / checkout paths are credential-free.
|
||||
- [ ] `uv run pytest` green; `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -0,0 +1,30 @@
|
||||
# Task 03 — UI: masked token field on add + edit
|
||||
|
||||
**Phase:** `121_git_source_tokens` · **Source:** `TODO.md:5` — "Need a way to add private repos without exposing the token in the UI …"
|
||||
|
||||
## Objective
|
||||
The Sources-page git-source form takes a separate masked token field (add and edit); every display surface shows the bare URL (server-sanitized) and the token is nowhere in the DOM.
|
||||
|
||||
## Work
|
||||
1. `frontend/assets/git-sources.js`:
|
||||
- Add form (the `#git-source-url` field at L241, submit wiring at L942–945): add a second labelled field
|
||||
```html
|
||||
<label for="git-source-token">Token <span class="field-hint">optional — private repos</span></label>
|
||||
<input type="password" id="git-source-token" autocomplete="off" placeholder="ghp_… or another PAT">
|
||||
```
|
||||
(reuse the existing form-field markup/CSS classes from the url field — the theme's input treatment, no new CSS needed beyond the existing `.field-hint` or an equivalent inline span). Submit body becomes `(url, token) => ({ url, ...(token ? { token } : {}) })` — blank token = key omitted (None = no credential).
|
||||
- Edit modal (the url display/edit at L697): the same masked token field, placeholder "leave blank to keep the current token"; PATCH body includes `token` ONLY when non-blank (blank → omitted → no change — the task-01 tri-state).
|
||||
- Display sites (L395–406 list cell incl. the `title` attribute, L533 delete row, L763/L830 ignore-list context): keep rendering `s.url` UNCHANGED — the server now returns bare URLs, so nothing to do per site. Add a source-comment note (one line) that URLs arrive sanitized server-side (phase 121) and the UI must never re-embed a credential.
|
||||
2. `frontend/assets/styles.css` — only if the "optional" hint span has no existing class to reuse: a minimal `.field-hint` (muted color, contrast ≥4.5:1 per PLAN §7, small).
|
||||
3. ASSUMPTION: the password field is `type="password"` with `autocomplete="off"` (a PAT is not a site credential; browsers must not offer to save it).
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: `tests/unit/test_git_source_token.py` (task 04) — house-style source assertions: `#git-source-token` is `type="password"` and `autocomplete="off"`; the submit body omits a blank token; the edit PATCH omits a blank token; no display site concatenates a token.
|
||||
- E2E: `tests/e2e/test_git_source_tokens.py` (task 04) — the UI scenarios.
|
||||
- Coverage: n/a (frontend) — the validate.sh `app/` gate must stay green.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] Adding a private repo through the UI with a token succeeds; the list row shows the bare URL.
|
||||
- [ ] The token is absent from the rendered page text, the `title` attribute, and the Sources-page DOM (E2E assertion).
|
||||
- [ ] Editing with a blank token keeps the existing credential (integration: the PATCH tri-state).
|
||||
- [ ] `uv run pytest` green; `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -0,0 +1,36 @@
|
||||
# Task 04 — Token tests: unit + integration + isolated E2E
|
||||
|
||||
**Phase:** `121_git_source_tokens` · **Source:** `TODO.md:5` — "Need a way to add private repos without exposing the token in the UI …"
|
||||
|
||||
## Objective
|
||||
Pin the whole credential contract: helpers are pure and correct, no token ever crosses the API boundary (raw-JSON assertion), legacy rows stay cloneable and masked, and the UI never renders a credential.
|
||||
|
||||
## Work
|
||||
1. `tests/unit/test_git_source_token.py` (new):
|
||||
- `sanitize_url` — https userinfo stripped (`https://myuser:ghp_x@github.com/x/y.git` → `https://github.com/x/y.git`), clean https unchanged byte-identically, `ssh://git@host/x.git` untouched, `git@github.com:x/y.git` untouched, a local path untouched, idempotent on already-clean URLs.
|
||||
- `clone_url_for` — `token` NULL → `row.url` verbatim; https + token → `https://x-access-token:<token>@host/path` (existing userinfo REPLACED); non-https + token → `row.url` (no crash, warning logged).
|
||||
- `normalize_credential` — embedded userinfo → bare URL + token populated; explicit token wins over embedded; clean URL + None token → unchanged.
|
||||
- Output models: `GitSourceOut`/`GitSourceRow` reject a `token` kwarg (no response field can ever carry it).
|
||||
- Frontend source assertions (house style): `#git-source-token` is `type="password"` + `autocomplete="off"`; submit/PATCH omit a blank token.
|
||||
2. `tests/integration/test_git_sources_api.py` (extend):
|
||||
- POST `{url: "https://github.com/acme/private.git", token: "ghp_test123"}` → 201; `GET /api/git-sources` raw response TEXT does not contain `ghp_test123`; the row's `url` is the bare URL; `GET` single likewise.
|
||||
- POST an old-style `https://myuser:ghp_legacy@github.com/acme/legacy.git` (no token field) → stored `url` bare, `token` = `ghp_legacy`; responses token-free.
|
||||
- POST the same repo a second time (different token) → 409 (duplicate on the bare URL).
|
||||
- PATCH token tri-state: absent → kept; non-empty → replaced (clone URL uses the new one); `""` → cleared (clone URL bare again).
|
||||
- Legacy-row simulation (insert a row directly with the embedded URL, `token` NULL): `GET` output masked; the sync path (mock `clone_or_pull`) still receives the ORIGINAL stored URL (clone works).
|
||||
- Sync flow: a token row → `clone_or_pull` called with the injected URL; the checkout path is credential-free.
|
||||
3. `tests/e2e/test_git_source_tokens.py` (new — isolated run per AGENTS.md §4: `uv run pytest tests/e2e/test_git_source_tokens.py -v --no-cov`):
|
||||
- Open the Sources page (admin), add a git source with a bare URL + a distinctive fake token (`ghp_e2esecret…`);
|
||||
- assert: the list row renders the BARE URL; the token string is absent from `document.body.innerText`, from every `title` attribute, and from the `GET /api/git-sources` JSON (via a `page.request.get` inside the test);
|
||||
- open the edit modal: the token field is blank (never pre-filled — a password must not be echoed back, so it is simply empty by design); save with it blank → 200, row intact;
|
||||
- remove the source (cleanup) — the list is empty again.
|
||||
4. Run the full gate: `uv run pytest`, `uv run pytest --cov=app --cov-report=term-missing` (TOTAL >90%), the isolated E2E file, `uv run ruff check . && uv run pyright`.
|
||||
|
||||
## Testing & Quality
|
||||
- This task IS the phase's test suite (see Work).
|
||||
- Coverage: **>90%** on `app/` — the phase's `app/` code (helpers + router + schema + model) is fully exercised by the unit/integration cases.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] All test artifacts exist and pass; the isolated E2E file passes standalone.
|
||||
- [ ] The raw-JSON "token nowhere" assertion covers list AND single AND sync-status surfaces.
|
||||
- [ ] `uv run pytest --cov=app` TOTAL >90%; lint + types clean.
|
||||
@@ -0,0 +1,54 @@
|
||||
# Phase 122 — Image documents: standalone images become first-class, retrievable documents
|
||||
|
||||
**Source:** `TODO.md` L6 — "Need to support images. Images uploaded as part of documents or as standalone images should be read, summarized, and retrieved like any other document. Note that the embedding model won't support images, so the only embedded part of an image will be the summary generated by the model. The user should be able to turn on and off image support in their .env depending on whether their model supports it. Images retrieved by the RAG should be shown in the chat nicely and users should be able to submit images as part of their question in brain of reese."
|
||||
**Story:** n/a (feature request; extends the phase-28/30/38 import pipeline, phase-90 no-scan uploads, and the RAG/chat assets).
|
||||
**Context:** `app/config.py` — `Settings` (`BOR_` prefix; `upload_dir` L361, `sources_dir` L355, `upload_max_mb` L370 — raw-string/`expanduser` house convention). `app/rag/importer.py` — `iter_importable_files` (L235, extension filter via `llm.settings.import_extension_set` + `match_extension` L217), `import_sources` (L279, `prune` L283), `_index_file` (L435 — `read_text` L453, sha256 over text, Document upsert, the unchanged/hash path with the phase-118 summary backfill), `_store_summary` (L582 — lite-model summary + the position −1 `is_summary` chunk), `_prune` (L649 — deletes docs of the imported sources not in `seen`). `app/models.py` — `Document` (L102: `content`, `content_hash`, `summary` L140, `created_at`/`created_at_manual`), `Chunk` (L148: `is_summary` L161). `app/rag/summarizer.py` + `app/rag/llm.py` — the lite summary path + the chat-model client (`Settings` model names, `check_models`). `app/rag/archive_upload.py` — archive unpack into `upload_dir` (image members land on disk today, then get filtered out by the extension walk). `app/api/git_sources.py:432` — the upload endpoint. `app/api/docs.py` — the document content endpoint (document viewer). `app/rag/retriever.py` / `app/rag/agent.py` (read tool, message build L1446–1448) / `app/api/chat.py` — RAG + the SSE sources frames. `app/api/config.py:30` — `GET /api/config` public flags dict (task 01 of phase 123 extends it). Frontend: `frontend/assets/app.js` (chat source chips), `frontend/assets/document.js` + `frontend/document.html` (`#doc-content` L144), `frontend/assets/sources.js` (Sources page). `alembic/` — migrations.
|
||||
|
||||
## Objective
|
||||
With `BOR_IMAGES=true`, a standalone image file — arriving as a direct upload, inside an uploaded archive, or as a file in a git/local source — becomes a first-class document: the vision model (the chat model) describes it, the description is the document's content AND summary, only the description is embedded (the embedding model never sees pixels), the image bytes persist and are served, and the image shows up in the Sources page, the document viewer, and the chat — with retrieved image docs rendered inline in the answer's sources.
|
||||
|
||||
## Dependencies
|
||||
- `121_git_source_tokens` (todo) — pipeline predecessor (execution order) only; no code dependency.
|
||||
- Code dependencies (all complete): phase 30 summary pipeline (`_store_summary`, `is_summary` chunk), phase 90 no-scan upload, phase 89/105 per-source walk options, the RAG agent + SSE sources frames.
|
||||
|
||||
## Design (shared by all tasks — the executor reads this, not the chat)
|
||||
- **Toggle (task 01, LOCKED A3):** three new `Settings` fields — `images: bool = False` (`BOR_IMAGES`, `0`/`false` off — the phase-67 `llm_retries` bool style), `image_extensions: str = "png,jpg,jpeg,webp,gif,bmp"` (`BOR_IMAGE_EXTENSIONS`, comma-separated, lowercased into a frozenset by the importer — the `import_extension_set` property precedent), and `image_dir: str = "~/bor-sources/images"` (`BOR_IMAGE_DIR`, raw-string/`expanduser` convention — the persistent home for image bytes, deliberately separate from `sources_dir`/`upload_dir`). `.env.example` gets all three with a comment: **off by default — enable only when your chat model supports vision, because image descriptions are generated by the chat model.** `GET /api/config` (task 01) gains `images: bool` so the UI can gate affordances (consumed by phase 123; the Sources page can show an "images off" hint — optional, not required).
|
||||
- **Why the bytes are copied (task 02):** upload dirs are REPLACED on every upload (`archive_upload.swap_in`), git checkouts are re-cloned, and local dirs are user-edited — a served image must outlive its source file. The importer copies each ingested image to `image_dir/<doc-uuid>.<ext>` (created on demand) and stores that path in `Document.image_path`. The copy happens ONLY when the doc is new or its hash changes; a replaced image deletes the stale copy; pruned docs delete their copy.
|
||||
- **Storage (task 02):** `Document.is_image: bool` (server default `false` — every pre-phase-122 row is a text doc) + `Document.image_path: str | None` (NULL for text docs). One migration, one downgrade.
|
||||
- **Ingest (task 02):** the walk: `iter_importable_files`/`import_sources` accept the image frozenset IN ADDITION to `import_extension_set`, ONLY when `settings.images` is true (images are never user-configurable via `BOR_IMPORT_EXTENSIONS` — the toggle is the single knob, LOCKED A3/A4). `_index_file` branches on image extension: read BYTES (not `read_text`), sha256 over the bytes (the digest rule is unchanged — content identity), copy to `image_dir`, set `is_image` + `image_path`, and `content` = the vision description (task 03). The normal chunk pipeline then embeds the content (= the description) — that is exactly the TODO's "the only embedded part of an image will be the summary generated by the model"; the phase-30 summary chunk (`is_summary`, position −1) mirrors `Document.summary`, which equals the description too. Title = the file stem (the non-markdown rule at L505–509). The unchanged/hash path works unmodified (byte digest → "unchanged" skips re-describing; the phase-118 backfill path re-describes a NULL-summary image doc on its next sync — same fail-soft). **Prune guard:** `_prune` (L649) must NOT delete `is_image` docs while `settings.images` is false (an image doc is invisible to an images-off walk, not a deleted file — otherwise turning the toggle off and syncing would silently destroy the image documents). Toggle ON → normal prune semantics (a deleted image file prunes its doc + copy).
|
||||
- **Description (task 03, LOCKED A3):** `describe_image` in `app/rag/summarizer.py` (one function, the summarizer module owns model-text generation): a SINGLE chat-model call (`Settings.llm_chat_model` — the vision model; the lite summary model is NOT assumed vision-capable, LOCKED A3) with a multimodal user message `[{type: "text", text: <fixed describe prompt>}, {type: "image_url", image_url: {url: <data URL from the bytes + mime>}}]`; the prompt asks for a faithful, retrieval-oriented description (what is shown, any text/labels/diagram content, salient details — the description is the ONLY thing retrievable, so it must carry the image's meaning). Output capped at `settings.summary_max_chars` (the description IS the doc's summary; the phase-30 cap keeps it uniform). Stored: `Document.summary = Document.content = description`. **Fail-soft:** a failed/empty description → the doc is SKIPPED (no row, `ImportSummary` counts it in a new `images_failed` counter + a `logger.warning` with source/path) — an undescribed image is unsearchable noise; the sync continues (the importer's existing fail-soft convention).
|
||||
- **Serve + display (task 04):** `GET /api/documents/{doc_id}/image` (new route in `app/api/docs.py`) — 404 for missing docs and non-image docs; serves `image_path` bytes with the correct `Content-Type` (ext → mime map: png/jpeg/webp/gif/bmp) — PUBLIC like the document content itself (this app's document content is already anonymous-readable; the image is part of that content). The document content endpoint (the one `frontend/assets/document.js` boots against) gains `is_image: bool` + `image_url` (the new route's path, absent for text docs) so `document.html` renders `<img src>` (max-width 100%, the theme's image treatment) with the summary/description text below it instead of the markdown content; the Sources page row for an image doc shows a small thumbnail (lazy-loaded, `loading="lazy"`, aspect-ratio box) or the existing doc icon when the fetch is not yet possible offline — the thumbnail is a progressive enhancement (a fetch failure falls back to the icon).
|
||||
- **RAG display (task 05):** the SSE sources frames (and any sources-list shape the chat bubble renders from) carry an OPTIONAL `image_url` on image docs (the retriever/agent know the `Document` row — add the field where `SourceRef`-shaped frames are built in `app/api/chat.py`/`app/rag/retriever.py`); the chat's sources block renders a compact inline `<img>` (capped height, the summary as caption/alt) for image docs — "shown in the chat nicely" (TODO L6). The agent's `read` tool on an image doc returns its description prefixed with a one-line marker (e.g. `Image document — description generated from the image:`) so the model knows what it is reading. `alt` text = the summary everywhere (WCAG).
|
||||
- **NOT touched (this phase):** chat-side image submission (phase 123), the lite summary path for TEXT docs, archive unpacking (image members already land on disk — only the walk filter changes), and git/local sync scheduling.
|
||||
- **Locked assumptions:** **A3** — descriptions use the CHAT model (`BOR_LLM_CHAT_MODEL`, must be vision-capable); `BOR_IMAGES` defaults to **false**; generation failure → doc skipped + logged. **A4** — "images uploaded as part of documents" = standalone image files arriving via direct upload / uploaded archives / source walks — NOT embedded-image extraction from PDFs/DOCX.
|
||||
|
||||
## Tasks
|
||||
1. `01_image_toggle.md` — `BOR_IMAGES` / `BOR_IMAGE_EXTENSIONS` / `BOR_IMAGE_DIR` settings + `.env.example` + `GET /api/config` flag; off = byte-identical behavior.
|
||||
2. `02_image_ingest.md` — `Document.is_image`/`image_path` + migration; walk accepts image extensions when on; `_index_file` binary branch + persistent copy; prune guard when off.
|
||||
3. `03_image_description.md` — `describe_image` (chat-model vision), content = summary = description, fail-soft skip + counter.
|
||||
4. `04_serve_and_display.md` — `GET /api/documents/{id}/image`; document viewer + Sources page rendering.
|
||||
5. `05_rag_display.md` — `image_url` on chat source frames + inline image in the chat sources block + the agent `read` marker.
|
||||
6. `06_image_tests.md` — unit + integration + isolated E2E `test_image_documents.py`.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: `tests/unit/test_image_documents.py` (new, task 06) — settings parsing (toggle off by default, extensions frozenset, mime map), the `_index_file` image branch (bytes digest, copy to `image_dir`, `is_image`/`image_path` set, text `content` never read for an image), the prune guard (toggle off → image docs survive; toggle on → deleted image prunes), `describe_image` prompt shape (multimodal content list, chat model, cap) with a mock client, and the fail-soft skip path.
|
||||
- Integration: `tests/integration/test_docs_api.py` (extend) — the image route (200 + correct Content-Type for a seeded image doc; 404 for text docs and missing ids); the content endpoint exposes `is_image`/`image_url` for image docs and omits them for text docs (byte-identical text-doc responses); `import_sources` end-to-end with `images=True` and a mock vision client (a fixture PNG → doc row with description content + `is_summary` chunk embedding; `images=False` → the file is ignored, pre-existing image doc survives prune).
|
||||
- E2E: `tests/e2e/test_image_documents.py` (new, task 06) — isolated run per AGENTS.md §4, `BOR_IMAGES=true` for this suite's app instance: upload a small fixture PNG (via the existing upload endpoint's UI or `page.request`) → sync → the Sources page lists it (thumbnail or icon) → open the document viewer → the image renders with its description → ask a question the mock LLM grounds on the image doc → the chat's sources block shows the inline image.
|
||||
- Coverage: **>90%** on `app/` (validate.sh gate).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] With `BOR_IMAGES=true`: an uploaded standalone image (direct or in an archive) and an image file in a git/local source become documents whose content/summary is the vision description and whose ONLY embedded text is that description.
|
||||
- [ ] With `BOR_IMAGES=false` (the default): every request, walk, and response is byte-identical to pre-phase; existing image docs (if any) survive a sync.
|
||||
- [ ] The image renders in the document viewer and in the chat's sources block (inline, with alt text); a failed description skips the doc and logs — the sync completes.
|
||||
- [ ] `uv run pytest` green; `uv run pytest --cov=app --cov-report=term-missing` TOTAL >90%; `uv run ruff check . && uv run pyright` clean.
|
||||
- [ ] One `--no-gpg-sign` commit; phase dir moved to `.agents/phases/complete/` by the pipeline gate.
|
||||
|
||||
## Locked decisions
|
||||
- **A3 — image descriptions are generated by the CHAT model (`BOR_LLM_CHAT_MODEL`, vision-capable); `BOR_IMAGES` defaults to false; a failed/empty description skips the doc and logs (owner-confirmed 2026-09-24, roadmap confirmation).**
|
||||
- **A4 — "images uploaded as part of documents" = standalone image files via direct upload / uploaded archives / source walks — no embedded-image extraction from PDFs/DOCX (owner-confirmed 2026-09-24).**
|
||||
- **Prune guard (derived from A3/A4, same confirmation):** images-off syncs never prune `is_image` docs — turning the toggle off must not destroy image documents.
|
||||
|
||||
## Commit
|
||||
```bash
|
||||
git add app/ alembic/ frontend/ tests/ .env.example .agents/phases/ && git commit --no-gpg-sign -m "feat(rag): index standalone images as documents — described, embedded, and displayed via the vision model"
|
||||
```
|
||||
@@ -0,0 +1,25 @@
|
||||
# Task 01 — Image toggle: BOR_IMAGES + extensions + dir, off by default
|
||||
|
||||
**Phase:** `122_image_documents` · **Source:** `TODO.md:6` — "The user should be able to turn on and off image support in their .env depending on whether their model supports it."
|
||||
|
||||
## Objective
|
||||
The single env knob for image support exists and is surfaced — `BOR_IMAGES` (default **false**), `BOR_IMAGE_EXTENSIONS`, `BOR_IMAGE_DIR` — with `GET /api/config` exposing the flag for UI gating. Toggle off = byte-identical behavior to pre-phase.
|
||||
|
||||
## Work
|
||||
1. `app/config.py` — three new `Settings` fields (house docstring style, the `upload_dir`/`llm_retries` precedents):
|
||||
- `images: bool = False` — `BOR_IMAGES`, `0`/`false` off (LOCKED A3 default). Docstring: master switch for image-document indexing (phase 122) — off by default, enable only when the chat model supports vision (descriptions come from it).
|
||||
- `image_extensions: str = "png,jpg,jpeg,webp,gif,bmp"` — `BOR_IMAGE_EXTENSIONS`, comma-separated, case-insensitive; a property/parse into a lowercased-dotted frozenset (the `import_extension_set` precedent) — the image set is SEPARATE from `import_extension_set` (images are never user-added via `BOR_IMPORT_EXTENSIONS`).
|
||||
- `image_dir: str = "~/bor-sources/images"` — `BOR_IMAGE_DIR`, raw string, `Path.expanduser()` applied by the importer (the `sources_dir`/`upload_dir` convention) — the persistent home for image bytes (uploads are replaced, checkouts re-cloned — the copy must outlive the source file).
|
||||
2. `.env.example` — the three entries with the comment block: off by default + the vision-model dependency note (LOCKED A3).
|
||||
3. `app/api/config.py:30` — the `app_config` dict gains `"images": settings.images` (the dict is `str | bool`-valued — bools already allowed). Extend the docstring: consumed by the chat composer (phase 123) to show/hide the attach control, optionally by the Sources page.
|
||||
4. ASSUMPTION: `GET /api/config` is already anonymous-readable (the UI gates on it pre-login in phase 123 — no auth change here).
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: `tests/unit/test_image_documents.py` (task 06 finalizes) — defaults (`images` False, extensions frozenset `{".png", …}` with the dotted form the matchers expect, dir default), env overrides, the frozenset parse is case-insensitive and trims spaces.
|
||||
- Integration: the existing `GET /api/config` test asserts the new `images` key (default false in the test env).
|
||||
- Coverage: **>90%** on the touched modules.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `Settings()` with no env: `images is False`, the extension set is the six defaults, `image_dir` is the default path.
|
||||
- [ ] `GET /api/config` returns `images: false` in the default test env (byte-check the other keys unchanged).
|
||||
- [ ] `uv run pytest` green; `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -0,0 +1,30 @@
|
||||
# Task 02 — Image ingest: model fields, walk filter, binary index path, prune guard
|
||||
|
||||
**Phase:** `122_image_documents` · **Source:** `TODO.md:6` — "Images uploaded as part of documents or as standalone images should be read, summarized, and retrieved like any other document."
|
||||
|
||||
## Objective
|
||||
When `BOR_IMAGES=true`, standalone image files in ANY ingest path (direct upload, uploaded archive, git/local source walk) become `Document` rows — bytes persisted to `image_dir`, `is_image`/`image_path` set, `content` = the vision description (task 03) — and an images-off sync never prunes existing image docs.
|
||||
|
||||
## Work
|
||||
1. `app/models.py` — `Document` (L102): add, with house docstrings (the `summary` L140 / `created_at_manual` precedent):
|
||||
- `is_image: Mapped[bool] = mapped_column(Boolean, default=False, server_default=text("false"), nullable=False)` — True iff the doc's content is a vision description of an image (phase 122); the image bytes live at `image_path`.
|
||||
- `image_path: Mapped[str | None] = mapped_column(Text, default=None)` — absolute path of the persistent copy in `settings.image_dir`; NULL for text docs.
|
||||
2. `alembic/versions/` — new revision: both columns (`is_image` NOT NULL server_default 'false'; `image_path` nullable) + downgrade.
|
||||
3. `app/rag/importer.py`:
|
||||
- `iter_importable_files` (L235) / the walk in `import_sources` (L279): when `llm.settings.images`, accept a file iff its extension matches `import_extension_set` OR the image frozenset (task 01) — pass the image set in (the function takes explicit extension sets; the image set is NOT merged into `import_extension_set`).
|
||||
- `_index_file` (L435): image branch FIRST (before the `read_text` at L453) — if the path's extension is in the image set: `data = full_path.read_bytes()`, `digest = sha256(data)`, and on new/changed: copy `data` to `image_dir/<doc-id or uuid4>.<ext>` (dir created with `mkdir(parents=True, exist_ok=True)`), set `is_image=True` + `image_path` on the `Document` row, `content` = the description (task 03's `describe_image` — this task wires the call; the function lands in task 03, so for THIS task store `content = ""` placeholder ONLY if task 03 is not yet merged — the phases run task-ordered, so in practice task 03's function exists; wire it directly and let task 03 implement it. If implementing strictly per task: this task stores `content` via a `_describe_or_skip` hook that task 03 fills — keep the seam single and commented).
|
||||
- A CHANGED image (hash differs) deletes the stale `image_path` copy before replacing it.
|
||||
- The unchanged/hash path (L470+) works unmodified for images (byte digest); the phase-118 summary-backfill branch (L480) re-describes an image doc whose `summary` is NULL on the next sync (same fail-soft).
|
||||
- `_prune` (L649): the prune guard (LOCKED derived decision) — when `settings.images` is FALSE, skip every `is_image` doc (invisible to the walk ≠ deleted); toggle TRUE → normal prune + delete the `image_path` copy of each pruned image doc (also on the normal prune path when the file is gone).
|
||||
- `ImportSummary` (L100): new `images_failed: int = 0` counter + its slot in `format_counts`/`log` (L135–151) — task 03 increments it; add it now so the log shape is stable.
|
||||
4. ASSUMPTION (A4 re-stated): only standalone image FILES are ingested — no archive-of-documents extraction, no PDF/DOCX embedded-image pulls (the archive unpacker already places image members on disk; the walk now just accepts them).
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: `tests/unit/test_image_documents.py` (task 06) — the walk accepts `.png` only when `images=True` (off → ignored, the byte-identical default), the binary branch (digest over bytes, copy made, fields set, `read_text` never called for an image), the changed-image stale-copy delete, the prune guard (off → image doc survives; on + file gone → pruned + copy deleted), `images_failed` in the log line.
|
||||
- Integration: `tests/integration/test_docs_api.py` (task 06) — the `import_sources` end-to-end cases.
|
||||
- Coverage: **>90%** on the touched modules.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `alembic upgrade head` applies; a seeded image walk with `images=True` creates the doc row + `image_dir` copy; `images=False` ignores the file entirely.
|
||||
- [ ] A sync with `images=False` leaves a pre-existing image doc untouched (prune guard).
|
||||
- [ ] `uv run pytest` green; `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -0,0 +1,28 @@
|
||||
# Task 03 — Image description: the vision model writes the only embedded text
|
||||
|
||||
**Phase:** `122_image_documents` · **Source:** `TODO.md:6` — "the only embedded part of an image will be the summary generated by the model."
|
||||
|
||||
## Objective
|
||||
`describe_image` generates the image's description with the CHAT model (vision), the description becomes BOTH `Document.content` and `Document.summary` (so the chunk pipeline embeds exactly that text — and only that text), and a failed description fails soft (skip + count + log, sync continues).
|
||||
|
||||
## Work
|
||||
1. `app/rag/summarizer.py` — new `async def describe_image(llm, data: bytes, mime: str, settings=None) -> str | None` (the summarizer module owns model-text generation; follow the existing summary-call conventions — client, model, timeout, the `summary_max_chars` cap):
|
||||
- ONE chat-model call (`settings.llm_chat_model` — LOCKED A3; the lite summary model is not assumed vision-capable) with messages `[{role: "user", content: [{type: "text", text: <DESCRIBE_PROMPT>}, {type: "image_url", image_url: {url: f"data:{mime};base64,{b64}"}}]}]` — the multimodal content-list shape the OpenAI-compatible API expects.
|
||||
- `DESCRIBE_PROMPT` (a module constant, pinned by a unit test): a faithful, retrieval-oriented description — what is depicted, any visible text/labels/titles, diagram/table structure, salient details; 2–4 sentences of substance (the description is the ONLY retrievable text of the doc, so it must carry the image's meaning).
|
||||
- Return the stripped text capped at `settings.summary_max_chars` (the phase-30 cap — the description IS the summary); return `None` on any client error, empty response, or non-2xx (the caller fails soft). No retries beyond the SDK's own — a description failure must not stall a sync.
|
||||
2. `app/rag/importer.py` — wire the task-02 seam: the image branch's `content`/`summary` come from `describe_image` —
|
||||
- description `None` → **skip the doc entirely** (no row, no `image_dir` copy kept — delete the copy if it was made, or make the copy AFTER a successful description so a failure never leaves an orphan), `summary.images_failed += 1`, `logger.warning("import: image description failed source=%s path=%s", source, rel)` — the fail-soft skip (LOCKED A3).
|
||||
- success → `content = description`, then the existing `_store_summary` path (L582) runs with the description as the summary (the `is_summary` position −1 chunk mirrors it — phase-30 behavior, unchanged), and the normal content chunks embed the description (for a short description that is typically ONE content chunk + the summary chunk — the chunker's existing behavior, no special case).
|
||||
- the phase-118 backfill branch (unchanged image doc, `summary is None`) calls the SAME path — a description failure there keeps the doc as-is and logs (no row mutation).
|
||||
3. `app/rag/llm.py` — no new client: `describe_image` reuses the existing `llm.chat`-equivalent client the summarizer already uses for text summaries (verify the exact client method name in `app/rag/summarizer.py` and match it — the multimodal payload is a plain `list[dict]` message, so no client change is needed; IF the existing client hard-codes text-only `content: str` typing, extend its signature to accept `content: str | list` — pyright-clean).
|
||||
4. ASSUMPTION (A3 re-stated): the CHAT model describes; if the owner's chat model lacks vision, `describe_image` returns `None` (the SDK errors) and every image doc is skipped + logged — honest, visible failure (the `images_failed` counter in the sync log is the signal).
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: `tests/unit/test_image_documents.py` (task 06) — with a MOCK client: the prompt shape (text part + `image_url` data-URL part, correct model), the cap is applied, whitespace stripped; `None` on mock error / empty string / client exception; the importer's skip path (no row, `images_failed == 1`, warning logged, no orphan copy) and the success path (content == summary == description, `is_summary` chunk present, embedding called with the description text — the ONLY text embedded).
|
||||
- Integration: the mock-vision `import_sources` end-to-end (task 06).
|
||||
- Coverage: **>90%** on the touched modules.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] A fixture PNG through the mock vision client yields a doc whose `content` == `summary` == the description, with its embedding(s) derived from that text only.
|
||||
- [ ] A failing mock client skips the doc, bumps `images_failed`, logs, and the sync completes with the other docs indexed.
|
||||
- [ ] `uv run pytest` green; `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -0,0 +1,31 @@
|
||||
# Task 04 — Serve the image + render it in the document viewer and Sources page
|
||||
|
||||
**Phase:** `122_image_documents` · **Source:** `TODO.md:6` — "Images … should be read, summarized, and retrieved like any other document."
|
||||
|
||||
## Objective
|
||||
Image bytes are served through a dedicated document route, the document viewer renders the image with its description, and the Sources page shows an image affordance — image docs read like first-class documents in every existing surface.
|
||||
|
||||
## Work
|
||||
1. `app/api/docs.py` — new route `GET /api/documents/{doc_id}/image`:
|
||||
- 404 (the router's existing "unknown document" shape) for a missing doc and for a doc with `is_image` false / `image_path` NULL;
|
||||
- 404 if the file is missing on disk (defensive — the row exists but the copy was lost);
|
||||
- otherwise `FileResponse` (or a `Response` with the bytes) with `Content-Type` from an ext→mime map (`png`→`image/png`, `jpg`/`jpeg`→`image/jpeg`, `webp`→`image/webp`, `gif`→`image/gif`, `bmp`→`image/bmp` — the map lives in `app/rag/importer.py` or a small shared spot the unit tests can import; default `application/octet-stream` for an unexpected ext) and `Cache-Control: private, max-age=3600` (the image bytes are content-hashed — long enough, bustable by re-upload).
|
||||
- PUBLIC, like the document content endpoint (this app serves document content to anonymous visitors — the image is part of that content).
|
||||
- The document CONTENT endpoint the viewer boots against (same module): response gains `is_image: bool` (always present) + `image_url` (the `/api/documents/{id}/image` path — ABSENT for text docs, the `_drop_absent_share_url` omission precedent; never `null`). Text-doc responses gain only `is_image: false` — one new key, documented in the response schema's docstring.
|
||||
2. `frontend/assets/document.js` + `frontend/document.html`:
|
||||
- boot reads `is_image`; when true, `#doc-content` renders `<img src="{image_url}" alt="{summary}">` (block, `max-width: 100%`, the theme's surface treatment) with the description/summary text in the normal content slot below it (the document's readable content IS the description — no markdown render of a non-markdown string is needed; render it as the existing plain-content path).
|
||||
- an `<img>` error fallback: on `onerror` the image area shows a small "image unavailable" note (the 404-on-missing-file case) — the page still shows the description.
|
||||
3. `frontend/assets/sources.js` — the Sources page row for an image doc: a small thumbnail (48px box, `object-fit: cover`, `loading="lazy"`, `alt = summary`) where the doc icon sits; the thumbnail is a PROGRESSIVE enhancement — a failed fetch (or the row rendered before the fetch resolves) falls back to the existing icon (no layout shift beyond the fixed box). The doc title/path columns are unchanged.
|
||||
4. `frontend/assets/styles.css` — the viewer image block + the Sources thumbnail box (theme tokens; WCAG: alt text everywhere, no contrast concerns for decorative images).
|
||||
5. ASSUMPTION: the thumbnail uses the SAME full-size route (no separate thumb route) — a KB-scale image set makes a thumb pipeline unjustified; lazy loading keeps the Sources page fast.
|
||||
|
||||
## Testing & Quality
|
||||
- Integration: `tests/integration/test_docs_api.py` (task 06) — the image route (200 + exact `Content-Type` per ext for a seeded doc; 404 for a text doc; 404 for a missing id; 404 for a row whose file is deleted); the content endpoint: `is_image` present in ALL responses, `image_url` absent for text docs and present for image docs.
|
||||
- Unit: `tests/unit/test_image_documents.py` (task 06) — the ext→mime map (all six + the octet-stream default); house-style source assertions: the viewer renders the `img` from `image_url` with `alt = summary`, the sources row falls back to the icon on image error, no `null` in the text-doc content response.
|
||||
- E2E: `test_image_documents.py` scenarios (task 06) cover viewer + Sources rendering.
|
||||
- Coverage: **>90%** on the touched modules.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `GET /api/documents/{id}/image` serves the exact uploaded bytes with the right Content-Type; text docs 404.
|
||||
- [ ] The document viewer shows the image + its description; the Sources page shows the thumbnail (or the icon fallback).
|
||||
- [ ] `uv run pytest` green; `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -0,0 +1,24 @@
|
||||
# Task 05 — RAG display: image docs in the chat sources + the agent read marker
|
||||
|
||||
**Phase:** `122_image_documents` · **Source:** `TODO.md:6` — "Images retrieved by the RAG should be shown in the chat nicely."
|
||||
|
||||
## Objective
|
||||
When a retrieved/agent-read document is an image, the chat shows it: the sources block renders a compact inline image with its summary as caption/alt, and the agent's `read` tool tells the model it is reading a generated image description.
|
||||
|
||||
## Work
|
||||
1. `app/rag/retriever.py` / `app/api/chat.py` — the sources frames the chat bubble renders (the SSE `sources`/related-doc frames and the agent-sourced doc list): add an OPTIONAL `image_url` field to the per-doc ref shape — populated (the `/api/documents/{id}/image` path) iff the doc row has `is_image`, absent otherwise (the omission rule — text-doc frames stay byte-identical). The retriever already has the `Document` row; the agent's doc refs (the read-tool results / source list) do too — set it at the frame-build sites (grep for the source-ref construction in both modules; one shared helper `source_ref_with_image(doc, …)` keeps the two sites in lockstep).
|
||||
2. `frontend/assets/app.js` — the chat's sources block renderer: when a source ref carries `image_url`, render a compact inline `<img>` (max-height ~96px, `object-fit: contain`, the theme's surface, `alt` + visible caption = the doc summary — the "shown nicely" requirement) in place of / beside the existing doc chip text (keep the title + the existing chip affordance — the image is additive, not a replacement). A failed image load collapses to the plain chip (never a broken-image icon).
|
||||
3. `app/rag/agent.py` — the `read` tool's result for an image doc: prefix the description with the marker line `Image document — the text below is a description generated from the image:` (a module constant) so the model reasons about what it is reading; non-image docs' results are byte-identical.
|
||||
4. ASSUMPTION: the chat QUESTION side (users submitting images) is phase 123 — this task only covers RETRIEVED images in the answer's sources.
|
||||
5. ASSUMPTION: the sources-frame `image_url` is the only new frame field — no doc-id leak beyond what the frame already carries (the path encodes the doc id, same as the content endpoint).
|
||||
|
||||
## Testing & Quality
|
||||
- Integration: `tests/integration/test_chat_api.py` (extend, task 06) — a mocked grounded answer that includes an image doc in its sources → the SSE frame carries `image_url` for that ref only; a text-only grounding has NO `image_url` key anywhere (byte check).
|
||||
- Unit: `tests/unit/test_image_documents.py` (task 06) — the frame-helper (present/absent), the agent marker (image vs non-image result), house-style source assertions: the sources renderer reads `image_url`, sets `alt`, and falls back on image error.
|
||||
- E2E: `test_image_documents.py` scenario (task 06) — ask a question the mock LLM grounds on the fixture image doc → the chat sources block shows the inline image with its caption.
|
||||
- Coverage: **>90%** on the touched modules.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] A chat answer grounded on an image doc shows the inline image + caption in its sources block; text-doc answers render byte-identically to before.
|
||||
- [ ] The agent `read` result for an image doc carries the marker; the model sees the description, not raw bytes.
|
||||
- [ ] `uv run pytest` green; `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -0,0 +1,33 @@
|
||||
# Task 06 — Image tests: unit + integration + isolated E2E
|
||||
|
||||
**Phase:** `122_image_documents` · **Source:** `TODO.md:6` — "Need to support images …"
|
||||
|
||||
## Objective
|
||||
Pin the whole image-document contract: the off-by-default byte-identity, the ingest/description/serve pipeline, the RAG display, and the end-to-end user path (upload → Sources → viewer → chat) as an isolated Playwright suite.
|
||||
|
||||
## Work
|
||||
1. `tests/unit/test_image_documents.py` (new) — consolidates the per-task unit cases (the tasks ship code; this task ships the full pin):
|
||||
- settings: defaults (`images` False, six-extension frozenset, dir default), env overrides, case-insensitive parse (task 01);
|
||||
- the walk: image accepted iff `images=True`; off → the file is ignored (the default byte-identity);
|
||||
- `_index_file` image branch: digest over BYTES, copy to `image_dir`, `is_image`/`image_path` set, changed-image stale-copy delete, prune guard (off → survives; on + gone → pruned + copy deleted), `images_failed` in the log line (task 02);
|
||||
- `describe_image`: mock-client prompt shape (multimodal parts, chat model), cap, `None` on error/empty, the importer skip path (no row, no orphan, counter, warning) and the success path (content == summary == description; embedding called with the description only) (task 03);
|
||||
- the ext→mime map (task 04);
|
||||
- the source-frame `image_url` helper (present/absent) + the agent `read` marker + house-style frontend assertions (viewer `img` + alt + fallback; sources thumbnail fallback; chat sources inline image + alt) (task 05).
|
||||
2. `tests/integration/test_docs_api.py` (extend, task 04's cases) — the image route (200 + Content-Type per ext; 404 text doc / missing id / missing file), the content endpoint's `is_image`/`image_url` omission rules; `tests/integration/test_chat_api.py` (extend, task 05's case) — the SSE `image_url` frame; `tests/integration/` (new file `test_image_import.py` or the existing import test file — follow whichever exists) — `import_sources` end-to-end: `images=True` + mock vision → the fixture PNG becomes a doc (description content, `is_summary` chunk, one content chunk); `images=False` → ignored + a pre-seeded image doc survives prune; a failing mock → `images_failed == 1`, no row, other docs indexed.
|
||||
- Fixtures: a tiny valid PNG (a few bytes, generated in-test or a committed fixture under `tests/` — check the existing fixture conventions), a mock vision client (the existing mock-LLM test patterns in `tests/`).
|
||||
3. `tests/e2e/test_image_documents.py` (new — isolated run per AGENTS.md §4: `uv run pytest tests/e2e/test_image_documents.py -v --no-cov`). The suite's app instance runs with `BOR_IMAGES=true` (env override in the E2E fixture — the `conftest.py` pattern for per-suite app env):
|
||||
- upload a fixture PNG (the Sources-page upload flow or `page.request` against the upload endpoint, then trigger the sync through the UI as the Sources page does);
|
||||
- the Sources page lists the image doc (thumbnail or icon fallback);
|
||||
- open the document viewer → the image renders + the description text below it;
|
||||
- ask a question the mock LLM grounds on the image doc (the existing mock-LLM grounding pattern) → the chat's sources block shows the inline image with its caption;
|
||||
- negative: with the DEFAULT env (`BOR_IMAGES` unset/false), the same upload produces NO image doc (the default-off contract).
|
||||
4. Run the full gate: `uv run pytest`, `uv run pytest --cov=app --cov-report=term-missing` (TOTAL >90%), the isolated E2E file, `uv run ruff check . && uv run pyright`.
|
||||
|
||||
## Testing & Quality
|
||||
- This task IS the phase's test suite (see Work).
|
||||
- Coverage: **>90%** on `app/` — the phase's `app/` surface (config, importer, summarizer, docs API, chat frames, agent marker) is fully exercised.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] All test artifacts exist and pass; the isolated E2E file passes standalone.
|
||||
- [ ] The default-off byte-identity is asserted (unit + integration + the E2E negative case).
|
||||
- [ ] `uv run pytest --cov=app` TOTAL >90%; lint + types clean.
|
||||
@@ -0,0 +1,50 @@
|
||||
# Phase 123 — Chat image questions: attach an image to a question
|
||||
|
||||
**Source:** `TODO.md` L6 — "…users should be able to submit images as part of their question in brain of reese."
|
||||
**Story:** n/a (feature request; completes the phase-122 image capability on the question side).
|
||||
**Context:** Phase 122 (todo, this pipeline) — `BOR_IMAGES` toggle + `GET /api/config` `images` flag (task 01), the ext→mime map, `image_dir` storage convention. `app/schemas.py:60` — `ChatRequest` (`message` min 1/max 4000, `history` ≤100 — `HistoryTurn` is text-only), `ChatMessage` (L742, `extra="forbid"`, phase-83 value bounds). `app/api/chat.py` — the turn pipeline: the user message is built at L645 (`{"role": "user", "content": request.message}`; a grounded turn runs `run_agent`, a deflected turn a direct `chat_stream` on the same `messages`), and `app/rag/agent.py:1370/1448` — `run_agent(..., user_message: str)` builds its own `[system, user]` (verify the data flow — if `run_agent` receives the already-built `messages`, the single edit site is chat.py). The phase-114 SSE error-frame-with-hint pattern (the "question too long" frame — `ChatErrorEvent.detail` + optional `hint`, consumed by the banner at app.js L2210). `frontend/index.html` — the composer (label L287, `#message-input` L292, `#send-btn` L325). `frontend/assets/app.js` — `handleSend` (L2306), `runTurn` (the turn driver + the user append/save-point-1 at send), `addMessage("user", …)` (user bubble), `rememberBrainTurn` (L2108, the brain save point), `renderStoredMessage` (L1642, user branch). `frontend/assets/shared.js` — the shared page's message render (text-only today). `app/api/config.py:30` — the public flags dict (phase 122 task 01 added `images`).
|
||||
|
||||
## Objective
|
||||
The user attaches one image to a question: a masked-by-server upload stores the bytes, the vision model (the chat model) receives a multimodal message, the user's bubble renders the image, the record persists the image path (not base64) so refresh and shared chats render it, and `BOR_IMAGES=false` rejects the request with a helpful hint.
|
||||
|
||||
## Dependencies
|
||||
- `122_image_documents` (todo) — CODE dependency: the `BOR_IMAGES`/`images` config flag (the toggle gates this feature), the ext→mime map, and the `image_dir` storage convention (this phase's `chat_image_dir` follows it).
|
||||
- Code dependencies (all complete): phase 14/50/55 conversation persistence, phase 74 history mapping, phase 114 SSE error-hint frames, phase 51 shared chats.
|
||||
|
||||
## Design (shared by all tasks — the executor reads this, not the chat)
|
||||
- **Storage (task 01, LOCKED A5):** user question-images are server-stored, NOT base64-in-saved-chats: `Settings.chat_image_dir: str = "~/bor-sources/chat-images"` (`BOR_CHAT_IMAGE_DIR`, the `image_dir` convention — a sibling of phase 122's `image_dir`, separate because question-images are per-conversation, not per-source) + `Settings.chat_image_max_mb: int = 10` (`BOR_CHAT_IMAGE_MAX_MB`, the ~10 MB cap of A5; `upload_max_mb`'s fail-loud validator precedent for `<= 0`). `POST /api/chat-images` (multipart, in `app/api/chat.py` or a small new `app/api/chat_images.py` router — the executor's call, following the repo's one-concern-per-module style): accepts an image file, validates the mime/ext against the SAME six-extension set as phase 122 (reuse the frozenset; the Content-Type header is a hint — the EXTENSION is the source of truth, the archive-uploader precedent), rejects oversize with a 413 (the fixed-detail style), stores `chat_image_dir/<uuid4().hex>.<ext>`, returns `{ "path": "/api/chat-images/<uuid-hex>.<ext>" }`. `GET /api/chat-images/{filename}` serves the bytes (404 on missing/unknown — the filename is a uuid, no enumeration value) with the phase-122 mime map; PUBLIC like saved-chat content (a saved chat's id is already its credential — phase 55 A1 — the image is part of that content).
|
||||
- **Request (task 01):** `ChatRequest.image: str | None = Field(default=None, max_length=500)` — a STORED PATH, pattern-validated (`^/api/chat-images/[0-9a-fA-F]{32}\.(png|jpe?g|webp|gif|bmp)$` — the stored filename is `uuid4().hex.<ext>`) — never a raw data URL (the upload endpoint already did the size/mime enforcement; re-validating a 10 MB base64 string in the schema would be the anti-pattern). Toggle OFF (`settings.images` false) with `image` set → the turn settles with the phase-114 SSE error frame: `detail` "Image support is turned off on this server." + `hint` "Enable BOR_IMAGES in the server's .env (and restart) to ask with an image." (the question itself is NOT persisted — a rejected turn saves nothing, the existing error-path convention). `image` set but file missing → the same frame shape with a "that image is no longer available" detail (a stale-path edge: the stored file was deleted out-of-band).
|
||||
- **Multimodal (task 01):** the user message becomes `{"role": "user", "content": [{"type": "text", "text": request.message}, {"type": "image_url", "image_url": {"url": <data URL from the stored file>}}]}` at BOTH construction sites (chat.py:645 and agent.py:1448 if it builds independently — verify the flow; when `request.image` is None the content stays the plain string, byte-identical to today). The data URL is built server-side from the stored bytes + mime map (the phase-122 `describe_image` data-URL construction — reuse it). `HistoryTurn`/`history_to_messages` are UNCHANGED (LOCKED A7): prior turns' images are never replayed into the model's history — the history budget is text, and a 10 MB image per past turn would blow every budget; the model simply sees the text of a prior turn that had an image.
|
||||
- **Persistence (tasks 01+02):** `ChatMessage.image: str | None = Field(default=None, max_length=500)` — the stored path, on the USER record (the image belongs to the question). The user record is saved at save-point-1 (send), BEFORE the turn resolves — so the client uploads FIRST (`POST /api/chat-images`) and stores the returned path in the user record, then POSTs `/api/chat` with `image=<path>`. Saved chats, shared chats, and the localStorage shape all carry the path (≤500 chars — no phase-83 cap pressure). A brain record never carries `image` (the answer may reference the image's sources, but the attachment is the user's).
|
||||
- **Composer (task 02):** the attach control appears ONLY when `GET /api/config` says `images: true` (phase 122's flag; fetched once at boot like the other config — the composer reads the existing cached config if present). A paperclip button (SVG, the icon style of the other composer glyphs, `aria-label="Attach an image"`) before the input → hidden `<input type="file" accept="image/png,image/jpeg,image/webp,image/gif,image/bmp">` → on select: a preview strip above the input (thumbnail ≤48px, the filename, a remove ✕) + the file's data URL kept client-side until send; on send with an attachment: `POST /api/chat-images` (the file) → the returned path goes into the user record + the `/api/chat` body → the preview clears. Upload failure (oversize, non-image, server down) → the phase-114-style out-of-turn banner ("Couldn't attach the image — …") and the send is BLOCKED (no question without the image the user attached — ASSUMPTION A8, locked below). The user bubble renders the image (from the data URL live, from the stored path after restore) with `alt = filename`, capped height, above/beside the text (the theme's bubble treatment; the image is part of the question, visible in both the live bubble and the restore).
|
||||
- **Restore + shared (task 03):** `renderStoredMessage`'s user branch: `m.image` present → the user bubble includes `<img src="{m.image}" alt="…">` (a load failure collapses to a small "image unavailable" line — never a broken icon). The shared page (`shared.js`) renders the user image the same way (the image route is public — the shared view is faithful; no new shared-shape field beyond `ChatMessage.image`, which the public `messages` shape already carries).
|
||||
- **NOT touched:** the history budget/trimming, the honesty gate, the suggestion chips, phase-122's document-image pipeline (a QUESTION image is a separate concern — it is NOT indexed as a document), and the stop/failed-turn paths (they persist whatever records exist, including the new `image` key, unmodified).
|
||||
|
||||
## Tasks
|
||||
1. `01_vision_request.md` — `POST/GET /api/chat-images`, `ChatRequest.image` + `ChatMessage.image`, the toggle-off/stale error frames, the multimodal user message at both construction sites.
|
||||
2. `02_composer_attach.md` — the config-gated attach control, preview, upload-then-send, the user bubble's image.
|
||||
3. `03_restore_and_shared.md` — `ChatMessage.image` on restore (chat page) and on the shared page.
|
||||
4. `04_chat_image_tests.md` — unit + integration + isolated E2E `test_chat_image_questions.py`.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: `tests/unit/test_chat_image_questions.py` (new, task 04) — the path pattern validator (accepts well-formed, rejects data URLs / wrong ext / traversal), the multimodal message build (both sites; `image=None` → byte-identical plain string), the toggle-off + stale-file error frames (detail + hint shapes), the upload endpoint's mime/size/ext rules (tmp-dir settings), the serve route (200/404), `ChatMessage.image` bounds + omission.
|
||||
- Integration: `tests/integration/test_chat_api.py` (extend, task 04) — upload → `POST /api/chat` with `image=<path>` → the mock client RECEIVES the multimodal content list (text part + image_url data URL); `image` with `images=false` → the SSE error frame with the hint and NO model call, no persisted record; `image=None` requests are byte-identical to pre-phase; a saved chat round-trips a user record with `image`; a shared chat serves it.
|
||||
- E2E: `tests/e2e/test_chat_image_questions.py` (new, task 04) — isolated run per AGENTS.md §4, `BOR_IMAGES=true`: attach a fixture PNG in the composer → preview + remove works → send → the user bubble shows the image → the (mock) answer streams → reload → the user bubble restores WITH its image → open the shared link → the shared page shows the image. Plus the default-off negative: with `BOR_IMAGES` unset, the attach control is ABSENT from the DOM.
|
||||
- Coverage: **>90%** on `app/` (validate.sh gate).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] With `BOR_IMAGES=true`: attach → send → the vision model gets text+image; the user bubble, the refreshed page, and the shared chat all show the image; the saved chat stores the PATH (assert no base64 in the stored payload).
|
||||
- [ ] With `BOR_IMAGES=false`: the attach control is absent, an API request with `image` gets the hinted error frame, and no model call / record happens.
|
||||
- [ ] Text-only questions behave byte-identically to pre-phase (the multimodal branch is inert).
|
||||
- [ ] `uv run pytest` green; `uv run pytest --cov=app --cov-report=term-missing` TOTAL >90%; `uv run ruff check . && uv run pyright` clean.
|
||||
- [ ] One `--no-gpg-sign` commit; phase dir moved to `.agents/phases/complete/` by the pipeline gate.
|
||||
|
||||
## Locked decisions
|
||||
- **A5 — one image per question; the ~10 MB cap (`BOR_CHAT_IMAGE_MAX_MB`); server-stored bytes under `chat_image_dir`; the saved/shared record carries the path, never base64 (owner-confirmed 2026-09-24, roadmap confirmation).**
|
||||
- **A7 — a question's image applies to the CURRENT turn only; prior turns' images are never replayed into the model's history (the text of a prior turn stands alone) (owner-confirmed: same confirmation — the proposed design).**
|
||||
- **A8 — if the image upload fails, the send is blocked with a banner (the question is never sent without the image the user attached) (owner-confirmed: same confirmation).**
|
||||
|
||||
## Commit
|
||||
```bash
|
||||
git add app/ frontend/ tests/ .env.example .agents/phases/ && git commit --no-gpg-sign -m "feat(chat): attach an image to a question — vision input, in-bubble render, persisted and shared"
|
||||
```
|
||||
@@ -0,0 +1,31 @@
|
||||
# Task 01 — Vision request: upload/serve endpoints, request + message schemas, multimodal build
|
||||
|
||||
**Phase:** `123_chat_image_questions` · **Source:** `TODO.md:6` — "…users should be able to submit images as part of their question in brain of reese."
|
||||
|
||||
## Objective
|
||||
The server side of the image question: a uuid-named upload/serve pair for question images, `ChatRequest.image` (stored path) + `ChatMessage.image` (persistence), the toggle-off/stale error frames, and the multimodal user message at both construction sites — with text-only requests byte-identical to pre-phase.
|
||||
|
||||
## Work
|
||||
1. `app/config.py` — `chat_image_dir: str = "~/bor-sources/chat-images"` (`BOR_CHAT_IMAGE_DIR`, the phase-122 `image_dir` convention) + `chat_image_max_mb: int = 10` (`BOR_CHAT_IMAGE_MAX_MB`, the A5 cap; the `upload_max_mb` fail-loud `<= 0` validator precedent). `.env.example` entries.
|
||||
2. New router (a small `app/api/chat_images.py`, registered in `app/main.py` next to the chat router — the one-concern-per-module house style):
|
||||
- `POST /api/chat-images` — `UploadFile` (the git-sources upload endpoint L432 pattern): extension must be in the phase-122 image frozenset (the EXTENSION is the source of truth — a Content-Type header is a hint); total bytes capped at `chat_image_max_mb` (stream-count the bytes — reject with 413 + a fixed detail that names the cap, never echoing the filename); store `chat_image_dir/<uuid4().hex>.<ext>` (dir created on demand); response `{"path": "/api/chat-images/<uuid>.<ext>"}`.
|
||||
- `GET /api/chat-images/{filename}` — filename must be `<uuid-hex>.<ext>` (the regex guard → 404 otherwise, no path traversal by construction); 404 on missing file; serve the bytes with the phase-122 ext→mime map + `Cache-Control: private, max-age=3600` (public, like saved-chat content — phase 55 A1).
|
||||
3. `app/schemas.py`:
|
||||
- `ChatRequest` (L60): `image: str | None = Field(default=None, max_length=500)` + a `field_validator` — when set, it must match `^/api/chat-images/[0-9a-fA-F]{32}\.(png|jpe?g|webp|gif|bmp)$` (the uuid4().hex shape — 32 hex chars; adjust if the uuid format differs) with a fixed 422 detail ("image must be an uploaded chat image path" — no echo). Docstring: the path from `POST /api/chat-images` (task 01) — never a data URL; the upload endpoint owns size/mime enforcement.
|
||||
- `ChatMessage` (L742, `extra="forbid"`): `image: str | None = Field(default=None, max_length=500)` — on the USER record only (the question's attachment); the docstring notes brain records never carry it and the saved/shared shape therefore gains one optional key (omitted when None — the phase-50 byte-identical contract for text-only chats holds).
|
||||
4. `app/api/chat.py` — the turn pipeline:
|
||||
- pre-stream validation (BEFORE any model call, at the top of the turn handler): `request.image` set → `settings.images` false → yield the phase-114 error frame (`ChatErrorEvent(detail="Image support is turned off on this server.", hint="Enable BOR_IMAGES in the server's .env (and restart) to ask with an image.")`) and return (NO model call, NO record — the existing error-path convention); file missing on disk → the same frame shape, `detail="That image is no longer available."` + a generic reachability-free hint (or no hint — the banner's default is fine).
|
||||
- the user message (L645): when `request.image` is set, `{"role": "user", "content": [{"type": "text", "text": request.message}, {"type": "image_url", "image_url": {"url": <data URL>}}]}` — the data URL built from the stored bytes + the phase-122 mime map (REUSE the data-URL construction from `describe_image` — factor it to a shared helper if it is buried in `app/rag/summarizer.py`); `request.image` None → the plain-string content, byte-identical.
|
||||
- `app/rag/agent.py` — verify the data flow: if `run_agent` (L1370) receives the already-built `messages` from chat.py, NO change here (the L1448 build is for a different entry); if it builds its own user message from `user_message`, extend `run_agent`'s signature (`user_message: str | list | None` — pyright-clean) and make chat.py pass the multimodal content. Pin the chosen flow in a code comment.
|
||||
- a QUESTION image is NEVER indexed as a document (no importer call) — it is turn-local storage.
|
||||
5. ASSUMPTION (A7 re-stated): `HistoryTurn`/`history_to_messages` unchanged — prior turns' images are not replayed (text-only history stands).
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: `tests/unit/test_chat_image_questions.py` (task 04 finalizes) — the path validator (well-formed ok; a data URL, a wrong ext, a traversal, and a 31-hex-char uuid all 422); the multimodal builder (both sites; None → plain string); the error frames' exact detail/hint strings; the upload endpoint's ext/size rules (tmp `chat_image_dir`); the serve route 200/404 + Content-Type; `ChatMessage.image` (bounds, omission, the `extra="forbid"` boundary intact).
|
||||
- Integration: `tests/integration/test_chat_api.py` (task 04) — the upload → chat flow asserts the MOCK client received the multimodal content list; the toggle-off frame + no model call; text-only byte-identity; saved + shared round-trips with `image`.
|
||||
- Coverage: **>90%** on the touched modules.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `POST /api/chat-images` stores a uuid-named file and returns its path; `GET` serves it; oversize/non-image → 413/422 with fixed details.
|
||||
- [ ] `POST /api/chat` with `image` (toggle on) delivers a multimodal user message to the model; toggle off → the hinted error frame, no model call; `image=None` → byte-identical behavior.
|
||||
- [ ] `uv run pytest` green; `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -0,0 +1,38 @@
|
||||
# Task 02 — Composer: attach control, preview, upload-then-send, the user bubble's image
|
||||
|
||||
**Phase:** `123_chat_image_questions` · **Source:** `TODO.md:6` — "…users should be able to submit images as part of their question in brain of reese."
|
||||
|
||||
## Objective
|
||||
The composer (when `GET /api/config` says `images: true`) takes one attached image — preview + remove before send, upload on send, the image in the user's bubble — and the user's conversation record carries the stored `image` path.
|
||||
|
||||
## Work
|
||||
1. `frontend/index.html` — the composer (the label L287 / `#message-input` L292 / `#send-btn` L325 region): before the input, the attach control
|
||||
```html
|
||||
<button type="button" class="attach-btn" id="attach-btn" hidden aria-label="Attach an image">
|
||||
<svg aria-hidden="true" …paperclip glyph, the icon style of the other composer glyphs…></svg>
|
||||
</button>
|
||||
<input type="file" id="attach-file" accept="image/png,image/jpeg,image/webp,image/gif,image/bmp" hidden>
|
||||
```
|
||||
plus the preview strip container (after the input row, `#attach-preview`, `hidden` by default — a thumbnail ≤48px + filename + a remove ✕ button `#attach-remove`). `#attach-btn` is `hidden` by default — JS reveals it only when the config flag is on (task 02 step 3); the hidden-by-default markup keeps the flag-off DOM byte-identical (A5's default-off contract).
|
||||
2. `frontend/assets/app.js`:
|
||||
- boot: read `images` from the `GET /api/config` fetch (the composer already consumes the cached whoami/config boot — extend that fetch's result use; ONE request, no extra round-trip) → `attachBtn.hidden = !images`.
|
||||
- attach flow: `#attach-btn` click → `attachFile.click()`; on change: validate the file's extension against the six (client-side pre-check, the server re-validates — a bad pick → the out-of-turn banner "Only PNG, JPEG, WebP, GIF, and BMP images can be attached." and no state change); keep `{ file, dataUrl (for the live preview) }` in a turn-local `attachedImage` var; show `#attach-preview` (thumbnail from the data URL, the filename, the remove ✕); the remove ✕ (or a new selection) clears the state + hides the strip.
|
||||
- send flow (`handleSend` L2306 / `runTurn`): when `attachedImage` is set:
|
||||
1. `POST /api/chat-images` (the File) — on failure (413/422/5xx) → the phase-114-style out-of-turn banner with the server's detail ("Couldn't attach the image — try again.") and the send is BLOCKED (LOCKED A8 — the question is never sent without its image; the input text stays).
|
||||
2. success → `runTurn(text, { image: <returned path> })`; `runTurn`'s user append (save point 1 — the user record) stores `{ who: "user", text, image: <path> }` (the `image` key joins the `bor.chat.v1` record — the phase-14 shape gains the optional key; `saveConversation()` + the phase-55 auto-save ride the existing path);
|
||||
3. the USER bubble renders the image: extend `addMessage("user", text)` with an optional `image` arg (data URL live, path after restore) → `<img src alt={filename}>` in the bubble (capped height ~240px, `max-width: 100%`, the theme's bubble treatment, `loading="lazy"`);
|
||||
4. clear `attachedImage` + the preview strip AFTER the user bubble is rendered (the strip must not linger into the turn).
|
||||
- text-only sends: `attachedImage` null → the request body omits `image`, the user record omits the key, the bubble is byte-identical to pre-phase.
|
||||
3. `frontend/assets/styles.css` — `.attach-btn` (the composer glyph button treatment — match the send-btn family, focus-visible ring per PLAN §7), `#attach-preview` (the strip: flex row, thumbnail box, filename ellipsis, the ✕), the user-bubble image block.
|
||||
4. ASSUMPTION (A8 re-stated): an upload failure blocks the send (no partial question-without-image) — the banner tells the user what failed; the typed question is preserved.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: `tests/unit/test_chat_image_questions.py` (task 04) — house-style source assertions: `#attach-btn` is `hidden` by default + `aria-label`; the reveal is gated on the config `images` flag; the extension pre-check list matches the server's six; the send path uploads BEFORE `runTurn` and blocks on failure (the A8 ordering); the user record gains `image` only when attached; the user bubble renders the `img` with `alt`.
|
||||
- E2E: `tests/e2e/test_chat_image_questions.py` (task 04) — the composer scenarios.
|
||||
- Coverage: n/a (frontend) — the validate.sh `app/` gate must stay green.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] Flag on: attach → preview → remove all work; send with an attachment uploads, the user bubble shows the image, and the question reaches the model.
|
||||
- [ ] Flag off: the attach button is ABSENT from the DOM; a hand-crafted `image` request still gets the server's error frame (task 01's contract, unchanged).
|
||||
- [ ] A text-only send produces the same request body and DOM as pre-phase (byte-check in the E2E where practical).
|
||||
- [ ] `uv run pytest` green; `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -0,0 +1,26 @@
|
||||
# Task 03 — Restore + shared: the question's image survives a refresh and a share link
|
||||
|
||||
**Phase:** `123_chat_image_questions` · **Source:** `TODO.md:6` — "…users should be able to submit images as part of their question in brain of reese."
|
||||
|
||||
## Objective
|
||||
A user record carrying `image` (the stored path) renders its image when the chat is restored from localStorage / the saved-chat API, and on the shared-chat page — a load failure degrades to a small note, never a broken icon.
|
||||
|
||||
## Work
|
||||
1. `frontend/assets/app.js` — `renderStoredMessage(m)` (L1642), the USER branch: when `m.image` is present, the restored user bubble includes `<img src="{m.image}" alt="{m.text || 'attached image'}">` through the SAME bubble-image helper task 02 built for the live bubble (one renderer — the live bubble passes the data URL, restore passes the path; the helper takes any `src`). `onerror` → replace the image with a small "image unavailable" line (the file was deleted out-of-band — the row keeps its path, the render degrades).
|
||||
- The restore paths that call `renderStoredMessage` (the localStorage restore ~L1700 and the saved-chat restore ~L1770) need NO other change — the record's `image` key flows through the phase-14/50 restore as any optional key.
|
||||
- `retryLastTurn` / regenerate: a RE-ASK of a question that had an image does NOT re-attach the image (the redo re-sends `prev.text` only — LOCKED A7, the image is turn-local to the original send; the restored image stays visible in the replaced record until the redo pops it, which is the existing redo-in-place behavior).
|
||||
2. `frontend/assets/shared.js` — the shared page's message render (its text-only loop over `messages`): the user-record branch gains the same image render (the image route is public — a shared chat is faithful; the `alt` + `onerror` degradation are identical to the chat page).
|
||||
3. `frontend/assets/styles.css` — no new rules beyond what task 02 added (the shared page reuses the bubble-image block; verify the shared page's bubble class shares it — if the shared page uses a different bubble class, scope the image rule to both).
|
||||
4. ASSUMPTION: the `image` key is optional and absent in every pre-phase saved chat — no data migration, no backfill (old chats have no question-images to restore).
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: `tests/unit/test_chat_image_questions.py` (task 04) — house-style source assertions: the user-branch render reads `m.image` and reuses the bubble-image helper; the `onerror` degradation exists on BOTH pages; the shared render includes the image; the redo path sends `prev.text` only (no `image` on the re-ask).
|
||||
- Integration: `tests/integration/test_chats_api.py` (extend, task 04) — a user record with `image` round-trips `POST`/`PUT /api/chats` and serves through `GET /api/shared/{token}` (the public shape carries it).
|
||||
- E2E: the refresh + shared scenarios of `tests/e2e/test_chat_image_questions.py` (task 04).
|
||||
- Coverage: n/a (frontend) — the validate.sh `app/` gate must stay green.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] Reload after an image question: the user bubble shows the image (from the stored path) + the rest of the conversation is unchanged.
|
||||
- [ ] The shared link renders the image on the shared page.
|
||||
- [ ] A deleted image file degrades to the "image unavailable" line on both pages (no broken-image icon).
|
||||
- [ ] `uv run pytest` green; `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -0,0 +1,39 @@
|
||||
# Task 04 — Chat-image tests: unit + integration + isolated E2E
|
||||
|
||||
**Phase:** `123_chat_image_questions` · **Source:** `TODO.md:6` — "…users should be able to submit images as part of their question in brain of reese."
|
||||
|
||||
## Objective
|
||||
Pin the whole question-image contract: the upload/serve rules, the multimodal model payload, the toggle-off rejection, the persistence shape (path, never base64), the composer gating, and the refresh/share rendering — as an isolated Playwright suite.
|
||||
|
||||
## Work
|
||||
1. `tests/unit/test_chat_image_questions.py` (new) — consolidates the per-task unit cases (tasks ship code; this task ships the full pin):
|
||||
- config: `chat_image_dir`/`chat_image_max_mb` defaults + env overrides;
|
||||
- the `ChatRequest.image` validator (well-formed path ok; data URL / wrong ext / traversal / malformed uuid → 422, fixed details, no echo);
|
||||
- the multimodal builder (text part + `image_url` data-URL part, correct mime; `image=None` → the plain string, byte-identical); the data-URL helper is shared with `describe_image` (assert the import, not a copy);
|
||||
- the error frames: toggle-off (exact detail + hint strings), stale file (exact detail) — both settle the turn WITHOUT a model call (the mock client must see zero calls);
|
||||
- the upload endpoint: the six exts accepted, others 422/413-style per the spec, oversize → 413 (fixed detail naming the cap), the stored filename is `<uuid-hex>.<ext>`;
|
||||
- the serve route: 200 + Content-Type per ext, 404 for missing/unknown/traversal filenames;
|
||||
- `ChatMessage.image` (max 500, omission when None, `extra="forbid"` intact — an unknown key still 422s);
|
||||
- frontend source assertions (tasks 02+03): the attach button hidden-by-default + config-gated reveal, the A8 upload-before-send ordering + block-on-failure, the user record's `image` key, the shared bubble render, the `onerror` degradation, the redo sends text-only.
|
||||
2. `tests/integration/test_chat_api.py` (extend, task 01's cases):
|
||||
- `POST /api/chat-images` → `POST /api/chat` with the returned path → the MOCK client received the multimodal content list (text == the question, data URL decodes to the uploaded bytes);
|
||||
- `images=false` + `image` → the SSE error frame with the hint; the mock client got NO call; NO saved record;
|
||||
- `image=None` → the model payload is byte-identical to a pre-phase request;
|
||||
- a saved chat (and a shared one) round-trips a user record with `image` — and assert the stored payload contains NO base64 (the path only — the A5 contract).
|
||||
3. `tests/integration/test_chats_api.py` (extend, task 03's case): the shared-chat serve includes the user record's `image` path.
|
||||
4. `tests/e2e/test_chat_image_questions.py` (new — isolated run per AGENTS.md §4: `uv run pytest tests/e2e/test_chat_image_questions.py -v --no-cov`), `BOR_IMAGES=true` for this suite's app instance (the phase-122 E2E env-override pattern):
|
||||
- attach a fixture PNG in the composer → the preview strip shows (thumbnail + filename) → remove → the strip clears and the file state is gone;
|
||||
- re-attach → send → the user bubble shows the image; the mock LLM's (text-only) answer streams normally (the mock ignores the image part — the assertion is on the REQUEST the server built, verified via the mock's capture);
|
||||
- `page.reload()` → the user bubble restores WITH its image (the stored path, not the data URL — the request count for the image route confirms the path fetch);
|
||||
- share the chat (the existing share flow) → open the shared link → the shared page shows the user's image;
|
||||
- default-off negative (a second app instance or the suite's flag-off fixture): `#attach-btn` is ABSENT from the DOM; a direct `POST /api/chat` with an `image` path returns the hinted error frame (no model call).
|
||||
5. Run the full gate: `uv run pytest`, `uv run pytest --cov=app --cov-report=term-missing` (TOTAL >90%), the isolated E2E file, `uv run ruff check . && uv run pyright`.
|
||||
|
||||
## Testing & Quality
|
||||
- This task IS the phase's test suite (see Work).
|
||||
- Coverage: **>90%** on `app/` — the phase's `app/` surface (config, the chat-images router, the chat pipeline, the schemas) is fully exercised.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] All test artifacts exist and pass; the isolated E2E file passes standalone.
|
||||
- [ ] The multimodal payload, the no-base64-in-storage, and the toggle-off rejection are each asserted at the unit AND integration level.
|
||||
- [ ] `uv run pytest --cov=app` TOTAL >90%; lint + types clean.
|
||||
@@ -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`.
|
||||
@@ -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
|
||||
+17
@@ -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`.
|
||||
+104
@@ -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
|
||||
+21
@@ -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`
|
||||
+104
@@ -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
|
||||
+16
@@ -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`.
|
||||
+104
@@ -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
|
||||
+19
-2
@@ -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 ``<b>``-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: <detail or fallback>, failed: true,
|
||||
error: <detail>}`` — 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):
|
||||
|
||||
+198
-10
@@ -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 =
|
||||
'<svg aria-hidden="true" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><path d="m21.73 18-8-14a2 2 0 0 0-3.48 0l-8 14A2 2 0 0 0 4 21h16a2 2 0 0 0 1.73-3Z"/><path d="M12 9v4"/><path d="M12 17h.01"/></svg>';
|
||||
|
||||
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;
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 <source/path>:\n<content>"``.
|
||||
_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 ``<tools>`` marker, so both
|
||||
|
||||
@@ -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)
|
||||
@@ -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)
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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/<token>`` 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"]
|
||||
|
||||
|
||||
@@ -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: <detail or fallback>, failed: true, error:
|
||||
<detail>}`` — 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,
|
||||
}
|
||||
@@ -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)
|
||||
@@ -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")
|
||||
|
||||
@@ -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]
|
||||
|
||||
Reference in New Issue
Block a user