-
Notifications
You must be signed in to change notification settings - Fork 2.5k
feat: edit message and delete #7391
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,5 @@ | ||||||||
| /* eslint-disable @typescript-eslint/no-explicit-any */ | ||||||||
| import { memo, useState, useCallback } from 'react' | ||||||||
| import type { UIMessage, ChatStatus } from 'ai' | ||||||||
| import { RenderMarkdown } from './RenderMarkdown' | ||||||||
| import { cn } from '@/lib/utils' | ||||||||
|
|
@@ -9,31 +9,33 @@ | |||||||
| ReasoningContent, | ||||||||
| ReasoningTrigger, | ||||||||
| } from '@/ai-elements/reasoning' | ||||||||
| import { | ||||||||
| Tool, | ||||||||
| ToolContent, | ||||||||
| ToolHeader, | ||||||||
| ToolInput, | ||||||||
| ToolOutput, | ||||||||
| } from '@/ai-elements/tool' | ||||||||
| import { CopyButton } from './CopyButton' | ||||||||
| import { AvatarEmoji } from '@/containers/AvatarEmoji' | ||||||||
| import { useModelProvider } from '@/hooks/useModelProvider' | ||||||||
| import { IconRefresh, IconPaperclip } from '@tabler/icons-react' | ||||||||
| import { EditMessageDialog } from '@/containers/dialogs/EditMessageDialog' | ||||||||
| import { DeleteMessageDialog } from '@/containers/dialogs/DeleteMessageDialog' | ||||||||
| import TokenSpeedIndicator from '@/containers/TokenSpeedIndicator' | ||||||||
| import { extractFilesFromPrompt, FileMetadata } from '@/lib/fileMetadata' | ||||||||
| import { useMemo } from 'react' | ||||||||
|
|
||||||||
| const CHAT_STATUS = { | ||||||||
| STREAMING: 'streaming', | ||||||||
| SUBMITTED: 'submitted', | ||||||||
| } as const | ||||||||
|
|
||||||||
| const CONTENT_TYPE = { | ||||||||
| TEXT: 'text', | ||||||||
| FILE: 'file', | ||||||||
| REASONING: 'reasoning', | ||||||||
| } as const | ||||||||
|
|
||||||||
| export type MessageItemProps = { | ||||||||
| message: UIMessage | ||||||||
|
|
@@ -42,30 +44,56 @@ | |||||||
| status: ChatStatus | ||||||||
| reasoningContainerRef?: React.RefObject<HTMLDivElement | null> | ||||||||
| onRegenerate?: (messageId: string) => void | ||||||||
| onEdit?: (messageId: string, newText: string) => void | ||||||||
| onDelete?: (messageId: string) => void | ||||||||
| assistant?: { avatar?: React.ReactNode; name?: string } | ||||||||
| showAssistant?: boolean | ||||||||
| } | ||||||||
|
|
||||||||
| export const MessageItem = memo( | ||||||||
| ({ | ||||||||
| message, | ||||||||
| isLastMessage, | ||||||||
| status, | ||||||||
| reasoningContainerRef, | ||||||||
| onRegenerate, | ||||||||
| onEdit, | ||||||||
| onDelete, | ||||||||
| assistant, | ||||||||
| showAssistant, | ||||||||
| }: MessageItemProps) => { | ||||||||
| const selectedModel = useModelProvider((state) => state.selectedModel) | ||||||||
| const [previewImage, setPreviewImage] = useState<{ | ||||||||
| url: string | ||||||||
| filename?: string | ||||||||
| } | null>(null) | ||||||||
|
|
||||||||
| const handleRegenerate = useCallback(() => { | ||||||||
| onRegenerate?.(message.id) | ||||||||
| }, [onRegenerate, message.id]) | ||||||||
|
|
||||||||
| const handleEdit = useCallback( | ||||||||
| (newText: string) => { | ||||||||
| onEdit?.(message.id, newText) | ||||||||
| }, | ||||||||
| [onEdit, message.id] | ||||||||
| ) | ||||||||
|
|
||||||||
| const handleDelete = useCallback(() => { | ||||||||
| onDelete?.(message.id) | ||||||||
| }, [onDelete, message.id]) | ||||||||
|
|
||||||||
| // Get image URLs from file parts for the edit dialog | ||||||||
| const imageUrls = useMemo(() => { | ||||||||
| return message.parts | ||||||||
| .filter((part) => { | ||||||||
| if (part.type !== 'file') return false | ||||||||
| const filePart = part as { type: 'file'; url?: string; mediaType?: string } | ||||||||
| return filePart.url && filePart.mediaType?.startsWith('image/') | ||||||||
| }) | ||||||||
| .map((part) => (part as { url: string }).url) | ||||||||
| }, [message.parts]) | ||||||||
|
|
||||||||
| const isStreaming = isLastMessage && status === CHAT_STATUS.STREAMING | ||||||||
|
|
||||||||
| // Extract file metadata from message text (for user messages with attachments) | ||||||||
|
|
@@ -341,17 +369,17 @@ | |||||||
| <div className="flex items-center justify-end gap-2 text-main-view-fg/60 text-xs mt-2"> | ||||||||
| <CopyButton text={getFullTextContent()} /> | ||||||||
|
|
||||||||
| {selectedModel && | ||||||||
| onRegenerate && | ||||||||
| status !== CHAT_STATUS.STREAMING && ( | ||||||||
| <button | ||||||||
| className="flex items-center gap-1 hover:text-accent transition-colors cursor-pointer group relative" | ||||||||
| onClick={handleRegenerate} | ||||||||
| title="Regenerate from this message" | ||||||||
| > | ||||||||
| <IconRefresh size={16} /> | ||||||||
| </button> | ||||||||
| )} | ||||||||
| {onEdit && status !== CHAT_STATUS.STREAMING && ( | ||||||||
| <EditMessageDialog | ||||||||
| message={getFullTextContent()} | ||||||||
| imageUrls={imageUrls.length > 0 ? imageUrls : undefined} | ||||||||
| onSave={handleEdit} | ||||||||
| /> | ||||||||
| )} | ||||||||
|
|
||||||||
| {onDelete && status !== CHAT_STATUS.STREAMING && ( | ||||||||
| <DeleteMessageDialog onDelete={handleDelete} /> | ||||||||
| )} | ||||||||
| </div> | ||||||||
| )} | ||||||||
|
|
||||||||
|
|
@@ -367,11 +395,22 @@ | |||||||
| > | ||||||||
| <CopyButton text={getFullTextContent()} /> | ||||||||
|
|
||||||||
| {selectedModel && onRegenerate && !isStreaming && ( | ||||||||
| {onEdit && !isStreaming && ( | ||||||||
| <EditMessageDialog | ||||||||
| message={getFullTextContent()} | ||||||||
| onSave={handleEdit} | ||||||||
| /> | ||||||||
| )} | ||||||||
|
|
||||||||
| {onDelete && !isStreaming && ( | ||||||||
| <DeleteMessageDialog onDelete={handleDelete} /> | ||||||||
| )} | ||||||||
|
|
||||||||
|
Comment on lines
+405
to
+408
|
||||||||
| {onDelete && !isStreaming && ( | |
| <DeleteMessageDialog onDelete={handleDelete} /> | |
| )} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,11 +12,6 @@ import { | |
| } from '@/components/ui/dialog' | ||
| import { Button } from '@/components/ui/button' | ||
| import { IconTrash } from '@tabler/icons-react' | ||
| import { | ||
| Tooltip, | ||
| TooltipContent, | ||
| TooltipTrigger, | ||
| } from '@/components/ui/tooltip' | ||
|
|
||
| interface DeleteMessageDialogProps { | ||
| onDelete: () => void | ||
|
|
@@ -39,26 +34,19 @@ export function DeleteMessageDialog({ onDelete }: DeleteMessageDialogProps) { | |
| } | ||
|
|
||
| const trigger = ( | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <div | ||
| className="flex items-center gap-1 hover:text-accent transition-colors cursor-pointer group relative" | ||
| role="button" | ||
| tabIndex={0} | ||
| onKeyDown={(e) => { | ||
| if (e.key === 'Enter' || e.key === ' ') { | ||
| e.preventDefault() | ||
| setIsOpen(true) | ||
| } | ||
| }} | ||
| > | ||
| <IconTrash size={16} /> | ||
| </div> | ||
| </TooltipTrigger> | ||
| <TooltipContent> | ||
| <p>{t('delete')}</p> | ||
| </TooltipContent> | ||
| </Tooltip> | ||
| <div | ||
| className="flex items-center gap-1 hover:text-accent transition-colors cursor-pointer group relative" | ||
| role="button" | ||
| tabIndex={0} | ||
| onKeyDown={(e) => { | ||
| if (e.key === 'Enter' || e.key === ' ') { | ||
| e.preventDefault() | ||
| setIsOpen(true) | ||
| } | ||
| }} | ||
| > | ||
| <IconTrash size={16} /> | ||
| </div> | ||
|
Comment on lines
+37
to
+49
|
||
| ) | ||
|
|
||
| return ( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,16 +12,11 @@ import { | |
| import { Button } from '@/components/ui/button' | ||
| import { Textarea } from '@/components/ui/textarea' | ||
| import { IconPencil, IconX } from '@tabler/icons-react' | ||
| import { | ||
| Tooltip, | ||
| TooltipContent, | ||
| TooltipTrigger, | ||
| } from '@/components/ui/tooltip' | ||
|
|
||
| interface EditMessageDialogProps { | ||
| message: string | ||
| imageUrls?: string[] | ||
| onSave: (message: string, imageUrls?: string[]) => void | ||
| onSave: (message: string) => void | ||
| triggerElement?: React.ReactNode | ||
| } | ||
|
|
||
|
|
@@ -53,14 +48,9 @@ export function EditMessageDialog({ | |
|
|
||
| const handleSave = () => { | ||
| const hasTextChanged = draft !== message && draft.trim() | ||
| const hasImageChanged = | ||
| JSON.stringify(imageUrls || []) !== JSON.stringify(keptImages) | ||
|
|
||
| if (hasTextChanged || hasImageChanged) { | ||
| onSave( | ||
| draft.trim() || message, | ||
| keptImages.length > 0 ? keptImages : undefined | ||
| ) | ||
| if (hasTextChanged) { | ||
| onSave(draft.trim() || message) | ||
| setIsOpen(false) | ||
| } | ||
| } | ||
|
|
@@ -73,26 +63,19 @@ export function EditMessageDialog({ | |
| } | ||
|
|
||
| const defaultTrigger = ( | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <div | ||
| className="flex outline-0 items-center gap-1 hover:text-accent transition-colors cursor-pointer group relative" | ||
| role="button" | ||
| tabIndex={0} | ||
| onKeyDown={(e) => { | ||
| if (e.key === 'Enter' || e.key === ' ') { | ||
| e.preventDefault() | ||
| setIsOpen(true) | ||
| } | ||
| }} | ||
| > | ||
| <IconPencil size={16} /> | ||
| </div> | ||
| </TooltipTrigger> | ||
| <TooltipContent> | ||
| <p>{t('edit')}</p> | ||
| </TooltipContent> | ||
| </Tooltip> | ||
| <div | ||
| className="flex outline-0 items-center gap-1 hover:text-accent transition-colors cursor-pointer group relative" | ||
| role="button" | ||
| tabIndex={0} | ||
| onKeyDown={(e) => { | ||
| if (e.key === 'Enter' || e.key === ' ') { | ||
| e.preventDefault() | ||
| setIsOpen(true) | ||
| } | ||
| }} | ||
| > | ||
| <IconPencil size={16} /> | ||
| </div> | ||
|
Comment on lines
+66
to
+78
|
||
| ) | ||
|
|
||
| return ( | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -33,7 +33,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||
| extractContentPartsFromUIMessage, | ||||||||||||||||||||||||||||||||||||||||||||||||
| } from '@/lib/messages' | ||||||||||||||||||||||||||||||||||||||||||||||||
| import { newUserThreadContent } from '@/lib/completion' | ||||||||||||||||||||||||||||||||||||||||||||||||
| import { ThreadMessage, MessageStatus, ChatCompletionRole } from '@janhq/core' | ||||||||||||||||||||||||||||||||||||||||||||||||
| import { ThreadMessage, MessageStatus, ChatCompletionRole, ContentType } from '@janhq/core' | ||||||||||||||||||||||||||||||||||||||||||||||||
| import { createImageAttachment } from '@/types/attachment' | ||||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||||
| useChatAttachments, | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -566,7 +566,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||
| // Handle regenerate from any message (user or assistant) | ||||||||||||||||||||||||||||||||||||||||||||||||
| // - For user messages: keeps the user message, deletes all after, regenerates assistant response | ||||||||||||||||||||||||||||||||||||||||||||||||
| // - For assistant messages: finds the closest preceding user message, deletes from there | ||||||||||||||||||||||||||||||||||||||||||||||||
| const handleRegenerate = (messageId?: string) => { | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Check warning on line 569 in web-app/src/routes/threads/$threadId.tsx
|
||||||||||||||||||||||||||||||||||||||||||||||||
| if (!languageModelId || !languageModelProvider) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| console.warn('No language model available') | ||||||||||||||||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -613,6 +613,82 @@ | |||||||||||||||||||||||||||||||||||||||||||||||
| regenerate(messageId ? { messageId } : undefined) | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| // Handle edit message - updates the message and regenerates from it | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
| // Handle edit message - updates the message and regenerates from it | |
| // Handle edit message - updates the message and regenerates from it. | |
| // | |
| // Note: EditMessageDialog can provide `imageUrls` when a message is edited, | |
| // but at the moment we only persist text changes here. Image additions, | |
| // removals, or modifications are intentionally not persisted yet. The | |
| // `_imageUrls` parameter is accepted to satisfy the dialog's interface and | |
| // reserved for future support of editing message images. |
Copilot
AI
Jan 23, 2026
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.
The handleEditMessage function has potential state synchronization issues. It updates both the backend messages (updateMessage) and the UI chatMessages (setChatMessages) separately, but these updates are not atomic. If a message deletion or regeneration happens concurrently, the states could become inconsistent. Additionally, the function reads chatMessages at line 646 but that value might be stale if the component hasn't re-rendered yet with the latest backend changes. Consider using a single source of truth or ensuring proper state synchronization between the backend messages and UI chatMessages.
| // Update chat messages for UI | |
| const updatedChatMessages = chatMessages.map((msg) => { | |
| if (msg.id === messageId) { | |
| return { | |
| ...msg, | |
| parts: [{ type: 'text' as const, text: newText }], | |
| } | |
| } | |
| return msg | |
| }) | |
| setChatMessages(updatedChatMessages) | |
| // Update chat messages for UI using functional update to avoid stale state | |
| setChatMessages((prevChatMessages) => | |
| prevChatMessages.map((msg) => { | |
| if (msg.id === messageId) { | |
| return { | |
| ...msg, | |
| parts: [{ type: 'text' as const, text: newText }], | |
| } | |
| } | |
| return msg | |
| }) | |
| ) |
Copilot
AI
Jan 23, 2026
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.
The handleDeleteMessage function only updates the local state (backend via deleteMessage and UI via setChatMessages) but doesn't consider the implications for conversation continuity. If an assistant message is deleted, the conversation might become incoherent (a user message with no response), or if a user message is deleted, the following assistant message loses its context. Consider adding validation to prevent deletion of messages that would break conversation flow, or implement cascade deletion logic (e.g., when deleting a user message, also delete subsequent assistant responses until the next user message).
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.
Allowing users to edit assistant messages (lines 398-403) is problematic because assistant messages represent AI-generated content that the user didn't create. Editing an assistant message and then regenerating from it could lead to confusion about what was AI-generated vs. user-modified. Consider restricting edit functionality to user messages only (role === 'user'), similar to how it's already done at lines 372-378. If editing assistant messages is intentionally allowed, this should be clearly documented with a use case justification.