fix(bedrock): preserve streaming tool call arguments at contentBlockStop - #6150
Conversation
Streaming Converse handlers accumulate tool input as JSON string deltas in
accumulated_tool_input but never fold it back into current_tool_use["input"],
so function_args reads an empty {} at contentBlockStop. Parse the accumulated
input into the tool-use block (with a {} fallback) in both the sync and async
streaming handlers. This is the streaming counterpart of the non-streaming fix
in crewAIInc#5415 (issue crewAIInc#4972).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughSync and async Bedrock streaming handlers now parse accumulated JSON-string tool input at ChangesBedrock Streaming Tool Call Argument Parsing
Merge Risk: ⚪ Minimal · up to The change preserves streamed Bedrock tool arguments for sync and async calls while safely coercing non-object JSON input to empty arguments. No concrete unresolved merge risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
Note for maintainers: per CONTRIBUTING, this PR was prepared with the help of an AI agent (reviewed by a human before submission). I don't have permission to apply the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/crewai/src/crewai/llms/providers/bedrock/completion.py`:
- Around line 1041-1049: The parsed tool input from
json.loads(accumulated_tool_input) can be a non-dict (e.g., list, string), which
will make function_args invalid for fn(**function_args); change the code that
assigns current_tool_use["input"] so that after json.loads it enforces a dict
shape (if the result is not an instance of dict, set current_tool_use["input"] =
{}), and update the subsequent function_args = cast(dict[str, Any],
current_tool_use.get("input", {})) logic accordingly; apply this same guard in
both the synchronous and asynchronous handlers (the sites that set
current_tool_use["input"] from json.loads and then build function_args) so both
paths validate the parsed JSON before use.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2c7ea93c-7855-4356-909f-becfeca420e5
📒 Files selected for processing (2)
lib/crewai/src/crewai/llms/providers/bedrock/completion.pylib/crewai/tests/llms/bedrock/test_bedrock_streaming_tool_args.py
json.loads on the accumulated tool input can return a valid-but-non-object JSON value (e.g. a string or list), which would fail at fn(**function_args) with a TypeError. Enforce a dict shape before use in both the sync and async streaming handlers, and add a regression test for the non-dict case. Addresses CodeRabbit review feedback on crewAIInc#6150.
|
Thanks for the careful review @coderabbitai — good catch. I've applied the suggested guard in both the sync and async handlers (coerce to |
|
Tip For best results, initiate chat on the files or code changes.
|
|
This PR is stale because it has been open for 45 days with no activity. |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
Still relevant on current
Happy to rebase or trim scope if preferred. |
|
Thanks for the pull request. First-time contributors need an associated open issue before we can review a PR.
See the contributing guide. |
|
Heads up on why this closed — it wasn't the stale bot.
I can't reopen #6149 or this PR myself (no permission on either), so I re-filed the bug as #7243 and pointed the body here at it. The gate passes against that issue now. If you still want this branch, a reopen from your side should be all it takes — the branch is up to date with If you'd rather take one of the other PRs for the same bug (#5739 from 2026-05-07, or #6151), that's fine too — I listed all three in #7243 so it's easy to pick one and close the rest. |
|
On the red #7244 bumps all three and its pip-audit is green; once that (or an equivalent bump) lands, a re-run of the check here should pass. I've left the dependency bump out of this PR to keep it scoped to the streaming fix. Tests, lint and type-check are green on the current head (1074788). |
Yup, already working on a PR to resolve that. |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
Thanks a lot @kimnamu |
2 participants
Thanks to the crewAI maintainers for the native Bedrock provider — this is a small follow-up in the same spirit as #5415.
Fixes #7243 (re-filed from #6149, which the stale bot auto-closed on 2026-07-20 while this PR was pending;
the first-time-contributor check then closed this PR because its linked issue was no longer open)
Problem
With
bedrock/...+stream=True, native tool calls reach the tool executor with empty arguments, causing pydanticField requirederrors.Root cause
In
_handle_streaming_converse/_ahandle_streaming_converse, tool input is delivered as JSON-string deltas accumulated into a localaccumulated_tool_input, but it is never folded back intocurrent_tool_use["input"]. AtcontentBlockStop,function_args = current_tool_use.get("input", {})therefore returns{}. The non-streaming path already reads the completedinputdirectly — only the streaming twin was missing this.Fix
Parse
accumulated_tool_inputintocurrent_tool_use["input"](with a{}fallback) atcontentBlockStop, in both sync and async handlers. ~3 lines each; fixes bothfunction_argsand the message-history append in one place.{}(Field required errors) ❌{"city": "Paris"}✅Tests
Added
test_bedrock_streaming_tool_args.py(sync + async, synthetic Converse stream). Verified it catches the bug:assert {} == {'city': 'Paris'}ruff/mypyclean; full bedrock suite 37 passed / 8 skipped (skips are VCR-gated async tests), no regressions.Relation to other PRs
This is the streaming counterpart of #5415 (which fixed the non-streaming path described in #4972). Open PR #5739 attempts a similar streaming fix but is stalled (no reviews, no tests) and references the non-streaming issue; this PR is scoped to the streaming bug and adds regression coverage.
This contribution was prepared with the help of an AI agent (Claude Code); a human reviewed the change, rationale, and test results before submission.