fix: normalize pnpm 12 package benchmark data - #168
Merged
Conversation
darcyclarke
approved these changes
Aug 28, 2026
darcyclarke
disabled auto-merge
August 28, 2026 15:51
There was a problem hiding this comment.
Pull request overview
This PR updates the benchmark ingestion pipeline to correctly attribute package counts for pnpm 12 (JSON-formatted .modules.yaml), and to make per-package chart data consistent by skipping entries with missing counts and normalizing stddev into the same units as the per-package mean.
Changes:
- Extend package manager inference to recognize pnpm 12’s JSON
.modules.yamlformat. - Normalize timing (mean + stddev) to ms/package when counts are available; otherwise exclude missing-count results from per-package charts.
- Add Node test coverage for pnpm 11 YAML vs pnpm 12 JSON metadata parsing and timing normalization behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| scripts/package-count.sh | Broaden pnpm metadata parsing to detect pnpm 12 JSON and classify pacquet correctly. |
| scripts/generate-chart.js | Centralize timing normalization, skip per-package entries with missing counts, export helper for tests, and guard CLI execution. |
| scripts/benchmark-data.test.js | Add regression tests for timing normalization and pnpm metadata parsing via package-count script. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+25
to
+28
| # pnpm 11 writes YAML, while pnpm 12 currently writes JSON despite the | ||
| # .yaml extension. Accept both `packageManager: pnpm@11...` and | ||
| # `"packageManager": "pnpm@12..."`. | ||
| pnpm_major=$(sed -En 's/^[[:space:]]*"?packageManager"?[[:space:]]*:[[:space:]]*"?pnpm@([0-9]+).*/\1/p' node_modules/.modules.yaml 2>/dev/null | head -1) |
Comment on lines
+57
to
+76
| const normalizeTiming = (result, count, perPackageCount) => { | ||
| if (!result || typeof result.mean !== "number") { | ||
| return undefined; | ||
| } | ||
|
|
||
| if (!perPackageCount) { | ||
| return { value: result.mean, stddev: result.stddev }; | ||
| } | ||
|
|
||
| if (typeof count !== "number" || count <= 0) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const scale = 1000 / count; | ||
| return { | ||
| value: result.mean * scale, | ||
| stddev: | ||
| typeof result.stddev === "number" ? result.stddev * scale : undefined, | ||
| }; | ||
| }; |
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
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.
Summary
.modules.yamlwhen attributing package countsProblem
pnpm 12 currently writes metadata like:
{ "packageManager": "pnpm@12.0.0" }The benchmark expected the pnpm 11-style YAML form:
Consequently, pnpm 12 received no package count. The chart generator then left its total duration in seconds while comparing it against other results normalized to milliseconds per package.
For the published Svelte clean result, this made pnpm 12’s
1.034total seconds appear comparable to Bun’s10.79milliseconds per package. With the correct 59-package count, pnpm 12 is approximately17.53 ms/package.Validation
node --test scripts/benchmark-data.test.jsshellcheck -s bash scripts/package-count.shnpm run --prefix app lintnpm run --prefix app buildAI-assisted — Tool: Codex; model: OpenAI/GPT-5; version: unavailable.