Merged
Conversation
📝 WalkthroughWalkthrough此PR向提供商编辑流程添加了克隆功能。新增中英文本地化键值以支持克隆UI文本，并在provider-edit-flow组件中实现了完整的克隆工作流，包括状态管理、模型映射复制和成功提示。 Changes
Sequence Diagram(s)sequenceDiagram
participant User as 用户
participant UI as 提供商编辑UI
participant CreateHook as useCreateProvider
participant API as 后端API
participant MappingHook as useCreateModelMapping
User->>UI: 点击Clone按钮
UI->>UI: 设置cloning=true，构建克隆数据
UI->>CreateHook: 调用createProvider（克隆的配置）
CreateHook->>API: 发送POST请求创建新提供商
API-->>CreateHook: 返回新提供商ID
CreateHook-->>UI: 创建成功，返回新提供商
UI->>MappingHook: 循环复制模型映射到新提供商
MappingHook->>API: 为每个映射发送createModelMapping
API-->>MappingHook: 映射创建成功
MappingHook-->>UI: 所有映射复制完成
UI->>UI: 显示克隆成功提示，设置cloning=false
UI-->>User: 展示成功消息
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
web/src/pages/providers/components/antigravity-provider-view.tsx (1)
482-486: 此处存在与前文相同的 Switch 可访问性问题该问题与
custom-config-step.tsx中已指出的问题一致（缺少可访问名称）。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/providers/components/antigravity-provider-view.tsx` around lines 482 - 486, The Switch instance controlling disableErrorCooldown lacks an accessible name; update the Switch (the element using checked={disableErrorCooldown}, onCheckedChange={handleToggleDisableErrorCooldown}, disabled={updateProvider.isPending}) to provide an accessible label—either add an explicit aria-label or aria-labelledby that references a visible label element (or wrap it with a <label> and use id/for), ensuring the label text clearly describes the switch’s purpose for screen readers.web/src/pages/providers/components/codex-provider-view.tsx (1)
589-593: 此处存在与前文相同的 Switch 可访问性问题该问题与
custom-config-step.tsx中已指出的问题一致（缺少可访问名称）。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/providers/components/codex-provider-view.tsx` around lines 589 - 593, The Switch instance (checked={disableErrorCooldown}, onCheckedChange={handleToggleDisableErrorCooldown}, disabled={updateProvider.isPending}) lacks an accessible name; add one by providing an explicit label/association (e.g., render a visible label element tied to the Switch or pass an aria-label/aria-labelledby prop) so screen readers can identify the control; ensure the label text clearly describes the switch purpose and uses the same identifiers as the Switch component to create the association.web/src/pages/providers/components/kiro-provider-view.tsx (1)
402-406: 此处存在与前文相同的 Switch 可访问性问题该问题与
custom-config-step.tsx中已指出的问题一致（缺少可访问名称）。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/providers/components/kiro-provider-view.tsx` around lines 402 - 406, The Switch instance using checked={disableErrorCooldown} and onCheckedChange={handleToggleDisableErrorCooldown} lacks an accessible name; add an explicit accessible label by either rendering a visible Label tied to the Switch (using an id + aria-labelledby) or by supplying a descriptive aria-label/aria-labelledby (e.g., "Disable error cooldown") so screen readers can announce the control; ensure the label text matches other switches' patterns in this file and keep disabled={updateProvider.isPending} unchanged.
🧹 Nitpick comments (4)
web/src/pages/projects/index.tsx (1)
44-60: 可选优化：减少重复日期解析并统一返回数组类型。Line [49]-[54] 对同一值重复构造
Date；另外 Line [45]-[47] 返回undefined会让渲染分支多一层判空。可统一为数组返回，逻辑更直观。♻️ 建议改动
const sortedProjects = useMemo(() => { - if (!projects) { - return undefined; - } + if (!projects) return []; + const toTime = (value: string) => { + const t = Date.parse(value); + return Number.isFinite(t) ? t : 0; + }; return projects.slice().sort((a, b) => { - const timeA = Number.isFinite(new Date(a.createdAt).getTime()) - ? new Date(a.createdAt).getTime() - : 0; - const timeB = Number.isFinite(new Date(b.createdAt).getTime()) - ? new Date(b.createdAt).getTime() - : 0; + const timeA = toTime(a.createdAt); + const timeB = toTime(b.createdAt); if (timeA !== timeB) { return timeA - timeB; } return a.id - b.id; }); }, [projects]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/projects/index.tsx` around lines 44 - 60, sortedProjects currently returns undefined when projects is falsy and reconstructs Date twice per comparison; change it to always return an array and parse each createdAt only once in the comparator. Update the useMemo to return [] when !projects, and inside the sort use a single parse per operand (e.g. const tA = Date.parse(a.createdAt) || 0; const tB = Date.parse(b.createdAt) || 0;) then compare tA and tB and fall back to a.id - b.id; keep the symbol names sortedProjects, useMemo, projects, createdAt and id so the change is easy to locate.web/src/locales/en.json (1)
934-934: 建议去掉文案里的硬编码步骤编号
"3. Error Cooldown"这类编号建议交给 UI 层渲染，文案仅保留标题本身，避免后续流程调整时出现编号错位。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/locales/en.json` at line 934, Update the locale entry for "errorCooldownTitle" to remove the hardcoded step number (change the string from "3. Error Cooldown" to "Error Cooldown") and ensure any numbering is produced by the UI layer instead of the translation string; modify the JSON value for the "errorCooldownTitle" key and adjust the UI where it's rendered to prepend step numbers if needed.web/src/pages/providers/components/codex-provider-view.tsx (1)
357-413: 错误冷却切换逻辑建议抽成复用 HookLine 357-413 的状态同步与提交逻辑在
codex/kiro/antigravity三个视图里基本重复。建议抽成通用 hook（例如useProviderErrorCooldownToggle），减少分叉与后续修复遗漏风险。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/providers/components/codex-provider-view.tsx` around lines 357 - 413, The repeated state-sync and submit logic for toggling error cooldown and CLI proxy API (seen in disableErrorCooldown/useCLIProxyAPI state, the two useEffect blocks, handleToggleCLIProxyAPI and handleToggleDisableErrorCooldown, and updateProvider.mutateAsync calls) should be extracted into a reusable hook (e.g., useProviderErrorCooldownToggle) that encapsulates initialization from provider.config, useEffect syncing, the optimistic update + rollback pattern, and the shape of the payload ({ id: provider.id, data: { ...provider, config: { ...provider.config, disableErrorCooldown, codex: { ...config, useCLIProxyAPI } } } }). Replace the inline state, effects, and both handler functions with calls to that hook in codex-provider-view (and reuse in kiro/antigravity), ensuring the hook accepts provider, config, updateProvider and returns current values and toggle functions.web/src/pages/providers/components/provider-edit-flow.tsx (1)
347-384: 保存与克隆的 payload 组装逻辑重复，建议抽成共享构建函数。两条路径都在手动拼装
supportedClientTypes/clientBaseURL/clientMultiplier/cloak/disableErrorCooldown，后续字段变更很容易只改一处。♻️ 重构方向
+ const buildProviderPayload = (name: string): CreateProviderData => { + // 统一组装 supportedClientTypes/clientBaseURL/clientMultiplier/cloak/config + }; - const data: Partial<CreateProviderData> = { ... }; + const data: Partial<CreateProviderData> = buildProviderPayload(formData.name); - const data: CreateProviderData = { ... }; + const data: CreateProviderData = buildProviderPayload(cloneName);Also applies to: 403-445
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/providers/components/provider-edit-flow.tsx` around lines 347 - 384, Duplicate payload-building logic for provider save/clone should be extracted into a shared builder function; create a helper (e.g., buildProviderPayload or buildCustomConfig) that accepts formData and existing provider (if needed) and returns the Partial<CreateProviderData> or at least the config.custom object. Move the computations for supportedClientTypes, clientBaseURL, clientMultiplier, cloak (using parseSensitiveWords), and disableErrorCooldown into that helper, preserving the current fallback for apiKey (provider.config?.custom?.apiKey) and optional fields logic (only include clientBaseURL/clientMultiplier/supportModels when non-empty). Replace both locations (this block and the other occurrence around the 403-445 region) to call the new helper to avoid duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/locales/zh.json`:
- Around line 934-936: The three i18n keys errorCooldownTitle,
disableErrorCooldown, and disableErrorCooldownDesc use the term “错误冷冻”/“冷冻”
which is inconsistent with other cooldown copy that uses “冷却”; update their
Chinese values to replace “冷冻/冷冻” with “冷却/冷却中” (e.g., "错误冷却", "禁用错误冷却", and
"开启后，错误将不再触发自动冷却；手动冷却与上游明确冷却时间仍会生效。") so terminology is consistent across the
locale.
In `@web/src/pages/providers/components/custom-config-step.tsx`:
- Around line 224-227: The Switch control rendering in custom-config-step.tsx
(the Switch with checked={!!formData.disableErrorCooldown} and onCheckedChange
calling updateFormData) lacks an accessible name; add an aria-label (or
associate it with a visible <label> via htmlFor and id) that clearly describes
its purpose (e.g., "Disable error cooldown") so screen readers can announce it,
ensuring the Switch component receives the aria-label prop or the input id
matches a label.
In `@web/src/pages/providers/components/provider-edit-flow.tsx`:
- Around line 453-477: When cloning provider mappings inside the try block
(looping over providerMappings and calling createModelMapping.mutateAsync),
capture per-mapping success/failure counts and errors instead of only
console.error in the catch; update the logic around
createModelMapping.mutateAsync, the catch block, setCloneToastMessage, and
onClose/newProvider handling to show three outcomes: all succeeded (current
success toast), partial success (toast indicating X succeeded / Y failed with
brief error summary), or total failure (error toast), and ensure
setCloning(false) remains in finally; include references to providerMappings,
createModelMapping.mutateAsync, newProvider.id, cloneName, setCloneToastMessage,
onClose, and setCloning when implementing the counting and user-facing messages.
- Around line 294-295: The clone logic uses useModelMappings() (allMappings) but
proceeds even when mappings are still loading, so update the clone flow (the
handler that performs the provider clone — e.g., the clone button handler like
onClone / handleCloneProvider) to wait for mappings to finish: either disable
the clone action while useModelMappings reports loading, or call the mappings
refetch/fetch method and await completion before performing the clone and
showing success; ensure you reference and use the allMappings/loading/refetch
values returned by useModelMappings and apply the same guard in the other clone
sites noted (around the blocks corresponding to lines ~449-451 and ~565-568).
---
Duplicate comments:
In `@web/src/pages/providers/components/antigravity-provider-view.tsx`:
- Around line 482-486: The Switch instance controlling disableErrorCooldown
lacks an accessible name; update the Switch (the element using
checked={disableErrorCooldown},
onCheckedChange={handleToggleDisableErrorCooldown},
disabled={updateProvider.isPending}) to provide an accessible label—either add
an explicit aria-label or aria-labelledby that references a visible label
element (or wrap it with a <label> and use id/for), ensuring the label text
clearly describes the switch’s purpose for screen readers.
In `@web/src/pages/providers/components/codex-provider-view.tsx`:
- Around line 589-593: The Switch instance (checked={disableErrorCooldown},
onCheckedChange={handleToggleDisableErrorCooldown},
disabled={updateProvider.isPending}) lacks an accessible name; add one by
providing an explicit label/association (e.g., render a visible label element
tied to the Switch or pass an aria-label/aria-labelledby prop) so screen readers
can identify the control; ensure the label text clearly describes the switch
purpose and uses the same identifiers as the Switch component to create the
association.
In `@web/src/pages/providers/components/kiro-provider-view.tsx`:
- Around line 402-406: The Switch instance using checked={disableErrorCooldown}
and onCheckedChange={handleToggleDisableErrorCooldown} lacks an accessible name;
add an explicit accessible label by either rendering a visible Label tied to the
Switch (using an id + aria-labelledby) or by supplying a descriptive
aria-label/aria-labelledby (e.g., "Disable error cooldown") so screen readers
can announce the control; ensure the label text matches other switches' patterns
in this file and keep disabled={updateProvider.isPending} unchanged.
---
Nitpick comments:
In `@web/src/locales/en.json`:
- Line 934: Update the locale entry for "errorCooldownTitle" to remove the
hardcoded step number (change the string from "3. Error Cooldown" to "Error
Cooldown") and ensure any numbering is produced by the UI layer instead of the
translation string; modify the JSON value for the "errorCooldownTitle" key and
adjust the UI where it's rendered to prepend step numbers if needed.
In `@web/src/pages/projects/index.tsx`:
- Around line 44-60: sortedProjects currently returns undefined when projects is
falsy and reconstructs Date twice per comparison; change it to always return an
array and parse each createdAt only once in the comparator. Update the useMemo
to return [] when !projects, and inside the sort use a single parse per operand
(e.g. const tA = Date.parse(a.createdAt) || 0; const tB =
Date.parse(b.createdAt) || 0;) then compare tA and tB and fall back to a.id -
b.id; keep the symbol names sortedProjects, useMemo, projects, createdAt and id
so the change is easy to locate.
In `@web/src/pages/providers/components/codex-provider-view.tsx`:
- Around line 357-413: The repeated state-sync and submit logic for toggling
error cooldown and CLI proxy API (seen in disableErrorCooldown/useCLIProxyAPI
state, the two useEffect blocks, handleToggleCLIProxyAPI and
handleToggleDisableErrorCooldown, and updateProvider.mutateAsync calls) should
be extracted into a reusable hook (e.g., useProviderErrorCooldownToggle) that
encapsulates initialization from provider.config, useEffect syncing, the
optimistic update + rollback pattern, and the shape of the payload ({ id:
provider.id, data: { ...provider, config: { ...provider.config,
disableErrorCooldown, codex: { ...config, useCLIProxyAPI } } } }). Replace the
inline state, effects, and both handler functions with calls to that hook in
codex-provider-view (and reuse in kiro/antigravity), ensuring the hook accepts
provider, config, updateProvider and returns current values and toggle
functions.
In `@web/src/pages/providers/components/provider-edit-flow.tsx`:
- Around line 347-384: Duplicate payload-building logic for provider save/clone
should be extracted into a shared builder function; create a helper (e.g.,
buildProviderPayload or buildCustomConfig) that accepts formData and existing
provider (if needed) and returns the Partial<CreateProviderData> or at least the
config.custom object. Move the computations for supportedClientTypes,
clientBaseURL, clientMultiplier, cloak (using parseSensitiveWords), and
disableErrorCooldown into that helper, preserving the current fallback for
apiKey (provider.config?.custom?.apiKey) and optional fields logic (only include
clientBaseURL/clientMultiplier/supportModels when non-empty). Replace both
locations (this block and the other occurrence around the 403-445 region) to
call the new helper to avoid duplication.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
internal/domain/model.gointernal/executor/executor.gointernal/executor/middleware_dispatch.goweb/src/components/layout/app-sidebar/nav-user.tsxweb/src/components/ui/chart.tsxweb/src/hooks/queries/use-usage-stats.tsweb/src/lib/transport/types.tsweb/src/locales/en.jsonweb/src/locales/zh.jsonweb/src/pages/projects/index.tsxweb/src/pages/providers/components/antigravity-provider-view.tsxweb/src/pages/providers/components/codex-provider-view.tsxweb/src/pages/providers/components/custom-config-step.tsxweb/src/pages/providers/components/kiro-provider-view.tsxweb/src/pages/providers/components/provider-edit-flow.tsxweb/src/pages/providers/context/provider-form-context.tsxweb/src/pages/providers/types.tsweb/src/pages/stats/index.tsx
📜 Review details
🧰 Additional context used
🧬 Code graph analysis (5)
internal/executor/executor.go (4)
internal/domain/model.go (1)
Provider(178-206)web/src/lib/transport/types.ts (1)
Provider(68-78)internal/repository/sqlite/models.go (2)
Provider(63-71)Provider(73-73)web/src/lib/transport/index.ts (1)
Provider(9-9)
internal/domain/model.go (2)
web/src/lib/transport/types.ts (4)
ProviderConfigCustom(12-23)ProviderConfigAntigravity(25-32)ProviderConfigKiro(34-42)ProviderConfigCodex(44-58)web/src/lib/transport/index.ts (3)
ProviderConfigCustom(11-11)ProviderConfigAntigravity(12-12)ProviderConfigCodex(58-58)
web/src/pages/providers/components/codex-provider-view.tsx (2)
launcher/script.js (1)
config(276-276)web/src/lib/transport/http-transport.ts (1)
updateProvider(115-118)
web/src/pages/providers/components/antigravity-provider-view.tsx (3)
web/src/lib/transport/http-transport.ts (1)
updateProvider(115-118)web/src/components/ui/index.ts (1)
Switch(51-51)web/src/components/ui/switch.tsx (1)
Switch(30-30)
web/src/pages/providers/components/kiro-provider-view.tsx (4)
web/src/lib/transport/http-transport.ts (1)
updateProvider(115-118)web/src/hooks/queries/index.ts (1)
useUpdateProvider(11-11)web/src/hooks/queries/use-providers.ts (1)
useUpdateProvider(50-70)web/src/components/ui/index.ts (1)
Switch(51-51)
🔇 Additional comments (17)
web/src/pages/projects/index.tsx (2)
1-1:useMemo引入与变更目标一致，OK。Line [1] 的导入与后续排序 memo 化实现匹配，改动清晰且必要。
110-112: 渲染切换到sortedProjects合理，顺序一致性提升。Line [110]-[112] 使用排序后的数据作为判定与渲染来源，避免了“判定与展示数据源不一致”的风险。
web/src/hooks/queries/use-usage-stats.ts (1)
6-6: 该查询占位策略实现合理。Line 120 使用
placeholderData: keepPreviousData可在筛选条件变化时减少图表闪烁，和当前查询逻辑匹配良好。Also applies to: 120-120
web/src/components/layout/app-sidebar/nav-user.tsx (1)
204-204: 该变更仅为空白行，无需评审动作。web/src/components/ui/chart.tsx (1)
66-68: 图表容器包装方式可行。将子图表放入
ResponsiveContainer并提供最小高度保护，能提升尺寸计算稳定性。web/src/lib/transport/types.ts (1)
61-61: 类型扩展设计兼容性良好。
disableErrorCooldown作为可选字段加入ProviderConfig，不会破坏现有调用方。internal/domain/model.go (1)
166-171: 领域模型字段新增位置与语义清晰。
DisableErrorCooldown与其余 provider 配置字段并列定义，便于统一配置管理。internal/executor/executor.go (1)
233-235: 冷冻跳过判定函数实现稳妥。该函数的空值判断完整，逻辑直观，适合作为统一门禁方法复用。
web/src/pages/stats/index.tsx (1)
901-903: 图表高度策略调整有效。通过固定容器高度可提升渲染稳定性，符合该页面的固定展示区域预期。
internal/executor/middleware_dispatch.go (1)
333-340: 错误冷冻开关接入点正确。Line 333 起在触发冷冻与冷冻事件广播前统一加门禁，能够保证“禁用错误冷冻”语义一致落地。
web/src/pages/providers/context/provider-form-context.tsx (1)
43-43: 默认值设置合理Line 43 将
disableErrorCooldown默认初始化为false，与创建流程中的开关语义一致。web/src/pages/providers/types.ts (1)
227-227: 类型扩展与表单状态对齐Line 227 新增
disableErrorCooldown?: boolean，与本 PR 的创建/编辑链路一致。web/src/pages/providers/components/custom-config-step.tsx (1)
61-61: 配置透传正确Line 61 将
disableErrorCooldown写入创建 payload，字段落点正确。web/src/pages/providers/components/kiro-provider-view.tsx (1)
280-302: 切换后回滚逻辑完整Line 280-302 在更新失败时回滚本地状态，避免了错误 UI 悬挂，处理方式合理。
web/src/pages/providers/components/antigravity-provider-view.tsx (1)
319-342: 状态更新与失败回滚处理到位Line 319-342 的 toggle 持久化逻辑与失败回滚行为完整，能保证 UI 与后端状态收敛。
web/src/pages/providers/components/provider-edit-flow.tsx (2)
333-338: 敏感词解析抽取做得很好。
parseSensitiveWords被统一复用于保存与克隆路径，减少了后续字段行为漂移风险。
315-315:disableErrorCooldown链路接入完整。初始值、保存、克隆和 UI 开关都已打通，字段不会在流程中丢失。
Also applies to: 363-363, 424-424, 677-698
Comment on lines
934
to
936
| "errorCooldownTitle": "3. 错误冷冻", | ||
| "disableErrorCooldown": "禁用错误冷冻", | ||
| "disableErrorCooldownDesc": "开启后，错误将不再触发自动冷冻；手动冷冻与上游明确冷冻时间仍会生效。" |
There was a problem hiding this comment.
“冷冻/冷却”术语不一致，建议统一
Line 934-936 使用了“错误冷冻”，但同页其他 cooldown 文案主要是“冷却”。建议统一术语，减少理解成本。
✏️ 建议修改
- "errorCooldownTitle": "3. 错误冷冻",
- "disableErrorCooldown": "禁用错误冷冻",
- "disableErrorCooldownDesc": "开启后，错误将不再触发自动冷冻；手动冷冻与上游明确冷冻时间仍会生效。"
+ "errorCooldownTitle": "3. 错误冷却",
+ "disableErrorCooldown": "禁用错误冷却",
+ "disableErrorCooldownDesc": "开启后，错误将不再触发自动冷却；手动冷却与上游明确冷却时间仍会生效。"📝 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
| "errorCooldownTitle": "3. 错误冷冻", | |
| "disableErrorCooldown": "禁用错误冷冻", | |
| "disableErrorCooldownDesc": "开启后，错误将不再触发自动冷冻；手动冷冻与上游明确冷冻时间仍会生效。" | |
| "errorCooldownTitle": "3. 错误冷却", | |
| "disableErrorCooldown": "禁用错误冷却", | |
| "disableErrorCooldownDesc": "开启后，错误将不再触发自动冷却；手动冷却与上游明确冷却时间仍会生效。" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/locales/zh.json` around lines 934 - 936, The three i18n keys
errorCooldownTitle, disableErrorCooldown, and disableErrorCooldownDesc use the
term “错误冷冻”/“冷冻” which is inconsistent with other cooldown copy that uses “冷却”;
update their Chinese values to replace “冷冻/冷冻” with “冷却/冷却中” (e.g., "错误冷却",
"禁用错误冷却", and "开启后，错误将不再触发自动冷却；手动冷却与上游明确冷却时间仍会生效。") so terminology is consistent
across the locale.
Comment on lines
224
to
227
| <Switch | ||
| checked={!!formData.disableErrorCooldown} | ||
| onCheckedChange={(checked) => updateFormData({ disableErrorCooldown: checked })} | ||
| /> |
There was a problem hiding this comment.
为开关补充可访问名称
Line 224-227 的 Switch 没有可访问名称，屏幕阅读器无法准确读出该控件含义。建议至少补 aria-label（或使用 label + htmlFor 关联）。
♿ 建议修复
<Switch
+ aria-label={t('provider.disableErrorCooldown')}
checked={!!formData.disableErrorCooldown}
onCheckedChange={(checked) => updateFormData({ disableErrorCooldown: checked })}
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <Switch | |
| checked={!!formData.disableErrorCooldown} | |
| onCheckedChange={(checked) => updateFormData({ disableErrorCooldown: checked })} | |
| /> | |
| <Switch | |
| aria-label={t('provider.disableErrorCooldown')} | |
| checked={!!formData.disableErrorCooldown} | |
| onCheckedChange={(checked) => updateFormData({ disableErrorCooldown: checked })} | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/pages/providers/components/custom-config-step.tsx` around lines 224 -
227, The Switch control rendering in custom-config-step.tsx (the Switch with
checked={!!formData.disableErrorCooldown} and onCheckedChange calling
updateFormData) lacks an accessible name; add an aria-label (or associate it
with a visible <label> via htmlFor and id) that clearly describes its purpose
(e.g., "Disable error cooldown") so screen readers can announce it, ensuring the
Switch component receives the aria-label prop or the input id matches a label.
Comment on lines
+294
to
295
| const { data: allMappings } = useModelMappings(); | ||
|
|
There was a problem hiding this comment.
克隆时存在“映射未加载即被当成空集合”的正确性风险。
当 allMappings 还在加载时，当前实现会直接继续克隆并提示成功，导致新 provider 可能没有复制到任何映射。建议在映射加载完成前禁用克隆，或在克隆前显式拉取一次映射。
🔧 建议修复
- const { data: allMappings } = useModelMappings();
+ const { data: allMappings, isLoading: mappingsLoading } = useModelMappings();
- if (!isValid() || cloning || cloneToastMessage) return;
+ if (!isValid() || cloning || cloneToastMessage || mappingsLoading) return;
<Button
onClick={handleClone}
- disabled={cloning || saving || !isValid() || !!cloneToastMessage}
+ disabled={cloning || saving || mappingsLoading || !isValid() || !!cloneToastMessage}
variant={'outline'}
>Also applies to: 449-451, 565-568
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/pages/providers/components/provider-edit-flow.tsx` around lines 294 -
295, The clone logic uses useModelMappings() (allMappings) but proceeds even
when mappings are still loading, so update the clone flow (the handler that
performs the provider clone — e.g., the clone button handler like onClone /
handleCloneProvider) to wait for mappings to finish: either disable the clone
action while useModelMappings reports loading, or call the mappings
refetch/fetch method and await completion before performing the clone and
showing success; ensure you reference and use the allMappings/loading/refetch
values returned by useModelMappings and apply the same guard in the other clone
sites noted (around the blocks corresponding to lines ~449-451 and ~565-568).
Comment on lines
+453
to
+477
| if (providerMappings.length > 0) { | ||
| for (const mapping of providerMappings) { | ||
| await createModelMapping.mutateAsync({ | ||
| scope: mapping.scope, | ||
| clientType: mapping.clientType, | ||
| providerType: mapping.providerType, | ||
| providerID: newProvider.id, | ||
| projectID: mapping.projectID, | ||
| routeID: mapping.routeID, | ||
| apiTokenID: mapping.apiTokenID, | ||
| pattern: mapping.pattern, | ||
| target: mapping.target, | ||
| priority: mapping.priority, | ||
| isEnabled: mapping.isEnabled, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| setCloneToastMessage(t('provider.cloneSuccess', { name: cloneName })); | ||
| setTimeout(() => onClose(), 800); | ||
| } catch (error) { | ||
| console.error('Failed to clone provider:', error); | ||
| } finally { | ||
| setCloning(false); | ||
| } |
There was a problem hiding this comment.
映射复制失败会产生“部分克隆”且缺少明确失败反馈。
当前实现中，provider 创建成功后若某条映射复制失败，会进入 catch 仅打印日志；用户侧没有失败/部分成功提示，数据容易处于不一致状态。建议统计失败数并区分“成功 / 部分成功 / 失败”提示。
🔧 建议修复
- if (providerMappings.length > 0) {
- for (const mapping of providerMappings) {
- await createModelMapping.mutateAsync({
- ...
- });
- }
- }
-
- setCloneToastMessage(t('provider.cloneSuccess', { name: cloneName }));
+ let failedCount = 0;
+ for (const mapping of providerMappings) {
+ try {
+ await createModelMapping.mutateAsync({
+ ...
+ });
+ } catch (err) {
+ failedCount += 1;
+ }
+ }
+
+ setCloneToastMessage(
+ failedCount === 0
+ ? t('provider.cloneSuccess', { name: cloneName })
+ : t('provider.clonePartialSuccess', { name: cloneName, failed: failedCount }),
+ );
...
} catch (error) {
console.error('Failed to clone provider:', error);
+ setCloneToastMessage(t('provider.cloneError'));
}Also applies to: 717-721
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/pages/providers/components/provider-edit-flow.tsx` around lines 453 -
477, When cloning provider mappings inside the try block (looping over
providerMappings and calling createModelMapping.mutateAsync), capture
per-mapping success/failure counts and errors instead of only console.error in
the catch; update the logic around createModelMapping.mutateAsync, the catch
block, setCloneToastMessage, and onClose/newProvider handling to show three
outcomes: all succeeded (current success toast), partial success (toast
indicating X succeeded / Y failed with brief error summary), or total failure
(error toast), and ensure setCloning(false) remains in finally; include
references to providerMappings, createModelMapping.mutateAsync, newProvider.id,
cloneName, setCloneToastMessage, onClose, and setCloning when implementing the
counting and user-facing messages.
ymkiux
force-pushed
the
feat/disable-error-cooldown
branch
from
55bcfb2 to
b4d8e12
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
web/src/pages/providers/components/provider-edit-flow.tsx (2)
398-398: （已有评论）allMappings未加载完成时克隆会导致映射丢失。此问题在之前的评审中已标记，当前代码未修复：
allMappings仍可能为undefined（加载中），克隆流程不加判断直接继续，导致providerMappings为空数组，新 provider 不会复制任何映射。Also applies to: 449-451
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/providers/components/provider-edit-flow.tsx` at line 398, The clone path proceeds even when allMappings is still loading, causing providerMappings to be empty; update the guard around the clone trigger (the check that currently uses isValid(), cloning, and cloneToastMessage) to also verify allMappings is loaded (e.g., allMappings !== undefined and/or allMappings.length >= 0) before allowing cloning, and apply the same guard where clone logic repeats (referenced around lines using isValid(), cloning, cloneToastMessage and the code that populates providerMappings) so the cloneProvider flow only runs after allMappings is available.
453-477: （已有评论）映射复制失败无用户侧反馈，且catch块未设置错误提示。此问题在之前的评审中已标记，当前代码未修复：provider 创建成功后若某条映射写入失败，只有
console.error，用户无法感知部分失败；整体克隆失败时catch同样没有设置cloneToastMessage，用户侧无任何错误提示。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/providers/components/provider-edit-flow.tsx` around lines 453 - 477, When cloning provider mappings (loop using providerMappings and createModelMapping.mutateAsync) ensure mapping write failures are surfaced to the user: wrap the per-mapping await in a try/catch, collect any failed mapping infos or the thrown error, and after the loop call setCloneToastMessage with a localized failure message (instead of only console.error) indicating partial or full failure; also update the outer catch block to call setCloneToastMessage with an error message before setCloning(false) and keep onClose() behavior unchanged for success paths. Use the existing symbols createModelMapping.mutateAsync, setCloneToastMessage, providerMappings, onClose, and setCloning to implement these changes.
🧹 Nitpick comments (3)
web/src/pages/providers/components/provider-edit-flow.tsx (3)
403-445:handleClone与handleSave存在大量重复的数据构建逻辑，建议提取公共函数。
handleClone（行 403–445）与handleSave（行 347–384）在构建clientBaseURL、clientMultiplier以及完整CreateProviderData对象时逻辑几乎完全相同，维护成本高，改动一处须同步两处。建议提取一个buildProviderPayload工具函数。♻️ 建议重构
+ const buildProviderPayload = (overrides?: { name?: string }): CreateProviderData => { + const supportedClientTypes = formData.clients.filter((c) => c.enabled).map((c) => c.id); + const clientBaseURL: Partial<Record<ClientType, string>> = {}; + const clientMultiplier: Partial<Record<ClientType, number>> = {}; + formData.clients.forEach((c) => { + if (c.enabled && c.urlOverride) clientBaseURL[c.id] = c.urlOverride; + if (c.enabled && c.multiplier !== 10000) clientMultiplier[c.id] = c.multiplier; + }); + return { + type: provider.type || 'custom', + name: overrides?.name ?? formData.name, + logo: provider.logo, + config: { + disableErrorCooldown: !!formData.disableErrorCooldown, + custom: { + baseURL: formData.baseURL, + apiKey: formData.apiKey || provider.config?.custom?.apiKey || '', + clientBaseURL: Object.keys(clientBaseURL).length > 0 ? clientBaseURL : undefined, + clientMultiplier: Object.keys(clientMultiplier).length > 0 ? clientMultiplier : undefined, + cloak: + formData.cloakMode !== 'auto' || + formData.cloakStrictMode || + parseSensitiveWords(formData.cloakSensitiveWords || '').length > 0 + ? { + mode: formData.cloakMode, + strictMode: formData.cloakStrictMode, + sensitiveWords: parseSensitiveWords(formData.cloakSensitiveWords || ''), + } + : undefined, + }, + }, + supportedClientTypes, + supportModels: formData.supportModels.length > 0 ? formData.supportModels : undefined, + }; + }; const handleSave = async () => { // ... - const supportedClientTypes = formData.clients.filter(...); - // ...construct data manually... + const data = buildProviderPayload(); await updateProvider.mutateAsync({ id: Number(provider.id), data }); // ... }; const handleClone = async () => { // ... - const supportedClientTypes = formData.clients.filter(...); - // ...construct data manually... + const data = buildProviderPayload({ name: cloneName }); const newProvider = await createProvider.mutateAsync(data); // ... };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/providers/components/provider-edit-flow.tsx` around lines 403 - 445, handleClone duplicates the payload construction logic from handleSave (building supportedClientTypes, clientBaseURL, clientMultiplier and the CreateProviderData object including config.custom.cloak logic); extract that shared logic into a new helper (e.g. buildProviderPayload(formData, provider): CreateProviderData) and use it from both handleClone and handleSave, moving uses of parseSensitiveWords, formData.clients iteration, baseName/cloneName logic, and provider fallback resolution into the helper so the two handlers just call buildProviderPayload and then perform their specific actions.
717-721: 成功 Toast 缺少手动关闭按钮，且依赖onClose卸载组件才消失。若
onClose因某些原因未能卸载该组件，cloneToastMessage不会被清除，Toast 将永久显示，且克隆按钮将持续处于禁用状态（因!!cloneToastMessage为true）。建议在setTimeout回调中同时调用setCloneToastMessage(null)，或为 Toast 添加关闭按钮。♻️ 建议修复
- setCloneToastMessage(t('provider.cloneSuccess', { name: cloneName })); - setTimeout(() => onClose(), 800); + setCloneToastMessage(t('provider.cloneSuccess', { name: cloneName })); + setTimeout(() => { + setCloneToastMessage(null); + onClose(); + }, 800);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/providers/components/provider-edit-flow.tsx` around lines 717 - 721, The success Toast rendered when cloneToastMessage is set lacks a manual close and relies solely on onClose to unmount, so the message can persist and keep the clone button disabled; update the Toast rendering (using cloneToastMessage) to provide a close control that calls setCloneToastMessage(null) (and also call setCloneToastMessage(null) inside any existing setTimeout cleanup where the toast is auto-hidden), ensuring the clone button disable condition (!!cloneToastMessage) is cleared; reference the cloneToastMessage state and the setter setCloneToastMessage (and the onClose handler) when adding the close button and when updating the timeout cleanup.
453-469: 映射逐条串行写入，建议并行化以提升性能。当前实现对每条映射顺序
await，若存在较多映射，总耗时为 N × 接口延迟。改用Promise.all可显著缩短克隆耗时。♻️ 建议重构
- if (providerMappings.length > 0) { - for (const mapping of providerMappings) { - await createModelMapping.mutateAsync({ - scope: mapping.scope, - clientType: mapping.clientType, - providerType: mapping.providerType, - providerID: newProvider.id, - projectID: mapping.projectID, - routeID: mapping.routeID, - apiTokenID: mapping.apiTokenID, - pattern: mapping.pattern, - target: mapping.target, - priority: mapping.priority, - isEnabled: mapping.isEnabled, - }); - } - } + await Promise.all( + providerMappings.map((mapping) => + createModelMapping.mutateAsync({ + scope: mapping.scope, + clientType: mapping.clientType, + providerType: mapping.providerType, + providerID: newProvider.id, + projectID: mapping.projectID, + routeID: mapping.routeID, + apiTokenID: mapping.apiTokenID, + pattern: mapping.pattern, + target: mapping.target, + priority: mapping.priority, + isEnabled: mapping.isEnabled, + }), + ), + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/providers/components/provider-edit-flow.tsx` around lines 453 - 469, The loop currently awaits each createModelMapping.mutateAsync call serially (providerMappings loop using await), causing O(N) latency; change it to run requests in parallel by mapping providerMappings to an array of createModelMapping.mutateAsync(...) promises (passing mapping.scope, mapping.clientType, mapping.providerType, providerID: newProvider.id, projectID, routeID, apiTokenID, pattern, target, priority, isEnabled) and then await Promise.all(...) (or Promise.allSettled(...) if you want independent failure handling) so all mappings are created concurrently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@web/src/pages/providers/components/provider-edit-flow.tsx`:
- Line 398: The clone path proceeds even when allMappings is still loading,
causing providerMappings to be empty; update the guard around the clone trigger
(the check that currently uses isValid(), cloning, and cloneToastMessage) to
also verify allMappings is loaded (e.g., allMappings !== undefined and/or
allMappings.length >= 0) before allowing cloning, and apply the same guard where
clone logic repeats (referenced around lines using isValid(), cloning,
cloneToastMessage and the code that populates providerMappings) so the
cloneProvider flow only runs after allMappings is available.
- Around line 453-477: When cloning provider mappings (loop using
providerMappings and createModelMapping.mutateAsync) ensure mapping write
failures are surfaced to the user: wrap the per-mapping await in a try/catch,
collect any failed mapping infos or the thrown error, and after the loop call
setCloneToastMessage with a localized failure message (instead of only
console.error) indicating partial or full failure; also update the outer catch
block to call setCloneToastMessage with an error message before
setCloning(false) and keep onClose() behavior unchanged for success paths. Use
the existing symbols createModelMapping.mutateAsync, setCloneToastMessage,
providerMappings, onClose, and setCloning to implement these changes.
---
Nitpick comments:
In `@web/src/pages/providers/components/provider-edit-flow.tsx`:
- Around line 403-445: handleClone duplicates the payload construction logic
from handleSave (building supportedClientTypes, clientBaseURL, clientMultiplier
and the CreateProviderData object including config.custom.cloak logic); extract
that shared logic into a new helper (e.g. buildProviderPayload(formData,
provider): CreateProviderData) and use it from both handleClone and handleSave,
moving uses of parseSensitiveWords, formData.clients iteration,
baseName/cloneName logic, and provider fallback resolution into the helper so
the two handlers just call buildProviderPayload and then perform their specific
actions.
- Around line 717-721: The success Toast rendered when cloneToastMessage is set
lacks a manual close and relies solely on onClose to unmount, so the message can
persist and keep the clone button disabled; update the Toast rendering (using
cloneToastMessage) to provide a close control that calls
setCloneToastMessage(null) (and also call setCloneToastMessage(null) inside any
existing setTimeout cleanup where the toast is auto-hidden), ensuring the clone
button disable condition (!!cloneToastMessage) is cleared; reference the
cloneToastMessage state and the setter setCloneToastMessage (and the onClose
handler) when adding the close button and when updating the timeout cleanup.
- Around line 453-469: The loop currently awaits each
createModelMapping.mutateAsync call serially (providerMappings loop using
await), causing O(N) latency; change it to run requests in parallel by mapping
providerMappings to an array of createModelMapping.mutateAsync(...) promises
(passing mapping.scope, mapping.clientType, mapping.providerType, providerID:
newProvider.id, projectID, routeID, apiTokenID, pattern, target, priority,
isEnabled) and then await Promise.all(...) (or Promise.allSettled(...) if you
want independent failure handling) so all mappings are created concurrently.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
web/src/locales/en.jsonweb/src/locales/zh.jsonweb/src/pages/providers/components/provider-edit-flow.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- web/src/locales/zh.json
- web/src/locales/en.json
📜 Review details
🔇 Additional comments (2)
web/src/pages/providers/components/provider-edit-flow.tsx (2)
333-338: 提取parseSensitiveWords为共享辅助函数，实现良好。此前内联在保存路径中的解析逻辑抽取为组件内帮助函数，
handleSave与handleClone均可复用，避免了重复代码，改动合理。
565-572: 克隆按钮的禁用逻辑与状态标签切换实现正确。
SurviveM
approved these changes
Feb 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
Summary by CodeRabbit
新功能