fix(config): now you can use the same model name between different providers - #14
Conversation
审阅者指南重构模型配置,将「模型→提供方」改为「提供方→模型数组」的映射,使同一个模型名称可以在多个提供方下复用;引入了用于处理该结构的辅助工具;并在 Notebook UI 和 HTTP API 中贯穿一个可选的提供方提示,以便在同一模型由多个提供方提供时,凭据查找可以正确区分。 HTTP 聊天模型 / 提供方解析的时序图sequenceDiagram
participant Client
participant HttpChat as handleChat
participant VSCodeConfig as VSCode_config
participant Utils as getModelCredentials
participant Providers as providers+models
Client->>HttpChat: POST /chat {prompt, model?, provider?}
HttpChat->>VSCodeConfig: get('defaultModel')
VSCodeConfig-->>HttpChat: defaultModel
HttpChat->>HttpChat: determine effectiveModel
HttpChat->>HttpChat: determine effectiveProvider
HttpChat->>Utils: getModelCredentials(effectiveModel, effectiveProvider)
Utils->>VSCodeConfig: get('providers'), get('models')
VSCodeConfig-->>Utils: providers, providerToModels
Utils->>Providers: find provider that lists model
Providers-->>Utils: matching provider (first or by hint)
Utils-->>HttpChat: {apiKey, baseUrl}
HttpChat-->>Client: stream or respond using credentials
文件级变更
可能关联的问题
提示和命令与 Sourcery 交互
自定义体验访问你的 控制面板 可以:
获取帮助Original review guide in EnglishReviewer's GuideRefactors model configuration to map providers to arrays of models so the same model name can be used across multiple providers, introduces helper utilities to work with this structure, and threads an optional provider hint through notebook UI and HTTP APIs so credential lookup can disambiguate models shared by multiple providers. Sequence diagram for HTTP chat model/provider resolutionsequenceDiagram
participant Client
participant HttpChat as handleChat
participant VSCodeConfig as VSCode_config
participant Utils as getModelCredentials
participant Providers as providers+models
Client->>HttpChat: POST /chat {prompt, model?, provider?}
HttpChat->>VSCodeConfig: get('defaultModel')
VSCodeConfig-->>HttpChat: defaultModel
HttpChat->>HttpChat: determine effectiveModel
HttpChat->>HttpChat: determine effectiveProvider
HttpChat->>Utils: getModelCredentials(effectiveModel, effectiveProvider)
Utils->>VSCodeConfig: get('providers'), get('models')
VSCodeConfig-->>Utils: providers, providerToModels
Utils->>Providers: find provider that lists model
Providers-->>Utils: matching provider (first or by hint)
Utils-->>HttpChat: {apiKey, baseUrl}
HttpChat-->>Client: stream or respond using credentials
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - 我发现了 3 个问题,并留下了一些高层次的反馈:
- 在
getModelCredentials中,你对 provider 名称做了归一化处理,但没有对传入的modelName或配置的模型标识符做同样的处理,这意味着设置中的前后空白可能会导致难以排查的查找失败;建议在比较前对modelName和模型 ID 都执行 trim 操作。 package.json中的mutsumi.models贡献 schema 不再约束对象的结构(例如,每个 provider 对应字符串数组),因此 VS Code 将无法帮助用户捕捉配置错误;值得更新 JSON schema,使其反映新的 provider → models 数组结构。
给 AI 代理的提示
请根据本次代码评审中的评论进行修改:
## 总体评论
- 在 `getModelCredentials` 中,你对 provider 名称做了归一化处理,但没有对传入的 `modelName` 或配置的模型标识符做同样的处理,这意味着设置中的前后空白可能会导致难以排查的查找失败;建议在比较前对 `modelName` 和模型 ID 都执行 trim 操作。
- `package.json` 中的 `mutsumi.models` 贡献 schema 不再约束对象的结构(例如,每个 provider 对应字符串数组),因此 VS Code 将无法帮助用户捕捉配置错误;值得更新 JSON schema,使其反映新的 provider → models 数组结构。
## 单条评论
### Comment 1
<location path="src/httpServer/model.ts" line_range="19-28" />
<code_context>
}
- const { model } = req.body ?? {};
+ const { model, provider } = req.body ?? {};
if (typeof model !== 'string' || !model.trim()) {
res.status(400).json({ status: 'error', content: 'Missing or invalid model parameter.' });
</code_context>
<issue_to_address>
**suggestion (bug_risk):** 在持久化之前校验传入的 `provider` 是否确实支持所选的 `model`。
来自请求体的 `provider` 被存储在 notebook 元数据中,但没有检查它是否实际支持 `model`。之后,`getModelCredentials(model, provider)` 会优先使用这个提示,如果该 provider 没有列出该 model(即便还有其他 provider 支持该 model),就可能失败。
请根据模型配置验证 `provider`(例如,确保 `modelsConfig[provider]` 中包含 `model`),对无效的组合返回 400,或者在不匹配时跳过持久化 `provider`。
</issue_to_address>
### Comment 2
<location path="src/httpServer/chat.ts" line_range="27-30" />
<code_context>
const uuid = Array.isArray(uuidParam) ? uuidParam[0] : uuidParam;
const body = req.body ?? {};
- const { prompt, model, stream } = body;
+ const { prompt, model, provider, stream } = body;
const hasReasoningEffort = Object.prototype.hasOwnProperty.call(body, 'reasoning_effort');
const bodyReasoningEffort = body.reasoning_effort;
</code_context>
<issue_to_address>
**suggestion (bug_risk):** 让 chat 中对 `provider` 的处理与 model/provider 校验策略保持一致。
chat 处理函数现在接受 `provider`,并将其传给 `getModelCredentials(effectiveModel, effectiveProvider)`,同时将其持久化到元数据,但没有验证该 `provider` 是否对所选 `model` 有效。如果客户端传入了错误的 provider,就可能导致隐蔽的失败。请:(a)验证 `(model, provider)` 组合是否在模型配置中有效,或(b)忽略无效的 `provider` 并回退到自动 provider 解析,以保持与 HTTP API / notebook 命令一致的行为,避免令人意外的失败。
</issue_to_address>
### Comment 3
<location path="docs/multi-provider-refactor.md" line_range="132" />
<code_context>
-Returns `Record<string, string>` (unchanged interface), but:
+Returns `Record<string, string[]>` (provider name → model identifiers array):
- If user-configured models is empty, return `DEFAULT_MODELS`
-- Values are now provider names (not display labels)
+- Keys are provider names, values are arrays of model identifiers
</code_context>
<issue_to_address>
**nitpick (typo):** 复数一致性:应使用 "models are" 而不是 "models is"。
可以改为:"If user-configured models are empty, return `DEFAULT_MODELS`"。或者:"If the user-configured models configuration is empty"。
```suggestion
- If user-configured models are empty, return `DEFAULT_MODELS`
```
</issue_to_address>帮我变得更有用!请在每条评论上点击 👍 或 👎,我会根据反馈改进后续评审。
Original comment in English
Hey - I've found 3 issues, and left some high level feedback:
- In
getModelCredentials, you normalize provider names but not the incomingmodelNameor the configured model identifiers, which means trailing/leading whitespace in the settings can cause hard-to-debug lookup failures; consider trimmingmodelNameand comparing against trimmed model IDs. - The
mutsumi.modelscontribution schema inpackage.jsonno longer constrains the shape of the object (e.g., arrays of strings per provider), so VS Code won’t help users catch misconfigurations; it’s worth updating the JSON schema to reflect the new provider→models array structure.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `getModelCredentials`, you normalize provider names but not the incoming `modelName` or the configured model identifiers, which means trailing/leading whitespace in the settings can cause hard-to-debug lookup failures; consider trimming `modelName` and comparing against trimmed model IDs.
- The `mutsumi.models` contribution schema in `package.json` no longer constrains the shape of the object (e.g., arrays of strings per provider), so VS Code won’t help users catch misconfigurations; it’s worth updating the JSON schema to reflect the new provider→models array structure.
## Individual Comments
### Comment 1
<location path="src/httpServer/model.ts" line_range="19-28" />
<code_context>
}
- const { model } = req.body ?? {};
+ const { model, provider } = req.body ?? {};
if (typeof model !== 'string' || !model.trim()) {
res.status(400).json({ status: 'error', content: 'Missing or invalid model parameter.' });
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Validate that the provided `provider` actually serves the chosen `model` before persisting.
`provider` from the request body is stored in notebook metadata without checking that it actually supports `model`. Later, `getModelCredentials(model, provider)` will favor this hint and may fail if the provider does not list that model, even though another provider does.
Please validate `provider` against the models config (e.g., ensure `modelsConfig[provider]` includes `model`) and either return a 400 for invalid combinations or skip persisting `provider` when it doesn’t match.
</issue_to_address>
### Comment 2
<location path="src/httpServer/chat.ts" line_range="27-30" />
<code_context>
const uuid = Array.isArray(uuidParam) ? uuidParam[0] : uuidParam;
const body = req.body ?? {};
- const { prompt, model, stream } = body;
+ const { prompt, model, provider, stream } = body;
const hasReasoningEffort = Object.prototype.hasOwnProperty.call(body, 'reasoning_effort');
const bodyReasoningEffort = body.reasoning_effort;
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Align `provider` handling in chat with model/provider validation strategy.
The chat handler now accepts `provider` and passes it to `getModelCredentials(effectiveModel, effectiveProvider)`, and persists it in metadata, but it doesn’t verify that the `provider` is valid for the chosen `model`. This can cause subtle failures if a client sends an incorrect provider. Please either (a) validate the `(model, provider)` pair against the models config, or (b) ignore an invalid `provider` and fall back to automatic provider resolution, to keep behavior consistent with the HTTP API/notebook command and avoid surprising failures.
</issue_to_address>
### Comment 3
<location path="docs/multi-provider-refactor.md" line_range="132" />
<code_context>
-Returns `Record<string, string>` (unchanged interface), but:
+Returns `Record<string, string[]>` (provider name → model identifiers array):
- If user-configured models is empty, return `DEFAULT_MODELS`
-- Values are now provider names (not display labels)
+- Keys are provider names, values are arrays of model identifiers
</code_context>
<issue_to_address>
**nitpick (typo):** Plural agreement: use "models are" instead of "models is"
Consider: "If user-configured models are empty, return `DEFAULT_MODELS`". Alternatively: "If the user-configured models configuration is empty".
```suggestion
- If user-configured models are empty, return `DEFAULT_MODELS`
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| Returns `Record<string, string>` (unchanged interface), but: | ||
| Returns `Record<string, string[]>` (provider name → model identifiers array): | ||
| - If user-configured models is empty, return `DEFAULT_MODELS` |
There was a problem hiding this comment.
nitpick (typo): 复数一致性:应使用 "models are" 而不是 "models is"。
可以改为:"If user-configured models are empty, return DEFAULT_MODELS"。或者:"If the user-configured models configuration is empty"。
| - If user-configured models is empty, return `DEFAULT_MODELS` | |
| - If user-configured models are empty, return `DEFAULT_MODELS` |
Original comment in English
nitpick (typo): Plural agreement: use "models are" instead of "models is"
Consider: "If user-configured models are empty, return DEFAULT_MODELS". Alternatively: "If the user-configured models configuration is empty".
| - If user-configured models is empty, return `DEFAULT_MODELS` | |
| - If user-configured models are empty, return `DEFAULT_MODELS` |
Fixed issue #4.
This Pull Request contains AI-generated content.
该 PR 含有 AI 生成内容。
通过重新组织 metadata 和 mutsumi.models 设置,使得同一个模型名称可以在不同模型供应商下使用。
Summary by Sourcery
通过更改模型配置,将其改为将提供商映射到模型名称数组的形式,并在整个扩展中接入“感知提供商”的模型选择逻辑,从而允许在多个提供商之间复用同一个模型标识符。
New Features:
Bug Fixes:
Enhancements:
Build:
Original summary in English
Summary by Sourcery
Allow the same model identifier to be used across multiple providers by changing the models configuration to map providers to arrays of model names and wiring provider-aware model selection throughout the extension.
New Features:
Bug Fixes:
Enhancements:
Build: