From 24f807fed05e391c5c69591d74112d1e875f7283 Mon Sep 17 00:00:00 2001 From: Ralph Chang Date: Mon, 27 Apr 2026 14:27:43 +0800 Subject: [PATCH] fix: account for absorbed pending memories - Add workspaceMemoryIdentityKey() to unify dedup/supersession identity semantics - Add accountPendingPromotions() to distinguish promoted/absorbed/rejected - Wire promotion accounting into promotePendingMemories() - Add clearableKeys.size > 0 guard to prevent journal wipe - Add regression tests for absorbed duplicate, cap-rejected, all-rejected edge cases Wave 1 of memory quality optimization plan. --- src/plugin.ts | 18 +- src/promotion-accounting.ts | 53 ++++++ src/workspace-memory.ts | 23 ++- tests/plugin.test.ts | 264 +++++++++++++++++++++++++++++ tests/promotion-accounting.test.ts | 109 ++++++++++++ 5 files changed, 455 insertions(+), 12 deletions(-) create mode 100644 src/promotion-accounting.ts create mode 100644 tests/promotion-accounting.test.ts diff --git a/src/plugin.ts b/src/plugin.ts index 184d2fb..913fcde 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -59,6 +59,7 @@ import { latestCompactionSummary, pendingTodos, } from "./opencode.ts"; +import { accountPendingPromotions } from "./promotion-accounting.ts"; /** * Build the complete compaction prompt. @@ -231,7 +232,10 @@ export const MemoryV2Plugin: Plugin = async (input) => { ]; if (pending.length === 0) return; + let beforeEntries: Awaited>["entries"] = []; + const updatedWorkspaceMemory = await updateWorkspaceMemory(directory, workspaceMemory => { + beforeEntries = [...workspaceMemory.entries]; const existingKeys = new Set(workspaceMemory.entries.map(memory => memoryKey(memory))); for (const memory of pending) { @@ -245,19 +249,23 @@ export const MemoryV2Plugin: Plugin = async (input) => { return workspaceMemory; }); - // Only clear pending memories that survived workspace normalization/limits. - // updateWorkspaceMemory() may dedupe, supersede, redact, or cap entries. - const retainedKeys = new Set(updatedWorkspaceMemory.entries.map(memory => memoryKey(memory))); + const accounting = accountPendingPromotions({ + pending, + before: beforeEntries, + after: updatedWorkspaceMemory.entries, + }); if (sessionID) { await updateSessionState(directory, sessionID, state => { - state.pendingMemories = state.pendingMemories.filter(memory => !retainedKeys.has(memoryKey(memory))); + state.pendingMemories = state.pendingMemories.filter(memory => !accounting.clearableKeys.has(memoryKey(memory))); return state; }); clearFrozenWorkspaceMemoryCache(sessionID); } - await clearPendingMemories(directory, retainedKeys); + if (accounting.clearableKeys.size > 0) { + await clearPendingMemories(directory, accounting.clearableKeys); + } } function bashExitCode(hookOutput: unknown): number | undefined { diff --git a/src/promotion-accounting.ts b/src/promotion-accounting.ts new file mode 100644 index 0000000..d8d9187 --- /dev/null +++ b/src/promotion-accounting.ts @@ -0,0 +1,53 @@ +import type { LongTermMemoryEntry } from "./types.ts"; +import { memoryKey } from "./pending-journal.ts"; +import { workspaceMemoryIdentityKey } from "./workspace-memory.ts"; + +export type PendingPromotionAccounting = { + promotedKeys: Set; + absorbedKeys: Set; + rejectedKeys: Set; + clearableKeys: Set; +}; + +export function accountPendingPromotions(input: { + pending: LongTermMemoryEntry[]; + before: LongTermMemoryEntry[]; + after: LongTermMemoryEntry[]; +}): PendingPromotionAccounting { + const beforeExactKeys = new Set(input.before.map(entry => memoryKey(entry))); + const afterExactKeys = new Set(input.after.map(entry => memoryKey(entry))); + const afterIdentityKeys = new Set(input.after.map(entry => workspaceMemoryIdentityKey(entry))); + + const promotedKeys = new Set(); + const absorbedKeys = new Set(); + const rejectedKeys = new Set(); + + for (const memory of input.pending) { + const key = memoryKey(memory); + const identityKey = workspaceMemoryIdentityKey(memory); + + if (beforeExactKeys.has(key)) { + absorbedKeys.add(key); + continue; + } + + if (afterExactKeys.has(key)) { + promotedKeys.add(key); + continue; + } + + if (afterIdentityKeys.has(identityKey)) { + absorbedKeys.add(key); + continue; + } + + rejectedKeys.add(key); + } + + return { + promotedKeys, + absorbedKeys, + rejectedKeys, + clearableKeys: new Set([...promotedKeys, ...absorbedKeys]), + }; +} diff --git a/src/workspace-memory.ts b/src/workspace-memory.ts index 24b90e5..86c13b5 100644 --- a/src/workspace-memory.ts +++ b/src/workspace-memory.ts @@ -291,6 +291,18 @@ function feedbackTopicKey(text: string): string | null { return null; } +export function workspaceMemoryIdentityKey(entry: Pick): string { + if (entry.type === "project" || entry.type === "reference") { + return `${entry.type}:${extractEntityKey(entry.text) ?? canonicalMemoryText(entry.text)}`; + } + + if (entry.type === "feedback") { + return `${entry.type}:${feedbackTopicKey(entry.text) ?? canonicalMemoryText(entry.text)}`; + } + + return `decision:${decisionTopicKey(entry.text) ?? canonicalMemoryText(entry.text)}`; +} + /** Check if entry should be pruned by age (for compaction/manual entries only) */ function isPrunableByAge(entry: LongTermMemoryEntry, now: number): boolean { // Never prune feedback or explicit entries @@ -350,17 +362,15 @@ export function enforceLongTermLimits(entries: LongTermMemoryEntry[]): LongTermM // Build entity key dedup for project/reference/feedback const entityDeduped = new Map(); for (const entry of projectRefEntries) { - const entityKey = entry.type === "project" || entry.type === "reference" - ? extractEntityKey(entry.text) - : feedbackTopicKey(entry.text); - const key = entityKey ? `${entry.type}:${entityKey}` : `${entry.type}:${canonicalMemoryText(entry.text)}`; + const key = workspaceMemoryIdentityKey(entry); + const hasTopicIdentity = key !== `${entry.type}:${canonicalMemoryText(entry.text)}`; const existing = entityDeduped.get(key); if (!existing) { entityDeduped.set(key, entry); } else { // Feedback topic conflicts use supersession mode (newer beats longer) - const mode = entry.type === "feedback" && entityKey ? "supersession" as const : "entity" as const; + const mode = entry.type === "feedback" && hasTopicIdentity ? "supersession" as const : "entity" as const; if (chooseBetterMemory(entry, existing, mode) === entry) { entityDeduped.set(key, entry); } @@ -371,8 +381,7 @@ export function enforceLongTermLimits(entries: LongTermMemoryEntry[]): LongTermM const decisionEntries = phase1.filter(e => e.type === "decision"); const decisionDeduped = new Map(); for (const entry of decisionEntries) { - const topic = decisionTopicKey(entry.text); - const key = topic ? `decision:${topic}` : `decision:${canonicalMemoryText(entry.text)}`; + const key = workspaceMemoryIdentityKey(entry); const existing = decisionDeduped.get(key); if (!existing) { diff --git a/tests/plugin.test.ts b/tests/plugin.test.ts index afa25f7..b7d7d1b 100644 --- a/tests/plugin.test.ts +++ b/tests/plugin.test.ts @@ -719,6 +719,270 @@ test("compaction intentionally refreshes frozen system[1] with promoted memories } }); +test("session.compacted clears pending memory absorbed by existing workspace duplicate", async () => { + const tmpDir = await mkdtemp(join(tmpdir(), "memory-plugin-test-")); + + try { + const now = new Date().toISOString(); + await updateWorkspaceMemory(tmpDir, store => { + store.entries.push({ + id: "mem_existing_duplicate", + type: "decision", + text: "Prefer stable cache boundaries.", + source: "explicit", + confidence: 1, + status: "active", + createdAt: now, + updatedAt: now, + }); + return store; + }); + + await saveSessionState(tmpDir, { + version: 1, + sessionID: "absorbed-duplicate-session", + turn: 0, + updatedAt: now, + activeFiles: [], + openErrors: [], + recentDecisions: [], + pendingMemories: [{ + id: "mem_pending_duplicate", + type: "decision", + text: "prefer stable cache boundaries.", + source: "explicit", + confidence: 1, + status: "active", + createdAt: now, + updatedAt: now, + }], + }); + + const plugin = await MemoryV2Plugin({ directory: tmpDir, client: mockRootClient() }); + await (plugin as Record)["event"]({ + event: { type: "session.compacted", properties: { sessionID: "absorbed-duplicate-session" } }, + }); + + const state = await loadSessionState(tmpDir, "absorbed-duplicate-session"); + assert.equal(state.pendingMemories.length, 0, + "duplicate pending memory should be cleared after it is absorbed by existing workspace memory"); + } finally { + await rm(tmpDir, { recursive: true, force: true }); + } +}); + +test("session.compacted clears pending memory absorbed by existing workspace identity", async () => { + const tmpDir = await mkdtemp(join(tmpdir(), "memory-plugin-test-")); + + try { + const now = new Date().toISOString(); + await updateWorkspaceMemory(tmpDir, store => { + store.entries.push({ + id: "mem_existing_parser_formats", + type: "decision", + text: "Parser supports 2 candidate formats.", + source: "compaction", + confidence: 0.9, + status: "active", + createdAt: "2026-04-27T10:00:00.000Z", + updatedAt: "2026-04-27T10:00:00.000Z", + }); + return store; + }); + + await saveSessionState(tmpDir, { + version: 1, + sessionID: "absorbed-identity-session", + turn: 0, + updatedAt: now, + activeFiles: [], + openErrors: [], + recentDecisions: [], + pendingMemories: [{ + id: "mem_pending_parser_formats", + type: "decision", + text: "Parser supports 3 candidate formats.", + source: "compaction", + confidence: 0.75, + status: "active", + createdAt: "2026-04-27T09:00:00.000Z", + updatedAt: "2026-04-27T09:00:00.000Z", + }], + }); + + const plugin = await MemoryV2Plugin({ directory: tmpDir, client: mockRootClient() }); + await (plugin as Record)["event"]({ + event: { type: "session.compacted", properties: { sessionID: "absorbed-identity-session" } }, + }); + + const state = await loadSessionState(tmpDir, "absorbed-identity-session"); + assert.equal(state.pendingMemories.length, 0, + "same-identity pending memory should be cleared after workspace normalization keeps an equivalent entry"); + } finally { + await rm(tmpDir, { recursive: true, force: true }); + } +}); + +test("session.compacted keeps pending memory rejected by workspace entry cap", async () => { + const tmpDir = await mkdtemp(join(tmpdir(), "memory-plugin-test-")); + + try { + const now = new Date().toISOString(); + await updateWorkspaceMemory(tmpDir, store => { + for (let i = 0; i < 28; i += 1) { + store.entries.push({ + id: `mem_high_${i}`, + type: "feedback", + text: `High priority user feedback memory ${i} that should outrank low priority references.`, + source: "explicit", + confidence: 1, + status: "active", + createdAt: now, + updatedAt: now, + }); + } + return store; + }); + + await saveSessionState(tmpDir, { + version: 1, + sessionID: "rejected-cap-session", + turn: 0, + updatedAt: now, + activeFiles: [], + openErrors: [], + recentDecisions: [], + pendingMemories: [{ + id: "mem_low_priority_reference", + type: "reference", + text: "Low priority reference memory that should not fit when the workspace cap is full.", + source: "compaction", + confidence: 0.1, + status: "active", + createdAt: now, + updatedAt: now, + }], + }); + + const plugin = await MemoryV2Plugin({ directory: tmpDir, client: mockRootClient() }); + await (plugin as Record)["event"]({ + event: { type: "session.compacted", properties: { sessionID: "rejected-cap-session" } }, + }); + + const state = await loadSessionState(tmpDir, "rejected-cap-session"); + assert.equal(state.pendingMemories.length, 1, + "pending memory rejected by workspace cap should remain pending for retry"); + assert.match(state.pendingMemories[0].text, /Low priority reference/); + } finally { + await rm(tmpDir, { recursive: true, force: true }); + } +}); + +test("session.compacted keeps pending memories when all rejected by workspace cap", async () => { + const tmpDir = await mkdtemp(join(tmpdir(), "memory-plugin-test-")); + + try { + const now = new Date().toISOString(); + await updateWorkspaceMemory(tmpDir, store => { + for (let i = 0; i < 28; i += 1) { + store.entries.push({ + id: `mem_high_all_rejected_${i}`, + type: "feedback", + text: `Pinned high priority feedback ${i} that keeps the workspace entry cap full.`, + source: "explicit", + confidence: 1, + status: "active", + createdAt: now, + updatedAt: now, + }); + } + return store; + }); + + await saveSessionState(tmpDir, { + version: 1, + sessionID: "all-rejected-session", + turn: 0, + updatedAt: now, + activeFiles: [], + openErrors: [], + recentDecisions: [], + pendingMemories: [{ + id: "mem_session_rejected", + type: "reference", + text: "Session pending reference should remain when every pending memory is rejected by cap.", + source: "compaction", + confidence: 0.1, + status: "active", + createdAt: now, + updatedAt: now, + }], + }); + + const journal = await loadPendingJournal(tmpDir); + journal.entries = [{ + id: "mem_journal_rejected_other_session", + type: "reference", + text: "Journal pending reference from another session should not be cleared by an empty clearable set.", + source: "compaction", + confidence: 0.1, + status: "active", + createdAt: now, + updatedAt: now, + }]; + await savePendingJournal(tmpDir, journal); + + const plugin = await MemoryV2Plugin({ directory: tmpDir, client: mockRootClient() }); + await (plugin as Record)["event"]({ + event: { type: "session.compacted", properties: { sessionID: "all-rejected-session" } }, + }); + + const state = await loadSessionState(tmpDir, "all-rejected-session"); + assert.equal(state.pendingMemories.length, 1, + "session pending memory must remain when all pending memories are rejected"); + + const pendingAfter = await loadPendingJournal(tmpDir); + assert.equal(pendingAfter.entries.length, 1, + "journal pending memories must not be cleared when accounting.clearableKeys is empty"); + assert.match(pendingAfter.entries[0].text, /another session/); + } finally { + await rm(tmpDir, { recursive: true, force: true }); + } +}); + +test("session.compacted keeps rejected journal memories when no workspace entries survive", async () => { + const tmpDir = await mkdtemp(join(tmpdir(), "memory-plugin-test-")); + + try { + const old = new Date(Date.now() - 90 * 86400000).toISOString(); + const journal = await loadPendingJournal(tmpDir); + journal.entries = [{ + id: "mem_stale_journal_rejected", + type: "reference", + text: "Stale journal pending reference should remain pending after pruning rejects it.", + source: "compaction", + confidence: 0.75, + status: "active", + createdAt: old, + updatedAt: old, + staleAfterDays: 1, + }]; + await savePendingJournal(tmpDir, journal); + + const plugin = await MemoryV2Plugin({ directory: tmpDir, client: mockRootClient() }); + await (plugin as Record)["event"]({ + event: { type: "session.compacted", properties: { sessionID: "stale-journal-rejected-session" } }, + }); + + const pendingAfter = await loadPendingJournal(tmpDir); + assert.equal(pendingAfter.entries.length, 1, + "rejected journal memory must not be cleared when accounting.clearableKeys is empty"); + assert.match(pendingAfter.entries[0].text, /Stale journal/); + } finally { + await rm(tmpDir, { recursive: true, force: true }); + } +}); + test("promotion failure does not clear pending memories in session or journal", async () => { const tmpDir = await mkdtemp(join(tmpdir(), "memory-plugin-test-")); diff --git a/tests/promotion-accounting.test.ts b/tests/promotion-accounting.test.ts new file mode 100644 index 0000000..52febda --- /dev/null +++ b/tests/promotion-accounting.test.ts @@ -0,0 +1,109 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import type { LongTermMemoryEntry } from "../src/types.ts"; +import { accountPendingPromotions } from "../src/promotion-accounting.ts"; +import { memoryKey } from "../src/pending-journal.ts"; + +function mem( + id: string, + text: string, + opts: Partial = {}, +): LongTermMemoryEntry { + const now = opts.createdAt ?? new Date().toISOString(); + return { + id, + type: opts.type ?? "decision", + text, + source: opts.source ?? "compaction", + confidence: opts.confidence ?? 0.75, + status: opts.status ?? "active", + createdAt: now, + updatedAt: opts.updatedAt ?? now, + staleAfterDays: opts.staleAfterDays, + rationale: opts.rationale, + supersedes: opts.supersedes, + tags: opts.tags, + }; +} + +test("accountPendingPromotions marks exact retained pending memory as promoted", () => { + const pending = [mem("pending", "Use frozen rendered snapshots for cache stability.")]; + const before: LongTermMemoryEntry[] = []; + const after = [pending[0]]; + + const result = accountPendingPromotions({ pending, before, after }); + + assert.deepEqual([...result.promotedKeys], [memoryKey(pending[0])]); + assert.equal(result.absorbedKeys.size, 0); + assert.equal(result.rejectedKeys.size, 0); + assert.deepEqual([...result.clearableKeys], [memoryKey(pending[0])]); +}); + +test("accountPendingPromotions marks exact duplicate already represented before promotion as absorbed", () => { + const existing = mem("existing", "Prefer stable cache boundaries.", { source: "explicit" }); + const pending = [mem("pending", "prefer stable cache boundaries.", { source: "explicit" })]; + const before = [existing]; + const after = [existing]; + + const result = accountPendingPromotions({ pending, before, after }); + + assert.equal(result.promotedKeys.size, 0); + assert.deepEqual([...result.absorbedKeys], [memoryKey(pending[0])]); + assert.equal(result.rejectedKeys.size, 0); + assert.deepEqual([...result.clearableKeys], [memoryKey(pending[0])]); +}); + +test("accountPendingPromotions marks same exact key present before promotion as absorbed, not promoted", () => { + const existing = mem("existing", "Use stable cache boundaries.", { source: "explicit" }); + const pending = [mem("pending", "Use stable cache boundaries.", { source: "explicit" })]; + const before = [existing]; + const after = [existing]; + + const result = accountPendingPromotions({ pending, before, after }); + + assert.equal(result.promotedKeys.size, 0, + "a pending memory whose exact key already existed before promotion is absorbed, not newly promoted"); + assert.deepEqual([...result.absorbedKeys], [memoryKey(pending[0])]); + assert.equal(result.rejectedKeys.size, 0); +}); + +test("accountPendingPromotions marks same-topic decision represented after normalization as absorbed", () => { + const existing = mem("existing", "Parser supports 2 candidate formats.", { + type: "decision", + source: "compaction", + confidence: 0.9, + createdAt: "2026-04-27T10:00:00.000Z", + updatedAt: "2026-04-27T10:00:00.000Z", + }); + const pending = [mem("pending", "Parser supports 3 candidate formats.", { + type: "decision", + source: "compaction", + confidence: 0.75, + createdAt: "2026-04-27T09:00:00.000Z", + updatedAt: "2026-04-27T09:00:00.000Z", + })]; + const before = [existing]; + const after = [existing]; + + const result = accountPendingPromotions({ pending, before, after }); + + assert.equal(result.promotedKeys.size, 0); + assert.deepEqual([...result.absorbedKeys], [memoryKey(pending[0])]); + assert.equal(result.rejectedKeys.size, 0); +}); + +test("accountPendingPromotions keeps pending memory rejected when no equivalent survived", () => { + const pending = [mem("pending", "Low priority memory that did not fit the workspace cap.", { + type: "reference", + source: "compaction", + })]; + const before: LongTermMemoryEntry[] = []; + const after: LongTermMemoryEntry[] = []; + + const result = accountPendingPromotions({ pending, before, after }); + + assert.equal(result.promotedKeys.size, 0); + assert.equal(result.absorbedKeys.size, 0); + assert.deepEqual([...result.rejectedKeys], [memoryKey(pending[0])]); + assert.equal(result.clearableKeys.size, 0); +});