Skip to content

Commit 9991c95

Browse files
committed
fix(memory): make quality cleanup migration conservative
1 parent f7139f0 commit 9991c95

4 files changed

Lines changed: 125 additions & 12 deletions

File tree

src/memory-quality.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,21 @@ export type MemoryQualityResult = {
99
reasons: string[];
1010
};
1111

12+
export const HARD_QUALITY_REASONS: ReadonlySet<string> = new Set([
13+
"empty",
14+
"progress_snapshot",
15+
"raw_error",
16+
"commit_or_ci_snapshot",
17+
"temporary_status",
18+
"active_file_snapshot",
19+
"code_or_api_signature",
20+
"path_heavy",
21+
]);
22+
23+
export function isHardQualityReason(reason: string): boolean {
24+
return HARD_QUALITY_REASONS.has(reason);
25+
}
26+
1227
export function assessMemoryQuality(entry: MemoryQualityInput): MemoryQualityResult {
1328
const reasons: string[] = [];
1429
const text = entry.text.trim();

src/workspace-memory.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { LongTermMemoryEntry, WorkspaceMemoryStore } from "./types.ts";
22
import { LONG_TERM_LIMITS } from "./types.ts";
33
import { workspaceKey, workspaceMemoryPath } from "./paths.ts";
44
import { atomicWriteJSON, readJSON, updateJSON } from "./storage.ts";
5-
import { assessMemoryQuality, isProgressSnapshotViolation } from "./memory-quality.ts";
5+
import { assessMemoryQuality, isHardQualityReason, isProgressSnapshotViolation } from "./memory-quality.ts";
66

77
// Minimum length for workspace_memory envelope: <workspace_memory>\n...\n</workspace_memory>
88
const MIN_ENVELOPE_LENGTH = 80;
@@ -188,9 +188,11 @@ export async function normalizeWorkspaceMemoryWithAccounting(
188188
};
189189
});
190190

191-
// One-time migration for legacy snapshot violations
192-
result = runMigrationP0Cleanup(result, nowIso);
191+
// One-time migrations for legacy/low-quality snapshot violations.
192+
// Run quality cleanup first so hard violations receive quality audit tags
193+
// before the older P0 project-only cleanup marks progress snapshots.
193194
result = runMigrationQualityCleanup(result, nowIso);
195+
result = runMigrationP0Cleanup(result, nowIso);
194196

195197
// P0 accounting only considers active entries. Entries that were already
196198
// superseded before this normalization are preserved in storage; entries that
@@ -285,7 +287,7 @@ export function runMigrationP0Cleanup(
285287
};
286288
}
287289

288-
function runMigrationQualityCleanup(
290+
export function runMigrationQualityCleanup(
289291
store: WorkspaceMemoryStore,
290292
nowIso: string,
291293
): WorkspaceMemoryStore {
@@ -301,11 +303,14 @@ function runMigrationQualityCleanup(
301303
const quality = assessMemoryQuality(entry);
302304
if (quality.accepted) return entry;
303305

306+
const hardReasons = quality.reasons.filter(isHardQualityReason);
307+
if (hardReasons.length === 0) return entry;
308+
304309
changed = true;
305310
const tags = new Set([
306311
...(entry.tags ?? []),
307312
"quality_cleanup",
308-
...quality.reasons.map(reason => `quality:${reason}`),
313+
...hardReasons.map(reason => `quality:${reason}`),
309314
]);
310315

311316
return {

tests/memory-quality-eval.test.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import test from "node:test";
22
import assert from "node:assert/strict";
33
import { extractExplicitMemories, parseWorkspaceMemoryCandidates } from "../src/extractors.ts";
4-
import { assessMemoryQuality } from "../src/memory-quality.ts";
4+
import { assessMemoryQuality, isHardQualityReason } from "../src/memory-quality.ts";
55
import { expectedAcceptedFixtureIds, reviewerCurrent28Fixture } from "./fixtures/memory-quality-current-28.ts";
66

77
const acceptedCases = [
@@ -159,3 +159,17 @@ test("explicit memories bypass extraction quality gate", () => {
159159
assert.equal(entries[0].source, "explicit");
160160
assert.match(entries[0].text, /Wave 1 completed/);
161161
});
162+
163+
test("hard quality reasons exclude soft whitelist failures", () => {
164+
assert.equal(isHardQualityReason("progress_snapshot"), true);
165+
assert.equal(isHardQualityReason("raw_error"), true);
166+
assert.equal(isHardQualityReason("commit_or_ci_snapshot"), true);
167+
assert.equal(isHardQualityReason("temporary_status"), true);
168+
assert.equal(isHardQualityReason("active_file_snapshot"), true);
169+
assert.equal(isHardQualityReason("code_or_api_signature"), true);
170+
assert.equal(isHardQualityReason("path_heavy"), true);
171+
assert.equal(isHardQualityReason("empty"), true);
172+
173+
assert.equal(isHardQualityReason("bad_feedback"), false);
174+
assert.equal(isHardQualityReason("bad_decision"), false);
175+
});

tests/workspace-memory.test.ts

Lines changed: 85 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ import {
2020
saveWorkspaceMemory,
2121
updateWorkspaceMemoryWithAccounting,
2222
} from "../src/workspace-memory.ts";
23-
import { isProgressSnapshotViolation } from "../src/memory-quality.ts";
24-
import { reviewerCurrent28Fixture, expectedAcceptedFixtureIds } from "./fixtures/memory-quality-current-28.ts";
23+
import { assessMemoryQuality, isHardQualityReason, isProgressSnapshotViolation } from "../src/memory-quality.ts";
24+
import { reviewerCurrent28Fixture } from "./fixtures/memory-quality-current-28.ts";
2525

2626
function entry(id: string, text: string, type: LongTermMemoryEntry["type"] = "decision"): LongTermMemoryEntry {
2727
const now = new Date().toISOString();
@@ -954,7 +954,84 @@ test("runMigrationP0Cleanup marks only non-explicit project snapshots and runs o
954954
assert.equal(twice.entries.find(e => e.id === "project-snapshot")?.updatedAt, once.entries.find(e => e.id === "project-snapshot")?.updatedAt);
955955
});
956956

957-
test("quality cleanup migration supersedes low-quality compaction memories from current-28 fixture", async () => {
957+
test("quality cleanup migration preserves soft-only feedback and decision violations", async () => {
958+
const root = await mkdtemp(join(tmpdir(), "wm-quality-soft-preserve-"));
959+
try {
960+
const now = new Date().toISOString();
961+
await saveWorkspaceMemory(root, {
962+
version: 1,
963+
workspace: { root, key: await workspaceKey(root) },
964+
limits: { maxRenderedChars: LONG_TERM_LIMITS.maxRenderedChars, maxEntries: LONG_TERM_LIMITS.maxEntries },
965+
entries: [
966+
{
967+
id: "soft_feedback",
968+
type: "feedback",
969+
text: "UI 要統一風格:兩個表格都要 scrollable,約 20 rows",
970+
source: "compaction",
971+
confidence: 0.75,
972+
status: "active",
973+
createdAt: now,
974+
updatedAt: now,
975+
},
976+
{
977+
id: "soft_decision",
978+
type: "decision",
979+
text: "Product branding is \"OpenCode Working Memory\" without \"Plugin\" in the name",
980+
source: "compaction",
981+
confidence: 0.75,
982+
status: "active",
983+
createdAt: now,
984+
updatedAt: now,
985+
staleAfterDays: 45,
986+
},
987+
],
988+
migrations: [],
989+
updatedAt: now,
990+
});
991+
992+
const loaded = await loadWorkspaceMemory(root);
993+
assert.equal(loaded.entries.find(e => e.id === "soft_feedback")?.status, "active");
994+
assert.equal(loaded.entries.find(e => e.id === "soft_decision")?.status, "active");
995+
assert.ok(loaded.migrations?.includes("2026-04-28-quality-cleanup"));
996+
} finally {
997+
await rm(root, { recursive: true, force: true });
998+
}
999+
});
1000+
1001+
test("quality cleanup migration supersedes hard quality violations", async () => {
1002+
const root = await mkdtemp(join(tmpdir(), "wm-quality-hard-supersede-"));
1003+
try {
1004+
const now = new Date().toISOString();
1005+
await saveWorkspaceMemory(root, {
1006+
version: 1,
1007+
workspace: { root, key: await workspaceKey(root) },
1008+
limits: { maxRenderedChars: LONG_TERM_LIMITS.maxRenderedChars, maxEntries: LONG_TERM_LIMITS.maxEntries },
1009+
entries: [{
1010+
id: "hard_progress",
1011+
type: "project",
1012+
text: "測試套件:1237 tests pass, 226 suites",
1013+
source: "compaction",
1014+
confidence: 0.75,
1015+
status: "active",
1016+
createdAt: now,
1017+
updatedAt: now,
1018+
staleAfterDays: 60,
1019+
}],
1020+
migrations: [],
1021+
updatedAt: now,
1022+
});
1023+
1024+
const loaded = await loadWorkspaceMemory(root);
1025+
const entry = loaded.entries.find(e => e.id === "hard_progress");
1026+
assert.equal(entry?.status, "superseded");
1027+
assert.ok(entry?.tags?.includes("quality_cleanup"));
1028+
assert.ok(entry?.tags?.includes("quality:progress_snapshot"));
1029+
} finally {
1030+
await rm(root, { recursive: true, force: true });
1031+
}
1032+
});
1033+
1034+
test("quality cleanup migration supersedes only hard violations from current fixture", async () => {
9581035
const root = await mkdtemp(join(tmpdir(), "wm-quality-cleanup-"));
9591036
try {
9601037
const now = new Date().toISOString();
@@ -972,10 +1049,12 @@ test("quality cleanup migration supersedes low-quality compaction memories from
9721049
const supersededIds = new Set(loaded.entries.filter(entry => entry.status === "superseded").map(entry => entry.id));
9731050

9741051
for (const entry of reviewerCurrent28Fixture) {
975-
if (expectedAcceptedFixtureIds.has(entry.id)) {
976-
assert.equal(activeIds.has(entry.id), true, `${entry.id} should remain active`);
977-
} else {
1052+
const quality = assessMemoryQuality(entry);
1053+
const hasHardReason = quality.reasons.some(isHardQualityReason);
1054+
if (entry.source === "compaction" && !quality.accepted && hasHardReason) {
9781055
assert.equal(supersededIds.has(entry.id), true, `${entry.id} should be superseded`);
1056+
} else {
1057+
assert.equal(activeIds.has(entry.id), true, `${entry.id} should remain active`);
9791058
}
9801059
}
9811060

0 commit comments

Comments
 (0)