fix(settings): stabilize provider model toggles#1444
Conversation
📝 WalkthroughWalkthroughThis PR stabilizes component identity in a virtualized model list by adding explicit keys to rendered items and optimizes status-based sorting through a precomputed sort-order map that automatically recomputes when the model structure changes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/src/components/settings/ModelConfigItem.vue (1)
11-24:⚠️ Potential issue | 🟡 MinorHardcoded Chinese strings should use vue-i18n keys.
The
titleattributes contain hardcoded Chinese text ("视觉能力", "函数调用能力", "推理能力", "联网搜索能力"). These user-facing strings should be internationalized.🌐 Proposed fix using i18n
- <Icon v-if="vision" icon="lucide:eye" class="w-4 h-4 text-blue-500" title="视觉能力" /> + <Icon v-if="vision" icon="lucide:eye" class="w-4 h-4 text-blue-500" :title="$t('model.capabilities.vision')" /> <Icon v-if="functionCall" icon="lucide:function-square" class="w-4 h-4 text-orange-500" - title="函数调用能力" + :title="$t('model.capabilities.functionCall')" /> - <Icon v-if="reasoning" icon="lucide:brain" class="w-4 h-4 text-purple-500" title="推理能力" /> + <Icon v-if="reasoning" icon="lucide:brain" class="w-4 h-4 text-purple-500" :title="$t('model.capabilities.reasoning')" /> <Icon v-if="enableSearch" icon="lucide:globe" class="w-4 h-4 text-green-500" - title="联网搜索能力" + :title="$t('model.capabilities.search')" />As per coding guidelines: "All user-facing strings must use vue-i18n keys located in
src/renderer/src/i18n"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/components/settings/ModelConfigItem.vue` around lines 11 - 24, The title attributes on the Icon elements in ModelConfigItem.vue are hardcoded Chinese strings; replace them with vue-i18n lookups (use $t with appropriate i18n keys) for each Icon (the ones rendered when props vision, functionCall, reasoning, enableSearch are true) so titles become translated keys (e.g., replace "视觉能力", "函数调用能力", "推理能力", "联网搜索能力" with corresponding i18n keys) and ensure the keys are added to the i18n resource files; update the Icon title usage to bind the translated value (use the component's $t calls) for accessibility and internationalization.
🧹 Nitpick comments (1)
src/renderer/settings/components/ProviderModelList.vue (1)
230-246: Duplicate:keyattributes on wrapper div and ModelConfigItem.Both the wrapper
<div>(line 230) and<ModelConfigItem>(line 232) use:key="item.id". Since theDynamicScrolleralready useskey-field="id"(line 190) for item identity, and Vue's virtual DOM reconciliation primarily needs the key on the component itself, the key on the wrapper div is redundant.This isn't harmful, but you could simplify by removing the key from one of them. The key on
ModelConfigItemis the more meaningful one as it ensures proper component instance recycling.♻️ Optional: Remove duplicate key from wrapper div
- <div v-else-if="isModelItem(item)" :key="item.id" class="bg-card"> + <div v-else-if="isModelItem(item)" class="bg-card"> <ModelConfigItem :key="item.id"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/settings/components/ProviderModelList.vue` around lines 230 - 246, Remove the redundant :key from the wrapper div that surrounds the ModelConfigItem to avoid duplicate keys (both currently use :key="item.id"); keep the :key="item.id" on the ModelConfigItem component so Vue/VirtualScroller (key-field="id") can properly reconcile component instances—locate the wrapper div around the ModelConfigItem in ProviderModelList.vue and delete its :key attribute.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/renderer/src/components/settings/ModelConfigItem.vue`:
- Around line 11-24: The title attributes on the Icon elements in
ModelConfigItem.vue are hardcoded Chinese strings; replace them with vue-i18n
lookups (use $t with appropriate i18n keys) for each Icon (the ones rendered
when props vision, functionCall, reasoning, enableSearch are true) so titles
become translated keys (e.g., replace "视觉能力", "函数调用能力", "推理能力", "联网搜索能力" with
corresponding i18n keys) and ensure the keys are added to the i18n resource
files; update the Icon title usage to bind the translated value (use the
component's $t calls) for accessibility and internationalization.
---
Nitpick comments:
In `@src/renderer/settings/components/ProviderModelList.vue`:
- Around line 230-246: Remove the redundant :key from the wrapper div that
surrounds the ModelConfigItem to avoid duplicate keys (both currently use
:key="item.id"); keep the :key="item.id" on the ModelConfigItem component so
Vue/VirtualScroller (key-field="id") can properly reconcile component
instances—locate the wrapper div around the ModelConfigItem in
ProviderModelList.vue and delete its :key attribute.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b1b79aa7-aae1-4862-9f04-27add2a42c0a
📒 Files selected for processing (2)
src/renderer/settings/components/ProviderModelList.vuesrc/renderer/src/components/settings/ModelConfigItem.vue
Summary by CodeRabbit
Release Notes