Conversation
…n not in fetched list (#136) When the provider /models endpoint returns a list that does not contain the currently-active model id, the native <select> silently fell back to rendering options[0] and the card claimed that was the active model. The top-bar ModelSwitcher and the actual request kept using the real config.modelPrimary, so this was a pure UI lie but it seriously misled users debugging 4xx failures (related: #124, #134). Pin the active model id at the top of the dropdown with an `(active, not in provider list)` hint whenever it is missing from the fetched list. The select now always reflects reality and surfaces the "your configured model isn't one the provider advertised" signal. Extracted the option-building logic as `computeModelOptions` for deterministic unit coverage. Signed-off-by: hqhq1025 <1506751656@qq.com>
5aefb3a to
0a67959
Compare
Contributor
There was a problem hiding this comment.
Findings
- None.
Summary
- Review mode: initial
- No issues found on added/modified lines in this diff.
- Residual risk/testing gap: coverage added for
computeModelOptionspure logic, but no UI-level interaction test confirms the Settings<select>renders the pinned active option in the DOM when the fetched model list excludes the active model.
Testing
- Not run (automation)
open-codesign Bot
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #136. Related: #124, #134.
The active-provider card in Settings was displaying the wrong model whenever
config.modelPrimarywas not in the list returned by the provider's/modelsendpoint. The native<select value={config.modelPrimary}>silently fell back to renderingoptions[0]— so the card visually claimed the active model was, say,claude-3-5-haiku-20241022, while:ModelSwitchercorrectly readconfig.modelPrimary(e.g.opus-4-7)cfg.activeModel(alsoopus-4-7)Pure UI inconsistency, but seriously misleading when debugging 4xx failures ("I thought I was using opus-4-7 but the card says haiku — am I?"), which is exactly the debugging loop #124/#134 are stuck in.
Fix
RowModelSelectornow injects the active model id at the top of the options list with an(active, not in provider list)hint whenever it's missing from the fetched list. The dropdown always matches reality, and users can spot at a glance that their configured model is not one the provider's/modelsreturned — a useful diagnostic signal.Extracted the option-building logic into a pure
computeModelOptionshelper and covered it with deterministic unit tests (loading / empty / active-in-list / active-not-in-list / inactive-row).Files
apps/desktop/src/renderer/src/components/Settings.tsx— inject pinned active option, exported helperapps/desktop/src/renderer/src/components/Settings.test.ts— 5 new tests forcomputeModelOptionspackages/i18n/src/locales/{en,zh-CN}.json— newsettings.providers.activeNotInListkeyPRINCIPLES §5b
Test plan
pnpm --filter @open-codesign/desktopvitest: 875/875 pass (includes 5 new)pnpm typecheck: 10/10 passpnpm lint(biome): clean/modelshides your configured model id; open Settings; confirm the card's<select>shows the active id with the hint, top-bar and card agree