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