diff --git a/plans/audio-review-fixes-handoff.md b/plans/audio-review-fixes-handoff.md index 3b553cffce..0e30c3eae0 100644 --- a/plans/audio-review-fixes-handoff.md +++ b/plans/audio-review-fixes-handoff.md @@ -4,13 +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` — 21 commits, **local only, never pushed** +**Working branch:** `wa-19-review-fixes` — 25 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). +> **Second session (2026-08-08 afternoon) added 14 commits.** All four of §4a +> is done, six of the eight in §4b, and 4 of the 9 in §4c. See §3b and the +> per-section notes. Everything left carries a written reason for being left. +> Baselines moved: studio **3662**, core **1672**, engine **736** +> (src/services), producer **534** under vitest plus **203** under bun test. --- @@ -19,7 +20,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` | 21 commits, **local only**. The review fixes. | +| `wa-19-review-fixes` | 25 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 @@ -30,7 +31,7 @@ plain fast-forward. ``` packages/studio 3662 passing, 18 todo, 1 file skipped -packages/core 1669 passing +packages/core 1672 passing packages/engine 736 passing, 3 skipped (src/services) packages/producer 534 passing under vitest, 203 under bun test (see below) ``` @@ -211,15 +212,54 @@ match nothing); they are inert and out of scope here. ### 4c. Cleanup list -- `audioFxRender.ts:43` duplicates `parseWavLayout` -- `propertyPanelFxControls.tsx:51` re-implements FlatSlider/FlatToggle/FlatSelectRow, losing their throttle, pointer capture, Escape-revert and design telemetry -- `PropertyPanelFlat.tsx:549` reads via hardcoded `"fx-chain"` literals while writing via constants -- `audioMixer.ts:328` re-rolls curve sampling at 30/s vs the scheduler's 100/s while `sampleAutomationCurve` names it as a consumer -- `build-audio-fx-runtime.ts` is a near-verbatim clone of `build-position-edits-render.ts` -- `audioFxGraph.ts:279`/`:352` four hand-rolled wet/dry shells whose dispose lists have already drifted -- `audioFxGraph.ts:448` enabled-node filter restated 3× while `enabledAudioFxNodes()` goes unused -- `audioFxGraph.ts:494` dead `shapeOf` recomputation on every observer tick -- `PropertyPanelFlat.tsx:624` a `doc!` non-null assertion plus an `as` cast laundering `undefined`; `audioAutomation.ts:122` a cast after the guard already narrowed — both against the repo's "avoid `any`/`as T`" rule and the standing no-`!` rule +**Done — 4 of the 9 items, in 3 commits:** + +| Commit | What | +| --- | --- | +| `ac0e37247` | the `doc!` assertion and two `as` casts the guards already covered | +| `ce7c9e486` | one enabled-node helper instead of three restatements; dead `shapeOf` recompute dropped | +| `5a2247435` | dataset keys derived from their attributes instead of four loose literals | + +The `enabledAudioFxNodes` consolidation was not cosmetic: lining the three call +sites up exposed that `buildFxChain`'s `update()` filter had **no test**, and +the failure it protects against is silent — a bypassed node shifts every later +node's parameters into its neighbour, and the shape is identical either way so +nothing forces a rebuild that would show it. Now covered. + +**Not done, with reasons:** + +- `audioFxRender.ts:44 readWavChunks` vs `audioVolumeEnvelope.ts:41 + parseWavLayout`. Real duplication — the same word-aligned chunk walk twice — + but they are **not** the same function. `readWavChunks` breaks at the first + `data` chunk, returns a slice, checks no format, and skips the `WAVE` check; + `parseWavLayout` scans every chunk, returns offsets (it edits in place), + rejects anything that is not 16-bit PCM, and does check `WAVE`. Unifying + means picking one behaviour for each of those four, in the parser every + render's audio goes through, for a cosmetic win. Deliberately not attempted + at the end of a long session. If taken up: the shared walk wants its own + module — `audioFxRender` already imports `audioVolumeEnvelope`, so putting it + in the former makes a cycle. +- `propertyPanelFxControls.tsx:51` re-implements FlatSlider/FlatToggle/ + FlatSelectRow, losing their throttle, pointer capture, Escape-revert and + design telemetry. The biggest item left in 4c and the one most likely to + change behaviour: the local versions grew a commit/preview split + (`onChainPreview` while dragging, `onChainChange` on release) and an + `edited` guard that the shared controls do not have, so this is a merge of + two behaviours, not a swap. Needs the FlatSlider throttle question settled + first — see the note about PR #2190 in the project memory, where a + throttle fix was held back because the same bug reproduced in the published + studio. +- `audioFxGraph.ts:279`/`:352` four hand-rolled wet/dry shells whose dispose + lists have already drifted. Mechanical but touches every modulated effect's + teardown; worth doing right after the LFO-phase rework (4b) rather than + twice, since that rework rebuilds two of the four anyway. +- `audioMixer.ts:328` re-rolls curve sampling at 30/s against the scheduler's + 100/s while `sampleAutomationCurve` names it as a consumer. Untouched: the + rate is a deliberate budget for the PCM baker (`CURVED_SEGMENT_SAMPLES_PER_SEC`) + and reconciling it is an audio-changing decision, not a cleanup. +- `build-audio-fx-runtime.ts` is a near-verbatim clone of + `build-position-edits-render.ts`. Build scripts; no consumer risk, no + behaviour to protect, just nobody's afternoon yet. ---