Skip to content

feat(chat): Grouping read and list directory messages UX #7006

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

Merged
merged 8 commits into from
Apr 11, 2025

Conversation

laileni-aws
Copy link
Contributor

@laileni-aws laileni-aws commented Apr 10, 2025

Problem:

We identified and fixed an issue where the IDE's UI would prematurely stop displaying when executing certain commands. This occurred specifically when the Language Model (LLM) attempted to run the executeBash command before the Read/List Directory tools.

Example:

When a user asks "write a script that tells me whoami", the operation only requires the fsWrite command and not any directory reading tools. In such cases, the IDE would execute fsWrite but fail to display the executeBash tool in the chat interface. This UI behavior has been corrected in this PR, along with improvements to the Reading File user experience.

  • Major changes lies in this commit in ChatStream.ts
        // For FsRead and ListDirectory tools If messageIdToUpdate is undefined, we need to first create an empty message with messageId so it can be updated later
        if (isReadorList && !messageIdToUpdate) {
            this.messenger.sendInitialToolMessage(tabID, triggerID, toolUse?.toolUseId)
        } else {
            this.messenger.sendInitalStream(tabID, triggerID)
        }

  • Treat all work as PUBLIC. Private feature/x branches will not be squash-merged at release time.
  • Your code changes must meet the guidelines in CONTRIBUTING.md.
  • License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link

  • This pull request modifies code in src/* but no tests were added/updated.
    • Confirm whether tests should be added or ensure the PR description explains why tests are not required.
  • This pull request implements a feat or fix, so it must include a changelog entry (unless the fix is for an unreleased feature). Review the changelog guidelines.
    • Note: beta or "experiment" features that have active users should announce fixes in the changelog.
    • If this is not a feature or fix, use an appropriate type from the title guidelines. For example, telemetry-only changes should use the telemetry type.

- We show read tool message for each file read which is occupying the
entire chat with these messages which is not super important.
- Group all the read tool messaged under ContextList.

![image](https://github.com/user-attachments/assets/194e4ca1-7645-4419-8b9b-b5ac9a58279f)

---

- Treat all work as PUBLIC. Private `feature/x` branches will not be
squash-merged at release time.
- Your code changes must meet the guidelines in
[CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines).
- License: I confirm that my contribution is made under the terms of the
Apache 2.0 license.
@laileni-aws laileni-aws marked this pull request as ready for review April 10, 2025 22:28
@laileni-aws laileni-aws requested a review from a team as a code owner April 10, 2025 22:28
@laileni-aws laileni-aws requested a review from jguoamz April 10, 2025 23:45
@Hweinstock
Copy link
Contributor

Hweinstock commented Apr 11, 2025

What is the regression that this fixes? Didn't see it mentioned in the revert PR so missing some context here. How does creating an empty message fix this?

@laileni-aws
Copy link
Contributor Author

What is the regression that this fixes? Didn't see it mentioned in the revert PR so missing some context here. How does creating an empty message fix this?

Problem:

We identified and fixed an issue where the IDE's UI would prematurely stop displaying when executing certain commands. This occurred specifically when the Language Model (LLM) attempted to run the executeBash command before the Read/List Directory tools.

Example:

When a user asks "write a script that tells me whoami", the operation only requires the fsWrite command and not any directory reading tools. In such cases, the IDE would execute fsWrite but fail to display the executeBash tool in the chat interface. This UI behavior has been corrected in this PR, along with improvements to the Reading File user experience.

@laileni-aws
Copy link
Contributor Author

/retryBuilds

Copy link
Contributor

@Hweinstock Hweinstock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding the explanation, thats super helpful!

It seems to me that the heavy conditional logic based on which tool is being used feels like the real problem here as it makes the expected behavior unclear and hard to follow.

However, this is a broader issue we can address in migration to Flare.

@Hweinstock Hweinstock merged commit b222e0b into aws:feature/agentic-chat Apr 11, 2025
34 of 37 checks passed
jpinkney-aws pushed a commit to aws/language-servers that referenced this pull request Apr 23, 2025
## 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.
![image](https://github.com/user-attachments/assets/08fff204-0bbc-4467-b05b-7f90be062fc1)

## 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.
opieter-aws pushed a commit to opieter-aws/language-servers that referenced this pull request Apr 23, 2025
## 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.
![image](https://github.com/user-attachments/assets/08fff204-0bbc-4467-b05b-7f90be062fc1)

## 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.
opieter-aws added a commit to aws/language-servers that referenced this pull request Apr 23, 2025
* 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.
![image](https://github.com/user-attachments/assets/08fff204-0bbc-4467-b05b-7f90be062fc1)

## 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants