From 44a75b14bdb43f5639d62913080d1c0431c20ecf Mon Sep 17 00:00:00 2001 From: James Wiesebron Date: Mon, 20 Jul 2026 09:58:46 -0700 Subject: [PATCH] [review-corpus-triage-fixes] review: fix the corpus defects behind the drift run's stable misses --- .changeset/review-corpus-triage-fixes.md | 5 + .../golden-retention-lifecycle-1/case.json | 2 +- .../tree/src/notes/retention.ts | 2 +- .../golden-retention-lifecycle-2/case.json | 13 ++- .../tree/src/notes/retention.ts | 2 +- .../golden-retention-lifecycle-3/case.json | 7 +- .../tree/src/notes/retention.ts | 2 +- .../case.json | 8 +- workflows/review/eval/live-match.test.ts | 97 ++++++++++++++++++- 9 files changed, 123 insertions(+), 15 deletions(-) create mode 100644 .changeset/review-corpus-triage-fixes.md diff --git a/.changeset/review-corpus-triage-fixes.md b/.changeset/review-corpus-triage-fixes.md new file mode 100644 index 00000000..0cc8ace0 --- /dev/null +++ b/.changeset/review-corpus-triage-fixes.md @@ -0,0 +1,5 @@ +--- +"review": patch +--- + +Corpus fixes from the triage of drift run 29724668102's stable misses (a ruler change: catch rates move because the ground truth moved, not the reviewer). The retention lifecycle trees replace `void pruneNotes(...)` with `pruneNotes(...).catch(() => {})` so the documented fire-and-forget trap actually delivers "a failing prune must not fail the save"; the old form left an unhandled rejection, making the reviewer's blocking flag on it technically correct, which forced a wrong verdict in every lifecycle-3 sample and injected a guaranteed noise finding into every lifecycle-1/2 run. The `retention-dedup-window-untested` spec gains an altLocation on the test file plus mechanism alternates: the reviewer's real dedup-coverage finding anchors there and could never match (replayed deterministic rate goes 0/6 to 4/6), while the three recorded arbiter fallback accepts for this spec were all the unrelated off-by-one finding; both recorded findings are pinned as matcher regression fixtures. `trial-dedup-eventual-consistency` now expects REQUEST_CHANGES (recorded fixture severity blocking at 0.8, the trial-amplified-default-limit pattern): the case simultaneously required catching a defect that defeats dedup for exactly its target traffic and an APPROVE verdict a blocking catch cannot produce. The lifecycle-3 void-prune trap spec gains a "swallow/empty catch" mechanism alternate to keep the must-not-flag guard effective against the new phrasing. diff --git a/workflows/review/eval/corpus/golden/golden-retention-lifecycle-1/case.json b/workflows/review/eval/corpus/golden/golden-retention-lifecycle-1/case.json index 91a75a11..484bcff9 100644 --- a/workflows/review/eval/corpus/golden/golden-retention-lifecycle-1/case.json +++ b/workflows/review/eval/corpus/golden/golden-retention-lifecycle-1/case.json @@ -140,7 +140,7 @@ "retention-test-asserts-nothing" ] }, - "diff": "diff --git a/src/notes/purge-user-data.ts b/src/notes/purge-user-data.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/purge-user-data.ts\n@@ -0,0 +1,10 @@\n+import type {Db} from \"./db\";\n+\n+/** Part of account deletion: remove every stored note for the user. */\n+export const purgeUserNotes = async (\n+ db: Db,\n+ userId: string,\n+): Promise => {\n+ const notes = await db.query(\"Note\", {userId});\n+ await db.deleteMulti(notes.map((note) => note.id));\n+};\ndiff --git a/src/notes/retention.test.ts b/src/notes/retention.test.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/retention.test.ts\n@@ -0,0 +1,11 @@\n+import {memDb, note} from \"./testing\";\n+import {pruneNotes, saveNote} from \"./retention\";\n+\n+describe(\"pruneNotes\", () => {\n+ it(\"prunes old notes past the cap\", async () => {\n+ const db = memDb();\n+ await saveNote(db, note(\"u1\", \"first\"));\n+ await saveNote(db, note(\"u1\", \"second\"));\n+ await expect(pruneNotes(db, \"u1\")).resolves.toBeUndefined();\n+ });\n+});\ndiff --git a/src/notes/retention.ts b/src/notes/retention.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/retention.ts\n@@ -0,0 +1,22 @@\n+import type {Db, Note} from \"./db\";\n+\n+/** Hard cap on stored notes per user; prune enforces it on every save. */\n+export const MAX_NOTES_PER_USER = 200;\n+\n+export const saveNote = async (db: Db, note: Note): Promise => {\n+ await db.put(note);\n+ // Intentionally fire-and-forget: a failing prune must not fail the save.\n+ void pruneNotes(db, note.userId);\n+};\n+\n+export const pruneNotes = async (db: Db, userId: string): Promise => {\n+ const stale = await db.query(\"Note\", {\n+ userId,\n+ orderDesc: \"createdAt\",\n+ offset: MAX_NOTES_PER_USER - 1,\n+ pageSize: \"all\",\n+ });\n+ if (stale.length > 0) {\n+ await db.deleteMulti(stale.map((note) => note.id));\n+ }\n+};\n", + "diff": "diff --git a/src/notes/purge-user-data.ts b/src/notes/purge-user-data.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/purge-user-data.ts\n@@ -0,0 +1,10 @@\n+import type {Db} from \"./db\";\n+\n+/** Part of account deletion: remove every stored note for the user. */\n+export const purgeUserNotes = async (\n+ db: Db,\n+ userId: string,\n+): Promise => {\n+ const notes = await db.query(\"Note\", {userId});\n+ await db.deleteMulti(notes.map((note) => note.id));\n+};\ndiff --git a/src/notes/retention.test.ts b/src/notes/retention.test.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/retention.test.ts\n@@ -0,0 +1,11 @@\n+import {memDb, note} from \"./testing\";\n+import {pruneNotes, saveNote} from \"./retention\";\n+\n+describe(\"pruneNotes\", () => {\n+ it(\"prunes old notes past the cap\", async () => {\n+ const db = memDb();\n+ await saveNote(db, note(\"u1\", \"first\"));\n+ await saveNote(db, note(\"u1\", \"second\"));\n+ await expect(pruneNotes(db, \"u1\")).resolves.toBeUndefined();\n+ });\n+});\ndiff --git a/src/notes/retention.ts b/src/notes/retention.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/retention.ts\n@@ -0,0 +1,22 @@\n+import type {Db, Note} from \"./db\";\n+\n+/** Hard cap on stored notes per user; prune enforces it on every save. */\n+export const MAX_NOTES_PER_USER = 200;\n+\n+export const saveNote = async (db: Db, note: Note): Promise => {\n+ await db.put(note);\n+ // Intentionally fire-and-forget: a failing prune must not fail the save.\n+ pruneNotes(db, note.userId).catch(() => {});\n+};\n+\n+export const pruneNotes = async (db: Db, userId: string): Promise => {\n+ const stale = await db.query(\"Note\", {\n+ userId,\n+ orderDesc: \"createdAt\",\n+ offset: MAX_NOTES_PER_USER - 1,\n+ pageSize: \"all\",\n+ });\n+ if (stale.length > 0) {\n+ await db.deleteMulti(stale.map((note) => note.id));\n+ }\n+};\n", "live": { "prContext": { "title": "notes: cap stored notes per user and delete them with user data", diff --git a/workflows/review/eval/corpus/golden/golden-retention-lifecycle-1/tree/src/notes/retention.ts b/workflows/review/eval/corpus/golden/golden-retention-lifecycle-1/tree/src/notes/retention.ts index 1cdeb849..06144347 100644 --- a/workflows/review/eval/corpus/golden/golden-retention-lifecycle-1/tree/src/notes/retention.ts +++ b/workflows/review/eval/corpus/golden/golden-retention-lifecycle-1/tree/src/notes/retention.ts @@ -6,7 +6,7 @@ export const MAX_NOTES_PER_USER = 200; export const saveNote = async (db: Db, note: Note): Promise => { await db.put(note); // Intentionally fire-and-forget: a failing prune must not fail the save. - void pruneNotes(db, note.userId); + pruneNotes(db, note.userId).catch(() => {}); }; export const pruneNotes = async (db: Db, userId: string): Promise => { diff --git a/workflows/review/eval/corpus/golden/golden-retention-lifecycle-2/case.json b/workflows/review/eval/corpus/golden/golden-retention-lifecycle-2/case.json index 9e6412a0..596efb9f 100644 --- a/workflows/review/eval/corpus/golden/golden-retention-lifecycle-2/case.json +++ b/workflows/review/eval/corpus/golden/golden-retention-lifecycle-2/case.json @@ -85,7 +85,7 @@ "retention-dedup-prefix-collision" ] }, - "diff": "diff --git a/src/notes/purge-user-data.ts b/src/notes/purge-user-data.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/purge-user-data.ts\n@@ -0,0 +1,15 @@\n+import type {Db} from \"./db\";\n+\n+/** Part of account deletion: remove every stored note for the user. */\n+export const purgeUserNotes = async (\n+ db: Db,\n+ userId: string,\n+): Promise => {\n+ for (;;) {\n+ const notes = await db.query(\"Note\", {userId, pageSize: 100});\n+ if (notes.length === 0) {\n+ return;\n+ }\n+ await db.deleteMulti(notes.map((note) => note.id));\n+ }\n+};\ndiff --git a/src/notes/retention.test.ts b/src/notes/retention.test.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/retention.test.ts\n@@ -0,0 +1,11 @@\n+import {memDb, note} from \"./testing\";\n+import {pruneNotes, saveNote} from \"./retention\";\n+\n+describe(\"pruneNotes\", () => {\n+ it(\"prunes old notes past the cap\", async () => {\n+ const db = memDb();\n+ await saveNote(db, note(\"u1\", \"first\"));\n+ await saveNote(db, note(\"u1\", \"second\"));\n+ await expect(pruneNotes(db, \"u1\")).resolves.toBeUndefined();\n+ });\n+});\ndiff --git a/src/notes/retention.ts b/src/notes/retention.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/retention.ts\n@@ -0,0 +1,44 @@\n+import type {Db, Note} from \"./db\";\n+\n+/** Hard cap on stored notes per user; prune enforces it on every save. */\n+export const MAX_NOTES_PER_USER = 200;\n+\n+/** Window inside which a repeated note is treated as a duplicate. */\n+export const DEDUP_WINDOW_MS = 30 * 24 * 60 * 60 * 1000;\n+\n+const PRUNE_BATCH = 50;\n+\n+/** Duplicate key: normalised content prefix, cheap to compare. */\n+const dedupKey = (content: string): string =>\n+ content.toLowerCase().slice(0, 8);\n+\n+export const saveNote = async (db: Db, note: Note): Promise => {\n+ const existing = await db.query(\"Note\", {\n+ userId: note.userId,\n+ pageSize: \"all\",\n+ });\n+ const since = note.createdAt - DEDUP_WINDOW_MS;\n+ const seen = new Set(\n+ existing\n+ .filter((stored) => stored.createdAt >= since)\n+ .map((stored) => dedupKey(stored.content)),\n+ );\n+ if (seen.has(dedupKey(note.content))) {\n+ return;\n+ }\n+ await db.put(note);\n+ // Intentionally fire-and-forget: a failing prune must not fail the save.\n+ void pruneNotes(db, note.userId);\n+};\n+\n+export const pruneNotes = async (db: Db, userId: string): Promise => {\n+ const stale = await db.query(\"Note\", {\n+ userId,\n+ orderDesc: \"createdAt\",\n+ offset: MAX_NOTES_PER_USER + 1,\n+ pageSize: PRUNE_BATCH,\n+ });\n+ if (stale.length > 0) {\n+ await db.deleteMulti(stale.map((note) => note.id));\n+ }\n+};\n", + "diff": "diff --git a/src/notes/purge-user-data.ts b/src/notes/purge-user-data.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/purge-user-data.ts\n@@ -0,0 +1,15 @@\n+import type {Db} from \"./db\";\n+\n+/** Part of account deletion: remove every stored note for the user. */\n+export const purgeUserNotes = async (\n+ db: Db,\n+ userId: string,\n+): Promise => {\n+ for (;;) {\n+ const notes = await db.query(\"Note\", {userId, pageSize: 100});\n+ if (notes.length === 0) {\n+ return;\n+ }\n+ await db.deleteMulti(notes.map((note) => note.id));\n+ }\n+};\ndiff --git a/src/notes/retention.test.ts b/src/notes/retention.test.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/retention.test.ts\n@@ -0,0 +1,11 @@\n+import {memDb, note} from \"./testing\";\n+import {pruneNotes, saveNote} from \"./retention\";\n+\n+describe(\"pruneNotes\", () => {\n+ it(\"prunes old notes past the cap\", async () => {\n+ const db = memDb();\n+ await saveNote(db, note(\"u1\", \"first\"));\n+ await saveNote(db, note(\"u1\", \"second\"));\n+ await expect(pruneNotes(db, \"u1\")).resolves.toBeUndefined();\n+ });\n+});\ndiff --git a/src/notes/retention.ts b/src/notes/retention.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/retention.ts\n@@ -0,0 +1,44 @@\n+import type {Db, Note} from \"./db\";\n+\n+/** Hard cap on stored notes per user; prune enforces it on every save. */\n+export const MAX_NOTES_PER_USER = 200;\n+\n+/** Window inside which a repeated note is treated as a duplicate. */\n+export const DEDUP_WINDOW_MS = 30 * 24 * 60 * 60 * 1000;\n+\n+const PRUNE_BATCH = 50;\n+\n+/** Duplicate key: normalised content prefix, cheap to compare. */\n+const dedupKey = (content: string): string =>\n+ content.toLowerCase().slice(0, 8);\n+\n+export const saveNote = async (db: Db, note: Note): Promise => {\n+ const existing = await db.query(\"Note\", {\n+ userId: note.userId,\n+ pageSize: \"all\",\n+ });\n+ const since = note.createdAt - DEDUP_WINDOW_MS;\n+ const seen = new Set(\n+ existing\n+ .filter((stored) => stored.createdAt >= since)\n+ .map((stored) => dedupKey(stored.content)),\n+ );\n+ if (seen.has(dedupKey(note.content))) {\n+ return;\n+ }\n+ await db.put(note);\n+ // Intentionally fire-and-forget: a failing prune must not fail the save.\n+ pruneNotes(db, note.userId).catch(() => {});\n+};\n+\n+export const pruneNotes = async (db: Db, userId: string): Promise => {\n+ const stale = await db.query(\"Note\", {\n+ userId,\n+ orderDesc: \"createdAt\",\n+ offset: MAX_NOTES_PER_USER + 1,\n+ pageSize: PRUNE_BATCH,\n+ });\n+ if (stale.length > 0) {\n+ await db.deleteMulti(stale.map((note) => note.id));\n+ }\n+};\n", "live": { "prContext": { "title": "notes: cap stored notes per user and delete them with user data", @@ -106,15 +106,22 @@ { "key": "retention-dedup-window-untested", "path": "src/notes/retention.ts", + "altLocations": [ + { + "path": "src/notes/retention.test.ts" + } + ], "mechanism": [ "window.*(untested|no test)|no test.*window", "DEDUP_WINDOW", - "boundary" + "boundary", + "dedup.{0,60}(untested|not (covered|tested|exercised)|no test)", + "(test|coverage).{0,80}dedup" ] } ], "rereview": { - "priorDiff": "diff --git a/src/notes/purge-user-data.ts b/src/notes/purge-user-data.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/purge-user-data.ts\n@@ -0,0 +1,10 @@\n+import type {Db} from \"./db\";\n+\n+/** Part of account deletion: remove every stored note for the user. */\n+export const purgeUserNotes = async (\n+ db: Db,\n+ userId: string,\n+): Promise => {\n+ const notes = await db.query(\"Note\", {userId});\n+ await db.deleteMulti(notes.map((note) => note.id));\n+};\ndiff --git a/src/notes/retention.test.ts b/src/notes/retention.test.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/retention.test.ts\n@@ -0,0 +1,11 @@\n+import {memDb, note} from \"./testing\";\n+import {pruneNotes, saveNote} from \"./retention\";\n+\n+describe(\"pruneNotes\", () => {\n+ it(\"prunes old notes past the cap\", async () => {\n+ const db = memDb();\n+ await saveNote(db, note(\"u1\", \"first\"));\n+ await saveNote(db, note(\"u1\", \"second\"));\n+ await expect(pruneNotes(db, \"u1\")).resolves.toBeUndefined();\n+ });\n+});\ndiff --git a/src/notes/retention.ts b/src/notes/retention.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/retention.ts\n@@ -0,0 +1,22 @@\n+import type {Db, Note} from \"./db\";\n+\n+/** Hard cap on stored notes per user; prune enforces it on every save. */\n+export const MAX_NOTES_PER_USER = 200;\n+\n+export const saveNote = async (db: Db, note: Note): Promise => {\n+ await db.put(note);\n+ // Intentionally fire-and-forget: a failing prune must not fail the save.\n+ void pruneNotes(db, note.userId);\n+};\n+\n+export const pruneNotes = async (db: Db, userId: string): Promise => {\n+ const stale = await db.query(\"Note\", {\n+ userId,\n+ orderDesc: \"createdAt\",\n+ offset: MAX_NOTES_PER_USER - 1,\n+ pageSize: \"all\",\n+ });\n+ if (stale.length > 0) {\n+ await db.deleteMulti(stale.map((note) => note.id));\n+ }\n+};\n", + "priorDiff": "diff --git a/src/notes/purge-user-data.ts b/src/notes/purge-user-data.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/purge-user-data.ts\n@@ -0,0 +1,10 @@\n+import type {Db} from \"./db\";\n+\n+/** Part of account deletion: remove every stored note for the user. */\n+export const purgeUserNotes = async (\n+ db: Db,\n+ userId: string,\n+): Promise => {\n+ const notes = await db.query(\"Note\", {userId});\n+ await db.deleteMulti(notes.map((note) => note.id));\n+};\ndiff --git a/src/notes/retention.test.ts b/src/notes/retention.test.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/retention.test.ts\n@@ -0,0 +1,11 @@\n+import {memDb, note} from \"./testing\";\n+import {pruneNotes, saveNote} from \"./retention\";\n+\n+describe(\"pruneNotes\", () => {\n+ it(\"prunes old notes past the cap\", async () => {\n+ const db = memDb();\n+ await saveNote(db, note(\"u1\", \"first\"));\n+ await saveNote(db, note(\"u1\", \"second\"));\n+ await expect(pruneNotes(db, \"u1\")).resolves.toBeUndefined();\n+ });\n+});\ndiff --git a/src/notes/retention.ts b/src/notes/retention.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/retention.ts\n@@ -0,0 +1,22 @@\n+import type {Db, Note} from \"./db\";\n+\n+/** Hard cap on stored notes per user; prune enforces it on every save. */\n+export const MAX_NOTES_PER_USER = 200;\n+\n+export const saveNote = async (db: Db, note: Note): Promise => {\n+ await db.put(note);\n+ // Intentionally fire-and-forget: a failing prune must not fail the save.\n+ pruneNotes(db, note.userId).catch(() => {});\n+};\n+\n+export const pruneNotes = async (db: Db, userId: string): Promise => {\n+ const stale = await db.query(\"Note\", {\n+ userId,\n+ orderDesc: \"createdAt\",\n+ offset: MAX_NOTES_PER_USER - 1,\n+ pageSize: \"all\",\n+ });\n+ if (stale.length > 0) {\n+ await db.deleteMulti(stale.map((note) => note.id));\n+ }\n+};\n", "priorVerdict": "REQUEST_CHANGES", "priorDepth": "full", "priorThreads": [ diff --git a/workflows/review/eval/corpus/golden/golden-retention-lifecycle-2/tree/src/notes/retention.ts b/workflows/review/eval/corpus/golden/golden-retention-lifecycle-2/tree/src/notes/retention.ts index fd38a5d1..6d08ba7e 100644 --- a/workflows/review/eval/corpus/golden/golden-retention-lifecycle-2/tree/src/notes/retention.ts +++ b/workflows/review/eval/corpus/golden/golden-retention-lifecycle-2/tree/src/notes/retention.ts @@ -28,7 +28,7 @@ export const saveNote = async (db: Db, note: Note): Promise => { } await db.put(note); // Intentionally fire-and-forget: a failing prune must not fail the save. - void pruneNotes(db, note.userId); + pruneNotes(db, note.userId).catch(() => {}); }; export const pruneNotes = async (db: Db, userId: string): Promise => { diff --git a/workflows/review/eval/corpus/golden/golden-retention-lifecycle-3/case.json b/workflows/review/eval/corpus/golden/golden-retention-lifecycle-3/case.json index b0a742c1..96aa0bdd 100644 --- a/workflows/review/eval/corpus/golden/golden-retention-lifecycle-3/case.json +++ b/workflows/review/eval/corpus/golden/golden-retention-lifecycle-3/case.json @@ -27,7 +27,7 @@ "postedCommentCount": 0, "mustNotPost": [] }, - "diff": "diff --git a/src/notes/purge-user-data.ts b/src/notes/purge-user-data.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/purge-user-data.ts\n@@ -0,0 +1,15 @@\n+import type {Db} from \"./db\";\n+\n+/** Part of account deletion: remove every stored note for the user. */\n+export const purgeUserNotes = async (\n+ db: Db,\n+ userId: string,\n+): Promise => {\n+ for (;;) {\n+ const notes = await db.query(\"Note\", {userId, pageSize: 100});\n+ if (notes.length === 0) {\n+ return;\n+ }\n+ await db.deleteMulti(notes.map((note) => note.id));\n+ }\n+};\ndiff --git a/src/notes/retention.test.ts b/src/notes/retention.test.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/retention.test.ts\n@@ -0,0 +1,30 @@\n+import {memDb, note} from \"./testing\";\n+import {MAX_NOTES_PER_USER, pruneNotes, saveNote} from \"./retention\";\n+\n+const count = async (db: ReturnType): Promise =>\n+ (await db.query(\"Note\", {userId: \"u1\", pageSize: \"all\"})).length;\n+\n+describe(\"retention\", () => {\n+ it(\"caps stored notes per user\", async () => {\n+ const db = memDb();\n+ for (let i = 0; i < MAX_NOTES_PER_USER + 5; i++) {\n+ await saveNote(db, note(\"u1\", `note ${i}`));\n+ }\n+ await pruneNotes(db, \"u1\");\n+ expect(await count(db)).toBeLessThanOrEqual(MAX_NOTES_PER_USER);\n+ });\n+\n+ it(\"dedups an identical note inside the window\", async () => {\n+ const db = memDb();\n+ await saveNote(db, note(\"u1\", \"same content\"));\n+ await saveNote(db, note(\"u1\", \"same content\"));\n+ expect(await count(db)).toBe(1);\n+ });\n+\n+ it(\"keeps distinct notes that share a prefix\", async () => {\n+ const db = memDb();\n+ await saveNote(db, note(\"u1\", \"prefix: alpha\"));\n+ await saveNote(db, note(\"u1\", \"prefix: beta\"));\n+ expect(await count(db)).toBe(2);\n+ });\n+});\ndiff --git a/src/notes/retention.ts b/src/notes/retention.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/retention.ts\n@@ -0,0 +1,39 @@\n+import type {Db, Note} from \"./db\";\n+\n+/** Hard cap on stored notes per user; prune enforces it on every save. */\n+export const MAX_NOTES_PER_USER = 200;\n+\n+/** Window inside which a repeated note is treated as a duplicate. */\n+export const DEDUP_WINDOW_MS = 30 * 24 * 60 * 60 * 1000;\n+\n+const PRUNE_BATCH = 50;\n+\n+export const saveNote = async (db: Db, note: Note): Promise => {\n+ const existing = await db.query(\"Note\", {\n+ userId: note.userId,\n+ pageSize: \"all\",\n+ });\n+ const since = note.createdAt - DEDUP_WINDOW_MS;\n+ const duplicate = existing.some(\n+ (stored) =>\n+ stored.createdAt >= since && stored.content === note.content,\n+ );\n+ if (duplicate) {\n+ return;\n+ }\n+ await db.put(note);\n+ // Intentionally fire-and-forget: a failing prune must not fail the save.\n+ void pruneNotes(db, note.userId);\n+};\n+\n+export const pruneNotes = async (db: Db, userId: string): Promise => {\n+ const stale = await db.query(\"Note\", {\n+ userId,\n+ orderDesc: \"createdAt\",\n+ offset: MAX_NOTES_PER_USER,\n+ pageSize: PRUNE_BATCH,\n+ });\n+ if (stale.length > 0) {\n+ await db.deleteMulti(stale.map((note) => note.id));\n+ }\n+};\n", + "diff": "diff --git a/src/notes/purge-user-data.ts b/src/notes/purge-user-data.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/purge-user-data.ts\n@@ -0,0 +1,15 @@\n+import type {Db} from \"./db\";\n+\n+/** Part of account deletion: remove every stored note for the user. */\n+export const purgeUserNotes = async (\n+ db: Db,\n+ userId: string,\n+): Promise => {\n+ for (;;) {\n+ const notes = await db.query(\"Note\", {userId, pageSize: 100});\n+ if (notes.length === 0) {\n+ return;\n+ }\n+ await db.deleteMulti(notes.map((note) => note.id));\n+ }\n+};\ndiff --git a/src/notes/retention.test.ts b/src/notes/retention.test.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/retention.test.ts\n@@ -0,0 +1,30 @@\n+import {memDb, note} from \"./testing\";\n+import {MAX_NOTES_PER_USER, pruneNotes, saveNote} from \"./retention\";\n+\n+const count = async (db: ReturnType): Promise =>\n+ (await db.query(\"Note\", {userId: \"u1\", pageSize: \"all\"})).length;\n+\n+describe(\"retention\", () => {\n+ it(\"caps stored notes per user\", async () => {\n+ const db = memDb();\n+ for (let i = 0; i < MAX_NOTES_PER_USER + 5; i++) {\n+ await saveNote(db, note(\"u1\", `note ${i}`));\n+ }\n+ await pruneNotes(db, \"u1\");\n+ expect(await count(db)).toBeLessThanOrEqual(MAX_NOTES_PER_USER);\n+ });\n+\n+ it(\"dedups an identical note inside the window\", async () => {\n+ const db = memDb();\n+ await saveNote(db, note(\"u1\", \"same content\"));\n+ await saveNote(db, note(\"u1\", \"same content\"));\n+ expect(await count(db)).toBe(1);\n+ });\n+\n+ it(\"keeps distinct notes that share a prefix\", async () => {\n+ const db = memDb();\n+ await saveNote(db, note(\"u1\", \"prefix: alpha\"));\n+ await saveNote(db, note(\"u1\", \"prefix: beta\"));\n+ expect(await count(db)).toBe(2);\n+ });\n+});\ndiff --git a/src/notes/retention.ts b/src/notes/retention.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/retention.ts\n@@ -0,0 +1,39 @@\n+import type {Db, Note} from \"./db\";\n+\n+/** Hard cap on stored notes per user; prune enforces it on every save. */\n+export const MAX_NOTES_PER_USER = 200;\n+\n+/** Window inside which a repeated note is treated as a duplicate. */\n+export const DEDUP_WINDOW_MS = 30 * 24 * 60 * 60 * 1000;\n+\n+const PRUNE_BATCH = 50;\n+\n+export const saveNote = async (db: Db, note: Note): Promise => {\n+ const existing = await db.query(\"Note\", {\n+ userId: note.userId,\n+ pageSize: \"all\",\n+ });\n+ const since = note.createdAt - DEDUP_WINDOW_MS;\n+ const duplicate = existing.some(\n+ (stored) =>\n+ stored.createdAt >= since && stored.content === note.content,\n+ );\n+ if (duplicate) {\n+ return;\n+ }\n+ await db.put(note);\n+ // Intentionally fire-and-forget: a failing prune must not fail the save.\n+ pruneNotes(db, note.userId).catch(() => {});\n+};\n+\n+export const pruneNotes = async (db: Db, userId: string): Promise => {\n+ const stale = await db.query(\"Note\", {\n+ userId,\n+ orderDesc: \"createdAt\",\n+ offset: MAX_NOTES_PER_USER,\n+ pageSize: PRUNE_BATCH,\n+ });\n+ if (stale.length > 0) {\n+ await db.deleteMulti(stale.map((note) => note.id));\n+ }\n+};\n", "live": { "prContext": { "title": "notes: cap stored notes per user and delete them with user data", @@ -41,12 +41,13 @@ "path": "src/notes/retention.ts", "mechanism": [ "void prune|fire.and.forget", - "discard(ed|s)?.*(error|rejection)|unhandled" + "discard(ed|s)?.*(error|rejection)|unhandled", + "swallow(s|ed|ing)?.*(error|rejection)|empty catch" ] } ], "rereview": { - "priorDiff": "diff --git a/src/notes/purge-user-data.ts b/src/notes/purge-user-data.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/purge-user-data.ts\n@@ -0,0 +1,15 @@\n+import type {Db} from \"./db\";\n+\n+/** Part of account deletion: remove every stored note for the user. */\n+export const purgeUserNotes = async (\n+ db: Db,\n+ userId: string,\n+): Promise => {\n+ for (;;) {\n+ const notes = await db.query(\"Note\", {userId, pageSize: 100});\n+ if (notes.length === 0) {\n+ return;\n+ }\n+ await db.deleteMulti(notes.map((note) => note.id));\n+ }\n+};\ndiff --git a/src/notes/retention.test.ts b/src/notes/retention.test.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/retention.test.ts\n@@ -0,0 +1,11 @@\n+import {memDb, note} from \"./testing\";\n+import {pruneNotes, saveNote} from \"./retention\";\n+\n+describe(\"pruneNotes\", () => {\n+ it(\"prunes old notes past the cap\", async () => {\n+ const db = memDb();\n+ await saveNote(db, note(\"u1\", \"first\"));\n+ await saveNote(db, note(\"u1\", \"second\"));\n+ await expect(pruneNotes(db, \"u1\")).resolves.toBeUndefined();\n+ });\n+});\ndiff --git a/src/notes/retention.ts b/src/notes/retention.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/retention.ts\n@@ -0,0 +1,44 @@\n+import type {Db, Note} from \"./db\";\n+\n+/** Hard cap on stored notes per user; prune enforces it on every save. */\n+export const MAX_NOTES_PER_USER = 200;\n+\n+/** Window inside which a repeated note is treated as a duplicate. */\n+export const DEDUP_WINDOW_MS = 30 * 24 * 60 * 60 * 1000;\n+\n+const PRUNE_BATCH = 50;\n+\n+/** Duplicate key: normalised content prefix, cheap to compare. */\n+const dedupKey = (content: string): string =>\n+ content.toLowerCase().slice(0, 8);\n+\n+export const saveNote = async (db: Db, note: Note): Promise => {\n+ const existing = await db.query(\"Note\", {\n+ userId: note.userId,\n+ pageSize: \"all\",\n+ });\n+ const since = note.createdAt - DEDUP_WINDOW_MS;\n+ const seen = new Set(\n+ existing\n+ .filter((stored) => stored.createdAt >= since)\n+ .map((stored) => dedupKey(stored.content)),\n+ );\n+ if (seen.has(dedupKey(note.content))) {\n+ return;\n+ }\n+ await db.put(note);\n+ // Intentionally fire-and-forget: a failing prune must not fail the save.\n+ void pruneNotes(db, note.userId);\n+};\n+\n+export const pruneNotes = async (db: Db, userId: string): Promise => {\n+ const stale = await db.query(\"Note\", {\n+ userId,\n+ orderDesc: \"createdAt\",\n+ offset: MAX_NOTES_PER_USER + 1,\n+ pageSize: PRUNE_BATCH,\n+ });\n+ if (stale.length > 0) {\n+ await db.deleteMulti(stale.map((note) => note.id));\n+ }\n+};\n", + "priorDiff": "diff --git a/src/notes/purge-user-data.ts b/src/notes/purge-user-data.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/purge-user-data.ts\n@@ -0,0 +1,15 @@\n+import type {Db} from \"./db\";\n+\n+/** Part of account deletion: remove every stored note for the user. */\n+export const purgeUserNotes = async (\n+ db: Db,\n+ userId: string,\n+): Promise => {\n+ for (;;) {\n+ const notes = await db.query(\"Note\", {userId, pageSize: 100});\n+ if (notes.length === 0) {\n+ return;\n+ }\n+ await db.deleteMulti(notes.map((note) => note.id));\n+ }\n+};\ndiff --git a/src/notes/retention.test.ts b/src/notes/retention.test.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/retention.test.ts\n@@ -0,0 +1,11 @@\n+import {memDb, note} from \"./testing\";\n+import {pruneNotes, saveNote} from \"./retention\";\n+\n+describe(\"pruneNotes\", () => {\n+ it(\"prunes old notes past the cap\", async () => {\n+ const db = memDb();\n+ await saveNote(db, note(\"u1\", \"first\"));\n+ await saveNote(db, note(\"u1\", \"second\"));\n+ await expect(pruneNotes(db, \"u1\")).resolves.toBeUndefined();\n+ });\n+});\ndiff --git a/src/notes/retention.ts b/src/notes/retention.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/notes/retention.ts\n@@ -0,0 +1,44 @@\n+import type {Db, Note} from \"./db\";\n+\n+/** Hard cap on stored notes per user; prune enforces it on every save. */\n+export const MAX_NOTES_PER_USER = 200;\n+\n+/** Window inside which a repeated note is treated as a duplicate. */\n+export const DEDUP_WINDOW_MS = 30 * 24 * 60 * 60 * 1000;\n+\n+const PRUNE_BATCH = 50;\n+\n+/** Duplicate key: normalised content prefix, cheap to compare. */\n+const dedupKey = (content: string): string =>\n+ content.toLowerCase().slice(0, 8);\n+\n+export const saveNote = async (db: Db, note: Note): Promise => {\n+ const existing = await db.query(\"Note\", {\n+ userId: note.userId,\n+ pageSize: \"all\",\n+ });\n+ const since = note.createdAt - DEDUP_WINDOW_MS;\n+ const seen = new Set(\n+ existing\n+ .filter((stored) => stored.createdAt >= since)\n+ .map((stored) => dedupKey(stored.content)),\n+ );\n+ if (seen.has(dedupKey(note.content))) {\n+ return;\n+ }\n+ await db.put(note);\n+ // Intentionally fire-and-forget: a failing prune must not fail the save.\n+ pruneNotes(db, note.userId).catch(() => {});\n+};\n+\n+export const pruneNotes = async (db: Db, userId: string): Promise => {\n+ const stale = await db.query(\"Note\", {\n+ userId,\n+ orderDesc: \"createdAt\",\n+ offset: MAX_NOTES_PER_USER + 1,\n+ pageSize: PRUNE_BATCH,\n+ });\n+ if (stale.length > 0) {\n+ await db.deleteMulti(stale.map((note) => note.id));\n+ }\n+};\n", "priorVerdict": "REQUEST_CHANGES", "priorDepth": "full", "priorThreads": [ diff --git a/workflows/review/eval/corpus/golden/golden-retention-lifecycle-3/tree/src/notes/retention.ts b/workflows/review/eval/corpus/golden/golden-retention-lifecycle-3/tree/src/notes/retention.ts index dfa31a21..25542c34 100644 --- a/workflows/review/eval/corpus/golden/golden-retention-lifecycle-3/tree/src/notes/retention.ts +++ b/workflows/review/eval/corpus/golden/golden-retention-lifecycle-3/tree/src/notes/retention.ts @@ -23,7 +23,7 @@ export const saveNote = async (db: Db, note: Note): Promise => { } await db.put(note); // Intentionally fire-and-forget: a failing prune must not fail the save. - void pruneNotes(db, note.userId); + pruneNotes(db, note.userId).catch(() => {}); }; export const pruneNotes = async (db: Db, userId: string): Promise => { diff --git a/workflows/review/eval/corpus/incidents/trial-dedup-eventual-consistency/case.json b/workflows/review/eval/corpus/incidents/trial-dedup-eventual-consistency/case.json index aa26b5e6..e18089ff 100644 --- a/workflows/review/eval/corpus/incidents/trial-dedup-eventual-consistency/case.json +++ b/workflows/review/eval/corpus/incidents/trial-dedup-eventual-consistency/case.json @@ -6,7 +6,7 @@ "live" ], "category": "incident-repro", - "description": "Sanitized structural rewrite of a v1.4.0 re-run miss (dedup push): the save path dedups against a read of the user's recent notes, but the store documents Run as eventually consistent, so a duplicate submitted moments after the original (a retry or double-click, the exact traffic dedup exists for) reads a stale set and is stored again. In the re-run the skill auditor investigated this mechanism and declined to report it for want of a quotable skill rule, and no correctness lens ever surfaced it; ground truth here is that a skill-adjacent correctness issue must surface regardless of which lens owns it.", + "description": "Sanitized structural rewrite of a v1.4.0 re-run miss (dedup push): the save path dedups against a read of the user's recent notes, but the store documents Run as eventually consistent, so a duplicate submitted moments after the original (a retry or double-click, the exact traffic dedup exists for) reads a stale set and is stored again. In the re-run the skill auditor investigated this mechanism and declined to report it for want of a quotable skill rule, and no correctness lens ever surfaced it; ground truth here is that a skill-adjacent correctness issue must surface regardless of which lens owns it. The verdict is REQUEST_CHANGES: the stale read defeats dedup on exactly the retry/double-submit traffic the feature exists for, so a blocking label on the catch is defensible and the 2026-07-20 drift run produced it in every sample that scored this case.", "changedFiles": [ { "path": "services/notes/save.go", @@ -31,8 +31,8 @@ "line": 23, "side": "RIGHT" }, - "severity": "advisory", - "confidence": 0.65, + "severity": "blocking", + "confidence": 0.8, "evidence_trace": [ "services/notes/save.go:23 reads the user's notes to build the dedup set immediately before writing", "services/notes/store.go documents Run as eventually consistent: a note stored by a recent Put can take a short time to become visible", @@ -45,7 +45,7 @@ } ], "expected": { - "verdict": "APPROVE", + "verdict": "REQUEST_CHANGES", "mustCatch": [ "save-dedup-stale-read" ], diff --git a/workflows/review/eval/live-match.test.ts b/workflows/review/eval/live-match.test.ts index d496ce43..693cbb20 100644 --- a/workflows/review/eval/live-match.test.ts +++ b/workflows/review/eval/live-match.test.ts @@ -1,6 +1,6 @@ import {describe, it, expect} from "vitest"; -import {parseCase, type LiveDefectSpec} from "./corpus/loader"; +import {loadLiveCorpus, parseCase, type LiveDefectSpec} from "./corpus/loader"; import { computeLiveMetrics, matchCase, @@ -197,6 +197,101 @@ const finding = (id: string, prose: string, severity = "blocking") => ({ model_authored_prose: prose, }); +describe("retention-dedup-window-untested spec (drift run 29724668102 regressions)", () => { + // That run exposed two matching failures on this spec. The reviewer's + // real dedup-coverage finding anchors on the TEST file, and the spec's + // only location was retention.ts, so it could never match and was + // recorded as a true miss. Meanwhile the Haiku arbiter accepted the + // unrelated cap off-by-one finding for this spec three times (recorded + // via: "fallback"), inflating the catch rate with a non-match. Both + // recorded findings are pinned here (quoted from the run artifact, + // punctuation lightly normalized) against the spec as loaded from the + // corpus, so the fixed spec keeps matching the real finding + // deterministically and never matches the arbiter's false accept. + const dedupSpec = loadLiveCorpus() + .find((c) => c.id === "golden-retention-lifecycle-2") + ?.live?.mustCatchSpecs?.find( + (s) => s.key === "retention-dedup-window-untested", + ); + if (dedupSpec === undefined) { + throw new Error( + "retention-dedup-window-untested spec missing from the corpus", + ); + } + + const coverageFinding = candidate({ + anchor: { + type: "line", + path: "src/notes/retention.test.ts", + line: 5, + side: "RIGHT", + }, + path: "src/notes/retention.test.ts", + line: 5, + finding: { + ...candidate().finding, + anchor: { + type: "line", + path: "src/notes/retention.test.ts", + line: 5, + side: "RIGHT", + }, + failure_scenario: + "The off-by-one at retention.ts:38 or a broken prune would " + + "ship undetected because this test, despite its name, saves " + + "only 2 notes (cap is 200) and asserts only that pruneNotes " + + "resolves; it never exceeds the cap nor checks the " + + "surviving count.", + model_authored_prose: + "Test does not exercise the cap or dedup it claims to " + + "cover. Add a test that saves more than MAX_NOTES_PER_USER " + + "notes and asserts the retained count equals the cap, plus " + + "a dedup test asserting a duplicate is not stored; " + + "otherwise the core new behavior is untested.", + }, + }); + + const arbiterFalseAccept = candidate({ + anchor: { + type: "line", + path: "src/notes/retention.ts", + line: 38, + side: "RIGHT", + }, + path: "src/notes/retention.ts", + line: 38, + finding: { + ...candidate().finding, + anchor: { + type: "line", + path: "src/notes/retention.ts", + line: 38, + side: "RIGHT", + }, + failure_scenario: + "A user accumulates 201 notes; pruneNotes queries with " + + "offset 201 (MAX_NOTES_PER_USER + 1) against a newest-first " + + "result, which returns zero stale rows, so nothing is " + + "deleted and 201 notes remain, one over the intended hard " + + "cap of 200.", + model_authored_prose: + "Off-by-one: offset MAX_NOTES_PER_USER + 1 lets the cap " + + "reach 201. To keep exactly 200 newest notes (indices " + + "0-199) the query must skip 200 and return index 200+, so " + + "offset should be MAX_NOTES_PER_USER, not +1; the current " + + "+1 permanently retains one extra note per user.", + }, + }); + + it("matches the recorded test-file coverage finding deterministically", () => { + expect(matchesSpec(coverageFinding, dedupSpec)).toBe(true); + }); + + it("never matches the off-by-one finding the arbiter wrongly accepted", () => { + expect(matchesSpec(arbiterFalseAccept, dedupSpec)).toBe(false); + }); +}); + describe("matchCase", () => { it("reports caught, missed, and unmatched findings", async () => { const {corpusCase, result} = liveRun({