feat: use provider-reported cost (usage.cost) for accurate cost tracking - #9719
feat: use provider-reported cost (usage.cost) for accurate cost tracking#9719henrydeclety wants to merge 2 commits into
Conversation
When a provider or gateway includes a `cost` field (USD) in its `usage` object — as OpenRouter does, and as an Anthropic-compatible gateway can — use it as the turn's cost instead of computing it from the local pricing catalog. The provider knows the real cost of the model/route it actually served (including failover and cache discounts), which a static catalog keyed by model id cannot — and custom or self-hosted model ids aren't in the catalog at all, so today they surface as "Pricing data unavailable". - Add optional `cost: Option<f64>` to `Usage`, parsed from `usage.cost` in both the Anthropic and OpenAI-compatible `get_usage` paths. It merges by taking the latest non-None value (the total carried on the final message_delta), not by summing. - `accumulate_cost` prefers the provider-reported cost when present, falling back to the catalog otherwise. - Desktop CostTracker: a provider-reported cost now wins the tooltip too, so a catalog miss no longer reads "Pricing data unavailable" when the real cost is known. No generated-type changes — the value flows through the existing TokenState.accumulatedCost. Adds unit tests for the cost field + merge semantics. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Henry Declety <henry.declety@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0cc38207c4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| Some(total_tokens_i32), | ||
| )) | ||
| ) | ||
| .with_cost(data.get("cost").and_then(|v| v.as_f64()))) |
There was a problem hiding this comment.
Preserve provider cost when merging Anthropic deltas
When an Anthropic-compatible streaming response has the normal message_start usage first and then reports usage.cost on a later message_delta, this parsed cost is dropped before it reaches accumulate_cost: the EVENT_MESSAGE_DELTA path rebuilds merged_usage with Usage::new(...) and never carries delta_usage.cost (see the merge around lines 877-888). As a result, the new provider-reported cost support does not work for the streaming Anthropic-compatible gateway case described here unless there was no prior message_start usage.
Useful? React with 👍 / 👎.
…rustfmt - anthropic.rs: the EVENT_MESSAGE_DELTA merge rebuilt usage with Usage::new(...) and dropped the parsed `cost`, so a provider-reported cost arriving on the message_delta (after a message_start usage) was lost before accumulate_cost. Carry it through with `.with_cost(delta_usage.cost.or(existing.cost))`. (Thanks to the Codex review for catching this.) - token_usage.rs: apply rustfmt formatting to the new test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Henry Declety <henry.declety@gmail.com>
|
Thanks for the reviews. Addressed in 3b942bb:
|
kimnamu
left a comment
There was a problem hiding this comment.
Thanks for this, Henry — provider-reported cost is the right fix for gateway/custom-id routing, and gating on usage.cost (USD) keeps it fully backward-compatible. I'm not a maintainer, just another Bedrock/provider user who read this carefully and ran the branch locally. Two things I checked specifically, both look correct:
1. Streaming merge no longer doubles or drops the cost (the codex P2 you already fixed in 3b942bb). I confirmed the fix works end-to-end at the SSE layer, not just at the Usage::add unit level. Feeding a real message_start (input only, no cost) + final message_delta (output + cost: 0.0123) through response_to_streaming_message yields:
cost=Some(0.0123) input=Some(100) output=Some(50)
i.e. the single delta total — not doubled (0.0246) and not lost (None). To prove the test actually catches the bug, I reverted just the .with_cost(delta_usage.cost.or(existing_usage.usage.cost)) line and re-ran:
assertion `left == right` failed: cost should be the delta total, not doubled/lost
So the streaming path is correct on 3b942bb.
2. No double-counting across the session accumulation either. accumulate_cost runs once per response (via update_session_metrics) with the final merged usage, and the provider branch existing + provider_cost is symmetric with the catalog branch existing + chunk_cost — both add one response's total exactly once, matching how tokens already accumulate. So mixing catalog-priced and provider-priced responses in one session stays consistent.
One non-blocking suggestion: the streaming-merge fix is the subtle part, and right now it's only covered by the Usage::add unit test (test_cost_merge_takes_latest_non_none_not_sum) — which exercises the operator, not the actual EVENT_MESSAGE_DELTA code path that rebuilds usage via Usage::new(...).with_cost(...). A regression here (someone dropping .with_cost again) would slip past the existing test. A small SSE-level test would lock it in. Here's one that passes on 3b942bb and fails if the .with_cost(...) is removed — happy for you to drop it into mod tests in crates/goose/src/providers/formats/anthropic.rs (it reuses the same response_to_streaming_message entrypoint the other streaming tests use):
#[tokio::test]
async fn test_streaming_preserves_provider_cost_from_delta() {
use futures::StreamExt;
// message_start has input tokens but no cost; the final message_delta
// carries the output tokens AND the provider-reported total cost.
let events = concat!(
r#"data: {"type":"message_start","message":{"id":"m1","role":"assistant","content":[],"model":"glm-4.7","usage":{"input_tokens":100,"output_tokens":0}}}"#,
"\n",
r#"data: {"type":"content_block_start","index":0,"content_block":{"type":"text","text":""}}"#,
"\n",
r#"data: {"type":"content_block_delta","index":0,"delta":{"type":"text_delta","text":"hi"}}"#,
"\n",
r#"data: {"type":"content_block_stop","index":0}"#,
"\n",
r#"data: {"type":"message_delta","delta":{"stop_reason":"end_turn"},"usage":{"output_tokens":50,"cost":0.0123}}"#,
"\n",
r#"data: {"type":"message_stop"}"#,
);
let lines: Vec<Result<String, anyhow::Error>> =
events.lines().map(|l| Ok(l.to_string())).collect();
let stream = Box::pin(futures::stream::iter(lines));
let mut msg_stream = std::pin::pin!(response_to_streaming_message(stream));
let mut final_usage = None;
while let Some(Ok((_msg, usage))) = msg_stream.next().await {
if usage.is_some() {
final_usage = usage;
}
}
let usage = final_usage.expect("stream should yield final usage");
// The provider cost from the delta survives the message_start merge,
// and is the single total — not doubled, not dropped.
assert_eq!(usage.usage.cost, Some(0.0123));
assert_eq!(usage.usage.input_tokens, Some(100));
assert_eq!(usage.usage.output_tokens, Some(50));
}Everything else — #[serde(skip_serializing_if)] keeping cost out of JSON when absent, the CostTracker.tsx change preferring the authoritative provider cost over "Pricing data unavailable" while suppressing the misleading $0.000000 per-token breakdown when the catalog has no prices — reads clean and well-scoped. I'd love to see it land.
Reviewed with the help of an AI agent (Claude Code); I ran the branch and verified the outputs above myself.
|
Thanks for this, and thanks @kimnamu for verifying the branch end-to-end. I dug into the design and I think the current shape is narrower than where I would like this to land, so I want to flag the direction before we go further. Right now The design I would like instead:
This also resolves the model-switching case cleanly: if a session uses an OpenRouter model that reports cost and later a direct model that only has catalog pricing, each chunk just records its own best estimate from whichever source applies, and the total falls out as the sum. So the catalog computation wants to move out of This is a bigger change than the current PR, so I would like to align on it before you invest more. Happy to discuss the approach here. Snoozing for a few days to give you a chance to weigh in. |
|
Hi @henrydeclety, and thank you for this — provider-reported cost is genuinely useful, and I appreciate you jumping on the codex feedback so quickly. As I mentioned in my earlier comment, I'd like this to go in as a slightly broader change rather than special-casing the cost in the accumulator: every chunk should carry its best cost estimate (provider-reported when available, catalog-derived otherwise), filled in where Since we haven't heard back on that direction, I'm going to close this for now to keep the queue tidy. This isn't a "no" to the feature at all — if you'd like to take on the redesign (or want to discuss it further), please reopen this or open a new PR and we'll pick it right back up. Thanks again! |
Summary
Today goose computes session cost as
tokens × catalog_price, looking the price up from the canonical (models.dev) catalog by(provider, model). For any model id that isn't in that catalog — custom or self-hosted ids, or anything served through a gateway — the lookup misses and the cost tracker shows "Pricing data unavailable for {model}", even though the gateway knows the exact cost.This change lets goose use a provider-reported cost when one is present: if the provider/gateway includes a
costfield (USD) in itsusageobject — exactly as OpenRouter already does (usage.cost), and as an Anthropic-compatible gateway can — goose uses that number. It reflects the model/route actually served (including failover and cache discounts), which a static catalog keyed by model id structurally can't.It's fully backward-compatible and provider-agnostic:
cost: Option<f64>toUsage(#[serde(default, skip_serializing_if = ...)]), parsed fromusage.costin both the Anthropic (providers/formats/anthropic.rs) and OpenAI-compatible (formats/openai.rs)get_usagepaths. On merge it takes the latest non-Nonevalue (the total carried on the finalmessage_delta), not a sum.Agent::accumulate_costprefers the provider-reported cost when present and falls back to the existing catalog computation otherwise.CostTracker: a provider-reported cost now also wins the tooltip, so a catalog miss no longer reads "Pricing data unavailable" when the real cost is known. No generated-type changes — the value flows through the existingTokenState.accumulatedCost.Testing
token_usage.rs:costdefaults toNoneand is omitted from JSON when absent; deserializes from ausagethat includescost; and the merge takes the latest non-Nonecost rather than summing (somessage_start+ finalmessage_deltayields the total).get_usage→ProviderUsage→accumulate_cost→session.accumulated_cost→TokenState.accumulatedCost→CostTracker.cargo test/pnpmlocally — relying on CI to validate the build/tests. The change is small and mechanical; happy to iterate on any CI feedback.Related Issues
Relates to gateway/custom-endpoint cost tracking (e.g. routing goose through an Anthropic-compatible proxy or OpenRouter-style gateway, where the cost is known per-request but the model id isn't in the public catalog).
🤖 Generated with Claude Code