All verification complete. Every gate is green and the docs check out. Final report:
## Phase 158 final verification pass — COMPLETE
**Verified (all tasks pre-completed; this pass re-ran the full gate):**
- Unit + integration: `uv run pytest --cov=app --cov-report=term-missing` → **3618 passed, 99% coverage on `app/`** (>90% gate)
- Phase E2E in isolation: `uv run pytest tests/e2e/test_thinking_timeline.py -v --no-cov` → **6/6 passed** (order pin, fold-at-rest, live-open, restore+`segments` array, shared page, no-thinking single disclosure)
- Regression E2E, each in isolation: `test_thinking_display` 5✓, `test_tool_call_compact` 4✓, `test_turn_progress_loader` 3✓ (per-run pin updated), `test_big_read_progress` 4✓, `test_agent_document_tools` 4✓, `test_share_chat` 4✓, `test_chat_persistence` 4✓, `test_smoke` 3✓
- Lint/types: `uv run ruff check . && uv run pyright` → **clean (0 errors)**
**Completion criteria:** all 9 met — timeline E2E pins ✓, restore/shared `segments` ✓, no-thinking single-disclosure + the three UNCHANGED regression suites green ✓, pre-phase byte-identical round-trip (integration pin: key absent, never null; >50 / >32 000 / empty text / unknown kind all 422) ✓, full suite + coverage + E2E + lint ✓, `git diff --stat` footprint limited to `app/schemas.py`, `frontend/assets/{app,shared}.js`, `tests/`, README, phase files (a pre-existing owner `TODO.md` edit is out of scope, not staged by the phase commit) ✓, D15→per-run supersession recorded in the updated pin ✓. (Commit + phase move are the harness's job; task dirs already in `complete/`.)
**Documentation:** README.md — Thinking section correctly describes the phase-158 timeline; config reference matches `.env.example` exactly; page table matches registered routes; no stale single-block wording. AGENTS.md — rules and quick-reference commands all accurate. **Verified consistent — no changes needed.**
**Decisions/deviations:** none. `test_tool_call_compact.py` carries a one-line docstring-only update (referenced the removed `closeToolCalls`); its test logic is byte-identical to the phase-117 contract and green.
**Next pending phase:** none — `todo/` contains only 158.
164 lines
7.5 KiB
Python
164 lines
7.5 KiB
Python
"""Unit: the navigate-away partial-persistence contract (phase 20).
|
|
|
|
The browser behavior is E2E-covered (tests/e2e/test_sources_midstream_bug.py);
|
|
here we pin the source-level wiring in app.js — the single `pagehide`
|
|
listener, its in-flight guard (uiState thinking/streaming + non-empty acc),
|
|
the `rememberBrainTurn(acc, { thinking: thinkingAcc || undefined })` reuse
|
|
(no duplicated storage code), the turn-scoped `persistedOnLeave` idempotency
|
|
flag (declared at module scope, reset per turn in the turn handler), the
|
|
updated persistence-block comment — so a silent regression is caught without
|
|
a browser. The deliberate New Chat clear (clearChatStorage in header.js,
|
|
phase 14/19) must stay the only other clear: untouched.
|
|
"""
|
|
from __future__ import annotations
|
|
|
|
import re
|
|
from pathlib import Path
|
|
|
|
FRONTEND = Path(__file__).resolve().parents[2] / "frontend"
|
|
ASSETS = FRONTEND / "assets"
|
|
APP_JS = ASSETS / "app.js"
|
|
HEADER_JS = ASSETS / "header.js"
|
|
|
|
|
|
def _js() -> str:
|
|
assert APP_JS.is_file()
|
|
return APP_JS.read_text(encoding="utf-8")
|
|
|
|
|
|
def _pagehide_body(js: str) -> str:
|
|
"""The body of the single `pagehide` handler in app.js."""
|
|
m = re.search(r'window\.addEventListener\("pagehide", \(\) => \{([\s\S]*?)\n\}\);', js)
|
|
assert m, "app.js must register a window `pagehide` handler"
|
|
return m.group(1)
|
|
|
|
|
|
def test_exactly_one_pagehide_listener_registered() -> None:
|
|
"""One and only one `pagehide` listener — the navigate-away save
|
|
point (phase 20); no duplicated registration (bfcache churn is
|
|
handled by the idempotency flag, not a second listener)."""
|
|
js = _js()
|
|
assert js.count('addEventListener("pagehide"') == 1
|
|
|
|
|
|
def test_pagehide_guard_requires_in_flight_state_and_streamed_text() -> None:
|
|
"""The handler persists ONLY when a turn is in flight (uiState is
|
|
`thinking` or `streaming`) and answer text has already streamed
|
|
(non-empty acc). Thinking-only turns persist nothing brain-side:
|
|
the question is already saved on send and the user can re-ask."""
|
|
body = _pagehide_body(_js())
|
|
assert "UI_STATE.thinking" in body and "UI_STATE.streaming" in body, (
|
|
"the guard must reference both in-flight states"
|
|
)
|
|
idx_state = body.find("uiState !== UI_STATE.thinking && uiState !== UI_STATE.streaming")
|
|
assert idx_state != -1, "the in-flight guard is missing"
|
|
idx_acc = body.find("if (!acc) return;")
|
|
assert idx_acc != -1, "the non-empty-acc guard is missing"
|
|
assert idx_state < idx_acc, "state guard must run before the acc guard"
|
|
|
|
|
|
def test_pagehide_reuses_remember_brain_turn_with_thinking() -> None:
|
|
"""The partial is persisted through the EXISTING save-point helper —
|
|
raw text, optional thinking field (`undefined` drops the key from the
|
|
JSON) + phase 158 (D4): the optional timeline (non-empty only), no
|
|
sources/deflection: a plain brain message that restore re-renders
|
|
exactly like a completed answer (no '(partial)' marker)."""
|
|
js = _js()
|
|
body = _pagehide_body(js)
|
|
assert "rememberBrainTurn(acc, {" in body
|
|
assert "thinking: thinkingAcc || undefined" in body
|
|
assert (
|
|
"segments: pagehideSegments.length ? pagehideSegments : undefined"
|
|
in body
|
|
), "phase 158: the partial's timeline persists (non-empty only)"
|
|
# No duplicated storage code: the handler pushes nothing itself.
|
|
assert "conversation.push" not in body
|
|
assert "saveConversation()" not in body
|
|
# The partial carries no done metadata.
|
|
assert "deflected" not in body and "sources" not in body
|
|
|
|
|
|
def test_persisted_on_leave_flag_is_module_scoped_and_turn_reset() -> None:
|
|
"""`persistedOnLeave` makes the save point idempotent (a second
|
|
pagehide / bfcache store+restore never appends the same partial
|
|
twice): checked first, set true immediately before the persist
|
|
call. It is declared at module scope (the handler reads it) and
|
|
reset to false at the top of the turn handler — turn-scoped like
|
|
the other turn locals. acc / thinkingAcc were hoisted the same way
|
|
(no behavior change: same reset point, same names)."""
|
|
js = _js()
|
|
# Module-scope declarations (column 0).
|
|
assert re.search(r"^let persistedOnLeave = false", js, re.M)
|
|
assert re.search(r"^let acc = \"\"", js, re.M)
|
|
assert re.search(r"^let thinkingAcc = \"\"", js, re.M)
|
|
# Each hoisted local is declared exactly once (module scope only —
|
|
# the turn handler assigns, never re-declares).
|
|
assert js.count("let acc = \"\"") == 1
|
|
assert js.count("let thinkingAcc = \"\"") == 1
|
|
assert js.count("let persistedOnLeave") == 1
|
|
|
|
body = _pagehide_body(js)
|
|
idx_check = body.find("if (persistedOnLeave) return;")
|
|
idx_set = body.find("persistedOnLeave = true;")
|
|
idx_call = body.find("rememberBrainTurn(acc,")
|
|
assert -1 < idx_check < idx_set < idx_call, (
|
|
"flag check first, set immediately before the persist call"
|
|
)
|
|
|
|
# Reset at the top of the turn handler (runTurn — phase 49 extracted
|
|
# the turn from handleSend) — before the turn's fetch, where the
|
|
# other turn locals are initialized. The pin is the ORDER (resets
|
|
# before the fetch), not a char window: phase 123 task 02 grew the
|
|
# save-point-1 region above the resets (the attached image's bubble
|
|
# + record + strip clear) without moving the resets.
|
|
turn = js.find("async function runTurn")
|
|
assert turn != -1
|
|
fetch_idx = js.find('fetch("/api/chat"', turn)
|
|
assert fetch_idx != -1
|
|
top = js[turn:fetch_idx]
|
|
assert "persistedOnLeave = false;" in top, (
|
|
"persistedOnLeave must be reset per turn, at the top of the turn handler"
|
|
)
|
|
assert "acc = \"\";" in top
|
|
assert "thinkingAcc = \"\";" in top
|
|
assert "let acc" not in top and "let thinkingAcc" not in top, (
|
|
"the turn handler must assign the hoisted locals, not re-declare them"
|
|
)
|
|
assert "let persistedOnLeave" not in top
|
|
|
|
|
|
def test_persistence_comment_lists_the_pagehide_save_point() -> None:
|
|
"""The phase-14 persistence block comment now lists THREE save
|
|
points: user message on send, brain message on `done`, and the
|
|
PARTIAL brain message on navigate-away (`pagehide`, phase 20)."""
|
|
js = _js()
|
|
comment_start = js.find("conversation persistence (phase 14)")
|
|
key_idx = js.find('const STORAGE_KEY = "bor.chat.v1"')
|
|
assert -1 < comment_start < key_idx
|
|
comment = js[comment_start:key_idx]
|
|
assert re.search(r"Save points:.*?pagehide", comment, re.S), (
|
|
"the Save points sentence must list the pagehide save point"
|
|
)
|
|
assert "PARTIAL brain" in comment
|
|
|
|
|
|
def test_new_chat_clear_is_untouched_and_still_the_only_deliberate_clear() -> None:
|
|
"""clearChatStorage (header.js) — the deliberate New Chat clear from
|
|
the non-chat pages — is untouched and remains the only place that
|
|
removes the literal key string; app.js's own clear goes through
|
|
STORAGE_KEY (#new-chat-btn, phase 14/19), and nothing new was added."""
|
|
header = HEADER_JS.read_text(encoding="utf-8")
|
|
fn = header.find("function clearChatStorage")
|
|
assert fn != -1
|
|
body = header[fn : header.find("\n}", fn)]
|
|
assert 'localStorage.removeItem("bor.chat.v1")' in body
|
|
assert "try" in body and "catch" in body
|
|
# No page script clears the key via the literal string.
|
|
for name in ("app.js", "sources.js", "document.js", "login.js"):
|
|
text = (ASSETS / name).read_text(encoding="utf-8")
|
|
assert 'removeItem("bor.chat.v1")' not in text, (
|
|
f"{name}: clearChatStorage in header.js is the only literal-key clear"
|
|
)
|
|
# app.js still has exactly one clear (clearStoredConversation, STORAGE_KEY).
|
|
assert _js().count("removeItem(STORAGE_KEY)") == 1
|