feat(providers): 按成本倍数自动排序供应商优先级 (#555)#569
Conversation
- 5 个语言的 dashboard.json 添加顶层 actions 翻译键
- 2 个测试文件 import 真实 locale 文件替代硬编码 mock
- 测试断言收敛到具体 tab 容器作用域
- CLAUDE.md 语言列表校正 (ko/de → ru/zh-TW)
fix(i18n): add top-level dashboard.actions keys and improve test i18n strategy
- Add `actions` (copy/download/copied) to dashboard.json for all 5 locales
to fix CodeDisplay component missing translation keys
- Refactor test files to import real locale messages instead of hardcoded
mocks, ensuring tests stay in sync with actual translations
- Scope test assertions to specific tab containers for better precision
- Fix CLAUDE.md language list to match actual locales (ru/zh-TW, not ko/de)
- tsconfig.json 添加 @messages/* 路径别名
- tsconfig.json 移除 src/components/ui/** exclude
- vitest.config.ts 添加 @messages 别名
- 测试文件使用新别名,修复类型断言
chore(config): add @messages path alias and fix TypeScript exclude
- Add `@messages/*` path alias to tsconfig.json and vitest.config.ts
for cleaner locale imports in tests
- Remove `src/components/ui/**` from tsconfig exclude to enable
TypeScript checking and IDE support for UI component tests
- Update test imports to use @messages/en/dashboard.json
- Fix non-null assertion in code-display.test.tsx to resolve TS error
- 为请求复制/下载两个纯图标按钮添加 aria-label,避免依赖 Tooltip 文本,同时提升可访问性 - 单测改为按 aria-label 定位按钮,并补齐 SessionMessagesDetailsTabs 的 mock(提供复制响应按钮)
- 将 Tab+空格混合缩进统一为纯空格,符合项目 Biome 规范 - 测试文件长行拆分以保持代码风格一致
- 移除 ProviderManagerLoader 内部 QueryClientProvider,避免页面存在两个 QueryClient 导致 invalidate 不生效 - 将 staleTime/refetchOnWindowFocus 下沉到各个 useQuery,保持原有缓存/刷新行为一致 - AutoSortPriorityDialog 应用后改为 invalidateQueries(["providers"]),不再使用 window.location.reload()
- vitest.config.ts: 排除 my-usage-readonly.test.ts(需要 DSN)
- vitest.integration.config.ts: 统一管理所有需要 DB 的 API 测试
- users-actions.test.ts
- providers-actions.test.ts
- keys-actions.test.ts
- my-usage-readonly.test.ts
确保 `bun run test` 在无数据库环境下正常通过,
需要 DB 的测试通过 `bun run test:integration` 单独运行。
AlertDialogDescription 默认渲染为 <p> 标签,内部嵌套 <div> 违反 HTML 规范, 导致 React hydration mismatch 警告。使用 asChild 属性让其渲染为 <div> 解决。
📝 WalkthroughWalkthrough新增基于成本倍数的提供商优先级自动排序功能:后端批量更新接口、前端预览与应用对话框、对应翻译条目及多处测试改动,且在若干文件中新增/调整了翻译与构建配置。 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Cache: Disabled due to Reviews > Disable Cache setting 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (5)
🧰 Additional context used📓 Path-based instructions (3)**/*.{js,ts,tsx,jsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx,jsx,js}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{tsx,jsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (3)
Comment |
Summary of ChangesHello @YangQing-Lin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求的核心目标是增强供应商管理功能,通过引入基于成本倍数的自动优先级排序机制,实现更智能的路由策略。除了这一主要功能,PR 还解决了几个关键的客户端和服务器端问题,包括 React Query 缓存一致性、DOM 结构合规性以及用户界面无障碍性。这些改进共同提升了系统的稳定性、用户体验和可维护性。 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/actions/providers.ts:
- Around line 58-77: Duplicate AutoSortResult type exists in providers.ts and
the AutoSortPriorityDialog component; extract a single exported AutoSortResult
into a shared types module (e.g., create and export AutoSortResult from a new
types file) and replace both local definitions with import type { AutoSortResult
} from that module. Update the providers.ts declaration and the
AutoSortPriorityDialog usage to import the shared type and remove the duplicated
inline type definitions so both files reference the same exported
AutoSortResult.
In
@src/app/[locale]/settings/providers/_components/auto-sort-priority-dialog.tsx:
- Around line 29-48: The AutoSortResult type is duplicated; extract
AutoSortResult into a single shared types module (e.g., create and export it
from a new providers/types module), remove the local AutoSortResult definition
in this component, and import the shared AutoSortResult type here and in the
other providers actions module where the duplicate currently exists; ensure the
exported name matches exactly and update any imports/usages to reference the
shared type.
🧹 Nitpick comments (2)
messages/zh-CN/settings.json (1)
133-148: autoSort 文案与功能契合,建议顺手统一“成本倍数/倍率”的用词这一段 autoSort 文案整体准确,清楚表达了“按成本倍率自动分配优先级(低成本=高优先级)”的含义,
{count}占位符用法也合理。同一文件中:
list.costMultiplier使用的是“成本倍数”,而 autoSort 和部分表单配置使用“成本倍率”。从体验上问题不大,但如果有精力可以考虑统一成同一个说法,避免用户在不同位置看到两个近义术语。src/app/[locale]/settings/providers/_components/provider-manager-loader.tsx (1)
47-85: useQuery 配置整体合理,但请确认全局 QueryClientProvider 与注释一致性
- 现在组件直接使用
useQuery,且关闭窗口聚焦自动刷新并统一staleTime: 30_000,有利于避免多处各写一套缓存策略;前提是外层已经在 App 级别包了一层QueryClientProvider,否则这些查询会在运行时报错,请确认上层结构。- 统计查询上方注释仍写着 “longer cache”,但
staleTime已与其他查询相同,如确实只需要更长缓存,可以单独调大该查询的staleTime,否则建议更新注释避免误导后续维护者。
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
📒 Files selected for processing (25)
CLAUDE.mdmessages/en/dashboard.jsonmessages/en/settings.jsonmessages/ja/dashboard.jsonmessages/ja/settings.jsonmessages/ru/dashboard.jsonmessages/ru/settings.jsonmessages/zh-CN/dashboard.jsonmessages/zh-CN/settings.jsonmessages/zh-TW/dashboard.jsonmessages/zh-TW/settings.jsonsrc/actions/providers.tssrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client-actions.test.tsxsrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client.test.tsxsrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client.tsxsrc/app/[locale]/settings/providers/_components/auto-sort-priority-dialog.tsxsrc/app/[locale]/settings/providers/_components/provider-manager-loader.tsxsrc/app/[locale]/settings/providers/page.tsxsrc/components/ui/__tests__/code-display.test.tsxsrc/repository/provider.tstests/unit/actions/providers.test.tstests/unit/repository/provider.test.tstsconfig.jsonvitest.config.tsvitest.integration.config.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
No emoji characters in any code, comments, or string literals
Files:
src/app/[locale]/settings/providers/page.tsxvitest.integration.config.tssrc/app/[locale]/settings/providers/_components/provider-manager-loader.tsxsrc/app/[locale]/settings/providers/_components/auto-sort-priority-dialog.tsxtests/unit/actions/providers.test.tssrc/repository/provider.tssrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client-actions.test.tsxsrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client.tsxsrc/components/ui/__tests__/code-display.test.tsxtests/unit/repository/provider.test.tssrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client.test.tsxvitest.config.tssrc/actions/providers.ts
**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,jsx,js}: All user-facing strings must use i18n (5 languages supported: zh-CN, en, ja, ko, de). Never hardcode display text
Use path alias @/ to map to ./src/
Use Biome for code formatting with configuration: double quotes, trailing commas, 2-space indent, 100 character line width
Prefer named exports over default exports
Use next-intl for internationalization
Use Next.js 16 App Router with Hono for API routes
Files:
src/app/[locale]/settings/providers/page.tsxvitest.integration.config.tssrc/app/[locale]/settings/providers/_components/provider-manager-loader.tsxsrc/app/[locale]/settings/providers/_components/auto-sort-priority-dialog.tsxtests/unit/actions/providers.test.tssrc/repository/provider.tssrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client-actions.test.tsxsrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client.tsxsrc/components/ui/__tests__/code-display.test.tsxtests/unit/repository/provider.test.tssrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client.test.tsxvitest.config.tssrc/actions/providers.ts
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use React 19, shadcn/ui, Tailwind CSS, and Recharts for the UI layer
Files:
src/app/[locale]/settings/providers/page.tsxsrc/app/[locale]/settings/providers/_components/provider-manager-loader.tsxsrc/app/[locale]/settings/providers/_components/auto-sort-priority-dialog.tsxsrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client-actions.test.tsxsrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client.tsxsrc/components/ui/__tests__/code-display.test.tsxsrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client.test.tsx
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place unit tests in tests/unit/, integration tests in tests/integration/, and source-adjacent tests in src/**/*.test.ts
Files:
tests/unit/actions/providers.test.tssrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client-actions.test.tsxsrc/components/ui/__tests__/code-display.test.tsxtests/unit/repository/provider.test.tssrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client.test.tsx
🧠 Learnings (10)
📚 Learning: 2026-01-05T03:02:06.594Z
Learnt from: NieiR
Repo: ding113/claude-code-hub PR: 539
File: src/app/[locale]/dashboard/_components/user/user-key-table-row.tsx:66-66
Timestamp: 2026-01-05T03:02:06.594Z
Learning: In the claude-code-hub project, the translations.actions.addKey field in UserKeyTableRowProps is defined as optional for backward compatibility, but all actual callers in the codebase provide the complete translations object. The field has been added to all 5 locale files (messages/{locale}/dashboard.json).
Applied to files:
messages/zh-CN/dashboard.jsonmessages/zh-TW/dashboard.jsonmessages/ja/dashboard.jsonmessages/ru/dashboard.jsonsrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client-actions.test.tsxsrc/components/ui/__tests__/code-display.test.tsxsrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client.test.tsxmessages/en/settings.json
📚 Learning: 2026-01-07T17:05:36.362Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T17:05:36.362Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Place unit tests in tests/unit/, integration tests in tests/integration/, and source-adjacent tests in src/**/*.test.ts
Applied to files:
vitest.integration.config.tsCLAUDE.mdtsconfig.jsonvitest.config.ts
📚 Learning: 2026-01-07T17:05:36.362Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T17:05:36.362Z
Learning: Applies to **/*.{ts,tsx,jsx,js} : All user-facing strings must use i18n (5 languages supported: zh-CN, en, ja, ko, de). Never hardcode display text
Applied to files:
CLAUDE.mdtsconfig.jsonsrc/components/ui/__tests__/code-display.test.tsxsrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client.test.tsx
📚 Learning: 2026-01-07T17:05:36.362Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T17:05:36.362Z
Learning: Applies to **/*.{ts,tsx,jsx,js} : Use next-intl for internationalization
Applied to files:
CLAUDE.mdtsconfig.jsonsrc/components/ui/__tests__/code-display.test.tsxsrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client.test.tsx
📚 Learning: 2026-01-07T17:05:36.362Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T17:05:36.362Z
Learning: Applies to **/*.{ts,tsx,jsx,js} : Use Biome for code formatting with configuration: double quotes, trailing commas, 2-space indent, 100 character line width
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-07T17:05:36.362Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T17:05:36.362Z
Learning: Applies to **/*.{ts,tsx,jsx,js} : Use path alias @/ to map to ./src/
Applied to files:
CLAUDE.mdtsconfig.jsonvitest.config.ts
📚 Learning: 2026-01-07T17:05:36.362Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T17:05:36.362Z
Learning: Applies to **/*.{tsx,jsx} : Use React 19, shadcn/ui, Tailwind CSS, and Recharts for the UI layer
Applied to files:
CLAUDE.mdsrc/components/ui/__tests__/code-display.test.tsx
📚 Learning: 2026-01-07T17:05:36.362Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T17:05:36.362Z
Learning: Applies to **/*.{ts,tsx,jsx,js} : Prefer named exports over default exports
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-07T17:05:36.362Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T17:05:36.362Z
Learning: Applies to **/*.{ts,tsx,jsx,js} : Use Next.js 16 App Router with Hono for API routes
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-05T03:01:39.354Z
Learnt from: NieiR
Repo: ding113/claude-code-hub PR: 539
File: src/types/user.ts:158-170
Timestamp: 2026-01-05T03:01:39.354Z
Learning: In TypeScript interfaces, explicitly document and enforce distinct meanings for null and undefined. Example: for numeric limits like limitTotalUsd, use 'number | null | undefined' when null signifies explicitly unlimited (e.g., matches DB schema or special UI logic) and undefined signifies 'inherit default'. This pattern should be consistently reflected in type definitions across related fields to preserve semantic clarity between database constraints and UI behavior.
Applied to files:
src/repository/provider.tssrc/actions/providers.ts
🧬 Code graph analysis (8)
src/app/[locale]/settings/providers/page.tsx (1)
src/app/[locale]/settings/providers/_components/auto-sort-priority-dialog.tsx (1)
AutoSortPriorityDialog(50-250)
src/app/[locale]/settings/providers/_components/provider-manager-loader.tsx (2)
src/types/provider.ts (1)
ProviderStatisticsMap(229-229)src/actions/providers.ts (1)
getProviderStatisticsAsync(301-339)
src/app/[locale]/settings/providers/_components/auto-sort-priority-dialog.tsx (1)
src/actions/providers.ts (1)
autoSortProviderPriority(765-889)
tests/unit/actions/providers.test.ts (1)
src/actions/providers.ts (1)
autoSortProviderPriority(765-889)
src/repository/provider.ts (1)
src/drizzle/db.ts (1)
db(37-44)
src/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client.tsx (2)
scripts/sync-settings-keys.js (1)
t(72-72)src/components/ui/alert-dialog.tsx (1)
AlertDialogDescription(131-131)
tests/unit/repository/provider.test.ts (1)
src/repository/provider.ts (1)
updateProviderPrioritiesBatch(498-533)
src/actions/providers.ts (5)
src/actions/types.ts (1)
ActionResult(31-31)src/lib/auth.ts (1)
getSession(116-128)src/drizzle/schema.ts (1)
providers(130-272)src/repository/provider.ts (2)
findAllProvidersFresh(201-265)updateProviderPrioritiesBatch(498-533)src/lib/cache/provider-cache.ts (1)
publishProviderCacheInvalidation(83-87)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Docker Build Test
- GitHub Check: 🌐 API Tests
- GitHub Check: pr-review
- GitHub Check: pr-description
- GitHub Check: check-codex-status
🔇 Additional comments (31)
messages/ru/dashboard.json (1)
2-6: 新增顶层 actions 文案合理且与其他语言版本保持一致
copy/download/copied的俄文翻译准确,也与文件中其它相似用语风格一致,适合作为通用按钮/aria 文案复用。messages/zh-TW/dashboard.json (1)
2-6: 新增頂層 actions 翻譯正確且結構一致
copy/download/copied的繁體翻譯自然,鍵名與其他語言版本保持一致,可安全用作通用動作文案。messages/zh-CN/dashboard.json (1)
2-6: 新增顶层 actions 文案清晰,便于复用
copy/download/copied的中文翻译准确,键名与其他 locale 保持一致,便于在多个组件中复用,不会与现有嵌套logs.actions等产生冲突。messages/ja/dashboard.json (1)
2-6: 新增顶层 actions(日文)翻译自然且与现有用语一致
copy/download/copied的日文翻译符合常用 UI 表达,并与本文件中其他“复制/下载”相关文案风格一致,可安全复用。vitest.config.ts (2)
107-107: LGTM!测试文件排除配置正确。将需要数据库的 API 测试文件从单元测试配置中排除,并移至集成测试配置是正确的做法,符合项目的测试分层策略。
135-135: LGTM!路径别名配置正确。新增的
@messages别名与tsconfig.json中的配置保持一致,支持测试文件和 UI 组件直接导入 messages 目录下的国际化资源。messages/en/dashboard.json (1)
2-6: LGTM!国际化配置符合规范。新增的
actions对象为导出按钮提供了标准化的操作标签,遵循了项目的 i18n 要求。翻译键命名清晰且符合语义。CLAUDE.md (1)
129-129: LGTM!文档更新准确反映了支持的语言列表。语言列表已更新为实际支持的 5 种语言(zh-CN、zh-TW、en、ja、ru),与 PR 中添加的翻译文件保持一致。
基于学习记录,先前支持的语言为(zh-CN, en, ja, ko, de),现已更新为(zh-CN, zh-TW, en, ja, ru)。
tsconfig.json (2)
24-24: LGTM!路径别名配置正确且一致。新增的
@messages/*别名与vitest.config.ts中的配置保持一致,遵循了项目使用路径别名的惯例,便于测试和组件导入国际化资源。
35-35: LGTM!移除 UI 组件的排除配置合理。将
src/components/ui/**从排除列表中移除,使这些组件被纳入 TypeScript 类型检查,有助于提升类型安全性。src/app/[locale]/settings/providers/page.tsx (1)
9-9: LGTM!组件集成干净且符合规范。
AutoSortPriorityDialog的导入和渲染遵循了 React 19 和 Next.js 16 的最佳实践,组件放置在操作区域的位置合理,与现有的SchedulingRulesDialog并列显示,保持了 UI 的一致性。Also applies to: 37-37
vitest.integration.config.ts (1)
19-30: 集成测试选择范围调整合理说明清晰地区分了“需要数据库”的 API 测试,并在 include 中显式列出这些用例,便于 CI 控制整体耗时;当前配置没有明显问题。
messages/zh-TW/settings.json (1)
601-616: autoSort 文案与新功能契合
providers.autoSort的 key 集合和占位符与其他语言版本一致,描述清楚「按成本倍率自动分配優先級」以及成功/失败态,适合直接上线使用。messages/ja/settings.json (1)
601-616: autoSort 日文文案完整且与其他语言版本对齐
providers.autoSort各字段语义清晰(按コスト倍率自动割り当て优先度),占位符{count}使用正确,与其它 locale 的 key 集合保持一致,没有发现问题。messages/ru/settings.json (1)
601-616: autoSort 俄文本地化与功能需求匹配
providers.autoSort的俄文翻译覆盖了按钮、对话框说明、变更统计、表头及成功/错误提示,语义准确(“множитель стоимости”“приоритет”等用词清晰),与其他语言的 key 结构保持一致,可直接使用。messages/en/settings.json (1)
610-625: LGTM!自动排序优先级功能的翻译键结构清晰,命名规范与现有代码保持一致。根据 PR 描述,已为 5 种语言(zh-CN、en、ja、ko、de)添加了相应的翻译。
tests/unit/repository/provider.test.ts (1)
50-126: LGTM!测试覆盖全面,涵盖了三个关键场景:
- 空数组返回 0 且不执行 SQL
- 批量更新生成正确的 CASE 语句
- 重复 ID 去重(后者优先)
sqlToString辅助函数设计合理,能正确处理 drizzle-orm 的 SQL 对象结构。src/components/ui/__tests__/code-display.test.tsx (2)
10-16: 使用真实翻译文件是良好的测试实践。从
@messages/en/dashboard.json导入实际翻译内容,确保测试与 UI 显示文本保持同步。当翻译发生变化时,测试能够自动反映这些变化,减少维护成本。
307-309: LGTM!非空断言(
!)的使用是合理的,因为第 307 行已经断言lastAnchor不为 null。src/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client.tsx (2)
375-376: 无障碍改进。为纯图标按钮添加
aria-label属性,提升了屏幕阅读器的可访问性。使用 i18n 键值确保标签文本也能正确国际化。
563-570: 修复 DOM 嵌套导致的 hydration 错误。使用
asChild属性让AlertDialogDescription直接渲染子元素,避免了默认<p>标签内嵌套<div>导致的无效 HTML 结构(<p>不能包含块级元素)。这是解决 React hydration 错误的正确方式。src/repository/provider.ts (1)
498-533: LGTM! 批量更新实现简洁高效。实现亮点:
- 单条 SQL 语句完成批量更新,性能优于逐条更新
- 使用
Map去重(后者优先)符合常见的 upsert 语义sql.identifier防止 SQL 注入ELSE ${priorityCol} END保护未在更新列表中的记录deleted_at IS NULL过滤确保只更新有效记录
rowCount的any类型断言是必要的,因为 drizzle-orm 的execute返回类型未直接暴露该属性。src/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client.test.tsx (2)
74-131: 测试断言范围优化。将 DOM 查询限定在特定标签页容器内(如
requestBodyTab.querySelector),避免了跨标签页内容干扰导致的误判。这种模式更准确地反映了用户的实际交互流程。
179-204: 空状态测试使用真实翻译。使用
dashboardMessages.sessions.details.storageTip和noData确保测试与实际 UI 文本一致,提高了测试的可维护性。src/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client-actions.test.tsx (2)
78-91: Mock 组件增强以支持回调测试。更新
SessionMessagesDetailsTabs的 mock 实现,接收onCopyResponse和isResponseCopied属性,能够正确测试复制响应功能的交互流程。
194-195: 使用 aria-label 选择器定位按钮。从基于文本内容的查询改为基于
aria-label属性的查询,与主组件的无障碍改进保持一致。这种方式更稳健,不受翻译文本变化的影响。tests/unit/actions/providers.test.ts (1)
194-395: 测试覆盖全面!autoSortProviderPriority 的测试套件覆盖了预览模式、应用模式、边界情况、错误处理和访问控制等多个场景,测试设计合理且全面。
src/actions/providers.ts (1)
765-889: 实现逻辑正确,错误处理完善autoSortProviderPriority 函数的实现逻辑清晰合理:
- 权限检查和边界条件处理正确
- 对无效的 costMultiplier 值的降级处理(fallback 到 0 并记录警告)符合预期
- 缓存失效失败时不中断主流程,增强了系统弹性
- 仅在有变更时才执行批量更新,优化了性能
src/app/[locale]/settings/providers/_components/auto-sort-priority-dialog.tsx (3)
85-108: 异步状态管理正确handleOpenChange 中使用 startTransition 包裹异步操作符合 React 19 的 Actions 规范。错误处理完善,失败时正确关闭对话框并显示提示。
131-250: UI 实现合理,用户体验良好对话框组件的实现具有以下优点:
- 预览表格清晰展示分组和优先级变更
- 加载状态和禁用逻辑正确处理用户交互
- 仅在有变更时才启用确认按钮,防止无效操作
- 错误处理友好,失败时保留对话框允许用户重试(应用流程)或关闭(预览失败)
116-116: 查询键"providers"使用一致,但可能缺少相关缓存失效查询键
["providers"]在定义(provider-manager-loader.tsx:48)和失效(本文件:116)处使用一致。但注意,在其他修改供应商的操作中(provider-rich-list-item.tsx、add-provider-dialog.tsx),同时失效
["providers"]和["providers-health"]。而此处仅失效["providers"],未失效["providers-health"]。由于自动排序改变了供应商优先级,建议确认是否也需要失效健康状态缓存。
| type AutoSortResult = { | ||
| groups: Array<{ | ||
| costMultiplier: number; | ||
| priority: number; | ||
| providers: Array<{ id: number; name: string }>; | ||
| }>; | ||
| changes: Array<{ | ||
| providerId: number; | ||
| name: string; | ||
| oldPriority: number; | ||
| newPriority: number; | ||
| costMultiplier: number; | ||
| }>; | ||
| summary: { | ||
| totalProviders: number; | ||
| changedCount: number; | ||
| groupCount: number; | ||
| }; | ||
| applied: boolean; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
建议将 AutoSortResult 类型提取到共享位置
此类型定义与 UI 组件文件 src/app/[locale]/settings/providers/_components/auto-sort-priority-dialog.tsx(第 29-48 行)中的定义重复。这违反了 DRY 原则,增加了维护成本和不一致的风险。
♻️ 建议的重构方案
将类型提取到共享类型文件(例如 src/types/provider.ts 或 src/actions/types.ts):
// src/types/provider.ts 或 src/actions/types.ts
export type AutoSortResult = {
groups: Array<{
costMultiplier: number;
priority: number;
providers: Array<{ id: number; name: string }>;
}>;
changes: Array<{
providerId: number;
name: string;
oldPriority: number;
newPriority: number;
costMultiplier: number;
}>;
summary: {
totalProviders: number;
changedCount: number;
groupCount: number;
};
applied: boolean;
};然后在两个文件中导入:
// src/actions/providers.ts
import type { AutoSortResult } from "@/types/provider";
// src/app/[locale]/settings/providers/_components/auto-sort-priority-dialog.tsx
import type { AutoSortResult } from "@/types/provider";🤖 Prompt for AI Agents
In @src/actions/providers.ts around lines 58 - 77, Duplicate AutoSortResult type
exists in providers.ts and the AutoSortPriorityDialog component; extract a
single exported AutoSortResult into a shared types module (e.g., create and
export AutoSortResult from a new types file) and replace both local definitions
with import type { AutoSortResult } from that module. Update the providers.ts
declaration and the AutoSortPriorityDialog usage to import the shared type and
remove the duplicated inline type definitions so both files reference the same
exported AutoSortResult.
| type AutoSortResult = { | ||
| groups: Array<{ | ||
| costMultiplier: number; | ||
| priority: number; | ||
| providers: Array<{ id: number; name: string }>; | ||
| }>; | ||
| changes: Array<{ | ||
| providerId: number; | ||
| name: string; | ||
| oldPriority: number; | ||
| newPriority: number; | ||
| costMultiplier: number; | ||
| }>; | ||
| summary: { | ||
| totalProviders: number; | ||
| changedCount: number; | ||
| groupCount: number; | ||
| }; | ||
| applied: boolean; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
类型定义重复
此 AutoSortResult 类型定义与 src/actions/providers.ts(第 58-77 行)中的定义完全相同。请参考该文件的重构建议,将类型提取到共享位置并在两处导入。
🤖 Prompt for AI Agents
In
@src/app/[locale]/settings/providers/_components/auto-sort-priority-dialog.tsx
around lines 29 - 48, The AutoSortResult type is duplicated; extract
AutoSortResult into a single shared types module (e.g., create and export it
from a new providers/types module), remove the local AutoSortResult definition
in this component, and import the shared AutoSortResult type here and in the
other providers actions module where the duplicate currently exists; ensure the
exported name matches exactly and update any imports/usages to reference the
shared type.
There was a problem hiding this comment.
Code Review Summary
This PR implements automatic provider priority sorting based on cost multipliers. It adds a preview/apply workflow, fixes cache invalidation issues, and improves accessibility.
PR Size: XL
- Lines changed: 1131 (999 additions + 132 deletions)
- Files changed: 25
Split Suggestions (XL PR):
This is a large PR but the changes are cohesive around a single feature. If splitting were required, logical divisions would be:
- Auto-sort feature (actions, repository, dialog) - core functionality
- Test improvements (test refactoring, locale message imports)
- Bug fixes (QueryClient isolation, AlertDialogDescription accessibility)
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Review Analysis
1. Auto-Sort Feature (src/actions/providers.ts)
- Permission check properly requires admin role
- Invalid costMultiplier values handled gracefully with fallback to 0 and logging
- Cache invalidation failure is caught and logged but doesn't fail the operation (graceful degradation)
- Preview mode (
confirm: false) prevents writes - good safeguard
2. Repository Batch Update (src/repository/provider.ts)
- Uses efficient CASE SQL for batch updates
- Deduplication handles duplicate IDs (last one wins)
deleted_at IS NULLfilter prevents updating soft-deleted providers- Returns rowCount for accurate affected count
3. QueryClient Fix (provider-manager-loader.tsx)
- Removing local QueryClientProvider is correct - the global provider in
src/app/providers.tsxalready provides QueryClient context - Individual query options now explicitly set
refetchOnWindowFocus: falseandstaleTime: 30_000to maintain the same behavior - This fix enables cache invalidation to work across components
4. UI Dialog (auto-sort-priority-dialog.tsx)
- Uses
useTransitionfor non-blocking preview loading - Proper loading states with
isPendingandisApplying - Error messages handled through i18n with fallback
- Disables confirm button when no changes or loading
5. Accessibility Improvements
- Added
aria-labelto icon-only buttons in session messages client AlertDialogDescription asChildfixes HTML validation (div inside p)
6. Test Coverage
- Comprehensive unit tests for
autoSortProviderPrioritycovering:- Preview mode vs apply mode
- Invalid costMultiplier handling
- Single provider, multiple providers
- Already-sorted scenarios
- Non-admin rejection
- Cache invalidation failure resilience
- Empty provider list
- Repository tests verify SQL generation and deduplication
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean (admin-only action, parameterized SQL)
- Error handling - Clean (graceful degradation for cache failures)
- Type safety - Clean (proper typing with AutoSortResult)
- Documentation accuracy - Clean
- Test coverage - Adequate (comprehensive edge case coverage)
- Code clarity - Good
Automated review by Claude AI
There was a problem hiding this comment.
Code Review Summary
This PR implements automatic provider priority sorting based on cost multipliers. It adds a preview/apply workflow, fixes cache invalidation issues, and improves accessibility.
PR Size: XL
- Lines changed: 1131 (999 additions + 132 deletions)
- Files changed: 25
Split Suggestions (XL PR):
This is a large PR but the changes are cohesive around a single feature. If splitting were required, logical divisions would be:
- Auto-sort feature (actions, repository, dialog) - core functionality
- Test improvements (test refactoring, locale message imports)
- Bug fixes (QueryClient isolation, AlertDialogDescription accessibility)
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Review Analysis
1. Auto-Sort Feature (src/actions/providers.ts)
- Permission check properly requires admin role
- Invalid costMultiplier values handled gracefully with fallback to 0 and logging
- Cache invalidation failure is caught and logged but does not fail the operation (graceful degradation)
- Preview mode (confirm: false) prevents writes - good safeguard
2. Repository Batch Update (src/repository/provider.ts)
- Uses efficient CASE SQL for batch updates
- Deduplication handles duplicate IDs (last one wins)
- deleted_at IS NULL filter prevents updating soft-deleted providers
- Returns rowCount for accurate affected count
3. QueryClient Fix (provider-manager-loader.tsx)
- Removing local QueryClientProvider is correct - the global provider in src/app/providers.tsx already provides QueryClient context
- Individual query options now explicitly set refetchOnWindowFocus: false and staleTime: 30_000 to maintain the same behavior
- This fix enables cache invalidation to work across components
4. UI Dialog (auto-sort-priority-dialog.tsx)
- Uses useTransition for non-blocking preview loading
- Proper loading states with isPending and isApplying
- Error messages handled through i18n with fallback
- Disables confirm button when no changes or loading
5. Accessibility Improvements
- Added aria-label to icon-only buttons in session messages client
- AlertDialogDescription asChild fixes HTML validation (div inside p)
6. Test Coverage
- Comprehensive unit tests for autoSortProviderPriority covering:
- Preview mode vs apply mode
- Invalid costMultiplier handling
- Single provider, multiple providers
- Already-sorted scenarios
- Non-admin rejection
- Cache invalidation failure resilience
- Empty provider list
- Repository tests verify SQL generation and deduplication
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean (admin-only action, parameterized SQL)
- Error handling - Clean (graceful degradation for cache failures)
- Type safety - Clean (proper typing with AutoSortResult)
- Documentation accuracy - Clean
- Test coverage - Adequate (comprehensive edge case coverage)
- Code clarity - Good
Automated review by Claude AI
There was a problem hiding this comment.
Code Review Summary
This PR mixes a large feature (provider auto-sort), UI/UX fixes, and test/config changes, which increases review surface area and makes regressions harder to isolate. One high-priority standards issue was found in new server-side code; otherwise no additional high-confidence defects were identified in the diff.
PR Size: XL
- Lines changed: 1131
- Files changed: 25
- Split suggestion (XL): consider separate PRs for (1) provider auto-sort feature + i18n, (2) React Query client unification fix, (3) sessions UI a11y/hydration fixes + related tests, (4) Vitest/TS config refactors.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 1 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
(none)
High Priority Issues (Should Fix)
- / [STANDARD-VIOLATION] Hardcoded non-i18n error strings in new response path (Confidence: 90)
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
There was a problem hiding this comment.
Code Review Summary
This PR mixes a large feature (provider auto-sort), UI/UX fixes, and test/config refactors, which increases review surface area and makes regressions harder to isolate. One high-priority standards issue was found in new server-side code; otherwise no additional high-confidence defects were identified in the diff.
PR Size: XL
- Lines changed: 1131
- Files changed: 25
- Split suggestion (XL): consider separate PRs for (1) provider auto-sort feature + i18n, (2) React Query client unification fix, (3) sessions UI a11y/hydration fixes + related tests, (4) Vitest/TS config refactors.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 1 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
(none)
High Priority Issues (Should Fix)
src/actions/providers.ts:771/src/actions/providers.ts:886[STANDARD-VIOLATION] Hardcoded non-i18n error strings in newautoSortProviderPriorityresponse path (Confidence: 90)
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
There was a problem hiding this comment.
- Applied PR size label
size/XLto PR#569(1131 lines changed / 25 files). - Left 1 inline review comment on
src/actions/providers.ts:771flagging a [High] [STANDARD-VIOLATION]: newautoSortProviderPriorityreturns hardcoded non-i18n error strings; suggested returningerrorCodevalues likePERMISSION_DENIED/UPDATE_FAILED(and updatingtests/unit/actions/providers.test.tsaccordingly). - Submitted the required summary review via
gh pr review(a corrected summary was posted after an earlier formatting-glitched review body from this bot).
|
LGTM. |
冲突已经解决,并修复了一个 |
|
hi,我注意到这一功能仅在 |
Summary
Implements automatic provider priority sorting based on cost multipliers, with a preview/confirm workflow. This feature allows administrators to automatically assign priorities to providers where lower cost = higher priority.
Fixes #555 - Implements the auto-sort priority feature requested in the issue.
Problem
When managing multiple providers with different cost multipliers, administrators had to manually set priorities for each provider. This was tedious and error-prone, especially when wanting to ensure that lower-cost providers get higher priority (lower priority number).
Solution
Added an
autoSortProviderPriorityaction with a two-step workflow:confirm: false): Shows what changes would be made without applying themconfirm: true): Executes the batch priority updateThe algorithm:
costMultipliervalue (same cost = same priority group)costMultiplier= lower priority number = higher prioritycostMultipliervalues gracefully fallback to 0Changes
Core Changes
src/actions/providers.ts: AddedautoSortProviderPriorityaction with preview/apply modessrc/repository/provider.ts: AddedupdateProviderPrioritiesBatchusing efficient CASE SQL for batch updates with deduplicationsrc/app/[locale]/settings/providers/_components/auto-sort-priority-dialog.tsx: New dialog component with preview table and confirmation flowBug Fixes (included in this PR)
src/app/[locale]/settings/providers/_components/provider-manager-loader.tsx: Fixed QueryClient isolation issue by removing local QueryClientProvider, using global QueryClient for proper cache invalidationsrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client.tsx: Fixed HTML validation error in AlertDialogDescription usingasChildprop; added aria-labels for accessibilitySupporting Changes
Breaking Changes
None. This is a new additive feature.
Testing
Automated Tests
autoSortProviderPriorityaction (edge cases: empty lists, invalid costMultipliers, duplicate IDs, already-sorted scenarios)updateProviderPrioritiesBatchrepository methodManual Testing
Architecture
sequenceDiagram participant User participant UI as AutoSortPriorityDialog participant Action as autoSortProviderPriority participant Repo as updateProviderPrioritiesBatch participant Cache as publishProviderCacheInvalidation participant DB as Database User->>UI: Click "Auto Sort Priority" button UI->>Action: autoSortProviderPriority({confirm: false}) Action->>DB: findAllProvidersFresh() DB-->>Action: providers list Action->>Action: Group by costMultiplier, calculate priorities Action-->>UI: Return preview data UI->>UI: Display before/after table User->>UI: Click "Apply Changes" UI->>Action: autoSortProviderPriority({confirm: true}) Action->>Repo: updateProviderPrioritiesBatch(changes) Repo->>DB: UPDATE providers SET priority = CASE... Action->>Cache: publishProviderCacheInvalidation() Action-->>UI: Return success UI->>UI: invalidateQueries(["providers"])Checklist
bun run test)bun run build)bun run lint)Description enhanced by Claude AI