Skip to content

Two shots one picture: count loads per URL, wait for the physics worker and the second take - #187

Open
abernier wants to merge 2 commits into
mainfrom
e2e-inflight-per-url
Open

Two shots one picture: count loads per URL, wait for the physics worker and the second take#187
abernier wants to merge 2 commits into
mainfrom
e2e-inflight-per-url

Conversation

@abernier

@abernier abernier commented Aug 13, 2026

Copy link
Copy Markdown
Member

Two harness fixes, one per commit -- both found by this stack's own CI, both invisible to a local sweep.

Count what loads per URL, and never below zero

waitForDecodes -- the wait that keeps a shot from going off mid-decode -- asks the page for one number: how many items DefaultLoadingManager still has in flight. deterministic.js keeps that number by counting itemStart up and itemEnd down, which trusts every loader to call both.

One of them does not. postprocessing 6.39's LUTCubeLoader.load() calls externalManager.itemEnd(url) on a URL it never called itemStart for:

// postprocessing 6.39.4, src/loaders/LUTCubeLoader.js
load(url, onLoad, onProgress, onError = null) {
  const externalManager = this.manager;      // <- no itemStart(url)
  ...
    const result = this.parse(data);
    externalManager.itemEnd(url);

6.36.6 -- the version on main today -- called both, so this changes no behaviour here. It matters on #166, which moves postprocessing to 6.39.4 (6.37+ requires three >= 0.174), and it is worth landing on its own: the harness should hold this invariant itself rather than borrow a loader's.

A single counter sits at real - 1 for the rest of the page's life, and that is wrong in both directions: it reads 0 while one real item is still in flight (the shot fires mid-decode -- two of the four .cube examples did this silently) and -1 the rest of the time (a wait that cannot end -- glass-flower and nextjs-prism each held the full 300s budget and failed). So: count per URL, ignore an itemEnd for a URL nobody started.

On the #166 branch, the four .cube examples the sweep covers, before -> after:

example before after
nextjs-prism 300s timeout 1 passed (8.4s)
glass-flower 300s timeout 1 passed (35.1s)
color-grading passed, shot early 1 passed (9.4s)
instanced-particles-effects passed, shot early 1 passed (28.1s)

Hold each frame for the physics worker, and the shot for the second take

Chromatic flagged basic-ballpit against a baseline nobody touched, twice in a row, while e2e-flaky swore it was stable locally. Both were right -- two mechanisms, both measured under CPU throttle:

1. cannon's steps are a ping-pong the pump never waited for. @react-three/cannon posts step and transfers the position buffers to its worker; until the worker's frame reply hands them back, every further step is silently skipped (byteLength === 0) and its 1/60th of simulation is dropped, not deferred. The picture is spawn + (completed round trips) x 1/60 -- and how many round trips fit into thirty pumped frames is the scheduler's call. Measured on basic-ballpit: 30/30 round trips at full speed, 28 under a x8 throttle, each side perfectly reproducible. Two stable pictures, chosen by machine speed; Chromatic's runner sits in between and picks per build.

2. the second take can finish mounting inside the shot. The remount's flushSync returns when the DOM side has committed -- but the scene lives behind the <Canvas> bridge in r3f's own root, whose render is scheduled, not flushed. Under throttle, trails was measured with both flushSyncs long returned and the second take still assembling itself at frames 1-2 of the pump: physics worker created, connected and populated across running frames, with the first take's worker, not yet unmounted, stepping in the meantime -- and a step posted to a worker that gets terminated the next frame is a reply that never comes.

The fix, in two halves:

  • deterministic.js counts the exchange -- step out, frame back, per worker -- and the pump holds the next frame until the count settles: every machine completes exactly one step per frame. terminate() forgives what a dying worker still owes, and a step posted to an already-terminated worker is never counted.
  • CheesyCanvas gains a Probe, last in the keyed fragment: its effect runs when the take has finished mounting -- render committed, every sibling's effects run. shoot.mjs waits for it after the remount, so the pump starts against a fully assembled scene.

Verification

Every cannon example, two runs at full speed and two under x8 CPU throttle, canvas hashed byte-for-byte through the same shoot() the test uses -- one hash per example, 30/30 round trips, across every rate and run: basic-ballpit (also held at x4 and x20; hash unchanged from before the fix, so CI converges back to the picture already measured), racing-game (29 posted / 28 answered before), trails (three distinct pictures before; 8/8 runs after), pmndrs-vercel, ragdoll-physics, pinball-in-70-lines, trigger-meshes, simple-physics-example, simple-physics-example-with-debug-bounds, physics-with-convex-polyhedrons.

Still true and worth writing down: object-clump moves under x8 -- a different, pre-existing channel (its seeded initial positions shift with load timing), stable at full speed and on every CI build to date. arkanoid is in EXCEPTIONS (throws on mount) and stays there. Non-cannon controls clones, springy-boxes, video-cookies: unchanged and stable; real pnpm test green on basic-ballpit and trails.

馃 Generated with Claude Code

abernier added a commit that referenced this pull request Aug 13, 2026
* Report the shot at the commit that has a branch

Chromatic posts `UI Tests` and `UI Review` on whatever commit the CLI is
given, and on a `pull_request` event that is `GITHUB_SHA` -- the merge
commit GitHub builds for the run, which exists on no branch.

The pull request displays them anyway, so nothing looked wrong. But a
required status check is read off the head commit, and on #187 the two sets
never met: twelve Actions check runs on `5520f5f2`, three Chromatic statuses
on the merge commit `aefcbe4`. Adding `UI Tests` to the ruleset today would
have waited for a status that never lands there, on every pull request,
forever.

So hand Chromatic the head commit, plus the branch and slug that have to
travel with it or the build attaches to the wrong ref. All three are empty
on a push, where `GITHUB_SHA` is already the commit itself.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Take the documented values, fallbacks and all

`github.repository` rather than the head repo's slug, and a fallback on each
so a push to main keeps reporting where it always did. This is the snippet
Chromatic's GitHub Actions guide gives for `pull_request` workflows, which is
what this one is.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
`waitForDecodes` asks the page for one number: how many items the default
loading manager still has in flight. That number is kept by counting
`itemStart` up and `itemEnd` down -- which trusts every loader to call both,
and one of them does not. `postprocessing` 6.39's `LUTCubeLoader.load()`
calls `itemEnd` on a URL it never called `itemStart` for; 6.36, the version
on this branch today, called both.

A single counter cannot survive that. It reads 0 at whatever moment exactly
one real item is in flight -- the shot going off in the middle of a decode,
which is the whole thing this wait exists to prevent -- and -1 the rest of
the time, which is a wait that cannot end. Measured on the three-0.181
branch, where 6.39 arrives: `glass-flower` and `nextjs-prism` each sat on
the full 300s budget and failed, and the four other `.cube` examples shot
early without saying anything.

So count per URL and ignore an `itemEnd` for a URL nobody started. No
behaviour change here -- 6.36 is balanced -- this is the harness holding its
own invariant rather than borrowing a loader's.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Chromatic flagged basic-ballpit against a baseline nobody touched, twice,
while e2e-flaky swore it was stable locally. Both were right: two
mechanisms, both invisible on an idle machine, both measured under CPU
throttle.

@react-three/cannon steps by ping-pong -- each frame posts `step` and
*transfers* the position buffers to its worker; until the `frame` reply
hands them back, every further step is silently skipped and its 1/60th of
simulation dropped, not deferred. The picture is spawn + completed round
trips x 1/60, and how many round trips fit into thirty pumped frames is the
scheduler's call. basic-ballpit: 30/30 at full speed, 28 under x8 throttle,
each side perfectly reproducible -- two stable pictures, chosen by machine
speed. So the exchange is counted, per worker, and the pump holds the next
frame until the count settles: one step per frame, every machine.
terminate() forgives what a dying worker owes, and a step posted to an
already-terminated worker is never counted -- posting into a dead worker is
a silent void, and counting it held the pump for the full 300s budget.

And the remount's flushSync returns when the *DOM* side has committed; the
scene lives behind the <Canvas> bridge in r3f's own root, whose render is
scheduled, not flushed. trails, throttled: both flushSyncs long returned,
and the second take still assembled itself at frames 1-2 of the pump --
worker created, connected and populated across running frames, with the
first take's worker, not yet unmounted, stepping in the meantime. So the
take announces when it has *finished* mounting (Probe, last in the keyed
fragment: by the time its effect runs, every sibling's have), and the shot
waits for the announcement.

Measured across the twelve cannon examples, full speed and x8, two runs
each: one hash per example, 30/30 round trips, racing-game (29 posted, 28
answered before) and pmndrs-vercel and trails (three distinct pictures)
included. basic-ballpit holds one hash at x1/x4/x8/x20 -- unchanged from
before the fix, so CI converges back to the picture already measured.
object-clump still moves under x8 -- a different, pre-existing channel (its
seeded initial positions shift with load timing); at full speed its hash is
unchanged and stable. clones, springy-boxes and video-cookies unchanged as
non-cannon controls; arkanoid stays in EXCEPTIONS.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@abernier abernier changed the title Count what loads per URL, and never below zero Two shots one picture: count loads per URL, wait for the physics worker and the second take Aug 13, 2026
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.

1 participant