mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
## Description
`HeadroomAgnoModel` blows up as soon as you stream a response that
includes a tool call:
```
ERROR Error in Agent run: 'ChoiceDeltaToolCall' object has no attribute 'get'
```
When streaming, Agno hands us `Message.tool_calls` as the raw OpenAI SDK
objects (`ChoiceDeltaToolCall`), not the OpenAI-style dicts we get on
the non-streaming path. Those objects are pydantic models — attribute
access only, no `.get()`. Our shared parser in `headroom/parser.py`
walks `tool_calls` and calls `tc.get("function", {})` / `tc.get("id")`,
so it throws `AttributeError`, and the Agno wrapper surfaces that as a
`RunErrorEvent` that kills the run.
I reproduced the exact error against `parse_message_to_blocks` with a
stand-in object before writing the fix.
Closes #1312
## Type of Change
- [x] Bug fix (non-breaking change that fixes an issue)
- [ ] New feature (non-breaking change that adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
- [ ] Performance improvement
- [ ] Code refactoring (no functional changes)
## Changes Made
- `parser.py`: added a small `_coerce_tool_call_to_dict()` helper that
takes a tool_call which might be a dict or a provider SDK object and
returns the canonical OpenAI dict (reading `.function.name` /
`.function.arguments` via `getattr`). Wired it into both `.get()` sites,
`parse_message_to_blocks` and `find_tool_units`. Dicts pass straight
through (same object, no copy); `None` or anything unexpected degrades
to `{}` instead of raising. The proxy, langchain, and strands
integrations go through this same parser, so they get the same
hardening.
- `integrations/agno/model.py`: normalize `tool_calls` to dicts in
`_convert_messages_to_openai`, so the Agno `Message` objects we rebuild
and hand back also carry clean dicts and Agno's own re-serialization
can't trip over the same thing.
## Testing
- [x] Unit tests pass (`pytest`)
- [ ] Linting passes (`ruff check .`)
- [ ] Type checking passes (`mypy headroom`)
- [x] New tests added for new functionality
- [ ] Manual testing performed
### Test Output
```text
$ python -m pytest tests/test_parser.py -q
93 passed
# 87 existing + 6 new regression tests in TestStreamingToolCallObjects.
$ python -m pytest tests/test_integrations/agno/test_model.py -q
59 skipped
# These skip locally because agno isn't installed here
# (pytestmark = skipif(not AGNO_AVAILABLE)); they run in CI. The new
# test_convert_messages_normalizes_streaming_tool_call_objects is in this file.
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.10, local clone. agno and the Rust
`headroom._core` extension aren't installed/built in this checkout.
- Exact command / steps: built a stand-in `ChoiceDeltaToolCall`
(attribute access, no `.get()`, nested `.function.name`/`.arguments`)
matching the OpenAI SDK streaming type, ran it through
`parse_message_to_blocks` and `find_tool_units` before and after the
change, then ran the parser suite.
- Observed result: before the fix I got `AttributeError:
'ChoiceDeltaToolCall' object has no attribute 'get'` — the exact error
from the issue. After the fix the same input produces a proper
`tool_call` block (correct `tool_call_id` / `function_name`) and
`find_tool_units` pairs the assistant call with its tool response.
Parser suite is green at 93 passed.
- Not tested: a full live `agent.run(stream=True)` against a real
OpenAI-compatible backend, since agno isn't installed here. That path is
covered by the Agno test in CI. I reproduced the failure at the parser
boundary instead, which is where the actual crash happens.
## Review Readiness
- [x] I have performed a self-review
- [x] This PR is ready for human review
## Checklist
- [x] My code follows the project's style guidelines
- [x] I have performed a self-review of my code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] New and existing unit tests pass locally with my changes
- [ ] I have updated the CHANGELOG.md if applicable
## Additional Notes
- No docs change — this is an internal robustness fix at the parsing
boundary, no user-facing API.
- CHANGELOG.md is generated from the Conventional Commit subject via
release-please, so the `fix(agno):` commit gets picked up on its own.
- I went with two layers (parser + the Agno boundary) on purpose so
neither our pipeline nor Agno's re-serialization can hit it. Since the
parser helper is shared, the proxy/langchain/strands paths are covered
too.
This commit is contained in:
Lakshya Sharma
GitHub
parent
52068dd650
commit
5986c2260f
4 changed files with 190 additions and 4 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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])
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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 ---
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue