feat: Show checkpoint button on all subsequent messages #6273
Conversation
…eating new checkpoints - Modified ChatView to track the last checkpoint info - Updated ChatRow to display checkpoint UI on all messages after a checkpoint - Created ChatRowWithCheckpoint wrapper component to handle checkpoint display logic - Removed rendering of checkpoint_saved messages themselves This change improves the UI by showing a persistent checkpoint button on all messages after a checkpoint is created, rather than creating new checkpoint messages after each interaction.
dosubot
bot
added
size:M
hannesrudolph
added
the
Issue/PR - Triage
hannesrudolph
added
PR - Needs Preliminary Review
and removed
Issue/PR - Triage
| onBatchFileResponse, | ||
| isFollowUpAnswered, | ||
| editable, | ||
| lastCheckpointInfo: _lastCheckpointInfo, |
There was a problem hiding this comment.
Is this intentional? The parameter is prefixed with underscore indicating it's unused, but it's still part of the component's props interface. Since explicitly passes for this parameter, could we remove it entirely from to avoid confusion?
| } | ||
|
|
||
| // Default return for messages that don't match any case | ||
| return null |
There was a problem hiding this comment.
This return statement appears to be unreachable. The switch statement above should handle all cases, and if it doesn't, TypeScript would typically catch this. Could we either remove this unreachable code or add a proper default case to the switch statement if there are actually unhandled message types?
| }, [messages]) | ||
|
|
||
| // Track the last checkpoint message | ||
| const lastCheckpointInfo = useMemo(() => { |
There was a problem hiding this comment.
Performance consideration: This recalculates on every render when any message changes. Since checkpoint messages are relatively rare, could we optimize this by only recalculating when messages with are added? Perhaps we could track the last checkpoint info in a ref and only update it when needed?
|
@roomote-agent Address the review I just left, commit to this PR, do not create a new one |
daniel-lxs
moved this from PR [Needs Prelim Review]
to PR [Changes Requested]
in Roo Code Roadmap
|
👋 I've seen your review feedback and will address the following issues:
Working on these changes now and will commit to this PR branch. |
hannesrudolph
added
PR - Changes Requested
and removed
PR - Needs Preliminary Review
labels
- Remove unused lastCheckpointInfo parameter from ChatRowContent - Remove unreachable return null statement - Optimize useMemo performance for lastCheckpointInfo calculation
|
✅ Review feedback addressed I've successfully addressed all the issues you raised in your review: Changes Made
Testing
The changes maintain the same functionality while addressing the code quality concerns you identified. |
github-project-automation
bot
moved this from PR [Changes Requested]
to Done
in Roo Code Roadmap
This PR addresses the feedback from @mrubens on PR #5996.
Summary
Instead of creating a new checkpoint after each message, this change shows the same checkpoint button on all subsequent messages from the point the checkpoint was created. This is a UI-only change as requested.
Changes
ChatViewto track the last checkpoint infoChatRowto display checkpoint UI on all messages after a checkpointChatRowWithCheckpointwrapper component to handle checkpoint display logiccheckpoint_savedmessages themselvesBenefits
Fixes the issue raised in #5996 (comment)
Important
UI updated to show checkpoint button on all messages after a checkpoint, using
ChatRowWithCheckpointcomponent.ChatRownow usesChatRowWithCheckpointto display checkpoint UI on all messages after a checkpoint.ChatRowContentno longer renderscheckpoint_savedmessages.ChatRowWithCheckpointcomponent handles logic for displaying checkpoint UI.ChatViewtrackslastCheckpointInfousinguseMemoto optimize recalculation.lastCheckpointInfopassed toChatRowfor rendering checkpoint UI.ChatRowWithCheckpointintroduced to wrapChatRowContentand manage checkpoint display logic.This description was created by
for ee6f074. You can customize this summary. It will automatically update as commits are pushed.