From e58ba737e48a7b6ff5f5a3e3a2a4a643e4135d8a Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Tue, 15 Sep 2026 22:54:54 +0200 Subject: [PATCH] fix: count rows, not seeds, and seed a mirror the live item took from Co-Authored-By: Claude Opus 5 (1M context) --- frontend/src/lib/itemStore.svelte.ts | 30 +++++++++------------- frontend/src/lib/itemStore.test.ts | 38 +++++++++++++++++++++++++--- frontend/src/lib/userDraft.svelte.ts | 25 +++++++++++++----- 3 files changed, 64 insertions(+), 29 deletions(-) diff --git a/frontend/src/lib/itemStore.svelte.ts b/frontend/src/lib/itemStore.svelte.ts index 30a3478fb3..31b9cc0198 100644 --- a/frontend/src/lib/itemStore.svelte.ts +++ b/frontend/src/lib/itemStore.svelte.ts @@ -249,17 +249,14 @@ class Entry { /** Counts outside writes, the ones that change nothing on screen included: those still put the * value on the server as a row, which a read already in flight knows nothing about. */ private externals = 0 - /** Counts every value this entry took from outside a command — typed or written in — once - * each, and each row a caller says it wrote for one of them. A row left over after that is a - * row nobody here has the value of. */ + /** Counts the rows this entry can account for: the ones it wrote itself, and the ones a caller + * says it wrote for a value handed in here. A row left over after that is a row nobody here + * has the value of. */ private values = 0 /** Whether the discard that last ran had anything to say about the key's row — it had one, or * it is deliberately keeping the one a newer edit put there. Decided inside the command, so * after whatever load was queued in front of it, not before. */ private ownsRow = false - /** Whether the last value handed in made this entry write a row of its own. Loading, it - * cannot: the caller's own row is then the only one, and is already counted. */ - private seedWroteRow = false private queue: Promise = Promise.resolve() private ports: ItemRowPort private store: StoreInternals @@ -283,7 +280,6 @@ class Entry { if (s === this.seen) return this.seen = s this.edits++ - this.values++ 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)) { @@ -314,6 +310,7 @@ class Entry { } if (desired === this.row) return this.row = desired + this.values++ this.ports.write(key, desired === null ? null : snapshot(this.value)) } @@ -454,8 +451,7 @@ class Entry { const standOff = unasked && this.ports.conflicted(key) // 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 = - standOff || this.edits !== at.edits || this.externals !== at.externals + const keepValue = standOff || this.edits !== at.edits || this.externals !== at.externals // More rows appeared while the read was out than values arrived to account for them, // so one was written by someone this entry cannot see — and it is later than anything // held here. Writing from this would post an older value over that row, on the @@ -577,26 +573,24 @@ class Entry { }) } - /** An outside write (the AI chat, another editor): a real divergence, never settling. */ - /** A caller that handed a value in through `applyExternal` and then wrote its own row for it, - * rather than leaving the row to this entry: the two are one value, and counting the second - * row as unaccounted would read as someone else having written it. */ + /** A caller that handed a value in through `applyExternal` and wrote its own row for it, + * rather than leaving that to this entry: the row is accounted for, and counting it as + * unaccounted would read as someone else having written it. */ noteRow(): void { - if (this.seedWroteRow) this.values++ + this.values++ } + /** An outside write (the AI chat, another editor): a real divergence, never settling. */ applyExternal(value: V): number { this.externals++ - this.values++ - this.seedWroteRow = false if (this.loaded && serialize(value) === serialize(this.value)) return this.revision this.edits++ this.pristine = false this.removed = false this.replaceValue(value) - const rowsBefore = this.ports.rowMark(this.key) + // Whatever row this produces is counted by `reconcile`; a value that produces none + // accounts for nothing, and must not pay for a row some other writer puts there. this.reconcile() - this.seedWroteRow = this.ports.rowMark(this.key) !== rowsBefore return this.revision } diff --git a/frontend/src/lib/itemStore.test.ts b/frontend/src/lib/itemStore.test.ts index ec6e60fe5e..c6246ff6f8 100644 --- a/frontend/src/lib/itemStore.test.ts +++ b/frontend/src/lib/itemStore.test.ts @@ -102,8 +102,7 @@ function fakeRows() { : parked.has(key.path) ? parked.get(key.path) : undefined, - unbased: (key) => - (queued.has(key.path) || parked.has(key.path)) && !baselines.has(key.path), + unbased: (key) => (queued.has(key.path) || parked.has(key.path)) && !baselines.has(key.path), markConflict: (key) => void conflicts.add(key.path), seedSync: (key, at, since) => { // The syncer refuses a baseline from a read a row handed over since has passed. @@ -734,7 +733,8 @@ describe('item store: origins', () => { // 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) - // `persistGlobalDraft` hands the value here and writes its own row through the syncer. + // `persistGlobalDraft` hands the value here, says it is writing the row for it, and does. + store.bridge.noteRow('w', 'resource', 'u/me/r') rows.handExternally('u/me/r') read.resolve({ deployed: deployedRes }) @@ -882,6 +882,7 @@ describe('item store: origins', () => { const fromChat = { ...deployedRes, description: 'from the chat' } item.applyExternal(fromChat) // `persistGlobalDraft` hands the value here and writes its own row through the syncer. + store.bridge.noteRow('w', 'resource', 'u/me/r') rows.handExternally('u/me/r') read.resolve({ deployed: deployedRes }) await settle() @@ -951,7 +952,7 @@ describe('item store: origins', () => { expect(item.canSave).toBe(false) }) - it('does not credit an edit made during a re-read\'s own flush', async () => { + it("does not credit an edit made during a re-read's own flush", async () => { const rows = fakeRows() const flushing = deferred() const read = deferred>() @@ -1009,6 +1010,35 @@ describe('item store: origins', () => { expect(item.canSave).toBe(false) }) + it('does not credit a row to a seed that wrote none', async () => { + const rows = fakeRows() + const read = deferred>() + const store = createItemStore(rows.port) + const key: ItemKey = { workspace: 'w', kind: 'resource', path: 'u/me/r' } + const { handle: item } = store.acquire( + key, + { workspace: 'w', path: 'u/me/r' }, + adapter(() => read.promise) + ) + await settle() + + // The agent editor seeds its restored draft here while the GET is out. Loading, this + // writes no row, and the seeding editor writes none for it either. + const seeded = { ...deployedRes, description: 'restored by the agent editor' } + item.applyExternal(seeded) + // It then autosaves something newer through the `UserDraft` handle it kept: a row whose + // value never came through here. + rows.handExternally('u/me/r') + const writesBefore = rows.writes.length + read.resolve({ deployed: deployedRes }) + await settle() + + // One row, nothing accounting for it. Posting the seed now would put it on the baseline + // that row advanced, silently replacing it. + expect(item.canSave).toBe(false) + expect(rows.writes.length).toBe(writesBefore) + }) + it('stays blocked when an unseen row follows the edit it kept', async () => { const rows = fakeRows() const read = deferred>() diff --git a/frontend/src/lib/userDraft.svelte.ts b/frontend/src/lib/userDraft.svelte.ts index ecec2933d0..25575d251f 100644 --- a/frontend/src/lib/userDraft.svelte.ts +++ b/frontend/src/lib/userDraft.svelte.ts @@ -333,10 +333,25 @@ export type UserDraftHandle = { set draft(value: V | undefined) } +/** + * Put a value into a caller's own mirror without POSTing it, for a key whose live item took the + * value and persists it. A mirror left untouched keeps the first-write guard it was acquired with + * armed, so the caller's next real edit is swallowed as initialization instead of being saved. + */ +function seedLocalMirror(mk: string, value: unknown): void { + const entry = entries.get(mk) + if (!entry) return + entry.seedNextWrite = true + entry.state.val = snapshotDraftValue(value) +} + export const UserDraft = { save(itemKind: UserDraftItemKind, path: string, value: V, opts?: UserDraftOptions): void { const ws = resolveWorkspace(opts) - if (liveItems?.seed(ws, itemKind, path, value)) return + if (liveItems?.seed(ws, itemKind, path, value)) { + seedLocalMirror(mapKey(ws, itemKind, path), value) + return + } const mk = mapKey(ws, itemKind, path) const entry = entries.get(mk) if (entry) { @@ -525,12 +540,8 @@ export const UserDraft = { */ seed(itemKind: UserDraftItemKind, path: string, value: V, opts?: UserDraftOptions): void { const ws = resolveWorkspace(opts) - if (liveItems?.seed(ws, itemKind, path, value)) return - const mk = mapKey(ws, itemKind, path) - const entry = entries.get(mk) - if (!entry) return - entry.seedNextWrite = true - entry.state.val = snapshotDraftValue(value) + liveItems?.seed(ws, itemKind, path, value) + seedLocalMirror(mapKey(ws, itemKind, path), value) }, /**