fix: make model select choose right model when pinned thread #914
Conversation
WalkthroughRefactors NewThread.vue’s model-selection logic: adds one-time async initialization on mount, helper functions for choosing an enabled model, restores from recent threads or preferred setting, falls back to first enabled model, and adds a focused watcher on enabledModels to validate or replace the active model post-initialization. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as NewThread.vue
participant S as ChatStore
participant E as EnabledModels
participant P as ConfigPresenter
participant A as ActiveModel
rect rgb(240,245,255)
note over C: onMounted
C->>C: initActiveModel()
alt Match from recent threads
C->>S: Get most recent model (pinned/normal)
C->>E: Check if model is enabled
E-->>C: Enabled?
C->>A: setActiveFromEnabled(model)
else Restore preferred model
C->>P: getSetting('preferredModel')
C->>E: Check if preferred is enabled
E-->>C: Enabled?
C->>A: setActiveFromEnabled(model)
else Fallback
C->>E: pickFirstEnabledModel(Chat/ImageGeneration)
C->>A: setActiveFromEnabled(model)
end
note over C: mark initialized
end
rect rgb(245,255,245)
note over C,E: Watch enabledModels (immediate: false)
C->>C: if not initialized → initActiveModel()
C->>E: Validate current active model exists
alt Active missing
C->>E: pickFirstEnabledModel(...)
C->>A: setActiveFromEnabled(model)
else Active valid
note over C: No change
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
![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
🧹 Nitpick comments (10)
src/renderer/src/components/NewThread.vue (10)
248-249: Guard against undefined dtThreads in predicate
g.dtThreads.lengthcan throw ifdtThreadsis undefined. Use optional chaining.-const normalGroup = chatStore.threads.find((g) => g.dt !== 'Pinned' && g.dtThreads.length > 0) +const normalGroup = chatStore.threads.find( + (g) => g.dt !== 'Pinned' && (g.dtThreads?.length ?? 0) > 0 +)
207-218: Type the return of findEnabledModel for strictnessAdd a precise return type (avoid implicit any).
-const findEnabledModel = (providerId: string, modelId: string) => { +const findEnabledModel = ( + providerId: string, + modelId: string +): { model: MODEL_META; providerId: string } | undefined => {
220-226: Prefer Chat models first, then Image; add return typeAligns with typical UX expectations and adds typing.
-const pickFirstEnabledModel = () => { - const found = settingsStore.enabledModels - .flatMap((p) => p.models.map((m) => ({ ...m, providerId: p.providerId }))) - .find((m) => m.type === ModelType.Chat || m.type === ModelType.ImageGeneration) - return found -} +const pickFirstEnabledModel = (): + | (MODEL_META & { providerId: string }) + | undefined => { + const flat = settingsStore.enabledModels.flatMap((p) => + p.models.map((m) => ({ ...m, providerId: p.providerId })) + ) + return flat.find((m) => m.type === ModelType.Chat) ?? + flat.find((m) => m.type === ModelType.ImageGeneration) +}
227-240: Preserve model tags in activeModelUI shows badges from
activeModel.tags, but this setter drops them.- activeModel.value = { + activeModel.value = { name: m.name, id: m.id, providerId: m.providerId, - tags: [], + tags: (m as any).tags ?? [], type: m.type ?? ModelType.Chat }If
MODEL_METAexposestags, tighten the param type instead of(m as any).
204-206: Avoid re-entrant init races
initActiveModelcan be invoked concurrently (watch + mount). Add aninitializingguard with try/finally; skip work in the watcher if initializing.-const initialized = ref(false) +const initialized = ref(false) +const initializing = ref(false) @@ -const initActiveModel = async () => { +const initActiveModel = async () => { if (initialized.value) return + if (initializing.value) return + initializing.value = true + try { @@ - initialized.value = true + initialized.value = true @@ - } + } finally { + initializing.value = false + } } @@ watch( () => settingsStore.enabledModels, async () => { - if (!initialized.value) { + if (!initialized.value) { + if (initializing.value) return await initActiveModel() return }Also applies to: 242-289, 290-315
244-246: Translate inline comments to English per codebase standardsComments here are in Chinese; guidelines require English for all comments/logs.
Example:
-// 1) 尝试根据最近会话（区分 pinned/非 pinned）选择 +// 1) Try recent threads (prefer pinned vs. non‑pinned)Also applies to: 265-266, 282-293
15-16: Add accessible labels and i18n for icon/imagery
- Image lacks alt text.
- Icon-only buttons need aria-labels (i18n).
-<img src="@/assets/logo-dark.png" class="w-24 h-24" loading="lazy" /> +<img src="@/assets/logo-dark.png" class="w-24 h-24" loading="lazy" :alt="t('app.logoAlt')" /> @@ -<Button +<Button variant="outline" class="flex border-none rounded-none shadow-none items-center gap-1.5 px-2 h-full" size="sm" + :aria-label="t('a11y.selectModel')" > @@ -<Button +<Button class="w-7 h-full rounded-none border-none shadow-none transition-all duration-300" :class="{ 'w-0 opacity-0 p-0 overflow-hidden': !showSettingsButton && !isHovering, 'w-7 opacity-100': showSettingsButton || isHovering }" size="icon" variant="outline" + :aria-label="t('a11y.settings')" >If these keys don’t exist, I can add them to i18n.
Also applies to: 36-40, 75-86
4-12: Sidebar toggle button: add aria-label (i18n)Icon-only control; screen readers need a label.
-<Button +<Button class="w-7 h-7 rounded-md" size="icon" variant="outline" @click="onSidebarButtonClick" + :aria-label="t(chatStore.isSidebarOpen ? 'a11y.closeSidebar' : 'a11y.openSidebar')" >
139-143: Prefer type alias over interface (project guideline)Convert
interface PreferredModelto atypealias.-interface PreferredModel { - modelId: string - providerId: string -} +type PreferredModel = { + modelId: string + providerId: string +}
471-475: Avoidas anywhen creating thread; tighten typesPass a well-typed options object; avoid
any.- } as any) + } as CreateThreadOptions)If
CreateThreadOptionsdoesn’t exist, define/export it wherecreateThreadis declared and import here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/renderer/src/components/NewThread.vue(2 hunks)
🧰 Additional context used
📓 Path-based instructions (15)
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/NewThread.vue
src/renderer/**/*.{vue,ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
渲染进程代码放在
src/renderer
Files:
src/renderer/src/components/NewThread.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/NewThread.vue
src/renderer/src/**/*.vue
📄 CodeRabbit inference engine (.cursor/rules/vue-best-practices.mdc)
Use scoped styles to prevent CSS conflicts between components
Files:
src/renderer/src/components/NewThread.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/NewThread.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/NewThread.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.Use Pinia for frontend state management (do not introduce alternative state libraries)
Files:
src/renderer/src/components/NewThread.vue
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/renderer/src/components/NewThread.vue
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Files:
src/renderer/src/components/NewThread.vue
src/renderer/{src,shell,floating}/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
src/renderer/{src,shell,floating}/**/*.vue: Use Vue 3 Composition API for all components
All user-facing strings must use i18n keys via vue-i18n (no hard-coded UI strings)
Use Tailwind CSS utilities and ensure styles are scoped in Vue components
Files:
src/renderer/src/components/NewThread.vue
src/renderer/src/components/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Organize UI components by feature within src/renderer/src/
Files:
src/renderer/src/components/NewThread.vue
src/renderer/src/**
📄 CodeRabbit inference engine (AGENTS.md)
Put application code for the Vue app under src/renderer/src (components, stores, views, i18n, lib)
Files:
src/renderer/src/components/NewThread.vue
src/renderer/src/**/*.{vue,ts}
📄 CodeRabbit inference engine (AGENTS.md)
All user-facing strings in the renderer must use vue-i18n keys defined in src/renderer/src/i18n
Files:
src/renderer/src/components/NewThread.vue
**/*.{js,jsx,ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Prettier formatting: single quotes, no semicolons, max width 100
Files:
src/renderer/src/components/NewThread.vue
src/renderer/**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
Name Vue components in PascalCase (e.g., ChatInput.vue)
Files:
src/renderer/src/components/NewThread.vue
🧠 Learnings (1)
📚 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 composition API and declarative programming patterns; avoid options API.
Applied to files:
src/renderer/src/components/NewThread.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)
| const initActiveModel = async () => { | ||
| if (initialized.value) return | ||
| // 1) 尝试根据最近会话（区分 pinned/非 pinned）选择 | ||
| if (chatStore.threads.length > 0) { | ||
| const pinnedGroup = chatStore.threads.find((g) => g.dt === 'Pinned') | ||
| const pinnedFirst = pinnedGroup?.dtThreads?.[0] | ||
| const normalGroup = chatStore.threads.find((g) => g.dt !== 'Pinned' && g.dtThreads.length > 0) | ||
| const normalFirst = normalGroup?.dtThreads?.[0] | ||
| const candidate = [pinnedFirst, normalFirst] | ||
| .filter(Boolean) | ||
| .sort((a, b) => (b!.updatedAt || 0) - (a!.updatedAt || 0))[0] as | ||
| | typeof pinnedFirst | ||
| | undefined | ||
| if (candidate?.settings?.modelId && candidate?.settings?.providerId) { | ||
| const match = findEnabledModel(candidate.settings.providerId, candidate.settings.modelId) | ||
| if (match) { | ||
| setActiveFromEnabled({ ...match.model, providerId: match.providerId }) | ||
| initialized.value = true | ||
| return | ||
| } | ||
| } catch (error) { | ||
| console.warn('Failed to get user preferred model:', error) | ||
| } | ||
| } | ||
|
|
||
| // 如果没有偏好模型或偏好模型不可用，使用第一个可用模型 | ||
| if (settingsStore.enabledModels.length > 0) { | ||
| const model = settingsStore.enabledModels | ||
| .flatMap((provider) => | ||
| provider.models.map((m) => ({ ...m, providerId: provider.providerId })) | ||
| ) | ||
| .find((m) => m.type === ModelType.Chat || m.type === ModelType.ImageGeneration) | ||
| if (model) { | ||
| activeModel.value = { | ||
| name: model.name, | ||
| id: model.id, | ||
| providerId: model.providerId, | ||
| tags: [], | ||
| type: model.type ?? ModelType.Chat | ||
| } | ||
| // 2) 尝试用户上次选择的偏好模型 | ||
| try { | ||
| const preferredModel = (await configPresenter.getSetting('preferredModel')) as | ||
| | PreferredModel | ||
| | undefined | ||
| if (preferredModel?.modelId && preferredModel?.providerId) { | ||
| const match = findEnabledModel(preferredModel.providerId, preferredModel.modelId) | ||
| if (match) { | ||
| setActiveFromEnabled({ ...match.model, providerId: match.providerId }) | ||
| initialized.value = true | ||
| return | ||
| } | ||
| } | ||
| } catch (error) { | ||
| console.warn('Failed to get user preferred model:', error) | ||
| } | ||
|
|
||
| // 3) 选择第一个可用模型 | ||
| const first = pickFirstEnabledModel() | ||
| if (first) { | ||
| setActiveFromEnabled(first) | ||
| initialized.value = true | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Do not overwrite user/deeplink model selection on mount
If a deeplink (or user action) sets the model before mount, await initActiveModel() can still override it because initialized isn’t flipped in handleModelUpdate. Add an early-return in initActiveModel when a valid activeModel already exists, and mark initialized in handleModelUpdate.
Apply this diff:
@@
-const initActiveModel = async () => {
+const initActiveModel = async () => {
if (initialized.value) return
+ // Respect an already-selected model (e.g., via deeplink or user action)
+ if (activeModel.value?.id && activeModel.value?.providerId) {
+ const exists = findEnabledModel(activeModel.value.providerId, activeModel.value.id)
+ if (exists) {
+ initialized.value = true
+ return
+ }
+ }
@@
const handleModelUpdate = (model: MODEL_META, providerId: string) => {
activeModel.value = {
@@
}
chatStore.updateChatConfig({
@@
})
@@
configPresenter.setSetting('preferredModel', {
@@
})
+
+ // Prevent init flow from overriding an explicit selection
+ initialized.value = true
@@
}
@@
onMounted(async () => {
@@
- // 组件激活时初始化一次默认模型
+ // Initialize default model once on component activation (if none selected yet)
await initActiveModel()Also applies to: 335-355, 426-427
🤖 Prompt for AI Agents
In src/renderer/src/components/NewThread.vue around lines 242-289 (also apply
same change at 335-355 and 426-427), initActiveModel can override a model set
via deeplink/user action because handleModelUpdate never flips initialized; add
an early return at the start of initActiveModel that returns if activeModel is
already set and valid (has providerId and modelId), and update handleModelUpdate
to set initialized.value = true when it applies a user/deeplink model (use the
same validity check) so initActiveModel won't overwrite it on mount.
Fixes an issue where the correct model may not be selected when there is a pinned conversation. Optimizes new conversation performance.
Summary by CodeRabbit