-
Notifications
You must be signed in to change notification settings - Fork 73
feat: send chat update on undo #1068
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
3748934
to
032ffb3
Compare
ci should pass once #1069 is merged |
@@ -171,6 +171,26 @@ export class AgenticChatController implements ChatHandlers { | |||
const toolUseId = params.messageId | |||
try { | |||
await this.#undoFileChange(toolUseId) | |||
const cachedToolUse = this.#triggerContext.getToolUseLookup().get(toolUseId) | |||
if (cachedToolUse) { | |||
this.#features.chat.sendChatUpdate({ |
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.
What exactly does sendChatUpdate do? Is it force pushing this chat message to the client? Is there a reason we can't do this through the chatResultStream interface?
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.
My understanding is this is used for updating an existing message with messageId. I guess since this isn't a progress message we shouldn't use chatResultStream? Not sure why one is under this.#features.chat
and the other is under this.#features.lsp
though
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 partial progress is a generic LSP feature and works for every/most request type, until the response has been sent. After the request is closed, so is the partial progress stream.
ChatUpdate can also be sent after the response has been sent.
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.
Approving to unblock but worth checking with @kmile if using chat directly is right approach going forward
@ctlai95 @Hweinstock Yup looks good to me! |
Problem
Card should be updated after undo is clicked
Solution
Implement the chat update mechanism on button click
Refer to https://github.com/aws/aws-toolkit-vscode/blob/feature/agentic-chat/packages/core/src/amazonq/webview/ui/apps/cwChatConnector.ts#L394-L404 for previous implementation.
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.