[#5540] fix(api): price cached input tokens, and stop losing costs to a missing model - #5714
[#5540] fix(api): price cached input tokens, and stop losing costs to a missing model#5714mmabrouk wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Railway Preview Environment
|
…ng model Three defects in cost estimation, all in `calculate_costs` and its roll-up guard. Cached input tokens were priced at the full uncached rate. `cost_per_token` never received the cache buckets. One measured span with 1 uncached prompt token, 25,182 cache-read tokens and 20 completion tokens was priced at $0.000303 against a harness-reported $0.0082, a 27x undercount. That is issue #5540. The fix passes the cache buckets and an inclusive prompt count, because litellm derives ordinary input by subtracting the cache details from whatever prompt count it is given. Reproduced against the pinned litellm 1.92.0 with 93 uncached and 13,463 cache-read tokens: 93, no cache arguments codex $0.00016275 sonnet $0.000279 93, plus cache arguments codex $0.002356025 sonnet $0.0040389 13,556 inclusive, plus cache codex $0.002518775 sonnet $0.0043179 The middle row silently loses the 93 ordinary tokens. Only the last row prices both kinds of input. Today's `prompt` bucket holds raw exclusive input, so summing the cache buckets into it is correct; the comment at the summation names the planned canonical change that would make that sum a double count. The model lookup never fell back to `ag.meta.request.model`. The runner sets the response model only for codex, so estimation was dead for every other agent span: 0 of 8,365 chat spans without a response model were priced, and 858 of 858 with one were. The lookup now tries response model, then request model, then the legacy parameters path. A reported cost of zero was treated as missing. The guard that keeps the roll-up from overwriting a producer-reported cost tested for a non-zero value, and the reader it used defaults every field to zero, so a genuine measured zero was indistinguishable from an absent attribute and got overwritten by an estimate. The guard now tests whether the attribute is present. Verification: 1,631 passing in OSS and 1,890 in EE. 23 new tests, including the pricing oracle parametrized over four cache shapes and two providers, the model lookup precedence, and containment of an unpriceable alias. Mutation checked by running the new tests against the previous version of the module: 9 fail, which confirms they pin the fix rather than the framework. Falling back to the request model starts feeding unpriceable aliases such as the bare string "sonnet" into litellm, where before it fed nothing. That failure stays contained: one warning, no cost written, no exception escaping into ingest. Claude-Session: https://claude.ai/code/session_01RkWWQUNNzRbaB5jnCAdjYA
9b06f03 to
d7c3adc
Compare
Closes #5540.
Stacked on #5709. Set the base to that branch, so this diff shows only its own change.
The symptom
Three separate defects in how Agenta estimates the cost of a model call.
Cached input is charged at the full uncached rate. One measured span with 1 uncached prompt token, 25,182 cache-read tokens and 20 completion tokens was priced at $0.000303 against a harness-reported $0.0082. That is a 27x undercount, and it grows with every warm turn.
Estimation is dead for most agent spans. The model lookup checks the response model and a legacy parameters path. The agent runner sets the response model only for codex. Measured: 0 of 8,365 chat spans without a response model were priced, and 858 of 858 with one were. A perfect discriminator.
A reported cost of zero is treated as missing. The guard that stops the roll-up overwriting a producer-reported cost tested for a non-zero value, and the reader it used defaults every field to
0.0. So a genuine measured zero, such as a fully cached turn or a free model, was indistinguishable from an absent attribute and got replaced by an estimate.The fix, and why the token math looks the way it does
Litellm derives ordinary input by subtracting the cache details from whatever prompt count you hand it. So it wants the inclusive figure, not the uncached remainder. Passing the uncached count plus the cache arguments silently drops the ordinary tokens.
Reproduced against the pinned litellm 1.92.0, with 93 uncached and 13,463 cache-read tokens:
gpt-5.3-codexclaude-sonnet-4-6The middle row loses the 93 ordinary tokens. Only the last row prices both kinds of input, and it holds on both the OpenAI and the Anthropic pricing routes.
One assumption, written into the code. Today's
promptbucket holds raw exclusive input, which is why summing the cache buckets into it is correct. A planned canonical change would make input inclusive at the boundary, at which point the same sum would double count. The comment at the summation names that condition, and the arithmetic sits in one place so the future migration has one thing to change. No heuristic tries to detect which convention is in force.Model lookup now tries the response model, then
ag.meta.request.model, then the legacy parameters path.The guard now tests whether the cumulative total attribute is present, not whether it is non-zero.
Verification
HEADunder the production module name: 9 fail. They pin the fix rather than the framework. The one cache-creation case that passes on the old code does so legitimately, because OpenAI has no separate cache-write price and both shapes agree there.Notes for the reviewer
"sonnet"that the model picker sends. Verified that the failure stays contained:litellm.BadRequestErroris caught by the existing handler, one warning is logged, no cost is written, no exception escapes into ingest. Litellm also prints its own provider banner to stderr on each such failure, which is new noise at comparable volume.cache_creation_input_tokensto a single write rate and the runner emits a single cache-write number, so long-lived cache writes are still undercounted.cost_per_tokenwith a fixed three-argument signature that rejected the new cache keywords.Follow-ups this does not do
calculate_costsassumes several attribute containers are dictionaries. A scalar there raises outside thetryand would fail the whole ingest batch. Pre-existing.