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

Safe TaskResult in BaseGroupChat #5109

Open
Leon0402 opened this issue Jan 18, 2025 · 2 comments
Open

Safe TaskResult in BaseGroupChat #5109

Leon0402 opened this issue Jan 18, 2025 · 2 comments

Comments

@Leon0402
Copy link
Contributor

Leon0402 commented Jan 18, 2025

What feature would you like to be added?

I think run_stream in BaseGroupChat is a little bit awkward to use, because it streams both messages and the task result in the end. This leads to code like this for example:

async for message in self.run_stream(
    task=task,
    cancellation_token=cancellation_token,
):
   if isinstance(message, BaseMessage) and message.models_usage is not None:
        usages.append(message.models_usage)
    if isinstance(message, TaskResult):
        result = message

So you basically have to check the types of the messages, find the task result in the message and such stuff. This is partially also an issue with Python, because typings do not allow to specify that TaskResult will always be the last message.

I wonder if it would be better to separate concerns here and have one method for the actual messages and one method for the task result or something like this?

Why is this needed?

Make code easier to use.

@ekzhu
Copy link
Collaborator

ekzhu commented Jan 20, 2025

The API is now "stable" so we can't change it for v0.4.*. However, run_stream is admittedly quite "low-level" such that it requires something like Console to help work with it.

Instead of changing run_stream we can add other sink functions similar to Console to make it easier to consume the messages? Ideas?

One pattern I have seen is an event handler class, such as the OpenAI Assistant's EventHandler class.

@Leon0402
Copy link
Contributor Author

Maybe just two new methods run_message_stream and get_result and the old run_stream could be implemented with these two methods internally to avoid duplication.

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

No branches or pull requests

2 participants