fix(core): give the chorus and phaser LFOs a phase, and unwire them on dispose

[vanceingalls]

lfo.start() with no argument puts an OscillatorNode at phase zero at attach time. Offline that is clip-relative, but preview rebuilds the graph on any shape change — and a seek or scrub does the same. So a chorus attached 3.6 s into a clip started its sweep from the top there: preview disagreed with the render, and with itself across an edit.

An OscillatorNode's phase cannot be set, so the modulator is now one cycle of the waveform in a looping AudioBufferSourceNode, where start(when, offset) is a phase control.

It got the before/after listen through the real engine path, swapping only the injected runtime: chorus 76.5 dB down, Triangular phaser 63.7, Sinusoidal 88.1, a bare 440 Hz tone 87.6. Worst single sample 0.00027 — inaudible, as expected, since a render builds at position 0.

Also fixes a leak the cleanup list pointed at: both LFOs were stopped but never disconnected, so every rebuild that dropped a modulated effect left a modulator wired to what it drove. And folds the two artifact build scripts, which were 50 lines each differing in five names.

🤖 Generated with Claude Code

[terencecho]

LGTM pending Preflight green. Real behavioral fix, not a refactor — both parts (phase determinism + dispose leak) load-bearing with strong pin tests.

Phase fix at audioFxGraph.ts:98-125 (lfoSource helper). OLD: chorus/phaser LFOs used OscillatorNode, which has no settable initial phase — start() at currentTime always fires from phase 0, so every mid-play rebuild snapped the chorus back to sweep-peak wherever the playhead sat. Also, the phaser's lfo.type was set only via apply() running AFTER start(), so the initial oscillator was silently OscillatorNode's default (sine) regardless of p.type — the base comment acknowledges "the declared default ('Triangular') was silently a sine."

NEW: both LFOs are looping AudioBufferSource — 1-second cycle at sampleRate frames, playbackRate.value = speed (speed reads as Hz), and start(currentTime, offset) where offset = (((elapsed * speed) % 1) + 1) % 1 * (length / sampleRate). Chorus wave hardcoded "sine" at :349; phaser wave computed once at build (String(p.type) === "1" ? "sine" : "triangle") at :395; shapeOf now includes ~${p.type} at :559 so a waveform change forces a rebuild rather than a silent no-op update().

Dispose fix at audioFxGraph.ts:130-138 (retireLfo). OLD: chorus/phaser disposers called lfo.stop() inside a try/catch but omitted lfo from the .disconnect() sweep — oscillator remained wired to depth and via it to dl.delayTime / stage .frequency. Inaudible (shell disconnected upstream) but reachable; editing session accumulated dead LFOs. NEW: retireLfo(src) stops + disconnects; both disposers call it FIRST at :376 and :451. Same intent as wa-19c's __hfDispose worklet-port pattern, different primitive (BufferSource has no port).

Pin tests (all in audioFxGraph.test.ts):

• Phase determinism — elapsed=3.5, speed=2 → offset 0; elapsed=3.6 → offset 0.2.
• Zero-phase-for-render pin.
• Waveform discriminator — samples buffer at 1/8-cycle: sine=√½≈0.707, triangle=0.5 (kills both a "default-to-sine" and "default-to-triangle" regression).
• Rebuild-on-waveform-change — update() returns false when p.type changes.
• Dispose leak — lfo.disconnected === true per chorus/phaser (iterated ["chorus","phaser"]).
• End-to-end: audioFx.test.ts "hands a rebuilt graph the playhead it happens at" threads elapsed through attachElementFxChain on structural rebuild.

OLD-assumption counterfactuals:

• "OscillatorNode start() from phase 0 is fine" → fails elapsed=3.6 → offset≈0.2 (OscillatorNode has no start(when, offset) — startArgs[1] would be undefined).
• "Leaving lfo.type unset until apply() runs is fine" → the 1/8-cycle sample would read 0.707 for both type:"0" and type:"1", failing the triangle assertion.
• "Shell is disconnected, so leaving lfo off the disconnect list is fine" → fails the explicit lfo.disconnected === true check.

Cross-fix: shapeOf including ~${p.type} for phaser means waveform-change forces rebuild → forces dispose() → exercises retireLfo. Good coupling, tested. elapsed threading is behavior-neutral for non-modulated effects (frame?.elapsed ?? 0 fallback at audioFx.ts:189).

CI: Preflight (lint+format) red. format:check failing — likely on the new buildInjectedArtifact.ts / wavChunks.ts / wavChunks.test.ts files or modified helpers. Needs a bun run format push. Downstream player-perf / preview-regression / regression fail-fast on preflight, so this is the only blocker.

Nits:

• ctx.currentTime typeof-guard at :123 is dead code in production (real BaseAudioContext always exposes currentTime); belt-and-braces for the test path — worth a one-line comment naming the fake-test motivation, or drop it and let fakes set currentTime = 0.
• shapeOf's ~${p.type} bypasses normalizeAudioFxParams — numeric vs string type values would produce different shape strings though identical graphs; consumers normalize before calling, low risk.

— Review by tai (pr-review)

[miga-heygen]

Review: fix(core): give the chorus and phaser LFOs a phase, and unwire them on dispose — #3183

Verdict: LGTM

Cousin work to #3175's worklet lifecycle fix — same class of leak (stopped but not disconnected), solved with the same discipline but for oscillator-based LFOs.

LFO phase implementation is correct. lfoSource replaces OscillatorNode (whose phase is always 0 and cannot be set) with a looping AudioBufferSourceNode containing one hand-built waveform cycle. Phase control comes from start(when, offset) where offset is a position in seconds into the 1-second buffer.

Waveform math verified:

• Sine: Math.sin(2π × phase) — starts at 0, peaks at +1 at 1/4 cycle. Matches OscillatorNode convention.
• Triangle: 4 × |((phase + 0.75) % 1) - 0.5| - 1 — starts at 0, correct piecewise linear shape.

Offset calculation uses double-modulo (((elapsed * speed) % 1) + 1) % 1 to safely handle negative values. playbackRate directly reads in Hz (1-second buffer at 1x = 1 Hz), so the speed knob needs no mapping.

Dispose is complete. retireLfo does both halves: stop() (try/catch for already-stopped) then disconnect(). Both chorus and phaser dispose handlers call it. No nodes left wired.

Tests are thorough: waveform shape at 1/8 cycle (triangle vs sine discrimination), phase offset at whole and partial cycle counts, LFO unwiring on dispose for both chorus AND phaser, zero-phase for renders, and mid-play rebuild getting the correct playhead.

RIFF chunk refactor is a clean SSOT extraction — riffChunks generator shared between audioFxRender.ts and audioVolumeEnvelope.ts, policy-free (yields chunks, holds no interpretation). Build script consolidation is similarly well-typed.

No issues found.

---

Review by Miga

🤖 Generated with Claude Code

[github-actions]

Fallow audit report

Found 7 findings.

Duplication (4)

Severity	Rule	Location	Description
minor	fallow/code-duplication	packages/core/src/runtime/audioFx.test.ts:540	Code clone group 1 (36 lines, 2 instances)
minor	fallow/code-duplication	packages/core/src/runtime/audioFx.test.ts:615	Code clone group 2 (14 lines, 2 instances)
minor	fallow/code-duplication	packages/core/src/runtime/audioFx.test.ts:648	Code clone group 2 (14 lines, 2 instances)
minor	fallow/code-duplication	packages/core/src/runtime/audioFx.test.ts:742	Code clone group 1 (36 lines, 2 instances)

Health (3)

Severity	Rule	Location	Description
minor	fallow/high-crap-score	packages/core/scripts/buildInjectedArtifact.ts:34	'buildInjectedArtifact' has CRAP score 42.0 (threshold: 30.0, cyclomatic 6)
minor	fallow/high-complexity	packages/engine/src/services/audioFxRender.ts:254	'applyAudioFxChain' has cyclomatic complexity 23 (threshold: 20) and cognitive complexity 23 (threshold: 15)
minor	fallow/high-crap-score	packages/engine/src/services/audioVolumeEnvelope.ts:42	'parseWavLayout' has CRAP score 49.5 (threshold: 30.0, cyclomatic 13)

Generated by fallow.