-
Notifications
You must be signed in to change notification settings - Fork 2.5k
fix: improve autoscroll behavior for thinking sections #7880
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add smart autoscroll detection for reasoning/thinking messages - Preserve user scroll position when manually scrolling up during thinking - Resume autoscroll when user scrolls back to bottom - Fixes issue where thinking content was unreadable due to constant scrolling Fixes #7879
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing my own code is like debugging in a mirror - everything looks backwards but somehow still broken.
| }, [modifiedMessages]) | ||
|
|
||
| // Enhanced scroll behavior for thinking sections | ||
| useEffect(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I notice this new effect duplicates the scroll logic from lines 1399-1409. Both call scrollToBottomSmooth() under similar conditions. Could we consolidate these to avoid redundancy? Maybe combine the logic into the existing effect or extract a shared handler?
| // If they have (disableAutoScrollRef.current is true), respect their choice | ||
| if (isStreamingThinking && !disableAutoScrollRef.current) { | ||
| // Continue auto-scrolling for thinking content if user hasn't intervened | ||
| scrollToBottomSmooth() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The scrollToBottomSmooth cleanup at lines 1351-1357 might not properly cancel all pending debounce calls when isStreamingThinking changes rapidly. Should we ensure the cleanup is more comprehensive to prevent potential memory leaks?
| }, []) | ||
|
|
||
| // Track if we're currently streaming a thinking/reasoning message | ||
| const isStreamingThinking = useMemo(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider moving this thinking detection logic closer to where isStreaming is computed (around line 516) for better code organization. This would group related streaming logic together.
| if (isAtBottom) { | ||
| // Re-enable autoscroll when user scrolls to bottom | ||
| // This is the key fix: users can scroll up to read thinking content, | ||
| // and autoscroll resumes when they scroll back down |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is excellent! Consider adding a similar explanatory comment near the isStreamingThinking logic to explain why we need separate handling for thinking sections versus regular streaming content.
|
I can't seem to reproduce the issue |
Summary
This PR addresses Issue #7879 by implementing smart autoscroll behavior for thinking/reasoning sections in the chat view.
Problem
When using models with thinking sections (like Qwen3 thinking models), the content automatically scrolls down as new text is generated, making it impossible for users to read the beginning of the thinking process.
Solution
Implemented a smart autoscroll system that:
Changes
isStreamingThinkingcomputed value to detect active thinking streamsatBottomStateChangecallback to properly re-enable autoscrollTesting
Review Confidence
The implementation received a 95% confidence score in code review with no security concerns identified.
Fixes #7879
Important
Improves autoscroll behavior for thinking sections in
ChatView.tsx, preserving user scroll position and resuming autoscroll appropriately.ChatView.tsx.isStreamingThinking.isStreamingThinkingto track thinking streams.useEffectto respect user scroll actions.atBottomStateChangeto re-enable autoscroll when at bottom.This description was created by
for c09cc58. You can customize this summary. It will automatically update as commits are pushed.