Skip to content

Commit 084f5f2

Browse files
eps1lonclaude
andauthored
[ci] Compare size against merge base (react#37355)
Sizebot compared the pull request head build against the build of `pull_request.base.sha`, which is the tip of the base branch at event time, not the commit the pull request diverged from. The field's semantics are undocumented in GitHub's API schema (the OpenAPI description types it as a bare string); the observed behavior and the compare API's `merge_base_commit` confirm the difference. The difference between `pull_request.base.sha` and the merge base is confirmed with an example in react#37356 The sizebot job now resolves the merge-base through the compare API and downloads the base build for that commit instead, so the report only ever contains the pull request's own changes. The job gains `contents: read` for the compare call. When no base build can be downloaded for the merge-base, for example because its artifacts aged out of the retention window or its run failed, the sizebot job records a `base-build-not-found` result instead of failing immediately. `render-comment.js` on the default branch renders that as a warning comment naming the base commit and writes the `sizebot-problem.txt` marker, so the comment workflow fails its check after posting the warning, the same pattern already used for build configuration drift. The sizebot job itself intentionally stays green: a failed run would make the renderer discard the results and mask the warning with a generic "did not complete" message. Co-authored-by: Claude Code (kimi-k3[1m]) <noreply@anthropic.com>
1 parent d04798f commit 084f5f2

3 files changed

Lines changed: 75 additions & 8 deletions

File tree

.github/workflows/runtime_build_and_test.yml

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -999,6 +999,8 @@ jobs:
999999
permissions:
10001000
# We use github.token to download the build artifact from a previous runtime_build_and_test.yml run
10011001
actions: read
1002+
# Used to resolve the merge-base commit through the compare API
1003+
contents: read
10021004
runs-on: ubuntu-latest
10031005
steps:
10041006
- uses: actions/checkout@v4
@@ -1020,21 +1022,40 @@ jobs:
10201022
if: steps.node_modules.outputs.cache-hit != 'true'
10211023
- run: yarn --cwd scripts/release install --frozen-lockfile
10221024
if: steps.node_modules.outputs.cache-hit != 'true'
1025+
- name: Resolve base commit for comparison
1026+
# Compare against the commit this pull request diverged from, not the
1027+
# current tip of the base branch. Otherwise, once main moves ahead,
1028+
# unrelated commits show up as size changes on every pull request.
1029+
id: base
1030+
run: |
1031+
echo "sha=$(gh api repos/${{ github.repository }}/compare/${{ github.event.pull_request.base.sha }}...${{ github.event.pull_request.head.sha }} --jq .merge_base_commit.sha)" >> "$GITHUB_OUTPUT"
1032+
env:
1033+
GH_TOKEN: ${{ github.token }}
10231034
- name: Download artifacts for base revision
1035+
id: download_base
10241036
# The build could have been generated from a fork, so we must download the build without
10251037
# any verification. This is safe since we only use this for sizebot calculation and the
10261038
# unverified artifact is not used. Additionally this workflow runs in the pull_request
10271039
# trigger so only restricted permissions are available.
10281040
run: |
1029-
GH_TOKEN=${{ github.token }} scripts/release/download-experimental-build.js --commit=$(git rev-parse ${{ github.event.pull_request.base.sha }}) ${{ (github.event.pull_request.head.repo.full_name != github.repository && '--noVerify') || ''}}
1030-
mv ./build ./base-build
1041+
if GH_TOKEN=${{ github.token }} scripts/release/download-experimental-build.js --commit=${{ steps.base.outputs.sha }} ${{ (github.event.pull_request.head.repo.full_name != github.repository && '--noVerify') || ''}}; then
1042+
mv ./build ./base-build
1043+
else
1044+
# Rendered as a warning comment by render-comment.js on the
1045+
# default branch, which also fails the comment workflow's check.
1046+
jq --null-input --arg baseSha "${{ steps.base.outputs.sha }}" \
1047+
'{version: 1, status: "base-build-not-found", baseSha: $baseSha}' \
1048+
> sizebot-results.json
1049+
echo "missing=true" >> "$GITHUB_OUTPUT"
1050+
fi
10311051
- name: Delete extraneous files
10321052
# TODO: The `download-experimental-build` script copies the npm
10331053
# packages into the `node_modules` directory. This is a historical
10341054
# quirk of how the release script works. Let's pretend they
10351055
# don't exist.
10361056
run: rm -rf ./base-build/node_modules
1037-
- name: Display structure of base-build from origin/main
1057+
- name: Display structure of base-build
1058+
if: steps.download_base.outputs.missing != 'true'
10381059
run: ls -R base-build
10391060
- name: Ensure clean build directory
10401061
run: rm -rf build
@@ -1056,10 +1077,18 @@ jobs:
10561077
# posted by runtime_sizebot_comment.yml, which runs on the workflow_run
10571078
# trigger because this job's token is read-only for pull requests from
10581079
# forks and so cannot comment.
1080+
# Skipped when the base build is missing: the download step already
1081+
# wrote the not-found results.
1082+
if: steps.download_base.outputs.missing != 'true'
10591083
run: node ./scripts/sizebot/compare-sizes.js
10601084
- name: Archive sizebot results
10611085
uses: actions/upload-artifact@v4
10621086
with:
10631087
name: sizebot-results
10641088
path: sizebot-results.json
10651089
if-no-files-found: error
1090+
# Note: a missing base build does not fail this job on purpose. A failed
1091+
# job would fail the whole run, and render-comment.js only reads the
1092+
# results of successful runs, so the warning would never be rendered.
1093+
# Instead the comment workflow fails on the sizebot-problem.txt marker
1094+
# after posting the warning, same as for build configuration drift.

.github/workflows/runtime_sizebot_comment.yml

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,10 +95,11 @@ jobs:
9595
const {post} = require(`${process.env.GITHUB_WORKSPACE}/scripts/sizebot/pull-request-comment.js`);
9696
await post({github, context, core});
9797
98-
- name: Fail if the build configuration drifted
98+
- name: Fail if sizebot reported a problem
9999
# The comment is posted first, so it explains the problem on the pull
100-
# request itself. This step exists so the drift also shows up as a failed
101-
# run rather than only in a comment.
100+
# request itself. This step exists so the problem (build configuration
101+
# drift, or no base build to compare against) also shows up as a
102+
# failed run rather than only in a comment.
102103
if: ${{ steps.resolve.outputs.action == 'continue' && hashFiles('sizebot-problem.txt') != '' }}
103104
run: |
104105
cat sizebot-problem.txt

scripts/sizebot/render-comment.js

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,11 @@ const {existsSync, readFileSync, writeFileSync} = require('fs');
2626
// Results shapes this file knows how to read. `compare-sizes.js` on the pull
2727
// request branch may be older or newer than this list.
2828
const SUPPORTED_VERSIONS = new Set([1]);
29-
const SUPPORTED_STATUSES = new Set(['ok', 'base-artifacts-unavailable']);
29+
const SUPPORTED_STATUSES = new Set([
30+
'ok',
31+
'base-artifacts-unavailable',
32+
'base-build-not-found',
33+
]);
3034

3135
const CRITICAL_THRESHOLD = 0.02;
3236
const SIGNIFICANCE_THRESHOLD = 0.002;
@@ -161,6 +165,15 @@ function validateResults(raw) {
161165
if (raw.status === 'base-artifacts-unavailable') {
162166
return {ok: true, results: {status: raw.status}};
163167
}
168+
if (raw.status === 'base-build-not-found') {
169+
return {
170+
ok: true,
171+
results: {
172+
status: raw.status,
173+
baseSha: isSha(raw.baseSha) ? raw.baseSha : null,
174+
},
175+
};
176+
}
164177
if (!isSha(raw.baseSha) || !isSha(raw.headSha)) {
165178
return {ok: false, reason: 'malformed'};
166179
}
@@ -337,6 +350,23 @@ function renderCompletedReport(context) {
337350
};
338351
}
339352

353+
if (validated.results.status === 'base-build-not-found') {
354+
const {baseSha} = validated.results;
355+
return {
356+
markdown:
357+
`No build was found for the base commit${
358+
baseSha === null ? '' : ` (${baseSha})`
359+
} that this pull request diverged from, so there is no size report. ` +
360+
'The build for that commit may have failed, or its artifacts may be ' +
361+
'older than the retention window. Rebase the pull request onto a ' +
362+
'newer `main` to compare against a base commit that has a build.',
363+
missingCriticalPaths: [],
364+
problem: `No base build found for ${
365+
baseSha === null ? 'the merge-base' : baseSha
366+
}`,
367+
};
368+
}
369+
340370
return renderTable(validated.results);
341371
}
342372

@@ -370,6 +400,7 @@ function renderBody(context) {
370400
let reportHead;
371401
let report;
372402
let missingCriticalPaths = [];
403+
let problem;
373404

374405
if (context.action === 'requested') {
375406
// Only a comment that names the commit it describes holds real numbers. A
@@ -392,6 +423,7 @@ function renderBody(context) {
392423
const rendered = renderCompletedReport(context);
393424
report = rendered.markdown;
394425
missingCriticalPaths = rendered.missingCriticalPaths;
426+
problem = rendered.problem;
395427
}
396428

397429
if (missingCriticalPaths.length > 0) {
@@ -428,6 +460,7 @@ ${REPORT_END}
428460
assemble,
429461
reportHead,
430462
missingCriticalPaths,
463+
problem,
431464
};
432465
}
433466

@@ -454,7 +487,8 @@ function parseReportHead(body) {
454487

455488
function main() {
456489
const context = JSON.parse(readFileSync(CONTEXT_PATH, 'utf8'));
457-
const {body, report, assemble, missingCriticalPaths} = renderBody(context);
490+
const {body, report, assemble, missingCriticalPaths, problem} =
491+
renderBody(context);
458492

459493
let comment = body;
460494
if (body.length > MAX_COMMENT_LENGTH) {
@@ -473,6 +507,9 @@ function main() {
473507
`Missing expected bundles:\n${missingCriticalPaths.join('\n')}\n`
474508
);
475509
}
510+
if (problem !== undefined) {
511+
writeFileSync(PROBLEM_PATH, problem + '\n');
512+
}
476513

477514
process.stdout.write(comment);
478515
}

0 commit comments

Comments
 (0)