reasampler/docs/product/code-organization.md

# Code organization & structural quality — Phase Q framing

The **why** behind a dedicated code-reorganization / structural-refactor pillar. This is
the framing and settled-decision record for **Phase Q (Quality)** — bringing the ReaSampler
`src/` tree into a healthier structure (more encapsulation, granular namespaces, `src/`
subdirectories) **without sacrificing runtime performance**, against a stated quality bar:

> "**mtytel Vital is my code reference for quality.**" — Daniel
>
> Bring the codebase "**into the realm of something I can stand to look at.**"

Its build roadmap lives in **PLAN.md §Phase Q** and its authoritative spec in
**CONTEXT.md §Phase Q**. This doc holds the *why* — the quality bar, the evidence base
(a grep-verified SOLID audit), the target directory/namespace shape grounded in the Vital
reference, and the numbered fork decisions.

**Status:** framed by product-designer (2026-07-26). Forks Q-1 … Q-6 below are the decision
record; **Q-1 (namespace letter) is settled by this doc**; the remaining forks carry a
leading recommendation and are Daniel's to call. The SOLID audit that grounds every claim
is a **grep-verified** staff-engineer analysis of the actual `src/` tree, reproduced in §2.

---

## 0. TL;DR

- **This is not a feature phase — it is a quality phase.** The code works. The pure-core /
  shell split is real and healthy (30 pure static libs, each with its own CTest executable,
  the discipline CMake-enforces). What Phase Q fixes is that the *shape* of the code doesn't
  yet read the way the architecture actually is: `src/` is one flat 45-file directory, all
  37 headers sit in one flat `reasampler` namespace, four modules have grown into
  god-modules, and one utility (JSON parsing) is copy-pasted across four models.
- **The quality bar is Vital** (§1). Vital groups its ~1,000-file synth by *subsystem*
  (`common/` `synthesis/` `interface/` `plugin/`) with nested functional sub-dirs. That is
  the aspirational shape: the directory tree *is* the architecture diagram. ReaSampler's
  subsystem groupings already exist — latent in the CMake link graph — but are invisible in
  the code's shape. Phase Q makes them visible.
- **Performance is a hard constraint, not a nicety.** The reorg is directory + namespace +
  file-split — it must cost **zero runtime**. The two hot paths (`peaks` envelope compute;
  audition/preview; the realtime-capture tick) must keep their exact call/inline shape: **no
  added virtual dispatch, no header→TU indirection on a hot path.** This is an explicit
  acceptance criterion on every point, not a footnote (§3).
- **This phase is GATED on the tree being otherwise quiescent** (§4). A structural reorg that
  lands while Phase S (on the phase-s worktree) and Phase L (L2/L3) are mid-flight would
  create catastrophic merge conflicts — the reorg touches nearly every file, and every
  in-flight branch is diffed against the *old* layout. Phase Q starts only when Phase S,
  Phase L, and any D2/M9 residuals have merged to dev and the tree is quiet. Stated
  prominently because getting the gate wrong is the one way this phase does real damage.
- **Every point is independently landable and CTest-green at every step** (§5). The CMake
  targets already draw the module seams; a file move + namespace change keeps
  `ctest --test-dir build` green at each point. Green-CTest-at-every-point is an acceptance
  criterion, not an aspiration — it is *how* a reorg this broad stays safe.

---

## 1. The quality bar — Vital, read from its actual structure

Daniel named **Vital** (`github.com/mtytel/vital`, the spectral-warping wavetable synth) as
his code-quality reference. Read from the actual repository, not from reputation, here is
what makes Vital's structure the aspirational shape — and what maps cleanly onto ReaSampler.

### 1.1 Vital groups by subsystem; the directory tree *is* the architecture

Vital's `src/` is organized top-level by **subsystem concern**:

| Vital `src/` dir | Contains | ReaSampler analogue |
|---|---|---|
| `common/` | shared utilities, data types used everywhere | the pure data/util core (`bank_model`, `app_version`, a new `json`) |
| `synthesis/` | the audio engine — nested further by concern | the capture/audio core (`capture`, `peaks`, `render_settings`, `wav_trim`) |
| `interface/` | all UI / editor code | the UI core + shells (`theme`, `component_geometry`, `bank_panel`, `draw_kit`) |
| `plugin/` | the plugin-wrapper / host-boundary layer | the REAPER-facing shells + `main.cpp` |
| `headless/` `standalone/` | build-shape entry points | (n/a in one extension binary; the Phase S VST3 is the second artifact) |

And critically, `synthesis/` is **itself** subdivided by function —
`synth_engine/ modulators/ filters/ effects/ producers/ framework/ lookups/ utilities/`.
The grouping principle is **layered functional architecture**: core generation → processing
→ infrastructure → support, each a directory. You can read the subsystem map off the folder
tree without opening a file. *That* is "code you can stand to look at": the structure
teaches you the architecture.

### 1.2 What ReaSampler has vs. what Vital has

ReaSampler's architecture is **already** subsystem-layered — it is just invisible:

- **The pure/shell split is real and CMake-enforced** — 30 pure static libraries, each with
  its own test executable, none linking a REAPER SDK. This is the *load-bearing* discipline
  and it is genuinely healthy (the SOLID audit confirms: "NO genuine core→REAPER leaks
  found — the split is intact and CMake-enforced").
- **But the *shape* hides it.** All 45 files sit in **one flat `src/` directory**; all 37
  headers sit in **one flat `reasampler` namespace**. The subsystem groupings —
  model / view / capture / audio / ui / reclaim / version — exist only in the link graph
  (`target_link_libraries` edges), never in the code you *read*.
- **Vital's lesson, applied:** ReaSampler doesn't need to *invent* an architecture — it needs
  to make its existing one **legible**. The reorg is a change of *shape*, not of *substance*.
  That is exactly why it can be zero-runtime-cost and CTest-green throughout: the seams
  already exist; Phase Q draws them where a reader can see them.

*(Note on the reference: Daniel wrote "mtytell"; the canonical repo is `mtytel/vital`. The
structural facts above are read from that repository's `src/` tree. Vital is GPLv3 — we
borrow its **structural pattern**, not its code.)*

---

## 2. The evidence base — grep-verified SOLID audit (Single-Responsibility lens)

The reorg is not driven by taste alone; it is grounded in a **grep-verified** staff-engineer
SOLID analysis of the actual `src/` tree (45 files / ~19,800 LOC). Reproduced here as the
decision-grade evidence; the PLAN points and CONTEXT spec cite back to it.

### 2.1 The three structural problems that dominate

1. **Four god-modules** — each carrying 4–8 distinct responsibilities:
   - `bank_panel.cpp` (**2,424 LOC**) — 8+ responsibilities: rendering, thumbnail
     compute+cache, audio audition engine, bank CRUD (*duplicates `actions.cpp`'s verbs*),
     context menus, input handling, new-content detection, OS drag-out + drop-target, window
     lifecycle + a ~20-function public API.
   - `main.cpp` (**1,762 LOC**) — owns-the-API-pointers is only ~120 lines of it. Also
     carries: full capture orchestration (`RunCapture` / `captureAndIndexOne` /
     `renderOffline` / batch/recapture/realtime `Run*`), `FxBypassGuard` (100+ LOC,
     precision-invariant-critical, buried), scope/source resolution, provenance assembly,
     the realtime-capture lifecycle state machine + globals, two RAII selection guards, and
     ~350 lines of hand-written registration boilerplate.
   - `actions.cpp` (**981 LOC**) — two unrelated command-id families in one TU (Design View
     family + multi-bank/prune family, the latter holding `doBankPruneFolder`, **the only
     file-deletion authority**), plus its own `promptText`/`mintBankId` *duplicating*
     `bank_panel`.
   - `persist.cpp` (**766 LOC**) — 5 responsibilities: session lifecycle+poll, ext-state
     serialization bridge, folder relocation, GUID minting, and prune scanning + filesystem
     deletion (`deleteOrphanFile` via `SHFileOperationW`).

2. **JSON serialization duplicated across four model modules.** `bank_model`, `bank_book`,
   `view_mode_model`, and `owned_manifest` **each hand-roll their own `Parser`**
   (parseString / parseInt / parseKey / skipValue + escape). This is the single largest
   DRY + SRP violation and the **highest-leverage single fix** in the phase.

3. **Zero namespace granularity.** All 37 headers live in one flat `reasampler` namespace;
   `src/` is one flat directory. Subsystem groupings are latent in the CMake link graph but
   invisible in code shape — the exact gap §1 identifies against Vital.

### 2.2 Per-module Single-Responsibility verdicts (the audit's own words)

- **Pure core is mostly clean.** `peaks`, `provenance`, `prune_reconcile`, and every geometry
  mirror (`mode_switch` / `bank_grid` / `tab_strip` / `action_buttons` /
  `component_geometry` / `prune_button`) are **single-responsibility exemplars** — leave them
  alone, just relocate + namespace them.
- **Mixed (model + JSON):** `bank_model`, `bank_book`, `view_mode_model` (1,040 LOC — 4
  indices + planner + JSON), `owned_manifest`. The JSON extraction (Q-W1) resolves the mixed
  half of each.
- **Shells split cleanly:** `capture` / `capture_realtime` (two backends behind one header);
  `view` (derive-visibility + apply-to-REAPER).

### 2.3 Other SOLID letters (secondary, in-scope where cheap)

- **O (Open/Closed):** the capture-action *table* is good OCP; the ~350-line hand-written
  **non-table** registration blocks in `main.cpp` are the opposite — every new action edits
  4 parallel places. A registration-table closes this (a candidate Q point, Q-6 dependent).
- **I (Interface Segregation):** `capture.h` / `persist.h` / `bank_panel.h` are **fat
  headers** — split them alongside their TU splits.
- **L / D:** `ICaptureBackend`/`Offline`/`Realtime` is a clean Liskov story (no violation);
  `main`/`bank_panel` depending on concrete backends is a low-priority D concern, explicitly
  **out of scope** for Phase Q (it is a design change, not a reorg).

### 2.4 Encapsulation gaps (beyond the four god-modules)

- **JSON parser duplicated 4× → extract a pure `json` core** (the highest-leverage change).
- **Bank-CRUD verbs duplicated** in `bank_panel.cpp` *and* `actions.cpp` — dedupe to one
  owner.
- `peaks` forces whole-file `std::vector<float>` materialization on the thumbnail path (clean
  API, but a data-ownership boundary forces a copy) — **noted, not scoped**: touching it
  risks the hot path (§3), so it is a deliberate non-goal for Phase Q.

---

## 3. Performance is a hard constraint (the guardrail, carried verbatim-in-spirit)

Daniel's stated non-negotiable: reorganize **without sacrificing actual performance.** The
reorg is directory / namespace / file-split — it is **zero runtime cost IF** the following
hold. These are acceptance criteria on every relevant point, not advice:

1. **Hot path — `peaks`** (`computeEnvelope` / `lastFrameAboveThreshold` over full PCM):
   **NO virtual dispatch, NO `peaks` interface, NO added header→TU indirection.** Keep
   `computeEnvelope` a **free function on `const std::vector<float>&`** so it inlines exactly
   as today. Relocating the file + namespacing it is fine; wrapping it in an abstraction is
   forbidden.
2. **Hot path — audition / preview:** splitting `panel_audition` into its own TU is fine —
   but the call must stay a **direct call-through, not virtual**.
3. **Hot path — realtime-capture tick:** keep the idle fast-path a **single pointer test.**
   The realtime lifecycle may move to its own TU (`realtime_lifecycle`) but the tick's
   branch shape must not change.
4. **JSON extraction is OFF all hot paths** — serialization runs at save/load, never per
   frame. Safe to abstract freely (this is *why* Q-W1 is the safe opener).
5. **`FxBypassGuard` runs per-capture, not per-frame** — keep it a **stack RAII** object when
   it moves out of `main.cpp`; do not heap-allocate or virtualize it.

**Net (the audit's own conclusion):** every recommended split falls on a **cold path** or
**preserves call/inline shape** on the two hot ones. The acceptance bar for the phase:
*if a split would add an indirection on a path in the list above, it is out of scope —
rework the split to avoid it, or drop it.*

---

## 4. The GATE — Phase Q starts only when the tree is quiescent (load-bearing)

**Phase Q is gated on the completion of ALL other scheduled and in-flight work.** State this
prominently; it is the single most important sequencing fact in the phase.

**Why the gate exists — merge-conflict blast radius.** Phase Q's whole nature is that it
touches **nearly every file in `src/`** (relocating into subdirectories, changing the
namespace of every header, splitting the four largest TUs). Meanwhile:

- **Phase S** lives on the **phase-s worktree**, is **not on dev**, and is a large body of
  work (a whole second VST3 build artifact + pure sampler core). Its branch is diffed against
  the *current* flat layout.
- **Phase L** has **L2** (dock-panel layout redesign, itself gated after M11) and **L3** (VST
  restyle, gated on Phase S) still to land — both touching `bank_panel` and the draw/UI
  layer, exactly the files Phase Q's god-module split rewrites.
- **D2 residuals / M9** (deferred) could reactivate.

A structural reorg landing while any of these is mid-flight would force every in-flight
branch through a **rename-and-relocate-everything** merge — the worst possible conflict
class (every hunk moved, every namespace-qualified reference changed). The cost is not linear;
it is a combinatorial re-resolution of every open branch against a moved tree.

**The gate, stated as a rule:** Phase Q does not begin until **Phase S has merged to dev**,
**Phase L (L2 + L3) has merged to dev**, any **D2 residuals** are closed, and **M9** is either
landed or confirmed-abandoned — i.e. the tree is **quiescent**, with no large branch
outstanding. Phase Q is the *last* structural pillar precisely because it reshapes the ground
every other pillar stands on. Landing it early would tax every subsequent phase; landing it
last taxes nothing.

*(Sequencing corollary: because the gate is "everything else first," Phase Q's own internal
sequencing —§5— is about risk-ordering the reorg, not about racing other phases.)*

---

## 5. Sequencing shape — big-bang vs. incremental (feeds the waves)

**Settled shape: incremental, risk-ordered waves, each independently landable and
CTest-green.** A big-bang "rename everything in one commit" is rejected — it defeats the one
safety property the CMake seams give us (green CTest at every step) and produces an
un-reviewable diff. Instead, the reorg is decomposed so each wave is a safe, reviewable,
individually-revertible step:

- **W1 — the safe, high-leverage opener (zero-god-module-risk):** extract the pure `json`
  core and delete the four duplicate `Parser`s, *plus* impose the `core/ shell/ app/`
  directory layout and sub-namespaces on the modules that **don't** need splitting (the 30
  clean pure libs + the clean shells). This is pure relocation + one genuine
  encapsulation win, no god-module surgery. Lowest risk, highest legibility payoff, done
  first.
- **W2–W5 — the four god-module splits, one per wave**, ordered by risk (the audit's own
  S-leverage ranking): `bank_panel` (W2, biggest), `main.cpp` (W3), `actions.cpp` (W4,
  includes deduping bank verbs against `bank_panel`), `persist.cpp` (W5, isolates the
  single file-deletion authority). Each god-module split is its own wave because each is a
  large, independently-reviewable change with its own verification surface.
- **W6 — the OCP registration-table** (close the ~350-line hand-written registration blocks)
  and any fat-header (I) splits not already resolved. Sequenced last because it depends on
  the `main.cpp` split (W3) having already isolated the registration code.

**Why incremental beats big-bang here, concretely:** the CMake per-module static-lib + per-
module test-executable structure means a file move + namespace change is *mechanically*
verifiable — `ctest` is green or it isn't, at every point. That property only pays off if the
reorg is *in points*. One giant commit throws the property away. Incremental is not just
safer; it is the only shape that uses the seams the architecture already gives us.

---

## 6. Forks — the decision record

**Fork Q-1 — namespace letter. SETTLED (this doc): `Q` (Quality).**

- **Settled:** the phase is namespaced **`Q` (Quality)**. M / D / B / R / V / S / L are all
  taken (Milestone / Design / Bank / Reclaim / Version / Sampler / Look-and-feel).
- **Considered and set aside:** `O` (Organization) — rejected on two grounds: (1) the glyph
  `O` reads ambiguously against zero in point ids (`O1`, `O10`), a real legibility cost in a
  phase *about* legibility; (2) "Organization" undersells the charter — this phase is
  measured against a **quality bar** Daniel set (Vital), and the reorg is the *means*, not
  the end. `Q` names the end.
- **Reasoning:** `Q` is unambiguous, unused, and reads sensibly ("Phase Q — Quality"). The
  point-id family is `Q1 … Qn` with wave prefixes `Q-W1 … Q-W6` matching the house
  wave-naming (cf. `D2-W1`, `ps-w6`, `pL-w2`).

**Fork Q-2 — JSON extraction in scope? RECOMMEND: yes, and it is the W1 opener.**

- **Recommendation:** **in scope, and first.** The 4× duplicated `Parser` is the single
  largest DRY+SRP violation (§2.1), it is entirely **off the hot paths** (§3.4, safe to
  abstract freely), and it is the highest-leverage single change in the audit. Extracting a
  pure `core/json` module and deleting the four copies is the ideal low-risk opener — it
  proves the wave discipline (relocate + encapsulate, CTest-green) before any god-module
  surgery.
- **Alternative considered:** defer JSON to a later, separate cleanup. Rejected — it is the
  cheapest, safest, highest-payoff move; deferring it wastes the opener slot on pure
  relocation with no encapsulation win.
- **Watch (the audit flags it):** guard the `Parser` name-unification against cross-lib
  collisions when the four copies merge into one; and mind `Sample` vs `AudioSample` when
  sub-namespacing (§Q-5 collision note).

**Fork Q-3 — directory naming: `core/ shell/ app/` vs a Vital-style subsystem-first tree.
RECOMMEND: `core/ shell/ app/` as the top split, subsystem dirs beneath `core/`.**

- **Recommendation:** top-level by the **load-bearing discipline** (`core/` = pure/testable,
  `shell/` = REAPER-facing, `app/` = the entry TU), then **subsystem dirs beneath** — the
  audit's proposed map: `core/model/ core/view/ core/capture/ core/audio/ core/ui/
  core/reclaim/ core/version/ core/json/`, and `shell/capture/ shell/panel/ shell/view/
  shell/persist/ shell/actions/`, with `app/main.cpp`.
- **Why this over pure-Vital (subsystem-first, e.g. `model/ capture/ ui/` at top):** the
  **pure/shell split is ReaSampler's most load-bearing invariant** — it is CMake-enforced and
  it is the thing that keeps the core unit-testable outside the DAW. Making it the *top* level
  of the directory tree makes the invariant **structurally visible and hard to violate** (a
  file's directory tells you instantly whether it may touch a REAPER type). Vital has no
  equivalent pure/host split to protect, so its subsystem-first tree is right *for Vital*;
  `core/ shell/` is the ReaSampler-native reading of the same "directory = architecture"
  lesson. Subsystem grouping still happens — one level down — so we get both readings.
- **Alternative:** flat subsystem-first (`model/ view/ capture/ audio/ ui/ …`) with the
  pure/shell distinction living only in namespaces. Rejected: it demotes the most important
  invariant from *structure* to *convention*, which is exactly the drift Phase Q exists to
  reverse.

**Fork Q-4 — sub-namespace, or sub-directory only? RECOMMEND: both — sub-namespace to match
the sub-directory.**

- **Recommendation:** granular sub-namespaces mirroring the directories:
  `reasampler::model`, `reasampler::view`, `reasampler::capture`, `reasampler::audio`,
  `reasampler::ui`, `reasampler::reclaim`, `reasampler::version`, `reasampler::json`. Directory
  and namespace agree, so a symbol's home is unambiguous from either.
- **Why:** namespaces are the *code-visible* half of the "make the architecture legible" goal
  (directories are the filesystem half). Sub-dirs without sub-namespaces leaves every symbol
  still in one flat `reasampler::` soup — half the win. Vital's own headers group by concern;
  matching namespace-to-directory is the standard C++ reading of that.
- **Cost (name it):** sub-namespacing is the change with the **widest edit surface** — every
  qualified reference across TUs updates. This is precisely why the GATE (§4) matters and why
  W1 does the namespace move on the *clean* modules first (mechanical, no logic change), with
  the god-module splits (W2–W5) namespacing their own new TUs as they land.
- **Collision watch (audit):** `Sample` (bank_model) vs `AudioSample` (peaks alias) and the
  unified `Parser` must not collide once sub-namespaced — resolve by their new
  `model::` / `audio::` / `json::` homes.

**Fork Q-5 — how aggressive the god-module split? RECOMMEND: split to the audit's named seams,
no finer.**

- **Recommendation:** split each god-module along the **exact seams the audit names** and
  stop there:
  - `bank_panel` → `panel_render` / `panel_thumbnails` / `panel_audition` / `panel_input` /
    `panel_bank_ops` / `panel_window`.
  - `main.cpp` → hoist `capture_orchestrator` / `scope_resolve` / `realtime_lifecycle`,
    leaving `main` = API pointers + entry + dispatch.
  - `actions.cpp` → `design_view_actions` / `bank_actions` / `prune_action`, deduping the
    bank verbs against `bank_panel`'s `panel_bank_ops`.
  - `persist.cpp` → `session` / `ext_state_io` / `prune_fs` (**isolating the single
    file-deletion authority** into `prune_fs`).
- **Why stop there:** the seams are already validated by the audit and correspond to real
  responsibilities. Splitting *finer* (one file per function) would trade a god-module for a
  fragmentation problem — the opposite failure. "Something Daniel can stand to look at" is
  well-factored, not atomized.
- **Alternative (lighter):** split only the two worst (`bank_panel`, `main.cpp`), leave
  `actions`/`persist` as-is. Rejected: `persist`'s buried file-deletion authority and
  `actions`'s duplicated bank verbs are real hazards worth resolving while the tree is open;
  doing them now (behind the gate) is cheaper than a second reorg later.

**Fork Q-6 — the OCP registration-table (W6): in scope or deferred? RECOMMEND: in scope, last.**

- **Recommendation:** **in scope as the final wave.** The ~350-line hand-written registration
  blocks are a genuine OCP wart (every new action edits 4 parallel places); a registration
  table closes it. Sequenced **last** because it depends on the `main.cpp` split (W3) having
  isolated the registration code — you can't table-ify code you haven't first extracted.
- **Alternative:** drop it — it's OCP, not the SRP focus of the phase. A reasonable trim if
  Daniel wants Phase Q strictly scoped to the reorg. Kept in with a leading recommendation
  because, once W3 has hoisted the registration code, tabling it is a small, high-legibility
  finish — but it is the most droppable point if the phase needs narrowing.

---

## 7. What Phase Q does NOT change (guardrails)

- **The pure-core / shell split is strengthened, never dissolved.** The whole point is to
  make it *more* legible (top-level `core/` vs `shell/`). No file moves across the boundary;
  no core file gains a REAPER type; the CMake per-module test executables stay green.
- **Every precision invariant holds.** Null-test, bit-identical repeats, non-destructive
  capture, exact bounds, relative-paths-only — none is code Phase Q rewrites. `FxBypassGuard`
  (the precision-critical guard) *moves* out of `main.cpp` but stays a stack RAII object with
  identical behavior (§3.5).
- **Capture ≠ placement.** No `Run*` / capture-orchestrator path may gain an `InsertMedia`
  call during the split. The load-bearing rule is invariant under reorganization.
- **The single file-deletion authority stays one obvious module.** `persist`'s `prune_fs`
  split *concentrates* the deletion authority (`SHFileOperationW`) into one named module —
  it must not spread it. This is a safety property the reorg improves, never dilutes.
- **Relative-paths-only in the persisted index** — untouched (a data invariant, not a
  structural one).
- **Performance** — §3 is a hard acceptance bar: no hot-path indirection, ever.
- **No behavior change.** Phase Q is a pure structural refactor. If a point changes observable
  behavior, it has exceeded its charter and must be reworked. The test suite passing
  unchanged **is** the proof of correctness (green CTest at every point).

---

## 8. Sources

- Vital source structure — `github.com/mtytel/vital`, `src/` subsystem tree
  (`common/ synthesis/ interface/ plugin/ headless/ standalone/`) and the nested
  `synthesis/` functional sub-dirs (`synth_engine/ modulators/ filters/ effects/ producers/
  framework/ lookups/ utilities/`). Read from the repository tree, 2026-07-26. Vital is
  GPLv3 — the borrowed artifact is the **structural pattern**, not code.
- The SOLID-compliance audit (§2) — grep-verified staff-engineer analysis of the actual
  ReaSampler `src/` tree (45 files / ~19,800 LOC), 2026-07-26. The evidence base for every
  reorg move and the performance guardrails (§3).