fix: improve default model selection logic to ensure that only 'chat' model can be selected by default #520
Conversation
WalkthroughThe code modifies the logic for selecting a default model in the NewThread component. Instead of picking the first model from the first enabled provider, it now selects the first model of type "Chat" or "ImageGeneration" from all enabled providers, using the model's own provider ID. Changes
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/renderer/src/components/NewThread.vue(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-check (x64)
| const model = settingsStore.enabledModels | ||
| .flatMap(provider => provider.models) | ||
| .find(m => m.type === ModelType.Chat) | ||
| if (model) { | ||
| activeModel.value = { | ||
| name: model.name, | ||
| id: model.id, | ||
| providerId: settingsStore.enabledModels[0].providerId, | ||
| providerId: model.providerId, | ||
| tags: [] |
There was a problem hiding this comment.
🛠️ Refactor suggestion
providerId can become undefined; selecting a model without its provider breaks thread creation
model.providerId is assumed to exist, but in every other place of the codebase the providerId is supplied by the provider wrapper, not by the MODEL_META itself (handleModelUpdate, previous loops, etc.).
If the meta objects in provider.models do not embed providerId, activeModel.value.providerId will be undefined, leading to:
createThread()receiving an invalidproviderId- Model-icon lookup in the UI failing
- Subsequent preference persistence writing an incomplete object
In addition, when no Chat-type model exists the watch leaves activeModel empty, later producing the same runtime failure.
-const model = settingsStore.enabledModels
- .flatMap(provider => provider.models)
- .find(m => m.type === ModelType.Chat)
-if (model) {
- activeModel.value = {
- name: model.name,
- id: model.id,
- providerId: model.providerId,
- tags: []
- }
+const record = settingsStore.enabledModels
+ .flatMap(p => p.models.map(m => ({ model: m, providerId: p.providerId })))
+ .find(r => r.model.type === ModelType.Chat)
+if (record) {
+ activeModel.value = {
+ name: record.model.name,
+ id: record.model.id,
+ providerId: record.providerId,
+ tags: []
+ }
+} else if (settingsStore.enabledModels.length) {
+ // graceful fallback – first available model of first provider
+ const p = settingsStore.enabledModels[0]
+ const m = p.models[0]
+ activeModel.value = {
+ name: m.name,
+ id: m.id,
+ providerId: p.providerId,
+ tags: []
+ }
}This preserves the provider context, prevents undefined values, and guarantees a fallback.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const model = settingsStore.enabledModels | |
| .flatMap(provider => provider.models) | |
| .find(m => m.type === ModelType.Chat) | |
| if (model) { | |
| activeModel.value = { | |
| name: model.name, | |
| id: model.id, | |
| providerId: settingsStore.enabledModels[0].providerId, | |
| providerId: model.providerId, | |
| tags: [] | |
| const record = settingsStore.enabledModels | |
| .flatMap(p => p.models.map(m => ({ model: m, providerId: p.providerId }))) | |
| .find(r => r.model.type === ModelType.Chat) | |
| if (record) { | |
| activeModel.value = { | |
| name: record.model.name, | |
| id: record.model.id, | |
| providerId: record.providerId, | |
| tags: [] | |
| } | |
| } else if (settingsStore.enabledModels.length) { | |
| // graceful fallback – first available model of first provider | |
| const p = settingsStore.enabledModels[0] | |
| const m = p.models[0] | |
| activeModel.value = { | |
| name: m.name, | |
| id: m.id, | |
| providerId: p.providerId, | |
| tags: [] | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/renderer/src/components/NewThread.vue around lines 233 to 241, the code
assigns activeModel.value.providerId directly from model.providerId, which can
be undefined because providerId is not embedded in model metadata but provided
by the provider wrapper. To fix this, retrieve providerId from the provider
object in the enabledModels array when selecting the model, ensuring providerId
is correctly assigned. Also, add a fallback to handle cases where no Chat-type
model exists to prevent activeModel from being empty and causing runtime errors.
| .flatMap((provider) => | ||
| provider.models.map((m) => ({ ...m, providerId: provider.providerId })) | ||
| ) | ||
| .find((m) => m.type === ModelType.Chat) |
There was a problem hiding this comment.
ModelType.ImageGeneration should also be selectable.
|
LGTM |
2 participants
Pull Request Description (中文)
你的功能请求是否与某个问题有关？请描述一下。
请对问题进行清晰扼要的描述。
newThread中默认模型可能会选择非chat模型。
请描述你希望的解决方案
优化默认模型选择的逻辑。
Summary by CodeRabbit