fix(usage): bill Codex requests whose cumulative counters rewind to an earlier snapshot#29366

External image Major · Data Integrity & Integration · Heavy lift

Do not treat every rewind as a new request.

After (110,22) is billed, a replay of (110,22) with last=(10,2) after (140,27) can pass the new rewind check because its prefix (100,20) is valid. The parser then bills it again and moves the baseline back to (110,22). A later (145,28) with last=(5,1) can be marked malformed.

The duplicate billing is newly introduced. The baseline regression and later malformed record also occur before this change because rebasing happens before validation. The existing replay guard covers only repeated restarted first requests. Track already-billed snapshots separately from genuine rewinds, preserve the accepted rewind behavior required by #29363, and add this middle-snapshot replay case.

✅ Confirmed as addressed by @jokaperes

Valid. Without a guard, a re-emitted older snapshot whose counters sit below the latest one passes the new check and gets billed twice. Addressed in 84f9e3aec.

A rewound snapshot only counts if its timestamp is later than every cumulative snapshot already seen. The scan state now stores that time as one optional field (latestCumulativeAtMs), so older cached states still decode. A re-emitted snapshot carries its original timestamp, so it keeps main's behaviour: it is skipped as malformed and not billed. The real cases in #29363 are all the first request of a new turn, 3 minutes to 2 hours after the previous snapshot, so they are still billed.

I didn't track every billed snapshot. That would grow the persisted scan state with each request, for a replay pattern that doesn't occur in any of the 786 real rollouts I replayed. Replaying them again with this commit gives the same result as before: exactly the 3 rewound requests recovered, nothing else changed.

Coverage: "does not bill a re-emitted earlier snapshot again when cumulative counters rewind" is your sequence: (100,20), (110,22), (140,27), then (110,22) re-emitted. It fails on the first commit ([120, 12, 35, 12], 0 skipped) and passes now ([120, 12, 35, 0], 1 skipped, same as main).

The later malformed record after such a replay comes from rebasing before validation, which is unchanged from main, so I left it out of scope. An older snapshot re-emitted with a new timestamp would still be billed; the PR description lists that as the limit.

@jokaperes The timestamp guard addresses the duplicate-billing case I reported. In the supplied code, rebaseCodexUsage retains the maximum timestamp, and rewound compares against that value before the current rebase. The replay with its original timestamp therefore fails reconciliation and is not billed. Later genuine rewinds can still pass unseen-prefix validation.

Tracking every billed snapshot is not required for this fix. The new-timestamp replay limit is explicit. The existing rebase-before-validation behavior can remain outside this change.

I checked the supplied implementation; I did not run the tests. Would you like a follow-up issue for the existing rebase-before-validation behavior?

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

Review thread resolved.

_{You are interacting with an AI system.}

External image Major · Data Integrity & Integration · Quick win

Re-parse all v11 Codex cache entries.

A clean v11 Codex entry restores its lastCumulativeUsage without latestCumulativeAtMs. The rewind check in codexUsage.ts treats that missing timestamp as acceptance. The first appended record that re-emits an older snapshot can then be billed again, for example a (110,22) snapshot after a cached (140,27) baseline. Forcing a re-parse of v11 Codex entries rebuilds the timestamp before incremental parsing resumes.

Update the comment above legacy so it no longer says clean v8+ entries stay served unchanged.

✅ Confirmed as addressed by @jokaperes

Agreed on the gap, fixed in 030ccd43d with a narrower rule than re-parsing every v11 entry.

resumable now excludes Codex entries cached before v12 (it was < 11, from #29415). A clean v11 entry is still served while its rollout is unchanged, so unchanged history isn't re-read. Once the file grows, it re-parses from offset 0 instead of resuming, which rebuilds latestCumulativeAtMs before any appended snapshot is checked. A re-emitted (110,22) after a cached (140,27) is then compared against the real latest snapshot time and skipped, as in "does not bill a re-emitted earlier snapshot again". v11 entries with skipped records are still fully re-parsed.

The new usageScanCache.test.ts case checks both v11 paths and fails without the change. Usage suites pass 336/336, and format, lint, knip, graphql and typecheck pass.

@jokaperes The narrower fix addresses the gap. The supplied code disables resumption for all pre-v12 Codex entries and resets their position and state.

Clean v11 entries can remain cached while unchanged. When a rollout grows, parsing starts at offset 0 and rebuilds latestCumulativeAtMs. Entries with skipped records still force a re-parse.

My proposed blanket invalidation was unnecessary. Thanks for fixing this while preserving unchanged history.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

Review thread resolved.

_{You are interacting with an AI system.}

External image Major · Data Integrity & Integration · Heavy lift

Preserve resolved tiers when reparsing a grown v11 file.

A grown clean v11 Codex file reparses from offset zero. UsageService.readFileRecords preserves cached tier corrections only when parsed.resumed is true. Therefore, if Codex history is unavailable, the full reparse can replace previously resolved fast or service tiers with standard pricing, and the replacement can be persisted.

Preserve matching cached tier corrections across full reparses. Add a service test that grows a v11 file after its tier history becomes unavailable.

✅ Addressed in commit 6b94121

Valid, though it isn't specific to v11: any whole-file re-parse (the pre-v11 rule from #29415, cache entries with skipped records, a rewritten file) dropped saved tier corrections the same way. Fixed in 6b9412129 in UsageService.readFileRecords.

When a re-parse doesn't resume, each re-emitted record whose tier is unresolved takes the saved correction of the cached record with the same identity. The key is the existing tentative-tail key: provider, session, timestamp, model, rate model, totals, reported cost and dedupe key. Records already resolved by the parser and records with no cached match go through normal resolution, unchanged.

New service test: "keeps resolved Codex tiers when a grown pre-v12 cache entry re-parses without history". It resolves an ultrafast turn, downgrades the saved cache to v11, makes the history table unavailable and appends a standard turn. The grown file re-parses from zero and keeps the ultrafast price (cost 8, persisted across a restart). Without the change it falls back to standard (cost 3). Usage suites pass 337/337, and format, lint, knip, graphql and typecheck pass.

@jokaperes You’re right: this affects whole-file reparses, not only v11 migration.

I verified the fix in 6b9412129 against the current code. It preserves saved tier corrections for matching unresolved records, including tail records. Parser-resolved records and unmatched records keep the existing resolution path.

The regression test checks cost 8 after the reparse and again after restart, with history unavailable. This addresses the finding. I inspected the test but did not run it in this sandbox.

📝 Learnings were not changed: only organization members or users with write access to this repository can add or remove learnings.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

Review thread resolved.

_{You are interacting with an AI system.}