ci: omit unchanged bundle size scenarios#7856
Conversation
|
View your CI Pipeline Execution ↗ for commit bceaf8c
☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughThe bundle-size PR report now omits unchanged metrics, handles missing baselines and empty measurements, and adds Node unit tests with Nx task wiring. ChangesBundle-size reporting
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
🚀 Changeset Version Preview1 package(s) bumped directly, 6 bumped as dependents. 🟩 Patch bumps
|
Bundle Size BenchmarksThis pull request does not affect bundle size in any measured scenario. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scripts/benchmarks/bundle-size/pr-report.test.mjs (1)
72-131: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGood coverage for filtering scenarios; missing test for the empty-metrics branch.
Tests correctly validate the unchanged/changed/no-baseline scenarios. However, the
metrics.length === 0branch ('No bundle-size scenarios were measured.'in pr-report.mjs) isn't exercised by any test, despite being part of the reworked conditional in this PR.✅ Suggested additional test
test('renders a placeholder message when no scenarios were measured', async () => { const current = currentJson([]) const report = await generateReport({ current }) assert.equal( report, '<!-- bundle-size-benchmark -->\n## Bundle Size Benchmarks\n\nNo bundle-size scenarios were measured.\n', ) })🤖 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/benchmarks/bundle-size/pr-report.test.mjs` around lines 72 - 131, Add a test in pr-report.test.mjs covering generateReport with currentJson([]) and no baseline, asserting the exact placeholder output “No bundle-size scenarios were measured.” and the existing report header.
🤖 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.
Inline comments:
In `@scripts/benchmarks/bundle-size/pr-report.mjs`:
- Around line 271-280: Update the skip condition in the metrics loop to compare
every displayed metric value—raw, gzip, brotli, and initial—against the
corresponding baseline data, so a row is skipped only when all measured values
are unchanged. Preserve the existing row-generation behavior for any metric with
a change.
---
Nitpick comments:
In `@scripts/benchmarks/bundle-size/pr-report.test.mjs`:
- Around line 72-131: Add a test in pr-report.test.mjs covering generateReport
with currentJson([]) and no baseline, asserting the exact placeholder output “No
bundle-size scenarios were measured.” and the existing report header.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: eb168601-89ae-4450-9cbf-367f523db217
📒 Files selected for processing (3)
benchmarks/bundle-size/package.jsonscripts/benchmarks/bundle-size/pr-report.mjsscripts/benchmarks/bundle-size/pr-report.test.mjs
| const metrics = current.metrics || [] | ||
| const rows = [] | ||
|
|
||
| for (const metric of current.metrics || []) { | ||
| for (const metric of metrics) { | ||
| const baselineValue = baseline.benchesByName.get(metric.id) | ||
|
|
||
| if (Number.isFinite(baselineValue) && metric.gzipBytes === baselineValue) { | ||
| continue | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file structure first, then inspect the relevant slice.
ast-grep outline scripts/benchmarks/bundle-size/pr-report.mjs --view expanded || true
echo '--- relevant lines ---'
sed -n '240,340p' scripts/benchmarks/bundle-size/pr-report.mjsRepository: TanStack/router
Length of output: 4428
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect how baseline and rows are constructed, and where the "does not affect bundle size" message is emitted.
rg -n "benchesByName|get\\(|rawBytes|brotliBytes|initialGzipBytes|does not affect bundle size|metric\\.gzipBytes|baselineValue" scripts/benchmarks/bundle-size/pr-report.mjsRepository: TanStack/router
Length of output: 1198
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,240p' scripts/benchmarks/bundle-size/pr-report.mjsRepository: TanStack/router
Length of output: 6049
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('scripts/benchmarks/bundle-size/pr-report.mjs')
text = p.read_text()
for needle in [
'metric.gzipBytes === baselineValue',
'does not affect bundle size',
'rawBytes',
'brotliBytes',
'initialGzipBytes',
]:
print(f'-- {needle} --')
for i, line in enumerate(text.splitlines(), 1):
if needle in line:
print(f'{i}: {line}')
PYRepository: TanStack/router
Length of output: 572
Rows can disappear when only non-gzip metrics change scripts/benchmarks/bundle-size/pr-report.mjs:271-280
The skip check only compares metric.gzipBytes to the baseline. Since the table also shows raw, brotli, and initial, a PR can report “does not affect bundle size in any measured scenario” even when those displayed values changed. Consider including them in the change check, or remove them if gzip is the only signal.
🤖 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/benchmarks/bundle-size/pr-report.mjs` around lines 271 - 280, Update
the skip condition in the metrics loop to compare every displayed metric
value—raw, gzip, brotli, and initial—against the corresponding baseline data, so
a row is skipped only when all measured values are unchanged. Preserve the
existing row-generation behavior for any metric with a change.
Merging this PR will regress 1 benchmark
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | mem server server-fn-churn (react) |
270.9 KB | 281.8 KB | -3.85% |
| ⚡ | Memory | mem server peak-large-page (react) |
1,841 KB | 955.1 KB | +92.77% |
| ⚡ | Memory | mem server error-paths not-found (vue) |
626.3 KB | 380 KB | +64.83% |
| ⚡ | Memory | mem server serialization-payload (react) |
5.3 MB | 3.4 MB | +53.9% |
| ⚡ | Memory | mem server streaming-peak chunked (vue) |
13.6 MB | 11.1 MB | +22.5% |
| ⚡ | Simulation | client-route-tree-scale navigation loop (react) |
77.1 ms | 74.1 ms | +4.05% |
| ⚡ | Memory | mem server error-paths unmatched (react) |
274.9 KB | 264.4 KB | +3.96% |
| ⚡ | Memory | mem server error-paths not-found (react) |
277.2 KB | 266.8 KB | +3.9% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing ci-bundlesize (bceaf8c) with main (49f6863)
2 participants
Summary by CodeRabbit
New Features
Tests