CLI: Sync storybook setup prompt with eval pattern-copy-play improvements#34596
CLI: Sync storybook setup prompt with eval pattern-copy-play improvements#34596kasperpeulen wants to merge 4 commits into
Conversation
The CLI `storybook setup` prompt (shipped to end users) carried a near-identical copy of the eval `pattern-copy-play` prompt. Missed it in the previous commit — syncing the same Step 7 now.
The eval `pattern-copy-play` prompt has two pieces of guidance that were not in the CLI's shipped `storybook setup` prompt: - An end-state paragraph clarifying the goal: every component, from a button to a full page, should be addable without story-specific workarounds. - Args-vs-render authoring guidance: prefer `args` for prop-driven stories, only reach for `render` when composition is needed. Syncing both into the CLI prompt. The new Args/Render subsection uses two helpers (`getArgsStoryExample`, `getRenderCompositionExample`) matching the existing CSF Factory / CSF3 branch style.
'Render call' could read as 'you need a render: () => ... function', which is wrong — args stories have no render call and that's exactly the preferred shape for prop-driven components. Softening to 'just rendering the component in the story is enough' keeps the intent (shared preview does the heavy lifting) without steering toward render().
|
View your CI Pipeline Execution ↗ for commit ed2f5dd
☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughUpdated the Storybook AI prompt generator to require a shared preview for providers/CSS/network mocks, add an "Args vs render" guidance with two story templates, replace Step 7 with a single Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
code/lib/cli-storybook/src/ai/prompt.ts (1)
777-780: Add a post-interaction assertion (or remove the click) inInsideCardexamples.Right now the example performs a click without verifying its effect, which models “click-around” behavior instead of behavior validation.
Suggested adjustment
- play: async ({ canvas, userEvent }) => { + play: async ({ canvas }) => { await expect(canvas.getByRole('button', { name: /save/i })).toBeVisible(); - await userEvent.click(canvas.getByRole('button', { name: /save/i })); },- play: async ({ canvas, userEvent }) => { + play: async ({ canvas }) => { await expect(canvas.getByRole('button', { name: /save/i })).toBeVisible(); - await userEvent.click(canvas.getByRole('button', { name: /save/i })); },Based on learnings: “Test real behavior, not just syntax patterns.”
Also applies to: 809-812
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/lib/cli-storybook/src/ai/prompt.ts` around lines 777 - 780, The story’s play handler for the InsideCard examples clicks the "save" button but never asserts the resulting behavior; update the play functions that use canvas.getByRole('button', { name: /save/i }) and userEvent.click to include a post-interaction assertion that verifies the expected outcome (for example, a "saved" confirmation appears, the button becomes disabled, or some UI text changes), or if there is no observable effect to assert, remove the userEvent.click call entirely; apply this change for the play blocks shown (the one using canvas.getByRole('button', { name: /save/i }) at the snippet and the similar occurrence around lines 809-812) so each play either validates behavior or does not perform the click.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@code/lib/cli-storybook/src/ai/prompt.ts`:
- Around line 777-780: The story’s play handler for the InsideCard examples
clicks the "save" button but never asserts the resulting behavior; update the
play functions that use canvas.getByRole('button', { name: /save/i }) and
userEvent.click to include a post-interaction assertion that verifies the
expected outcome (for example, a "saved" confirmation appears, the button
becomes disabled, or some UI text changes), or if there is no observable effect
to assert, remove the userEvent.click call entirely; apply this change for the
play blocks shown (the one using canvas.getByRole('button', { name: /save/i })
at the snippet and the similar occurrence around lines 809-812) so each play
either validates behavior or does not perform the click.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6caea4f5-7303-4cf8-a584-67c691aa26a6
📒 Files selected for processing (1)
code/lib/cli-storybook/src/ai/prompt.ts
yannbf
left a comment
There was a problem hiding this comment.
Nice! Can you add the project language (js/ts) to the metadata banner at the top of the prompt and conditionally mention typescript stuff as well?
Mirrors the eval-side rename (PR #34595). The prompt now asks the agent to name the CSS-check story `CssCheck` specifically (instead of any story that happens to call `getComputedStyle`). That lets the AI-stories vitest run in core identify the story by name and report its pass/fail result in the \`ai-setup-final-scoring\` telemetry event. Consensus from Slack: keep the story named, drop the tag idea. The story also ends up being educational — a visible example of how to verify CSS loaded.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/lib/cli-storybook/src/ai/prompt.ts`:
- Around line 780-783: The play function contains a click-only interaction (play
async ({ canvas, userEvent }) => { ...
userEvent.click(canvas.getByRole('button', { name: /save/i })) }) with no
post-click assertion; update the play to include a post-condition assertion
after userEvent.click (for example assert that a success message appears, the
save button becomes disabled, a form value changes, or a specific element is
visible/contains expected text) OR remove the click entirely; apply the same
change to the other similar play helper that uses canvas.getByRole('button', {
name: /save/i }) and userEvent.click to ensure each example asserts real
behavior rather than only performing a click.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d26dbb4e-a1bb-44d9-9e08-8f3032f18a38
📒 Files selected for processing (1)
code/lib/cli-storybook/src/ai/prompt.ts
| play: async ({ canvas, userEvent }) => { | ||
| await expect(canvas.getByRole('button', { name: /save/i })).toBeVisible(); | ||
| await userEvent.click(canvas.getByRole('button', { name: /save/i })); | ||
| }, |
There was a problem hiding this comment.
Avoid click-only interactions in example play functions.
These clicks have no post-click assertion, which teaches a weaker pattern than the surrounding guidance. Add a post-condition assertion or remove the click.
Based on learnings, "Test real behavior, not just syntax patterns".
Also applies to: 812-815
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/lib/cli-storybook/src/ai/prompt.ts` around lines 780 - 783, The play
function contains a click-only interaction (play async ({ canvas, userEvent })
=> { ... userEvent.click(canvas.getByRole('button', { name: /save/i })) }) with
no post-click assertion; update the play to include a post-condition assertion
after userEvent.click (for example assert that a success message appears, the
save button becomes disabled, a form value changes, or a specific element is
visible/contains expected text) OR remove the click entirely; apply the same
change to the other similar play helper that uses canvas.getByRole('button', {
name: /save/i }) and userEvent.click to ensure each example asserts real
behavior rather than only performing a click.
The AI setup prompt now asks the agent to include exactly one story
named `CssCheck` — whose `play` function asserts a component-specific
computed style via `getComputedStyle`. That story proves the shared
preview actually loaded the app's CSS (rather than just: something
rendered that we can call `toBeVisible` on).
Currently, the `ai-setup-final-scoring` telemetry event only reports
an aggregate pass/fail count. We cannot tell "all tests passed" from
"CSS check passed" — if the suite fails, the reason is opaque.
Wires the CssCheck story's individual result into the event:
- `TestRunSummary`: new optional `storyResults` field with the raw
`StoryTestResult[]` (no other callers changed — `ghost-stories`
stays on the aggregate summary).
- `parseVitestResults`: populates `storyResults` (it was already
building the array internally and discarding it).
- `ai-setup-utils`: new `findCssCheckStoryResult(storyResults)`
helper, locates the story by its CSF-sanitized `--css-check`
storyId suffix. Defensive `.toLowerCase()` kept in case upstream
stops sanitizing.
- `ai-setup-channel`: calls the helper after `runStoryTests`, spreads
`cssCheck: { storyId, status }` into the telemetry payload when
a match is present; omits the field when the agent did not create
the story.
Background: the alternative considered was an after-each runtime hook
that sniffs stylesheet loading. Rejected upstream because it needs a
fragile "not ours" filter against vitest-browser, Storybook, and
addon sheets, and it can only answer "was something applied" rather
than "did this component's expected styles apply". A specifically
named story gives us a clean telemetry signal without any of that.
Companion PRs add the story name to both prompts:
- #34595 (eval harness prompt + grade check)
- #34596 (user-facing `storybook setup` prompt)
Bring the three prompt-content changes that were about to ship in #34596 onto the post-refactor layout. Applies to code/lib/cli-storybook/src/ai/prompts/ pattern-copy-play.ts (previously getSetupInstructions in prompt.ts): - New end-state paragraph in the intro clarifying that the shared preview should own all providers, CSS, browser state, and network mocks so rendering the component in the story is enough. - New "#### Args vs render" subsection under Step 5 with two full examples (args-driven Button, render-based composition inside Card), via two new self-contained helpers getArgsStoryExample and getRenderCompositionExample. - New Step 7 "Prove CSS is loaded in exactly one story named CssCheck" asserting a component-specific computed style via getComputedStyle to catch "renders but CSS never loaded" failures. Steps 8 and 9 renumbered accordingly. Makes #34596 redundant against this branch.
|
Superseded by #34602, which ports these same changes (end-state paragraph, Args vs render subsection, Step 7 Closing; the eval grade-check companion is #34595, and the core telemetry companion is #34600. |
What I did
Brings the CLI's shipped
storybook setupAI prompt (code/lib/cli-storybook/src/ai/prompt.ts) up to parity with recent improvements made to the eval harness'spattern-copy-playprompt. The two were near-identical until recently, but eval-side edits have drifted forward. This PR ports the drift that applies to end-users without touching anything eval-only.Three synced pieces:
New Step 7: "Prove CSS is loaded in exactly one story" — asks the agent to include, in exactly one story, a
playfunction that asserts a component-specific computed style viagetComputedStyle. Catches the "component renders but CSS never loaded" failure (missing global CSS import, Tailwind not configured, theme provider not wired up) that otherwise passestoBeVisiblechecks. Step 8/9 renumbered accordingly.End-state paragraph in the intro — clarifies the goal: the shared preview should own all providers/CSS/browser state/network mocks so that just rendering the component in the story is enough. (Original eval text said "the component import and a render call", which biased toward
render: () => ...; softened in a later commit here to avoid that bias.)New
#### Args vs rendersubsection under Step 5 — teaches when to preferargsvs when to reach forrender, with two full examples (Buttonwithargs,Button inside Cardwithrender). Follows the existinghasCsfFactoryPreview/ CSF3 branch pattern via two new helpers:getArgsStoryExampleandgetRenderCompositionExample.None of this changes behavior — it's prompt content shipped to the user as markdown by
storybook setup. The only code additions are the two new helper functions that follow the style of the existinggetStoryExample/getNeedsWorkTagExample/getPageStoryExamplehelpers.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Pure prompt content change in a function that returns a dedented markdown string. No existing tests target
getSetupInstructionsoutput; I did not add snapshot tests — the prompt evolves too often for snapshots to be useful, and the behavior we care about is "does the agent follow the prompt", which is measured by the eval harness (companion PR #34595 adds a diff-level assertion for the CSS step specifically).Manual testing
storybook setup(orstorybook initflow that invokes it) against a small React app.#### Args vs rendersubsection with two code examples.getComputedStyleexample.preview.meta(...)/meta.story(...)syntax; for CSF3 projects, they useMeta<typeof ...>/StoryObj<typeof meta>.Documentation
No user-facing documentation changes — the prompt itself is the doc, delivered at
storybook setuptime.Checklist for Maintainers
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes.Labels applied:
build,ci:normal.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
Documentation
New Features