akadmin/headroom
1
0
Fork 0
mirror of https://github.com/headroomlabs-ai/headroom.git synced 2026-08-27 14:17:10 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions
2630 commits 410 branches 169 tags 142 MiB
Commit graph

3 commits

Author SHA1 Message Date
Abhay Singh
9ca5a16bde
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`.
 2026-08-17 20:21:09 -07:00
Abhay Singh
c19e412b33
fix(proxy/bedrock): report uncached input tokens from backend usage, not the live-zone count (#2318) 
## Description

On the buffered Anthropic-backend path (Bedrock / Vertex /
LiteLLM-anthropic, non-streaming) the proxy reports
`uncached_input_tokens` as `0` for essentially every cached multi-turn
request.

The handler reads the backend's Anthropic-shaped `usage` and then
re-derives the uncached count from a re-tokenized live-zone count:

```python
usage = backend_response.body.get("usage", {})
...
attempted_input_tokens = tokenizer.count_messages(
    original_client_messages[frozen_message_count:]   # the LIVE ZONE only
)
...
uncached_input_tokens = max(0, attempted_input_tokens - cr_tokens - cw_tokens)
```

`attempted_input_tokens` is deliberately the **live-zone** token count
(the new-turn messages after the frozen prefix), kept as the denominator
for the active-compression ratio. It is not the full request size.
Subtracting the whole-request cache metrics (`cache_read` +
`cache_creation`) from it is nonsensical: on any turn whose cached
prefix is larger than the new turn -- the normal multi-turn case --
`attempted_input_tokens - cr - cw` goes negative and `max(0, ...)`
clamps it to `0`. So the uncached input, which feeds the cost/uncached
dashboards, is reported as `0`.

Meanwhile the backend already computes the correct value.
`_anthropic_usage_from_litellm` (added in #1345) sets:

```python
"input_tokens": max(prompt_tokens - cache_read - cache_write, 0),
```

i.e. `usage.input_tokens` is exactly the uncached input, in Anthropic
semantics. The direct-API path already uses it (`uncached_input_tokens =
usage.get("input_tokens", 0)`); the backend path was the one re-deriving
it.

## Fix

Prefer the backend's `usage.input_tokens`, matching the direct-API path
-- but guard on the backend actually reporting it, so a backend that
omits `input_tokens` (or sends `null`) does not silently record
`uncached=0`:

```python
_reported_input_tokens = usage.get("input_tokens")
if _reported_input_tokens is not None:
    uncached_input_tokens = int(_reported_input_tokens)
else:
    # Backend did not report it: fall back to the live-zone derivation,
    # which is never worse than the previous behaviour.
    uncached_input_tokens = max(0, attempted_input_tokens - cr_tokens - cw_tokens)
```

A plain `usage.get("input_tokens", 0)` would have re-introduced the `0`
on any backend that doesn't translate the prompt-token field; the guard
keeps the authoritative value when present and the old estimate
otherwise. `attempted_input_tokens` is unchanged and still used as the
compression-ratio denominator.

## 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

- `headroom/proxy/handlers/anthropic.py`: set `uncached_input_tokens`
from `usage.input_tokens` on the buffered anthropic-backend path when
the backend reports it; otherwise fall back to the prior live-zone
derivation.
- `tests/test_backend_nonstreaming_cache_metrics.py`: added two tests
driving the buffered path -- one asserting the recorded
`RequestOutcome.uncached_input_tokens == usage.input_tokens` (with a
live zone far smaller than the cache), and one asserting that when the
backend omits `input_tokens` the value falls back to the non-zero
live-zone derivation instead of collapsing to `0`.

## Testing

- [x] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check`)
- [x] Type checking passes (`mypy`)
- [x] New tests added for the fix and the fallback guard

### Test Output

```text
# Fail-before, primary fix (old max(0, attempted - cr - cw)):
tests/..::test_anthropic_backend_nonstreaming_uncached_from_usage_input_tokens
  -> uncached=0, expected 1000  (FAIL)

# Fail-before, safety guard (naive usage.get("input_tokens", 0)):
tests/..::test_anthropic_backend_nonstreaming_uncached_falls_back_when_input_tokens_absent
  -> assert 0 > 0  (FAIL)

# Pass-after (guarded fix):
tests/test_backend_nonstreaming_cache_metrics.py  6 passed

# uvx ruff@0.15.17 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.17 and mypy 1.20.2 via uvx.
- Steps: drove the buffered anthropic-backend path end to end via
`create_app` + FastAPI `TestClient` with a mock `AnyLLMBackend`
returning an Anthropic-shaped body, and spied on
`HeadroomProxy._record_request_outcome` to capture the recorded
`RequestOutcome`. With `usage.input_tokens=1000`, `cache_read=500`,
`cache_write=200` and a two-token live zone, the old derivation recorded
`uncached=0`; the fix records `1000`. With `input_tokens` omitted from
`usage`, the naive default records `0` while the guarded fallback
records the non-zero live-zone count.
- Observed result: `RequestOutcome.uncached_input_tokens` now reflects
the real uncached input on cached backend turns, and never regresses
below the previous estimate when a backend omits the field.
- Not tested: a live Bedrock/Vertex call (no cloud credentials here).
The value flows through the same `RequestOutcome` funnel the proxy uses
for cost/telemetry, exercised directly.

## 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
- [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

Rebased onto current `main` and squashed to a single commit. The
fallback guard is the only behavioural difference from a plain "use
`usage.input_tokens`" change: it ensures the fix cannot regress a
backend that doesn't report the field back to `uncached=0`.
 2026-08-11 23:47:33 -05:00
Sujit Patil
85804043ff
fix(proxy): record cache metrics for non-streaming backend paths (#1271) 
## Description

Fixes missing cache metric propagation in backend-routed non-streaming
request paths.

The streaming implementations already populate cache usage metrics
(`cache_read`, `cache_write`, cache hit percentage) in `RequestOutcome`,
but the equivalent non-streaming paths were left incomplete after the P0
proxy pipeline audit:

- `anthropic.py` (Bedrock / Vertex non-streaming): extracted only
`output_tokens` from the backend usage block — `cache_read_input_tokens`
and `cache_creation_input_tokens` were never read. A comment in the code
explicitly acknowledged this: *"Cache metrics aren't extracted from the
backend response here yet — that's a follow-up."*
- `openai.py` (OpenAI backend non-streaming): extracted cache metrics
and fed them to `openai_prefix_tracker`, but never forwarded them into
`RequestOutcome`. The values were computed then silently dropped.

As a result, all non-streaming backend-routed requests reported:

```text
cache_read=0 cache_write=0 cache_hit_pct=0
```

even when upstream usage data contained valid cache counters.

## Type of Change

- [x] Bug fix (non-breaking change that fixes an issue)

## Changes Made

- `headroom/proxy/handlers/anthropic.py`: Extract
`cache_read_input_tokens`, `cache_creation_input_tokens`, and TTL bucket
splits (`cache_write_5m_tokens`, `cache_write_1h_tokens`) from the
Bedrock non-streaming usage block. Compute `uncached_input_tokens`. Pass
all five fields to `RequestOutcome`.
- `headroom/proxy/handlers/openai.py`: Compute `uncached_input_tokens`
and forward the already-extracted `cache_read_tokens`,
`cache_write_tokens`, and `uncached_input_tokens` into `RequestOutcome`
in the backend non-streaming path.

## Testing

- [x] New tests added for new functionality
- [x] Manual testing performed

### Test Output

```text
# Existing regression suite that specifically targets this omission:
# tests/test_backend_nonstreaming_cache_metrics.py
#
# Module docstring from the file explicitly documents the bug class:
#
#   "The **non-streaming** backend paths were left behind — the same bug class
#    on the parallel code path: anthropic.py extracted only output_tokens;
#    openai.py extracted cache fields but never threaded them into RequestOutcome."
#
# Four tests cover both handlers and both the positive (cache data present)
# and zero (no cache data in upstream response) cases:
#
#   test_openai_backend_nonstreaming_emits_perf_with_cache_read_and_inferred_write
#   test_openai_backend_nonstreaming_perf_zeros_when_upstream_omits_cache_usage
#   test_anthropic_backend_nonstreaming_emits_perf_with_cache_read_and_write
#   test_anthropic_backend_nonstreaming_perf_zeros_when_upstream_omits_cache_usage
#
# Tests were written to fail on main before this fix (intentional regression tests).
# Local test execution is blocked by a missing MSVC toolchain (maturin/headroom._core
# Rust extension cannot compile on this machine without VS Build Tools).
```

## Real Behavior Proof

- **Environment:** Windows, Python 3.13, headroom main branch (commit
`b70fccbe`)
- **Exact steps:** Inspected the `RequestOutcome` construction in both
non-streaming backend branches. Confirmed that `cache_read_tokens` and
`cache_write_tokens` defaulted to `0` in both paths because the
constructor calls omitted them.
- **Observed result (pre-fix):** `PERF` log line emitted `cache_read=0
cache_write=0 cache_hit_pct=0` for every non-streaming Bedrock/backend
request, even when the upstream response body contained
`cache_read_input_tokens: 500, cache_creation_input_tokens: 200`.
- **Observed result (post-fix):** `RequestOutcome` now receives the
extracted values; the funnel passes them through to Prometheus, the cost
tracker, `RequestLog`, and the `PERF` line — matching the existing
streaming path behavior.
- **Not tested:** Live Bedrock / Vertex endpoint (no credentials on this
machine). The fix is a pure pass-through of values already present in
the parsed response body.

## Review Readiness

- [x] I have performed a self-review before requesting human 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 or that my
feature works

## Additional Notes

The regression test file
`tests/test_backend_nonstreaming_cache_metrics.py` was intentionally
written to expose this exact omission (it was not added after the fix).
The streaming sibling fix was tracked as issue #327; this PR closes the
parallel non-streaming gap. The fix is a pure observability change — no
request or response payloads are modified.

---------

Co-authored-by: Sujit <sujit@example.com>
 2026-07-09 14:06:05 -05:00