mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-10 14:27:00 -04:00
fix(ccr): don't crash tool-call detection on a null function/functionCall (#2269)
## Description
CCR tool-call detection crashes when an upstream response carries a tool
call whose `function` (or `functionCall`) field is explicitly `null`.
`is_ccr_tool_call` and `parse_tool_call` both read the nested name like
this:
```python
tool_call.get("function", {}).get("name")
tool_call.get("functionCall", {}).get("name")
```
`dict.get("function", {})` only substitutes `{}` when the key is
**missing**. When the key is present but `null` — `{"id": "call_1",
"type": "function", "function": null}`, which upstreams (and gateways
like LiteLLM/OpenRouter) emit for a partial or streamed tool call — the
result is `None`, and `None.get("name")` raises `AttributeError`.
These functions run over the untrusted upstream response
(`has_ccr_tool_calls` → `is_ccr_tool_call` for every tool call, and
`parse_tool_call` on the retrieve path), so a single malformed tool call
takes down CCR detection for the whole response. The sibling
`tool_call_id_for_provider` in the same module already guards this shape
(`if isinstance(function_call, dict)`); these two paths just weren't
updated to match.
## Fix
Coalesce with `or {}` so a `null` (or any falsy) value collapses to
`{}`:
```python
(tool_call.get("function") or {}).get("name")
(tool_call.get("functionCall") or {}).get("name")
```
and in `parse_tool_call`:
```python
function = tool_call.get("function") or {}
function_call = tool_call.get("functionCall") or {}
```
A null tool call now reports "not a CCR call" and is passed through as a
normal tool, and real CCR calls are still detected.
Closes #
## 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
- `headroom/ccr/tool_calls.py`: `is_ccr_tool_call` coalesces `function`
/ `functionCall` with `or {}`.
- `headroom/ccr/tool_injection.py`: `parse_tool_call` coalesces
`function` (openai) and `functionCall` (google) with `or {}`.
- `tests/test_ccr_tool_calls.py`, `tests/test_ccr_tool_injection.py`:
new tests covering a null-function tool call in detection and parsing.
- `CHANGELOG.md`: Bug Fixes entry.
## Testing
- [ ] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check .`)
- [x] Type checking passes (`mypy headroom`)
- [x] New tests added for new functionality
- [ ] Manual testing performed
### Test Output
```text
$ uvx ruff@0.15.17 check headroom/ccr/tool_calls.py headroom/ccr/tool_injection.py tests/test_ccr_tool_calls.py tests/test_ccr_tool_injection.py
All checks passed!
$ uvx mypy@1.20.2 --ignore-missing-imports headroom/ccr/tool_calls.py headroom/ccr/tool_injection.py
Success: no issues found in 2 source files
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.12, `uvx ruff@0.15.17` / `uvx
mypy@1.20.2`. A full `pytest` OOM-kills this box (ML stack import), so I
reproduced the detection logic with a dependency-free script and left
the full pytest to CI.
- Exact command / steps: ran an OpenAI tool call `{"function": null}`
(plus a real CCR call) through the OLD `get("function", {})` form and
the NEW `get("function") or {}` form.
- Observed result: OLD raises `AttributeError` on the null function; NEW
returns `False`/`None` for it and still detects the real CCR call and
both `functionCall`/`name` shapes.
- Not tested: a live upstream emitting a null-function tool call; full
local `pytest` deferred to CI (OOM).
## 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
- [ ] New and existing unit tests pass locally with my changes
- [x] I have updated the CHANGELOG.md if applicable
## Additional Notes
The "unit tests pass locally" box is unchecked because the full suite
imports the ML stack, which I can't run here. The new tests live
alongside the existing CCR tool-call tests so they run under the normal
CI pytest job; behaviour is additionally verified by the standalone
proof above.
---------
Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
This commit is contained in:
parent
8b7e797ed4
commit
1612f06a4c
4 changed files with 44 additions and 4 deletions
|
|
@ -73,8 +73,8 @@ def is_ccr_tool_call(tool_call: dict[str, Any]) -> bool:
|
|||
"""Return true when a provider-native tool call names the CCR retrieval tool."""
|
||||
return (
|
||||
tool_call.get("name") == CCR_TOOL_NAME
|
||||
or tool_call.get("function", {}).get("name") == CCR_TOOL_NAME
|
||||
or tool_call.get("functionCall", {}).get("name") == CCR_TOOL_NAME
|
||||
or (tool_call.get("function") or {}).get("name") == CCR_TOOL_NAME
|
||||
or (tool_call.get("functionCall") or {}).get("name") == CCR_TOOL_NAME
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -467,7 +467,10 @@ def parse_tool_call(
|
|||
name = tool_call.get("name")
|
||||
input_data = tool_call.get("input", {})
|
||||
elif provider == "openai":
|
||||
function = tool_call.get("function", {})
|
||||
# `get("function", {})` returns None for an explicit {"function": null}
|
||||
# (the default only applies to a missing key), so `.get` below would
|
||||
# raise AttributeError on a malformed/partial tool call. Coalesce to {}.
|
||||
function = tool_call.get("function") or {}
|
||||
name = function.get("name")
|
||||
# OpenAI passes args as JSON string
|
||||
args_str = function.get("arguments", "{}")
|
||||
|
|
@ -478,7 +481,8 @@ def parse_tool_call(
|
|||
input_data = {}
|
||||
elif provider == "google":
|
||||
# Google/Gemini format: {"functionCall": {"name": "...", "args": {...}}}
|
||||
function_call = tool_call.get("functionCall", {})
|
||||
# Coalesce to {} so an explicit {"functionCall": null} does not crash.
|
||||
function_call = tool_call.get("functionCall") or {}
|
||||
name = function_call.get("name")
|
||||
input_data = function_call.get("args", {})
|
||||
elif provider == "openai_responses":
|
||||
|
|
|
|||
|
|
@ -58,6 +58,36 @@ def test_has_ccr_tool_calls_uses_provider_native_names() -> None:
|
|||
)
|
||||
|
||||
|
||||
def test_ccr_detection_survives_null_function_tool_call() -> None:
|
||||
# A partial/streamed OpenAI tool call with an explicit {"function": null}
|
||||
# must not crash detection: dict.get("function", {}) returns None for a
|
||||
# present-but-null key, and .get on None raises AttributeError.
|
||||
response = {
|
||||
"choices": [
|
||||
{
|
||||
"message": {
|
||||
"tool_calls": [
|
||||
{"id": "call_1", "type": "function", "function": None},
|
||||
{
|
||||
"id": "call_2",
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": CCR_TOOL_NAME,
|
||||
"arguments": '{"hash": "' + HASH + '"}',
|
||||
},
|
||||
},
|
||||
]
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
assert has_ccr_tool_calls(response, "openai")
|
||||
ccr_calls, other_calls = parse_ccr_tool_calls(response, "openai")
|
||||
assert ccr_calls == [CCRToolCall(tool_call_id="call_2", hash_key=HASH)]
|
||||
assert other_calls == [{"id": "call_1", "type": "function", "function": None}]
|
||||
|
||||
|
||||
def test_parse_ccr_tool_calls_splits_retrievals_from_other_tools() -> None:
|
||||
response = {
|
||||
"content": [
|
||||
|
|
|
|||
|
|
@ -310,6 +310,12 @@ class TestParseToolCall:
|
|||
|
||||
assert hash_key == "def456abc123def456abc123"
|
||||
|
||||
def test_parse_null_function_returns_none_without_crashing(self):
|
||||
"""A tool call with an explicit {"function": null} / {"functionCall": null}
|
||||
must return None, not raise AttributeError."""
|
||||
assert parse_tool_call({"id": "c1", "function": None}, "openai") is None
|
||||
assert parse_tool_call({"functionCall": None}, "google") is None
|
||||
|
||||
def test_parse_normalises_uppercase_hash_to_lowercase(self):
|
||||
"""An uppercase hash echoed by the model must be lowercased so it
|
||||
matches the store (which keys entries by a lowercase hash)."""
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue