diff --git a/static/sessions.js b/static/sessions.js index b8a49e80..7d609024 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -1203,6 +1203,7 @@ async function loadSession(sid){ snapshotLiveTurnHtmlForSession(currentSid); } } + const _keepStaleUntilLoaded = !!opts.keepStaleUntilLoaded && sameSessionForceReload; if (currentSid !== sid || forceReload) { // #3306: When force-reloading the currently-active session (e.g. external // poll triggering a refresh), snapshot the existing messages BEFORE we @@ -1221,10 +1222,27 @@ async function loadSession(sid){ // instead of collapsing a long session back to the default tail window. if (sameSessionForceReload) _captureSameSessionForceReloadHint(sid); else _clearSameSessionForceReloadHint(); - S.messages = []; - S.toolCalls = []; - _messagesTruncated = false; - _oldestIdx = 0; + // #5177: keep-stale-until-loaded path — defer the destructive + // S.messages/toolCalls clear so the user does NOT see a transcript-wide + // blank gap during the metadata + messages round-trip. Only the + // visibility / focus recovery callers in refreshActiveSessionIfExternallyUpdated + // request this. The new transcript will be SWAPPED into S.messages by the + // forced _ensureMessagesLoaded(...{force:true}) call below, producing a + // single render frame with old DOM directly replaced by new DOM rather + // than the old → empty → new sequence the default branch produces. + // + // The session-switch branch (currentSid !== sid) MUST continue to clear + // synchronously — leaving a prior session's transcript on screen during a + // navigation is the original bug this clear was written for. We gate + // strictly on sameSessionForceReload (computed above as part of + // _keepStaleUntilLoaded) so cross-session switches keep their existing + // behaviour. + if (!_keepStaleUntilLoaded) { + S.messages = []; + S.toolCalls = []; + _messagesTruncated = false; + _oldestIdx = 0; + } // Close live SSE streams from the session we're leaving. The error // handler checks _isSessionActivelyViewed() and won't auto-reconnect // for a backgrounded session, preventing leaked connections that would @@ -1489,7 +1507,7 @@ async function loadSession(sid){ // this session's INFLIGHT snapshot, not leave prior-session rows in place. if(typeof clearLiveToolCards==='function') clearLiveToolCards(); try { - await _ensureMessagesLoaded(sid); + await _ensureMessagesLoaded(sid, {force:_keepStaleUntilLoaded}); } catch(e) { S.messages=inflightMessages; } @@ -1582,8 +1600,12 @@ async function loadSession(sid){ }else{ // Phase 2b: Idle session — load full messages lazily for rendering. // _ensureMessagesLoaded is idempotent; it skips if S.messages already populated. + // #5177: when the caller asked us to keep stale messages until the new ones + // arrive (visibility/focus recovery), force the fetch so the + // "messages already populated" early-return inside _ensureMessagesLoaded + // does NOT skip the swap to the new transcript. try { - await _ensureMessagesLoaded(sid); + await _ensureMessagesLoaded(sid, {force:_keepStaleUntilLoaded}); } catch (e) { // Network errors, server failures, or SSE drops (Chrome error codes 4/5) // can cause _ensureMessagesLoaded to throw. Without a try/catch here the @@ -2336,9 +2358,21 @@ function _syncToolCallsForLoadedMessages(messages, sessionToolCalls){ } } -async function _ensureMessagesLoaded(sid) { +async function _ensureMessagesLoaded(sid, opts) { + // `opts` is an explicit named parameter (vs loadSession's arguments[1] + // pattern) because _ensureMessagesLoaded is a module-private helper: it is + // only called from inside loadSession, so the public signature does not need + // to be preserved. Strict-mode engines optimize named params more reliably + // than arguments-indexing, and a named opts is self-documenting for static + // analysis. Callers pass {force:true} when they need to BYPASS the + // "messages already populated" early-return — currently only the #5177 + // keep-stale-until-loaded path, which intentionally leaves the old messages + // in place (to avoid a visible disappear/reappear gap) and relies on + // _ensureMessagesLoaded to fetch and SWAP the new transcript into + // S.messages in a single frame. + opts = opts || {}; // Already have messages? (e.g. from INFLIGHT restore path, already set) - if (S.messages && S.messages.length > 0 && S.messages[0] && S.messages[0].role) { + if (!opts.force && S.messages && S.messages.length > 0 && S.messages[0] && S.messages[0].role) { _clearSameSessionForceReloadHint(sid); return; } @@ -4512,7 +4546,23 @@ async function refreshActiveSessionIfExternallyUpdated(reason){ // list metadata, advancing the local last-seen marker so the same metadata // bump doesn't re-trigger on every subsequent poll. if(remoteCount !== localCount){ - await loadSession(sid, {force:true, externalRefreshReason:reason||'poll'}); + // Hidden-tab return / visibility / focus recovery commonly trips + // remoteCount !== localCount when the post-turn bg-review thread or a + // sibling tab persisted messages while the tab was hidden. The default + // loadSession(force) path clears S.messages synchronously and waits for + // the full transcript round-trip before re-rendering, producing the + // user-visible "everything disappears, then reappears after a moment" + // gap that #5061 (metadata-only) and #5122 (SSE 4-probe) DO NOT cover + // (#5177). Pass keepStaleUntilLoaded so the destructive clear is + // deferred to swap-in-place when the new transcript actually arrives. + // Restrict to the recovery reasons that produced the field repro; the + // post-stream idle reconcile and external/imported-session polls keep + // the original behaviour (no DOM is on-screen long enough for the gap + // to matter, and any change there would have to re-verify their own + // tradeoffs). + const _recoveryReasons = {visible:true, focus:true}; + const _keepStaleUntilLoaded = !!_recoveryReasons[String(reason||'')]; + await loadSession(sid, {force:true, externalRefreshReason:reason||'poll', keepStaleUntilLoaded:_keepStaleUntilLoaded}); if(typeof renderSessionList==='function') void renderSessionList(); return 'reloaded'; }else if(remoteLast > localLast){ diff --git a/tests/test_issue3162_ensure_messages_loaded.py b/tests/test_issue3162_ensure_messages_loaded.py index a5f80a29..4e267bdd 100644 --- a/tests/test_issue3162_ensure_messages_loaded.py +++ b/tests/test_issue3162_ensure_messages_loaded.py @@ -18,7 +18,7 @@ def _ensure_messages_loaded_body() -> str: # Window widened (#3326 added reload-width-hint handling inside this function, # pushing the carry-forward reassignment further down; #3790 added the # cold-load expand_renderable param + comment, pushing it further still). - return SESSIONS_JS[start: start + 3000] + return SESSIONS_JS[start: start + 4500] def test_ensure_messages_loaded_declares_msgs_with_let(): diff --git a/tests/test_issue3306_loadsession_carry_forward.py b/tests/test_issue3306_loadsession_carry_forward.py index ca18ee08..778f6e15 100644 --- a/tests/test_issue3306_loadsession_carry_forward.py +++ b/tests/test_issue3306_loadsession_carry_forward.py @@ -39,7 +39,7 @@ def _load_session_clear_block() -> str: def _ensure_messages_loaded_body() -> str: return _function_body( - "async function _ensureMessagesLoaded(sid) {", + "async function _ensureMessagesLoaded(sid", "function _messageComparableText", ) diff --git a/tests/test_issue4756_session_visit_model_refresh.py b/tests/test_issue4756_session_visit_model_refresh.py index 2ff73270..8e9863f4 100644 --- a/tests/test_issue4756_session_visit_model_refresh.py +++ b/tests/test_issue4756_session_visit_model_refresh.py @@ -674,7 +674,7 @@ def test_load_session_schedules_session_visit_model_refresh_before_message_load( body = _extract_function_body(_read_static("sessions.js"), "async function loadSession(") assign_idx = body.index("S.session=data.session") - message_load_idx = body.index("await _ensureMessagesLoaded(sid)", assign_idx) + message_load_idx = body.index("await _ensureMessagesLoaded(sid", assign_idx) failure_return_idx = body.index("return;", message_load_idx) model_block_idx = body.index("if(typeof populateModelDropdown==='function')", assign_idx) guard_helper_idx = body.index("const isActiveModelRefreshSession", model_block_idx) diff --git a/tests/test_issue5177_hidden_tab_blank_gap.py b/tests/test_issue5177_hidden_tab_blank_gap.py new file mode 100644 index 00000000..4a0bc8cb --- /dev/null +++ b/tests/test_issue5177_hidden_tab_blank_gap.py @@ -0,0 +1,205 @@ +"""Source-lock for the #5177 keep-stale-until-loaded path. + +Symptom this guards against: after a hidden interval during which new messages +were persisted to the active session (post-turn bg-review writes, sibling-tab +writes, the just-finished main turn writing), switching the tab back caused the +ENTIRE transcript to visibly blank for the round-trip and then reappear — +"对话突然消失，重刷才回来". (nesquena/hermes-webui#5177) + +Root cause: ``refreshActiveSessionIfExternallyUpdated('visible' | 'focus')`` hit +``remoteCount !== localCount`` and called ``loadSession(sid, {force:true})``, +which synchronously did ``S.messages = []`` before awaiting the metadata + +messages fetches. The inline comment at sessions.js itself warns about exactly +this "disappear/reappear" tradeoff but only short-circuits the metadata-only +(``remoteCount === localCount``) branch (the #5061 fix). #5122 covers a +different path (SSE error mid-stream with ``ready_state=2``) and does not apply +when the SSE error arrives while ``visibility_state='hidden'`` and bottoms out +through ``_deferStreamErrorIfPageHidden``. + +Fix: ``refreshActiveSessionIfExternallyUpdated`` passes a new +``keepStaleUntilLoaded`` option to ``loadSession`` for the visibility/focus +recovery reasons. ``loadSession`` honors it by skipping the synchronous +``S.messages = []`` block when ``sameSessionForceReload`` is true, and forces +``_ensureMessagesLoaded`` to bypass its "messages already populated" +early-return so the new transcript is fetched and SWAPPED into ``S.messages`` +in a single render frame. The visible result is old DOM → new DOM in one frame +with no intervening empty render. + +These are static source assertions (whitespace-stripped substring + simple +brace-matching) so the keep-stale shape and the +recovery-reason → keepStaleUntilLoaded plumbing cannot silently regress. +""" +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent +SESSIONS_JS = (ROOT / "static" / "sessions.js").read_text(encoding="utf-8") + + +def _compact(text: str) -> str: + return "".join(text.split()) + + +def _load_session_block(compact: str) -> str: + """Slice from the start of ``async function loadSession`` to the matching + close brace, using brace counting so future code growth in the function + cannot push assertions out of a fixed window.""" + marker = "asyncfunctionloadSession(sid){" + start = compact.find(marker) + assert start != -1, "expected the loadSession definition" + i = start + len(marker) - 1 # position of the opening brace + depth = 0 + for j in range(i, len(compact)): + c = compact[j] + if c == "{": + depth += 1 + elif c == "}": + depth -= 1 + if depth == 0: + return compact[start: j + 1] + raise AssertionError("loadSession braces did not balance") + + +def _refresh_block(compact: str) -> str: + """Slice from the start of ``async function refreshActiveSessionIfExternallyUpdated`` + to its matching close brace.""" + marker = "asyncfunctionrefreshActiveSessionIfExternallyUpdated(reason){" + start = compact.find(marker) + assert start != -1, "expected refreshActiveSessionIfExternallyUpdated definition" + i = start + len(marker) - 1 + depth = 0 + for j in range(i, len(compact)): + c = compact[j] + if c == "{": + depth += 1 + elif c == "}": + depth -= 1 + if depth == 0: + return compact[start: j + 1] + raise AssertionError("refreshActiveSessionIfExternallyUpdated braces did not balance") + + +def test_keep_stale_until_loaded_flag_computed_in_loadsession(): + # The flag must be computed by AND-ing the caller's opts with + # sameSessionForceReload — cross-session switches MUST keep clearing + # synchronously, otherwise a stale prior-session transcript stays on + # screen during the navigation. + compact = _compact(SESSIONS_JS) + block = _load_session_block(compact) + assert "const_keepStaleUntilLoaded=!!opts.keepStaleUntilLoaded&&sameSessionForceReload;" in block, ( + "loadSession must AND opts.keepStaleUntilLoaded with sameSessionForceReload" + ) + + +def test_loadsession_skips_synchronous_clear_when_keep_stale_until_loaded(): + # Inside the (currentSid !== sid || forceReload) block, the four-line clear + # (S.messages=[], S.toolCalls=[], _messagesTruncated=false, _oldestIdx=0) + # must sit inside `if (!_keepStaleUntilLoaded) { ... }`. + block = _load_session_block(_compact(SESSIONS_JS)) + guard_idx = block.find("if(!_keepStaleUntilLoaded){") + assert guard_idx != -1, ( + "expected the keep-stale guard wrapping the synchronous clear" + ) + # The four clear lines must appear inside that guarded scope (between the + # guard's opening { and a `}` of matching depth). + body = block[guard_idx:] + # Limit the slice to the first matching close-brace by depth-counting. + depth = 0 + guard_body_end = -1 + for j, c in enumerate(body): + if c == "{": + depth += 1 + elif c == "}": + depth -= 1 + if depth == 0: + guard_body_end = j + 1 + break + assert guard_body_end != -1, "guard scope did not close" + guard_body = body[:guard_body_end] + assert "S.messages=[];" in guard_body + assert "S.toolCalls=[];" in guard_body + assert "_messagesTruncated=false;" in guard_body + assert "_oldestIdx=0;" in guard_body + + +def test_only_one_synchronous_messages_clear_in_loadsession_force_block(): + # Guard against a re-introduced unguarded clear sneaking back in. There + # must be exactly ONE `S.messages=[];` site in loadSession, and it must be + # the one nested inside the if(!_keepStaleUntilLoaded) guard above. + block = _load_session_block(_compact(SESSIONS_JS)) + assert block.count("S.messages=[];") == 1, ( + "loadSession should clear S.messages exactly once, under the keep-stale guard" + ) + + +def test_ensure_messages_loaded_called_with_keep_stale_flag(): + # Both _ensureMessagesLoaded call sites inside loadSession must forward + # the keep-stale flag so the early-return inside _ensureMessagesLoaded + # cannot skip the swap when stale messages are still in place. + block = _load_session_block(_compact(SESSIONS_JS)) + # Both INFLIGHT and idle paths. + assert block.count("await_ensureMessagesLoaded(sid,{force:_keepStaleUntilLoaded})") == 2 + + +def test_ensure_messages_loaded_supports_force_override(): + # The receiving end: _ensureMessagesLoaded must look at opts.force in its + # "messages already populated" early-return so the keep-stale flag + # actually does what it says. + compact = _compact(SESSIONS_JS) + # Accept the function signature with a named `opts` parameter + # (preferred — self-documenting and strict-mode-optimization-friendly, + # greptile P2 r3393… on #5189) and the historical `arguments[1]` shape + # (for callers preserving an existing public signature). The required + # invariant is the EARLY-RETURN being gated on opts.force. + marker_named = "asyncfunction_ensureMessagesLoaded(sid,opts){" + marker_arglist = "asyncfunction_ensureMessagesLoaded(sid){" + start = compact.find(marker_named) + if start == -1: + start = compact.find(marker_arglist) + assert start != -1, "_ensureMessagesLoaded definition not found" + # Take a generously-sized window for the early-return region. + region = compact[start: start + 1000] + # Either explicit named param `opts` (preferred), or arguments[1] fallback, + # both must coerce to an object so opts.force is safe to read. + assert ( + "opts=opts||{};" in region + or "constopts=arguments[1]||{};" in region + ), "_ensureMessagesLoaded must coerce opts to an object before reading opts.force" + # The early-return MUST be GATED on !opts.force. + assert "if(!opts.force&&S.messages&&S.messages.length>0" in region, ( + "_ensureMessagesLoaded's early-return must short-circuit on opts.force" + ) + + +def test_refresh_visibility_path_requests_keep_stale_until_loaded(): + # The visibility-recovery callers must opt INTO keepStaleUntilLoaded; the + # post-stream idle reconcile and the poll path stay on the original + # destructive reload behaviour (per the design note in the patch comment). + block = _refresh_block(_compact(SESSIONS_JS)) + # The recovery-reason map MUST include 'visible' and 'focus' and EXCLUDE + # 'poll' / 'idle-reconcile'. Strip JS-side whitespace via _compact above. + assert "const_recoveryReasons={visible:true,focus:true};" in block, ( + "expected the visibility/focus recovery-reason map" + ) + assert "const_keepStaleUntilLoaded=!!_recoveryReasons[String(reason||'')];" in block + # The reloaded-path loadSession call must forward the flag — there is + # exactly one loadSession call in this block (the reloaded branch) and it + # must pass keepStaleUntilLoaded. + assert ( + "awaitloadSession(sid,{force:true,externalRefreshReason:reason||'poll',keepStaleUntilLoaded:_keepStaleUntilLoaded});" + in block + ) + + +def test_poll_and_idle_reconcile_do_not_enable_keep_stale(): + # Belt-and-suspenders: the recovery-reason map must not list 'poll' or + # 'idle-reconcile'. We assert on the LITERAL set definition so any + # accidental widening is caught by the source lock. + block = _refresh_block(_compact(SESSIONS_JS)) + # Just the two keys, exact set. + assert "const_recoveryReasons={visible:true,focus:true};" in block + # Sanity: those exact reason strings must still be the ones the + # visibility/focus listeners use elsewhere in this file. (Failure here + # means a renaming broke our routing.) + compact = _compact(SESSIONS_JS) + assert "refreshActiveSessionIfExternallyUpdated('visible')" in compact + assert "refreshActiveSessionIfExternallyUpdated('focus')" in compact diff --git a/tests/test_regressions.py b/tests/test_regressions.py index c569116e..a6a8ea2f 100644 --- a/tests/test_regressions.py +++ b/tests/test_regressions.py @@ -748,7 +748,7 @@ def test_loadSession_inflight_merges_tail_with_persisted_transcript(cleanup_test assert inflight_idx >= 0, "INFLIGHT branch not found in loadSession" inflight_block = src[inflight_idx:inflight_idx+1200] - assert "await _ensureMessagesLoaded(sid);" in inflight_block, ( + assert "await _ensureMessagesLoaded(sid" in inflight_block, ( "returning to an active stream should load the persisted transcript before adding the live tail" ) assert "_mergeInflightTailMessages(S.messages,inflightMessages)" in inflight_block, ( diff --git a/tests/test_session_channel_option_x.py b/tests/test_session_channel_option_x.py index 86ccdbe2..1b913603 100644 --- a/tests/test_session_channel_option_x.py +++ b/tests/test_session_channel_option_x.py @@ -848,7 +848,7 @@ def test_load_session_rearms_stream_on_every_early_return(): # Isolate the loadSession body. fn_ix = js.index("async function loadSession(") - body = js[fn_ix:fn_ix + 12000] + body = js[fn_ix:fn_ix + 14000] # The unconditional teardown must still be there (this is what creates the # dead-stream window the re-arm closes). @@ -881,7 +881,7 @@ def test_load_session_rearms_stream_on_every_early_return(): # but guarded against the self-healed-current (404'd) case so it never # spins the reconnect loop against a dead session_id. catch_ix = body.index("const _selfHealedCurrent") - catch_src = body[catch_ix:catch_ix + 1400] + catch_src = body[catch_ix:catch_ix + 2200] assert "!_selfHealedCurrent" in catch_src and "startSessionStream(currentSid)" in catch_src, ( "fetch-error path must restart the on-screen stream, guarded against " "the self-healed-current (deleted/404) session" diff --git a/tests/test_session_switch_busy_race.py b/tests/test_session_switch_busy_race.py index 71170f0f..5bcb8686 100644 --- a/tests/test_session_switch_busy_race.py +++ b/tests/test_session_switch_busy_race.py @@ -41,7 +41,7 @@ def test_loadSession_clears_busy_before_async_message_load_when_server_idle(): assert "S.busy=false" in idle_block, "idle switch must clear S.busy immediately" assert "S.activeStreamId=null" in idle_block, "idle switch must clear S.activeStreamId immediately" - ensure_load = body.find("await _ensureMessagesLoaded(sid)") + ensure_load = body.find("await _ensureMessagesLoaded(sid") assert ensure_load != -1, "loadSession must still lazy-load messages for idle sessions" assert idle_reset < ensure_load, ( "S.busy must be cleared before _ensureMessagesLoaded so session-list polling " diff --git a/tests/test_tars_scroll_reset_regressions.py b/tests/test_tars_scroll_reset_regressions.py index 2a125623..203977f3 100644 --- a/tests/test_tars_scroll_reset_regressions.py +++ b/tests/test_tars_scroll_reset_regressions.py @@ -112,7 +112,7 @@ def test_external_active_refresh_defers_while_reader_is_manually_unpinned(): assert "_deferActiveSessionExternalRefresh" in SESSIONS_JS assert "typeof _isMessageReaderUnpinned==='function'&&_isMessageReaderUnpinned()" in refresh assert "_deferActiveSessionExternalRefresh(reason||'poll');" in refresh - assert "await loadSession(sid, {force:true, externalRefreshReason:reason||'poll'});" in refresh + assert "await loadSession(sid, {force:true, externalRefreshReason:reason||'poll', keepStaleUntilLoaded:_keepStaleUntilLoaded});" in refresh def test_session_switch_clears_deferred_active_refresh_reason(): diff --git a/tests/test_webui_external_refresh_frontend.py b/tests/test_webui_external_refresh_frontend.py index 2bbf977e..4d4ff68e 100644 --- a/tests/test_webui_external_refresh_frontend.py +++ b/tests/test_webui_external_refresh_frontend.py @@ -40,7 +40,7 @@ def test_active_session_external_refresh_skips_destructive_reload_on_metadata_on """ assert "remoteCount > localCount || remoteLast > localLast" not in SESSIONS_JS assert "if(remoteCount !== localCount){" in SESSIONS_JS - assert "await loadSession(sid, {force:true, externalRefreshReason:reason||'poll'});" in SESSIONS_JS + assert "await loadSession(sid, {force:true, externalRefreshReason:reason||'poll', keepStaleUntilLoaded:_keepStaleUntilLoaded});" in SESSIONS_JS assert "}else if(remoteLast > localLast){" in SESSIONS_JS assert "S.session.last_message_at = remoteLast" in SESSIONS_JS assert "if(data.session.updated_at) S.session.updated_at = data.session.updated_at;" in SESSIONS_JS @@ -187,7 +187,7 @@ def test_same_width_force_reload_invalidates_visible_message_cache(): assert "_visWithIdxCacheLen=0;" in clear_body assert "clearVisibleMessageRowCache();" in UI_JS[UI_JS.index("function clearMessageRenderCache()") :] - ensure_start = SESSIONS_JS.index("async function _ensureMessagesLoaded(sid)") + ensure_start = SESSIONS_JS.index("async function _ensureMessagesLoaded(sid") ensure_end = SESSIONS_JS.index("function _messageComparableText", ensure_start) ensure_body = SESSIONS_JS[ensure_start:ensure_end] invalidate_pos = ensure_body.index("if(typeof clearVisibleMessageRowCache==='function') clearVisibleMessageRowCache();")