-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
MagenticOne Orchestrator Fixes #4430
Conversation
@ekzhu still working on this, but curious, is this better for integrating the M1 orchestrator as an actual group chat manager? Anything else that would improve it? |
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.
Looks good. Don't forget about adding unit tests for the magentic one group chat. Use ReplayChatCompletionClient #4412
Yes it is good to make it a group chat manager. It allows us to refactor group chat manager logic into a developer facing API. |
Also, remember to add API docs with examples and argument list for MagenticOneGroupChat |
Thanks Eric! Added two simple tests and API docs, and ironed a few bugs + trace logger. I ran into a mypy error because of overriding handle_agent_response and handle_start, added # type ignore because I couldn't figure out how to fix. Please merge when appropriate |
One last small lint error see checks. |
Overriding handlers causes mypy error is a know issue. It's okay to ignore it for now. |
Why are these changes needed?
Add back some verification logic to MagenticOne Orchestrator.
Base the orchestrator on the base group chat manager.
Remaining:
Related issue number
Will resolve #4177
#3756 - Updates to this PR