Repository navigation
Conversation
Output costs came out high on Anthropic reasoning requests. In costs.ts the completionIncludesReasoning allow-list covered Google and the OpenAI-style Responses providers but not anthropic/vertex-anthropic, so those fell through to completionTokens + reasoningTokens — adding reasoning that is already inside completionTokens. extract-token-usage reads Anthropic's output_tokens straight into completionTokens and builds totalTokens as prompt + output without re-adding reasoning; its spec pins this (51 in, 136 out incl. 31 reasoning = 187). The extract layer and the cost layer disagreed; this lines them up. Verified against real upstream (Vertex Anthropic, claude-sonnet-4-5): raw usage returned output_tokens 72 with thinking_tokens 65 for a one-token text answer, confirming output_tokens includes thinking. Before the fix a live request billed 179 output tokens for 93 completion tokens; after, output cost is exactly completionTokens * price. aws-bedrock bundles reasoning into outputTokens the same way and has no separate reasoning source, so it is already correct. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G6eFuuNRtA7bogrRtAHAYV
The reported total_tokens added reasoning on top of a completion count that, for several providers, already contained it. A live Vertex Anthropic request came back as prompt 64 + completion 135 + reasoning 78 = 277, when completion already included the reasoning and the total should have been 199. This contradicted extract-token-usage, whose spec pins the Anthropic total at 51 + 136 = 187 rather than 218. The extract layer was right and every downstream total-tokens site re-added reasoning behind its back. Extract the provider predicate the cost path already used into an exported completionIncludesReasoning(), add sumTotalTokens() alongside it, and route every total-tokens site through it: the five in chat.ts (cached streaming log, streaming fallback, streaming usage chunk, non-streaming prompt-delta recompute, non-streaming response), the cached-response log entry, the three parse-provider-response fallbacks, and the transform-response-to-openai fallback. This affects every provider whose completion count already includes reasoning — Anthropic, Vertex Anthropic, Google, OpenAI, Azure, RanoAI, Sakana, Meta, aws-mantle — not just Anthropic. Billing was already correct; this only fixes the totals reported back to callers. The chat-basic e2e asserted the old formula, so it encoded the bug; it now asserts the same invariant via sumTotalTokens. Verified against real upstream (Vertex Anthropic, claude-sonnet-4-5): non-streaming 66 + 101 (72 reasoning) now totals 167 instead of 239, and streaming 65 + 50 (43 reasoning) totals 115 instead of 158, with output cost still exactly completionTokens * price. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G6eFuuNRtA7bogrRtAHAYV
Probed all 25 providers with credentials against live upstream: a completion count that already contains reasoning is the near-universal convention, not the exception. Adds the 14 confirmed providers to completionIncludesReasoning. Canopywave is the one actively mis-billed: it reports reasoning_tokens at the top level of usage, which is the only shape extract-token-usage reads for OpenAI-format providers, so its reasoning reached the cost engine and was billed twice (92% over) and inflated streaming total_tokens. xAI stays off the list — grok-4 answers "9" with completion_tokens 1, reasoning_tokens 118 and total_tokens 324, so its completion genuinely excludes reasoning. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
🚧 Files skipped from review as they are similar to previous changes (4)
Included review availability: Your plan includes up to 4 reviews per rolling hour; 3 remain after this review. WalkthroughThe gateway now applies provider-specific reasoning-token rules to billing, storage costs, response usage normalization, and cached, streaming, and non-streaming chat totals. Documentation and tests cover inclusive and additive provider behavior. ChangesReasoning-token accounting
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to The change prevents reasoning tokens from being counted twice for providers whose output count already includes them while preserving additive behavior for providers that report reasoning separately; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant Provider as Provider response
participant Transform as Response transformer
participant Chat as chat.ts
participant Costs as costs.ts
participant Logs as logs.ts
Provider->>Transform: return usage fields
Transform->>Costs: sumTotalTokens(provider, prompt, completion, reasoning)
Costs-->>Transform: normalized total tokens
Transform->>Chat: return provider-aware usage
Chat->>Logs: calculateDataStorageCost(..., provider)
Logs->>Costs: classify completion reasoning inclusion
Costs-->>Logs: adjusted reasoning token count
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/gateway/src/lib/costs.spec.ts (1)
2218-2229: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover every reasoning-inclusive provider ID.
The direct test covers four of the eleven inclusive IDs. Add cases for
glacier,iceberg,google-vertex,quartz,sakana,meta, andranoai. A classification regression for one of these providers can restore double billing without failing this test.Proposed test structure
+ const inclusiveProviders = [ + "google-ai-studio", + "glacier", + "iceberg", + "google-vertex", + "quartz", + "openai", + "azure", + "sakana", + "meta", + "ranoai", + "aws-mantle", + ]; + it("treats OpenAI-style Responses API providers as reasoning-inclusive", () => { - expect(completionTokensIncludeReasoning("openai")).toBe(true); - expect(completionTokensIncludeReasoning("azure")).toBe(true); - expect(completionTokensIncludeReasoning("aws-mantle")).toBe(true); - expect(completionTokensIncludeReasoning("google-ai-studio")).toBe(true); + for (const provider of inclusiveProviders) { + expect(completionTokensIncludeReasoning(provider)).toBe(true); + } });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/gateway/src/lib/costs.spec.ts` around lines 2218 - 2229, Add assertions in the “reasoning-inclusive” test within completionTokensIncludeReasoning to cover glacier, iceberg, google-vertex, quartz, sakana, meta, and ranoai, ensuring each provider ID returns true alongside the existing inclusive cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/docs/content/features/data-retention.mdx`:
- Around line 43-44: Update the documentation text near the reasoning-token
billing explanation to remove the hardcoded provider names and use
provider-agnostic wording based on each provider’s usage contract. Explicitly
state whether the example assumes output tokens already include reasoning
tokens, and preserve the rule that separately reported reasoning tokens are only
added when excluded from the output count.
---
Nitpick comments:
In `@apps/gateway/src/lib/costs.spec.ts`:
- Around line 2218-2229: Add assertions in the “reasoning-inclusive” test within
completionTokensIncludeReasoning to cover glacier, iceberg, google-vertex,
quartz, sakana, meta, and ranoai, ensuring each provider ID returns true
alongside the existing inclusive cases.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: afe92682-24bb-4eec-95d7-4ae18105bc73
📒 Files selected for processing (6)
apps/docs/content/features/data-retention.mdxapps/gateway/src/chat/chat.tsapps/gateway/src/lib/costs.spec.tsapps/gateway/src/lib/costs.tsapps/gateway/src/lib/logs.spec.tsapps/gateway/src/lib/logs.ts
| Most providers (including OpenAI, Azure, and Google) already count reasoning tokens inside the output token count, so each token is billed exactly once. Reasoning tokens are only added separately for providers that report them outside the output count. | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the hardcoded provider list and clarify the example.
This documentation hardcodes a provider catalogue, which violates the documentation guideline and can become stale. Use provider-agnostic wording based on each provider’s usage contract. State whether the example assumes that output tokens already include reasoning tokens.
Proposed wording
-Most providers (including OpenAI, Azure, and Google) already count reasoning tokens inside the output token count, so each token is billed exactly once. Reasoning tokens are only added separately for providers that report them outside the output count.
+Storage billing follows the provider's usage contract. When completion tokens include reasoning tokens, each token is billed once. When reasoning tokens are reported separately, they are added to the storage total once. The example below assumes reasoning-inclusive completion tokens.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Most providers (including OpenAI, Azure, and Google) already count reasoning tokens inside the output token count, so each token is billed exactly once. Reasoning tokens are only added separately for providers that report them outside the output count. | |
| Storage billing follows the provider's usage contract. When completion tokens include reasoning tokens, each token is billed once. When reasoning tokens are reported separately, they are added to the storage total once. The example below assumes reasoning-inclusive completion tokens. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/docs/content/features/data-retention.mdx` around lines 43 - 44, Update
the documentation text near the reasoning-token billing explanation to remove
the hardcoded provider names and use provider-agnostic wording based on each
provider’s usage contract. Explicitly state whether the example assumes output
tokens already include reasoning tokens, and preserve the rule that separately
reported reasoning tokens are only added when excluded from the output count.
Source: Coding guidelines
calculateDataStorageCost summed prompt + completion + reasoning unconditionally, but most providers (OpenAI, Azure, Google, aws-mantle, Sakana, Meta, RanoAI) already count reasoning inside the completion count, so retention storage billed those tokens twice whenever a reasoning count reached a call site. Thread the used provider into the storage calculation and skip the reasoning add-on when completionIncludesReasoning() says the completion count already contains it. Providers that report reasoning outside the completion count (e.g. xAI) keep the additive behavior. Stacked on #3486, which provides completionIncludesReasoning() and fixes the equivalent double-count in usage.total_tokens. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
151dc76 to
93a016e
Compare
…n-cost # Conflicts: # apps/gateway/src/chat/tools/parse-provider-response.ts # apps/gateway/src/chat/tools/transform-response-to-openai.ts # apps/gateway/src/lib/costs.spec.ts # apps/gateway/src/lib/costs.ts
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Stacked on #3486 — until that merges, this PR's diff also shows #3486's commits; the delta belonging to this PR is the single top commit (
fix(gateway): provider-aware data storage cost).Problem
calculateDataStorageCostsums prompt + completion + reasoning tokens unconditionally. But for most providers (OpenAI, Azure, Google, aws-mantle, Sakana, Meta, RanoAI) the completion/output token count already contains reasoning tokens — the reportedreasoning_tokensis an informational subset, not an additional count. Whenever a reasoning count reached one of the storage call sites, retention storage billed those tokens twice.This is the storage-cost sibling of the
usage.total_tokensdouble-count that #3486 fixes: same root cause (derived numbers adding reasoning on top of an already-inclusive completion count), different consumer.Approach
calculateDataStorageCost/toDataStorageCostNumberand every chat call site that has a reasoning count in scope.completionIncludesReasoning()(exported by fix(gateway): properly track reasoning tokens #3486) says the completion count already contains it. Providers that genuinely report reasoning outside the completion count (e.g. xAI) keep the additive behavior.Verification
gpt-5.6-sol, flex tier, reasoning enabled), streaming and non-streaming, on/v1/chat/completionsand/v1/responses: the log row and client-visible usage now agree end-to-end — e.g. 30 prompt + 142 completion (135 reasoning inside) →total_tokens172,data_storage_costcomputed on 172 (previously 307), output cost unchanged at the flex rate.pnpm buildand the gatewaylogs/costsspec suites pass.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Documentation