Uh oh!
There was an error while loading. Please reload this page.
fix(pricing): stop billing cached tokens twice - #427
Merged
Jason Robert (jrob5756) merged 3 commits intoAug 13, 2026
Conversation
`AgentOutput.input_tokens` is the total prompt and already contains `cache_read_tokens` / `cache_write_tokens`, but `calculate_cost` billed all four buckets additively — charging every cached token at the full input rate *and again* at the cache rate (11x on claude-sonnet-5). A long tool-calling agent re-reads nearly its whole prompt from cache each turn, so the error compounded: a real run reporting $51.08 actually cost about $8. Subtract the cache buckets from the input bucket before applying the input rate, so each physical token is priced exactly once — the same treatment genai-prices uses. Clamp rather than raise on inconsistent counts, since a cost annotation must never abort a workflow. Verified against a live Copilot session: inputTokens minus the two cache counters left just 2 genuinely-fresh tokens on turns 2 and 3, confirming the field is cache-inclusive. Bring claude-agent-sdk onto that one convention too. Its Anthropic-shaped usage dict reports cached tokens outside `input_tokens` and it surfaced neither cache bucket, so cached tokens there were billed at nothing at all; it now folds them into `input_tokens` and reports both buckets. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
`DEFAULT_PRICING` had no `claude-opus-5`, and `get_pricing`'s versioned-suffix fallback only extends a key with a `-` delimiter, so the SDK-advertised `claude-haiku-4.5` never matched the dashed `claude-haiku-4-5` entry. Both fell through to `None` and were reported as unpriced whenever the provider's live pricing hook was unavailable — an older Copilot SDK (whose `ModelBilling` has no `token_prices` field at all), or any non-Copilot provider. Rates are the published Anthropic ones and agree with what the Copilot SDK advertises for the same models. The remaining unpriced ids the SDK advertises (gpt-5.6-*, gemini-3.6-flash, grok-4.5, mai-code-*) are deliberately left out: this table holds vendor public rates, not Copilot's resale rates -- the two demonstrably differ (`gpt-5.5` is $2/$8 here against Copilot's $5/$30) -- and inventing numbers would be the confident-wrong answer the module's own fallbacks avoid. The pricing hook prices them exactly on Copilot. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Three blocking issues from the review of this PR. 1. The cache subtraction was unconditional, so a bucket with no published rate was removed from the input bucket and then priced at the 0.0 dataclass default -- billing cached tokens at nothing. 20 of the 48 DEFAULT_PRICING entries (every GPT, o-series and Gemini model) rely on that default while their providers do report cache counts, so gpt-5.5 on a 95%-cached prompt fell from $2.16 to $0.20. That is the same silent under-billing this PR removes, in the other direction. Each bucket is now only subtracted when a rate exists to charge it at, matching genai_prices.types.ModelPrice.calc_price, which gates its subtractions the same way. 2. The ResultMessage branch read four values from one usage dict under three different .get() default conventions. With input_tokens absent the fallback was the already-cache-inclusive running total and the buckets were added on top again (5300 -> 10500); with the cache keys absent, input_tokens dropped below the buckets it is contractually supposed to contain (100 < 5200). The three prompt figures are one snapshot and are now replaced together or not at all. Both usage sites parse through a shared _read_usage helper, since reading the keys inline at each site is what let the defaults drift apart. 3. Both CHANGELOG entries were filed under the already-released 0.1.28 section rather than [Unreleased], and would have been invisible when 0.1.29 was cut. Moved, and noted that existing limits.budget_usd values were calibrated against the inflated figures. The clamp is now audible: a provider whose cache counts exceed its own input_tokens has broken the contract AgentOutput.input_tokens declares, and a clamped cost is an ordinary float that flows past every unpriced guard and prints as a certainty. Warns once per model, following the _warn_fuzzy_match pattern already in this module. Mutation testing had shown five mutations surviving a green suite, including reverting the entire provider half of the fix on the path that determines billing in every completed run. Each is now caught: ResultMessage cache totals, multi-turn bucket accumulation, the partial/interrupt path, dotted-vs-dashed alias rate drift, and the write-only and combined clamp overflows. test_usage_tracker_record asserted only cost_usd > 0 and passed identically before and after the fix; it now pins the exact figure. Also corrects the docstring's claim to match genai-prices (it gates its subtractions and raises where conductor clamps), scopes the superseded FR5 notes in the claude-provider design doc, drops the past-tense bug narrative from the tracker comment, and removes the blank line that made the whole AGENTS.md Provider Parity list render loose. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
Jason Robert (jrob5756)
deleted the
fix/cost-double-counts-cache-tokens
branch
August 13, 2026 14:50
Jason Robert (jrob5756) pushed a commit
to lukeellison/conductor
that referenced
this pull request
Aug 17, 2026
Resolves two conflicts and one regression that only appears on merge. CHANGELOG.md: main released 0.1.28 through 0.1.32 since this branch last merged, so the section the entries sat under is now a shipped release. Moved both under the current, empty [Unreleased] rather than letting the textual merge file them under 0.1.31. tests/test_executor/test_agent.py: both sides appended an independent provider stub plus test class at the same point. Kept both; each stub carries its own validate_connection/close, which the textual merge had collapsed onto whichever class happened to come last. claude_agent_sdk.py auto-merged, but the reconciliation was the risky one and was checked by hand: main added _read_usage (microsoft#427, stop billing cached tokens twice) inside execute's message loop, and this branch moved that loop into _execute_session. Both call sites landed in the relocated function with the cache counters and last_call_input_tokens intact, and this branch's session-id capture still runs ahead of the interrupt check. skill_injection.max_bytes: 128KB -> 160KB. This is the regression neither side could see alone. The bundled conductor skill was ~117KB when the ceiling was chosen; this branch added ~1KB of session_key documentation and main added ~1.1KB of mid-run guidance documentation (microsoft#400), which together carry the rendered content to 131,923 bytes against a 131,072 limit. Eleven tests failed, and a claude or hermes agent enabling the shipped skill would have hard-failed rather than warned -- the exact inversion the two defaults exist to prevent. warn_bytes stays at 64KB so the combination still warns. The stale ~117KB figure is corrected in AGENTS.md, docs/workflow-syntax.md and the schema docstrings, along with one dependent claim that described 260KB as twice the default. Gates on the merged tree: ruff clean, ty clean, 7559 passed / 45 skipped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
AgentOutput.input_tokensis the total prompt and already containscache_read_tokens/cache_write_tokens.calculate_costbilled all four buckets additively, so every cached token was charged at the full input rate and again at the cache rate — 11x onclaude-sonnet-5.Verified against a live Copilot session (
claude-sonnet-5, 3 turns):inputTokenscacheReadTokenscacheWriteTokensinputTokens - (cacheRead + cacheWrite)leaves 2 tokens. The field is unambiguously cache-inclusive.Because an agentic loop re-reads nearly its whole prompt from cache every turn, the error compounds across turns. A real
shiprun reported $51.08; it actually cost about $8:Token counts were always correct — only the dollar conversion was wrong.
The fix
Subtract the cache buckets from the input bucket before applying the input rate, so each physical token is priced exactly once. This matches the reference treatment in
genai_prices.types.ModelPrice.calc_price(the library pydantic-ai itself prices with), which derivespriced_text_input_tokensthe same way.Inconsistent counts clamp at zero rather than raise — a cost annotation must never abort a running workflow. (
genai-pricesraises here; conductor cannot afford to.)Provider parity
claude-agent-sdkhad the opposite bug. Anthropic's usage dict reports cached tokens outsideinput_tokens, and the provider surfaced neither cache bucket — so cached tokens there were billed at nothing at all. It now folds them intoinput_tokensand reports both buckets, putting every provider on one convention:Documented on
AgentOutput.input_tokensand in the AGENTS.md provider-parity section.copilotandclaude(pydantic-ai) already satisfied it natively;hermes/acareport no cache fields and are unaffected.Second commit: unpriced models
claude-opus-5had no table entry, andget_pricing's versioned-suffix fallback only extends a key with a-delimiter — so the SDK-advertisedclaude-haiku-4.5never matched the dashedclaude-haiku-4-5entry either. Both priced asNonewhenever the live pricing hook was unavailable (an older Copilot SDK whoseModelBillinghas notoken_pricesfield at all, or a non-Copilot provider). Addedclaude-opus-5,claude-opus-4.5,claude-sonnet-4.5,claude-haiku-4.5at published Anthropic rates.The other unpriced ids the SDK advertises (
gpt-5.6-*,gemini-3.6-flash,grok-4.5,mai-code-*) are deliberately left out: this table holds vendor public rates, not Copilot's resale rates — the two demonstrably differ (gpt-5.5is $2/$8 here vs Copilot's $5/$30) — so inventing numbers would be exactly the confident-wrong answer the module's fallbacks are written to avoid. The pricing hook prices them exactly on Copilot.Impact
Cost figures on the dashboard, CLI summary,
agent_completedevents and the JSONL event log all drop to their true values. No workflow config changes. Token counts, budgets and the context-window bar are untouched.Tests
test_cache_tokens_are_not_billed_twice— a fully-cached prompt costs the cache rate, not input + cache.test_calculate_cost_clamps_inconsistent_cache_counts— no negative input bucket.test_billing_totals_are_cache_inclusive_and_priced_once— end-to-end through theclaude-agent-sdkprovider intocalculate_cost.test_dotted_claude_names_are_priced— guards the alias-gap class of bug.test_calculate_cost_with_cache_tokens, which encoded the old additive behaviour.Full suite: 6444 passed, 24 skipped. Lint and format clean;
make typecheckreports 3 pre-existingunused-ignore-commentwarnings (confirmed present onmainwith these changes stashed).