Repository navigation
Pass Deep Agents tool_choice through the provider's bind_tools - #249
Merged
Merged
Conversation
The model activity bound tool schemas with bind_tools() and then applied
tool_choice and parallel_tool_calls with .bind(). Providers convert these
options only inside bind_tools(), so the raw values reached the API:
create_agent's tool_choice="any" went to Anthropic as "any" instead of
{"type": "any"}, and to OpenAI-compatible servers as "any" instead of
"required", which vLLM rejects with a 400.
Pass both options to bind_tools() on the worker. The activity input is
unchanged, so existing histories are not affected.
Fixes temporalio#248
Signed-off-by: sohomchakraborty <16609933+sohom-cs@users.noreply.github.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
|
brianstrauch
requested changes
Oct 28, 2026
brianstrauch
left a comment
brianstrauch
left a comment
Member
There was a problem hiding this comment.
Please make the new regression coverage run with the plugin's committed CI dependencies before merging. A provider-independent forwarding test should let us address the skipped case without adding a development dependency; see the inline suggestion.
Contributor
Author
|
@brianstrauch Thanks for the review! Could you merge this when you get a chance? |
brianstrauch
enabled auto-merge (squash)
October 29, 2026 17:49
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was changed
DeepAgentActivities._build_bound_modelnow passestool_choiceandparallel_tool_callsto the provider'sbind_toolsinstead of.bind(). Other bind options still go through.bind(), as before.Why?
Providers convert these two options only inside
bind_tools. Without that,create_agent'stool_choice="any"(used for structured output) reaches Anthropic as"any"instead of{"type": "any"}, and OpenAI-compatible servers as"any"instead of"required". vLLM answers with a 400. Details and an offline repro are in #248.The activity input does not change, so no
patchedgate is needed and existing histories replay as before. The test from temporalio/sdk-python#1901 (test_model_binding_preserves_kwargs_with_tools) still passes: its fakebind_toolskeeps raw kwargs, so it sees the same result on either path.Checklist
Closes [Bug] Deep Agents sends tool_choice "any" without the provider's conversion; vLLM rejects it #248
How was this tested:
test_tool_options_go_to_bind_toolswraps the test's_ToolBindingModelinMock(wraps=...)and checks thatbind_toolsreceivestool_choiceandparallel_tool_calls, separately and together, while other options still go through.bind(). On main all three cases fail; with the change they pass.test_tool_options_use_provider_formatbuilds the real Anthropic request payload offline:'any'becomes{'type': 'any'}, andparallel_tool_calls=Falsebecomesdisable_parallel_tool_use. On main both cases fail.make lintpasses. Plugin suite: 98 passed, none skipped (main: 93 passed).Any docs updates needed? No.
I used an AI assistant while tracing this and writing the tests; I reviewed every line and ran the tests myself.