Skip to content

test(studio): serve the edit bench GSAP from the repo, never the network - #4968

Merged
miguel-heygen merged 3 commits into
mainfrom
test/studio-bench-gsap-offline
Oct 3, 2026
Merged

miguel-heygen merged 3 commits into
mainfrom
test/studio-bench-gsap-offline

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Oct 3, 2026 •

Copy link
Copy Markdown
Collaborator

What

The edit-accuracy bench loaded GSAP from jsdelivr in every case. When that request was slow or failed in CI, the preview logged gsap is not defined, the case's timeline never registered, and the case failed for a reason that had nothing to do with Studio. That is the root of the intermittent nudge-fromto-px-r0-root-z100-mid failure (for example run 37120535633, shard 2).

Fixtures still contain the CDN <script> tag a user would write. The bench now intercepts requests to the CDN host in each Studio page and answers any file of the installed gsap version from the repo's own gsap/dist: today the fixture's gsap.min.js, plus any plugin the preview server pins to the same version. Any other request to that host is blocked and its URL logged once (edit bench: blocked <url>), so a new remote dependency is named instead of silently reaching the network. Today that log names one URL: Studio's own MotionPathPlugin, pinned at 3.12.5 in gsapSoftReload.ts. Without it, motionPath: tweens (which no fixture or bench edit writes) cannot play, and Studio hides its "Set motion destination" toolbar button. Hiding that button can only remove overlap with a drag, and this PR's full-grid gate passed against main's baseline.

  • grid.mjs: the fixture URL takes its version from the installed gsap package, so the URL and the bytes served always match. localAsset(url) maps a CDN URL under that version's dist/ to the repo file, when it exists.
  • case.mjs: inStudio enables the DevTools Fetch domain for the CDN host and fulfils matching requests from localAsset. A request whose frame already went away is ignored rather than ending the run.
  • grid.test.mjs (new): writes every fixture in the full grid and fails on any http(s) URL the bench does not serve from the repo, including a mapped URL on a host the bench does not intercept.

The fixture GSAP moves from 3.14.2 to 3.15.0, the only version installed in the repo (the same version the producer uses). This PR's edit-accuracy CI runs the full grid against main's baseline, so that run is the before/after for the version change.

Follow-up, not in this PR

The bench's render checks run in the producer's own browser, which still loads the fixture's CDN URL from the network (the producer also points a missing local gsap at the CDN: GSAP_CDN_BASE in packages/producer/src/services/htmlCompiler.ts). A network failure can still fail render there. That needs its own fix.

No visible change

Test-only.

Test plan

  • grid.test.mjs passes 3 runs in a row. It fails, listing the URL, when the local map does not cover the fixture URL.
  • Proof the preview gets GSAP from the repo: with the fixture URL pointed at a GSAP version jsdelivr does not have (404), four cases ran with the same drag, drop and undo verdicts as with the real URL. Only render changed, which is the producer path above.
  • After the review changes, three cases on one devbox: only the 3.12.5 plugin is logged as blocked, and verdicts match main's baseline.
  • With the real URL, the same four cases (resize, keyframed move, nudge on a from/to tween, nested move) ran 3 times on one devbox, each time matching main's baseline verdict.
  • oxfmt and oxlint clean.
  • This PR's edit-accuracy CI run (full grid against main's baseline) passed the gate.

Kept as its own PR under the size floor: it is the bench's network root, test-only and off main, and no open PR carries bench harness changes.

@github-actions

github-actions Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Edit accuracy: accurate 1301 (base branch 1301), smooth 1114 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (0)

Unstable (2)

  • crop-none-pct-r0-nested-z50: tracking 0.05, pressJump 0, drop 40.07, reload 40.07, render 40.02, renderKey -, undo true, teleport true / tracking 0.05, pressJump 0, drop 0.07, reload 0.07, render 0.02, renderKey -, undo true, teleport true / tracking 0.05, pressJump 0, drop 0.07, reload 0.07, render 0.02, renderKey -, undo true, teleport true
  • crop-none-px-r30-nested-z200: tracking 0.02, pressJump 0, drop 40.03, reload 40.05, render 34.78, renderKey -, undo true, teleport true / tracking 0.02, pressJump 0, drop 0.04, reload 0.05, render 0.3, renderKey -, undo true, teleport true / tracking 0.02, pressJump 0, drop 0.04, reload 0.05, render 0.3, renderKey -, undo true, teleport true

@miguel-heygen
miguel-heygen marked this pull request as ready for review October 3, 2026 21:32

@somanshreddy somanshreddy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed at 56c43366 (test-only, 3 files).

No blockers from me. The interception does what the PR says for the Studio page. The PR body already discloses the version bump and the producer render path. My main ask is about what the new unit test actually pins. One correction to the body's MotionPathPlugin note is below. This is a comment, not an approval.

1. grid.test.mjs pins the fixture/map agreement, not the interception (low, test gap). I mutated the code and re-ran grid.test.mjs each time (vitest, NODE_ENV=test):

  • dropped await serveFixtureAssetsLocally(page) in case.mjs:810: survived
  • reverted case.mjs to base: survived
  • changed the blocked branch to Fetch.continueRequest (a silent network fallback): survived
  • pointed the Fetch.enable urlPattern (case.mjs:800) at another host: survived
  • dropped the isFile() check in localAsset (grid.mjs:25) with the fixture pointed at a missing dist file: survived
  • put back the old gsap@3.14.2 literal in GSAP_CDN: caught (lists the 3.14.2 URL)
  • control, fixture URL on unpkg: caught

So if someone later removes the interception, every check stays green. The bench just goes back to the network and still passes while jsdelivr is up. One cheap way to pin "never the network" in CI is to start the bench Chrome with a dead proxy in run.mjs (--proxy-server=http://127.0.0.1:9; Chrome sends loopback direct by default, so the Studio server still works). I tried that on two cases, nudge-tween-px-r0-root-z100 and nudge-tween-px-r0-nested-z50, with a local CLI build. At this head both were accurate on every metric. With the serveFixtureAssetsLocally call removed and the proxy still on, neither was accurate (render 0/2, reload 1/2, undo 1/2). I didn't try a narrower --host-resolver-rules variant.

2. render still reaches the network, so the title overclaims (low, already disclosed). I confirmed the follow-up you describe. With https_proxy set to a dead proxy, which reaches the producer's own Chrome, render failed on both cases (worst 25.02) and every other metric passed. Without it, render passed on both. render.mjs loads the fixture through createFileServer + createCaptureSession, and nothing intercepts there. "never the network" in the title is only true for the Studio page.

3. The body's MotionPathPlugin note looks inaccurate, in the harmless direction (info). The 3.12.5 request from ensureMotionPathPluginLoaded (gsapSoftReload.ts:30-31,69) does get blocked. But studio-server's injected GSAP_CDN_FALLBACK_SCRIPT (studio-server/src/routes/preview.ts:199-218) catches the script error and reloads the same file from gsap@3.15.0, and the bench then serves that version locally. I checked with a temporary log line in the Fetch.requestPaused handler on one case. The run logged .../gsap@3.15.0/dist/MotionPathPlugin.min.js as served twice and the 3.12.5 URL as blocked once. So the plugin file does load in the bench. I didn't check that it registers, or whether the "Set motion destination" button shows. You may want to fix the "Without it… Studio hides its button" sentence. Nothing in the code needs to change.

4. Nits in serveFixtureAssetsLocally / inStudio (low).

  • Both Fetch.fulfillRequest and Fetch.failRequest end in .catch(() => undefined) (case.mjs:786-797), but the comment only justifies requests whose frame went away. Any other CDP failure leaves the request paused. The case then shows up as a Studio readiness timeout, not an interception error. Consider ignoring only the closed-target error and logging the rest.
  • serveFixtureAssetsLocally(page) is awaited before the try/finally (case.mjs:810 vs the try at :824). If createCDPSession or Fetch.enable rejects, the context never closes, even though the doc comment says "the context always closes". newPage() was already outside the try on base; this adds two more awaits there. Moving the try up to just after createBrowserContext would cover all of them.

Things I checked that hold up:

  • Version: the old URL pinned gsap@3.14.2. The new one comes from require("gsap/package.json").version, and bun.lock has only gsap@3.15.0 (studio declares ^3.13.0). So the bench moves 3.14.2 → 3.15.0, as the body says. That matches studio-server's GSAP_CDN_VERSION and the producer's GSAP_CDN_BASE (htmlCompiler.ts:1887). The fixture URL and the bytes served can't drift apart, because both come from the installed package.
  • Path resolution: createRequire(import.meta.url) doesn't depend on the working directory. gsap is a direct studio dependency, linked at packages/studio/node_modules/gsap under bun's isolated layout, and it's a plain npm package with no build step. Test (studio) is green at this head, and the edit-accuracy shards were green at bbd6a16c7 and 4a46f7943 with the same resolution code.
  • Served files: other plugins of the installed version resolve too (ScrollTrigger.min.js, MotionPathPlugin.min.js). Query strings, the bare dist/ path and gsap.min.js/x return undefined without throwing, so those requests get blocked.
  • Headers: text/javascript with no cache headers is fine. Each case gets a fresh browser context, and a classic <script> with no crossorigin needs no CORS header. .map files would also go out as text/javascript, which is harmless.
  • No silent fallback to the network: an unmapped jsdelivr URL gets Fetch.failRequest plus a one-time warning.

CI at 56c43366: 78 passed, 1 skipped, 35 still running when I checked. The running jobs are the 20 Studio: edit accuracy shards, 10 producer regression-shards, and the Windows render job and 4 Windows test jobs. The edit-accuracy gate hasn't reported at this head yet. The "gate passed" claim and the sticky comment (accurate 1301 vs base 1301) are from earlier heads; the gate passed at 4a46f7943 and at bbd6a16c7. The only change since 4a46f7943 is existsSync → statSync(...).isFile(). There were no other reviews on the PR.

What I ran:

  • grid.test.mjs at head (1/1 pass) plus the 7 mutations above.
  • The bench on two nudge cases, three ways: head with a dead Chrome proxy, the call-removed control with the same proxy, and head with a dead https_proxy for the producer.
  • One more bench case with a temporary log line to list the URLs it served and blocked.
  • Direct localAsset calls with edge-case URLs.
  • A static Codex pass. It found the test-coverage gap independently, plus the two item-4 nits and the MotionPath fallback in item 3. I confirmed all of these at source before including them.

I reverted all local edits.

— Somu

@jrusso1020 jrusso1020 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.

Approve. This adds to the existing review. I'm not repeating its points: interception isn't pinned by a test, setup runs before the try/finally, CDP errors are swallowed, and render still goes to the network.

Interception covers the preview iframe. The Fetch domain on the page session sees the iframe's requests. Shard 2's CI log names the 3.12.5 MotionPathPlugin, which Studio adds inside the iframe document. Locally I ran nudge-fromto-px-r0-root-z100-mid and move-fromto-px-r0-nested-z100-mid with the edit-accuracy-cli artifact built at this head, both with the network and with cdn.jsdelivr.net made unresolvable in the bench Chrome. Both runs gave the same verdicts, and only the 3.12.5 plugin was logged as blocked. Every paused request gets either a fulfillRequest or a failRequest. Serving the wrong Content-Type turns the case into ERROR, so the response type matters and is right.

Nits (none blocking):

  • localAsset doesn't confine the path to dist/ (grid.mjs:22-26). join() runs on the raw URL text, so …/dist/../package.json maps to gsap's package.json. A file name over 255 bytes throws ENAMETOOLONG; inside requestPaused that would leave the request paused. Chrome normalizes .. and %2e%2e before the request, so the bench can't reach this, but grid.test.mjs checks the raw fixture URLs. Normalizing with new URL(url).href, or checking the resolved path stays under dist/, would close it.

  • grid.test.mjs doesn't check localAsset itself. Three changes leave it green:

    • dropping the isFile() guard;
    • mapping a 3.14.2 URL onto the 3.15.0 bytes;
    • a fixture that loses its CDN tag, after which the scan passes with zero URLs.

    Asserting urls.length > 0, plus a directory case and a wrong-version case, would pin all three.

  • The plugin fallback works by coincidence of versions. The preview server pins GSAP_CDN_VERSION = "3.15.0" (studio-server/src/routes/preview.ts:66). The bench follows whatever gsap Studio resolves (^3.13.0). A lockfile bump that moves Studio off 3.15.0 would start blocking that fallback. It would be logged rather than silent, and bun.lock triggers the bench, so it would show up.

Checks:

  • grid.test.mjs passes 1/1 in three local runs. It scans 1562 specs and 2334 pages and finds one distinct URL. It also runs in CI under Test (studio).
  • baseline.json has no gsap URL, so the version change doesn't change any case keys.
  • All checks are green at this head, including the edit-accuracy gate.

— Rames

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 3, 2026
Merged via the queue into main with commit 2f14008 Oct 3, 2026
170 checks passed
@miguel-heygen
miguel-heygen deleted the test/studio-bench-gsap-offline branch October 3, 2026 22:36
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.

3 participants