-
Couldn't load subscription status.
- Fork 0
feat: purpose interface change to bidi #11
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
base: main
Are you sure you want to change the base?
Conversation
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| class BidirectionalModelSession(abc.ABC): |
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 we should move away from the name Session as to not confuse with our SessionManager. Suggest "Connection"
| """ | ||
|
|
||
| @abc.abstractmethod | ||
| async def receive_events(self) -> AsyncIterable[BidirectionalStreamEvent]: |
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.
nit: can this is send() and recieve? similar to the agent methods
| metadata: Optional[Dict[str, Any]] | ||
|
|
||
|
|
||
| class ToolResultInputEvent(TypedDict): |
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 we can use the existing ToolResult?
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.
remove this class.
| pass | ||
|
|
||
| @abc.abstractmethod | ||
| def _format_tools_for_provider(self, tool_specs: list[ToolSpec]) -> Any: |
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.
remove private function
| async def send_events(self, content: Union[str, ImageInputEvent, AudioInputEvent, ToolResultInputEvent]) -> None: | ||
| """Send structured content (text, images,audio tool results) to the model. | ||
| Args: |
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.
One other thing: Realtime Event| other event.
Description
This PR refactors the bidirectional streaming interface to provide a cleaner, more generic API that aligns with industry patterns (Google Gemini, OpenAI) while maintaining strong typing and provider flexibility.
This PR should also kickoff the bar-raise dicsussion. We should agree on the interface before making any code changes to model providers.
What have changed?
1. Unified Send Interface
Before:
After:
Need to discuss: if we want to sperate static(image, text, etc) content vs realtime(audio for now, maybe video in future) content?
2. New Typed Events
2.1 Added
ToolResultInputEventWe should discuss: We already have
ToolResultclass in SDK, why we need this? What is the diffrenece.2.2 Enhanced
ImageInputEventSupports OpenAI/Gemini realtime API patterns:
Let' discuss the interface from below dimensions:
sendfunctions3. Model/Session Separation Clarified
Model (Stateless):
Session (Stateful):
4. Interface Simplification
_format_tools_for_provider)send_events()Breaking change
Method signatures changed from specific to generic
send_text_content()→send_events()send_audio_content()→send_events()send_image_content()→send_events()send_tool_result()→send_events()Related Issues
Documentation PR
Type of Change
Bug fix
New feature
Breaking change
Documentation update
Other (please describe):
Testing
How have you tested the change? Verify that the changes do not break functionality or introduce warnings in consuming repositories: agents-docs, agents-tools, agents-cli
hatch run prepareChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.