fix(chat): keep generating while the tab is hidden
Root cause (task 01): none of C1-C3 - in Chromium 151 (real mode) a merely-hidden tab neither stops the stream (frames arrive at full rate; turn completes) nor fires pagehide on tab switch; C1's double-record path was proven latent via a synthetic pagehide (trigger is browser-dependent, e.g. Safari) and C2 (the 120s pre-token guard) was confirmed to fire while hidden. - C1: the pagehide partial-persist is correlated with the turn's settle (leavePartialIndex) - the done/stop settle REPLACES it in place (identity-guarded rememberBrainTurn in-place mode), so bor.chat.v1 and the auto-saved saved_chats row keep exactly ONE brain turn per question; a real navigation never runs a settle, so the leave-save is unchanged. - C2: the visibility re-arm gives the still-armed pre-token guard a fresh TURN_TIMEOUT_MS when the tab returns to visible - hidden time no longer counts toward the 120s guard. - Phase-48 teardown contract untouched: Stop / tab close / real navigation still cancel the fetch and stop the model. - Unit pins: tests/unit/test_frontend_hidden_tab.py (the app.js mechanisms without a browser). - E2E pins: tests/e2e/test_hidden_tab_stream.py - synthetic pagehide mid-stream completes exactly once with one brain turn (localStorage + auto-saved row), reload restores one bubble, no-event baseline, and the fake-clock pre-token guard re-arm (discriminating: fails with the re-arm disabled).
This commit is contained in:
@@ -0,0 +1,217 @@
|
||||
"""Unit: a hidden tab never stops a turn (phase 73, task 02).
|
||||
|
||||
The JS behavior itself is E2E-covered (tests/e2e/test_hidden_tab_stream.py,
|
||||
task 03); here we pin the app.js mechanisms the story depends on — the
|
||||
pagehide-partial ↔ settle correlation (C1 hardening, unconditional) and the
|
||||
visibility-aware pre-token guard (task 01, C2 confirmed) — so a silent
|
||||
regression in app.js is caught without a browser (house pattern:
|
||||
tests/unit/test_frontend_feedback.py).
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
FRONTEND = Path(__file__).resolve().parents[2] / "frontend"
|
||||
APP_JS = FRONTEND / "assets" / "app.js"
|
||||
|
||||
|
||||
def _js() -> str:
|
||||
return APP_JS.read_text(encoding="utf-8")
|
||||
|
||||
|
||||
# ---------- C1: the pagehide partial is replaced, never duplicated ----------
|
||||
|
||||
|
||||
def test_leave_partial_index_is_a_module_turn_local() -> None:
|
||||
"""`leavePartialIndex` is a module-scope turn local (like
|
||||
persistedOnLeave): declared -1, reset at the top of runTurn, and
|
||||
written ONLY by the pagehide handler (the one `= conversation.length
|
||||
- 1` assignment). No other code may move the correlation."""
|
||||
js = _js()
|
||||
assert "let leavePartialIndex = -1" in js, "module-scope declaration"
|
||||
# The two -1 writes are the declaration and the runTurn reset.
|
||||
assert js.count("leavePartialIndex = -1;") == 2, (
|
||||
"only the declaration and the runTurn reset write -1"
|
||||
)
|
||||
# The one and only index write is the pagehide handler's.
|
||||
assert js.count("leavePartialIndex = conversation.length - 1") == 1, (
|
||||
"only the pagehide handler records the partial's index"
|
||||
)
|
||||
# The reset sits at the top of runTurn, with the other turn locals
|
||||
# (the acc / thinkingAcc / persistedOnLeave group, just before the
|
||||
# fresh AbortController).
|
||||
turn = js.find("async function runTurn")
|
||||
abort_idx = js.find("turnAbort = new AbortController()", turn)
|
||||
assert -1 < turn < abort_idx, "runTurn must exist"
|
||||
turn_top = js[turn:abort_idx]
|
||||
assert "leavePartialIndex = -1;" in turn_top, "reset per turn at the top"
|
||||
assert turn_top.index("persistedOnLeave = false;") < turn_top.index(
|
||||
"leavePartialIndex = -1;"
|
||||
), "reset alongside the other turn locals (phase-20 group)"
|
||||
|
||||
|
||||
def test_pagehide_records_the_partial_index_after_the_push() -> None:
|
||||
"""The pagehide handler keeps its phase-20 guards (idempotency,
|
||||
in-flight only, nothing brain-side yet) and, AFTER the partial is
|
||||
pushed through rememberBrainTurn, records the pushed record's index
|
||||
so the turn's settle can find it."""
|
||||
js = _js()
|
||||
fn = js.find('window.addEventListener("pagehide"')
|
||||
assert fn != -1, "the pagehide listener must exist"
|
||||
body = js[fn: js.find("\n});", fn)]
|
||||
assert "if (persistedOnLeave) return;" in body, "phase-20 idempotency guard"
|
||||
assert "if (!acc) return;" in body, "nothing brain-side yet → no save"
|
||||
i_guard = body.index("persistedOnLeave = true;")
|
||||
i_push = body.index("rememberBrainTurn(acc, { thinking: thinkingAcc || undefined });")
|
||||
i_index = body.index("leavePartialIndex = conversation.length - 1")
|
||||
assert i_guard < i_push < i_index, (
|
||||
"flag → push the partial → record its index (the record it just pushed)"
|
||||
)
|
||||
|
||||
|
||||
def test_remember_brain_turn_replaces_in_place_with_identity_guard() -> None:
|
||||
"""rememberBrainTurn's optional in-place mode: `replaceIndex >= 0`
|
||||
REPLACES the record at that index instead of appending — and only
|
||||
when the index STILL points at a brain record (the identity guard:
|
||||
a New-Chat click or restore between pagehide and settle falls back
|
||||
to the append). Either way the record is written once, and the
|
||||
save points (localStorage + the headless auto-save) run once on the
|
||||
written record."""
|
||||
js = _js()
|
||||
fn = js.find("function rememberBrainTurn")
|
||||
assert fn != -1, "rememberBrainTurn must exist"
|
||||
body = js[fn: js.find("\n}\n", fn)]
|
||||
assert "function rememberBrainTurn(rawText, meta, replaceIndex = -1)" in body, (
|
||||
"the optional in-place mode defaults to the plain append"
|
||||
)
|
||||
assert 'const rec = { who: "brain", text: rawText || "…", ...meta }' in body, (
|
||||
"one record object, written once"
|
||||
)
|
||||
assert (
|
||||
'if (replaceIndex >= 0 && conversation[replaceIndex]?.who === "brain")' in body
|
||||
), "identity guard: the index must still point at a brain record"
|
||||
assert "conversation[replaceIndex] = rec" in body, "the in-place replace"
|
||||
assert "conversation.push(rec)" in body, "the append (no partial / guard miss)"
|
||||
i_replace = body.index("conversation[replaceIndex] = rec")
|
||||
i_push = body.index("conversation.push(rec)")
|
||||
assert i_replace < i_push, "replace branch precedes the append fallback"
|
||||
i_save = body.index("saveConversation()")
|
||||
i_persist = body.index("persistConversation()")
|
||||
assert i_push < i_save < i_persist, (
|
||||
"both save points run ONCE, after the single write — the auto-save "
|
||||
"refreshes the row exactly once for the replaced record"
|
||||
)
|
||||
|
||||
|
||||
def test_every_settle_writes_through_the_correlation() -> None:
|
||||
"""The three brain-record write sites in runTurn's settle paths —
|
||||
the `done` save point, the zero-frame empty-answer fallback, and the
|
||||
stop-path partial — all pass `leavePartialIndex`, so the replace is
|
||||
the only write after a pagehide partial (the invariant). The
|
||||
pagehide handler itself pushes with the plain two-arg form (it IS
|
||||
the partial)."""
|
||||
js = _js()
|
||||
# done save point.
|
||||
done = js.find('ev.type === "done"')
|
||||
done_branch = js[done: js.find('ev.type === "error"', done)]
|
||||
assert "leavePartialIndex" in done_branch, "the done settle is correlated"
|
||||
# zero-frame empty-answer fallback.
|
||||
fallback = js.find("!aborted && !wrap")
|
||||
fallback_block = js[fallback: js.find(") catch (err) {", fallback)]
|
||||
assert "leavePartialIndex" in fallback_block, "the fallback settle is correlated"
|
||||
# stop-path partial (the `stopped` marker persist).
|
||||
catch = js.find("} catch (err) {")
|
||||
stop_idx = js.find('stoppedByUser || err?.name === "AbortError"', catch)
|
||||
stop_branch = js[stop_idx: js.find("} else {", stop_idx)]
|
||||
assert "leavePartialIndex" in stop_branch, "the stop settle is correlated"
|
||||
# The pagehide handler pushes the plain two-arg partial (the record
|
||||
# that the settles replace).
|
||||
fn = js.find('window.addEventListener("pagehide"')
|
||||
body = js[fn: js.find("\n});", fn)]
|
||||
assert "rememberBrainTurn(acc, { thinking: thinkingAcc || undefined });" in body
|
||||
assert "leavePartialIndex" not in body.replace(
|
||||
"leavePartialIndex = conversation.length - 1", ""
|
||||
), "the pagehide handler only records the index — it never settles"
|
||||
|
||||
|
||||
def test_real_navigation_behavior_is_unchanged() -> None:
|
||||
"""The leave-save invariant: a REAL navigation never runs a settle
|
||||
(the page unloads), so the pagehide partial stays persisted exactly
|
||||
as phase 20 left it — the handler's guards and the rememberBrainTurn
|
||||
ride-through (localStorage + auto-save) are untouched."""
|
||||
js = _js()
|
||||
fn = js.find('window.addEventListener("pagehide"')
|
||||
body = js[fn: js.find("\n});", fn)]
|
||||
for guard in (
|
||||
"if (persistedOnLeave) return;",
|
||||
"if (uiState !== UI_STATE.thinking && uiState !== UI_STATE.streaming)",
|
||||
"if (!acc) return;",
|
||||
):
|
||||
assert guard in body, f"phase-20 guard intact: {guard}"
|
||||
# The phase-48 teardown contract (real close / navigation / Stop still
|
||||
# aborts the fetch) is untouched.
|
||||
assert "turnAbort?.abort()" in js, "the abort owner still aborts the fetch"
|
||||
assert re.search(r"export\s+const\s+TURN_TIMEOUT_MS\s*=\s*120_?000\s*;", js), (
|
||||
"the guard constant is still 120s (owner-locked phase-17/48 value)"
|
||||
)
|
||||
|
||||
|
||||
# ---------- C2: hidden time does not count toward the pre-token guard ----------
|
||||
|
||||
|
||||
def test_guard_arms_with_a_rearmable_callback() -> None:
|
||||
"""armTurnTimeout remembers the guard's callback (turnTimeoutCb) so
|
||||
the visibility listener can re-arm it; clearTurnTimeout clears BOTH
|
||||
the timer and the callback — armed ⇔ callback set."""
|
||||
js = _js()
|
||||
fn = js.find("function armTurnTimeout")
|
||||
assert fn != -1, "armTurnTimeout must exist"
|
||||
body = js[fn: js.find("\n}\n", fn)]
|
||||
assert "turnTimeoutCb = onTimeout" in body, "the callback is remembered"
|
||||
assert body.index("turnTimeoutCb = onTimeout") < body.index(
|
||||
"turnTimeout = setTimeout(onTimeout, TURN_TIMEOUT_MS)"
|
||||
), "the callback is stored with the arm"
|
||||
cfn = js.find("function clearTurnTimeout")
|
||||
cbody = js[cfn: js.find("\n}\n", cfn)]
|
||||
assert "clearTimeout(turnTimeout)" in cbody
|
||||
assert "turnTimeout = 0" in cbody
|
||||
assert "turnTimeoutCb = null" in cbody, "clearing the timer drops the callback"
|
||||
|
||||
|
||||
def test_visibility_rearm_counts_only_visible_pre_token_time() -> None:
|
||||
"""On visibilitychange, when the tab returns to VISIBLE with the
|
||||
guard still armed (the pre-token window — the timer id is non-zero
|
||||
and the callback set), it re-arms with a FRESH TURN_TIMEOUT_MS via
|
||||
armTurnTimeout (the same arm path — one timer, one owner). Hidden
|
||||
transitions never re-arm. Exactly one such listener exists."""
|
||||
js = _js()
|
||||
assert js.count('document.addEventListener("visibilitychange"') == 1, (
|
||||
"one visibility listener (the guard re-arm)"
|
||||
)
|
||||
fn = js.find('document.addEventListener("visibilitychange"')
|
||||
body = js[fn: js.find("});", fn)]
|
||||
assert 'document.visibilityState === "visible"' in body, (
|
||||
"only a return to visible re-arms — hidden transitions do not extend it"
|
||||
)
|
||||
assert "turnTimeout && turnTimeoutCb" in body, (
|
||||
"only an ARMED guard (pre-token window) is re-armed — the timer clears "
|
||||
"on the first thinking/delta/retry frame and every terminal transition"
|
||||
)
|
||||
assert "armTurnTimeout(turnTimeoutCb)" in body, (
|
||||
"the re-arm goes through the same arm path (a fresh TURN_TIMEOUT_MS)"
|
||||
)
|
||||
assert "TURN_TIMEOUT_MS" in body or "armTurnTimeout" in body, (
|
||||
"the fresh deadline is the owner-locked TURN_TIMEOUT_MS"
|
||||
)
|
||||
|
||||
|
||||
def test_header_documents_the_phase_73_contract() -> None:
|
||||
"""House convention: the file-header doc inventories each phase's
|
||||
contract — phase 73 documents the hidden-tab rule (only close /
|
||||
navigation / Stop aborts) and both hardenings."""
|
||||
js = _js()
|
||||
header = js[: js.find("import {")]
|
||||
assert "phase 73" in header.lower(), "the header must carry the phase-73 section"
|
||||
assert "leavePartialIndex" in header, "the correlation local is documented"
|
||||
assert "visibilitychange" in header, "the visibility re-arm is documented"
|
||||
Reference in New Issue
Block a user