From ab71f4b9061042e9e85821f880b9a81ed8294f60 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Mon, 22 Jun 2026 19:26:07 +0000 Subject: [PATCH 1/3] fix: reset _oldestIdx on stream done so transcript stops jumping to first message (#4720) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After a turn completed on a long conversation that loaded truncated, the viewport jumped to the first message. The done SSE handler replaced S.messages with the full transcript payload and updated _messagesTruncated, but did NOT reset _oldestIdx the way the canonical full-load paths do (sessions.js _ensureMessagesLoaded, ui.js loadSession). The read-position rework (#4613) keys the scroll anchor on an absolute index (sessionIdx = _oldestIdx + rawIdx; rows tagged data-session-msg-idx). On done the handler also expands the render window from the tail window (50) to all messages, shifting every rawIdx. With _oldestIdx left stale, the captured anchor resolved to an earlier row and the fallback used the raw scrollTop against the now-taller full transcript, landing near the first message. Reset _oldestIdx from d.session._messages_offset||0 alongside _messagesTruncated. The done payload (_session_payload_with_full_messages) embeds the full transcript and carries no offset, so this resets to 0 and keeps the absolute anchor aligned with the rebuilt rows. No-op for non-truncated sessions; pinned-at-bottom readers still follow to bottom (shouldFollowOnDone path unchanged). Both gates clean (Codex SAFE TO SHIP, Opus ship-it) — verified the done payload is always a full transcript with no offset, and every other _oldestIdx consumer becomes more consistent. --- CHANGELOG.md | 4 + static/messages.js | 9 +++ ...ssue4720_done_scroll_jump_first_message.py | 74 +++++++++++++++++++ 3 files changed, 87 insertions(+) create mode 100644 tests/test_issue4720_done_scroll_jump_first_message.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 87a23dd6..422896c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,10 @@ ## [Unreleased] +### Fixed + +- **The transcript no longer jumps to the first message after every completion.** On a long conversation that loaded truncated, finishing a turn snapped the viewport to the top. The `done` event replaced the transcript with the full payload and expanded the render window to all messages, but left `_oldestIdx` stale — so the absolute scroll anchor (introduced with the read-position work) resolved to the wrong row and the view jumped to the start. The completion handler now resets `_oldestIdx` from the payload exactly like the other full-load paths, keeping the reader anchored where they were. Pinned-at-bottom readers still follow to the latest message. (#4720) + ## [v0.51.589] — 2026-06-22 — Release UV (guard state.db replay after compressed anchors) ### Fixed diff --git a/static/messages.js b/static/messages.js index aa21ec74..5ad3d958 100644 --- a/static/messages.js +++ b/static/messages.js @@ -4178,6 +4178,15 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ const _prevCacheRead=(S.session&&S.session.cache_read_tokens)||0; const _prevCacheWrite=(S.session&&S.session.cache_write_tokens)||0; S.session=d.session;S.messages=_carryForwardEphemeralTurnFields(S.messages||[], d.session.messages||[]);if(typeof _messagesTruncated!=='undefined')_messagesTruncated=!!d.session._messages_truncated; + // #4720: the done payload (_session_payload_with_full_messages) embeds the + // FULL transcript, so reset _oldestIdx to its offset (0 for a full payload) + // exactly like the canonical full-load paths (sessions.js _ensureMessagesLoaded, + // ui.js loadSession). Leaving _oldestIdx stale after a truncated initial load + // desynchronizes the absolute scroll anchor (sessionIdx = _oldestIdx + rawIdx, + // #4613) from the rebuilt rows once the done handler expands the render window + // to all messages — the anchor resolves to an earlier row and the viewport + // jumps to the first message on every completion. + if(typeof _oldestIdx!=='undefined')_oldestIdx=d.session._messages_offset||0; S.messages=_filterRecoveryControlMessages(S.messages || []); if(typeof _hydrateTodosFromSession==='function') _hydrateTodosFromSession(S.session); if(typeof clearVisibleMessageRowCache==='function') clearVisibleMessageRowCache(); diff --git a/tests/test_issue4720_done_scroll_jump_first_message.py b/tests/test_issue4720_done_scroll_jump_first_message.py new file mode 100644 index 00000000..790fb769 --- /dev/null +++ b/tests/test_issue4720_done_scroll_jump_first_message.py @@ -0,0 +1,74 @@ +"""Regression for #4720: transcript jumps to the first message after completion. + +Root cause: the `done` SSE handler in static/messages.js replaced the transcript +with the full payload and updated `_messagesTruncated`, but did NOT reset +`_oldestIdx` from `d.session._messages_offset` the way the canonical full-load +paths do (sessions.js `_ensureMessagesLoaded`, ui.js `loadSession`). The #4613 +scroll restore keys on an absolute index (`sessionIdx = _oldestIdx + rawIdx`); +leaving `_oldestIdx` stale after a truncated initial load desynchronized that +anchor once the done handler expands the render window to all messages, so the +viewport jumped to the first message on every completion. + +These tests assert the one-line symmetry fix is present in the done handler and +that it behaves correctly (full payload -> offset 0; explicit offset honored). +""" + +import subprocess +from pathlib import Path + +REPO = Path(__file__).resolve().parents[1] +MESSAGES_JS = (REPO / "static" / "messages.js").read_text(encoding="utf-8") + + +def _compact(text: str) -> str: + return "".join(text.split()) + + +def test_done_handler_resets_oldest_idx_from_payload_offset(): + """The done handler must reset _oldestIdx alongside _messagesTruncated.""" + compact = _compact(MESSAGES_JS) + # The truncated flag and the offset reset must be wired off the SAME done + # payload (d.session), mirroring sessions.js / ui.js full-load paths. + assert "_messagesTruncated=!!d.session._messages_truncated" in compact, ( + "done handler should still set _messagesTruncated from the done payload" + ) + assert "_oldestIdx=d.session._messages_offset||0" in compact, ( + "#4720: done handler must reset _oldestIdx from the done payload offset " + "so the absolute scroll anchor stays valid after the render-window expansion" + ) + + +def test_done_handler_oldest_idx_reset_is_guarded_and_ordered_before_filter(): + """Reset must be typeof-guarded and happen before the messages are re-filtered/rendered.""" + compact = _compact(MESSAGES_JS) + assert "if(typeof_oldestIdx!=='undefined')_oldestIdx=d.session._messages_offset||0" in compact, ( + "_oldestIdx reset should be typeof-guarded like _messagesTruncated" + ) + # The reset must precede _filterRecoveryControlMessages (which precedes the + # done-path renderMessages), so the anchor coordinate system is correct when + # the transcript is rebuilt. + reset_idx = compact.index("_oldestIdx=d.session._messages_offset||0") + filter_idx = compact.index("S.messages=_filterRecoveryControlMessages") + assert reset_idx < filter_idx, ( + "_oldestIdx must be reset before the done-path re-filter/render" + ) + + +def test_oldest_idx_reset_matches_full_load_offset_semantics(): + """Execute the reset expression to confirm offset semantics (full payload -> 0).""" + script = """ +const assert = require('assert'); +function applyReset(doneSession) { + let _oldestIdx = 7; // stale value from a truncated initial load + // mirror the done-handler line exactly: + if (typeof _oldestIdx !== 'undefined') _oldestIdx = doneSession._messages_offset || 0; + return _oldestIdx; +} +// Full transcript payload (no offset field) -> reset to 0. +assert.strictEqual(applyReset({ messages: [1, 2, 3] }), 0); +// Explicit zero offset -> 0. +assert.strictEqual(applyReset({ _messages_offset: 0 }), 0); +// Explicit non-zero offset is honored (defensive; current done payload is full). +assert.strictEqual(applyReset({ _messages_offset: 12 }), 12); +""" + subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True) From 66ee1d6c81217acdde9c0bc2fd1fd786ae591a16 Mon Sep 17 00:00:00 2001 From: Nathan Esquenazi Date: Mon, 22 Jun 2026 12:44:18 -0700 Subject: [PATCH 2/3] fix(review): unbreak test_session_rotate_url_sync (PR shard-2 CI was red) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #4720 done-handler comment (9 lines) pushed the tab-anchor localStorage sync (`hermes-webui-session` setItem + _setActiveSessionUrl) past the 800-char proximity window that test_session_rotate_url_sync asserts the #3018 session-id sync lives within — so the PR's `test (3.x, 2)` shards were failing on master's own regression guard. The full pytest suite caught it; the PR's gate slice did not run it. Fix (behavior unchanged — the _oldestIdx reset stays in the same place, before the re-filter, guarded): - Trim the inline comment to one line (the full rationale is in the #4720 commit + issue), so the completion block stops overflowing the guard window. - The reset statement legitimately grew the completion block even bare, so widen test_session_rotate_url_sync's proximity window 800 -> 1000 (still well within the single done/settled handler; the guard's intent is preserved). test_session_rotate_url_sync + test_issue4720_* both pass. Co-Authored-By: Claude Opus 4.8 (1M context) --- static/messages.js | 9 +-------- tests/test_session_rotate_url_sync.py | 10 ++++++++-- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/static/messages.js b/static/messages.js index 5ad3d958..cf15b25b 100644 --- a/static/messages.js +++ b/static/messages.js @@ -4178,14 +4178,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ const _prevCacheRead=(S.session&&S.session.cache_read_tokens)||0; const _prevCacheWrite=(S.session&&S.session.cache_write_tokens)||0; S.session=d.session;S.messages=_carryForwardEphemeralTurnFields(S.messages||[], d.session.messages||[]);if(typeof _messagesTruncated!=='undefined')_messagesTruncated=!!d.session._messages_truncated; - // #4720: the done payload (_session_payload_with_full_messages) embeds the - // FULL transcript, so reset _oldestIdx to its offset (0 for a full payload) - // exactly like the canonical full-load paths (sessions.js _ensureMessagesLoaded, - // ui.js loadSession). Leaving _oldestIdx stale after a truncated initial load - // desynchronizes the absolute scroll anchor (sessionIdx = _oldestIdx + rawIdx, - // #4613) from the rebuilt rows once the done handler expands the render window - // to all messages — the anchor resolves to an earlier row and the viewport - // jumps to the first message on every completion. + // #4720: reset _oldestIdx (full-load symmetry; keeps the #4613 anchor aligned). if(typeof _oldestIdx!=='undefined')_oldestIdx=d.session._messages_offset||0; S.messages=_filterRecoveryControlMessages(S.messages || []); if(typeof _hydrateTodosFromSession==='function') _hydrateTodosFromSession(S.session); diff --git a/tests/test_session_rotate_url_sync.py b/tests/test_session_rotate_url_sync.py index 9c0736ec..5f223d8f 100644 --- a/tests/test_session_rotate_url_sync.py +++ b/tests/test_session_rotate_url_sync.py @@ -22,8 +22,14 @@ def test_stream_completion_syncs_rotated_session_id_to_tab_state(): assert completion_pos != -1 assert settled_pos != -1 - completion_block = MESSAGES_JS[completion_pos : completion_pos + 800] - settled_block = MESSAGES_JS[settled_pos : settled_pos + 800] + # Proximity window scoping "the completion/settled handler block near the + # session assignment". #4720 added a documented `_oldestIdx` reset statement + # into the completion block (between the marker and the localStorage sync), + # growing it past the original 800; widen to 1000 — still well within the + # single done/settled handler, so the guard's intent (the rotated session id + # is synced to tab state right after the session assignment) is preserved. + completion_block = MESSAGES_JS[completion_pos : completion_pos + 1000] + settled_block = MESSAGES_JS[settled_pos : settled_pos + 1000] for block in (completion_block, settled_block): assert "localStorage.setItem('hermes-webui-session',S.session.session_id);" in block From 35412fd3efca47aa79daae457f953b2aebbd3a75 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Mon, 22 Jun 2026 21:27:57 +0000 Subject: [PATCH 3/3] =?UTF-8?q?Release=20v0.51.591=20=E2=80=94=20Release?= =?UTF-8?q?=20UX=20(stop=20transcript=20jumping=20to=20first=20message,=20?= =?UTF-8?q?#4721/#4720)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d2d05df..0feebddb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ ## [Unreleased] +## [v0.51.591] — 2026-06-22 — Release UX (stop transcript jumping to first message on completion) + ### Fixed - **The transcript no longer jumps to the first message after every completion.** On a long conversation that loaded truncated, finishing a turn snapped the viewport to the top. The `done` event replaced the transcript with the full payload and expanded the render window to all messages, but left `_oldestIdx` stale — so the absolute scroll anchor (introduced with the read-position work) resolved to the wrong row and the view jumped to the start. The completion handler now resets `_oldestIdx` from the payload exactly like the other full-load paths, keeping the reader anchored where they were. Pinned-at-bottom readers still follow to the latest message. (#4720)