fix: fix retry message #1269
fix: fix retry message #1269
Conversation
📝 WalkthroughWalkthroughThis PR refactors variant and message retry handling to use message IDs instead of array indices throughout the codebase. Key changes include converting selectedVariantsMap from a Map to a plain object record, updating the retry composable and store to accept messageId parameters, and modifying the main message query to enforce assistant role filtering and descending timestamp ordering. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/src/components/message/MessageToolbar.vue (1)
59-59: Inconsistent handling of undefinedcurrentVariantIndex.Line 59 uses strict equality
currentVariantIndex === 0, but whencurrentVariantIndexisundefined, this evaluates tofalse, leaving the "prev" button enabled incorrectly. Lines 71 and 77 correctly use nullish coalescing. Apply the same pattern here for consistency.🐛 Proposed fix
- :disabled="currentVariantIndex === 0" + :disabled="(currentVariantIndex ?? 0) === 0"
🧹 Nitpick comments (2)
src/renderer/src/stores/chat.ts (2)
691-735: Potential duplicateupdateSelectedVariantcalls.The function calls
updateSelectedVarianttwice:
- Line 715:
await updateSelectedVariant(parentMsg.id, aiResponseMessage.id)- Lines 725-726:
await updateSelectedVariant(mainMessage.id, aiResponseMessage.id)If
parentMsg.idequalsmainMessage.id(which it should for the same parent), this results in redundant persistence operations. Consider consolidating these calls or adding a guard to avoid the second call when the first already succeeded.♻️ Suggested fix
await loadMessages() - if (aiResponseMessage.parentId) { - const mainMessage = await threadP.getMainMessageByParentId( - getActiveThreadId()!, - aiResponseMessage.parentId - ) - if (mainMessage) { - await updateSelectedVariant(mainMessage.id, aiResponseMessage.id) - } - } generatingThreadIds.value.add(getActiveThreadId()!)The first
updateSelectedVariantcall at line 715 should be sufficient sinceparentMsg.idis the main message ID.
1556-1581: Consider the timing implications of the 100ms delay.The
setTimeoutwith 100ms delay for resettingisUpdatingVariantis a simple debounce approach, but it has potential edge cases:
- If the user rapidly switches variants, multiple calls could overlap and the flag might be reset prematurely by an earlier call's timeout
- The 100ms is arbitrary and may not align with actual IPC round-trip times
For robustness, consider tracking the update with a counter or promise instead:
♻️ Alternative approach using a counter
- let isUpdatingVariant = false + let variantUpdateCounter = 0 const updateSelectedVariant = async (mainMessageId: string, selectedVariantId: string | null) => { const activeThreadId = getActiveThreadId() if (!activeThreadId) return - isUpdatingVariant = true + const currentUpdate = ++variantUpdateCounter // ... rest of function ... } finally { - setTimeout(() => { - isUpdatingVariant = false - }, 100) + // Only clear if this is the latest update + if (variantUpdateCounter === currentUpdate) { + variantUpdateCounter = 0 + } } }Then check
variantUpdateCounter > 0instead ofisUpdatingVariant.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/presenter/sqlitePresenter/tables/messages.tssrc/renderer/src/components/message/MessageItemAssistant.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/components/message/MessageToolbar.vuesrc/renderer/src/composables/message/useMessageRetry.tssrc/renderer/src/stores/chat.ts
🧰 Additional context used
📓 Path-based instructions (19)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for logs and comments in TypeScript/JavaScript code
Files:
src/main/presenter/sqlitePresenter/tables/messages.tssrc/renderer/src/stores/chat.tssrc/renderer/src/composables/message/useMessageRetry.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript with strict type checking enabled
Use OxLint for linting JavaScript and TypeScript files; ensure lint-staged hooks and typecheck pass before commits
Files:
src/main/presenter/sqlitePresenter/tables/messages.tssrc/renderer/src/stores/chat.tssrc/renderer/src/composables/message/useMessageRetry.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
src/main/presenter/**/*.ts: Use EventBus to broadcast events from main to renderer viamainWindow.webContents.send()
Implement one presenter per functional domain in the main process
Files:
src/main/presenter/sqlitePresenter/tables/messages.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
src/main/**/*.ts: Use EventBus fromsrc/main/eventbus.tsfor decoupled inter-process communication
Context isolation must be enabled with preload scripts for secure IPC communicationElectron main process code should reside in
src/main/, with presenters organized inpresenter/subdirectory (Window, Tab, Thread, Mcp, Config, LLMProvider), and app events managed viaeventbus.ts
Files:
src/main/presenter/sqlitePresenter/tables/messages.ts
**/*.{js,ts,tsx,jsx,vue,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
All logs and comments must be in English
Files:
src/main/presenter/sqlitePresenter/tables/messages.tssrc/renderer/src/components/message/MessageToolbar.vuesrc/renderer/src/components/message/MessageItemAssistant.vuesrc/renderer/src/stores/chat.tssrc/renderer/src/composables/message/useMessageRetry.tssrc/renderer/src/components/message/MessageList.vue
**/*.{js,ts,tsx,jsx,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
Use OxLint as the linter
Files:
src/main/presenter/sqlitePresenter/tables/messages.tssrc/renderer/src/stores/chat.tssrc/renderer/src/composables/message/useMessageRetry.ts
**/*.{js,ts,tsx,jsx,vue,json,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
Use Prettier as the code formatter
Files:
src/main/presenter/sqlitePresenter/tables/messages.tssrc/renderer/src/components/message/MessageToolbar.vuesrc/renderer/src/components/message/MessageItemAssistant.vuesrc/renderer/src/stores/chat.tssrc/renderer/src/composables/message/useMessageRetry.tssrc/renderer/src/components/message/MessageList.vue
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,vue}: Use camelCase for variable and function names; use PascalCase for types and classes; use SCREAMING_SNAKE_CASE for constants
Configure Prettier with single quotes, no semicolons, and line width of 100 characters. Runpnpm run formatafter completing features
Files:
src/main/presenter/sqlitePresenter/tables/messages.tssrc/renderer/src/components/message/MessageToolbar.vuesrc/renderer/src/components/message/MessageItemAssistant.vuesrc/renderer/src/stores/chat.tssrc/renderer/src/composables/message/useMessageRetry.tssrc/renderer/src/components/message/MessageList.vue
src/renderer/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
src/renderer/**/*.vue: Use Vue 3 Composition API for all components
Use Tailwind CSS for styling with scoped styles
All user-facing strings must use i18n keys via vue-i18n
Files:
src/renderer/src/components/message/MessageToolbar.vuesrc/renderer/src/components/message/MessageItemAssistant.vuesrc/renderer/src/components/message/MessageList.vue
src/renderer/src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
usePresenter.tscomposable for renderer-to-main IPC communication via direct presenter method callsEnsure all code comments are in English and all log messages are in English, with no non-English text in code comments or console statements
Use VueUse composables for common utilities like
useLocalStorage,useClipboard,useDebounceFnVue 3 renderer app code should be organized in
src/renderer/srcwith subdirectories forcomponents/,stores/,views/,i18n/, andlib/
Files:
src/renderer/src/components/message/MessageToolbar.vuesrc/renderer/src/components/message/MessageItemAssistant.vuesrc/renderer/src/stores/chat.tssrc/renderer/src/composables/message/useMessageRetry.tssrc/renderer/src/components/message/MessageList.vue
src/renderer/src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
src/renderer/src/**/*.{vue,ts,tsx}: Use vue-i18n framework for internationalization located at src/renderer/src/i18n/
All user-facing strings must use i18n keys, not hardcoded text
src/renderer/src/**/*.{vue,ts,tsx}: Usereffor primitives and references,reactivefor objects in Vue 3 Composition API
Prefercomputedproperties over methods for derived state in Vue components
Import Shadcn Vue components from@/shadcn/components/ui/path alias
Use thecn()utility function combining clsx and tailwind-merge for dynamic Tailwind classes
UsedefineAsyncComponent()for lazy loading heavy Vue components
Use TypeScript for all Vue components and composables with explicit type annotations
Define TypeScript interfaces for Vue component props and data structures
UseusePresentercomposable for main process communication instead of direct IPC calls
Files:
src/renderer/src/components/message/MessageToolbar.vuesrc/renderer/src/components/message/MessageItemAssistant.vuesrc/renderer/src/stores/chat.tssrc/renderer/src/composables/message/useMessageRetry.tssrc/renderer/src/components/message/MessageList.vue
src/renderer/src/**/*.vue
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
Import useI18n from vue-i18n in Vue components to access translation functions t and locale
src/renderer/src/**/*.vue: Use<script setup>syntax for concise Vue 3 component definitions with Composition API
Define props and emits explicitly in Vue components usingdefinePropsanddefineEmitswith TypeScript interfaces
Useprovide/injectfor dependency injection in Vue components instead of prop drilling
Use Tailwind CSS for all styling instead of writing scoped CSS files
Use mobile-first responsive design approach with Tailwind breakpoints
Use Iconify Vue with lucide icons as primary choice, following patternlucide:{icon-name}
Usev-memodirective for memoizing expensive computations in templates
Usev-oncedirective for rendering static content without reactivity updates
Use virtual scrolling withRecycleScrollercomponent for rendering long lists
Subscribe to events usingrendererEvents.on()and unsubscribe inonUnmountedlifecycle hook
Files:
src/renderer/src/components/message/MessageToolbar.vuesrc/renderer/src/components/message/MessageItemAssistant.vuesrc/renderer/src/components/message/MessageList.vue
src/renderer/src/components/**/*.vue
📄 CodeRabbit inference engine (.cursor/rules/vue-stack-guide.mdc)
Name Vue components using PascalCase (e.g.,
ChatInput.vue,MessageItemUser.vue)
Files:
src/renderer/src/components/message/MessageToolbar.vuesrc/renderer/src/components/message/MessageItemAssistant.vuesrc/renderer/src/components/message/MessageList.vue
**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
Vue components must be named in PascalCase (e.g.,
ChatInput.vue) and use Vue 3 Composition API with Pinia for state management and Tailwind for styling
Files:
src/renderer/src/components/message/MessageToolbar.vuesrc/renderer/src/components/message/MessageItemAssistant.vuesrc/renderer/src/components/message/MessageList.vue
src/renderer/src/**/stores/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use Pinia for frontend state management
Files:
src/renderer/src/stores/chat.ts
src/renderer/src/stores/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/vue-stack-guide.mdc)
src/renderer/src/stores/**/*.ts: Use Setup Store syntax withdefineStorefunction pattern in Pinia stores
Use getters (computed properties) for derived state in Pinia stores
Keep Pinia store actions focused on state mutations and async operations
Files:
src/renderer/src/stores/chat.ts
src/renderer/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/vue-stack-guide.mdc)
Use class-variance-authority (CVA) for defining component variants with Tailwind classes
Files:
src/renderer/src/stores/chat.tssrc/renderer/src/composables/message/useMessageRetry.ts
src/renderer/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/vue-stack-guide.mdc)
src/renderer/src/**/*.{ts,tsx}: UseshallowRefandshallowReactivefor optimizing reactivity with large objects
Prefertypeoverinterfacein TypeScript unless using inheritance withextends
Files:
src/renderer/src/stores/chat.tssrc/renderer/src/composables/message/useMessageRetry.ts
src/renderer/src/composables/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/vue-stack-guide.mdc)
Name composables using camelCase with
useprefix (e.g.,useChatState.ts,useMessageList.ts)
Files:
src/renderer/src/composables/message/useMessageRetry.ts
🧠 Learnings (6)
📚 Learning: 2026-01-05T02:41:31.661Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-stack-guide.mdc:0-0
Timestamp: 2026-01-05T02:41:31.661Z
Learning: Applies to src/renderer/src/**/*.{ts,tsx,js,jsx} : Use class-variance-authority (CVA) for defining component variants with Tailwind classes
Applied to files:
src/renderer/src/components/message/MessageToolbar.vue
📚 Learning: 2025-06-21T15:49:17.044Z
Learnt from: neoragex2002
Repo: ThinkInAIXYZ/deepchat PR: 550
File: src/renderer/src/stores/chat.ts:1011-1035
Timestamp: 2025-06-21T15:49:17.044Z
Learning: In src/renderer/src/stores/chat.ts, the user prefers to keep both `text` and `content` properties in the `handleMeetingInstruction` function's `sendMessage` call, even though they are redundant, rather than removing the `content` property.
Applied to files:
src/renderer/src/stores/chat.ts
📚 Learning: 2026-01-05T02:41:31.661Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-stack-guide.mdc:0-0
Timestamp: 2026-01-05T02:41:31.661Z
Learning: Applies to src/renderer/src/stores/**/*.ts : Keep Pinia store actions focused on state mutations and async operations
Applied to files:
src/renderer/src/stores/chat.ts
📚 Learning: 2026-01-05T02:41:31.661Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-stack-guide.mdc:0-0
Timestamp: 2026-01-05T02:41:31.661Z
Learning: Applies to src/renderer/src/composables/**/*.ts : Name composables using camelCase with `use` prefix (e.g., `useChatState.ts`, `useMessageList.ts`)
Applied to files:
src/renderer/src/composables/message/useMessageRetry.tssrc/renderer/src/components/message/MessageList.vue
📚 Learning: 2026-01-05T02:41:31.661Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-stack-guide.mdc:0-0
Timestamp: 2026-01-05T02:41:31.661Z
Learning: Applies to src/renderer/src/**/*.{ts,tsx,vue} : Use VueUse composables for common utilities like `useLocalStorage`, `useClipboard`, `useDebounceFn`
Applied to files:
src/renderer/src/components/message/MessageList.vue
📚 Learning: 2026-01-05T02:41:31.661Z
Learnt from: CR
Repo: ThinkInAIXYZ/deepchat PR: 0
File: .cursor/rules/vue-stack-guide.mdc:0-0
Timestamp: 2026-01-05T02:41:31.661Z
Learning: Applies to src/renderer/src/**/*.vue : Use virtual scrolling with `RecycleScroller` component for rendering long lists
Applied to files:
src/renderer/src/components/message/MessageList.vue
⏰ 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 (24)
src/main/presenter/sqlitePresenter/tables/messages.ts (1)
273-334: LGTM! The query refinements correctly enforce assistant role and recency.The addition of
role = 'assistant'filter andORDER BY created_at DESCensures deterministic selection of the most recent assistant message as the main message. This aligns well with the ID-based retry flow changes across the PR.src/renderer/src/components/message/MessageToolbar.vue (1)
71-77: LGTM! Boundary checks are now more robust.The updated display and disabled logic correctly handle undefined values with nullish coalescing. The
>=comparison provides additional safety against out-of-bounds indices.src/renderer/src/composables/message/useMessageRetry.ts (2)
13-62: LGTM! ID-based retry logic is well-structured.The implementation correctly:
- Guards against undefined/null messageId inputs
- Uses optional chaining to handle null messages in the array
- Falls back to store-level regeneration when ref-based retry fails
- Includes proper error handling with console.error
5-5: No callers ofuseMessageRetryfound in the codebase.The composable's
retryFromUserMessagefunction is not imported or used anywhere. The type signature does acceptRef<Array<Message | null>>, and the implementation defensively handles null elements with optional chaining (e.g.,msg?.id,nextMessage?.role). However, since there are no current callers, the concern about "verifying that callers handle null elements" cannot be evaluated. If this composable is meant to be integrated elsewhere, ensure callers are compatible with the nullable message array.src/renderer/src/components/message/MessageItemAssistant.vue (4)
180-187: LGTM! Correct migration to object-based selectedVariantsMap.The bracket notation access
chatStore.selectedVariantsMap[props.message.id]correctly retrieves the selected variant ID from the Record structure, with proper fallback to index 0 when undefined.
199-219: LGTM! Improved variant aggregation with deduplication.Using a Map keyed by variant ID ensures no duplicates when merging persisted variants with cached generating variants. The
is_variant !== 0filter correctly excludes the main message from the variant list.
240-249: LGTM! Defensive handling of new variant selection.The ternary
lastVariant ? lastVariant.id : nullprovides a safe fallback, thoughlastVariantshould always exist whennewLength > oldLength. The early return when the thread is generating prevents disruptive auto-switching.
322-335: LGTM! Simplified navigation logic.The prev/next handling is now cleaner with direct index manipulation and proper boundary checks. The null fallback for
selectedVariantIdwhennewIndex === 0correctly returns to the main message.src/renderer/src/components/message/MessageList.vue (3)
35-36: LGTM! Correctly passing messageId for retry.The change from index-based to ID-based retry aligns with the PR objective. Using optional chaining
item.message?.idsafely handles cases where message might be undefined.
219-223: LGTM! Correct access pattern for Record-based selectedVariantsMap.The bracket notation
chatStore.selectedVariantsMap[message.id]correctly accesses the Record structure, with nullish coalescing returning an empty string as fallback.
244-249: LGTM! Clean delegation to store-level retry.The simplified
handleRetrycorrectly validates the messageId before delegating tochatStore.retryFromUserMessage. This centralizes retry logic in the store rather than managing refs locally.src/renderer/src/stores/chat.ts (13)
129-132: Appropriate refactor from Map to Record.Using
Record<string, string>simplifies reactivity and serialization compared toMap. The non-reactiveisUpdatingVariantflag is appropriate as a synchronization guard to prevent event loops during variant updates.
245-266: LGTM!The function correctly handles the object-based lookup. The bracket notation access properly returns
undefinedfor missing keys, which is falsy and correctly handled by the conditional.
634-638: Correct filtering of variant messages from main list.Variant messages should only be accessible through their parent message's
variantsarray, not as standalone entries in the message list. This prevents duplicate rendering.
764-781: LGTM!Clean implementation that provides a unified entry point for retry logic. The function correctly:
- First attempts to find and retry an existing assistant message
- Falls back to regenerating from the user message if no assistant response exists
- Maintains consistent error handling patterns
1115-1117: Consistent variant filtering during streaming.This correctly prevents variant messages from being added to the main message list during streaming, maintaining consistency with the filtering in
loadMessages.
1340-1342: LGTM!Correct conversion from Map to object operations for variant cleanup during message deletion.
1584-1589: LGTM!Correct conversion from Map
has()check to object bracket notation.
1725-1736: Good synchronization guard for multi-window scenarios.The
isUpdatingVariantcheck correctly prevents theLIST_UPDATEDevent from overwriting variant selections that are currently being persisted. This avoids race conditions when multiple windows are open.
1508-1547: Correct handling of variant message edits.When a variant message is edited, the function correctly fetches and updates the parent main message instead of adding the variant as a standalone entry. The early
returnstatements properly short-circuit the flow.
1427-1429: LGTM!Consistent variant filtering when canceling generation.
1300-1304: LGTM!Correct loading and defaulting of
selectedVariantsMapfrom conversation settings.
425-426: LGTM!Assigning a new empty object correctly clears the reactive state and triggers reactivity updates.
2017-2017: LGTM!New
retryFromUserMessagecorrectly exposed in the store's public API.
2 participants
Summary by CodeRabbit
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.