fix(telemetry): only advance usage-report baseline after a 200 (#2149)

## Description

The usage reporter permanently drops a reporting window's usage whenever
the send to the cloud fails.

`UsageReporter._report_usage` computes usage as a **delta** against the
last snapshot, POSTs it, and then rebases the baseline:

```python
try:
    resp = await client.post(f"{self._cloud_url}/v1/license/usage", json=payload, timeout=10.0)
    if resp.status_code == 200:
        ...
    else:
        logger.warning("Usage report returned status %d", resp.status_code)
except Exception:
    logger.warning("Failed to send usage report", exc_info=True)

# Update snapshot
self._snapshot_metrics()      # runs on success, non-200, AND exception
self._last_report_time = now
```

`_snapshot_metrics()` rebases `_last_tokens_saved_by_model` /
`_last_tokens_sent_by_model` / `_last_requests_by_model` to the current
cumulative counters. Because it runs unconditionally after the POST, a
report that fails to send — non-200 or a raised exception — still
advances the baseline. The module is explicitly built to tolerate a
briefly-unreachable cloud (7-day grace, cached license), so this is a
normal, recurring situation.

The consequence: the failed window's requests and tokens are never
re-included. The next report is a delta from the advanced baseline, so
that window is silently and permanently lost from usage-based billing /
quota. Every transient network blip under-counts usage. (The
`total_requests == 0` early-return already gets this right — it advances
only `_last_report_time`, without snapshotting, since there's nothing to
lose.)

## Fix

Advance the baseline (`_snapshot_metrics()` and `_last_report_time`)
only inside the `resp.status_code == 200` branch. On a non-200 or an
exception, both baselines are left intact, so the next report covers the
full period since the last successful send and re-includes the
previously-failed window.

Closes #

## 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/telemetry/reporter.py`: move `_snapshot_metrics()` +
`_last_report_time = now` into the 200 branch of `_report_usage`.
- `tests/test_usage_reporter_snapshot.py`: new tests — baseline advances
on 200, and stays intact on a non-200 and on an exception (window
preserved).
- `CHANGELOG.md`: Bug Fixes entry.

## Testing

- [ ] 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
$ uvx ruff@0.15.17 check headroom/telemetry/reporter.py tests/test_usage_reporter_snapshot.py
All checks passed!
$ python -m py_compile headroom/telemetry/reporter.py tests/test_usage_reporter_snapshot.py
OK
```

## Real Behavior Proof

- Environment: Windows 11, Python 3.12, `uvx ruff@0.15.17`. Importing
`headroom` pulls in the torch/transformers stack and a full `pytest`
gets OOM-killed on this box, so I verified the delta accounting with a
dependency-free script that models two windows across a failed then
successful send, and left the full pytest to CI.
- Exact command / steps: window 1 saves 100 tokens and the send fails;
window 2 saves another 50 (cumulative 150) and the send succeeds. Ran
under the old (unconditional snapshot) and new (snapshot-on-200) logic.
- Observed result: old delivers only 50 tokens total (window 1's 100
dropped when the baseline advanced on the failed send); new delivers the
full 150 (window 2's delta re-includes window 1). The new tests assert
the baseline advances on 200 and is untouched on a non-200 / exception,
driving the real `_report_usage` with a fake proxy + client.
- Not tested: a live cloud round-trip; full local `pytest` deferred to
CI (OOM, per above).

## 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 or that my
feature works
- [ ] New and existing unit tests pass locally with my changes
- [x] I have updated the CHANGELOG.md if applicable

## Additional Notes

The "unit tests pass locally" and "type checking" boxes are unchecked
because the full suite imports the ML stack, which I can't run in this
environment; the change moves two lines into the success branch,
verified by the standalone delta-accounting proof and new tests that
drive the real `_report_usage` via `object.__new__` with a fake proxy
and HTTP client (200, non-200, and exception).

Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
This commit is contained in:
Abhay Singh 2026-07-14 21:49:43 +05:30 committed by GitHub
parent fb17156bfa
commit 0cddac632d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 83 additions and 4 deletions

View file

@ -118,6 +118,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* **proxy:** run a cold-start fast pass before background-compression deferral so byte-identical freeze doesn't lock sessions to the uncompressed transcript. Since #1850, a session's provider-cached prefix is frozen in whatever form its cold start forwarded; deferring the WHOLE pipeline (`HEADROOM_BACKGROUND_COMPRESSION=1`, frozen=0, ≥50k tokens) therefore cached the raw transcript and forfeited the session's compression savings for its lifetime — including sub-second lossless wins like `read_lifecycle` stale-read drops, observed in the field as sessions permanently stuck at 0 savings. The deferral branch now runs the pipeline synchronously with the new `skip_kompress=True` kwarg (everything except the Kompress ML stage — the only stage that can blow the request budget per #1171) under a bounded fast-pass budget (`HEADROOM_COLD_START_FAST_PASS_TIMEOUT_SECONDS`, default 10s), forwards the pruned form, and defers only Kompress to the background job (tagged `deferred:kompress_background`). Fail-open: on fast-pass timeout/error the request forwards uncompressed exactly as before. Units routed to Kompress under `skip_kompress` take the same fallback as when the model isn't ready.
* **init/codex:** don't overwrite the user's `hooks.json`. `_ensure_codex_hooks` wrote a fresh payload containing only Headroom's two hooks, wholesale-replacing `~/.codex/hooks.json` — so any user-managed Codex hooks (and other top-level keys) were silently destroyed on `headroom init codex`. It now read-merges: existing entries are preserved, Headroom's are deduped on the `headroom-init-codex` marker and appended, matching `_ensure_claude_hooks` / `_ensure_copilot_hooks`.
* **ccr:** detect `read_lifecycle` stale/superseded markers in the retrieve-tool injector so they stay redeemable. Those markers (`[Read content stale: … Retrieve original: hash=<hash>]`) store the original bytes in the CCR store under a valid hash, but none of `CCRToolInjector`'s patterns matched them — every pattern required the word "compressed" or the `<<ccr:` form. So on a frozen-prefix turn (both `read_lifecycle` and prefix freezing are on by default) the injector reported no compressed content, the `headroom_retrieve` tool was not injected, and the model was handed a marker advertising `Retrieve original: hash=X` with no tool to redeem it — silent data loss for stale reads, where retrieval is the only way to recover the original-at-read-time content (the exact case the #1006 guard exists to prevent). Added a pattern matching the load-bearing `Retrieve original: hash=` phrase, aligning the injector with the sibling `read_maturation` marker that was already (incidentally) detected.
* **telemetry:** only advance the usage-report baseline after a confirmed 200. `UsageReporter._report_usage` sends usage as a delta against the last snapshot, but it called `_snapshot_metrics()` (and advanced `_last_report_time`) unconditionally after the POST — including when the send returned non-200 or raised. So a report that failed to reach the cloud (which the module is explicitly designed to tolerate) still rebased the baseline, permanently dropping that window's requests/tokens from usage-based billing/quota; the next report started from the advanced baseline and never re-included them. The baseline now advances only on a 200, so a failed send leaves the window intact for the next report to retry.
* **savings:** stop the durable savings ledger from billing free (0-priced) models at the `$3/M` fallback. `estimate_cost_usd` guarded the litellm estimate with `if priced > 0`, so a genuinely free model — where `_estimate_compression_savings_usd` correctly returns `0.0` — was treated as "unpriced" and fell through to the blended fallback rate, writing phantom cost-avoided into the JSONL ledger and surfacing it in `headroom savings`. The ledger now trusts the estimate verbatim for known models (it already falls back internally for models litellm can't price and returns `0.0` for free ones), fixing the same defect at this call site that was already fixed inside the helper.
* **init/codex:** stop `headroom init codex` from deleting per-profile provider settings. `_ensure_codex_provider` removed the root-level `model_provider`/`openai_base_url` (which init owns) with a multiline regex that matched those keys in **every** table, so a user's `[profiles.*]` overrides (e.g. `[profiles.work] model_provider = "azure"`) were silently stripped and those profiles fell through to the injected `"headroom"` default — config corruption. The strip is now scoped to the document root (everything before the first table header), so per-profile overrides are preserved while init still replaces a root-level assignment.
* **proxy:** strip output-only content blocks from request messages before forwarding. Anthropic's server-side refusal-fallback feature (`server-side-fallback-2026-06-01`) emits a `{"type":"fallback","from":{...},"to":{...}}` block inside the assistant response to signal that a refused request was re-served by the fallback model. That block is valid on the *response* path but rejected on the *request* path, so when a client replays the assistant turn the next request 400s (`invalid_request_error: messages.N.content.0: Input tag 'fallback' ...`) and the conversation gets permanently stuck through the proxy. `read_request_json_with_bytes` (Anthropic/OpenAI/Bedrock) and `_read_request_json` (Gemini) now drop such blocks — re-encoding the raw bytes so byte-faithful passthrough cannot leak the pre-strip body, backfilling a benign text block if a turn is emptied, and leaving requests without such blocks byte-identical (no cache churn).

View file

@ -332,15 +332,21 @@ class UsageReporter:
total_requests,
total_tokens_saved,
)
# Only advance the baseline after a confirmed 200. Usage is a
# delta against the last snapshot, so snapshotting on a failed
# send (non-200 or exception) would permanently drop this
# window's requests/tokens from billing — the next report starts
# from the advanced baseline and never re-includes them. Leaving
# both baselines intact makes the next report retry the full
# period. (The empty-report early-return above only advances the
# time, since there is nothing to lose.)
self._snapshot_metrics()
self._last_report_time = now
else:
logger.warning("Usage report returned status %d", resp.status_code)
except Exception:
logger.warning("Failed to send usage report", exc_info=True)
# Update snapshot
self._snapshot_metrics()
self._last_report_time = now
def _snapshot_metrics(self) -> None:
"""Take a snapshot of current proxy metrics for delta computation."""
if self._proxy is None or self._proxy.cost_tracker is None:

View file

@ -0,0 +1,72 @@
"""UsageReporter must only advance its delta baseline after a confirmed 200.
Snapshotting on a failed send permanently drops that window's usage from the
delta-based usage report (billing/quota under-count)."""
from __future__ import annotations
from types import SimpleNamespace
import anyio
from headroom.telemetry.reporter import UsageReporter
class _Resp:
def __init__(self, status_code: int) -> None:
self.status_code = status_code
def json(self) -> dict:
return {}
def _make_reporter(post_outcome) -> UsageReporter:
r = object.__new__(UsageReporter)
ct = SimpleNamespace(
_tokens_saved_by_model={"m": 100},
_tokens_sent_by_model={"m": 400},
_requests_by_model={"m": 5},
)
r._proxy = SimpleNamespace(cost_tracker=ct)
r._last_report_time = None
r._last_tokens_saved_by_model = {}
r._last_tokens_sent_by_model = {}
r._last_requests_by_model = {}
r._license_key = "k"
r._cloud_url = "https://cloud.example"
r._license_info = None
class _Client:
async def post(self, *args, **kwargs):
if isinstance(post_outcome, Exception):
raise post_outcome
return _Resp(post_outcome)
async def _get_client():
return _Client()
r._get_client = _get_client
return r
def test_baseline_advances_only_on_success():
r = _make_reporter(200)
anyio.run(r._report_usage)
# Success -> baseline rebased to the current cumulative counters.
assert r._last_tokens_saved_by_model == {"m": 100}
assert r._last_requests_by_model == {"m": 5}
def test_baseline_not_advanced_on_non_200():
r = _make_reporter(500)
anyio.run(r._report_usage)
# Failed send -> baseline untouched so the window is retried next report.
assert r._last_tokens_saved_by_model == {}
assert r._last_requests_by_model == {}
def test_baseline_not_advanced_on_exception():
r = _make_reporter(RuntimeError("network down"))
anyio.run(r._report_usage)
assert r._last_tokens_saved_by_model == {}
assert r._last_requests_by_model == {}