-
Notifications
You must be signed in to change notification settings - Fork 2.7k
fix: remove 500-message limit to prevent scrollbar jumping in long conversations #7064
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
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 the bugs are still mine.
| const recentMessages = modifiedMessages.slice(startIndex) | ||
|
|
||
| const newVisibleMessages = recentMessages.filter((message: ClineMessage) => { | ||
| // Remove the 500-message limit to prevent array index shifting |
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.
Is this intentional? PR #7053 appears to implement the exact same fix and was created earlier. Should we close one to avoid confusion, or is there a specific reason for having both PRs open?
|
|
||
| const newVisibleMessages = recentMessages.filter((message: ClineMessage) => { | ||
| // Remove the 500-message limit to prevent array index shifting | ||
| // Virtuoso is designed to efficiently handle large lists through virtualization |
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 enhancing this comment to explain why removing the limit fixes the scrollbar jumping:
This would help future developers understand the context better.
| const newVisibleMessages = recentMessages.filter((message: ClineMessage) => { | ||
| // Remove the 500-message limit to prevent array index shifting | ||
| // Virtuoso is designed to efficiently handle large lists through virtualization | ||
| const newVisibleMessages = modifiedMessages.filter((message: ClineMessage) => { |
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.
While Virtuoso handles virtualization efficiently, have we considered adding telemetry or monitoring for extremely long conversations (thousands of messages)? This could help us track memory usage in production and identify if we need additional optimizations in the future.
Perhaps we could log a metric when conversations exceed certain thresholds (e.g., 1000, 5000 messages)?
daniel-lxs
left a comment
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.
LGTM
Fixes #7063 and #7052
Summary
This PR fixes the scrollbar jumping issue that occurs in long conversations (>500 messages) by removing the array slicing that was causing index shifting.
Problem
When conversations exceeded 500 messages, the chat view would limit the visible messages to the last 500 by slicing the array. This caused:
Solution
Implemented Option 1 from the issue analysis: Remove the 500-message limit entirely and let Virtuoso handle virtualization of all messages. Virtuoso is designed to efficiently handle large lists through virtualization, so the memory impact is minimal.
Changes
webview-ui/src/components/chat/ChatView.tsxTesting
✅ All existing ChatView tests pass
✅ Linting passes
✅ Type checking passes
✅ The fix completely eliminates array index shifting
Impact
This fix ensures that users can follow along with streaming responses in long conversations without the view jumping around, significantly improving the user experience for extended chat sessions.
Related Issues
Important
Removes 500-message limit in
ChatView.tsxto fix scrollbar jumping in long conversations, allowing Virtuoso to handle all messages efficiently.ChatView.tsxto prevent scrollbar jumping in long conversations.visibleMessagesuseMemo inChatView.tsx.visibleMessagesto filtermodifiedMessageswithout slicing.ChatViewtests pass.This description was created by
for c88cb35. You can customize this summary. It will automatically update as commits are pushed.