feat: enhance model selection logic in common setting #533
Conversation
WalkthroughA new embedding model default setting was added, and the model selection UI was updated to allow filtering by model types via a new prop. Components using the model selector now restrict selectable models to "Chat" and "ImageGeneration" types. The fallback model selection logic was also updated to prefer these types. Changes
Sequence Diagram(s)sequenceDiagram
participant UI_Component as UI Component (e.g., NewThread, TitleView)
participant ModelSelect as ModelSelect.vue
participant ModelStore as Model Store
UI_Component->>ModelSelect: Render with :type=[Chat, ImageGeneration]
ModelSelect->>ModelStore: Fetch models for providers
ModelStore-->>ModelSelect: Return all models
ModelSelect->>ModelSelect: Filter models by type prop
ModelSelect-->>UI_Component: Display filtered model list
sequenceDiagram
participant Settings as settings.ts
participant Providers as Providers List
Settings->>Providers: Get enabled providers and their models
Providers-->>Settings: Return providers with models
Settings->>Settings: Flatten all enabled models
Settings->>Settings: Find first model where type is Chat or ImageGeneration
Settings-->>Providers: Return found model and providerId
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/renderer/src/components/ModelSelect.vue (1)
62-66: Prop lacks default & documentation
type?: ModelType[]is introduced but no default value or JSDoc comment is provided. Consumers must now remember that “undefined” ≠ “empty array”. Consider:defineProps({ type: { type: Array as PropType<ModelType[]>, default: undefined // ← explicit for clarity } })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/presenter/configPresenter/modelDefaultSettings.ts(1 hunks)src/renderer/src/components/ModelSelect.vue(2 hunks)src/renderer/src/components/NewThread.vue(1 hunks)src/renderer/src/components/TitleView.vue(2 hunks)src/renderer/src/components/mcp-config/mcpServerForm.vue(2 hunks)src/renderer/src/components/settings/CommonSettings.vue(2 hunks)src/renderer/src/stores/settings.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-check (x64)
🔇 Additional comments (6)
src/main/presenter/configPresenter/modelDefaultSettings.ts (1)
1674-1690:matchpatterns are overly-generic – may capture unintended modelsUsing very broad keywords (
'embedding','embed') makes this entry the default match for any provider/model id that merely contains those substrings.
Side-effects:
- Genuine provider-specific embedding models (e.g. OpenAI
text-embedding-*) will also satisfy this rule, causing ambiguous or duplicate hits.- If multiple entries match, whichever is iterated first “wins”, resulting in hard-to-trace behaviour.
Consider tightening the pattern or turning this into an explicit fallback entry only when no other embedding model matches, e.g.:
- match: ['embedding', 'embed'], + // Only match the literal id "embedding" + match: ['^embedding$'],(or move this object to the end of the list and document that it is a last-resort default).
Also double-check that the plain id
embeddingwill not collide with any real provider id.
[ suggest_essential_refactor ]src/renderer/src/components/TitleView.vue (1)
30-33: Hard-coded allowed types duplicated in several views
[ModelType.Chat, ModelType.ImageGeneration]is now copy-pasted across at least four components.
Extracting a small constant (e.g.ASSISTANT_MODEL_TYPES) in a shared module, or givingModelSelectits own sensible default, removes the duplication and the risk of future divergence.[ suggest_optional_refactor ]
src/renderer/src/components/settings/CommonSettings.vue (1)
80-83: Same concern as raised inTitleView.vue– see previous comment.
[ duplicate_comment ]src/renderer/src/components/mcp-config/mcpServerForm.vue (1)
798-801: Duplication of the hard-coded type array – see earlier comment.
[ duplicate_comment ]src/renderer/src/components/NewThread.vue (1)
61-64: Duplication of the hard-coded type array – see earlier comment.
[ duplicate_comment ]src/renderer/src/stores/settings.ts (1)
85-95: Fallback now returnsnullwhen no Chat / ImageGeneration model exists—verify downstream handlingThe new logic sensibly prefers
ChatorImageGenerationtypes, but if none are enabledfindPriorityModel()returnsnullwhere the old version always returned “something”. Down-stream callers (initOrUpdateSearchAssistantModel, etc.) guard againstnull, yet any new caller must remember to do the same. Confirm UI code won’t break for users who only enable “Embedding” models.
你的功能请求是否与某个问题有关？请描述一下。
通用设置（commonSetting）中助手模型默认值可能错误的选择embedding模型。
请描述你希望的解决方案
修改settings中findPriorityModel逻辑，过滤chat和ImageGeneration模型
修改ModelSelect，增加基于类别过滤选项。
平台兼容性注意事项
如果此 PR 具有特定的平台兼容性考虑因素（Windows、macOS、Linux），请在此处描述。
附加背景
模型分类ModelType和模型能力functionCall、reasoning、vision是否需要做整合？例如直接通过tags标记模型能力。
另外ollama界面无法修改模型属性，对于未识别到类型的模型无法手动修改。
Summary by CodeRabbit