Files
brain-of-reese/tests/unit/test_chat_persistence.py
T
ducoterra ffa919b8bf fix(chat): keep in-flight answers alive across in-app view switches
Root cause (owner repro, verified in a real browser 2026-09-06): the
five navbar views (Chat, RAG, Sources, Tuning, History) were separate
HTML documents, so a navbar click was a REAL cross-document navigation
— the chat page unloaded, the in-flight SSE fetch was aborted, and the
phase-48 teardown (app/api/chat.py `finally`, "chat: turn cancelled")
stopped the model. Observed: send question -> click RAG mid-stream ->
click Chat -> the answer never finished: no `query_log` row, and on
return a dangling question with no brain record (the pre-token pagehide
partial persist skips because `acc` is empty).

Phase-48 LOCKED-DECISION REFINEMENT (owner-confirmed 2026-09-06,
flagged per AGENTS.md rule 3, not silently deviated): "real navigation
cancels the fetch" now means LEAVING THE APP — tab close,
external/other-document navigation, the Stop button. In-app navbar
switches are client-side view switches and no longer cancel.

Fix — Option A (SPA shell), chosen over B (Service Worker owns the
stream) and C (server-side turn registry + resume):
- frontend/index.html is the shell: ONE `<main id="main">` holds the
  five `<section class="view">` blocks; hidden views carry BOTH
  `hidden` and `inert` (WCAG — no focus/keyboard traversal). The
  shared header, the single `doc-modal-*` skeleton, and the
  `#app-version` footer each exist exactly once; the per-view copies
  from the four folded pages are dropped.
- New frontend/assets/router.js (vanilla module — no framework, no
  bundler, No-CDN rule intact): lazy-imports a view module on FIRST
  show only (mount-once, hide-forever — the chat view's in-flight SSE
  reader persists across switches; that persistence IS the fix);
  intercepts same-shell navbar links with preventDefault +
  history.pushState (never a document load); handles popstate; single
  writer of `.nav-link` active state (is-active + aria-current),
  document.title, and the per-view meta description (values carried
  over from the old pages' heads, brand-resolved at write time).
- Each folded page's JS becomes `export async function mount(root)` —
  root-scoped queries; `initSharedHeader()` dropped (the header boots
  once in the shell via the chat module; the admin flag comes from the
  same cached `fetchIsAdmin()` promise — zero extra requests).
- app/main.py: a small list-driven route factory serves the shell for
  /tuning.html, /sources.html, /git-sources.html, /history.html —
  registered AFTER the API routers and BEFORE the static catch-all
  (routes-first). The phase-33 caching middleware applies no-cache +
  `?v=` rewriting unchanged; app/core/caching.py needed NO change
  (the view paths did not change — pinned by the integration tests).
- The four old view .html files are DELETED (one source of truth);
  deep links to the old URLs keep working (the router picks the view
  from the pathname); `/?chat=<id>` is unaffected; the Containerfile
  bundles router.js (inlining the lazy view modules) and drops the
  folded page files.
- app/schemas.py: HistoryTurn.text cap 4000 -> 32000 — the shell
  keeps long saved answers in the chat, and the old cap (stricter than
  the 24_000-char total history budget) 422-rejected any second turn
  in such a chat (found by the phase-42 E2E suite on the shell).

Boundaries: login.html, shared.html, doc-edit.html, document.html
REMAIN separate documents (flow pages, not navbar tabs); a mid-stream
navigation to doc-edit/document.html still cancels per phase 48
(follow-up candidate, out of scope). The SSE API is unchanged. Real
departures still cancel the turn — phase 48 intact (pinned by
tests/e2e/test_stop_generation.py, unchanged, and by the new suite's
real-departure control).

Tests:
- Phase-20 suite REWRITTEN to the new semantics
  (tests/e2e/test_sources_midstream_bug.py): a navbar switch no longer
  cancels — the stream survives the switch and the FULL answer
  settles; the pagehide partial persist REMAINS for real departures
  (the partial's exact shape — first streamed chunk prefix, no done
  metadata — is still pinned there).
- NEW story suite tests/e2e/test_nav_switch_keeps_stream.py (mock
  LLM): the owner repro (send -> RAG mid-stream -> Chat: window
  sentinel survives = same document, FULL answer, exactly one brain
  turn in bor.chat.v1, exactly one settled query_log row, auto-saved
  row matches) + the same mid-stream switch against the other three
  views + the real-departure-still-cancels control + the no-switch
  baseline.
- tests/unit/test_frontend_router.py: source-level pins of the router
  invariants (click interceptor targets ONLY same-shell view paths,
  pushState-only switches, mount-once guard, hidden+inert pair,
  single-writer active state/title); shell-route integration tests
  (each folded path serves the shell with no-cache + `?v=` body; a
  non-view path still 404s); the file-reading unit pins re-pointed at
  the shell (the four view files are gone — the shell is the source
  of truth).

Verification (this commit): full suite green — 1565 unit+integration
tests, app/ coverage 99% (>90% floor); ruff + pyright clean; the
phase's E2E suites green in isolation (house protocol, AGENTS.md rule
9). Owner repro verified in a real browser against the real LLM
(dev server :8010, headful Chromium): "tell me about everquest" ->
RAG mid-stream -> Chat — the answer completed with one brain bubble
and no error banner, `query_log` gained exactly one settled row
(deflected=True: the dev KB holds no EverQuest docs — the settle, not
the topic, is the proof), zero "chat: turn cancelled" lines for that
turn; the control (real navigation to /shared.html mid-stream) still
cancelled (no settled row, the cancel line logged, the partial
persisted on return). Screenshots: .agents/screenshots/76_manual_*.

Phase 76 (76_spa_nav_shell) complete — moved to
.agents/phases/complete/.
2026-09-06 06:31:31 -04:00

320 lines
15 KiB
Python

"""Unit: the chat-persistence contract in the static frontend (phase 14).
The browser behavior itself is E2E-covered (tests/e2e/test_chat_persistence.py);
here we pin the localStorage persistence markers in app.js/index.html/
styles.css so a silent regression (key rename, dropped try/catch, missing
restore, New chat control lost) is caught without a browser.
Pinned design (PLAN §7.4 note / phase 14):
* versioned key ``bor.chat.v1`` → ``{v: 1, chatId: string | null,
messages: [...]}`` (phase 55 A2: the shape extends IN PLACE with the
saved_chats row link — a pre-55 record without it reads as null),
raw text only;
* save points: user message on send, brain message on ``done``;
* every ``localStorage`` access wrapped in try/catch (failure-safe);
* size budget ~700k chars, oldest dropped first;
* ``#new-chat-btn`` chat-page only — phase 19 shipped it in the shared
bar (chat, sources, viewer, owner permission 2026-08-23); it moved
from the navbar to index.html's .chat-shell at owner request
(2026-08-28). ≥44px, solid brand pill.
"""
from __future__ import annotations
import re
from pathlib import Path
FRONTEND = Path(__file__).resolve().parents[2] / "frontend"
APP_JS = FRONTEND / "assets" / "app.js"
INDEX_HTML = FRONTEND / "index.html"
DOCUMENT_HTML = FRONTEND / "document.html"
LOGIN_HTML = FRONTEND / "login.html"
SHARED_HTML = FRONTEND / "shared.html"
DOC_EDIT_HTML = FRONTEND / "doc-edit.html"
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 _index() -> str:
return INDEX_HTML.read_text(encoding="utf-8")
def test_versioned_storage_key_and_v1_payload() -> None:
"""`bor.chat.v1` (versioned — a format bump is a clean start) with the
{v, chatId, messages} payload shape (A11: raw localStorage JSON, no
library). Phase 55 (A2): the shape extends IN PLACE with `chatId` —
the saved_chats row link (null when unlinked), so a reload restores
the conversation AND its link; the trimToBudget size probe measures
the same shape."""
js = _js()
assert 'const STORAGE_KEY = "bor.chat.v1"' in js
assert "export const STORAGE_VERSION = 1" in js
# The write carries the version, the row link, and the trimmed
# messages (the single write path — saveConversation).
save_start = js.find("function saveConversation")
save_body = js[save_start : js.find("\n}\n", save_start)]
assert "v: STORAGE_VERSION" in save_body
assert "chatId: currentChatId" in save_body, (
"phase 55: the link is written with the record (null when unlinked)"
)
assert "trimToBudget(conversation)" in save_body
# The size probe measures the same shape (the link is a fixed-length
# field — null stands in for the size estimate).
probe_start = js.find("function trimToBudget")
probe = js[probe_start : js.find("\n}\n", probe_start)]
assert "v: STORAGE_VERSION" in probe and "chatId: null" in probe
# Restore validates the version before trusting anything.
assert "data.v !== STORAGE_VERSION" in js
def test_storage_size_budget_drops_oldest_first() -> None:
"""~700k-char serialized budget (far under the ~5MB quota); the loop
drops messages from the FRONT (oldest) until the state fits."""
js = _js()
assert "export const STORAGE_BUDGET_CHARS = 700_000" in js
assert "out.length <= 1" in js, "never drop the last remaining message"
assert "out = out.slice(1)" in js, "oldest-first drop (slice(1), not pop)"
assert "STORAGE_BUDGET_CHARS" in js
def test_every_storage_access_is_failure_safe() -> None:
"""AC4: every localStorage access (getItem/setItem/removeItem) must be
inside a try/ that is closer than the enclosing function boundary —
private mode or quota exhaustion must never throw into the UI."""
js = _js()
accesses = list(re.finditer(r"localStorage\.(?:getItem|setItem|removeItem)", js))
assert len(accesses) == 3, f"expected exactly 3 localStorage accesses, got {len(accesses)}"
for m in accesses:
try_idx = js.rfind("try {", 0, m.start())
fn_idx = js.rfind("function ", 0, m.start())
assert try_idx != -1, f"no try before {m.group(0)!r}"
assert try_idx > fn_idx, (
f"{m.group(0)!r} is not inside its function's try block "
f"(function boundary at {fn_idx} is after try at {try_idx})"
)
# Each access has its own catch that degrades silently.
assert js.count("} catch {") >= len(accesses)
def test_raw_text_only_stored_and_re_rendered_on_restore() -> None:
"""The value is raw text (re-rendered through the escape-first markdown
on restore) — no HTML is ever stored. Restore re-applies the full
brain-message chrome: is-deflected styling, maybe-try chips, sources."""
js = _js()
# Phase 42 (owner direction 2026-08-27): the reply autoscroll is gone;
# restore landings keep their one-shot load-time scroll via the
# explicit intent (scroll=true) — the new addMessage signature has no
# per-call behavior override (default SCROLL instead of forced
# "auto" — documented at the call site).
assert 'addMessage("user", renderMarkdown(m.text), true)' in js
assert 'addMessage("brain", renderMarkdown(m.text), true)' in js
assert "wrap.classList.add(\"is-deflected\")" in js
assert "appendMaybeTry(wrap, m.suggestions)" in js
assert "appendSources(wrap, m.sources)" in js
# Restore runs on load (module scope, after the handlers are wired).
assert "restoreConversation();" in js
# Corrupt/legacy payloads degrade to a clean start, never a crash.
assert "Array.isArray(data.messages)" in js
def test_save_points_user_on_send_and_brain_on_done() -> None:
"""Save points: the user message is stored the moment it is sent (BEFORE
the fetch — a failed turn keeps the question); the brain message is
stored on `done` with the done metadata (sources/deflected/suggestions)."""
js = _js()
user_push = js.find('conversation.push({ who: "user", text })')
assert user_push != -1
assert user_push < js.find('fetch("/api/chat"'), (
"the user message must be saved before the turn starts"
)
# Brain save point is wired into the done handler with full metadata
# (phase 17: the persisted text is finalText — the empty-answer
# fallback substitution — and the optional thinking field rides along
# in the same meta object).
done_idx = js.find('ev.type === "done"')
assert done_idx != -1
# Window: the whole done branch (up to the error branch) — the meta
# object legitimately grows with phases (phase 17: thinking, phase
# 37: tools), so a fixed char offset would false-fail.
done_block = js[done_idx : js.find('ev.type === "error"')]
assert "rememberBrainTurn(finalText || acc" in done_block
assert "thinking: thinkingAcc || undefined" in done_block
assert "deflected: !!ev.deflected" in done_block
assert "sources: ev.sources" in done_block
assert "suggestions: ev.suggestions" in done_block
# rememberBrainTurn stores raw text and saves immediately.
assert "text: rawText ||" in js
body = js[js.find("function rememberBrainTurn") :]
assert "saveConversation()" in body[: body.find("\n}\n") + 3]
def test_new_chat_clears_key_and_ui() -> None:
"""New chat: clears the stored key, the rendered list, restores the
empty state, and reuses the #send-status live region for the
confirmation. A live turn is never hijacked."""
js = _js()
fn_start = js.find("function startNewChat")
assert fn_start != -1
body = js[fn_start : js.find("\n}\n", fn_start)]
assert "clearStoredConversation()" in body
assert 'querySelectorAll(".msg")' in body
assert "emptyState.hidden = false" in body
assert "setUiState(UI_STATE.idle)" in body
assert "sendStatus.textContent" in body, "confirmation via the live region"
assert "UI_STATE.thinking" in body and "UI_STATE.streaming" in body, (
"new chat must be ignored while a turn is in flight"
)
assert "removeItem(STORAGE_KEY)" in js
def test_new_chat_button_lives_only_on_the_chat_page() -> None:
"""#new-chat-btn is a real type=button with an accessible name. Moved
from the shared header bar to the chat page at owner request
(2026-08-28): it lives ONLY in index.html — inside <main>, in
.chat-shell. Phase 65 (2026-09-01, `TODO.md` L3, owner confirmation)
relocated the .chat-actions row from the top of the column to the
bottom: the button now sits BELOW the #messages section, directly
above the composer (the single module binding + the no-op guard are
pinned in test_shared_header.py).
Phase 76 (task 01) — the final shell form, converted in ONE step so
the later fold tasks (02/03) leave this pin alone: the file-level
"only on the chat page" semantics die with the shell — the button
must sit inside the shell's CHAT VIEW section (#view-chat; the
hidden views' elements remain in the DOM, so a per-file negative
check is meaningless inside the shell), and it must be absent from
the SURVIVING separate documents (the flow pages that are not
navbar views)."""
html = _index()
btn = re.search(r'<button[^>]*id="new-chat-btn"[^>]*>', html)
assert btn, "index.html must contain #new-chat-btn"
tag = btn.group(0)
assert 'type="button"' in tag
assert 'aria-label="New chat"' in tag
main_idx = html.find('main id="main"')
assert main_idx != -1 and btn.start() > main_idx, "the button belongs inside <main>"
# The button sits inside the shell's chat view section (phase 76):
# after the #view-chat open, before the #view-tuning section starts
# (the chat section closes before it — hidden views are separate).
view_chat_idx = html.find('id="view-chat"')
view_tuning_idx = html.find('id="view-tuning"')
assert -1 < view_chat_idx < btn.start() < view_tuning_idx, (
"the button belongs inside the #view-chat section"
)
shell_idx = html.find('class="container chat-shell"')
assert shell_idx != -1 and shell_idx < btn.start(), "the button belongs in .chat-shell"
messages_idx = html.find('id="messages"')
messages_end = html.find("</section>", messages_idx)
composer_idx = html.find('<form class="composer" id="composer"')
assert -1 < messages_idx < messages_end < btn.start() < composer_idx, (
"the button sits below the #messages section, above the composer"
)
# No SURVIVING separate document carries the button (owner request
# 2026-08-28, final shell form: the folded navbar views are GONE
# from this check — they are views of the shell now).
for other in (DOCUMENT_HTML, LOGIN_HTML, SHARED_HTML, DOC_EDIT_HTML):
assert 'id="new-chat-btn"' not in other.read_text(encoding="utf-8"), (
f"{other.name}: the New chat button is chat-view only"
)
def test_new_chat_button_style_contract() -> None:
"""Solid brand pill (the 2026-08-28 rebrand; it was a ghost): --bg
text on --brand (5.2:1 per the token note — WCAG AA), borderless,
≥44px target; hover lightens the brand fill; focus-visible via the
global rule. On the chat page the label stays visible even ≤640px
(the button is in .chat-shell, not the navbar) with the plus icon
hidden (aria-label keeps the accessible name either way)."""
css = _css()
block = re.search(r"\.new-chat-btn \{([\s\S]*?)\n\}", css)
assert block, "styles.css must style .new-chat-btn"
body = block.group(1)
assert "min-height: 44px" in body
assert "border-radius: 999px" in body
assert "border: 0" in body
assert "background: var(--brand)" in body, "solid brand fill (the rebrand)"
assert "color: var(--bg)" in body, "--bg text on --brand = 5.2:1 (AA)"
hover = re.search(r"\.new-chat-btn:hover \{([\s\S]*?)\n\}", css)
assert hover and "#f55a72" in hover.group(1), "hover lightens the brand fill"
# Mobile (≤640px): the button sits in .chat-shell, not the navbar —
# the label stays visible and the plus icon is hidden (room in the
# body); the pill stays ≥44px via min-height.
mobile = re.search(r"@media \(max-width: 640px\) \{([\s\S]*?)\n\}", css)
assert mobile, "mobile media query missing"
assert ".chat-shell .new-chat-label { display: inline; }" in mobile.group(1)
assert ".chat-shell .new-chat-btn svg { display: none; }" in mobile.group(1)
assert ".new-chat-btn svg { display: block; }" in mobile.group(1)
def test_brain_turn_persists_optional_thinking_field() -> None:
"""Phase 17: the done save point carries `thinking: thinkingAcc ||
undefined` — `undefined` drops the key from the JSON, so turns without
thinking persist byte-identical to before (no version bump). A
thinking-without-answer turn (reasoning exhausts max_tokens) renders
+ persists the shared empty-answer fallback: what the user saw is what
is stored."""
js = _js()
done_idx = js.find('ev.type === "done"')
error_idx = js.find('ev.type === "error"')
assert -1 < done_idx < error_idx, "done branch missing from the turn handler"
branch = js[done_idx:error_idx]
assert "thinking: thinkingAcc || undefined" in branch
assert (
'const finalText = acc || (sawThinking ? EMPTY_ANSWER_FALLBACK : "")'
in branch
)
assert "renderMarkdown(finalText)" in branch, (
"the substituted fallback must render into the bubble"
)
def test_restore_renders_collapsed_thinking_block() -> None:
"""Phase 17: a stored brain message carrying `thinking` re-renders the
block COLLAPSED above its bubble (escape-first markdown, as everywhere
else in the persistence contract); messages without the field render
exactly as before — no block."""
js = _js()
fn_start = js.find("function renderStoredMessage")
assert fn_start != -1
body = js[fn_start : js.find("\n}\n", fn_start)]
assert "if (m.thinking)" in body
assert "ensureThinkingBlock(wrap)" in body
assert "block.open = false" in body, "restored blocks must be collapsed"
assert "renderMarkdown(m.thinking)" in body
def test_thinking_block_css_uses_phase08_tokens() -> None:
"""Phase 17 styling (Phase-08 tokens, WCAG AA): the block frame, the
≥44px summary control (brand-ink ≈8.7:1 on surface) and the scratchpad
(ink-soft ≈6.9:1 on surface, 320px cap; user-scrollable again since
phase 43 — owner direction 2026-08-27, TODO.md L7)."""
css = _css()
block = re.search(r"details\.thinking \{([\s\S]*?)\n\}", css)
assert block, "styles.css must style details.thinking"
body = block.group(1)
assert "var(--surface)" in body
assert "var(--line)" in body
assert "var(--brand-soft)" in body
assert "var(--radius-sm)" in body
summary = re.search(r"details\.thinking summary \{([\s\S]*?)\n\}", css)
assert summary, "the summary must be a styled focusable control"
sbody = summary.group(1)
assert "min-height: 44px" in sbody
assert "var(--brand-ink)" in sbody
assert "cursor: pointer" in sbody
text = re.search(r"details\.thinking \.thinking-text \{([\s\S]*?)\n\}", css)
assert text, "the .thinking-text live-tail area must be styled"
tbody = text.group(1)
assert "var(--ink-soft)" in tbody
assert "max-height: 320px" in tbody
# Phase 43 (owner direction 2026-08-27): user-scrollable window again;
# the phase-17 bottom-pin (gated in app.js) is the autoscroll.
assert "overflow-y: auto" in tbody
assert "overflow-y: hidden" not in tbody