serumalt/AUDIT_REPORT.md

# SerumAlt codebase audit (Phase 1)

Historical snapshot, taken before the platform/harness build separation. Build
commands, defaults, target locations and line numbers below describe that earlier
state. Use README.md's Building section and the current CMake files for builds.
The proposed source moves are suggestions, not an approved implementation plan.

Scope: the SerumAlt C++/JUCE VST3 + Standalone synthesiser. This report records the
implicit standards, anti-patterns, memory choices, error handling, concurrency model and
module boundaries found by reading the real repository files. No source, build or
documentation files were modified. All findings are cited with `path:line` references read
during this task.

Repository note: at the start of this audit the working tree already had uncommitted
changes to `CMakeLists.txt`, several `Source/` files and several `third_party/JUCE/` files,
plus untracked `build/`, `build_windows/` and `SerumAlt_Windows/` directories. Those
changes predate this audit and are not analysed here except where the current on-disk
content is the source of truth.

---

## 1. Build & test setup

### Project metadata and targets

- Project name and version: `project(SerumAlt VERSION 1.0.0 LANGUAGES CXX)` at `CMakeLists.txt:3`.
- C++ standard: C++17, required, extensions off at `CMakeLists.txt:8-10`
  (`CMAKE_CXX_STANDARD 17`, `CMAKE_CXX_STANDARD_REQUIRED ON`, `CMAKE_CXX_EXTENSIONS OFF`).
- The plugin target is `SerumAlt`, declared with `juce_add_plugin(...)` at
  `CMakeLists.txt:33-46`. Formats are `VST3 AU Standalone` at `CMakeLists.txt:44`. JUCE
  generates the concrete per-format targets (`SerumAlt_VST3`, `SerumAlt_Standalone`, and
  `SerumAlt_AU` on macOS). There is no separate `PluginProcessor`/`PluginEditor` target;
  those are ordinary classes compiled into `SerumAlt`.
- A console test target `SerumAltTest` is added at `CMakeLists.txt:126-140` via
  `juce_add_console_app`. It compiles the entire `${SERUMALT_SOURCES}` list plus
  `Source/Tests/TestMain.cpp` (`CMakeLists.txt:128`).

### Source list

Sources are an explicit list, not a glob. `set(SERUMALT_SOURCES ...)` spans
`CMakeLists.txt:50-87` and is consumed by `target_sources(SerumAlt PRIVATE
${SERUMALT_SOURCES})` at `CMakeLists.txt:89`. The list enumerates every `.cpp` used by the
plugin, including the subdirectory files `Source/EffectUnits/*.cpp`, `Source/GUI/*.cpp`
and `Source/Presets/FactoryPresets.cpp`.

Two header-only files are intentionally absent from the list because they have no
translation unit: `Source/EffectUnits/Biquad.h` and `Source/GUI/SerumLookAndFeel.h`.

Consequence for later phases: any file move or rename must update this list, because there
is no glob to pick files up automatically.

### Custom flags and linkage

- Compile definitions at `CMakeLists.txt:91-95`: `JUCE_WEB_BROWSER=0`, `JUCE_USE_CURL=0`,
  `JUCE_VST3_CAN_REPLACE_VST2=0`.
- Link libraries at `CMakeLists.txt:97-103`: `juce::juce_audio_utils` and `juce::juce_dsp`
  (private), `juce::juce_recommended_config_flags` and `juce::juce_recommended_warning_flags`
  (public).
- Warning flags at `CMakeLists.txt:142-146`: `/W4` under MSVC, `-Wall -Wextra` otherwise.
- MinGW cross-build static runtime at `CMakeLists.txt:113-118`: when
  `CMAKE_CROSSCOMPILING`, `SerumAlt_VST3` gets `-static-libgcc -static-libstdc++ -static`
  so the DLL is self-contained.
- VST3 manifest handling at `CMakeLists.txt:23-31`: `SERUMALT_VST3_AUTO_MANIFEST` is `FALSE`
  when cross-compiling (because JUCE 7.0.12 builds the manifest helper with the same
  toolchain, producing a Windows `.exe` that cannot run on the Linux host). The manifest is
  instead injected by `build_windows.sh`.

### Windows cross-build (build_windows.sh)

`build_windows.sh` is the canonical Windows build path. Exact steps, in order:

1. Verify `x86_64-w64-mingw32-g++` exists, else exit with an error (`build_windows.sh:16-21`).
2. Regenerate `mingw64-toolchain.cmake` from a heredoc inside the script
   (`build_windows.sh:26-42`). This is identical in content to the checked-in
   `mingw64-toolchain.cmake` (see below), so the toolchain file is defined in two places.
3. Configure: `cmake -S . -B build_windows -G Ninja -DCMAKE_BUILD_TYPE=Release
   -DCMAKE_TOOLCHAIN_FILE=mingw64-toolchain.cmake -DSERUMALT_BUILD_TESTS=OFF`
   (`build_windows.sh:47-50`). Note the test harness is explicitly disabled here.
4. Build: `cmake --build build_windows --target SerumAlt_VST3` (`build_windows.sh:53`).
5. Inject `Contents/Resources/moduleinfo.json` into the `.vst3` bundle by writing the file
   directly, because the automatic manifest step was disabled (`build_windows.sh:63-126`).
6. Copy the bundle to `SerumAlt_Windows/SerumAlt.vst3` (`build_windows.sh:129-131`).

The toolchain (`mingw64-toolchain.cmake:1-15`) sets `CMAKE_SYSTEM_NAME Windows`,
`CMAKE_SYSTEM_PROCESSOR x86_64`, the `x86_64-w64-mingw32-*` compilers/tools, and
`CMAKE_FIND_ROOT_PATH /usr/x86_64-w64-mingw32` with `PROGRAM NEVER` and the other
`..._MODE_*` values `ONLY`.

### Native Linux build (README only)

`README.md:76-78` documents a native build:

```
cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Release
cmake --build build
```

Outputs listed at `README.md:80-84` are
`build/SerumAlt_artefacts/Release/VST3/SerumAlt.vst3` and the standalone app.

### Tests

Tests exist. `Source/Tests/TestMain.cpp` is a console harness that instantiates the
processor, plays notes through every factory preset, checks for NaN/inf and silence, and
toggles RAVE (`TestMain.cpp:37-135`). It returns a non-zero exit code on failure
(`TestMain.cpp:134`).

Tests are run through the native build, documented at `README.md:96-98`:

```
cmake --build build --target SerumAltTest
./build/SerumAltTest_artefacts/Release/SerumAltTest
```

The Windows cross-build does not build or run tests: `build_windows.sh:50` passes
`-DSERUMALT_BUILD_TESTS=OFF`.

### Clean build and test command

There is no clean step in `build_windows.sh`. The script configures into `build_windows/`
and builds without removing or cleaning it first. A clean rebuild is not scripted; it would
require manually removing `build_windows/` (or invoking Ninja's clean target, which the
script does not do). A combined "clean build + run tests" command does not exist: the
Windows script disables tests, and the README test instructions assume an already-built
native `build/` tree.

---

## 2. Proposed module boundaries

The codebase already has three real subdirectory boundaries (`EffectUnits/`, `GUI/`,
`Presets/`) plus `Resources/` (assets only) and `Tests/`. The flat files at `Source/` root
mix four distinct concerns, and three root files clearly belong inside existing
subdirectories. The proposal below is based on the include graph actually present, not on
generic synth architecture.

### Dependency facts that drive the split

- `Params.h`/`Params.cpp` depend only on `<JuceHeader.h>` (`Params.h:3`). It declares the
  `ids`, `maps` namespaces, all enums, `ModSource`/`ModTarget`, and the wavetable name
  table. It is the shared foundation: nearly every other file includes it.
- `Wavetable`, `Oscillator`, `SubOscillator`, `NoiseOscillator`, `Envelope`, `LFO`,
  `Filter`, `FilterBank`, `SynthVoice` depend only on `Params`, `JuceHeader`, and each
  other (`Oscillator.h:4-5`, `FilterBank.h:4-5`, `SynthVoice.h:4-12`).
- `ModulationMatrix` and `MacroControls` depend only on `Params` (`ModulationMatrix.h:4`,
  `MacroControls.h:4`).
- `RAVEButton.h`/`RaveController` depends only on `Params` (`RAVEButton.h:4`) but manipulates
  the APVTS, so it sits between modulation and plugin plumbing.
- `FXProcessor.h` depends only on `Params` (`FXProcessor.h:4`); every file in
  `EffectUnits/` includes `../FXProcessor.h` (`Hyper.h:3`, `Chorus.h:3`, etc.), so
  `FXProcessor` is the base the effect units build on.
- `Engine` aggregates the DSP: `Engine.h:4-10` includes `Wavetable`, `SynthVoice`, `LFO`,
  `FXProcessor`, `ModulationMatrix`, `MacroControls`.
- `PluginProcessor.h` includes `Params`, `Engine`, `RAVEButton` (`PluginProcessor.h:4-6`);
  it is the JUCE `AudioProcessor` boundary plus state/preset/RAVE glue.
- `PluginEditor.h` includes `PluginProcessor` and every `GUI/` header
  (`PluginEditor.h:4-14`); it is pure GUI.
- `Resources.h` depends on `JuceHeader` plus `Params` for the format helpers
  (`Resources.cpp:2`); it holds the theme and SVG strings. The SVG asset files live in
  `Source/Resources/`, but the `Resources.{h,cpp}` code lives at root, splitting one
  concept across two locations.
- `Presets/FactoryPresets` depends on `Params` and `ModulationMatrix`
  (`FactoryPresets.h:4-5`).
- `Tests/TestMain.cpp` depends only on `PluginProcessor` (`TestMain.cpp:4`).

### Proposed modules

| Module | Files (current location) | Depends on |
| --- | --- | --- |
| params (foundation) | `Params.{h,cpp}` | JuceHeader only |
| synth (DSP building blocks + voice) | `Wavetable.{h,cpp}`, `Oscillator.{h,cpp}`, `SubOscillator.{h,cpp}`, `NoiseOscillator.{h,cpp}`, `Envelope.{h,cpp}`, `LFO.{h,cpp}`, `Filter.{h,cpp}`, `FilterBank.{h,cpp}`, `SynthVoice.{h,cpp}` | params |
| modulation | `ModulationMatrix.{h,cpp}`, `MacroControls.{h,cpp}`, `RAVEButton.{h,cpp}` | params |
| effects | `FXProcessor.{h,cpp}` plus all of `EffectUnits/` (including `Biquad.h`) | params |
| engine | `Engine.{h,cpp}` | synth, effects, modulation |
| gui | `PluginEditor.{h,cpp}` plus all of `GUI/` (including `SerumLookAndFeel.h`) | params, synth (for wavetable display), plugin |
| resources | `Resources.{h,cpp}` plus `Resources/*.svg` | params |
| presets | `Presets/FactoryPresets.{h,cpp}` | params, modulation |
| plugin (JUCE plumbing) | `PluginProcessor.{h,cpp}` | engine, modulation, presets, params |
| tests | `Tests/TestMain.cpp` | plugin |

### Where the current layout violates the boundaries

- `FXProcessor.{h,cpp}` sits at `Source/` root but is the base class and owner of the
  `EffectUnits/` classes; `EffectUnits/*.h` include it as `../FXProcessor.h`. It belongs in
  `EffectUnits/` (or in a renamed `FX/` directory with the units).
- `PluginEditor.{h,cpp}` sits at root but is pure GUI and includes every `GUI/` header. It
  belongs in `GUI/`.
- `Resources.{h,cpp}` holds the theme and SVG strings, while the `.svg` assets live in
  `Source/Resources/`. The code and assets should live together under one `resources`
  module.
- `RAVEButton.{h,cpp}` is orphaned at root under a filename that does not match its class
  (`RaveController`). It is a macro-like boost over the APVTS, so it fits the `modulation`
  module, though its APVTS manipulation also makes `plugin` a defensible home.
- The remaining root files (`Params`, `Wavetable`, `Oscillator`, `SubOscillator`,
  `NoiseOscillator`, `Envelope`, `LFO`, `Filter`, `FilterBank`, `SynthVoice`,
  `ModulationMatrix`, `MacroControls`, `Engine`, `PluginProcessor`) are the synthesis core
  and JUCE boundary. Grouping them into `synth/` and `modulation/` subdirectories would
  match the dependencies above, but the flat layout is at least internally consistent:
  they are the only files that legitimately belong at root today.

---

## 3. Memory management standards

### Dominant idiom: value semantics plus `std::unique_ptr` for ownership

- Owned sub-objects are held by value, not by pointer: `juce::AudioProcessorValueTreeState
  parameters; Engine engine; RaveController rave;` at `PluginProcessor.h:56-58`; the
  `Engine` holds `std::array<SynthVoice, kNumVoices> voices`, `std::array<LFO, kNumLfos>
  lfos`, and `WavetableLibrary`, `FXProcessor`, `ModulationMatrix`, `MacroControls` by value
  at `Engine.h:45-50`.
- Owned polymorphic objects use `std::unique_ptr`: `std::array<std::unique_ptr<FXUnit>, 9>
  units` at `FXProcessor.h:50`, populated with `std::make_unique` at `FXProcessor.cpp:15-26`.
- GUI attachments are owned by `std::unique_ptr`: `std::vector<std::unique_ptr<...
  SliderAttachment>>` and `...ComboBoxAttachment` at `PluginEditor.h:103-104`, filled at
  `PluginEditor.cpp:169-170` and `183-184`.
- Transient GUI objects are `std::unique_ptr`: `std::unique_ptr<juce::Label> valuePopup`
  at `Knob.h:38`, created at `Knob.cpp:61` and released with `valuePopup.reset()` at
  `Knob.cpp:83`.
- JUCE-managed objects are returned as raw `new` pointers because JUCE takes ownership
  through `addAndMakeVisible` or the plugin API: `createEditor()` returns
  `new PluginEditor (*this)` at `PluginProcessor.cpp:195`; the plugin entry point returns
  `new serum::SerumAltAudioProcessor()` at `PluginProcessor.cpp:380`; GUI children are
  created with `new` at `PluginEditor.cpp:166,178,193,602,606`. These raw pointers are never
  deleted by the code, which is correct only because JUCE's component tree and plugin host
  own them.
- Non-owning cross-object references are raw pointers or references: `juce::RangedAudioParameter*
  param` at `ToggleButton.h:38`, `const WavetableLibrary* wtLib` at `WaveformDisplay.h:24`,
  and the `RenderContext` raw `const` pointers to the library/matrix/macros at
  `SynthVoice.h:24,32-33`.

### Containers and buffers

- `std::vector<float>` is the standard delay line / table buffer: `Wavetable.h:24`
  (`frames`), `Filter.h:39` (`combLine`), `Chorus.h:21`, `Delay.h:21`, `Reverb.h:22-29`.
- `std::array` is used for fixed-size DSP state: `Oscillator.h:62`
  (`std::array<SubVoice, kMaxUnison>`), `Filter.h:36,44`, `Reverb.h:22-27`.
- `juce::AudioBuffer<float>` is used for block buffers: `Engine.h:59` (`mixBuffer`),
  `SynthVoice.h:95` (`scratch`), `FXProcessor.h:51` (`dry`, `wet`). These are sized once in
  `prepare()` (for example `Engine.cpp:68`, `SynthVoice.cpp:23`, `FXProcessor.cpp:34-35`)
  and cleared per block.

### Realtime vs non-realtime allocation

The code intends to allocate only at prepare time, but it does not fully achieve this:

- Prepare-time allocation is the norm: delay lines and buffers are sized in `prepare()`
  (`Filter.cpp:24`, `Chorus.cpp:9-11`, `Reverb.cpp:17-33`, `Engine.cpp:68`).
- Violation 1: `Engine::processBlock` calls `mixBuffer.setSize (2, n, false, false, true)`
  on every block at `Engine.cpp:211`. `AudioBuffer::setSize` is a no-op when the size is
  unchanged, but it is still a per-callback reconfiguration call on the audio thread.
- Violation 2: wavetables are built lazily. `WavetableLibrary::getTable` builds the table
  on first access (`Wavetable.cpp:126-132`) and `getTable` is reached from the audio path at
  `SynthVoice.cpp:177-178`. The `prebuild()` method exists at `Wavetable.cpp:120-124` but is
  never called (it is not referenced anywhere else, and `Engine::prepare` at
  `Engine.cpp:58-70` does not call it). The first rendered block for a wavetable therefore
  allocates `256 x 2048` floats plus harmonic work buffers on the audio thread.

---

## 4. Error handling

There is no exception handling and no user-facing alerting. Failures are handled with a
mix of asserts, null guards, and return codes.

- `jassert` is used exactly once in the entire codebase: `jassert (freqHz > 0.0)` at
  `Oscillator.cpp:20`. There are no `throw`, `try` or `catch` statements (verified by grep).
- Null guards with silent fallback are the dominant pattern. The APVTS helper `v()` returns
  `0.0f` when a parameter is missing: `Engine.cpp:8-13`. State restore silently returns on
  malformed input: `PluginProcessor.cpp:313-315` returns when the XML is null and
  `PluginProcessor.cpp:318-319` returns when the `ValueTree` is invalid. Preset loads skip
  missing parameters with `if (auto* param = ...)` at `PluginProcessor.cpp:248-250`.
- Return codes carry failure for the two bounded collections:
  `ModulationMatrix::addConnection` returns `false` when the target is `None` or the
  connection limit is reached (`ModulationMatrix.cpp:6-12`), and
  `MacroControls::addAssignment` returns `false` similarly (`MacroControls.cpp:6-13`).
  These return values are ignored by their callers (`PluginEditor.cpp:536`, `PluginEditor.cpp:687`,
  `PluginProcessor.cpp:254`, `PluginProcessor.cpp:258`).
- Out-of-range lookups degrade to empty/default values: `getProgramName` returns an empty
  string for a bad index (`PluginProcessor.cpp:218-224`); the enum-string converters fall
  through to `return {}` (`Params.cpp:27,44,91`).
- There are no `juce::AlertWindow`, no `MessageManager` calls, and no logging in the plugin
  itself. The only reporting is the test harness printing to `std::cout` and returning an
  exit code (`TestMain.cpp:73,103,129,132-134`).

Net style: programmer errors use `jassert` (very sparingly), runtime/state errors use null
guards or default values, and bounded-insertion failures use `bool` return codes that
callers currently ignore.

---

## 5. Concurrency

The codebase is single-threaded and has no explicit synchronisation. A grep for threads,
mutexes, locks, atomics, `CriticalSection`, `SpinLock` and `ScopedLock` returns nothing
outside JUCE's own headers.

- The only non-audio execution context is the JUCE message thread, driven by a `juce::Timer`:
  `PluginEditor` inherits `juce::Timer` (`PluginEditor.h:24`), starts it at 30 Hz
  (`PluginEditor.cpp:135`), and refreshes visuals in `timerCallback` (`PluginEditor.cpp:875-880`).
- The audio thread is `SerumAltAudioProcessor::processBlock` (`PluginProcessor.cpp:187-191`)
  delegating to `Engine::processBlock` (`Engine.cpp:176-318`). It reads the playhead tempo at
  `Engine.cpp:184-187`.

The significant issue is unsynchronised sharing between the message thread and the audio
thread:

- The GUI mutates `ModulationMatrix::connections` via `addConnection`/`removeConnection`/
  `clear` (`PluginEditor.cpp:536,549,559`) while the audio thread iterates the same vector in
  `SynthVoice::render` at `SynthVoice.cpp:114`. `connections` is a public `std::vector`
  (`ModulationMatrix.h:30`).
- The GUI mutates `MacroControls::assignments` (`PluginEditor.cpp:687,699`) while the audio
  thread iterates it at `SynthVoice.cpp:125`. `assignments` is a public `std::array` of
  vectors (`MacroControls.h:28`).
- The GUI writes LFO shape data through `engine.setLfoShapeData` (`PluginEditor.cpp:483-486`)
  which calls `LFO::setShapeData` (`Engine.cpp:84-88`, `LFO.cpp:51-59`), while the audio
  thread reads the same `shapeBuffer` in `LFO::shapeValue` (`LFO.cpp:82-91`).

None of these shared collections are locked or double-buffered. This is a real data race
today: resizing `connections` while the audio thread iterates it is undefined behaviour.

The processor header acknowledges GUI-thread access but does not address the race: the
comment "Public DSP state (read/write from the GUI thread)" at `PluginProcessor.h:55` only
describes intent.

---

## 6. Naming & style conventions

- Namespace: every file is inside `namespace serum` (`PluginProcessor.h:8`, `Engine.h:12`,
  etc.). File-local helpers use an anonymous namespace (`Engine.cpp:6`, `SynthVoice.cpp:6`,
  `PluginEditor.cpp:6`, `FactoryPresets.cpp:6`).
- Include guard: `#pragma once` in every header (for example `PluginProcessor.h:1`,
  `Params.h:1`, `Biquad.h:1`). No `#ifndef` guards exist.
- Include order: `<JuceHeader.h>` first, then local headers (`PluginProcessor.h:3-6`).
  Subdirectory files include root files with a `../` relative path (`Hyper.h:3`,
  `Knob.h:5`, `FactoryPresets.h:4`).
- File/header pairs: one primary class per `.h`/`.cpp` pair, with the pair named after the
  class (`Oscillator.h`/`Oscillator.cpp`, `FilterBank.h`/`FilterBank.cpp`). Header-only
  helpers exist where they have no state logic worth a TU (`Biquad.h`,
  `SerumLookAndFeel.h`).
- Class and struct names: PascalCase, no prefix (`SynthVoice`, `RenderContext`,
  `FilterBankParams`, `ModConnection`).
- Members: no `m_` prefix and no trailing underscore. Plain camelCase fields such as
  `note`, `velocity`, `baseFreq`, `active`, `released`, `noteId` at `SynthVoice.h:83-88`,
  and `sr`, `blockSize`, `bpm`, `pitchBend` at `Engine.h:52-57`.
- Constants: `k` prefix for compile-time constants (`kNumVoices`, `kNumLfos` at
  `Params.h:221-227`; `kFrames`, `kTableSize` at `Wavetable.h:18-19`; `kMaxConnections` at
  `ModulationMatrix.h:28`; `kMaxAssignments` at `MacroControls.h:26`). Local constant arrays
  in anonymous namespaces also use `k` (`kEnvAttack` at `Engine.cpp:30`, `kFxType` at
  `PluginEditor.cpp:97`).
- Enums: `enum class` (scoped) with PascalCase enumerators (`WarpMode::BendPlus`,
  `ModSource::Lfo1`, `ModTarget::Filter1Cutoff`) at `Params.h:213-219` and `Params.h:232-256`.
- Namespaces for related constants: `ids::` for parameter ID strings (`Params.h:18`),
  `theme::` for colours (`Resources.h:12`), `maps::` for unit mappings (`Params.h:309`).
- Method naming: camelCase with a leading verb (`noteOn`, `processAdd`, `getValue`).
  JUCE overrides keep JUCE's spelling (`processBlock`, `prepareToPlay`, `resized`,
  `timerCallback`).
- `noexcept` is applied consistently to the per-sample DSP hot paths (`Oscillator.cpp:93`,
  `Filter.cpp:181,236`, `Wavetable.cpp:81`, `Envelope.cpp:54`, `SynthVoice.cpp:72`,
  `LFO.cpp:98`) and to cheap getters (`SynthVoice.h:71-74`).
- Formatting: two-space indentation; a space before the opening parenthesis in function
  calls and definitions (`f (x)` at `PluginProcessor.cpp:12`, `juce::jlimit (...)`;
  `if (...)`, `for (...)`). Aligned member initialiser lists (`PluginEditor.cpp:113-115`).
- Section dividers: `// ===...` banners at class/file tops and `// ---...` dividers within
  files (`PluginProcessor.cpp:17,198,261`, `PluginEditor.cpp:161`).
- Casting idiom: `(size_t)` casts on int loop indices when indexing `std::vector`/`std::array`
  (`Engine.cpp:220`, `SynthVoice.cpp:79`, `FactoryPresets.cpp` throughout), and `(int)`
  casts on `size()` results.

---

## 7. Anti-pattern catalog

Each entry lists the location, the problem, and the standard that a later AGENT.md phase
should encode.

1. Parameter ID tables duplicated three times.
   - `Params.h:18-208` (canonical `ids::`), `Engine.cpp:30-55` (`kEnv*`, `kLfo*`, `kFx*`),
     `PluginEditor.cpp:97-108,433-437,458-464,820-825,833` (the same arrays again), plus the
     display names and defaults in `PluginProcessor.cpp:31-171`.
   - Standard: one source of truth in `Params.h` (or a single generated table); engine and
     editor must reference it rather than re-declaring ID arrays.

2. Wavetable library allocates on the audio thread.
   - `WavetableLibrary::getTable` builds lazily (`Wavetable.cpp:126-132`), reached from
     `SynthVoice::render` (`SynthVoice.cpp:177-178`); `prebuild()` is defined
     (`Wavetable.cpp:120-124`) but never called.
   - Standard: call `prebuild()` (or build all tables) in `prepareToPlay`, never in the
     render path.

3. Per-block buffer resize in the audio callback.
   - `Engine::processBlock` calls `mixBuffer.setSize(...)` every block at `Engine.cpp:211`.
   - Standard: size the buffer once in `prepare()` and only `clear()` per block.

4. Unsynchronised GUI/audio sharing of container state.
   - `ModulationMatrix::connections` is public (`ModulationMatrix.h:30`), written by the GUI
     (`PluginEditor.cpp:536,549,559`) and read on the audio thread (`SynthVoice.cpp:114`).
     `MacroControls::assignments` is public (`MacroControls.h:28`), written
     (`PluginEditor.cpp:687,699`) and read (`SynthVoice.cpp:125`). LFO `shapeBuffer` is
     written via `setShapeData` (`LFO.cpp:51-59`) and read (`LFO.cpp:82-91`).
   - Standard: either protect these collections with a lock, or use atomic/double-buffered
     exchange between the message and audio threads.

5. Class name does not match its file.
   - `Source/RAVEButton.h` declares `RaveController` (`RAVEButton.h:14`), and
     `RAVEButton.cpp` implements it.
   - Standard: file pair name matches the class (`RaveController.h/.cpp`), or the class is
     renamed to match the file.

6. Dead code in `SubOscillator::noteOn`.
   - `SubOscillator.cpp:9-12` computes `mult` and then discards it with `(void) freqHz;
     (void) mult;` because the octave shift is already baked into the frequency by the voice.
   - Standard: remove the unused parameters or make the signature reflect what is actually
     used (the octave shift is applied at `SynthVoice.cpp:179`).

7. No-op override.
   - `changeProgramName` is an empty override at `PluginProcessor.cpp:226-228`.
   - Standard: implement it or do not override it.

8. Ignored prepare-time parameter.
   - `Filter::prepare` accepts `maxBlockSize` and discards it with `(void) maxBlockSize;`
     (`Filter.cpp:21-29`); effect units name the parameter `int` but never use it
     (`Hyper.cpp:6`, `Chorus.cpp:6`, `Flanger.cpp:6`, etc.).
   - Standard: keep the `FXUnit` interface uniform but do not add unused parameters to
     concrete `prepare()` implementations.

9. Duplicated envelope curve constants.
   - The attack/decay shape mapping `0.3 + curve * 2.7` / `3.0 - curve * 2.7` appears in
     `Envelope.cpp:44-45` and again in the display `EnvelopeDisplay.cpp:19-20`.
   - Standard: expose one shared helper so the preview cannot drift from the DSP.

10. Filter response reimplemented for display.
    - `FilterDisplay::magnitude` (`FilterDisplay.cpp:6-36`) hardcodes a parallel set of
      transfer-function approximations that duplicate the real DSP in `Filter.cpp`
      (`Filter.cpp:87-234`).
    - Standard: keep the display response derived from one documented source, or mark it as
      an approximation with a comment that the two are not the same code.

11. Inconsistent pi constants.
    - `kTwoPi` is defined at `Oscillator.h:67` and `SubOscillator.h:26`; `twoPi` at
      `Wavetable.cpp:21`; `kPi` at `Wavetable.cpp:156` and `Filter.cpp:8`; raw literals
      `6.28318530717958647692` appear at `Chorus.cpp:40-41`, `Flanger.cpp:38-39`,
      `Phaser.cpp:30-31`, and `6.2831853f` at `LFO.cpp:67` and `LFODisplay.cpp:19`.
    - Standard: use `juce::MathConstants<T>::pi` / `twoPi` everywhere.

12. Public mutable state exposes internals.
    - `PluginProcessor.h:56-58` exposes `parameters`, `engine` and `rave` as public fields;
      `ModulationMatrix.h:30` and `MacroControls.h:28` expose their collections;
      `Engine.h:36-38` returns non-const references from `getMatrix()`, `getMacros()` and
      `getWavetables()`.
    - Standard: return `const&` from accessors and route mutation through member functions,
      except where a public field is an explicit, documented design choice.

13. Unchecked raw parameter dereference.
    - `PluginEditor.cpp:794` dereferences `getRawParameterValue(id)->load()` without a null
      check, while the equivalent helper in `Engine.cpp:8-13` guards the pointer.
    - Standard: always guard `getRawParameterValue`/`getParameter` results before use.

14. Mixed ownership for GUI children.
    - `makeKnob`/`makeCombo`/`makeToggle` return raw pointers to `new`-allocated children
      that JUCE owns via `addAndMakeVisible` (`PluginEditor.cpp:162-199`), and `fxUp`/
      `fxDown` use raw `new` at `PluginEditor.cpp:602,606`. This is correct only because of
      JUCE's component-tree ownership, but it is easy to misread as a leak.
    - Standard: document that JUCE-owned children are raw pointers and never deleted, and
      keep `std::unique_ptr` for everything the plugin owns directly.

---

## 8. Hard constraints worth codifying

Only rules that the current code and build actually support are listed.

1. Do not allocate or resize containers in the audio callback. Evidence that this is the
   intent: all DSP state is sized in `prepare()` (`Engine.cpp:68`, `SynthVoice.cpp:23`,
   `FXProcessor.cpp:34-35`) and the code uses pre-sized `juce::AudioBuffer` and
   `std::array`. The current violations (`Engine.cpp:211` and the lazy wavetable build) are
   exceptions to fix, not the rule.
2. All audio parameters are normalised to 0..1 in the APVTS, and physical units are mapped
   only through `maps::` (`Params.h:309-342`). DSP must consume the normalised values, not
   physical units.
3. Parameter IDs live in the `ids` namespace in `Params.h` (`Params.h:18-208`). Engine,
   editor and preset code must reference `ids::`, not re-declare string literals. (This is
   the stated intent at `Params.h:5-9`; the current duplication in `Engine.cpp` and
   `PluginEditor.cpp` is a violation to eliminate.)
4. The CMake source list is explicit (`CMakeLists.txt:50-87`), not a glob. Every file move
   or rename must update `SERUMALT_SOURCES`.
5. All code is inside `namespace serum`, and every header uses `#pragma once` (verified
   across all headers).
6. No exceptions and no user-facing alert dialogs. Use `jassert` for programmer errors,
   null guards for runtime lookups, and `bool` returns for bounded inserts (see
   `Oscillator.cpp:20`, `Engine.cpp:8-13`, `ModulationMatrix.cpp:6-12`).
7. When cross-compiling for Windows, keep the VST3 manifest step disabled and let
   `build_windows.sh` inject `moduleinfo.json`, because JUCE's manifest helper must not be
   built as a Windows executable on a Linux host (`CMakeLists.txt:23-31`,
   `build_windows.sh:55-126`). Keep the injected metadata in sync with
   `PLUGIN_CODE`/`PLUGIN_MANUFACTURER_CODE`/`VERSION` (`build_windows.sh:60-62`).
8. The project is single-threaded by design. Any future thread, mutex or atomic must come
   with an explicit plan for the GUI-to-audio boundary, because the current matrix/macro/LFO
   state is shared without synchronisation (section 5).