-
Notifications
You must be signed in to change notification settings - Fork 625
fix global ignore of message variants in context #961
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
Conversation
# Conflicts: # src/renderer/src/components/message/MessageItemAssistant.vue
WalkthroughAdds per-message assistant variant selection end-to-end: database schema (conversations.context_chain), thread presenter APIs to accept/preserve selectedVariantsMap, chat store persistence and propagation, and renderer UI changes to centralize variant selection, eventing, and variant-aware message display. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as UI (MessageItemAssistant)
participant Store as Chat Store
participant TP as ThreadPresenter
participant DB as Conversations (context_chain)
participant LLM as LLM
UI->>Store: updateSelectedVariant(mainId, variantId)
Store->>Store: selectedVariantsMap.set(mainId, variantId)
Store->>DB: updateConversationSettings({ selectedVariantsMap })
DB-->>Store: ack
Store->>TP: start/continue/regenerate(..., selectedVariantsMap)
TP->>TP: prepareConversationContext(selectedVariantsMap)
TP->>TP: substitute assistant messages with selected variants
TP->>LLM: stream with substituted context
LLM-->>TP: tokens/content
TP-->>Store: store assistant message (uses substituted fields)
Store-->>UI: render updated variant-aware messages
sequenceDiagram
autonumber
participant UI as UI (Fork action)
participant Store as Chat Store
participant TP as ThreadPresenter
participant DB as Conversations
UI->>Store: forkThread(convId, msgId, title)
Store->>TP: forkConversation(..., settings?, selectedVariantsMap)
TP->>TP: build history up to target message
TP->>TP: apply variant substitutions on copied messages
TP->>DB: insert new conversation/messages (with substituted fields)
DB-->>TP: newConversationId
TP-->>Store: newConversationId
Store->>DB: persist selectedVariantsMap in new conversation settings
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
707-707: Fix logic error: comparinggetActiveThreadId()with itself.Lines 707, 716, and 730 compare
getActiveThreadId() === getActiveThreadId(), which is alwaystrue. Based on the surrounding context (checkingcached.threadId === getActiveThreadId()elsewhere), these should compare againstcached.threadId.Apply this diff:
- if (getActiveThreadId() === getActiveThreadId()) { + if (cached.threadId === getActiveThreadId()) {Apply the same fix at lines 716 and 730.
Also applies to: 716-716, 730-730
src/main/presenter/sqlitePresenter/tables/conversations.ts (1)
60-127: Add a migration forcontext_chain.We now read/write
context_chain, butgetMigrationSQL/getLatestVersionwere not bumped. Existing installs (schema v7) won’t add the column, so every SELECT/UPDATE referencingcontext_chainwill throwno such column. Please add a v8 migration (ALTER TABLE … ADD COLUMN context_chain TEXT DEFAULT '{}') and updategetLatestVersion()accordingly.
🧹 Nitpick comments (3)
src/renderer/src/stores/chat.ts (2)
1061-1085: ClarifychatConfigsynchronization and consider improving error handling.The function updates
chatConfig.value.selectedVariantsMap(line 1074) but doesn't callsaveChatConfig(). Instead, it directly callsupdateConversationSettings(line 1079). While this may work, it creates two divergent code paths for persisting settings.Additionally, errors are only logged to console without user notification or state rollback.
Consider unifying the persistence approach:
// Sync with chatConfig and persist through existing mechanism if (chatConfig.value) { chatConfig.value.selectedVariantsMap = Object.fromEntries(selectedVariantsMap.value) + await saveChatConfig() } - // Persist to backend - try { - await threadP.updateConversationSettings(activeThreadId, { - selectedVariantsMap: Object.fromEntries(selectedVariantsMap.value) - }) - } catch (error) { - console.error('Failed to update selected variant:', error) - }Alternatively, if direct
updateConversationSettingsis preferred, remove thechatConfig.valueupdate sinceloadChatConfig()will sync it from the backend.
851-891: Optimize variant auto-selection in deleteMessageIn
src/renderer/src/stores/chat.ts(around lines 878–881), only callupdateSelectedVariantwhen the deleted variant is the one currently selected:const remainingVariants = parentMessage.variants?.filter((v) => v.id !== messageId) || [] parentMessage.variants = remainingVariants messages[parentIndex] = { ...parentMessage } - const newSelectedVariantId = - remainingVariants.length > 0 ? remainingVariants[remainingVariants.length - 1].id : null - await updateSelectedVariant(parentMessage.id, newSelectedVariantId) + // Only update if we removed the active variant + if (selectedVariantsMap.value.get(parentMessage.id) === messageId) { + const newSelectedVariantId = + remainingVariants.length > 0 ? remainingVariants[remainingVariants.length - 1].id : null + await updateSelectedVariant(parentMessage.id, newSelectedVariantId) + }This avoids redundant backend writes when deleting non-selected variants.
src/main/presenter/sqlitePresenter/tables/conversations.ts (1)
30-37: Translate inline comment to English.Project rules require logs/comments in English. Please update
// 解析 JSON 字段to an English equivalent (e.g.// Parse JSON field).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/presenter/sqlitePresenter/tables/conversations.ts(8 hunks)src/main/presenter/threadPresenter/index.ts(11 hunks)src/renderer/src/components/MessageNavigationSidebar.vue(1 hunks)src/renderer/src/components/message/MessageItemAssistant.vue(4 hunks)src/renderer/src/components/message/MessageList.vue(2 hunks)src/renderer/src/stores/chat.ts(16 hunks)src/renderer/src/views/ChatTabView.vue(2 hunks)src/shared/presenter.d.ts(1 hunks)src/shared/types/presenters/legacy.presenters.d.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (27)
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/components/MessageNavigationSidebar.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/stores/chat.tssrc/renderer/src/components/message/MessageItemAssistant.vuesrc/renderer/src/views/ChatTabView.vue
src/renderer/**/*.{vue,ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
渲染进程代码放在
src/renderer
Files:
src/renderer/src/components/MessageNavigationSidebar.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/stores/chat.tssrc/renderer/src/components/message/MessageItemAssistant.vuesrc/renderer/src/views/ChatTabView.vue
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/components/MessageNavigationSidebar.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/stores/chat.tssrc/renderer/src/components/message/MessageItemAssistant.vuesrc/renderer/src/views/ChatTabView.vue
src/renderer/src/**/*.vue
📄 CodeRabbit inference engine (.cursor/rules/vue-best-practices.mdc)
Use scoped styles to prevent CSS conflicts between components
Files:
src/renderer/src/components/MessageNavigationSidebar.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/components/message/MessageItemAssistant.vuesrc/renderer/src/views/ChatTabView.vue
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/components/MessageNavigationSidebar.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/stores/chat.tssrc/renderer/src/components/message/MessageItemAssistant.vuesrc/renderer/src/views/ChatTabView.vue
src/renderer/**/*.{vue,ts}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
Implement lazy loading for routes and components.
Files:
src/renderer/src/components/MessageNavigationSidebar.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/stores/chat.tssrc/renderer/src/components/message/MessageItemAssistant.vuesrc/renderer/src/views/ChatTabView.vue
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/components/MessageNavigationSidebar.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/stores/chat.tssrc/renderer/src/components/message/MessageItemAssistant.vuesrc/renderer/src/views/ChatTabView.vue
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/renderer/src/components/MessageNavigationSidebar.vuesrc/shared/presenter.d.tssrc/renderer/src/components/message/MessageList.vuesrc/main/presenter/threadPresenter/index.tssrc/renderer/src/stores/chat.tssrc/renderer/src/components/message/MessageItemAssistant.vuesrc/shared/types/presenters/legacy.presenters.d.tssrc/renderer/src/views/ChatTabView.vuesrc/main/presenter/sqlitePresenter/tables/conversations.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Files:
src/renderer/src/components/MessageNavigationSidebar.vuesrc/shared/presenter.d.tssrc/renderer/src/components/message/MessageList.vuesrc/main/presenter/threadPresenter/index.tssrc/renderer/src/stores/chat.tssrc/renderer/src/components/message/MessageItemAssistant.vuesrc/shared/types/presenters/legacy.presenters.d.tssrc/renderer/src/views/ChatTabView.vuesrc/main/presenter/sqlitePresenter/tables/conversations.ts
src/renderer/{src,shell,floating}/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
src/renderer/{src,shell,floating}/**/*.vue: Use Vue 3 Composition API for all components
All user-facing strings must use i18n keys via vue-i18n (no hard-coded UI strings)
Use Tailwind CSS utilities and ensure styles are scoped in Vue components
Files:
src/renderer/src/components/MessageNavigationSidebar.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/components/message/MessageItemAssistant.vuesrc/renderer/src/views/ChatTabView.vue
src/renderer/src/components/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Organize UI components by feature within src/renderer/src/
Files:
src/renderer/src/components/MessageNavigationSidebar.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/components/message/MessageItemAssistant.vue
src/renderer/src/**
📄 CodeRabbit inference engine (AGENTS.md)
Put application code for the Vue app under src/renderer/src (components, stores, views, i18n, lib)
Files:
src/renderer/src/components/MessageNavigationSidebar.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/stores/chat.tssrc/renderer/src/components/message/MessageItemAssistant.vuesrc/renderer/src/views/ChatTabView.vue
src/renderer/src/**/*.{vue,ts}
📄 CodeRabbit inference engine (AGENTS.md)
All user-facing strings in the renderer must use vue-i18n keys defined in src/renderer/src/i18n
Files:
src/renderer/src/components/MessageNavigationSidebar.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/stores/chat.tssrc/renderer/src/components/message/MessageItemAssistant.vuesrc/renderer/src/views/ChatTabView.vue
**/*.{js,jsx,ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Prettier formatting: single quotes, no semicolons, max width 100
Files:
src/renderer/src/components/MessageNavigationSidebar.vuesrc/shared/presenter.d.tssrc/renderer/src/components/message/MessageList.vuesrc/main/presenter/threadPresenter/index.tssrc/renderer/src/stores/chat.tssrc/renderer/src/components/message/MessageItemAssistant.vuesrc/shared/types/presenters/legacy.presenters.d.tssrc/renderer/src/views/ChatTabView.vuesrc/main/presenter/sqlitePresenter/tables/conversations.ts
src/renderer/**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
Name Vue components in PascalCase (e.g., ChatInput.vue)
Files:
src/renderer/src/components/MessageNavigationSidebar.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/components/message/MessageItemAssistant.vuesrc/renderer/src/views/ChatTabView.vue
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
**/*.{js,jsx,ts,tsx}: Use OxLint for JS/TS code; pre-commit hooks run lint-staged and typecheck
Use camelCase for variables and functions
Use PascalCase for types and classes
Use SCREAMING_SNAKE_CASE for constants
Files:
src/shared/presenter.d.tssrc/main/presenter/threadPresenter/index.tssrc/renderer/src/stores/chat.tssrc/shared/types/presenters/legacy.presenters.d.tssrc/main/presenter/sqlitePresenter/tables/conversations.ts
src/shared/*.d.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
The shared/*.d.ts files are used to define the types of objects exposed by the main process to the renderer process
Files:
src/shared/presenter.d.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪(如适用)、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别
Files:
src/shared/presenter.d.tssrc/main/presenter/threadPresenter/index.tssrc/renderer/src/stores/chat.tssrc/shared/types/presenters/legacy.presenters.d.tssrc/main/presenter/sqlitePresenter/tables/conversations.ts
src/shared/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
共享类型定义放在
shared目录
Files:
src/shared/presenter.d.tssrc/shared/types/presenters/legacy.presenters.d.ts
src/shared/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place shared types, utilities, constants, and IPC contract definitions under src/shared/
Files:
src/shared/presenter.d.tssrc/shared/types/presenters/legacy.presenters.d.ts
src/shared/**
📄 CodeRabbit inference engine (AGENTS.md)
Store shared TypeScript types/utilities in src/shared/
Files:
src/shared/presenter.d.tssrc/shared/types/presenters/legacy.presenters.d.ts
src/{main,renderer}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
src/{main,renderer}/**/*.ts: Use context isolation for improved security
Implement proper inter-process communication (IPC) patterns
Optimize application startup time with lazy loading
Implement proper error handling and logging for debugging
Files:
src/main/presenter/threadPresenter/index.tssrc/renderer/src/stores/chat.tssrc/main/presenter/sqlitePresenter/tables/conversations.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/index.tssrc/main/presenter/sqlitePresenter/tables/conversations.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/presenter/threadPresenter/index.tssrc/main/presenter/sqlitePresenter/tables/conversations.ts
src/main/**
📄 CodeRabbit inference engine (AGENTS.md)
Place all Electron main-process code under src/main/
Files:
src/main/presenter/threadPresenter/index.tssrc/main/presenter/sqlitePresenter/tables/conversations.ts
src/main/presenter/**
📄 CodeRabbit inference engine (AGENTS.md)
src/main/presenter/**: Organize main-process presenters under src/main/presenter/ (Window/Tab/Thread/Mcp/Config/LLMProvider)
Follow the Presenter pattern for main-process modules
Files:
src/main/presenter/threadPresenter/index.tssrc/main/presenter/sqlitePresenter/tables/conversations.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
🧠 Learnings (1)
📓 Common learnings
Learnt from: neoragex2002
PR: ThinkInAIXYZ/deepchat#550
File: src/main/presenter/mcpPresenter/inMemoryServers/meetingServer.ts:258-324
Timestamp: 2025-06-21T15:53:34.641Z
Learning: User neoragex2002 prefers concise, brief responses rather than chatty or verbose communication. Avoid lengthy explanations, excessive enthusiasm, or ASCII art in responses.
🧬 Code graph analysis (4)
src/shared/presenter.d.ts (2)
src/shared/types/presenters/thread.presenter.d.ts (1)
CONVERSATION_SETTINGS(15-30)src/shared/types/presenters/index.d.ts (1)
CONVERSATION_SETTINGS(27-27)
src/main/presenter/threadPresenter/index.ts (4)
src/shared/types/core/chat.ts (1)
UserMessageContent(23-37)src/main/presenter/threadPresenter/fileContext.ts (1)
getFileContext(3-22)src/shared/presenter.d.ts (1)
CONVERSATION_SETTINGS(4-20)src/shared/types/presenters/thread.presenter.d.ts (2)
CONVERSATION_SETTINGS(15-30)MESSAGE_METADATA(46-58)
src/renderer/src/stores/chat.ts (2)
src/shared/chat.d.ts (2)
AssistantMessage(39-42)UserMessage(34-37)src/renderer/src/events.ts (1)
CONVERSATION_EVENTS(31-37)
src/shared/types/presenters/legacy.presenters.d.ts (3)
src/shared/presenter.d.ts (1)
CONVERSATION_SETTINGS(4-20)src/shared/types/presenters/thread.presenter.d.ts (2)
CONVERSATION_SETTINGS(15-30)MESSAGE(60-72)src/shared/chat.d.ts (1)
AssistantMessage(39-42)
⏰ 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/renderer/src/stores/chat.ts (6)
67-68: LGTM: Config state additions.The additions of
verbosityandselectedVariantsMaptochatConfigalign with the PR's objective to persist variant selections.
80-81: LGTM: In-memory variant tracking.The
selectedVariantsMapref provides efficient in-memory tracking of variant selections per message.
189-189: LGTM: Proper cleanup on thread clear.Clearing
selectedVariantsMapwhen clearing the active thread ensures state consistency.
302-306: LGTM: Consistent variant map propagation to backend.All streaming and generation methods correctly pass
Object.fromEntries(selectedVariantsMap.value)to enable variant-aware context construction in the backend.Also applies to: 326-330, 345-346, 377-378, 982-986
820-827: LGTM: Proper rehydration of variant selections.Correctly loads
selectedVariantsMapfrom conversation settings and populates the in-memory Map on activation.
1357-1357: LGTM: Appropriate public API additions.The exports of
selectedVariantsMap,variantAwareMessages, andupdateSelectedVariantcorrectly expose the variant selection functionality to consuming components.Also applies to: 1360-1360, 1393-1393
| const variantAwareMessages = computed(() => { | ||
| const messages = getMessages() | ||
| const currentSelectedVariants = selectedVariantsMap.value | ||
|
|
||
| if (currentSelectedVariants.size === 0) { | ||
| return messages | ||
| } | ||
|
|
||
| return messages.map((msg) => { | ||
| if (msg.role === 'assistant' && currentSelectedVariants.has(msg.id)) { | ||
| const selectedVariantId = currentSelectedVariants.get(msg.id) | ||
|
|
||
| if (!selectedVariantId) { | ||
| return msg | ||
| } | ||
|
|
||
| const selectedVariant = (msg as AssistantMessage).variants?.find( | ||
| (v) => v.id === selectedVariantId | ||
| ) | ||
|
|
||
| if (selectedVariant) { | ||
| const newMsg = JSON.parse(JSON.stringify(msg)) | ||
|
|
||
| newMsg.content = selectedVariant.content | ||
| newMsg.usage = selectedVariant.usage | ||
| newMsg.model_id = selectedVariant.model_id | ||
| newMsg.model_provider = selectedVariant.model_provider | ||
| newMsg.model_name = selectedVariant.model_name | ||
|
|
||
| return newMsg | ||
| } | ||
| } | ||
|
|
||
| return msg | ||
| }) as AssistantMessage[] | UserMessage[] | ||
| }) |
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.
Replace JSON-based cloning with structuredClone and optimize mapping.
Line 136 uses JSON.parse(JSON.stringify(msg)) for deep cloning, which:
- Loses non-JSON-serializable data (functions, circular references)
- Converts
Dateobjects to strings - Removes
undefinedvalues - Has poor performance
Additionally, the computed maps over all messages even when only a few have selected variants.
Apply this diff to fix the cloning:
- const newMsg = JSON.parse(JSON.stringify(msg))
+ const newMsg = structuredClone(msg)Optionally optimize by early-returning when no variants are selected and filtering messages before mapping:
const variantAwareMessages = computed(() => {
const messages = getMessages()
const currentSelectedVariants = selectedVariantsMap.value
if (currentSelectedVariants.size === 0) {
return messages
}
return messages.map((msg) => {
- if (msg.role === 'assistant' && currentSelectedVariants.has(msg.id)) {
+ // Only process assistant messages with selected variants
+ if (msg.role !== 'assistant' || !currentSelectedVariants.has(msg.id)) {
+ return msg
+ }
+
- const selectedVariantId = currentSelectedVariants.get(msg.id)
+ const selectedVariantId = currentSelectedVariants.get(msg.id)
-
- if (!selectedVariantId) {
- return msg
- }
-
- const selectedVariant = (msg as AssistantMessage).variants?.find(
- (v) => v.id === selectedVariantId
- )
-
- if (selectedVariant) {
- const newMsg = JSON.parse(JSON.stringify(msg))
-
- newMsg.content = selectedVariant.content
- newMsg.usage = selectedVariant.usage
- newMsg.model_id = selectedVariant.model_id
- newMsg.model_provider = selectedVariant.model_provider
- newMsg.model_name = selectedVariant.model_name
-
- return newMsg
- }
- }
-
+ if (!selectedVariantId) {
+ return msg
+ }
+
+ const selectedVariant = (msg as AssistantMessage).variants?.find(
+ (v) => v.id === selectedVariantId
+ )
+
+ if (!selectedVariant) {
+ return msg
+ }
+
+ const newMsg = structuredClone(msg)
+ newMsg.content = selectedVariant.content
+ newMsg.usage = selectedVariant.usage
+ newMsg.model_id = selectedVariant.model_id
+ newMsg.model_provider = selectedVariant.model_provider
+ newMsg.model_name = selectedVariant.model_name
+
return msg
}) as AssistantMessage[] | UserMessage[]
})Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/renderer/src/stores/chat.ts around lines 115 to 150, replace the
JSON.parse(JSON.stringify(msg)) deep clone with structuredClone(msg) to preserve
non-JSON-serializable values and improve performance, and change the mapping so
you only clone/replace messages that are assistant messages with a selected
variant (leave others untouched) — keep the early-return when
selectedVariants.size === 0, find messages whose id exists in selectedVariants
and for those create a shallow copy via structuredClone, set
content/usage/model_* from the selected variant, and return the original msg for
all other cases; ensure TypeScript typings remain AssistantMessage|UserMessage
and, if structuredClone may be unavailable in the target runtime, add a small
fallback (e.g., use a safe clone utility) while preserving behavior.
src/renderer/src/stores/chat.ts
Outdated
| // 竞态? | ||
| await loadChatConfig() // 加载对话配置 | ||
| await loadMessages() |
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.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Investigate acknowledged race condition concern.
The comment "竞态?" (Race condition?) on line 1252 flags a potential issue where sequential loading of config and messages during activation could be disrupted by:
- Multiple rapid activation events
- Tab switching before loads complete
- Activation event for a different thread arriving mid-load
Run the following script to search for similar activation patterns and guard mechanisms in the codebase:
Consider adding a guard to verify the thread ID hasn't changed:
const threadIdBeforeLoad = msg.conversationId
await loadChatConfig()
if (getActiveThreadId() !== threadIdBeforeLoad) {
console.warn('Thread changed during activation, aborting message load')
return
}
await loadMessages()🏁 Script executed:
#!/bin/bash
# Search for activation handling and race condition guards
echo "=== Searching for activation event handlers ==="
rg -nP 'CONVERSATION_EVENTS\.ACTIVATED' -C3
echo -e "\n=== Searching for threadId/conversationId guards in async flows ==="
rg -nP '(getActiveThreadId|activeThreadId).*\!==.*\).*return' -C2
echo -e "\n=== Searching for similar race condition comments ==="
rg -nP '(race|竞态|concurrent|parallel).*\?' -C2Length of output: 286116
Abort activation loads if the active conversation/tab changed while awaiting.
The ACTIVATED handler (src/renderer/src/stores/chat.ts — window.electron.ipcRenderer.on(CONVERSATION_EVENTS.ACTIVATED, ...) around line 1235) does:
// 竞态?
await loadChatConfig()
await loadMessages()
Problem: if another activation (or tab switch) occurs while awaiting loadChatConfig(), loadMessages() can load the wrong conversation. Capture msg.conversationId (and tabId) before the async calls and verify the current active conversation/tab still matches after each await; abort if it doesn't. Example pattern:
const convBefore = msg.conversationId
await loadChatConfig()
if (currentActiveConversationId() !== convBefore) return
await loadMessages()
Use the same getter the store already uses for tab checks (the existing getTabId()/store accessor) to perform the verification.
🤖 Prompt for AI Agents
In src/renderer/src/stores/chat.ts around lines 1252 to 1254, the ACTIVATED
handler awaits loadChatConfig() and loadMessages() without guarding against race
switches; capture msg.conversationId and msg.tabId into local vars before the
awaits, after each await check the store's current active conversation/tab via
the existing getters (the same getTabId()/store accessor used elsewhere) and
return early if they no longer match the captured values so loadMessages() (and
any subsequent work) is aborted when the active conversation or tab changed.
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: 5
♻️ Duplicate comments (2)
src/renderer/src/stores/chat.ts (2)
115-150: Use structuredClone and avoid unnecessary cloning in variantAwareMessagesJSON cloning is lossy and slow. Also return early for non‑assistant or no selection.
- return messages.map((msg) => { - if (msg.role === 'assistant' && currentSelectedVariants.has(msg.id)) { + return messages.map((msg) => { + if (msg.role !== 'assistant' || !currentSelectedVariants.has(msg.id)) { + return msg + } + if (msg.role === 'assistant') { const selectedVariantId = currentSelectedVariants.get(msg.id) if (!selectedVariantId) { return msg } const selectedVariant = (msg as AssistantMessage).variants?.find( (v) => v.id === selectedVariantId ) if (selectedVariant) { - const newMsg = JSON.parse(JSON.stringify(msg)) + const newMsg = structuredClone(msg) newMsg.content = selectedVariant.content newMsg.usage = selectedVariant.usage newMsg.model_id = selectedVariant.model_id newMsg.model_provider = selectedVariant.model_provider newMsg.model_name = selectedVariant.model_name return newMsg } } return msg }) as AssistantMessage[] | UserMessage[]
1235-1254: Add race guard while awaiting ACTIVATED loads (thread may change mid‑await)Capture conversationId before awaits and verify it remains active after each await; abort if it changed.
- await loadChatConfig() // 加载对话配置 - await loadMessages() + const convoAtStart = msg.conversationId + await loadChatConfig() + if (getActiveThreadId() !== convoAtStart) return + await loadMessages() + if (getActiveThreadId() !== convoAtStart) return
🧹 Nitpick comments (6)
src/renderer/src/components/message/MessageItemAssistant.vue (1)
113-141: Translate inline comments to EnglishComments are in Chinese. Per repo guidelines, use English for logs and comments.
As per coding guidelines
Also applies to: 166-239, 312-337
src/main/presenter/threadPresenter/index.ts (3)
2033-2035: Replace JSON cloning with structuredClone; early‑exit when no matchJSON.parse(JSON.stringify(...)) is slow and lossy. Use structuredClone (Node 20+). Also avoid cloning when no selected variant is found.
Apply:
- if (selectedVariantsMap && Object.keys(selectedVariantsMap).length > 0) { - contextMessages = contextMessages.map((msg) => { + if (selectedVariantsMap && Object.keys(selectedVariantsMap).length > 0) { + contextMessages = contextMessages.map((msg) => { if (msg.role === 'assistant' && selectedVariantsMap[msg.id] && msg.variants) { const selectedVariantId = selectedVariantsMap[msg.id] const selectedVariant = msg.variants.find((v) => v.id === selectedVariantId) if (selectedVariant) { - const newMsg = JSON.parse(JSON.stringify(msg)) + const newMsg = structuredClone(msg) newMsg.content = selectedVariant.content newMsg.usage = selectedVariant.usage newMsg.model_id = selectedVariant.model_id newMsg.model_provider = selectedVariant.model_provider return newMsg } } return msg }) }Also applies to: 2081-2105
2961-2970: Forking with variant substitution: logic OK; prefer structuredClone for safetyFlow is sound: copy main history up to main target, substitute selected variant when provided, and insert with correct metadata. Consider structuredClone when deriving finalMsg to avoid accidental shared references.
- let finalMsg = msg + let finalMsg = msg if (msg.role === 'assistant' && selectedVariantsMap && selectedVariantsMap[msg.id]) { const selectedVariantId = selectedVariantsMap[msg.id] const variant = msg.variants?.find((v) => v.id === selectedVariantId) if (variant) { - finalMsg = variant + finalMsg = structuredClone(variant) } }Also applies to: 2981-3081
1-100: Translate logs/comments to EnglishLarge portions of comments/logs are Chinese. Please switch to English as per repo guidelines.
As per coding guidelines
Also applies to: 147-158, 296-306
src/renderer/src/stores/chat.ts (2)
189-190: Clear Map by replacing reference to trigger reactivityUsing .clear() won’t notify dependents.
- selectedVariantsMap.value.clear() + selectedVariantsMap.value = new Map()
851-887: Avoid double mutation; delegate to updateSelectedVariant onlyYou delete from the Map then call updateSelectedVariant, which replaces it again. Remove the direct delete to keep a single source of truth.
- if (selectedVariantsMap.value.has(messageId)) { - selectedVariantsMap.value.delete(messageId) - await updateSelectedVariant(messageId, null) - } + if (selectedVariantsMap.value.has(messageId)) { + await updateSelectedVariant(messageId, null) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
package.json(1 hunks)src/main/presenter/threadPresenter/index.ts(11 hunks)src/renderer/src/components/message/MessageItemAssistant.vue(4 hunks)src/renderer/src/stores/chat.ts(16 hunks)src/shared/types/presenters/legacy.presenters.d.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (26)
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/components/message/MessageItemAssistant.vuesrc/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/components/message/MessageItemAssistant.vuesrc/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/components/message/MessageItemAssistant.vuesrc/renderer/src/stores/chat.ts
src/renderer/src/**/*.vue
📄 CodeRabbit inference engine (.cursor/rules/vue-best-practices.mdc)
Use scoped styles to prevent CSS conflicts between components
Files:
src/renderer/src/components/message/MessageItemAssistant.vue
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/components/message/MessageItemAssistant.vuesrc/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/components/message/MessageItemAssistant.vuesrc/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/components/message/MessageItemAssistant.vuesrc/renderer/src/stores/chat.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/renderer/src/components/message/MessageItemAssistant.vuesrc/shared/types/presenters/legacy.presenters.d.tssrc/renderer/src/stores/chat.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)
Files:
src/renderer/src/components/message/MessageItemAssistant.vuesrc/shared/types/presenters/legacy.presenters.d.tssrc/renderer/src/stores/chat.tssrc/main/presenter/threadPresenter/index.ts
src/renderer/{src,shell,floating}/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
src/renderer/{src,shell,floating}/**/*.vue: Use Vue 3 Composition API for all components
All user-facing strings must use i18n keys via vue-i18n (no hard-coded UI strings)
Use Tailwind CSS utilities and ensure styles are scoped in Vue components
Files:
src/renderer/src/components/message/MessageItemAssistant.vue
src/renderer/src/components/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Organize UI components by feature within src/renderer/src/
Files:
src/renderer/src/components/message/MessageItemAssistant.vue
src/renderer/src/**
📄 CodeRabbit inference engine (AGENTS.md)
Put application code for the Vue app under src/renderer/src (components, stores, views, i18n, lib)
Files:
src/renderer/src/components/message/MessageItemAssistant.vuesrc/renderer/src/stores/chat.ts
src/renderer/src/**/*.{vue,ts}
📄 CodeRabbit inference engine (AGENTS.md)
All user-facing strings in the renderer must use vue-i18n keys defined in src/renderer/src/i18n
Files:
src/renderer/src/components/message/MessageItemAssistant.vuesrc/renderer/src/stores/chat.ts
**/*.{js,jsx,ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Prettier formatting: single quotes, no semicolons, max width 100
Files:
src/renderer/src/components/message/MessageItemAssistant.vuesrc/shared/types/presenters/legacy.presenters.d.tssrc/renderer/src/stores/chat.tssrc/main/presenter/threadPresenter/index.ts
src/renderer/**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
Name Vue components in PascalCase (e.g., ChatInput.vue)
Files:
src/renderer/src/components/message/MessageItemAssistant.vue
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
**/*.{js,jsx,ts,tsx}: Use OxLint for JS/TS code; pre-commit hooks run lint-staged and typecheck
Use camelCase for variables and functions
Use PascalCase for types and classes
Use SCREAMING_SNAKE_CASE for constants
Files:
src/shared/types/presenters/legacy.presenters.d.tssrc/renderer/src/stores/chat.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/shared/types/presenters/legacy.presenters.d.tssrc/renderer/src/stores/chat.tssrc/main/presenter/threadPresenter/index.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
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)
Store shared TypeScript types/utilities in src/shared/
Files:
src/shared/types/presenters/legacy.presenters.d.ts
src/{main,renderer}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
src/{main,renderer}/**/*.ts: Use context isolation for improved security
Implement proper inter-process communication (IPC) patterns
Optimize application startup time with lazy loading
Implement proper error handling and logging for debugging
Files:
src/renderer/src/stores/chat.tssrc/main/presenter/threadPresenter/index.ts
src/renderer/src/stores/**/*.{vue,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/pinia-best-practices.mdc)
src/renderer/src/stores/**/*.{vue,ts,tsx,js,jsx}: Use modules to organize related state and actions
Implement proper state persistence for maintaining data across sessions
Use getters for computed state properties
Utilize actions for side effects and asynchronous operations
Keep the store focused on global state, not component-specific data
Files:
src/renderer/src/stores/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/index.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/presenter/threadPresenter/index.ts
src/main/**
📄 CodeRabbit inference engine (AGENTS.md)
Place all Electron main-process code under src/main/
Files:
src/main/presenter/threadPresenter/index.ts
src/main/presenter/**
📄 CodeRabbit inference engine (AGENTS.md)
src/main/presenter/**: Organize main-process presenters under src/main/presenter/ (Window/Tab/Thread/Mcp/Config/LLMProvider)
Follow the Presenter pattern for main-process modules
Files:
src/main/presenter/threadPresenter/index.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: neoragex2002
PR: ThinkInAIXYZ/deepchat#550
File: src/main/presenter/mcpPresenter/inMemoryServers/meetingServer.ts:258-324
Timestamp: 2025-06-21T15:53:34.641Z
Learning: User neoragex2002 prefers concise, brief responses rather than chatty or verbose communication. Avoid lengthy explanations, excessive enthusiasm, or ASCII art in responses.
📚 Learning: 2025-09-16T07:06:30.939Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: AGENTS.md:0-0
Timestamp: 2025-09-16T07:06:30.939Z
Learning: Use pnpm run dev (or dev:inspect / dev:linux) for development with HMR; pnpm start for preview
Applied to files:
package.json
🧬 Code graph analysis (3)
src/shared/types/presenters/legacy.presenters.d.ts (2)
src/shared/types/presenters/thread.presenter.d.ts (1)
CONVERSATION_SETTINGS(15-30)src/shared/chat.d.ts (1)
AssistantMessage(39-42)
src/renderer/src/stores/chat.ts (2)
src/shared/chat.d.ts (2)
AssistantMessage(39-42)UserMessage(34-37)src/renderer/src/events.ts (1)
CONVERSATION_EVENTS(31-37)
src/main/presenter/threadPresenter/index.ts (2)
src/shared/types/presenters/legacy.presenters.d.ts (1)
CONVERSATION_SETTINGS(709-725)src/shared/types/presenters/thread.presenter.d.ts (1)
CONVERSATION_SETTINGS(15-30)
⏰ 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 (5)
src/renderer/src/components/message/MessageItemAssistant.vue (2)
160-161: Event API change: replaced scrollToBottom with variantChangedEnsure all parent listeners are updated to handle variantChanged and no code relies on the removed scrollToBottom event.
312-337: Variant navigation: good single‑source of truth via storeprev/next computes bounds, no local state mutation, updates store and emits variantChanged. Looks correct.
src/shared/types/presenters/legacy.presenters.d.ts (1)
762-765: API surface alignment looks consistentforkConversation/startStreamCompletion/regenerateFromUserMessage/continueStreamCompletion now carry selectedVariantsMap. Matches PR goals.
Also applies to: 787-797, 826-831
src/main/presenter/threadPresenter/index.ts (1)
1697-1701: Plumbed selectedVariantsMap into startStreamCompletionAPI change is correct and used downstream in context prep.
src/renderer/src/stores/chat.ts (1)
820-827: Rehydrate Map: good — keep replacing referenceThis correctly assigns a new Map instance so dependents update.
| // 用于管理当前激活会话的 selectedVariantsMap | ||
| const selectedVariantsMap = ref<Map<string, string>>(new Map()) | ||
|
|
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.
Map reactivity bug: in‑place mutations on a ref(Map) won’t trigger recompute
Vue doesn’t track Map.set/delete inside a ref unless you replace the ref’s value. currentVariantIndex/variantAwareMessages may not update after selection changes.
Apply:
-import { ref, computed, onMounted } from 'vue'
+import { ref, computed, onMounted, shallowRef } from 'vue'
-const selectedVariantsMap = ref<Map<string, string>>(new Map())
+// Use shallowRef and always replace the Map to trigger reactivity
+const selectedVariantsMap = shallowRef<Map<string, string>>(new Map())And update all mutations to replace the Map (see updateSelectedVariant/clearActiveThread diffs below).
🤖 Prompt for AI Agents
In src/renderer/src/stores/chat.ts around lines 80–82, the ref<Map>
selectedVariantsMap is being mutated in-place (Map.set/delete), which Vue won't
track; replace in-place mutations with assignments to a new Map so reactivity
triggers. For any code that adds/updates: const m = new
Map(selectedVariantsMap.value); m.set(key, value); selectedVariantsMap.value =
m. For deletes: const m = new Map(selectedVariantsMap.value); m.delete(key);
selectedVariantsMap.value = m. Apply this pattern in updateSelectedVariant,
clearActiveThread, and any other places that modify selectedVariantsMap.
| const updateSelectedVariant = async (mainMessageId: string, selectedVariantId: string | null) => { | ||
| const activeThreadId = getActiveThreadId() | ||
| if (!activeThreadId) return | ||
|
|
||
| // 更新内存中的 Map | ||
| if (selectedVariantId && selectedVariantId !== mainMessageId) { | ||
| selectedVariantsMap.value.set(mainMessageId, selectedVariantId) | ||
| } else { | ||
| selectedVariantsMap.value.delete(mainMessageId) | ||
| } | ||
|
|
||
| // 同步更新 chatConfig 内的 selectedVariantsMap | ||
| if (chatConfig.value) { | ||
| chatConfig.value.selectedVariantsMap = Object.fromEntries(selectedVariantsMap.value) | ||
| } | ||
|
|
||
| // 持久化到后端 | ||
| try { | ||
| await threadP.updateConversationSettings(activeThreadId, { | ||
| selectedVariantsMap: Object.fromEntries(selectedVariantsMap.value) | ||
| }) | ||
| } catch (error) { | ||
| console.error('Failed to update selected variant:', error) | ||
| } | ||
| } |
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.
Replace Map instead of mutating in updateSelectedVariant
Ensure reactivity by creating a new Map and assigning it; also sync chatConfig from the new Map.
- const updateSelectedVariant = async (mainMessageId: string, selectedVariantId: string | null) => {
+ const updateSelectedVariant = async (mainMessageId: string, selectedVariantId: string | null) => {
const activeThreadId = getActiveThreadId()
if (!activeThreadId) return
- // 更新内存中的 Map
- if (selectedVariantId && selectedVariantId !== mainMessageId) {
- selectedVariantsMap.value.set(mainMessageId, selectedVariantId)
- } else {
- selectedVariantsMap.value.delete(mainMessageId)
- }
+ // Build a new Map to trigger reactivity
+ const next = new Map(selectedVariantsMap.value)
+ if (selectedVariantId && selectedVariantId !== mainMessageId) {
+ next.set(mainMessageId, selectedVariantId)
+ } else {
+ next.delete(mainMessageId)
+ }
+ selectedVariantsMap.value = next
- // 同步更新 chatConfig 内的 selectedVariantsMap
- if (chatConfig.value) {
- chatConfig.value.selectedVariantsMap = Object.fromEntries(selectedVariantsMap.value)
- }
+ // Sync chatConfig from the new Map
+ if (chatConfig.value) {
+ chatConfig.value.selectedVariantsMap = Object.fromEntries(next)
+ }
// 持久化到后端
try {
- await threadP.updateConversationSettings(activeThreadId, {
- selectedVariantsMap: Object.fromEntries(selectedVariantsMap.value)
- })
+ await threadP.updateConversationSettings(activeThreadId, {
+ selectedVariantsMap: Object.fromEntries(next)
+ })
} catch (error) {
console.error('Failed to update selected variant:', error)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const updateSelectedVariant = async (mainMessageId: string, selectedVariantId: string | null) => { | |
| const activeThreadId = getActiveThreadId() | |
| if (!activeThreadId) return | |
| // 更新内存中的 Map | |
| if (selectedVariantId && selectedVariantId !== mainMessageId) { | |
| selectedVariantsMap.value.set(mainMessageId, selectedVariantId) | |
| } else { | |
| selectedVariantsMap.value.delete(mainMessageId) | |
| } | |
| // 同步更新 chatConfig 内的 selectedVariantsMap | |
| if (chatConfig.value) { | |
| chatConfig.value.selectedVariantsMap = Object.fromEntries(selectedVariantsMap.value) | |
| } | |
| // 持久化到后端 | |
| try { | |
| await threadP.updateConversationSettings(activeThreadId, { | |
| selectedVariantsMap: Object.fromEntries(selectedVariantsMap.value) | |
| }) | |
| } catch (error) { | |
| console.error('Failed to update selected variant:', error) | |
| } | |
| } | |
| const updateSelectedVariant = async (mainMessageId: string, selectedVariantId: string | null) => { | |
| const activeThreadId = getActiveThreadId() | |
| if (!activeThreadId) return | |
| // Build a new Map to trigger reactivity | |
| const next = new Map(selectedVariantsMap.value) | |
| if (selectedVariantId && selectedVariantId !== mainMessageId) { | |
| next.set(mainMessageId, selectedVariantId) | |
| } else { | |
| next.delete(mainMessageId) | |
| } | |
| selectedVariantsMap.value = next | |
| // Sync chatConfig from the new Map | |
| if (chatConfig.value) { | |
| chatConfig.value.selectedVariantsMap = Object.fromEntries(next) | |
| } | |
| // Persist to backend | |
| try { | |
| await threadP.updateConversationSettings(activeThreadId, { | |
| selectedVariantsMap: Object.fromEntries(next) | |
| }) | |
| } catch (error) { | |
| console.error('Failed to update selected variant:', error) | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/renderer/src/stores/chat.ts around lines 1061 to 1085, the function
mutates the existing selectedVariantsMap which can break reactivity; instead
create a new Map copying the existing entries, apply the add/delete on that new
Map, then assign selectedVariantsMap.value to the new Map, update
chatConfig.value.selectedVariantsMap using Object.fromEntries(newMap) (not from
the old map), and persist using threadP.updateConversationSettings with
Object.fromEntries(newMap); ensure you no longer call .set/.delete on the old
map in place.
| selectedVariantsMap?: Record<string, string> | ||
| } |
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.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Add selectedVariantsMap to all public presenter typings to avoid drift
This file defines CONVERSATION_SETTINGS.selectedVariantsMap, but thread.presenter.d.ts (snippet) doesn’t. Align both to prevent type drift between imports.
Run to verify and locate all occurrences:
🏁 Script executed:
#!/bin/bash
rg -nP --type=ts 'selectedVariantsMap\?\s*:\s*Record<string,\s*string>' -C2
rg -nP --type=ts 'startStreamCompletion\([^\)]*selectedVariantsMap\?' -C2Length of output: 4317
🏁 Script executed:
#!/bin/bash
echo "Presenter type declaration files:"
find src/shared/types/presenters -type f -name "*.d.ts"
echo
echo "Searching for selectedVariantsMap in thread.presenter.d.ts:"
rg -nP --context 2 'selectedVariantsMap' src/shared/types/presenters/thread.presenter.d.ts || echo "No matches in thread.presenter.d.ts"Length of output: 375
Align thread.presenter.d.ts with legacy.presenters.d.ts by adding selectedVariantsMap
In src/shared/types/presenters/thread.presenter.d.ts, add the optional parameter
selectedVariantsMap?: Record<string, string>to each IThreadPresenter method signature to match legacy.presenters.d.ts.
🤖 Prompt for AI Agents
In src/shared/types/presenters/legacy.presenters.d.ts around lines 724-725, the
legacy presenter defines selectedVariantsMap?: Record<string, string>; update
src/shared/types/presenters/thread.presenter.d.ts by adding the same optional
parameter selectedVariantsMap?: Record<string, string> to every IThreadPresenter
method signature so the thread presenter API matches the legacy presenter;
ensure the parameter is added at the end of each method's parameter list and
update any related overloads/types to keep TypeScript types consistent.
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
package.json(2 hunks)src/main/presenter/threadPresenter/index.ts(13 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 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和注释使用英文书写
**/*.{js,jsx,ts,tsx}: Use OxLint for JS/TS code; pre-commit hooks run lint-staged and typecheck
Use camelCase for variables and functions
Use PascalCase for types and classes
Use SCREAMING_SNAKE_CASE for constants
Files:
src/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/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/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/index.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/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/index.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Files:
src/main/presenter/threadPresenter/index.ts
src/main/**
📄 CodeRabbit inference engine (AGENTS.md)
Place all Electron main-process code under src/main/
Files:
src/main/presenter/threadPresenter/index.ts
src/main/presenter/**
📄 CodeRabbit inference engine (AGENTS.md)
src/main/presenter/**: Organize main-process presenters under src/main/presenter/ (Window/Tab/Thread/Mcp/Config/LLMProvider)
Follow the Presenter pattern for main-process modules
Files:
src/main/presenter/threadPresenter/index.ts
**/*.{js,jsx,ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Prettier formatting: single quotes, no semicolons, max width 100
Files:
src/main/presenter/threadPresenter/index.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: neoragex2002
PR: ThinkInAIXYZ/deepchat#550
File: src/main/presenter/mcpPresenter/inMemoryServers/meetingServer.ts:258-324
Timestamp: 2025-06-21T15:53:34.641Z
Learning: User neoragex2002 prefers concise, brief responses rather than chatty or verbose communication. Avoid lengthy explanations, excessive enthusiasm, or ASCII art in responses.
🧬 Code graph analysis (1)
src/main/presenter/threadPresenter/index.ts (4)
src/shared/types/core/chat.ts (1)
UserMessageContent(23-37)src/main/presenter/threadPresenter/fileContext.ts (1)
getFileContext(3-22)src/shared/types/presenters/legacy.presenters.d.ts (2)
CONVERSATION_SETTINGS(709-725)MESSAGE_METADATA(851-863)src/shared/types/presenters/thread.presenter.d.ts (2)
CONVERSATION_SETTINGS(15-30)MESSAGE_METADATA(46-58)
🔇 Additional comments (8)
src/main/presenter/threadPresenter/index.ts (8)
1697-1716: startStreamCompletion now accepts selectedVariantsMap — LGTMParameter added and plumbed into prepareConversationContext. No issues spotted.
1831-1835: continueStreamCompletion variant‑aware — LGTMSignature + propagation look correct.
Also applies to: 1901-1904
2716-2719: Regenerate now propagates selectedVariantsMap — LGTMFlow is variant‑aware and calls startStreamCompletion with the map.
Also applies to: 2758-2758
2999-3002: Do not inherit variant selections on fork — LGTMResetting selectedVariantsMap for new conversation is correct.
2876-2894: Summary uses variant‑aware messages — LGTMConsistent with persistence; includes model_name too.
3326-3345: Export builds variant‑aware message list — LGTMCorrectly substitutes content and model metadata before format.
3359-3366: Export uses variant‑aware messages across formats — LGTMPassing the substituted list into all exporters is correct.
2033-2035: Invalid fallback: settings lackselectedVariantsMap.CONVERSATION_SETTINGSdoes not includeselectedVariantsMap, soconversation.settings.selectedVariantsMapwill not compile—either extend the settings type or remove this fallback.Likely an incorrect or invalid review comment.
| let finalMsg = msg | ||
| // 当循环到我们关心的主消息时,检查 selectedVariantsMap | ||
| if (msg.role === 'assistant' && selectedVariantsMap && selectedVariantsMap[msg.id]) { | ||
| const selectedVariantId = selectedVariantsMap[msg.id] | ||
| const variant = msg.variants?.find((v) => v.id === selectedVariantId) | ||
| if (variant) { | ||
| finalMsg = variant // 使用选定的变体对象进行复制 | ||
| } | ||
| } | ||
|
|
||
| const metadata: MESSAGE_METADATA = { | ||
| totalTokens: msg.usage?.total_tokens || 0, | ||
| totalTokens: finalMsg.usage?.total_tokens || 0, | ||
| generationTime: 0, | ||
| firstTokenTime: 0, | ||
| tokensPerSecond: 0, | ||
| contextUsage: 0, | ||
| inputTokens: msg.usage?.input_tokens || 0, | ||
| outputTokens: msg.usage?.output_tokens || 0, | ||
| ...(msg.model_id ? { model: msg.model_id } : {}), | ||
| ...(msg.model_provider ? { provider: msg.model_provider } : {}) | ||
| inputTokens: finalMsg.usage?.input_tokens || 0, | ||
| outputTokens: finalMsg.usage?.output_tokens || 0, | ||
| ...(finalMsg.model_id ? { model: finalMsg.model_id } : {}), | ||
| ...(finalMsg.model_provider ? { provider: finalMsg.model_provider } : {}) | ||
| } | ||
|
|
||
| // 计算token数量 | ||
| const tokenCount = msg.usage?.total_tokens || 0 | ||
|
|
||
| // 内容处理(确保是字符串) | ||
| const content = typeof msg.content === 'string' ? msg.content : JSON.stringify(msg.content) | ||
| const tokenCount = finalMsg.usage?.total_tokens || 0 | ||
| const content = | ||
| typeof finalMsg.content === 'string' ? finalMsg.content : JSON.stringify(finalMsg.content) | ||
|
|
||
| // 插入消息记录,暂时不设置父ID | ||
| const newMessageId = await this.sqlitePresenter.insertMessage( | ||
| newConversationId, // 新会话ID | ||
| content, // 内容 | ||
| msg.role, // 角色 | ||
| '', // 暂时无父消息ID,稍后更新 | ||
| JSON.stringify(metadata), // 元数据 | ||
| orderSeq, // 序号 | ||
| tokenCount, // token数 | ||
| 'sent', // 状态固定为sent | ||
| 0, // 不是上下文边界 | ||
| 0 // 不是变体 | ||
| newConversationId, | ||
| content, | ||
| finalMsg.role, | ||
| '', | ||
| JSON.stringify(metadata), | ||
| orderSeq, | ||
| tokenCount, | ||
| 'sent', | ||
| 0, | ||
| 0 | ||
| ) |
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.
Fix metadata field mapping and add role fallback when copying variants
Likely usage keys are prompt_tokens/completion_tokens; current input_tokens/output_tokens will be 0. Also guard missing role from variant objects.
- let finalMsg = msg
+ let finalMsg = msg
@@
- const metadata: MESSAGE_METADATA = {
- totalTokens: finalMsg.usage?.total_tokens || 0,
- generationTime: 0,
- firstTokenTime: 0,
- tokensPerSecond: 0,
- contextUsage: 0,
- inputTokens: finalMsg.usage?.input_tokens || 0,
- outputTokens: finalMsg.usage?.output_tokens || 0,
- ...(finalMsg.model_id ? { model: finalMsg.model_id } : {}),
- ...(finalMsg.model_provider ? { provider: finalMsg.model_provider } : {})
- }
+ const u = (finalMsg as any).usage || {}
+ const metadata: MESSAGE_METADATA = {
+ totalTokens: u.total_tokens || 0,
+ inputTokens: u.prompt_tokens ?? u.input_tokens ?? 0,
+ outputTokens: u.completion_tokens ?? u.output_tokens ?? 0,
+ generationTime: 0,
+ firstTokenTime: 0,
+ tokensPerSecond: 0,
+ contextUsage: 0,
+ ...((finalMsg as any).model_id ? { model: (finalMsg as any).model_id } : {}),
+ ...((finalMsg as any).model_provider ? { provider: (finalMsg as any).model_provider } : {})
+ }
@@
- const newMessageId = await this.sqlitePresenter.insertMessage(
- newConversationId,
- content,
- finalMsg.role,
+ const newMessageId = await this.sqlitePresenter.insertMessage(
+ newConversationId,
+ content,
+ (finalMsg as any).role || msg.role || 'assistant',
'',
JSON.stringify(metadata),
orderSeq,
tokenCount,
'sent',
0,
0
)Also applies to: 3073-3082, 3084-3087
|
@codex review |
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| return threads.value.flatMap((t) => t.dtThreads).find((t) => t.id === getActiveThreadId()) | ||
| }) | ||
|
|
||
| const variantAwareMessages = computed(() => { | ||
| const messages = getMessages() | ||
| const currentSelectedVariants = selectedVariantsMap.value | ||
|
|
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.
Variant selection state never re-renders
The new per-conversation variant selection is stored in selectedVariantsMap as a ref<Map<string,string>>, but both the map initialization (lines 81‑118) and the update action (lines 1061‑1074) mutate the same Map instance via .set/.delete without reassigning or calling triggerRef. Vue/Pinia will not emit change notifications when the contents of a Map are mutated in place, so computed properties like variantAwareMessages and the currentVariantIndex computed in MessageItemAssistant.vue never recompute after a user clicks “next/prev” or when a new variant is generated. As a result the UI stays on the old variant even though the store state was logically updated. Replace the Map with a reactive object, or reassign a new Map (or call triggerRef) whenever its contents change so that dependant components react.
Useful? React with 👍 / 👎.
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.
vue3不存在上述问题, codex不行啊~
fix: 系统性修复"历史上下文构建中对消息变体选择的全局性忽略"BUG
fix: 用户在UI界面中的会话变体消息选择实现永固化(后向兼容)
fix: prepareMsg/forkMsg/continueMsg/starStreaming等上下文构建全修正,以用户UI界面所选择的变体为准构建会话历史
fix: UI切换变体时修正视口滚动,同步切换消息导航内容
fix: 修正exportConversation/summaryConversation忽略变体选择的bug
fix: 修正pnpm dev命令,支持跨平台环境变量
Summary by CodeRabbit