feat: inline user message editing in timeline (with image)#1681
feat: inline user message editing in timeline (with image)#1681ChrisLally wants to merge 1 commit intopingdotgg:mainfrom
Conversation
Made-with: Cursor
github-actions
bot
added
size:XL
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| } | ||
|
|
||
| await waitForThreadMessageRemoval(activeThread.id, message.id, EDIT_REVERT_SYNC_TIMEOUT_MS); | ||
|
|
There was a problem hiding this comment.
🟠 High components/ChatView.tsx:3140
In onSubmitEditUserMessage, the editing state is cleared before calling submitComposerTurn (lines 3140-3143). If the submission fails, the user's edited content is lost because submitComposerTurn only restores draft state when clearComposerDraft is true, but edits pass clearComposerDraft: false. Clear the editing state only after submitComposerTurn succeeds, or restore it on failure.
🤖 Copy this AI Prompt to have your agent fix this:
In file apps/web/src/components/ChatView.tsx around line 3140:
In `onSubmitEditUserMessage`, the editing state is cleared before calling `submitComposerTurn` (lines 3140-3143). If the submission fails, the user's edited content is lost because `submitComposerTurn` only restores draft state when `clearComposerDraft` is true, but edits pass `clearComposerDraft: false`. Clear the editing state only after `submitComposerTurn` succeeds, or restore it on failure.
Evidence trail:
ChatView.tsx lines 3140-3143 (clearing editing state): `setEditingUserMessageId(null); setEditingUserMessageText(""); setEditingUserMessageImages([]);`
ChatView.tsx lines 3147-3153 (submitComposerTurn call with clearComposerDraft: false)
ChatView.tsx lines 2977-2996 (error recovery gated by `clearComposerDraft && !turnStartSucceeded`)
Commit: REVIEWED_COMMIT
There was a problem hiding this comment.
🟠 High
t3code/apps/web/src/components/ChatView.tsx
Line 2239 in 449f3ab
When navigating to a different thread while editing a user message, the editing state (editingUserMessageId, editingUserMessageText, editingUserMessageImages) is not reset. This causes memory leaks because blob URLs created via URL.createObjectURL for editing images are never revoked, and stale editing state may persist causing UI confusion. The thread-change cleanup effect resets expandedImage and other state but omits these editing variables.
🤖 Copy this AI Prompt to have your agent fix this:
In file apps/web/src/components/ChatView.tsx around line 2239:
When navigating to a different thread while editing a user message, the editing state (`editingUserMessageId`, `editingUserMessageText`, `editingUserMessageImages`) is not reset. This causes memory leaks because blob URLs created via `URL.createObjectURL` for editing images are never revoked, and stale editing state may persist causing UI confusion. The thread-change cleanup effect resets `expandedImage` and other state but omits these editing variables.
Evidence trail:
apps/web/src/components/ChatView.tsx lines 499-503 (editing state declarations), line 2557 (URL.createObjectURL call for previewUrl), lines 2239-2253 (thread-change cleanup effect with threadId dependency that omits editing state), lines 3111-3119 (onCancelEditUserMessage showing proper cleanup pattern with revokeBlobPreviewUrl), line 2583 (removeEditingUserMessageImage proper cleanup).
| const unsubscribe = useStore.subscribe((state) => { | ||
| const stillPresent = | ||
| state.threads | ||
| .find((thread) => thread.id === threadId) | ||
| ?.messages.some((message) => message.id === messageId) ?? false; | ||
| if (!stillPresent) { | ||
| finish(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🟡 Medium components/ChatView.logic.ts:157
waitForThreadMessageRemoval has a race condition: if the message is removed between the initial hasMessage() check and the useStore.subscribe() call, the subscription callback never fires and the function waits the full timeout before resolving. After subscribing, re-check the condition immediately to handle the case where the state already changed.
- const unsubscribe = useStore.subscribe((state) => {
+ const unsubscribe = useStore.subscribe((state) => {
const stillPresent =
state.threads
.find((thread) => thread.id === threadId)
@@ -168,6 +168,9 @@ export function waitForThreadMessageRemoval(
if (!stillPresent) {
finish();
}
});
+
+ if (!hasMessage()) {
+ finish();
+ }🤖 Copy this AI Prompt to have your agent fix this:
In file apps/web/src/components/ChatView.logic.ts around lines 157-165:
`waitForThreadMessageRemoval` has a race condition: if the message is removed between the initial `hasMessage()` check and the `useStore.subscribe()` call, the subscription callback never fires and the function waits the full timeout before resolving. After subscribing, re-check the condition immediately to handle the case where the state already changed.
Evidence trail:
apps/web/src/components/ChatView.logic.ts lines 128-170 at REVIEWED_COMMIT. The function `waitForThreadMessageRemoval` calls `hasMessage()` at line 140, then subscribes at line 156 without re-checking the condition after subscribing. The subscription callback (lines 157-163) only fires on future state changes, not on current state, confirming the race condition exists.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| } | ||
| return nextImages.flatMap((image) => (image ? [image] : [])); | ||
| }); | ||
| }, []); |
There was a problem hiding this comment.
Race condition: async image loading overwrites wrong edit session
Medium Severity
onStartEditUserMessage (with [] deps) sets editingUserMessageId synchronously but then awaits image materialization. If the user clicks Edit on message A (with slow-to-fetch images), then quickly clicks Edit on message B, B's ID and text are set immediately, but when A's Promise.all resolves afterward, setEditingUserMessageImages overwrites B's images with A's materialized images. The user would be editing message B but seeing (and potentially submitting) message A's images.
| </div> | ||
| </div> | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Editing text prop causes all user rows to re-render
Low Severity
editingUserMessageText and editingUserMessageImages are passed as props to every EditableUserMessageTimelineRow instance. Since memo does shallow comparison, every visible user-message row re-renders on each keystroke during editing, even though only the actively-edited row needs the value. Additionally, onSubmitEditUserMessage is not wrapped in useCallback, producing a new function reference every render, which further defeats memo for all rows at all times.
Additional Locations (1)
ApprovabilityVerdict: Needs human review 3 blocking correctness issues found. This PR adds a new user-facing feature (inline message editing with images) introducing new state management, async workflows, and UI components. Additionally, an open review comment identifies a potential race condition bug with async image loading that should be addressed before merging. You can customize Macroscope's approvability policy. Learn more. |
1 participant
What changed
submitComposerTurn+clearComposerDraftfor normal vs edit resend.Why
Implementation summary
ChatView.logic.ts(+82 / -1):waitForThreadMessageRemoval,materializeMessageImageAttachmentForEdit,EDIT_REVERT_SYNC_TIMEOUT_MS.ChatView.tsx(+288 / -82):submitComposerTurn;onRevertToTurnCounttakes optional{ confirm?: boolean }(default / true = dialog;false= silent revert for edit→send), returnsPromise<boolean>; edit state + handlers.MessagesTimeline.tsx(+310 / -80): pencil, inline editor (text/images, shortcuts), wires new props fromChatView.MessagesTimeline.test.tsx(+22 / -0): props updated for new timeline API.MessagesTimeline.virtualization.browser.tsx(+11 / -0): harness props updated.Credits
Inspiration and reference implementation: pingdotgg/t3code#346 — feat: Add inline user message editing (@6crucified). That PR established the core UX and mechanics (inline bubble editor, silent revert, wait for the old message row to disappear, then resend; shared send helper with a flag to preserve vs clear the composer; image attachment handling in the editor).
This change cleans up and adapts that approach for the current codebase:
MessagesTimeline(instead of a monolithicChatViewbubble), the present composer/send pipeline (submitComposerTurn, terminal context handling,deriveDisplayedUserMessageStatefor editable text), and updated tests/harness.UI changes
Order of assets:
Before
After
Demo (video)
download.mp4
Checklist
Note
Medium Risk
Adds a new edit→silent-revert→resend flow that coordinates UI state, store synchronization, and attachment handling; mistakes could cause lost drafts, stale timeline rows, or leaked blob URLs. Changes are localized to chat UI but touch send/revert logic and async orchestration timing.
Overview
Adds inline editing for revert-eligible user messages directly in the timeline, including updating text and managing image attachments (add via file picker/paste, remove, keyboard shortcuts, and disabled states while busy).
Refactors message sending into a shared
submitComposerTurnhelper so normal composer sends clear the draft while edit-resends preserve the main composer; the edit-submit path performs a silent checkpoint revert, waits (bounded byEDIT_REVERT_SYNC_TIMEOUT_MS) for the old message to be removed from the store, then dispatches the new turn.Introduces utilities in
ChatView.logic.tsto wait for message removal and to re-materialize timeline image attachments intoFiles for editing, and updates timeline tests/virtualization harness for the expandedMessagesTimelineprops API.Written by Cursor Bugbot for commit 449f3ab. This will update automatically on new commits. Configure here.
Note
Add inline editing for user messages in the chat timeline
EditableUserMessageTimelineRowcomponent to MessagesTimeline.tsx that renders user messages with an Edit button in read-only mode and an inline editor (text + images) in edit mode.submitComposerTurnin ChatView.tsx to share the send pipeline between normal sends and edit submissions.📊 Macroscope summarized 449f3ab. 5 files reviewed, 3 issues evaluated, 0 issues filtered, 3 comments posted
🗂️ Filtered Issues