feat: advanced model whitelist matching, match tester, and scheduling simulator#995
Closed
ding113 wants to merge 1 commit into
Closed
feat: advanced model whitelist matching, match tester, and scheduling simulator#995ding113 wants to merge 1 commit into
ding113 wants to merge 1 commit into
Conversation
… simulator - Upgrade provider allowedModels from simple string[] to rule-based matching with 5 match types (exact/prefix/suffix/contains/regex), consistent with model redirect rules - Add ModelMatchTester component for both redirect and whitelist editors, enabling inline client-side match testing - Add SchedulingTestDialog on providers page to simulate the full provider selection decision chain (group filter, format/model/schedule checks, circuit breaker, priority/weight selection) - Extract shared matchPattern() utility used by both redirect and whitelist - Backward compatible: legacy string[] allowedModels normalized at read boundary - i18n support for all 5 languages (en, zh-CN, zh-TW, ja, ru) - TDD: 80+ new unit tests covering matching, normalization, schema validation, and scheduling simulation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthrough本PR将provider的允许模型配置从简单字符串数组重构为结构化规则对象数组，支持精确、前缀、后缀、包含和正则表达式五种匹配类型。新增UI组件用于编辑模型规则、测试匹配结果和模拟调度流程，同时为英文、日文、俄文、简体中文和繁体中文添加了对应的本地化文本。 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 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 docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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 |
github-actions
Bot
added
enhancement
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request enhances the provider model filtering system by replacing the string-based whitelist with a flexible rule-based approach supporting exact, prefix, suffix, contains, and regex matching. Key additions include an AllowedModelEditor component, a ModelMatchTester for rule verification, and a Scheduling Test feature to simulate the provider selection chain. Review feedback identifies a case-sensitivity inconsistency between the matching logic and uniqueness validation, and points out redundant onInput event handlers in the new UI components.
Comment on lines
+14
to
+31
| switch (matchType) { | ||
| case "exact": | ||
| return value === pattern; | ||
| case "prefix": | ||
| return value.startsWith(pattern); | ||
| case "suffix": | ||
| return value.endsWith(pattern); | ||
| case "contains": | ||
| return value.includes(pattern); | ||
| case "regex": | ||
| try { | ||
| return new RegExp(pattern).test(value); | ||
| } catch { | ||
| return false; | ||
| } | ||
| default: | ||
| return false; | ||
| } |
Contributor
There was a problem hiding this comment.
There is a case-sensitivity inconsistency between the matching logic here and the uniqueness validation in the UI and schema.
In src/lib/provider-allowed-model-schema.ts (line 54) and allowed-model-editor.tsx (line 34), uniqueness is enforced case-insensitively using .toLowerCase(). However, the matching logic here is case-sensitive (e.g., value === pattern).
This means if a user adds "GPT-4" to the whitelist, a request for "gpt-4" will fail to match, but the user will be prevented from adding a separate "gpt-4" rule because it's considered a duplicate. For model identifiers, case-insensitive matching is generally preferred and expected.
const v = value.toLowerCase();
const p = pattern.toLowerCase();
switch (matchType) {
case "exact":
return v === p;
case "prefix":
return v.startsWith(p);
case "suffix":
return v.endsWith(p);
case "contains":
return v.includes(p);
case "regex":
try {
return new RegExp(pattern, "i").test(value);
} catch {
return false;
}
default:
return false;
}
Comment on lines
+223
to
+228
| onInput={(e) => | ||
| setEditRule((current) => ({ | ||
| ...current, | ||
| pattern: (e.target as HTMLInputElement).value, | ||
| })) | ||
| } |
Contributor
There was a problem hiding this comment.
The onInput handler is redundant here. In React, the onChange event on an element already fires on every keystroke (behaving like the native onInput). Since the logic in both handlers is identical, onInput can be safely removed to simplify the code.
Comment on lines
+360
to
+365
| onInput={(e) => | ||
| setNewRule((current) => ({ | ||
| ...current, | ||
| pattern: (e.target as HTMLInputElement).value, | ||
| })) | ||
| } |
Contributor
There was a problem hiding this comment.
The onInput handler is redundant because the onChange handler on line 359 already performs the same state update. In React, onChange for text inputs is triggered on every change, making onInput unnecessary.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a40a3933b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // 2. 设置了 allowedModels：只按原始请求模型做白名单匹配 | ||
| return provider.allowedModels.includes(requestedModel); | ||
| // 2. 设置了 allowedModels：按规则匹配（支持 exact/prefix/suffix/contains/regex） | ||
| return modelMatchesAllowedRules(requestedModel, provider.allowedModels); |
There was a problem hiding this comment.
This new whitelist path assumes provider.allowedModels is already ProviderAllowedModelRule[], but existing rows can still contain legacy string[] values (the schema type still permits that). When those legacy arrays reach modelMatchesAllowedRules, each string element has no matchType, so every rule check returns false and providers that previously matched exact model names are now filtered out. This can break routing for existing providers immediately after deploy unless data is normalized on read.
Useful? React with 👍 / 👎.
Comment on lines
283
to
+287
| case "allowed_models": | ||
| return Array.isArray(value) && value.every((model) => typeof model === "string"); | ||
| return ( | ||
| Array.isArray(value) && | ||
| value.every( | ||
| (rule) => |
There was a problem hiding this comment.
The batch patch validator for allowed_models only checks that each item is an object with string fields, so invalid matchType values, empty patterns, duplicates, and unsafe regex patterns can all pass through this path. Those rules are later executed in request-time model matching, which can silently block all models or introduce expensive regex evaluation, bypassing the stricter protections already enforced in Create/Update schemas. This should use PROVIDER_ALLOWED_MODEL_RULE_LIST_SCHEMA.safeParse(...) like model_redirects does.
Useful? React with 👍 / 👎.
Comment on lines
+208
to
+212
| const checks = await Promise.all( | ||
| currentPool.map(async (p) => { | ||
| const providerOpen = await isCircuitOpen(p.id); | ||
| const state = getCircuitState(p.id); | ||
| return { provider: p, providerOpen, state }; |
There was a problem hiding this comment.
The simulator’s health stage only checks isCircuitOpen, but production selection also filters by vendor-type circuit state and provider spend limits in ProxyProviderResolver.filterByLimits. In environments where providers are rate-limited or spend-capped, the simulator will report candidates as selectable even though production routing will reject them, which makes the new scheduling diagnostics materially inaccurate.
Useful? React with 👍 / 👎.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review Summary
This is a high-quality feature implementation introducing advanced model whitelist matching, match testing UI, and a scheduling simulator. The code demonstrates excellent engineering practices with comprehensive test coverage (80+ new tests), full i18n support across 5 languages, and proper TypeScript typing throughout.
PR Size: XL
- Lines changed: 3007 (2880 additions, 127 deletions)
- Files changed: 50
- Split suggestion: This could theoretically be split into 3 focused PRs (pattern matching core, whitelist UI, scheduling simulator), but the tight coupling between features makes the current bundling reasonable.
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 |
No significant issues identified.
Review Coverage
- Logic and correctness - Clean implementation with proper edge case handling
- Security (OWASP Top 10) - ReDoS protection via
safe-regexfor user-provided patterns - Error handling - Proper error propagation with user-facing feedback
- Type safety - Strong typing throughout, no
anyusage - Documentation accuracy - Comments match implementation
- Test coverage - 4184 tests passing, 80+ new tests for new features
- Code clarity - Well-structured with clear separation of concerns
Notable Strengths
-
Backward Compatibility: Legacy
string[]format forallowedModelsis gracefully normalized to the new rule format at read-time vianormalizeAllowedModelRules() -
Security: ReDoS protection for user-provided regex patterns using
safe-regexlibrary with proper error handling inprovider-allowed-model-schema.ts:36-52 -
Type Safety: Clean type hierarchy with
ProviderAllowedModelRuleandProviderModelRedirectMatchTypeproperly shared between redirect and whitelist features -
Test Quality: Comprehensive unit tests covering edge cases including empty patterns, invalid regex, case-insensitive duplicates, and ReDoS detection
-
i18n: Complete translation coverage for 5 languages (en, zh-CN, zh-TW, ja, ru) across 3 new namespaces
Automated review by Claude AI
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+104
to
+107
| {t( | ||
| `reason${f.reason.replace(/_./g, (m) => m[1].toUpperCase()).replace(/^./, (c) => c.toUpperCase())}` | ||
| )} | ||
| </Badge> |
There was a problem hiding this comment.
The StepCard component derives a translation key by camelCasing f.reason and prefixing with "reason". Two reason codes emitted by scheduling-simulator.ts map to keys that do not exist in any of the 5 language files:
"group_mismatch"(emitted atscheduling-simulator.tsline 115) →reasonGroupMismatch— absent from all language files"format_type_mismatch"(emitted atscheduling-simulator.tsline 167) →reasonFormatTypeMismatch— absent from all language files; the files definereasonFormatMismatch(noTypesegment)
Whenever these two filter steps eliminate a provider, next-intl will render the raw key path instead of human-readable text. Fix: add "reasonGroupMismatch" and "reasonFormatTypeMismatch" to all 5 messages/*/settings/providers/schedulingTest.json files.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/[locale]/settings/providers/_components/scheduling-test-dialog.tsx
Line: 104-107
Comment:
**Two missing i18n reason keys break simulator UI**
The `StepCard` component derives a translation key by camelCasing `f.reason` and prefixing with `"reason"`. Two reason codes emitted by `scheduling-simulator.ts` map to keys that do not exist in any of the 5 language files:
- `"group_mismatch"` (emitted at `scheduling-simulator.ts` line 115) → `reasonGroupMismatch` — absent from all language files
- `"format_type_mismatch"` (emitted at `scheduling-simulator.ts` line 167) → `reasonFormatTypeMismatch` — absent from all language files; the files define `reasonFormatMismatch` (no `Type` segment)
Whenever these two filter steps eliminate a provider, `next-intl` will render the raw key path instead of human-readable text. Fix: add `"reasonGroupMismatch"` and `"reasonFormatTypeMismatch"` to all 5 `messages/*/settings/providers/schedulingTest.json` files.
How can I resolve this? If you propose a fix, please make it concise.| toast.error(res.error); | ||
| } | ||
| } catch { | ||
| toast.error("Simulation failed"); |
There was a problem hiding this comment.
CLAUDE.md mandates that all user-facing strings use i18n. The t hook is already in scope — add a "simulationFailed" key to all 5 language files and replace the literal.
Suggested change
| toast.error("Simulation failed"); | |
| toast.error(t("simulationFailed")); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/[locale]/settings/providers/_components/scheduling-test-dialog.tsx
Line: 158
Comment:
**Hardcoded English string violates i18n rule**
CLAUDE.md mandates that all user-facing strings use i18n. The `t` hook is already in scope — add a `"simulationFailed"` key to all 5 language files and replace the literal.
```suggestion
toast.error(t("simulationFailed"));
```
How can I resolve this? If you propose a fix, please make it concise.| `step${step.name.replace(/_./g, (m) => m[1].toUpperCase()).replace(/^./, (c) => c.toUpperCase())}` | ||
| )} | ||
| </Badge> | ||
| <span className="text-xs text-muted-foreground truncate">{step.description}</span> |
There was a problem hiding this comment.
step.description is a hardcoded English string from the server side (e.g. "Filter by provider group tags"). All 5 language files already define matching step*Desc keys (stepGroupFilterDesc, stepBasicFilterDesc, etc.) that are never referenced by the component. Replace the static field with a translated lookup using the same camelCase pattern already applied to step names:
Suggested change
| <span className="text-xs text-muted-foreground truncate">{step.description}</span> | |
| <span className="text-xs text-muted-foreground truncate">{t(`step${step.name.replace(/_./g, (m) => m[1].toUpperCase()).replace(/^./, (c) => c.toUpperCase())}Desc`)}</span> |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/[locale]/settings/providers/_components/scheduling-test-dialog.tsx
Line: 53
Comment:
**Step descriptions bypass i18n — translation keys exist but are never used**
`step.description` is a hardcoded English string from the server side (e.g. `"Filter by provider group tags"`). All 5 language files already define matching `step*Desc` keys (`stepGroupFilterDesc`, `stepBasicFilterDesc`, etc.) that are never referenced by the component. Replace the static field with a translated lookup using the same camelCase pattern already applied to step names:
```suggestion
<span className="text-xs text-muted-foreground truncate">{t(`step${step.name.replace(/_./g, (m) => m[1].toUpperCase()).replace(/^./, (c) => c.toUpperCase())}Desc`)}</span>
```
How can I resolve this? If you propose a fix, please make it concise.| import type { SchedulingSimulationResult, SimulationStep } from "@/lib/scheduling-simulator"; | ||
| import { cn } from "@/lib/utils"; | ||
|
|
||
| type FormatOption = "claude" | "openai" | "response" | "gemini"; |
There was a problem hiding this comment.
gemini-cli format missing from simulator
checkFormatProviderTypeCompatibility in provider-selector.ts explicitly handles "gemini-cli" as a distinct format that maps to providerType === "gemini-cli" providers. Users with gemini-cli providers cannot simulate routing for that format in the dialog.
Suggested change
| type FormatOption = "claude" | "openai" | "response" | "gemini"; | |
| type FormatOption = "claude" | "openai" | "response" | "gemini" | "gemini-cli"; |
Also add a corresponding <SelectItem value="gemini-cli"> entry in the <SelectContent> block.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/[locale]/settings/providers/_components/scheduling-test-dialog.tsx
Line: 32
Comment:
**`gemini-cli` format missing from simulator**
`checkFormatProviderTypeCompatibility` in `provider-selector.ts` explicitly handles `"gemini-cli"` as a distinct format that maps to `providerType === "gemini-cli"` providers. Users with `gemini-cli` providers cannot simulate routing for that format in the dialog.
```suggestion
type FormatOption = "claude" | "openai" | "response" | "gemini" | "gemini-cli";
```
Also add a corresponding `<SelectItem value="gemini-cli">` entry in the `<SelectContent>` block.
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/actions/providers.ts (1)
2265-2269:⚠️ Potential issue | 🟠 Major
allowed_models的批量更新分支绕过了后端规则校验。
model_redirects这里会先safeParse()，但allowed_models直接透传给updateProvidersBatch()。这样空 pattern、非法 regex，甚至不安全 regex 都能绕过本 PR 新增的 schema/ReDoS 保护落库。建议修改
+import { PROVIDER_ALLOWED_MODEL_RULE_LIST_SCHEMA } from "@/lib/provider-allowed-model-schema"; @@ if (updates.allowed_models !== undefined) { - repositoryUpdates.allowedModels = - Array.isArray(updates.allowed_models) && updates.allowed_models.length === 0 - ? null - : updates.allowed_models; + if (updates.allowed_models === null) { + repositoryUpdates.allowedModels = null; + } else if (Array.isArray(updates.allowed_models) && updates.allowed_models.length === 0) { + repositoryUpdates.allowedModels = null; + } else { + const parsedAllowedModels = + PROVIDER_ALLOWED_MODEL_RULE_LIST_SCHEMA.safeParse(updates.allowed_models); + if (!parsedAllowedModels.success) { + return { ok: false, error: "允许模型规则格式无效" }; + } + repositoryUpdates.allowedModels = parsedAllowedModels.data; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/providers.ts` around lines 2265 - 2269, The batch update path lets updates.allowed_models bypass schema validation and ReDoS protections; before assigning repositoryUpdates.allowedModels (and before calling updateProvidersBatch), validate and sanitize updates.allowed_models the same way model_redirects is handled: run it through the schema/safeParse used for provider/model patterns, reject or filter out empty strings and invalid/unsafe regex patterns, and convert an empty validated array to null; update the logic around repositoryUpdates.allowedModels and the updateProvidersBatch call to use the validated result so no unvalidated patterns reach the database.
🧹 Nitpick comments (8)
src/app/[locale]/settings/providers/_components/forms/provider-form/provider-form-context.tsx (1)
369-369: 建议统一allowedModels的规范化处理，确保批量与编辑路径行为一致。Line 369 的编辑/克隆路径已应用
normalizeAllowedModelRules()来处理新旧格式兼容（数据库允许存储ProviderAllowedModelRule[] | string[]），但第 116-119 行的批量分支仍直接使用analysis.routing.allowedModels.value，未经规范化处理。建议在批量分支也应用同样的规范化逻辑，避免不同入口处理相同字段时产生格式差异。建议修改
allowedModels: analysis.routing.allowedModels.status === "uniform" - ? analysis.routing.allowedModels.value + ? (normalizeAllowedModelRules(analysis.routing.allowedModels.value) ?? []) : [],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/settings/providers/_components/forms/provider-form/provider-form-context.tsx at line 369, The batch path is using analysis.routing.allowedModels.value directly while the edit/clone path uses normalizeAllowedModelRules(sourceProvider?.allowedModels); update the batch branch to call normalizeAllowedModelRules on analysis.routing.allowedModels.value (or its nullable form) so both code paths produce the same normalized ProviderAllowedModelRule[]; locate the batch handling that references analysis.routing.allowedModels.value and replace/pass it through normalizeAllowedModelRules, keeping existing null/undefined fallbacks consistent with the edit/clone logic.tests/unit/dashboard/provider-form-clone-deep-copy.test.ts (1)
21-24: 建议补充元素级引用断言，防止对象数组“浅拷贝漏检”。
allowedModels现在是对象数组（Line 21-24），当前只校验数组本身not.toBe，若元素对象被复用，测试仍可能通过。建议在 clone/edit 两处都补首元素引用断言。可选修正示例
it("allowedModels is a distinct array with equal values", () => { const source = makeProvider(); const state = createInitialState("create", undefined, source); expect(state.routing.allowedModels).toEqual(source.allowedModels); expect(state.routing.allowedModels).not.toBe(source.allowedModels); + if (state.routing.allowedModels.length > 0 && source.allowedModels && source.allowedModels.length > 0) { + expect(state.routing.allowedModels[0]).not.toBe(source.allowedModels[0]); + } }); @@ it("nested objects are isolated from source provider", () => { const source = makeProvider(); const state = createInitialState("edit", source); expect(state.routing.modelRedirects).toEqual(source.modelRedirects); expect(state.routing.modelRedirects).not.toBe(source.modelRedirects); expect(state.routing.allowedModels).not.toBe(source.allowedModels); + if (state.routing.allowedModels.length > 0 && source.allowedModels && source.allowedModels.length > 0) { + expect(state.routing.allowedModels[0]).not.toBe(source.allowedModels[0]); + } expect(state.routing.groupPriorities).not.toBe(source.groupPriorities); expect(state.routing.anthropicAdaptiveThinking).not.toBe(source.anthropicAdaptiveThinking); });Also applies to: 79-84, 146-147
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/dashboard/provider-form-clone-deep-copy.test.ts` around lines 21 - 24, The test currently only asserts the array identity for allowedModels and can miss shallow-copy issues; update the unit test in provider-form-clone-deep-copy.test.ts to also assert element-level reference inequality for the objects inside allowedModels (e.g., check that allowedModels[0] in the cloned result is not the same reference as the original), and add the same element-reference assertion in the edit-path assertions; target the allowedModels array checks around the clone and edit assertions (first element reference) so shallow copies are detected.src/app/[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx (1)
22-22: 建议改为@/路径别名导入，避免深层相对路径。当前新增导入继续使用
../../../，与仓库的 TS/TSX 导入规范不一致，建议统一改为别名路径。As per coding guidelines
**/*.{ts,tsx,js,jsx}: Use path alias@/mapped to ./src/ for imports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx at line 22, Replace the deep relative import of AllowedModelEditor with the project path-alias form (start with "@/") to follow the repo's TS/TSX import convention; locate the import statement for AllowedModelEditor in routing-section.tsx and change "../../../allowed-model-editor" to the corresponding "@/..." alias path that resolves to the same module while keeping the symbol name AllowedModelEditor unchanged (also update any other sibling deep-relative imports in this file to use the "@/..." alias).src/repository/provider.ts (1)
194-195: 建议把allowedModels归一化扩展到更新路径，保持写入一致性。目前仅创建路径做了
normalizeAllowedModelRules，但同文件的updateProvider/updateProvidersBatch仍直接透传，建议统一处理，避免不同入口写入形态不一致。建议修改
@@ - if (providerData.allowed_models !== undefined) dbData.allowedModels = providerData.allowed_models; + if (providerData.allowed_models !== undefined) { + dbData.allowedModels = normalizeAllowedModelRules(providerData.allowed_models); + } @@ - if (updates.allowedModels !== undefined) { - setClauses.allowedModels = updates.allowedModels; - } + if (updates.allowedModels !== undefined) { + setClauses.allowedModels = normalizeAllowedModelRules(updates.allowedModels); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/repository/provider.ts` around lines 194 - 195, The creation path normalizes allowed model rules via normalizeAllowedModelRules(providerData.allowed_models) but updateProvider and updateProvidersBatch still write allowed_models directly, causing inconsistent stored shapes; update those update paths to call normalizeAllowedModelRules on incoming allowed_models (and set allowedModels/allowedModels property consistent with creation) and ensure allowedClients still use providerData.allowed_clients ?? [] so all write paths use the same normalized form (referencing normalizeAllowedModelRules, allowedModels, updateProvider, updateProvidersBatch, and providerData.allowed_models).tests/unit/lib/scheduling-simulator.test.ts (1)
218-243: 这个用例的 mock 恢复放在断言后面，失败时会污染后续 case。如果前面的
expect()抛错，isCircuitOpen/getCircuitState不会被重置，后面的测试会继承这个 “open” 状态。把恢复逻辑移到afterEach()或try/finally会更稳。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/lib/scheduling-simulator.test.ts` around lines 218 - 243, The test leaves mocks for circuitBreaker.isCircuitOpen and circuitBreaker.getCircuitState reset only after assertions, which can leak state if an assertion fails; update the test to always restore mocks by moving the mock reset into a shared afterEach() or wrap the assertions in a try/finally around the call to simulateProviderScheduling/expect, ensuring vi.mocked(circuitBreaker.isCircuitOpen).mockResolvedValue(false) and vi.mocked(circuitBreaker.getCircuitState).mockReturnValue("closed") are executed in the finally/afterEach; reference the mocked symbols isCircuitOpen and getCircuitState and the test that calls simulateProviderScheduling to locate where to apply the change.src/app/[locale]/settings/providers/_components/allowed-model-editor.tsx (1)
12-13: 这里新增了相对导入，和仓库的@/约定不一致。这是新文件，建议直接改成
@/app/[locale]/settings/providers/_components/...形式，后续移动目录时会更稳。As per coding guidelines "Use path alias
@/mapped to ./src/ for imports".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/settings/providers/_components/allowed-model-editor.tsx around lines 12 - 13, The new imports use relative paths; change them to the project's path-alias form so they follow the "@/..." convention: replace the relative imports for getMatchTypeOptions, MatchTypeBadge, MatchTypeSelect and ModelMatchTester with alias-based imports (using the `@/` path mapped to ./src/) that point to the same components under the app settings providers components directory so future moves remain stable.src/lib/provider-allowed-models.ts (1)
4-10: 建议解耦 allowed-model 与 redirect 的 matchType 类型定义。
ALLOWED_MODEL_MATCH_TYPES目前依赖ProviderModelRedirectMatchType，会把两个领域规则耦合在一起。建议改为ProviderAllowedModelRule["matchType"]（或独立的 allowed-model match type），避免未来两侧规则演进时互相牵连。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/provider-allowed-models.ts` around lines 4 - 10, ALLOWED_MODEL_MATCH_TYPES currently typeset using ProviderModelRedirectMatchType which couples allowed-model rules with redirect rules; change the type to reference ProviderAllowedModelRule["matchType"] (or define a standalone AllowedModelMatchType) and update the declaration of ALLOWED_MODEL_MATCH_TYPES to use that type instead of ProviderModelRedirectMatchType so allowed-model concerns are decoupled from redirect match types; ensure any imports/types referring to ProviderModelRedirectMatchType are replaced where ALLOWED_MODEL_MATCH_TYPES is declared and update any usages to the new type.src/lib/scheduling-simulator.ts (1)
24-25: 建议收窄providerType类型，避免丢失编译期校验。这里使用
string会放宽约束。建议改为Provider["providerType"]，与主数据模型保持一致，避免后续拼写或非法值静默进入模拟结果。Also applies to: 44-45
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/scheduling-simulator.ts` around lines 24 - 25, The providerType fields in the scheduling simulator are typed too loosely as string; update their types to reference the canonical model so typo/invalid values are caught at compile time — change the declarations of providerType (the ones shown around providerType: string and the other occurrence at lines 44–45) to use Provider["providerType"] (or import/alias the Provider type used in your main data model) so the simulator shares the same allowed providerType union as the rest of the codebase.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@messages/ja/settings/providers/schedulingTest.json`:
- Line 25: The value for the JSON key "stepHealthFilterDesc" contains garbled
characters ("レー��制限"); replace the corrupted substring with the correct Japanese
"レート制限" so the full string reads "サーキットブレーカーとレート制限の状態を確認" (update the
"stepHealthFilterDesc" entry accordingly).
In `@src/app/`[locale]/settings/providers/_components/forms/api-test-button.tsx:
- Around line 95-103: The current loop only extracts exact rules into unique and
then returns Array.from(unique), which causes a misleading fallback to a generic
default model when allowedModels contains only non-exact rules; change the logic
to detect when allowedModels is non-empty but contains zero entries with
rule.matchType === "exact" and in that case return an empty array (or otherwise
signal "no automatic model") instead of populating/using a generic default.
Update the code that builds unique from allowedModels (the loop referencing
allowedModels, rule.matchType, rule.pattern, and the return Array.from(unique))
to first check for any exact rules and only collect/return exact patterns when
present; if none exist, return [] so the UI can require manual input.
In `@src/app/`[locale]/settings/providers/_components/model-match-tester.tsx:
- Around line 103-107: The UI is rendering raw enum values via
redirectResult.rule.matchType (and similarly at the other location) instead of
localized labels; replace the direct rendering with the existing i18n mapping by
using getMatchTypeOptions() (or reuse MatchTypeBadge) to lookup the localized
label for matchType and render that inside the Badge (or swap to MatchTypeBadge)
so all user-facing match type text goes through the translation mapping.
In `@src/app/`[locale]/settings/providers/_components/scheduling-test-dialog.tsx:
- Around line 157-159: The catch block in the SchedulingTestDialog component
currently calls toast.error("Simulation failed") with a hardcoded English
string; replace this with the app i18n key usage consistent with the rest of the
file (e.g., use the local translation helper like t(...) or
getTranslation/locale hook used elsewhere in this component) and call
toast.error(t('schedulingTest.simulationFailed')) (or similar key naming
matching other keys), then add that key and its translations to the locales
(zh-CN, zh-TW, en, ja, ru) to ensure all user-facing text is localized.
- Around line 141-162: handleRun can be invoked multiple times concurrently
because it neither checks isRunning nor disables the input, causing overlapping
simulateSchedulingAction calls and out‑of‑order results; fix by early-returning
if isRunning at the top of handleRun, disable the related input/submit while
isRunning (the JSX input/button tied to model/isRunning), and guard the other
similar handler (the one at lines ~203-209) the same way; for stronger
correctness optionally attach a request sequence token or AbortController to
each simulateSchedulingAction call and ignore stale responses before calling
setResult.
In `@src/lib/provider-allowed-model-schema.ts`:
- Around line 50-64: The deduplication key currently lowercases patterns (and
effectively matchType), causing distinct case-sensitive rules to be treated as
duplicates; update the dedupe logic in the refine callback in
provider-allowed-model-schema (the function that builds keys for rules) to use
exact pattern matching by changing the key to
`${rule.matchType}:${rule.pattern.trim()}` (remove .toLowerCase()), and make the
analogous change in the UI helper getRuleIdentity() inside
allowed-model-editor.tsx (use `${rule.matchType}:${rule.pattern.trim()}`) so
both schema validation and the editor use precise, case-sensitive identity
checks.
In `@src/lib/provider-allowed-models.ts`:
- Around line 51-75: The patch set path currently bypasses normalization causing
inconsistent allowed_models values; update the set branch in the patch handler
(the code that applies updates in the provider patch flow, referenced around
provider-patch-contract / provider repository logic) to call
normalizeAllowedModelRules(value) before persisting, and if it returns null
throw/return a validation error instead of writing the raw value; ensure any
stored rules come from the normalized ProviderAllowedModelRule[] produced by
normalizeAllowedModelRules and reject empty/invalid inputs rather than
persisting them.
In `@src/lib/provider-patch-contract.ts`:
- Around line 284-293: The current validator for allowed_models (the function
that returns Array.isArray(value) && value.every(...)) is too permissive:
restrict matchType to the expected enum strings (e.g., "exact", "regex",
"prefix", etc.) instead of any string, require pattern to be a non-empty string,
and attempt to compile pattern when matchType === "regex" to ensure it's a valid
RegExp (catch and reject invalid regexes). Update the validator logic in the
allowed_models check to enforce those enum values and pattern checks and return
false on invalid/empty patterns or failed RegExp compilation so malformed rules
cannot be stored.
In `@src/lib/scheduling-simulator.ts`:
- Around line 130-136: The review flags that user-facing hardcoded English
strings in the scheduling simulator (the description fields in the step objects,
e.g., the object with name "group_filter" and the other step objects at the
noted ranges) must use i18n keys instead; replace literal strings like "Filter
by provider group tags" and "Weighted random selection ..." with stable i18n
keys (for example simulation.step.group_filter.description,
simulation.step.basic_filter.description, etc.) and ensure the code returns the
key (or accepts an injected translator) rather than raw text so the UI layer can
perform translation; update all step objects that set description (including the
entries around lines 190–197, 231–238, 281–287, 304–310) to use keys and adjust
any consuming code to call the i18n translator.
---
Outside diff comments:
In `@src/actions/providers.ts`:
- Around line 2265-2269: The batch update path lets updates.allowed_models
bypass schema validation and ReDoS protections; before assigning
repositoryUpdates.allowedModels (and before calling updateProvidersBatch),
validate and sanitize updates.allowed_models the same way model_redirects is
handled: run it through the schema/safeParse used for provider/model patterns,
reject or filter out empty strings and invalid/unsafe regex patterns, and
convert an empty validated array to null; update the logic around
repositoryUpdates.allowedModels and the updateProvidersBatch call to use the
validated result so no unvalidated patterns reach the database.
---
Nitpick comments:
In `@src/app/`[locale]/settings/providers/_components/allowed-model-editor.tsx:
- Around line 12-13: The new imports use relative paths; change them to the
project's path-alias form so they follow the "@/..." convention: replace the
relative imports for getMatchTypeOptions, MatchTypeBadge, MatchTypeSelect and
ModelMatchTester with alias-based imports (using the `@/` path mapped to ./src/)
that point to the same components under the app settings providers components
directory so future moves remain stable.
In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/provider-form-context.tsx:
- Line 369: The batch path is using analysis.routing.allowedModels.value
directly while the edit/clone path uses
normalizeAllowedModelRules(sourceProvider?.allowedModels); update the batch
branch to call normalizeAllowedModelRules on
analysis.routing.allowedModels.value (or its nullable form) so both code paths
produce the same normalized ProviderAllowedModelRule[]; locate the batch
handling that references analysis.routing.allowedModels.value and replace/pass
it through normalizeAllowedModelRules, keeping existing null/undefined fallbacks
consistent with the edit/clone logic.
In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx:
- Line 22: Replace the deep relative import of AllowedModelEditor with the
project path-alias form (start with "@/") to follow the repo's TS/TSX import
convention; locate the import statement for AllowedModelEditor in
routing-section.tsx and change "../../../allowed-model-editor" to the
corresponding "@/..." alias path that resolves to the same module while keeping
the symbol name AllowedModelEditor unchanged (also update any other sibling
deep-relative imports in this file to use the "@/..." alias).
In `@src/lib/provider-allowed-models.ts`:
- Around line 4-10: ALLOWED_MODEL_MATCH_TYPES currently typeset using
ProviderModelRedirectMatchType which couples allowed-model rules with redirect
rules; change the type to reference ProviderAllowedModelRule["matchType"] (or
define a standalone AllowedModelMatchType) and update the declaration of
ALLOWED_MODEL_MATCH_TYPES to use that type instead of
ProviderModelRedirectMatchType so allowed-model concerns are decoupled from
redirect match types; ensure any imports/types referring to
ProviderModelRedirectMatchType are replaced where ALLOWED_MODEL_MATCH_TYPES is
declared and update any usages to the new type.
In `@src/lib/scheduling-simulator.ts`:
- Around line 24-25: The providerType fields in the scheduling simulator are
typed too loosely as string; update their types to reference the canonical model
so typo/invalid values are caught at compile time — change the declarations of
providerType (the ones shown around providerType: string and the other
occurrence at lines 44–45) to use Provider["providerType"] (or import/alias the
Provider type used in your main data model) so the simulator shares the same
allowed providerType union as the rest of the codebase.
In `@src/repository/provider.ts`:
- Around line 194-195: The creation path normalizes allowed model rules via
normalizeAllowedModelRules(providerData.allowed_models) but updateProvider and
updateProvidersBatch still write allowed_models directly, causing inconsistent
stored shapes; update those update paths to call normalizeAllowedModelRules on
incoming allowed_models (and set allowedModels/allowedModels property consistent
with creation) and ensure allowedClients still use providerData.allowed_clients
?? [] so all write paths use the same normalized form (referencing
normalizeAllowedModelRules, allowedModels, updateProvider, updateProvidersBatch,
and providerData.allowed_models).
In `@tests/unit/dashboard/provider-form-clone-deep-copy.test.ts`:
- Around line 21-24: The test currently only asserts the array identity for
allowedModels and can miss shallow-copy issues; update the unit test in
provider-form-clone-deep-copy.test.ts to also assert element-level reference
inequality for the objects inside allowedModels (e.g., check that
allowedModels[0] in the cloned result is not the same reference as the
original), and add the same element-reference assertion in the edit-path
assertions; target the allowedModels array checks around the clone and edit
assertions (first element reference) so shallow copies are detected.
In `@tests/unit/lib/scheduling-simulator.test.ts`:
- Around line 218-243: The test leaves mocks for circuitBreaker.isCircuitOpen
and circuitBreaker.getCircuitState reset only after assertions, which can leak
state if an assertion fails; update the test to always restore mocks by moving
the mock reset into a shared afterEach() or wrap the assertions in a try/finally
around the call to simulateProviderScheduling/expect, ensuring
vi.mocked(circuitBreaker.isCircuitOpen).mockResolvedValue(false) and
vi.mocked(circuitBreaker.getCircuitState).mockReturnValue("closed") are executed
in the finally/afterEach; reference the mocked symbols isCircuitOpen and
getCircuitState and the test that calls simulateProviderScheduling to locate
where to apply the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 50a3d368-b252-4973-b2c3-1f2419851947
📒 Files selected for processing (50)
messages/en/settings/providers/form/allowedModelRule.jsonmessages/en/settings/providers/form/matchTester.jsonmessages/en/settings/providers/schedulingTest.jsonmessages/ja/settings/providers/form/allowedModelRule.jsonmessages/ja/settings/providers/form/matchTester.jsonmessages/ja/settings/providers/schedulingTest.jsonmessages/ru/settings/providers/form/allowedModelRule.jsonmessages/ru/settings/providers/form/matchTester.jsonmessages/ru/settings/providers/schedulingTest.jsonmessages/zh-CN/settings/providers/form/allowedModelRule.jsonmessages/zh-CN/settings/providers/form/matchTester.jsonmessages/zh-CN/settings/providers/schedulingTest.jsonmessages/zh-TW/settings/providers/form/allowedModelRule.jsonmessages/zh-TW/settings/providers/form/matchTester.jsonmessages/zh-TW/settings/providers/schedulingTest.jsonsrc/actions/providers.tssrc/actions/scheduling-simulator.tssrc/app/[locale]/settings/providers/_components/allowed-model-editor.tsxsrc/app/[locale]/settings/providers/_components/batch-edit/analyze-batch-settings.tssrc/app/[locale]/settings/providers/_components/forms/api-test-button.tsxsrc/app/[locale]/settings/providers/_components/forms/provider-form.legacy.tsxsrc/app/[locale]/settings/providers/_components/forms/provider-form/provider-form-context.tsxsrc/app/[locale]/settings/providers/_components/forms/provider-form/provider-form-types.tssrc/app/[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsxsrc/app/[locale]/settings/providers/_components/match-rule-shared.tsxsrc/app/[locale]/settings/providers/_components/model-match-tester.tsxsrc/app/[locale]/settings/providers/_components/model-redirect-editor.tsxsrc/app/[locale]/settings/providers/_components/scheduling-test-dialog.tsxsrc/app/[locale]/settings/providers/page.tsxsrc/app/v1/_lib/models/available-models.tssrc/app/v1/_lib/proxy/provider-selector.tssrc/drizzle/schema.tssrc/lib/model-pattern-matching.tssrc/lib/provider-allowed-model-schema.tssrc/lib/provider-allowed-models.tssrc/lib/provider-model-redirects.tssrc/lib/provider-patch-contract.tssrc/lib/scheduling-simulator.tssrc/lib/validation/schemas.tssrc/repository/provider.tssrc/types/provider.tstests/unit/actions/providers-patch-contract.test.tstests/unit/actions/providers.test.tstests/unit/dashboard/provider-form-clone-deep-copy.test.tstests/unit/lib/model-pattern-matching.test.tstests/unit/lib/provider-allowed-model-schema.test.tstests/unit/lib/provider-allowed-models.test.tstests/unit/lib/scheduling-simulator.test.tstests/unit/proxy/provider-selector-cross-type-model.test.tstests/unit/proxy/provider-selector-model-redirect.test.ts
| "stepWeightSelection": "重み選択", | ||
| "stepGroupFilterDesc": "グループ所属でプロバイダーを絞り込み", | ||
| "stepBasicFilterDesc": "有効状態、スケジュール、プロトコル形式、モデル互換性を確認", | ||
| "stepHealthFilterDesc": "サーキットブレーカーとレー��制限の状態を確認", |
There was a problem hiding this comment.
日语文本编码损坏。
"stepHealthFilterDesc" 的值包含乱码字符 "レー��制限"，应该是 "レート制限"（レート制限 = rate limit）。这会导致日语 UI 显示不正确。
建议修复
- "stepHealthFilterDesc": "サーキットブレーカーとレー��制限の状態を確認",
+ "stepHealthFilterDesc": "サーキットブレーカーとレート制限の状態を確認",📝 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
| "stepHealthFilterDesc": "サーキットブレーカーとレー��制限の状態を確認", | |
| "stepHealthFilterDesc": "サーキットブレーカーとレート制限の状態を確認", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@messages/ja/settings/providers/schedulingTest.json` at line 25, The value for
the JSON key "stepHealthFilterDesc" contains garbled characters ("レー��制限");
replace the corrupted substring with the correct Japanese "レート制限" so the full
string reads "サーキットブレーカーとレート制限の状態を確認" (update the "stepHealthFilterDesc" entry
accordingly).
Comment on lines
+95
to
103
| for (const rule of allowedModels) { | ||
| if (rule.matchType === "exact") { | ||
| const trimmed = rule.pattern.trim(); | ||
| if (trimmed) { | ||
| unique.add(trimmed); | ||
| } | ||
| } | ||
| }); | ||
| } | ||
| return Array.from(unique); |
There was a problem hiding this comment.
仅提取 exact 规则会在“仅非精确规则”场景下产生误导性默认模型。
当配置只有 prefix/suffix/contains/regex 规则时，这里会回退到通用默认模型，常与白名单不匹配，用户直接测试可能得到误导性失败结果。建议在“有规则但无 exact”时不要自动填充通用模型（例如置空并提示手动输入）。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`[locale]/settings/providers/_components/forms/api-test-button.tsx
around lines 95 - 103, The current loop only extracts exact rules into unique
and then returns Array.from(unique), which causes a misleading fallback to a
generic default model when allowedModels contains only non-exact rules; change
the logic to detect when allowedModels is non-empty but contains zero entries
with rule.matchType === "exact" and in that case return an empty array (or
otherwise signal "no automatic model") instead of populating/using a generic
default. Update the code that builds unique from allowedModels (the loop
referencing allowedModels, rule.matchType, rule.pattern, and the return
Array.from(unique)) to first check for any exact rules and only collect/return
exact patterns when present; if none exist, return [] so the UI can require
manual input.
Comment on lines
+103
to
+107
| {redirectResult.rule && ( | ||
| <div className="flex flex-wrap items-center gap-2 text-xs"> | ||
| <Badge variant="outline" className="text-[10px]"> | ||
| {redirectResult.rule.matchType} | ||
| </Badge> |
There was a problem hiding this comment.
匹配类型标签没有本地化。
这里直接渲染 matchType，结果面板会显示 exact/prefix/... 原始枚举值。既然同目录已经有 MatchTypeBadge / getMatchTypeOptions()，建议直接复用那套翻译映射。
As per coding guidelines "All user-facing strings must use i18n (5 languages supported: zh-CN, zh-TW, en, ja, ru). Never hardcode display text".
Also applies to: 143-147
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`[locale]/settings/providers/_components/model-match-tester.tsx
around lines 103 - 107, The UI is rendering raw enum values via
redirectResult.rule.matchType (and similarly at the other location) instead of
localized labels; replace the direct rendering with the existing i18n mapping by
using getMatchTypeOptions() (or reuse MatchTypeBadge) to lookup the localized
label for matchType and render that inside the Badge (or swap to MatchTypeBadge)
so all user-facing match type text goes through the translation mapping.
Comment on lines
+141
to
+162
| const handleRun = async () => { | ||
| if (!model.trim()) return; | ||
| setIsRunning(true); | ||
| setResult(null); | ||
|
|
||
| try { | ||
| const res = await simulateSchedulingAction({ | ||
| format, | ||
| model: model.trim(), | ||
| groups, | ||
| }); | ||
| if (res.ok) { | ||
| setResult(res.data); | ||
| } else { | ||
| toast.error(res.error); | ||
| } | ||
| } catch { | ||
| toast.error("Simulation failed"); | ||
| } finally { | ||
| setIsRunning(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
运行中仍可通过回车重复触发模拟。
handleRun() 没有先判断 isRunning，而输入框在运行期间也没禁用。连按 Enter 会并发发出多次 simulateSchedulingAction()，晚返回的旧请求还能把新的结果覆盖掉。
建议修改
const handleRun = async () => {
- if (!model.trim()) return;
+ if (isRunning || !model.trim()) return;
setIsRunning(true);
setResult(null);Also applies to: 203-209
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`[locale]/settings/providers/_components/scheduling-test-dialog.tsx
around lines 141 - 162, handleRun can be invoked multiple times concurrently
because it neither checks isRunning nor disables the input, causing overlapping
simulateSchedulingAction calls and out‑of‑order results; fix by early-returning
if isRunning at the top of handleRun, disable the related input/submit while
isRunning (the JSX input/button tied to model/isRunning), and guard the other
similar handler (the one at lines ~203-209) the same way; for stronger
correctness optionally attach a request sequence token or AbortController to
each simulateSchedulingAction call and ignore stale responses before calling
setResult.
Comment on lines
+157
to
+159
| } catch { | ||
| toast.error("Simulation failed"); | ||
| } finally { |
There was a problem hiding this comment.
异常分支提示还是硬编码英文。
这里直接 toast.error("Simulation failed")，非英文 locale 会漏出未翻译文案。请补一个翻译键，和其它按钮/标签保持一致。
As per coding guidelines "All user-facing strings must use i18n (5 languages supported: zh-CN, zh-TW, en, ja, ru). Never hardcode display text".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`[locale]/settings/providers/_components/scheduling-test-dialog.tsx
around lines 157 - 159, The catch block in the SchedulingTestDialog component
currently calls toast.error("Simulation failed") with a hardcoded English
string; replace this with the app i18n key usage consistent with the rest of the
file (e.g., use the local translation helper like t(...) or
getTranslation/locale hook used elsewhere in this component) and call
toast.error(t('schedulingTest.simulationFailed')) (or similar key naming
matching other keys), then add that key and its translations to the locales
(zh-CN, zh-TW, en, ja, ru) to ensure all user-facing text is localized.
Comment on lines
+50
to
+64
| .refine( | ||
| (rules) => { | ||
| const keys = new Set<string>(); | ||
| for (const rule of rules) { | ||
| const key = `${rule.matchType}:${rule.pattern.trim().toLowerCase()}`; | ||
| if (keys.has(key)) { | ||
| return false; | ||
| } | ||
| keys.add(key); | ||
| } | ||
| return true; | ||
| }, | ||
| { | ||
| message: "Duplicate allowed model rule for matchType+pattern", | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 检查去重键是否对 pattern 做了 lower-case
rg -n -C2 'toLowerCase\(\)' src/lib/provider-allowed-model-schema.ts 'src/app/[locale]/settings/providers/_components/allowed-model-editor.tsx'
# 检查运行时匹配是否仍然使用大小写敏感的直接比较
rg -n -C3 'startsWith|endsWith|includes|new RegExp' src/lib/model-pattern-matching.tsRepository: ding113/claude-code-hub
Length of output: 1432
🏁 Script executed:
# 检查 schema 中是否有 preprocess 或其他转换逻辑
rg -n -A 5 -B 5 'PROVIDER_ALLOWED_MODEL_RULE_LIST_SCHEMA\s*=' src/lib/provider-allowed-model-schema.ts
# 检查是否有任何地方对 pattern 进行 toLowerCase 存储
rg -n 'pattern.*toLowerCase|toLowerCase.*pattern' src/
# 检查数据库 model 定义，看是否有 case-insensitive 的约束
fd -t f -e ts -e tsx | xargs rg -l 'ProviderAllowedModelRule' | head -5Repository: ding113/claude-code-hub
Length of output: 1644
🏁 Script executed:
# 查看 model-guard.ts 中的完整上下文，特别是第 49 行及其周围
sed -n '40,65p' src/app/v1/_lib/proxy/model-guard.ts
# 查看 matchPattern 的调用和实现关系
rg -n 'matchPattern|requestedModelLower' src/app/v1/_lib/proxy/model-guard.ts -A 3 -B 3
# 确认 model-pattern-matching.ts 中 matchPattern 是否真的被 model-guard.ts 使用
rg -n 'model-pattern-matching|matchPattern' src/app/v1/_lib/proxy/model-guard.tsRepository: ding113/claude-code-hub
Length of output: 979
🏁 Script executed:
# 查看 model-guard.ts 的完整背景和 allowedModels 来源
sed -n '1,70p' src/app/v1/_lib/proxy/model-guard.ts
# 查看 allowedModels 是如何被填充的
rg -n 'allowedModels\s*=' src/app/v1/_lib/proxy/model-guard.ts -B 10 -A 2
# 查看 model-pattern-matching.ts 与 ProviderAllowedModelRule 的关系
rg -n 'ProviderAllowedModelRule|matchPattern' src/lib/model-pattern-matching.ts -B 2 -A 5
# 确认 matchPattern 是否被 model-guard 使用
rg -n 'matchPattern' src/app/v1/_lib/proxy/model-guard.tsRepository: ding113/claude-code-hub
Length of output: 2818
🏁 Script executed:
# 查看是否有代码实际使用 ProviderAllowedModelRule 调用 matchPattern
rg -n 'ProviderAllowedModelRule' src/ -A 3 -B 1 | grep -A 5 -B 5 'matchPattern'
# 更直接地搜索 ProviderAllowedModelRule 的所有使用
rg -n 'ProviderAllowedModelRule' src/ --type ts --type tsx | head -20
# 查看是否有地方真正在运行时匹配这些 rules
rg -n 'allowedModels|allowedModelRules' src/ --type ts --type tsx -A 3 | grep -A 3 'matchPattern\|for.*rule'Repository: ding113/claude-code-hub
Length of output: 956
🏁 Script executed:
# 读取 provider-allowed-models.ts 的完整内容
cat -n src/lib/provider-allowed-models.ts
# 确认是否真的在某处调用 matchPattern 来匹配规则
rg -n 'matchPattern' src/lib/provider-allowed-models.ts -A 5 -B 2Repository: ding113/claude-code-hub
Length of output: 3636
🏁 Script executed:
# 查看 schema 周围是否有注释或其他上下文
sed -n '40,70p' src/lib/provider-allowed-model-schema.ts
# 查看 allowed-model-editor.tsx 的 getRuleIdentity 周围是否有注释
sed -n '25,45p' src/app/[locale]/settings/providers/_components/allowed-model-editor.tsxRepository: ding113/claude-code-hub
Length of output: 1769
去重键不应该对 regex pattern 做 toLowerCase()，实际上任何 matchType 都不应该。
运行时匹配（provider-allowed-models.ts 第 97 行）调用 matchPattern() 时使用的是原始 pattern（case-sensitive），但 schema 和前端编辑器都在去重时使用 toLowerCase() 规范化。这导致以下问题：
exact:"GPT"和"gpt"被视为重复，用户无法同时配置两条规则，但运行时这两个是不同的regex:"[A-Z]"和"[a-z]"被视为重复，但语义完全不同prefix/suffix/contains也有同样问题
修复方案：移除 toLowerCase()，在两处改为精确匹配：
src/lib/provider-allowed-model-schema.ts第 54 行的去重键改为${rule.matchType}:${rule.pattern.trim()}src/app/[locale]/settings/providers/_components/allowed-model-editor.tsx第 34 行的getRuleIdentity()改为${rule.matchType}:${rule.pattern.trim()}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/provider-allowed-model-schema.ts` around lines 50 - 64, The
deduplication key currently lowercases patterns (and effectively matchType),
causing distinct case-sensitive rules to be treated as duplicates; update the
dedupe logic in the refine callback in provider-allowed-model-schema (the
function that builds keys for rules) to use exact pattern matching by changing
the key to `${rule.matchType}:${rule.pattern.trim()}` (remove .toLowerCase()),
and make the analogous change in the UI helper getRuleIdentity() inside
allowed-model-editor.tsx (use `${rule.matchType}:${rule.pattern.trim()}`) so
both schema validation and the editor use precise, case-sensitive identity
checks.
Comment on lines
+51
to
+75
| export function normalizeAllowedModelRules(value: unknown): ProviderAllowedModelRule[] | null { | ||
| if (value == null) { | ||
| return null; | ||
| } | ||
|
|
||
| // 新格式：ProviderAllowedModelRule[] | ||
| if (isProviderAllowedModelRuleList(value)) { | ||
| return value.map(normalizeRule); | ||
| } | ||
|
|
||
| // 旧格式：string[] | ||
| if (Array.isArray(value) && value.every((item) => typeof item === "string")) { | ||
| return (value as string[]) | ||
| .map((s) => s.trim()) | ||
| .filter((s) => s.length > 0) | ||
| .map( | ||
| (pattern): ProviderAllowedModelRule => ({ | ||
| matchType: "exact", | ||
| pattern, | ||
| }) | ||
| ); | ||
| } | ||
|
|
||
| return null; | ||
| } |
There was a problem hiding this comment.
allowed_models 的写入/读取路径未统一规范化，可能造成行为不一致。
这里定义了完整规范化逻辑，但批量 patch 路径会绕过该函数直接写入 allowed_models（见 src/repository/provider.ts 与 src/lib/provider-patch-contract.ts 相关片段）。结果是同一字段在不同入口下语义不一致，可能出现空白 pattern、未 trim pattern 被落库并参与匹配。建议在 patch 的 set 分支统一调用 normalizeAllowedModelRules，并在返回 null 时直接报错而非透传。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/provider-allowed-models.ts` around lines 51 - 75, The patch set path
currently bypasses normalization causing inconsistent allowed_models values;
update the set branch in the patch handler (the code that applies updates in the
provider patch flow, referenced around provider-patch-contract / provider
repository logic) to call normalizeAllowedModelRules(value) before persisting,
and if it returns null throw/return a validation error instead of writing the
raw value; ensure any stored rules come from the normalized
ProviderAllowedModelRule[] produced by normalizeAllowedModelRules and reject
empty/invalid inputs rather than persisting them.
Comment on lines
+284
to
+293
| return ( | ||
| Array.isArray(value) && | ||
| value.every( | ||
| (rule) => | ||
| typeof rule === "object" && | ||
| rule !== null && | ||
| typeof rule.matchType === "string" && | ||
| typeof rule.pattern === "string" | ||
| ) | ||
| ); |
There was a problem hiding this comment.
allowed_models 的 set 校验过宽，可能写入无效规则。
Line 284-293 仅校验 matchType/pattern 为字符串，未约束 matchType 枚举值，也未做 regex 最低限度合法性校验。这样会让无效规则进入存储，后续匹配退化为“永不命中”或引入高复杂度风险。
建议修正（收紧结构校验）
+const ALLOWED_MODEL_MATCH_TYPES = new Set([
+ "exact",
+ "prefix",
+ "suffix",
+ "contains",
+ "regex",
+] as const);
+
+function isAllowedModelRule(value: unknown): value is ProviderAllowedModelRule {
+ if (typeof value !== "object" || value === null) return false;
+ const rule = value as { matchType?: unknown; pattern?: unknown };
+ if (typeof rule.matchType !== "string" || !ALLOWED_MODEL_MATCH_TYPES.has(rule.matchType as never)) {
+ return false;
+ }
+ if (typeof rule.pattern !== "string") return false;
+ if (rule.matchType === "regex") {
+ try {
+ new RegExp(rule.pattern);
+ } catch {
+ return false;
+ }
+ }
+ return true;
+}
@@
case "allowed_models":
- return (
- Array.isArray(value) &&
- value.every(
- (rule) =>
- typeof rule === "object" &&
- rule !== null &&
- typeof rule.matchType === "string" &&
- typeof rule.pattern === "string"
- )
- );
+ return Array.isArray(value) && value.every(isAllowedModelRule);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/provider-patch-contract.ts` around lines 284 - 293, The current
validator for allowed_models (the function that returns Array.isArray(value) &&
value.every(...)) is too permissive: restrict matchType to the expected enum
strings (e.g., "exact", "regex", "prefix", etc.) instead of any string, require
pattern to be a non-empty string, and attempt to compile pattern when matchType
=== "regex" to ensure it's a valid RegExp (catch and reject invalid regexes).
Update the validator logic in the allowed_models check to enforce those enum
values and pattern checks and return false on invalid/empty patterns or failed
RegExp compilation so malformed rules cannot be stored.
Comment on lines
+130
to
+136
| name: "group_filter", | ||
| description: "Filter by provider group tags", | ||
| inputCount, | ||
| outputCount: currentPool.length, | ||
| passed, | ||
| failed, | ||
| }); |
There was a problem hiding this comment.
模拟步骤描述存在硬编码展示文案，需走 i18n。
description 字段（如 “Filter by ...”、“Weighted random selection ...”）看起来会用于前端展示，当前为硬编码英文。建议返回稳定 key（例如 simulation.step.basic_filter.description）并在 UI 层翻译，或在调用层注入翻译文本。
As per coding guidelines: **/*.{tsx,ts}: All user-facing strings must use i18n (5 languages supported: zh-CN, zh-TW, en, ja, ru). Never hardcode display text.
Also applies to: 190-197, 231-238, 281-287, 304-310
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/scheduling-simulator.ts` around lines 130 - 136, The review flags
that user-facing hardcoded English strings in the scheduling simulator (the
description fields in the step objects, e.g., the object with name
"group_filter" and the other step objects at the noted ranges) must use i18n
keys instead; replace literal strings like "Filter by provider group tags" and
"Weighted random selection ..." with stable i18n keys (for example
simulation.step.group_filter.description,
simulation.step.basic_filter.description, etc.) and ensure the code returns the
key (or accepts an injected translator) rather than raw text so the UI layer can
perform translation; update all step objects that set description (including the
entries around lines 190–197, 231–238, 281–287, 304–310) to use keys and adjust
any consuming code to call the i18n translator.
11 tasks
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
allowedModelsnow supports 5 match types (exact/prefix/suffix/contains/regex), consistent with model redirect rules. Backward compatible with legacystring[]format via read-time normalization.ModelRedirectEditorandAllowedModelEditor. Users can test if a specific model name hits any rule.SchedulingTestDialogon providers page simulates the full provider selection decision chain. Users select entry protocol, model name, and groups to see step-by-step filtering results with priority/weight breakdowns.Problem
Provider
allowedModelspreviously supported only exact string matching (string[]), making it impossible to match model families (e.g., allclaude-sonnet-4-*variants) with a single rule. Users had to manually enumerate every model name, which was error-prone and hard to maintain. Additionally, debugging provider selection failures (why a request was routed to or skipped a provider) required reading server logs with no UI tooling.Related Issues & PRs:
matchPattern()utilitySolution
matchPattern()intomodel-pattern-matching.ts, reused by both redirect rules (feat: support provider model redirect rules #993) and whitelist rules. Supports exact, prefix, suffix, contains, and regex match types with ReDoS protection.allowedModels: string[]withProviderAllowedModelRule[]in provider types. Legacystring[]values are transparently normalized to exact-match rules at the read boundary (provider-allowed-models.ts), so no database migration is required.ModelMatchTestercomponent lets users enter a model name and instantly see which rules match, integrated into both the redirect and whitelist editors.SchedulingTestDialogruns the full provider selection pipeline client-side (group filter, format/model/schedule checks, circuit breaker, priority/weight selection) and displays step-by-step results.Changes
model-pattern-matching.tsmatchPattern()reused by redirect & whitelistprovider-allowed-models.ts,provider-allowed-model-schema.tsscheduling-simulator.tsprovider-selector.tsproviderSupportsModel()upgraded to rule-based matchingallowed-model-editor.tsx,model-match-tester.tsx,scheduling-test-dialog.tsx,match-rule-shared.tsxprovider.ts, 20+ consumer filesallowedModels: string[]->ProviderAllowedModelRule[]Breaking Changes
allowedModelsfieldaddProvider/editProviderwithallowedModels: string[]should use newProviderAllowedModelRule[]format["claude-sonnet-4"]to[{"matchType":"exact","pattern":"claude-sonnet-4"}]allowedModelsfieldallowedModelsfrom provider responses receiveProviderAllowedModelRule[]instead ofstring[]Note: Existing database records in
string[]format are automatically normalized to exact-match rules on read. No database migration is required.Testing
Automated Tests
Manual Testing
Checklist
Description enhanced by Claude AI
Greptile Summary
This PR adds advanced model whitelist matching (5 match types: exact/prefix/suffix/contains/regex), an inline match tester for redirect and whitelist rules, and a scheduling simulator dialog. The core logic — shared
matchPattern, backward-compatible normalization of legacystring[]format, and ReDoS protection viasafe-regex— is well-designed and correct.reasonGroupMismatch(produced when the group filter eliminates a provider) andreasonFormatTypeMismatch(produced when the basic filter finds a format/type mismatch). TheStepCardcomponent derives these keys by camelCasingf.reasonand prefixing\"reason\", but the files definereasonFormatMismatch(noType) and have noreasonGroupMismatchat all — sonext-intlfalls back to rendering raw key paths in the simulator results UI.Confidence Score: 3/5
Do not merge — two P1 i18n key mismatches cause broken UI in the scheduling simulator for the two most common filter scenarios (group mismatch and format type mismatch).
Two P1 defects: missing reasonGroupMismatch and reasonFormatTypeMismatch keys in all 5 language files. When providers are filtered by group or format type, next-intl renders raw key paths instead of human-readable text. The underlying simulation logic, pattern matching, and normalization are sound and well-tested.
src/app/[locale]/settings/providers/_components/scheduling-test-dialog.tsx and all 5 messages/*/settings/providers/schedulingTest.json files need reasonGroupMismatch and reasonFormatTypeMismatch keys added.
Important Files Changed
Sequence Diagram
sequenceDiagram participant UI as SchedulingTestDialog participant SA as simulateSchedulingAction participant SIM as scheduling-simulator participant PS as provider-selector UI->>SA: {format, model, groups} SA->>SA: getSession() admin check SA->>SA: findAllProviders() SA->>SIM: simulateProviderScheduling(input, providers) Note over SIM: Step 1: Group Filter SIM->>PS: checkProviderGroupMatch(groupTag, userGroups) PS-->>SIM: pass/fail Note right of SIM: reason=group_mismatch Note right of SIM: ❌ reasonGroupMismatch key missing Note over SIM: Step 2: Basic Filter SIM->>PS: checkFormatProviderTypeCompatibility(format, type) PS-->>SIM: pass/fail Note right of SIM: reason=format_type_mismatch Note right of SIM: ❌ reasonFormatTypeMismatch key missing SIM->>SIM: modelMatchesAllowedRules(model, rules) Note over SIM: Step 3: Health Filter SIM->>SIM: isCircuitOpen(id) x N (parallel) Note over SIM: Steps 4-5: Priority + Weight SIM->>SIM: resolveLocalEffectivePriority() SIM->>SIM: compute weights and probabilities SIM-->>SA: SchedulingSimulationResult SA-->>UI: ok=true data UI->>UI: render StepCards Note over UI: f.reason camelCase → t(reasonXxx) Note over UI: ❌ missing keys render as raw pathsComments Outside Diff (4)
src/app/[locale]/settings/providers/_components/scheduling-test-dialog.tsx, line 104-107 (link)The
StepCardcomponent derives a translation key by camelCasingf.reasonand prefixing with"reason". Two reason codes emitted byscheduling-simulator.tsmap to keys that do not exist in any of the 5 language files:"group_mismatch"(emitted atscheduling-simulator.tsline 115) →reasonGroupMismatch— absent from all language files"format_type_mismatch"(emitted atscheduling-simulator.tsline 167) →reasonFormatTypeMismatch— absent from all language files; the files definereasonFormatMismatch(noTypesegment)Whenever these two filter steps eliminate a provider,
next-intlwill render the raw key path instead of human-readable text. Fix: add"reasonGroupMismatch"and"reasonFormatTypeMismatch"to all 5messages/*/settings/providers/schedulingTest.jsonfiles.Prompt To Fix With AI
src/app/[locale]/settings/providers/_components/scheduling-test-dialog.tsx, line 158 (link)CLAUDE.md mandates that all user-facing strings use i18n. The
thook is already in scope — add a"simulationFailed"key to all 5 language files and replace the literal.Prompt To Fix With AI
src/app/[locale]/settings/providers/_components/scheduling-test-dialog.tsx, line 53 (link)step.descriptionis a hardcoded English string from the server side (e.g."Filter by provider group tags"). All 5 language files already define matchingstep*Desckeys (stepGroupFilterDesc,stepBasicFilterDesc, etc.) that are never referenced by the component. Replace the static field with a translated lookup using the same camelCase pattern already applied to step names:Prompt To Fix With AI
src/app/[locale]/settings/providers/_components/scheduling-test-dialog.tsx, line 32 (link)gemini-cliformat missing from simulatorcheckFormatProviderTypeCompatibilityinprovider-selector.tsexplicitly handles"gemini-cli"as a distinct format that maps toproviderType === "gemini-cli"providers. Users withgemini-cliproviders cannot simulate routing for that format in the dialog.Also add a corresponding
<SelectItem value="gemini-cli">entry in the<SelectContent>block.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "feat: advanced model whitelist matching,..." | Re-trigger Greptile