fix: preserve plateaus in sampled audio automation - #2863

Reviewed at 8a43e09.

Correct fix. Traced the plateau-preserving logic against representative inputs and it holds in every case I could construct.

Trace verification

Scenario: audio at volume=0.8 from t=0 to t=1.05, then volume=0.2 for the rest of a 2s clip, sampled at 10fps (step=0.1).

• Old (change-only compression): samples at t=0.0, 0.1, ..., 1.0 all read 0.8 → only t=0.0 pushed (!last). Then t=1.1 reads 0.2 → push {1.1, 0.2}. Final bounded === sampleEnd forces {2.0, 0.2}. Result: [{0, 0.8}, {1.1, 0.2}, {2.0, 0.2}]. Linear interpolation at t=0.5 = 0.8 + (0.2 - 0.8) * (0.5/1.1) = 0.527 — the reported plateau ramp bug.

• New (plateau-aware): t=0 pushed, previousSample = {0, 0.8}. Samples 0.1–1.0 same value → not pushed, previousSample advances to {1.0, 0.8}. At t=1.1 the delta triggers: previousSample.time (1.0) > last.time (0) → push previousSample first (bounding the plateau tail), then push {1.1, 0.2}. Final t=2.0 same as last → sample.time (2.0) > last.time (1.1) → push. Result: [{0, 0.8}, {1.0, 0.8}, {1.1, 0.2}, {2.0, 0.2}]. Interpolation at t=0.5 stays at 0.8 across the plateau, transitions from 0.8→0.2 across [1.0, 1.1]. Matches the assertion in the new htmlCompiler.test.ts test (interpolateVolumeGain(automation.keyframes, 0.5) === 0.8).

Boundary edge cases

• Same-value plateau reaching exactly sampleEnd. The else if (isFinalSample && sample.time > last.time) guard prevents pushing a duplicate keyframe when the last-pushed sample is already at sampleEnd — verified by walking a constant-volume clip. Keyframes end up [{0, v}, {end, v}], exactly one at each boundary.
• Ramp with no plateau. During a continuous ramp, every sample delta > 0.0001, so every one triggers the push branch — and previousSample.time === last.time inside that branch (the previous iteration's sample IS what's on top of the keyframes stack). The previousSample.time > last.time check evaluates false → no double-push. Correctly matches the third new test (preserves every sampled point of a continuous ramp).
• Loop clamp t = Math.min(sampleEnd, t + step). Combined with if (bounded === sampleEnd) break; at loop tail — no infinite-loop path for positive step; the clamp only affects the last iteration where t + step > sampleEnd. Correct.

Epsilon choice

Math.abs(last.volume - sample.volume) > 0.0001 — 1e-4 on a 0-1 range is 0.01% quantization tolerance. Given toFixed(6) on both stored time and volume, sub-1e-4 quantization noise gets absorbed. Home flagged this as a risk; verified the epsilon is generous enough that fixed-rate audio sampled through GSAP won't false-trigger changes from FP noise alone. Reasonable choice.

Nits

• fallow-ignore-next-line code-duplication comment placement. In mediaVolumeEnvelope.ts:64, the comment sits between the docblock-style block-comment ("The PCM baker intentionally inlines this lookup...") and the while loop it applies to. Reads clean enough, but a future reader might not tie the ignore directive to the while — worth verifying the linter still respects the directive with the two-comment sandwich, or collapse the two into one comment. Minor.
• Extracted helpers are extraction, not fix. parseFiniteDatasetNumber and resolveVolumeProbeWindow are pure refactors of pre-existing dataset-parsing code, mostly to shorten probeElementVolumeKeyframes. Behavior-preserving; noted for scope clarity. The staticVolume fallback semantics change slightly — old code did Number.parseFloat("") = NaN → Number.isFinite(NaN) = false → staticAttr = 1; new code does parseFiniteDatasetNumber("") = undefined → staticAttr = undefined → staticVolume = 1 via ?? 1`. Same result, different path. Fine.

What I didn't verify

• The producer-side change in discoverAudioVolumeAutomationFromTimeline is line-parallel with the core-side change; the mocked test exercises the plateau path end-to-end. Didn't verify the mixer/PCM baker downstream — trusted per PR body's "Verified plateau interpolation, ... PCM envelope application, mixer behavior."

— Review by Rames D Jusso

Verdict: APPROVE — grade A — rubric CORRECT

Root-cause fit: The fix identifies both defects: (1) change-only compression discarded the last plateau sample so downstream linear interpolation bridged across the flat run, and (2) the loop skipped a short final transition that fell between step intervals. recordVolumeSample inserts the retained pre-change sample as a plateau-boundary keyframe, and the new loop t = Math.min(sampleEnd, t + step) guarantees the exact clip-end sample is always visited. Applied consistently in the core probe (mediaVolumeEnvelope.ts) and the producer's page-evaluated equivalent (htmlCompiler.ts), so preview and render stay in agreement.

Claims verified (each with file:line at HEAD):

• "Retain preceding real sample when changed value follows omitted flat run" — packages/core/src/runtime/mediaVolumeEnvelope.ts:83-93, producer mirror packages/producer/src/services/htmlCompiler.ts:2172-2179.
• "Continuous ramps unchanged" — in a ramp previousSample.time === last.time, so the plateau-preservation branch is skipped; only the new sample is pushed (mediaVolumeEnvelope.ts:85).
• "Always include exact clip-end sample" — loop t = Math.min(sampleEnd, t + step) at mediaVolumeEnvelope.ts:147 + producer htmlCompiler.ts:2156 guarantees a terminal iteration at sampleEnd; isFinalSample branch (mediaVolumeEnvelope.ts:94-96) pushes even when volume equals last.volume.
• "Unit tests added/updated" — 3 core tests (plateau retention, short-end transition, ramp preservation) and 1 producer test (plateau boundaries + interpolation gain) (mediaVolumeEnvelope.test.ts:5-70, htmlCompiler.test.ts:1767-1817).
• File-count / +200/-27 arithmetic: 69+57+53+21=200, 1+19+0+7=27. MATCHES.

Adversarial findings:

• nit — pre-existing divergence not introduced here: core resolves data-end before data-duration (mediaVolumeEnvelope.ts:110-115), producer resolves data-duration before data-end (htmlCompiler.ts:2137-2142). Same clip with both attrs would probe different windows in preview vs render. Flag for follow-up; not blocking this PR.
• nit — const bounded = Math.min(sampleEnd, t) (mediaVolumeEnvelope.ts:148, htmlCompiler.ts:2157) is now redundant since t is already clamped by the loop update; cosmetic dead computation.
• No P0/P1. Root-cause fit, boundary handling (first + last sample, ramp→plateau→ramp), sample-rate independence (step derived from capped fps), equal-value epsilon (0.0001 unchanged, generous vs toFixed(6)), and normaliseEnvelope integration all check out.

CI state: All required checks pass.

Suggested next step: Ship. Optional follow-up ticket to reconcile the pre-existing core-vs-producer end-resolution priority so the two paths never disagree on the sampled window.

— Review by Via