# 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` 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&`** 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).