refactor: optimize billing flow for OpenAI-to-Anthropic convert - #3398
Closed
seefs001 wants to merge 2 commits into
Closed
refactor: optimize billing flow for OpenAI-to-Anthropic convert#3398seefs001 wants to merge 2 commits into
seefs001 wants to merge 2 commits into
Conversation
Contributor
WalkthroughThis PR consolidates quota consumption logic by introducing a unified Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@relay/channel/claude/relay-claude.go`:
- Around line 576-583: The recomputed totalInputTokens includes
cacheCreationTokens but you never update the per-field cached_creation token
counter; set clone.PromptTokensDetails.CachedCreationTokens =
cacheCreationTokens (ensuring PromptTokensDetails is non-nil) immediately after
computing cacheCreationTokens/totalInputTokens so the OpenAI-shaped usage
(clone.PromptTokens, clone.InputTokens, clone.TotalTokens) stays consistent with
the per-field breakdown; this touches the variables/functions
cacheCreationTokensForOpenAIUsage, cacheCreationTokens, totalInputTokens, clone
and clone.PromptTokensDetails.CachedCreationTokens.
In `@service/text_quota.go`:
- Around line 106-114: summary.TotalTokens is incorrectly set to only
PromptTokens + CompletionTokens; update the calculation and related gating logic
to include all billable signals (cache creations/writes, cache read/write
distinctions you track, PromptTokensDetails.CachedCreationTokens,
CacheCreationTokens, CacheCreationTokens5m/1h, PromptTokensDetails.ImageTokens,
PromptTokensDetails.AudioTokens and any web/file/tool/fixed-price token fields
from usage) so TotalTokens reflects true billable usage; then change the later
conditional blocks referenced (the logic around resetting to 0 at the section
that currently covers lines 272-275 and the user/channel usage skip at the
section that currently covers lines 319-325) to use this new TotalTokens or an
explicit isBillable flag (e.g., hasBillableTokens := sumOfAllBillableFields > 0)
rather than the previous Prompt+Completion-only check so cache-only or tool-only
requests are handled correctly.
In `@web/src/hooks/usage-logs/useUsageLogsData.jsx`:
- Around line 384-386: The current hasSplitCacheCreation guard only checks for
cache_creation_tokens_5m/_1h and suppresses the aggregate total when the splits
don't sum to the stored total; update the logic that computes
hasSplitCacheCreation to also compute splitSum =
(other?.cache_creation_tokens_5m || 0) + (other?.cache_creation_tokens_1h || 0)
and, if other?.cache_creation_tokens exists and other.cache_creation_tokens >
splitSum, treat this as an "incomplete split" and preserve/show the aggregate
total (i.e., do not hide other.cache_creation_tokens), applying the same change
to the other occurrence around the 428-445 block so aggregate totals are kept
when splits don't add up.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4b6153fa-cd43-4447-a1a9-40f2806ab5f1
📒 Files selected for processing (21)
dto/openai_response.godto/openai_response_test.gorelay/audio_handler.gorelay/channel/claude/relay-claude.gorelay/channel/claude/relay_claude_test.gorelay/claude_handler.gorelay/compatible_handler.gorelay/embedding_handler.gorelay/gemini_handler.gorelay/image_handler.gorelay/rerank_handler.gorelay/responses_handler.goservice/convert.goservice/log_info_generate.goservice/quota.goservice/text_quota.goservice/text_quota_test.goweb/src/components/table/usage-logs/UsageLogsColumnDefs.jsxweb/src/components/table/usage-logs/detailSummary.jsweb/src/helpers/render.jsxweb/src/hooks/usage-logs/useUsageLogsData.jsx
💤 Files with no reviewable changes (1)
- service/quota.go
Comment on lines
+576
to
+583
| clone := *usage | ||
| cacheCreationTokens := cacheCreationTokensForOpenAIUsage(usage) | ||
| totalInputTokens := usage.PromptTokens + usage.PromptTokensDetails.CachedTokens + cacheCreationTokens | ||
| clone.PromptTokens = totalInputTokens | ||
| clone.InputTokens = totalInputTokens | ||
| clone.TotalTokens = totalInputTokens + usage.CompletionTokens | ||
| clone.UsageSemantic = "openai" | ||
| clone.UsageSource = "anthropic" |
Contributor
There was a problem hiding this comment.
Normalize cached_creation_tokens alongside the recomputed prompt total.
totalInputTokens includes cacheCreationTokens, but clone.PromptTokensDetails.CachedCreationTokens is left unchanged. When Claude only sends the split 5m/1h fields, the OpenAI-shaped usage returned from Line 797 and Line 858 will report prompt/input totals that include cache creation while cached_creation_tokens stays 0 or disappears entirely, and any OpenAI-semantic quota math will overcount base tokens.
🩹 Proposed fix
func buildOpenAIStyleUsageFromClaudeUsage(usage *dto.Usage) dto.Usage {
if usage == nil {
return dto.Usage{}
}
clone := *usage
cacheCreationTokens := cacheCreationTokensForOpenAIUsage(usage)
+ clone.PromptTokensDetails.CachedCreationTokens = cacheCreationTokens
totalInputTokens := usage.PromptTokens + usage.PromptTokensDetails.CachedTokens + cacheCreationTokens
clone.PromptTokens = totalInputTokens
clone.InputTokens = totalInputTokens
clone.TotalTokens = totalInputTokens + usage.CompletionTokens
clone.UsageSemantic = "openai"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@relay/channel/claude/relay-claude.go` around lines 576 - 583, The recomputed
totalInputTokens includes cacheCreationTokens but you never update the per-field
cached_creation token counter; set
clone.PromptTokensDetails.CachedCreationTokens = cacheCreationTokens (ensuring
PromptTokensDetails is non-nil) immediately after computing
cacheCreationTokens/totalInputTokens so the OpenAI-shaped usage
(clone.PromptTokens, clone.InputTokens, clone.TotalTokens) stays consistent with
the per-field breakdown; this touches the variables/functions
cacheCreationTokensForOpenAIUsage, cacheCreationTokens, totalInputTokens, clone
and clone.PromptTokensDetails.CachedCreationTokens.
Comment on lines
+106
to
+114
| summary.PromptTokens = usage.PromptTokens | ||
| summary.CompletionTokens = usage.CompletionTokens | ||
| summary.TotalTokens = usage.PromptTokens + usage.CompletionTokens | ||
| summary.CacheTokens = usage.PromptTokensDetails.CachedTokens | ||
| summary.CacheCreationTokens = usage.PromptTokensDetails.CachedCreationTokens | ||
| summary.CacheCreationTokens5m = usage.ClaudeCacheCreation5mTokens | ||
| summary.CacheCreationTokens1h = usage.ClaudeCacheCreation1hTokens | ||
| summary.ImageTokens = usage.PromptTokensDetails.ImageTokens | ||
| summary.AudioTokens = usage.PromptTokensDetails.AudioTokens |
Contributor
There was a problem hiding this comment.
Don't use PromptTokens + CompletionTokens as the only billable signal.
summary.TotalTokens ignores cache reads/writes, image/audio tokens, web/file search, and fixed-price calls. A cache-only or tool-only request can compute a positive quota above, then Lines 272-275 reset it to 0 and Lines 319-325 skip the user/channel usage counters as well.
💡 Suggested fix
+func hasBillableUsage(summary textQuotaSummary, relayInfo *relaycommon.RelayInfo) bool {
+ return summary.PromptTokens > 0 ||
+ summary.CompletionTokens > 0 ||
+ summary.CacheTokens > 0 ||
+ cacheWriteTokensTotal(summary) > 0 ||
+ summary.ImageTokens > 0 ||
+ summary.AudioTokens > 0 ||
+ summary.WebSearchCallCount > 0 ||
+ summary.ClaudeWebSearchCallCount > 0 ||
+ summary.FileSearchCallCount > 0 ||
+ summary.ImageGenerationCallPrice > 0 ||
+ (relayInfo.PriceData.UsePrice && summary.ModelPrice > 0)
+}
...
- if summary.TotalTokens == 0 {
+ if !hasBillableUsage(summary, relayInfo) {
summary.Quota = 0
} else if !ratio.IsZero() && summary.Quota == 0 {
summary.Quota = 1
}
...
- if summary.TotalTokens == 0 {
+ if !hasBillableUsage(summary, relayInfo) {
extraContent = append(extraContent, "上游没有返回计费信息，无法扣费（可能是上游超时）")
logger.LogError(ctx, fmt.Sprintf("total tokens is 0, cannot consume quota, userId %d, channelId %d, tokenId %d, model %s， pre-consumed quota %d", relayInfo.UserId, relayInfo.ChannelId, relayInfo.TokenId, summary.ModelName, relayInfo.FinalPreConsumedQuota))
} else {
model.UpdateUserUsedQuotaAndRequestCount(relayInfo.UserId, summary.Quota)
model.UpdateChannelUsedQuota(relayInfo.ChannelId, summary.Quota)
}Also applies to: 272-275, 319-325
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@service/text_quota.go` around lines 106 - 114, summary.TotalTokens is
incorrectly set to only PromptTokens + CompletionTokens; update the calculation
and related gating logic to include all billable signals (cache
creations/writes, cache read/write distinctions you track,
PromptTokensDetails.CachedCreationTokens, CacheCreationTokens,
CacheCreationTokens5m/1h, PromptTokensDetails.ImageTokens,
PromptTokensDetails.AudioTokens and any web/file/tool/fixed-price token fields
from usage) so TotalTokens reflects true billable usage; then change the later
conditional blocks referenced (the logic around resetting to 0 at the section
that currently covers lines 272-275 and the user/channel usage skip at the
section that currently covers lines 319-325) to use this new TotalTokens or an
explicit isBillable flag (e.g., hasBillableTokens := sumOfAllBillableFields > 0)
rather than the previous Prompt+Completion-only check so cache-only or tool-only
requests are handled correctly.
Comment on lines
+384
to
+386
| const hasSplitCacheCreation = | ||
| (other?.cache_creation_tokens_5m || 0) > 0 || | ||
| (other?.cache_creation_tokens_1h || 0) > 0; |
Contributor
There was a problem hiding this comment.
Keep the aggregate cache-creation total when split windows do not add up to it.
The backend now preserves cache_creation_tokens alongside cache_creation_tokens_5m/_1h when there is an unsplit remainder. With the new guard here, a record like 50 total / 10 (5m) / 20 (1h) only shows 30 in the expanded details, while the prompt column still shows 50 via the normalized cache-write total.
💡 Suggested fix
- const hasSplitCacheCreation =
- (other?.cache_creation_tokens_5m || 0) > 0 ||
- (other?.cache_creation_tokens_1h || 0) > 0;
+ const cacheCreationTokens = Number(other?.cache_creation_tokens || 0);
+ const cacheCreationTokens5m = Number(
+ other?.cache_creation_tokens_5m || 0,
+ );
+ const cacheCreationTokens1h = Number(
+ other?.cache_creation_tokens_1h || 0,
+ );
+ const splitCacheCreationTotal =
+ cacheCreationTokens5m + cacheCreationTokens1h;
+ const hasSplitCacheCreation = splitCacheCreationTotal > 0;
...
- if (!hasSplitCacheCreation && other?.cache_creation_tokens > 0) {
+ if (
+ cacheCreationTokens > 0 &&
+ (!hasSplitCacheCreation ||
+ cacheCreationTokens > splitCacheCreationTotal)
+ ) {
expandDataLocal.push({
key: t('缓存创建 Tokens'),
- value: other.cache_creation_tokens,
+ value: cacheCreationTokens,
});
}
- if (other?.cache_creation_tokens_5m > 0) {
+ if (cacheCreationTokens5m > 0) {
expandDataLocal.push({
key: t('缓存创建 Tokens (5m)'),
- value: other.cache_creation_tokens_5m,
+ value: cacheCreationTokens5m,
});
}
- if (other?.cache_creation_tokens_1h > 0) {
+ if (cacheCreationTokens1h > 0) {
expandDataLocal.push({
key: t('缓存创建 Tokens (1h)'),
- value: other.cache_creation_tokens_1h,
+ value: cacheCreationTokens1h,
});
}Also applies to: 428-445
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/hooks/usage-logs/useUsageLogsData.jsx` around lines 384 - 386, The
current hasSplitCacheCreation guard only checks for cache_creation_tokens_5m/_1h
and suppresses the aggregate total when the splits don't sum to the stored
total; update the logic that computes hasSplitCacheCreation to also compute
splitSum = (other?.cache_creation_tokens_5m || 0) +
(other?.cache_creation_tokens_1h || 0) and, if other?.cache_creation_tokens
exists and other.cache_creation_tokens > splitSum, treat this as an "incomplete
split" and preserve/show the aggregate total (i.e., do not hide
other.cache_creation_tokens), applying the same change to the other occurrence
around the 428-445 block so aggregate totals are kept when splits don't add up.
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 free
to 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.
Summary by CodeRabbit
New Features
Bug Fixes
Tests