Skip to content
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

Fixed and chain stream #39029

Open
wants to merge 5 commits into
base: feature/azure-ai-projects-beta5
Choose a base branch
from

Conversation

howieleung
Copy link
Member

@howieleung howieleung commented Jan 3, 2025

  1. Fixed event streaming so that it can encounter multiple event from a buffer or multiple of buffer to form a complete event.
  2. Fixed Issue with event handler making function tool calls. #39028. When we call create_stream, an event handler is associated to the response iterator. Usually people write code like this:
    with project_client.agents.create_stream(
        thread_id=thread.id, assistant_id=agent.id, event_handler=MyEventHandler(functions)
    ) as stream:
        for event_type, event_data, func_return in stream:
            print(f"Event Data: {str(event_data)")

Then when function tool calls are executed by the event handler, the response iterator of tool call stream is associated to the same event handler followed by calling until_done(). The call of until_done() pulls all events until no more response from the stream. As a result, the for loop above don't receive the events.

To fix the issue, I don't do until_done() after making tool call, so the for loop above will receive events from tool calls.

  1. Make submit_tool_outputs_to_stream function to return None instead of AgentRunStream because developers are not expected to iterate events from submit_tool_outputs_to_stream by context manager.

  2. Before the fix, when you make create_stream calls, the event handler received response_iterator and create_buffers from response_iterator. When the event handler makes tool calls, it will overwrite the response_iterator from the toolcall stream and reset the buffer. This might cause buffer and event lost. With the fix, when event handler make toolcalls, we chain the response_iterator of the toolcall stream to the original stream and retain buffer.

@jhakulin
Copy link
Member

jhakulin commented Jan 3, 2025

We need to add extensive tests because lots of changes

async for event_type, event_data, func_return in stream:
print(f"Received data.")
print(f"Streaming receive Event Type: {event_type}")
print(f"Event Data: {str(event_data)[:100]}...")
Copy link
Member

@jhakulin jhakulin Jan 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the reason for 100?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is purely for easy reading to show that there is an event data object. I can change to 150. It just depends how wide the terminal for display.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we avoid using any magic number?

Copy link
Member

@jhakulin jhakulin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to integrate with CI tests

@azure-sdk
Copy link
Collaborator

API change check

APIView has identified API level changes in this PR and created following API reviews.

azure-ai-projects

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants