Conversation
- Add "models-list" format for /v1/models, /v1/responses/models, /v1/chat/completions/models, /v1beta/models endpoints - Models-list format is provider-agnostic (no provider type restriction) - Add path rewriting: /v1/responses/models → /v1/models - Update provider selector to handle format-based filtering for requests without model parameter
Summary of ChangesHello @NieiR, 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! This pull request significantly enhances the API's capability to handle model listing requests by introducing a dedicated 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
|
github-actions
bot
added
size/S
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
This PR adds support for the models-list format across multiple endpoints (/v1/models, /v1/responses/models, /v1/chat/completions/models, /v1beta/models), implementing format-agnostic provider selection and path rewriting for model list requests.
PR Size: S
- Lines changed: 89 additions, 4 deletions
- Files changed: 4
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 | 1 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
High Priority Issues (Should Fix)
None
Medium Priority Issues (Consider Fixing)
[TEST-MISSING-CRITICAL] Missing tests for new models-list format functionality
The PR adds significant new functionality without accompanying tests. While existing tests exercise related code paths, the new models-list format detection, path rewriting, and provider filtering logic lacks dedicated test coverage.
Why this is a problem:
- New format detection in
format-mapper.ts:70-76is untested - Path rewriting logic in
forwarder.ts:1089-1102is untested - Provider filtering changes in
provider-selector.ts:197-200andprovider-selector.ts:723-728are untested - The
STANDARD_ENDPOINTSarray changes (adding 4 new endpoints) have no regression tests
Confidence Score: 85
Suggested fix:
Add tests covering:
detectFormatByEndpoint()correctly identifies all 4 models endpointsisModelsListFormat()helper function behavior- Path rewriting for
/v1/responses/modelsand/v1/chat/completions/models - Provider selection allows all provider types for
models-listformat - Integration test verifying the complete flow
Example test structure:
describe("models-list format", () => {
it("should detect /v1/models endpoint", () => {
expect(detectFormatByEndpoint("/v1/models")).toBe("models-list");
});
it("should detect /v1/responses/models and rewrite path", () => {
// Test path rewriting logic
});
it("should allow all provider types for models-list format", () => {
// Test provider filtering logic
});
});Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Needs tests
- Code clarity - Good
Detailed Analysis
Perspective 1: Comment Analyzer ✓ Clean
- Comments accurately describe the new
models-listformat behavior - No noise, outdated, or inaccurate comments detected
- Documentation properly explains the provider-agnostic nature
Perspective 2: Test Analyzer ⚠ Missing critical tests
- No tests found for the new
models-listformat detection - Path rewriting logic lacks test coverage
- Provider filtering changes are untested
- No regression tests for STANDARD_ENDPOINTS additions
Perspective 3: Silent Failure Hunter ✓ Clean
- No new try/catch blocks added
- No error handling concerns
- Path rewriting uses existing logging patterns
Perspective 4: Type Design Auditor ✓ Clean
ClientFormattype correctly extended with"models-list"- Type discrimination in
checkFormatProviderTypeCompatibility()is sound - No
anytype usage - Exhaustive type checking maintained
Perspective 5: General Code Reviewer ✓ Clean
- No logic bugs identified
- No security vulnerabilities
- Follows existing code patterns (Biome style, path alias usage)
- Provider filtering logic correctly implements format-based selection
- Path rewriting is straightforward and maintainable
Perspective 6: Code Simplifier ✓ Clean
- Code is clear and readable
- No unnecessary complexity
- Variable names are descriptive (
MODELS_PATH_REWRITES,rewrittenPath) - Logic is straightforward
Technical Notes
Architecture Compliance:
- Follows the established proxy request pipeline
- Consistent with format mapper pattern
- Provider selector changes align with existing filtering logic
Code Quality:
- Proper use of constants (
MODELS_PATH_REWRITES) - Debug logging added for path rewriting (line 1094-1098)
- Comments explain the "why" (provider-agnostic design)
Integration Points:
detectFormatByEndpoint()is called fromproxy-handler.ts:22- Path rewriting happens before
buildProxyUrl()call - Provider filtering integrates with existing
checkFormatProviderTypeCompatibility()
Automated review by Claude AI
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for a provider-agnostic models-list endpoint, which is a great enhancement for client compatibility. The changes include adding a new format type, endpoint detection, path rewriting for aliased model endpoints, and updating the provider selection logic to handle these new model-less requests. The implementation is solid and covers the necessary areas. I've provided one suggestion to improve maintainability by consolidating related regular expressions. Additionally, the CHANGELOG.md file appears to contain entries for a different release and is missing a description of the changes in this PR, which should be corrected.
| ### 新增 | ||
|
|
||
| - Session 详情页新增请求/响应头日志展示，支持 Tab 切换查看 (#469) | ||
| - 排行榜新增排序和供应商类型筛选功能 (#448) [@YewFence](https://github.com/YewFence) | ||
| - 虚拟化表格组件 (use-virtualizer hook) 用于大数据量列表性能优化 (#467) [@NightYuYyy](https://github.com/NightYuYyy) | ||
| - 新增 `FETCH_CONNECT_TIMEOUT` 环境变量，统一配置 Undici 连接超时（默认 30 秒）(#479, #480) | ||
|
|
||
| ### 优化 | ||
|
|
||
| - 供应商管理页面 UX 改进，优化交互体验 (#446) [@miraserver](https://github.com/miraserver) | ||
| - 用户筛选与排序体验优化，移除使用日志用户筛选限制 (#462, #449) [@NightYu](https://github.com/NightYuYyy) | ||
| - 缓存 tooltip 显示改进，当 5m/1h breakdown 不可用时提供友好提示 (#445) [@Hwwwww](https://github.com/Hwwwww-dev) | ||
| - TagInput 组件和虚拟化表格稳定性增强 (#467) [@NightYuYyy](https://github.com/NightYuYyy) | ||
| - SSE 解析工具增强，添加错误处理和测试 (#469) | ||
| - Session 消息客户端 SSE 性能和 matchMedia 回退优化 (#469) | ||
|
|
||
| ### 修复 | ||
|
|
||
| - 修复计费模型来源配置不生效问题 (#464) | ||
| - Codex instructions 一律透传，移除缓存与策略 (#475) | ||
| - 修复 Session 详情页中的 tool_use_id 验证问题 (#473, #472) | ||
| - 修复日志表格中供应商名称溢出问题 (#478) [@YangQing-Lin](https://github.com/YangQing-Lin) | ||
| - 请求过滤器 header 修改追踪修复，确保在 Session 详情中正确显示 (#465) | ||
| - 数据导入组件优化，移除重复描述文本 (#458) [@Abner](https://github.com) | ||
|
|
||
| ### 其他 | ||
|
|
||
| - 新增多项单元测试：undici 超时、proxy forwarder、session 等 (#469, #479) | ||
| - 移除 codex-instructions-cache.ts 模块，简化代码结构 (#475) |
There was a problem hiding this comment.
The changelog entries added in this pull request do not seem to correspond to the feature being introduced (feat: add models-list endpoint support). The changelog should accurately reflect the changes made in this PR. Please update this section to describe the new models-list endpoint support.
| { pattern: /^\/v1\/models$/i, format: "models-list" }, | ||
| { pattern: /^\/v1\/responses\/models$/i, format: "models-list" }, | ||
| { pattern: /^\/v1\/chat\/completions\/models$/i, format: "models-list" }, |
There was a problem hiding this comment.
For better maintainability and conciseness, the three regular expressions for the /v1 model list endpoints can be combined into a single one using a non-capturing group.
| { pattern: /^\/v1\/models$/i, format: "models-list" }, | |
| { pattern: /^\/v1\/responses\/models$/i, format: "models-list" }, | |
| { pattern: /^\/v1\/chat\/completions\/models$/i, format: "models-list" }, | |
| { pattern: /^\/v1(?:\/(?:responses|chat\/completions))?\/models$/i, format: "models-list" }, |
|
Closing: need to investigate Gemini /v1beta/models issue |
github-project-automation
bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
1 participant
Summary
Add support for models-list endpoints (
/v1/models,/v1/responses/models,/v1/chat/completions/models,/v1beta/models) with provider-agnostic format handling and path rewriting.Problem
Previously, the proxy system did not properly handle client requests to retrieve available models. Clients often request models by appending
/modelsto their configured base URL (e.g.,/v1/responses/models,/v1/chat/completions/models), but the proxy lacked:/v1/responses/modelsor/v1/chat/completions/modelswould fail because these paths needed to be rewritten to/v1/modelsfor upstream providersSolution
This PR introduces a new
models-listclient format that:/v1/modelsKey Implementation Details
New Client Format (
format-mapper.ts):models-listtoClientFormattype/v1/models,/v1/responses/models,/v1/chat/completions/models,/v1beta/modelsopenai-compatibletransformer formatisModelsListFormat()helper for type checkingProvider Type Compatibility (
provider-selector.ts):checkFormatProviderTypeCompatibility()to allow all provider types formodels-listformatPath Rewriting (
forwarder.ts):/v1/responses/models→/v1/models/v1/chat/completions/models→/v1/models/v1/modelsand/v1beta/modelspathsRelated Work
Changes
Core Changes
src/app/v1/_lib/proxy/format-mapper.ts(+23/-1):models-listtoClientFormattypeisModelsListFormat()helper functionopenai-compatibletransformersrc/app/v1/_lib/proxy/provider-selector.ts(+14/-2):checkFormatProviderTypeCompatibility()to returntruefor all provider types when format ismodels-listsrc/app/v1/_lib/proxy/forwarder.ts(+23/-1):/v1/responses/modelsand/v1/chat/completions/modelsMODELS_PATH_REWRITESmapping constantSupporting Changes
CHANGELOG.md(+29/-0): Added v0.3.38 release notesBreaking Changes
None. This is a new feature that adds support for previously unsupported endpoints.
Testing
Manual Testing
/v1/modelsendpoint returns model list/v1/responses/modelsrewrites to/v1/models/v1/chat/completions/modelsendpoint/v1beta/modelsendpoint/v1/responsescan successfully request models at/v1/responses/modelsChecklist
Description enhanced by Claude AI
Greptile Summary
Added comprehensive support for model listing endpoints across multiple API formats by introducing a new
models-listformat type that is provider-agnostic. The implementation includes:ClientFormattypemodels-listthat allows requests to work with any provider type/v1/models,/v1/responses/models,/v1/chat/completions/models, and/v1beta/models/v1/modelsendpointThis change enables clients with different base URL configurations (like
/v1/responsesor/v1/chat/completions) to successfully retrieve model lists by rewriting their paths to the standard endpoint.Confidence Score: 5/5
Important Files Changed
isModelsListFormat(), maps to openai-compatible transformer, returns provider-agnostic format/v1/responses/modelsand/v1/chat/completions/modelsto/v1/modelsSequence Diagram
sequenceDiagram participant Client participant FormatMapper participant ProviderSelector participant Forwarder participant Provider Client->>FormatMapper: Request /v1/responses/models FormatMapper->>FormatMapper: detectFormatByEndpoint() FormatMapper-->>FormatMapper: Returns "models-list" FormatMapper->>ProviderSelector: Check format compatibility ProviderSelector->>ProviderSelector: isModelsListFormat(format) ProviderSelector-->>ProviderSelector: Returns true (all providers allowed) ProviderSelector->>ProviderSelector: Filter providers by format ProviderSelector-->>ProviderSelector: All provider types pass ProviderSelector->>Forwarder: Selected provider Forwarder->>Forwarder: Check if path needs rewriting Forwarder-->>Forwarder: /v1/responses/models → /v1/models Forwarder->>Provider: GET /v1/models Provider-->>Forwarder: Model list response Forwarder-->>Client: Return model list