Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Improve agentic execution UI #999

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Improve agentic execution UI #999

wants to merge 1 commit into from

Conversation

GoodluckH
Copy link
Collaborator

See #998 for motivation

V1:

Screen.Recording.2025-03-28.at.3.27.23.PM.mov

@GoodluckH GoodluckH requested review from mr-lee and mschrage March 28, 2025 22:32
@GoodluckH GoodluckH requested a review from a team as a code owner March 28, 2025 22:32
@GoodluckH GoodluckH linked an issue Mar 28, 2025 that may be closed by this pull request
[UX] Improve the agentic execution UI for clarity #998
Open
@codecov-commenter
Copy link

codecov-commenter commented Mar 28, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 58.33333% with 10 lines in your changes missing coverage. Please review.

Project coverage is 13.82%. Comparing base (f4b1edd) to head (25b3062).

Files with missing lines Patch % Lines
crates/q_cli/src/cli/chat/mod.rs 55.55% 0 Missing and 4 partials ⚠️
crates/q_cli/src/cli/chat/tools/mod.rs 20.00% 4 Missing ⚠️
crates/q_cli/src/cli/chat/tools/fs_read.rs 80.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #999      +/-   ##
==========================================
- Coverage   13.83%   13.82%   -0.01%     
==========================================
  Files        2361     2361              
  Lines      204862   204847      -15     
  Branches   185226   185211      -15     
==========================================
- Hits        28339    28325      -14     
+ Misses     175027   175022       -5     
- Partials     1496     1500       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@GoodluckH GoodluckH mentioned this pull request Mar 29, 2025
Open
dingfeli
dingfeli reviewed Mar 31, 2025
View reviewed changes
crates/q_cli/src/cli/chat/mod.rs Outdated
style::Print(format!("{}\n", "▔".repeat(terminal_width))),
style::SetForegroundColor(Color::Reset),
style::SetForegroundColor(Color::Blue),
style::Print(format!("🧰 using tool: {}\n", tool.display_name())),
Copy link
Contributor

Choose a reason for hiding this comment

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

I am little hesitant to put emojis in because some of them behave drastically differently depending on which terminal you are on.
Have you tried testing to see how they look with different terminals? Some candidates are iTerm, ghostty, wez, and the native mac terminal.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good thoughts! We're actually already using emojis (see the "Completed in s" text). And I tested on my Mac terminal, iTerm and Wez (see screenshots).

They're fine displaying. Only Wez is showing some variations but not really breaking.

I will change the emoji to 🛠️ tho since ppl don't really like the current one

Screenshot 2025-04-01 at 8 27 28 AM Screenshot 2025-04-01 at 8 29 16 AM Screenshot 2025-04-01 at 8 30 27 AM

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh great! Thanks for confirming. Looks great.
This is a big visual change though. I want to run this by the team before we merge this.

@GoodluckH GoodluckH force-pushed the 998-ux-improve-the-agentic-execution-ui-for-clarity branch from 0640af1 to 2335c09 Compare April 1, 2025 16:06
@GoodluckH GoodluckH requested a review from dingfeli April 1, 2025 16:06
@GoodluckH GoodluckH force-pushed the 998-ux-improve-the-agentic-execution-ui-for-clarity branch from 2335c09 to 25b3062 Compare April 1, 2025 16:11
hayemaxi added a commit to hayemaxi/amazon-q-developer-cli that referenced this pull request Apr 2, 2025
@hayemaxi
feat(commands): granular permissions with /tools
2dee775 
Implements aws#921 which describes trusting and untrusting specific tools for the current session.

- Start with default permissions. Users can change them from command line or within chat via /tools
- `/acceptall` and `--accept-all` were deprecated in favor of `--trust-all-tools` and `/tools trustall`. They will continue to work, but display a notice and activation functionality of the new commands.
  - Command::AcceptAll no longer exists
- UI has been reword a little, see screenshots.
- `report_issue` will include trust override settings in the report.
- Chat supports multiple tool use requests from Q at once. I have refactored the tool flow to allow asking permissions for individual tool requests at a time
  - Now, ExecuteTools (checks for acceptance one tool at a time) -> PromptUser (ask for acceptance) -> HandleInput (handle acceptance) -> ExecuteTools (find next tool that needs acceptance OR trigger execution if none remaining).
  - I was not able to find a prompt that made Q send multiple tool_uses in a single request coming from Q. It seems to do them iteratively even when asked to do them in parallel.
  - In any case, that original logic to support parallel requests is preserved with this new structure.

Risks: May conflict with aws#999
hayemaxi added a commit to hayemaxi/amazon-q-developer-cli that referenced this pull request Apr 2, 2025
@hayemaxi
feat(commands): granular permissions with /tools
bc196a7 
Implements aws#921 which describes trusting and untrusting specific tools for the current session.

- Start with default permissions. Users can change them from command line or within chat via /tools
- `/acceptall` and `--accept-all` were deprecated in favor of `--trust-all-tools` and `/tools trustall`. They will continue to work, but display a notice and activation functionality of the new commands.
  - Command::AcceptAll no longer exists
- UI has been reword a little, see screenshots.
- `report_issue` will include trust override settings in the report.
- Chat supports multiple tool use requests from Q at once. I have refactored the tool flow to allow asking permissions for individual tool requests at a time
  - Now, ExecuteTools (checks for acceptance one tool at a time) -> PromptUser (ask for acceptance) -> HandleInput (handle acceptance) -> ExecuteTools (find next tool that needs acceptance OR trigger execution if none remaining).
  - I was not able to find a prompt that made Q send multiple tool_uses in a single request coming from Q. It seems to do them iteratively even when asked to do them in parallel.
  - In any case, that original logic to support parallel requests is preserved with this new structure.

Risks: May conflict with aws#999
@hayemaxi hayemaxi mentioned this pull request Apr 2, 2025
Open
hayemaxi added a commit to hayemaxi/amazon-q-developer-cli that referenced this pull request Apr 2, 2025
@hayemaxi
feat(commands): granular permissions with /tools
0ba142d 
Implements aws#921 which describes trusting and untrusting specific tools for the current session.

- Start with default permissions. Users can change them from command line or within chat via /tools
- `/acceptall` and `--accept-all` were deprecated in favor of `--trust-all-tools` and `/tools trustall`. They will continue to work, but display a notice and activation functionality of the new commands.
  - Command::AcceptAll no longer exists
- UI has been reword a little, see screenshots.
- `report_issue` will include trust override settings in the report.
- Chat supports multiple tool use requests from Q at once. I have refactored the tool flow to allow asking permissions for individual tool requests at a time
  - Now, ExecuteTools (checks for acceptance one tool at a time) -> PromptUser (ask for acceptance) -> HandleInput (handle acceptance) -> ExecuteTools (find next tool that needs acceptance OR trigger execution if none remaining).
  - I was not able to find a prompt that made Q send multiple tool_uses in a single request coming from Q. It seems to do them iteratively even when asked to do them in parallel.
  - In any case, that original logic to support parallel requests is preserved with this new structure.

Risks: May conflict with aws#999
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mr-lee mr-lee Awaiting requested review from mr-lee

@mschrage mschrage Awaiting requested review from mschrage

@dingfeli dingfeli Awaiting requested review from dingfeli

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[UX] Improve the agentic execution UI for clarity
3 participants
@GoodluckH @codecov-commenter @dingfeli