Review: coder_eval — pr:221 (4 files) axis:1,2,3,4,5,6,7,8

Scope: pr:221 (4 files) axis:1,2,3,4,5,6,7,8 · branch bai/multiturn-usage-double-count (PR #221 → main) · 92ed611 · 2026-10-07T23:04Z · workflow variant

Change class: complex — changes token/cost accounting semantics for resumed claude-code sessions (per-model cumulative-delta subtraction with session-baseline state across turns)

The change is in good condition: type safety, security and harness quality have no findings, and the PR reduces the token over-count on resumed sessions. One real risk remains: in some edge cases, the per-turn cost slice in claude_code_agent.py can still count cost twice or count the full session cost on a resumed Claude session. This changes total_token_usage and the max_usd gate, so the same agent output can stop at a different point. Fix the two cost paths and add tests for them before you treat the accounting as final.

Summary

Overall Score: 9.7 / 10 · Weakest Axis: Code Quality & Style at 9.3 / 10
Totals: 🔴 0 · 🟠 0 · 🟡 3 · 🔵 8 across 8 axes.

Blockers

None.

Non-blocking, but please consider before merge

1. [Axis 1] on_result_message complexity rises from C(13) to C(17); the new _SessionUsage snapshot block repeats the dict/non-empty/numeric-cost guards that _turn_usage_slice already applies (src/coder_eval/agents/claude_code_agent.py:558) — radon on origin/main gives _ClaudeTurnState.on_result_message - C (13) at line 471. At PR HEAD it gives C (17) at line 546. The +4 comes from the new baseline-recording block at lines 557-561: if isinstance(model_usage, dict) and model_usage and isinstance(new_session_id, str): self._agent._session_usage = _SessionUsage(new_session_id, model_usage, cost if isinstance(cost, int | float) else None). This block repeats guards that the new _turn_usage_slice already does: if not isinstance(model_usage, dict) or not model_usage: (line 238) and isinstance(cost, int | float) and isinstance(baseline.cost, int | float) (lines 247-249). The same validation now lives in two places, and the method moved further into the CC 10-20 band. Move the snapshot construction into one helper next to _SessionUsage, for example _SessionUsage.from_result(session_id, model_usage, cost) -> _SessionUsage | None. That helper owns the dict, non-empty and numeric-cost checks, and on_result_message keeps one line (if (snap := _SessionUsage.from_result(...)): self._agent._session_usage = snap). This brings the method back to about its earlier complexity. Note: the new _model_usage_since (line 200) is also exactly B(10), at the lower edge of the same band.
2. [Axis 3] Resumed turn with no model_usage is untested (line 241), and that branch books the session-cumulative total_cost_usd unsliced (src/coder_eval/agents/claude_code_agent.py:240) — Branch coverage over the 8 new TestResumedSessionUsage tests reports line 241 as missing. The full-suite coverage reports it missing too. The untested branch is if not isinstance(model_usage, dict) or not model_usage: / return model_usage, cost. On a resumed turn (baseline.session_id == resumed_from) it returns the raw cost. That value is the session-cumulative total_cost_usd, the same quantity this PR says counts turns 1..N. _build_token_usage then attaches it to the per-turn stream or usage-snapshot fallback (total_cost_usd=sdk_result_cost), so the turn's cost is turns 1..N and the max_usd gate reads it. Two more edges have no test: (a) branch 558->563, where a ResultMessage with no model_usage must NOT move _session_usage; (b) the else None arm at line 249, where baseline.cost or cost is not numeric. Add a _turn_usage_slice(None, 0.17, _SessionUsage("s1", first, 0.14), "s1") test and an {} test that state the intended cost: subtract baseline.cost from cost, or return None. Then fix the code to match. Add one test that sends a no-model_usage ResultMessage between two normal turns and asserts that the baseline did not change.
3. [Axis 6] Crashed or killed attempt on a resumed session is booked twice because the baseline moves only on a ResultMessage (notes say 'absorbed'; no test covers crash then resume) (src/coder_eval/agents/claude_code_agent.py:557) — The session baseline moves only inside on_result_message (line 557: # Every ResultMessage, error or not: the CLI saves its running totals on exit. followed by self._agent._session_usage = _SessionUsage(...)). A mid-turn AgentCrashError with no ResultMessage leaves the baseline and _session_id unchanged, so the retry resumes the same session (self.resumed_from = agent._session_id, line 329). That crash is the generic-Exception or ProcessError path in communicate(), which closes the CLI gracefully: the SDK's SubprocessCLITransport.close() sends stdin EOF, waits, then SIGTERM, so the CLI's exit handler can save its running totals. The crashed partial is booked from stream telemetry. orchestrator._drain_pending_turn appends it to result.iterations, and _aggregate_token_usage sums it (# Include crashed=True partials). The retry's ResultMessage then reports restored totals that include the crashed attempt. _turn_usage_slice subtracts only the pre-crash baseline, so the crashed attempt's tokens and cost are counted a second time in total_token_usage and in the max_usd budget gate. The note at .claude/notes/agents.md:316 admits this ('if its CLI saved totals, the next turn absorbs its usage') but calls it a 'killed turn'. A SIGKILLed (timeout) turn cannot save, so the turn that really absorbs is a crashed turn that closed gracefully. Fix: on the graceful-close crash path (_finalize_and_raise_crash, not the SIGKILL timeout path), add the partial's booked usage to _session_usage so the retry's slice excludes it. Alternatively, start the retry on a fresh session after a crash with no ResultMessage. Add a crash-then-retry test on the same agent instance (Review Criteria #13). This is not statically detectable because it depends on CLI save-on-exit behavior. Severity: the Axis 6 anchor puts 'retry that double-counts tokens/cost' at High. It is set one level lower because it needs a crash with no ResultMessage and a graceful CLI exit, and because the PR still reduces the over-count from before.

Nits

1. [Axis 1] Two new async tests copy the agent-setup and mock_query loop scaffolding (tests/test_token_usage.py:823) — The 4-line agent setup config = parse_agent_config(type=AgentKind.CLAUDE_CODE, permission_mode="acceptEdits", allowed_tools=["Read"]) / agent = ClaudeCodeAgent(config) / agent.working_directory = MagicMock() / agent.working_directory.rglob.return_value = [] now appears at lines 672-675 (already there), 823-826 and 850-853 (new). The per-turn async def mock_query(...): yield ... + with patch("coder_eval.agents.claude_code_agent.query", side_effect=mock_query): records.append(await agent.communicate("next")) loop also appears twice (lines ~829-837 and ~860-866). Add a small helper or fixture, for example _communicating_agent() plus async def _run_turns(agent, results) -> list[TurnRecord]. Both tests then only state their inputs and assertions, and the _LIVE_SESSION fixture stays the only per-test variable.
2. [Axis 1] Re-wrapped _aggregate_model_usage docstring leaves a short broken line mid-sentence (src/coder_eval/agents/claude_code_agent.py:1463) — The PR rewrote the docstring as Maps each model id to its billing (camelCase, unlike ``usage``). The SDK's / authoritative cost breakdown: summed and priced it / reconciles to ``total_cost_usd`` exactly, .... Line 1463 ends at about 59 columns in the middle of a clause, so the sentence reads badly. Re-flow the paragraph to the file's normal width ("...The SDK's authoritative cost breakdown: summed and priced, it reconciles to total_cost_usd exactly..."). This is cosmetic only.
3. [Axis 3] Error-turn test asserts only the turn after the is_error turn, not the is_error turn's own booked slice (tests/test_token_usage.py:869) — test_error_turn_still_moves_the_baseline asserts only records[2].token_usage.output_tokens == 48 and records[2].token_usage.cache_read_input_tokens == 26920. This shows that the baseline moved. It does not assert that the is_error turn (records[1]) booked its own slice (output 361, cache_read 51624, cost ~0.0254338) rather than the cumulative 829/75727. It also does not assert that agent._session_id stayed at "s1" across the error turn. Add both assertions, and add a cost-sum assertion like the one at line 845 (assert sum(t.total_cost_usd for t in turns) == pytest.approx(0.1782134)), so the error path has the same contract check as the clean dialog.
4. [Axis 5] Second per-harness 'cumulative snapshot minus baseline' mechanism added with drifted semantics from Codex's _ThreadTotals.since (src/coder_eval/agents/claude_code_agent.py:200) — The PR adds a second implementation of the session/thread-cumulative slice concept. It does not reuse or mirror the existing Codex one, and the two contracts now differ in three places. (1) Backward detection: Claude's _model_usage_since (line 200) uses a tolerance, if now < before - 1e-9: (line 219), and returns None when any baseline model is missing. Codex's _ThreadTotals.since (codex_agent.py:246) is strict: if self.input < baseline.input or ... : return self. (2) Crash handling: Codex advances its baseline past a crashed turn (self._agent._advance_usage_baseline(token_usage), codex_agent.py:776). Its note (.claude/notes/agents.md:406-408) says the baseline "must still advance past them, or the next turn's delta re-books everything the crashed turn already reported". The Claude baseline moves only in on_result_message (lines 558-561). The new note (agents.md:316-317) accepts the opposite rule: "if its CLI saved totals, the next turn absorbs its usage". The crashed turn's stream tokens are already booked on its partial TurnRecord, so this re-books them. The impact today is small: a watchdog timeout is a SIGKILL, which cannot save totals, and most non-kill failures send an is_error ResultMessage first, which does move the baseline. (3) Shape: Codex keeps a typed NamedTuple with a .since() method. Claude keeps a NamedTuple that holds dict[str, Any], plus two free functions typed Any -> tuple[Any, Any] (_turn_usage_slice, line 226). The PR adds these ~75 lines to a module that is already 1936 lines long. Recommendation: no shared abstraction is needed, because the input shapes are different (fixed int fields vs. a per-model camelCase dict). But make the two contracts match on purpose, or record the divergence. Add one line to .claude/notes/agents.md § Token accounting that names Codex's crash-advance rule and says why Claude differs (the SIGKILL'd CLI does not save totals). Or add a Claude analogue of _advance_usage_baseline that runs when a crashed turn exits through a non-SIGKILL path. Also consider moving _SessionUsage / _model_usage_since / _turn_usage_slice to the same place as the Codex slice logic (for example a small agents/_usage_slice.py), so the next harness that reports cumulative usage copies one contract, not two.
5. [Axis 6] Restore detection checks only for counters that went down, so an unrestored session whose counters all grew is under-counted (unpinned CLI behaviour, debug-only log) (src/coder_eval/agents/claude_code_agent.py:219) — _model_usage_since decides that a session was not restored only when a counter went down (line 219: if now < before - 1e-9: / return None). The fallback at line 244 logs only logger.debug("model_usage of resumed session %s moved backwards; booking it whole", resumed_from). The PR's docstring says the CLI sometimes does not restore. The CLI's restore depends on the per-project lastSessionId in the shared ~/.claude.json, which coder_eval does not isolate (no CLAUDE_CONFIG_DIR). When restore does not happen and this turn's own counters are all at or above the previous turn's, the subtraction runs anyway and silently under-counts this turn's tokens and cost by the previous turn's total. This is likely on a resumed turn, because cache_read grows with the replayed context. Fix: cross-check the slice against the ResultMessage's per-turn usage (the note says usage and num_turns stay per-turn). If the unsliced main-model totals already match the per-turn usage, the snapshot was not restored, so book it whole. Raise the fallback log to warning, so a misdetected restore is visible in task logs.
6. [Axis 7] Hand-written usage key tuple has no static link to the booked keys or the SDK ModelUsage TypedDict (src/coder_eval/agents/claude_code_agent.py:181) — _turn_usage_slice returns a sliced model_usage dict. Its only consumer is _aggregate_model_usage (line 1458), and that function reads its own hard-coded keys: entry.get("inputTokens"...), "outputTokens", "cacheCreationInputTokens", "cacheReadInputTokens", "costUSD" (lines 1476-1480). The slicer subtracts a separate list: _CUMULATIVE_MODEL_USAGE_KEYS = ("inputTokens", "outputTokens", "cacheReadInputTokens", "cacheCreationInputTokens", "thinkingTokens", "webSearchRequests", "costUSD") (lines 181-189). Every key that is not in this tuple passes through with its session-cumulative value. Two of the sliced keys (thinkingTokens, webSearchRequests) are not read anywhere in src/, so slicing them is speculative. The failure case: someone later makes _aggregate_model_usage read a new counter (for example a split cache-write bucket) but does not add it to the tuple. Then a resumed turn books that counter with the whole session's total. No test fails, and the cost is silently inflated. This is the same double-count the PR fixes. Fix: keep one source of truth. Option (a): slice after aggregation. Run _aggregate_model_usage on both the snapshot and the baseline, then subtract the TokenUsage buckets. This removes the tuple and the dict-space slicing. Keep the per-model 'model dropped' check if needed. Option (b): make _aggregate_model_usage iterate over a shared key constant, and drop the keys that nothing reads.
7. [Axis 7] A sibling docstring that the PR did not update still calls the turn's TokenUsage 'cumulative' (src/coder_eval/agents/claude_code_agent.py:680) — The PR changes _aggregate_model_usage ("Sum the turn's model_usage...") and _build_token_usage ("Build the turn's TokenUsage") so they no longer say 'cumulative'. The wrapper that calls them still says """Build the turn's cumulative TokenUsage, repricing for LiteLLM. (line 680). After this PR, the value is explicitly the turn's own slice, not a session-cumulative total, so a reader of this docstring will reach the wrong conclusion. Change it to "Build the turn's TokenUsage, repricing for LiteLLM." Optionally, also change .claude/notes/agents.md:302 ("the SDK's cumulative per-model billing") to match the edited docs/agents/CLAUDE_CODE.md wording.
8. [Axis 7] One baseline has two names, and the parallel Codex baseline uses a third naming scheme (src/coder_eval/agents/claude_code_agent.py:330) — One object is stored as self._session_usage on the agent (line 829). It is copied into the turn state as self.session_usage_baseline = agent._session_usage (line 330), and is written back as self._agent._session_usage = _SessionUsage(...) (line 559). The Codex twin of this concept is self._thread_usage_baseline = _ThreadTotals(), and its subtraction is the _ThreadTotals.since() method. Claude instead uses the free functions _model_usage_since and _turn_usage_slice. Use _session_usage_baseline on the agent so the name says the role. Consider also making the subtraction a method of _SessionUsage (as _ThreadTotals.since is), so the two harnesses' baseline code reads the same way.

What's Missing

Nightly pipeline:

• 🟡 Daily/nightly: the PR lists the published runs that are inflated, but it does not say what happens next on the nightly pipeline. (a) On the merge day, claude-code token and cost totals for multi-turn tasks (total_token_usage, agent_cost_usd, simulator_cost_usd and total_cost_usd in run.json/task.json) drop by about 11% on Sonnet, and cache reads drop about 2.7x on Haiku. Evalboard trends and A/B comparisons across that date (for example claude-code vs codex) are then not valid. (b) Published runs are not backfilled. evaluate <run_dir> re-grades but does not re-cost, so the listed runs stay inflated. (c) A dialog that stopped on max_usd / max_total_tokens / simulation.max_total_tokens because of the inflated totals now runs more turns, so its final status (COST/TOKEN_BUDGET_EXCEEDED) and pass rate can change, not only its cost. Add a note for the external coder-eval-uipath consumers: the date of the step change, and that historical claude-code multi-turn costs are over-stated. (trigger: src/coder_eval/agents/claude_code_agent.py)

Downstream consumers:

• 🔵 Downstream consumers: the budget gates read the per-turn TokenUsage, which the PR changes. These are the orchestrator run_limits.max_usd / max_total_tokens sum (orchestrator.py ~1170/1296) and the dialog budget simulation.max_total_tokens (simulation/termination.py:73). No orchestrator-level or dialog-level test runs a multi-turn claude-code task and checks that the gate sums the sliced per-turn values. The new tests stop at ClaudeCodeAgent.communicate(). Add one test that runs a 3-turn resumed dialog with cumulative ResultMessages and asserts that the gate fires on the true session total, not on the sum of the snapshots. (trigger: src/coder_eval/agents/claude_code_agent.py)

Tests:

• 🔵 Tests: the PR says the simulated-user fix (simulator_cost_usd $0.0107 -> $0.0039) works because UserSimulator runs on a resumed ClaudeCodeAgent. Only one live run checks this. No test in tests/test_user_simulator.py sends cumulative ResultMessages through UserSimulator.next_user_message() and asserts that turn 2's input_tokens / output_tokens (user_simulator.py:340-342) are the sliced values. A later change to how the simulator builds or reuses its agent (for example a fresh agent per call, or a different usage field) would lose the fix silently. (trigger: tests/test_token_usage.py)
• 🔵 Tests: the PR's main validation metric, 'reconciliation row on turns 2-4 = 0' (it was equal to the previous turn's total), has no test. test_dialog_books_each_turn_once asserts token_usage only. It does not assert that TurnRecord.messages on records[1..] has no ReconciliationMessage (or a zero one) when the stream matches the sliced model_usage. Add that assertion so the EventCollector side of the contract (streaming/collector.py _reconciled_messages) is covered too. (trigger: tests/test_token_usage.py)
• 🟡 Tests: the new branches in _turn_usage_slice / on_result_message have no tests. These are: a resumed turn with no or empty model_usage (it returns the cumulative total_cost_usd), a non-numeric baseline cost (turn_cost=None, so the cost comes from the rate-card backfill on sliced tokens), and a ResultMessage with no model_usage that must not move _session_usage. (trigger: src/coder_eval/agents/claude_code_agent.py) (restates: Axis 3: Resumed turn with no model_usage is untested (line 241), and that branch books the session-cumulative total_cost_usd unsliced)
• 🟡 Tests: no test runs a crash (AgentCrashError, no ResultMessage) and then a retry on the same ClaudeCodeAgent instance on iteration 2 or later, with the orchestrator also summing the crashed partial. This is the path where the crashed attempt's usage can be counted twice. (trigger: tests/test_token_usage.py) (restates: Axis 6: Crashed or killed attempt on a resumed session is booked twice because the baseline moves only on a ResultMessage (notes say 'absorbed'; no test covers crash then resume))

Parallel paths:

• 🔵 Parallel paths: the PR removed the word 'cumulative' from _aggregate_model_usage and _build_token_usage and from docs/agents/CLAUDE_CODE.md, but other public model docs still describe model_usage as the run's cumulative billing. These are AssistantMessage's docstring in src/coder_eval/models/telemetry.py:230-233 ('authoritative cumulative billing ... model_usage (cumulative per-model ...)') and the input_tokens / output_tokens Field descriptions (telemetry.py ~288, ~297: 'the run's cumulative input/output is taken from ResultMessage model_usage'). These Field descriptions are user-facing schema text (the Pydantic SSOT for the docs), so after this PR they now point readers the wrong way. Fix them together with _finalize_token_usage (line 680) and .claude/notes/agents.md:302. (trigger: src/coder_eval/agents/claude_code_agent.py) (restates: Axis 7: A sibling docstring that the PR did not update still calls the turn's TokenUsage 'cumulative')
• 🔵 Parallel paths: the PR body says the fallback works 'as the Codex harness does in _ThreadTotals.since'. But Codex also advances its baseline past a crashed turn (codex_agent.py:776, _advance_usage_baseline), and the PR adds no Claude equivalent. Antigravity's 'previous conversation.total_usage' subtraction is a third copy. The cross-harness table in the PR body ('Other harnesses') is not recorded in .claude/notes/agents.md or docs/agents/HARNESS_PARITY.md. The next reviewer then cannot see which harnesses slice session totals and how each one handles a crash. (trigger: src/coder_eval/agents/claude_code_agent.py) _(restates: Axis 5: Second per-harness 'cumulative snapshot minus baseline' mechanism added with drifted semantics from Codex's ThreadTotals.since)

Harness & Lint Improvements

Static checks (lint / type):

• [ci-gate] Pyright tightening (runs in make typecheck). Type the SDK-sourced usage with the SDK's own TypedDict. claude_agent_sdk.ResultMessage.model_usage is already dict[str, ModelUsage] | None (claude_agent_sdk/types.py:1314,1354). The PR drops this to dict[str, Any] in _SessionUsage and to Any -> tuple[Any, Any] in _turn_usage_slice. Set _SessionUsage.model_usage: dict[str, ModelUsage], _model_usage_since(...) -> dict[str, ModelUsage] | None, and _aggregate_model_usage(model_usage: dict[str, ModelUsage] | None). Derive the slice key set from the TypedDict, for example _CUMULATIVE_MODEL_USAGE_KEYS = tuple(k for k, t in get_type_hints(ModelUsage).items() if t in (int, float)), so that one SSOT feeds both the slicer and the reader. Pyright then reports a literal key that _aggregate_model_usage reads but ModelUsage does not declare. A new SDK counter is sliced automatically. Prevents: Finding 'Hand-written usage key tuple has no static link to the booked keys or the SDK ModelUsage TypedDict' (claude_code_agent.py:181). With a typed input, the dict and numeric guards also live at one boundary, which removes part of the guard duplication in finding 'on_result_message complexity rises from C(13) to C(17)' (claude_code_agent.py:558).
• [ci-gate] Radon complexity delta gate in make verify / CI. For each function that the diff changes (compare with the merge-base), compute radon CC on both sides. Fail when CC rises AND the new value is >= 11 (radon band C), unless the function has a visible # noqa: CE-cc debt marker. Radon is already a runtime dependency. Ruff C901 cannot do this: mccabe does not count boolean operators or conditional expressions. I measured it: at PR HEAD, ruff --select C901 with max-complexity=9 does not flag on_result_message, but radon gives C(17). Cap rule note: this is a ratchet on growth, not an absolute cap, so it does not need a backlog clean-up first. Prevents: Finding 'on_result_message complexity rises from C(13) to C(17)' (claude_code_agent.py:558). It also shows the edge case _model_usage_since B(10) for review.
• [ci-gate] Diff-coverage gate. [tool.coverage.run] branch = true is already set, but the only gate is the global 80% floor. Add diff-cover coverage.xml --compare-branch=origin/main --fail-under=90 (branch-aware) to CI, and scope it at minimum to src/coder_eval/agents/ and src/coder_eval/orchestrator.py, where token and cost booking live. The uncovered line 241 and the uncovered branch 558->563 were new lines in this PR. A diff gate fails on them. The global floor cannot. Prevents: Finding 'Resumed turn with no model_usage is untested (line 241)' (claude_code_agent.py:240), including the untested 558->563 branch where a ResultMessage with no model_usage must not move _session_usage.

Harness improvements (not statically reachable):

• Cross-harness cumulative-usage contract suite, shaped like the CE036 ContractCase pattern. Make it one parametrized table over every agent that slices a session or thread cumulative snapshot (Codex _ThreadTotals.since, Claude _turn_usage_slice). Script these turn sequences on ONE agent instance: clean dialog; is_error ResultMessage mid-dialog; crash with no ResultMessage followed by a resumed retry; ResultMessage with total_cost_usd but no modelUsage; unrestored session whose counters all grew. Check one invariant: the sum of per-turn booked tokens and cost, including crashed partials, equals the final cumulative snapshot. No turn may book the cumulative cost unsliced. A CE rule could then require each new cumulative-usage harness to register cases, as CE036 does for live criteria. Why not static: The defects are about state across turns (where the baseline moves on crash, error and missing-usage paths). Only a scripted event stream that runs through communicate() can show them. An AST cannot. Prevents: Findings 'Crashed or killed attempt on a resumed session is booked twice' (claude_code_agent.py:557), 'Resumed turn with no model_usage ... books the session-cumulative total_cost_usd unsliced' (line 240), 'Restore detection checks only for counters that went down' (line 219), and the Codex/Claude crash-rule drift in 'Second per-harness cumulative snapshot minus baseline mechanism' (line 200).
• Delete before guard: extract one agents/_usage_slice.py with the snapshot-minus-baseline contract, including the backward-detection rule and the crash-advance rule. Codex and Claude then adapt their input shapes into it. Add a 'crashed-turn usage baseline' row to docs/agents/HARNESS_PARITY.md, so a divergence is a recorded decision and not an accident. Why not static: The two input shapes are different (fixed int fields vs. a per-model camelCase dict). To decide if the contracts are the same, you must compare semantics, not syntax. Prevents: Findings 'Second per-harness cumulative snapshot minus baseline mechanism added with drifted semantics' (claude_code_agent.py:200) and 'One baseline has two names, and the parallel Codex baseline uses a third naming scheme' (claude_code_agent.py:330).
• Live pin test (marked live, Haiku model) for the CLI behaviour that the slicer assumes. (1) Does a resumed session restore modelUsage? (2) Does a CLI closed gracefully after a crash (stdin EOF / SIGTERM) save its running totals? (3) Does a SIGKILLed CLI save nothing? Also add a run-time reconciliation check. When the per-task sum of booked iteration costs is larger than the last session-cumulative cost that the agent reported (with a tolerance), write a WARNING to the task log. Raise the 'moved backwards; booking it whole' fallback from debug to warning. Why not static: These depend on the external Claude Code CLI version and on its save-on-exit behaviour. Code cannot show them; only a live process can. Prevents: Findings 'Crashed or killed attempt on a resumed session is booked twice' (premise not verified: the CLI saves on graceful exit) and 'Restore detection checks only for counters that went down ... debug-only log' (claude_code_agent.py:219).
• Shared multi-turn test helpers for ClaudeCodeAgent: _communicating_agent() (config + working_directory mock) and async _run_turns(agent, results) -> list[TurnRecord]. Add assert_turns_reconcile(records, final_cumulative), which checks every turn's own slice, the stable _session_id, and the cost sum. Every multi-turn token-usage test calls it, so an error-path test cannot assert only the next turn. Why not static: Test duplication and weak assertions are test-design judgment. No reliable AST pattern separates acceptable repeated setup from harmful duplication. Prevents: Findings 'Two new async tests copy the agent-setup and mock_query loop scaffolding' (tests/test_token_usage.py:823) and 'Error-turn test asserts only the turn after the is_error turn' (tests/test_token_usage.py:869).
• Review-checklist step, not a rule: when a PR changes the meaning of a term (here 'cumulative' became 'the turn's own slice'), grep that term in the touched module's docstrings and in .claude/notes/. Fix every stale use in the same change. The broken docstring re-wrap is cosmetic and is left to review. Why not static: A grep for 'cumulative' gives many correct hits (session-cumulative is still a real concept in this module). Only a reader can tell a stale use from a correct one. Line-length heuristics for re-wrapped docstrings give too many false positives to justify a rule. Prevents: Findings 'A sibling docstring that the PR did not update still calls the turn's TokenUsage cumulative' (claude_code_agent.py:680) and 'Re-wrapped _aggregate_model_usage docstring leaves a short broken line' (claude_code_agent.py:1463).

Top 5 Priority Actions

1. The baseline moves only on a ResultMessage (src/coder_eval/agents/claude_code_agent.py:557-561). If an attempt crashes and the CLI closes gracefully without a ResultMessage, its partial usage is booked, then counted again on the retry, which inflates total cost and the max_usd gate. Fix: in _finalize_and_raise_crash (not the SIGKILL path), add the booked partial usage to _session_usage, or start the retry on a fresh session. Add a test that crashes and then retries on the same agent instance.
2. On a resumed turn with no modelUsage, _turn_usage_slice returns the session-cumulative total_cost_usd without slicing it (src/coder_eval/agents/claude_code_agent.py:240-241), so max_usd reads the cost of turns 1..N for one turn. Subtract baseline.cost or return None, and add tests for None/{} model_usage and for 'no model_usage does not move the baseline' (branch 558->563).
3. _model_usage_since detects an unrestored session only when a counter goes down (src/coder_eval/agents/claude_code_agent.py:219). If all counters grew, it subtracts the previous turn's total and books too little. Cross-check the slice against the per-turn usage in the ResultMessage, and raise the fallback log at line 244 from debug to warning.
4. Make one source of truth for the sliced keys: _CUMULATIVE_MODEL_USAGE_KEYS (src/coder_eval/agents/claude_code_agent.py:181) and the keys that _aggregate_model_usage reads (lines 1476-1480) can drift apart, and a key that drifts is silently booked with its full session total. Slice after aggregation, or iterate one shared constant. In the same change, record in .claude/notes/agents.md how this crash/baseline contract differs from Codex's _ThreadTotals.since and _advance_usage_baseline.
5. Move the snapshot guards into a _SessionUsage.from_result(...) helper to bring on_result_message back from C(17) to about C(13) (src/coder_eval/agents/claude_code_agent.py:558). Also strengthen test_error_turn_still_moves_the_baseline (tests/test_token_usage.py:869) to check the is_error turn's own slice, the session id and the cost sum. Fix the stale 'cumulative' docstring at line 680.

Stats: 0 🔴 · 0 🟠 · 3 🟡 · 8 🔵 across 8 axes reviewed.