Skip to content

Core: Refactor channel initialization#33520

Merged
yannbf merged 5 commits into
nextfrom
yann/adjust-ghost-stories-run
Jan 14, 2026
Merged

Core: Refactor channel initialization#33520
yannbf merged 5 commits into
nextfrom
yann/adjust-ghost-stories-run

Conversation

@yannbf
Copy link
Copy Markdown
Member

@yannbf yannbf commented Jan 13, 2026

Closes #

What I did

The ghost stories channel needed an extra validation for the session id. This PR also adds fixes and restructures telemetry data.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end 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

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

  • Make 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 pull request has been released as version 0.0.0-pr-33520-sha-090864bc. Try it out in a new sandbox by running npx storybook@0.0.0-pr-33520-sha-090864bc sandbox or in an existing project with npx storybook@0.0.0-pr-33520-sha-090864bc upgrade.

More information
Published version 0.0.0-pr-33520-sha-090864bc
Triggered by @yannbf
Repository storybookjs/storybook
Branch yann/adjust-ghost-stories-run
Commit 090864bc
Datetime Tue Jan 13 14:52:36 UTC 2026 (1768315956)
Workflow run 20961165502

To request a new release of this pull request, mention the @storybookjs/core team.

core team members can create a new canary release here or locally with gh workflow run --repo storybookjs/storybook publish.yml --field pr=33520

Summary by CodeRabbit

  • Bug Fixes

    • Added early guards to avoid processing when required data is missing and standardized error handling paths.
  • Enhancements

    • Unified error reporting under a single runError field.
    • Test/run reports now include unique error counts and no longer expose explicit failed/failureRate metrics.
    • Narrowed package matching for router detection to reduce false positives.
  • New Features

    • Added two import-related error categories for clearer diagnostics.
  • Tests

    • Updated fixtures and assertions to match the new reporting shape.

✏️ Tip: You can customize this high-level summary in your review settings.

@yannbf yannbf self-assigned this Jan 13, 2026
@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented Jan 13, 2026

View your CI Pipeline Execution ↗ for commit 090864b

Command Status Duration Result
nx run-many -t compile,check,knip,test,pretty-d... ✅ Succeeded 9m 58s View ↗

☁️ Nx Cloud last updated this comment at 2026-01-13 15:01:58 UTC

@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented Jan 13, 2026

View your CI Pipeline Execution ↗ for commit e6921f6

Command Status Duration Result
nx run-many -t compile,check,knip,test,pretty-d... ✅ Succeeded 9m 39s View ↗

☁️ Nx Cloud last updated this comment at 2026-01-13 08:58:07 UTC

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 13, 2026

📝 Walkthrough

Walkthrough

Replaces success/error telemetry fields with runError, updates ghost-stories control guards, changes Vitest parsing to track unique error messages (removing failed/failureRate), adjusts TestRunSummary/type shapes, adds two import-related error categories, tightens router package list, and small error-message simplifications in candidate discovery.

Changes

Cohort / File(s) Summary
Ghost-stories channel & tests
code/core/src/core-server/server-channel/ghost-stories-channel.ts, code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
Added early guard for missing lastEvents/lastInit, deferred sessionId access; replaced telemetry payload success/error fields with runError; updated tests/expectations to use runError.
Run/test utilities & types
code/core/src/core-server/utils/ghost-stories/run-story-tests.ts, code/core/src/core-server/utils/ghost-stories/types.ts
Error reporting shape changed: remove top-level success/error and summary.failed/failureRate; use runError?: string; CategorizedError drops description, adds uniqueCount. Public TestRunSummary shape adjusted.
Vitest report parsing & tests
code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts, code/core/src/core-server/utils/ghost-stories/parse-vitest-report.test.ts
Renamed param to report, iterate report.testResults; track per-category uniqueErrors → expose uniqueCount; removed failed and failureRate from outputs; removed getCategoryDescription usage; updated tests to new summary shape/assertions.
Error categorization & tests
code/core/src/shared/utils/categorize-render-errors.ts, code/core/src/shared/utils/categorize-render-errors.test.ts
Added TEST_FILE_IMPORT_ERROR and DYNAMIC_MODULE_IMPORT_ERROR categories and descriptions with high priority; added tests covering test-file and dynamic-module import failures.
Ecosystem identifier
code/core/src/shared/utils/ecosystem-identifier.ts
Replaced wildcard router package patterns with an explicit list of router package names in ROUTER_PACKAGES.
Candidate discovery error handling
code/core/src/core-server/utils/ghost-stories/get-candidates.ts
Replaced typed catch extraction with a bare catch and a generic 'Failed to find candidates' error return (preserves globMatchCount).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

✨ Finishing touches
  • 📝 Generate docstrings

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
code/core/src/core-server/utils/ghost-stories/parse-vitest-report.test.ts (1)

94-159: Test data has inconsistent counts: 5 assertion results defined but numTotalTests is 4.

The mock data defines 5 stories (Story1-Story5) but sets numTotalTests: 4. While the test correctly expects total: 4 (since parseVitestResults uses numTotalTests directly), the test fixture should have consistent counts that reflect realistic Vitest output. Either reduce to 4 assertion results with numTotalTests: 4, or update to 5 with numTotalTests: 5.

🧹 Nitpick comments (2)
code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts (1)

41-51: Consider using proper typing instead of any.

The Record<string, any> loses type safety. Consider using the actual CategorizedError type for better type checking.

♻️ Suggested improvement
-  const categorizedErrors = Array.from(map.entries()).reduce<Record<string, any>>(
+  const categorizedErrors = Array.from(map.entries()).reduce<Record<string, CategorizedError>>(
     (acc, [category, data]) => {
       acc[category] = {
         uniqueCount: data.uniqueErrors.size,
         count: data.count,
+        category,
         matchedDependencies: Array.from(data.matchedDependencies).sort(),
       };
       return acc;
     },
     {}
   );

Note: You'd need to import CategorizedError from ./types and include the category field to satisfy the interface.

code/core/src/core-server/server-channel/ghost-stories-channel.ts (1)

102-106: Consider logging the actual error for debugging purposes.

The bare catch clause discards the actual error. While the generic runError message is appropriate for telemetry, the original error could be valuable for debugging.

♻️ Optional: Log error before sending generic telemetry
-    } catch {
+    } catch (err) {
+      // Log the actual error for debugging, but send generic message to telemetry
+      console.error('[ghost-stories]', err);
       telemetry('ghost-stories', {
         runError: 'Unknown error during ghost run',
         stats,
       });

Note: Per coding guidelines, if a logger from storybook/internal/node-logger is available in this context, prefer using it over console.error.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e6921f6 and 1a755c1.

📒 Files selected for processing (9)
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.ts
  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.test.ts
  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts
  • code/core/src/core-server/utils/ghost-stories/run-story-tests.ts
  • code/core/src/core-server/utils/ghost-stories/types.ts
  • code/core/src/shared/utils/categorize-render-errors.test.ts
  • code/core/src/shared/utils/categorize-render-errors.ts
  • code/core/src/shared/utils/ecosystem-identifier.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,jsx,ts,tsx,json,md,html,css,scss}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Format code using Prettier with yarn prettier --write <file>

Files:

  • code/core/src/core-server/utils/ghost-stories/run-story-tests.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.ts
  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.test.ts
  • code/core/src/shared/utils/ecosystem-identifier.ts
  • code/core/src/shared/utils/categorize-render-errors.test.ts
  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts
  • code/core/src/shared/utils/categorize-render-errors.ts
  • code/core/src/core-server/utils/ghost-stories/types.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
**/*.{js,jsx,json,html,ts,tsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Run ESLint checks using yarn lint:js:cmd <file> or the full command cross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directives to fix linting errors before committing

Files:

  • code/core/src/core-server/utils/ghost-stories/run-story-tests.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.ts
  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.test.ts
  • code/core/src/shared/utils/ecosystem-identifier.ts
  • code/core/src/shared/utils/categorize-render-errors.test.ts
  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts
  • code/core/src/shared/utils/categorize-render-errors.ts
  • code/core/src/core-server/utils/ghost-stories/types.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Enable TypeScript strict mode across all packages

Files:

  • code/core/src/core-server/utils/ghost-stories/run-story-tests.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.ts
  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.test.ts
  • code/core/src/shared/utils/ecosystem-identifier.ts
  • code/core/src/shared/utils/categorize-render-errors.test.ts
  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts
  • code/core/src/shared/utils/categorize-render-errors.ts
  • code/core/src/core-server/utils/ghost-stories/types.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx,js,jsx}: Export functions from modules if they need to be tested
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size

Files:

  • code/core/src/core-server/utils/ghost-stories/run-story-tests.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.ts
  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.test.ts
  • code/core/src/shared/utils/ecosystem-identifier.ts
  • code/core/src/shared/utils/categorize-render-errors.test.ts
  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts
  • code/core/src/shared/utils/categorize-render-errors.ts
  • code/core/src/core-server/utils/ghost-stories/types.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use logger from storybook/internal/node-logger for server-side logging in Node.js code

Files:

  • code/core/src/core-server/utils/ghost-stories/run-story-tests.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.ts
  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.test.ts
  • code/core/src/shared/utils/ecosystem-identifier.ts
  • code/core/src/shared/utils/categorize-render-errors.test.ts
  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts
  • code/core/src/shared/utils/categorize-render-errors.ts
  • code/core/src/core-server/utils/ghost-stories/types.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
**/*.{test,spec}.{ts,tsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{test,spec}.{ts,tsx}: Test files should follow the naming pattern *.test.ts, *.test.tsx, *.spec.ts, or *.spec.tsx
Follow the spy mocking rules defined in .cursor/rules/spy-mocking.mdc for consistent mocking patterns with Vitest

Files:

  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.test.ts
  • code/core/src/shared/utils/categorize-render-errors.test.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
**/*.test.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)

**/*.test.{ts,tsx,js,jsx}: Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Use vi.mocked() to type and access the mocked functions in Vitest tests
Implement mock behaviors in beforeEach blocks in Vitest tests
Mock all required dependencies that the test subject uses
Each mock implementation should return a Promise for async functions in Vitest
Mock implementations should match the expected return type of the original function
Mock all required properties and methods that the test subject uses in Vitest tests
Avoid direct function mocking without vi.mocked() in Vitest tests
Avoid mock implementations outside of beforeEach blocks in Vitest tests
Avoid mocking without the spy: true option in Vitest tests
Avoid inline mock implementations within test cases in Vitest tests
Avoid mocking only a subset of required dependencies in Vitest tests
Mock at the highest level of abstraction needed in Vitest tests
Keep mock implementations simple and focused in Vitest tests
Use type-safe mocking with vi.mocked() in Vitest tests
Document complex mock behaviors in Vitest tests
Group related mocks together in Vitest tests

Files:

  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.test.ts
  • code/core/src/shared/utils/categorize-render-errors.test.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{test,spec}.{ts,tsx,js,jsx}: Write meaningful unit tests that actually import and call the functions being tested, not just verify syntax patterns
Achieve high test coverage of business logic, aiming for 75%+ coverage of statements/lines
Cover all branches, conditions, edge cases, error paths, and different input variations in unit tests
Use vi.mock() to mock file system, loggers, and other external dependencies in tests

Files:

  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.test.ts
  • code/core/src/shared/utils/categorize-render-errors.test.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
🧠 Learnings (20)
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests

Applied to files:

  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.test.ts
  • code/core/src/shared/utils/categorize-render-errors.test.ts
  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases in Vitest tests

Applied to files:

  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.test.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies in Vitest tests

Applied to files:

  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.test.ts
  • code/core/src/shared/utils/categorize-render-errors.test.ts
  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Keep mock implementations simple and focused in Vitest tests

Applied to files:

  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together in Vitest tests

Applied to files:

  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.test.ts
  • code/core/src/shared/utils/categorize-render-errors.test.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
📚 Learning: 2025-12-22T22:03:40.123Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Applies to **/*.{test,spec}.{ts,tsx,js,jsx} : Write meaningful unit tests that actually import and call the functions being tested, not just verify syntax patterns

Applied to files:

  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.test.ts
  • code/core/src/shared/utils/categorize-render-errors.test.ts
📚 Learning: 2025-12-22T22:03:40.123Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Applies to **/*.{test,spec}.{ts,tsx,js,jsx} : Cover all branches, conditions, edge cases, error paths, and different input variations in unit tests

Applied to files:

  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.test.ts
  • code/core/src/shared/utils/categorize-render-errors.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mock implementations outside of `beforeEach` blocks in Vitest tests

Applied to files:

  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.test.ts
  • code/core/src/shared/utils/categorize-render-errors.test.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in `beforeEach` blocks in Vitest tests

Applied to files:

  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.test.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses in Vitest tests

Applied to files:

  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.test.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Each mock implementation should return a Promise for async functions in Vitest

Applied to files:

  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.test.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock at the highest level of abstraction needed in Vitest tests

Applied to files:

  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.test.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
📚 Learning: 2025-12-22T22:03:40.123Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Export functions from modules if they need to be tested

Applied to files:

  • code/core/src/shared/utils/categorize-render-errors.test.ts
📚 Learning: 2025-12-22T22:03:40.123Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Applies to **/*.{test,spec}.{ts,tsx,js,jsx} : Use `vi.mock()` to mock file system, loggers, and other external dependencies in tests

Applied to files:

  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock implementations should match the expected return type of the original function

Applied to files:

  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required dependencies that the test subject uses

Applied to files:

  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use `vi.mock()` with the `spy: true` option for all package and file mocks in Vitest tests

Applied to files:

  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use type-safe mocking with `vi.mocked()` in Vitest tests

Applied to files:

  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid direct function mocking without `vi.mocked()` in Vitest tests

Applied to files:

  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use `vi.mocked()` to type and access the mocked functions in Vitest tests

Applied to files:

  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
🧬 Code graph analysis (3)
code/core/src/core-server/utils/ghost-stories/run-story-tests.ts (1)
code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts (1)
  • parseVitestResults (61-121)
code/core/src/core-server/server-channel/ghost-stories-channel.ts (1)
code/core/src/telemetry/index.ts (2)
  • getSessionId (21-21)
  • telemetry (31-72)
code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts (1)
code/core/src/shared/utils/categorize-render-errors.ts (1)
  • ErrorCategory (26-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: normal
  • GitHub Check: nx
  • GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (20)
code/core/src/shared/utils/categorize-render-errors.ts (3)

20-24: LGTM!

The new error categories are well-named and appropriately placed. The inline comments provide useful context for categorizing Vite vs Vitest-specific errors.


95-105: LGTM!

The priority level of 95 appropriately positions these specific import errors below the generic MODULE_IMPORT_ERROR (100) while still being higher priority than hook and provider errors. The match conditions are specific and well-targeted.


261-266: LGTM!

Descriptions are clear and consistent with the existing pattern.

code/core/src/shared/utils/categorize-render-errors.test.ts (1)

28-54: LGTM!

Good test coverage for the new error categories. The tests use realistic error messages that would be encountered in actual Vitest/Vite environments. Both categories are tested with full error messages and shorter variants.

code/core/src/core-server/utils/ghost-stories/types.ts (2)

8-13: LGTM!

The addition of uniqueCount to CategorizedError provides valuable insight into the distinct error patterns within each category, complementing the total count.


32-33: LGTM!

Renaming error to runError improves clarity by distinguishing run-level failures (e.g., "Playwright not installed", "JSON report not found") from individual test errors.

code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts (2)

16-20: LGTM!

Good approach using a Set<string> for uniqueErrors to track distinct error messages per category. This enables accurate unique counting without duplicates.


110-120: LGTM!

The return structure correctly matches the updated TestRunSummary type with the summary wrapper property.

code/core/src/core-server/utils/ghost-stories/run-story-tests.ts (4)

65-70: LGTM!

Clean transition from the old success/error pattern to runError. The error message is preserved for telemetry purposes.


72-77: LGTM!

Consistent use of runError for the missing JSON report case.


85-90: LGTM!

Consistent use of runError for JSON parsing failures.


93-98: LGTM!

The catch-all error handler correctly returns runError with a generic message. Using duration: 0 is acceptable here since timing wasn't captured before the exception.

code/core/src/core-server/server-channel/ghost-stories-channel.test.ts (5)

416-441: Test correctly updated to use runError field.

The mock return value and telemetry assertion are properly updated to use runError instead of error, consistent with the implementation changes.


471-482: LGTM!

The "no candidates found" error case correctly uses runError in the telemetry assertion.


515-527: LGTM!

The "JSON report not found" error case correctly uses runError in the telemetry assertion.


568-580: LGTM!

The "test startup error" case correctly uses runError in the telemetry assertion.


594-598: LGTM!

The general error handler test correctly expects runError: 'Unknown error during ghost run'.

code/core/src/core-server/server-channel/ghost-stories-channel.ts (3)

39-50: LGTM! Good defensive guard added.

The early return when lastEvents or lastInit are missing prevents potential undefined access issues. Moving sessionId retrieval after the guard avoids unnecessary async calls when we'd return early anyway.


97-101: LGTM!

The conditional spread pattern correctly includes runError only when present, avoiding adding runError: undefined to the telemetry payload.


74-81: No issues found. The code correctly checks candidatesResult.error (the actual property returned by getComponentCandidates()) and maps it to the runError telemetry key—this is intentional semantic transformation, not an inconsistency.

Comment on lines 100 to 109
export const ROUTER_PACKAGES = [
// e.g. react-router, react-easy-router
'*-router',
// e.g. react-router-dom
'*-router-*',
// e.g. @reach/router, @remix-run/router
'*/router',
'react-router',
'react-router-dom',
'react-easy-router',
'@remix-run/router',
'expo-router',
'@tanstack/*-router',
'wouter',
'@reach/router',
] as const;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for router-related package references in package.json files
rg -n --type json '"[^"]*router[^"]*"' -g 'package.json' | head -40

Repository: storybookjs/storybook

Length of output: 835


🏁 Script executed:

#!/bin/bash
# Check if there are any tests or documentation mentioning router package identification
rg -n -i 'router.*package' --type ts --type md -C2 | head -50

Repository: storybookjs/storybook

Length of output: 3704


🏁 Script executed:

#!/bin/bash
# Check the broader context of the ecosystem-identifier.ts file
head -120 code/core/src/shared/utils/ecosystem-identifier.ts | tail -30

Repository: storybookjs/storybook

Length of output: 842


🏁 Script executed:

cat -n code/core/src/telemetry/get-has-router-package.ts

Repository: storybookjs/storybook

Length of output: 1209


🏁 Script executed:

# Check if ecosystem-identifier.ts is imported and used in get-has-router-package.ts
rg -n 'ecosystem-identifier' code/core/src/telemetry/

Repository: storybookjs/storybook

Length of output: 160


🏁 Script executed:

# Check all usages of ROUTER_PACKAGES to understand scope
rg -n 'ROUTER_PACKAGES' code/

Repository: storybookjs/storybook

Length of output: 447


🏁 Script executed:

# Check git history to see when these diverged
git log --oneline --all -n 50 -- code/core/src/telemetry/get-has-router-package.ts code/core/src/shared/utils/ecosystem-identifier.ts

Repository: storybookjs/storybook

Length of output: 165


🏁 Script executed:

# Check if there's any documentation or comments explaining the difference
rg -n -B3 -A3 'routerPackages|ROUTER_PACKAGES' code/core/src/shared/utils/ecosystem-identifier.ts | head -50

Repository: storybookjs/storybook

Length of output: 493


🏁 Script executed:

# Check the context around pickDepsObject usage to understand how ROUTER_PACKAGES is used
rg -n -B5 -A5 'pickDepsObject.*ROUTER_PACKAGES' code/core/src/telemetry/get-known-packages.ts

Repository: storybookjs/storybook

Length of output: 665


🏁 Script executed:

# Let me see the pickDepsObject function to understand how it uses ROUTER_PACKAGES
rg -n -B10 -A15 'function pickDepsObject' code/core/src/telemetry/get-known-packages.ts

Repository: storybookjs/storybook

Length of output: 47


🏁 Script executed:

# Check what matchesPackagePattern does with wildcards
rg -n -B5 -A10 'function matchesPackagePattern' code/core/src/shared/utils/ecosystem-identifier.ts

Repository: storybookjs/storybook

Length of output: 622


🏁 Script executed:

# Find pickDepsObject definition in get-known-packages.ts
cat -n code/core/src/telemetry/get-known-packages.ts | head -100

Repository: storybookjs/storybook

Length of output: 3297


🏁 Script executed:

# Check if there are tests that validate the router package detection
find . -name "*.test.ts" -o -name "*.spec.ts" | xargs grep -l "ROUTER_PACKAGES\|routerPackages" 2>/dev/null

Repository: storybookjs/storybook

Length of output: 170


🏁 Script executed:

# Check the test file for ROUTER_PACKAGES tests
cat -n code/core/src/telemetry/get-known-packages.test.ts | grep -A 20 -B 5 "router\|ROUTER"

Repository: storybookjs/storybook

Length of output: 2232


🏁 Script executed:

# Also check the storybook-metadata.test.ts for router tests
cat -n code/core/src/telemetry/storybook-metadata.test.ts | grep -A 20 -B 5 "router\|ROUTER"

Repository: storybookjs/storybook

Length of output: 2591


🏁 Script executed:

# Check git history more detailed for the specific change to ROUTER_PACKAGES
git show 1a755c142d:code/core/src/shared/utils/ecosystem-identifier.ts 2>/dev/null | grep -A 15 "ROUTER_PACKAGES" || echo "Commit not found or file didn't exist"

Repository: storybookjs/storybook

Length of output: 747


🏁 Script executed:

# Check if there's documentation about the two-tier router detection approach
rg -n -i 'ecosystem.*analysis\|telemetry.*router' code/core/src/telemetry/ code/core/src/shared/utils/

Repository: storybookjs/storybook

Length of output: 47


🏁 Script executed:

# Verify that @tanstack/*-router wildcard properly matches @tanstack/react-router
cat > /tmp/test_pattern.js << 'EOF'
function globToRegex(pattern) {
  const escaped = pattern.replace(/[.+?^${}()|[\]\\]/g, '\\$&');
  const regexPattern = escaped.replace(/\*/g, '.*');
  return new RegExp(`^${regexPattern}$`);
}

const pattern = '@tanstack/*-router';
const regex = globToRegex(pattern);
const testPackages = [
  '@tanstack/react-router',
  '@tanstack/vue-router',
  '@tanstack/solid-router',
  '@tanstack/angular-router'
];

console.log(`Pattern: ${pattern}`);
console.log(`Regex: ${regex}`);
testPackages.forEach(pkg => {
  console.log(`  ${pkg}: ${regex.test(pkg) ? 'MATCH' : 'NO MATCH'}`);
});
EOF
node /tmp/test_pattern.js

Repository: storybookjs/storybook

Length of output: 252


Clarify the purpose and scope of ROUTER_PACKAGES; consider synchronizing with the comprehensive router list in get-has-router-package.ts if telemetry should cover all router ecosystems.

The narrowed scope from wildcards to explicit packages appears intentional (confirmed by tests that validate the current 8-package list). However, there's a significant inconsistency: ecosystem-identifier.ts defines ROUTER_PACKAGES with 8 packages, while a separate, more comprehensive list exists in get-has-router-package.ts with 26 explicit packages including routers for Vue (vue-router, unplugin-vue-router), Angular (@angular/router), Solid.js (@solidjs/router), and metaframeworks (next, gatsby, nuxt, @sveltejs/kit).

The two files serve different purposes—ecosystem-identifier is for telemetry analysis, while get-has-router-package is a simple boolean check—but this mismatch may miss router detection for non-React ecosystems. If telemetry should capture all supported routers, consider expanding ROUTER_PACKAGES or consolidating the definitions.

@storybook-app-bot
Copy link
Copy Markdown

storybook-app-bot Bot commented Jan 13, 2026

Package Benchmarks

Commit: 090864b, ran on 13 January 2026 at 14:59:50 UTC

The following packages have significant changes to their size or dependencies:

storybook

Before After Difference
Dependency count 49 49 0
Self size 20.30 MB 20.27 MB 🎉 -27 KB 🎉
Dependency size 16.52 MB 16.52 MB 0 B
Bundle Size Analyzer Link Link

@storybook/cli

Before After Difference
Dependency count 183 183 0
Self size 775 KB 775 KB 0 B
Dependency size 67.38 MB 67.35 MB 🎉 -27 KB 🎉
Bundle Size Analyzer Link Link

@storybook/codemod

Before After Difference
Dependency count 176 176 0
Self size 30 KB 30 KB 🎉 -36 B 🎉
Dependency size 65.95 MB 65.92 MB 🎉 -27 KB 🎉
Bundle Size Analyzer Link Link

create-storybook

Before After Difference
Dependency count 50 50 0
Self size 999 KB 999 KB 🎉 -42 B 🎉
Dependency size 36.82 MB 36.79 MB 🎉 -27 KB 🎉
Bundle Size Analyzer node node

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
code/core/src/core-server/utils/ghost-stories/get-candidates.ts (1)

188-194: Loss of specific error details in catch block.

The bare catch discards the original error information. While this aligns with the PR's uniform runError reporting strategy, consider whether logging the actual error (without exposing it in telemetry) would aid debugging.

💡 Optional: Log the error for debugging
-  } catch {
+  } catch (err) {
+    logger.debug('Failed to find candidates:', err);
     return {
       candidates: [],
       error: 'Failed to find candidates',
       globMatchCount,
     };
   }
code/core/src/core-server/server-channel/ghost-stories-channel.ts (1)

109-113: Generic error message in catch block.

Same pattern as get-candidates.ts - the specific error is discarded. Consider logging the error for debugging while keeping the telemetry message generic.

💡 Optional: Log the error for debugging
-    } catch {
+    } catch (err) {
+      // Log for debugging but don't expose details in telemetry
+      console.error('Ghost stories run failed:', err);
       telemetry('ghost-stories', {
         stats,
         runError: 'Unknown error during ghost run',
       });
     } finally {

Note: Per coding guidelines, consider using logger from storybook/internal/node-logger instead of console.error for server-side logging.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1a755c1 and 090864b.

📒 Files selected for processing (5)
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.ts
  • code/core/src/core-server/utils/ghost-stories/get-candidates.ts
  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts
  • code/core/src/core-server/utils/ghost-stories/run-story-tests.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • code/core/src/core-server/utils/ghost-stories/run-story-tests.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,jsx,ts,tsx,json,md,html,css,scss}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Format code using Prettier with yarn prettier --write <file>

Files:

  • code/core/src/core-server/utils/ghost-stories/get-candidates.ts
  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.ts
**/*.{js,jsx,json,html,ts,tsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Run ESLint checks using yarn lint:js:cmd <file> or the full command cross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directives to fix linting errors before committing

Files:

  • code/core/src/core-server/utils/ghost-stories/get-candidates.ts
  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Enable TypeScript strict mode across all packages

Files:

  • code/core/src/core-server/utils/ghost-stories/get-candidates.ts
  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx,js,jsx}: Export functions from modules if they need to be tested
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size

Files:

  • code/core/src/core-server/utils/ghost-stories/get-candidates.ts
  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.ts
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use logger from storybook/internal/node-logger for server-side logging in Node.js code

Files:

  • code/core/src/core-server/utils/ghost-stories/get-candidates.ts
  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.ts
**/*.{test,spec}.{ts,tsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{test,spec}.{ts,tsx}: Test files should follow the naming pattern *.test.ts, *.test.tsx, *.spec.ts, or *.spec.tsx
Follow the spy mocking rules defined in .cursor/rules/spy-mocking.mdc for consistent mocking patterns with Vitest

Files:

  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
**/*.test.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)

**/*.test.{ts,tsx,js,jsx}: Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Use vi.mocked() to type and access the mocked functions in Vitest tests
Implement mock behaviors in beforeEach blocks in Vitest tests
Mock all required dependencies that the test subject uses
Each mock implementation should return a Promise for async functions in Vitest
Mock implementations should match the expected return type of the original function
Mock all required properties and methods that the test subject uses in Vitest tests
Avoid direct function mocking without vi.mocked() in Vitest tests
Avoid mock implementations outside of beforeEach blocks in Vitest tests
Avoid mocking without the spy: true option in Vitest tests
Avoid inline mock implementations within test cases in Vitest tests
Avoid mocking only a subset of required dependencies in Vitest tests
Mock at the highest level of abstraction needed in Vitest tests
Keep mock implementations simple and focused in Vitest tests
Use type-safe mocking with vi.mocked() in Vitest tests
Document complex mock behaviors in Vitest tests
Group related mocks together in Vitest tests

Files:

  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{test,spec}.{ts,tsx,js,jsx}: Write meaningful unit tests that actually import and call the functions being tested, not just verify syntax patterns
Achieve high test coverage of business logic, aiming for 75%+ coverage of statements/lines
Cover all branches, conditions, edge cases, error paths, and different input variations in unit tests
Use vi.mock() to mock file system, loggers, and other external dependencies in tests

Files:

  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
🧠 Learnings (14)
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies in Vitest tests

Applied to files:

  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests

Applied to files:

  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses in Vitest tests

Applied to files:

  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases in Vitest tests

Applied to files:

  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Each mock implementation should return a Promise for async functions in Vitest

Applied to files:

  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together in Vitest tests

Applied to files:

  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Keep mock implementations simple and focused in Vitest tests

Applied to files:

  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in `beforeEach` blocks in Vitest tests

Applied to files:

  • code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
📚 Learning: 2025-12-22T22:03:40.123Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Applies to **/*.{test,spec}.{ts,tsx,js,jsx} : Use `vi.mock()` to mock file system, loggers, and other external dependencies in tests

Applied to files:

  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock implementations should match the expected return type of the original function

Applied to files:

  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required dependencies that the test subject uses

Applied to files:

  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use `vi.mock()` with the `spy: true` option for all package and file mocks in Vitest tests

Applied to files:

  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock at the highest level of abstraction needed in Vitest tests

Applied to files:

  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mock implementations outside of `beforeEach` blocks in Vitest tests

Applied to files:

  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
🧬 Code graph analysis (1)
code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts (2)
code/core/src/shared/utils/categorize-render-errors.ts (1)
  • ErrorCategory (26-26)
code/core/src/core-server/utils/ghost-stories/types.ts (2)
  • TestRunSummary (21-34)
  • StoryTestResult (1-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Core Unit Tests, windows-latest
  • GitHub Check: nx
  • GitHub Check: normal
🔇 Additional comments (12)
code/core/src/core-server/server-channel/ghost-stories-channel.ts (3)

39-50: Guard logic looks correct.

The early return when lastEvents or lastInit is missing is appropriate. The deferred sessionId retrieval after the guard avoids an unnecessary async call.

One subtlety: if lastInit.body is undefined, lastInit.body?.sessionId evaluates to undefined, and the condition undefined !== sessionId will be true (assuming sessionId is a string), causing an early return. Confirm this is the intended behavior for missing body data.


74-90: Telemetry payload updated to use runError.

The change from error to runError is consistent with the updated TestRunSummary type and aligns with the PR's uniform error reporting strategy.


97-108: Clean error vs. success path separation.

The early return on testRunResult.runError cleanly separates failure handling from the success path. The success telemetry correctly includes results: testRunResult.summary.

code/core/src/core-server/server-channel/ghost-stories-channel.test.ts (5)

431-441: Test expectation correctly updated.

The assertion now expects runError instead of error, matching the implementation change.


471-481: Test expectation correctly updated for no candidates scenario.


515-527: Test expectation correctly updated for JSON report not found scenario.


568-580: Test expectation correctly updated for startup error scenario.


594-598: Test expectation correctly updated for general error scenario.

code/core/src/core-server/utils/ghost-stories/parse-vitest-report.ts (4)

16-29: Per-category unique error tracking added.

The uniqueErrors: Set<string> addition enables tracking unique error messages within each category, which provides more granular insights than just the count.


38-47: Unique error count per category correctly implemented.

The uniqueCount is derived from data.uniqueErrors.size, providing an accurate count of distinct error messages within each category.


61-66: Cleaner parameter naming.

Renaming to report clarifies that this is the full Vitest JSON report object, and report.testResults is more intuitive than testResults.testResults.


100-120: Return shape matches TestRunSummary type.

The returned structure { summary: { ... } } correctly aligns with the TestRunSummary interface from types.ts. The caller (runStoryTests) can add duration and runError fields as needed.

@yannbf yannbf merged commit 2840142 into next Jan 14, 2026
71 checks passed
@yannbf yannbf deleted the yann/adjust-ghost-stories-run branch January 14, 2026 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants