-
Notifications
You must be signed in to change notification settings - Fork 614
feat: remove models list dialog & show list with DynamicScroller #1185
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
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds AddCustomModelButton and shifts model-list UI to a virtualized ProviderModelList. ProviderModelManager now accepts provider/custom model props and emits a custom-model-added event; ProviderDialogContainer removes model-list props/state. Provider settings components refresh customModels when custom-model-added fires. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant AddCustomModelButton
participant ModelConfigDialog
participant ProviderModelManager
participant ProviderSettings as ModelProviderSettingsDetail
participant ModelStore
User->>AddCustomModelButton: click "Add"
AddCustomModelButton->>ModelConfigDialog: open (v-model:open)
User->>ModelConfigDialog: configure & save
ModelConfigDialog-->>AddCustomModelButton: emit saved
AddCustomModelButton->>ProviderModelManager: emit saved
ProviderModelManager-->>ProviderSettings: emit custom-model-added
ProviderSettings->>ModelStore: request refresh customModels
ModelStore-->>ProviderSettings: return updated customModels
ProviderSettings->>ProviderModelManager: update custom-models prop
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
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: Icon title attributes use hardcoded Chinese text.Per coding guidelines, all user-facing strings must use i18n keys. The
titleattributes on these icons contain hardcoded Chinese text.- <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')" />
🧹 Nitpick comments (9)
src/renderer/settings/components/ProviderModelList.vue (3)
287-295: Use English for comments.Per coding guidelines, new code should use English for logs and comments.
-// 启用提供商下所有模型 +// Enable all models for the provider const enableAllModels = (providerId: string) => { modelStore.enableAllModels(providerId) } -// 禁用提供商下所有模型 +// Disable all models for the provider const disableAllModels = (providerId: string) => { modelStore.disableAllModels(providerId) }
307-332: Unused sticky header tracking code.
stickyHeaderInfoandupdateStickyHeaderare defined and watched but never used in the template or elsewhere. Consider removing this dead code or implementing its intended functionality.-const stickyHeaderInfo = ref<{ - provider?: { providerId: string } -}>({}) - -const updateStickyHeader = (startIndex: number) => { - if (startIndex < 0 || startIndex >= virtualItems.value.length) return - - const currentItem = virtualItems.value[startIndex] - let providerItem: { providerId: string } | undefined - - if (currentItem.type === 'model' || currentItem.type === 'provider-actions') { - providerItem = { providerId: currentItem.providerId } - } - - stickyHeaderInfo.value = { - provider: providerItem - } -} - -watch( - virtualItems, - () => { - updateStickyHeader(0) - }, - { immediate: true } -)
182-202: Simplify filteredCustomModels computed property.The current implementation uses verbose for-loops where spread operators and filter would be more idiomatic and concise.
const filteredCustomModels = computed(() => { - const customModelsList: RENDERER_MODEL_META[] = [] - for (const model of props.customModels) { - customModelsList.push(model) - } - - if (!modelSearchQuery.value) { - return customModelsList - } - - const filteredModels: RENDERER_MODEL_META[] = [] - for (const model of customModelsList) { - if ( - model.name.toLowerCase().includes(modelSearchQuery.value.toLowerCase()) || - model.id.toLowerCase().includes(modelSearchQuery.value.toLowerCase()) - ) { - filteredModels.push(model) - } - } - return filteredModels + if (!modelSearchQuery.value) { + return [...props.customModels] + } + + const query = modelSearchQuery.value.toLowerCase() + return props.customModels.filter( + (model) => + model.name.toLowerCase().includes(query) || + model.id.toLowerCase().includes(query) + ) })src/renderer/src/components/settings/ModelConfigItem.vue (2)
63-63: Chinese comment should be in English.Per coding guidelines, new code should use English for comments.
- <!-- 模型配置对话框 --> + <!-- Model configuration dialog -->
112-113: Chinese comment should be in English.-// 配置对话框状态 +// Configuration dialog statesrc/renderer/settings/components/AnthropicProviderSettingsDetail.vue (1)
693-695: Consider adding error handling.The
handleAddModelSavedfunction doesn't handle potential errors fromrefreshCustomModels. This could silently fail without user feedback.const handleAddModelSaved = async () => { - await modelStore.refreshCustomModels(props.provider.id) + try { + await modelStore.refreshCustomModels(props.provider.id) + } catch (error) { + console.error('Failed to refresh custom models:', error) + } }src/renderer/settings/components/ModelProviderSettingsDetail.vue (1)
274-276: Redundant variable assignment.
initDataImmediateis assigned to_initDatabut could simply reference_initDatadirectly where needed, or be removed if the intent is just to have a semantic alias.This is a minor style preference - keeping it as-is is also acceptable for readability and self-documenting code.
src/renderer/settings/components/BedrockProviderSettingsDetail.vue (2)
263-268: Translate Chinese comments to English.The comments at lines 263 and 268 are in Chinese. Per coding guidelines, all logs and comments should be in English.
Apply this diff:
console.log('验证成功') + // Verification successful checkResult.value = true showCheckModelDialog.value = true // 验证成功后刷新当前provider的模型列表 + // Refresh provider model list after successful verification await modelStore.refreshProviderModels(props.provider.id) } else { console.log('验证失败', resp.errorMsg) + // Verification failed checkResult.value = falseAs per coding guidelines, use English for logs and comments.
327-330: Translate Chinese comment to English.The comment at line 328 is in Chinese. Per coding guidelines, all comments should be in English.
Apply this diff:
// Handler for config changes const handleConfigChanged = async () => { - // 模型配置变更后重新初始化数据 + // Reinitialize data after model configuration changes await initData() }As per coding guidelines, use English for logs and comments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/renderer/settings/components/AddCustomModelButton.vue(1 hunks)src/renderer/settings/components/AnthropicProviderSettingsDetail.vue(3 hunks)src/renderer/settings/components/BedrockProviderSettingsDetail.vue(3 hunks)src/renderer/settings/components/ModelProviderSettingsDetail.vue(4 hunks)src/renderer/settings/components/ProviderDialogContainer.vue(0 hunks)src/renderer/settings/components/ProviderModelList.vue(3 hunks)src/renderer/settings/components/ProviderModelManager.vue(1 hunks)src/renderer/src/components/settings/ModelConfigItem.vue(3 hunks)
💤 Files with no reviewable changes (1)
- src/renderer/settings/components/ProviderDialogContainer.vue
🧰 Additional context used
📓 Path-based instructions (14)
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for logs and comments (Chinese text exists in legacy code, but new code should use English)
Files:
src/renderer/src/components/settings/ModelConfigItem.vuesrc/renderer/settings/components/AddCustomModelButton.vuesrc/renderer/settings/components/AnthropicProviderSettingsDetail.vuesrc/renderer/settings/components/ProviderModelList.vuesrc/renderer/settings/components/ModelProviderSettingsDetail.vuesrc/renderer/settings/components/BedrockProviderSettingsDetail.vuesrc/renderer/settings/components/ProviderModelManager.vue
**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.vue: Use Vue 3 Composition API for all components instead of Options API
Use Tailwind CSS with scoped styles for component styling
Files:
src/renderer/src/components/settings/ModelConfigItem.vuesrc/renderer/settings/components/AddCustomModelButton.vuesrc/renderer/settings/components/AnthropicProviderSettingsDetail.vuesrc/renderer/settings/components/ProviderModelList.vuesrc/renderer/settings/components/ModelProviderSettingsDetail.vuesrc/renderer/settings/components/BedrockProviderSettingsDetail.vuesrc/renderer/settings/components/ProviderModelManager.vue
src/renderer/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
src/renderer/**/*.vue: All user-facing strings must use i18n keys via vue-i18n for internationalization
Ensure proper error handling and loading states in all UI components
Implement responsive design using Tailwind CSS utilities for all UI components
src/renderer/**/*.vue: Use composition API and declarative programming patterns; avoid options API
Structure files: exported component, composables, helpers, static content, types
Use PascalCase for component names (e.g., AuthWizard.vue)
Use Vue 3 with TypeScript, leveraging defineComponent and PropType
Use template syntax for declarative rendering
Use Shadcn Vue, Radix Vue, and Tailwind for components and styling
Implement responsive design with Tailwind CSS; use a mobile-first approach
Use Suspense for asynchronous components
Use <script setup> syntax for concise component definitions
Prefer 'lucide:' icon family as the primary choice for Iconify icons
Import Icon component from '@iconify/vue' and use with lucide icons following pattern '{collection}:{icon-name}'
Files:
src/renderer/src/components/settings/ModelConfigItem.vuesrc/renderer/settings/components/AddCustomModelButton.vuesrc/renderer/settings/components/AnthropicProviderSettingsDetail.vuesrc/renderer/settings/components/ProviderModelList.vuesrc/renderer/settings/components/ModelProviderSettingsDetail.vuesrc/renderer/settings/components/BedrockProviderSettingsDetail.vuesrc/renderer/settings/components/ProviderModelManager.vue
src/renderer/src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
src/renderer/src/**/*.{vue,ts,tsx}: All user-facing strings must use i18n keys with vue-i18n framework in the renderer
Import and use useI18n() composable with the t() function to access translations in Vue components and TypeScript files
Use the dynamic locale.value property to switch languages at runtime
Avoid hardcoding user-facing text and ensure all user-visible text uses the i18n translation system
Files:
src/renderer/src/components/settings/ModelConfigItem.vue
src/**/*
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
New features should be developed in the
srcdirectory
Files:
src/renderer/src/components/settings/ModelConfigItem.vuesrc/renderer/settings/components/AddCustomModelButton.vuesrc/renderer/settings/components/AnthropicProviderSettingsDetail.vuesrc/renderer/settings/components/ProviderModelList.vuesrc/renderer/settings/components/ModelProviderSettingsDetail.vuesrc/renderer/settings/components/BedrockProviderSettingsDetail.vuesrc/renderer/settings/components/ProviderModelManager.vue
src/renderer/**/*.{vue,js,ts}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Renderer process code should be placed in
src/renderer(Vue 3 application)
Files:
src/renderer/src/components/settings/ModelConfigItem.vuesrc/renderer/settings/components/AddCustomModelButton.vuesrc/renderer/settings/components/AnthropicProviderSettingsDetail.vuesrc/renderer/settings/components/ProviderModelList.vuesrc/renderer/settings/components/ModelProviderSettingsDetail.vuesrc/renderer/settings/components/BedrockProviderSettingsDetail.vuesrc/renderer/settings/components/ProviderModelManager.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 in Vue.js applications
Implement proper state management with Pinia in Vue.js applications
Utilize Vue Router for navigation and route management in Vue.js applications
Leverage Vue's built-in reactivity system for efficient data handling
Files:
src/renderer/src/components/settings/ModelConfigItem.vue
src/renderer/src/**/*.vue
📄 CodeRabbit inference engine (.cursor/rules/vue-best-practices.mdc)
Use scoped styles to prevent CSS conflicts between Vue components
Files:
src/renderer/src/components/settings/ModelConfigItem.vue
src/renderer/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,tsx,vue}: Write concise, technical TypeScript code with accurate examples
Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError)
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 statementsVue 3 app code in
src/renderer/srcshould be organized intocomponents/,stores/,views/,i18n/,lib/directories with shell UI insrc/renderer/shell/
Files:
src/renderer/src/components/settings/ModelConfigItem.vuesrc/renderer/settings/components/AddCustomModelButton.vuesrc/renderer/settings/components/AnthropicProviderSettingsDetail.vuesrc/renderer/settings/components/ProviderModelList.vuesrc/renderer/settings/components/ModelProviderSettingsDetail.vuesrc/renderer/settings/components/BedrockProviderSettingsDetail.vuesrc/renderer/settings/components/ProviderModelManager.vue
src/renderer/**
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
Use lowercase with dashes for directories (e.g., components/auth-wizard)
Files:
src/renderer/src/components/settings/ModelConfigItem.vuesrc/renderer/settings/components/AddCustomModelButton.vuesrc/renderer/settings/components/AnthropicProviderSettingsDetail.vuesrc/renderer/settings/components/ProviderModelList.vuesrc/renderer/settings/components/ModelProviderSettingsDetail.vuesrc/renderer/settings/components/BedrockProviderSettingsDetail.vuesrc/renderer/settings/components/ProviderModelManager.vue
src/renderer/**/*.{ts,vue}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,vue}: Use useFetch and useAsyncData for data fetching
Leverage ref, reactive, and computed for reactive state management
Use provide/inject for dependency injection when appropriate
Use Iconify/Vue for icon implementation
Files:
src/renderer/src/components/settings/ModelConfigItem.vuesrc/renderer/settings/components/AddCustomModelButton.vuesrc/renderer/settings/components/AnthropicProviderSettingsDetail.vuesrc/renderer/settings/components/ProviderModelList.vuesrc/renderer/settings/components/ModelProviderSettingsDetail.vuesrc/renderer/settings/components/BedrockProviderSettingsDetail.vuesrc/renderer/settings/components/ProviderModelManager.vue
src/renderer/src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
src/renderer/src/**/*.{ts,tsx,vue}: Use TypeScript with Vue 3 Composition API for the renderer application
All user-facing strings must use vue-i18n keys insrc/renderer/src/i18n
Files:
src/renderer/src/components/settings/ModelConfigItem.vue
src/renderer/src/components/**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
src/renderer/src/components/**/*.vue: Use Tailwind for styles in Vue components
Vue component files must use PascalCase naming (e.g.,ChatInput.vue)
Files:
src/renderer/src/components/settings/ModelConfigItem.vue
src/**/*.{ts,tsx,vue,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier with single quotes, no semicolons, and 100 character width
Files:
src/renderer/src/components/settings/ModelConfigItem.vuesrc/renderer/settings/components/AddCustomModelButton.vuesrc/renderer/settings/components/AnthropicProviderSettingsDetail.vuesrc/renderer/settings/components/ProviderModelList.vuesrc/renderer/settings/components/ModelProviderSettingsDetail.vuesrc/renderer/settings/components/BedrockProviderSettingsDetail.vuesrc/renderer/settings/components/ProviderModelManager.vue
🧠 Learnings (16)
📚 Learning: 2025-11-25T05:27:45.545Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-best-practices.mdc:0-0
Timestamp: 2025-11-25T05:27:45.545Z
Learning: Applies to src/renderer/src/**/*.{vue,ts,tsx,js,jsx} : Use the Composition API for better code organization and reusability in Vue.js applications
Applied to files:
src/renderer/src/components/settings/ModelConfigItem.vuesrc/renderer/settings/components/ProviderModelList.vue
📚 Learning: 2025-11-25T05:27:45.545Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-best-practices.mdc:0-0
Timestamp: 2025-11-25T05:27:45.545Z
Learning: Applies to src/renderer/src/**/*.{vue,ts,tsx,js,jsx} : Leverage Vue's built-in reactivity system for efficient data handling
Applied to files:
src/renderer/src/components/settings/ModelConfigItem.vuesrc/renderer/settings/components/ProviderModelList.vue
📚 Learning: 2025-11-25T05:28:04.454Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-11-25T05:28:04.454Z
Learning: Applies to src/renderer/**/*.vue : Use composition API and declarative programming patterns; avoid options API
Applied to files:
src/renderer/src/components/settings/ModelConfigItem.vuesrc/renderer/settings/components/ProviderModelList.vue
📚 Learning: 2025-11-25T05:28:04.454Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-11-25T05:28:04.454Z
Learning: Applies to src/renderer/**/*.{ts,vue} : Use Iconify/Vue for icon implementation
Applied to files:
src/renderer/src/components/settings/ModelConfigItem.vuesrc/renderer/settings/components/AddCustomModelButton.vue
📚 Learning: 2025-11-25T05:28:04.454Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-11-25T05:28:04.454Z
Learning: Applies to src/renderer/**/*.{ts,vue} : Leverage ref, reactive, and computed for reactive state management
Applied to files:
src/renderer/src/components/settings/ModelConfigItem.vuesrc/renderer/settings/components/ProviderModelList.vue
📚 Learning: 2025-11-25T05:28:04.454Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-11-25T05:28:04.454Z
Learning: Applies to src/renderer/**/*.vue : Use <script setup> syntax for concise component definitions
Applied to files:
src/renderer/src/components/settings/ModelConfigItem.vue
📚 Learning: 2025-11-25T05:28:04.454Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-11-25T05:28:04.454Z
Learning: Applies to src/renderer/**/*.vue : Use Vue 3 with TypeScript, leveraging defineComponent and PropType
Applied to files:
src/renderer/src/components/settings/ModelConfigItem.vue
📚 Learning: 2025-11-25T05:28:04.454Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-11-25T05:28:04.454Z
Learning: Applies to src/renderer/**/*.vue : Use Shadcn Vue, Radix Vue, and Tailwind for components and styling
Applied to files:
src/renderer/src/components/settings/ModelConfigItem.vue
📚 Learning: 2025-11-25T05:26:43.510Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/i18n.mdc:0-0
Timestamp: 2025-11-25T05:26:43.510Z
Learning: Applies to src/renderer/src/**/*.{vue,ts,tsx} : Import and use useI18n() composable with the t() function to access translations in Vue components and TypeScript files
Applied to files:
src/renderer/src/components/settings/ModelConfigItem.vue
📚 Learning: 2025-11-25T05:28:04.454Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-11-25T05:28:04.454Z
Learning: Applies to src/renderer/**/*.vue : Import Icon component from 'iconify/vue' and use with lucide icons following pattern '{collection}:{icon-name}'
Applied to files:
src/renderer/src/components/settings/ModelConfigItem.vue
📚 Learning: 2025-11-25T05:28:04.454Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-11-25T05:28:04.454Z
Learning: Applies to src/renderer/**/composables/*.ts : Use VueUse for common composables and utility functions
Applied to files:
src/renderer/src/components/settings/ModelConfigItem.vue
📚 Learning: 2025-11-25T05:26:43.510Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/i18n.mdc:0-0
Timestamp: 2025-11-25T05:26:43.510Z
Learning: Applies to src/renderer/src/**/*.{vue,ts,tsx} : Avoid hardcoding user-facing text and ensure all user-visible text uses the i18n translation system
Applied to files:
src/renderer/settings/components/AddCustomModelButton.vue
📚 Learning: 2025-11-25T05:28:04.454Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-11-25T05:28:04.454Z
Learning: Applies to src/renderer/**/*.vue : Structure files: exported component, composables, helpers, static content, types
Applied to files:
src/renderer/settings/components/ProviderModelList.vue
📚 Learning: 2025-11-25T05:26:11.312Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T05:26:11.312Z
Learning: Applies to src/renderer/**/*.vue : Ensure proper error handling and loading states in all UI components
Applied to files:
src/renderer/settings/components/ProviderModelList.vue
📚 Learning: 2025-11-25T05:27:39.200Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/provider-guidelines.mdc:0-0
Timestamp: 2025-11-25T05:27:39.200Z
Learning: Applies to **/*Provider**/index.ts : Do not introduce renderer dependencies inside Provider implementations
Applied to files:
src/renderer/settings/components/ProviderModelList.vuesrc/renderer/settings/components/ProviderModelManager.vue
📚 Learning: 2025-11-25T05:28:04.454Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-11-25T05:28:04.454Z
Learning: Applies to src/renderer/**/*.{ts,vue} : Use provide/inject for dependency injection when appropriate
Applied to files:
src/renderer/settings/components/ProviderModelManager.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 (12)
src/renderer/settings/components/AddCustomModelButton.vue (1)
1-59: LGTM!The component is well-structured following Vue 3 Composition API patterns with proper TypeScript typing. It correctly uses i18n for user-facing text, follows the lucide icon convention, and implements clean event handling for the dialog save flow.
src/renderer/settings/components/ProviderModelList.vue (1)
154-156: Potential edge case when providers array is empty.If
props.providersis an empty array,providers?.[0]returnsundefined, and the optional chaining?.idhandles this. However, the fallback to empty string''may cause issues ifAddCustomModelButtonexpects a valid provider ID.Consider adding a guard or showing a different UI state when no providers are available:
const newProviderModel = computed(() => { - return props.providers?.[0].id ?? '' + return props.providers?.[0]?.id ?? '' })Also verify that
AddCustomModelButtonhandles an emptyproviderIdgracefully.src/renderer/src/components/settings/ModelConfigItem.vue (1)
36-40: LGTM on Switch integration.The Switch component is properly integrated using the
model-valueandupdate:model-valuepattern, which is the correct approach for v-model binding with Shadcn components.src/renderer/settings/components/AnthropicProviderSettingsDetail.vue (1)
242-253: LGTM on ProviderModelManager integration.The updated bindings correctly pass
provider-models,custom-models, and handle thecustom-model-addedevent. The integration aligns with the new event-driven architecture introduced in this PR.src/renderer/settings/components/ModelProviderSettingsDetail.vue (2)
61-68: LGTM on ProviderModelManager integration.The component correctly passes the new props (
provider-models,custom-models,is-model-list-loading) and handles thecustom-model-addedevent to refresh data.
415-418: Good pattern: immediate refresh after model save.Calling both
refreshCustomModelsandinitDataImmediateensures the UI reflects the newly added model immediately without debounce delay.src/renderer/settings/components/ProviderModelManager.vue (2)
2-13: LGTM! Clean sticky header implementation.The sticky header with backdrop blur and model count display provides good UX. The
z-30index should work fine for this use case.
30-53: LGTM! Props and emits are well-defined.The new props (
providerModels,customModels,isModelListLoading) and thecustom-model-addedevent are correctly typed and follow Vue 3 Composition API best practices.src/renderer/settings/components/BedrockProviderSettingsDetail.vue (4)
100-110: LGTM! ProviderModelManager integration is correct.The component correctly passes
provider-models,custom-models, and handles the newcustom-model-addedevent withhandleAddModelSaved. Thetotal-models-countcalculation as a simple sum is appropriate.
164-169: LGTM! Clean computed property implementation.The
customModelscomputed property correctly filters custom models for the current provider with a safe fallback to an empty array.
177-191: Verify deduplication priority: custom models shadow built-in models.The deduplication logic gives priority to custom models over built-in models when they share the same ID (since
enabledCustomis merged first and the Map only keeps the first occurrence).Please confirm this is the intended behavior. If built-in models should take precedence, reverse the merge order or adjust the deduplication logic.
332-335: LGTM! Clean event handler implementation.The
handleAddModelSavedhandler correctly refreshes custom models and reinitializes data after a custom model is added.
src/renderer/settings/components/ModelProviderSettingsDetail.vue
Outdated
Show resolved
Hide resolved
|
@codex review |
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
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
🧹 Nitpick comments (5)
src/renderer/settings/components/ModelProviderSettingsDetail.vue (2)
139-142: initData/loading refactor is sound; behavior matches requirements
- Using
isModelListLoading/hasInitializedModelListas refs gives per-instance, reactive control over the loading indicator and avoids stale module-level flags.- First run shows the loading state, while later refreshes (watchers, OAuth, custom-model saves) skip the spinner but still refresh data, which is a reasonable UX tradeoff.
- Azure API version and Gemini safety settings fetches are wrapped in try/catch with sensible local defaults, so errors won’t break the component.
handleAddModelSavedforcing a one-off immediate_initDataafterrefreshCustomModelsensures newly added models appear right away; the existing store watchers may trigger an extra throttled refresh, but that’s just minor redundancy, not a correctness issue.Overall this block looks correct and aligns with the loading/error-handling guidance.
Based on learnings, this is a good application of reactive state + loading guards; if you want to micro-optimize later, you could rely solely on the
customModelswatcher or onhandleAddModelSaved(but not both) to avoid double-calling_initDataon custom-model changes.Also applies to: 197-265, 415-418
235-238: Use English for the new Gemini safety commentThe inline comment here is in Chinese; new comments should be in English per repo guidelines.
- // 先清空现有数据 + // Clear existing Gemini safety levels before reloadingAs per coding guidelines, new logs/comments should use English.
src/renderer/settings/components/ProviderModelList.vue (3)
14-15: Guard AddCustomModelButton when no providers and clarify computed namingRight now
newProviderModelreturns an empty string whenprops.providersis empty, but the button still renders and will pass''asprovider-id. IfAddCustomModelButtonexpects a valid provider id, this is fragile and also a bit confusingly named.Consider:
- Renaming the computed to reflect what it is (e.g.
defaultProviderId).- Only rendering the button when you actually have a provider id.
For example:
-const isLoading = computed(() => props.isLoading ?? false) -const newProviderModel = computed(() => { - return props.providers?.[0].id ?? '' -}) +const isLoading = computed(() => props.isLoading ?? false) +const defaultProviderId = computed(() => props.providers?.[0]?.id ?? '')- <AddCustomModelButton :provider-id="newProviderModel" @saved="$emit('config-changed')" /> + <AddCustomModelButton + v-if="defaultProviderId" + :provider-id="defaultProviderId" + @saved="$emit('config-changed')" + />This avoids passing an invalid id and makes the intent clearer.
Also applies to: 149-157
25-43: Virtualized provider/custom model rendering is solid; consider small cleanups
- The split between a non-virtualized custom-model block and a virtualized official/provider list (
DynamicScroller+VirtualModelListItem) is coherent, and the search filters for both provider and custom models by name/id behave as expected.getItemSizeDependenciescovers the right fields so the scroller can react when model properties or provider model counts change.- Minor readability nit:
filteredCustomModelsis currently implemented with manual loops; afilter-based implementation with a precomputedquery = modelSearchQuery.value.toLowerCase().trim()would be more concise while preserving behavior.One place that could be safer:
- In the official-model branch (
:is-custom-model="false"), you still attach@delete-model="() => handleDeleteCustomModel(item.model)", which callsremoveCustomModel. IfModelConfigItemnever emitsdelete-modelfor non-custom models this is effectively dead, but if that ever changes you would be routing deletions for official models through the custom-model removal path. It may be clearer to drop the@delete-modelbinding in this branch (or wire it to a distinct handler) and keep deletion strictly for actual custom models.Also applies to: 45-117, 165-180, 182-202, 204-244, 246-268, 275-295
307-332: Remove or wire up unused stickyHeaderInfo/updateStickyHeader/watch
stickyHeaderInfo,updateStickyHeader, and thewatch(virtualItems, ...)block are currently unused: nothing in the template or script readsstickyHeaderInfo. This adds reactive work without affecting the UI.Unless you plan to immediately hook this into a sticky provider header, consider removing this block to keep the component lean; you can always reintroduce it when the feature is implemented.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/renderer/settings/components/ModelProviderSettingsDetail.vue(4 hunks)src/renderer/settings/components/ProviderModelList.vue(3 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for logs and comments (Chinese text exists in legacy code, but new code should use English)
Files:
src/renderer/settings/components/ProviderModelList.vuesrc/renderer/settings/components/ModelProviderSettingsDetail.vue
**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.vue: Use Vue 3 Composition API for all components instead of Options API
Use Tailwind CSS with scoped styles for component styling
Files:
src/renderer/settings/components/ProviderModelList.vuesrc/renderer/settings/components/ModelProviderSettingsDetail.vue
src/renderer/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
src/renderer/**/*.vue: All user-facing strings must use i18n keys via vue-i18n for internationalization
Ensure proper error handling and loading states in all UI components
Implement responsive design using Tailwind CSS utilities for all UI components
src/renderer/**/*.vue: Use composition API and declarative programming patterns; avoid options API
Structure files: exported component, composables, helpers, static content, types
Use PascalCase for component names (e.g., AuthWizard.vue)
Use Vue 3 with TypeScript, leveraging defineComponent and PropType
Use template syntax for declarative rendering
Use Shadcn Vue, Radix Vue, and Tailwind for components and styling
Implement responsive design with Tailwind CSS; use a mobile-first approach
Use Suspense for asynchronous components
Use <script setup> syntax for concise component definitions
Prefer 'lucide:' icon family as the primary choice for Iconify icons
Import Icon component from '@iconify/vue' and use with lucide icons following pattern '{collection}:{icon-name}'
Files:
src/renderer/settings/components/ProviderModelList.vuesrc/renderer/settings/components/ModelProviderSettingsDetail.vue
src/**/*
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
New features should be developed in the
srcdirectory
Files:
src/renderer/settings/components/ProviderModelList.vuesrc/renderer/settings/components/ModelProviderSettingsDetail.vue
src/renderer/**/*.{vue,js,ts}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Renderer process code should be placed in
src/renderer(Vue 3 application)
Files:
src/renderer/settings/components/ProviderModelList.vuesrc/renderer/settings/components/ModelProviderSettingsDetail.vue
src/renderer/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,tsx,vue}: Write concise, technical TypeScript code with accurate examples
Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError)
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 statementsVue 3 app code in
src/renderer/srcshould be organized intocomponents/,stores/,views/,i18n/,lib/directories with shell UI insrc/renderer/shell/
Files:
src/renderer/settings/components/ProviderModelList.vuesrc/renderer/settings/components/ModelProviderSettingsDetail.vue
src/renderer/**
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
Use lowercase with dashes for directories (e.g., components/auth-wizard)
Files:
src/renderer/settings/components/ProviderModelList.vuesrc/renderer/settings/components/ModelProviderSettingsDetail.vue
src/renderer/**/*.{ts,vue}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,vue}: Use useFetch and useAsyncData for data fetching
Leverage ref, reactive, and computed for reactive state management
Use provide/inject for dependency injection when appropriate
Use Iconify/Vue for icon implementation
Files:
src/renderer/settings/components/ProviderModelList.vuesrc/renderer/settings/components/ModelProviderSettingsDetail.vue
src/**/*.{ts,tsx,vue,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier with single quotes, no semicolons, and 100 character width
Files:
src/renderer/settings/components/ProviderModelList.vuesrc/renderer/settings/components/ModelProviderSettingsDetail.vue
🧠 Learnings (8)
📚 Learning: 2025-11-25T05:27:45.545Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-best-practices.mdc:0-0
Timestamp: 2025-11-25T05:27:45.545Z
Learning: Applies to src/renderer/src/**/*.{vue,ts,tsx,js,jsx} : Use the Composition API for better code organization and reusability in Vue.js applications
Applied to files:
src/renderer/settings/components/ProviderModelList.vue
📚 Learning: 2025-11-25T05:28:04.454Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-11-25T05:28:04.454Z
Learning: Applies to src/renderer/**/*.vue : Structure files: exported component, composables, helpers, static content, types
Applied to files:
src/renderer/settings/components/ProviderModelList.vue
📚 Learning: 2025-11-25T05:26:11.312Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T05:26:11.312Z
Learning: Applies to src/renderer/**/*.vue : Ensure proper error handling and loading states in all UI components
Applied to files:
src/renderer/settings/components/ProviderModelList.vuesrc/renderer/settings/components/ModelProviderSettingsDetail.vue
📚 Learning: 2025-11-25T05:27:45.545Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-best-practices.mdc:0-0
Timestamp: 2025-11-25T05:27:45.545Z
Learning: Applies to src/renderer/src/**/*.{vue,ts,tsx,js,jsx} : Leverage Vue's built-in reactivity system for efficient data handling
Applied to files:
src/renderer/settings/components/ProviderModelList.vue
📚 Learning: 2025-11-25T05:27:39.200Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/provider-guidelines.mdc:0-0
Timestamp: 2025-11-25T05:27:39.200Z
Learning: Applies to **/*Provider**/index.ts : Do not introduce renderer dependencies inside Provider implementations
Applied to files:
src/renderer/settings/components/ProviderModelList.vue
📚 Learning: 2025-11-25T05:28:04.454Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-11-25T05:28:04.454Z
Learning: Applies to src/renderer/**/*.vue : Use composition API and declarative programming patterns; avoid options API
Applied to files:
src/renderer/settings/components/ProviderModelList.vue
📚 Learning: 2025-11-25T05:28:04.454Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-11-25T05:28:04.454Z
Learning: Applies to src/renderer/**/*.{ts,vue} : Leverage ref, reactive, and computed for reactive state management
Applied to files:
src/renderer/settings/components/ProviderModelList.vuesrc/renderer/settings/components/ModelProviderSettingsDetail.vue
📚 Learning: 2025-11-25T05:28:04.454Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-11-25T05:28:04.454Z
Learning: Applies to src/renderer/**/*.{ts,tsx,vue} : Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError)
Applied to files:
src/renderer/settings/components/ModelProviderSettingsDetail.vue
🪛 GitHub Actions: PR Check
src/renderer/settings/components/ProviderModelList.vue
[warning] 1-1: Code style issues found by Prettier. Run 'prettier --write' to fix formatting.
🔇 Additional comments (2)
src/renderer/settings/components/ModelProviderSettingsDetail.vue (1)
57-68: ProviderModelManager wiring for models and loading state looks consistentPassing
providerModels,customModels, andisModelListLoadingplus handlingcustom-model-addedviahandleAddModelSavedmatches the local state shape and should give the manager everything it needs to render and refresh correctly. No issues from this side.src/renderer/settings/components/ProviderModelList.vue (1)
2-7: Sticky search/custom headers and offsets look correctThe sticky positioning for the search bar and custom-model label, combined with
stickyOffsetand the measuredsearchContainerHeight, gives a coherent layout across different shells and avoids overlap when there are no custom models. I don’t see functional issues here; just ensure in the UI that varyingstickyOffsetvalues and narrow viewports still keep the headers aligned as expected.Also applies to: 17-24, 163-163, 297-305
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 (2)
src/renderer/settings/components/ProviderModelList.vue (2)
290-298: Use English for comments in new code.Per coding guidelines, new code should use English for comments. These Chinese comments should be translated.
-// 启用提供商下所有模型 +// Enable all models for the provider const enableAllModels = (providerId: string) => { modelStore.enableAllModels(providerId) } -// 禁用提供商下所有模型 +// Disable all models for the provider const disableAllModels = (providerId: string) => { modelStore.disableAllModels(providerId) }
310-335: Remove unused sticky header tracking code.
stickyHeaderInfo,updateStickyHeader, and the associated watcher compute state that is never consumed in the template or elsewhere. This appears to be dead code that should be removed to reduce complexity.-const stickyHeaderInfo = ref<{ - provider?: { providerId: string } -}>({}) - -const updateStickyHeader = (startIndex: number) => { - if (startIndex < 0 || startIndex >= virtualItems.value.length) return - - const currentItem = virtualItems.value[startIndex] - let providerItem: { providerId: string } | undefined - - if (currentItem.type === 'model' || currentItem.type === 'provider-actions') { - providerItem = { providerId: currentItem.providerId } - } - - stickyHeaderInfo.value = { - provider: providerItem - } -} - -watch( - virtualItems, - () => { - updateStickyHeader(0) - }, - { immediate: true } -)If this is intended for a future feature, consider adding a
TODOcomment or implementing it fully.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/renderer/settings/components/ProviderModelList.vue(3 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for logs and comments (Chinese text exists in legacy code, but new code should use English)
Files:
src/renderer/settings/components/ProviderModelList.vue
**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.vue: Use Vue 3 Composition API for all components instead of Options API
Use Tailwind CSS with scoped styles for component styling
Files:
src/renderer/settings/components/ProviderModelList.vue
src/renderer/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
src/renderer/**/*.vue: All user-facing strings must use i18n keys via vue-i18n for internationalization
Ensure proper error handling and loading states in all UI components
Implement responsive design using Tailwind CSS utilities for all UI components
src/renderer/**/*.vue: Use composition API and declarative programming patterns; avoid options API
Structure files: exported component, composables, helpers, static content, types
Use PascalCase for component names (e.g., AuthWizard.vue)
Use Vue 3 with TypeScript, leveraging defineComponent and PropType
Use template syntax for declarative rendering
Use Shadcn Vue, Radix Vue, and Tailwind for components and styling
Implement responsive design with Tailwind CSS; use a mobile-first approach
Use Suspense for asynchronous components
Use <script setup> syntax for concise component definitions
Prefer 'lucide:' icon family as the primary choice for Iconify icons
Import Icon component from '@iconify/vue' and use with lucide icons following pattern '{collection}:{icon-name}'
Files:
src/renderer/settings/components/ProviderModelList.vue
src/**/*
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
New features should be developed in the
srcdirectory
Files:
src/renderer/settings/components/ProviderModelList.vue
src/renderer/**/*.{vue,js,ts}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Renderer process code should be placed in
src/renderer(Vue 3 application)
Files:
src/renderer/settings/components/ProviderModelList.vue
src/renderer/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,tsx,vue}: Write concise, technical TypeScript code with accurate examples
Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError)
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 statementsVue 3 app code in
src/renderer/srcshould be organized intocomponents/,stores/,views/,i18n/,lib/directories with shell UI insrc/renderer/shell/
Files:
src/renderer/settings/components/ProviderModelList.vue
src/renderer/**
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
Use lowercase with dashes for directories (e.g., components/auth-wizard)
Files:
src/renderer/settings/components/ProviderModelList.vue
src/renderer/**/*.{ts,vue}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,vue}: Use useFetch and useAsyncData for data fetching
Leverage ref, reactive, and computed for reactive state management
Use provide/inject for dependency injection when appropriate
Use Iconify/Vue for icon implementation
Files:
src/renderer/settings/components/ProviderModelList.vue
src/**/*.{ts,tsx,vue,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier with single quotes, no semicolons, and 100 character width
Files:
src/renderer/settings/components/ProviderModelList.vue
🧠 Learnings (8)
📚 Learning: 2025-11-25T05:27:45.545Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-best-practices.mdc:0-0
Timestamp: 2025-11-25T05:27:45.545Z
Learning: Applies to src/renderer/src/**/*.{vue,ts,tsx,js,jsx} : Use the Composition API for better code organization and reusability in Vue.js applications
Applied to files:
src/renderer/settings/components/ProviderModelList.vue
📚 Learning: 2025-11-25T05:28:04.454Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-11-25T05:28:04.454Z
Learning: Applies to src/renderer/**/*.vue : Structure files: exported component, composables, helpers, static content, types
Applied to files:
src/renderer/settings/components/ProviderModelList.vue
📚 Learning: 2025-11-25T05:27:39.200Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/provider-guidelines.mdc:0-0
Timestamp: 2025-11-25T05:27:39.200Z
Learning: Applies to **/*Provider**/index.ts : Do not introduce renderer dependencies inside Provider implementations
Applied to files:
src/renderer/settings/components/ProviderModelList.vue
📚 Learning: 2025-11-25T05:28:04.454Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-11-25T05:28:04.454Z
Learning: Applies to src/renderer/**/*.vue : Use composition API and declarative programming patterns; avoid options API
Applied to files:
src/renderer/settings/components/ProviderModelList.vue
📚 Learning: 2025-11-25T05:26:11.312Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T05:26:11.312Z
Learning: Applies to src/renderer/**/*.vue : Ensure proper error handling and loading states in all UI components
Applied to files:
src/renderer/settings/components/ProviderModelList.vue
📚 Learning: 2025-11-25T05:27:45.545Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-best-practices.mdc:0-0
Timestamp: 2025-11-25T05:27:45.545Z
Learning: Applies to src/renderer/src/**/*.{vue,ts,tsx,js,jsx} : Leverage Vue's built-in reactivity system for efficient data handling
Applied to files:
src/renderer/settings/components/ProviderModelList.vue
📚 Learning: 2025-11-25T05:28:04.454Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-11-25T05:28:04.454Z
Learning: Applies to src/renderer/**/*.{ts,vue} : Use provide/inject for dependency injection when appropriate
Applied to files:
src/renderer/settings/components/ProviderModelList.vue
📚 Learning: 2025-11-25T05:28:04.454Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-11-25T05:28:04.454Z
Learning: Applies to src/renderer/**/*.{ts,vue} : Use Iconify/Vue for icon implementation
Applied to files:
src/renderer/settings/components/ProviderModelList.vue
🔇 Additional comments (1)
src/renderer/settings/components/ProviderModelList.vue (1)
45-124: LGTM on the virtualization approach and conditional rendering.The loading state, virtual scroller, and empty state conditions are well-structured. The fix for the empty-state logic (
v-else-if="filteredCustomModels.length === 0") correctly prevents showing the "no models" message when custom models exist but no provider models match the search.
|
LGTM |
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| :provider-models="[{ providerId: provider.id, models: providerModels }]" | ||
| :custom-models="customModels" | ||
| :providers="[{ id: provider.id, name: provider.name }]" | ||
| @enabled-change="(model, enabled) => $emit('model-enabled-change', model, enabled)" | ||
| @saved="$emit('custom-model-added')" |
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.
Custom model save event never fires
ProviderModelManager listens for a @saved event from ProviderModelList to emit custom-model-added, but ProviderModelList declares only enabledChange/config-changed emits and never emits saved. This breaks template type-checking (onSaved is not a known event) and prevents parent handlers such as handleAddModelSaved from running after a custom model is added, so the intended refresh logic (refreshCustomModels/initDataImmediate) is skipped.
Useful? React with 👍 / 👎.
20251215_132801.mp4
Summary by CodeRabbit
New Features
UI/UX Improvements
✏️ Tip: You can customize this high-level summary in your review settings.