-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Fix apply cancellation and make slash commands cancelable #5907
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
Your cubic subscription is currently inactive. Please reactivate your subscription to receive AI reviews and use cubic. |
✅ Deploy Preview for continuedev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
@RomneyDa thank you for all your amazing work on solving for being able to cancel various pieces of the experience. I closed my cancel button fix. Noting that my only remaining issue appears to be cancelling "thinking/reasoning" and ending up with unmatched messages in the history. |
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.
I know most of this is just passing around abortController / signal, but at least some part of this should get locked down with a test. otherwise just feels like setting up for whack-a-mole
Proposed one for the keyboard shortcut in the GUI
@@ -139,7 +139,7 @@ export function Chat() { | |||
!e.shiftKey | |||
) { | |||
dispatch(cancelStream()); |
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.
I think this would be a quick and high-value thing to test. There's already a setup for the Chat.tsx component in vitest I think, and would just want to spy on ideMessenger to make sure that it calls post("rejectDiff", ...).
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.
Added!
|
😱 Found 2 issues. Time to roll up your sleeves! 😱 |
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.
This is a great PR
Moving the inline method out of commands.ts and the test both
#5694 was intended to make apply cancelable but only worked for jetbrains because of how it was set up.
This PR fixes for VS Code
It also passes abort controllers to slash commands to make them cancelable as well.