From 7968daa97a15470054554ac1c24dfa7b18c90899 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Sat, 8 Aug 2026 13:02:46 -0700 Subject: [PATCH] docs(plans): record the 4c cleanups taken and why the rest were left Four of the nine cleanup items are done in three commits. The enabledAudioFxNodes consolidation is worth reading as more than tidying: it exposed that buildFxChain's update() filter had no test, and the failure it prevents is silent. The five left each carry a reason rather than a line item. The two WAV chunk walkers are genuinely duplicated but differ in four behaviours, in the parser every render's audio passes through. The FlatSlider reuse is a merge of two behaviours rather than a swap, and is entangled with the held-back throttle fix in PR #2190. The wet/dry shells want doing after the LFO rework, not twice. The 30/s curve sampling is an audio-changing decision, not a cleanup. Verified at this HEAD: studio 3662, core 1672, engine 736, producer 534 under vitest plus 203 under bun test; tsc and oxlint clean across all five packages. Co-Authored-By: Claude Opus 5 (1M context) --- plans/audio-review-fixes-handoff.md | 72 ++++++++++++++++++++++------- 1 file changed, 56 insertions(+), 16 deletions(-) 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. ---