-
Notifications
You must be signed in to change notification settings - Fork 614
fix(formatMessage): system prompt is spliced twice #581
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(formatMessage): system prompt is spliced twice #581
Conversation
WalkthroughThe update modifies the Changes
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/presenter/threadPresenter/index.ts (1)
1747-1747: Consider removing debug log or making it conditional for production.This debug log was uncommented to help observe the system prompt duplication issue mentioned in the PR. However, it logs potentially sensitive conversation data that should not be exposed in production environments.
Consider making this conditional based on a debug flag:
- console.log('preparePromptContent', mergedMessages, promptTokens) + if (process.env.NODE_ENV === 'development') { + console.log('preparePromptContent', mergedMessages, promptTokens) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/presenter/threadPresenter/index.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
`**/*.{js,jsx,ts,tsx}`: 使用 OxLint 进行代码检查 Log和注释使用英文书写
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
📄 Source: CodeRabbit Inference Engine (.cursor/rules/development-setup.mdc)
List of files the instruction was applied to:
src/main/presenter/threadPresenter/index.ts
`src/main/presenter/**/*.ts`: Use context isolation for improved security Use El...
src/main/presenter/**/*.ts: Use context isolation for improved security
Use Electron's built-in APIs for file system and native dialogs
Optimize application startup time with lazy loading
📄 Source: CodeRabbit Inference Engine (.cursor/rules/electron-best-practices.mdc)
List of files the instruction was applied to:
src/main/presenter/threadPresenter/index.ts
`{src/main/presenter/**/*.ts,src/renderer/stores/**/*.ts}`: Implement proper inter-process communication (IPC) patterns Implement proper error handling and logging for debugging
{src/main/presenter/**/*.ts,src/renderer/stores/**/*.ts}: Implement proper inter-process communication (IPC) patterns
Implement proper error handling and logging for debugging
📄 Source: CodeRabbit Inference Engine (.cursor/rules/electron-best-practices.mdc)
List of files the instruction was applied to:
src/main/presenter/threadPresenter/index.ts
`**/*.{ts,tsx}`: 始终使用 try-catch 处理可能的错误 提供有意义的错误信息 记录详细的错误日志 优雅降级处理 日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪(如适用)、相关上下文信息 使用结构化日志 避免记录敏感信息 设置适当的日志级别 不要吞掉错误 提供用户友好的错误信息 实现错误重试机制
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪(如适用)、相关上下文信息
使用结构化日志
避免记录敏感信息
设置适当的日志级别
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
📄 Source: CodeRabbit Inference Engine (.cursor/rules/error-logging.mdc)
List of files the instruction was applied to:
src/main/presenter/threadPresenter/index.ts
`src/main/**`: 主进程代码放在 `src/main`
src/main/**: 主进程代码放在src/main
📄 Source: CodeRabbit Inference Engine (.cursor/rules/project-structure.mdc)
List of files the instruction was applied to:
src/main/presenter/threadPresenter/index.ts
🧠 Learnings (1)
src/main/presenter/threadPresenter/index.ts (10)
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-06-30T12:24:03.565Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Provider files should include provider-specific helper functions such as `formatMessages`, `convertToProviderTools`, `parseFunctionCalls`, and `prepareFunctionCallPrompt` as needed.
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-06-30T12:24:03.565Z
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 communication with the frontend via `eventBus`.
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-06-30T12:24:03.565Z
Learning: Applies to src/main/presenter/llmProviderPresenter/index.ts : The main Agent loop, including the `while` loop for conversation flow, state management, provider interaction, event handling, and frontend communication, must be implemented in `src/main/presenter/llmProviderPresenter/index.ts`.
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-06-30T12:24:03.565Z
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, management of native/non-native tool call mechanisms (prompt wrapping), and standardizing output streams to a common event format.
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/electron-best-practices.mdc:0-0
Timestamp: 2025-06-30T12:23:13.338Z
Learning: Applies to src/main/presenter/**/*.ts : Use context isolation for improved security
Learnt from: neoragex2002
PR: ThinkInAIXYZ/deepchat#550
File: src/renderer/src/stores/chat.ts:1011-1035
Timestamp: 2025-06-21T15:49:17.044Z
Learning: In src/renderer/src/stores/chat.ts, the user prefers to keep both `text` and `content` properties in the `handleMeetingInstruction` function's `sendMessage` call, even though they are redundant, rather than removing the `content` property.
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/llm-agent-loop.mdc:0-0
Timestamp: 2025-06-30T12:24:03.565Z
Learning: Applies to src/main/presenter/llmProviderPresenter/providers/*.ts : Provider files must implement the `coreStream(messages, modelId, temperature, maxTokens)` method, which receives formatted messages and generation parameters, handles tool support (native or via prompt wrapping), makes a single streaming API call, parses provider-specific data, and yields standardized stream events.
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/electron-best-practices.mdc:0-0
Timestamp: 2025-06-30T12:23:13.338Z
Learning: Applies to src/renderer/src/composables/usePresenter.ts : The IPC in the renderer process is implemented in usePresenter.ts, allowing direct calls to the presenter-related interfaces exposed by the main process
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/electron-best-practices.mdc:0-0
Timestamp: 2025-06-30T12:23:13.338Z
Learning: Applies to {src/main/presenter/**/*.ts,src/renderer/stores/**/*.ts} : Implement proper error handling and logging for debugging
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/electron-best-practices.mdc:0-0
Timestamp: 2025-06-30T12:23:13.338Z
Learning: Applies to {src/main/presenter/**/*.ts,src/renderer/stores/**/*.ts} : Implement proper inter-process communication (IPC) patterns
🧬 Code Graph Analysis (1)
src/main/presenter/threadPresenter/index.ts (1)
src/shared/presenter.d.ts (1)
ChatMessage(1066-1078)
🔇 Additional comments (2)
src/main/presenter/threadPresenter/index.ts (2)
1818-1831: Good fix for the system prompt duplication issue.The reordering correctly addresses the bug mentioned in the PR objectives:
- Context messages are added first (lines 1818-1821)
- System prompt is inserted at the front using
unshift(line 1826)This ensures the system prompt appears first in the conversation without duplication, replacing the commented
addSystemPromptmethod that was likely causing the issue.
1874-1879: Good refactoring of method signature.The simplified signature removes the unnecessary
formattedMessagesparameter and makes the method more self-contained by initializing its own result array. This improves the method's interface while maintaining the same functionality.
|
LGTM |
Pull Request Description
Is your feature request related to a problem? Please describe.
When synthesizing the conversation context message, the system prompt word is spliced twice. You can observe this by uncommenting the
preparePromptContentlog in the code.Describe the solution you'd like
Fix the above bug.
UI/UX changes for Desktop Application
None
Platform Compatibility Notes
None
Additional context
None
Pull Request Description (中文)
你的功能请求是否与某个问题有关?请描述一下。
在合成对话上下文消息时,系统提示词被合并了两次。取消代码中
preparePromptContent这条log的注释可以观察到。请描述你希望的解决方案
修复以上问题。
桌面应用程序的 UI/UX 更改
无
平台兼容性注意事项
无
附加背景
无
Summary by CodeRabbit
Bug Fixes
Other