Uh oh!
There was an error while loading. Please reload this page.
fix(files): image drag-reorder move + unwedge post-stream editor + bullet/image Backspace fixes - #5608
Conversation
…troying the image below it Reported: clearing an empty bullet with an image after it "nuked the bullet point and the image". Reproduced: when the emptied bullet is the doc's first block, removeEmptyWrappedBlock's Selection.near(resolve(start), -1) finds no text position behind it and silently lands a NodeSelection on the FOLLOWING image — so the user's next keystroke is destructive (a second Backspace while clearing deletes the image; typing replaces it). Only-first-block explains why it wouldn't re-repro. The selection left behind is now always a caret: end of the previous textblock first, else a gap cursor at the deletion point when the neighbour is a leaf (typing there inserts a new block instead of replacing the image), else the next textblock. The regression test surfaced two adjacent gap-cursor crashes on Backspace, both reachable on current staging whenever a gap cursor exists (e.g. between two dividers/images, the data-gap-between-leaves state): - our own handler threw RangeError from $from.before(0) on a depth-0 (doc-start) gap cursor - with that guarded by falling through, TipTap's blockquote Backspace handler crashes on the same resolution ($from.node(-1) is undefined) — so a doc-start gap cursor consumes the key instead (there is nothing before it for Backspace to act on)
…its, so the editor can't wedge read-only Reported: images "don't get selected sometimes" (can't grab/drag/resize, doc uneditable) until a full refresh. Reproduced in a real-Chromium harness driving the actual RichMarkdownEditor + engine: after an agent stream settles, the reconcile phase exits only when a fetch shows the server content advanced past the pre-stream baseline — but that exit had no retry. A single refetch racing the agent's write (or the mothership invalidation never reaching this surface — it's the only place that invalidates this query) left the editor locked read-only indefinitely: contenteditable=false, images not grabbable, until refetchOnWindowFocus or a reload happened to run. Fix: while (and only while) the reducer is in `reconciling`, the content query polls via react-query's refetchInterval — the same pattern this module already uses for the generated-doc 409 polling, and like it, bounded (45s window; past that the write has almost certainly failed and refetchOnWindowFocus remains the recovery). The interval is the function form reading the phase through a ref, re-evaluated by react-query after every fetch, so polling stops the moment a fetch advances without needing an extra render. Harness (real Chromium, real editor + engine, in-memory server): pre-fix the editor stays editable=false with fetches frozen at 1 indefinitely while the server holds the new content; post-fix it unlocks within one poll (~1.5s), polling stops immediately after finalize, and the streamed image click-selects. Unit tests drive stream -> settle -> advance through the real engine and assert the interval flips on/off with the phase (2 of 3 fail pre-fix), plus the bounded window and a no-polling guard for plain at-rest editing.
The latest updates on your projects. Learn more about Vercel for GitHub. |
waleedlatif1
commented
Jul 11, 2026
waleedlatif1
commented
Jul 11, 2026
@cursor review |
PR SummaryMedium Risk Overview Rich markdown Backspace after removing an empty list/blockquote item no longer leaves a NodeSelection on a neighboring image/divider ( Brex Get Company maps Reviewed by Cursor Bugbot for commit 28fdba0. Configure here. |
Greptile SummaryThis PR fixes several file-editor edge cases. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (4): Last reviewed commit: "fix(files): degrade reconcile polling to..." | Re-trigger Greptile |
Uh oh!
There was an error while loading. Please reload this page.
Greptile SummaryThis PR fixes two editor interaction paths around streamed file content and Backspace handling. The main changes are:
Confidence Score: 4/5The changed keymap path needs a fix before merging.
apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/keymap.ts Important Files Changed
Reviews (2): Last reviewed commit: "fix(files): poll the content query while..." | Re-trigger Greptile |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit adcbe0a. Configure here.
…-query Both consumers' test setups (the reconcile unit tests and the browser harness) replace @/hooks/queries/workspace-files, so the real hook's two changed lines were exercised by nothing but the type-checker. These render the real useWorkspaceFileContent under a real QueryClientProvider with a stubbed fetch: no polling by default, polling with a numeric interval, and the function form re-evaluated so flipping its condition stops the polling — the exact mechanism the reconcile fix depends on. The two polling tests fail against the pre-fix hook.
…opping; prove findFrom textOnly never leaf-selects Greptile round 1: - Real gap in my bounded window: past 45s the poll stopped outright, leaving the reducer wedged in reconciling with only focus-refetch/reload as recovery — the exact failure shape this PR exists to remove, just later. Polling now degrades to a 15s cadence instead of stopping, so a write landing late (slow job, replica catch-up) is still picked up automatically; react-query pauses interval refetches in background tabs by default, so an abandoned doc doesn't poll unattended. - Refuted with source + an executable test: Selection.findFrom($gap, -1, true) cannot return a NodeSelection — prosemirror-state's findSelectionIn skips atoms entirely under textOnly (`!text && isSelectable`). New regression test pins the exact scenario (image directly BEFORE the emptied bullet): backward search returns null, the gap-cursor/forward-caret branches take over, and the image is never silently selected.
waleedlatif1
commented
Jul 11, 2026
waleedlatif1
commented
Jul 11, 2026
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 28fdba0. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
waleedlatif1
commented
Jul 12, 2026
waleedlatif1
commented
Jul 12, 2026
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 28fdba0. Configure here.
Summary
<img>turns into a native drag+drop-on-self — killed the ring and duplicated too, reading as "I can't select this image"). Root causes empirically pinned: TipTap node-view dragstart bypasses PM's drag serialization entirely (verified in @tiptap/core source — no PMtext/html, noview.dragging; it does NodeSelect the node), so the drop carries only the browser's native enrichment whose<img src>is the ABSOLUTE rendered URL — which both hosted-image recognizers rejected. Fix: origin-aware src normalization (toSameOriginPath, deliberatelywindow.location.origin— the browser serializes against the actual viewing origin, not the configuredNEXT_PUBLIC_APP_URL) +handleDropperforms the internal move itself when the drop's html references the currently-selected image node (PM's default would parse-copy with the display-layer src and never delete the original). Drop-on-self is a no-op that keeps the selection ring. The paste-clone path had the same absolute-URL gap for browser-native "Copy Image" — also fixed.contenteditable=falseforever with the server holding the new content) and verified recovering in one poll post-fix.Selection.nearused to silently NodeSelect the following image, making the next keystroke delete/replace it. The regression test also surfaced two pre-existing gap-cursor Backspace crashes (ours and TipTap's blockquote handler), both fixed.Type of Change
Testing
RichMarkdownEditor+ engine (network stubbed with an in-memory server mirroring the real save→invalidate→refetch dynamics): drag-reorder before/after (duplicate+no-move → clean single move, stays selected), real-mouse micro-drag gesture proof (becomes native drag, never a click), reconcile wedge before/after, paste-clone for relative and absolute payloads, plus regression sweeps over fresh-upload/click/focus-refetch/save-cycle scenarios.Checklist