Fix token counters: post-finish usage frame, rate-limit wipe, session fidelity, estimate disclosure#78
Merged
Merged
Conversation
rate limits, persist full usage fidelity, disclose estimates Wire-level diagnosis (HI_DEBUG_STREAM against pipenetwork/glm-5.2-fast): with stream_options.include_usage, spec-compliant providers send usage in a separate chunk AFTER the finish chunk (empty choices). The collector broke on finish_reason, so that frame was never read — streamed calls got zero provider usage and every counter silently ran on chars/4 estimates. (pipenetwork's frame is currently all zeros anyway, so numbers there stay estimates — but now correctly captured when any route/provider populates it, and honestly labeled either way.) Fixes: - openai/stream: after finish_reason, keep reading for the trailing usage frame, bounded by a 2s grace (POST_FINISH_USAGE_GRACE) so providers that hold the stream open after finish still can't hang a completed answer — the guarantee break-on-finish gave. Usage on the finish chunk itself still terminates immediately. - Usage::add: a booking with no rate-limit snapshot (side-calls, error usage, estimates) no longer wipes the last observed one — that made the rate-limit display blank out mid-session. - Usage.estimated (new, serde-default): set whenever a token field was backfilled from a chars/4 estimate; sticky across add(); surfaced as "usage_estimated" in --report so estimated numbers are never mistaken for provider-metered ones. - backfill_missing_usage: also backfills context_occupancy (previously stayed 0 on the estimate path, blinding occupancy consumers). - session JSONL: persist cache_read/cache_creation/estimated on the usage record (serde defaults keep old session files loading); resume restores full-fidelity totals instead of silently dropping cache counters. Tests: usage_in_post_finish_chunk_is_captured (the regression), all_zero_usage_frame_marks_output_estimate (the observed pipenetwork shape), add_preserves_last_observed_rate_limits_and_sticks_estimated, usage_round_trips_cache_tokens_and_estimated_marker; legacy usage-line compat already covered by load_history_skips_corrupted_lines. Full workspace suite green; fmt/clippy clean. Live: identical call old vs fixed binary → same totals (1809/1/1810), no latency regression (drain breaks on the usage frame, 1.2s e2e), usage_estimated: true now disclosed on pipenetwork. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
The report: "token counters sometimes stop counting"
Wire-level diagnosis (
HI_DEBUG_STREAMagainst pipenetwork /glm-5.2-fast):With
stream_options.include_usage(hi's default), spec-compliant providers send usage in a separate chunk after the finish chunk. The collector broke onfinish_reason— chunk [3] was never read. So streamed calls got zero provider usage and every counter has been running onchars/4estimates viabackfill_missing_usage. On pipenetwork specifically the frame is currently all zeros anyway (so numbers there remain estimates even post-fix) — but the frame is now captured wherever it's populated, and labeled honestly either way.Three more defects found on the same trail:
Usage::addsetrate_limits = other.rate_limitsunconditionallybackfill_missing_usagenever setcontext_occupancyFixes
finish_reason, keep reading for the trailing usage frame under a 2sPOST_FINISH_USAGE_GRACE, so a provider that holds the stream open after finish still can't hang a completed answer (the guarantee break-on-finish gave). Usage attached to the finish chunk itself still terminates immediately — zero added latency on the normal path.Usage::add: rate-limits only replaced by a booking that actually carries a snapshot ("latest observed" semantics).Usage.estimated(new, serde-default): set whenever a token field was backfilled from achars/4estimate; sticky acrossadd(); surfaced asusage_estimatedin--report. Estimated numbers can no longer masquerade as provider-metered ones — on pipenetwork today,usage_estimated: true.backfill_missing_usagealso backfillscontext_occupancy.cache_read/cache_creation/estimated(serde defaults — old session files still load); resume restores full totals.Tests
usage_in_post_finish_chunk_is_captured— the headline regression (spec chunk shape)all_zero_usage_frame_marks_output_estimate— the exact observed pipenetwork shapeadd_preserves_last_observed_rate_limits_and_sticks_estimatedusage_round_trips_cache_tokens_and_estimated_marker(+ legacy-line compat already covered byload_history_skips_corrupted_lines)-D warningsclean.Live verification
Identical bare call, old binary vs fixed: same totals (
1809/1/1810), 1.2s end-to-end (drain exits on the usage frame — no grace penalty), and the report now carriesusage_estimated: trueon pipenetwork.Caveat worth knowing: pipenetwork's usage frame is all zeros (their side), so counters there remain hi-side estimates — now disclosed. The frame does carry
ipop.cost_usd; capturing provider-billed cost is a possible follow-up.🤖 Generated with Claude Code