feat: auto分组 - #1239
Conversation
WalkthroughThis update introduces support for an "auto" user group that dynamically aggregates multiple configured groups for model selection and channel allocation. It adds new configuration options for auto groups, modifies token group handling, updates quota and pricing calculations to incorporate auto group logic, and refactors related code for consistent context-aware channel retrieval and group ratio management. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant Model
participant Cache
participant Setting
participant RelayHelper
participant Service
participant Middleware
Client->>Controller: Request with token group "auto"
Controller->>Setting: Check if "auto" group enabled and get AutoGroups list
Controller->>Model: ListModels for each group in AutoGroups
Model->>Cache: Retrieve models per group
Cache-->>Model: Return aggregated unique models
Model-->>Controller: Return combined model list
Controller->>Cache: CacheGetRandomSatisfiedChannel(ctx, "auto", model, retry)
Cache->>Setting: Get AutoGroups list
Cache->>Cache: Iterate AutoGroups to find channel
Cache-->>Controller: Return channel and selectedGroup
Controller->>Middleware: Pass selectedGroup via context
Middleware->>Cache: CacheGetRandomSatisfiedChannel(ctx, userGroup, model, flag)
Cache-->>Middleware: Channel, selectedGroup
Middleware->>Controller: Continue with selectedGroup info
Controller->>RelayHelper: ModelPriceHelper(ctx, relayInfo, tokens...)
RelayHelper->>Setting: Check context for "auto_group"
RelayHelper->>Setting: Get group ratio or special ratio
RelayHelper-->>Controller: Return PriceData with GroupRatioInfo
Controller->>Service: PostConsumeQuota(ctx, relayInfo, priceData, ...)
Service->>RelayHelper: Use GroupRatioInfo for quota calculation
Possibly related PRs
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🔭 Outside diff range comments (2)
controller/relay.go (1)
262-267: DiscardingselectedGrouploses useful context
CacheGetRandomSatisfiedChannelnow returns the actual group picked.
Ignoring it means logs, billing or downstream logic still “see” the originalgroup(often"auto").- channel, _, err := model.CacheGetRandomSatisfiedChannel(c, group, originalModel, retryCount) + channel, selectedGroup, err := model.CacheGetRandomSatisfiedChannel(c, group, originalModel, retryCount) + if err == nil { + c.Set("group", selectedGroup) // keep context consistent + }Propagating the real group keeps metrics accurate.
model/cache.go (1)
127-133: Nil-map index may panic when group/model unknown
group2model2channels[group]returnsnilwhen the group key is missing; indexing that nil map ([model]) panics.
Auto groups provided viasetting.AutoGroupsmight legitimately reference a group not yet cached.channelSyncLock.RLock() -modelMap := group2model2channels[group] -channels := modelMap[model] +modelMap, ok := group2model2channels[group] +if !ok { + channelSyncLock.RUnlock() + return nil, errors.New("group not found") +} +channels := modelMap[model] channelSyncLock.RUnlock()
🧹 Nitpick comments (13)
controller/user.go (1)
229-231: Extract hard-coded"auto"into a shared constantThe literal is used in multiple packages (controller, model, middleware, setting).
Introduce something likeconst AutoGroupName = "auto"(e.g.
constant/group.go) and reference it here to avoid typos and ease future refactors.controller/relay.go (1)
391-397: Same issue inside task-relay retry loopThe second return value is again ignored; consider applying the same fix here to avoid inconsistencies between normal relay and task relay paths.
web/src/components/settings/OperationSetting.js (1)
81-82: Boolean/JSON parsing condition is getting unwieldyYou now check the key three different times:
if ( item.key.endsWith('Enabled') || ['DefaultCollapseSidebar'].includes(item.key) || ['DefaultUseAutoGroup'].includes(item.key) )
- The two
includescalls allocate new arrays every iteration.- Readability suffers.
A compact alternative:
- if ( - item.key.endsWith('Enabled') || - ['DefaultCollapseSidebar'].includes(item.key) || - ['DefaultUseAutoGroup'].includes(item.key) - ) { + const booleanKeys = ['DefaultCollapseSidebar', 'DefaultUseAutoGroup']; + if (item.key.endsWith('Enabled') || booleanKeys.includes(item.key)) {Not critical but keeps maintenance simple.
Also applies to: 90-93
controller/model.go (1)
184-196:O(n²)duplicate elimination can degrade with many models
common.StringsContainsinside the nested loop turns the aggregation into quadratic complexity.
For dozens of auto-groups × hundreds of models this becomes noticeable.- var models []string + modelSet := map[string]struct{}{} if tokenGroup == "auto" { for _, autoGroup := range setting.AutoGroups { groupModels := model.GetGroupModels(autoGroup) - for _, g := range groupModels { - if !common.StringsContains(models, g) { - models = append(models, g) - } - } + for _, m := range groupModels { + modelSet[m] = struct{}{} + } } } else { - models = model.GetGroupModels(group) + for _, m := range model.GetGroupModels(group) { + modelSet[m] = struct{}{} + } } + for m := range modelSet { + models = append(models, m) + }Keeps insertion
O(1)and avoids repeated scans.controller/playground.go (1)
61-64: Error path loses original error context
model.CacheGetRandomSatisfiedChannelalready returns anerror; wrapping it with a generic
message hides the root cause (e.g., DB outage vs. no channel).Consider preserving it:
- message := fmt.Sprintf("当前分组 %s 下对于模型 %s 无可用渠道", finalGroup, playgroundRequest.Model) - openaiErr = service.OpenAIErrorWrapperLocal(errors.New(message), "get_playground_channel_failed", http.StatusInternalServerError) + openaiErr = service.OpenAIErrorWrapperLocal( + fmt.Errorf("group %s / model %s: %w", finalGroup, playgroundRequest.Model, err), + "get_playground_channel_failed", + http.StatusInternalServerError, + )Makes troubleshooting simpler.
web/src/pages/Token/EditToken.js (2)
46-47:statusDispatchis retrieved but never used
useContextdestructures[statusState, statusDispatch], yetstatusDispatchis not referenced afterwards, leading to an unused-variable lint error in most React setups.
Delete it or switch to{state} = useContext(StatusContext)if only the state is needed.- const [statusState, statusDispatch] = useContext(StatusContext); + const [statusState] = useContext(StatusContext);
127-129: Comparator can return only-1or1
sort((a,b)=>a.value==='auto'?-1:1)never returns0, so non-autoitems keep relative order only by engine’s stability.
Return0when neither element isautofor clarity.- localGroupOptions.sort((a, b) => (a.value === 'auto' ? -1 : 1)); + localGroupOptions.sort((a, b) => { + if (a.value === 'auto') return -1; + if (b.value === 'auto') return 1; + return 0; + });relay/helper/price.go (2)
36-41: Unconditionallog.Printfwill spam production logs
log.Printf("final group ratio: …")executes on every request.
Either guard behindcommon.DebugEnabled(like later in the file) or switch tocommon.LogDebug.- log.Printf("final group ratio: %f", groupRatio) + if common.DebugEnabled { + log.Printf("final group ratio: %f", groupRatio) + }
42-48:groupRatioreassignment can be simplifiedAfter computing
actualGroupRatio, the code immediately doesgroupRatio = actualGroupRatio.
All later references already usegroupRatio; the extra variable adds noise.
Optionally dropactualGroupRatioor return it directly.middleware/distributor.go (1)
52-56: Magic string"auto"appears in multiple placesHard-coded literals make refactors error-prone.
Consider promoting"auto"to aconst tokenGroupAuto = "auto"in a shared package.service/quota.go (1)
6-7:logimported solely for debug printsThe new
logimport is only used for the same unconditional prints added elsewhere. Prefer the project’s logging util and guard behind debug flag to avoid extra dependency noise.model/cache.go (2)
158-165: Hard-coded magic numbersmoothingFactor := 10Consider turning
10into aconstat package scope or a configurable option so tuning doesn’t require recompilation.const weightSmoothingFactor = 10 … totalWeight += channel.GetWeight() + weightSmoothingFactor
82-112: Return value naming inconsistencyInside the function the variable is called
selectGroup, but the exported name in docs/call-sites will likely be “selectedGroup”. Minor but improves readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
controller/group.go(2 hunks)controller/misc.go(2 hunks)controller/model.go(2 hunks)controller/playground.go(2 hunks)controller/relay.go(2 hunks)controller/user.go(1 hunks)middleware/distributor.go(2 hunks)model/cache.go(2 hunks)model/option.go(2 hunks)relay/helper/price.go(2 hunks)service/quota.go(5 hunks)setting/auto_group.go(1 hunks)setting/user_usable_group.go(1 hunks)web/src/components/settings/OperationSetting.js(3 hunks)web/src/pages/Setting/Operation/GroupRatioSettings.js(2 hunks)web/src/pages/Token/EditToken.js(9 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (11)
model/option.go (2)
common/constants.go (1)
OptionMap(36-36)setting/auto_group.go (3)
AutoGroups2JsonString(25-31)DefaultUseAutoGroup(9-9)UpdateAutoGroupsByJsonString(20-23)
controller/user.go (1)
setting/auto_group.go (1)
DefaultUseAutoGroup(9-9)
controller/relay.go (1)
model/cache.go (1)
CacheGetRandomSatisfiedChannel(82-112)
controller/misc.go (6)
common/constants.go (25)
Version(13-13)StartTime(12-12)EmailVerificationEnabled(44-44)GitHubOAuthEnabled(45-45)GitHubClientId(82-82)LinuxDOOAuthEnabled(46-46)LinuxDOClientId(84-84)TelegramOAuthEnabled(48-48)TelegramBotName(95-95)SystemName(14-14)Logo(16-16)Footer(15-15)WeChatAccountQRCodeImageURL(89-89)WeChatAuthEnabled(47-47)TurnstileCheckEnabled(49-49)TurnstileSiteKey(91-91)TopUpLink(17-17)QuotaPerUnit(21-21)DisplayInCurrencyEnabled(22-22)BatchUpdateEnabled(117-117)DrawingEnabled(24-24)TaskEnabled(25-25)DataExportEnabled(26-26)DataExportDefaultTime(28-28)DefaultCollapseSidebar(29-29)setting/system_setting.go (1)
ServerAddress(3-3)setting/operation_setting/general_setting.go (1)
GetGeneralSetting(23-25)setting/chat.go (1)
Chats(8-24)setting/operation_setting/operation_setting.go (2)
DemoSiteEnabled(5-5)SelfUseModeEnabled(6-6)setting/auto_group.go (1)
DefaultUseAutoGroup(9-9)
controller/group.go (1)
setting/user_usable_group.go (2)
GroupInUserUsableGroups(49-52)GetUsableGroupDescription(54-59)
controller/model.go (2)
setting/auto_group.go (1)
AutoGroups(5-7)model/ability.go (1)
GetGroupModels(24-29)
web/src/pages/Setting/Operation/GroupRatioSettings.js (2)
web/src/helpers/utils.js (2)
verifyJSON(236-243)verifyJSON(236-243)web/src/components/settings/OperationSetting.js (1)
inputs(21-67)
service/quota.go (2)
setting/operation_setting/model-ratio.go (1)
GetModelRatio(345-357)setting/group_ratio.go (2)
GetGroupRatio(64-74)GetGroupGroupRatio(76-89)
relay/helper/price.go (1)
setting/group_ratio.go (2)
GetGroupRatio(64-74)GetGroupGroupRatio(76-89)
controller/playground.go (1)
model/cache.go (1)
CacheGetRandomSatisfiedChannel(82-112)
middleware/distributor.go (1)
model/cache.go (1)
CacheGetRandomSatisfiedChannel(82-112)
🔇 Additional comments (9)
controller/user.go (1)
229-231: Confirm that the new token is visible to the userWhen
DefaultUseAutoGroupistrue, the initial token is stored in the"auto"group.
Please double-check that
setting.GetUserUsableGroups(user.Group)and UI components already include"auto", otherwise the freshly-created token will not appear in group selectors.setting/user_usable_group.go (1)
54-59: Helper looks good
GetUsableGroupDescriptionis concise, nil-safe, and avoids repeated map look-ups.controller/misc.go (1)
78-79: API surface updated correctly
default_use_auto_groupis exposed with consistent snake_case naming – no further issues spotted.web/src/pages/Setting/Operation/GroupRatioSettings.js (2)
20-22: Ensure value types match what the Switch expectsIf
props.options.DefaultUseAutoGrouparrives as the string"true"/"false"(current backend pattern), the controlledSwitchwill receive a string instead of boolean, triggering a React warning.
Cast to boolean inuseEffect(currentInputs[key] = props.options[key] === 'true') or adjust backend serialization.
195-205: Confirm option parsing for"DefaultUseAutoGroup"
compareObjectsconverts booleans to"true"/"false"strings before the PUT call.
Verify that the server side usesstrconv.ParseBool(or equivalent) when reading this option; otherwise"false"may be mis-interpreted.controller/group.go (1)
38-43: ```shell
#!/bin/bashSearch for all occurrences of "ratio" in the codebase with context
rg -n -A3 -B3 '"ratio"' .
Locate where usableGroups is defined and populated in controller/group.go
rg -n "usableGroups" -n controller/group.go
</details> <details> <summary>model/option.go (1)</summary> `79-81`: **Option keys added, but not documented** `AutoGroups` and `DefaultUseAutoGroup` are persisted immediately, yet there is no comment in the file header or in `setting/auto_group.go` describing their expected format & semantics. Add brief godoc to help future maintainers. [nitpick] </details> <details> <summary>middleware/distributor.go (1)</summary> `100-108`: **Possible empty group in error message** When `CacheGetRandomSatisfiedChannel` fails before selecting a concrete group, `selectGroup` may still equal `"auto"`, resulting in `auto()` in the message. Ensure `selectGroup` is non-empty before interpolation or fall back to `"unknown"` to avoid confusing responses. </details> <details> <summary>service/quota.go (1)</summary> `101-105`: **Potential panic if `auto_group` is not `string`** `ctx.Get` returns `any`; if some middleware sets a non-string value, `.(string)` panics. Use a type-assertion check: ```go if g, ok := autoGroup.(string); ok { groupRatio = setting.GetGroupRatio(g) relayInfo.Group = g }
| if (statusState?.status?.default_use_auto_group) { | ||
| // if contain auto, add it to the first position | ||
| if (localGroupOptions.some((group) => group.value === 'auto')) { | ||
| // 排序 | ||
| localGroupOptions.sort((a, b) => (a.value === 'auto' ? -1 : 1)); | ||
| } else { | ||
| localGroupOptions.unshift({ label: t('自动选择'), value: 'auto' }); | ||
| } | ||
| } | ||
| setGroups(localGroupOptions); | ||
| if (statusState?.status?.default_use_auto_group) { | ||
| setInputs({ ...inputs, group: 'auto' }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Editing an existing token is silently reset to auto group
loadGroups unconditionally overrides the form state with group: 'auto' whenever default_use_auto_group is true.
When editing a token whose group ≠ auto, that value is lost because loadGroups runs after loadToken. Users will save an unexpected change.
- if (statusState?.status?.default_use_auto_group) {
- setInputs({ ...inputs, group: 'auto' });
- }
+ if (!isEdit && statusState?.status?.default_use_auto_group) {
+ // only default to auto for NEW tokens
+ setInputs((prev) => ({ ...prev, group: 'auto' }));
+ }Note the functional setInputs form to avoid stale-state issues.
Consider the same guard for the insertion/sort logic to keep UI consistent.
📝 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.
| if (statusState?.status?.default_use_auto_group) { | |
| // if contain auto, add it to the first position | |
| if (localGroupOptions.some((group) => group.value === 'auto')) { | |
| // 排序 | |
| localGroupOptions.sort((a, b) => (a.value === 'auto' ? -1 : 1)); | |
| } else { | |
| localGroupOptions.unshift({ label: t('自动选择'), value: 'auto' }); | |
| } | |
| } | |
| setGroups(localGroupOptions); | |
| if (statusState?.status?.default_use_auto_group) { | |
| setInputs({ ...inputs, group: 'auto' }); | |
| } | |
| if (statusState?.status?.default_use_auto_group) { | |
| // if contain auto, add it to the first position | |
| if (localGroupOptions.some((group) => group.value === 'auto')) { | |
| // 排序 | |
| localGroupOptions.sort((a, b) => (a.value === 'auto' ? -1 : 1)); | |
| } else { | |
| localGroupOptions.unshift({ label: t('自动选择'), value: 'auto' }); | |
| } | |
| } | |
| setGroups(localGroupOptions); | |
| if (!isEdit && statusState?.status?.default_use_auto_group) { | |
| // only default to auto for NEW tokens | |
| setInputs((prev) => ({ ...prev, group: 'auto' })); | |
| } |
🤖 Prompt for AI Agents
In web/src/pages/Token/EditToken.js around lines 124 to 136, the code
unconditionally sets the input group to 'auto' if default_use_auto_group is
true, which overwrites the existing token's group when editing. To fix this,
update the setInputs call to use the functional form and only set group to
'auto' if the current input group is not already set or different. Also, apply a
similar conditional guard to the insertion and sorting logic for 'auto' in
localGroupOptions to keep the UI consistent and avoid unnecessary changes.
| func UpdateAutoGroupsByJsonString(jsonString string) error { | ||
| AutoGroups = make([]string, 0) | ||
| return json.Unmarshal([]byte(jsonString), &AutoGroups) | ||
| } |
There was a problem hiding this comment.
State is lost on JSON unmarshal error
UpdateAutoGroupsByJsonString clears AutoGroups before checking for errors.
If jsonString is malformed you end up with an empty config.
-func UpdateAutoGroupsByJsonString(jsonString string) error {
- AutoGroups = make([]string, 0)
- return json.Unmarshal([]byte(jsonString), &AutoGroups)
+func UpdateAutoGroupsByJsonString(jsonString string) error {
+ var tmp []string
+ if err := json.Unmarshal([]byte(jsonString), &tmp); err != nil {
+ return err
+ }
+ AutoGroups = tmp
+ return nil
}📝 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.
| func UpdateAutoGroupsByJsonString(jsonString string) error { | |
| AutoGroups = make([]string, 0) | |
| return json.Unmarshal([]byte(jsonString), &AutoGroups) | |
| } | |
| func UpdateAutoGroupsByJsonString(jsonString string) error { | |
| var tmp []string | |
| if err := json.Unmarshal([]byte(jsonString), &tmp); err != nil { | |
| return err | |
| } | |
| AutoGroups = tmp | |
| return nil | |
| } |
🤖 Prompt for AI Agents
In setting/auto_group.go around lines 20 to 23, the function
UpdateAutoGroupsByJsonString clears the AutoGroups slice before unmarshaling the
JSON, which causes loss of the previous state if unmarshaling fails. To fix
this, first attempt to unmarshal the JSON into a temporary variable, and only if
unmarshaling succeeds, assign the result to AutoGroups. This preserves the
original AutoGroups if the JSON is malformed.
| var AutoGroups = []string{ | ||
| "default", | ||
| } | ||
|
|
||
| var DefaultUseAutoGroup = false |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Global slice mutated without synchronisation
AutoGroups is read/written from multiple goroutines (HTTP handlers, cron jobs, …).
Without a mutex you risk data races under -race.
-var AutoGroups = []string{ "default" }
+var (
+ autoGroups = []string{"default"}
+ autoGroupsLock sync.RWMutex
+)
+
+func AutoGroups() []string {
+ autoGroupsLock.RLock()
+ defer autoGroupsLock.RUnlock()
+ return append([]string(nil), autoGroups...)
+}…and adapt setters/getters similarly.
📝 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.
| var AutoGroups = []string{ | |
| "default", | |
| } | |
| var DefaultUseAutoGroup = false | |
| var ( | |
| autoGroups = []string{"default"} | |
| autoGroupsLock sync.RWMutex | |
| ) | |
| func AutoGroups() []string { | |
| autoGroupsLock.RLock() | |
| defer autoGroupsLock.RUnlock() | |
| return append([]string(nil), autoGroups...) | |
| } | |
| var DefaultUseAutoGroup = false |
🤖 Prompt for AI Agents
In setting/auto_group.go around lines 5 to 9, the global slice AutoGroups is
accessed concurrently from multiple goroutines without synchronization, risking
data races. Introduce a mutex to protect all accesses and modifications to
AutoGroups. Refactor any setters and getters to lock and unlock this mutex
appropriately when reading or writing the slice to ensure thread safety.
| autoGroup, exists := ctx.Get("auto_group") | ||
| if exists { | ||
| groupRatio = setting.GetGroupRatio(autoGroup.(string)) | ||
| log.Printf("final group ratio: %f", groupRatio) | ||
| relayInfo.Group = autoGroup.(string) | ||
| } | ||
|
|
||
| actualGroupRatio := groupRatio | ||
| userGroupRatio, ok := setting.GetGroupGroupRatio(relayInfo.UserGroup, relayInfo.Group) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
auto_group handling duplicated in three places
The override logic (ctx.Get("auto_group") → fetch ratio → log) is copy-pasted in PreWssConsumeQuota, PostWssConsumeQuota, and PostAudioConsumeQuota.
Extract to a small helper:
func overrideGroupRatio(ctx *gin.Context, relayInfo *relaycommon.RelayInfo) float64 {
if g, ok := ctx.Get("auto_group"); ok {
r := setting.GetGroupRatio(g.(string))
if common.DebugEnabled {
log.Printf("final group ratio: %f", r)
}
relayInfo.Group = g.(string)
return r
}
return setting.GetGroupRatio(relayInfo.Group)
}🤖 Prompt for AI Agents
In service/quota.go around lines 100 to 108, the logic for handling "auto_group"
from the context and overriding the group ratio is duplicated in multiple
functions. To fix this, extract this logic into a helper function that takes the
context and relayInfo as parameters, checks for "auto_group", updates
relayInfo.Group if present, logs the ratio if debugging is enabled, and returns
the appropriate group ratio. Replace the duplicated code in the three functions
with calls to this new helper function.
| log.Printf("autoGroup: %s", autoGroup) | ||
| channel, _ = getRandomSatisfiedChannel(autoGroup, model, retry) | ||
| if channel == nil { | ||
| continue | ||
| } else { | ||
| c.Set("auto_group", autoGroup) | ||
| selectGroup = autoGroup | ||
| log.Printf("selectGroup: %s", selectGroup) | ||
| break |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Inconsistent logging API – use common.SysLog for uniformity
Lines 91 & 98 rely on log.Printf, whereas the rest of the file (e.g. line 71) uses common.SysLog.
Mixing logging back-ends complicates log collection and formatting.
- log.Printf("autoGroup: %s", autoGroup)
+ common.SysLog(fmt.Sprintf("autoGroup: %s", autoGroup))
…
- log.Printf("selectGroup: %s", selectGroup)
+ common.SysLog(fmt.Sprintf("selectGroup: %s", selectGroup))🤖 Prompt for AI Agents
In model/cache.go between lines 91 and 99, replace the log.Printf calls with
common.SysLog to maintain consistent logging across the file. Identify the
log.Printf statements on lines 91 and 98 and change them to use common.SysLog
with equivalent formatting and message content. This ensures uniform log
collection and formatting throughout the code.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
relay/relay-text.go (1)
513-514: Parameter semantics drift – rename for clarity
GenerateTextOtherInfostill names its last argumentuserGroupRatio, but the caller now passesGroupSpecialRatio.
The values align, yet the semantic mismatch is confusing. Either:
- Rename the parameter inside
GenerateTextOtherInfotogroupSpecialRatio, or- Add a short comment at the call site to clarify the mapping.
No functional bug, just readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
controller/channel-test.go(2 hunks)model/cache.go(2 hunks)model/option.go(4 hunks)relay/helper/price.go(2 hunks)relay/relay-image.go(1 hunks)relay/relay-text.go(2 hunks)relay/websocket.go(3 hunks)service/quota.go(12 hunks)web/src/pages/Setting/Operation/GroupRatioSettings.js(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- controller/channel-test.go
🚧 Files skipped from review as they are similar to previous changes (5)
- web/src/pages/Setting/Operation/GroupRatioSettings.js
- model/cache.go
- model/option.go
- service/quota.go
- relay/helper/price.go
🧰 Additional context used
🧬 Code Graph Analysis (2)
relay/relay-image.go (2)
relay/helper/price.go (1)
GroupRatioInfo(14-17)common/constants.go (1)
QuotaPerUnit(21-21)
relay/relay-text.go (2)
relay/helper/price.go (1)
GroupRatioInfo(14-17)service/log_info_generate.go (1)
GenerateTextOtherInfo(10-32)
🔇 Additional comments (3)
relay/relay-image.go (1)
165-166: Guard against nil / zeroGroupRatioInfo.GroupRatiobefore quota calculation
quotanow relies onpriceData.GroupRatioInfo.GroupRatio.
IfGroupRatioInfois a pointer and happens to benil, or ifGroupRatiois0(e.g. when no group/auto-group resolution succeeded), the multiplication silently yields0, letting the request escape billing.Consider validating the ratio before use:
- quota = int(priceData.ModelPrice * priceData.GroupRatioInfo.GroupRatio * common.QuotaPerUnit) +gr := priceData.GroupRatioInfo.GroupRatio +if gr <= 0 { + return service.OpenAIErrorWrapperLocal( + fmt.Errorf("invalid group ratio: %v", gr), + "invalid_group_ratio", http.StatusInternalServerError) +} +quota = int(priceData.ModelPrice * gr * common.QuotaPerUnit)This prevents accidental free quota and avoids a potential nil-pointer panic.
relay/websocket.go (2)
41-48: WebSocket path now depends onModelPriceHelper– double-check pre-consume logic
ModelPriceHelperis invoked withpromptTokens = 0andmaxTokens = 0, thenpriceData.ShouldPreConsumedQuotais used for pre-consumption.If
ShouldPreConsumedQuotais derived from token counts, it may now always be zero, bypassing the safety deposit that previously protected long-running WS sessions.Please verify the helper’s implementation for the WS code path.
85-86: Compile-time check only
service.PostWssConsumeQuotanow receivespriceData. Ensure its signature was updated accordingly across the codebase; otherwise this will fail to compile.
| groupRatio := priceData.GroupRatioInfo.GroupRatio | ||
| modelPrice := priceData.ModelPrice |
There was a problem hiding this comment.
Same nil / zero-ratio risk as in image helper
groupRatio := priceData.GroupRatioInfo.GroupRatio inherits the same pitfalls:
a missing or zero ratio collapses all subsequent quota math (ratio := dModelRatio.Mul(dGroupRatio)), effectively charging nothing.
Add a sanity check right after assignment (or inside ModelPriceHelper) to ensure groupRatio > 0, otherwise abort with an error.
🤖 Prompt for AI Agents
In relay/relay-text.go around lines 364 to 365, the assignment of groupRatio
from priceData.GroupRatioInfo.GroupRatio risks being zero or nil, which causes
incorrect quota calculations and results in no charge. Add a check immediately
after this assignment to verify that groupRatio is greater than zero; if not,
return or raise an error to abort processing. This validation ensures that
subsequent math using groupRatio is safe and meaningful.
2 participants
Summary by CodeRabbit
New Features
Improvements
Bug Fixes