headroom/tests/test_issue_728_empty_tools_injection.py

Ignoring revisions in .git-blame-ignore-revs. Click here to bypass and see the normal blame view.

185 lines
7.8 KiB
Python
Raw Permalink Normal View History

fix: don't inject empty tools:[] when client omitted the tools field (#772) Fixes #728 ## Summary - `apply_session_sticky_ccr_tool` and `apply_session_sticky_memory_tools` always return a list — returning `[]` when `existing_tools=None` and nothing was injected - The old handler guard `if tools is not None:` evaluated `True` for `[]`, causing `body["tools"] = []` to be sent upstream on every request - vLLM-based providers (Venice.ai, etc.) strictly reject empty `tools` arrays with a 400 error **Fix:** Change the guard in both the OpenAI and Anthropic handlers from: ```python if tools is not None: body["tools"] = tools ``` to: ```python if tools or _original_tools is not None: body["tools"] = tools ``` The `_original_tools` variable is already defined in both handlers (`_original_tools = body.get("tools")`). This condition correctly handles all four cases: | Scenario | `tools` | `_original_tools` | Result | |---|---|---|---| | No client tools, no injection | `[]` | `None` | `False` → don't inject ✅ | | No client tools, CCR injected | `[ccr_tool]` | `None` | `True` → inject ✅ | | Client sent `tools: []` | `[]` | `[]` | `True` → preserve ✅ | | Client sent tools | `[A, ...]` | `[A, ...]` | `True` → preserve ✅ | ## Test plan - [x] New test file `tests/test_issue_728_empty_tools_injection.py` with 7 tests covering the guard condition and helper behavior - [x] All 51 existing CCR/golden-bytes tests still pass - [x] Zero changes to helper function return types or signatures ## Real behavior proof Tested against the helpers directly: ``` tests/test_issue_728_empty_tools_injection.py::TestHandlerGuardCondition::test_no_tools_no_injection_does_not_inject PASSED tests/test_issue_728_empty_tools_injection.py::TestHandlerGuardCondition::test_client_sent_empty_tools_is_preserved PASSED tests/test_issue_728_empty_tools_injection.py::TestHandlerGuardCondition::test_ccr_injection_sets_body_tools PASSED tests/test_issue_728_empty_tools_injection.py::TestHandlerGuardCondition::test_client_tools_always_set PASSED tests/test_issue_728_empty_tools_injection.py::TestCCRHelperNoToolsNoCompression::test_returns_empty_list_and_false_when_no_session_ccr PASSED tests/test_issue_728_empty_tools_injection.py::TestCCRHelperNoToolsNoCompression::test_returns_tool_list_when_compression_occurred PASSED tests/test_issue_728_empty_tools_injection.py::TestCCRHelperNoToolsNoCompression::test_no_double_injection_when_client_pre_registered_ccr_tool PASSED 7 passed in 1.81s ``` **What I did not test:** end-to-end against a live Venice.ai endpoint (no API key available), or passthrough mode with a real vLLM backend. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
2026-06-08 22:55:59 -07:00
"""Issue #728: proxy must not inject ``tools: []`` when the client omitted the tools field.
vLLM-based providers (Venice.ai, etc.) reject requests containing an empty ``tools``
array. The bug: ``apply_session_sticky_ccr_tool`` / ``apply_session_sticky_memory_tools``
always return a list (empty when no tools exist and none were injected), and the
old handler guard ``if tools is not None`` evaluated True for ``[]``, causing
``body["tools"] = []`` to be sent upstream unconditionally.
Fix: the guard was changed to ``if tools or _original_tools is not None`` in both
the OpenAI and Anthropic handlers so that an empty result list only reaches the
outgoing body when the original request already carried a ``tools`` field.
"""
from __future__ import annotations
import pytest
from headroom.ccr.tool_injection import CCR_TOOL_NAME
fix(proxy): preserve byte-faithful Anthropic tool forwarding (#1222) ## Description Anthropic tool sorting was rewriting `tools` lists even when canonical order was already present, which forced a mutation path that bypassed byte-faithful forwarding and reduced prefix-cache hit stability on repeated `/v1/messages` turns. Closes #1042 ## 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 - changed Anthropic tool canonicalization so already-canonical tool arrays are not rewritten in both single-request and batch paths - preserved byte-faithful forwarding for no-op single-request canonical tool-order requests by avoiding unnecessary `body["tools"]` reassignment - kept canonicalization behavior intact for out-of-order tool arrays - added one true regression proof for the PRE_SEND empty-tools path, plus forward-coverage tests for canonical no-op and real-sort-mutation request bodies in `test_proxy_byte_faithful_forwarding.py` - updated `CHANGELOG.md` to document the prefix-cache fix ## Testing - [x] Unit tests pass (`uv run pytest tests/test_proxy_handler_helpers.py tests/test_proxy_byte_faithful_forwarding.py -x -v`) - [x] Linting passes (`uv run ruff check headroom/proxy/handlers/anthropic.py tests/test_proxy_handler_helpers.py tests/test_proxy_byte_faithful_forwarding.py`) - [ ] Type checking passes (`uv run mypy headroom`) or explain N/A truthfully - [x] New tests added for new functionality when applicable - [ ] Manual testing performed ### Test Output ```text uv run pytest tests/test_proxy_handler_helpers.py tests/test_proxy_byte_faithful_forwarding.py -x -v 56 passed in 12.42s uv run ruff check headroom/proxy/handlers/anthropic.py tests/test_proxy_handler_helpers.py tests/test_proxy_byte_faithful_forwarding.py All checks passed! uv run ruff format headroom/proxy/handlers/anthropic.py tests/test_proxy_handler_helpers.py tests/test_proxy_byte_faithful_forwarding.py --check 3 files already formatted ``` ## Real Behavior Proof - Environment: Windows, local proxy test environment, Anthropic request-forwarding path - Exact command / steps: post `/v1/messages` requests with canonical tool order and then intentionally unsorted tool order through the `TestClient` path with the no-optimize app variant - Observed result: the PRE_SEND empty-tools runtime path now keeps `body_mutated` false where base would mark the request mutated, canonical-order tool payloads still preserve exact inbound bytes end-to-end, and unsorted tool-order payloads are still canonicalized as expected. - Batch coverage: no dedicated runtime batch regression test was added; batch no-op/mutation correctness is addressed through the same compare-and-assign pattern on both batch canonical-sort call sites. - Not tested: full-suite behavior outside the touched Anthropic forwarding regression surface ## 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 - [x] 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 ## Screenshots (if applicable) Not applicable. ## Additional Notes The important boundary is not whether tool arrays can be sorted. The real contract is whether a no-op canonicalization should count as a mutation. This change keeps canonicalization for real reorder cases and restores byte-faithful forwarding for already-canonical requests.
2026-06-21 13:07:29 -04:00
from headroom.proxy.handlers.anthropic import AnthropicHandlerMixin
fix: don't inject empty tools:[] when client omitted the tools field (#772) Fixes #728 ## Summary - `apply_session_sticky_ccr_tool` and `apply_session_sticky_memory_tools` always return a list — returning `[]` when `existing_tools=None` and nothing was injected - The old handler guard `if tools is not None:` evaluated `True` for `[]`, causing `body["tools"] = []` to be sent upstream on every request - vLLM-based providers (Venice.ai, etc.) strictly reject empty `tools` arrays with a 400 error **Fix:** Change the guard in both the OpenAI and Anthropic handlers from: ```python if tools is not None: body["tools"] = tools ``` to: ```python if tools or _original_tools is not None: body["tools"] = tools ``` The `_original_tools` variable is already defined in both handlers (`_original_tools = body.get("tools")`). This condition correctly handles all four cases: | Scenario | `tools` | `_original_tools` | Result | |---|---|---|---| | No client tools, no injection | `[]` | `None` | `False` → don't inject ✅ | | No client tools, CCR injected | `[ccr_tool]` | `None` | `True` → inject ✅ | | Client sent `tools: []` | `[]` | `[]` | `True` → preserve ✅ | | Client sent tools | `[A, ...]` | `[A, ...]` | `True` → preserve ✅ | ## Test plan - [x] New test file `tests/test_issue_728_empty_tools_injection.py` with 7 tests covering the guard condition and helper behavior - [x] All 51 existing CCR/golden-bytes tests still pass - [x] Zero changes to helper function return types or signatures ## Real behavior proof Tested against the helpers directly: ``` tests/test_issue_728_empty_tools_injection.py::TestHandlerGuardCondition::test_no_tools_no_injection_does_not_inject PASSED tests/test_issue_728_empty_tools_injection.py::TestHandlerGuardCondition::test_client_sent_empty_tools_is_preserved PASSED tests/test_issue_728_empty_tools_injection.py::TestHandlerGuardCondition::test_ccr_injection_sets_body_tools PASSED tests/test_issue_728_empty_tools_injection.py::TestHandlerGuardCondition::test_client_tools_always_set PASSED tests/test_issue_728_empty_tools_injection.py::TestCCRHelperNoToolsNoCompression::test_returns_empty_list_and_false_when_no_session_ccr PASSED tests/test_issue_728_empty_tools_injection.py::TestCCRHelperNoToolsNoCompression::test_returns_tool_list_when_compression_occurred PASSED tests/test_issue_728_empty_tools_injection.py::TestCCRHelperNoToolsNoCompression::test_no_double_injection_when_client_pre_registered_ccr_tool PASSED 7 passed in 1.81s ``` **What I did not test:** end-to-end against a live Venice.ai endpoint (no API key available), or passthrough mode with a real vLLM backend. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
2026-06-08 22:55:59 -07:00
from headroom.proxy.helpers import (
_reset_session_ccr_tracker_for_test,
apply_session_sticky_ccr_tool,
)
@pytest.fixture(autouse=True)
def _reset_tracker():
_reset_session_ccr_tracker_for_test()
yield
_reset_session_ccr_tracker_for_test()
# ---------------------------------------------------------------------------
# Guard-condition logic (the actual fix)
# ---------------------------------------------------------------------------
def _should_set_body_tools(tools: list | None, original_tools: list | None) -> bool:
"""Mirror the fixed handler condition: ``if tools or _original_tools is not None``."""
return bool(tools or original_tools is not None)
fix(proxy): keep PRE_SEND from reintroducing empty tool arrays (#2015) ## Description The direct body-write fix for empty `tools: []` already landed, but the later OpenAI PRE_SEND write-back path still reintroduces the empty array. This aligns that guard with the existing direct-assignment contract so tools-free requests stay tools-free while explicit client `tools: []` stays preserved. Anthropic's current-main PRE_SEND path already had the equivalent empty-tools protection and needed no code change. Closes #1983 ## 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 - Mirror the direct `tools or _original_tools is not None` guard in the OpenAI PRE_SEND write-back path. - Leave Anthropic unchanged because current `main` already protects the empty-tools case there. - Extend the focused #728 regression file with PRE_SEND-specific coverage. - Add a changelog note for providers that reject empty `tools` arrays. ## Testing - [x] Unit tests pass - [x] Linting passes - [ ] Type checking passes - [x] New tests added for new functionality when applicable - [ ] Manual testing performed ### Test Output ```text uv run pytest tests/test_issue_728_empty_tools_injection.py -q 11 passed uv run ruff check headroom/proxy/handlers/openai.py tests/test_issue_728_empty_tools_injection.py All checks passed uv run ruff format --check headroom/proxy/handlers/openai.py tests/test_issue_728_empty_tools_injection.py 2 files already formatted ``` ## Real Behavior Proof - Environment: OpenAI-compatible provider that rejects empty `tools` arrays - Exact command / steps: send a request without `tools`, then repeat with explicit `tools: []` - Observed result: the OpenAI PRE_SEND path now skips `tools: []` when the client omitted tools, while the focused regression still preserves explicit client `tools: []` and deliberate clearing of a previously present tool list - Not tested: live provider run on this host - Scope: PRE_SEND request-body write-back ## 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 - [x] 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 have updated the CHANGELOG.md if applicable ## Additional Notes The change is intentionally narrow. It only brings PRE_SEND write-back into parity with the direct-assignment guard that already exists.
2026-07-10 23:38:27 -04:00
def _should_apply_presend_tools(presend_tools: list | None, original_tools: list | None) -> bool:
"""Mirror the fixed PRE_SEND write-back condition in the OpenAI handler."""
return bool(presend_tools or original_tools is not None)
fix(proxy): preserve byte-faithful Anthropic tool forwarding (#1222) ## Description Anthropic tool sorting was rewriting `tools` lists even when canonical order was already present, which forced a mutation path that bypassed byte-faithful forwarding and reduced prefix-cache hit stability on repeated `/v1/messages` turns. Closes #1042 ## 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 - changed Anthropic tool canonicalization so already-canonical tool arrays are not rewritten in both single-request and batch paths - preserved byte-faithful forwarding for no-op single-request canonical tool-order requests by avoiding unnecessary `body["tools"]` reassignment - kept canonicalization behavior intact for out-of-order tool arrays - added one true regression proof for the PRE_SEND empty-tools path, plus forward-coverage tests for canonical no-op and real-sort-mutation request bodies in `test_proxy_byte_faithful_forwarding.py` - updated `CHANGELOG.md` to document the prefix-cache fix ## Testing - [x] Unit tests pass (`uv run pytest tests/test_proxy_handler_helpers.py tests/test_proxy_byte_faithful_forwarding.py -x -v`) - [x] Linting passes (`uv run ruff check headroom/proxy/handlers/anthropic.py tests/test_proxy_handler_helpers.py tests/test_proxy_byte_faithful_forwarding.py`) - [ ] Type checking passes (`uv run mypy headroom`) or explain N/A truthfully - [x] New tests added for new functionality when applicable - [ ] Manual testing performed ### Test Output ```text uv run pytest tests/test_proxy_handler_helpers.py tests/test_proxy_byte_faithful_forwarding.py -x -v 56 passed in 12.42s uv run ruff check headroom/proxy/handlers/anthropic.py tests/test_proxy_handler_helpers.py tests/test_proxy_byte_faithful_forwarding.py All checks passed! uv run ruff format headroom/proxy/handlers/anthropic.py tests/test_proxy_handler_helpers.py tests/test_proxy_byte_faithful_forwarding.py --check 3 files already formatted ``` ## Real Behavior Proof - Environment: Windows, local proxy test environment, Anthropic request-forwarding path - Exact command / steps: post `/v1/messages` requests with canonical tool order and then intentionally unsorted tool order through the `TestClient` path with the no-optimize app variant - Observed result: the PRE_SEND empty-tools runtime path now keeps `body_mutated` false where base would mark the request mutated, canonical-order tool payloads still preserve exact inbound bytes end-to-end, and unsorted tool-order payloads are still canonicalized as expected. - Batch coverage: no dedicated runtime batch regression test was added; batch no-op/mutation correctness is addressed through the same compare-and-assign pattern on both batch canonical-sort call sites. - Not tested: full-suite behavior outside the touched Anthropic forwarding regression surface ## 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 - [x] 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 ## Screenshots (if applicable) Not applicable. ## Additional Notes The important boundary is not whether tool arrays can be sorted. The real contract is whether a no-op canonicalization should count as a mutation. This change keeps canonicalization for real reorder cases and restores byte-faithful forwarding for already-canonical requests.
2026-06-21 13:07:29 -04:00
def _sort_tools(tools: list | None) -> list | None:
return AnthropicHandlerMixin._sort_tools_deterministically(tools)
def _should_set_body_tools_after_sort(tools: list | None, original_tools: list | None) -> bool:
"""Mirror fixed logic when candidate tools may already be sorted."""
if not _should_set_body_tools(tools, original_tools):
return False
sorted_tools = _sort_tools(tools)
if sorted_tools != tools:
tools = sorted_tools
return tools != original_tools
def _legacy_should_set_body_tools_after_sort(
tools: list | None, original_tools: list | None
) -> bool:
"""Older comparator that only wrote when sorting reordered."""
if not _should_set_body_tools(tools, original_tools):
return False
return _sort_tools(tools) != tools
fix: don't inject empty tools:[] when client omitted the tools field (#772) Fixes #728 ## Summary - `apply_session_sticky_ccr_tool` and `apply_session_sticky_memory_tools` always return a list — returning `[]` when `existing_tools=None` and nothing was injected - The old handler guard `if tools is not None:` evaluated `True` for `[]`, causing `body["tools"] = []` to be sent upstream on every request - vLLM-based providers (Venice.ai, etc.) strictly reject empty `tools` arrays with a 400 error **Fix:** Change the guard in both the OpenAI and Anthropic handlers from: ```python if tools is not None: body["tools"] = tools ``` to: ```python if tools or _original_tools is not None: body["tools"] = tools ``` The `_original_tools` variable is already defined in both handlers (`_original_tools = body.get("tools")`). This condition correctly handles all four cases: | Scenario | `tools` | `_original_tools` | Result | |---|---|---|---| | No client tools, no injection | `[]` | `None` | `False` → don't inject ✅ | | No client tools, CCR injected | `[ccr_tool]` | `None` | `True` → inject ✅ | | Client sent `tools: []` | `[]` | `[]` | `True` → preserve ✅ | | Client sent tools | `[A, ...]` | `[A, ...]` | `True` → preserve ✅ | ## Test plan - [x] New test file `tests/test_issue_728_empty_tools_injection.py` with 7 tests covering the guard condition and helper behavior - [x] All 51 existing CCR/golden-bytes tests still pass - [x] Zero changes to helper function return types or signatures ## Real behavior proof Tested against the helpers directly: ``` tests/test_issue_728_empty_tools_injection.py::TestHandlerGuardCondition::test_no_tools_no_injection_does_not_inject PASSED tests/test_issue_728_empty_tools_injection.py::TestHandlerGuardCondition::test_client_sent_empty_tools_is_preserved PASSED tests/test_issue_728_empty_tools_injection.py::TestHandlerGuardCondition::test_ccr_injection_sets_body_tools PASSED tests/test_issue_728_empty_tools_injection.py::TestHandlerGuardCondition::test_client_tools_always_set PASSED tests/test_issue_728_empty_tools_injection.py::TestCCRHelperNoToolsNoCompression::test_returns_empty_list_and_false_when_no_session_ccr PASSED tests/test_issue_728_empty_tools_injection.py::TestCCRHelperNoToolsNoCompression::test_returns_tool_list_when_compression_occurred PASSED tests/test_issue_728_empty_tools_injection.py::TestCCRHelperNoToolsNoCompression::test_no_double_injection_when_client_pre_registered_ccr_tool PASSED 7 passed in 1.81s ``` **What I did not test:** end-to-end against a live Venice.ai endpoint (no API key available), or passthrough mode with a real vLLM backend. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
2026-06-08 22:55:59 -07:00
class TestHandlerGuardCondition:
"""Verify the guard condition that decides whether to write body['tools']."""
def test_no_tools_no_injection_does_not_inject(self):
"""Client sent no tools and nothing was injected → body must stay tools-free."""
original_tools = None # client did not send tools
tools_after_helpers = [] # helpers return [] when existing_tools=None and no inject
assert not _should_set_body_tools(tools_after_helpers, original_tools), (
"Empty tools from helpers + no original tools must NOT write body['tools']"
)
def test_client_sent_empty_tools_is_preserved(self):
"""Client explicitly sent ``tools: []`` → preserve that field (their choice)."""
original_tools = [] # client explicitly sent an empty array
tools_after_helpers = [] # nothing injected
assert _should_set_body_tools(tools_after_helpers, original_tools), (
"Client's explicit tools:[] should be preserved in body"
)
def test_ccr_injection_sets_body_tools(self):
"""When CCR injects a tool into an originally tool-free request → set body."""
original_tools = None
from headroom.ccr.tool_injection import create_ccr_tool_definition
tools_after_helpers = [create_ccr_tool_definition("openai")]
assert _should_set_body_tools(tools_after_helpers, original_tools), (
"Injected CCR tool must reach body['tools']"
)
def test_client_tools_always_set(self):
"""Client provided real tools → always write body['tools']."""
original_tools = [{"type": "function", "function": {"name": "my_tool"}}]
tools_after_helpers = original_tools[:]
assert _should_set_body_tools(tools_after_helpers, original_tools)
fix(proxy): preserve byte-faithful Anthropic tool forwarding (#1222) ## Description Anthropic tool sorting was rewriting `tools` lists even when canonical order was already present, which forced a mutation path that bypassed byte-faithful forwarding and reduced prefix-cache hit stability on repeated `/v1/messages` turns. Closes #1042 ## 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 - changed Anthropic tool canonicalization so already-canonical tool arrays are not rewritten in both single-request and batch paths - preserved byte-faithful forwarding for no-op single-request canonical tool-order requests by avoiding unnecessary `body["tools"]` reassignment - kept canonicalization behavior intact for out-of-order tool arrays - added one true regression proof for the PRE_SEND empty-tools path, plus forward-coverage tests for canonical no-op and real-sort-mutation request bodies in `test_proxy_byte_faithful_forwarding.py` - updated `CHANGELOG.md` to document the prefix-cache fix ## Testing - [x] Unit tests pass (`uv run pytest tests/test_proxy_handler_helpers.py tests/test_proxy_byte_faithful_forwarding.py -x -v`) - [x] Linting passes (`uv run ruff check headroom/proxy/handlers/anthropic.py tests/test_proxy_handler_helpers.py tests/test_proxy_byte_faithful_forwarding.py`) - [ ] Type checking passes (`uv run mypy headroom`) or explain N/A truthfully - [x] New tests added for new functionality when applicable - [ ] Manual testing performed ### Test Output ```text uv run pytest tests/test_proxy_handler_helpers.py tests/test_proxy_byte_faithful_forwarding.py -x -v 56 passed in 12.42s uv run ruff check headroom/proxy/handlers/anthropic.py tests/test_proxy_handler_helpers.py tests/test_proxy_byte_faithful_forwarding.py All checks passed! uv run ruff format headroom/proxy/handlers/anthropic.py tests/test_proxy_handler_helpers.py tests/test_proxy_byte_faithful_forwarding.py --check 3 files already formatted ``` ## Real Behavior Proof - Environment: Windows, local proxy test environment, Anthropic request-forwarding path - Exact command / steps: post `/v1/messages` requests with canonical tool order and then intentionally unsorted tool order through the `TestClient` path with the no-optimize app variant - Observed result: the PRE_SEND empty-tools runtime path now keeps `body_mutated` false where base would mark the request mutated, canonical-order tool payloads still preserve exact inbound bytes end-to-end, and unsorted tool-order payloads are still canonicalized as expected. - Batch coverage: no dedicated runtime batch regression test was added; batch no-op/mutation correctness is addressed through the same compare-and-assign pattern on both batch canonical-sort call sites. - Not tested: full-suite behavior outside the touched Anthropic forwarding regression surface ## 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 - [x] 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 ## Screenshots (if applicable) Not applicable. ## Additional Notes The important boundary is not whether tool arrays can be sorted. The real contract is whether a no-op canonicalization should count as a mutation. This change keeps canonicalization for real reorder cases and restores byte-faithful forwarding for already-canonical requests.
2026-06-21 13:07:29 -04:00
def test_sorted_replacement_reaches_body(self):
"""A sorted replacement that differs from payload still needs to be written."""
original_tools = [{"name": "zeta"}, {"name": "alpha"}]
tools_after_helpers = [{"name": "alpha"}, {"name": "zeta"}]
assert not _legacy_should_set_body_tools_after_sort(tools_after_helpers, original_tools)
assert _should_set_body_tools_after_sort(tools_after_helpers, original_tools)
fix(proxy): keep PRE_SEND from reintroducing empty tool arrays (#2015) ## Description The direct body-write fix for empty `tools: []` already landed, but the later OpenAI PRE_SEND write-back path still reintroduces the empty array. This aligns that guard with the existing direct-assignment contract so tools-free requests stay tools-free while explicit client `tools: []` stays preserved. Anthropic's current-main PRE_SEND path already had the equivalent empty-tools protection and needed no code change. Closes #1983 ## 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 - Mirror the direct `tools or _original_tools is not None` guard in the OpenAI PRE_SEND write-back path. - Leave Anthropic unchanged because current `main` already protects the empty-tools case there. - Extend the focused #728 regression file with PRE_SEND-specific coverage. - Add a changelog note for providers that reject empty `tools` arrays. ## Testing - [x] Unit tests pass - [x] Linting passes - [ ] Type checking passes - [x] New tests added for new functionality when applicable - [ ] Manual testing performed ### Test Output ```text uv run pytest tests/test_issue_728_empty_tools_injection.py -q 11 passed uv run ruff check headroom/proxy/handlers/openai.py tests/test_issue_728_empty_tools_injection.py All checks passed uv run ruff format --check headroom/proxy/handlers/openai.py tests/test_issue_728_empty_tools_injection.py 2 files already formatted ``` ## Real Behavior Proof - Environment: OpenAI-compatible provider that rejects empty `tools` arrays - Exact command / steps: send a request without `tools`, then repeat with explicit `tools: []` - Observed result: the OpenAI PRE_SEND path now skips `tools: []` when the client omitted tools, while the focused regression still preserves explicit client `tools: []` and deliberate clearing of a previously present tool list - Not tested: live provider run on this host - Scope: PRE_SEND request-body write-back ## 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 - [x] 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 have updated the CHANGELOG.md if applicable ## Additional Notes The change is intentionally narrow. It only brings PRE_SEND write-back into parity with the direct-assignment guard that already exists.
2026-07-10 23:38:27 -04:00
def test_presend_empty_list_stays_omitted_when_client_omitted_tools(self):
"""PRE_SEND must not re-introduce ``tools: []`` for a tools-free request."""
assert not _should_apply_presend_tools([], None)
def test_presend_preserves_explicit_client_empty_tools(self):
"""PRE_SEND must still preserve an explicit client ``tools: []`` field."""
assert _should_apply_presend_tools([], [])
def test_presend_can_clear_previously_present_tools(self):
"""PRE_SEND may deliberately replace a real tool list with ``[]``."""
original_tools = [{"type": "function", "function": {"name": "my_tool"}}]
assert _should_apply_presend_tools([], original_tools)
fix: don't inject empty tools:[] when client omitted the tools field (#772) Fixes #728 ## Summary - `apply_session_sticky_ccr_tool` and `apply_session_sticky_memory_tools` always return a list — returning `[]` when `existing_tools=None` and nothing was injected - The old handler guard `if tools is not None:` evaluated `True` for `[]`, causing `body["tools"] = []` to be sent upstream on every request - vLLM-based providers (Venice.ai, etc.) strictly reject empty `tools` arrays with a 400 error **Fix:** Change the guard in both the OpenAI and Anthropic handlers from: ```python if tools is not None: body["tools"] = tools ``` to: ```python if tools or _original_tools is not None: body["tools"] = tools ``` The `_original_tools` variable is already defined in both handlers (`_original_tools = body.get("tools")`). This condition correctly handles all four cases: | Scenario | `tools` | `_original_tools` | Result | |---|---|---|---| | No client tools, no injection | `[]` | `None` | `False` → don't inject ✅ | | No client tools, CCR injected | `[ccr_tool]` | `None` | `True` → inject ✅ | | Client sent `tools: []` | `[]` | `[]` | `True` → preserve ✅ | | Client sent tools | `[A, ...]` | `[A, ...]` | `True` → preserve ✅ | ## Test plan - [x] New test file `tests/test_issue_728_empty_tools_injection.py` with 7 tests covering the guard condition and helper behavior - [x] All 51 existing CCR/golden-bytes tests still pass - [x] Zero changes to helper function return types or signatures ## Real behavior proof Tested against the helpers directly: ``` tests/test_issue_728_empty_tools_injection.py::TestHandlerGuardCondition::test_no_tools_no_injection_does_not_inject PASSED tests/test_issue_728_empty_tools_injection.py::TestHandlerGuardCondition::test_client_sent_empty_tools_is_preserved PASSED tests/test_issue_728_empty_tools_injection.py::TestHandlerGuardCondition::test_ccr_injection_sets_body_tools PASSED tests/test_issue_728_empty_tools_injection.py::TestHandlerGuardCondition::test_client_tools_always_set PASSED tests/test_issue_728_empty_tools_injection.py::TestCCRHelperNoToolsNoCompression::test_returns_empty_list_and_false_when_no_session_ccr PASSED tests/test_issue_728_empty_tools_injection.py::TestCCRHelperNoToolsNoCompression::test_returns_tool_list_when_compression_occurred PASSED tests/test_issue_728_empty_tools_injection.py::TestCCRHelperNoToolsNoCompression::test_no_double_injection_when_client_pre_registered_ccr_tool PASSED 7 passed in 1.81s ``` **What I did not test:** end-to-end against a live Venice.ai endpoint (no API key available), or passthrough mode with a real vLLM backend. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
2026-06-08 22:55:59 -07:00
# ---------------------------------------------------------------------------
# apply_session_sticky_ccr_tool behaviour with no existing tools
# ---------------------------------------------------------------------------
class TestCCRHelperNoToolsNoCompression:
"""Verify what the helper returns when there are no tools and no CCR happened."""
def test_returns_empty_list_and_false_when_no_session_ccr(self):
"""No session CCR history + no compression this turn → ([], False)."""
tools_out, was_injected = apply_session_sticky_ccr_tool(
provider="openai",
session_id="fresh-session-728",
request_id="req-1",
existing_tools=None,
has_compressed_content_this_turn=False,
)
assert was_injected is False
# Helper still returns [] — the guard in the handler is what prevents injection.
assert tools_out == []
def test_returns_tool_list_when_compression_occurred(self):
"""First turn with CCR → helper returns the CCR tool definition."""
tools_out, was_injected = apply_session_sticky_ccr_tool(
provider="openai",
session_id="ccr-session-728",
request_id="req-1",
existing_tools=None,
has_compressed_content_this_turn=True,
)
assert was_injected is True
tool_names = [t.get("function", {}).get("name") or t.get("name") for t in tools_out]
assert CCR_TOOL_NAME in tool_names
def test_no_double_injection_when_client_pre_registered_ccr_tool(self):
"""If the client already included the CCR tool, the helper must not duplicate it."""
from headroom.ccr.tool_injection import create_ccr_tool_definition
existing = [create_ccr_tool_definition("openai")]
tools_out, was_injected = apply_session_sticky_ccr_tool(
provider="openai",
session_id="pre-reg-session-728",
request_id="req-1",
existing_tools=existing,
has_compressed_content_this_turn=True,
)
assert was_injected is False
ccr_count = sum(
1
for t in tools_out
if (t.get("function", {}).get("name") or t.get("name")) == CCR_TOOL_NAME
)
assert ccr_count == 1, "CCR tool should appear exactly once"