diff --git a/headroom/integrations/agno/model.py b/headroom/integrations/agno/model.py index f6a7abb04..136f53b41 100644 --- a/headroom/integrations/agno/model.py +++ b/headroom/integrations/agno/model.py @@ -30,6 +30,7 @@ except ImportError: ModelResponse = dict # type: ignore[misc,assignment] from headroom import HeadroomConfig, HeadroomMode +from headroom.parser import _coerce_tool_call_to_dict from headroom.providers import OpenAIProvider from headroom.transforms import TransformPipeline @@ -320,9 +321,16 @@ class HeadroomAgnoModel(Model): # type: ignore[misc] else: entry["content"] = content - # Handle tool calls + # Handle tool calls. During streaming, Agno may surface + # tool_calls as raw provider SDK objects (OpenAI's + # `ChoiceDeltaToolCall`) rather than plain dicts. The + # Headroom pipeline + Agno's own re-serialization both call + # `.get()` on each entry, which raises + # `'ChoiceDeltaToolCall' object has no attribute 'get'` + # (issue #1312). Normalize to OpenAI-format dicts here so + # every downstream consumer sees a uniform shape. if hasattr(msg, "tool_calls") and msg.tool_calls: - entry["tool_calls"] = msg.tool_calls + entry["tool_calls"] = [_coerce_tool_call_to_dict(tc) for tc in msg.tool_calls] # Handle tool call ID for tool responses if hasattr(msg, "tool_call_id") and msg.tool_call_id: entry["tool_call_id"] = msg.tool_call_id diff --git a/headroom/parser.py b/headroom/parser.py index 4ef7692d6..a51d435a3 100644 --- a/headroom/parser.py +++ b/headroom/parser.py @@ -53,6 +53,52 @@ def compute_hash(text: str) -> str: return hashlib.md5(text.encode()).hexdigest()[:16] # nosec B324 +def _coerce_tool_call_to_dict(tc: Any) -> dict[str, Any]: + """Normalize a single tool_call into the canonical OpenAI dict shape. + + `tc` is usually already an OpenAI-format dict + (``{"id": ..., "function": {"name": ..., "arguments": ...}}``), but + streaming integrations can hand us the raw provider SDK object instead. + The OpenAI Python SDK's streaming path yields ``ChoiceDeltaToolCall`` + objects (and the non-streaming path ``ChatCompletionMessageToolCall``), + which are Pydantic models with attribute access and NO ``.get()`` — so + calling ``tc.get("function")`` blows up with + ``'ChoiceDeltaToolCall' object has no attribute 'get'`` (issue #1312, + seen via the Agno wrapper streaming tool calls). + + Accept both. Dicts pass through untouched; attribute-style objects are + read via ``getattr`` and flattened to a dict with the same keys the + parser expects (``id`` + nested ``function.name`` / ``function.arguments``). + A nested ``function`` may itself be a dict or an SDK object, so it gets + the same treatment. Anything unrecognized degrades to an empty dict + rather than raising — over-compression of a malformed tool call is far + cheaper than crashing the whole agent run. + """ + if isinstance(tc, dict): + return tc + + # Attribute-style provider object (e.g. OpenAI ChoiceDeltaToolCall). + if tc is None: + return {} + + func = getattr(tc, "function", None) + if isinstance(func, dict): + func_dict = func + elif func is not None: + func_dict = { + "name": getattr(func, "name", None), + "arguments": getattr(func, "arguments", ""), + } + else: + func_dict = {} + + return { + "id": getattr(tc, "id", None), + "type": getattr(tc, "type", "function"), + "function": func_dict, + } + + def _canonical_call_key(name: str, arguments: Any) -> str: """Canonical identity for a tool invocation: name + arguments with JSON key order normalized, so semantically identical calls hash equal even @@ -301,7 +347,8 @@ def parse_message_to_blocks( # Handle tool calls (assistant messages with tool_calls) tool_calls = message.get("tool_calls") if tool_calls: - for tc in tool_calls: + for raw_tc in tool_calls: + tc = _coerce_tool_call_to_dict(raw_tc) func = tc.get("function", {}) tc_text = f"{func.get('name', 'unknown')}({func.get('arguments', '')})" @@ -527,7 +574,8 @@ def find_tool_units(messages: list[dict[str, Any]]) -> list[tuple[int, list[int] # OpenAI format: tool_calls array tool_calls = msg.get("tool_calls") if tool_calls: - for tc in tool_calls: + for raw_tc in tool_calls: + tc = _coerce_tool_call_to_dict(raw_tc) tc_id = tc.get("id") if tc_id and tc_id in tool_response_map: response_indices.append(tool_response_map[tc_id]) diff --git a/tests/test_integrations/agno/test_model.py b/tests/test_integrations/agno/test_model.py index 0f0c97ab7..26545996c 100644 --- a/tests/test_integrations/agno/test_model.py +++ b/tests/test_integrations/agno/test_model.py @@ -270,6 +270,46 @@ class TestHeadroomAgnoModel: assert "tool_calls" in openai_msgs[0] assert openai_msgs[1]["tool_call_id"] == "call_123" + def test_convert_messages_normalizes_streaming_tool_call_objects(self, mock_agno_model): + """Regression for issue #1312: in streaming mode Agno can surface + tool_calls as raw OpenAI SDK objects (`ChoiceDeltaToolCall`) with + attribute access and no `.get()`. `_convert_messages_to_openai` + must flatten them to OpenAI-format dicts so neither the Headroom + pipeline nor Agno's re-serialization hits + `'ChoiceDeltaToolCall' object has no attribute 'get'`.""" + from headroom.integrations.agno import HeadroomAgnoModel + + # Mimic the OpenAI SDK streaming object: attribute access, no .get(). + class _Fn: + def __init__(self, name, arguments): + self.name = name + self.arguments = arguments + + class _ChoiceDeltaToolCall: + def __init__(self, id, name, arguments): + self.id = id + self.index = 0 + self.type = "function" + self.function = _Fn(name, arguments) + + assistant_msg = MagicMock() + assistant_msg.role = "assistant" + assistant_msg.content = "" + assistant_msg.tool_calls = [ + _ChoiceDeltaToolCall("call_999", "dummy_tool", '{"query": "test"}') + ] + assistant_msg.tool_call_id = None + + model = HeadroomAgnoModel(wrapped_model=mock_agno_model) + openai_msgs = model._convert_messages_to_openai([assistant_msg]) + + tool_calls = openai_msgs[0]["tool_calls"] + # Every entry must now be a plain dict, not the SDK object. + assert all(isinstance(tc, dict) for tc in tool_calls) + assert tool_calls[0]["id"] == "call_999" + assert tool_calls[0]["function"]["name"] == "dummy_tool" + assert tool_calls[0]["function"]["arguments"] == '{"query": "test"}' + def test_response_applies_optimization(self, mock_agno_model, sample_messages): """response() applies Headroom optimization.""" from headroom.integrations.agno import HeadroomAgnoModel diff --git a/tests/test_parser.py b/tests/test_parser.py index 22fe0424a..bfd1908ac 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -15,6 +15,7 @@ from unittest.mock import Mock import pytest from headroom.parser import ( + _coerce_tool_call_to_dict, compute_hash, detect_waste_signals, find_tool_units, @@ -24,6 +25,31 @@ from headroom.parser import ( parse_messages, ) +# --- Streaming SDK tool-call objects (issue #1312) --- + + +class _FakeDeltaToolCallFunction: + """Mimics openai.types...ChoiceDeltaToolCallFunction: attribute access, + no `.get()`.""" + + def __init__(self, name: str, arguments: str) -> None: + self.name = name + self.arguments = arguments + + +class _FakeChoiceDeltaToolCall: + """Mimics the OpenAI SDK streaming tool-call object that the Agno + wrapper surfaces. It is a Pydantic-style model — attribute access only, + crucially with NO `.get()` — which is exactly what triggered issue + #1312 (`'ChoiceDeltaToolCall' object has no attribute 'get'`).""" + + def __init__(self, id: str, name: str, arguments: str, index: int = 0) -> None: + self.id = id + self.index = index + self.type = "function" + self.function = _FakeDeltaToolCallFunction(name, arguments) + + # --- Fixtures --- @@ -363,6 +389,70 @@ class TestParseMessageToBlocks: assert blocks[0].tokens_est > 0 +class TestStreamingToolCallObjects: + """Regression coverage for issue #1312: streaming integrations (Agno + over OpenAILike) can hand the parser raw OpenAI SDK `ChoiceDeltaToolCall` + objects instead of OpenAI-format dicts. The parser called `.get()` on + them and crashed the whole agent run with + `'ChoiceDeltaToolCall' object has no attribute 'get'`. Both the parser + call sites must now tolerate attribute-style tool-call objects.""" + + def test_coerce_dict_is_passthrough_identity(self): + d = {"id": "call_1", "function": {"name": "f", "arguments": "{}"}} + # A dict must be returned untouched (same object) — no needless copy. + assert _coerce_tool_call_to_dict(d) is d + + def test_coerce_sdk_object_flattens_to_openai_dict(self): + tc = _FakeChoiceDeltaToolCall("call_1", "search", '{"q": "x"}') + out = _coerce_tool_call_to_dict(tc) + assert out == { + "id": "call_1", + "type": "function", + "function": {"name": "search", "arguments": '{"q": "x"}'}, + } + + def test_coerce_object_with_dict_function(self): + # Some providers nest a dict `function` on an attribute-style object. + class _TC: + id = "call_2" + type = "function" + function = {"name": "g", "arguments": "1"} + + out = _coerce_tool_call_to_dict(_TC()) + assert out["function"] == {"name": "g", "arguments": "1"} + + def test_coerce_none_degrades_to_empty_dict(self): + assert _coerce_tool_call_to_dict(None) == {} + + def test_parse_message_to_blocks_with_sdk_tool_call(self, mock_tokenizer): + """The original crash site: parsing an assistant message whose + tool_calls are SDK objects must produce a tool_call block, not + raise AttributeError.""" + tc = _FakeChoiceDeltaToolCall("call_abc", "dummy_tool", '{"query": "test"}') + msg = {"role": "assistant", "content": "", "tool_calls": [tc]} + + blocks = parse_message_to_blocks(msg, 0, mock_tokenizer) + + tool_call_blocks = [b for b in blocks if b.kind == "tool_call"] + assert len(tool_call_blocks) == 1 + assert tool_call_blocks[0].flags.get("tool_call_id") == "call_abc" + assert tool_call_blocks[0].flags.get("function_name") == "dummy_tool" + assert "dummy_tool" in tool_call_blocks[0].text + + def test_find_tool_units_with_sdk_tool_call(self): + """The second `.get()` site: find_tool_units must still pair an + SDK-object tool_call with its tool response message.""" + tc = _FakeChoiceDeltaToolCall("call_abc", "dummy_tool", "{}") + messages = [ + {"role": "assistant", "content": "", "tool_calls": [tc]}, + {"role": "tool", "content": "result", "tool_call_id": "call_abc"}, + ] + + units = find_tool_units(messages) + + assert units == [(0, [1])] + + # --- TestParseMessages ---