Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.go📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🔇 Additional comments (9)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR adds named auto-route chains across backend routing, controller responses, frontend configuration/UI, and locale text, while also enforcing token-group assignment rules and adding panic recovery in the timeout runner. ChangesNamed Auto Routes
Panic Recovery in Timeout Runner
Estimated code review effort: 5 (Critical) | ~120 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
main.go (1)
255-265: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueCorrect defer ordering; panic recovery works as intended.
close(done)is deferred first, so it runs after the recover-defer completes (LIFO), meaningdoneis still closed on panic and the caller getstrue. Logging viacommon.SysErrormatches the upstream helper.One consideration: silently returning
truewhenfnpanicked hides the failure from the caller — only a log line signals it happened. If callers rely on the boolean to gate follow-up behavior, this could be worth a short doc comment noting that a panic infnis treated as "completed" (not "succeeded").🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@main.go` around lines 255 - 265, The runWithTimeout helper currently treats a panic in fn as a successful completion because the recovered panic only gets logged and the returned bool still indicates completion. Update runWithTimeout to make this behavior explicit by documenting it in a short comment near the runWithTimeout function or by changing the completion signal so callers can distinguish panic from success; use the existing runWithTimeout and common.SysError symbols to locate the code.web/default/src/features/pricing/components/model-details.tsx (1)
842-850: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated
AutoGroupChaininvocation across 5 branches.The identical
<AutoGroupChain model={props.model} autoGroups={props.autoGroups} autoRoutes={props.autoRoutes} />call is repeated in every early-return branch ofGroupPricingSection. Consider computing the element once (e.g.const autoChain = <AutoGroupChain .../>) at the top of the function and reusing it, reducing duplication and function length.Also applies to: 863-872, 923-931, 970-977, 1050-1057
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/default/src/features/pricing/components/model-details.tsx` around lines 842 - 850, In GroupPricingSection, the same AutoGroupChain element is duplicated across multiple early-return branches. Create a single reusable element near the top of the function (for example, an autoChain constant using props.model, props.autoGroups, and props.autoRoutes) and render that in each branch instead of repeating the JSX. Apply this cleanup across all branches where the identical AutoGroupChain invocation appears to reduce duplication and keep the function shorter.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@controller/group.go`:
- Around line 40-46: The auto-route population in GetUserAutoGroups is using
route.Key directly, which can overwrite existing ratio-group entries when a
ratio group shares the same namespace like auto or auto:*. Update the logic
around service.GetUserAutoRoutes(userGroup, true) so auto-route keys cannot
collide with ratio_setting names, either by validating the key set against
existing ratio groups before inserting or by storing auto routes under a
separate namespace/merged structure instead of writing straight into
usableGroups[route.Key].
In `@controller/token.go`:
- Line 47: The authorization error in token handling is hardcoded Chinese text
instead of using i18n. Update the error path in the token controller to use
common.ApiErrorI18n with the new i18n key MsgTokenGroupNotAssignable, and add
that message key to the i18n message set so the group-access denial is localized
consistently with the other errors in this file.
In `@web/default/src/features/keys/components/api-keys-mutate-drawer.tsx`:
- Around line 103-106: The fallback in api-keys-mutate-drawer.tsx is hardcoding
the auto-route key instead of using the shared source of truth. Update the
defaultAutoRoute assignment to use DEFAULT_AUTO_ROUTE_KEY from auto-routes.ts,
matching the existing isAutoRouteKey import in this file and removing the
literal 'auto' fallback. Keep the same status.default_auto_route check, but
return the shared constant when the status value is not a string.
In `@web/default/src/features/keys/lib/api-key-form.ts`:
- Around line 79-88: The default value in getApiKeyFormDefaultValues currently
hardcodes the auto route key as 'auto', duplicating the same value used
elsewhere. Update the function signature in api-key-form.ts to use
DEFAULT_AUTO_ROUTE_KEY instead of the literal default, so the default route key
comes from a single source of truth and stays aligned with
api-keys-mutate-drawer.tsx.
In `@web/default/src/features/pricing/components/model-details.tsx`:
- Around line 742-792: The fallback route label in AutoGroupChain is hardcoded
as “Auto” and bypasses i18n. Update the fallback route object so its display
name comes from t() via useTranslation(), and ensure the rendered
label={route.name || route.key} uses the translated value rather than a literal
string. Keep the change localized to AutoGroupChain and preserve the existing
GroupBadge rendering logic.
In
`@web/default/src/features/system-settings/models/group-ratio-visual-editor.tsx`:
- Around line 350-358: The default key in handleAutoRouteAdd can collide with an
existing auto route, so update the add flow to generate or select a unique
suggested key instead of always using 'auto:fast'. Use the existing auto-route
state helpers around setAutoRouteDialogData and any route list/accessors in
group-ratio-visual-editor.tsx to check for conflicts and prefill a
non-conflicting key before opening the dialog.
- Around line 890-931: The route group action buttons in the group list are
icon-only and lack accessible names, so add `aria-label` to the up/down move
buttons and the `Trash2` delete button in the `route.groups.map` block. Use
clear labels consistent with the existing `removeRow` button pattern in this
file, and keep the icon decorative by marking it hidden from assistive tech if
needed.
In `@web/default/src/features/system-settings/models/ratio-settings-card.tsx`:
- Around line 455-464: `saveGroupRatios` currently calls
`parseAutoGroupRoutesConfigStrict` without protection, so a bad persisted
`AutoGroupRoutes` value can throw synchronously and block saves. Update the
`saveGroupRatios` flow in `ratio-settings-card.tsx` to catch parse failures and
surface a user-visible error (or otherwise abort gracefully) before building
`normalized`, and keep the existing normalization helpers like
`formatAutoGroupRoutesForTextarea`/`stringifyAutoGroupRoutesConfig` as-is. Also
confirm `GroupRatioForm`’s `onSave` is only reached through
`groupForm.handleSubmit(...)` so the zod `superRefine` validation runs first in
the normal path.
In `@web/default/src/lib/auto-routes.test.ts`:
- Around line 27-90: Add unit coverage for the lenient normalization path and
getDefaultAutoRouteGroups in auto-routes.test.ts, since only
parseAutoGroupRoutesConfigStrict is currently exercised. Add a test around
parseAutoGroupRoutesConfig/normalizeAutoGroupRoutesConfig that verifies a config
with an existing but disabled default route is still normalized successfully,
and assert getDefaultAutoRouteGroups returns the expected groups for that case.
Keep the test focused on the helper behavior used by ratio-settings-card.tsx
textarea formatting and save flow.
In `@web/default/src/lib/auto-routes.ts`:
- Around line 95-112: The default route name in createLegacyAutoRouteConfig is
hardcoded to English and can bypass the localized label path used by
AutoGroupChain and GroupBadge. Remove the literal name for the default route or
ensure model-details.tsx does not pass a label override for
DEFAULT_AUTO_ROUTE_KEY so GroupBadge can use its own translated getGroupLabel
behavior. Keep the default route key handling intact while letting downstream UI
render the i18n-aware “Auto” label.
- Around line 55-84: The validation errors emitted by the auto-route config
helpers are still hardcoded English strings and are passed through to
user-facing form issues. Update the relevant validators in auto-routes.ts,
especially normalizeGroupListStrict and the related auto route key/config checks
used by validateAutoGroupRoutesConfigString, to generate messages via i18n using
t() instead of raw Error text. Keep the existing validation behavior, but ensure
all messages surfaced to ratio-settings-card.tsx remain localized and consistent
with the sibling AutoGroups/GroupRatio patterns.
---
Outside diff comments:
In `@main.go`:
- Around line 255-265: The runWithTimeout helper currently treats a panic in fn
as a successful completion because the recovered panic only gets logged and the
returned bool still indicates completion. Update runWithTimeout to make this
behavior explicit by documenting it in a short comment near the runWithTimeout
function or by changing the completion signal so callers can distinguish panic
from success; use the existing runWithTimeout and common.SysError symbols to
locate the code.
In `@web/default/src/features/pricing/components/model-details.tsx`:
- Around line 842-850: In GroupPricingSection, the same AutoGroupChain element
is duplicated across multiple early-return branches. Create a single reusable
element near the top of the function (for example, an autoChain constant using
props.model, props.autoGroups, and props.autoRoutes) and render that in each
branch instead of repeating the JSX. Apply this cleanup across all branches
where the identical AutoGroupChain invocation appears to reduce duplication and
keep the function shorter.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: ea3f938d-64fe-4020-b1fe-bf8d5c66e864
📒 Files selected for processing (48)
controller/group.gocontroller/misc.gocontroller/model.gocontroller/option.gocontroller/perf_metrics.gocontroller/pricing.gocontroller/token.gocontroller/token_test.gocontroller/user.gomain.gomain_test.gomiddleware/auth.gomiddleware/distributor.gomodel/option.goservice/channel_select.goservice/group.goservice/group_test.gosetting/auto_group.gosetting/auto_group_test.goweb/default/src/components/group-badge.tsxweb/default/src/components/model-group-selector.tsxweb/default/src/features/keys/components/api-keys-columns.tsxweb/default/src/features/keys/components/api-keys-mutate-drawer.tsxweb/default/src/features/keys/lib/api-key-form.tsweb/default/src/features/models/components/drawers/model-mutate-drawer.tsxweb/default/src/features/playground/types.tsweb/default/src/features/pricing/components/model-details.tsxweb/default/src/features/pricing/hooks/use-pricing-data.tsweb/default/src/features/pricing/index.tsxweb/default/src/features/pricing/lib/mock-stats.tsweb/default/src/features/pricing/lib/model-helpers.tsweb/default/src/features/pricing/types.tsweb/default/src/features/system-settings/billing/index.tsxweb/default/src/features/system-settings/billing/section-registry.tsxweb/default/src/features/system-settings/models/group-ratio-form.tsxweb/default/src/features/system-settings/models/group-ratio-visual-editor.tsxweb/default/src/features/system-settings/models/index.tsxweb/default/src/features/system-settings/models/ratio-settings-card.tsxweb/default/src/features/system-settings/types.tsweb/default/src/i18n/locales/en.jsonweb/default/src/i18n/locales/fr.jsonweb/default/src/i18n/locales/ja.jsonweb/default/src/i18n/locales/ru.jsonweb/default/src/i18n/locales/vi.jsonweb/default/src/i18n/locales/zh.jsonweb/default/src/lib/api.tsweb/default/src/lib/auto-routes.test.tsweb/default/src/lib/auto-routes.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
web/default/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/default/AGENTS.md)
web/default/src/**/*.{ts,tsx}: 前端页面文本与组件内文案必须支持 i18n：React 组件中应使用useTranslation()取得t，并通过t()渲染用户可见文本；子组件也应自行使用useTranslation()保持独立性。
禁止使用两层及以上嵌套三元表达式；复杂逻辑应改用if-else、提前返回或抽取函数。
控制函数圈复杂度，复杂逻辑应拆成更小的函数；变量与函数命名应有意义并遵循驼峰等常规命名约定。
TypeScript 代码应避免使用any，优先使用具体类型或unknown；参数与返回值应显式标注类型；仅类型用途的导入应使用import type。
修改 TypeScript 或 TSX 代码后必须执行类型检查，并修复所有类型错误，不得遗留。
对象非必要不要解构，尤其是组件 props；优先直接使用props.xxx以保持代码清晰。
组件应使用函数式组件与 Hooks，遵循单一职责；组件 props 必须有明确类型（接口或类型别名）。
单文件超过约 200 行时应考虑拆分子组件或抽取自定义 Hooks；类型定义可与组件同文件或放在同模块的types中。
在 React 中应合理使用useMemo、useCallback、React.memo，避免在渲染路径中创建新对象或数组；必要时进行代码分割与动态import。
React Query 的数据获取应使用useQuery、变更应使用useMutation；每个查询需配置唯一queryKey，并在成功后对相关查询执行invalidateQueries；服务端错误应统一交给handleServerError。
Axios 请求应使用项目统一的api实例；GET 请求默认去重，特殊请求可显式关闭；认证与通用错误应在拦截器中统一处理。
服务端错误应统一使用handleServerError，展示层应使用toast.error等统一方式；文案需走 i18n；路由级错误应由errorComponent承接；表单错误应通过form.setError等方式映射到字段。
样式应以 Tailwind 工具类为主，动态类名使用cn()合并；非动态场景避免内联样式；响应式采用移动优先与 Tailwind 断点，主题与暗色模式通过 CSS 变量与dark:处理。
应使用语义化 HTML、正确关联label与输入、保证键盘可操作与合理焦点顺序；必要时添加 ARIA 属性，装饰性图标应使用aria-hidden="true"。
认证与权限应在路由与接口层校验；前后端都应做数据校验（如 Zod）；敏感信息不得落前端存储；避免使用dangerouslySetInnerHTML；跨域与 Cookie 需配合withCredentials并按后端要求处理 CSRF。
组件测试应使用 React Testing Library，关注交互与行为，避免测试实现细节；关键流程可补充集成与 E2E 测试。
环境变量应通过.env读取，并使用VITE_前缀；代码中不得硬编码密钥。
Files:
web/default/src/features/playground/types.tsweb/default/src/lib/auto-routes.test.tsweb/default/src/features/pricing/hooks/use-pricing-data.tsweb/default/src/features/models/components/drawers/model-mutate-drawer.tsxweb/default/src/features/system-settings/billing/index.tsxweb/default/src/components/group-badge.tsxweb/default/src/features/system-settings/models/index.tsxweb/default/src/features/system-settings/billing/section-registry.tsxweb/default/src/lib/api.tsweb/default/src/features/keys/components/api-keys-columns.tsxweb/default/src/features/pricing/types.tsweb/default/src/features/system-settings/types.tsweb/default/src/features/pricing/lib/mock-stats.tsweb/default/src/components/model-group-selector.tsxweb/default/src/features/keys/lib/api-key-form.tsweb/default/src/features/keys/components/api-keys-mutate-drawer.tsxweb/default/src/features/pricing/lib/model-helpers.tsweb/default/src/features/pricing/index.tsxweb/default/src/features/system-settings/models/ratio-settings-card.tsxweb/default/src/features/system-settings/models/group-ratio-form.tsxweb/default/src/features/pricing/components/model-details.tsxweb/default/src/features/system-settings/models/group-ratio-visual-editor.tsxweb/default/src/lib/auto-routes.ts
web/default/src/features/**
📄 CodeRabbit inference engine (web/default/AGENTS.md)
功能模块应放在
src/features/<feature>/，并按需包含components/、lib/、hooks/、api.ts、types.ts、constants.ts等；通用组件应放在src/components/，通用工具与类型应放在src/lib/。
Files:
web/default/src/features/playground/types.tsweb/default/src/features/pricing/hooks/use-pricing-data.tsweb/default/src/features/models/components/drawers/model-mutate-drawer.tsxweb/default/src/features/system-settings/billing/index.tsxweb/default/src/features/system-settings/models/index.tsxweb/default/src/features/system-settings/billing/section-registry.tsxweb/default/src/features/keys/components/api-keys-columns.tsxweb/default/src/features/pricing/types.tsweb/default/src/features/system-settings/types.tsweb/default/src/features/pricing/lib/mock-stats.tsweb/default/src/features/keys/lib/api-key-form.tsweb/default/src/features/keys/components/api-keys-mutate-drawer.tsxweb/default/src/features/pricing/lib/model-helpers.tsweb/default/src/features/pricing/index.tsxweb/default/src/features/system-settings/models/ratio-settings-card.tsxweb/default/src/features/system-settings/models/group-ratio-form.tsxweb/default/src/features/pricing/components/model-details.tsxweb/default/src/features/system-settings/models/group-ratio-visual-editor.tsx
web/default/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Use
bunas the preferred package manager and script runner for the frontend inweb/default/(bun install,bun run dev,bun run build, andbun run i18n:*).
Files:
web/default/src/features/playground/types.tsweb/default/src/lib/auto-routes.test.tsweb/default/src/features/pricing/hooks/use-pricing-data.tsweb/default/src/features/models/components/drawers/model-mutate-drawer.tsxweb/default/src/features/system-settings/billing/index.tsxweb/default/src/components/group-badge.tsxweb/default/src/features/system-settings/models/index.tsxweb/default/src/features/system-settings/billing/section-registry.tsxweb/default/src/lib/api.tsweb/default/src/features/keys/components/api-keys-columns.tsxweb/default/src/features/pricing/types.tsweb/default/src/features/system-settings/types.tsweb/default/src/features/pricing/lib/mock-stats.tsweb/default/src/components/model-group-selector.tsxweb/default/src/features/keys/lib/api-key-form.tsweb/default/src/features/keys/components/api-keys-mutate-drawer.tsxweb/default/src/features/pricing/lib/model-helpers.tsweb/default/src/features/pricing/index.tsxweb/default/src/features/system-settings/models/ratio-settings-card.tsxweb/default/src/features/system-settings/models/group-ratio-form.tsxweb/default/src/features/pricing/components/model-details.tsxweb/default/src/i18n/locales/fr.jsonweb/default/src/i18n/locales/vi.jsonweb/default/src/features/system-settings/models/group-ratio-visual-editor.tsxweb/default/src/i18n/locales/en.jsonweb/default/src/i18n/locales/zh.jsonweb/default/src/i18n/locales/ru.jsonweb/default/src/i18n/locales/ja.jsonweb/default/src/lib/auto-routes.ts
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrappers incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly callingencoding/json;encoding/jsontypes likejson.RawMessageandjson.Numbermay still be used as types.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+ simultaneously: prefer GORM methods over raw SQL, avoid directAUTO_INCREMENT/SERIAL, use cross-DB quoting/boolean helpers when raw SQL is unavoidable, avoid DB-specific functions/operators without fallbacks, and ensure migrations work on all three databases.
For request structs parsed from client JSON and later re-marshaled to upstream providers, especially in relay/convert paths, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved and omitted-only fields remainnil.
**/*.go: Usecommon.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson, andcommon.GetJsonTypefromcommon/json.gofor all JSON marshal/unmarshal operations; do not directly callencoding/jsonin business code.
Keep all database code fully compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless cross-DB-safe, and use the provided DB-specific helpers/flags for quoting, booleans, and branching.
For request structs parsed from client JSON and re-marshaled to upstream providers, especially in relay/convert paths, use pointer types withomitemptyfor optional scalar fields so explicit zero/false values are preserved.
Files:
main_test.gocontroller/pricing.gocontroller/misc.gomain.goservice/group_test.goservice/channel_select.gocontroller/perf_metrics.gocontroller/option.gomiddleware/distributor.gocontroller/token.gocontroller/user.gomiddleware/auth.gomodel/option.gosetting/auto_group_test.goservice/group.gocontroller/model.gocontroller/group.gocontroller/token_test.gosetting/auto_group.go
web/default/src/**/*.test.ts
📄 CodeRabbit inference engine (web/default/AGENTS.md)
工具函数与纯逻辑应优先编写单元测试；测试文件应命名为
*.test.ts。
Files:
web/default/src/lib/auto-routes.test.ts
web/default/src/features/**/lib/**/*.ts
📄 CodeRabbit inference engine (web/default/AGENTS.md)
表单应使用 React Hook Form + Zod：在功能模块的
lib/下定义 schema，并用z.infer导出表单类型；useForm应配合@hookform/resolvers/zod进行校验。
Files:
web/default/src/features/pricing/lib/mock-stats.tsweb/default/src/features/keys/lib/api-key-form.tsweb/default/src/features/pricing/lib/model-helpers.ts
🔇 Additional comments (62)
main_test.go (1)
27-31: LGTM!web/default/src/lib/api.ts (1)
199-209: LGTM!web/default/src/components/group-badge.tsx (1)
20-20: LGTM!Also applies to: 52-54, 69-69
web/default/src/features/keys/components/api-keys-columns.tsx (1)
23-23: LGTM!Also applies to: 203-214
web/default/src/features/keys/components/api-keys-mutate-drawer.tsx (1)
27-27: LGTM!Also applies to: 132-142, 155-169, 181-181, 338-338
web/default/src/features/keys/lib/api-key-form.ts (1)
21-21: LGTM!Also applies to: 113-115
web/default/src/features/system-settings/models/ratio-settings-card.tsx (2)
26-32: LGTM!Also applies to: 218-236, 295-298, 335-338, 386-389, 403-406
179-192: 📐 Maintainability & Code QualityRemove the dead-code concern for
AutoGroups.group-ratio-form.tsxstill renders and bindsAutoGroups, so its zod validation and default-value wiring are still used.> Likely an incorrect or invalid review comment.web/default/src/lib/auto-routes.ts (1)
35-35: 🗄️ Data Integrity & IntegrationNo change needed for auto-route schema alignment.
web/default/src/components/model-group-selector.tsx (1)
56-56: LGTM!web/default/src/features/pricing/components/model-details.tsx (1)
805-807: LGTM!Also applies to: 1185-1188, 1254-1254, 1333-1333, 1412-1412
web/default/src/features/pricing/hooks/use-pricing-data.ts (1)
70-70: LGTM!web/default/src/features/pricing/index.tsx (1)
21-21: LGTM!Also applies to: 51-51, 100-106, 279-283
web/default/src/features/pricing/lib/mock-stats.ts (1)
313-315: LGTM!Also applies to: 813-815
web/default/src/features/pricing/lib/model-helpers.ts (1)
19-19: LGTM!Also applies to: 30-42
web/default/src/features/pricing/types.ts (1)
19-20: 🎯 Functional CorrectnessCheck whether
usable_group.auto/usable_group.groupsare used anywhere. No code inweb/default/srcreads these fields, so they may be dead additions toPricingData.web/default/src/features/system-settings/models/group-ratio-visual-editor.tsx (2)
337-348: See root-cause comment in group-ratio-form.tsx.
emitAutoRoutesConfigis the only place that keeps the legacyAutoGroupsfield in sync withAutoGroupRoutes, but it only runs on visual-editor mutations — not when a user edits theAutoGroupRoutesJSON textarea directly in JSON mode. See the comment ingroup-ratio-form.tsx(AutoGroups FormField) for the downstream impact.
22-31: LGTM!Also applies to: 64-64, 73-73, 95-100, 185-220, 230-237, 261-264, 417-484, 784-937, 948-1021, 1087-1094, 1316-1325, 1406-1415
web/default/src/features/playground/types.ts (1)
140-140: LGTM!web/default/src/features/system-settings/models/group-ratio-form.tsx (2)
450-475: LGTM!
68-68: 🗄️ Data Integrity & Integration
AutoGroupsis already derived on save.saveGroupRatiosrecomputesAutoGroupsfromAutoGroupRoutesbefore persisting, so direct JSON edits do not save a stale value.> Likely an incorrect or invalid review comment.web/default/src/i18n/locales/ja.json (1)
172-172: LGTM!Also applies to: 236-236, 542-544, 942-942, 991-991, 1272-1273, 1513-1513, 1657-1657, 1906-1906, 2245-2245, 2322-2322, 2365-2365, 3860-3863, 4114-4114, 4862-4862, 5072-5072
web/default/src/i18n/locales/ru.json (1)
172-172: LGTM!Also applies to: 236-236, 542-544, 942-942, 991-991, 1272-1273, 1513-1513, 1657-1657, 1906-1906, 2245-2245, 2322-2322, 2365-2365, 3860-3863, 4114-4114, 4862-4862, 5072-5072
web/default/src/i18n/locales/vi.json (1)
172-172: LGTM!Also applies to: 236-236, 542-544, 942-942, 991-991, 1272-1273, 1513-1513, 1657-1657, 1906-1906, 2245-2245, 2322-2322, 2365-2365, 3860-3863, 4009-4017, 4114-4114, 4862-4862, 5072-5072
web/default/src/i18n/locales/zh.json (1)
172-172: LGTM!Also applies to: 236-236, 542-544, 942-942, 991-991, 1272-1273, 1513-1513, 1657-1657, 1906-1906, 2245-2245, 2322-2322, 2365-2365, 3860-3863, 4016-4017, 4114-4114, 4862-4862, 5072-5072
web/default/src/features/system-settings/models/index.tsx (1)
64-64: LGTM!web/default/src/features/models/components/drawers/model-mutate-drawer.tsx (1)
193-193: LGTM!web/default/src/features/system-settings/billing/index.tsx (1)
60-60: LGTM!web/default/src/features/system-settings/billing/section-registry.tsx (1)
44-54: LGTM!web/default/src/features/system-settings/types.ts (1)
227-227: LGTM!Also applies to: 273-273
web/default/src/i18n/locales/en.json (2)
172-172: LGTM!Also applies to: 236-236, 542-544, 942-942, 991-991, 1272-1273, 1513-1513, 1657-1657, 1906-1906, 2322-2322, 2365-2365, 3860-3863, 4009-4017, 4114-4114, 4862-4862, 5072-5072
2244-2245: 📐 Maintainability & Code QualityLegacy auto-group string is no longer used — the form now references the
configured default auto routetext, so the oldstarts with autolocale entry looks stale.> Likely an incorrect or invalid review comment.web/default/src/i18n/locales/fr.json (2)
1272-1273: 📐 Maintainability & Code Quality | ⚡ Quick winMinor wording drift in French translation.
"Default auto route must be enabled" is translated as "doit rester activée" (must remain enabled) rather than "doit être activée" (must be enabled). Low-impact wording nuance.
💬 Suggested wording fix
- "Default auto route must be enabled": "La route auto par défaut doit rester activée", + "Default auto route must be enabled": "La route auto par défaut doit être activée",
172-172: LGTM!Also applies to: 236-236, 542-544, 942-942, 991-991, 1513-1513, 1657-1657, 1906-1906, 2245-2245, 2322-2322, 2365-2365, 3860-3863, 4009-4017, 4114-4114, 4862-4862, 5072-5072
setting/auto_group.go (3)
29-52: LGTM!
84-162: LGTM!
164-366: LGTM!setting/auto_group_test.go (1)
9-121: LGTM!model/option.go (2)
126-126: LGTM!
413-422: LGTM!controller/misc.go (1)
95-95: LGTM!controller/option.go (1)
236-244: LGTM!middleware/auth.go (1)
472-479: LGTM!service/group.go (3)
43-62: LGTM!
64-117: LGTM!
39-41: 🎯 Functional CorrectnessNo in-repo callers to audit
GroupInUserUsableGroupsisn’t referenced anywhere else in the repo; it just aliasesCanUseTokenGroup, so the caller-compatibility concern doesn’t apply here.> Likely an incorrect or invalid review comment.service/channel_select.go (1)
52-53: LGTM!Also applies to: 90-94
controller/group.go (1)
7-8: LGTM!controller/model.go (1)
21-21: LGTM!Also applies to: 182-182, 200-204, 273-273
controller/perf_metrics.go (2)
8-8: LGTM!Also applies to: 23-26
80-85: 🎯 Functional Correctness | ⚡ Quick winInconsistent auto-route filtering vs.
GetPerfMetricsSummary.
GetPerfMetricsSummary(Lines 23-26) restrictsactiveGroupsto the exact set of currently configured route keys viasetting.GetAutoRoutes().filterActiveGroupshere instead accepts any group matching the loosesetting.IsAutoRouteKeyprefix pattern, so a deleted/renamed route (e.g. old"auto:legacy"data) will keep showing as "active" indefinitely inGetPerfMetrics, while it would correctly disappear fromGetPerfMetricsSummary. Consider filtering against the samesetting.GetAutoRoutes()key set for consistency.🐛 Proposed fix for consistent filtering
func filterActiveGroups(groups []perfmetrics.GroupResult) []perfmetrics.GroupResult { activeRatios := ratio_setting.GetGroupRatioCopy() + activeRoutes := make(map[string]struct{}) + for _, route := range setting.GetAutoRoutes() { + activeRoutes[route.Key] = struct{}{} + } return lo.Filter(groups, func(g perfmetrics.GroupResult, _ int) bool { _, ok := activeRatios[g.Group] - return ok || setting.IsAutoRouteKey(g.Group) + if ok { + return true + } + _, ok = activeRoutes[g.Group] + return ok }) }middleware/distributor.go (1)
94-99: LGTM!Also applies to: 106-115, 143-148
service/group_test.go (1)
1-51: LGTM!controller/pricing.go (1)
67-77: LGTM!controller/user.go (1)
254-256: LGTM!controller/token.go (3)
178-181: LGTM!
296-299: LGTM!
35-49: 🎯 Functional CorrectnessNo issue here: middleware populates both
groupanduser_group, andCanUseTokenGroupalready checks the configured allowed groups whenuserGroupis empty.> Likely an incorrect or invalid review comment.controller/token_test.go (4)
118-161: LGTM!
255-258: LGTM!
524-558: LGTM!
596-632: LGTM!
| function AutoGroupChain(props: { | ||
| model: PricingModel | ||
| autoGroups: string[] | ||
| autoRoutes?: AutoGroupRoute[] | ||
| }) { | ||
| const { t } = useTranslation() | ||
| const modelEnableGroups = Array.isArray(props.model.enable_groups) | ||
| ? props.model.enable_groups | ||
| : [] | ||
| const autoChain = props.autoGroups.filter((g) => | ||
| modelEnableGroups.includes(g) | ||
| ) | ||
|
|
||
| if (autoChain.length === 0) return null | ||
| const routes = | ||
| props.autoRoutes && props.autoRoutes.length > 0 | ||
| ? props.autoRoutes | ||
| : [ | ||
| { | ||
| key: 'auto', | ||
| name: 'Auto', | ||
| enabled: true, | ||
| user_selectable: true, | ||
| groups: props.autoGroups, | ||
| }, | ||
| ] | ||
| const routeChains = routes | ||
| .filter((route) => route.enabled) | ||
| .map((route) => ({ | ||
| route, | ||
| groups: route.groups.filter((g) => modelEnableGroups.includes(g)), | ||
| })) | ||
| .filter((item) => item.groups.length > 0) | ||
|
|
||
| if (routeChains.length === 0) return null | ||
|
|
||
| return ( | ||
| <div className='text-muted-foreground mb-3 flex flex-wrap items-center gap-1 text-xs'> | ||
| <span className='font-medium'>{t('Auto Group Chain')}</span> | ||
| <span className='text-muted-foreground/40'>→</span> | ||
| {autoChain.map((g, idx) => ( | ||
| <span key={g} className='flex items-center gap-1'> | ||
| <GroupBadge group={g} size='sm' /> | ||
| {idx < autoChain.length - 1 && ( | ||
| <span className='text-muted-foreground/40'>→</span> | ||
| )} | ||
| </span> | ||
| <div className='text-muted-foreground mb-3 flex flex-col gap-1.5 text-xs'> | ||
| <span className='font-medium'>{t('Auto Route Chains')}</span> | ||
| {routeChains.map(({ route, groups }) => ( | ||
| <div key={route.key} className='flex flex-wrap items-center gap-1'> | ||
| <GroupBadge | ||
| group={route.key} | ||
| label={route.name || route.key} | ||
| size='sm' | ||
| /> | ||
| <span className='text-muted-foreground/40'>→</span> | ||
| {groups.map((g, idx) => ( | ||
| <span key={g} className='flex items-center gap-1'> | ||
| <GroupBadge group={g} size='sm' /> | ||
| {idx < groups.length - 1 && ( | ||
| <span className='text-muted-foreground/40'>→</span> | ||
| )} | ||
| </span> | ||
| ))} | ||
| </div> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Hardcoded 'Auto' label bypasses i18n.
The fallback route's name: 'Auto' is rendered directly as label={route.name || route.key} without going through t(), unlike other user-facing strings in this file (e.g. t('Auto Route Chains')).
As per coding guidelines, "前端页面文本与组件内文案必须支持 i18n：React 组件中应使用 useTranslation() 取得 t，并通过 t() 渲染用户可见文本".
🌐 Proposed fix
: [
{
key: 'auto',
- name: 'Auto',
+ name: t('Auto'),
enabled: true,
user_selectable: true,
groups: props.autoGroups,
},
]📝 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.
| function AutoGroupChain(props: { | |
| model: PricingModel | |
| autoGroups: string[] | |
| autoRoutes?: AutoGroupRoute[] | |
| }) { | |
| const { t } = useTranslation() | |
| const modelEnableGroups = Array.isArray(props.model.enable_groups) | |
| ? props.model.enable_groups | |
| : [] | |
| const autoChain = props.autoGroups.filter((g) => | |
| modelEnableGroups.includes(g) | |
| ) | |
| if (autoChain.length === 0) return null | |
| const routes = | |
| props.autoRoutes && props.autoRoutes.length > 0 | |
| ? props.autoRoutes | |
| : [ | |
| { | |
| key: 'auto', | |
| name: 'Auto', | |
| enabled: true, | |
| user_selectable: true, | |
| groups: props.autoGroups, | |
| }, | |
| ] | |
| const routeChains = routes | |
| .filter((route) => route.enabled) | |
| .map((route) => ({ | |
| route, | |
| groups: route.groups.filter((g) => modelEnableGroups.includes(g)), | |
| })) | |
| .filter((item) => item.groups.length > 0) | |
| if (routeChains.length === 0) return null | |
| return ( | |
| <div className='text-muted-foreground mb-3 flex flex-wrap items-center gap-1 text-xs'> | |
| <span className='font-medium'>{t('Auto Group Chain')}</span> | |
| <span className='text-muted-foreground/40'>→</span> | |
| {autoChain.map((g, idx) => ( | |
| <span key={g} className='flex items-center gap-1'> | |
| <GroupBadge group={g} size='sm' /> | |
| {idx < autoChain.length - 1 && ( | |
| <span className='text-muted-foreground/40'>→</span> | |
| )} | |
| </span> | |
| <div className='text-muted-foreground mb-3 flex flex-col gap-1.5 text-xs'> | |
| <span className='font-medium'>{t('Auto Route Chains')}</span> | |
| {routeChains.map(({ route, groups }) => ( | |
| <div key={route.key} className='flex flex-wrap items-center gap-1'> | |
| <GroupBadge | |
| group={route.key} | |
| label={route.name || route.key} | |
| size='sm' | |
| /> | |
| <span className='text-muted-foreground/40'>→</span> | |
| {groups.map((g, idx) => ( | |
| <span key={g} className='flex items-center gap-1'> | |
| <GroupBadge group={g} size='sm' /> | |
| {idx < groups.length - 1 && ( | |
| <span className='text-muted-foreground/40'>→</span> | |
| )} | |
| </span> | |
| ))} | |
| </div> | |
| function AutoGroupChain(props: { | |
| model: PricingModel | |
| autoGroups: string[] | |
| autoRoutes?: AutoGroupRoute[] | |
| }) { | |
| const { t } = useTranslation() | |
| const modelEnableGroups = Array.isArray(props.model.enable_groups) | |
| ? props.model.enable_groups | |
| : [] | |
| const routes = | |
| props.autoRoutes && props.autoRoutes.length > 0 | |
| ? props.autoRoutes | |
| : [ | |
| { | |
| key: 'auto', | |
| name: t('Auto'), | |
| enabled: true, | |
| user_selectable: true, | |
| groups: props.autoGroups, | |
| }, | |
| ] | |
| const routeChains = routes | |
| .filter((route) => route.enabled) | |
| .map((route) => ({ | |
| route, | |
| groups: route.groups.filter((g) => modelEnableGroups.includes(g)), | |
| })) | |
| .filter((item) => item.groups.length > 0) | |
| if (routeChains.length === 0) return null | |
| return ( | |
| <div className='text-muted-foreground mb-3 flex flex-col gap-1.5 text-xs'> | |
| <span className='font-medium'>{t('Auto Route Chains')}</span> | |
| {routeChains.map(({ route, groups }) => ( | |
| <div key={route.key} className='flex flex-wrap items-center gap-1'> | |
| <GroupBadge | |
| group={route.key} | |
| label={route.name || route.key} | |
| size='sm' | |
| /> | |
| <span className='text-muted-foreground/40'>→</span> | |
| {groups.map((g, idx) => ( | |
| <span key={g} className='flex items-center gap-1'> | |
| <GroupBadge group={g} size='sm' /> | |
| {idx < groups.length - 1 && ( | |
| <span className='text-muted-foreground/40'>→</span> | |
| )} | |
| </span> | |
| ))} | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/default/src/features/pricing/components/model-details.tsx` around lines
742 - 792, The fallback route label in AutoGroupChain is hardcoded as “Auto” and
bypasses i18n. Update the fallback route object so its display name comes from
t() via useTranslation(), and ensure the rendered label={route.name ||
route.key} uses the translated value rather than a literal string. Keep the
change localized to AutoGroupChain and preserve the existing GroupBadge
rendering logic.
Source: Coding guidelines
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
controller/group.go (1)
26-61: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winGuarded insert resolves prior shadowing bug — but protection is order-dependent.
The new
addAutoRouteUsableGroupno-ops when the key already exists, fixing the previously flagged issue where auto-route keys could overwrite ratio-group entries. This works today because ratio groups are inserted (lines 32-40, unconditional overwrite) before auto routes (lines 41-43, guarded). If this ordering is ever changed, the ratio-group loop would silently overwrite an already-inserted auto route since it has no existence check. Consider guarding the ratio-group loop symmetrically, or adding a comment documenting the ordering invariant this relies on.♻️ Proposed symmetric guard
for groupName, _ := range ratio_setting.GetGroupRatioCopy() { // UserUsableGroups contains the groups that the user can use if desc, ok := userUsableGroups[groupName]; ok { + if _, exists := usableGroups[groupName]; exists { + continue + } usableGroups[groupName] = map[string]interface{}{ "ratio": service.GetUserGroupRatio(userGroup, groupName), "desc": desc, } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controller/group.go` around lines 26 - 61, The insertion logic in GetUserGroups is order-dependent because the ratio-group loop writes into usableGroups without checking for existing keys, while addAutoRouteUsableGroup is guarded. Update the GetUserGroups loop to use the same existence check before assigning, or add an explicit comment/invariant near GetUserUsableGroups and addAutoRouteUsableGroup documenting that ratio groups must not overwrite auto-route entries. Ensure the protection is symmetric so future reordering does not reintroduce the overwrite bug.web/default/src/features/system-settings/models/group-ratio-visual-editor.tsx (2)
417-427: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDeleting the default route can silently promote a disabled route to default.
nextConfig.default_route = nextConfig.routes[0]?.key ?? 'auto'doesn't check.enabled. If the first remaining route happens to be disabled, the config now has a disabled default route.emitAutoRoutesConfigonly lenient-normalizes (per the new test "keeps disabled default route available in lenient config reads"), so this invalid state is silently persisted into the form and only surfaces later as a generic parse error whenparseAutoGroupRoutesConfigStrictruns on Save — with the UI meanwhile showing both "Default" and "Disabled" badges on the same route.🐛 Proposed fix: pick an enabled route as the new default
if (nextConfig.default_route === routeKey) { - nextConfig.default_route = nextConfig.routes[0]?.key ?? 'auto' + nextConfig.default_route = + nextConfig.routes.find((route) => route.enabled)?.key ?? + nextConfig.routes[0]?.key ?? + 'auto' }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/default/src/features/system-settings/models/group-ratio-visual-editor.tsx` around lines 417 - 427, When deleting the current default in handleAutoRouteDelete, avoid assigning a disabled route as the replacement default. Update the fallback logic in cloneAutoRoutesConfig/emitAutoRoutesConfig flow so nextConfig.default_route is chosen from the remaining enabled routes, and only fall back to a non-disabled route key when available. If no enabled routes remain, keep the config in a valid state by selecting an appropriate enabled default candidate before emitting.
837-851: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAdd
aria-labelto the icon-only route Edit/Delete buttons.These two buttons (Pencil / Trash2) render no visible text, unlike the "Set default" button next to them. The route-group order controls further down (up/down/delete) were already given
aria-labels for the same guideline; these route-level controls in the same new card were missed.As per coding guidelines, "必要时添加 ARIA 属性，装饰性图标应使用 aria-hidden="true"".
♿ Proposed fix
<Button variant='outline' size='sm' onClick={() => handleAutoRouteEdit(route)} + aria-label={t('Edit')} > - <Pencil className='h-4 w-4' /> + <Pencil className='h-4 w-4' aria-hidden='true' /> </Button> <Button variant='outline' size='sm' disabled={autoRoutesConfig.routes.length <= 1} onClick={() => handleAutoRouteDelete(route.key)} + aria-label={t('Delete')} > - <Trash2 className='h-4 w-4' /> + <Trash2 className='h-4 w-4' aria-hidden='true' /> </Button>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/default/src/features/system-settings/models/group-ratio-visual-editor.tsx` around lines 837 - 851, The icon-only Edit/Delete controls in the route actions are missing accessible labels. Update the two Button components in group-ratio-visual-editor’s route action area (the ones wrapping Pencil and Trash2 in the same section as handleAutoRouteEdit and handleAutoRouteDelete) to include descriptive aria-labels, and mark the decorative Pencil/Trash2 icons as aria-hidden="true" to match the existing accessibility pattern used for the route-group order controls.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@i18n/locales/zh-TW.yaml`:
- Line 43: The translation for token.group_not_assignable in zh-TW uses
access-language instead of assignment-language, so update the wording to match
the English meaning of “No permission to assign group {{.Group}}”. Adjust the
localized string in the zh-TW locale entry to clearly express that the user
cannot assign the {{.Group}} group, and keep the same key and placeholder name
intact.
In `@setting/ratio_setting/group_ratio.go`:
- Around line 118-138: The generic config update path still bypasses the
reserved-name validation for GroupRatio, so add the same guard used in
CheckGroupRatio to the model/option.go update flow. In the GroupRatio handling
around updateGroupRatioByJSONString, validate the incoming JSON with
CheckGroupRatio before calling ratio_setting.UpdateGroupRatioByJSONString,
ensuring auto and auto:* keys are rejected consistently with
controller/option.go.
---
Outside diff comments:
In `@controller/group.go`:
- Around line 26-61: The insertion logic in GetUserGroups is order-dependent
because the ratio-group loop writes into usableGroups without checking for
existing keys, while addAutoRouteUsableGroup is guarded. Update the
GetUserGroups loop to use the same existence check before assigning, or add an
explicit comment/invariant near GetUserUsableGroups and addAutoRouteUsableGroup
documenting that ratio groups must not overwrite auto-route entries. Ensure the
protection is symmetric so future reordering does not reintroduce the overwrite
bug.
In
`@web/default/src/features/system-settings/models/group-ratio-visual-editor.tsx`:
- Around line 417-427: When deleting the current default in
handleAutoRouteDelete, avoid assigning a disabled route as the replacement
default. Update the fallback logic in cloneAutoRoutesConfig/emitAutoRoutesConfig
flow so nextConfig.default_route is chosen from the remaining enabled routes,
and only fall back to a non-disabled route key when available. If no enabled
routes remain, keep the config in a valid state by selecting an appropriate
enabled default candidate before emitting.
- Around line 837-851: The icon-only Edit/Delete controls in the route actions
are missing accessible labels. Update the two Button components in
group-ratio-visual-editor’s route action area (the ones wrapping Pencil and
Trash2 in the same section as handleAutoRouteEdit and handleAutoRouteDelete) to
include descriptive aria-labels, and mark the decorative Pencil/Trash2 icons as
aria-hidden="true" to match the existing accessibility pattern used for the
route-group order controls.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: b7ee7cd3-40cf-4f76-846b-70a49b5786eb
📒 Files selected for processing (24)
controller/group.gocontroller/group_test.gocontroller/token.gocontroller/token_test.goi18n/keys.goi18n/locales/en.yamli18n/locales/zh-CN.yamli18n/locales/zh-TW.yamlsetting/ratio_setting/group_ratio.gosetting/ratio_setting/group_ratio_test.goweb/default/src/components/model-group-selector.tsxweb/default/src/features/keys/components/api-keys-mutate-drawer.tsxweb/default/src/features/keys/lib/api-key-form.tsweb/default/src/features/pricing/components/model-details.tsxweb/default/src/features/system-settings/models/group-ratio-visual-editor.tsxweb/default/src/features/system-settings/models/ratio-settings-card.tsxweb/default/src/i18n/locales/en.jsonweb/default/src/i18n/locales/fr.jsonweb/default/src/i18n/locales/ja.jsonweb/default/src/i18n/locales/ru.jsonweb/default/src/i18n/locales/vi.jsonweb/default/src/i18n/locales/zh.jsonweb/default/src/lib/auto-routes.test.tsweb/default/src/lib/auto-routes.ts
📜 Review details
⚠️ CI failures not shown inline (2)
GitHub Actions: CI / 1_Backend checks.txt: v1.0.4-preview.2
Conclusion: failure
##[group]Run go run ./tools/jsonwrapcheck
�[36;1mgo run ./tools/jsonwrapcheck�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
stale json wrapper allowlist entries:
setting/ratio_setting/group_ratio.go|CheckGroupRatio|Unmarshal|***REDACTED***
Regenerate the baseline with: go run ./tools/jsonwrapcheck -list > tools/jsonwrapcheck/allowlist.txt
exit status 1
##[error]Process completed with exit code 1.
GitHub Actions: CI / Backend checks: v1.0.4-preview.2
Conclusion: failure
##[group]Run go run ./tools/jsonwrapcheck
�[36;1mgo run ./tools/jsonwrapcheck�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
stale json wrapper allowlist entries:
setting/ratio_setting/group_ratio.go|CheckGroupRatio|Unmarshal|***REDACTED***
Regenerate the baseline with: go run ./tools/jsonwrapcheck -list > tools/jsonwrapcheck/allowlist.txt
exit status 1
##[error]Process completed with exit code 1.
🧰 Additional context used
📓 Path-based instructions (6)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrappers incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly callingencoding/json;encoding/jsontypes likejson.RawMessageandjson.Numbermay still be used as types.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+ simultaneously: prefer GORM methods over raw SQL, avoid directAUTO_INCREMENT/SERIAL, use cross-DB quoting/boolean helpers when raw SQL is unavoidable, avoid DB-specific functions/operators without fallbacks, and ensure migrations work on all three databases.
For request structs parsed from client JSON and later re-marshaled to upstream providers, especially in relay/convert paths, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved and omitted-only fields remainnil.
**/*.go: Usecommon.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson, andcommon.GetJsonTypefromcommon/json.gofor all JSON marshal/unmarshal operations; do not directly callencoding/jsonin business code.
Keep all database code fully compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless cross-DB-safe, and use the provided DB-specific helpers/flags for quoting, booleans, and branching.
For request structs parsed from client JSON and re-marshaled to upstream providers, especially in relay/convert paths, use pointer types withomitemptyfor optional scalar fields so explicit zero/false values are preserved.
Files:
i18n/keys.gosetting/ratio_setting/group_ratio_test.gocontroller/token.gocontroller/group.gosetting/ratio_setting/group_ratio.gocontroller/group_test.gocontroller/token_test.go
web/default/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/default/AGENTS.md)
web/default/src/**/*.{ts,tsx}: 前端页面文本与组件内文案必须支持 i18n：React 组件中应使用useTranslation()取得t，并通过t()渲染用户可见文本；子组件也应自行使用useTranslation()保持独立性。
禁止使用两层及以上嵌套三元表达式；复杂逻辑应改用if-else、提前返回或抽取函数。
控制函数圈复杂度，复杂逻辑应拆成更小的函数；变量与函数命名应有意义并遵循驼峰等常规命名约定。
TypeScript 代码应避免使用any，优先使用具体类型或unknown；参数与返回值应显式标注类型；仅类型用途的导入应使用import type。
修改 TypeScript 或 TSX 代码后必须执行类型检查，并修复所有类型错误，不得遗留。
对象非必要不要解构，尤其是组件 props；优先直接使用props.xxx以保持代码清晰。
组件应使用函数式组件与 Hooks，遵循单一职责；组件 props 必须有明确类型（接口或类型别名）。
单文件超过约 200 行时应考虑拆分子组件或抽取自定义 Hooks；类型定义可与组件同文件或放在同模块的types中。
在 React 中应合理使用useMemo、useCallback、React.memo，避免在渲染路径中创建新对象或数组；必要时进行代码分割与动态import。
React Query 的数据获取应使用useQuery、变更应使用useMutation；每个查询需配置唯一queryKey，并在成功后对相关查询执行invalidateQueries；服务端错误应统一交给handleServerError。
Axios 请求应使用项目统一的api实例；GET 请求默认去重，特殊请求可显式关闭；认证与通用错误应在拦截器中统一处理。
服务端错误应统一使用handleServerError，展示层应使用toast.error等统一方式；文案需走 i18n；路由级错误应由errorComponent承接；表单错误应通过form.setError等方式映射到字段。
样式应以 Tailwind 工具类为主，动态类名使用cn()合并；非动态场景避免内联样式；响应式采用移动优先与 Tailwind 断点，主题与暗色模式通过 CSS 变量与dark:处理。
应使用语义化 HTML、正确关联label与输入、保证键盘可操作与合理焦点顺序；必要时添加 ARIA 属性，装饰性图标应使用aria-hidden="true"。
认证与权限应在路由与接口层校验；前后端都应做数据校验（如 Zod）；敏感信息不得落前端存储；避免使用dangerouslySetInnerHTML；跨域与 Cookie 需配合withCredentials并按后端要求处理 CSRF。
组件测试应使用 React Testing Library，关注交互与行为，避免测试实现细节；关键流程可补充集成与 E2E 测试。
环境变量应通过.env读取，并使用VITE_前缀；代码中不得硬编码密钥。
Files:
web/default/src/lib/auto-routes.test.tsweb/default/src/features/keys/components/api-keys-mutate-drawer.tsxweb/default/src/features/system-settings/models/ratio-settings-card.tsxweb/default/src/features/keys/lib/api-key-form.tsweb/default/src/lib/auto-routes.tsweb/default/src/components/model-group-selector.tsxweb/default/src/features/system-settings/models/group-ratio-visual-editor.tsxweb/default/src/features/pricing/components/model-details.tsx
web/default/src/**/*.test.ts
📄 CodeRabbit inference engine (web/default/AGENTS.md)
工具函数与纯逻辑应优先编写单元测试；测试文件应命名为
*.test.ts。
Files:
web/default/src/lib/auto-routes.test.ts
web/default/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Use
bunas the preferred package manager and script runner for the frontend inweb/default/(bun install,bun run dev,bun run build, andbun run i18n:*).
Files:
web/default/src/lib/auto-routes.test.tsweb/default/src/features/keys/components/api-keys-mutate-drawer.tsxweb/default/src/features/system-settings/models/ratio-settings-card.tsxweb/default/src/i18n/locales/zh.jsonweb/default/src/features/keys/lib/api-key-form.tsweb/default/src/i18n/locales/vi.jsonweb/default/src/lib/auto-routes.tsweb/default/src/components/model-group-selector.tsxweb/default/src/i18n/locales/fr.jsonweb/default/src/features/system-settings/models/group-ratio-visual-editor.tsxweb/default/src/i18n/locales/ru.jsonweb/default/src/i18n/locales/en.jsonweb/default/src/features/pricing/components/model-details.tsxweb/default/src/i18n/locales/ja.json
web/default/src/features/**
📄 CodeRabbit inference engine (web/default/AGENTS.md)
功能模块应放在
src/features/<feature>/，并按需包含components/、lib/、hooks/、api.ts、types.ts、constants.ts等；通用组件应放在src/components/，通用工具与类型应放在src/lib/。
Files:
web/default/src/features/keys/components/api-keys-mutate-drawer.tsxweb/default/src/features/system-settings/models/ratio-settings-card.tsxweb/default/src/features/keys/lib/api-key-form.tsweb/default/src/features/system-settings/models/group-ratio-visual-editor.tsxweb/default/src/features/pricing/components/model-details.tsx
web/default/src/features/**/lib/**/*.ts
📄 CodeRabbit inference engine (web/default/AGENTS.md)
表单应使用 React Hook Form + Zod：在功能模块的
lib/下定义 schema，并用z.infer导出表单类型；useForm应配合@hookform/resolvers/zod进行校验。
Files:
web/default/src/features/keys/lib/api-key-form.ts
🔇 Additional comments (26)
i18n/keys.go (1)
50-50: LGTM!i18n/locales/zh-CN.yaml (1)
43-43: LGTM!setting/ratio_setting/group_ratio_test.go (1)
1-24: LGTM!setting/ratio_setting/group_ratio.go (1)
3-22: LGTM!Also applies to: 136-138
controller/group_test.go (1)
1-29: LGTM!web/default/src/features/keys/components/api-keys-mutate-drawer.tsx (1)
27-27: LGTM!Also applies to: 103-106, 132-142, 155-169, 181-181, 338-338
web/default/src/i18n/locales/ru.json (1)
172-172: LGTM!Also applies to: 236-236, 542-563, 961-961, 1010-1010, 1291-1292, 1532-1532, 1676-1676, 1925-1925, 2264-2264, 2341-2342, 2385-2385, 2771-2776, 3882-3885, 4031-4039, 4136-4136, 4884-4884, 5094-5094
web/default/src/features/system-settings/models/group-ratio-visual-editor.tsx (2)
350-358: 📐 Maintainability & Code QualityHardcoded default suggested key
'auto:fast'can still collide with an existing route.Same issue flagged previously and still present;
handleAutoRouteAddalways pre-fillskey: 'auto:fast'regardless of whether that key is already used, tripping the "Auto route key already exists" error on save without edits.
337-349: LGTM!Also applies to: 360-368, 429-484
web/default/src/lib/auto-routes.test.ts (1)
1-165: LGTM!web/default/src/features/system-settings/models/ratio-settings-card.tsx (1)
456-500: LGTM!web/default/src/components/model-group-selector.tsx (1)
56-64: LGTM!Also applies to: 430-430, 492-492
web/default/src/i18n/locales/fr.json (1)
172-172: LGTM!Also applies to: 236-236, 542-563, 961-961, 1010-1010, 1291-1292, 1532-1532, 1676-1676, 1925-1925, 2264-2264, 2341-2342, 2385-2385, 2771-2776, 3882-3885, 4031-4039, 4136-4136, 4884-4884, 5094-5094
web/default/src/features/pricing/components/model-details.tsx (1)
742-806: LGTM!web/default/src/lib/auto-routes.ts (1)
19-66: 🎯 Functional CorrectnessNo locale sync issue The new auto-route keys are present in
en.json,fr.json,ja.json,ru.json,vi.json, andzh.json.> Likely an incorrect or invalid review comment.controller/token.go (1)
35-49: 🎯 Functional CorrectnessNo change needed The caller’s group is written to both
groupanduser_groupin auth middleware, so this check resolves to the same value in the supported auth paths.web/default/src/features/keys/lib/api-key-form.ts (2)
21-21: Past feedback addressed: usesDEFAULT_AUTO_ROUTE_KEYinstead of literal'auto'.This resolves the earlier review comment requesting a single source of truth for the default auto route key.
Also applies to: 79-88
113-115: LGTM!web/default/src/i18n/locales/zh.json (1)
544-563: LGTM!Also applies to: 2341-2342, 2385-2385, 3882-3885
i18n/locales/en.yaml (1)
42-42: LGTM!web/default/src/i18n/locales/vi.json (1)
544-563: LGTM!Also applies to: 2341-2342, 2385-2385, 2771-2776, 3882-3885, 4031-4039
web/default/src/i18n/locales/en.json (1)
172-172: LGTM!Also applies to: 236-236, 542-563, 961-961, 1010-1010, 1291-1292, 1532-1532, 1676-1676, 1925-1925, 2264-2264, 2341-2342, 2385-2385, 2771-2776, 3882-3885, 4031-4039, 4136-4136, 4884-4884, 5094-5094
web/default/src/i18n/locales/ja.json (1)
172-172: LGTM!Also applies to: 236-236, 542-563, 961-961, 1010-1010, 1291-1292, 1532-1532, 1676-1676, 1925-1925, 2264-2264, 2341-2342, 2385-2385, 2771-2776, 3882-3885, 4031-4039, 4136-4136, 4884-4884, 5094-5094
controller/token_test.go (3)
16-18: LGTM!
119-163: LGTM!Also applies to: 256-260
525-567: Good coverage of hidden auto-route rejection on both create and update paths.Both tests verify failure response, localized error content, and that no state was persisted/mutated on rejection.
Also applies to: 604-648
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
model/option.go (1)
633-671: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPropagate errors from layered config updates
handleConfigUpdatereturns(bool, error), but both failure paths are still dropped:config.UpdateConfigFromMap(...)andtask_billing_setting.ValidateRateCardsJSON(...). As written, handled keys always returnnil, soupdateOptionMapcan’t surface invalid config values.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@model/option.go` around lines 633 - 671, handleConfigUpdate currently swallows errors from layered config updates, so invalid values never reach callers. Update the logic around config.UpdateConfigFromMap and the task_billing_setting.ValidateRateCardsJSON branch to capture and return their errors instead of discarding them. Keep the existing control flow in handleConfigUpdate, but make sure updateOptionMap can receive a non-nil error whenever a handled config key fails validation or update.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@model/option.go`:
- Around line 579-583: The GroupRatio branch in the option update flow is
redundantly validating the same value twice. Since validateOptionUpdate(key,
value) already checks this input before reaching the switch, remove the extra
ratio_setting.CheckGroupRatio(value) call and keep only the update path in the
GroupRatio case of the option handling logic. Use the existing key/value
handling in option.go and the GroupRatio case to simplify the flow without
re-parsing the same JSON.
- Around line 271-278: UpdateOptionBulk is not atomic because
validateOptionUpdate only validates GroupRatio, allowing other keys to fail
later in updateOptionMap after the DB transaction has already committed. Extend
validateOptionUpdate to pre-validate all option keys that can affect runtime
state, especially AutoGroups, AutoGroupRoutes, ModelRatio, and PayMethods, using
the same validation logic as updateOptionMap. Make sure the bulk update rejects
any malformed value before committing so persisted options and in-memory config
stay consistent.
---
Outside diff comments:
In `@model/option.go`:
- Around line 633-671: handleConfigUpdate currently swallows errors from layered
config updates, so invalid values never reach callers. Update the logic around
config.UpdateConfigFromMap and the task_billing_setting.ValidateRateCardsJSON
branch to capture and return their errors instead of discarding them. Keep the
existing control flow in handleConfigUpdate, but make sure updateOptionMap can
receive a non-nil error whenever a handled config key fails validation or
update.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 8ed65e00-d14d-414c-8efd-174ccc528ff3
📒 Files selected for processing (2)
model/option.gomodel/option_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrappers incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly callingencoding/json;encoding/jsontypes likejson.RawMessageandjson.Numbermay still be used as types.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+ simultaneously: prefer GORM methods over raw SQL, avoid directAUTO_INCREMENT/SERIAL, use cross-DB quoting/boolean helpers when raw SQL is unavoidable, avoid DB-specific functions/operators without fallbacks, and ensure migrations work on all three databases.
For request structs parsed from client JSON and later re-marshaled to upstream providers, especially in relay/convert paths, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved and omitted-only fields remainnil.
**/*.go: Usecommon.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson, andcommon.GetJsonTypefromcommon/json.gofor all JSON marshal/unmarshal operations; do not directly callencoding/jsonin business code.
Keep all database code fully compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless cross-DB-safe, and use the provided DB-specific helpers/flags for quoting, booleans, and branching.
For request structs parsed from client JSON and re-marshaled to upstream providers, especially in relay/convert paths, use pointer types withomitemptyfor optional scalar fields so explicit zero/false values are preserved.
Files:
model/option_test.gomodel/option.go
🔇 Additional comments (3)
model/option_test.go (2)
1-66: LGTM!
59-63: 🎯 Functional Correctness | ⚡ Quick winMissing coverage for the "success" path of the dotted key.
The test only verifies the rejection path for
group_ratio_setting.group_ratio. Since valid values for this dotted key are routed throughhandleConfigUpdate/layered config rather than the legacyratio_setting.UpdateGroupRatioByJSONStringcall used for the plainGroupRatiokey (seemodel/option.golines 271-292, 579-583, 633-671), a passing test here wouldn't catch a divergence between the two paths. Consider adding a case asserting that a validgroup_ratio_setting.group_ratioupdate also updatesratio_setting.GetGroupRatioCopy()/GetGroupRatio(...)consistently with the legacy key.model/option.go (1)
213-231: LGTM!Also applies to: 242-246, 280-292, 435-444
1 participant
Important
📝 变更描述 / Description
(简述：做了什么？为什么这样改能生效？请基于你对代码逻辑的理解来写，避免粘贴未经整理的内容)
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)