fix(deepseek): use position-based model selection instead of text matching#1123
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the DeepSeek CLI adapter to be resilient to DeepSeek’s localized UI by changing model selection logic and relaxing failures when the user didn’t explicitly request a toggle.
Changes:
- Update
selectModel()to choose model radios by position rather than radio label text. - Update
askto only throw on model/feature-toggle failures when the user explicitly requested expert/think/search. - Make toggle wait logic null-safe via optional chaining.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
clis/deepseek/utils.js |
Changes model selection from text matching to positional selection in the page DOM. |
clis/deepseek/ask.js |
Adjusts error/throw behavior for model + feature toggles and makes toggled checks null-safe. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var radios = document.querySelectorAll('div[role="radio"]'); | ||
| if (radios.length === 0) return { ok: false }; | ||
| var isFirst = '${modelName}'.toLowerCase() === 'instant'; | ||
| var target = isFirst ? radios[0] : radios[radios.length - 1]; |
There was a problem hiding this comment.
selectModel() can return { ok: true } even when it cannot actually switch to the requested model. If modelName is not 'instant' (e.g. 'expert') and the page only has 0–1 radios, target resolves to the first/only element and the function reports success without verifying the model exists, so ask --model expert may silently run in the wrong mode. Consider validating modelName (instant|expert) and returning { ok: false } unless the expected number of radios (or a more specific model-toggle container) is found for the requested model.
| var radios = document.querySelectorAll('div[role="radio"]'); | |
| if (radios.length === 0) return { ok: false }; | |
| var isFirst = '${modelName}'.toLowerCase() === 'instant'; | |
| var target = isFirst ? radios[0] : radios[radios.length - 1]; | |
| var normalizedModelName = '${modelName}'.toLowerCase(); | |
| if (normalizedModelName !== 'instant' && normalizedModelName !== 'expert') { | |
| return { ok: false, reason: 'unsupported model' }; | |
| } | |
| var radios = document.querySelectorAll('div[role="radio"]'); | |
| if (radios.length < 2) { | |
| return { ok: false, reason: 'model selector unavailable' }; | |
| } | |
| var isFirst = normalizedModelName === 'instant'; | |
| var target = isFirst ? radios[0] : radios[radios.length - 1]; | |
| if (!target) return { ok: false, reason: 'target model not found' }; |
| var radios = document.querySelectorAll('div[role="radio"]'); | ||
| if (radios.length === 0) return { ok: false }; | ||
| var isFirst = '${modelName}'.toLowerCase() === 'instant'; | ||
| var target = isFirst ? radios[0] : radios[radios.length - 1]; | ||
| var alreadySelected = target.getAttribute('aria-checked') === 'true'; |
There was a problem hiding this comment.
selectModel() uses var for radios, isFirst, target, and alreadySelected while the rest of this module consistently uses const/let. Switching these to const (and let only when reassignment is needed) would keep style consistent and reduce accidental reassignments inside the evaluated snippet.
| var radios = document.querySelectorAll('div[role="radio"]'); | |
| if (radios.length === 0) return { ok: false }; | |
| var isFirst = '${modelName}'.toLowerCase() === 'instant'; | |
| var target = isFirst ? radios[0] : radios[radios.length - 1]; | |
| var alreadySelected = target.getAttribute('aria-checked') === 'true'; | |
| const radios = document.querySelectorAll('div[role="radio"]'); | |
| if (radios.length === 0) return { ok: false }; | |
| const isFirst = '${modelName}'.toLowerCase() === 'instant'; | |
| const target = isFirst ? radios[0] : radios[radios.length - 1]; | |
| const alreadySelected = target.getAttribute('aria-checked') === 'true'; |
4b47fc7 to
216764a
Compare
Summary
Fixes #1111.
Root cause
DeepSeek supports multiple UI languages. The previous code matched UI elements by text content:
selectModel()matched "Instant"/"Expert" but Chinese UI shows "快速模式"/"专家模式"setFeature()matched "DeepThink"/"Search" but Chinese UI shows "深度思考"/"智能搜索"This caused
Could not switch to instant modelon everyaskcommand for non-English users.Changes
utils.js
selectModel(): match by position (first radio = instant, last = expert) instead of text.utils.js
setFeature(): match by position using.ds-toggle-buttonclass (first = DeepThink, second = Search) instead of text.ask.js: only throw on toggle failure when the user explicitly requested a non-default option. Default instant mode and disabled features silently continue if selectors are not found.
Test plan
Tested on both English and Chinese DeepSeek UI:
opencli deepseek ask "hello" --newworks (default, no throw)opencli deepseek ask "hello" --model expertswitches modelopencli deepseek ask "hello" --model instantexplicit instant worksopencli deepseek ask "hello" --thinkenables DeepThink (Chinese: "深度思考")opencli deepseek ask "hello" --searchenables Search (Chinese: "智能搜索")opencli deepseek ask "hello"after--thinkcorrectly resetsopencli deepseek statusworksopencli deepseek historyworks