mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
fix(proxy): skip OpenAI tool_search deferral for Codex client (#2729)
## Description
OpenAI Responses tool_search deferral injects `defer_loading` and a
`tool_search` tool for eligible gpt-5.4+ requests. When the model later
calls a deferred tool, the function_call item carries a `namespace`
field. Codex CLI round-trip structs drop unknown fields, so the next
request omits `namespace` and OpenAI returns 400, killing the session
mid-run. Proxy logs for these Codex turns show no tool savings, so the
injection breaks Codex without benefit.
This skips OpenAI tool_search deferral when the classified client is
Codex, leaving other clients unchanged.
Closes #2726
## 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
- Add `openai_tool_search_client_supported` and a Codex-only unsupported
client set
- Pass optional `client` into `inject_tool_search_deferral_openai` and
no-op for Codex
- Plumb `client` through Responses compression (HTTP, WebSocket,
passthrough) with legacy-signature retries
- Add regression tests for Codex skip and non-Codex still injects
## 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
$ .venv/bin/python -m pytest tests/test_openai_tool_search_deferral.py -q -o addopts=
34 passed, 1 warning in 0.27s
$ .venv/bin/python -m ruff check headroom/proxy/helpers.py headroom/proxy/handlers/openai.py tests/test_openai_tool_search_deferral.py
All checks passed!
$ .venv/bin/python -m ruff format --check headroom/proxy/helpers.py headroom/proxy/handlers/openai.py tests/test_openai_tool_search_deferral.py
3 files already formatted
```
## Real Behavior Proof
- Environment: Linux x86_64, Python 3.14.5 in repo .venv, shallow
checkout of headroomlabs-ai/headroom main at 01df245 plus this branch
- Exact command / steps: `.venv/bin/python -m pytest
tests/test_openai_tool_search_deferral.py -q -o addopts=`;
`.venv/bin/python -m ruff check headroom/proxy/helpers.py
headroom/proxy/handlers/openai.py
tests/test_openai_tool_search_deferral.py`; `.venv/bin/python -m ruff
format --check headroom/proxy/helpers.py
headroom/proxy/handlers/openai.py
tests/test_openai_tool_search_deferral.py`
- Observed result: 34 targeted tests passed, including Codex client
identity no-op and non-Codex still injecting tool_search; ruff check and
format check clean on touched files
- Not tested: live `codex exec` multi-turn session through headroom
proxy with >=12 tools; full monorepo `make ci-precheck`; 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
- [ ] 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
- [x] I did **not** edit `CHANGELOG.md` - it is generated by
release-please from my Conventional Commit PR title (a CI guard enforces
this)
## Screenshots (if applicable)
N/A
## Additional Notes
- Scoped to OpenAI Responses tool_search deferral client gating only;
Anthropic tool_search path unchanged
- Related open work for OpenCode (#2696) is separate; this PR only
excludes Codex
- mypy not run on this VPS for this change
This commit is contained in:
parent
01df245252
commit
56b3e4c1b1
3 changed files with 98 additions and 30 deletions
|
|
@ -2209,6 +2209,7 @@ class OpenAIHandlerMixin:
|
|||
model: str,
|
||||
request_id: str,
|
||||
timing: dict[str, float] | None = None,
|
||||
client: str | None = None,
|
||||
) -> tuple[dict[str, Any], bool, int, list[str], str | None, int, int, int]:
|
||||
"""Compress an OpenAI Responses payload through the shared router.
|
||||
|
||||
|
|
@ -2346,7 +2347,9 @@ class OpenAIHandlerMixin:
|
|||
# one — hence a transform tag but no tokens_saved claim.
|
||||
from headroom.proxy.helpers import inject_tool_search_deferral_openai
|
||||
|
||||
_deferred_tools = inject_tool_search_deferral_openai(working.get("tools"), model)
|
||||
_deferred_tools = inject_tool_search_deferral_openai(
|
||||
working.get("tools"), model, client=client
|
||||
)
|
||||
if _deferred_tools is not working.get("tools"):
|
||||
if working is payload:
|
||||
working = copy.deepcopy(payload)
|
||||
|
|
@ -2548,6 +2551,7 @@ class OpenAIHandlerMixin:
|
|||
model: str,
|
||||
request_id: str,
|
||||
timeout: float = COMPRESSION_TIMEOUT_SECONDS,
|
||||
client: str | None = None,
|
||||
) -> tuple[dict[str, Any], bool, int, list[str], str | None, int, int, int, dict[str, float]]:
|
||||
timing: dict[str, float] = {}
|
||||
|
||||
|
|
@ -2562,21 +2566,32 @@ class OpenAIHandlerMixin:
|
|||
shape_labels, shape_mutated = _shape_openai_responses_payload(
|
||||
payload, model=model, request_id=request_id
|
||||
)
|
||||
try:
|
||||
result = self._compress_openai_responses_payload(
|
||||
payload,
|
||||
model=model,
|
||||
request_id=request_id,
|
||||
timing=timing,
|
||||
)
|
||||
except TypeError as exc:
|
||||
if "unexpected keyword argument 'timing'" not in str(exc):
|
||||
raise
|
||||
result = self._compress_openai_responses_payload(
|
||||
payload,
|
||||
model=model,
|
||||
request_id=request_id,
|
||||
)
|
||||
compression_kwargs: dict[str, Any] = {
|
||||
"model": model,
|
||||
"request_id": request_id,
|
||||
"timing": timing,
|
||||
"client": client,
|
||||
}
|
||||
while True:
|
||||
try:
|
||||
result = self._compress_openai_responses_payload(
|
||||
payload,
|
||||
**compression_kwargs,
|
||||
)
|
||||
break
|
||||
except TypeError as exc:
|
||||
unsupported_kwarg = next(
|
||||
(
|
||||
name
|
||||
for name in ("client", "timing")
|
||||
if f"unexpected keyword argument '{name}'" in str(exc)
|
||||
and name in compression_kwargs
|
||||
),
|
||||
None,
|
||||
)
|
||||
if unsupported_kwarg is None:
|
||||
raise
|
||||
compression_kwargs.pop(unsupported_kwarg)
|
||||
if shape_labels:
|
||||
# Carry the shaper labels on the transforms channel so the
|
||||
# outcome funnel feeds the output-savings ledger
|
||||
|
|
@ -4917,6 +4932,7 @@ class OpenAIHandlerMixin:
|
|||
body,
|
||||
model=model,
|
||||
request_id=request_id,
|
||||
client=client,
|
||||
)
|
||||
attempted_input_tokens = int(_attempted_tokens)
|
||||
if _transforms:
|
||||
|
|
@ -6559,6 +6575,7 @@ class OpenAIHandlerMixin:
|
|||
timeout=_codex_ws_compression_timeout_seconds()
|
||||
if client == "codex"
|
||||
else COMPRESSION_TIMEOUT_SECONDS,
|
||||
client=client,
|
||||
)
|
||||
for _timing_name, _timing_ms in _ws_compression_timing.items():
|
||||
_record_ws_compression_timing(_timing_name, _timing_ms)
|
||||
|
|
@ -6904,6 +6921,7 @@ class OpenAIHandlerMixin:
|
|||
timeout=_codex_ws_compression_timeout_seconds()
|
||||
if client == "codex"
|
||||
else COMPRESSION_TIMEOUT_SECONDS,
|
||||
client=client,
|
||||
)
|
||||
for _timing_name, _timing_ms in frame_compression_timing.items():
|
||||
_record_ws_compression_timing(_timing_name, _timing_ms)
|
||||
|
|
@ -8650,7 +8668,9 @@ class OpenAIHandlerMixin:
|
|||
},
|
||||
)
|
||||
|
||||
async def _maybe_compress_passthrough_responses(self, body: bytes) -> bytes:
|
||||
async def _maybe_compress_passthrough_responses(
|
||||
self, body: bytes, *, client: str | None = None
|
||||
) -> bytes:
|
||||
"""Compress an OpenAI Responses-shaped passthrough body, fail-open.
|
||||
|
||||
Reuses the native `/v1/responses` compression path so custom
|
||||
|
|
@ -8669,15 +8689,22 @@ class OpenAIHandlerMixin:
|
|||
model = str(payload.get("model") or "passthrough")
|
||||
request_id = await self._next_request_id()
|
||||
try:
|
||||
(
|
||||
compressed_payload,
|
||||
modified,
|
||||
*_rest,
|
||||
) = await self._compress_openai_responses_payload_in_executor(
|
||||
payload,
|
||||
model=model,
|
||||
request_id=request_id,
|
||||
)
|
||||
try:
|
||||
result = await self._compress_openai_responses_payload_in_executor(
|
||||
payload,
|
||||
model=model,
|
||||
request_id=request_id,
|
||||
client=client,
|
||||
)
|
||||
except TypeError as exc:
|
||||
if "unexpected keyword argument 'client'" not in str(exc):
|
||||
raise
|
||||
result = await self._compress_openai_responses_payload_in_executor(
|
||||
payload,
|
||||
model=model,
|
||||
request_id=request_id,
|
||||
)
|
||||
compressed_payload, modified, *_rest = result
|
||||
except Exception as exc: # noqa: BLE001 — fail-open on any compressor error
|
||||
logger.warning(
|
||||
"[%s] passthrough Responses compression failed, forwarding verbatim: %s",
|
||||
|
|
@ -8792,7 +8819,7 @@ class OpenAIHandlerMixin:
|
|||
and path.rstrip("/").endswith("/responses")
|
||||
and body
|
||||
):
|
||||
compressed = await self._maybe_compress_passthrough_responses(body)
|
||||
compressed = await self._maybe_compress_passthrough_responses(body, client=client)
|
||||
if compressed != body:
|
||||
body = compressed
|
||||
# Body size changed — let httpx recompute Content-Length.
|
||||
|
|
|
|||
|
|
@ -2421,6 +2421,7 @@ def inject_tool_search_deferral(
|
|||
_OPENAI_TOOL_SEARCH_TYPE = "tool_search"
|
||||
_OPENAI_TOOL_SEARCH_MIN_TOOLS = 12
|
||||
_OPENAI_TOOL_SEARCH_RESIDENT_NAMES = frozenset({"terminal"})
|
||||
_OPENAI_TOOL_SEARCH_UNSUPPORTED_CLIENTS = frozenset({"codex"})
|
||||
# gpt-5.4 is the first model with Responses tool_search (OpenAI docs). Version-
|
||||
# gated by default; overridable per deployment via a regex in
|
||||
# HEADROOM_OPENAI_TOOL_SEARCH_MODELS (matched against the model name) so new
|
||||
|
|
@ -2451,24 +2452,34 @@ def _model_supports_openai_tool_search(model: str | None) -> bool:
|
|||
return (major, minor) >= _OPENAI_TOOL_SEARCH_MIN_VERSION
|
||||
|
||||
|
||||
def openai_tool_search_client_supported(client: str | None) -> bool:
|
||||
"""Return whether OpenAI tool search deferral is safe for this client."""
|
||||
normalized = client.strip().lower() if client else ""
|
||||
return normalized not in _OPENAI_TOOL_SEARCH_UNSUPPORTED_CLIENTS
|
||||
|
||||
|
||||
def inject_tool_search_deferral_openai(
|
||||
tools: Any,
|
||||
model: str | None,
|
||||
*,
|
||||
client: str | None = None,
|
||||
core_tools: frozenset[str] = _TOOL_SEARCH_CORE_TOOLS,
|
||||
) -> Any:
|
||||
"""Return a new Responses ``tools`` list with non-core function/MCP tools
|
||||
deferred + a ``{"type": "tool_search"}`` tool injected, or the original list
|
||||
unchanged when injection doesn't apply.
|
||||
|
||||
No-op when: the model doesn't support tool search (gpt-5.4+ only), ``tools``
|
||||
No-op for Codex, whose round-trip structs drop deferred-call namespaces. Also
|
||||
no-op when: the model doesn't support tool search (gpt-5.4+ only), ``tools``
|
||||
is not a list, there are fewer than ``_OPENAI_TOOL_SEARCH_MIN_TOOLS``, a
|
||||
tool_search tool is already present (client already defers), or nothing would
|
||||
be deferred. Core coding tools and hosted/typed tools (web_search,
|
||||
file_search, code_interpreter, computer, …) stay resident and unchanged, so
|
||||
routine edit/read/run loops never pay a search round-trip and the request
|
||||
file_search, code_interpreter, computer, ...) stay resident and unchanged,
|
||||
so routine edit/read/run loops never pay a search round-trip and the request
|
||||
stays valid; the injected search tool is itself resident.
|
||||
"""
|
||||
if not openai_tool_search_client_supported(client):
|
||||
return tools
|
||||
if not _model_supports_openai_tool_search(model):
|
||||
return tools
|
||||
if not isinstance(tools, list) or len(tools) < _OPENAI_TOOL_SEARCH_MIN_TOOLS:
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@ import pytest
|
|||
from headroom.proxy.helpers import (
|
||||
_model_supports_openai_tool_search,
|
||||
inject_tool_search_deferral_openai,
|
||||
openai_tool_search_client_supported,
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -57,6 +58,35 @@ def test_env_override_wins_then_falls_back(monkeypatch):
|
|||
# --- deferral behavior -------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("client", "supported"),
|
||||
[(None, True), ("codex", False), (" CODEX ", False), ("opencode", True), ("claude", True)],
|
||||
)
|
||||
def test_client_supported(client, supported):
|
||||
assert openai_tool_search_client_supported(client) is supported
|
||||
|
||||
|
||||
def test_codex_client_does_not_inject():
|
||||
tools = _tools()
|
||||
|
||||
out = inject_tool_search_deferral_openai(tools, "gpt-5.5", client="codex")
|
||||
|
||||
assert out is tools
|
||||
assert all(tool.get("type") != "tool_search" for tool in out)
|
||||
assert all("defer_loading" not in tool for tool in out)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("client", [None, "opencode"])
|
||||
def test_supported_clients_still_inject(client):
|
||||
tools = _tools()
|
||||
|
||||
out = inject_tool_search_deferral_openai(tools, "gpt-5.5", client=client)
|
||||
|
||||
assert out is not tools
|
||||
assert out[0] == {"type": "tool_search"}
|
||||
assert any(tool.get("defer_loading") is True for tool in out)
|
||||
|
||||
|
||||
def test_defers_non_core_and_injects_search_tool():
|
||||
tools = _tools()
|
||||
out = inject_tool_search_deferral_openai(tools, "gpt-5.5")
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue