fix: extract Bedrock-style dict tool call arguments correctly (#4495)#4496
Open
devin-ai-integration[bot] wants to merge 2 commits intomainfrom
Open
fix: extract Bedrock-style dict tool call arguments correctly (#4495)#4496devin-ai-integration[bot] wants to merge 2 commits intomainfrom
devin-ai-integration[bot] wants to merge 2 commits intomainfrom
Conversation
The default value '{}' for func_info.get('arguments', '{}') was a truthy
string that prevented the 'or' operator from falling through to
tool_call.get('input') for Bedrock-style dict tool calls. This caused
tool arguments to be silently replaced with an empty dict, resulting in
_run() being called without required arguments.
Changed to: func_info.get('arguments') or tool_call.get('input') or {}
Added regression tests covering Bedrock-style, OpenAI-style, and
multi-argument dict tool calls.
Co-Authored-By: João <joao@crewai.com>
Contributor
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Co-Authored-By: João <joao@crewai.com>
| } | ||
| ] | ||
|
|
||
| result = executor._handle_native_tool_calls( |
| } | ||
| ] | ||
|
|
||
| result = executor._handle_native_tool_calls( |
| } | ||
| ] | ||
|
|
||
| result = executor._handle_native_tool_calls( |
| This reproduces the exact scenario from issue #4495 where a custom wrapper | ||
| around BedrockKBRetrieverTool fails with '_run() missing 1 required positional argument'. | ||
| """ | ||
| class InnerResult: |
| } | ||
| ] | ||
|
|
||
| result = executor._handle_native_tool_calls( |
| } | ||
| ] | ||
|
|
||
| result = executor._handle_native_tool_calls( |
| } | ||
| ] | ||
|
|
||
| result = executor._handle_native_tool_calls( |
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.
fix: extract Bedrock-style dict tool call arguments correctly (#4495)
Summary
One-line fix for a regression where Bedrock-style dict tool calls (
{"toolUseId": "...", "name": "...", "input": {...}}) silently lost their arguments, causing_run()to be called with no args.Root cause: In
_handle_native_tool_calls, the expression:For Bedrock dicts without a
"function"key,func_infois{}, so.get("arguments", "{}")returns the default string"{}"— which is truthy. Theorshort-circuits and the actual arguments fromtool_call["input"]are never read.Fix:
Without a default,
.get("arguments")returnsNone(falsy), allowing proper fallback totool_call.get("input").Review & Testing Checklist for Human
"arguments": ""(empty string, falsy), the fix would now fall through totool_call.get("input")instead of using the empty string. This should not happen in practice (OpenAI always sends valid JSON strings), but worth a mental trace to confirm acceptability.BaseToolwithargs_schema(like theParsedBedrockKBRetrieverToolfrom the issue) to confirm the fix resolves the reported issue. The new unit tests mock the tool call dicts directly and do not go through a real LLM provider."function"."arguments"is present (test_openai_dict_tool_call_passes_argumentscovers this, but worth a manual trace).Recommended test plan: Deploy against a Bedrock-backed agent with a custom
BaseToolthat has required arguments and verify the tool receives its arguments correctly.Notes
executor.messages[-2]to find the tool result message, relying on the internal message ordering (assistant → tool → reasoning prompt). This is correct per current code but couples tests to implementation detail.