fix(chat): keep just-settled worklog open for unpinned readers too (mobile 往回大跳)
The STREAM_DONE keep-open exception (#4970 round 6/7) was scoped to PINNED followers only, on the assumption that unpinned readers 'preserve their viewport normally'. That holds on desktop (overflow-anchor:none + JS snapshot restore) but NOT on mobile. Repro (isolated debug instance + CDP touch emulation, faithful to a real phone): a reader who scrolls UP to read inside the just-settled turn is UNPINNED. At STREAM_DONE the live worklog/thinking cards collapse into a compact summary, shrinking the transcript by hundreds of px ABOVE their viewport. On mobile the CSS resting value is overflow-anchor:auto, but _fixMobileScrollJank() flips an inline overflow-anchor:none over the settle render — so native scroll-anchoring is suppressed during exactly the frame the unpinned reader needs it to absorb the above-viewport shrink. The content then leaps to the top of the latest turn (the '往回大跳' report). Single-variable measurement: collapse-with-auto shifts the reader 0px; collapse-with-none shifts -248px. Fix: keep the just-settled worklog open for BOTH pin states (still one-shot scoped to the turn that just settled, so historical worklogs still collapse compact). This removes the shrink entirely, fixing the jump for every device/anchor-mode instead of fighting the anchor engine. The #4856 guard and the mobile overflow-anchor:auto CSS are untouched, so Boev's Android DOM-wipe fix and akrhin's #MOBILESCROLL fix both stay intact. - Rename _shouldKeepSettledWorklogOpenForPinnedFollow -> _shouldKeepSettledWorklogOpenForStreamSettle (drops the _scrollPinned / _messageUserUnpinned gate; keeps the one-shot stream-id token). - Update the #4970 behavioral regression test to assert keep-open is TRUE for the armed turn under BOTH pin states, FALSE for historical turns and after disarm.
This commit is contained in:
1 parent
1a523926d5
commit
d9622bf9b0
2 files changed
+77
-37
No files matched your search
+26
-17
@@ -10501,24 +10501,33 @@ function _renderSettledAnchorSceneTransparentForMessage(message, segment, rawIdx
|
||||
// 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 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.
|
||||
function _shouldKeepSettledWorklogOpenForStreamSettle(streamId){
|
||||
// Round 6 scroll-jump guard: collapsing the JUST-settled live worklog into a
|
||||
// compact summary at STREAM_DONE shrinks the transcript by hundreds of px. The
|
||||
// resulting backward jump hits readers in TWO positions, so keep that one
|
||||
// worklog open for BOTH on the settle render — the live->settled DOM swap is
|
||||
// then height-stable and there is no shrink for any scroll path to mishandle:
|
||||
//
|
||||
// 1. PINNED follower at the live tail: the shrink lowers scrollHeight, the
|
||||
// browser clamps scrollTop to the new max, and the viewport snaps upward
|
||||
// even though pin state is correct.
|
||||
// 2. UNPINNED reader who scrolled UP to read inside the just-settled turn
|
||||
// (the mobile "往回大跳" report, #MOBILESCROLL follow-up): the worklog sits
|
||||
// ABOVE their viewport, so collapsing it pulls their content up to the top
|
||||
// of the turn. On desktop overflow-anchor:none + the JS snapshot restore
|
||||
// keep them put, but on mobile the CSS resting value is overflow-anchor:
|
||||
// auto AND _fixMobileScrollJank() flips an inline overflow-anchor:none over
|
||||
// the settle render — which is exactly the wrong state: native anchoring is
|
||||
// suppressed during the one frame the unpinned reader needs it to absorb
|
||||
// the above-viewport shrink, so the content leaps to the turn's top. Keeping
|
||||
// the worklog open removes the shrink entirely, which fixes it for every
|
||||
// device/anchor-mode combination instead of fighting the anchor engine.
|
||||
//
|
||||
// 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);
|
||||
// historical settled worklog on every re-render (which would defeat the
|
||||
// compact-worklog default for past turns).
|
||||
return !!(streamId&&_keepSettledWorklogOpenForStreamId===streamId);
|
||||
}
|
||||
// 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,
|
||||
@@ -10555,7 +10564,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 keepSettledWorklogOpen=_shouldKeepSettledWorklogOpenForStreamSettle(streamId);
|
||||
const activityKey=`anchor-scene:${rawIdx}`;
|
||||
if(streamId&&!_readActivityDisclosureState(activityKey)){
|
||||
_copyActivityDisclosureState(`live:${streamId}`, activityKey);
|
||||
|
||||
@@ -2,10 +2,26 @@
|
||||
|
||||
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.
|
||||
settled worklog on every re-render.
|
||||
|
||||
Round 8 (mobile unpinned fix, #MOBILESCROLL follow-up): the keep-open exception
|
||||
must also cover the UNPINNED reader who scrolled up to read inside the
|
||||
just-settled turn. The earlier round scoped keep-open to pinned followers only,
|
||||
on the assumption that unpinned readers "preserve their viewport normally". That
|
||||
holds on desktop (overflow-anchor:none + JS snapshot restore) but NOT on mobile:
|
||||
the CSS resting value is overflow-anchor:auto and _fixMobileScrollJank() flips an
|
||||
inline overflow-anchor:none over the settle render, so native anchoring is
|
||||
suppressed during the one frame the unpinned reader needs it to absorb the
|
||||
above-viewport worklog shrink — the content leaps to the top of the turn (the
|
||||
"往回大跳" report). Keeping the just-settled worklog open removes the shrink for
|
||||
every device/anchor-mode, so the helper now returns true for the armed turn
|
||||
regardless of pin state.
|
||||
|
||||
These tests are BEHAVIORAL: they extract the real
|
||||
`_shouldKeepSettledWorklogOpenForStreamSettle` helper plus its arm/disarm token
|
||||
API from static/ui.js and execute them in Node, then drive two settled turns and
|
||||
assert the second (historical) turn collapses while the just-settled one stays
|
||||
open for both pin states.
|
||||
"""
|
||||
import json
|
||||
import shutil
|
||||
@@ -48,11 +64,17 @@ def _extract(name: str) -> str:
|
||||
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")
|
||||
helper = _function_body(UI_JS, "_shouldKeepSettledWorklogOpenForStreamSettle")
|
||||
assert "_keepSettledWorklogOpenForStreamId" in helper
|
||||
assert "_scrollPinned" in helper and "!_messageUserUnpinned" in helper
|
||||
# Round 8: the helper must NOT re-narrow to pinned-only — keep-open now covers
|
||||
# the unpinned reader too, so the pin-state gate is gone from the return.
|
||||
assert "_scrollPinned" not in helper, (
|
||||
"keep-open must apply to the just-settled turn regardless of pin state "
|
||||
"(unpinned mobile readers also get the shrink jump); do not gate on "
|
||||
"_scrollPinned/_messageUserUnpinned."
|
||||
)
|
||||
render_fn = _function_body(UI_JS, "_renderSettledAnchorSceneForMessage")
|
||||
assert "_shouldKeepSettledWorklogOpenForPinnedFollow(streamId)" in render_fn
|
||||
assert "_shouldKeepSettledWorklogOpenForStreamSettle(streamId)" in render_fn
|
||||
assert "collapsed:!keepSettledWorklogOpen" in render_fn
|
||||
group_fn = _function_body(UI_JS, "_anchorSceneWorklogGroup")
|
||||
assert "collapsed:(opts&&opts.collapsed!==undefined)?opts.collapsed:!live" in group_fn
|
||||
@@ -62,9 +84,9 @@ def test_helper_and_token_threaded_through_render():
|
||||
|
||||
|
||||
@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")
|
||||
def test_just_settled_turn_stays_open_for_both_pin_states_history_collapses():
|
||||
"""Only the just-settled turn stays open; it does so for pinned AND unpinned."""
|
||||
helper = _extract("_shouldKeepSettledWorklogOpenForStreamSettle")
|
||||
arm = _extract("_armKeepSettledWorklogOpen")
|
||||
disarm = _extract("_disarmKeepSettledWorklogOpen")
|
||||
harness = textwrap.dedent(f"""
|
||||
@@ -76,20 +98,29 @@ def test_only_just_settled_turn_stays_open_pinned_history_collapses():
|
||||
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
|
||||
out.A_open_pinned = _shouldKeepSettledWorklogOpenForStreamSettle('streamA'); // expect true
|
||||
out.B_history = _shouldKeepSettledWorklogOpenForStreamSettle('streamB'); // expect false
|
||||
// Round 8: an UNPINNED reader of the just-settled turn must ALSO keep it open
|
||||
// (the mobile above-viewport shrink jump). This is the behavior change.
|
||||
_messageUserUnpinned=true; _scrollPinned=false;
|
||||
out.A_open_unpinned = _shouldKeepSettledWorklogOpenForStreamSettle('streamA'); // expect true
|
||||
out.B_history_unpinned = _shouldKeepSettledWorklogOpenForStreamSettle('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
|
||||
// After disarm, even A collapses on a later re-render (one-shot scope intact).
|
||||
out.A_after_disarm = _shouldKeepSettledWorklogOpenForStreamSettle('streamA'); // false
|
||||
// No token at all → never keep open.
|
||||
out.no_token = _shouldKeepSettledWorklogOpenForStreamSettle(''); // 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_open_pinned"] is True, "just-settled turn must keep worklog open for pinned follower"
|
||||
assert out["A_open_unpinned"] is True, (
|
||||
"just-settled turn must ALSO keep worklog open for an unpinned reader — "
|
||||
"the above-viewport collapse shrink janks them to the turn top on mobile"
|
||||
)
|
||||
assert out["B_history"] is False, "historical settled worklog must stay collapsed (pinned)"
|
||||
assert out["B_history_unpinned"] is False, "historical settled worklog must stay collapsed (unpinned)"
|
||||
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"
|
||||
assert out["no_token"] is False, "no armed stream id → never keep open"
|
||||
Reference in new issue
Block a user