-
Notifications
You must be signed in to change notification settings - Fork 2.5k
feat: implement incremental message updates for improved UI performance #6522
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.
Pull Request Overview
This PR implements incremental message updates to improve UI performance by sending only new or updated messages to the webview instead of the entire application state. The solution reduces data transfer and minimizes UI re-renders, especially beneficial for tasks with large message histories.
Key changes include:
- Modified message handling to send individual messages instead of full state updates
- Added new message event types (
messageCreatedandmessageUpdated) for granular updates - Enhanced webview state management to process incremental updates
- Implemented special handling for loading existing tasks with gradual message rendering
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| ExtensionStateContext.tsx | Added handler for messageCreated events to append new messages to state |
| ExtensionMessage.ts | Added messageCreated message type to support incremental updates |
| ClineProvider.ts | Enhanced postStateToWebview to send messages incrementally for existing tasks |
| Task.ts | Modified message methods to send individual message updates instead of full state |
src/core/task/Task.ts
Outdated
|
|
||
| await abortStream(cancelReason, streamingFailedMessage) | ||
|
|
||
| const provider = this.providerRef.deref() |
Copilot
AI
Jul 31, 2025
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 provider is retrieved multiple times in this method (lines 1655, 1666). Consider extracting it once at the beginning of the method to avoid duplication.
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.
Thank you for your contribution! I've reviewed the changes and found some issues that need attention. The implementation successfully reduces data transfer and improves UI performance, but there are a few areas that could be improved for robustness and maintainability.
Review Findings:
Critical Issues:
- Magic number in delay (ClineProvider.ts:1349) - The hardcoded 10ms delay should be extracted to a named constant with documentation
- Duplicate provider retrieval (Task.ts:1655,1666) - The provider is retrieved twice in the same method
Important Suggestions:
3. Performance concern - The 10ms delay per message could cause noticeable delays for tasks with many messages (100 messages = 1 second)
4. Missing error handling - The postStateToWebview method doesn't handle potential errors when sending messages incrementally
5. Type safety issue - The clineMessage property in ExtensionMessage is optional but used without null checks
Minor Improvements:
6. Code duplication - Similar logic for sending individual messages is repeated in multiple places in Task.ts
7. Missing tests - No unit tests mentioned for the new incremental loading functionality
Note: One of Copilot's comments about a missing delay import is incorrect - the import exists on line 7 of ClineProvider.ts.
hannesrudolph
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.
Thank you for your contribution! I've reviewed the changes and found that the implementation successfully improves UI performance through incremental message updates. The critical issues from the previous review have been addressed.
Summary
This PR effectively reduces data transfer and minimizes UI re-renders by sending only new or updated messages instead of the entire state. The implementation is solid with proper error handling.
Resolved Issues from Previous Review
✅ Magic number in delay - now uses constant
✅ Type safety issue - code now uses non-null assertions appropriately
✅ Missing error handling - now has try-catch with logging
I've left some suggestions inline for further improvements, primarily around performance optimization for very large message histories and code organization.
src/core/task/Task.ts
Outdated
| // Send only the updated API request message instead of the entire state | ||
| if (provider && lastApiReqIndex >= 0) { | ||
| try { | ||
| await provider.postMessageToWebview({ |
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 pattern of updating and sending messages is repeated in multiple places. Could we extract it into a helper method to reduce duplication?
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
|
@roomote-agent Use switch_mode to switch to pr-fixer mode and solve the issues on this PR. Do not create a new one. |
|
Hi @daniel-lxs! I see your request and I'm now working on fixing the issues with this PR. I've identified that the PR has merge conflicts that need to be resolved. I'll resolve these conflicts and ensure all tests pass before pushing the changes. I'll update you once the fixes are complete. |
- Modified Task.ts to send only new messages instead of entire state - Added messageCreated and messageUpdated event types to ExtensionMessage - Updated webview handler to process incremental message updates - Enhanced postStateToWebview to handle existing tasks with incremental loading - Improved performance by reducing data transfer and UI re-renders This change significantly improves UI responsiveness, especially for tasks with many messages.
…ic 10ms with INCREMENTAL_SEND_DELAY_MS; guard Task message send calls with non-fatal errors
…or postMessageToWebview calls
6aa4728 to
b5b5b8e
Compare
|
✅ @daniel-lxs PR #6522 has been successfully fixed! Summary of fixes applied:Merge Conflicts ResolvedThe PR had merge conflicts in
CI StatusAll CI checks are now passing:
The PR is now ready for merge with all conflicts resolved and tests passing. |
Fixes #6513
Summary
This PR implements incremental message updates to significantly improve UI performance, especially for tasks with many messages.
Problem
Previously, the entire application state (including all messages) was sent to the webview on every message creation or update. This caused performance issues with large message histories, as the UI had to re-render all messages each time.
Solution
messageCreatedandmessageUpdatedChanges
addToClineMessagesandupdateClineMessageto send only the affected messagepostStateToWebviewto handle existing tasks with incremental loadingBenefits
Testing
This change provides a significant performance improvement for the UI without changing the user experience.
Important
Implement incremental message updates in
Task.tsfor improved UI performance by reducing data transfer and minimizing re-renders.Task.tsto send only new or updated messages to the webview.messageCreatedandmessageUpdatedadded inExtensionMessage.ts.ExtensionStateContext.tsxupdated to handle incremental message updates.This description was created by
for b5b5b8e. You can customize this summary. It will automatically update as commits are pushed.