Skip to content

Commit c00e322

Browse files
kimnamuclaudeVidit-Ostwal
authored
fix(bedrock): preserve streaming tool call arguments at contentBlockStop (#6150)
* fix(bedrock): preserve streaming tool call arguments at contentBlockStop 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 #5415 (issue #4972). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(bedrock): coerce non-dict streaming tool input to empty dict 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 #6150. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Vidit Ostwal <110953813+Vidit-Ostwal@users.noreply.github.com>
1 parent a024115 commit c00e322

2 files changed

Lines changed: 208 additions & 0 deletions

File tree

lib/crewai/src/crewai/llms/providers/bedrock/completion.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1040,6 +1040,19 @@ def _handle_streaming_converse(
10401040
logging.debug("Content block stopped in stream")
10411041
if current_tool_use:
10421042
function_name = current_tool_use["name"]
1043+
# Streamed tool input arrives as JSON string deltas in
1044+
# accumulated_tool_input; fold it back into the tool-use
1045+
# block so function_args (and the message history below)
1046+
# carry the real arguments instead of an empty input.
1047+
try:
1048+
parsed_input = json.loads(accumulated_tool_input)
1049+
current_tool_use["input"] = (
1050+
parsed_input
1051+
if isinstance(parsed_input, dict)
1052+
else {}
1053+
)
1054+
except (json.JSONDecodeError, ValueError, TypeError):
1055+
current_tool_use["input"] = {}
10431056
function_args = cast(
10441057
dict[str, Any], current_tool_use.get("input", {})
10451058
)
@@ -1638,6 +1651,19 @@ async def _ahandle_streaming_converse(
16381651
logging.debug("Content block stopped in stream")
16391652
if current_tool_use:
16401653
function_name = current_tool_use["name"]
1654+
# Streamed tool input arrives as JSON string deltas in
1655+
# accumulated_tool_input; fold it back into the tool-use
1656+
# block so function_args (and the message history below)
1657+
# carry the real arguments instead of an empty input.
1658+
try:
1659+
parsed_input = json.loads(accumulated_tool_input)
1660+
current_tool_use["input"] = (
1661+
parsed_input
1662+
if isinstance(parsed_input, dict)
1663+
else {}
1664+
)
1665+
except (json.JSONDecodeError, ValueError, TypeError):
1666+
current_tool_use["input"] = {}
16411667
function_args = cast(
16421668
dict[str, Any], current_tool_use.get("input", {})
16431669
)
Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
"""Regression tests for streaming Bedrock tool-call argument handling.
2+
3+
The streaming Converse handlers deliver tool input as a sequence of JSON
4+
string deltas (``contentBlockDelta`` -> ``toolUse.input``) that are
5+
accumulated separately from the tool-use block. These tests assert that the
6+
accumulated input is folded back into the tool call at ``contentBlockStop``,
7+
so executed tools receive their real arguments instead of an empty ``{}``.
8+
9+
This is the streaming counterpart of the non-streaming fix in #5415
10+
(issue #4972).
11+
"""
12+
13+
import os
14+
from unittest.mock import MagicMock, patch
15+
16+
import pytest
17+
18+
from crewai.llm import LLM
19+
from crewai.llms.providers.bedrock.completion import BedrockCompletion
20+
21+
22+
def _make_tool_use_stream() -> list[dict]:
23+
"""Synthetic Converse stream: a single tool call with JSON-chunked input."""
24+
# Tool input is delivered as two partial JSON string fragments that only
25+
# form valid JSON once concatenated: '{"city":' + ' "Paris"}'.
26+
chunk1 = '{"city":'
27+
chunk2 = ' "Paris"}'
28+
return [
29+
{"messageStart": {"role": "assistant"}},
30+
{
31+
"contentBlockStart": {
32+
"start": {"toolUse": {"toolUseId": "tool-1", "name": "get_weather"}},
33+
"contentBlockIndex": 0,
34+
}
35+
},
36+
{"contentBlockDelta": {"delta": {"toolUse": {"input": chunk1}}}},
37+
{"contentBlockDelta": {"delta": {"toolUse": {"input": chunk2}}}},
38+
{"contentBlockStop": {}},
39+
{"messageStop": {"stopReason": "tool_use"}},
40+
]
41+
42+
43+
def _make_non_dict_tool_use_stream() -> list[dict]:
44+
"""Synthetic Converse stream whose tool input is valid JSON but not an object.
45+
46+
``json.loads`` succeeds here (returns a string), so the parsed value must
47+
still be coerced to a dict before it reaches ``fn(**function_args)``.
48+
"""
49+
return [
50+
{"messageStart": {"role": "assistant"}},
51+
{
52+
"contentBlockStart": {
53+
"start": {"toolUse": {"toolUseId": "tool-1", "name": "get_weather"}},
54+
"contentBlockIndex": 0,
55+
}
56+
},
57+
{"contentBlockDelta": {"delta": {"toolUse": {"input": '"oops"'}}}},
58+
{"contentBlockStop": {}},
59+
{"messageStop": {"stopReason": "tool_use"}},
60+
]
61+
62+
63+
def _build_completion() -> BedrockCompletion:
64+
"""Build a BedrockCompletion with mocked AWS credentials/session."""
65+
with patch.dict(
66+
os.environ,
67+
{
68+
"AWS_ACCESS_KEY_ID": "test-access-key",
69+
"AWS_SECRET_ACCESS_KEY": "test-secret-key",
70+
"AWS_DEFAULT_REGION": "us-east-1",
71+
},
72+
):
73+
with patch("crewai.llms.providers.bedrock.completion.Session"):
74+
llm = LLM(model="bedrock/anthropic.claude-3-5-sonnet-20241022-v2:0")
75+
assert isinstance(llm, BedrockCompletion)
76+
return llm
77+
78+
79+
def test_streaming_tool_call_preserves_arguments():
80+
"""Sync streaming: function_args must carry the streamed tool input."""
81+
llm = _build_completion()
82+
83+
captured: dict = {}
84+
85+
def capture(function_args, **kwargs):
86+
captured["args"] = function_args
87+
return None # returning None stops the recursive _handle_converse call
88+
89+
mock_client = MagicMock()
90+
mock_client.converse_stream.return_value = {"stream": _make_tool_use_stream()}
91+
92+
with (
93+
patch.object(llm, "_get_sync_client", return_value=mock_client),
94+
patch.object(llm, "_handle_tool_execution", side_effect=capture),
95+
):
96+
llm._handle_streaming_converse(
97+
messages=[{"role": "user", "content": "weather in Paris?"}],
98+
body={},
99+
available_functions={"get_weather": lambda **kw: "sunny"},
100+
)
101+
102+
assert captured["args"] == {"city": "Paris"}
103+
104+
105+
@pytest.mark.asyncio
106+
async def test_async_streaming_tool_call_preserves_arguments():
107+
"""Async streaming: function_args must carry the streamed tool input."""
108+
llm = _build_completion()
109+
110+
class _AsyncStream:
111+
def __init__(self, events):
112+
self._events = events
113+
114+
def __aiter__(self):
115+
self._it = iter(self._events)
116+
return self
117+
118+
async def __anext__(self):
119+
try:
120+
return next(self._it)
121+
except StopIteration:
122+
raise StopAsyncIteration
123+
124+
async def _converse_stream(**kwargs):
125+
return {"stream": _AsyncStream(_make_tool_use_stream())}
126+
127+
mock_async_client = MagicMock()
128+
mock_async_client.converse_stream = _converse_stream
129+
130+
async def _ensure(*args, **kwargs):
131+
return mock_async_client
132+
133+
captured: dict = {}
134+
135+
def capture(function_args, **kwargs):
136+
captured["args"] = function_args
137+
return None
138+
139+
with (
140+
patch.object(llm, "_ensure_async_client", side_effect=_ensure),
141+
patch.object(llm, "_handle_tool_execution", side_effect=capture),
142+
):
143+
await llm._ahandle_streaming_converse(
144+
messages=[{"role": "user", "content": "weather in Paris?"}],
145+
body={},
146+
available_functions={"get_weather": lambda **kw: "sunny"},
147+
)
148+
149+
assert captured["args"] == {"city": "Paris"}
150+
151+
152+
def test_streaming_non_dict_tool_input_coerced_to_empty_dict():
153+
"""Valid-but-non-object JSON input must be coerced to ``{}``.
154+
155+
``json.loads('"oops"')`` returns a string; passing it on as
156+
``fn(**function_args)`` would raise ``TypeError``. The handler must
157+
guard against this and fall back to an empty dict.
158+
"""
159+
llm = _build_completion()
160+
161+
captured: dict = {}
162+
163+
def capture(function_args, **kwargs):
164+
captured["args"] = function_args
165+
return None
166+
167+
mock_client = MagicMock()
168+
mock_client.converse_stream.return_value = {
169+
"stream": _make_non_dict_tool_use_stream()
170+
}
171+
172+
with (
173+
patch.object(llm, "_get_sync_client", return_value=mock_client),
174+
patch.object(llm, "_handle_tool_execution", side_effect=capture),
175+
):
176+
llm._handle_streaming_converse(
177+
messages=[{"role": "user", "content": "weather in Paris?"}],
178+
body={},
179+
available_functions={"get_weather": lambda **kw: "sunny"},
180+
)
181+
182+
assert captured["args"] == {}

0 commit comments

Comments
 (0)