Skip to content

test(studio): gate timeline viewport performance in Chromium - #2697

Open
miguel-heygen wants to merge 1 commit into
codex/studio-timeline-d-performance-contract-v2from
codex/studio-timeline-d-chromium-gate-v2
Open

test(studio): gate timeline viewport performance in Chromium#2697
miguel-heygen wants to merge 1 commit into
codex/studio-timeline-d-performance-contract-v2from
codex/studio-timeline-d-chromium-gate-v2

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

What

Add a reproducible Chromium runner that enforces the D1 timeline viewport contract.

How

  • Read budgets, fixture loading, and diagnostics only through window.__studioTest.
  • Run warmups plus measured samples and emit machine-readable evidence for responsiveness, DOM bounds, scroll safety, and heap return.
  • Require the Long Tasks API instead of silently treating unsupported browsers as zero long tasks.
  • Record Long Tasks through one collector, drain queued entries with takeRecords() in finally, then disconnect the observer.
  • Before evaluating the gate, inject a 75ms browser task in a disposable page and require the same collector to report it above the 50ms budget.
  • Run Chrome with GPU disabled for a stable DOM/CPU timeline measurement.
  • Enforce exactly one Timeline, the mounted-row ceiling, and inclusive clip/descendant ceilings.

The injected assertion is isolated from the timeline page, so it proves the observer contract without contaminating measured runs.

This PR provides the runnable gate. Permanent CI wiring for both the default and virtualized states is intentionally owned by #2884 later in this same Graphite stack.

Test plan

  • Harness syntax, formatting, and lint
  • Full Studio suite at the final stack tip: 291 files / 3,193 tests
  • Studio typecheck
  • Exact-tip local Chrome: injected 75ms task observed as 75ms
  • Exact-tip local Chrome: default state passed 5/5 measured runs
  • Exact-tip local Chrome: 50k virtualized state passed required 4/5 quorum on clean rerun
  • Exact-head hosted Chromium run (triggered by the updated CI stack)

Review fixes are included at head abb921f5f.

@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-performance-contract-v2 branch from 2216018 to fa190d8 Compare July 27, 2026 20:51
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-chromium-gate-v2 branch from 259b0f1 to bad5a54 Compare July 27, 2026 20:51
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-performance-contract-v2 branch from fa190d8 to d49c381 Compare July 28, 2026 20:42
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-chromium-gate-v2 branch from bad5a54 to ac1bbdd Compare July 28, 2026 20:42
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-performance-contract-v2 branch from d49c381 to 2540026 Compare July 28, 2026 22:09
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-chromium-gate-v2 branch from ac1bbdd to 3e11275 Compare July 28, 2026 22:09
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-performance-contract-v2 branch from 2540026 to a26435d Compare July 28, 2026 22:35
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-chromium-gate-v2 branch 2 times, most recently from 81e3a70 to dcbe193 Compare July 28, 2026 23:02
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-performance-contract-v2 branch from a26435d to 5d7f12d Compare July 28, 2026 23:03
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-chromium-gate-v2 branch from dcbe193 to f869766 Compare July 28, 2026 23:22
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-performance-contract-v2 branch 2 times, most recently from b8882aa to f411080 Compare July 28, 2026 23:53
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-chromium-gate-v2 branch from f869766 to 3467adf Compare July 28, 2026 23:53
@miguel-heygen
miguel-heygen changed the base branch from codex/studio-timeline-d-performance-contract-v2 to main July 29, 2026 02:06
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-chromium-gate-v2 branch 2 times, most recently from e1fd1f2 to 9eaec45 Compare July 29, 2026 14:46
@miguel-heygen
miguel-heygen changed the base branch from main to graphite-base/2697 July 29, 2026 14:46
@miguel-heygen
miguel-heygen changed the base branch from graphite-base/2697 to main July 29, 2026 14:49
@miguel-heygen
miguel-heygen changed the base branch from main to codex/studio-timeline-d-performance-contract-v2 July 29, 2026 15:03
@miguel-heygen
miguel-heygen marked this pull request as ready for review July 29, 2026 15:06
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-performance-contract-v2 branch from 4033f5f to e9360d1 Compare July 29, 2026 23:35

@vanceingalls vanceingalls left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE — grade B — rubric CORRECT

Role in stack: D2 gate — wires D1 contract into Chromium-run perf test.

Root-cause fit: Script drives Studio preview via puppeteer-core, reads budgets/fixture/diagnostics off window.__studioTest.* (all provisioned by D1 useStudioTestHooks.ts), and emits machine-readable evidence. Contract fit is clean — no local re-declaration of thresholds, no fixture reimplementation. Body's "reproducible gate" framing is accurate: opt-in pnpm test:timeline-virtualization, not CI-enforced.

Claims verified (file:line at HEAD c3b04be4):

  • Budgets come from D1 not local: timeline-virtualization.mjs:172budgets = await page.evaluate(() => window.__studioTest.timelineViewportBudgets).
  • Fixture reused, not reimplemented: :157,168window.__studioTest.loadTimelinePerformanceFixture({elementCount, profile}).
  • Warmup separated from measurement: :180-183if (index >= budgets.warmupRuns) runs.push(run).
  • Deterministic env: viewport 1440x900 at :143-148; DPR 2 only under high-dpr; CPU throttle 4x only under low-resource.
  • Precise heap via CDP: :63-66 Runtime.getHeapUsage + HeapProfiler.collectGarbage before both baseline (:161) and returned (:194).
  • Chrome major pin optional but honored: :134-140.
  • Longtask via PerformanceObserver('longtask') :106-116; asserted :187.

Adversarial findings:

  • P3 — no CI wiring; test:timeline-virtualization is added to packages/studio/package.json but not invoked by any workflow. "Gate" in the title reads aspirational; PR body honestly calls it a package script.
  • P3 — operator inconsistency at :189: mountedTimelineDescendants < budgets.maxMountedTimelineDescendants uses strict <, while sibling clip-root checks (:187-188) use <=. Likely intentional (spec says "less than N descendants") but worth a comment for future readers.
  • P3 — silent longtask fallback: if supportedEntryTypes omits longtask the observer is null and Math.max(0, ...[]) returns 0, trivially passing longTaskLimitMs. Chrome major pin mitigates but the pin is optional.
  • P3 — no --disable-gpu / compositor lockdown; frame-cadence numbers may pick up compositor variance, but D1 budgets appear sized with variance in mind (constrained tier + 4/5 quorum).

CI state: All required checks green. Mergeable=MERGEABLE.

Suggested next step: Land as D2; open a follow-up to schedule the script (nightly or weekly) once host baseline is characterized, and normalize the < vs <= operator + guard the longtask-unsupported branch to fail rather than pass.

Review by Via

@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-performance-contract-v2 branch from e9360d1 to 09b469a Compare July 30, 2026 00:23
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-chromium-gate-v2 branch from c3b04be to 287a998 Compare July 30, 2026 00:23
@miguel-heygen

Copy link
Copy Markdown
Collaborator Author

Addressed at 287a99814.

  • The harness enforces D1’s exact-one-Timeline and mounted-row contracts.
  • Descendant ceilings now use the same inclusive operator as the other DOM ceilings.
  • Unsupported Long Tasks API is a hard error rather than a silent zero.
  • Chrome runs with --disable-gpu.
  • The runner remains explicit in D2; permanent CI execution for both default and virtualized states is now implemented by ci(studio): run the timeline viewport gate on studio changes #2884 in the same stack.

@miguel-heygen

Copy link
Copy Markdown
Collaborator Author

One blocker in the performance-gate contract:\n\n- packages/studio/tests/e2e/timeline-virtualization.mjs:69-80 starts a PerformanceObserver, awaits the scroll measurements, and then immediately disconnects it. PerformanceObserver delivery is asynchronous; disconnecting without draining takeRecords() can discard entries queued by the final measured render task. That means longestTaskMs can remain below budget even when the last scroll produced an over-budget long task. Because this PR exists to make that measurement a gate, this is a false-green path, not just test polish. Please drain the pending records before disconnecting (ideally in a finally block) and add a focused browser assertion that an injected >50 ms task is observed before the gate evaluates.\n\nI verified the current-head improvements from the prior review: unsupported Long Tasks now fails closed, the DOM ceilings are inclusive, Timeline/root cardinality is pinned, and GPU is disabled. The remaining hosted exact-head checks are green/skipped as expected; Graphite mergeability is still pending because this is downstack.\n\nLocal note: node --check passes. I could not reuse the existing checkout dependencies for Vitest because its installed Studio dependency graph is missing wawoff2; this does not change the source-level blocker above.\n\nGitHub will not let the shared author identity submit a formal request-changes review, so recording the verdict as a PR comment.\n\nVerdict: REQUEST CHANGES\nReasoning: The Long Tasks budget can silently miss queued measurements and allow the new Chromium gate to pass falsely.\n\n— Magi

@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-chromium-gate-v2 branch from 287a998 to abb921f Compare July 30, 2026 01:31
@miguel-heygen

Copy link
Copy Markdown
Collaborator Author

Resolved at abb921f5f.

  • The observer callback and takeRecords() drain now feed one recordLongTasks owner.
  • Collection is wrapped in try/finally; pending entries are drained before disconnect(), including when scroll measurement throws.
  • A focused browser assertion uses the same collectRun path, injects a 75ms task, and fails before measured runs if Chrome does not report it above the 50ms budget.
  • The assertion runs in a disposable page. An initial implementation on the timeline page proved the task was observable but could contaminate later samples, so the final version isolates and closes the probe page before loading the measured fixture.

Exact-tip Chrome evidence:

  • Probe: injected 75ms, observed 75ms.
  • Default / 1,000 elements: 5/5 measured runs passed.
  • Virtualized / 50,000 elements: clean rerun passed the required 4/5 quorum; all DOM budgets passed in every sample.

The full Studio suite passes (291 files, 3,193 tests), Studio typecheck passes, and the harness passes format, lint, and node --check.

D3 and D4 were parent-restacked only. Their exact-delta patch IDs are unchanged from the approved revisions; the inherited D3 click-suppression rAF note remains deliberately outside this blocker fix.

@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-chromium-gate-v2 branch from abb921f to a870cf4 Compare July 30, 2026 03:15
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-performance-contract-v2 branch 2 times, most recently from a186ca4 to f158e6c Compare July 30, 2026 03:47
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-chromium-gate-v2 branch from a870cf4 to 0011368 Compare July 30, 2026 03:47
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-performance-contract-v2 branch from f158e6c to c458578 Compare July 30, 2026 04:05
@miguel-heygen
miguel-heygen force-pushed the codex/studio-timeline-d-chromium-gate-v2 branch from 0011368 to 4aaf015 Compare July 30, 2026 04:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants