fix(frontend): rewind forks into a new session instead of resurrecting - #5860
Open
mmabrouk wants to merge 1 commit into
Open
fix(frontend): rewind forks into a new session instead of resurrecting#5860mmabrouk wants to merge 1 commit into
mmabrouk wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
dosubot
Bot
added
size:L
📝 WalkthroughSummary by CodeRabbit
WalkthroughRewind now creates a new session from a retained message prefix. User rewinds restore an editable draft. Assistant rewinds schedule a rerun. Unlogged fork history is replayed in full requests and cleared after successful completion. ChangesRewind fork replay
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant AgentConversation
participant rewindForkAtomFamily
participant useAgentChatSession
participant buildAgentRequest
AgentConversation->>rewindForkAtomFamily: Create fork from retained messages
rewindForkAtomFamily-->>AgentConversation: Return fork session ID
AgentConversation->>useAgentChatSession: Regenerate trailing user turn
useAgentChatSession->>buildAgentRequest: Build request with replayHistory
buildAgentRequest-->>useAgentChatSession: Send full conversation history
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 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 |
Contributor
Railway Preview Environment
|
Mahmoud's bug: rewind to the first message, resend, and moments later the whole 'rewound' conversation returns - hydration re-adopts the server records under the same session id. Chose the FORK model (new session id at the rewind point) because the truncation was broken in TWO layers: records are append-only with no branch pointer (a local truncation always loses to the next hydration), and the runner's continuity would keep answering from its own native transcript even if the UI hid the records - a silent version of the same bug. The fork: rewindFork mints the new session id, keeps the sliced prefix, and the fork's FIRST request carries the transcript (replayHistory on buildAgentRequest - the prefix was logged under the OLD id), cleared at first onFinish, after which runner continuity owns it. The original session stays whole under its own id in History. No hydration change needed: shouldAdoptServerTranscript's count floor already rejects the fork's subset transcript (pinned by a named test). One info toast tells the user the tab became a new chat. 158 + 216 + 955 tests pass; tsc + eslint clean.
mmabrouk
force-pushed
the
fix/rewind-session-hydration
branch
from
August 10, 2026 22:15
c17d554 to
14b8837
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/packages/agenta-playground/tests/unit/agentRequest.test.ts (1)
253-262: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace
anyin the replay request test.Line 260 uses
any. Workspace packages must avoidany. Use a narrow structural assertion forrequestBody.data.Proposed fix
- expect((req!.requestBody.data as any).inputs.messages).toEqual([u1, a1, u2]) + const data = req!.requestBody.data as {inputs: {messages: unknown[]}} + expect(data.inputs.messages).toEqual([u1, a1, u2])Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 479ed2ba-5480-4f1d-be45-44b6254c94d0
📒 Files selected for processing (8)
web/oss/src/components/AgentChatSlice/AgentConversation.tsxweb/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.tsweb/oss/src/components/AgentChatSlice/state/rewindFork.test.tsweb/oss/src/components/AgentChatSlice/state/rewindFork.tsweb/oss/src/components/AgentChatSlice/state/sessionEphemera.tsweb/packages/agenta-entities/tests/unit/session-transcript-adoption.test.tsweb/packages/agenta-playground/src/state/execution/agentRequest.tsweb/packages/agenta-playground/tests/unit/agentRequest.test.ts
Comment on lines
+528
to
+535
| rewindFork({ | ||
| fromSessionId: sessionId, | ||
| messages: msgs.slice(0, idx), | ||
| // A user turn re-opens for editing; an assistant turn re-runs the turn as-is | ||
| // (its user message, attachments included, is the prefix's last message). | ||
| draft: isUser ? messageText(message) : undefined, | ||
| rerun: !isUser, | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Preserve attachments during a user-side rewind.
When the selected user message has file parts, Line 530 removes that message from the fork prefix and Line 533 retains only its text. The fork then cannot re-send the original attachments.
Store a typed draft payload that includes text and file parts, and initialize both composer states for the fork. Add a user-rewind test with an attachment-only message.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mahmoud's bug: rewind to the first message, resend it, and moments later the entire "rewound" conversation reappears — hydration refetches the append-only server records under the same session id and merges them back.
Why FORK (a new session id), not supersede or anchor-respecting hydration
The rewind was broken in TWO layers, and only a new id fixes both:
session/load) answers from its own native transcript; the wire protocol has no "continue from turn N". Hiding records client-side would have made the UI show a truncated chat while the model answers as if nothing was cut — the same bug, made silent.The fork mechanics
Rewind mints a new session id, keeps the sliced prefix locally, and the fork's FIRST request carries the transcript explicitly (
replayHistoryonbuildAgentRequest) because the prefix was logged under the old id; the flag clears at the firstonFinish, after which the runner's own continuity carries the conversation. The original session stays whole under its own id, still in History. No hydration change needed —shouldAdoptServerTranscript's existing count floor already rejects the fork's subset transcript (pinned by a named test). An info toast says "Rewound into a new chat — the original is in History."Also recorded for later
agenta-chat/src/hooks/useAgentConversation.ts:520,546carries the same un-forked rewind, unconsumed today — needs the same treatment before anything ships it.Verification
158 (AgentChatSlice) + 216 (playground) + 955 (entities) tests pass, including new ones for the fork atom, the replayHistory request shape, and the subset-rejection floor. tsc + eslint clean. Live repro for Mahmoud: converse, rewind to message one, resend — the old conversation must NOT come back; the original must still sit complete in History.