fix(stats): stop rejected upstream turns from feeding the savings funnel

[nangsontay]

Description

emit_request_outcome short-circuits failed turns before the savings/cost/log funnel, but only at status_code >= 500. The comment on that guard justified the cut as "4xx stay on the normal funnel: they are client errors the proxy still served."

For accounting that is the wrong cut. A 4xx is an error the proxy served but the provider did not: nothing was generated, nothing was billed, so compression on that turn saved nothing. Counting it lets rejected turns inflate every savings surface.

This was found on a live Claude Code session against Anthropic where fast mode was out of credits, so 143 of 300 turns came back 429 "Usage credits are required for fast mode". All 143 ran the full success funnel.

Type of Change

• 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/outcome.py: move the funnel short-circuit from status_code >= 500 to >= 400, so a turn the provider never billed cannot reach metrics.record_request, cost_tracker.record_tokens, the RequestLog feed, or the PERF line.
• headroom/proxy/outcome.py: route 429 to record_rate_limited instead of record_failed. An upstream rate limit is the one 4xx a user is expected to act on (back off, raise a cap); folding it into a generic failure count hides exactly that. Both counters are already exported and neither feeds savings.
• tests/test_outcome_records_5xx_as_failed.py: cover 429 (rate-limited), a parametrized sweep of other 4xx (400/401/403/404/413/422, all failed), and a 200 control asserting a served turn still reaches the funnel with its tokens_saved intact.
• tests/test_anthropic_buffered_ccr_usage.py (new): cover the Anthropic buffered-CCR route (stream:true + headroom_retrieve rewritten to a buffered stream:false upstream call). This is the dominant Claude Code path and had no test asserting its provider usage reaches the outcome. It does; these pin it so a regression cannot silently empty metrics.cache_by_provider.

Scope note: only outcome sites that pass a real status_code change behaviour. Sites that leave the field at its 200 default are untouched, including the response-cache-hit site in handlers/anthropic.py, which is correctly a served turn.

Testing

• Unit tests pass (pytest)
• Linting passes (ruff check .)
• Type checking passes (mypy headroom)
• New tests added for new functionality
• Manual testing performed

Test Output

$ uv run pytest tests/test_outcome_records_5xx_as_failed.py tests/test_anthropic_buffered_ccr_usage.py -q
13 passed, 1 warning in 2.95s

# Regression sweep over every consumer of the outcome funnel (13 files importing proxy.outcome)
$ uv run pytest $(grep -rln "proxy.outcome import" tests/) tests/test_anthropic_buffered_ccr_usage.py -q
218 passed, 1 warning in 6.98s

# stats / savings / metrics / anthropic / rate-limit / dashboard (50 files)
$ uv run pytest <50 files> -q
495 passed, 3 skipped, 2 warnings in 31.42s

# openai / gemini / bedrock / outcome / 5xx / retry / streaming (60 files)
$ uv run pytest <60 files> -q
595 passed, 44 skipped, 1 warning in 23.10s

$ uv run ruff check .
All checks passed!

$ uv run mypy headroom
Success: no issues found in 519 source files

RED proof — the new assertions fail against the pre-fix guard. Reverting only headroom/proxy/outcome.py to >= 500:

$ uv run pytest tests/test_outcome_records_5xx_as_failed.py -q
FAILED tests/test_outcome_records_5xx_as_failed.py::test_429_recorded_as_rate_limited_and_skips_success_funnel
FAILED tests/test_outcome_records_5xx_as_failed.py::test_other_4xx_recorded_as_failed_and_skips_success_funnel[400]
FAILED tests/test_outcome_records_5xx_as_failed.py::test_other_4xx_recorded_as_failed_and_skips_success_funnel[401]
FAILED tests/test_outcome_records_5xx_as_failed.py::test_other_4xx_recorded_as_failed_and_skips_success_funnel[403]
FAILED tests/test_outcome_records_5xx_as_failed.py::test_other_4xx_recorded_as_failed_and_skips_success_funnel[404]
FAILED tests/test_outcome_records_5xx_as_failed.py::test_other_4xx_recorded_as_failed_and_skips_success_funnel[413]
FAILED tests/test_outcome_records_5xx_as_failed.py::test_other_4xx_recorded_as_failed_and_skips_success_funnel[422]
7 failed, 3 passed in 0.16s

The 3 that pass either way are the pre-existing 529/503 cases and the 200 control, so the existing contract is unchanged.

Real Behavior Proof

• Environment: macOS 25.4 (arm64), Python 3.13.14, live headroom proxy --port 8787 --mode token --backend anthropic, client Claude Code against claude-opus-5, Anthropic fast mode out of credits.
• Exact command / steps: ran a normal Claude Code session through the proxy, then curl -s http://127.0.0.1:8787/stats and cross-checked against ~/.headroom/logs/proxy.log PERF lines; correlated request ids with grep 'UPSTREAM_ERROR status=429' and summed total_saved per id with awk.
• Observed result: 143 of the last 300 turns returned upstream 429 and every one ran the full success funnel. 46.5% of the headline savings (2,659,531 of 5,718,095 tokens) was compression on rejected turns; tokens.input accumulated 60.1M tokens never sent to a model; cost.savings_usd read $8.90 against a measured spend of $2.26 (a 79.7% "savings" rate); requests.rate_limited read 0 throughout because only Headroom's own limiter ever incremented it. proxy_inbound.by_status independently confirmed 145 x 429 on POST /v1/messages. With this patch those turns short-circuit: they land in requests_rate_limited and contribute nothing to savings, cost, or the request log.
• Not tested: the full pytest suite was not run end to end (unrelated pre-existing hangs in content_router native-detector and a serena subprocess block it on this machine); the 158 test files above were run instead. No re-run of the live 429 session against a patched proxy build — the production evidence is from the pre-fix build, and the post-fix behaviour is pinned by unit tests rather than re-observed live. Non-Anthropic providers were exercised only through their existing test suites, not live.

Runtime Rollout Safety

• Rollout-managed feature(s): none — this is unconditional accounting logic in the shared outcome funnel, not behind a rollout gate.
• Minimum rollout channel: N/A, ships on stable with no gate.
• Stable/default behavior changed: yes — 4xx outcomes no longer increment requests_total/tokens_saved_total/cost entries, and 429 now increments requests_rate_limited. Dashboards will show lower request and savings totals on deployments with upstream 4xx traffic. That is the correction, but it is a visible metric discontinuity worth calling out in release notes.
• Kill switch / disable path: none added. Revert is a one-line change of the threshold constant back to 500.
• Unsafe override required: no.
• Qualification impact: none — no rollout registry entry, digest, or qualification gate is touched.
• Rollback path: git revert this commit; the guard returns to >= 500 and no persisted state needs migrating (the change only affects in-memory counters and forward-going log rows).

Review Readiness

• I have performed a self-review
• This PR is ready for human review

Checklist

• My code follows the project's style guidelines
• 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
• My changes generate no new warnings
• 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
• 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 — no UI change. The dashboard renders the same fields; only the values stop counting rejected turns.

Additional Notes

Documentation checklist item is N/A: no documented behaviour or configuration surface changes, only the correctness of counters already described in docs/.

The comment the guard carried was a deliberate decision, not an oversight, so this PR is arguing the cut should move rather than that a line was forgotten. If maintainers prefer to keep 4xx visible in the per-request log feed while still excluding them from savings and cost, that is a reasonable alternative shape — it costs a partial-funnel branch instead of an early return, and I am happy to rework it that way.

Two related reporting problems found in the same investigation are deliberately not in this PR, to keep it to one logical change:

1. Output-shaping savings is a lifetime figure rendered inside the session payload. SavingsRecorder persists to ~/.headroom/output_savings.json and estimate_from_baseline sums the whole file, so /stats reported tokens.output_saved = 30,338,830 over n_requests = 40,575 next to a session tokens.output of 951. Scoping the estimator per-process would break it (it needs history to have a baseline stratum at all), so the fix is a labelling/rendering decision I did not want to make unilaterally.
2. Lifetime totals in SavingsTracker are internally impossible on a long-lived install: cache_read_tokens 4,944,608,741 exceeds total_input_tokens 3,978,346,063, and cache_savings_usd $20,544 exceeds total_input_cost_usd $5,273. The per-token cache-savings estimate deliberately diverges from proxy/cost.py's _CACHE_ECONOMICS multipliers (documented in _estimate_cache_savings_usd), which also yields three different cache-savings figures for one session.

Happy to open issues for either if useful.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[github-actions]

PR governance

This PR follows the template and is marked ready for human review.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[JerrettDavis]

Reviewed the latest head after updating it to current main. The early rejection boundary is correctly placed after operational outcome telemetry but before every success-only savings/cost/request-log side effect. Provider 429s are separated into the existing rate-limit counter; other 4xx/5xx remain failed, and Headroom’s own pre-upstream limiter still exits before this funnel, so this does not double-count its 429s. I also traced the buffered-CCR usage path to confirm the added coverage is relevant rather than incidental.

The exact-head focused suite passes locally (13/13). No blocking correctness findings.