mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-04 02:32:07 +01:00
feat: maintain Storybook through a downstream feature catalog (#16046)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Storybook supports review of its production UI and proposed designs. > - Its 223 story files have no consistent link to product capabilities or record of component and mock provenance. > - Old compositions can drift while they still look like current product examples. > - This pull request adds two maintenance skills and a catalog that consumes the existing feature map. > - The first audit preserves all stories and fixes proven fixture drift. > - Contributors can now find examples by feature and see what still needs review. ## Linked Issues or Issue Description **What existing behavior does this improve?** Storybook maintenance, feature discovery, and the accuracy of isolated UI examples. **Current behavior** Stories are already centralized under `ui/storybook/`, but their sidebar has 51 top-level groups. Production components, local prototypes, and historical designs are not consistently distinguished. The old guide uses manual coverage claims. GitHub searches found no duplicate feature-map catalog change. **Proposed behavior** Keep existing story locations and IDs. Add a downstream catalog keyed to stable feature capabilities. Record product status, rendered subject, data boundary, and declared review dependencies separately. Use dedicated maintenance and review skills to reconcile drift and report evidence. **Reason and benefit** The catalog makes existing examples reusable without treating mock behavior or old designs as production acceptance. The feature map remains independent of Storybook. ## What Changed - Add `maintain-storybook` and `review-storybook` skills with contributor guidance. - Inventory all 223 story files. Generate a feature index and provide searchable navigation in the existing Overview story. Track direct subjects through relative imports and local wrappers, including the primitives demonstrated by foundation suites. - Add a checker for inventory, capability references, duplicate IDs, and declared-source drift, with eight regression tests. The parser test covers real nested CSF files, export filters, explicit IDs, and play counts. - Preserve historical designs and classify mixed wrappers. Record the initial audit and remaining parity work. - Align the question-dismiss/send fixture with production behavior. Add dismissal and confirmation-reopen regressions and repair the editor selector. - Exclude three exported fast-response fixtures from story discovery and add the missing render-time callback spy. ## Verification - Passed: `node scripts/storybook-catalog.mjs`, `node --test scripts/storybook-catalog.test.mjs` (8 tests), and `node scripts/check-maps.mjs`. - Passed: validators for both new skills, `pnpm check:token-gates`, `pnpm -r typecheck`, and `pnpm build`. - Passed: final Storybook build; nine selected browser `play` cases; three fast-response render cases; feature-guide search/link navigation; question-fixture reset on reload. - Built-index comparison preserved all 2,208 original story IDs and added only two new regression cases. - Passed: `pnpm exec vitest run --project @paperclipai/ui` (728 files, 8,176 tests). - The full `pnpm test:run` has reported a 15-second timeout in the untouched backend case `connects advertised Workspace gmail.read without mutating definitions in authenticated`. The exact test passed standalone without changing code, expectations, or timeouts. The remaining broad run was stopped after this failure. The full suite is not verified. - Visual comparison is blocked by the existing empty baseline manifest. The managed-accounts API/DB fixture was not started. See `maps/storybook-map/REVIEW.md` for exact scope. ## Risks - This is an initial inventory and targeted audit. 188 suites still require deeper state and fixture parity review. - Review hashes cover declared dependencies. They do not follow all transitive production imports or prove semantic correctness. - The catalog requires upkeep when stories, mapped recipes, or declared subjects change. Shared mocks still need feature-by-feature contract review. - No production application source, database schema, or upstream feature-map contract changes. ## Model Used OpenAI GPT-6 via Codex. Used reasoning, shell tools, file editing, and Chromium browser verification. The exact runtime model ID and context window are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [ ] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
DottaandPaperclip authored and GitHub committed
1 parent
f5346c05b9
commit
9e721264e9
19 files changed
+6445
-192
No files matched your search
@@ -0,0 +1,134 @@
|
||||
#!/usr/bin/env node
|
||||
import { createHash } from "node:crypto";
|
||||
import { existsSync, readFileSync, readdirSync, writeFileSync } from "node:fs";
|
||||
import { createRequire } from "node:module";
|
||||
import path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
|
||||
const scriptPath = fileURLToPath(import.meta.url);
|
||||
const defaultRoot = path.resolve(path.dirname(scriptPath), "..");
|
||||
const storyRoot = "ui/storybook/stories";
|
||||
const catalogPath = "maps/storybook-map/catalog.json";
|
||||
const indexPath = "maps/storybook-map/INDEX.md";
|
||||
const statuses = ["current", "proposed", "historical", "mixed", "unreviewed"];
|
||||
const implementations = ["production", "prototype", "mixed", "guide"];
|
||||
const boundaries = ["fixtures", "fixture-server", "static"];
|
||||
|
||||
function files(root, dir) {
|
||||
return readdirSync(path.join(root, dir), { withFileTypes: true }).flatMap(entry => {
|
||||
const name = `${dir}/${entry.name}`;
|
||||
return entry.isDirectory() ? files(root, name) : [name];
|
||||
}).sort();
|
||||
}
|
||||
|
||||
export function fingerprint(root, sources) {
|
||||
const hash = createHash("sha256");
|
||||
for (const file of [...new Set(sources)].sort()) {
|
||||
hash.update(file).update("\0").update(readFileSync(path.join(root, file))).update("\0");
|
||||
}
|
||||
return hash.digest("hex");
|
||||
}
|
||||
|
||||
export function capabilities(root) {
|
||||
const result = new Map();
|
||||
for (const file of files(root, "maps/feature-map").filter(file => file.endsWith(".md") && !file.endsWith("README.md"))) {
|
||||
const text = readFileSync(path.join(root, file), "utf8");
|
||||
const area = path.basename(file, ".md");
|
||||
const section = text.split(/^## Sub-features\s*$/m)[1]?.split(/^## /m)[0] ?? "";
|
||||
for (const match of section.matchAll(/^- `([^`]+)`:/gm)) result.set(`${area}/${match[1]}`, file);
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
export function scanStories(root = defaultRoot) {
|
||||
const require = createRequire(path.join(root, "ui/package.json"));
|
||||
const { loadCsf } = require("storybook/internal/csf-tools");
|
||||
return files(root, storyRoot).filter(file => /\.stories\.(ts|tsx|mdx)$/.test(file)).map(file => {
|
||||
const csf = loadCsf(readFileSync(path.join(root, file), "utf8"), { fileName: file, makeTitle: title => title }).parse();
|
||||
return { file, title: csf.meta.title, ids: csf.stories.map(story => story.id), plays: csf.stories.filter(story => story.__stats?.play).length };
|
||||
});
|
||||
}
|
||||
|
||||
export function validateCatalog(root, catalog, stories) {
|
||||
const errors = [];
|
||||
const refs = capabilities(root);
|
||||
const byFile = new Map(stories.map(story => [story.file, story]));
|
||||
const seen = new Set();
|
||||
const local = file => typeof file === "string" && !path.isAbsolute(file) && !file.split("/").includes("..") && existsSync(path.join(root, file));
|
||||
if (catalog.version !== 1 || !Array.isArray(catalog.entries)) return ["Unsupported catalog version or missing entries"];
|
||||
function review(label, sources, hash) {
|
||||
if (!Array.isArray(sources) || !sources.length || sources.some(file => !local(file))) {
|
||||
errors.push(`${label}: missing/invalid review sources`);
|
||||
} else if (fingerprint(root, sources) !== hash) errors.push(`${label}: source changed; review before updating its fingerprint`);
|
||||
}
|
||||
review("Shared Storybook setup", catalog.sharedReview?.sources, catalog.sharedReview?.fingerprint);
|
||||
for (const entry of catalog.entries) {
|
||||
const label = entry.file;
|
||||
if (seen.has(label)) errors.push(`${label}: duplicate catalog entry`);
|
||||
seen.add(label);
|
||||
if (!byFile.has(label)) errors.push(`${label}: missing story file`);
|
||||
if (!statuses.includes(entry.status)) errors.push(`${label}: invalid product status`);
|
||||
if (!implementations.includes(entry.implementation)) errors.push(`${label}: invalid implementation`);
|
||||
if (!boundaries.includes(entry.boundary)) errors.push(`${label}: invalid data boundary`);
|
||||
if (!entry.purpose?.trim() || !entry.notes?.trim()) errors.push(`${label}: purpose and review notes are required`);
|
||||
if (!/^\d{4}-\d{2}-\d{2}$/.test(entry.reviewedAt ?? "")) errors.push(`${label}: missing review date`);
|
||||
if (!Array.isArray(entry.capabilities)) { errors.push(`${label}: missing capabilities`); continue; }
|
||||
for (const ref of entry.capabilities) if (!refs.has(ref)) errors.push(`${label}: unknown feature capability ${ref}`);
|
||||
if (!entry.capabilities.length && !entry.exception?.trim()) errors.push(`${label}: unmapped story needs an explicit reason`);
|
||||
if (entry.capabilities.length !== new Set(entry.capabilities).size) errors.push(`${label}: duplicate capability references`);
|
||||
if (entry.replacement && !byFile.has(entry.replacement)) errors.push(`${label}: replacement story is missing`);
|
||||
if (!entry.sources?.includes(label)) errors.push(`${label}: review must include its story source`);
|
||||
review(label, entry.sources, entry.fingerprint);
|
||||
}
|
||||
for (const story of stories) if (!seen.has(story.file)) errors.push(`${story.file}: story missing from catalog`);
|
||||
const ids = stories.flatMap(story => story.ids);
|
||||
if (ids.length !== new Set(ids).size) errors.push("Duplicate story IDs");
|
||||
return errors;
|
||||
}
|
||||
|
||||
function cell(value) { return String(value).replaceAll("|", "\\|").replaceAll("\n", " "); }
|
||||
|
||||
export function renderIndex(catalog, stories, refs) {
|
||||
const byFile = new Map(stories.map(story => [story.file, story]));
|
||||
const groups = new Map();
|
||||
for (const entry of catalog.entries) {
|
||||
const area = entry.capabilities[0]?.split("/")[0] ?? "foundations-and-guides";
|
||||
if (!groups.has(area)) groups.set(area, []);
|
||||
groups.get(area).push(entry);
|
||||
}
|
||||
const lines = ["# Storybook by feature", "", "<!-- Generated by node scripts/storybook-catalog.mjs --write. Edit catalog.json. -->", "",
|
||||
"[Contract and maintenance](README.md) · [Review findings](REVIEW.md)", "",
|
||||
`${stories.length} story files; ${stories.reduce((n, story) => n + story.ids.length, 0)} indexed stories. Counts describe inventory, not passed tests or complete feature coverage.`, "",
|
||||
"Status describes product alignment; implementation describes the rendered subject; boundary describes simulated data. Sources and limitations are in catalog.json. Related capabilities cross-list one canonical story file.", ""];
|
||||
for (const [area, entries] of [...groups].sort(([a], [b]) => a.localeCompare(b))) {
|
||||
lines.push(`## ${area}`, "", ...(area !== "foundations-and-guides" ? [`[Feature recipe](../feature-map/${area}.md)`, ""] : []),
|
||||
"| Story suite | Capabilities | Product / implementation / data | Stories / play functions | Review note |",
|
||||
"| --- | --- | --- | --- | --- |");
|
||||
for (const entry of entries.sort((a, b) => a.file.localeCompare(b.file))) {
|
||||
const story = byFile.get(entry.file);
|
||||
lines.push(`| [${cell(story.title)}](../../${entry.file}) | ${entry.capabilities.map(ref => `\`${ref}\``).join(", ") || cell(entry.exception)} | ${entry.status} / ${entry.implementation} / ${entry.boundary} | ${story.ids.length} / ${story.plays} | ${cell(entry.notes)} |`);
|
||||
}
|
||||
lines.push("");
|
||||
}
|
||||
lines.push("## Capability lookup", "", "References are navigation links, not proof of parity. An absent link means no cataloged story, not a missing product feature. CLI/operator capabilities may need no Storybook example.", "");
|
||||
for (const ref of refs.keys()) {
|
||||
const entries = catalog.entries.filter(entry => entry.capabilities.includes(ref));
|
||||
lines.push(`- \`${ref}\`: ${entries.length ? entries.map(entry => `[${cell(byFile.get(entry.file).title)}](../../${entry.file}) (${entry.status})`).join(", ") : "No cataloged story."}`);
|
||||
}
|
||||
return lines.join("\n") + "\n";
|
||||
}
|
||||
|
||||
if (process.argv[1] && path.resolve(process.argv[1]) === scriptPath) {
|
||||
const args = process.argv.slice(2);
|
||||
if (args.some(arg => arg !== "--write")) throw new Error("Usage: node scripts/storybook-catalog.mjs [--write]");
|
||||
const catalog = JSON.parse(readFileSync(path.join(defaultRoot, catalogPath), "utf8"));
|
||||
const stories = scanStories();
|
||||
const errors = validateCatalog(defaultRoot, catalog, stories);
|
||||
if (!errors.length) {
|
||||
const generated = renderIndex(catalog, stories, capabilities(defaultRoot));
|
||||
if (args.includes("--write")) writeFileSync(path.join(defaultRoot, indexPath), generated);
|
||||
else if (!existsSync(path.join(defaultRoot, indexPath)) || readFileSync(path.join(defaultRoot, indexPath), "utf8") !== generated) errors.push("Generated INDEX.md is stale; run with --write after reviewing catalog changes");
|
||||
}
|
||||
if (errors.length) { errors.forEach(error => console.error(error)); process.exitCode = 1; }
|
||||
else console.log(`Storybook catalog aligned: ${stories.length} files, ${stories.reduce((n, story) => n + story.ids.length, 0)} stories. Structure and review fingerprints only; no runtime proof.`);
|
||||
}
|
||||
@@ -0,0 +1,114 @@
|
||||
import assert from "node:assert/strict";
|
||||
import { mkdtempSync, mkdirSync, writeFileSync, rmSync, symlinkSync } from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { test } from "node:test";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { capabilities, fingerprint, renderIndex, scanStories, validateCatalog } from "./storybook-catalog.mjs";
|
||||
|
||||
function fixture(t) {
|
||||
const root = mkdtempSync(path.join(os.tmpdir(), "storybook-catalog-"));
|
||||
t.after(() => rmSync(root, { recursive: true, force: true }));
|
||||
function write(file, text) {
|
||||
mkdirSync(path.dirname(path.join(root, file)), { recursive: true });
|
||||
writeFileSync(path.join(root, file), text);
|
||||
}
|
||||
write("maps/feature-map/tasks.md", "# Tasks\n\n## Sub-features\n\n- `create`: create\n- `discussion`: discuss\n\n## How to get to it\n");
|
||||
write("ui/storybook/.storybook/preview.tsx", "// shared fixtures\n");
|
||||
const file = "ui/storybook/stories/task.stories.tsx";
|
||||
write(file, "// a story\n");
|
||||
write("ui/src/Task.tsx", "// production subject\n");
|
||||
const sharedSources = ["ui/storybook/.storybook/preview.tsx"];
|
||||
const sources = [file, "ui/src/Task.tsx"];
|
||||
const catalog = {
|
||||
version: 1, sharedReview: { sources: sharedSources, fingerprint: fingerprint(root, sharedSources) },
|
||||
entries: [{ file, capabilities: ["tasks/create", "tasks/discussion"], purpose: "Create a task", status: "current",
|
||||
implementation: "production", boundary: "fixtures", notes: "Production subject; callbacks simulated.",
|
||||
reviewedAt: "2026-10-12", sources, fingerprint: fingerprint(root, sources) }],
|
||||
};
|
||||
const stories = [{ file, title: "Tasks/Create", ids: ["tasks-create--empty"], plays: 0 }];
|
||||
return { root, write, file, catalog, stories };
|
||||
}
|
||||
|
||||
test("complete inventory accepts production subjects with simulated data", t => {
|
||||
const f = fixture(t);
|
||||
assert.deepEqual(validateCatalog(f.root, f.catalog, f.stories), []);
|
||||
});
|
||||
|
||||
test("adding, removing or duplicating story files requires reconciliation", t => {
|
||||
const f = fixture(t);
|
||||
assert.match(validateCatalog(f.root, { ...f.catalog, entries: [] }, f.stories).join("\n"), /missing from catalog/);
|
||||
assert.match(validateCatalog(f.root, f.catalog, []).join("\n"), /missing story file/);
|
||||
f.catalog.entries.push(f.catalog.entries[0]);
|
||||
assert.match(validateCatalog(f.root, f.catalog, f.stories).join("\n"), /duplicate catalog entry/);
|
||||
});
|
||||
|
||||
test("capability renames and unjustified unmapped stories fail", t => {
|
||||
const f = fixture(t);
|
||||
f.catalog.entries[0].capabilities = ["tasks/old-name"];
|
||||
assert.match(validateCatalog(f.root, f.catalog, f.stories).join("\n"), /unknown feature capability/);
|
||||
f.catalog.entries[0].capabilities = [];
|
||||
assert.match(validateCatalog(f.root, f.catalog, f.stories).join("\n"), /explicit reason/);
|
||||
f.catalog.entries[0].exception = "Shared foundation";
|
||||
assert.deepEqual(validateCatalog(f.root, f.catalog, f.stories), []);
|
||||
});
|
||||
|
||||
test("production and shared fixture drift invalidate review fingerprints", t => {
|
||||
const f = fixture(t);
|
||||
f.write("ui/src/Task.tsx", "// changed contract\n");
|
||||
f.write("ui/storybook/.storybook/preview.tsx", "// changed shared mocks\n");
|
||||
const errors = validateCatalog(f.root, f.catalog, f.stories);
|
||||
assert.equal(errors.filter(error => error.includes("source changed")).length, 2);
|
||||
assert.equal(f.catalog.entries[0].fingerprint.length, 64);
|
||||
});
|
||||
|
||||
test("missing and escaping source references cannot certify a review", t => {
|
||||
const f = fixture(t);
|
||||
f.catalog.entries[0].sources.push("../outside.ts");
|
||||
assert.match(validateCatalog(f.root, f.catalog, f.stories).join("\n"), /invalid review sources/);
|
||||
});
|
||||
|
||||
test("cross-capability lookup retains one canonical file and honest status", t => {
|
||||
const f = fixture(t);
|
||||
f.catalog.entries[0].status = "historical";
|
||||
const index = renderIndex(f.catalog, f.stories, capabilities(f.root));
|
||||
assert.match(index, /tasks\/create.*\(historical\)/);
|
||||
assert.match(index, /tasks\/discussion.*\(historical\)/);
|
||||
assert.match(index, /1 story files; 1 indexed stories/);
|
||||
assert.match(index, /historical \/ production \/ fixtures/);
|
||||
});
|
||||
|
||||
test("duplicate story IDs cannot silently collapse distinct examples", t => {
|
||||
const f = fixture(t);
|
||||
f.stories[0].ids.push(f.stories[0].ids[0]);
|
||||
assert.match(validateCatalog(f.root, f.catalog, f.stories).join("\n"), /Duplicate story IDs/);
|
||||
});
|
||||
|
||||
test("Storybook parser discovers nested files, filters helpers, preserves explicit IDs and counts play functions", t => {
|
||||
const f = fixture(t);
|
||||
f.write("ui/package.json", '{"type":"module"}');
|
||||
symlinkSync(fileURLToPath(new URL("../ui/node_modules", import.meta.url)), path.join(f.root, "ui/node_modules"), "junction");
|
||||
f.write(f.file, `
|
||||
import type { Meta, StoryObj } from "@storybook/react-vite";
|
||||
const meta = { title: "Tasks/Create", id: "stable-task", excludeStories: ["empty"] } satisfies Meta;
|
||||
export default meta;
|
||||
type Story = StoryObj<typeof meta>;
|
||||
export const empty = { value: "fixture" };
|
||||
export const Empty: Story = {};
|
||||
export const Filled: Story = { play: async () => {} };
|
||||
`);
|
||||
f.write("ui/storybook/stories/nested/other.stories.ts", `
|
||||
export default { title: "Other/Subject", includeStories: ["Included"] };
|
||||
export const helper = {};
|
||||
export const Included = {};
|
||||
`);
|
||||
f.write("ui/storybook/stories/nested/fixture.ts", "export const fixture = {};");
|
||||
const stories = scanStories(f.root);
|
||||
assert.equal(stories.length, 2);
|
||||
assert.deepEqual(stories.find(story => story.file === f.file), {
|
||||
file: f.file, title: "Tasks/Create", ids: ["stable-task--empty", "stable-task--filled"], plays: 1,
|
||||
});
|
||||
assert.deepEqual(stories.find(story => story.file.endsWith("other.stories.ts")), {
|
||||
file: "ui/storybook/stories/nested/other.stories.ts", title: "Other/Subject", ids: ["other-subject--included"], plays: 0,
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user