Feature/pool rolling window quota - #4332
Closed
bllackhu wants to merge 4 commits into
Closed
Conversation
Refactor pool management tabs to use SideSheet create/edit flows, add clearer binding filtering and metadata display, and enforce duplicate binding guards per pool to reduce admin confusion and data mistakes. Made-with: Cursor
Contributor
🚥 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 |
…g-window-quota Resolve conflicts in docker-compose.yml (keep named services, align Redis URL with requirepass), web/package.json (antd + axios 1.15.0), TokensColumnDefs (Token ID + upstream quota/group order with groupRatios). Refresh web/bun.lock for axios. Made-with: Cursor
![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: 14
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/src/pages/Setting/Operation/SettingsSidebarModulesAdmin.jsx (1)
61-70:⚠️ Potential issue | 🟡 MinorSeed
admin.poolin the initial admin sidebar state.The reset and parse-fallback defaults include
pool: true, but the initial state does not. When no savedSidebarModulesAdminoption exists, the new Pool switch starts asundefined/off and can be saved that way.✅ Proposed default-state fix
admin: { enabled: true, + pool: true, channel: true, models: true, deployment: true,Also applies to: 259-259
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/Setting/Operation/SettingsSidebarModulesAdmin.jsx` around lines 61 - 70, The initial admin sidebar state in SettingsSidebarModulesAdmin.jsx is missing the pool key so the new Pool switch defaults to undefined; update the initial admin object (the admin property in the sidebar state) to include pool: true alongside enabled/channel/models/deployment/redemption/user/subscription/setting so it matches the reset and parse-fallback defaults and ensures Pool starts enabled when no saved SidebarModulesAdmin exists.middleware/distributor.go (1)
39-53:⚠️ Potential issue | 🟠 MajorApply pool membership checks to token-specific channels too.
The new pool filtering covers affinity and random selection, but the
ContextKeyTokenSpecificChannelIdbranch still accepts any enabled specific channel. A token pinned to a channel outside the selected pool bypasses pool isolation.🛡️ Proposed pool check for specific-channel tokens
var channel *model.Channel channelId, ok := common.GetContextKey(c, constant.ContextKeyTokenSpecificChannelId) + poolId := common.GetContextKeyInt(c, constant.ContextKeyPoolId) modelRequest, shouldSelectChannel, err := getModelRequest(c) if err != nil { abortWithOpenAiMessage(c, http.StatusBadRequest, i18n.T(c, i18n.MsgDistributorInvalidRequest, map[string]any{"Error": err.Error()})) return @@ if channel.Status != common.ChannelStatusEnabled { abortWithOpenAiMessage(c, http.StatusForbidden, i18n.T(c, i18n.MsgDistributorChannelDisabled)) return } + if poolId > 0 { + inPool, poolErr := model.IsChannelInPool(poolId, channel.Id) + if poolErr != nil { + abortWithOpenAiMessage(c, http.StatusInternalServerError, "failed to check specific channel in pool") + return + } + if !inPool { + abortWithOpenAiMessage(c, http.StatusForbidden, "specific channel is not available in current pool") + return + } + } } else { @@ } var selectGroup string - poolId := common.GetContextKeyInt(c, constant.ContextKeyPoolId) usingGroup := common.GetContextKeyString(c, constant.ContextKeyUsingGroup)Also applies to: 83-115, 143-150
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@middleware/distributor.go` around lines 39 - 53, The token-specific-channel branch (when using ContextKeyTokenSpecificChannelId) currently skips pool membership checks allowing a token pinned to a channel outside the selected pool; after retrieving channel via model.GetChannelById in that branch, verify the channel belongs to the active pool (e.g., compare channel.PoolID or call the existing pool-membership helper used in the pool-filter path) and abort with a forbidden error (same i18n.MsgDistributorChannelDisabled or a new message) if it does not; apply the same membership check logic to the other token-specific-channel handling sites referenced (the other ContextKeyTokenSpecificChannelId handling blocks around the other ranges) so token-pinned channels cannot bypass pool isolation.
🟡 Minor comments (10)
docker-compose.yml-29-29 (1)
29-29:⚠️ Potential issue | 🟡 MinorKeep the MySQL example credentials in sync.
The commented MySQL DSN still uses
123456, butMYSQL_ROOT_PASSWORDis nowchange-me; uncommenting the documented MySQL path will fail authentication.Proposed fix
-# - SQL_DSN=root:123456@tcp(new-api-mysql:3306)/new-api # Point to the mysql service, uncomment if using MySQL +# - SQL_DSN=root:change-me@tcp(new-api-mysql:3306)/new-api # Point to the mysql service, uncomment if using MySQLAlso applies to: 80-80
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-compose.yml` at line 29, Update the commented MySQL DSN so it matches the declared root password (MYSQL_ROOT_PASSWORD) instead of the hardcoded 123456; locate the commented line containing SQL_DSN and replace the password portion with the actual secret or a reference to the env var (e.g., use change-me or ${MYSQL_ROOT_PASSWORD}) so uncommenting SQL_DSN will authenticate correctly.web/src/components/layout/SiderBar.jsx-151-156 (1)
151-156:⚠️ Potential issue | 🟡 Minori18n key should use a Chinese source string for consistency.
All other sidebar entries in this file use Chinese strings as i18n keys (e.g.,
t('渠道管理'),t('订阅管理'),t('模型管理')). Usingt('Coding Plan')here breaks that convention — the English literal becomes the translation key, which means zh-CN renders "Coding Plan" by default and the key won't be picked up by the existing extraction workflow the same way.Proposed fix
{ - text: t('Coding Plan'), + text: t('编码方案'), itemKey: 'pool', to: '/pool', className: isAdmin() ? '' : 'tableHiddle', },Then add the
编码方案key toweb/src/i18n/locales/{lang}.json(e.g.,"编码方案": "Coding Plan"inen.json).As per coding guidelines: "Translation files in
web/src/i18n/locales/{lang}.jsonmust be flat JSON with Chinese source strings as keys ... callt('中文key')in components."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/layout/SiderBar.jsx` around lines 151 - 156, Replace the i18n key t('Coding Plan') in the sidebar entry object (the item with itemKey 'pool' in SiderBar.jsx) with a Chinese source string t('编码方案') to match the project's i18n convention, then add the corresponding entries in each locale JSON (e.g., in en.json add "编码方案": "Coding Plan", and mirror for other languages) so the translation extraction and rendering remain consistent.web/src/components/table/pools/modals/PoolBindingFormSideSheet.jsx-56-106 (1)
56-106:⚠️ Potential issue | 🟡 MinorUI strings bypass i18n.
Select options (
token/user/group/default/subscription_plan), allInputplaceholders (token_id,user_id,pool_id,priority,binding_value), and the literalEnabledlabel are hardcoded English. Per project i18n guidelines (i18next+t('中文key')with Chinese source keys inweb/src/i18n/locales/{lang}.json), these should be wrapped int(...)like the Create/Update/Cancel buttons already are.As per coding guidelines: "Translation files in
web/src/i18n/locales/{lang}.jsonmust be flat JSON with Chinese source strings as keys. UseuseTranslation()hook and callt('中文key')in components."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/table/pools/modals/PoolBindingFormSideSheet.jsx` around lines 56 - 106, The UI strings in PoolBindingFormSideSheet are hardcoded instead of using i18n; update Select option labels (the values shown: 'token','user','group','default','subscription_plan'), all Input placeholders (token_id, user_id, pool_id, priority, binding_value) and the literal "Enabled" label to use the i18next translator. Import and call useTranslation() in this component, replace literal strings passed to Select.Option children, Input.placeholder props and the Text content with t('...') using Chinese source keys per project convention (mirror keys used elsewhere for Create/Update/Cancel), and keep Select option values and formData.binding_type values unchanged; ensure Switch label uses t('中文key') as well so all visible text is localized.middleware/pool_select.go-39-42 (1)
39-42:⚠️ Potential issue | 🟡 Minor
ContextKeyPoolScopeKeyis alwaysuser:<userId>regardless of how the pool was resolved.The scope key is set to
"user:" + strconv.Itoa(userId)unconditionally, even whenResolvePoolForContextmatched via a token or group binding, and even whenuserId == 0(producing the misleading"user:0"). The downstream rolling-quota logic inloadPoolQuotaScopePoliciesAndScopeKeyoverrides it totoken:<id>when token policies exist, so functionally it may not matter today, but storing a possibly-stale scope key here is easy to misuse later.Consider either (a) setting it based on the actual binding path that matched (token/user/group) or (b) deferring the scope-key write to the consumer that actually knows the scope. At minimum, guard against
userId <= 0.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@middleware/pool_select.go` around lines 39 - 42, The middleware currently always writes ContextKeyPoolScopeKey as "user:<userId>" which can be incorrect (e.g. token/group bindings or userId==0); update the pool selection logic in pool_select.go (where common.SetContextKey is called) to only set ContextKeyPoolScopeKey when the resolved binding path indicates a user binding and userId > 0 (use the resolution result from ResolvePoolForContext to detect token/group/user), otherwise avoid writing the scope key here and let the consumer (e.g. loadPoolQuotaScopePoliciesAndScopeKey) set the correct "token:<id>" or "group:<id>" value; alternatively remove the unconditional SetContextKey call for ContextKeyPoolScopeKey and add a guard so you never store "user:0".web/src/components/table/pools/modals/PoolChannelFormSideSheet.jsx-30-89 (1)
30-89:⚠️ Potential issue | 🟡 MinorLocalize the new side-sheet labels with Chinese source keys.
This component introduces English
t()keys, placeholders, and the rawEnabledlabel. Please switch user-facing strings tot('中文key')and add the corresponding flat locale entries. As per coding guidelines, “Translation files inweb/src/i18n/locales/{lang}.jsonmust be flat JSON with Chinese source strings as keys. UseuseTranslation()hook and callt('中文key')in components.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/table/pools/modals/PoolChannelFormSideSheet.jsx` around lines 30 - 89, Replace all hardcoded English strings in PoolChannelFormSideSheet.jsx (titles, Tag labels, Button text, Input placeholders 'pool_id','channel_id','weight','priority', and the raw 'Enabled' Text) with translation calls t('中文Key...') and ensure the component imports and uses useTranslation() to get t; add matching flat entries in both web/src/i18n/locales/en.json and web/src/i18n/locales/zh.json (or the project locale files) using Chinese source strings as the keys and the English/Chinese values accordingly, keeping keys unique and descriptive (e.g., '创建通道' -> 'Create Pool Channel', '启用' -> 'Enabled', etc.) so every user-facing string in render (Title, Tag, Buttons, Input placeholders, and Switch label) is wrapped with t('中文key').model/pool_binding_resolution_test.go-15-20 (1)
15-20:⚠️ Potential issue | 🟡 MinorClear the pool tables before each test as well as during cleanup.
Right now this helper only schedules post-test cleanup, so a test can start with stale rows if the shared test DB was not clean. Also check cleanup errors so isolation failures are visible.
Suggested test-isolation fix
func truncatePoolBindingResolutionTables(t *testing.T) { t.Helper() + require.NoError(t, DB.Exec("DELETE FROM pool_bindings").Error) + require.NoError(t, DB.Exec("DELETE FROM pools").Error) t.Cleanup(func() { - DB.Exec("DELETE FROM pool_bindings") - DB.Exec("DELETE FROM pools") + require.NoError(t, DB.Exec("DELETE FROM pool_bindings").Error) + require.NoError(t, DB.Exec("DELETE FROM pools").Error) }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@model/pool_binding_resolution_test.go` around lines 15 - 20, The helper truncatePoolBindingResolutionTables currently only schedules post-test cleanup and ignores DB errors; modify it to run immediate DELETEs on "pool_bindings" and "pools" at the start (so each test begins with a clean DB) and keep the existing t.Cleanup deletes, and check the returned error from DB.Exec for both immediate and cleanup calls (calling t.Fatalf or t.Fatalf-like failure on error) so failures are surfaced; reference the truncatePoolBindingResolutionTables helper and the DB.Exec calls for both "DELETE FROM pool_bindings" and "DELETE FROM pools".web/src/components/table/pools/modals/PoolPolicyFormSideSheet.jsx-31-103 (1)
31-103:⚠️ Potential issue | 🟡 MinorLocalize the policy form labels and placeholders.
This new side sheet mixes English
t()keys, English placeholders/options, and raw text likeEnabled. Please use Chinese source keys consistently for all user-facing strings. As per coding guidelines, “Translation files inweb/src/i18n/locales/{lang}.jsonmust be flat JSON with Chinese source strings as keys. UseuseTranslation()hook and callt('中文key')in components.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/table/pools/modals/PoolPolicyFormSideSheet.jsx` around lines 31 - 103, Update PoolPolicyFormSideSheet to localize all user-facing strings using the useTranslation() hook with Chinese source keys: replace raw placeholders ("pool_id", "metric", "window_seconds", "limit_count"), Select option labels ("token", "user"), the Text label "Enabled", and any Title/Tag/Button visible text that currently uses English t('...') keys with t('中文 key') calls (keep internal values like Select value='token' if needed but translate the displayed label), referencing the component/JSX elements Tag, Title, Button, Input placeholders, Select.Option, Text and Switch to find each string; ensure each replacement uses the project's flat Chinese key naming convention and that translations exist in web/src/i18n/locales/*.json.web/src/components/table/pools/index.jsx-157-375 (1)
157-375:⚠️ Potential issue | 🟡 MinorUse the project i18n convention for the new Pool page copy.
The new tabs, placeholders, select labels, button text, and helper text are currently English literals or English
t()keys. Please convert the user-facing copy tot('中文key')and add the flat locale entries. As per coding guidelines, “Translation files inweb/src/i18n/locales/{lang}.jsonmust be flat JSON with Chinese source strings as keys. UseuseTranslation()hook and callt('中文key')in components.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/table/pools/index.jsx` around lines 157 - 375, The component currently uses English literals and non-conforming t() keys across UI pieces (Tabs/TabPane labels, Button texts like the Refresh/Create/Apply Filter/Clear Filters, Input placeholders, Select options, helper Text near Rolling Usage Query, and labels in usageQuery controls such as placeholders and Select.Option values); update each user-facing string in this file to call t('中文key') with appropriate Chinese-source keys and add corresponding flat entries in the locale JSONs, and ensure useTranslation() is imported and used; specifically review usages in functions/variables like Tabs (activeTab), TabPane labels, Button onClick handlers (openCreateBinding/openCreatePool/openCreateChannel/openCreatePolicy/queryUsage/querySelf), Input placeholders and value setters (bindingValueFilter, bindingNameFilter, channelPoolFilter, policyPoolFilter, usageQuery.*), Select.Option values for scope_type/window and bindingTypeFilter options, and the helper Text under Rolling Usage Query, replacing literals with t('中文key') and adding flat i18n keys to web/src/i18n/locales/{lang}.json.web/src/hooks/pools/usePoolsData.jsx-30-31 (1)
30-31:⚠️ Potential issue | 🟡 MinorLocalize table labels, action text, and status tags.
The new table column titles, action buttons, and
Enabled/Disabledtags are English literals generated from the hook. Please move these tot('中文key')so the Pool console follows the repository i18n contract. As per coding guidelines, “Translation files inweb/src/i18n/locales/{lang}.jsonmust be flat JSON with Chinese source strings as keys. UseuseTranslation()hook and callt('中文key')in components.”Also applies to: 599-782
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/hooks/pools/usePoolsData.jsx` around lines 30 - 31, The boolTag helper returns hard-coded English labels; update usePoolsData.jsx to import and call useTranslation() (const { t } = useTranslation()) and replace the string literals in boolTag (and all literal column titles, action button text, and status labels in the same file/range lines ~599-782) with t('中文key') calls using flat Chinese keys per repo convention (e.g., t('已启用') / t('已禁用') for Enabled/Disabled), and ensure each key matches the flat JSON in web/src/i18n/locales/{lang}.json; keep the boolTag identifier but return t(...) wrapped in the same <Tag> elements so UI behavior is unchanged.model/pool.go-147-163 (1)
147-163:⚠️ Potential issue | 🟡 MinorBinding resolution silently swallows DB errors.
GetPoolByIdreturnsgorm.ErrRecordNotFoundboth when the pool is genuinely missing/disabled and is indistinguishable from a transient DB error at this call site. The loop only advances onerr == nil && pool != nil, so any real error (connection drop, etc.) fromGetPoolByIdis discarded and resolution falls through to the next binding / default pool — masking outages and potentially mis-routing requests.Consider distinguishing
gorm.ErrRecordNotFoundfrom other errors (propagate the latter), or inline the lookup via a join so a single query returns only enabled pools.pool, err := GetPoolById(binding.PoolId) if err != nil && !errors.Is(err, gorm.ErrRecordNotFound) { return nil, err } if pool != nil { return pool, nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@model/pool.go` around lines 147 - 163, The function resolvePoolByBindingType currently ignores non-NotFound DB errors from GetPoolById and can mask real outages; modify the loop in resolvePoolByBindingType to check the error returned by GetPoolById and propagate any error that is not gorm.ErrRecordNotFound (e.g., using errors.Is(err, gorm.ErrRecordNotFound)) instead of swallowing it, returning that error immediately, and only continue when the error is a NotFound; alternatively, replace the per-binding GetPoolById calls with a single joined query that only selects enabled pools to avoid the multi-call failure mode (refer to resolvePoolByBindingType, GetPoolById and gorm.ErrRecordNotFound).
🧹 Nitpick comments (4)
relay/common/stream_status.go (1)
73-91: Lock ordering note forMergePriorSoftErrors.The method acquires
prior.muthens.mu. Given the current single call site instream_scanner.go(whereprevStreamStatusis local and no goroutine will ever calls.MergePriorSoftErrors(prior)with arguments swapped), this is safe today. Worth a brief doc comment documenting thatpriormust not be the same instance assand must not be concurrently targeted by a reverse merge, to avoid future AB/BA deadlocks if this helper is reused.Minor: the copy on line 79 can be skipped when
prior.ErrorCount == 0/len(prior.Errors) == 0to avoid a tiny allocation on the common path, but this is cosmetic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/common/stream_status.go` around lines 73 - 91, Add a doc comment above MergePriorSoftErrors explaining the lock-ordering precondition: callers must ensure prior != s and must not perform the reverse merge concurrently (i.e., no concurrent call that would acquire s.mu then prior.mu) so callers avoid AB/BA deadlocks; then optimize the fast path by early-returning when prior.ErrorCount == 0 || len(prior.Errors) == 0 to skip the slice copy/allocation and mutex work (keep the existing sequence of locking prior.mu then s.mu and the references to prior.mu and s.mu unchanged).web/src/components/settings/personal/cards/NotificationSettings.jsx (1)
298-298: Use a Chinese source key for the new sidebar label.
t('Coding Plan')adds an English i18n key in a codepath where translation keys are expected to be Chinese source strings. Consider using a Chinese key and mapping it to “Coding Plan” in locale files if the English display is intentional.🌐 Proposed i18n alignment
- { key: 'pool', title: t('Coding Plan'), description: t('Pool与配额策略') }, + { key: 'pool', title: t('编码方案'), description: t('编码方案与配额策略') },As per coding guidelines,
web/src/**/*.{ts,tsx,js,jsx}must use translation files with Chinese source strings as keys and callt('中文key')in components.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/settings/personal/cards/NotificationSettings.jsx` at line 298, The new sidebar entry uses an English i18n key; update the call in NotificationSettings.jsx so the component uses a Chinese source key (e.g., replace t('Coding Plan') with t('编码计划') or another agreed Chinese key) for the item with key 'pool' (the object { key: 'pool', title: t(...) , description: ... }), and add the corresponding mapping in the locale files to map that Chinese key to "Coding Plan" (and any other locales) so translations remain consistent with the project's Chinese-key convention.web/src/pages/Setting/Operation/SettingsSidebarModulesAdmin.jsx (1)
259-259: Use a Chinese source key for this new module label.
t('Coding Plan')does not follow the project’s frontend i18n convention. Use a Chinese source key and put the English wording in locale JSON if needed.🌐 Proposed i18n alignment
- { key: 'pool', title: t('Coding Plan'), description: t('Pool与配额策略') }, + { key: 'pool', title: t('编码方案'), description: t('编码方案与配额策略') },As per coding guidelines,
web/src/**/*.{ts,tsx,js,jsx}must use translation files with Chinese source strings as keys and callt('中文key')in components.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/Setting/Operation/SettingsSidebarModulesAdmin.jsx` at line 259, Replace the English source string passed to the translator in the module entry where key: 'pool' inside SettingsSidebarModulesAdmin.jsx (the object with title: t('Coding Plan')) with a Chinese source key, e.g. title: t('编码计划'); then add the corresponding mapping in the locale JSON files (e.g. en.json -> "编码计划": "Coding Plan", zh.json -> "编码计划": "编码计划") so the component calls t('中文key') and translations remain correct; keep the description as-is if it already uses Chinese.service/channel_select.go (1)
119-165: Minor: consider collapsing the pool/non-pool branches.The
if param.PoolID > 0 { GetRandomSatisfiedChannelByPool } else { GetRandomSatisfiedChannel }pattern is duplicated in both the"auto"group loop and the else-branch. SinceGetRandomSatisfiedChannelByPoolalready short-circuits toGetRandomSatisfiedChannelwhenpoolId <= 0(permodel/channel_cache.go:121-151), callingGetRandomSatisfiedChannelByPool(..., param.PoolID)unconditionally would remove the duplication without changing behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@service/channel_select.go` around lines 119 - 165, Duplicate branching calls to GetRandomSatisfiedChannelByPool and GetRandomSatisfiedChannel can be collapsed: always call model.GetRandomSatisfiedChannelByPool(...) with param.PoolID (it already falls back to GetRandomSatisfiedChannel when poolId <= 0), replacing the two if/else sites inside the auto-group selection (where channel = model.GetRandomSatisfiedChannelByPool or model.GetRandomSatisfiedChannel) and the else-branch where channel, err = ...; remove the duplicated conditional on param.PoolID and call GetRandomSatisfiedChannelByPool uniformly (keeping param.PoolID and other args the same).
🤖 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/pool.go`:
- Around line 200-205: The code sets defaults for req.Metric and req.ScopeType
but doesn't validate/normalize values, so add a call to the existing
normalizePoolPolicy(req) helper before persisting to ensure only supported
metrics/scopes (e.g., request_count and token/user) are accepted; update both
the SavePoolPolicy path where req.Metric/req.ScopeType are set and the
UpdatePoolPolicy handler (referencing normalizePoolPolicy, req.Metric,
req.ScopeType, model.PoolQuotaMetricRequestCount, model.PoolQuotaScopeUser) to
call normalizePoolPolicy(req) and return a validation error if normalization
fails instead of saving arbitrary strings.
- Around line 328-330: The current validation in the pool binding create/update
endpoints rejects empty binding_value even though ResolvePoolForContext and
tests use PoolBindingTypeDefault with binding_value == ""; update the validation
so binding_value is only required when req.BindingType !=
PoolBindingTypeDefault. Concretely, in the Create (the block checking
req.PoolId, req.BindingType, req.BindingValue) and the UpdatePoolBinding handler
(the analogous check at 349-351), change the condition to allow
strings.TrimSpace(req.BindingValue) == "" when req.BindingType ==
PoolBindingTypeDefault; keep the PoolId and BindingType checks as-is and reuse
the same helper/logic used by ResolvePoolForContext to detect
PoolBindingTypeDefault.
- Around line 466-480: The endpoint currently builds redisKey as
"pool:rq:events:%d:user:%d" which reads user-scoped usage even when requests are
authenticated by token; update the key construction in the handler (near
tokenId, ResolvePoolForContext, parseRollingWindow, redisKey) to choose the
scope based on tokenId (or the presence of a token-scoped policy): if tokenId >
0 use "pool:rq:events:%d:token:%d" with tokenId, otherwise keep the existing
"user:%d" key so the ZCount reads the same scope where PoolRollingQuota writes
usage.
In `@docker-compose.yml`:
- Line 12: The compose file currently embeds a shared DB password; replace the
hardcoded password with an environment variable and fail fast if it's not set:
in the postgres service env, remove the default value and use POSTGRES_PASSWORD
via the Compose variable expansion with a required check like
${POSTGRES_PASSWORD:?Postgres password not set} so Docker Compose/CLI will error
when missing, and ensure any other occurrences (lines referenced) are changed
similarly; also add a .env.example or document that POSTGRES_PASSWORD must be
provided via a local .env or deployment secret.
- Line 5: The compose file exposes http://localhost:6001 but the application
default ServerAddress in setting/system_setting/system_setting_old.go still
points to http://localhost:3000, causing broken external URLs; either update the
default ServerAddress constant/field in system_setting_old.go to
"http://localhost:6001" (look for the ServerAddress default or
NewDefault/SystemSetting initializer) or change docker-compose.yml port mapping
back to 3000:3000 to match the existing ServerAddress, and then update all
README files (README.md, README.zh_CN.md, README.fr.md, README.ja.md,
README.zh_TW.md) so the documented access URL matches the chosen port.
In `@middleware/pool_quota_rolling.go`:
- Around line 117-137: The current trim/count/check/reserve flow (calls to
trimPoolWindowEvents, countRollingWindowEvents, and reservePoolRequestEvent
using redisKey and requestId) is not atomic and can race under concurrency;
replace it with an atomic Redis operation (Lua script or WATCH/EXEC transaction)
that: performs the trimming, computes counts for all entries in validPolicies
windows, checks limits, and only if all policies allow, inserts the new event
and sets the appropriate TTL in one atomic operation; modify the calling code to
call a single new helper (e.g., reservePoolRequestEventAtomic) that returns
success/failure and error, and remove the separate trim/count/reserve sequence.
In `@model/ability.go`:
- Around line 146-190: The issue is that getPoolChannelQuery builds its ability
set by reusing getChannelQuery which calls getPriority on the unscoped
abilities, so priority selection ignores the pool join and can pick a priority
that has no pool members; fix by computing priorities using the same pool-scoped
join: add a new helper getPoolPriority(group, model, retry, poolId) (or modify
getPriority to accept an optional poolId) that applies the JOIN/Where used in
getPoolChannelQuery before selecting a priority, then use that pool-aware
priority when constructing the channel query in GetChannelByPool (i.e., replace
the call path that leads to getPriority via getChannelQuery). Also consolidate
the duplicate weighted-selection code in GetChannelByPool and GetChannel into a
shared helper (e.g., selectChannelFromAbilities([]Ability)) to avoid
duplication.
In `@model/channel_cache.go`:
- Around line 146-154: FilterChannelIDsByPool currently returns only IDs so
pool-specific PoolChannel.Weight and PoolChannel.Priority are lost; change
FilterChannelIDsByPool to return a slice of structs (channelID plus weight and
priority) instead of []string, update the call site here to accept that new
return type, and change getRandomChannelFromIDs to accept that slice and perform
selection using the pool-specific Weight/Priority (falling back to global
channel.GetPriority()/weight from channelsIDM when pool fields are zero/missing)
so memory-cache path mirrors GetChannelByPool's weighted/priority logic; update
any other callers of FilterChannelIDsByPool/getRandomChannelFromIDs accordingly.
In `@model/main.go`:
- Around line 283-286: Add a composite unique index on the PoolBinding model to
prevent duplicates by tagging the fields BindingType, BindingValue, and PoolId
with the same GORM unique index name and ordering; update the PoolBinding struct
so BindingType has `uniqueIndex:uk_pool_binding,priority:1`, BindingValue has
`uniqueIndex:uk_pool_binding,priority:2`, and PoolId has
`uniqueIndex:uk_pool_binding,priority:3` (matching the pattern used by
PoolChannel and PoolQuotaPolicy) so GORM enforces a DB-level unique constraint
across SQLite/MySQL/Postgres.
In `@model/pool.go`:
- Around line 299-304: DeletePool currently only deletes the Pool row and leaves
orphaned PoolChannel/PoolBinding/PoolQuotaPolicy rows; update DeletePool to
perform the removal in a DB transaction: begin a transaction via DB.Begin(),
then delete from PoolChannel, PoolBinding, and PoolQuotaPolicy where pool_id =
poolId (use the same poolId match used in ResolvePoolForContext/GetPoolById),
then delete the Pool row (e.g., DB.Where("id = ?", poolId).Delete(&Pool{})), and
commit; ensure you rollback on any error and return that error. Alternatively,
implement a soft-delete flag on Pool and make ResolvePoolForContext/GetPoolById
and GetPoolBindings/GetPoolChannels/GetPoolPolicies consistently ignore disabled
pools, but prefer the transactional hard-delete approach above for immediate
cleanup.
- Around line 63-72: The PoolBinding uniqueness check is racy because
CreatePoolBinding/UpdatePoolBinding use Count + Create/Updates without a DB
constraint; modify the PoolBinding struct to declare a UNIQUE composite index on
(BindingType, BindingValue, PoolId) (e.g. change the gorm tag for
idx_pool_binding to include "unique") so the database enforces uniqueness, add a
migration/backfill to deduplicate existing rows before applying the constraint,
and update CreatePoolBinding and UpdatePoolBinding to translate the DB
duplicate-key error into the existing user-facing "duplicate" error path instead
of relying solely on the pre-check.
In `@router/relay-router.go`:
- Around line 73-74: The /suno and Midjourney routes are skipping pool
middleware so ContextKeyPoolId isn't set and quota enforcement is bypassed;
update the route registration for the Suno handler (where TokenAuth() currently
flows straight to Distribute()) and the registerMjRouterGroup (Midjourney) to
insert middleware.PoolSelect() and middleware.PoolRollingQuota() before calling
Distribute(), mirroring how relayV1Router and the /gemini group apply
PoolSelect() and PoolRollingQuota(), so pool-aware channel selection and rolling
quota enforcement run and ContextKeyPoolId is populated.
In `@web/package.json`:
- Line 13: Remove the unused "antd" dependency from package.json: delete the
"antd": "^5.23.0" entry, then run the project package manager (npm install or
yarn install) to update node_modules and regenerate the lockfile
(package-lock.json or yarn.lock); confirm no imports of "antd" remain and run
the build/test to ensure nothing breaks (references: package.json entry "antd"
and existing UI usage of "@douyinfe/semi-ui").
In `@web/src/components/table/pools/modals/PoolFormSideSheet.jsx`:
- Around line 22-76: The form uses English and hardcoded strings and has UX
issues: update all visible strings to use t('中文key') (replace t('Update'),
t('Create'), t('Update Pool'), t('Create Pool'), t('Cancel') and the Input
placeholders and Select.Option labels with Chinese keys via useTranslation()),
fix the SideSheet placement to a fixed value (change placement={isEdit ? 'right'
: 'left'} to placement='right'), guard the select value so it never becomes
'undefined' (change String(formData.status) to String(formData.status ?? 1) or
ensure parent seeds status), and convert the loose Inputs into a Form with
labeled Form.Item fields (wrap name/description/status in a Form, add a required
validator for name) so validation and labels match other SideSheet forms; locate
these changes around isEdit, SideSheet, the Input components, Select and
formData.status references in PoolFormSideSheet.jsx.
---
Outside diff comments:
In `@middleware/distributor.go`:
- Around line 39-53: The token-specific-channel branch (when using
ContextKeyTokenSpecificChannelId) currently skips pool membership checks
allowing a token pinned to a channel outside the selected pool; after retrieving
channel via model.GetChannelById in that branch, verify the channel belongs to
the active pool (e.g., compare channel.PoolID or call the existing
pool-membership helper used in the pool-filter path) and abort with a forbidden
error (same i18n.MsgDistributorChannelDisabled or a new message) if it does not;
apply the same membership check logic to the other token-specific-channel
handling sites referenced (the other ContextKeyTokenSpecificChannelId handling
blocks around the other ranges) so token-pinned channels cannot bypass pool
isolation.
In `@web/src/pages/Setting/Operation/SettingsSidebarModulesAdmin.jsx`:
- Around line 61-70: The initial admin sidebar state in
SettingsSidebarModulesAdmin.jsx is missing the pool key so the new Pool switch
defaults to undefined; update the initial admin object (the admin property in
the sidebar state) to include pool: true alongside
enabled/channel/models/deployment/redemption/user/subscription/setting so it
matches the reset and parse-fallback defaults and ensures Pool starts enabled
when no saved SidebarModulesAdmin exists.
---
Minor comments:
In `@docker-compose.yml`:
- Line 29: Update the commented MySQL DSN so it matches the declared root
password (MYSQL_ROOT_PASSWORD) instead of the hardcoded 123456; locate the
commented line containing SQL_DSN and replace the password portion with the
actual secret or a reference to the env var (e.g., use change-me or
${MYSQL_ROOT_PASSWORD}) so uncommenting SQL_DSN will authenticate correctly.
In `@middleware/pool_select.go`:
- Around line 39-42: The middleware currently always writes
ContextKeyPoolScopeKey as "user:<userId>" which can be incorrect (e.g.
token/group bindings or userId==0); update the pool selection logic in
pool_select.go (where common.SetContextKey is called) to only set
ContextKeyPoolScopeKey when the resolved binding path indicates a user binding
and userId > 0 (use the resolution result from ResolvePoolForContext to detect
token/group/user), otherwise avoid writing the scope key here and let the
consumer (e.g. loadPoolQuotaScopePoliciesAndScopeKey) set the correct
"token:<id>" or "group:<id>" value; alternatively remove the unconditional
SetContextKey call for ContextKeyPoolScopeKey and add a guard so you never store
"user:0".
In `@model/pool_binding_resolution_test.go`:
- Around line 15-20: The helper truncatePoolBindingResolutionTables currently
only schedules post-test cleanup and ignores DB errors; modify it to run
immediate DELETEs on "pool_bindings" and "pools" at the start (so each test
begins with a clean DB) and keep the existing t.Cleanup deletes, and check the
returned error from DB.Exec for both immediate and cleanup calls (calling
t.Fatalf or t.Fatalf-like failure on error) so failures are surfaced; reference
the truncatePoolBindingResolutionTables helper and the DB.Exec calls for both
"DELETE FROM pool_bindings" and "DELETE FROM pools".
In `@model/pool.go`:
- Around line 147-163: The function resolvePoolByBindingType currently ignores
non-NotFound DB errors from GetPoolById and can mask real outages; modify the
loop in resolvePoolByBindingType to check the error returned by GetPoolById and
propagate any error that is not gorm.ErrRecordNotFound (e.g., using
errors.Is(err, gorm.ErrRecordNotFound)) instead of swallowing it, returning that
error immediately, and only continue when the error is a NotFound;
alternatively, replace the per-binding GetPoolById calls with a single joined
query that only selects enabled pools to avoid the multi-call failure mode
(refer to resolvePoolByBindingType, GetPoolById and gorm.ErrRecordNotFound).
In `@web/src/components/layout/SiderBar.jsx`:
- Around line 151-156: Replace the i18n key t('Coding Plan') in the sidebar
entry object (the item with itemKey 'pool' in SiderBar.jsx) with a Chinese
source string t('编码方案') to match the project's i18n convention, then add the
corresponding entries in each locale JSON (e.g., in en.json add "编码方案": "Coding
Plan", and mirror for other languages) so the translation extraction and
rendering remain consistent.
In `@web/src/components/table/pools/index.jsx`:
- Around line 157-375: The component currently uses English literals and
non-conforming t() keys across UI pieces (Tabs/TabPane labels, Button texts like
the Refresh/Create/Apply Filter/Clear Filters, Input placeholders, Select
options, helper Text near Rolling Usage Query, and labels in usageQuery controls
such as placeholders and Select.Option values); update each user-facing string
in this file to call t('中文key') with appropriate Chinese-source keys and add
corresponding flat entries in the locale JSONs, and ensure useTranslation() is
imported and used; specifically review usages in functions/variables like Tabs
(activeTab), TabPane labels, Button onClick handlers
(openCreateBinding/openCreatePool/openCreateChannel/openCreatePolicy/queryUsage/querySelf),
Input placeholders and value setters (bindingValueFilter, bindingNameFilter,
channelPoolFilter, policyPoolFilter, usageQuery.*), Select.Option values for
scope_type/window and bindingTypeFilter options, and the helper Text under
Rolling Usage Query, replacing literals with t('中文key') and adding flat i18n
keys to web/src/i18n/locales/{lang}.json.
In `@web/src/components/table/pools/modals/PoolBindingFormSideSheet.jsx`:
- Around line 56-106: The UI strings in PoolBindingFormSideSheet are hardcoded
instead of using i18n; update Select option labels (the values shown:
'token','user','group','default','subscription_plan'), all Input placeholders
(token_id, user_id, pool_id, priority, binding_value) and the literal "Enabled"
label to use the i18next translator. Import and call useTranslation() in this
component, replace literal strings passed to Select.Option children,
Input.placeholder props and the Text content with t('...') using Chinese source
keys per project convention (mirror keys used elsewhere for
Create/Update/Cancel), and keep Select option values and formData.binding_type
values unchanged; ensure Switch label uses t('中文key') as well so all visible
text is localized.
In `@web/src/components/table/pools/modals/PoolChannelFormSideSheet.jsx`:
- Around line 30-89: Replace all hardcoded English strings in
PoolChannelFormSideSheet.jsx (titles, Tag labels, Button text, Input
placeholders 'pool_id','channel_id','weight','priority', and the raw 'Enabled'
Text) with translation calls t('中文Key...') and ensure the component imports and
uses useTranslation() to get t; add matching flat entries in both
web/src/i18n/locales/en.json and web/src/i18n/locales/zh.json (or the project
locale files) using Chinese source strings as the keys and the English/Chinese
values accordingly, keeping keys unique and descriptive (e.g., '创建通道' -> 'Create
Pool Channel', '启用' -> 'Enabled', etc.) so every user-facing string in render
(Title, Tag, Buttons, Input placeholders, and Switch label) is wrapped with
t('中文key').
In `@web/src/components/table/pools/modals/PoolPolicyFormSideSheet.jsx`:
- Around line 31-103: Update PoolPolicyFormSideSheet to localize all user-facing
strings using the useTranslation() hook with Chinese source keys: replace raw
placeholders ("pool_id", "metric", "window_seconds", "limit_count"), Select
option labels ("token", "user"), the Text label "Enabled", and any
Title/Tag/Button visible text that currently uses English t('...') keys with
t('中文 key') calls (keep internal values like Select value='token' if needed but
translate the displayed label), referencing the component/JSX elements Tag,
Title, Button, Input placeholders, Select.Option, Text and Switch to find each
string; ensure each replacement uses the project's flat Chinese key naming
convention and that translations exist in web/src/i18n/locales/*.json.
In `@web/src/hooks/pools/usePoolsData.jsx`:
- Around line 30-31: The boolTag helper returns hard-coded English labels;
update usePoolsData.jsx to import and call useTranslation() (const { t } =
useTranslation()) and replace the string literals in boolTag (and all literal
column titles, action button text, and status labels in the same file/range
lines ~599-782) with t('中文key') calls using flat Chinese keys per repo
convention (e.g., t('已启用') / t('已禁用') for Enabled/Disabled), and ensure each key
matches the flat JSON in web/src/i18n/locales/{lang}.json; keep the boolTag
identifier but return t(...) wrapped in the same <Tag> elements so UI behavior
is unchanged.
---
Nitpick comments:
In `@relay/common/stream_status.go`:
- Around line 73-91: Add a doc comment above MergePriorSoftErrors explaining the
lock-ordering precondition: callers must ensure prior != s and must not perform
the reverse merge concurrently (i.e., no concurrent call that would acquire s.mu
then prior.mu) so callers avoid AB/BA deadlocks; then optimize the fast path by
early-returning when prior.ErrorCount == 0 || len(prior.Errors) == 0 to skip the
slice copy/allocation and mutex work (keep the existing sequence of locking
prior.mu then s.mu and the references to prior.mu and s.mu unchanged).
In `@service/channel_select.go`:
- Around line 119-165: Duplicate branching calls to
GetRandomSatisfiedChannelByPool and GetRandomSatisfiedChannel can be collapsed:
always call model.GetRandomSatisfiedChannelByPool(...) with param.PoolID (it
already falls back to GetRandomSatisfiedChannel when poolId <= 0), replacing the
two if/else sites inside the auto-group selection (where channel =
model.GetRandomSatisfiedChannelByPool or model.GetRandomSatisfiedChannel) and
the else-branch where channel, err = ...; remove the duplicated conditional on
param.PoolID and call GetRandomSatisfiedChannelByPool uniformly (keeping
param.PoolID and other args the same).
In `@web/src/components/settings/personal/cards/NotificationSettings.jsx`:
- Line 298: The new sidebar entry uses an English i18n key; update the call in
NotificationSettings.jsx so the component uses a Chinese source key (e.g.,
replace t('Coding Plan') with t('编码计划') or another agreed Chinese key) for the
item with key 'pool' (the object { key: 'pool', title: t(...) , description: ...
}), and add the corresponding mapping in the locale files to map that Chinese
key to "Coding Plan" (and any other locales) so translations remain consistent
with the project's Chinese-key convention.
In `@web/src/pages/Setting/Operation/SettingsSidebarModulesAdmin.jsx`:
- Line 259: Replace the English source string passed to the translator in the
module entry where key: 'pool' inside SettingsSidebarModulesAdmin.jsx (the
object with title: t('Coding Plan')) with a Chinese source key, e.g. title:
t('编码计划'); then add the corresponding mapping in the locale JSON files (e.g.
en.json -> "编码计划": "Coding Plan", zh.json -> "编码计划": "编码计划") so the component
calls t('中文key') and translations remain correct; keep the description as-is if
it already uses Chinese.
🪄 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: 0ea91b18-c3b2-4036-ac4c-e4a607885ec0
📒 Files selected for processing (43)
common/constants.gocommon/init.goconstant/context_key.gocontroller/pool.gocontroller/relay.godocker-compose.ymlmiddleware/distributor.gomiddleware/pool_quota_rolling.gomiddleware/pool_quota_rolling_test.gomiddleware/pool_select.gomodel/ability.gomodel/channel_cache.gomodel/main.gomodel/option.gomodel/pool.gomodel/pool_binding_resolution_test.gorelay/common/stream_status.gorelay/helper/stream_scanner.gorouter/api-router.gorouter/relay-router.goservice/channel_affinity_usage_cache_test.goservice/channel_select.goweb/package.jsonweb/src/App.jsxweb/src/components/layout/PageLayout.jsxweb/src/components/layout/SiderBar.jsxweb/src/components/settings/personal/cards/NotificationSettings.jsxweb/src/components/table/channels/ChannelsColumnDefs.jsxweb/src/components/table/pools/index.jsxweb/src/components/table/pools/modals/PoolBindingFormSideSheet.jsxweb/src/components/table/pools/modals/PoolChannelFormSideSheet.jsxweb/src/components/table/pools/modals/PoolFormSideSheet.jsxweb/src/components/table/pools/modals/PoolPolicyFormSideSheet.jsxweb/src/components/table/tokens/TokensColumnDefs.jsxweb/src/helpers/render.jsxweb/src/hooks/common/useSidebar.jsweb/src/hooks/pools/usePoolsData.jsxweb/src/hooks/pools/usePoolsData.test.jsxweb/src/index.jsxweb/src/pages/Pool/index.jsxweb/src/pages/Setting/Operation/SettingsSidebarModulesAdmin.jsxweb/src/test/setupTests.jsweb/vite.config.js
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
docker-compose.yml (1)
28-30:⚠️ Potential issue | 🟠 MajorHardcoded Redis password also needs to move to env/secret (extends prior finding).
The prior review flagged the Postgres credential at lines 12/28/66 (now 68). The same concern applies to the new Redis password
123456qwertyon line 30 here and line 58 below — it is a committed shared secret and, per Betterleaks, line 68 is still flagged as a leaked generic key. Please consolidate both into.env-driven variables with required checks, e.g.:Proposed fix
- - SQL_DSN=postgresql://root:u0HjoW0z9uHzHu84EFrM7OcKZthscsNi@koooyooo-newapi-postgres:5432/new-api # ⚠️ IMPORTANT: Change the password in production! -# - SQL_DSN=root:123456qwerty@tcp(koooyooo-newapi-mysql:3306)/new-api # Point to the mysql service, uncomment if using MySQL - - REDIS_CONN_STRING=redis://:123456qwerty@koooyooo-newapi-redis:6379 # ⚠️ IMPORTANT: Change the password in production! (must match koooyooo-newapi-redis requirepass) + - SQL_DSN=postgresql://root:${POSTGRES_PASSWORD:?set POSTGRES_PASSWORD in .env}@koooyooo-newapi-postgres:5432/new-api +# - SQL_DSN=root:${MYSQL_ROOT_PASSWORD:?set MYSQL_ROOT_PASSWORD in .env}@tcp(koooyooo-newapi-mysql:3306)/new-api + - REDIS_CONN_STRING=redis://:${REDIS_PASSWORD:?set REDIS_PASSWORD in .env}@koooyooo-newapi-redis:6379And mirror
${REDIS_PASSWORD:?...}on line 58 and${POSTGRES_PASSWORD:?...}on line 68. Add a.env.exampledocumenting the required variables.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-compose.yml` around lines 28 - 30, Replace the hardcoded Redis and Postgres credentials with env-driven variables and runtime checks: stop embedding the password in REDIS_CONN_STRING and SQL_DSN and instead construct them from environment vars (e.g. use REDIS_PASSWORD and POSTGRES_PASSWORD) and validate presence using the shell parameter expansion pattern (e.g. ${REDIS_PASSWORD:?} / ${POSTGRES_PASSWORD:?}) where REDIS_CONN_STRING and SQL_DSN are defined; update the other occurrence of REDIS_CONN_STRING (the duplicate at line 58) to use the same env var, and add a .env.example documenting REDIS_PASSWORD, POSTGRES_PASSWORD and any host/port names so deployments must supply secrets rather than committing them.
🧹 Nitpick comments (1)
docker-compose.yml (1)
18-18: Pinning:latestharms reproducibility.
harbor.circledigital.cn/koooyooo-api/koooyooo-newapi:latest(andredis:lateston line 55) make deployments non-deterministic — twodocker compose pullruns on different days can land on different app/middleware versions, which is particularly risky for this PR since the rolling-window quota logic depends on a specific Redis client/server contract. Prefer an immutable tag or digest for the app image, and pinredisto a supported major (e.g.,redis:7-alpine).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-compose.yml` at line 18, The compose file uses floating tags (image: harbor.circledigital.cn/koooyooo-api/koooyooo-newapi:latest and redis:latest) which breaks reproducibility; update the service image entries to use an immutable tag or digest for koooyooo-newapi (replace :latest with the exact release tag or `@sha256`:... digest) and pin Redis to a supported major like redis:7-alpine so the docker-compose image fields are deterministic and stable for deployments.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker-compose.yml`:
- Around line 17-19: The docker-compose change renames the
service/image/container to downstream-specific values (koooyooo-newapi, image
harbor.circledigital.cn/koooyooo-api/koooyooo-newapi:latest, container_name
koooyooo-newapi-app) which is out of scope; revert these entries to the original
upstream generic names and image (e.g., new-api and the upstream image tag) or
move the rebranding into a separate PR/override file; specifically restore the
service key, the image value, and the container_name back to the upstream
identifiers or extract them into a separate compose override so upstream
docker-compose.yml remains generic.
In `@web/src/components/table/tokens/TokensColumnDefs.jsx`:
- Around line 485-494: The column title currently uses t('Token ID') in
TokensColumnDefs.jsx; replace this English key with the Chinese source key used
by your i18n files (for example t('令牌 ID') or the exact Chinese key used by
adjacent columns) in the column object where title is set, keeping the render
block with Typography.Text unchanged; also ensure the corresponding flat JSON
entries are added/updated in web/src/i18n/locales/{lang}.json so the key
resolves in all locales.
---
Duplicate comments:
In `@docker-compose.yml`:
- Around line 28-30: Replace the hardcoded Redis and Postgres credentials with
env-driven variables and runtime checks: stop embedding the password in
REDIS_CONN_STRING and SQL_DSN and instead construct them from environment vars
(e.g. use REDIS_PASSWORD and POSTGRES_PASSWORD) and validate presence using the
shell parameter expansion pattern (e.g. ${REDIS_PASSWORD:?} /
${POSTGRES_PASSWORD:?}) where REDIS_CONN_STRING and SQL_DSN are defined; update
the other occurrence of REDIS_CONN_STRING (the duplicate at line 58) to use the
same env var, and add a .env.example documenting REDIS_PASSWORD,
POSTGRES_PASSWORD and any host/port names so deployments must supply secrets
rather than committing them.
---
Nitpick comments:
In `@docker-compose.yml`:
- Line 18: The compose file uses floating tags (image:
harbor.circledigital.cn/koooyooo-api/koooyooo-newapi:latest and redis:latest)
which breaks reproducibility; update the service image entries to use an
immutable tag or digest for koooyooo-newapi (replace :latest with the exact
release tag or `@sha256`:... digest) and pin Redis to a supported major like
redis:7-alpine so the docker-compose image fields are deterministic and stable
for deployments.
🪄 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: 812a4d9d-60b6-4521-a57d-e92f772f2982
📒 Files selected for processing (11)
common/constants.gocommon/init.goconstant/context_key.gocontroller/relay.godocker-compose.ymlmodel/option.gorouter/api-router.goweb/package.jsonweb/src/components/layout/PageLayout.jsxweb/src/components/table/tokens/TokensColumnDefs.jsxweb/src/helpers/render.jsx
✅ Files skipped from review due to trivial changes (2)
- web/package.json
- web/src/components/layout/PageLayout.jsx
🚧 Files skipped from review as they are similar to previous changes (5)
- constant/context_key.go
- model/option.go
- common/init.go
- router/api-router.go
- web/src/helpers/render.jsx
Comment on lines
+17
to
+19
| koooyooo-newapi: | ||
| image: harbor.circledigital.cn/koooyooo-api/koooyooo-newapi:latest | ||
| container_name: koooyooo-newapi-app |
Contributor
There was a problem hiding this comment.
Rebranding appears out of scope for this PR.
This PR is titled "Feature/pool rolling window quota" and its stated objectives are middleware pool selection, sliding-window quota checks, uniqueness of pool bindings, and admin SideSheet UI changes. Renaming the service/image/container from new-api / calciumion/new-api:latest to koooyooo-newapi / harbor.circledigital.cn/koooyooo-api/koooyooo-newapi:latest (and all derived hostnames, network name, and node name) is unrelated to the feature and points at a private registry fork.
If this is a downstream fork-specific change, please split it out of this PR so upstream docker-compose.yml stays generic; otherwise, please clarify in the PR description why upstream users should adopt the koooyooo-newapi naming and a private harbor image as defaults.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docker-compose.yml` around lines 17 - 19, The docker-compose change renames
the service/image/container to downstream-specific values (koooyooo-newapi,
image harbor.circledigital.cn/koooyooo-api/koooyooo-newapi:latest,
container_name koooyooo-newapi-app) which is out of scope; revert these entries
to the original upstream generic names and image (e.g., new-api and the upstream
image tag) or move the rebranding into a separate PR/override file; specifically
restore the service key, the image value, and the container_name back to the
upstream identifiers or extract them into a separate compose override so
upstream docker-compose.yml remains generic.
Comment on lines
+485
to
+494
| { | ||
| title: t('Token ID'), | ||
| dataIndex: 'id', | ||
| width: 120, | ||
| render: (text, record) => ( | ||
| <Typography.Text copyable={{ content: String(record.id) }}> | ||
| {record.id} | ||
| </Typography.Text> | ||
| ), | ||
| }, |
Contributor
There was a problem hiding this comment.
Use a Chinese source key for this column label.
Line 486 adds t('Token ID'), but this frontend uses Chinese source strings as i18n keys. Prefer a Chinese key consistent with the adjacent columns.
🌐 Proposed fix
{
- title: t('Token ID'),
+ title: t('令牌ID'),
dataIndex: 'id',
width: 120,As per coding guidelines, “Translation files in web/src/i18n/locales/{lang}.json must be flat JSON with Chinese source strings as keys. Use useTranslation() hook and call t('中文key') in components.”
📝 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
| { | |
| title: t('Token ID'), | |
| dataIndex: 'id', | |
| width: 120, | |
| render: (text, record) => ( | |
| <Typography.Text copyable={{ content: String(record.id) }}> | |
| {record.id} | |
| </Typography.Text> | |
| ), | |
| }, | |
| { | |
| title: t('令牌ID'), | |
| dataIndex: 'id', | |
| width: 120, | |
| render: (text, record) => ( | |
| <Typography.Text copyable={{ content: String(record.id) }}> | |
| {record.id} | |
| </Typography.Text> | |
| ), | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/table/tokens/TokensColumnDefs.jsx` around lines 485 - 494,
The column title currently uses t('Token ID') in TokensColumnDefs.jsx; replace
this English key with the Chinese source key used by your i18n files (for
example t('令牌 ID') or the exact Chinese key used by adjacent columns) in the
column object where title is set, keeping the render block with Typography.Text
unchanged; also ensure the corresponding flat JSON entries are added/updated in
web/src/i18n/locales/{lang}.json so the key resolves in all locales.
Author
|
Opened by mistake; we deploy from our fork only. |
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.
Important
本 PR 引入 Pool（编码方案） 与 滚动窗口配额，并把管理端从行内表单改成 SideSheet，同时在服务端禁止同一池下重复绑定，减少配置错误。前后端均有单测覆盖；合并前请维护者关注 DB 迁移 与 历史重复绑定数据。
📝 变更描述 / Description
做了什么
在 relay 分发路径上增加 Pool 选择与滚动配额校验：请求进入后先按 token/用户等解析应落入的池，再在窗口内累计用量，超限则拒绝，从而把「按池限流」从纯前端配置变成请求链路中的硬约束。管理端新增 Pool 相关 Tab（池、绑定、通道、策略），并把创建/编辑收到 SideSheet，避免长表格里行内编辑易点错、难对照的问题。
为何能生效
滚动配额逻辑放在 middleware 层、与现有 distributor 衔接，保证命中路由的请求都会经过同一套计数与窗口判断；绑定去重在 CreatePoolBinding / UpdatePoolBinding 里对 (binding_type, binding_value, pool_id) 做唯一性约束，错误在写入前即返回，不依赖前端防呆。SideSheet 只改交互层，数据仍走原有 API，状态集中在 usePoolsData，列表刷新与筛选参数与之前同一数据源。
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
Go（middleware + model，节选）
ok github.com/QuantumNous/new-api/middleware 0.140s
ok github.com/QuantumNous/new-api/model 0.169s
命令：go test ./middleware/... ./model/... -count=1 -short
前端（Pool hook 单测）
✓ src/hooks/pools/usePoolsData.test.jsx (4 tests) 114ms
Test Files 1 passed (1)
Tests 4 passed (4)
命令：npx vitest run src/hooks/pools/usePoolsData.test.jsx（Vitest 退出时若提示 close timed out，属环境/插件问题，用例已通过、exit code 0。）
Summary by CodeRabbit
New Features
Configuration
Chores
Tests