feat: sticky manual unpin for streaming chat scroll (#3343)

Supersedes the v0.51.199 proximity-re-pin (#3330) and the #3250 upward-intent
timeout with a sticky-unpin model (ChatGPT/Claude/Codex behavior): scroll up =
stay put until you return to the bottom or click the scroll-to-bottom control.
Reconciled against the shipped #3330 code: removed the now-dead
_recentMessageUpwardIntent reference from the #3319 rAF retry, kept the
load-time -Infinity intent-init fix, kept the pinned-only >500 catch-up.

Co-authored-by: pamnard <pamnard@users.noreply.github.com>
This commit is contained in:
nesquena-hermes
2026-06-01 20:34:06 +00:00
parent 720695a0c2
commit d09f2e4efb
7 changed files with 123 additions and 185 deletions

View File

@@ -3,6 +3,11 @@
## [Unreleased]
## [v0.51.203] — 2026-06-01 — Release FW (stage-batch15 — sticky manual unpin for streaming chat scroll)
### Changed
- Streaming chat scroll now uses a sticky manual-unpin model: once you scroll up to read earlier content during a streaming response, the view stays put and no longer auto-follows the live tail until you scroll back to the bottom (near-bottom hysteresis on downward motion) or click the scroll-to-bottom control. Tool cards, token updates, and layout growth no longer re-pin the viewport after a reading pause. This replaces the #3250 upward-intent timeout and supersedes the v0.51.199 proximity-re-pin (#3330), matching the streaming-scroll behavior of ChatGPT/Claude/Codex. Fresh streams reset the follow state on attach (#3343, @pamnard).
## [v0.51.202] — 2026-06-01 — Release FV (stage-batch14 — filter interrupted-recovery control text from visible transcript)
### Fixed

View File

@@ -685,6 +685,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
closeOtherLiveStreams(activeSid);
closeLiveStream(activeSid);
if(!reconnecting&&typeof resetTurnWorkspaceMutations==='function') resetTurnWorkspaceMutations();
if(!reconnecting&&typeof _resetStreamScrollFollow==='function') _resetStreamScrollFollow();
// On reconnect, restore accumulated text from INFLIGHT so we don't lose
// progress made before the session switch. Without this the closure starts

View File

@@ -2197,58 +2197,32 @@ window.addEventListener('resize',function(){
});
// ── Scroll pinning ──────────────────────────────────────────────────────────
// When streaming, auto-scroll only if the user hasn't manually scrolled up.
// Once the user scrolls back to within 250px of the bottom, re-pin.
// Uses a guard flag to avoid the race where programmatic scrolls (from
// scrollIfPinned / scrollToBottom) re-set _scrollPinned=true, overriding
// the user's explicit scroll-up. Fixes #1469 / #1360.
// Direction-aware unpin (issue #1731): the hysteresis below is correct
// for re-pinning (entering the near-bottom zone), but applying it to
// unpinning stranded users who scrolled up by a small amount inside the
// 250px zone — every upward sample still landed in the near-bottom
// region, so the counter kept incrementing and _scrollPinned stayed
// true. The next streaming token snapped them back. We now track
// scrollTop direction: an explicit upward movement (scrollTop decreased
// by more than 2px between samples) unpins immediately and resets the
// counter, while downward / stationary movement falls through the
// original hysteresis path so the macOS momentum re-pin protection from
// #1360 is preserved.
// rAF-debounced scroll listener (issue #1360): on macOS WKWebView, trackpad
// momentum scrolling fires scroll events that interleave with the
// _programmaticScroll setTimeout(0) guard. A mid-momentum scroll event can
// either get swallowed (_programmaticScroll still true) or falsely report
// the user is at the bottom (momentum hasn't settled). rAF defers the
// distance check to the next paint frame when the browser's scroll
// position has settled. A hysteresis counter requires two consecutive
// near-bottom samples before re-pinning, preventing accidental re-pin
// during initial deceleration.
// When streaming, auto-scroll only while the user is following the live tail.
// Any manual scroll up sets a sticky unpinned flag until the user scrolls back
// to the bottom (near-bottom hysteresis on downward motion) or clicks ↓.
// Programmatic scrolls are ignored via _programmaticScroll. Fixes #1469 / #1360 / #1731.
let _scrollPinned=true;
let _programmaticScroll=false;
let _nearBottomCount=0;
let _lastScrollTop=null;
// Sticky-unpin model (#3343 supersedes #3330's proximity re-pin): once the user
// scrolls up, streaming stops auto-following until they return to the bottom or
// click ↓. The upward-intent TIMEOUT mechanism (_lastMessageUpwardIntentMs /
// MESSAGE_UPWARD_INTENT_MS) is removed — sticky-unpin makes it unnecessary.
// Keep the non-message intent timestamp at -Infinity so load-time isn't read as
// intent (the #3330 follow-up fix); 0 would mark the first NON_MESSAGE_SCROLL_INTENT
// window after load as suppressed.
let _lastNonMessageScrollIntentMs=-Infinity;
let _lastMessageUpwardIntentMs=-Infinity;
let _messageUserUnpinned=false;
let _bottomSettleToken=0;
const NON_MESSAGE_SCROLL_INTENT_SUPPRESS_MS=350;
const MESSAGE_UPWARD_INTENT_MS=2000;
function _cancelBottomSettle(){ _bottomSettleToken++; }
function _recordNonMessageScrollIntent(e){
const el=document.getElementById('messages');
const target=e&&e.target;
if(!el||!target) return;
// Streaming token renders should keep pinning the chat only while the user is
// actually interacting with the chat pane. A wheel/touch gesture over the
// session sidebar (or another independent pane) must not be immediately fought
// by scrollIfPinned() writing #messages.scrollTop on the next token (#1784).
if(!el.contains(target)) _lastNonMessageScrollIntentMs=performance.now();
else if(e.type==='touchmove'||(typeof e.deltaY==='number'&&e.deltaY<0)){
// User is intentionally moving upward in the transcript. Record the real
// input event so later scrollTop decreases caused by layout/windowing do
// not masquerade as user intent and strand live streaming away from bottom.
_lastMessageUpwardIntentMs=performance.now();
// User is intentionally moving in the transcript. Cancel any delayed
// scrollToBottom settling that was scheduled by session-load/layout growth.
_cancelBottomSettle();
if(typeof e.deltaY==='number'&&e.deltaY<0){
_messageUserUnpinned=true;
@@ -2257,9 +2231,6 @@ function _recordNonMessageScrollIntent(e){
}
}
}
function _recentMessageUpwardIntent(){
return performance.now()-_lastMessageUpwardIntentMs<MESSAGE_UPWARD_INTENT_MS;
}
function _recentNonMessageScrollIntent(){
return performance.now()-_lastNonMessageScrollIntentMs<NON_MESSAGE_SCROLL_INTENT_SUPPRESS_MS;
}
@@ -2270,8 +2241,23 @@ if(typeof document!=='undefined'){
// Reset hook for session-switch — called from sessions.js loadSession() to
// prevent the new chat's first scroll comparing against the previous chat's
// scrollTop (Opus stage-302 SHOULD-FIX, #1731 follow-up).
function _resetScrollDirectionTracker(){ _lastScrollTop=null; }
if(typeof window!=='undefined') window._resetScrollDirectionTracker=_resetScrollDirectionTracker;
function _resetScrollDirectionTracker(){
_lastScrollTop=null;
_messageUserUnpinned=false;
_scrollPinned=true;
_nearBottomCount=0;
}
function _resetStreamScrollFollow(){
_messageUserUnpinned=false;
_scrollPinned=true;
_nearBottomCount=0;
_lastScrollTop=null;
_cancelBottomSettle();
}
if(typeof window!=='undefined'){
window._resetScrollDirectionTracker=_resetScrollDirectionTracker;
window._resetStreamScrollFollow=_resetStreamScrollFollow;
}
/* ── Pull-to-refresh for PWA standalone (Android) ── */
(function(){
if(typeof document==='undefined') return;
@@ -2351,17 +2337,32 @@ if(typeof window!=='undefined') window._resetScrollDirectionTracker=_resetScroll
_scrollRaf=requestAnimationFrame(()=>{
const top=el.scrollTop;
const nearBottom=el.scrollHeight-top-el.clientHeight<250;
// scrollToBottomBtn visibility is updated below after pin state settles.
const movedUp=_lastScrollTop!==null && top<_lastScrollTop-2 && _recentMessageUpwardIntent();
const movedUp=_lastScrollTop!==null&&top<_lastScrollTop-2;
const movedDown=_lastScrollTop!==null&&top>_lastScrollTop+2;
_lastScrollTop=top;
if(movedUp){ _cancelBottomSettle(); _nearBottomCount=0; _scrollPinned=false; _messageUserUnpinned=true; } // #1731
else {
if(movedUp){
_cancelBottomSettle();
_nearBottomCount=0;
_scrollPinned=false;
_messageUserUnpinned=true;
}else if(movedDown&&nearBottom){
_nearBottomCount=_nearBottomCount+1;
if(_nearBottomCount>=2){
_scrollPinned=true;
_messageUserUnpinned=false;
}
}else if(!_messageUserUnpinned){
if(nearBottom){
_nearBottomCount=_nearBottomCount+1;
if(_nearBottomCount>=2) _scrollPinned=true;
} else { _nearBottomCount=0; _scrollPinned=false; }
if(_scrollPinned) _messageUserUnpinned=false;
} // #1360
}else{
_nearBottomCount=0;
_scrollPinned=false;
}
}else if(!nearBottom){
_nearBottomCount=0;
_scrollPinned=false;
}
const btn=$('scrollToBottomBtn');
const showBottomButton=!_scrollPinned && el.scrollHeight-top-el.clientHeight>80;
if(btn) btn.style.display=showBottomButton?'flex':'none';
@@ -2762,11 +2763,10 @@ function _setMessageScrollToBottom(){
// Retry the bottom write on the next layout frame so a DOM rebuild that
// grows the transcript after the first write doesn't strand a pinned
// conversation mid-scroll (#3319). But by this frame the user may have
// scrolled up — re-check intent and DON'T snap them back or re-pin if so;
// only release the programmatic-scroll latch.
if(_messageUserUnpinned || !_scrollPinned
|| (typeof _recentMessageUpwardIntent==='function' && _recentMessageUpwardIntent())
|| _recentNonMessageScrollIntent()){
// scrolled up — under the sticky-unpin model (#3343) _messageUserUnpinned
// is the authoritative "user scrolled away" signal, so DON'T snap them back
// or re-pin if so; only release the programmatic-scroll latch.
if(_messageUserUnpinned || !_scrollPinned || _recentNonMessageScrollIntent()){
requestAnimationFrame(()=>{ setTimeout(()=>{_programmaticScroll=false;},0); });
return;
}
@@ -2800,19 +2800,20 @@ function _settleMessageScrollToBottom(force){
const passes=[0,16,80,180];
passes.forEach(delay=>setTimeout(()=>{
if(token!==_bottomSettleToken) return;
if(!force && (!_scrollPinned||_recentNonMessageScrollIntent())) return;
if(!force && (!_scrollPinned||_messageUserUnpinned||_recentNonMessageScrollIntent())) return;
_setMessageScrollToBottom();
},delay));
requestAnimationFrame(()=>{
if(token!==_bottomSettleToken) return;
if(force || (_scrollPinned&&!_recentNonMessageScrollIntent())) _setMessageScrollToBottom();
if(force || (_scrollPinned&&!_messageUserUnpinned&&!_recentNonMessageScrollIntent())) _setMessageScrollToBottom();
requestAnimationFrame(()=>{
if(token!==_bottomSettleToken) return;
if(force || (_scrollPinned&&!_recentNonMessageScrollIntent())) _setMessageScrollToBottom();
if(force || (_scrollPinned&&!_messageUserUnpinned&&!_recentNonMessageScrollIntent())) _setMessageScrollToBottom();
});
});
}
function scrollIfPinned(){
if(_messageUserUnpinned) return;
if(!_scrollPinned) return;
if(_recentNonMessageScrollIntent()) return;
if(_messageBottomDistance()>500) _setMessageScrollToBottom();
@@ -6326,6 +6327,7 @@ function _restoreMessageScrollSnapshot(snapshot){
const maxTop=Math.max(0,el.scrollHeight-el.clientHeight);
_programmaticScroll=true;
el.scrollTop=Math.max(0,Math.min(Number(snapshot.top)||0,maxTop));
// Sync _lastScrollTop after programmatic restore so sticky-unpin does not false-trigger (#1731).
_lastScrollTop=el.scrollTop;
requestAnimationFrame(()=>{ setTimeout(()=>{_programmaticScroll=false;},0); });
}

View File

@@ -72,7 +72,6 @@ def test_upward_scroll_unpins_immediately_without_hysteresis():
"""Upward motion sets _scrollPinned=false and resets the counter, no count needed."""
block = _scroll_listener_block()
if_idx = block.index("if(movedUp)")
# Tolerate either single-line or multi-line if/else formatting.
else_idx = block.find("else", if_idx)
assert else_idx > if_idx, "upward / downward branches not found (#1731)"
upward_branch = block[if_idx:else_idx]
@@ -85,74 +84,40 @@ def test_upward_scroll_unpins_immediately_without_hysteresis():
"Upward scroll must reset _nearBottomCount so a subsequent "
"downward motion has to clear the hysteresis fresh (#1731)."
)
assert "_nearBottomCount>=2" not in upward_branch, (
"The upward branch must not gate unpinning on hysteresis — that "
"was the bug (#1731)."
assert "_messageUserUnpinned=true" in upward_branch, (
"Upward scroll must set the sticky manual-unpin flag."
)
def test_upward_motion_only_unpins_after_recent_user_intent():
"""Layout/programmatic scrollTop decreases must not masquerade as user scroll-up.
Long-session windowing can preserve/restore scroll positions while the live
stream is growing. If a plain scrollTop decrease always clears
``_scrollPinned``, the viewport can be visually at bottom while the state says
"not pinned", so streaming stops auto-following. Explicit wheel/touch upward
input must still unpin immediately; passive layout movement must not.
"""
assert "let _lastMessageUpwardIntentMs=" in UI_JS, (
"ui.js must track recent upward wheel/touch intent inside #messages so "
"programmatic/layout scroll changes do not permanently unpin streaming."
)
assert "function _recentMessageUpwardIntent()" in UI_JS, (
"ui.js must expose a recent upward transcript intent helper."
)
def test_upward_motion_unpins_on_scroll_top_delta_without_intent_timeout():
"""Scrollbar / keyboard upward scroll must unpin without a wheel intent window."""
block = _scroll_listener_block()
moved_idx = block.index("const movedUp=")
moved_expr = block[moved_idx : block.find(";", moved_idx)]
assert "_recentMessageUpwardIntent()" in moved_expr, (
"movedUp must require recent wheel/touch upward intent, not only a "
"scrollTop decrease caused by DOM/layout changes."
assert "_recentMessageUpwardIntent()" not in moved_expr, (
"movedUp must use scrollTop direction only; sticky unpin replaces the #3250 timeout."
)
assert "_lastScrollTop-2" in moved_expr or "top<_lastScrollTop -" in moved_expr
def test_wheel_touch_upward_intent_is_recorded_inside_messages():
"""Wheel/touch gestures inside #messages must mark real upward user intent."""
def test_wheel_touch_upward_intent_unpins_immediately_inside_messages():
"""Wheel/touch up inside #messages must unpin before the scroll listener runs."""
fn_start = UI_JS.index("function _recordNonMessageScrollIntent")
fn_end = UI_JS.index("function _recentNonMessageScrollIntent", fn_start)
fn = UI_JS[fn_start:fn_end]
assert "_lastMessageUpwardIntentMs=performance.now()" in fn, (
"_recordNonMessageScrollIntent must timestamp real upward transcript "
"wheel/touch gestures before clearing _scrollPinned."
)
assert "e.deltaY<0" in fn and "e.type==='touchmove'" in fn, (
"Both wheel-up and touchmove gestures inside #messages should count as "
"user upward intent."
)
assert "_messageUserUnpinned=true" in fn.replace(" ", "")
assert "e.deltaY<0" in fn and "e.type==='touchmove'" in fn
def test_downward_path_preserves_macos_momentum_hysteresis():
"""Downward / stationary motion must still go through the original
hysteresis re-pin path so the #1360 macOS trackpad momentum protection
is preserved.
"""
"""Downward motion into the near-bottom zone re-follows with hysteresis (#1360)."""
block = _scroll_listener_block()
else_idx = block.index("else", block.index("if(movedUp)"))
# End of else branch is at the next btn lookup line.
end_idx = block.index("const btn=", else_idx)
downward_branch = block[else_idx:end_idx]
assert "if(nearBottom)" in downward_branch, (
"Downward path must branch on near-bottom state so the macOS momentum "
"re-pin guard still applies (#1360)."
assert "elseif(movedDown&&nearBottom)" in block.replace(" ", ""), (
"Explicit downward scroll into the near-bottom zone must be the re-follow path "
"after a sticky manual unpin."
)
assert "_nearBottomCount=_nearBottomCount+1" in downward_branch, (
"Downward path must keep incrementing the near-bottom counter so "
"the macOS momentum re-pin guard still applies (#1360)."
)
assert "if(_nearBottomCount>=2) _scrollPinned=true" in downward_branch, (
"Downward path must keep the >=2 hysteresis re-pin requirement "
"without downgrading an explicit bottom pin on the first near-bottom event (#1360)."
assert "if(_nearBottomCount>=2)" in block, (
"Re-follow still requires two consecutive near-bottom samples."
)

View File

@@ -1,83 +1,23 @@
"""Regression test for #3250: upward-scroll intent window during streaming.
"""Regression: sticky manual unpin during streaming (supersedes #3250 timeout tuning).
The pre-fix `MESSAGE_UPWARD_INTENT_MS` window was only 450ms. When a user
scrolled up to read earlier content during a streaming response and then
*paused* to read (>450ms since their last wheel/touch event), the intent
expired. Subsequent DOM-layout changes from the streaming markdown parser
(smd), tool-card insertions, or code re-highlighting then produced scroll
events that `_recentMessageUpwardIntent()` no longer attributed to the user
(`movedUp = false`). If the resulting position sat inside the 250px
near-bottom zone for two consecutive samples, `_scrollPinned` flipped back to
true and the next streaming token snapped the user to the bottom.
The fix widens the window to 2000ms so a brief reading pause no longer drops
the user's intent. Direction detection is unchanged — downward motion still
re-pins regardless of the timeout because `movedUp` additionally requires
`top < _lastScrollTop - 2` — so this is a pure intent-duration tuning, not a
relaxation of the re-pin semantics.
After the user scrolls up to read earlier content, streaming tokens, tool cards,
and layout growth must not re-pin the viewport until the user scrolls back to
the bottom or clicks the scroll-to-bottom control.
"""
from pathlib import Path
REPO = Path(__file__).resolve().parents[1]
UI_JS = (REPO / "static" / "ui.js").read_text(encoding="utf-8")
MESSAGES_JS = (REPO / "static" / "messages.js").read_text(encoding="utf-8")
def _intent_window_ms() -> int:
"""Extract the numeric value of the MESSAGE_UPWARD_INTENT_MS constant."""
marker = "const MESSAGE_UPWARD_INTENT_MS="
idx = UI_JS.find(marker)
assert idx != -1, "MESSAGE_UPWARD_INTENT_MS constant not found in ui.js"
start = idx + len(marker)
end = UI_JS.find(";", start)
raw = UI_JS[start:end].strip()
return int(raw)
def test_upward_intent_window_is_widened_for_reading_pauses():
"""The intent window must be >=2000ms so a brief reading pause during a
streaming response does not drop upward-scroll intent and re-pin the view
(#3250). The pre-fix 450ms value was too short for a real read-pause.
"""
window = _intent_window_ms()
assert window >= 2000, (
f"MESSAGE_UPWARD_INTENT_MS is {window}ms; #3250 requires >=2000ms so a "
"reading pause during streaming does not expire upward-scroll intent "
"and snap the user back to the bottom."
)
def test_intent_helper_compares_against_the_window_constant():
"""_recentMessageUpwardIntent() must gate on the window constant, so the
widened value actually takes effect (guards against the helper being
rewritten with a hardcoded duration).
"""
marker = "function _recentMessageUpwardIntent()"
idx = UI_JS.find(marker)
assert idx != -1, "_recentMessageUpwardIntent() not found in ui.js"
body = UI_JS[idx:UI_JS.find("}", idx) + 1]
assert "MESSAGE_UPWARD_INTENT_MS" in body, (
"_recentMessageUpwardIntent() must compare against MESSAGE_UPWARD_INTENT_MS "
"rather than a hardcoded duration (#3250)."
)
assert "_lastMessageUpwardIntentMs" in body, (
"_recentMessageUpwardIntent() must measure elapsed time since the last "
"recorded upward intent timestamp (#3250)."
)
def test_downward_repin_is_independent_of_the_intent_window():
"""Widening the intent window must not weaken downward re-pin: the movedUp
flag still requires an actual upward scrollTop delta (`top < _lastScrollTop
- 2`), so downward motion re-pins regardless of how long the intent window
is. This is what keeps the #3250 tuning safe.
"""
def _scroll_listener_block() -> str:
anchor = "el.addEventListener('scroll'"
start = UI_JS.index(anchor)
raf_start = UI_JS.index("requestAnimationFrame", start)
brace = UI_JS.index("{", raf_start)
depth = 0
block = ""
for i in range(brace, len(UI_JS)):
ch = UI_JS[i]
if ch == "{":
@@ -85,12 +25,37 @@ def test_downward_repin_is_independent_of_the_intent_window():
elif ch == "}":
depth -= 1
if depth == 0:
block = UI_JS[brace:i + 1]
break
assert block, "scroll listener rAF callback not found"
moved_idx = block.index("const movedUp=")
moved_expr = block[moved_idx:block.find(";", moved_idx)]
assert "_lastScrollTop-2" in moved_expr or "_lastScrollTop -" in moved_expr, (
"movedUp must still require an explicit upward scrollTop delta so "
"downward motion re-pins independently of the intent window (#3250)."
return UI_JS[brace : i + 1]
raise AssertionError("scroll listener rAF callback not found")
def test_scroll_if_pinned_respects_sticky_user_unpin():
fn = UI_JS[UI_JS.index("function scrollIfPinned"): UI_JS.index("function scrollToBottom")]
compact = fn.replace(" ", "")
assert "if(_messageUserUnpinned)return" in compact, (
"scrollIfPinned() must not fight a sticky manual unpin during streaming"
)
def test_sticky_unpin_blocks_near_bottom_repin_without_downward_scroll():
block = _scroll_listener_block()
assert "_recentMessageUpwardIntent()" not in block
compact = block.replace(" ", "")
assert "elseif(!_messageUserUnpinned)" in compact, (
"Near-bottom hysteresis re-pin must be gated off while the user is manually unpinned"
)
assert "elseif(movedDown&&nearBottom)" in compact, (
"Re-follow must require explicit downward scroll into the near-bottom zone"
)
def test_new_stream_resets_follow_state():
attach = MESSAGES_JS[MESSAGES_JS.index("function attachLiveStream"):]
assert "_resetStreamScrollFollow" in attach, (
"A fresh live stream should default to following the tail until the user scrolls up"
)
def test_scroll_to_bottom_clears_sticky_unpin():
fn = UI_JS[UI_JS.index("function scrollToBottom"): UI_JS.index("function _fmtOllamaLabel")]
assert "_messageUserUnpinned=false" in fn.replace(" ", "")

View File

@@ -128,7 +128,7 @@ class TestScrollPinningFix:
assert scroll_listener_start != -1, "scroll event listener not found"
# After #1360 fix, the nearBottom + btn logic lives inside an rAF
# callback — extend search window to cover the full listener block.
listener_block = UI_JS[scroll_listener_start:scroll_listener_start + 600]
listener_block = UI_JS[scroll_listener_start:scroll_listener_start + 1200]
assert "scrollToBottomBtn" in listener_block, (
"Scroll listener must show/hide scrollToBottomBtn based on _scrollPinned (#677)"
)

View File

@@ -72,7 +72,7 @@ def test_message_scroll_listener_does_not_downgrade_explicit_bottom_pin_on_first
assert "_nearBottomCount=2" in set_bottom
assert "_scrollPinned=_nearBottomCount>=2" not in listener_block
assert "if(_nearBottomCount>=2) _scrollPinned=true" in listener_block
assert "else { _nearBottomCount=0; _scrollPinned=false; }" in listener_block
assert "_scrollPinned=false" in listener_block
def test_user_scroll_cancels_delayed_bottom_settling():