Skip to content

ci(webv2): judge byte budgets as a delta against the base branch - #253

Merged
lstein merged 3 commits into
mainfrom
feat/webv2-perf-delta-gating
Sep 14, 2026
Merged

ci(webv2): judge byte budgets as a delta against the base branch#253
lstein merged 3 commits into
mainfrom
feat/webv2-perf-delta-gating

Conversation

@lstein

@lstein lstein commented Sep 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

A pull request's webv2 byte budgets were checked against ceilings committed in performance/*-baseline.json — pinned at the captured measurement for ownedRawBytes and 1% above it for the rest. Because those are absolute totals against a snapshot, a PR failed for growth main had already accepted since the last capture, and a PR left open while main moved failed again for the same unchanged diff. One 64-byte change forced five baseline re-records in a day across three different metrics, each surfacing only after the previous was raised.

Now a pull request into main (and a merge-group run) is judged as a delta against the base branch's own measurements. Every push to main writes two small, versioned reference files and stores them in the Actions cache under its SHA; a PR run restores the entry for its base commit — or, if that commit has none (a main job that failed, is still running, or was a merge-queue batch), the most recent main entry — and the gate derives each ceiling from that. The gate logs which reference it used, or that it fell back to the committed baseline and why.

Each ceiling is the lower of two bounds: an allowance of max(1%, 4 KB) over the baseline being compared against, and a hard ceiling of max(10%, 32 KB) over the committed baseline. The allowance is what lets a PR answer only for its own bytes. The hard ceiling is what keeps the committed file the outer bound a person has reviewed: without it, allowance-sized merges could compound indefinitely with the committed numbers having no effect before a merge, and a deliberate downward re-record would not bite until main next pushed. Request counts carry no allowance under either: their limit is the lower of the two counts.

What is deliberately unchanged: sourceOwners and every structural rule still come from the committed baseline and are pinned exactly — a module entering a route's initial graph fails at zero bytes. That check is what catches architectural regressions; the byte budgets only need to catch gross size changes. Timing enforcement stays off.

Pushes to main compare against the committed files. A main that has outgrown the allowance since its last capture therefore fails rather than having its ceiling ratchet up unreviewed — the failure text says so and names the re-record commands (pnpm test:performance:build:update-baseline is new; the browser one existed). That remedy is appended only when a byte or request limit failed — an owner-graph leak or importer-count failure gets no advice to re-record, since re-recording would erase exactly that signal. That keeps .github/AGENTS.md's "do not silently relax budgets" rule intact.

Two consequences of deriving ceilings instead of committing them: the limits blocks disappear from both baseline files (only measurements are recorded, and the duplicated-number merge conflicts that came with every re-record go with them), and the dead getLegacyOwnedLimit zero-headroom fallback is removed along with both limit-derivation helpers. ARCHITECTURE.md documents the derivation, the reference, and the recovery commands.

QA Instructions

All from invokeai/frontend/webv2 on the final candidate; each gate run alone, since the browser stage binds a preview server and a mock backend and two gates race on dist/ and the report files regardless of ports (INVOKEAI_PERFORMANCE_PORT / INVOKEAI_PERFORMANCE_BACKEND_PORT move them).

  • pnpm run test:performance:contracts — pass. Covers the allowance, the hard ceiling over the committed baseline (including min on request counts), the browser gate driven through checkBrowserRouteBudget (within/beyond allowance, exact request counts, reference by id:profile with both origins named, the activatedResources NaN guard), reference-artifact shape, version and metric-set incompatibility, named errors for a non-reference file, and rejection of the removed limits keys, and the loader's missing/invalid-JSON/no-version wiring faults (42 cases).
  • pnpm lint — pass (format, oxlint --deny-warnings, tsc, architecture policy).
  • INVOKEAI_PERFORMANCE_PORT=4276 INVOKEAI_PERFORMANCE_BACKEND_PORT=4277 pnpm run test:performance:architecture — no reference — pass, exit 0. Logs Byte budgets: against committed baseline captured 2026-09-11. and Resource budgets: against committed baseline captured 2026-09-07.; both reports record reference: null; the run wrote build-reference.json (1,089 B) and browser-reference.json (7,053 B — the cached payload was 1.24 MB before).
  • The same with WEBV2_PERF_REFERENCE_DIR=artifacts/perf-reference WEBV2_PERF_REFERENCE_LABEL=local-self, the directory holding this tree's own two reference files (a self-comparison, which exercises the delta path end to end in both gates) — pass, exit 0; logged Byte budgets: delta against base-branch reference local-self captured 2026-09-13T19:42:11.057Z. and Resource budgets: delta against base-branch reference local-self captured 2026-09-13T19:43:45.399Z.; build-report.json records reference: {applied: true, capturedAt, label: "local-self", reason: null, uncoveredRoutes: []} and browser-report.json records the same shape
  • Fail-loud check: WEBV2_PERF_REFERENCE_DIR set to a directory lacking the files throws naming the variable and path (loadPerformanceReference), and a reference route missing a metric throws before any NaN ceiling can be derived (contract-tested on both gates).

Not run: actionlint is not installed here; the workflow YAML parses and was reviewed by five independent read-only reviewers. The cache save/restore steps can only be exercised in Actions — the first push to main after merge bootstraps the reference, and until then PRs restore nothing and compare against the committed baseline, which the log and a ::warning:: annotation state.

Review

Three independent read-only reviewers (correctness/spec; architecture, operational safety, performance; test value and gaps) reviewed the first candidate. Material findings, all resolved: budgets were advisory before merge (a PR could grow past the committed file because only main was compared against it — hence the hard ceiling over the committed baseline); an artifact-schema change in a PR would have wedged CI on the base's cached reference (hence schemaVersion + metric-key list with a logged fallback); a main job that failed early left a sticky cache hole and merge-queue batches never matched (hence restore-keys on the prefix); the reference tests passed vacuously (routes and decoys added, the browser gate driven through checkBrowserRouteBudget, source-owner pinning asserted under a reference); the re-record remedy was appended to owner-graph failures (hence isBudgetFailure); reports did not say which reference was used (hence label, reason, uncoveredRoutes).

A blocker-only re-review of the resulting candidate found none and confirmed each finding resolved; six of its minors were fixed (version judged before shape, browser report records uncoveredRoutes, remedy scoping, wording, stage/warn only when the gate ran, merge groups scoped to main) and a third blocker-only review of those found none. One minor from that round was then taken — a reference file with no integer schemaVersion is a wiring fault and throws, rather than being reported as drift — covered by a contract test; that two-line change was verified by the contract suite and pnpm lint and not by re-running the full gates, whose path (a version-1 file) it does not alter. Remaining accepted minors: an unknown per-route key in a version-1 reference throws (only reachable by hand-editing), a malformed committed baseline throws before the report is written, and --update-baseline with the reference variable set records applied: false with no reason (local-only mode).

Compatibility / Rollout

  • Baseline schema: limits, resourceLimits and activatedResourceLimits are no longer valid keys; the validators reject them (tested), so a stale re-record cannot reintroduce the old shape. --update-baseline on either gate writes the new shape and does not read a reference.
  • Reference files (build-reference.json, browser-reference.json) carry schemaVersion and their metric key list. A PR that changes the metric set, or a revert, reads a reference recorded by a different checkout: that is detected and falls back to the committed baseline with the reason logged, never a hard failure. A set WEBV2_PERF_REFERENCE_DIR whose file is missing or malformed does fail, naming the variable and path — a wiring fault must not pass as "no reference".
  • Cache bootstrap: the cache is empty until the first push to main after this merges; that push touches webv2, runs the gate, and records itself. Until then PRs fall back (logged, annotated). A main job that ran the gate but failed before its browser stage records nothing for its SHA and says so in a ::warning::; one that left webv2 untouched records nothing silently. Later PRs restore the most recent main entry either way, so holes do not propagate.
  • Merge queue: later entries in a batch have a base that is never a main push; they restore the most recent main entry. Merge groups targeting any other branch receive no reference.
  • Stacked PRs (base branch other than main) get no reference and compare against the committed baseline — a documented limit, not addressed here.
  • Retention: unused cache entries expire after ~7 days; a miss is a logged, annotated fallback. Re-running a main job hits an existing key; actions/cache/save warns and skips.
  • No persisted application data, API, or generated artifact is affected.

Checklist

  • The PR has a short but descriptive title, suitable for a changelog
  • Meaningful regression coverage added / updated where needed; obsolete tests/code removed
  • Persisted-state and API changes include required migrations / compatibility validation (baseline schema: old limits keys rejected by the validators; no application data)
  • Relevant performance/efficiency opportunities considered; material claims have evidence
  • Material review findings resolved and relevant checks rerun
  • Documentation added / updated (if applicable)
  • Updated What's New copy (if doing a release after this PR)

- Ceilings are derived at check time: the lower of max(1%, 4 KB) over the
  base-branch reference and max(10%, 32 KB) over the committed baseline;
  request counts take the lower count. `limits` blocks leave both baselines.
- Every push to main caches versioned build/browser reference files by SHA;
  PRs into main restore their base's entry (or the latest main) and fall
  back to the committed baseline, logged and annotated, when none applies.
- The re-record remedy is appended only for byte/request failures; source
  owners stay pinned exactly. ARCHITECTURE.md documents the derivation.
…-gating

# Conflicts:
#	invokeai/frontend/webv2/performance/architecture-baseline.json
#	invokeai/frontend/webv2/performance/browser-baseline.json
…-gating

# Conflicts:
#	invokeai/frontend/webv2/performance/architecture-baseline.json
#	invokeai/frontend/webv2/performance/browser-baseline.json
@lstein
lstein merged commit 8c85405 into main Sep 14, 2026
20 of 21 checks passed
@lstein
lstein deleted the feat/webv2-perf-delta-gating branch September 14, 2026 02:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant