Feat/acp model enhance #1272
Feat/acp model enhance #1272
Conversation
- Add SkillTools class for handling skill list and control actions. - Extend ConversationsTable to include active_skills field in the database. - Create SkillsSettings.vue component for managing skills in the UI. - Update router to include skills settings route. - Add translations for skills management in English and Chinese. - Implement skills store using Pinia for state management. - Define skill-related types in shared types directory. - Integrate skill installation and uninstallation functionalities.
- Add SkillTools class for handling skill list and control actions. - Extend ConversationsTable to include active_skills field in the database. - Create SkillsSettings.vue component for managing skills in the UI. - Update router to include skills settings route. - Add translations for skills management in English and Chinese. - Implement skills store using Pinia for state management. - Define skill-related types in shared types directory. - Integrate skill installation and uninstallation functionalities.
- Added `selectFiles` method to `IDevicePresenter` interface for file selection with options for filters and multiple selections. - Created comprehensive tests for `SkillPresenter`, covering skill discovery, installation, activation, and deactivation. - Introduced tests for `SkillTools` to validate skill handling, including edge cases and input validation. - Mocked necessary dependencies and ensured proper integration with the event bus for skill events.
…ll management - Delegated skills metadata prompt construction to skillPresenter.getMetadataPrompt() to reduce code duplication. - Initialized activeSkills in default conversation settings to maintain state. - Introduced SKILL_CONFIG constants for better configuration management, including limits on file sizes and download timeouts. - Implemented checks for file sizes and content types during skill downloads to prevent memory exhaustion and ensure valid files. - Enhanced folder tree building to limit depth and skip symbolic links, preventing infinite recursion. - Updated SkillEditorSheet.vue to use YAML serialization for SKILL.md content, improving format consistency. - Simplified drag-and-drop functionality in SkillInstallDialog.vue by removing unsupported features and adding URL validation. - Added user feedback for invalid URL formats in skill installation. - Updated translations for skill name hints in both English and Chinese.
…pchat into feat/skills
- Implement comprehensive unit tests for the security module, covering path safety, file size limits, permissions, and input validation. - Introduce unit tests for the ToolScanner class, validating external tool configurations, path expansion, and tool availability. - Mock filesystem interactions to isolate tests and ensure reliability. - Validate filename safety and prevent path traversal in directory scanning. - Ensure proper handling of oversized files during tool scanning.
…o Code, and OpenCode; update Cursor adapter to use SKILL.md format
…tences", "כישורים", "技能", "Habilidades", "Навыки" with "Skills" in various languages for consistency across the application.
- Introduced SkillsPanel.vue to display and manage user skills. - Implemented useSkillsData composable for managing skills state and interactions. - Added useSlashMentionData composable to handle slash mention functionality for skills, prompts, and tools. - Updated MentionList.vue to support new categorization and display logic for skills and prompts. - Created SlashMention extension for TipTap editor to handle slash-triggered mentions. - Enhanced slashSuggestion.ts to manage slash mention data and filtering. - Updated i18n files for English and Chinese to include new skills-related translations. - Added new events for skill activation and deactivation. - Removed unused watchers and code related to tools and prompts from useMentionData.
…content management
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (6)
You can disable this status message by setting the
📝 WalkthroughWalkthroughAdds ACP session model/mode management and UI integration, enables MCP by default, normalizes chatMode to 'agent', removes legacy local-search pieces, implements single-WebContents renderer layout and many renderer composables/components, and adds extensive architecture/specification documentation and enforcement scripts. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant ChatInput as ChatInput.vue
participant Renderer as Renderer (IPC)
participant SessionPres as SessionPresenter (main)
participant AcpProv as AcpProvider (main/process)
participant Process as ACP Process Manager
User->>ChatInput: Select ACP session model/mode
ChatInput->>Renderer: call setAcpSessionModel / setAcpSessionMode
Renderer-->>SessionPres: IPC -> setAcpSessionModel(conversationId, modelId)
SessionPres->>AcpProv: delegate setSessionModel(conversationId, modelId)
AcpProv->>Process: apply model to bound process handle
Process-->>AcpProv: ack / update currentModelId
AcpProv->>SessionPres: emit SESSION_MODELS_READY(payload)
SessionPres->>Renderer: IPC: SESSION_MODELS_READY (conversationId, agentId, workdir, current, available)
Renderer->>ChatInput: update reactive availableModels/currentModelId
ChatInput->>User: display updated model selection
sequenceDiagram
participant User as User
participant Sidebar as IconSidebar.vue
participant App as App.vue
participant Store as sidebarStore
participant Router as Vue Router
participant ChatView as ChatView.vue
User->>Sidebar: Click conversation icon
Sidebar->>App: emit conversation-select(id)
App->>Store: openConversation(id)
Store->>Router: navigate /conversation/:id
Router->>ChatView: mount conversation route
ChatView->>Store: load messages, render
Store->>Store: persist open conversations state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 3
🤖 Fix all issues with AI agents
In @src/main/presenter/agentPresenter/message/messageBuilder.ts:
- Around line 101-108: The code awaits a synchronous method: remove the
unnecessary await from presenter.configPresenter.getSetting when assigning
storedChatMode, i.e., change "const storedChatMode = (await
presenter.configPresenter.getSetting(...)) as 'chat' | 'agent' | 'acp agent' |
undefined" to call getSetting synchronously and cast its return ("const
storedChatMode = presenter.configPresenter.getSetting(...) as 'chat' | 'agent' |
'acp agent' | undefined"); keep the subsequent normalization (normalizedChatMode
and chatMode) unchanged.
In @src/main/presenter/sessionPresenter/index.ts:
- Around line 932-933: The change is forcefully converting existing
conversation.settings.chatMode to 'agent' and defaulting new sessions to
'agent', which overwrites user data; revert this by (1) removing or guarding the
migration in conversationManager so it only sets settings.chatMode when
undefined (do not overwrite existing 'chat' values), and (2) change the
resolvedChatMode assignment in sessionPresenter (the resolvedChatMode expression
using conversation.settings.chatMode ?? 'agent') to default to 'chat' instead of
'agent' or only apply the 'agent' default for truly new sessions without any
stored chatMode; target the conversationManager migration logic and the
resolvedChatMode expression to preserve existing 'chat' settings.
In @src/renderer/src/i18n/zh-CN/settings.json:
- Around line 287-301: Add the missing "acpSession" translation block (same
keys: title, description, model.{label,description,placeholder,empty},
mode.{label,description,placeholder,empty}) to the 10 language files: da-DK,
fa-IR, fr-FR, he-IL, ja-JP, ko-KR, pt-BR, ru-RU, zh-HK, zh-TW, using the en-US
structure as the source; ensure the key names and JSON structure exactly match
the en-US/zh-CN versions and provide appropriate localized strings for each
field (including placeholders and empty messages) so all 12 locales contain an
identical "acpSession" block.
🧹 Nitpick comments (10)
src/renderer/src/stores/mcp.ts (1)
35-40: Comment should be in English per coding guidelines.The change to enable MCP by default aligns with the PR objective. However, the inline comment is in Chinese and should be translated to English.
Suggested fix
const config = ref<MCPConfig>({ mcpServers: {}, defaultServers: [], - mcpEnabled: true, // 添加MCP启用状态 + mcpEnabled: true, // MCP enabled by default ready: false // if init finished, the ready will be true })src/renderer/src/components/chat-input/composables/useChatMode.ts (1)
36-40: Consider future cleanup of 'chat' mode handling.The 'chat' case in
currentLabelis needed during the migration period. Once the migration is considered complete, this branch along withMODE_ICONS.chatand the 'chat' entry inChatModetype could be removed.src/renderer/src/components/chat-input/composables/useAcpSessionModel.ts (1)
183-191: Guard against potential null dereference.The non-null assertions
options.activeModel.value!.id!at line 185 could throw at runtime if the checks earlier insetModeldon't guarantee these values exist. WhileisAcpModelensuresproviderId === 'acp' && !!id, there's a race condition window between the check and the async call.♻️ Suggested safer approach
} else if (selectedWorkdir.value) { + const activeModelId = options.activeModel.value?.id + if (!activeModelId) { + console.warn('[useAcpSessionModel] Cannot set preferred model: activeModel.id is missing') + return + } await sessionPresenter.setAcpPreferredProcessModel( - options.activeModel.value!.id!, + activeModelId, selectedWorkdir.value, modelId )src/renderer/src/components/chat-input/composables/useAcpMode.ts (1)
188-216: Same non-null assertion concern as useAcpSessionModel.Line 204 uses
options.activeModel.value!.id!which could throw ifactiveModelbecomes null between the initial check and the async operation.♻️ Suggested safer approach
} else if (selectedWorkdir.value) { + const activeModelId = options.activeModel.value?.id + if (!activeModelId) { + console.warn('[useAcpMode] Cannot set preferred mode: activeModel.id is missing') + return + } await sessionPresenter.setAcpPreferredProcessMode( - options.activeModel.value!.id!, + activeModelId, selectedWorkdir.value, modeId )src/main/presenter/agentPresenter/message/messageBuilder.ts (1)
355-360: Hardcoded Chinese text should use i18n.The prompt text on line 356 is hardcoded in Chinese. Per coding guidelines, user-facing strings should use i18n keys for proper internationalization support. This appears to be a pre-existing issue but worth addressing.
♻️ Suggested approach
Consider extracting this string to an i18n key:
- const userPromptText = - '以上是你刚执行的工具调用及其响应信息，已帮你插入，请仔细阅读工具响应，并继续你的回答。' + // TODO: Move to i18n - this is a system prompt for the model + const userPromptText = + 'Above is the tool call you just executed and its response. Please carefully read the tool response and continue your answer.'src/main/presenter/agentPresenter/session/sessionManager.ts (1)
65-70: Consider extracting duplicated chat mode normalization logic.The pattern of retrieving
input_chatModeand normalizing'chat'to'agent'is duplicated three times (lines 65-70, 106-112, 132-138). This violates the DRY principle and increases maintenance burden.♻️ Suggested refactor: Extract helper method
+ private getNormalizedFallbackChatMode(): 'agent' | 'acp agent' { + const raw = this.options.configPresenter.getSetting('input_chatMode') as + | 'chat' + | 'agent' + | 'acp agent' + | undefined + return (raw === 'chat' ? 'agent' : raw) ?? 'agent' + } + async resolveSession(agentId: string): Promise<SessionContextResolved> { const conversation = await this.options.sessionPresenter.getConversation(agentId) - const rawFallbackChatMode = this.options.configPresenter.getSetting('input_chatMode') as - | 'chat' - | 'agent' - | 'acp agent' - | undefined - const fallbackChatMode = rawFallbackChatMode === 'chat' ? 'agent' : rawFallbackChatMode + const fallbackChatMode = this.getNormalizedFallbackChatMode()Then use
this.getNormalizedFallbackChatMode()in all three locations.src/renderer/src/stores/chat.ts (1)
40-41: Pre-existing: Chinese comments should be translated to English.Per coding guidelines, all comments must be in English. There are numerous Chinese comments in this file (lines 40, 73, 90, 93, 128, 131, etc.). While this is not introduced by this PR, consider addressing it in a follow-up to align with project standards.
src/shared/types/presenters/session.presenter.d.ts (1)
139-157: Consider usingAcpSessionModelInfotype for consistency.The inline type
{ id: string; name: string; description?: string }is defined inlegacy.presenters.d.tsasAcpSessionModelInfo. Using the shared type would improve maintainability and ensure consistency across the codebase.♻️ Suggested refactor
+import type { AcpSessionModelInfo } from './legacy.presenters' getAcpProcessModels( agentId: string, workdir: string ): Promise< | { - availableModels?: Array<{ id: string; name: string; description?: string }> + availableModels?: AcpSessionModelInfo[] currentModelId?: string } | undefined > // ... getAcpSessionModels(conversationId: string): Promise<{ current: string - available: Array<{ id: string; name: string; description?: string }> + available: AcpSessionModelInfo[] } | null>src/renderer/src/components/ChatConfig.vue (1)
155-156: Consider extracting eligible agents to a shared constant.The
acpSessionEligibleAgentsSet is hardcoded here. If this list is used elsewhere or may change, consider extracting it to a shared constants file for maintainability.src/renderer/src/components/chat-input/ChatInput.vue (1)
964-980: Consider extracting repeated loading guard pattern.Multiple handlers (
handleAcpSessionModelSelect,handleAcpSessionModeSelect,handleAcpModeSelect) share the same loading guard pattern. This is a minor code smell but acceptable for clarity.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
docs/specs/acp-mode-defaults/plan.mddocs/specs/acp-mode-defaults/spec.mddocs/specs/acp-mode-defaults/tasks.mdsrc/main/events.tssrc/main/presenter/agentPresenter/acp/acpProcessManager.tssrc/main/presenter/agentPresenter/acp/acpSessionManager.tssrc/main/presenter/agentPresenter/message/messageBuilder.tssrc/main/presenter/agentPresenter/session/sessionManager.tssrc/main/presenter/agentPresenter/session/sessionResolver.tssrc/main/presenter/configPresenter/mcpConfHelper.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/main/presenter/sessionPresenter/const.tssrc/main/presenter/sessionPresenter/index.tssrc/main/presenter/sessionPresenter/managers/conversationManager.tssrc/main/presenter/toolPresenter/index.tssrc/renderer/src/components/ChatConfig.vuesrc/renderer/src/components/ChatConfig/ConfigSelectField.vuesrc/renderer/src/components/NewThread.vuesrc/renderer/src/components/chat-input/ChatInput.vuesrc/renderer/src/components/chat-input/composables/useAcpMode.tssrc/renderer/src/components/chat-input/composables/useAcpSessionModel.tssrc/renderer/src/components/chat-input/composables/useChatMode.tssrc/renderer/src/events.tssrc/renderer/src/i18n/en-US/settings.jsonsrc/renderer/src/i18n/zh-CN/settings.jsonsrc/renderer/src/stores/chat.tssrc/renderer/src/stores/mcp.tssrc/shared/types/presenters/legacy.presenters.d.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/shared/types/presenters/session.presenter.d.ts
🧰 Additional context used
📓 Path-based instructions (23)
**/*.{js,ts,tsx,jsx,vue,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
All logs and comments must be in English
Files:
src/renderer/src/components/ChatConfig/ConfigSelectField.vuesrc/main/presenter/toolPresenter/index.tssrc/renderer/src/components/chat-input/composables/useChatMode.tssrc/main/presenter/sessionPresenter/const.tssrc/main/presenter/configPresenter/mcpConfHelper.tssrc/main/presenter/agentPresenter/message/messageBuilder.tssrc/renderer/src/components/chat-input/composables/useAcpSessionModel.tssrc/main/presenter/agentPresenter/session/sessionResolver.tssrc/renderer/src/components/ChatConfig.vuesrc/renderer/src/components/NewThread.vuesrc/main/events.tssrc/renderer/src/stores/mcp.tssrc/shared/types/presenters/legacy.presenters.d.tssrc/main/presenter/agentPresenter/acp/acpProcessManager.tssrc/main/presenter/sessionPresenter/index.tssrc/main/presenter/agentPresenter/acp/acpSessionManager.tssrc/main/presenter/sessionPresenter/managers/conversationManager.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/agentPresenter/session/sessionManager.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/shared/types/presenters/session.presenter.d.tssrc/main/presenter/llmProviderPresenter/index.tssrc/renderer/src/stores/chat.tssrc/renderer/src/components/chat-input/composables/useAcpMode.tssrc/renderer/src/components/chat-input/ChatInput.vuesrc/renderer/src/events.ts
**/*.{js,ts,tsx,jsx,vue,json,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
Use Prettier as the code formatter
Files:
src/renderer/src/components/ChatConfig/ConfigSelectField.vuesrc/main/presenter/toolPresenter/index.tssrc/renderer/src/components/chat-input/composables/useChatMode.tssrc/main/presenter/sessionPresenter/const.tssrc/renderer/src/i18n/zh-CN/settings.jsonsrc/main/presenter/configPresenter/mcpConfHelper.tssrc/main/presenter/agentPresenter/message/messageBuilder.tssrc/renderer/src/components/chat-input/composables/useAcpSessionModel.tssrc/main/presenter/agentPresenter/session/sessionResolver.tssrc/renderer/src/components/ChatConfig.vuesrc/renderer/src/components/NewThread.vuesrc/main/events.tssrc/renderer/src/stores/mcp.tssrc/shared/types/presenters/legacy.presenters.d.tssrc/main/presenter/agentPresenter/acp/acpProcessManager.tssrc/main/presenter/sessionPresenter/index.tssrc/main/presenter/agentPresenter/acp/acpSessionManager.tssrc/main/presenter/sessionPresenter/managers/conversationManager.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/agentPresenter/session/sessionManager.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/shared/types/presenters/session.presenter.d.tssrc/main/presenter/llmProviderPresenter/index.tssrc/renderer/src/stores/chat.tssrc/renderer/src/i18n/en-US/settings.jsonsrc/renderer/src/components/chat-input/composables/useAcpMode.tssrc/renderer/src/components/chat-input/ChatInput.vuesrc/renderer/src/events.ts
src/renderer/src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
src/renderer/src/**/*.{vue,ts,tsx}: Use vue-i18n framework for internationalization located at src/renderer/src/i18n/
All user-facing strings must use i18n keys, not hardcoded text
src/renderer/src/**/*.{vue,ts,tsx}: Usereffor primitives and references,reactivefor objects in Vue 3 Composition API
Prefercomputedproperties over methods for derived state in Vue components
Import Shadcn Vue components from@/shadcn/components/ui/path alias
Use thecn()utility function combining clsx and tailwind-merge for dynamic Tailwind classes
UsedefineAsyncComponent()for lazy loading heavy Vue components
Use TypeScript for all Vue components and composables with explicit type annotations
Define TypeScript interfaces for Vue component props and data structures
UseusePresentercomposable for main process communication instead of direct IPC calls
Files:
src/renderer/src/components/ChatConfig/ConfigSelectField.vuesrc/renderer/src/components/chat-input/composables/useChatMode.tssrc/renderer/src/components/chat-input/composables/useAcpSessionModel.tssrc/renderer/src/components/ChatConfig.vuesrc/renderer/src/components/NewThread.vuesrc/renderer/src/stores/mcp.tssrc/renderer/src/stores/chat.tssrc/renderer/src/components/chat-input/composables/useAcpMode.tssrc/renderer/src/components/chat-input/ChatInput.vuesrc/renderer/src/events.ts
src/renderer/src/**/*.vue
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
Import useI18n from vue-i18n in Vue components to access translation functions t and locale
src/renderer/src/**/*.vue: Use<script setup>syntax for concise Vue 3 component definitions with Composition API
Define props and emits explicitly in Vue components usingdefinePropsanddefineEmitswith TypeScript interfaces
Useprovide/injectfor dependency injection in Vue components instead of prop drilling
Use Tailwind CSS for all styling instead of writing scoped CSS files
Use mobile-first responsive design approach with Tailwind breakpoints
Use Iconify Vue with lucide icons as primary choice, following patternlucide:{icon-name}
Usev-memodirective for memoizing expensive computations in templates
Usev-oncedirective for rendering static content without reactivity updates
Use virtual scrolling withRecycleScrollercomponent for rendering long lists
Subscribe to events usingrendererEvents.on()and unsubscribe inonUnmountedlifecycle hook
Files:
src/renderer/src/components/ChatConfig/ConfigSelectField.vuesrc/renderer/src/components/ChatConfig.vuesrc/renderer/src/components/NewThread.vuesrc/renderer/src/components/chat-input/ChatInput.vue
src/renderer/src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
Ensure all code comments are in English and all log messages are in English, with no non-English text in code comments or console statements
Use VueUse composables for common utilities like
useLocalStorage,useClipboard,useDebounceFnVue 3 renderer app code should be organized in
src/renderer/srcwith subdirectories forcomponents/,stores/,views/,i18n/, andlib/
Files:
src/renderer/src/components/ChatConfig/ConfigSelectField.vuesrc/renderer/src/components/chat-input/composables/useChatMode.tssrc/renderer/src/components/chat-input/composables/useAcpSessionModel.tssrc/renderer/src/components/ChatConfig.vuesrc/renderer/src/components/NewThread.vuesrc/renderer/src/stores/mcp.tssrc/renderer/src/stores/chat.tssrc/renderer/src/components/chat-input/composables/useAcpMode.tssrc/renderer/src/components/chat-input/ChatInput.vuesrc/renderer/src/events.ts
src/renderer/src/components/**/*.vue
📄 CodeRabbit inference engine (.cursor/rules/vue-stack-guide.mdc)
Name Vue components using PascalCase (e.g.,
ChatInput.vue,MessageItemUser.vue)
Files:
src/renderer/src/components/ChatConfig/ConfigSelectField.vuesrc/renderer/src/components/ChatConfig.vuesrc/renderer/src/components/NewThread.vuesrc/renderer/src/components/chat-input/ChatInput.vue
**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
Vue components must be named in PascalCase (e.g.,
ChatInput.vue) and use Vue 3 Composition API with Pinia for state management and Tailwind for styling
**/*.vue: Use Vue 3 Composition API with<script setup>syntax
Use Pinia stores for state management
Style Vue components using Tailwind CSS (v4) with shadcn/ui (reka-ui)
Files:
src/renderer/src/components/ChatConfig/ConfigSelectField.vuesrc/renderer/src/components/ChatConfig.vuesrc/renderer/src/components/NewThread.vuesrc/renderer/src/components/chat-input/ChatInput.vue
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,vue}: Use camelCase for variable and function names; use PascalCase for types and classes; use SCREAMING_SNAKE_CASE for constants
Configure Prettier with single quotes, no semicolons, and line width of 100 characters. Runpnpm run formatafter completing features
Files:
src/renderer/src/components/ChatConfig/ConfigSelectField.vuesrc/main/presenter/toolPresenter/index.tssrc/renderer/src/components/chat-input/composables/useChatMode.tssrc/main/presenter/sessionPresenter/const.tssrc/main/presenter/configPresenter/mcpConfHelper.tssrc/main/presenter/agentPresenter/message/messageBuilder.tssrc/renderer/src/components/chat-input/composables/useAcpSessionModel.tssrc/main/presenter/agentPresenter/session/sessionResolver.tssrc/renderer/src/components/ChatConfig.vuesrc/renderer/src/components/NewThread.vuesrc/main/events.tssrc/renderer/src/stores/mcp.tssrc/shared/types/presenters/legacy.presenters.d.tssrc/main/presenter/agentPresenter/acp/acpProcessManager.tssrc/main/presenter/sessionPresenter/index.tssrc/main/presenter/agentPresenter/acp/acpSessionManager.tssrc/main/presenter/sessionPresenter/managers/conversationManager.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/agentPresenter/session/sessionManager.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/shared/types/presenters/session.presenter.d.tssrc/main/presenter/llmProviderPresenter/index.tssrc/renderer/src/stores/chat.tssrc/renderer/src/components/chat-input/composables/useAcpMode.tssrc/renderer/src/components/chat-input/ChatInput.vuesrc/renderer/src/events.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/renderer/src/components/ChatConfig/ConfigSelectField.vuesrc/main/presenter/toolPresenter/index.tssrc/renderer/src/components/chat-input/composables/useChatMode.tssrc/main/presenter/sessionPresenter/const.tssrc/main/presenter/configPresenter/mcpConfHelper.tssrc/main/presenter/agentPresenter/message/messageBuilder.tssrc/renderer/src/components/chat-input/composables/useAcpSessionModel.tssrc/main/presenter/agentPresenter/session/sessionResolver.tssrc/renderer/src/components/ChatConfig.vuesrc/renderer/src/components/NewThread.vuesrc/main/events.tssrc/renderer/src/stores/mcp.tssrc/shared/types/presenters/legacy.presenters.d.tssrc/main/presenter/agentPresenter/acp/acpProcessManager.tssrc/main/presenter/sessionPresenter/index.tssrc/main/presenter/agentPresenter/acp/acpSessionManager.tssrc/main/presenter/sessionPresenter/managers/conversationManager.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/agentPresenter/session/sessionManager.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/shared/types/presenters/session.presenter.d.tssrc/main/presenter/llmProviderPresenter/index.tssrc/renderer/src/stores/chat.tssrc/renderer/src/components/chat-input/composables/useAcpMode.tssrc/renderer/src/components/chat-input/ChatInput.vuesrc/renderer/src/events.ts
src/renderer/**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
All user-facing strings must use i18n keys (supports 12 languages)
Files:
src/renderer/src/components/ChatConfig/ConfigSelectField.vuesrc/renderer/src/components/chat-input/composables/useChatMode.tssrc/renderer/src/components/chat-input/composables/useAcpSessionModel.tssrc/renderer/src/components/ChatConfig.vuesrc/renderer/src/components/NewThread.vuesrc/renderer/src/stores/mcp.tssrc/renderer/src/stores/chat.tssrc/renderer/src/components/chat-input/composables/useAcpMode.tssrc/renderer/src/components/chat-input/ChatInput.vuesrc/renderer/src/events.ts
**/*.{js,ts,tsx,jsx,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
Use OxLint as the linter
Files:
src/main/presenter/toolPresenter/index.tssrc/renderer/src/components/chat-input/composables/useChatMode.tssrc/main/presenter/sessionPresenter/const.tssrc/main/presenter/configPresenter/mcpConfHelper.tssrc/main/presenter/agentPresenter/message/messageBuilder.tssrc/renderer/src/components/chat-input/composables/useAcpSessionModel.tssrc/main/presenter/agentPresenter/session/sessionResolver.tssrc/main/events.tssrc/renderer/src/stores/mcp.tssrc/shared/types/presenters/legacy.presenters.d.tssrc/main/presenter/agentPresenter/acp/acpProcessManager.tssrc/main/presenter/sessionPresenter/index.tssrc/main/presenter/agentPresenter/acp/acpSessionManager.tssrc/main/presenter/sessionPresenter/managers/conversationManager.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/agentPresenter/session/sessionManager.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/shared/types/presenters/session.presenter.d.tssrc/main/presenter/llmProviderPresenter/index.tssrc/renderer/src/stores/chat.tssrc/renderer/src/components/chat-input/composables/useAcpMode.tssrc/renderer/src/events.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Electron main process code should reside in
src/main/, with presenters organized inpresenter/subdirectory (Window, Tab, Thread, Mcp, Config, LLMProvider), and app events managed viaeventbus.ts
Files:
src/main/presenter/toolPresenter/index.tssrc/main/presenter/sessionPresenter/const.tssrc/main/presenter/configPresenter/mcpConfHelper.tssrc/main/presenter/agentPresenter/message/messageBuilder.tssrc/main/presenter/agentPresenter/session/sessionResolver.tssrc/main/events.tssrc/main/presenter/agentPresenter/acp/acpProcessManager.tssrc/main/presenter/sessionPresenter/index.tssrc/main/presenter/agentPresenter/acp/acpSessionManager.tssrc/main/presenter/sessionPresenter/managers/conversationManager.tssrc/main/presenter/agentPresenter/session/sessionManager.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/main/presenter/llmProviderPresenter/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use OxLint for linting JavaScript and TypeScript files; ensure lint-staged hooks and typecheck pass before commits
Enable strict TypeScript type checking
Files:
src/main/presenter/toolPresenter/index.tssrc/renderer/src/components/chat-input/composables/useChatMode.tssrc/main/presenter/sessionPresenter/const.tssrc/main/presenter/configPresenter/mcpConfHelper.tssrc/main/presenter/agentPresenter/message/messageBuilder.tssrc/renderer/src/components/chat-input/composables/useAcpSessionModel.tssrc/main/presenter/agentPresenter/session/sessionResolver.tssrc/main/events.tssrc/renderer/src/stores/mcp.tssrc/shared/types/presenters/legacy.presenters.d.tssrc/main/presenter/agentPresenter/acp/acpProcessManager.tssrc/main/presenter/sessionPresenter/index.tssrc/main/presenter/agentPresenter/acp/acpSessionManager.tssrc/main/presenter/sessionPresenter/managers/conversationManager.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/agentPresenter/session/sessionManager.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/shared/types/presenters/session.presenter.d.tssrc/main/presenter/llmProviderPresenter/index.tssrc/renderer/src/stores/chat.tssrc/renderer/src/components/chat-input/composables/useAcpMode.tssrc/renderer/src/events.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Implement all system capabilities as main-process presenters following the Presenter Pattern
Files:
src/main/presenter/toolPresenter/index.tssrc/main/presenter/sessionPresenter/const.tssrc/main/presenter/configPresenter/mcpConfHelper.tssrc/main/presenter/agentPresenter/message/messageBuilder.tssrc/main/presenter/agentPresenter/session/sessionResolver.tssrc/main/presenter/agentPresenter/acp/acpProcessManager.tssrc/main/presenter/sessionPresenter/index.tssrc/main/presenter/agentPresenter/acp/acpSessionManager.tssrc/main/presenter/sessionPresenter/managers/conversationManager.tssrc/main/presenter/agentPresenter/session/sessionManager.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/main/presenter/llmProviderPresenter/index.ts
{src/main,src/renderer,test}/**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Use IPC communication: Renderer calls main process via
usePresentercomposable, Main sends to Renderer via EventBus
Files:
src/main/presenter/toolPresenter/index.tssrc/renderer/src/components/chat-input/composables/useChatMode.tssrc/main/presenter/sessionPresenter/const.tssrc/main/presenter/configPresenter/mcpConfHelper.tssrc/main/presenter/agentPresenter/message/messageBuilder.tssrc/renderer/src/components/chat-input/composables/useAcpSessionModel.tssrc/main/presenter/agentPresenter/session/sessionResolver.tssrc/main/events.tssrc/renderer/src/stores/mcp.tssrc/main/presenter/agentPresenter/acp/acpProcessManager.tssrc/main/presenter/sessionPresenter/index.tssrc/main/presenter/agentPresenter/acp/acpSessionManager.tssrc/main/presenter/sessionPresenter/managers/conversationManager.tssrc/main/presenter/agentPresenter/session/sessionManager.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/main/presenter/llmProviderPresenter/index.tssrc/renderer/src/stores/chat.tssrc/renderer/src/components/chat-input/composables/useAcpMode.tssrc/renderer/src/events.ts
src/renderer/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/vue-stack-guide.mdc)
Use class-variance-authority (CVA) for defining component variants with Tailwind classes
Files:
src/renderer/src/components/chat-input/composables/useChatMode.tssrc/renderer/src/components/chat-input/composables/useAcpSessionModel.tssrc/renderer/src/stores/mcp.tssrc/renderer/src/stores/chat.tssrc/renderer/src/components/chat-input/composables/useAcpMode.tssrc/renderer/src/events.ts
src/renderer/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/vue-stack-guide.mdc)
src/renderer/src/**/*.{ts,tsx}: UseshallowRefandshallowReactivefor optimizing reactivity with large objects
Prefertypeoverinterfacein TypeScript unless using inheritance withextends
Files:
src/renderer/src/components/chat-input/composables/useChatMode.tssrc/renderer/src/components/chat-input/composables/useAcpSessionModel.tssrc/renderer/src/stores/mcp.tssrc/renderer/src/stores/chat.tssrc/renderer/src/components/chat-input/composables/useAcpMode.tssrc/renderer/src/events.ts
src/renderer/src/i18n/**/*.json
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
src/renderer/src/i18n/**/*.json: Use dot-separated hierarchical structure for translation key naming with lowercase letters and descriptive names grouped by feature/context (e.g., common.button.submit, chat.send.placeholder)
Add new translations to ALL language files (da-DK, en-US, fa-IR, fr-FR, he-IL, ja-JP, ko-KR, pt-BR, ru-RU, zh-CN, zh-HK, zh-TW) with consistent key names across all locales
Keep proper nouns and technical terms untranslated (e.g., 'DeepChat', 'MCP', 'Agents' in English) unless established convention exists, and follow established conventions for well-known technical terms
Files:
src/renderer/src/i18n/zh-CN/settings.jsonsrc/renderer/src/i18n/en-US/settings.json
src/renderer/src/i18n/{zh-CN,zh-HK,zh-TW}/**/*.json
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
For Chinese translations, consider using '智能体' for 'Agents' instead of the English term
Files:
src/renderer/src/i18n/zh-CN/settings.json
src/renderer/src/stores/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/vue-stack-guide.mdc)
src/renderer/src/stores/**/*.ts: Use Setup Store syntax withdefineStorefunction pattern in Pinia stores
Use getters (computed properties) for derived state in Pinia stores
Keep Pinia store actions focused on state mutations and async operations
Files:
src/renderer/src/stores/mcp.tssrc/renderer/src/stores/chat.ts
src/shared/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Shared TypeScript types and utilities should be placed in
src/shared/
Files:
src/shared/types/presenters/legacy.presenters.d.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/shared/types/presenters/session.presenter.d.ts
src/main/presenter/llmProviderPresenter/providers/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Implement
coreStreammethod following standardized event interface for LLM providers
Files:
src/main/presenter/llmProviderPresenter/providers/acpProvider.ts
src/renderer/src/i18n/en-US/**/*.json
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
Use English (en-US) as the reference for translation accuracy when adding new keys
Files:
src/renderer/src/i18n/en-US/settings.json
🧠 Learnings (19)
📓 Common learnings
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T02:41:45.219Z
Learning: Use Presenter pattern in main process, EventBus for inter-process events, and two-layer LLM provider architecture (Agent Loop + Provider) with integrated MCP tools
📚 Learning: 2026-01-05T02:41:31.661Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-stack-guide.mdc:0-0
Timestamp: 2026-01-05T02:41:31.661Z
Learning: Applies to src/renderer/src/**/*.vue : Define props and emits explicitly in Vue components using `defineProps` and `defineEmits` with TypeScript interfaces
Applied to files:
src/renderer/src/components/ChatConfig/ConfigSelectField.vuesrc/renderer/src/components/chat-input/ChatInput.vue
📚 Learning: 2026-01-13T09:23:07.415Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:23:07.415Z
Learning: Applies to src/main/presenter/mcpPresenter/inMemoryServers/**/*.ts : Implement new MCP tools in the inMemoryServers directory and register in mcpPresenter/index.ts
Applied to files:
src/main/presenter/toolPresenter/index.tssrc/main/presenter/configPresenter/mcpConfHelper.tssrc/renderer/src/stores/mcp.tssrc/shared/types/presenters/legacy.presenters.d.tssrc/main/presenter/sessionPresenter/index.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/shared/types/presenters/session.presenter.d.tssrc/main/presenter/llmProviderPresenter/index.ts
📚 Learning: 2026-01-05T02:41:31.661Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-stack-guide.mdc:0-0
Timestamp: 2026-01-05T02:41:31.661Z
Learning: Applies to src/renderer/src/composables/**/*.ts : Name composables using camelCase with `use` prefix (e.g., `useChatState.ts`, `useMessageList.ts`)
Applied to files:
src/renderer/src/components/chat-input/composables/useChatMode.tssrc/renderer/src/components/chat-input/composables/useAcpSessionModel.tssrc/renderer/src/components/ChatConfig.vuesrc/renderer/src/components/NewThread.vuesrc/renderer/src/components/chat-input/composables/useAcpMode.tssrc/renderer/src/components/chat-input/ChatInput.vue
📚 Learning: 2025-06-21T15:49:17.044Z
Learnt from: neoragex2002
Repo: ThinkInAIXYZ/deepchat PR: 550
File: src/renderer/src/stores/chat.ts:1011-1035
Timestamp: 2025-06-21T15:49:17.044Z
Learning: In src/renderer/src/stores/chat.ts, the user prefers to keep both `text` and `content` properties in the `handleMeetingInstruction` function's `sendMessage` call, even though they are redundant, rather than removing the `content` property.
Applied to files:
src/main/presenter/sessionPresenter/const.tssrc/renderer/src/stores/chat.ts
📚 Learning: 2026-01-05T02:41:13.302Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/i18n.mdc:0-0
Timestamp: 2026-01-05T02:41:13.302Z
Learning: Applies to src/renderer/src/i18n/**/*.json : Add new translations to ALL language files (da-DK, en-US, fa-IR, fr-FR, he-IL, ja-JP, ko-KR, pt-BR, ru-RU, zh-CN, zh-HK, zh-TW) with consistent key names across all locales
Applied to files:
src/renderer/src/i18n/zh-CN/settings.jsonsrc/renderer/src/i18n/en-US/settings.json
📚 Learning: 2026-01-05T02:41:13.302Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/i18n.mdc:0-0
Timestamp: 2026-01-05T02:41:13.302Z
Learning: Applies to src/renderer/src/i18n/**/*.json : Use dot-separated hierarchical structure for translation key naming with lowercase letters and descriptive names grouped by feature/context (e.g., common.button.submit, chat.send.placeholder)
Applied to files:
src/renderer/src/i18n/zh-CN/settings.jsonsrc/renderer/src/i18n/en-US/settings.json
📚 Learning: 2026-01-05T02:41:13.302Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/i18n.mdc:0-0
Timestamp: 2026-01-05T02:41:13.302Z
Learning: Applies to src/renderer/src/i18n/{zh-CN,zh-HK,zh-TW}/**/*.json : For Chinese translations, consider using '智能体' for 'Agents' instead of the English term
Applied to files:
src/renderer/src/i18n/zh-CN/settings.jsonsrc/renderer/src/i18n/en-US/settings.json
📚 Learning: 2026-01-05T02:41:13.302Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/i18n.mdc:0-0
Timestamp: 2026-01-05T02:41:13.302Z
Learning: Applies to src/renderer/src/i18n/**/*.json : Keep proper nouns and technical terms untranslated (e.g., 'DeepChat', 'MCP', 'Agents' in English) unless established convention exists, and follow established conventions for well-known technical terms
Applied to files:
src/renderer/src/i18n/zh-CN/settings.jsonsrc/renderer/src/i18n/en-US/settings.json
📚 Learning: 2026-01-05T02:41:13.302Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/i18n.mdc:0-0
Timestamp: 2026-01-05T02:41:13.302Z
Learning: Applies to src/renderer/src/i18n/en-US/**/*.json : Use English (en-US) as the reference for translation accuracy when adding new keys
Applied to files:
src/renderer/src/i18n/zh-CN/settings.jsonsrc/renderer/src/i18n/en-US/settings.json
📚 Learning: 2026-01-05T02:41:31.661Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-stack-guide.mdc:0-0
Timestamp: 2026-01-05T02:41:31.661Z
Learning: Applies to src/renderer/src/**/*.{ts,tsx,vue} : Use VueUse composables for common utilities like `useLocalStorage`, `useClipboard`, `useDebounceFn`
Applied to files:
src/renderer/src/components/chat-input/composables/useAcpSessionModel.tssrc/renderer/src/components/ChatConfig.vuesrc/renderer/src/components/NewThread.vuesrc/renderer/src/components/chat-input/ChatInput.vue
📚 Learning: 2026-01-05T02:41:31.661Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-stack-guide.mdc:0-0
Timestamp: 2026-01-05T02:41:31.661Z
Learning: Applies to src/renderer/src/**/*.{vue,ts,tsx} : Use `usePresenter` composable for main process communication instead of direct IPC calls
Applied to files:
src/renderer/src/components/chat-input/composables/useAcpSessionModel.tssrc/renderer/src/components/ChatConfig.vuesrc/renderer/src/components/NewThread.vuesrc/main/presenter/sessionPresenter/index.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/shared/types/presenters/session.presenter.d.tssrc/renderer/src/components/chat-input/ChatInput.vue
📚 Learning: 2026-01-05T02:41:31.661Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-stack-guide.mdc:0-0
Timestamp: 2026-01-05T02:41:31.661Z
Learning: Applies to src/renderer/src/components/**/*.vue : Name Vue components using PascalCase (e.g., `ChatInput.vue`, `MessageItemUser.vue`)
Applied to files:
src/renderer/src/components/ChatConfig.vuesrc/renderer/src/components/NewThread.vuesrc/renderer/src/components/chat-input/ChatInput.vue
📚 Learning: 2026-01-05T02:41:31.661Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-stack-guide.mdc:0-0
Timestamp: 2026-01-05T02:41:31.661Z
Learning: Applies to src/renderer/src/components/ : Organize Vue component directories using lowercase with dashes (e.g., `chat-input/`, `message/`)
Applied to files:
src/renderer/src/components/ChatConfig.vuesrc/renderer/src/components/chat-input/ChatInput.vue
📚 Learning: 2026-01-05T02:41:13.302Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/i18n.mdc:0-0
Timestamp: 2026-01-05T02:41:13.302Z
Learning: Applies to src/renderer/src/**/*.{vue,ts,tsx} : Use vue-i18n framework for internationalization located at src/renderer/src/i18n/
Applied to files:
src/renderer/src/components/ChatConfig.vue
📚 Learning: 2026-01-13T09:23:07.415Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:23:07.415Z
Learning: Applies to {src/main,src/renderer,test}/**/*.{ts,tsx,js} : Use IPC communication: Renderer calls main process via `usePresenter` composable, Main sends to Renderer via EventBus
Applied to files:
src/renderer/src/components/NewThread.vuesrc/main/presenter/sessionPresenter/index.tssrc/shared/types/presenters/llmprovider.presenter.d.ts
📚 Learning: 2026-01-13T09:23:07.415Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:23:07.415Z
Learning: Applies to src/main/presenter/**/*.ts : Implement all system capabilities as main-process presenters following the Presenter Pattern
Applied to files:
src/shared/types/presenters/legacy.presenters.d.tssrc/main/presenter/agentPresenter/acp/acpProcessManager.tssrc/main/presenter/sessionPresenter/index.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/shared/types/presenters/session.presenter.d.tssrc/main/presenter/llmProviderPresenter/index.ts
📚 Learning: 2026-01-13T09:23:07.415Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:23:07.415Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Implement `coreStream` method following standardized event interface for LLM providers
Applied to files:
src/shared/types/presenters/legacy.presenters.d.tssrc/main/presenter/sessionPresenter/index.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/main/presenter/llmProviderPresenter/index.ts
📚 Learning: 2026-01-05T02:41:45.219Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T02:41:45.219Z
Learning: Use Presenter pattern in main process, EventBus for inter-process events, and two-layer LLM provider architecture (Agent Loop + Provider) with integrated MCP tools
Applied to files:
src/main/presenter/sessionPresenter/index.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/main/presenter/llmProviderPresenter/index.ts
🧬 Code graph analysis (6)
src/main/presenter/agentPresenter/message/messageBuilder.ts (1)
src/main/presenter/index.ts (1)
presenter(334-334)
src/renderer/src/components/chat-input/composables/useAcpSessionModel.ts (2)
src/renderer/src/composables/usePresenter.ts (1)
usePresenter(103-105)src/renderer/src/events.ts (1)
ACP_WORKSPACE_EVENTS(208-211)
src/main/presenter/sessionPresenter/index.ts (1)
src/shared/types/presenters/session.presenter.d.ts (1)
Session(50-58)
src/main/presenter/agentPresenter/acp/acpSessionManager.ts (2)
test/mocks/electron.ts (1)
session(34-34)scripts/fetch-provider-db.mjs (1)
models(37-37)
src/shared/types/presenters/llmprovider.presenter.d.ts (1)
src/shared/types/presenters/legacy.presenters.d.ts (1)
AcpSessionModelInfo(906-910)
src/main/presenter/llmProviderPresenter/providers/acpProvider.ts (4)
test/mocks/electron.ts (1)
session(34-34)src/main/eventbus.ts (1)
eventBus(151-151)src/main/events.ts (1)
ACP_WORKSPACE_EVENTS(256-259)src/renderer/src/events.ts (1)
ACP_WORKSPACE_EVENTS(208-211)
🪛 LanguageTool
docs/specs/acp-mode-defaults/spec.md
[style] ~22-~22: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ACP Agent modes and default to Agent. - As a user, switching to ACP Agent should i...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~23-~23: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...t without opening the model selector. - As a user, when using Claude Code or Codex...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/renderer/src/components/chat-input/ChatInput.vue (4)
876-876: Confusing naming forisAgentModeSelected.The computed property returns
truewhen the mode is NOT 'acp agent', which makes it select the "Agent" row. The nameisAgentModeSelectedsuggests it checks if agent mode is selected, but it actually checks if we're NOT in ACP agent mode. Consider renaming for clarity.💡 Suggested rename for clarity
-const isAgentModeSelected = computed(() => chatMode.currentMode.value !== 'acp agent') +const isNonAcpAgentMode = computed(() => chatMode.currentMode.value !== 'acp agent')Or alternatively, if the intent is to highlight the "agent" row:
-const isAgentModeSelected = computed(() => chatMode.currentMode.value !== 'acp agent') +const isBuiltInAgentSelected = computed(() => chatMode.currentMode.value === 'agent')
312-331: Consider disabling click during loading state at the template level.The loading state adds visual disabled styling (
opacity-60 cursor-not-allowed) but the click handler is still attached. WhilehandleAcpModeSelectchecksacpMode.loading.valueinternally, adding@click.stopor a conditional guard at the template level would be more consistent.💡 Suggested improvement
@click="handleAcpModeSelect(mode.id)" + @click="!acpMode.loading.value && handleAcpModeSelect(mode.id)"Or using a modifier:
- @click="handleAcpModeSelect(mode.id)" + @click="acpMode.loading.value ? undefined : handleAcpModeSelect(mode.id)"
937-947: Consider adding error handling for async operations.Both handlers call async methods (
setModel,setMode) but don't handle potential failures. If these operations can fail, the popover would close without user feedback.💡 Suggested improvement
const handleAcpSessionModelSelect = async (modelId: string) => { if (acpSessionModel.loading.value) return - await acpSessionModel.setModel(modelId) - modelSelectOpen.value = false + try { + await acpSessionModel.setModel(modelId) + modelSelectOpen.value = false + } catch (error) { + console.warn('Failed to set ACP session model:', error) + } } const handleAcpModeSelect = async (modeId: string) => { if (acpMode.loading.value) return - await acpMode.setMode(modeId) - acpModeSelectOpen.value = false + try { + await acpMode.setMode(modeId) + acpModeSelectOpen.value = false + } catch (error) { + console.warn('Failed to set ACP mode:', error) + } }
1017-1021: Type casting withas anymay hide type mismatches.The casts at lines 1018 and 1020 suggest the payload type doesn't match what
config.handleModelUpdateor themodel-updateevent expect. Consider aligning the types properly to catch potential issues at compile time.💡 Investigation suggestion
#!/bin/bash # Check the expected type signature for handleModelUpdate and model-update event ast-grep --pattern 'handleModelUpdate($$$)' rg -n "emit\('model-update'" --type=vue --type=ts -A2 rg -n "defineEmits" src/renderer/src/components/chat-input/ChatInput.vue -A5
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/renderer/src/components/chat-input/ChatInput.vue
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,ts,tsx,jsx,vue,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
All logs and comments must be in English
Files:
src/renderer/src/components/chat-input/ChatInput.vue
**/*.{js,ts,tsx,jsx,vue,json,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
Use Prettier as the code formatter
Files:
src/renderer/src/components/chat-input/ChatInput.vue
src/renderer/src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
src/renderer/src/**/*.{vue,ts,tsx}: Use vue-i18n framework for internationalization located at src/renderer/src/i18n/
All user-facing strings must use i18n keys, not hardcoded text
src/renderer/src/**/*.{vue,ts,tsx}: Usereffor primitives and references,reactivefor objects in Vue 3 Composition API
Prefercomputedproperties over methods for derived state in Vue components
Import Shadcn Vue components from@/shadcn/components/ui/path alias
Use thecn()utility function combining clsx and tailwind-merge for dynamic Tailwind classes
UsedefineAsyncComponent()for lazy loading heavy Vue components
Use TypeScript for all Vue components and composables with explicit type annotations
Define TypeScript interfaces for Vue component props and data structures
UseusePresentercomposable for main process communication instead of direct IPC calls
Files:
src/renderer/src/components/chat-input/ChatInput.vue
src/renderer/src/**/*.vue
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
Import useI18n from vue-i18n in Vue components to access translation functions t and locale
src/renderer/src/**/*.vue: Use<script setup>syntax for concise Vue 3 component definitions with Composition API
Define props and emits explicitly in Vue components usingdefinePropsanddefineEmitswith TypeScript interfaces
Useprovide/injectfor dependency injection in Vue components instead of prop drilling
Use Tailwind CSS for all styling instead of writing scoped CSS files
Use mobile-first responsive design approach with Tailwind breakpoints
Use Iconify Vue with lucide icons as primary choice, following patternlucide:{icon-name}
Usev-memodirective for memoizing expensive computations in templates
Usev-oncedirective for rendering static content without reactivity updates
Use virtual scrolling withRecycleScrollercomponent for rendering long lists
Subscribe to events usingrendererEvents.on()and unsubscribe inonUnmountedlifecycle hook
Files:
src/renderer/src/components/chat-input/ChatInput.vue
src/renderer/src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
Ensure all code comments are in English and all log messages are in English, with no non-English text in code comments or console statements
Use VueUse composables for common utilities like
useLocalStorage,useClipboard,useDebounceFnVue 3 renderer app code should be organized in
src/renderer/srcwith subdirectories forcomponents/,stores/,views/,i18n/, andlib/
Files:
src/renderer/src/components/chat-input/ChatInput.vue
src/renderer/src/components/**/*.vue
📄 CodeRabbit inference engine (.cursor/rules/vue-stack-guide.mdc)
Name Vue components using PascalCase (e.g.,
ChatInput.vue,MessageItemUser.vue)
Files:
src/renderer/src/components/chat-input/ChatInput.vue
**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
Vue components must be named in PascalCase (e.g.,
ChatInput.vue) and use Vue 3 Composition API with Pinia for state management and Tailwind for styling
**/*.vue: Use Vue 3 Composition API with<script setup>syntax
Use Pinia stores for state management
Style Vue components using Tailwind CSS (v4) with shadcn/ui (reka-ui)
Files:
src/renderer/src/components/chat-input/ChatInput.vue
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,vue}: Use camelCase for variable and function names; use PascalCase for types and classes; use SCREAMING_SNAKE_CASE for constants
Configure Prettier with single quotes, no semicolons, and line width of 100 characters. Runpnpm run formatafter completing features
Files:
src/renderer/src/components/chat-input/ChatInput.vue
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/renderer/src/components/chat-input/ChatInput.vue
src/renderer/**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
All user-facing strings must use i18n keys (supports 12 languages)
Files:
src/renderer/src/components/chat-input/ChatInput.vue
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T02:41:45.219Z
Learning: Use Presenter pattern in main process, EventBus for inter-process events, and two-layer LLM provider architecture (Agent Loop + Provider) with integrated MCP tools
📚 Learning: 2026-01-05T02:41:31.661Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-stack-guide.mdc:0-0
Timestamp: 2026-01-05T02:41:31.661Z
Learning: Applies to src/renderer/src/components/**/*.vue : Name Vue components using PascalCase (e.g., `ChatInput.vue`, `MessageItemUser.vue`)
Applied to files:
src/renderer/src/components/chat-input/ChatInput.vue
📚 Learning: 2026-01-05T02:41:31.661Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-stack-guide.mdc:0-0
Timestamp: 2026-01-05T02:41:31.661Z
Learning: Applies to src/renderer/src/composables/**/*.ts : Name composables using camelCase with `use` prefix (e.g., `useChatState.ts`, `useMessageList.ts`)
Applied to files:
src/renderer/src/components/chat-input/ChatInput.vue
📚 Learning: 2026-01-05T02:41:31.661Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-stack-guide.mdc:0-0
Timestamp: 2026-01-05T02:41:31.661Z
Learning: Applies to src/renderer/src/**/*.{ts,tsx,vue} : Use VueUse composables for common utilities like `useLocalStorage`, `useClipboard`, `useDebounceFn`
Applied to files:
src/renderer/src/components/chat-input/ChatInput.vue
📚 Learning: 2026-01-05T02:41:31.661Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-stack-guide.mdc:0-0
Timestamp: 2026-01-05T02:41:31.661Z
Learning: Applies to src/renderer/src/**/*.{vue,ts,tsx} : Use `usePresenter` composable for main process communication instead of direct IPC calls
Applied to files:
src/renderer/src/components/chat-input/ChatInput.vue
📚 Learning: 2026-01-05T02:41:31.661Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-stack-guide.mdc:0-0
Timestamp: 2026-01-05T02:41:31.661Z
Learning: Applies to src/renderer/src/**/*.vue : Define props and emits explicitly in Vue components using `defineProps` and `defineEmits` with TypeScript interfaces
Applied to files:
src/renderer/src/components/chat-input/ChatInput.vue
🔇 Additional comments (9)
src/renderer/src/components/chat-input/ChatInput.vue (9)
121-146: LGTM!The ACP agent options section is well-structured with proper conditional rendering, i18n usage, and visual feedback for selection state.
371-386: LGTM!The model selector label and conditional badge rendering correctly adapts the UI based on whether ACP session model selection is active.
391-426: LGTM with minor note.The ACP session model selector is well-structured with proper empty states, selection indicators, and i18n compliance. Note: Same loading-state click handling consideration applies here as mentioned for the ACP mode selector.
858-874: LGTM!The computed properties correctly filter ACP models from the model store and extract the selected ACP agent ID with proper null safety.
914-935: LGTM!The ACP session model composable integration and derived computed properties follow consistent patterns with appropriate fallback handling for the label display.
1044-1071: LGTM!The mode selection handler correctly implements automatic model fallback logic when switching between ACP agent and built-in agent modes, with appropriate error handling for configuration persistence.
1073-1093: LGTM!The ACP agent selection handler correctly ensures mode is set before applying the model, with consistent error handling and popover management.
516-577: LGTM - Imports and initialization.The new imports (
useAcpSessionModel,useModelStore) and emit definition follow established patterns in the codebase.
436-437: Config button intentionally hidden for ACP agent mode.When chat mode is 'acp agent', the config button (temperature, system prompt, etc.) is hidden via the
!isAcpChatModecondition at line 437. This is intentional—ACP agents use a separate mode selection system (see line 922showAcpSessionModelSelector) rather than standard configuration options. Verify with the team whether this design aligns with ACP agent capabilities and if any config options should be exposed for ACP workflows.
- Add config warmup directory for fetching modes/models before workdir selection - Add ensureAcpWarmup method to support warmup without user-selected workdir - Add workdir change confirmation dialog to warn users about session reset - Add ACP commands support with COMMANDS_UPDATE event - Add useAcpCommands composable for handling agent commands - Clean up config warmup directory on shutdown
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/renderer/src/components/chat-input/ChatInput.vue`:
- Line 909: The computed property isAgentModeSelected currently uses inverted
logic (chatMode.currentMode.value !== 'acp agent') causing it to be true for
'chat' and other modes; update the computed in ChatInput.vue
(isAgentModeSelected) to check for equality with 'agent' (i.e., use
chatMode.currentMode.value === 'agent') so the Agent button is only active when
the mode is 'agent'.
In `@src/renderer/src/i18n/en-US/chat.json`:
- Around line 24-27: The new translation keys acpWorkdirChangeTitle,
acpWorkdirChangeDescription, acpWorkdirChangeConfirm, and acpWorkdirChangeCancel
were added only to en-US (and zh-CN) but must exist in all 12 language JSON
files; update the remaining language files (da-DK, fa-IR, fr-FR, he-IL, ja-JP,
ko-KR, pt-BR, ru-RU, zh-HK, zh-TW) by adding those four keys with appropriate
localized strings (or temporary English placeholders if translations are not
available), using the exact key names so they match across locales and preserve
JSON formatting and trailing commas consistent with each file.
♻️ Duplicate comments (1)
src/main/presenter/sessionPresenter/index.ts (1)
936-937: Verify: DefaultchatModefallback changed to'agent'.This changes the default fallback for sessions without an explicit
chatModefrom'chat'to'agent'. While this affects only new sessions or sessions with no storedchatMode, the past review indicated there may be migration logic inconversationManager.tsthat forcefully converts existing'chat'settings to'agent'.Please confirm whether existing user sessions with
chatMode: 'chat'are preserved or migrated, and whether this behavioral change is intentional and documented for users.#!/bin/bash # Check for chatMode migration or conversion logic in conversationManager rg -n "chatMode" --type ts -C 5 src/main/presenter/sessionPresenter/managers/conversationManager.ts # Check if there's any migration that converts 'chat' to 'agent' rg -n "'chat'|\"chat\"" --type ts src/main/presenter/sessionPresenter/managers/conversationManager.ts
🧹 Nitpick comments (7)
src/renderer/src/components/chat-input/composables/useAcpWorkdir.ts (1)
195-206: Consider adding error handling to confirmWorkdirChange.The
confirmWorkdirChangefunction callsapplyWorkdirChangewithout a try/catch. If the apply fails, the dialog state is already cleared but the user has no feedback about the failure.🔧 Suggested improvement
const confirmWorkdirChange = async () => { showWorkdirChangeConfirm.value = false if (pendingWorkdirChange.value) { - await applyWorkdirChange(pendingWorkdirChange.value) - pendingWorkdirChange.value = null + const pathToApply = pendingWorkdirChange.value + pendingWorkdirChange.value = null + try { + await applyWorkdirChange(pathToApply) + } catch (error) { + console.warn('[useAcpWorkdir] Failed to apply confirmed workdir change', error) + } } }src/renderer/src/components/chat-input/composables/useAcpCommands.ts (1)
12-16: Consider importingAcpCommandtype from shared types to avoid duplication.This interface duplicates the definition in
src/main/presenter/agentPresenter/acp/acpContentMapper.ts(lines 10-14). To maintain consistency and avoid drift, consider either:
- Moving this type to
src/shared/types/and importing it in both locations, or- Re-exporting from the main process types if already exposed.
docs/specs/acp-integration/enhancement-spec.md (2)
34-51: Add language specifier to fenced code block.The ASCII diagram should have a language specifier for proper rendering and to satisfy the markdownlint rule MD040.
📝 Suggested fix
-``` +```text ┌─────────────────────────────────────────────────────────────┐ │ 配置获取流程（改进后） │
124-134: Add language specifier to fenced code block.Same markdownlint MD040 issue - add a language specifier.
📝 Suggested fix
-``` +```text Agent 发送 available_commands_update ↓ AcpContentMapper 处理通知docs/specs/acp-integration/spec.md (2)
1-69: Consider providing English documentation or bilingual content.The specification is well-structured with clear architecture diagrams and component descriptions. However, per the coding guidelines stating "All logs and comments must be in English", consider whether this extends to technical documentation for consistency across the codebase.
This is a minor concern for internal documentation, but if this spec is intended for broader consumption or contribution, an English version would improve accessibility.
23-69: Add language identifiers to fenced code blocks.Several code blocks are missing language specifiers as flagged by markdownlint. Adding identifiers improves syntax highlighting and tooling support.
Suggested fixes
For ASCII diagrams (line 23), use:
-``` +```textFor table-like content (line 91), use:
-``` +```textFor TypeScript interfaces (lines 133, 310, 322), use:
-``` +```typescriptAlso applies to: 91-99, 133-142, 310-316, 322-331
src/renderer/src/components/chat-input/composables/useAcpMode.ts (1)
96-108: Consider replacing the fixed 500ms delay with an event-driven approach.The hardcoded delay (
setTimeout(resolve, 500)) is a pragmatic workaround but could be fragile under varying system loads or slow agent startups. If the warmup takes longer, modes might not be available yet.Consider either:
- Listening for the
SESSION_MODES_READYevent after triggering warmup- Implementing a polling mechanism with backoff
- Documenting the limitation if the current approach is acceptable for now
That said, this is a warmup path for pre-session UI state, so the impact of occasional misses is limited.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
docs/specs/acp-integration/enhancement-spec.mddocs/specs/acp-integration/spec.mddocs/specs/acp-integration/ux-issues-research.mdsrc/main/events.tssrc/main/presenter/agentPresenter/acp/acpContentMapper.tssrc/main/presenter/agentPresenter/acp/acpProcessManager.tssrc/main/presenter/agentPresenter/acp/index.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/main/presenter/sessionPresenter/index.tssrc/renderer/src/components/chat-input/ChatInput.vuesrc/renderer/src/components/chat-input/composables/useAcpCommands.tssrc/renderer/src/components/chat-input/composables/useAcpMode.tssrc/renderer/src/components/chat-input/composables/useAcpWorkdir.tssrc/renderer/src/components/chat-input/composables/useAgentWorkspace.tssrc/renderer/src/events.tssrc/renderer/src/i18n/en-US/chat.jsonsrc/renderer/src/i18n/zh-CN/chat.jsonsrc/shared/types/presenters/llmprovider.presenter.d.tssrc/shared/types/presenters/session.presenter.d.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/shared/types/presenters/session.presenter.d.ts
- src/renderer/src/events.ts
🧰 Additional context used
📓 Path-based instructions (22)
**/*.{js,ts,tsx,jsx,vue,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
All logs and comments must be in English
Files:
src/main/presenter/agentPresenter/acp/index.tssrc/renderer/src/components/chat-input/composables/useAgentWorkspace.tssrc/renderer/src/components/chat-input/composables/useAcpCommands.tssrc/main/presenter/agentPresenter/acp/acpContentMapper.tssrc/main/events.tssrc/renderer/src/components/chat-input/composables/useAcpMode.tssrc/renderer/src/components/chat-input/ChatInput.vuesrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/renderer/src/components/chat-input/composables/useAcpWorkdir.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/agentPresenter/acp/acpProcessManager.tssrc/main/presenter/sessionPresenter/index.ts
**/*.{js,ts,tsx,jsx,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
Use OxLint as the linter
Files:
src/main/presenter/agentPresenter/acp/index.tssrc/renderer/src/components/chat-input/composables/useAgentWorkspace.tssrc/renderer/src/components/chat-input/composables/useAcpCommands.tssrc/main/presenter/agentPresenter/acp/acpContentMapper.tssrc/main/events.tssrc/renderer/src/components/chat-input/composables/useAcpMode.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/renderer/src/components/chat-input/composables/useAcpWorkdir.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/agentPresenter/acp/acpProcessManager.tssrc/main/presenter/sessionPresenter/index.ts
**/*.{js,ts,tsx,jsx,vue,json,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
Use Prettier as the code formatter
Files:
src/main/presenter/agentPresenter/acp/index.tssrc/renderer/src/components/chat-input/composables/useAgentWorkspace.tssrc/renderer/src/components/chat-input/composables/useAcpCommands.tssrc/renderer/src/i18n/zh-CN/chat.jsonsrc/main/presenter/agentPresenter/acp/acpContentMapper.tssrc/main/events.tssrc/renderer/src/components/chat-input/composables/useAcpMode.tssrc/renderer/src/components/chat-input/ChatInput.vuesrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/renderer/src/components/chat-input/composables/useAcpWorkdir.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/renderer/src/i18n/en-US/chat.jsonsrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/agentPresenter/acp/acpProcessManager.tssrc/main/presenter/sessionPresenter/index.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Electron main process code should reside in
src/main/, with presenters organized inpresenter/subdirectory (Window, Tab, Thread, Mcp, Config, LLMProvider), and app events managed viaeventbus.ts
Files:
src/main/presenter/agentPresenter/acp/index.tssrc/main/presenter/agentPresenter/acp/acpContentMapper.tssrc/main/events.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/agentPresenter/acp/acpProcessManager.tssrc/main/presenter/sessionPresenter/index.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,vue}: Use camelCase for variable and function names; use PascalCase for types and classes; use SCREAMING_SNAKE_CASE for constants
Configure Prettier with single quotes, no semicolons, and line width of 100 characters. Runpnpm run formatafter completing features
Files:
src/main/presenter/agentPresenter/acp/index.tssrc/renderer/src/components/chat-input/composables/useAgentWorkspace.tssrc/renderer/src/components/chat-input/composables/useAcpCommands.tssrc/main/presenter/agentPresenter/acp/acpContentMapper.tssrc/main/events.tssrc/renderer/src/components/chat-input/composables/useAcpMode.tssrc/renderer/src/components/chat-input/ChatInput.vuesrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/renderer/src/components/chat-input/composables/useAcpWorkdir.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/agentPresenter/acp/acpProcessManager.tssrc/main/presenter/sessionPresenter/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use OxLint for linting JavaScript and TypeScript files; ensure lint-staged hooks and typecheck pass before commits
Enable strict TypeScript type checking
Files:
src/main/presenter/agentPresenter/acp/index.tssrc/renderer/src/components/chat-input/composables/useAgentWorkspace.tssrc/renderer/src/components/chat-input/composables/useAcpCommands.tssrc/main/presenter/agentPresenter/acp/acpContentMapper.tssrc/main/events.tssrc/renderer/src/components/chat-input/composables/useAcpMode.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/renderer/src/components/chat-input/composables/useAcpWorkdir.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/agentPresenter/acp/acpProcessManager.tssrc/main/presenter/sessionPresenter/index.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/main/presenter/agentPresenter/acp/index.tssrc/renderer/src/components/chat-input/composables/useAgentWorkspace.tssrc/renderer/src/components/chat-input/composables/useAcpCommands.tssrc/main/presenter/agentPresenter/acp/acpContentMapper.tssrc/main/events.tssrc/renderer/src/components/chat-input/composables/useAcpMode.tssrc/renderer/src/components/chat-input/ChatInput.vuesrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/renderer/src/components/chat-input/composables/useAcpWorkdir.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/agentPresenter/acp/acpProcessManager.tssrc/main/presenter/sessionPresenter/index.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Implement all system capabilities as main-process presenters following the Presenter Pattern
Files:
src/main/presenter/agentPresenter/acp/index.tssrc/main/presenter/agentPresenter/acp/acpContentMapper.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/agentPresenter/acp/acpProcessManager.tssrc/main/presenter/sessionPresenter/index.ts
{src/main,src/renderer,test}/**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Use IPC communication: Renderer calls main process via
usePresentercomposable, Main sends to Renderer via EventBus
Files:
src/main/presenter/agentPresenter/acp/index.tssrc/renderer/src/components/chat-input/composables/useAgentWorkspace.tssrc/renderer/src/components/chat-input/composables/useAcpCommands.tssrc/main/presenter/agentPresenter/acp/acpContentMapper.tssrc/main/events.tssrc/renderer/src/components/chat-input/composables/useAcpMode.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/renderer/src/components/chat-input/composables/useAcpWorkdir.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/agentPresenter/acp/acpProcessManager.tssrc/main/presenter/sessionPresenter/index.ts
src/renderer/src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
src/renderer/src/**/*.{vue,ts,tsx}: Use vue-i18n framework for internationalization located at src/renderer/src/i18n/
All user-facing strings must use i18n keys, not hardcoded text
src/renderer/src/**/*.{vue,ts,tsx}: Usereffor primitives and references,reactivefor objects in Vue 3 Composition API
Prefercomputedproperties over methods for derived state in Vue components
Import Shadcn Vue components from@/shadcn/components/ui/path alias
Use thecn()utility function combining clsx and tailwind-merge for dynamic Tailwind classes
UsedefineAsyncComponent()for lazy loading heavy Vue components
Use TypeScript for all Vue components and composables with explicit type annotations
Define TypeScript interfaces for Vue component props and data structures
UseusePresentercomposable for main process communication instead of direct IPC calls
Files:
src/renderer/src/components/chat-input/composables/useAgentWorkspace.tssrc/renderer/src/components/chat-input/composables/useAcpCommands.tssrc/renderer/src/components/chat-input/composables/useAcpMode.tssrc/renderer/src/components/chat-input/ChatInput.vuesrc/renderer/src/components/chat-input/composables/useAcpWorkdir.ts
src/renderer/src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
Ensure all code comments are in English and all log messages are in English, with no non-English text in code comments or console statements
Use VueUse composables for common utilities like
useLocalStorage,useClipboard,useDebounceFnVue 3 renderer app code should be organized in
src/renderer/srcwith subdirectories forcomponents/,stores/,views/,i18n/, andlib/
Files:
src/renderer/src/components/chat-input/composables/useAgentWorkspace.tssrc/renderer/src/components/chat-input/composables/useAcpCommands.tssrc/renderer/src/components/chat-input/composables/useAcpMode.tssrc/renderer/src/components/chat-input/ChatInput.vuesrc/renderer/src/components/chat-input/composables/useAcpWorkdir.ts
src/renderer/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/vue-stack-guide.mdc)
Use class-variance-authority (CVA) for defining component variants with Tailwind classes
Files:
src/renderer/src/components/chat-input/composables/useAgentWorkspace.tssrc/renderer/src/components/chat-input/composables/useAcpCommands.tssrc/renderer/src/components/chat-input/composables/useAcpMode.tssrc/renderer/src/components/chat-input/composables/useAcpWorkdir.ts
src/renderer/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/vue-stack-guide.mdc)
src/renderer/src/**/*.{ts,tsx}: UseshallowRefandshallowReactivefor optimizing reactivity with large objects
Prefertypeoverinterfacein TypeScript unless using inheritance withextends
Files:
src/renderer/src/components/chat-input/composables/useAgentWorkspace.tssrc/renderer/src/components/chat-input/composables/useAcpCommands.tssrc/renderer/src/components/chat-input/composables/useAcpMode.tssrc/renderer/src/components/chat-input/composables/useAcpWorkdir.ts
src/renderer/**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
All user-facing strings must use i18n keys (supports 12 languages)
Files:
src/renderer/src/components/chat-input/composables/useAgentWorkspace.tssrc/renderer/src/components/chat-input/composables/useAcpCommands.tssrc/renderer/src/components/chat-input/composables/useAcpMode.tssrc/renderer/src/components/chat-input/ChatInput.vuesrc/renderer/src/components/chat-input/composables/useAcpWorkdir.ts
src/renderer/src/i18n/**/*.json
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
src/renderer/src/i18n/**/*.json: Use dot-separated hierarchical structure for translation key naming with lowercase letters and descriptive names grouped by feature/context (e.g., common.button.submit, chat.send.placeholder)
Add new translations to ALL language files (da-DK, en-US, fa-IR, fr-FR, he-IL, ja-JP, ko-KR, pt-BR, ru-RU, zh-CN, zh-HK, zh-TW) with consistent key names across all locales
Keep proper nouns and technical terms untranslated (e.g., 'DeepChat', 'MCP', 'Agents' in English) unless established convention exists, and follow established conventions for well-known technical terms
Files:
src/renderer/src/i18n/zh-CN/chat.jsonsrc/renderer/src/i18n/en-US/chat.json
src/renderer/src/i18n/{zh-CN,zh-HK,zh-TW}/**/*.json
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
For Chinese translations, consider using '智能体' for 'Agents' instead of the English term
Files:
src/renderer/src/i18n/zh-CN/chat.json
src/renderer/src/**/*.vue
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
Import useI18n from vue-i18n in Vue components to access translation functions t and locale
src/renderer/src/**/*.vue: Use<script setup>syntax for concise Vue 3 component definitions with Composition API
Define props and emits explicitly in Vue components usingdefinePropsanddefineEmitswith TypeScript interfaces
Useprovide/injectfor dependency injection in Vue components instead of prop drilling
Use Tailwind CSS for all styling instead of writing scoped CSS files
Use mobile-first responsive design approach with Tailwind breakpoints
Use Iconify Vue with lucide icons as primary choice, following patternlucide:{icon-name}
Usev-memodirective for memoizing expensive computations in templates
Usev-oncedirective for rendering static content without reactivity updates
Use virtual scrolling withRecycleScrollercomponent for rendering long lists
Subscribe to events usingrendererEvents.on()and unsubscribe inonUnmountedlifecycle hook
Files:
src/renderer/src/components/chat-input/ChatInput.vue
src/renderer/src/components/**/*.vue
📄 CodeRabbit inference engine (.cursor/rules/vue-stack-guide.mdc)
Name Vue components using PascalCase (e.g.,
ChatInput.vue,MessageItemUser.vue)
Files:
src/renderer/src/components/chat-input/ChatInput.vue
**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
Vue components must be named in PascalCase (e.g.,
ChatInput.vue) and use Vue 3 Composition API with Pinia for state management and Tailwind for styling
**/*.vue: Use Vue 3 Composition API with<script setup>syntax
Use Pinia stores for state management
Style Vue components using Tailwind CSS (v4) with shadcn/ui (reka-ui)
Files:
src/renderer/src/components/chat-input/ChatInput.vue
src/main/presenter/llmProviderPresenter/providers/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Implement
coreStreammethod following standardized event interface for LLM providers
Files:
src/main/presenter/llmProviderPresenter/providers/acpProvider.ts
src/shared/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Shared TypeScript types and utilities should be placed in
src/shared/
Files:
src/shared/types/presenters/llmprovider.presenter.d.ts
src/renderer/src/i18n/en-US/**/*.json
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
Use English (en-US) as the reference for translation accuracy when adding new keys
Files:
src/renderer/src/i18n/en-US/chat.json
🧠 Learnings (22)
📓 Common learnings
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T02:41:45.219Z
Learning: Use Presenter pattern in main process, EventBus for inter-process events, and two-layer LLM provider architecture (Agent Loop + Provider) with integrated MCP tools
📚 Learning: 2026-01-13T09:23:07.415Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:23:07.415Z
Learning: Applies to src/main/presenter/mcpPresenter/inMemoryServers/**/*.ts : Implement new MCP tools in the inMemoryServers directory and register in mcpPresenter/index.ts
Applied to files:
src/main/presenter/agentPresenter/acp/index.tssrc/main/presenter/agentPresenter/acp/acpContentMapper.tssrc/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/sessionPresenter/index.ts
📚 Learning: 2026-01-05T02:41:31.661Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-stack-guide.mdc:0-0
Timestamp: 2026-01-05T02:41:31.661Z
Learning: Applies to src/renderer/src/composables/**/*.ts : Name composables using camelCase with `use` prefix (e.g., `useChatState.ts`, `useMessageList.ts`)
Applied to files:
src/renderer/src/components/chat-input/composables/useAgentWorkspace.tssrc/renderer/src/components/chat-input/composables/useAcpCommands.tssrc/renderer/src/components/chat-input/composables/useAcpMode.tssrc/renderer/src/components/chat-input/ChatInput.vuesrc/renderer/src/components/chat-input/composables/useAcpWorkdir.ts
📚 Learning: 2026-01-05T02:41:31.661Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-stack-guide.mdc:0-0
Timestamp: 2026-01-05T02:41:31.661Z
Learning: Applies to src/renderer/src/**/*.{ts,tsx,vue} : Use VueUse composables for common utilities like `useLocalStorage`, `useClipboard`, `useDebounceFn`
Applied to files:
src/renderer/src/components/chat-input/composables/useAcpCommands.tssrc/renderer/src/components/chat-input/ChatInput.vue
📚 Learning: 2026-01-05T02:41:31.661Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-stack-guide.mdc:0-0
Timestamp: 2026-01-05T02:41:31.661Z
Learning: Applies to src/renderer/src/**/*.{vue,ts,tsx} : Use `usePresenter` composable for main process communication instead of direct IPC calls
Applied to files:
src/renderer/src/components/chat-input/composables/useAcpCommands.tssrc/renderer/src/components/chat-input/ChatInput.vuesrc/renderer/src/components/chat-input/composables/useAcpWorkdir.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/sessionPresenter/index.ts
📚 Learning: 2026-01-05T02:41:13.302Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/i18n.mdc:0-0
Timestamp: 2026-01-05T02:41:13.302Z
Learning: Applies to src/renderer/src/i18n/**/*.json : Use dot-separated hierarchical structure for translation key naming with lowercase letters and descriptive names grouped by feature/context (e.g., common.button.submit, chat.send.placeholder)
Applied to files:
src/renderer/src/i18n/zh-CN/chat.jsonsrc/renderer/src/i18n/en-US/chat.json
📚 Learning: 2026-01-05T02:41:13.302Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/i18n.mdc:0-0
Timestamp: 2026-01-05T02:41:13.302Z
Learning: Applies to src/renderer/src/i18n/**/*.json : Add new translations to ALL language files (da-DK, en-US, fa-IR, fr-FR, he-IL, ja-JP, ko-KR, pt-BR, ru-RU, zh-CN, zh-HK, zh-TW) with consistent key names across all locales
Applied to files:
src/renderer/src/i18n/zh-CN/chat.jsonsrc/renderer/src/i18n/en-US/chat.json
📚 Learning: 2026-01-05T02:41:13.302Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/i18n.mdc:0-0
Timestamp: 2026-01-05T02:41:13.302Z
Learning: Applies to src/renderer/src/i18n/**/*.json : Keep proper nouns and technical terms untranslated (e.g., 'DeepChat', 'MCP', 'Agents' in English) unless established convention exists, and follow established conventions for well-known technical terms
Applied to files:
src/renderer/src/i18n/zh-CN/chat.jsonsrc/renderer/src/i18n/en-US/chat.json
📚 Learning: 2026-01-05T02:41:13.302Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/i18n.mdc:0-0
Timestamp: 2026-01-05T02:41:13.302Z
Learning: Applies to src/renderer/src/i18n/{zh-CN,zh-HK,zh-TW}/**/*.json : For Chinese translations, consider using '智能体' for 'Agents' instead of the English term
Applied to files:
src/renderer/src/i18n/zh-CN/chat.json
📚 Learning: 2026-01-13T09:23:07.415Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:23:07.415Z
Learning: Applies to src/renderer/**/*.{ts,tsx,js,jsx,vue} : All user-facing strings must use i18n keys (supports 12 languages)
Applied to files:
src/renderer/src/i18n/zh-CN/chat.jsonsrc/renderer/src/i18n/en-US/chat.json
📚 Learning: 2026-01-05T02:41:13.302Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/i18n.mdc:0-0
Timestamp: 2026-01-05T02:41:13.302Z
Learning: Applies to src/renderer/src/**/*.{ts,tsx,vue} : Ensure all code comments are in English and all log messages are in English, with no non-English text in code comments or console statements
Applied to files:
src/renderer/src/i18n/zh-CN/chat.json
📚 Learning: 2026-01-05T02:41:31.661Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-stack-guide.mdc:0-0
Timestamp: 2026-01-05T02:41:31.661Z
Learning: Applies to src/renderer/src/components/**/*.vue : Name Vue components using PascalCase (e.g., `ChatInput.vue`, `MessageItemUser.vue`)
Applied to files:
src/renderer/src/components/chat-input/ChatInput.vue
📚 Learning: 2026-01-05T02:41:31.661Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-stack-guide.mdc:0-0
Timestamp: 2026-01-05T02:41:31.661Z
Learning: Applies to src/renderer/src/components/ : Organize Vue component directories using lowercase with dashes (e.g., `chat-input/`, `message/`)
Applied to files:
src/renderer/src/components/chat-input/ChatInput.vue
📚 Learning: 2026-01-05T02:41:31.661Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-stack-guide.mdc:0-0
Timestamp: 2026-01-05T02:41:31.661Z
Learning: Applies to src/renderer/src/**/*.{vue,ts,tsx} : Import Shadcn Vue components from `@/shadcn/components/ui/` path alias
Applied to files:
src/renderer/src/components/chat-input/ChatInput.vue
📚 Learning: 2026-01-05T02:41:31.661Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-stack-guide.mdc:0-0
Timestamp: 2026-01-05T02:41:31.661Z
Learning: Applies to src/renderer/src/**/*.vue : Define props and emits explicitly in Vue components using `defineProps` and `defineEmits` with TypeScript interfaces
Applied to files:
src/renderer/src/components/chat-input/ChatInput.vue
📚 Learning: 2026-01-13T09:23:07.415Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:23:07.415Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Implement `coreStream` method following standardized event interface for LLM providers
Applied to files:
src/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/llmProviderPresenter/index.ts
📚 Learning: 2026-01-13T09:23:07.415Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:23:07.415Z
Learning: Applies to src/main/presenter/**/*.ts : Implement all system capabilities as main-process presenters following the Presenter Pattern
Applied to files:
src/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/agentPresenter/acp/acpProcessManager.tssrc/main/presenter/sessionPresenter/index.ts
📚 Learning: 2026-01-05T02:41:45.219Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T02:41:45.219Z
Learning: Use Presenter pattern in main process, EventBus for inter-process events, and two-layer LLM provider architecture (Agent Loop + Provider) with integrated MCP tools
Applied to files:
src/main/presenter/llmProviderPresenter/providers/acpProvider.tssrc/shared/types/presenters/llmprovider.presenter.d.tssrc/main/presenter/llmProviderPresenter/index.tssrc/main/presenter/sessionPresenter/index.ts
📚 Learning: 2026-01-13T09:23:07.415Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T09:23:07.415Z
Learning: Applies to {src/main,src/renderer,test}/**/*.{ts,tsx,js} : Use IPC communication: Renderer calls main process via `usePresenter` composable, Main sends to Renderer via EventBus
Applied to files:
src/shared/types/presenters/llmprovider.presenter.d.ts
📚 Learning: 2026-01-05T02:41:13.302Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/i18n.mdc:0-0
Timestamp: 2026-01-05T02:41:13.302Z
Learning: Applies to src/renderer/src/i18n/en-US/**/*.json : Use English (en-US) as the reference for translation accuracy when adding new keys
Applied to files:
src/renderer/src/i18n/en-US/chat.json
📚 Learning: 2026-01-05T02:41:45.219Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-05T02:41:45.219Z
Learning: Applies to src/renderer/src/i18n/**/*.ts : All user-facing strings must use vue-i18n keys defined in `src/renderer/src/i18n/`
Applied to files:
src/renderer/src/i18n/en-US/chat.json
📚 Learning: 2026-01-05T02:41:13.302Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/i18n.mdc:0-0
Timestamp: 2026-01-05T02:41:13.302Z
Learning: Applies to src/renderer/src/**/*.{vue,ts,tsx} : All user-facing strings must use i18n keys, not hardcoded text
Applied to files:
src/renderer/src/i18n/en-US/chat.json
🧬 Code graph analysis (6)
src/renderer/src/components/chat-input/composables/useAcpCommands.ts (4)
src/main/presenter/agentPresenter/acp/acpContentMapper.ts (2)
AcpCommand(11-15)handleCommandsUpdate(248-264)src/main/presenter/agentPresenter/acp/index.ts (1)
AcpCommand(13-13)src/main/events.ts (1)
ACP_WORKSPACE_EVENTS(256-260)src/renderer/src/events.ts (1)
ACP_WORKSPACE_EVENTS(208-212)
src/main/presenter/agentPresenter/acp/acpContentMapper.ts (2)
src/main/presenter/agentPresenter/acp/index.ts (1)
AcpCommand(13-13)src/renderer/src/components/chat-input/composables/useAcpCommands.ts (1)
AcpCommand(12-16)
src/main/presenter/llmProviderPresenter/providers/acpProvider.ts (3)
src/main/eventbus.ts (1)
eventBus(151-151)src/main/events.ts (1)
ACP_WORKSPACE_EVENTS(256-260)src/renderer/src/events.ts (1)
ACP_WORKSPACE_EVENTS(208-212)
src/shared/types/presenters/llmprovider.presenter.d.ts (1)
src/shared/types/presenters/legacy.presenters.d.ts (1)
AcpSessionModelInfo(906-910)
src/main/presenter/agentPresenter/acp/acpProcessManager.ts (6)
test/mocks/electron.ts (1)
app(2-10)src/shared/types/presenters/legacy.presenters.d.ts (1)
AcpAgentConfig(874-880)src/main/presenter/agentPresenter/acp/index.ts (1)
AcpProcessHandle(5-5)src/main/eventbus.ts (1)
eventBus(151-151)src/main/events.ts (1)
ACP_WORKSPACE_EVENTS(256-260)src/renderer/src/events.ts (1)
ACP_WORKSPACE_EVENTS(208-212)
src/main/presenter/sessionPresenter/index.ts (1)
src/shared/types/presenters/session.presenter.d.ts (1)
Session(50-58)
🪛 markdownlint-cli2 (0.18.1)
docs/specs/acp-integration/enhancement-spec.md
34-34: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
124-124: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/specs/acp-integration/spec.md
23-23: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
91-91: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
133-133: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
310-310: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
322-322: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (53)
src/main/events.ts (1)
256-260: LGTM!The new ACP workspace events
SESSION_MODELS_READYandCOMMANDS_UPDATEfollow the established naming convention and integrate well with the existing event structure. These events support the broader ACP model/session management changes in this PR.src/main/presenter/agentPresenter/acp/index.ts (1)
13-13: LGTM!The type-only export of
AcpCommandfollows the established pattern in this index file and properly exposes the new interface for consumers.src/main/presenter/agentPresenter/acp/acpContentMapper.ts (3)
11-26: LGTM!The
AcpCommandinterface is well-defined and matches the corresponding interface in the renderer (src/renderer/src/components/chat-input/composables/useAcpCommands.ts). The extension ofMappedContentwithavailableCommandsfollows the established pattern for optional payload fields.
66-72: LGTM!The
available_commands_updatecase is handled consistently with other session update types, with appropriate logging and delegation to the handler.
248-264: LGTM!The
handleCommandsUpdatemethod correctly maps ACP commands to the payload format. The null-coalescing toundefinedfor optional fields (description,input) ensures type consistency with the interface definition.src/renderer/src/components/chat-input/composables/useAcpWorkdir.ts (4)
24-27: LGTM!The confirmation dialog state refs follow Vue 3 composition API best practices with appropriate initial values.
151-174: LGTM!The
applyWorkdirChangefunction cleanly encapsulates the workdir change logic, correctly handling both active conversation and pre-conversation scenarios. The preference sync and warmup are properly sequenced.
176-193: Well-designed confirmation flow.The conditional at line 186 appropriately gates the confirmation dialog to only show when there's a meaningful change (existing conversation with different workdir). This prevents unnecessary prompts for new conversations or same-path selections.
210-221: LGTM!The return object properly exposes the new confirmation state and handlers, enabling the UI layer to implement the confirmation dialog.
src/renderer/src/components/chat-input/composables/useAgentWorkspace.ts (1)
237-243: LGTM!Clean composition pattern - exposing the ACP workdir confirmation state and methods through the unified
useAgentWorkspacefacade allows consumers to access the confirmation flow without needing to know the internal composition.src/renderer/src/components/chat-input/composables/useAcpCommands.ts (1)
18-103: LGTM! Well-structured composable with proper lifecycle management.The composable correctly:
- Uses
onMounted/onUnmountedfor IPC listener lifecycle management to prevent memory leaks- Clears commands appropriately when agent changes or when switching away from ACP model
- Uses computed properties for derived state (
isAcpModel,hasCommands)src/shared/types/presenters/llmprovider.presenter.d.ts (2)
5-10: LGTM! Import properly includesAcpSessionModelInfo.The type-only import correctly brings in the new
AcpSessionModelInfotype needed for the new model management methods.
272-304: LGTM! Well-designed API surface for ACP model management.The new methods follow the established pattern with symmetric process-level and session-level APIs:
getAcpProcessModels/setAcpPreferredProcessModelfor warmup process configurationgetAcpSessionModels/setAcpSessionModelfor active session configurationensureAcpWarmupallowsnullworkdir to support the config-warmup directory fallback described in the specAll implementations are complete and actively integrated with the renderer composables (
useAcpMode.tsanduseAcpSessionModel.ts), confirming the full feature is working end-to-end.docs/specs/acp-integration/enhancement-spec.md (1)
1-317: Well-structured design specification.The document clearly outlines the three-phase implementation plan for ACP UX enhancements:
- Mode/Model pre-fetch via warmup directory
- Available commands UI exposure
- Workdir switching with confirmation dialog
The API designs align with the interface changes in
llmprovider.presenter.d.ts.docs/specs/acp-integration/ux-issues-research.md (1)
1-388: Comprehensive UX research document.This research document provides excellent context for the design decisions, including:
- Analysis of the two-layer mode system (warmup vs session)
- Root cause identification for mode/model pre-loading issues
- Prioritized recommendations with feasibility ratings
The recommended solution (internal tmp directory for warmup) is now implemented as
ensureAcpWarmup(agentId, workdir: null)in the presenter interface.src/main/presenter/llmProviderPresenter/providers/acpProvider.ts (7)
343-355: LGTM! COMMANDS_UPDATE event emission is well-placed.The event emission is conditionally triggered only when
availableCommandsis present and non-empty, preventing unnecessary IPC traffic.
381-393: LGTM! SESSION_MODELS_READY event follows the same pattern as SESSION_MODES_READY.The implementation is consistent with the existing modes notification at lines 367-379, maintaining symmetry between mode and model handling.
459-475: LGTM! ensureWarmup provides a clean abstraction for pre-session model/mode discovery.The null coalescing to
getConfigWarmupDir()allows fetching modes/models before a user selects a workdir, which is a useful UX improvement for the ACP agent selection flow.
489-499: LGTM! getProcessModels mirrors getProcessModes for consistency.The method signature and implementation follow the established pattern.
515-527: LGTM! setPreferredProcessModel follows the setPreferredProcessMode pattern.Error handling is consistent with the mode counterpart at lines 501-513.
1188-1236: LGTM! setSessionModel is well-implemented with proper validation and notifications.The implementation:
- Validates session existence
- Logs available models for debugging
- Warns when requested model isn't in available list (but doesn't block - good for flexibility)
- Updates both session and process handle state
- Notifies renderer via SESSION_MODELS_READY event
- Has comprehensive error handling with re-throw
This mirrors the
setSessionModeimplementation pattern exactly, maintaining consistency.
1264-1288: LGTM! getSessionModels mirrors getSessionModes implementation.The method provides proper null handling and informative logging for debugging.
src/main/presenter/agentPresenter/acp/acpProcessManager.ts (11)
33-38: LGTM! AcpProcessHandle extension for model support.The interface extension cleanly adds model tracking fields alongside the existing mode fields, maintaining consistency.
152-203: Good implementation of config warmup directory management.The
getConfigWarmupDircreates a dedicated directory under userData for fetching modes/models before workdir selection. ThecleanupConfigWarmupDirproperly handles nested directories and files with appropriate error handling.One minor consideration: The cleanup iterates synchronously which could block if there are many files, but for a warmup directory this is unlikely to be an issue.
241-243: LGTM! Consistent application of preferred model alongside preferred mode.The warmup flow now applies both preferred mode and model to handles, ensuring user preferences are respected across all reuse paths (existing handle, inflight handle, and newly spawned handle).
Also applies to: 257-259, 273-275, 301-303
332-347: LGTM! setPreferredModel follows the setPreferredMode pattern exactly.The implementation correctly:
- Stores the preference keyed by agent+workdir
- Applies to existing warmup handles if available
- Notifies renderer of the change
419-420: Good practice: Cleanup warmup directory on shutdown.This prevents disk space accumulation from orphaned warmup files across application restarts.
463-465: LGTM! Model readiness notification added to bindProcess.The renderer is now notified of both modes and models when a process is bound to a conversation, enabling proper UI state initialization.
504-524: LGTM! getProcessModels mirrors getProcessModes implementation.The method follows the established pattern for retrieving process-level state.
647-658: LGTM! Model data mapping from init response.The mapping correctly transforms
modelIdtoidand provides sensible defaults fornameanddescriptionfields.
1061-1079: LGTM! Model handling in fetchProcessModes.The implementation correctly:
- Maps model data with proper field transformation
- Preserves user-selected preferred model if it exists in available list
- Falls back to agent's current model or first available
- Notifies renderer of model readiness
This mirrors the mode handling logic below it.
1130-1140: LGTM! notifyModelsReady follows notifyModesReady pattern.The guard condition and event payload structure are consistent with the mode notification at lines 1118-1128.
1197-1201: LGTM! applyPreferredModel helper mirrors applyPreferredMode.Simple and consistent implementation.
src/renderer/src/components/chat-input/composables/useAcpMode.ts (4)
34-34: LGTM! Clean extraction of agentId as a computed property.This provides a reusable reference for the current agent's ID throughout the composable.
66-69: LGTM! Explicit state reset on empty/failed results.Previously, the state might remain stale if
loadModesreturned no results. Explicitly resetting to defaults ensures consistent UI state.
155-161: LGTM! Agent change resets state appropriately.This prevents stale mode data from a previous agent from bleeding into the new agent's UI state. The guard
if (!newId || newId === oldId) returnprevents unnecessary resets.
209-254: LGTM! Clean refactor separating setMode from cycleMode.The
setModefunction now handles the core logic of mode changes, whilecycleModefocuses solely on computing the next mode ID. This separation of concerns improves testability and enables direct mode selection from UI dropdowns.src/renderer/src/components/chat-input/ChatInput.vue (9)
94-146: LGTM! ACP agent selection UI in mode popover.The implementation provides clear visual distinction between the generic "agent" mode and specific ACP agents. Model icons and selection checkmarks improve discoverability.
289-335: LGTM! ACP mode selector refactored to use popover.The dropdown approach is more user-friendly than cycling, especially when agents support multiple modes. The disabled state during loading prevents race conditions.
377-386: LGTM! Conditional badge rendering based on ACP session model state.Hiding the model tags when showing ACP session model selector prevents visual clutter and confusion.
391-426: LGTM! ACP session model selector integration.The selector follows the same UI patterns as other selectors in the component, with proper loading states and selection indicators.
513-535: LGTM! Workdir change confirmation dialog.Using AlertDialog for the confirmation provides a clear user interaction pattern. The event handlers properly delegate to the workspace composable methods.
947-980: LGTM! ACP session model and mode handlers are well-structured.Both handlers:
- Check loading state to prevent race conditions
- Close the popover after selection
- Delegate to the respective composable methods
1037-1075: LGTM! Centralized model selection logic.
applyModelSelectionproperly handles both chat and newThread variants, routing to the appropriate update mechanism. The fallback helperspickFirstAcpModelandpickFirstNonAcpModelensure valid model state when switching between modes.
1077-1104: LGTM! Enhanced handleModeSelect with automatic model switching.When switching between agent modes, the handler automatically selects an appropriate model:
- Switching away from ACP agent → picks first non-ACP model
- Switching to ACP agent → picks first ACP model
This provides a smoother UX by preventing invalid model/mode combinations.
1106-1126: LGTM! handleAcpAgentSelect properly sets mode and applies model.The handler ensures both the chat mode and model are updated atomically, with proper persistence to chat config.
src/main/presenter/llmProviderPresenter/index.ts (4)
505-525: LGTM! Consistent error handling pattern.The
ensureAcpWarmupmethod follows the same error handling pattern as the existingwarmupAcpProcessmethod (lines 488-503), including the graceful handling of shutdown scenarios. The JSDoc clearly documents the null workdir behavior.
544-559: LGTM! Consistent with existing modes API.The
getAcpProcessModelsmethod mirrors the structure ofgetAcpProcessModes, maintaining API consistency for the ACP model layer.
568-573: LGTM! Appropriate error handling differentiation.The
setAcpPreferredProcessModelsilently returns when provider is unavailable (consistent withsetAcpPreferredProcessMode), whilesetAcpSessionModelthrows an error (consistent withsetAcpSessionMode). This distinction makes sense: process-level preferences are optional configuration, whereas session-level operations are critical path actions that should fail explicitly.Also applies to: 583-589
602-611: LGTM! Consistent null handling.The
getAcpSessionModelsmethod properly returnsnullwhen the provider is unavailable, matching the behavior ofgetAcpSessionModes.src/main/presenter/sessionPresenter/index.ts (3)
792-794: LGTM! Simple delegation.The
ensureAcpWarmupmethod properly delegates to the LLM provider presenter.
809-820: LGTM! Consistent delegate pattern.The method properly delegates to
llmProviderPresenter.getAcpProcessModelswith matching return type.
826-828: LGTM! Consistent ACP model delegates.The
setAcpPreferredProcessModel,setAcpSessionModel, andgetAcpSessionModelsmethods follow the established delegation pattern to the LLM provider presenter.Also applies to: 834-836, 845-850
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/main/presenter/windowPresenter/index.ts`:
- Around line 134-137: The shortcut handler currently calls the async method
createChatWindow() without handling its returned Promise, which can cause
unhandled rejections; update the shortcut callback in the window presenter to
either await createChatWindow() inside an async handler or explicitly suppress
the Promise with void (e.g., void this.createChatWindow()), and add a try/catch
around the awaited call to log or handle errors from createChatWindow() so
failures don't produce unhandled rejection warnings.
- Around line 1725-1728: The chatWindow.loadFile call is passing the renderer
directory instead of the HTML file referenced in the log; update the call so
chatWindow.loadFile receives the concrete HTML path (i.e., the same path used in
the console.log: join(__dirname, '../renderer/index.html')) to match other
usages (see chatWindow.loadFile and the console.log message) and ensure the
renderer loads correctly.
♻️ Duplicate comments (2)
src/renderer/src/components/ChatAppBar.vue (1)
12-78: Add aria-labels for icon-only controlsIcon-only buttons are missing accessible names; screen readers won’t announce their purpose. Bind
aria-labelto the same i18n strings used for tooltips.♿ Proposed fix (aria-labels)
<Button size="icon" + :aria-label="t('common.browser.name')" class="window-no-drag-region shrink-0 w-10 bg-transparent shadow-none rounded-none hover:bg-card/80 text-xs font-medium text-foreground flex items-center justify-center transition-all duration-200 group" `@click`="onBrowserClick" `@mouseenter`="onOverlayMouseEnter('browser', t('common.browser.name'), $event)" `@mouseleave`="onOverlayMouseLeave('browser')" > <Icon icon="lucide:compass" class="w-4 h-4" /> </Button> <Button size="icon" + :aria-label="t('common.history')" class="window-no-drag-region shrink-0 w-10 bg-transparent shadow-none rounded-none hover:bg-card/80 text-xs font-medium text-foreground flex items-center justify-center transition-all duration-200 group" `@click`="onHistoryClick" `@mouseenter`="onOverlayMouseEnter('history', t('common.history'), $event)" `@mouseleave`="onOverlayMouseLeave('history')" > <Icon icon="lucide:history" class="w-4 h-4" /> </Button> <Button size="icon" + :aria-label="t('routes.settings')" class="window-no-drag-region shrink-0 w-10 bg-transparent shadow-none rounded-none hover:bg-card/80 text-xs font-medium text-foreground flex items-center justify-center transition-all duration-200 group" `@click`="openSettings" `@mouseenter`="onOverlayMouseEnter('settings', t('routes.settings'), $event)" `@mouseleave`="onOverlayMouseLeave('settings')" > <Icon icon="lucide:ellipsis" class="w-4 h-4" /> </Button> <Button v-if="!isMacOS" + :aria-label="t('common.minimize')" class="window-no-drag-region shrink-0 w-12 bg-transparent shadow-none rounded-none hover:bg-card/80 text-xs font-medium text-foreground flex items-center justify-center transition-all duration-200 group" `@click`="minimizeWindow" `@mouseenter`="onOverlayMouseEnter('minimize', t('common.minimize'), $event)" `@mouseleave`="onOverlayMouseLeave('minimize')" > <MinimizeIcon class="h-3! w-3!" /> </Button> <Button v-if="!isMacOS" + :aria-label="isMaximized ? t('common.restore') : t('common.maximize')" class="window-no-drag-region shrink-0 w-12 bg-transparent shadow-none rounded-none hover:bg-card/80 text-xs font-medium text-foreground flex items-center justify-center transition-all duration-200 group" `@click`="toggleMaximize" `@mouseenter`=" onOverlayMouseEnter( 'toggle-maximize', isMaximized ? t('common.restore') : t('common.maximize'), $event ) " `@mouseleave`="onOverlayMouseLeave('toggle-maximize')" > <MaximizeIcon v-if="!isMaximized" class="h-3! w-3!" /> <RestoreIcon v-else class="h-3! w-3!" /> </Button> <Button v-if="!isMacOS" + :aria-label="t('common.close')" class="window-no-drag-region shrink-0 w-12 bg-transparent shadow-none rounded-none hover:bg-red-700/80 hover:text-white text-xs font-medium text-foreground flex items-center justify-center transition-all duration-200 group" `@click`="closeWindow" `@mouseenter`="onOverlayMouseEnter('close-window', t('common.close'), $event)" `@mouseleave`="onOverlayMouseLeave('close-window')" > <CloseIcon class="h-3! w-3!" /> </Button>src/main/presenter/windowPresenter/index.ts (1)
1641-1644: Shared chat window state file still causes collisions.
This is the same issue flagged earlier: all chat windows write tochat-window-state.json, so multiple windows overwrite each other’s state. Please switch to a per-window filename.
🧹 Nitpick comments (3)
src/renderer/src/components/ChatAppBar.vue (3)
90-92: Use the standard shadcn alias for ButtonThe import should use the
@/shadcn/components/ui/alias to match the repo conventions.As per coding guidelines, please align shadcn imports with the configured alias.♻️ Proposed fix
-import { Button } from '@shadcn/components/ui/button' +import { Button } from '@/shadcn/components/ui/button'
109-129: Route tooltip IPC through a presenter/event busDirect
ipcRenderer.sendbypasses the renderer↔main abstraction. Consider a presenter (or EventBus) wrapper forshell-tooltip:show/hideso IPC stays centralized and testable. Based on learnings, please useusePresenterfor main-process communication.
222-233: Move window-drag styles to Tailwind utilitiesThe scoped CSS block should be replaced with Tailwind utility classes (e.g.,
[-webkit-app-region:drag]/[-webkit-app-region:no-drag]) to keep styling consistent with the project’s Tailwind-only rule. As per coding guidelines, please avoid scoped CSS for styling.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/renderer/settings/App.vue (3)
5-8: Fix the typo in the non‑macOS class name.Line 6 uses
ounded-t-none(missingr), so the class never applies.🩹 Proposed fix
- isMacOS ? '' : ' ounded-t-none', + isMacOS ? '' : 'rounded-t-none',
206-214: Preserve explicit0positions in route meta.Line 211 and Line 213 use
|| 999, which treats0as “missing” and reorders items unexpectedly. Use??for numeric defaults.🩹 Proposed fix
- position: (route.meta.position as number) || 999, + position: (route.meta.position as number) ?? 999, group: (route.meta.group as string) || 'system', - groupPosition: (route.meta.groupPosition as number) || 999, + groupPosition: (route.meta.groupPosition as number) ?? 999, groupTitleKey: (route.meta.groupTitleKey as string) || 'settings.groups.system'
240-247: Title can stay generic until navigation.
updateTitle()runs beforesettingsItemsis populated, and the watcher only reacts to route changes. Initial titles can stay as “Settings” until navigation. WatchsettingsItemstoo (or callupdateTitle()after the list is set).🩹 Proposed fix
-watch( - () => route.name, - () => { - updateTitle() - }, - { immediate: true } -) +watch( + [() => route.name, settingsItems], + () => { + updateTitle() + }, + { immediate: true } +)
🤖 Fix all issues with AI agents
In `@src/renderer/settings/App.vue`:
- Around line 337-346: onBeforeUnmount currently always calls
window.electron.ipcRenderer.removeListener(SETTINGS_EVENTS.NAVIGATE,
handleSettingsNavigate) which can throw when window.electron is missing; mirror
the registration guard by first checking that window.electron and
window.electron.ipcRenderer exist before calling removeListener for
SETTINGS_EVENTS.NAVIGATE and handleSettingsNavigate, and leave the existing
cleanupErrorNotifications and cleanupSearchEngineListeners nulling as-is.
🧹 Nitpick comments (7)
docs/specs/renderer-store-composables-rules/plan.md (1)
7-27: Define category taxonomy + dependency direction explicitly to reduce ambiguity.Right now “UI/App/Adapter/Store” and dependency direction are implied but not spelled out. A short definition block will prevent inconsistent classification during the “现状盘点” phase.
♻️ Suggested addition
## 计划步骤 +### 定义与依赖方向（建议补充） +- UI: 仅视图与交互；禁止直接调用 Store/Adapter 内部实现。 +- App Composable: 编排流程与跨域协作；可调用 Store 与 Adapter。 +- Adapter Composable: 进程/IPC/外部系统边界；仅暴露最小 API 给 App。 +- Store: 状态与异步动作；禁止直接操作 UI 或 IPC。 +- 依赖方向：UI -> App -> Adapter/Store（单向）docs/specs/renderer-store-composables-rules/audit.md (3)
7-8: Consider adding English translation for consistency.The objectives section is only in Chinese, while the rest of the document mixes both languages. For better accessibility across the team, consider adding an English version of the objectives.
📝 Suggested addition
## 目标 +## Objectives 通过"重复性与必要性审计"确定权威入口，合并或移除重复职责,避免并行执行链。 +Identify authoritative entry points through "redundancy and necessity audit", merge or remove duplicate responsibilities, and avoid parallel execution chains.
17-19: Enhance next batch items with actionable details.The pending items lack specific details like file paths, assignees, priority, or links to tracking issues. Adding these would make the audit more actionable and easier to track.
📋 Suggested enhancements
Consider adding:
- File paths for each composable mentioned (e.g.,
src/renderer/src/composables/useCleanDialog.ts)- Priority levels (P0/P1/P2) for next batch items
- Assignees or team ownership
- Links to related GitHub issues for tracking
- Estimated effort or timeline
Example format:
### 仍需处理（下一批次） - **P1** `useCleanDialog` (`src/renderer/src/composables/useCleanDialog.ts`): UI 与流程耦合，建议拆为 UI Composable + App Composable。 - Assignee: [Name/Team] - Related issue: #[issue-number] - **P2** `useSearchResultState`: 混合 UI 状态与流程编排，建议拆分并明确权威入口。 - Assignee: [Name/Team] - Related issue: #[issue-number]
21-23: Consider expanding the authoritative entry points snapshot.While the current snapshot provides a useful quick reference, it could be more helpful with additional context such as file paths, usage guidelines, and examples.
📚 Suggested enhancement
## 权威入口清单（快照） -- Chat retry: `chatStore.retryFromUserMessage` -- Notifications: `useNotificationAdapter` / `useNotificationToasts` + +### Chat retry +- **Entry point**: `chatStore.retryFromUserMessage` +- **Location**: `src/renderer/src/stores/chatStore.ts` +- **Usage**: Call this method to retry a chat message from a user message node + +### Notifications +- **System notifications**: `useNotificationAdapter.showSystemNotification` + - **Location**: `src/renderer/src/composables/useNotificationAdapter.ts` + - **Usage**: Display OS-level system notifications +- **UI Toasts**: `useNotificationToasts.showErrorToast` + - **Location**: `src/renderer/src/composables/useNotificationToasts.ts` + - **Usage**: Display in-app error toast messagesscripts/check-renderer-adapters.mjs (1)
41-44: Consider handling inline comments.The current regex
^\s*\/\/.*$only strips full-line comments but misses inline comments (e.g.,const x = 1 // usePresenter deprecated). This could cause false positives if boundary patterns appear in trailing comments.♻️ Optional enhancement to strip inline comments
const stripComments = (source) => { const withoutBlock = source.replace(/\/\*[\s\S]*?\*\//g, '') - return withoutBlock.replace(/^\s*\/\/.*$/gm, '') + // Remove full-line comments and inline comments (outside strings - simplified approach) + return withoutBlock.replace(/\/\/.*$/gm, '') }Note: A fully accurate approach would need to handle strings containing
//, but the simplified version above covers most cases.src/renderer/settings/components/skills/SyncStatusSection.vue (1)
95-100: Remove the unusedtypefield from theshowErrorToastcall.The
typefield is defined as a required property in theErrorNotificationtype, but it's never actually used in the toast implementation—error styling is determined solely byvariant: 'destructive'. This suggests either removing the field from the type definition or making it optional if it's not needed.src/renderer/settings/App.vue (1)
218-223: Move sorting/assignment outside the loop.Sorting and assigning on every iteration causes repeated sorts, reactive updates, and “Final sorted” logs. Move them after the loop.
♻️ Suggested refactor
routes.forEach((route) => { // In settings window, all routes are top-level, no parent 'settings' route if (route.path !== '/' && route.meta?.titleKey) { console.log(`Adding settings route: ${route.path} with titleKey: ${route.meta.titleKey}`) tempArray.push({ title: route.meta.titleKey as string, icon: route.meta.icon as string, path: route.path, name: route.name as string, position: (route.meta.position as number) || 999, group: (route.meta.group as string) || 'system', groupPosition: (route.meta.groupPosition as number) || 999, groupTitleKey: (route.meta.groupTitleKey as string) || 'settings.groups.system' }) } - // Sort by position meta field, default to 999 if not present - tempArray.sort((a, b) => { - return a.position - b.position - }) - settingsItems.value = tempArray - console.log('Final sorted settings routes:', settingsItems.value) }) + // Sort by position meta field, default to 999 if not present + tempArray.sort((a, b) => { + return a.position - b.position + }) + settingsItems.value = tempArray + console.log('Final sorted settings routes:', settingsItems.value)
| onBeforeUnmount(() => { | ||
| if (errorDisplayTimer.value) { | ||
| clearTimeout(errorDisplayTimer.value) | ||
| errorDisplayTimer.value = null | ||
| if (cleanupErrorNotifications) { | ||
| cleanupErrorNotifications() | ||
| cleanupErrorNotifications = null | ||
| } | ||
| if (cleanupSearchEngineListeners) { | ||
| cleanupSearchEngineListeners() | ||
| cleanupSearchEngineListeners = null | ||
| } | ||
|
|
||
| window.electron.ipcRenderer.removeAllListeners(NOTIFICATION_EVENTS.SHOW_ERROR) | ||
| window.electron.ipcRenderer.removeListener(SETTINGS_EVENTS.NAVIGATE, handleSettingsNavigate) |
There was a problem hiding this comment.
Guard IPC removal in non‑electron/test contexts.
You guard registration on Line 140; removal should match to avoid runtime errors when window.electron is absent.
🩹 Proposed fix
- window.electron.ipcRenderer.removeListener(SETTINGS_EVENTS.NAVIGATE, handleSettingsNavigate)
+ if (window?.electron?.ipcRenderer) {
+ window.electron.ipcRenderer.removeListener(SETTINGS_EVENTS.NAVIGATE, handleSettingsNavigate)
+ }📝 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.
| onBeforeUnmount(() => { | |
| if (errorDisplayTimer.value) { | |
| clearTimeout(errorDisplayTimer.value) | |
| errorDisplayTimer.value = null | |
| if (cleanupErrorNotifications) { | |
| cleanupErrorNotifications() | |
| cleanupErrorNotifications = null | |
| } | |
| if (cleanupSearchEngineListeners) { | |
| cleanupSearchEngineListeners() | |
| cleanupSearchEngineListeners = null | |
| } | |
| window.electron.ipcRenderer.removeAllListeners(NOTIFICATION_EVENTS.SHOW_ERROR) | |
| window.electron.ipcRenderer.removeListener(SETTINGS_EVENTS.NAVIGATE, handleSettingsNavigate) | |
| onBeforeUnmount(() => { | |
| if (cleanupErrorNotifications) { | |
| cleanupErrorNotifications() | |
| cleanupErrorNotifications = null | |
| } | |
| if (cleanupSearchEngineListeners) { | |
| cleanupSearchEngineListeners() | |
| cleanupSearchEngineListeners = null | |
| } | |
| if (window?.electron?.ipcRenderer) { | |
| window.electron.ipcRenderer.removeListener(SETTINGS_EVENTS.NAVIGATE, handleSettingsNavigate) | |
| } |
🤖 Prompt for AI Agents
In `@src/renderer/settings/App.vue` around lines 337 - 346, onBeforeUnmount
currently always calls
window.electron.ipcRenderer.removeListener(SETTINGS_EVENTS.NAVIGATE,
handleSettingsNavigate) which can throw when window.electron is missing; mirror
the registration guard by first checking that window.electron and
window.electron.ipcRenderer exist before calling removeListener for
SETTINGS_EVENTS.NAVIGATE and handleSettingsNavigate, and leave the existing
cleanupErrorNotifications and cleanupSearchEngineListeners nulling as-is.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/main/presenter/agentPresenter/index.ts`:
- Around line 365-372: The promise chain using
this.sessionPresenter.generateTitle(state.conversationId).then(...).then(...)
can produce unhandled rejections; modify the code that calls generateTitle and
renameConversation (on sessionPresenter) to handle failures by either awaiting
the chain inside the surrounding async function or appending a .catch(...) to
the chain (and optionally prefixing with void if keeping it fire-and-forget).
Ensure the .catch logs or forwards the error (e.g., via processLogger or
console.error) and references generateTitle and renameConversation so any
rejection is handled and doesn't bypass the outer try/catch.
🧹 Nitpick comments (2)
src/main/presenter/agentPresenter/message/messageBuilder.ts (1)
99-109: Consider removing or adjusting the debug log.The
console.log('store chat mode', storedChatMode)on line 104 appears to be debug output. Consider removing it or using a conditional log gated by a debug flag to avoid polluting production logs.The chat mode normalization logic (
'chat'→'agent') and the fallback chain are correctly implemented.🔧 Suggested change
const storedChatMode = presenter.configPresenter.getSetting('input_chatMode') as | 'chat' | 'agent' | 'acp agent' | undefined - console.log('store chat mode', storedChatMode) const normalizedChatMode = storedChatMode === 'chat' ? 'agent' : storedChatModesrc/main/presenter/index.ts (1)
411-423: Consider extracting the fallback logic to a helper.The
BrowserWindowfallback pattern for resolvingwindowIdis duplicated here and in the success path (lines 361-368). While not critical, extracting this to a small helper function would improve maintainability.♻️ Optional refactor to reduce duplication
+// Helper to resolve windowId from WebContents +function resolveWindowId(tabPresenter: ITabPresenter, webContentsId: number, sender: Electron.WebContents): number | undefined { + let windowId = tabPresenter.getWindowIdByWebContentsId(webContentsId) + if (windowId === undefined) { + const browserWindow = BrowserWindow.fromWebContents(sender) + if (browserWindow && !browserWindow.isDestroyed()) { + windowId = browserWindow.id + } + } + return windowId +} // Then use in both places: - let windowId = presenter.tabPresenter.getWindowIdByWebContentsId(webContentsId) - if (windowId === undefined) { - const browserWindow = BrowserWindow.fromWebContents(event.sender) - if (browserWindow && !browserWindow.isDestroyed()) { - windowId = browserWindow.id - } - } + const windowId = resolveWindowId(presenter.tabPresenter, webContentsId, event.sender)
| this.sessionPresenter | ||
| .generateTitle(state.conversationId) | ||
| .then((title) => { | ||
| return this.sessionPresenter.renameConversation(state.conversationId, title) | ||
| }) | ||
| .then(() => { | ||
| console.log('title updated') | ||
| }) |
There was a problem hiding this comment.
Handle async title-generation failures to avoid unhandled rejections.
The promise chain isn’t awaited and lacks .catch, so errors bypass the try/catch. Add a rejection handler (and optionally void the chain) to prevent unhandled rejections.
🐛 Proposed fix
- try {
- this.sessionPresenter
- .generateTitle(state.conversationId)
- .then((title) => {
- return this.sessionPresenter.renameConversation(state.conversationId, title)
- })
- .then(() => {
- console.log('title updated')
- })
- } catch (error) {
- console.error('[AgentPresenter] Failed to summarize title', {
- conversationId: state.conversationId,
- err: error
- })
- }
+ void this.sessionPresenter
+ .generateTitle(state.conversationId)
+ .then((title) => {
+ return this.sessionPresenter.renameConversation(state.conversationId, title)
+ })
+ .then(() => {
+ console.log('[AgentPresenter] Title updated')
+ })
+ .catch((error) => {
+ console.error('[AgentPresenter] Failed to summarize title', {
+ conversationId: state.conversationId,
+ err: 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.
| this.sessionPresenter | |
| .generateTitle(state.conversationId) | |
| .then((title) => { | |
| return this.sessionPresenter.renameConversation(state.conversationId, title) | |
| }) | |
| .then(() => { | |
| console.log('title updated') | |
| }) | |
| void this.sessionPresenter | |
| .generateTitle(state.conversationId) | |
| .then((title) => { | |
| return this.sessionPresenter.renameConversation(state.conversationId, title) | |
| }) | |
| .then(() => { | |
| console.log('[AgentPresenter] Title updated') | |
| }) | |
| .catch((error) => { | |
| console.error('[AgentPresenter] Failed to summarize title', { | |
| conversationId: state.conversationId, | |
| err: error | |
| }) | |
| }) |
🤖 Prompt for AI Agents
In `@src/main/presenter/agentPresenter/index.ts` around lines 365 - 372, The
promise chain using
this.sessionPresenter.generateTitle(state.conversationId).then(...).then(...)
can produce unhandled rejections; modify the code that calls generateTitle and
renameConversation (on sessionPresenter) to handle failures by either awaiting
the chain inside the surrounding async function or appending a .catch(...) to
the chain (and optionally prefixing with void if keeping it fire-and-forget).
Ensure the .catch logs or forwards the error (e.g., via processLogger or
console.error) and references generateTitle and renameConversation so any
rejection is handled and doesn't bypass the outer try/catch.
- Redesigned splash window to a compact 320x80px transparent window with logo and progress bar - Deferred splash creation by 1s to avoid unnecessary splash on fast startups - Added progress state buffering to prevent missing updates when renderer not ready - Removed circular loader animation in favor of clean linear progress bar - Improved window configuration: transparent, frameless, unfocusable, no shadow - Added proper preload support for splash window - Removed artificial delays for faster, more responsive splash UX
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@docs/specs/splash-window-ux/plan.md`:
- Around line 31-33: Add automated tests for the timing-sensitive splash logic:
create unit tests for SplashWindowManager that use fake timers to assert delayed
creation behavior (verify no BrowserWindow created before the 1s threshold and
created after), test progress caching by sending progress events before window
creation and asserting cached values are applied when createSplashWindow runs,
and test dismissal by simulating the app window ready/loaded events to ensure
splash removal; add integration-style tests that mock Electron BrowserWindow/IPC
to simulate fast vs slow startup scenarios and verify IPC event flow (progress
updates and close messages) between SplashWindowManager and the splash window.
Ensure tests reference SplashWindowManager, createSplashWindow,
scheduleDelayedSplash, handleProgressUpdate, and dismissSplash (or the
equivalent method names) and use Electron mocks and timer stubs to
deterministically reproduce timing edge cases.
In `@docs/specs/splash-window-ux/spec.md`:
- Around line 25-29: Update the "Acceptance Criteria" section so each item is
specific and testable: replace "compact (narrow bar)" with explicit dimensions
(e.g., height 48px, width 360px, or allowable range), define status update
behavior for "status text" (e.g., updates no faster than 250ms and on each
lifecycle progress event), specify visual details (background color hex, padding
in px, alignment/positioning like centered top-right with 16px margin), and add
edge-case criteria covering: failure to create splash (fallback behavior and
error logging), main window closing while splash is shown (expected transition
or auto-close behavior), and validation of the "1s" threshold (how to measure
startup completion — use monotonic timer and tolerance ±50ms and automated test
hook). Ensure you reference the existing acceptance items (e.g., "If startup
completes..." and "If no window exists after 1s...") when replacing them so the
new criteria directly map to the original bullets.
🧹 Nitpick comments (2)
docs/skills/deepchat-sdd-skills/SKILL.md (2)
14-16: Reduce repetitive sentence starts in Usage bullets.Optional rewording to improve readability.
✏️ Proposed wording tweak
- - You need a reliable `spec.md` / `plan.md` / `tasks.md` set under `docs/specs/<feature>/`. - - You are about to implement a non-trivial change and want spec → plan → code traceability. - - You need to clarify scope, risks, IPC surface, and test strategy before coding. + - Create a reliable `spec.md` / `plan.md` / `tasks.md` set under `docs/specs/<feature>/`. + - Prepare for non-trivial changes that need spec → plan → code traceability. + - Clarify scope, risks, IPC surface, and test strategy before coding.
205-210: Consider adding PR checklist items to DoD.If this template is the standard guardrail, include the repo’s PR expectations (issue linking, screenshots for UI changes) so they’re not missed.
✅ Proposed DoD addition
- Acceptance criteria satisfied (ideally with tests) - No unresolved `[NEEDS CLARIFICATION]` - i18n applied to user-facing strings - Security/privacy constraints met (no secrets leaked; IPC surface minimal) +- PR description includes `Closes #...` when applicable +- UI changes include BEFORE/AFTER screenshots or ASCII layout blocks - `pnpm run format && pnpm run lint && pnpm run typecheck` pass locally - Tests for critical paths pass (`pnpm test` or targeted suites)Based on learnings, ...
| ## Test strategy (pragmatic) | ||
| - Manual: verify fast start shows no splash; slow start shows compact splash with updating text. | ||
| - Optional later: main-process unit test via Electron mocks for delayed splash scheduling. |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Strengthen the test strategy beyond manual-only testing.
The test strategy relies solely on manual verification with unit tests marked as "optional later." Given the timing-sensitive logic (1s delay threshold, window existence checks, progress caching), automated tests should be part of the implementation to prevent regressions.
Consider adding:
- Unit tests for SplashWindowManager's delayed creation logic and progress caching
- Integration tests to verify splash creation conditions (fast vs. slow startup scenarios)
- Tests for IPC event flow and splash dismissal
🤖 Prompt for AI Agents
In `@docs/specs/splash-window-ux/plan.md` around lines 31 - 33, Add automated
tests for the timing-sensitive splash logic: create unit tests for
SplashWindowManager that use fake timers to assert delayed creation behavior
(verify no BrowserWindow created before the 1s threshold and created after),
test progress caching by sending progress events before window creation and
asserting cached values are applied when createSplashWindow runs, and test
dismissal by simulating the app window ready/loaded events to ensure splash
removal; add integration-style tests that mock Electron BrowserWindow/IPC to
simulate fast vs slow startup scenarios and verify IPC event flow (progress
updates and close messages) between SplashWindowManager and the splash window.
Ensure tests reference SplashWindowManager, createSplashWindow,
scheduleDelayedSplash, handleProgressUpdate, and dismissSplash (or the
equivalent method names) and use Electron mocks and timer stubs to
deterministically reproduce timing edge cases.
| ## Acceptance Criteria | ||
| - [ ] If startup completes and at least one non-splash window exists within 1s, splash is never created. | ||
| - [ ] If no window exists after 1s and startup is still running, splash is created and shown. | ||
| - [ ] Splash displays DeepChat icon and a status text that updates as lifecycle progress updates arrive. | ||
| - [ ] Splash window is compact (narrow bar) and avoids white/blank flashes. |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Make acceptance criteria more specific and measurable.
The acceptance criteria include subjective terms like "compact (narrow bar)" without concrete dimensions or measurements. To ensure consistent implementation and testability, consider specifying:
- Exact window dimensions or dimension ranges for the "narrow bar"
- Expected update frequency or conditions for status text updates
- Visual appearance details (e.g., background color, padding, positioning)
Additionally, the criteria don't cover edge cases such as:
- Behavior when splash creation fails
- Handling if the main window closes while splash is displayed
- Validation mechanism for the 1s timing threshold
🤖 Prompt for AI Agents
In `@docs/specs/splash-window-ux/spec.md` around lines 25 - 29, Update the
"Acceptance Criteria" section so each item is specific and testable: replace
"compact (narrow bar)" with explicit dimensions (e.g., height 48px, width 360px,
or allowable range), define status update behavior for "status text" (e.g.,
updates no faster than 250ms and on each lifecycle progress event), specify
visual details (background color hex, padding in px, alignment/positioning like
centered top-right with 16px margin), and add edge-case criteria covering:
failure to create splash (fallback behavior and error logging), main window
closing while splash is shown (expected transition or auto-close behavior), and
validation of the "1s" threshold (how to measure startup completion — use
monotonic timer and tolerance ±50ms and automated test hook). Ensure you
reference the existing acceptance items (e.g., "If startup completes..." and "If
no window exists after 1s...") when replacing them so the new criteria directly
map to the original bullets.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/main/presenter/agentPresenter/message/messageBuilder.ts`:
- Line 131: Remove the stray debug console.log from messageBuilder.ts: delete
the line console.log('store chat mode', storedChatMode) and, if diagnostic
logging is required, replace it with a structured logger call at debug level
(e.g., processLogger.debug or the existing logger.debug) that includes the
context and the storedChatMode variable; locate this in the MessageBuilder code
where storedChatMode is set/used and use the project’s logger instead of
console.log.
🧹 Nitpick comments (1)
docs/specs/model-selector-config-popover/tasks.md (1)
9-9: Specify the exact file path instead of using wildcards.The wildcard pattern
src/shared/**is too vague for implementers. Please specify the exact file whereCONVERSATION_SETTINGSis defined (e.g.,src/shared/types/conversation.tsor similar).
| | 'agent' | ||
| | 'acp agent' | ||
| | undefined | ||
| console.log('store chat mode', storedChatMode) |
There was a problem hiding this comment.
Remove debug console.log statement.
This appears to be a debug log that should not be present in production code. If logging is needed for diagnostics, consider using structured logging at an appropriate level.
Suggested fix
- console.log('store chat mode', storedChatMode)📝 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.
| console.log('store chat mode', storedChatMode) |
🤖 Prompt for AI Agents
In `@src/main/presenter/agentPresenter/message/messageBuilder.ts` at line 131,
Remove the stray debug console.log from messageBuilder.ts: delete the line
console.log('store chat mode', storedChatMode) and, if diagnostic logging is
required, replace it with a structured logger call at debug level (e.g.,
processLogger.debug or the existing logger.debug) that includes the context and
the storedChatMode variable; locate this in the MessageBuilder code where
storedChatMode is set/used and use the project’s logger instead of console.log.
There was a problem hiding this comment.
Actionable comments posted: 19
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🤖 Fix all issues with AI agents
In `@docs/acp-agent-architecture.md`:
- Around line 761-778: Update the "兼容性考虑" section to provide concrete migration
and user-communication steps: under "会话持久化" (mentioning loadSession) add a
requirement to implement a one-time Migration Assistant UI that guides users
through session reload on first launch and offer a one-time data export tool;
under "历史数据清理" (referencing acp_sessions) replace the dismissive sentence with:
"ACP session metadata will be removed to align with the architecture where
Agents manage persistence" and add an explicit pre-cleanup data export option;
under "UI 变化" add specific copy and UX items (first-launch onboarding tooltip,
in-app changelog entry, and contextual banners) that instruct users how to
recreate/load sessions and where to find migration tools.
- Around line 728-760: Add testing, rollback, feature-flag, success-metrics and
timeline details to each migration phase: for Phase 1 (AcpPresenter creation)
list prerequisites, unit/integration tests for AcpPresenter and migration steps,
a rollback plan to revert to AgentPresenter usage and config flag details (name,
default=false, rollout steps), success criteria and estimated duration; for
Phase 2 (AcpInputFormatter, message flow, STREAM_EVENTS→ACP_EVENTS) add
automated tests for AcpInputFormatter and IPC serialization checks, staged
feature-flag switch procedure, rollback steps to re-enable AgentPresenter calls,
acceptance tests and duration; for Phase 3 (remove acp_sessions,
AcpSessionPersistence, shift to in-memory, conversations cleanup) define
migration tests, data backup/restore rollback plan, validation scripts, success
metrics and timeline; for Phase 4 (delete AcpProvider, update
LLMProviderPresenter and UI) include integration and UAT plans, feature-flag
finalization and cutover checklist, explicit deliverables and estimated
durations—reference symbols: AcpPresenter, AcpInputFormatter, AgentPresenter,
STREAM_EVENTS, ACP_EVENTS, acp_sessions, AcpSessionPersistence, AcpProvider,
LLMProviderPresenter, conversations.
In `@docs/architecture/acp-commands-specification.md`:
- Around line 468-486: The parent .command-item div currently has
`@click`="emit('command-click', cmd)" while its child "Run" button is also
clickable, causing nested click events; to fix, remove the parent `@click` from
the .command-item element and move the emit('command-click', cmd) handler to the
button (add `@click`="emit('command-click', cmd)" on the "Run" button element),
ensuring only the Run button triggers the action and avoiding duplicate events
from command-item; update any references to keyboard/aria handlers as needed for
accessibility on the button.
In `@docs/architecture/acp-modes-models-specification.md`:
- Around line 416-421: The spec references getEmitter(sessionId) from setMode
and other methods but doesn't define it; add a brief note or implementation for
getEmitter (e.g., a private helper returning AgenticEventEmitter | undefined) or
explicitly state in the example that getEmitter is provided by a base
class/utility with a reference to its real definition; update the class example
to either include a small stubbed method named getEmitter(sessionId: string):
AgenticEventEmitter | undefined or add a comment above setMode/other calls
indicating where getEmitter is implemented so readers can locate it.
- Around line 669-696: Add the missing UnifiedModelSelector.vue implementation:
create a Vue SFC that declares props availableModels, currentModelId, and
disabled, emits an event (e.g., "update:currentModelId" or "model-selected")
when a model is picked, and exposes a computed selectedModel based on
currentModelId; implement a template rendering the header "Model:
{selectedModel.name} ▼ (N available)" and a dropdown list that iterates over
availableModels showing name, optional description, and a checkmark for the
currently selected model; ensure clicking an item sets/emit the new model id and
respects disabled state, and include minimal script setup (props typing, emits,
methods/handlers) and accessible markup for the dropdown.
- Around line 267-284: The normalizeSessionModesUpdate function emits
sessionUpdated without the required sessionId; update
normalizeSessionModesUpdate to read data.conversationId (per the comment that
conversationId maps to sessionId), map it to sessionId and include it in the
payload passed to emitter.sessionUpdated({ availableModes, currentModeId,
sessionId }), ensuring the emitted event matches what handleSessionUpdated
expects.
- Around line 315-331: normalizeSessionModelsUpdate currently emits
availableModels and currentModelId but omits the sessionId; update the function
to read the sessionId from the incoming data (e.g., const sessionId =
data.sessionId as string) and include sessionId in the emitter.sessionUpdated
payload so the event includes { sessionId, availableModels: available,
currentModelId: current }; adjust any typing if needed to reflect sessionId
presence.
- Around line 382-414: The getSession method in AgentPresenter reads
currentModeId from the sessionModes map which can be undefined for new sessions;
update getSession (AgentPresenter.getSession) to return a default mode when
sessionModes.get(sessionId) is falsy (e.g., use a sensible default like
'balanced' or the first entry of the availableModes array) so currentModeId is
never undefined; implement the fallback when constructing the returned object
(currentModeId: this.sessionModes.get(sessionId) ?? 'balanced' or ??
availableModes[0].id) and ensure sessionModes is populated elsewhere when a mode
is explicitly changed.
- Around line 699-733: Update the implementation checklist to accurately reflect
current state and prioritize missing items: mark SessionInfo in
agentic.presenter.d.ts and existing
useAgenticSession/useModelSelection/UnifiedModeSelector.vue/UnifiedModelSelector.vue
as complete, add a task to export ModeInfo (currently only a local interface in
useAcpMode.ts) alongside ModelInfo, add the missing useModeSelection composable
to Phase 3, mark useAcpMode.ts and useAcpSessionModel.ts for deprecation (add
deprecation notices), and prioritize implementing the Phase 2 normalizer
functions normalizeSessionModesUpdate() and normalizeSessionModelsUpdate() in
acpPresenter/normalizer.ts.
In `@docs/architecture/mcp/refactor.md`:
- Around line 595-688: Add migration safety: implement the
ToolParameterPreprocessor interface (e.g., GlobSearchPreprocessor) and integrate
it into ToolManager.callTool(), then add comprehensive tests (unit tests for
each preprocessor and integration tests for ToolManager.callTool() covering
preprocessing+validation as suggested); add a feature flag in
useMcpStoreService.callTool() to toggle between old parameter conversion and new
passthrough and implement a dual-write/dual-validate mode to run both paths in
parallel for a staged rollout (enable per-tool), and create a test matrix of all
tools×parameter combos plus a documented rollback procedure and runtime
monitoring hooks to detect and revert regressions.
In `@docs/architecture/renderer-integration-design.md`:
- Around line 233-244: Update the design doc to add a "Mapping Layer
Specification" subsection that specifies exactly where conversationId↔sessionId
conversion occurs (clarify whether conversion lives in AgenticPresenter,
per-agent presenters, or a shared utility and state the chosen single source of
truth), define function interfaces such as toSessionId(conversationId) and
toConversationId(sessionId) and where they are invoked in the call chain
(presenter input/output boundaries), document query implications (DB APIs use
conversationId and any incoming sessionId must be reverse-mapped before DB
calls), note performance considerations and a recommended caching strategy for
high-frequency operations (e.g., in-memory LRU cache in presenter/shared util)
and error handling for invalid IDs (explicit errors and fallbacks), and
reference AgenticPresenter, individual agent presenters, and the presenter layer
as the loci of mapping to help implementers locate the code.
In `@docs/architecture/unified-components-specification.md`:
- Around line 198-203: The AgentHeader contains branching on agentId (if
(agentId.startsWith('acp.'))) which violates the "No Branching" design
principle; update the SessionInfo type/contract to include precomputed display
metadata (e.g., displayName and agentType or similar) produced by the presenter
layer, then remove the parsing/branching from AgentHeader so it simply reads
SessionInfo.displayName and SessionInfo.agentType; ensure any code that
currently constructs agentId-based labels is moved into the presenter that
builds SessionInfo and that AgentHeader and related components only consume the
new display metadata.
In `@docs/specs/agentic-renderer-integration/chatConfig-removal-spec.md`:
- Around line 82-96: Do not drop the conversations.settings column or remove the
CONVERSATION_SETTINGS type directly; instead implement a phased, reversible
migration: 1) Deprecate by renaming conversations.settings to
settings_deprecated and add a comment to mark it deprecated (so code referencing
conversations.settings can be switched safely). 2) Optionally preserve data by
creating a conversation_settings_archive table and copying non-null
settings_deprecated rows into it for rollback/inspection. 3) After a validation
period, remove settings_deprecated and delete the CONVERSATION_SETTINGS type
(thread.presenter.d.ts) once all references are migrated; ensure migration
scripts reference conversations.settings_deprecated,
conversation_settings_archive, and the CONVERSATION_SETTINGS type names so the
process is reversible.
- Around line 398-404: Update the rollback plan to include concrete, executable
steps: list the exact Git revert command pattern (git revert
<migration-commit-sha>) and sequence for immediate rollback, SQL to re-add the
settings column (ALTER TABLE conversations ADD COLUMN settings JSONB) and to
restore from conversation_settings_archive, instructions to redeploy the
previous release and monitor metrics; add a partial-rollback section describing
toggling the feature flag USE_SESSION_CONFIG = false to revert to chatConfig and
which components to isolate/leave running; include a clear timeline for
deprecated mode (weeks 1–4, months 2–4) and measurable success criteria (error
rate threshold, no P0/P1 bugs, user feedback) so the document references
conversation_settings_archive, USE_SESSION_CONFIG, chatConfig, and the settings
column for easy location.
- Around line 3-8: Add a new "User Impact & Migration" section to the spec that
documents which chatConfig/CONVERSATION_SETTINGS features are user-facing and
usage signals, and define a concrete migration plan for the existing settings
column: describe exporting current settings, a transformation rule to map legacy
LLM/tool/prompt fields to any new global/session defaults or archival format,
and steps to drop the column; include a deprecation timeline (phased opt-out,
migration window, and final removal) and a user communication plan (release
notes, migration guide, and in-app notices). Reference the removed symbol names
chatConfig, CONVERSATION_SETTINGS and the database settings column when
describing the migration steps and deprecation milestones so implementers can
locate and update code and schema accordingly.
- Around line 28-60: Revert the blanket removal in the spec: restore a minimal
user-config surface by keeping at least `temperature` and `maxTokens` (or
replace them with named presets like "Creative", "Balanced", "Precise") instead
of deleting all LLM parameters, and change the MCP tool policy from "all
enabled" to "default-on with per-session disable" (or allow a simple
`enabledMcpTools`/`disabledMcpTools` list) so users can opt out of sensitive
tools; update the sections that currently remove `temperature`, `maxTokens`,
`enabledMcpTools`, `artifacts`, and `enableSearch` to reflect these minimal
configurable options and presets.
- Around line 282-328: The migration plan lacks explicit execution order,
validation gates, and rollback controls—add a pre-migration DB backup step and a
sequenced plan that runs: 1) DB backup, 2) update useSessionConfig, 3) deploy
backend changes ensuring SessionInfo is available to SessionPresenter, 4) update
high-priority components (ModelSelect.vue, ChatInput.vue, McpToolsList.vue) with
per-component validation, 5) replace consumers (references to
chatStore.chatConfig → useSessionConfig(sessionId)), 6) run the ALTER TABLE
conversations DROP COLUMN settings only after successful validation, and 7)
cleanup; introduce feature flags (e.g., ENABLE_SESSION_CONFIG,
DROP_SETTINGS_COLUMN) to toggle the new behavior, add validation gates after
each step (no console errors, session state loads, model/mode selection works,
regression tests pass), and ensure rollback plans for each step before removing
usePromptInputConfig.ts and related types (CONVERSATION_SETTINGS) or updating
SessionPresenter interfaces.
- Around line 1-404: The spec aggressively removes chatConfig and drops the
conversations.settings column but lacks safety, rollback, and staged rollout
details; implement a non-destructive migration and phased rollout: (1) create a
migration that copies settings to a backup column/table before ALTER TABLE
(e.g., add conversations.settings_backup and populate it) and provide a
reversible SQL script for rollback; (2) gate removals behind a feature flag /
rollout config so components like useAgenticSessionStore, ModelSelect.vue, and
useSessionConfig can read from either chatConfig or SessionInfo during
transition; (3) add automated unit/integration tests and a performance test to
validate message send/retry and exports without chatConfig and update the
Testing Checklist; and (4) update docs with explicit rollback steps and a
migration verification step that checks existing conversations load when
settings is absent.
In `@docs/specs/agentic-renderer-integration/spec.md`:
- Around line 46-52: The "Non-Goals" and "Migration Strategy" are inconsistent:
update the "Non-Goals" and the corresponding lines in "Migration Strategy" so
they explicitly reconcile the two (e.g., change the Non-Goals bullet "NO feature
flags or gradual migration (atomic change)" to clarify whether that means "no
public incremental shipping" or remove it and instead state "no
backward-compatible public migration; internal phased rollout allowed for
testing." Reference the "Non-Goals" section and the "Migration Strategy" section
and make a single clear sentence that either (A) prohibits incremental public
shipping and notes that any phased steps are internal testing prior to one
atomic ship, or (B) permits a phased public rollout and remove the "NO gradual
migration" line—pick one policy and make the language explicit so the two
sections no longer conflict.
🟡 Minor comments (14)
docs/architecture/workspace-integration-analysis.md-3-5 (1)
3-5: Hyphenate the compound adjective.Consider “HIGH-PRIORITY research” for standard compound-adjective usage.
docs/architecture/agent-abstraction-layer.md-58-80 (1)
58-80: Add a language identifier to this fenced code block.
This block is missing a language tag, which breaks markdownlint MD040.docs/architecture/agent-abstraction-layer.md-220-264 (1)
220-264: Specify a language for the architecture diagram code block.
Add a language (e.g.,text) to satisfy MD040 and improve editor rendering.docs/architecture/event-payload-specification.md-459-477 (1)
459-477: Fix variable-name mismatch in normalizer snippet.The example defines
payloadbut usesdatainside the switch, which is confusing and error-prone for implementers. Please align the variable name throughout the snippet.📝 Suggested doc fix
-export function normalizeAndEmit( - acpEvent: keyof typeof ACP_EVENTS, - payload: unknown, - sessionId: string, - emitter: AgenticEventEmitter -): void { - switch (acpEvent) { - case ACP_EVENTS.SESSION_UPDATE: - normalizeSessionUpdateEventAndEmit(data, emitter) +export function normalizeAndEmit( + acpEvent: keyof typeof ACP_EVENTS, + payload: unknown, + sessionId: string, + emitter: AgenticEventEmitter +): void { + switch (acpEvent) { + case ACP_EVENTS.SESSION_UPDATE: + normalizeSessionUpdateEventAndEmit(payload, emitter) break case ACP_EVENTS.PROMPT_COMPLETED: emitter.messageEnd(sessionId) break case ACP_EVENTS.ERROR: - emitter.statusChanged('error', new Error(data.error as string)) + emitter.statusChanged('error', new Error((payload as any).error as string)) breakdocs/architecture/session-lifecycle-specification.md-19-62 (1)
19-62: Add language tags to fenced code blocks.Markdownlint flags these fences; adding a language (e.g.,
text) improves readability and keeps lint clean.📝 Proposed fix
-``` +```text ┌─────────────────────────────────────────────────────────────────────────┐ │ DeepChat Session Lifecycle │ └─────────────────────────────────────────────────────────────────────────┘ ... -``` +``` -``` +```text ┌─────────────────────────────────────────────────────────────────────────┐ │ ACP Session Lifecycle │ └─────────────────────────────────────────────────────────────────────────┘ ... -``` +``` -``` +```text ┌─────────────────────────────────────────────────────────────────────────┐ │ Unified Session Creation │ └─────────────────────────────────────────────────────────────────────────┘ ... -``` +``` -``` +```text ┌─────────────────────────────────────────────────────────────────────────┐ │ Unified Session Loading │ └─────────────────────────────────────────────────────────────────────────┘ ... -``` +``` -``` +```text ┌─────────────────────────────────────────────────────────────────────────┐ │ Unified Session Closing │ └─────────────────────────────────────────────────────────────────────────┘ ... -``` +```Also applies to: 68-118, 160-208, 212-245, 249-288
docs/architecture/session-lifecycle-specification.md-318-323 (1)
318-323: Clarify the origin oftabIdin the sample.The snippet references
tabIdwithout declaring or explaining it; a short comment about deriving it from IPC context would prevent confusion.📝 Suggested tweak
- const conversationId = await this.conversationManager.createConversation( + // tabId is derived from IPC context / current window + const conversationId = await this.conversationManager.createConversation( title, settings, tabId )docs/architecture/agentic-unified-presenter.md-36-76 (1)
36-76: Add language identifiers to fenced code blocks.Several fenced blocks lack a language tag; markdownlint flags this and it reduces readability. Please add appropriate identifiers (e.g.,
text,mermaid,typescript) and apply consistently across the document.✅ Example fix
-``` +```text ┌─────────────────────────────────────────────────────────────────────┐ … └─────────────────────────────────────────────────────────────────────┘ -``` +```docs/specs/acp-architecture-refactor/spec.md-93-129 (1)
93-129: Reconcile completion status with acceptance criteria.Several inconsistencies between phase completion markers and acceptance criteria:
- Lines 114-115: Items marked "Phase 2" are unchecked, but Phase 2 is marked complete (✅) at line 256
- Lines 119-123: Code quality acceptance criteria (tests, lint, format) are mostly unchecked despite 4 phases being marked complete
- Lines 124-129: All documentation acceptance criteria remain unchecked
This creates ambiguity about the true completion status of the refactoring.
Recommendation: Either update the checkboxes to reflect actual completion status, or clarify in the phase notes which acceptance criteria are intentionally deferred.
docs/specs/agentic-renderer-integration/plan.md-51-117 (1)
51-117: Missing import forwatchin code example.Line 96 uses
watchbut it's not imported from Vue. Developers copying this example will encounter a runtime error.🔧 Proposed fix
-import { computed, ref, type Ref } from 'vue' +import { computed, ref, watch, type Ref } from 'vue' import { usePresenter } from '@/composables/usePresenter' import type { SessionInfo } from '@shared/types/agentic'docs/specs/agentic-renderer-integration/chatConfig-removal-spec.md-330-367 (1)
330-367: Automated test files need updates for removed composables.The specification includes a Testing Checklist (manual testing), but misses updates to actual test files. Two test files reference the removed composables and patterns:
test/renderer/composables/useExecutionAdapter.test.ts- RemoveselectedVariantsMapreferences and variant-related test casestest/main/presenter/modelConfig.test.ts- Update or remove tests that usechatConfigdirectlyAdd an "Affected Test Files" section to the specification listing these files and their required updates to ensure test suite remains passing after the refactoring.
docs/acp-agent-architecture.md-65-77 (1)
65-77: Clarify the session recovery flow and process crash handling as part of the user experience narrative.The document addresses session recovery (lines 488-490:
loadSessionpattern) and process crashes (lines 671-681: cleanup with UI notification), but these mechanisms are scattered across error handling sections rather than integrated into the expected user experience flow. The zero-persistence design is architecturally sound and properly justified.Consider consolidating these recovery scenarios in the compatibility section to make the recovery workflow explicit:
- Process crash during active session: Agent process terminates → UI notified via
PROCESS_CRASHEDevent → user can restart Agent from UI- App restart: DeepChat restarts → existing
sessionIdin memory becomes stale → user must callloadSessionto recover from Agent- Agent vs. persistence responsibility: DeepChat holds only ephemeral in-memory state; Agent manages all persistent state and recovery via its own storage (e.g.,
.claude/sessions/)This clarifies that the recovery responsibility is distributed: DeepChat triggers the recovery process (UI events, restart options), but the Agent is responsible for whether/how a session can be restored.
docs/architecture/acp-modes-models-specification.md-7-7 (1)
7-7: Clarify specification status.The document is marked as "Complete Specification," but Part VIII (lines 703-733) shows many unchecked implementation tasks. Consider updating the status to "Draft Specification" or "Design Specification" to reflect that implementation is pending.
docs/architecture/mcp/refactor.md-691-710 (1)
691-710: Fix relative paths for referenced documentation files.The referenced files
tool-system.mdandevent-system.mdexist indocs/architecture/, but the relative paths in the references are incorrect. Fromdocs/architecture/mcp/refactor.md, they should reference the parent directory:
- Change
./tool-system.mdto../tool-system.md- Change
./event-system.mdto../event-system.mdThe
CLAUDE.mdreference path is correct.docs/architecture/mcp/refactor.md-98-207 (1)
98-207: Well-designed architectural proposal for Prompt management that aligns with established codebase patterns.The proposed PromptPresenter architecture demonstrates strong adherence to SOLID principles:
- Single Responsibility: PromptPresenter focuses solely on Prompt domain logic
- Dependency Inversion: High-level module (PromptPresenter) doesn't depend on low-level modules (McpPresenter), both depend on abstractions
- Interface Segregation: Clean interface with focused methods
The layering is correct:
- Main Process handles business logic (aggregation, validation, transformation)
- Preload provides IPC bridge
- Renderer reduced to UI state management and query caching
Key strengths:
- Multi-source aggregation pattern (MCP + Custom + future sources)
- Security: validation and template substitution in trusted Main Process
- Multi-window consistency: single source of truth in Main Process
- Extensibility: new Prompt sources can be added without modifying existing code
Dependencies verified:
- ✓
ConfigPresenter.getCustomPrompts()exists at src/main/presenter/configPresenter/index.ts:1387- ✓
ConfigPresentersupports full CRUD operations (addCustomPrompt, updateCustomPrompt, deleteCustomPrompt)- ✓ Presenter pattern is well-established in codebase (agentPresenter, mcpPresenter, etc.)
- ⚠
getCustomPrompts()method exists in ConfigPresenter class but is not declared in IConfigPresenter interface—add method signature to interface definition before implementationImplementation notes:
- Current preload exposes simple API object via
contextBridge.exposeInMainWorld; extend pattern for PromptPresenter exposure- McpPresenter already handles MCP Prompts (listPrompts, getPrompt); propose delegation approach aligns well
🧹 Nitpick comments (35)
docs/architecture/workspace-implementation-plan.md (2)
15-62: Add fenced code block languages for markdownlint compliance.Several fenced blocks are missing a language tag; please add the appropriate language (
typescript,diff,vue,mermaid, ortext) to avoid MD040 warnings.Also applies to: 98-110, 128-148, 191-218, 229-293, 305-345, 349-355, 367-372, 384-436, 442-513
66-70: Surround tables with blank lines.Tables should be preceded and followed by a blank line to satisfy MD058 and improve readability.
Also applies to: 118-122, 156-161, 526-531
docs/architecture/event-payload-specification.md (2)
590-601: Avoid shadowing in the renderer example.The example reuses
availableModesboth as a payload field and a ref, which reads ambiguously. Consider renaming the destructured field to avoid confusion.📝 Suggested doc tweak
- const { availableModes, currentModeId } = event.sessionInfo + const { availableModes: updatedModes, currentModeId } = event.sessionInfo - if (availableModes) { - availableModes.value = availableModes + if (updatedModes) { + availableModes.value = updatedModes }
483-495: Prefer constants over raw event strings in the switch example.To reduce typo risk and keep parity with source definitions, consider referencing
ACP_WORKSPACE_EVENTSconstants in the snippet instead of string literals.docs/specs/agentic-renderer-integration/spec.md (1)
117-121: Define source-of-truth and sync rules forsessionsvssessionMetadata.REQ-2.1/2.2 specify both a list and a Map, but no authoritative source or sync/update rules are stated. This can lead to drift and reactivity issues in Pinia/Vue. Please add explicit rules (e.g., Map derived from array, or array derived from Map) and update strategy.
docs/architecture/agentic-unified-presenter.md (1)
86-430: Clarify “No Breaking Changes” vs “No Migration Path.”These statements can read as conflicting. Consider clarifying the scope (e.g., no breaking changes for existing presenters, but no migration path for renderer integration).
docs/specs/acp-architecture-refactor/spec.md (3)
203-221: Update open questions to reflect resolution status.Questions Q1-Q3 (lines 207-215) have documented decisions ("决策: ...") but remain marked as unchecked
[ ]. These should be marked as resolved[x]since decisions have been made.Questions Q4-Q6 (lines 218-221) are marked "待讨论" but lack priority indicators or target timelines. Consider adding urgency levels (e.g., "需要在 Phase 5 前决策" or "可推迟") to help prioritize discussions.
✏️ Suggested update
- [ ] **Q1**: 旧的 Session 数据是否需要导出功能？ - - **决策**: 不需要，直接清理（Session 数据由 Agent 管理） + - **决策**: 不需要，直接清理（Session 数据由 Agent 管理） - [ ] **Q2**: UI 层的 Session-UI 绑定是否需要持久化？ - - **决策**: 使用 localStorage，不需要数据库持久化 + - **决策**: 使用 localStorage，不需要数据库持久化 - [ ] **Q3**: 是否需要支持 Session 迁移到其他 Workdir？ - - **决策**: 不支持，切换 Workdir 需要创建新 Session + - **决策**: 不支持，切换 Workdir 需要创建新 Session
188-202: Consider tracking success metrics incrementally.All success metrics remain unchecked despite 4 phases being marked complete. Some metrics should be measurable now:
- Line 192: Code line reduction can be calculated (AcpProvider deletion mentioned in Phase 4)
- Line 195: Test coverage for completed modules (Phases 1-4)
- Line 121: Type checking already passes per Phase 1, 2, 4 notes
Updating these incrementally provides visibility into progress and helps validate that the refactoring is achieving its goals.
33-33: Optional: Minor grammar refinement.The static analysis tool suggests a stylistic improvement for Chinese grammar: "更快的响应速度" could be "更快地响应". However, the current phrasing is acceptable and commonly used in technical documentation.
docs/specs/agentic-renderer-integration/plan.md (3)
13-30: Add language specifier to fenced code block.The ASCII diagram would benefit from a language specifier (e.g.,
textorplaintext) for better syntax highlighting and tool compatibility.📝 Suggested fix
-``` +```text Phase 1: Foundation New composables alongside old ones │Based on static analysis hints from markdownlint-cli2.
224-348: Consider automated refactoring tools for large-scale rename.With ~350 files affected, global find-replace carries risk of unintended changes in comments, strings, or identifiers in different scopes. Consider using AST-based refactoring tools (e.g.,
jscodeshift, IDE refactoring) for safer, scope-aware renaming.The batched approach with validation mitigates risk well, but automated refactoring would provide additional safety by understanding code structure rather than text patterns.
464-474: Add language specifier to visual design block.The visual design diagram would benefit from a language specifier for better rendering.
📝 Suggested fix
-``` +```text ┌──────────────────────────────────────────────────────────┐ │ [Agent Icon] Agent Name [Status: ●] │Based on static analysis hints from markdownlint-cli2.
docs/specs/agentic-renderer-integration/chatConfig-removal-spec.md (2)
191-205: Reconsider removing variant management entirely.The rationale states this feature "adds complexity" but variant management provides real user value:
- Use case: Users comparing different AI responses to choose the best one
- Use case: A/B testing different approaches to a problem
- Use case: Recovering from a bad response without losing context
Impact: Users lose the ability to explore alternative responses, which can be valuable for iterative refinement.
Alternative: Instead of full removal, consider:
- Simplified variant management: Keep last 2 variants max (current + 1 alternative)
- Opt-in: Disable by default, enable via a simple toggle
- UI simplification: Remove complex variant selection UI, keep simple "Try another response" button
If the decision stands, document this breaking change prominently in release notes.
369-396: Expand testing checklist with edge cases and performance.Add these critical test scenarios:
Edge Cases
- Sessions created before migration still load correctly
- Sessions created during migration (mid-deploy) handle gracefully
- Sessions with null/undefined settings handle correctly
- Concurrent model/mode changes don't cause race conditions
- Network failures during session state updates are handled
Performance
- Session load time without settings column (should be faster)
- Memory usage with simplified state (should be lower)
- Model/mode switching response time
Cross-browser/Platform
- SessionInfo propagation works on Windows/Mac/Linux
- IPC message handling for session updates
Regression Prevention
- All existing automated tests pass
- No new console errors or warnings
- Error tracking (Sentry/etc.) shows no new error patterns
docs/acp-agent-architecture.md (7)
195-224: Add language specifier to the ASCII diagram code block.The fenced code block containing the architecture diagram is missing a language specifier. While ASCII diagrams don't have a standard markdown language, consider using
textormermaidfor better rendering and accessibility.📝 Suggested fix
-``` +```text ┌─────────────────────────────────────────────────────────────┐ │ Agent 对话流程 │
304-361: Well-designed interface with clear separation of concerns.The
IAcpPresenterinterface provides a clean API surface. Key strengths:
- Session lifecycle methods are explicit (create/load/close)
- Multimodal input support via
AcpPromptInput- Plain Object return types for IPC compatibility (line 333)
Note: The
sendPromptmethod (line 318) returnsPromise<void>, indicating responses are delivered via events (ACP_EVENTS.SESSION_UPDATE). Consider adding a JSDoc comment to make this async response pattern explicit for future maintainers.
376-396: Document the workdir immutability constraint prominently.The design decision that "Workdir is immutable after Session creation" (lines 392-395) has significant UX implications:
- Users cannot switch workspace within an active session
- Changing workspace requires creating a new session
- This may feel restrictive if users expect to switch directories within a session
Recommendation: Ensure this constraint is clearly communicated in:
- UI tooltips/help text when creating sessions
- Error messages if users attempt to change workdir
- User documentation
The constraint itself is architecturally sound (simplifies session management and aligns with how many Agent systems work), but user education is critical.
Do you want me to draft user-facing messaging for this constraint?
620-665: Clarify enforcement responsibility for Session-UI binding protocol.The concurrency design (lines 627-637) explicitly delegates Session-UI binding to the UI layer: "This is a UI layer protocol, not implemented in Presenter." While this separation of concerns is reasonable, it creates enforcement risks:
Concerns:
- If UI developers don't implement the binding protocol correctly, the system allows concurrent access to the same session
- No defensive checks at the Presenter level to detect or prevent this
- The Pinia store suggestion (line 640) is advisory, not enforced
Recommendations:
- Add defensive logging in AcpPresenter to detect concurrent
sendPromptcalls on the same session- Consider a lightweight session lock at the Presenter level (even if advisory) to help UI developers detect bugs
- Provide a reference implementation or code snippet for the UI binding logic to reduce implementation errors
Would you like me to draft the reference implementation for the UI binding protocol?
666-727: Consider making health check and timeout values configurable.The exception handling design is comprehensive with clear taxonomies. Two observations:
Health check frequency (line 678): "Every 5 seconds" might be aggressive for battery-powered devices. Consider:
- Making this configurable
- Using adaptive intervals (e.g., less frequent when idle)
- Allowing per-agent configuration for resource-intensive agents
Prompt timeout (line 694): "10 minutes" is a one-size-fits-all value. Some operations (simple queries) should time out faster, while others (complex code generation) might legitimately take longer. Consider:
- Making timeout configurable per-agent or per-operation
- Implementing progressive timeout warnings (e.g., warn at 5 min, timeout at 10 min)
- Allowing session-level timeout overrides
787-869: Helpful reference appendices for implementation.The appendices provide valuable implementation guidance:
- Appendix A (current structure): Clear inventory of code to migrate/remove
- Appendix B (target structure): Well-organized directory layout following single-responsibility principles
- Appendix C (key interfaces): Reinforces the zero-persistence principle (lines 860-861)
Minor note: Some interface definitions in Appendix C overlap with earlier sections. Consider using references (e.g., "See Section 2.2 for AcpSessionRecord definition") to reduce duplication and ensure consistency.
30-39: Add language specifiers to remaining fenced code blocks.Fenced code blocks at lines 31-39 and 391-395 are missing language specifiers, flagged by markdownlint-cli2. For ASCII diagrams and comments, use
textas the language identifier.📝 Suggested fixes
For lines 31-39:
-``` +```text 当前架构(错误):For lines 391-395:
-``` +```text // 关键约束:Also applies to: 376-396
docs/architecture/unified-components-specification.md (1)
27-27: Add language specifiers to fenced code blocks.Several fenced code blocks lack language identifiers for syntax highlighting. While this doesn't affect functionality, adding language specifiers improves documentation readability.
Based on learnings, the static analysis hint is valid - these are diagram/flowchart blocks that could use a text identifier or be left as-is for plain formatting.
Also applies to: 100-100, 421-421, 474-474, 716-716, 749-749, 992-992, 1017-1017
docs/architecture/renderer-integration-design.md (1)
1118-1126: Synchronize open questions with completed research.Question Q3 (line 1124) asks "How to handle ACP workdir in unified config?" but this appears to be resolved in
workspace-implementation-plan.mdreferenced in other docs. Consider updating Appendix C to reflect completed research or cross-reference the resolution.Update the open questions section to reference completed work:
-3. **ACP Workspace**: How to handle ACP workdir in unified config? - - **Recommendation**: Use `workspacePath` field, let ACP presenter handle workdir mapping +3. **ACP Workspace**: ~~How to handle ACP workdir in unified config?~~ + - **Status**: ✅ Resolved in `workspace-implementation-plan.md` + - **Resolution**: Use `workspacePath` field in SessionConfig, presenter handles agent-specific workspace managementdocs/architecture/renderer-analysis-research.md (1)
105-105: Minor grammar issue in component requirements.Line 105 uses non-standard grammar: "Each component needs detailed spec" should be "Each component needs a detailed spec" or "Each component needs to be detailed."
Based on static analysis, this is a minor style issue but affects documentation clarity.
docs/architecture/renderer-investigation.md (1)
150-189: Clarify the agentId/threadId parameter overloading.The implementation shows that
threadIdis passed as theagentIdparameter toagentPresenter.sendMessage()(line 162):const aiResponseMessage = await agentP.sendMessage( threadId, // NOTE: Uses conversationId as agentId ... )This overloading of parameter semantics (passing
threadIdwhereagentIdis expected) could cause confusion during the migration to the unified system. Consider adding a clarifying note explaining:
- Why this overloading exists in the current system
- How the presenter resolves
threadIdto the actualagentId- How this will change in the unified system (sessionId-based flow)
This will help developers understand the current architecture's quirks and why the refactoring is necessary.
docs/architecture/acp-commands-specification.md (6)
71-98: Add language identifier to fenced code block.The lifecycle diagram fenced code block is missing a language identifier, which triggers a markdownlint warning. For ASCII diagrams, use
textas the language identifier.📝 Suggested fix
-``` +```text ┌─────────────────────────────────────────────────────────────────────┐ │ Command Lifecycle │
220-256: Add language identifier to fenced code block.The event flow diagram is missing a language identifier. Use
textfor ASCII diagrams.📝 Suggested fix
-``` +```text ┌──────────────────────────────────────────────────────────────────┐ │ Command Update Event Flow │
391-410: Add language identifier to fenced code block.The visual design mockup is missing a language identifier. Use
textfor ASCII art.📝 Suggested fix
-``` +```text ┌─────────────────────────────────────────────────────────────────┐ │ Available Commands (3) [Collapse ▼] │
440-446: Improve command icon mapping logic.The
getCommandIconfunction usesString.includes()which may produce false positives. For example,read_configurationwould match the 'read' condition even though it's not a file operation.🔍 More robust icon mapping
const getCommandIcon = (name: string): string => { - // Map command names to icons - if (name.includes('run') || name.includes('exec')) return '⚡' - if (name.includes('read') || name.includes('file')) return '📄' - if (name.includes('write') || name.includes('save')) return '✏️' - return '🔧' + // Map command names to icons with exact matches first, then substring + const iconMap: Record<string, string> = { + 'run_command': '⚡', + 'exec_command': '⚡', + 'read_file': '📄', + 'write_file': '✏️', + 'save_file': '✏️' + } + + // Try exact match first + if (iconMap[name]) return iconMap[name] + + // Fall back to substring matching with more specific patterns + if (/^(run|exec)_/.test(name)) return '⚡' + if (/^read_/.test(name)) return '📄' + if (/^(write|save)_/.test(name)) return '✏️' + + return '🔧' }
453-489: Add accessibility attributes.The component is missing accessibility attributes for interactive elements. Screen reader users won't understand the purpose of buttons and clickable areas.
♿ Add ARIA attributes
<div v-if="commands.length > 0" class="commands-display"> <div class="commands-header"> <span class="commands-title"> Available Commands ({{ commands.length }}) </span> <button v-if="commands.length > maxVisible" class="toggle-button" + :aria-label="isExpanded ? 'Collapse commands' : 'Expand commands'" + :aria-expanded="isExpanded" `@click`="isExpanded = !isExpanded" > {{ isExpanded ? 'Collapse ▼' : 'Expand ▶' }} </button> </div> - <div v-if="isExpanded" class="commands-list"> + <div + v-if="isExpanded" + class="commands-list" + role="list" + aria-label="Available commands" + > <div v-for="cmd in visibleCommands" :key="cmd.name" class="command-item" + role="listitem" + tabindex="0" + :aria-label="`Command: ${cmd.name}${cmd.description ? '. ' + cmd.description : ''}`" `@click`="emit('command-click', cmd)" + `@keydown.enter`="emit('command-click', cmd)" + `@keydown.space.prevent`="emit('command-click', cmd)" >
608-635: Consider adding error handling to implementation checklist.The checklist is comprehensive but doesn't include error handling scenarios. Consider adding items for:
- Handling malformed command data from agent process
- Handling command invocation failures
- Error states in UI (e.g., agent disconnection while showing commands)
- Graceful degradation when commands are not available
docs/architecture/acp-modes-models-specification.md (1)
616-625: Consider making mode icons configurable.The
modeIconcomputed property uses a hardcodediconMap(lines 618-624). For ACP agents with custom modes, these icons won't apply. Consider:
- Accepting an optional
iconMapprop- Using a default icon for unmapped modes (currently done at line 624)
- Allowing mode definitions to include an
iconpropertydocs/architecture/mcp/refactor.md (3)
1-95: Excellent architectural analysis and problem identification.The document correctly identifies the fundamental issues with the current architecture:
- Mixed responsibilities in
useMcpStoreService- Business logic in the Renderer process (security and consistency concerns)
- Unclear domain boundaries between MCP and Prompts
The three core insights are architecturally sound:
- Prompt as independent domain - correctly recognizes that Prompts are not MCP-specific
- Main Process for business logic - aligns with Electron's security model and the Presenter pattern from the retrieved learnings
- Data transformation in infrastructure layer - proper application of Dependency Inversion Principle (DIP)
Based on learnings, the Presenter pattern approach and EventBus for inter-process communication are the established patterns in this codebase.
Minor suggestion: Consider avoiding specific line number references (e.g., "168 行" on line 13, "lines 745-791" on line 371) as they can become outdated quickly. Instead, reference files/functions/classes for more durable documentation.
279-336: Solid migration strategy with clear phases.The four-phase approach is well-structured and follows a safe incremental pattern (create new → bridge → migrate → cleanup). The SOLID principles section effectively reinforces the architectural decisions.
Strengths:
- Incremental, low-risk approach
- Clear responsibilities per phase
- Strong grounding in design principles (DIP, SRP, OCP, ISP)
Suggested enhancements:
- Testing strategy: Consider adding testing checkpoints throughout phases 1-3, not just in Phase 4 cleanup
- Component migration tracking: Phase 3 mentions "更新所有使用 Prompts 的组件" - this could be substantial. Consider:
- Creating a checklist of affected components
- Running a script to identify all Prompt consumers before migration
- Backward compatibility: Consider whether both old and new APIs should coexist temporarily during Phase 3
- Rollback strategy: Document how to revert if critical issues are discovered mid-migration
🔍 Script to identify Prompt consumers for migration tracking
#!/bin/bash # Find all Vue components and composables that use MCP/Prompt functionality echo "=== Vue components using Prompts ===" rg -t vue -l "useMcpStoreService|getAllPrompts|getPrompt" src/renderer echo -e "\n=== TypeScript files using Prompts ===" rg -t ts -l "useMcpStoreService.*prompt|getAllPrompts|getPrompt" src/renderer echo -e "\n=== Detailed usage patterns ===" rg -n --type ts --type vue "getAllPrompts|getPrompt" src/renderer -C 2
39-49: Add language specifiers to fenced code blocks for better rendering.Several fenced code blocks are missing language specifiers, which affects syntax highlighting and markdown rendering. Static analysis correctly identifies these at lines 17, 63, 209, and 562 (among others).
Recommended language tags:
- TypeScript code blocks:
```typescript- Text/ASCII diagrams:
```text(or leave plain if preferred)- Generic pseudocode:
```textor specific languageExamples needing tags:
- Lines 39-42, 45-49: Architecture diagrams →
```text- Lines 162-173: TypeScript interface →
```typescript- Lines 311-335: SOLID principles explanations →
```text- Lines 430-487: TypeScript preprocessor code →
```typescript- Lines 557-592: TypeScript ToolManager code →
```typescript- Lines 607-618: TypeScript simplified code →
```typescript✏️ Example fixes
**错误的理解：** -``` +```text ❌ Prompt 属于 MCP └─ MCP Service 管理 Prompts关键方法：
-typescript +typescript
interface IPromptPresenter {
// 查询
getAllPrompts(): Promise<PromptListEntry[]></details> This is a documentation quality improvement that enhances readability and tool support. Also applies to: 58-68, 162-173, 214-223, 311-314, 318-321, 324-328, 332-335, 430-487, 557-592, 607-618, 635-640, 643-646 </blockquote></details> </blockquote></details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| ## 迁移策略 | ||
|
|
||
| ### Phase 1: 创建独立的 ACP 服务层 | ||
|
|
||
| 1. 新建 `src/main/presenter/acpPresenter/` 目录 | ||
| 2. 实现 `AcpPresenter` 接口及相关组件 | ||
| 3. 迁移现有 ACP 相关代码（不删除旧代码） | ||
| 4. 新旧系统并存，通过配置切换 | ||
|
|
||
| ### Phase 2: 拆分消息处理逻辑 | ||
|
|
||
| **关键改动**： | ||
| 1. 实现 `AcpInputFormatter`：支持多模态输入格式化 | ||
| 2. 在 `AcpPresenter` 中实现独立的消息发送流程 | ||
| 3. 移除 `AgentPresenter` 对 ACP 的调用 | ||
| 4. 更新事件监听：从 `STREAM_EVENTS` 迁移到 `ACP_EVENTS` | ||
| 5. 确保所有通过 IPC 传递的对象都是可序列化的 Plain Object | ||
|
|
||
| ### Phase 3: 移除数据持久化 | ||
|
|
||
| **改动清单**： | ||
| 1. 删除 `acp_sessions` 表及相关代码 | ||
| 2. 移除 `AcpSessionPersistence` 类 | ||
| 3. 将 Session 管理改为纯内存实现 | ||
| 4. 清理与 `conversations` 表的关联 | ||
|
|
||
| ### Phase 4: 移除旧代码 | ||
|
|
||
| **清理范围**： | ||
| 1. 删除 `AcpProvider` 类（1289 行） | ||
| 2. 清理 `LLMProviderPresenter` 中的 ACP 相关逻辑 | ||
| 3. 更新 UI 组件，使用新的 ACP 服务接口 | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Enhance migration plan with testing and rollback strategies.
The four-phase migration (lines 730-760) provides a good high-level structure, but lacks operational details:
Missing elements:
- Testing strategy: How will each phase be validated? Unit tests, integration tests, user acceptance testing?
- Rollback plan: If Phase 2 or 3 reveals critical issues, what's the rollback procedure?
- Feature flag management: Line 735 mentions "switch via configuration" - specify the flag name, default value, and transition timeline
- Success metrics: What criteria determine each phase is complete and safe to proceed?
- Timeline estimates: Rough duration for each phase to set expectations
Recommendation: Expand each phase with:
- Prerequisites and dependencies
- Specific deliverables
- Testing acceptance criteria
- Rollback procedure
- Estimated duration
🤖 Prompt for AI Agents
In `@docs/acp-agent-architecture.md` around lines 728 - 760, Add testing,
rollback, feature-flag, success-metrics and timeline details to each migration
phase: for Phase 1 (AcpPresenter creation) list prerequisites, unit/integration
tests for AcpPresenter and migration steps, a rollback plan to revert to
AgentPresenter usage and config flag details (name, default=false, rollout
steps), success criteria and estimated duration; for Phase 2 (AcpInputFormatter,
message flow, STREAM_EVENTS→ACP_EVENTS) add automated tests for
AcpInputFormatter and IPC serialization checks, staged feature-flag switch
procedure, rollback steps to re-enable AgentPresenter calls, acceptance tests
and duration; for Phase 3 (remove acp_sessions, AcpSessionPersistence, shift to
in-memory, conversations cleanup) define migration tests, data backup/restore
rollback plan, validation scripts, success metrics and timeline; for Phase 4
(delete AcpProvider, update LLMProviderPresenter and UI) include integration and
UAT plans, feature-flag finalization and cutover checklist, explicit
deliverables and estimated durations—reference symbols: AcpPresenter,
AcpInputFormatter, AgentPresenter, STREAM_EVENTS, ACP_EVENTS, acp_sessions,
AcpSessionPersistence, AcpProvider, LLMProviderPresenter, conversations.
| ## 兼容性考虑 | ||
|
|
||
| ### 对现有用户的影响 | ||
|
|
||
| 1. **会话持久化**：调整后，DeepChat 不再持久化 ACP Session | ||
| - 影响：应用重启后，需要重新创建或加载 Session | ||
| - 解决方案：由 ACP Agent 自己管理 Session 恢复（通过 `loadSession`） | ||
|
|
||
| 2. **历史数据清理**：现有的 `acp_sessions` 表数据将被清理 | ||
| - 影响：旧的 Session 元数据将丢失 | ||
| - 解决方案：这些数据本来就不应该存在，清理是正确的 | ||
|
|
||
| 3. **配置迁移**：ACP Agent 配置需要迁移到新的存储位置 | ||
| - 解决方案：自动迁移脚本 | ||
|
|
||
| 4. **UI 变化**：ACP 会话入口将与普通对话分离 | ||
| - 解决方案：提供清晰的 UI 引导 | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Improve user communication and migration support.
The compatibility section identifies key impacts but lacks concrete mitigation:
-
Session persistence (lines 765-767): "Need to recreate or reload Session" - this is a significant UX degradation for users accustomed to persistent sessions.
- Recommendation: Provide a migration assistant UI that guides users through the first reload after upgrade
- Consider a one-time data export tool to help users understand their existing ACP sessions before cleanup
-
Historical data cleanup (lines 769-772): The justification "This data shouldn't exist anyway" is technically correct but tone-deaf to users who may have relied on it.
- Recommendation: Rephrase to acknowledge the change: "ACP session metadata will be removed to align with the correct architecture where Agents manage their own persistence."
- Offer a data export option before cleanup
-
UI changes (lines 776-777): "Provide clear UI guidance" is too vague.
- Recommendation: Draft specific UI messaging, onboarding tooltips, or a changelog entry users will see on first launch
🤖 Prompt for AI Agents
In `@docs/acp-agent-architecture.md` around lines 761 - 778, Update the "兼容性考虑"
section to provide concrete migration and user-communication steps: under
"会话持久化" (mentioning loadSession) add a requirement to implement a one-time
Migration Assistant UI that guides users through session reload on first launch
and offer a one-time data export tool; under "历史数据清理" (referencing acp_sessions)
replace the dismissive sentence with: "ACP session metadata will be removed to
align with the architecture where Agents manage persistence" and add an explicit
pre-cleanup data export option; under "UI 变化" add specific copy and UX items
(first-launch onboarding tooltip, in-app changelog entry, and contextual
banners) that instruct users how to recreate/load sessions and where to find
migration tools.
| <div v-if="isExpanded" class="commands-list"> | ||
| <div | ||
| v-for="cmd in visibleCommands" | ||
| :key="cmd.name" | ||
| class="command-item" | ||
| @click="emit('command-click', cmd)" | ||
| > | ||
| <div class="command-main"> | ||
| <span class="command-icon">{{ getCommandIcon(cmd.name) }}</span> | ||
| <span class="command-name">{{ cmd.name }}</span> | ||
| <button class="command-run">Run</button> | ||
| </div> | ||
| <div v-if="cmd.description" class="command-description"> | ||
| {{ cmd.description }} | ||
| </div> | ||
| <div v-if="cmd.inputHint" class="command-hint"> | ||
| Hint: {{ cmd.inputHint }} | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Resolve nested clickable elements UX issue.
The component has nested clickable elements: the entire command-item div (line 473) is clickable, but it also contains a "Run" button (line 478). This creates an ambiguous interaction model:
- What happens when the user clicks the command name vs the Run button?
- The Run button will trigger both its own click and the parent div's click event.
🎯 Recommended fix: Remove nested clickability
Option 1: Make only the Run button clickable:
<div
v-for="cmd in visibleCommands"
:key="cmd.name"
class="command-item"
- `@click`="emit('command-click', cmd)"
>
<div class="command-main">
<span class="command-icon">{{ getCommandIcon(cmd.name) }}</span>
<span class="command-name">{{ cmd.name }}</span>
- <button class="command-run">Run</button>
+ <button
+ class="command-run"
+ `@click`="emit('command-click', cmd)"
+ >
+ Run
+ </button>
</div>Option 2: Make the whole card clickable and remove the Run button:
<div class="command-main">
<span class="command-icon">{{ getCommandIcon(cmd.name) }}</span>
<span class="command-name">{{ cmd.name }}</span>
- <button class="command-run">Run</button>
</div>📝 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.
| <div v-if="isExpanded" class="commands-list"> | |
| <div | |
| v-for="cmd in visibleCommands" | |
| :key="cmd.name" | |
| class="command-item" | |
| @click="emit('command-click', cmd)" | |
| > | |
| <div class="command-main"> | |
| <span class="command-icon">{{ getCommandIcon(cmd.name) }}</span> | |
| <span class="command-name">{{ cmd.name }}</span> | |
| <button class="command-run">Run</button> | |
| </div> | |
| <div v-if="cmd.description" class="command-description"> | |
| {{ cmd.description }} | |
| </div> | |
| <div v-if="cmd.inputHint" class="command-hint"> | |
| Hint: {{ cmd.inputHint }} | |
| </div> | |
| </div> | |
| <div v-if="isExpanded" class="commands-list"> | |
| <div | |
| v-for="cmd in visibleCommands" | |
| :key="cmd.name" | |
| class="command-item" | |
| > | |
| <div class="command-main"> | |
| <span class="command-icon">{{ getCommandIcon(cmd.name) }}</span> | |
| <span class="command-name">{{ cmd.name }}</span> | |
| <button | |
| class="command-run" | |
| `@click`="emit('command-click', cmd)" | |
| > | |
| Run | |
| </button> | |
| </div> | |
| <div v-if="cmd.description" class="command-description"> | |
| {{ cmd.description }} | |
| </div> | |
| <div v-if="cmd.inputHint" class="command-hint"> | |
| Hint: {{ cmd.inputHint }} | |
| </div> | |
| </div> |
🤖 Prompt for AI Agents
In `@docs/architecture/acp-commands-specification.md` around lines 468 - 486, The
parent .command-item div currently has `@click`="emit('command-click', cmd)" while
its child "Run" button is also clickable, causing nested click events; to fix,
remove the parent `@click` from the .command-item element and move the
emit('command-click', cmd) handler to the button (add
`@click`="emit('command-click', cmd)" on the "Run" button element), ensuring only
the Run button triggers the action and avoiding duplicate events from
command-item; update any references to keyboard/aria handlers as needed for
accessibility on the button.
| ```typescript | ||
| function normalizeSessionModesUpdate( | ||
| data: Record<string, unknown>, | ||
| emitter: AgenticEventEmitter | ||
| ): void { | ||
| const available = data.available as Array<{ | ||
| id: string | ||
| name: string | ||
| description: string | ||
| }> | ||
| const current = data.current as string | ||
|
|
||
| emitter.sessionUpdated({ | ||
| availableModes: available, | ||
| currentModeId: current | ||
| }) | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Missing sessionId in event emission.
The normalizeSessionModesUpdate function emits sessionUpdated() without including the sessionId. The renderer's handleSessionUpdated (line 477) checks event.sessionId, but this normalizer doesn't provide it.
The comment at line 247 indicates conversationId maps to sessionId, but the code doesn't perform this mapping.
🐛 Proposed fix to include sessionId mapping
function normalizeSessionModesUpdate(
data: Record<string, unknown>,
emitter: AgenticEventEmitter
): void {
+ const sessionId = data.conversationId as string
const available = data.available as Array<{
id: string
name: string
description: string
}>
const current = data.current as string
emitter.sessionUpdated({
+ sessionId,
availableModes: available,
currentModeId: current
})
}🤖 Prompt for AI Agents
In `@docs/architecture/acp-modes-models-specification.md` around lines 267 - 284,
The normalizeSessionModesUpdate function emits sessionUpdated without the
required sessionId; update normalizeSessionModesUpdate to read
data.conversationId (per the comment that conversationId maps to sessionId), map
it to sessionId and include it in the payload passed to emitter.sessionUpdated({
availableModes, currentModeId, sessionId }), ensuring the emitted event matches
what handleSessionUpdated expects.
| function normalizeSessionModelsUpdate( | ||
| data: Record<string, unknown>, | ||
| emitter: AgenticEventEmitter | ||
| ): void { | ||
| const available = data.available as Array<{ | ||
| id: string | ||
| name: string | ||
| description?: string | ||
| }> | ||
| const current = data.current as string | ||
|
|
||
| emitter.sessionUpdated({ | ||
| availableModels: available, | ||
| currentModelId: current | ||
| }) | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Missing sessionId in event emission.
Same issue as the modes normalizer: normalizeSessionModelsUpdate doesn't include sessionId in the emission, making it impossible for the renderer to route the update to the correct session.
🐛 Proposed fix to include sessionId mapping
function normalizeSessionModelsUpdate(
data: Record<string, unknown>,
emitter: AgenticEventEmitter
): void {
+ const sessionId = data.conversationId as string
const available = data.available as Array<{
id: string
name: string
description?: string
}>
const current = data.current as string
emitter.sessionUpdated({
+ sessionId,
availableModels: available,
currentModelId: current
})
}📝 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.
| function normalizeSessionModelsUpdate( | |
| data: Record<string, unknown>, | |
| emitter: AgenticEventEmitter | |
| ): void { | |
| const available = data.available as Array<{ | |
| id: string | |
| name: string | |
| description?: string | |
| }> | |
| const current = data.current as string | |
| emitter.sessionUpdated({ | |
| availableModels: available, | |
| currentModelId: current | |
| }) | |
| } | |
| ``` | |
| function normalizeSessionModelsUpdate( | |
| data: Record<string, unknown>, | |
| emitter: AgenticEventEmitter | |
| ): void { | |
| const sessionId = data.conversationId as string | |
| const available = data.available as Array<{ | |
| id: string | |
| name: string | |
| description?: string | |
| }> | |
| const current = data.current as string | |
| emitter.sessionUpdated({ | |
| sessionId, | |
| availableModels: available, | |
| currentModelId: current | |
| }) | |
| } |
🤖 Prompt for AI Agents
In `@docs/architecture/acp-modes-models-specification.md` around lines 315 - 331,
normalizeSessionModelsUpdate currently emits availableModels and currentModelId
but omits the sessionId; update the function to read the sessionId from the
incoming data (e.g., const sessionId = data.sessionId as string) and include
sessionId in the emitter.sessionUpdated payload so the event includes {
sessionId, availableModels: available, currentModelId: current }; adjust any
typing if needed to reflect sessionId presence.
| ### ❌ REMOVE: All LLM Parameters | ||
|
|
||
| These parameters are **no longer configurable** by users: | ||
|
|
||
| | Field | Removal Reason | | ||
| |-------|----------------| | ||
| | `temperature` | Use agent default | | ||
| | `contextLength` | Use agent default | | ||
| | `maxTokens` | Use agent default | | ||
| | `thinkingBudget` | Use agent default | | ||
| | `reasoningEffort` | Use agent default | | ||
| | `verbosity` | Use agent default | | ||
|
|
||
| --- | ||
|
|
||
| ### ❌ REMOVE: Feature Flags | ||
|
|
||
| | Field | Removal Reason | | ||
| |-------|----------------| | ||
| | `artifacts` | Feature removed | | ||
| | `enableSearch` | Always enabled if supported | | ||
| | `forcedSearch` | Feature removed | | ||
| | `searchStrategy` | Use agent default | | ||
|
|
||
| --- | ||
|
|
||
| ### ❌ REMOVE: Tool Selection | ||
|
|
||
| | Field | Removal Reason | | ||
| |-------|----------------| | ||
| | `enabledMcpTools` | All available tools enabled by default | | ||
|
|
||
| **Note**: Tools can still be disabled at the MCP server level, but per-session tool selection is removed. |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Reconsider removing all user configurability.
While simplification is valuable, removing all LLM parameters and tool selection may overcorrect:
LLM Parameters (lines 28-40):
- Common use cases require different settings (creative writing needs high temperature, code generation needs low)
- Consider keeping a minimal set:
temperatureandmaxTokens(most commonly adjusted) - Could use presets ("Creative", "Balanced", "Precise") instead of raw parameters
MCP Tool Selection (lines 54-60):
- Always enabling all tools raises privacy/security concerns (e.g., a tool that reads files shouldn't always be available)
- Users may want to disable specific tools per-session
- Consider: Default-on with ability to disable, rather than forced-always-on
Alternative approach: Keep a minimal, high-level configuration surface (3-5 settings max) rather than zero configurability.
🤖 Prompt for AI Agents
In `@docs/specs/agentic-renderer-integration/chatConfig-removal-spec.md` around
lines 28 - 60, Revert the blanket removal in the spec: restore a minimal
user-config surface by keeping at least `temperature` and `maxTokens` (or
replace them with named presets like "Creative", "Balanced", "Precise") instead
of deleting all LLM parameters, and change the MCP tool policy from "all
enabled" to "default-on with per-session disable" (or allow a simple
`enabledMcpTools`/`disabledMcpTools` list) so users can opt out of sensitive
tools; update the sections that currently remove `temperature`, `maxTokens`,
`enabledMcpTools`, `artifacts`, and `enableSearch` to reflect these minimal
configurable options and presets.
| ## Database Schema Changes | ||
|
|
||
| ### Remove Column | ||
|
|
||
| ```sql | ||
| -- Remove settings column from conversations table | ||
| ALTER TABLE conversations DROP COLUMN settings; | ||
| ``` | ||
|
|
||
| ### Remove Type | ||
|
|
||
| ```typescript | ||
| // Remove from thread.presenter.d.ts | ||
| export type CONVERSATION_SETTINGS = { ... } // DELETE THIS | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Implement safe, reversible database migration.
The current migration is destructive and irreversible. Recommend a phased approach:
Phase 1: Deprecate (don't drop)
-- Mark column as deprecated, but keep data
ALTER TABLE conversations RENAME COLUMN settings TO settings_deprecated;
COMMENT ON COLUMN conversations.settings_deprecated IS 'Deprecated: Will be removed in v2.0';Phase 2: Data preservation (optional)
-- For users who want to preserve their settings
CREATE TABLE conversation_settings_archive (
conversation_id TEXT PRIMARY KEY,
settings JSONB,
archived_at TIMESTAMP DEFAULT NOW()
);
INSERT INTO conversation_settings_archive
SELECT id, settings_deprecated FROM conversations WHERE settings_deprecated IS NOT NULL;Phase 3: Drop (after validation period)
-- Only after confirming no issues in production
ALTER TABLE conversations DROP COLUMN settings_deprecated;This allows rollback if critical issues are discovered post-deployment.
🤖 Prompt for AI Agents
In `@docs/specs/agentic-renderer-integration/chatConfig-removal-spec.md` around
lines 82 - 96, Do not drop the conversations.settings column or remove the
CONVERSATION_SETTINGS type directly; instead implement a phased, reversible
migration: 1) Deprecate by renaming conversations.settings to
settings_deprecated and add a comment to mark it deprecated (so code referencing
conversations.settings can be switched safely). 2) Optionally preserve data by
creating a conversation_settings_archive table and copying non-null
settings_deprecated rows into it for rollback/inspection. 3) After a validation
period, remove settings_deprecated and delete the CONVERSATION_SETTINGS type
(thread.presenter.d.ts) once all references are migrated; ensure migration
scripts reference conversations.settings_deprecated,
conversation_settings_archive, and the CONVERSATION_SETTINGS type names so the
process is reversible.
|
|
||
| ## Migration Plan | ||
|
|
||
| ### Step 1: Update useSessionConfig | ||
|
|
||
| Ensure `useSessionConfig` provides all needed session state: | ||
| - ✅ Already done in Phase 1-5 | ||
| - Returns `modelId`, `modeId`, `workspace`, `agentId` | ||
|
|
||
| ### Step 2: Update Consumers | ||
|
|
||
| Replace all `chatConfig` usage: | ||
|
|
||
| | Before | After | | ||
| |--------|-------| | ||
| | `chatStore.chatConfig.modelId` | `useSessionConfig(sessionId).modelId` | | ||
| | `chatStore.chatConfig.providerId` | `useSessionConfig(sessionId).agentId` | | ||
| | `chatStore.chatConfig.contextLength` | *Remove (use default)* | | ||
| | `chatStore.chatConfig.temperature` | *Remove (use default)* | | ||
| | `chatStore.chatConfig.enabledMcpTools` | *Remove (all tools enabled)* | | ||
|
|
||
| ### Step 3: Update Components | ||
|
|
||
| 1. **ModelSelect.vue** - Read from SessionInfo | ||
| 2. **ChatInput.vue** - Remove config props | ||
| 3. **McpToolsList.vue** - Simplify to display-only | ||
| 4. **usePromptInputConfig.ts** - DELETE | ||
|
|
||
| ### Step 4: Database Migration | ||
|
|
||
| ```sql | ||
| -- Migration script | ||
| ALTER TABLE conversations DROP COLUMN settings; | ||
| ``` | ||
|
|
||
| ### Step 5: Cleanup | ||
|
|
||
| 1. Delete `useChatConfig.ts` | ||
| 2. Delete `useVariantManagement.ts` (variant management removed) | ||
| 3. Delete `useMcpSamplingStoreService.ts` (MCP sampling removed) | ||
| 4. Delete `useThreadExport.ts` (use `useSessionExport`) | ||
| 5. Delete `useChatAdapter.ts` (use `useAgenticAdapter`) | ||
| 6. Delete `usePromptInputConfig.ts` | ||
| 7. Remove `CONVERSATION_SETTINGS` type | ||
| 8. Update `SessionPresenter` interfaces | ||
| 9. Remove variant-related UI components | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add sequencing, validation, and risk mitigation to migration plan.
The migration plan is comprehensive but would benefit from:
1. Execution Order & Dependencies
Step 1: Database backup (new)
Step 2: Update useSessionConfig (current Step 1)
Step 3: Deploy backend changes (ensure SessionInfo available)
Step 4: Update components (current Step 3)
↳ Sub-step: Update high-priority components first
↳ Sub-step: Validate each component before proceeding
Step 5: Update consumers (current Step 2)
Step 6: Database migration (current Step 4)
↳ Only after all code changes deployed and validated
Step 7: Cleanup (current Step 5)
2. Validation Gates
- After each step, validate:
- No console errors
- Session state loads correctly
- Model/mode selection functional
- Run regression tests before proceeding to next step
3. Feature Flags (recommended)
const ENABLE_SESSION_CONFIG = true // Toggle new architecture
const DROP_SETTINGS_COLUMN = false // Enable only after validationThis allows incremental rollout and quick rollback without database changes.
🤖 Prompt for AI Agents
In `@docs/specs/agentic-renderer-integration/chatConfig-removal-spec.md` around
lines 282 - 328, The migration plan lacks explicit execution order, validation
gates, and rollback controls—add a pre-migration DB backup step and a sequenced
plan that runs: 1) DB backup, 2) update useSessionConfig, 3) deploy backend
changes ensuring SessionInfo is available to SessionPresenter, 4) update
high-priority components (ModelSelect.vue, ChatInput.vue, McpToolsList.vue) with
per-component validation, 5) replace consumers (references to
chatStore.chatConfig → useSessionConfig(sessionId)), 6) run the ALTER TABLE
conversations DROP COLUMN settings only after successful validation, and 7)
cleanup; introduce feature flags (e.g., ENABLE_SESSION_CONFIG,
DROP_SETTINGS_COLUMN) to toggle the new behavior, add validation gates after
each step (no console errors, session state loads, model/mode selection works,
regression tests pass), and ensure rollback plans for each step before removing
usePromptInputConfig.ts and related types (CONVERSATION_SETTINGS) or updating
SessionPresenter interfaces.
|
|
||
| ## Rollback Plan | ||
|
|
||
| If issues arise: | ||
|
|
||
| 1. **Keep settings column** but mark as deprecated | ||
| 2. **Add default values** for removed fields |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Provide concrete rollback procedures.
The current rollback plan is too abstract. Specify executable steps:
Immediate Rollback (within 24h of deployment)
- Revert Git commit:
git revert <migration-commit-sha> - Restore settings column (if dropped):
ALTER TABLE conversations ADD COLUMN settings JSONB - Restore from
conversation_settings_archive(if created) - Redeploy previous version
- Monitor error rates and user reports
Partial Rollback (if some changes work)
- Use feature flags to disable problematic parts:
const USE_SESSION_CONFIG = false // Rollback to chatConfig
- Keep working components, revert broken ones
- Document which parts are reverted
Timeline for Deprecated Mode
- Week 1-2: Monitor closely, immediate rollback if critical issues
- Week 3-4: Address minor issues, gather feedback
- Month 2-3: Final validation period
- Month 4+: Remove deprecated code if stable
Success Criteria for NOT Rolling Back
- Error rate < 0.1% increase
- No P0/P1 bugs related to session state
- User feedback predominantly neutral or positive
🤖 Prompt for AI Agents
In `@docs/specs/agentic-renderer-integration/chatConfig-removal-spec.md` around
lines 398 - 404, Update the rollback plan to include concrete, executable steps:
list the exact Git revert command pattern (git revert <migration-commit-sha>)
and sequence for immediate rollback, SQL to re-add the settings column (ALTER
TABLE conversations ADD COLUMN settings JSONB) and to restore from
conversation_settings_archive, instructions to redeploy the previous release and
monitor metrics; add a partial-rollback section describing toggling the feature
flag USE_SESSION_CONFIG = false to revert to chatConfig and which components to
isolate/leave running; include a clear timeline for deprecated mode (weeks 1–4,
months 2–4) and measurable success criteria (error rate threshold, no P0/P1
bugs, user feedback) so the document references conversation_settings_archive,
USE_SESSION_CONFIG, chatConfig, and the settings column for easy location.
| ### Non-Goals | ||
|
|
||
| - NO changes to SQLite schema (database keeps `conversationId`) | ||
| - NO changes to ACP protocol (agent interface unchanged) | ||
| - NO backward compatibility with old event types (clean break) | ||
| - NO feature flags or gradual migration (atomic change) | ||
|
|
There was a problem hiding this comment.
Clarify “no gradual migration” vs phased migration plan.
Non-Goals says an atomic change with no gradual migration, but Migration Strategy describes a phased rollout. Please reconcile to avoid conflicting implementation guidance (e.g., clarify if phases are internal planning with a single ship, or if incremental shipping is allowed).
Also applies to: 352-362
🤖 Prompt for AI Agents
In `@docs/specs/agentic-renderer-integration/spec.md` around lines 46 - 52, The
"Non-Goals" and "Migration Strategy" are inconsistent: update the "Non-Goals"
and the corresponding lines in "Migration Strategy" so they explicitly reconcile
the two (e.g., change the Non-Goals bullet "NO feature flags or gradual
migration (atomic change)" to clarify whether that means "no public incremental
shipping" or remove it and instead state "no backward-compatible public
migration; internal phased rollout allowed for testing." Reference the
"Non-Goals" section and the "Migration Strategy" section and make a single clear
sentence that either (A) prohibits incremental public shipping and notes that
any phased steps are internal testing prior to one atomic ship, or (B) permits a
phased public rollout and remove the "NO gradual migration" line—pick one policy
and make the language explicit so the two sections no longer conflict.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/architecture/unified-components-specification.md`:
- Around line 27-46: Several fenced code blocks in the Unified Components
Specification are missing language identifiers (markdownlint MD040); update each
triple-backtick fence to include a language (use "text") so blocks like the one
starting with "ChatInput.vue ├── AgentHeader (NEW)..." and the other affected
blocks (the blocks beginning around the sections that list component trees and
examples at the ranges referenced in your comment) use ```text instead of ```;
search for the fenced blocks that render plain diagrams or component lists and
replace the opening ``` with ```text for each occurrence.
♻️ Duplicate comments (1)
docs/architecture/unified-components-specification.md (1)
195-203: Branching on agentId contradicts the “No Branching” principle.The spec’s examples still parse
agentIdin components (Lines 195–203, 463–465, 536–538, 1052–1053), which conflicts with Design Principle#5and the earlier review note. Consider pushing agent-type display data intoSessionInfoand keeping components fully agent-agnostic.Also applies to: 463-465, 536-538, 1052-1053
🧹 Nitpick comments (1)
docs/architecture/unified-components-specification.md (1)
1125-1129: Route directory picking through a presenter instead of direct IPC.The spec suggests
window.electron.ipcRenderer.invoke(...)in the renderer. This conflicts with the repo guideline to useusePresenterfor main-process communication. Please update the spec to call a presenter method (e.g.,workspacePresenter.selectDirectory()), and keep IPC behind the presenter boundary. Based on learnings, ...
| ``` | ||
| ChatInput.vue | ||
| ├── AgentHeader (NEW) | ||
| │ └── Displays: agent name, icon, status, capabilities | ||
| │ | ||
| ├── UnifiedModelSelector (NEW) | ||
| │ ├── DeepChat: Provider-based model selection | ||
| │ └── ACP: Session-scoped model selection | ||
| │ | ||
| ├── UnifiedModeSelector (NEW) | ||
| │ ├── DeepChat: Static permission policies | ||
| │ └── ACP: Dynamic agent-declared modes | ||
| │ | ||
| ├── WorkspaceSelector (NEW) | ||
| │ ├── DeepChat: Optional workspace (mutable) | ||
| │ └── ACP: Required workdir (immutable) | ||
| │ | ||
| └── CommandsDisplay (from acp-commands-specification.md) | ||
| └── Only shown when agent supports commands | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks.
markdownlint flags these blocks (MD040). Please add a language like text to each affected fence.
🔧 Example fix (apply similarly to other blocks)
-```
+```text
ChatInput.vue
├── AgentHeader (NEW)
...
-```
+```Also applies to: 100-116, 421-452, 474-496, 716-732, 749-774, 992-1013, 1017-1040
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
27-27: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@docs/architecture/unified-components-specification.md` around lines 27 - 46,
Several fenced code blocks in the Unified Components Specification are missing
language identifiers (markdownlint MD040); update each triple-backtick fence to
include a language (use "text") so blocks like the one starting with
"ChatInput.vue ├── AgentHeader (NEW)..." and the other affected blocks (the
blocks beginning around the sections that list component trees and examples at
the ranges referenced in your comment) use ```text instead of ```; search for
the fenced blocks that render plain diagrams or component lists and replace the
opening ``` with ```text for each occurrence.
Summary by CodeRabbit
New Features
Bug Fixes & Improvements
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.