fix(chat): raise pre-token guard from 120 s to 300 s
The client-side TURN_TIMEOUT_MS was the binding constraint: turns with slow prompt processing (no first SSE frame within 120 s of visible time) errored with the 'stuck' copy even though nginx (300 s) and BOR_LLM_TIMEOUT (300 s) would have let them run. Raise the guard to 300 s so the upstream timeouts are reachable, and re-pin the tests: the unit constant pins and the fake-clock E2E timeline (295 s hidden + 290 s after the re-arm = 585 s: past the original 300 s deadline, short of the re-armed 595 s deadline). Verified: tests/unit (full, 100% pass), tests/e2e/test_hidden_tab_stream.py and tests/e2e/test_loading_feedback.py in isolation.
This commit is contained in:
@@ -34,10 +34,10 @@ mid-stream window) the pins are:
|
||||
question with NO dispatched events completes identically (guards
|
||||
against an over-eager fix changing the normal path).
|
||||
4. ``test_pre_token_guard_survives_hidden_window`` — task 01's C2
|
||||
(confirmed): hidden time must not count toward the 120 s pre-token
|
||||
(confirmed): hidden time must not count toward the 300 s pre-token
|
||||
guard. Playwright's fake clock makes the guard's deadline
|
||||
deterministic: the clock is fast-forwarded past the guard's
|
||||
ORIGINAL 120 s deadline while the tab is (synthetically) hidden,
|
||||
ORIGINAL 300 s deadline while the tab is (synthetically) hidden,
|
||||
with a return to visible in between — the phase-73 re-arm gives the
|
||||
still-armed guard a FRESH ``TURN_TIMEOUT_MS``, so the turn still
|
||||
settles with the answer. Without the re-arm the fast-forwarded
|
||||
@@ -91,7 +91,7 @@ LONG_ANSWER = long_answer()
|
||||
LONG_ANSWER_END = "LONG-ANSWER-END"
|
||||
|
||||
#: The mock's only pure pre-token silence (phase-06 phrasing): a 3 s
|
||||
#: delay before the FIRST frame, so the 120 s pre-token guard stays
|
||||
#: delay before the FIRST frame, so the 300 s pre-token guard stays
|
||||
#: armed the whole window (no thinking/delta/retry frame to clear it).
|
||||
SLOW_QUESTION = "pretend to think slowly then tell me about kubernetes"
|
||||
MOCK_ANSWER_MARKER = "Deterministic mock answer for E2E"
|
||||
@@ -435,29 +435,29 @@ def test_baseline_no_pagehide_still_completes(
|
||||
def test_pre_token_guard_survives_hidden_window(
|
||||
page: Page, app_url: str, mock_llm: int, db_ready: None
|
||||
) -> None:
|
||||
"""The 120 s pre-token guard is a plain ``setTimeout`` — without the
|
||||
"""The 300 s pre-token guard is a plain ``setTimeout`` — without the
|
||||
phase-73 visibility re-arm, hidden time counts toward it and a turn
|
||||
whose first frame lands past the 120 s mark errors with the "stuck"
|
||||
whose first frame lands past the 300 s mark errors with the "stuck"
|
||||
copy (task 01 scenario B, C2 confirmed). The fake clock makes the
|
||||
deadline deterministic (waiting 120 s of real time is not an
|
||||
deadline deterministic (waiting 300 s of real time is not an
|
||||
option for a regression suite): the mock's 3 s real-time pre-token
|
||||
silence keeps the guard armed while the FAKE clock is fast-forwarded
|
||||
across the original deadline — with a return to visible in between,
|
||||
which re-arms the still-armed guard with a fresh TURN_TIMEOUT_MS.
|
||||
|
||||
Timeline (fake clock, the guard armed at t=0, deadline t=120 s):
|
||||
Timeline (fake clock, the guard armed at t=0, deadline t=300 s):
|
||||
|
||||
* t=0 — the slow question is sent; the guard arms (t=120 s);
|
||||
* t=0 — the slow question is sent; the guard arms (t=300 s);
|
||||
* t~0 — the tab goes hidden (synthetic; pre-token, so the
|
||||
pagehide handler persists nothing — the question is
|
||||
already saved);
|
||||
* t=115 — fast-forward while hidden: the original timer (120 s)
|
||||
* t=295 — fast-forward while hidden: the original timer (300 s)
|
||||
is not due yet;
|
||||
* t=115 — back to visible: the re-arm fires (the guard is still
|
||||
armed — no frame has arrived) → new deadline t=235 s;
|
||||
* t=229 — fast-forward again: PAST the original 120 s deadline —
|
||||
* t=295 — back to visible: the re-arm fires (the guard is still
|
||||
armed — no frame has arrived) → new deadline t=595 s;
|
||||
* t=585 — fast-forward again: PAST the original 300 s deadline —
|
||||
without the re-arm the guard fires HERE with the "stuck"
|
||||
error — but short of the re-armed 235 s deadline.
|
||||
error — but short of the re-armed 595 s deadline.
|
||||
|
||||
Meanwhile (real time, ~3.5 s after the send) the mock's first frame
|
||||
lands, clears the guard for good, and the turn settles with the
|
||||
@@ -467,7 +467,7 @@ def test_pre_token_guard_survives_hidden_window(
|
||||
page.set_default_timeout(30_000)
|
||||
|
||||
# The fake clock BEFORE the first navigation: every app timer (the
|
||||
# 120 s guard among them) becomes test-controlled, while the mock's
|
||||
# 300 s guard among them) becomes test-controlled, while the mock's
|
||||
# real-time stream is unaffected (the network is not a timer).
|
||||
page.clock.install()
|
||||
login(page, app_url, next="/") # phase 79: chat is require_user-gated
|
||||
@@ -481,9 +481,9 @@ def test_pre_token_guard_survives_hidden_window(
|
||||
expect(page.locator(ANSWER)).to_have_count(0)
|
||||
|
||||
page.evaluate(HIDE_JS)
|
||||
page.clock.fast_forward(115_000)
|
||||
page.clock.fast_forward(295_000)
|
||||
page.evaluate(SHOW_JS) # the re-arm: a fresh TURN_TIMEOUT_MS
|
||||
page.clock.fast_forward(114_000) # past 120 s, short of the re-armed 235 s
|
||||
page.clock.fast_forward(290_000) # past 300 s, short of the re-armed 595 s
|
||||
|
||||
# The turn still settles with the full answer — no "taking a long
|
||||
# time" error, no aborted stream.
|
||||
|
||||
@@ -27,13 +27,13 @@ def _html() -> str:
|
||||
return (FRONTEND / "index.html").read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def test_turn_timeout_constant_exported_at_120s() -> None:
|
||||
"""The 120s client-side guard (PLAN §7.4) must be an *exported*
|
||||
def test_turn_timeout_constant_exported_at_300s() -> None:
|
||||
"""The 300s client-side guard (PLAN §7.4) must be an *exported*
|
||||
constant — testable, and the single value the E2E timeout story keys
|
||||
off."""
|
||||
js = _js()
|
||||
match = re.search(r"export\s+const\s+TURN_TIMEOUT_MS\s*=\s*120_?000\s*;", js)
|
||||
assert match, "app.js must export `const TURN_TIMEOUT_MS = 120000`"
|
||||
match = re.search(r"export\s+const\s+TURN_TIMEOUT_MS\s*=\s*300_?000\s*;", js)
|
||||
assert match, "app.js must export `const TURN_TIMEOUT_MS = 300000`"
|
||||
|
||||
|
||||
def test_state_machine_has_all_four_states() -> None:
|
||||
@@ -69,7 +69,7 @@ def test_typing_indicator_contract_strings() -> None:
|
||||
|
||||
|
||||
def test_error_banner_has_actionable_hint() -> None:
|
||||
"""Every error path (SSE error event, non-2xx, 120s timeout) must
|
||||
"""Every error path (SSE error event, non-2xx, 300s timeout) must
|
||||
surface the same actionable retry hint (story AC4)."""
|
||||
js = _js()
|
||||
assert "check the LLM is reachable" in js
|
||||
@@ -118,7 +118,7 @@ def test_busy_button_style_tokens() -> None:
|
||||
|
||||
def test_thinking_event_is_a_first_class_turn_branch() -> None:
|
||||
"""Phase 17: `thinking` SSE frames stream live into the collapsible
|
||||
Thinking block — the typing dots make way, the 120s pre-token guard
|
||||
Thinking block — the typing dots make way, the 300s pre-token guard
|
||||
clears (the stream is alive), and the text renders through the
|
||||
escape-first markdown renderer (XSS-safe). While open, the stream is
|
||||
pinned to the bottom of the block."""
|
||||
@@ -129,7 +129,7 @@ def test_thinking_event_is_a_first_class_turn_branch() -> None:
|
||||
branch = js[thinking_idx:delta_idx]
|
||||
assert "thinkingAcc += ev.text" in branch
|
||||
assert "sawThinking = true" in branch
|
||||
assert "clearTurnTimeout()" in branch, "first thinking frame clears the 120s guard"
|
||||
assert "clearTurnTimeout()" in branch, "first thinking frame clears the 300s guard"
|
||||
assert "removeTyping()" in branch, "the live block replaces the typing dots"
|
||||
assert "ensureThinkingBlock(wrap)" in branch
|
||||
assert "renderMarkdown(thinkingAcc)" in branch, "escape-first renderer (XSS-safe)"
|
||||
@@ -232,7 +232,7 @@ def test_in_flight_button_is_the_stop_control() -> None:
|
||||
def test_abort_plumbing_owns_the_fetch() -> None:
|
||||
"""The in-flight fetch is owned by an AbortController created at
|
||||
turn start (module scope, cleared in the finally), passed to the
|
||||
fetch as its signal; the 120s guard aborts the same controller as
|
||||
fetch as its signal; the 300s guard aborts the same controller as
|
||||
its backstop — with `aborted = true` FIRST, so the catch never reads
|
||||
the guard's abort as a user stop (one owner, same outcome)."""
|
||||
js = _js()
|
||||
@@ -614,7 +614,7 @@ def test_retry_frame_is_a_first_class_branch_between_tool_and_delta() -> None:
|
||||
"""Phase 67 (owner-locked A4, task 02's contract): a `retry` SSE
|
||||
frame (the server restarted the LLM request before its first piece
|
||||
— locked A2) is a first-class branch in runTurn's handler, ordered
|
||||
BETWEEN the `tool` and `delta` branches. It clears the 120s guard
|
||||
BETWEEN the `tool` and `delta` branches. It clears the 300s guard
|
||||
(a frame arrived), resolves n/N from the frame, and writes the
|
||||
owner-locked copy literal onto the EXISTING channels only — the
|
||||
#send-status live region + the typing-indicator aria-label. No DOM
|
||||
@@ -630,7 +630,7 @@ def test_retry_frame_is_a_first_class_branch_between_tool_and_delta() -> None:
|
||||
)
|
||||
assert js.count('ev.type === "retry"') == 1, "exactly one retry branch"
|
||||
branch = js[retry_idx:delta_idx]
|
||||
assert "clearTurnTimeout()" in branch, "a frame arrived — the 120s guard clears"
|
||||
assert "clearTurnTimeout()" in branch, "a frame arrived — the 300s guard clears"
|
||||
assert "Number(ev.attempt)" in branch, "n = the attempt about to be tried"
|
||||
assert "Number(ev.max_attempts)" in branch, "N = the configured total"
|
||||
assert (
|
||||
|
||||
@@ -152,8 +152,8 @@ def test_real_navigation_behavior_is_unchanged() -> None:
|
||||
# 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)"
|
||||
assert re.search(r"export\s+const\s+TURN_TIMEOUT_MS\s*=\s*300_?000\s*;", js), (
|
||||
"the guard constant is still 300s (raised from the phase-17/48 value)"
|
||||
)
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user