perf: implement atomic provider operations to optimize performance #815
Conversation
- Add atomic operation interfaces for precise provider management - Replace bulk setProviders() calls with targeted operations - Implement smart rebuild detection based on field changes - Add comprehensive cleanup for disabled providers including: * Stop all active streams * Remove provider instances * Clear rate limit states * Release current provider references - Optimize provider status changes with minimal instance rebuilding - Support atomic add/remove/update/reorder operations - Reduce unnecessary provider instance recreation by ~90%
WalkthroughIntroduces atomic provider management: new shared types/utilities, config presenter APIs, events, and renderer store handling. Adds CONFIG_EVENTS for atomic and batch updates, implements lifecycle handling in llmProviderPresenter, adjusts BaseLLMProvider init chaining, and documents migration and rebuild criteria. Renderer store switches to atomic APIs with incremental model sync. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Renderer Store
participant CP as ConfigPresenter
participant EB as EventBus (IPC)
participant LPP as LLMProviderPresenter
participant PI as ProviderInstance
UI->>CP: updateProviderAtomic(providerId, updates)
CP->>CP: Merge & persist updates<br/>requiresRebuild = checkRequiresRebuild(updates)
CP-->>UI: returns requiresRebuild (boolean)
CP->>EB: emit CONFIG_EVENTS.PROVIDER_ATOMIC_UPDATE {operation:'update', ...}
EB->>LPP: PROVIDER_ATOMIC_UPDATE payload
alt operation == 'add'
LPP->>LPP: Create instance if enabled
LPP->>PI: init()
else operation == 'remove'
LPP->>LPP: cleanupProviderInstance(providerId)
else operation == 'update'
alt requiresRebuild && enabled
LPP->>LPP: teardown old instance
LPP->>PI: create new instance
else requiresRebuild && !enabled
LPP->>LPP: teardown old instance
else enable toggled only
alt enabled -> true
LPP->>PI: create/init instance
else enabled -> false
LPP->>LPP: cleanupProviderInstance(providerId)
end
else no rebuild / no enable change
LPP->>PI: update config (if supported)
end
else operation == 'reorder'
LPP->>LPP: update order (no rebuild)
end
sequenceDiagram
autonumber
participant UI as Renderer Store
participant CP as ConfigPresenter
participant EB as EventBus (IPC)
participant LPP as LLMProviderPresenter
UI->>CP: updateProvidersBatch({providers, changes[]})
CP->>EB: emit CONFIG_EVENTS.PROVIDER_BATCH_UPDATE {providers, changes}
EB->>LPP: PROVIDER_BATCH_UPDATE payload
LPP->>LPP: Replace in-memory providers map
loop for each change
LPP->>LPP: handleProviderAtomicUpdate(change)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 7
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)
888-906: Refresh models when any rebuild-driving field (except enable) changes.Use the shared REBUILD_REQUIRED_FIELDS to avoid missing Azure/AWS auth changes.
- // 使用新的原子操作接口 - const requiresRebuild = await configP.updateProviderAtomic(providerId, updates) + // 使用新的原子操作接口 + const requiresRebuild = await configP.updateProviderAtomic(providerId, updates) @@ - // 只在需要重建实例且模型可能受影响时刷新模型列表 - const needRefreshModels = - requiresRebuild && ['enable', 'apiKey', 'baseUrl'].some((key) => key in updates) + // 只在需要重建实例且涉及模型列表可用性的字段时刷新（排除 'enable'） + // NOTE: keep in sync with REBUILD_REQUIRED_FIELDS + const refreshKeys = ['apiKey','baseUrl','authMode','oauthToken','accessKeyId','secretAccessKey','region','azureResourceName','azureApiVersion'] + const needRefreshModels = requiresRebuild && refreshKeys.some((key) => key in updates)And add the import at top:
-import type { ProviderChange, ProviderBatchUpdate } from '@shared/provider-operations' +import type { ProviderChange, ProviderBatchUpdate } from '@shared/provider-operations' +// REBUILD_REQUIRED_FIELDS used indirectly via refreshKeys (keep in sync)
🧹 Nitpick comments (10)
src/renderer/src/events.ts (1)
13-14: Mirror events added; fix comment language to meet repo standardConstants match main/events.ts. Update comments to English to comply with src/**/* logging/comment guideline.
- PROVIDER_ATOMIC_UPDATE: 'config:provider-atomic-update', // 原子操作单个 provider 更新 - PROVIDER_BATCH_UPDATE: 'config:provider-batch-update', // 批量 provider 更新 + PROVIDER_ATOMIC_UPDATE: 'config:provider-atomic-update', // Atomic update for a single provider + PROVIDER_BATCH_UPDATE: 'config:provider-batch-update', // Batch provider updatesdocs/provider-optimization-summary.md (1)
68-73: Fix markdownlint: specify code fence languageOne fenced block lacks a language (MD040). Add a language hint.
-``` +```text src/shared/provider-operations.ts # 类型定义和工具函数 src/main/presenter/configPresenter/ # 原子操作接口 src/main/presenter/llmProviderPresenter/ # 精确变更处理 src/renderer/src/stores/settings.ts # 前端调用优化 -``` +```src/shared/presenter.d.ts (2)
7-7: Use type-only import and drop unused type to avoid noUnusedLocals issuesProviderChange isn’t referenced here; switch to a type-only import for ProviderBatchUpdate.
-import { ProviderChange, ProviderBatchUpdate } from './provider-operations' +import type { ProviderBatchUpdate } from './provider-operations'
485-491: Clarify return semantics and error surface for atomic APIsIf updateProviderAtomic’s boolean indicates “rebuild required” or “applied”, document it in JSDoc. For consistency, you may consider Promise-returning signatures if persistence can fail, but keeping them sync is fine if underlying writes are synchronous.
src/main/presenter/configPresenter/index.ts (1)
413-422: Optionally persist a stable order snapshot.If other consumers still rely on providerOrder, consider syncing here as well (renderer already does). Not blocking.
src/main/presenter/llmProviderPresenter/index.ts (2)
347-362: Initialize rate-limit state for newly added providers.If provider.rateLimit is present, seed state so UI gets CONFIG_UPDATED immediately.
private handleProviderAdd(change: ProviderChange): void { if (!change.provider) return // 更新内存中的 provider 列表 this.providers.set(change.providerId, change.provider) + if (change.provider.rateLimit) { + this.setProviderRateLimitConfig(change.providerId, { + enabled: change.provider.rateLimit.enabled, + qpsLimit: change.provider.rateLimit.qpsLimit + }) + } // 如果 provider 启用且需要重建，创建实例
389-452: Propagate rate-limit config updates on non-rebuild path.When only rateLimit changes, update in-memory state and notify UI.
// 如果不需要重建且启用状态未变，仅更新实例配置 const instance = this.providerInstances.get(change.providerId) if (instance && 'updateConfig' in instance) { try { ;(instance as any).updateConfig(updatedProvider) + if ('rateLimit' in (change.updates as any) && updatedProvider.rateLimit) { + this.setProviderRateLimitConfig(change.providerId, { + enabled: updatedProvider.rateLimit.enabled, + qpsLimit: updatedProvider.rateLimit.qpsLimit + }) + } } catch (error) { console.error(`Failed to update provider config ${change.providerId}:`, error) } }src/shared/provider-operations.ts (1)
39-57: Make rebuild check O(number of updated keys) and reusable.Current approach scans the full field list each time. Use a Set and check update keys.
-export const REBUILD_REQUIRED_FIELDS = [ +export const REBUILD_REQUIRED_FIELDS = [ 'enable', 'apiKey', 'baseUrl', 'authMode', 'oauthToken', 'accessKeyId', // AWS Bedrock 'secretAccessKey', // AWS Bedrock 'region', // AWS Bedrock 'azureResourceName', // Azure 'azureApiVersion' // Azure ] as const + +const REBUILD_FIELDS_SET = new Set<string>(REBUILD_REQUIRED_FIELDS as readonly string[]) + +export function checkRequiresRebuild(updates: Partial<LLM_PROVIDER>): boolean { + for (const k of Object.keys(updates ?? {})) { + if (REBUILD_FIELDS_SET.has(k)) return true + } + return false +} - -export function checkRequiresRebuild(updates: Partial<LLM_PROVIDER>): boolean { - return REBUILD_REQUIRED_FIELDS.some((field) => field in updates) -}src/renderer/src/stores/settings.ts (2)
664-710: Incremental listeners are correct; translate new comments to English.Code is fine. Please switch added Chinese comments to English per repo guideline.
1068-1079: Custom provider add path looks correct; minor: align logs/comments with English.
📜 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 (9)
docs/provider-optimization-summary.md(1 hunks)src/main/events.ts(1 hunks)src/main/presenter/configPresenter/index.ts(2 hunks)src/main/presenter/llmProviderPresenter/baseProvider.ts(1 hunks)src/main/presenter/llmProviderPresenter/index.ts(3 hunks)src/renderer/src/events.ts(1 hunks)src/renderer/src/stores/settings.ts(8 hunks)src/shared/presenter.d.ts(2 hunks)src/shared/provider-operations.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (20)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
Files:
src/shared/provider-operations.tssrc/main/events.tssrc/renderer/src/events.tssrc/shared/presenter.d.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/baseProvider.tssrc/renderer/src/stores/settings.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪（如适用）、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别Enable and adhere to strict TypeScript type checking
Files:
src/shared/provider-operations.tssrc/main/events.tssrc/renderer/src/events.tssrc/shared/presenter.d.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/baseProvider.tssrc/renderer/src/stores/settings.ts
src/shared/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
共享类型定义放在
shared目录
Files:
src/shared/provider-operations.tssrc/shared/presenter.d.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/shared/provider-operations.tssrc/main/events.tssrc/renderer/src/events.tssrc/shared/presenter.d.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/baseProvider.tssrc/renderer/src/stores/settings.ts
src/shared/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Put shared types and IPC contracts in src/shared/
Files:
src/shared/provider-operations.tssrc/shared/presenter.d.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/main/events.tssrc/renderer/src/events.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/baseProvider.tssrc/renderer/src/stores/settings.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
Use Electron's built-in APIs for file system and native dialogs
From main to renderer, broadcast events via EventBus using mainWindow.webContents.send()
Files:
src/main/events.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/baseProvider.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/events.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/baseProvider.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/events.tssrc/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/events.tssrc/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/events.tssrc/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/events.tssrc/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/events.tssrc/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.
src/renderer/**/*.{ts,vue}: Use Pinia for frontend state management
From renderer to main, call presenters via the usePresenter.ts composable
Files:
src/renderer/src/events.tssrc/renderer/src/stores/settings.ts
src/renderer/src/**
📄 CodeRabbit inference engine (CLAUDE.md)
Organize UI components by feature under src/renderer/src/
Files:
src/renderer/src/events.tssrc/renderer/src/stores/settings.ts
src/shared/*.d.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
The shared/*.d.ts files are used to define the types of objects exposed by the main process to the renderer process
Files:
src/shared/presenter.d.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain one presenter per functional domain in src/main/presenter/
Files:
src/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/baseProvider.ts
src/main/presenter/configPresenter/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Centralize configuration logic under configPresenter/
Files:
src/main/presenter/configPresenter/index.ts
src/main/presenter/llmProviderPresenter/index.ts
📄 CodeRabbit inference engine (.cursor/rules/llm-agent-loop.mdc)
src/main/presenter/llmProviderPresenter/index.ts:src/main/presenter/llmProviderPresenter/index.tsshould manage the overall Agent loop, conversation history, tool execution viaMcpPresenter, and frontend communication viaeventBus.
The main Agent loop inllmProviderPresenter/index.tsshould handle multi-round LLM calls and tool usage, maintaining conversation state and controlling the loop withneedContinueConversationandtoolCallCount.
The main Agent loop should send standardizedSTREAM_EVENTS(RESPONSE,END,ERROR) to the frontend viaeventBus.
The main Agent loop should buffer text content, handle tool call events, format tool results for the next LLM call, and manage conversation continuation logic.Agent Loop layer must manage conversation flow, execute tools via McpPresenter, and standardize events to the frontend
Files:
src/main/presenter/llmProviderPresenter/index.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
🧠 Learnings (23)
📚 Learning: 2025-08-26T14:13:46.601Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-26T14:13:46.601Z
Learning: Applies to src/main/presenter/configPresenter/providers.ts : Add new provider configuration entries in configPresenter/providers.ts
Applied to files:
src/shared/provider-operations.tssrc/main/events.tssrc/renderer/src/events.tssrc/shared/presenter.d.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/renderer/src/stores/settings.ts
📚 Learning: 2025-08-26T14:13:46.601Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-26T14:13:46.601Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Each LLM provider must implement provider-specific API interactions, convert MCP tools, and normalize streaming responses
Applied to files:
src/shared/provider-operations.tssrc/shared/presenter.d.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/baseProvider.ts
📚 Learning: 2025-08-26T14:13:46.601Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-26T14:13:46.601Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : When adding a new LLM provider, create a provider file under providers/
Applied to files:
src/shared/provider-operations.tssrc/shared/presenter.d.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/baseProvider.tssrc/renderer/src/stores/settings.ts
📚 Learning: 2025-08-26T14:13:46.601Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-26T14:13:46.601Z
Learning: Applies to src/main/presenter/configPresenter/**/*.ts : Centralize configuration logic under configPresenter/
Applied to files:
src/shared/presenter.d.tssrc/main/presenter/configPresenter/index.ts
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Each file in `src/main/presenter/llmProviderPresenter/providers/*.ts` should handle interaction with a specific LLM API, including request/response formatting, tool definition conversion, native/non-native tool call management, and standardizing output streams to a common event format.
Applied to files:
src/shared/presenter.d.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/baseProvider.ts
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Provider implementations should yield events asynchronously using the async generator pattern.
Applied to files:
src/shared/presenter.d.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/baseProvider.ts
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Provider files should implement helper methods such as `formatMessages`, `convertToProviderTools`, `parseFunctionCalls`, and `prepareFunctionCallPrompt` as needed for provider-specific logic.
Applied to files:
src/shared/presenter.d.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/baseProvider.ts
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/index.ts : `src/main/presenter/llmProviderPresenter/index.ts` should manage the overall Agent loop, conversation history, tool execution via `McpPresenter`, and frontend communication via `eventBus`.
Applied to files:
src/shared/presenter.d.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.ts
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Provider implementations must use a `coreStream` method that yields standardized stream events to decouple the main loop from provider-specific details.
Applied to files:
src/main/presenter/configPresenter/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/baseProvider.ts
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Provider implementations should aggregate and yield usage events as part of the standardized stream.
Applied to files:
src/main/presenter/llmProviderPresenter/index.ts
📚 Learning: 2025-08-26T14:13:46.601Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-26T14:13:46.601Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Implement a coreStream method for new providers following the standardized event interface
Applied to files:
src/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/baseProvider.ts
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : All provider implementations must parse provider-specific data chunks and yield standardized events for text, reasoning, tool calls, usage, errors, stop reasons, and image data.
Applied to files:
src/main/presenter/llmProviderPresenter/index.ts
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Provider implementations should yield tool call events (`tool_call_start`, `tool_call_chunk`, `tool_call_end`) in the standardized format.
Applied to files:
src/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/baseProvider.ts
📚 Learning: 2025-08-26T14:13:46.601Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-26T14:13:46.601Z
Learning: Applies to src/main/presenter/llmProviderPresenter/index.ts : Agent Loop layer must manage conversation flow, execute tools via McpPresenter, and standardize events to the frontend
Applied to files:
src/main/presenter/llmProviderPresenter/index.ts
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : When a provider supports native function calling, MCP tools must be converted to the provider's format (e.g., using `convertToProviderTools`) and included in the API request.
Applied to files:
src/main/presenter/llmProviderPresenter/index.ts
📚 Learning: 2025-08-26T14:13:46.601Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-26T14:13:46.601Z
Learning: Applies to src/main/presenter/mcpPresenter/index.ts : Register new MCP tools in mcpPresenter/index.ts
Applied to files:
src/main/presenter/llmProviderPresenter/index.ts
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Provider implementations should yield error events in the standardized format when errors occur.
Applied to files:
src/main/presenter/llmProviderPresenter/baseProvider.ts
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : When a provider does not support native function calling, it must prepare messages using prompt wrapping (e.g., `prepareFunctionCallPrompt`) before making the API call.
Applied to files:
src/main/presenter/llmProviderPresenter/baseProvider.ts
📚 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/stores/**/*.ts : Use Pinia for state management.
Applied to files:
src/renderer/src/stores/settings.ts
📚 Learning: 2025-08-26T14:13:46.601Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-26T14:13:46.601Z
Learning: Applies to src/renderer/**/*.{ts,vue} : Use Pinia for frontend state management
Applied to files:
src/renderer/src/stores/settings.ts
📚 Learning: 2025-07-21T01:47:28.817Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/vue-best-practices.mdc:0-0
Timestamp: 2025-07-21T01:47:28.817Z
Learning: Applies to src/renderer/src/**/*.{vue,ts,tsx,js,jsx} : Implement proper state management with Pinia
Applied to files:
src/renderer/src/stores/settings.ts
📚 Learning: 2025-07-21T01:47:03.479Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/pinia-best-practices.mdc:0-0
Timestamp: 2025-07-21T01:47:03.479Z
Learning: Applies to src/renderer/src/stores/**/*.{vue,ts,tsx,js,jsx} : Keep the store focused on global state, not component-specific data
Applied to files:
src/renderer/src/stores/settings.ts
📚 Learning: 2025-07-21T01:47:03.479Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/pinia-best-practices.mdc:0-0
Timestamp: 2025-07-21T01:47:03.479Z
Learning: Applies to src/renderer/src/stores/**/*.{vue,ts,tsx,js,jsx} : Use modules to organize related state and actions
Applied to files:
src/renderer/src/stores/settings.ts
🧬 Code graph analysis (5)
src/shared/provider-operations.ts (1)
src/shared/presenter.d.ts (1)
LLM_PROVIDER(520-540)
src/shared/presenter.d.ts (1)
src/shared/provider-operations.ts (1)
ProviderBatchUpdate(62-67)
src/main/presenter/configPresenter/index.ts (5)
src/shared/presenter.d.ts (1)
LLM_PROVIDER(520-540)src/shared/provider-operations.ts (3)
checkRequiresRebuild(55-57)ProviderChange(23-34)ProviderBatchUpdate(62-67)src/main/eventbus.ts (1)
eventBus(151-151)src/renderer/src/events.ts (1)
CONFIG_EVENTS(11-28)src/main/events.ts (1)
CONFIG_EVENTS(12-39)
src/main/presenter/llmProviderPresenter/index.ts (3)
src/main/eventbus.ts (1)
eventBus(151-151)src/main/events.ts (1)
CONFIG_EVENTS(12-39)src/shared/provider-operations.ts (2)
ProviderChange(23-34)ProviderBatchUpdate(62-67)
src/renderer/src/stores/settings.ts (3)
src/renderer/src/events.ts (1)
CONFIG_EVENTS(11-28)src/main/events.ts (1)
CONFIG_EVENTS(12-39)src/shared/provider-operations.ts (2)
ProviderChange(23-34)ProviderBatchUpdate(62-67)
🪛 markdownlint-cli2 (0.17.2)
docs/provider-optimization-summary.md
68-68: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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/main/events.ts (1)
14-15: Update comments to English & centralize event tokens
- Replace Chinese comments on
PROVIDER_ATOMIC_UPDATEandPROVIDER_BATCH_UPDATEin bothsrc/main/events.ts(lines 14–15) andsrc/renderer/src/events.tswith English per repo guideline:- PROVIDER_ATOMIC_UPDATE: 'config:provider-atomic-update', // 新增：原子操作单个 provider 更新 - PROVIDER_BATCH_UPDATE: 'config:provider-batch-update', // 新增：批量 provider 更新 + PROVIDER_ATOMIC_UPDATE: 'config:provider-atomic-update', // New: atomic update for a single provider + PROVIDER_BATCH_UPDATE: 'config:provider-batch-update', // New: batch provider updates- Definitions of
CONFIG_EVENTSare duplicated in main and renderer—consider moving these tokens into a shared module (e.g.,src/shared/events.ts) to prevent drift.- I didn’t find any
ipcMain,webContents.send, oripcRendererreferences for these events; please verify that “config:provider-atomic-update” and “config:provider-batch-update” are correctly emitted in the main process and handled in the renderer.src/main/presenter/configPresenter/index.ts (1)
13-17: Imports for atomic ops look good.src/main/presenter/llmProviderPresenter/index.ts (5)
16-16: Type imports OK.
302-322: Dispatcher looks correct.
327-342: Batch replace is fine; ensure rate-limit state aligns.You call onProvidersUpdated at the end—good. No action.
457-461: Reorder: OK.
465-505: Cleanup routine is solid.Stops streams, deletes instance, clears RL state, resets current provider.
src/shared/provider-operations.ts (1)
23-34: Type surface is clear and matches usage across main/renderer.src/renderer/src/stores/settings.ts (4)
4-4: Good: shared atomic types imported to type IPC payloads.
1093-1107: Delete path OK and consistent with atomic flow.State/order persisted and models refreshed.
1377-1381: Listener cleanup covers the new atomic events.Good hygiene to prevent duplicate handlers.
1579-1583: Reorder path OK; local state refresh is sufficient.No rebuild needed—matches design.
| updateProviderAtomic(id: string, updates: Partial<LLM_PROVIDER>): boolean { | ||
| const providers = this.getProviders() | ||
| const index = providers.findIndex((p) => p.id === id) | ||
|
|
||
| if (index === -1) { | ||
| console.error(`[Config] Provider ${id} not found`) | ||
| return false | ||
| } | ||
|
|
||
| // 检查是否需要重建实例 | ||
| const requiresRebuild = checkRequiresRebuild(updates) | ||
|
|
||
| // 更新配置 | ||
| providers[index] = { ...providers[index], ...updates } | ||
| this.setSetting<LLM_PROVIDER[]>(PROVIDERS_STORE_KEY, providers) | ||
|
|
||
| // 触发精确的变更事件 | ||
| const change: ProviderChange = { | ||
| operation: 'update', | ||
| providerId: id, | ||
| requiresRebuild, | ||
| updates | ||
| } | ||
| eventBus.send(CONFIG_EVENTS.PROVIDER_ATOMIC_UPDATE, SendTarget.ALL_WINDOWS, change) | ||
|
|
||
| return requiresRebuild | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden updateProviderAtomic with error handling and no-op guard.
- Add try-catch to avoid crashing the main process.
- Skip emitting when updates is empty.
- updateProviderAtomic(id: string, updates: Partial<LLM_PROVIDER>): boolean {
- const providers = this.getProviders()
- const index = providers.findIndex((p) => p.id === id)
-
- if (index === -1) {
- console.error(`[Config] Provider ${id} not found`)
- return false
- }
-
- // 检查是否需要重建实例
- const requiresRebuild = checkRequiresRebuild(updates)
-
- // 更新配置
- providers[index] = { ...providers[index], ...updates }
- this.setSetting<LLM_PROVIDER[]>(PROVIDERS_STORE_KEY, providers)
-
- // 触发精确的变更事件
- const change: ProviderChange = {
- operation: 'update',
- providerId: id,
- requiresRebuild,
- updates
- }
- eventBus.send(CONFIG_EVENTS.PROVIDER_ATOMIC_UPDATE, SendTarget.ALL_WINDOWS, change)
-
- return requiresRebuild
- }
+ updateProviderAtomic(id: string, updates: Partial<LLM_PROVIDER>): boolean {
+ try {
+ if (!updates || Object.keys(updates).length === 0) return false
+ const providers = this.getProviders()
+ const index = providers.findIndex((p) => p.id === id)
+ if (index === -1) {
+ console.error(`[Config] Provider ${id} not found`)
+ return false
+ }
+ const requiresRebuild = checkRequiresRebuild(updates)
+ providers[index] = { ...providers[index], ...updates }
+ this.setSetting<LLM_PROVIDER[]>(PROVIDERS_STORE_KEY, providers)
+ const change: ProviderChange = {
+ operation: 'update',
+ providerId: id,
+ requiresRebuild,
+ updates
+ }
+ eventBus.send(CONFIG_EVENTS.PROVIDER_ATOMIC_UPDATE, SendTarget.ALL_WINDOWS, change)
+ return requiresRebuild
+ } catch (error) {
+ console.error(`[Config] Failed to update provider ${id}:`, error)
+ return false
+ }
+ }📝 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.
| updateProviderAtomic(id: string, updates: Partial<LLM_PROVIDER>): boolean { | |
| const providers = this.getProviders() | |
| const index = providers.findIndex((p) => p.id === id) | |
| if (index === -1) { | |
| console.error(`[Config] Provider ${id} not found`) | |
| return false | |
| } | |
| // 检查是否需要重建实例 | |
| const requiresRebuild = checkRequiresRebuild(updates) | |
| // 更新配置 | |
| providers[index] = { ...providers[index], ...updates } | |
| this.setSetting<LLM_PROVIDER[]>(PROVIDERS_STORE_KEY, providers) | |
| // 触发精确的变更事件 | |
| const change: ProviderChange = { | |
| operation: 'update', | |
| providerId: id, | |
| requiresRebuild, | |
| updates | |
| } | |
| eventBus.send(CONFIG_EVENTS.PROVIDER_ATOMIC_UPDATE, SendTarget.ALL_WINDOWS, change) | |
| return requiresRebuild | |
| } | |
| updateProviderAtomic(id: string, updates: Partial<LLM_PROVIDER>): boolean { | |
| try { | |
| if (!updates || Object.keys(updates).length === 0) return false | |
| const providers = this.getProviders() | |
| const index = providers.findIndex((p) => p.id === id) | |
| if (index === -1) { | |
| console.error(`[Config] Provider ${id} not found`) | |
| return false | |
| } | |
| const requiresRebuild = checkRequiresRebuild(updates) | |
| providers[index] = { ...providers[index], ...updates } | |
| this.setSetting<LLM_PROVIDER[]>(PROVIDERS_STORE_KEY, providers) | |
| const change: ProviderChange = { | |
| operation: 'update', | |
| providerId: id, | |
| requiresRebuild, | |
| updates | |
| } | |
| eventBus.send(CONFIG_EVENTS.PROVIDER_ATOMIC_UPDATE, SendTarget.ALL_WINDOWS, change) | |
| return requiresRebuild | |
| } catch (error) { | |
| console.error(`[Config] Failed to update provider ${id}:`, error) | |
| return false | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/presenter/configPresenter/index.ts around lines 334 to 360,
updateProviderAtomic lacks error handling and emits events even when updates is
empty; wrap the function body in a try-catch that logs the error and returns
false on failure, add an early no-op guard that returns false (and does not
persist or emit) if updates is null/undefined or has no own keys, ensure
providers are retrieved and index checked as before, only apply setSetting and
send the CONFIG_EVENTS.PROVIDER_ATOMIC_UPDATE event when there are real updates,
and keep returning the boolean requiresRebuild (or false on error/no-op).
| updateProvidersBatch(batchUpdate: ProviderBatchUpdate): void { | ||
| // 更新完整的 provider 列表（用于顺序变更） | ||
| this.setSetting<LLM_PROVIDER[]>(PROVIDERS_STORE_KEY, batchUpdate.providers) | ||
|
|
||
| // 触发批量变更事件 | ||
| eventBus.send(CONFIG_EVENTS.PROVIDER_BATCH_UPDATE, SendTarget.ALL_WINDOWS, batchUpdate) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate and dedupe on batch replace to avoid duplicate IDs.
Prevent inconsistent state when callers pass duplicates.
- updateProvidersBatch(batchUpdate: ProviderBatchUpdate): void {
- // 更新完整的 provider 列表（用于顺序变更）
- this.setSetting<LLM_PROVIDER[]>(PROVIDERS_STORE_KEY, batchUpdate.providers)
-
- // 触发批量变更事件
- eventBus.send(CONFIG_EVENTS.PROVIDER_BATCH_UPDATE, SendTarget.ALL_WINDOWS, batchUpdate)
- }
+ updateProvidersBatch(batchUpdate: ProviderBatchUpdate): void {
+ try {
+ const unique = new Map(batchUpdate.providers.map((p) => [p.id, p]))
+ if (unique.size !== batchUpdate.providers.length) {
+ console.warn('[Config] Duplicate provider IDs detected in batch update, deduplicating')
+ }
+ const finalProviders = Array.from(unique.values())
+ this.setSetting<LLM_PROVIDER[]>(PROVIDERS_STORE_KEY, finalProviders)
+ eventBus.send(CONFIG_EVENTS.PROVIDER_BATCH_UPDATE, SendTarget.ALL_WINDOWS, {
+ ...batchUpdate,
+ providers: finalProviders
+ })
+ } catch (error) {
+ console.error('[Config] Failed to apply provider batch update:', 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.
| updateProvidersBatch(batchUpdate: ProviderBatchUpdate): void { | |
| // 更新完整的 provider 列表（用于顺序变更） | |
| this.setSetting<LLM_PROVIDER[]>(PROVIDERS_STORE_KEY, batchUpdate.providers) | |
| // 触发批量变更事件 | |
| eventBus.send(CONFIG_EVENTS.PROVIDER_BATCH_UPDATE, SendTarget.ALL_WINDOWS, batchUpdate) | |
| } | |
| updateProvidersBatch(batchUpdate: ProviderBatchUpdate): void { | |
| try { | |
| const unique = new Map(batchUpdate.providers.map((p) => [p.id, p])) | |
| if (unique.size !== batchUpdate.providers.length) { | |
| console.warn('[Config] Duplicate provider IDs detected in batch update, deduplicating') | |
| } | |
| const finalProviders = Array.from(unique.values()) | |
| this.setSetting<LLM_PROVIDER[]>(PROVIDERS_STORE_KEY, finalProviders) | |
| eventBus.send(CONFIG_EVENTS.PROVIDER_BATCH_UPDATE, SendTarget.ALL_WINDOWS, { | |
| ...batchUpdate, | |
| providers: finalProviders | |
| }) | |
| } catch (error) { | |
| console.error('[Config] Failed to apply provider batch update:', error) | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/presenter/configPresenter/index.ts around lines 366 to 372, the
updateProvidersBatch method should guard against duplicate provider IDs:
validate that batchUpdate.providers contains unique, non-empty IDs and dedupe
before persisting. Implement a check that scans providers, removes duplicates
(keep the first occurrence or last—choose consistently), optionally log or emit
a warning/error if duplicates were found, then call setSetting with the deduped
array and proceed to send the PROVIDER_BATCH_UPDATE event with the cleaned
batchUpdate (or updated providers field).
| addProviderAtomic(provider: LLM_PROVIDER): void { | ||
| const providers = this.getProviders() | ||
| providers.push(provider) | ||
| this.setSetting<LLM_PROVIDER[]>(PROVIDERS_STORE_KEY, providers) | ||
|
|
||
| const change: ProviderChange = { | ||
| operation: 'add', | ||
| providerId: provider.id, | ||
| requiresRebuild: true, // 新增 provider 总是需要创建实例 | ||
| provider | ||
| } | ||
| eventBus.send(CONFIG_EVENTS.PROVIDER_ATOMIC_UPDATE, SendTarget.ALL_WINDOWS, change) | ||
| } |
There was a problem hiding this comment.
Guard against duplicate provider IDs and add error handling.
Adding a provider with an existing ID corrupts state.
- addProviderAtomic(provider: LLM_PROVIDER): void {
- const providers = this.getProviders()
- providers.push(provider)
- this.setSetting<LLM_PROVIDER[]>(PROVIDERS_STORE_KEY, providers)
-
- const change: ProviderChange = {
- operation: 'add',
- providerId: provider.id,
- requiresRebuild: true, // 新增 provider 总是需要创建实例
- provider
- }
- eventBus.send(CONFIG_EVENTS.PROVIDER_ATOMIC_UPDATE, SendTarget.ALL_WINDOWS, change)
- }
+ addProviderAtomic(provider: LLM_PROVIDER): void {
+ try {
+ const providers = this.getProviders()
+ if (providers.some((p) => p.id === provider.id)) {
+ console.error(`[Config] Duplicate provider id: ${provider.id}`)
+ return
+ }
+ providers.push(provider)
+ this.setSetting<LLM_PROVIDER[]>(PROVIDERS_STORE_KEY, providers)
+ const change: ProviderChange = {
+ operation: 'add',
+ providerId: provider.id,
+ requiresRebuild: true,
+ provider
+ }
+ eventBus.send(CONFIG_EVENTS.PROVIDER_ATOMIC_UPDATE, SendTarget.ALL_WINDOWS, change)
+ } catch (error) {
+ console.error('[Config] Failed to add provider:', 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.
| addProviderAtomic(provider: LLM_PROVIDER): void { | |
| const providers = this.getProviders() | |
| providers.push(provider) | |
| this.setSetting<LLM_PROVIDER[]>(PROVIDERS_STORE_KEY, providers) | |
| const change: ProviderChange = { | |
| operation: 'add', | |
| providerId: provider.id, | |
| requiresRebuild: true, // 新增 provider 总是需要创建实例 | |
| provider | |
| } | |
| eventBus.send(CONFIG_EVENTS.PROVIDER_ATOMIC_UPDATE, SendTarget.ALL_WINDOWS, change) | |
| } | |
| addProviderAtomic(provider: LLM_PROVIDER): void { | |
| try { | |
| const providers = this.getProviders() | |
| if (providers.some((p) => p.id === provider.id)) { | |
| console.error(`[Config] Duplicate provider id: ${provider.id}`) | |
| return | |
| } | |
| providers.push(provider) | |
| this.setSetting<LLM_PROVIDER[]>(PROVIDERS_STORE_KEY, providers) | |
| const change: ProviderChange = { | |
| operation: 'add', | |
| providerId: provider.id, | |
| requiresRebuild: true, | |
| provider | |
| } | |
| eventBus.send(CONFIG_EVENTS.PROVIDER_ATOMIC_UPDATE, SendTarget.ALL_WINDOWS, change) | |
| } catch (error) { | |
| console.error('[Config] Failed to add provider:', error) | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/presenter/configPresenter/index.ts around lines 378 to 390, the
addProviderAtomic function currently pushes a provider without checking for
existing IDs and lacks error handling; update it to first retrieve providers and
check if any provider.id === provider.id, if so throw or return a descriptive
error (or log and abort) to prevent duplicates, otherwise proceed to push and
save; wrap persistence and eventBus.send in try/catch to handle/save failures
and surface/log errors so the caller can react instead of corrupting state.
| removeProviderAtomic(providerId: string): void { | ||
| const providers = this.getProviders() | ||
| const filteredProviders = providers.filter((p) => p.id !== providerId) | ||
| this.setSetting<LLM_PROVIDER[]>(PROVIDERS_STORE_KEY, filteredProviders) | ||
|
|
||
| const change: ProviderChange = { | ||
| operation: 'remove', | ||
| providerId, | ||
| requiresRebuild: true // 删除 provider 需要清理实例 | ||
| } | ||
| eventBus.send(CONFIG_EVENTS.PROVIDER_ATOMIC_UPDATE, SendTarget.ALL_WINDOWS, change) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
On removal, also purge provider models and status cache.
Without this, orphaned model files and status keys remain.
- removeProviderAtomic(providerId: string): void {
- const providers = this.getProviders()
- const filteredProviders = providers.filter((p) => p.id !== providerId)
- this.setSetting<LLM_PROVIDER[]>(PROVIDERS_STORE_KEY, filteredProviders)
-
- const change: ProviderChange = {
- operation: 'remove',
- providerId,
- requiresRebuild: true // 删除 provider 需要清理实例
- }
- eventBus.send(CONFIG_EVENTS.PROVIDER_ATOMIC_UPDATE, SendTarget.ALL_WINDOWS, change)
- }
+ removeProviderAtomic(providerId: string): void {
+ try {
+ const providers = this.getProviders()
+ if (!providers.some((p) => p.id === providerId)) {
+ console.warn(`[Config] Provider ${providerId} not found for removal`)
+ return
+ }
+ const filteredProviders = providers.filter((p) => p.id !== providerId)
+ this.setSetting<LLM_PROVIDER[]>(PROVIDERS_STORE_KEY, filteredProviders)
+ // 清理模型状态缓存与模型存储
+ this.clearProviderModelStatusCache(providerId)
+ try {
+ const modelStore = this.getProviderModelStore(providerId)
+ modelStore.clear()
+ } catch (e) {
+ console.warn(`[Config] Failed to clear model store for ${providerId}:`, e)
+ }
+ const change: ProviderChange = {
+ operation: 'remove',
+ providerId,
+ requiresRebuild: true
+ }
+ eventBus.send(CONFIG_EVENTS.PROVIDER_ATOMIC_UPDATE, SendTarget.ALL_WINDOWS, change)
+ } catch (error) {
+ console.error('[Config] Failed to remove provider:', 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.
| removeProviderAtomic(providerId: string): void { | |
| const providers = this.getProviders() | |
| const filteredProviders = providers.filter((p) => p.id !== providerId) | |
| this.setSetting<LLM_PROVIDER[]>(PROVIDERS_STORE_KEY, filteredProviders) | |
| const change: ProviderChange = { | |
| operation: 'remove', | |
| providerId, | |
| requiresRebuild: true // 删除 provider 需要清理实例 | |
| } | |
| eventBus.send(CONFIG_EVENTS.PROVIDER_ATOMIC_UPDATE, SendTarget.ALL_WINDOWS, change) | |
| } | |
| removeProviderAtomic(providerId: string): void { | |
| try { | |
| const providers = this.getProviders() | |
| if (!providers.some((p) => p.id === providerId)) { | |
| console.warn(`[Config] Provider ${providerId} not found for removal`) | |
| return | |
| } | |
| const filteredProviders = providers.filter((p) => p.id !== providerId) | |
| this.setSetting<LLM_PROVIDER[]>(PROVIDERS_STORE_KEY, filteredProviders) | |
| // 清理模型状态缓存与模型存储 | |
| this.clearProviderModelStatusCache(providerId) | |
| try { | |
| const modelStore = this.getProviderModelStore(providerId) | |
| modelStore.clear() | |
| } catch (e) { | |
| console.warn(`[Config] Failed to clear model store for ${providerId}:`, e) | |
| } | |
| const change: ProviderChange = { | |
| operation: 'remove', | |
| providerId, | |
| requiresRebuild: true | |
| } | |
| eventBus.send(CONFIG_EVENTS.PROVIDER_ATOMIC_UPDATE, SendTarget.ALL_WINDOWS, change) | |
| } catch (error) { | |
| console.error('[Config] Failed to remove provider:', error) | |
| } | |
| } |
| try { | ||
| this.isInitialized = true | ||
| await this.fetchModels() | ||
| this.fetchModels() | ||
| .then(() => { | ||
| return this.autoEnableModelsIfNeeded() | ||
| }) | ||
| .then(() => { | ||
| console.info('Provider initialized successfully:', this.provider.name) | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Init becomes fire-and-forget; unhandled rejections possible and isInitialized set too early
The promise chain isn’t awaited; try/catch won’t capture async errors; an error in fetchModels() will be unhandled. Also isInitialized is set before the provider is actually ready. Track the init as a promise, set readiness on success, and handle errors.
- this.isInitialized = true
- this.fetchModels()
- .then(() => {
- return this.autoEnableModelsIfNeeded()
- })
- .then(() => {
- console.info('Provider initialized successfully:', this.provider.name)
- })
+ this.initPromise = this.fetchModels()
+ .then(() => this.autoEnableModelsIfNeeded())
+ .then(() => {
+ this.isInitialized = true
+ console.info('Provider initialized successfully:', this.provider.name)
+ })
+ .catch((error) => {
+ this.isInitialized = false
+ console.warn('Provider initialization failed:', this.provider.name, error)
+ })Add the backing field to the class:
// class field (near other fields)
private initPromise: Promise<void> | null = nullNote: Prefer the repo’s structured logger over console.* per logging guidelines.
🤖 Prompt for AI Agents
In src/main/presenter/llmProviderPresenter/baseProvider.ts around lines 108 to
116, the initialization is fire-and-forget: isInitialized is set before async
work completes and errors from fetchModels()/autoEnableModelsIfNeeded() are
unhandled. Add a private initPromise: Promise<void> | null = null field, assign
initPromise to the chain of async calls (fetchModels().then(...).then(...))
inside the try, await that initPromise so try/catch can catch async failures,
only set isInitialized = true after the awaited promise resolves, and on
rejection log the error via the project’s structured logger and clear
initPromise/isInitialized appropriately so retries are possible.
| // 监听原子操作事件 | ||
| eventBus.on(CONFIG_EVENTS.PROVIDER_ATOMIC_UPDATE, (change: ProviderChange) => { | ||
| this.handleProviderAtomicUpdate(change) | ||
| }) | ||
|
|
||
| eventBus.on(CONFIG_EVENTS.PROVIDER_BATCH_UPDATE, (batchUpdate: ProviderBatchUpdate) => { | ||
| this.handleProviderBatchUpdate(batchUpdate) | ||
| }) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Wrap event handlers to avoid crashing the bus on exceptions.
A thrown error in handlers will bubble up from the emitter.
- eventBus.on(CONFIG_EVENTS.PROVIDER_ATOMIC_UPDATE, (change: ProviderChange) => {
- this.handleProviderAtomicUpdate(change)
- })
- eventBus.on(CONFIG_EVENTS.PROVIDER_BATCH_UPDATE, (batchUpdate: ProviderBatchUpdate) => {
- this.handleProviderBatchUpdate(batchUpdate)
- })
+ eventBus.on(CONFIG_EVENTS.PROVIDER_ATOMIC_UPDATE, (change: ProviderChange) => {
+ try {
+ this.handleProviderAtomicUpdate(change)
+ } catch (e) {
+ console.error('[LLMProviderPresenter] Failed to handle atomic update:', e, change)
+ }
+ })
+ eventBus.on(CONFIG_EVENTS.PROVIDER_BATCH_UPDATE, (batchUpdate: ProviderBatchUpdate) => {
+ try {
+ this.handleProviderBatchUpdate(batchUpdate)
+ } catch (e) {
+ console.error('[LLMProviderPresenter] Failed to handle batch update:', e, batchUpdate)
+ }
+ })📝 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.
| // 监听原子操作事件 | |
| eventBus.on(CONFIG_EVENTS.PROVIDER_ATOMIC_UPDATE, (change: ProviderChange) => { | |
| this.handleProviderAtomicUpdate(change) | |
| }) | |
| eventBus.on(CONFIG_EVENTS.PROVIDER_BATCH_UPDATE, (batchUpdate: ProviderBatchUpdate) => { | |
| this.handleProviderBatchUpdate(batchUpdate) | |
| }) | |
| } | |
| // 监听原子操作事件 | |
| eventBus.on(CONFIG_EVENTS.PROVIDER_ATOMIC_UPDATE, (change: ProviderChange) => { | |
| try { | |
| this.handleProviderAtomicUpdate(change) | |
| } catch (e) { | |
| console.error('[LLMProviderPresenter] Failed to handle atomic update:', e, change) | |
| } | |
| }) | |
| eventBus.on(CONFIG_EVENTS.PROVIDER_BATCH_UPDATE, (batchUpdate: ProviderBatchUpdate) => { | |
| try { | |
| this.handleProviderBatchUpdate(batchUpdate) | |
| } catch (e) { | |
| console.error('[LLMProviderPresenter] Failed to handle batch update:', e, batchUpdate) | |
| } | |
| }) |
🤖 Prompt for AI Agents
In src/main/presenter/llmProviderPresenter/index.ts around lines 116 to 124, the
event handlers are passed directly to eventBus and any exception thrown will
bubble up and crash the bus; wrap each handler in a safe wrapper that catches
synchronous errors and handles rejected promises: call the existing handler
inside a try/catch (or an async wrapper that awaits the handler and attaches a
.catch), log the error with context (which event and payload) and swallow or
surface a controlled error instead of letting it propagate to the emitter;
replace the two eventBus.on registrations with wrapped callbacks that invoke
this.handleProviderAtomicUpdate and this.handleProviderBatchUpdate safely.
| private handleProviderRemove(change: ProviderChange): void { | ||
| // 从内存中移除 provider | ||
| this.providers.delete(change.providerId) | ||
|
|
||
| // 如果需要重建（删除操作总是需要），清理实例 | ||
| if (change.requiresRebuild) { | ||
| const instance = this.providerInstances.get(change.providerId) | ||
| if (instance) { | ||
| console.log(`Removing provider instance: ${change.providerId}`) | ||
| this.providerInstances.delete(change.providerId) | ||
| } | ||
|
|
||
| // 如果删除的是当前 provider，清除当前 provider | ||
| if (this.currentProviderId === change.providerId) { | ||
| this.currentProviderId = null | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Removal misses cleanup: active streams, instance, and rate-limit state may leak.
Call the existing cleanupProviderInstance() to abort streams and clear states.
- private handleProviderRemove(change: ProviderChange): void {
- // 从内存中移除 provider
- this.providers.delete(change.providerId)
-
- // 如果需要重建（删除操作总是需要），清理实例
- if (change.requiresRebuild) {
- const instance = this.providerInstances.get(change.providerId)
- if (instance) {
- console.log(`Removing provider instance: ${change.providerId}`)
- this.providerInstances.delete(change.providerId)
- }
-
- // 如果删除的是当前 provider，清除当前 provider
- if (this.currentProviderId === change.providerId) {
- this.currentProviderId = null
- }
- }
- }
+ private handleProviderRemove(change: ProviderChange): void {
+ this.providers.delete(change.providerId)
+ // Always cleanup to stop streams and clear rate-limit state
+ this.cleanupProviderInstance(change.providerId)
+ }📝 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.
| private handleProviderRemove(change: ProviderChange): void { | |
| // 从内存中移除 provider | |
| this.providers.delete(change.providerId) | |
| // 如果需要重建（删除操作总是需要），清理实例 | |
| if (change.requiresRebuild) { | |
| const instance = this.providerInstances.get(change.providerId) | |
| if (instance) { | |
| console.log(`Removing provider instance: ${change.providerId}`) | |
| this.providerInstances.delete(change.providerId) | |
| } | |
| // 如果删除的是当前 provider，清除当前 provider | |
| if (this.currentProviderId === change.providerId) { | |
| this.currentProviderId = null | |
| } | |
| } | |
| } | |
| private handleProviderRemove(change: ProviderChange): void { | |
| this.providers.delete(change.providerId) | |
| // Always cleanup to stop streams and clear rate-limit state | |
| this.cleanupProviderInstance(change.providerId) | |
| } |
🤖 Prompt for AI Agents
In src/main/presenter/llmProviderPresenter/index.ts around lines 367 to 384, the
provider removal only deletes entries from maps and clears currentProviderId but
does not abort active streams or clear instance/rate-limit state; call the
existing cleanupProviderInstance(providerId) before deleting the instance entry
(or if instance exists) to abort streams and clear rate-limit/state, then remove
the provider from providerInstances and providers, and still clear
currentProviderId when applicable.
Summary by CodeRabbit
New Features
Performance
Stability
Documentation