From d9622bf9b0bf966e6dcd5863badfceb3a3a415bf Mon Sep 17 00:00:00 2001 From: allenliang2022 Date: Tue, 30 Jun 2026 11:40:30 +0800 Subject: [PATCH] =?UTF-8?q?fix(chat):=20keep=20just-settled=20worklog=20op?= =?UTF-8?q?en=20for=20unpinned=20readers=20too=20(mobile=20=E5=BE=80?= =?UTF-8?q?=E5=9B=9E=E5=A4=A7=E8=B7=B3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- static/ui.js | 43 ++++++----- ...issue4970_stream_done_shrink_regression.py | 71 +++++++++++++------ 2 files changed, 77 insertions(+), 37 deletions(-) diff --git a/static/ui.js b/static/ui.js index 238383a2..bf6b7189 100644 --- a/static/ui.js +++ b/static/ui.js @@ -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); diff --git a/tests/test_issue4970_stream_done_shrink_regression.py b/tests/test_issue4970_stream_done_shrink_regression.py index e76ef702..a8dfb6b5 100644 --- a/tests/test_issue4970_stream_done_shrink_regression.py +++ b/tests/test_issue4970_stream_done_shrink_regression.py @@ -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"