From 978114a12f5b0fd28c23b4d8931b83b37ce00e3d Mon Sep 17 00:00:00 2001 From: Saul Dominguez Date: Tue, 25 Aug 2026 12:29:18 +0200 Subject: [PATCH 1/2] feat(storybook): add on-demand performance metrics scripts Expose the Storybook performance panel's numbers as JSON, so agents and humans can check a component while building or changing it: pnpm perf-metrics F0Button --snapshot perf-changed.ts measures the stories a PR adds or changes and reduces them to the few facts worth attention, for a future PR comment. Highlight thresholds are set from the measured distribution across the library, not by feel: 102 of 102 snapshot stories record at least one render cascade, so "any cascade" would highlight nothing. Layout shift is highlighted by CLS score rather than shift count because the count is not reproducible between runs, while the score is. Co-Authored-By: Claude Opus 5 --- .github/agent-prompts/performance.md | 64 +++ packages/react/.scripts/perf-changed.ts | 340 ++++++++++++++++ packages/react/.scripts/perf-metrics.ts | 517 ++++++++++++++++++++++++ packages/react/package.json | 1 + 4 files changed, 922 insertions(+) create mode 100644 .github/agent-prompts/performance.md create mode 100644 packages/react/.scripts/perf-changed.ts create mode 100644 packages/react/.scripts/perf-metrics.ts diff --git a/.github/agent-prompts/performance.md b/.github/agent-prompts/performance.md new file mode 100644 index 0000000000..e9b4ce4f2d --- /dev/null +++ b/.github/agent-prompts/performance.md @@ -0,0 +1,64 @@ +# Performance Report — PR #{{PR_NUMBER}} + +You are a frontend performance engineer reporting on pull request #{{PR_NUMBER}} on `{{REPO}}` (branch `{{HEAD_BRANCH}}` → `{{BASE_BRANCH}}`). This is an authorized internal report on Factorial's F0 design system, run automatically in CI with the repository owner's consent. + +## Goal + +Turn a machine-generated performance measurement into a short, human-readable PR comment that tells the author what — if anything — is worth their attention. + +**This check never blocks a merge.** It is informational. Your verdict is always `pass: true` (see Verdict below). You are writing a note to a colleague, not gating their work. + +## Inputs + +- `/tmp/perf-report.json` — measurements for every story this PR adds or changes. **This is your primary input; read it first.** +- `/tmp/pr.diff` — the PR diff, so you can connect a measurement to the code that caused it. + +### Reading the report + +Each entry in `stories[]` has: + +- `id`, `title`, `name` — which story was measured +- `storyFile` — the file it came from +- `isNew` — whether this PR adds the story +- `deterministic` — counts of work done: `mounts`, `renders`, `updates`, `cascades`, `slowUpdates`, `domElements`, `styleWrites`, `forcedReflows`, `layoutShifts` +- `timing` — wall-clock samples +- `highlights` — pre-computed strings for the things that crossed an attention threshold + +**Only `deterministic` numbers and `highlights` are trustworthy.** The `timing` numbers vary run to run on CI hardware and include page-wide work that is not the component (`longTasks` is frequently axe-core, not the story). Do not report a timing number as though it means something, and never describe a component as "slow" or "fast" based on one. + +An empty `highlights` array means nothing crossed a threshold. That is the normal, healthy case. + +## Instructions + +1. Read `/tmp/perf-report.json`. +2. If `stories` is empty, or every story has an empty `highlights` array, say so in one line. Do not manufacture concerns. Do not pad the comment with a table of unremarkable numbers. +3. For each story that does have highlights, look at the diff (and the source file if needed) and try to explain **why** — a `setState` in an effect that could be derived during render, a value or callback rebuilt every render and passed to a memoized child, a layout read after a style write, an unkeyed list. If you cannot find a plausible cause from the diff, say the measurement stands but the cause is not obvious from this change; do not invent one. +4. Group by component rather than listing every story separately. Ten stories from one component with the same highlight is one finding, not ten. +5. Prefer silence over noise. A short comment that names two real things beats a long one that names twelve maybes. + +## Important context before you conclude anything + +- **A cascade count above zero is normal here.** Every story in this library records at least one render cascade (median 3) because Storybook's own decorators and providers render around the story. The report only raises a cascade highlight well above that norm. Never tell an author to "eliminate render cascades" on the basis of a number the report did not highlight. +- **A new story has no "before" to compare against.** The report measures this PR only; there is no baseline from `{{BASE_BRANCH}}`. So do not claim a change made something "worse", "slower", or "a regression" — you cannot know that. Describe what the numbers are, not how they moved. +- **Do not comment on unchanged components.** Only stories this PR adds or changes are measured, and only those are in scope. +- `truncated: true` means more stories were affected than were measured — mention that the report is partial. + +## Writing the comment + +Write the comment to `/tmp/perf-comment.md` as GitHub-flavoured markdown. It is posted verbatim on the PR, so it must stand alone. + +Structure it as: + +- A one-line summary — e.g. `Measured 6 stories across 2 components. Two things worth a look.` or `Measured 4 stories. Nothing stood out.` +- Then, only if there are highlights, a short section per affected component: what was measured, what the likely cause is, and a concrete suggestion. +- Close with a one-line note that this check is informational and never blocks a merge. + +Keep it under roughly 250 words unless there are genuinely several distinct findings. Use a table only when comparing three or more stories on the same metric; prose is better for one or two. + +Do not include the raw JSON. Do not restate every metric for every story. + +## Verdict + +After writing `/tmp/perf-comment.md`, output a verdict line in exactly this format. `pass` is **always** `true` — this check reports and never fails a PR, even when it finds something notable: + + diff --git a/packages/react/.scripts/perf-changed.ts b/packages/react/.scripts/perf-changed.ts new file mode 100644 index 0000000000..516fe0ec05 --- /dev/null +++ b/packages/react/.scripts/perf-changed.ts @@ -0,0 +1,340 @@ +#!/usr/bin/env tsx +/** + * perf-changed.ts + * + * Measure the performance-panel metrics of every story a PR adds or changes, + * and reduce them to the handful of facts worth a human's attention. + * + * tsx .scripts/perf-changed.ts --compare-commit origin/main --out perf-report.json + * + * The JSON it writes is the input to the 🚀 Performance agentic check, which + * turns it into a PR comment. This script deliberately does NOT decide whether a + * PR is acceptable: it has no thresholds and always exits 0 (barring a genuine + * crash). It reports; a human reads. + * + * ── What counts as "worth highlighting" ─────────────────────────────────── + * Emitting every number for every story would produce a wall of noise nobody + * reads. So each story carries a `highlights` array, and only the deterministic + * metrics can produce one — see perf-metrics.ts for why the timing numbers are + * not trustworthy enough to draw a reader's eye to (they swing run to run and + * include axe-core's work, not just the component's). + * + * The highlight rules encode "this is unusual enough that someone should look", + * not "this is forbidden": + * + * - forcedReflows > 0 a layout read interleaved with writes (offsetWidth + * after a style change) — the classic layout thrash. + * Measured base rate across the library: 0 of 40. + * - cls >= CLS_* cumulative layout shift above the noise floor — + * by score, never by shift count (not reproducible) + * - slowUpdates > 0 an update that blew a 16ms frame budget (2 of 40) + * - cascades > CASCADE_* renders scheduling further renders, well past the + * library norm — see the threshold block for why this + * is not simply "> 0" + * - updates > UPDATE_* re-rendering well past mount without interaction + * - domElements > DOM_* an unusually heavy tree for one story + * + * A story with nothing notable still appears in the JSON, with an empty + * `highlights` array, so the agent can say "the other six look fine" instead of + * silently dropping them. + */ +import { execFileSync } from "node:child_process" +import { writeFileSync } from "node:fs" + +import consola from "consola" + +import { + DEFAULT_SETTLE_MS, + detectStorybookUrl, + fetchIndex, + measure, + note, + type StoryIndexEntry, + type StoryReport, +} from "./perf-metrics" + +/** + * Attention thresholds — not limits. Nothing fails for crossing one; crossing + * one only means the comment mentions it. + * + * These are set from the measured distribution across the library rather than + * picked by feel, because the intuitive rules are wrong here. "Flag any render + * cascade" sounds right and is useless: every one of the 102 snapshot stories + * measured has at least one cascade (median 3), because Storybook's own + * decorators and providers re-render around the story. A rule that fires on + * 100% of stories highlights nothing. + * + * Measured across 102 snapshot stories (the heaviest population — a snapshot + * story renders every variant at once, so thresholds drawn from it do not fire + * on ordinary stories): + * + * metric p50 p90 p95 max + * cascades 3 6 8 18 + * updates 6 10 13 26 + * domElements 89 678 884 5250 + * + * Each threshold sits at roughly p95, so about one story in twenty is + * mentioned for it. + */ +const CASCADE_HIGHLIGHT_THRESHOLD = 8 +const UPDATE_HIGHLIGHT_THRESHOLD = 13 +const DOM_HIGHLIGHT_THRESHOLD = 900 + +/** + * Cumulative layout shift score worth mentioning. See highlightsFor for why + * this is a score threshold and not "any layout shift". + */ +const CLS_HIGHLIGHT_THRESHOLD = 0.01 + +/** A story plus the reasons it is worth mentioning. */ +type ChangedStoryReport = StoryReport & { + /** Story file this story came from, repo-relative. */ + storyFile: string + /** Whether the story file is newly added by this PR. */ + isNew: boolean + highlights: string[] +} + +/** + * Story files added or modified versus the comparison commit. + * + * `--diff-filter=d` keeps additions and modifications while dropping deletions: + * a deleted story cannot be measured, and its absence is not a performance + * finding. + * + * The `:(top)` pathspec prefix anchors the glob to the repository root. Without + * it the pathspec resolves relative to the current directory, so running this + * from `packages/react` (which is where `pnpm --filter` puts you) silently + * matches nothing and the script reports "no story files changed" on a PR that + * changed plenty. + */ +function changedStoryFiles(compareCommit: string): { + file: string + isNew: boolean +}[] { + const run = (args: string[]) => + execFileSync("git", args, { encoding: "utf-8" }) + .split("\n") + .map((l) => l.trim()) + .filter(Boolean) + + const changed = run([ + "diff", + "--name-only", + "--diff-filter=d", + `${compareCommit}...HEAD`, + "--", + ":(top)packages/react/src/**/*.stories.tsx", + ]) + const added = new Set( + run([ + "diff", + "--name-only", + "--diff-filter=A", + `${compareCommit}...HEAD`, + "--", + ":(top)packages/react/src/**/*.stories.tsx", + ]) + ) + + return changed.map((file) => ({ file, isNew: added.has(file) })) +} + +/** + * Map a repo-relative story file to its stories in the index. + * + * The index records `importPath` package-relative ("./src/components/F0Button/ + * index.stories.tsx") while git reports repo-relative ("packages/react/src/…"), + * so compare on the normalized tail. Matching on the full path rather than the + * basename matters — `index.stories.tsx` is the same basename for most + * components in the repo. + */ +function storiesForFile( + index: StoryIndexEntry[], + repoRelativeFile: string +): StoryIndexEntry[] { + const normalize = (p: string) => + p.replace(/^\.\//, "").replace(/^packages\/react\//, "") + const target = normalize(repoRelativeFile) + return index.filter((e) => e.importPath && normalize(e.importPath) === target) +} + +/** Reduce a measurement to the facts worth surfacing. */ +function highlightsFor(report: StoryReport): string[] { + const d = report.deterministic + const out: string[] = [] + + if (d.cascades > CASCADE_HIGHLIGHT_THRESHOLD) { + out.push( + `${d.cascades} render cascades — renders that scheduled another render (typical is 3, attention threshold ${CASCADE_HIGHLIGHT_THRESHOLD})` + ) + } + if (d.forcedReflows > 0) { + out.push( + `${d.forcedReflows} forced reflow${d.forcedReflows === 1 ? "" : "s"} — layout was read back synchronously after a style write` + ) + } + // Layout shift is highlighted by CLS *score*, never by shift *count*. + // + // The count is not reproducible: the same five F0Card stories measured three + // times gave three different sets, because a shift is only recorded when the + // browser happens to paint between the two layouts. The score is, once it is + // above the noise floor — across three runs ApplicationFrame scored 0.1943 / + // 0.1932 / 0.1936 and AnalyticsDashboard scored 0.0472 all three times, while + // the stories that flickered in and out all scored ~0.0001. + // + // CLS_HIGHLIGHT_THRESHOLD sits an order of magnitude above that noise floor + // and an order of magnitude below Core Web Vitals' 0.1 "needs improvement" + // line. Across the library's 102 snapshot stories it selects 4. + if (report.timing.cls >= CLS_HIGHLIGHT_THRESHOLD) { + out.push( + `cumulative layout shift of ${report.timing.cls} — content moved after it was first painted (attention threshold ${CLS_HIGHLIGHT_THRESHOLD}; Core Web Vitals calls 0.1 "needs improvement")` + ) + } + if (d.slowUpdates > 0) { + out.push( + `${d.slowUpdates} update${d.slowUpdates === 1 ? "" : "s"} over one frame (16ms)` + ) + } + if (d.updates > UPDATE_HIGHLIGHT_THRESHOLD) { + out.push( + `${d.updates} re-renders after mount with no interaction (typical is 6, attention threshold ${UPDATE_HIGHLIGHT_THRESHOLD})` + ) + } + if (d.domElements > DOM_HIGHLIGHT_THRESHOLD) { + out.push( + `${d.domElements} DOM elements in one story (typical is 89, attention threshold ${DOM_HIGHLIGHT_THRESHOLD})` + ) + } + return out +} + +function parseArgs(argv: string[]) { + let compareCommit = "origin/main" + let out = "perf-report.json" + let url: string | undefined + let settleMs = DEFAULT_SETTLE_MS + let maxStories = 40 + + for (let i = 0; i < argv.length; i++) { + const arg = argv[i] + if (arg === "--compare-commit") compareCommit = argv[++i] + else if (arg === "--out") out = argv[++i] + else if (arg === "--url") url = argv[++i] + else if (arg === "--settle") settleMs = Number(argv[++i]) + else if (arg === "--max-stories") maxStories = Number(argv[++i]) + else throw new Error(`Unknown argument: ${arg}`) + } + return { compareCommit, out, url, settleMs, maxStories } +} + +async function main() { + const args = parseArgs(process.argv.slice(2)) + + const files = changedStoryFiles(args.compareCommit) + if (!files.length) { + note("No story files added or changed — nothing to measure.") + writeFileSync( + args.out, + JSON.stringify( + { + changedStoryFiles: 0, + storiesMeasured: 0, + truncated: false, + stories: [], + }, + null, + 2 + ) + "\n" + ) + return + } + note( + `${files.length} story file${files.length === 1 ? "" : "s"} changed vs ${args.compareCommit}.` + ) + + const baseUrl = ( + args.url ?? + process.env.STORYBOOK_URL ?? + (await detectStorybookUrl()) ?? + "" + ).replace(/\/$/, "") + if (!baseUrl) { + throw new Error( + "No running Storybook found. Serve the static build first, or pass --url." + ) + } + + const index = await fetchIndex(baseUrl) + + // Expand files → stories, keeping the file association for the report. + const targets: { entry: StoryIndexEntry; file: string; isNew: boolean }[] = [] + for (const { file, isNew } of files) { + const stories = storiesForFile(index, file) + if (!stories.length) { + consola.warn(`No stories in the index for ${file} — skipping.`) + continue + } + for (const entry of stories) targets.push({ entry, file, isNew }) + } + + // A PR that touches a very large number of stories would otherwise dominate + // the job's runtime. Truncation is reported in the JSON so the comment can say + // so out loud rather than quietly under-reporting. + const truncated = targets.length > args.maxStories + const measured = truncated ? targets.slice(0, args.maxStories) : targets + if (truncated) { + consola.warn( + `${targets.length} stories affected; measuring the first ${args.maxStories}.` + ) + } + + const { chromium } = await import("@playwright/test") + const browser = await chromium.launch() + const stories: ChangedStoryReport[] = [] + try { + for (const { entry, file, isNew } of measured) { + const report = await measure(browser, baseUrl, entry, args.settleMs) + if (!report) continue + stories.push({ + ...report, + storyFile: file, + isNew, + highlights: highlightsFor(report), + }) + } + } finally { + await browser.close() + } + + const withHighlights = stories.filter((s) => s.highlights.length > 0) + const output = { + comparedAgainst: args.compareCommit, + changedStoryFiles: files.length, + storiesAffected: targets.length, + storiesMeasured: stories.length, + storiesWithHighlights: withHighlights.length, + truncated, + thresholds: { + cascades: CASCADE_HIGHLIGHT_THRESHOLD, + updates: UPDATE_HIGHLIGHT_THRESHOLD, + domElements: DOM_HIGHLIGHT_THRESHOLD, + cls: CLS_HIGHLIGHT_THRESHOLD, + }, + note: + "Reporting only — no thresholds gate this PR. `deterministic` metrics are stable across runs and safe to compare; " + + "`timing` metrics vary run to run and include page-wide work such as axe-core, so they are never used to raise a highlight.", + stories, + } + + writeFileSync(args.out, JSON.stringify(output, null, 2) + "\n") + note( + `Measured ${stories.length} stor${stories.length === 1 ? "y" : "ies"}; ` + + `${withHighlights.length} with highlights. Wrote ${args.out}.` + ) +} + +main().catch((error) => { + consola.error(error instanceof Error ? error.message : error) + process.exitCode = 1 +}) diff --git a/packages/react/.scripts/perf-metrics.ts b/packages/react/.scripts/perf-metrics.ts new file mode 100644 index 0000000000..c873f4b70d --- /dev/null +++ b/packages/react/.scripts/perf-metrics.ts @@ -0,0 +1,517 @@ +#!/usr/bin/env tsx +/** + * perf-metrics.ts + * + * Print the numbers the Storybook performance panel shows, as JSON, for one or + * more stories — without opening Storybook. Intended for humans and agents + * checking a component while building or changing it: + * + * pnpm perf-metrics F0Button # every story of a component + * pnpm perf-metrics components-button-button--variants + * pnpm perf-metrics F0Button --snapshot # just its snapshot story + * pnpm perf-metrics F0Button --out perf.json + * + * This is a reporting tool. It has no thresholds, no baseline and no pass/fail + * verdict — it does not gate anything in CI. Interpreting the numbers is the + * caller's job, so read the `stability` note in the output before comparing two + * runs. + * + * ── Which numbers can you trust? ────────────────────────────────────────── + * The output is split into `deterministic` and `timing` for a reason, measured + * across repeat runs of the same story on an idle machine: + * + * deterministic — identical on every run (mounts 1, renders 4, cascades 1, + * domElements 33, …). These count *work done*, not time taken, so they are + * machine-independent and safe to compare between two runs, two branches or + * two components. This is what you want when asking "did my change make + * this component render more than it used to?". + * + * timing — varies run to run even when nothing changes (totalBlockingTime + * ranged 42–47ms across five identical runs) and is contaminated by + * whatever else shares the page: in Storybook, `longTasks` is frequently + * axe-core rather than the component under test. Treat as a rough signal + * only; never diff it between runs and conclude anything. + * + * ── How it works ────────────────────────────────────────────────────────── + * The addon collects metrics in the preview and ships them to the manager panel + * over the Storybook channel. There is no manager here, so this script plays the + * part of one: it answers the addon's `request-panel-visibility` handshake with + * `true` (which is what actually starts the browser collectors), waits for the + * story to settle, then asks for a metrics snapshot. + * + * The handshake is installed via an init script rather than after load, so the + * collectors are already running when the story mounts. That matters: forced + * reflows, style writes and layout shifts all happen *during* mount, and are + * silently reported as 0 if collection starts afterwards. + */ +import { chromium, type Browser, type Page } from "@playwright/test" +import { writeFileSync } from "node:fs" +import { pathToFileURL } from "node:url" + +import consola from "consola" + +/** Addon id — namespaces every channel event the addon uses. */ +const ADDON_ID = "primer-performance-monitor" + +/** + * Progress output, always on stderr. + * + * stdout carries the JSON and nothing else, so `perf-metrics F0Button | jq` and + * agent tooling that parses stdout keep working. consola cannot be used for + * this: `consola.info` and `consola.success` write to **stdout**, so routing + * progress through them corrupts the payload — piping to jq fails with + * "Invalid numeric literal" on the info line. `consola.warn`/`consola.error` + * do go to stderr and are used as-is. + */ +export function note(message: string): void { + process.stderr.write(`${message}\n`) +} + +/** + * Ports probed for a running Storybook when `--url` is not given. 6006 is the + * `pnpm dev` default; 6008 is where `storybook dev` lands when 6006 is taken + * (a second checkout or worktree already running one). + */ +const DEFAULT_PORTS = [6006, 6008] + +/** + * How long to let the story run before snapshotting, in ms. + * + * The deterministic metrics are complete as soon as the story has mounted — the + * React Profiler records renders as they happen, regardless of when collection + * started — so they do not need this window at all. The timing metrics do: they + * are sampled over a period, and with no window `fps` is computed from a + * two-frame sample and reads in the thousands. + */ +export const DEFAULT_SETTLE_MS = 1000 + +export type StoryIndexEntry = { + id: string + title: string + name: string + type: string + exportName?: string + /** e.g. "./src/components/F0Button/index.tsx" */ + componentPath?: string + /** e.g. "./src/components/F0Button/index.stories.tsx" */ + importPath?: string +} + +/** + * Whether a story is a Chromatic snapshot story. + * + * Detected by export name, not by `withSnapshot()`'s `chromatic.disableSnapshot` + * parameter: Storybook's index.json carries no story parameters at all, so the + * parameter is simply not visible from here. Every snapshot story in the repo is + * exported as `Snapshot` (or a `…SnapshotMatrix` variant), which is. + */ +export function isSnapshotStory(entry: StoryIndexEntry): boolean { + return /snapshot/i.test(entry.exportName ?? entry.name) +} + +/** Raw metrics payload emitted by the addon over the channel. */ +type RawMetrics = Record + +export type StoryReport = { + id: string + title: string + name: string + deterministic: { + /** Times the story's React tree mounted. Normally 1. */ + mounts: number + /** Total Profiler renders (mount + every re-render). */ + renders: number + /** Re-renders after mount settled. High values mean wasted work. */ + updates: number + /** Renders that scheduled another render — a cascade. Ideally 0. */ + cascades: number + /** Updates that took over one frame (>16ms). */ + slowUpdates: number + /** DOM elements rendered by the story. */ + domElements: number + /** Inline style / CSS-var writes observed during mount. */ + styleWrites: number + /** Layout reads that forced a synchronous reflow. Ideally 0. */ + forcedReflows: number + } + timing: { + /** Wall-clock ms spent in the mount render. */ + mountMs: number + /** P95 of post-mount update durations, ms. */ + p95UpdateMs: number + /** + * Layout shifts observed, and their cumulative score. + * + * These sit in `timing`, not `deterministic`, despite being counts — they + * are not reproducible. Measuring the same five F0Card stories three times + * gave three different sets: two stories shifted on every run, two shifted + * on some runs and not others. A shift is only recorded if the browser + * happens to paint between the two layouts, which depends on load timing. + * + * So a non-zero value is worth investigating, but a zero does not prove + * there is no shift, and a difference between two runs proves nothing. + */ + layoutShifts: number + /** Cumulative layout shift score. */ + cls: number + /** Total blocking time, ms — includes non-component work such as axe. */ + totalBlockingTime: number + /** Long tasks (>50ms) seen on the main thread, from any source. */ + longTasks: number + /** Sampled frames per second. */ + fps: number + /** JS heap in use, MB — whole page, not just this component. */ + heapMB: number | null + } +} + +function num(v: unknown, fallback = 0): number { + return typeof v === "number" && Number.isFinite(v) ? v : fallback +} + +function round(v: number, dp = 1): number { + const f = 10 ** dp + return Math.round(v * f) / f +} + +/** Shape the addon's flat payload into the deterministic/timing split. */ +function toReport(entry: StoryIndexEntry, m: RawMetrics): StoryReport { + return { + id: entry.id, + title: entry.title, + name: entry.name, + deterministic: { + mounts: num(m.reactMountCount), + renders: num(m.reactRenderCount), + updates: num(m.reactPostMountUpdateCount), + cascades: num(m.renderCascades), + slowUpdates: num(m.slowReactUpdates), + domElements: num(m.domElements), + styleWrites: num(m.styleWrites), + forcedReflows: num(m.forcedReflowCount), + }, + timing: { + mountMs: round(num(m.reactMountDuration)), + p95UpdateMs: round(num(m.reactP95Duration)), + layoutShifts: num(m.layoutShiftCount), + cls: round(num(m.layoutShiftScore), 4), + totalBlockingTime: num(m.totalBlockingTime), + longTasks: num(m.longTasks), + fps: num(m.fps), + heapMB: typeof m.memoryUsedMB === "number" ? m.memoryUsedMB : null, + }, + } +} + +/** First URL that answers with a Storybook story index, or null. */ +export async function detectStorybookUrl(): Promise { + for (const port of DEFAULT_PORTS) { + const url = `http://localhost:${port}` + try { + const res = await fetch(`${url}/index.json`, { + signal: AbortSignal.timeout(2500), + }) + if (res.ok) return url + } catch { + // not running on this port — try the next + } + } + return null +} + +export async function fetchIndex(url: string): Promise { + const res = await fetch(`${url}/index.json`, { + signal: AbortSignal.timeout(15_000), + }) + if (!res.ok) { + throw new Error(`Could not read ${url}/index.json (HTTP ${res.status})`) + } + const json = (await res.json()) as { + entries: Record + } + return Object.values(json.entries).filter((e) => e.type === "story") +} + +/** + * Resolve user-supplied selectors to stories. + * + * Accepts an exact story id, a component name (`F0Button`) or any substring of + * the story id or title, so callers do not have to know Storybook's id + * mangling. Matching is case-insensitive and the results are de-duplicated. + * + * The source paths are part of the haystack because the F0 name is often absent + * from both the id and the title — F0Button's stories are titled + * "Components/Button/Button" with ids like `components-button-button--variants`, + * so a bare `F0Button` would otherwise match nothing at all. It appears only in + * `componentPath` / `importPath`. + */ +function selectStories( + all: StoryIndexEntry[], + selectors: string[], + snapshotOnly: boolean +): StoryIndexEntry[] { + const picked = new Map() + + for (const selector of selectors) { + const needle = selector.toLowerCase() + const exact = all.filter((s) => s.id.toLowerCase() === needle) + const matches = exact.length + ? exact + : all.filter((s) => + [s.id, s.title, s.componentPath ?? "", s.importPath ?? ""] + .join("\n") + .toLowerCase() + .includes(needle) + ) + + if (!matches.length) { + consola.warn(`No story matched "${selector}"`) + continue + } + for (const m of matches) picked.set(m.id, m) + } + + const chosen = Array.from(picked.values()) + if (!snapshotOnly) return chosen + + const snapshots = chosen.filter(isSnapshotStory) + if (!snapshots.length) { + // Deliberately not falling back to every matched story: silently measuring + // 30 stories when --snapshot was asked for reads as success and buries the + // fact that the component has no snapshot story at all. + consola.warn( + `--snapshot: none of the ${chosen.length} matched ${chosen.length === 1 ? "story is a" : "stories are"} snapshot ${chosen.length === 1 ? "story" : "stories"}. ` + + `Drop --snapshot to measure them anyway.` + ) + } + return snapshots +} + +/** + * Measure one story. Returns null when the story never rendered or the addon + * never answered — a broken story should not take the whole run down. + */ +export async function measure( + browser: Browser, + baseUrl: string, + entry: StoryIndexEntry, + settleMs: number +): Promise { + const page: Page = await browser.newPage() + try { + // Answer the panel-visibility handshake before any story code runs, so the + // collectors are live for the mount itself (see file header). + await page.addInitScript( + ({ addonId }: { addonId: string }) => { + const install = (channel: { + on: (e: string, cb: () => void) => void + emit: (e: string, v?: unknown) => void + }) => { + channel.on(`${addonId}/request-panel-visibility`, () => { + channel.emit(`${addonId}/panel-visibility`, true) + }) + channel.emit(`${addonId}/panel-visibility`, true) + } + + // The channel global appears partway through preview bootstrap, so trap + // the assignment rather than polling for it. + let current: unknown = + (window as unknown as Record) + .__STORYBOOK_ADDONS_CHANNEL__ ?? undefined + if (current) install(current as Parameters[0]) + Object.defineProperty(window, "__STORYBOOK_ADDONS_CHANNEL__", { + configurable: true, + get: () => current, + set: (value) => { + current = value + if (value) install(value as Parameters[0]) + }, + }) + }, + { addonId: ADDON_ID } + ) + + await page.goto( + `${baseUrl}/iframe.html?id=${encodeURIComponent(entry.id)}&viewMode=story`, + { waitUntil: "load", timeout: 30_000 } + ) + await page.waitForFunction( + () => + (document.querySelector("#storybook-root")?.children.length ?? 0) > 0, + undefined, + { timeout: 20_000 } + ) + + const metrics = await page.evaluate( + async ({ addonId, waitMs }) => { + const channel = (window as unknown as Record) + .__STORYBOOK_ADDONS_CHANNEL__ as + | { + on: (e: string, cb: (p: unknown) => void) => void + once: (e: string, cb: (p: unknown) => void) => void + emit: (e: string, v?: unknown) => void + } + | undefined + if (!channel) return null + + // Belt and braces: the init-script handshake should already have started + // collection, but a story that mounted before the trap fired would not + // have seen it. + channel.emit(`${addonId}/panel-visibility`, true) + await new Promise((r) => setTimeout(r, waitMs)) + + return await new Promise((resolve) => { + const timer = setTimeout(() => resolve(null), 10_000) + channel.once(`${addonId}/metrics-update`, (payload) => { + clearTimeout(timer) + resolve(payload) + }) + channel.emit(`${addonId}/request-metrics`) + }) + }, + { addonId: ADDON_ID, waitMs: settleMs } + ) + + if (!metrics) { + consola.warn(`No metrics returned for ${entry.id}`) + return null + } + return toReport(entry, metrics as RawMetrics) + } catch (error) { + consola.warn( + `Failed to measure ${entry.id}: ${error instanceof Error ? error.message : String(error)}` + ) + return null + } finally { + await page.close() + } +} + +function parseArgs(argv: string[]) { + const selectors: string[] = [] + let url: string | undefined + let out: string | undefined + let settleMs = DEFAULT_SETTLE_MS + let snapshotOnly = false + + for (let i = 0; i < argv.length; i++) { + const arg = argv[i] + if (arg === "--url") url = argv[++i] + else if (arg === "--out") out = argv[++i] + else if (arg === "--settle") settleMs = Number(argv[++i]) + else if (arg === "--snapshot") snapshotOnly = true + else if (arg === "--help" || arg === "-h") return null + else if (arg.startsWith("-")) throw new Error(`Unknown flag: ${arg}`) + else selectors.push(arg) + } + return { selectors, url, out, settleMs, snapshotOnly } +} + +const USAGE = ` +Print Storybook performance-panel metrics as JSON. + +Usage: + pnpm perf-metrics [more…] [options] + +Options: + --snapshot Only the snapshot story of each matched component + --url Storybook base URL (default: $STORYBOOK_URL, else :6006/:6008) + --settle Collection window before snapshotting (default ${DEFAULT_SETTLE_MS}) + --out Write JSON to a file as well as stdout + -h, --help Show this help + +Requires a running Storybook (pnpm dev). +`.trim() + +async function main() { + const args = parseArgs(process.argv.slice(2)) + if (!args) { + note(USAGE) + return + } + if (!args.selectors.length) { + consola.error("Pass at least one story id or component name.\n") + note(USAGE) + process.exitCode = 1 + return + } + if (!Number.isFinite(args.settleMs) || args.settleMs < 0) { + consola.error(`--settle must be a non-negative number of milliseconds.`) + process.exitCode = 1 + return + } + + const baseUrl = ( + args.url ?? + process.env.STORYBOOK_URL ?? + (await detectStorybookUrl()) ?? + "" + ).replace(/\/$/, "") + + if (!baseUrl) { + consola.error( + `No running Storybook found on ${DEFAULT_PORTS.map((p) => `:${p}`).join(" or ")}.\n` + + `Start one with \`pnpm dev\`, or point at it with --url.` + ) + process.exitCode = 1 + return + } + + const index = await fetchIndex(baseUrl) + const stories = selectStories(index, args.selectors, args.snapshotOnly) + if (!stories.length) { + consola.error("No stories matched.") + process.exitCode = 1 + return + } + + note( + `Measuring ${stories.length} ${stories.length === 1 ? "story" : "stories"} at ${baseUrl}…` + ) + + const browser = await chromium.launch() + const reports: StoryReport[] = [] + try { + for (const story of stories) { + const report = await measure(browser, baseUrl, story, args.settleMs) + if (report) reports.push(report) + } + } finally { + await browser.close() + } + + const output = { + storybookUrl: baseUrl, + settleMs: args.settleMs, + measuredAt: new Date().toISOString(), + stability: { + deterministic: + "Counts of work done. Stable across runs and machines — safe to compare between runs, branches or components.", + timing: + "Wall-clock samples. Vary run to run and include page-wide work that is not this component (in Storybook, longTasks is often axe-core). Indicative only; do not diff between runs.", + }, + stories: reports, + } + + const json = JSON.stringify(output, null, 2) + if (args.out) { + writeFileSync(args.out, json + "\n") + note(`Wrote ${args.out}`) + } + // stdout stays pure JSON (consola writes to stderr) so callers can pipe it. + process.stdout.write(json + "\n") + + if (!reports.length) process.exitCode = 1 +} + +// Only run the CLI when invoked directly — perf-changed.ts imports the +// measurement helpers above and must not trigger a second run on import. +if ( + process.argv[1] && + import.meta.url === pathToFileURL(process.argv[1]).href +) { + main().catch((error) => { + consola.error(error instanceof Error ? error.message : error) + process.exitCode = 1 + }) +} diff --git a/packages/react/package.json b/packages/react/package.json index fa67bfe946..c3d2ae7848 100644 --- a/packages/react/package.json +++ b/packages/react/package.json @@ -47,6 +47,7 @@ "format": "sh -c 'oxfmt \"${@:-src/}\"' --", "format:check": "sh -c 'oxfmt --check \"${@:-src/}\"' --", "tsc": "tsc --noEmit", + "perf-metrics": "tsx .scripts/perf-metrics.ts", "check:api-surface": "tsx .scripts/check-api-surface.ts", "check:bugfix-red-green": "tsx .scripts/check-bugfix-red-green.ts", "check:new-component-dod": "tsx .scripts/check-new-component-dod.ts", From 4dff9639079b023f1043ecdfe1e097eba1ab7b91 Mon Sep 17 00:00:00 2001 From: Saul Dominguez Date: Fri, 28 Aug 2026 10:49:00 +0200 Subject: [PATCH 2/2] feat(storybook): measure stories when their component changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit perf-changed.ts keyed off changed *.stories.tsx files, so a PR that changed a component without touching its story measured nothing at all — the common shape of a fix. It now diffs every source file under packages/react/src and maps each to the stories that render it, walking up to the owning component directory so nested files attribute correctly. The walk stops at src// depth so a shared utility cannot be attributed to half the library, and test, snapshot and docs files are excluded outright: fix(F0Chat) (bf41fa6e4) changed two components and two __tests__/ files, and the test files alone attributed 38 stories. Each measured story records `measuredBecause` ("story" or "source") so the comment can attribute a finding to the component rather than imply the author edited a story they never opened. Adds --head so a range other than "…...HEAD" can be previewed locally. Verified against real history: the component-only commit reports 39 stories affected where it previously reported none, and a docs-only commit still reports nothing. Co-Authored-By: Claude Opus 5 --- .github/agent-prompts/performance.md | 4 +- packages/react/.scripts/perf-changed.ts | 196 +++++++++++++++++++----- 2 files changed, 163 insertions(+), 37 deletions(-) diff --git a/.github/agent-prompts/performance.md b/.github/agent-prompts/performance.md index e9b4ce4f2d..47372ee5c3 100644 --- a/.github/agent-prompts/performance.md +++ b/.github/agent-prompts/performance.md @@ -18,7 +18,8 @@ Turn a machine-generated performance measurement into a short, human-readable PR Each entry in `stories[]` has: - `id`, `title`, `name` — which story was measured -- `storyFile` — the file it came from +- `changedFile` — the changed file that pulled this story into the report +- `measuredBecause` — `"story"` if the story's own file changed, `"source"` if a source file in its component changed but the story did not - `isNew` — whether this PR adds the story - `deterministic` — counts of work done: `mounts`, `renders`, `updates`, `cascades`, `slowUpdates`, `domElements`, `styleWrites`, `forcedReflows`, `layoutShifts` - `timing` — wall-clock samples @@ -41,6 +42,7 @@ An empty `highlights` array means nothing crossed a threshold. That is the norma - **A cascade count above zero is normal here.** Every story in this library records at least one render cascade (median 3) because Storybook's own decorators and providers render around the story. The report only raises a cascade highlight well above that norm. Never tell an author to "eliminate render cascades" on the basis of a number the report did not highlight. - **A new story has no "before" to compare against.** The report measures this PR only; there is no baseline from `{{BASE_BRANCH}}`. So do not claim a change made something "worse", "slower", or "a regression" — you cannot know that. Describe what the numbers are, not how they moved. - **Do not comment on unchanged components.** Only stories this PR adds or changes are measured, and only those are in scope. +- **Mind `measuredBecause`.** When it is `"source"` the author changed the component, not the story — they may never have opened that story file. Attribute the finding to the component ("F0Button's stories show…"), and never say or imply they edited the story. When it is `"story"` the story file itself changed and you can refer to it directly. - `truncated: true` means more stories were affected than were measured — mention that the report is partial. ## Writing the comment diff --git a/packages/react/.scripts/perf-changed.ts b/packages/react/.scripts/perf-changed.ts index 516fe0ec05..f6f6e47799 100644 --- a/packages/react/.scripts/perf-changed.ts +++ b/packages/react/.scripts/perf-changed.ts @@ -88,15 +88,27 @@ const CLS_HIGHLIGHT_THRESHOLD = 0.01 /** A story plus the reasons it is worth mentioning. */ type ChangedStoryReport = StoryReport & { - /** Story file this story came from, repo-relative. */ - storyFile: string - /** Whether the story file is newly added by this PR. */ + /** The changed file that pulled this story into the report, repo-relative. */ + changedFile: string + /** + * How this story was selected: + * "story" — its own story file changed + * "source" — a source file in its component changed; the story itself did not + * + * Worth surfacing to the agent: under "source" the author may not have looked + * at this story at all, so the comment should name the component rather than + * imply they edited the story. + */ + measuredBecause: "story" | "source" + /** Whether this story's own file is newly added by this PR. */ isNew: boolean highlights: string[] } /** - * Story files added or modified versus the comparison commit. + * Source files under packages/react/src added or modified versus the comparison + * commit. Every file, not just `*.stories.tsx` — storiesForFile decides which + * of them map to stories and how. * * `--diff-filter=d` keeps additions and modifications while dropping deletions: * a deleted story cannot be measured, and its absence is not a performance @@ -105,10 +117,13 @@ type ChangedStoryReport = StoryReport & { * The `:(top)` pathspec prefix anchors the glob to the repository root. Without * it the pathspec resolves relative to the current directory, so running this * from `packages/react` (which is where `pnpm --filter` puts you) silently - * matches nothing and the script reports "no story files changed" on a PR that + * matches nothing and the script reports "no files changed" on a PR that * changed plenty. */ -function changedStoryFiles(compareCommit: string): { +function changedSourceFiles( + compareCommit: string, + head: string +): { file: string isNew: boolean }[] { @@ -118,45 +133,116 @@ function changedStoryFiles(compareCommit: string): { .map((l) => l.trim()) .filter(Boolean) + const SPEC = ":(top)packages/react/src/**" + const changed = run([ "diff", "--name-only", "--diff-filter=d", - `${compareCommit}...HEAD`, + `${compareCommit}...${head}`, "--", - ":(top)packages/react/src/**/*.stories.tsx", + SPEC, ]) const added = new Set( run([ "diff", "--name-only", "--diff-filter=A", - `${compareCommit}...HEAD`, + `${compareCommit}...${head}`, "--", - ":(top)packages/react/src/**/*.stories.tsx", + SPEC, ]) ) - return changed.map((file) => ({ file, isNew: added.has(file) })) + return changed + .filter((file) => !isNonRuntimeFile(file)) + .map((file) => ({ file, isNew: added.has(file) })) +} + +/** Strip the "./" and "packages/react/" prefixes so git and index paths compare. */ +function normalizePath(p: string): string { + return p.replace(/^\.\//, "").replace(/^packages\/react\//, "") } +function isStoryFile(file: string): boolean { + return file.endsWith(".stories.tsx") +} + +/** + * Files that cannot change what a story renders, and so must not pull its + * component into the report. + * + * Unit tests are the reason this exists. `fix(F0Chat)` (bf41fa6e4) changed two + * components and two `__tests__/` files; without this filter the test files + * alone attributed 38 stories, so a PR that only adjusted assertions would + * trigger a full performance comment about code whose behaviour never moved. + * Docs are excluded on the same grounds. + */ +function isNonRuntimeFile(file: string): boolean { + return ( + /(^|\/)__tests__\//.test(file) || + /(^|\/)__snapshots__\//.test(file) || + /\.(test|spec)\.[jt]sx?$/.test(file) || + /\.mdx?$/.test(file) + ) +} + +/** + * Shallowest directory a source file may be attributed to, in path segments. + * + * Component directories look like `src//` — three segments. Stopping + * there keeps a change to a broadly shared file from being attributed to + * everything below it: `src/lib/utils.ts` would otherwise walk up to `src/lib` + * (two segments) and drag in every story in the tree. + */ +const MIN_ATTRIBUTION_DEPTH = 3 + /** - * Map a repo-relative story file to its stories in the index. + * Map a changed file to the stories that render it, and say how it got there. + * + * Two different rules, because the two cases warrant different precision: + * + * A changed **story file** maps to exactly the stories declared in it, by + * comparing full normalized paths. The index records `importPath` as + * "./src/components/F0Button/index.stories.tsx" while git reports + * "packages/react/src/…", hence the normalization. Comparing full paths and + * not basenames matters — `index.stories.tsx` is the same basename for most + * components in the repo. * - * The index records `importPath` package-relative ("./src/components/F0Button/ - * index.stories.tsx") while git reports repo-relative ("packages/react/src/…"), - * so compare on the normalized tail. Matching on the full path rather than the - * basename matters — `index.stories.tsx` is the same basename for most - * components in the repo. + * Any **other source file** maps to the stories of the component that owns + * it. This is what makes a component-only change visible: editing + * `F0Button/index.tsx` without touching a story used to measure nothing at + * all. The file's directory is walked upwards until one is found that has + * stories beneath it, which handles nested layouts (`F0Button/internal/ + * helpers.ts` attributes to `F0Button`, whose stories live at the root or + * under `__stories__/`). The walk stops at MIN_ATTRIBUTION_DEPTH so a shared + * utility cannot be attributed to half the library. */ function storiesForFile( index: StoryIndexEntry[], repoRelativeFile: string -): StoryIndexEntry[] { - const normalize = (p: string) => - p.replace(/^\.\//, "").replace(/^packages\/react\//, "") - const target = normalize(repoRelativeFile) - return index.filter((e) => e.importPath && normalize(e.importPath) === target) +): { entries: StoryIndexEntry[]; via: "story" | "source" } { + const target = normalizePath(repoRelativeFile) + + if (isStoryFile(target)) { + return { + entries: index.filter( + (e) => e.importPath && normalizePath(e.importPath) === target + ), + via: "story", + } + } + + let dir = target.split("/").slice(0, -1) + while (dir.length >= MIN_ATTRIBUTION_DEPTH) { + const prefix = `${dir.join("/")}/` + const entries = index.filter( + (e) => e.importPath && normalizePath(e.importPath).startsWith(prefix) + ) + if (entries.length) return { entries, via: "source" } + dir = dir.slice(0, -1) + } + return { entries: [], via: "source" } } /** Reduce a measurement to the facts worth surfacing. */ @@ -215,6 +301,7 @@ function parseArgs(argv: string[]) { let url: string | undefined let settleMs = DEFAULT_SETTLE_MS let maxStories = 40 + let head = "HEAD" for (let i = 0; i < argv.length; i++) { const arg = argv[i] @@ -223,22 +310,25 @@ function parseArgs(argv: string[]) { else if (arg === "--url") url = argv[++i] else if (arg === "--settle") settleMs = Number(argv[++i]) else if (arg === "--max-stories") maxStories = Number(argv[++i]) + // --head exists so a range other than "…///HEAD" can be previewed locally: + // "what would this commit have reported?". CI always leaves it at HEAD. + else if (arg === "--head") head = argv[++i] else throw new Error(`Unknown argument: ${arg}`) } - return { compareCommit, out, url, settleMs, maxStories } + return { compareCommit, out, url, settleMs, maxStories, head } } async function main() { const args = parseArgs(process.argv.slice(2)) - const files = changedStoryFiles(args.compareCommit) + const files = changedSourceFiles(args.compareCommit, args.head) if (!files.length) { - note("No story files added or changed — nothing to measure.") + note("No source files added or changed — nothing to measure.") writeFileSync( args.out, JSON.stringify( { - changedStoryFiles: 0, + changedSourceFiles: 0, storiesMeasured: 0, truncated: false, stories: [], @@ -250,7 +340,7 @@ async function main() { return } note( - `${files.length} story file${files.length === 1 ? "" : "s"} changed vs ${args.compareCommit}.` + `${files.length} source file${files.length === 1 ? "" : "s"} changed vs ${args.compareCommit}.` ) const baseUrl = ( @@ -268,15 +358,42 @@ async function main() { const index = await fetchIndex(baseUrl) // Expand files → stories, keeping the file association for the report. - const targets: { entry: StoryIndexEntry; file: string; isNew: boolean }[] = [] - for (const { file, isNew } of files) { - const stories = storiesForFile(index, file) - if (!stories.length) { - consola.warn(`No stories in the index for ${file} — skipping.`) + // + // Deduplicated by story id: a PR that changes a component and its story (the + // common case) reaches the same stories twice, and several files inside one + // component all attribute to the same set. First writer wins, and story files + // are processed first so a story-file attribution is never overwritten by the + // vaguer source-file one. + const byStory = new Map< + string, + { entry: StoryIndexEntry; file: string; isNew: boolean; via: string } + >() + const ordered = [ + ...files.filter((f) => isStoryFile(f.file)), + ...files.filter((f) => !isStoryFile(f.file)), + ] + let unmapped = 0 + for (const { file, isNew } of ordered) { + const { entries, via } = storiesForFile(index, file) + if (!entries.length) { + unmapped++ continue } - for (const entry of stories) targets.push({ entry, file, isNew }) + for (const entry of entries) { + if (!byStory.has(entry.id)) { + byStory.set(entry.id, { + entry, + file, + isNew: isNew && via === "story", + via, + }) + } + } + } + if (unmapped) { + note(`${unmapped} changed file(s) map to no story — skipped.`) } + const targets = Array.from(byStory.values()) // A PR that touches a very large number of stories would otherwise dominate // the job's runtime. Truncation is reported in the JSON so the comment can say @@ -293,12 +410,13 @@ async function main() { const browser = await chromium.launch() const stories: ChangedStoryReport[] = [] try { - for (const { entry, file, isNew } of measured) { + for (const { entry, file, isNew, via } of measured) { const report = await measure(browser, baseUrl, entry, args.settleMs) if (!report) continue stories.push({ ...report, - storyFile: file, + changedFile: file, + measuredBecause: via as "story" | "source", isNew, highlights: highlightsFor(report), }) @@ -310,8 +428,14 @@ async function main() { const withHighlights = stories.filter((s) => s.highlights.length > 0) const output = { comparedAgainst: args.compareCommit, - changedStoryFiles: files.length, + changedSourceFiles: files.length, storiesAffected: targets.length, + storiesFromStoryChanges: stories.filter( + (s) => s.measuredBecause === "story" + ).length, + storiesFromSourceChanges: stories.filter( + (s) => s.measuredBecause === "source" + ).length, storiesMeasured: stories.length, storiesWithHighlights: withHighlights.length, truncated,