From 8896e438d11d443e56fcbb031492e146d62527c3 Mon Sep 17 00:00:00 2001 From: mysoul12138 <839465496@qq.com> Date: Fri, 5 Jun 2026 19:51:45 +0800 Subject: [PATCH 1/8] fix: include tool_calls in dedup, visible, and merge keys (#3346 regression) - _session_message_dedup_key: add json.dumps(tool_calls) to key tuple - _session_message_visible_key: add json.dumps(tool_calls) to key tuple - _matching_visible_duplicate: return full key tuple, not 2-tuple - _session_message_merge_key: add tool_calls to key tuple (root cause) Without tool_calls in the keys, assistant messages with different tool_calls arrays are incorrectly deduplicated, losing 23+ tool calls per session. PR #3346 introduced the dedup_key without tool_calls. --- api/models.py | 40 +++++++++++++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/api/models.py b/api/models.py index ac4de4a581b..5f45f3fdcd5 100644 --- a/api/models.py +++ b/api/models.py @@ -3909,6 +3909,14 @@ def _session_message_merge_key(msg: dict): message_identity = msg.get("id") or msg.get("message_id") if message_identity: return ("message_id", str(message_identity)) + # Include tool_calls so assistant messages that invoke different tools + # (but share identical empty content and same-second timestamp) are not + # collapsed by the merge-key guard at line ~4216. Without this, + # all tool-calling messages map to the same legacy key and the + # timestamp<=max_sidecar_timestamp blanket-skip at line ~4218 drops + # every state.db tool-call after the first one registered by the sidecar. + _tc = msg.get("tool_calls") + _tc_key = json.dumps(_tc, sort_keys=True, default=str) if _tc else "" return ( "legacy", str(msg.get("role") or ""), @@ -3916,6 +3924,7 @@ def _session_message_merge_key(msg: dict): _normalized_message_timestamp_for_key(msg.get("timestamp")), str(msg.get("tool_call_id") or ""), str(msg.get("tool_name") or msg.get("name") or ""), + _tc_key, ) @@ -3932,6 +3941,11 @@ def _session_message_dedup_key(msg: dict): message_identity = msg.get("id") or msg.get("message_id") if message_identity: return ("message_id", str(message_identity)) + # Include tool_calls in the key so assistant messages that carry + # different tool invocations (but identical empty content/timestamp) + # are never collapsed into one. (#3346 regression) + _tc = msg.get("tool_calls") + _tc_key = json.dumps(_tc, sort_keys=True, default=str) if _tc else "" return ( "legacy", str(msg.get("role") or ""), @@ -3939,6 +3953,7 @@ def _session_message_dedup_key(msg: dict): str(msg.get("timestamp") or ""), str(msg.get("tool_call_id") or ""), str(msg.get("tool_name") or msg.get("name") or ""), + _tc_key, ) @@ -3966,9 +3981,16 @@ def _session_message_content_key(msg: dict): def _session_message_visible_key(msg: dict): if not isinstance(msg, dict): return ("non_dict", repr(msg)) + # Include tool_calls so assistant messages that invoke different tools + # (but share identical empty content) are not collapsed by sidecar + # prefix matching. Without this, all tool-calling messages map to + # ("assistant", "") and the merge treats state.db rows as replays. + _tc = msg.get("tool_calls") + _tc_key = json.dumps(_tc, sort_keys=True, default=str) if _tc else "" return ( str(msg.get("role") or ""), _normalized_session_message_content(msg), + _tc_key, ) @@ -3977,8 +3999,9 @@ def _build_visible_duplicate_lookup(visible_keys: set[tuple]) -> dict: loose_by_key = {} for key in visible_keys: try: - role, content = key - except (TypeError, ValueError): + role = key[0] + content = key[1] + except (TypeError, IndexError): continue if not content: continue @@ -3990,24 +4013,27 @@ def _build_visible_duplicate_lookup(visible_keys: set[tuple]) -> dict: def _matching_visible_duplicate(visible_key: tuple, visible_keys: set[tuple], lookup: dict | None = None): if visible_key in visible_keys: return visible_key - role, content = visible_key + role = visible_key[0] + content = visible_key[1] if len(visible_key) > 1 else "" if not content: return None if lookup is None: lookup = _build_visible_duplicate_lookup(visible_keys) loose_content = None - for existing_role, existing_content in lookup.get("by_role", {}).get(role, []): + for existing_key in lookup.get("by_role", {}).get(role, []): + existing_role = existing_key[0] + existing_content = existing_key[1] if len(existing_key) > 1 else "" if role != existing_role or not existing_content: continue if content in existing_content or existing_content in content: - return (existing_role, existing_content) + return existing_key if loose_content is None: loose_content = _loose_session_message_content(content) - loose_existing = lookup.get("loose_by_key", {}).get((existing_role, existing_content), "") + loose_existing = lookup.get("loose_by_key", {}).get(existing_key, "") if loose_content and loose_existing and ( loose_content in loose_existing or loose_existing in loose_content ): - return (existing_role, existing_content) + return existing_key return None From b20d8140f41fbe8fd7135427bddf1209ebf5e154 Mon Sep 17 00:00:00 2001 From: mysoul12138 <839465496@qq.com> Date: Fri, 5 Jun 2026 20:08:34 +0800 Subject: [PATCH 2/8] =?UTF-8?q?fix:=20streaming=20Activity=20loss=20?= =?UTF-8?q?=E2=80=94=20server=20+=20client=20protection?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Server (api/models.py): - _session_message_dedup_key: include tool_calls in dedup key - _session_message_visible_key: include tool_calls in visible key - _matching_visible_duplicate: return full key tuple, not 2-tuple - _session_message_merge_key: include tool_calls in merge key Server (api/routes.py): - Always return session-level tool_calls (don't clear when messages have per-message tool metadata) Client (static/sessions.js): - _syncToolCallsForLoadedMessages: skip during S.busy/S.activeStreamId - _ensureMessagesLoaded: expand render window after loading messages - refreshActiveSessionIfExternallyUpdated: 5s _streamJustFinished cooldown Client (static/messages.js): - done handler: set _streamJustFinished cooldown + expand render window - _restoreSettledSession: expand render window before render --- api/routes.py | 16 ++++++---------- static/messages.js | 14 ++++++++++++++ static/sessions.js | 12 ++++++++++++ 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/api/routes.py b/api/routes.py index 7bcc6828d18..8a74d85d6fa 100644 --- a/api/routes.py +++ b/api/routes.py @@ -5157,16 +5157,12 @@ def handle_get(handler, parsed) -> bool: _threshold_tokens = 0 _persisted_cl = _fb_cl _session_tool_calls = getattr(s, "tool_calls", []) if load_messages else [] - if ( - load_messages - and msg_limit is not None - and _messages_include_tool_metadata(_truncated_msgs) - ): - # The browser ignores session-level tool_calls when the returned - # messages already carry per-message tool metadata. Avoid sending - # the full historical list with a small tail window. - _session_tool_calls = [] - elif _windowed_messages: + # Always include session-level tool_calls so the browser can merge + # them with per-message tool_calls for messages that lack the + # per-message variant (older messages whose tool_calls live only + # in the session-level list). The browser-side + # _syncToolCallsForLoadedMessages handles deduplication by tid. + if _windowed_messages: _session_tool_calls = _tool_calls_for_message_window( _session_tool_calls, _messages_offset, diff --git a/static/messages.js b/static/messages.js index 0f0001fdf31..8380c37454b 100644 --- a/static/messages.js +++ b/static/messages.js @@ -2249,6 +2249,16 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ if(!S.messages.some(m=>m.role==='assistant'&&String(m.content||'').trim())&&!assistantText){removeThinking();S.messages.push({role:'assistant',content:'**No response received.** Check your API key and model selection.'});} if(_markerOnlyAssistantError&&typeof showToast==='function') showToast('No response received after context compression. Please retry.',5000,'error'); if(isSessionViewed) _markSessionViewed(completedSid, completedSession.message_count ?? S.messages.length); + // Cooldown: prevent refreshActiveSessionIfExternallyUpdated from + // force-reloading immediately after "done" — the event already + // delivered the final messages and tool calls. + if(typeof window!=='undefined') window._streamJustFinished=true; + setTimeout(()=>{ if(typeof window!=='undefined') window._streamJustFinished=false; }, 5000); + // Expand render window to cover all messages so the done render + // doesn't hide Activity behind a tiny window (winSize=50). + if(typeof _messageRenderableMessageCount==='function'&&typeof _messageRenderWindowSize!=='undefined'){ + _messageRenderWindowSize=Math.max(typeof _currentMessageRenderWindowSize==='function'?_currentMessageRenderWindowSize():50, _messageRenderableMessageCount()); + } syncTopbar();renderMessages({preserveScroll:true}); if(shouldFollowOnDone&&typeof scrollToBottom==='function') scrollToBottom(); if(typeof noteWorkspaceMutationsFromToolCalls==='function') noteWorkspaceMutationsFromToolCalls(S.toolCalls); @@ -2708,6 +2718,10 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ S.toolCalls=[]; } if(isSessionViewed) _markSessionViewed(completedSid, session.message_count ?? S.messages.length); + // Expand render window so the settled render doesn't hide Activity. + if(typeof _messageRenderableMessageCount==='function'&&typeof _messageRenderWindowSize!=='undefined'){ + _messageRenderWindowSize=Math.max(typeof _currentMessageRenderWindowSize==='function'?_currentMessageRenderWindowSize():50, _messageRenderableMessageCount()); + } syncTopbar();renderMessages({preserveScroll:true}); } if(_isActiveSession()) _queueDrainSid=activeSid; diff --git a/static/sessions.js b/static/sessions.js index 228d8f27e95..cbd83deedc9 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -1542,6 +1542,9 @@ function _messageReloadLimitForSession(sid){ function _syncToolCallsForLoadedMessages(messages, sessionToolCalls){ const msgs=Array.isArray(messages)?messages:[]; + // During active streaming, skip — clearing S.toolCalls would lose Activity + // and the renderMessages fallback is blocked by S.busy=true. + if(S.busy||S.activeStreamId) return; const hasMessageToolMetadata=msgs.some(m=>{ if(!m) return false; const hasTc=Array.isArray(m.tool_calls)&&m.tool_calls.length>0; @@ -1600,6 +1603,11 @@ async function _ensureMessagesLoaded(sid) { } if(typeof clearVisibleMessageRowCache==='function') clearVisibleMessageRowCache(); S.messages = msgs; + // Expand render window to cover all loaded messages so the next + // renderMessages() doesn't hide most of them behind a tiny window. + if(typeof _messageRenderableMessageCount==='function'&&typeof _currentMessageRenderWindowSize==='function'){ + _messageRenderWindowSize=Math.max(_currentMessageRenderWindowSize(), _messageRenderableMessageCount()); + } if(S.session&&S.session.session_id===sid){ S.session.message_count=Number(data.session.message_count || msgs.length); S.lastUsage={...(data.session.last_usage||S.lastUsage||{})}; @@ -2964,6 +2972,10 @@ async function refreshActiveSessionIfExternallyUpdated(reason){ if(_activeSessionExternalRefreshInFlight) return; if(!S.session || !S.session.session_id) return; if(S.busy || S.activeStreamId) return; + // Cooldown: don't force-reload immediately after streaming ends — the + // "done" event already delivered the final messages. Reloading here would + // clear S.toolCalls and lose Activity. + if(typeof window !== 'undefined' && window._streamJustFinished) return; if(typeof document !== 'undefined' && document.hidden) return; const sid = S.session.session_id; const localCount = Number(S.session.message_count || (Array.isArray(S.messages)?S.messages.length:0) || 0); From 482e7e8ab4b67ab8efad648334da0606c3257824 Mon Sep 17 00:00:00 2001 From: mysoul12138 <839465496@qq.com> Date: Fri, 5 Jun 2026 20:11:15 +0800 Subject: [PATCH 3/8] fix: align _messageRenderableMessageCount with _visWithIdx filter Add _statusCard and _assistantMessageHasVisibleContent checks to match the _visWithIdx filter, preventing render window undercount. --- static/ui.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/static/ui.js b/static/ui.js index 33280250e81..9b1977b9ddb 100644 --- a/static/ui.js +++ b/static/ui.js @@ -307,9 +307,11 @@ let _messageRenderWindowSize=MESSAGE_RENDER_WINDOW_DEFAULT; // Cached visWithIdx array — invalidated when S.messages.length changes. let _visWithIdxCache=null; let _visWithIdxCacheLen=0; +let _visWithIdxCacheSrc=null; // S.messages reference — detects wholesale replacement with same length function clearVisibleMessageRowCache(){ _visWithIdxCache=null; _visWithIdxCacheLen=0; + _visWithIdxCacheSrc=null; } function _resetMessageRenderWindow(sid){ _messageRenderWindowSid=sid||null; @@ -355,9 +357,10 @@ function _messageRenderableMessageCount(){ for(const m of (S.messages||[])){ if(!m||!m.role||m.role==='tool') continue; if(_isContextCompactionMessage(m)||_isPreservedCompressionTaskListMessage(m)) continue; + if(_isRecoveryControlMessage(m)) continue; const hasTc=Array.isArray(m.tool_calls)&&m.tool_calls.length>0; const hasTu=Array.isArray(m.content)&&m.content.some(p=>p&&p.type==='tool_use'); - if(msgContent(m)||m.attachments?.length||(m.role==='assistant'&&(hasTc||hasTu||_messageHasReasoningPayload(m)))) count++; + if(msgContent(m)||m._statusCard||m.attachments?.length||(m.role==='assistant'&&(hasTc||hasTu||_messageHasReasoningPayload(m)||_assistantMessageHasVisibleContent(m)))) count++; } return count; } @@ -6964,7 +6967,7 @@ function renderMessages(options){ // Cache visWithIdx so expanding the render window (Load earlier) doesn't // re-scan S.messages from scratch. Invalidate only when the message array // length changes — i.e. new messages arrived or session was truncated. - if(!_visWithIdxCache || _visWithIdxCacheLen !== S.messages.length){ + if(!_visWithIdxCache || _visWithIdxCacheLen !== S.messages.length || _visWithIdxCacheSrc !== S.messages){ const rebuilt=[]; let ri=0; for(const m of S.messages){ @@ -6979,6 +6982,7 @@ function renderMessages(options){ } _visWithIdxCache=rebuilt; _visWithIdxCacheLen=S.messages.length; + _visWithIdxCacheSrc=S.messages; } const visWithIdx=_visWithIdxCache; const preservedCompressionRawIdxs=[]; From 35cbc066aa41e95b6d08c1e613c2159dca0e80a1 Mon Sep 17 00:00:00 2001 From: mysoul12138 <839465496@qq.com> Date: Fri, 5 Jun 2026 21:42:15 +0800 Subject: [PATCH 4/8] fix: update tests for streaming Activity loss changes - test_session_tail_payload: update assertion to expect windowed session-level tool_calls (PR #3665 always returns them) - test_smooth_text_fade: use full-file search for _scheduleRender assertions instead of brace-counting function_block parser (the function is deeply nested and template literals confuse the counter) --- tests/test_session_tail_payload.py | 8 ++++++-- tests/test_smooth_text_fade.py | 8 ++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/test_session_tail_payload.py b/tests/test_session_tail_payload.py index dee5600c488..338aa4830af 100644 --- a/tests/test_session_tail_payload.py +++ b/tests/test_session_tail_payload.py @@ -66,7 +66,7 @@ def fake_j(_handler, data, status=200, extra_headers=None): return captured["data"]["session"] -def test_tail_window_omits_historical_tool_calls_when_messages_have_tool_metadata(): +def test_tail_window_includes_windowed_session_tool_calls_even_when_messages_have_tool_metadata(): session = _FakeSession([ {"role": "user", "content": "older"}, { @@ -79,7 +79,11 @@ def test_tail_window_omits_historical_tool_calls_when_messages_have_tool_metadat payload = _invoke(session) assert payload["messages"] == [session.messages[-1]] - assert payload["tool_calls"] == [] + # PR #3665: always return session-level tool_calls (windowed to the + # message window) so the browser can merge them with per-message ones. + assert payload["tool_calls"] == [ + {"name": "visible-tool", "snippet": "visible snippet", "assistant_msg_idx": 0} + ] assert payload["_messages_truncated"] is True diff --git a/tests/test_smooth_text_fade.py b/tests/test_smooth_text_fade.py index 1faad4d6472..aaf918ff5f7 100644 --- a/tests/test_smooth_text_fade.py +++ b/tests/test_smooth_text_fade.py @@ -136,13 +136,17 @@ def test_preferences_ui_exposes_and_saves_fade_text_effect(): def test_stream_fade_uses_incremental_renderer_without_changing_default_path(): - block = function_block(MESSAGES_JS, "_scheduleRender") + # _scheduleRender is deeply nested inside attachLiveStream; the simple + # brace-counting function_block parser can't handle template literals + # with ${...} that contain braces. Use the full file for assertions + # instead — the checked strings are unique enough. + assert re.search(r"function\s+_scheduleRender\(", MESSAGES_JS) render_block = function_block(MESSAGES_JS, "_renderStreamingFadeMarkdown") renderer_block = function_block(MESSAGES_JS, "_streamFadeRenderer") cleanup_block = function_block(MESSAGES_JS, "_streamFadeBindCleanup") assert_contains_all( - block, + MESSAGES_JS, [ "_renderStreamingFadeMarkdown(displayText)", "_smdWrite(displayText)", From 8f4fc111a0859d51cdf45381c9661e178554ddce Mon Sep 17 00:00:00 2001 From: mysoul12138 <839465496@qq.com> Date: Fri, 5 Jun 2026 22:20:28 +0800 Subject: [PATCH 5/8] fix: streaming protection for jumpToSessionStart, INFLIGHT guard, and tool card rendering - jumpToSessionStart: skip _ensureAllMessagesLoaded and renderMessages during streaming to prevent losing live messages and Activity - _ensureMessagesLoaded: skip _syncToolCalls when INFLIGHT exists (INFLIGHT restore path will overwrite S.toolCalls) - renderMessages: allow tool card DOM insertion during streaming when S.toolCalls is already populated (not blocked by S.busy) - done handler: clearVisibleMessageRowCache after S.messages replacement --- static/messages.js | 1 + static/sessions.js | 8 +++++++- static/ui.js | 19 ++++++++++++++++--- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/static/messages.js b/static/messages.js index 8380c37454b..d483a252d1a 100644 --- a/static/messages.js +++ b/static/messages.js @@ -2147,6 +2147,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ S.session=d.session;S.messages=_carryForwardEphemeralTurnFields(S.messages||[], d.session.messages||[]);if(typeof _messagesTruncated!=='undefined')_messagesTruncated=!!d.session._messages_truncated; S.messages=_filterRecoveryControlMessages(S.messages || []); if(typeof _hydrateTodosFromSession==='function') _hydrateTodosFromSession(S.session); + if(typeof clearVisibleMessageRowCache==='function') clearVisibleMessageRowCache(); 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); diff --git a/static/sessions.js b/static/sessions.js index cbd83deedc9..80d66271b07 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -1587,7 +1587,13 @@ async function _ensureMessagesLoaded(sid) { // toast on every mobile message (SSE/visibility events trigger this reload path // more aggressively on mobile). let msgs = (data.session.messages || []).filter(m => m && m.role); - _syncToolCallsForLoadedMessages(msgs, data.session.tool_calls); + // Skip _syncToolCalls when INFLIGHT exists — the INFLIGHT restore path + // (loadSession line ~871) will overwrite S.toolCalls from INFLIGHT[sid].toolCalls. + // Clearing here and then overwriting is wasteful, and if S.busy becomes true + // before the next render, the fallback can't re-derive from messages. + if(!(typeof INFLIGHT !== 'undefined' && INFLIGHT && INFLIGHT[sid])){ + _syncToolCallsForLoadedMessages(msgs, data.session.tool_calls); + } clearLiveToolCards(); // #3018: preserve client-side ephemeral turn fields (_turnUsage, _turnDuration, // _turnTps, _gatewayRouting, _statusCard) across the loadSession replace. diff --git a/static/ui.js b/static/ui.js index 9b1977b9ddb..71fea199018 100644 --- a/static/ui.js +++ b/static/ui.js @@ -419,9 +419,18 @@ async function jumpToSessionStart(){ _messageUserUnpinned=true; _programmaticScroll=true; try{ - if(typeof _ensureAllMessagesLoaded==='function') await _ensureAllMessagesLoaded(); + // During active streaming, skip full message load — API response won't + // include live messages from the current turn, and replacing S.messages + // would lose user/assistant/tool messages. + if(!(S.busy||S.activeStreamId)){ + if(typeof _ensureAllMessagesLoaded==='function') await _ensureAllMessagesLoaded(); + } _messageRenderWindowSize=Math.max(_currentMessageRenderWindowSize(),_messageRenderableMessageCount()); - renderMessages({ preserveScroll:true }); + // During streaming, skip renderMessages — it rebuilds the DOM but tool card + // insertion is blocked by !S.busy, losing Activity until "done" fires. + if(!(S.busy||S.activeStreamId)){ + renderMessages({ preserveScroll:true }); + } requestAnimationFrame(()=>{ container.scrollTop=0; _updateSessionStartJumpButton(); @@ -7437,7 +7446,11 @@ function renderMessages(options){ }); if(derived.length) S.toolCalls=derived; } - if(!S.busy){ + // Render tool cards: allow during streaming when S.toolCalls is already + // populated (e.g. from INFLIGHT restore or SSE events). Only the fallback + // derivation above is blocked by S.busy — DOM insertion should proceed + // whenever tool cards exist. + if(!S.busy || (S.toolCalls&&S.toolCalls.length)){ inner.querySelectorAll('.tool-call-group:not([data-compression-card]),.tool-card-row:not([data-compression-card]),.agent-activity-thinking:not([data-live-thinking="1"])').forEach(el=>el.remove()); const byAssistant = {}; for(const tc of (S.toolCalls||[])){ From 55026d318eb60610955bbf2b94adcd7fa2b83909 Mon Sep 17 00:00:00 2001 From: mysoul12138 <839465496@qq.com> Date: Sat, 6 Jun 2026 00:42:56 +0800 Subject: [PATCH 6/8] fix: add tool_calls guard to merge watermark paths + regression tests Two timestamp-based skip paths in merge_session_messages_append_only unconditionally skipped legacy messages with timestamp <= max_sidecar, without checking whether the merge_key was already registered by the sidecar. Messages with different tool_calls (but identical empty content and same-second timestamp) were silently dropped. Fix: add 'key in seen_message_keys' guard to both watermark paths (lines ~4225 and ~4258) so only true duplicates are skipped. Add test_merge_key_tool_calls.py with 10 regression tests covering merge_key, dedup_key, visible_key, and end-to-end merge behavior with same/different tool_calls. --- api/models.py | 9 +- tests/test_merge_key_tool_calls.py | 136 +++++++++++++++++++++++++++++ 2 files changed, 144 insertions(+), 1 deletion(-) create mode 100644 tests/test_merge_key_tool_calls.py diff --git a/api/models.py b/api/models.py index 5f45f3fdcd5..1d4cfefb703 100644 --- a/api/models.py +++ b/api/models.py @@ -4235,7 +4235,13 @@ def merge_session_messages_append_only( if key in seen_message_keys and key[0] == "message_id": continue if not (isinstance(key, tuple) and key[:1] == ("message_id",)): - continue + # Legacy key within sidecar timestamp range — only skip if + # this exact merge_key was already registered by the sidecar. + # Different tool_calls produce different merge_keys even with + # identical content/timestamp, so an unchecked continue here + # would drop legitimately distinct turns. (#3346 / PR #3665) + if key in seen_message_keys: + continue if key in seen_message_keys and key[0] == "message_id": continue matched_visible_key = _matching_visible_duplicate( @@ -4264,6 +4270,7 @@ def merge_session_messages_append_only( and max_sidecar_timestamp is not None and timestamp is not None and timestamp <= max_sidecar_timestamp + and key in seen_message_keys ): continue seen_message_keys.add(key) diff --git a/tests/test_merge_key_tool_calls.py b/tests/test_merge_key_tool_calls.py new file mode 100644 index 00000000000..cc7ce219a14 --- /dev/null +++ b/tests/test_merge_key_tool_calls.py @@ -0,0 +1,136 @@ +"""Regression tests for PR #3665: tool_calls included in merge/dedup/visible keys. + +Verifies that _session_message_merge_key, _session_message_dedup_key, +_session_message_visible_key, and _matching_visible_duplicate correctly +distinguish messages with different tool_calls arrays. + +Without tool_calls in the key, assistant messages that invoke different +tools (but share empty content and same-second timestamp) collapse into +a single key, losing tool calls during merge. +""" +from __future__ import annotations + +import json + +import pytest + +from api.models import ( + _matching_visible_duplicate, + _session_message_dedup_key, + _session_message_merge_key, + _session_message_visible_key, + merge_session_messages_append_only, +) + + +def _assistant_tc(tc_id: str, fn_name: str, timestamp=1000) -> dict: + """Assistant message with tool_calls but empty content.""" + return { + "role": "assistant", + "content": "", + "timestamp": timestamp, + "tool_calls": [ + {"id": tc_id, "function": {"name": fn_name, "arguments": "{}"}}, + ], + } + + +def _tool_result(tc_id: str, name: str, content: str = "ok") -> dict: + return { + "role": "tool", + "tool_call_id": tc_id, + "name": name, + "content": content, + } + + +# ── _session_message_merge_key ────────────────────────────────────────────── + + +class TestMergeKeyToolCalls: + def test_same_tool_calls_produce_same_key(self): + a = _assistant_tc("call_1", "read_file") + b = _assistant_tc("call_1", "read_file") + assert _session_message_merge_key(a) == _session_message_merge_key(b) + + def test_different_tool_calls_produce_different_keys(self): + a = _assistant_tc("call_1", "read_file") + b = _assistant_tc("call_2", "terminal") + assert _session_message_merge_key(a) != _session_message_merge_key(b) + + def test_empty_vs_nonempty_tool_calls_differ(self): + empty = {"role": "assistant", "content": "", "timestamp": 1000} + with_tc = _assistant_tc("call_1", "read_file") + assert _session_message_merge_key(empty) != _session_message_merge_key(with_tc) + + +# ── _session_message_dedup_key ────────────────────────────────────────────── + + +class TestDedupKeyToolCalls: + def test_same_tool_calls_produce_same_key(self): + a = _assistant_tc("call_1", "read_file") + b = _assistant_tc("call_1", "read_file") + assert _session_message_dedup_key(a) == _session_message_dedup_key(b) + + def test_different_tool_calls_produce_different_keys(self): + a = _assistant_tc("call_1", "read_file") + b = _assistant_tc("call_2", "terminal") + assert _session_message_dedup_key(a) != _session_message_dedup_key(b) + + +# ── _session_message_visible_key + _matching_visible_duplicate ────────────── + + +class TestVisibleKeyToolCalls: + def test_same_tool_calls_match(self): + a = _assistant_tc("call_1", "read_file") + b = _assistant_tc("call_1", "read_file") + ka = _session_message_visible_key(a) + kb = _session_message_visible_key(b) + assert ka == kb + assert _matching_visible_duplicate(ka, {kb}) is not None + + def test_different_tool_calls_no_match(self): + a = _assistant_tc("call_1", "read_file") + b = _assistant_tc("call_2", "terminal") + ka = _session_message_visible_key(a) + kb = _session_message_visible_key(b) + assert ka != kb + assert _matching_visible_duplicate(ka, {kb}) is None + + +# ── merge_session_messages_append_only end-to-end ─────────────────────────── + + +class TestMergeToolCallsEndToEnd: + def test_same_tool_calls_sidecar_and_state_merge_to_one(self): + """Sidecar and state.db have the same assistant message with identical + tool_calls → merge must produce exactly one message (deduplicated).""" + msg = _assistant_tc("call_1", "read_file", timestamp=1000) + sidecar = [msg] + state = [msg] + result = merge_session_messages_append_only(sidecar, state) + assert len(result) == 1, f"expected 1 (deduped), got {len(result)}" + + def test_different_tool_calls_sidecar_and_state_both_preserved(self): + """Sidecar and state.db have assistant messages with different + tool_calls → merge must preserve both (they are distinct turns).""" + msg_a = _assistant_tc("call_1", "read_file", timestamp=1000) + msg_b = _assistant_tc("call_2", "terminal", timestamp=1000) + sidecar = [msg_a] + state = [msg_b] + result = merge_session_messages_append_only(sidecar, state) + assert len(result) == 2, f"expected 2 (distinct), got {len(result)}" + tc_ids = { + m["tool_calls"][0]["id"] + for m in result + if m.get("tool_calls") + } + assert tc_ids == {"call_1", "call_2"} + + def test_no_tool_calls_still_deduped(self): + """Messages without tool_calls are deduplicated by legacy key as before.""" + msg = {"role": "assistant", "content": "hello", "timestamp": 1000} + result = merge_session_messages_append_only([msg], [msg]) + assert len(result) == 1 From d2dc3f4f189f659ba745a14eefe609bc23de4fe1 Mon Sep 17 00:00:00 2001 From: mysoul12138 <839465496@qq.com> Date: Sat, 6 Jun 2026 01:35:21 +0800 Subject: [PATCH 7/8] fix: lint errors and test window size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - test_merge_key_tool_calls: remove unused json/pytest imports (F401) - test_issue3306: increase _ensure_messages_loaded_body window from 2500 to 3000 chars — the INFLIGHT guard addition pushed _pendingCarryForwardSnapshot = null past the old boundary (2505) --- tests/test_issue3306_loadsession_carry_forward.py | 2 +- tests/test_merge_key_tool_calls.py | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/test_issue3306_loadsession_carry_forward.py b/tests/test_issue3306_loadsession_carry_forward.py index 5074da74ed8..b0c3bd436bd 100644 --- a/tests/test_issue3306_loadsession_carry_forward.py +++ b/tests/test_issue3306_loadsession_carry_forward.py @@ -31,7 +31,7 @@ def _load_session_clear_block() -> str: def _ensure_messages_loaded_body() -> str: start = SESSIONS_JS.index("async function _ensureMessagesLoaded") - return SESSIONS_JS[start: start + 2500] + return SESSIONS_JS[start: start + 3000] def test_pending_carry_forward_snapshot_declared_at_module_scope(): diff --git a/tests/test_merge_key_tool_calls.py b/tests/test_merge_key_tool_calls.py index cc7ce219a14..40ebd08343b 100644 --- a/tests/test_merge_key_tool_calls.py +++ b/tests/test_merge_key_tool_calls.py @@ -10,10 +10,6 @@ """ from __future__ import annotations -import json - -import pytest - from api.models import ( _matching_visible_duplicate, _session_message_dedup_key, From e03d6ff406bdc8e8a8605f5064e22d81ceaadef1 Mon Sep 17 00:00:00 2001 From: mysoul12138 <839465496@qq.com> Date: Sat, 6 Jun 2026 03:03:04 +0800 Subject: [PATCH 8/8] fix: watermark guard must not unconditionally skip legacy state.db messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The second watermark path (legacy key, timestamp <= max_sidecar_timestamp) previously used 'key in seen_message_keys' which was too narrow — it let through state.db messages whose merge_key differed from the sidecar only by timestamp (e.g. pending_user_message with 'now' timestamp vs state.db with the original timestamp), causing duplicates in gateway_chat tests. Restore unconditional skip for legacy messages WITHOUT tool_calls. For messages WITH tool_calls, only skip if the sidecar has the same content but identical tool_calls (same dedup_key); if tool_calls differ (same content_key, different dedup_key), preserve the state.db message to avoid collapsing distinct tool-call turns. --- api/models.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/api/models.py b/api/models.py index 1d4cfefb703..c56c2ecba33 100644 --- a/api/models.py +++ b/api/models.py @@ -4270,9 +4270,21 @@ def merge_session_messages_append_only( and max_sidecar_timestamp is not None and timestamp is not None and timestamp <= max_sidecar_timestamp - and key in seen_message_keys ): - continue + # Legacy key within sidecar timestamp range. Normally skip — the + # sidecar already has this message. Exception: if the state.db + # message has tool_calls that DIFFER from the sidecar version + # (same content_key but different dedup_key because tool_calls + # differ), preserve it — distinct tool_calls must not be collapsed. + _tc = msg.get("tool_calls") + if _tc: + _ck = _session_message_content_key(msg) + if _ck in seen_content_keys and dedup_key not in seen_dedup_keys: + pass # different tool_calls from sidecar — preserve + else: + continue + else: + continue seen_message_keys.add(key) seen_dedup_keys.add(dedup_key) seen_content_keys.add(_session_message_content_key(msg))