MetaMask / MetaMask/metamask-extension
[P1] Benchmark step timers are not a disjoint partition of the flow they measure
- Dominant language
- TypeScript
- Stars
- 13.2k
- Forks
- 5.6k
- Avg merge
- 2d 5h
- Merged PRs (30d)
- 451
Description
**Parent Epic:** [#6944 Performance Quality Gates](https://github.com/MetaMask/MetaMask-planning/issues/6944) → [#7182 Benchmarks: Reliability](https://github.com/MetaMask/MetaMask-planning/issues/7182) → [#7181 Benchmark Harness Fixes](https://github.com/MetaMask/MetaMask-planning/issues/7181)
**File:** `test/e2e/benchmarks/flows/user-journey/onboarding-import-wallet.ts:184` (at `0350a018`)
**Size:** M | **Hours:** ~6-10
---
## Problem
The step timers are not a partition of the flow. `measure()` in [`timer-helper.ts:55-63`](https://github.com/MetaMask/metamask-extension/blob/0350a01878e0d5e0eb1553b18ca7d653b4157cbb/test/e2e/benchmarks/utils/timer-helper.ts#L55-L63) starts and stops an independent `performance.now()` pair per step, and nothing constrains those pairs to be adjacent — the flows do real work between them.
Two consequences:
1. **Named steps exclude their own trigger.** `onboarding-new-wallet.ts` calls `completeOnboarding()` and `handleSidepanelPostOnboarding(driver)` at lines 150–152, *before* starting the timer named `doneButtonToAssetList` at line 156. The Done-button click is outside a metric named for it. `onboarding-import-wallet.ts` has the identical shape around `doneButtonToHomeScreen` (`completeOnboarding()` at 172, `handleSidepanelPostOnboarding(driver)` at 173, timer opens at 177).
2. **Adjacent labels trade a shared wait.** `doneButtonToHomeScreen` and `openAccountMenuToAccountListLoaded` are two labels either side of a floating boundary — a single ~5.6s wait is billed to whichever step it lands in.
`total` is the sum of the step timers rather than the elapsed run — [`runner.ts:242-244`](https://github.com/MetaMask/metamask-extension/blob/0350a01878e0d5e0eb1553b18ca7d653b4157cbb/test/e2e/benchmarks/utils/runner.ts#L242-L244) reduces the per-run timer values into `runTotal` — so time in the gaps between steps is in no metric at all. How much time that is is what [#45462 fix step timers measuring non-disjoint adjacent spans](https://github.com/MetaMask/metamask-extension/pull/45462) (open) is written to make measurable.
`doneButtonToHomeScreen` was demoted out of `GATED_METRIC_VALUES` by [#45444 demote the three bimodal onboarding metrics out of the gate](https://github.com/MetaMask/metamask-extension/pull/45444) (merged 2026-08-28), so the gate no longer reads boundary placement as duration. The metric is still measured and still published, and it cannot be restored — its restore condition is [#45266 onboarding benchmarks are bimodal](https://github.com/MetaMask/metamask-extension/issues/45266) — until the boundaries are disjoint.
### Observation
Per-iteration values from two `chrome-webpack-userJourneyOnboardingImport` job logs (2026-08-05, mocked population):
| run | `doneButtonToHomeScreen` | `openAccountMenuToAccountListLoaded` | sum |
|---|---|---|---|
| [31026037892](https://github.com/MetaMask/metamask-extension/actions/runs/31026037892) | 3.38, 3.48, 3.34, **9.72**, 3.19 | 7.87, 7.51, 7.88, **1.05**, 8.15 | 11.25, 10.99, 11.22, 10.77, 11.34 |
| [31042229025](https://github.com/MetaMask/metamask-extension/actions/runs/31042229025) | 3.35, 3.42, **8.94**, 3.24, **8.77** | 6.82, 6.87, **1.05**, 6.85, **1.04** | 10.17, 10.29, 9.99, 10.09, 9.81 |
Whenever one step is slow the other collapses to ~1.05s, and **the sum is flat** (sd ≈ 0.2s).
Across 30 sampled runs, within-run standard deviation:
| metric | median within-run sd |
|---|---:|
| `doneButtonToHomeScreen` | 2662ms |
| `openAccountMenuToAccountListLoaded` | 2674ms |
| `total` | **120ms** (< 400ms in 30/30 runs) |
Near-identical spread in the two components against a stable container is the signature of two variables trading a fixed quantity. Median ratio `sd(doneButton)/sd(total)` = 12.4×.
A second, related defect follows from the same cause: in **16 of 30** runs the two components **sum to more than `total`** (e.g. 8625 + 6519 = 15144 against a total of 10292). The gated steps are not a partition of the flow, so reasoning about `total` as the sum of its steps — which the threshold table implicitly does by carrying ceilings for both — is not valid.
### Mechanism
[`onboarding-import-wallet.ts:184`](https://github.com/MetaMask/metamask-extension/blob/0350a01878e0d5e0eb1553b18ca7d653b4157cbb/test/e2e/benchmarks/flows/user-journey/onboarding-import-wallet.ts#L184) closes `doneButtonToHomeScreen` on `waitForTokenToBeDisplayed('Solana', 120000)`, then `openAccountMenu()` opens the next step at line 191. When the token is already present, the wait returns immediately and the still-running work blocks the *following* step instead. Same work, different label. [`onboarding-new-wallet.ts:163`](https://github.com/MetaMask/metamask-extension/blob/0350a01878e0d5e0eb1553b18ca7d653b4157cbb/test/e2e/benchmarks/flows/user-journey/onboarding-new-wallet.ts#L163) is the same call with a 60000ms timeout, closing `doneButtonToAssetList`.
Distinct from — but sharing a root with — [#45266 onboarding benchmarks are bimodal](https://github.com/MetaMask/metamask-extension/issues/45266), which covers making that wait deterministic. This ticket is about the step boundaries themselves: even with a deterministic wait, adjacent timers that can absorb each other's work are not measuring what their names claim.
---
## Solution
1. **Make boundaries deterministic.** A step should open and close on events that cannot reorder relative to each other, not on a wait whose completion races the boundary.
2. **Then choose per metric:** merge the two steps into one gated span, or keep them separate with boundaries that guarantee disjointness.
3. **Give the flow a `total` that can disagree with its steps, then assert against it.** The assertion as originally written here cannot fire: [`performance-tracker.ts`](https://github.com/MetaMask/metamask-extension/blob/0350a01878e0d5e0eb1553b18ca7d653b4157cbb/test/e2e/benchmarks/utils/performance-tracker.ts#L63-L80) computes `metrics.total` by summing exactly the durations it pushes into `metrics.steps`, so the steps sum to `total` identically, always. There is no flow-level span to compare them against, which is why no coverage gap has ever been detectable. Measure the whole flow as its own span first, then `total` minus the sum of steps becomes a real number: the work the harness does not measure. Assert on that residual, and publish it rather than discarding it.
4. **Fail on a step that records nothing.** The same loop filters on `duration > 0`, so a step that produced no duration is dropped from both the step list and the sum without trace. A broken timer currently makes `total` smaller.
---
## Acceptance Criteria
- [ ] The onboarding flows' step timers partition the span they cover, or the unaccounted remainder is reported
- [ ] A step named for a UI action includes that action
- [ ] Sum of a flow's measured steps never exceeds its `total`; violation fails the run
- [ ] `doneButtonToHomeScreen` within-run sd falls to the order of `total`'s (~120ms)
- [ ] The same audit is applied to the other gated step timers, which have not been checked for this defect
- [x] `doneButtonToHomeScreen` out of `GATED_METRIC_VALUES` until the above lands — [#45444 demote the three bimodal metrics](https://github.com/MetaMask/metamask-extension/pull/45444), merged 2026-08-28
---
## Labels
`team-extension-platform`, `area-testSuite`
---
## Dependencies
**Related:** [#7202 deterministic render-complete waits for account menu and swap](https://github.com/MetaMask/MetaMask-planning/issues/7202) (the account menu is one half of this boundary), [#45266 onboarding benchmarks are bimodal](https://github.com/MetaMask/metamask-extension/issues/45266), [#45205 restore benchmark gate signal](https://github.com/MetaMask/metamask-extension/issues/45205), [#45046 benchmark `quality-gate` red on every `main` commit since 2026-07-17](https://github.com/MetaMask/metamask-extension/issues/45046), [#45462 fix step timers measuring non-disjoint adjacent spans](https://github.com/MetaMask/metamask-extension/pull/45462)
Contributor guide
Research direction
Start with test/e2e/benchmarks/flows/user-journey/onboarding-import-wallet.ts and onboarding-new-wallet.ts, then read utils/timer-helper.ts, runner.ts, and performance-tracker.ts. Trace the timer boundaries and flow total before running the onboarding benchmark checks. Done means the action-inclusive steps are disjoint or the residual is reported, invalid sums fail, and the other gated timers are audited.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- performance, testing-qa
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100