From 3174461d131dd45d1c3bb8f137c03abf24065652 Mon Sep 17 00:00:00 2001 From: Direct Launch Date: Fri, 2 Oct 2026 17:28:59 +0100 Subject: [PATCH 1/2] fix(pending-changes): prune the stored payload of settled entries --- src/stores/pendingChangesStore.svelte.ts | 28 ++++++++++ test/stores/pendingChangesStore.test.ts | 65 ++++++++++++++++++++++++ 2 files changed, 93 insertions(+) diff --git a/src/stores/pendingChangesStore.svelte.ts b/src/stores/pendingChangesStore.svelte.ts index f8fce27b..eb4b44a7 100644 --- a/src/stores/pendingChangesStore.svelte.ts +++ b/src/stores/pendingChangesStore.svelte.ts @@ -166,6 +166,7 @@ export class PendingChangesStore { } this.#backfillShortIds(); await this.#pruneOrphanedThreads(); + this.#pruneResolvedEntries(); this.#renameHandler = this.#plugin.app.vault.on("rename", (file, oldPath) => { this.#handleFileRename(oldPath, file.path); }); @@ -263,10 +264,37 @@ export class PendingChangesStore { if (!(await this.#plugin.app.vault.adapter.exists(dir))) { await this.#plugin.app.vault.adapter.mkdir(dir); } + this.#pruneResolvedEntries(); const snapshot = $state.snapshot(this.#entries); await this.#plugin.app.vault.adapter.write(path, JSON.stringify(snapshot, null, 2)); } + /** + * Drop the stored payload of entries that can never be needed again. + * + * A resolved entry keeps the full `originalContent` / `newContent` text of the + * note it touched, and nothing ever dropped it — so the store grew with every + * proposal ever made and was re-parsed and re-written whole on load and after + * each change. An entry is only prunable once the outcome is settled *and* the + * model has been told about it (an unreported outcome still has to reach the + * next turn, and `takeReviewOutcomesForThread` skips `reportedToModel` ones). + * + * `hasUnrevertedApplication` is the safety gate rather than `status`: a + * partially-accepted entry keeps `originalContent`/`newContent` live so + * `revertAppliedGroups` can restore the note. Both places that settle that + * snapshot clear it — the full accept and the revert — so this cannot race a + * revert in flight. + */ + #pruneResolvedEntries(): void { + const kept = this.#entries.filter( + (e) => e.status === "pending" || !e.reportedToModel || this.hasUnrevertedApplication(e), + ); + if (kept.length === this.#entries.length) return; + const removed = this.#entries.length - kept.length; + this.#entries = kept; + Logger.log(`[PendingChanges] Pruned ${removed} resolved ${removed === 1 ? "entry" : "entries"} from the store.`); + } + /** Reactive revision counter – read this inside `$derived` to track store mutations. */ get revision(): number { return this.#revision; diff --git a/test/stores/pendingChangesStore.test.ts b/test/stores/pendingChangesStore.test.ts index 2b4ee6f9..fa759dec 100644 --- a/test/stores/pendingChangesStore.test.ts +++ b/test/stores/pendingChangesStore.test.ts @@ -1326,4 +1326,69 @@ describe("PendingChangesStore", () => { expect(store.revision).toBe(before); }); }); + + /* -------------------------------------------------------------------------- + * Payload retention + * ------------------------------------------------------------------------*/ + + describe("payload pruning of settled entries", () => { + const mkEntry = ( + id: string, + status: "pending" | "accepted" | "rejected", + opts: { reported?: boolean; initial?: string } = {}, + ) => ({ + id, + change: { + type: "update" as const, + path: "note.md", + originalContent: "x".repeat(64), + newContent: "y".repeat(64), + ...(opts.initial !== undefined ? { initialOriginalContent: opts.initial } : {}), + }, + status, + toolCallId: "tc", + threadId: "thread-1", + createdAt: 1, + ...(opts.reported !== undefined ? { reportedToModel: opts.reported } : {}), + }); + + async function loadWith(entries: unknown[]) { + vi.mocked(plugin.app.vault.adapter.exists).mockResolvedValue(true); + vi.mocked(plugin.app.vault.adapter.read).mockResolvedValue(JSON.stringify(entries)); + await store.load(); + } + + it("drops settled entries once the model has seen the outcome", async () => { + await loadWith([ + mkEntry("01a00000-0000-7000-8000-000000000001", "pending"), + mkEntry("01a00000-0000-7000-8000-000000000002", "accepted", { reported: true }), + mkEntry("01a00000-0000-7000-8000-000000000003", "rejected", { reported: true }), + ]); + + expect(store.getEntriesForThread("thread-1").map((e) => e.id)).toEqual([ + "01a00000-0000-7000-8000-000000000001", + ]); + }); + + it("keeps a settled entry whose outcome has not reached the model yet", async () => { + // The next turn still has to be told this proposal was resolved, and + // `takeReviewOutcomesForThread` only skips entries already reported. + await loadWith([mkEntry("01a00000-0000-7000-8000-000000000004", "accepted")]); + + expect(store.getEntriesForThread("thread-1")).toHaveLength(1); + }); + + it("keeps a settled entry holding an unreverted partial application", async () => { + // `initialOriginalContent` means applied text the user has not signed + // off on — the payload is the only undo record, so it must survive. + await loadWith([ + mkEntry("01a00000-0000-7000-8000-000000000005", "accepted", { + reported: true, + initial: "x".repeat(64), + }), + ]); + + expect(store.getEntriesForThread("thread-1")).toHaveLength(1); + }); + }); }); From a2b6022a8d638fbe4411f66c5f887d1ae8bbd3c1 Mon Sep 17 00:00:00 2001 From: Direct Launch Date: Fri, 2 Oct 2026 17:52:13 +0100 Subject: [PATCH 2/2] fix(pending-changes): persist the load-time prune and keep settled rows for the review chips --- src/stores/pendingChangesStore.svelte.ts | 57 ++++++++++++++++++------ test/stores/pendingChangesStore.test.ts | 35 +++++++++++++-- 2 files changed, 74 insertions(+), 18 deletions(-) diff --git a/src/stores/pendingChangesStore.svelte.ts b/src/stores/pendingChangesStore.svelte.ts index eb4b44a7..2e1ababb 100644 --- a/src/stores/pendingChangesStore.svelte.ts +++ b/src/stores/pendingChangesStore.svelte.ts @@ -166,7 +166,10 @@ export class PendingChangesStore { } this.#backfillShortIds(); await this.#pruneOrphanedThreads(); - this.#pruneResolvedEntries(); + // Persist the prune when it changed anything: without a save, every idle + // restart would re-read, re-parse and re-validate the full historical file, + // which is the cost this exists to remove. + if (this.#pruneSettledPayloads()) this.flushSave(); this.#renameHandler = this.#plugin.app.vault.on("rename", (file, oldPath) => { this.#handleFileRename(oldPath, file.path); }); @@ -264,35 +267,61 @@ export class PendingChangesStore { if (!(await this.#plugin.app.vault.adapter.exists(dir))) { await this.#plugin.app.vault.adapter.mkdir(dir); } - this.#pruneResolvedEntries(); + this.#pruneSettledPayloads(); const snapshot = $state.snapshot(this.#entries); await this.#plugin.app.vault.adapter.write(path, JSON.stringify(snapshot, null, 2)); } /** - * Drop the stored payload of entries that can never be needed again. + * Drop the note text of entries that can never be needed again. * * A resolved entry keeps the full `originalContent` / `newContent` text of the * note it touched, and nothing ever dropped it — so the store grew with every * proposal ever made and was re-parsed and re-written whole on load and after - * each change. An entry is only prunable once the outcome is settled *and* the - * model has been told about it (an unreported outcome still has to reach the - * next turn, and `takeReviewOutcomesForThread` skips `reportedToModel` ones). + * each change. An entry is only settled once the outcome has reached the model + * (an unreported outcome still has to reach the next turn, and + * `takeReviewOutcomesForThread` skips `reportedToModel` ones) *and* it holds no + * unreverted partial application. * * `hasUnrevertedApplication` is the safety gate rather than `status`: a * partially-accepted entry keeps `originalContent`/`newContent` live so * `revertAppliedGroups` can restore the note. Both places that settle that * snapshot clear it — the full accept and the revert — so this cannot race a * revert in flight. + * + * The row itself is KEPT and only its text blanked: the `manage_notes` card + * reads `getEntriesByToolCallId` to show "accepted"/"rejected" chips, so + * dropping the entry would silently turn a completed review back into the + * static "will be reviewed" summary. Nothing renders a resolved entry's diff — + * `PendingChangesBar` lists only actionable entries — so the text costs + * nothing to drop and the metadata stays honest. */ - #pruneResolvedEntries(): void { - const kept = this.#entries.filter( - (e) => e.status === "pending" || !e.reportedToModel || this.hasUnrevertedApplication(e), - ); - if (kept.length === this.#entries.length) return; - const removed = this.#entries.length - kept.length; - this.#entries = kept; - Logger.log(`[PendingChanges] Pruned ${removed} resolved ${removed === 1 ? "entry" : "entries"} from the store.`); + #pruneSettledPayloads(): boolean { + let changed = false; + for (const entry of this.#entries) { + if (entry.status === "pending") continue; + if (!entry.reportedToModel) continue; + if (this.hasUnrevertedApplication(entry)) continue; + const change = entry.change; + if (change.type === "update") { + if (change.originalContent === "" && change.newContent === "") continue; + change.originalContent = ""; + change.newContent = ""; + } else if (change.type === "delete") { + if (change.originalContent === "") continue; + change.originalContent = ""; + } else if (change.type === "create") { + if (change.content === "") continue; + change.content = ""; + } else { + continue; + } + changed = true; + } + if (changed) { + Logger.log("[PendingChanges] Dropped the stored text of settled entries."); + } + return changed; } /** Reactive revision counter – read this inside `$derived` to track store mutations. */ diff --git a/test/stores/pendingChangesStore.test.ts b/test/stores/pendingChangesStore.test.ts index fa759dec..125db161 100644 --- a/test/stores/pendingChangesStore.test.ts +++ b/test/stores/pendingChangesStore.test.ts @@ -1349,6 +1349,7 @@ describe("PendingChangesStore", () => { toolCallId: "tc", threadId: "thread-1", createdAt: 1, + shortId: "s", ...(opts.reported !== undefined ? { reportedToModel: opts.reported } : {}), }); @@ -1358,24 +1359,37 @@ describe("PendingChangesStore", () => { await store.load(); } - it("drops settled entries once the model has seen the outcome", async () => { + it("drops the note text of a settled entry, keeping the row and its status", async () => { await loadWith([ mkEntry("01a00000-0000-7000-8000-000000000001", "pending"), mkEntry("01a00000-0000-7000-8000-000000000002", "accepted", { reported: true }), mkEntry("01a00000-0000-7000-8000-000000000003", "rejected", { reported: true }), ]); + // Rows survive — the chat card reads them for its review chips. expect(store.getEntriesForThread("thread-1").map((e) => e.id)).toEqual([ "01a00000-0000-7000-8000-000000000001", + "01a00000-0000-7000-8000-000000000002", + "01a00000-0000-7000-8000-000000000003", ]); + + const pending = store.getEntry("01a00000-0000-7000-8000-000000000001"); + expect(pending?.change.type === "update" && pending.change.originalContent).toBe("x".repeat(64)); + + const accepted = store.getEntry("01a00000-0000-7000-8000-000000000002"); + expect(accepted?.status).toBe("accepted"); + expect(accepted?.change.type === "update" && accepted.change.originalContent).toBe(""); + expect(accepted?.change.type === "update" && accepted.change.newContent).toBe(""); }); - it("keeps a settled entry whose outcome has not reached the model yet", async () => { + it("keeps a settled entry's text until the outcome has reached the model", async () => { // The next turn still has to be told this proposal was resolved, and // `takeReviewOutcomesForThread` only skips entries already reported. await loadWith([mkEntry("01a00000-0000-7000-8000-000000000004", "accepted")]); - expect(store.getEntriesForThread("thread-1")).toHaveLength(1); + const entry = store.getEntry("01a00000-0000-7000-8000-000000000004"); + expect(entry?.change.type === "update" && entry.change.originalContent).toBe("x".repeat(64)); + expect(plugin.app.vault.adapter.write).not.toHaveBeenCalled(); }); it("keeps a settled entry holding an unreverted partial application", async () => { @@ -1388,7 +1402,20 @@ describe("PendingChangesStore", () => { }), ]); - expect(store.getEntriesForThread("thread-1")).toHaveLength(1); + const entry = store.getEntry("01a00000-0000-7000-8000-000000000005"); + expect(entry?.change.type === "update" && entry.change.originalContent).toBe("x".repeat(64)); + expect(plugin.app.vault.adapter.write).not.toHaveBeenCalled(); + }); + + it("persists the load-time prune so idle restarts stop re-reading the old file", async () => { + await loadWith([mkEntry("01a00000-0000-7000-8000-000000000006", "accepted", { reported: true })]); + + // Nothing else in `load()` writes for this fixture (short ids already + // present, no `.chat` thread to sweep), so a write here is the prune + // persisting itself — without it every restart pays the full cost again. + expect(plugin.app.vault.adapter.write).toHaveBeenCalled(); + const last = vi.mocked(plugin.app.vault.adapter.write).mock.calls.at(-1)!; + expect(last[1]).not.toContain("x".repeat(64)); }); }); });