Merge pull request #4726 from nesquena/stage-4721-release

Release v0.51.591 — Release UX (stop transcript jumping to first message, #4721/#4720)

CHANGELOG.md

@@ -3,6 +3,12 @@
 
 ## [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)
+
 ## [v0.51.590] — 2026-06-22 — Release UW (close cross-profile credential leak on /api/providers + /api/models)
 
 ### Fixed

static/messages.js

@@ -4178,6 +4178,8 @@ 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: 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);
           if(typeof clearVisibleMessageRowCache==='function') clearVisibleMessageRowCache();

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)

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