feat(litellm): actual per-call cost + cache accounting for the open-weight backend - #66
feat(litellm): actual per-call cost + cache accounting for the open-weight backend#66CarlesUIPath wants to merge 10 commits into
Conversation
…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>
🔍 Reviewing PR: feat(litellm): actual per-call cost + cache accounting for the open-weight backendTodo List:
|
…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
left a comment
There was a problem hiding this comment.
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
- [Axis 1] LiteLLM-only
cost_log_tagskwarg is passed through the agent-agnosticcreate_agentfactory, so every non-Claude agent raises TypeError under the LiteLLM backend (and the branch is untested) (src/coder_eval/orchestrator.py:1252) —_create_agentdoesif isinstance(self.route, LiteLLMRoute): kwargs["cost_log_tags"] = {...}(orchestrator.py:1252-1256) and thenreturn 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 onlyClaudeCodeAgent.__init__grew the parameter —NoOpAgent,CodexAgent(codex_agent.py:642) andAntigravityAgent(antigravity_agent.py:192) have no such kwarg and no**kwargs, andcreate_agentforwards 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 (whichSettings.validate_api_keysexplicitly allows credential-free, config.py:231) and every codex/antigravity task fails to construct underAPI_BACKEND=litellm. Gate on agent capability rather than route: add asupports_cost_log_tagsClassVar onAgent(mirroring the existingsupports_cooperative_stopat claude_code_agent.py:657) and only pass the kwarg when it is set, or accept**_: Anyon the base constructors. Add a regression test building anone-agent task under aLiteLLMRoute. - [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 acrossTurnRecord.messages(assistant + reconciliation) equalstoken_usageexactly", "booked at the singleEventCollectorseam"._distribute_onto_messagesnow 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
- [Axis 1]
provider_call_costsis a write-only field and three comments point readers atdistributeActualCost/ 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 distributeActualCostreturns 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 evalboardreturns only pricing.ts:57, so the newly persistedTurnRecord.provider_call_costshas no dashboard consumer, contradicting litellm_cost.py:11-12 ("attached asTurnRecord.provider_call_costs(for the evalboard's per-call cache/cost view)"). Delete thedistributeActualCostreferences, reword to what shipped (Python-side join surfacing inline asMessageEvent.costUsd), fix the litellm_cost.py docstring, and either wire a consumer forprovider_call_costsor say in models/results.py:354 that it is an audit-only record. Also attach the orphaned block at runs.ts:278-283 toMessageEntry.cost_usd(runs.ts:1201) instead of leaving it floating between two interfaces. - [Axis 1] Investigation scaffolding shipped in the proxy callback: dead
_debug_idsplus the undocumented one-offCODER_EVAL_COST_DEBUGbranch (litellm/cost_logger.py:223) — The helper's own docstring (cost_logger.py:225-228) says it was "Used once to determine whether the Anthropicmsg_...message id ... is reachable in the callback, or only OpenRouter'sgen-...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_DEBUGhits only cost_logger.py:205 and its own test — notSettings(config.py), notlitellm/start-litellm.sh, notdocs/. Delete_debug_ids(lines 223-246) plus therecord["_debug"] = _debug_ids(...)branch (lines 205-210) and its test; if a hook is still wanted, use a one-linelogger.debug(...)and document the variable next toLITELLM_COST_LOGin start-litellm.sh. - [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 recomputesrun_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 inrun_dirmakesapply_actual_costreturn 0 at litellm_cost.py:91-93 and every turn silently reverts to the static rate-card estimate — nothing warns, becauseif 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 butapplied == 0. - [Axis 2]
turn: Anyon_distribute_onto_messagesdisables 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 toAny, soturn.messages,turn.token_usage, and the subsequentusage.uncached_input_tokens/usage.cache_read_input_tokens/usage.cache_creation_input_tokens/usage.output_tokensreads (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), soTurnRecordcan be imported the same way (or under the existingTYPE_CHECKINGblock withEvaluationResult), and the call site at line 139 already has a properly typedTurnRecord. This matters concretely becauseTokenUsage's prompt buckets were recently renamed/split (uncached_input_tokensplus the derivedinput_tokenscomputed field, telemetry.py:52-85, with a_adopt_legacy_input_tokensshim for the old name) — a further rename would fail silently here at runtime instead of atmake typecheck. Change the signature toturn: TurnRecord. - [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, somake verifynever sees it):uv run coverage run --source=litellm -m pytest tests/test_litellm_cost_logger.py -n 0→litellm/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.yamlregisterscallbacks: cost_logger.proxy_handler_instance), and no test invokes either; every test calls_emitdirectly.
(b) Lines 212-214 (except Exception:/pass) are uncovered even thoughtest_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 ownlitellm/directory shadows the pip package, sofrom litellm.integrations.custom_logger import CustomLoggerat line 42 always falls through to the# pragma: no coverstub at lines 45-46. Nothing therefore verifies the hook names/signatures against the real LiteLLMCustomLoggerABC: 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 callproxy_handler_instance.log_success_event(...)/await ...async_log_success_event(...)with the full 4-arg signature, one that forces_emitto raise (e.g. monkeypatchappend_recordto throw) and asserts no propagation, and either addlitellm/to the coverage source or add a conformance assertion (hasattr/inspect.signature) that skips when the real litellm is absent. - [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-195binds 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) whoseoutputhappens 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.dumps → load_cost_records → apply_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
- [Axis 1]
cost_log_tagsomitted from both docstringArgs: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) documentsconfig,route,instance_name,extra_mcp_serversbut not the newly addedcost_log_tags(line 666);_build_sdk_env's docstring (lines 741-756) documentsroute,path_prepend,plugin_tools_dirbut notcost_log_tags(line 739). Add anArgs:entry to both — the required key set (x-ce-run-id/x-ce-task-id, withx-ce-iterationappended per turn) is otherwise only discoverable from the orchestrator. The inline comment at lines 687-691 already has the wording. - [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_costcallsload_cost_records(settings.litellm_cost_log)once per task (orchestrator.py:916), andload_cost_recordsdoesp.read_text(encoding="utf-8").splitlines()over the entire file (litellm_cost.py:46) beforeapply_actual_costfilters to this run/task (litellm_cost.py:91).start-litellm.sh:61defaults to one shared$REPO_ROOT/tmp/litellm-costs.jsonlthatappend_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 defaultLITELLM_COST_LOGto a run-scoped filename, and state the rotation expectation in the module docstring. - [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) + cacheis 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. - [Axis 2]
_numaccepts non-finite floats, so a NaN/Inf provider cost propagates intototal_cost_usdand makes task.json invalid JSON (litellm/cost_logger.py:55) — The numeric filter isreturn value if isinstance(value, int | float) and not isinstance(value, bool) else None— it deliberately rejectsbooland strings but notnan/inf. A non-finiteusage.costtherefore survives into the JSONL (json.dumps({'cost': float('nan')})emits the non-standard bare{"cost": NaN}, whichjson.loadsaccepts on the read side), is accepted byProviderCallCost.cost_usd: float | None(models/telemetry.py:130 — pydantic'sallow_inf_nandefaults to True; verified by execution), and is then summed intoturn.token_usage.total_cost_usdat litellm_cost.py:133-138. Downstream,nan > limitis always False so therun_limits.max_usdgate silently never fires, and the persistedtask.jsoncontains a bareNaNtoken thatJSON.parsein the evalboard rejects for the whole file. Cheap fix at the boundary:import mathand returnvalue 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. - [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:826builds the header block by raw f-string join, with no validation ofkorv:
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.
Review: coder_eval — pr:66 (21 files) axis:1,2,3,4,5,6,7,8 — continued (2/2)What's MissingParallel paths:
Tests:
Downstream consumers:
Display & mapping dicts:
Daily/nightly:
Harness & Lint ImprovementsStatic checks (lint / type):
Harness improvements (not statically reachable):
Top 5 Priority Actions
Stats: 0 🔴 · 5 🟠 · 16 🟡 · 5 🔵 across 8 axes reviewed. |
…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
left a comment
There was a problem hiding this comment.
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. Theonlylists are the right fix, butfalsestill 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'sprovideron 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.

Summary
The Claude binary's Anthropic transport drops OpenRouter's real
usage.costandper-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.
runs/2026-07-29_17-22-05) — 5/5 tasks clean.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).
runs/2026-07-30_11-19-06) — run ≈ $1.04. Exercises theaccounting under stress: it stayed exact on 2
FAILUREtasks and 1MAX_TURNS_EXHAUSTEDtask (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.costis the amount actually billed, not anestimate. 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.
The three seams
claude_code_agentstampsANTHROPIC_CUSTOM_HEADERSwithx-ce-run-id/x-ce-task-id/x-ce-iteration(LiteLLM route only).litellm/cost_logger.py(a LiteLLM success callback) reads eachcall's real
usage.cost+ cache buckets (OpenRouter'susage.cost, notLiteLLM's
response_cost, which is0.0for models it doesn't price) andappends one JSONL record per call to
$LITELLM_COST_LOG.src/coder_eval/litellm_cost.pyoverrides each turn's cost with thesummed 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_usageholds.Changes
litellm/cost_logger.py(proxy callback, self-contained — nocoder_evalimport so it runs in the proxy's isolated env),src/coder_eval/litellm_cost.py(the join).ProviderCallCost;TurnRecord.provider_call_costs;cost_usdonAssistantMessage/ReconciliationMessage(join-populated)._create_agent;_join_litellm_actual_cost()runs before token aggregation.Settings.litellm_cost_log(LITELLM_COST_LOG);litellm-config.yamlgains
usage.include: trueper model +callbacks: cost_logger.proxy_handler_instance;start-litellm.shexportsLITELLM_COST_LOG+PYTHONPATH.cost_usdover 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.yamlpreviously had noprovider.onlylist, sosort: price+allow_fallbacks: falsepinned the single global-cheapestOpenRouter 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.onlyallowlist of upstreamswe've observed to be reliable (tried cheapest-first via
sort: price), and keepsallow_fallbacks: falseso routing can't silently substitute a provider outsidethat 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.pybooks the realusage.cost. The threesweeps above ran on these provider sets. Committed separately as
fix(litellm): ….Backend safety
Gated to
LiteLLMRoute+LITELLM_COST_LOG.cost_usd/provider_call_costsdefault to
None/[], so Claude, Bedrock, and Codex fall back to the staticrate 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; retryno-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 usageextraction; tag extraction; append; the emit path; debug capture.
tests/test_litellm_route.py,tests/test_event_collector.py— route wiring +field-parity.
messageActualCost.test.ts(actual-over-static preference), plusupdated
parseMessages/pricing-parity/pricingsuites.(166 rules) + pyright green (bar the optional
openai_codeximport andenv-only live tests).
Known limitations (by design)
the callback only sees OpenRouter's
gen-…id; the Anthropicmsg_…transcriptid 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.
Reconciliation-row residual tokens carry
$0(they're a counting artifactalready paid for in the per-call costs — re-pricing them at the rate card would
overstate vs. the bill).
GLM) shows a blank price on that one timeline row; it contributes
$0, so thetotal stays exact.
join runs at run time), and coder_eval's
.envLITELLM_COST_LOGmust point atthe same path the proxy writes, or the join is skipped and static pricing is
used.
model to a curated
onlyset of reliable providers (withallow_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 outremaining 429 behavior and tune the provider sets.
Out of scope