From a0077ce5e98ce10d5f2906ca2ed5584b64cbf193 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Tue, 15 Sep 2026 12:08:22 +0200 Subject: [PATCH] fix: measure a discard against what moved under it, not against the value Co-Authored-By: Claude Opus 5 (1M context) --- frontend/src/lib/itemStore.svelte.ts | 26 ++++++++++---------------- frontend/src/lib/itemStore.test.ts | 21 +++++++++++++++++++++ 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/frontend/src/lib/itemStore.svelte.ts b/frontend/src/lib/itemStore.svelte.ts index 09d3f40775..ff21861c74 100644 --- a/frontend/src/lib/itemStore.svelte.ts +++ b/frontend/src/lib/itemStore.svelte.ts @@ -145,9 +145,10 @@ let nextPatch = 1 const superseded = { ok: false, error: 'Another save of this item replaced this one' } as const -/** An item as it stood when a command was asked for: whether it was on screen at all, the value - * it showed, the count of edits and outside writes it has taken, and the rows sent for its key. */ -type AsOf = { loaded: boolean; value: string | undefined; edits: number; rows: number } +/** An item as it stood when a command was asked for: the count of edits and outside writes it + * had taken, and the rows sent for its key. Both are counts of things the command did not do, + * so a command can tell what moved under it without mistaking its own effects for movement. */ +type AsOf = { edits: number; rows: number } class Entry { key: ItemKey = $state()! @@ -332,12 +333,7 @@ class Entry { /** The registers that move on their own: what the user is editing, and the rows already sent. * A command compares against these to tell what has happened since it was asked for. */ private asOf(): AsOf { - return { - loaded: this.loaded, - value: serialize(this.value), - edits: this.edits, - rows: this.rowWrites - } + return { edits: this.edits, rows: this.rowWrites } } /** Count a command as started now, ahead of its turn in the queue; returns its release. */ @@ -518,15 +514,13 @@ class Entry { } discard(): Promise { - // A discard throws away the value as of the click, as a save sends the value as of the - // click: one typed while it waited its turn is newer, and reverting it would drop it with - // nothing left holding it. The value and not a count of writes, so that an outside write - // of what is already on screen, changing nothing the user sees, leaves the discard good. return this.run(async (at) => { if (!this.loaded || this.retired) return { removed: false } - // Nothing was on screen to compare against: the discard was asked of the item, not of - // a value, so whatever the load in front of it turned up is what it throws away. - if (at.loaded && serialize(this.value) !== at.value) return { removed: false } + // An edit typed or an outside write landed since the discard was asked for is newer + // than it, and reverting that would drop it with nothing left holding it. A value a + // read in front of it installed is not something that moved, it is the read + // answering, and is what the discard was asked to throw away. + if (this.edits !== at.edits) return { removed: false } if (this.origin === 'draft') { const kept = snapshot(this.value) const keptRow = this.row diff --git a/frontend/src/lib/itemStore.test.ts b/frontend/src/lib/itemStore.test.ts index 1faa1914b4..4437968f17 100644 --- a/frontend/src/lib/itemStore.test.ts +++ b/frontend/src/lib/itemStore.test.ts @@ -658,6 +658,27 @@ describe('item store: origins', () => { expect(rows.writes.at(-1)).toEqual({ path: 'u/me/r', value: null }) }) + it('keeps an outside write that landed after a discard was asked for, before the first read', async () => { + const rows = fakeRows() + const read = deferred>() + const store = createItemStore(rows.port) + const { handle: item } = store.acquire( + { workspace: 'w', kind: 'resource', path: 'u/me/r' }, + { workspace: 'w', path: 'u/me/r' }, + adapter(() => read.promise) + ) + const discarding = item.discard() + // The chat writes after asking for the discard, so its value is the newer of the two. + const fromChat = { ...deployedRes, description: 'from the chat' } + item.applyExternal(fromChat) + read.resolve({ deployed: deployedRes }) + + expect(await discarding).toEqual({ removed: false }) + expect(item.value).toEqual(fromChat) + expect(item.dirty).toBe(true) + expect(rows.writes.at(-1)).toEqual({ path: 'u/me/r', value: fromChat }) + }) + it('keeps a draft-only item an outside write put back while its delete was going', async () => { const rows = fakeRows() const draft = { ...deployedRes, description: 'only a draft' }