refactor(thread): extract conversation lifecycle #1061
Conversation
WalkthroughAdds modular conversation export (markdown/html/txt), message-content utilities, a prompt-builder, content-buffer and lifecycle managers in ThreadPresenter, a GeneratingMessageState type, assistant-block finalization, message-manager enhancements, a TabPresenter accessor, and renderer updates to use the new finalizer. Changes
Sequence Diagram(s)sequenceDiagram
actor UI
participant TP as ThreadPresenter
participant CBM as ContentBufferManager
participant CLM as ConversationLifecycleManager
participant MM as MessageManager
participant MC as MessageContent
participant CE as ConversationExporter
participant SH as messageBlocks
Note over TP,CE: Export / generation flow (high-level)
UI->>TP: user: create/send/export
TP->>CLM: getActiveConversation / broadcast updates
TP->>CBM: append/flush buffer
CBM-->>TP: buffer state
TP->>MM: getContextMessages(...options)
MM->>MC: formatUserMessageContent() / getFileContext()
MC-->>MM: normalized text & file context
MM-->>TP: context messages
TP->>CE: buildConversationExportContent(conversation, messages, format)
CE->>MC: helpers for file/content formatting
CE-->>TP: export content (string)
TP->>SH: finalizeAssistantMessageBlocks(curMsg.content)
SH-->>TP: finalized blocks
TP-->>UI: emit updated state / export result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–75 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (10)**/*.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
Files:
src/{main,renderer}/**/*.ts📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
Files:
src/main/**/*.ts📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)
Files:
src/main/**/*.{ts,js,tsx,jsx}📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Files:
**/*.{ts,tsx,js,vue}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx,vue}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/main/presenter/**/*.ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,js,jsx,vue,css,scss,md,json,yml,yaml}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,js,jsx,vue}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧬 Code graph analysis (1)src/main/presenter/threadPresenter/conversationExporter.ts (2)
🔇 Additional comments (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/presenter/threadPresenter/index.ts (2)
324-343: Unify finalize logic and avoid auto‑granting permissionsThis local finalizeLastBlock duplicates logic and auto‑grants permission. Delegate to ContentBufferManager.finalizeLastBlock (which should not auto‑grant) to keep behavior consistent.
- const finalizeLastBlock = () => { - const lastBlock = - state.message.content.length > 0 - ? state.message.content[state.message.content.length - 1] - : undefined - if (lastBlock) { - if ( - lastBlock.type === 'action' && - lastBlock.action_type === 'tool_call_permission' && - lastBlock.status === 'pending' - ) { - lastBlock.status = 'granted' - return - } - // 只有当上一个块不是一个正在等待结果的工具调用时，才将其标记为成功 - if (!(lastBlock.type === 'tool_call' && lastBlock.status === 'loading')) { - lastBlock.status = 'success' - } - } - } + const finalizeLastBlock = () => this.contentBufferManager.finalizeLastBlock(state)
792-799: Bug: clearContext deletes all messages, not just this conversationdeleteAllMessages() likely clears the entire DB. Scope deletion to the provided conversationId.
async clearContext(conversationId: string): Promise<void> { await this.sqlitePresenter.runTransaction(async () => { const conversation = await this.getConversation(conversationId) if (conversation) { - await this.sqlitePresenter.deleteAllMessages() + // Limit deletion to this conversation only + await this.messageManager.clearAllMessages(conversationId) } }) }
🧹 Nitpick comments (15)
src/main/presenter/threadPresenter/promptBuilder.ts (2)
73-79: Harden MCP tool definition retrieval with try/catch and English logs.External presenter calls can throw. Add error handling and avoid failing prompt prep.
- const mcpTools = !isImageGeneration - ? await presenter.mcpPresenter.getAllToolDefinitions(enabledMcpTools) - : [] + let mcpTools: unknown[] = [] + if (!isImageGeneration) { + try { + mcpTools = await presenter.mcpPresenter.getAllToolDefinitions(enabledMcpTools) + } catch (err) { + console.error('[PromptBuilder] Failed to load MCP tool definitions', { + error: err instanceof Error ? err.message : String(err) + }) + mcpTools = [] + } + }
256-258: Use English log text and appropriate level.Replace the Chinese log with a concise English debug note.
- if (artifacts === 1) { - console.log('artifacts目前由mcp提供，此处为兼容性保留') - } + if (artifacts === 1) { + console.debug('[PromptBuilder] Artifacts are provided by MCP; keeping for compatibility') + }src/main/presenter/threadPresenter/conversationLifecycleManager.ts (5)
184-188: Typed cleanup of settings without implicit-any indexing.Indexing settings via string keys violates strict typing. Cast keys to keyof CONVERSATION_SETTINGS.
- Object.keys(settings).forEach((key) => { - if (settings[key] === undefined || settings[key] === null || settings[key] === '') { - delete settings[key] - } - }) + ;(Object.keys(settings) as Array<keyof CONVERSATION_SETTINGS>).forEach((key) => { + const v = settings[key] + if (v === undefined || v === null || v === '') { + delete settings[key] + } + })
89-91: Avoid reaching into private tabPresenter fields. Add a public accessor.Using presenter.tabPresenter['tabWindowMap'] is brittle. Please expose a typed getter on tabPresenter (e.g., getWindowIdByTabId(tabId): number | null) and use it here.
Also applies to: 257-263
152-240: Wrap persistence and presenter calls with try/catch; emit structured logs and user-friendly errors.createConversation performs multiple async calls that can fail (SQLite, presenter). Align with guidelines to not swallow errors and log with context (tabId, latestConversationId, settings deltas).
I can provide a guarded version with try/catch blocks and structured logging if you want it included in this PR.
298-307: When modelId changes, refresh all dependent defaults (not only max/context).You update maxTokens and contextLength only. Consider also temperature, thinkingBudget, enableSearch/forcedSearch/searchStrategy, reasoningEffort, etc., from modelConfig to keep settings coherent.
101-103: Prefer structured logs and a stable prefix.Replace console.log with a structured INFO log including conversationId and tab IDs to aid triage.
src/main/presenter/threadPresenter/messageContent.ts (2)
45-47: Use English logs and WARN level for non-fatal parse errors.Switch to an English warning without leaking prompt content.
- console.log('解析prompt内容失败:', e) + console.warn('[MessageContent] Failed to parse prompt JSON for @prompts mention', { + error: e instanceof Error ? e.message : String(e) + })
61-81: Large file content in prompts — confirm intent and add size guard.Embedding full file content can blow up token budgets. Consider truncation or a size cap (e.g., first N KB) with an indicator, or a configurable limit via configPresenter.
src/main/presenter/threadPresenter/contentBufferManager.ts (3)
131-146: Replaceanywith precise block typingUse a typed alias for the 'content' block to satisfy strict TS and prevent accidental shape drift.
+import type { AssistantMessageBlock } from '@shared/chat' + +type ContentBlock = Extract<AssistantMessageBlock, { type: 'content' }> - let contentBlock: any + let contentBlock: ContentBlock @@ - let contentBlock: any + let contentBlock: ContentBlockAlso applies to: 214-228
203-204: Wrap persistence in try/catch with structured loggingeditMessage can throw; per guidelines, add error handling with useful context.
- await this.messageManager.editMessage(eventId, JSON.stringify(state.message.content)) + try { + await this.messageManager.editMessage(eventId, JSON.stringify(state.message.content)) + } catch (e) { + console.error('[ContentBufferManager] Failed to edit message', { + eventId, + err: e instanceof Error ? e.message : String(e) + }) + throw e + }
34-55: Optional: guard against in‑flight async chunk processing during flushIf adaptiveBuffer.isProcessing is true, flushAdaptiveBuffer might race with processLargeContentAsynchronously. Consider waiting until processing completes or coalescing flush calls.
Would you like a small promise-based guard (poll with a short timeout) added here?
src/main/presenter/threadPresenter/index.ts (3)
626-632: Add try/catch around content processing to meet error‑handling guidelineWrap processContentDirectly to log context and prevent unhandled rejections.
- await this.contentBufferManager.processContentDirectly( - state.message.id, - content, - currentTime - ) + try { + await this.contentBufferManager.processContentDirectly( + state.message.id, + content, + currentTime + ) + } catch (e) { + console.error('[ThreadPresenter] Failed processing content', { + eventId: state.message.id, + err: e instanceof Error ? e.message : String(e) + }) + throw e + }
1700-1707: Avoid reporting totalTokens before completion; record inputTokens onlySetting totalTokens = promptTokens early can mislead metrics. Prefer updating inputTokens now and totalTokens on finalize.
- await this.messageManager.updateMessageMetadata(state.message.id, { - totalTokens: promptTokens, + await this.messageManager.updateMessageMetadata(state.message.id, { + inputTokens: promptTokens, generationTime: 0, firstTokenTime: 0, tokensPerSecond: 0 })
1845-1852: Stop flow cleanup is thoroughFlush + cleanup + search stop + status update is correct. Consider adding a DEBUG log for time spent to help triage slow cancellations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/presenter/threadPresenter/contentBufferManager.ts(1 hunks)src/main/presenter/threadPresenter/conversationExporter.ts(1 hunks)src/main/presenter/threadPresenter/conversationLifecycleManager.ts(1 hunks)src/main/presenter/threadPresenter/fileContext.ts(0 hunks)src/main/presenter/threadPresenter/index.ts(27 hunks)src/main/presenter/threadPresenter/messageContent.ts(1 hunks)src/main/presenter/threadPresenter/promptBuilder.ts(1 hunks)src/main/presenter/threadPresenter/types.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/presenter/threadPresenter/fileContext.ts
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/types.tssrc/main/presenter/threadPresenter/contentBufferManager.tssrc/main/presenter/threadPresenter/messageContent.tssrc/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/index.ts
src/{main,renderer}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
src/{main,renderer}/**/*.ts: Use context isolation for improved security
Implement proper inter-process communication (IPC) patterns
Optimize application startup time with lazy loading
Implement proper error handling and logging for debugging
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/types.tssrc/main/presenter/threadPresenter/contentBufferManager.tssrc/main/presenter/threadPresenter/messageContent.tssrc/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/index.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
Use Electron's built-in APIs for file system and native dialogs
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/types.tssrc/main/presenter/threadPresenter/contentBufferManager.tssrc/main/presenter/threadPresenter/messageContent.tssrc/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪（如适用）、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/types.tssrc/main/presenter/threadPresenter/contentBufferManager.tssrc/main/presenter/threadPresenter/messageContent.tssrc/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/index.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/types.tssrc/main/presenter/threadPresenter/contentBufferManager.tssrc/main/presenter/threadPresenter/messageContent.tssrc/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/index.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/types.tssrc/main/presenter/threadPresenter/contentBufferManager.tssrc/main/presenter/threadPresenter/messageContent.tssrc/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/index.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Use PascalCase for TypeScript types and classes
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/types.tssrc/main/presenter/threadPresenter/contentBufferManager.tssrc/main/presenter/threadPresenter/messageContent.tssrc/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/index.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Place Electron main-process presenters under src/main/presenter/ (Window, Tab, Thread, Mcp, Config, LLMProvider)
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/types.tssrc/main/presenter/threadPresenter/contentBufferManager.tssrc/main/presenter/threadPresenter/messageContent.tssrc/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/index.ts
**/*.{ts,tsx,js,jsx,vue,css,scss,md,json,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Prettier style: single quotes, no semicolons, print width 100; run pnpm run format
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/types.tssrc/main/presenter/threadPresenter/contentBufferManager.tssrc/main/presenter/threadPresenter/messageContent.tssrc/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/index.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,vue}: Use OxLint for JS/TS code; keep lint clean
Use camelCase for variables and functions
Use SCREAMING_SNAKE_CASE for constants
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/types.tssrc/main/presenter/threadPresenter/contentBufferManager.tssrc/main/presenter/threadPresenter/messageContent.tssrc/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/index.ts
🧠 Learnings (3)
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/index.ts : `src/main/presenter/llmProviderPresenter/index.ts` should manage the overall Agent loop, conversation history, tool execution via `McpPresenter`, and frontend communication via `eventBus`.
Applied to files:
src/main/presenter/threadPresenter/index.ts
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/index.ts : The main Agent loop should buffer text content, handle tool call events, format tool results for the next LLM call, and manage conversation continuation logic.
Applied to files:
src/main/presenter/threadPresenter/index.ts
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Each file in `src/main/presenter/llmProviderPresenter/providers/*.ts` should handle interaction with a specific LLM API, including request/response formatting, tool definition conversion, native/non-native tool call management, and standardizing output streams to a common event format.
Applied to files:
src/main/presenter/threadPresenter/index.ts
🧬 Code graph analysis (6)
src/main/presenter/threadPresenter/conversationExporter.ts (1)
src/main/presenter/threadPresenter/messageContent.ts (1)
getNormalizedUserMessageText(83-93)
src/main/presenter/threadPresenter/types.ts (1)
src/shared/chat.d.ts (1)
AssistantMessage(39-42)
src/main/presenter/threadPresenter/contentBufferManager.ts (3)
src/main/presenter/threadPresenter/messageManager.ts (1)
MessageManager(20-331)src/main/presenter/threadPresenter/types.ts (1)
GeneratingMessageState(3-34)src/main/eventbus.ts (1)
eventBus(151-151)
src/main/presenter/threadPresenter/promptBuilder.ts (5)
src/shared/types/presenters/legacy.presenters.d.ts (3)
ModelConfig(132-150)ChatMessage(1375-1375)ChatMessageContent(1377-1377)src/main/presenter/threadPresenter/searchManager.ts (1)
generateSearchPrompt(231-247)src/main/presenter/threadPresenter/contentEnricher.ts (1)
ContentEnricher(12-383)src/main/presenter/index.ts (1)
presenter(223-223)src/main/presenter/threadPresenter/messageContent.ts (2)
buildUserMessageContext(95-104)getNormalizedUserMessageText(83-93)
src/main/presenter/threadPresenter/conversationLifecycleManager.ts (4)
src/shared/types/presenters/legacy.presenters.d.ts (2)
ISQLitePresenter(276-329)IConfigPresenter(380-549)src/main/eventbus.ts (1)
eventBus(151-151)src/main/presenter/index.ts (1)
presenter(223-223)src/main/presenter/threadPresenter/const.ts (1)
DEFAULT_SETTINGS(112-120)
src/main/presenter/threadPresenter/index.ts (7)
src/main/presenter/threadPresenter/contentBufferManager.ts (1)
ContentBufferManager(12-261)src/main/presenter/threadPresenter/conversationLifecycleManager.ts (2)
ConversationLifecycleManager(23-379)CreateConversationOptions(13-15)src/main/presenter/threadPresenter/messageContent.ts (3)
formatUserMessageContent(14-59)buildUserMessageContext(95-104)getNormalizedUserMessageText(83-93)src/main/presenter/threadPresenter/promptBuilder.ts (2)
preparePromptContent(40-117)buildContinueToolCallContext(119-181)src/main/presenter/threadPresenter/contentEnricher.ts (1)
ContentEnricher(12-383)src/main/presenter/threadPresenter/types.ts (1)
GeneratingMessageState(3-34)src/main/presenter/threadPresenter/conversationExporter.ts (3)
ConversationExportFormat(5-5)generateExportFilename(7-19)buildConversationExportContent(21-36)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-check (x64)
🔇 Additional comments (7)
src/main/presenter/threadPresenter/promptBuilder.ts (2)
222-227: User-first context enforcement — confirm intent.Dropping leading non-user messages can remove crucial assistant/tool context. Confirm UX/design intent; consider keeping a small assistant preface if within token budget.
81-84: Token reservation includes MCP tools — confirm parity with actual API payload.You reserve tokens for mcpTools but do not add tool schemas to finalContent here. If tools are sent via a separate API field, reservation is fine; otherwise you may over-reserve.
src/main/presenter/threadPresenter/types.ts (1)
3-34: Well-typed generation state model.Interface cleanly captures timing, usage, and buffering. Suitable for main-process management.
src/main/presenter/threadPresenter/index.ts (4)
2326-2329: Export path looks goodUsing generateExportFilename + buildConversationExportContent centralizes export logic and reduces duplication.
1667-1672: Good normalization of user text vs URL enrichmentbuildUserMessageContext + getNormalizedUserMessageText use is correct and will keep prompts consistent.
244-256: Finalize metadata computation is coherentToken/time calculations and metadata update after generation are aligned with usage fields.
Ensure approximateTokenSize aligns with your current tokenizer; if migrating models, verify counts against provider telemetry.
Also applies to: 221-238
85-99: The review comment is inaccurate for the primary location specifiedThe console.log at line 90 (
ThreadPresenter: Cleaned up conversation binding for closed tab ${tabId}) is already in English and follows proper format. The secondary locations mentioned (686-697, 898-904, 2152-2154, 2207-2216, 2275-2277) do contain Chinese logs requiring conversion, but the primary snippet at lines 85-99 is already correct.Likely an incorrect or invalid review comment.
src/main/presenter/threadPresenter/conversationLifecycleManager.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/renderer/src/stores/chat.ts (1)
694-713: Bug: self-comparison always true; should compare to cached.threadIdBoth conditions use getActiveThreadId() === getActiveThreadId(), which is always true. Compare against the cached thread id instead.
- if (getActiveThreadId() === getActiveThreadId()) { + if (getActiveThreadId() === cached.threadId) { const mainMsgIndex = getMessages().findIndex((m) => m.id === mainMessage.id) if (mainMsgIndex !== -1) { getMessages()[mainMsgIndex] = enrichedMainMessage as AssistantMessage | UserMessage } } ... - if (getActiveThreadId() === getActiveThreadId()) { + if (getActiveThreadId() === cached.threadId) { const msgIndex = getMessages().findIndex((m) => m.id === msg.eventId) if (msgIndex !== -1) { getMessages()[msgIndex] = enrichedMessage as AssistantMessage | UserMessage } }src/main/presenter/threadPresenter/index.ts (1)
770-777: Danger: clearing context calls global deleteAllMessages()This likely deletes messages for all conversations. Scope deletion to the target conversation.
async clearContext(conversationId: string): Promise<void> { await this.sqlitePresenter.runTransaction(async () => { const conversation = await this.getConversation(conversationId) if (conversation) { - await this.sqlitePresenter.deleteAllMessages() + // Limit to this conversation only + await this.messageManager.clearAllMessages(conversationId) } }) }
♻️ Duplicate comments (1)
src/main/presenter/threadPresenter/contentBufferManager.ts (1)
59-61: Resolved prior auto‑grant concern via shared finalizerDelegating to finalizeAssistantMessageBlocks avoids mutating pending permission blocks. This addresses the earlier review about not auto‑granting tool permissions.
🧹 Nitpick comments (5)
src/renderer/src/stores/chat.ts (1)
21-47: Normalize comments/logs to English per repo guidelineComments are largely Chinese. The guidelines require English for logs/comments in TS/JS files. Consider normalizing gradually.
src/main/presenter/threadPresenter/contentBufferManager.ts (1)
113-127: Replaceanywith strict block typing for content blocksUse precise typing to satisfy strict TS and avoid runtime drift.
- let contentBlock: any + import type { AssistantMessageBlock } from '@shared/chat' + let contentBlock: AssistantMessageBlock & { type: 'content' }And ensure the pushed shape matches the type.
Also applies to: 196-210
src/main/presenter/threadPresenter/index.ts (3)
526-560: i18n: avoid hardcoded user-facing permission textUse an i18n key for the permission request message to keep renderer translations consistent.
- content: - typeof tool_call_response === 'string' - ? tool_call_response - : 'Permission required for this operation', + content: + typeof tool_call_response === 'string' + ? tool_call_response + : 'common.permission.required',
2566-2571: i18n: replace hardcoded denial textReturn a translatable key instead of literal.
- content.push({ + content.push({ type: 'error', - content: 'Permission denied by user', + content: 'common.error.permissionDenied', status: 'error', timestamp: Date.now() })
85-99: Standardize logs to English and structured formatLogs in this file are mixed Chinese/English and plain console.*. Consider a lightweight logger wrapper providing level, timestamp, and context. This aligns with repo logging guidance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/presenter/threadPresenter/contentBufferManager.ts(1 hunks)src/main/presenter/threadPresenter/index.ts(32 hunks)src/renderer/src/stores/chat.ts(6 hunks)src/shared/chat/messageBlocks.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (22)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
Files:
src/shared/chat/messageBlocks.tssrc/main/presenter/threadPresenter/contentBufferManager.tssrc/main/presenter/threadPresenter/index.tssrc/renderer/src/stores/chat.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪（如适用）、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别
Files:
src/shared/chat/messageBlocks.tssrc/main/presenter/threadPresenter/contentBufferManager.tssrc/main/presenter/threadPresenter/index.tssrc/renderer/src/stores/chat.ts
src/shared/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
共享类型定义放在
shared目录
Files:
src/shared/chat/messageBlocks.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/shared/chat/messageBlocks.tssrc/main/presenter/threadPresenter/contentBufferManager.tssrc/main/presenter/threadPresenter/index.tssrc/renderer/src/stores/chat.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Use PascalCase for TypeScript types and classes
Files:
src/shared/chat/messageBlocks.tssrc/main/presenter/threadPresenter/contentBufferManager.tssrc/main/presenter/threadPresenter/index.tssrc/renderer/src/stores/chat.ts
src/shared/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place shared types, utilities, constants, and IPC contract definitions under src/shared/
Files:
src/shared/chat/messageBlocks.ts
src/shared/**
📄 CodeRabbit inference engine (AGENTS.md)
Put shared TypeScript types and utilities under src/shared
Files:
src/shared/chat/messageBlocks.ts
**/*.{ts,tsx,js,jsx,vue,css,scss,md,json,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Prettier style: single quotes, no semicolons, print width 100; run pnpm run format
Files:
src/shared/chat/messageBlocks.tssrc/main/presenter/threadPresenter/contentBufferManager.tssrc/main/presenter/threadPresenter/index.tssrc/renderer/src/stores/chat.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,vue}: Use OxLint for JS/TS code; keep lint clean
Use camelCase for variables and functions
Use SCREAMING_SNAKE_CASE for constants
Files:
src/shared/chat/messageBlocks.tssrc/main/presenter/threadPresenter/contentBufferManager.tssrc/main/presenter/threadPresenter/index.tssrc/renderer/src/stores/chat.ts
src/{main,renderer}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
src/{main,renderer}/**/*.ts: Use context isolation for improved security
Implement proper inter-process communication (IPC) patterns
Optimize application startup time with lazy loading
Implement proper error handling and logging for debugging
Files:
src/main/presenter/threadPresenter/contentBufferManager.tssrc/main/presenter/threadPresenter/index.tssrc/renderer/src/stores/chat.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
Use Electron's built-in APIs for file system and native dialogs
Files:
src/main/presenter/threadPresenter/contentBufferManager.tssrc/main/presenter/threadPresenter/index.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/presenter/threadPresenter/contentBufferManager.tssrc/main/presenter/threadPresenter/index.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Place Electron main-process presenters under src/main/presenter/ (Window, Tab, Thread, Mcp, Config, LLMProvider)
Files:
src/main/presenter/threadPresenter/contentBufferManager.tssrc/main/presenter/threadPresenter/index.ts
src/renderer/src/**/*
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
src/renderer/src/**/*: All user-facing strings must use i18n keys (avoid hardcoded user-visible text in code)
Use the 'vue-i18n' framework for all internationalization in the renderer
Ensure all user-visible text in the renderer uses the translation system
Files:
src/renderer/src/stores/chat.ts
src/renderer/src/stores/**/*.{vue,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/pinia-best-practices.mdc)
src/renderer/src/stores/**/*.{vue,ts,tsx,js,jsx}: Use modules to organize related state and actions
Implement proper state persistence for maintaining data across sessions
Use getters for computed state properties
Utilize actions for side effects and asynchronous operations
Keep the store focused on global state, not component-specific data
Files:
src/renderer/src/stores/chat.ts
src/renderer/**/*.{vue,ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
渲染进程代码放在
src/renderer
Files:
src/renderer/src/stores/chat.ts
src/renderer/src/**/*.{vue,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/vue-best-practices.mdc)
src/renderer/src/**/*.{vue,ts,tsx,js,jsx}: Use the Composition API for better code organization and reusability
Implement proper state management with Pinia
Utilize Vue Router for navigation and route management
Leverage Vue's built-in reactivity system for efficient data handling
Files:
src/renderer/src/stores/chat.ts
src/renderer/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,tsx,vue}: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Use TypeScript for all code; prefer types over interfaces.
Avoid enums; use const objects instead.
Use arrow functions for methods and computed properties.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Files:
src/renderer/src/stores/chat.ts
src/renderer/**/*.{vue,ts}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
Implement lazy loading for routes and components.
Files:
src/renderer/src/stores/chat.ts
src/renderer/**/*.{ts,vue}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,vue}: Use useFetch and useAsyncData for data fetching.
Implement SEO best practices using Nuxt's useHead and useSeoMeta.Use Pinia for frontend state management (do not introduce alternative state libraries)
Files:
src/renderer/src/stores/chat.ts
src/renderer/src/**
📄 CodeRabbit inference engine (AGENTS.md)
Place Vue 3 app source under src/renderer/src (components, stores, views, i18n, lib)
Files:
src/renderer/src/stores/chat.ts
src/renderer/src/**/*.{vue,ts}
📄 CodeRabbit inference engine (AGENTS.md)
All user-facing strings must use vue-i18n ($t/keys) rather than hardcoded literals
Files:
src/renderer/src/stores/chat.ts
🧠 Learnings (3)
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/index.ts : `src/main/presenter/llmProviderPresenter/index.ts` should manage the overall Agent loop, conversation history, tool execution via `McpPresenter`, and frontend communication via `eventBus`.
Applied to files:
src/main/presenter/threadPresenter/index.ts
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/index.ts : The main Agent loop should buffer text content, handle tool call events, format tool results for the next LLM call, and manage conversation continuation logic.
Applied to files:
src/main/presenter/threadPresenter/index.ts
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Each file in `src/main/presenter/llmProviderPresenter/providers/*.ts` should handle interaction with a specific LLM API, including request/response formatting, tool definition conversion, native/non-native tool call management, and standardizing output streams to a common event format.
Applied to files:
src/main/presenter/threadPresenter/index.ts
🧬 Code graph analysis (3)
src/main/presenter/threadPresenter/contentBufferManager.ts (4)
src/main/presenter/threadPresenter/messageManager.ts (1)
MessageManager(20-331)src/main/presenter/threadPresenter/types.ts (1)
GeneratingMessageState(3-34)src/shared/chat/messageBlocks.ts (1)
finalizeAssistantMessageBlocks(3-25)src/main/eventbus.ts (1)
eventBus(151-151)
src/main/presenter/threadPresenter/index.ts (7)
src/main/presenter/threadPresenter/contentBufferManager.ts (1)
ContentBufferManager(14-243)src/main/presenter/threadPresenter/conversationLifecycleManager.ts (2)
ConversationLifecycleManager(23-379)CreateConversationOptions(13-15)src/main/presenter/threadPresenter/messageContent.ts (3)
formatUserMessageContent(14-59)buildUserMessageContext(95-104)getNormalizedUserMessageText(83-93)src/main/presenter/threadPresenter/promptBuilder.ts (2)
preparePromptContent(40-117)buildContinueToolCallContext(119-181)src/main/presenter/threadPresenter/contentEnricher.ts (1)
ContentEnricher(12-383)src/main/presenter/threadPresenter/types.ts (1)
GeneratingMessageState(3-34)src/main/presenter/threadPresenter/conversationExporter.ts (3)
ConversationExportFormat(5-5)generateExportFilename(7-19)buildConversationExportContent(21-36)
src/renderer/src/stores/chat.ts (1)
src/shared/chat/messageBlocks.ts (1)
finalizeAssistantMessageBlocks(3-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-check (x64)
🔇 Additional comments (2)
src/shared/chat/messageBlocks.ts (1)
3-25: Centralized, safe finalization logic LGTMCovers empty/falsy, preserves tool_call_permission and in‑flight tool_call, and finalizes generic loading blocks. Good reuse point across main/renderer.
src/renderer/src/stores/chat.ts (1)
434-457: Good use of shared finalizer before appending new blocksFinalizing the trailing block before pushing tool/image/rate‑limit blocks prevents mixed statuses and UI glitches. Consistent across paths.
Also applies to: 511-513, 555-556, 569-571
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (5)
src/main/presenter/threadPresenter/messageContent.ts (1)
71-77: Good fix: optional chaining on metadata/mimeType and no absolute paths.src/main/presenter/threadPresenter/promptBuilder.ts (3)
125-134: LGTM: fixed image token double-counting.Image tokens are now added only for the final user message with image blocks.
168-187: LGTM: corrected permission acknowledgment role.Permission message now uses
role: 'user', keeping tool-call semantics correct.
202-239: Non-mutating selection of context looks good.The function avoids mutating historical messages during selection. Thanks for addressing prior feedback.
src/main/presenter/threadPresenter/conversationLifecycleManager.ts (1)
206-221: LGTM: explicit checks for numeric settings (0 honored).Thanks for fixing truthy checks for
artifacts,maxTokens, andcontextLength.
🧹 Nitpick comments (2)
src/main/presenter/threadPresenter/messageContent.ts (1)
19-58: Consider separators between blocks for readability.Joining with
''can mash tokens;'\n'(or a space) improves readability without changing meaning.- return msgContentBlock + return msgContentBlock .map((block) => { ... }) - .join('') + .join('\n')src/main/presenter/threadPresenter/conversationLifecycleManager.ts (1)
89-91: Avoid accessing private properties via bracket notation.
presenter.tabPresenter['tabWindowMap']breaks encapsulation and can regress silently. Expose a public getter intabPresenter(e.g.,getWindowIdForTab(tabId)).Also applies to: 257-263
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/presenter/threadPresenter/contentBufferManager.ts(1 hunks)src/main/presenter/threadPresenter/conversationExporter.ts(1 hunks)src/main/presenter/threadPresenter/conversationLifecycleManager.ts(1 hunks)src/main/presenter/threadPresenter/messageContent.ts(1 hunks)src/main/presenter/threadPresenter/promptBuilder.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/presenter/threadPresenter/conversationExporter.ts
- src/main/presenter/threadPresenter/contentBufferManager.ts
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
Files:
src/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.ts
src/{main,renderer}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
src/{main,renderer}/**/*.ts: Use context isolation for improved security
Implement proper inter-process communication (IPC) patterns
Optimize application startup time with lazy loading
Implement proper error handling and logging for debugging
Files:
src/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
Use Electron's built-in APIs for file system and native dialogs
Files:
src/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪（如适用）、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别
Files:
src/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Use PascalCase for TypeScript types and classes
Files:
src/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Place Electron main-process presenters under src/main/presenter/ (Window, Tab, Thread, Mcp, Config, LLMProvider)
Files:
src/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.ts
**/*.{ts,tsx,js,jsx,vue,css,scss,md,json,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Prettier style: single quotes, no semicolons, print width 100; run pnpm run format
Files:
src/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,vue}: Use OxLint for JS/TS code; keep lint clean
Use camelCase for variables and functions
Use SCREAMING_SNAKE_CASE for constants
Files:
src/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.ts
🧬 Code graph analysis (2)
src/main/presenter/threadPresenter/promptBuilder.ts (5)
src/shared/types/presenters/legacy.presenters.d.ts (1)
ModelConfig(132-150)src/main/presenter/threadPresenter/searchManager.ts (1)
generateSearchPrompt(231-247)src/main/presenter/threadPresenter/contentEnricher.ts (1)
ContentEnricher(12-383)src/main/presenter/index.ts (1)
presenter(223-223)src/main/presenter/threadPresenter/messageContent.ts (1)
buildUserMessageContext(96-105)
src/main/presenter/threadPresenter/conversationLifecycleManager.ts (4)
src/shared/types/presenters/legacy.presenters.d.ts (2)
ISQLitePresenter(276-329)IConfigPresenter(380-549)src/main/eventbus.ts (1)
eventBus(151-151)src/main/presenter/index.ts (1)
presenter(223-223)src/main/presenter/threadPresenter/const.ts (1)
DEFAULT_SETTINGS(112-120)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-check (x64)
src/main/presenter/threadPresenter/conversationLifecycleManager.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
src/main/presenter/threadPresenter/conversationLifecycleManager.ts (1)
257-282: Consider exposingtabWindowMapvia a public TabPresenter API.Line 272 accesses a private property using bracket notation (same issue as Line 89).
See earlier comment on lines 83-95 for the suggested refactor.
src/main/presenter/threadPresenter/promptBuilder.ts (1)
168-187: Correct role for permission acknowledgement — good fixUsing a user-role message for permission acknowledgement keeps tool-call semantics correct.
🧹 Nitpick comments (8)
src/main/presenter/threadPresenter/conversationLifecycleManager.ts (3)
83-95: Consider exposingtabWindowMapvia a public TabPresenter API.Line 89 accesses a private property using bracket notation, which bypasses TypeScript's access control and is brittle if the internal property name changes.
If feasible, add a public method to
TabPresentersuch as:getTabWindowId(tabId: number): number | undefined { return this.tabWindowMap.get(tabId) }Then refactor this line to:
const windowId = presenter.tabPresenter.getTabWindowId(tabId)
257-288: Add try-catch error handling for async database and event bus operations.Unlike
createConversation, these methods lack error handling. Database and IPC calls can fail; catching and logging errors aids debugging and prevents silent failures.Wrap the method bodies in try-catch blocks similar to
createConversation:async renameConversation(conversationId: string, title: string): Promise<CONVERSATION> { try { await this.sqlitePresenter.renameConversation(conversationId, title) // ... rest of method } catch (error) { console.error('Failed to rename conversation:', { conversationId, title, error: error instanceof Error ? error.message : String(error) }) throw error } }Apply similar guards to
deleteConversation,toggleConversationPinned,updateConversationTitle, andupdateConversationSettings.
300-326: Improve type safety in settings iteration.Lines 307-311 iterate over
settingsusing a plainstringkey, which bypasses TypeScript's type checking forCONVERSATION_SETTINGS.Use a type-safe approach:
- for (const key in settings) { - if (settings[key] !== undefined) { - mergedSettings[key] = settings[key] - } - } + for (const key in settings) { + const typedKey = key as keyof CONVERSATION_SETTINGS + if (settings[typedKey] !== undefined) { + ;(mergedSettings as any)[typedKey] = settings[typedKey] + } + }Or use
Object.assignfor a cleaner approach:Object.assign(mergedSettings, settings)src/main/presenter/threadPresenter/messageContent.ts (2)
79-81: Use the project logger with structured fields instead of console.warnReplace console.* with your logger (level=warn) and include context (e.g., block.id). This aligns with the logging guidelines (timestamps, level, context).
As per coding guidelines.
9-11: Make truncation cap configurableExpose FILE_CONTENT_MAX_CHARS via settings/config so ops can tune token usage without code changes; keep FILE_CONTENT_TRUNCATION_SUFFIX.
As per coding guidelines.
Also applies to: 42-48
src/main/presenter/threadPresenter/promptBuilder.ts (3)
221-225: Improve token accounting for assistant/system historyJSON.stringify overcounts/undercounts. Prefer text-only extraction similar to the main loop when content is string/array; fall back to stringify only when needed.
- const msgTokens = approximateTokenSize( - msgContent ? getNormalizedUserMessageText(msgContent) : JSON.stringify(msg.content) - ) + let msgTokens = 0 + if (msgContent) { + msgTokens = approximateTokenSize(getNormalizedUserMessageText(msgContent)) + } else if (typeof (msg as any).content === 'string' || (msg as any).content === undefined) { + msgTokens = approximateTokenSize(((msg as any).content as string) || '') + } else if (Array.isArray((msg as any).content)) { + const txt = + ((msg as any).content as ChatMessageContent[]) + .filter((b) => b.type === 'text' && typeof (b as any).text === 'string') + .map((b: any) => b.text) + .join('') || '' + msgTokens = approximateTokenSize(txt) + } else { + msgTokens = approximateTokenSize(JSON.stringify((msg as any).content ?? '')) + }As per coding guidelines.
268-270: Replace console.debug with structured loggerUse your app logger (level=debug) with context instead of console.debug to meet logging standards.
As per coding guidelines.
417-427: Avoid repeated JSON deep clones in mergeConsecutiveMessagesJSON.parse/stringify on every iteration is costly. Consider shallow copies and immutable updates, or use structuredClone when needed. This path runs per prompt; optimizing reduces latency.
As per coding guidelines.
Also applies to: 441-493
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/presenter/threadPresenter/conversationLifecycleManager.ts(1 hunks)src/main/presenter/threadPresenter/messageContent.ts(1 hunks)src/main/presenter/threadPresenter/promptBuilder.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
Files:
src/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.tssrc/main/presenter/threadPresenter/promptBuilder.ts
src/{main,renderer}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
src/{main,renderer}/**/*.ts: Use context isolation for improved security
Implement proper inter-process communication (IPC) patterns
Optimize application startup time with lazy loading
Implement proper error handling and logging for debugging
Files:
src/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.tssrc/main/presenter/threadPresenter/promptBuilder.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
Use Electron's built-in APIs for file system and native dialogs
Files:
src/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.tssrc/main/presenter/threadPresenter/promptBuilder.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪（如适用）、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别
Files:
src/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.tssrc/main/presenter/threadPresenter/promptBuilder.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.tssrc/main/presenter/threadPresenter/promptBuilder.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.tssrc/main/presenter/threadPresenter/promptBuilder.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Use PascalCase for TypeScript types and classes
Files:
src/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.tssrc/main/presenter/threadPresenter/promptBuilder.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Place Electron main-process presenters under src/main/presenter/ (Window, Tab, Thread, Mcp, Config, LLMProvider)
Files:
src/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.tssrc/main/presenter/threadPresenter/promptBuilder.ts
**/*.{ts,tsx,js,jsx,vue,css,scss,md,json,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Prettier style: single quotes, no semicolons, print width 100; run pnpm run format
Files:
src/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.tssrc/main/presenter/threadPresenter/promptBuilder.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,vue}: Use OxLint for JS/TS code; keep lint clean
Use camelCase for variables and functions
Use SCREAMING_SNAKE_CASE for constants
Files:
src/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.tssrc/main/presenter/threadPresenter/promptBuilder.ts
🧬 Code graph analysis (2)
src/main/presenter/threadPresenter/conversationLifecycleManager.ts (4)
src/shared/types/presenters/legacy.presenters.d.ts (2)
ISQLitePresenter(276-329)IConfigPresenter(380-549)src/main/eventbus.ts (1)
eventBus(151-151)src/main/presenter/index.ts (1)
presenter(223-223)src/main/presenter/threadPresenter/const.ts (1)
DEFAULT_SETTINGS(112-120)
src/main/presenter/threadPresenter/promptBuilder.ts (4)
src/shared/types/presenters/legacy.presenters.d.ts (3)
ModelConfig(132-150)ChatMessage(1375-1375)ChatMessageContent(1377-1377)src/main/presenter/threadPresenter/searchManager.ts (1)
generateSearchPrompt(231-247)src/main/presenter/index.ts (1)
presenter(223-223)src/main/presenter/threadPresenter/messageContent.ts (2)
buildUserMessageContext(132-141)getNormalizedUserMessageText(120-130)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-check (x64)
🔇 Additional comments (12)
src/main/presenter/threadPresenter/conversationLifecycleManager.ts (8)
1-22: LGTM!The imports and interface definitions are clean, well-organized, and properly typed.
23-38: LGTM!The class structure and constructor follow good dependency injection patterns with appropriate encapsulation.
40-58: LGTM!The active conversation management methods are straightforward and correctly implemented with appropriate optional notification.
60-81: LGTM!Good defensive programming with the
webContents.isDestroyed()check to avoid accessing destroyed web contents.
97-138: LGTM!The logic correctly handles the complex scenarios of cross-tab and cross-window conversation activation, including proper validation and event broadcasting.
140-150: LGTM!These getter methods are straightforward and correctly implemented.
328-388: LGTM with a note on error handling.The pagination and thread grouping logic (pinned first, then grouped by date) is well-implemented and correctly sorts conversations.
Consider adding error handling similar to other async methods, though the impact is lower for read operations.
390-393: LGTM!Simple helper method that correctly retrieves the latest conversation.
src/main/presenter/threadPresenter/messageContent.ts (2)
60-93: Verify block joining won’t run tokens togetherYou join blocks with ''. Mentions and code fences can glue to adjacent text (e.g.,
@toolDo X). Consider inserting minimal separators where needed, or ensure upstream content includes spacing.Do you want a small patch that adds a trailing space after mention/code blocks only when the next block is text?
95-118: LGTM: guarded fields and capped non-image contentOptional chaining for metadata/mimeType and truncation via truncateFileContent look good; path leakage removed. Nice hardening.
src/main/presenter/threadPresenter/promptBuilder.ts (2)
288-292: Confirm image URL/source formatEnsure MessageFile.content is always a safe string URL or data URL. If not guaranteed, add a runtime guard to avoid passing non-strings to image_url.url.
Would you like a small guard that filters out non-string images and logs a warning?
263-266: Confirm intention: searchPrompt replaces userContentWhen searchPrompt is present, userContent is ignored (then enriched content is appended). Verify this is desired UX; otherwise, concatenate both with clear delineation.
I can propose a patch to include both with separators if needed.
src/main/presenter/threadPresenter/conversationLifecycleManager.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/main/presenter/threadPresenter/promptBuilder.ts (1)
427-506: Consider alternatives to JSON deep cloning for better performance.The function uses
JSON.parse(JSON.stringify(...))to deep clone messages. While this works, it can be inefficient for large message arrays and strips non-JSON-serializable properties (functions, undefined values in arrays, etc.). Consider using a structured clone approach or a shallow copy if deep cloning isn't strictly necessary.Example alternative:
// If full deep clone is needed: const currentMessage = structuredClone(messages[i]) as ChatMessage const lastPushedMessage = mergedResult[mergedResult.length - 1] // Or if shallow copy of message with content clone is sufficient: const currentMessage = { ...messages[i], content: Array.isArray(messages[i].content) ? [...messages[i].content] : messages[i].content }src/main/presenter/threadPresenter/conversationLifecycleManager.ts (1)
100-103: Replace console.log with structured logger at appropriate level.Use a structured logger (e.g.,
logger.debugorlogger.info) instead ofconsole.logto align with logging guidelines. Include relevant context in a structured format.As per coding guidelines.
- console.log( - `Conversation ${conversationId} is already open in tab ${existingTabId}. Switching to it.` - ) + // TODO: Replace with structured logger + console.debug('Conversation already open, switching tab', { + conversationId, + existingTabId, + requestedTabId: tabId + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/presenter/threadPresenter/conversationLifecycleManager.ts(1 hunks)src/main/presenter/threadPresenter/messageContent.ts(1 hunks)src/main/presenter/threadPresenter/promptBuilder.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
Files:
src/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.ts
src/{main,renderer}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
src/{main,renderer}/**/*.ts: Use context isolation for improved security
Implement proper inter-process communication (IPC) patterns
Optimize application startup time with lazy loading
Implement proper error handling and logging for debugging
Files:
src/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
Use Electron's built-in APIs for file system and native dialogs
Files:
src/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪（如适用）、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别
Files:
src/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Use PascalCase for TypeScript types and classes
Files:
src/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Place Electron main-process presenters under src/main/presenter/ (Window, Tab, Thread, Mcp, Config, LLMProvider)
Files:
src/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.ts
**/*.{ts,tsx,js,jsx,vue,css,scss,md,json,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Prettier style: single quotes, no semicolons, print width 100; run pnpm run format
Files:
src/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,vue}: Use OxLint for JS/TS code; keep lint clean
Use camelCase for variables and functions
Use SCREAMING_SNAKE_CASE for constants
Files:
src/main/presenter/threadPresenter/promptBuilder.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/threadPresenter/messageContent.ts
🧠 Learnings (1)
📚 Learning: 2025-09-06T03:07:23.817Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T03:07:23.817Z
Learning: Applies to src/main/presenter/mcpPresenter/index.ts : Register new MCP tools in src/main/presenter/mcpPresenter/index.ts
Applied to files:
src/main/presenter/threadPresenter/promptBuilder.ts
🧬 Code graph analysis (2)
src/main/presenter/threadPresenter/promptBuilder.ts (5)
src/shared/types/presenters/legacy.presenters.d.ts (3)
ModelConfig(132-150)ChatMessage(1375-1375)ChatMessageContent(1377-1377)src/main/presenter/threadPresenter/searchManager.ts (1)
generateSearchPrompt(231-247)src/main/presenter/threadPresenter/contentEnricher.ts (1)
ContentEnricher(12-383)src/main/presenter/index.ts (1)
presenter(223-223)src/main/presenter/threadPresenter/messageContent.ts (2)
buildUserMessageContext(155-164)getNormalizedUserMessageText(143-153)
src/main/presenter/threadPresenter/conversationLifecycleManager.ts (4)
src/shared/types/presenters/legacy.presenters.d.ts (2)
ISQLitePresenter(276-329)IConfigPresenter(380-549)src/main/eventbus.ts (1)
eventBus(151-151)src/main/presenter/index.ts (1)
presenter(223-223)src/main/presenter/threadPresenter/const.ts (1)
DEFAULT_SETTINGS(112-120)
🪛 Biome (2.1.2)
src/main/presenter/threadPresenter/messageContent.ts
[error] 51-51: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 51-51: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-check (x64)
🔇 Additional comments (12)
src/main/presenter/threadPresenter/promptBuilder.ts (4)
76-84: LGTM: MCP tool fetch properly guarded with error handling.The try-catch block correctly handles potential failures when loading MCP tool definitions, logs a warning, and gracefully falls back to an empty array, ensuring prompt building continues even if MCP tools are unavailable.
149-210: LGTM: Tool call continuation context correctly structured.The function properly handles both
functionCallmodes, using the correct message roles (assistant with tool_calls, then user for permission acknowledgment) and building appropriate context from user messages.
212-249: LGTM: Context selection properly uses normalized text for token accounting.The function correctly uses
getNormalizedUserMessageTextto compute token size for historical user messages, avoiding file content inflation, and ensures the selected context starts with a user message.
307-425: LGTM: Context messages correctly use normalized text to avoid file content bloat.Both branches (with and without function call support) properly use
getNormalizedUserMessageTextfor historical user messages, preventing large file bodies from being re-embedded in context and keeping token counts accurate.src/main/presenter/threadPresenter/messageContent.ts (4)
50-69: Static analysis warning is a false positive; control character handling is intentional.The Biome linter flags control characters in the regex on line 51. However, this is intentional XML sanitization: the function escapes common entities (
&,<,>) and explicitly preserves whitespace control characters (\n,\r,\t) while stripping other control characters (0x00-0x08, 0x0B-0x0C, 0x0E-0x1F). This is appropriate for preventing malformed XML and injection issues.
76-116: LGTM: User message content formatting is robust and properly escaped.The function correctly handles various block types (text, code, mentions by category), applies XML escaping to prevent injection issues, includes defensive array checks, and logs parse failures in English with appropriate level.
118-141: LGTM: File context generation is safe and privacy-conscious.The function properly guards against undefined fields, truncates large file content to prevent token bloat, omits image content, and avoids leaking absolute file paths. All past security and privacy concerns have been addressed.
143-164: LGTM: Message text utilities provide clear separation for history vs. current context.
getNormalizedUserMessageTextreturns only text content (used for token accounting in message history), whilebuildUserMessageContextincludes file attachments (used for the current user turn). This separation correctly prevents file content bloat in historical context.src/main/presenter/threadPresenter/conversationLifecycleManager.ts (4)
23-38: LGTM: Clean dependency injection pattern.The constructor uses dependency injection for presenters and message manager, making the class testable and decoupled from global state. State initialization is straightforward with no side effects.
152-250: LGTM: Settings merging follows clear precedence and past issues resolved.The function correctly:
- Sanitizes a copy of input settings without mutating the original (line 185)
- Uses explicit
!== undefinedchecks for numeric settings that can be 0 (lines 204, 207, 213)- Applies settings in clear precedence: conversation defaults → model defaults → user-provided settings (line 219)
- Includes comprehensive error logging with structured context (lines 240-247)
Past refactor suggestions have been addressed.
295-321: LGTM: Conversation settings update correctly handles model changes.The method properly merges settings and, when the model changes, loads the new model's default config to update
maxTokensandcontextLength. This ensures consistency between model selection and token limits.
342-383: LGTM: Thread list grouping and broadcasting logic is well-structured.The method correctly:
- Separates pinned conversations
- Sorts both groups by
updatedAtdescending- Groups normal conversations by date
- Broadcasts a structured list to all windows
The logic is clear and handles edge cases (empty lists, no pinned conversations).
src/main/presenter/threadPresenter/conversationLifecycleManager.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/presenter/threadPresenter/conversationLifecycleManager.ts (1)
370-370: Consider more defensive array access.The non-null assertion
groupedThreads.get(date)!is safe because lines 367-369 ensure the key exists, but could be more defensive.- groupedThreads.get(date)!.push(conv) + const dateThreads = groupedThreads.get(date) + if (dateThreads) { + dateThreads.push(conv) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/presenter/tabPresenter.ts(1 hunks)src/main/presenter/threadPresenter/conversationLifecycleManager.ts(1 hunks)src/shared/types/presenters/legacy.presenters.d.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
Files:
src/shared/types/presenters/legacy.presenters.d.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/tabPresenter.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪（如适用）、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别
Files:
src/shared/types/presenters/legacy.presenters.d.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/tabPresenter.ts
src/shared/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
共享类型定义放在
shared目录
Files:
src/shared/types/presenters/legacy.presenters.d.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/shared/types/presenters/legacy.presenters.d.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/tabPresenter.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Use PascalCase for TypeScript types and classes
Files:
src/shared/types/presenters/legacy.presenters.d.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/tabPresenter.ts
src/shared/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place shared types, utilities, constants, and IPC contract definitions under src/shared/
Files:
src/shared/types/presenters/legacy.presenters.d.ts
src/shared/**
📄 CodeRabbit inference engine (AGENTS.md)
Put shared TypeScript types and utilities under src/shared
Files:
src/shared/types/presenters/legacy.presenters.d.ts
**/*.{ts,tsx,js,jsx,vue,css,scss,md,json,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Prettier style: single quotes, no semicolons, print width 100; run pnpm run format
Files:
src/shared/types/presenters/legacy.presenters.d.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/tabPresenter.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,vue}: Use OxLint for JS/TS code; keep lint clean
Use camelCase for variables and functions
Use SCREAMING_SNAKE_CASE for constants
Files:
src/shared/types/presenters/legacy.presenters.d.tssrc/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/tabPresenter.ts
src/{main,renderer}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
src/{main,renderer}/**/*.ts: Use context isolation for improved security
Implement proper inter-process communication (IPC) patterns
Optimize application startup time with lazy loading
Implement proper error handling and logging for debugging
Files:
src/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/tabPresenter.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
Use Electron's built-in APIs for file system and native dialogs
Files:
src/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/tabPresenter.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/tabPresenter.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Place Electron main-process presenters under src/main/presenter/ (Window, Tab, Thread, Mcp, Config, LLMProvider)
Files:
src/main/presenter/threadPresenter/conversationLifecycleManager.tssrc/main/presenter/tabPresenter.ts
🧬 Code graph analysis (1)
src/main/presenter/threadPresenter/conversationLifecycleManager.ts (4)
src/shared/types/presenters/legacy.presenters.d.ts (5)
ISQLitePresenter(277-330)IConfigPresenter(381-550)IMessageManager(884-917)CONVERSATION(746-755)CONVERSATION_SETTINGS(728-744)src/main/eventbus.ts (1)
eventBus(151-151)src/main/presenter/index.ts (1)
presenter(223-223)src/main/presenter/threadPresenter/const.ts (1)
DEFAULT_SETTINGS(112-120)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-check (x64)
🔇 Additional comments (6)
src/shared/types/presenters/legacy.presenters.d.ts (1)
230-230: LGTM! Clean interface addition.The method signature is well-typed and consistent with similar accessor methods in the interface. The return type correctly handles the case where a tab might not be found.
src/main/presenter/tabPresenter.ts (1)
487-489: LGTM! Simple and correct implementation.The method correctly exposes the tab-to-window mapping with appropriate handling of missing tabs. The implementation is consistent with similar accessor methods in the class.
src/main/presenter/threadPresenter/conversationLifecycleManager.ts (4)
1-21: LGTM! Clean interface definitions.The imports and interface definitions follow TypeScript best practices with clear dependency injection patterns.
185-223: Settings merging logic is clear and correct.The precedence order is well-established: default settings → model defaults → user settings. The sanitization properly filters out undefined/null/empty values with correct typing, and the temperature fallback (lines 221-223) provides defensive programming for edge cases.
83-95: Good use of public API for tab window access.The method now correctly uses
presenter.tabPresenter.getTabWindowId(tabId)as a public API, addressing previous concerns about accessing private fields via bracket notation.
152-250: Excellent error handling in conversation creation.The try-catch block with detailed structured logging (lines 240-247) follows best practices, including relevant context (title, tabId, options, error details) without exposing sensitive information.
| for (const key in settings) { | ||
| if (settings[key] !== undefined) { | ||
| mergedSettings[key] = settings[key] | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix type safety issue in settings iteration.
The for...in loop accessing settings[key] and mergedSettings[key] with a string key will cause TypeScript errors because CONVERSATION_SETTINGS doesn't have an index signature.
Apply this diff to properly type the iteration:
- for (const key in settings) {
- if (settings[key] !== undefined) {
- mergedSettings[key] = settings[key]
+ for (const key in settings) {
+ const typedKey = key as keyof CONVERSATION_SETTINGS
+ if (settings[typedKey] !== undefined) {
+ mergedSettings[typedKey] = settings[typedKey]
}
}📝 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.
| for (const key in settings) { | |
| if (settings[key] !== undefined) { | |
| mergedSettings[key] = settings[key] | |
| } | |
| } | |
| for (const key in settings) { | |
| const typedKey = key as keyof CONVERSATION_SETTINGS | |
| if (settings[typedKey] !== undefined) { | |
| mergedSettings[typedKey] = settings[typedKey] | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/presenter/threadPresenter/conversationLifecycleManager.ts around
lines 302-306, the for...in loop uses a string key which breaks TypeScript since
CONVERSATION_SETTINGS lacks an index signature; replace the loop with iterating
Object.keys(settings) and assert the key as keyof CONVERSATION_SETTINGS (e.g.
const keys = Object.keys(settings) as (keyof CONVERSATION_SETTINGS)[]; for
(const key of keys) { if (settings[key] !== undefined) mergedSettings[key] =
settings[key]; }) so both settings[key] and mergedSettings[key] are correctly
typed.
| if (settings.modelId && settings.modelId !== conversation.settings.modelId) { | ||
| const modelConfig = this.configPresenter.getModelConfig( | ||
| mergedSettings.modelId, | ||
| mergedSettings.providerId | ||
| ) | ||
| if (modelConfig) { | ||
| mergedSettings.maxTokens = modelConfig.maxTokens | ||
| mergedSettings.contextLength = modelConfig.contextLength | ||
| } | ||
| } |
There was a problem hiding this comment.
Check both modelId and providerId changes when updating model config.
The condition only checks if modelId changed, but if providerId changes while modelId stays the same, the model config should still be updated. Different providers may have different maxTokens/contextLength settings for the same model ID.
Apply this diff:
- if (settings.modelId && settings.modelId !== conversation.settings.modelId) {
+ if (
+ (settings.modelId && settings.modelId !== conversation.settings.modelId) ||
+ (settings.providerId && settings.providerId !== conversation.settings.providerId)
+ ) {
const modelConfig = this.configPresenter.getModelConfig(
mergedSettings.modelId,
mergedSettings.providerId
)
if (modelConfig) {
mergedSettings.maxTokens = modelConfig.maxTokens
mergedSettings.contextLength = modelConfig.contextLength
}
}🤖 Prompt for AI Agents
In src/main/presenter/threadPresenter/conversationLifecycleManager.ts around
lines 308 to 317, the code only updates model config when modelId changed;
modify the condition to trigger when either modelId or providerId changed (i.e.,
if mergedSettings.modelId !== conversation.settings.modelId ||
mergedSettings.providerId !== conversation.settings.providerId), then call
this.configPresenter.getModelConfig with mergedSettings.modelId and
mergedSettings.providerId and apply modelConfig.maxTokens and
modelConfig.contextLength to mergedSettings as before.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
src/main/presenter/threadPresenter/messageManager.ts (1)
231-236: Type the options param and avoid mutating nested content; guard with Array.isArray.Prevents implicit-any drift, side effects on shared objects, and runtime on unexpected shapes.
Apply:
- async getContextMessages( - conversationId: string, - messageCount: number, - { ensureUserStart = true, normalizeUserText = true } = {} - ): Promise<Message[]> { + async getContextMessages( + conversationId: string, + messageCount: number, + options: { ensureUserStart?: boolean; normalizeUserText?: boolean } = {} + ): Promise<Message[]> { + const { ensureUserStart = true, normalizeUserText = true } = options @@ - if (normalizeUserText) { - return messages.map((msg) => { - if (msg.role !== 'user') { - return msg - } - const normalized = { ...msg } - const userContent = normalized.content as UserMessageContent - if (userContent?.content) { - ;(normalized.content as UserMessageContent).text = formatUserMessageContent( - userContent.content - ) - } - return normalized - }) - } + if (normalizeUserText) { + return messages.map((msg) => { + if (msg.role !== 'user') return msg + const userContent = msg.content as UserMessageContent + if (Array.isArray(userContent?.content)) { + return { + ...msg, + content: { + ...userContent, + text: formatUserMessageContent(userContent.content) + } + } + } + return msg + }) + }Also applies to: 258-271
src/main/presenter/threadPresenter/llmGenerationManager.ts (2)
164-205: Logs must be English; batch DB writes; simplify favicon handling.
- Replace Chinese logs with English per guidelines.
- Use Promise.all for attachments to reduce latency.
- Drop nonexistent favicon field in pages mapping for correctness.
- console.error('解析搜索结果失败:', e) + console.error('[LLMGeneration] Failed to parse search result blob', { + eventId, + err: e + }) @@ - const pages = searchResults - .filter((item) => item && (item.icon || item.favicon)) + const pages = searchResults + .filter((item) => item && item.icon) .slice(0, 6) .map((item) => ({ - url: item?.url ?? '', - icon: item?.icon || item?.favicon || '' + url: item?.url ?? '', + icon: item?.icon || '' })) @@ - for (const result of searchResults) { - await this.sqlitePresenter.addMessageAttachment( - eventId, - 'search_result', - JSON.stringify({ - title: result?.title || '', - url: result?.url || '', - content: result?.content || '', - description: result?.description || '', - icon: result?.icon || result?.favicon || '', - rank: typeof result?.rank === 'number' ? result.rank : undefined, - searchId - }) - ) - } + await Promise.all( + searchResults.map((result) => + this.sqlitePresenter.addMessageAttachment( + eventId, + 'search_result', + JSON.stringify({ + title: result?.title || '', + url: result?.url || '', + content: result?.content || '', + description: result?.description || '', + icon: result?.icon || '', + rank: typeof result?.rank === 'number' ? result.rank : undefined, + searchId + }) + ) + ) + ) @@ - } catch (error) { - console.error('处理搜索结果时出错:', error) + } catch (error) { + console.error('[LLMGeneration] Error while processing search results', { + eventId, + err: error + }) }As per coding guidelines.
Also applies to: 206-254, 255-258
41-56: Also clear searchingMessages on error.Keeps Set consistent if a run fails.
async handleLLMAgentError(msg: LLMAgentEventData): Promise<void> { @@ - await this.messageManager.handleMessageError(eventId, String(error)) - this.generatingMessages.delete(eventId) + await this.messageManager.handleMessageError(eventId, String(error)) + this.generatingMessages.delete(eventId) + this.searchingMessages.delete(eventId)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/presenter/threadPresenter/llmGenerationManager.ts(1 hunks)src/main/presenter/threadPresenter/messageManager.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
Files:
src/main/presenter/threadPresenter/messageManager.tssrc/main/presenter/threadPresenter/llmGenerationManager.ts
src/{main,renderer}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
src/{main,renderer}/**/*.ts: Use context isolation for improved security
Implement proper inter-process communication (IPC) patterns
Optimize application startup time with lazy loading
Implement proper error handling and logging for debugging
Files:
src/main/presenter/threadPresenter/messageManager.tssrc/main/presenter/threadPresenter/llmGenerationManager.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
Use Electron's built-in APIs for file system and native dialogs
Files:
src/main/presenter/threadPresenter/messageManager.tssrc/main/presenter/threadPresenter/llmGenerationManager.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪（如适用）、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别
Files:
src/main/presenter/threadPresenter/messageManager.tssrc/main/presenter/threadPresenter/llmGenerationManager.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/presenter/threadPresenter/messageManager.tssrc/main/presenter/threadPresenter/llmGenerationManager.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/main/presenter/threadPresenter/messageManager.tssrc/main/presenter/threadPresenter/llmGenerationManager.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Use PascalCase for TypeScript types and classes
Files:
src/main/presenter/threadPresenter/messageManager.tssrc/main/presenter/threadPresenter/llmGenerationManager.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Place Electron main-process presenters under src/main/presenter/ (Window, Tab, Thread, Mcp, Config, LLMProvider)
Files:
src/main/presenter/threadPresenter/messageManager.tssrc/main/presenter/threadPresenter/llmGenerationManager.ts
**/*.{ts,tsx,js,jsx,vue,css,scss,md,json,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Prettier style: single quotes, no semicolons, print width 100; run pnpm run format
Files:
src/main/presenter/threadPresenter/messageManager.tssrc/main/presenter/threadPresenter/llmGenerationManager.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,vue}: Use OxLint for JS/TS code; keep lint clean
Use camelCase for variables and functions
Use SCREAMING_SNAKE_CASE for constants
Files:
src/main/presenter/threadPresenter/messageManager.tssrc/main/presenter/threadPresenter/llmGenerationManager.ts
🧠 Learnings (5)
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/index.ts : The main Agent loop should buffer text content, handle tool call events, format tool results for the next LLM call, and manage conversation continuation logic.
Applied to files:
src/main/presenter/threadPresenter/llmGenerationManager.ts
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/index.ts : `src/main/presenter/llmProviderPresenter/index.ts` should manage the overall Agent loop, conversation history, tool execution via `McpPresenter`, and frontend communication via `eventBus`.
Applied to files:
src/main/presenter/threadPresenter/llmGenerationManager.ts
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/index.ts : The main Agent loop in `llmProviderPresenter/index.ts` should handle multi-round LLM calls and tool usage, maintaining conversation state and controlling the loop with `needContinueConversation` and `toolCallCount`.
Applied to files:
src/main/presenter/threadPresenter/llmGenerationManager.ts
📚 Learning: 2025-10-14T08:02:59.495Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: AGENTS.md:0-0
Timestamp: 2025-10-14T08:02:59.495Z
Learning: Applies to src/main/presenter/LLMProvider/**/*.ts : Implement the two-layer LLM provider (Agent Loop + Provider) under src/main/presenter/LLMProvider
Applied to files:
src/main/presenter/threadPresenter/llmGenerationManager.ts
📚 Learning: 2025-07-21T01:46:52.880Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-07-21T01:46:52.880Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Each file in `src/main/presenter/llmProviderPresenter/providers/*.ts` should handle interaction with a specific LLM API, including request/response formatting, tool definition conversion, native/non-native tool call management, and standardizing output streams to a common event format.
Applied to files:
src/main/presenter/threadPresenter/llmGenerationManager.ts
🧬 Code graph analysis (2)
src/main/presenter/threadPresenter/messageManager.ts (2)
src/shared/chat.d.ts (2)
Message(3-32)UserMessageContent(62-72)src/main/presenter/threadPresenter/messageContent.ts (1)
formatUserMessageContent(76-116)
src/main/presenter/threadPresenter/llmGenerationManager.ts (6)
src/shared/types/presenters/legacy.presenters.d.ts (1)
ISQLitePresenter(277-330)src/main/presenter/threadPresenter/messageManager.ts (1)
MessageManager(21-364)src/main/presenter/threadPresenter/contentBufferManager.ts (1)
ContentBufferManager(14-260)src/main/presenter/threadPresenter/conversationLifecycleManager.ts (1)
ConversationLifecycleManager(23-389)src/main/presenter/threadPresenter/types.ts (1)
GeneratingMessageState(3-34)src/main/eventbus.ts (1)
eventBus(151-151)
🔇 Additional comments (1)
src/main/presenter/threadPresenter/llmGenerationManager.ts (1)
57-91: Confirm END event behavior when permissions are pending.Currently returns early and does not emit STREAM_EVENTS.END. If the UI expects END to stop spinners regardless of pending permissions, emit END with a flag instead.
Would you like me to adjust to always emit END with { hasPendingPermissions: true }?
| @@ -0,0 +1,531 @@ | |||
| import { ISQLitePresenter, MESSAGE_METADATA, LLMAgentEventData } from '../../../shared/presenter' | |||
There was a problem hiding this comment.
Unify import path alias for shared presenter types.
Use the same alias as other modules to avoid path divergence in builds.
-import { ISQLitePresenter, MESSAGE_METADATA, LLMAgentEventData } from '../../../shared/presenter'
+import { ISQLitePresenter, MESSAGE_METADATA, LLMAgentEventData } from '@shared/presenter'🤖 Prompt for AI Agents
In src/main/presenter/threadPresenter/llmGenerationManager.ts at line 1, the
import uses a relative path ('../../../shared/presenter'); replace it with the
project's configured path alias used by other modules for shared presenter types
(e.g. the same alias like '@shared/presenter' or '@/shared/presenter' used
elsewhere) so the import reads from that alias and avoid path divergence in
builds; update the import statement accordingly and run a quick build/typecheck
to verify the alias resolves.
| this.searchingMessages.add(eventId) | ||
| state.isSearching = true | ||
| } else if (tool_call === 'permission-granted') { |
There was a problem hiding this comment.
Potential leak: searchingMessages never cleared.
Delete the eventId when permission is resolved or tool run ends; also on error/finalize.
} else if (tool_call === 'permission-granted') {
@@
- lastBlock.content = tool_call_response || ''
+ lastBlock.content = tool_call_response || ''
+ this.searchingMessages.delete(eventId)
}
} else if (tool_call === 'permission-denied') {
@@
- lastBlock.content = tool_call_response || ''
+ lastBlock.content = tool_call_response || ''
+ this.searchingMessages.delete(eventId)
}
} else if (tool_call === 'end') {
@@
- }
+ }
+ this.searchingMessages.delete(eventId)
}
@@
- this.generatingMessages.delete(eventId)
+ this.generatingMessages.delete(eventId)
+ this.searchingMessages.delete(eventId)Also applies to: 337-346, 355-376, 487-490
🤖 Prompt for AI Agents
In src/main/presenter/threadPresenter/llmGenerationManager.ts around lines
326-328 (and also apply same fix at 337-346, 355-376, 487-490), eventId is added
to this.searchingMessages but never removed causing a potential leak; update the
logic to ensure this.searchingMessages.delete(eventId) is called whenever the
permission is resolved or the tool run finishes, and also in all error/finalize
branches (catch blocks, finally handlers, and any early returns) so the eventId
is always removed regardless of outcome; make the deletion idempotent (only
delete if present) and keep it adjacent to the corresponding
state.isSearching/state.* updates for clarity.
| const generationTime = Date.now() - (state.firstTokenTime ?? state.startTime) | ||
| const tokensPerSecond = completionTokens / (generationTime / 1000) | ||
| const contextUsage = state?.totalUsage?.context_length | ||
| ? (totalTokens / state.totalUsage.context_length) * 100 | ||
| : 0 | ||
|
|
There was a problem hiding this comment.
Guard tokens-per-second against zero elapsed time.
Avoid Infinity/NaN in metrics.
- const generationTime = Date.now() - (state.firstTokenTime ?? state.startTime)
- const tokensPerSecond = completionTokens / (generationTime / 1000)
+ const generationTime = Date.now() - (state.firstTokenTime ?? state.startTime)
+ const safeMs = Math.max(1, generationTime)
+ const tokensPerSecond = completionTokens / (safeMs / 1000)📝 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.
| const generationTime = Date.now() - (state.firstTokenTime ?? state.startTime) | |
| const tokensPerSecond = completionTokens / (generationTime / 1000) | |
| const contextUsage = state?.totalUsage?.context_length | |
| ? (totalTokens / state.totalUsage.context_length) * 100 | |
| : 0 | |
| const generationTime = Date.now() - (state.firstTokenTime ?? state.startTime) | |
| const safeMs = Math.max(1, generationTime) | |
| const tokensPerSecond = completionTokens / (safeMs / 1000) | |
| const contextUsage = state?.totalUsage?.context_length | |
| ? (totalTokens / state.totalUsage.context_length) * 100 | |
| : 0 |
🤖 Prompt for AI Agents
In src/main/presenter/threadPresenter/llmGenerationManager.ts around lines 465
to 470, the tokensPerSecond calculation can produce Infinity/NaN when
generationTime is 0; change the logic to guard against zero or negative elapsed
time by ensuring generationTime is at least 1 millisecond (or explicitly
checking for 0) before dividing, and set tokensPerSecond to 0 (or a safe
fallback) when generationTime is zero or invalid; keep the existing contextUsage
logic as-is.
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 (1)
src/main/presenter/threadPresenter/messageManager.ts (1)
141-141: Translate comments to English.The Chinese comments at lines 141, 187, 311, 333, and 337 violate the coding guideline: "Use English for all logs and comments."
As per coding guidelines.
Please translate these comments:
- Line 141:
// 创建一个新的变体消息→// Create a new variant message- Line 187:
// 处理消息的排序和变体关系→// Handle message sorting and variant relationships- Line 311:
// 初始化未完成的消息→// Initialize unfinished messages- Line 333:
// 初始化未完成消息失败:→// Failed to initialize unfinished messages:- Line 337:
// 处理消息错误状态的公共函数→// Common function to handle message error statusAlso applies to: 187-187, 311-311, 333-333, 337-337
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/presenter/threadPresenter/messageManager.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
Files:
src/main/presenter/threadPresenter/messageManager.ts
src/{main,renderer}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
src/{main,renderer}/**/*.ts: Use context isolation for improved security
Implement proper inter-process communication (IPC) patterns
Optimize application startup time with lazy loading
Implement proper error handling and logging for debugging
Files:
src/main/presenter/threadPresenter/messageManager.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
Use Electron's built-in APIs for file system and native dialogs
Files:
src/main/presenter/threadPresenter/messageManager.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪（如适用）、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别
Files:
src/main/presenter/threadPresenter/messageManager.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/presenter/threadPresenter/messageManager.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/main/presenter/threadPresenter/messageManager.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Use PascalCase for TypeScript types and classes
Files:
src/main/presenter/threadPresenter/messageManager.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Place Electron main-process presenters under src/main/presenter/ (Window, Tab, Thread, Mcp, Config, LLMProvider)
Files:
src/main/presenter/threadPresenter/messageManager.ts
**/*.{ts,tsx,js,jsx,vue,css,scss,md,json,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Prettier style: single quotes, no semicolons, print width 100; run pnpm run format
Files:
src/main/presenter/threadPresenter/messageManager.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,vue}: Use OxLint for JS/TS code; keep lint clean
Use camelCase for variables and functions
Use SCREAMING_SNAKE_CASE for constants
Files:
src/main/presenter/threadPresenter/messageManager.ts
🧬 Code graph analysis (1)
src/main/presenter/threadPresenter/messageManager.ts (2)
src/shared/chat.d.ts (2)
Message(3-32)UserMessageContent(62-72)src/main/presenter/threadPresenter/messageContent.ts (1)
formatUserMessageContent(76-116)
🔇 Additional comments (3)
src/main/presenter/threadPresenter/messageManager.ts (3)
17-17: LGTM: Import supports new normalization feature.The import of
formatUserMessageContentis correctly used in thegetContextMessagesmethod to normalize user message text.
231-256: LGTM: Signature and ensureUserStart logic are sound.The optional parameters with defaults maintain backward compatibility, and the logic correctly trims leading non-user messages.
277-297: LGTM: Correctly implements the previous review's suggestion.This implementation properly addresses the earlier concern by fetching all messages, sorting chronologically, and slicing around the target message. The bounds checking and fallback logic are correct.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
src/main/presenter/threadPresenter/templates/conversationExportTemplates.ts (1)
109-116: Use a semantic class for the links section (keep styling).The links section reuses the
attachmentsclass; prefer a dedicated class to avoid confusion and allow independent styling later.Apply this diff:
- linksSection: [ - ' <div class="section attachments">', + linksSection: [ + ' <div class="section links">', ' <div class="section-title">🔗 链接</div>', ' <ul>', '{{items}}', ' </ul>', ' </div>' ],And add a style alias that inherits the same look:
'.attachments, .search-block, .tool-call, .error-block, .reasoning-block { border-radius: 12px; padding: 18px 20px; border: 1px solid rgba(203, 213, 225, 0.7); background: #f8fafc; }', + '.links { border-radius: 12px; padding: 18px 20px; border: 1px solid rgba(203, 213, 225, 0.7); background: #f8fafc; }',src/main/presenter/threadPresenter/conversationExporter.ts (4)
135-139: Preserve zero search results in Markdown.Current truthy check hides
0. Check for!== undefinedlike HTML exporter.Apply this diff:
- if (block.extra?.total) { + if (block.extra?.total !== undefined) { lines.push(`找到 ${block.extra.total} 个搜索结果`) }
503-507: Preserve zero search results in Text.Match the HTML logic.
Apply this diff:
- if (block.extra?.total) { + if (block.extra?.total !== undefined) { lines.push(`找到 ${block.extra.total} 个搜索结果`) }
405-408: Avoid dynamic RegExp; use split/join to silence ReDoS lint and reduce overhead.Keys are static, but this removes the warning with zero behavior change. Based on static analysis hints.
Apply this diff:
- for (const [key, value] of Object.entries(replacements)) { - const pattern = new RegExp(`\\{\\{${key}\\}\\}`, 'g') - rendered = rendered.replace(pattern, value) - } + for (const [key, value] of Object.entries(replacements)) { + const token = `{{${key}}}` + if (rendered.includes(token)) { + rendered = rendered.split(token).join(value) + } + }
35-36: Consider English or i18n for thrown error message.Align error text with our guidelines and enable localization.
Apply this diff:
- throw new Error(`不支持的导出格式: ${format}`) + throw new Error(`Unsupported export format: ${format}`)As per coding guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/presenter/threadPresenter/conversationExporter.ts(1 hunks)src/main/presenter/threadPresenter/templates/conversationExportTemplates.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/templates/conversationExportTemplates.ts
src/{main,renderer}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
src/{main,renderer}/**/*.ts: Use context isolation for improved security
Implement proper inter-process communication (IPC) patterns
Optimize application startup time with lazy loading
Implement proper error handling and logging for debugging
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/templates/conversationExportTemplates.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
Use Electron's built-in APIs for file system and native dialogs
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/templates/conversationExportTemplates.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪（如适用）、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/templates/conversationExportTemplates.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/templates/conversationExportTemplates.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/templates/conversationExportTemplates.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Use PascalCase for TypeScript types and classes
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/templates/conversationExportTemplates.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Place Electron main-process presenters under src/main/presenter/ (Window, Tab, Thread, Mcp, Config, LLMProvider)
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/templates/conversationExportTemplates.ts
**/*.{ts,tsx,js,jsx,vue,css,scss,md,json,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Prettier style: single quotes, no semicolons, print width 100; run pnpm run format
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/templates/conversationExportTemplates.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,vue}: Use OxLint for JS/TS code; keep lint clean
Use camelCase for variables and functions
Use SCREAMING_SNAKE_CASE for constants
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/templates/conversationExportTemplates.ts
🧬 Code graph analysis (1)
src/main/presenter/threadPresenter/conversationExporter.ts (2)
src/main/presenter/threadPresenter/messageContent.ts (1)
getNormalizedUserMessageText(143-153)src/main/presenter/threadPresenter/templates/conversationExportTemplates.ts (1)
conversationExportTemplates(10-169)
🪛 ast-grep (0.39.6)
src/main/presenter/threadPresenter/conversationExporter.ts
[warning] 405-405: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(\\{\\{${key}\\}\\}, 'g')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔇 Additional comments (1)
src/main/presenter/threadPresenter/conversationExporter.ts (1)
8-20: Filename generator looks solid.Stable, cross‑platform timestamp formatting; correct extension mapping.
src/main/presenter/threadPresenter/templates/conversationExportTemplates.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/presenter/threadPresenter/conversationExporter.ts (1)
480-486: Guard undefined file properties in Text export.Similar to the fixes applied in Markdown and HTML exports, the text export should guard against undefined
mimeTypeandnameproperties to avoid rendering "undefined" in the exported content.Apply this diff:
if (userContent.files && userContent.files.length > 0) { lines.push('附件:') for (const file of userContent.files) { - lines.push(`- ${file.name} (${file.mimeType})`) + lines.push(`- ${file.name ?? ''} (${file.mimeType ?? 'unknown'})`) } lines.push('') }
🧹 Nitpick comments (3)
src/main/presenter/threadPresenter/conversationExporter.ts (3)
22-37: Consider adding error handling and logging.The coding guidelines require try-catch blocks with meaningful error messages and structured logging. The format-specific export functions could throw errors during execution (e.g., during JSON parsing or template rendering).
Consider wrapping the export calls:
export function buildConversationExportContent( conversation: CONVERSATION, messages: Message[], format: ConversationExportFormat ): string { + try { switch (format) { case 'markdown': return exportToMarkdown(conversation, messages) case 'html': return exportToHtml(conversation, messages) case 'txt': return exportToText(conversation, messages) default: throw new Error(`Unsupported export format: ${format}`) } + } catch (error) { + console.error('[ERROR] Export failed:', { format, conversationId: conversation.id, error }) + throw error + } }Based on coding guidelines
565-572: Consider adding defensive null handling.While all current call sites should pass strings, adding a guard would make the function more robust against future changes.
function escapeHtml(text: string): string { + if (text == null) return '' return text .replace(/&/g, '&') .replace(/</g, '<') .replace(/>/g, '>') .replace(/"/g, '"') .replace(/'/g, ''') }
175-392: Consider extracting helper functions to reduce complexity.The
exportToHtmlfunction is 217 lines long with high cyclomatic complexity. Extracting helpers for rendering different message types and assistant blocks would improve readability and maintainability.Example extraction for assistant block rendering:
function renderAssistantBlock( block: AssistantMessageBlock, templates: typeof conversationExportTemplates.templates ): string[] { const blockLines: string[] = [] switch (block.type) { case 'content': if (block.content) { blockLines.push( ...renderTemplate(templates.assistantContent, { content: formatInlineHtml(block.content) }) ) } break // ... other cases } return blockLines }Then in exportToHtml:
for (const block of assistantBlocks) { blockLines.push(...renderAssistantBlock(block, templates)) }This pattern could be applied to user message rendering as well.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/presenter/threadPresenter/conversationExporter.ts(1 hunks)src/main/presenter/threadPresenter/messageManager.ts(2 hunks)src/main/presenter/threadPresenter/templates/conversationExportTemplates.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/presenter/threadPresenter/templates/conversationExportTemplates.ts
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/messageManager.ts
src/{main,renderer}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
src/{main,renderer}/**/*.ts: Use context isolation for improved security
Implement proper inter-process communication (IPC) patterns
Optimize application startup time with lazy loading
Implement proper error handling and logging for debugging
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/messageManager.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
Use Electron's built-in APIs for file system and native dialogs
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/messageManager.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪（如适用）、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/messageManager.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/messageManager.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/messageManager.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Use PascalCase for TypeScript types and classes
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/messageManager.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Place Electron main-process presenters under src/main/presenter/ (Window, Tab, Thread, Mcp, Config, LLMProvider)
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/messageManager.ts
**/*.{ts,tsx,js,jsx,vue,css,scss,md,json,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Prettier style: single quotes, no semicolons, print width 100; run pnpm run format
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/messageManager.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,vue}: Use OxLint for JS/TS code; keep lint clean
Use camelCase for variables and functions
Use SCREAMING_SNAKE_CASE for constants
Files:
src/main/presenter/threadPresenter/conversationExporter.tssrc/main/presenter/threadPresenter/messageManager.ts
🧬 Code graph analysis (2)
src/main/presenter/threadPresenter/conversationExporter.ts (2)
src/main/presenter/threadPresenter/messageContent.ts (1)
getNormalizedUserMessageText(143-153)src/main/presenter/threadPresenter/templates/conversationExportTemplates.ts (1)
conversationExportTemplates(10-170)
src/main/presenter/threadPresenter/messageManager.ts (2)
src/shared/chat.d.ts (2)
Message(3-32)UserMessageContent(62-72)src/main/presenter/threadPresenter/messageContent.ts (1)
formatUserMessageContent(76-116)
🔇 Additional comments (6)
src/main/presenter/threadPresenter/messageManager.ts (3)
17-17: LGTM! Import supports new text normalization feature.The import is correctly used in the
normalizeUserTextlogic below to format user message content blocks into plain text.
231-278: Excellent fix! The shallow-copy mutation bug has been properly addressed.The previous review correctly identified that shallow-copying the message while mutating its content object would affect the original. The current implementation properly deep-copies both levels:
return { ...msg, // new message object content: { ...userContent, // new content object text: formatUserMessageContent(userContent.content) } }Both
ensureUserStartandnormalizeUserTextoptions work correctly. The mutation viashift()is safe since it operates on a local array.
280-300: Well done! The pagination issue has been properly resolved.The method now correctly fetches and sorts all messages in the conversation before slicing, ensuring the target message is found regardless of its position. The slice calculation is accurate, and edge cases (invalid limit, missing target) are handled appropriately.
src/main/presenter/threadPresenter/conversationExporter.ts (3)
70-76: LGTM! Undefined guards properly applied.The file attachment rendering now correctly guards against undefined
mimeTypeandnameproperties with appropriate fallbacks.
238-253: LGTM! Links are sanitized before rendering.The HTML export now properly sanitizes href values using
sanitizeHrefbefore HTML escaping, addressing the previous security concern about unsafe URL schemes.
516-529: LGTM! Tool call properties properly guarded.The text export correctly uses nullish coalescing for
tool_call.nameand checks for existence before accessingparamsandresponseproperties.
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_6900a5c21928832ca92acbb2d9f98fac
Summary by CodeRabbit
New Features
Improvements