diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 2ce017cf..e26d7613 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -534,7 +534,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 @@ -1402,7 +1404,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/api/routes.py b/api/routes.py index 06539bc3..c0e5f92c 100644 --- a/api/routes.py +++ b/api/routes.py @@ -9132,9 +9132,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 @@ -9145,6 +9147,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. @@ -13181,8 +13203,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. @@ -13191,6 +13215,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. @@ -13400,9 +13427,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 da741566..ff7950c3 100644 --- a/static/commands.js +++ b/static/commands.js @@ -930,6 +930,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(); @@ -1028,14 +1034,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); @@ -1823,11 +1833,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 @@ -1845,8 +1857,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 dbd13588..99ec6e8f 100644 --- a/static/messages.js +++ b/static/messages.js @@ -6766,6 +6766,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){ @@ -7007,6 +7011,11 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ S.session=sessionPayload; const _nextMsgs3018=(sessionPayload.messages||[]).filter(m=>m&&m.role); if(typeof _adoptRegenerationRevision==='function')_adoptRegenerationRevision(sessionPayload); + // A bounded cancel-recovery reload returns a tail window: keep the + // 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); @@ -7028,8 +7037,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 @@ -7121,7 +7132,10 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ return returnStatus?'stale':false; } try{ - const data=await api(`/api/session?session_id=${encodeURIComponent(activeSid)}`,options&&options.requestOptions||{}); + // 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`,options&&options.requestOptions||{}); if(isCurrent&&!isCurrent()) return returnStatus?'stale':false; // 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. @@ -7156,6 +7170,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 && @@ -7171,18 +7195,40 @@ 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; + }) + ); + // 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=(typeof _messagesTruncated!=='undefined'&&!!_messagesTruncated)||(typeof _oldestIdx!=='undefined'&&!!(_oldestIdx>0)); + const _preserveCurrentTranscript=preserveVisibleOnShorterTerminalSnapshot&&(_truncatedRecovery?_stagedMatchesCurrentSuffix:_stagedMatchesCurrentPrefix); const _resolvedMessages=_preserveCurrentTranscript - ? [..._stagedMessages,..._currentVisibleMessages.slice(_stagedMessages.length)] + ? [..._stagedMessages,..._currentVisibleMessages.slice(_truncatedRecovery?_stagedSuffixStart+_stagedMessages.length:_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); - } - 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/static/outline.js b/static/outline.js index 45acffe2..0d2a361c 100644 --- a/static/outline.js +++ b/static/outline.js @@ -104,16 +104,15 @@ 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 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) + - '&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 593d660a..14c936b5 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -4007,7 +4007,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 201eb7fd..add5b0fe 100644 --- a/static/ui.js +++ b/static/ui.js @@ -10353,7 +10353,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_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_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" + ) 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_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 589ed37b..90dc5c38 100644 --- a/tests/test_live_to_final_anchor_visible_order.py +++ b/tests/test_live_to_final_anchor_visible_order.py @@ -816,20 +816,26 @@ 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;") - 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 diff --git a/tests/test_session_default_window.py b/tests/test_session_default_window.py new file mode 100644 index 00000000..3676d061 --- /dev/null +++ b/tests/test_session_default_window.py @@ -0,0 +1,229 @@ +"""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 _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 "(_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=(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 + + +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 + + +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