-
Notifications
You must be signed in to change notification settings - Fork 996
[BREAKING] Python: Refactor orchestrations #3023
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?
[BREAKING] Python: Refactor orchestrations #3023
Conversation
python/packages/core/agent_framework/_workflows/_orchestration_request_info_old.py
Outdated
Show resolved
Hide resolved
python/samples/getting_started/workflows/orchestration/group_chat_philosophical_debate.py
Show resolved
Hide resolved
python/samples/getting_started/workflows/orchestration/handoff_return_to_previous.py
Show resolved
Hide resolved
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.
Pull request overview
This PR refactors Python orchestrations in the Agent Framework, introducing breaking changes to group chat, handoff, magentic, and other orchestration patterns. The refactoring aims to improve maintainability, simplify workflows, and move to a broadcasting model for better participant synchronization.
Key Changes:
- Refactored group chat to split agent-based and function-based orchestrators with a star topology
- Simplified handoff by removing single-tier and coordinator concepts, moving to a decentralized broadcasting model
- Updated magentic orchestration with new event types and progress tracking
- Simplified request info mechanisms across sequential and concurrent workflows
- Enhanced sub-workflow request propagation capabilities
Reviewed changes
Copilot reviewed 46 out of 50 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| group_chat_builder_tool_approval.py | Updated to use new group chat API with with_select_speaker_func, GroupChatState, and new event types |
| concurrent_builder_tool_approval.py | Updated tool approval sample with modified request info handling and agent setup |
| sequential_custom_executors.py | Updated custom executor to handle AgentExecutorResponse instead of raw conversation |
| magentic_human_replan.py | Deleted - functionality moved to other samples |
| magentic_human_plan_update.py | Deleted - replaced by magentic_human_plan_review.py |
| magentic_human_plan_review.py | New sample demonstrating plan review with simplified API |
| magentic_checkpoint.py | Updated for new magentic API with MagenticPlanReviewRequest |
| magentic_agent_clarification.py | Deleted - functionality consolidated |
| magentic.py | Significantly updated with new event types and simplified orchestrator handling |
| handoff_with_code_interpreter_file.py | Updated handoff API from set_coordinator to with_start_agent |
| handoff_specialist_to_specialist.py | Deleted - functionality replaced by simpler handoff model |
| handoff_simple.py | Major refactor with new API (with_start_agent, HandoffAgentUserRequest) |
| handoff_return_to_previous.py | Deleted - return-to-previous pattern removed |
| handoff_participant_factory.py | Updated to use new handoff API |
| handoff_autonomous.py | Updated autonomous mode API with per-agent turn limits |
| group_chat_simple_selector.py | Completely rewritten with new selector function signature |
| group_chat_philosophical_debate.py | Updated to use with_agent_orchestrator instead of set_manager |
| group_chat_agent_manager.py | Updated orchestrator agent setup and API |
| sequential_request_info.py | Updated request info to work after agent execution instead of before |
| group_chat_request_info.py | Updated for new request info API with AgentExecutorResponse |
| concurrent_request_info.py | Updated request info handling for concurrent workflows |
| sub_workflow_basics.py | Minor whitespace fix |
| magentic_workflow_as_agent.py | Simplified event handling, removed specific event type processing |
| handoff_workflow_as_agent.py | Major refactor with new handoff API and request handling |
| group_chat_workflow_as_agent.py | Updated to use with_agent_orchestrator |
| test_workflow_kwargs.py | Updated tests for new APIs with breaking changes marked as xfail where needed |
| test_sequential.py | Updated test to handle AgentExecutorResponse in custom executors |
| test_orchestration_request_info.py | Extensive test updates for new request info architecture |
| test_magentic.py | Major test refactoring for new magentic implementation |
| test_handoff.py | Extensive test refactoring for simplified handoff model |
| test_executor.py | Added comprehensive tests for executor output type introspection |
| test_agent_run_event_typing.py | Removed tests for nullable event data |
| _workflow_context.py | Added request_id parameter support for request info tracking |
| _workflow.py | Updated response type validation with utility function |
| _runner_context.py | Changed request tracking to use RequestInfoEvent instead of raw data |
moonbox3
left a comment
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.
After another look, some questions for you.
| .add_handoff(summary_agent, [coordinator]) | ||
| .with_autonomous_mode( | ||
| turn_limits={ | ||
| coordinator.display_name: 5, |
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.
It's not clear to me what an agent's display_name has to do with the turn limit?
| participants=[coordinator, research_agent, summary_agent], | ||
| ) | ||
| .set_coordinator(coordinator) | ||
| .with_start_agent(coordinator) |
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.
Aren't we planning to allow the use of executors in the future with these patterns (not only agents as we do today)? .with_start_agent kind of locks us into agent-only use, right?
| # Please refer to `with_plan_review` for proper human interaction during planning phases. | ||
| await asyncio.get_event_loop().run_in_executor(None, input, "Press Enter to continue...") | ||
|
|
||
| elif isinstance(event, GroupChatRequestSentEvent): |
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.
Do we need this elif isinstance(event, GroupChatRequestSentEvent) in this sample? As a dev, I need to know that the GroupChatRequestSentEvent type is applicable to the magentic pattern?
| last_message_id = message_id | ||
| print(event.data, end="", flush=True) | ||
|
|
||
| elif isinstance(event, MagenticOrchestratorEvent): |
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.
This is going to break DevUI - we previously moved away from custom magentic events (we had these in the past) to purely raising AgentRunUpdateEvent which was used throughout workflows.
| elif isinstance(event, WorkflowOutputEvent): | ||
| output_event = event | ||
|
|
||
| if not output_event: |
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.
It feels like we should look at somehow always producing a "WorkflowOutputEvent" to avoid the need to have to check if the event is present or not (idle, error, complete, etc).
| name="Coordinator", | ||
| description="Coordinates multi-agent collaboration by selecting speakers", | ||
| instructions=""" | ||
| ORCHESTRATOR_AGENT_INSTRUCTIONS = """ |
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.
These instructions aren't super verbose. Why can't we have them inline below?
| automatically replanning. | ||
| Key concepts: | ||
| - with_human_input_on_stall(): Enables human intervention when workflow detects stalls |
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 don't see us configuring the builder with this with_human_input_on_stall(). It would be good to have this in a sample (which looks to now be deleted).
| self._full_conversation.extend(self._cache) | ||
|
|
||
| # Check termination condition before running the agent | ||
| if await self._check_terminate_and_yield(cast(WorkflowContext[Never, list[ChatMessage]], ctx)): |
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.
Is it intended that termination conditions can short-circuit immediately on initial input? This could cause surprising behavior where a workflow terminates before any agent has spoken if the initial message triggers the condition.
| new_tools: list[AIFunction[Any, Any]] = [] | ||
| for target in targets: | ||
| tool = self._create_handoff_tool(target.target_id, target.description) | ||
| if tool.name in existing_names: |
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.
To make sure I understand: if tools are added dynamically at runtime, could there still be conflicts?
Motivation and Context
Required by #429
Description
This PR extensively refactors and simplifies the orchestrations in Agent Framework Workflows. Details are as follows.
Group Chat
BaseGroupChatOrchestrator,GroupChatOrchestratorandAgentBasedGroupChatOrchestrator.GroupChatRequestMessageandGroupChatParticipantMessage.AgentApprovalExecutorandAgentRequestInfoExecutor. HIL happens after an agent responds and the agent-HIL loop stops until instructed.Handoff
HandoffAgentExecutorwhich derives fromAgentExecutor. This executor handles handoff.Magentic
Sequential & Concurrent
AgentApprovalExecutorandAgentRequestInfoExecutor. HIL happens after an agent responds and the agent-HIL loop stops until instructed.Sub-workflow (
WorkflowExecutor)Contribution Checklist