fix(settings): 用当前模型验证连接#187
Conversation
The settings validation button now uses the currently selected LLM model for a real chat-completion probe instead of only listing models/auth state. The frontend message was simplified to reflect validation success without conflating it with model-count discovery, which remains handled by the dedicated fetch-models action. Constraint: Keep the change minimal and local; preserve the separate fetch-models workflow for listing available models. Rejected: Keep auth-only validation | it would keep missing the failure mode reported in issue Open-Less#136. Rejected: Add a broader provider refactor | unnecessary for the narrow validation behavior change. Confidence: high Scope-risk: narrow Directive: Validation should probe the selected model path; model enumeration belongs to fetch-models. Tested: npm ci; npm run build; cargo test --lib Not-tested: Live provider endpoint behavior against real credentials on the user’s machine
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @H-Chris233, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
![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: a259b57d5e
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .polish("验证连接", PolishMode::Raw, &[], &[], None) | ||
| .await | ||
| .map(|_| ()) | ||
| .map_err(|e| e.to_string()) |
There was a problem hiding this comment.
When validate_llm_provider maps LLMError with e.to_string(), non-2xx provider responses become strings like invalid response: status 401... instead of the existing providerHttpStatus:<code> format used elsewhere. In the settings flow, providerErrorMessage only recognizes the providerHttpStatus: prefix, so bad API key/model/endpoints now show a generic failure instead of the specific HTTP-status guidance, which regresses diagnosability for real connection failures.
Useful? React with 👍 / 👎.
LLM validation now maps non-2xx completion responses back to the same providerHttpStatus:<code> marker used by the model-listing path. That keeps the settings page on its existing error-copy branch so real API-key, endpoint, and model failures stay diagnosable instead of collapsing into a generic message. Constraint: Keep the change minimal and preserve the existing settings error-message parser. Rejected: Teach the frontend to parse LLMError::InvalidResponse text | this would duplicate provider-status semantics and broaden the UI change. Confidence: high Scope-risk: narrow Directive: Validation error strings for provider status should stay normalized at the command boundary. Tested: cargo test --lib Not-tested: Live provider error text against a real non-2xx endpoint
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
/review |
PR Reviewer Guide 🔍(Review updated until commit e996199)Here are some key observations to aid the review process:
|
The LLM connection check no longer falls back to an Ark-specific default model when the user has not configured one. Instead, it returns a dedicated marker and the settings UI surfaces the empty-state copy, which keeps the validation result aligned with the actual configuration state. Constraint: Preserve the existing provider-status parsing and keep the change minimal. Rejected: Keep the deepseek-v3-2 fallback | it creates a false positive for unconfigured non-Ark providers. Confidence: high Scope-risk: narrow Directive: Validation should never invent a model when the user has not saved one. Tested: npm run build; cargo test --lib Not-tested: Live provider behavior with a truly empty model field on a real endpoint
|
/review |
|
Persistent review updated to latest commit e996199 |
1 participant
摘要
Fixes #136。
本 PR 修复设置页“连接检查”只验证鉴权或模型列表，无法覆盖当前选中 LLM 模型真实调用路径的问题。
此前用户可能遇到一种情况：模型列表可以正常拉取，API Key / Endpoint 看起来配置正确，但实际语音识别后的润色阶段仍然失败。原因是“验证”入口没有真正调用当前选中的 LLM 模型，只能证明鉴权或模型枚举接口可用，不能证明当前模型可用于 chat-completion / polish 流程。
本次改动让 LLM 连接检查直接使用当前保存的模型 ID 执行一次真实 polish 探测。模型枚举仍保留在独立的“拉取模型”入口中，避免把“验证当前模型可用”和“列出可用模型”两个行为混在一起。
修复 / 新增 / 改进
调整
validate_provider_credentials行为：llm验证改为执行当前选中模型的真实调用探测asr验证继续走现有模型列表 / 鉴权检查路径新增 LLM 验证路径：
保留独立“拉取模型”流程：
简化前端验证结果：
更新 IPC 类型：
ProviderCheckResult移除modelCount{ ok: true }更新中英文 i18n 文案：
兼容
不包含：
对现有用户 / 本地环境 / 构建流程的影响：