Bench: Declare the React renderer dependency the harnesses rely on

[valentinpalkovic]

Last of the docgen bench stack. Closes the one real coupling problem the suite had left.

The problem

The React harnesses measure the renderer's own extraction code, so they import it from source rather than through the published surface. That reach was a hardcoded relative path with no dependency behind it:

const COMPONENT_MANIFEST = '../../../code/renderers/react/src/componentManifest/';

scripts/package.json declared exactly one workspace:* dependency and it was eslint-plugin-storybook, so nothing in the workspace graph recorded that scripts needs the React renderer at all.

It works today by accident. The renderer's source imports storybook/internal/common, and storybook is a peerDependency of @storybook/react - so the bench only runs because a full install happens to put the renderer's own dependencies somewhere its source can reach from a process launched out of scripts/. On a partial install it fails like this:

Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'storybook' imported from
  code/renderers/react/src/componentManifest/utils.ts

Nothing in that message mentions the bench, and nothing tells you what to do.

The change

Declare @storybook/react": "workspace:*" in scripts, and anchor on the resolved package instead of a relative path. The path now moves with the package, and the coupling is visible to Nx and to anyone reading the manifest.

Both failure modes now say what to do instead of naming a file the harness never mentioned:

• No renderer source (a published copy rather than the workspace checkout) names the directory it resolved to and why source is required.
• Missing code/core build becomes Run \yarn nx compile core` and try again, with Node's original error preserved as the cause. This is the one that actually bites - the renderer imports storybook/internal/*, which resolves to code/core's dist/`, so a fresh checkout hits it immediately.

Verification

• React perf engine run end to end before and after: cold pass: 35ms → 29ms, same output shape.
• Both error paths exercised directly rather than reasoned about - the missing-build message above is real output from hiding code/core/dist.
• yarn dedupe --check clean. The lockfile gains exactly one line, the workspace descriptor; no new resolutions, nothing re-resolved.
• 116 bench tests pass (5 new), 0 type errors.

What this does not do

It does not unblock a react-docgen version pair. Declaring the dependency makes the source reachable, but the renderer still imports react-docgen by bare specifier, so pointing it at a second install would need a module resolution hook registered in the child. Noted in PERF-METHODOLOGY.md so the next person does not rediscover it.

Considered and rejected: moving the whole suite into @storybook/docgen-harness. The workspace shares one lockfile, so there are no duplicate installs to reclaim - vue-docgen-api is declared by three workspaces and resolves to one copy. Compodoc would get worse, since docgen-harness deliberately commits captured compodoc-input.json fixtures instead of depending on it. And the two harnesses pull in opposite directions: docgen-harness wants real framework types for correctness, the bench ships a fake minimal @angular/core so cold time does not move when Angular is upgraded.

Manual testing

yarn install
cd scripts && yarn vitest run --config vitest.config.ts bench/docgen   # 116 tests, no compile needed
cd .. && yarn nx compile core                                          # the React engines need core's dist
node scripts/bench/docgen-perf/engines/react-legacy.ts --components 2 --saves 1 --json /tmp/react.json

Expect a cold pass and one save timing, identical in shape to before the change.

To see the coupling actually being resolved rather than guessed, check that the anchor is the package rather than a relative path:

cd scripts && node -e "console.log(require.resolve('@storybook/react/package.json'))"

To see the error redirect that motivates the change, hide the core build and re-run the engine:

mv code/core/dist code/core/dist.bak
node scripts/bench/docgen-perf/engines/react-legacy.ts --components 2 --saves 1 --json /tmp/x.json
mv code/core/dist.bak code/core/dist

Expect Run `yarn nx compile core` and try again with Node's original resolution error kept as the cause, rather than a bare ERR_MODULE_NOT_FOUND naming a storybook/dist path the harness never mentioned.

[github-actions]

	Warnings
⚠️	This PR targets valentin/bench-6-version-pairs. The default branch for contributions is next. Please make sure you are targeting the correct branch.

Generated by 🚫 dangerJS against babd0f6

[coderabbitai]

Walkthrough

The benchmark tooling now resolves React renderer modules from the installed @storybook/react source. It validates the source directory, wraps import failures, provides guidance for missing core builds, and adds tests and methodology documentation.

Changes

React renderer resolution

Layer / File(s)	Summary
Resolve the renderer source scripts/bench/docgen-shared/react-renderer-module.ts, scripts/package.json	The loader resolves and caches @storybook/react/src/componentManifest, validates the source directory, and uses the required workspace packages.
Load modules and report errors scripts/bench/docgen-shared/react-renderer-module.ts, scripts/bench/docgen-shared/react-renderer-module.test.ts, scripts/bench/PERF-METHODOLOGY.md	loadReactRendererModule imports modules from the resolved directory. rendererModuleError adds guidance for missing storybook/dist output and preserves unrelated and non-Error throws. Tests cover these cases and successful loading. The methodology documents the module-resolution limitation.

Sequence Diagram(s)

sequenceDiagram
  participant BenchmarkLoader
  participant StorybookReactSource
  participant ErrorNormalizer
  BenchmarkLoader->>StorybookReactSource: Resolve componentManifest directory
  StorybookReactSource-->>BenchmarkLoader: Return renderer source path
  BenchmarkLoader->>StorybookReactSource: Import renderer module
  StorybookReactSource-->>BenchmarkLoader: Return module or import error
  BenchmarkLoader->>ErrorNormalizer: Normalize import error
  ErrorNormalizer-->>BenchmarkLoader: Return contextual error

Loading

Possibly related PRs

• storybookjs/storybook#35689: Modifies the same renderer-module loader and tests.
• storybookjs/storybook#35324: Uses React renderer module resolution for worker-based docgen extraction.
• storybookjs/storybook#35651: Modifies the docgen benchmark infrastructure and React renderer-module loading.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Caution

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

⚠️ Outside diff range comments (1)

scripts/bench/PERF-METHODOLOGY.md (1)

29-31: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Correct the latency-metric definition.

Line 31 says that every latency metric records a median for cold extractions. The same sentence then defines warm extraction separately. This conflicts with the warm metric in Line 14 and can cause readers to apply the cold sampling rule to warm measurements.

Suggested wording

- Every latency metric records a median for cold extractions. We need to make sure that these are triggered in fresh processes, so there are n samples that come from n separate spawns, while warm extracts takes the median of the per-save durations inside one run.
+ Every latency metric records a median. Cold extraction uses N samples from N separate spawns. Warm extraction uses the median of per-save durations inside one run.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/bench/PERF-METHODOLOGY.md` around lines 29 - 31, Clarify the
“Median-of-N for latency” definition in PERF-METHODOLOGY.md so the median across
fresh process spawns applies only to cold extraction metrics, while warm
extraction metrics use the median of per-save durations within a single run.
Remove or revise the wording that says every latency metric records a
cold-extraction median, preserving the definitions established for warm
measurements.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@scripts/bench/PERF-METHODOLOGY.md`:
- Around line 29-31: Clarify the “Median-of-N for latency” definition in
PERF-METHODOLOGY.md so the median across fresh process spawns applies only to
cold extraction metrics, while warm extraction metrics use the median of
per-save durations within a single run. Remove or revise the wording that says
every latency metric records a cold-extraction median, preserving the
definitions established for warm measurements.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5b676abb-a866-48af-8e0d-6f17ab8cca7e

📥 Commits

Reviewing files that changed from the base of the PR and between 0717d83 and babd0f6.

⛔ Files ignored due to path filters (1)

• yarn.lock is excluded by !**/yarn.lock, !**/*.lock

📒 Files selected for processing (4)

• scripts/bench/PERF-METHODOLOGY.md
• scripts/bench/docgen-shared/react-renderer-module.test.ts
• scripts/bench/docgen-shared/react-renderer-module.ts
• scripts/package.json

🚧 Files skipped from review as they are similar to previous changes (3)

• scripts/bench/docgen-shared/react-renderer-module.test.ts
• scripts/package.json
• scripts/bench/docgen-shared/react-renderer-module.ts