From 545681ce4c7a61b7a5fbbfff334c61443c4cfb6e Mon Sep 17 00:00:00 2001 From: allenliang2022 Date: Mon, 6 Jul 2026 13:01:40 +0800 Subject: [PATCH] =?UTF-8?q?refactor(webui):=20address=20review=20=E2=80=94?= =?UTF-8?q?=20drop=20dead=20field,=20rename,=20add=20fallback=20intent=20t?= =?UTF-8?q?est=20(#5637)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile review follow-ups (no behavior change): - Remove the captured-but-never-read scrollTopAtCapture field from _captureMessageViewportAnchor; only scrollHeightAtCapture is consulted. - Rename _grewAbove -> _grewSinceCapture in the realign guard (the check is overall scrollHeight growth since capture, not specifically above-viewport), matching the fallback guard's _grewSinceSnap. - Add a mutation-checked fallback active-intent test: content grew but the reader has recent real input intent, so the absolute snapshot.top restore is kept. --- static/ui.js | 13 ++++++------- tests/test_issue5637_stale_anchor_guard.py | 14 ++++++++++++++ 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/static/ui.js b/static/ui.js index cb65ae87..63d592ee 100644 --- a/static/ui.js +++ b/static/ui.js @@ -953,12 +953,11 @@ function _captureMessageViewportAnchor(){ key:row&&row.dataset?String(row.dataset.messageAnchorKey||''):'', topOffset:rect.top-containerRect.top, topPadBefore, - // Snapshot the scroll geometry at capture so a later realign can detect that - // content grew ABOVE the viewport between capture and restore — the streaming - // case where the anchor's topOffset is stale and realigning to it would yank a - // still reader backward (issue #5637). + // Snapshot the scroll height at capture so a later realign can detect that + // content grew between capture and restore — the streaming case where the + // anchor's topOffset is stale and realigning to it would yank a still reader + // backward (issue #5637). scrollHeightAtCapture:container.scrollHeight, - scrollTopAtCapture:container.scrollTop, }; } } @@ -1050,10 +1049,10 @@ function _restoreMessageViewportAnchor(anchor, rawIdxDelta){ const _realignDelta=(rect.top-containerRect.top)-targetTop; const _shAtCap=Number(anchor.scrollHeightAtCapture); if(Number.isFinite(_shAtCap)){ - const _grewAbove=(container.scrollHeight-_shAtCap)>4; + const _grewSinceCapture=(container.scrollHeight-_shAtCap)>4; const _activeIntent=(typeof _recentMessageScrollIntent==='function' && _recentMessageScrollIntent()) || (typeof _recentMessageTouchScrollIntent==='function' && _recentMessageTouchScrollIntent()); - if(_grewAbove&&!_activeIntent&&Math.abs(_realignDelta)>8){ + if(_grewSinceCapture&&!_activeIntent&&Math.abs(_realignDelta)>8){ return false; } } diff --git a/tests/test_issue5637_stale_anchor_guard.py b/tests/test_issue5637_stale_anchor_guard.py index 403cd661..93cada57 100644 --- a/tests/test_issue5637_stale_anchor_guard.py +++ b/tests/test_issue5637_stale_anchor_guard.py @@ -217,3 +217,17 @@ def test_fallback_backward_compatible_without_capture_scrollheight(): active_intent=False, ))) assert m["writes"] == [89577] + + +def test_fallback_allows_snapshot_top_with_active_intent(): + """Content grew since snapshot, but the reader has recent real input intent -> the + fallback keeps its legitimate absolute restore (an actively-scrolling reader owns the + snapshot). Mirrors the realign-guard active-intent case. + Mutation: drop the `!_activeIntent` term from the fallback guard and this fails + (the write would be wrongly refused).""" + m = json.loads(_run_node(_fallback_harness( + snapshot_scroll_height=90000, cur_scroll_height=90453, snapshot_top=89577, + active_intent=True, + ))) + assert m["writes"] == [89577] +