-
Notifications
You must be signed in to change notification settings - Fork 30.2k
perf: improve stats action reliability and reduce CI noise #87945
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
3517bf6 to
4f78126
Compare
5a4a839 to
118aab8
Compare
118aab8 to
de079d4
Compare
4f78126 to
63e2c4f
Compare
de079d4 to
b8f46bf
Compare
63e2c4f to
0aaff46
Compare
b8f46bf to
207d6ce
Compare
Stats from current PR✅ No significant changes detected📊 All Metrics📖 Metrics GlossaryDev Server Metrics:
Build Metrics:
Change Thresholds:
⚡ Dev Server
📦 Dev Server (Webpack) (Legacy)📦 Dev Server (Webpack)
⚡ Production Builds
📦 Production Builds (Webpack) (Legacy)📦 Production Builds (Webpack)
📦 Bundle SizesBundle Sizes⚡ TurbopackClient Main Bundles: **430 kB** → **430 kB** ✅ -28 B82 files with content-based hashes (individual files not comparable between builds) Server Middleware
Build DetailsBuild Manifests
📦 WebpackClient Main Bundles
Polyfills
Pages
Server Edge SSR
Middleware
Build DetailsBuild Manifests
Build Cache
🔄 Shared (bundler-independent)Runtimes
|
207d6ce to
eee9da7
Compare
0aaff46 to
38330f4
Compare
44aa30a to
d058637
Compare
38330f4 to
e71c420
Compare
d058637 to
1171a4d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional Suggestion:
The content-type: application/json header is missing when posting to the GitHub API with token authentication. This header should always be included when sending JSON data.
View Details
📝 Patch Details
diff --git a/.github/actions/next-stats-action/src/add-comment.js b/.github/actions/next-stats-action/src/add-comment.js
index c393729e2e..6cfbdfe599 100644
--- a/.github/actions/next-stats-action/src/add-comment.js
+++ b/.github/actions/next-stats-action/src/add-comment.js
@@ -656,13 +656,12 @@ module.exports = async function addComment(
const res = await fetch(endpoint, {
method,
headers: {
+ 'content-type': 'application/json',
...(actionInfo.githubToken
? {
Authorization: `bearer ${actionInfo.githubToken}`,
}
- : {
- 'content-type': 'application/json',
- }),
+ : {}),
},
body: JSON.stringify(body),
})
Analysis
Missing Content-Type header in GitHub API POST/PATCH requests with token authentication
What fails: When posting or patching comments to the GitHub API with token authentication, the Content-Type: application/json header is omitted, causing node-fetch to default to text/plain;charset=UTF-8. This violates GitHub API requirements and can cause request failures.
How to reproduce: The bug occurs in .github/actions/next-stats-action/src/add-comment.js lines 656-665. When actionInfo.githubToken is set (from the PR_STATS_COMMENT_TOKEN workflow variable), the code sends a POST or PATCH request to the GitHub API with a JSON body but without the required Content-Type: application/json header.
Testing with node-fetch@2.6.9 confirms that without an explicit content-type header, JSON data is sent as text/plain;charset=UTF-8:
// Without explicit header:
const res = await fetch(endpoint, {
method: 'POST',
headers: { Authorization: 'bearer token' },
body: JSON.stringify(body),
})
// Actual header sent: content-type: text/plain;charset=UTF-8
Expected behavior: According to GitHub API v3 documentation, "For POST, PATCH, PUT, and DELETE requests, parameters not included in the URL should be encoded as JSON with a Content-Type of 'application/json'". Additionally, node-fetch documentation states: "The Content-Type header is only set automatically to x-www-form-urlencoded when an instance of URLSearchParams is given as such" - meaning it does not automatically add application/json for JSON stringified bodies.
Fix: Always include the content-type header in the fetch request, regardless of whether token authentication is being used.
1171a4d to
3020c41
Compare
0fd2a8f to
c093116
Compare
9080141 to
75312e9
Compare
c093116 to
d312852
Compare
5860a4d to
9a87659
Compare
d312852 to
098df8c
Compare
✅ Tests PassedUpdated 2026-01-05 13:06:31 UTC · Commit: 9a87659 |
✅ Tests PassedUpdated 2026-01-05 13:58:12 UTC · Commit: 4c094a2 |
5ba5283 to
adde1b3
Compare
098df8c to
2b3bcde
Compare
cc63654 to
fdeb939
Compare
9d6987d to
a4f5cf1
Compare
7230576 to
41d7210
Compare
Merge activity
|
- Lower regression thresholds to 10ms/2% and 1KB/1% to catch all meaningful Next.js regressions - Add 3-decimal precision for seconds (1.234s instead of 1.2s) - Rename metrics: 'Boot/Ready' -> 'Listen/First Request' for clarity - Emphasize Turbopack (default), collapse Webpack as legacy - Add single glossary instead of 5 repetitive dropdowns - Simplify group names (no 'Turbopack' prefix since it's default)
- Increase benchmark iterations (dev: 5→9, build: 3→5) for more stable medians - Adjust significance thresholds to 50ms AND 10% (was 10ms AND 2%) - Add trend sparklines to summary section when KV history available - Consolidate calcStats() into shared util/stats.js - Standardize metric keys (remove (ms) suffix) - Add test-local.js for easier local testing - Better KV logging for debugging
- Move Next Runtimes out of server category into new 'shared' category - Rename 'Client Bundles (main, webpack)' to 'Client Bundles (main)' - The old name was confusing when displayed under Turbopack section
- Add computeBundleGroupTotals() for KV bundle size trend tracking - generateMetricsTable now conditionally shows Trend column based on data - Each table independently decides based on its own metrics - Empty Trend column hidden completely when no history available
Add <2% threshold for time metrics to filter noise on long-running operations like builds. A 70ms change on a 13s build (0.5%) is noise, not a real regression. New threshold logic: - Time: (<50ms AND <10%) OR <2% = insignificant - Size: <1KB AND <1% = insignificant
File renames (hash normalization) were invalidating the build cache, causing cached build times to be the same as fresh builds. Now: 1. Fresh builds (clean .next each iteration) 2. Cached builds (immediately after, .next intact) 3. Apply renames (for deterministic file names) 4. Collect file stats (after renames)
41d7210 to
a5e7ddb
Compare
| } | ||
|
|
||
| const res = await fetch(endpoint, { | ||
| method, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Content-Type header was conditionally omitted when sending JSON POST/PATCH requests with GitHub authentication token
View Details
📝 Patch Details
diff --git a/.github/actions/next-stats-action/src/add-comment.js b/.github/actions/next-stats-action/src/add-comment.js
index fa532bc3aa..818bb5dbb6 100644
--- a/.github/actions/next-stats-action/src/add-comment.js
+++ b/.github/actions/next-stats-action/src/add-comment.js
@@ -1084,13 +1084,12 @@ module.exports = async function addComment(
const res = await fetch(endpoint, {
method,
headers: {
+ 'content-type': 'application/json',
...(actionInfo.githubToken
? {
Authorization: `bearer ${actionInfo.githubToken}`,
}
- : {
- 'content-type': 'application/json',
- }),
+ : {}),
},
body: JSON.stringify(body),
})
Analysis
The code at lines 1085-1093 had a ternary operator that selected headers based on whether actionInfo.githubToken was set:
Original Logic:
- When
githubTokenIS present: Only setAuthorizationheader - When
githubTokenis NOT present: Only setContent-Type: application/jsonheader
The Problem:
Both execution paths send JSON data via JSON.stringify(body). However, when the GitHub token IS set (the primary authenticated path to the GitHub API), the Content-Type: application/json header was missing. This violates GitHub API v3 requirements, which mandate the Content-Type header for JSON POST/PATCH requests. Without this header, the API may not properly parse the request body, leading to potential API errors or unexpected behavior.
The Fix:
The Content-Type: application/json header is now always included when sending JSON data, regardless of authentication method. The Authorization header is conditionally added only when a GitHub token is present. This ensures compliance with GitHub API requirements while maintaining proper headers in both authenticated and custom endpoint scenarios.
Changed Code:
// Before: Headers were mutually exclusive based on githubToken presence
headers: {
...(actionInfo.githubToken
? { Authorization: `bearer ${actionInfo.githubToken}` }
: { 'content-type': 'application/json' }),
}
// After: Content-Type is always included, Authorization is conditional
headers: {
'content-type': 'application/json',
...(actionInfo.githubToken
? { Authorization: `bearer ${actionInfo.githubToken}` }
: {}),
}This ensures both headers are present when needed, which is essential for proper HTTP communication with the GitHub API.
| endpoint = actionInfo.commentEndpoint.replace( | ||
| /\/issues\/\d+\/comments$/, | ||
| `/issues/comments/${existingCommentId}` | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| endpoint = actionInfo.commentEndpoint.replace( | |
| /\/issues\/\d+\/comments$/, | |
| `/issues/comments/${existingCommentId}` | |
| ) | |
| // or PATCH /repos/{owner}/{repo}/comments/{comment_id} for commit comments | |
| if (/\/issues\/\d+\/comments$/.test(actionInfo.commentEndpoint)) { | |
| endpoint = actionInfo.commentEndpoint.replace( | |
| /\/issues\/\d+\/comments$/, | |
| `/issues/comments/${existingCommentId}` | |
| ) | |
| } else if (/\/commits\/[a-f0-9]+\/comments$/.test(actionInfo.commentEndpoint)) { | |
| endpoint = actionInfo.commentEndpoint.replace( | |
| /\/commits\/[a-f0-9]+\/comments$/, | |
| `/comments/${existingCommentId}` | |
| ) | |
| } |
Regex pattern for comment endpoint update fails to match commit comment URLs, causing PATCH requests to wrong endpoint
View Details
📝 Patch Details
diff --git a/.github/actions/next-stats-action/src/add-comment.js b/.github/actions/next-stats-action/src/add-comment.js
index fa532bc3aa..42a3b160d1 100644
--- a/.github/actions/next-stats-action/src/add-comment.js
+++ b/.github/actions/next-stats-action/src/add-comment.js
@@ -1071,10 +1071,18 @@ module.exports = async function addComment(
if (existingCommentId && actionInfo.githubToken) {
// GitHub API: PATCH /repos/{owner}/{repo}/issues/comments/{comment_id}
- endpoint = actionInfo.commentEndpoint.replace(
- /\/issues\/\d+\/comments$/,
- `/issues/comments/${existingCommentId}`
- )
+ // or PATCH /repos/{owner}/{repo}/comments/{comment_id} for commit comments
+ if (/\/issues\/\d+\/comments$/.test(actionInfo.commentEndpoint)) {
+ endpoint = actionInfo.commentEndpoint.replace(
+ /\/issues\/\d+\/comments$/,
+ `/issues/comments/${existingCommentId}`
+ )
+ } else if (/\/commits\/[a-f0-9]+\/comments$/.test(actionInfo.commentEndpoint)) {
+ endpoint = actionInfo.commentEndpoint.replace(
+ /\/commits\/[a-f0-9]+\/comments$/,
+ `/comments/${existingCommentId}`
+ )
+ }
method = 'PATCH'
logger(`Updating existing comment at ${endpoint}`)
} else {
Analysis
The code at line 1074-1076 uses a regex pattern /\/issues\/\d+\/comments$/ to convert comment creation endpoints into update endpoints. However, for commit comments (used in release scenarios), the endpoint has format /repos/owner/repo/commits/{commitId}/comments instead of /repos/owner/repo/issues/{id}/comments.
The regex fails to match commit comment endpoints, causing the replace() to have no effect. This results in PATCH requests being sent to the creation endpoint instead of the update endpoint. According to GitHub API:
- Issue comment updates: PATCH /repos/{owner}/{repo}/issues/comments/{comment_id}
- Commit comment updates: PATCH /repos/{owner}/{repo}/comments/{comment_id}
The fix adds conditional logic to detect and handle both endpoint types correctly, with different replacement patterns for each. This ensures PATCH requests are sent to the correct update endpoints.

Summary
Comprehensive improvements to the PR stats action for more reliable benchmarks and reduced CI noise.
Vercel KV Integration
@vercel/kvfor historical data persistenceloadHistory()/saveToHistory()Comment Generation Refactor
METRIC_LABELSandMETRIC_GROUPSconfiguration systemprettifyTime,formatChange,generateTrendBar)Noise Reduction
50ms AND 10%for time,1KB AND 1%for size<2%filter for long-running operations (builds)Stats Config Updates
measureDevBoot: trueto enable dev boot benchmarksappDevCommandto includedevsubcommand--webpackflag toappBuildCommandturbopack: {}to test configsGitHub Actions Updates
action.ymlwith bundler input parameterpull_request_stats.ymlNew Files
scripts/test-stats-benchmark.sh- Local testing script.github/actions/next-stats-action/src/util/stats.js- Shared stats utilities.github/actions/next-stats-action/test-local.js- Local comment testingTest Plan