fix(config): honor explicit custom model for non-DeepSeek providers (#1714)#1740
fix(config): honor explicit custom model for non-DeepSeek providers (#1714)#1740LeoLin990405 wants to merge 2 commits into
Conversation
…mbown#1714) The CLI --model handoff only exports DEEPSEEK_MODEL, which apply_env_overrides funneled into the DeepSeek-only root default_text_model slot. default_model() then treated it as a normalizable weak default and fell back to a DeepSeek/provider default for an unrecognized custom model on a non-DeepSeek provider, so e.g. --provider openai --model MiniMax-M2.7 silently sent a DeepSeek model to the endpoint. Route DEEPSEEK_MODEL into the active provider's model slot for non-DeepSeek providers (mirroring the existing OPENAI_MODEL branch), and return an explicit non-DeepSeek-alias model verbatim from default_model(). DeepSeek/DeepSeekCN behavior is unchanged. Adds a regression test next to openai_provider_accepts_custom_model_and_base_url. Refs Hmbown#1714, Hmbown#1739, Hmbown#1736.
There was a problem hiding this comment.
Code Review
This pull request addresses issue #1714 by ensuring that custom model names are correctly passed through for non-DeepSeek providers when specified via environment variables. The changes include logic to preserve verbatim model strings and an update to environment variable overriding to map values to the correct provider-specific configuration. A regression test was added to verify these behaviors. Feedback was provided to improve code clarity by marking logically unreachable match arms with unreachable!() in the environment override logic.
| let entry = match provider { | ||
| ApiProvider::Deepseek => &mut providers.deepseek, | ||
| ApiProvider::DeepseekCN => &mut providers.deepseek_cn, | ||
| ApiProvider::NvidiaNim => &mut providers.nvidia_nim, | ||
| ApiProvider::Openai => &mut providers.openai, | ||
| ApiProvider::Atlascloud => &mut providers.atlascloud, | ||
| ApiProvider::Openrouter => &mut providers.openrouter, | ||
| ApiProvider::Novita => &mut providers.novita, | ||
| ApiProvider::Fireworks => &mut providers.fireworks, | ||
| ApiProvider::Sglang => &mut providers.sglang, | ||
| ApiProvider::Vllm => &mut providers.vllm, | ||
| ApiProvider::Ollama => &mut providers.ollama, | ||
| }; |
There was a problem hiding this comment.
The ApiProvider::Deepseek and ApiProvider::DeepseekCN match arms are logically unreachable here because they are already handled by the if branch at line 2388. While listing them ensures exhaustiveness, they should be explicitly marked as unreachable to clarify that this else block specifically handles non-DeepSeek providers and to avoid confusion for future maintainers.
let entry = match provider {
ApiProvider::Deepseek | ApiProvider::DeepseekCN => unreachable!("DeepSeek providers are handled in the if branch"),
ApiProvider::NvidiaNim => &mut providers.nvidia_nim,
ApiProvider::Openai => &mut providers.openai,
ApiProvider::Atlascloud => &mut providers.atlascloud,
ApiProvider::Openrouter => &mut providers.openrouter,
ApiProvider::Novita => &mut providers.novita,
ApiProvider::Fireworks => &mut providers.fireworks,
ApiProvider::Sglang => &mut providers.sglang,
ApiProvider::Vllm => &mut providers.vllm,
ApiProvider::Ollama => &mut providers.ollama,
};Address gemini-code-assist review on PR Hmbown#1740 (MEDIUM, clarity): the non-DeepSeek else-branch match listed ApiProvider::Deepseek / DeepseekCN arms that are logically unreachable (handled by the if branch above). Collapse to a single 'Deepseek | DeepseekCN => unreachable!(...)' arm so the intent is explicit for future maintainers. No behavior change. Refs Hmbown#1714.
|
Done in the latest push — the two logically-unreachable arms are collapsed to |
Address gemini-code-assist review on PR Hmbown#1740 (MEDIUM, clarity): the non-DeepSeek else-branch match listed ApiProvider::Deepseek / DeepseekCN arms that are logically unreachable (handled by the if branch above). Collapse to a single 'Deepseek | DeepseekCN => unreachable!(...)' arm so the intent is explicit for future maintainers. No behavior change. Refs Hmbown#1714.
Summary / 概述
EN: Fixes #1714. An explicit custom model for a non‑DeepSeek provider (e.g.
deepseek --provider openai --model MiniMax-M2.7) was silently replaced by a DeepSeek/provider default, so the configured model never reached the endpoint. This PR makes an explicitly‑chosen custom model pass through verbatim for every non‑DeepSeek provider.中文: 修复 #1714。当使用非 DeepSeek 提供方并显式指定自定义模型时(例如
deepseek --provider openai --model MiniMax-M2.7),配置的模型名会被静默替换为 DeepSeek/提供方默认值,导致请求实际发送的并不是用户指定的模型。本 PR 让任何非 DeepSeek 提供方下显式指定的自定义模型原样透传。Root cause / 根因
EN: The CLI
--modelhandoff only ever exportsDEEPSEEK_MODEL(never the provider‑specific*_MODELvar). Inapply_env_overrides,DEEPSEEK_MODELwas funneled into the rootdefault_text_modelslot — a DeepSeek‑only slot the validator rejects for non‑DeepSeek IDs.Config::default_model()then treated it as a weak, normalizable default and, for an unrecognized custom model on a non‑DeepSeek provider, fell through to a DeepSeek/provider default instead of returning the user's string. This matches the maintainer's own suspicion in the v0.8.40 audit (#1736): “explicitDEEPSEEK_MODELor--modelgetting overridden later bysettings.provider_modelsor provider‑switch state.”中文: CLI 的
--model透传只会导出DEEPSEEK_MODEL(永远不会导出提供方专用的*_MODEL变量)。apply_env_overrides把DEEPSEEK_MODEL写进了根级default_text_model槽位——而该槽位是 DeepSeek 专用的,校验器会拒绝非 DeepSeek 的模型 ID。随后Config::default_model()把它当作一个可被规范化的弱默认值;对于非 DeepSeek 提供方上无法识别的自定义模型,便回退成 DeepSeek/提供方默认值,而不是返回用户指定的字符串。这与维护者在 v0.8.40 审计(#1736)中的判断一致:“显式的DEEPSEEK_MODEL或--model之后被settings.provider_models或切换提供方的状态覆盖。”Fix / 修复
Two surgical changes in
crates/tui/src/config.rs, reusing the existing provider‑scoped passthrough mechanism (no new code path):apply_env_overrides: for a non‑DeepSeek provider, routeDEEPSEEK_MODELinto the active provider's model slot (mirroring the existingOPENAI_MODELbranch) instead of the DeepSeek‑only root slot. DeepSeek/DeepSeekCN behavior is unchanged.default_model(): an explicit provider‑scoped model that is not a recognized DeepSeek alias is returned verbatim for non‑DeepSeek providers, instead of falling back to a default.中文: 在
crates/tui/src/config.rs中做两处精准改动,复用既有的按提供方透传机制(不新增代码路径):①apply_env_overrides对非 DeepSeek 提供方,将DEEPSEEK_MODEL写入当前提供方的模型槽位(与既有OPENAI_MODEL分支一致),而非 DeepSeek 专用根槽位;②default_model()对非 DeepSeek 提供方,若显式模型不是已知 DeepSeek 别名,则原样返回,不再回退默认值。DeepSeek/DeepSeekCN 行为保持不变。Scope / 改动边界
normalize_model_config,provider_passes_model_through, the validator, the existingSGLANG/VLLM/OLLAMA/NVIDIA_NIM_MODELenv branches, or any other file.Testing / 测试
New regression test
deepseek_model_env_passes_custom_model_through_for_non_deepseek_providers(next toopenai_provider_accepts_custom_model_and_base_url), covering: (a)provider=openai+DEEPSEEK_MODEL=MiniMax-M2.7, noOPENAI_MODEL, default base_url →default_model() == "MiniMax-M2.7"; (b) a non‑passthrough provider (novita) with the same custom model preserved verbatim.新增回归测试
deepseek_model_env_passes_custom_model_through_for_non_deepseek_providers,覆盖 OpenAI 兼容端点与一个非透传提供方两种情形;既有openai_provider_accepts_custom_model_and_base_url等仍全部通过。Refs #1714, #1739, #1736.