-
Notifications
You must be signed in to change notification settings - Fork 1
refactor: remove emit_interrupt_event from chat protocol [JAR-8666] #104
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -102,11 +102,7 @@ async def stream( | |
| resume_map: dict[str, Any] = {} | ||
|
|
||
| for trigger in api_triggers: | ||
| await self.chat_bridge.emit_interrupt_event(trigger) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line reacts to the interruption in the runtime code (be it tool call confirmation or another type of interrupt)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't have another interrupt type. We assumed there would be other interrupt use case with convo agents but there isn't
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What I'm saying is that you don't need to make any changes to the runtime contracts right now. You can simply leave this code as it is (change the CAS bridge implementation of I want us to be flexible, in case we change our mind again, or decide that we actually need an interrupt event (for coded agents there's nothing preventing the user from doing |
||
|
|
||
| resume_data = ( | ||
| await self.chat_bridge.wait_for_resume() | ||
| ) | ||
| resume_data = await self.chat_bridge.wait_for_tool_confirmation() | ||
|
|
||
| assert trigger.interrupt_id is not None, ( | ||
| "Trigger interrupt_id cannot be None" | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Uh oh!
There was an error while loading. Please reload this page.
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.
Don't remove this, we'll need it in the future. There are multiple types of interrupt, not just tool call confirmation. The runtime contract is fine even if you decide to handle tool call confirmations in another way
Uh oh!
There was an error while loading. Please reload this page.
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.
We decided to remove interrupts from conversational agents and only support tool confirmation because interrupts are too much a low-level abstraction and has no use case for convo agents other than tool confirmation
Here is the relevant thread but this is the general consensus amongst other conversational agent SDK as well