Skip to content

pending-changes.json grows without bound — resolved proposals keep their full note text, parsed and rewritten whole on the main thread #539

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Direct-Launch wants to merge 2 commits into
base: main
Choose a base branch
from
Open

pending-changes.json grows without bound — resolved proposals keep their full note text, parsed and rewritten whole on the main thread #539

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3174461
fix(pending-changes): prune the stored payload of settled entries
Oct 2, 2026
a2b6022
fix(pending-changes): persist the load-time prune and keep settled ro…
Oct 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions src/stores/pendingChangesStore.svelte.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,10 @@ export class PendingChangesStore {
}
this.#backfillShortIds();
await this.#pruneOrphanedThreads();
// 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);
});
Expand Down Expand Up @@ -263,10 +267,63 @@ export class PendingChangesStore {
if (!(await this.#plugin.app.vault.adapter.exists(dir))) {
await this.#plugin.app.vault.adapter.mkdir(dir);
}
this.#pruneSettledPayloads();
const snapshot = $state.snapshot(this.#entries);
await this.#plugin.app.vault.adapter.write(path, JSON.stringify(snapshot, null, 2));
}

/**
* 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 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.
*/
#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;
}
Comment on lines +301 to +320

@greptile-apps greptile-apps Bot Oct 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Historical entries remain unbounded

Once an outcome is reported, this loop blanks its note text but keeps the entry forever. Every future load still parses and validates every historical entry, and every save serializes them all. Over time, that brings back growing main-thread work and file size, even though the old note text is gone. Consider a bounded status record that still lets chat cards show completed reviews.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/stores/pendingChangesStore.svelte.ts
Line: 301-320

Comment:
**Historical entries remain unbounded**

Once an outcome is reported, this loop blanks its note text but keeps the entry forever. Every future load still parses and validates every historical entry, and every save serializes them all. Over time, that brings back growing main-thread work and file size, even though the old note text is gone. Consider a bounded status record that still lets chat cards show completed reviews.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code

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. */
get revision(): number {
return this.#revision;
Expand Down
92 changes: 92 additions & 0 deletions test/stores/pendingChangesStore.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1326,4 +1326,96 @@ 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,
shortId: "s",
...(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 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'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")]);

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 () => {
// `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),
}),
]);

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));
});
});
});
Loading