fix(chat): keep autoscroll pinned when the virtualizer re-scrolls during streaming#5093
Conversation
…ing streaming The sticky-scroll detach heuristic (scrollTop drops while scrollHeight doesn't grow) could not distinguish a user scrollbar drag from a programmatic scroll. react-virtual re-pins content by moving scrollTop whenever a measured row's size changes — including the transient height shrinks streamdown emits as it re-parses each streaming token — so the hook misread those upward programmatic scrolls as the user scrolling away and detached mid-stream. Gate the scroll-delta detach branch behind a genuine recent user gesture (pointerdown/up tracking + wheel/touch/keydown stamp). Programmatic scrolls have no preceding gesture, so they no longer detach; scrollbar drag, wheel, and keyboard detach are preserved.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryLow Risk Overview
Streaming start re-seeds stickiness from the current position (users already scrolled up stay detached). Gesture refs reset on effect teardown so state does not leak across streams. Listeners are passive and cleaned up with the streaming effect. Reviewed by Cursor Bugbot for commit 7746faa. Configure here. |
Greptile SummaryThis PR fixes a mid-stream autoscroll detachment regression caused by
Confidence Score: 4/5The core fix is correct and well-scoped; one inconsistency in the detach path could allow autoscroll to re-engage unexpectedly after a scrollbar drag. The gesture-detection approach is sound and the cleanup is correct. One gap: the onScroll detach branch sets only stickyRef.current = false rather than calling detach(), leaving userDetachedRef.current = false. This keeps the re-engagement threshold at the lenient 30 px (STICK_THRESHOLD) instead of the strict 5 px (REATTACH_THRESHOLD), so a virtualizer re-pin landing within 30 px of the bottom can immediately pull autoscroll back on after the user has just intentionally dragged the scrollbar away. apps/sim/hooks/use-auto-scroll.ts — specifically the onScroll detach branch (lines 145-151) Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
S([scroll event fires]) --> UD{userDetachedRef?}
UD -->|true| TH2[threshold = REATTACH_THRESHOLD 5px]
UD -->|false| TH1[threshold = STICK_THRESHOLD 30px]
TH1 & TH2 --> NEAR{distanceFromBottom <= threshold?}
NEAR -->|yes| RE[re-engage: stickyRef=true, userDetachedRef=false]
NEAR -->|no| DRV{userDriven? pointerDownRef OR recent keydown < 250ms}
DRV -->|no programmatic virtualizer re-pin| SKIP[no detach, autoscroll stays pinned]
DRV -->|yes| UP{scrollTop dropped AND scrollHeight stable?}
UP -->|yes| DET[soft detach: stickyRef=false, userDetachedRef unchanged]
UP -->|no| SKIP2[no change]
W([wheel deltaY < 0]) --> HARD[hard detach: stickyRef=false, userDetachedRef=true]
T([touch swipe up]) --> HARD
HARD -.->|tighter 5px re-engagement| TH2
DET -.->|lenient 30px re-engagement| TH1
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
S([scroll event fires]) --> UD{userDetachedRef?}
UD -->|true| TH2[threshold = REATTACH_THRESHOLD 5px]
UD -->|false| TH1[threshold = STICK_THRESHOLD 30px]
TH1 & TH2 --> NEAR{distanceFromBottom <= threshold?}
NEAR -->|yes| RE[re-engage: stickyRef=true, userDetachedRef=false]
NEAR -->|no| DRV{userDriven? pointerDownRef OR recent keydown < 250ms}
DRV -->|no programmatic virtualizer re-pin| SKIP[no detach, autoscroll stays pinned]
DRV -->|yes| UP{scrollTop dropped AND scrollHeight stable?}
UP -->|yes| DET[soft detach: stickyRef=false, userDetachedRef unchanged]
UP -->|no| SKIP2[no change]
W([wheel deltaY < 0]) --> HARD[hard detach: stickyRef=false, userDetachedRef=true]
T([touch swipe up]) --> HARD
HARD -.->|tighter 5px re-engagement| TH2
DET -.->|lenient 30px re-engagement| TH1
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…/touch opening detach window - Reset pointerDownRef in effect cleanup so a pointer held through teardown (e.g. dragging the scrollbar as a stream finishes) can't leak a stuck-true ref into the next session and detach on the first programmatic re-pin. - Wheel-up and touch-drag already detach directly, so the onScroll delta heuristic only needs to authorize scrollbar drag (pointerDownRef) and keyboard. Stop stamping the gesture window on wheel/touch, which otherwise let a harmless downward wheel open a 250ms window where a virtualizer shrink could falsely detach.
|
@greptile review |
|
@cursor review |
…comments - onPointerDown only marks an active drag when the press targets the scroll container itself (the scrollbar), not its content, so a text-selection drag on a message can't authorize a detach during a programmatic re-pin. - Reset lastUserGestureAtRef on teardown alongside pointerDownRef so neither a held pointer nor a late keydown can leak across streaming sessions. - Convert the hook's inline comments to TSDoc on the relevant declarations per codebase conventions.
|
@greptile review |
|
@cursor review |
onKeyDown stamped the gesture window on any bubbling key, so an unrelated keypress within USER_GESTURE_WINDOW of a programmatic virtualizer re-pin could satisfy userDriven and detach mid-stream. Filter to the upward scroll keys (ArrowUp, PageUp, Home, Shift+Space), mirroring the wheel handler's upward-only rule, so only a genuine upward keyboard scroll authorizes detach.
|
@greptile review |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 7746faa. Configure here.
1 participant
Summary
react-virtualre-pins content by movingscrollTopwhenever a measured row's size changes — including the transient height shrinks streamdown emits as it re-parses each streaming token — so the hook misread those upward programmatic scrolls as the user scrolling away. This is the "perfect autoscroll" regression introduced alongside virtualization + streamdown.mothership-chat.tsxconsumes this hook; new listeners are passive and torn down with the streaming-gated effect.Type of Change
Testing
Confirmed against the exact
onScrolllogic in headless Chromium: a programmatic up-scroll on a row-size shrink flips the original hook to detached, while the patched hook stays pinned and still detaches on genuine scrollbar-drag / wheel / keyboard.tscclean, biome clean.Checklist