Skip to content

feat(litellm): actual per-call cost + cache accounting for the open-weight backend - #66

Open
CarlesUIPath wants to merge 10 commits into
mainfrom
feat/openweight-actual-cost
Open

feat(litellm): actual per-call cost + cache accounting for the open-weight backend#66
CarlesUIPath wants to merge 10 commits into
mainfrom
feat/openweight-actual-cost

Conversation

@CarlesUIPath

@CarlesUIPath CarlesUIPath commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

The Claude binary's Anthropic transport drops OpenRouter's real usage.cost and
per-call cache-read tokens before the harness can see them, so open-weight
(LiteLLM) runs were priced at static Claude list rates and showed 0 cache reads.
This PR captures the real numbers proxy-side and joins them back onto each
run's turns, so open-weight runs are billed and displayed at their true per-call
cost with real prompt-cache reads visible per generation.

Verified end-to-end

Verified on three real open-weight sweeps, each the 5-task skill-flow suite
(calculator / switch / transform-map / terminate / dice-roller-simulated). In
every case, for every turn: proxy-calls == generations == distributed
messages
, the turn cost equals the sum of its proxy calls to the cent, and
the reconciliation residual is $0.

  • Kimi K3 (runs/2026-07-29_17-22-05) — 5/5 tasks clean.
  • GLM 5.2 (runs/2026-07-30_10-26-30) — 5/5 SUCCESS; run ≈ $1.91 (e.g.
    switch: 72 calls → $0.9152); real cache reads captured (768 cache-read tokens
    on a 21,477-input call).
  • DeepSeek V4 Pro (runs/2026-07-30_11-19-06) — run ≈ $1.04. Exercises the
    accounting under stress: it stayed exact on 2 FAILURE tasks and 1
    MAX_TURNS_EXHAUSTED task (cost attribution is orthogonal to task outcome),
    and on a genuine two-iteration turn (dice-roller it=1: 8 calls → $0.0284;
    it=2: 56 calls → $0.2928) each iteration's calls were credited to its own turn
    — the per-iteration partition, not the empty-trailing-turn fallback.

Ground truth: the per-run totals above were cross-checked against the
corresponding charges on OpenRouter's billing/activity dashboard and reconcile —
confirming the captured usage.cost is the amount actually billed, not an
estimate. This is the whole point of capturing the real per-call cost rather than
re-pricing tokens at a static rate card.

Evalboard cost and cache breakdown.

image

The three seams

  1. Tagclaude_code_agent stamps ANTHROPIC_CUSTOM_HEADERS with
    x-ce-run-id / x-ce-task-id / x-ce-iteration (LiteLLM route only).
  2. Capturelitellm/cost_logger.py (a LiteLLM success callback) reads each
    call's real usage.cost + cache buckets (OpenRouter's usage.cost, not
    LiteLLM's response_cost, which is 0.0 for models it doesn't price) and
    appends one JSONL record per call to $LITELLM_COST_LOG.
  3. Joinsrc/coder_eval/litellm_cost.py overrides each turn's cost with the
    summed actual per-call cost, credits a retried iteration's calls to its
    surviving generation-bearing turn, and distributes per-call cache/cost onto
    the message timeline via an order-respecting output-token walk — always
    reconciling the residual so Σ(token buckets) == token_usage holds.

Changes

  • New: litellm/cost_logger.py (proxy callback, self-contained — no
    coder_eval import so it runs in the proxy's isolated env),
    src/coder_eval/litellm_cost.py (the join).
  • Models: ProviderCallCost; TurnRecord.provider_call_costs; cost_usd on
    AssistantMessage / ReconciliationMessage (join-populated).
  • Orchestrator: correlation tags on _create_agent;
    _join_litellm_actual_cost() runs before token aggregation.
  • Config: Settings.litellm_cost_log (LITELLM_COST_LOG); litellm-config.yaml
    gains usage.include: true per model + callbacks: cost_logger.proxy_handler_instance;
    start-litellm.sh exports LITELLM_COST_LOG + PYTHONPATH.
  • Evalboard: the message timeline prefers the actual per-message cost_usd
    over the static estimate; the 3 OpenRouter models are dropped from the static
    price table so they no longer mis-price against list rates.

Side fix: pin each model to a vetted provider set (avoids a hard 429)

litellm/litellm-config.yaml previously had no provider.only list, so
sort: price + allow_fallbacks: false pinned the single global-cheapest
OpenRouter upstream — when that provider was at capacity it returned HTTP 429 with
nowhere to go, failing even the first call (observed: Baidu for deepseek-v4-pro).
The fix restricts each model to a hard provider.only allowlist of upstreams
we've observed to be reliable
(tried cheapest-first via sort: price), and keeps
allow_fallbacks: false so routing can't silently substitute a provider outside
that vetted set. A bounded known-good set is deliberately preferred over open
fallback: the harness can't see which upstream served a given call (there is no
per-call provider field), so an unannounced fallback would produce confusing
latency/behavior instrumentation. Cost is exact regardless of which listed
provider serves the call — cost_logger.py books the real usage.cost. The three
sweeps above ran on these provider sets. Committed separately as fix(litellm): ….

Backend safety

Gated to LiteLLMRoute + LITELLM_COST_LOG. cost_usd / provider_call_costs
default to None / [], so Claude, Bedrock, and Codex fall back to the static
rate card unchanged
. The join is never fatal — a missing, empty, or mismatched
log leaves each turn's static estimate in place (whole-turn fallback).

Test coverage

  • tests/test_litellm_cost.py — record loading; cost override; retry
    no-double-count; the multi-turn survivor rule; message distribution
    (output-token match, content-block split, aux call → reconcile, order-divergence
    bail); the orchestrator join hook.
  • tests/test_litellm_cost_logger.py — both OpenAI- and Anthropic-shaped usage
    extraction; tag extraction; append; the emit path; debug capture.
  • tests/test_litellm_route.py, tests/test_event_collector.py — route wiring +
    field-parity.
  • Evalboard: messageActualCost.test.ts (actual-over-static preference), plus
    updated parseMessages / pricing-parity / pricing suites.
  • Gates: Python 3587 pass; evalboard tsc + 347 tests; ruff + custom lint
    (166 rules) + pyright green (bar the optional openai_codex import and
    env-only live tests).

Known limitations (by design)

  • No deterministic join key between a proxy call and a transcript generation:
    the callback only sees OpenRouter's gen-… id; the Anthropic msg_… transcript
    id is generated after the callback fires. So per-generation attribution
    matches on output tokens via an order-respecting walk, and bails to a
    whole-turn total on ambiguity
    (the one sub-agent run where the two orderings
    diverge) rather than present a guessed per-generation split. The run total is
    always correct regardless.
  • Cost is proxy-authoritative; tokens stay SDK-authoritative.
    Reconciliation-row residual tokens carry $0 (they're a counting artifact
    already paid for in the per-call costs — re-pricing them at the rate card would
    overstate vs. the bill).
  • A degenerate call that returns no usage (cost/tokens null — observed once on
    GLM) shows a blank price on that one timeline row; it contributes $0, so the
    total stays exact.
  • Requires a fresh run + operator wiring: old runs were never distributed (the
    join runs at run time), and coder_eval's .env LITELLM_COST_LOG must point at
    the same path the proxy writes, or the join is skipped and static pricing is
    used.
  • 429s under concurrency aren't fully eliminated. The side fix restricts each
    model to a curated only set of reliable providers (with allow_fallbacks: false),
    which removes the original single-contended-provider failure — but a provider in
    the set can still be at capacity under load, and routing is bounded to that set by
    design. The sweeps here ran at low parallelism; a harder stress test at higher
    concurrency (--max-parallel 5–20) hasn't been run yet and is the way to shake out
    remaining 429 behavior and tune the provider sets.

Out of scope

  • Docker support for containerized runs. TBA in a separate PR

CarlesUIPath and others added 2 commits July 30, 2026 11:46
…eight backend

The Claude binary's Anthropic transport drops OpenRouter's real usage.cost and
per-call cache-read tokens before the harness can see them, so open-weight
(LiteLLM) runs were priced at static Claude list rates and showed 0 cache reads.
Capture the real numbers proxy-side and join them back onto each run's turns.

- litellm/cost_logger.py: a LiteLLM success callback writes one JSONL record per
  call (real usage.cost + cache buckets, read from OpenRouter's usage.cost, not
  LiteLLM's response_cost which is 0.0 for models it doesn't price) to
  $LITELLM_COST_LOG, correlated by the x-ce-run-id / x-ce-task-id /
  x-ce-iteration headers the agent now stamps via ANTHROPIC_CUSTOM_HEADERS.
- src/coder_eval/litellm_cost.py: join the records back onto the run — override
  each turn's cost with the summed actual per-call cost, credit a retried
  iteration's calls to its surviving generation-bearing turn, and distribute
  per-call cache/cost onto the message timeline via an order-respecting
  output-token walk, always reconciling the residual so the four-bucket token
  invariant holds.
- Gated to LiteLLMRoute + LITELLM_COST_LOG; other backends fall back to the
  static rate card unchanged. Never fatal — a missing/mismatched log keeps the
  static estimate (whole-turn fallback).
- evalboard: prefer the actual per-message cost_usd over the static estimate in
  the timeline; drop the 3 OpenRouter models from the static price table so they
  no longer mis-price against list rates.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… silent fallback)

With no provider.only list, `sort: price` + `allow_fallbacks: false` pinned the
single global-cheapest OpenRouter upstream — so when that provider was at capacity
it returned HTTP 429 with nowhere to go, failing even the first request (observed:
Baidu for deepseek-v4-pro).

Fix by restricting each model to a HARD `provider.only` allowlist of upstreams
we've observed to be reliable, tried cheapest-first (`sort: price`), and keep
`allow_fallbacks: false` so routing can't silently substitute a provider outside
that vetted set. A bounded known-good set is preferable to open fallback here
because the harness can't see which upstream served a given call (there is no
per-call provider field), so an unannounced fallback would produce confusing
latency/behavior instrumentation. Cost stays exact either way — cost_logger.py
books each call's real usage.cost. The three 5-task open-weight sweeps validated
in this PR ran on these provider sets.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown

🔍 Reviewing PR: feat(litellm): actual per-call cost + cache accounting for the open-weight backend

Working on review...

Todo List:

  • Read code review guidelines from .github/code_review.md
  • Read project conventions from CLAUDE.md
  • Get full diff with git diff origin/main...HEAD
  • Review all 20 changed files with full context
  • Check implementation correctness and design
  • Verify test coverage
  • Check for cross-file consistency
  • Analyze what's missing or could be improved
  • Post comprehensive review feedback

View job run

Comment thread litellm/cost_logger.py Fixed
Comment thread tests/test_litellm_cost.py Fixed
CarlesUIPath and others added 2 commits July 30, 2026 13:47
…branches

Fill the coverage gaps a two-part audit surfaced (core logic was already well
covered; these close the tail):

- tests/test_litellm_config.py (new): shape-guard litellm-config.yaml — each
  OpenRouter model has usage.include + a vetted provider pin (sort: price,
  allow_fallbacks: false, non-empty only-list), the cost callback is registered,
  and `cost_logger.proxy_handler_instance` actually exists (so the config string
  can't drift from the symbol). A YAML typo otherwise only surfaces at proxy startup.
- Orchestrator join ordering: a 2-turn case that runs _join_litellm_actual_cost
  then _aggregate_token_usage and asserts the run total re-derives from the actual
  per-call costs (Σ actual, not Σ static) — guards against reordering the two.
- Two defensive branches in _distribute_onto_messages: a cost-less run leaves the
  reconcile row's cost None (not 0.0), and a turn with no reconciliation row still
  gets its real cost. litellm_cost.py branch coverage now 99%.

Also addresses PR review:
- import AgentKind from coder_eval.models (not the .enums submodule) per the
  repo's "core models from coder_eval.models" convention.
- document cost_logger._to_dict's best-effort broad-except (a logging callback
  must never break the proxy) and cover its raising-dumper fall-through to {}.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@uipreliga uipreliga left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Scope: pr:66 (21 files) axis:1,2,3,4,5,6,7,8 · branch feat/openweight-actual-cost · 327e6b3 · 2026-07-30T14:00Z · workflow variant

Change class: complex — introduces a new proxy-side cost-capture callback plus a join/redistribution algorithm that overrides each turn's authoritative cost and rewrites per-message token/cost buckets, touching scoring-adjacent accounting and the reconciliation invariant

The codebase stays healthy where it has long been strong — near-flawless type safety, security, and architectural discipline (8.3/10 overall) — but this PR's LiteLLM actual-cost path concentrates the real risk: an agent-agnostic factory now forwards a Claude-only cost_log_tags kwarg that hard-crashes every non-Claude and no-op agent under the LiteLLM backend, and the cost join can double-count, under-report, or clamp away authoritative numbers in ways that change a task's recorded cost and final_status for byte-identical agent output, so the feature is architecturally sound but should not ship until the factory crash and the cost-correctness defects are fixed and pinned by tests.

Summary

Axis Score 🔴 🟠 🟡 🔵 Top Issue
1. Code Quality & Style 7.2 / 10 0 1 3 3 LiteLLM-only cost_log_tags kwarg is passed through the agent-agnostic create_agent factory, so every non-Claude agent raises TypeError under the LiteLLM backend (and the branch is untested)
2. Type Safety 9.4 / 10 0 0 1 1 turn: Any on _distribute_onto_messages disables type checking of every TurnRecord/TokenUsage field it reads and writes
3. Test Health 8 / 10 0 0 4 0 litellm/cost_logger.py: the two hooks LiteLLM actually calls (log_success_event / async_log_success_event, lines 216-220) and _emit's never-break-the-proxy except guard (212-214) are uncovered, and the file sits outside the --cov=coder_eval gate
4. Security 9.9 / 10 0 0 0 1 Unsanitized author-defined task/variant ids are interpolated verbatim into the newline-delimited ANTHROPIC_CUSTOM_HEADERS string (header injection; bypasses the repo's hash_identifier convention)
5. Architecture & Design 9 / 10 0 1 0 0 Actual-cost join is a second writer of the message-stream token-bucket invariant and silently clamps it away with max(0, …), with no test pinning the clamp
6. Error Handling & Resilience 7 / 10 0 1 4 0 Partial cost coverage silently understates the turn bill: calls with no reported cost are billed at $0 and still override the static estimate
7. API Surface & Maintainability 7.5 / 10 0 1 3 0 LITELLM_COST_LOG is missing from the .env copy-paste block, .env.example, and the docker env allowlist/mounts, so the actual-cost join silently no-ops (notably under --driver docker)
8. Evaluation Harness Quality 8.5 / 10 0 1 1 0 Cost-log join key has no per-attempt component, so a reused --run-dir double-counts a prior attempt's proxy cost rows

Overall Score: 8.3 / 10 · Weakest Axis: Error Handling & Resilience at 7 / 10
Totals: 🔴 0 · 🟠 5 · 🟡 16 · 🔵 5 across 8 axes.

Blockers

  1. [Axis 1] LiteLLM-only cost_log_tags kwarg is passed through the agent-agnostic create_agent factory, so every non-Claude agent raises TypeError under the LiteLLM backend (and the branch is untested) (src/coder_eval/orchestrator.py:1252) — _create_agent does if isinstance(self.route, LiteLLMRoute): kwargs["cost_log_tags"] = {...} (orchestrator.py:1252-1256) and then return create_agent(self.task.agent.type, self.task.agent, route=self.route, **kwargs) (line 1257). The route is settings-derived (resolve_route) and independent of agent type, but only ClaudeCodeAgent.__init__ grew the parameter — NoOpAgent, CodexAgent (codex_agent.py:642) and AntigravityAgent (antigravity_agent.py:192) have no such kwarg and no **kwargs, and create_agent forwards it verbatim (registry.py:162: cast(Any, registration.agent_class)(config, route=route, **kwargs)). Reproduced against PR HEAD: TypeError: NoOpAgent.__init__() got an unexpected keyword argument 'cost_log_tags' — i.e. every agentless task (which Settings.validate_api_keys explicitly allows credential-free, config.py:231) and every codex/antigravity task fails to construct under API_BACKEND=litellm. Gate on agent capability rather than route: add a supports_cost_log_tags ClassVar on Agent (mirroring the existing supports_cooperative_stop at claude_code_agent.py:657) and only pass the kwarg when it is set, or accept **_: Any on the base constructors. Add a regression test building a none-agent task under a LiteLLMRoute.
  2. [Axis 5] Actual-cost join is a second writer of the message-stream token-bucket invariant and silently clamps it away with max(0, …), with no test pinning the clamp (src/coder_eval/litellm_cost.py:225) — CLAUDE.md declares the reconciliation invariant as booked at exactly one seam: "summing the four token buckets across TurnRecord.messages (assistant + reconciliation) equals token_usage exactly", "booked at the single EventCollector seam". _distribute_onto_messages now rewrites both the assistant buckets (lines 207-215) and the reconciliation row from a different source (the proxy log), and absorbs any disagreement with a clamp:
reconciliation.cache_read_tokens = max(
    0, usage.cache_read_input_tokens - sum(m.cache_read_tokens for m in assistants)
)

(litellm_cost.py:225-227)

The two sources disagree by construction on precisely the field this PR exists to add: the SDK-side turn usage arrives with cache_read_input_tokens == 0 on the LiteLLM route (the PR's own premise — "open-weight runs were priced at static Claude list rates with 0 cache reads"), while the proxy record carries the real cache_read. Reproduced against PR HEAD with TokenUsage(uncached_input_tokens=5000, cache_read_input_tokens=0, output_tokens=20) and one call record {input: 5000, cache_read: 4096, output: 20}:

assistant: input=904  cache_read=4096
reconcile: input=4096 cache_read=0
sum cache_read = 4096 vs token_usage.cache_read = 0   <-- invariant broken
sum input      = 5000 (the same 4096 cached tokens are also counted as reconcile input)

Every unit test in tests/test_litellm_cost.py::TestDistributeOntoMessages feeds a token_usage that already agrees with the proxy (uncached=1904, cache_read=4096), so the real-world shape is untested. The consumer this PR wires up assumes the invariant holds — selectTokenTotals (evalboard/lib/runs.ts:1988-1999) returns the raw stream sum whenever a reconciliation entry is present ("the message stream sums EXACTLY to the authoritative total") — so open-weight runs with cache hits will display inflated input/cache token totals.

Fix: decide on one owner for the buckets. Either reconcile against the proxy totals (recompute turn.token_usage from the summed calls when distribution succeeds, as is already done for cost), or leave the token buckets untouched and attach only cost_usd per generation (which the module docstring at lines 14-17 already claims it does: "token buckets are left untouched (SDK-authoritative)"). Whichever is chosen, assert the invariant in the join instead of silently max(0, …)-clamping it, and add a test with cache_read_input_tokens=0 on the turn.
3. [Axis 6] Partial cost coverage silently understates the turn bill: calls with no reported cost are billed at $0 and still override the static estimate (src/coder_eval/litellm_cost.py:132) — costs = [c.cost_usd for c in calls if c.cost_usd is not None] / if costs: / turn.token_usage.total_cost_usd = total (lines 132-138) overrides the authoritative per-turn cost with the sum of only the priced calls, treating every call whose cost was null as $0 — there is no completeness check. Verified against PR HEAD with apply_actual_cost on a turn whose static estimate was $0.42 and two proxy records cost=0.01 and cost=None: the turn's total_cost_usd became 0.01 and the reconciliation residual 0.0, i.e. the unpriced call's real spend vanished from the run total (which _aggregate_token_usage then re-derives from these per-turn costs). This is reachable without any bug in the proxy: the same LiteLLM proxy also serves the Bedrock model entries in litellm/litellm-config.yaml (deepseek.v3.2, zai.glm-5, moonshotai.kimi-k2.5), whose responses carry no OpenRouter usage.cost — so a run whose ANTHROPIC_SMALL_FAST_MODEL resolves to a Bedrock-served model, or any call whose record was lost/unpriced, produces a silently understated authoritative cost that now looks like the real bill. Fix: only override when every call in the turn reports a cost (if costs and len(costs) == len(calls)); otherwise keep the static estimate for the turn and log a warning naming the unpriced call ids.
4. [Axis 7] LITELLM_COST_LOG is missing from the .env copy-paste block, .env.example, and the docker env allowlist/mounts, so the actual-cost join silently no-ops (notably under --driver docker) (litellm/start-litellm.sh:90) — The proxy side defaults the path (start-litellm.sh:62 export LITELLM_COST_LOG="${LITELLM_COST_LOG:-$REPO_ROOT/tmp/litellm-costs.jsonl}") but the harness side does not (src/coder_eval/config.py:122 litellm_cost_log: str | None = None), and the script's own copy-paste block omits it:

90:Set these in coder_eval's .env (or shell) to use it:
91:  API_BACKEND=litellm
92:  LITELLM_BASE_URL=http://localhost:$PORT
93:  LITELLM_AUTH_TOKEN=$LITELLM_MASTER_KEY

grep -rn LITELLM_COST_LOG --include='*.md' --include='*.example' over the PR HEAD tree returns nothing (only a comment in litellm/litellm-config.yaml:108); .env.example has no LITELLM_* section at all. An operator who follows the printed instructions exactly gets settings.litellm_cost_log is None, so Orchestrator._join_litellm_actual_cost returns at orchestrator.py:909 with no log line and no warning — the run is priced at the static rate card with 0 cache reads, i.e. exactly the bug this PR exists to fix, with zero signal that it did not engage. Two secondary gaps in the same surface: .env.example documents every other backend (BEDROCK_, CODEX_, GEMINI_*) but no LiteLLM vars; and LITELLM_COST_LOG is not in the docker driver's env allowlist (src/coder_eval/models/sandbox.py:235-238 lists only LITELLM_BASE_URL/AUTH_TOKEN/MODEL/SMALL_MODEL) and the log file is not bind-mounted, so --driver docker runs silently keep static pricing while local runs get real cost — an undocumented per-driver divergence in recorded cost. Fix: print LITELLM_COST_LOG=$LITELLM_COST_LOG in the script's block, add it to .env.example, and log one WARNING in _join_litellm_actual_cost when the route is LiteLLM but the setting is unset/missing. n/a
5. [Axis 8] Cost-log join key has no per-attempt component, so a reused --run-dir double-counts a prior attempt's proxy cost rows (src/coder_eval/litellm_cost.py:91) — mine = [r for r in records if r.get("run_id") == run_id and r.get("task_id") == task_id] (line 91) plus grouping on str(record.get("iteration")) (line 97) is the whole join key, and run_id is hash_identifier(self.run_dir.as_posix()) (orchestrator.py:914) — a deterministic SHA-256 of the path (telemetry.py:123). $LITELLM_COST_LOG is append-only and never truncated (litellm/start-litellm.sh:61-62 only mkdir -ps it). So a task that ran partway, was interrupted, and is re-run via coder-eval run --resume --run-dir ./r (batch.py:325 partition_for_resume re-runs every non-finalized task) produces the SAME (run_id, task_id, iteration) tuples, and the join sums BOTH attempts. Executed at PR HEAD: two rows of cost=0.03 under the same key -> turn.token_usage.total_cost_usd == 0.06 for a single attempt's work. This is the stale-artifact class the codebase already guards elsewhere — clear_rerun_artifacts (orchestration/batch.py:358) exists specifically because "stale files [would satisfy] file-based criteria and [skew] the resumed task's score" — with no equivalent for the cost log. Fix: stamp a per-Orchestrator-attempt nonce (e.g. uuid4().hex in x-ce-run-id, or a new x-ce-attempt tag) so a prior attempt's rows cannot match; add a test that feeds two attempts' rows under identical keys and asserts a single attempt's cost.

Non-blocking, but please consider before merge

  1. [Axis 1] provider_call_costs is a write-only field and three comments point readers at distributeActualCost / a per-call detail view that does not exist (evalboard/lib/pricing.ts:57) — pricing.ts:56-58 states "the harness captures each call's ACTUAL cost proxy-side and the detail view renders it per call (TurnRecord.provider_call_costs → distributeActualCost)", while runs.ts:282-283 states "There is no separate per-call panel and no fragile evalboard-side distribution". grep -rn distributeActualCost returns only comments (pricing.ts:58, evalboard/lib/tests/parseMessages.test.ts:815, evalboard/lib/tests/pricing-parity.test.ts:91) — no implementation exists; grep -rn provider_call_costs evalboard returns only pricing.ts:57, so the newly persisted TurnRecord.provider_call_costs has no dashboard consumer, contradicting litellm_cost.py:11-12 ("attached as TurnRecord.provider_call_costs (for the evalboard's per-call cache/cost view)"). Delete the distributeActualCost references, reword to what shipped (Python-side join surfacing inline as MessageEvent.costUsd), fix the litellm_cost.py docstring, and either wire a consumer for provider_call_costs or say in models/results.py:354 that it is an audit-only record. Also attach the orphaned block at runs.ts:278-283 to MessageEntry.cost_usd (runs.ts:1201) instead of leaving it floating between two interfaces.
  2. [Axis 1] Investigation scaffolding shipped in the proxy callback: dead _debug_ids plus the undocumented one-off CODER_EVAL_COST_DEBUG branch (litellm/cost_logger.py:223) — The helper's own docstring (cost_logger.py:225-228) says it was "Used once to determine whether the Anthropic msg_... message id ... is reachable in the callback, or only OpenRouter's gen-... id — deciding whether the harness can join per-generation by id instead of by position." That decision is already made in this same PR (litellm_cost.py:184-201 joins by position/output-token), so the 24-line, radon-B(9) key-inventory dumper can no longer change any outcome. Its env knob appears nowhere else: grep -rn CODER_EVAL_COST_DEBUG hits only cost_logger.py:205 and its own test — not Settings (config.py), not litellm/start-litellm.sh, not docs/. Delete _debug_ids (lines 223-246) plus the record["_debug"] = _debug_ids(...) branch (lines 205-210) and its test; if a hook is still wanted, use a one-line logger.debug(...) and document the variable next to LITELLM_COST_LOG in start-litellm.sh.
  3. [Axis 1] Correlation run-id derived by a duplicated expression at both the stamp and join sites, with no shared accessor and no test coupling them (src/coder_eval/orchestrator.py:914) — The stamp side writes "x-ce-run-id": hash_identifier(self.run_dir.as_posix()) (orchestrator.py:1254) and the join side independently recomputes run_id=hash_identifier(self.run_dir.as_posix()) (orchestrator.py:914); the comment at lines 1248-1249 acknowledges the coupling ("the join, in _finalize_result, recomputes it identically"). Any future normalization difference in run_dir makes apply_actual_cost return 0 at litellm_cost.py:91-93 and every turn silently reverts to the static rate-card estimate — nothing warns, because if applied: (orchestrator.py:918) only logs the success case. Extract a single accessor (e.g. @property def _cost_correlation_run_id) used by both call sites, and log when tags were stamped but applied == 0.
  4. [Axis 2] turn: Any on _distribute_onto_messages disables type checking of every TurnRecord/TokenUsage field it reads and writes (src/coder_eval/litellm_cost.py:144) — def _distribute_onto_messages(turn: Any, calls: list[ProviderCallCost]) -> None: widens the only structured parameter to Any, so turn.messages, turn.token_usage, and the subsequent usage.uncached_input_tokens / usage.cache_read_input_tokens / usage.cache_creation_input_tokens / usage.output_tokens reads (lines 224-233) are entirely unchecked. The widening is gratuitous: the module already imports from the same package at runtime (from coder_eval.models import AssistantMessage, ProviderCallCost, ReconciliationMessage, TokenUsage, line 32), so TurnRecord can be imported the same way (or under the existing TYPE_CHECKING block with EvaluationResult), and the call site at line 139 already has a properly typed TurnRecord. This matters concretely because TokenUsage's prompt buckets were recently renamed/split (uncached_input_tokens plus the derived input_tokens computed field, telemetry.py:52-85, with a _adopt_legacy_input_tokens shim for the old name) — a further rename would fail silently here at runtime instead of at make typecheck. Change the signature to turn: TurnRecord.
  5. [Axis 3] litellm/cost_logger.py: the two hooks LiteLLM actually calls (log_success_event / async_log_success_event, lines 216-220) and _emit's never-break-the-proxy except guard (212-214) are uncovered, and the file sits outside the --cov=coder_eval gate (litellm/cost_logger.py:216) — Measured directly (the file is outside --cov=coder_eval, so make verify never sees it): uv run coverage run --source=litellm -m pytest tests/test_litellm_cost_logger.py -n 0litellm/cost_logger.py 91 4 38 2 95.35% Missing: 72->64, 212-214, 217, 220, 235->234. Three concrete gaps behind those numbers:
    (a) Lines 216-220 — async def async_log_success_event(self, kwargs, response_obj, start_time, end_time): self._emit(kwargs, response_obj) and its sync twin — are the ONLY methods LiteLLM ever calls (litellm-config.yaml registers callbacks: cost_logger.proxy_handler_instance), and no test invokes either; every test calls _emit directly.
    (b) Lines 212-214 (except Exception: / pass) are uncovered even though test_emit_never_raises_on_garbage (tests/test_litellm_cost_logger.py:143) is named for them — _emit({}, None) and _emit({"litellm_params": None}, object()) both complete without raising, so the guard the docstring calls out ("A logging callback must NEVER break the proxy's response path") is never exercised.
    (c) The base class is a stub under test. Verified at PR HEAD: CustomLogger base: <class 'cost_logger.CustomLogger'> cost_logger / is fallback stub: True — the repo's own litellm/ directory shadows the pip package, so from litellm.integrations.custom_logger import CustomLogger at line 42 always falls through to the # pragma: no cover stub at lines 45-46. Nothing therefore verifies the hook names/signatures against the real LiteLLM CustomLogger ABC: an upgrade that renames or re-signs the hook yields zero captured records with a fully green suite, and the harness degrades to the exact static pricing this PR exists to remove, silently. Add tests that call proxy_handler_instance.log_success_event(...) / await ...async_log_success_event(...) with the full 4-arg signature, one that forces _emit to raise (e.g. monkeypatch append_record to throw) and asserts no propagation, and either add litellm/ to the coverage source or add a conformance assertion (hasattr/inspect.signature) that skips when the real litellm is absent.
  6. [Axis 3] Order-respecting call walk mis-binds a generation to an auxiliary call with equal output_tokens; the tie case is untested (tests/test_litellm_cost.py:286) — src/coder_eval/litellm_cost.py:192-195 binds greedily on equality — if gi < len(order) and (call.output_tokens or 0) == gen_outputs[gi]: — so a preceding auxiliary call (Claude Code's background haiku/init call) whose output happens to equal the next generation's output is consumed into that generation's slot. test_aux_call_unmatched_goes_to_reconcile (line 286) only covers a NON-colliding aux (_call_rec(1, 0.001, 50, 0, "aux", out=5) against generations at 10 and 20), so the tie case is untested. Verified by direct execution at PR HEAD with generations [10, 20] and calls [aux(out=10, cost=0.001, input=50), g1(out=10, cost=0.01, input=600), g2(out=20, cost=0.02, input=900)]:
m1 cost 0.001 input 50 (should be 0.01/600, real g1)
m2 cost 0.02 input 900
recon cost 0.009999999999999998

The walk reaches gi == len(order) so it distributes rather than bailing, and generation m1's timeline row shows $0.001 / 50 input tokens instead of $0.01 / 600. The turn and run totals stay correct (only per-message display is wrong), which is why this is Medium and not High. Add a test pinning the tie case, and consider requiring the walk to also agree on input_tokens (or the call count to match the generation count) before it commits to a distribution.
7. [Axis 3] test_litellm_config.py:21 hardcodes _OPENROUTER_MODELS instead of deriving it from model_list, so a fourth openrouter/* entry missing extra_body.usage.include passes the shape guard (tests/test_litellm_config.py:22) — Line 22 hardcodes _OPENROUTER_MODELS = {"moonshotai/kimi-k3", "z-ai/glm-5.2", "deepseek/deepseek-v4-pro"} and test_openrouter_models_capture_usage_and_pin_providers iterates only that set. Adding a fourth openrouter/* entry to litellm/litellm-config.yaml without extra_body.usage.include: true therefore passes the guard — and that omission is silent at runtime: build_cost_record reads usage.get("cost") (cost_logger.py:94), gets None, and apply_actual_cost falls back to static pricing (if costs: at litellm_cost.py:132), which is exactly the bug this PR fixes. It also violates the project's own test rule ("no hardcoded magic values from config", .claude/shared/review-rubric.md § Review Criteria #4). Derive the set from the config instead — e.g. {m["model_name"] for m in cfg["model_list"] if str(m["litellm_params"]["model"]).startswith("openrouter/")} — and assert it is non-empty, so every present and future OpenRouter model is checked for usage.include plus the provider pin automatically.
8. [Axis 3] The new evalboard TypeScript tests are not executed by any CI gate or make target this repo owns (evalboard/lib/__tests__/messageActualCost.test.ts:1) — This PR's only guard on the new evalboard/lib/runs.ts cost-preference logic (the costUsd: haveActualCost ? costUsdSum : messageCostUsd({...}) branch and the reconciliation-row typeof msg.cost_usd === "number" ? msg.cost_usd : ... branch) and on the evalboard/lib/pricing.ts OpenRouter de-pricing is this test file plus the parseMessages.test.ts / pricing-parity.test.ts edits — none of which ever run. evalboard/package.json defines "test": "vitest run" and "verify": "tsc --noEmit && vitest run && next build", but grep -rn "pnpm|vitest|npm " .github/workflows/ Makefile returns only npm install -g @anthropic-ai/claude-code install lines and a single osv-scanner comment (.github/workflows/pr-checks.yml:102: "Sibling lockfiles in this repo (evalboard/pnpm-lock.yaml, template node_modules) are intentionally out of scope here"). The gate gap is pre-existing debt, but this PR materially increases what rides on it — the evalboard is now the only surface that renders the actual per-call cost. Add an evalboard job to pr-checks.yml (pnpm install + pnpm --dir evalboard verify), path-filtered on evalboard/**, or at minimum an evalboard-test Makefile target wired into make verify.
9. [Axis 6] Cost-less join half-applies: the whole static turn cost is drained onto the reconciliation row (litellm_cost.py:233) while generations keep their rate-card estimate — timeline double-count (src/coder_eval/litellm_cost.py:232) — When NO call in the turn reports a cost, apply_actual_cost still sets turn.provider_call_costs = calls (line 131), still runs _distribute_onto_messages, and still does applied += 1 (line 140) — and the unconditional reconcile at lines 232-233 (reconciliation.cost_usd = usage.total_cost_usd - sum(m.cost_usd or 0.0 for m in assistants)) then writes the turn's ENTIRE static cost onto the reconciliation row, because every generation's cost_usd stayed None. Verified at PR HEAD: turn with static total_cost_usd=0.42 + two records with cost=None → per-message cost_usd = [None, None, 0.42], applied == 1. evalboard/lib/runs.ts:1760 renders msg.cost_usd verbatim on the reconciliation row while the generations fall back to messageCostUsd(...) (line 1676-1683), so the timeline shows the turn's full cost twice, on a documented configuration (litellm/start-litellm.sh advertises --model zai.glm-5 (or deepseek.v3.2 / moonshotai.kimi-k2.5), all Bedrock-served through this proxy). orchestrator.py:919 compounds it by logging "real per-call cost applied to 1 turn(s)" when zero real cost was applied. tests/test_litellm_cost.py:328 test_cost_less_run_leaves_reconcile_cost_none only covers the cost-less case when static_cost=None, so this path is untested. Fix: bail out of the join for a turn where no call carries cost_usd (don't attach null-cost provider_call_costs, don't touch the reconcile row, don't count it as applied).
10. [Axis 6] apply_actual_cost mutates turns in place with no rollback, so a mid-loop raise leaves a half-joined run while orchestrator.py:920 logs "keeping static pricing" (src/coder_eval/orchestrator.py:920) — except Exception: / logger.warning("LiteLLM actual-cost join failed; keeping static pricing", exc_info=True) (orchestrator.py:920-921) is not true of the code it guards: apply_actual_cost mutates result.iterations turn-by-turn (litellm_cost.py:118-140) with no snapshot/rollback, so an exception on turn N leaves turns 1..N-1 overridden with actual cost, crashed siblings already zeroed at line 127, and turns N..end on the static estimate. Verified at PR HEAD: two turns (static $0.42 / $0.50), records for iteration 1 valid and one iteration-2 record carrying a non-numeric cost ({"bad": 1}ProviderCallCost ValidationError in _to_call, line 59) → the call raised and left turn1.total_cost_usd == 0.03 while turn2 kept 0.50, with the log asserting static pricing was kept. Note load_cost_records (line 39) deliberately tolerates garbled lines only at the JSON level — a well-formed JSON object with a wrong-typed field still raises downstream. Fix: build the per-turn overrides into a local list first and apply them only after the loop completes (or deep-copy result.iterations and restore on exception), and make the warning message state what actually happened.
11. [Axis 6] No operator signal for which pricing regime a run used: no log on applied == 0, partial joins log success, and no run-level actual-vs-static marker (src/coder_eval/orchestrator.py:918) — if applied: / logger.info(...) (orchestrator.py:918-919) logs only on success — when settings.litellm_cost_log is explicitly configured but the join matches nothing (file absent, proxy never wrote, wrong path, run_id/task_id mismatch, tags not forwarded), apply_actual_cost returns 0 and nothing at all is emitted; the run then reports static Claude/rate-card pricing as if it were authoritative, which is exactly the wrong number this PR exists to fix. The inverse guard is missing too: litellm_cost.py:120-122 handles "turn with no records" (keep static) but the reverse — records whose iteration matches no TurnRecord — is never noticed. Verified at PR HEAD: records tagged iteration="2" against a result holding only iteration 1 → applied == 0, $9.99 of real recorded spend silently discarded, no log line. Fix: else: logger.warning("LiteLLM actual-cost join found no matching records for run=%s task=%s in %s; cost stays a static estimate", ...), and have apply_actual_cost also report the count of records whose iteration matched no turn so orphaned spend is visible.
12. [Axis 6] Proxy-side billing capture can lose every record with no diagnostic: blanket except Exception: pass plus an unguarded open() in append_record (litellm/cost_logger.py:212) — except Exception: / # A logging callback must NEVER break the proxy's response path. / pass (lines 212-214) is the right call for proxy liveness but leaves no trace whatsoever: append_record (line 169) does with open(path, "a", encoding="utf-8") as fh (line 179) with no guard, so a non-existent parent dir, a read-only path, or ENOSPC raises on EVERY call and every billing record for the whole run is lost — and the harness side is equally silent (see the orchestrator finding), so the operator sees a normal run priced at static rates. start-litellm.sh only mkdir -ps the dirname when it launches the proxy; an operator who exports LITELLM_COST_LOG by hand or runs the proxy another way gets the silent-loss path. Note the same handler also swallows genuine first-party bugs in build_cost_record / extract_tags. Fix: catch and emit once per process to stderr (a module-level _warned flag) — e.g. except Exception as exc: if not _warned: print(f"cost_logger: cost capture disabled ({exc})", file=sys.stderr); _warned = True — and pre-flight the log path (create the parent dir / test-write) at import so a misconfigured $LITELLM_COST_LOG is loud at proxy start rather than invisible for the whole run.
13. [Axis 7] New persisted task.json fields are absent from the documented consumer contract, and the documented reconciliation invariant is now false (src/coder_eval/models/telemetry.py:325) — docs/REPORT_SCHEMA.md is the declared cross-repo contract ("field-level reference for consumers (dashboards, CI parsers, evalboard forks)"), and its ### TurnRecord section enumerates fields verbatim:

168: `iteration`, `user_input`, `agent_output`, `commands` (`list[CommandTelemetry]`),
169: `timestamp`, `duration_seconds`, `token_usage`, `model_used`, `assistant_turn_count`,
...
175: > **Token invariant.** ... 177: The `reconciliation` message carries the residual the per-message stream
178: > under-reports; it has no cost and is excluded from turn/generation counts.

This PR adds three persisted fields consumed by the external evalboard / coder-eval-uipath pipeline — TurnRecord.provider_call_costs (models/results.py:354), AssistantMessage.cost_usd (telemetry.py:261) and ReconciliationMessage.cost_usd (telemetry.py:325) — and updates neither doc. Worse, line 178 ("it has no cost") and the identical claim in CLAUDE.md:141 ("carries no cost (cost stays on token_usage)", "excluded from ... the cost simulator") are now factually wrong: litellm_cost.py:232-235 always assigns reconciliation.cost_usd. Same theme, two more members: CLAUDE.md's module map has no entry for the new top-level src/coder_eval/litellm_cost.py (it lists every sibling — pricing.py, telemetry.py, utils.py, lines 26-28) and CLAUDE.md:44's telemetry.py contents line does not list ProviderCallCost. None of this is caught by CI: CE030 doc-parity tracks only TaskDefinition/RunLimits/Dataset/SimulationConfig, so persisted TurnRecord/message fields are unenforced. Update REPORT_SCHEMA.md's TurnRecord section + invariant note and the CLAUDE.md map; consider extending CE030's tracked-model list to the persisted record models. n/a
14. [Axis 7] The proxy→harness JSONL wire contract is untyped, unversioned, and hand-duplicated in four places with no round-trip parity guard (src/coder_eval/litellm_cost.py:59) — The record keys are written as string literals by the producer (litellm/cost_logger.py:96-108: "run_id", "task_id", "iteration", "call_id", "cost", "input", "cache_read", "cache_write", "output"), re-typed by hand in the consumer:

59: def _to_call(record: dict[str, Any]) -> ProviderCallCost:
61:     call_id=record.get("call_id"),
62:     cost_usd=record.get("cost"),
63:     input_tokens=record.get("input"),

plus the join keys at litellm_cost.py:91/97, and a third copy in the test fixture tests/test_litellm_cost.py:_rec (lines 49-61). The same duplication applies to the correlation-header names (x-ce-run-id/x-ce-task-id/x-ce-iteration in claude_code_agent.py:1140 vs cost_logger.py:50,97-99). A rename on either side silently yields cost_usd=None for every call → costs empty at litellm_cost.py:132 → the turn keeps its static estimate with no error, i.e. the failure mode is silent mispricing. The proxy genuinely cannot import coder_eval (by design, cost_logger.py:27-29), but a test can: tests/test_litellm_cost_logger.py already loads cost_logger by path via importlib.util.spec_from_file_location, so a single round-trip test — build_cost_record(...)json.dumpsload_cost_recordsapply_actual_cost → assert the cost landed — would pin both halves. tests/test_litellm_config.py guards the YAML/callback symbol but nothing guards the record shape. Also consider a "v": 1 schema key on the record so a future format change is detectable rather than silent. n/a
15. [Axis 7] Dropping the three OpenRouter models from evalboard PRICING silently hides the whole thinking-cost simulator (and the timeline Δ badges) for every open-weight run, including historical ones (evalboard/lib/pricing.ts:53) — The PR removes "moonshotai/kimi-k3", "z-ai/glm-5.2" and "deepseek/deepseek-v4-pro" from PRICING (replaced by the comment at pricing.ts:53-59, "DELIBERATELY NOT priced here"). The stated rationale only covers the message timeline, but PRICING has a second consumer: lib/thinkingSim.ts:317-318

const pricing = resolvePricing(primaryModel);
if (!pricing || !primaryModel) return null;

and the module's own comment at thinkingSim.ts:289 says "callers then hide the simulator" on null. So the task-detail ThinkingSimulator panel (app/runs/[id]/[...task]/thinking-simulator.tsx, wired in _sections.tsx:19) now renders nothing for open-weight runs — including all already-stored historical runs, which have no cost_usd to fall back on and previously simulated fine. The per-call cost_usd join does not help the simulator, which needs per-bucket rates to model a counterfactual. Either keep the entries for the simulator's use (documenting that they are approximate), or derive an effective per-bucket rate from provider_call_costs for it, or state explicitly in the PR/comment that the simulator is intentionally dropped for these models and update thinkingSim's fallback message. n/a
16. [Axis 8] COST_BUDGET_EXCEEDED rows persist a cost below max_usd on LiteLLM runs: the gate trips on the inflated static estimate that the new actual-cost join then overwrites (src/coder_eval/orchestrator.py:707) — self._join_litellm_actual_cost() (line 707) runs inside _finalize_result, which executes on the budget-breach path too (the except BudgetExceededError handler at line 539 sets FinalStatus.COST_BUDGET_EXCEEDED and finalization still runs at line 616). The live gate sums per-turn total_cost_usd (costs = [u.total_cost_usd for u in usages ...]; if total_cost > limits.max_usd at lines 851-861) using the static _reprice_for_litellm estimate, which this PR's premise says is systematically too high for open-weight runs (all prompt tokens billed as uncached because the SDK sees 0 cache reads). The join then rewrites those same turn costs to the real bill, so a task can persist final_status: COST_BUDGET_EXCEEDED alongside a total_cost_usd far BELOW max_usd — an internally inconsistent record for the out-of-repo dashboard/eval-runner to interpret. Fix: either read the cost log incrementally so the gate uses actuals, or preserve the tripping figure in environment_info (the estimate is currently only recoverable from the free-text error_message), and document the estimate-vs-actual gate semantics next to run_limits.max_usd.

Nits

  1. [Axis 1] cost_log_tags omitted from both docstring Args: blocks that document every sibling parameter (src/coder_eval/agents/claude_code_agent.py:666) — ClaudeCodeAgent.__init__'s docstring (claude_code_agent.py:668-684) documents config, route, instance_name, extra_mcp_servers but not the newly added cost_log_tags (line 666); _build_sdk_env's docstring (lines 741-756) documents route, path_prepend, plugin_tools_dir but not cost_log_tags (line 739). Add an Args: entry to both — the required key set (x-ce-run-id / x-ce-task-id, with x-ce-iteration appended per turn) is otherwise only discoverable from the orchestrator. The inline comment at lines 687-691 already has the wording.
  2. [Axis 1] Shared per-call cost log is read whole into memory and fully parsed by every task finalize, and is never rotated or truncated (src/coder_eval/litellm_cost.py:39) — _join_litellm_actual_cost calls load_cost_records(settings.litellm_cost_log) once per task (orchestrator.py:916), and load_cost_records does p.read_text(encoding="utf-8").splitlines() over the entire file (litellm_cost.py:46) before apply_actual_cost filters to this run/task (litellm_cost.py:91). start-litellm.sh:61 defaults to one shared $REPO_ROOT/tmp/litellm-costs.jsonl that append_record (cost_logger.py:179) only ever appends to, so a long-lived proxy accumulates every call from every past run and each task in an N-task batch re-parses all of it. Either filter line-by-line while streaming (memory O(matches)) or default LITELLM_COST_LOG to a run-scoped filename, and state the rotation expectation in the module docstring.
  3. [Axis 1] Identical 5-line comment block pasted verbatim above each of the three OpenRouter models (litellm/litellm-config.yaml:60) — The block starting # Ask OpenRouter to return the ACTUAL per-call cost (usage.cost) + cache is byte-identical at litellm-config.yaml:60, :76 and :92. Keep the rationale once in the shared OpenRouter header that already spans all three models (lines 41-54) and leave a one-line pointer (# usage.include: see the OpenRouter note above) on each entry, so a wording change can't leave two stale copies.
  4. [Axis 2] _num accepts non-finite floats, so a NaN/Inf provider cost propagates into total_cost_usd and makes task.json invalid JSON (litellm/cost_logger.py:55) — The numeric filter is return value if isinstance(value, int | float) and not isinstance(value, bool) else None — it deliberately rejects bool and strings but not nan/inf. A non-finite usage.cost therefore survives into the JSONL (json.dumps({'cost': float('nan')}) emits the non-standard bare {"cost": NaN}, which json.loads accepts on the read side), is accepted by ProviderCallCost.cost_usd: float | None (models/telemetry.py:130 — pydantic's allow_inf_nan defaults to True; verified by execution), and is then summed into turn.token_usage.total_cost_usd at litellm_cost.py:133-138. Downstream, nan > limit is always False so the run_limits.max_usd gate silently never fires, and the persisted task.json contains a bare NaN token that JSON.parse in the evalboard rejects for the whole file. Cheap fix at the boundary: import math and return value if isinstance(value, int | float) and not isinstance(value, bool) and math.isfinite(value) else None. This is the same class as Review Criterion 15 ("NaN / non-finite guards") applied to a money value crossing a process boundary.
  5. [Axis 4] Unsanitized author-defined task/variant ids are interpolated verbatim into the newline-delimited ANTHROPIC_CUSTOM_HEADERS string (header injection; bypasses the repo's hash_identifier convention) (src/coder_eval/agents/claude_code_agent.py:826) — claude_code_agent.py:826 builds the header block by raw f-string join, with no validation of k or v:
env["ANTHROPIC_CUSTOM_HEADERS"] = "\n".join(f"{k}: {v}" for k, v in cost_log_tags.items())

The values come from orchestrator.py:1254-1255:

"x-ce-run-id": hash_identifier(self.run_dir.as_posix()),
"x-ce-task-id": self._log_task_id,

_log_task_id is format_task_log_id(variant_id, task.task_id, replicate_index) (orchestrator.py:398) → f"{variant_id}/{task_id}/{NN}" (path_utils.py:52). Neither component is constrained: TaskDefinition.task_id is a bare str = Field(description="Unique identifier for this task") (models/tasks.py:310) and ExperimentVariant.variant_id is a bare str (models/experiment.py:25). The delimiter of this format is exactly the character that is unvalidated, so a task YAML with task_id: "probe\nAuthorization: Bearer forged" (or \r, or a value containing : ) appends attacker-chosen headers to every SDK→proxy request for that run — enabling forged x-ce-run-id/x-ce-task-id tags that mis-attribute another run's cost in the shared JSONL log, or an override of the auth/routing headers sent to ANTHROPIC_BASE_URL (which is operator-configurable and need not be localhost). Note the dataset path is already guarded — task_loader.py:417 rejects row ids not matching _ROW_ID_PATTERN = re.compile(r"^[A-Za-z0-9_][A-Za-z0-9_.\-]*$") — so the gap is specifically the YAML-authored task_id/variant_id.

Fix: reject or percent-encode header-unsafe characters at the seam — e.g. in _build_sdk_env, if any(c in v for c in "\r\n") or not v.isascii(): raise ValueError(...) (non-ASCII also breaks latin-1 header encoding), or apply the same _ROW_ID_PATTERN-style validator to TaskDefinition.task_id / ExperimentVariant.variant_id so the whole codebase benefits. Mechanically guardable as a CEnnn lint rule: no f-string interpolation into a value assigned to env["*_HEADERS"]. CVSS:3.1/AV:L/AC:L/PR:L/UI:N/S:U/C:N/I:L/A:N (3.3)


Continued in the next comment (2/2): What's Missing · Harness & Lint Improvements · Top 5 Priority Actions.

@uipreliga

Copy link
Copy Markdown
Collaborator

Review: coder_eval — pr:66 (21 files) axis:1,2,3,4,5,6,7,8 — continued (2/2)

What's Missing

Parallel paths:

  • 🟠 Only ClaudeCodeAgent grew the cost_log_tags seam, but _create_agent (orchestrator.py:1252-1257) gates the kwarg on the route, not the agent — NoOpAgent/CodexAgent/AntigravityAgent and every out-of-tree plugin agent were not updated (no kwarg, no **kwargs), so the parallel agent paths crash under API_BACKEND=litellm; the tagging/join feature also simply doesn't exist for codex/antigravity. (trigger: src/coder_eval/orchestrator.py) (restates: Axis 1: LiteLLM-only cost_log_tags kwarg passed through the agent-agnostic create_agent factory)
  • 🟠 The docker driver path was not updated alongside the new Settings.litellm_cost_log: models/sandbox.py:235-238 still forwards only LITELLM_BASE_URL/AUTH_TOKEN/MODEL/SMALL_MODEL and isolation/docker_runner.py:1114-1133 has a bespoke host-rewrite for LITELLM_BASE_URL but no forwarding or bind-mount for the cost log — so --driver docker runs (where the orchestrator, and hence the join, executes in-container) silently keep static pricing while local runs get real cost. (trigger: src/coder_eval/config.py) (restates: Axis 7: LITELLM_COST_LOG missing from the .env block, .env.example, and the docker env allowlist/mounts)
  • 🟡 The existing LiteLLM preflight (cli/run_command.py::_litellm_preflight_error, which already fails fast on an unreachable proxy) was not extended to check that LITELLM_COST_LOG is set/readable and matches the proxy's path — the one seam designed to catch open-weight misconfiguration before the run starts still passes a config that will silently fall back to rate-card pricing. (trigger: src/coder_eval/config.py) (restates: Axis 6: No operator signal for which pricing regime a run used)
  • 🟡 orchestration/batch.py::clear_rerun_artifacts exists precisely to purge stale per-run artifacts before a resumed/re-run task, but nothing was added there (or anywhere) to scope/prune the append-only shared cost log, so the join reads a prior attempt's rows for the same run_dir/task_id/iteration key. (trigger: src/coder_eval/litellm_cost.py) (restates: Axis 8: Cost-log join key has no per-attempt component (reused --run-dir double-counts))
  • 🔵 start-litellm.sh:61-62 only exports and mkdir -ps a single shared tmp/litellm-costs.jsonl; the proxy side has no run-scoping, truncation, or rotation counterpart to the harness's per-run run_dir, so the file grows across every run and each task re-parses all of it. (trigger: litellm/start-litellm.sh) (restates: Axis 1: Shared per-call cost log is read whole into memory and never rotated)

Tests:

  • 🟠 No test asserts the framework's headline invariant after the join — Σ(four token buckets over TurnRecord.messages) == token_usage — even though _distribute_onto_messages now rewrites both the assistant rows and the reconciliation residual with max(0, …); every TestDistributeOntoMessages case pre-agrees with the proxy, so the real LiteLLM shape (cache_read_input_tokens=0 on the turn) is untested. (trigger: tests/test_litellm_cost.py) (restates: Axis 5: Actual-cost join is a second writer of the message-stream token-bucket invariant and clamps it away)
  • 🟡 The new _join_litellm_actual_cost() call site at orchestrator.py:707 has no test through a real Orchestrator: TestOrchestratorJoinHook invokes the unbound method on a SimpleNamespace and hand-orders _join_litellm_actual_cost then _aggregate_token_usage itself, so deleting or reordering the call inside _finalize_result keeps the suite green — despite test_orchestrator.py:1986 already providing a primed-orchestrator _finalize_result harness that could have been reused. (trigger: src/coder_eval/orchestrator.py)
  • 🟡 No test covers the sub-agent transcript shape that _distribute_onto_messages' own docstring names as the known-diverging case ("Empirically clean on 14/15 observed runs (all but the sub-agent one)") — a stream containing a synthesized message_id="subagent-<tool_use_id>" terminal generation plus parent_tool_use_id-tagged children; the bail-to-reconcile branch is only exercised via a synthetic permuted-order fixture, so the behavior on the feature CLAUDE.md documents as first-class (sub-agent token accounting) is unpinned. (trigger: src/coder_eval/litellm_cost.py)
  • 🟡 The two methods LiteLLM actually invokes (log_success_event / async_log_success_event) and the blanket except Exception: pass are never executed by a test, and litellm/ is outside --cov=coder_eval, so make verify reports nothing for the whole capture seam. (trigger: litellm/cost_logger.py) (restates: Axis 3: LiteLLM callback hooks and the never-break-the-proxy guard are uncovered and outside the coverage gate)
  • 🟡 No round-trip test crosses the producer/consumer boundary (build_cost_recordjson.dumpsload_cost_recordsapply_actual_cost), even though the test file already loads cost_logger by path; the record keys are re-declared by hand in four places (producer, _to_call, and two test fixtures) with nothing pinning them together. (trigger: tests/test_litellm_cost_logger.py) (restates: Axis 7: The proxy→harness JSONL wire contract is untyped, unversioned, and hand-duplicated)
  • 🟡 The three new/edited vitest files are the only guard on the haveActualCost preference branch and the reconciliation-row cost branch, and no make target or workflow runs vitest — the PR adds tests to an unexecuted suite. (trigger: evalboard/lib/tests/messageActualCost.test.ts) (restates: Axis 3: The new evalboard TypeScript tests are not executed by any CI gate or make target)
  • 🔵 The new DROP_KEYS mechanism itself has no test: it drops cost_usd/provider_call_costs by name at any nesting depth, so a future agent-emitted cost_usd (e.g. on a message or command telemetry record) would silently vanish from every golden with no failure — the golden-master suite can no longer regress-detect per-message cost at all. _(trigger: tests/_fixtures/golden_streams/scrub.py)

Downstream consumers:

  • 🟠 selectTokenTotals (runs.ts:1988-1999) returns the raw message-stream sum whenever a reconciliation row is present, on the documented promise that the stream sums exactly to token_usage; the join now rewrites those buckets from a second source without updating that consumer or re-asserting the contract, so open-weight runs with cache hits render inflated input/cache totals. (trigger: evalboard/lib/runs.ts) (restates: Axis 5: Actual-cost join is a second writer of the message-stream token-bucket invariant and clamps it away)
  • 🟡 The run_limits.max_usd gate (_check_run_limits, orchestrator.py:851-861) still sums the static per-turn costs that the join later overwrites; no consumer of the changed cost formula (the live budget check, its COST_BUDGET_EXCEEDED status, and the error_message carrying the tripping figure) was updated for estimate-vs-actual semantics. (trigger: src/coder_eval/orchestrator.py) (restates: Axis 8: COST_BUDGET_EXCEEDED rows persist a cost below max_usd on LiteLLM runs)
  • 🟡 No cost-provenance marker was added to the persisted record: environment_info["cost_data_available"] (orchestrator.py:718) is still a bare max_usd-enforceability boolean, so reports.py cost rollups and — worse — reports_experiment.py's cross-variant cost deltas can compare a real OpenRouter bill against a rate-card estimate with nothing in the report saying which regime each side used. (trigger: src/coder_eval/orchestrator.py) (restates: Axis 6: No operator signal for which pricing regime a run used)
  • 🟡 docs/REPORT_SCHEMA.md's ### TurnRecord field list and its "the reconciliation message has no cost" invariant note (plus the same claim in CLAUDE.md and in ReconciliationMessage's own docstring) were not updated for provider_call_costs / AssistantMessage.cost_usd / ReconciliationMessage.cost_usd; CE030's tracked-model list doesn't cover persisted record models, so nothing in CI catches the drift. (trigger: src/coder_eval/models/results.py) (restates: Axis 7: New persisted task.json fields are absent from the documented consumer contract)
  • 🔵 aggregateSubAgentUsage (runs.ts:300-322) — the documented way per-sub-agent usage is derived — sums only the four token buckets and was not extended to the new per-message cost_usd, so the "per-sub-agent cache/cost view" the provider_call_costs docstring points at still has no cost dimension. (trigger: evalboard/lib/runs.ts)

Display & mapping dicts:

  • 🟡 Removing the three ids from the PRICING map without extending its other consumer makes resolvePricing return null, so buildThinkingModel bails and the task-detail ThinkingSimulator panel plus the per-message projected-Δ badges silently disappear for every open-weight run, including historical ones; pricing.ts:5-8 still claims the map is the frontend's single source of truth. (trigger: evalboard/lib/pricing.ts) (restates: Axis 7: Dropping the three OpenRouter models from evalboard PRICING hides the thinking-cost simulator)
  • 🔵 The Cost column (_sections.tsx:737 / :1102) formats a real proxy-captured cost and a rate-card estimate identically with no legend, tooltip, or badge, and the reconciliation row's note text was not extended for the fallback case where it now carries the entire turn's bill — the display mapping gained a new value class but no rendering distinction. (trigger: evalboard/lib/runs.ts) (restates: Axis 6: Cost-less join half-applies — the whole static turn cost is drained onto the reconciliation row)

Daily/nightly:

  • 🟡 The PR states no blast radius for the production path: it changes the task.json schema consumed by the external coder-eval-uipath / evalboard pipeline (three new persisted fields, plus per-turn total_cost_usd now meaning "real bill" on one backend only) and leaves the --driver docker entrypoint — the production runner — on static pricing; neither is mentioned in the summary or the docs. (trigger: src/coder_eval/orchestrator.py) (restates: Axis 7: LITELLM_COST_LOG missing from the .env block, .env.example, and the docker env allowlist/mounts)
  • 🟡 Only the three openrouter/* entries request usage.include; the three Bedrock-served models that start-litellm.sh's closing banner advertises as the default run command (--model zai.glm-5 (or deepseek.v3.2 / moonshotai.kimi-k2.5)) report no usage.cost, so the documented happy path takes the cost-less join branch — the PR never says which models actually benefit and which degrade. (trigger: litellm/litellm-config.yaml) (restates: Axis 6: Cost-less join half-applies — the whole static turn cost is drained onto the reconciliation row)
  • 🔵 There is zero operator documentation for this feature outside the script and code comments: git grep -i litellm over docs/, README.md and .env.example returns nothing at either main or PR HEAD, so anyone standing up an open-weight nightly has no documented procedure for wiring LITELLM_COST_LOG or for interpreting actual-vs-static cost in the resulting reports. (trigger: litellm/start-litellm.sh) (restates: Axis 7: LITELLM_COST_LOG missing from the .env block, .env.example, and the docker env allowlist/mounts)

Harness & Lint Improvements

Static checks (lint / type):

  • [ce-lint] CE026 — agent-factory kwargs must be accepted by every registered agent. New AST rule in tests/lint/rules/ce026_agent_factory_kwarg_parity.py, wired into ALL_RULES in tests/lint/runner.py. Two clauses: (1) any keyword passed to create_agent(...) must be a named parameter of every Agent subclass __init__ under src/coder_eval/agents/ (or that constructor must accept **kwargs); (2) a **-splat of a locally-built dict into create_agent(...) is flagged outright as unverifiable — pass kwargs explicitly, or gate them on an Agent capability ClassVar (the existing supports_cooperative_stop pattern at agent.py:80 / claude_code_agent.py:657). This is the AST-detectable analogue of CE020 (no SDK-typed fields on the base agent): a Claude-only parameter must not reach the agent-agnostic seam. Prevents: Finding 1 (high): orchestrator.py:1252-1257 forwards the LiteLLM-only cost_log_tags through create_agent, so NoOpAgent/CodexAgent/AntigravityAgent (and every out-of-tree plugin agent) raise TypeError under API_BACKEND=litellm. Clause (2) fires on the exact **kwargs splat at line 1257.
  • [ce-lint] Extend the three existing gates to litellm/. tests/test_custom_lint.py:20 sets SRC = <repo>/src and check_paths([SRC]); pyproject.toml has [tool.pyright] include = ["src/coder_eval"] and [tool.coverage.run] source = ["src/coder_eval"]. litellm/cost_logger.py is production code (it runs inside the proxy and is the sole producer of the billing record), yet no gate sees it. Add litellm/ to the lint check_paths roots, to pyright.include, and to coverage.run.source (with a --cov-fail-under carve-out if needed initially). Zero new rule code required — the already-registered CE005 no_silent_except immediately flags except Exception: / pass at cost_logger.py:212-214, and the encoding/json rules apply too. Prevents: Finding 19 (medium: blanket except Exception: pass silently discards every billing record, CE005 catches it verbatim); Finding 10 (medium: litellm/cost_logger.py sits outside --cov=coder_eval, so its 95%/missing-lines profile is invisible to make verify); Finding 3 (dead _debug_ids at cost_logger.py:223-246 — dead-code/complexity gates would surface it).
  • [ruff] Enable ANN401 (flake8-annotations: dynamically-typed expression in argument annotation) in [tool.ruff.lint] select, scoped to src/coder_eval/** via per-file-ignores for tests. ANN401 flags only a bare Any annotation on a parameter — it does not touch legitimate generic containers like record: dict[str, Any], so the JSONL row helpers (load_cost_records, _to_call) stay clean while the widened structured parameter is caught. Verified cheap: substituting the narrow type on the offending function and running pyright yields 0 errors, so this is a no-churn tightening. Prevents: Finding 8 (medium): _distribute_onto_messages(turn: Any, ...) at src/coder_eval/litellm_cost.py:144 disables checking of every TurnRecord/TokenUsage field it reads and writes (uncached_input_tokens, cache_read_input_tokens, …), exactly the buckets recently renamed with a legacy shim — a further rename would fail at runtime instead of at make typecheck.
  • [ce-lint] CE032 — inverse env-var parity (consumed ⇒ documented), plus a LITELLM_ prefix. tests/lint/doc_env_parity.py (CE027) currently enforces one direction only: a documented framework-prefixed var must be consumed. Add the inverse pass in the same module (wired as a second test class in tests/test_custom_lint.py): every framework-prefixed env name that is either a Settings field or read via os.getenv/os.environ[...] in src/ or litellm/ must appear in at least one operator surface (.env.example, README.md, docs/**, litellm/start-litellm.sh) or carry an EXEMPT entry with a reason. Also add "LITELLM_" to FRAMEWORK_ENV_PREFIXES (currently CODER_EVAL_, API_, BEDROCK_, CODEX_, ANTIGRAVITY_, TELEMETRY_), and add .env.example to the scanned surfaces. Prevents: Finding 'LITELLM_COST_LOG missing from the .env block/.env.example' (high): config.py:122 defines litellm_cost_log but git grep LITELLM_COST_LOG hits only one YAML comment and the launcher, so an operator following the printed instructions gets settings.litellm_cost_log is None and silent static pricing. Also Finding 3: CODER_EVAL_COST_DEBUG is read at cost_logger.py:205 and documented in no Settings field, no docs page, no launcher.
  • [ce-lint] CE033 — docker env-passthrough parity. New whole-tree rule (tests/lint/docker_env_parity.py, wired as a test class like CE027-CE031): every Settings field carrying a backend prefix that the in-container orchestrator needs (LITELLM_*, BEDROCK_*, CODEX_*, ANTIGRAVITY_*) must appear in the SandboxConfig.env_passthrough default allowlist (src/coder_eval/models/sandbox.py:208-262) or have an EXEMPT entry naming the reason. Path-valued settings additionally require a mount, which the exemption text is the place to record — that forces the author to confront the docker case rather than discover it as a driver-dependent behavior difference. Prevents: The docker half of the LITELLM_COST_LOG finding (high): the allowlist at sandbox.py:235-238 lists only LITELLM_BASE_URL/AUTH_TOKEN/MODEL/SMALL_MODEL, and the JSONL is not bind-mounted, so --driver docker runs always keep static pricing while local runs get real cost — an undocumented per-driver divergence in recorded cost, the harness's primary output.
  • [ce-lint] CE034 — single-writer seam for message token buckets. New AST rule tests/lint/rules/ce034_token_bucket_single_writer.py in ALL_RULES: assignment to .input_tokens / .cache_read_tokens / .cache_creation_tokens / .output_tokens on a transcript-message object is permitted only in src/coder_eval/streaming/collector.py. CLAUDE.md already declares this seam normatively ('booked at the single EventCollector seam'), so the rule just mechanizes a written invariant — same shape as CE017/CE023 (import-direction) rules. Add a companion clause flagging max(0, ...) around a reconciliation-residual computation, since the residual is signed by design (collector.py:111-141 even documents the negative case) and clamping is what silently destroys the sum. Prevents: Finding 15 (high): litellm_cost.py:207-231 becomes a second writer of those buckets from a different source and absorbs the disagreement with max(0, …), breaking the documented Σ(buckets) == token_usage invariant (reproduced: stream sum 9116 vs authoritative 5020) that selectTokenTotals in the evalboard relies on.
  • [ce-lint] CE035 — non-finite floats must not cross a serialization boundary. New rule with two clauses: (1) every json.dumps(...) / json.dump(...) call must pass allow_nan=False (grep/AST-trivial, and directly in the family of the existing open_explicit_encoding / read_text_explicit_encoding / subprocess_run_explicit_encoding rules — there are ~13 call sites in src/); (2) float fields on persisted models under src/coder_eval/models/ that carry money or token values must declare allow_inf_nan=False (or use a shared Annotated finite-float type). Clause (2) is the durable fix, since it rejects the value at the model boundary regardless of which producer emitted it. Prevents: Finding 9 (low, but wide blast radius): _num at cost_logger.py:55 rejects bool but not nan/inf, so a non-finite usage.cost is written as a bare NaN token (non-standard JSON that JSON.parse rejects for the whole task.json), is accepted by ProviderCallCost.cost_usd (pydantic allow_inf_nan defaults True), and then makes nan > limits.max_usd always False — the run_limits.max_usd gate silently never fires.
  • [ce-lint] CE036 — cross-process wire-contract parity. New whole-tree rule in the CE025 (live_verdict_consistency) style, which already precedents cross-file consistency checks. Three clauses: (1) the string keys written in litellm/cost_logger.py::build_cost_record must equal the keys read via record.get("…") in src/coder_eval/litellm_cost.py; (2) the x-ce-* correlation header names must be identical across orchestrator.py:1253-1256, claude_code_agent.py:1140, and cost_logger.py:50,97-99; (3) the correlation-key derivation (hash_identifier(self.run_dir.as_posix())) may appear at most once — forbid a second call site, forcing a named accessor. The proxy genuinely cannot import coder_eval (documented at cost_logger.py:27-29), so a shared constants module is unavailable and a lint rule is the only mechanical option. Prevents: Finding 20 (medium): the record shape is hand-duplicated in four places with no guard, and a rename on either side yields costs == [] → silent revert to static pricing with no error. Finding 4 (medium): the run-id expression is duplicated verbatim at the stamp (orchestrator.py:1254) and join (orchestrator.py:914) sites, with the author's own comment documenting the fragile coupling instead of removing it.
  • [ce-lint] CE037 — litellm/litellm-config.yaml shape rule. New config-surface rule (a test class, like CE028's mkdocs-nav check): for every entry in model_list, if litellm_params.model starts with openrouter/ then extra_body.usage.include: true must be set and the provider pin block must be present; assert the derived set is non-empty so a renamed/emptied model_list cannot pass vacuously. This replaces the hardcoded literal set rather than merely testing around it. Prevents: Finding 12 (medium): tests/test_litellm_config.py:21 hardcodes _OPENROUTER_MODELS = {…} instead of deriving it from model_list, so a fourth openrouter/* entry missing extra_body.usage.include passes the guard and silently regresses to static pricing — and it trips the project's own rubric rule ('no hardcoded magic values from config', .claude/shared/review-rubric.md:46).
  • [bandit-codeql] CE038 — header-value interpolation must be sanitized (custom rule; neither bandit nor CodeQL reaches this shape). Forbid f-string/str.join interpolation of non-literal values into a value assigned to env["*_HEADERS"] in src/coder_eval/agents/**; require the value to come from a sanitizing helper that rejects \r/\n/non-ASCII. Pair it with the durable model-level fix: constrain TaskDefinition.task_id (models/tasks.py:310) and ExperimentVariant.variant_id (models/experiment.py:25) with the _ROW_ID_PATTERN-style pattern= already applied to dataset row ids at task_loader.py:417. Why a CE rule and not the scanners — recorded deliberately: bandit ships no header-injection check, and CodeQL's py/http-response-splitting never fires because the taint terminates in an os.environ dict consumed by a child SDK process, not in an HTTP-response/request call, so no dataflow query in the standard pack models this sink. Prevents: Finding 14 (low, CVSS 3.3): claude_code_agent.py:826 builds ANTHROPIC_CUSTOM_HEADERS by raw newline-join, and _log_task_id is f"{variant_id}/{task_id}/{NN}" from two unvalidated author-supplied str fields — a task_id containing \n appends attacker-chosen headers to every SDK→proxy request (forged cost attribution, or an auth/routing header override against an operator-configurable ANTHROPIC_BASE_URL).
  • [ce-lint] Extend CE030's DOCUMENTED_MODELS to the persisted record models. tests/lint/doc_schema_parity.py:45-48 tracks only TaskDefinition/RunLimits/Dataset/SimulationConfig, all paired with docs/TASK_DEFINITION_GUIDE.md. Add (TurnRecord, "docs/REPORT_SCHEMA.md"), (AssistantMessage, …), (ReconciliationMessage, …), (ProviderCallCost, …)docs/REPORT_SCHEMA.md is self-declared as the cross-repo consumer contract, so a field added to a persisted record is exactly the kind of standing documentation obligation this rule exists to enforce. Four tuples of new code. Prevents: Finding 'new persisted task.json fields absent from the consumer contract' (medium): TurnRecord.provider_call_costs, AssistantMessage.cost_usd, and ReconciliationMessage.cost_usd ship undocumented, while REPORT_SCHEMA.md:178 and CLAUDE.md:141 still assert the reconciliation row 'has no cost' — a fork or CI parser written to the contract would under-report the true bill on every open-weight run.
  • [ce-lint] Extend CE031 (dead config fields) to persisted record models, with a TypeScript consumer sweep. tests/lint/dead_config_fields.py currently flags behavior-driving fields on SimulationConfig/RunLimits/Dataset that no code reads by name. Add persisted-record fields, and widen consumed_attr_names to also scan evalboard/lib/**.ts for the snake_case key (the dashboard is the declared consumer of task.json), so a field written by Python and read by nobody in either language is flagged. An EXEMPT entry with the reason 'audit-only record, no consumer' is the escape hatch — which is precisely the disclosure this finding wants. Prevents: Finding 2 (medium): TurnRecord.provider_call_costs is write-only — grep -rn provider_call_costs evalboard returns a single comment — while litellm_cost.py:11-12 and pricing.ts:57 both advertise an evalboard 'per-call view' and point at distributeActualCost, a symbol that exists nowhere in the tree.
  • [ruff] Enable the preview pydoclint rules DOC101/DOC103 (docstring has fewer/mismatched arguments than the signature) with preview = true, scoped initially to src/coder_eval/agents/** and src/coder_eval/orchestrator.py via per-file-ignores so the rest of the tree can adopt incrementally. These are the highest-churn public seams and the ones whose Args: blocks are the de-facto API reference for plugin authors. Prevents: Finding 5 (low): ClaudeCodeAgent.__init__'s docstring (claude_code_agent.py:668-684) documents config/route/instance_name/extra_mcp_servers but omits the new cost_log_tags, and _build_sdk_env's (741-756) omits it too — so the required key set (x-ce-run-id/x-ce-task-id + per-turn x-ce-iteration) is discoverable only from the orchestrator.

Harness improvements (not statically reachable):

  • Gate the evalboard test suite in CI and in make verify. Add a path-filtered (evalboard/**) evalboard job to .github/workflows/pr-checks.yml running pnpm install --frozen-lockfile + pnpm --dir evalboard verify (tsc --noEmit && vitest run && next build), and an evalboard-verify Makefile target chained into verify (Makefile:50, currently ruff/pyright/pytest only). All tooling is already committed (evalboard/vitest.config.ts, vitest.setup.ts, the vitest/jsdom devDeps) — this is pure CI wiring. Why not static: No lint or type rule can execute a vitest suite; the gap is that 20 existing TypeScript test files under evalboard/lib/__tests__/ are never run by anything this repo owns (grep -rn 'vitest|pnpm' .github/workflows/ Makefile returns only npm install -g @anthropic-ai/claude-code lines and one osv-scanner scope comment). Prevents: Finding 13 (medium): this PR's only guard on the new runs.ts cost-preference branch and the pricing.ts de-pricing is a test file that never executes — and the evalboard is now the only surface rendering actual per-call cost.
  • Round-trip test across the proxy↔harness process boundary. One test that loads litellm/cost_logger.py by path (the importlib.util.spec_from_file_location pattern tests/test_litellm_cost_logger.py:20-26 already uses), calls build_cost_record(...)json.dumpsload_cost_recordsapply_actual_cost, and asserts the cost landed on the turn. Add a "v": 1 schema key to the record so a future format change is detectable rather than silent. Why not static: CE036 catches key renames but not type or semantic drift (a key kept with a changed unit, a value type pydantic coerces differently, an iteration stringification change). Proving the record survives the JSON round trip and produces a nonzero applied count requires running both halves. Prevents: Finding 20 (medium: four hand-duplicated copies of the wire shape with no parity guard); backstops Finding 12 (silent costs == [] regression).
  • LiteLLM CustomLogger conformance + hook-invocation tests. Add a test with pytest.importorskip("litellm") asserting the real CustomLogger ABC declares log_success_event / async_log_success_event with the 4-arg signature CostLogger implements, plus tests that invoke proxy_handler_instance.log_success_event(...) and await …async_log_success_event(...) directly (every current test calls the private _emit), and one that monkeypatches append_record to raise and asserts no propagation. Why not static: litellm is deliberately absent from the dev env, so from litellm.integrations.custom_logger import CustomLogger always falls through to the # pragma: no cover stub at cost_logger.py:45-46 — the hook contract is unverifiable from source alone and needs the real package imported at its installed version. Prevents: Finding 10 (medium): lines 216-220 (the only methods LiteLLM ever calls) and the never-break-the-proxy guard at 212-214 are both uncovered; an upstream rename yields zero captured records with a fully green suite and a silent fall back to static pricing.
  • Per-attempt scoping of the cost-correlation key, plus a rerun regression test. Stamp an attempt nonce (x-ce-attempt, or a uuid4().hex component in x-ce-run-id) so a prior attempt's rows cannot match, and add a test that feeds two attempts' rows under identical (run_id, task_id, iteration) keys and asserts a single attempt's cost. This is the same stale-artifact class clear_rerun_artifacts (orchestration/batch.py:358) already exists to handle for file-based criteria — the cost log has no equivalent. Why not static: Double counting is a cross-process runtime-state property: $LITELLM_COST_LOG is append-only and never truncated, and the key is a deterministic hash of the reused --run-dir path. No source-level property distinguishes the first attempt's rows from the second's. Prevents: Finding 21 (high): re-running a task into the same run_dir (with or without --resume, since prepare_run_directory uses exist_ok=True) sums both attempts — reproduced: $0.06 recorded for $0.03 of work.
  • Assert the token/cost invariants inside the join instead of clamping, and test the real-world shape. After apply_actual_cost, assert Σ(four buckets over TurnRecord.messages) == token_usage and Σ(per-message cost_usd) == turn.token_usage.total_cost_usd; on violation, log and skip the distribution rather than absorbing it with max(0, …). Add the missing fixtures: a turn with cache_read_input_tokens=0 against a proxy record with nonzero cache_read (the case the PR's own premise makes the normal one), a partially-priced turn (some calls with cost=None), and an all-unpriced turn with a non-None static cost. Why not static: These are arithmetic relations over runtime values from two independent sources; CE034 can force single-writer discipline but cannot evaluate the sum. Prevents: Finding 15 (high: invariant broken, evalboard displays inflated totals); Finding 16 (high: partial coverage bills unpriced calls at $0 and still overrides the estimate); Finding 6 (medium: the whole static turn cost is drained onto the reconciliation row, double-counting the timeline).
  • Make apply_actual_cost transactional, with a fault-injection test. Stage per-turn overrides in a local list and commit only after the loop completes (or snapshot/restore result.iterations), then add a test that injects a malformed record mid-loop and asserts the result is byte-identical to the pre-join state — matching what the caller's except Exception: … "keeping static pricing" at orchestrator.py:920-921 already claims. Why not static: Half-applied mutation is only observable by executing the loop to a raise and inspecting the intermediate object graph; no static rule can tell a transactional mutator from a non-transactional one. Prevents: Finding 17 (medium): a raise on turn N leaves turns 1..N-1 overridden and crashed siblings zeroed, and because the join runs before _aggregate_token_usage (orchestrator.py:707 vs 710) the persisted run-level total_cost_usd is a silent mix of actual and static.
  • Record the pricing regime as structured run output, and log the non-success paths. Emit a WARNING when the route is LiteLLM and applied == 0 (naming the log path and the run/task key), and when the join is partial; add an environment_info marker (cost_regime: actual | static | mixed, plus the count of records whose iteration matched no turn) so run.json is self-describing. For budget breaches, persist the tripping figure structurally instead of only in the free-text error_message. Cover each with a caplog/field assertion. Why not static: Silence is a behavioral property of a specific runtime branch — a lint rule can see that if applied: has no else, but not that the missing branch is the one that matters, nor that the resulting persisted record is internally inconsistent. Prevents: Finding 18 (medium: a misconfigured path or a partial join reports static pricing as if authoritative, with zero signal — the exact wrong number the feature exists to fix); Finding 22 (medium: COST_BUDGET_EXCEEDED rows persist a total_cost_usd below max_usd because the gate tripped on the estimate the join then overwrote).
  • Golden test for the call↔generation distribution walk, including the tie case, plus a stronger commit condition. Pin the case where a preceding auxiliary call's output_tokens equals the next generation's (reproduced: the real $0.01/600-input generation is displaced onto the reconciliation row while the $0.001 aux lands on the message). Require the walk to also agree on input_tokens, or the call count to match the generation count, before it commits to a distribution. Why not static: The defect is in positional-matching semantics over runtime token counts — the code is well-typed and structurally unremarkable; only executing it with colliding values exposes the mis-binding. Prevents: Finding 11 (medium): the greedy equality bind at litellm_cost.py:193 reaches gi == len(order) and therefore distributes rather than bailing, so per-message cost/tokens are wrong while turn and run totals stay correct — invisible to every aggregate assertion.
  • Model-coverage test for the frontend thinking-cost simulator. Assert buildThinkingModel / resolvePricing returns non-null for every model_name in litellm/litellm-config.yaml and every key of src/coder_eval/pricing.py, or that the id appears on an explicit SIMULATOR_DISABLED list with a reason. Also refresh the now-false SSOT claim at pricing.ts:5-8 and delete the three dangling distributeActualCost references. Why not static: Requires the TS resolver's runtime lookup behavior (exact + prefix-stripped matching) across a language boundary, driven by ids that live in a Python module and a YAML file — not expressible as a single-language AST check. Depends on the evalboard CI gate above to have any teeth. Prevents: Finding 23 (medium): removing the three OpenRouter entries from PRICING silently disables the entire ThinkingSimulator panel and the per-message projected-Δ badges for every open-weight run, including already-stored historical ones.
  • Give the cost log a bounded lifecycle. Default LITELLM_COST_LOG to a run-scoped filename (or filter line-by-line while streaming instead of p.read_text().splitlines() at litellm_cost.py:46), state the rotation expectation in the module docstring, and add a test that a large log parses in bounded memory and that an N-task batch does not re-parse the whole file N times. Why not static: Unbounded growth and repeated full-file parsing are runtime resource properties of a shared, operator-configured, append-only path — the code shape (read_text on a Settings-supplied path) is indistinguishable from dozens of legitimate small-file reads. Prevents: Finding 'shared cost log read whole into memory, never rotated' (low); compounds with Finding 21, since an un-rotated shared log is precisely what makes cross-attempt key collisions reachable.

Top 5 Priority Actions

  1. Fix the agent-agnostic factory crash first: src/coder_eval/orchestrator.py:1252 stamps cost_log_tags on route alone and src/coder_eval/agents/registry.py:163 forwards it verbatim into NoOpAgent/CodexAgent/AntigravityAgent/plugin constructors that do not accept it, so every such task dies with TypeError under API_BACKEND=litellm — gate on an agent capability ClassVar mirroring supports_cooperative_stop and add a regression test for a none-agent task on a LiteLLMRoute.
  2. Make the cost-log join key attempt-scoped: src/coder_eval/litellm_cost.py:91 joins only on (run_id, task_id, iteration) where run_id is a deterministic hash of a reusable --run-dir, so a re-run against the append-only $LITELLM_COST_LOG sums both attempts (reproduced: $0.06 billed for $0.03 of work), which can spuriously trip run_limits.max_usd and flip final_status — add an x-ce-attempt nonce plus a two-attempt test.
  3. Stop the join from overriding authoritative cost under partial coverage at src/coder_eval/litellm_cost.py:132-140: calls with a null usage.cost (every Bedrock-served model on the same proxy) are billed as $0 yet still replace the static estimate, and the all-unpriced case drains the entire static turn cost onto the reconciliation row (litellm_cost.py:233) while still logging success — only override when every call is priced, otherwise bail and warn.
  4. Restore the single-writer token invariant broken by the max(0, ...) clamp at src/coder_eval/litellm_cost.py:224-231: on the LiteLLM route the SDK reports cache_read=0 while the proxy reports the real value, so Σ(message buckets) no longer equals token_usage (measured 9116 vs 5020) and evalboard's selectTokenTotals displays inflated totals — pick one bucket owner, assert the invariant instead of clamping, and add tests for cache_read_input_tokens=0 plus the equal-output_tokens tie that mis-binds an aux call at litellm_cost.py:193.
  5. Close the wiring and observability gaps that make the whole feature no-op silently: LITELLM_COST_LOG is absent from the start-litellm.sh .env block (litellm/start-litellm.sh:90), .env.example, and the docker env allowlist (src/coder_eval/models/sandbox.py:235-238), and orchestrator.py:918 logs only on success — add the missing wiring, a warning when the route is LiteLLM but applied==0 or the join is partial, and a CI gate that actually runs the evalboard vitest suite and litellm/cost_logger.py coverage (plus the REPORT_SCHEMA.md/CLAUDE.md updates for the now-false "reconciliation carries no cost" contract).

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

CarlesUIPath and others added 6 commits July 30, 2026 15:52
…s non-Claude crash)

Review blocker (PR #66): `_create_agent` stamped `cost_log_tags` whenever the
route was LiteLLM, and `create_agent` forwards kwargs verbatim to the registered
agent class — but only `ClaudeCodeAgent.__init__` accepts the parameter. So every
NoOp / Codex / Antigravity / plugin task raised `TypeError:
__init__() got an unexpected keyword argument 'cost_log_tags'` under
API_BACKEND=litellm (agentless canary tasks are explicitly allowed credential-free,
so this is a reachable path).

Gate on agent capability instead of the route: add a `supports_cost_log_tags`
ClassVar on the `Agent` base (default False; True on ClaudeCodeAgent), mirroring
the existing `supports_cooperative_stop` pattern, and only forward the kwarg when
the resolved agent class sets it. Regression test builds a `none`-agent on a
LiteLLMRoute (constructs cleanly) and pins that forwarding the kwarg would crash.

Also clears two CodeQL alerts on the branch:
- litellm/cost_logger.py: add an explanatory comment inside the `_to_dict`
  best-effort `except` (py/empty-except).
- tests/test_litellm_cost.py: drop the duplicate `from ... import Orchestrator`
  and reference it via the existing `orch_mod` alias (py/import-and-import-from).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ransactional join

Address the cost-correctness review blockers (PR #66) in the actual-cost join:

- Token-bucket invariant (blocker): the join rewrote per-message cache-read from
  the proxy while the turn total kept the SDK's cache_read=0, then absorbed the
  disagreement with max(0, …) — breaking Σ(message buckets) == token_usage, which
  the evalboard's selectTokenTotals sums as truth. On a clean generation↔call bind
  the proxy is now authoritative for that turn: token_usage is recomputed from the
  summed calls so the residual is exactly zero (no clamp) AND the real per-call
  cache read still displays. On a bail (sub-agent orderings diverge) the SDK
  buckets are left untouched (invariant already holds) and only cost is attributed.

- Partial coverage (blocker): a turn's cost is overridden only when EVERY call is
  priced. A null-cost call (e.g. a Bedrock-served model on the same proxy) would
  otherwise be billed at $0 and understate the turn — now the turn keeps its static
  estimate, attaches no misleading breakdown, and warns naming the unpriced ids.
  This also fixes the cost-less turn that drained the whole static cost onto the
  reconciliation row.

- Transactional: the full plan is computed before any turn is mutated, so a
  malformed record (which raises while building the breakdown) aborts the whole
  join with the run untouched — matching the caller's "keeping static pricing".

- Observability: warn on orphaned cost-record iterations that match no turn.

Tests: cache_read=0 real-shape invariant; partial/all-unpriced fallback;
transactional no-mutation on a malformed record; the equal-output aux tie case
pinned as a documented display-only limitation (totals + invariant stay correct).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… no-match warning

- Rerun double-count (blocker): the join key was (run_id, task_id, iteration) with
  run_id a deterministic hash of --run-dir and the proxy log append-only, so
  re-running a task into the same run_dir re-matched a prior attempt's rows and
  summed both ($0.06 recorded for $0.03 of work). Stamp a per-Orchestrator
  x-ce-attempt nonce (uuid4) and filter the join on it, so a prior attempt's rows
  can't match this one. Regression test feeds two attempts' rows under identical
  keys and asserts a single attempt's cost.
- Single run-id accessor: the correlation run-id was derived by a duplicated
  hash_identifier(run_dir) expression at both the stamp and join sites; extract one
  `_cost_correlation_run_id` property used by both so they can't drift.
- Observability: warn when the route is LiteLLM and the join matched nothing
  (applied == 0) instead of silently reporting static pricing as authoritative.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ug scaffolding

Address the review's security/robustness nits + dead-code cleanup:

- Header injection (Axis 4): the correlation headers carry the author-defined
  task_id/variant_id, so a value with CR/LF would inject extra headers into every
  SDK->proxy request (forged cost attribution, or an auth/routing override against
  an operator-configurable base URL). _build_sdk_env now rejects non-single-line /
  non-ASCII cost_log_tags values at the seam.
- Non-finite cost (Axis 2): _num now rejects NaN/Inf — a non-finite usage.cost
  would serialize as a bare NaN token (invalid JSON for the whole task.json) and
  make the max_usd gate's cost>limit silently never fire.
- Dead code (Axis 1): remove the _debug_ids key-inventory dumper and its one-off
  CODER_EVAL_COST_DEBUG branch — the join-by-position decision it existed to inform
  is already made; the knob was documented nowhere.
- Test health (Axis 3): cover the hooks LiteLLM actually calls (log_success_event /
  async_log_success_event) + the never-break-the-proxy guard (append_record raising
  must not propagate) + a real-CustomLogger signature conformance check
  (importorskip); add a producer->JSONL->consumer round-trip pinning the wire
  contract; derive the OpenRouter set in the config shape-guard from model_list
  instead of a hardcoded literal.
- Docs: document cost_log_tags in both __init__ and _build_sdk_env Args blocks.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…liation-cost contract

Close the review's wiring + documentation gaps:

- LITELLM_COST_LOG was undocumented, so an operator following the printed
  instructions got settings.litellm_cost_log=None and silent static pricing (the
  exact bug the feature fixes). Add it to start-litellm.sh's printed .env block, to
  .env.example (with the new LiteLLM section), and to the docker env-passthrough
  allowlist (models/sandbox.py) — with a note that the file must also be
  bind-mounted for --driver docker runs to see it (follow-up).
- REPORT_SCHEMA.md (the cross-repo consumer contract) now lists
  TurnRecord.provider_call_costs and states that assistant/reconciliation messages
  carry a per-message cost_usd on the LiteLLM actual-cost path — the "reconciliation
  has no cost" invariant note was factually false after this feature.
- CLAUDE.md: add litellm_cost.py to the module map, list ProviderCallCost under
  telemetry.py, and correct the same reconciliation-carries-no-cost claim.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…config comment

Close the two remaining review nits:

- NIT 2: load_cost_records streamed the whole file into memory via
  read_text().splitlines(); read it line-by-line instead, and document that the
  shared log is append-only + un-rotated (scope LITELLM_COST_LOG per run if large).
- NIT 3: the 5-line usage.include comment was pasted verbatim above all three
  OpenRouter models; keep the rationale once in the shared OpenRouter header and
  leave a one-line pointer on each entry.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@bai-uipath bai-uipath left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One requested change: cut the distribution onto transcript messages. Everything else, fix what you agree with.

Keep

  • The capture and turn-level join, including the retry and rerun dedup. Proxy-side capture plus a post-run join on (run, task, attempt, iteration) is the only way to get these numbers, and that key is fully deterministic, so run totals are exact. Reconciling per-run totals against OpenRouter's billing dashboard is the standard of proof this should meet.
  • The per-call audit record on the turn, with the real cache buckets. This is what makes the dashboard cross-check possible, and it is the natural source for a per-call view later. Keep it even though the cut below removes its current consumer.
  • Dropping the OpenRouter models from the evalboard price table. Blank is honest; a headline rate the request never paid invites arithmetic. Worth keeping after the cut too.

The cut

  • The distribution onto transcript messages. It is the only part with no deterministic join key, so it binds generations to calls by output-token order, needs a bail path when that fails, and takes the four token buckets away from the SDK to hold the invariant. What it buys is a per-generation cost column that is sometimes silently wrong (your own tie-case test pins a row showing the aux call's price). Put it in the per-call breakdown you already persist, as a small table in a follow-up. Cutting it also removes the token double-count on the retry path, where the survivor absorbs the crashed sibling's calls while the sibling keeps its own buckets.

Fix what you agree with

  • Retry path loses cost when the survivor falls back to static. This one I'd fix regardless of the above. Non-survivor turns are zeroed before the all-priced check runs, so an unpriced call on the survivor leaves the sibling at $0 and the survivor on its static estimate: the run silently drops that spend while logging that static pricing was kept. Fix: decide the fallback during planning and don't zero a sibling whose survivor won't be credited.
  • The all-priced gate is too coarse. One degenerate no-usage call, which you've seen on GLM, reverts a whole turn to the headline rate, a larger error than billing that one call at $0. Fix: price the unpriced calls from the rate card and mark the turn blended, or distinguish "no usage reported" from "usage but no cost".
  • The provider allowlist probably wants allow_fallbacks: true. The only lists are the right fix, but false still sends each request to a single upstream, so a saturated provider in the set has nowhere to go and the original 429 hole stays open. That flag existed to keep the billed rate matching the static table, which this PR replaces with the real per-call cost, so it is now vestigial. Capturing OpenRouter's provider on the cost record while you're in there also answers the "we can't see which upstream served it" objection to fallbacks.

Minor: the tags would sit more naturally on the route than as a constructor kwarg plus a capability ClassVar; the evalboard comments reference a distributeActualCost that doesn't exist; the docker env var does nothing without the bind mount.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants