mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-11-14 21:29:34 +00:00
docs(plans): record the second session's ten fixes and what is left
All four of section 4a and six of 4b are done. Adds section 3b with the commits and the three judgement calls behind them: 4a-1 changes rendered audio and its before/after pair is written but unheard; 4a-4's striding was measured, found to move the carve's band selection, and rejected; 4a-2's size ceiling has no test because reproducing it needs a 256 MB payload. Rewrites 4b around what remains — the chorus/phaser LFO phase, which needs a buffer-source rework and its own listen, and the per-element browser setup — and records why the over-broad media.ts health ignore was left alone. Refreshes the baselines (studio 3662, core 1669, engine 736) and adds producer, which was never in the set: 534 passing plus 50 pre-existing collect failures from files importing bun:test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Vance Ingalls
co-authored by
Claude Opus 5
Claude Opus 5
parent
fddd596e8f
commit
0518a10033
@@ -4,9 +4,14 @@ Written 2026-08-08 so a fresh session can continue without re-deriving anything.
|
||||
Everything here was verified in this worktree unless it says otherwise.
|
||||
|
||||
**Worktree:** `~/src/wt/hyperframes/webaudio-fx`
|
||||
**Working branch:** `wa-19-review-fixes` — 11 commits, **local only, never pushed**
|
||||
**Working branch:** `wa-19-review-fixes` — 21 commits, **local only, never pushed**
|
||||
**Branched from:** `wa-18-lane-stretch` @ `4bcc7e188`
|
||||
|
||||
> **Second session (2026-08-08 afternoon) added 10 commits.** All four of §4a is
|
||||
> done, plus six of §4b. See §3b. Baselines moved: studio **3662**, core
|
||||
> **1669**, engine **736** (src/services), producer **534** passing with 50
|
||||
> pre-existing collect failures (files importing `bun:test` under vitest).
|
||||
|
||||
---
|
||||
|
||||
## 1. Where everything is
|
||||
@@ -14,7 +19,7 @@ Everything here was verified in this worktree unless it says otherwise.
|
||||
| Branch | State |
|
||||
| --- | --- |
|
||||
| `wa-18-lane-stretch` | **pushed**, matches remote. PR **#3058** open, ready for review, base `wa-17-lane-clipboard` |
|
||||
| `wa-19-review-fixes` | 11 commits, **local only**. The review fixes. |
|
||||
| `wa-19-review-fixes` | 21 commits, **local only**. The review fixes. |
|
||||
| `carve-reconcile-wip` | 1 commit (`7b57000a1`), **local only**. Working feature, but it breaks undo — see §5. |
|
||||
|
||||
Every other `wa-*` branch is in sync with its remote. The handoff §6 warning about
|
||||
@@ -24,12 +29,17 @@ plain fast-forward.
|
||||
### Test baselines (all green at `wa-19-review-fixes` HEAD)
|
||||
|
||||
```
|
||||
packages/studio 3661 passing, 18 todo, 1 file skipped
|
||||
packages/core 1657 passing
|
||||
packages/engine 733 passing, 3 skipped (src/services)
|
||||
packages/studio 3662 passing, 18 todo, 1 file skipped
|
||||
packages/core 1669 passing
|
||||
packages/engine 736 passing, 3 skipped (src/services)
|
||||
packages/producer 534 passing — plus 50 PRE-EXISTING collect failures
|
||||
```
|
||||
|
||||
`tsc`, `oxlint`, `oxfmt` clean across studio, core, engine, lint.
|
||||
The producer collect failures are files importing `bun:test`, which `vitest run`
|
||||
cannot load. Same 50 before and after this work; only the passing count moved
|
||||
(533 → 534). Producer was never in the original baseline set, which is why.
|
||||
|
||||
`tsc`, `oxlint`, `oxfmt` clean across studio, core, engine, producer, lint.
|
||||
|
||||
---
|
||||
|
||||
@@ -56,7 +66,7 @@ upstack.
|
||||
|
||||
---
|
||||
|
||||
## 3. Fixed (11 commits on `wa-19-review-fixes`)
|
||||
## 3. Fixed in the first session (11 commits)
|
||||
|
||||
Each was verified against current code, fixed, given a test, and the test
|
||||
**falsified by deleting the fix**. Three ship without tests and say so in their
|
||||
@@ -88,72 +98,90 @@ Two judgement calls worth knowing:
|
||||
|
||||
---
|
||||
|
||||
## 3b. Fixed in the second session (10 more commits)
|
||||
|
||||
Same method: verified against current code, fixed, tested, and every test
|
||||
falsified by mutating the fix out.
|
||||
|
||||
| Commit | What |
|
||||
| --- | --- |
|
||||
| `55f852a72` | §4a-1 FX output ducked before it is quantised, not after |
|
||||
| `c9448aec0` | §4a-2 a track's PCM crosses the CDP wire in 8 MiB chunks |
|
||||
| `5e166ad47` | §4a-3 `setRate` re-aims every FX automation lane |
|
||||
| `71ca86456` | §4a-4 FFT scratch reused across carve windows |
|
||||
| `85257571c` | §4b a failed worklet registration can be retried |
|
||||
| `eb60133e3` | §4b a worklet processor retires on dispose |
|
||||
| `eb0080dec` | §4b a blank FX parameter no longer reads as 0 |
|
||||
| `b1faacb3a` | §4b the fatal audio failure gets classified |
|
||||
| `7ecc99605` | §4b dead `ignoreExports` entry dropped |
|
||||
| `fddd596e8` | §4b FX rows keyed by node id |
|
||||
|
||||
Three judgement calls worth knowing:
|
||||
|
||||
- **§4a-1 changes rendered audio** for every track with both a chain and a
|
||||
lane. A before/after pair driving the real code path on real narration is at
|
||||
`~/audio-fx-clip-ab/{before,after}-levelmatched.wav`, level-matched so the
|
||||
only difference is what the clamp destroyed. **Nobody has listened to it
|
||||
yet.** The numbers: the chain peaks at 1.346, the old order sheared 19,166
|
||||
samples (1.7% of the file) flat against ±1, and level-matched the two differ
|
||||
by an error signal 26.3 dB down with a worst single-sample delta of 0.242.
|
||||
- **§4a-4 striding was tried and rejected.** Capping Welch at N windows is 27x
|
||||
faster on a 5-minute voiceover (427 ms → 15 ms) but *changes the carve*: at
|
||||
strength 0.9 the selected band set moved (630 Hz → 160 Hz) and it did not
|
||||
converge back to the full read even at 2048 windows. Only the allocation
|
||||
hoist shipped; wall-clock is unchanged and the values are bit-identical. The
|
||||
reason is recorded in the code so it is not re-attempted.
|
||||
- **§4a-2's ceiling has no test.** Reproducing the 8.7-minute failure needs a
|
||||
256 MB payload. What is tested is chunk assembly, which is where chunking
|
||||
actually goes wrong.
|
||||
|
||||
`applyAudioFxChain` now returns `{ path, envelopeBaked }` rather than a bare
|
||||
path — inferring the bake from `path !== inputWav` breaks on the two early
|
||||
returns (empty chain, empty track).
|
||||
|
||||
## 4. Remaining work
|
||||
|
||||
### 4a. The four still open from the top 15
|
||||
|
||||
**1. FX output clipped to ±1 before the volume lane ducks it.**
|
||||
`packages/engine/src/services/audioFxRender.ts:~136` clamps and quantises to
|
||||
int16 in `writeWav`; `audioMixer.ts:~936` then calls `applyVolumeEnvelopeToWav`,
|
||||
which re-reads that file and multiplies. So a chain that overshoots full scale is
|
||||
destructively clipped even when the envelope immediately pulls the track 12 dB
|
||||
down — audible distortion in the render that preview does not have.
|
||||
|
||||
*I started this one and stopped deliberately.* The fix is to apply the envelope
|
||||
to the float samples inside `applyAudioFxChain` before `writeWav` quantises.
|
||||
That means:
|
||||
- hoisting `envelopeKeyframes` (currently computed AFTER the FX call, `audioMixer.ts:~933`) above `applyAudioFxChain`
|
||||
- extending its options with the envelope + `trackStart` + base volume
|
||||
- setting `bakedEnvelope = true` so the mixer skips the second pass
|
||||
- extracting the incremental segment-cursor gain walk out of
|
||||
`audioVolumeEnvelope.ts:~107-116` so both paths share one implementation
|
||||
- keeping the ffmpeg-expression fallback intact for non-int16 WAVs
|
||||
|
||||
It changes rendered audio for every track with both a chain and a lane, so it
|
||||
wants its own session and a before/after listen.
|
||||
|
||||
**2. Base64 PCM round-tripped whole through one `page.evaluate`.**
|
||||
`audioFxRender.ts:~256`. ~184 MB per 3-minute stereo track across one CDP message
|
||||
pair. Verified ceilings: puppeteer-core sets `maxPayload: 256 MB` and
|
||||
`browserManager.ts` does not pass `pipe: true`, so stereo 48 kHz past **~8.7 min**
|
||||
exceeds the cap; V8's max string length is a second wall at ~17.5 min. Both raise
|
||||
`AudioFxRenderError`, fatal. Compounded by `processCompositionAudio`'s unbounded
|
||||
`Promise.all` holding N payloads at once. Also `Array.from(u8.subarray(...))`
|
||||
boxes each 32 KiB chunk for nothing — `apply` takes array-likes.
|
||||
|
||||
**3. `setRate` does not reschedule FX automation.**
|
||||
`webAudioTransport.ts:~263`. `attachElementFxChain` gets a frozen
|
||||
`{scheduledAt, elapsed, rate}` and commits ramps to absolute context time;
|
||||
`setRate` only assigns `playbackRate.value`. A lowpass sweeping over 10 clip-
|
||||
seconds, switched to 2× mid-play, eats 20 s of material in 10 s of wall clock
|
||||
while the sweep keeps its original schedule. The recovery path in
|
||||
`runtime/init.ts` (`stopAll()` + reschedule) only fires when
|
||||
`hasBoundedActiveSources()` is true — a project-level music bed with no
|
||||
`data-duration` is unbounded, so it never recovers. Second consequence:
|
||||
`timingNow()` advances `elapsed` with the stale rate, so any later chain edit
|
||||
re-aims the envelope at the wrong clip position.
|
||||
|
||||
**4. `powerSpectrum` allocates on the main thread.**
|
||||
`audioCarve.ts:~100` — two `Float64Array(4096)` inside the per-hop loop. A
|
||||
5-minute 48 kHz voiceover is ~461 MB of transient garbage and ~7030 FFTs in one
|
||||
uninterruptible block, called synchronously from the panel. `re` is fully
|
||||
overwritten each iteration and `im` only needs `.fill(0)`, so both hoist; Welch
|
||||
converges after a few hundred frames, so striding the hops gives the same
|
||||
spectrum for ~2% of the work. **Now copied into two more loops** —
|
||||
`audioCarve.ts:~320` and `~558` — so each needs its own spectrum check.
|
||||
### 4a. The four from the top 15 — ALL DONE, see §3b
|
||||
|
||||
### 4b. Confirmed but cut by the reviewer's 15-cap
|
||||
|
||||
Rough severity order, file:line at the review tip:
|
||||
Six of the eight are done (§3b). Two remain:
|
||||
|
||||
- `audioFxGraph.ts:254`/`:316` — chorus + phaser `lfo.start()` at build time, not clip-relative → preview ≠ render phase
|
||||
- `audioFxWorklets.ts:247` — a rejected `addModule` is cached per-context forever while `readyContexts` is only written on success → all worklet effects permanently silent, no retry
|
||||
- `audioFxGraph.ts:159` — worklet `dispose` only disconnects while every `process()` returns `true` → abandoned processors never released
|
||||
- `audioFx.ts:754` — `Number(null)`/`Number("")` === 0 passes `Number.isFinite`, so a null param clamps to 0 instead of its declared default (compressor threshold 0 dB = never engages). `numberOrNull` at `audioAutomation.ts:141` already guards exactly this
|
||||
- `audioStage.ts:89` — `audioFailures: undefined` flattens the typed failure the stage comment says it exists to attach
|
||||
- `audioFxRender.ts:204` — per-element mkdtemp + page + `goto` + runtime re-injection
|
||||
- `.fallowrc.jsonc:662` — file-scoped health ignore also exempts `syncRuntimeMedia`; `:157` `ignoreExports` names two symbols that do not exist
|
||||
- `propertyPanelFxSection.tsx:287` — `key={type-index}` (PLAUSIBLE; likely inert now that the `latest` ref issue is fixed)
|
||||
**LFO phase — the one real piece of work left here.** `audioFxGraph.ts:274`
|
||||
and `:336`: chorus and phaser call `lfo.start()` with no argument, so the
|
||||
oscillator's phase is zero at *attach* time. Offline that IS clip-relative (the
|
||||
graph is built at ctx time 0 and the source starts at 0), but in preview a chain
|
||||
attached mid-play — a seek, a scrub, or any structural edit, which rebuilds the
|
||||
graph — starts the LFO at phase 0 wherever the playhead happens to be. So
|
||||
preview and render disagree on modulation phase, and preview disagrees with
|
||||
itself across an edit.
|
||||
|
||||
*This is not a small fix.* An OscillatorNode's phase cannot be set, and
|
||||
`start(when)` clamps a past `when` to now. The real fix is an
|
||||
AudioBufferSourceNode looping one cycle, started with
|
||||
`offset = (elapsed * speed) % cycle` — which gives phase control but moves the
|
||||
`speed` automation target from `frequency` to `playbackRate` with a
|
||||
`v => v / BASE` map, for both effects. It changes the DSP for every chorus and
|
||||
phaser render, so like §4a-1 it wants a before/after listen. Deliberately not
|
||||
attempted in a session that was already changing rendered audio elsewhere.
|
||||
|
||||
**Per-element browser setup.** `audioFxRender.ts:~240`: every track pays its own
|
||||
`mkdtemp` + `newPage` + `goto` + runtime injection. The browser lease is already
|
||||
pooled across tracks; the page is not. Untouched — it is a throughput
|
||||
optimisation with no correctness claim behind it, and `applyAudioFxChain` was
|
||||
already rewritten twice in this session.
|
||||
|
||||
Also left, deliberately: the file-scoped `health.ignore` on
|
||||
`packages/core/src/runtime/media.ts` in `.fallowrc.jsonc`. Its comment says it
|
||||
is for `refreshRuntimeMediaCache`, but being file-scoped it also exempts
|
||||
`syncRuntimeMedia`, which IS what this stack changed. `health.ignore` takes file
|
||||
paths only, so it cannot be narrowed in config; removing it wants a fallow run
|
||||
to see what it then reports, and fallow already fails on this stack (§6.7). The
|
||||
dead `ignoreExports` entry beside it *was* removed (`7ecc99605`) — both symbols
|
||||
it named were gone. A sweep found five more stale `ignoreExports` symbols
|
||||
belonging to other stacks (`DeleteConfirm`, `GsapKeyframeFormat`,
|
||||
`GSAP_CSS_FALLBACK_BLOCKED_MESSAGE`, and two glob-style `file` entries that
|
||||
match nothing); they are inert and out of scope here.
|
||||
|
||||
### 4c. Cleanup list
|
||||
|
||||
@@ -237,7 +265,15 @@ real store-resync gap could be observed.
|
||||
here and on `wa-18` used `--no-verify`, as the previous 49 did.
|
||||
8. **Two vitest runs racing on this machine** produce spurious 5 s timeouts in
|
||||
unrelated PropertyPanel tests. Do not chase them; re-run alone.
|
||||
9. **The timeline store does not refresh after a delete** — the deleted clip stays
|
||||
9. **`git add -A <pkg>` sweeps up more than the fix you meant to commit.** Two
|
||||
separate worklet fixes landed in one commit that way and had to be split back
|
||||
out. Stage the files the fix touches, or check `git show --stat` before
|
||||
moving on.
|
||||
10. **`applyVolumeEnvelopeToWav` is not the only envelope baker any more.** The
|
||||
FX pass bakes it too and reports `envelopeBaked`; the mixer's own pass runs
|
||||
only for tracks the FX pass did not handle. Anything that reasons about
|
||||
"where does the volume lane get applied" has to look at both.
|
||||
11. **The timeline store does not refresh after a delete** — the deleted clip stays
|
||||
rendered until a page reload. Independent of this work, but it gates when
|
||||
store-driven reconciliation fires.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user