From bd3f01d0f3690b8771c6a3816aff158c60226c1d Mon Sep 17 00:00:00 2001 From: allenliang2022 Date: Mon, 6 Jul 2026 22:12:06 +0800 Subject: [PATCH 1/3] fix(webui): stop pinned tail-follower mid-stream scroll jitter A pinned/tail-following reader gets a fast ~1-row back-and-forth bounce on every mid-stream re-render (tool completion, activity-scene refresh, clarify echo). renderMessages() rebuilds the transcript (inner.innerHTML='' then re-append). For a pinned reader the post-render path writes scrollTop=scrollHeight while still INSIDE the render sync stack, where the browser reports a transient scrollHeight a few px short of the settled value (layout is batched). scrollToBottom lands scrollTop a little high (short of the true tail); that intermediate is painted this frame and the settle rAF corrects it the next frame -> a ~1-row bounce (measured ~82px, confirmed with a per-frame scrollTop flight recorder and an isolated CSSOM A/B). Fix: capture the pre-wipe near-tail state (geometry, since the wipe's clamp scroll event can perturb the closure pin flags) and, at the end of renderMessages, schedule _reanchorPinnedTailAfterRender in a microtask. The microtask runs after the sync stack flushes layout to the settled height but before paint, so writing the settled max lands the tail exactly and the short intermediate never reaches the screen. A synchronous call reads the same transient short height and is a no-op (verified: sync leaves the 82px bounce; microtask drives it to 0). Only re-anchors a pre-wipe tail-follower left short of the settled max, so an unpinned reader parked in history is never moved (verified 0px displacement); arms the existing _programmaticScroll latch; idempotent. Tests: test_pinned_tail_midstream_jitter.py runs the real extracted helper in a Node harness (re-anchor short tail-follower / never move a history reader / idempotent no-op / mutation check / source locks on the queueMicrotask call site and settled-max write). Neighboring scroll suites remain green. --- static/ui.js | 54 +++++ tests/test_pinned_tail_midstream_jitter.py | 221 +++++++++++++++++++++ 2 files changed, 275 insertions(+) create mode 100644 tests/test_pinned_tail_midstream_jitter.py diff --git a/static/ui.js b/static/ui.js index 352caf47..75fee134 100644 --- a/static/ui.js +++ b/static/ui.js @@ -13402,6 +13402,36 @@ function _assistantTurnAnchorSettledFinalAnswer(message, content, context){ return null; } } +// Re-anchor a pinned/tail-following reader to the settled bottom after a full +// renderMessages() rebuild, eliminating the one-frame mid-stream jitter. MUST be scheduled +// in a MICROTASK from the end of renderMessages (see the queueMicrotask call site), NOT run +// synchronously. Why: the mid-stream re-render bug is that renderMessages wipes #msgInner +// then rebuilds, and the pinned tail-follow path (scrollIfPinned → scrollToBottom) writes +// scrollTop while still INSIDE the render sync stack, where the browser reports a TRANSIENT +// scrollHeight a few px short of the settled value (layout is batched — every geometry read +// inside the sync stack returns the mid-settle height). So scrollToBottom lands scrollTop a +// little HIGH (short of the true tail); that intermediate is painted this frame and the +// settle rAF corrects it the next frame → a fast ~1-row back-and-forth bounce (~82px). A +// microtask runs AFTER the sync stack unwinds (layout has flushed, so scrollHeight/clientHeight +// are the settled values) but BEFORE the browser paints — so writing the now-correct settled +// max here lands the tail exactly and the short intermediate never reaches the screen. Only +// fires for a pre-wipe tail-follower left short of the settled max, so an unpinned reader +// parked in history is never moved (orthogonal to the unpinned jump-back class). The +// _programmaticScroll latch (armed at the wipe) keeps the scroll listener from misreading +// this write as a manual unpin. Idempotent: a no-op once scrollTop already equals the max. +function _reanchorPinnedTailAfterRender(wasNearTail){ + if(!wasNearTail) return; + const el=$('messages'); + if(!el) return; + const settledMax=Math.max(0, el.scrollHeight-el.clientHeight); + if(el.scrollTop < settledMax-1){ + _programmaticScroll=true;_programmaticScrollSetAt=performance.now(); + el.scrollTop=settledMax; + _lastScrollTop=el.scrollTop;_lastMessageClientHeight=el.clientHeight; + _nearBottomCount=2; + _scrollPinned=true; + } +} function _scrollAfterMessageRender(preserveScroll, scrollSnapshot){ // Terminal stream renders can happen after S.activeStreamId is cleared. // In that case, preserveScroll asks the normal pin-state helper to decide: @@ -13595,6 +13625,20 @@ function renderMessages(options){ // Mobile scroll-jank fix: temporarily disable overflow-anchor so Chromium // cannot re-anchor to the topmost row during the DOM wipe-and-rebuild gap. if(window._fixMobileScrollJank) window._fixMobileScrollJank(); + // Capture whether the reader was at/near the tail BEFORE the wipe. A tail-follower + // hit by a mid-stream re-render gets a one-frame jitter: the wipe+rebuild lands the + // sync scrollTop write against a transient layout whose above-viewport height is a + // few px short of the settled value, so the browser clamps scrollTop a little high; + // the settle rAF corrects it the next frame, producing a fast ~1-row back-and-forth + // bounce. We remember the pre-wipe near-tail state here (geometry, not closure pin + // flags — the wipe's clamp scroll event can transiently perturb those) so the tail + // of renderMessages can re-anchor to the settled bottom before the intermediate is + // painted. See _reanchorPinnedTailAfterRender + its queueMicrotask call site. + const _preWipeNearTail=(()=>{ + const _m=$('messages'); + if(!_m) return false; + return (_m.scrollHeight-_m.scrollTop-_m.clientHeight)<=8; + })(); // The DOM wipe can briefly collapse #msgInner to zero height, causing the // browser to clamp #messages.scrollTop to 0 and emit a scroll event. That // event is a render artifact, not user intent; if the scroll listener sees it @@ -14929,6 +14973,16 @@ function renderMessages(options){ } } _updateMessageVirtualMeasurements(renderVisWithIdx, renderVisibleIdxs, virtualWindow); + // Kill the pinned/tail-follower mid-stream jitter. Schedule the re-anchor in a MICROTASK, + // not synchronously: inside this render sync stack the browser still reports a transient + // scrollHeight (layout is batched), so a synchronous re-anchor would read the SAME short + // value scrollToBottom already clamped against and be a no-op. The microtask runs after the + // stack unwinds (scrollHeight has flushed to the settled value) but before the browser + // paints this frame, so writing the settled max lands the tail exactly and the ~1-row high + // intermediate never reaches the screen. Only re-anchors a pre-wipe tail-follower left short + // of the settled max — an unpinned reader parked in history is never moved (orthogonal to + // the unpinned jump-back class). See _reanchorPinnedTailAfterRender for the full rationale. + queueMicrotask(()=>_reanchorPinnedTailAfterRender(_preWipeNearTail)); _recycleStash.clear(); if(typeof _deferClearProgrammaticScroll==='function') _deferClearProgrammaticScroll(160); } diff --git a/tests/test_pinned_tail_midstream_jitter.py b/tests/test_pinned_tail_midstream_jitter.py new file mode 100644 index 00000000..1a6615ef --- /dev/null +++ b/tests/test_pinned_tail_midstream_jitter.py @@ -0,0 +1,221 @@ +"""Regression tests for the pinned/tail-follower mid-stream scroll JITTER (a fast +~1-row back-and-forth bounce), distinct from the unpinned jump-back class. + +Root cause (reproduced on desktop Chromium with a per-frame scrollTop flight +recorder + an isolated CSSOM A/B): when a mid-stream re-render fires (tool +completion, activity-scene refresh, clarify echo), renderMessages() wipes +#msgInner (`inner.innerHTML=''`) then rebuilds and, inside the SAME synchronous +stack, the pinned/tail-follow path (scrollIfPinned -> scrollToBottom -> +_setMessageScrollToBottom) writes scrollTop against a TRANSIENT layout whose +above-viewport height is a few px short of the settled value. The browser clamps +scrollTop a little HIGH (short of the true max); that intermediate is PAINTED this +frame, and the settle rAF corrects it the next frame -> a visible fast bounce of +~1 row (measured ~82px on a real session). scrollHeight/max are back to their +settled values by the end of the renderMessages sync stack, so re-anchoring to the +settled max THERE lands exactly, before the intermediate is painted. + +Fix: capture the near-tail state BEFORE the wipe (geometry, not closure pin flags, +which the wipe's clamp scroll event can transiently perturb); at the very end of +renderMessages call `_reanchorPinnedTailAfterRender(preWipeNearTail)`, which -- only +for a pre-wipe tail-follower still sitting short of the settled max -- snaps scrollTop +to the pre-computed settled max synchronously (writing the max, NOT scrollHeight, +which would reflow to the same transient short value and re-clamp). An unpinned reader +parked in history is never moved (orthogonal to the unpinned jump-back class). + +Each behavioral test FAILS on the known-buggy version (guard disabled) and PASSES +only on the fixed version. A pure-string structural test locks the call site + the +"write settled max, not scrollHeight" invariant. +""" +import json +import pathlib +import shutil +import subprocess +import tempfile + +import pytest + +ROOT = pathlib.Path(__file__).parent.parent +UI_JS_PATH = ROOT / "static" / "ui.js" +UI_JS = UI_JS_PATH.read_text(encoding="utf-8") +NODE = shutil.which("node") + +pytestmark = pytest.mark.skipif(NODE is None, reason="node not on PATH") + + +def _function_body(src: str, name: str) -> str: + start = src.index(f"function {name}") + brace = src.index("{", start) + depth = 0 + for i in range(brace, len(src)): + if src[i] == "{": + depth += 1 + elif src[i] == "}": + depth -= 1 + if depth == 0: + return src[start : i + 1] + raise AssertionError(f"function {name} body not found") + + +def _run_node(source: str) -> str: + with tempfile.NamedTemporaryFile( + "w", suffix=".cjs", encoding="utf-8", dir=ROOT, delete=False + ) as script: + script.write(source) + script_path = pathlib.Path(script.name) + try: + result = subprocess.run( + [NODE, str(script_path)], + cwd=str(ROOT), + capture_output=True, + text=True, + timeout=30, + ) + finally: + script_path.unlink(missing_ok=True) + if result.returncode != 0: + raise RuntimeError(result.stderr) + return result.stdout.strip() + + +def _harness(helper_body: str, scenario_js: str, mutate=None) -> str: + """Run the REAL extracted _reanchorPinnedTailAfterRender body against a fake + #messages scroller. The scenario sets the pre-wipe near-tail flag + the current + (clamped) scrollTop and settled geometry, then prints the resulting scrollTop and + the pin-state bookkeeping the helper writes. + """ + body = helper_body + if mutate: + old, new = mutate + assert old in body, "mutation target not found in extracted helper" + body = body.replace(old, new) + return ( + r""" +'use strict'; +// ---- fake #messages scroller: settled layout (scrollHeight/clientHeight are the +// POST-render settled values, matching the end of the renderMessages sync stack). ---- +const el = { scrollTop: 0, scrollHeight: 9235, clientHeight: 626 }; +function $(id){ return id === 'messages' ? el : null; } +// ---- module-level state the helper closes over ---- +let _programmaticScroll = false; +let _programmaticScrollSetAt = 0; +let _lastScrollTop = 0; +let _lastMessageClientHeight = 0; +let _nearBottomCount = 0; +let _scrollPinned = false; +const performance = { now: () => 0 }; +""" + + body + + "\n" + + scenario_js + ) + + +HELPER = _function_body(UI_JS, "_reanchorPinnedTailAfterRender") + + +def test_pinned_tailfollower_clamped_short_is_reanchored_to_settled_max(): + """The exact jitter event: a pre-wipe tail-follower whose scrollTop the browser + left clamped SHORT of the settled max (8527 vs settled max 9235-626=8609). The + fixed helper must snap scrollTop to the settled max (8609) synchronously, so the + short intermediate is never painted. On the buggy version scrollTop stays 8527. + """ + scenario = r""" +el.scrollTop = 8527; // browser clamped a bit high during the transient-layout write +_reanchorPinnedTailAfterRender(true); // reader WAS at/near tail before the wipe +console.log(JSON.stringify({ top: el.scrollTop, prog: _programmaticScroll, pinned: _scrollPinned })); +""" + m = json.loads(_run_node(_harness(HELPER, scenario))) + assert m["top"] == 8609, ( + "a pre-wipe tail-follower left clamped short (8527) must be re-anchored to the " + f"settled max (8609); got {m['top']}. On the buggy version it stays at 8527, " + "which is the painted intermediate that produces the ~1-row bounce." + ) + assert m["prog"] is True, ( + "the re-anchor write must arm _programmaticScroll so the scroll listener does " + "not misread it as a manual unpin" + ) + assert m["pinned"] is True, "re-anchoring a tail-follower must keep it pinned" + + +def test_unpinned_reader_not_near_tail_is_never_moved(): + """An unpinned reader parked in history (wasNearTail=false) must NOT be moved -- + this is the orthogonality guarantee vs the unpinned jump-back class. Even though scrollTop is far below + the settled max, the helper must leave it exactly where it is. + """ + scenario = r""" +el.scrollTop = 3232; // reader parked mid-history +_reanchorPinnedTailAfterRender(false); // was NOT near tail before the wipe +console.log(JSON.stringify({ top: el.scrollTop, prog: _programmaticScroll })); +""" + m = json.loads(_run_node(_harness(HELPER, scenario))) + assert m["top"] == 3232, ( + "a reader who was not a tail-follower before the wipe must never be snapped " + f"to the bottom; got {m['top']}" + ) + assert m["prog"] is False, "no scroll write should happen for a non-tail reader" + + +def test_already_at_settled_bottom_is_idempotent_noop(): + """When scrollTop already equals the settled max, the helper must be a no-op (no + redundant write / no _programmaticScroll churn) -- guards against re-arming the + latch every render on a steady tail-follower. + """ + scenario = r""" +el.scrollTop = 8609; // already at settled max (9235 - 626) +_reanchorPinnedTailAfterRender(true); +console.log(JSON.stringify({ top: el.scrollTop, prog: _programmaticScroll })); +""" + m = json.loads(_run_node(_harness(HELPER, scenario))) + assert m["top"] == 8609 + assert m["prog"] is False, ( + "already at the settled max: the helper must not re-write scrollTop or arm the " + "programmatic latch (idempotent no-op)" + ) + + +def test_mutation_disabling_the_guard_fails_the_reanchor(): + """MUTATION CHECK: reverting the fix (make the short-clamp branch dead) must break + the re-anchor. Proves test_pinned_...reanchored actually exercises the fix and is + not vacuously green. + """ + scenario = r""" +el.scrollTop = 8527; +_reanchorPinnedTailAfterRender(true); +console.log(JSON.stringify({ top: el.scrollTop })); +""" + # Disable the guard body: the branch that performs the re-anchor never runs. + mutated = _harness( + HELPER, scenario, mutate=("if(el.scrollTop < settledMax-1){", "if(false){") + ) + m = json.loads(_run_node(mutated)) + assert m["top"] == 8527, ( + "with the guard disabled the reader must stay at the clamped-short 8527 -- if " + "this still reported 8609 the behavioral test would be vacuous" + ) + + +def test_source_locks_call_site_and_settled_max_write(): + """Structural locks so a future refactor can't silently drop the fix or regress it + to (a) a SYNCHRONOUS call (which reads the transient short height inside the render + stack and no-ops) or (b) writing scrollHeight (which reflows to the transient short + value and re-clamps). + """ + # The pre-wipe near-tail capture exists and is taken before the wipe. + assert "const _preWipeNearTail=(()=>{" in UI_JS + # renderMessages schedules the re-anchor in a MICROTASK (NOT synchronously): the + # microtask runs after the render sync stack flushes layout to the settled height but + # before paint. A bare synchronous call reads the mid-settle height and is a no-op. + assert "queueMicrotask(()=>_reanchorPinnedTailAfterRender(_preWipeNearTail));" in UI_JS + # Guard against regressing to a synchronous call. + assert "\n _reanchorPinnedTailAfterRender(_preWipeNearTail);" not in UI_JS, ( + "the re-anchor must be scheduled via queueMicrotask, not called synchronously " + "at the end of renderMessages (a sync call reads the transient short height)" + ) + # The helper writes the settled max (read fresh in the microtask), NOT scrollHeight. + helper = _function_body(UI_JS, "_reanchorPinnedTailAfterRender") + assert "const settledMax=Math.max(0, el.scrollHeight-el.clientHeight);" in helper + assert "el.scrollTop=settledMax;" in helper + assert "el.scrollTop=el.scrollHeight" not in helper, ( + "must write the pre-computed settledMax, not scrollHeight (re-reading " + "scrollHeight reflows to the transient short value and re-clamps)" + ) From 496f3693043f22347073cc4517e4a30a883bff28 Mon Sep 17 00:00:00 2001 From: allenliang2022 Date: Mon, 6 Jul 2026 22:20:42 +0800 Subject: [PATCH 2/3] fix(webui): typeof-guard the mid-stream re-anchor microtask call The renderMessages() re-anchor call `queueMicrotask(()=>_reanchorPinnedTailAfterRender(...))` threw `ReferenceError: _reanchorPinnedTailAfterRender is not defined` in the standalone renderMessages() node harness (test_anchor_fallback_ownership), which evals the extracted renderMessages body with only a minimal set of stubbed helpers. Guard the call with `typeof` checks on both `queueMicrotask` and `_reanchorPinnedTailAfterRender`, mirroring the existing `if(typeof _deferClearProgrammaticScroll==='function')` guard a couple of lines below, so a harness that evals renderMessages without defining these helpers does not throw. In the browser both are always defined, so the fix path is unchanged. Add a source-lock asserting the guard so a future refactor can't drop it and re-break the harness. --- static/ui.js | 6 +++++- tests/test_pinned_tail_midstream_jitter.py | 11 +++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/static/ui.js b/static/ui.js index 75fee134..d99fc4c7 100644 --- a/static/ui.js +++ b/static/ui.js @@ -14982,7 +14982,11 @@ function renderMessages(options){ // intermediate never reaches the screen. Only re-anchors a pre-wipe tail-follower left short // of the settled max — an unpinned reader parked in history is never moved (orthogonal to // the unpinned jump-back class). See _reanchorPinnedTailAfterRender for the full rationale. - queueMicrotask(()=>_reanchorPinnedTailAfterRender(_preWipeNearTail)); + // (typeof guards mirror the _deferClearProgrammaticScroll call below so standalone + // renderMessages() test harnesses that don't define these helpers still run.) + if(typeof queueMicrotask==='function' && typeof _reanchorPinnedTailAfterRender==='function'){ + queueMicrotask(()=>_reanchorPinnedTailAfterRender(_preWipeNearTail)); + } _recycleStash.clear(); if(typeof _deferClearProgrammaticScroll==='function') _deferClearProgrammaticScroll(160); } diff --git a/tests/test_pinned_tail_midstream_jitter.py b/tests/test_pinned_tail_midstream_jitter.py index 1a6615ef..99904a84 100644 --- a/tests/test_pinned_tail_midstream_jitter.py +++ b/tests/test_pinned_tail_midstream_jitter.py @@ -206,6 +206,17 @@ def test_source_locks_call_site_and_settled_max_write(): # microtask runs after the render sync stack flushes layout to the settled height but # before paint. A bare synchronous call reads the mid-settle height and is a no-op. assert "queueMicrotask(()=>_reanchorPinnedTailAfterRender(_preWipeNearTail));" in UI_JS + # The call is typeof-guarded so a standalone renderMessages() node harness (which does + # not define queueMicrotask / _reanchorPinnedTailAfterRender) does not throw a + # ReferenceError when it evals the extracted renderMessages body. + assert ( + "if(typeof queueMicrotask==='function' && " + "typeof _reanchorPinnedTailAfterRender==='function'){" in UI_JS + ), ( + "the queueMicrotask re-anchor must be typeof-guarded (mirroring the " + "_deferClearProgrammaticScroll guard) so harnesses that eval renderMessages " + "without these helpers defined don't ReferenceError" + ) # Guard against regressing to a synchronous call. assert "\n _reanchorPinnedTailAfterRender(_preWipeNearTail);" not in UI_JS, ( "the re-anchor must be scheduled via queueMicrotask, not called synchronously " From b08e6d91afd5421ec335c505e441ca4baa1d89a7 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Mon, 6 Jul 2026 17:24:26 +0000 Subject: [PATCH 3/3] docs(changelog): stamp #5685 pinned tail jitter fix --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c9b49141..2ce75a01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,10 @@ ## [Unreleased] +### Fixed + +- **No more mid-stream scroll jitter while following a streaming reply.** When you were pinned to the bottom of a live-streaming answer, every mid-stream re-render (tool completion, activity-scene refresh, clarify echo) caused a brief ~1-row up-and-back bounce, so a long streaming reply visibly shuddered. The tail is now re-anchored to the settled bottom in a microtask after the re-render (after layout flushes, before paint), so the short intermediate never reaches the screen. Only a reader already following the tail is affected — an unpinned reader parked up in history is never moved. Thanks @allenliang2022. (#5685) + ### Documentation - **Documented the reverse-proxy basic-auth caveat for installed PWAs.** README, onboarding, and troubleshooting now explain that proxy basic auth can block the same-origin service-worker/shell update fetches an installed PWA needs (leaving it on a blank screen after an update), and give recovery steps — prefer WebUI's built-in password, or scope proxy auth so `sw.js`/manifest/shell requests complete. Thanks @rodboev. (#5673, #2781)