fix(chat): settle screenshots chain-insert at the turn instead of after your next message - #578
Conversation
…r your next message In computer/browser modes every turn ends with a screen capture that runs AFTER the bot goes idle (a real screenshot round trip). A fast follow-up send — or the steer-queue drain — landed first, and the frame then chained onto the new leaf, stranding the user's message ABOVE the screenshot. In browser mode this reproduced on nearly every send. The fold now anchors the frame to the turn's actual last message before the capture starts, and the store gains insertMessageAfter: when the anchor is no longer the leaf, the artifact is inserted mid-chain and the anchor's children are re-parented onto it, so the transcript reads turn → screenshot → follow-up and the leaf stays with the user's message. The renderer adopts a new message as the leaf only when it actually chains onto the current one, matching the server rule. Room threads keep the old append (their order is array-based, not chained); called out in the fold comment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe store now supports inserting messages after an active-chain anchor. One-to-one turn completion uses this path for final screenshots. The client preserves a newer follow-up as the active leaf when a late screenshot arrives. ChangesScreen settlement ordering
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🟡 Moderate · up to The change fixes screenshot ordering during live conversations, but persisted transcript state can still differ from the live branch after a restart, and an interrupted or repeated settlement can leave broken parent links or duplicate screenshots. The PR is not merge-ready until these persistence and retry risks are fixed or explicitly accepted by the owner. Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant TurnCompletion
participant Store
participant MessageDB
participant ClientState
TurnCompletion->>TurnCompletion: Capture settleLeafId
TurnCompletion->>Store: Insert final screen after settleLeafId
Store->>MessageDB: Persist screen and parent updates
Store->>ClientState: Emit message and message.patch events
ClientState->>ClientState: Preserve activeLeafId for late artifact
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description explains what changed, why it changed, how it was verified, the scope limitation for room threads, and the checklist status. The omitted screenshots section is not required because this is not a UI change.
✨ Finishing Touches 💡 1📝 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 |
1 participant
What changed
server/index.tsturn.completed fold: the settle-time screen capture records the turn's real last message (store.activePath(...).at(-1)) before the async capture, and delivers the frame via chain-insert instead of a plain append.server/store.ts: newinsertMessageAfter(threadId, anchorId, message)— plain append when the anchor is still the leaf (or unknown); otherwise the artifact is parented to the anchor and the anchor's children are re-parented onto it, announced asmessage+message.patchso live clients converge. Screen-frame pruning still applies.src/state/store.tsx:messageAddedadopts the incoming message asactiveLeafIdonly when it chains onto the current leaf — a mid-branch insert leaves the leaf (the user's follow-up) in place.Why
Field report: with browser mode on, a message sent right after a turn appeared above the last message. Root cause:
void finalScreenFrame(...).then(pushMessage)runs afterbusyclears, so a quick send (or the steer-queue drain) appends first and the screenshot then chains after it — and browser/computer turns end with that capture, making it near-deterministic.After: the transcript reads
turn → screenshot → your message, and your message stays the tail.How it was verified
server/store.test.ts: mid-chain insert yieldsanchor → artifact → follow-upwith the leaf preserved and the re-parent announced; anchor-still-leaf and unknown-anchor degrade to plain appends.src/state/store.test.ts: reducer keeps the leaf for a mid-branch insert, adopts it for a chaining message.pnpm typecheckclean; lint parity with main on every touched file.Known scope cut
Room threads render by array order, not parent chains, so a room's settle frame keeps the old append (noted in the fold comment).
Checklist
pnpm typecheckand the touched test files pass locallydist-server/edits · no macOS-only code · noshell: true· no secrets🤖 Generated with Claude Code
Summary by CodeRabbit