-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Make slash command output streamable #19632
Conversation
The protocol requires that certain elements can't be null but instead must be skipped. We correctly skip these now.
JSON RPC notification do not have to contain an id. We do not generate or use id's to ensure we don't rely on them being there.
Unlike normal commands which usually always have autocompletion, context server commands often don't. This makes it hard to understand what can be passed, particularly as commands change often and depending on config. We hence show the name of the argument now as a label (arguable all commands could benefit from that).
This commit replaces the existing SlashCommandOutput with a stream based approach. This allows for slash commands to (1) stream results if needed (2) produce multi-turn messages, e.g. asssistant or system messages (3) will allow for intermediate progress updates during slash command execution. The commit does the following * Introduce `SlashOutputEvent` as events that slash commands can emit. * Introduce `SlashCommandResult` as an alias to a boxed stream of events. * Move all commands to the new command result format.
We are updating the way we deal with commands in output text. Since commands can now emit content events one at a time, each event can determine if it should be run. To support this, we now grab a range and run only commands in the ranges of the content that specified to do so.
When a slash command issues multiple `StartMessage` events with the same role we merge them if merge_same_roles is set to true.
…rom slash commands
Co-Authored-By: David <[email protected]>
Co-Authored-By: David <[email protected]>
Co-Authored-By: Marshall <[email protected]>
Co-authored-by: Antonio <[email protected]>
Co-authored-by: Max <[email protected]> Co-authored-by: Antonio <[email protected]>
Co-authored-by: Max <[email protected]>
701df6b
to
5c10a5a
Compare
Co-Authored-By: Max <[email protected]> Co-Authored-By: Will <[email protected]>
…nding slash commands
…d is running Co-authored-by: Will <[email protected]>
…holder type tag Co-authored-by: Will <[email protected]>
912d388
to
d4eac88
Compare
} | ||
|
||
reserved 4; |
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.
The thought here is that we aren't bothering to make this change fully backward-compatible; if new versions of Zed collaborate with earlier versions of Zed in the assistant panel, the slash commands will just not fully replicate.
We are making sure to prevent decoding errors or type confusion by using new variant numbers for the new slash command messages.
This PR adds support for streaming output from slash commands
In this PR we are focused primarily on the interface of the
SlashCommand
trait to support streaming the output. We will follow up later with support for extensions and context servers to take advantage of the streaming nature.Release Notes: