fix: streaming Activity loss during scrolling

[mysoul12138]

Problem

During active streaming, loading earlier messages (scroll, button, or jump-to-start) causes:

• Activity panel: all tool calls disappear (entire session)
• Chat area: current streaming turn messages disappear

Root cause: _syncToolCallsForLoadedMessages clears S.toolCalls when messages have tool metadata, and S.busy=true blocks the renderMessages fallback rebuild. Additionally, _session_message_merge_key didn't include tool_calls, causing different tool-call messages to be incorrectly deduplicated.

Changes

Server (api/models.py) — data layer

• _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 (root cause)

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: _streamJustFinished cooldown + render window expansion
• _restoreSettledSession: render window expansion

Client (static/ui.js)

• Align _messageRenderableMessageCount with _visWithIdx filter

[greptile-apps]

Greptile Summary

This PR fixes Activity panel and chat message loss during scroll operations while streaming by addressing multiple root causes: including tool_calls in the server-side dedup/merge/visible key functions so tool-calling messages are no longer collapsed, removing the client-side _syncToolCallsForLoadedMessages clear that fired during streaming, and guarding refreshActiveSessionIfExternallyUpdated with a 5-second post-stream cooldown.

• Server (models.py / routes.py): tool_calls is now included in all three key functions and the session-level tool_calls list is always returned to the client.
• Client (sessions.js / messages.js / ui.js): _syncToolCallsForLoadedMessages now returns immediately when S.busy or S.activeStreamId is set; render-window size is expanded at key handoff points; renderMessages now re-renders settled tool cards even during streaming if S.toolCalls is populated.

Confidence Score: 5/5

Safe to merge; all changes are tightly scoped to the streaming/scroll race and the server-side dedup logic, with targeted regression tests added for the key paths.

The server-side key changes are well-guarded by new tests covering same/different tool_call identity, end-to-end merge, and the tail-payload shape. The client-side guards close discrete, understood races rather than introducing new state. No data-loss or auth paths are touched.

No files require special attention beyond the pre-existing observations already noted in earlier review threads.

Important Files Changed

Filename	Overview
api/models.py	Adds tool_calls to merge/dedup/visible key functions and refines the sidecar-timestamp guard to preserve distinct tool-call messages; _matching_visible_duplicate updated to handle new 3-tuple keys.
api/routes.py	Removes conditional that zeroed _session_tool_calls when messages have per-message tool metadata; session-level tool_calls are now always returned.
static/sessions.js	Adds streaming guard to _syncToolCallsForLoadedMessages and INFLIGHT guard to _ensureMessagesLoaded; expands render window after loading; adds _streamJustFinished cooldown to external-refresh gate.
static/messages.js	Adds _streamJustFinished cooldown and render-window expansion in the done handler and _restoreSettledSession; clears visible-message-row cache after session data arrives.
static/ui.js	renderMessages tool-card block now fires whenever S.toolCalls is non-empty; _visWithIdxCacheSrc reference check prevents stale cache on same-length wholesale replacement; _messageRenderableMessageCount now excludes recovery-control messages.
tests/test_merge_key_tool_calls.py	New regression test file covering merge-key, dedup-key, visible-key, and end-to-end merge behaviour for messages with distinct tool_calls.
tests/test_session_tail_payload.py	Renamed and updated test verifies session-level tool_calls are always returned windowed rather than cleared when messages have per-message tool metadata.

_{Reviews (9): Last reviewed commit: "Merge branch 'master' into fix/streaming..." | Re-trigger Greptile}

[nesquena-hermes]

Pulled the branch and read the diff across api/models.py, api/routes.py, static/sessions.js, static/messages.js, static/ui.js against origin/master, plus the merge reconciliation body merge_session_messages_append_only (models.py:4081-4262) where the key functions are consumed. The diagnosis is sound and the four-part fix (server payload + merge key + busy-guard + render-window) addresses distinct facets of the same symptom. A few things worth confirming before merge.

Server payload change is correct

Dropping the _messages_include_tool_metadata branch in routes.py:5122 and always returning the windowed session-level tool_calls is the right call:

# Always include session-level tool_calls so the browser can merge
# them with per-message tool_calls ...
if _windowed_messages:
    _session_tool_calls = _tool_calls_for_message_window(...)

The renamed test_session_tail_payload.py test now asserts the windowed list is returned (assistant_msg_idx: 0 preserved) rather than [], which matches the new contract. Good.

The merge-key change is the root-cause fix — but it's untested

Adding _tc_key to _session_message_visible_key, _session_message_dedup_key, and _session_message_merge_key (models.py:3896-3946) is the deepest change here, because those keys drive the sidecar↔state.db reconciliation in merge_session_messages_append_only. The _matching_visible_duplicate tuple-shape refactor (models.py:4000-4023) is internally consistent — by_role, loose_by_key, and the skipped_state_visible_counts/sidecar_visible_counts dicts are all keyed by the full existing_key, and the len(visible_key) > 1 guards are safe.

My concern: none of the three changed key functions has a direct regression test in this PR. grep -rln "_session_message_visible_key" tests/ returns nothing. The new _tc_key uses json.dumps(_tc, sort_keys=True, default=str) — so the two stores must serialize tool_calls identically for the same logical message, or the visible_key diverges and a row that previously deduped will now be emitted twice. If state.db ever carries tool_calls: None while the sidecar carries [] (or vice versa), _tc_key is "" in both cases (the if _tc else "" guard handles falsy), so that's fine — but if one store has the populated list and the other has it stripped/normalized differently (key ordering is handled by sort_keys, but nested non-serializable values fall back to default=str repr, which can differ), the keys won't match. This is exactly the append-only-duplication failure mode that test_issue2592_partial_dedupe.py and test_session_lost_response_regression.py exist to guard.

Recommend adding a test alongside those that feeds merge_session_messages_append_only a sidecar row + a state.db row representing the same tool-calling assistant message (matching tool_calls) and asserts they collapse to one, plus a second case with different tool_calls asserting both survive. That locks the actual contract rather than the source shape.

Client guards look right

The renderMessages change at ui.js:7089 —

if(!S.busy || (S.toolCalls&&S.toolCalls.length)){

— correctly still excludes live thinking via :not([data-live-thinking="1"]) in the removal selector (ui.js:7093), so allowing DOM insertion during S.busy won't strip the in-progress thinking card. The _streamJustFinished 5s cooldown in messages.js:2081 + the matching guard in refreshActiveSessionIfExternallyUpdated (sessions.js:2921) is a reasonable way to stop the post-done external-refresh from clobbering S.toolCalls. One nit: _streamJustFinished is a window global with a bare 5s setTimeout — if two turns complete within 5s the second's timer resets fine, but if a reload races the timer there's no cleanup on navigation. Low risk given it fails safe (just permits a refresh slightly early).

test_smooth_text_fade change

Switching that assertion from function_block(...) to scanning the whole MESSAGES_JS because the brace-counter chokes on ${...} template literals is pragmatic, but it weakens the test — the strings are no longer scoped to _scheduleRender, so an identical string appearing elsewhere would mask a regression. Acceptable given the parser limitation, just noting the reduced specificity.

Overall this is a solid fix for a real, multi-layered bug. The main ask is server-side merge-key test coverage before merge, since that's the highest-blast-radius change and currently unverified.