From d887c68014996e7f8974d90c605b51420e0fa927 Mon Sep 17 00:00:00 2001 From: OmikronApex Date: Tue, 14 Jul 2026 03:24:46 +0200 Subject: [PATCH] docs: commit the audio-engine TLC findings + plan this branch implements Co-Authored-By: Claude Fable 5 --- docs/audio-engine-tlc.md | 940 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 940 insertions(+) create mode 100644 docs/audio-engine-tlc.md diff --git a/docs/audio-engine-tlc.md b/docs/audio-engine-tlc.md new file mode 100644 index 0000000..131c7bb --- /dev/null +++ b/docs/audio-engine-tlc.md @@ -0,0 +1,940 @@ +> **Execution status (2026-07-14, branch `refactor/audio-engine-tlc`):** implemented. +> Phases 0-8 of Part IV/V are complete on this branch, including the chain-mutation +> serializer + chainGeneration (native + executor), the monitor-mute arbiter, the +> single persistence store, and getLatencyBreakdown. Two sequencing changes vs. the +> roadmap below: the gain-sanitization fix shipped first (before phase 1), and the +> Part II ownership work that needs the rig_builder repo (single chain owner, +> legacy-path deletion, alias removal) is NOT on this branch. Drift note: the +> "21 clearChain call sites" count in Part II grew to 30 by execution time. + +# Audio Engine TLC — Consolidated Findings & Refactor Plan + +TLC pass on the feedBack-desktop audio engine (2026-07-12, branch `fix/loopback-capture-permission`). +Single consolidated document; supersedes the four separate docs +(overview / collisions / deep-read / refactor-plan). + +Contents: +- **Part I** — how the engine works and integrates with the app, effects (NAM/VST/IR), plugins. +- **Part II** — redundant code, settings with multiple writers, control collisions. +- **Part III** — line-level deep read of `AudioEngine.cpp` and `NodeAddon.cpp`. +- **Part IV** — decomposition plan for the monolithic files. +- **Part V** — merged priority roadmap. + +--- + +# Part I — Architecture Overview +Covers: engine architecture, desktop-app integration, input/output paths, effects (NAM / VST / IR), +detection pipeline, and which bundled plugins touch the engine and how. + +--- + +### 1. Layer map + +``` +Renderer plugins (rig_builder, note_detect, stems, …) + │ window.feedBackDesktop.audio.* / .audioEffects.* (aliased: slopsmithDesktop) + ▼ +preload.ts ── contextBridge, ~99 audio methods + audioEffects methods + │ ipcRenderer.invoke / send + ▼ +Main process + audio-bridge.ts 102 ipcMain.handle channels (audio:*, audio-effects:*) + audio-effects-executor.ts validates chain plans, drives native chain + vst-crash-guard.ts sentinel files → blocklist crashy VSTs across restarts + plugin-manager.ts git-based plugin install/update (server plugins) + │ require('slopsmith_audio.node') + ▼ +NodeAddon.cpp (N-API, ~160 KB) ── marshals JS ⇄ C++, async workers for VST loads + ▼ +AudioEngine (JUCE, C++) src/audio/ + ├── SourceChain ×8 (pooled) per-input capture + detect + tone chain + │ └── SignalChain ordered ProcessorSlots (VST | NAM | IR) + ├── Backing-track transport + signalsmith-stretch + ├── Stream sink (2nd output device for OBS/Discord) + └── Renderer bus (WebAudio master → engine output) + ▼ +JUCE AudioDeviceManager(s) → WASAPI / ASIO / DirectSound / CoreAudio / ALSA / JACK + +Out-of-process: slopsmith-vst-host.exe (src/vst-host/main.cpp) — VST3 sandbox child. +``` + +Key files: + +| Area | File | +|---|---| +| Engine core | `src/audio/AudioEngine.{h,cpp}` (47K header / 156K impl) | +| Per-input chain | `src/audio/SourceChain.{h,cpp}` | +| Effects chain | `src/audio/SignalChain.{h,cpp}` | +| NAM | `src/audio/NAMProcessor.{h,cpp}` (wraps NeuralAmpModelerCore, `src/audio/third_party/NAM`) | +| IR / cab | `src/audio/IRLoader.{h,cpp}` (juce::dsp::Convolution) | +| VST hosting | `src/audio/VSTHost.{h,cpp}` | +| VST sandbox | `src/audio/Sandbox/*` + `src/vst-host/main.cpp` | +| Detection | `PitchDetector` (YIN), `MlNoteDetector` (Basic Pitch ONNX), `ChordScorer`, `NoteVerifier`, `OnsetDetector` | +| Utility DSP | `NoiseGate`, `TonePolish`, `BackingLeveler`, `AudioSanitize` | +| JS bridge | `src/audio/NodeAddon.cpp`, `src/main/audio-bridge.ts`, `src/main/preload.ts` | + +--- + +### 2. AudioEngine core + +`AudioEngine` is a `juce::AudioIODeviceCallback` owning **two** `AudioDeviceManager`s: + +- **Duplex mode** (default): `inputDeviceManager` owns both directions; one callback + (`audioDeviceIOCallbackWithContext`) reads input, processes, writes output directly. +- **Split mode**: input-only on `inputDeviceManager`, output-only on `outputDeviceManager` + (separate device types possible, e.g. ASIO in + WASAPI out). Processed stereo crosses via + `outputPendingRing` — a lock-free SPSC ring of 4096 frames where each stereo frame is packed + into one `atomic` (bit_cast L|R) so reads are tear-free. ~85 ms of drift absorption. + Input and output block sizes may differ; the ring absorbs asymmetry. + +Device management surface: enumerate types/devices, dual-type probing +(`probeDeviceOptionsDual` — sample-rate intersection, `compatible` flag), `setAudioDevices`, +metrics (overflow/underflow counters, ring fill). Config persisted by the renderer via +`audio:saveDeviceSettings` / `loadDeviceSettings` (legacy single-`type` settings are mirrored to +input+output type). + +Threading model (recurring pattern throughout): audio thread never locks — atomics everywhere, +lock-free SPSC rings, `static_assert(is_always_lock_free)`, control-thread mutation via +pending-flag handoff (e.g. `backingPendingSpeed`), and drop-oldest on overflow with counters. + +--- + +### 3. Audio input + +#### Sources (multi-input) + +Per-input state lives on **SourceChain**, a fixed pool of `kMaxSources = 8` constructed up front +(pointers never reassigned → no race with audio thread; add/remove only flips an atomic `active` +flag). `sources[0]` is the permanent legacy default input; the engine facade forwards the +single-source API to it so NodeAddon/renderer needed no change. + +- `addSource(inputChannel, deviceKey)` — bind another channel of the current device + (multi-channel interfaces, e.g. Valeton GP-5) or of an **additional physical input device** + (`bindInputDevice(deviceKey, name)`, up to 3 extras, each at its own clock; forces split mode). +- Removal uses a per-deviceKey `callbacksInFlight` counter handshake; wedged callbacks defer the + release (`pendingRelease[]`) instead of blocking. +- Per-source: input gain, channel select (-1 = mono mix), monitor mute/kill, meters, verifier + offsets (auto device-latency delta + user fine-tune, summed). + +#### Per-source capture path (SourceChain::processBlock, audio thread) + +``` +device input → channel select / mono mix → inputGain + ├─→ MlNoteDetector feed + pre-gate inputFrameRing (8192, SPSC) [getInputFrame/getInputSince] + → NoiseGate (post-gain, pre-FX; pitch detector sees ungated signal) + ├─→ YIN PitchDetector feed + post-gate rawAudioRing (16384) [getRawAudioFrame → tuner] + → SignalChain (VST/NAM/IR tone chain) + → sanitize (non-finite/runaway scrub, counted — issue #403) + → monitor mute / monitor kill / chainOutputGain → TonePolish (fixed 3-band EQ, guitar bus only) + → summed into output mix (sourceMonitorScratch, pre-sized) +``` + +Monitor semantics: `monitorMute` mutes dry pass-through only when chain empty (suppressible +around song-load chain rebuilds); `monitorKill` silences the guitar bus unconditionally +(external-rig users), applied to every pooled source. + +--- + +### 4. Effects — SignalChain + +Ordered `ProcessorSlot`s, each `Type::{VST, NAM, IR, Empty}` holding a +`unique_ptr`. Features: + +- **Routing**: per-slot `pan` (constant-power), `branch` (0 = serial trunk, ≥1 = parallel branch, + branches read the pre-split signal, panned outputs summed at merge), `branchSrc` (branch reads + L / R / both), `postGain` (per-amp loudness trim). Pre-allocated scratch buffers; all-trunk path + pays nothing. +- **State**: per-slot base64 VST state; whole-chain JSON preset save/load (`savePreset` / + `loadPreset`); `replaceProcessor` swaps a slot in place (same id/position) — used for sandbox + promotion and `replaceIR` cab swaps; `setSlotState` for the tone-switcher's incremental rebuild. +- **MIDI**: lock-free SPSC queue (64 msgs), `queueMidiMessage(slotId, msg)` from the N-API thread + → drained on audio thread (`audio:sendMidiToSlot`). +- Oversized device blocks (WASAPI shared after device start) are sliced to the prepared block size. +- SEH/signal guards around plugin prepare/state calls; faults blocklist the plugin path. + +#### Processor types + +- **NAMProcessor** — mono in/out neural amp model (`.nam`), NeuralAmpModelerCore backend + (`SLOPSMITH_NAM_SUPPORT`). Async-safe model load: staged `pendingModel`, atomic swap. + Input/output level params. No editor. +- **IRLoader** — cab IR convolution (`.wav/.aif/.ir`) via `juce::dsp::Convolution`. +- **VST3** via **VSTHost**: background directory scanning (per-file subprocess probe through + `slopsmith-vst-host --scan-plugin` → XML merge, so crashy plugins can't kill the app), + known-plugin persistence, sync `loadPlugin` + async `loadPluginAsync` (message-thread pumping + required for AmpliTube-class plugins that post messages to themselves during init). + +#### VST sandbox (out-of-process) + +`SandboxedProcessor` (src/audio/Sandbox) is a `juce::AudioProcessor` façade that forwards +everything to a spawned `slopsmith-vst-host.exe` child over a control pipe + shared-memory audio +channel (`Protocol.h`; platform impls `_win` / `_posix` / `_shared`). Properties: + +- One child per plugin; child dies with the processor. Crash → `isAlive()` false → audio thread + inserts silence; `CrashCallback` + `CrashAttribution` report which plugin died. +- Child owns the plugin editor as its own top-level window (Reaper-style; cross-process HWND + reparenting broke D3D/GL plugins like Neural DSP Archetypes). +- **Promotion path**: an in-process VST3 is promoted to the sandbox when its editor is opened + (in-process editors are the Windows WndProc/Qt crash path). `captureVstStateForPromotion` + snapshots state under lock + SEH guard, then `replaceProcessor` swaps in the sandboxed twin. +- Child guarantees JUCE MessageManager on the OS main thread (impossible in the Node addon where + V8 owns it); audio runs on a dedicated ring-drain worker. +- Known v1 gaps (documented in header): no `AudioProcessorParameter` proxies; bus layout + hard-coded stereo↔stereo. +- **vst-crash-guard.ts** (main process): arms a sentinel file before risky load/editor operations; + a crash leaves the sentinel behind → next launch blocklists that plugin. + +--- + +### 5. Audio output + +Three output paths, mixed in the engine: + +1. **Monitor output** (primary device): sum of active sources' processed guitar buses + + backing-track mix (`backingVolume` fader, `BackingLeveler` per-song loudness normalizer) + + master `outputGain`. Duplex writes in-callback; split drains `outputPendingRing`. +2. **Stream sink** — an ADDITIONAL output device carrying an independent submix + (backing/game and/or guitar monitor, own gain, sanitized 0..8) for OBS/Discord capture. + `setStreamOutputDevice` / `setStreamBus`; underflow/overflow counters + level meter exposed. +3. **Renderer bus** (Phase 2, exclusive-mode support): the renderer's WebAudio master mix is + pushed over IPC (`audio:pushRendererAudio`, fire-and-forget `ipcRenderer.send`) into a large + SPSC ring (65536 frames ≈ 1.5 s; producer is the jittery IPC thread), linear-resampled + producer-side to device rate, mixed into engine output. Keeps song/stem audio audible when the + output device is exclusive-style (ASIO / WASAPI exclusive) and the OS mixer path is silent. + Fed renderer-side by a whole-app `getDisplayMedia({audio})` loopback capture — + `setDisplayMediaRequestHandler` in `main.ts` grants it (audio-only, own-app loopback; other + apps' audio not captured). Current branch (`fix/loopback-capture-permission`) fixes the media + permission handler to allow this getDisplayMedia request. + +**Backing track**: JUCE `AudioFormatReaderSource` → `AudioTransportSource` buffered by a +`TimeSliceThread` read-ahead → optional signalsmith-stretch phase vocoder for speed change +(1x bypass path; lock-free speed handoff via pending atomic so slider drags never block the RT +tryLock). Playhead = accumulated heard frames minus stretcher latency; non-blocking cached +position/duration getters. Used for local audio-file playback; sloppak/HTML5-routed songs play +through the renderer's WebAudio instead (engine playhead frozen; the verifier is fed the +renderer's corrected playhead via `setPlayhead`). + +--- + +### 6. Detection & scoring (engine-side) + +- **PitchDetector** — monophonic YIN, sub-Hz parabolic interpolation, reads post-gate signal + (silent when gate closed). Backs the always-on home tuner (`audio:getRawPitch`). +- **MlNoteDetector** — polyphonic Basic Pitch ONNX model (`loadNoteModel`); armed only while a + consumer actually reads ML notes (`setMlNoteDetectionEnabled`) so ONNX inference isn't paid + otherwise. `getActiveDetection()` prefers ML when loaded, else YIN — same shape either way. +- **ChordScorer** — scores a renderer-supplied chord context against the input ring + (`audio:scoreChord`); ML-backed variant when the ML detector is live. +- **NoteVerifier** — background thread per source; renderer pushes the chart once (`setChart`), + verifier scores each note's timing window against the live playhead + input ring, renderer + drains verdicts (`getNoteVerdicts`). Replaced the per-tick scoreChord IPC loop that starved on + dense passages. Playhead offset = auto device-latency delta + user fine-tune. +- **OnsetDetector** — consumes the input ring gaplessly via `getInputSince`. + +--- + +### 7. Main-process integration + +- **audio-bridge.ts** (65K): loads `slopsmith_audio.node`, registers all 102 `audio:*` / + `audio-effects:*` IPC handlers, normalizes/persists device settings, wires vst-crash-guard + sentinels around VST loads and editor opens, forwards renderer-bus audio. +- **audio-effects-executor.ts**: the capability-pipeline backend for the `audio-effects` + capability. Accepts validated **chain plans** (`feedBack.audio_effects.chain_plan.v1`, legacy + `slopsmith.…` schema accepted): up to 24 stages of kind `nam | ir | vst | utility | bypass` + with roles (pedal/amp/cab/…), route keys (default `desktop-main`), gain sets, authorization + gating (`user-action` / `restore-selection` / `playback-session`). Translates plans into native + calls (loadPreset/clearChain/setBypass/setParameter/setGain/…) and reports structured outcomes + (`handled | degraded | failed | unavailable | no-target | user-action-required`). +- **preload.ts**: exposes the whole surface as `window.feedBackDesktop` (alias + `window.slopsmithDesktop`) — `audio.*` (~99 methods) + `audioEffects.*`. +- **NodeAddon.cpp**: N-API glue; libuv async workers for plugin loads (message thread keeps + pumping); sandbox-aware VST loading (`loadVstSandboxAware`); shutdown cancels pending loads. + +--- + +### 8. Bundled plugins that touch audio + +Plugins are renderer-side (screen.js + plugin.json manifest, capability-pipelines.v1). The ones +interacting with the engine: + +| Plugin | Interaction | +|---|---| +| **audio_engine** (bundled in this repo, `src/renderer/`) | The engine's own UI: device setup, chain editor, meters. Declares provider capabilities `audio-input`, `audio-mix`, `audio-monitoring`; observes `playback` lifecycle to rebuild tone automation / tear down native chain state. | +| **rig_builder** | Biggest consumer. Builds amp/cab/pedal rigs; declares `audio-effects` capability and submits chain plans (NAM stages, IRs, VSTs, per-stage gain/bypass/params) through audio-effects-executor. Also privileged-capabilities, jobs, library. Own repo also contains a VST (`vst/`) and tone-curation tooling. | +| **nam_tone** | NAM tone library (server-side): manages `nam_models/`, `nam_irs/`, `nam_tone.db` on the Python backend; models/IRs are what `audio:loadNAMModel` / `loadIR` consume. | +| **note_detect** | Real-time detection/scoring: arms ML detection, pushes charts (`setChart`), drains verdicts, reads pitch/raw frames, drives per-source scoring APIs. | +| **stems** / **stem_mixer** | WebAudio-side stem mix (`audio-mix` capability, mute/volume commands). Their master mix reaches the engine only via the renderer bus on exclusive-mode outputs. | +| **midi_amp** | Sends MIDI Program Change to external amps/modelers on tone switches (external gear path; engine-side per-slot MIDI exists via `audio:sendMidiToSlot`). | +| **tuner (built-in home tuner)** | Always-on YIN readout via `getRawPitch` / `getRawAudioFrame` — deliberately never pays ONNX cost. | +| **virtuoso / practice / minigames** | Consume detection results (verdicts/pitch) rather than driving the chain. | + +Plugin lifecycle: `plugin-manager.ts` installs/updates plugins as git checkouts under the +plugins dir (https-only remotes, path-safe names); restart of the Python backend activates them. + +--- + +### 9. Observations for the TLC pass (starting points) + +- `AudioEngine.cpp` (156K) and `NodeAddon.cpp` (160K) are monoliths; SourceChain extraction + ("Phase 0/2" comments) is mid-flight — multi-source fan-out phases still landing. +- Duplicated facade surface: engine forwards ~40 single-source methods to `source0()` while a + parallel `getSource(id)`-indexed API grows alongside (`audio:*` vs `audio:setSource*`). +- Sandbox v1 gaps documented in `SandboxedProcessor.h` (no parameter proxies, fixed stereo buses). +- Three separate SPSC ring implementations (outputPendingRing, renderer bus, stream sink) share + the packed-LR pattern — candidate for one templated ring. +- Backing-track transport is legacy for sloppak songs (renderer WebAudio does playback); the + frozen-playhead special case leaks into NoteVerifier via `setPlayhead`. +- Naming drift: slopsmith → feedBack rebrand half-done (addon name `slopsmith_audio.node`, + `slopsmith-vst-host.exe`, legacy schema ids, `window.slopsmithDesktop` alias). + +--- + +# Part II — Redundancy & Control Collisions +Every finding below was +verified in source; file references point at the current tree. + +Severity legend: 🔴 active conflict (two writers fight at runtime) · 🟠 dual ownership +(same setting settable from two places, last-writer-wins, no arbitration) · 🟡 redundancy +(duplicate surface/code, no runtime conflict yet). + +--- + +### 1. 🔴 Signal chain has three independent writers + +The native `SignalChain` is a single global resource, but three parties load/clear it: + +1. **audio_engine bundle** (`src/renderer/screen.js`): direct `api.loadVST` / `loadNAMModel` + / `loadIR` / `loadPreset` / `clearChain` (21 `clearChain` call sites), plus its own tone + auto-switch/automation (`applyToneMappingsNow`, `applyToneAutomationFor`, + `_restorePresetBlob` → `clearChain` + `loadPreset`). +2. **rig_builder via capability pipeline**: `audioEffects.loadPlan` → main-process + `audio-effects-executor.ts` → `nativeAudio.loadPreset`. +3. **rig_builder legacy direct path**: `feedBackDesktop.audio.loadPreset` (tracked in its own + telemetry as `audio-effects.legacy-native-load`). + +Concrete evidence of the fight (rig_builder `screen.js`): + +> "PROACTIVE TRANSIENT KILL: the bundle calls loadPreset ~1ms after we return this response. +> We can't monkey-patch `feedBackDesktop.audio.loadPreset` … the object is frozen by +> contextBridge" + +rig_builder ships timing hacks (`_rbUnmuteTimer`, transient kill, fallback unmute) purely to +survive the bundle re-loading the chain right after it did. That is two plugins racing on the +same native chain with wall-clock heuristics as the arbiter. + +**Additional executor-state hazard**: the executor keeps a `routes` map with +`stageSlots` (stageId → native slotId). Any direct `loadPreset` / `clearChain` / +`removeProcessor` / `moveProcessor` from path 1 or 3 invalidates those slot ids silently — +subsequent `setStageBypass` / `setStageParameter` / `activateSegment` then flip +bypass/params on the **wrong slots** (slot ids are reused sequentially by `nextSlotId`) or +return `no-target`. Nothing detects the divergence. + +### 2. 🔴 Monitor mute: five writers, one atomic, persisted preference gets clobbered + +`SourceChain::monitorMuted` writers: + +| Writer | Where | When | +|---|---|---| +| audio_engine settings UI checkbox | `screen.js` (`ae-monitor-mute`) | user toggle; persisted in device settings | +| startup restore | `screen.js` ~901 | pushes saved value into engine on boot | +| executor preload-mute | `audio-effects-executor.ts:479-489` | saves `previousMonitorMute`, forces mute/unmute during chain load, restores on a `setTimeout` ramp | +| executor `releaseRoute` | `:616` | **unconditionally** `setMonitorMute(true)` + `setMonitorMuteSuppressed(false)` | +| renderer song-load suppression | `screen.js` (2 sites) + `audio:setMonitorMuteSuppressed` | temporary override around chain rebuild | + +Collisions: + +- `releaseRoute` forces mute=true regardless of the user's persisted `monitorMute:false` + preference — the checkbox UI and the engine now disagree until the next toggle/restart. +- The executor's read-modify-restore (`previousMonitorMute` + delayed `schedulePreloadRestore`) + races a user toggling the checkbox during the hold window: the restore overwrites the fresh + user choice with the stale snapshot. `preloadRestoreVersion` guards against *newer executor + loads*, not against other writers. +- `monitorMuteSuppressed` is set by both the renderer (song load) and executor flows with no + refcount — whoever clears last wins; overlapping windows un-suppress early. + +### 3. 🔴/🟠 Gain: four knobs, three surfaces, inconsistent clamping + +Native gains: per-source `inputGain`, per-source `chainOutputGain`, global `outputGain` +(master), `backingVolume` — all reachable through `audio:setGain(which, value)` +(`NodeAddon.cpp SetGain`), and `input`/`chain` also through +`audio-effects:setRouteGain` + chain-plan `options.gains` + `preloadMute.targetGain`. + +- **Dual ownership of `chain` gain**: audio_engine screen sets it (9 `setGain` sites); + the executor zeroes it (`trySetGain('chain', 0)` on load and on `releaseRoute`) and later + ramps it to `targetGain` (default **1**, or plan-supplied) on a timer. If the user (or tone + automation) set chain gain meanwhile, the ramp silently overwrites it. Same + stale-snapshot race as monitor mute. +- **Clamp inconsistency**: executor clamps to `0..32` (`clampGain`); `NodeAddon::SetGain` does + **no** validation — `NaN`/`Infinity` from any direct `audio:setGain` caller reaches + `outputGain.store()` / `inputGain.store()` raw. The engine sanitizes only the *stream* and + *renderer-bus* gains (`sanitizeStreamGain`, 0..8, explicitly "so a NaN/Inf from JS can never + reach the ring") — the exact same hazard is unguarded for master/input/chain/backing. + A NaN master gain silences output and poisons the peak meters. +- Per-slot `postGain` overlaps conceptually with `chainOutputGain` (both are "level after the + amp"): rig plans carry per-stage loudness trims while the screen's chain gain scales the + same signal — two normalization layers, no documented ownership. + +### 4. 🟠 Device settings: two persistence stores, newest-timestamp arbitration + +`screen.js loadDeviceSettings()` merges **file-backed** settings (main process, +`audio:saveDeviceSettings`) with **`localStorage['slopsmith-audio-device']`**, picking +whichever has the newer `savedAt`. Two stores for one setting means: + +- A main-side migration/reset (`config-reset.ts` territory) leaves stale localStorage that can + win the timestamp race and resurrect wiped settings. +- `monitorMute` / `monitorKill` ride inside the *device* settings blob, so a device re-save + from one path re-persists mute flags captured from checkbox state at that moment — + interleaving with §2's runtime writers. +- Renderer keeps 10 `slopsmith-*` localStorage keys total (`slopsmith-signal-chain`, + `slopsmith-chain-presets`, `slopsmith-tone-automation`, …) — the chain is *also* persisted + renderer-side while rig_builder persists rigs server-side (`routes.py` / DB): two saved + descriptions of the same chain that can disagree on restore. + +### 5. 🟡 Legacy alias surfaces (three layers deep) + +Same setting, multiple entry points kept for back-compat — each a place for behavior to drift: + +- **Engine facade**: `getDeviceManager()` ≡ `getInputDeviceManager()`; + `setInputDeviceType()` ≡ `setDeviceType()`; `DeviceOptions.type` ≡ `inputType`; + single-source methods (`setInputGain`, `setMonitorMute`, `setChart`, `scoreChord`, ~40 of + them) forward to `source0()` while a parallel indexed API (`getSource(id)` → + `audio:setSource*`) does the same thing for id 0. Two IPC routes mutate the same atomic + (`audio:setMonitorMute` vs `audio:setSourceMonitorMute(0, …)`). +- **Settings shape**: legacy `{type}` vs `{inputType, outputType}` normalized in **two + places** — `audio-bridge.ts normalizeDeviceSettings` *and* `screen.js + normalizeDeviceSettings` (duplicated logic, must stay in sync by hand). +- **Schema/branding**: `feedBack.audio_effects.chain_plan.v1` + accepted legacy + `slopsmith.…` id; `window.feedBackDesktop` + `window.slopsmithDesktop`; localStorage keys + still `slopsmith-*`. Each alias doubles the grep surface for every future change. + +### 6. 🟡 Duplicated implementation code + +- **Three packed-LR SPSC rings** in `AudioEngine.h` (split-mode `outputPendingRing`, renderer + bus, stream sink) — same pack/unpack, same power-of-two asserts, same drop-oldest logic, + three hand-maintained copies. One templated ring kills ~2/3 of the index math. +- **Two fail-soft wrappers per method** in the JS layer: audio-bridge's typeof-guarded + handlers and the executor's `trySetGain`/`trySetMonitorMute`/… re-wrap the same native + calls with slightly different error policy (bridge: silent no-op; executor: outcome + strings). A single native-call helper with one policy would remove a class of divergence. +- **`normalizeLoadResult` tolerance duplicated**: both rig_builder (`screen.js`: "Some JUCE + bridges return {success:false} or bare …") and the executor normalize loadPreset results + independently. +- **Chain-restore logic**: executor rollback (`rollbackPreset` + `restorePreset`) vs + screen.js `_restorePresetBlob` — two snapshot/rollback implementations for the same chain. + +### 7. 🟠 `startAudio` / route lifecycle from two sides + +`audio:startAudio` is invoked by the renderer UI **and** best-effort by the executor when a +chain plan carries `startAudio: true` (`:574`). Neither side knows the other's intent; there's +no matching stop ownership — `releaseRoute` clears the chain and mutes but leaves the device +running or not depending on who started it. + +--- + +### Recommended direction (for TLC scoping, not yet implemented) + +1. **Single chain owner**: make the audio-effects executor the *only* writer of the native + chain; migrate the audio_engine screen's direct loadVST/loadPreset/tone-switch calls onto + route-scoped executor operations; then delete rig_builder's transient-kill hacks and the + legacy direct `loadPreset` path. Executor should reject/re-sync when + `getChainState` disagrees with its `stageSlots` map (generation counter on the native chain). +2. **Arbitrated monitor state**: replace raw `setMonitorMute` writes with a small state owner + (user preference + N stackable suppressions/overrides, refcounted). `releaseRoute` releases + its override instead of forcing `true`. +3. **Sanitize all gains natively**: extend `sanitizeStreamGain`-style clamping to + input/chain/output/backing in `AudioEngine` setters (single choke point) and drop the + JS-side clamp divergence. +4. **One persistence store per setting**: file-backed settings as the single source; treat + localStorage as a migration source only, delete after import. Move mute flags out of the + device blob. +5. **Deprecation plan for aliases**: freeze `slopsmith*` surfaces, log-once on use, remove on + next major. + +--- + +# Part III — Deep Read: AudioEngine.cpp + NodeAddon.cpp + +Full read of `src/audio/AudioEngine.cpp` (3223 lines) and the load-bearing regions of +`src/audio/NodeAddon.cpp` (3699 lines). Line refs current as of +`fix/loopback-capture-permission`. + +**Overall verdict first**: the RT core is in much better shape than its size suggests — +disciplined lock-free SPSC rings, no allocation on the audio thread, denormal flushing on every +callback clock, a correct per-deviceKey quiescence handshake for source removal, and unusually +good comments that cite the bug each guard fixes. The problems live at the *edges*: the JS↔native +boundary, concurrency between async workers, and inconsistent input sanitization. + +--- + +### 1. 🔴 Chain-mutating async workers are not serialized (NodeAddon) + +`LoadPresetWorker`, `LoadVSTWorker`, `LoadNAMWorker`, `LoadIRWorker`, `ReplaceIRWorker` all queue +on the libuv threadpool (default 4 threads) with **no mutual exclusion between workers**. +`SignalChain` locks per-operation only, so the sequence `clear() → addProcessor() × N` +(`NodeAddon.cpp:3312-3408`) is not atomic. + +Two `loadPreset` calls in flight — which is precisely the documented rig_builder-vs-bundle +"~1ms later" race from the collisions doc — can interleave as: + +``` +worker A: clear() worker B: clear() +worker A: add(ampA) worker B: add(ampB) +worker A: add(irA) → final chain: [ampA, ampB, irA, irB] (merged garbage) +``` + +Both report `success:true` with wrong `slotsLoaded` semantics; the executor's stageId→slotId map +is then built against a chain that neither caller described. A `loadVST` concurrent with a +`loadPreset` similarly lands a slot into (or after) someone else's rebuild. + +**Fix shape**: one native "chain mutation" mutex (or a serial dispatch queue) around +clear+rebuild and single-slot adds; alternatively a chain generation counter returned to JS so +callers detect they lost the race. This is the single highest-value fix of the whole pass — +it converts the plugin-vs-plugin fight from corruption to last-writer-wins. + +### 2. 🔴 Argument sanitization is inconsistent across the N-API surface + +The addon knows the hazard — `getValidatedSource` (`NodeAddon.cpp:89-103`) documents that +`Int32Value()` coerces NaN→0, and `setAudioDevices` normalizes sampleRate against "NaN slipping +past N-API" (`AudioEngine.cpp:700-708`). But that rigor is only applied to the *newer* bindings: + +| Guarded (fail-soft) | Unguarded (blind `As<>()` coercion) | +|---|---| +| `getValidatedSource` (all `*Source*` methods) | `SetGain` — NaN/Inf reaches `outputGain.store()` raw | +| `SetSlotState` (IsNumber/IsString checks) | `SetParameter`, `SetBypass`, `RemoveProcessor`, `MoveProcessor` — NaN slotId → **slot 0** | +| `SetMonitorMuteSuppressed`, `SetMonitorKill` (bridge-side Boolean coercion) | `SetMultiBypass` (per-item `As` uncheck) | +| `setBackingSpeed` (isfinite + clamp, engine-side) | `SendMidiToSlot` (channel/program unclamped → JUCE assertions) | + +Consequences of the worst one: a NaN master gain via `audio:setGain('output', NaN)` multiplies +the entire device output to NaN (`buffer.applyGain(outputGain.load())`, +`AudioEngine.cpp:2407/3083`) — full silence plus poisoned peak meters, and nothing scrubs it +(the per-source NaN scrub runs *before* the master gain). Engine-side clamps at the four gain +setters (mirroring `sanitizeStreamGain`) fix every caller at once. + +### 3. 🔴 `wasRunning` race in `setAudioDevices` (AudioEngine.cpp:658) + +`audioDeviceStopped()` clears `audioRunning` on **transient** stops, and the code's own comment +says WASAPI exclusive opens "routinely fire one mid-start". `setAudioDevices` captures +`wasRunning = audioRunning.load()` and only calls `startAudio()` at the end when it was true. +The comment above it (`:651-657`) fixes the *detach* half of this race (stopAudio is now +unconditional) but the *restart* half still reads the racy flag: a reconfigure landing inside a +transient-stop window sees `wasRunning == false` and leaves the engine configured but stopped — +"no audio until user presses Start/Apply again". The intent flag it should read is "did the user +want audio running", which currently doesn't exist separately from device state (see §6). + +### 4. 🟠 `setRendererBus(false)` violates the ring's own SPSC discipline + +`AudioEngine.h` (`setRendererBus`) drops buffered audio on disable by writing +`rendererBusReadIndex` from the **control thread**, while `pullRendererBus` +(`AudioEngine.cpp:3143-3208`) is the designated single consumer-side writer of that index (the +file's comments elsewhere are explicit that "only the consumer ever moves readIndex"). A +concurrent output callback mid-`pullRendererBus` can overwrite the control thread's store with +`r + pull`, replaying a stale tail after re-enable — exactly what the drop was meant to prevent. +Low probability, audible-blip severity; fix by setting a "flush requested" atomic the consumer +honors instead of writing its index. + +### 5. 🟠 Latency accounting has three unreconciled truths + +- `getLatencyMs()` (`:469-496`): device latencies + (split only) a static `kOutputRingFrames/2` + ≈ 42.7 ms ring-residency guess. The actual ring fill is measurable (`getDeviceMetrics` reports + it) but not used. +- Verifier auto-offset (`extraInputAboutToStart`, `:2554-2568`): per-device *input-latency + delta* only, 0 on JACK/PipeWire (documented), user offset summed on top. +- Renderer bus: adds `kRendererBusPrimeFrames` (~10 ms) prime + fill drift + producer-side + resample, none of it surfaced in any latency figure; stems audio through the bus is delayed by + an amount the UI never reports and the verifier never compensates. + +For a TLC pass: one `getLatencyBreakdown()` that owns all terms would replace three ad-hoc sums. + +### 6. 🟠 `audioRunning` conflates user intent with device state + +Writers: `startAudio`/`stopAudio` (user intent), `audioDeviceAboutToStart` (device came up — +including JUCE auto-restarts the user never asked for, `:1802`), `audioDeviceStopped` (device +went down — including transient stops the user didn't ask for). Readers assume different +meanings: `setAudioDevices` reads it as intent (§3), detection guards read it as device state +(correct), the bridge's `isAudioRunning` surfaces it to the UI as intent. Two booleans +(`userWantsAudio`, `deviceRunning`) would kill the §3 race and make the auto-restart paths +self-explanatory. Related: `stopAudio()` does not stop the backing transport — `backingPlaying` +stays true and playback resumes on the next start, which is intentional for unplug-recovery but +surprising for an explicit user stop. + +### 7. 🟡 Probe/apply duplication — three copies of the rate-tolerance logic + +The `|r - r2| <= 0.5` sample-rate matching + round-to-nominal logic exists in +`probeDeviceOptionsDual` (`:316-350`), `applySplitSetup::rateSupportedBy` (`:961-982`), and the +post-open verify (`:1146`). The comments at each site narrate keeping the three in sync by hand +("<= 0.5 (not <) to match…", "Tolerance matches the probe-side rounding…") — i.e. they've +already been bitten. Same story for empty-name→first-enumerated resolution (probe, preflight, +apply must agree; three sites). One shared helper each. + +### 8. 🟡 Device identity is display-name only (documented limitation) + +`getBindableInputDevices` (`:120-161`) and `bindInputDevice`'s duplicate/primary checks compare +`juce::String` names. Two identical interfaces collapse to one entry; a device exposed under two +backends may bind the wrong one. The comment block is honest about it; flagging here because the +fix ((typeName, name) identity threaded through bind/reopen/persistence) also touches the +renderer's saved settings — a cross-layer change worth scheduling deliberately. + +### 9. 🟡 NodeAddon miscellany + +- **`LoadPresetWorker` state restore bypasses `setSlotState`**: it `const_cast`s the slot from + `getSlot()` and calls `slot->setState(state)` directly (`:3402-3404`) — outside whatever + synchronization `SignalChain::setSlotState` provides against a concurrently-processing audio + thread. During a preset load the chain was just rebuilt so the window is small, but it's the + only chain mutation in the file that dodges the class's own API. +- **Every preset load closes every editor window** (`LoadPreset:3452`, `ClearChain:2668` — + required by the #56 use-after-free). Combined with the tone auto-switch calling loadPreset on + song events, a user tweaking an amp editor mid-song has the window yanked. A single-slot + replace path (`replaceProcessor` exists) for tone switches would avoid the nuke. +- **macOS is a second-class citizen by design**: no JUCE dispatch loop (`startJuceMessageThread` + JUCE_MAC branch), `dispatchOnMessageThread` runs inline, VST/AU instantiation "given up until + a proper libuv-based pump lands". Every load path carries a divergent `#if JUCE_MAC` branch — + a large, mostly-untested platform fork woven through the file. +- **`loadVstSandboxAware` holds a libuv worker for the whole plugin init** (documented tradeoff, + `:2241-2248`); concurrent slow loads can starve fs/crypto AsyncWorkers. +- **Misnomer**: `inputOverflowCount` is incremented by the *output consumer's* catch-up on the + primary split ring (`:2941`) — it counts ring overruns, not input overflows; the metric name + leaks into `DeviceMetrics`/diagnostics. + +### 10. What is genuinely solid (don't "fix") + +- The per-deviceKey `callbacksInFlight` handshake + `pendingRelease` deferral for source removal + (`:1554-1618`) — correct, well-reasoned, and the 200 ms bounded wait is the right call. +- Ring discipline: packed-LR single-atomic frames, consumer-side drop-oldest, `w < r` resync + after index resets, consume-vs-pull split to avoid clock skew after clamps (`:2944-2974`). +- RT allocation hygiene: every scratch pre-sized in about-to-start, every hot path clamps to + capacity instead of resizing; stream scratches deliberately fixed at ring capacity so a + hotplug about-to-start can't realloc under a live producer (`:1830-1841`). +- `ScopedNoDenormals` on *both* callback clocks with the explanation of why (`:2268-2273,2898`). +- Backing speed hand-off (pending atomic + same-block stretcher reset, `:1670-1691`) and the + read-ahead thread rationale, including the honest note about `BufferingAudioSource`'s residual + lock (`:1334-1341`). +- `bindInputDevice`'s failure hygiene: every abort path closes the half-open device; validate- + eagerly-then-close when the engine is stopped (`:2774-2783`). + +--- + +### Priority order for the TLC pass + +1. **Serialize chain mutations** (§1) — prerequisite for any single-chain-owner work from the + collisions doc; without it the executor can't even trust its own load result. +2. **Sanitize gains + slot ids natively** (§2) — small, mechanical, kills a user-visible + silence-the-app bug class. +3. **Split `audioRunning` into intent + state** (§6, fixes §3) — unlocks correct + reconfigure-under-transient-stop and clarifies every auto-restart path. +4. **Renderer-bus flush flag** (§4) — one-line-ish, closes the last SPSC discipline hole. +5. **Latency breakdown API** (§5) and **probe/apply shared helpers** (§7) — quality-of-life, + schedule with the settings-ownership work. + +--- + +# Part IV — Decomposition / Refactor Plan +Targets the two monoliths: +`AudioEngine.{h,cpp}` (819 + 3223 lines) and `NodeAddon.cpp` (3699 lines). +Builds on the findings in Part II and Part III — +several fixes there (chain-mutation serialization, gain sanitization, intent/state split) +get a natural home in the new units instead of being bolted onto the monolith. + +**Precedent**: the SourceChain extraction already proved the working method on this codebase — +move a cohesive member cluster verbatim into a class, bind shared engine atomics by reference, +keep the facade byte-identical, land in phases. This plan repeats that recipe seven more times. + +**Prime directive**: no behavior change per phase. Every phase is a pure code move that +compiles + passes the existing tests (`tests/audio_sanitize`, `tests/sandbox/*`, e2e) before +the next starts. Bug fixes ride in separate commits on top of the phase that creates their home. + +--- + +### 1. Target layout + +``` +src/audio/ + engine/ + AudioEngine.{h,cpp} facade + callback orchestration only (~500 lines total) + DeviceSetup.{h,cpp} probe/apply/teardown for duplex + split (§2.1) + SourcePool.{h,cpp} source add/remove/reclaim + in-flight counts (§2.2) + ExtraInputs.{h,cpp} InputDeviceSlot registry, bind/unbind/reopen (§2.3) + BackingPlayer.{h,cpp} transport + stretch + leveler + playhead (§2.4) + StreamSink.{h,cpp} 2nd output device + submix compose (§2.5) + RendererBus.{h,cpp} WebAudio→engine ring, push/pull/metrics (§2.6) + PackedStereoRing.h the one SPSC ring template (§2.7) + EngineState.h shared atomics: rates, block sizes, run state(§2.8) + addon/ + NodeAddon.cpp module init + binding registration only + AddonContext.{h,cpp} engine/vstHost lifetime, message thread, shutdown latch + NapiHelpers.h arg validation (the getValidatedSource pattern, generalized) + ChainOps.{h,cpp} chain workers (LoadPreset/VST/NAM/IR) + mutation queue + DeviceBindings.cpp device enumeration/config/metrics bindings + ControlBindings.cpp gain/mute/gate/stream/renderer-bus bindings + DetectionBindings.cpp pitch/chord/chart/verdict/source-indexed bindings + BackingBindings.cpp backing-track bindings + EditorWindows.{h,cpp} PluginEditorWindow + open/close/promotion + (existing DSP files stay where they are) +``` + +CMake: append the new files to the existing source list in `src/audio/CMakeLists.txt`; no +target restructuring needed. + +--- + +### 2. AudioEngine decomposition (one phase per unit) + +Ordering is by extraction risk, lowest first. Each unit lists what moves, its boundary, and +which known bug lands in it afterwards. + +#### 2.1 `PackedStereoRing` — first, everything else builds on it + +Template over capacity; owns `array>`, write/read indices, the pack/unpack +helpers, and the three ritual moves currently copy-pasted at six sites: producer publish +(`packStereoIntoRing`), consumer `w < r` resync, lapped catch-up with overflow counter, and the +pull-vs-consume split. Replaces: `outputPendingRing`, each `InputDeviceSlot::ring`, +`streamSink.ring`, `rendererBusRing`. The static_asserts move inside the template. +**Bug fixed here after the move**: none — but §2.6's flush fix becomes a one-method addition +(`requestFlush()` honored by the consumer) instead of index surgery. + +#### 2.2 `SourcePool` + +Moves: `sources[]` array, `sourcesMutex`, `callbacksInFlight[]`, `pendingRelease[]`, +`addSource/removeSource/reclaimPendingReleases/listSources/getSource`, the fan-out helpers +(`setMlNoteDetectionEnabled`, `setMonitorKill` loop, `resetPeaks` loop), and +`mixSourcesForDevice`. Boundary: callbacks call `pool.enterCallback(deviceKey)` / +`pool.exitCallback(deviceKey)` (RAII guard) and `pool.mixForDevice(...)`. +This is the most delicate move (RT-shared state) but it is also the best-commented, most +self-contained cluster — the handshake logic doesn't touch any other member. + +#### 2.3 `ExtraInputs` + +Moves: `InputDeviceSlot` + `extraInputs[]`, `bindInputDevice/unbindInputDevice/ +closeExtraInputDevice/reopenDesiredExtraInputs/activeExtraInputCount`, the extra callback +trio (`extraInputCallback/AboutToStart/Stopped`). Depends on SourcePool (prepares/releases +sources by deviceKey) and PackedStereoRing. The (typeName, name) device-identity fix +(deep-read §8) lands here later without touching the engine again. + +#### 2.4 `BackingPlayer` + +Moves: transport + reader + read-ahead thread, signalsmith stretch state, `backingLock`, +speed hand-off atomics, `BackingLeveler`, `renderBackingBlockLocked`, all playhead caches, +load/start/stop/seek/speed. Boundary: `backing.renderInto(buffer, numSamples)` returning frames +(caller mixes + meters), `backing.prepare(sr, bs)` from the about-to-start hooks. The duplex and +split callbacks already share `renderBackingBlockLocked`, so the seam exists. +**Lands here later**: the "stop engine ≠ stop backing" intent decision (deep-read §6 note). + +#### 2.5 `StreamSink` + +Already 80% a struct — promote to a class owning its manager, callback, ring, scratches, +`composeAndPushStreamMix`, `set/clear/reopen/close`. Bus flags (`includeBacking/includeGuitar/ +gain`) move in. The engine's callbacks call `sink.publish(guitarMix, backing, renderer, n)`. + +#### 2.6 `RendererBus` + +Moves: ring + indices + resampler carry-state (`rendererBusSrcPos/PrevL/PrevR`), prime/fill +constants, `pushRendererAudio/pullRendererBus/getRendererBusMetrics/setRendererBus`. +**Bug fixed here after the move**: the control-thread readIndex write on disable (deep-read §4) +becomes an atomic `flushRequested` flag consumed in `pull()`. + +#### 2.7 `DeviceSetup` + +Moves: `probeDeviceOptions[Dual]`, `applyDuplexSetup`, `applySplitSetup`, `teardownSplitMode`, +type resolution/preference tables, and the three hand-synced helpers extracted once: +`rateIntersection()`, `bufferIntersection()`, `resolveDeviceName()` (deep-read §7). Stateless +apart from references to the two managers + EngineState; takes managers by reference so it owns +no lifetime. `setAudioDevices` stays on the facade as the orchestrator (stop → resolve → +duplex-or-split → restart) but shrinks to ~40 lines. + +#### 2.8 `EngineState` + +Tiny header: `currentSampleRate`, `inputBlockSize`, `outputBlockSize`, `duplexMode`, and — +**the deliberate fix from deep-read §3/§6** — `userWantsAudio` (intent, written only by +start/stopAudio) split from `deviceRunning` (state, written by the device callbacks). +SourceChain already binds engine atomics by reference; it re-binds to this struct unchanged. +Every unit above takes `EngineState&`, which is what keeps them unit-testable without JUCE +devices (hand them a state struct + a fake ring). + +#### What remains on `AudioEngine` + +The `AudioIODeviceCallback` implementations (now ~60 lines each: enter pool guard → mix → +backing → renderer bus → sink publish → master gain → meters), the facade forwarding to +`source0()` (unchanged for NodeAddon compatibility), device enumeration getters, and +construction/destruction ordering. Header drops from 819 lines to roughly 250. + +--- + +### 3. NodeAddon decomposition + +The file is 100+ bindings sharing four bits of infrastructure. Split infrastructure first, +then the bindings become mechanical moves. + +#### 3.1 `AddonContext` — lifetime + threading + +Moves: `engine/vstHost` globals + mutexes + `snapshotEngine/snapshotVstHost`, the JUCE message +thread (`startJuceMessageThread/stop/dispatchOnMessageThread` with the macOS fork in ONE place), +`alreadyShutDown`, `doShutdown`, `registerPendingLoad/cancelAllPendingLoads`. Everything else +receives `AddonContext&`. This quarantines the `#if JUCE_MAC` platform fork (deep-read §9) into +a single file instead of a branch inside every load path. + +#### 3.2 `NapiHelpers.h` — kill the validation inconsistency structurally + +Generalize the `getValidatedSource` pattern into typed extractors: + +```cpp +std::optional argSlotId(info, i); // finite integer, >= 0 +std::optional argGain(info, i); // finite, clamped 0..8 (one policy) +std::optional argParamValue(info, i); // finite, clamped 0..1 +std::optional argBool(info, i); +``` + +Then rewriting `SetGain/SetParameter/SetBypass/SendMidiToSlot/SetMultiBypass` onto them is the +deep-read §2 fix, done once, enforced by convention (new bindings have no raw `As<>()` path to +copy). Engine-side clamps in the gain setters stay as the second belt. + +#### 3.3 `ChainOps` — the serialization point + +Moves: all five chain workers + `loadVstSandboxAware` + `decodeStateBlob`. Adds the +**chain-mutation serializer** (deep-read §1): a single `std::mutex chainMutationMutex` acquired +for the full Execute() of every worker, plus a monotonic `chainGeneration` bumped on every +mutation and returned in load results — the executor and renderer can then detect a lost race +instead of trusting a corrupted merge. The `const_cast` slot-state bypass (deep-read §9) is +replaced with `setSlotState()` during this move. + +#### 3.4 `EditorWindows` + +Moves: `PluginEditorWindow`, the window map, open/close/destroy-on-message-thread, and the +sandbox-promotion flow inside `OpenPluginEditor`. Later improvement lands here: tone-switch +single-slot replace instead of close-all-editors (collisions/deep-read editor-nuke issue). + +#### 3.5 Binding files + +`DeviceBindings/ControlBindings/DetectionBindings/BackingBindings` — pure moves, grouped to +match the preload API sections, each ~400-600 lines. `NodeAddon.cpp` keeps only `Init/Shutdown` +and the `exports.Set(...)` table (which doubles as the API index the current file lacks). + +--- + +### 4. Phase 0 — Compatibility contract & test scaffolding + +Runs BEFORE any code moves. Purpose: turn "no public API change" from a review rule into a +failing CI check, and codify the compat decisions consumers (core screen, rig_builder, +note_detect, stems) depend on. Test infrastructure follows the repo's existing two-track +convention: native `tests//test.cpp` targets registered in `tests/CMakeLists.txt`, and +Node `tests/*.test.js` for the JS/addon boundary. + +**0.a Contract snapshots (`tests/contracts/`)** + +| Snapshot | Source | How | +|---|---|---| +| `addon-exports.json` | `slopsmith_audio.node` export table | Node script: `Object.keys(require(addon)).sort()` | +| `preload-audio-api.json` | `window.feedBackDesktop.audio.*` + `audioEffects.*` key lists | static extraction from `preload.ts` | +| `ipc-channels.json` | every `ipcMain.handle`/`ipcMain.on` name in `audio-bridge.ts` | static extraction | +| `result-shapes.json` | golden key/type shapes (not values) for `loadPreset`, `loadVST`, `loadNAM/IR`, `getChainState`, `savePreset`, `getDeviceMetrics`, `getRendererBusMetrics`, and every executor outcome (`loadChainPlan`/`releaseRoute`/`setRouteGain`/…) | run against the real addon (null audio device) + executor with a stubbed native | + +CI job `contract-check` regenerates all four and diffs against the committed snapshots. +Additive keys require a deliberate snapshot update in the same PR; removals/renames fail. + +**0.b Compat decisions codified as tests** + +- `isAudioRunning` reports **device state** (current semantics) — pinned by a test across a + simulated transient stop, so the phase-1 intent/state split can't silently change it. +- Native gain clamp bounds = **0..32** (matching the executor's `clampGain`), NaN/Inf rejected + universally; only stream/renderer-bus gains keep the tighter 0..8 `sanitizeStreamGain`. + Pinned by a table-driven test so phase 8's clamps can't under-shoot a legit rig gain. +- Concurrent `loadPreset` storm test written NOW (expected-fail / quarantined): two overlapping + loads must end with the chain equal to exactly one caller's preset. Documents today's + corruption, flips to expected-pass at phase 7, and doubles as the rig_builder timing smoke + (its transient-kill/unmute heuristics tolerate serialized latency — assert its fallback + unmute path still fires). + +**0.c Native unit-test harness** + +Add a `tests/engine_units/` CMake target (same pattern as `tests/audio_sanitize`) that links the +audio sources without a real device — the home for every per-unit test below. Add a tiny +`FakeClock`/`NullDevice` helper pair here once; all later phases reuse them. + +### 5. Bespoke tests per phase (unit + integration) + +Each extraction phase ships WITH its tests in the same PR — the unit tests pin the moved logic, +the integration gate proves the seam. "U" = `tests/engine_units` C++ test, "I" = Node/e2e. + +| Phase | Unit tests (new) | Integration tests | +|---|---|---| +| 1 `PackedStereoRing` | U: SPSC threaded stress (producer/consumer at different block sizes); wrap + drop-oldest lap; `w < r` resync after index reset; L/R tear check under lap (packed-atomic invariant); pull-vs-consume skew accounting; overflow/underflow counters | I: existing audio smoke (duplex, split, stream, renderer bus pass audio); contract-check green | +| 1 `EngineState` | U: intent/state transition table — user start/stop × device aboutToStart/stopped × transient stop; `isAudioRunning` compat pin from 0.b stays green | I: reconfigure-during-transient-stop scenario (documents deep-read §3; expected-fail until phase 8) | +| 2 `RendererBus` | U: resampler continuity across pushes (fractional pos + carried frame → no discontinuity at chunk seams); equal-rate degenerate path bit-exact; prime gate (no output until ~10 ms); underflow → silence + re-prime; fill clamp trims to prime target; flush-on-disable drops tail (expected-fail until phase 8 flag fix); metrics arithmetic | I: `getRendererBusMetrics` shape + push/consume accounting via addon against null device | +| 2 `StreamSink` | U: submix compose matrix (guitar/backing/renderer × include flags × gain); oversized-block skipped AND counted; scratch-not-sized skip is silent-safe | I: OBS-capture manual smoke; stream under/overflow counters via IPC | +| 3 `BackingPlayer` | U (synthetic reader source): speed change adopts rate + stretch reset in same block; EOF short-read playhead clamp; stretch-latency compensation vs 1× bypass; leveler re-prepare on SR change; tryLock-miss drops block without state damage | I: existing backing play/seek/speed e2e, duplex + split; `audio-chain-persistence.test.js` green | +| 4 `DeviceSetup` | U: `rateIntersection`/`bufferIntersection`/`resolveDeviceName` helpers — incl. the 0.5 Hz boundary cases the three duplicated sites hand-narrate today, midpoint-rounding fail-closed cases, empty-name resolution parity | I: manual device matrix (WASAPI shared/exclusive, ASIO, dual-type split); probe verdict == apply outcome assertion in a scripted run | +| 5 `SourcePool` | U: threaded add/remove storm under a fake callback loop — per-deviceKey quiescence handshake, deferred release + reclaim, no release while in-flight (TSAN job on this target); active-snapshot consistency in `mixForDevice` | I: `multi-source.test.js` + `tests/sandbox` e2e (GP-5 scenario), remove-under-load | +| 5 `ExtraInputs` | U: bind rejection matrix (duplicate name, primary device, duplex mode, out-of-range key); transient close keeps intent, permanent unbind clears + deactivates; reopen-failure ghost-source cleanup | I: second-interface e2e; meters zeroed while device gone | +| 6 `NapiHelpers` | I (Node, real addon): table-driven arg fuzz per extractor — NaN/Inf/negative/string/missing/object for slot ids, gains, params, midi bytes → no crash, documented no-op or clamp; pins the 0.b clamp decisions | I: addon init→shutdown→init cycle; pending-load cancellation on shutdown | +| 7 `ChainOps` | U: serializer — N threads × (loadPreset/loadVST/clearChain) → final chain equals exactly one caller's request, `chainGeneration` strictly monotonic, per-caller result reports the generation it produced | I: 0.b storm test flips to expected-pass; `audio-effects-executor.test.js` extended — executor detects stale generation; rig_builder legacy-path telemetry smoke; editor open/close + sandbox promotion e2e | +| 8 bug fixes | U: gain clamp tables (0..32 native, NaN reject); renderer flush flag; `wasRunning` intent read (phase-1 expected-fail flips to pass); latency breakdown terms sum | I: full regression: all snapshots + all suites green | + +Cross-cutting: + +- **TSAN/ASAN lane** for `tests/engine_units` in CI (the ring/pool tests are exactly what + sanitizers are for; the RT code has never had one). +- **Expected-fail discipline**: known bugs get their test at the phase that creates the home, + marked expected-fail with the deep-read § reference; the fix commit flips the mark. No fix + lands without its test having existed first. +- Existing suites (`audio_sanitize`, `chordscorer`, `mlnotedetector`, `sandbox/*`, + `*.test.js`) run on every phase — they are the behavior-freeze net. + +### 6. Phasing & verification + +| Phase | Content | Risk | Gate | +|---|---|---|---| +| **0** | **Contract snapshots + compat pins + `tests/engine_units` harness + storm test (expected-fail)** | **none (test-only)** | **`contract-check` job green; snapshots committed; harness builds on all 3 platforms** | +| 1 | `PackedStereoRing` + `EngineState` (incl. intent/state split behind a facade-compatible `isAudioRunning`) | low | phase-1 unit tests + audio smoke: duplex, split, stream sink, renderer bus each pass audio | +| 2 | `RendererBus`, `StreamSink` | low | phase-2 unit tests; renderer-bus metrics unchanged in diag build; OBS capture works | +| 3 | `BackingPlayer` | low-med | phase-3 unit tests; backing play/seek/speed e2e; split + duplex | +| 4 | `DeviceSetup` | med | helper unit tests; device matrix: WASAPI shared/exclusive, ASIO, dual-type split, probe==apply verdicts | +| 5 | `SourcePool` + `ExtraInputs` | med-high | TSAN-clean pool stress; multi-source + second-interface tests (`tests/sandbox` e2e, GP-5 scenario), remove-under-load | +| 6 | `AddonContext` + `NapiHelpers` | low | arg-fuzz suite; addon init/shutdown cycles, macOS build | +| 7 | `ChainOps` (with serializer) + `EditorWindows` + binding split | med | storm test flips to pass; serializer unit tests; editor open/close, sandbox promotion | +| 8 | Bug-fix commits now homed: gain clamps, renderer flush flag, `wasRunning` intent read, latency breakdown | — | each fix flips its pre-existing expected-fail test | + +Every phase additionally requires: `contract-check` green (public surface unchanged) and the +full pre-existing suite green. + +Rules that keep this safe: + +- **Move, don't edit**: each phase's diff should be reviewable as "same lines, new file" plus a + thin call seam. The excellent existing comments move with their code. +- **Reference-bind shared state** (the SourceChain trick) rather than adding getters — keeps the + RT paths free of indirection changes. +- **No public API change**: NodeAddon exports, IPC channel names, and preload surface stay + identical throughout; the collisions-doc ownership work (single chain owner, monitor-state + arbiter) is a separate track that starts after phase 7 gives it `chainGeneration`. +- Tester diag counters (`audiodiag`, `[asio-diag]`) must survive verbatim — they're how field + regressions get caught. + +### 7. Explicit non-goals + +- Rewriting the JS layer (`audio-bridge.ts` 65K / `preload.ts`) — separate track; its shape + already mirrors the binding groups this plan creates. +- Replacing JUCE transport/BufferingAudioSource, adaptive resampling for split mode, sandbox + parameter proxies — feature work, not decomposition. +- Renaming slopsmith→feedBack artifacts — orthogonal, and renaming during a move-refactor + destroys diff reviewability. + +--- + +# Part V — Merged Priority Roadmap + +One ordered list combining the collision remediation (Part II), the deep-read fixes (Part III), +and the decomposition phases (Part IV). Decomposition and fixes interleave: each fix lands as a +separate commit in the unit that becomes its home. + +1. **Phase 0** — contract snapshots, compat pins (gain bounds, `isAudioRunning` semantics), + unit-test harness, concurrency storm test (expected-fail). +2. **Refactor phases 1–2** (`PackedStereoRing`, `EngineState` with intent/state split, + `RendererBus`, `StreamSink`) — low risk, creates the homes. +3. **Fix: renderer-bus flush flag** (III §4) and **gain/slot-id sanitization** (III §2) — + small, kills the NaN-master-gain and stale-tail bug classes. +4. **Refactor phases 3–5** (`BackingPlayer`, `DeviceSetup`, `SourcePool` + `ExtraInputs`). +5. **Fix: `wasRunning` intent read in setAudioDevices** (III §3) — now trivial on the split + intent/state atomics. +6. **Refactor phases 6–7** (`AddonContext`, `NapiHelpers`, `ChainOps` + serializer + + `chainGeneration`, `EditorWindows`, binding split). +7. **Ownership work (Part II)** — single chain owner via executor (needs `chainGeneration`), + refcounted monitor-state arbiter, one persistence store per setting, tone-switch + single-slot replace instead of editor nuke. +8. **Long tail** — latency breakdown API, (typeName, name) device identity, + slopsmith→feedBack alias deprecation.