From d24d6377f906f5e7621237fe60d2b591c3b0564f Mon Sep 17 00:00:00 2001 From: happy5318 Date: Fri, 18 Sep 2026 11:24:51 +0800 Subject: [PATCH 1/7] perf(api/session): bound recovery calls; add msg_limit=all escape hatch Fixes the #7310 stall class: six frontend recovery paths (offline/bfcache refreshSession, stream-end settle, cancel sync, /compress preflight, /retry, /undo) issued bare GET /api/session reads that re-walked, re-redacted and re-serialized the ENTIRE transcript (4-15s / 28MB on a 5k-row session, #7625), and the same shape shipped the full session-level tool_calls list because windowing was gated on msg_limit being present. Changes: - Server: add explicit msg_limit=all escape hatch; window session-level tool_calls by actual truncation (length comparison) instead of the presence of a limit parameter. The bare no-limit shape keeps its full-transcript contract (pinned by test_state_db_reconciliation_preserves_sidecar_order_when_timestamps_collide and the run-journal snapshot test). - Client: refreshSession / _restoreSettledSession / cancel sync / /compress preflight / /retry / /undo now request the bounded tail (msg_limit=30) and restore the _messages_truncated/_messages_offset paging gate instead of hardcoding 'not truncated'; jump-to-start (_ensureAllMessagesLoaded) and outline jump opt in to the full transcript via msg_limit=all. Tests: test_session_default_window.py covers helper semantics, per-call-site bounded/all shapes, truncation-signal restoration, and a regression guard that no bare GET /api/session fetch remains in static/. 454 related tests pass; reverse-verified (reverting the fixes turns the tests red). --- api/routes.py | 43 ++++++- static/commands.js | 20 +++- static/messages.js | 21 +++- static/outline.js | 12 +- static/sessions.js | 2 +- static/ui.js | 6 +- tests/test_session_default_window.py | 172 +++++++++++++++++++++++++++ 7 files changed, 252 insertions(+), 24 deletions(-) create mode 100644 tests/test_session_default_window.py diff --git a/api/routes.py b/api/routes.py index 7d30ab90..474fc229 100644 --- a/api/routes.py +++ b/api/routes.py @@ -8977,9 +8977,11 @@ def _parse_msg_limit(raw): """Parse and clamp the ``?msg_limit=`` query value. Returns a positive int clamped to ``[1, _MAX_MSG_LIMIT]``, or ``None`` when - the value is absent/empty/malformed (the bare no-``msg_limit`` path, which - intentionally returns the full transcript for callers that need it). - Extracted from the handler so the clamp expression has direct test coverage. + the value is absent/empty/malformed. ``?msg_limit=all`` also returns + ``None`` — an explicit escape hatch for the full-transcript paths the + frontend genuinely needs; the handler distinguishes it from a bare request + via :func:`_resolve_effective_msg_limit`. Extracted from the handler so + the clamp expression has direct test coverage. """ if not raw: return None @@ -8990,6 +8992,26 @@ def _parse_msg_limit(raw): return max(1, min(value, _MAX_MSG_LIMIT)) +def _resolve_effective_msg_limit(raw_limit): + """Resolve the effective ``msg_limit`` for ``GET /api/session``. + + Returns ``(effective_limit, explicit_all)``. + + - numeric ``?msg_limit=N`` → clamped int (existing pagination). + - ``?msg_limit=all`` → ``(None, True)``: explicit full-transcript escape + hatch. Frontend paths that address rows by absolute transcript index + (outline jump, jump-to-start) genuinely need everything; they pass + ``all`` instead of relying on the bare no-limit shape. + - any other bare shape (no limit) → ``(None, False)``: the historical + full-transcript contract is preserved (contract tests pin tool-row + preservation and the runtime-journal snapshot on this shape), so the + bounded-window fix is enforced at the frontend call sites instead. + """ + explicit_all = str(raw_limit or "").strip().lower() == "all" + limit = _parse_msg_limit(raw_limit) + return limit, explicit_all + + # If a sidecar JSON file exceeds this threshold, the display-path tail # optimization fires regardless of message count. Sessions with few messages # but large tool outputs (multi-MB JSON) should not force a full-scan merge. @@ -13002,8 +13024,10 @@ def _handle_session_get(handler, parsed) -> bool: # payload; the existing _messages_truncated signal covers the clamped # case (the client sees there are more rows than returned). Parsing + # clamping live in _parse_msg_limit so the expression has direct test - # coverage; None means the bare no-msg_limit path (full transcript). - msg_limit = _parse_msg_limit(query.get("msg_limit", [None])[0]) + # coverage. The frontend recovery paths request a bounded tail + # explicitly (msg_limit=30), and the two absolute-index paths opt in to + # the full transcript via msg_limit=all (#7310/#7625). + _raw_msg_limit = query.get("msg_limit", [None])[0] # ?msg_before=N — 0-based index into the full message array. # Returns messages before this index (for scroll-to-top lazy loading). # Combined with msg_limit for paging. @@ -13012,6 +13036,9 @@ def _handle_session_get(handler, parsed) -> bool: msg_before = int(_msg_before) if _msg_before else None except (ValueError, TypeError): msg_before = None + msg_limit, _msg_limit_explicit_all = _resolve_effective_msg_limit( + _raw_msg_limit, + ) # ?expand_renderable=1 is retained for compatibility with older # frontends. msg_limit now counts visible transcript rows by default, so # the flag no longer changes the server-side pagination semantics. @@ -13221,9 +13248,13 @@ def _handle_session_get(handler, parsed) -> bool: _messages_offset = 0 # Index of the first returned message in the full message array. # Frontend uses this as cursor for scroll-to-top paging. + # Session-level tool_calls windowing keys off whether the returned + # message array was actually truncated (msg_before paging, any + # effective msg_limit) rather than whether a limit parameter was + # present — the full-transcript shape returns everything, so the + # length comparison alone decides (#7310/#7625). _windowed_messages = ( load_messages - and msg_limit is not None and (msg_before is not None or len(_truncated_msgs) < len(_all_msgs)) ) # Resolve effective context_length with model-metadata fallback so diff --git a/static/commands.js b/static/commands.js index e322c1d8..336b70c2 100644 --- a/static/commands.js +++ b/static/commands.js @@ -965,14 +965,18 @@ async function _runManualCompression(focusTopic){ // Preflight: verify the viewed session still exists before compressing. // This avoids a confusing "not found" toast when the UI is stale. try{ - const live=await api(`/api/session?session_id=${encodeURIComponent(sid)}`); + // Bounded tail: a bare preflight used to pull and re-redact the whole + // transcript before every manual compression (#7310/#7625). Session + // existence + the current tail is all this preflight needs. + const live=await api(`/api/session?session_id=${encodeURIComponent(sid)}&messages=1&resolve_model=0&msg_limit=30&expand_renderable=1`); if(!live||!live.session||live.session.session_id!==sid){ throw new Error('session no longer available'); } S.session=live.session; S.messages=live.session.messages||[]; S.toolCalls=live.session.tool_calls||[]; - if(typeof _messagesTruncated!=='undefined') _messagesTruncated=false; + if(typeof _messagesTruncated!=='undefined') _messagesTruncated=!!(live.session._messages_truncated); + if(typeof _oldestIdx!=='undefined') _oldestIdx=live.session._messages_offset||0; }catch(preflightErr){ if(typeof clearCompressionUi==='function') clearCompressionUi(); if(typeof _setCompressionSessionLock==='function') _setCompressionSessionLock(null); @@ -1748,11 +1752,13 @@ async function cmdRetry(){ const r=await api('/api/session/retry',{method:'POST',body:JSON.stringify({session_id:activeSid})}); if(r&&r.error){showToast(r.error);return;} if(!S.session||S.session.session_id!==activeSid)return; - const data=await api('/api/session?session_id='+encodeURIComponent(activeSid)); + // Bounded tail: a bare reload used to pull and re-redact the whole + // transcript on every /retry recovery (#7310/#7625). + const data=await api('/api/session?session_id='+encodeURIComponent(activeSid)+'&messages=1&resolve_model=0&msg_limit=30&expand_renderable=1'); // #5924 SILENT-race guard: a session switch during the GET await must not let // this recovery apply session A's intent to whatever session is now visible. if(!S.session||S.session.session_id!==activeSid)return; - if(data&&data.session){S.messages=data.session.messages||[];S.toolCalls=[];if(typeof clearLiveToolCards==='function')clearLiveToolCards();if(typeof _messagesTruncated!=='undefined')_messagesTruncated=false;renderMessages();} + if(data&&data.session){S.messages=data.session.messages||[];S.toolCalls=[];if(typeof clearLiveToolCards==='function')clearLiveToolCards();if(typeof _messagesTruncated!=='undefined')_messagesTruncated=!!(data.session._messages_truncated);if(typeof _oldestIdx!=='undefined')_oldestIdx=data.session._messages_offset||0;renderMessages();} $('msg').value=r.last_user_text||'';if(typeof autoResize==='function')autoResize(); // Re-arm the single-shot explicit-pick marker from the captured non-default // pick — but only if it's still safe at fire time (session unchanged, current @@ -1770,8 +1776,10 @@ async function cmdUndo(){ const r=await api('/api/session/undo',{method:'POST',body:JSON.stringify({session_id:activeSid})}); if(r&&r.error){showToast(r.error);return;} if(!S.session||S.session.session_id!==activeSid)return; - const data=await api('/api/session?session_id='+encodeURIComponent(activeSid)); - if(data&&data.session){S.messages=data.session.messages||[];S.toolCalls=[];if(typeof clearLiveToolCards==='function')clearLiveToolCards();if(typeof _messagesTruncated!=='undefined')_messagesTruncated=false;renderMessages();} + // Bounded tail: a bare reload used to pull and re-redact the whole + // transcript on every /undo recovery (#7310/#7625). + const data=await api('/api/session?session_id='+encodeURIComponent(activeSid)+'&messages=1&resolve_model=0&msg_limit=30&expand_renderable=1'); + if(data&&data.session){S.messages=data.session.messages||[];S.toolCalls=[];if(typeof clearLiveToolCards==='function')clearLiveToolCards();if(typeof _messagesTruncated!=='undefined')_messagesTruncated=!!(data.session._messages_truncated);if(typeof _oldestIdx!=='undefined')_oldestIdx=data.session._messages_offset||0;renderMessages();} showToast(`↩ ${t('undid_n_messages')} ${r.removed_count} ${t('undid_messages_suffix')}`); }catch(e){showToast(t('undo_failed')+e.message);} } diff --git a/static/messages.js b/static/messages.js index 92a10712..8764fbf0 100644 --- a/static/messages.js +++ b/static/messages.js @@ -6883,6 +6883,11 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ if(typeof _adoptRegenerationRevision==='function')_adoptRegenerationRevision(sessionPayload); _attachProjectedAnchorSceneToLastAssistant(_nextMsgs3018); S.messages=_carryForwardEphemeralTurnFields(S.messages||[], _nextMsgs3018); + // A bounded cancel-recovery reload returns a tail window: keep the + // Load-earlier paging gate honest instead of hardcoding "not truncated" + // (#7310/#7625). + if(typeof _messagesTruncated!=='undefined') _messagesTruncated=!!sessionPayload._messages_truncated; + if(typeof _oldestIdx!=='undefined') _oldestIdx=sessionPayload._messages_offset||0; if(typeof _hydrateTodosFromSession==='function') _hydrateTodosFromSession(S.session); clearLiveToolCards();if(!assistantText)removeThinking(); _markSessionViewed(activeSid, sessionPayload.message_count ?? S.messages.length); @@ -6902,8 +6907,10 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ if(_applyCancelSessionPayload(_cancelSessionPayload)) return; // Fetch latest session from server to get accurate message list (includes cancel status) // This ensures messages stay in sync with server, fixing race condition where local - // "*Task cancelled.*" message gets lost when done event overwrites S.messages - const data=await api(`/api/session?session_id=${encodeURIComponent(activeSid)}`); + // "*Task cancelled.*" message gets lost when done event overwrites S.messages. + // Bounded tail: a bare reload used to pull and re-redact the whole transcript + // on every cancel recovery (#7310/#7625). + const data=await api(`/api/session?session_id=${encodeURIComponent(activeSid)}&messages=1&resolve_model=0&msg_limit=30&expand_renderable=1`); if(data&&data.session) _applyCancelSessionPayload(data.session); }catch(_){ // Fallback to local cancel message if API fails @@ -6993,7 +7000,10 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ return returnStatus?'stale':false; } try{ - const data=await api(`/api/session?session_id=${encodeURIComponent(activeSid)}`); + // Bounded tail: a bare reload used to pull and re-redact the whole + // transcript on every stream-end settle/reconnect recovery (#7310/#7625). + // The Load-earlier paging gate is restored from the response below. + const data=await api(`/api/session?session_id=${encodeURIComponent(activeSid)}&messages=1&resolve_model=0&msg_limit=30&expand_renderable=1`); // Opus #2852 race-fix: if a late `done` event ran the finalize path while // we were awaiting the network roundtrip, bail out — done already settled. if(_streamFinalized) return returnStatus?'restored':true; @@ -7047,6 +7057,11 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ ? [..._stagedMessages,..._currentVisibleMessages.slice(_stagedMessages.length)] : _stagedMessages; S.messages=_filterRecoveryControlMessages(_resolvedMessages || []); + // Bounded settle reload returns a tail window: keep the Load-earlier + // paging gate honest instead of leaving the previous (possibly full) + // transcript's truncation state stale (#7310/#7625). + if(typeof _messagesTruncated!=='undefined') _messagesTruncated=!!session._messages_truncated; + if(typeof _oldestIdx!=='undefined') _oldestIdx=session._messages_offset||0; _attachProjectedAnchorSceneToLastAssistant(S.messages); if(typeof _hydrateTodosFromSession==='function') _hydrateTodosFromSession(S.session); if(S.session&&S.session.session_id){ diff --git a/static/outline.js b/static/outline.js index 45acffe2..8923775f 100644 --- a/static/outline.js +++ b/static/outline.js @@ -104,16 +104,14 @@ function _jumpToMessage(rawIdx) { } // Row is outside the render window — reload the full session and retry. - // Use the bare messages=1 path (no msg_limit) so the server returns the - // COMPLETE transcript: the target row is addressed by absolute index - // (msg-user-), so a bounded tail window would miss early rows. - // (A previous version sent msg_limit=9999 as a "give me everything" hack, - // but the server now clamps msg_limit, so the bare path is the correct way - // to request the full transcript here.) + // Request the COMPLETE transcript via the explicit msg_limit=all escape + // hatch: the target row is addressed by absolute index (msg-user-), + // so a bounded tail window would miss early rows. (The server now defaults + // bare requests to a bounded window, so the full transcript must be opt-in.) if (typeof api !== 'function') return; if (S.busy || S.activeStreamId) return; api('/api/session?session_id=' + encodeURIComponent(sid) + - '&messages=1&resolve_model=0') + '&messages=1&resolve_model=0&msg_limit=all') .then(function(data) { if (!data || !data.session) return; if (!S.session || S.session.session_id !== sid) return; // session switched diff --git a/static/sessions.js b/static/sessions.js index 7be28623..48249fbc 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -3904,7 +3904,7 @@ async function _ensureAllMessagesLoaded() { _loadingOlder = true; try { const sid = S.session.session_id; - const data = await api(`/api/session?session_id=${encodeURIComponent(sid)}&messages=1&resolve_model=0`, {timeoutMs:120000}); + const data = await api(`/api/session?session_id=${encodeURIComponent(sid)}&messages=1&resolve_model=0&msg_limit=all`, {timeoutMs:120000}); // Guard: api() may have redirected (401) and returned undefined. if (!data || !data.session) return; // Session may have been switched while we awaited. Bail rather than diff --git a/static/ui.js b/static/ui.js index 4a631482..8ad074bc 100644 --- a/static/ui.js +++ b/static/ui.js @@ -10135,7 +10135,11 @@ async function refreshSession() { dismissReconnect(); if (!S.session) return; try { - const data = await api(`/api/session?session_id=${encodeURIComponent(S.session.session_id)}`); + // Bounded tail (msg_limit=30) — a bare reload used to pull and re-redact + // the ENTIRE transcript on every offline/bfcache recovery (#7310/#7625). + // truncation signal + _oldestIdx are read below, so the Load-earlier + // paging gate still works after the windowed refresh. + const data = await api(`/api/session?session_id=${encodeURIComponent(S.session.session_id)}&messages=1&resolve_model=0&msg_limit=30&expand_renderable=1`); S.session = data.session; if(typeof _adoptRegenerationRevision==='function') _adoptRegenerationRevision(data.session); S.messages = data.session.messages || []; diff --git a/tests/test_session_default_window.py b/tests/test_session_default_window.py new file mode 100644 index 00000000..e3850879 --- /dev/null +++ b/tests/test_session_default_window.py @@ -0,0 +1,172 @@ +"""Tests: GET /api/session full-transcript escape hatch + bounded recovery +calls — #7310 / #7625. + +Bare ``/api/session`` reads (no ``msg_limit``) were issued by six frontend +recovery paths (offline/bfcache ``refreshSession``, stream-end settle, cancel +sync, /compress preflight, /retry, /undo). Each used to re-walk, re-redact and +re-serialize the ENTIRE transcript (4-15s / 28MB on a 5k-row session, measured +in #7625). Those call sites now request a bounded tail explicitly +(``msg_limit=30``), while the two paths that address rows by absolute +transcript index (outline jump, jump-to-start) opt in to the full transcript +via the explicit ``msg_limit=all`` escape hatch. + +The bare no-limit HTTP shape deliberately keeps its historical full-transcript +contract: upstream contract tests pin tool-row preservation +(``test_state_db_reconciliation_preserves_sidecar_order_when_timestamps_collide``) +and the runtime-journal live snapshot +(``test_paginated_session_followup_does_not_repeat_runtime_snapshot``) on that +shape, so the fix is enforced at the frontend call sites rather than by +changing the server default. +""" +from __future__ import annotations + +import re +from pathlib import Path + +from api.routes import ( + _MAX_MSG_LIMIT, + _parse_msg_limit, + _resolve_effective_msg_limit, +) + +_ROOT = Path(__file__).resolve().parents[1] + + +# ── helper: _resolve_effective_msg_limit ── + + +def test_bare_request_keeps_full_transcript_contract(): + """A bare reload (no msg_limit, no msg_before) keeps the historical + full-transcript contract — server-side default windowing was rejected + because contract tests pin tool-row preservation and the runtime-journal + snapshot on this shape (#7310/#7625).""" + limit, explicit_all = _resolve_effective_msg_limit(None) + assert limit is None + assert explicit_all is False + + +def test_empty_and_malformed_keep_contract(): + assert _resolve_effective_msg_limit("") == (None, False) + assert _resolve_effective_msg_limit("not-a-number") == (None, False) + + +def test_explicit_all_resolves_to_full_transcript(): + """"msg_limit=all → (None, True): the explicit full-transcript escape + hatch for outline jump / jump-to-start.""" + limit, explicit_all = _resolve_effective_msg_limit("all") + assert limit is None + assert explicit_all is True + + +def test_explicit_all_case_insensitive(): + assert _resolve_effective_msg_limit("ALL") == (None, True) + assert _resolve_effective_msg_limit(" All ") == (None, True) + + +def test_numeric_limit_unchanged(): + """Explicit numeric limits keep the clamp behaviour — real pagination is + unaffected.""" + assert _resolve_effective_msg_limit("30") == (30, False) + assert _resolve_effective_msg_limit("9999") == (_MAX_MSG_LIMIT, False) + assert _resolve_effective_msg_limit("0") == (1, False) + + +def test_parse_msg_limit_all_returns_none(): + """The parse helper returns None for 'all' — the handler distinguishes it + from a bare request via _resolve_effective_msg_limit.""" + assert _parse_msg_limit("all") is None + assert _parse_msg_limit("ALL") is None + + +# ── frontend call sites: bounded shapes (no bare GET /api/session left) ── + +_UI_JS = (_ROOT / "static" / "ui.js").read_text(encoding="utf-8") +_MESSAGES_JS = (_ROOT / "static" / "messages.js").read_text(encoding="utf-8") +_SESSIONS_JS = (_ROOT / "static" / "sessions.js").read_text(encoding="utf-8") +_COMMANDS_JS = (_ROOT / "static" / "commands.js").read_text(encoding="utf-8") +_OUTLINE_JS = (_ROOT / "static" / "outline.js").read_text(encoding="utf-8") + +_BOUNDED = "&messages=1&resolve_model=0&msg_limit=30&expand_renderable=1" +_ALL_QS = "&messages=1&resolve_model=0&msg_limit=all" + + +def test_refresh_session_uses_bounded_tail(): + """Offline/bfcache recovery refreshSession must not pull the full + transcript (#7310/#7625).""" + assert _BOUNDED in _UI_JS + + +def test_settle_and_cancel_paths_use_bounded_tail(): + """stream_end settle recovery AND cancel sync must not pull the full + transcript.""" + assert _BOUNDED in _MESSAGES_JS + assert _MESSAGES_JS.count(_BOUNDED) >= 2 + + +def test_cancel_payload_restores_truncation_signal(): + """_applyCancelSessionPayload keeps the Load-earlier paging gate honest + after a bounded cancel reload.""" + assert "!!sessionPayload._messages_truncated" in _MESSAGES_JS + assert "sessionPayload._messages_offset||0" in _MESSAGES_JS + + +def test_settle_restores_truncation_signal(): + """"_restoreSettledSession keeps the Load-earlier paging gate honest after + a bounded settle reload.""" + assert "!!session._messages_truncated" in _MESSAGES_JS + assert "session._messages_offset||0" in _MESSAGES_JS + + +def test_compress_preflight_uses_bounded_tail(): + """/compress preflight only needs session existence + the current tail.""" + assert _BOUNDED in _COMMANDS_JS + + +def test_retry_uses_bounded_tail(): + """/retry recovery must not pull the full transcript.""" + assert _COMMANDS_JS.count(_BOUNDED) >= 1 + + +def test_undo_uses_bounded_tail(): + """/undo recovery must not pull the full transcript.""" + assert _COMMANDS_JS.count(_BOUNDED) >= 2 + + +def test_recovery_paths_restore_truncation_signal(): + """preflight/retry/undo must NOT hardcode _messagesTruncated=false — they + read the server's truncation signal instead.""" + assert "!!(live.session._messages_truncated)" in _COMMANDS_JS + assert "!!(data.session._messages_truncated)" in _COMMANDS_JS + + +def test_jump_to_start_requests_explicit_full_transcript(): + """_ensureAllMessagesLoaded genuinely needs everything (absolute-index + addressing of the earliest rows) — it must opt in via msg_limit=all.""" + assert _ALL_QS in _SESSIONS_JS + + +def test_outline_jump_requests_explicit_full_transcript(): + """Outline jump addresses rows by absolute transcript index — it must opt + in via msg_limit=all.""" + assert _ALL_QS in _OUTLINE_JS + + +def test_no_bare_session_fetch_remains_in_frontend(): + """Regression guard: NO static/ caller may issue a bare GET /api/session + (no messages=/msg_limit= params). Use the bounded tail shape or the + explicit msg_limit=all escape hatch.""" + bare_template = re.compile( + r"api\(`/api/session\?session_id=\$\{encodeURIComponent\([^)]*\}\)`\)" + ) + bare_concat = re.compile( + r"api\('/api/session\?session_id='\s*\+\s*encodeURIComponent\([^)]*\)\)" + ) + for name, src in ( + ("ui.js", _UI_JS), + ("messages.js", _MESSAGES_JS), + ("sessions.js", _SESSIONS_JS), + ("commands.js", _COMMANDS_JS), + ("outline.js", _OUTLINE_JS), + ): + assert not bare_template.findall(src), f"{name} still has a bare template-literal /api/session fetch" + assert not bare_concat.findall(src), f"{name} still has a bare concatenated /api/session fetch" \ No newline at end of file From 73b7b949635d54295aaad0565f429b17da719e4e Mon Sep 17 00:00:00 2001 From: happy5318 Date: Fri, 18 Sep 2026 11:29:08 +0800 Subject: [PATCH 2/7] docs(outline): clarify full-transcript intent must be explicit --- static/outline.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/static/outline.js b/static/outline.js index 8923775f..0d2a361c 100644 --- a/static/outline.js +++ b/static/outline.js @@ -106,8 +106,9 @@ function _jumpToMessage(rawIdx) { // Row is outside the render window — reload the full session and retry. // Request the COMPLETE transcript via the explicit msg_limit=all escape // hatch: the target row is addressed by absolute index (msg-user-), - // so a bounded tail window would miss early rows. (The server now defaults - // bare requests to a bounded window, so the full transcript must be opt-in.) + // so a bounded tail window would miss early rows. The bare no-limit shape + // is not used here intentionally — full-transcript intent must be explicit + // so recovery paths stay bounded (#7310/#7625). if (typeof api !== 'function') return; if (S.busy || S.activeStreamId) return; api('/api/session?session_id=' + encodeURIComponent(sid) + From 39643e748f810629ab554a9fe60c09306ebc53e8 Mon Sep 17 00:00:00 2001 From: happy5318 Date: Fri, 18 Sep 2026 11:43:29 +0800 Subject: [PATCH 3/7] test: update cancel/settle source-shape assertions to bounded tail #7310/#7625: the cancel HTTP fallback and settle restore now request the bounded tail (msg_limit=30) instead of the bare full-transcript shape. test_cancelled_turn_status and test_live_to_final_anchor_visible_order assert the exact URL string inside the extracted cancel/settle blocks; the assertions' intent (embedded SSE payload applied BEFORE the HTTP fallback; fallback fetch precedes fallback apply) is unchanged, so the pinned strings are updated to the new bounded shape and the ordering contracts are untouched. Also moves the settle-path truncation-signal restore after the localStorage/update-url block so the 1800-char proximity window in test_session_rotate_url_sync keeps containing the tab-state sync lines. --- static/messages.js | 9 ++++----- tests/test_cancelled_turn_status.py | 4 +++- tests/test_live_to_final_anchor_visible_order.py | 5 ++++- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/static/messages.js b/static/messages.js index 8764fbf0..0397738c 100644 --- a/static/messages.js +++ b/static/messages.js @@ -7057,17 +7057,16 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ ? [..._stagedMessages,..._currentVisibleMessages.slice(_stagedMessages.length)] : _stagedMessages; S.messages=_filterRecoveryControlMessages(_resolvedMessages || []); - // Bounded settle reload returns a tail window: keep the Load-earlier - // paging gate honest instead of leaving the previous (possibly full) - // transcript's truncation state stale (#7310/#7625). - if(typeof _messagesTruncated!=='undefined') _messagesTruncated=!!session._messages_truncated; - if(typeof _oldestIdx!=='undefined') _oldestIdx=session._messages_offset||0; _attachProjectedAnchorSceneToLastAssistant(S.messages); if(typeof _hydrateTodosFromSession==='function') _hydrateTodosFromSession(S.session); if(S.session&&S.session.session_id){ try{localStorage.setItem('hermes-webui-session',S.session.session_id);}catch(_){} if(typeof _setActiveSessionUrl==='function') _setActiveSessionUrl(S.session.session_id); } + // Bounded settle reload returns a tail window: keep the Load-earlier + // paging gate honest (#7310/#7625). + if(typeof _messagesTruncated!=='undefined') _messagesTruncated=!!session._messages_truncated; + if(typeof _oldestIdx!=='undefined') _oldestIdx=session._messages_offset||0; if(typeof _adoptRegenerationRevision==='function')_adoptRegenerationRevision(session); const _markerOnlyAssistantError=_replaceMarkerOnlyAssistantWithStreamError(S.messages); if(_markerOnlyAssistantError&&typeof showToast==='function') showToast('No response received after context compression. Please retry.',5000,'error'); diff --git a/tests/test_cancelled_turn_status.py b/tests/test_cancelled_turn_status.py index bfeebe5b..df9554cb 100644 --- a/tests/test_cancelled_turn_status.py +++ b/tests/test_cancelled_turn_status.py @@ -219,7 +219,9 @@ class TestCancelledTurnPersistenceGuards: assert "const _applyCancelSessionPayload=(sessionPayload)=>" in block assert "const _cancelSessionPayload=_cancelData&&typeof _cancelData.session==='object'?_cancelData.session:null;" in block assert "if(_applyCancelSessionPayload(_cancelSessionPayload)) return;" in block - assert "const data=await api(`/api/session?session_id=${encodeURIComponent(activeSid)}`);" in block + # #7310/#7625: the HTTP fallback is a bounded tail now — the full + # transcript was being re-walked/re-redacted on every cancel recovery. + assert "const data=await api(`/api/session?session_id=${encodeURIComponent(activeSid)}&messages=1&resolve_model=0&msg_limit=30&expand_renderable=1`);" in block assert block.index("if(_applyCancelSessionPayload(_cancelSessionPayload)) return;") < block.index("const data=await api("), ( "Cancel handler must apply the terminal SSE session payload before falling back " "to /api/session so captured _partial reasoning/tool rows are visible immediately." diff --git a/tests/test_live_to_final_anchor_visible_order.py b/tests/test_live_to_final_anchor_visible_order.py index 589ed37b..77ae81e7 100644 --- a/tests/test_live_to_final_anchor_visible_order.py +++ b/tests/test_live_to_final_anchor_visible_order.py @@ -829,7 +829,10 @@ def test_cancel_settlement_attaches_projected_anchor_scene_before_render(): assert fetch_idx < attach_idx < carry_idx < render_idx embedded_idx = cancel.index("if(_applyCancelSessionPayload(_cancelSessionPayload)) return;") - fallback_get_idx = cancel.index("const data=await api(`/api/session?session_id=${encodeURIComponent(activeSid)}`);") + # #7310/#7625: the HTTP fallback is a bounded tail now (full-transcript + # reloads were stacking on every cancel recovery); ordering contract below + # is unchanged. + fallback_get_idx = cancel.index("const data=await api(`/api/session?session_id=${encodeURIComponent(activeSid)}&messages=1&resolve_model=0&msg_limit=30&expand_renderable=1`);") fallback_apply_idx = cancel.index("if(data&&data.session) _applyCancelSessionPayload(data.session);") assert embedded_idx < fallback_get_idx < fallback_apply_idx From f4b28346e5d2050fc9051f4cd981cab3cbf50924 Mon Sep 17 00:00:00 2001 From: happy5318 Date: Fri, 18 Sep 2026 22:00:45 +0800 Subject: [PATCH 4/7] fix(session): keep bounded-tail recovery suffix and paging offset Bounded settle/cancel reloads return the latest 30 messages, not a prefix of the visible window. The shorter-terminal preserve path only compared prefixes, so a long session dropped the Connection-interrupted marker. Anchor persist also ran before _oldestIdx was refreshed from the bounded response, writing the scene against the wrong transcript row. - Treat a suffix match like a prefix match when preserving the terminal recovery marker - Refresh _messagesTruncated / _oldestIdx before attaching/persisting the projected scene on settle, cancel, and apperror - Document the msg_limit=30 / msg_limit=all contract - Add runtime + source-order regression tests --- ARCHITECTURE.md | 13 +- static/messages.js | 54 +++++--- ...5224_terminal_error_transcript_preserve.py | 124 +++++++++++++++++- ...test_live_to_final_anchor_visible_order.py | 7 +- tests/test_session_default_window.py | 47 ++++++- 5 files changed, 224 insertions(+), 21 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 72be8bc8..3fb292ff 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -511,7 +511,9 @@ to the active conversation rather than a global app setting. Session management: newSession() POST /api/session/new, update S.session, save to localStorage - loadSession(sid) GET /api/session?session_id=X, check INFLIGHT first, update S + loadSession(sid) GET /api/session?session_id=X (initial load uses the + bounded tail `msg_limit=30`; jump-to-start and outline + jump pass `msg_limit=all`), check INFLIGHT first, update S deleteSession(sid) POST /api/session/delete, handle active/inactive cases correctly renderSessionList() GET /api/sessions, rebuild #sessionList DOM @@ -1379,7 +1381,14 @@ Complete list of all HTTP endpoints as of Sprint 1 (v0.3). / Returns full HTML app (index page) /index.html Same as / /health {"status":"ok","sessions":N} - /api/session ?session_id=X -> full session + messages. 400 if no ID. + /api/session ?session_id=X -> session + messages. 400 if no ID. + Bare (no msg_limit) keeps the historical full-transcript + contract. Recovery paths request a bounded tail + (`msg_limit=30`) and restore `_messages_truncated` / + `_messages_offset` before persisting anchor-scene + metadata. Outline jump and jump-to-start opt in to the + full transcript via the explicit `msg_limit=all` + escape hatch. See #7310 / #7625 / #7628. /api/sessions List of all session compact() dicts, sorted by updated_at /api/list ?session_id=X&path=. -> directory listing for session workspace /api/file ?session_id=X&path=rel -> file content (text, 200KB limit) diff --git a/static/messages.js b/static/messages.js index 0397738c..a4555bbf 100644 --- a/static/messages.js +++ b/static/messages.js @@ -6640,6 +6640,10 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ S.session=d.session; const _nextMsgs3018=(d.session.messages||[]).filter(m=>m&&m.role); if(typeof _adoptRegenerationRevision==='function')_adoptRegenerationRevision(d.session); + // Same persist-before-offset trap as settle/cancel: refresh paging + // before attaching the projected scene (#7628). + if(typeof _messagesTruncated!=='undefined') _messagesTruncated=!!d.session._messages_truncated; + if(typeof _oldestIdx!=='undefined') _oldestIdx=d.session._messages_offset||0; _attachProjectedAnchorSceneToLastAssistant(_nextMsgs3018); S.messages=_carryForwardEphemeralTurnFields(S.messages||[], _nextMsgs3018); if(S.session&&S.session.session_id){ @@ -6881,13 +6885,13 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ S.session=sessionPayload; const _nextMsgs3018=(sessionPayload.messages||[]).filter(m=>m&&m.role); if(typeof _adoptRegenerationRevision==='function')_adoptRegenerationRevision(sessionPayload); - _attachProjectedAnchorSceneToLastAssistant(_nextMsgs3018); - S.messages=_carryForwardEphemeralTurnFields(S.messages||[], _nextMsgs3018); // A bounded cancel-recovery reload returns a tail window: keep the - // Load-earlier paging gate honest instead of hardcoding "not truncated" - // (#7310/#7625). + // Load-earlier paging gate honest, and refresh _oldestIdx BEFORE + // persisting the projected scene (#7310/#7625/#7628). if(typeof _messagesTruncated!=='undefined') _messagesTruncated=!!sessionPayload._messages_truncated; if(typeof _oldestIdx!=='undefined') _oldestIdx=sessionPayload._messages_offset||0; + _attachProjectedAnchorSceneToLastAssistant(_nextMsgs3018); + S.messages=_carryForwardEphemeralTurnFields(S.messages||[], _nextMsgs3018); if(typeof _hydrateTodosFromSession==='function') _hydrateTodosFromSession(S.session); clearLiveToolCards();if(!assistantText)removeThinking(); _markSessionViewed(activeSid, sessionPayload.message_count ?? S.messages.length); @@ -7037,6 +7041,16 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ const _nextMsgs3018=(session.messages||[]).filter(m=>m&&m.role); const _currentMessages=Array.isArray(S.messages)?S.messages:[]; const _currentVisibleMessages=_filterRecoveryControlMessages(_currentMessages || []); + // Restore paging from the bounded tail BEFORE attaching/persisting the + // projected scene: _persistSettledAnchorScene reads _oldestIdx to + // compute the absolute transcript index (#7628). + if(typeof _messagesTruncated!=='undefined') _messagesTruncated=!!session._messages_truncated; + if(typeof _oldestIdx!=='undefined') _oldestIdx=session._messages_offset||0; + if(typeof _adoptRegenerationRevision==='function')_adoptRegenerationRevision(session); + if(S.session&&S.session.session_id){ + try{localStorage.setItem('hermes-webui-session',S.session.session_id);}catch(_){} + if(typeof _setActiveSessionUrl==='function') _setActiveSessionUrl(S.session.session_id); + } const _stagedMessages=_carryForwardEphemeralTurnFields(_currentMessages, _nextMsgs3018); const _currentVisibleEndsWithTerminalMarker=( _currentVisibleMessages.length>0 && @@ -7052,22 +7066,32 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ return !!stagedKey && stagedKey===currentKey; }) ); - const _preserveCurrentTranscript=preserveVisibleOnShorterTerminalSnapshot&&_stagedMatchesCurrentPrefix; + // Bounded settle returns a suffix of the durable transcript, not a + // prefix of the currently visible window. On a long session the + // prefix check fails and would drop the terminal recovery marker + // (#7628). Treat a suffix match the same as a prefix match. + const _durableVisibleCount=_currentVisibleEndsWithTerminalMarker + ? _currentVisibleMessages.length-1 + : _currentVisibleMessages.length; + const _stagedSuffixStart=_durableVisibleCount-_stagedMessages.length; + const _stagedMatchesCurrentSuffix=( + _stagedMessages.length>0 && + _stagedSuffixStart>=0 && + _stagedMessages.length<_currentVisibleMessages.length && + _currentVisibleEndsWithTerminalMarker && + _stagedMessages.every((message, idx)=>{ + const stagedKey=_messageIdentityKey(message); + const currentKey=_messageIdentityKey(_currentVisibleMessages[_stagedSuffixStart+idx]); + return !!stagedKey && stagedKey===currentKey; + }) + ); + const _preserveCurrentTranscript=preserveVisibleOnShorterTerminalSnapshot&&(_stagedMatchesCurrentPrefix||_stagedMatchesCurrentSuffix); const _resolvedMessages=_preserveCurrentTranscript - ? [..._stagedMessages,..._currentVisibleMessages.slice(_stagedMessages.length)] + ? [..._stagedMessages,..._currentVisibleMessages.slice(_stagedMatchesCurrentPrefix?_stagedMessages.length:_stagedSuffixStart+_stagedMessages.length)] : _stagedMessages; S.messages=_filterRecoveryControlMessages(_resolvedMessages || []); _attachProjectedAnchorSceneToLastAssistant(S.messages); if(typeof _hydrateTodosFromSession==='function') _hydrateTodosFromSession(S.session); - if(S.session&&S.session.session_id){ - try{localStorage.setItem('hermes-webui-session',S.session.session_id);}catch(_){} - if(typeof _setActiveSessionUrl==='function') _setActiveSessionUrl(S.session.session_id); - } - // Bounded settle reload returns a tail window: keep the Load-earlier - // paging gate honest (#7310/#7625). - if(typeof _messagesTruncated!=='undefined') _messagesTruncated=!!session._messages_truncated; - if(typeof _oldestIdx!=='undefined') _oldestIdx=session._messages_offset||0; - if(typeof _adoptRegenerationRevision==='function')_adoptRegenerationRevision(session); const _markerOnlyAssistantError=_replaceMarkerOnlyAssistantWithStreamError(S.messages); if(_markerOnlyAssistantError&&typeof showToast==='function') showToast('No response received after context compression. Please retry.',5000,'error'); const hasMessageToolMetadata=S.messages.some(m=>{ diff --git a/tests/test_issue5224_terminal_error_transcript_preserve.py b/tests/test_issue5224_terminal_error_transcript_preserve.py index 29ac384d..1a381f6b 100644 --- a/tests/test_issue5224_terminal_error_transcript_preserve.py +++ b/tests/test_issue5224_terminal_error_transcript_preserve.py @@ -109,7 +109,6 @@ function buildRuntime() { globalThis.removeThinking = () => calls.push('removeThinking'); globalThis._flushReasoningToAnchor = () => calls.push('flushReasoning'); globalThis._applyToAnchor = () => calls.push('applyToAnchor'); - globalThis._attachProjectedAnchorSceneToLastAssistant = () => calls.push('attachProjected'); globalThis._hydrateTodosFromSession = () => calls.push('hydrateTodos'); globalThis._scheduleAnchorRegistryCleanup = () => calls.push('scheduleAnchorRegistryCleanup'); globalThis._smdEndParser = () => calls.push('smdEndParser'); @@ -141,6 +140,15 @@ function buildRuntime() { globalThis._messageRenderWindowSize = 20; globalThis._streamFinalized = !!scenario.streamFinalized; globalThis._persistTimer = null; + globalThis._oldestIdx = Number.isFinite(scenario.oldestIdx) ? scenario.oldestIdx : 0; + globalThis._messagesTruncated = !!scenario.messagesTruncated; + globalThis.oldestIdxAtAttach = null; + globalThis.messagesTruncatedAtAttach = null; + globalThis._attachProjectedAnchorSceneToLastAssistant = () => { + calls.push('attachProjected'); + globalThis.oldestIdxAtAttach = globalThis._oldestIdx; + globalThis.messagesTruncatedAtAttach = globalThis._messagesTruncated; + }; globalThis.api = async () => scenario.apiPayload || { session: null }; globalThis.msgContent = undefined; globalThis._isPreservedCompressionTaskListMarkerOnlyText = () => false; @@ -162,6 +170,10 @@ function buildRuntime() { status, messages: messages.map((m) => ({ role: m.role, content: m.content })), terminalMarkerCount, + oldestIdx: globalThis._oldestIdx, + oldestIdxAtAttach: globalThis.oldestIdxAtAttach, + messagesTruncated: !!globalThis._messagesTruncated, + messagesTruncatedAtAttach: !!globalThis.messagesTruncatedAtAttach, calls, })); return; @@ -179,6 +191,10 @@ function buildRuntime() { status, messages: messages.map((m) => ({ role: m.role, content: m.content })), terminalMarkerCount, + oldestIdx: globalThis._oldestIdx, + oldestIdxAtAttach: globalThis.oldestIdxAtAttach, + messagesTruncated: !!globalThis._messagesTruncated, + messagesTruncatedAtAttach: !!globalThis.messagesTruncatedAtAttach, calls, })); return; @@ -421,3 +437,109 @@ def test_terminal_error_marker_is_single_instance_and_not_duplicated(driver_path assert outcome["terminalMarkerCount"] == 1, f"terminal marker should be deduped to one, got {outcome['terminalMarkerCount']}" assert outcome["messages"][-1]["content"].startswith("**Connection interrupted:**") + + +def test_bounded_tail_preserves_terminal_marker_when_server_returns_suffix(driver_path): + """A bounded settle tail is a suffix of the visible window, not a prefix. + + Long-session recovery used to compare the 30-row tail against the start of + S.messages. The prefix check failed, so preserveVisibleOnShorterTerminalSnapshot + dropped the client-side Connection-interrupted marker (#7628). + """ + historical = [] + for i in range(2): + historical.append({"role": "user", "content": f"Earlier question {i}", "_ts": f"u{i}"}) + historical.append({"role": "assistant", "content": f"Earlier answer {i}", "_ts": f"a{i}"}) + tail = [ + {"role": "user", "content": "Current question", "_ts": "u-cur"}, + {"role": "assistant", "content": "Current fragment one", "_ts": "a-cur"}, + ] + terminal = { + "role": "assistant", + "content": "**Connection interrupted:** The browser lost the live SSE connection before the response finished.", + "_ts": "err", + } + outcome = _run_scenario(driver_path, { + "action": "restore_shorter_terminal", + "oldestIdx": 0, + "messagesTruncated": False, + "state": { + "session": {"session_id": "session-5224", "message_count": 7}, + "messages": historical + tail + [terminal], + "activeStreamId": "stream-5224", + }, + "apiPayload": { + "session": { + "session_id": "session-5224", + "active_stream_id": None, + "pending_user_message": None, + "messages": tail, + "_messages_truncated": True, + "_messages_offset": 4, + }, + }, + "activeSid": "session-5224", + "streamId": "stream-5224", + "isActiveSession": True, + "isSessionCurrentPane": True, + "isSessionActivelyViewed": False, + }) + + observed = [(item["role"], item["content"]) for item in outcome["messages"]] + assert observed == [ + ("user", "Current question"), + ("assistant", "Current fragment one"), + ("assistant", "**Connection interrupted:** The browser lost the live SSE connection before the response finished."), + ], f"bounded tail must keep the terminal recovery suffix: {observed}" + assert outcome["terminalMarkerCount"] == 1, ( + f"expected the terminal marker to survive a suffix-only tail, got {outcome['terminalMarkerCount']}" + ) + assert outcome["oldestIdx"] == 4, f"paging offset should come from the bounded response, got {outcome['oldestIdx']}" + assert outcome["oldestIdxAtAttach"] == 4, ( + f"anchor persist must see the new offset, got {outcome['oldestIdxAtAttach']}" + ) + assert outcome["messagesTruncated"] is True + assert outcome["messagesTruncatedAtAttach"] is True + + +def test_settle_refreshes_paging_offset_before_anchor_persist(driver_path): + """_persistSettledAnchorScene reads _oldestIdx; a stale 0 would write the scene to the wrong row.""" + outcome = _run_scenario(driver_path, { + "action": "restore_fuller_terminal", + "oldestIdx": 0, + "messagesTruncated": False, + "state": { + "session": {"session_id": "session-5224", "message_count": 1}, + "messages": [ + {"role": "assistant", "content": "Old interrupted shell", "_ts": "old"}, + ], + "activeStreamId": "stream-5224", + }, + "apiPayload": { + "session": { + "session_id": "session-5224", + "active_stream_id": None, + "pending_user_message": None, + "messages": [ + {"role": "user", "content": "Question about data?", "_ts": "u1"}, + {"role": "assistant", "content": "Settled first segment", "_ts": "a1"}, + {"role": "assistant", "content": "Settled second segment", "_ts": "a2"}, + {"role": "assistant", "content": "Settled third segment", "_ts": "a3"}, + ], + "_messages_truncated": True, + "_messages_offset": 40, + }, + }, + "activeSid": "session-5224", + "streamId": "stream-5224", + "isActiveSession": True, + "isSessionCurrentPane": True, + "isSessionActivelyViewed": False, + }) + + assert outcome["status"] == "restored" + assert outcome["oldestIdx"] == 40 + assert outcome["oldestIdxAtAttach"] == 40, ( + f"stale offset 0 must not leak into anchor persist, got {outcome['oldestIdxAtAttach']}" + ) + assert outcome["messagesTruncatedAtAttach"] is True diff --git a/tests/test_live_to_final_anchor_visible_order.py b/tests/test_live_to_final_anchor_visible_order.py index 77ae81e7..90dc5c38 100644 --- a/tests/test_live_to_final_anchor_visible_order.py +++ b/tests/test_live_to_final_anchor_visible_order.py @@ -816,17 +816,20 @@ def test_stream_end_restore_attaches_projected_anchor_scene_before_render(): filter_idx = restore.index("S.messages=_filterRecoveryControlMessages(_resolvedMessages || []);") attach_idx = restore.index("_attachProjectedAnchorSceneToLastAssistant(S.messages);") render_idx = restore.index("syncTopbar();renderMessages({preserveScroll:true})") - assert carry_idx < filter_idx < attach_idx < render_idx + offset_idx = restore.index("_oldestIdx=session._messages_offset||0") + assert offset_idx < carry_idx < filter_idx < attach_idx < render_idx + assert "_stagedMatchesCurrentSuffix" in restore def test_cancel_settlement_attaches_projected_anchor_scene_before_render(): cancel = _event_listener_body(MESSAGES_JS, "cancel") fetch_idx = cancel.index("const _nextMsgs3018=(sessionPayload.messages||[]).filter(m=>m&&m.role);") + offset_idx = cancel.index("_oldestIdx=sessionPayload._messages_offset||0") attach_idx = cancel.index("_attachProjectedAnchorSceneToLastAssistant(_nextMsgs3018);") carry_idx = cancel.index("S.messages=_carryForwardEphemeralTurnFields(S.messages||[], _nextMsgs3018);") render_idx = cancel.index("renderMessages({preserveScroll:true});") - assert fetch_idx < attach_idx < carry_idx < render_idx + assert fetch_idx < offset_idx < attach_idx < carry_idx < render_idx embedded_idx = cancel.index("if(_applyCancelSessionPayload(_cancelSessionPayload)) return;") # #7310/#7625: the HTTP fallback is a bounded tail now (full-transcript diff --git a/tests/test_session_default_window.py b/tests/test_session_default_window.py index e3850879..8e94e455 100644 --- a/tests/test_session_default_window.py +++ b/tests/test_session_default_window.py @@ -111,12 +111,57 @@ def test_cancel_payload_restores_truncation_signal(): def test_settle_restores_truncation_signal(): - """"_restoreSettledSession keeps the Load-earlier paging gate honest after + """_restoreSettledSession keeps the Load-earlier paging gate honest after a bounded settle reload.""" assert "!!session._messages_truncated" in _MESSAGES_JS assert "session._messages_offset||0" in _MESSAGES_JS +def _restore_settled_session_body(): + start = _MESSAGES_JS.index("async function _restoreSettledSession") + end = _MESSAGES_JS.index("function _handleStreamError", start) + return _MESSAGES_JS[start:end] + + +def _apply_cancel_session_payload_body(): + start = _MESSAGES_JS.index("const _applyCancelSessionPayload=(sessionPayload)=>") + end = _MESSAGES_JS.index("Prefer the canonical session snapshot", start) + return _MESSAGES_JS[start:end] + + +def test_settle_refreshes_paging_before_anchor_persist(): + """Anchor persist reads _oldestIdx; a bounded tail must refresh it first (#7628).""" + restore = _restore_settled_session_body() + offset_idx = restore.index("_oldestIdx=session._messages_offset||0") + attach_idx = restore.index("_attachProjectedAnchorSceneToLastAssistant(S.messages);") + assert offset_idx < attach_idx + + +def test_cancel_refreshes_paging_before_anchor_persist(): + """Cancel recovery has the same persist-before-offset trap as settle (#7628).""" + cancel = _apply_cancel_session_payload_body() + offset_idx = cancel.index("_oldestIdx=sessionPayload._messages_offset||0") + attach_idx = cancel.index("_attachProjectedAnchorSceneToLastAssistant(_nextMsgs3018);") + assert offset_idx < attach_idx + + +def test_settle_preserves_terminal_marker_against_bounded_suffix(): + """Long-session bounded tails are suffixes of S.messages, not prefixes (#7628).""" + restore = _restore_settled_session_body() + assert "_stagedMatchesCurrentSuffix" in restore + assert "_preserveCurrentTranscript=preserveVisibleOnShorterTerminalSnapshot&&(_stagedMatchesCurrentPrefix||_stagedMatchesCurrentSuffix)" in restore + + +def test_apperror_embedded_session_refreshes_paging_before_anchor_persist(): + """The apperror embedded-session path has the same persist-before-offset trap (#7628).""" + start = _MESSAGES_JS.index("source.addEventListener('apperror'") + end = _MESSAGES_JS.index("source.addEventListener('warning'", start) + block = _MESSAGES_JS[start:end] + offset_idx = block.index("_oldestIdx=d.session._messages_offset||0") + attach_idx = block.index("_attachProjectedAnchorSceneToLastAssistant(_nextMsgs3018);") + assert offset_idx < attach_idx + + def test_compress_preflight_uses_bounded_tail(): """/compress preflight only needs session existence + the current tail.""" assert _BOUNDED in _COMMANDS_JS From 95c174f038313184837bb6984cdfb3ef9c19fa22 Mon Sep 17 00:00:00 2001 From: happy5318 Date: Sat, 19 Sep 2026 12:55:40 +0800 Subject: [PATCH 5/7] fix(session): restore paging after compress; suffix-exclusive settle strategy (review round 3) Two data-integrity blockers from round-3 review (nesquena-hermes 2026-09-19): 1. [CORE] A successful /compress returns the full transcript, but _applyManualCompressionResult kept the bounded preflight's paging signals (_messagesTruncated/_oldestIdx). Render now restores them before renderMessages(), same ordering as settle/cancel paths. 2. [SILENT] _restoreSettledSession preservation used (prefix || suffix), so repeated identical turns (colliding _messageIdentityKey) made BOTH comparisons succeed and the prefix branch spliced at the wrong offset - silently dropping rows and duplicating others. The server truncation signal now decides the strategy: truncated => suffix-exclusive, untruncated => prefix. Tests: node behavioral driver covering /compress paging restore, repeated-turn settle (no drop/dup, authoritative offset), and gap-free msg_before paging after bounded recovery. --- static/commands.js | 6 + static/messages.js | 12 +- tests/test_7628_recovery_paging.py | 363 +++++++++++++++++++++++++++ tests/test_session_default_window.py | 14 +- 4 files changed, 392 insertions(+), 3 deletions(-) create mode 100644 tests/test_7628_recovery_paging.py diff --git a/static/commands.js b/static/commands.js index 336b70c2..869d64c2 100644 --- a/static/commands.js +++ b/static/commands.js @@ -873,6 +873,12 @@ async function _applyManualCompressionResult(data, focusTopic, visibleCount, com clearLiveToolCards(); try{localStorage.setItem('hermes-webui-session',S.session.session_id);}catch(_){} if(typeof _setActiveSessionUrl==='function') _setActiveSessionUrl(S.session.session_id); + // Restore paging signals from the (possibly full) transcript response. + // A successful /compress returns the full transcript, so the bounded + // preflight's _messagesTruncated/_oldestIdx must be reset before render + // (#7628). Same restore-before-render ordering as the settle/cancel paths. + if(typeof _messagesTruncated!=='undefined') _messagesTruncated=!!data.session._messages_truncated; + if(typeof _oldestIdx!=='undefined') _oldestIdx=data.session._messages_offset||0; syncTopbar(); renderMessages(); await renderSessionList(); diff --git a/static/messages.js b/static/messages.js index a4555bbf..8295794d 100644 --- a/static/messages.js +++ b/static/messages.js @@ -7085,9 +7085,17 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ return !!stagedKey && stagedKey===currentKey; }) ); - const _preserveCurrentTranscript=preserveVisibleOnShorterTerminalSnapshot&&(_stagedMatchesCurrentPrefix||_stagedMatchesCurrentSuffix); + // The server's truncation signal decides the strategy, not an `||`: + // a bounded settle returns a SUFFIX of the durable transcript, so + // suffix matching is the only correct strategy when truncated. Prefix + // preservation is reserved for untruncated snapshots. When repeated + // identical turns make BOTH comparisons succeed, preferring the prefix + // splices at the wrong offset and silently drops/duplicates rows + // (#7628). Never prefer prefix when truncation is active. + const _truncatedRecovery=!!_messagesTruncated||!!(_oldestIdx>0); + const _preserveCurrentTranscript=preserveVisibleOnShorterTerminalSnapshot&&(_truncatedRecovery?_stagedMatchesCurrentSuffix:_stagedMatchesCurrentPrefix); const _resolvedMessages=_preserveCurrentTranscript - ? [..._stagedMessages,..._currentVisibleMessages.slice(_stagedMatchesCurrentPrefix?_stagedMessages.length:_stagedSuffixStart+_stagedMessages.length)] + ? [..._stagedMessages,..._currentVisibleMessages.slice(_truncatedRecovery?_stagedSuffixStart+_stagedMessages.length:_stagedMessages.length)] : _stagedMessages; S.messages=_filterRecoveryControlMessages(_resolvedMessages || []); _attachProjectedAnchorSceneToLastAssistant(S.messages); diff --git a/tests/test_7628_recovery_paging.py b/tests/test_7628_recovery_paging.py new file mode 100644 index 00000000..64885907 --- /dev/null +++ b/tests/test_7628_recovery_paging.py @@ -0,0 +1,363 @@ +"""Behavioral tests for #7628 review round: /compress paging restore + suffix +strategy for repeated identical turns. + +Round-3 review (nesquena-hermes 2026-09-19) found two data-integrity blockers: + +1. [CORE] A successful /compress returns the FULL transcript, but + `_applyManualCompressionResult` did not restore `_messagesTruncated` / + `_oldestIdx` — the bounded preflight's values (e.g. truncated=true, + oldestIdx=70) survived, producing a false "70 older messages" affordance + and shifting every absolute transcript index by 70. + +2. [SILENT] `_restoreSettledSession` preservation used + `(_stagedMatchesCurrentPrefix || _stagedMatchesCurrentSuffix)`, so when + identical repeated turns made BOTH comparisons succeed the prefix branch + spliced at the wrong offset — silently dropping and duplicating rows. + The server truncation signal now decides: truncated => suffix-exclusive. + +Node-backed tests extract the real functions from static/ and drive them +against a stub DOM (same harness pattern as test_issue5224). +""" +from __future__ import annotations + +import json +import shutil +import subprocess +from pathlib import Path + +import pytest + +_ROOT = Path(__file__).resolve().parents[1] +_COMMANDS_JS = _ROOT / "static" / "commands.js" +_MESSAGES_JS = _ROOT / "static" / "messages.js" +_SESSIONS_JS = _ROOT / "static" / "sessions.js" +_NODE = shutil.which("node") + +pytestmark = pytest.mark.skipif(_NODE is None, reason="node is required to execute message runtime tests") + + +_DRIVER = r""" +const fs = require('fs'); +const commandsSrc = fs.readFileSync(process.argv[2], 'utf8'); +const messagesSrc = fs.readFileSync(process.argv[3], 'utf8'); +const sessionsSrc = fs.readFileSync(process.argv[4], 'utf8'); +const scenario = JSON.parse(process.argv[5] || '{}'); + +function extractFunction(source, name) { + const markers = [`async function ${name}(`, `function ${name}(${name}`, `function ${name}(`, `${name}=async(`, `${name}=async (`]; + let start = -1; + for (const marker of markers) { + start = source.indexOf(marker); + if (start >= 0) break; + } + if (start < 0) throw new Error(`missing ${name}`); + let i = source.indexOf('{', start); + if (i < 0) throw new Error(`missing body for ${name}`); + let depth = 1; i++; + while (i < source.length) { + const ch = source[i]; + if (ch === '{') depth++; + if (ch === '}') { depth--; if (depth === 0) return source.slice(start, i + 1); } + i++; + } + throw new Error(`unterminated body for ${name}`); +} + +function install(name, src) { + const body = extractFunction(src, name); + // Drop the `async function name(` or `function name(` prefix, keep body. + const eq = body.indexOf('{'); + const inner = body.slice(eq); + globalThis[name] = new Function(`return (${name === 'jumpToSessionStart' ? 'async function' : 'function'} ${inner})`)(); +} + +// ---- helpers from messages.js (mirror test_issue5224 set) ---- +const MESSAGE_HELPERS = [ + "_isMarkerOnlyAssistantMessage", "_streamRecoveryControlMessageText", + "_streamRecoveryControlMessage", "_filterRecoveryControlMessages", + "_replaceMarkerOnlyAssistantWithStreamError", "_messageIdentityKey", + "_isHistoricalAnchorActivityScene", "_carryForwardEphemeralTurnFields", + "_isTerminalStreamErrorMarkerMessage", "_restoreSettledSession", + "_mergeSettledToolCallsWithLiveMetadata", +]; +for (const h of MESSAGE_HELPERS) { + const body = extractFunction(messagesSrc, h); + const factory = new Function(`${body}; return ${h};`); + globalThis[h] = factory(); +} + +// ---- compress result applier from commands.js ---- +const COMPRESS_BODY = extractFunction(commandsSrc, '_applyManualCompressionResult'); +// Rebuild as a normal function (it contains `await` – keep async). +globalThis._applyManualCompressionResult = new Function( + 'return (' + COMPRESS_BODY + ')' +)(); + +function runtimeStubs() { + const calls = []; + globalThis.S = { session: { session_id: 's1' }, messages: [], toolCalls: [] }; + globalThis.activeSid = 's1'; + globalThis.streamId = 'stream-1'; + globalThis.assistantText = false; + globalThis._messagesTruncated = !!scenario.prestateTruncated; + globalThis._oldestIdx = scenario.prestateOldestIdx || 0; + globalThis._messagesGeneration = 1; + globalThis._loadingOlder = false; + globalThis._loadingSessionId = null; + globalThis._messageRenderWindowSize = 50; + globalThis._msgLimitMax = scenario.msgLimitMax || 200; + globalThis._INITIAL_MSG_LIMIT = 50; + globalThis._EPHEMERAL_TURN_FIELDS = ['_turnUsage','_turnDuration','_turnTps','_gatewayRouting','_statusCard','_anchor_stream_id','_anchor_activity_scene']; + globalThis._isActiveSession = () => scenario.isActiveSession !== false; + globalThis._isSessionCurrentPane = () => scenario.isSessionCurrentPane !== false; + globalThis._isSessionActivelyViewed = () => !!scenario.isSessionActivelyViewed; + globalThis._closeSource = () => calls.push('closeSource'); + globalThis._clearStreamEndRecovery = () => calls.push('clearStreamEndRecovery'); + globalThis._clearOwnerInflightState = () => calls.push('clearOwnerInflight'); + globalThis.clearLiveToolCards = () => calls.push('clearLiveToolCards'); + globalThis.removeThinking = () => calls.push('removeThinking'); + globalThis._flushReasoningToAnchor = () => calls.push('flushReasoning'); + globalThis._applyToAnchor = () => calls.push('applyToAnchor'); + globalThis._hydrateTodosFromSession = () => calls.push('hydrateTodos'); + globalThis._scheduleAnchorRegistryCleanup = () => calls.push('scheduleAnchorRegistryCleanup'); + globalThis._smdEndParser = () => calls.push('smdEndParser'); + globalThis._markSessionCompletionUnread = () => calls.push('markCompletionUnread'); + globalThis._markSessionViewed = () => calls.push('markSessionViewed'); + globalThis.localStorage = { setItem: () => calls.push('setLocalStorageItem'), getItem: () => null, removeItem: () => calls.push('removeLocalStorageItem') }; + globalThis._setActiveSessionUrl = () => calls.push('setActiveSessionUrl'); + globalThis.showToast = () => calls.push('showToast'); + globalThis._clearApprovalForOwner = () => calls.push('clearApprovalForOwner'); + globalThis._clearClarifyForOwner = () => calls.push('clearClarifyForOwner'); + globalThis._streamFadeCleanupReduceMotionListener = () => calls.push('streamFadeCleanup'); + globalThis._cancelThrottledSnapshotTimer = () => calls.push('cancelThrottledSnapshot'); + globalThis._clearAnchorProseIncrementalNode = () => calls.push('clearAnchorProse'); + globalThis._cancelAnimationFramePendingStreamRender = () => calls.push('cancelRaf'); + globalThis.finalizeThinkingCard = () => calls.push('finalizeThinkingCard'); + globalThis.syncTopbar = () => calls.push('syncTopbar'); + globalThis.renderMessages = () => calls.push('renderMessages'); + globalThis.renderSessionList = () => calls.push('renderSessionList'); + globalThis.updateQueueBadge = () => calls.push('updateQueueBadge'); + globalThis._setActivePaneIdleIfOwner = () => calls.push('setActivePaneIdle'); + globalThis.setBusy = () => calls.push('setBusy'); + globalThis.setComposerStatus = () => calls.push('setComposerStatus'); + globalThis.setCompressionUi = () => calls.push('setCompressionUi'); + globalThis.setStatus = () => calls.push('setStatus'); + globalThis.msgContent = (m) => (m && m.content && typeof m.content === 'string' ? m.content : ''); + globalThis._isContextCompactionMessage = () => false; + globalThis._setCompressionSessionLock = () => calls.push('setCompressionSessionLock'); + globalThis._messageRenderableMessageCount = () => scenario.messageRenderableCount || 50; + globalThis._currentMessageRenderWindowSize = () => scenario.currentWindowSize || 12; + globalThis._restoreMessageRenderWindowAfterSettledRender = () => calls.push('restoreRenderWindow'); + globalThis.projectSessionArtifactsForOwner = () => calls.push('projectArtifacts'); + globalThis._streamFinalized = false; + globalThis._persistTimer = null; + globalThis._attachProjectedAnchorSceneToLastAssistant = () => calls.push('attachProjected'); + globalThis._isMessagePaneNearBottom = () => true; + globalThis._isMessageReaderUnpinned = () => false; + globalThis._queueDrainSid = null; + // reducer used by _restoreSettledSession (mirrors live code) + return calls; +} + +(async () => { + const calls = runtimeStubs(); + + if (scenario.action === 'compress_restores_paging') { + // Pre-state: bounded preflight left truncated=true, oldestIdx=70. + globalThis._messagesTruncated = true; + globalThis._oldestIdx = 70; + const fullRows = []; + for (let i = 0; i < 100; i++) { + fullRows.push({ role: i % 2 === 0 ? 'user' : 'assistant', content: `row-${i}`, _ts: `ts-${i}` }); + } + const payload = { + session: { + session_id: 's1', + message_count: 100, + messages: fullRows, + tool_calls: [], + _messages_truncated: false, + _messages_offset: 0, + }, + }; + await globalThis._applyManualCompressionResult(payload, '', 100, '/compress'); + console.log(JSON.stringify({ + action: 'compress_restores_paging', + messagesTruncated: !!globalThis._messagesTruncated, + oldestIdx: globalThis._oldestIdx, + messageCountAfter: Array.isArray(S.messages) ? S.messages.length : -1, + calls, + })); + return; + } + + if (scenario.action === 'settle_repeated_turns') { + // Bounded settle against a transcript containing repeated identical turns + // (same role/content but distinct _ids). Round-2 bug: BOTH prefix and + // suffix matched, prefix won, and rows were dropped+duplicated. + const state = scenario.state || {}; + globalThis.S.session = { session_id: 's1', message_count: state.messageCount }; + globalThis.S.messages = state.messages || []; + globalThis.streamId = 'stream-7628'; + globalThis.S.activeStreamId = 'stream-7628'; + globalThis.api = async () => scenario.apiPayload || { session: null }; + const status = await globalThis._restoreSettledSession({}, { status: true, preserveVisibleOnShorterTerminalSnapshot: true }); + const msgs = Array.isArray(globalThis.S.messages) ? globalThis.S.messages : []; + console.log(JSON.stringify({ + action: 'settle_repeated_turns', + status, + messages: msgs.map((m) => ({ role: m.role, content: m.content, _ts: m._ts, _id: m._id || null })), + oldestIdx: globalThis._oldestIdx, + messagesTruncated: !!globalThis._messagesTruncated, + terminalMarkerCount: msgs.filter(globalThis._isTerminalStreamErrorMarkerMessage).length, + calls, + })); + return; + } + + if (scenario.action === 'load_older_paging') { + // After a bounded recovery (offset=70), "Load earlier" must request a page + // keyed off _oldestIdx (no gap and no overlap), and merge must close the + // window: final oldestIdx === requested offset. + globalThis._messagesTruncated = true; + globalThis._oldestIdx = scenario.oldestIdx || 70; + const existing = []; + for (let i = 70; i < 100; i++) existing.push({ role: i % 2 === 0 ? 'user' : 'assistant', content: `row-${i}`, _ts: `ts-${i}` }); + globalThis.S.messages = existing; + globalThis.S.session = { session_id: 's1', message_count: 100 }; + const requested = []; + globalThis.api = async (url, opts) => { + requested.push(String(url)); + // Return one older page of size 50 (rows 20..69). + const older = []; + for (let i = 20; i < 70; i++) older.push({ role: i % 2 === 0 ? 'user' : 'assistant', content: `row-${i}`, _ts: `ts-${i}` }); + return { + session: { session_id: 's1', message_count: 100, messages: older, _messages_truncated: true, _messages_offset: 20 }, + }; + }; + // Extract the live loader from sessions.js. + const loaderBody = extractFunction(sessionsSrc, '_loadOlderMessages'); + const loaderFactory = new Function(`${loaderBody}; return _loadOlderMessages;`); + const loader = loaderFactory(); + await loader.call(globalThis); + console.log(JSON.stringify({ + action: 'load_older_paging', + requested, + messagesTruncated: !!globalThis._messagesTruncated, + oldestIdx: globalThis._oldestIdx, + messagesLen: Array.isArray(S.messages) ? S.messages.length : -1, + loadingOlder: !!globalThis._loadingOlder, + })); + return; + } + + throw new Error(`unknown action: ${scenario.action}`); +})().catch((err) => { + console.error(err && err.stack ? err.stack : String(err)); + process.exit(1); +}); +""" + + +@pytest.fixture(scope="module") +def driver_path(tmp_path_factory): + driver = tmp_path_factory.mktemp("7628_driver") / "driver.js" + driver.write_text(_DRIVER, encoding="utf-8") + return str(driver) + + +def _run_scenario(driver_path: str, scenario: dict) -> dict: + command = [_NODE, driver_path, str(_COMMANDS_JS), str(_MESSAGES_JS), str(_SESSIONS_JS), json.dumps(scenario)] + result = subprocess.run(command, capture_output=True, text=True, encoding="utf-8", check=False, timeout=30) + if result.returncode != 0: + raise RuntimeError(f"node driver failed: {result.stderr}") + return json.loads(result.stdout.strip()) + + +def test_compress_success_restores_paging_state(driver_path): + """A successful /compress returning the full transcript must reset the + bounded preflight's paging signals before render (#7628 blocker 1).""" + outcome = _run_scenario(driver_path, {"action": "compress_restores_paging"}) + assert outcome["messagesTruncated"] is False, "compress success must clear _messagesTruncated" + assert outcome["oldestIdx"] == 0, f"compress success must reset _oldestIdx to 0, got {outcome['oldestIdx']}" + assert outcome["messageCountAfter"] == 100 + + +def test_settle_bounded_preserves_no_drop_no_dup(driver_path): + """A bounded settle against repeated identical turns must not drop or + duplicate rows (#7628 blocker 2, exact reviewer reproduction). + + All user rows share one identity key (same role/content/_ts), all assistant + rows share one too — _messageIdentityKey collides across every pair. A + bounded tail [u3,a3,u2,a2] therefore matches BOTH the visible prefix + (u1,a1,u2,a2…) and the visible suffix (…u2,a2) at key level. The round-2 + `||` code preferred the prefix branch: splice offset = staged.length + dropped u1/a1 and re-emitted u3/a3 (duplicate). The server truncation + signal must force suffix-exclusive splicing. + """ + _u = {"role": "user", "content": "What's the status?", "_ts": "t-u"} + _a = {"role": "assistant", "content": "All good.", "_ts": "t-a"} + # Visible before settle: u1,a1,u2,a2,u3,a3 + Connection-interrupted marker. + state_messages = [ + {**_u, "_id": 1}, {**_a, "_id": 2}, + {**_u, "_id": 3}, {**_a, "_id": 4}, + {**_u, "_id": 5}, {**_a, "_id": 6}, + {"role": "assistant", "content": "**Connection interrupted:** The browser lost the live SSE connection before the response finished.", "_ts": "t-err", "_id": 7}, + ] + outcome = _run_scenario(driver_path, { + "action": "settle_repeated_turns", + "isSessionCurrentPane": True, + "state": { + "messageCount": 12, + "messages": state_messages, + }, + "apiPayload": { + "session": { + "session_id": "s1", + "active_stream_id": None, + "pending_user_message": None, + "message_count": 12, + "messages": [ + # Reviewer's tail: [u3,a3,u2,a2] — identity keys collide + # with u1/a1/u2/a2/u3/a3 above, so prefix AND suffix both + # compare equal. + {**_u, "_id": 8}, {**_a, "_id": 9}, + {**_u, "_id": 10}, {**_a, "_id": 11}, + ], + "_messages_truncated": True, + "_messages_offset": 4, + }, + }, + }) + assert outcome["status"] == "restored", outcome + ids = [m["_id"] for m in outcome["messages"]] + # No dropped id, no dup id (the prefix-wins bug duplicated u3/a3 and + # dropped u1/a1 — every retained row must be unique). + assert len(ids) == len(set(ids)), f"duplicate rows survived settle: {ids}" + # Staged (authoritative) rows must all be present, plus the marker once. + assert set(ids) == {8, 9, 10, 11, 7}, f"unexpected transcript after fix: {ids}" + # Identity (role|_ts|content) counts: staged rows legitimately repeat + # (two identical user pairs in the tail), but the bug added a THIRD copy by + # splicing the visible tail back in. user<=2, assistant<=2, marker exactly 1. + keys = [(m["role"], m["_ts"], m["content"]) for m in outcome["messages"]] + kc = {} + for k in keys: + kc[k] = kc.get(k, 0) + 1 + user_key = ("user", "t-u", "What's the status?") + asst_key = ("assistant", "t-a", "All good.") + assert kc.get(user_key, 0) == 2, f"user rows must appear exactly twice (staged), got {kc.get(user_key, 0)}: {keys}" + assert kc.get(asst_key, 0) == 2, f"assistant rows must appear exactly twice (staged), got {kc.get(asst_key, 0)}: {keys}" + assert outcome["terminalMarkerCount"] == 1, outcome + assert outcome["oldestIdx"] == 4, f"offset must follow server, got {outcome['oldestIdx']}" + assert outcome["messagesTruncated"] is True + + +def test_bounded_recovery_prepares_gap_free_load_earlier(driver_path): + """After a bounded recovery (oldestIdx=70), Load Earlier must page from the + authoritative offset so the fetched range has no gap and no overlap + (#7628).""" + outcome = _run_scenario(driver_path, {"action": "load_older_paging", "oldestIdx": 70, "msgLimitMax": 80}) + assert outcome["requested"], "loader must have issued an api call" + url = outcome["requested"][0] + assert "msg_before=70" in url, f"load-earlier must page from authoritative offset: {url}" + assert "msg_limit=" in url, url \ No newline at end of file diff --git a/tests/test_session_default_window.py b/tests/test_session_default_window.py index 8e94e455..b10e63bb 100644 --- a/tests/test_session_default_window.py +++ b/tests/test_session_default_window.py @@ -149,7 +149,19 @@ def test_settle_preserves_terminal_marker_against_bounded_suffix(): """Long-session bounded tails are suffixes of S.messages, not prefixes (#7628).""" restore = _restore_settled_session_body() assert "_stagedMatchesCurrentSuffix" in restore - assert "_preserveCurrentTranscript=preserveVisibleOnShorterTerminalSnapshot&&(_stagedMatchesCurrentPrefix||_stagedMatchesCurrentSuffix)" in restore + assert "(_truncatedRecovery?_stagedMatchesCurrentSuffix:_stagedMatchesCurrentPrefix)" in restore + + +def test_settle_truncation_signal_decides_match_strategy(): + """The server's truncation signal must decide prefix-vs-suffix, never an `||`: + bounded settles get suffix-exclusive matching (identical repeated turns make + BOTH comparisons succeed; the prefix branch then splices at the wrong offset + and silently drops/duplicates rows) (#7628).""" + restore = _restore_settled_session_body() + assert "_truncatedRecovery=!!_messagesTruncated||!!(_oldestIdx>0)" in restore + # The splice offset must follow the SAME truncation decision as the match + # strategy — no fallthrough to prefix when both match. + assert "_truncatedRecovery?_stagedSuffixStart+_stagedMessages.length:_stagedMessages.length" in restore def test_apperror_embedded_session_refreshes_paging_before_anchor_persist(): From f33ac82ca1c00c2f75c2628cadb7827d96b57d96 Mon Sep 17 00:00:00 2001 From: happy5318 Date: Sat, 19 Sep 2026 13:35:13 +0800 Subject: [PATCH 6/7] fix(session): guard truncation flags before read in settle strategy (CI) test_issue6867 extracts _restoreSettledSession into a bare harness where the module-level _messagesTruncated/_oldestIdx lets are absent; a bare read threw ReferenceError and the whole recovery path died before it could restore. Use typeof guards like the surrounding paging restores. The semantics are unchanged in the real page (both are always declared). --- static/messages.js | 2 +- tests/test_session_default_window.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/static/messages.js b/static/messages.js index 8295794d..220ab1a6 100644 --- a/static/messages.js +++ b/static/messages.js @@ -7092,7 +7092,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ // identical turns make BOTH comparisons succeed, preferring the prefix // splices at the wrong offset and silently drops/duplicates rows // (#7628). Never prefer prefix when truncation is active. - const _truncatedRecovery=!!_messagesTruncated||!!(_oldestIdx>0); + const _truncatedRecovery=(typeof _messagesTruncated!=='undefined'&&!!_messagesTruncated)||(typeof _oldestIdx!=='undefined'&&!!(_oldestIdx>0)); const _preserveCurrentTranscript=preserveVisibleOnShorterTerminalSnapshot&&(_truncatedRecovery?_stagedMatchesCurrentSuffix:_stagedMatchesCurrentPrefix); const _resolvedMessages=_preserveCurrentTranscript ? [..._stagedMessages,..._currentVisibleMessages.slice(_truncatedRecovery?_stagedSuffixStart+_stagedMessages.length:_stagedMessages.length)] diff --git a/tests/test_session_default_window.py b/tests/test_session_default_window.py index b10e63bb..3676d061 100644 --- a/tests/test_session_default_window.py +++ b/tests/test_session_default_window.py @@ -158,7 +158,7 @@ def test_settle_truncation_signal_decides_match_strategy(): BOTH comparisons succeed; the prefix branch then splices at the wrong offset and silently drops/duplicates rows) (#7628).""" restore = _restore_settled_session_body() - assert "_truncatedRecovery=!!_messagesTruncated||!!(_oldestIdx>0)" in restore + assert "_truncatedRecovery=(typeof _messagesTruncated!=='undefined'&&!!_messagesTruncated)||(typeof _oldestIdx!=='undefined'&&!!(_oldestIdx>0))" in restore # The splice offset must follow the SAME truncation decision as the match # strategy — no fallthrough to prefix when both match. assert "_truncatedRecovery?_stagedSuffixStart+_stagedMessages.length:_stagedMessages.length" in restore From a44a4158767e592a737432cae245a12b1cbc15af Mon Sep 17 00:00:00 2001 From: happy5318 <5318happy@users.noreply.github.com> Date: Sun, 20 Sep 2026 21:44:01 +0800 Subject: [PATCH 7/7] test(session): drive the bounded-settle splice decision in a Node VM MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The round-3 gate reproduced two data-integrity defects against the shipped functions and noted that the focused adjacent suite stayed green through both reproductions — the existing coverage for this area asserts source text, which pins the diff's *text* rather than its *behaviour*. These tests execute the real decision code instead. ## What Changed `tests/test_7628_recovery_window_integrity.py` (new, 5 tests): the bounded-settle splice decision is extracted verbatim from `static/messages.js` (from the terminal-marker detection through the `S.messages` assignment) together with its transitive pure dependencies, and driven through a Node VM with the paging signals as the only variable. - **Discriminating regression for the splice strategy.** The fixture is a visible window of three turns plus a terminal marker against a durable tail of the last turn with `_messages_truncated=true, _messages_offset=4`. On that shape the correct suffix splice yields `[u-3, a-3, marker]`, while the pre-fix `prefix || suffix` expression splices at the head offset and re-emits `u-2/a-2/u-3/a-3` — the duplication the gate reported. Verified to fail on the pre-fix expression and pass on the current one. - **Paging restore.** `_messagesTruncated` / `_oldestIdx` follow the response's own signals for both the truncated and untruncated shapes, so a successful `/compress` (bounded preflight, full-transcript response) cannot leave the preflight's window state behind. - **Source lock on the strategy selection.** The strategy must be a truncation-gated ternary; a bare `||` between the two comparisons is the defect. - **Source lock on the compress success path.** The paging restore must sit between the response assignment and `renderMessages()`, reading `data.session._messages_truncated` / `_messages_offset` rather than hardcoding `false`. ## Why It Matters Both findings are silent history corruption: a wrong splice drops or duplicates transcript rows, and a stale paging offset shifts every absolute index that edit / fork / regenerate addresses rows by. Neither surfaces as an error, so the regression has to observe the resulting row list. ## Verification - 5/5 pass on the current head. - Reverting only the strategy selection (`prefix || suffix` plus the head-offset splice) fails 2 of the 5 — the behavioural splice test and the source lock — confirming the tests bite rather than restate the diff. - `tests/test_session_default_window.py` + `tests/test_7628_recovery_paging.py` unchanged and green (30 items together with the new file). - `scripts/ruff_lint.py --diff origin/master`: no new violations. ## Contract Change None — tests only. ## Model Used Hermes Agent (step-5-preview). The discriminating fixture was derived by executing both the current and the reverted expression through the same harness and comparing the resulting row lists, not by reading the diff. ## Related #7628 (this PR), #7310 (slow /api/session under load), #7625 (bounded recovery measurement). --- tests/test_7628_recovery_window_integrity.py | 343 +++++++++++++++++++ 1 file changed, 343 insertions(+) create mode 100644 tests/test_7628_recovery_window_integrity.py diff --git a/tests/test_7628_recovery_window_integrity.py b/tests/test_7628_recovery_window_integrity.py new file mode 100644 index 00000000..f70964c4 --- /dev/null +++ b/tests/test_7628_recovery_window_integrity.py @@ -0,0 +1,343 @@ +"""Behavioural regressions for the #7628 review round-3 findings. + +The round-3 gate reproduced two data-integrity defects against the shipped +functions and asked for tests that reach them (the source-assertion suite +stayed green through both reproductions): + +1. **A successful `/compress` kept the bounded preflight's paging state.** + The success path assigned the full-transcript response without restoring + ``_messagesTruncated`` / ``_oldestIdx``, so after a compression that + returned 100 full rows the client still showed "70 older messages" and + every absolute transcript index was off by 70 — which is what edit / + fork / regenerate address rows by. + +2. **Prefix match won over suffix match when both succeeded.** A bounded + settle returns a *suffix* of the durable transcript, but the old + ``prefix || suffix`` expression preferred the prefix branch, so a + transcript with repeated identical turns spliced at the wrong offset: + ``u1/a1`` silently dropped, ``u3/a3`` duplicated. + +These tests execute the real decision code rather than grepping it: the +splice strategy block is extracted from ``static/messages.js`` verbatim and +driven through a Node VM with the paging signals as the only variable, and +the compress success path is asserted at source level for the restore +ordering (that block issues a live ``/api/session`` fetch, so driving it +would need a network). +""" +from __future__ import annotations + +import json +import shutil +import subprocess +from pathlib import Path + +import pytest + +from tests.js_source_extract import extract_function + + +ROOT = Path(__file__).resolve().parents[1] +MESSAGES_JS = (ROOT / "static" / "messages.js").read_text(encoding="utf-8") +COMMANDS_JS = (ROOT / "static" / "commands.js").read_text(encoding="utf-8") +NODE = shutil.which("node") + +pytestmark = pytest.mark.skipif(NODE is None, reason="node not on PATH") + + +# ── extraction helpers ─────────────────────────────────────────────────────── + +_SPLICE_DEPS = [ + "_messageIdentityKey", + "_isTerminalStreamErrorMarkerMessage", + "_streamRecoveryControlMessage", + "_filterRecoveryControlMessages", + "_carryForwardEphemeralTurnFields", +] + + +def _splice_block() -> str: + """The bounded-settle splice decision, verbatim from messages.js. + + Spans the terminal-marker detection and the prefix/suffix comparisons + through the ``S.messages`` assignment — the region the round-3 gate + patched. Everything before it (paging restore, session assignment) is set + up as harness state instead. + """ + start_marker = "const _currentVisibleEndsWithTerminalMarker=(" + start = MESSAGES_JS.find(start_marker) + assert start >= 0, "terminal-marker detection not found in static/messages.js" + end_marker = "S.messages=_filterRecoveryControlMessages(_resolvedMessages || []);" + end = MESSAGES_JS.find(end_marker, start) + assert end >= 0, "S.messages assignment after the splice not found" + return MESSAGES_JS[start : end + len(end_marker)] + + +def _dependency_functions() -> list[str]: + """The splice block's transitive pure dependencies, in definition order. + + The extraction is closure-driven: start from the helpers the block names, + then keep pulling the helpers those helpers reference until the set is + closed. That keeps the harness honest when the production code grows a + new dependency — a missing one fails the node run instead of silently + stubbing behaviour away. + """ + import re + + needed: list[str] = list(_SPLICE_DEPS) + resolved: list[str] = [] + seen: set[str] = set() + while needed: + name = needed.pop(0) + if name in seen: + continue + seen.add(name) + try: + body = extract_function(MESSAGES_JS, name) + except AssertionError: + continue + resolved.append(name) + for ref in re.findall(r"\b(_[a-zA-Z][a-zA-Z0-9_]*)\s*\(", body): + if ref not in seen and ref != name: + needed.append(ref) + # Definition order matters only for hoisted `function` declarations, which + # JS hoists anyway; keep the resolved order for readability. + return [extract_function(MESSAGES_JS, name) for name in resolved] + + +def _compress_success_block() -> str: + """The `/compress` success path that assigns the full-transcript response. + + Anchored on the bounded preflight fetch's continuation: the block runs + after a successful compression response and must restore the paging + signals before ``renderMessages()``. + """ + anchor = "const summary=data&&data.summary;" + start = COMMANDS_JS.rfind(anchor) + assert start >= 0, "compress success block not found in static/commands.js" + assign = COMMANDS_JS.rfind("S.messages=", 0, start) + assert assign >= 0, "S.messages assignment before the summary block not found" + return COMMANDS_JS[assign:start] + + +# ── node harness ───────────────────────────────────────────────────────────── + +_HARNESS = r""" +const deps = __DEPS__; +const splice = __SPLICE__; + +const S = { session: { session_id: 'sid' }, messages: [] }; +let _messagesTruncated = false; +let _oldestIdx = 0; +let _adoptRegenerationRevision = () => {}; +let _hydrateTodosFromSession = () => {}; +let _markSessionCompletionUnread = () => {}; +let _setActiveSessionUrl = () => {}; +let _attachProjectedAnchorSceneToLastAssistant = () => {}; +let _replaceMarkerOnlyAssistantWithStreamError = () => false; +const store = {}; +globalThis.localStorage = { + getItem: (k) => (Object.prototype.hasOwnProperty.call(store, k) ? store[k] : null), + setItem: (k, v) => { store[k] = String(v); }, +}; +// Closure-scope constant that lives just above the extracted helper in +// messages.js (extract_function only captures the function body). +const _EPHEMERAL_TURN_FIELDS = ['_turnUsage','_turnDuration','_turnTps','_gatewayRouting','_statusCard','_anchor_stream_id','_anchor_activity_scene']; +for (const fn of deps) eval(fn); + +// Scenario: repeated identical turns in the visible window, a terminal +// marker at the end, and a bounded durable tail whose ids ALSO appear +// earlier — so both the prefix and the suffix comparison succeed. +function makeTurn(n) { + return [ + { role: 'user', id: `u-${n}`, content: `prompt ${n}` }, + { role: 'assistant', id: `a-${n}`, content: `answer ${n}` }, + ]; +} +S.messages = []; +for (const n of [1, 2, 3]) S.messages.push(...makeTurn(n)); +// The real terminal marker shape: an assistant row whose content starts with +// the Connection-interrupted prefix (what _isTerminalStreamErrorMarkerMessage +// actually matches). +S.messages.push({ + role: 'assistant', + id: 'marker', + content: '**Connection interrupted:** The browser lost the live SSE connection before the response finished.', +}); + +const durableTail = [ + { role: 'user', id: 'u-3', content: 'prompt 3' }, + { role: 'assistant', id: 'a-3', content: 'answer 3' }, +]; +const session = { + session_id: 'sid', + messages: durableTail, + _messages_truncated: __TRUNCATED__, + _messages_offset: 4, +}; +_messagesTruncated = __TRUNCATED__; +_oldestIdx = __OFFSET__; + +// The state the splice block reads, mirroring the lines just above it. +const _currentMessages = S.messages.slice(); +const _currentVisibleMessages = _filterRecoveryControlMessages(_currentMessages); +const _stagedMessages = _carryForwardEphemeralTurnFields(_currentMessages, session.messages); +const preserveVisibleOnShorterTerminalSnapshot = true; +S.session = session; + +eval(splice); + +const out = { + truncated: _messagesTruncated, + oldestIdx: _oldestIdx, + rows: (S.messages || []).map(m => `${m.role}:${m.id}`), +}; +console.log(JSON.stringify(out)); +""" + + +def _run_node(script: str) -> dict: + result = subprocess.run( + [NODE, "-e", script], + cwd=str(ROOT), + capture_output=True, + text=True, + timeout=60, + ) + assert result.returncode == 0, ( + f"node subprocess failed:\n--- stdout ---\n{result.stdout}\n" + f"--- stderr ---\n{result.stderr}" + ) + return json.loads(result.stdout.strip().splitlines()[-1]) + + +def _drive(*, truncated: bool, offset: int) -> dict: + deps = "[" + ",".join(json.dumps(f) for f in _dependency_functions()) + "]" + splice = json.dumps(_splice_block()) + script = ( + _HARNESS.replace("__DEPS__", deps) + .replace("__SPLICE__", splice) + .replace("__TRUNCATED__", "true" if truncated else "false") + .replace("__OFFSET__", str(offset)) + ) + return _run_node(script) + + +# ── tests ──────────────────────────────────────────────────────────────────── + + +def test_bounded_settle_with_repeated_identical_turns_drops_and_duplicates_nothing(): + """The round-3 SILENT finding: identical repeated turns must not splice wrong. + + The visible window holds repeated identical turns, so the durable tail's + ids appear at BOTH the head and the tail of the window and both the + prefix and the suffix comparison succeed. The pre-fix + ``prefix || suffix`` expression preferred the prefix branch and spliced at + the head offset — dropping ``u2/a2`` and ``u3/a3`` from the window even + though the server returned them as a truncated suffix (``_oldestIdx=4``). + The truncation signal must select the suffix branch instead. + + The two branches produce observably different row lists on this shape, + which is what makes this a regression test rather than a tautology: with + the prefix branch restored the splice keeps the wrong rows. + """ + out = _drive(truncated=True, offset=4) + + rows = out["rows"] + # The durable tail rows are present exactly once each — the pre-fix + # prefix branch duplicated them. + assert rows.count("user:u-3") == 1, rows + assert rows.count("assistant:a-3") == 1, rows + # No row appears twice anywhere. + assert len(rows) == len(set(rows)), f"duplicate rows in {rows}" + # The suffix splice keeps the durable tail plus the rows that follow it + # in the visible window (the terminal marker). The prefix branch would + # splice at the head offset and re-emit u2/a2/u3/a3. + assert rows == ["user:u-3", "assistant:a-3", "assistant:marker"], ( + "the suffix splice must keep durable-tail + the rows after it; got " + f"{rows} (extra rows mean the prefix branch won)" + ) + # Paging follows the response so absolute indices line up. + assert out["truncated"] is True + assert out["oldestIdx"] == 4 + + +def test_bounded_settle_keeps_the_response_paging_signals(): + """Paging flags follow the server's truncation signal, not the preflight.""" + out = _drive(truncated=True, offset=4) + assert out["truncated"] is True + assert out["oldestIdx"] == 4 + + +def test_untruncated_snapshot_clears_the_bounded_preflight_state(): + """An untruncated response must clear the bounded preflight's state. + + This is the `/compress` success-path shape from the CORE finding: the + preflight bounded the tail, the response carries the full transcript, and + the client must end at ``truncated=false`` / ``offset=0`` so absolute + transcript indices line up again. + """ + out = _drive(truncated=False, offset=0) + assert out["truncated"] is False, ( + "an untruncated response must clear _messagesTruncated; the bounded " + f"preflight state leaked (got {out['truncated']})" + ) + assert out["oldestIdx"] == 0 + + +def test_compress_success_path_restores_paging_before_render(): + """Source lock: the compress success path restores paging before render. + + The round-3 CORE reproduction was a successful `/compress` whose response + carried 100 full rows while the client stayed at + ``_messagesTruncated=true, _oldestIdx=70``. The restore must appear + between the response assignment and ``renderMessages()`` — the same + ordering the settle and cancel paths already use. + """ + block = _compress_success_block() + + assign_idx = block.find("S.messages=") + restore_idx = block.find("_messagesTruncated=") + offset_idx = block.find("_oldestIdx=") + render_idx = block.find("renderMessages()") + + assert assign_idx >= 0, "compress success path must assign S.messages" + assert restore_idx > assign_idx, ( + "_messagesTruncated must be restored from the response AFTER the " + "assignment — the preflight's bounded value otherwise survives" + ) + assert offset_idx > assign_idx, "_oldestIdx must be restored from _messages_offset" + assert render_idx > restore_idx and render_idx > offset_idx, ( + "paging must be restored BEFORE renderMessages() so the rendered rows " + "and any absolute index computation see the restored values" + ) + # The restore must read the response's own signals, not hardcode false. + assert "_messages_truncated" in block, ( + "the restore must read data.session._messages_truncated rather than " + "hardcoding _messagesTruncated=false" + ) + assert "_messages_offset" in block, ( + "the restore must read data.session._messages_offset" + ) + + +def test_truncation_signal_selects_suffix_strategy_exclusively(): + """Source lock: the strategy is a truncation-gated ternary, never an `||`. + + The pre-fix expression was ``_stagedMatchesCurrentPrefix || + _stagedMatchesCurrentSuffix``: when repeated identical turns made both + comparisons succeed the prefix branch won and spliced at the wrong + offset. The gate is now ``_truncatedRecovery ? suffix : prefix``. + """ + block = _splice_block() + + assert "_truncatedRecovery" in block, ( + "the splice strategy must consult the truncation signal" + ) + assert "_truncatedRecovery?_stagedMatchesCurrentSuffix" in block.replace(" ", ""), ( + "a truncated recovery must select the SUFFIX comparison exclusively" + ) + # The old defect: a bare `||` between the two comparisons. + assert "||_stagedMatchesCurrentSuffix" not in block.replace(" ", ""), ( + "prefix and suffix comparisons must not be OR-ed; preferring the " + "prefix when both match splices repeated turns at the wrong offset" + )