fix(desktop): restore streaming auto-scroll and keep composer visible - #42570
fix(desktop): restore streaming auto-scroll and keep composer visible#42570hanZeng-08 wants to merge 1 commit into
Conversation
Re-introduce a ResizeObserver-gated bottom-pin during active streaming so the chat viewport follows content growth while the user is parked at the bottom. The observer is disconnected when the run completes, so post-run layout shifts (e.g. Shiki re-highlight) do not yank the viewport back down. - Add RO on scroll content in VirtualizedThread that calls pinToBottom() only when stickyBottomRef is armed and isRunning is true. - Update streaming.test.tsx to assert bottom-follow during streaming and maintain no-follow when the user has scrolled up. Fixes NousResearch#42366
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Overview
This PR restores streaming auto-scroll functionality that was previously removed. The fix ensures that when a user is parked at the bottom of a chat during streaming, the viewport follows the growing content, but stops following when the user scrolls up.
Changes
- Restores ResizeObserver-based auto-follow in
thread-virtualizer.tsx - Adds new test case for user-scrolled-up scenario (auto-follow should not fire)
- Updates existing test to verify correct scroll behavior when parked at bottom
Quality
- Well-scoped fix (2 files changed)
- Good test coverage with edge case handling
- Clean implementation with proper cleanup of ResizeObserver
Looks Good
- Gated on
isRunningto avoid snapping after completion - Uses
pinToBottomwhich bails if already at bottom - No security concerns
- No performance concerns
Reviewed by Hermes Agent
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Restores streaming auto-follow: the viewport now pins to the bottom when a turn is running AND the user is parked at the bottom, via a ResizeObserver watching content growth. When the user scrolls up (disarming sticky-bottom), auto-follow stops. This is gated on isRunning so the viewport does not snap after completion.
- Clean reimplementation of streaming auto-follow behavior
- Two new test cases: follows when at bottom, does not follow when scrolled up
- No security concerns
Reviewed by Hermes Agent (cron batch)
@tonydwb Thanks for your approval. Could you approve the CI workflows so we can run all checks and merge this PR? |
|
@teknium1 — flagging this one for your call before it goes anywhere, since it directly reverses a decision you made. This PR re-adds the streaming auto-follow (the ResizeObserver re-pin loop in The driver is #42366, where a user reports the missing auto-scroll as a bug — so there's a real tension between your decision and that report. In fairness to the PR, it's not a naive revert: it disconnects the observer the moment Two open questions for you:
Separately: the PR title says "and keep composer visible," but the diff only touches auto-follow — the composer-disappearing half of #42366 isn't addressed here, so the title overclaims regardless of the decision above. (Note: the existing approvals on this PR are automated Hermes Agent cron-batch reviews, not a product sign-off.) |
|
|
Thanks for the work here, and for the careful, gated re-implementation — disconnecting the observer on That said, we're closing this. The no-follow behavior in #41414 was a deliberate product decision: the desktop chat viewport should stay exactly where the user left it once a turn is running, rather than chasing streaming output. We don't want to restore streaming auto-follow, even gated. #42366 will be addressed differently (e.g. a one-click jump-to-bottom affordance) rather than by re-pinning the viewport to the bottom during streaming. Appreciate the contribution — this was a clean PR, just pointed against a direction we've intentionally set. |
Obviously, more people need an auto-scrolling feature, which also aligns with the usage patterns of most software. For tasks that are automatically approved and executed, regardless of whether auto-scrolling is enabled, people won't be staring at the interface—they only care about the generated results. However, there are also many who want to follow the output without having to scroll back and forth. And since it's a desktop version, I think it should be designed in a way that conforms to human visual habits. |
+1 depending on the output of the task (sensitive operations/tightly guided prompts) auto-scroll is a requirement, at least as an option. Additionally, I believe it's useful for some of us to see the final output to plan the next input to the agent. |
fix #42366
Re-introduce a ResizeObserver-gated bottom-pin during active streaming so the chat viewport follows content growth while the user is parked at the bottom. The observer is disconnected when the run completes, so post-run layout shifts (e.g. Shiki re-highlight) do not yank the viewport back down.
What does this PR do?
Restores streaming auto-scroll in the desktop chat thread.
Problem: When the assistant is streaming a response and the user is already at the bottom of the thread, the viewport does not follow the growing content. The user has to manually scroll down to see newly generated tokens. This was reported in #42366.
Solution: Add a
ResizeObserveron the scroll content (not the viewport) insideVirtualizedThread. The observer callspinToBottom()only when:stickyBottomRefis armed (the user is parked at the bottom), andisRunningistrue(a turn is actively streaming).The observer is disconnected as soon as the run completes. This prevents post-run layout shifts—such as Shiki re-highlight or image load—from snapping the viewport back down after the user has already scrolled away.
pinToBottombails if the viewport is already at the bottom, andscrollToFnsuppresses the virtualizer's measurement-based adjustments while sticky, so the two scrolling systems do not fight each other.Related Issue
Fixes #42366
Type of Change
Testing
Updated
apps/desktop/src/components/assistant-ui/streaming.test.tsx:follows streaming content growth while parked at the bottomAsserts that when
scrollHeightincreases from 1,000 to 1,200 during streaming, the viewport scrolls to the new bottom (scrollTop === 960).does not follow streaming content growth when the user has scrolled upAsserts that if the user has intentionally scrolled up (disarming sticky-bottom), a subsequent content growth leaves the viewport at
420.Next Steps
ResizeObserverperformance impact in threads with very large message counts.