MetaMask / MetaMask/metamask-extension

[P1] The gate's only blocking metric has overlapping pass and fail ranges

Open
#45,935 1 comment 0 reactions 0 assignees View on GitHub
INVALID-ISSUE-TEMPLATE Sev2-normal ta-ai-fixable ta-needs-engineer-escalation ta-triaged team-qa
Dominant language
TypeScript
Stars
13.2k
Forks
5.6k
Avg merge
2d 5h
Merged PRs (30d)
451

Description

**Parent:** [#7207 Statistical Sanitization](https://github.com/MetaMask/MetaMask-planning/issues/7207)
**Files:** `test/e2e/benchmarks/utils/gated-metrics.ts`, `test/e2e/benchmarks/utils/statistics.ts`, `development/metamaskbot-build-announce/comparison-utils.ts`
**Size:** M | **Hours:** 4-8

---

## Problem

`onboardingImportWallet.total` is the **only** metric that has ever blocked the gate, and its passing and failing measurements overlap. So a PR unrelated to benchmarks can be blocked or let through by which side of a non-deterministic boundary its run happens to land on.

Measured over **87 completed `main`-branch push runs**, 7 days to `2026-09-01T12:07:29Z`, of which **85 yielded a recoverable gate verdict**:

| observation | value |
|---|---|
| Runs where the gate returned `FAIL` | 15 of 85 |
| Metric responsible for every one of them | `onboardingImportWallet.total` |
| Passing range | 9174 – 11236 ms |
| Failing range | 11108 – 11640 ms |
| **Overlap** | **128 ms** |

**`11108 ms` has both passed and failed**, on the same metric, against the same printed `limit 11050ms`. That printed limit is `THRESHOLD_REGISTRY.onboardingImportWallet.total.p75.fail` = 8500ms times `CI_MULTIPLIER.TIER_1` = 1.3, at [`thresholds.ts` L85-L89](https://github.com/MetaMask/metamask-extension/blob/0350a01878e0d5e0eb1553b18ca7d653b4157cbb/test/e2e/benchmarks/utils/thresholds.ts#L85-L89).

Note on sample size: at 61 gate logs the two ranges appeared **disjoint with a 56 ms gap**. Over the complete 85 they overlap by 128 ms. The partial sample understated the problem, so any re-measurement should use the full window.

### Two run-dependent adjustments sit between the printed limit and the blocking decision

Neither is disclosed on the line the developer reads, and both move with the run's own samples rather than with the code under test.

1. **CV-adaptive widening.** [`getEffectiveThreshold`](https://github.com/MetaMask/metamask-extension/blob/0350a01878e0d5e0eb1553b18ca7d653b4157cbb/test/e2e/benchmarks/utils/statistics.ts#L444-L462) multiplies the ceiling by `1 + cv/200` when the run's measured CV falls in the 25–50 band, so a noisier run receives a more permissive ceiling — up to 1.25× at cv 50. Outside the band the multiplier is not applied, and above cv 50 [`validateResultThresholds`](https://github.com/MetaMask/metamask-extension/blob/0350a01878e0d5e0eb1553b18ca7d653b4157cbb/test/e2e/benchmarks/utils/statistics.ts#L636-L639) drops the metric from validation entirely. One printed limit, three outcomes. The discontinuity itself is [#45544 CV-adaptive threshold widening switches off above `CV_ADAPTIVE_MAX`](https://github.com/MetaMask/metamask-extension/issues/45544).
2. **Noise tolerance.** [`applyNoiseTolerance`](https://github.com/MetaMask/metamask-extension/blob/0350a01878e0d5e0eb1553b18ca7d653b4157cbb/development/metamaskbot-build-announce/comparison-utils.ts#L341-L375) downgrades a `Fail` to `Warn` when `value − threshold` is smaller than the run's own stdDev for that metric. The blocking line is therefore `threshold + stdDev`, and stdDev is measured per run. A 241ms breach blocked or did not block depending on whether that run's stdDev came in under or over 241ms.

A measurement that is over the line in a quiet run is under it in a noisy one, twice over.

### The other eleven gated timing metrics cannot reach their ceilings

The claim above — that one metric produces every block — holds across a second, later window, and the reason the other eleven contribute nothing is not that they came close and stayed under.

**116 Chrome runs on `main`, 2026-08-27T00:05Z to 2026-09-03T03:03Z**, read from the per-run `benchmark-chrome-webpack-*.json` artifacts rather than from any aggregate — for example [run 33709976042](https://github.com/MetaMask/metamask-extension/actions/runs/33709976042). 120 runs fell in the window, 118 carried all five Chrome benchmark artifacts, and 2 expired at the 7-day retention boundary during the harvest. Ceiling is the base `p75.fail` times its `ciMultiplier` from [`thresholds.ts`](https://github.com/MetaMask/metamask-extension/blob/19d71034e6bb1adc7ecc16725ba4e5a5a882cfce/test/e2e/benchmarks/utils/thresholds.ts); headroom is ceiling divided by the highest per-run p75 observed.

| gated timing metric | tier | ceiling | p50 | max | headroom | blocked |
|---|---|---:|---:|---:|---:|---:|
| `onboardingImportWallet.total` | TIER_1 | 11,050 | 10,325 | 11,640 | **0.9x** | **27/116** |
| `swap.total` | TIER_2 | 4,420 | 2,956 | 3,076 | 1.4x | 0/116 |
| `importSrpHome.total` | TIER_2 | 42,500 | 22,759 | 25,710 | 1.7x | 0/116 |
| `importSrpHome.homeAfterImportWithNewWallet` | TIER_2 | 45,900 | 20,513 | 23,487 | 2.0x | 0/116 |
| `startupStandardHome.uiStartup` | TIER_1 | 3,250 | 1,057 | 1,176 | 2.8x | 0/116 |
| `swap.fetchAndDisplaySwapQuotes` | TIER_2 | 8,500 | 2,718 | 2,910 | 2.9x | 0/116 |
| `startupStandardHome.loadScripts` | TIER_1 | 2,340 | 722 | 791 | 3.0x | 0/116 |
| `startupStandardHome.load` | TIER_1 | 2,860 | 731 | 804 | 3.6x | 0/116 |
| `importSrpHome.loginToHomeScreen` | TIER_1 | 9,100 | 1,339 | 1,659 | 5.5x | 0/116 |
| `sendTransactions.openSendPageFromHome` | TIER_2 | 4,590 | 15 | 53 | 86x | 0/104 |
| `onboardingImportWallet.metricsToWalletReadyScreen` | TIER_2 | 5,950 | 31 | 54 | 109x | 0/116 |
| `onboardingNewWallet.agreeButtonToOnboardingSuccess` | TIER_2 | 5,950 | 31 | 41 | **147x** | 0/116 |

Three metrics peak near 1% of their ceiling and would need a hundredfold slowdown to register. The tightest of the eleven still holds 1.4x. **So the gate's blocking behaviour is not merely dominated by one metric in this window — the other eleven are positioned where no plausible regression trips them.** That is the same calibration defect as `onboardingImportWallet.total` in the opposite direction: no ceiling was compared against its metric's own run-to-run distribution, so each landed wherever a local-timing calibration times a fixed tier multiplier put it.

`importSrpHome.total` is `CI_MULTIPLIER.TIER_2`, giving a 42,500 ms ceiling rather than the 32,500 a `TIER_1` reading produces. Any recalibration list assembled from the tier rather than read off [`thresholds.ts`](https://github.com/MetaMask/metamask-extension/blob/19d71034e6bb1adc7ecc16725ba4e5a5a882cfce/test/e2e/benchmarks/utils/thresholds.ts) will carry that error.

**The two adjustments above were replayed against the gate's own verdicts over these 116 runs.** Applying only the CV skip and CV-adaptive widening predicts 33 blocked runs, against an observed 27 failed `quality-gate` jobs. Adding [`applyNoiseTolerance`](https://github.com/MetaMask/metamask-extension/blob/19d71034e6bb1adc7ecc16725ba4e5a5a882cfce/development/metamaskbot-build-announce/comparison-utils.ts#L341-L375) reconciles it exactly at 27 — a positive control on the rule as this ticket describes it, and a measurement of its effect: 6 of 33 ceiling breaches were absorbed because the breach came in under that run's own stdDev.

`sendTransactions.openSendPageFromHome` supplies a live instance of the third outcome named above. Its CV exceeds 50 in **12 of its 116 runs**, so the metric is dropped from validation entirely in roughly one run in ten. At a p50 of 15 ms it sits below the ~50 ms floor that [MetaMask-planning#7281 suppress percentage-based severity below the noise floor](https://github.com/MetaMask/MetaMask-planning/issues/7281) covers, and it is gated regardless.

## Why this is not already covered

- [#45444 demote the three bimodal onboarding metrics](https://github.com/MetaMask/metamask-extension/pull/45444), merged 2026-08-28, demoted three onboarding metrics, but [`gated-metrics.ts`](https://github.com/MetaMask/metamask-extension/blob/0350a01878e0d5e0eb1553b18ca7d653b4157cbb/test/e2e/benchmarks/utils/gated-metrics.ts#L60-L63) deliberately **keeps** `onboardingImportWallet.total` gated — the comment reads that it "stays gated: a step follows its slow one". This ticket is not a request to reverse that call.
- [#45266 onboarding benchmarks are bimodal](https://github.com/MetaMask/metamask-extension/issues/45266), open, covers the *cause* in the flow — a ~7s slow path on a large minority of iterations.
- [#45046 benchmark `quality-gate` red on every `main` commit](https://github.com/MetaMask/metamask-extension/issues/45046), open, covers the gate being red on `main`.
- [#7480 harden absolute benchmark gate against within-noise breaches](https://github.com/MetaMask/MetaMask-planning/issues/7480), closed 2026-07-22, introduced the noise tolerance in item 2 above. It treats a within-noise breach as a reason not to fail; it does not address the same value landing on both sides across runs.

None covers the gating *decision* being non-deterministic at the boundary, which is what reaches unrelated PRs.

## Solution

Options, to be decided rather than assumed:

1. **Make the ceiling independent of the run's own variance** for gated metrics — a fixed ceiling, with CV and stdDev used to report confidence rather than to move the line. This means opting gated metrics out of both adjustments above, not just one.
2. **Require the violation to clear the overlap band** before it blocks, i.e. a dead zone rather than a point threshold.
3. **Demote `onboardingImportWallet.total` to warn** until #45266 closes. The allowlist holds twenty entries, so nineteen would remain, but the twelve timing metrics are measured above and eleven of them sit 1.4x to 147x under their ceilings, so in practice the gate would stop blocking — not just for the length of a quiet window, but until those ceilings are re-derived.

Option 3 is the fastest way to stop unrelated PRs being blocked, and the most costly in signal — it should be weighed against how long #45266 is expected to take, not chosen by default.

## Acceptance Criteria

- [ ] The same measured value cannot both pass and fail across runs at the same configured threshold
- [ ] The blocking decision is reproducible from the measurement and the configured threshold alone
- [ ] Every adjustment applied between the configured threshold and the blocking decision is printed on the line the developer reads, or removed
- [ ] Whichever option is taken, the gate's blocking set is stated explicitly rather than being "whatever is left"
- [ ] Re-measured over a full window after the fix, the passing and failing ranges are disjoint

## Dependencies

**Related:** [#45266 onboarding benchmarks are bimodal](https://github.com/MetaMask/metamask-extension/issues/45266) — open, the underlying cause. [#45544 CV-adaptive threshold widening switches off above `CV_ADAPTIVE_MAX`](https://github.com/MetaMask/metamask-extension/issues/45544) — open, adjustment 1 above. [#45934 gate output cannot tell a developer whether a red is about their change](https://github.com/MetaMask/metamask-extension/issues/45934) — open, the reporting half of the same undisclosed-adjustment problem. [#45046 benchmark `quality-gate` red on every `main` commit](https://github.com/MetaMask/metamask-extension/issues/45046) — open. [#45205 restore benchmark gate signal](https://github.com/MetaMask/metamask-extension/issues/45205) — open, and the place `onboardingImportWallet.total`'s ceiling is to be re-derived

Contributor guide

Open the contributing guide

Research direction

Start with getEffectiveThreshold and validateResultThresholds in test/e2e/benchmarks/utils/statistics.ts, then trace applyNoiseTolerance in development/metamaskbot-build-announce/comparison-utils.ts and the gated metric definitions. Reproduce the gate behavior against the documented full run window and verify the selected approach satisfies the acceptance criteria, including disjoint outcomes and reproducible blocking decisions.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
ci-cd, performance, testing
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.