fix(codex): stop double-counting reasoning in derived token totals - #1499
Conversation
Prepared with AI assistance (audit + implementation: gpt-5.6-sol; review: Claude Fable 5); reviewed and tested before filing. Co-Authored-By: gpt-5.6-sol <codex@openai.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A recorded `total_tokens` of zero alongside nonzero components means the field is unusable, not that the turn spent nothing, so it belongs on the same path as an absent total. Keeping reasoning in that branch left the inflated sum in place for exactly the records that cannot be trusted to report a total. Codex reports `reasoning_output_tokens` as a subset of `output_tokens`. Verified against 21815 real `token_count` records carrying nonzero reasoning: every one satisfies `total_tokens == input_tokens + output_tokens`, and none satisfies `total_tokens == input_tokens + output_tokens + reasoning_output_tokens`. `docs/guide/codex/index.md` documents the same rule, noting that reasoning tokens are part of the output charge rather than billed separately. Rollout session logs always record a usable total, so this only affects saved exec JSON usage, where OpenAI-style payloads may omit or zero the field. Costs were never affected because pricing reads input, cached input and output. Also cover a recorded nonzero total, the OpenAI field spellings, and an empty usage object, so the derivation stays pinned from every direction.
📝 WalkthroughWalkthroughCodex usage deserialization now derives missing or zero totals from input and output tokens without double-counting reasoning tokens. Tests cover the updated behavior, including saturation and saved usage expectations. ChangesCodex usage total handling
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
ccusage-guide | 6c5c4f6 | Commit Preview URL Branch Preview URL |
Jul 27 2026, 01:30 PM |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — fixes Codex derived token totals by dropping reasoning from the fallback (reasoning is already part of output_tokens) and treating explicit-zero totals the same as missing.
- Derive
total_tokensasinput + output— the oldinput + output + reasoningdouble-counted because Codex reports reasoning as a subset of output. Verified against real data (21,815 records with reasoning, zero counterexamples). - Treat zero total as absent — a recorded zero alongside nonzero components means the field is untrustworthy. The old filter kept zero only when all components were zero; the new filter treats all zeros uniformly, and the
leaves_an_empty_usage_total_at_zerotest confirms the all-zero case still yields 0. - Add 5 focused unit tests covering absent total, explicit zero total, recorded nonzero total, OpenAI field spellings, and empty usage.
@v0 or keep the SHA fresh with Dependabot ｜ View workflow run ｜ Using DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
rust/adapters/codex/src/types.rs (1)
519-530: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMake the recorded-total test exercise the preservation branch.
The fixture’s recorded total (
150) equalsinput + output, so this test would pass even if the implementation ignored the recorded field and always derived the value. Use a distinct positive recorded total and assert that exact value.Suggested test adjustment
- "total_tokens": 150 + "total_tokens": 151 ... - assert_eq!(usage.total_tokens, 150); + assert_eq!(usage.total_tokens, 151);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/adapters/codex/src/types.rs` around lines 519 - 530, Update the keeps_a_recorded_total test fixture so total_tokens is a positive value different from input_tokens plus output_tokens, then assert that exact recorded value to exercise preservation of the serialized total field.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rust/adapters/codex/src/types.rs`:
- Around line 291-297: Update the total_tokens calculation in the surrounding
token-field conversion to defer fallback evaluation and use saturating addition:
retain positive total_tokens values, while invoking input.saturating_add(output)
only when the total is absent or non-positive.
---
Nitpick comments:
In `@rust/adapters/codex/src/types.rs`:
- Around line 519-530: Update the keeps_a_recorded_total test fixture so
total_tokens is a positive value different from input_tokens plus output_tokens,
then assert that exact recorded value to exercise preservation of the serialized
total field.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a041d3ec-cf99-4f5e-b95c-840bff237ccb
📒 Files selected for processing (2)
rust/adapters/codex/src/loader.rsrust/adapters/codex/src/types.rs
ccusage
@ccusage/ccusage-darwin-arm64
@ccusage/ccusage-darwin-x64
@ccusage/ccusage-linux-arm64
@ccusage/ccusage-linux-x64
@ccusage/ccusage-win32-x64
commit: |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 2 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
`unwrap_or` evaluates its argument eagerly, so the sum was computed even when a recorded total was preserved. A corrupt or hostile log carrying two near-`u64` component values would then panic under the checked arithmetic of a debug build and wrap in release. Defer the sum and saturate it. Also pin the preservation branch properly. The recorded total in `keeps_a_recorded_total` was `input + output`, so the test passed whether the value was preserved or re-derived. Use a total that cannot be derived. Raised by CodeRabbit on #1499.
ccusage performance comparisonPR SHA: Performance comparison skipped. Base package URL was not ready before 300.000s. Fixture performance comparison requires a base package when --base-dir is not provided. Base package: |
ccusage performance comparisonPR SHA: Performance comparison skipped. Base package URL was not ready before 300.000s. Fixture performance comparison requires a base package when --base-dir is not provided. Base package: |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — commit 6c5c4f6 addresses review-bot feedback from the prior round: deferred evaluation with saturating_add and a recorded-total test that cannot accidentally pass through derivation.
- Switch to
unwrap_or_elsewithsaturating_add— the inner addition now evaluates lazily and saturates atu64::MAXinstead of wrapping (release) or panicking (debug). - Strengthen
keeps_a_recorded_total— the fixture total is now 151, distinct from the derivedinput + outputof 150, so the test genuinely exercises the preservation branch. - Add
saturates_a_derived_total_instead_of_overflowing— input ofu64::MAX+ 5 saturates tou64::MAX.
@v0 or keep the SHA fresh with Dependabot ｜ View workflow run ｜ Using DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
ccusage performance comparisonPR SHA: Performance comparison skipped. Base package URL was not ready before 300.000s. Fixture performance comparison requires a base package when --base-dir is not provided. Base package: |
ccusage performance comparisonPR SHA: Performance comparison skipped. Base package URL was not ready before 300.000s. Fixture performance comparison requires a base package when --base-dir is not provided. Base package: |
Supersedes #1458, keeping @MaxGhenis's commit intact. Their branch could not be updated in place:
MaxGhenis/ccusageis a fork ofjackcpku/ccusagerather than a direct fork of this repository, so GitHub reportsmaintainer_can_modify: truebut rejects the push.Problem
Codex reports
reasoning_output_tokensas a subset ofoutput_tokens, sototal_tokensisinput_tokens + output_tokens. The fallback used when a record omitstotal_tokensadded reasoning on top, inflating the reported total.A saved
codex exec --jsonturn with 100 input, 50 output and 20 reasoning tokens reported 170 instead of 150:Costs were never affected, because pricing reads input, cached input and output directly.
Verification
The semantics were checked against real data rather than assumed. Across 170,429
token_countrecords in a local~/.codex/sessions, 21,815 carry a nonzeroreasoning_output_tokens:total_tokens == input_tokens + output_tokenstotal_tokens == input_tokens + output_tokens + reasoning_output_tokensdocs/guide/codex/index.mdalready documents the same rule, noting that reasoning tokens are part of the output charge rather than billed separately, so no documentation change is needed — the code now matches the guide.Rollout session logs always record a usable total (0 of 170,429 records omit it or report zero alongside nonzero components), so this only affects saved exec JSON usage, where OpenAI-style payloads may omit the field.
Changes
fix(codex): correct legacy token total fallback(@MaxGhenis) — derive an absent total as input plus output.fix(codex): derive a recorded zero token total the same way— a recordedtotal_tokensof zero alongside nonzero components means the field is unusable, not that the turn spent nothing, so it takes the same path. Leaving reasoning in that branch kept the inflated sum for exactly the records that cannot be trusted to report a total. Updates theloader.rsassertion that encoded the old value (14 to 13) and pins the derivation from every direction: recorded nonzero total, absent total, recorded zero, OpenAI field spellings, empty usage object.Testing
just test— full suite green, 0 failuresjust fmt— 0 changedcargo clippy -p ccusage-adapter-codex --all-targets— no warningsNeed help on this PR? Tag
@codesmith-botwith what you need. Autofix is enabled.Summary by cubic
Fixes Codex token totals by removing double-counting of reasoning tokens and treating zero totals as absent. Derived totals now use saturating input + output, so saved exec JSON reports correct totals; costs were never affected.
total_tokensasinput_tokens + output_tokenswhen missing or explicitly zero; do not add reasoning tokens.Written for commit 6c5c4f6. Summary will update on new commits.
Summary by CodeRabbit
input + output(with saturation to prevent overflow) while preserving any explicitly reported non-zero totals.