test(agents): cover Bedrock Converse toolUse arg parsing#6192
test(agents): cover Bedrock Converse toolUse arg parsing#6192HumphreySun98 wants to merge 1 commit into
Conversation
Bedrock's Converse API returns tool calls as
{"toolUseId": ..., "name": ..., "input": {...}} with no "function" wrapper
or "arguments" key, so _parse_native_tool_call must read "input". This path
had no offline coverage — the existing Bedrock tests are gated behind live
AWS credentials — leaving the arg extraction unprotected against regression
(see crewAIInc#4972, where Converse args were dropped and tools received {}).
Add credential-free unit tests asserting the args are extracted and reach
the tool with their real values.
Refs crewAIInc#4972
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughTwo regression tests are added to ChangesBedrock Converse toolUse regression tests
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
Bedrock's Converse API returns tool calls shaped as
{"toolUseId": ..., "name": ..., "input": {...}}— nofunctionwrapper and noargumentskey — soCrewAgentExecutor._parse_native_tool_callmust readinput. Today it does (func_info.get("arguments") or tool_call.get("input", {})), but this path has no offline coverage: the existing Bedrock tests are gated behind live AWS credentials (RUN_BEDROCK_LIVE_TESTS), leaving the arg extraction unprotected against regression.This is exactly what #4972 reported (Converse args dropped, every tool receiving
{}). The bug is already fixed atmain; this PR locks the behavior in.Tests
Adds two credential-free unit tests to
TestNativeToolCallingJsonParseError:test_bedrock_converse_tooluse_args_pass_through— a raw ConversetoolUsedict yields itsinputas the args (not{}).test_bedrock_converse_tooluse_args_reach_the_tool— those args actually execute the tool with real values.Both would have failed under the buggy behavior described in the issue.
Refs #4972
This PR was authored with Claude Code. Per
CONTRIBUTING.md, AI-generated contributions require thellm-generatedlabel — I don't have triage permission to set it, so could a maintainer please add it? 🤖 Generated with Claude CodeSummary by CodeRabbit