Repository navigation
fix(codex): exclude compaction estimates from model call count - #252
cosminacho wants to merge 2 commits into
Conversation
|
Claude finished @cosminacho's task in 1m 41s —— View job PR Review in Progress
|
uipreliga
left a comment
There was a problem hiding this comment.
Review: fix(codex): exclude compaction estimates from model call count (#252)
PR #252 by @cosminacho · fix/codex-compaction-counter → main · OPEN · reviewed against 664a716 · 2026-11-07T12:27Z
Change class: complex — changes the Codex adapter's model-call counting control flow, which gates max_turns exhaustion
PR #252 correctly stops Codex from counting the compaction context estimate as a second model call, and it makes max_turns parity better; the remaining risk is that the extra zero-delta check at codex_agent.py:891-904 compares the nullable cache_write_input_tokens with == 0, which can bring back the over-count, while the tests do not pin the reset for a second compaction, the "outside compaction still counts" contract, or the usage update from the estimate, so the change is safe to merge after one simplification and three test fixes.
Summary
| Axis | Score | 🔴 | 🟠 | 🟡 | 🔵 | Top Issue |
|---|---|---|---|---|---|---|
| 1. Code Quality & Style | 8.9 / 10 | 0 | 0 | 2 | 1 | Redundant zero-delta guard on top of compaction state flags raises on_token_usage_updated CC from A(4) to C(11) |
| 2. Type Safety | 9.9 / 10 | 0 | 0 | 0 | 1 | The new compaction-estimate predicate reads an Any-typed usage breakdown through hand-typed getattr string keys that default to 0, so pyright cannot check the field names and a missing or renamed field counts as a zero delta |
| 3. Test Health | 9.3 / 10 | 0 | 0 | 1 | 2 | The documented contract that the compaction estimate still updates usage and transcript capture has no test |
| 4. Security | 10 / 10 | 0 | 0 | 0 | 0 | — |
| 5. Architecture & Design | 9.7 / 10 | 0 | 0 | 0 | 3 | Codex compaction-as-a-max_turns-call divergence is recorded only in .claude/notes; user-facing parity/max_turns docs are stale |
| 6. Error Handling & Resilience | 10 / 10 | 0 | 0 | 0 | 0 | — |
| 7. API Surface & Maintainability | 10 / 10 | 0 | 0 | 0 | 0 | — |
| 8. Evaluation Harness Quality | 9.9 / 10 | 0 | 0 | 0 | 1 | Rationale note cites an upstream codex tag (rust-v0.163.0) that differs from the pinned CLI version |
Overall Score: 9.7 / 10 · Weakest Axis: Code Quality & Style at 8.9 / 10
Totals: 🔴 0 · 🟠 0 · 🟡 3 · 🔵 8 across 8 axes.
Blockers
None.
Non-blocking, but please consider before merge
- [Axis 1] Redundant zero-delta guard on top of compaction state flags raises on_token_usage_updated CC from A(4) to C(11) (
src/coder_eval/agents/codex_agent.py:891) — The goal is to skip one known bookkeeping event: the second tokenUsage update inside a contextCompaction item. The PR uses two boolean flags (self.in_context_compaction,self.compaction_usage_seen, lines 560-561) to track a three-state lifecycle (outside / in compaction before usage / in compaction after usage). It also adds a second, independent check over five hard-coded attribute names:estimate = ( self.in_context_compaction and self.compaction_usage_seen and last is not None and all( getattr(last, key, 0) == 0 for key in ( "input_tokens", "output_tokens", "cached_input_tokens", "cache_write_input_tokens", "reasoning_output_tokens", ) ) )(lines 891-904). Only the positional state check is needed: the zero-delta check alone would wrongly skip a zero-usage update outside compaction, or a zero-usage first compaction response (verifier correction). The result is that radon CC for this method goes from A (4) on origin/main to C (11). Recommendation: keep one check. Option 1: replace the two booleans with onecompaction_usage_updates: int | None(None outside compaction, incremented per update), and skip counting when it is >= 1. Option 2: if the zero-delta check must stay, extract it to a named module-level helper (_is_zero_delta(last)) so the handler reads as oneif. Both options keep the four new tests green and bring the method back to an A/B grade. - [Axis 1] Compaction zero-delta check treats SDK None token fields as non-zero (no
or 0normalization, no None fixture) (src/coder_eval/agents/codex_agent.py:896) — The new predicategetattr(last, key, 0) == 0(line 896) runs over a key list that includes "cache_write_input_tokens". The installed SDK (openai_codex 0.156.1, generated/v2_all.py:6160) types that field as nullable:cache_write_input_tokens: Annotated[int | None, Field(alias="cacheWriteInputTokens")] = 0. If the app-server sendscacheWriteInputTokens: null, thenNone == 0is False,estimateis False, and the compaction estimate is counted as a model call again. That is the max_turns over-count this PR fixes, now back without any error (cross-axis: 8, since num_turns / max_turns_exhausted can change for identical agent output). The same file already handles this correctly withcached = (getattr(last, "cached_input_tokens", 0) or 0) if last else 0(line 590) andreasoning_tok = (getattr(last, "reasoning_output_tokens", 0) or 0) if last else 0(line 596). Use the same idiom:not (getattr(last, key, 0) or 0). Add a test case wherecache_write_input_tokens=Noneon the estimate update. - [Axis 3] The documented contract that the compaction estimate still updates usage and transcript capture has no test (
tests/test_codex_agent.py:2248) — The PR notes say the zero-delta estimate 'still updates usage and transcript capture', and the handler does this at codex_agent.py:915-917 (if notification.payload:/self.latest_token_usage = usage/self._flush_message(last)). No test pins it. I changed line 915 toif notification.payload and not estimate:in the PR worktree and all 155 tests in tests/test_codex_agent.py still passed. Two things hide the gap. The fixture gives the estimate the same cumulative total as the compaction response (recomputed = _token_usage(inp=10, out=5, cached=0)at line 2248, same asresponse = _token_usage(inp=10, out=5, cached=0)). Also, every compaction test ends with a later ordinary tokenUsage that overwriteslatest_token_usage. Fix: give the estimate a differenttotal(for example total input 20, which matches thetotal_tokens=20estimate). Add a case where the estimate is the last usage beforeturn/completed(or the cap stops the pump), and assertrecord.token_usageequals the estimate's total. Also assert that the token-bucket reconciliation holds (sum of buckets overrecord.messages==record.token_usage) for a turn with compaction.
Nits
- [Axis 1] Compaction test fixtures repeat the zero-usage SimpleNamespace and splice at positional indices (
tests/test_codex_agent.py:2238) — The blockX.payload.token_usage.last = SimpleNamespace(input_tokens=..., output_tokens=..., cached_input_tokens=0, cache_write_input_tokens=0, reasoning_output_tokens=0, total_tokens=...)is written out by hand at lines 2238, 2249, 2287 and 2304. Three of the four are all-zero copies that differ only in total_tokens. Add alast=keyword to the existing_token_usage(inp, out, cached)helper (line 544), or add a small_last(inp=0, out=0, total=0)helper. Also,notifications[5:5] = ...(lines 2264, 2276) andnotifications[10:10] = ...(line 2275) depend on_cmd_notificationsemitting exactly 5 notifications per call. Name that stride (e.g._NOTIFS_PER_CMD = 5, thennotifications[k * _NOTIFS_PER_CMD:k * _NOTIFS_PER_CMD]) so a change to the helper does not silently move the compaction insertion point. - [Axis 2] The new compaction-estimate predicate reads an
Any-typed usage breakdown through hand-typedgetattrstring keys that default to 0, so pyright cannot check the field names and a missing or renamed field counts as a zero delta (src/coder_eval/agents/codex_agent.py:896) —on_token_usage_updated(self, notification: Any)getslast = getattr(usage, "last", None)(line 890). It then decides whether a usage update is an estimate withgetattr(last, key, 0) == 0over a literal tuple:"input_tokens", "output_tokens", "cached_input_tokens", "cache_write_input_tokens", "reasoning_output_tokens"(lines 896-903).lastisAny, so pyright reports 0 errors whether or not these names are correct. Because the getattr default is 0, a key that is misspelled or later renamed in the SDK reads as a zero delta. This fails open: a real model call inside a compaction (after the first usage update) is then classified as an estimate and left out ofapi_calls, which weakens themax_turnscap. It does not tighten the cap.
The code also departs from this file's own handling of nullable fields. _flush_message coalesces each field with (getattr(last, "cached_input_tokens", 0) or 0) (line 590). In the SDK, TokenUsageBreakdown.cache_write_input_tokens is int | None (openai_codex 0.156.1 v2_all.py:6160), and None == 0 is False. The JSON schema of the 0.156.1 app-server sets that field to a non-nullable integer with default 0, so the binary cannot send a null today. This is a latent inconsistency, not a live bug.
The new fixtures in tests/test_codex_agent.py also build last as an untyped SimpleNamespace(...) (lines 2238, 2249, 2287, 2306), which repeat the same key names. No test binds the predicate to the real SDK schema, and no test sends a non-zero second usage update inside a compaction, so a misspelled key would pass the whole suite.
Fix: type last as TokenUsageBreakdown, with the import under if TYPE_CHECKING: (the codex extra is optional, and the file already imports TurnCompletedNotification lazily at line 922). Read the fields directly, for example last.input_tokens == 0 and ... and (last.cache_write_input_tokens or 0) == 0, so that pyright checks the names. In the tests, build last with the real TokenUsageBreakdown(...); the test module already imports from openai_codex.generated.v2_all at line 505.
3. [Axis 3] Nothing tests the reset of compaction_usage_seen on a second compaction item (src/coder_eval/agents/codex_agent.py:762) — self.compaction_usage_seen = False (line 762, in on_item_started for contextCompaction) is the only thing that makes the FIRST usage of a SECOND compaction in the same turn count as a model call. When I deleted that line, all 155 tests passed. test_repeated_compaction_resets_before_normal_tool_calls (line 2273) has the right name but calls self._compaction_notifications(item_id=...) with the default empty_response=False. The first response of each compaction is therefore non-zero and never needs the reset. Failure case: two compactions in one turn where the second compaction's model response reports zero deltas. Without the reset, that response is treated as an estimate, api_calls is too low, and the max_turns cap fires one call late. The note promises 'The first compaction usage update counts even when its reported deltas are zero', but a test pins this only for the first compaction in a turn. Fix: add empty_response=True to the second _compaction_notifications(item_id="compact-2") call (or parametrize it) and assert num_turns.
4. [Axis 3] test_zero_usage_outside_compaction_still_counts asserts nothing because num_turns is floored at 1 (tests/test_codex_agent.py:2299) — The test sends one zero-delta usage update and then asserts record.num_turns == 1 with max_turns=1 (lines 2297-2299). Finalization reports num_turns=max(self.api_calls, 1) (codex_agent.py:991). So if the update is NOT counted, api_calls is 0, num_turns is still 1, and max_turns=1 cannot trip. I applied a mutation that skips EVERY zero-delta update (I removed the self.in_context_compaction and self.compaction_usage_seen guards). This test still PASSED, and only the two post-compaction tests failed. The contract named in the test is therefore not pinned by this test. Fix: put a normal counted call before the zero-delta update and assert num_turns == 2, or assert on the turn state's api_calls directly.
5. [Axis 5] Codex compaction-as-a-max_turns-call divergence is recorded only in .claude/notes; user-facing parity/max_turns docs are stale (.claude/notes/agents.md:89) — The new note says "The first compaction usage update counts even when its reported deltas are zero". So on Codex an auto-compaction uses one unit of run_limits.max_turns. The claude-code adapter counts distinct main-thread assistant message_ids (claude_code_agent.py:406-493). Claude Code compaction does not stream an assistant message, so it does not count there. CLAUDE.md's 'Harness run-limit parity' rule says such a divergence must be documented in docs/agents/HARNESS_PARITY.md. That doc still says (§ max_turns counts model API calls) "Codex sends thread/tokenUsage/updated once per call". This PR shows that statement is false for compaction, which sends two updates. The PR makes parity better (the old code counted 2 calls, the new code counts 1), so this is Low. Fix: add one sentence to the Codex bullet in HARNESS_PARITY.md, which is the SSOT the notes defer to. It must say that compaction sends a second, zero-delta estimate update that is not counted, and that the compaction call itself counts toward max_turns on Codex, unlike claude-code. Also, the note cites openai/codex tag rust-v0.163.0, but pyproject.toml pins openai-codex-cli-bin==0.156.1. Cite the pinned version, or say that the contract is the same in 0.156.1.
6. [Axis 5] Compaction call-boundary state adds two more interacting flags to a ~500-line turn-state class instead of one cohesive call counter (src/coder_eval/agents/codex_agent.py:560) — Before this PR, three fields detected the max_turns call boundary: api_calls, in_api_call and call_ran_tools. Three handlers mutate them: on_item_started (763), on_token_usage_updated (908-913) and _open_api_call (709-712). The PR adds self.in_context_compaction = False and self.compaction_usage_seen = False (560-561). Their transitions are spread across on_item_started (760-762), on_item_completed (798-799) and on_token_usage_updated (891-907). The correctness of max_turns_reached() (707) now depends on a 5-flag implicit state machine split between transcript-reconstruction handlers in _CodexTurnState. That class also owns message flushing, tool telemetry and sub-agent tracking. This does not hurt today. Consider extracting a small _CallCounter with on_item_started(root_type), on_item_completed(root_type) and on_usage(last) methods. The boundary rule (including the compaction lifecycle) would then live in one unit that can be tested without building notifications.
7. [Axis 5] Bare "contextCompaction" literal repeated instead of a named item-type constant (src/coder_eval/agents/codex_agent.py:760) — if root_type == "contextCompaction": appears at line 760 (on_item_started) and line 798 (on_item_completed). The same literal already appears in _NON_MODEL_ITEM_TYPES (95) and _CONTENT_ITEM_TYPES (106). Elsewhere the module names the item types it branches on, for example _SUB_AGENT_ACTIVITY_ITEM = "subAgentActivity" (136). Add _CONTEXT_COMPACTION_ITEM = "contextCompaction", use it in both frozensets and both new branches, and a typo in one site can no longer silently disable the compaction lifecycle.
8. [Axis 8] Rationale note cites an upstream codex tag (rust-v0.163.0) that differs from the pinned CLI version (.claude/notes/agents.md:92) — Line 92 says "The source contract is visible in openai/codex tag rust-v0.163.0:", but pyproject.toml:222 pins override-dependencies = ["openai-codex-cli-bin==0.156.1"]. I checked rust-v0.156.1 and the event order is the same there: compact.rs l.258 started, l.839 update_token_usage_info, l.404 recompute_token_usage, l.406 completed. Cite the pinned tag so that an audit or a later bump compares against the binary that actually ships. Also, line 86 ("Codex emits usage for the compaction model response, then emits another usage update") is true only for local compaction. Remote compaction (compact_remote_v2.rs at the same tag) never calls update_token_usage_info: its Completed arm at l.462-477 only records the usage. It sends only the recompute estimate (l.374). In that case the single zero-delta update is the one that counts as the call, which the 'first counts' rule handles correctly. Add one sentence for the remote case, so that nobody later 'simplifies' the rule to 'always skip zero-delta updates during compaction', which would undercount remote compaction.
What's Missing
Tests:
- 🟡 Tests: no test checks that the max_turns cap still fires on a compaction. All four new compaction tests assert
max_turns_exhausted is False. No case sets the cap so that the compaction response is call N+1 (for example,_cmd_notifications(1)with compaction spliced in andmax_turns=1), then assertsmax_turns_exhausted is Trueand that the pump stops before the estimate update and the compactionitem/completed. The cap fires while the compaction is still open, and no test covers that path. Undercounting is the opposite failure from the one this PR fixes, and no test pins it. (trigger: tests/test_codex_agent.py)
Parallel paths:
- 🔵 Parallel paths: docs/agents/CODEX.md:252 (the run-limits table row) still says max_turns is "counted per
thread/tokenUsage/updated". This PR shows that statement is false: a compaction's second, zero-delta tokenUsage update is not counted. The finding cites only the HARNESS_PARITY.md bullet, so this second user-facing surface also needs the same one-sentence fix. (trigger: src/coder_eval/agents/codex_agent.py) (restates: Axis 5: Codex compaction-as-a-max_turns-call divergence is recorded only in .claude/notes; user-facing parity/max_turns docs are stale) - 🔵 Parallel paths: the PR decides that on Codex the compaction model call counts as one unit of run_limits.max_turns, and the bookkeeping estimate does not count. It does not check how the other call-counting adapters treat a compaction step: OpenCode counts
step_start(opencode_agent.py:1269), Pi countsturn_start, and Antigravity counts a rise in cumulative usage. Under the Harness run-limit parity rule, compaction must mean the same thing on every backend, or the difference must be written in HARNESS_PARITY.md. Today only the Codex behaviour is defined, and only in .claude/notes. (trigger: .claude/notes/agents.md) (restates: Axis 5: Codex compaction-as-a-max_turns-call divergence is recorded only in .claude/notes; user-facing parity/max_turns docs are stale)
Nightly pipeline:
- 🔵 Daily/nightly: on every Codex run whose turn auto-compacts, this change lowers
num_turnsby one per compaction, and it changesmax_turns_exhaustedand the MAX_TURNS_EXHAUSTED end status. These values go into run.json and task.json (run_record.py), the markdown and html reports, judge_context.py, and the evalboard turn views (evalboard/lib/turns.ts, task-grid.tsx). The consumers need no code change, but the PR description does not say that Codex turn counts and cap-exhaustion rates in nightly runs and in A/B baselines recorded before 664a716 are not directly comparable with runs after it. (trigger: src/coder_eval/agents/codex_agent.py)
Harness & Lint Improvements
Static checks (lint / type):
- [ce-lint] CE070 (next free number: CE069 is in use in an open worktree, CE067 is retired). Rule: in src/coder_eval/agents/*.py, a
getattr(<obj>, <key>, <int literal>)must be the left operand ofor <int>. Any other use (== 0, arithmetic, a comparison) is a violation, because an SDK field typedint | Nonereturns None, not the default. Add tests/lint/rules/ce070_int_getattr_must_coalesce.py, wire it in tests/lint/runner.py, and add CE070 toexternalin pyproject.toml. I ran a prototype over agents/ at PR HEAD. It reports exactly 2 hits: codex_agent.py:896 (the PR line) and codex_agent.py:932 (sequence_number, a legitimate# noqa: CE070). The existing sites (codex_agent.py:590-596 and 2658-2660) already conform, so no other code must change. Better fix (delete before guard): add one_usage_int(obj, field) -> inthelper and route every token read through it. Prevents: A1/A3/A6/A7 medium finding at codex_agent.py:896.getattr(last, key, 0) == 0treats a nullcache_write_input_tokensas non-zero, so the compaction estimate counts as a model call again. The key is a variable, so a literal-key rule would miss it. This rule checks the parent node, not the key. - [ce-lint] CE071: no getattr probe for a Codex
TokenUsageBreakdownfield. Use the CE050 mechanism: get the field names at collection time fromopenai_codex.generated.v2_all.TokenUsageBreakdown.model_fields, skip the rule if the codex extra is absent, and flaggetattr(x, "<field>", ...)and a literal tuple of those names that is iterated into getattr, in src/coder_eval/agents/codex_agent.py. The only exception is one typed reader. That reader typeslast: TokenUsageBreakdownunderif TYPE_CHECKING:and reads attributes directly, so pyright checks the names. Add tests/lint/rules/ce071_no_sdk_usage_getattr_probe.py and wire it in runner.py. Prevents: A2 finding at codex_agent.py:896-903. The five hard-coded key strings on anAnyobject are not checked by pyright, and a misspelled or renamed key reads as 0. That makes the estimate fail open and weakens max_turns. The rule also removes the duplicate key list behind the A1 medium complexity finding. - [ce-lint] CE072: in src/coder_eval/agents/*.py, an
==/!=/incomparison against a str literal that is also an element of a module-level frozenset/set/tuple constant in the same module is forbidden. Use a named module constant, the same pattern as_SUB_AGENT_ACTIVITY_ITEM. Add tests/lint/rules/ce072_item_type_literal_named.py. In codex_agent.py it reports the PR lines 760 and 798 ("contextCompaction", which is in_NON_MODEL_ITEM_TYPESand_CONTENT_ITEM_TYPES). It also reports the existing lines 853, 864, 1904 and 1909 ("reasoning","agentMessage"). Migrate those 4 sites in the same change. Prevents: A1/A5 low finding at codex_agent.py:760. The bare "contextCompaction" literal is repeated, so a typo at one site silently disables the compaction lifecycle while the frozensets stay correct. - [ce-lint] CE073 (a doc-surface rule, so a
@pytest.mark.lintclass in tests/test_custom_lint.py): everyrust-vX.Y.Ztag cited in .claude/notes//*.md and docs//*.md must equal theopenai-codex-cli-bin==X.Y.Zpin in pyproject.tomloverride-dependencies. Read the pin from pyproject; do not hard-code it. Today the rule fails on .claude/notes/agents.md:92 (rust-v0.163.0against pin 0.156.1) and passes on .claude/notes/context-window.md:55-56 (rust-v0.156.1). Prevents: A1/A7/A8 low finding at .claude/notes/agents.md:92. The rationale note audits the event order against an upstream tag that the shipped binary is not built from. It also makes a future pin bump re-check every cited source contract. - [ci-gate] Add a radon cyclomatic-complexity ratchet to
make lint/make verify. radon is already a runtime dependency. Store a per-function baseline (tests/lint/cc_baseline.json, fromradon cc -j src/coder_eval/agents/). Fail when a function's radon grade rises (for example A->C) or when a new function enters at C or worse, unless the baseline is updated in the same change and the PR body gives the reason. Ruff C901 cannot do this job. mccabe does not count boolean operators, so it scoreson_token_usage_updatedat 6 at PR HEAD, and it already reports 79 offenders at threshold 10. Radon scores the same method A(4)->C(11). Prevents: A1 medium finding at codex_agent.py:891. The redundant zero-deltaall(getattr(...)==0 ...)guard on top of the two state flags raised CC from A(4) to C(11). The ratchet also slows the growth of the A5 state machine: it would have flagged on_item_started at C(12) and kept on_item_completed at D(25) from growing. - [ci-gate] Add a changed-files coupling gate in CI. If a PR diff changes a line that mutates
api_calls,in_api_callormax_turns_reachedin src/coder_eval/agents/*_agent.py, it must also touch docs/agents/HARNESS_PARITY.md, or the PR body must carry aParity-Unchanged: <reason>trailer. The presence check is mechanical. Whether the doc text is correct needs a human, so the gate only forces the question. Prevents: A5/A7/A8 low finding at .claude/notes/agents.md:89. The Codex compaction divergence (compaction counts as one max_turns call on Codex but not on claude-code) is recorded only in .claude/notes. HARNESS_PARITY.md still says Codex sendsthread/tokenUsage/updatedonce per call.
Harness improvements (not statically reachable):
- Add a golden-stream scenario
codex_i_context_compactionto tests/_fixtures/golden_streams/codex_fixtures.py. It must contain a contextCompaction item with the response usage update, then the zero-delta recompute estimate with a different cumulativetotal, and the estimate must be the last usage before turn/completed. Add a second compaction with a zero-delta first response. test_codex_golden and test_codex_reconciliation_invariant then pin num_turns, the final token_usage and the per-bucket reconciliation automatically. Why not static: The contract (the estimate still updates usage and transcript; the second compaction resets compaction_usage_seen) is a property of a sequence of runtime events. Only a replayed event stream, diffed against a snapshot, can observe it. Prevents: A3 medium finding (the 'estimate still updates usage and transcript' contract is untested; the mutationif notification.payload and not estimate:passed all 155 tests) and A3 low finding at codex_agent.py:762 (deleting the compaction_usage_seen reset passed all tests). - Add a scoped mutation-testing target,
make mutate-run-limits: mutmut or cosmic-ray, restricted to the call-boundary methods of every agent turn state (_CodexTurnState.on_token_usage_updated,on_item_started,on_item_completed,_open_api_call, and the delegate equivalent). Run it nightly or on PRs that touch those methods. Fail on a surviving mutant in the counter logic. Why not static: Whether a test actually pins a branch is a property of test execution. Reviewers found this here only by mutating code by hand (removing guards and deleting line 762). No AST check can decide whether an assertion is vacuous. Prevents: A3 low findings: test_zero_usage_outside_compaction_still_counts asserts nothing becausenum_turns=max(self.api_calls, 1)floors at 1; the compaction_usage_seen reset is untested. A3 medium finding: the estimate-updates-usage branch is unpinned. - Make the raw call count observable in tests. Add a turn-state test helper that asserts
state.api_callsdirectly, or a test-only hook. Document in the agent test guide that a counter test must assert a value of 2 or more, or the raw counter, becausenum_turnsis floored at 1 for codex (codex_agent.py:991) and delegate (delegate_agent.py:1168). Why not static: A lint cannot tell from the assertion text thatnum_turns == 1undermax_turns=1is vacuous. That needs knowledge of the floor in the finalize path and of how the scenario counts. Prevents: A3 low finding at tests/test_codex_agent.py:2299 (a vacuousnum_turns == 1assertion). - Replace the hand-built
SimpleNamespace(input_tokens=..., ...)usage fixtures in tests/test_codex_agent.py (32 sites) with one typed builder that returns the realTokenUsageBreakdown/ThreadTokenUsage(the module already imports fromopenai_codex.generated.v2_all). Give itlast=and aNone-field option. Also make_cmd_notificationsreturn per-command chunks, so compaction is inserted by command index and not bynotifications[5:5]. Only after that migration, consider a test-scope lint that forbids SimpleNamespace carrying TokenUsageBreakdown field names. Why not static: This can become a lint, but it is not one yet. A rule today would hit 32 existing sites, so the shared builder must come first (delete before guard). Whether a positional splice index is coupled to a helper's output length is a semantic property of the helper, not a pattern an AST check can find. Prevents: A1/A3 low finding at tests/test_codex_agent.py:2238/2264 (duplicated zero-usage fixtures, splice at a positional index). The A2 test-side gap (misspelled keys pass because fixtures are untyped). The missing None fixture for cache_write_input_tokens in the A1 medium finding. - Extract the max_turns call-boundary rule into a small
_CallCounterunit (on_item_started(root_type), on_item_completed(root_type), on_usage(last)) with one state field in place of the in_context_compaction/compaction_usage_seen boolean pair. Pin it with a table-driven test over event sequences, including remote compaction (estimate only, which must count) and local compaction (response, then estimate, where the estimate must not count). Why not static: Whether 5 interacting flags form a correct state machine is a semantic design judgment. Only the complexity side can be gated (by the radon ratchet). The cases where the rule is correct (remote and local compaction per upstream compact.rs/compact_remote_v2.rs) need behavioural tests. Prevents: A5 low finding at codex_agent.py:560 (the 5-flag implicit state machine spread over transcript handlers). A1 medium finding (redundant dual guard). The A8 risk that someone 'simplifies' the rule to 'always skip zero-delta updates' and undercounts remote compaction.
Top 5 Priority Actions
- Remove the five-key zero-delta predicate at src/coder_eval/agents/codex_agent.py:891-904 and keep only the positional check (in compaction AND first usage already seen), ideally as one
compaction_usage_updates: int | Nonecounter in place of the two flags at lines 560-561; this removes theNone == 0path at line 896 (SDK typescache_write_input_tokensasint | None) that would count the estimate again and change num_turns / max_turns_exhausted for identical agent output, and it brings on_token_usage_updated back from CC C(11) to A/B. - In tests/test_codex_agent.py:2273 (test_repeated_compaction_resets_before_normal_tool_calls), call the second
_compaction_notifications(item_id="compact-2")withempty_response=Trueand assert num_turns, because deleting the reset at codex_agent.py:762 makes no test fail, and a missing reset undercounts api_calls so the max_turns cap fires one call late. - Fix test_zero_usage_outside_compaction_still_counts at tests/test_codex_agent.py:2297-2299 so that it can fail: put a normal counted call before the zero-delta update and assert
num_turns == 2(or assert api_calls directly), becausenum_turns = max(api_calls, 1)at codex_agent.py:991 hides an uncounted update. - At tests/test_codex_agent.py:2248, give the compaction estimate a cumulative total that is different from the response, add a case where the estimate is the last usage before turn/completed, and assert record.token_usage and the per-message token-bucket reconciliation, because changing codex_agent.py:915 to skip estimates still passes all 155 tests.
- Update the Codex bullet in docs/agents/HARNESS_PARITY.md (it still says one thread/tokenUsage/updated per call) to say that compaction counts as one max_turns call on Codex but not on claude-code, and that the zero-delta estimate is not counted; in .claude/notes/agents.md:92 cite the pinned
rust-v0.156.1tag in place ofrust-v0.163.0and add one sentence about remote compaction, where the single zero-delta update is the one that counts.
Stats: 0 🔴 · 0 🟠 · 3 🟡 · 8 🔵 across 8 axes reviewed.

Codex emits a second usage update when it estimates a compacted context. Counting that bookkeeping update as another model call could exhaust
max_turnsbefore the next real call. This excludes only repeated zero-delta updates inside compaction, including nullable SDK token buckets, while preserving normal calls and reported usage.The tests cover repeated compactions, zero-usage real calls, a final context estimate, and transcript token reconciliation. The parity guide documents that the compaction model call itself still counts for Codex. Historical Codex call-count and cap-exhaustion metrics can differ by one per compaction across this fix.
Validation:
68cd17f.make verify: 6,830 passed, 4 skipped, 93.39% coverage. Its two Docker mount source-assertion failures reproduce unchanged on main; the full command therefore exits nonzero locally.fizzbuzz.pywas absent. The job log does not establish the underlying cause. All reported checks on updated head949db4fpassed, including the Quality Gate, Harbor round trip, Codex Live, and Delegate Live E2E.No paid local agent run or change to an existing benchmark snapshot is included.