Add OpenAI service tier channel pricing#3482
Closed
69gg wants to merge 4 commits intoQuantumNous:mainfrom
Closed
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughRefreshes channel-specific pricing and applies service-tier ratios during relay retries; ensures pre-consumed billing quota before using a channel; extends PriceData and billing/session APIs; adds channel DTO/UI for service-tier ratios; surfaces service-tier info in logs and usage UI; includes unit tests for service-tier and billing-session behavior. Changes
Sequence DiagramsequenceDiagram
participant Client
participant RelayController as Relay Controller
participant PriceHelper as Price Helper
participant ServiceTier as Service Tier Helper
participant Billing as BillingSession
participant Logger as Log Service
Client->>RelayController: Send request (may include service_tier)
RelayController->>RelayController: Select channel / enter retry loop
RelayController->>ServiceTier: RefreshChannelSpecificPriceData(relayInfo)
ServiceTier->>ServiceTier: Extract/derive service_tier and ratio
ServiceTier-->>RelayController: Updated PriceData (ServiceTier, Ratio, adjusted quotas)
RelayController->>Billing: EnsurePreConsumedQuota(PriceData.QuotaToPreConsume)
Billing-->>RelayController: OK / error
RelayController->>PriceHelper: Build model price (uses BaseQuota & ServiceTier)
PriceHelper->>ServiceTier: applyChannelServiceTierPricing(...)
ServiceTier-->>PriceHelper: Adjusted PriceData
PriceHelper-->>RelayController: Final price & quotas
RelayController->>Client: Forward request/response via selected channel
RelayController->>Logger: GenerateTextOtherInfo(relayInfo)
Logger-->>RelayController: Log metas (includes service_tier fields)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@controller/relay.go`:
- Around line 197-199: The quota refresh is happening after pre-consume (quota
reservation) so stale cheaper reservations can persist when channel pricing
changes; update logic in relay.go to call helper.RefreshChannelSpecificPriceData
(from helper/service_tier.go) before any billing/reservation step and ensure it
uses updatePreConsumedQuota=true or re-reserve the quota for each selected
channel after refresh; if RefreshChannelSpecificPriceData returns an error,
abort the request (or trigger a retry) instead of clearing the tier and
proceeding—i.e., re-resolve pricing and re-reserve pre-consumed quota per
channel prior to billing, or fail fast on refresh error.
In `@relay/helper/price.go`:
- Around line 135-137: The call to applyChannelServiceTierPricing(c, info,
&priceData) currently only logs a warning on error (logger.LogWarn) which lets
quota pre-consume proceed with incorrect base QuotaToPreConsume; instead
propagate the failure so the reservation is not made with wrong pricing: change
the handling in the caller to return the serviceTierErr (or convert it into a
wrapped error) when applyChannelServiceTierPricing fails, ensuring the request
halts or retries before QuotaToPreConsume is used; update any function
signatures/returns that call this code path to accept and forward the error
(references: applyChannelServiceTierPricing, priceData, QuotaToPreConsume,
logger.LogWarn).
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cb58e0c1-6cbf-4672-b3f9-ba76efbe2f01
⛔ Files ignored due to path filters (1)
web/dist/index.htmlis excluded by!**/dist/**
📒 Files selected for processing (9)
controller/relay.godto/channel_settings.gorelay/helper/price.gorelay/helper/service_tier.gorelay/helper/service_tier_test.goservice/log_info_generate.gotypes/price_data.goweb/src/components/table/channels/modals/EditChannelModal.jsxweb/src/components/table/usage-logs/UsageLogsColumnDefs.jsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
service/billing_session.go (1)
158-161: Redundant check:delta <= 0is already guaranteed false.Line 154 checks
targetQuota <= s.preConsumedQuotaand returns early. If execution reaches line 158, thentargetQuota > s.preConsumedQuota, sodelta = targetQuota - s.preConsumedQuotais guaranteed to be positive.♻️ Remove redundant check
if s.settled || s.refunded || targetQuota <= s.preConsumedQuota { return nil } delta := targetQuota - s.preConsumedQuota - if delta <= 0 { - return nil - } if err := PreConsumeTokenQuota(s.relayInfo, delta); err != nil {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@service/billing_session.go` around lines 158 - 161, In the method containing the pre-check "if targetQuota <= s.preConsumedQuota { return nil }" (within billing_session.go), remove the redundant "if delta <= 0 { return nil }" after computing "delta := targetQuota - s.preConsumedQuota" because delta must be positive at that point; simply compute delta and proceed with the subsequent logic (keeping the variable name delta and all downstream uses intact).relay/helper/service_tier.go (2)
59-65: Consider rounding behavior for quota calculation.The conversion
int(float64(baseQuota) * ratio)truncates toward zero. For example, ifbaseQuota=100andratio=1.5, the result is150, which is correct. However, ifratio=1.999, the result is199(truncated from199.9).If consistent rounding is desired (e.g., round to nearest), consider using
math.Round:♻️ Optional: Use explicit rounding
+import "math" + if updatePreConsumedQuota && priceData.QuotaToPreConsume > 0 { baseQuota := priceData.BaseQuotaToPreConsume if baseQuota <= 0 { baseQuota = priceData.QuotaToPreConsume } - priceData.QuotaToPreConsume = int(float64(baseQuota) * ratio) + priceData.QuotaToPreConsume = int(math.Round(float64(baseQuota) * ratio)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/helper/service_tier.go` around lines 59 - 65, The current quota recalculation uses int(float64(baseQuota) * ratio) which truncates decimals; update the calculation in the block guarded by updatePreConsumedQuota to use explicit rounding (e.g., math.Round) when assigning priceData.QuotaToPreConsume so values are rounded to the nearest integer rather than truncated—use priceData.BaseQuotaToPreConsume (or fallback baseQuota) multiplied by ratio, rounded, then cast to int.
283-316: Missing handling for unsigned integer types and NaN.The
parsePositiveFloat64function handlesfloat64,float32,int,int32,int64, andstring, but doesn't handle unsigned integers (uint,uint32,uint64) which could be present in JSON unmarshaling depending on the source.Additionally, while
value > 0correctly rejects+Inf(sinceInf > 0is true but we return it), it doesn't explicitly handleNaNwhich would failNaN > 0and be correctly rejected—so this is actually fine.♻️ Add unsigned integer handling
func parsePositiveFloat64(raw any) (float64, bool) { switch value := raw.(type) { case float64: if value > 0 { return value, true } case float32: if value > 0 { return float64(value), true } case int: if value > 0 { return float64(value), true } case int32: if value > 0 { return float64(value), true } case int64: if value > 0 { return float64(value), true } + case uint: + if value > 0 { + return float64(value), true + } + case uint32: + if value > 0 { + return float64(value), true + } + case uint64: + if value > 0 { + return float64(value), true + } case string:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/helper/service_tier.go` around lines 283 - 316, The parsePositiveFloat64 function is missing handling for unsigned integer types; update parsePositiveFloat64 to add type switch cases for uint, uint32, and uint64 (and optionally other unsigned variants you use) that convert the unsigned value to float64 and return it when > 0, keeping the same return pattern as float/int cases; keep the existing string parsing and NaN/Inf behavior unchanged and reference the parsePositiveFloat64 function in your change so the unsigned branches are added to the existing type switch.types/price_data.go (1)
43-45: Consider breaking the long format string for readability.The
ToSetting()method produces a very long line that may be difficult to read and maintain. While functionally correct, breaking it into multiple lines would improve readability.♻️ Optional readability improvement
func (p *PriceData) ToSetting() string { - return fmt.Sprintf("ModelPrice: %f, ModelRatio: %f, CompletionRatio: %f, CacheRatio: %f, GroupRatio: %f, UsePrice: %t, CacheCreationRatio: %f, CacheCreation5mRatio: %f, CacheCreation1hRatio: %f, QuotaToPreConsume: %d, ImageRatio: %f, AudioRatio: %f, AudioCompletionRatio: %f, ServiceTier: %s, ServiceTierRatio: %f", p.ModelPrice, p.ModelRatio, p.CompletionRatio, p.CacheRatio, p.GroupRatioInfo.GroupRatio, p.UsePrice, p.CacheCreationRatio, p.CacheCreation5mRatio, p.CacheCreation1hRatio, p.QuotaToPreConsume, p.ImageRatio, p.AudioRatio, p.AudioCompletionRatio, p.ServiceTier, p.ServiceTierRatio) + return fmt.Sprintf( + "ModelPrice: %f, ModelRatio: %f, CompletionRatio: %f, CacheRatio: %f, GroupRatio: %f, "+ + "UsePrice: %t, CacheCreationRatio: %f, CacheCreation5mRatio: %f, CacheCreation1hRatio: %f, "+ + "QuotaToPreConsume: %d, ImageRatio: %f, AudioRatio: %f, AudioCompletionRatio: %f, "+ + "ServiceTier: %s, ServiceTierRatio: %f", + p.ModelPrice, p.ModelRatio, p.CompletionRatio, p.CacheRatio, p.GroupRatioInfo.GroupRatio, + p.UsePrice, p.CacheCreationRatio, p.CacheCreation5mRatio, p.CacheCreation1hRatio, + p.QuotaToPreConsume, p.ImageRatio, p.AudioRatio, p.AudioCompletionRatio, + p.ServiceTier, p.ServiceTierRatio, + ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@types/price_data.go` around lines 43 - 45, The ToSetting method on PriceData currently builds a very long fmt.Sprintf call; split the format string across multiple concatenated literals or build the output in pieces (e.g., multiple fmt.Sprintf calls or a strings.Builder) inside PriceData.ToSetting to improve readability and maintainability; update the function name PriceData.ToSetting and references to the format tokens (ModelPrice, ModelRatio, CompletionRatio, CacheRatio, GroupRatioInfo.GroupRatio, UsePrice, CacheCreationRatio, CacheCreation5mRatio, CacheCreation1hRatio, QuotaToPreConsume, ImageRatio, AudioRatio, AudioCompletionRatio, ServiceTier, ServiceTierRatio) so the final returned string is identical but the format is broken into multiple shorter lines.service/billing_session_test.go (1)
24-57: Good test coverage for the happy path; consider adding edge case tests.The test correctly verifies the top-up behavior and the no-op case when target is lower. Consider adding tests for:
- Sessions that are already
settledorrefunded(should no-op)- Non-playground mode to verify token quota operations
- Error handling when
PreConsumeTokenQuotaorfunding.SettlefailsWould you like me to generate additional test cases covering these edge cases?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@service/billing_session_test.go` around lines 24 - 57, Add unit tests that cover the edge cases missing from TestBillingSessionEnsurePreConsumedQuotaTopUpsReservation: create cases where BillingSession is already in settled or refunded state and assert EnsurePreConsumedQuota is a no-op (no changes to preConsumedQuota/preConsumedTopUp and no calls to funding.Settle), a non-playground RelayInfo case to verify token quota logic differs (exercise PreConsumeTokenQuota path and assert expected behavior), and failure paths where PreConsumeTokenQuota or billingSessionTestFunding.Settle return errors (mock those methods to return errors and assert EnsurePreConsumedQuota propagates or handles them as intended). Use the existing BillingSession, EnsurePreConsumedQuota, billingSessionTestFunding and relaycommon.RelayInfo symbols to locate and extend test coverage.
🤖 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/helper/service_tier_test.go`:
- Around line 3-8: The test imports encoding/json only to use json.RawMessage
which violates the guideline; remove the encoding/json import in
relay/helper/service_tier_test.go and update test fixtures to use a plain string
(e.g., set ServiceTier as "standard" or the expected string) or use []byte for
raw JSON payloads instead of json.RawMessage, and adjust any references to
json.RawMessage to the chosen type so the resolver that normalizes/extracts the
string still receives the same value (update variable names and test assertions
around ServiceTier to match the new type).
---
Nitpick comments:
In `@relay/helper/service_tier.go`:
- Around line 59-65: The current quota recalculation uses int(float64(baseQuota)
* ratio) which truncates decimals; update the calculation in the block guarded
by updatePreConsumedQuota to use explicit rounding (e.g., math.Round) when
assigning priceData.QuotaToPreConsume so values are rounded to the nearest
integer rather than truncated—use priceData.BaseQuotaToPreConsume (or fallback
baseQuota) multiplied by ratio, rounded, then cast to int.
- Around line 283-316: The parsePositiveFloat64 function is missing handling for
unsigned integer types; update parsePositiveFloat64 to add type switch cases for
uint, uint32, and uint64 (and optionally other unsigned variants you use) that
convert the unsigned value to float64 and return it when > 0, keeping the same
return pattern as float/int cases; keep the existing string parsing and NaN/Inf
behavior unchanged and reference the parsePositiveFloat64 function in your
change so the unsigned branches are added to the existing type switch.
In `@service/billing_session_test.go`:
- Around line 24-57: Add unit tests that cover the edge cases missing from
TestBillingSessionEnsurePreConsumedQuotaTopUpsReservation: create cases where
BillingSession is already in settled or refunded state and assert
EnsurePreConsumedQuota is a no-op (no changes to
preConsumedQuota/preConsumedTopUp and no calls to funding.Settle), a
non-playground RelayInfo case to verify token quota logic differs (exercise
PreConsumeTokenQuota path and assert expected behavior), and failure paths where
PreConsumeTokenQuota or billingSessionTestFunding.Settle return errors (mock
those methods to return errors and assert EnsurePreConsumedQuota propagates or
handles them as intended). Use the existing BillingSession,
EnsurePreConsumedQuota, billingSessionTestFunding and relaycommon.RelayInfo
symbols to locate and extend test coverage.
In `@service/billing_session.go`:
- Around line 158-161: In the method containing the pre-check "if targetQuota <=
s.preConsumedQuota { return nil }" (within billing_session.go), remove the
redundant "if delta <= 0 { return nil }" after computing "delta := targetQuota -
s.preConsumedQuota" because delta must be positive at that point; simply compute
delta and proceed with the subsequent logic (keeping the variable name delta and
all downstream uses intact).
In `@types/price_data.go`:
- Around line 43-45: The ToSetting method on PriceData currently builds a very
long fmt.Sprintf call; split the format string across multiple concatenated
literals or build the output in pieces (e.g., multiple fmt.Sprintf calls or a
strings.Builder) inside PriceData.ToSetting to improve readability and
maintainability; update the function name PriceData.ToSetting and references to
the format tokens (ModelPrice, ModelRatio, CompletionRatio, CacheRatio,
GroupRatioInfo.GroupRatio, UsePrice, CacheCreationRatio, CacheCreation5mRatio,
CacheCreation1hRatio, QuotaToPreConsume, ImageRatio, AudioRatio,
AudioCompletionRatio, ServiceTier, ServiceTierRatio) so the final returned
string is identical but the format is broken into multiple shorter lines.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 48e071da-b4bf-476d-a101-70f8b7b11654
📒 Files selected for processing (8)
controller/relay.gorelay/common/billing.gorelay/helper/price.gorelay/helper/service_tier.gorelay/helper/service_tier_test.goservice/billing_session.goservice/billing_session_test.gotypes/price_data.go
🚧 Files skipped from review as they are similar to previous changes (2)
- controller/relay.go
- relay/helper/price.go
Member
|
main分支不再接受价格相关pr，如需阶梯计费，请使用nigtly分支测试版本 |
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
2 participants
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
Testing
Summary by CodeRabbit
New Features
Bug Fixes
Tests