fix(codex): preserve distinct requests when cumulative usage is missing

[roy-tong]

When total_token_usage is 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:

• applies the cumulative guard only to a finite, nonnegative reported total_tokens;
• treats absent, null and partial totals as missing identity rather than zero;
• uses source path and physical record offset for fallback identity, preserving distinct equal-usage rows and stable same-file rereads;
• deliberately makes no cross-file replay-deduplication claim when cumulative identity is unavailable;
• invalidates both Codex result and provider/session caches so existing aggregates are reparsed.

Validation: 37 tests pass across the relevant Codex test files, including the new three-case regression and cache invalidation coverage; TypeScript --noEmit passes. The three missing-cumulative regressions fail on the previous PR head and pass with this revision. Fixtures are synthetic.

[ozymandiashh]

Thanks for the fix and the cache bump. Two blockers.

1. tests/providers/codex.test.ts:1225 fails (still dedups consecutive zero-cumulative duplicates: expected 1 call, got 2), on this head and on the head merged with current main. docs/providers/codex.md:164 asks for exactly that suite plus tests/parser-filter.test.ts to be run when the dedup key shape changes, and the PR template requires npm test to pass.

2. The failing test is not stale; it is #257's deliberate decision. #257 changed the guard from cumulativeTotal > 0 to prevCumulativeTotal !== null specifically to stop double-counting consecutive zero-cumulative token_count events. Your new tests/codex-missing-cumulative.test.ts asserts 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 a token_count event for the same request? If you have one, attach a redacted excerpt (per docs/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.

[roy-tong]

You asked for a real Codex rollout to settle it, so I scanned one: codeset-ai/codeset-release-evals (public eval corpus) — 53 sessions, 1313 token_count events:

• 603 events (46%) are byte-identical repeats of their predecessor — same total_token_usage AND same last_token_usage, ~0.5–3s apart. Re-emission is the norm, not the exception.
• 0 cumulative monotonicity violations — totals never regress within a session.
• 54 events (4%) carry no usable total_token_usage at all — so the missing-cumulative case the PR addresses is real, it just cannot be allowed to un-collapse the re-emissions.

Verbatim excerpt (lines 14/17 vs 24 of gpt54_codeset_gym/baseline/simonw__symbex-2/sessions/…jsonl; timestamps differ, info is identical):

{"timestamp":"…T16:22:18.697Z","type":"event_msg","payload":{"type":"token_count","info":{"total_token_usage":{"input_tokens":11923,"cached_input_tokens":0,"output_tokens":208,"reasoning_output_tokens":54,"total_tokens":12131},"last_token_usage":{"input_tokens":11923,…identical…},"model_context_window":258400}}}
{"timestamp":"…T16:22:19.467Z","type":"event_msg","payload":{"type":"token_count","info":{…identical to the line above…}}}
{"timestamp":"…T16:22:22.661Z","type":"event_msg","payload":{"type":"token_count","info":{"total_token_usage":{"input_tokens":26873,…},"last_token_usage":{"input_tokens":14950,…}}}}   ← next request, payload differs

That lands on your "it does repeat" branch, so 4c3a2b6 narrows the fix exactly as you specified:

1. A record whose info payload is byte-identical to the previous event's collapses, with or without cumulative totals — tests/providers/codex.test.ts:1225 passes again unchanged. Identity compares info only, deliberately: rate_limits mutate between re-emissions (credits drain) and must not mint a new request.
2. Only records with differing payloads and no cumulative identity count as distinct — the rewritten tests/codex-missing-cumulative.test.ts now asserts both halves: identical ×3 → 1 call; differing payloads ×3 → 3 calls.
3. The fix(codex): exclude harness startup and fix partial-dedup timing in Tok/s #1088 invariant note is restored at the dedupKey in adapted form, stating the accepted trade-off: without cumulative identity, replay protection past the 5s fork cutoff is deliberately not guessed.
4. docs/providers/codex.md documents the three-layer dedup and the Quirks entry for the no-cumulative minority.

prevInfoIdentity is persisted in the resume state so an incremental re-read keeps the collapse boundary. Locally: tests/providers/codex.test.ts, tests/codex-missing-cumulative.test.ts, tests/parser-filter.test.ts all green (65 tests). The full npm test has environment-dependent CLI-integration failures on this machine that reproduce identically on the base commit before my changes (they need built artifacts), so CI is the honest referee for those.

Dismissed: both blockers verified as addressed on 4c3a2b6 — tests/providers/codex.test.ts (incl. the #257 zero-cumulative dedup test at :1201) and tests/parser-filter.test.ts pass; the fix was narrowed per your 'it does repeat' branch (byte-identical consecutive records collapse via info-payload identity, rate_limits excluded as payload sibling); #1088 invariant note restored at dedupKey; docs/providers/codex.md dedup + Quirks updated. Independent spot-check of codeset-ai/codeset-release-evals confirms the re-emission data (cited file: 9/20 byte-identical repeats, 0 monotonicity violations). Full suite green locally; CI never ran on the PR (action_required first-contributor gate).

[ozymandiashh]

Re-reviewed 4c3a2b6 — both blockers addressed exactly as specified.

1. The #257 suite passes again: tests/providers/codex.test.ts 48/48 (incl. zero-cumulative dedup at :1201), tests/codex-missing-cumulative.test.ts 6/6 (both halves: identical ×3 → 1 call, differing ×3 → 3 calls), tests/parser-filter.test.ts 11/11. tsc --noEmit clean. Full npm test: 3744 passed / 0 failed (one lock-timing flake on the first run, passes in isolation on both main and this head).
2. The real-data answer checks out: independently scanned the cited file from codeset-ai/codeset-release-evals — 20 token_count events, 9 byte-identical repeats (45%), 0 monotonicity violations, plus one null-info event (handled by the est path). The byte-identity collapse on info (rate_limits correctly excluded as a payload sibling) is the right narrowing.

Also verified: #1088 invariant note restored at dedupKey with the fork-cutoff trade-off stated; docs dedup + Quirks updated; prevInfoIdentity persisted in resume state; both cache layers invalidated (v16 + parse-version suffix). My previous review is dismissed as addressed.

[ozymandiashh]

Reviewed the full diff and validated it beyond CI: the patch applies cleanly on current main, and applied on a temp branch the codex suites (new missing-cumulative tests plus providers/codex and pricing-1075) pass 61/61 locally. The three-layer dedup is the right shape — byte-identity collapse for re-emissions with real-world measurements behind it (603/1313 events), equal-cumulative guard only when a finite nonnegative total is actually reported, and physical record position as identity when cumulative is absent. The weakened cross-file replay protection without cumulative is a documented, reasoned trade-off. Cache bumps (v16 + missing-cumulative-v1 parse version) follow the house convention so stale caches reprice.

[iamtoruk]

Ran this against my own corpus: 1,399 rollouts, 136,145 token_count events. The shape this fixes (info present, total_token_usage missing) shows up 0 times. All 685 no-cumulative events are info: null, one per session, the opening rate-limit ping, and those already take the estimate path at codex.ts:1108.

Checked your corpus too. Spread-sampled 30 sessions from codeset-release-evals: 1,308 events, 30 null-info, 0 of the target shape. The "54 of 1313" figure is those null-info pings. The re-emission half of your data holds up (624 of 1308 byte-identical repeats), but that half is already collapsed by the cumulative guard.

Built both sides. audit --provider codex --period lifetime is byte-identical: 51,399 calls, $5,226.17. Delta is $0.00.

Tests and tsc are green, rebase onto main is clean. So this is harmless, but the cache bump makes every user cold-reparse their whole Codex history to get the same numbers back. Drop CODEX_CACHE_VERSION back to 15 and drop the -missing-cumulative-v1 suffix.

Also fix the Quirks line in docs/providers/codex.md. It says those 54 events key on record position. They do not, they are null-info and go to the estimate path.

One thing worth noting: the codex:record:: fallback throws away the forkedFromId identity #1383 just added for sub-agent parents. Nothing hits it today, but if the missing-cumulative shape ever appears in a sub-agent rollout it double counts the replayed parent history.

Fix the docs and the version bumps and I will take it.

[roy-tong]

Your corpus wins, and my "54" was a conflation — re-verified before conceding. Fresh spread sample of codeset-release-evals (21 sessions, 404 events): exactly 21 info: null events (one rate-limit ping per session), zero events with info present but total_token_usage absent. Same split you found. My original counter treated info → {} and total → None as one bucket, which is how the pings got mislabeled as partial-info events. That's a bug in my analysis script, now corrected on our side too.

a7fae30 applies all three asks:

1. CODEX_CACHE_VERSION back to 15 and the -missing-cumulative-v1 suffix dropped, with the comment stating why there is no bump (no-op on real data, v16 never shipped).
2. The Quirks line rewritten: null-info pings take the estimate path; the 0-occurrence fact is stated with both corpus sizes.
3. The codex:record: fallback's cost is now documented as you framed it — it drops the forkedFromId identity fix(codex): do not double-count a spawned sub-agent's replayed parent history #1383 adds for sub-agent parents, and nothing hits that path in any examined corpus.

Locally: the three named suites pass (65/65), tsc --noEmit clean. The byte-identical audit result on your side ($0.00 delta) is the correct disposition for a shape with zero real occurrences — thank you for running it against 136k events before saying so.

[iamtoruk]

Merged. Thanks for re-verifying the spread sample and conceding the count instead of arguing it, that is exactly how these should go. The docs now say what the code does and the cache version stays put, so nobody gets a cold reparse for identical numbers.

[roy-tong]

Closing the loop on the merge — thank you both. This thread ended up mattering beyond the patch: the re-review against a 136k-event corpus caught a conflation in my own analysis (null-info rate-limit pings vs partial-info events), which is now corrected in our published rollout-topology reference and analysis script, and recorded as a methodology rule — classify before normalizing, and let a second corpus check any sampled claim. The three-layer dedup as merged (byte-identity collapse with the measured 45-46% re-emission rate, equal-cumulative guard, record-position fallback with the #1383 caveat documented) is the shape we'll cite elsewhere.

_{Repro fixtures: AgentMeasure conformance pack — plain JSONL + expected totals, no runtime install. Found during an open ecosystem audit of usage-accounting tools.}