mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
fix(proxy/anthropic): coerce present-null usage counters on the buffered backend path (#3084)
## Description
The buffered (non-streaming) Anthropic backend branch in
`handle_anthropic_messages` (`headroom/proxy/handlers/anthropic.py`) —
the path taken by Bedrock / Vertex / LiteLLM(anthropic) traffic — read
the response usage counters with a bare default:
```python
output_tokens = usage.get("output_tokens", 0)
...
cr_tokens = usage.get("cache_read_input_tokens", 0)
cw_tokens = usage.get("cache_creation_input_tokens", 0)
```
A backend can report these counters as JSON `null` (key **present**,
value null) rather than omitting them. For a present-null key
`dict.get(key, 0)` returns `None`, not the default `0`. That `None` then
flowed into:
```python
provider_input_tokens=(uncached_input_tokens + cr_tokens + cw_tokens)
```
raising `TypeError: unsupported operand type(s) for +: 'NoneType' and
'NoneType'`, which the outer handler converted into a failed turn (HTTP
500 `api_error`) instead of a normal 200 with zeroed counters.
The direct-Anthropic-API branch a few hundred lines down already guards
this exact case with `int(usage.get(key, 0) or 0)`, and the surrounding
code even comments that a backend may "send null" for `input_tokens`
(and None-guards that field). The buffered branch was simply left
behind, so the two parallel paths disagreed on null handling.
## Fix
Coerce the three counters on the buffered path with `int(usage.get(key,
0) or 0)`, exactly matching the direct-API idiom, so a present-null
value becomes `0` instead of `None`. The already-present `input_tokens
is not None` guard is unaffected, and its fallback subtraction now
operates on coerced ints.
## Type of Change
- [x] Bug fix (non-breaking change that fixes an issue)
- [ ] New feature
- [ ] Breaking change
- [ ] Documentation update
- [ ] Performance improvement
- [ ] Code refactoring (no functional changes)
## Changes Made
- `headroom/proxy/handlers/anthropic.py` (buffered backend branch of
`handle_anthropic_messages`): coerce `output_tokens`,
`cache_read_input_tokens` and `cache_creation_input_tokens` with
`int(usage.get(key, 0) or 0)` so a present-null value is treated as `0`,
matching the direct-Anthropic path.
- `tests/test_backend_nonstreaming_cache_metrics.py`: added
`test_anthropic_backend_nonstreaming_present_null_cache_counters_do_not_crash`,
driving the buffered backend path with present-null `output_tokens` /
`cache_read_input_tokens` / `cache_creation_input_tokens` and asserting
a 200 with a recorded `RequestOutcome` whose counters are `0` and whose
uncached input comes from the present `input_tokens`.
## Testing
- [x] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check`)
- [x] Type checking passes (`mypy`)
- [x] New tests added
### Test Output
```text
tests/test_backend_nonstreaming_cache_metrics.py 7 passed
# uvx ruff@0.15.22 check -> All checks passed!
# uvx mypy@1.20.2 headroom/proxy/handlers/anthropic.py -> Success: no issues found in 1 source file
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.12.11, project venv, pytest 9.1.1,
ruff 0.15.22 and mypy 1.20.2 via uvx.
- Exact command / steps: ran the new regression against the unpatched
handler and captured the crash (`python -m pytest
tests/test_backend_nonstreaming_cache_metrics.py::test_anthropic_backend_nonstreaming_present_null_cache_counters_do_not_crash
-x -q` -> `assert 500 == 200` with body
`{"type":"error","error":{"type":"api_error","message":"unsupported
operand type(s) for +: 'NoneType' and 'NoneType'"}}`); applied the
`int(... or 0)` coercion; re-ran the whole file (`python -m pytest
tests/test_backend_nonstreaming_cache_metrics.py -q` -> 7 passed); then
`uvx ruff@0.15.22 format`, `uvx ruff@0.15.22 check`, and `uvx
mypy@1.20.2 headroom/proxy/handlers/anthropic.py`.
- Observed result: before the fix a backend response whose usage carries
`cache_read_input_tokens: null` (or a null `output_tokens` /
`cache_creation_input_tokens`) returned HTTP 500 and recorded no
outcome; after the fix the same response returns 200, the counters
coerce to `0`, and the `PERF` line reports `cache_read=0 cache_write=0`.
- Not tested: a live Bedrock/Vertex session emitting a real null-counter
usage block (the null-usage shape is reproduced directly through the
mocked backend that the existing suite already uses for this path).
## Runtime Rollout Safety
- Rollout-managed feature(s): none. This is the buffered Anthropic
response-accounting path behind `handle_anthropic_messages`, not a
rollout-channel-gated runtime feature.
- Minimum rollout channel: N/A (no rollout-managed behavior).
- Stable/default behavior changed: yes, as a bug fix. A backend response
with present-null usage counters now completes with a 200 and zeroed
counters instead of failing the turn with a 500. Responses with numeric
counters are unaffected.
- Kill switch / disable path: N/A. There is no behavioral toggle; the
change only hardens numeric coercion on the accounting path and does not
alter routing, compression, or request forwarding.
- Unsafe override required: no.
- Qualification impact: Bedrock / Vertex / LiteLLM(anthropic)
non-streaming turns that report a null cache/output counter stop 500-ing
and are recorded with zeroed counters, matching the direct-Anthropic
path.
- Rollback path: revert this PR; the buffered path returns to the bare
`usage.get(key, 0)` reads.
## 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] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective
- [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
## Additional Notes
This mirrors the recently fixed Gemini CCR-continuation present-null
usage bug: the same `dict.get(key, default)` present-null trap, on the
parallel Anthropic backend path. Only the buffered (non-streaming)
backend branch was affected; the direct-Anthropic and streaming paths
already coerce with `or 0`.
This commit is contained in:
Abhay Singh
GitHub
parent
c3c921f2f7
commit
9ca5a16bde
2 changed files with 89 additions and 3 deletions
|
|
@ -3267,7 +3267,12 @@ class AnthropicHandlerMixin:
|
|||
# Track metrics
|
||||
total_latency = (time.time() - start_time) * 1000
|
||||
usage = backend_response.body.get("usage", {})
|
||||
output_tokens = usage.get("output_tokens", 0)
|
||||
# A backend may report these counters as JSON null (key
|
||||
# present, value null), for which ``.get(key, 0)`` returns
|
||||
# ``None`` rather than the default. Coerce with ``or 0`` so
|
||||
# the arithmetic below (and ``RequestOutcome``) never sees
|
||||
# ``None`` — matching the direct-Anthropic path.
|
||||
output_tokens = int(usage.get("output_tokens", 0) or 0)
|
||||
|
||||
_backend_name = request_backend.name if request_backend else "anthropic"
|
||||
# Eligible-only denominator for the active
|
||||
|
|
@ -3286,8 +3291,8 @@ class AnthropicHandlerMixin:
|
|||
except Exception:
|
||||
attempted_input_tokens = original_tokens
|
||||
|
||||
cr_tokens = usage.get("cache_read_input_tokens", 0)
|
||||
cw_tokens = usage.get("cache_creation_input_tokens", 0)
|
||||
cr_tokens = int(usage.get("cache_read_input_tokens", 0) or 0)
|
||||
cw_tokens = int(usage.get("cache_creation_input_tokens", 0) or 0)
|
||||
cw_5m_tokens, cw_1h_tokens = self._extract_anthropic_cache_ttl_metrics(
|
||||
usage
|
||||
)
|
||||
|
|
|
|||
|
|
@ -492,3 +492,84 @@ def test_anthropic_backend_nonstreaming_uncached_falls_back_when_input_tokens_ab
|
|||
# No input_tokens reported and no cache: the live-zone derivation yields the
|
||||
# non-zero token count of the new turn, never the 0 the naive default gave.
|
||||
assert outcome.uncached_input_tokens > 0
|
||||
|
||||
|
||||
def test_anthropic_backend_nonstreaming_present_null_cache_counters_do_not_crash() -> None:
|
||||
"""Present-but-null usage counters must coerce to 0, not crash the turn.
|
||||
|
||||
A backend can send the cache/output counters as JSON ``null`` (key present,
|
||||
value null) rather than omitting them — the direct-Anthropic path already
|
||||
guards this with ``int(usage.get(key, 0) or 0)`` and the surrounding code
|
||||
even acknowledges a backend that "sends null" for ``input_tokens``. The
|
||||
buffered backend branch, however, read ``usage.get(key, 0)`` for
|
||||
``output_tokens`` / ``cache_read_input_tokens`` / ``cache_creation_input_tokens``,
|
||||
and ``.get`` returns ``None`` for a present-null key (the default applies
|
||||
only to an absent key). That ``None`` then flowed into
|
||||
``uncached_input_tokens + cr_tokens + cw_tokens`` and the prefix-tracker
|
||||
calls, raising ``TypeError`` that the outer handler turned into a failed
|
||||
request instead of a normal 200 with zeroed counters.
|
||||
"""
|
||||
from headroom.proxy.server import HeadroomProxy
|
||||
|
||||
config = ProxyConfig(
|
||||
optimize=False,
|
||||
cache_enabled=False,
|
||||
rate_limit_enabled=False,
|
||||
backend="anyllm",
|
||||
anyllm_provider="anthropic",
|
||||
)
|
||||
body = {
|
||||
"id": "msg_1",
|
||||
"type": "message",
|
||||
"role": "assistant",
|
||||
"model": "claude-3-5-sonnet-20241022",
|
||||
"content": [{"type": "text", "text": "hi"}],
|
||||
"stop_reason": "end_turn",
|
||||
"usage": {
|
||||
"input_tokens": 1000,
|
||||
"output_tokens": None,
|
||||
"cache_read_input_tokens": None,
|
||||
"cache_creation_input_tokens": None,
|
||||
},
|
||||
}
|
||||
backend = _make_anthropic_backend(body)
|
||||
|
||||
captured: list[Any] = []
|
||||
orig_record = HeadroomProxy._record_request_outcome
|
||||
|
||||
async def _spy(self, outcome): # noqa: ANN001, ANN202
|
||||
captured.append(outcome)
|
||||
return await orig_record(self, outcome)
|
||||
|
||||
log_handle = _attach_proxy_log_capture()
|
||||
try:
|
||||
with (
|
||||
patch("headroom.proxy.server.AnyLLMBackend", return_value=backend),
|
||||
patch.object(HeadroomProxy, "_record_request_outcome", _spy),
|
||||
):
|
||||
app = create_app(config)
|
||||
with TestClient(app) as client:
|
||||
resp = client.post(
|
||||
"/v1/messages",
|
||||
json={
|
||||
"model": "claude-3-5-sonnet-20241022",
|
||||
"messages": [{"role": "user", "content": "hi"}],
|
||||
"max_tokens": 64,
|
||||
},
|
||||
headers={"x-api-key": "sk-ant-test", "anthropic-version": "2023-06-01"},
|
||||
)
|
||||
assert resp.status_code == 200, resp.text[:200]
|
||||
finally:
|
||||
_detach_proxy_log_capture(*log_handle)
|
||||
|
||||
assert captured, "expected a recorded RequestOutcome (turn must not have crashed)"
|
||||
outcome = captured[-1]
|
||||
# Null counters coerce to 0; the present input_tokens still drives uncached.
|
||||
assert outcome.output_tokens == 0
|
||||
assert outcome.cache_read_tokens == 0
|
||||
assert outcome.cache_write_tokens == 0
|
||||
assert outcome.uncached_input_tokens == 1000
|
||||
|
||||
handler = log_handle[0]
|
||||
cr, cw, chp = _find_perf_record(handler.records)
|
||||
assert (cr, cw, chp) == (0, 0, 0)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue