mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
fix(parser): detect waste signals in Anthropic tool_result content blocks (#815)
## Description
The dashboard's "What Headroom Removed" panel (waste signals) stays
permanently empty for Anthropic-format traffic.
`parse_message_to_blocks()` only extracted text from content parts with
`type == "text"`, so the `tool_result` blocks that carry the bulk of
agentic conversations (Claude Code, and aider/cursor/copilot in
anthropic mode) were invisible to `detect_waste_signals()`. The pipeline
then reported `waste_signals=None` and `/stats` returned
`"waste_signals": {}` forever.
This PR emits a dedicated `tool_result` Block per Anthropic
`tool_result` content part — handling both string-form content and the
nested text-block-list form — with waste detection and a `tool_call_id`
pairing flag. OpenAI chat-completions behavior is unchanged (parity test
included).
Fixes #813
## Type of Change
- [x] Bug fix (non-breaking change that fixes an issue)
## Changes Made
- `headroom/parser.py`: new `_extract_tool_result_text()` helper;
`parse_message_to_blocks()` collects `tool_result` content parts and
emits a `Block(kind="tool_result")` per part with waste signals and
`tool_call_id` flags
- `tests/test_parser.py`: 7 new tests — nested text-list form, string
form, mixed text+tool_result, empty content, non-text inner blocks,
`parse_messages` aggregation, and waste parity with the OpenAI `role:
"tool"` format
## Testing
- [x] Unit tests pass (`pytest tests/test_parser.py` — 60 passed)
- [x] Linting passes (`ruff check`, `ruff format --check`)
- [x] New tests added for new functionality
- [x] Manual testing performed (real pipeline run below)
Also ran `tests/test_pipeline.py`, `tests/test_canonical_pipeline.py`,
`tests/test_proxy_pipeline_lifecycle.py`: 3 failures there are
pre-existing on a clean `upstream/main` checkout (verified via `git
stash`) and unrelated to this change.
## Test Output
```
$ pytest tests/test_parser.py -q
60 passed in 0.14s
$ ruff check headroom/parser.py tests/test_parser.py
All checks passed!
```
## Real behavior proof
Real `TransformPipeline` (CacheAligner + ContentRouter, same
construction as the proxy server) over an Anthropic-format conversation
with four large JSON `tool_result` blocks:
```
# before this fix
before=37265 after=16013 saved=21252
transforms: ['router:protected:user_message', 'router:tool_result:smart_crusher']
waste_signals: None <- SmartCrusher removed 21k tokens, dashboard shows nothing
# after this fix (identical input)
before=37265 after=16013 saved=21252
transforms: ['router:protected:user_message', 'router:tool_result:smart_crusher']
waste_signals: {'json_bloat': 37140, 'html_noise': 0, 'base64': 0, 'whitespace': 0, 'dynamic_date': 0, 'repetition': 0}
```
Parser-level parity (same JSON payload, both wire formats):
```
anthropic tool_result waste total: 0 -> 1745 after fix
openai role:"tool" waste total: 1745 (unchanged)
```
## 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
- [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
- [ ] CHANGELOG.md — not edited manually; release-please generates
entries from the conventional `fix:` commit
## Additional Notes
Scoped to the Anthropic `tool_result` parsing bug per issue #813. Two
related-but-separate gaps noted there: `handle_openai_responses` (codex)
never computes waste signals at all, and Gemini `functionResponse` parts
are preserved verbatim — both deserve their own issues/PRs.
---------
Co-authored-by: Ash Rhodes <ashley.rhodes@king.com>
This commit is contained in:
parent
914a60a2b0
commit
929698af10
2 changed files with 303 additions and 9 deletions
|
|
@ -3,6 +3,7 @@
|
|||
from __future__ import annotations
|
||||
|
||||
import hashlib
|
||||
import json
|
||||
import re
|
||||
from typing import TYPE_CHECKING, Any
|
||||
|
||||
|
|
@ -36,6 +37,38 @@ def compute_hash(text: str) -> str:
|
|||
return hashlib.md5(text.encode()).hexdigest()[:16] # nosec B324
|
||||
|
||||
|
||||
def _extract_tool_result_text(payload: dict[str, Any]) -> str:
|
||||
"""Extract text from a tool result payload.
|
||||
|
||||
Handles the Anthropic ``tool_result`` block (``payload["content"]``
|
||||
is a plain string or a list of ``{"type": "text", ...}`` blocks) and
|
||||
the Strands/Bedrock ``toolResult`` payload (content items keyed as
|
||||
``{"text": ...}`` or ``{"json": ...}`` without a ``type`` field).
|
||||
Non-text inner blocks (e.g. images) are skipped.
|
||||
"""
|
||||
inner = payload.get("content")
|
||||
if inner is None:
|
||||
return ""
|
||||
if isinstance(inner, str):
|
||||
return inner
|
||||
if isinstance(inner, list):
|
||||
pieces = []
|
||||
for item in inner:
|
||||
if isinstance(item, dict):
|
||||
if item.get("type") == "text":
|
||||
pieces.append(item.get("text", ""))
|
||||
elif "type" not in item and isinstance(item.get("text"), str):
|
||||
pieces.append(item["text"])
|
||||
elif "type" not in item and "json" in item:
|
||||
pieces.append(json.dumps(item["json"], default=str))
|
||||
elif isinstance(item, str):
|
||||
pieces.append(item)
|
||||
return "\n".join(pieces)
|
||||
if isinstance(inner, dict):
|
||||
return json.dumps(inner, default=str)
|
||||
return str(inner)
|
||||
|
||||
|
||||
def detect_waste_signals(text: str, tokenizer: Tokenizer) -> WasteSignals:
|
||||
"""
|
||||
Detect waste signals in text.
|
||||
|
|
@ -112,6 +145,7 @@ def parse_message_to_blocks(
|
|||
# Handle content
|
||||
content = message.get("content")
|
||||
if content:
|
||||
tool_result_parts: list[dict[str, Any]] = []
|
||||
if isinstance(content, str):
|
||||
text = content
|
||||
elif isinstance(content, list):
|
||||
|
|
@ -120,6 +154,13 @@ def parse_message_to_blocks(
|
|||
for part in content:
|
||||
if isinstance(part, dict) and part.get("type") == "text":
|
||||
text_parts.append(part.get("text", ""))
|
||||
elif isinstance(part, dict) and part.get("type") == "tool_result":
|
||||
# Anthropic Messages format nests tool output one level
|
||||
# deeper; collect for dedicated tool_result blocks below.
|
||||
tool_result_parts.append(part)
|
||||
elif isinstance(part, dict) and "toolResult" in part:
|
||||
# Strands/Bedrock converse format; same treatment.
|
||||
tool_result_parts.append(part)
|
||||
elif isinstance(part, str):
|
||||
text_parts.append(part)
|
||||
text = "\n".join(text_parts)
|
||||
|
|
@ -149,16 +190,46 @@ def parse_message_to_blocks(
|
|||
if waste.total() > 0:
|
||||
flags["waste_signals"] = waste.to_dict()
|
||||
|
||||
blocks.append(
|
||||
Block(
|
||||
kind=kind, # type: ignore[arg-type]
|
||||
text=text,
|
||||
tokens_est=tokenizer.count_text(text) + 4, # Add message overhead
|
||||
content_hash=compute_hash(text),
|
||||
source_index=index,
|
||||
flags=flags,
|
||||
tr_blocks: list[Block] = []
|
||||
for part in tool_result_parts:
|
||||
payload = part["toolResult"] if "toolResult" in part else part
|
||||
if not isinstance(payload, dict):
|
||||
continue
|
||||
tr_text = _extract_tool_result_text(payload)
|
||||
if not tr_text:
|
||||
continue
|
||||
|
||||
tr_id = payload.get("toolUseId") if "toolResult" in part else part.get("tool_use_id")
|
||||
tr_flags: dict[str, Any] = {"tool_call_id": tr_id}
|
||||
tr_waste = detect_waste_signals(tr_text, tokenizer)
|
||||
if tr_waste.total() > 0:
|
||||
tr_flags["waste_signals"] = tr_waste.to_dict()
|
||||
|
||||
tr_blocks.append(
|
||||
Block(
|
||||
kind="tool_result",
|
||||
text=tr_text,
|
||||
tokens_est=tokenizer.count_text(tr_text) + 4, # Add message overhead
|
||||
content_hash=compute_hash(tr_text),
|
||||
source_index=index,
|
||||
flags=tr_flags,
|
||||
)
|
||||
)
|
||||
)
|
||||
|
||||
# Tool-result-only messages are fully represented by their dedicated
|
||||
# blocks; skip the empty container block in that case.
|
||||
if text or not tr_blocks:
|
||||
blocks.append(
|
||||
Block(
|
||||
kind=kind, # type: ignore[arg-type]
|
||||
text=text,
|
||||
tokens_est=tokenizer.count_text(text) + 4, # Add message overhead
|
||||
content_hash=compute_hash(text),
|
||||
source_index=index,
|
||||
flags=flags,
|
||||
)
|
||||
)
|
||||
blocks.extend(tr_blocks)
|
||||
|
||||
# Handle tool calls (assistant messages with tool_calls)
|
||||
tool_calls = message.get("tool_calls")
|
||||
|
|
|
|||
|
|
@ -703,3 +703,226 @@ def sample_messages_with_tools():
|
|||
},
|
||||
{"role": "assistant", "content": "I found user Alice with ID 12345."},
|
||||
]
|
||||
|
||||
|
||||
# --- Anthropic tool_result content blocks (chopratejas/headroom#813) ---
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def big_json_payload():
|
||||
"""JSON blob large enough to trip the json_bloat detector (>500 tokens)."""
|
||||
return "{" + ",".join(f'"key_{i}": "value padding text {i}"' for i in range(200)) + "}"
|
||||
|
||||
|
||||
class TestAnthropicToolResultBlocks:
|
||||
"""Anthropic Messages format nests tool output in tool_result content blocks."""
|
||||
|
||||
def test_tool_result_with_nested_text_list(self, mock_tokenizer, big_json_payload):
|
||||
message = {
|
||||
"role": "user",
|
||||
"content": [
|
||||
{
|
||||
"type": "tool_result",
|
||||
"tool_use_id": "toolu_01",
|
||||
"content": [{"type": "text", "text": big_json_payload}],
|
||||
}
|
||||
],
|
||||
}
|
||||
|
||||
blocks = parse_message_to_blocks(message, 0, mock_tokenizer)
|
||||
|
||||
tool_blocks = [b for b in blocks if b.kind == "tool_result"]
|
||||
assert len(tool_blocks) == 1
|
||||
assert tool_blocks[0].text == big_json_payload
|
||||
assert tool_blocks[0].flags["tool_call_id"] == "toolu_01"
|
||||
assert tool_blocks[0].flags["waste_signals"]["json_bloat"] > 0
|
||||
|
||||
def test_tool_result_with_string_content(self, mock_tokenizer, big_json_payload):
|
||||
message = {
|
||||
"role": "user",
|
||||
"content": [
|
||||
{
|
||||
"type": "tool_result",
|
||||
"tool_use_id": "toolu_02",
|
||||
"content": big_json_payload,
|
||||
}
|
||||
],
|
||||
}
|
||||
|
||||
blocks = parse_message_to_blocks(message, 0, mock_tokenizer)
|
||||
|
||||
tool_blocks = [b for b in blocks if b.kind == "tool_result"]
|
||||
assert len(tool_blocks) == 1
|
||||
assert tool_blocks[0].text == big_json_payload
|
||||
assert tool_blocks[0].flags["waste_signals"]["json_bloat"] > 0
|
||||
|
||||
def test_mixed_text_and_tool_result(self, mock_tokenizer, big_json_payload):
|
||||
message = {
|
||||
"role": "user",
|
||||
"content": [
|
||||
{"type": "text", "text": "Here is the output:"},
|
||||
{
|
||||
"type": "tool_result",
|
||||
"tool_use_id": "toolu_03",
|
||||
"content": [{"type": "text", "text": big_json_payload}],
|
||||
},
|
||||
],
|
||||
}
|
||||
|
||||
blocks = parse_message_to_blocks(message, 0, mock_tokenizer)
|
||||
|
||||
assert [b.kind for b in blocks] == ["user", "tool_result"]
|
||||
assert blocks[0].text == "Here is the output:"
|
||||
assert blocks[1].text == big_json_payload
|
||||
|
||||
def test_empty_tool_result_content_emits_no_block(self, mock_tokenizer):
|
||||
message = {
|
||||
"role": "user",
|
||||
"content": [
|
||||
{"type": "tool_result", "tool_use_id": "toolu_04", "content": []},
|
||||
{"type": "tool_result", "tool_use_id": "toolu_05"},
|
||||
],
|
||||
}
|
||||
|
||||
blocks = parse_message_to_blocks(message, 0, mock_tokenizer)
|
||||
|
||||
# Nothing extractable: keep the container block so every message
|
||||
# still yields at least one block.
|
||||
assert [b.kind for b in blocks] == ["user"]
|
||||
|
||||
def test_tool_result_only_message_skips_container_block(self, mock_tokenizer, big_json_payload):
|
||||
message = {
|
||||
"role": "user",
|
||||
"content": [
|
||||
{"type": "tool_result", "tool_use_id": "toolu_08", "content": big_json_payload}
|
||||
],
|
||||
}
|
||||
|
||||
blocks = parse_message_to_blocks(message, 0, mock_tokenizer)
|
||||
|
||||
assert [b.kind for b in blocks] == ["tool_result"]
|
||||
|
||||
def test_tool_result_dict_content_serialized_as_json(self, mock_tokenizer):
|
||||
rows = {"rows": [{"id": i, "padding": "x" * 30} for i in range(120)]}
|
||||
message = {
|
||||
"role": "user",
|
||||
"content": [{"type": "tool_result", "tool_use_id": "toolu_09", "content": rows}],
|
||||
}
|
||||
|
||||
blocks = parse_message_to_blocks(message, 0, mock_tokenizer)
|
||||
|
||||
tool_blocks = [b for b in blocks if b.kind == "tool_result"]
|
||||
assert len(tool_blocks) == 1
|
||||
assert tool_blocks[0].text.startswith('{"rows":')
|
||||
assert tool_blocks[0].flags["waste_signals"]["json_bloat"] > 0
|
||||
|
||||
def test_missing_tool_use_id_yields_none_flag(self, mock_tokenizer, big_json_payload):
|
||||
message = {
|
||||
"role": "user",
|
||||
"content": [{"type": "tool_result", "content": big_json_payload}],
|
||||
}
|
||||
|
||||
blocks = parse_message_to_blocks(message, 0, mock_tokenizer)
|
||||
|
||||
assert blocks[0].kind == "tool_result"
|
||||
assert blocks[0].flags["tool_call_id"] is None
|
||||
|
||||
|
||||
class TestStrandsToolResultBlocks:
|
||||
"""Strands/Bedrock converse format: toolResult content parts."""
|
||||
|
||||
def test_strands_text_content(self, mock_tokenizer, big_json_payload):
|
||||
message = {
|
||||
"role": "user",
|
||||
"content": [
|
||||
{
|
||||
"toolResult": {
|
||||
"toolUseId": "strands_01",
|
||||
"content": [{"text": big_json_payload}],
|
||||
"status": "success",
|
||||
}
|
||||
}
|
||||
],
|
||||
}
|
||||
|
||||
blocks = parse_message_to_blocks(message, 0, mock_tokenizer)
|
||||
|
||||
assert [b.kind for b in blocks] == ["tool_result"]
|
||||
assert blocks[0].text == big_json_payload
|
||||
assert blocks[0].flags["tool_call_id"] == "strands_01"
|
||||
assert blocks[0].flags["waste_signals"]["json_bloat"] > 0
|
||||
|
||||
def test_strands_json_content(self, mock_tokenizer):
|
||||
rows = {"rows": [{"id": i, "padding": "x" * 30} for i in range(120)]}
|
||||
message = {
|
||||
"role": "user",
|
||||
"content": [{"toolResult": {"toolUseId": "strands_02", "content": [{"json": rows}]}}],
|
||||
}
|
||||
|
||||
blocks = parse_message_to_blocks(message, 0, mock_tokenizer)
|
||||
|
||||
assert [b.kind for b in blocks] == ["tool_result"]
|
||||
assert blocks[0].flags["waste_signals"]["json_bloat"] > 0
|
||||
|
||||
def test_non_text_inner_blocks_skipped(self, mock_tokenizer):
|
||||
message = {
|
||||
"role": "user",
|
||||
"content": [
|
||||
{
|
||||
"type": "tool_result",
|
||||
"tool_use_id": "toolu_06",
|
||||
"content": [
|
||||
{"type": "image", "source": {"type": "base64", "data": "abc"}},
|
||||
{"type": "text", "text": "small result"},
|
||||
"raw string piece",
|
||||
],
|
||||
}
|
||||
],
|
||||
}
|
||||
|
||||
blocks = parse_message_to_blocks(message, 0, mock_tokenizer)
|
||||
|
||||
tool_blocks = [b for b in blocks if b.kind == "tool_result"]
|
||||
assert len(tool_blocks) == 1
|
||||
assert tool_blocks[0].text == "small result\nraw string piece"
|
||||
|
||||
def test_parse_messages_aggregates_tool_result_waste(self, mock_tokenizer, big_json_payload):
|
||||
messages = [
|
||||
{"role": "user", "content": "Run the tool"},
|
||||
{
|
||||
"role": "user",
|
||||
"content": [
|
||||
{
|
||||
"type": "tool_result",
|
||||
"tool_use_id": "toolu_07",
|
||||
"content": [{"type": "text", "text": big_json_payload}],
|
||||
}
|
||||
],
|
||||
},
|
||||
]
|
||||
|
||||
_, breakdown, waste = parse_messages(messages, mock_tokenizer)
|
||||
|
||||
assert waste.json_bloat_tokens > 0
|
||||
assert breakdown["tool_result"] > 0
|
||||
|
||||
def test_waste_parity_with_openai_tool_role(self, mock_tokenizer, big_json_payload):
|
||||
anthropic_messages = [
|
||||
{
|
||||
"role": "user",
|
||||
"content": [
|
||||
{
|
||||
"type": "tool_result",
|
||||
"tool_use_id": "t1",
|
||||
"content": [{"type": "text", "text": big_json_payload}],
|
||||
}
|
||||
],
|
||||
}
|
||||
]
|
||||
openai_messages = [{"role": "tool", "tool_call_id": "t1", "content": big_json_payload}]
|
||||
|
||||
_, _, anthropic_waste = parse_messages(anthropic_messages, mock_tokenizer)
|
||||
_, _, openai_waste = parse_messages(openai_messages, mock_tokenizer)
|
||||
|
||||
assert anthropic_waste.total() > 0
|
||||
assert anthropic_waste.total() == openai_waste.total()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue