Centralize critique snapshot reading (#511)

Make critique storage the single owner of snapshot discovery and frontmatter parsing, and keep context signals focused on summarizing the canonical result.

AI-assisted: Prepared by Codex under pbakaus's scheduled architecture-refactor authorization.

skill/scripts/context-signals.mjs

@@ -22,7 +22,7 @@ import path from 'node:path';
 import { fileURLToPath } from 'node:url';
 import { execFileSync } from 'node:child_process';
 import { loadContext, extractPlatform } from './context.mjs';
-import { getCritiqueDir } from './lib/impeccable-paths.mjs';
+import { readLatestSnapshotAcrossTargets } from './critique-storage.mjs';
 
 /** Is there code here at all, or just context files / an empty repo? */
 function hasCode(cwd) {
@@ -34,23 +34,13 @@ function hasCode(cwd) {
 }
 
 /**
- * The most recent critique snapshot across all targets. Filenames are
- * timestamp-prefixed (`<iso>__<slug>.md`), so a lexical sort is chronological.
- * Parses the small frontmatter for score + P0/P1 counts.
+ * Summarize the most recent critique snapshot across all targets.
  */
 function latestCritique(cwd) {
   try {
-    const dir = getCritiqueDir(cwd);
-    if (!fs.existsSync(dir)) return null;
-    const files = fs.readdirSync(dir).filter((f) => f.endsWith('.md')).sort();
-    if (!files.length) return null;
-    const newest = files[files.length - 1];
-    const text = fs.readFileSync(path.join(dir, newest), 'utf-8');
-    const front = text.split('---')[1] || '';
-    const get = (k) => {
-      const m = front.match(new RegExp(`^${k}:\\s*(.+)$`, 'm'));
-      return m ? m[1].trim() : null;
-    };
+    const latest = readLatestSnapshotAcrossTargets({ cwd });
+    if (!latest) return null;
+    const get = (key) => latest.meta[key] ?? null;
     const num = (v) => {
       const n = Number(v);
       return Number.isFinite(n) ? n : null;
@@ -61,7 +51,7 @@ function latestCritique(cwd) {
       p0: num(get('p0')),
       p1: num(get('p1')),
       timestamp: get('timestamp'),
-      file: path.relative(cwd, path.join(dir, newest)),
+      file: path.relative(cwd, latest.path),
     };
   } catch {
     return null;

skill/scripts/critique-storage.mjs

@@ -105,28 +105,37 @@ function parseFrontmatter(text) {
 }
 
 /**
- * Return all snapshot files for `slug`, sorted oldest → newest.
+ * Return snapshot files matching `suffix`, sorted oldest → newest.
  */
-function listSnapshotsForSlug(slug, cwd) {
+const SNAPSHOT_FILENAME = /^\d{4}-\d{2}-\d{2}T\d{2}-\d{2}-\d{2}Z__.+\.md$/;
+
+function listSnapshots(suffix, cwd) {
   const dir = getCritiqueDir(cwd);
   if (!fs.existsSync(dir)) return [];
-  const suffix = `__${slug}.md`;
   return fs.readdirSync(dir)
-    .filter((f) => f.endsWith(suffix))
+    .filter((f) => SNAPSHOT_FILENAME.test(f) && f.endsWith(suffix))
     .sort()
     .map((f) => path.join(dir, f));
 }
 
+function readLatestSnapshotMatching(suffix, cwd) {
+  const filePath = listSnapshots(suffix, cwd).at(-1);
+  if (!filePath) return null;
+  const body = fs.readFileSync(filePath, 'utf-8');
+  return { path: filePath, body, meta: parseFrontmatter(body) };
+}
+
 /**
  * Return the most recent snapshot for `slug`, or null. Polish reads this
  * to find its fix backlog when the slug matches.
  */
 export function readLatestSnapshot(slug, { cwd = process.cwd() } = {}) {
-  const all = listSnapshotsForSlug(slug, cwd);
-  if (!all.length) return null;
-  const latest = all[all.length - 1];
-  const body = fs.readFileSync(latest, 'utf-8');
-  return { path: latest, body, meta: parseFrontmatter(body) };
+  return readLatestSnapshotMatching(`__${slug}.md`, cwd);
+}
+
+/** Return the most recent snapshot across all targets, or null. */
+export function readLatestSnapshotAcrossTargets({ cwd = process.cwd() } = {}) {
+  return readLatestSnapshotMatching('.md', cwd);
 }
 
 /**
@@ -134,7 +143,7 @@ export function readLatestSnapshot(slug, { cwd = process.cwd() } = {}) {
  * Critique appends a one-line trend to its output using this.
  */
 export function readTrend(slug, { limit = 5, cwd = process.cwd() } = {}) {
-  const all = listSnapshotsForSlug(slug, cwd);
+  const all = listSnapshots(`__${slug}.md`, cwd);
   const slice = all.slice(-limit);
   return slice.map((file) => parseFrontmatter(fs.readFileSync(file, 'utf-8')));
 }

tests/context-signals.test.mjs

@@ -76,6 +76,22 @@ describe('gatherSignals', () => {
     assert.equal(s.critique.latest.slug, 'home');
   });
 
+  it('reads the newest critique snapshot across target slugs', async () => {
+    write('.impeccable/critique/2026-05-01T10-00-00Z__home.md',
+      '---\nslug: home\nscore: 6\np0: 1\np1: 3\ntimestamp: 2026-05-01T10-00-00Z\n---\nbody\n');
+    write('.impeccable/critique/2026-05-02T10-00-00Z__pricing.md',
+      '---\nslug: pricing\nscore: 9\np0: 0\np1: 1\ntimestamp: 2026-05-02T10-00-00Z\n---\nbody\n');
+    write('.impeccable/critique/ignore.md', '# Critique ignores\n');
+    write('.impeccable/critique/9999-not-a-snapshot.md', '# Draft\n');
+    const s = await gatherSignals(scratch);
+    assert.equal(s.critique.latest.slug, 'pricing');
+    assert.equal(s.critique.latest.score, 9);
+    assert.equal(
+      s.critique.latest.file,
+      '.impeccable/critique/2026-05-02T10-00-00Z__pricing.md',
+    );
+  });
+
   it('handles a non-git dir without throwing', async () => {
     const s = await gatherSignals(scratch);
     assert.equal(s.git.isRepo, false);

tests/critique-storage.test.mjs

@@ -5,7 +5,7 @@
 
 import { describe, it, beforeEach, afterEach } from 'node:test';
 import assert from 'node:assert/strict';
-import { mkdtempSync, rmSync, symlinkSync } from 'node:fs';
+import { mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs';
 import { join } from 'node:path';
 import { tmpdir } from 'node:os';
 import { spawnSync } from 'node:child_process';
@@ -17,6 +17,7 @@ import {
   slugFromTarget,
   writeSnapshot,
   readLatestSnapshot,
+  readLatestSnapshotAcrossTargets,
   readTrend,
   nowFilenameStamp,
 } from '../skill/scripts/critique-storage.mjs';
@@ -114,6 +115,16 @@ describe('writeSnapshot + readLatestSnapshot', () => {
     assert.match(latest.body, /new/);
   });
 
+  it('picks the newest snapshot across target slugs', () => {
+    writeSnapshot({ slug: 'home', meta: {}, body: 'old', cwd, now: new Date('2026-05-01T00:00:00Z') });
+    writeSnapshot({ slug: 'pricing', meta: {}, body: 'new', cwd, now: new Date('2026-05-12T00:00:00Z') });
+    writeFileSync(join(cwd, '.impeccable', 'critique', 'ignore.md'), '# Critique ignores\n');
+    writeFileSync(join(cwd, '.impeccable', 'critique', '9999-not-a-snapshot.md'), '# Draft\n');
+    const latest = readLatestSnapshotAcrossTargets({ cwd });
+    assert.equal(latest.meta.slug, 'pricing');
+    assert.match(latest.body, /new/);
+  });
+
   it('does not see snapshots for a different slug', () => {
     writeSnapshot({ slug: 'pricing-astro', meta: { total_score: 10 }, body: 'b', cwd });
     assert.equal(readLatestSnapshot('index-astro', { cwd }), null);