-
Notifications
You must be signed in to change notification settings - Fork 294
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
refactor: commands #2869
refactor: commands #2869
Conversation
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.
Took a high level look over the code and it's looking really good. Left some minor comments and a few possible bugs I noticed
@umpox Thanks for the thorough review!
That's intended as part of removing slash commands from chat. This allows user to see the underlaying prompt and tweak it via the new edit buttons as right now, we are disabling the edit buttons on command messages due to this issue.
Yea that's the current behavior! I do think we can give this menu a makeover in a follow up, but i've updated the menu so that it runs edit command by default instead of ask :D |
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.
Thanks! and nice tests 👍
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.
@abeatrix the very large addition to the recording file (+15k lines) hints to me that we're adding a lot of unused recordings. The pnpm update-agent-recordings
command is supposed to clean up unused recordings but I'm sensing it's not working 100% correctly yet.
You can run this command to reset the commands from origin/main
./agent/scripts/reset-recordings-to-main.sh
I don't see any new LLM requests in the updated tests that could explain why we need 15k new lines to the recording file.
FYI I figured out the bug with |
Refactor and clean up the existing commands set up, update and add new e2e tests to cover commands and custom commands.
Note: A lot of the files were updated due to attempts to clean up and unify method names, commands, and file names.
For example:
The major change in this PR is moving
command
to the top level and decouple it from chat and edit. This allows other services to execute a command anywhere through:CommandsController
class.CommandsController
CommandRunner
CommandRunner
handles identifying if the command is achat request
or anedit request
, then fetch for the relevant context before sending creating the request via executeChat for chat request and executeEdit for edit request respectively.executeCodyCommand
.Summary:
vscode/src/commands/context
vscode/src/commands/default
Checklist:
addEnhancedContext
incody.action.chat
commandTest plan
Try executing