fix: ux polish for list directory tool messages. #1075
Conversation
server/aws-lsp-codewhisperer/src/language-server/agenticChat/agenticChatResultStream.ts
Outdated
Show resolved
Hide resolved
laileni-aws
force-pushed
the
flare
branch
2 times, most recently
from
c124de2 to
78c275a
Compare
| messageId = chatResultStream.getMessageIdToUpdate()! | ||
| } else if (messageId) { | ||
| chatResultStream.setMessageIdToUpdate(messageId) | ||
| let messageIdToUpdate = toolUse.toolUseId! |
There was a problem hiding this comment.
is this always going to be defined? what happens if toolUseId isn't?
There was a problem hiding this comment.
We come to this part of code processReadOrList once toolName and toolId is available!
We has existing check here So don't want to duplicate by adding one more check.
There was a problem hiding this comment.
Ah so really the ToolUse type should really be something like Required<ToolUse> as an input to this function since the optional fields are actually defined? Not a blocker, but worth a consideration
| } else if (messageId) { | ||
| chatResultStream.setMessageIdToUpdate(messageId) | ||
| let messageIdToUpdate = toolUse.toolUseId! | ||
| const currentId = chatResultStream.getMessageIdToUpdateForTool(toolUse.name!) |
There was a problem hiding this comment.
will toolUse name always be defined? what happens if name isn't?
| * @param filePaths Array of FileDetailsWithPath involved in the operation | ||
| */ | ||
| addMessageOperation(messageId: string, type: OperationType, filePaths: FileDetailsWithPath[]) { | ||
| this.#state.messageOperations.set(messageId, { type, filePaths }) | ||
| addMessageOperation(messageId: string, type: string, filePaths: FileDetailsWithPath[]) { |
There was a problem hiding this comment.
can we make a type here instead of converting to string? then you wouldn't need to cast to an operation type on the next line
There was a problem hiding this comment.
I tried this approach,
OperationType can not be imported to agenticChatController.ts so explicitly mentioning here!
| messageId = chatResultStream.getMessageIdToUpdate()! | ||
| } else if (messageId) { | ||
| chatResultStream.setMessageIdToUpdate(messageId) | ||
| let messageIdToUpdate = toolUse.toolUseId! |
There was a problem hiding this comment.
Ah so really the ToolUse type should really be something like Required<ToolUse> as an input to this function since the optional fields are actually defined? Not a blocker, but worth a consideration
## Problem - Existing UX shows multiple context list chatItem cards for list directory tool messages. ## Solution - Use a `messageIdToUpdateListDir` state to store the messageId and update this for next list directory tool messages.  ## TODO: - Need to combine both read and list directory tool messages into one context list. - This code change is equivalent to this PR: aws/aws-toolkit-vscode#7006 in `feature/agentic-chat` branch.
* fix: add result attribute when emitting telemetry event * fix: remove comment * fix: unit test * fix: fix unit test * fix: fix unit test * feat(chat-client): open use input prompt for agentic chat and new prompt should st… (#1081) feat: open use input prompt for agentic chat and new prompt should stop current response * fix: ux polish for list directory tool messages. (#1075) ## Problem - Existing UX shows multiple context list chatItem cards for list directory tool messages. ## Solution - Use a `messageIdToUpdateListDir` state to store the messageId and update this for next list directory tool messages.  ## TODO: - Need to combine both read and list directory tool messages into one context list. - This code change is equivalent to this PR: aws/aws-toolkit-vscode#7006 in `feature/agentic-chat` branch. * fix: execute command should show when no approval required & add more loading (#1091) * fix: fix unit test --------- Co-authored-by: invictus <[email protected]> Co-authored-by: Laxman Reddy <[email protected]> Co-authored-by: Zoe Lin <[email protected]>
Problem
Solution
messageIdToUpdateListDirstate to store the messageId and update this for next list directory tool messages.TODO:
feature/agentic-chatbranch.License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.