Python: [BREAKING] simplify ag-ui run logic, fix mcp bugs, fix anthropic client issues in ag-ui #3322
Conversation
moonbox3
added
the
breaking change
There was a problem hiding this comment.
Pull request overview
This PR performs a major refactoring of the AG-UI orchestration system to simplify the run logic and fix several bugs. The changes consolidate orchestration into a single linear flow in _run.py, removing the complex orchestrator pattern and confirmation strategies.
Changes:
- Simplified orchestration by consolidating into a single
run_agent_stream()function in_run.py - Fixed Anthropic client bug where empty text content was causing API errors
- Fixed MCP tool approval handling by ensuring approval tools are properly passed to the execution handler
- Removed confirmation strategies (BREAKING CHANGE)
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
python/packages/core/agent_framework/_tools.py |
Added debug logging and fixed conversation_id filtering in tool invocation |
python/packages/core/agent_framework/_agents.py |
Added tool deduplication by name when merging options |
python/packages/anthropic/agent_framework_anthropic/_chat_client.py |
Fixed empty text content handling and added conversation_id to ignored options |
python/packages/ag-ui/agent_framework_ag_ui/_run.py |
New 949-line orchestration implementation replacing orchestrators |
python/packages/ag-ui/agent_framework_ag_ui/_orchestration/_tooling.py |
Fixed approval tool detection to ensure tools are passed when needed |
python/packages/ag-ui/agent_framework_ag_ui/_agent.py |
Simplified to use new run_agent_stream() function |
python/packages/ag-ui/agent_framework_ag_ui/__init__.py |
Removed confirmation strategy exports |
python/packages/ag-ui/agent_framework_ag_ui_examples/agents/ |
Removed confirmation_strategy parameters from agent examples |
| Multiple test files | Removed obsolete tests and added new tests for simplified architecture |
| logger.info( | ||
| f"[APPROVAL-DEBUG] _try_execute_function_calls: tool_map keys={list(tool_map.keys())}, " | ||
| f"approval_tools={approval_tools}" | ||
| ) |
There was a problem hiding this comment.
Debug logging statements should be removed before merging to production. The [APPROVAL-DEBUG] prefix and associated logger.info calls appear to be temporary debugging code that should not be committed. Consider using logger.debug() for detailed debugging or removing these statements entirely.
| fc_count = len(function_calls) if function_calls else 0 | ||
| logger.info( | ||
| f"[APPROVAL-DEBUG-STREAMING] tools extracted: {tools is not None}, function_calls: {fc_count}" | ||
| ) | ||
| if tools: | ||
| for t in tools if isinstance(tools, list) else [tools]: | ||
| t_name = getattr(t, "name", "unknown") | ||
| t_approval = getattr(t, "approval_mode", None) | ||
| logger.info(f"[APPROVAL-DEBUG-STREAMING] - {t_name}: approval_mode={t_approval}") |
There was a problem hiding this comment.
Debug logging statements with [APPROVAL-DEBUG-STREAMING] prefix should be removed before merging. These appear to be temporary debugging code used during development.
| # Note: TestExecutionContext was removed along with _orchestrators.py | ||
| # Tests should now use run_agent_stream() directly or the StubAgent class |
There was a problem hiding this comment.
The comment at the end of the file references code that has been removed. The note about TestExecutionContext being removed is useful during transition, but consider removing this comment in a follow-up cleanup or documenting the replacement pattern more clearly in the docstring of the test utilities that remain.
| # chat_client: BaseChatClient[ChatOptions] = AzureOpenAIChatClient() | ||
| chat_client: BaseChatClient[ChatOptions] = AnthropicClient() |
There was a problem hiding this comment.
The hardcoded change from AzureOpenAIChatClient to AnthropicClient appears to be temporary for testing purposes. This should be reverted before merging or made configurable through environment variables so developers can choose their chat client.
| if "accepted" in result and "steps" in result: | ||
| return True | ||
| except json.JSONDecodeError: | ||
| pass |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| pass | |
| logger.debug( | |
| "Failed to parse confirm_changes tool result as JSON; treating as non-confirmation." | |
| ) |
| flow.current_state[state_key] = state_value | ||
| yield StateSnapshotEvent(snapshot=flow.current_state) | ||
| except json.JSONDecodeError: | ||
| pass |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| pass | |
| # Ignore malformed JSON in tool arguments for predictive state; | |
| # predictive updates are best-effort and should not break the flow. | |
| logger.warning( | |
| "Failed to decode JSON arguments for predictive tool '%s' (tool_call_id=%s).", | |
| tool_name, | |
| tool_call_id, | |
| ) |
2 participants
Motivation and Context
This refactor restructures AG-UI orchestration to reduce bespoke state handling and make the flow maintainable. The existing code was growing in complexity and brittleness; this change consolidates orchestration, simplifies state transitions, and removes ad hoc management paths.
Important
The only breaking change here is we're removing the confirmation strategies - no need for this custom behavior. The
confirmation_strategyarg is no longer passed into AgentFrameworkAgent.Description
Testing:
Contribution Checklist