Python: [BREAKING] Standardize model selection on model - #4999
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aligns the Python Anthropic integration with newer framework naming by hard-renaming model_id → model, adds first-class wrapper clients for Anthropic-hosted transports (Foundry/Bedrock/Vertex), and exposes these clients consistently via agent_framework.* provider namespaces. It also updates core option handling and observability to recognize model, and refreshes docs/samples/tests accordingly.
Changes:
- Breaking rename of Anthropic configuration and options surface from
model_idtomodel(including env var updates and validation). - Added Anthropic wrapper clients for Azure AI Foundry, Amazon Bedrock, and Google Vertex AI, plus new
agent_framework.googlenamespace export. - Updated core
Agentoption handling and observability to acceptmodel, with corresponding tests and doc updates.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| python/samples/02-agents/providers/anthropic/README.md | Updates sample env var docs to ANTHROPIC_CHAT_MODEL and Foundry vars. |
| python/samples/02-agents/providers/anthropic/anthropic_skills.py | Adds typing ignores to keep sample checks passing with current SDK stubs. |
| python/samples/02-agents/providers/anthropic/anthropic_foundry.py | Switches sample to new AnthropicFoundryClient wrapper. |
| python/packages/core/tests/core/test_observability.py | Adds coverage that telemetry captures options["model"]. |
| python/packages/core/tests/core/test_agents.py | Adds coverage for agent default model selection via client .model. |
| python/packages/core/agent_framework/observability.py | Expands model resolution to prefer model while still accepting model_id. |
| python/packages/core/agent_framework/google/init.py | New lazy-loading namespace exporting Vertex Anthropic clients. |
| python/packages/core/agent_framework/google/init.pyi | Type stub for the new agent_framework.google namespace. |
| python/packages/core/agent_framework/foundry/init.py | Adds lazy exports for Foundry Anthropic wrapper clients. |
| python/packages/core/agent_framework/foundry/init.pyi | Adds type exports for Foundry Anthropic wrapper clients. |
| python/packages/core/agent_framework/anthropic/init.py | Adds lazy exports for Bedrock/Foundry/Vertex wrapper clients. |
| python/packages/core/agent_framework/anthropic/init.pyi | Adds stub exports for new Anthropic wrapper/raw clients. |
| python/packages/core/agent_framework/amazon/init.py | Adds lazy exports for Bedrock Anthropic wrapper/raw clients. |
| python/packages/core/agent_framework/amazon/init.pyi | Adds stub exports for Bedrock Anthropic wrapper/raw clients. |
| python/packages/core/agent_framework/_types.py | Adds model to common ChatOptions base typing. |
| python/packages/core/agent_framework/_agents.py | Updates Agent default/runtime option building to prefer model. |
| python/packages/anthropic/tests/test_anthropic_provider_clients.py | New tests for wrapper/raw client construction and layer ordering. |
| python/packages/anthropic/tests/test_anthropic_client.py | Updates tests for model rename and validates model_id rejection. |
| python/packages/anthropic/tests/conftest.py | Updates Anthropic env fixture to ANTHROPIC_CHAT_MODEL. |
| python/packages/anthropic/README.md | Documents new transport wrapper clients. |
| python/packages/anthropic/AGENTS.md | Updates docs to use model= in examples and lists new clients. |
| python/packages/anthropic/agent_framework_anthropic/_chat_client.py | Implements model rename and rejects model_id in Anthropic options. |
| python/packages/anthropic/agent_framework_anthropic/_foundry_client.py | Adds Foundry wrapper/raw client. |
| python/packages/anthropic/agent_framework_anthropic/_bedrock_client.py | Adds Bedrock wrapper/raw client. |
| python/packages/anthropic/agent_framework_anthropic/_vertex_client.py | Adds Vertex wrapper/raw client. |
| python/packages/anthropic/agent_framework_anthropic/init.py | Re-exports new wrapper/raw clients from the Anthropic package. |
Tao Chen (TaoChenOSU)
left a comment
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 79%
✗ Correctness
The PR adds Anthropic Bedrock, Foundry, and Vertex provider clients and renames
model_idtomodelacross the Anthropic package. However, there is a critical runtime bug: the_process_messageand_process_stream_eventmethods now passmodel=when constructingChatResponseandChatResponseUpdate, but neither class accepts amodelkeyword argument—they only acceptmodel_id. This will cause aTypeErrorat runtime on every non-streaming and streaming response. The core_merge_options,Agent.__init__, and observability changes look correct and properly handle themodel/model_idmutual-exclusion logic.
✓ Security Reliability
✓ Test Coverage
The PR adds three new Anthropic provider clients (Bedrock, Foundry, Vertex), renames
model_idtomodelacross the Anthropic package, and updates_merge_options/Agentto handlemodel/model_idmutual exclusion. Test coverage is generally good: the new provider clients have construction and validation tests, themodel_id→modelrename is tested, and_merge_optionsmutual-exclusion is partially tested. However, there are two notable coverage gaps: (1) the Foundry client'sbase_urlcode path is untested, and (2) the initial_merge_optionscleanup when the base dict itself contains bothmodelandmodel_idhas no test.
✗ Design Approach
This PR migrates the Anthropic client family from
model_idtomodeland introduces three new provider clients (Foundry, Bedrock, Vertex). The provider client structure, mutual-exclusion logic in_merge_options, and namespace layout are all sound. One significant design issue stands out: removing"model_id": "model"fromOPTION_TRANSLATIONSand replacing it with a hardraise ValueErrorin_prepare_optionsis a breaking change that contradicts the sharedChatOptionscontract._ChatOptionsBasestill declaresmodel_id: stras a valid key (other clients still consume it), so the type system tells users the key is valid but the Anthropic runtime throws. Users calling the Anthropic client directly withoptions={"model_id": ".."}get an opaque runtime error with no deprecation period; the old translation approach was the correct design. A secondary, minor inconsistency exists in_vertex_client.pywhereaccess_tokenbypassesload_settingswhile all other credentials in Bedrock and Foundry clients go through it.
Flagged Issues
-
ChatResponse.__init__andChatResponseUpdate.__init__do not accept amodelkeyword argument (onlymodel_id). The Anthropic client now passesmodel=to both constructors, which will crash withTypeErrorat runtime on every non-streaming and streaming response. -
_prepare_optionshard-raisesValueErrorwhenmodel_idappears in runtime options, but_ChatOptionsBasestill declaresmodel_id: stras a valid key and other framework clients consume it. This contract violation means the type system permitsmodel_idwhile the Anthropic runtime rejects it. The originalOPTION_TRANSLATIONS = {"model_id": "model"}pattern was the correct design—it translated silently (or could emit aDeprecationWarning) without breaking calers.
Suggestions
- Add a test for
RawAnthropicFoundryClientwhenbase_urlis provided instead ofresource—the Foundry client has two distinct construction branches and only theresourcepath is currently covered. - Add a
_merge_optionstest where the base dict already contains bothmodelandmodel_idto cover the cleanup logic (e.g.,_merge_options({'model': 'a', 'model_id': 'b'}, {})should dropmodel_id). - Consider adding an end-to-end test exercising
Agent.run(model='override-model')to verify_prepare_run_contextmodel/model_id handling at runtime, not just atAgent.__init__time. - In
_vertex_client.py,access_tokenis resolved from the constructor argument directly and never loaded from settings/env, unlike credentials in the Bedrock and Foundry clients. Consider adding it toAnthropicVertexSettingsfor consistency, or document thataccess_tokenis constructor-only. - The
agent_framework.googlenamespace currently contains only Anthropic Vertex clients. Consider adding a docstring note clarifying this is limited to the Anthropic SDK's Vertex transport, to avoid misleading users expecting native Gemini/VertexAI support.
Automated review by TaoChenOSU's agents
40610c7 to
77f6abe
Compare
192310f to
a0c7f8f
Compare
286a95a to
3b45390
Compare
784d3b8 to
5ce911a
Compare
Rename the Anthropic client model option from model_id to model, add provider-specific Anthropic wrappers for Foundry, Bedrock, and Vertex, and expose them through the Anthropic, Foundry, Amazon, and Google namespaces. Update core option handling, docs, samples, and tests accordingly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Cast the Anthropic beta client to Any in the skills sample so the pre-commit sample pyright check no longer fails on beta skills and files endpoints that are not exposed by the current SDK stubs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Retrigger PR validation after an unrelated Copilot review workflow SAML failure and a transient external tau2 git fetch failure in the Windows Python test setup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
5ce911a to
39e26b2
Compare
Motivation and Context
The Python codebase still mixed
model_id,deployment_name, and other legacy model-selection aliases across core types, provider clients, env vars, telemetry sourcing, docs, and samples. The Anthropic package was the most visible gap because it still exposedmodel_idand lacked first-class Foundry, Bedrock, and Vertex wrapper clients, but the inconsistency extended well beyond Anthropic.This PR standardizes AF-owned Python model-selection surfaces on
model, updates the related configuration and documentation surfaces, and adds the Anthropic transport wrappers. This is a breaking change because callers and configuration now need to use the newmodel-based API and env-var names.Description
modelacross shared core types/runtime, provider clients, helper packages, and provider settings/env varsagent_framework.anthropic,agent_framework.foundry,agent_framework.amazon, andagent_framework.googlemodelwhile keeping the existing spec-defined emitted attribute keys unchangedmodel_id/ deployment-style aliases from docs, samples, README and.env.examplefiles, including the follow-up Foundry-backed evaluation and workflow sample env-var cleanupdeployment_nameormodel_nameFixes #5004
Contribution Checklist