fix(codex): preserve distinct requests when cumulative usage is missing - #1264
fix(codex): preserve distinct requests when cumulative usage is missing#1264roy-tong wants to merge 2 commits into
Conversation
ozymandiashh
left a comment
There was a problem hiding this comment.
Thanks for the fix and the cache bump. Two blockers.
-
tests/providers/codex.test.ts:1225fails (still dedups consecutive zero-cumulative duplicates: expected 1 call, got 2), on this head and on the head merged with currentmain.docs/providers/codex.md:164asks for exactly that suite plustests/parser-filter.test.tsto be run when the dedup key shape changes, and the PR template requiresnpm testto pass. -
The failing test is not stale; it is #257's deliberate decision. #257 changed the guard from
cumulativeTotal > 0toprevCumulativeTotal !== nullspecifically to stop double-counting consecutive zero-cumulativetoken_countevents. Your newtests/codex-missing-cumulative.test.tsasserts three identical consecutive events produce three calls, which reinstates exactly what #257 removed. Both sides rest on synthetic fixtures. The only thing that settles it is a real Codex rollout JSONL: does Codex ever repeat atoken_countevent for the same request? If you have one, attach a redacted excerpt (perdocs/providers/codex.md"Real data first"). If it does not repeat, delete the old test with that justification in the commit message; if it does, narrow the fix so byte-identical consecutive records still collapse.
On your question about double-counting: I probed it and all-cumulative and mixed sessions still behave like main thanks to the seenKeys backstop. The one weakened path is a forked session lacking cumulative totals outside the 5 s cutoff, where the forkedFromId replay protection no longer applies. You acknowledge the trade-off; please state it in the code comment you replaced rather than deleting the #1088 invariant note outright.
Also update docs/providers/codex.md:40-42 (dedup key shape) and the Quirks note on prevCumulativeTotal.
When
total_token_usageis absent, the old cumulative guard and all-zero deduplication key collapse subsequent requests. Keying only on last usage also loses distinct requests with equal token amounts.This revision:
total_tokens;Validation: 37 tests pass across the relevant Codex test files, including the new three-case regression and cache invalidation coverage; TypeScript
--noEmitpasses. The three missing-cumulative regressions fail on the previous PR head and pass with this revision. Fixtures are synthetic.