Prompt: Run vitest fewer times, improve play functions#34651
Open
Prompt: Run vitest fewer times, improve play functions#34651
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughaiSetup now records a human-friendly package manager, removes CSF-factory preview flag from ProjectInfo/telemetry, delays majorVersion parsing until after framework/renderer/builder detection, makes markdown title configurable and writes to stdout when no output path is given, and refactors prompt/story/template generation and preview/main helpers. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as CLI
participant Detector as StorybookDetector
participant Generator as MarkdownGenerator
participant Telemetry as Telemetry
participant Snapshot as SnapshotStore
participant Logger as Logger/Stdout
CLI->>Detector: run aiSetup -> detect projectInfo()
alt Detector returns projectInfo
Detector-->>CLI: projectInfo (with pretty packageManager)
CLI->>Generator: generateMarkdownOutput(projectInfo, title)
Generator-->>Logger: write via logger (or stdout)
CLI->>Telemetry: emit telemetry (project section without CSF factory)
CLI->>Snapshot: write ai-setup-pending record
else Detector returns undefined
Detector-->>CLI: undefined
CLI->>Generator: generateMarkdownOutput(undefined, title)
Generator-->>Logger: write directly to stdout
CLI->>Telemetry: emit telemetry without project section
CLI->>Snapshot: skip write
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Comment |
8 tasks
yannbf
commented
Apr 29, 2026
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 184 | 184 | 0 |
| Self size | 836 KB | 876 KB | 🚨 +40 KB 🚨 |
| Dependency size | 68.26 MB | 68.27 MB | 🚨 +2 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
Sidnioulz
reviewed
Apr 29, 2026
Sidnioulz
reviewed
Apr 29, 2026
Sidnioulz
reviewed
Apr 29, 2026
Sidnioulz
reviewed
Apr 29, 2026
Sidnioulz
reviewed
Apr 29, 2026
Sidnioulz
reviewed
Apr 29, 2026
Sidnioulz
reviewed
Apr 29, 2026
Sidnioulz
approved these changes
Apr 29, 2026
shilman
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/index.ts`:
- Around line 61-65: The gate in ai/index.ts uses an && between the
rendererPackage and builderPackage mismatch checks, allowing a project with only
one mismatch to pass; update the condition around projectInfo so it rejects when
either side mismatches (i.e., use || between the mismatch checks or switch to a
positive check that requires rendererPackage === '@storybook/react' &&
builderPackage === '@storybook/builder-vite'), referencing
projectInfo.rendererPackage and projectInfo.builderPackage and the constants
'@storybook/react' and '@storybook/builder-vite'.
- Around line 56-59: The empty catch block in the Storybook setup-detection flow
swallows all errors; update the catch to capture the exception (e.g., catch
(err)) and only suppress known/expected cases (like file-not-found/ENOENT,
permission errors, or JSON/SyntaxError from config parsing) while rethrowing or
logging unexpected errors. Change the anonymous catch in the try that
reads/parses Storybook config to inspect err.code or err.constructor (e.g.,
'ENOENT'/'EACCES' or instance of SyntaxError) and handle those by falling
through, but rethrow or surface any other errors so regressions or I/O failures
aren’t hidden. Ensure the updated catch references the same try block (the
Storybook project detection code) and preserves the existing generic-prompt
fallback for expected cases.
- Line 52: The code is setting projectInfo.packageManager to
getPrettyPackageManagerName(packageManagerName) which uses CLI input instead of
the detected project value; update the assignment to call
getPrettyPackageManagerName with the detected package manager value (the
variable/field that holds the detection result, e.g., detectedPackageManager or
projectInfo.packageManagerRaw) so projectInfo.packageManager reflects detection
when the CLI option is omitted; locate the site using
getPrettyPackageManagerName and packageManagerName in ai/index.ts and replace
the argument with the detected package-manager variable or projectInfo's raw
detection field.
🪄 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: b124b928-de7a-4ed4-8e44-d31a91c5a8b6
📒 Files selected for processing (1)
code/lib/cli-storybook/src/ai/index.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
code/lib/cli-storybook/src/ai/index.ts (2)
58-58:⚠️ Potential issue | 🟠 MajorUse the detected package-manager ID here, not the pretty label or raw CLI option.
Line 58 is still populating
projectInfo.packageManagerfrompackageManagerName, so detection-only runs end up asunknown package manager, and the prompt falls back to<your-package-manager> ...even whengetStorybookData()already found one. Keep the raw detected value inprojectInfo, and pretty-print only when rendering the overview table.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/lib/cli-storybook/src/ai/index.ts` at line 58, The projectInfo.packageManager is being set to the pretty label instead of the raw detected ID; update the assignment so projectInfo.packageManager uses the detected package-manager ID (packageManagerName or the value returned by getStorybookData) and keep getPrettyPackageManagerName(packageManagerName) only where the overview table is rendered; locate the packageManager property assignment in the code that builds projectInfo and replace the pretty-print usage with the raw ID while leaving getPrettyPackageManagerName for display code that renders project info.
71-74:⚠️ Potential issue | 🟠 MajorReject when either side is not React+Vite.
This guard only exits when both packages mismatch. A React+Webpack project or a Vue+Vite project still passes and gets React/Vite-specific setup instructions. This needs
||between the mismatch checks, or a positive===check on both packages.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/lib/cli-storybook/src/ai/index.ts` around lines 71 - 74, The condition in the if statement currently uses && to check if both rendererPackage and builderPackage are not the expected React and Vite packages, which allows mismatched pairs through. Update this logic in the conditional guarding the React+Vite setup instructions to use || instead of &&, so it correctly rejects when either rendererPackage is not '@storybook/react' or builderPackage is not '@storybook/builder-vite'.
🤖 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 423-428: The example smoke `play` for the exported Primary story
uses a visibility-only assertion (Primary.play calling
canvas.getByRole(...).toBeVisible()), which violates Step 6; update both Primary
plays (the ones around the current block and the one at 453-458) to either be
removed or replaced with a non-trivial signal assertion (for example assert
enabled state, text content, or checked state) — e.g., change the play to await
expect(canvas.getByRole('button', { name: /order now/i })).toBeEnabled() or
await expect(...).toHaveTextContent('Order now'), referencing the Primary export
and its play function and the canvas.getByRole call so reviewers can locate the
spots to modify.
- Around line 63-67: The packageManagerRule currently prints raw package-manager
IDs (e.g., yarn2) which can produce invalid shell guidance; update
packageManagerRule to normalize the detected token (mirror the same mapping used
by installCommand) before embedding it in the string — e.g., map 'yarn1' and
'yarn2' to 'yarn', keep 'pnpm' → 'pnpm', 'bun' → 'bun', otherwise use 'npm' —
then use that normalized command in the returned message for the "Use `<pkg>`
for every install" branch so prompts show valid installer names.
---
Duplicate comments:
In `@code/lib/cli-storybook/src/ai/index.ts`:
- Line 58: The projectInfo.packageManager is being set to the pretty label
instead of the raw detected ID; update the assignment so
projectInfo.packageManager uses the detected package-manager ID
(packageManagerName or the value returned by getStorybookData) and keep
getPrettyPackageManagerName(packageManagerName) only where the overview table is
rendered; locate the packageManager property assignment in the code that builds
projectInfo and replace the pretty-print usage with the raw ID while leaving
getPrettyPackageManagerName for display code that renders project info.
- Around line 71-74: The condition in the if statement currently uses && to
check if both rendererPackage and builderPackage are not the expected React and
Vite packages, which allows mismatched pairs through. Update this logic in the
conditional guarding the React+Vite setup instructions to use || instead of &&,
so it correctly rejects when either rendererPackage is not '@storybook/react' or
builderPackage is not '@storybook/builder-vite'.
🪄 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: 233c84c3-2f15-4ef4-89a5-1af87788781b
📒 Files selected for processing (5)
code/lib/cli-storybook/src/ai/index.tscode/lib/cli-storybook/src/ai/prompt.tsdocs/configure/integration/eslint-plugin.mdxscripts/eval/prompts/optimized-tests.mdscripts/eval/prompts/rac-prompt.md
✅ Files skipped from review due to trivial changes (1)
- docs/configure/integration/eslint-plugin.mdx
Sidnioulz
force-pushed
the
yann/prompt-optimized-tests
branch
from
9ff26ad to
bf4846d
Compare
Sidnioulz
force-pushed
the
yann/prompt-optimized-tests
branch
from
bf4846d to
ec34264
Compare
…relaxed-limits
yannbf
commented
Apr 30, 2026
| } | ||
|
|
||
| getCommandName(): string { | ||
| return 'npm'; |
Member
Author
There was a problem hiding this comment.
Suggested change
| return 'npm'; | |
| return 'bun'; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #
What I did
This PR adds improvements in a new prompt which tries to optimize run times by only running Vitest at the end for verification and analyzing the codebase less deeply.
Eval runs can be seen here
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Caution
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 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