-
Notifications
You must be signed in to change notification settings - Fork 740
feat: allow editing messages in AI chat #6117
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 editMessage() method to AIChatManager for editing user messages - Add edit button UI that appears on hover for all user messages - Transform message display into textarea when editing - Remove subsequent messages and resend on Enter - Add keyboard shortcuts: Enter to save, Escape to cancel - Maintain existing restart generation functionality Co-authored-by: Ruben Fiszel <[email protected]>
Deploying windmill with
|
Latest commit: |
117ed4f
|
Status: | ✅ Deploy successful! |
Preview URL: | https://ef9bb07e.windmill.pages.dev |
Branch Preview URL: | https://claude-issue-6116-20250702-1.windmill.pages.dev |
e76ed63
to
3706c34
Compare
3706c34
to
2b5685c
Compare
d56c8c5
to
7ca4169
Compare
/ai review this PR |
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
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.
Important
Looks good to me! 👍
Reviewed b1d98fe in 1 minute and 33 seconds. Click for details.
- Reviewed
181
lines of code in3
files - Skipped
0
files when reviewing. - Skipped posting
4
draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. frontend/src/lib/components/copilot/chat/AIChatDisplay.svelte:53
- Draft comment:
Shared editing state (editingMessageIndex) is set up clearly. Verify that allowing only one message to be edited at a time fits the intended UX. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
2. frontend/src/lib/components/copilot/chat/AIChatInput.svelte:44
- Draft comment:
The clickOutside directive correctly cancels edit mode via onEditEnd. Consider extracting this utility to a shared helper if it’s reused elsewhere. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
3. frontend/src/lib/components/copilot/chat/AIChatMessage.svelte:28
- Draft comment:
Consider renaming the function 'editMessage' to 'startEditMessage' for consistency with the PR description and clarity of purpose. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
4. frontend/src/lib/components/copilot/chat/AIChatMessage.svelte:42
- Draft comment:
For improved accessibility, add aria-labels or appropriate roles on the clickable message container that triggers editing. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_1QgkHoQ7ob5xJYQI
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
/ai review this PR |
Claude finished @centdix's task —— View job Code Review Complete ✅I've thoroughly analyzed the AI chat message editing implementation. Overall, this is a well-structured feature with good component separation and proper use of Svelte 5 patterns. Here's my detailed assessment: ✅ StrengthsComponent Architecture
Implementation Quality
|
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.
Important
Looks good to me! 👍
Reviewed 117ed4f in 2 minutes and 9 seconds. Click for details.
- Reviewed
36
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
3
draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts:219
- Draft comment:
retryRequest now requires a messageIndex and throws an error if no valid user message is found. Consider handling this case more gracefully rather than throwing an error, and ensure that all call sites pass a valid index. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment addresses a real change in the code - the function now takes an index parameter and throws an error in invalid cases. However, throwing an error here is likely intentional and appropriate since: 1. This represents a programming error (invalid index passed) 2. The error message is clear and descriptive 3. The calling code should validate indices before calling this method 4. Graceful handling would mask bugs in the calling code Perhaps throwing an error is too harsh and could cause UI issues if not caught properly. Maybe returning false or undefined would be more idiomatic for a UI component. No, throwing an error is appropriate here because: 1. It clearly indicates a programming bug that should be fixed 2. The error has a descriptive message to help debugging 3. Silent failures would make bugs harder to track down The comment should be deleted. Throwing an error is appropriate for this programming error case and handling it "more gracefully" would actually make the code worse by masking bugs.
2. frontend/src/lib/components/copilot/chat/AIChatMessage.svelte:45
- Draft comment:
The onkeydown handler is empty. For better accessibility, implement key handling (e.g. Enter/Space to edit) or remove the handler to avoid confusion. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. frontend/src/lib/components/copilot/chat/AIChatMessage.svelte:28
- Draft comment:
The editMessage function is triggered by clicking the entire message container. Consider adding a dedicated edit icon/button to reduce accidental edits and clarify the action to users. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
Workflow ID: wflow_jYEjb5FeGCvKpITg
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Summary
Changes Made
AIChatManager.svelte.ts
editMessage()
method that:AIChatDisplay.svelte
editingMessageIndex
,editingMessageContent
)startEditMessage()
,cancelEditMessage()
,saveEditMessage()
)Implementation Details
Test Plan
npm run check
) passesCloses #6116
Generated with Claude Code
Co-authored-by: Ruben Fiszel [email protected]
Important
This PR adds functionality to edit user messages in the AI chat interface, including UI updates and message handling logic.
AIChatManager.svelte.ts
: AddeditMessage()
method to update message content, remove subsequent messages, and send new request.AIChatDisplay.svelte
: Add state management for editing and edit functionality functions. Modify message display logic to show textarea when editing.AIChatInput.svelte
: Handle input focus, context selection, and request sending during editing.AIChatMessage.svelte
to handle individual message rendering and editing logic.ContextTextarea.svelte
to support context selection and input handling during editing.shared.ts
to includeerror
property inUserDisplayMessage
.This description was created by
for 117ed4f. You can customize this summary. It will automatically update as commits are pushed.