fix: sync provider order changes across all tabs #765
Conversation
WalkthroughAdds saved provider ordering application on provider change and persists provider ordering updates. Enhances logging for provider change events. The settings store now calls loadSavedOrder() before refreshing models and saves the updated providers list via configP.setProviders() after order changes. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant SettingsStore
participant Config as configP
participant Models as ModelRefresher
Note over App,SettingsStore: Provider changed event
App->>SettingsStore: CONFIG_EVENTS.PROVIDER_CHANGED
SettingsStore->>SettingsStore: loadSavedOrder()
SettingsStore->>Models: refreshModels()
Note over App,SettingsStore: Manual provider reordering
App->>SettingsStore: updateProvidersOrder(newOrder)
SettingsStore->>SettingsStore: update in-memory providers
SettingsStore->>Config: setProviders(providers.value)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/stores/settings.ts (1)
928-933: Fix identical branches: disabled providers aren’t moved to the disabled segmentBoth branches set the same newOrder, so toggling enable=false won’t push the provider into the disabled block. This breaks expected ordering behavior.
Apply this diff:
- if (enable) { - newOrder = [...enabledInOrder, providerId, ...disabledInOrder] - } else { - newOrder = [...enabledInOrder, providerId, ...disabledInOrder] - } + if (enable) { + // Move into the enabled segment + newOrder = [...enabledInOrder, providerId, ...disabledInOrder] + } else { + // Move into the disabled segment (after all currently enabled+disabled) + newOrder = [...enabledInOrder, ...disabledInOrder, providerId] + }
🧹 Nitpick comments (2)
src/renderer/src/stores/settings.ts (2)
1516-1519: Prefer emitting an explicit “order changed” event over rewriting providersUsing setProviders() as a broadcast mechanism is heavy and risks unnecessary churn. Consider emitting a dedicated CONFIG_EVENTS.PROVIDER_ORDER_CHANGED (or a generic SETTING_CHANGED for key 'providerOrder') from configPresenter when setSetting('providerOrder', ...) is called, and listen for that in the renderer. It keeps concerns separated and avoids rewriting provider configs just to notify other windows.
371-373: De-duplicate listener registration for setupProviderListenersetupProviderListener() is invoked inside initSettings() (Line 372) and again in onMounted() (Lines 1320–1321), which can register duplicate IPC handlers and cause double refreshes. Make it idempotent or call it only once.
Minimal change:
onMounted(async () => { await initSettings() - await setupProviderListener() + // setupProviderListener() is already called inside initSettings() })Also applies to: 1318-1321
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/renderer/src/stores/settings.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for logs and comments
Files:
src/renderer/src/stores/settings.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Strict type checking enabled for TypeScript
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪（如适用）、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别
Files:
src/renderer/src/stores/settings.ts
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/stores/settings.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
Files:
src/renderer/src/stores/settings.ts
src/{main,renderer}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
src/{main,renderer}/**/*.ts: Use context isolation for improved security
Implement proper inter-process communication (IPC) patterns
Optimize application startup time with lazy loading
Implement proper error handling and logging for debugging
Files:
src/renderer/src/stores/settings.ts
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/stores/settings.ts
src/renderer/src/stores/**/*.{vue,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/pinia-best-practices.mdc)
src/renderer/src/stores/**/*.{vue,ts,tsx,js,jsx}: Use modules to organize related state and actions
Implement proper state persistence for maintaining data across sessions
Use getters for computed state properties
Utilize actions for side effects and asynchronous operations
Keep the store focused on global state, not component-specific data
Files:
src/renderer/src/stores/settings.ts
src/renderer/**/*.{vue,ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
渲染进程代码放在
src/renderer
Files:
src/renderer/src/stores/settings.ts
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/stores/settings.ts
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/stores/settings.ts
src/renderer/**/*.{vue,ts}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
Implement lazy loading for routes and components.
Files:
src/renderer/src/stores/settings.ts
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/stores/settings.ts
🧠 Learnings (1)
📚 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/main/presenter/configPresenter/providers.ts : Add provider configuration in configPresenter/providers.ts when adding a new LLM provider
Applied to files:
src/renderer/src/stores/settings.ts
🔇 Additional comments (1)
src/renderer/src/stores/settings.ts (1)
656-661: Good: apply saved provider order before refreshing modelsCalling loadSavedOrder() on PROVIDER_CHANGED ensures the current tab reflects the persisted ordering before model refresh. This is aligned with the PR goal to keep tabs in sync.
| window.electron.ipcRenderer.on(CONFIG_EVENTS.PROVIDER_CHANGED, async () => { | ||
| console.log('changed') | ||
| console.log('Provider changed - updating providers and order') | ||
| providers.value = await configP.getProviders() | ||
| await loadSavedOrder() | ||
| await refreshAllModels() | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Wrap async IPC handler in try–catch and use structured logging
Per the repo’s TS guidelines, guard presenter calls with try–catch and log with context to avoid unhandled rejections from IPC failures.
Apply this diff:
- window.electron.ipcRenderer.on(CONFIG_EVENTS.PROVIDER_CHANGED, async () => {
- console.log('Provider changed - updating providers and order')
- providers.value = await configP.getProviders()
- await loadSavedOrder()
- await refreshAllModels()
- })
+ window.electron.ipcRenderer.on(CONFIG_EVENTS.PROVIDER_CHANGED, async () => {
+ try {
+ console.info('[settings][PROVIDER_CHANGED]', { ts: Date.now() })
+ providers.value = await configP.getProviders()
+ await loadSavedOrder()
+ await refreshAllModels()
+ } catch (error) {
+ console.error('[settings][PROVIDER_CHANGED][ERROR]', {
+ ts: Date.now(),
+ message: 'Failed to reload providers/order after provider change',
+ error
+ })
+ }
+ })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| window.electron.ipcRenderer.on(CONFIG_EVENTS.PROVIDER_CHANGED, async () => { | |
| console.log('changed') | |
| console.log('Provider changed - updating providers and order') | |
| providers.value = await configP.getProviders() | |
| await loadSavedOrder() | |
| await refreshAllModels() | |
| }) | |
| window.electron.ipcRenderer.on(CONFIG_EVENTS.PROVIDER_CHANGED, async () => { | |
| try { | |
| console.info('[settings][PROVIDER_CHANGED]', { ts: Date.now() }) | |
| providers.value = await configP.getProviders() | |
| await loadSavedOrder() | |
| await refreshAllModels() | |
| } catch (error) { | |
| console.error('[settings][PROVIDER_CHANGED][ERROR]', { | |
| ts: Date.now(), | |
| message: 'Failed to reload providers/order after provider change', | |
| error | |
| }) | |
| } | |
| }) |
🤖 Prompt for AI Agents
In src/renderer/src/stores/settings.ts around lines 656 to 661, the async IPC
handler passed to window.electron.ipcRenderer.on is not wrapped in try–catch and
may produce unhandled promise rejections; update the handler to be an async
function wrapped in try { ... } catch (err) { logger.error({ context:
'PROVIDER_CHANGED handler', error: err }) } (or use the repo's structured
logger) and move calls to providers.value = await configP.getProviders(); await
loadSavedOrder(); await refreshAllModels(); inside the try block so any failure
is caught and logged with contextual information.
| // 强制更新 providers 以触发视图更新 | ||
| providers.value = [...providers.value] | ||
| await configP.setProviders(providers.value) | ||
| } catch (error) { |
There was a problem hiding this comment.
Sanitize before persisting providers to avoid leaking transient fields
Persisting providers.value directly may write large/ephemeral fields (e.g., websites) to config, which other code paths explicitly strip before saving. Save a sanitized, raw copy instead.
Apply this diff:
- // 强制更新 providers 以触发视图更新
- providers.value = [...providers.value]
- await configP.setProviders(providers.value)
+ // 强制更新 providers 以触发视图更新
+ providers.value = [...providers.value]
+ // Persist a sanitized snapshot (strip transient fields like `websites`)
+ const sanitizedProviders = providers.value.map((p) => {
+ const raw = toRaw(p) as any
+ const { websites, ...rest } = raw
+ return rest
+ })
+ await configP.setProviders(sanitizedProviders)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // 强制更新 providers 以触发视图更新 | |
| providers.value = [...providers.value] | |
| await configP.setProviders(providers.value) | |
| } catch (error) { | |
| // 强制更新 providers 以触发视图更新 | |
| providers.value = [...providers.value] | |
| // Persist a sanitized snapshot (strip transient fields like `websites`) | |
| const sanitizedProviders = providers.value.map((p) => { | |
| const raw = toRaw(p) as any | |
| const { websites, ...rest } = raw | |
| return rest | |
| }) | |
| await configP.setProviders(sanitizedProviders) |
🤖 Prompt for AI Agents
In src/renderer/src/stores/settings.ts around lines 1516 to 1519, the code
currently persists providers.value directly which can include large/ephemeral
fields (e.g., websites); before calling await configP.setProviders(...) create a
sanitized deep-copy (map over providers.value) that strips transient/large
fields (such as websites, caches, or runtime-only metadata) and persist that
sanitized array instead, while still updating providers.value for the UI;
replace setProviders(providers.value) with setProviders(sanitizedProviders).
sync provider order changes across all tabs
CleanShot.2025-08-21.at.17.32.25.mp4
Summary by CodeRabbit
Bug Fixes
Improvements