From 448e28f44292c34ceaa0427572790c11be8aa30e Mon Sep 17 00:00:00 2001 From: Frank Song Date: Tue, 30 Jun 2026 10:28:36 +0800 Subject: [PATCH 1/2] fix(chat): preserve manual live worklog scroll snapshots --- static/ui.js | 22 +++- .../test_issue4295_midstream_scroll_anchor.py | 119 ++++++++++++++++++ 2 files changed, 139 insertions(+), 2 deletions(-) diff --git a/static/ui.js b/static/ui.js index c5064caa..67161b60 100644 --- a/static/ui.js +++ b/static/ui.js @@ -4038,6 +4038,7 @@ const MESSAGE_TOUCH_SCROLL_SUPPRESS_MS=1200; // post-render artifact suppression would swallow it for the whole window. const MESSAGE_WHEEL_INTENT_SUPPRESS_MS=1200; let _lastMessageWheelIntentMs=-Infinity; +let _lastMessageScrollIntentMs=-Infinity; // #4970 review (greptile P1): keyboard scrolling of the message pane (PageUp/Down, // arrows, Space, Home/End) fires a native `scroll` event with NO wheel/touch/ // scrollbar/non-message intent. Without recording it, a keyboard scroll-up inside @@ -4065,6 +4066,12 @@ function _recentMessageTouchScrollIntent(){ function _recentMessageWheelIntent(){ return performance.now()-_lastMessageWheelIntentMs120) _lastMessageScrollIntentMs=performance.now(); + } if(typeof e.deltaY==='number'&&e.deltaY<0) _lastMessageWheelIntentMs=performance.now(); if(e.type==='touchmove'||(typeof e.deltaY==='number'&&e.deltaY< -30)){ _cancelBottomSettle(); @@ -4197,6 +4208,7 @@ function _resetScrollDirectionTracker(){ // into the new chat's first post-render window — the artifact then isn't // suppressed, falls into movedUp, and falsely unpins live-follow. _lastMessageWheelIntentMs=-Infinity; + _lastMessageScrollIntentMs=-Infinity; // #4970 review (greptile P1): same hygiene for keyboard scroll intent. _lastMessageKeyScrollIntentMs=-Infinity; clearTimeout(_deferredOlderMessagesTimer); @@ -4212,6 +4224,7 @@ function _resetStreamScrollFollow(){ // gentle upward wheel within the prior 1200ms can under-suppress a genuine // no-intent render artifact and silently disable live follow for the new stream. _lastMessageWheelIntentMs=-Infinity; + _lastMessageScrollIntentMs=-Infinity; // #4970 review (greptile P1): same hygiene for keyboard scroll intent. _lastMessageKeyScrollIntentMs=-Infinity; _cancelBottomSettle(); @@ -11606,13 +11619,18 @@ function _captureMessageScrollSnapshot(){ const el=$('messages'); if(!el) return null; const bottom=Math.max(0,el.scrollHeight-el.scrollTop-el.clientHeight); + const readerAwayFromBottom=bottom>250&&( + _messageUserUnpinned || + _scrollPinned===false || + (typeof _recentMessageScrollIntent==='function'&&_recentMessageScrollIntent()) + ); return { anchor:(typeof _captureMessageViewportAnchor==='function')?_captureMessageViewportAnchor():null, top:el.scrollTop, bottom, scrollHeight:el.scrollHeight, - pinned:_shouldFollowMessagesOnDomReplace(), - userUnpinned:_messageUserUnpinned, + pinned:readerAwayFromBottom?false:_shouldFollowMessagesOnDomReplace(), + userUnpinned:readerAwayFromBottom?true:_messageUserUnpinned, }; } function _restorePinnedMessageScrollSnapshot(snapshot){ diff --git a/tests/test_issue4295_midstream_scroll_anchor.py b/tests/test_issue4295_midstream_scroll_anchor.py index a27e9fc5..4ecfe363 100644 --- a/tests/test_issue4295_midstream_scroll_anchor.py +++ b/tests/test_issue4295_midstream_scroll_anchor.py @@ -255,6 +255,125 @@ assert.strictEqual(_programmaticScroll, false); subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True) +def test_capture_snapshot_treats_recent_scroll_away_as_manual_reader(): + """Manual live-Worklog browsing must not reuse a stale pinned snapshot. + + A reader can be far from the live tail while scrolling down through a long + Compact Worklog. If a live activity rebuild captures that state while the + sticky globals still say pinned, restoring as a pinned follower preserves a + huge bottom gap and can keep yanking the viewport back toward the turn start. + Recent message-pane scroll intent makes the snapshot explicitly unpinned. + """ + + script = f""" +const assert = require('assert'); +let _messageUserUnpinned = false; +let _scrollPinned = true; +let _lastMessageScrollIntentMs = 1000; +const MESSAGE_WHEEL_INTENT_SUPPRESS_MS = 1200; +const container = {{ + scrollTop: 900, + scrollHeight: 5000, + clientHeight: 600, + getBoundingClientRect() {{ return {{ top: 0, bottom: 600 }}; }}, + querySelectorAll() {{ return []; }}, +}}; +function $(id) {{ return id === 'messages' ? container : null; }} +const performance = {{ now() {{ return 1500; }} }}; +function _captureMessageViewportAnchor() {{ return null; }} +function _recentMessageTouchScrollIntent() {{ return false; }} +function _recentMessageKeyScrollIntent() {{ return false; }} +function _shouldFollowMessagesOnDomReplace() {{ return true; }} +{_function_body(UI_JS, "_recentMessageScrollIntent")} +{_function_body(UI_JS, "_captureMessageScrollSnapshot")} +const snapshot = _captureMessageScrollSnapshot(); +assert.strictEqual(snapshot.bottom, 3500); +assert.strictEqual(snapshot.pinned, false); +assert.strictEqual(snapshot.userUnpinned, true); +""" + subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True) + + +def test_capture_snapshot_keeps_true_pinned_follower_pinned_despite_large_gap(): + """Large bottom distance alone is not enough to unpin a follower. + + During fast streaming, content can grow under a followed viewport before the + follow write lands. Without a recent user scroll intent or explicit unpin + state, the snapshot must stay pinned so the existing tail-relative restore + continues to protect pinned live followers. + """ + + script = f""" +const assert = require('assert'); +let _messageUserUnpinned = false; +let _scrollPinned = true; +let _lastMessageScrollIntentMs = -Infinity; +const MESSAGE_WHEEL_INTENT_SUPPRESS_MS = 1200; +const container = {{ + scrollTop: 900, + scrollHeight: 5000, + clientHeight: 600, + getBoundingClientRect() {{ return {{ top: 0, bottom: 600 }}; }}, + querySelectorAll() {{ return []; }}, +}}; +function $(id) {{ return id === 'messages' ? container : null; }} +const performance = {{ now() {{ return 1500; }} }}; +function _captureMessageViewportAnchor() {{ return null; }} +function _recentMessageTouchScrollIntent() {{ return false; }} +function _recentMessageKeyScrollIntent() {{ return false; }} +function _shouldFollowMessagesOnDomReplace() {{ return true; }} +{_function_body(UI_JS, "_recentMessageScrollIntent")} +{_function_body(UI_JS, "_captureMessageScrollSnapshot")} +const snapshot = _captureMessageScrollSnapshot(); +assert.strictEqual(snapshot.bottom, 3500); +assert.strictEqual(snapshot.pinned, true); +assert.strictEqual(snapshot.userUnpinned, false); +""" + subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True) + + +def test_wheel_scroll_intent_only_records_when_reader_is_away_from_bottom(): + """Downward wheel intent should protect manual reading, not bottom following.""" + + script = f""" +const assert = require('assert'); +let _lastNonMessageScrollIntentMs = -Infinity; +let _lastMessageWheelIntentMs = -Infinity; +let _lastMessageScrollIntentMs = -Infinity; +let _messageUserUnpinned = false; +let _nearBottomCount = 2; +let _scrollPinned = true; +let _messageTouchScrollActive = false; +let _lastMessageTouchScrollIntentMs = -Infinity; +let _touchStartY = null; +const child = {{}}; +const el = {{ + scrollTop: 900, + scrollHeight: 5000, + clientHeight: 600, + contains(target) {{ return target === child; }}, +}}; +function _cancelBottomSettle() {{}} +function _markMessageTouchScrollIntent(active) {{ + _messageTouchScrollActive = !!active; + _lastMessageTouchScrollIntentMs = performance.now(); +}} +const document = {{ getElementById(id) {{ return id === 'messages' ? el : null; }} }}; +const performance = {{ now() {{ return 1234; }} }}; +{_function_body(UI_JS, "_recordNonMessageScrollIntent")} +_recordNonMessageScrollIntent({{ target: child, type: 'wheel', deltaY: 24 }}); +assert.strictEqual(_lastMessageScrollIntentMs, 1234); +assert.strictEqual(_messageUserUnpinned, false); + +_lastMessageScrollIntentMs = -Infinity; +el.scrollTop = 4400; // bottomDistance = 0 +_recordNonMessageScrollIntent({{ target: child, type: 'wheel', deltaY: 24 }}); +assert.strictEqual(_lastMessageScrollIntentMs, -Infinity); +assert.strictEqual(_scrollPinned, true); +""" + subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True) + + def test_restore_message_scroll_snapshot_prefers_semantic_anchor_with_virtual_fallback(): body = _function_body(UI_JS, "_restoreMessageScrollSnapshot") helper = _function_body(UI_JS, "_remountMessageViewportAnchor") From 71a458784acf9bdee325acd90183214ab68e49a7 Mon Sep 17 00:00:00 2001 From: Frank Song Date: Tue, 30 Jun 2026 10:52:11 +0800 Subject: [PATCH 2/2] fix(chat): guard live worklog manual scroll intent Co-Authored-By: Paperclip --- static/ui.js | 12 ++-- .../test_issue3479_ios_stream_scroll_jump.py | 4 +- .../test_issue4295_midstream_scroll_anchor.py | 66 ++++++++++++++++++- ...est_issue4856_android_scroll_regression.py | 6 +- 4 files changed, 78 insertions(+), 10 deletions(-) diff --git a/static/ui.js b/static/ui.js index 67161b60..7fcdee47 100644 --- a/static/ui.js +++ b/static/ui.js @@ -4067,9 +4067,10 @@ function _recentMessageWheelIntent(){ return performance.now()-_lastMessageWheelIntentMs120) _lastMessageScrollIntentMs=performance.now(); } @@ -4351,7 +4352,10 @@ if(typeof window!=='undefined'){ // Count only when the message pane itself is the scroll target: it is focused, // contains the focus, or the pointer is over it (keyboard scroll w/o focus). if(a===el||el.contains(a)||el.matches(':hover')){ - _lastMessageKeyScrollIntentMs=performance.now(); + const now=performance.now(); + _lastMessageKeyScrollIntentMs=now; + const bottomDistance=el.scrollHeight-el.scrollTop-el.clientHeight; + if(bottomDistance>120) _lastMessageScrollIntentMs=now; } },{capture:true,passive:true}); let _scrollRaf=0; diff --git a/tests/test_issue3479_ios_stream_scroll_jump.py b/tests/test_issue3479_ios_stream_scroll_jump.py index 3dbadce8..c803abf3 100644 --- a/tests/test_issue3479_ios_stream_scroll_jump.py +++ b/tests/test_issue3479_ios_stream_scroll_jump.py @@ -101,8 +101,8 @@ def test_same_frame_snapshot_preserves_bottom_distance_and_unpinned_state(): wrapper = _function_body(UI_JS, "_renderMessagesWithScrollSnapshot") assert "bottom" in capture - assert "pinned:_shouldFollowMessagesOnDomReplace()" in _compact(capture) - assert "userUnpinned:_messageUserUnpinned" in _compact(capture) + assert "readerAwayFromBottom?false:_shouldFollowMessagesOnDomReplace()" in _compact(capture) + assert "readerAwayFromBottom?true:_messageUserUnpinned" in _compact(capture) assert "maxTop-Math.max(0,bottom)" in restore assert "_messageUserUnpinned=true" in restore assert "_scrollPinned=false" in restore diff --git a/tests/test_issue4295_midstream_scroll_anchor.py b/tests/test_issue4295_midstream_scroll_anchor.py index 4ecfe363..063a1c9e 100644 --- a/tests/test_issue4295_midstream_scroll_anchor.py +++ b/tests/test_issue4295_midstream_scroll_anchor.py @@ -300,7 +300,10 @@ def test_capture_snapshot_keeps_true_pinned_follower_pinned_despite_large_gap(): During fast streaming, content can grow under a followed viewport before the follow write lands. Without a recent user scroll intent or explicit unpin state, the snapshot must stay pinned so the existing tail-relative restore - continues to protect pinned live followers. + continues to protect pinned live followers. Raw touch/key recency is not + enough here because those helpers also track near-tail artifact-suppression + windows; only the guarded away-from-bottom intent stamp can classify a + snapshot as manual-reader state. """ script = f""" @@ -319,8 +322,8 @@ const container = {{ function $(id) {{ return id === 'messages' ? container : null; }} const performance = {{ now() {{ return 1500; }} }}; function _captureMessageViewportAnchor() {{ return null; }} -function _recentMessageTouchScrollIntent() {{ return false; }} -function _recentMessageKeyScrollIntent() {{ return false; }} +function _recentMessageTouchScrollIntent() {{ return true; }} +function _recentMessageKeyScrollIntent() {{ return true; }} function _shouldFollowMessagesOnDomReplace() {{ return true; }} {_function_body(UI_JS, "_recentMessageScrollIntent")} {_function_body(UI_JS, "_captureMessageScrollSnapshot")} @@ -374,6 +377,63 @@ assert.strictEqual(_scrollPinned, true); subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True) +def test_manual_scroll_snapshot_intent_excludes_raw_touch_and_key_recency(): + """Only bottom-guarded message scroll intent can drive snapshot unpinning.""" + + compact = _compact(_function_body(UI_JS, "_recentMessageScrollIntent")) + + assert "_lastMessageScrollIntentMs" in compact + assert "_scrollbarDragActive" in compact + assert "_recentMessageTouchScrollIntent" not in compact + assert "_recentMessageKeyScrollIntent" not in compact + assert "_lastMessageKeyScrollIntentMs=now;" in _compact(UI_JS) + assert "if(bottomDistance>120)_lastMessageScrollIntentMs=now;" in _compact(UI_JS) + + +def test_touch_scroll_intent_only_records_when_reader_is_away_from_bottom(): + """Touch recency is broad, but snapshot intent is bottom-distance guarded.""" + + script = f""" +const assert = require('assert'); +let _lastNonMessageScrollIntentMs = -Infinity; +let _lastMessageWheelIntentMs = -Infinity; +let _lastMessageScrollIntentMs = -Infinity; +let _messageUserUnpinned = false; +let _nearBottomCount = 2; +let _scrollPinned = true; +let _messageTouchScrollActive = false; +let _lastMessageTouchScrollIntentMs = -Infinity; +let _touchStartY = null; +const child = {{}}; +const el = {{ + scrollTop: 900, + scrollHeight: 5000, + clientHeight: 600, + contains(target) {{ return target === child; }}, +}}; +function _cancelBottomSettle() {{}} +function _markMessageTouchScrollIntent(active) {{ + _messageTouchScrollActive = !!active; + _lastMessageTouchScrollIntentMs = performance.now(); +}} +const document = {{ getElementById(id) {{ return id === 'messages' ? el : null; }} }}; +const performance = {{ now() {{ return 1234; }} }}; +{_function_body(UI_JS, "_recordNonMessageScrollIntent")} +_recordNonMessageScrollIntent({{ target: child, type: 'touchmove' }}); +assert.strictEqual(_lastMessageScrollIntentMs, 1234); +assert.strictEqual(_lastMessageTouchScrollIntentMs, 1234); +assert.strictEqual(_messageUserUnpinned, false); + +_lastMessageScrollIntentMs = -Infinity; +el.scrollTop = 4400; // bottomDistance = 0 +_recordNonMessageScrollIntent({{ target: child, type: 'touchmove' }}); +assert.strictEqual(_lastMessageScrollIntentMs, -Infinity); +assert.strictEqual(_lastMessageTouchScrollIntentMs, 1234); +assert.strictEqual(_scrollPinned, true); +""" + subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True) + + def test_restore_message_scroll_snapshot_prefers_semantic_anchor_with_virtual_fallback(): body = _function_body(UI_JS, "_restoreMessageScrollSnapshot") helper = _function_body(UI_JS, "_remountMessageViewportAnchor") diff --git a/tests/test_issue4856_android_scroll_regression.py b/tests/test_issue4856_android_scroll_regression.py index e66dbe7c..aa65e5b1 100644 --- a/tests/test_issue4856_android_scroll_regression.py +++ b/tests/test_issue4856_android_scroll_regression.py @@ -420,10 +420,14 @@ def test_keyboard_scroll_intent_tracked_and_gated(): assert "let _lastMessageKeyScrollIntentMs=-Infinity" in UI_JS assert "function _recentMessageKeyScrollIntent" in UI_JS # A keydown listener must stamp the intent for the pane scroll keys. - assert "_lastMessageKeyScrollIntentMs=performance.now()" in UI_JS, ( + assert "_lastMessageKeyScrollIntentMs=now;" in UI_JS, ( "a keydown handler must stamp _lastMessageKeyScrollIntentMs when the " "reader uses the keyboard to scroll the message pane." ) + assert "if(bottomDistance>120) _lastMessageScrollIntentMs=now;" in UI_JS, ( + "keyboard-driven manual-reader snapshot intent must be guarded by " + "distance from the live tail." + ) assert "'PageUp'" in UI_JS and "'PageDown'" in UI_JS # The suppression branch must consult it. listener_idx = UI_JS.find("el.addEventListener('scroll'")