feat(models): record the full token breakdown, not just the totals - #101
Merged
Conversation
`UsageStats` declared `reasoning_tokens` and `cached_tokens` and no dialect ever populated them, so a converted checkpoint that held its score while burning twice the reasoning budget was invisible -- the delivery regression no accuracy metric catches. Both OpenAI-shaped dialects now lift `prompt_tokens_details` / `completion_tokens_details`, and sglang surfaces the `cached_tokens` it was already reading twenty lines below for its radix-cache guard. The optional counts are `int | None`, not `int = 0`. Most OpenAI-compatible servers omit the detail objects entirely, so a zero default makes "no cache hits" and "never said" the same number, and any average over a mixed fleet silently wrong. The distinction is carried to disk: `ModelUsage` gains `NotRequired` keys written only where the server reported one, so an absent key means unreported and a present `0` is a real measurement. The same rule holds one layer up: `profile.json`'s `share` is absent when its parent count is 0. A server counting reasoning outside its completion count can report reasoning against 0 completion tokens, and `0.0` there is not a measured 0% but an undefined ratio -- the same lie the counts are optional to avoid. `total` still carries what was reported. No subset relation is enforced. `reasoning <= completion` is an OpenAI convention rather than a wire guarantee, and a server counting reasoning outside its completion count is exactly the one whose reported total exceeds the computed one. That case is now recorded instead of discarded: `reported_total_tokens` is kept only where it disagrees with prompt+completion, so it is absent on almost every call and, when present, is the only evidence that a provider's accounting differs from ours. #100 had nowhere to put that difference and dropped it silently. The profiler reports the new fields as shares against the tokens of the calls that reported them -- never against every call, which understates a share by the non-reporting fraction -- and prints that denominator on every line rather than only when partial, since a coverage note given sometimes reads as full coverage the rest of the time. They are never folded into `total_tokens`, which stays prompt+completion. `record_model_usage` and `aggregate_token_usage` carried the same accumulation logic twice and now share one accumulator: a field added to the live path alone would have vanished on the next resume. They share its admission test too -- the rebuild used to count a usage the live path skips, inflating `calls_total` after a resume but not on the original run. The rebuild clears the new accumulators for the same reason: they are additive. The record projection lands in `_legacy_bridge`, not `model`. #99 moved `ModelUsage` and `response_to_model_output` there after this work was branched, and the conflict it raises is only reported against the old home. Deliberately not collected: `audio_tokens`. Nothing in sieval sends or receives audio, so it would ship as a permanently absent key. Both changed record shapes are released -- `ModelUsage` and `ProfileStageTokenUsage` each ship at v0.7.0 -- so these additions belong in the release notes. Every new key is `NotRequired` and written only where the server reported it, so nothing reading the existing keys breaks and no migration is needed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ethan-scitix
force-pushed
the
feat/usage-token-breakdown
branch
from
August 14, 2026 07:15
5afd054 to
8729a19
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type
Summary
UsageStatshas declaredreasoning_tokensandcached_tokenssince the binding plane landed, and no dialect ever populated them. So a converted or requantized checkpoint that held its score while burning twice the reasoning budget was invisible — the delivery regression no accuracy metric catches. This collects the breakdown, persists it, and reports it.prompt_tokens_details/completion_tokens_details(reasoning, cached, accepted/rejected speculative-decoding tokens) through one shareddialects/_usage.py; chat and completions receive the same wire object, so the reader has one owner rather than two copies that drift the next time OpenAI adds a field. sglang surfaces thecached_tokensit was already reading twenty lines below for its radix-cache guard.int | None, notint = 0. Most OpenAI-compatible servers omit the detail objects entirely, so a zero default makes "no cache hits" and "never said" the same number, and any average over a mixed fleet silently wrong. Carried to disk:ModelUsagegainsNotRequiredkeys written only where the server reported one, so an absent key means unreported and a present0is a real measurement.reasoning <= completionis an OpenAI convention, not a wire guarantee — a server counting reasoning outside its completion count is exactly the one whose reported total exceeds the computed one.reported_total_tokensis kept only where it disagrees with prompt+completion, so it is absent on almost every call and, when present, is the only evidence that a provider's accounting differs from ours. fix(models): compute usage totals, and reject bad builder defaults at bind time #100 had nowhere to put that difference and dropped it silently; this closes that.total_tokens, which stays prompt+completion.One coupling fixed on the way.
record_model_usageandaggregate_token_usagecarried the same accumulation logic twice — the live path and the resume rebuild. They now share one accumulator, because a field added to the live path alone would have vanished on the next resume. The rebuild also clears the new accumulators, which are additive and would otherwise double-count everything they re-read.Deliberately not collected:
audio_tokens. Nothing in sieval sends or receives audio, so it would ship as a permanently absent key.Related Issues
Refs #25. Follow-up from the review of #100, which established the computed total and flagged that a server/computed disagreement was being discarded with no trace.
Test Plan
Automated
ruff check && ruff format --check)ty check)pdm run pytest) — 5359 passed. Twotests/performancegates (efficiency/memory ratios) fail only when the whole suite runs and pass in isolation; pre-existing load sensitivity, unrelated to this diff.sieval/coregate:_usage.py100%,transports/sglang.py100%,model.py99%,ir.py98%,openai_completions.py97%,openai_chat.py96%,profiler.py96%.Manual
Mutation-tested every new test — each reverted decision fails only the tests that should catch it:
0instead ofNonereported_total_tokensrecorded even when it agreescached_tokensacross the n samples_model_usagewrites0instead of omitting an unreported keyThe
Nonevs0distinction is pinned from both directions, since it is the whole reason the fields are optional: sglang's_metafixture really does reportcached_tokens: 0, and that 0 survives to the record; a meta with the key deleted yieldsNone. Same pair at the dialect layer and across theModelUsagerecord boundary.Checklist
Required (all PRs)
type(scope): description)AI-Generated Code - <model> (<provider>)in module docstring — newdialects/_usage.pycarries itcore/aggregate_token_usagemoved into the shared_accumulate_usageand is covered by the rebuild testIf: Breaking Change
On-disk record shape only, and all of it is unreleased — no migration path, no CHANGELOG entry.
sieval/core/models/dialects/does not exist atv0.7.0; the whole binding plane arrived in #45 after that tag. Two existing dialect tests asserted that a divergent reported total was discarded, and one sglang pair assertedcached_tokenswas dropped; all four now assert the new, deliberate behaviour rather than the old absence.