From d17f60945a4cb41a70ce9985f259988d880c87de Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Tue, 15 Sep 2026 09:19:05 +0200 Subject: [PATCH] fix: count a re-read's edits from before its row went out Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015vn1jCHUcAdaG9C4NsUehF --- frontend/src/lib/itemStore.svelte.ts | 22 +++++++---- frontend/src/lib/itemStore.test.ts | 55 ++++++++++++++++++++++++++-- 2 files changed, 66 insertions(+), 11 deletions(-) diff --git a/frontend/src/lib/itemStore.svelte.ts b/frontend/src/lib/itemStore.svelte.ts index 80d26ecb1f..b3b2516d42 100644 --- a/frontend/src/lib/itemStore.svelte.ts +++ b/frontend/src/lib/itemStore.svelte.ts @@ -304,9 +304,11 @@ class Entry { this.replaceValue(template) } - load(adapter: ItemAdapter): Promise { + /** `changedBefore`: the value's change count as of when this read was decided on, for a caller + * that did something async first. */ + load(adapter: ItemAdapter, changedBefore?: number): Promise { this.loads++ - const changesAtStart = this.changes + const changesAtStart = changedBefore ?? this.changes return this.run(async () => { const key = this.key let res: ItemLoad @@ -349,9 +351,15 @@ class Entry { /** Read the item again after someone else wrote it. The edits on screen are in the draft row, * so they come back over what was written; one typed since the read was asked for stays. */ - reread(): Promise { - if (!this.loaded || this.retired || !this.adapter) return Promise.resolve({ ok: true }) - return this.load(this.adapter) + async reread(): Promise { + if (!this.loaded || this.retired || !this.adapter) return { ok: true } + // Counted before the row goes, not after it lands: an edit typed while that write is in + // flight is in neither it nor the read that follows, and is newer than both. + // Counted before the row goes, not after it lands: an edit typed while that write is in + // flight is in neither it nor the read that follows, and is newer than both. + const changesAtStart = this.changes + await this.ports.flush(this.key) + return this.load(this.adapter, changesAtStart) } /** An outside write (the AI chat, another editor): a real divergence, never settling. */ @@ -825,10 +833,8 @@ export function createItemStore(ports: ItemRowPort) { async changed(key, by) { const holder = entries.get(keyString(key)) if (holder === by) return + // Its row is debounced, so `reread` sends what is queued before reading it back. if (holder) { - // Its row is debounced: unsent, an edit typed in the last moment is not what the - // re-read would find, and the read would put the screen back before it. - await ports.flush(key) await holder.reread() return } diff --git a/frontend/src/lib/itemStore.test.ts b/frontend/src/lib/itemStore.test.ts index 87b6539ee0..72975d721c 100644 --- a/frontend/src/lib/itemStore.test.ts +++ b/frontend/src/lib/itemStore.test.ts @@ -35,6 +35,8 @@ function fakeRows() { /** What a write leaves queued, and what a flush sends: the syncer debounces. */ const queued = new Map() const sent = new Map() + /** Set by a test to keep a flush in flight. */ + let holdFlush: Promise | undefined const port: ItemRowPort = { write: (key, value) => { writes.push({ path: key.path, value }) @@ -42,9 +44,12 @@ function fakeRows() { if (failing.has(key.path)) failures.set(key.path, 'unreachable') }, flush: async (key) => { - if (!queued.has(key.path)) return - sent.set(key.path, queued.get(key.path)) + // A POST carries what was queued when it went out; what is typed while it is in flight + // queues behind it. + const going = queued.has(key.path) ? { value: queued.get(key.path) } : undefined queued.delete(key.path) + if (holdFlush) await holdFlush + if (going) sent.set(key.path, going.value) }, overwrite: async (key, value) => { conflicts.delete(key.path) @@ -56,7 +61,16 @@ function fakeRows() { dropPending: (key) => void dropped.push(key.path), hint: (key, on) => void hints.set(key.path, on) } - return { port, writes, conflicts, failing, hints, dropped, sent } + return { + port, + writes, + conflicts, + failing, + hints, + dropped, + sent, + hold: (until: Promise) => void (holdFlush = until) + } } const deployedRes: Res = { path: 'u/me/r', description: 'deployed', args: { a: 1 } } @@ -563,6 +577,41 @@ describe('item store: one entry per key', () => { expect(open.dirty).toBe(true) }) + it('keeps an edit typed while the row it re-reads behind is still going', async () => { + const rows = fakeRows() + const store = createItemStore(rows.port) + const b = { ...deployedRes, path: 'u/me/b' } + let deployed = b + const { handle: open } = store.acquire( + { workspace: 'w', kind: 'resource', path: 'u/me/b' }, + { workspace: 'w', path: 'u/me/b' }, + adapter(async () => ({ deployed, draft: rows.sent.get('u/me/b') as Res | undefined })) + ) + await settle() + open.value = { ...b, description: 'typed first' } + + const flushing = deferred() + rows.hold(flushing.promise) + const temporary = newItemPath() + const { handle: panel } = store.acquire( + { workspace: 'w', kind: 'resource', path: temporary }, + { workspace: 'w', path: temporary, template: b, standsFor: 'u/me/b' }, + adapter({}, async (ctx) => void (deployed = { ...ctx.value })) + ) + await settle() + panel.value = { ...b, args: { a: 2 } } + const wrote = panel.save() + await settle() + // In neither the row going out nor the read coming back, and newer than both. + const late = { ...b, description: 'typed while that row was going' } + open.value = late + flushing.resolve() + + expect(await wrote).toMatchObject({ ok: true, path: 'u/me/b' }) + expect(open.value).toEqual(late) + expect(open.deployed).toEqual({ ...b, args: { a: 2 } }) + }) + it('writes an item it moves onto after the saves queued there, and supersedes later ones', async () => { const rows = fakeRows() const store = createItemStore(rows.port)