[DO NOT MERGE] Change where tool request content event is sent #60
Conversation
| tool_request_content = ToolRequestContent( | ||
| author="agent", | ||
| tool_call_id=tool_call_item.call_id, | ||
| name=tool_call_item.name, | ||
| arguments=json.loads(tool_call_item.arguments), | ||
| ) | ||
|
|
||
| # Create tool request using streaming context (immediate completion) | ||
| async with ( | ||
| self.streaming_service.streaming_task_message_context( | ||
| task_id=task_id, | ||
| initial_content=tool_request_content, | ||
| ) as streaming_context | ||
| ): | ||
| # The message has already been persisted, but we still need to send an upda | ||
| await streaming_context.stream_update( | ||
| update=StreamTaskMessageFull( | ||
| parent_task_message=streaming_context.task_message, | ||
| content=tool_request_content, | ||
| ), | ||
| ) |
There was a problem hiding this comment.
We used to stream back a ToolRequestContent when event.item.type=="tool_call_item". This caused unexpected behavior when using MCP servers as a tool provider.
The unexpected behavior was that tool requests and tool responses would be streamed back at the same time, after the tool had been completed. I.e If a tool request was initiated, you would see nothing streamed back until it was completed, At which point both the request and response would be streamed back nearly simultaneously.
| if isinstance(event.data, ResponseOutputItemAddedEvent): | ||
| # Handle tool call initiation - stream tool request immediately when tool call starts | ||
| if (event.data.item.type == "function_call" and | ||
| hasattr(event.data.item, 'call_id') and | ||
| hasattr(event.data.item, 'name')): | ||
|
|
||
| tool_request_content = ToolRequestContent( | ||
| author="agent", | ||
| tool_call_id=event.data.item.call_id, | ||
| name=event.data.item.name, | ||
| arguments={}, # Empty arguments at initiation time | ||
| ) | ||
|
|
||
| # Create tool request using streaming context (immediate completion) | ||
| async with self.streaming_service.streaming_task_message_context( | ||
| task_id=task_id, | ||
| initial_content=tool_request_content, | ||
| ) as streaming_context: | ||
| await streaming_context.stream_update( | ||
| update=StreamTaskMessageFull( | ||
| parent_task_message=streaming_context.task_message, | ||
| content=tool_request_content, | ||
| ), | ||
| ) |
There was a problem hiding this comment.
We now move the ToolRequestContent event when event.data.item.type == "function_call". The events we receive from OpenAI when a tool request is made seem to be:
- function call event: The agent decides to use this tool, and the tool request is initiated
- argument delta events: The agent streams back the arguments it wants to pass to the tool
- argument done event: The agent finishes streaming the argument, and hands it off to the MCP server to execute
- tool_call_item event: The mcp server completes execution and sends this event before the actual output
- tool_call_output_item event: The mcp sends back the tool call output
In a nutshell, the previous logic sent the tool call request event at step 4, after the tool had already executed and completed. Whereas this new logic sends the request at step 1, before the tool call is executed.
Here are the potential concerns with making this change:
- I'm not exactly sure how this would affect OpenAI tool call events, since the events sent during the tool execution cycle is handled differently between it and MCP servers.
- Now, the tool call request contains none of the arguments. That might be okay for some applications, but we probably should handle the argument delta events in the future if we want to make this change.
NOTE: This PR is not meant to be merged. It's just meant to demonstrate off the proposed change for discussion purposes. If we decide this is a good change, I'll refactor to make it merge-ready