feat: implement message editing and thread rollback functionality with UI support#592
feat: implement message editing and thread rollback functionality with UI support#592zip700 wants to merge 6 commits intoDimillian:mainfrom
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 330794611a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b481d27d8f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
src/utils/threadItems.conversion.ts
Outdated
| import { asNumber, asString } from "./threadItems.shared"; | ||
|
|
||
| function extractTurnId(record: Record<string, unknown>) { | ||
| return asString(record.id ?? record.turnId ?? record.turn_id).trim(); |
There was a problem hiding this comment.
extractTurnId currently reads record.id before record.turnId/record.turn_id, so payloads that include both identifiers will store the message item id as turnId. editAndRegenerateMessage then trusts that non-empty value and skips the refresh fallback, which can send rollback_thread the wrong identifier and fail rollback (or rewind the wrong turn) when item ids and turn ids differ.
Useful? React with 👍 / 👎.
| editText={isEditingThis ? editText : undefined} | ||
| isConfirming={isEditingThis ? isConfirmingEdit : undefined} | ||
| isRegenerating={isEditingThis ? isRegeneratingEdit : undefined} | ||
| onStartEdit={isThinking ? undefined : onStartEdit} |
There was a problem hiding this comment.
Messages only gates onStartEdit on isThinking, but not on isRegeneratingEdit. During rollback/regenerate, only the active row receives the regenerating flag, so other user messages can still enter edit mode; this races with the first executeRegenerate completion path (which clears edit state) and can drop the newly opened edit session.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c4451246b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const trimmed = editState.editText.trim(); | ||
| if (!trimmed) { | ||
| return; |
There was a problem hiding this comment.
executeRegenerate hard-blocks whenever the edited text trims to empty, but startEdit preserves attached images and sendMessageToThread already allows sends where text is empty and images are present. This means editing/regenerating a user prompt that only contains images (or intentionally clearing text while keeping images) can never proceed, so image-based turns cannot be corrected via the new edit flow.
Useful? React with 👍 / 👎.
zip700
force-pushed
the
issue-511-edit-regenerate-user-messages
branch
from
4c44512 to
5fe9c04
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5fe9c049e2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| await refreshThread(workspace.id, threadId); | ||
| return readTurnId(); |
There was a problem hiding this comment.
The fallback path in resolveRollbackTurnId calls refreshThread(...) and immediately re-reads via getItemsForThread, but in useThreads that getter is backed by itemsByThreadRef.current (updated on render), so this second read can still see pre-refresh items and return null. In the exact scenario this fallback is meant to handle (messages missing turnId), edit/regenerate can fail with "Failed to determine the turn for the edited message" even after a successful refresh.
Useful? React with 👍 / 👎.
1 participant
PR Architecture Summary: Edit and Regenerate Message History (Issue #511)
*Disclaimer: This Pull Request and summary were structurally generated and organized by an AI Assistant.
High-Level Objective
This PR implements the "Edit and Regenerate" feature for message history, allowing users to modify an existing message mid-thread and seamlessly rewind the conversational context to branch off from that point.
Explicit Architectural Choices
State Isolation (
useMessageEdit.ts)Instead of ballooning
useMainAppLayoutSurfacesoruseThreadswith transient UI state for hovering, editing, and prompt validations, we created a dedicated hookuseMessageEdit.ts. This encapsulates text mutation, image retention, UI transition states (idle, editing, confirming, regenerating), and validation guards natively.Frontend Reducer Action (
truncateThreadItems)Truncation of orphaned thread items (any downstream context succeeding the edited turn ID) was explicitly wired into
useThreadsReducer.ts. The UI optimistically and cleanly prunes these items leveragingafterItemIdimmediately following a successful network rollback, prior to sending the new generation request. It utilizes the existing thread hierarchy invariants safely.Backend / Daemon Parity
Following backend routing rules, the new
rollback_threadcommand traverses all necessary stacks cleanly:rollback_thread_coreinsrc-tauri/src/shared/codex_core.rs.src/services/tauri.ts.src-tauri/src/bin/codex_monitor_daemon/rpc/codex.rs.remote_backendremote switch logic properly withinsrc-tauri/src/codex/mod.rs.UI/UX Aesthetics & Design System Reuse
Rather than introducing a global top-level modal for editing logic, the message edit menu and confirmation warning inject conditionally via
MessageRows.tsx. All CSS builds purely upon existing tokenized primitives (--surface-accent,--cm-surface-panel-strong, etc.) avoiding drift and duplicated components as outlined in theAGENTS.mdguidelines.Areas of Consideration / Potential Concerns
The
useMessageEditdisables interactable elements via anisRegeneratingstate guard. In addition, the controller (useThreadMessaging.ts) double-checksthreadStatusById[threadId]?.isProcessingto prevent an incoming rollback while a background thought is actively polling.Currently, if
rollbackThreadService(...)throws an error on the Rust backend, the subsequenttruncateThreadItemsaction is bypassed, and the UI will push a standard generic Error Message into the chat stream viapushThreadErrorMessage. This prevents local desync where the UI wipes out items but the database retains them, guaranteeing server authority.Images present in previous prompts are automatically extracted, buffered in
EditState, and re-attached dynamically into the new regenerated payload without requiring the user to manually re-upload them.Addressed implicit/explicit typing where applicable. For example,
rollback_threadandarchive_threadIPC services return strictly typedinvoke<void>instead ofinvoke<any>.