fix: reset _oldestIdx on stream done so transcript stops jumping to first message (#4720)

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.
This commit is contained in:
nesquena-hermes committed 2026-06-22 19:26:07 +00:00
1 parent 80d6514c9d
commit ab71f4b906
3 files changed
+87

No files matched your search

+4
View File
@@ -3,6 +3,10 @@
## [Unreleased] ## [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) ## [v0.51.589] — 2026-06-22 — Release UV (guard state.db replay after compressed anchors)
### Fixed ### Fixed
+9
View File
@@ -4178,6 +4178,15 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
const _prevCacheRead=(S.session&&S.session.cache_read_tokens)||0; const _prevCacheRead=(S.session&&S.session.cache_read_tokens)||0;
const _prevCacheWrite=(S.session&&S.session.cache_write_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; 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 || []); S.messages=_filterRecoveryControlMessages(S.messages || []);
if(typeof _hydrateTodosFromSession==='function') _hydrateTodosFromSession(S.session); if(typeof _hydrateTodosFromSession==='function') _hydrateTodosFromSession(S.session);
if(typeof clearVisibleMessageRowCache==='function') clearVisibleMessageRowCache(); if(typeof clearVisibleMessageRowCache==='function') clearVisibleMessageRowCache();
@@ -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)