From e46e0da8852aba714d070ad5ec00264540e72d02 Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Wed, 5 Aug 2026 15:27:02 -0700 Subject: [PATCH] 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 ++++++---------------- skill/scripts/critique-storage.mjs | 29 +++++++++++++++++++---------- tests/context-signals.test.mjs | 16 ++++++++++++++++ tests/critique-storage.test.mjs | 13 ++++++++++++- 4 files changed, 53 insertions(+), 27 deletions(-) diff --git a/skill/scripts/context-signals.mjs b/skill/scripts/context-signals.mjs index 743bb220a..e56214be1 100644 --- a/skill/scripts/context-signals.mjs +++ b/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 (`__.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; diff --git a/skill/scripts/critique-storage.mjs b/skill/scripts/critique-storage.mjs index a8b36b025..f23fded37 100644 --- a/skill/scripts/critique-storage.mjs +++ b/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'))); } diff --git a/tests/context-signals.test.mjs b/tests/context-signals.test.mjs index 505258c76..5480496a3 100644 --- a/tests/context-signals.test.mjs +++ b/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); diff --git a/tests/critique-storage.test.mjs b/tests/critique-storage.test.mjs index 6fcf746e2..3f21601b2 100644 --- a/tests/critique-storage.test.mjs +++ b/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);