feat(ci): measure the per-commit benchmark path against the mocked population - #45455
Open
MajorLift wants to merge 3 commits into
Open
feat(ci): measure the per-commit benchmark path against the mocked population#45455MajorLift wants to merge 3 commits into
MajorLift wants to merge 3 commits into
Conversation
MajorLift
requested review from
a team,
HowardBraham and
itsyoboieltr
as code owners
August 12, 2026 09:56
Contributor
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
metamask-ci
Bot
added
team-extension-platform
Contributor
✨ Files requiring CODEOWNER review ✨👨🔧 @MetaMask/extension-platform (5 files, +67 -8)
👨🔧 @HowardBraham (5 files, +67 -8)
👨🔧 @itsyoboieltr (5 files, +67 -8)
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 90740f9. Configure here.
5 tasks
Contributor
Builds ready [90740f9]
⚡ Performance Benchmarks (Total: 🟢 16 pass · 🟡 7 warn · 🔴 1 fail)
Bundle size diffs
|
MajorLift
force-pushed
the
jongsun/ci/benchmark-population-consistency
branch
from
August 12, 2026 10:35
fb38a10 to
338de43
Compare
…anch `resolveBenchmarkMockMode` takes an explicit override that beats the branch heuristic, threaded from a `benchmark-mock-mode` workflow input defaulting to `mocked`. Every job that resolves the population reads it, so the harness, the gate and the publisher cannot disagree. The two series get separate files — `performance_data_mocked.json` alongside the existing `performance_data.json` — so a mocked run can never overwrite the live series, and the consumer's mockMode filter stops being the only thing keeping them apart. Live measurement moves to the scheduled drift job in #45446, which passes `live` through the same input.
MajorLift
force-pushed
the
jongsun/ci/benchmark-mocked-series
branch
from
August 12, 2026 10:36
461b7a4 to
f020eb5
Compare
5 tasks
|
Contributor
Builds ready [f020eb5]
⚡ Performance Benchmarks (Total: 🟢 13 pass · 🟡 10 warn · 🔴 1 fail)
Bundle size diffs
|
Contributor
Builds ready [f020eb5]
⚡ Performance Benchmarks (Total: 🟢 13 pass · 🟡 10 warn · 🔴 1 fail)
Bundle size diffs
|
Splitting the write path left the read path pointing at the live file unconditionally, so a mocked consumer filtered an unsuffixed series that holds no mocked entries and reported no baseline forever — the mocked series would have been published and never read. `getBenchmarkStatsUrl` selects the file; the PR comment's "Baseline logs" link follows the same selection so it cannot contradict the numbers beside it.
Contributor
Builds ready [64c8933] [reused from f020eb5]
⚡ Performance Benchmarks (Total: 🟢 13 pass · 🟡 10 warn · 🔴 1 fail)
Bundle size diffs
|
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
1 participant
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.
Changelog
CHANGELOG entry: null
Description
#45204 keeps the two populations from being blended. It does not give the gate a population it can actually use: only
mainandrelease/*publish, and both run live, so a mocked PR run resolves no baseline at all.This makes the per-commit path measure
mockedon every branch includingmain, which is what produces a mocked series for PRs to compare against. Live measurement does not disappear — it moves to the scheduled drift job in #45446, which passeslivethrough the same input.Why the per-commit path should not be live
The live population's timings are substantially a property of CDN cache state rather than of the commit.
tokens.api.cx.metamask.io/v3/assetsservescache-control: max-age=10800, and on that endpoint a cache miss costs 292ms against 105ms for a hit — 2.77× at the median, on an identical request seconds apart (#45446). The cache is warmed by everyone else's traffic, so the hit ratio is exogenous: two runs of the same commit minutes apart can land either side of a TTL boundary.That is tolerable for drift detection, which averages over many runs. It is not tolerable per-commit, where one run has to decide whether a merge proceeds.
What changed
resolveBenchmarkMockMode(branch, override?)— an explicitmocked/liveoverride beats the branch heuristic. Anything else, including the empty string an unset workflow input expands to, falls through to the existing branch behaviour.benchmark-mock-modeworkflow input, defaultmocked, threaded into all four jobs —benchmarks,benchmarks-page-load,quality-gate,store-benchmark-stats. Every site that resolves the population reads the same value, so the harness, the gate and the publisher cannot disagree. A disagreement would file a live measurement in the mocked series, which is the failure this is shaped to prevent.stats/<branch>/performance_data_mocked.jsonalongside the existingperformance_data.json. Keying only by branch would now put both series in one file, leaving the consumer'smockModefilter as the only thing keeping them apart — one unstamped entry and a mocked baseline silently absorbs live latency.livekeeps the historical path so the existing series stays continuous.resolve_mock_mode()inbenchmark-stats-commit.sh, mirroring the TypeScript resolver including its precedence.Verification
Both arms of the resolver change:
The two failures on base are exactly the two override cases. The fallback test — unset, empty,
MOCKED,mock,trueall falling through to the branch heuristic — passes in both arms, because it pins behaviour this must not change.The shell resolver must agree with the TypeScript one, since they independently decide the same fact. All 20 branch × override combinations:
shell-resolver-check.txtAn unrecognised override never picks a population — it falls through to the branch, in both implementations.
bash -npasses on the script; the workflow parses and all four jobs carryBENCHMARK_MOCK_MODE.What is not proven here: that the mocked series is usable as a baseline. That needs the series to exist and accumulate depth, which is weeks of runs, not a PR. This delivers the publishing half of #45205; re-deriving the ceilings is the other half and stays open.
Gap to close after this lands: until #45446's scheduled job exists, nothing runs live. That is the intended sequencing — per-commit stops measuring the CDN first — but the live series pauses in the interval, and #45446 should follow closely.
Related issues
Progresses: #45205
performance_data.jsonexceeds GitHub's 100 MB limit #45451 — the size cap, without which a second series hits the same 100 MiB wallManual testing steps
yarn jest shared/constants/benchmarks.test.ts development/metamaskbot-build-announce/mainpush, confirmstats/main/performance_data_mocked.jsonis created andperformance_data.jsonis untouched.Pre-merge author checklist
Note
Medium Risk
Touches CI quality-gate baseline selection and stats publishing paths; a harness/publisher/gate disagreement could silently mix live and mocked series or leave PRs without a usable baseline.
Overview
Makes the per-commit benchmark path measure the mocked population on every branch (including
main), so PRs can eventually gate against a matching historical series instead of finding no baseline.resolveBenchmarkMockMode(branch, override?)now lets an explicitmocked/livevalue beat the branch heuristic; unrecognized or empty overrides still fall through. The same override is threaded via a newbenchmark-mock-modeworkflow input (defaultmocked) into the harness, quality gate, and stats publisher so they cannot disagree.Mocked results are written to
performance_data_mocked.json; live keeps the unsuffixed historical path. Readers (getBenchmarkStatsUrl, announce/gate baseline fetch) select the matching file rather than relying only on in-filemockModefiltering.Reviewed by Cursor Bugbot for commit 64c8933. Bugbot is set up for automated code reviews on this repo. Configure here.