feat(producer): seedable Math.random / crypto.getRandomValues shim, gated#769
Conversation
693bd29 to
f485c81
Compare
b6fabaa to
e5b891f
Compare
miguel-heygen
left a comment
There was a problem hiding this comment.
Solid PRNG shim. The build-time gating approach (template-literal interpolation that emits empty string for the off-path) is clean — keeps the legacy shim byte-identical, which the toBe(VIRTUAL_TIME_SHIM) pinning test enforces.
Mulberry32 is a good choice: single uint32 state, deterministic, no external deps, and the Knuth multiplicative hash + golden-ratio offset for seeding avoids the degenerate-seed-at-frame-0 problem.
The reseedRngFromTime call placement before flushAnimationFrame() in seekToTime is correct — compositions that call Math.random() during rAF callbacks will see the reseeded state.
crypto.getRandomValues filling 4 bytes at a time with a byte-level tail loop handles odd lengths properly — test confirms this with lengths [1,2,3,4,5,7,31,33].
The node:vm test harness is a nice pattern — each test gets an isolated Math / crypto without Chrome overhead, and the makeShimContext bootstrap provides just enough browser globals for the shim's IIFE to run.
LGTM.
— Magi
vanceingalls
left a comment
There was a problem hiding this comment.
Strengths:
- The
seedRandomFromFrame: falsebyte-identity contract is pinned viaexpect(built).toBe(VIRTUAL_TIME_SHIM)atfileServer-seededRandom.test.ts:78— strongest possible regression guard for the gating claim. seekToTimereseed viareseedRngFromTime(safeTimeMs)(fileServer.ts:271) is correctly inserted beforeflushAnimationFrame, so any rAF callback that readsMath.random()during the seek sees the seeded PRNG. Order matters here — if reseed came afterflushAnimationFrame, the first frame would use stale state.- Knuth's multiplicative hash + golden-ratio offset for the seed derivation (
fileServer.ts:222) is the right shape — avoids therngState=0degenerate Mulberry32 output for frame 0. crypto.getRandomValueshandles non-multiple-of-4 byte lengths (fileServer.ts:248-253) with a per-byte tail loop. Pinned by the[1, 2, 3, 4, 5, 7, 31, 33]test (fileServer-seededRandom.test.ts:174) — easy to get wrong, good coverage.node:vmtest harness with independent context per shim invocation (fileServer-seededRandom.test.ts:40) is the right way to test global-overriding scripts — avoids cross-test contamination.
Findings:
important: fileServer.ts:236 Math.random = function() { return mulberry32(); }; runs inside a try { … } catch (e) {} that silently swallows TypeError. In strict-mode iframes or pages where Math is frozen by a CSP/Trusted-Types policy, the assignment will throw, the catch will swallow, and Math.random will remain native — but the seeded crypto.getRandomValues block below may still install, producing partial determinism (crypto reads deterministic, Math.random reads native). That's worse than fully failing closed, because a composition mixing both APIs would render in a way that's neither identical-across-machines nor identical-on-retry. Recommend logging or throwing on the catch when seedRandomFromFrame: true — distributed callers need this to be all-or-nothing.
nit: fileServer.ts:248 var view = new DataView(arr.buffer, arr.byteOffset, byteLen) assumes arr.buffer is a real ArrayBuffer. For SharedArrayBuffer-backed typed arrays (postMessage targets), DataView works but the write may be observed mid-stream by other workers. Compositions rendering off SharedArrayBuffer audio buffers (Web Audio) are exotic but not impossible — worth a comment noting the assumption.
nit: fileServer.ts:218 function mulberry32() — uses rngState as a single uint32. Mulberry32 normally takes a seed parameter; the global-state form is fine but means concurrent calls from rAF and a microtask in the same frame WILL interleave their draws. The shim doesn't currently expose any reentrant entry points (seekToTime is the only mutator), so this is safe in practice, but mention if any future Phase-3 work adds another mutator.
Verdict: APPROVE
Reasoning: Byte-identity pin on the unseeded shim is rock-solid; seeded path is mathematically clean and well-tested across same/different/repeat-seek matrices. The Math-frozen partial-seeding concern is worth a follow-up but doesn't gate this default-off land.
— Vai
…ated
Part of Phase 2 of the distributed rendering plan (determinism hardening).
See DISTRIBUTED-RENDERING-PLAN.md §5.2 (Math.random row) and §17.2
(gating table).
The existing `VIRTUAL_TIME_SHIM` freezes Date.now / performance.now / rAF
on a render seek but leaves `Math.random` and `crypto.getRandomValues` as
native non-deterministic. Compositions that paint stochastic visuals
through these APIs produce different pixels on distributed retries.
This change adds `buildVirtualTimeShim({ seedRandomFromFrame: boolean })`.
Default `false` returns a string byte-identical to today's
`VIRTUAL_TIME_SHIM` (pinned by a new unit test). When `true`, the script
additionally:
- Installs a Mulberry32 PRNG with a single uint32 state
- Reseeds the state from the current virtual time on every
`seekToTime(ms)` call (Knuth multiplicative hash + golden-ratio offset)
- Replaces `Math.random` with the PRNG output
- Replaces `crypto.getRandomValues` to fill the buffer from the PRNG
`VIRTUAL_TIME_SHIM` (the const consumed by `renderOrchestrator` +
`probeStage`) is now `buildVirtualTimeShim({ seedRandomFromFrame: false })`
— in-process behavior unchanged, producer regression baselines unaffected.
Phase 3 distributed primitives will pass `true` when building the chunk
worker's file-server scripts.
10 new unit tests at packages/producer/src/services/
fileServer-seededRandom.test.ts use node:vm to evaluate the shim in
isolated contexts and pin both branches:
- default emits no RNG override and leaves Math.random native
- locked emits the seeded block, produces identical sequences across
fresh VMs at the same time, and yields different sequences for
different times
This is part of a stack of 10 PRs; this is PR 4 of 10.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
e5b891f to
d54ad9c
Compare
f485c81 to
8bac7ba
Compare
The base branch was changed.
miguel-heygen
left a comment
There was a problem hiding this comment.
Re-review after rebase-triggered dismissal. The diff is unchanged from the original approval — the commit was rebased as part of the Graphite stack (old SHA f485c81 -> current 8bac7ba), not amended with new code.
Vai's frozen-Math concern is still present and still valid. Lines 236/263 in fileServer.ts:
try {
Math.random = function() { return mulberry32(); };
} catch (e) {}If Math is frozen (Object.freeze, strict-mode iframe, CSP policy), this assignment throws, the catch swallows, and Math.random stays native — but the crypto.getRandomValues replacement below may still succeed. That gives you partial determinism: crypto reads are seeded, Math.random is native. For distributed rendering where the contract is "same frame -> identical pixels," partial determinism is worse than no determinism because it's invisible and intermittent.
That said, I'm re-approving for the same reasons as before:
- The flag is
falseby default. No call site passestruetoday. The partial-determinism bug can only manifest when Phase 3 distributed primitives opt in — and those don't exist yet. - The fix is straightforward — track whether
Math.randomassignment succeeded and skip the crypto replacement (or throw) if it didn't. That can land as a follow-up before any distributed caller enables the flag. - The rest of the implementation is solid — Mulberry32 choice, Knuth hash seeding, reseed-before-flush ordering, byte-tail loop in getRandomValues, node:vm test isolation.
Recommend adding a // TODO(phase-3) comment at the catch (e) {} on line 263 noting the all-or-nothing requirement before any distributed caller passes seedRandomFromFrame: true. That way whoever writes the Phase 3 caller hits the breadcrumb.
LGTM with that note.
vanceingalls
left a comment
There was a problem hiding this comment.
Re-review of the new HEAD (8bac7ba0) after the prior approvals at f485c817 were dismissed by force-push. The actual PR diff vs. base is unchanged in substance: fileServer.ts extracts a buildVirtualTimeShim(options) builder, gates the Mulberry32 RNG + crypto override behind seedRandomFromFrame, and a 185-line bun test covers both branches.
Strengths
fileServer.ts:323— keepingVIRTUAL_TIME_SHIM = buildVirtualTimeShim({ seedRandomFromFrame: false })as the in-process default and pinning it byte-identical in tests (fileServer-seededRandom.test.ts:77-79) is the right shape: existing callers (renderOrchestrator,probeStage) keep the legacy script exactly.- Mulberry32 seed via Knuth multiplicative hash + golden-ratio offset (
fileServer.ts:258) — frame-0 degeneracy is a real footgun, and the test "different times produce different Math.random sequences" (fileServer-seededRandom.test.ts:149) pins it. crypto.getRandomValueswrites 4-byte words then byte tail (fileServer.ts:271-279) and the odd-length test (:176) catches the boundary.
Notes (not blocking)
- CI is fully green.
Smoke: global installis pending but it's a deploy-side check; required test/typecheck/lint/format/regression-shards all pass.
— Vai
Verdict: APPROVE
Reasoning: Diff is byte-identical-shaped to the prior approval, tests pin both gate branches, all required CI green.

What
VIRTUAL_TIME_SHIMbecomes the default return of a newbuildVirtualTimeShim({ seedRandomFromFrame: boolean })function. WhenseedRandomFromFrame: true, the emitted script additionally replacesMath.randomandcrypto.getRandomValueswith a Mulberry32 PRNG that reseeds from the current virtual time on everyseekToTime(ms)call. The default (false) emits a string byte-identical to today'sVIRTUAL_TIME_SHIM.Why
Part of Phase 2, §5.2 (
Math.randomrow) and §17.2 (gating table).The existing shim freezes
Date.now/performance.now/ rAF on a render seek but leaves stochastic APIs as native. Compositions that paint withMath.random()produce different pixels on each retry — that's fine in-process but breaks the distributed pixel-identical-retry contract.The flag is gated
falsefor in-process callers so producer regression baselines stay byte-identical; Phase 3's distributed primitives will passtrue.How
VIRTUAL_TIME_SHIMconst into a functionbuildVirtualTimeShim(options)and re-exported the const asbuildVirtualTimeShim({ seedRandomFromFrame: false }).uint32state, reseeded byreseedRngFromTime(ms)using Knuth's multiplicative hash + golden-ratio offset (so the seed for frame 0 isn't degenerate).Math.random = mulberry32;crypto.getRandomValues(arr)fillsarrfour bytes at a time from the PRNG.seekToTime(ms)reseeds before flushing the rAF queue, so twoseekToTime(N)calls separated by intervening renders produce the sameMath.random()sequence.toBe(VIRTUAL_TIME_SHIM)test).Test plan
fileServer-seededRandom.test.ts(10 cases). Usesnode:vmto evaluate the shim in fresh isolated contexts so eachMath.randomoverride doesn't clobber the host.VIRTUAL_TIME_SHIM, mentions no RNG identifiers, leavesMath.randomnative.Math.randomtoString stops being[native code]; two fresh VMs atseekToTime(1234)produce identical 16-sample sequences; reseeking back to the same time mid-stream restarts the sequence; different times produce different sequences;crypto.getRandomValuesis deterministic and handles odd byte lengths.fileServer.test.ts(23 cases) still passes — confirms in-process consumers (renderOrchestrator,probeStage) see the same shim.bun run --cwd packages/producer typecheckclean.bunx oxlint+bunx oxfmt --checkclean.This is part of a stack of 10 PRs; this is PR 4 of 10. Stacked on top of #768.
🤖 Generated with Claude Code