Skip to content

ci(storybook): post an agent-written performance comment on PRs - #5232

Open
sauldom102 wants to merge 3 commits into
mainfrom
claude/perf-report-workflow
Open

ci(storybook): post an agent-written performance comment on PRs#5232
sauldom102 wants to merge 3 commits into
mainfrom
claude/perf-report-workflow

Conversation

@sauldom102

@sauldom102 sauldom102 commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Description

Wires up the reporting scripts from #5229 so a PR that changes stories gets a short, agent-written comment describing anything notable about their performance. Informational only: there is no gate job and no required check, because a performance observation is not a reason to block a merge and these numbers are too environment-sensitive to be a merge condition.

Depends on #5229 — this workflow runs perf-changed.ts and uses the performance.md prompt, both of which land there. Merge that one first.

Implementation details

  • ci: add the 🚀 Performance Report workflow — measure changed stories, narrate, comment
  • ci: post via the existing add-or-update-pr-comment action under comment type performance_report, so re-runs update the comment instead of stacking new ones

Notes for reviewers

Why this is two jobs rather than a new entry in agentic-checks.yaml

A trust boundary runs between them.

Measuring has to build and drive the PR's Storybook, which executes PR-authored code — its Vite config, its components, its scripts. Narrating needs DX_AI_WORKFLOWS_API_KEY. Doing both in one job would run the PR author's code in a process holding that key.

So measure has contents: read and no secrets. reference anywhere in the job, and hands its result on as a JSON artifact. narrate holds the key and never executes anything from the head: it reads the measurement as data, and takes its prompt and agentic-check.sh from the base branch using the same stash-and-restore dance as agentic-checks.yaml. The head checkout in that job exists only so the agent can read source while explaining a number.

This is also why it is a separate workflow rather than a matrix entry: adding it to the agentic-checks.yaml matrix would put every other check (code review, a11y, storybook, test coverage) behind a Storybook build via a shared needs:.

It cannot turn a PR red

Deliberately, at four points:

  • the prompt's verdict is hardcoded pass: true
  • the measurement step ends in || true
  • run-agent and the comment steps are continue-on-error
  • there is no ✅ … gate job for branch protection to require

A failed measurement or a model refusal means no comment, never a failed check.

Cost, and when it runs at all

paths restricts it to PRs touching *.stories.tsx (or this workflow's own inputs), and narrate is skipped entirely unless storiesMeasured > 0. So the common PR pays nothing.

When it does run, measure costs one Storybook build (~170s) plus a few seconds per changed story; it runs in parallel with everything else, so wall-clock impact is roughly nil. It reuses the same Playwright cache key as storybook-tests.yaml rather than populating its own.

Gate logic was checked against all three states — a populated report, an empty one, and a missing file:

populated report -> has-report=true
empty report     -> has-report=false
missing file     -> has-report=false
What triggers a report

perf-changed.ts maps any changed source file under packages/react/src to the stories that render it, walking up to the owning component directory. So a component-only change is measured — verified against bf41fa6e4 (fix(F0Chat)), which reports 39 stories affected where the story-file-only version reported none.

Test, snapshot and docs files are excluded in both the script and this paths filter, so they cannot start the workflow or pull a component in. That mattered: on that same commit the two __tests__/ files alone attributed 38 stories, meaning a PR that only adjusted assertions would have triggered a full performance comment. A docs-only commit correctly reports nothing.

Each measured story records measuredBecause: "story" when its own file changed, "source" when only its component did. The prompt uses it so the comment attributes a finding to the component rather than implying the author edited a story they never opened.

sauldom102 and others added 2 commits August 25, 2026 12:31
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 <noreply@anthropic.com>
Wires up the performance report added in #5229: measure the stories a PR
changes, have an agent turn the measurements into a short comment, and
post it. Informational only — there is no gate job and no required check,
because a performance observation is not a reason to block a merge.

Split into two jobs on a trust boundary. Measuring builds and drives the
PR's own Storybook, so that job executes PR-authored code and holds no
secrets. Narrating holds the Azure key, and reads the measurement as JSON
from an artifact plus its prompt and script from the base branch, so no
PR-authored code runs alongside the key.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sauldom102
sauldom102 requested a review from a team as a code owner August 25, 2026 10:53
@github-actions github-actions Bot added ci react Changes affect packages/react labels Aug 25, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Review policy: Code change

Default rule: any other change needs one approval from f0-devs (rule 4).

Required approvals

Team Why Status
@factorialco/f0-devs Every code change needs a dev approval ⏳ pending
How this was decided
  • PRs touching only sds/ modules require their owners and nothing else.
  • Otherwise, docs-only changes (*.md, *.mdx, *.stories.tsx, anything in __stories__/) → one f0-general approval.
  • Otherwise, feat: titles → one f0-devs and one f0-designers approval. Not a feature? Fix the title prefix.
  • Anything else → one f0-devs approval.
  • Add the needs-design-review label to also request a design approval on any PR.
  • Creating a new sds/ module (new package.yml) additionally requires an f0-general approval.

Policy source: ownership/review-policy.ts · Team members: ownership/teams.yml

@github-actions

Copy link
Copy Markdown
Contributor

✅ No New Circular Dependencies

No new circular dependencies detected. Current count: 0

@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

📦 Alpha Package Version Published

Use pnpm i github:factorialco/f0#npm/alpha-pr-5232 to install the package

Use pnpm i github:factorialco/f0#f352443f468bfc19a76d0c97ab7f62ee86130bb6 to install this specific commit

@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

🔍 Visual review for your branch is published 🔍

Here are the links to:

@github-actions

Copy link
Copy Markdown
Contributor

✅ No breaking public API changes

No public exports were removed, renamed, or had existing props/types changed in a breaking way compared to main.

Comparing f0, experimental and ai against main. Adding components, types, or optional props is safe. This check is non-blocking.

⚠️ Could not analyze component-status (no-base) — a build may have failed; results may be incomplete.

@github-actions

Copy link
Copy Markdown
Contributor

♿ Accessibility (axe) — components changed in this PR

✅ No a11y issues in the stories this PR changed.

Scope: only stories in the files/component folders this PR changed. It can't yet flag downstream ripple from shared-code/token changes, or diff against main (planned: base-vs-head delta).

@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Coverage Report for packages/react

Status Category Percentage Covered / Total
🔵 Lines 69.17% 29318 / 42381
🔵 Statements 68.13% 31019 / 45526
🔵 Functions 62.03% 6993 / 11273
🔵 Branches 61.88% 22020 / 35584
File CoverageNo changed files found.
Generated in workflow #17605 for commit bf3d33a by the Vitest Coverage Report Action

Follows perf-changed.ts now mapping any changed source file to the
stories that render it. The paths filter only matched *.stories.tsx, so
a component-only change never started the workflow in the first place.

The negations mirror the script's own exclusions: test, snapshot and docs
files cannot change what a story renders, so without them the workflow
would build Storybook only for the measurement to find nothing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

✅ No untranslated copy added

Every user-visible string in this PR comes from the i18n layer. Codebase total unchanged at 133.

@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

✅ Storybook docs — no pages lost

Every page reachable on main is still reachable here.

Links point at this PR's Storybook build — browse the full Storybook.

Snapshot of the Storybook index (docs pages + stories) compared against main. Non-blocking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci react Changes affect packages/react

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant