mirror of
https://github.com/AgentSeal/codeburn.git
synced 2026-10-21 05:23:19 +00:00
fix(codex): preserve distinct requests when cumulative usage is missing (#1264)
* fix(codex): keep token_count events in sessions that never write total_token_usage * fix(codex): preserve distinct requests when cumulative usage is missing * fix(codex): narrow missing-cumulative fix - byte-identical re-emissions still collapse (#257) Real-data answer to the review's question: Codex does re-emit byte-identical token_count events. Scanned 53 public rollout sessions (codeset-ai/ codeset-release-evals, 1313 token_count events): 603 (46%) are byte-identical repeats of their predecessor (identical total AND last usage), 0 cumulative monotonicity violations, 54 events carry no total_token_usage at all. The missing-cumulative path now collapses a record whose info payload is byte-identical to the previous event's (#257 semantics restored); only records with differing payloads count as distinct requests. prevInfoIdentity is persisted in the resume state. The #1088 invariant note is restored at the dedupKey in adapted form; docs/providers/codex.md documents the three-layer dedup and the accepted fork-cutoff trade-off. * fix(codex): revert cache/parse version bumps, correct null-info docs per review Re-verified on a fresh spread sample of codeset-release-evals (21 sessions, 404 events): exactly one info:null rate-limit ping per session, zero events with info present but total_token_usage absent — matching the reviewer's 137k-event corpus. The missing-cumulative branch is a no-op on real data: - CODEX_CACHE_VERSION stays 15 (no cold reparse for identical numbers; v16 never shipped in a release) - drop the -missing-cumulative-v1 parse suffix - Quirks: null-info pings take the estimate path, not the dedup path; the earlier 54-of-1313 figure conflated them with partial-info events - dedup layer 3 now documents the forkedFromId/#1383 sub-agent caveat --------- Co-authored-by: Roy tong <tongxiarui@elliwooddeMacBook-Air.local>
This commit is contained in:
parent
3a1dac20a1
commit
47b7441dd6
4 changed files with 108 additions and 14 deletions
|
|
@ -40,7 +40,13 @@ A session that yielded zero parseable lines does **not** write to the cache (`co
|
|||
|
||||
## Deduplication
|
||||
|
||||
`codex:<sessionId>:<timestamp>:<cumulativeTotal>` for accounted events, plus `codex:<sessionId>:<timestamp>:est<n>` for estimated events that fall back to char-counting.
|
||||
Three layers, in order:
|
||||
|
||||
1. **Byte-identity collapse (#257)**: a `token_count` event whose `info` payload is byte-identical to the previous event's is a re-emission of the same event, not a new request, and is skipped regardless of cumulative presence. Measured on public rollouts (53 sessions / 1313 events): 603 are such repeats.
|
||||
2. **Equal-cumulative guard**: with `total_token_usage.total_tokens` present, an event whose cumulative total equals the predecessor's is skipped.
|
||||
3. **`seenKeys` cross-session key**: with cumulative identity — `codex:<forkedFromId|sessionId>:<total>:<input>:<cached>:<output>:<reasoning>` (fork replays collide with the parent). Without cumulative — `codex:record:<path>:<line offset>`, i.e. physical record position: stable on cache resume/re-read, but it drops the `forkedFromId` identity #1383 adds for sub-agent parents, so if the missing-cumulative shape ever appeared in a sub-agent rollout the replayed parent history would double-count. Nothing hits this path in any examined corpus.
|
||||
|
||||
Estimated events that fall back to char-counting use `codex:<sessionId>:<timestamp>:est<n>`.
|
||||
|
||||
## Quirks
|
||||
|
||||
|
|
@ -48,7 +54,8 @@ A session that yielded zero parseable lines does **not** write to the cache (`co
|
|||
1. `last_token_usage` present: use it directly.
|
||||
2. Only cumulative: compute deltas against the prior turn.
|
||||
3. Neither: estimate from message text length (`CHARS_PER_TOKEN = 4`).
|
||||
- `prevCumulativeTotal` is initialized to `null`, not `0`. A session whose first event reports `total = 0` would otherwise be dropped as a "duplicate" of the initial state.
|
||||
- Sessions open with one `token_count` event carrying `info: null` (the rate-limit ping) — one per session in every corpus examined, including a 136k-event private one; those take the char-count estimate path, not the dedup path. Events with `info` present but `total_token_usage` absent have 0 observed occurrences (spread-sampled codeset-release-evals: 30 null-info, 0 partial); if that shape ever appears, the parser treats non-identical payloads as distinct requests and keys on record position.
|
||||
- `prevCumulativeTotal` is initialized to `null`, not `0`. A session whose first event reports `total = 0` would otherwise be dropped as a "duplicate" of the initial state. `prevInfoIdentity` (the byte-identity string) is persisted in the resume state alongside it.
|
||||
- `prev*` token counters are advanced on **every** event, including ones that used `last_token_usage`. Earlier code only updated them on the fallback branch, which double-counted any session that mixed modes.
|
||||
- OpenAI counts cached tokens **inside** `input_tokens`. The parser subtracts them so the rest of the codebase can assume Anthropic semantics (cached are separate).
|
||||
|
||||
|
|
|
|||
|
|
@ -43,6 +43,10 @@ import type { ParsedProviderCall } from './providers/types.js'
|
|||
// v15: builtin alias prices `codex-auto-review` (#1047). Exact-hit cache
|
||||
// entries still hold the pre-alias $0; bump so unchanged rollouts reprice.
|
||||
// Must be max(main v14 #1092, this)+1 — #1092 spent v14 on MCP/skills.
|
||||
// No bump for #1264: the missing-cumulative branch is a no-op on real data
|
||||
// (0 occurrences of info-without-total across 137k+ events; null-info pings
|
||||
// already take the estimate path), so cached numbers are identical and a
|
||||
// bump would only force a cold reparse. v16 was never shipped in a release.
|
||||
export const CODEX_CACHE_VERSION = 15
|
||||
export const CODEX_LEGACY_CACHE_FILE = 'codex-results.json'
|
||||
export function codexCacheFileName(version = CODEX_CACHE_VERSION): string {
|
||||
|
|
|
|||
|
|
@ -632,6 +632,9 @@ type CodexResumeState = {
|
|||
forkedFromId: string
|
||||
forkCutoff: string
|
||||
prevCumulativeTotal: number | null
|
||||
/// Byte-identity of the last token_count info payload (#257 re-emission
|
||||
/// collapse). Optional so resume states written before it still decode.
|
||||
prevInfoIdentity?: string | null
|
||||
prevInput: number
|
||||
prevCached: number
|
||||
prevCacheWrite: number
|
||||
|
|
@ -668,6 +671,7 @@ function isResumeState(value: unknown): value is CodexResumeState {
|
|||
&& typeof v['forkedFromId'] === 'string'
|
||||
&& typeof v['forkCutoff'] === 'string'
|
||||
&& (v['prevCumulativeTotal'] === null || typeof v['prevCumulativeTotal'] === 'number')
|
||||
&& (v['prevInfoIdentity'] === undefined || v['prevInfoIdentity'] === null || typeof v['prevInfoIdentity'] === 'string')
|
||||
&& typeof v['prevInput'] === 'number'
|
||||
&& typeof v['prevCached'] === 'number'
|
||||
&& typeof v['prevCacheWrite'] === 'number'
|
||||
|
|
@ -728,6 +732,7 @@ function createParser(source: SessionSource, seenKeys: Set<string>, capture?: {
|
|||
// dropped. Once we've observed any event, we record its cumulative
|
||||
// total and dedup on equality regardless of whether it is zero.
|
||||
let prevCumulativeTotal: number | null = resume?.state.prevCumulativeTotal ?? null
|
||||
let prevInfoIdentity: string | null = resume?.state.prevInfoIdentity ?? null
|
||||
let prevInput = resume?.state.prevInput ?? 0
|
||||
let prevCached = resume?.state.prevCached ?? 0
|
||||
let prevCacheWrite = resume?.state.prevCacheWrite ?? 0
|
||||
|
|
@ -910,6 +915,7 @@ function createParser(source: SessionSource, seenKeys: Set<string>, capture?: {
|
|||
forkedFromId,
|
||||
forkCutoff,
|
||||
prevCumulativeTotal,
|
||||
prevInfoIdentity,
|
||||
prevInput,
|
||||
prevCached,
|
||||
prevCacheWrite,
|
||||
|
|
@ -1155,13 +1161,23 @@ function createParser(source: SessionSource, seenKeys: Set<string>, capture?: {
|
|||
}
|
||||
|
||||
const cumulativeTotal = info.total_token_usage?.total_tokens ?? 0
|
||||
// Dedup guard. Two consecutive events with cumulativeTotal=0 but
|
||||
// non-empty last_token_usage would have been double-counted with
|
||||
// the previous `> 0` clause. The null sentinel ensures the FIRST
|
||||
// event always passes (so a session that never reports cumulative
|
||||
// doesn't lose its opening turn).
|
||||
if (prevCumulativeTotal !== null && cumulativeTotal === prevCumulativeTotal) continue
|
||||
prevCumulativeTotal = cumulativeTotal
|
||||
// Missing/null/partial cumulative data is not a repeated zero total.
|
||||
const reportsCumulative = typeof info.total_token_usage?.total_tokens === 'number'
|
||||
&& Number.isFinite(info.total_token_usage.total_tokens)
|
||||
&& info.total_token_usage.total_tokens >= 0
|
||||
// #257 regression half: a byte-identical consecutive record is a
|
||||
// re-emission of the same token_count event, never a new request.
|
||||
// This collapse applies with or without cumulative totals: measured
|
||||
// on public Codex rollouts (codeset-ai/codeset-release-evals,
|
||||
// 53 sessions / 1313 token_count events), 603 events are
|
||||
// byte-identical repeats of their predecessor, all carrying the
|
||||
// same cumulative snapshot. What the missing-cumulative path must
|
||||
// preserve is records whose payload DIFFERS (distinct requests).
|
||||
const infoIdentity = JSON.stringify(info)
|
||||
if (infoIdentity === prevInfoIdentity) continue
|
||||
prevInfoIdentity = infoIdentity
|
||||
if (reportsCumulative && prevCumulativeTotal !== null && cumulativeTotal === prevCumulativeTotal) continue
|
||||
prevCumulativeTotal = reportsCumulative ? cumulativeTotal : null
|
||||
|
||||
const last = info.last_token_usage
|
||||
let inputTokens = 0
|
||||
|
|
@ -1240,12 +1256,21 @@ function createParser(source: SessionSource, seenKeys: Set<string>, capture?: {
|
|||
// are computed against a running `prev` that the fork advances
|
||||
// differently once the 5s cutoff skips some replays, so a delta-based
|
||||
// key would spuriously diverge on a replay and double-count it.
|
||||
const dedupKey = `codex:${forkedFromId || sessionId}:${cumulativeTotal}:${total?.input_tokens ?? 0}:${total?.cached_input_tokens ?? 0}:${total?.output_tokens ?? 0}:${total?.reasoning_output_tokens ?? 0}`
|
||||
// Without cumulative identity, equal usage can be distinct requests.
|
||||
// Use the physical record position: stable on cache resume/re-read,
|
||||
// but deliberately do not guess cross-file replay identity.
|
||||
// Invariant (#1088, restored): anything dropped below is a record
|
||||
// that survived both guards -- with cumulative identity it is a
|
||||
// strictly-advanced total, so no tokens are lost and no active-time
|
||||
// rescaling is needed; without it the record differs in payload
|
||||
// from its predecessor and is treated as a distinct request, which
|
||||
// deliberately weakens forkedFromId replay protection past the 5s
|
||||
// fork cutoff (accepted trade-off: the alternative collapsed
|
||||
// distinct requests wholesale).
|
||||
const dedupKey = reportsCumulative
|
||||
? `codex:${forkedFromId || sessionId}:${cumulativeTotal}:${total?.input_tokens ?? 0}:${total?.cached_input_tokens ?? 0}:${total?.output_tokens ?? 0}:${total?.reasoning_output_tokens ?? 0}`
|
||||
: `codex:record:${JSON.stringify([source.path, tracker.lastCompleteLineOffset])}`
|
||||
|
||||
// A drop here can only be a byte-identical replay: the
|
||||
// prevCumulativeTotal guard above already discards a repeated
|
||||
// running total, so nothing reaching this point ever loses real
|
||||
// tokens -- no active-time rescaling needed (#1088 investigation).
|
||||
if (seenKeys.has(dedupKey)) continue
|
||||
seenKeys.add(dedupKey)
|
||||
|
||||
|
|
|
|||
58
tests/codex-missing-cumulative.test.ts
Normal file
58
tests/codex-missing-cumulative.test.ts
Normal file
|
|
@ -0,0 +1,58 @@
|
|||
import { it, expect, vi } from 'vitest'
|
||||
import { mkdtemp, writeFile, rm } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { createCodexProvider } from '../src/providers/codex.js'
|
||||
|
||||
vi.mock('../src/codex-cache.js', async (original) => ({
|
||||
...await original<typeof import('../src/codex-cache.js')>(),
|
||||
readCachedCodexResults: async () => null,
|
||||
readCodexResume: async () => null,
|
||||
writeCachedCodexResults: async () => {},
|
||||
}))
|
||||
|
||||
// Real-data basis (codeset-ai/codeset-release-evals, 53 sessions / 1313
|
||||
// token_count events): 603 events are byte-identical repeats of their
|
||||
// predecessor -- Codex re-emits token_count snapshots unchanged within a
|
||||
// response -- while 54 events carry no total_token_usage at all. Both
|
||||
// behaviors must be handled: identical repeats collapse (#257), distinct
|
||||
// payloads without cumulative stay distinct.
|
||||
for (const total of [undefined, null, {}]) {
|
||||
it(`collapses byte-identical re-emitted records with cumulative ${JSON.stringify(total)}`, async () => {
|
||||
const dir = await mkdtemp(join(tmpdir(), 'codeburn-missing-total-'))
|
||||
try {
|
||||
const path = join(dir, 'rollout-reemit.jsonl')
|
||||
const identical = { last_token_usage: { input_tokens: 100, output_tokens: 20 }, total_token_usage: total }
|
||||
const lines = [
|
||||
{ type: 'session_meta', timestamp: '2026-09-01T10:00:00Z', payload: { session_id: 's', model: 'gpt-5.3-codex' } },
|
||||
...[1, 2, 3].map(n => ({ type: 'event_msg', timestamp: `2026-09-01T10:00:0${n}Z`, payload: { type: 'token_count', info: identical } })),
|
||||
]
|
||||
await writeFile(path, lines.map(line => JSON.stringify(line)).join('\n') + '\n')
|
||||
const parser = createCodexProvider(dir).createSessionParser({ path, project: 'test', provider: 'codex' }, new Set())
|
||||
const calls = []
|
||||
for await (const call of parser.parse()) calls.push(call)
|
||||
expect(calls).toHaveLength(1)
|
||||
expect(calls[0].outputTokens).toBe(20)
|
||||
} finally { await rm(dir, { recursive: true, force: true }) }
|
||||
})
|
||||
|
||||
it(`preserves distinct records with differing payloads when cumulative is ${JSON.stringify(total)}`, async () => {
|
||||
const dir = await mkdtemp(join(tmpdir(), 'codeburn-missing-total-'))
|
||||
try {
|
||||
const path = join(dir, 'rollout-distinct.jsonl')
|
||||
const lines = [
|
||||
{ type: 'session_meta', timestamp: '2026-09-01T10:00:00Z', payload: { session_id: 's', model: 'gpt-5.3-codex' } },
|
||||
...[0, 1, 2].map(n => ({ type: 'event_msg', timestamp: `2026-09-01T10:00:0${n + 1}Z`, payload: {
|
||||
type: 'token_count', info: { last_token_usage: { input_tokens: 100 + n, output_tokens: 20 + n }, total_token_usage: total },
|
||||
} })),
|
||||
]
|
||||
await writeFile(path, lines.map(line => JSON.stringify(line)).join('\n') + '\n')
|
||||
const parser = createCodexProvider(dir).createSessionParser({ path, project: 'test', provider: 'codex' }, new Set())
|
||||
const calls = []
|
||||
for await (const call of parser.parse()) calls.push(call)
|
||||
expect(calls).toHaveLength(3)
|
||||
expect(calls.reduce((sum, call) => sum + call.outputTokens, 0)).toBe(63)
|
||||
expect(new Set(calls.map(call => call.deduplicationKey)).size).toBe(3)
|
||||
} finally { await rm(dir, { recursive: true, force: true }) }
|
||||
})
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue