Conversation
…ver xhprof) Mirrors the a11y runner: two-layer normalized report (lab Core Web Vitals + Lighthouse, plus optional server-side xhprof hotspots over WP-CLI). Ships src/perf/*, the perf CLI command, package.json exports, tests, the setup/perf scaffold with a hardened server-profile.php shim, and CHANGELOG/issue-tracking entries. Includes two pre-existing lint-gate fixes (no-shadow + prettier in src/init/index.js and tests/ui/selects.test.js) needed for npm run check to pass, matching the fix already on the a11y branch.
… retention - server-profile: catch malformed URL instead of throwing - run.js: skip Lighthouse when scanError set; extractLighthouse immediately after run; show server.error in text output; guard dry-run against string command - normalize.js: derive METRIC_NAMES from THRESHOLDS - collect-vitals.js: import METRIC_NAMES from normalize - scaffold: split server_enabled from server_env_cwd - test: de-flake dry-run assertion
No safe default for server_env_cwd — wrong guess silently misroutes the server layer. Malformed config now exits 2 (usage error, not run failure).
8252bf2 to
9255880
Compare
There was a problem hiding this comment.
Pull request overview
Adds a two-layer performance CLI combining browser metrics, Lighthouse, and optional server-side XHProf profiling.
Changes:
- Implements performance collection, normalization, reporting, and CLI handling.
- Adds the
setup/perfconsumer scaffold and PHP profiling shim. - Adds comprehensive fixtures and Jest coverage.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
node-packages/wp-tooling/tests/scaffolds/bundled-manifests.test.js |
Tests scaffold rendering. |
node-packages/wp-tooling/tests/perf/server-profile.test.js |
Tests WP-CLI profiling. |
node-packages/wp-tooling/tests/perf/resolve-module.test.js |
Tests consumer module resolution. |
node-packages/wp-tooling/tests/perf/normalize.test.js |
Tests report normalization. |
node-packages/wp-tooling/tests/perf/lighthouse.test.js |
Tests Lighthouse invocation. |
node-packages/wp-tooling/tests/perf/fixtures/xhprof.json |
Provides XHProf fixture data. |
node-packages/wp-tooling/tests/perf/fixtures/web-vitals.attribution.iife.js |
Provides web-vitals fixture script. |
node-packages/wp-tooling/tests/perf/fixtures/partial.perfrc.json |
Provides partial configuration fixture. |
node-packages/wp-tooling/tests/perf/fixtures/malformed.perfrc.json |
Provides malformed configuration fixture. |
node-packages/wp-tooling/tests/perf/fixtures/lighthouse-lhr.json |
Provides Lighthouse fixture data. |
node-packages/wp-tooling/tests/perf/fixtures/.perfrc.no-urls.json |
Covers empty URL configuration. |
node-packages/wp-tooling/tests/perf/fixtures/.perfrc.json |
Provides complete configuration fixture. |
node-packages/wp-tooling/tests/perf/config.test.js |
Tests configuration resolution. |
node-packages/wp-tooling/tests/perf/collect-vitals.test.js |
Tests browser metric collection. |
node-packages/wp-tooling/tests/perf/cli.test.js |
Tests CLI behavior and exit codes. |
node-packages/wp-tooling/src/perf/server-profile.js |
Invokes server profiling through WP-CLI. |
node-packages/wp-tooling/src/perf/run.js |
Orchestrates layers and CLI output. |
node-packages/wp-tooling/src/perf/resolve-module.js |
Resolves consumer-installed modules. |
node-packages/wp-tooling/src/perf/resolve-bin.js |
Resolves consumer binaries. |
node-packages/wp-tooling/src/perf/normalize.js |
Builds normalized performance reports. |
node-packages/wp-tooling/src/perf/lighthouse.js |
Runs Lighthouse scans. |
node-packages/wp-tooling/src/perf/index.js |
Exposes the performance API. |
node-packages/wp-tooling/src/perf/errors.js |
Defines structured runner errors. |
node-packages/wp-tooling/src/perf/config.js |
Loads and merges performance configuration. |
node-packages/wp-tooling/src/perf/collect-vitals.js |
Collects lab web-vitals metrics. |
node-packages/wp-tooling/src/cli/commands/perf.js |
Registers the perf command. |
node-packages/wp-tooling/scaffolds/setup/perf/templates/server-profile.php |
Implements the PHP profiling shim. |
node-packages/wp-tooling/scaffolds/setup/perf/templates/.perfrc.json.mustache |
Generates consumer configuration. |
node-packages/wp-tooling/scaffolds/setup/perf/scaffold.json |
Defines scaffold inputs and actions. |
node-packages/wp-tooling/package.json |
Exports the performance API. |
node-packages/wp-tooling/CHANGELOG.md |
Documents the new functionality. |
.claude/issues/wp-devtools-22-36-37-perf-runner.md |
Records implementation decisions. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…t rendering, report gaps
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 35 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
Previously missed (2) — in code that hasn't changed since the last review.
node-packages/wp-tooling/src/perf/config.js:39
- Using
npx wp-envwithout--no-installallows npm to download@wordpress/envwhen it is absent locally, violating the runner's no-fetch behavior and potentially prompting or modifying the npm cache in CI. Make the fallback explicitly local-only.
command: ['npx', 'wp-env', 'run', 'cli', '--env-cwd=.', '--', 'wp'],
node-packages/wp-tooling/scaffolds/setup/perf/templates/.perfrc.json.mustache:10
- The generated config likewise invokes
npx wp-envwithout--no-install, so running the optional server layer can fetch@wordpress/envinstead of degrading when it is not installed. Include the no-install flag here and update the scaffold expectations accordingly.
"command": ["npx", "wp-env", "run", "cli", "--env-cwd={{server_env_cwd}}", "--", "wp"]
node-packages/wp-tooling/src/perf/run.js:311
- The default text report drops both
server.noteandserver.diagnostic. As a result, users do not see the required CLI-context fidelity warning or the captured route diagnostic unless they choose JSON output; emit both fields when present.
lines.push(` server top: ${top || 'none'}`);
…, server resilience
- read attribution.target (not element) for web-vitals v5; use ?? for settleMs/timeoutMs
- skip malformed xhprof entries instead of aborting the entire perf run
- unwind output buffers to pre-render level in server-profile.php
- tryParse iterates [/{ candidates past preambles; non-zero exit checked before parsing
- dry-run uses resolveBin so lighthouse --version is never spawned
- pre-render script templates before file writes; validate checks scripts alongside files
- emit server fidelity note in default text output
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 38 changed files in this pull request and generated 3 comments.
Suppressed comments (3)
Previously missed (2) — in code that hasn't changed since the last review.
node-packages/wp-tooling/src/perf/normalize.js:101
- A configured
topAudits: 0is replaced with 5 because0is falsy, so consumers cannot request zero audit details while retaining Lighthouse scores. Use nullish defaulting so the documented maximum of zero is preserved.
const topAudits = options.topAudits || 5;
node-packages/wp-tooling/src/perf/run.js:286
failedUrlsalso includes avitalsErrorfor pages that loaded successfully but returned no metrics, so this summary can incorrectly say those URLs “failed to load.” Use wording that covers both navigation and metric-collection failures.
This issue also appears on line 495 of the same file.
const failed =
summary.failedUrls > 0 ? `, ${summary.failedUrls} failed to load` : '';
node-packages/wp-tooling/src/perf/run.js:498
- This diagnostic is also emitted for
vitalsError, where the page did load (and Lighthouse may have run). Saying every failed URL “failed to load” obscures the actual metric-harvest failure reported in the per-URL notes.
if (report.summary.failedUrls > 0) {
process.stderr.write(
`perf: ${report.summary.failedUrls} URL(s) failed to load — treating as a run failure.\n`
);
| const candidate = path.join(dir, 'node_modules', '.bin', binName); | ||
| if (fs.existsSync(candidate)) { |
| const command = server.command[0]; | ||
| let result; | ||
| try { | ||
| // spawnSync itself can throw synchronously (e.g. command undefined), | ||
| // separately from the return-based result.error handled below. | ||
| result = spawnSync(command, args, { |
There was a problem hiding this comment.
🟡 Changes recommended
Server execution can fetch packages, and browser-layer error handling can incorrectly suppress Lighthouse.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (8)
Previously missed (6) — in code that hasn't changed since the last review.
node-packages/wp-tooling/src/perf/config.js:39
- The default server command invokes bare
npx, so enablingserverwithout a locally installedwp-envcan download and execute it from the registry. That contradicts this PR's never-fetch/no-package-manager-execution contract. Use the same no-install resolution policy as Lighthouse, and keep the scaffold template in sync.
node-packages/wp-tooling/src/perf/normalize.js:101 - An explicit
topAudits: 0is replaced by the default because0is falsy, so consumers cannot request zero audit details. Use nullish defaulting so the documented maximum is respected at this boundary.
node-packages/wp-tooling/src/perf/run.js:183 scanErrorcovers every exception fromcollectVitals, including injection, evaluation, and page-close failures after navigation succeeded. This guard then skips Lighthouse as if the URL were unreachable, so a collector-specific failure disables the supposedly independent Lighthouse layer. Distinguish navigation failures from collection failures, or run Lighthouse after non-navigation errors.
node-packages/wp-tooling/package.json:30- Adding this public export leaves the package guide stale:
node-packages/wp-tooling/AGENTS.md:61-72still says the exports map has eight entries and omits./perf; its CLI directory listing also omits the new command. Update that authoritative package documentation so future changes do not treat this export as unsupported.
node-packages/wp-tooling/src/perf/resolve-module.js:96 - This variable-path
requireviolates the package rule forbidding dynamic require paths (node-packages/wp-tooling/AGENTS.md:104). Replace the generic loader with a constrained consumer-module mechanism, such as a dedicated Puppeteer loader usingmodule.createRequirewith a statically named package.
node-packages/wp-tooling/src/perf/run.js:286 failedUrlsalso includesvitalsErrorresults where navigation succeeded but no metrics were harvested, so reporting all of them as “failed to load” is inaccurate. Use wording such as “failed to collect metrics” that covers both load and harvest failures.
This issue also appears on line 495 of the same file.
node-packages/wp-tooling/src/perf/resolve-bin.js:32
- On Windows, npm's extensionless
.bin/<name>file is a POSIX shim;execFileSynccannot execute it. Because this path is preferred whenever it exists, locally installed Lighthouse is detected but its version probe fails. Resolve the package's JavaScript bin entry and invoke it withprocess.execPath, rather than selecting an extensionless or shell-based shim.
const candidate = path.join(dir, 'node_modules', '.bin', binName);
if (fs.existsSync(candidate)) {
node-packages/wp-tooling/src/perf/run.js:498
- This message says every failed URL failed to load, but
failedUrlsis also incremented for an empty web-vitals harvest after a successful load. Report a generic collection failure, or track load and harvest failures separately.
if (report.summary.failedUrls > 0) {
process.stderr.write(
`perf: ${report.summary.failedUrls} URL(s) failed to load — treating as a run failure.\n`
);
- Files reviewed: 38/38 changed files
- Comments generated: 1
- Review effort level: Balanced
| ], | ||
| "server": { | ||
| "enabled": {{#server_enabled}}true{{/server_enabled}}{{^server_enabled}}false{{/server_enabled}}, | ||
| "command": ["npx", "wp-env", "run", "cli", "--env-cwd={{server_env_cwd}}", "--", "wp"] |
What this PR does
wp-tooling perf— a zero-runtime-dep CLI runner that collects Core Web Vitals (web-vitals + Lighthouse) and optional server-side xhprof profiling per URL, with sensible degradation for every failure modesetup/perfscaffold so consumers can bootstrap config, npm scripts, and the PHP xhprof shim in one commandCloses
Closes https://github.com/rtcamp/wp-devtools/issues/22
Closes https://github.com/rtcamp/wp-devtools/issues/36
Closes https://github.com/rtcamp/wp-devtools/issues/37
Changes
--urloverride and optional config files. Binary/module resolution walks consumer node_modules withnpx --no-installfallback, never fetching from registry. Normalisation pipeline rates metrics against CWV thresholds, extracts Lighthouse scores + failing audits, builds human-readable assessment lines from a single THRESHOLDS registry. CLI layer with dry-run plan printer, text/json emitter, exit codes 0/1/2/3 matching the a11y convention, and a RunnerError hierarchy with machine-readable codes..perfrc.jsontemplate emitting only URLs and server settings — all other sections fall through to built-in defaults, preventing silent drift. Server-enabled boolean separated from env-cwd path so profiling the WordPress root (--env-cwd=.) is expressible. Hardened PHP shim that detects xhprof/tideways backends, degrades gracefully to[]when the profiler is absent, unhooksredirect_canonicalto prevent earlyexit(), installs a shutdown-fallback emitter with buffer drain, populates$_GETandREQUEST_URIfrom the positional path argument, and writes a route diagnostic to stderr.{ data: null, error }instead of throwing. URL splitter separating origin from path+query for the shim's two-argument contract. Normaliser maps raw xhprof function data into the report's server section with ct/wt/cpu/mu/pmu fields and a fidelity note explaining CLI-context limitations. Server layer wired into the per-URL collector independently of the browser layer — still profiles even when the frontend scan failed.How I verified
Fixture testing for all exit codes and degradation paths. Scaffold render tests confirm default and custom config output. Also tested end-to-end by running wp-tooling perf against the features skeleton plugin.
Acceptance criteria
Runtime behavior
wp-tooling perf --dry-runresolves and prints the plan without executing anything.wp-tooling add setup/perfyields a runnabletest:perfon a consumer, with the dev deps surfaced as developer actions (not auto-installed).wp eval-file server-profile.php --url=... --top=15 --format=jsonreturns top-N JSON when the extension is present, and[]when it is not.serverlayer per URL.fidelitynote is present in the report.Code quality
Housekeeping
CHANGELOG.mdentry under## Unreleased.claude/issues/<N>-<slug>.mdupdated with final stateFull decision log and verification history for this work lives in
.claude/issues/wp-devtools-22-36-37-perf-runner.mdon this branch.