From 47b6934266ed7d93c803b7fffbf1cc55fd25f4ea Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Tue, 15 Sep 2026 09:33:02 +0200 Subject: [PATCH] fix: leave a command's own value replacement out of the edit count Co-Authored-By: Claude Opus 5 (1M context) --- frontend/src/lib/itemStore.svelte.ts | 26 +++++++++---------- frontend/src/lib/itemStore.test.ts | 38 ++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 13 deletions(-) diff --git a/frontend/src/lib/itemStore.svelte.ts b/frontend/src/lib/itemStore.svelte.ts index b3b2516d42..91a127356c 100644 --- a/frontend/src/lib/itemStore.svelte.ts +++ b/frontend/src/lib/itemStore.svelte.ts @@ -201,8 +201,10 @@ class Entry { /** The row last handed to the syncer (`null`: none, `undefined`: not known — the next * reconcile writes whatever the rule says). What `reconcile` diffs against. */ private row: string | null | undefined = null - /** Counts value changes, so a load can tell whether one landed while it was in flight. */ - private changes = 0 + /** Counts what the user typed and what an outside writer put here, so a read can tell whether + * one landed while it was in flight. A command's own replacement of the value is not one: + * a discard that ran meanwhile is the answer to a question the read is not asking. */ + private edits = 0 private queue: Promise = Promise.resolve() private ports: ItemRowPort private store: StoreInternals @@ -225,7 +227,7 @@ class Entry { const s = serialize(this.value) if (s === this.seen) return this.seen = s - this.changes++ + this.edits++ if (this.pristine && this.origin === 'deployed' && this.value !== undefined) { const value = snapshot(this.value) if (!this.absorbs || this.deployed === undefined || this.absorbs(value, this.deployed)) { @@ -257,7 +259,6 @@ class Entry { private replaceValue(value: V | undefined): void { this.value = snapshot(value) this.seen = serialize(this.value) - this.changes++ this.revision = nextRevision++ } @@ -304,11 +305,11 @@ class Entry { this.replaceValue(template) } - /** `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 { + /** `editedBefore`: the edit count as of when this read was decided on, for a caller that did + * something async first. */ + load(adapter: ItemAdapter, editedBefore?: number): Promise { this.loads++ - const changesAtStart = changedBefore ?? this.changes + const editsAtStart = editedBefore ?? this.edits return this.run(async () => { const key = this.key let res: ItemLoad @@ -324,7 +325,7 @@ class Entry { } // Whatever landed since the read was asked for — an external write, an edit — is // newer than what it read, so the read only moves the deployed side under it. - const keepValue = this.changes !== changesAtStart + const keepValue = this.edits !== editsAtStart this.meta = res.meta this.error = undefined if (isTemporaryPath(key.path)) { @@ -355,16 +356,15 @@ class Entry { 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 + const editsAtStart = this.edits await this.ports.flush(this.key) - return this.load(this.adapter, changesAtStart) + return this.load(this.adapter, editsAtStart) } /** An outside write (the AI chat, another editor): a real divergence, never settling. */ applyExternal(value: V): number { if (this.loaded && serialize(value) === serialize(this.value)) return this.revision + this.edits++ this.pristine = false this.removed = false this.replaceValue(value) diff --git a/frontend/src/lib/itemStore.test.ts b/frontend/src/lib/itemStore.test.ts index 72975d721c..dbb3cd80b5 100644 --- a/frontend/src/lib/itemStore.test.ts +++ b/frontend/src/lib/itemStore.test.ts @@ -577,6 +577,44 @@ describe('item store: one entry per key', () => { expect(open.dirty).toBe(true) }) + it('leaves an item discarded while a write was landing on the deployed value', 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 })) + ) + const gate = deferred() + 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) => { + await gate.promise + deployed = { ...ctx.value } + }) + ) + await settle() + open.value = { ...b, description: 'typed in the open editor' } + panel.value = { ...b, args: { a: 2 } } + const wrote = panel.save() + await settle() + // Discard while the write is in flight: it queues behind the writer's claim on the key. + const discarded = open.discard() + gate.resolve() + + expect(await wrote).toMatchObject({ ok: true, path: 'u/me/b' }) + expect(await discarded).toEqual({ removed: false }) + // The discard holds: what it dropped does not come back as a draft over the new value. + expect(open.deployed).toEqual({ ...b, args: { a: 2 } }) + expect(open.value).toEqual({ ...b, args: { a: 2 } }) + expect(open.dirty).toBe(false) + expect(rows.writes.at(-1)).toEqual({ path: 'u/me/b', value: null }) + }) + it('keeps an edit typed while the row it re-reads behind is still going', async () => { const rows = fakeRows() const store = createItemStore(rows.port)