Files

T

daniel 5ec84ab0eb docs: mark M9 abandoned in Phase Q framing; add L3 + bank_grid/tab_strip to CLAUDE.md

M9 deferred->abandoned in code-organization.md gate (disposition note removed).
CLAUDE.md: L3 identity clause, M9-abandoned line, bank_grid/tab_strip in module
list and targets table.

2026-07-27 12:51:38 -04:00

43 KiB

Raw Blame History

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); gate reconciled + naming dimension added (2026-07-27). Forks Q-1 … Q-9 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. Two grep-verified audits ground every claim: a SOLID audit (§2) and a naming/symbol-consistency audit (§2b) — both staff-engineer-rigor analyses of the actual src/ tree.

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, one utility (JSON parsing) is copy-pasted across four models, and a set of symbols are named inconsistently or collision-prone (§2b). Phase Q addresses four dimensions: structure (directories), encapsulation (namespaces + JSON dedupe), factoring (god-module splits), and naming (a consistent component-naming scheme, §2b + Q-7…Q-9).
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) is mid-flight would create catastrophic merge conflicts — the reorg touches nearly every file, and every in-flight branch is diffed against the old layout. As of 2026-07-27 the outstanding work is Phase S (merged to dev) and Phase L L3 (the VST restyle, itself gated on Phase S) — Daniel's plain target: "when Phase S and L3 are finished." (L1/L2/L4–L7 have already landed; D2 is functionally complete; M9 is abandoned.) Stated prominently because getting the gate wrong is the one way this phase does real damage.
Beyond SOLID, Phase Q also fixes naming. The reorg gives every symbol a directory + namespace home (Q-3/Q-4); §2b's grep-verified naming audit adds the orthogonal dimension of giving poorly/inconsistently-named symbols a consistent name, measured against the same Vital "something I can stand to look at" bar. Renames ride the waves that already relocate the file (a rename is nearly free when a file is already moving); the scheme + new forks (Q-7…Q-9) are in §6.
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

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).
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.
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.

2b. The naming audit — grep-verified symbol/module-naming inconsistencies

The SOLID audit (§2) grounds where responsibilities live; this section grounds what things are called. Same rigor: every claim below cites a symbol or file verified by grep/read of the actual src/ tree (2026-07-27), not taste asserted in the abstract. The reorg is the moment to fix naming because a rename is nearly free when the file is already being relocated or split (Q-W1–Q-W6). Measured against the same bar: "something I can stand to look at" means a newcomer can predict a symbol's name from its role and never meets two unrelated things sharing one name.

2b.1 The good news — what is already consistent (leave alone)

Two families are already named on a legible principle; the audit's job there is only to protect them through the reorg, not to change them:

The geometry-mirror verb vocabulary is consistent. Every pure layout/hit-test module uses the same two verbs: compute<Thing>Rects / compute<Thing> for layout and hitTest<Thing> for hit-testing — verified across bank_grid (computeCellRects/hitTestCell), mode_switch (computeSegmentRects/hitTestSegment), action_buttons (computeButtonRects/hitTestButton), action_bar (computeBarSlots/hitTestActionBar), tab_strip (computeTabRects/hitTestTabStrip), prune_button (computePruneButton/hitTestPruneButton), overflow_menu (computeMenuButton/hitTestMenuButton), footer_bar (computeFooterBar/hitTestFooterBar), card_drag (computeSlotRects/hitTestSlot), component_geometry (computeButtonBox/hitTestBox). This is a real, followed convention — preserve it verbatim.
The _tests suffix is uniform. Every pure module's CTest executable is <module>_tests (30 targets in CLAUDE.md's table, no exceptions). No action.

2b.2 Ambiguous / collision-prone symbols (the highest-priority fixes)

These are the naming equivalent of the JSON-Parser DRY violation — concrete hazards, not taste:

Four hand-rolled Parser classes, one name. class Parser is defined four times — bank_model.cpp:306, bank_book.cpp:663, owned_manifest.cpp:107, view_mode_model.cpp:654. Q-W1 already deletes three of them by extracting core/json; the naming rule is that the survivor is json::Parser (or a more specific json::Reader/json::Writer pair — see Q-8), never a bare Parser in flat scope.
FooterRect and ButtonRect are shared across pure UI modules — and the codebase already knows it. struct FooterRect and struct ButtonRect are defined in prune_button.h (lines 32, 46) and reused by footer_bar.h, which carries an explicit in-file "NAME NOTE" (footer_bar.h:27–34) documenting that ButtonRect / FooterRect / SegmentRect / ActionBarRect / KitBox / KitButtonBox are "already owned in this namespace" and that new types must carry a FooterBar* prefix to avoid collision. That comment is a smell made visible: the flat reasampler:: namespace forces every pure-UI author to hand-check for name collisions before minting a type. This is the single strongest in-codebase argument for the Q-4 sub-namespaces — under reasampler::ui these shared rect types get one clear owner and the hand-checking stops.
Sample (bank_model.h:69, the bank metadata struct) vs AudioSample (the peaks float alias). Already flagged in §2.4/Q-4; verified — Sample is the model record, AudioSample is a raw PCM float. Under model::Sample vs audio::AudioSample the collision risk is gone, but the names still read oddly side by side (a Sample that is metadata, an AudioSample that is one float). Noted; the namespace split is the required fix, a rename is optional (Q-8).
Selection (bank_grid.h:112) and CellRect (bank_grid.h:23) are generic names in a flat namespace. Selection in particular is the kind of name a newcomer cannot place without opening the file. ui::Selection / ui::CellRect resolve it structurally; no rename needed beyond the namespace.

2b.3 Inconsistent module/type naming families (the taste-but-grounded tier)

Here the names are legal and non-colliding but do not read on one principle — the "stand to look at" gap:

The model-family suffixes disagree: _model vs _book vs Index. Verified: the pure model modules are bank_model.{h,cpp} (owning class BankIndex, bank_model.h:132), bank_book.{h,cpp} (owning class BankBook, bank_book.h:208), view_mode_model.{h,cpp} (owning class ViewModeModel, view_mode_model.h:376), owned_manifest.{h,cpp} (owning class OwnedFileManifest, owned_manifest.h:52). Four modules, four different file↔class naming relationships: bank_model→BankIndex (file says "model," class says "index"), bank_book→BankBook (file = class), view_mode_model→ViewModeModel (file = class), owned_manifest→OwnedFileManifest (file ≈ class, but the class adds "File"). The bank_model/BankIndex mismatch is the worst: the file name and its primary class name share no word. This is a genuine legibility wart — the fix is a rename decision (Q-8), not something the directory move alone resolves.
The bank_book "wraps bank_model" relationship is invisible in the names. BankBook (bank_book.h:208) is a registry of Bank (bank_book.h:147), each wrapping a BankIndex (bank_model.h:132). The names Book → Bank → Index do not read as a containment hierarchy; a reader has to learn it. (Not necessarily worth a rename — "book of banks" is evocative — but it is the kind of call Q-8 should make deliberately, not by accident.)
realtime_record.h (pure) vs capture_realtime.cpp (shell) — the word order flips. Verified: the pure realtime module is realtime_record.{h} (owning RecordModePlan/RecordPhase/ RecordTickInputs, realtime_record.h:57–173) while its shell is capture_realtime.cpp. So the pure core is realtime_record but the shell is capture_realtime — the two halves of one feature are named on inverted word order (realtime_record vs capture_realtime). Compare the clean shell-pair convention elsewhere: drag_out (pure) ↔ drag_out_win (shell) — same stem, suffix marks the platform shell. The realtime pair breaks that pattern. This is the clearest shell↔core naming-drift instance in the tree (Q-9).
capture.{h,cpp} is the offline backend shell, but the name claims all of capture. Verified: capture.h declares ICaptureBackend, OfflineRenderBackend, and RealtimeRecordBackend (capture.h:112,124,201), while the realtime implementation lives in capture_realtime.cpp and its pure planner in realtime_record.h. So capture is really "capture interface + offline backend," a fat header (the §2.3 Interface-Segregation concern) whose name oversells its scope. Its Q-W3 hoist (capture_orchestrator/scope_resolve) is the moment to right-size the name.

2b.4 Abbreviations / opacity (low-severity, opportunistic)

Swept for names a newcomer couldn't decode; the tree is mostly clean here (a credit to it). Two minor notes:

guid_diff / GuidBaseline (guid_diff.h:40) — "GUID diff" is decodable in context (it diffs the live track/item GUID set between polls) but GuidBaseline reads more clearly as "the previous-poll snapshot" than the module name suggests. Low priority; leave unless its core/view relocation invites it.
MinMax (peaks.h:30), KitBox (component_geometry.h:28) — terse but correct and local; no change. Named here only to record they were swept and cleared.

2b.5 What the naming audit does NOT touch (hard boundary)

The FOREVER-STABLE on-the-wire/on-disk contracts are not C++ symbol names and are out of scope for every rename: command_id strings (CEREBELLUM_REASAMPLER_* / _BETA_), action display names ("ReaSampler: …"), ext-state namespace ("reasampler" / "reasampler_beta") and its keys ("banks", "view_state", "tail_setting", "owned_files", "version"), the reasampler: lane-name prefix, and the Phase S VST3 class UID. Renaming a C++ class is orthogonal to these strings; the audit's renames touch symbols only, never a shipped contract literal. This is the same guardrail §7 states for the reorg, restated for the naming dimension because a careless "tidy the names" pass is exactly how a shipped id gets broken.

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:

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.
Hot path — audition / preview: splitting panel_audition into its own TU is fine — but the call must stay a direct call-through, not virtual.
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.
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).
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 L3 (VST editor + embed-strip restyle, gated on Phase S landing on dev) still to land — it touches the Phase S draw shells (reasampler_editor / reasampler_embed), which arrive on dev with Phase S. (L1/L2/L4/L5/L6/L7 have already landed — see COMPLETED.md; the once-listed "L2 pending" is stale and has been corrected here.)
D2 is functionally complete (D2-W1..W3-B landed; the only open item — a per-track lane-split panel indicator — is explicitly deferred, not a blocking residual). M9 (slots) is abandoned (Daniel, 2026-07-27) — will not be built. D2 is named in the gate only so a future reactivation of its deferred panel indicator re-arms the "tree must be quiescent" condition; M9 is out and cannot re-arm it.

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 (reconciled to reality, product-designer 2026-07-27): Phase Q does not begin until the tree is quiescent, with no large branch outstanding. As of 2026-07-27 the outstanding work is precisely:

Phase S — merged to dev (currently on the phase-s worktree; the large second-artifact branch, the dominant gate item).
Phase L L3 — merged to dev (the VST restyle; itself gated on Phase S, so it lands after Phase S reaches dev). L2 is already landed — the earlier "L2 + L3" wording was stale.
D2 — confirmed complete or its deferred indicator explicitly re-deferred. It is functionally complete today; this line stays only so that if the deferred panel indicator is picked up as active work, it re-arms the quiescence condition.
M9 — abandoned (Daniel, 2026-07-27). This gate condition is permanently satisfied; M9 will not be reactivated.

Restated as the plain readiness target Daniel named: "when Phase S and L3 are finished." 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 Parsers, 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.

Where the naming work (§2b) rides — renames follow relocations, no dedicated wave (Q-7). A rename is cheapest when the file is already moving or splitting, so naming does not get its own wave; each fix rides the wave that already touches its file:

W1 absorbs the collision fixes (§2b.2): the survivor Parser becomes json::Parser (or the Q-8 Reader/Writer pair) as the four copies collapse; and every clean pure-UI type (FooterRect/ButtonRect/Selection/CellRect) gets its ui:: (etc.) home as the modules relocate — retiring the footer_bar.h hand-collision "NAME NOTE." W1 already re-namespaces the clean modules, so the sub-namespace half of every §2b fix lands here for free.
W1 also carries any pure-model class rename Q-8 settles (e.g. BankIndex→a name matching bank_model), because those modules relocate in W1 and a class rename is a mechanical find-replace verified by the module's own test executable.
W2 absorbs the bank_panel-side names as the god-module splits into panel_*.
W3 absorbs the capture/realtime shell↔core word-order fix (Q-9) — the realtime lifecycle is already being hoisted in W3, so aligning capture_realtime/realtime_record naming is a rider on a move that is happening regardless.

The rule (Q-7): no rename lands on a file that is not otherwise being touched by its wave. A rename that would force a file to move only to be renamed is deferred — the churn/legibility trade isn't worth a standalone edit. This keeps the naming dimension inside the same "green-CTest-at-every-point, minimal-diff-per-wave" discipline as the rest of Phase Q.

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.

Fork Q-7 — is naming its own wave, or does it ride the relocation waves? RECOMMEND: rides the waves; no dedicated naming wave. SETTLED-by-structure once Q-3/Q-4 are settled.

Recommendation: naming fixes ride the wave that already relocates or splits the file (§5), under the rule no rename lands on a file the wave isn't otherwise touching. A rename is nearly free during a relocation (the file is open, the diff is already large, the module's own test executable verifies it) and near-pure-churn as a standalone edit. Because Q-3 (directories) and Q-4 (sub-namespaces) already move and re-namespace every file, the collision half of the naming audit (§2b.2) is resolved by the namespace split with zero extra renames — the sub- namespace is the fix. Only the genuine class/module renames (Q-8/Q-9) add symbol churn, and those are scoped to files already in motion.
Alternative considered: a dedicated final "naming pass" wave (Q-W7). Rejected: it would re-open files W1–W6 just closed, producing exactly the churn-without-relocation the rule forbids, and a diff that touches everything again defeats the per-wave reviewability property.
This makes Q-7 not really a judgment call once Q-3/Q-4 are settled — it is the forced consequence of "renames are cheapest during relocation." Recorded as a fork only because Daniel might still want naming called out as a first-class deliverable rather than folded silently into the reorg waves; if so, the plan names the riders per wave (it does, §5) without adding a wave.

Fork Q-8 — how far to push class/module renames (beyond the free namespace fix)? RECOMMEND: fix the two that actively mislead; leave the merely-quirky. Daniel's to call.

Recommendation (leading): rename only where a name actively misleads a reader, and stop:
- BankIndex → a name matching bank_model (§2b.3.1) — the file/class word-mismatch is the worst offender (bank_model.h owns class BankIndex; the two share no word). Two shapes: (a) rename the class to BankModel (file = class, matches ViewModeModel/BankBook); or (b) rename the file to bank_index.{h,cpp} (class stays BankIndex). Prefer (a) — it makes the model family read on one principle (BankModel/BankBook/ViewModeModel, all <noun>Model/<noun>Book), and it is a class rename W1 verifies via bank_model_tests.
- The unified JSON Parser (§2b.2.1) — make it a json::Reader + json::Writer pair (or keep json::Parser if extraction stays parse-only). This is a new module's naming, decided at W1 mint time, so it costs nothing to get right.
Leave quirky-but-harmless: Book→Bank→Index containment (§2b.3.2 — evocative, learnable), Sample/AudioSample (§2b.2.3 — the namespace split already de-collides them; renaming AudioSample→Pcm/PcmSample is optional polish), guid_diff/GuidBaseline, MinMax, KitBox. Renaming these is pure taste with no misleading-a-reader payoff — the "stand to look at" bar is met by the namespace homes alone.
Alternative (more aggressive): normalize the entire model family to one suffix (BankModel/BankBookModel/ViewModeModel/OwnedManifestModel) and rename AudioSample→ PcmSample. Rejected as the lead because it renames things that already read fine, adding symbol churn (every call site, every test) for marginal legibility — but it is a coherent option if Daniel wants the model family rigidly uniform. Kept on the table as Daniel's call.
Alternative (minimal): do zero class renames; let the sub-namespaces (Q-4) carry the whole naming win. Defensible — it is the lowest-churn, lowest-risk reading, and the namespace split genuinely resolves every collision. Rejected as the lead only because bank_model/BankIndex is a standing "what is this file" cost the reorg is uniquely cheap to fix.

Fork Q-9 — align the capture_realtime / realtime_record shell↔core word order? RECOMMEND: yes, during W3. Daniel's to call.

Recommendation: align the pair to the house shell↔core convention (drag_out ↔ drag_out_win: shared stem, suffix marks the shell). The pure planner realtime_record and its shell capture_realtime invert word order for one feature — the tree's clearest shell/core drift (§2b.3.3). W3 is already hoisting the realtime lifecycle, so aligning the names is a rider on a move that happens anyway. Two shapes: (a) core capture_realtime / shell capture_realtime_shell (stem = capture_realtime, matches the capture offline pair); (b) core realtime_record / shell realtime_record_shell. Prefer (a) — it nests the realtime naming under capture_* alongside the offline path, so the whole capture subsystem reads on one stem.
Alternative: leave it — the inversion is cosmetic and both names are individually clear. Reasonable if Daniel wants W3 kept strictly to the god-module split with no adjacent renames. The fix is cheap enough (W3 touches these files regardless) that the lead is to take it, but it is the most droppable of the three naming forks.

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).

43 KiB Raw Blame History Unescape Escape