Open
Conversation
WalkthroughAdds priority-based selection for API keys and sub-groups, database-persisted key weights, configurable validation payload modes for channel tests, new endpoints and UI for editing weights, and supporting tests and model/config changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant KeyPool as KeyPool Service
participant DB as Database
participant Cache as Memory/Cache
participant Enc as EncryptionSvc
Client->>KeyPool: SelectKey(groupID)
KeyPool->>DB: Load Group config (including KeySelectionMode)
alt key_selection_mode == "priority"
KeyPool->>DB: Query APIKeys (WHERE active ORDER BY weight DESC, id ASC LIMIT 1)
DB-->>KeyPool: APIKey record
KeyPool->>Enc: Decrypt(APIKey.KeyValue)
alt decrypt succeeds
Enc-->>KeyPool: plaintext key
else decrypt fails
Enc-->>KeyPool: error
KeyPool->>KeyPool: fallback to stored KeyValue
end
KeyPool->>Cache: Store selected key map (includes weight)
else
KeyPool->>Cache: Use rotate/active_keys list selection (existing flow)
end
KeyPool-->>Client: Return selected API key
sequenceDiagram
participant ChannelValidator
participant PayloadBuilder as BuildValidationPayload
participant ChannelCfg as Channel Config (mode, model)
participant HTTP as HTTP Client
participant RemoteAPI as Remote Model API
ChannelValidator->>PayloadBuilder: BuildValidationPayload(channelType, mode, model)
PayloadBuilder->>ChannelCfg: Resolve/derive mode if empty
alt mode == "chat"
PayloadBuilder->>PayloadBuilder: build {model, messages:[{role:user, content:"hi"}]}
else mode == "responses_simple"
PayloadBuilder->>PayloadBuilder: build {model, input: "hi"}
else mode == "responses_messages"
PayloadBuilder->>PayloadBuilder: build {model, input:[{role:user, content:"hi"}]}
else
PayloadBuilder-->>ChannelValidator: error (unsupported mode)
end
PayloadBuilder-->>ChannelValidator: payload
ChannelValidator->>HTTP: POST payload to RemoteAPI
HTTP->>RemoteAPI: send request
RemoteAPI-->>HTTP: response (200/other)
HTTP-->>ChannelValidator: success/failure
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (4)
.gitignore (1)
216-216: Anchor the ignore rule if this is meant to be repo-root only.
openspec/matches any nestedopenspecdirectory. If the intent is only the top-level folder, prefer/openspec/for precision.Suggested tweak
-openspec/ +/openspec/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore at line 216, The .gitignore entry "openspec/" currently matches any nested openspec directory; change it to an anchored rule "/openspec/" so it only ignores the top-level repository folder named openspec. Update the "openspec/" line in .gitignore to "/openspec/" to scope the ignore to the repo root.web/src/types/models.ts (1)
24-24: Consider makingweightoptional for backward compatibility.Making
weight: numberrequired will force every code path that constructs anAPIKey(e.g., mocks, partial objects, test fixtures) to supply a value even though the field is new. Since the backend defaults it to0and older serialized payloads may not include it everywhere,weight?: number(orweight: numberwith a documented default at read sites) may be a safer typing choice. Not a blocker if all producers are under your control.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/types/models.ts` at line 24, Change the APIKey model so the new field "weight" is optional: update the APIKey interface/type to use "weight?: number" (or keep it required but ensure all constructors supply a value), and audit any consumers (tests/mocks/fixtures) that construct APIKey instances to either omit weight or explicitly set it; also ensure read sites that consume APIKey (where weight may be undefined) apply the backend default (0) when accessing weight.web/src/api/keys.ts (1)
143-145: Nit: add a comment header for consistency.All other methods in this module have a Chinese comment summarizing their purpose (e.g.,
// 更新密钥备注just above). Please add one forupdateKeyWeightto match the style.Proposed diff
+ // 更新密钥优先级/权重 async updateKeyWeight(keyId: number, weight: number): Promise<void> { await http.put(`/keys/${keyId}/weight`, { weight }, { hideMessage: true }); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/api/keys.ts` around lines 143 - 145, Add a short Chinese comment header above the updateKeyWeight function to match the module's style (similar to other methods which use comments like “// 更新密钥备注”); locate the async function updateKeyWeight(keyId: number, weight: number): Promise<void> and insert a single-line Chinese comment describing its purpose (e.g., “// 更新密钥权重”) immediately above it so the file remains consistent.internal/services/key_service.go (1)
244-246: Consider returning a not-found signal when no row is updated.
gorm.DB.Update(...)returnsnilerror even if no row matched theidfilter. The current handler ininternal/handler/key_handler.gopre-checks existence viaFirst, so this is safe today, but the method silently succeeds for a non-existent key if called from any future code path. Consider checkingRowsAffectedand returninggorm.ErrRecordNotFoundto make the service self-contained.♻️ Proposed refactor
func (s *KeyService) UpdateKeyWeight(keyID uint, weight int) error { - return s.DB.Model(&models.APIKey{}).Where("id = ?", keyID).Update("weight", weight).Error + res := s.DB.Model(&models.APIKey{}).Where("id = ?", keyID).Update("weight", weight) + if res.Error != nil { + return res.Error + } + if res.RowsAffected == 0 { + return gorm.ErrRecordNotFound + } + return nil }Additionally, the new public method has no doc comment unlike its neighbors — please add one for
golint/reviveconsistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/services/key_service.go` around lines 244 - 246, The UpdateKeyWeight method on KeyService currently calls s.DB.Model(&models.APIKey{}).Where("id = ?", keyID).Update("weight", weight) which returns nil even when no rows match; update the method (KeyService.UpdateKeyWeight) to check the returned gorm DB result.RowsAffected and if zero return gorm.ErrRecordNotFound so callers get a not-found signal, and add a doc comment above UpdateKeyWeight matching the style of other methods; keep the existing behavior of returning any underlying DB error when present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/channel/validation_payload.go`:
- Around line 19-57: Add explicit validation at the start of
BuildValidationPayload to reject incompatible mode/channel combinations before
constructing any payload: if mode is models.ValidationPayloadModeResponsesSimple
or models.ValidationPayloadModeResponsesMessages, ensure channelType ==
"openai-response" (otherwise return an error); if mode is
models.ValidationPayloadModeChat, reject when channelType == "openai-response"
(return an error); keep the existing switch body unchanged but assume the guard
has already validated the mode/channel pair. Use the named symbols
models.ValidationPayloadModeResponsesSimple,
models.ValidationPayloadModeResponsesMessages, models.ValidationPayloadModeChat
and the channelType string "openai-response" to locate and implement the checks.
In `@internal/handler/key_handler.go`:
- Around line 506-508: Change the UpdateKeyWeightRequest.Weight field from int
to *int so you can distinguish “missing” from an explicit zero (i.e., use Weight
*int `json:"weight"`), then update any validation in the handler (e.g., the code
that reads UpdateKeyWeightRequest in the key update endpoint) to reject requests
where Weight is nil and only accept/apply the provided value when Weight != nil;
apply the same pointer-and-nil-check pattern to other request structs in this
file that accept a weight (the ones referenced around lines 560-586).
In `@internal/keypool/provider.go`:
- Around line 39-46: The change added a DB read on every key-selection request
by checking p.db and loading models.Group to call group.KeySelectionMode(),
which makes the hot path DB-bound; revert this by reading/passing the group's
selection mode from the existing group cache (use the same cache invalidation
path you already have) instead of doing p.db.Select(...) on every request, and
only call selectPriorityKey(groupID) when the cached mode equals
models.SelectionModePriority; if you must keep a DB-backed fallback for priority
selection, ensure the priority query used by selectPriorityKey has an
appropriate composite index to avoid creating a DB throughput bottleneck.
In `@web/src/components/keys/EditSubGroupWeightModal.vue`:
- Around line 46-50: The component currently swaps the label via the metricLabel
computed but still renders weighted-only UI (the "Preview Percentage" display
and the weight preview note); update the template and any related
computed/conditional logic to hide the preview percentage element and the
weight-preview note when props.aggregateGroup?.config?.sub_group_selection_mode
=== "priority". Use the existing metricLabel computed and the same
props.aggregateGroup?.config?.sub_group_selection_mode check to conditionally
render (or omit) the preview percentage block and the weight preview note in the
modal so that they only appear when mode !== "priority".
In `@web/src/components/keys/GroupFormModal.vue`:
- Around line 168-177: The keySelectionModeOptions array is created once at
setup() so its labels won't update when locale changes; convert
keySelectionModeOptions into a computed like validationPayloadModeOptions so it
re-runs t(...) on locale change — replace the top-level const
keySelectionModeOptions with a computed returning [{ label:
t("keys.weightMode"), value: "weighted" }, { label: t("keys.priorityMode"),
value: "priority" }] and ensure n-select uses this computed property
(referencing keySelectionModeOptions and validationPayloadModeOptions symbols).
- Around line 260-262: The code unconditionally resets
formData.validation_payload_mode on channel changes which loses user selections;
change this to only set validation_payload_mode when the current value equals
the previous channel default or the field is not user-modified (use the same
userModifiedFields pattern used for test_model/first upstream URL) — consult
defaultValidationPayloadMode(formData.channel_type) and compare against the
prior default before assigning, and mark validation_payload_mode in
userModifiedFields when the user edits it. Also update handleSubmit to avoid
writing config.validation_payload_mode when channel_type === 'gemini' (either
omit the key or clear it before submit) so hidden fields don't persist
unexpectedly.
In `@web/src/components/keys/KeyTable.vue`:
- Around line 101-102: The editingWeight ref should be nullable to match
n-input-number's contract: change the declaration of editingWeight to a nullable
type (e.g., ref<number | null>(null) or equivalent) and update any other similar
refs in the file (the refs around the other occurrences mentioned) to be
nullable as well; then, before persisting/updating (the save/update handler that
reads editingWeight.value), normalize the value (e.g., coerce null to a safe
default or validate and reject) so the save uses a non-null number. Ensure
references to weightDialogShow remain unchanged but use the nullable
editingWeight.value in UI and save logic consistently.
In `@web/src/components/keys/SubGroupTable.vue`:
- Around line 124-134: The UI currently always shows weighted percentage and
proportional bars even when selection mode is "priority"; update the logic to
hide weighted-only visuals when
props.selectedGroup?.config?.sub_group_selection_mode === "priority".
Specifically, adjust the computed values and template guards around metricLabel
and sortHint (computed properties named metricLabel and sortHint) and wrap the
parts that render subGroup.percentage and the proportional bar (the template
blocks referenced around the other ranges: the code near lines 252-269 and
393-405) so they only render when sub_group_selection_mode !== "priority" (i.e.,
show percentage/bar only in weighted mode). Ensure labels and hints still
reflect priority mode but the percentage value and bar are conditionally omitted
in priority mode.
In `@web/src/locales/zh-CN.ts`:
- Around line 253-265: Add the 14 missing i18n keys to the ja-JP.ts locale
object so it matches en-US/zh-CN; specifically add entries for editPriority,
priority, priorityShort, enterPriority, priorityUpdated, weightMode,
priorityMode, keySelectionMode, subGroupSelectionMode, validationPayloadMode,
validationPayloadChat, validationPayloadResponsesSimple,
validationPayloadResponsesMessages and sortedByPriority in the ja-JP.ts export
and provide appropriate Japanese translations for each key so the priority UI no
longer shows raw i18n keys.
---
Nitpick comments:
In @.gitignore:
- Line 216: The .gitignore entry "openspec/" currently matches any nested
openspec directory; change it to an anchored rule "/openspec/" so it only
ignores the top-level repository folder named openspec. Update the "openspec/"
line in .gitignore to "/openspec/" to scope the ignore to the repo root.
In `@internal/services/key_service.go`:
- Around line 244-246: The UpdateKeyWeight method on KeyService currently calls
s.DB.Model(&models.APIKey{}).Where("id = ?", keyID).Update("weight", weight)
which returns nil even when no rows match; update the method
(KeyService.UpdateKeyWeight) to check the returned gorm DB result.RowsAffected
and if zero return gorm.ErrRecordNotFound so callers get a not-found signal, and
add a doc comment above UpdateKeyWeight matching the style of other methods;
keep the existing behavior of returning any underlying DB error when present.
In `@web/src/api/keys.ts`:
- Around line 143-145: Add a short Chinese comment header above the
updateKeyWeight function to match the module's style (similar to other methods
which use comments like “// 更新密钥备注”); locate the async function
updateKeyWeight(keyId: number, weight: number): Promise<void> and insert a
single-line Chinese comment describing its purpose (e.g., “// 更新密钥权重”)
immediately above it so the file remains consistent.
In `@web/src/types/models.ts`:
- Line 24: Change the APIKey model so the new field "weight" is optional: update
the APIKey interface/type to use "weight?: number" (or keep it required but
ensure all constructors supply a value), and audit any consumers
(tests/mocks/fixtures) that construct APIKey instances to either omit weight or
explicitly set it; also ensure read sites that consume APIKey (where weight may
be undefined) apply the backend default (0) when accessing weight.
🪄 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: 5329c248-3db2-4966-9c0f-a3919432b765
📒 Files selected for processing (27)
.gitignoreinternal/channel/anthropic_channel.gointernal/channel/base_channel.gointernal/channel/factory.gointernal/channel/openai_channel.gointernal/channel/openai_response_channel.gointernal/channel/validation_payload.gointernal/channel/validation_payload_test.gointernal/handler/key_handler.gointernal/keypool/provider.gointernal/keypool/provider_priority_test.gointernal/models/types.gointernal/router/router.gointernal/services/group_service.gointernal/services/group_service_config_test.gointernal/services/key_service.gointernal/services/subgroup_manager.gointernal/services/subgroup_manager_priority_test.goweb/src/api/keys.tsweb/src/components/keys/AggregateGroupModal.vueweb/src/components/keys/EditSubGroupWeightModal.vueweb/src/components/keys/GroupFormModal.vueweb/src/components/keys/KeyTable.vueweb/src/components/keys/SubGroupTable.vueweb/src/locales/en-US.tsweb/src/locales/zh-CN.tsweb/src/types/models.ts
Comment on lines
+19
to
+57
| switch mode { | ||
| case models.ValidationPayloadModeResponsesSimple: | ||
| return map[string]any{ | ||
| "model": model, | ||
| "input": "hi", | ||
| }, nil | ||
| case models.ValidationPayloadModeResponsesMessages: | ||
| return map[string]any{ | ||
| "model": model, | ||
| "input": []map[string]any{ | ||
| { | ||
| "type": "message", | ||
| "role": "user", | ||
| "content": []map[string]any{ | ||
| { | ||
| "type": "input_text", | ||
| "text": "hi", | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, nil | ||
| case models.ValidationPayloadModeChat: | ||
| payload := map[string]any{ | ||
| "model": model, | ||
| "messages": []map[string]any{ | ||
| { | ||
| "role": "user", | ||
| "content": "hi", | ||
| }, | ||
| }, | ||
| } | ||
| if channelType == "anthropic" { | ||
| payload["max_tokens"] = 100 | ||
| } | ||
| return payload, nil | ||
| default: | ||
| return nil, fmt.Errorf("unsupported validation payload mode: %s", mode) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the validation_payload.go file and examine its content
find . -name "validation_payload.go" -type fRepository: tbphp/gpt-load
Length of output: 98
🏁 Script executed:
# Search for allowlist configuration related to validation payload modes
rg -i "allowlist|validation.*payload.*mode" --type go -A 3 -B 3Repository: tbphp/gpt-load
Length of output: 11363
🏁 Script executed:
# Find channel type definitions and supported modes
rg "ValidationPayloadMode" --type go -B 2 -A 2Repository: tbphp/gpt-load
Length of output: 6842
🏁 Script executed:
# Search for how BuildValidationPayload is called
rg "BuildValidationPayload" --type go -B 3 -A 3Repository: tbphp/gpt-load
Length of output: 4315
Add mode/channel validation guards to prevent incompatible payload structures.
The validation mode allowlist accepts all modes for all groups. BuildValidationPayload must reject mismatches before constructing payloads: responses_* modes are incompatible with non-openai-response channels, and chat is incompatible with openai-response.
🛡️ Proposed guard
switch mode {
case models.ValidationPayloadModeResponsesSimple:
+ if channelType != "openai-response" {
+ return nil, fmt.Errorf("validation payload mode %s is not supported for channel %s", mode, channelType)
+ }
return map[string]any{
"model": model,
"input": "hi",
}, nil
case models.ValidationPayloadModeResponsesMessages:
+ if channelType != "openai-response" {
+ return nil, fmt.Errorf("validation payload mode %s is not supported for channel %s", mode, channelType)
+ }
return map[string]any{
"model": model,
"input": []map[string]any{
@@
}, nil
case models.ValidationPayloadModeChat:
+ if channelType == "openai-response" {
+ return nil, fmt.Errorf("validation payload mode %s is not supported for channel %s", mode, channelType)
+ }
payload := map[string]any{
"model": model,
"messages": []map[string]any{📝 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.
Suggested change
| switch mode { | |
| case models.ValidationPayloadModeResponsesSimple: | |
| return map[string]any{ | |
| "model": model, | |
| "input": "hi", | |
| }, nil | |
| case models.ValidationPayloadModeResponsesMessages: | |
| return map[string]any{ | |
| "model": model, | |
| "input": []map[string]any{ | |
| { | |
| "type": "message", | |
| "role": "user", | |
| "content": []map[string]any{ | |
| { | |
| "type": "input_text", | |
| "text": "hi", | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, nil | |
| case models.ValidationPayloadModeChat: | |
| payload := map[string]any{ | |
| "model": model, | |
| "messages": []map[string]any{ | |
| { | |
| "role": "user", | |
| "content": "hi", | |
| }, | |
| }, | |
| } | |
| if channelType == "anthropic" { | |
| payload["max_tokens"] = 100 | |
| } | |
| return payload, nil | |
| default: | |
| return nil, fmt.Errorf("unsupported validation payload mode: %s", mode) | |
| } | |
| switch mode { | |
| case models.ValidationPayloadModeResponsesSimple: | |
| if channelType != "openai-response" { | |
| return nil, fmt.Errorf("validation payload mode %s is not supported for channel %s", mode, channelType) | |
| } | |
| return map[string]any{ | |
| "model": model, | |
| "input": "hi", | |
| }, nil | |
| case models.ValidationPayloadModeResponsesMessages: | |
| if channelType != "openai-response" { | |
| return nil, fmt.Errorf("validation payload mode %s is not supported for channel %s", mode, channelType) | |
| } | |
| return map[string]any{ | |
| "model": model, | |
| "input": []map[string]any{ | |
| { | |
| "type": "message", | |
| "role": "user", | |
| "content": []map[string]any{ | |
| { | |
| "type": "input_text", | |
| "text": "hi", | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, nil | |
| case models.ValidationPayloadModeChat: | |
| if channelType == "openai-response" { | |
| return nil, fmt.Errorf("validation payload mode %s is not supported for channel %s", mode, channelType) | |
| } | |
| payload := map[string]any{ | |
| "model": model, | |
| "messages": []map[string]any{ | |
| { | |
| "role": "user", | |
| "content": "hi", | |
| }, | |
| }, | |
| } | |
| if channelType == "anthropic" { | |
| payload["max_tokens"] = 100 | |
| } | |
| return payload, nil | |
| default: | |
| return nil, fmt.Errorf("unsupported validation payload mode: %s", mode) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/channel/validation_payload.go` around lines 19 - 57, Add explicit
validation at the start of BuildValidationPayload to reject incompatible
mode/channel combinations before constructing any payload: if mode is
models.ValidationPayloadModeResponsesSimple or
models.ValidationPayloadModeResponsesMessages, ensure channelType ==
"openai-response" (otherwise return an error); if mode is
models.ValidationPayloadModeChat, reject when channelType == "openai-response"
(return an error); keep the existing switch body unchanged but assume the guard
has already validated the mode/channel pair. Use the named symbols
models.ValidationPayloadModeResponsesSimple,
models.ValidationPayloadModeResponsesMessages, models.ValidationPayloadModeChat
and the channelType string "openai-response" to locate and implement the checks.
Comment on lines
+506
to
+508
| type UpdateKeyWeightRequest struct { | ||
| Weight int `json:"weight"` | ||
| } |
There was a problem hiding this comment.
Require weight explicitly before accepting zero.
With Weight int, {} or {"weight": null} binds to 0 and passes validation, which can unintentionally disable/deprioritize a key. Use a pointer so “missing” is distinguishable from an intentional zero.
🛡️ Proposed fix: distinguish missing weight from zero
type UpdateKeyWeightRequest struct {
- Weight int `json:"weight"`
+ Weight *int `json:"weight"`
} var req UpdateKeyWeightRequest
if err := c.ShouldBindJSON(&req); err != nil {
response.Error(c, app_errors.NewAPIError(app_errors.ErrInvalidJSON, err.Error()))
return
}
- if req.Weight < 0 {
+ if req.Weight == nil {
+ response.Error(c, app_errors.NewAPIError(app_errors.ErrValidation, "weight is required"))
+ return
+ }
+
+ if *req.Weight < 0 {
response.Error(c, app_errors.NewAPIError(app_errors.ErrValidation, "weight must be >= 0"))
return
}
- if req.Weight > 1000 {
+ if *req.Weight > 1000 {
response.Error(c, app_errors.NewAPIError(app_errors.ErrValidation, "weight must be <= 1000"))
return
}
@@
- if err := s.KeyService.UpdateKeyWeight(uint(keyID), req.Weight); err != nil {
+ if err := s.KeyService.UpdateKeyWeight(uint(keyID), *req.Weight); err != nil {
response.Error(c, app_errors.ParseDBError(err))
return
}Also applies to: 560-586
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/handler/key_handler.go` around lines 506 - 508, Change the
UpdateKeyWeightRequest.Weight field from int to *int so you can distinguish
“missing” from an explicit zero (i.e., use Weight *int `json:"weight"`), then
update any validation in the handler (e.g., the code that reads
UpdateKeyWeightRequest in the key update endpoint) to reject requests where
Weight is nil and only accept/apply the provided value when Weight != nil; apply
the same pointer-and-nil-check pattern to other request structs in this file
that accept a weight (the ones referenced around lines 560-586).
Comment on lines
+39
to
+46
| if p.db != nil { | ||
| var group models.Group | ||
| if err := p.db.Select("id", "config").First(&group, groupID).Error; err == nil { | ||
| if group.KeySelectionMode() == models.SelectionModePriority { | ||
| return p.selectPriorityKey(groupID) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Avoid adding DB reads to every key-selection request.
Line 39 now puts a groups table lookup in front of the existing Redis rotation path for all groups, including weighted mode. Priority mode then does another DB read at Lines 101-105. This makes the request hot path DB-bound and can become a throughput bottleneck; prefer passing/caching the group selection mode with existing group-cache invalidation, and ensure the priority query has a matching composite index if it remains DB-backed.
Also applies to: 99-105
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/keypool/provider.go` around lines 39 - 46, The change added a DB
read on every key-selection request by checking p.db and loading models.Group to
call group.KeySelectionMode(), which makes the hot path DB-bound; revert this by
reading/passing the group's selection mode from the existing group cache (use
the same cache invalidation path you already have) instead of doing
p.db.Select(...) on every request, and only call selectPriorityKey(groupID) when
the cached mode equals models.SelectionModePriority; if you must keep a
DB-backed fallback for priority selection, ensure the priority query used by
selectPriorityKey has an appropriate composite index to avoid creating a DB
throughput bottleneck.
Comment on lines
+46
to
+50
| const metricLabel = computed(() => | ||
| props.aggregateGroup?.config?.sub_group_selection_mode === "priority" | ||
| ? t("keys.priority") | ||
| : t("subGroups.weight") | ||
| ); |
There was a problem hiding this comment.
Remove weighted preview semantics when editing priority.
The modal switches the label to “Priority”, but still shows “Preview Percentage” and the weight preview note. In priority mode, the highest active priority is selected, so this percentage is not meaningful.
🎯 Proposed fix: make priority mode hide weighted-only preview
+const isPriorityMode = computed(
+ () => props.aggregateGroup?.config?.sub_group_selection_mode === "priority"
+);
+
const metricLabel = computed(() =>
- props.aggregateGroup?.config?.sub_group_selection_mode === "priority"
+ isPriorityMode.value
? t("keys.priority")
: t("subGroups.weight")
); :title="
- props.aggregateGroup?.config?.sub_group_selection_mode === 'priority'
+ isPriorityMode
? t('keys.editPriority')
: t('keys.editWeight')
"- :placeholder="t('keys.enterWeight')"
+ :placeholder="isPriorityMode ? t('keys.enterPriority') : t('keys.enterWeight')"- <div class="preview-section">
+ <div v-if="!isPriorityMode" class="preview-section">
<div class="preview-item">
<span class="preview-label">{{ t("keys.previewPercentage") }}:</span>
<span class="preview-value">{{ previewPercentage }}%</span>
</div>
<div class="preview-note">
{{ t("keys.weightPreviewNote") }}
</div>
</div>Also applies to: 153-157, 198-236
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/keys/EditSubGroupWeightModal.vue` around lines 46 - 50,
The component currently swaps the label via the metricLabel computed but still
renders weighted-only UI (the "Preview Percentage" display and the weight
preview note); update the template and any related computed/conditional logic to
hide the preview percentage element and the weight-preview note when
props.aggregateGroup?.config?.sub_group_selection_mode === "priority". Use the
existing metricLabel computed and the same
props.aggregateGroup?.config?.sub_group_selection_mode check to conditionally
render (or omit) the preview percentage block and the weight preview note in the
modal so that they only appear when mode !== "priority".
Comment on lines
+168
to
+177
| const keySelectionModeOptions = [ | ||
| { label: t("keys.weightMode"), value: "weighted" }, | ||
| { label: t("keys.priorityMode"), value: "priority" }, | ||
| ]; | ||
|
|
||
| const validationPayloadModeOptions = computed(() => [ | ||
| { label: t("keys.validationPayloadChat"), value: "chat" }, | ||
| { label: t("keys.validationPayloadResponsesSimple"), value: "responses_simple" }, | ||
| { label: t("keys.validationPayloadResponsesMessages"), value: "responses_messages" }, | ||
| ]); |
There was a problem hiding this comment.
Make keySelectionModeOptions reactive to locale changes.
The array is built at setup() time, so t() is resolved once. When the user switches language, the n-select labels will stay in the old locale — unlike validationPayloadModeOptions directly below, which is computed and correctly re-evaluates. Use computed for consistency:
♻️ Proposed fix
-const keySelectionModeOptions = [
- { label: t("keys.weightMode"), value: "weighted" },
- { label: t("keys.priorityMode"), value: "priority" },
-];
+const keySelectionModeOptions = computed(() => [
+ { label: t("keys.weightMode"), value: "weighted" },
+ { label: t("keys.priorityMode"), value: "priority" },
+]);📝 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.
Suggested change
| const keySelectionModeOptions = [ | |
| { label: t("keys.weightMode"), value: "weighted" }, | |
| { label: t("keys.priorityMode"), value: "priority" }, | |
| ]; | |
| const validationPayloadModeOptions = computed(() => [ | |
| { label: t("keys.validationPayloadChat"), value: "chat" }, | |
| { label: t("keys.validationPayloadResponsesSimple"), value: "responses_simple" }, | |
| { label: t("keys.validationPayloadResponsesMessages"), value: "responses_messages" }, | |
| ]); | |
| const keySelectionModeOptions = computed(() => [ | |
| { label: t("keys.weightMode"), value: "weighted" }, | |
| { label: t("keys.priorityMode"), value: "priority" }, | |
| ]); | |
| const validationPayloadModeOptions = computed(() => [ | |
| { label: t("keys.validationPayloadChat"), value: "chat" }, | |
| { label: t("keys.validationPayloadResponsesSimple"), value: "responses_simple" }, | |
| { label: t("keys.validationPayloadResponsesMessages"), value: "responses_messages" }, | |
| ]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/keys/GroupFormModal.vue` around lines 168 - 177, The
keySelectionModeOptions array is created once at setup() so its labels won't
update when locale changes; convert keySelectionModeOptions into a computed like
validationPayloadModeOptions so it re-runs t(...) on locale change — replace the
top-level const keySelectionModeOptions with a computed returning [{ label:
t("keys.weightMode"), value: "weighted" }, { label: t("keys.priorityMode"),
value: "priority" }] and ensure n-select uses this computed property
(referencing keySelectionModeOptions and validationPayloadModeOptions symbols).
Comment on lines
+260
to
262
|
|
||
| formData.validation_payload_mode = defaultValidationPayloadMode(formData.channel_type); | ||
| } |
There was a problem hiding this comment.
validation_payload_mode is silently overwritten on channel type change.
Unlike test_model and the first upstream URL (which respect userModifiedFields and only update when untouched or still equal to the previous channel default), validation_payload_mode is reset unconditionally. A user who explicitly picked e.g. responses_messages and then tweaks channel_type will lose that selection without notice. Track user modification consistently, or at least only overwrite when the current value still equals the previous channel's default:
♻️ Suggested behavior
- formData.validation_payload_mode = defaultValidationPayloadMode(formData.channel_type);
+ if (
+ formData.validation_payload_mode === defaultValidationPayloadMode(oldChannelType)
+ ) {
+ formData.validation_payload_mode = defaultValidationPayloadMode(formData.channel_type);
+ }Secondary note: in handleSubmit (Line 570), config.validation_payload_mode is written even when channel_type === 'gemini' where the field is hidden from the UI. The value will persist and could unexpectedly take effect if the group is later switched back to a non-gemini channel. Consider omitting the field from the submitted config for gemini, or clearing it on channel switch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/keys/GroupFormModal.vue` around lines 260 - 262, The code
unconditionally resets formData.validation_payload_mode on channel changes which
loses user selections; change this to only set validation_payload_mode when the
current value equals the previous channel default or the field is not
user-modified (use the same userModifiedFields pattern used for test_model/first
upstream URL) — consult defaultValidationPayloadMode(formData.channel_type) and
compare against the prior default before assigning, and mark
validation_payload_mode in userModifiedFields when the user edits it. Also
update handleSubmit to avoid writing config.validation_payload_mode when
channel_type === 'gemini' (either omit the key or clear it before submit) so
hidden fields don't persist unexpectedly.
Comment on lines
+101
to
+102
| const weightDialogShow = ref(false); | ||
| const editingWeight = ref(0); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the KeyTable.vue file
find . -name "KeyTable.vue" -type f | head -5Repository: tbphp/gpt-load
Length of output: 96
🏁 Script executed:
# Let's read the file to see the current state of the code
wc -l web/src/components/keys/KeyTable.vueRepository: tbphp/gpt-load
Length of output: 99
🏁 Script executed:
# Read the relevant sections - start with lines 95-110 to see the variable declarations
sed -n '95,110p' web/src/components/keys/KeyTable.vueRepository: tbphp/gpt-load
Length of output: 479
🏁 Script executed:
# Read the saveKeyWeight function at lines 338-345
sed -n '330,350p' web/src/components/keys/KeyTable.vueRepository: tbphp/gpt-load
Length of output: 706
🏁 Script executed:
# Read the n-input-number binding at lines 900-920
sed -n '900,920p' web/src/components/keys/KeyTable.vueRepository: tbphp/gpt-load
Length of output: 708
🏁 Script executed:
# Let's also check the imports to understand what version of naive-ui is being used
head -30 web/src/components/keys/KeyTable.vueRepository: tbphp/gpt-load
Length of output: 707
🏁 Script executed:
# Check if there's a package.json to see the naive-ui version
cat web/package.json | grep -A 2 "naive-ui"Repository: tbphp/gpt-load
Length of output: 259
🌐 Web query:
naive-ui n-input-number v-model null behavior when cleared
💡 Result:
In Naive UI's n-input-number component, when using v-model with a value initialized as null and the input is cleared (using the clearable feature if enabled), the v-model value becomes null. This behavior was specifically fixed in early development via PR #242 titled "fix(n-input-number): clear input box but value not null", which addressed the issue where clearing the input box did not set the value to null. The component supports a clearable prop (added in PR #868) that shows a clear button, and clicking it or manually deleting the content sets v-model to null when the value is controlled. Naive UI components distinguish between controlled (v-model provides value) and uncontrolled (uses internal state) modes. In controlled mode with v-model:null, clearing properly updates to null, unlike undefined which treats the component as uncontrolled and retains internal display state (as noted in issue #3876 for n-input, with similar logic applying). Examples from integrations like form-create confirm clearable: true usage with v-model, where clearing sets value to null.
Citations:
- 1: fix(n-input-number): clear input box but value not null tusen-ai/naive-ui#242
- 2: feat(NInputNumber): add clearable prop tusen-ai/naive-ui#868
- 3: Setting value of n-input to undefined does not clear entered text tusen-ai/naive-ui#3876
- 4: https://www.form-create.com/v3/naive-ui/components/input-number
Type the weight input as nullable.
n-input-number emits null when cleared, so align the ref type with the actual component contract and normalize explicitly before saving.
🔧 Proposed fix
-const editingWeight = ref(0);
+const editingWeight = ref<number | null>(0);
@@
- const nextWeight = Math.max(0, Math.min(1000, Math.trunc(editingWeight.value || 0)));
+ const rawWeight = editingWeight.value ?? 0;
+ const normalizedWeight = Number.isFinite(rawWeight) ? rawWeight : 0;
+ const nextWeight = Math.max(0, Math.min(1000, Math.trunc(normalizedWeight)));Also applies to: 338-345, 904-912
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/keys/KeyTable.vue` around lines 101 - 102, The
editingWeight ref should be nullable to match n-input-number's contract: change
the declaration of editingWeight to a nullable type (e.g., ref<number |
null>(null) or equivalent) and update any other similar refs in the file (the
refs around the other occurrences mentioned) to be nullable as well; then,
before persisting/updating (the save/update handler that reads
editingWeight.value), normalize the value (e.g., coerce null to a safe default
or validate and reject) so the save uses a non-null number. Ensure references to
weightDialogShow remain unchanged but use the nullable editingWeight.value in UI
and save logic consistently.
Comment on lines
+124
to
+134
| const metricLabel = computed(() => | ||
| props.selectedGroup?.config?.sub_group_selection_mode === "priority" | ||
| ? t("keys.priority") | ||
| : t("subGroups.weight") | ||
| ); | ||
|
|
||
| const sortHint = computed(() => | ||
| props.selectedGroup?.config?.sub_group_selection_mode === "priority" | ||
| ? t("subGroups.sortedByPriority") | ||
| : t("subGroups.sortedByWeight") | ||
| ); |
There was a problem hiding this comment.
Hide weighted percentage UI in priority mode.
In priority mode, subGroup.weight is used as a failover priority, not a traffic percentage. The relabeled card still shows subGroup.percentage and a proportional bar, which can make users think traffic is split by priority ratio.
🎯 Proposed fix: gate weighted-only visuals behind weighted mode
+const isPriorityMode = computed(
+ () => props.selectedGroup?.config?.sub_group_selection_mode === "priority"
+);
+
const metricLabel = computed(() =>
- props.selectedGroup?.config?.sub_group_selection_mode === "priority"
+ isPriorityMode.value
? t("keys.priority")
: t("subGroups.weight")
);
const sortHint = computed(() =>
- props.selectedGroup?.config?.sub_group_selection_mode === "priority"
+ isPriorityMode.value
? t("subGroups.sortedByPriority")
: t("subGroups.sortedByWeight")
);- <div class="weight-bar">
+ <div v-if="!isPriorityMode" class="weight-bar">
<div
class="weight-fill"
:class="{
'weight-fill-active': subGroup.weight > 0 && subGroup.active_keys > 0,
'weight-fill-unavailable': subGroup.weight > 0 && subGroup.active_keys === 0,
}"
:style="{ width: `${subGroup.percentage}%` }"
/>
</div>
- <span class="weight-text">{{ subGroup.percentage }}%</span>
+ <span v-if="!isPriorityMode" class="weight-text">{{ subGroup.percentage }}%</span>Also applies to: 252-269, 393-405
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/keys/SubGroupTable.vue` around lines 124 - 134, The UI
currently always shows weighted percentage and proportional bars even when
selection mode is "priority"; update the logic to hide weighted-only visuals
when props.selectedGroup?.config?.sub_group_selection_mode === "priority".
Specifically, adjust the computed values and template guards around metricLabel
and sortHint (computed properties named metricLabel and sortHint) and wrap the
parts that render subGroup.percentage and the proportional bar (the template
blocks referenced around the other ranges: the code near lines 252-269 and
393-405) so they only render when sub_group_selection_mode !== "priority" (i.e.,
show percentage/bar only in weighted mode). Ensure labels and hints still
reflect priority mode but the percentage value and bar are conditionally omitted
in priority mode.
Comment on lines
+253
to
+265
| editPriority: "编辑优先级", | ||
| priority: "优先级", | ||
| priorityShort: "优先", | ||
| enterPriority: "请输入优先级", | ||
| priorityUpdated: "优先级已更新", | ||
| weightMode: "权重模式", | ||
| priorityMode: "优先级模式", | ||
| keySelectionMode: "密钥选择模式", | ||
| subGroupSelectionMode: "子分组选择模式", | ||
| validationPayloadMode: "验证参数模式", | ||
| validationPayloadChat: "Chat 参数", | ||
| validationPayloadResponsesSimple: "Responses 简单参数", | ||
| validationPayloadResponsesMessages: "Responses 消息参数", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Enumerate locale files and check which of the new keys are missing.
fd . web/src/locales -t f
keys=(editPriority priority priorityShort enterPriority priorityUpdated weightMode priorityMode keySelectionMode subGroupSelectionMode validationPayloadMode validationPayloadChat validationPayloadResponsesSimple validationPayloadResponsesMessages sortedByPriority)
for f in $(fd . web/src/locales -t f); do
echo "=== $f ==="
for k in "${keys[@]}"; do
if ! rg -q "\\b${k}\\b" "$f"; then
echo "MISSING: $k"
fi
done
doneRepository: tbphp/gpt-load
Length of output: 1057
Add missing keys to ja-JP.ts locale file.
While zh-CN.ts now has parity with en-US.ts, the Japanese locale file (ja-JP.ts) is missing all 14 new keys: editPriority, priority, priorityShort, enterPriority, priorityUpdated, weightMode, priorityMode, keySelectionMode, subGroupSelectionMode, validationPayloadMode, validationPayloadChat, validationPayloadResponsesSimple, validationPayloadResponsesMessages, and sortedByPriority. Users on the Japanese locale will see raw i18n keys in the priority UI unless these are added.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/locales/zh-CN.ts` around lines 253 - 265, Add the 14 missing i18n
keys to the ja-JP.ts locale object so it matches en-US/zh-CN; specifically add
entries for editPriority, priority, priorityShort, enterPriority,
priorityUpdated, weightMode, priorityMode, keySelectionMode,
subGroupSelectionMode, validationPayloadMode, validationPayloadChat,
validationPayloadResponsesSimple, validationPayloadResponsesMessages and
sortedByPriority in the ja-JP.ts export and provide appropriate Japanese
translations for each key so the priority UI no longer shows raw i18n keys.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/docker-build.yml (1)
27-32:⚠️ Potential issue | 🟠 Major
secretscontext isn't available in step-levelifconditionals — move toenvinstead.GitHub Actions documentation confirms that
secretscontext cannot be referenced in step-levelifexpressions. Only these contexts are available:github,needs,strategy,matrix,job,runner,env,vars,steps, andinputs. The condition on line 28 must be moved to an environment variable first (either job-level or step-level), then tested viaenv.*.Proposed fix
- name: Log in to Docker Hub - if: ${{ secrets.DOCKERHUB_USERNAME != '' && secrets.DOCKERHUB_TOKEN != '' }} + if: ${{ env.HAS_DOCKERHUB == 'true' }} + env: + HAS_DOCKERHUB: ${{ secrets.DOCKERHUB_USERNAME != '' && secrets.DOCKERHUB_TOKEN != '' }} uses: docker/login-action@v3 with: username: ${{ secrets.DOCKERHUB_USERNAME }} password: ${{ secrets.DOCKERHUB_TOKEN }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docker-build.yml around lines 27 - 32, The step "Log in to Docker Hub" currently uses secrets in its step-level if (secrets.DOCKERHUB_USERNAME/DOCKERHUB_TOKEN) which is invalid; fix it by exposing those secrets into env (preferably at job-level) and then change the step if to reference env.*. Concretely, add job-level env entries DOCKERHUB_USERNAME: ${{ secrets.DOCKERHUB_USERNAME }} and DOCKERHUB_TOKEN: ${{ secrets.DOCKERHUB_TOKEN }}, then update the "Log in to Docker Hub" step's if to something like ${{ env.DOCKERHUB_USERNAME != '' && env.DOCKERHUB_TOKEN != '' }} while keeping uses: docker/login-action@v3 and with: username/password using the same env variables.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.github/workflows/docker-build.yml:
- Around line 27-32: The step "Log in to Docker Hub" currently uses secrets in
its step-level if (secrets.DOCKERHUB_USERNAME/DOCKERHUB_TOKEN) which is invalid;
fix it by exposing those secrets into env (preferably at job-level) and then
change the step if to reference env.*. Concretely, add job-level env entries
DOCKERHUB_USERNAME: ${{ secrets.DOCKERHUB_USERNAME }} and DOCKERHUB_TOKEN: ${{
secrets.DOCKERHUB_TOKEN }}, then update the "Log in to Docker Hub" step's if to
something like ${{ env.DOCKERHUB_USERNAME != '' && env.DOCKERHUB_TOKEN != '' }}
while keeping uses: docker/login-action@v3 and with: username/password using the
same env variables.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dc7a7d8c-67f2-4f87-97ff-e9c427a59bc9
📒 Files selected for processing (1)
.github/workflows/docker-build.yml
Akimixu-19897
force-pushed
the
codex/issue-396-key-priority-spec
branch
from
ac30354 to
c1c7ab3
Compare
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.
关联 Issue / Related Issue
Closes #
变更内容 / Change Content
自查清单 / Checklist
Summary by CodeRabbit
New Features
Improvements
Localization