From 93a13f9882042c4ae1ed2ada2bd92f30c7f22d58 Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Wed, 13 May 2026 00:06:02 -0700 Subject: [PATCH] Critique persistence: per-run snapshots, ignore list, polish reads as signal (#153) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * critique-storage: new helper for per-run snapshot persistence Adds skill/scripts/critique-storage.mjs with: - slugFromTarget(): mechanically derive a stable slug from a resolved file path or URL (NOT from the user's natural-language phrasing), so the same target lands in the same stream across runs even when dev-server ports drift or the user phrases it differently. - writeSnapshot(): writes .impeccable/critique/__.md with a small YAML frontmatter (timestamp, slug, target, total_score, p0_count, p1_count) plus the report body. - readLatestSnapshot(): newest snapshot for a slug, used by polish. - readTrend(): last N frontmatter entries for a slug, used by critique to print the score trend line. - readIgnoreList(): non-empty non-comment lines from ignore.md, the ONLY input critique consumes from prior runs. No separate index.json. The snapshot files are the single source of truth; trend reader globs them and parses frontmatter. Deleting a snapshot removes it from the trend cleanly with no orphan rows. CRITIQUE_DIR constant + getCritiqueDir / getCritiqueIgnorePath added to impeccable-paths.mjs alongside the existing live-dir helpers. 19 unit tests in tests/critique-storage.test.mjs cover slug stability, URL and file inputs, round-trip read/write, trend filtering by slug, and ignore-list parsing. Co-Authored-By: Claude Opus 4.7 (1M context) * critique: persist snapshot per run, respect ignore.md Two new steps wired into the critique flow: - Setup: Resolve Target and Load Ignore List. Before gathering assessments, resolve the user's natural-language target ("the homepage") to a concrete artifact, compute the slug via critique-storage.mjs, and read ignore.md. Matching findings drop silently from the report. This is the only prior-run input critique consumes; anchoring on prior findings would defeat independent assessment. - Persist the Snapshot. After the report is finalized (before Ask the User), write it to .impeccable/critique/__.md with structured frontmatter, then surface a one-line trend ("Trend for index-astro: 24 → 28 → 32") and the written path. First run says "no trend yet". Persistence is fire-and-forget; failures print and move on rather than blocking the rest of the flow. Co-Authored-By: Claude Opus 4.7 (1M context) * polish: read latest matching critique as fix backlog When polish is invoked after critique on the same target, the critique's P0/P1 findings are the right backlog; don't re-derive them. Adds a Setup step that resolves the target, computes the slug via critique-storage.mjs slug, and reads the latest matching snapshot via critique-storage.mjs latest. Found → use those P0/P1 items as the polish backlog and mention the snapshot path. Not found → proceed independently from a clean slate. Explicitly does NOT read snapshots for other targets (cross-target context is pollution). Explicitly does NOT cascade to atomic moves (bolder, quieter, clarify, animate, etc.); those act on a specific selection where the page-level critique would be noise. Co-Authored-By: Claude Opus 4.7 (1M context) * gitignore: .impeccable/critique/, opt ignore.md back in Per-run critique snapshots are local artifacts (same precedent as .impeccable/live/sessions/), but ignore.md carries user-curated deferrals that may be worth sharing across a team. Negate-pattern keeps it trackable while the snapshot files stay local. Co-Authored-By: Claude Opus 4.7 (1M context) * polish: reframe prior critique as additional signal, not backlog Three corrections to the previous polish.md addition: - "Polish is usually invoked after critique" is wrong; people polish without ever running critique. Dropped the presumption. - "This is the only command that auto-reads prior critique" leaks cross-command scope into polish's reference file. Dropped. - Treating critique findings as THE polish backlog biased polish to only fix what critique flagged, skipping its own checklist. The critique is one input among many; fold its P0/P1 items into the polish list, then do the normal pass. Now lives as a short item 4 in Pre-Polish Assessment ("Pull in any prior critique — optional signal") instead of a top-level Setup section. Less prominent, doesn't presume invocation order. Co-Authored-By: Claude Opus 4.7 (1M context) * critique-storage: drop the ignore subcommand, read ignore.md directly The ignore-list helper did nothing the model can't do inline: read a markdown file, skip blank and #-prefix lines. It added a tool roundtrip for no real value. Other helpers earn their keep by doing work the model can't trivially do (path normalization, filename generation, glob + frontmatter parsing); ignore-list did not. Removed: - `ignore` CLI subcommand - readIgnoreList() module export + its tests - getCritiqueIgnorePath() from impeccable-paths.mjs (now dead code) Critique.md step 3 now just says "read .impeccable/critique/ignore.md if it exists" and explains the format inline. Simpler. Co-Authored-By: Claude Opus 4.7 (1M context) * critique-storage: caller meta cannot override timestamp or slug Spotted by Cursor Bugbot on the PR. writeSnapshot built frontmatter as { timestamp, slug, ...meta } so a caller-supplied meta blob (parsed from the IMPECCABLE_CRITIQUE_META env var) could silently clobber the computed timestamp and slug. The filename keeps the computed values, so the frontmatter would drift from the filename and readTrend would attribute scores to wrong timestamps with no visible error. Swap to { ...meta, timestamp, slug } so internal values always win. Add a regression test that passes corrupt meta and asserts the frontmatter still matches the filename. Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- .../skills/impeccable/reference/critique.md | 48 ++++ .agents/skills/impeccable/reference/polish.md | 9 +- .../impeccable/scripts/critique-storage.mjs | 226 ++++++++++++++++++ .../impeccable/scripts/impeccable-paths.mjs | 5 + .../skills/impeccable/reference/critique.md | 48 ++++ .claude/skills/impeccable/reference/polish.md | 9 +- .../impeccable/scripts/critique-storage.mjs | 226 ++++++++++++++++++ .../impeccable/scripts/impeccable-paths.mjs | 5 + .../skills/impeccable/reference/critique.md | 48 ++++ .cursor/skills/impeccable/reference/polish.md | 9 +- .../impeccable/scripts/critique-storage.mjs | 226 ++++++++++++++++++ .../impeccable/scripts/impeccable-paths.mjs | 5 + .../skills/impeccable/reference/critique.md | 48 ++++ .gemini/skills/impeccable/reference/polish.md | 9 +- .../impeccable/scripts/critique-storage.mjs | 226 ++++++++++++++++++ .../impeccable/scripts/impeccable-paths.mjs | 5 + .../skills/impeccable/reference/critique.md | 48 ++++ .github/skills/impeccable/reference/polish.md | 9 +- .../impeccable/scripts/critique-storage.mjs | 226 ++++++++++++++++++ .../impeccable/scripts/impeccable-paths.mjs | 5 + .gitignore | 5 + .kiro/skills/impeccable/reference/critique.md | 48 ++++ .kiro/skills/impeccable/reference/polish.md | 9 +- .../impeccable/scripts/critique-storage.mjs | 226 ++++++++++++++++++ .../impeccable/scripts/impeccable-paths.mjs | 5 + .../skills/impeccable/reference/critique.md | 48 ++++ .../skills/impeccable/reference/polish.md | 9 +- .../impeccable/scripts/critique-storage.mjs | 226 ++++++++++++++++++ .../impeccable/scripts/impeccable-paths.mjs | 5 + .pi/skills/impeccable/reference/critique.md | 48 ++++ .pi/skills/impeccable/reference/polish.md | 9 +- .../impeccable/scripts/critique-storage.mjs | 226 ++++++++++++++++++ .../impeccable/scripts/impeccable-paths.mjs | 5 + .../skills/impeccable/reference/critique.md | 48 ++++ .qoder/skills/impeccable/reference/polish.md | 9 +- .../impeccable/scripts/critique-storage.mjs | 226 ++++++++++++++++++ .../impeccable/scripts/impeccable-paths.mjs | 5 + .../skills/impeccable/reference/critique.md | 48 ++++ .../skills/impeccable/reference/polish.md | 9 +- .../impeccable/scripts/critique-storage.mjs | 226 ++++++++++++++++++ .../impeccable/scripts/impeccable-paths.mjs | 5 + .../skills/impeccable/reference/critique.md | 48 ++++ .../skills/impeccable/reference/polish.md | 9 +- .../impeccable/scripts/critique-storage.mjs | 226 ++++++++++++++++++ .../impeccable/scripts/impeccable-paths.mjs | 5 + .trae/skills/impeccable/reference/critique.md | 48 ++++ .trae/skills/impeccable/reference/polish.md | 9 +- .../impeccable/scripts/critique-storage.mjs | 226 ++++++++++++++++++ .../impeccable/scripts/impeccable-paths.mjs | 5 + .../skills/impeccable/reference/critique.md | 48 ++++ plugin/skills/impeccable/reference/polish.md | 9 +- .../impeccable/scripts/critique-storage.mjs | 226 ++++++++++++++++++ .../impeccable/scripts/impeccable-paths.mjs | 5 + skill/reference/critique.md | 48 ++++ skill/reference/polish.md | 9 +- skill/scripts/critique-storage.mjs | 226 ++++++++++++++++++ skill/scripts/impeccable-paths.mjs | 5 + tests/critique-storage.test.mjs | 173 ++++++++++++++ 58 files changed, 4196 insertions(+), 14 deletions(-) create mode 100644 .agents/skills/impeccable/scripts/critique-storage.mjs create mode 100644 .claude/skills/impeccable/scripts/critique-storage.mjs create mode 100644 .cursor/skills/impeccable/scripts/critique-storage.mjs create mode 100644 .gemini/skills/impeccable/scripts/critique-storage.mjs create mode 100644 .github/skills/impeccable/scripts/critique-storage.mjs create mode 100644 .kiro/skills/impeccable/scripts/critique-storage.mjs create mode 100644 .opencode/skills/impeccable/scripts/critique-storage.mjs create mode 100644 .pi/skills/impeccable/scripts/critique-storage.mjs create mode 100644 .qoder/skills/impeccable/scripts/critique-storage.mjs create mode 100644 .rovodev/skills/impeccable/scripts/critique-storage.mjs create mode 100644 .trae-cn/skills/impeccable/scripts/critique-storage.mjs create mode 100644 .trae/skills/impeccable/scripts/critique-storage.mjs create mode 100644 plugin/skills/impeccable/scripts/critique-storage.mjs create mode 100644 skill/scripts/critique-storage.mjs create mode 100644 tests/critique-storage.test.mjs diff --git a/.agents/skills/impeccable/reference/critique.md b/.agents/skills/impeccable/reference/critique.md index 8079dd3ff..9c33472e7 100644 --- a/.agents/skills/impeccable/reference/critique.md +++ b/.agents/skills/impeccable/reference/critique.md @@ -1,5 +1,23 @@ > **Additional context needed**: what the interface is trying to accomplish. +### Setup: Resolve Target and Load Ignore List + +Before gathering assessments, do two small bookkeeping steps. They cost almost nothing and they're what makes critique iterative across runs. + +1. **Resolve the primary artifact.** The user's phrasing ("the homepage", "the pricing flow") is not stable enough to track across runs. Resolve it to a concrete file path or URL: the same one you'd already need to scan code or open in a browser. Examples: + - "the homepage" → `site/pages/index.astro` (or `http://localhost:3000/` if you're inspecting live) + - "the settings modal" → the primary component file (e.g., `src/components/Settings.tsx`) + - "this page" → the URL or the page's source file + Prefer the source file path over the dev-server URL when both exist; ports drift between runs (`bun dev` vs `bun preview`), file paths don't. + +2. **Compute the slug.** Run: + ```bash + node .agents/skills/impeccable/scripts/critique-storage.mjs slug "" + ``` + Keep the printed slug. It identifies this target's stream across runs. If the command exits non-zero ("no stable slug for input"), skip persistence for this run and tell the user; the trend won't update but the critique still goes ahead. + +3. **Read the ignore list** at `.impeccable/critique/ignore.md` if it exists. Plain markdown; each non-empty, non-comment line is something the user has marked as "do not re-raise" (deferred tradeoffs, designer-intended deviations, detector false-positives the user accepts). When a finding's text matches a line here (case-insensitive substring against rule name or snippet), **drop it silently**. Do not mention it in the report. This is the ONLY input critique consumes from prior runs; anchoring on prior findings would defeat the point of independent assessment. + ### Gather Assessments Launch two independent assessments. **Neither may see the other's output.** This isolation is what makes the combined score honest. Running both in one head silently anchors them to each other; do not shortcut it for cost, speed, or context-size reasons. @@ -164,6 +182,36 @@ Provocative questions that might unlock better solutions: - Prioritize ruthlessly. If everything is important, nothing is. - Don't soften criticism. Developers need honest feedback to ship great design. +### Persist the Snapshot + +Once the report above is finalized, write it to `.impeccable/critique/` so the user can refer back, and so `$impeccable polish` can pick up the priority issues without a copy-paste. + +Skip this step if the Setup slug was null (vague or root-level target). + +1. **Write the body to a temp file** so you can pipe it to the helper. Use the full report (heuristic table, anti-patterns verdict, priority issues, persona red flags) but stop before the "Ask the User" / "Recommended Actions" sections that come later. + +2. **Pass the structured metadata** through `IMPECCABLE_CRITIQUE_META` (JSON), then run the write command: + ```bash + IMPECCABLE_CRITIQUE_META='{"target":"","total_score":,"p0_count":,"p1_count":}' \ + node .agents/skills/impeccable/scripts/critique-storage.mjs write + ``` + The helper prints the absolute path it wrote. + +3. **Read the trend** for context: + ```bash + node .agents/skills/impeccable/scripts/critique-storage.mjs trend 5 + ``` + This returns a JSON array of the last 5 frontmatter entries (including the one you just wrote). + +4. **Append a single line to the user-visible output**, after the report and before the questions: + + > **Trend for `` (last 5 runs): 24 → 28 → 32 → 29 → 32** + > Wrote `.impeccable/critique/`. + + If this is the first run for the slug, the trend is just one score; say so: "First run for this target, no trend yet." + +This is fire-and-forget. Do not show the user the helper's JSON output; only the human-readable trend line and the written path. Failures here should not block the rest of the flow; print the error and move on. + ### Ask the User **After presenting findings**, use targeted questions based on what was actually found. STOP and use Codex's structured user-input/question tool when available; if unavailable, ask directly in chat to clarify what you cannot infer. These answers will shape the action plan. diff --git a/.agents/skills/impeccable/reference/polish.md b/.agents/skills/impeccable/reference/polish.md index eed3dbd27..f67b5e379 100644 --- a/.agents/skills/impeccable/reference/polish.md +++ b/.agents/skills/impeccable/reference/polish.md @@ -35,7 +35,14 @@ Understand the current state and goals before touching anything: - Loading and transition smoothness - Information architecture and flow drift (does this feature reveal complexity the way neighboring features do?) -4. **Triage cosmetic vs functional**: Classify each issue as **cosmetic** (looks off, doesn't impede the user) or **functional** (breaks, blocks, or confuses the experience). When polish time is tight, functional issues ship first; cosmetic ones can land in a follow-up. Quality should be consistent; never perfect one corner while leaving another rough. +4. **Pull in any prior critique** (optional signal): If `$impeccable critique` has been run on the same target, its priority issues are a useful prior for what to address first. Resolve the target to a file path or URL, then: + ```bash + slug=$(node .agents/skills/impeccable/scripts/critique-storage.mjs slug "") + node .agents/skills/impeccable/scripts/critique-storage.mjs latest "$slug" + ``` + Exit 0 with body = found; fold the P0/P1 items into your polish list and mention the snapshot path so the user sees what you read. Exit 2 = no snapshot, continue without it. The critique is one input among many. Do your own pass either way. + +5. **Triage cosmetic vs functional**: Classify each issue as **cosmetic** (looks off, doesn't impede the user) or **functional** (breaks, blocks, or confuses the experience). When polish time is tight, functional issues ship first; cosmetic ones can land in a follow-up. Quality should be consistent; never perfect one corner while leaving another rough. **CRITICAL**: Polish is the last step, not the first. Don't polish work that's not functionally complete. diff --git a/.agents/skills/impeccable/scripts/critique-storage.mjs b/.agents/skills/impeccable/scripts/critique-storage.mjs new file mode 100644 index 000000000..fa7bdf385 --- /dev/null +++ b/.agents/skills/impeccable/scripts/critique-storage.mjs @@ -0,0 +1,226 @@ +#!/usr/bin/env node +/** + * Critique persistence helper. + * + * Each run of /impeccable critique writes a per-target snapshot to + * .impeccable/critique/__.md + * with a small YAML frontmatter carrying the score + P0/P1 counts. + * + * /impeccable polish reads the latest matching snapshot at start as its + * fix backlog. No other skill auto-reads critique output. + * + * The slug is derived mechanically from the *resolved* primary artifact + * (file path or URL), never from the user's natural-language phrasing. + * Slug stability across runs is what lets the trend display work. + * + * CLI entry points (called from skill instructions): + * node critique-storage.mjs slug + * node critique-storage.mjs write + * node critique-storage.mjs latest + * node critique-storage.mjs trend [limit] + * + * Note: there is intentionally no `ignore` subcommand. ignore.md is a plain + * markdown file; the model reads it directly with its file-read tool. This + * helper only exists for operations the model can't trivially do inline + * (normalizing paths, generating filenames, globbing + parsing frontmatter). + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import { getCritiqueDir } from './impeccable-paths.mjs'; + +const SLUG_MAX = 50; + +/** + * Mechanically derive a slug from a resolved target. Returns null if the + * input doesn't look like a stable identifier (empty, project root, etc). + * + * Accepts file paths and URLs. The model resolves "the homepage" to a + * concrete artifact before calling this — we never slug a natural-language + * phrase. + */ +export function slugFromTarget(resolved, { cwd = process.cwd() } = {}) { + if (!resolved || typeof resolved !== 'string') return null; + const trimmed = resolved.trim(); + if (!trimmed) return null; + + // URL + if (/^https?:\/\//i.test(trimmed)) { + let url; + try { url = new URL(trimmed); } catch { return null; } + const hostPath = `${url.hostname}${url.pathname}`; + return kebab(hostPath); + } + + // File path. Make it project-relative so two devs critiquing the same + // checkout get the same slug regardless of where their repo is cloned. + const abs = path.isAbsolute(trimmed) ? trimmed : path.resolve(cwd, trimmed); + let rel = path.relative(cwd, abs); + // If the target is outside cwd, fall back to the basename so we still + // produce a stable slug (vs the absolute path, which would include + // home dirs / usernames). + if (rel.startsWith('..') || path.isAbsolute(rel)) { + rel = path.basename(abs); + } + if (!rel || rel === '.' || rel === '') return null; + return kebab(rel); +} + +function kebab(s) { + const slug = s + .toLowerCase() + .replace(/[/\\.]+/g, '-') + .replace(/[^a-z0-9-]+/g, '-') + .replace(/-+/g, '-') + .replace(/^-|-$/g, ''); + if (!slug) return null; + // Cap from the tail — the tail (filename) is more identifying than the + // top-level directory. + return slug.length <= SLUG_MAX ? slug : slug.slice(slug.length - SLUG_MAX).replace(/^-/, ''); +} + +/** + * Filename-safe UTC ISO timestamp: hyphens for separators, trailing Z. + * Plain colons aren't allowed on Windows filesystems. + */ +export function nowFilenameStamp(date = new Date()) { + const iso = date.toISOString(); // 2026-05-12T18:30:00.123Z + return iso.replace(/[:.]/g, '-').replace(/-\d+Z$/, 'Z'); +} + +/** + * Write a snapshot for `slug`. `meta` carries the small structured frontmatter + * keys read back by readTrend(). `body` is the human-readable critique + * report (everything below the frontmatter). + * + * Returns the absolute path written. + */ +export function writeSnapshot({ slug, meta, body, cwd = process.cwd(), now = new Date() }) { + if (!slug) throw new Error('writeSnapshot requires a slug'); + const dir = getCritiqueDir(cwd); + fs.mkdirSync(dir, { recursive: true }); + const timestamp = nowFilenameStamp(now); + const filePath = path.join(dir, `${timestamp}__${slug}.md`); + // Spread `meta` first so internally computed `timestamp` and `slug` + // always win. Otherwise a caller-supplied meta blob (parsed from the + // IMPECCABLE_CRITIQUE_META env var) could clobber them, leaving the + // filename in disagreement with its frontmatter and corrupting trends. + const front = serializeFrontmatter({ ...meta, timestamp, slug }); + fs.writeFileSync(filePath, `${front}\n${body.trim()}\n`, 'utf-8'); + return filePath; +} + +function serializeFrontmatter(obj) { + const lines = ['---']; + for (const [key, value] of Object.entries(obj)) { + if (value === undefined || value === null) continue; + const str = typeof value === 'string' ? value : String(value); + // Quote strings that contain : or # to keep parsing simple. + const needsQuotes = typeof value === 'string' && /[:#]/.test(str); + lines.push(`${key}: ${needsQuotes ? JSON.stringify(str) : str}`); + } + lines.push('---'); + return lines.join('\n'); +} + +function parseFrontmatter(text) { + const match = text.match(/^---\r?\n([\s\S]*?)\r?\n---/); + if (!match) return {}; + const out = {}; + for (const line of match[1].split(/\r?\n/)) { + const colon = line.indexOf(':'); + if (colon < 0) continue; + const key = line.slice(0, colon).trim(); + let value = line.slice(colon + 1).trim(); + if (/^".*"$/.test(value)) { + try { value = JSON.parse(value); } catch { /* leave as-is */ } + } else if (/^-?\d+$/.test(value)) { + value = Number(value); + } + out[key] = value; + } + return out; +} + +/** + * Return all snapshot files for `slug`, sorted oldest → newest. + */ +function listSnapshotsForSlug(slug, cwd) { + const dir = getCritiqueDir(cwd); + if (!fs.existsSync(dir)) return []; + const suffix = `__${slug}.md`; + return fs.readdirSync(dir) + .filter((f) => f.endsWith(suffix)) + .sort() + .map((f) => path.join(dir, f)); +} + +/** + * 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 the last `limit` snapshots' frontmatter, oldest → newest. + * 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 slice = all.slice(-limit); + return slice.map((file) => parseFrontmatter(fs.readFileSync(file, 'utf-8'))); +} + +// ---- CLI --------------------------------------------------------------- + +function main(argv) { + const [cmd, ...args] = argv; + switch (cmd) { + case 'slug': { + const slug = slugFromTarget(args[0]); + if (!slug) { process.stderr.write('no stable slug for input\n'); process.exit(1); } + process.stdout.write(`${slug}\n`); + return; + } + case 'write': { + const [slug, bodyFile] = args; + if (!slug || !bodyFile) { process.stderr.write('usage: write \n'); process.exit(1); } + const raw = fs.readFileSync(bodyFile, 'utf-8'); + // The body file may be a full report. The caller passes the meta as + // a JSON object on stdin if it wants structured frontmatter; otherwise + // we write with minimal metadata. + let meta = {}; + const metaArg = process.env.IMPECCABLE_CRITIQUE_META; + if (metaArg) { + try { meta = JSON.parse(metaArg); } catch { /* ignore */ } + } + const out = writeSnapshot({ slug, meta, body: raw }); + process.stdout.write(`${out}\n`); + return; + } + case 'latest': { + const latest = readLatestSnapshot(args[0]); + if (!latest) { process.exit(2); } + process.stdout.write(latest.body); + return; + } + case 'trend': { + const rows = readTrend(args[0], { limit: args[1] ? Number(args[1]) : 5 }); + process.stdout.write(JSON.stringify(rows, null, 2) + '\n'); + return; + } + default: + process.stderr.write('usage: critique-storage.mjs [args]\n'); + process.exit(1); + } +} + +if (import.meta.url === `file://${process.argv[1]}`) { + main(process.argv.slice(2)); +} diff --git a/.agents/skills/impeccable/scripts/impeccable-paths.mjs b/.agents/skills/impeccable/scripts/impeccable-paths.mjs index ba852bae9..6befa9cd3 100644 --- a/.agents/skills/impeccable/scripts/impeccable-paths.mjs +++ b/.agents/skills/impeccable/scripts/impeccable-paths.mjs @@ -3,6 +3,7 @@ import path from 'node:path'; export const IMPECCABLE_DIR = '.impeccable'; export const LIVE_DIR = 'live'; +export const CRITIQUE_DIR = 'critique'; export function getImpeccableDir(cwd = process.cwd()) { return path.join(cwd, IMPECCABLE_DIR); @@ -96,6 +97,10 @@ export function getLiveAnnotationsDir(cwd = process.cwd()) { return path.join(getLiveDir(cwd), 'annotations'); } +export function getCritiqueDir(cwd = process.cwd()) { + return path.join(getImpeccableDir(cwd), CRITIQUE_DIR); +} + export function getLegacyLiveAnnotationsDir(cwd = process.cwd()) { return path.join(cwd, '.impeccable-live', 'annotations'); } diff --git a/.claude/skills/impeccable/reference/critique.md b/.claude/skills/impeccable/reference/critique.md index fddf20f11..5e3f3a771 100644 --- a/.claude/skills/impeccable/reference/critique.md +++ b/.claude/skills/impeccable/reference/critique.md @@ -1,5 +1,23 @@ > **Additional context needed**: what the interface is trying to accomplish. +### Setup: Resolve Target and Load Ignore List + +Before gathering assessments, do two small bookkeeping steps. They cost almost nothing and they're what makes critique iterative across runs. + +1. **Resolve the primary artifact.** The user's phrasing ("the homepage", "the pricing flow") is not stable enough to track across runs. Resolve it to a concrete file path or URL: the same one you'd already need to scan code or open in a browser. Examples: + - "the homepage" → `site/pages/index.astro` (or `http://localhost:3000/` if you're inspecting live) + - "the settings modal" → the primary component file (e.g., `src/components/Settings.tsx`) + - "this page" → the URL or the page's source file + Prefer the source file path over the dev-server URL when both exist; ports drift between runs (`bun dev` vs `bun preview`), file paths don't. + +2. **Compute the slug.** Run: + ```bash + node .claude/skills/impeccable/scripts/critique-storage.mjs slug "" + ``` + Keep the printed slug. It identifies this target's stream across runs. If the command exits non-zero ("no stable slug for input"), skip persistence for this run and tell the user; the trend won't update but the critique still goes ahead. + +3. **Read the ignore list** at `.impeccable/critique/ignore.md` if it exists. Plain markdown; each non-empty, non-comment line is something the user has marked as "do not re-raise" (deferred tradeoffs, designer-intended deviations, detector false-positives the user accepts). When a finding's text matches a line here (case-insensitive substring against rule name or snippet), **drop it silently**. Do not mention it in the report. This is the ONLY input critique consumes from prior runs; anchoring on prior findings would defeat the point of independent assessment. + ### Gather Assessments Launch two independent assessments. **Neither may see the other's output.** This isolation is what makes the combined score honest. Running both in one head silently anchors them to each other; do not shortcut it for cost, speed, or context-size reasons. @@ -164,6 +182,36 @@ Provocative questions that might unlock better solutions: - Prioritize ruthlessly. If everything is important, nothing is. - Don't soften criticism. Developers need honest feedback to ship great design. +### Persist the Snapshot + +Once the report above is finalized, write it to `.impeccable/critique/` so the user can refer back, and so `/impeccable polish` can pick up the priority issues without a copy-paste. + +Skip this step if the Setup slug was null (vague or root-level target). + +1. **Write the body to a temp file** so you can pipe it to the helper. Use the full report (heuristic table, anti-patterns verdict, priority issues, persona red flags) but stop before the "Ask the User" / "Recommended Actions" sections that come later. + +2. **Pass the structured metadata** through `IMPECCABLE_CRITIQUE_META` (JSON), then run the write command: + ```bash + IMPECCABLE_CRITIQUE_META='{"target":"","total_score":,"p0_count":,"p1_count":}' \ + node .claude/skills/impeccable/scripts/critique-storage.mjs write + ``` + The helper prints the absolute path it wrote. + +3. **Read the trend** for context: + ```bash + node .claude/skills/impeccable/scripts/critique-storage.mjs trend 5 + ``` + This returns a JSON array of the last 5 frontmatter entries (including the one you just wrote). + +4. **Append a single line to the user-visible output**, after the report and before the questions: + + > **Trend for `` (last 5 runs): 24 → 28 → 32 → 29 → 32** + > Wrote `.impeccable/critique/`. + + If this is the first run for the slug, the trend is just one score; say so: "First run for this target, no trend yet." + +This is fire-and-forget. Do not show the user the helper's JSON output; only the human-readable trend line and the written path. Failures here should not block the rest of the flow; print the error and move on. + ### Ask the User **After presenting findings**, use targeted questions based on what was actually found. STOP and call the AskUserQuestion tool to clarify. These answers will shape the action plan. diff --git a/.claude/skills/impeccable/reference/polish.md b/.claude/skills/impeccable/reference/polish.md index eed3dbd27..0ba47d19d 100644 --- a/.claude/skills/impeccable/reference/polish.md +++ b/.claude/skills/impeccable/reference/polish.md @@ -35,7 +35,14 @@ Understand the current state and goals before touching anything: - Loading and transition smoothness - Information architecture and flow drift (does this feature reveal complexity the way neighboring features do?) -4. **Triage cosmetic vs functional**: Classify each issue as **cosmetic** (looks off, doesn't impede the user) or **functional** (breaks, blocks, or confuses the experience). When polish time is tight, functional issues ship first; cosmetic ones can land in a follow-up. Quality should be consistent; never perfect one corner while leaving another rough. +4. **Pull in any prior critique** (optional signal): If `/impeccable critique` has been run on the same target, its priority issues are a useful prior for what to address first. Resolve the target to a file path or URL, then: + ```bash + slug=$(node .claude/skills/impeccable/scripts/critique-storage.mjs slug "") + node .claude/skills/impeccable/scripts/critique-storage.mjs latest "$slug" + ``` + Exit 0 with body = found; fold the P0/P1 items into your polish list and mention the snapshot path so the user sees what you read. Exit 2 = no snapshot, continue without it. The critique is one input among many. Do your own pass either way. + +5. **Triage cosmetic vs functional**: Classify each issue as **cosmetic** (looks off, doesn't impede the user) or **functional** (breaks, blocks, or confuses the experience). When polish time is tight, functional issues ship first; cosmetic ones can land in a follow-up. Quality should be consistent; never perfect one corner while leaving another rough. **CRITICAL**: Polish is the last step, not the first. Don't polish work that's not functionally complete. diff --git a/.claude/skills/impeccable/scripts/critique-storage.mjs b/.claude/skills/impeccable/scripts/critique-storage.mjs new file mode 100644 index 000000000..fa7bdf385 --- /dev/null +++ b/.claude/skills/impeccable/scripts/critique-storage.mjs @@ -0,0 +1,226 @@ +#!/usr/bin/env node +/** + * Critique persistence helper. + * + * Each run of /impeccable critique writes a per-target snapshot to + * .impeccable/critique/__.md + * with a small YAML frontmatter carrying the score + P0/P1 counts. + * + * /impeccable polish reads the latest matching snapshot at start as its + * fix backlog. No other skill auto-reads critique output. + * + * The slug is derived mechanically from the *resolved* primary artifact + * (file path or URL), never from the user's natural-language phrasing. + * Slug stability across runs is what lets the trend display work. + * + * CLI entry points (called from skill instructions): + * node critique-storage.mjs slug + * node critique-storage.mjs write + * node critique-storage.mjs latest + * node critique-storage.mjs trend [limit] + * + * Note: there is intentionally no `ignore` subcommand. ignore.md is a plain + * markdown file; the model reads it directly with its file-read tool. This + * helper only exists for operations the model can't trivially do inline + * (normalizing paths, generating filenames, globbing + parsing frontmatter). + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import { getCritiqueDir } from './impeccable-paths.mjs'; + +const SLUG_MAX = 50; + +/** + * Mechanically derive a slug from a resolved target. Returns null if the + * input doesn't look like a stable identifier (empty, project root, etc). + * + * Accepts file paths and URLs. The model resolves "the homepage" to a + * concrete artifact before calling this — we never slug a natural-language + * phrase. + */ +export function slugFromTarget(resolved, { cwd = process.cwd() } = {}) { + if (!resolved || typeof resolved !== 'string') return null; + const trimmed = resolved.trim(); + if (!trimmed) return null; + + // URL + if (/^https?:\/\//i.test(trimmed)) { + let url; + try { url = new URL(trimmed); } catch { return null; } + const hostPath = `${url.hostname}${url.pathname}`; + return kebab(hostPath); + } + + // File path. Make it project-relative so two devs critiquing the same + // checkout get the same slug regardless of where their repo is cloned. + const abs = path.isAbsolute(trimmed) ? trimmed : path.resolve(cwd, trimmed); + let rel = path.relative(cwd, abs); + // If the target is outside cwd, fall back to the basename so we still + // produce a stable slug (vs the absolute path, which would include + // home dirs / usernames). + if (rel.startsWith('..') || path.isAbsolute(rel)) { + rel = path.basename(abs); + } + if (!rel || rel === '.' || rel === '') return null; + return kebab(rel); +} + +function kebab(s) { + const slug = s + .toLowerCase() + .replace(/[/\\.]+/g, '-') + .replace(/[^a-z0-9-]+/g, '-') + .replace(/-+/g, '-') + .replace(/^-|-$/g, ''); + if (!slug) return null; + // Cap from the tail — the tail (filename) is more identifying than the + // top-level directory. + return slug.length <= SLUG_MAX ? slug : slug.slice(slug.length - SLUG_MAX).replace(/^-/, ''); +} + +/** + * Filename-safe UTC ISO timestamp: hyphens for separators, trailing Z. + * Plain colons aren't allowed on Windows filesystems. + */ +export function nowFilenameStamp(date = new Date()) { + const iso = date.toISOString(); // 2026-05-12T18:30:00.123Z + return iso.replace(/[:.]/g, '-').replace(/-\d+Z$/, 'Z'); +} + +/** + * Write a snapshot for `slug`. `meta` carries the small structured frontmatter + * keys read back by readTrend(). `body` is the human-readable critique + * report (everything below the frontmatter). + * + * Returns the absolute path written. + */ +export function writeSnapshot({ slug, meta, body, cwd = process.cwd(), now = new Date() }) { + if (!slug) throw new Error('writeSnapshot requires a slug'); + const dir = getCritiqueDir(cwd); + fs.mkdirSync(dir, { recursive: true }); + const timestamp = nowFilenameStamp(now); + const filePath = path.join(dir, `${timestamp}__${slug}.md`); + // Spread `meta` first so internally computed `timestamp` and `slug` + // always win. Otherwise a caller-supplied meta blob (parsed from the + // IMPECCABLE_CRITIQUE_META env var) could clobber them, leaving the + // filename in disagreement with its frontmatter and corrupting trends. + const front = serializeFrontmatter({ ...meta, timestamp, slug }); + fs.writeFileSync(filePath, `${front}\n${body.trim()}\n`, 'utf-8'); + return filePath; +} + +function serializeFrontmatter(obj) { + const lines = ['---']; + for (const [key, value] of Object.entries(obj)) { + if (value === undefined || value === null) continue; + const str = typeof value === 'string' ? value : String(value); + // Quote strings that contain : or # to keep parsing simple. + const needsQuotes = typeof value === 'string' && /[:#]/.test(str); + lines.push(`${key}: ${needsQuotes ? JSON.stringify(str) : str}`); + } + lines.push('---'); + return lines.join('\n'); +} + +function parseFrontmatter(text) { + const match = text.match(/^---\r?\n([\s\S]*?)\r?\n---/); + if (!match) return {}; + const out = {}; + for (const line of match[1].split(/\r?\n/)) { + const colon = line.indexOf(':'); + if (colon < 0) continue; + const key = line.slice(0, colon).trim(); + let value = line.slice(colon + 1).trim(); + if (/^".*"$/.test(value)) { + try { value = JSON.parse(value); } catch { /* leave as-is */ } + } else if (/^-?\d+$/.test(value)) { + value = Number(value); + } + out[key] = value; + } + return out; +} + +/** + * Return all snapshot files for `slug`, sorted oldest → newest. + */ +function listSnapshotsForSlug(slug, cwd) { + const dir = getCritiqueDir(cwd); + if (!fs.existsSync(dir)) return []; + const suffix = `__${slug}.md`; + return fs.readdirSync(dir) + .filter((f) => f.endsWith(suffix)) + .sort() + .map((f) => path.join(dir, f)); +} + +/** + * 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 the last `limit` snapshots' frontmatter, oldest → newest. + * 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 slice = all.slice(-limit); + return slice.map((file) => parseFrontmatter(fs.readFileSync(file, 'utf-8'))); +} + +// ---- CLI --------------------------------------------------------------- + +function main(argv) { + const [cmd, ...args] = argv; + switch (cmd) { + case 'slug': { + const slug = slugFromTarget(args[0]); + if (!slug) { process.stderr.write('no stable slug for input\n'); process.exit(1); } + process.stdout.write(`${slug}\n`); + return; + } + case 'write': { + const [slug, bodyFile] = args; + if (!slug || !bodyFile) { process.stderr.write('usage: write \n'); process.exit(1); } + const raw = fs.readFileSync(bodyFile, 'utf-8'); + // The body file may be a full report. The caller passes the meta as + // a JSON object on stdin if it wants structured frontmatter; otherwise + // we write with minimal metadata. + let meta = {}; + const metaArg = process.env.IMPECCABLE_CRITIQUE_META; + if (metaArg) { + try { meta = JSON.parse(metaArg); } catch { /* ignore */ } + } + const out = writeSnapshot({ slug, meta, body: raw }); + process.stdout.write(`${out}\n`); + return; + } + case 'latest': { + const latest = readLatestSnapshot(args[0]); + if (!latest) { process.exit(2); } + process.stdout.write(latest.body); + return; + } + case 'trend': { + const rows = readTrend(args[0], { limit: args[1] ? Number(args[1]) : 5 }); + process.stdout.write(JSON.stringify(rows, null, 2) + '\n'); + return; + } + default: + process.stderr.write('usage: critique-storage.mjs [args]\n'); + process.exit(1); + } +} + +if (import.meta.url === `file://${process.argv[1]}`) { + main(process.argv.slice(2)); +} diff --git a/.claude/skills/impeccable/scripts/impeccable-paths.mjs b/.claude/skills/impeccable/scripts/impeccable-paths.mjs index ba852bae9..6befa9cd3 100644 --- a/.claude/skills/impeccable/scripts/impeccable-paths.mjs +++ b/.claude/skills/impeccable/scripts/impeccable-paths.mjs @@ -3,6 +3,7 @@ import path from 'node:path'; export const IMPECCABLE_DIR = '.impeccable'; export const LIVE_DIR = 'live'; +export const CRITIQUE_DIR = 'critique'; export function getImpeccableDir(cwd = process.cwd()) { return path.join(cwd, IMPECCABLE_DIR); @@ -96,6 +97,10 @@ export function getLiveAnnotationsDir(cwd = process.cwd()) { return path.join(getLiveDir(cwd), 'annotations'); } +export function getCritiqueDir(cwd = process.cwd()) { + return path.join(getImpeccableDir(cwd), CRITIQUE_DIR); +} + export function getLegacyLiveAnnotationsDir(cwd = process.cwd()) { return path.join(cwd, '.impeccable-live', 'annotations'); } diff --git a/.cursor/skills/impeccable/reference/critique.md b/.cursor/skills/impeccable/reference/critique.md index 1f8ab2868..c22571165 100644 --- a/.cursor/skills/impeccable/reference/critique.md +++ b/.cursor/skills/impeccable/reference/critique.md @@ -1,5 +1,23 @@ > **Additional context needed**: what the interface is trying to accomplish. +### Setup: Resolve Target and Load Ignore List + +Before gathering assessments, do two small bookkeeping steps. They cost almost nothing and they're what makes critique iterative across runs. + +1. **Resolve the primary artifact.** The user's phrasing ("the homepage", "the pricing flow") is not stable enough to track across runs. Resolve it to a concrete file path or URL: the same one you'd already need to scan code or open in a browser. Examples: + - "the homepage" → `site/pages/index.astro` (or `http://localhost:3000/` if you're inspecting live) + - "the settings modal" → the primary component file (e.g., `src/components/Settings.tsx`) + - "this page" → the URL or the page's source file + Prefer the source file path over the dev-server URL when both exist; ports drift between runs (`bun dev` vs `bun preview`), file paths don't. + +2. **Compute the slug.** Run: + ```bash + node .cursor/skills/impeccable/scripts/critique-storage.mjs slug "" + ``` + Keep the printed slug. It identifies this target's stream across runs. If the command exits non-zero ("no stable slug for input"), skip persistence for this run and tell the user; the trend won't update but the critique still goes ahead. + +3. **Read the ignore list** at `.impeccable/critique/ignore.md` if it exists. Plain markdown; each non-empty, non-comment line is something the user has marked as "do not re-raise" (deferred tradeoffs, designer-intended deviations, detector false-positives the user accepts). When a finding's text matches a line here (case-insensitive substring against rule name or snippet), **drop it silently**. Do not mention it in the report. This is the ONLY input critique consumes from prior runs; anchoring on prior findings would defeat the point of independent assessment. + ### Gather Assessments Launch two independent assessments. **Neither may see the other's output.** This isolation is what makes the combined score honest. Running both in one head silently anchors them to each other; do not shortcut it for cost, speed, or context-size reasons. @@ -164,6 +182,36 @@ Provocative questions that might unlock better solutions: - Prioritize ruthlessly. If everything is important, nothing is. - Don't soften criticism. Developers need honest feedback to ship great design. +### Persist the Snapshot + +Once the report above is finalized, write it to `.impeccable/critique/` so the user can refer back, and so `/impeccable polish` can pick up the priority issues without a copy-paste. + +Skip this step if the Setup slug was null (vague or root-level target). + +1. **Write the body to a temp file** so you can pipe it to the helper. Use the full report (heuristic table, anti-patterns verdict, priority issues, persona red flags) but stop before the "Ask the User" / "Recommended Actions" sections that come later. + +2. **Pass the structured metadata** through `IMPECCABLE_CRITIQUE_META` (JSON), then run the write command: + ```bash + IMPECCABLE_CRITIQUE_META='{"target":"","total_score":,"p0_count":,"p1_count":}' \ + node .cursor/skills/impeccable/scripts/critique-storage.mjs write + ``` + The helper prints the absolute path it wrote. + +3. **Read the trend** for context: + ```bash + node .cursor/skills/impeccable/scripts/critique-storage.mjs trend 5 + ``` + This returns a JSON array of the last 5 frontmatter entries (including the one you just wrote). + +4. **Append a single line to the user-visible output**, after the report and before the questions: + + > **Trend for `` (last 5 runs): 24 → 28 → 32 → 29 → 32** + > Wrote `.impeccable/critique/`. + + If this is the first run for the slug, the trend is just one score; say so: "First run for this target, no trend yet." + +This is fire-and-forget. Do not show the user the helper's JSON output; only the human-readable trend line and the written path. Failures here should not block the rest of the flow; print the error and move on. + ### Ask the User **After presenting findings**, use targeted questions based on what was actually found. ask the user directly to clarify what you cannot infer. These answers will shape the action plan. diff --git a/.cursor/skills/impeccable/reference/polish.md b/.cursor/skills/impeccable/reference/polish.md index eed3dbd27..96a410804 100644 --- a/.cursor/skills/impeccable/reference/polish.md +++ b/.cursor/skills/impeccable/reference/polish.md @@ -35,7 +35,14 @@ Understand the current state and goals before touching anything: - Loading and transition smoothness - Information architecture and flow drift (does this feature reveal complexity the way neighboring features do?) -4. **Triage cosmetic vs functional**: Classify each issue as **cosmetic** (looks off, doesn't impede the user) or **functional** (breaks, blocks, or confuses the experience). When polish time is tight, functional issues ship first; cosmetic ones can land in a follow-up. Quality should be consistent; never perfect one corner while leaving another rough. +4. **Pull in any prior critique** (optional signal): If `/impeccable critique` has been run on the same target, its priority issues are a useful prior for what to address first. Resolve the target to a file path or URL, then: + ```bash + slug=$(node .cursor/skills/impeccable/scripts/critique-storage.mjs slug "") + node .cursor/skills/impeccable/scripts/critique-storage.mjs latest "$slug" + ``` + Exit 0 with body = found; fold the P0/P1 items into your polish list and mention the snapshot path so the user sees what you read. Exit 2 = no snapshot, continue without it. The critique is one input among many. Do your own pass either way. + +5. **Triage cosmetic vs functional**: Classify each issue as **cosmetic** (looks off, doesn't impede the user) or **functional** (breaks, blocks, or confuses the experience). When polish time is tight, functional issues ship first; cosmetic ones can land in a follow-up. Quality should be consistent; never perfect one corner while leaving another rough. **CRITICAL**: Polish is the last step, not the first. Don't polish work that's not functionally complete. diff --git a/.cursor/skills/impeccable/scripts/critique-storage.mjs b/.cursor/skills/impeccable/scripts/critique-storage.mjs new file mode 100644 index 000000000..fa7bdf385 --- /dev/null +++ b/.cursor/skills/impeccable/scripts/critique-storage.mjs @@ -0,0 +1,226 @@ +#!/usr/bin/env node +/** + * Critique persistence helper. + * + * Each run of /impeccable critique writes a per-target snapshot to + * .impeccable/critique/__.md + * with a small YAML frontmatter carrying the score + P0/P1 counts. + * + * /impeccable polish reads the latest matching snapshot at start as its + * fix backlog. No other skill auto-reads critique output. + * + * The slug is derived mechanically from the *resolved* primary artifact + * (file path or URL), never from the user's natural-language phrasing. + * Slug stability across runs is what lets the trend display work. + * + * CLI entry points (called from skill instructions): + * node critique-storage.mjs slug + * node critique-storage.mjs write + * node critique-storage.mjs latest + * node critique-storage.mjs trend [limit] + * + * Note: there is intentionally no `ignore` subcommand. ignore.md is a plain + * markdown file; the model reads it directly with its file-read tool. This + * helper only exists for operations the model can't trivially do inline + * (normalizing paths, generating filenames, globbing + parsing frontmatter). + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import { getCritiqueDir } from './impeccable-paths.mjs'; + +const SLUG_MAX = 50; + +/** + * Mechanically derive a slug from a resolved target. Returns null if the + * input doesn't look like a stable identifier (empty, project root, etc). + * + * Accepts file paths and URLs. The model resolves "the homepage" to a + * concrete artifact before calling this — we never slug a natural-language + * phrase. + */ +export function slugFromTarget(resolved, { cwd = process.cwd() } = {}) { + if (!resolved || typeof resolved !== 'string') return null; + const trimmed = resolved.trim(); + if (!trimmed) return null; + + // URL + if (/^https?:\/\//i.test(trimmed)) { + let url; + try { url = new URL(trimmed); } catch { return null; } + const hostPath = `${url.hostname}${url.pathname}`; + return kebab(hostPath); + } + + // File path. Make it project-relative so two devs critiquing the same + // checkout get the same slug regardless of where their repo is cloned. + const abs = path.isAbsolute(trimmed) ? trimmed : path.resolve(cwd, trimmed); + let rel = path.relative(cwd, abs); + // If the target is outside cwd, fall back to the basename so we still + // produce a stable slug (vs the absolute path, which would include + // home dirs / usernames). + if (rel.startsWith('..') || path.isAbsolute(rel)) { + rel = path.basename(abs); + } + if (!rel || rel === '.' || rel === '') return null; + return kebab(rel); +} + +function kebab(s) { + const slug = s + .toLowerCase() + .replace(/[/\\.]+/g, '-') + .replace(/[^a-z0-9-]+/g, '-') + .replace(/-+/g, '-') + .replace(/^-|-$/g, ''); + if (!slug) return null; + // Cap from the tail — the tail (filename) is more identifying than the + // top-level directory. + return slug.length <= SLUG_MAX ? slug : slug.slice(slug.length - SLUG_MAX).replace(/^-/, ''); +} + +/** + * Filename-safe UTC ISO timestamp: hyphens for separators, trailing Z. + * Plain colons aren't allowed on Windows filesystems. + */ +export function nowFilenameStamp(date = new Date()) { + const iso = date.toISOString(); // 2026-05-12T18:30:00.123Z + return iso.replace(/[:.]/g, '-').replace(/-\d+Z$/, 'Z'); +} + +/** + * Write a snapshot for `slug`. `meta` carries the small structured frontmatter + * keys read back by readTrend(). `body` is the human-readable critique + * report (everything below the frontmatter). + * + * Returns the absolute path written. + */ +export function writeSnapshot({ slug, meta, body, cwd = process.cwd(), now = new Date() }) { + if (!slug) throw new Error('writeSnapshot requires a slug'); + const dir = getCritiqueDir(cwd); + fs.mkdirSync(dir, { recursive: true }); + const timestamp = nowFilenameStamp(now); + const filePath = path.join(dir, `${timestamp}__${slug}.md`); + // Spread `meta` first so internally computed `timestamp` and `slug` + // always win. Otherwise a caller-supplied meta blob (parsed from the + // IMPECCABLE_CRITIQUE_META env var) could clobber them, leaving the + // filename in disagreement with its frontmatter and corrupting trends. + const front = serializeFrontmatter({ ...meta, timestamp, slug }); + fs.writeFileSync(filePath, `${front}\n${body.trim()}\n`, 'utf-8'); + return filePath; +} + +function serializeFrontmatter(obj) { + const lines = ['---']; + for (const [key, value] of Object.entries(obj)) { + if (value === undefined || value === null) continue; + const str = typeof value === 'string' ? value : String(value); + // Quote strings that contain : or # to keep parsing simple. + const needsQuotes = typeof value === 'string' && /[:#]/.test(str); + lines.push(`${key}: ${needsQuotes ? JSON.stringify(str) : str}`); + } + lines.push('---'); + return lines.join('\n'); +} + +function parseFrontmatter(text) { + const match = text.match(/^---\r?\n([\s\S]*?)\r?\n---/); + if (!match) return {}; + const out = {}; + for (const line of match[1].split(/\r?\n/)) { + const colon = line.indexOf(':'); + if (colon < 0) continue; + const key = line.slice(0, colon).trim(); + let value = line.slice(colon + 1).trim(); + if (/^".*"$/.test(value)) { + try { value = JSON.parse(value); } catch { /* leave as-is */ } + } else if (/^-?\d+$/.test(value)) { + value = Number(value); + } + out[key] = value; + } + return out; +} + +/** + * Return all snapshot files for `slug`, sorted oldest → newest. + */ +function listSnapshotsForSlug(slug, cwd) { + const dir = getCritiqueDir(cwd); + if (!fs.existsSync(dir)) return []; + const suffix = `__${slug}.md`; + return fs.readdirSync(dir) + .filter((f) => f.endsWith(suffix)) + .sort() + .map((f) => path.join(dir, f)); +} + +/** + * 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 the last `limit` snapshots' frontmatter, oldest → newest. + * 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 slice = all.slice(-limit); + return slice.map((file) => parseFrontmatter(fs.readFileSync(file, 'utf-8'))); +} + +// ---- CLI --------------------------------------------------------------- + +function main(argv) { + const [cmd, ...args] = argv; + switch (cmd) { + case 'slug': { + const slug = slugFromTarget(args[0]); + if (!slug) { process.stderr.write('no stable slug for input\n'); process.exit(1); } + process.stdout.write(`${slug}\n`); + return; + } + case 'write': { + const [slug, bodyFile] = args; + if (!slug || !bodyFile) { process.stderr.write('usage: write \n'); process.exit(1); } + const raw = fs.readFileSync(bodyFile, 'utf-8'); + // The body file may be a full report. The caller passes the meta as + // a JSON object on stdin if it wants structured frontmatter; otherwise + // we write with minimal metadata. + let meta = {}; + const metaArg = process.env.IMPECCABLE_CRITIQUE_META; + if (metaArg) { + try { meta = JSON.parse(metaArg); } catch { /* ignore */ } + } + const out = writeSnapshot({ slug, meta, body: raw }); + process.stdout.write(`${out}\n`); + return; + } + case 'latest': { + const latest = readLatestSnapshot(args[0]); + if (!latest) { process.exit(2); } + process.stdout.write(latest.body); + return; + } + case 'trend': { + const rows = readTrend(args[0], { limit: args[1] ? Number(args[1]) : 5 }); + process.stdout.write(JSON.stringify(rows, null, 2) + '\n'); + return; + } + default: + process.stderr.write('usage: critique-storage.mjs [args]\n'); + process.exit(1); + } +} + +if (import.meta.url === `file://${process.argv[1]}`) { + main(process.argv.slice(2)); +} diff --git a/.cursor/skills/impeccable/scripts/impeccable-paths.mjs b/.cursor/skills/impeccable/scripts/impeccable-paths.mjs index ba852bae9..6befa9cd3 100644 --- a/.cursor/skills/impeccable/scripts/impeccable-paths.mjs +++ b/.cursor/skills/impeccable/scripts/impeccable-paths.mjs @@ -3,6 +3,7 @@ import path from 'node:path'; export const IMPECCABLE_DIR = '.impeccable'; export const LIVE_DIR = 'live'; +export const CRITIQUE_DIR = 'critique'; export function getImpeccableDir(cwd = process.cwd()) { return path.join(cwd, IMPECCABLE_DIR); @@ -96,6 +97,10 @@ export function getLiveAnnotationsDir(cwd = process.cwd()) { return path.join(getLiveDir(cwd), 'annotations'); } +export function getCritiqueDir(cwd = process.cwd()) { + return path.join(getImpeccableDir(cwd), CRITIQUE_DIR); +} + export function getLegacyLiveAnnotationsDir(cwd = process.cwd()) { return path.join(cwd, '.impeccable-live', 'annotations'); } diff --git a/.gemini/skills/impeccable/reference/critique.md b/.gemini/skills/impeccable/reference/critique.md index 0c63f1b7d..fa2e55f7e 100644 --- a/.gemini/skills/impeccable/reference/critique.md +++ b/.gemini/skills/impeccable/reference/critique.md @@ -1,5 +1,23 @@ > **Additional context needed**: what the interface is trying to accomplish. +### Setup: Resolve Target and Load Ignore List + +Before gathering assessments, do two small bookkeeping steps. They cost almost nothing and they're what makes critique iterative across runs. + +1. **Resolve the primary artifact.** The user's phrasing ("the homepage", "the pricing flow") is not stable enough to track across runs. Resolve it to a concrete file path or URL: the same one you'd already need to scan code or open in a browser. Examples: + - "the homepage" → `site/pages/index.astro` (or `http://localhost:3000/` if you're inspecting live) + - "the settings modal" → the primary component file (e.g., `src/components/Settings.tsx`) + - "this page" → the URL or the page's source file + Prefer the source file path over the dev-server URL when both exist; ports drift between runs (`bun dev` vs `bun preview`), file paths don't. + +2. **Compute the slug.** Run: + ```bash + node .gemini/skills/impeccable/scripts/critique-storage.mjs slug "" + ``` + Keep the printed slug. It identifies this target's stream across runs. If the command exits non-zero ("no stable slug for input"), skip persistence for this run and tell the user; the trend won't update but the critique still goes ahead. + +3. **Read the ignore list** at `.impeccable/critique/ignore.md` if it exists. Plain markdown; each non-empty, non-comment line is something the user has marked as "do not re-raise" (deferred tradeoffs, designer-intended deviations, detector false-positives the user accepts). When a finding's text matches a line here (case-insensitive substring against rule name or snippet), **drop it silently**. Do not mention it in the report. This is the ONLY input critique consumes from prior runs; anchoring on prior findings would defeat the point of independent assessment. + ### Gather Assessments Launch two independent assessments. **Neither may see the other's output.** This isolation is what makes the combined score honest. Running both in one head silently anchors them to each other; do not shortcut it for cost, speed, or context-size reasons. @@ -164,6 +182,36 @@ Provocative questions that might unlock better solutions: - Prioritize ruthlessly. If everything is important, nothing is. - Don't soften criticism. Developers need honest feedback to ship great design. +### Persist the Snapshot + +Once the report above is finalized, write it to `.impeccable/critique/` so the user can refer back, and so `/impeccable polish` can pick up the priority issues without a copy-paste. + +Skip this step if the Setup slug was null (vague or root-level target). + +1. **Write the body to a temp file** so you can pipe it to the helper. Use the full report (heuristic table, anti-patterns verdict, priority issues, persona red flags) but stop before the "Ask the User" / "Recommended Actions" sections that come later. + +2. **Pass the structured metadata** through `IMPECCABLE_CRITIQUE_META` (JSON), then run the write command: + ```bash + IMPECCABLE_CRITIQUE_META='{"target":"","total_score":,"p0_count":,"p1_count":}' \ + node .gemini/skills/impeccable/scripts/critique-storage.mjs write + ``` + The helper prints the absolute path it wrote. + +3. **Read the trend** for context: + ```bash + node .gemini/skills/impeccable/scripts/critique-storage.mjs trend 5 + ``` + This returns a JSON array of the last 5 frontmatter entries (including the one you just wrote). + +4. **Append a single line to the user-visible output**, after the report and before the questions: + + > **Trend for `` (last 5 runs): 24 → 28 → 32 → 29 → 32** + > Wrote `.impeccable/critique/`. + + If this is the first run for the slug, the trend is just one score; say so: "First run for this target, no trend yet." + +This is fire-and-forget. Do not show the user the helper's JSON output; only the human-readable trend line and the written path. Failures here should not block the rest of the flow; print the error and move on. + ### Ask the User **After presenting findings**, use targeted questions based on what was actually found. ask the user directly to clarify what you cannot infer. These answers will shape the action plan. diff --git a/.gemini/skills/impeccable/reference/polish.md b/.gemini/skills/impeccable/reference/polish.md index eed3dbd27..17c93a0d3 100644 --- a/.gemini/skills/impeccable/reference/polish.md +++ b/.gemini/skills/impeccable/reference/polish.md @@ -35,7 +35,14 @@ Understand the current state and goals before touching anything: - Loading and transition smoothness - Information architecture and flow drift (does this feature reveal complexity the way neighboring features do?) -4. **Triage cosmetic vs functional**: Classify each issue as **cosmetic** (looks off, doesn't impede the user) or **functional** (breaks, blocks, or confuses the experience). When polish time is tight, functional issues ship first; cosmetic ones can land in a follow-up. Quality should be consistent; never perfect one corner while leaving another rough. +4. **Pull in any prior critique** (optional signal): If `/impeccable critique` has been run on the same target, its priority issues are a useful prior for what to address first. Resolve the target to a file path or URL, then: + ```bash + slug=$(node .gemini/skills/impeccable/scripts/critique-storage.mjs slug "") + node .gemini/skills/impeccable/scripts/critique-storage.mjs latest "$slug" + ``` + Exit 0 with body = found; fold the P0/P1 items into your polish list and mention the snapshot path so the user sees what you read. Exit 2 = no snapshot, continue without it. The critique is one input among many. Do your own pass either way. + +5. **Triage cosmetic vs functional**: Classify each issue as **cosmetic** (looks off, doesn't impede the user) or **functional** (breaks, blocks, or confuses the experience). When polish time is tight, functional issues ship first; cosmetic ones can land in a follow-up. Quality should be consistent; never perfect one corner while leaving another rough. **CRITICAL**: Polish is the last step, not the first. Don't polish work that's not functionally complete. diff --git a/.gemini/skills/impeccable/scripts/critique-storage.mjs b/.gemini/skills/impeccable/scripts/critique-storage.mjs new file mode 100644 index 000000000..fa7bdf385 --- /dev/null +++ b/.gemini/skills/impeccable/scripts/critique-storage.mjs @@ -0,0 +1,226 @@ +#!/usr/bin/env node +/** + * Critique persistence helper. + * + * Each run of /impeccable critique writes a per-target snapshot to + * .impeccable/critique/__.md + * with a small YAML frontmatter carrying the score + P0/P1 counts. + * + * /impeccable polish reads the latest matching snapshot at start as its + * fix backlog. No other skill auto-reads critique output. + * + * The slug is derived mechanically from the *resolved* primary artifact + * (file path or URL), never from the user's natural-language phrasing. + * Slug stability across runs is what lets the trend display work. + * + * CLI entry points (called from skill instructions): + * node critique-storage.mjs slug + * node critique-storage.mjs write + * node critique-storage.mjs latest + * node critique-storage.mjs trend [limit] + * + * Note: there is intentionally no `ignore` subcommand. ignore.md is a plain + * markdown file; the model reads it directly with its file-read tool. This + * helper only exists for operations the model can't trivially do inline + * (normalizing paths, generating filenames, globbing + parsing frontmatter). + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import { getCritiqueDir } from './impeccable-paths.mjs'; + +const SLUG_MAX = 50; + +/** + * Mechanically derive a slug from a resolved target. Returns null if the + * input doesn't look like a stable identifier (empty, project root, etc). + * + * Accepts file paths and URLs. The model resolves "the homepage" to a + * concrete artifact before calling this — we never slug a natural-language + * phrase. + */ +export function slugFromTarget(resolved, { cwd = process.cwd() } = {}) { + if (!resolved || typeof resolved !== 'string') return null; + const trimmed = resolved.trim(); + if (!trimmed) return null; + + // URL + if (/^https?:\/\//i.test(trimmed)) { + let url; + try { url = new URL(trimmed); } catch { return null; } + const hostPath = `${url.hostname}${url.pathname}`; + return kebab(hostPath); + } + + // File path. Make it project-relative so two devs critiquing the same + // checkout get the same slug regardless of where their repo is cloned. + const abs = path.isAbsolute(trimmed) ? trimmed : path.resolve(cwd, trimmed); + let rel = path.relative(cwd, abs); + // If the target is outside cwd, fall back to the basename so we still + // produce a stable slug (vs the absolute path, which would include + // home dirs / usernames). + if (rel.startsWith('..') || path.isAbsolute(rel)) { + rel = path.basename(abs); + } + if (!rel || rel === '.' || rel === '') return null; + return kebab(rel); +} + +function kebab(s) { + const slug = s + .toLowerCase() + .replace(/[/\\.]+/g, '-') + .replace(/[^a-z0-9-]+/g, '-') + .replace(/-+/g, '-') + .replace(/^-|-$/g, ''); + if (!slug) return null; + // Cap from the tail — the tail (filename) is more identifying than the + // top-level directory. + return slug.length <= SLUG_MAX ? slug : slug.slice(slug.length - SLUG_MAX).replace(/^-/, ''); +} + +/** + * Filename-safe UTC ISO timestamp: hyphens for separators, trailing Z. + * Plain colons aren't allowed on Windows filesystems. + */ +export function nowFilenameStamp(date = new Date()) { + const iso = date.toISOString(); // 2026-05-12T18:30:00.123Z + return iso.replace(/[:.]/g, '-').replace(/-\d+Z$/, 'Z'); +} + +/** + * Write a snapshot for `slug`. `meta` carries the small structured frontmatter + * keys read back by readTrend(). `body` is the human-readable critique + * report (everything below the frontmatter). + * + * Returns the absolute path written. + */ +export function writeSnapshot({ slug, meta, body, cwd = process.cwd(), now = new Date() }) { + if (!slug) throw new Error('writeSnapshot requires a slug'); + const dir = getCritiqueDir(cwd); + fs.mkdirSync(dir, { recursive: true }); + const timestamp = nowFilenameStamp(now); + const filePath = path.join(dir, `${timestamp}__${slug}.md`); + // Spread `meta` first so internally computed `timestamp` and `slug` + // always win. Otherwise a caller-supplied meta blob (parsed from the + // IMPECCABLE_CRITIQUE_META env var) could clobber them, leaving the + // filename in disagreement with its frontmatter and corrupting trends. + const front = serializeFrontmatter({ ...meta, timestamp, slug }); + fs.writeFileSync(filePath, `${front}\n${body.trim()}\n`, 'utf-8'); + return filePath; +} + +function serializeFrontmatter(obj) { + const lines = ['---']; + for (const [key, value] of Object.entries(obj)) { + if (value === undefined || value === null) continue; + const str = typeof value === 'string' ? value : String(value); + // Quote strings that contain : or # to keep parsing simple. + const needsQuotes = typeof value === 'string' && /[:#]/.test(str); + lines.push(`${key}: ${needsQuotes ? JSON.stringify(str) : str}`); + } + lines.push('---'); + return lines.join('\n'); +} + +function parseFrontmatter(text) { + const match = text.match(/^---\r?\n([\s\S]*?)\r?\n---/); + if (!match) return {}; + const out = {}; + for (const line of match[1].split(/\r?\n/)) { + const colon = line.indexOf(':'); + if (colon < 0) continue; + const key = line.slice(0, colon).trim(); + let value = line.slice(colon + 1).trim(); + if (/^".*"$/.test(value)) { + try { value = JSON.parse(value); } catch { /* leave as-is */ } + } else if (/^-?\d+$/.test(value)) { + value = Number(value); + } + out[key] = value; + } + return out; +} + +/** + * Return all snapshot files for `slug`, sorted oldest → newest. + */ +function listSnapshotsForSlug(slug, cwd) { + const dir = getCritiqueDir(cwd); + if (!fs.existsSync(dir)) return []; + const suffix = `__${slug}.md`; + return fs.readdirSync(dir) + .filter((f) => f.endsWith(suffix)) + .sort() + .map((f) => path.join(dir, f)); +} + +/** + * 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 the last `limit` snapshots' frontmatter, oldest → newest. + * 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 slice = all.slice(-limit); + return slice.map((file) => parseFrontmatter(fs.readFileSync(file, 'utf-8'))); +} + +// ---- CLI --------------------------------------------------------------- + +function main(argv) { + const [cmd, ...args] = argv; + switch (cmd) { + case 'slug': { + const slug = slugFromTarget(args[0]); + if (!slug) { process.stderr.write('no stable slug for input\n'); process.exit(1); } + process.stdout.write(`${slug}\n`); + return; + } + case 'write': { + const [slug, bodyFile] = args; + if (!slug || !bodyFile) { process.stderr.write('usage: write \n'); process.exit(1); } + const raw = fs.readFileSync(bodyFile, 'utf-8'); + // The body file may be a full report. The caller passes the meta as + // a JSON object on stdin if it wants structured frontmatter; otherwise + // we write with minimal metadata. + let meta = {}; + const metaArg = process.env.IMPECCABLE_CRITIQUE_META; + if (metaArg) { + try { meta = JSON.parse(metaArg); } catch { /* ignore */ } + } + const out = writeSnapshot({ slug, meta, body: raw }); + process.stdout.write(`${out}\n`); + return; + } + case 'latest': { + const latest = readLatestSnapshot(args[0]); + if (!latest) { process.exit(2); } + process.stdout.write(latest.body); + return; + } + case 'trend': { + const rows = readTrend(args[0], { limit: args[1] ? Number(args[1]) : 5 }); + process.stdout.write(JSON.stringify(rows, null, 2) + '\n'); + return; + } + default: + process.stderr.write('usage: critique-storage.mjs [args]\n'); + process.exit(1); + } +} + +if (import.meta.url === `file://${process.argv[1]}`) { + main(process.argv.slice(2)); +} diff --git a/.gemini/skills/impeccable/scripts/impeccable-paths.mjs b/.gemini/skills/impeccable/scripts/impeccable-paths.mjs index ba852bae9..6befa9cd3 100644 --- a/.gemini/skills/impeccable/scripts/impeccable-paths.mjs +++ b/.gemini/skills/impeccable/scripts/impeccable-paths.mjs @@ -3,6 +3,7 @@ import path from 'node:path'; export const IMPECCABLE_DIR = '.impeccable'; export const LIVE_DIR = 'live'; +export const CRITIQUE_DIR = 'critique'; export function getImpeccableDir(cwd = process.cwd()) { return path.join(cwd, IMPECCABLE_DIR); @@ -96,6 +97,10 @@ export function getLiveAnnotationsDir(cwd = process.cwd()) { return path.join(getLiveDir(cwd), 'annotations'); } +export function getCritiqueDir(cwd = process.cwd()) { + return path.join(getImpeccableDir(cwd), CRITIQUE_DIR); +} + export function getLegacyLiveAnnotationsDir(cwd = process.cwd()) { return path.join(cwd, '.impeccable-live', 'annotations'); } diff --git a/.github/skills/impeccable/reference/critique.md b/.github/skills/impeccable/reference/critique.md index abb442146..ec74f1b47 100644 --- a/.github/skills/impeccable/reference/critique.md +++ b/.github/skills/impeccable/reference/critique.md @@ -1,5 +1,23 @@ > **Additional context needed**: what the interface is trying to accomplish. +### Setup: Resolve Target and Load Ignore List + +Before gathering assessments, do two small bookkeeping steps. They cost almost nothing and they're what makes critique iterative across runs. + +1. **Resolve the primary artifact.** The user's phrasing ("the homepage", "the pricing flow") is not stable enough to track across runs. Resolve it to a concrete file path or URL: the same one you'd already need to scan code or open in a browser. Examples: + - "the homepage" → `site/pages/index.astro` (or `http://localhost:3000/` if you're inspecting live) + - "the settings modal" → the primary component file (e.g., `src/components/Settings.tsx`) + - "this page" → the URL or the page's source file + Prefer the source file path over the dev-server URL when both exist; ports drift between runs (`bun dev` vs `bun preview`), file paths don't. + +2. **Compute the slug.** Run: + ```bash + node .github/skills/impeccable/scripts/critique-storage.mjs slug "" + ``` + Keep the printed slug. It identifies this target's stream across runs. If the command exits non-zero ("no stable slug for input"), skip persistence for this run and tell the user; the trend won't update but the critique still goes ahead. + +3. **Read the ignore list** at `.impeccable/critique/ignore.md` if it exists. Plain markdown; each non-empty, non-comment line is something the user has marked as "do not re-raise" (deferred tradeoffs, designer-intended deviations, detector false-positives the user accepts). When a finding's text matches a line here (case-insensitive substring against rule name or snippet), **drop it silently**. Do not mention it in the report. This is the ONLY input critique consumes from prior runs; anchoring on prior findings would defeat the point of independent assessment. + ### Gather Assessments Launch two independent assessments. **Neither may see the other's output.** This isolation is what makes the combined score honest. Running both in one head silently anchors them to each other; do not shortcut it for cost, speed, or context-size reasons. @@ -164,6 +182,36 @@ Provocative questions that might unlock better solutions: - Prioritize ruthlessly. If everything is important, nothing is. - Don't soften criticism. Developers need honest feedback to ship great design. +### Persist the Snapshot + +Once the report above is finalized, write it to `.impeccable/critique/` so the user can refer back, and so `/impeccable polish` can pick up the priority issues without a copy-paste. + +Skip this step if the Setup slug was null (vague or root-level target). + +1. **Write the body to a temp file** so you can pipe it to the helper. Use the full report (heuristic table, anti-patterns verdict, priority issues, persona red flags) but stop before the "Ask the User" / "Recommended Actions" sections that come later. + +2. **Pass the structured metadata** through `IMPECCABLE_CRITIQUE_META` (JSON), then run the write command: + ```bash + IMPECCABLE_CRITIQUE_META='{"target":"","total_score":,"p0_count":,"p1_count":}' \ + node .github/skills/impeccable/scripts/critique-storage.mjs write + ``` + The helper prints the absolute path it wrote. + +3. **Read the trend** for context: + ```bash + node .github/skills/impeccable/scripts/critique-storage.mjs trend 5 + ``` + This returns a JSON array of the last 5 frontmatter entries (including the one you just wrote). + +4. **Append a single line to the user-visible output**, after the report and before the questions: + + > **Trend for `` (last 5 runs): 24 → 28 → 32 → 29 → 32** + > Wrote `.impeccable/critique/`. + + If this is the first run for the slug, the trend is just one score; say so: "First run for this target, no trend yet." + +This is fire-and-forget. Do not show the user the helper's JSON output; only the human-readable trend line and the written path. Failures here should not block the rest of the flow; print the error and move on. + ### Ask the User **After presenting findings**, use targeted questions based on what was actually found. ask the user directly to clarify what you cannot infer. These answers will shape the action plan. diff --git a/.github/skills/impeccable/reference/polish.md b/.github/skills/impeccable/reference/polish.md index eed3dbd27..836f0d8e4 100644 --- a/.github/skills/impeccable/reference/polish.md +++ b/.github/skills/impeccable/reference/polish.md @@ -35,7 +35,14 @@ Understand the current state and goals before touching anything: - Loading and transition smoothness - Information architecture and flow drift (does this feature reveal complexity the way neighboring features do?) -4. **Triage cosmetic vs functional**: Classify each issue as **cosmetic** (looks off, doesn't impede the user) or **functional** (breaks, blocks, or confuses the experience). When polish time is tight, functional issues ship first; cosmetic ones can land in a follow-up. Quality should be consistent; never perfect one corner while leaving another rough. +4. **Pull in any prior critique** (optional signal): If `/impeccable critique` has been run on the same target, its priority issues are a useful prior for what to address first. Resolve the target to a file path or URL, then: + ```bash + slug=$(node .github/skills/impeccable/scripts/critique-storage.mjs slug "") + node .github/skills/impeccable/scripts/critique-storage.mjs latest "$slug" + ``` + Exit 0 with body = found; fold the P0/P1 items into your polish list and mention the snapshot path so the user sees what you read. Exit 2 = no snapshot, continue without it. The critique is one input among many. Do your own pass either way. + +5. **Triage cosmetic vs functional**: Classify each issue as **cosmetic** (looks off, doesn't impede the user) or **functional** (breaks, blocks, or confuses the experience). When polish time is tight, functional issues ship first; cosmetic ones can land in a follow-up. Quality should be consistent; never perfect one corner while leaving another rough. **CRITICAL**: Polish is the last step, not the first. Don't polish work that's not functionally complete. diff --git a/.github/skills/impeccable/scripts/critique-storage.mjs b/.github/skills/impeccable/scripts/critique-storage.mjs new file mode 100644 index 000000000..fa7bdf385 --- /dev/null +++ b/.github/skills/impeccable/scripts/critique-storage.mjs @@ -0,0 +1,226 @@ +#!/usr/bin/env node +/** + * Critique persistence helper. + * + * Each run of /impeccable critique writes a per-target snapshot to + * .impeccable/critique/__.md + * with a small YAML frontmatter carrying the score + P0/P1 counts. + * + * /impeccable polish reads the latest matching snapshot at start as its + * fix backlog. No other skill auto-reads critique output. + * + * The slug is derived mechanically from the *resolved* primary artifact + * (file path or URL), never from the user's natural-language phrasing. + * Slug stability across runs is what lets the trend display work. + * + * CLI entry points (called from skill instructions): + * node critique-storage.mjs slug + * node critique-storage.mjs write + * node critique-storage.mjs latest + * node critique-storage.mjs trend [limit] + * + * Note: there is intentionally no `ignore` subcommand. ignore.md is a plain + * markdown file; the model reads it directly with its file-read tool. This + * helper only exists for operations the model can't trivially do inline + * (normalizing paths, generating filenames, globbing + parsing frontmatter). + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import { getCritiqueDir } from './impeccable-paths.mjs'; + +const SLUG_MAX = 50; + +/** + * Mechanically derive a slug from a resolved target. Returns null if the + * input doesn't look like a stable identifier (empty, project root, etc). + * + * Accepts file paths and URLs. The model resolves "the homepage" to a + * concrete artifact before calling this — we never slug a natural-language + * phrase. + */ +export function slugFromTarget(resolved, { cwd = process.cwd() } = {}) { + if (!resolved || typeof resolved !== 'string') return null; + const trimmed = resolved.trim(); + if (!trimmed) return null; + + // URL + if (/^https?:\/\//i.test(trimmed)) { + let url; + try { url = new URL(trimmed); } catch { return null; } + const hostPath = `${url.hostname}${url.pathname}`; + return kebab(hostPath); + } + + // File path. Make it project-relative so two devs critiquing the same + // checkout get the same slug regardless of where their repo is cloned. + const abs = path.isAbsolute(trimmed) ? trimmed : path.resolve(cwd, trimmed); + let rel = path.relative(cwd, abs); + // If the target is outside cwd, fall back to the basename so we still + // produce a stable slug (vs the absolute path, which would include + // home dirs / usernames). + if (rel.startsWith('..') || path.isAbsolute(rel)) { + rel = path.basename(abs); + } + if (!rel || rel === '.' || rel === '') return null; + return kebab(rel); +} + +function kebab(s) { + const slug = s + .toLowerCase() + .replace(/[/\\.]+/g, '-') + .replace(/[^a-z0-9-]+/g, '-') + .replace(/-+/g, '-') + .replace(/^-|-$/g, ''); + if (!slug) return null; + // Cap from the tail — the tail (filename) is more identifying than the + // top-level directory. + return slug.length <= SLUG_MAX ? slug : slug.slice(slug.length - SLUG_MAX).replace(/^-/, ''); +} + +/** + * Filename-safe UTC ISO timestamp: hyphens for separators, trailing Z. + * Plain colons aren't allowed on Windows filesystems. + */ +export function nowFilenameStamp(date = new Date()) { + const iso = date.toISOString(); // 2026-05-12T18:30:00.123Z + return iso.replace(/[:.]/g, '-').replace(/-\d+Z$/, 'Z'); +} + +/** + * Write a snapshot for `slug`. `meta` carries the small structured frontmatter + * keys read back by readTrend(). `body` is the human-readable critique + * report (everything below the frontmatter). + * + * Returns the absolute path written. + */ +export function writeSnapshot({ slug, meta, body, cwd = process.cwd(), now = new Date() }) { + if (!slug) throw new Error('writeSnapshot requires a slug'); + const dir = getCritiqueDir(cwd); + fs.mkdirSync(dir, { recursive: true }); + const timestamp = nowFilenameStamp(now); + const filePath = path.join(dir, `${timestamp}__${slug}.md`); + // Spread `meta` first so internally computed `timestamp` and `slug` + // always win. Otherwise a caller-supplied meta blob (parsed from the + // IMPECCABLE_CRITIQUE_META env var) could clobber them, leaving the + // filename in disagreement with its frontmatter and corrupting trends. + const front = serializeFrontmatter({ ...meta, timestamp, slug }); + fs.writeFileSync(filePath, `${front}\n${body.trim()}\n`, 'utf-8'); + return filePath; +} + +function serializeFrontmatter(obj) { + const lines = ['---']; + for (const [key, value] of Object.entries(obj)) { + if (value === undefined || value === null) continue; + const str = typeof value === 'string' ? value : String(value); + // Quote strings that contain : or # to keep parsing simple. + const needsQuotes = typeof value === 'string' && /[:#]/.test(str); + lines.push(`${key}: ${needsQuotes ? JSON.stringify(str) : str}`); + } + lines.push('---'); + return lines.join('\n'); +} + +function parseFrontmatter(text) { + const match = text.match(/^---\r?\n([\s\S]*?)\r?\n---/); + if (!match) return {}; + const out = {}; + for (const line of match[1].split(/\r?\n/)) { + const colon = line.indexOf(':'); + if (colon < 0) continue; + const key = line.slice(0, colon).trim(); + let value = line.slice(colon + 1).trim(); + if (/^".*"$/.test(value)) { + try { value = JSON.parse(value); } catch { /* leave as-is */ } + } else if (/^-?\d+$/.test(value)) { + value = Number(value); + } + out[key] = value; + } + return out; +} + +/** + * Return all snapshot files for `slug`, sorted oldest → newest. + */ +function listSnapshotsForSlug(slug, cwd) { + const dir = getCritiqueDir(cwd); + if (!fs.existsSync(dir)) return []; + const suffix = `__${slug}.md`; + return fs.readdirSync(dir) + .filter((f) => f.endsWith(suffix)) + .sort() + .map((f) => path.join(dir, f)); +} + +/** + * 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 the last `limit` snapshots' frontmatter, oldest → newest. + * 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 slice = all.slice(-limit); + return slice.map((file) => parseFrontmatter(fs.readFileSync(file, 'utf-8'))); +} + +// ---- CLI --------------------------------------------------------------- + +function main(argv) { + const [cmd, ...args] = argv; + switch (cmd) { + case 'slug': { + const slug = slugFromTarget(args[0]); + if (!slug) { process.stderr.write('no stable slug for input\n'); process.exit(1); } + process.stdout.write(`${slug}\n`); + return; + } + case 'write': { + const [slug, bodyFile] = args; + if (!slug || !bodyFile) { process.stderr.write('usage: write \n'); process.exit(1); } + const raw = fs.readFileSync(bodyFile, 'utf-8'); + // The body file may be a full report. The caller passes the meta as + // a JSON object on stdin if it wants structured frontmatter; otherwise + // we write with minimal metadata. + let meta = {}; + const metaArg = process.env.IMPECCABLE_CRITIQUE_META; + if (metaArg) { + try { meta = JSON.parse(metaArg); } catch { /* ignore */ } + } + const out = writeSnapshot({ slug, meta, body: raw }); + process.stdout.write(`${out}\n`); + return; + } + case 'latest': { + const latest = readLatestSnapshot(args[0]); + if (!latest) { process.exit(2); } + process.stdout.write(latest.body); + return; + } + case 'trend': { + const rows = readTrend(args[0], { limit: args[1] ? Number(args[1]) : 5 }); + process.stdout.write(JSON.stringify(rows, null, 2) + '\n'); + return; + } + default: + process.stderr.write('usage: critique-storage.mjs [args]\n'); + process.exit(1); + } +} + +if (import.meta.url === `file://${process.argv[1]}`) { + main(process.argv.slice(2)); +} diff --git a/.github/skills/impeccable/scripts/impeccable-paths.mjs b/.github/skills/impeccable/scripts/impeccable-paths.mjs index ba852bae9..6befa9cd3 100644 --- a/.github/skills/impeccable/scripts/impeccable-paths.mjs +++ b/.github/skills/impeccable/scripts/impeccable-paths.mjs @@ -3,6 +3,7 @@ import path from 'node:path'; export const IMPECCABLE_DIR = '.impeccable'; export const LIVE_DIR = 'live'; +export const CRITIQUE_DIR = 'critique'; export function getImpeccableDir(cwd = process.cwd()) { return path.join(cwd, IMPECCABLE_DIR); @@ -96,6 +97,10 @@ export function getLiveAnnotationsDir(cwd = process.cwd()) { return path.join(getLiveDir(cwd), 'annotations'); } +export function getCritiqueDir(cwd = process.cwd()) { + return path.join(getImpeccableDir(cwd), CRITIQUE_DIR); +} + export function getLegacyLiveAnnotationsDir(cwd = process.cwd()) { return path.join(cwd, '.impeccable-live', 'annotations'); } diff --git a/.gitignore b/.gitignore index 6a4b1356e..e2e2cbbc8 100644 --- a/.gitignore +++ b/.gitignore @@ -44,6 +44,11 @@ Thumbs.db .impeccable/live/annotations/ .impeccable/live/cache/ .impeccable/history/ +# Per-run critique snapshots are local artifacts. ignore.md (also under +# this dir) carries deferrals the user may want to share, so it's +# explicitly re-included below. +.impeccable/critique/ +!.impeccable/critique/ignore.md # Legacy live mode session file + annotation screenshots .impeccable-live.json diff --git a/.kiro/skills/impeccable/reference/critique.md b/.kiro/skills/impeccable/reference/critique.md index 5ed7159be..fa2880c6f 100644 --- a/.kiro/skills/impeccable/reference/critique.md +++ b/.kiro/skills/impeccable/reference/critique.md @@ -1,5 +1,23 @@ > **Additional context needed**: what the interface is trying to accomplish. +### Setup: Resolve Target and Load Ignore List + +Before gathering assessments, do two small bookkeeping steps. They cost almost nothing and they're what makes critique iterative across runs. + +1. **Resolve the primary artifact.** The user's phrasing ("the homepage", "the pricing flow") is not stable enough to track across runs. Resolve it to a concrete file path or URL: the same one you'd already need to scan code or open in a browser. Examples: + - "the homepage" → `site/pages/index.astro` (or `http://localhost:3000/` if you're inspecting live) + - "the settings modal" → the primary component file (e.g., `src/components/Settings.tsx`) + - "this page" → the URL or the page's source file + Prefer the source file path over the dev-server URL when both exist; ports drift between runs (`bun dev` vs `bun preview`), file paths don't. + +2. **Compute the slug.** Run: + ```bash + node .kiro/skills/impeccable/scripts/critique-storage.mjs slug "" + ``` + Keep the printed slug. It identifies this target's stream across runs. If the command exits non-zero ("no stable slug for input"), skip persistence for this run and tell the user; the trend won't update but the critique still goes ahead. + +3. **Read the ignore list** at `.impeccable/critique/ignore.md` if it exists. Plain markdown; each non-empty, non-comment line is something the user has marked as "do not re-raise" (deferred tradeoffs, designer-intended deviations, detector false-positives the user accepts). When a finding's text matches a line here (case-insensitive substring against rule name or snippet), **drop it silently**. Do not mention it in the report. This is the ONLY input critique consumes from prior runs; anchoring on prior findings would defeat the point of independent assessment. + ### Gather Assessments Launch two independent assessments. **Neither may see the other's output.** This isolation is what makes the combined score honest. Running both in one head silently anchors them to each other; do not shortcut it for cost, speed, or context-size reasons. @@ -164,6 +182,36 @@ Provocative questions that might unlock better solutions: - Prioritize ruthlessly. If everything is important, nothing is. - Don't soften criticism. Developers need honest feedback to ship great design. +### Persist the Snapshot + +Once the report above is finalized, write it to `.impeccable/critique/` so the user can refer back, and so `/impeccable polish` can pick up the priority issues without a copy-paste. + +Skip this step if the Setup slug was null (vague or root-level target). + +1. **Write the body to a temp file** so you can pipe it to the helper. Use the full report (heuristic table, anti-patterns verdict, priority issues, persona red flags) but stop before the "Ask the User" / "Recommended Actions" sections that come later. + +2. **Pass the structured metadata** through `IMPECCABLE_CRITIQUE_META` (JSON), then run the write command: + ```bash + IMPECCABLE_CRITIQUE_META='{"target":"","total_score":,"p0_count":,"p1_count":}' \ + node .kiro/skills/impeccable/scripts/critique-storage.mjs write + ``` + The helper prints the absolute path it wrote. + +3. **Read the trend** for context: + ```bash + node .kiro/skills/impeccable/scripts/critique-storage.mjs trend 5 + ``` + This returns a JSON array of the last 5 frontmatter entries (including the one you just wrote). + +4. **Append a single line to the user-visible output**, after the report and before the questions: + + > **Trend for `` (last 5 runs): 24 → 28 → 32 → 29 → 32** + > Wrote `.impeccable/critique/`. + + If this is the first run for the slug, the trend is just one score; say so: "First run for this target, no trend yet." + +This is fire-and-forget. Do not show the user the helper's JSON output; only the human-readable trend line and the written path. Failures here should not block the rest of the flow; print the error and move on. + ### Ask the User **After presenting findings**, use targeted questions based on what was actually found. ask the user directly to clarify what you cannot infer. These answers will shape the action plan. diff --git a/.kiro/skills/impeccable/reference/polish.md b/.kiro/skills/impeccable/reference/polish.md index eed3dbd27..e20e1467c 100644 --- a/.kiro/skills/impeccable/reference/polish.md +++ b/.kiro/skills/impeccable/reference/polish.md @@ -35,7 +35,14 @@ Understand the current state and goals before touching anything: - Loading and transition smoothness - Information architecture and flow drift (does this feature reveal complexity the way neighboring features do?) -4. **Triage cosmetic vs functional**: Classify each issue as **cosmetic** (looks off, doesn't impede the user) or **functional** (breaks, blocks, or confuses the experience). When polish time is tight, functional issues ship first; cosmetic ones can land in a follow-up. Quality should be consistent; never perfect one corner while leaving another rough. +4. **Pull in any prior critique** (optional signal): If `/impeccable critique` has been run on the same target, its priority issues are a useful prior for what to address first. Resolve the target to a file path or URL, then: + ```bash + slug=$(node .kiro/skills/impeccable/scripts/critique-storage.mjs slug "") + node .kiro/skills/impeccable/scripts/critique-storage.mjs latest "$slug" + ``` + Exit 0 with body = found; fold the P0/P1 items into your polish list and mention the snapshot path so the user sees what you read. Exit 2 = no snapshot, continue without it. The critique is one input among many. Do your own pass either way. + +5. **Triage cosmetic vs functional**: Classify each issue as **cosmetic** (looks off, doesn't impede the user) or **functional** (breaks, blocks, or confuses the experience). When polish time is tight, functional issues ship first; cosmetic ones can land in a follow-up. Quality should be consistent; never perfect one corner while leaving another rough. **CRITICAL**: Polish is the last step, not the first. Don't polish work that's not functionally complete. diff --git a/.kiro/skills/impeccable/scripts/critique-storage.mjs b/.kiro/skills/impeccable/scripts/critique-storage.mjs new file mode 100644 index 000000000..fa7bdf385 --- /dev/null +++ b/.kiro/skills/impeccable/scripts/critique-storage.mjs @@ -0,0 +1,226 @@ +#!/usr/bin/env node +/** + * Critique persistence helper. + * + * Each run of /impeccable critique writes a per-target snapshot to + * .impeccable/critique/__.md + * with a small YAML frontmatter carrying the score + P0/P1 counts. + * + * /impeccable polish reads the latest matching snapshot at start as its + * fix backlog. No other skill auto-reads critique output. + * + * The slug is derived mechanically from the *resolved* primary artifact + * (file path or URL), never from the user's natural-language phrasing. + * Slug stability across runs is what lets the trend display work. + * + * CLI entry points (called from skill instructions): + * node critique-storage.mjs slug + * node critique-storage.mjs write + * node critique-storage.mjs latest + * node critique-storage.mjs trend [limit] + * + * Note: there is intentionally no `ignore` subcommand. ignore.md is a plain + * markdown file; the model reads it directly with its file-read tool. This + * helper only exists for operations the model can't trivially do inline + * (normalizing paths, generating filenames, globbing + parsing frontmatter). + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import { getCritiqueDir } from './impeccable-paths.mjs'; + +const SLUG_MAX = 50; + +/** + * Mechanically derive a slug from a resolved target. Returns null if the + * input doesn't look like a stable identifier (empty, project root, etc). + * + * Accepts file paths and URLs. The model resolves "the homepage" to a + * concrete artifact before calling this — we never slug a natural-language + * phrase. + */ +export function slugFromTarget(resolved, { cwd = process.cwd() } = {}) { + if (!resolved || typeof resolved !== 'string') return null; + const trimmed = resolved.trim(); + if (!trimmed) return null; + + // URL + if (/^https?:\/\//i.test(trimmed)) { + let url; + try { url = new URL(trimmed); } catch { return null; } + const hostPath = `${url.hostname}${url.pathname}`; + return kebab(hostPath); + } + + // File path. Make it project-relative so two devs critiquing the same + // checkout get the same slug regardless of where their repo is cloned. + const abs = path.isAbsolute(trimmed) ? trimmed : path.resolve(cwd, trimmed); + let rel = path.relative(cwd, abs); + // If the target is outside cwd, fall back to the basename so we still + // produce a stable slug (vs the absolute path, which would include + // home dirs / usernames). + if (rel.startsWith('..') || path.isAbsolute(rel)) { + rel = path.basename(abs); + } + if (!rel || rel === '.' || rel === '') return null; + return kebab(rel); +} + +function kebab(s) { + const slug = s + .toLowerCase() + .replace(/[/\\.]+/g, '-') + .replace(/[^a-z0-9-]+/g, '-') + .replace(/-+/g, '-') + .replace(/^-|-$/g, ''); + if (!slug) return null; + // Cap from the tail — the tail (filename) is more identifying than the + // top-level directory. + return slug.length <= SLUG_MAX ? slug : slug.slice(slug.length - SLUG_MAX).replace(/^-/, ''); +} + +/** + * Filename-safe UTC ISO timestamp: hyphens for separators, trailing Z. + * Plain colons aren't allowed on Windows filesystems. + */ +export function nowFilenameStamp(date = new Date()) { + const iso = date.toISOString(); // 2026-05-12T18:30:00.123Z + return iso.replace(/[:.]/g, '-').replace(/-\d+Z$/, 'Z'); +} + +/** + * Write a snapshot for `slug`. `meta` carries the small structured frontmatter + * keys read back by readTrend(). `body` is the human-readable critique + * report (everything below the frontmatter). + * + * Returns the absolute path written. + */ +export function writeSnapshot({ slug, meta, body, cwd = process.cwd(), now = new Date() }) { + if (!slug) throw new Error('writeSnapshot requires a slug'); + const dir = getCritiqueDir(cwd); + fs.mkdirSync(dir, { recursive: true }); + const timestamp = nowFilenameStamp(now); + const filePath = path.join(dir, `${timestamp}__${slug}.md`); + // Spread `meta` first so internally computed `timestamp` and `slug` + // always win. Otherwise a caller-supplied meta blob (parsed from the + // IMPECCABLE_CRITIQUE_META env var) could clobber them, leaving the + // filename in disagreement with its frontmatter and corrupting trends. + const front = serializeFrontmatter({ ...meta, timestamp, slug }); + fs.writeFileSync(filePath, `${front}\n${body.trim()}\n`, 'utf-8'); + return filePath; +} + +function serializeFrontmatter(obj) { + const lines = ['---']; + for (const [key, value] of Object.entries(obj)) { + if (value === undefined || value === null) continue; + const str = typeof value === 'string' ? value : String(value); + // Quote strings that contain : or # to keep parsing simple. + const needsQuotes = typeof value === 'string' && /[:#]/.test(str); + lines.push(`${key}: ${needsQuotes ? JSON.stringify(str) : str}`); + } + lines.push('---'); + return lines.join('\n'); +} + +function parseFrontmatter(text) { + const match = text.match(/^---\r?\n([\s\S]*?)\r?\n---/); + if (!match) return {}; + const out = {}; + for (const line of match[1].split(/\r?\n/)) { + const colon = line.indexOf(':'); + if (colon < 0) continue; + const key = line.slice(0, colon).trim(); + let value = line.slice(colon + 1).trim(); + if (/^".*"$/.test(value)) { + try { value = JSON.parse(value); } catch { /* leave as-is */ } + } else if (/^-?\d+$/.test(value)) { + value = Number(value); + } + out[key] = value; + } + return out; +} + +/** + * Return all snapshot files for `slug`, sorted oldest → newest. + */ +function listSnapshotsForSlug(slug, cwd) { + const dir = getCritiqueDir(cwd); + if (!fs.existsSync(dir)) return []; + const suffix = `__${slug}.md`; + return fs.readdirSync(dir) + .filter((f) => f.endsWith(suffix)) + .sort() + .map((f) => path.join(dir, f)); +} + +/** + * 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 the last `limit` snapshots' frontmatter, oldest → newest. + * 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 slice = all.slice(-limit); + return slice.map((file) => parseFrontmatter(fs.readFileSync(file, 'utf-8'))); +} + +// ---- CLI --------------------------------------------------------------- + +function main(argv) { + const [cmd, ...args] = argv; + switch (cmd) { + case 'slug': { + const slug = slugFromTarget(args[0]); + if (!slug) { process.stderr.write('no stable slug for input\n'); process.exit(1); } + process.stdout.write(`${slug}\n`); + return; + } + case 'write': { + const [slug, bodyFile] = args; + if (!slug || !bodyFile) { process.stderr.write('usage: write \n'); process.exit(1); } + const raw = fs.readFileSync(bodyFile, 'utf-8'); + // The body file may be a full report. The caller passes the meta as + // a JSON object on stdin if it wants structured frontmatter; otherwise + // we write with minimal metadata. + let meta = {}; + const metaArg = process.env.IMPECCABLE_CRITIQUE_META; + if (metaArg) { + try { meta = JSON.parse(metaArg); } catch { /* ignore */ } + } + const out = writeSnapshot({ slug, meta, body: raw }); + process.stdout.write(`${out}\n`); + return; + } + case 'latest': { + const latest = readLatestSnapshot(args[0]); + if (!latest) { process.exit(2); } + process.stdout.write(latest.body); + return; + } + case 'trend': { + const rows = readTrend(args[0], { limit: args[1] ? Number(args[1]) : 5 }); + process.stdout.write(JSON.stringify(rows, null, 2) + '\n'); + return; + } + default: + process.stderr.write('usage: critique-storage.mjs [args]\n'); + process.exit(1); + } +} + +if (import.meta.url === `file://${process.argv[1]}`) { + main(process.argv.slice(2)); +} diff --git a/.kiro/skills/impeccable/scripts/impeccable-paths.mjs b/.kiro/skills/impeccable/scripts/impeccable-paths.mjs index ba852bae9..6befa9cd3 100644 --- a/.kiro/skills/impeccable/scripts/impeccable-paths.mjs +++ b/.kiro/skills/impeccable/scripts/impeccable-paths.mjs @@ -3,6 +3,7 @@ import path from 'node:path'; export const IMPECCABLE_DIR = '.impeccable'; export const LIVE_DIR = 'live'; +export const CRITIQUE_DIR = 'critique'; export function getImpeccableDir(cwd = process.cwd()) { return path.join(cwd, IMPECCABLE_DIR); @@ -96,6 +97,10 @@ export function getLiveAnnotationsDir(cwd = process.cwd()) { return path.join(getLiveDir(cwd), 'annotations'); } +export function getCritiqueDir(cwd = process.cwd()) { + return path.join(getImpeccableDir(cwd), CRITIQUE_DIR); +} + export function getLegacyLiveAnnotationsDir(cwd = process.cwd()) { return path.join(cwd, '.impeccable-live', 'annotations'); } diff --git a/.opencode/skills/impeccable/reference/critique.md b/.opencode/skills/impeccable/reference/critique.md index 3a8d39c41..ffb00522b 100644 --- a/.opencode/skills/impeccable/reference/critique.md +++ b/.opencode/skills/impeccable/reference/critique.md @@ -1,5 +1,23 @@ > **Additional context needed**: what the interface is trying to accomplish. +### Setup: Resolve Target and Load Ignore List + +Before gathering assessments, do two small bookkeeping steps. They cost almost nothing and they're what makes critique iterative across runs. + +1. **Resolve the primary artifact.** The user's phrasing ("the homepage", "the pricing flow") is not stable enough to track across runs. Resolve it to a concrete file path or URL: the same one you'd already need to scan code or open in a browser. Examples: + - "the homepage" → `site/pages/index.astro` (or `http://localhost:3000/` if you're inspecting live) + - "the settings modal" → the primary component file (e.g., `src/components/Settings.tsx`) + - "this page" → the URL or the page's source file + Prefer the source file path over the dev-server URL when both exist; ports drift between runs (`bun dev` vs `bun preview`), file paths don't. + +2. **Compute the slug.** Run: + ```bash + node .opencode/skills/impeccable/scripts/critique-storage.mjs slug "" + ``` + Keep the printed slug. It identifies this target's stream across runs. If the command exits non-zero ("no stable slug for input"), skip persistence for this run and tell the user; the trend won't update but the critique still goes ahead. + +3. **Read the ignore list** at `.impeccable/critique/ignore.md` if it exists. Plain markdown; each non-empty, non-comment line is something the user has marked as "do not re-raise" (deferred tradeoffs, designer-intended deviations, detector false-positives the user accepts). When a finding's text matches a line here (case-insensitive substring against rule name or snippet), **drop it silently**. Do not mention it in the report. This is the ONLY input critique consumes from prior runs; anchoring on prior findings would defeat the point of independent assessment. + ### Gather Assessments Launch two independent assessments. **Neither may see the other's output.** This isolation is what makes the combined score honest. Running both in one head silently anchors them to each other; do not shortcut it for cost, speed, or context-size reasons. @@ -164,6 +182,36 @@ Provocative questions that might unlock better solutions: - Prioritize ruthlessly. If everything is important, nothing is. - Don't soften criticism. Developers need honest feedback to ship great design. +### Persist the Snapshot + +Once the report above is finalized, write it to `.impeccable/critique/` so the user can refer back, and so `/impeccable polish` can pick up the priority issues without a copy-paste. + +Skip this step if the Setup slug was null (vague or root-level target). + +1. **Write the body to a temp file** so you can pipe it to the helper. Use the full report (heuristic table, anti-patterns verdict, priority issues, persona red flags) but stop before the "Ask the User" / "Recommended Actions" sections that come later. + +2. **Pass the structured metadata** through `IMPECCABLE_CRITIQUE_META` (JSON), then run the write command: + ```bash + IMPECCABLE_CRITIQUE_META='{"target":"","total_score":,"p0_count":,"p1_count":}' \ + node .opencode/skills/impeccable/scripts/critique-storage.mjs write + ``` + The helper prints the absolute path it wrote. + +3. **Read the trend** for context: + ```bash + node .opencode/skills/impeccable/scripts/critique-storage.mjs trend 5 + ``` + This returns a JSON array of the last 5 frontmatter entries (including the one you just wrote). + +4. **Append a single line to the user-visible output**, after the report and before the questions: + + > **Trend for `` (last 5 runs): 24 → 28 → 32 → 29 → 32** + > Wrote `.impeccable/critique/`. + + If this is the first run for the slug, the trend is just one score; say so: "First run for this target, no trend yet." + +This is fire-and-forget. Do not show the user the helper's JSON output; only the human-readable trend line and the written path. Failures here should not block the rest of the flow; print the error and move on. + ### Ask the User **After presenting findings**, use targeted questions based on what was actually found. STOP and call the `question` tool to clarify. These answers will shape the action plan. diff --git a/.opencode/skills/impeccable/reference/polish.md b/.opencode/skills/impeccable/reference/polish.md index eed3dbd27..1d940a30f 100644 --- a/.opencode/skills/impeccable/reference/polish.md +++ b/.opencode/skills/impeccable/reference/polish.md @@ -35,7 +35,14 @@ Understand the current state and goals before touching anything: - Loading and transition smoothness - Information architecture and flow drift (does this feature reveal complexity the way neighboring features do?) -4. **Triage cosmetic vs functional**: Classify each issue as **cosmetic** (looks off, doesn't impede the user) or **functional** (breaks, blocks, or confuses the experience). When polish time is tight, functional issues ship first; cosmetic ones can land in a follow-up. Quality should be consistent; never perfect one corner while leaving another rough. +4. **Pull in any prior critique** (optional signal): If `/impeccable critique` has been run on the same target, its priority issues are a useful prior for what to address first. Resolve the target to a file path or URL, then: + ```bash + slug=$(node .opencode/skills/impeccable/scripts/critique-storage.mjs slug "") + node .opencode/skills/impeccable/scripts/critique-storage.mjs latest "$slug" + ``` + Exit 0 with body = found; fold the P0/P1 items into your polish list and mention the snapshot path so the user sees what you read. Exit 2 = no snapshot, continue without it. The critique is one input among many. Do your own pass either way. + +5. **Triage cosmetic vs functional**: Classify each issue as **cosmetic** (looks off, doesn't impede the user) or **functional** (breaks, blocks, or confuses the experience). When polish time is tight, functional issues ship first; cosmetic ones can land in a follow-up. Quality should be consistent; never perfect one corner while leaving another rough. **CRITICAL**: Polish is the last step, not the first. Don't polish work that's not functionally complete. diff --git a/.opencode/skills/impeccable/scripts/critique-storage.mjs b/.opencode/skills/impeccable/scripts/critique-storage.mjs new file mode 100644 index 000000000..fa7bdf385 --- /dev/null +++ b/.opencode/skills/impeccable/scripts/critique-storage.mjs @@ -0,0 +1,226 @@ +#!/usr/bin/env node +/** + * Critique persistence helper. + * + * Each run of /impeccable critique writes a per-target snapshot to + * .impeccable/critique/__.md + * with a small YAML frontmatter carrying the score + P0/P1 counts. + * + * /impeccable polish reads the latest matching snapshot at start as its + * fix backlog. No other skill auto-reads critique output. + * + * The slug is derived mechanically from the *resolved* primary artifact + * (file path or URL), never from the user's natural-language phrasing. + * Slug stability across runs is what lets the trend display work. + * + * CLI entry points (called from skill instructions): + * node critique-storage.mjs slug + * node critique-storage.mjs write + * node critique-storage.mjs latest + * node critique-storage.mjs trend [limit] + * + * Note: there is intentionally no `ignore` subcommand. ignore.md is a plain + * markdown file; the model reads it directly with its file-read tool. This + * helper only exists for operations the model can't trivially do inline + * (normalizing paths, generating filenames, globbing + parsing frontmatter). + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import { getCritiqueDir } from './impeccable-paths.mjs'; + +const SLUG_MAX = 50; + +/** + * Mechanically derive a slug from a resolved target. Returns null if the + * input doesn't look like a stable identifier (empty, project root, etc). + * + * Accepts file paths and URLs. The model resolves "the homepage" to a + * concrete artifact before calling this — we never slug a natural-language + * phrase. + */ +export function slugFromTarget(resolved, { cwd = process.cwd() } = {}) { + if (!resolved || typeof resolved !== 'string') return null; + const trimmed = resolved.trim(); + if (!trimmed) return null; + + // URL + if (/^https?:\/\//i.test(trimmed)) { + let url; + try { url = new URL(trimmed); } catch { return null; } + const hostPath = `${url.hostname}${url.pathname}`; + return kebab(hostPath); + } + + // File path. Make it project-relative so two devs critiquing the same + // checkout get the same slug regardless of where their repo is cloned. + const abs = path.isAbsolute(trimmed) ? trimmed : path.resolve(cwd, trimmed); + let rel = path.relative(cwd, abs); + // If the target is outside cwd, fall back to the basename so we still + // produce a stable slug (vs the absolute path, which would include + // home dirs / usernames). + if (rel.startsWith('..') || path.isAbsolute(rel)) { + rel = path.basename(abs); + } + if (!rel || rel === '.' || rel === '') return null; + return kebab(rel); +} + +function kebab(s) { + const slug = s + .toLowerCase() + .replace(/[/\\.]+/g, '-') + .replace(/[^a-z0-9-]+/g, '-') + .replace(/-+/g, '-') + .replace(/^-|-$/g, ''); + if (!slug) return null; + // Cap from the tail — the tail (filename) is more identifying than the + // top-level directory. + return slug.length <= SLUG_MAX ? slug : slug.slice(slug.length - SLUG_MAX).replace(/^-/, ''); +} + +/** + * Filename-safe UTC ISO timestamp: hyphens for separators, trailing Z. + * Plain colons aren't allowed on Windows filesystems. + */ +export function nowFilenameStamp(date = new Date()) { + const iso = date.toISOString(); // 2026-05-12T18:30:00.123Z + return iso.replace(/[:.]/g, '-').replace(/-\d+Z$/, 'Z'); +} + +/** + * Write a snapshot for `slug`. `meta` carries the small structured frontmatter + * keys read back by readTrend(). `body` is the human-readable critique + * report (everything below the frontmatter). + * + * Returns the absolute path written. + */ +export function writeSnapshot({ slug, meta, body, cwd = process.cwd(), now = new Date() }) { + if (!slug) throw new Error('writeSnapshot requires a slug'); + const dir = getCritiqueDir(cwd); + fs.mkdirSync(dir, { recursive: true }); + const timestamp = nowFilenameStamp(now); + const filePath = path.join(dir, `${timestamp}__${slug}.md`); + // Spread `meta` first so internally computed `timestamp` and `slug` + // always win. Otherwise a caller-supplied meta blob (parsed from the + // IMPECCABLE_CRITIQUE_META env var) could clobber them, leaving the + // filename in disagreement with its frontmatter and corrupting trends. + const front = serializeFrontmatter({ ...meta, timestamp, slug }); + fs.writeFileSync(filePath, `${front}\n${body.trim()}\n`, 'utf-8'); + return filePath; +} + +function serializeFrontmatter(obj) { + const lines = ['---']; + for (const [key, value] of Object.entries(obj)) { + if (value === undefined || value === null) continue; + const str = typeof value === 'string' ? value : String(value); + // Quote strings that contain : or # to keep parsing simple. + const needsQuotes = typeof value === 'string' && /[:#]/.test(str); + lines.push(`${key}: ${needsQuotes ? JSON.stringify(str) : str}`); + } + lines.push('---'); + return lines.join('\n'); +} + +function parseFrontmatter(text) { + const match = text.match(/^---\r?\n([\s\S]*?)\r?\n---/); + if (!match) return {}; + const out = {}; + for (const line of match[1].split(/\r?\n/)) { + const colon = line.indexOf(':'); + if (colon < 0) continue; + const key = line.slice(0, colon).trim(); + let value = line.slice(colon + 1).trim(); + if (/^".*"$/.test(value)) { + try { value = JSON.parse(value); } catch { /* leave as-is */ } + } else if (/^-?\d+$/.test(value)) { + value = Number(value); + } + out[key] = value; + } + return out; +} + +/** + * Return all snapshot files for `slug`, sorted oldest → newest. + */ +function listSnapshotsForSlug(slug, cwd) { + const dir = getCritiqueDir(cwd); + if (!fs.existsSync(dir)) return []; + const suffix = `__${slug}.md`; + return fs.readdirSync(dir) + .filter((f) => f.endsWith(suffix)) + .sort() + .map((f) => path.join(dir, f)); +} + +/** + * 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 the last `limit` snapshots' frontmatter, oldest → newest. + * 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 slice = all.slice(-limit); + return slice.map((file) => parseFrontmatter(fs.readFileSync(file, 'utf-8'))); +} + +// ---- CLI --------------------------------------------------------------- + +function main(argv) { + const [cmd, ...args] = argv; + switch (cmd) { + case 'slug': { + const slug = slugFromTarget(args[0]); + if (!slug) { process.stderr.write('no stable slug for input\n'); process.exit(1); } + process.stdout.write(`${slug}\n`); + return; + } + case 'write': { + const [slug, bodyFile] = args; + if (!slug || !bodyFile) { process.stderr.write('usage: write \n'); process.exit(1); } + const raw = fs.readFileSync(bodyFile, 'utf-8'); + // The body file may be a full report. The caller passes the meta as + // a JSON object on stdin if it wants structured frontmatter; otherwise + // we write with minimal metadata. + let meta = {}; + const metaArg = process.env.IMPECCABLE_CRITIQUE_META; + if (metaArg) { + try { meta = JSON.parse(metaArg); } catch { /* ignore */ } + } + const out = writeSnapshot({ slug, meta, body: raw }); + process.stdout.write(`${out}\n`); + return; + } + case 'latest': { + const latest = readLatestSnapshot(args[0]); + if (!latest) { process.exit(2); } + process.stdout.write(latest.body); + return; + } + case 'trend': { + const rows = readTrend(args[0], { limit: args[1] ? Number(args[1]) : 5 }); + process.stdout.write(JSON.stringify(rows, null, 2) + '\n'); + return; + } + default: + process.stderr.write('usage: critique-storage.mjs [args]\n'); + process.exit(1); + } +} + +if (import.meta.url === `file://${process.argv[1]}`) { + main(process.argv.slice(2)); +} diff --git a/.opencode/skills/impeccable/scripts/impeccable-paths.mjs b/.opencode/skills/impeccable/scripts/impeccable-paths.mjs index ba852bae9..6befa9cd3 100644 --- a/.opencode/skills/impeccable/scripts/impeccable-paths.mjs +++ b/.opencode/skills/impeccable/scripts/impeccable-paths.mjs @@ -3,6 +3,7 @@ import path from 'node:path'; export const IMPECCABLE_DIR = '.impeccable'; export const LIVE_DIR = 'live'; +export const CRITIQUE_DIR = 'critique'; export function getImpeccableDir(cwd = process.cwd()) { return path.join(cwd, IMPECCABLE_DIR); @@ -96,6 +97,10 @@ export function getLiveAnnotationsDir(cwd = process.cwd()) { return path.join(getLiveDir(cwd), 'annotations'); } +export function getCritiqueDir(cwd = process.cwd()) { + return path.join(getImpeccableDir(cwd), CRITIQUE_DIR); +} + export function getLegacyLiveAnnotationsDir(cwd = process.cwd()) { return path.join(cwd, '.impeccable-live', 'annotations'); } diff --git a/.pi/skills/impeccable/reference/critique.md b/.pi/skills/impeccable/reference/critique.md index d5563b0d3..5a0bd36d1 100644 --- a/.pi/skills/impeccable/reference/critique.md +++ b/.pi/skills/impeccable/reference/critique.md @@ -1,5 +1,23 @@ > **Additional context needed**: what the interface is trying to accomplish. +### Setup: Resolve Target and Load Ignore List + +Before gathering assessments, do two small bookkeeping steps. They cost almost nothing and they're what makes critique iterative across runs. + +1. **Resolve the primary artifact.** The user's phrasing ("the homepage", "the pricing flow") is not stable enough to track across runs. Resolve it to a concrete file path or URL: the same one you'd already need to scan code or open in a browser. Examples: + - "the homepage" → `site/pages/index.astro` (or `http://localhost:3000/` if you're inspecting live) + - "the settings modal" → the primary component file (e.g., `src/components/Settings.tsx`) + - "this page" → the URL or the page's source file + Prefer the source file path over the dev-server URL when both exist; ports drift between runs (`bun dev` vs `bun preview`), file paths don't. + +2. **Compute the slug.** Run: + ```bash + node .pi/skills/impeccable/scripts/critique-storage.mjs slug "" + ``` + Keep the printed slug. It identifies this target's stream across runs. If the command exits non-zero ("no stable slug for input"), skip persistence for this run and tell the user; the trend won't update but the critique still goes ahead. + +3. **Read the ignore list** at `.impeccable/critique/ignore.md` if it exists. Plain markdown; each non-empty, non-comment line is something the user has marked as "do not re-raise" (deferred tradeoffs, designer-intended deviations, detector false-positives the user accepts). When a finding's text matches a line here (case-insensitive substring against rule name or snippet), **drop it silently**. Do not mention it in the report. This is the ONLY input critique consumes from prior runs; anchoring on prior findings would defeat the point of independent assessment. + ### Gather Assessments Launch two independent assessments. **Neither may see the other's output.** This isolation is what makes the combined score honest. Running both in one head silently anchors them to each other; do not shortcut it for cost, speed, or context-size reasons. @@ -164,6 +182,36 @@ Provocative questions that might unlock better solutions: - Prioritize ruthlessly. If everything is important, nothing is. - Don't soften criticism. Developers need honest feedback to ship great design. +### Persist the Snapshot + +Once the report above is finalized, write it to `.impeccable/critique/` so the user can refer back, and so `/impeccable polish` can pick up the priority issues without a copy-paste. + +Skip this step if the Setup slug was null (vague or root-level target). + +1. **Write the body to a temp file** so you can pipe it to the helper. Use the full report (heuristic table, anti-patterns verdict, priority issues, persona red flags) but stop before the "Ask the User" / "Recommended Actions" sections that come later. + +2. **Pass the structured metadata** through `IMPECCABLE_CRITIQUE_META` (JSON), then run the write command: + ```bash + IMPECCABLE_CRITIQUE_META='{"target":"","total_score":,"p0_count":,"p1_count":}' \ + node .pi/skills/impeccable/scripts/critique-storage.mjs write + ``` + The helper prints the absolute path it wrote. + +3. **Read the trend** for context: + ```bash + node .pi/skills/impeccable/scripts/critique-storage.mjs trend 5 + ``` + This returns a JSON array of the last 5 frontmatter entries (including the one you just wrote). + +4. **Append a single line to the user-visible output**, after the report and before the questions: + + > **Trend for `` (last 5 runs): 24 → 28 → 32 → 29 → 32** + > Wrote `.impeccable/critique/`. + + If this is the first run for the slug, the trend is just one score; say so: "First run for this target, no trend yet." + +This is fire-and-forget. Do not show the user the helper's JSON output; only the human-readable trend line and the written path. Failures here should not block the rest of the flow; print the error and move on. + ### Ask the User **After presenting findings**, use targeted questions based on what was actually found. ask the user directly to clarify what you cannot infer. These answers will shape the action plan. diff --git a/.pi/skills/impeccable/reference/polish.md b/.pi/skills/impeccable/reference/polish.md index eed3dbd27..63a0f4bb0 100644 --- a/.pi/skills/impeccable/reference/polish.md +++ b/.pi/skills/impeccable/reference/polish.md @@ -35,7 +35,14 @@ Understand the current state and goals before touching anything: - Loading and transition smoothness - Information architecture and flow drift (does this feature reveal complexity the way neighboring features do?) -4. **Triage cosmetic vs functional**: Classify each issue as **cosmetic** (looks off, doesn't impede the user) or **functional** (breaks, blocks, or confuses the experience). When polish time is tight, functional issues ship first; cosmetic ones can land in a follow-up. Quality should be consistent; never perfect one corner while leaving another rough. +4. **Pull in any prior critique** (optional signal): If `/impeccable critique` has been run on the same target, its priority issues are a useful prior for what to address first. Resolve the target to a file path or URL, then: + ```bash + slug=$(node .pi/skills/impeccable/scripts/critique-storage.mjs slug "") + node .pi/skills/impeccable/scripts/critique-storage.mjs latest "$slug" + ``` + Exit 0 with body = found; fold the P0/P1 items into your polish list and mention the snapshot path so the user sees what you read. Exit 2 = no snapshot, continue without it. The critique is one input among many. Do your own pass either way. + +5. **Triage cosmetic vs functional**: Classify each issue as **cosmetic** (looks off, doesn't impede the user) or **functional** (breaks, blocks, or confuses the experience). When polish time is tight, functional issues ship first; cosmetic ones can land in a follow-up. Quality should be consistent; never perfect one corner while leaving another rough. **CRITICAL**: Polish is the last step, not the first. Don't polish work that's not functionally complete. diff --git a/.pi/skills/impeccable/scripts/critique-storage.mjs b/.pi/skills/impeccable/scripts/critique-storage.mjs new file mode 100644 index 000000000..fa7bdf385 --- /dev/null +++ b/.pi/skills/impeccable/scripts/critique-storage.mjs @@ -0,0 +1,226 @@ +#!/usr/bin/env node +/** + * Critique persistence helper. + * + * Each run of /impeccable critique writes a per-target snapshot to + * .impeccable/critique/__.md + * with a small YAML frontmatter carrying the score + P0/P1 counts. + * + * /impeccable polish reads the latest matching snapshot at start as its + * fix backlog. No other skill auto-reads critique output. + * + * The slug is derived mechanically from the *resolved* primary artifact + * (file path or URL), never from the user's natural-language phrasing. + * Slug stability across runs is what lets the trend display work. + * + * CLI entry points (called from skill instructions): + * node critique-storage.mjs slug + * node critique-storage.mjs write + * node critique-storage.mjs latest + * node critique-storage.mjs trend [limit] + * + * Note: there is intentionally no `ignore` subcommand. ignore.md is a plain + * markdown file; the model reads it directly with its file-read tool. This + * helper only exists for operations the model can't trivially do inline + * (normalizing paths, generating filenames, globbing + parsing frontmatter). + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import { getCritiqueDir } from './impeccable-paths.mjs'; + +const SLUG_MAX = 50; + +/** + * Mechanically derive a slug from a resolved target. Returns null if the + * input doesn't look like a stable identifier (empty, project root, etc). + * + * Accepts file paths and URLs. The model resolves "the homepage" to a + * concrete artifact before calling this — we never slug a natural-language + * phrase. + */ +export function slugFromTarget(resolved, { cwd = process.cwd() } = {}) { + if (!resolved || typeof resolved !== 'string') return null; + const trimmed = resolved.trim(); + if (!trimmed) return null; + + // URL + if (/^https?:\/\//i.test(trimmed)) { + let url; + try { url = new URL(trimmed); } catch { return null; } + const hostPath = `${url.hostname}${url.pathname}`; + return kebab(hostPath); + } + + // File path. Make it project-relative so two devs critiquing the same + // checkout get the same slug regardless of where their repo is cloned. + const abs = path.isAbsolute(trimmed) ? trimmed : path.resolve(cwd, trimmed); + let rel = path.relative(cwd, abs); + // If the target is outside cwd, fall back to the basename so we still + // produce a stable slug (vs the absolute path, which would include + // home dirs / usernames). + if (rel.startsWith('..') || path.isAbsolute(rel)) { + rel = path.basename(abs); + } + if (!rel || rel === '.' || rel === '') return null; + return kebab(rel); +} + +function kebab(s) { + const slug = s + .toLowerCase() + .replace(/[/\\.]+/g, '-') + .replace(/[^a-z0-9-]+/g, '-') + .replace(/-+/g, '-') + .replace(/^-|-$/g, ''); + if (!slug) return null; + // Cap from the tail — the tail (filename) is more identifying than the + // top-level directory. + return slug.length <= SLUG_MAX ? slug : slug.slice(slug.length - SLUG_MAX).replace(/^-/, ''); +} + +/** + * Filename-safe UTC ISO timestamp: hyphens for separators, trailing Z. + * Plain colons aren't allowed on Windows filesystems. + */ +export function nowFilenameStamp(date = new Date()) { + const iso = date.toISOString(); // 2026-05-12T18:30:00.123Z + return iso.replace(/[:.]/g, '-').replace(/-\d+Z$/, 'Z'); +} + +/** + * Write a snapshot for `slug`. `meta` carries the small structured frontmatter + * keys read back by readTrend(). `body` is the human-readable critique + * report (everything below the frontmatter). + * + * Returns the absolute path written. + */ +export function writeSnapshot({ slug, meta, body, cwd = process.cwd(), now = new Date() }) { + if (!slug) throw new Error('writeSnapshot requires a slug'); + const dir = getCritiqueDir(cwd); + fs.mkdirSync(dir, { recursive: true }); + const timestamp = nowFilenameStamp(now); + const filePath = path.join(dir, `${timestamp}__${slug}.md`); + // Spread `meta` first so internally computed `timestamp` and `slug` + // always win. Otherwise a caller-supplied meta blob (parsed from the + // IMPECCABLE_CRITIQUE_META env var) could clobber them, leaving the + // filename in disagreement with its frontmatter and corrupting trends. + const front = serializeFrontmatter({ ...meta, timestamp, slug }); + fs.writeFileSync(filePath, `${front}\n${body.trim()}\n`, 'utf-8'); + return filePath; +} + +function serializeFrontmatter(obj) { + const lines = ['---']; + for (const [key, value] of Object.entries(obj)) { + if (value === undefined || value === null) continue; + const str = typeof value === 'string' ? value : String(value); + // Quote strings that contain : or # to keep parsing simple. + const needsQuotes = typeof value === 'string' && /[:#]/.test(str); + lines.push(`${key}: ${needsQuotes ? JSON.stringify(str) : str}`); + } + lines.push('---'); + return lines.join('\n'); +} + +function parseFrontmatter(text) { + const match = text.match(/^---\r?\n([\s\S]*?)\r?\n---/); + if (!match) return {}; + const out = {}; + for (const line of match[1].split(/\r?\n/)) { + const colon = line.indexOf(':'); + if (colon < 0) continue; + const key = line.slice(0, colon).trim(); + let value = line.slice(colon + 1).trim(); + if (/^".*"$/.test(value)) { + try { value = JSON.parse(value); } catch { /* leave as-is */ } + } else if (/^-?\d+$/.test(value)) { + value = Number(value); + } + out[key] = value; + } + return out; +} + +/** + * Return all snapshot files for `slug`, sorted oldest → newest. + */ +function listSnapshotsForSlug(slug, cwd) { + const dir = getCritiqueDir(cwd); + if (!fs.existsSync(dir)) return []; + const suffix = `__${slug}.md`; + return fs.readdirSync(dir) + .filter((f) => f.endsWith(suffix)) + .sort() + .map((f) => path.join(dir, f)); +} + +/** + * 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 the last `limit` snapshots' frontmatter, oldest → newest. + * 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 slice = all.slice(-limit); + return slice.map((file) => parseFrontmatter(fs.readFileSync(file, 'utf-8'))); +} + +// ---- CLI --------------------------------------------------------------- + +function main(argv) { + const [cmd, ...args] = argv; + switch (cmd) { + case 'slug': { + const slug = slugFromTarget(args[0]); + if (!slug) { process.stderr.write('no stable slug for input\n'); process.exit(1); } + process.stdout.write(`${slug}\n`); + return; + } + case 'write': { + const [slug, bodyFile] = args; + if (!slug || !bodyFile) { process.stderr.write('usage: write \n'); process.exit(1); } + const raw = fs.readFileSync(bodyFile, 'utf-8'); + // The body file may be a full report. The caller passes the meta as + // a JSON object on stdin if it wants structured frontmatter; otherwise + // we write with minimal metadata. + let meta = {}; + const metaArg = process.env.IMPECCABLE_CRITIQUE_META; + if (metaArg) { + try { meta = JSON.parse(metaArg); } catch { /* ignore */ } + } + const out = writeSnapshot({ slug, meta, body: raw }); + process.stdout.write(`${out}\n`); + return; + } + case 'latest': { + const latest = readLatestSnapshot(args[0]); + if (!latest) { process.exit(2); } + process.stdout.write(latest.body); + return; + } + case 'trend': { + const rows = readTrend(args[0], { limit: args[1] ? Number(args[1]) : 5 }); + process.stdout.write(JSON.stringify(rows, null, 2) + '\n'); + return; + } + default: + process.stderr.write('usage: critique-storage.mjs [args]\n'); + process.exit(1); + } +} + +if (import.meta.url === `file://${process.argv[1]}`) { + main(process.argv.slice(2)); +} diff --git a/.pi/skills/impeccable/scripts/impeccable-paths.mjs b/.pi/skills/impeccable/scripts/impeccable-paths.mjs index ba852bae9..6befa9cd3 100644 --- a/.pi/skills/impeccable/scripts/impeccable-paths.mjs +++ b/.pi/skills/impeccable/scripts/impeccable-paths.mjs @@ -3,6 +3,7 @@ import path from 'node:path'; export const IMPECCABLE_DIR = '.impeccable'; export const LIVE_DIR = 'live'; +export const CRITIQUE_DIR = 'critique'; export function getImpeccableDir(cwd = process.cwd()) { return path.join(cwd, IMPECCABLE_DIR); @@ -96,6 +97,10 @@ export function getLiveAnnotationsDir(cwd = process.cwd()) { return path.join(getLiveDir(cwd), 'annotations'); } +export function getCritiqueDir(cwd = process.cwd()) { + return path.join(getImpeccableDir(cwd), CRITIQUE_DIR); +} + export function getLegacyLiveAnnotationsDir(cwd = process.cwd()) { return path.join(cwd, '.impeccable-live', 'annotations'); } diff --git a/.qoder/skills/impeccable/reference/critique.md b/.qoder/skills/impeccable/reference/critique.md index d5563b0d3..4153108f6 100644 --- a/.qoder/skills/impeccable/reference/critique.md +++ b/.qoder/skills/impeccable/reference/critique.md @@ -1,5 +1,23 @@ > **Additional context needed**: what the interface is trying to accomplish. +### Setup: Resolve Target and Load Ignore List + +Before gathering assessments, do two small bookkeeping steps. They cost almost nothing and they're what makes critique iterative across runs. + +1. **Resolve the primary artifact.** The user's phrasing ("the homepage", "the pricing flow") is not stable enough to track across runs. Resolve it to a concrete file path or URL: the same one you'd already need to scan code or open in a browser. Examples: + - "the homepage" → `site/pages/index.astro` (or `http://localhost:3000/` if you're inspecting live) + - "the settings modal" → the primary component file (e.g., `src/components/Settings.tsx`) + - "this page" → the URL or the page's source file + Prefer the source file path over the dev-server URL when both exist; ports drift between runs (`bun dev` vs `bun preview`), file paths don't. + +2. **Compute the slug.** Run: + ```bash + node .qoder/skills/impeccable/scripts/critique-storage.mjs slug "" + ``` + Keep the printed slug. It identifies this target's stream across runs. If the command exits non-zero ("no stable slug for input"), skip persistence for this run and tell the user; the trend won't update but the critique still goes ahead. + +3. **Read the ignore list** at `.impeccable/critique/ignore.md` if it exists. Plain markdown; each non-empty, non-comment line is something the user has marked as "do not re-raise" (deferred tradeoffs, designer-intended deviations, detector false-positives the user accepts). When a finding's text matches a line here (case-insensitive substring against rule name or snippet), **drop it silently**. Do not mention it in the report. This is the ONLY input critique consumes from prior runs; anchoring on prior findings would defeat the point of independent assessment. + ### Gather Assessments Launch two independent assessments. **Neither may see the other's output.** This isolation is what makes the combined score honest. Running both in one head silently anchors them to each other; do not shortcut it for cost, speed, or context-size reasons. @@ -164,6 +182,36 @@ Provocative questions that might unlock better solutions: - Prioritize ruthlessly. If everything is important, nothing is. - Don't soften criticism. Developers need honest feedback to ship great design. +### Persist the Snapshot + +Once the report above is finalized, write it to `.impeccable/critique/` so the user can refer back, and so `/impeccable polish` can pick up the priority issues without a copy-paste. + +Skip this step if the Setup slug was null (vague or root-level target). + +1. **Write the body to a temp file** so you can pipe it to the helper. Use the full report (heuristic table, anti-patterns verdict, priority issues, persona red flags) but stop before the "Ask the User" / "Recommended Actions" sections that come later. + +2. **Pass the structured metadata** through `IMPECCABLE_CRITIQUE_META` (JSON), then run the write command: + ```bash + IMPECCABLE_CRITIQUE_META='{"target":"","total_score":,"p0_count":,"p1_count":}' \ + node .qoder/skills/impeccable/scripts/critique-storage.mjs write + ``` + The helper prints the absolute path it wrote. + +3. **Read the trend** for context: + ```bash + node .qoder/skills/impeccable/scripts/critique-storage.mjs trend 5 + ``` + This returns a JSON array of the last 5 frontmatter entries (including the one you just wrote). + +4. **Append a single line to the user-visible output**, after the report and before the questions: + + > **Trend for `` (last 5 runs): 24 → 28 → 32 → 29 → 32** + > Wrote `.impeccable/critique/`. + + If this is the first run for the slug, the trend is just one score; say so: "First run for this target, no trend yet." + +This is fire-and-forget. Do not show the user the helper's JSON output; only the human-readable trend line and the written path. Failures here should not block the rest of the flow; print the error and move on. + ### Ask the User **After presenting findings**, use targeted questions based on what was actually found. ask the user directly to clarify what you cannot infer. These answers will shape the action plan. diff --git a/.qoder/skills/impeccable/reference/polish.md b/.qoder/skills/impeccable/reference/polish.md index eed3dbd27..f1ea94747 100644 --- a/.qoder/skills/impeccable/reference/polish.md +++ b/.qoder/skills/impeccable/reference/polish.md @@ -35,7 +35,14 @@ Understand the current state and goals before touching anything: - Loading and transition smoothness - Information architecture and flow drift (does this feature reveal complexity the way neighboring features do?) -4. **Triage cosmetic vs functional**: Classify each issue as **cosmetic** (looks off, doesn't impede the user) or **functional** (breaks, blocks, or confuses the experience). When polish time is tight, functional issues ship first; cosmetic ones can land in a follow-up. Quality should be consistent; never perfect one corner while leaving another rough. +4. **Pull in any prior critique** (optional signal): If `/impeccable critique` has been run on the same target, its priority issues are a useful prior for what to address first. Resolve the target to a file path or URL, then: + ```bash + slug=$(node .qoder/skills/impeccable/scripts/critique-storage.mjs slug "") + node .qoder/skills/impeccable/scripts/critique-storage.mjs latest "$slug" + ``` + Exit 0 with body = found; fold the P0/P1 items into your polish list and mention the snapshot path so the user sees what you read. Exit 2 = no snapshot, continue without it. The critique is one input among many. Do your own pass either way. + +5. **Triage cosmetic vs functional**: Classify each issue as **cosmetic** (looks off, doesn't impede the user) or **functional** (breaks, blocks, or confuses the experience). When polish time is tight, functional issues ship first; cosmetic ones can land in a follow-up. Quality should be consistent; never perfect one corner while leaving another rough. **CRITICAL**: Polish is the last step, not the first. Don't polish work that's not functionally complete. diff --git a/.qoder/skills/impeccable/scripts/critique-storage.mjs b/.qoder/skills/impeccable/scripts/critique-storage.mjs new file mode 100644 index 000000000..fa7bdf385 --- /dev/null +++ b/.qoder/skills/impeccable/scripts/critique-storage.mjs @@ -0,0 +1,226 @@ +#!/usr/bin/env node +/** + * Critique persistence helper. + * + * Each run of /impeccable critique writes a per-target snapshot to + * .impeccable/critique/__.md + * with a small YAML frontmatter carrying the score + P0/P1 counts. + * + * /impeccable polish reads the latest matching snapshot at start as its + * fix backlog. No other skill auto-reads critique output. + * + * The slug is derived mechanically from the *resolved* primary artifact + * (file path or URL), never from the user's natural-language phrasing. + * Slug stability across runs is what lets the trend display work. + * + * CLI entry points (called from skill instructions): + * node critique-storage.mjs slug + * node critique-storage.mjs write + * node critique-storage.mjs latest + * node critique-storage.mjs trend [limit] + * + * Note: there is intentionally no `ignore` subcommand. ignore.md is a plain + * markdown file; the model reads it directly with its file-read tool. This + * helper only exists for operations the model can't trivially do inline + * (normalizing paths, generating filenames, globbing + parsing frontmatter). + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import { getCritiqueDir } from './impeccable-paths.mjs'; + +const SLUG_MAX = 50; + +/** + * Mechanically derive a slug from a resolved target. Returns null if the + * input doesn't look like a stable identifier (empty, project root, etc). + * + * Accepts file paths and URLs. The model resolves "the homepage" to a + * concrete artifact before calling this — we never slug a natural-language + * phrase. + */ +export function slugFromTarget(resolved, { cwd = process.cwd() } = {}) { + if (!resolved || typeof resolved !== 'string') return null; + const trimmed = resolved.trim(); + if (!trimmed) return null; + + // URL + if (/^https?:\/\//i.test(trimmed)) { + let url; + try { url = new URL(trimmed); } catch { return null; } + const hostPath = `${url.hostname}${url.pathname}`; + return kebab(hostPath); + } + + // File path. Make it project-relative so two devs critiquing the same + // checkout get the same slug regardless of where their repo is cloned. + const abs = path.isAbsolute(trimmed) ? trimmed : path.resolve(cwd, trimmed); + let rel = path.relative(cwd, abs); + // If the target is outside cwd, fall back to the basename so we still + // produce a stable slug (vs the absolute path, which would include + // home dirs / usernames). + if (rel.startsWith('..') || path.isAbsolute(rel)) { + rel = path.basename(abs); + } + if (!rel || rel === '.' || rel === '') return null; + return kebab(rel); +} + +function kebab(s) { + const slug = s + .toLowerCase() + .replace(/[/\\.]+/g, '-') + .replace(/[^a-z0-9-]+/g, '-') + .replace(/-+/g, '-') + .replace(/^-|-$/g, ''); + if (!slug) return null; + // Cap from the tail — the tail (filename) is more identifying than the + // top-level directory. + return slug.length <= SLUG_MAX ? slug : slug.slice(slug.length - SLUG_MAX).replace(/^-/, ''); +} + +/** + * Filename-safe UTC ISO timestamp: hyphens for separators, trailing Z. + * Plain colons aren't allowed on Windows filesystems. + */ +export function nowFilenameStamp(date = new Date()) { + const iso = date.toISOString(); // 2026-05-12T18:30:00.123Z + return iso.replace(/[:.]/g, '-').replace(/-\d+Z$/, 'Z'); +} + +/** + * Write a snapshot for `slug`. `meta` carries the small structured frontmatter + * keys read back by readTrend(). `body` is the human-readable critique + * report (everything below the frontmatter). + * + * Returns the absolute path written. + */ +export function writeSnapshot({ slug, meta, body, cwd = process.cwd(), now = new Date() }) { + if (!slug) throw new Error('writeSnapshot requires a slug'); + const dir = getCritiqueDir(cwd); + fs.mkdirSync(dir, { recursive: true }); + const timestamp = nowFilenameStamp(now); + const filePath = path.join(dir, `${timestamp}__${slug}.md`); + // Spread `meta` first so internally computed `timestamp` and `slug` + // always win. Otherwise a caller-supplied meta blob (parsed from the + // IMPECCABLE_CRITIQUE_META env var) could clobber them, leaving the + // filename in disagreement with its frontmatter and corrupting trends. + const front = serializeFrontmatter({ ...meta, timestamp, slug }); + fs.writeFileSync(filePath, `${front}\n${body.trim()}\n`, 'utf-8'); + return filePath; +} + +function serializeFrontmatter(obj) { + const lines = ['---']; + for (const [key, value] of Object.entries(obj)) { + if (value === undefined || value === null) continue; + const str = typeof value === 'string' ? value : String(value); + // Quote strings that contain : or # to keep parsing simple. + const needsQuotes = typeof value === 'string' && /[:#]/.test(str); + lines.push(`${key}: ${needsQuotes ? JSON.stringify(str) : str}`); + } + lines.push('---'); + return lines.join('\n'); +} + +function parseFrontmatter(text) { + const match = text.match(/^---\r?\n([\s\S]*?)\r?\n---/); + if (!match) return {}; + const out = {}; + for (const line of match[1].split(/\r?\n/)) { + const colon = line.indexOf(':'); + if (colon < 0) continue; + const key = line.slice(0, colon).trim(); + let value = line.slice(colon + 1).trim(); + if (/^".*"$/.test(value)) { + try { value = JSON.parse(value); } catch { /* leave as-is */ } + } else if (/^-?\d+$/.test(value)) { + value = Number(value); + } + out[key] = value; + } + return out; +} + +/** + * Return all snapshot files for `slug`, sorted oldest → newest. + */ +function listSnapshotsForSlug(slug, cwd) { + const dir = getCritiqueDir(cwd); + if (!fs.existsSync(dir)) return []; + const suffix = `__${slug}.md`; + return fs.readdirSync(dir) + .filter((f) => f.endsWith(suffix)) + .sort() + .map((f) => path.join(dir, f)); +} + +/** + * 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 the last `limit` snapshots' frontmatter, oldest → newest. + * 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 slice = all.slice(-limit); + return slice.map((file) => parseFrontmatter(fs.readFileSync(file, 'utf-8'))); +} + +// ---- CLI --------------------------------------------------------------- + +function main(argv) { + const [cmd, ...args] = argv; + switch (cmd) { + case 'slug': { + const slug = slugFromTarget(args[0]); + if (!slug) { process.stderr.write('no stable slug for input\n'); process.exit(1); } + process.stdout.write(`${slug}\n`); + return; + } + case 'write': { + const [slug, bodyFile] = args; + if (!slug || !bodyFile) { process.stderr.write('usage: write \n'); process.exit(1); } + const raw = fs.readFileSync(bodyFile, 'utf-8'); + // The body file may be a full report. The caller passes the meta as + // a JSON object on stdin if it wants structured frontmatter; otherwise + // we write with minimal metadata. + let meta = {}; + const metaArg = process.env.IMPECCABLE_CRITIQUE_META; + if (metaArg) { + try { meta = JSON.parse(metaArg); } catch { /* ignore */ } + } + const out = writeSnapshot({ slug, meta, body: raw }); + process.stdout.write(`${out}\n`); + return; + } + case 'latest': { + const latest = readLatestSnapshot(args[0]); + if (!latest) { process.exit(2); } + process.stdout.write(latest.body); + return; + } + case 'trend': { + const rows = readTrend(args[0], { limit: args[1] ? Number(args[1]) : 5 }); + process.stdout.write(JSON.stringify(rows, null, 2) + '\n'); + return; + } + default: + process.stderr.write('usage: critique-storage.mjs [args]\n'); + process.exit(1); + } +} + +if (import.meta.url === `file://${process.argv[1]}`) { + main(process.argv.slice(2)); +} diff --git a/.qoder/skills/impeccable/scripts/impeccable-paths.mjs b/.qoder/skills/impeccable/scripts/impeccable-paths.mjs index ba852bae9..6befa9cd3 100644 --- a/.qoder/skills/impeccable/scripts/impeccable-paths.mjs +++ b/.qoder/skills/impeccable/scripts/impeccable-paths.mjs @@ -3,6 +3,7 @@ import path from 'node:path'; export const IMPECCABLE_DIR = '.impeccable'; export const LIVE_DIR = 'live'; +export const CRITIQUE_DIR = 'critique'; export function getImpeccableDir(cwd = process.cwd()) { return path.join(cwd, IMPECCABLE_DIR); @@ -96,6 +97,10 @@ export function getLiveAnnotationsDir(cwd = process.cwd()) { return path.join(getLiveDir(cwd), 'annotations'); } +export function getCritiqueDir(cwd = process.cwd()) { + return path.join(getImpeccableDir(cwd), CRITIQUE_DIR); +} + export function getLegacyLiveAnnotationsDir(cwd = process.cwd()) { return path.join(cwd, '.impeccable-live', 'annotations'); } diff --git a/.rovodev/skills/impeccable/reference/critique.md b/.rovodev/skills/impeccable/reference/critique.md index d5563b0d3..abfeae20e 100644 --- a/.rovodev/skills/impeccable/reference/critique.md +++ b/.rovodev/skills/impeccable/reference/critique.md @@ -1,5 +1,23 @@ > **Additional context needed**: what the interface is trying to accomplish. +### Setup: Resolve Target and Load Ignore List + +Before gathering assessments, do two small bookkeeping steps. They cost almost nothing and they're what makes critique iterative across runs. + +1. **Resolve the primary artifact.** The user's phrasing ("the homepage", "the pricing flow") is not stable enough to track across runs. Resolve it to a concrete file path or URL: the same one you'd already need to scan code or open in a browser. Examples: + - "the homepage" → `site/pages/index.astro` (or `http://localhost:3000/` if you're inspecting live) + - "the settings modal" → the primary component file (e.g., `src/components/Settings.tsx`) + - "this page" → the URL or the page's source file + Prefer the source file path over the dev-server URL when both exist; ports drift between runs (`bun dev` vs `bun preview`), file paths don't. + +2. **Compute the slug.** Run: + ```bash + node .rovodev/skills/impeccable/scripts/critique-storage.mjs slug "" + ``` + Keep the printed slug. It identifies this target's stream across runs. If the command exits non-zero ("no stable slug for input"), skip persistence for this run and tell the user; the trend won't update but the critique still goes ahead. + +3. **Read the ignore list** at `.impeccable/critique/ignore.md` if it exists. Plain markdown; each non-empty, non-comment line is something the user has marked as "do not re-raise" (deferred tradeoffs, designer-intended deviations, detector false-positives the user accepts). When a finding's text matches a line here (case-insensitive substring against rule name or snippet), **drop it silently**. Do not mention it in the report. This is the ONLY input critique consumes from prior runs; anchoring on prior findings would defeat the point of independent assessment. + ### Gather Assessments Launch two independent assessments. **Neither may see the other's output.** This isolation is what makes the combined score honest. Running both in one head silently anchors them to each other; do not shortcut it for cost, speed, or context-size reasons. @@ -164,6 +182,36 @@ Provocative questions that might unlock better solutions: - Prioritize ruthlessly. If everything is important, nothing is. - Don't soften criticism. Developers need honest feedback to ship great design. +### Persist the Snapshot + +Once the report above is finalized, write it to `.impeccable/critique/` so the user can refer back, and so `/impeccable polish` can pick up the priority issues without a copy-paste. + +Skip this step if the Setup slug was null (vague or root-level target). + +1. **Write the body to a temp file** so you can pipe it to the helper. Use the full report (heuristic table, anti-patterns verdict, priority issues, persona red flags) but stop before the "Ask the User" / "Recommended Actions" sections that come later. + +2. **Pass the structured metadata** through `IMPECCABLE_CRITIQUE_META` (JSON), then run the write command: + ```bash + IMPECCABLE_CRITIQUE_META='{"target":"","total_score":,"p0_count":,"p1_count":}' \ + node .rovodev/skills/impeccable/scripts/critique-storage.mjs write + ``` + The helper prints the absolute path it wrote. + +3. **Read the trend** for context: + ```bash + node .rovodev/skills/impeccable/scripts/critique-storage.mjs trend 5 + ``` + This returns a JSON array of the last 5 frontmatter entries (including the one you just wrote). + +4. **Append a single line to the user-visible output**, after the report and before the questions: + + > **Trend for `` (last 5 runs): 24 → 28 → 32 → 29 → 32** + > Wrote `.impeccable/critique/`. + + If this is the first run for the slug, the trend is just one score; say so: "First run for this target, no trend yet." + +This is fire-and-forget. Do not show the user the helper's JSON output; only the human-readable trend line and the written path. Failures here should not block the rest of the flow; print the error and move on. + ### Ask the User **After presenting findings**, use targeted questions based on what was actually found. ask the user directly to clarify what you cannot infer. These answers will shape the action plan. diff --git a/.rovodev/skills/impeccable/reference/polish.md b/.rovodev/skills/impeccable/reference/polish.md index eed3dbd27..debc5f096 100644 --- a/.rovodev/skills/impeccable/reference/polish.md +++ b/.rovodev/skills/impeccable/reference/polish.md @@ -35,7 +35,14 @@ Understand the current state and goals before touching anything: - Loading and transition smoothness - Information architecture and flow drift (does this feature reveal complexity the way neighboring features do?) -4. **Triage cosmetic vs functional**: Classify each issue as **cosmetic** (looks off, doesn't impede the user) or **functional** (breaks, blocks, or confuses the experience). When polish time is tight, functional issues ship first; cosmetic ones can land in a follow-up. Quality should be consistent; never perfect one corner while leaving another rough. +4. **Pull in any prior critique** (optional signal): If `/impeccable critique` has been run on the same target, its priority issues are a useful prior for what to address first. Resolve the target to a file path or URL, then: + ```bash + slug=$(node .rovodev/skills/impeccable/scripts/critique-storage.mjs slug "") + node .rovodev/skills/impeccable/scripts/critique-storage.mjs latest "$slug" + ``` + Exit 0 with body = found; fold the P0/P1 items into your polish list and mention the snapshot path so the user sees what you read. Exit 2 = no snapshot, continue without it. The critique is one input among many. Do your own pass either way. + +5. **Triage cosmetic vs functional**: Classify each issue as **cosmetic** (looks off, doesn't impede the user) or **functional** (breaks, blocks, or confuses the experience). When polish time is tight, functional issues ship first; cosmetic ones can land in a follow-up. Quality should be consistent; never perfect one corner while leaving another rough. **CRITICAL**: Polish is the last step, not the first. Don't polish work that's not functionally complete. diff --git a/.rovodev/skills/impeccable/scripts/critique-storage.mjs b/.rovodev/skills/impeccable/scripts/critique-storage.mjs new file mode 100644 index 000000000..fa7bdf385 --- /dev/null +++ b/.rovodev/skills/impeccable/scripts/critique-storage.mjs @@ -0,0 +1,226 @@ +#!/usr/bin/env node +/** + * Critique persistence helper. + * + * Each run of /impeccable critique writes a per-target snapshot to + * .impeccable/critique/__.md + * with a small YAML frontmatter carrying the score + P0/P1 counts. + * + * /impeccable polish reads the latest matching snapshot at start as its + * fix backlog. No other skill auto-reads critique output. + * + * The slug is derived mechanically from the *resolved* primary artifact + * (file path or URL), never from the user's natural-language phrasing. + * Slug stability across runs is what lets the trend display work. + * + * CLI entry points (called from skill instructions): + * node critique-storage.mjs slug + * node critique-storage.mjs write + * node critique-storage.mjs latest + * node critique-storage.mjs trend [limit] + * + * Note: there is intentionally no `ignore` subcommand. ignore.md is a plain + * markdown file; the model reads it directly with its file-read tool. This + * helper only exists for operations the model can't trivially do inline + * (normalizing paths, generating filenames, globbing + parsing frontmatter). + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import { getCritiqueDir } from './impeccable-paths.mjs'; + +const SLUG_MAX = 50; + +/** + * Mechanically derive a slug from a resolved target. Returns null if the + * input doesn't look like a stable identifier (empty, project root, etc). + * + * Accepts file paths and URLs. The model resolves "the homepage" to a + * concrete artifact before calling this — we never slug a natural-language + * phrase. + */ +export function slugFromTarget(resolved, { cwd = process.cwd() } = {}) { + if (!resolved || typeof resolved !== 'string') return null; + const trimmed = resolved.trim(); + if (!trimmed) return null; + + // URL + if (/^https?:\/\//i.test(trimmed)) { + let url; + try { url = new URL(trimmed); } catch { return null; } + const hostPath = `${url.hostname}${url.pathname}`; + return kebab(hostPath); + } + + // File path. Make it project-relative so two devs critiquing the same + // checkout get the same slug regardless of where their repo is cloned. + const abs = path.isAbsolute(trimmed) ? trimmed : path.resolve(cwd, trimmed); + let rel = path.relative(cwd, abs); + // If the target is outside cwd, fall back to the basename so we still + // produce a stable slug (vs the absolute path, which would include + // home dirs / usernames). + if (rel.startsWith('..') || path.isAbsolute(rel)) { + rel = path.basename(abs); + } + if (!rel || rel === '.' || rel === '') return null; + return kebab(rel); +} + +function kebab(s) { + const slug = s + .toLowerCase() + .replace(/[/\\.]+/g, '-') + .replace(/[^a-z0-9-]+/g, '-') + .replace(/-+/g, '-') + .replace(/^-|-$/g, ''); + if (!slug) return null; + // Cap from the tail — the tail (filename) is more identifying than the + // top-level directory. + return slug.length <= SLUG_MAX ? slug : slug.slice(slug.length - SLUG_MAX).replace(/^-/, ''); +} + +/** + * Filename-safe UTC ISO timestamp: hyphens for separators, trailing Z. + * Plain colons aren't allowed on Windows filesystems. + */ +export function nowFilenameStamp(date = new Date()) { + const iso = date.toISOString(); // 2026-05-12T18:30:00.123Z + return iso.replace(/[:.]/g, '-').replace(/-\d+Z$/, 'Z'); +} + +/** + * Write a snapshot for `slug`. `meta` carries the small structured frontmatter + * keys read back by readTrend(). `body` is the human-readable critique + * report (everything below the frontmatter). + * + * Returns the absolute path written. + */ +export function writeSnapshot({ slug, meta, body, cwd = process.cwd(), now = new Date() }) { + if (!slug) throw new Error('writeSnapshot requires a slug'); + const dir = getCritiqueDir(cwd); + fs.mkdirSync(dir, { recursive: true }); + const timestamp = nowFilenameStamp(now); + const filePath = path.join(dir, `${timestamp}__${slug}.md`); + // Spread `meta` first so internally computed `timestamp` and `slug` + // always win. Otherwise a caller-supplied meta blob (parsed from the + // IMPECCABLE_CRITIQUE_META env var) could clobber them, leaving the + // filename in disagreement with its frontmatter and corrupting trends. + const front = serializeFrontmatter({ ...meta, timestamp, slug }); + fs.writeFileSync(filePath, `${front}\n${body.trim()}\n`, 'utf-8'); + return filePath; +} + +function serializeFrontmatter(obj) { + const lines = ['---']; + for (const [key, value] of Object.entries(obj)) { + if (value === undefined || value === null) continue; + const str = typeof value === 'string' ? value : String(value); + // Quote strings that contain : or # to keep parsing simple. + const needsQuotes = typeof value === 'string' && /[:#]/.test(str); + lines.push(`${key}: ${needsQuotes ? JSON.stringify(str) : str}`); + } + lines.push('---'); + return lines.join('\n'); +} + +function parseFrontmatter(text) { + const match = text.match(/^---\r?\n([\s\S]*?)\r?\n---/); + if (!match) return {}; + const out = {}; + for (const line of match[1].split(/\r?\n/)) { + const colon = line.indexOf(':'); + if (colon < 0) continue; + const key = line.slice(0, colon).trim(); + let value = line.slice(colon + 1).trim(); + if (/^".*"$/.test(value)) { + try { value = JSON.parse(value); } catch { /* leave as-is */ } + } else if (/^-?\d+$/.test(value)) { + value = Number(value); + } + out[key] = value; + } + return out; +} + +/** + * Return all snapshot files for `slug`, sorted oldest → newest. + */ +function listSnapshotsForSlug(slug, cwd) { + const dir = getCritiqueDir(cwd); + if (!fs.existsSync(dir)) return []; + const suffix = `__${slug}.md`; + return fs.readdirSync(dir) + .filter((f) => f.endsWith(suffix)) + .sort() + .map((f) => path.join(dir, f)); +} + +/** + * 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 the last `limit` snapshots' frontmatter, oldest → newest. + * 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 slice = all.slice(-limit); + return slice.map((file) => parseFrontmatter(fs.readFileSync(file, 'utf-8'))); +} + +// ---- CLI --------------------------------------------------------------- + +function main(argv) { + const [cmd, ...args] = argv; + switch (cmd) { + case 'slug': { + const slug = slugFromTarget(args[0]); + if (!slug) { process.stderr.write('no stable slug for input\n'); process.exit(1); } + process.stdout.write(`${slug}\n`); + return; + } + case 'write': { + const [slug, bodyFile] = args; + if (!slug || !bodyFile) { process.stderr.write('usage: write \n'); process.exit(1); } + const raw = fs.readFileSync(bodyFile, 'utf-8'); + // The body file may be a full report. The caller passes the meta as + // a JSON object on stdin if it wants structured frontmatter; otherwise + // we write with minimal metadata. + let meta = {}; + const metaArg = process.env.IMPECCABLE_CRITIQUE_META; + if (metaArg) { + try { meta = JSON.parse(metaArg); } catch { /* ignore */ } + } + const out = writeSnapshot({ slug, meta, body: raw }); + process.stdout.write(`${out}\n`); + return; + } + case 'latest': { + const latest = readLatestSnapshot(args[0]); + if (!latest) { process.exit(2); } + process.stdout.write(latest.body); + return; + } + case 'trend': { + const rows = readTrend(args[0], { limit: args[1] ? Number(args[1]) : 5 }); + process.stdout.write(JSON.stringify(rows, null, 2) + '\n'); + return; + } + default: + process.stderr.write('usage: critique-storage.mjs [args]\n'); + process.exit(1); + } +} + +if (import.meta.url === `file://${process.argv[1]}`) { + main(process.argv.slice(2)); +} diff --git a/.rovodev/skills/impeccable/scripts/impeccable-paths.mjs b/.rovodev/skills/impeccable/scripts/impeccable-paths.mjs index ba852bae9..6befa9cd3 100644 --- a/.rovodev/skills/impeccable/scripts/impeccable-paths.mjs +++ b/.rovodev/skills/impeccable/scripts/impeccable-paths.mjs @@ -3,6 +3,7 @@ import path from 'node:path'; export const IMPECCABLE_DIR = '.impeccable'; export const LIVE_DIR = 'live'; +export const CRITIQUE_DIR = 'critique'; export function getImpeccableDir(cwd = process.cwd()) { return path.join(cwd, IMPECCABLE_DIR); @@ -96,6 +97,10 @@ export function getLiveAnnotationsDir(cwd = process.cwd()) { return path.join(getLiveDir(cwd), 'annotations'); } +export function getCritiqueDir(cwd = process.cwd()) { + return path.join(getImpeccableDir(cwd), CRITIQUE_DIR); +} + export function getLegacyLiveAnnotationsDir(cwd = process.cwd()) { return path.join(cwd, '.impeccable-live', 'annotations'); } diff --git a/.trae-cn/skills/impeccable/reference/critique.md b/.trae-cn/skills/impeccable/reference/critique.md index 6db43b3ec..666fe38c0 100644 --- a/.trae-cn/skills/impeccable/reference/critique.md +++ b/.trae-cn/skills/impeccable/reference/critique.md @@ -1,5 +1,23 @@ > **Additional context needed**: what the interface is trying to accomplish. +### Setup: Resolve Target and Load Ignore List + +Before gathering assessments, do two small bookkeeping steps. They cost almost nothing and they're what makes critique iterative across runs. + +1. **Resolve the primary artifact.** The user's phrasing ("the homepage", "the pricing flow") is not stable enough to track across runs. Resolve it to a concrete file path or URL: the same one you'd already need to scan code or open in a browser. Examples: + - "the homepage" → `site/pages/index.astro` (or `http://localhost:3000/` if you're inspecting live) + - "the settings modal" → the primary component file (e.g., `src/components/Settings.tsx`) + - "this page" → the URL or the page's source file + Prefer the source file path over the dev-server URL when both exist; ports drift between runs (`bun dev` vs `bun preview`), file paths don't. + +2. **Compute the slug.** Run: + ```bash + node .trae-cn/skills/impeccable/scripts/critique-storage.mjs slug "" + ``` + Keep the printed slug. It identifies this target's stream across runs. If the command exits non-zero ("no stable slug for input"), skip persistence for this run and tell the user; the trend won't update but the critique still goes ahead. + +3. **Read the ignore list** at `.impeccable/critique/ignore.md` if it exists. Plain markdown; each non-empty, non-comment line is something the user has marked as "do not re-raise" (deferred tradeoffs, designer-intended deviations, detector false-positives the user accepts). When a finding's text matches a line here (case-insensitive substring against rule name or snippet), **drop it silently**. Do not mention it in the report. This is the ONLY input critique consumes from prior runs; anchoring on prior findings would defeat the point of independent assessment. + ### Gather Assessments Launch two independent assessments. **Neither may see the other's output.** This isolation is what makes the combined score honest. Running both in one head silently anchors them to each other; do not shortcut it for cost, speed, or context-size reasons. @@ -164,6 +182,36 @@ Provocative questions that might unlock better solutions: - Prioritize ruthlessly. If everything is important, nothing is. - Don't soften criticism. Developers need honest feedback to ship great design. +### Persist the Snapshot + +Once the report above is finalized, write it to `.impeccable/critique/` so the user can refer back, and so `/impeccable polish` can pick up the priority issues without a copy-paste. + +Skip this step if the Setup slug was null (vague or root-level target). + +1. **Write the body to a temp file** so you can pipe it to the helper. Use the full report (heuristic table, anti-patterns verdict, priority issues, persona red flags) but stop before the "Ask the User" / "Recommended Actions" sections that come later. + +2. **Pass the structured metadata** through `IMPECCABLE_CRITIQUE_META` (JSON), then run the write command: + ```bash + IMPECCABLE_CRITIQUE_META='{"target":"","total_score":,"p0_count":,"p1_count":}' \ + node .trae-cn/skills/impeccable/scripts/critique-storage.mjs write + ``` + The helper prints the absolute path it wrote. + +3. **Read the trend** for context: + ```bash + node .trae-cn/skills/impeccable/scripts/critique-storage.mjs trend 5 + ``` + This returns a JSON array of the last 5 frontmatter entries (including the one you just wrote). + +4. **Append a single line to the user-visible output**, after the report and before the questions: + + > **Trend for `` (last 5 runs): 24 → 28 → 32 → 29 → 32** + > Wrote `.impeccable/critique/`. + + If this is the first run for the slug, the trend is just one score; say so: "First run for this target, no trend yet." + +This is fire-and-forget. Do not show the user the helper's JSON output; only the human-readable trend line and the written path. Failures here should not block the rest of the flow; print the error and move on. + ### Ask the User **After presenting findings**, use targeted questions based on what was actually found. ask the user directly to clarify what you cannot infer. These answers will shape the action plan. diff --git a/.trae-cn/skills/impeccable/reference/polish.md b/.trae-cn/skills/impeccable/reference/polish.md index eed3dbd27..65564ecec 100644 --- a/.trae-cn/skills/impeccable/reference/polish.md +++ b/.trae-cn/skills/impeccable/reference/polish.md @@ -35,7 +35,14 @@ Understand the current state and goals before touching anything: - Loading and transition smoothness - Information architecture and flow drift (does this feature reveal complexity the way neighboring features do?) -4. **Triage cosmetic vs functional**: Classify each issue as **cosmetic** (looks off, doesn't impede the user) or **functional** (breaks, blocks, or confuses the experience). When polish time is tight, functional issues ship first; cosmetic ones can land in a follow-up. Quality should be consistent; never perfect one corner while leaving another rough. +4. **Pull in any prior critique** (optional signal): If `/impeccable critique` has been run on the same target, its priority issues are a useful prior for what to address first. Resolve the target to a file path or URL, then: + ```bash + slug=$(node .trae-cn/skills/impeccable/scripts/critique-storage.mjs slug "") + node .trae-cn/skills/impeccable/scripts/critique-storage.mjs latest "$slug" + ``` + Exit 0 with body = found; fold the P0/P1 items into your polish list and mention the snapshot path so the user sees what you read. Exit 2 = no snapshot, continue without it. The critique is one input among many. Do your own pass either way. + +5. **Triage cosmetic vs functional**: Classify each issue as **cosmetic** (looks off, doesn't impede the user) or **functional** (breaks, blocks, or confuses the experience). When polish time is tight, functional issues ship first; cosmetic ones can land in a follow-up. Quality should be consistent; never perfect one corner while leaving another rough. **CRITICAL**: Polish is the last step, not the first. Don't polish work that's not functionally complete. diff --git a/.trae-cn/skills/impeccable/scripts/critique-storage.mjs b/.trae-cn/skills/impeccable/scripts/critique-storage.mjs new file mode 100644 index 000000000..fa7bdf385 --- /dev/null +++ b/.trae-cn/skills/impeccable/scripts/critique-storage.mjs @@ -0,0 +1,226 @@ +#!/usr/bin/env node +/** + * Critique persistence helper. + * + * Each run of /impeccable critique writes a per-target snapshot to + * .impeccable/critique/__.md + * with a small YAML frontmatter carrying the score + P0/P1 counts. + * + * /impeccable polish reads the latest matching snapshot at start as its + * fix backlog. No other skill auto-reads critique output. + * + * The slug is derived mechanically from the *resolved* primary artifact + * (file path or URL), never from the user's natural-language phrasing. + * Slug stability across runs is what lets the trend display work. + * + * CLI entry points (called from skill instructions): + * node critique-storage.mjs slug + * node critique-storage.mjs write + * node critique-storage.mjs latest + * node critique-storage.mjs trend [limit] + * + * Note: there is intentionally no `ignore` subcommand. ignore.md is a plain + * markdown file; the model reads it directly with its file-read tool. This + * helper only exists for operations the model can't trivially do inline + * (normalizing paths, generating filenames, globbing + parsing frontmatter). + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import { getCritiqueDir } from './impeccable-paths.mjs'; + +const SLUG_MAX = 50; + +/** + * Mechanically derive a slug from a resolved target. Returns null if the + * input doesn't look like a stable identifier (empty, project root, etc). + * + * Accepts file paths and URLs. The model resolves "the homepage" to a + * concrete artifact before calling this — we never slug a natural-language + * phrase. + */ +export function slugFromTarget(resolved, { cwd = process.cwd() } = {}) { + if (!resolved || typeof resolved !== 'string') return null; + const trimmed = resolved.trim(); + if (!trimmed) return null; + + // URL + if (/^https?:\/\//i.test(trimmed)) { + let url; + try { url = new URL(trimmed); } catch { return null; } + const hostPath = `${url.hostname}${url.pathname}`; + return kebab(hostPath); + } + + // File path. Make it project-relative so two devs critiquing the same + // checkout get the same slug regardless of where their repo is cloned. + const abs = path.isAbsolute(trimmed) ? trimmed : path.resolve(cwd, trimmed); + let rel = path.relative(cwd, abs); + // If the target is outside cwd, fall back to the basename so we still + // produce a stable slug (vs the absolute path, which would include + // home dirs / usernames). + if (rel.startsWith('..') || path.isAbsolute(rel)) { + rel = path.basename(abs); + } + if (!rel || rel === '.' || rel === '') return null; + return kebab(rel); +} + +function kebab(s) { + const slug = s + .toLowerCase() + .replace(/[/\\.]+/g, '-') + .replace(/[^a-z0-9-]+/g, '-') + .replace(/-+/g, '-') + .replace(/^-|-$/g, ''); + if (!slug) return null; + // Cap from the tail — the tail (filename) is more identifying than the + // top-level directory. + return slug.length <= SLUG_MAX ? slug : slug.slice(slug.length - SLUG_MAX).replace(/^-/, ''); +} + +/** + * Filename-safe UTC ISO timestamp: hyphens for separators, trailing Z. + * Plain colons aren't allowed on Windows filesystems. + */ +export function nowFilenameStamp(date = new Date()) { + const iso = date.toISOString(); // 2026-05-12T18:30:00.123Z + return iso.replace(/[:.]/g, '-').replace(/-\d+Z$/, 'Z'); +} + +/** + * Write a snapshot for `slug`. `meta` carries the small structured frontmatter + * keys read back by readTrend(). `body` is the human-readable critique + * report (everything below the frontmatter). + * + * Returns the absolute path written. + */ +export function writeSnapshot({ slug, meta, body, cwd = process.cwd(), now = new Date() }) { + if (!slug) throw new Error('writeSnapshot requires a slug'); + const dir = getCritiqueDir(cwd); + fs.mkdirSync(dir, { recursive: true }); + const timestamp = nowFilenameStamp(now); + const filePath = path.join(dir, `${timestamp}__${slug}.md`); + // Spread `meta` first so internally computed `timestamp` and `slug` + // always win. Otherwise a caller-supplied meta blob (parsed from the + // IMPECCABLE_CRITIQUE_META env var) could clobber them, leaving the + // filename in disagreement with its frontmatter and corrupting trends. + const front = serializeFrontmatter({ ...meta, timestamp, slug }); + fs.writeFileSync(filePath, `${front}\n${body.trim()}\n`, 'utf-8'); + return filePath; +} + +function serializeFrontmatter(obj) { + const lines = ['---']; + for (const [key, value] of Object.entries(obj)) { + if (value === undefined || value === null) continue; + const str = typeof value === 'string' ? value : String(value); + // Quote strings that contain : or # to keep parsing simple. + const needsQuotes = typeof value === 'string' && /[:#]/.test(str); + lines.push(`${key}: ${needsQuotes ? JSON.stringify(str) : str}`); + } + lines.push('---'); + return lines.join('\n'); +} + +function parseFrontmatter(text) { + const match = text.match(/^---\r?\n([\s\S]*?)\r?\n---/); + if (!match) return {}; + const out = {}; + for (const line of match[1].split(/\r?\n/)) { + const colon = line.indexOf(':'); + if (colon < 0) continue; + const key = line.slice(0, colon).trim(); + let value = line.slice(colon + 1).trim(); + if (/^".*"$/.test(value)) { + try { value = JSON.parse(value); } catch { /* leave as-is */ } + } else if (/^-?\d+$/.test(value)) { + value = Number(value); + } + out[key] = value; + } + return out; +} + +/** + * Return all snapshot files for `slug`, sorted oldest → newest. + */ +function listSnapshotsForSlug(slug, cwd) { + const dir = getCritiqueDir(cwd); + if (!fs.existsSync(dir)) return []; + const suffix = `__${slug}.md`; + return fs.readdirSync(dir) + .filter((f) => f.endsWith(suffix)) + .sort() + .map((f) => path.join(dir, f)); +} + +/** + * 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 the last `limit` snapshots' frontmatter, oldest → newest. + * 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 slice = all.slice(-limit); + return slice.map((file) => parseFrontmatter(fs.readFileSync(file, 'utf-8'))); +} + +// ---- CLI --------------------------------------------------------------- + +function main(argv) { + const [cmd, ...args] = argv; + switch (cmd) { + case 'slug': { + const slug = slugFromTarget(args[0]); + if (!slug) { process.stderr.write('no stable slug for input\n'); process.exit(1); } + process.stdout.write(`${slug}\n`); + return; + } + case 'write': { + const [slug, bodyFile] = args; + if (!slug || !bodyFile) { process.stderr.write('usage: write \n'); process.exit(1); } + const raw = fs.readFileSync(bodyFile, 'utf-8'); + // The body file may be a full report. The caller passes the meta as + // a JSON object on stdin if it wants structured frontmatter; otherwise + // we write with minimal metadata. + let meta = {}; + const metaArg = process.env.IMPECCABLE_CRITIQUE_META; + if (metaArg) { + try { meta = JSON.parse(metaArg); } catch { /* ignore */ } + } + const out = writeSnapshot({ slug, meta, body: raw }); + process.stdout.write(`${out}\n`); + return; + } + case 'latest': { + const latest = readLatestSnapshot(args[0]); + if (!latest) { process.exit(2); } + process.stdout.write(latest.body); + return; + } + case 'trend': { + const rows = readTrend(args[0], { limit: args[1] ? Number(args[1]) : 5 }); + process.stdout.write(JSON.stringify(rows, null, 2) + '\n'); + return; + } + default: + process.stderr.write('usage: critique-storage.mjs [args]\n'); + process.exit(1); + } +} + +if (import.meta.url === `file://${process.argv[1]}`) { + main(process.argv.slice(2)); +} diff --git a/.trae-cn/skills/impeccable/scripts/impeccable-paths.mjs b/.trae-cn/skills/impeccable/scripts/impeccable-paths.mjs index ba852bae9..6befa9cd3 100644 --- a/.trae-cn/skills/impeccable/scripts/impeccable-paths.mjs +++ b/.trae-cn/skills/impeccable/scripts/impeccable-paths.mjs @@ -3,6 +3,7 @@ import path from 'node:path'; export const IMPECCABLE_DIR = '.impeccable'; export const LIVE_DIR = 'live'; +export const CRITIQUE_DIR = 'critique'; export function getImpeccableDir(cwd = process.cwd()) { return path.join(cwd, IMPECCABLE_DIR); @@ -96,6 +97,10 @@ export function getLiveAnnotationsDir(cwd = process.cwd()) { return path.join(getLiveDir(cwd), 'annotations'); } +export function getCritiqueDir(cwd = process.cwd()) { + return path.join(getImpeccableDir(cwd), CRITIQUE_DIR); +} + export function getLegacyLiveAnnotationsDir(cwd = process.cwd()) { return path.join(cwd, '.impeccable-live', 'annotations'); } diff --git a/.trae/skills/impeccable/reference/critique.md b/.trae/skills/impeccable/reference/critique.md index 6db43b3ec..9b66caaad 100644 --- a/.trae/skills/impeccable/reference/critique.md +++ b/.trae/skills/impeccable/reference/critique.md @@ -1,5 +1,23 @@ > **Additional context needed**: what the interface is trying to accomplish. +### Setup: Resolve Target and Load Ignore List + +Before gathering assessments, do two small bookkeeping steps. They cost almost nothing and they're what makes critique iterative across runs. + +1. **Resolve the primary artifact.** The user's phrasing ("the homepage", "the pricing flow") is not stable enough to track across runs. Resolve it to a concrete file path or URL: the same one you'd already need to scan code or open in a browser. Examples: + - "the homepage" → `site/pages/index.astro` (or `http://localhost:3000/` if you're inspecting live) + - "the settings modal" → the primary component file (e.g., `src/components/Settings.tsx`) + - "this page" → the URL or the page's source file + Prefer the source file path over the dev-server URL when both exist; ports drift between runs (`bun dev` vs `bun preview`), file paths don't. + +2. **Compute the slug.** Run: + ```bash + node .trae/skills/impeccable/scripts/critique-storage.mjs slug "" + ``` + Keep the printed slug. It identifies this target's stream across runs. If the command exits non-zero ("no stable slug for input"), skip persistence for this run and tell the user; the trend won't update but the critique still goes ahead. + +3. **Read the ignore list** at `.impeccable/critique/ignore.md` if it exists. Plain markdown; each non-empty, non-comment line is something the user has marked as "do not re-raise" (deferred tradeoffs, designer-intended deviations, detector false-positives the user accepts). When a finding's text matches a line here (case-insensitive substring against rule name or snippet), **drop it silently**. Do not mention it in the report. This is the ONLY input critique consumes from prior runs; anchoring on prior findings would defeat the point of independent assessment. + ### Gather Assessments Launch two independent assessments. **Neither may see the other's output.** This isolation is what makes the combined score honest. Running both in one head silently anchors them to each other; do not shortcut it for cost, speed, or context-size reasons. @@ -164,6 +182,36 @@ Provocative questions that might unlock better solutions: - Prioritize ruthlessly. If everything is important, nothing is. - Don't soften criticism. Developers need honest feedback to ship great design. +### Persist the Snapshot + +Once the report above is finalized, write it to `.impeccable/critique/` so the user can refer back, and so `/impeccable polish` can pick up the priority issues without a copy-paste. + +Skip this step if the Setup slug was null (vague or root-level target). + +1. **Write the body to a temp file** so you can pipe it to the helper. Use the full report (heuristic table, anti-patterns verdict, priority issues, persona red flags) but stop before the "Ask the User" / "Recommended Actions" sections that come later. + +2. **Pass the structured metadata** through `IMPECCABLE_CRITIQUE_META` (JSON), then run the write command: + ```bash + IMPECCABLE_CRITIQUE_META='{"target":"","total_score":,"p0_count":,"p1_count":}' \ + node .trae/skills/impeccable/scripts/critique-storage.mjs write + ``` + The helper prints the absolute path it wrote. + +3. **Read the trend** for context: + ```bash + node .trae/skills/impeccable/scripts/critique-storage.mjs trend 5 + ``` + This returns a JSON array of the last 5 frontmatter entries (including the one you just wrote). + +4. **Append a single line to the user-visible output**, after the report and before the questions: + + > **Trend for `` (last 5 runs): 24 → 28 → 32 → 29 → 32** + > Wrote `.impeccable/critique/`. + + If this is the first run for the slug, the trend is just one score; say so: "First run for this target, no trend yet." + +This is fire-and-forget. Do not show the user the helper's JSON output; only the human-readable trend line and the written path. Failures here should not block the rest of the flow; print the error and move on. + ### Ask the User **After presenting findings**, use targeted questions based on what was actually found. ask the user directly to clarify what you cannot infer. These answers will shape the action plan. diff --git a/.trae/skills/impeccable/reference/polish.md b/.trae/skills/impeccable/reference/polish.md index eed3dbd27..9a3891540 100644 --- a/.trae/skills/impeccable/reference/polish.md +++ b/.trae/skills/impeccable/reference/polish.md @@ -35,7 +35,14 @@ Understand the current state and goals before touching anything: - Loading and transition smoothness - Information architecture and flow drift (does this feature reveal complexity the way neighboring features do?) -4. **Triage cosmetic vs functional**: Classify each issue as **cosmetic** (looks off, doesn't impede the user) or **functional** (breaks, blocks, or confuses the experience). When polish time is tight, functional issues ship first; cosmetic ones can land in a follow-up. Quality should be consistent; never perfect one corner while leaving another rough. +4. **Pull in any prior critique** (optional signal): If `/impeccable critique` has been run on the same target, its priority issues are a useful prior for what to address first. Resolve the target to a file path or URL, then: + ```bash + slug=$(node .trae/skills/impeccable/scripts/critique-storage.mjs slug "") + node .trae/skills/impeccable/scripts/critique-storage.mjs latest "$slug" + ``` + Exit 0 with body = found; fold the P0/P1 items into your polish list and mention the snapshot path so the user sees what you read. Exit 2 = no snapshot, continue without it. The critique is one input among many. Do your own pass either way. + +5. **Triage cosmetic vs functional**: Classify each issue as **cosmetic** (looks off, doesn't impede the user) or **functional** (breaks, blocks, or confuses the experience). When polish time is tight, functional issues ship first; cosmetic ones can land in a follow-up. Quality should be consistent; never perfect one corner while leaving another rough. **CRITICAL**: Polish is the last step, not the first. Don't polish work that's not functionally complete. diff --git a/.trae/skills/impeccable/scripts/critique-storage.mjs b/.trae/skills/impeccable/scripts/critique-storage.mjs new file mode 100644 index 000000000..fa7bdf385 --- /dev/null +++ b/.trae/skills/impeccable/scripts/critique-storage.mjs @@ -0,0 +1,226 @@ +#!/usr/bin/env node +/** + * Critique persistence helper. + * + * Each run of /impeccable critique writes a per-target snapshot to + * .impeccable/critique/__.md + * with a small YAML frontmatter carrying the score + P0/P1 counts. + * + * /impeccable polish reads the latest matching snapshot at start as its + * fix backlog. No other skill auto-reads critique output. + * + * The slug is derived mechanically from the *resolved* primary artifact + * (file path or URL), never from the user's natural-language phrasing. + * Slug stability across runs is what lets the trend display work. + * + * CLI entry points (called from skill instructions): + * node critique-storage.mjs slug + * node critique-storage.mjs write + * node critique-storage.mjs latest + * node critique-storage.mjs trend [limit] + * + * Note: there is intentionally no `ignore` subcommand. ignore.md is a plain + * markdown file; the model reads it directly with its file-read tool. This + * helper only exists for operations the model can't trivially do inline + * (normalizing paths, generating filenames, globbing + parsing frontmatter). + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import { getCritiqueDir } from './impeccable-paths.mjs'; + +const SLUG_MAX = 50; + +/** + * Mechanically derive a slug from a resolved target. Returns null if the + * input doesn't look like a stable identifier (empty, project root, etc). + * + * Accepts file paths and URLs. The model resolves "the homepage" to a + * concrete artifact before calling this — we never slug a natural-language + * phrase. + */ +export function slugFromTarget(resolved, { cwd = process.cwd() } = {}) { + if (!resolved || typeof resolved !== 'string') return null; + const trimmed = resolved.trim(); + if (!trimmed) return null; + + // URL + if (/^https?:\/\//i.test(trimmed)) { + let url; + try { url = new URL(trimmed); } catch { return null; } + const hostPath = `${url.hostname}${url.pathname}`; + return kebab(hostPath); + } + + // File path. Make it project-relative so two devs critiquing the same + // checkout get the same slug regardless of where their repo is cloned. + const abs = path.isAbsolute(trimmed) ? trimmed : path.resolve(cwd, trimmed); + let rel = path.relative(cwd, abs); + // If the target is outside cwd, fall back to the basename so we still + // produce a stable slug (vs the absolute path, which would include + // home dirs / usernames). + if (rel.startsWith('..') || path.isAbsolute(rel)) { + rel = path.basename(abs); + } + if (!rel || rel === '.' || rel === '') return null; + return kebab(rel); +} + +function kebab(s) { + const slug = s + .toLowerCase() + .replace(/[/\\.]+/g, '-') + .replace(/[^a-z0-9-]+/g, '-') + .replace(/-+/g, '-') + .replace(/^-|-$/g, ''); + if (!slug) return null; + // Cap from the tail — the tail (filename) is more identifying than the + // top-level directory. + return slug.length <= SLUG_MAX ? slug : slug.slice(slug.length - SLUG_MAX).replace(/^-/, ''); +} + +/** + * Filename-safe UTC ISO timestamp: hyphens for separators, trailing Z. + * Plain colons aren't allowed on Windows filesystems. + */ +export function nowFilenameStamp(date = new Date()) { + const iso = date.toISOString(); // 2026-05-12T18:30:00.123Z + return iso.replace(/[:.]/g, '-').replace(/-\d+Z$/, 'Z'); +} + +/** + * Write a snapshot for `slug`. `meta` carries the small structured frontmatter + * keys read back by readTrend(). `body` is the human-readable critique + * report (everything below the frontmatter). + * + * Returns the absolute path written. + */ +export function writeSnapshot({ slug, meta, body, cwd = process.cwd(), now = new Date() }) { + if (!slug) throw new Error('writeSnapshot requires a slug'); + const dir = getCritiqueDir(cwd); + fs.mkdirSync(dir, { recursive: true }); + const timestamp = nowFilenameStamp(now); + const filePath = path.join(dir, `${timestamp}__${slug}.md`); + // Spread `meta` first so internally computed `timestamp` and `slug` + // always win. Otherwise a caller-supplied meta blob (parsed from the + // IMPECCABLE_CRITIQUE_META env var) could clobber them, leaving the + // filename in disagreement with its frontmatter and corrupting trends. + const front = serializeFrontmatter({ ...meta, timestamp, slug }); + fs.writeFileSync(filePath, `${front}\n${body.trim()}\n`, 'utf-8'); + return filePath; +} + +function serializeFrontmatter(obj) { + const lines = ['---']; + for (const [key, value] of Object.entries(obj)) { + if (value === undefined || value === null) continue; + const str = typeof value === 'string' ? value : String(value); + // Quote strings that contain : or # to keep parsing simple. + const needsQuotes = typeof value === 'string' && /[:#]/.test(str); + lines.push(`${key}: ${needsQuotes ? JSON.stringify(str) : str}`); + } + lines.push('---'); + return lines.join('\n'); +} + +function parseFrontmatter(text) { + const match = text.match(/^---\r?\n([\s\S]*?)\r?\n---/); + if (!match) return {}; + const out = {}; + for (const line of match[1].split(/\r?\n/)) { + const colon = line.indexOf(':'); + if (colon < 0) continue; + const key = line.slice(0, colon).trim(); + let value = line.slice(colon + 1).trim(); + if (/^".*"$/.test(value)) { + try { value = JSON.parse(value); } catch { /* leave as-is */ } + } else if (/^-?\d+$/.test(value)) { + value = Number(value); + } + out[key] = value; + } + return out; +} + +/** + * Return all snapshot files for `slug`, sorted oldest → newest. + */ +function listSnapshotsForSlug(slug, cwd) { + const dir = getCritiqueDir(cwd); + if (!fs.existsSync(dir)) return []; + const suffix = `__${slug}.md`; + return fs.readdirSync(dir) + .filter((f) => f.endsWith(suffix)) + .sort() + .map((f) => path.join(dir, f)); +} + +/** + * 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 the last `limit` snapshots' frontmatter, oldest → newest. + * 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 slice = all.slice(-limit); + return slice.map((file) => parseFrontmatter(fs.readFileSync(file, 'utf-8'))); +} + +// ---- CLI --------------------------------------------------------------- + +function main(argv) { + const [cmd, ...args] = argv; + switch (cmd) { + case 'slug': { + const slug = slugFromTarget(args[0]); + if (!slug) { process.stderr.write('no stable slug for input\n'); process.exit(1); } + process.stdout.write(`${slug}\n`); + return; + } + case 'write': { + const [slug, bodyFile] = args; + if (!slug || !bodyFile) { process.stderr.write('usage: write \n'); process.exit(1); } + const raw = fs.readFileSync(bodyFile, 'utf-8'); + // The body file may be a full report. The caller passes the meta as + // a JSON object on stdin if it wants structured frontmatter; otherwise + // we write with minimal metadata. + let meta = {}; + const metaArg = process.env.IMPECCABLE_CRITIQUE_META; + if (metaArg) { + try { meta = JSON.parse(metaArg); } catch { /* ignore */ } + } + const out = writeSnapshot({ slug, meta, body: raw }); + process.stdout.write(`${out}\n`); + return; + } + case 'latest': { + const latest = readLatestSnapshot(args[0]); + if (!latest) { process.exit(2); } + process.stdout.write(latest.body); + return; + } + case 'trend': { + const rows = readTrend(args[0], { limit: args[1] ? Number(args[1]) : 5 }); + process.stdout.write(JSON.stringify(rows, null, 2) + '\n'); + return; + } + default: + process.stderr.write('usage: critique-storage.mjs [args]\n'); + process.exit(1); + } +} + +if (import.meta.url === `file://${process.argv[1]}`) { + main(process.argv.slice(2)); +} diff --git a/.trae/skills/impeccable/scripts/impeccable-paths.mjs b/.trae/skills/impeccable/scripts/impeccable-paths.mjs index ba852bae9..6befa9cd3 100644 --- a/.trae/skills/impeccable/scripts/impeccable-paths.mjs +++ b/.trae/skills/impeccable/scripts/impeccable-paths.mjs @@ -3,6 +3,7 @@ import path from 'node:path'; export const IMPECCABLE_DIR = '.impeccable'; export const LIVE_DIR = 'live'; +export const CRITIQUE_DIR = 'critique'; export function getImpeccableDir(cwd = process.cwd()) { return path.join(cwd, IMPECCABLE_DIR); @@ -96,6 +97,10 @@ export function getLiveAnnotationsDir(cwd = process.cwd()) { return path.join(getLiveDir(cwd), 'annotations'); } +export function getCritiqueDir(cwd = process.cwd()) { + return path.join(getImpeccableDir(cwd), CRITIQUE_DIR); +} + export function getLegacyLiveAnnotationsDir(cwd = process.cwd()) { return path.join(cwd, '.impeccable-live', 'annotations'); } diff --git a/plugin/skills/impeccable/reference/critique.md b/plugin/skills/impeccable/reference/critique.md index fddf20f11..5e3f3a771 100644 --- a/plugin/skills/impeccable/reference/critique.md +++ b/plugin/skills/impeccable/reference/critique.md @@ -1,5 +1,23 @@ > **Additional context needed**: what the interface is trying to accomplish. +### Setup: Resolve Target and Load Ignore List + +Before gathering assessments, do two small bookkeeping steps. They cost almost nothing and they're what makes critique iterative across runs. + +1. **Resolve the primary artifact.** The user's phrasing ("the homepage", "the pricing flow") is not stable enough to track across runs. Resolve it to a concrete file path or URL: the same one you'd already need to scan code or open in a browser. Examples: + - "the homepage" → `site/pages/index.astro` (or `http://localhost:3000/` if you're inspecting live) + - "the settings modal" → the primary component file (e.g., `src/components/Settings.tsx`) + - "this page" → the URL or the page's source file + Prefer the source file path over the dev-server URL when both exist; ports drift between runs (`bun dev` vs `bun preview`), file paths don't. + +2. **Compute the slug.** Run: + ```bash + node .claude/skills/impeccable/scripts/critique-storage.mjs slug "" + ``` + Keep the printed slug. It identifies this target's stream across runs. If the command exits non-zero ("no stable slug for input"), skip persistence for this run and tell the user; the trend won't update but the critique still goes ahead. + +3. **Read the ignore list** at `.impeccable/critique/ignore.md` if it exists. Plain markdown; each non-empty, non-comment line is something the user has marked as "do not re-raise" (deferred tradeoffs, designer-intended deviations, detector false-positives the user accepts). When a finding's text matches a line here (case-insensitive substring against rule name or snippet), **drop it silently**. Do not mention it in the report. This is the ONLY input critique consumes from prior runs; anchoring on prior findings would defeat the point of independent assessment. + ### Gather Assessments Launch two independent assessments. **Neither may see the other's output.** This isolation is what makes the combined score honest. Running both in one head silently anchors them to each other; do not shortcut it for cost, speed, or context-size reasons. @@ -164,6 +182,36 @@ Provocative questions that might unlock better solutions: - Prioritize ruthlessly. If everything is important, nothing is. - Don't soften criticism. Developers need honest feedback to ship great design. +### Persist the Snapshot + +Once the report above is finalized, write it to `.impeccable/critique/` so the user can refer back, and so `/impeccable polish` can pick up the priority issues without a copy-paste. + +Skip this step if the Setup slug was null (vague or root-level target). + +1. **Write the body to a temp file** so you can pipe it to the helper. Use the full report (heuristic table, anti-patterns verdict, priority issues, persona red flags) but stop before the "Ask the User" / "Recommended Actions" sections that come later. + +2. **Pass the structured metadata** through `IMPECCABLE_CRITIQUE_META` (JSON), then run the write command: + ```bash + IMPECCABLE_CRITIQUE_META='{"target":"","total_score":,"p0_count":,"p1_count":}' \ + node .claude/skills/impeccable/scripts/critique-storage.mjs write + ``` + The helper prints the absolute path it wrote. + +3. **Read the trend** for context: + ```bash + node .claude/skills/impeccable/scripts/critique-storage.mjs trend 5 + ``` + This returns a JSON array of the last 5 frontmatter entries (including the one you just wrote). + +4. **Append a single line to the user-visible output**, after the report and before the questions: + + > **Trend for `` (last 5 runs): 24 → 28 → 32 → 29 → 32** + > Wrote `.impeccable/critique/`. + + If this is the first run for the slug, the trend is just one score; say so: "First run for this target, no trend yet." + +This is fire-and-forget. Do not show the user the helper's JSON output; only the human-readable trend line and the written path. Failures here should not block the rest of the flow; print the error and move on. + ### Ask the User **After presenting findings**, use targeted questions based on what was actually found. STOP and call the AskUserQuestion tool to clarify. These answers will shape the action plan. diff --git a/plugin/skills/impeccable/reference/polish.md b/plugin/skills/impeccable/reference/polish.md index eed3dbd27..0ba47d19d 100644 --- a/plugin/skills/impeccable/reference/polish.md +++ b/plugin/skills/impeccable/reference/polish.md @@ -35,7 +35,14 @@ Understand the current state and goals before touching anything: - Loading and transition smoothness - Information architecture and flow drift (does this feature reveal complexity the way neighboring features do?) -4. **Triage cosmetic vs functional**: Classify each issue as **cosmetic** (looks off, doesn't impede the user) or **functional** (breaks, blocks, or confuses the experience). When polish time is tight, functional issues ship first; cosmetic ones can land in a follow-up. Quality should be consistent; never perfect one corner while leaving another rough. +4. **Pull in any prior critique** (optional signal): If `/impeccable critique` has been run on the same target, its priority issues are a useful prior for what to address first. Resolve the target to a file path or URL, then: + ```bash + slug=$(node .claude/skills/impeccable/scripts/critique-storage.mjs slug "") + node .claude/skills/impeccable/scripts/critique-storage.mjs latest "$slug" + ``` + Exit 0 with body = found; fold the P0/P1 items into your polish list and mention the snapshot path so the user sees what you read. Exit 2 = no snapshot, continue without it. The critique is one input among many. Do your own pass either way. + +5. **Triage cosmetic vs functional**: Classify each issue as **cosmetic** (looks off, doesn't impede the user) or **functional** (breaks, blocks, or confuses the experience). When polish time is tight, functional issues ship first; cosmetic ones can land in a follow-up. Quality should be consistent; never perfect one corner while leaving another rough. **CRITICAL**: Polish is the last step, not the first. Don't polish work that's not functionally complete. diff --git a/plugin/skills/impeccable/scripts/critique-storage.mjs b/plugin/skills/impeccable/scripts/critique-storage.mjs new file mode 100644 index 000000000..fa7bdf385 --- /dev/null +++ b/plugin/skills/impeccable/scripts/critique-storage.mjs @@ -0,0 +1,226 @@ +#!/usr/bin/env node +/** + * Critique persistence helper. + * + * Each run of /impeccable critique writes a per-target snapshot to + * .impeccable/critique/__.md + * with a small YAML frontmatter carrying the score + P0/P1 counts. + * + * /impeccable polish reads the latest matching snapshot at start as its + * fix backlog. No other skill auto-reads critique output. + * + * The slug is derived mechanically from the *resolved* primary artifact + * (file path or URL), never from the user's natural-language phrasing. + * Slug stability across runs is what lets the trend display work. + * + * CLI entry points (called from skill instructions): + * node critique-storage.mjs slug + * node critique-storage.mjs write + * node critique-storage.mjs latest + * node critique-storage.mjs trend [limit] + * + * Note: there is intentionally no `ignore` subcommand. ignore.md is a plain + * markdown file; the model reads it directly with its file-read tool. This + * helper only exists for operations the model can't trivially do inline + * (normalizing paths, generating filenames, globbing + parsing frontmatter). + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import { getCritiqueDir } from './impeccable-paths.mjs'; + +const SLUG_MAX = 50; + +/** + * Mechanically derive a slug from a resolved target. Returns null if the + * input doesn't look like a stable identifier (empty, project root, etc). + * + * Accepts file paths and URLs. The model resolves "the homepage" to a + * concrete artifact before calling this — we never slug a natural-language + * phrase. + */ +export function slugFromTarget(resolved, { cwd = process.cwd() } = {}) { + if (!resolved || typeof resolved !== 'string') return null; + const trimmed = resolved.trim(); + if (!trimmed) return null; + + // URL + if (/^https?:\/\//i.test(trimmed)) { + let url; + try { url = new URL(trimmed); } catch { return null; } + const hostPath = `${url.hostname}${url.pathname}`; + return kebab(hostPath); + } + + // File path. Make it project-relative so two devs critiquing the same + // checkout get the same slug regardless of where their repo is cloned. + const abs = path.isAbsolute(trimmed) ? trimmed : path.resolve(cwd, trimmed); + let rel = path.relative(cwd, abs); + // If the target is outside cwd, fall back to the basename so we still + // produce a stable slug (vs the absolute path, which would include + // home dirs / usernames). + if (rel.startsWith('..') || path.isAbsolute(rel)) { + rel = path.basename(abs); + } + if (!rel || rel === '.' || rel === '') return null; + return kebab(rel); +} + +function kebab(s) { + const slug = s + .toLowerCase() + .replace(/[/\\.]+/g, '-') + .replace(/[^a-z0-9-]+/g, '-') + .replace(/-+/g, '-') + .replace(/^-|-$/g, ''); + if (!slug) return null; + // Cap from the tail — the tail (filename) is more identifying than the + // top-level directory. + return slug.length <= SLUG_MAX ? slug : slug.slice(slug.length - SLUG_MAX).replace(/^-/, ''); +} + +/** + * Filename-safe UTC ISO timestamp: hyphens for separators, trailing Z. + * Plain colons aren't allowed on Windows filesystems. + */ +export function nowFilenameStamp(date = new Date()) { + const iso = date.toISOString(); // 2026-05-12T18:30:00.123Z + return iso.replace(/[:.]/g, '-').replace(/-\d+Z$/, 'Z'); +} + +/** + * Write a snapshot for `slug`. `meta` carries the small structured frontmatter + * keys read back by readTrend(). `body` is the human-readable critique + * report (everything below the frontmatter). + * + * Returns the absolute path written. + */ +export function writeSnapshot({ slug, meta, body, cwd = process.cwd(), now = new Date() }) { + if (!slug) throw new Error('writeSnapshot requires a slug'); + const dir = getCritiqueDir(cwd); + fs.mkdirSync(dir, { recursive: true }); + const timestamp = nowFilenameStamp(now); + const filePath = path.join(dir, `${timestamp}__${slug}.md`); + // Spread `meta` first so internally computed `timestamp` and `slug` + // always win. Otherwise a caller-supplied meta blob (parsed from the + // IMPECCABLE_CRITIQUE_META env var) could clobber them, leaving the + // filename in disagreement with its frontmatter and corrupting trends. + const front = serializeFrontmatter({ ...meta, timestamp, slug }); + fs.writeFileSync(filePath, `${front}\n${body.trim()}\n`, 'utf-8'); + return filePath; +} + +function serializeFrontmatter(obj) { + const lines = ['---']; + for (const [key, value] of Object.entries(obj)) { + if (value === undefined || value === null) continue; + const str = typeof value === 'string' ? value : String(value); + // Quote strings that contain : or # to keep parsing simple. + const needsQuotes = typeof value === 'string' && /[:#]/.test(str); + lines.push(`${key}: ${needsQuotes ? JSON.stringify(str) : str}`); + } + lines.push('---'); + return lines.join('\n'); +} + +function parseFrontmatter(text) { + const match = text.match(/^---\r?\n([\s\S]*?)\r?\n---/); + if (!match) return {}; + const out = {}; + for (const line of match[1].split(/\r?\n/)) { + const colon = line.indexOf(':'); + if (colon < 0) continue; + const key = line.slice(0, colon).trim(); + let value = line.slice(colon + 1).trim(); + if (/^".*"$/.test(value)) { + try { value = JSON.parse(value); } catch { /* leave as-is */ } + } else if (/^-?\d+$/.test(value)) { + value = Number(value); + } + out[key] = value; + } + return out; +} + +/** + * Return all snapshot files for `slug`, sorted oldest → newest. + */ +function listSnapshotsForSlug(slug, cwd) { + const dir = getCritiqueDir(cwd); + if (!fs.existsSync(dir)) return []; + const suffix = `__${slug}.md`; + return fs.readdirSync(dir) + .filter((f) => f.endsWith(suffix)) + .sort() + .map((f) => path.join(dir, f)); +} + +/** + * 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 the last `limit` snapshots' frontmatter, oldest → newest. + * 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 slice = all.slice(-limit); + return slice.map((file) => parseFrontmatter(fs.readFileSync(file, 'utf-8'))); +} + +// ---- CLI --------------------------------------------------------------- + +function main(argv) { + const [cmd, ...args] = argv; + switch (cmd) { + case 'slug': { + const slug = slugFromTarget(args[0]); + if (!slug) { process.stderr.write('no stable slug for input\n'); process.exit(1); } + process.stdout.write(`${slug}\n`); + return; + } + case 'write': { + const [slug, bodyFile] = args; + if (!slug || !bodyFile) { process.stderr.write('usage: write \n'); process.exit(1); } + const raw = fs.readFileSync(bodyFile, 'utf-8'); + // The body file may be a full report. The caller passes the meta as + // a JSON object on stdin if it wants structured frontmatter; otherwise + // we write with minimal metadata. + let meta = {}; + const metaArg = process.env.IMPECCABLE_CRITIQUE_META; + if (metaArg) { + try { meta = JSON.parse(metaArg); } catch { /* ignore */ } + } + const out = writeSnapshot({ slug, meta, body: raw }); + process.stdout.write(`${out}\n`); + return; + } + case 'latest': { + const latest = readLatestSnapshot(args[0]); + if (!latest) { process.exit(2); } + process.stdout.write(latest.body); + return; + } + case 'trend': { + const rows = readTrend(args[0], { limit: args[1] ? Number(args[1]) : 5 }); + process.stdout.write(JSON.stringify(rows, null, 2) + '\n'); + return; + } + default: + process.stderr.write('usage: critique-storage.mjs [args]\n'); + process.exit(1); + } +} + +if (import.meta.url === `file://${process.argv[1]}`) { + main(process.argv.slice(2)); +} diff --git a/plugin/skills/impeccable/scripts/impeccable-paths.mjs b/plugin/skills/impeccable/scripts/impeccable-paths.mjs index ba852bae9..6befa9cd3 100644 --- a/plugin/skills/impeccable/scripts/impeccable-paths.mjs +++ b/plugin/skills/impeccable/scripts/impeccable-paths.mjs @@ -3,6 +3,7 @@ import path from 'node:path'; export const IMPECCABLE_DIR = '.impeccable'; export const LIVE_DIR = 'live'; +export const CRITIQUE_DIR = 'critique'; export function getImpeccableDir(cwd = process.cwd()) { return path.join(cwd, IMPECCABLE_DIR); @@ -96,6 +97,10 @@ export function getLiveAnnotationsDir(cwd = process.cwd()) { return path.join(getLiveDir(cwd), 'annotations'); } +export function getCritiqueDir(cwd = process.cwd()) { + return path.join(getImpeccableDir(cwd), CRITIQUE_DIR); +} + export function getLegacyLiveAnnotationsDir(cwd = process.cwd()) { return path.join(cwd, '.impeccable-live', 'annotations'); } diff --git a/skill/reference/critique.md b/skill/reference/critique.md index 72744845c..2e1b81060 100644 --- a/skill/reference/critique.md +++ b/skill/reference/critique.md @@ -1,5 +1,23 @@ > **Additional context needed**: what the interface is trying to accomplish. +### Setup: Resolve Target and Load Ignore List + +Before gathering assessments, do two small bookkeeping steps. They cost almost nothing and they're what makes critique iterative across runs. + +1. **Resolve the primary artifact.** The user's phrasing ("the homepage", "the pricing flow") is not stable enough to track across runs. Resolve it to a concrete file path or URL: the same one you'd already need to scan code or open in a browser. Examples: + - "the homepage" → `site/pages/index.astro` (or `http://localhost:3000/` if you're inspecting live) + - "the settings modal" → the primary component file (e.g., `src/components/Settings.tsx`) + - "this page" → the URL or the page's source file + Prefer the source file path over the dev-server URL when both exist; ports drift between runs (`bun dev` vs `bun preview`), file paths don't. + +2. **Compute the slug.** Run: + ```bash + node {{scripts_path}}/critique-storage.mjs slug "" + ``` + Keep the printed slug. It identifies this target's stream across runs. If the command exits non-zero ("no stable slug for input"), skip persistence for this run and tell the user; the trend won't update but the critique still goes ahead. + +3. **Read the ignore list** at `.impeccable/critique/ignore.md` if it exists. Plain markdown; each non-empty, non-comment line is something the user has marked as "do not re-raise" (deferred tradeoffs, designer-intended deviations, detector false-positives the user accepts). When a finding's text matches a line here (case-insensitive substring against rule name or snippet), **drop it silently**. Do not mention it in the report. This is the ONLY input critique consumes from prior runs; anchoring on prior findings would defeat the point of independent assessment. + ### Gather Assessments Launch two independent assessments. **Neither may see the other's output.** This isolation is what makes the combined score honest. Running both in one head silently anchors them to each other; do not shortcut it for cost, speed, or context-size reasons. @@ -164,6 +182,36 @@ Provocative questions that might unlock better solutions: - Prioritize ruthlessly. If everything is important, nothing is. - Don't soften criticism. Developers need honest feedback to ship great design. +### Persist the Snapshot + +Once the report above is finalized, write it to `.impeccable/critique/` so the user can refer back, and so `{{command_prefix}}impeccable polish` can pick up the priority issues without a copy-paste. + +Skip this step if the Setup slug was null (vague or root-level target). + +1. **Write the body to a temp file** so you can pipe it to the helper. Use the full report (heuristic table, anti-patterns verdict, priority issues, persona red flags) but stop before the "Ask the User" / "Recommended Actions" sections that come later. + +2. **Pass the structured metadata** through `IMPECCABLE_CRITIQUE_META` (JSON), then run the write command: + ```bash + IMPECCABLE_CRITIQUE_META='{"target":"","total_score":,"p0_count":,"p1_count":}' \ + node {{scripts_path}}/critique-storage.mjs write + ``` + The helper prints the absolute path it wrote. + +3. **Read the trend** for context: + ```bash + node {{scripts_path}}/critique-storage.mjs trend 5 + ``` + This returns a JSON array of the last 5 frontmatter entries (including the one you just wrote). + +4. **Append a single line to the user-visible output**, after the report and before the questions: + + > **Trend for `` (last 5 runs): 24 → 28 → 32 → 29 → 32** + > Wrote `.impeccable/critique/`. + + If this is the first run for the slug, the trend is just one score; say so: "First run for this target, no trend yet." + +This is fire-and-forget. Do not show the user the helper's JSON output; only the human-readable trend line and the written path. Failures here should not block the rest of the flow; print the error and move on. + ### Ask the User **After presenting findings**, use targeted questions based on what was actually found. {{ask_instruction}} These answers will shape the action plan. diff --git a/skill/reference/polish.md b/skill/reference/polish.md index eed3dbd27..7274c89f6 100644 --- a/skill/reference/polish.md +++ b/skill/reference/polish.md @@ -35,7 +35,14 @@ Understand the current state and goals before touching anything: - Loading and transition smoothness - Information architecture and flow drift (does this feature reveal complexity the way neighboring features do?) -4. **Triage cosmetic vs functional**: Classify each issue as **cosmetic** (looks off, doesn't impede the user) or **functional** (breaks, blocks, or confuses the experience). When polish time is tight, functional issues ship first; cosmetic ones can land in a follow-up. Quality should be consistent; never perfect one corner while leaving another rough. +4. **Pull in any prior critique** (optional signal): If `{{command_prefix}}impeccable critique` has been run on the same target, its priority issues are a useful prior for what to address first. Resolve the target to a file path or URL, then: + ```bash + slug=$(node {{scripts_path}}/critique-storage.mjs slug "") + node {{scripts_path}}/critique-storage.mjs latest "$slug" + ``` + Exit 0 with body = found; fold the P0/P1 items into your polish list and mention the snapshot path so the user sees what you read. Exit 2 = no snapshot, continue without it. The critique is one input among many. Do your own pass either way. + +5. **Triage cosmetic vs functional**: Classify each issue as **cosmetic** (looks off, doesn't impede the user) or **functional** (breaks, blocks, or confuses the experience). When polish time is tight, functional issues ship first; cosmetic ones can land in a follow-up. Quality should be consistent; never perfect one corner while leaving another rough. **CRITICAL**: Polish is the last step, not the first. Don't polish work that's not functionally complete. diff --git a/skill/scripts/critique-storage.mjs b/skill/scripts/critique-storage.mjs new file mode 100644 index 000000000..fa7bdf385 --- /dev/null +++ b/skill/scripts/critique-storage.mjs @@ -0,0 +1,226 @@ +#!/usr/bin/env node +/** + * Critique persistence helper. + * + * Each run of /impeccable critique writes a per-target snapshot to + * .impeccable/critique/__.md + * with a small YAML frontmatter carrying the score + P0/P1 counts. + * + * /impeccable polish reads the latest matching snapshot at start as its + * fix backlog. No other skill auto-reads critique output. + * + * The slug is derived mechanically from the *resolved* primary artifact + * (file path or URL), never from the user's natural-language phrasing. + * Slug stability across runs is what lets the trend display work. + * + * CLI entry points (called from skill instructions): + * node critique-storage.mjs slug + * node critique-storage.mjs write + * node critique-storage.mjs latest + * node critique-storage.mjs trend [limit] + * + * Note: there is intentionally no `ignore` subcommand. ignore.md is a plain + * markdown file; the model reads it directly with its file-read tool. This + * helper only exists for operations the model can't trivially do inline + * (normalizing paths, generating filenames, globbing + parsing frontmatter). + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import { getCritiqueDir } from './impeccable-paths.mjs'; + +const SLUG_MAX = 50; + +/** + * Mechanically derive a slug from a resolved target. Returns null if the + * input doesn't look like a stable identifier (empty, project root, etc). + * + * Accepts file paths and URLs. The model resolves "the homepage" to a + * concrete artifact before calling this — we never slug a natural-language + * phrase. + */ +export function slugFromTarget(resolved, { cwd = process.cwd() } = {}) { + if (!resolved || typeof resolved !== 'string') return null; + const trimmed = resolved.trim(); + if (!trimmed) return null; + + // URL + if (/^https?:\/\//i.test(trimmed)) { + let url; + try { url = new URL(trimmed); } catch { return null; } + const hostPath = `${url.hostname}${url.pathname}`; + return kebab(hostPath); + } + + // File path. Make it project-relative so two devs critiquing the same + // checkout get the same slug regardless of where their repo is cloned. + const abs = path.isAbsolute(trimmed) ? trimmed : path.resolve(cwd, trimmed); + let rel = path.relative(cwd, abs); + // If the target is outside cwd, fall back to the basename so we still + // produce a stable slug (vs the absolute path, which would include + // home dirs / usernames). + if (rel.startsWith('..') || path.isAbsolute(rel)) { + rel = path.basename(abs); + } + if (!rel || rel === '.' || rel === '') return null; + return kebab(rel); +} + +function kebab(s) { + const slug = s + .toLowerCase() + .replace(/[/\\.]+/g, '-') + .replace(/[^a-z0-9-]+/g, '-') + .replace(/-+/g, '-') + .replace(/^-|-$/g, ''); + if (!slug) return null; + // Cap from the tail — the tail (filename) is more identifying than the + // top-level directory. + return slug.length <= SLUG_MAX ? slug : slug.slice(slug.length - SLUG_MAX).replace(/^-/, ''); +} + +/** + * Filename-safe UTC ISO timestamp: hyphens for separators, trailing Z. + * Plain colons aren't allowed on Windows filesystems. + */ +export function nowFilenameStamp(date = new Date()) { + const iso = date.toISOString(); // 2026-05-12T18:30:00.123Z + return iso.replace(/[:.]/g, '-').replace(/-\d+Z$/, 'Z'); +} + +/** + * Write a snapshot for `slug`. `meta` carries the small structured frontmatter + * keys read back by readTrend(). `body` is the human-readable critique + * report (everything below the frontmatter). + * + * Returns the absolute path written. + */ +export function writeSnapshot({ slug, meta, body, cwd = process.cwd(), now = new Date() }) { + if (!slug) throw new Error('writeSnapshot requires a slug'); + const dir = getCritiqueDir(cwd); + fs.mkdirSync(dir, { recursive: true }); + const timestamp = nowFilenameStamp(now); + const filePath = path.join(dir, `${timestamp}__${slug}.md`); + // Spread `meta` first so internally computed `timestamp` and `slug` + // always win. Otherwise a caller-supplied meta blob (parsed from the + // IMPECCABLE_CRITIQUE_META env var) could clobber them, leaving the + // filename in disagreement with its frontmatter and corrupting trends. + const front = serializeFrontmatter({ ...meta, timestamp, slug }); + fs.writeFileSync(filePath, `${front}\n${body.trim()}\n`, 'utf-8'); + return filePath; +} + +function serializeFrontmatter(obj) { + const lines = ['---']; + for (const [key, value] of Object.entries(obj)) { + if (value === undefined || value === null) continue; + const str = typeof value === 'string' ? value : String(value); + // Quote strings that contain : or # to keep parsing simple. + const needsQuotes = typeof value === 'string' && /[:#]/.test(str); + lines.push(`${key}: ${needsQuotes ? JSON.stringify(str) : str}`); + } + lines.push('---'); + return lines.join('\n'); +} + +function parseFrontmatter(text) { + const match = text.match(/^---\r?\n([\s\S]*?)\r?\n---/); + if (!match) return {}; + const out = {}; + for (const line of match[1].split(/\r?\n/)) { + const colon = line.indexOf(':'); + if (colon < 0) continue; + const key = line.slice(0, colon).trim(); + let value = line.slice(colon + 1).trim(); + if (/^".*"$/.test(value)) { + try { value = JSON.parse(value); } catch { /* leave as-is */ } + } else if (/^-?\d+$/.test(value)) { + value = Number(value); + } + out[key] = value; + } + return out; +} + +/** + * Return all snapshot files for `slug`, sorted oldest → newest. + */ +function listSnapshotsForSlug(slug, cwd) { + const dir = getCritiqueDir(cwd); + if (!fs.existsSync(dir)) return []; + const suffix = `__${slug}.md`; + return fs.readdirSync(dir) + .filter((f) => f.endsWith(suffix)) + .sort() + .map((f) => path.join(dir, f)); +} + +/** + * 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 the last `limit` snapshots' frontmatter, oldest → newest. + * 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 slice = all.slice(-limit); + return slice.map((file) => parseFrontmatter(fs.readFileSync(file, 'utf-8'))); +} + +// ---- CLI --------------------------------------------------------------- + +function main(argv) { + const [cmd, ...args] = argv; + switch (cmd) { + case 'slug': { + const slug = slugFromTarget(args[0]); + if (!slug) { process.stderr.write('no stable slug for input\n'); process.exit(1); } + process.stdout.write(`${slug}\n`); + return; + } + case 'write': { + const [slug, bodyFile] = args; + if (!slug || !bodyFile) { process.stderr.write('usage: write \n'); process.exit(1); } + const raw = fs.readFileSync(bodyFile, 'utf-8'); + // The body file may be a full report. The caller passes the meta as + // a JSON object on stdin if it wants structured frontmatter; otherwise + // we write with minimal metadata. + let meta = {}; + const metaArg = process.env.IMPECCABLE_CRITIQUE_META; + if (metaArg) { + try { meta = JSON.parse(metaArg); } catch { /* ignore */ } + } + const out = writeSnapshot({ slug, meta, body: raw }); + process.stdout.write(`${out}\n`); + return; + } + case 'latest': { + const latest = readLatestSnapshot(args[0]); + if (!latest) { process.exit(2); } + process.stdout.write(latest.body); + return; + } + case 'trend': { + const rows = readTrend(args[0], { limit: args[1] ? Number(args[1]) : 5 }); + process.stdout.write(JSON.stringify(rows, null, 2) + '\n'); + return; + } + default: + process.stderr.write('usage: critique-storage.mjs [args]\n'); + process.exit(1); + } +} + +if (import.meta.url === `file://${process.argv[1]}`) { + main(process.argv.slice(2)); +} diff --git a/skill/scripts/impeccable-paths.mjs b/skill/scripts/impeccable-paths.mjs index ba852bae9..6befa9cd3 100644 --- a/skill/scripts/impeccable-paths.mjs +++ b/skill/scripts/impeccable-paths.mjs @@ -3,6 +3,7 @@ import path from 'node:path'; export const IMPECCABLE_DIR = '.impeccable'; export const LIVE_DIR = 'live'; +export const CRITIQUE_DIR = 'critique'; export function getImpeccableDir(cwd = process.cwd()) { return path.join(cwd, IMPECCABLE_DIR); @@ -96,6 +97,10 @@ export function getLiveAnnotationsDir(cwd = process.cwd()) { return path.join(getLiveDir(cwd), 'annotations'); } +export function getCritiqueDir(cwd = process.cwd()) { + return path.join(getImpeccableDir(cwd), CRITIQUE_DIR); +} + export function getLegacyLiveAnnotationsDir(cwd = process.cwd()) { return path.join(cwd, '.impeccable-live', 'annotations'); } diff --git a/tests/critique-storage.test.mjs b/tests/critique-storage.test.mjs new file mode 100644 index 000000000..6524cf950 --- /dev/null +++ b/tests/critique-storage.test.mjs @@ -0,0 +1,173 @@ +/** + * Tests for critique snapshot persistence. + * Run with: node --test tests/critique-storage.test.mjs + */ + +import { describe, it, beforeEach, afterEach } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; + +import { + slugFromTarget, + writeSnapshot, + readLatestSnapshot, + readTrend, + nowFilenameStamp, +} from '../skill/scripts/critique-storage.mjs'; + +let cwd; +beforeEach(() => { cwd = mkdtempSync(join(tmpdir(), 'imp-critique-')); }); +afterEach(() => { rmSync(cwd, { recursive: true, force: true }); }); + +describe('slugFromTarget', () => { + it('kebabs a relative file path', () => { + assert.equal(slugFromTarget('site/pages/index.astro', { cwd }), 'site-pages-index-astro'); + }); + + it('kebabs an absolute path inside cwd by relativizing', () => { + const abs = join(cwd, 'site/pages/index.astro'); + assert.equal(slugFromTarget(abs, { cwd }), 'site-pages-index-astro'); + }); + + it('uses basename for absolute paths outside cwd', () => { + // Sibling path, not under cwd + const abs = join(tmpdir(), 'somewhere', 'else', 'page.html'); + assert.equal(slugFromTarget(abs, { cwd }), 'page-html'); + }); + + it('drops port from URL', () => { + assert.equal(slugFromTarget('http://localhost:3000/pricing', { cwd }), 'localhost-pricing'); + }); + + it('normalizes URL casing and trailing slash', () => { + assert.equal( + slugFromTarget('https://Impeccable.Style/docs/audit/', { cwd }), + 'impeccable-style-docs-audit', + ); + }); + + it('strips query strings', () => { + assert.equal( + slugFromTarget('https://example.com/x?utm=1&foo=bar', { cwd }), + 'example-com-x', + ); + }); + + it('returns null for empty / project-root inputs', () => { + assert.equal(slugFromTarget('', { cwd }), null); + assert.equal(slugFromTarget('.', { cwd }), null); + assert.equal(slugFromTarget(null, { cwd }), null); + }); + + it('caps overly long slugs from the tail', () => { + const longPath = 'a/'.repeat(60) + 'file.tsx'; // way over 50 + const slug = slugFromTarget(longPath, { cwd }); + assert.ok(slug.length <= 50); + assert.ok(slug.endsWith('file-tsx')); + }); + + it('is stable: same input → same slug', () => { + const a = slugFromTarget('site/pages/index.astro', { cwd }); + const b = slugFromTarget('site/pages/index.astro', { cwd }); + assert.equal(a, b); + }); +}); + +describe('nowFilenameStamp', () => { + it('is windows-safe (no colons or dots in the time fragment)', () => { + const stamp = nowFilenameStamp(new Date('2026-05-12T18:30:00.123Z')); + assert.equal(stamp, '2026-05-12T18-30-00Z'); + }); +}); + +describe('writeSnapshot + readLatestSnapshot', () => { + it('round-trips body and frontmatter', () => { + const out = writeSnapshot({ + slug: 'index-astro', + meta: { target: 'the homepage', total_score: 28, p0_count: 1, p1_count: 3 }, + body: '# Critique\n\nP0: nested cards', + cwd, + }); + assert.ok(out.endsWith('__index-astro.md')); + const latest = readLatestSnapshot('index-astro', { cwd }); + assert.equal(latest.meta.slug, 'index-astro'); + assert.equal(latest.meta.target, 'the homepage'); + assert.equal(latest.meta.total_score, 28); + assert.match(latest.body, /P0: nested cards/); + }); + + it('returns null when no snapshot for slug', () => { + assert.equal(readLatestSnapshot('nope', { cwd }), null); + }); + + it('picks the newest by filename when multiple exist', () => { + writeSnapshot({ slug: 'index-astro', meta: { total_score: 22 }, body: 'old', cwd, now: new Date('2026-05-01T00:00:00Z') }); + writeSnapshot({ slug: 'index-astro', meta: { total_score: 30 }, body: 'new', cwd, now: new Date('2026-05-12T00:00:00Z') }); + const latest = readLatestSnapshot('index-astro', { cwd }); + assert.equal(latest.meta.total_score, 30); + 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); + }); + + it('caller-supplied meta cannot override computed timestamp or slug', () => { + // Defends against a corrupt IMPECCABLE_CRITIQUE_META blob (parsed from + // an env var) silently rewriting fields that must agree with the + // filename. Otherwise readTrend would attribute scores to the wrong + // timestamps with no error. + const out = writeSnapshot({ + slug: 'index-astro', + meta: { timestamp: 'NOT_A_REAL_STAMP', slug: 'somewhere-else', total_score: 50 }, + body: 'b', + cwd, + now: new Date('2026-05-12T18:30:00Z'), + }); + const latest = readLatestSnapshot('index-astro', { cwd }); + assert.equal(latest.meta.slug, 'index-astro'); + assert.equal(latest.meta.timestamp, '2026-05-12T18-30-00Z'); + // The legit meta field still lands. + assert.equal(latest.meta.total_score, 50); + // The filename matches the computed slug. + assert.ok(out.endsWith('2026-05-12T18-30-00Z__index-astro.md')); + }); + + it('quotes values containing : or # to keep parsing simple', () => { + writeSnapshot({ + slug: 'x', + meta: { target: 'docs: critique # main' }, + body: '...', + cwd, + }); + const latest = readLatestSnapshot('x', { cwd }); + assert.equal(latest.meta.target, 'docs: critique # main'); + }); +}); + +describe('readTrend', () => { + it('returns last N entries oldest → newest, filtered by slug', () => { + for (let i = 0; i < 6; i++) { + writeSnapshot({ + slug: 'index-astro', + meta: { total_score: 20 + i }, + body: `run ${i}`, + cwd, + now: new Date(2026, 4, i + 1), + }); + } + writeSnapshot({ slug: 'pricing-astro', meta: { total_score: 99 }, body: 'unrelated', cwd }); + const trend = readTrend('index-astro', { limit: 5, cwd }); + assert.equal(trend.length, 5); + assert.equal(trend[0].total_score, 21); // dropped the oldest + assert.equal(trend[4].total_score, 25); + }); + + it('returns empty when no snapshots', () => { + assert.deepEqual(readTrend('nope', { cwd }), []); + }); +}); +