From 0930beb099a19b2fe33c83caa8a2aba2a416302f Mon Sep 17 00:00:00 2001 From: Matt Van Horn Date: Tue, 26 May 2026 21:27:34 -0700 Subject: [PATCH] fix: preserve every generated design variant in plan-design-review --- design/src/cli.ts | 227 ++++++++++++++++++++++- design/test/variant-preservation.test.ts | 125 +++++++++++++ 2 files changed, 345 insertions(+), 7 deletions(-) create mode 100644 design/test/variant-preservation.test.ts diff --git a/design/src/cli.ts b/design/src/cli.ts index a55d92555..046801c0a 100644 --- a/design/src/cli.ts +++ b/design/src/cli.ts @@ -32,6 +32,8 @@ import { shutdownDaemon, } from "./daemon-client"; import { spawn as nodeSpawn } from "child_process"; +import fs from "fs"; +import path from "path"; function parseArgs(argv: string[]): { command: string; @@ -121,8 +123,8 @@ async function runSetup(): Promise { } } -async function main(): Promise { - const { command, flags, positionals } = parseArgs(process.argv); +async function main(argv = process.argv): Promise { + const { command, flags, positionals } = parseArgs(argv); if (!COMMANDS.has(command)) { console.error(`Unknown command: ${command}`); @@ -132,7 +134,7 @@ async function main(): Promise { switch (command) { case "generate": - await generate({ + await generateWithRoundArtifacts({ brief: flags.brief as string, briefFile: flags["brief-file"] as string, output: (flags.output as string) || "/tmp/gstack-mockup.png", @@ -206,7 +208,7 @@ async function main(): Promise { break; case "iterate": - await iterate({ + await iterateWithRoundArtifacts({ session: flags.session as string, feedback: flags.feedback as string, output: (flags.output as string) || "/tmp/gstack-iterate.png", @@ -318,6 +320,129 @@ async function main(): Promise { } } +const ROUND_MANIFEST = ".gstack-design-rounds.json"; + +interface RoundAttempt { + label: string; + path: string; + success: boolean; + error?: string; +} + +type RoundManifest = Record; + +interface RoundArtifactPlan { + aliasOutput: string; + primaryOutput: string; + roundKey: string; + label: string; +} + +function roundBaseName(outputPath: string): string | null { + const parsed = path.parse(outputPath); + if (parsed.ext !== ".png") return null; + if (parsed.name === "variant-recommended") return parsed.name; + if (/^variant-iteration-\d+$/.test(parsed.name)) return parsed.name; + return null; +} + +function labelForIndex(index: number): string { + const alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; + if (index < alphabet.length) return alphabet[index]; + return `${index + 1}`; +} + +function roundKey(outputPath: string, baseName: string): string { + return path.join(path.dirname(outputPath), baseName); +} + +function readRoundManifest(dir: string): RoundManifest { + const manifestPath = path.join(dir, ROUND_MANIFEST); + if (!fs.existsSync(manifestPath)) return {}; + try { + return JSON.parse(fs.readFileSync(manifestPath, "utf-8")) as RoundManifest; + } catch { + return {}; + } +} + +function writeRoundManifest(dir: string, manifest: RoundManifest): void { + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, ROUND_MANIFEST), JSON.stringify(manifest, null, 2)); +} + +function planRoundArtifacts(outputPath: string): RoundArtifactPlan | null { + const baseName = roundBaseName(outputPath); + if (!baseName) return null; + + const dir = path.dirname(outputPath); + const manifest = readRoundManifest(dir); + const key = roundKey(outputPath, baseName); + const attempts = manifest[key] || []; + const label = labelForIndex(attempts.length); + + return { + aliasOutput: outputPath, + primaryOutput: path.join(dir, `${baseName}-${label}.png`), + roundKey: key, + label, + }; +} + +function recordRoundAttempt(plan: RoundArtifactPlan, success: boolean, error?: string): void { + const dir = path.dirname(plan.aliasOutput); + const manifest = readRoundManifest(dir); + const attempts = manifest[plan.roundKey] || []; + const existing = attempts.find(attempt => attempt.label === plan.label); + const attempt = { label: plan.label, path: plan.primaryOutput, success, error }; + if (existing) { + Object.assign(existing, attempt); + } else { + attempts.push(attempt); + } + manifest[plan.roundKey] = attempts; + writeRoundManifest(dir, manifest); +} + +function copyRoundAlias(plan: RoundArtifactPlan): void { + if (plan.primaryOutput === plan.aliasOutput) return; + fs.copyFileSync(plan.primaryOutput, plan.aliasOutput); +} + +async function generateWithRoundArtifacts(options: Parameters[0]): Promise { + const plan = planRoundArtifacts(options.output); + if (!plan) { + await generate(options); + return; + } + + try { + await generate({ ...options, output: plan.primaryOutput }); + recordRoundAttempt(plan, true); + copyRoundAlias(plan); + } catch (err: any) { + recordRoundAttempt(plan, false, err.message || String(err)); + throw err; + } +} + +async function iterateWithRoundArtifacts(options: Parameters[0]): Promise { + const plan = planRoundArtifacts(options.output); + if (!plan) { + await iterate(options); + return; + } + + try { + await iterate({ ...options, output: plan.primaryOutput }); + recordRoundAttempt(plan, true); + copyRoundAlias(plan); + } catch (err: any) { + recordRoundAttempt(plan, false, err.message || String(err)); + throw err; + } +} + /** * Default `$D compare --serve` path: ensure the persistent daemon is up, * publish the board, open the browser to its URL, then exit. The daemon @@ -400,7 +525,88 @@ async function resolveImagePaths(input: string): Promise { } // Comma-separated or single path - return input.split(",").map(p => p.trim()); + const resolved: string[] = []; + const missing: string[] = []; + for (const imagePath of input.split(",").map(p => p.trim()).filter(Boolean)) { + const roundImages = resolveRoundImageAlias(imagePath); + if (roundImages) { + resolved.push(...roundImages.paths); + missing.push(...roundImages.missing); + } else { + resolved.push(imagePath); + if (!fs.existsSync(imagePath)) missing.push(path.basename(imagePath)); + } + } + + if (missing.length > 0) { + throw new Error(`Missing generated design variants: ${missing.join(", ")}`); + } + + return resolved; +} + +function resolveRoundImageAlias(imagePath: string): { paths: string[]; missing: string[] } | null { + const baseName = roundBaseName(imagePath); + if (!baseName) return null; + + const dir = path.dirname(imagePath); + const manifest = readRoundManifest(dir); + const attempts = manifest[roundKey(imagePath, baseName)] || []; + + if (attempts.length > 0) { + return { + paths: attempts.filter(attempt => attempt.success && fs.existsSync(attempt.path)).map(attempt => attempt.path), + missing: attempts + .filter(attempt => !attempt.success || !fs.existsSync(attempt.path)) + .map(attempt => `${baseName}-${attempt.label}.png`), + }; + } + + const discovered = discoverRoundImages(dir, baseName); + if (discovered.paths.length > 0 || discovered.missing.length > 0) { + return discovered; + } + + if (fs.existsSync(imagePath)) { + return { paths: [imagePath], missing: [] }; + } + + return { paths: [], missing: [path.basename(imagePath)] }; +} + +function discoverRoundImages(dir: string, baseName: string): { paths: string[]; missing: string[] } { + if (!fs.existsSync(dir)) return { paths: [], missing: [] }; + + const matches = fs.readdirSync(dir) + .map(name => { + const match = name.match(new RegExp(`^${escapeRegExp(baseName)}-([A-Z])\\.png$`)); + return match ? { label: match[1], name } : null; + }) + .filter((match): match is { label: string; name: string } => match !== null) + .sort((a, b) => a.label.localeCompare(b.label)); + + if (matches.length === 0) return { paths: [], missing: [] }; + + const highestIndex = matches[matches.length - 1].label.charCodeAt(0) - 65; + const byLabel = new Map(matches.map(match => [match.label, match.name])); + const paths: string[] = []; + const missing: string[] = []; + + for (let i = 0; i <= highestIndex; i++) { + const label = labelForIndex(i); + const name = byLabel.get(label); + if (name) { + paths.push(path.join(dir, name)); + } else { + missing.push(`${baseName}-${label}.png`); + } + } + + return { paths, missing }; +} + +function escapeRegExp(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); } // Self-execution shortcut: when invoked with --daemon-mode, this same @@ -408,14 +614,21 @@ async function resolveImagePaths(input: string): Promise { // the production install to a single executable; daemon-client.ts spawns // ` --daemon-mode` (or `bun run cli.ts --daemon-mode` in dev) // rather than relying on a separate daemon.ts file at a known path. -if (process.argv.includes("--daemon-mode")) { +if (import.meta.main && process.argv.includes("--daemon-mode")) { const { start } = await import("./daemon"); start(); // start() binds Bun.serve and registers signal handlers; this branch // never falls through to main(). Process stays alive on the bound port. -} else { +} else if (import.meta.main) { main().catch((err) => { console.error(err.message || err); process.exit(1); }); } + +export { + main, + resolveImagePaths, + planRoundArtifacts, + resolveRoundImageAlias, +}; diff --git a/design/test/variant-preservation.test.ts b/design/test/variant-preservation.test.ts new file mode 100644 index 000000000..da9ed79cd --- /dev/null +++ b/design/test/variant-preservation.test.ts @@ -0,0 +1,125 @@ +import { describe, test, expect, beforeEach, afterEach } from "bun:test"; +import fs from "fs"; +import os from "os"; +import path from "path"; +import { + planRoundArtifacts, + resolveImagePaths, +} from "../src/cli"; + +const PNG_BYTES = Buffer.from( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkAAIAAAoAAv/lxKUAAAAASUVORK5CYII=", + "base64", +); + +function writePng(filePath: string): void { + fs.writeFileSync(filePath, PNG_BYTES); +} + +function writeManifest(tmpDir: string, entries: Record): void { + fs.writeFileSync( + path.join(tmpDir, ".gstack-design-rounds.json"), + JSON.stringify(entries, null, 2), + ); +} + +describe("plan-design-review round variant preservation", () => { + let tmpDir: string; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "variant-preservation-")); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + test("recommended round allocates sequence-stable suffixed paths", () => { + const alias = path.join(tmpDir, "variant-recommended.png"); + const first = planRoundArtifacts(alias); + + expect(first?.primaryOutput).toBe(path.join(tmpDir, "variant-recommended-A.png")); + expect(first?.aliasOutput).toBe(alias); + + writeManifest(tmpDir, { + [path.join(tmpDir, "variant-recommended")]: [ + { label: "A", path: first!.primaryOutput, success: true }, + ], + }); + + const second = planRoundArtifacts(alias); + expect(second?.primaryOutput).toBe(path.join(tmpDir, "variant-recommended-B.png")); + }); + + test("recommended round alias expands to every successful generated candidate", async () => { + const alias = path.join(tmpDir, "variant-recommended.png"); + const variantA = path.join(tmpDir, "variant-recommended-A.png"); + const variantB = path.join(tmpDir, "variant-recommended-B.png"); + writePng(variantA); + writePng(variantB); + writePng(alias); + writeManifest(tmpDir, { + [path.join(tmpDir, "variant-recommended")]: [ + { label: "A", path: variantA, success: true }, + { label: "B", path: variantB, success: true }, + ], + }); + + await expect(resolveImagePaths(alias)).resolves.toEqual([variantA, variantB]); + }); + + test("iteration round discovers A/B/C candidates without collapsing onto the alias", async () => { + const alias = path.join(tmpDir, "variant-iteration-01.png"); + const variantA = path.join(tmpDir, "variant-iteration-01-A.png"); + const variantB = path.join(tmpDir, "variant-iteration-01-B.png"); + const variantC = path.join(tmpDir, "variant-iteration-01-C.png"); + writePng(variantA); + writePng(variantB); + writePng(variantC); + writePng(alias); + + await expect(resolveImagePaths(alias)).resolves.toEqual([variantA, variantB, variantC]); + }); + + test("single recommended generation preserves the suffixed candidate and ignores alias copy", async () => { + const alias = path.join(tmpDir, "variant-recommended.png"); + const variantA = path.join(tmpDir, "variant-recommended-A.png"); + writePng(variantA); + writePng(alias); + writeManifest(tmpDir, { + [path.join(tmpDir, "variant-recommended")]: [ + { label: "A", path: variantA, success: true }, + ], + }); + + await expect(resolveImagePaths(alias)).resolves.toEqual([variantA]); + }); + + test("failed sibling leaves successful candidate in place and reports missing index", async () => { + const alias = path.join(tmpDir, "variant-recommended.png"); + const variantA = path.join(tmpDir, "variant-recommended-A.png"); + const variantB = path.join(tmpDir, "variant-recommended-B.png"); + writePng(variantA); + writeManifest(tmpDir, { + [path.join(tmpDir, "variant-recommended")]: [ + { label: "A", path: variantA, success: true }, + { label: "B", path: variantB, success: false, error: "API error" }, + ], + }); + + await expect(resolveImagePaths(alias)).rejects.toThrow("variant-recommended-B.png"); + expect(fs.existsSync(variantA)).toBe(true); + }); + + test("initial 3-option board paths are unchanged", async () => { + const variantA = path.join(tmpDir, "variant-A.png"); + const variantB = path.join(tmpDir, "variant-B.png"); + const variantC = path.join(tmpDir, "variant-C.png"); + writePng(variantA); + writePng(variantB); + writePng(variantC); + + const input = `${variantA},${variantB},${variantC}`; + await expect(resolveImagePaths(input)).resolves.toEqual([variantA, variantB, variantC]); + }); +});