From e63173353df52661b2504f6951fe7918ee014680 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 06:13:35 +0000 Subject: [PATCH] fix(performance): scope the push-time label and refuse raised counter values - A master push compares the whole push, but the label was read from the head commit's PR only, so one labelled PR could cover another commit's increase in the same push. The label now counts only when the push added exactly one first-parent commit (a squash or merge of one PR); any other push that weakens a baseline fails. - Raising a counter's `value` while narrowing its `slack` lowered the enforced limit and was reported as "lowered". A counter's value is the measured evidence and only moves down, so that raise is now a weakening that needs the label. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0166PWobUjpoiWt8E9bBdsCs --- .github/workflows/ci.yml | 19 ++++++++---- docs/architecture/performance-journeys.md | 10 +++++-- .../performance/check-baseline-direction.mjs | 11 ++++++- .../check-baseline-direction.test.mjs | 29 +++++++++++++++++++ 4 files changed, 60 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 91b9d0238..6f1716886 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -158,10 +158,13 @@ jobs: # branch for a pull request, the previous head for a master push, # master for a manual dispatch. Any raised limit, widened tolerance # or slack, or removed entry fails, unless a maintainer put the - # perf-baseline-increase label on the pull request (for a master - # push: on the pull request merged as the pushed commit). Labels - # are read from the API, not the event payload, so re-running this - # job after adding the label picks it up. + # perf-baseline-increase label on the pull request. For a master + # push the label counts only when the push added exactly one + # first-parent commit (a squash or merge of one PR) and that + # commit's PR carries it: the check compares the whole push, so a + # multi-commit push cannot borrow one PR's label for another's + # increase. Labels are read from the API, not the event payload, + # so re-running this job after adding the label picks it up. - name: Refuse baseline increases against the previous revision env: EVENT_NAME: ${{ github.event_name }} @@ -196,7 +199,13 @@ jobs: labels="$(gh api "repos/$REPOSITORY/issues/$PR_NUMBER/labels" --jq '.[].name')" ;; push) - labels="$(gh api "repos/$REPOSITORY/commits/$HEAD_SHA/pulls" --jq '.[].labels[].name')" + parent="$(gh api "repos/$REPOSITORY/commits/$HEAD_SHA" --jq '.parents[0].sha')" + if [ "$parent" = "$BEFORE_SHA" ]; then + labels="$(gh api "repos/$REPOSITORY/commits/$HEAD_SHA/pulls" --jq '.[].labels[].name')" + else + echo "This push added more than one commit; the $INCREASE_LABEL label is not consulted." + labels="" + fi ;; *) labels="" diff --git a/docs/architecture/performance-journeys.md b/docs/architecture/performance-journeys.md index b411d40ba..fbc445d14 100644 --- a/docs/architecture/performance-journeys.md +++ b/docs/architecture/performance-journeys.md @@ -358,7 +358,9 @@ The job also refuses a weakened baselines file: `master` for a manual dispatch) and fails when any entry's enforced limit (`value × toleranceRatio` or `value + slack`) went up, a tolerance or slack widened or an entry disappeared, so a PR cannot grow the -payload and raise the baseline to match. Lowered limits and new entries pass. +payload and raise the baseline to match. A counter's `value` may not go up +either, even when narrower slack lowers its limit. Lowered limits and new +entries pass. Baselines only move down. Lower `value` in the same PR as the change that earned it, set `updatedAt` and `evidencePr`, and paste the measurement output @@ -369,8 +371,10 @@ per-file breakdown, and ask a maintainer to add the `perf-baseline-increase` label. With the label the direction check prints the weakened entries as `ALLOWED` and passes; the job reads labels from the API when it runs, so re-run the job after the label is added. For a `master` push the label is -read from the pull request merged as the pushed commit, so a squash merge of -a labelled PR passes and a direct push that raises a baseline still fails. +read from the pull request merged as the pushed commit, and only when the +push added exactly one first-parent commit: a squash or merge of a labelled +PR passes, while a direct push, or a push of several commits (which the check +compares as a whole), that raises a baseline still fails. Only people with triage access can set labels, so the label is the maintainer decision. diff --git a/tools/performance/check-baseline-direction.mjs b/tools/performance/check-baseline-direction.mjs index e17b150a2..b0b8b9949 100644 --- a/tools/performance/check-baseline-direction.mjs +++ b/tools/performance/check-baseline-direction.mjs @@ -7,7 +7,9 @@ * check closes that gap: given the baselines file of the target branch and * the one of the PR, any entry whose enforced limit (`value × toleranceRatio` * or `value + slack`) went up, whose tolerance or slack widened, or that - * disappeared, is a failure. New entries and lowered limits pass. + * disappeared, is a failure. A counter's `value` may not go up either, even + * when narrower slack lowers its limit: the value is the measured evidence. + * New entries and lowered limits pass. * * `--allow-increase` turns those failures into printed "allowed" lines. CI * passes it only when a maintainer put the perf-baseline-increase label on @@ -97,6 +99,13 @@ export function compareBaselineDirection({ weakened.push( `${label}: baseline raised from ${formatNumber(baseLimit)} to ${formatNumber(headLimit)}${unit}. Baselines only move down; bring the measurement back under ${formatNumber(baseLimit)}, or make the case for the increase in the PR and ask a maintainer to add the ${BASELINE_INCREASE_LABEL} label.` ); + } else if ( + headEntry.toleranceRatio === undefined && + headEntry.value > baseEntry.value + ) { + weakened.push( + `${label}: baseline value raised from ${formatNumber(baseEntry.value)} to ${formatNumber(headEntry.value)}${unit} while slack narrowed from ${formatNumber(baseSlack)} to ${formatNumber(headSlack)}. A counter's value only moves down; narrowing its slack does not offset a raise. Ask a maintainer to add the ${BASELINE_INCREASE_LABEL} label if the raise is deliberate.` + ); } else if (headLimit < baseLimit) { result.lowered.push( `${label}: ${formatNumber(baseLimit)} -> ${formatNumber(headLimit)}${unit}.` diff --git a/tools/performance/check-baseline-direction.test.mjs b/tools/performance/check-baseline-direction.test.mjs index 87b3ba9e6..54b1346a2 100644 --- a/tools/performance/check-baseline-direction.test.mjs +++ b/tools/performance/check-baseline-direction.test.mjs @@ -184,6 +184,35 @@ test('a widened or newly added slack fails; a narrowed one lowers the limit', () assert.match(raised.failures[0], /perf-baseline-increase label/); }); +test('a counter value raised behind narrower slack still fails', () => { + const withSlack = (value, slack) => ({ + version: 1, + journeys: { + launch: { + 'renderer.initialBytes': { value, unit: 'bytes', slack }, + }, + }, + }); + const result = compareBaselineDirection({ + base: withSlack(100, 10), + head: withSlack(105, 0), + }); + assert.equal(result.failures.length, 1); + assert.deepEqual(result.lowered, []); + assert.match( + result.failures[0], + /renderer\.initialBytes: baseline value raised from 100 to 105 bytes while slack narrowed from 10 to 0/ + ); + + const allowed = compareBaselineDirection({ + base: withSlack(100, 10), + head: withSlack(105, 0), + allowIncrease: true, + }); + assert.deepEqual(allowed.failures, []); + assert.equal(allowed.allowed.length, 1); +}); + test('allowIncrease reports every weakening as allowed instead of failing', () => { const result = compareBaselineDirection({ base: file(100, { cdTicks: { value: 5 } }),