Python: [BREAKING] Replace Hosted*Tool classes with tool methods#3634
Python: [BREAKING] Replace Hosted*Tool classes with tool methods#3634giles17 wants to merge 15 commits intomicrosoft:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements a breaking change that refactors hosted tool support from standalone Hosted*Tool classes to static factory methods on chat clients. This improves API discoverability by making tool support explicit per client type.
Changes:
- Removed
HostedCodeInterpreterTool,HostedWebSearchTool,HostedFileSearchTool,HostedImageGenerationTool, andHostedMCPToolclasses - Added static factory methods (
get_code_interpreter_tool(),get_web_search_tool(), etc.) to relevant client classes - Introduced protocol classes (
SupportsCodeInterpreterTool,SupportsWebSearchTool, etc.) for runtime checking of tool support - Updated all samples, tests, and documentation to use the new API
Reviewed changes
Copilot reviewed 119 out of 119 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
python/packages/core/agent_framework/_clients.py |
Added tool support protocol definitions |
python/packages/core/agent_framework/openai/_chat_client.py |
Added get_web_search_tool() static method |
python/packages/core/agent_framework/openai/_assistants_client.py |
Added get_code_interpreter_tool() and get_file_search_tool() static methods |
python/packages/core/agent_framework/openai/_responses_client.py |
Added static methods for all hosted tools (code interpreter, web search, file search, image generation, MCP) |
python/packages/azure-ai/agent_framework_azure_ai/_client.py |
Added Azure-specific get_mcp_tool() implementation |
python/packages/anthropic/agent_framework_anthropic/_chat_client.py |
Added static methods for Anthropic-supported tools |
| Multiple sample files | Updated to use new static factory methods instead of Hosted*Tool classes |
| Multiple test files | Updated test assertions to check for dict-based tools instead of class instances |
python/packages/declarative/agent_framework_declarative/_loader.py |
Updated to create dict-based tools directly |
eavanvalkenburg
left a comment
There was a problem hiding this comment.
Couple of notes, overall I think we will have to move to remove some additional pieces of from tools and streamline things even more, I would expect only the functionTool to be handled in the each clients prepare code, everything else should already be setup correctly.
python/packages/anthropic/agent_framework_anthropic/_chat_client.py
Outdated
Show resolved
Hide resolved
python/packages/anthropic/agent_framework_anthropic/_chat_client.py
Outdated
Show resolved
Hide resolved
python/packages/azure-ai/agent_framework_azure_ai/_chat_client.py
Outdated
Show resolved
Hide resolved
python/packages/core/agent_framework/openai/_responses_client.py
Outdated
Show resolved
Hide resolved
python/samples/getting_started/agents/anthropic/anthropic_skills.py
Outdated
Show resolved
Hide resolved
| }) | ||
| elif isinstance(tool, MutableMapping): | ||
| # Handle dict-based tools from static factory methods | ||
| tool_dict = tool if isinstance(tool, dict) else dict(tool) |
There was a problem hiding this comment.
This is not needed, you can safely ignore and assume these are dicts
|
|
||
| server_label = tool_dict.get("server_label") | ||
| if not server_label: | ||
| continue |
There was a problem hiding this comment.
This seems strange, it is a mcp, but not valid, then it should raise!
| } | ||
|
|
||
| mcp_resources.append(mcp_resource) | ||
| for tool in tools: |
There was a problem hiding this comment.
I think this can be simpler, just do a list comprehension with if type == 'mcp' and then pop type, and pass through, the server label should come with the get.. Method already, if people do not put in the server label when constructing manually, let the service raise
| if not bing_search: | ||
| if isinstance(tool, FunctionTool): | ||
| tool_definitions.append(tool.to_json_schema_spec()) # type: ignore[reportUnknownArgumentType] | ||
| elif isinstance(tool, ToolDefinition): |
There was a problem hiding this comment.
This seems to me like we should allow Any as type for the tool list, and then we can make it more specific for each implementation
| if not server_label or not server_url: | ||
| raise ServiceInitializationError("MCP tool requires 'server_label' and 'server_url'.") | ||
| allowed_tools = tool_dict.get("allowed_tools", []) | ||
| mcp_tool = McpTool(server_label=server_label, server_url=server_url, allowed_tools=allowed_tools) |
There was a problem hiding this comment.
Why wouldnt a MCPTool be passed instead of this dict?
| allowed_tools: list[str] | None = None, | ||
| headers: dict[str, str] | None = None, | ||
| project_connection_id: str | None = None, | ||
| ) -> Any: |
There was a problem hiding this comment.
You know what type this is, no?
| """ | ||
|
|
||
| @staticmethod | ||
| def get_code_interpreter_tool(**kwargs: Any) -> dict[str, Any]: |
There was a problem hiding this comment.
The return type should be more flexibel
| async def main() -> None: | ||
| """Example of streaming response (get results as they are generated).""" | ||
| # Create MCP tool configuration using static method | ||
| mcp_tool = AnthropicClient.get_mcp_tool( |
There was a problem hiding this comment.
In the samples I think we should follow the pattern of client. Instead of class, because clients should be easy and this makes it harder!
Motivation and Context
This PR refactors hosted tool support from standalone
Hosted*Toolclasses to@staticmethodfactory methods on the chat clients that support them. This is a breaking change that improves API discoverability and makes tool support explicit per clientBreaking Changes
Removed classes:
HostedCodeInterpreterToolHostedWebSearchToolHostedImageGenerationToolHostedFileSearchToolHostedMCPToolMigration:
#3586
Description
Contribution Checklist