fix(chat): scope STREAM_DONE keep-open to just-settled turn via one-shot token
Address maintainer must-fix: _shouldKeepSettledWorklogOpenForPinnedFollow() keyed only on global pin state, so every historical settled worklog re-expanded on any pinned re-render — defeating the compact default. Gate the keep-open exception on a one-shot token = the just-settled stream id (armed at STREAM_DONE renderMessages, disarmed right after), threaded as _shouldKeep...(streamId). Unpinned path unchanged. Replace source-string test with a behavioral Node test that drives a second pinned turn and asserts it collapses.
This commit is contained in:
1 parent
0d198d677d
commit
2aa1d168b0
3 files changed
+105
-35
No files matched your search
@@ -5001,6 +5001,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
|
||||
const shouldFollowOnDone=isActiveSession&&((typeof _shouldFollowMessagesOnDomReplace==='function')
|
||||
? _shouldFollowMessagesOnDomReplace()
|
||||
: (typeof _isMessagePaneNearBottom==='function'&&_isMessagePaneNearBottom(1200)));
|
||||
const _settledStreamId=isActiveSession?(S.activeStreamId||(d&&d.stream_id)||''):'';
|
||||
if(isActiveSession){
|
||||
S.activeStreamId=null;
|
||||
}
|
||||
@@ -5151,7 +5152,13 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
|
||||
// turn boundary so the following syncTopbar() refetches the authoritative
|
||||
// effort exactly once (not per-token — the storm short-circuit is intact).
|
||||
if(typeof _lastReasoningFetchKey!=='undefined') _lastReasoningFetchKey=null;
|
||||
// Arm the one-shot keep-open token for JUST this settled turn so a
|
||||
// pinned follower's worklog stays height-stable (no STREAM_DONE shrink
|
||||
// jump); disarm right after the render so historical worklogs collapse
|
||||
// compact as normal. Scoped to the just-settled stream id.
|
||||
if(typeof _armKeepSettledWorklogOpen==='function') _armKeepSettledWorklogOpen(_settledStreamId);
|
||||
syncTopbar();renderMessages({preserveScroll:true});
|
||||
if(typeof _disarmKeepSettledWorklogOpen==='function') _disarmKeepSettledWorklogOpen();
|
||||
if(shouldFollowOnDone&&typeof scrollToBottom==='function') scrollToBottom();
|
||||
if(typeof noteWorkspaceMutationsFromToolCalls==='function') noteWorkspaceMutationsFromToolCalls(S.toolCalls);
|
||||
loadDir('.', { preservePreview: true });
|
||||
|
||||
+33
-11
@@ -10368,23 +10368,45 @@ function _renderSettledAnchorSceneTransparentForMessage(message, segment, rawIdx
|
||||
}
|
||||
return wrote;
|
||||
}
|
||||
function _shouldKeepSettledWorklogOpenForPinnedFollow(){
|
||||
// One-shot token: the stream id of the turn that JUST settled at STREAM_DONE.
|
||||
// The keep-open exception applies to ONLY this one turn's settled render, then
|
||||
// is cleared so every other (historical) settled worklog renders compact even
|
||||
// while the reader is pinned. Set right before the STREAM_DONE
|
||||
// renderMessages({preserveScroll:true}) call and cleared after the settled-scene
|
||||
// render pass; null at all other times.
|
||||
let _keepSettledWorklogOpenForStreamId=null;
|
||||
function _shouldKeepSettledWorklogOpenForPinnedFollow(streamId){
|
||||
// Round 6 scroll-jump guard: while the reader is pinned at the live tail,
|
||||
// collapsing the live worklog into a compact settled summary can shrink the
|
||||
// transcript by hundreds of px at STREAM_DONE. The browser clamps scrollTop to
|
||||
// the new max, which looks like a large backward jump even though pinned state
|
||||
// is correct. Keep the just-settled worklog open for pinned followers so the
|
||||
// collapsing the JUST-settled live worklog into a compact summary can shrink
|
||||
// the transcript by hundreds of px at STREAM_DONE. The browser clamps scrollTop
|
||||
// to the new max, which looks like a large backward jump even though pinned
|
||||
// state is correct. Keep that one worklog open for pinned followers so the
|
||||
// live->settled DOM swap is height-stable; unpinned readers still get compact
|
||||
// settled worklogs and preserve their viewport normally. This intentionally
|
||||
// wins over a transient user-collapsed live worklog while the reader remains
|
||||
// pinned: avoiding the visible STREAM_DONE jump takes precedence for followers.
|
||||
// Use the sticky pin state as the authority. During live DOM rebuilds the raw
|
||||
// bottom distance can transiently exceed a threshold even for a pinned follower
|
||||
// (the assistant body/worklog grows before follow writes land), so a near-bottom
|
||||
// check here would incorrectly collapse the settled worklog and reintroduce the
|
||||
// STREAM_DONE shrink jump.
|
||||
// SCOPING: the exception is gated on the one-shot token matching this turn's
|
||||
// stream id, so it applies ONLY to the turn that just settled — not to every
|
||||
// historical settled worklog on every pinned re-render (which would defeat the
|
||||
// compact-worklog default for past turns). Pin flags use the sticky pin state
|
||||
// because during live DOM rebuilds the raw bottom distance can transiently
|
||||
// exceed a threshold even for a pinned follower.
|
||||
if(!streamId||_keepSettledWorklogOpenForStreamId!==streamId) return false;
|
||||
return !!(_scrollPinned && !_messageUserUnpinned);
|
||||
}
|
||||
// One-shot token set/clear API used by the STREAM_DONE handler (messages.js):
|
||||
// arm the keep-open exception for exactly the turn that just settled, render,
|
||||
// then disarm so subsequent re-renders collapse historical worklogs as normal.
|
||||
function _armKeepSettledWorklogOpen(streamId){
|
||||
_keepSettledWorklogOpenForStreamId=streamId?String(streamId):null;
|
||||
}
|
||||
function _disarmKeepSettledWorklogOpen(){
|
||||
_keepSettledWorklogOpenForStreamId=null;
|
||||
}
|
||||
if(typeof window!=='undefined'){
|
||||
window._armKeepSettledWorklogOpen=_armKeepSettledWorklogOpen;
|
||||
window._disarmKeepSettledWorklogOpen=_disarmKeepSettledWorklogOpen;
|
||||
}
|
||||
function _renderSettledAnchorSceneForMessage(message, segment, rawIdx){
|
||||
if(!message||!message._anchor_activity_scene||!segment) return false;
|
||||
if(typeof isTransparentStream==='function'&&isTransparentStream()){
|
||||
@@ -10393,7 +10415,6 @@ function _renderSettledAnchorSceneForMessage(message, segment, rawIdx){
|
||||
if(typeof isCompactWorklogMode==='function'&&!isCompactWorklogMode()) return false;
|
||||
const blocks=_assistantTurnBlocks(segment.closest('.assistant-turn'));
|
||||
if(!blocks) return false;
|
||||
const keepSettledWorklogOpen=_shouldKeepSettledWorklogOpenForPinnedFollow();
|
||||
const scene=message._anchor_activity_scene;
|
||||
const rows=_anchorSceneRowsForRendering(scene,{settled:true});
|
||||
if(!rows.length) return false;
|
||||
@@ -10407,6 +10428,7 @@ function _renderSettledAnchorSceneForMessage(message, segment, rawIdx){
|
||||
});
|
||||
blocks.querySelectorAll('.tool-worklog-group:not([data-anchor-scene-owner="1"]),.tool-call-group:not([data-anchor-scene-owner="1"]),.agent-activity-thinking:not([data-anchor-scene-row="1"]),.wl-reason').forEach(el=>el.remove());
|
||||
const streamId=String(message._anchor_stream_id||scene.stream_id||scene.identity&&scene.identity.stream_id||'');
|
||||
const keepSettledWorklogOpen=_shouldKeepSettledWorklogOpenForPinnedFollow(streamId);
|
||||
const activityKey=`anchor-scene:${rawIdx}`;
|
||||
if(streamId&&!_readActivityDisclosureState(activityKey)){
|
||||
_copyActivityDisclosureState(`live:${streamId}`, activityKey);
|
||||
|
||||
@@ -1,8 +1,23 @@
|
||||
"""Regression locks for #4970 round 6: stream-end worklog collapse shrink jump."""
|
||||
"""Regression locks for #4970 round 6: stream-end worklog collapse shrink jump.
|
||||
|
||||
Round 7 (scoping fix): the keep-open exception must apply to ONLY the turn that
|
||||
just settled, gated on a one-shot stream-id token, NOT to every historical
|
||||
settled worklog on every pinned re-render. These tests are BEHAVIORAL: they
|
||||
extract the real `_shouldKeepSettledWorklogOpenForPinnedFollow` helper plus its
|
||||
arm/disarm token API from static/ui.js and execute them in Node, then drive two
|
||||
settled turns while pinned and assert the second (historical) turn collapses.
|
||||
"""
|
||||
import json
|
||||
import shutil
|
||||
import subprocess
|
||||
import textwrap
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
ROOT = Path(__file__).resolve().parent.parent
|
||||
UI_JS = (ROOT / "static" / "ui.js").read_text(encoding="utf-8")
|
||||
MESSAGES_JS = (ROOT / "static" / "messages.js").read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def _function_body(src: str, name: str) -> str:
|
||||
@@ -21,34 +36,60 @@ def _function_body(src: str, name: str) -> str:
|
||||
raise AssertionError(f"function {name} body not found")
|
||||
|
||||
|
||||
def test_pinned_follow_keeps_settled_worklog_open_to_avoid_stream_done_shrink():
|
||||
"""A pinned reader must not see a huge upward clamp when live worklog settles.
|
||||
def _extract(name: str) -> str:
|
||||
"""Return the full `function name(...){...}` text from ui.js."""
|
||||
marker = f"function {name}"
|
||||
start = UI_JS.index(marker)
|
||||
body = _function_body(UI_JS, name)
|
||||
sig = UI_JS[start : UI_JS.index("{", start)]
|
||||
return f"{sig}{{{body}}}"
|
||||
|
||||
The live worklog can be hundreds of px tall. If the settled worklog is forced
|
||||
collapsed at STREAM_DONE, scrollHeight shrinks and the browser clamps
|
||||
scrollTop, which looks like a large backward jump even though pinned state is
|
||||
correct. Pinned followers keep the settled worklog open for this turn; users
|
||||
who scrolled away still get compact/collapsed settled worklogs.
|
||||
"""
|
||||
assert "function _shouldKeepSettledWorklogOpenForPinnedFollow" in UI_JS
|
||||
|
||||
def test_helper_and_token_threaded_through_render():
|
||||
# Structural: the helper takes a streamId and gates on the one-shot token,
|
||||
# and the call site threads the message's stream id (not a no-arg call).
|
||||
helper = _function_body(UI_JS, "_shouldKeepSettledWorklogOpenForPinnedFollow")
|
||||
assert "_scrollPinned" in helper
|
||||
assert "!_messageUserUnpinned" in helper
|
||||
assert "bottom distance can transiently exceed" in helper
|
||||
assert "avoiding the visible STREAM_DONE jump takes precedence" in helper
|
||||
|
||||
assert "_keepSettledWorklogOpenForStreamId" in helper
|
||||
assert "_scrollPinned" in helper and "!_messageUserUnpinned" in helper
|
||||
render_fn = _function_body(UI_JS, "_renderSettledAnchorSceneForMessage")
|
||||
assert "const keepSettledWorklogOpen=_shouldKeepSettledWorklogOpenForPinnedFollow();" in render_fn
|
||||
assert "_shouldKeepSettledWorklogOpenForPinnedFollow(streamId)" in render_fn
|
||||
assert "collapsed:!keepSettledWorklogOpen" in render_fn
|
||||
|
||||
group_fn = _function_body(UI_JS, "_anchorSceneWorklogGroup")
|
||||
assert "opts&&opts.collapsed!==undefined" in group_fn
|
||||
assert "collapsed:(opts&&opts.collapsed!==undefined)?opts.collapsed:!live" in group_fn
|
||||
# The STREAM_DONE handler arms one-shot then disarms around the render.
|
||||
assert "_armKeepSettledWorklogOpen(_settledStreamId)" in MESSAGES_JS
|
||||
assert "_disarmKeepSettledWorklogOpen()" in MESSAGES_JS
|
||||
|
||||
|
||||
def test_unpinned_reader_still_gets_compact_settled_worklog():
|
||||
helper = _function_body(UI_JS, "_shouldKeepSettledWorklogOpenForPinnedFollow")
|
||||
assert "!_messageUserUnpinned" in helper, (
|
||||
"The keep-open exception must be limited to pinned followers; unpinned "
|
||||
"readers should not get their viewport or settled worklog state changed."
|
||||
)
|
||||
@pytest.mark.skipif(shutil.which("node") is None, reason="node required for behavioral test")
|
||||
def test_only_just_settled_turn_stays_open_pinned_history_collapses():
|
||||
"""Drive two settled turns while pinned; only the just-settled one stays open."""
|
||||
helper = _extract("_shouldKeepSettledWorklogOpenForPinnedFollow")
|
||||
arm = _extract("_armKeepSettledWorklogOpen")
|
||||
disarm = _extract("_disarmKeepSettledWorklogOpen")
|
||||
harness = textwrap.dedent(f"""
|
||||
let _keepSettledWorklogOpenForStreamId=null;
|
||||
let _scrollPinned=true, _messageUserUnpinned=false; // pinned follower
|
||||
{helper}
|
||||
{arm}
|
||||
{disarm}
|
||||
const out={{}};
|
||||
// Turn A just settled: arm A, render A (open), render historical B (collapsed), disarm.
|
||||
_armKeepSettledWorklogOpen('streamA');
|
||||
out.A_open = _shouldKeepSettledWorklogOpenForPinnedFollow('streamA'); // expect true
|
||||
out.B_history = _shouldKeepSettledWorklogOpenForPinnedFollow('streamB'); // expect false
|
||||
_disarmKeepSettledWorklogOpen();
|
||||
// After disarm, even A collapses on a later pinned re-render.
|
||||
out.A_after_disarm = _shouldKeepSettledWorklogOpenForPinnedFollow('streamA'); // false
|
||||
// Unpinned reader never keeps open even for the armed turn.
|
||||
_armKeepSettledWorklogOpen('streamA'); _messageUserUnpinned=true; _scrollPinned=false;
|
||||
out.unpinned = _shouldKeepSettledWorklogOpenForPinnedFollow('streamA'); // false
|
||||
console.log(JSON.stringify(out));
|
||||
""")
|
||||
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["A_open"] is True, "just-settled turn must keep worklog open for pinned follower"
|
||||
assert out["B_history"] is False, "historical settled worklog must stay collapsed while pinned"
|
||||
assert out["A_after_disarm"] is False, "exception must be one-shot, cleared after the render"
|
||||
assert out["unpinned"] is False, "unpinned reader always gets compact settled worklog"
|
||||
Reference in new issue
Block a user