mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
## Description `HEADROOM_EXCLUDE_TOOLS` protects excluded tool outputs for Anthropic `tool_result` blocks and OpenAI chat `role=tool` messages, but was ignored on the Codex `/v1/responses` path. Large exact MCP outputs (e.g. Serena `find_symbol` / `get_symbols_overview`) were compressed even when the tool name was explicitly excluded, so the model saw summarized output and fell back to raw file reads. Closes #940 ## 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 Root cause: `ContentRouter` consults `exclude_tools` via a `tool_call_id -> name` map built from chat `tool_calls` / Anthropic `tool_use` blocks (`_build_tool_name_map`). The Responses adapter (`_compress_openai_responses_live_text_units_with_router`) extracted every `function_call_output` as a compression unit without correlating it to the originating `function_call`'s name, so `exclude_tools` was never consulted for Responses tool outputs. - `headroom/proxy/handlers/openai.py`: - Build a `call_id -> tool name` map from the Responses `function_call` items (the name lives on `function_call`, the originating `call_id` on the matching `function_call_output`). - Resolve the effective exclude set the same way `ContentRouter` does (`router.config.exclude_tools`, falling back to `DEFAULT_EXCLUDE_TOOLS` when `None`). - Skip extraction of outputs whose originating tool is excluded, mirroring the existing `headroom_retrieve` output guard. Name matching also tests the lowercased name defensively for case-insensitivity. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [ ] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text tests/test_openai_responses_compression_units.py::test_openai_responses_adapter_preserves_excluded_tool_outputs PASSED tests/test_openai_responses_compression_units.py::test_openai_responses_adapter_compresses_non_excluded_tool_outputs PASSED tests/test_openai_responses_compression_units.py::test_openai_responses_adapter_preserves_headroom_retrieve_outputs PASSED tests/test_openai_responses_compression_units.py::test_openai_responses_adapter_compresses_custom_tool_call_output PASSED 4 passed $ ruff check headroom/proxy/handlers/openai.py tests/test_openai_responses_compression_units.py All checks passed! ``` ## Real Behavior Proof - Environment: macOS (ARM64), Python 3.13. - Exact command / steps: ran the new and adjacent unit tests for the Responses compression adapter. The native `headroom._core` extension could not be compiled locally (macOS 26 C++ toolchain), so these tests were executed with a stubbed `_core`; the changed code path is pure Python and the tests override `router.compress`, so the stub does not affect what is exercised. CI builds the real core. - Observed result: outputs for an excluded tool (`serena.find_symbol`) are left untouched (`modified=False`), while outputs for a non-excluded tool still compress and are replaced with the routed summary. - Not tested: full native build / live Codex end-to-end run; `mypy`. ## 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 Documentation / CHANGELOG updates are N/A: this restores the documented behavior of `HEADROOM_EXCLUDE_TOOLS` on a path where it was silently dropped. `mypy` and a full native build were not run in this environment; the change is pure Python.
This commit is contained in:
parent
9f7f3adfea
commit
f03e77bec0
2 changed files with 191 additions and 1 deletions
|
|
@ -764,19 +764,44 @@ class OpenAIHandlerMixin:
|
|||
item["output"] = replacement
|
||||
|
||||
headroom_retrieve_call_ids: set[str] = set()
|
||||
# Map each Responses tool call to its name so that outputs belonging to
|
||||
# excluded tools (HEADROOM_EXCLUDE_TOOLS) can be protected from
|
||||
# compression. The chat/Anthropic paths get this via
|
||||
# ContentRouter._build_tool_name_map; the Responses payload carries the
|
||||
# name on the `function_call` item and the originating call_id on the
|
||||
# matching `function_call_output`, so we correlate them here.
|
||||
function_name_by_call_id: dict[str, str] = {}
|
||||
for item in items:
|
||||
if not isinstance(item, dict):
|
||||
continue
|
||||
if item.get("type") != "function_call":
|
||||
continue
|
||||
name = item.get("name")
|
||||
call_id = item.get("call_id")
|
||||
if isinstance(name, str) and isinstance(call_id, str) and call_id:
|
||||
function_name_by_call_id[call_id] = name
|
||||
if isinstance(name, str) and (
|
||||
name == "headroom_retrieve" or name.endswith("__headroom_retrieve")
|
||||
):
|
||||
call_id = item.get("call_id")
|
||||
if isinstance(call_id, str) and call_id:
|
||||
headroom_retrieve_call_ids.add(call_id)
|
||||
|
||||
# Resolve the effective exclude set once (None -> built-in defaults),
|
||||
# mirroring ContentRouter's policy. exclude_tools already contains both
|
||||
# original and lowercased name variants (see _parse_exclude_tools), but
|
||||
# we also test the lowercased name defensively for case-insensitivity.
|
||||
from headroom.config import DEFAULT_EXCLUDE_TOOLS
|
||||
|
||||
router_exclude_tools = getattr(router.config, "exclude_tools", None)
|
||||
effective_exclude_tools = (
|
||||
router_exclude_tools if router_exclude_tools is not None else DEFAULT_EXCLUDE_TOOLS
|
||||
)
|
||||
excluded_call_ids: set[str] = {
|
||||
call_id
|
||||
for call_id, fn_name in function_name_by_call_id.items()
|
||||
if fn_name in effective_exclude_tools or fn_name.lower() in effective_exclude_tools
|
||||
}
|
||||
|
||||
timing_sink: dict[str, float] = timing if timing is not None else {}
|
||||
|
||||
def _add_timing(name: str, started_at: float) -> None:
|
||||
|
|
@ -816,6 +841,20 @@ class OpenAIHandlerMixin:
|
|||
}
|
||||
)
|
||||
continue
|
||||
if isinstance(call_id, str) and call_id in excluded_call_ids:
|
||||
if debug_enabled:
|
||||
extraction_debug.append(
|
||||
{
|
||||
"index": idx,
|
||||
"eligible": False,
|
||||
"reason": "exclude_tools_protected",
|
||||
"item_type": item_type,
|
||||
"call_id": call_id,
|
||||
"tool_name": function_name_by_call_id.get(call_id),
|
||||
"item": item,
|
||||
}
|
||||
)
|
||||
continue
|
||||
slot = _slot_text(item)
|
||||
if slot is not None:
|
||||
text, slot_ref = slot
|
||||
|
|
|
|||
|
|
@ -399,6 +399,157 @@ def test_openai_responses_adapter_preserves_headroom_retrieve_outputs():
|
|||
assert strategy_chain == []
|
||||
|
||||
|
||||
def test_openai_responses_adapter_preserves_excluded_tool_outputs():
|
||||
"""Regression for #940: outputs for HEADROOM_EXCLUDE_TOOLS tools stay raw.
|
||||
|
||||
The Responses path carries the tool name on the ``function_call`` item and
|
||||
the originating ``call_id`` on the matching ``function_call_output``; the
|
||||
adapter must correlate them and skip compression for excluded tools.
|
||||
"""
|
||||
router = ContentRouter()
|
||||
router.config.exclude_tools = {"serena.find_symbol", "find_symbol"}
|
||||
|
||||
def compress(self, content: str, **_kwargs):
|
||||
return RouterCompressionResult(
|
||||
compressed="should not be used",
|
||||
original=content,
|
||||
strategy_used=CompressionStrategy.KOMPRESS,
|
||||
)
|
||||
|
||||
router.compress = MethodType(compress, router)
|
||||
handler = _handler_with_router(router)
|
||||
output = " ".join(f"sym{i}" for i in range(180))
|
||||
payload = {
|
||||
"model": "gpt-5",
|
||||
"input": [
|
||||
{
|
||||
"type": "function_call",
|
||||
"call_id": "call_1",
|
||||
"name": "serena.find_symbol",
|
||||
"arguments": "{}",
|
||||
},
|
||||
{
|
||||
"type": "function_call_output",
|
||||
"call_id": "call_1",
|
||||
"output": output,
|
||||
},
|
||||
],
|
||||
}
|
||||
|
||||
new_payload, modified, saved, transforms, units_by_category, strategy_chain, _attempted = (
|
||||
handler._compress_openai_responses_live_text_units_with_router(
|
||||
payload,
|
||||
model="gpt-5",
|
||||
request_id="req_test",
|
||||
)
|
||||
)
|
||||
|
||||
assert modified is False
|
||||
assert saved == 0
|
||||
assert transforms == []
|
||||
assert new_payload == payload
|
||||
assert units_by_category == {}
|
||||
assert strategy_chain == []
|
||||
|
||||
|
||||
def test_openai_responses_adapter_excludes_tool_case_insensitively_with_debug(monkeypatch):
|
||||
"""Excluded match is case-insensitive, and the debug path stays exercised.
|
||||
|
||||
The configured name is lowercase only; the call advertises a mixed-case
|
||||
name, so the protection must hit via the lowercased fallback. Debug logging
|
||||
is enabled so the protected-extraction debug record is also covered.
|
||||
"""
|
||||
monkeypatch.setattr(openai_handler, "_log_codex_compression_debug", lambda *_a, **_k: None)
|
||||
router = ContentRouter()
|
||||
router.config.exclude_tools = {"serena.find_symbol"}
|
||||
|
||||
def compress(self, content: str, **_kwargs):
|
||||
return RouterCompressionResult(
|
||||
compressed="should not be used",
|
||||
original=content,
|
||||
strategy_used=CompressionStrategy.KOMPRESS,
|
||||
)
|
||||
|
||||
router.compress = MethodType(compress, router)
|
||||
handler = _handler_with_router(router)
|
||||
output = " ".join(f"sym{i}" for i in range(180))
|
||||
payload = {
|
||||
"model": "gpt-5",
|
||||
"input": [
|
||||
{
|
||||
"type": "function_call",
|
||||
"call_id": "call_1",
|
||||
"name": "Serena.Find_Symbol",
|
||||
"arguments": "{}",
|
||||
},
|
||||
{
|
||||
"type": "function_call_output",
|
||||
"call_id": "call_1",
|
||||
"output": output,
|
||||
},
|
||||
],
|
||||
}
|
||||
|
||||
new_payload, modified, saved, *_ = (
|
||||
handler._compress_openai_responses_live_text_units_with_router(
|
||||
payload,
|
||||
model="gpt-5",
|
||||
request_id="req_test",
|
||||
)
|
||||
)
|
||||
|
||||
assert modified is False
|
||||
assert saved == 0
|
||||
assert new_payload == payload
|
||||
|
||||
|
||||
def test_openai_responses_adapter_compresses_non_excluded_tool_outputs():
|
||||
"""Only excluded tools are protected; other tool outputs still compress."""
|
||||
router = ContentRouter()
|
||||
router.config.exclude_tools = {"serena.find_symbol"}
|
||||
|
||||
def compress(self, content: str, **_kwargs):
|
||||
return RouterCompressionResult(
|
||||
compressed="compressed tool output",
|
||||
original=content,
|
||||
strategy_used=CompressionStrategy.KOMPRESS,
|
||||
)
|
||||
|
||||
router.compress = MethodType(compress, router)
|
||||
handler = _handler_with_router(router)
|
||||
output = " ".join(f"word{i}" for i in range(180))
|
||||
payload = {
|
||||
"model": "gpt-5",
|
||||
"input": [
|
||||
{
|
||||
"type": "function_call",
|
||||
"call_id": "call_1",
|
||||
"name": "some.other_tool",
|
||||
"arguments": "{}",
|
||||
},
|
||||
{
|
||||
"type": "function_call_output",
|
||||
"call_id": "call_1",
|
||||
"output": output,
|
||||
},
|
||||
],
|
||||
}
|
||||
|
||||
new_payload, modified, saved, transforms, units_by_category, strategy_chain, _attempted = (
|
||||
handler._compress_openai_responses_live_text_units_with_router(
|
||||
payload,
|
||||
model="gpt-5",
|
||||
request_id="req_test",
|
||||
)
|
||||
)
|
||||
|
||||
assert modified is True
|
||||
assert saved > 0
|
||||
assert new_payload["input"][1]["output"] == "compressed tool output"
|
||||
assert "router:openai:responses:function_call_output:kompress" in transforms
|
||||
assert units_by_category == {"applied": 1}
|
||||
|
||||
|
||||
def test_openai_responses_adapter_keeps_small_and_opaque_items():
|
||||
router = ContentRouter()
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue