-
Notifications
You must be signed in to change notification settings - Fork 625
Feat/button cancel #1002
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
Feat/button cancel #1002
Conversation
WalkthroughAdds streaming-aware cancel behavior to PromptInput, introduces a confirmation dialog to clear all messages from MessageList, and adjusts MessageBlockError to render a distinct UI for cancelled blocks versus other errors. New control flows handle canceling generation and confirming thread-wide message clearing. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant PromptInput
participant ChatStore as chatStore
participant Thread as ActiveThread
User->>PromptInput: Press Cancel (while streaming)
PromptInput->>ChatStore: getActiveThreadId()
alt Active thread exists and is generating
PromptInput->>ChatStore: cancelGenerating(threadId)
ChatStore-->>PromptInput: ack/caught error
else No active thread
Note over PromptInput: No-op
end
sequenceDiagram
autonumber
actor User
participant MessageList
participant Dialog
participant ChatStore as chatStore
User->>MessageList: Click "Clean messages"
MessageList->>Dialog: Open
User->>Dialog: Confirm
Dialog->>MessageList: onConfirm
MessageList->>ChatStore: getActiveThreadId()
MessageList->>ChatStore: clearAllMessages(threadId)
alt Success
ChatStore-->>MessageList: resolved
MessageList->>Dialog: Close
else Error
ChatStore-->>MessageList: error
MessageList->>Dialog: Close (after handling)
end
sequenceDiagram
participant UI as MessageBlockError.vue
participant Block as block
alt block.status == 'cancel'
UI->>UI: Render muted cancel row with icon + i18n text
else Other statuses
UI->>UI: Render red error block (existing)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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: 1
🧹 Nitpick comments (2)
src/renderer/src/components/prompt-input/PromptInput.vue (1)
729-732: Consider providing user feedback when cancellation fails.The
handleCancelfunction silently returns when there's no active thread. While this prevents errors, it could leave users uncertain about why the cancel action had no effect.Consider logging or providing user feedback:
const handleCancel = () => { - if (!chatStore.getActiveThreadId()) return + const activeThreadId = chatStore.getActiveThreadId() + if (!activeThreadId) { + console.warn('Cannot cancel: no active thread') + return + } - chatStore.cancelGenerating(chatStore.getActiveThreadId()!) + chatStore.cancelGenerating(activeThreadId) }src/renderer/src/components/message/MessageBlockError.vue (1)
2-8: Avoid empty translation lookups:t(block.content || '')will callt('')whenblock.contentis falsy—either wrap the<span>inv-if="block.content"or supply a default key (e.g.'common.cancelled') to prevent empty lookups.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/renderer/src/components/message/MessageBlockError.vue(1 hunks)src/renderer/src/components/message/MessageList.vue(6 hunks)src/renderer/src/components/prompt-input/PromptInput.vue(3 hunks)
🧰 Additional context used
📓 Path-based instructions (16)
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/prompt-input/PromptInput.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/components/message/MessageBlockError.vue
src/renderer/**/*.{vue,ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
渲染进程代码放在
src/renderer
Files:
src/renderer/src/components/prompt-input/PromptInput.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/components/message/MessageBlockError.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/prompt-input/PromptInput.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/components/message/MessageBlockError.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/prompt-input/PromptInput.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/components/message/MessageBlockError.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/prompt-input/PromptInput.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/components/message/MessageBlockError.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/prompt-input/PromptInput.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/components/message/MessageBlockError.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/prompt-input/PromptInput.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/components/message/MessageBlockError.vue
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/renderer/src/components/prompt-input/PromptInput.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/components/message/MessageBlockError.vue
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Use PascalCase for TypeScript types and classes
Files:
src/renderer/src/components/prompt-input/PromptInput.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/components/message/MessageBlockError.vue
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/prompt-input/PromptInput.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/components/message/MessageBlockError.vue
src/renderer/src/components/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Organize UI components by feature within src/renderer/src/
Files:
src/renderer/src/components/prompt-input/PromptInput.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/components/message/MessageBlockError.vue
src/renderer/src/**
📄 CodeRabbit inference engine (AGENTS.md)
Place Vue 3 app source under src/renderer/src (components, stores, views, i18n, lib)
Files:
src/renderer/src/components/prompt-input/PromptInput.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/components/message/MessageBlockError.vue
src/renderer/src/**/*.{vue,ts}
📄 CodeRabbit inference engine (AGENTS.md)
All user-facing strings must use vue-i18n ($t/keys) rather than hardcoded literals
Files:
src/renderer/src/components/prompt-input/PromptInput.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/components/message/MessageBlockError.vue
**/*.{ts,tsx,js,jsx,vue,css,scss,md,json,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Prettier style: single quotes, no semicolons, print width 100; run pnpm run format
Files:
src/renderer/src/components/prompt-input/PromptInput.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/components/message/MessageBlockError.vue
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,vue}: Use OxLint for JS/TS code; keep lint clean
Use camelCase for variables and functions
Use SCREAMING_SNAKE_CASE for constants
Files:
src/renderer/src/components/prompt-input/PromptInput.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/components/message/MessageBlockError.vue
src/renderer/**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
Name Vue component files in PascalCase (e.g., ChatInput.vue)
Files:
src/renderer/src/components/prompt-input/PromptInput.vuesrc/renderer/src/components/message/MessageList.vuesrc/renderer/src/components/message/MessageBlockError.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 (6)
src/renderer/src/components/prompt-input/PromptInput.vue (2)
1073-1079: LGTM: isStreaming implementation is clean.The computed property correctly checks if the active thread is in the generating state by looking up the thread ID in
generatingThreadIds.
196-214: Verify conditional rendering with v-if and keys.The Send and Cancel buttons use
v-ifwithout explicitkeyattributes (except Cancel haskey="cancel"). While Vue 3 generally handles this well, consider adding a key to the Send button for explicit control during transitions.<Button + key="send" variant="default" size="icon" class="w-7 h-7 text-xs rounded-lg"This ensures Vue properly distinguishes between the two buttons during the transition when
isStreamingchanges.src/renderer/src/components/message/MessageList.vue (4)
93-110: LGTM: Dialog implementation follows best practices.The dialog component correctly uses:
- i18n keys for all user-facing text
- Destructive variant for the confirm button
- Proper state management with
cleanMessagesDialogref- Standard dialog composition with header, description, and footer
As per coding guidelines.
188-206: Verify error handling in chatStore.clearAllMessages.The
handleThreadCleanMessagesfunction includes a try-catch block, but only logs the error without providing user feedback (e.g., a toast notification).Consider whether users should be notified of failure:
const handleThreadCleanMessages = async () => { try { const threadId = chatStore.getActiveThreadId() if (!threadId) { return } await chatStore.clearAllMessages(threadId) cleanMessagesDialog.value = false } catch (error) { console.error(t('common.error.cleanMessagesFailed'), error) // Consider adding user-facing error notification: // toast({ title: t('common.error.cleanMessagesFailed'), variant: 'destructive' }) } }Note: The dialog is closed outside the try-catch, so it closes even on error. Consider moving it inside the try block if you want it to stay open on failure.
40-40: Approve flex direction change for vertical button stacking.Changing from horizontal to vertical layout (
flex flex-col) appropriately accommodates the new dialog-triggered clean action alongside the scroll-to-bottom button.
130-139: LGTM: Dialog imports and state initialization.The dialog components are properly imported from the shadcn UI library, and the
cleanMessagesDialogref is correctly initialized as a boolean.
| > | ||
| <Icon icon="lucide:arrow-up" class="w-4 h-4" /> | ||
| </Button> | ||
| <!-- 取消按钮 --> |
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 Chinese comment with English.
The comment on line 201 is in Chinese. Per coding guidelines, all comments must be in English.
As per coding guidelines.
Apply this diff:
- <!-- 取消按钮 -->
+ <!-- Cancel button -->📝 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.
| <!-- 取消按钮 --> | |
| <!-- Cancel button --> |
🤖 Prompt for AI Agents
In src/renderer/src/components/prompt-input/PromptInput.vue around line 201, the
inline comment "取消按钮" is in Chinese; update that comment to English (e.g.,
"Cancel button") by replacing the Chinese text with an equivalent English
comment, preserving comment style/placement and whitespace.
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting |
Summary by CodeRabbit
New Features
Style