From e226143f4ced98e59ebdea812dcc800ad3d0cfb3 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Tue, 15 Sep 2026 14:02:16 +0200 Subject: [PATCH] fix: open an item on the row its syncer could not send Co-Authored-By: Claude Opus 5 (1M context) --- frontend/src/lib/itemStore.svelte.ts | 21 +++++++++++++++----- frontend/src/lib/itemStore.test.ts | 11 ++++++++-- frontend/src/lib/userDraftDbSyncer.svelte.ts | 11 ++++++++++ 3 files changed, 36 insertions(+), 7 deletions(-) diff --git a/frontend/src/lib/itemStore.svelte.ts b/frontend/src/lib/itemStore.svelte.ts index 63df04dbc1..edfcf92ac9 100644 --- a/frontend/src/lib/itemStore.svelte.ts +++ b/frontend/src/lib/itemStore.svelte.ts @@ -91,6 +91,8 @@ export type ItemRowPort = { /** Rows handed to the syncer for `key` so far, by any writer, counted before the debounce * rather than on the response. Handed back to `seedSync` to order it. */ rowMark(key: ItemKey): number + /** A row waiting for another attempt, if any: `undefined` when none, `null` for a delete. */ + pending(key: ItemKey): unknown | undefined seedSync(key: ItemKey, draftSavedAt: string | undefined, since: number): void conflicted(key: ItemKey): boolean failure(key: ItemKey): string | undefined @@ -284,14 +286,14 @@ class Entry { * `last_sync`: one fact, so taken together and against one `asOf`, the rows sent when the * read was issued. Seeding behind a row sent since puts `last_sync` before what the syncer * has already sent, which the server refuses from then on. */ - private adoptRow(key: ItemKey, draft: unknown, draftSavedAt: string | undefined, asOf: number) { + private adoptRow(key: ItemKey, row: unknown, draftSavedAt: string | undefined, asOf: number) { if (this.ports.rowMark(key) !== asOf) return // A payload the server *refused* predates this read and the baseline taken below would // make it acceptable, so it goes; the reconcile after the read re-queues whatever the // value needs. One merely unsent — a network failure the syncer parks to retry — is the // only record of an edit that never reached anyone, so it stays. if (this.ports.conflicted(key)) this.ports.dropPending(key) - this.row = serialize(draft) ?? null + this.row = row === null || row === undefined ? null : (serialize(row) ?? null) this.ports.seedSync(key, draftSavedAt, asOf) } @@ -401,13 +403,19 @@ class Entry { } this.serverDeployed = snapshot(res.deployed) this.origin = res.deployed !== undefined ? 'deployed' : 'draft' + // A row the syncer took but could not send is newer than this response and is the only + // copy of that edit, so the item opens on it. One the server *refused* is the other + // way round — the response is what won — and `adoptRow` drops it below. + const unsent = this.ports.conflicted(key) ? undefined : this.ports.pending(key) + const row = unsent === undefined ? res.draft : unsent + const draft = (row ?? undefined) as V | undefined // The deployed side above always advances: the item did change, and a discard after // the user keeps theirs has to land on what the server holds now. - if (!standOff) this.adoptRow(key, res.draft, res.draftSavedAt, rowsAtRead) + if (!standOff) this.adoptRow(key, row, res.draftSavedAt, rowsAtRead) this.loaded = true if (!keepValue) { - this.pristine = this.settles && this.origin === 'deployed' && res.draft === undefined - this.replaceValue(res.draft ?? res.deployed) + this.pristine = this.settles && this.origin === 'deployed' && draft === undefined + this.replaceValue(draft ?? res.deployed) } this.removed = this.value === undefined this.reconcile() @@ -1112,6 +1120,9 @@ const syncerRows: ItemRowPort = { rowMark(key) { return UserDraftDbSyncer.sendsSoFar(keyQuery(key)) }, + pending(key) { + return UserDraftDbSyncer.pendingValue(keyQuery(key)) + }, seedSync(key, draftSavedAt, since) { UserDraftDbSyncer.recordRemoteSync(keyQuery(key), draftSavedAt, since) }, diff --git a/frontend/src/lib/itemStore.test.ts b/frontend/src/lib/itemStore.test.ts index 9d1d903ba6..9079c2951f 100644 --- a/frontend/src/lib/itemStore.test.ts +++ b/frontend/src/lib/itemStore.test.ts @@ -88,6 +88,7 @@ function fakeRows() { handed(key.path) }, rowMark: (key) => sends.get(key.path) ?? 0, + pending: (key) => (parked.has(key.path) ? parked.get(key.path) : undefined), seedSync: (key, at, since) => { // The syncer refuses a baseline from a read a row handed over since has passed. if (since !== (sends.get(key.path) ?? 0)) return @@ -1234,13 +1235,19 @@ describe('item store: conflicts', () => { const second = store.acquire(key, { workspace: 'w', path: 'u/me/r' }, a) await settle() - // The parked payload is the only record of that edit. Dropping it loses it for good. + // The parked payload is the only record of that edit, so the reopened editor has to be + // showing it: anything else and the next keystroke replaces it with nobody the wiser. expect(rows.dropped).not.toContain('u/me/r') + expect(second.handle.value?.description).toBe('never reached the server') + expect(second.handle.dirty).toBe(true) + + // Typing now builds on what is on screen, and that is what eventually lands. + second.handle.value = { ...deployedRes, description: 'and then some more' } rows.failing.delete('u/me/r') await rows.port.flush(key) expect(rows.sent.get('u/me/r')).toEqual({ ...deployedRes, - description: 'never reached the server' + description: 'and then some more' }) }) diff --git a/frontend/src/lib/userDraftDbSyncer.svelte.ts b/frontend/src/lib/userDraftDbSyncer.svelte.ts index 0af0dcc817..337f894dc6 100644 --- a/frontend/src/lib/userDraftDbSyncer.svelte.ts +++ b/frontend/src/lib/userDraftDbSyncer.svelte.ts @@ -532,6 +532,17 @@ export const UserDraftDbSyncer = { } }, + /** + * The row parked for a later attempt, when one is waiting: a payload the server refused or + * never received. It was handed over after the last successful sync, so it is newer than + * anything a read can report, and an editor opening the key shows it rather than the + * response. `undefined` when nothing is parked; `null` is a parked delete. + */ + pendingValue(query: UserDraftLastSyncQuery): unknown | undefined { + const parked = pendingSaveOpts.get(draftKey(query.workspace, query.itemKind, query.path)) + return parked ? parked.value : undefined + }, + /** Rows handed over for `query` so far, for a reader to hand back to `recordRemoteSync`. */ sendsSoFar(query: UserDraftLastSyncQuery): number { return sends.get(draftKey(query.workspace, query.itemKind, query.path)) ?? 0