refactor(chat): streamline message handling and error notification logic #974
Conversation
WalkthroughThe chat store’s streaming update logic was reworked: content and reasoning_content blocks now finalize prior blocks to success before appending or creating new loading blocks; message enrichment precedes UI updates; variant vs non-variant update paths were unified; error handling and notifications were centralized and gated by window focus. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Stream as Model Stream
participant Store as Chat Store
participant Enricher as Enrich Message
participant UI as UI State
participant Notify as Notifications
participant Focus as Window Focus
rect rgb(240,248,255)
note over Store,UI: Token/content arrives
Stream->>Store: content | reasoning_content | error
end
alt content or reasoning_content
Store->>Store: Finalize previous block to success
Store->>Store: Create/append current loading block
Store->>UI: Update blocks (loading)
Store-->>UI: Play typing sound (if applicable)
else error chunk
Store->>Enricher: Extract/derive error message
Enricher-->>Store: enriched error
end
rect rgb(245,255,250)
note over Store,Enricher: Periodic/EOF enrichment
Store->>Enricher: Fetch and enrich latest message
Enricher-->>Store: enrichedMessage
end
alt Variant message
Store->>UI: Update parent message's variants
opt If main message active
Store->>UI: Update main message from variant
end
else Non-variant message
Store->>UI: Update message in place
end
alt Error detected
Store->>Focus: isFocused?
Focus-->>Store: true/false
opt If not focused
Store->>Notify: Emit error notification
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/src/stores/chat.ts (1)
614-625: Reasoning stream suffers the same drop + premature finalization.Identical to the content path: when the prior block isn’t
reasoning_content, the new reasoning text never gets stored and active tool-call blocks are forced tosuccess. Finalize first, then create a new reasoning block; only append when the existing block already matches.- const lastReasoningBlock = curMsg.content[curMsg.content.length - 1] - if (lastReasoningBlock) { - lastReasoningBlock.status = 'success' - if (lastReasoningBlock.type === 'reasoning_content') { - lastReasoningBlock.content += msg.reasoning_content - } - } else { + const lastReasoningBlock = curMsg.content[curMsg.content.length - 1] + if (lastReasoningBlock?.type === 'reasoning_content') { + lastReasoningBlock.content += msg.reasoning_content + } else { + finalizeLastBlock() curMsg.content.push({ type: 'reasoning_content', content: msg.reasoning_content, status: 'loading', timestamp: Date.now() }) }
🧹 Nitpick comments (1)
src/renderer/src/stores/chat.ts (1)
919-924: Keep the tuple typing instead of casting tostring[].
cache.entries()yields[messageId, cached], so asserting tostring[]erases the actual tuple shape and hides the cached payload type. Preserve the tuple typing (e.g.,as [string, { message: AssistantMessage | UserMessage; threadId: string }] | undefined) to keep type safety.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/renderer/src/stores/chat.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (15)
**/*.{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/renderer/src/stores/chat.ts
src/{main,renderer}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
src/{main,renderer}/**/*.ts: Use context isolation for improved security
Implement proper inter-process communication (IPC) patterns
Optimize application startup time with lazy loading
Implement proper error handling and logging for debugging
Files:
src/renderer/src/stores/chat.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪（如适用）、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别
Files:
src/renderer/src/stores/chat.ts
src/renderer/src/**/*
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
src/renderer/src/**/*: All user-facing strings must use i18n keys (avoid hardcoded user-visible text in code)
Use the 'vue-i18n' framework for all internationalization in the renderer
Ensure all user-visible text in the renderer uses the translation system
Files:
src/renderer/src/stores/chat.ts
src/renderer/src/stores/**/*.{vue,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/pinia-best-practices.mdc)
src/renderer/src/stores/**/*.{vue,ts,tsx,js,jsx}: Use modules to organize related state and actions
Implement proper state persistence for maintaining data across sessions
Use getters for computed state properties
Utilize actions for side effects and asynchronous operations
Keep the store focused on global state, not component-specific data
Files:
src/renderer/src/stores/chat.ts
src/renderer/**/*.{vue,ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
渲染进程代码放在
src/renderer
Files:
src/renderer/src/stores/chat.ts
src/renderer/src/**/*.{vue,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/vue-best-practices.mdc)
src/renderer/src/**/*.{vue,ts,tsx,js,jsx}: Use the Composition API for better code organization and reusability
Implement proper state management with Pinia
Utilize Vue Router for navigation and route management
Leverage Vue's built-in reactivity system for efficient data handling
Files:
src/renderer/src/stores/chat.ts
src/renderer/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,tsx,vue}: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Use TypeScript for all code; prefer types over interfaces.
Avoid enums; use const objects instead.
Use arrow functions for methods and computed properties.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Files:
src/renderer/src/stores/chat.ts
src/renderer/**/*.{vue,ts}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
Implement lazy loading for routes and components.
Files:
src/renderer/src/stores/chat.ts
src/renderer/**/*.{ts,vue}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,vue}: Use useFetch and useAsyncData for data fetching.
Implement SEO best practices using Nuxt's useHead and useSeoMeta.Use Pinia for frontend state management (do not introduce alternative state libraries)
Files:
src/renderer/src/stores/chat.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/renderer/src/stores/chat.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Files:
src/renderer/src/stores/chat.ts
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/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/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/stores/chat.ts
🧬 Code graph analysis (1)
src/renderer/src/stores/chat.ts (1)
src/shared/chat.d.ts (2)
AssistantMessage(39-42)UserMessage(34-37)
🪛 Biome (2.1.2)
src/renderer/src/stores/chat.ts
[error] 744-744: Comparing to itself is potentially pointless.
(lint/suspicious/noSelfCompare)
[error] 752-755: Comparing to itself is potentially pointless.
(lint/suspicious/noSelfCompare)
⏰ 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 (1)
src/renderer/src/stores/chat.ts (1)
724-781: Unified focus-gated error notification flow looks solid.Variant vs. non-variant updates now converge before emitting notifications, and the focus check prevents noisy popups. Nice cleanup.
| if (lastContentBlock) { | ||
| lastContentBlock.status = 'success' | ||
| if (lastContentBlock.type === 'content') { | ||
| lastContentBlock.content += msg.content | ||
| } | ||
| } else { | ||
| curMsg.content.push({ | ||
| type: 'content', | ||
| content: msg.content, | ||
| status: 'loading', | ||
| timestamp: Date.now() | ||
| }) | ||
| // 如果是新块的第一个字符，也播放声音 | ||
| playTypewriterSound() | ||
| } |
There was a problem hiding this comment.
Content chunks are dropped when the previous block isn’t content.
As soon as a text chunk arrives while the last block is a tool call/action/etc., this branch marks that block success but never appends a new content block, so the incoming text is lost. It also forces running tool-call blocks to success, breaking their lifecycle. Reuse finalizeLastBlock() before creating a fresh content block when the prior block isn’t textual, and only append to the existing block when it’s already content.
- const lastContentBlock = curMsg.content[curMsg.content.length - 1]
- if (lastContentBlock) {
- lastContentBlock.status = 'success'
- if (lastContentBlock.type === 'content') {
- lastContentBlock.content += msg.content
- }
- } else {
+ const lastContentBlock = curMsg.content[curMsg.content.length - 1]
+ if (lastContentBlock?.type === 'content') {
+ lastContentBlock.content += msg.content
+ } else {
+ finalizeLastBlock()
curMsg.content.push({
type: 'content',
content: msg.content,
status: 'loading',
timestamp: Date.now()
})
}📝 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.
| if (lastContentBlock) { | |
| lastContentBlock.status = 'success' | |
| if (lastContentBlock.type === 'content') { | |
| lastContentBlock.content += msg.content | |
| } | |
| } else { | |
| curMsg.content.push({ | |
| type: 'content', | |
| content: msg.content, | |
| status: 'loading', | |
| timestamp: Date.now() | |
| }) | |
| // 如果是新块的第一个字符，也播放声音 | |
| playTypewriterSound() | |
| } | |
| const lastContentBlock = curMsg.content[curMsg.content.length - 1] | |
| if (lastContentBlock?.type === 'content') { | |
| lastContentBlock.content += msg.content | |
| } else { | |
| finalizeLastBlock() | |
| curMsg.content.push({ | |
| type: 'content', | |
| content: msg.content, | |
| status: 'loading', | |
| timestamp: Date.now() | |
| }) | |
| } |
🤖 Prompt for AI Agents
In src/renderer/src/stores/chat.ts around lines 595 to 607: the current branch
marks the last block `success` and drops incoming text when the previous block
isn’t of type 'content'; instead call finalizeLastBlock() to properly complete
non-content blocks, then create and push a new 'content' block for the incoming
msg.content; only append to lastContentBlock.content when lastContentBlock
exists and lastContentBlock.type === 'content', otherwise create a fresh
{type:'content', content: msg.content, status:'loading', timestamp: Date.now()}
entry on curMsg.content so text is not lost and tool-call blocks keep their
lifecycle.
|
@codex review |
There was a problem hiding this comment.
💡 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
| // 处理普通内容 | ||
| else if (msg.content) { | ||
| const lastContentBlock = curMsg.content[curMsg.content.length - 1] | ||
| if (lastContentBlock && lastContentBlock.type === 'content') { | ||
| lastContentBlock.content += msg.content | ||
| // 打字机音效，与实际数据流同步 | ||
| playTypewriterSound() | ||
| } else { | ||
| if (lastContentBlock) { | ||
| lastContentBlock.status = 'success' | ||
| if (lastContentBlock) { | ||
| lastContentBlock.status = 'success' | ||
| if (lastContentBlock.type === 'content') { | ||
| lastContentBlock.content += msg.content | ||
| } | ||
| } else { | ||
| curMsg.content.push({ | ||
| type: 'content', | ||
| content: msg.content, | ||
| status: 'loading', | ||
| timestamp: Date.now() | ||
| }) |
There was a problem hiding this comment.
Append a new content block when previous block is not text
In the streaming handler, when msg.content arrives and the last block exists but is not of type content, the new code marks that block as success and returns without pushing a new text block. Previously the else branch covered both "no block" and "non‑content block" so the plain response text was appended. With the current logic, any stream that first emits a tool/action/reasoning block and then regular content will drop the textual response entirely while still playing the typing sound. Users will observe empty assistant replies whenever content follows a different block type.
Useful? React with 👍 / 👎.
| // 处理推理内容 | ||
| if (msg.reasoning_content) { | ||
| const lastReasoningBlock = curMsg.content[curMsg.content.length - 1] | ||
| if (lastReasoningBlock && lastReasoningBlock.type === 'reasoning_content') { | ||
| lastReasoningBlock.content += msg.reasoning_content | ||
| } else { | ||
| if (lastReasoningBlock) { | ||
| lastReasoningBlock.status = 'success' | ||
| if (lastReasoningBlock) { | ||
| lastReasoningBlock.status = 'success' | ||
| if (lastReasoningBlock.type === 'reasoning_content') { | ||
| lastReasoningBlock.content += msg.reasoning_content | ||
| } | ||
| } else { | ||
| curMsg.content.push({ | ||
| type: 'reasoning_content', | ||
| content: msg.reasoning_content, |
There was a problem hiding this comment.
Push reasoning block when switching from other block types
The refactor applies the same pattern to msg.reasoning_content: if any block exists, it is marked success but a new reasoning block is only created when there were no blocks at all. When reasoning content is streamed after another block (e.g., a tool call or prior content), no reasoning block is appended and the explanation text disappears. The previous code handled this case by finalizing the prior block and then pushing a new reasoning block; that behaviour is now lost, so reasoning output never renders when it arrives after a different block type.
Useful? React with 👍 / 👎.
Summary by CodeRabbit
Bug Fixes
Refactor