-
Notifications
You must be signed in to change notification settings - Fork 516
feat: sync provider sorting from settings to model selection #736
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughReplaces onMounted population with a computed providers list derived from settingsStore.sortedProviders and settingsStore.enabledModels, filtering by enabled providers and optional type, removing onMounted usage and on-load error handling, and preserving provider order. No public API/signature changes. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant MS as ModelSelect.vue (computed)
participant SS as settingsStore
U->>MS: Open model selector / read UI
MS->>SS: Read sortedProviders, enabledModels
SS-->>MS: Providers + enabled models
MS->>MS: Build orderedProviders: filter enabled, apply type filter, drop empties
MS-->>U: Render ordered providers and models
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/renderer/src/components/ModelSelect.vue
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Use English for logs and comments
Files:
src/renderer/src/components/ModelSelect.vue
src/renderer/src/**/*.vue
📄 CodeRabbit Inference Engine (CLAUDE.md)
src/renderer/src/**/*.vue
: Use Composition API for all Vue 3 components
Use Tailwind CSS with scoped styles for styling
Organize components by feature in src/renderer/src/
Follow existing component patterns in src/renderer/src/ when creating new UI components
Use Composition API with proper TypeScript typing for new UI components
Implement responsive design with Tailwind CSS for new UI components
Add proper error handling and loading states for new UI componentsUse scoped styles to prevent CSS conflicts between components
Files:
src/renderer/src/components/ModelSelect.vue
src/renderer/src/**/*.{ts,tsx,vue}
📄 CodeRabbit Inference Engine (CLAUDE.md)
src/renderer/src/**/*.{ts,tsx,vue}
: Use Pinia for frontend state management
Renderer to Main: Use usePresenter.ts composable for direct presenter method calls
Files:
src/renderer/src/components/ModelSelect.vue
src/renderer/src/**/*
📄 CodeRabbit Inference Engine (.cursor/rules/i18n.mdc)
src/renderer/src/**/*
: All user-facing strings must use i18n keys (avoid hardcoded user-visible text in code)
Use the 'vue-i18n' framework for all internationalization in the renderer
Ensure all user-visible text in the renderer uses the translation system
Files:
src/renderer/src/components/ModelSelect.vue
src/renderer/**/*.{vue,ts,js,tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/project-structure.mdc)
渲染进程代码放在
src/renderer
Files:
src/renderer/src/components/ModelSelect.vue
src/renderer/src/**/*.{vue,ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/vue-best-practices.mdc)
src/renderer/src/**/*.{vue,ts,tsx,js,jsx}
: Use the Composition API for better code organization and reusability
Implement proper state management with Pinia
Utilize Vue Router for navigation and route management
Leverage Vue's built-in reactivity system for efficient data handling
Files:
src/renderer/src/components/ModelSelect.vue
src/renderer/**/*.{ts,tsx,vue}
📄 CodeRabbit Inference Engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,tsx,vue}
: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Use TypeScript for all code; prefer types over interfaces.
Avoid enums; use const objects instead.
Use arrow functions for methods and computed properties.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Files:
src/renderer/src/components/ModelSelect.vue
src/renderer/**/*.{vue,ts}
📄 CodeRabbit Inference Engine (.cursor/rules/vue-shadcn.mdc)
Implement lazy loading for routes and components.
Files:
src/renderer/src/components/ModelSelect.vue
src/renderer/**/*.{ts,vue}
📄 CodeRabbit Inference Engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,vue}
: Use useFetch and useAsyncData for data fetching.
Implement SEO best practices using Nuxt's useHead and useSeoMeta.
Files:
src/renderer/src/components/ModelSelect.vue
🔇 Additional comments (2)
src/renderer/src/components/ModelSelect.vue (2)
131-133
: Good use of a TS type predicate in filterThe type guard correctly narrows out nulls and ensures models are present, improving type safety of providers.value.
111-111
: Ignore the “enabled” rename suggestion
After tracingsortedProviders
back to its definition insettings.ts
(and all its usages), the property is consistently namedenable
, notenabled
. No change is needed here.Likely an incorrect or invalid review comment.
// 使用排序后的供应商列表 | ||
const sortedProviders = settingsStore.sortedProviders | ||
const enabledModels = settingsStore.enabledModels | ||
providers.value = enabledModels.map(({ providerId, models }) => { | ||
const provider = settingsStore.providers.find((p) => p.id === providerId) | ||
return { | ||
id: providerId, | ||
name: provider?.name || providerId, | ||
models: | ||
!props.type || props.type.length === 0 | ||
? models | ||
: models.filter( | ||
(model) => model.type !== undefined && props.type!.includes(model.type as ModelType) | ||
) | ||
} | ||
}) | ||
// 根据 sortedProviders 的顺序重新组织 enabledModels | ||
const orderedProviders = sortedProviders | ||
.filter((provider) => provider.enable) // 只保留启用的供应商 | ||
.map((provider) => { | ||
const enabledProvider = enabledModels.find((ep) => ep.providerId === provider.id) | ||
if (!enabledProvider || enabledProvider.models.length === 0) { | ||
return null | ||
} | ||
return { | ||
id: provider.id, | ||
name: provider.name, | ||
models: | ||
!props.type || props.type.length === 0 | ||
? enabledProvider.models | ||
: enabledProvider.models.filter( | ||
(model) => | ||
model.type !== undefined && props.type!.includes(model.type as ModelType) | ||
) | ||
} | ||
}) | ||
.filter( | ||
(provider): provider is { id: string; name: string; models: RENDERER_MODEL_META[] } => | ||
provider !== null && provider.models.length > 0 | ||
) // 使用类型谓词过滤掉空的供应商 | ||
providers.value = orderedProviders | ||
} catch (error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Keep provider sorting in sync reactively; update on store changes (watch/computed), not just on mount
Current logic builds the list once on mount. If settingsStore.sortedProviders, settingsStore.enabledModels, or props.type change later, the UI won’t update. Also, inline comments are not in English, which violates the coding guidelines.
Refactor to a reactive sync using watch (or computed). Below keeps the store’s order, avoids repeated Array.find with a Map, filters by type using a Set, and logs errors via i18n. Comments are in English.
Apply this diff within the selected lines:
- // 使用排序后的供应商列表
- const sortedProviders = settingsStore.sortedProviders
- const enabledModels = settingsStore.enabledModels
-
- // 根据 sortedProviders 的顺序重新组织 enabledModels
- const orderedProviders = sortedProviders
- .filter((provider) => provider.enable) // 只保留启用的供应商
- .map((provider) => {
- const enabledProvider = enabledModels.find((ep) => ep.providerId === provider.id)
- if (!enabledProvider || enabledProvider.models.length === 0) {
- return null
- }
-
- return {
- id: provider.id,
- name: provider.name,
- models:
- !props.type || props.type.length === 0
- ? enabledProvider.models
- : enabledProvider.models.filter(
- (model) =>
- model.type !== undefined && props.type!.includes(model.type as ModelType)
- )
- }
- })
- .filter(
- (provider): provider is { id: string; name: string; models: RENDERER_MODEL_META[] } =>
- provider !== null && provider.models.length > 0
- ) // 使用类型谓词过滤掉空的供应商
-
- providers.value = orderedProviders
+ // Keep providers in sync with settings' sortedProviders order
+ const syncProviders = () => {
+ try {
+ // Index enabled models by providerId for O(1) lookups
+ const enabledByProvider = new Map(
+ settingsStore.enabledModels.map((ep) => [ep.providerId, ep])
+ )
+
+ const orderedProviders = settingsStore.sortedProviders
+ .filter((provider) => provider.enable) // keep only enabled providers (verify property name)
+ .map((provider) => {
+ const enabledProvider = enabledByProvider.get(provider.id)
+ if (!enabledProvider || enabledProvider.models.length === 0) return null
+
+ const allowedTypes = props.type?.length ? new Set(props.type) : null
+ const models = allowedTypes
+ ? enabledProvider.models.filter(
+ (model) => model.type !== undefined && allowedTypes.has(model.type as ModelType)
+ )
+ : enabledProvider.models
+
+ if (models.length === 0) return null
+ return { id: provider.id, name: provider.name, models }
+ })
+ .filter(
+ (provider): provider is { id: string; name: string; models: RENDERER_MODEL_META[] } =>
+ provider !== null
+ )
+
+ providers.value = orderedProviders
+ } catch (error) {
+ console.error(t('model.error.loadFailed'), error)
+ }
+ }
+
+ // React to settings and prop changes
+ watch(
+ [() => settingsStore.sortedProviders, () => settingsStore.enabledModels, () => props.type],
+ () => syncProviders(),
+ { deep: true, immediate: true }
+ )
Additionally, update imports to include watch:
// at the imports block
import { ref, computed, onMounted, PropType, watch } from 'vue'
🤖 Prompt for AI Agents
In src/renderer/src/components/ModelSelect.vue around lines 105 to 136, the
provider list is built once on mount and won’t react to changes in
settingsStore.sortedProviders, settingsStore.enabledModels or props.type;
replace the one-time build with a reactive approach (use watch or computed) so
the providers array updates when those sources change; optimize by creating a
Map from enabledModels to avoid repeated Array.find, use a Set for props.type
filtering, move logic into a computed (or a watch that assigns providers.value)
and wrap errors with i18n-based logging; also remove or translate Chinese
comments to English and add the watch import at the top (import { ref, computed,
onMounted, PropType, watch } from 'vue').
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/renderer/src/components/ModelSelect.vue (1)
67-95
: Reactive provider ordering looks correct; consider Map/Set + nullish guards for performance and safetyCurrent logic is correct and preserves settings’ provider order. Two optimizations and one resilience improvement:
- Build a Map from enabledModels for O(1) lookups instead of Array.find in a loop.
- Use a Set for type filtering to avoid repeated includes.
- Guard against undefined store arrays to avoid edge-case runtime errors during store hydration.
Apply this diff within the selected lines:
-const providers = computed(() => { - const sortedProviders = settingsStore.sortedProviders - const enabledModels = settingsStore.enabledModels +const providers = computed<{ id: string; name: string; models: RENDERER_MODEL_META[] }[]>(() => { + const sortedProviders = settingsStore.sortedProviders ?? [] + const enabledModels = settingsStore.enabledModels ?? [] + const enabledByProvider = new Map(enabledModels.map((ep) => [ep.providerId, ep])) + const allowedTypes = props.type?.length ? new Set(props.type) : null const orderedProviders = sortedProviders .filter((provider) => provider.enable) .map((provider) => { - const enabledProvider = enabledModels.find((ep) => ep.providerId === provider.id) + const enabledProvider = enabledByProvider.get(provider.id) if (!enabledProvider || enabledProvider.models.length === 0) { return null } return { id: provider.id, name: provider.name, models: - !props.type || props.type.length === 0 - ? enabledProvider.models - : enabledProvider.models.filter( - (model) => model.type !== undefined && props.type!.includes(model.type as ModelType) - ) + allowedTypes + ? enabledProvider.models.filter( + (model) => model.type !== undefined && allowedTypes.has(model.type as ModelType) + ) + : enabledProvider.models } }) .filter( (provider): provider is { id: string; name: string; models: RENDERER_MODEL_META[] } => provider !== null && provider.models.length > 0 ) return orderedProviders })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/renderer/src/components/ModelSelect.vue
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Use English for logs and comments
Files:
src/renderer/src/components/ModelSelect.vue
src/renderer/src/**/*.vue
📄 CodeRabbit Inference Engine (CLAUDE.md)
src/renderer/src/**/*.vue
: Use Composition API for all Vue 3 components
Use Tailwind CSS with scoped styles for styling
Organize components by feature in src/renderer/src/
Follow existing component patterns in src/renderer/src/ when creating new UI components
Use Composition API with proper TypeScript typing for new UI components
Implement responsive design with Tailwind CSS for new UI components
Add proper error handling and loading states for new UI componentsUse scoped styles to prevent CSS conflicts between components
Files:
src/renderer/src/components/ModelSelect.vue
src/renderer/src/**/*.{ts,tsx,vue}
📄 CodeRabbit Inference Engine (CLAUDE.md)
src/renderer/src/**/*.{ts,tsx,vue}
: Use Pinia for frontend state management
Renderer to Main: Use usePresenter.ts composable for direct presenter method calls
Files:
src/renderer/src/components/ModelSelect.vue
src/renderer/src/**/*
📄 CodeRabbit Inference Engine (.cursor/rules/i18n.mdc)
src/renderer/src/**/*
: All user-facing strings must use i18n keys (avoid hardcoded user-visible text in code)
Use the 'vue-i18n' framework for all internationalization in the renderer
Ensure all user-visible text in the renderer uses the translation system
Files:
src/renderer/src/components/ModelSelect.vue
src/renderer/**/*.{vue,ts,js,tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/project-structure.mdc)
渲染进程代码放在
src/renderer
Files:
src/renderer/src/components/ModelSelect.vue
src/renderer/src/**/*.{vue,ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/vue-best-practices.mdc)
src/renderer/src/**/*.{vue,ts,tsx,js,jsx}
: Use the Composition API for better code organization and reusability
Implement proper state management with Pinia
Utilize Vue Router for navigation and route management
Leverage Vue's built-in reactivity system for efficient data handling
Files:
src/renderer/src/components/ModelSelect.vue
src/renderer/**/*.{ts,tsx,vue}
📄 CodeRabbit Inference Engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,tsx,vue}
: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Use TypeScript for all code; prefer types over interfaces.
Avoid enums; use const objects instead.
Use arrow functions for methods and computed properties.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Files:
src/renderer/src/components/ModelSelect.vue
src/renderer/**/*.{vue,ts}
📄 CodeRabbit Inference Engine (.cursor/rules/vue-shadcn.mdc)
Implement lazy loading for routes and components.
Files:
src/renderer/src/components/ModelSelect.vue
src/renderer/**/*.{ts,vue}
📄 CodeRabbit Inference Engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,vue}
: Use useFetch and useAsyncData for data fetching.
Implement SEO best practices using Nuxt's useHead and useSeoMeta.
Files:
src/renderer/src/components/ModelSelect.vue
🧠 Learnings (3)
📚 Learning: 2025-07-23T00:45:57.322Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-07-23T00:45:57.322Z
Learning: Applies to src/renderer/**/*.{vue} : Leverage ref, reactive, and computed for reactive state management.
Applied to files:
src/renderer/src/components/ModelSelect.vue
📚 Learning: 2025-07-23T00:45:57.322Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-07-23T00:45:57.322Z
Learning: Applies to src/renderer/**/*.{vue} : Use provide/inject for dependency injection when appropriate.
Applied to files:
src/renderer/src/components/ModelSelect.vue
📚 Learning: 2025-07-21T01:45:33.790Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T01:45:33.790Z
Learning: Applies to src/renderer/src/**/*.vue : Use Composition API with proper TypeScript typing for new UI components
Applied to files:
src/renderer/src/components/ModelSelect.vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-check (x64)
🔇 Additional comments (2)
src/renderer/src/components/ModelSelect.vue (2)
41-41
: Good shift to reactive computation; removing onMounted is appropriateImporting computed and dropping onMounted aligns the component with reactive store-driven updates, which matches the PR’s goal to keep provider order in sync with settings.
71-71
: No changes needed – the provider flag isenable
I confirmed in
src/renderer/src/stores/settings.ts
(lines 260–263) thatsortedProviders
filters byprovider.enable
, and all downstream uses (including inModelSelect.vue
) consistently reference.enable
. You can safely keep the filter as-is.
* fix: add AlertDialogDescription to resolve accessibility warning (#706) * fix: resolve focus flicker when creating new windows with Ctrl+Shift+N (#707) * feat: enhance window management by implementing main window ID handling (#709) * docs: update zhipu developer doc website link (#715) Co-authored-by: gongchao <[email protected]> * refactor: better translate (#716) * chore: en-us i18n * chore(i18n): polish ja-JP translations across UI; keep chat.input.placeholder unchanged * chore(i18n): polish fr-FR translations; keep chat.input.placeholder unchanged * chore(i18n): refine fr-FR MCP & Settings copy; idiomatic, concise, brand-consistent * chore(i18n): polish ru-RU translations across UI; keep chat.input.placeholder unchanged * chore(i18n): polish fa-IR translations across UI; keep chat.input.placeholder unchanged * chore: fix format * chore: fix i18n * chore: lock rolldown-vite version * feat: add GPT-5 series model support (#717) * ci(vite): Bundle the main file into a single file to speed up loading. (#718) * fix(math): parser by upgrade vue-renderer-markdown (#722) * chore: bump deps (#721) * chore: bump deps * fix: rolldown-vite 7.1.0 and duckdb bundle issue * chore: back to vite * chore: update electron * chore: update versions * fix(math): parser by upgrade vue-renderer-markdown (#722) * chore: bump deps --------- Co-authored-by: Simon He <[email protected]> * fix: add scrollable support to PopoverContent to prevent overflow (#720) * feat: implement floating chat window system with performance optimization (#724) * feat: add mcp sync and modelscope provider #615 (#723) * wip: add modelscope provider * feat: add mcp sync to modelscope * fix: add scrollable support to PopoverContent to prevent overflow (#720) * feat: implement floating chat window system with performance optimization (#724) * chore: i18n and format * feat: better style * fix: mcp tool display --------- Co-authored-by: yyhhyyyyyy <[email protected]> * fix: move_files newPath parse issue (#725) * fix: move_files newPath 参数计算规则 * fix: move_files 移动前需要判断dest是目录还是文件 * feat: add Claude Opus 4.1 to anthropic default model list (#726) * feat: Add mcprouter's MCP marketplace api support (#727) * wip: add mcp market * feat: mcp market install * wip: mcp install status sync * feat: mcp server config mask * chore: remove working doc * chore: add translate * feat: add ESC key to close floating chat window (#728) * feat: add floating button position persistence with boundary validation (#729) * feat: add floating button position persistence with boundary validation * feat: refactor floating button to use electron-window-state * chore: bump to 0.3.0 * feat: add reasoning_effort parameter support for gpt-oss models (#731) * feat: add reasoning_effort parameter support for gpt-oss models - add reasoning effort UI support across all components * fix: preserve user reasoning effort settings and improve display logic * fix: artifacts code not streaming (#732) * fix: artifact react load failed * chore: remove log * fix: artifacts code not stream * fix: format * feat: disable automatic model enabling for better UX (#734) * feat: sync provider sorting from settings to model selection (#736) * feat: sync provider sorting from settings to model selection * feat: refactor ModelSelect to use computed providers for better reactivity --------- Co-authored-by: yyhhyyyyyy <[email protected]> Co-authored-by: hllshiro <[email protected]> Co-authored-by: tomsun28 <[email protected]> Co-authored-by: gongchao <[email protected]> Co-authored-by: Simon He <[email protected]> Co-authored-by: wanna <[email protected]>
sync provider sorting from settings to model selection
Implement the second and third points of this issue #733
Summary by CodeRabbit