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 }), []); + }); +}); +