Add tool_choice setting#3611
Conversation
|
haven't integrated reviewed the code with #1820 in mind, so putting this back to draft |
|
I'll push an update in a moment, just wanted to note two things:
warnings to keep
warnings removed
let me know if this is fine |
6d68488 to
d91890c
Compare
1bdac3c to
dcf932b
Compare
|
two main things from the latest changes:
|
| if not openai_profile.openai_supports_tool_choice_required: | ||
| explicit_choice = (model_settings or {}).get('tool_choice') | ||
| if explicit_choice == 'required' or isinstance(explicit_choice, list): | ||
| raise UserError( |
There was a problem hiding this comment.
Same pattern as Bedrock — backticks needed around code identifiers in error messages:
f'`tool_choice={explicit_choice!r}` is not supported by model {model_name!r}. '| if resolved_tool_choice in ('auto', 'none'): | ||
| tool_choice = resolved_tool_choice | ||
| elif resolved_tool_choice == 'required': | ||
| tool_choice = 'required' if profile.grok_supports_tool_choice_required else 'auto' |
There was a problem hiding this comment.
Same issue as flagged in anthropic.py: silently downgrading 'required' to 'auto' changes the semantic contract without the caller knowing. At minimum, add a comment explaining why this is safe (the user asked for required but this model doesn't support it — what are the implications?).
This pattern repeats at lines 520 and 526 in this file as well.
| elif model_request_parameters.output_tools: # pragma: no cover | ||
| # this branch is dead code (output tool is being handled above) | ||
| # leaving it in for the TODO (support NativeOutput properly) |
There was a problem hiding this comment.
This dead code branch is concerning. If the _get_tool_choice refactoring means output tools are now always included in the filtered tools list (so this branch is unreachable), consider either:
- Removing it entirely (with the TODO moved to a GitHub issue), or
- Keeping it but with a clear explanation of when/why it could become reachable again
Leaving dead code behind pragma: no cover makes it easy to forget and rot.
| @dataclass | ||
| class ToolOrOutput: | ||
| """Restricts function tools while keeping output tools and direct text/image output available. | ||
|
|
||
| Use this when you want to control which function tools the model can use | ||
| in an agent run while still allowing the agent to complete with structured output, | ||
| text, or images. | ||
|
|
||
| See the [Tool Choice guide](../tools-advanced.md#tool-choice) for examples. | ||
| """ | ||
|
|
||
| function_tools: list[str] |
There was a problem hiding this comment.
🚩 ToolOrOutput dataclass uses plain @DataClass instead of Pydantic model, but ModelSettings docstring says 'All types must be serializable using Pydantic'
The ToolOrOutput class at pydantic_ai_slim/pydantic_ai/settings.py:27-38 is a plain @dataclass. The ModelSettings docstring at line 49 states 'All types must be serializable using Pydantic.' While Pydantic can serialize/deserialize plain dataclasses, the round-trip behavior may differ from a Pydantic BaseModel — specifically, when deserializing from JSON, Pydantic may reconstruct ToolOrOutput as a dict rather than a dataclass instance, depending on the validation mode. Since resolve_tool_choice uses isinstance(function_tool_choice, ToolOrOutput) at line 108 of _tool_choice.py, deserialization producing a dict instead of a ToolOrOutput instance would cause the isinstance check to fail, falling through to assert_never. This matters for any code path that serializes and deserializes ModelSettings (e.g., Temporal workflows, message history persistence).
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Claude here: Re this comment about ToolOrOutput being a plain @dataclass:
ModelSettings can't actually be round-tripped through Pydantic serialization — it fails on httpx.Timeout which lacks a pydantic-core schema. The 'All types must be serializable' docstring is aspirational. ToolOrOutput is consumed internally by resolve_tool_choice() before any API call and is never serialized. Using @dataclass is consistent with other internal types like ToolDefinition.
# Conflicts: # pydantic_ai_slim/pydantic_ai/models/anthropic.py # pydantic_ai_slim/pydantic_ai/models/bedrock.py # pydantic_ai_slim/pydantic_ai/models/openai.py # pydantic_ai_slim/pydantic_ai/models/xai.py
… thinking detection Move tool_choice validation from per-request (in `_agent_graph`) to a baseline-only check at `Agent.iter()` entry, validating only the static dict layers (model defaults, agent settings, run-level settings) and trusting callable layers — including capability-supplied `get_model_settings()` callables. This unblocks the canonical "force a tool until it succeeds, then step aside" pattern documented under Dynamic tool choice via capabilities. Fix `_is_thinking_enabled` (Bedrock) and `_support_tool_forcing` (Anthropic) to also read `model_request_parameters.thinking`. `Model.prepare_request` strips unified `thinking` from `model_settings` into `params.thinking` before the tool-choice helpers run, so the prior settings-only checks silently bypassed both the thinking+output-tool guard and the thinking+tool-forcing guard for users on the unified `thinking` field. Reframe docs: settings.py docstring and tools-advanced.md now point users to `pydantic_ai.direct.model_request` for single-shot calls and to capability callables for per-step dynamic tool choice; capabilities.md cross-links the new section. Tests: - New `test_capability_can_inject_forcing_tool_choice_per_step` proving the validator relaxation end-to-end (parametrized over `'required'` and `list[str]`) - New `test_bedrock_unified_thinking_with_tool_forcing_raises` regression for the post-`prepare_request` strip path - New `test_support_tool_forcing_reads_params_thinking` unit regression covering Anthropic + Bedrock helpers - Rewrote `test_openai_tool_choice_required_unsupported_raises_error` to use `direct.model_request` so it actually hits the OpenAI-specific `_support_tool_forcing` UserError path instead of the agent baseline validator
`_get_responses_tool_choice` returned `tool_choice=None` whenever `tool_defs` was empty, but `_responses_create` later merges builtin tools on top, so the resolved `'required'` (from `allow_text_output=False`) was silently dropped and the API used its `'auto'` default. Move the "no tools → no tool_choice" decision to the caller, where it can see the combined tool list including builtins. Also fix the doc example docstring quotes (Q002 from CI) — `'''…'''` to `"""…"""` in `RequireFirstCall`.
Parametrize the existing `test_bedrock_output_tool_with_thinking_raises` across the legacy `bedrock_additional_model_requests_fields` form and the unified `thinking=True` form. The unified case exercises the pre-strip branch in `_is_thinking_enabled` (line 1436) that the post-strip regression test couldn't reach.
`_get_tool_choice` (Chat) and `_get_responses_tool_choice` (Responses)
sent forced `tool_choice` for tuple-resolved values (`('required', {names})`
from `tool_choice=['x', ...]` with extra registered tools, or
`ToolOrOutput` without direct output) without consulting the model
profile. On models with `openai_supports_tool_choice_required=False`,
this would push an unsupported parameter to the API.
Mirror the Anthropic tuple-branch pattern: call `_support_tool_forcing`
and fall back to `'auto'` (single-tool case) or flip `tool_choice_mode`
to `'auto'` (multi-tool subset case) when forcing isn't supported.
The previous parametrize referenced `BedrockModelSettings(...)` directly in `pytest.param`, which evaluates at module-import time and crashes collection in pydantic-ai-slim CI where the bedrock extra isn't installed. Splitting into two test functions keeps the references inside the bodies, where the file's module-level skipif already gates them.
# Conflicts: # pydantic_ai_slim/pydantic_ai/models/google.py # pydantic_ai_slim/pydantic_ai/settings.py
Closes #2799
Closes #3092
Adds
tool_choicetoModelSettings, letting users control how the model interacts with function tools.Currently Pydantic AI internally decides whether to use
tool_choice='auto'or'required'based on output configuration, but users have no way to override this. The workaround was usingextra_body={'tool_choice': 'none'}which is provider-specific and doesn't work everywhere.This PR allows the user to set
tool_choiceto:One important distinction: this only affects
function tools(the ones you register on the agent), not output tools (used internally for structured output). So if you have an agent withoutput_type=SomeModeland you set
tool_choice='none', the output tool stays available - you'll just get a warning about it.Implementation is spread across all model providers since each has its own API format for tool_choice.
Added a
resolve_tool_choiceutility that handles validation (checking tool names exist, warning aboutconflicts with output tools) and returns a normalized representation that each provider then maps to their specific format.
Bedrock is a bit of a special case - it doesn't support 'none' at all, so we fall back to 'auto' with a warning. Anthropic has a constraint where 'required' and specific tool selection don't work with thinking/extended thinking enabled.
TODO