Skip to content

Commit e8fc991

Browse files
authored
callback handler - fix reporting of tool when missing delta (strands-agents#1573)
1 parent 694c4a7 commit e8fc991

2 files changed

Lines changed: 31 additions & 82 deletions

File tree

src/strands/handlers/callback_handler.py

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ def __init__(self, verbose_tool_use: bool = True) -> None:
1414
verbose_tool_use: Print out verbose information about tool calls.
1515
"""
1616
self.tool_count = 0
17-
self.previous_tool_use = None
1817
self._verbose_tool_use = verbose_tool_use
1918

2019
def __call__(self, **kwargs: Any) -> None:
@@ -25,26 +24,24 @@ def __call__(self, **kwargs: Any) -> None:
2524
- reasoningText (Optional[str]): Reasoning text to print if provided.
2625
- data (str): Text content to stream.
2726
- complete (bool): Whether this is the final chunk of a response.
28-
- current_tool_use (dict): Information about the current tool being used.
27+
- event (dict): ModelStreamChunkEvent.
2928
"""
3029
reasoningText = kwargs.get("reasoningText", False)
3130
data = kwargs.get("data", "")
3231
complete = kwargs.get("complete", False)
33-
current_tool_use = kwargs.get("current_tool_use", {})
32+
tool_use = kwargs.get("event", {}).get("contentBlockStart", {}).get("start", {}).get("toolUse")
3433

3534
if reasoningText:
3635
print(reasoningText, end="")
3736

3837
if data:
3938
print(data, end="" if not complete else "\n")
4039

41-
if current_tool_use and current_tool_use.get("name"):
42-
if self.previous_tool_use != current_tool_use:
43-
self.previous_tool_use = current_tool_use
44-
self.tool_count += 1
45-
if self._verbose_tool_use:
46-
tool_name = current_tool_use.get("name", "Unknown tool")
47-
print(f"\nTool #{self.tool_count}: {tool_name}")
40+
if tool_use:
41+
self.tool_count += 1
42+
if self._verbose_tool_use:
43+
tool_name = tool_use["name"]
44+
print(f"\nTool #{self.tool_count}: {tool_name}")
4845

4946
if complete and data:
5047
print("\n")

tests/strands/handlers/test_callback_handler.py

Lines changed: 24 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -72,56 +72,21 @@ def test_call_with_data_complete(handler, mock_print):
7272
mock_print.assert_any_call("\n")
7373

7474

75-
def test_call_with_current_tool_use_new(handler, mock_print):
76-
"""Test calling the handler with a new tool use."""
77-
current_tool_use = {"name": "test_tool", "input": {"param": "value"}}
78-
79-
handler(current_tool_use=current_tool_use)
80-
81-
# Should print tool information
82-
mock_print.assert_called_once_with("\nTool #1: test_tool")
83-
84-
# Should update the handler state
85-
assert handler.tool_count == 1
86-
assert handler.previous_tool_use == current_tool_use
87-
88-
89-
def test_call_with_current_tool_use_same(handler, mock_print):
90-
"""Test calling the handler with the same tool use twice."""
91-
current_tool_use = {"name": "test_tool", "input": {"param": "value"}}
92-
93-
# First call
94-
handler(current_tool_use=current_tool_use)
95-
mock_print.reset_mock()
96-
97-
# Second call with same tool use
98-
handler(current_tool_use=current_tool_use)
99-
100-
# Should not print tool information again
101-
mock_print.assert_not_called()
102-
103-
# Tool count should not increase
104-
assert handler.tool_count == 1
105-
106-
107-
def test_call_with_current_tool_use_different(handler, mock_print):
75+
def test_call_with_tool_uses(handler, mock_print):
10876
"""Test calling the handler with different tool uses."""
109-
first_tool_use = {"name": "first_tool", "input": {"param": "value1"}}
110-
second_tool_use = {"name": "second_tool", "input": {"param": "value2"}}
111-
112-
# First call
113-
handler(current_tool_use=first_tool_use)
114-
mock_print.reset_mock()
77+
first_event = {"contentBlockStart": {"start": {"toolUse": {"name": "first_tool"}}}}
78+
second_event = {"contentBlockStart": {"start": {"toolUse": {"name": "second_tool"}}}}
11579

116-
# Second call with different tool use
117-
handler(current_tool_use=second_tool_use)
80+
handler(event=first_event)
81+
handler(event=second_event)
11882

119-
# Should print info for the new tool
120-
mock_print.assert_called_once_with("\nTool #2: second_tool")
83+
assert mock_print.call_args_list == [
84+
unittest.mock.call("\nTool #1: first_tool"),
85+
unittest.mock.call("\nTool #2: second_tool"),
86+
]
12187

12288
# Tool count should increase
12389
assert handler.tool_count == 2
124-
assert handler.previous_tool_use == second_tool_use
12590

12691

12792
def test_call_with_data_and_complete_extra_newline(handler, mock_print):
@@ -146,42 +111,30 @@ def test_call_with_message_no_effect(handler, mock_print):
146111

147112
def test_call_with_multiple_parameters(handler, mock_print):
148113
"""Test calling handler with multiple parameters."""
149-
current_tool_use = {"name": "test_tool", "input": {"param": "value"}}
114+
event = {"contentBlockStart": {"start": {"toolUse": {"name": "test_tool"}}}}
150115

151-
handler(data="Test output", complete=True, current_tool_use=current_tool_use)
116+
handler(data="Test output", complete=True, event=event)
152117

153-
# Should print data with newline, an extra newline for completion, and tool information
154-
assert mock_print.call_count == 3
155-
mock_print.assert_any_call("Test output", end="\n")
156-
mock_print.assert_any_call("\n")
157-
mock_print.assert_any_call("\nTool #1: test_tool")
158-
159-
160-
def test_unknown_tool_name_handling(handler, mock_print):
161-
"""Test handling of a tool use without a name."""
162-
# The SDK implementation doesn't have a fallback for tool uses without a name field
163-
# It checks for both presence of current_tool_use and current_tool_use.get("name")
164-
current_tool_use = {"input": {"param": "value"}, "name": "Unknown tool"}
165-
166-
handler(current_tool_use=current_tool_use)
167-
168-
# Should print the tool information
169-
mock_print.assert_called_once_with("\nTool #1: Unknown tool")
118+
# Should print data with newline, tool information, and an extra newline for completion
119+
assert mock_print.call_args_list == [
120+
unittest.mock.call("Test output", end="\n"),
121+
unittest.mock.call("\nTool #1: test_tool"),
122+
unittest.mock.call("\n"),
123+
]
170124

171125

172126
def test_tool_use_empty_object(handler, mock_print):
173-
"""Test handling of an empty tool use object."""
127+
"""Test handling of an empty tool use object in event."""
174128
# Tool use is an empty dict
175-
current_tool_use = {}
129+
event = {"contentBlockStart": {"start": {"toolUse": {}}}}
176130

177-
handler(current_tool_use=current_tool_use)
131+
handler(event=event)
178132

179133
# Should not print anything
180134
mock_print.assert_not_called()
181135

182136
# Should not update state
183137
assert handler.tool_count == 0
184-
assert handler.previous_tool_use is None
185138

186139

187140
def test_composite_handler_forwards_to_all_handlers():
@@ -193,7 +146,7 @@ def test_composite_handler_forwards_to_all_handlers():
193146
kwargs = {
194147
"data": "Test output",
195148
"complete": True,
196-
"current_tool_use": {"name": "test_tool", "input": {"param": "value"}},
149+
"event": {"contentBlockStart": {"start": {"toolUse": {"name": "test_tool"}}}},
197150
}
198151

199152
# Call the composite handler
@@ -215,12 +168,11 @@ def test_verbose_tool_use_disabled(mock_print):
215168
handler = PrintingCallbackHandler(verbose_tool_use=False)
216169
assert handler._verbose_tool_use is False
217170

218-
current_tool_use = {"name": "test_tool", "input": {"param": "value"}}
219-
handler(current_tool_use=current_tool_use)
171+
event = {"contentBlockStart": {"start": {"toolUse": {"name": "test_tool"}}}}
172+
handler(event=event)
220173

221174
# Should not print tool information when verbose_tool_use is False
222175
mock_print.assert_not_called()
223176

224-
# Should still update tool count and previous_tool_use
177+
# Should still update tool count
225178
assert handler.tool_count == 1
226-
assert handler.previous_tool_use == current_tool_use

0 commit comments

Comments
 (0)