diff --git a/static/messages.js b/static/messages.js index 9f1532f146a..75a3ddbed31 100644 --- a/static/messages.js +++ b/static/messages.js @@ -5941,10 +5941,17 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ // render also populates the cache with the correctly-collapsed DOM, and // the same-frame JS restore absorbs the collapse so there is no jump. // (#5260 gate-cert: keep-open must be transient + uncached for everyone.) + // #6385: capture the scroll snapshot from the LIVE DOM before arming + // keep-open, so the collapse render below anchors to the content the + // reader was actually viewing — not to a stale intermediate state where + // the worklog was temporarily expanded. + const _doneLiveScrollSnapshot=typeof _captureMessageScrollSnapshot==='function' + ? _captureMessageScrollSnapshot() + : null; if(typeof _armKeepSettledWorklogOpen==='function') _armKeepSettledWorklogOpen(_settledStreamId); syncTopbar();renderMessages({preserveScroll:true}); if(typeof _disarmKeepSettledWorklogOpen==='function') _disarmKeepSettledWorklogOpen(); - if(typeof _renderMessagesWithScrollSnapshot==='function') _renderMessagesWithScrollSnapshot(); + if(typeof _renderMessagesWithScrollSnapshot==='function') _renderMessagesWithScrollSnapshot({_prescrollSnapshot:_doneLiveScrollSnapshot}); else renderMessages({preserveScroll:true}); if(shouldFollowOnDone&&typeof scrollToBottom==='function') scrollToBottom(); if(typeof noteWorkspaceMutationsFromToolCalls==='function') noteWorkspaceMutationsFromToolCalls(S.toolCalls); diff --git a/static/ui.js b/static/ui.js index 3e92cd3411b..a156eb3a353 100644 --- a/static/ui.js +++ b/static/ui.js @@ -15086,7 +15086,13 @@ function _restoreMessageScrollSnapshotSameFrame(snapshot){ } } function _renderMessagesWithScrollSnapshot(options){ - const scrollSnapshot=_captureMessageScrollSnapshot(); + // Accept an optional pre-captured scroll snapshot via _prescrollSnapshot. + // When provided, it is used INSTEAD of capturing a fresh one from the current + // DOM state — essential for the STREAM_DONE collapse render: the caller has + // already captured the snapshot from the LIVE DOM (before keep-open was armed), + // and re-capturing from the intermediate expanded-worklog state would capture + // stale anchors that no longer exist after the worklog collapses. (#6385) + const scrollSnapshot=(options&&options._prescrollSnapshot)||_captureMessageScrollSnapshot(); renderMessages({...(options||{}),preserveScroll:true}); _restoreMessageScrollSnapshotSameFrame(scrollSnapshot); } diff --git a/tests/test_issue4970_stream_done_shrink_regression.py b/tests/test_issue4970_stream_done_shrink_regression.py index 0a4fa8d7cdf..e4de60ee3d0 100644 --- a/tests/test_issue4970_stream_done_shrink_regression.py +++ b/tests/test_issue4970_stream_done_shrink_regression.py @@ -171,18 +171,32 @@ def test_stream_done_runs_scroll_preserving_collapse_pass_after_disarm(): # UNCONDITIONALLY right after disarm (covers both pin states), THEN scrollToBottom() # only for followers to re-settle at the tail. This makes keep-open genuinely # one-frame for everyone. + # + # Round 10 (#6385): capture the scroll snapshot from the LIVE DOM before arming + # keep-open, so the collapse render below anchors to the content the reader was + # actually viewing — not to a stale intermediate state where the worklog was + # temporarily expanded. The pre-capture variable must be defined before arm and + # threaded into the collapse pass as `_prescrollSnapshot`. + pre_capture_idx = MESSAGES_JS.index("_doneLiveScrollSnapshot") + arm_idx = MESSAGES_JS.index("_armKeepSettledWorklogOpen") + assert pre_capture_idx < arm_idx, ( + "_doneLiveScrollSnapshot must be captured before _armKeepSettledWorklogOpen " + "to snapshot the live (not expanded-worklog) DOM positions." + ) disarm_idx = MESSAGES_JS.index("_disarmKeepSettledWorklogOpen()") after = MESSAGES_JS[disarm_idx : disarm_idx + 700] - # The collapse pass must run after disarm for BOTH pin states. - assert "_renderMessagesWithScrollSnapshot()" in after, ( + # The collapse pass must run after disarm for BOTH pin states, and it must + # carry the pre-captured live snapshot. + assert "_renderMessagesWithScrollSnapshot({_prescrollSnapshot:_doneLiveScrollSnapshot})" in after, ( "after _disarmKeepSettledWorklogOpen() the STREAM_DONE handler must run a " - "scroll-preserving collapse pass (_renderMessagesWithScrollSnapshot) so the " - "forced-open worklog collapses back to the user/live state without the jump." + "scroll-preserving collapse pass (_renderMessagesWithScrollSnapshot) with the " + "pre-captured live snapshot so the forced-open worklog collapses back to " + "the user/live state without the jump." ) # The follower re-settle (scrollToBottom) must come AFTER the collapse render — # otherwise a pinned follower keeps the forced-open DOM (scrollToBottom does not # re-render). This is the exact pinned-path bug the second RED gate-cert caught. - collapse_pos = after.index("_renderMessagesWithScrollSnapshot()") + collapse_pos = after.index("_renderMessagesWithScrollSnapshot") follow_pos = after.index("shouldFollowOnDone") assert collapse_pos < follow_pos, ( "the collapse render must run BEFORE the shouldFollowOnDone scrollToBottom() " @@ -200,3 +214,86 @@ def test_stream_done_runs_scroll_preserving_collapse_pass_after_disarm(): assert "_captureMessageScrollSnapshot()" in wrapper assert "_restoreMessageScrollSnapshotSameFrame" in wrapper + +@pytest.mark.skipif(shutil.which("node") is None, reason="node required for behavioral test") +def test_prescroll_snapshot_bypasses_capture_no_option_fallback_still_captures(): + """Sentinel _prescrollSnapshot reaches restore without re-capture. + + Behavioral harness for _renderMessagesWithScrollSnapshot: + + 1. Supply a sentinel snapshot via _prescrollSnapshot, stub + _captureMessageScrollSnapshot with a counter — proves the + sentinel reaches _restoreMessageScrollSnapshotSameFrame + without a second capture. + + 2. Call with no options — proves the no-option fallback still + calls _captureMessageScrollSnapshot() normally (the contract + for callers at static/ui.js:9565, 14416, 14424). + + 3. Call with empty options {} — same fallback proof. + """ + wrapper = _extract("_renderMessagesWithScrollSnapshot") + harness = textwrap.dedent(f""" + let captureCount = 0; + let restoredSnapshot = null; + let renderedOptions = null; + globalThis._captureMessageScrollSnapshot = () => {{ + captureCount++; + return {{_sentinel: 'fresh-capture', bottom: 0, pinned: true}}; + }}; + globalThis.renderMessages = (opts) => {{ renderedOptions = opts; }}; + globalThis._restoreMessageScrollSnapshotSameFrame = (snap) => {{ restoredSnapshot = snap; }}; + {wrapper} + // Test 1: supplied _prescrollSnapshot -> use it, do NOT capture + const sentinel = {{_sentinel: 'pre-captured', bottom: 42, pinned: true}}; + _renderMessagesWithScrollSnapshot({{_prescrollSnapshot: sentinel}}); + const test1_usedPrescroll = restoredSnapshot === sentinel; + const test1_noCapture = captureCount === 0; + const test1_preservedArgs = renderedOptions && renderedOptions._prescrollSnapshot === sentinel; + // Reset + captureCount = 0; restoredSnapshot = null; renderedOptions = null; + // Test 2: no options -> fall back to _captureMessageScrollSnapshot() + _renderMessagesWithScrollSnapshot(); + const test2_captured = captureCount === 1; + const test2_usedCapture = restoredSnapshot && restoredSnapshot._sentinel === 'fresh-capture'; + // Reset + captureCount = 0; restoredSnapshot = null; renderedOptions = null; + // Test 3: empty options object -> still fall back to capture + _renderMessagesWithScrollSnapshot({{}}); + const test3_captured = captureCount === 1; + const test3_usedCapture = restoredSnapshot && restoredSnapshot._sentinel === 'fresh-capture'; + console.log(JSON.stringify({{ + test1_usedPrescroll, + test1_noCapture, + test1_preservedArgs, + test2_captured, + test2_usedCapture, + test3_captured, + test3_usedCapture + }})); + """) + res = subprocess.run(["node", "-e", harness], capture_output=True, text=True, timeout=30) + assert res.returncode == 0, res.stderr + out = json.loads(res.stdout.strip()) + assert out["test1_usedPrescroll"] is True, ( + "supplied _prescrollSnapshot must reach _restoreMessageScrollSnapshotSameFrame" + ) + assert out["test1_noCapture"] is True, ( + "supplied _prescrollSnapshot must bypass _captureMessageScrollSnapshot()" + ) + assert out["test1_preservedArgs"] is True, ( + "options passed to _renderMessagesWithScrollSnapshot must be forwarded to renderMessages" + ) + assert out["test2_captured"] is True, ( + "no-option call must fall back to _captureMessageScrollSnapshot()" + ) + assert out["test2_usedCapture"] is True, ( + "no-option call's captured snapshot must reach _restoreMessageScrollSnapshotSameFrame" + ) + assert out["test3_captured"] is True, ( + "empty-options call must fall back to _captureMessageScrollSnapshot()" + ) + assert out["test3_usedCapture"] is True, ( + "empty-options call's captured snapshot must reach _restoreMessageScrollSnapshotSameFrame" + ) +