fix: /v1/models 返回 modelRedirects key,修复模型调度误判 (#786)#787
fix: /v1/models 返回 modelRedirects key,修复模型调度误判 (#786)#787tesgth032 wants to merge 3 commits intoding113:devfrom
Conversation
Summary of ChangesHello @tesgth032, 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! 此拉取请求旨在优化模型调度和模型列表暴露机制,解决了一个关键问题:当使用模型重定向时,调度器可能错误地将提供商标记为不支持模型。通过引入统一的模型支持判断逻辑,并确保 Highlights
Changelog
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
|
📝 WalkthroughWalkthrough引入并使用新的工具函数 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
这个 Pull Request 旨在修复模型调度中的一个错误,并增强了 /v1/models 端点的功能。主要变更包括:
- 修复调度逻辑:当 Claude provider 使用
modelRedirects时,即使原始模型名不在allowedModels中,调度器现在也能正确识别其为支持的模型。 - 增强模型列表:
/v1/models端点现在会返回modelRedirects中定义的模型别名,使得客户端能看到所有可用的入口模型。 - 代码重构:将模型支持的判断逻辑抽离到独立的
provider-supports-model.ts文件中,提高了代码的模块化和可维护性。
整体来看,这些变更是非常积极的,解决了 issue #786 中描述的问题,并且附带了相应的单元测试来验证修复。代码质量很高。
我只对新抽离的 providerSupportsModel 函数提出了一点重构建议,旨在进一步提升其可读性。除此之外,没有发现其他问题。
| export function providerSupportsModel(provider: Provider, requestedModel: string): boolean { | ||
| const isClaudeModel = requestedModel.startsWith("claude-"); | ||
| const isClaudeProvider = | ||
| provider.providerType === "claude" || provider.providerType === "claude-auth"; | ||
|
|
||
| // Case 1: Claude 模型请求 | ||
| if (isClaudeModel) { | ||
| // 1a. Anthropic 提供商 | ||
| if (isClaudeProvider) { | ||
| // 未设置 allowedModels 或为空数组:允许所有 claude 模型 | ||
| if (!provider.allowedModels || provider.allowedModels.length === 0) { | ||
| return true; | ||
| } | ||
|
|
||
| // Fix #786:当存在 modelRedirects 映射时,映射前模型名(key)也应视为可请求模型 | ||
| return ( | ||
| provider.allowedModels.includes(requestedModel) || | ||
| !!provider.modelRedirects?.[requestedModel] | ||
| ); | ||
| } | ||
|
|
||
| // 1b. 非 Anthropic 提供商不支持 Claude 模型调度 | ||
| return false; | ||
| } | ||
|
|
||
| // Case 2: 非 Claude 模型请求(gpt-*, gemini-*, 或其他任意模型) | ||
| // 2a. 优先检查显式声明(支持跨类型别名) | ||
| const explicitlyDeclared = !!( | ||
| provider.allowedModels?.includes(requestedModel) || provider.modelRedirects?.[requestedModel] | ||
| ); | ||
|
|
||
| if (explicitlyDeclared) { | ||
| return true; // 显式声明优先级最高,允许跨类型别名 | ||
| } | ||
|
|
||
| // 2b. Anthropic 提供商不支持非声明的非 Claude 模型 | ||
| // 保护机制:防止将非 Claude 模型误路由到 Anthropic API | ||
| if (isClaudeProvider) { | ||
| return false; | ||
| } | ||
|
|
||
| // 2c. 非 Anthropic 提供商(codex, gemini, gemini-cli, openai-compatible) | ||
| // 未设置 allowedModels 或为空数组:接受任意模型(由上游提供商判断) | ||
| if (!provider.allowedModels || provider.allowedModels.length === 0) { | ||
| return true; | ||
| } | ||
|
|
||
| // 不在声明列表中且无重定向配置(前面已检查 explicitlyDeclared) | ||
| return false; | ||
| } |
There was a problem hiding this comment.
当前函数的逻辑有些复杂,分支较多,可以重构以提高可读性。我们可以将逻辑分为两个主要部分:
- 显式声明: 如果模型在
allowedModels或modelRedirects中明确列出,则始终支持。这具有最高优先级,并允许跨提供商类型的别名。 - 隐式规则: 对于未显式声明的模型,我们根据提供商和模型的类型应用默认规则。
这样的结构可以使逻辑更清晰,更容易理解和维护。
export function providerSupportsModel(provider: Provider, requestedModel: string): boolean {
const isClaudeModel = requestedModel.startsWith("claude-");
const isClaudeProvider =
provider.providerType === "claude" || provider.providerType === "claude-auth";
// 1. 显式声明的模型(在 allowedModels 或 modelRedirects 中)始终被支持。
// 这是最高优先级,允许跨类型代理。
const isExplicitlyDeclared =
provider.allowedModels?.includes(requestedModel) || !!provider.modelRedirects?.[requestedModel];
if (isExplicitlyDeclared) {
return true;
}
// 2. 对于未显式声明的模型,根据 provider 和 model 类型进行隐式规则判断。
// 2a. Claude 模型只能由 Claude provider 支持。
if (isClaudeModel) {
// 如果 Claude provider 没有设置 allowedModels,则默认支持所有 Claude 模型。
if (isClaudeProvider) {
return !provider.allowedModels || provider.allowedModels.length === 0;
}
// 非 Claude provider 不支持 Claude 模型。
return false;
}
// 2b. 非 Claude 模型。
// Claude provider 不支持未声明的非 Claude 模型。
if (isClaudeProvider) {
return false;
}
// 非 Claude provider,如果未设置 allowedModels,则默认支持所有非 Claude 模型。
return !provider.allowedModels || provider.allowedModels.length === 0;
}There was a problem hiding this comment.
Code Review Summary
This PR properly fixes issue #786 by treating modelRedirects keys as supported models when the target model is in allowedModels. The extraction of providerSupportsModel to a dedicated module improves code reusability and the /v1/models endpoint now correctly exposes redirect source models. Implementation is clean with appropriate test coverage.
PR Size: S
- Lines changed: 370 (299 additions, 71 deletions)
- Files changed: 6
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 Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Proper logging and fallback behavior
- Type safety - Proper TypeScript usage
- Documentation accuracy - Comments match implementation
- Test coverage - Adequate (4 new tests covering fix and edge cases)
- Code clarity - Good (function extraction improves maintainability)
Automated review by Claude AI
Additional Comments (1)
The Prompt To Fix With AIThis is a comment left during a code review.
Path: src/app/v1/_lib/proxy/provider-selector.ts
Line: 96:115
Comment:
**Stale JSDoc comment left behind**
The `providerSupportsModel` function body was extracted to its own module, but its JSDoc comment block (lines 96–114) was left behind. This creates two consecutive JSDoc blocks — the orphaned one followed by the `checkFormatProviderTypeCompatibility` JSDoc — which is confusing and misleading.
```suggestion
/**
* 根据原始请求格式限制可选供应商类型
```
How can I resolve this? If you propose a fix, please make it concise. |
Summary
Fixes model scheduling misjudgment when provider uses
modelRedirectswithallowedModelswhitelist, and exposesmodelRedirectskeys in/v1/modelsendpoint.Fixes #786
Problem
When a Claude provider is configured with:
allowedModels: ["claude-opus-4-6-think"](only redirect target)modelRedirects: { "claude-opus-4-6": "claude-opus-4-6-think" }(redirect mapping)The provider selection logic incorrectly filters out this provider when users request
claude-opus-4-6, because:claude-opus-4-6is inallowedModels- it's notmodelRedirectsmakes this model requestable as an "entry alias"Additionally,
/v1/modelsendpoint doesn't expose the redirect source model names (keys), causing clients/frontend to incorrectly judge entry models as unavailable.Solution
src/app/v1/_lib/proxy/provider-supports-model.tsto centralize the logicmodelRedirects[requestedModel]exists, treat it as supported even if not inallowedModels/v1/models: MergemodelRedirectskeys into provider model list (deduplicated, using same filtering rules to avoid exposingclaude-*on non-Claude providers)Changes
Core Changes
src/app/v1/_lib/proxy/provider-supports-model.ts(+60 lines): New module with unifiedproviderSupportsModel()functionsrc/app/v1/_lib/proxy/provider-selector.ts(-52/+1 lines): Extract model support logic to new modulesrc/app/v1/_lib/models/available-models.ts(+47/-18 lines): IncludemodelRedirectskeys in model listSupporting Changes
biome.json: Update schema to 2.3.15 (match current CLI version)Tests
tests/unit/proxy/provider-selector-model-redirect.test.ts(+70 lines): Test redirect key as supported with whitelisttests/unit/models/available-models-include-redirect-keys.test.ts(+120 lines): Test/v1/modelsincludes redirect keysTesting
bun run build- Production buildbun run lint/bun run lint:fix- Biome checkbun run typecheck- TypeScript checkbun run test- Vitest testsRelated
Background (中文)
issue #786:当 Claude provider 配置了
modelRedirects(例如把客户端请求的claude-opus-4-6重定向到上游实际可用的claude-opus-4-6-think),但该 provider 的allowedModels只包含重定向后的模型时,调度筛选会误判为"不支持模型",从而把该 provider 过滤掉。另外,
/v1/models目前不会暴露modelRedirects的 key(映射前模型名)。这会导致依赖模型列表的客户端/前端误判入口模型不可用。变更 (中文)
src/app/v1/_lib/proxy/provider-supports-model.tsmodelRedirects[requestedModel],即使requestedModel不在allowedModels中也视为支持(入口别名语义)/v1/models:在 provider 模型列表中合并modelRedirects的 key(去重,并复用同一规则过滤,避免在非 Claude provider 暴露claude-*key)biome.json的$schema到 2.3.15(与当前 CLI 版本匹配)测试 (中文)
bun run buildbun run lint/bun run lint:fixbun run typecheckbun run testDescription enhanced by Claude AI
Greptile Overview
Greptile Summary
Fixes model scheduling misjudgment when providers use
modelRedirectswithallowedModelswhitelist, and exposes redirect keys in/v1/modelsendpoint.Key Changes:
provider-supports-model.tsmodule for better code reusemodelRedirectskeys as supported entry models, even when only redirect targets are inallowedModels/v1/modelsto include redirect source model names (keys), filtered by sameproviderSupportsModellogic to maintain consistencyImpact:
Resolves issue #786 where Claude providers configured with
modelRedirects: { "claude-opus-4-6": "claude-opus-4-6-think" }andallowedModels: ["claude-opus-4-6-think"]were incorrectly filtered out when users requestedclaude-opus-4-6. Both the scheduling logic and model listing endpoint now correctly recognize redirect keys as valid entry points.Confidence Score: 5/5
Important Files Changed
Flowchart
flowchart TD A[Client requests claude-opus-4-6] --> B[Provider Selection] B --> C[providerSupportsModel check] C --> D{Check explicitlyDeclared} D -->|modelRedirects key exists| E[Provider matches] D -->|allowedModels includes requested| E D -->|Not explicitly declared| F{Check implicit rules} F -->|Claude model on Claude provider| G{allowedModels set?} G -->|No or empty| E G -->|Yes but not in list| H[Provider filtered out] I[v1 models endpoint] --> J[fetchModelsFromProvider] J --> K[Get base models from allowedModels or upstream] K --> L{Has modelRedirects?} L -->|Yes| M[Add redirect keys to model list] M --> N{providerSupportsModel for each key} N -->|Supported| O[Include redirect key in response] N -->|Not supported| P[Skip redirect key] L -->|No| Q[Return base models] O --> Q P --> QLast reviewed commit: d1d4bc4