Skip to content

Commit eb2db57

Browse files
committed
fix(files): fail-close base-less previews + operation-based stream hold
Cursor/Greptile round 3 (High + Medium) — remove the fragile string-prefix "extend gate", which was the root of both findings: - Server: `buildFilePreviewText` now fails closed for an `append` whose base content hasn't loaded (returns undefined, like patch/update), so a base-less fragment never reaches the client. This eliminates the base-less wipe at settle (Greptile P1) at the source; an empty file (existingContent === '') still previews normally. - Client: the collab streaming tick no longer string-prefixes the raw preview against the editor's canonical markdown (the '*' vs '-' / emphasis mismatch that froze every append frame — Cursor). The mid-stream hold is now purely operation-based: `update` waits for settle; append/patch/create apply each frame via the (peer-safe) shadow reconcile. lastStreamedBodyRef is now a plain dedup guard, not a prefix baseline. Keeps the shadow, durable write, and SYNC_NO_PERSIST unchanged.
1 parent db571c4 commit eb2db57

3 files changed

Lines changed: 53 additions & 33 deletions

File tree

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx

Lines changed: 22 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,10 @@ interface RichMarkdownEditorProps {
8585
*/
8686
streamIsIncremental?: boolean
8787
/**
88-
* The agent edit operation driving the stream, when known (`create`/`append`/`update`/`patch`). Used
89-
* only to relax the "must extend" gate for `patch` (which legitimately replaces a mid-document region):
90-
* every other operation's snapshot must extend what's shown, so a base-less `append` fragment can't
91-
* reconcile the live doc to a wipe.
88+
* The agent edit operation driving the stream, when known (`create`/`append`/`update`/`patch`). In the
89+
* collaborative path it decides only whether to stream mid-flight: an `update` (from-scratch rewrite) is
90+
* HELD until settle so the open doc doesn't collapse to a partial result, while `append`/`patch`/`create`
91+
* apply each frame.
9292
*/
9393
streamOperation?: string
9494
disableStreamingAutoScroll?: boolean
@@ -339,11 +339,11 @@ export function LoadedRichMarkdownEditor({
339339
streamingAtMountRef.current ? null : splitFrontmatter(content).body
340340
)
341341
/**
342-
* The body the AGENT last streamed into the collaborative doc — the extend-gate baseline for the collab
343-
* streaming path. Written ONLY at stream start (snapshotting the pre-stream base) and by the streaming
344-
* tick, never by `onUpdate`, so a concurrent PEER edit (which does clobber {@link lastSyncedBodyRef} via
345-
* `onUpdate`) can't make the agent's growing snapshot stop prefixing the shown body and stall the stream.
346-
* Reset to `null` on settle so the next stream re-captures its own baseline.
342+
* The body the AGENT last applied into the collaborative doc — a dedup guard for the collab streaming
343+
* tick, so an unchanged frame skips a redundant shadow reconcile/reparse. Written ONLY by the streaming
344+
* tick (never by `onUpdate`), and reset to `null` on settle for the next stream. It is NOT a string-prefix
345+
* baseline: the mid-stream hold is decided by operation (`update` waits for settle), not by comparing the
346+
* raw preview against the editor's canonical markdown.
347347
*/
348348
const lastStreamedBodyRef = useRef<string | null>(null)
349349
const onChangeRef = useRef(onChange)
@@ -872,13 +872,10 @@ export function LoadedRichMarkdownEditor({
872872
// re-runs and applies once it lands (the read-only placeholder shows the base content meanwhile —
873873
// see `showPlaceholder`).
874874
if (!collabReady) return
875-
// Open the stream's shadow on the FIRST ready frame — BEFORE the extend gate — so its shadow
876-
// captures the pre-stream base (immune to later peer edits) even for an `update` whose every frame
877-
// is gated out until settle. Snapshot that base as the agent's private extend-gate baseline.
878-
if (agentStreamSessionRef.current === null) {
879-
agentStreamSessionRef.current = beginAgentStream(editor)
880-
lastStreamedBodyRef.current = lastSyncedBodyRef.current
881-
}
875+
// Open the stream's shadow on the FIRST ready frame so it captures the pre-stream base (immune to
876+
// later peer edits) — including for an `update`, whose frames are all held until settle, so settle
877+
// still has a shadow through which to apply the final rewrite.
878+
agentStreamSessionRef.current ??= beginAgentStream(editor)
882879
const session = agentStreamSessionRef.current
883880
const body = splitFrontmatter(content).body
884881
if (body === lastStreamedBodyRef.current) return
@@ -890,15 +887,11 @@ export function LoadedRichMarkdownEditor({
890887
streamRafRef.current = null
891888
return
892889
}
893-
const shownBody = lastStreamedBodyRef.current
894-
const extendsShown = shownBody === null || pending.startsWith(shownBody)
895-
// Every snapshot except a mid-document `patch` must EXTEND what the AGENT last streamed: a
896-
// from-scratch rebuild (`create`/`update`) is only revealed as it grows, and an `append`
897-
// snapshot that doesn't extend the base is a base-less fragment (the server emits one before the
898-
// base loads) which would reconcile the seeded doc down to a wipe. Only `patch` replaces a
899-
// mid-region. The baseline is the agent's own last frame (not the editor body), so a concurrent
900-
// peer edit can't make the growing snapshot stop prefixing it and stall the stream.
901-
if (!extendsShown && streamOperationRef.current !== 'patch') {
890+
// Hold a from-scratch rewrite (`update`) until settle so the open doc doesn't collapse to a
891+
// partial rewrite mid-stream (matching `main`). `append`/`patch`/`create` apply each frame — the
892+
// shadow reconcile is peer-safe, and base-less `append` fragments no longer reach the client (the
893+
// server fail-closes them), so there is nothing here to string-prefix or wipe-guard against.
894+
if (streamOperationRef.current === 'update') {
902895
streamRafRef.current = null
903896
return
904897
}
@@ -929,11 +922,10 @@ export function LoadedRichMarkdownEditor({
929922
cancelAnimationFrame(streamRafRef.current)
930923
streamRafRef.current = null
931924
}
932-
// Settle: apply the FINAL body once so the Y.Doc exactly equals the streamed result — even when
933-
// every mid-stream frame was gated out (an `update` rewrite never extends the base) or the stream
934-
// finished before the seed, cases where no frame applied. Reuse the stream's shadow when it exists
935-
// (seeded from the pre-stream base, so peer edits survive); otherwise open one on demand. The
936-
// durable server write then lands as a noop diff.
925+
// Settle: apply the FINAL body once so the Y.Doc exactly equals the streamed result — even when no
926+
// frame applied mid-stream (an `update` is held until settle, or the stream finished before the
927+
// seed). Reuse the stream's shadow when it exists (seeded from the pre-stream base, so peer edits
928+
// survive); otherwise open one on demand. The durable server write then lands as a noop diff.
937929
if (wasStreamingRef.current && collabReady) {
938930
wasStreamingRef.current = false
939931
const finalBody = splitFrontmatter(content).body

apps/sim/lib/copilot/tools/server/files/file-preview.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,29 @@ describe('buildFilePreviewText', () => {
3131
).toBe('line one\nline two')
3232
})
3333

34+
it('fails closed (returns undefined) for an append when the base content has not loaded', () => {
35+
// A base-less append preview is just the streamed fragment; a collaborative editor applying it as the
36+
// full body would reconcile the seeded doc down to that fragment (a wipe). It must fail closed until
37+
// the base loads, exactly like patch/update.
38+
expect(
39+
buildFilePreviewText({
40+
operation: 'append',
41+
existingContent: undefined,
42+
streamedContent: 'orphan fragment',
43+
})
44+
).toBeUndefined()
45+
})
46+
47+
it('still previews an append into an EMPTY file (existingContent is "", not undefined)', () => {
48+
expect(
49+
buildFilePreviewText({
50+
operation: 'append',
51+
existingContent: '',
52+
streamedContent: 'first line',
53+
})
54+
).toBe('first line')
55+
})
56+
3457
it('applies anchored replace_between previews', () => {
3558
expect(
3659
buildFilePreviewText({

apps/sim/lib/copilot/tools/server/files/file-preview.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -182,10 +182,15 @@ export function buildFilePreviewText({
182182
}
183183

184184
if (operation === 'append') {
185-
if (existingContent !== undefined) {
186-
return buildAppendPreview(existingContent, streamedContent)
185+
// Fail closed (like `patch`/`update` below) when the base file content has not loaded yet: a base-less
186+
// `append` preview is just the streamed fragment, and a collaborative editor applying it as the full
187+
// body would reconcile the seeded doc down to that fragment (a wipe). Skipping the preview until the
188+
// base is available costs only a brief render delay; the final durable `edit_content` write is
189+
// authoritative. An empty file has `existingContent === ''` (defined), so it is unaffected.
190+
if (existingContent === undefined) {
191+
return undefined
187192
}
188-
return streamedContent
193+
return buildAppendPreview(existingContent, streamedContent)
189194
}
190195

191196
if (existingContent === undefined) {

0 commit comments

Comments
 (0)