diff --git a/docs/product/code-quality-audit.md b/docs/product/code-quality-audit.md new file mode 100644 index 0000000..df59c85 --- /dev/null +++ b/docs/product/code-quality-audit.md @@ -0,0 +1,370 @@ +# ReaSampler code-quality audit — Q-W0 findings report and triage + +Date: 2026-07-28 · Branch: `pq-w0-audit` · Static analysis only; no code changed by the audit. + +This is the committed Q-W0 findings report (Q-10 SETTLED: a committed doc beside the SOLID/naming +audit — `docs/product/code-organization.md` §2c.3; deliverable contract in PLAN.md §Q-W0). It +synthesizes four parallel audit tracks; the full track notes remain in the tree as appendices and +are the evidence base for every claim here — this report cites finding IDs and does not restate +mechanisms in full: + +- **Track 1 — DSP / audio algorithm quality:** [`audit-notes/q-w0-t1-dsp.md`](audit-notes/q-w0-t1-dsp.md) (T1-01…T1-11) +- **Track 2 — architecture smells (functional lens):** [`audit-notes/q-w0-t2-architecture.md`](audit-notes/q-w0-t2-architecture.md) (T2-01…T2-11) +- **Track 3 — env-coupled-constant domain modeling:** [`audit-notes/q-w0-t3-env-constants.md`](audit-notes/q-w0-t3-env-constants.md) (T3-01…T3-07) +- **Track 4 — structural sizing + placement:** [`audit-notes/q-w0-t4-sizing.md`](audit-notes/q-w0-t4-sizing.md) (T4-01…T4-30) + +Dispositions below are **proposals**. Per the Q-W0 sign-off gate, Q-W1 does not begin until Daniel +has signed off on every disposition; the open calls are collected in §4. + +--- + +## 1. Verdicts up front + +**DSP / pitch engine (the Q-11 question).** The correlation-aligned SOLA in `pitch_shift` is +**sound — no technique replacement (phase-vocoder / WSOLA) is warranted on this evidence** (T1 +overall verdict). Track 1 finds the implementation "unusually well-defended" (normalized +correlation, ratio-scaled fades with drain-headroom derivation, prime-with-real-content onset, +frozen-writer tail, filled-span clamping) with RT discipline intact throughout. Every T1 finding +sits on the Q-11 escalation ladder's first rungs — bounded fixes within the existing technique, or +documented operating limits — exactly the settled default. The one High finding (T1-01, stereo +splice decorrelation) is a bounded fix inside the current design (link the per-channel lag +search), not a technique change. Track 1 states plainly that it cannot listen: every artifact is +mechanism + predicted audible consequence, and perceptual materiality is Daniel's call. + +**Architecture (functional smells).** The load-bearing boundaries hold: no pure module includes a +host type, the VST bridge is genuinely read-only, WAV *decoding* has exactly one owner, the prune +and exception boundaries audit clean (T2 clean list). What Track 2 found instead is the classic +cost of duplicated *algorithms*: the length-prefixed wire `Cursor` copy-pasted 3× **with +security-hardening drift** — the oldest copy (`provenance`) missing the overflow guards its +siblings gained (T2-01, High, with a cheap Q-W0 backport); a fifth hand-rolled JSON decoder the +§2 audit did not count (T2-02); `readFileBytes` ×5 (T2-03); the ext-state grow-loop ×3 with its +pure decode half-adopted (T2-04); a 19-struct rect zoo (T2-05); and drifting copy-paste in the +capture-stamp epilogue (T2-09). All are dedup/relocation-shaped and route onto the reorg waves; +none is a live user-facing bug except the T2-01 robustness gap. + +**Env-coupled constants (the prior-incident category).** The persistence surfaces — the +highest-stakes case — are **clean**: every wall-clock quantity written to disk since the S12 +remediation is in seconds or ms, and every persisted frame-domain value is a source-file fact +whose rate travels with it (T3 clean list). Seven findings, only two fix-now: the master-gain +ramp step hard-codes 20 ms × 48 kHz (T3-01) and the Trigger-fade UI ceiling hard-codes +2 s × 44.1 kHz (T3-03) — both live hardcoded-rate residues in `src/`, both trivial, both in files +no then-planned downstream wave opens. The rest are deliberate-and-documented couplings or +recorded legacy residue, plus one systemic usability note (no DPI/content-scale support, T3-05) +that is a future phase of its own. + +**Sizing + placement.** The wave plan's four planned splits are necessary but no longer +sufficient: the census measures **9 files needing TU splits and 2 needing header-only splits — 5 +of them VST-side, which currently have no owning wave** (T4 §1.5). `reasampler_editor.cpp` +(3,065 LOC) is now the largest unowned file in the repo. Track 4's structural answer is a new +VST god-module wave (**Q-W2v**), reshaped seam lists for Q-W2 (6→8) and Q-W3 (3→4 hoists), one +documented exception (`sampler_core.cpp` stays whole — hot path, T4-14/T4-27), and one dead +abstraction to delete (`ICaptureBackend`, T4-26 — the brief's "two implementations" assumption is +false at current state). The `src/vst/` placement question is a genuine fork for Daniel +(T4-18/T4-19, §4a). + +--- + +## 2. Unified findings register + +Every finding from all four tracks, exactly once, with proposed disposition. Severities are the +tracks' own. "Q-W0" as a destination means remediated in this wave before it closes (post +sign-off). Cross-track overlaps are reconciled in §2.5. + +### 2.1 Track 1 — DSP (appendix: `q-w0-t1-dsp.md`) + +| ID | Finding (one line) | Sev | Proposed disposition | Rationale | +|----|--------------------|-----|----------------------|-----------| +| T1-01 | Stereo Preserve: per-channel independent splice alignment decorrelates L/R (image wander + mono-sum combing on stereo captures) | High | **Fix-now in Q-W0** (bounded SOLA fix: linked lag/schedule across channels) — **Daniel's call, §4b** | Hits the flagship path (Preserve default + permanently stereo bus); standard stereo-SOLA practice; no technique change | +| T1-02 | Ratio slew mid-fade can drain the outgoing tap past the writer (pitch-env attack case) | Med | Document-and-defer; bounded re-cap noted for when the file is next opened | Needs pitch-env + Preserve + steep attack to trigger; constant-ratio case already covered | +| T1-03 | Preserve prime ignores Trigger `playEnd_` bound; zero-pads sub-window samples as declared-valid ring content | Med | **Fix-now in Q-W0** (prime to feedBound + immediate `freezeTail()`) **if Daniel agrees short one-shots matter, else defer with note — §4b** | Small, contained in `Voice::start`; re-uses designed GA3 machinery | +| T1-04 | No sustain-loop crossfade — hard loop seam clicks unless loop points amplitude-matched | Med | Document-and-defer (record beside the zone-loop spec) | A crossfade is a feature (parameter + UI), wrong scope for a reorg phase | +| T1-05 | Linear interpolation + no band-limiting on repitch (both engines) | Low | Document-and-defer (recorded trade-off; cubic Hermite noted as drop-in if ever wanted) | Classic sampler behavior, deterministic and consistent across engines | +| T1-06 | Correlation search: coarse step-4 can mis-lock above ~5 kHz; maxLag bounds alignment to ≥ ~80 Hz | Low | Document-and-defer (record as the engine's stated operating range) | Inherent SOLA range/cost trades; widening costs splice-burst CPU linearly | +| T1-07 | `splice()` up-jump clamp comment contradicts the code (code is exactly tight; comment's margin direction is backwards) | Low | **Fix-now in Q-W0** (comment rewrite, zero behavior change) | Misleads the next maintainer of a safety-critical clamp; one line | +| T1-08 | Linear-in-amplitude ADSR decay/release (constant-dB nowhere; abrupt-late releases) | Low | Document-and-defer | Character-vs-correctness product decision; changing it alters every existing instrument's feel | +| T1-09 | `declickR_` is dead state (blend correctly shares one weight; R is seeded/decayed, never read) | Low | Fix-now-trivial **as a rider on any Q-W0 `sampler_core` edit** (T1-01/T1-03); else defer | Hygiene only, no audio effect; not worth a standalone change | +| T1-10 | `planWavTruncate` silently drops RIFF chunks after `data` (metadata loss on trim) | Low | Document-and-defer (note in the header's FORMAT ASSUMPTION block when next touched) | Metadata-only; preserving trailing chunks complicates the single-truncating-write design for no audio benefit | +| T1-11 | `makeUniqueTag` 1 s resolution → same-second batch captures collide (silent overwrite) | Low-Med | **Fix-now, assigned to Q-W3** (per-session monotonic counter, both call sites); Q-W0 fallback if triage prefers | T1 explicitly routes at triage; Q-W3 is the nearest wave opening the extension capture flow | + +### 2.2 Track 2 — architecture (appendix: `q-w0-t2-architecture.md`) + +| ID | Finding (one line) | Sev | Proposed disposition | Rationale | +|----|--------------------|-----|----------------------|-----------| +| T2-01 | Wire `Cursor` ×3 with hardening drift — `provenance` lacks the overflow/length guards its siblings have; unbounded `reserve` reachable from persisted input | High | **Fix-now, split:** (a) backport hardened `field()` + count sanity bound to `provenance.cpp` **in Q-W0** (§4c); (b) structural collapse to one shared wire codec **in Q-W1** | Hazard is cheap to close now with existing `provenance_tests`; the dedup should ride the wave already creating `core/` | +| T2-02 | Fifth hand-rolled JSON decoder in `tail_control` (the §2 "4× Parser" undercount) | Med | **Fix-now, folded into Q-W1** — add `tail_control` to the `core/json` consumer list explicitly | Zero extra cost when `core/json` lands; a stray fifth decoder afterward would be a defect of the wave | +| T2-03 | `readFileBytes` hand-rolled ×5 across both artifacts | Med | **Fix-now, folded into Q-W1** — one pure helper in the `core/` utility home; both targets link it | Five copies of a ten-line function; creating its home is exactly Q-W1's job | +| T2-04 | `GetProjExtState` grow-loop ×3; pure `bridge_marshal` decode only half-adopted (`usage_scan`'s copy is prune-safety-adjacent) | Med | **Fix-now, assigned to Q-W5** (the wave that splits `persist.cpp` — T2's own rule; its "Q-W4" label predates the plan's persist=W5 numbering) | Touching persist's session machinery outside its own wave risks the highest-traffic shell for a dedup with no live bug | +| T2-05 | 19 rect structs + ~15 inline point-in-rect predicates across the pure UI family | Med | **Fix-now, folded into Q-W1** — reconciled with T4-21 into one disposition, see §2.5(1) | Same-moment-as-relocation principle; PLAN Q-W1 already owns the `ui::` rect unification | +| T2-06 | Pure-computable layout math stranded in the VST editor shell (~49 inline geometry computations; §2's VST scope gap) | Med | Document-and-defer **with named reshape — satisfied by Q-W2v's `editor_layout` pure-candidate TU** (see §2.5(4), §3) | Behavior-preserving hoist best done under the reorg's test discipline; must be a recorded point or the layer keeps growing | +| T2-07 | Extension links the entire voice engine to serialize one preset blob (codec not separable from engine) | Low | Document-and-defer → **executed by Q-W2v's `component_state_io` split** (same split as T4-13, see §2.5(3)) | Right abstraction, wrong granularity; a module-homing decision the reorg waves exist to make once | +| T2-08 | WAV/RIFF container knowledge in 4 modules / 2 chunk walkers (dedup-by-hash + null-test invariants sit on their agreement) | Low | Document-and-defer → **consolidation moment is a triage question, §4e** (T2 prefers the `core/wav` homing moment; T4-23 prefers the wave that opens `ingest.cpp`) | All four currently correct against each other; pre-reorg consolidation churns the capture hot path for no functional gain | +| T2-09 | Capture backends' Sample-stamping epilogue copy-paste with silent divergences (active-project vs pinned-project time-sig read) | Med | **Fix-now, folded into Q-W3** — extract shared `stampCaptureSample` helper; divergent bits stay in the realtime caller | Q-W3 opens both backend TUs anyway; keeps one review of precision-invariant-adjacent code | +| T2-10 | Thumbnail cache-invalidation drifted across the split (extension: pure generation-baked key; editor: ad-hoc string key + call-site `clear()`s) | Low | Document-and-defer — adopt the pure `ThumbnailKey` on the VST side **as a rider on Q-W2v's editor work** | No live bug; pointless as standalone churn, natural rider on the editor wave | +| T2-11 | ComponentState v1→v11 deserialize chain sound, but v3/v4/v5 legacy branches triplicate the shared read | Low | Document-and-defer, explicitly — record that the next envelope bump (v12) extends the shared path rather than minting another branch | Legacy branches are frozen back-compat contract; rewriting them risks the one thing they must never break | + +### 2.3 Track 3 — env-coupled constants (appendix: `q-w0-t3-env-constants.md`) + +| ID | Finding (one line) | Sev | Proposed disposition | Rationale | +|----|--------------------|-----|----------------------|-----------| +| T3-01 | Master-gain ramp step is a per-sample constant baking in 20 ms × 48 kHz (`kGainRampRate = 1/960`); FB1 no-zipper contract degrades silently at higher rates | Med | **Fix-now in Q-W0** — store `kGainRampSeconds`, derive step from `sampleRate_` (the file's own `kPreserveWindowMs` pattern) — **§4d** | Trivial, isolated, behavior-identical at 48 kHz; a live violation of the no-hardcoded-rate ruling in a file no then-planned wave opens | +| T3-02 | Takeover-declick decay is a per-frame coefficient (~2× faster at 96 kHz) — documented deliberate in-code | Low | Document-and-defer — triage ratifies the in-code note as the record | Already an explicit, written, bounded design decision; converting buys no audible improvement | +| T3-03 | Trigger-fade UI throw ceiling hardcodes 2 s × 44 100 as `88200.0` frames (knob full-scale varies per source rate) | Low | **Fix-now in Q-W0** — `kFadeMaxSeconds = 2.0` resolved against the loaded source's rate; T3's stated fallback (defer, amend comment) if zero UI-feel change is preferred — **§4d** | Small and contained; storage domain unchanged; the editor already threads `frameCount + rate` through pack/unpack | +| T3-04 | Drop-hint banner duration stored in sync-timer ticks (6 × 500 ms) | Low | Document-and-defer; fold in opportunistically if the file is opened | Cosmetic, self-documenting, cadence and decay live three lines apart | +| T3-05 | Systemic: no DPI/content-scale support in either UI surface (all layout constants are physical px at ~96 DPI) | Med | Document-and-defer — record as a named future phase; Q-W1's geometry relocation keeps constants centralized so the eventual scale factor lands in one place | A proper UI-scaling pass is a feature wave of its own, far outside Q-W0's remediation budget | +| T3-06 | Legacy v3 zone-payload lift divides by the *current* project rate (skewed times if the rate changed since write; write-era rate never recorded) | Low | Document-and-defer — this report is the record; the skew is a known, not a future mystery bug | Unrecoverable in principle; the documented residue of the incident that motivated the seconds invariant | +| T3-07 | SOLA correlation-segment cap of 512 frames — frame-domain by design (CPU bound); wall-clock span halves at 96 kHz | Low | Document-and-defer — **resolved against Track 1's verdict, see §2.5(2)** | T3 judged the frame domain arguably correct for a compute bound and handed the quality call to T1 | + +### 2.4 Track 4 — sizing + placement (appendix: `q-w0-t4-sizing.md`) + +| ID | Finding (one line) | Sev | Proposed disposition | Rationale | +|----|--------------------|-----|----------------------|-----------| +| T4-01 | `bank_panel.cpp` (3459): the plan's six seams no longer land sub-600 — `panel_render` ~700, `panel_input` ~800 | High | **Fix-now → reshapes Q-W2:** eight TUs, adding `panel_layout` and `panel_drag` | Without the two new seams, two of six TUs ship >600 on day one | +| T4-02 | `main.cpp` (1897): `capture_orchestrator` as specced lands ~885 | High | **Fix-now → reshapes Q-W3:** fourth hoist `capture_batch` (batch family + recapture + selection guards) | Recapture is planner-driven like batch and shares the guard machinery — it belongs with batch | +| T4-03 | `actions.cpp` (1016): plan's seams still land sub-600 | — | No change to Q-W4 (confirmation) | Measured against the current tree | +| T4-04 | `persist.cpp` (852): plan's seams still land sub-600; pS-usage growth landed exactly where the plan isolates it | — | No change to Q-W5 (confirmation) | Measured against the current tree | +| T4-05 | `bank_book.cpp` (1109): `SlotMap` is a self-contained type; post-JSON-extraction remainder splits cleanly | Med | Fix-now, fold into Q-W1 (`slot_map` extraction rides the JSON rewire already opening this file) | One `git mv`-shaped extraction on top of owned work | +| T4-06 | `view_mode_model.cpp` (1049) + `.h` (748): planners separable from model+indexes after JSON extraction | Med | Fix-now, fold into Q-W1 (planner split rides the JSON rewire); T4 allows deferring the planner split if the wave wants to stay minimal (~660 post-extraction is marginal) | JSON rewire opens the file; header splits the same way | +| T4-07 | `bank_model.cpp` (767): model vs JSON — the Q-W1 poster child, ~250 after extraction | — | Already owned by Q-W1; no new seam (confirmation) | — | +| T4-08 | `capture_realtime.cpp` (867): async lifecycle vs file-side finalize are distinct concerns | Low-Med | Fix-now, ride Q-W3 (`capture_realtime_finalize.cpp` split rides the Q-9 naming rider) | Same-wave file surgery is free | +| T4-09 | `view.cpp` (677): park/restore vs D2 lane machinery are separable halves | Low | Document-and-defer unless Q-W1's relocation touches it — then take the free `view_lanes` split | 677 is barely over the bar | +| T4-10 | `ingest.cpp` (636): pure WAV/PCM build helpers trapped in a shell TU | Low-Med | Fix in whichever wave lands the WAV consolidation — **moment is the §4e triage question** (circular with T4-23; no current wave opens `ingest.cpp`) | Extracting the pure WAV build gains a test target and drops the shell to ~500 | +| T4-11 | `vst/reasampler_editor.cpp` (3065): the largest unowned file — the `bank_panel` of the VST artifact | Highest | **Fix-now → new wave Q-W2v:** eight TUs (session / controls / layout / paint ×2 / input ×2 / platform), split axis = the face structure | Grew past `main.cpp` after the plan was written; same god-module profile | +| T4-12 | `vst/reasampler_processor.cpp` (1164): four seams | Med-High | **Fix-now → Q-W2v:** three TUs (`processor_state` / `processor_reload` / lifecycle+`process()` whole) | Partial-class-across-TUs is the same pattern as the Q-W2 panel split | +| T4-13 | `vst/sample_map.cpp` (970) + `.h` (708): resolution core vs binary ComponentState codec | Med-High | **Fix-now → Q-W2v:** split `component_state_io.cpp` + matching header split — one disposition with T2-07, see §2.5(3) | The codec grows every envelope bump (v6→v11 in one quarter); reduces editor/processor rebuild fan-out | +| T4-14 | `vst/sampler_core.cpp` (968) + `.h` (762): TU is a genuine single responsibility on the hottest path | Med | **Fix-now → Q-W2v, header only:** split `zone_params.h` from `sampler_core.h`; **TU stays whole — documented exception to the 600 bar** (record in the wave brief so nobody "fixes" it later) | Same-TU inlining on the per-sample path; no LTO in the build (see T4-27) | +| T4-15 | `view_mode_model.h` (748) | — | Covered under T4-06 (splits with its TU) | — | +| T4-16 | `vst/sample_map.h` (708) | — | Covered under T4-13 | — | +| T4-17 | `vst/sampler_core.h` (762) | — | Covered under T4-14 | — | +| T4-18 | VST placement, leading recommendation: integrate into the one `core/`/`shell/` split with `instrument/` subsystem dirs | — | **Daniel's fork — §4a** (T4 recommends T4-18) | One rule, no special case; the artifact boundary is a link-graph fact the sources already straddle | +| T4-19 | VST placement, alternative: parallel artifact-first subtree (`src/vst/core|shell` behind one SDK-gated `add_subdirectory`) | — | **Daniel's fork — §4a** | Defensible if artifact legibility outweighs discipline uniformity; T4 notes its self-containment is cosmetic | +| T4-20 | Little-endian byte codec hand-rolled ×5 — real template win (`putLE`/`readLE`) | Med | Fix-now, lands with `component_state_io` in Q-W2v (its biggest consumer); other consumers rewire opportunistically — relationship to T2-03/T2-04 noted in §2.5(5) | Compile-time dispatch, zero runtime cost, entirely off hot paths; gives the codec a tested primitive | +| T4-21 | Rect family: unify with one **concrete** `ui::Rect` + `contains()` + per-role aliases — NOT a template; retire the XYWH-vs-LTRB fork | Med | **Fix-now, ride Q-W1** — reconciled with T2-05 into one disposition, see §2.5(1) | The types differ in name only; a template would model nothing; cross-lib `Rect` collision is fixed by the Q-4 sub-namespaces | +| T4-22 | Linear rect-scan hit-tests: one small `hitIndex` template collapses them — only after T4-21 | Low | Document-and-defer; opportunistic rider on Q-W1 once the rect unification lands | Alone it would be a forced template | +| T4-23 | WAV build/parse consolidation into one pure `wav_codec` owner (dedup, not template) | Low-Med | Fix-now-sized, but the owning moment is the **§4e triage question** (T4 assigns it to the wave opening `ingest.cpp`; T2-08 prefers the `core/wav` homing moment; no current wave opens ingest) | Avoid a standalone churn commit; one tested owner of the RIFF layout | +| T4-24 | `clamp01` ×6 — one `constexpr inline` (or `std::clamp` at sites); anti-template | Trivial | Fix-now, rider on Q-W1 relocation | — | +| T4-25 | JSON `Parser` ×4 confirmed still accurate; `sample_usage` is **not** a fifth JSON parser (no scope growth from this track) | — | No change to Q-W1 (confirmation; T2-02's fifth decoder is a different file and does grow the consumer list) | — | +| T4-26 | `ICaptureBackend` is a dead abstraction: one deriver, zero polymorphic call sites; the brief's "two implementations" assumption is FALSE (realtime deliberately has a bespoke seam) | Low (runtime) / Med (hygiene) | **Fix-now, in Q-W3:** delete the interface, `OfflineRenderBackend` becomes concrete; **correct CLAUDE.md/CONTEXT ("ICaptureBackend interface; two backends") in the same commit** | It misleads — this audit's own brief was misled; Q-W3 touches every call site | +| T4-27 | Warning: do **not** split `sampler_core.cpp` along class lines — envelope `tick()`s are per-voice-per-sample; a by-class split is the exact heuristic-(c) dispatch blowout | — | Record as a guardrail in the Q-W2v brief (pairs with T4-14's whole-TU exception) | Same-TU definition is what lets the compiler inline the stack today | +| T4-28 | Warning to Q-W2: audition/preview stays a direct call-through across the 8-TU split; per-mouse-move work stays plain free-function calls | — | Record as a guardrail in the (reshaped) Q-W2 brief — reaffirms the plan's own guardrail; the two added TUs introduce no new risk | — | +| T4-29 | Warning to the processor split: `process()` + per-block helpers stay one TU; the atomic-pointer-swap pattern must not gain a virtual seam | — | Record as a guardrail in the Q-W2v brief | — | +| T4-30 | No other gratuitous indirection found (verified: only extension-side `virtual` is T4-26; VST virtuals are SDK-mandated; layered pure→shell pairs are the discipline, all direct calls) | — | Clean verification — see §5 | — | + +### 2.5 Cross-track dedupes and reconciliations + +1. **T2-05 ≡ T4-21 (+ T4-22 rider) — the rect zoo. Reconciled into ONE disposition: fix-now, + ride Q-W1.** Both tracks found the same duplication (19 XYWH structs + inline predicates; + T4-21 adds the VST side's second LTRB grammar) and both prescribe the same mechanism — one + concrete `ui::Rect` + `contains()` with per-role aliases, explicitly **not** a template + (T4-21's ruling). The only divergence was the wave label: T2-05 said "Q-W2 (the ui/ + relocation wave)", but in the plan the relocation wave — and the settled `ui::` rect + unification (PLAN §Q-W1, "shared pure-UI rect types … get one `ui::` owner") — is **Q-W1**. + T2-05's own rationale ("same-moment-as-relocation") therefore points at Q-W1; reconciled + there. T4-22's hit-test template stays a deferred opportunistic rider behind it. +2. **T3-07 → T1 — the 512-frame correlation cap. Resolved: document-and-defer.** T3 handed the + "is 512 the right number" quality call to Track 1. Track 1's verdict on the correlation + search's bounds (T1-06) is document-and-defer — record the alignment limits as the engine's + stated operating range; "widening either costs splice-burst CPU linearly." Note for accuracy: + T1-06 adjudicates the coarse-step and `maxLag` bounds specifically and does not name the + 512-frame `corrFrames_` cap; the resolution here rests on T1's general verdict (bounded + limits recorded, no change recommended) applied to the same search-cost family. If triage + wants the 512 value separately adjudicated, that is a residual question — flagged rather than + silently absorbed. +3. **T2-07 ≡ T4-13 — the ComponentState codec split. One disposition: Q-W2v's + `component_state_io`.** T2-07's complaint (extension links the whole voice engine to share + the preset serializer) is *solved by* T4-13's proposed split; T2-07's link-weight rationale + rides the T4-13 seam. T4-14's `zone_params.h` header split completes the extension-side + decoupling. T2 had provisionally pointed at "Q-W1/Q-W2 module-homing"; with Q-W2v now + proposed as the wave that opens `sample_map`, that is the owning wave. +4. **T2-06 ≡ T4-11's `editor_layout` — the stranded editor layout math.** T2-06's + document-and-defer explicitly demanded a *named* downstream reshape; Q-W2v's `editor_layout` + TU (T4-11, marked "pure candidate") is that reshape. One disposition: assigned to Q-W2v, with + the hoist targeting the existing pure homes (`editor_geometry` is T2's named natural owner). + T2-06's scope note (the §2 god-module catalogue missed the VST side) is also the evidence + base for Q-W2v existing at all. +5. **T4-20 / T2-03 / T2-04 — related but distinct dedup family, three separate dispositions.** + All three converge on shared `core/` utility homes but touch disjoint code: T4-20 (LE byte + codec ×5) lands with `component_state_io` in Q-W2v; T2-03 (`readFileBytes` ×5) lands in + Q-W1's utility home; T2-04 (ext-state grow-loop ×3, generalizing `bridge_marshal`) lands in + Q-W5 with the persist split. Marked here so triage sees them as one family and no wave + assumes another already covered its slice. +6. **T2-08 / T4-23 / T4-10 — WAV/RIFF consolidation. Genuine track disagreement on the moment; + surfaced as §4e** rather than silently picked. T2-08 defers to "the reorg wave that + relocates `wav_trim`" (Q-W1's relocation); T4-23 assigns to "the wave that opens + `ingest.cpp`" — and T4-10 assigns the ingest split to "whichever wave lands the WAV + consolidation," which is circular: **no current wave opens `ingest.cpp`.** +7. **T1-11 — capture-tag collision.** T1 offered it to Track 2 ("Track 2 may claim it"); + Track 2 did not. It remains a single T1 finding, routed per T1's own suggestion to Q-W3. +8. **Cross-track interaction on the DSP/VST fix-nows.** T1 and T3 both justified + fix-now-in-Q-W0 partly by "no downstream wave opens these files" — written before T4 + proposed Q-W2v, which *does* open `reasampler_processor.cpp` / `reasampler_editor.cpp` / + `sampler_core.h`. The recommendation stands unchanged: Q-W2v is a behavior-preserving + mechanical-split wave, and folding behavior-changing DSP/domain fixes into it would break the + wave discipline (CTest-green mechanical moves, no logic change). Fix-now items stay in Q-W0; + noted so the rationale reads correctly against the reshaped plan. + +--- + +## 3. Plan reshape — what Q-W0 proposes for Q-W1..Q-W6 + +The concrete deltas to the PLAN §Phase Q wave graph. Every wave is named; "no change" +confirmations included deliberately. + +**Q-W0 (this wave, post sign-off) — remediations before close:** T2-01(a) provenance cursor +hardening backport; T3-01 gain-ramp seconds; T3-03 fade-ceiling seconds (or its stated fallback); +T1-07 comment fix; pending §4b — T1-01 linked-lag stereo fix and T1-03 prime bound, with T1-09 +riding any `sampler_core` edit. Each behavior-touching remediation lands with its module's CTest +target green per the Q-W0 verify contract. + +**Q-W1 (safe opener — scope grows):** +- Add `tail_control` to the `core/json` consumer list (T2-02) — the wave's "one JSON path" goal + is not met without it. (T4-25 confirms the original 4× scope is otherwise accurate.) +- Add the shared `readFileBytes` pure helper to the `core/` utility home (T2-03). +- Rect unification: one concrete `ui::Rect` + `contains()` + per-role aliases, including + retiring the XYWH-vs-LTRB fork and folding in `editor_geometry`'s `Rect` (T2-05 ≡ T4-21); + `clamp01` dedup rider (T4-24); `hitIndex` template only as an opportunistic follow-on (T4-22). +- Structural collapse of the wire `Cursor` family into one shared `wire` codec module beside + `core/json`, consumed by `provenance` / `assignment_request` / `sample_usage` / + `parseBankGeneration` (T2-01(b)). +- `slot_map` extraction rider on the `bank_book` JSON rewire (T4-05); `view_mode_model` planner + split rider (T4-06, optional per T4); `bank_model` unchanged-as-planned (T4-07); `view_lanes` + split only if relocation touches `view.cpp` anyway (T4-09). +- Relocation scope grows to include the ~20 clean VST pure libs, under whichever placement shape + §4a settles (T4 §1.5, T4-18/T4-19). + +**Q-W2 (bank_panel split — 6→8 seams):** the wave brief must name **eight** TUs — the planned +six plus `panel_layout` and `panel_drag` (T4-01) — or two of its TUs ship >600 on day one. +Guardrails reaffirmed: audition direct call-through; per-mouse-move work stays plain free-function +calls (T4-28). + +**Q-W2v (NEW — VST god-module wave; the T2-06/T4 §1.5 scope gap made structural):** +- `reasampler_editor.cpp` → eight TUs: session / controls / **layout (pure-candidate hoist into + the existing pure homes — this discharges T2-06)** / paint_sample / paint_browse_zone / + input_sample / input_browse_zone / platform (T4-11). +- `reasampler_processor.cpp` → three TUs: `processor_state` / `processor_reload` / + lifecycle+`process()` kept whole (T4-12), with the T4-29 guardrail (no virtual seam on the + atomic-swap pattern). +- `sample_map` → `component_state_io` codec split + matching header split (T4-13 ≡ T2-07). +- `sampler_core`: **TU stays whole at 968 — documented hot-path exception** recorded in the wave + brief (T4-14, T4-27); header splits into `zone_params.h` + `sampler_core.h`. +- The `core/wire/bytes.h` LE-codec template lands here with its biggest consumer (T4-20). +- Rider: adopt the pure `ThumbnailKey` on the VST side while the editor is open (T2-10). +- **Scheduling:** parallel-safe with Q-W2 (different artifact, zero file overlap) — T4 also + offers serial-after-W5 as "Q-W7" if Daniel prefers serial waves (§4f). + +**Q-W3 (main.cpp split — 3→4 hoists):** add **`capture_batch`** (batch family + +`RunRecaptureFromSource` + the two selection guards) as a fourth TU so `capture_orchestrator` +lands ~450 (T4-02). Also owned here: **delete `ICaptureBackend`** and correct the +CLAUDE.md/CONTEXT description in the same commit (T4-26); the shared `stampCaptureSample` +epilogue dedup (T2-09); the `capture_realtime_finalize` split riding the Q-9 naming rider +(T4-08); the `makeUniqueTag` monotonic-counter fix (T1-11). + +**Q-W4 (actions split): no change** — the planned seams still land sub-600 (T4-03). + +**Q-W5 (persist split): seams unchanged** (T4-04); **add** the ext-state grow-loop dedup — +generalize the retry policy into `bridge_marshal` (or its `core/` successor) and rewire all three +loops, `usage_scan`'s prune-safety-adjacent copy included (T2-04). + +**Q-W6 (registration table): no change**; T4-02 notes the `main.cpp` registration residue (~385) +shrinks further under its table. + +**Unassigned pending §4e:** the WAV/RIFF consolidation family (T2-08 / T4-23 / T4-10) — no +current wave opens `ingest.cpp`; the owning moment is Daniel's call. + +Updated dependency sketch: + +``` +Q-W0 (this report + remediations) ── SUB-GATE: Daniel signs off every disposition (§4) ── + ▼ +Q-W1 (core/json + wire codec + relocation incl. VST pure libs + rect unification + riders) + ├─► Q-W2 (bank_panel split, 8 seams) ──► Q-W4 (actions; unchanged) + ├─► Q-W2v (NEW: VST god-modules — editor/processor/sample_map splits, sampler_core header, + │ LE codec) [parallel-safe with Q-W2; serial "Q-W7" alternative — §4f] + ├─► Q-W3 (main split, 4 hoists; ICaptureBackend deletion + doc fix; stamp dedup; T1-11) + │ └──► Q-W6 (registration table; unchanged) + └─► Q-W5 (persist; seams unchanged; + ext-state-loop dedup) [best after Q-W4] +``` + +--- + +## 4. Daniel's decision list (sign-off gate) + +Each item: leading recommendation first, alternative second. Settled matters (Q-10, Q-11 framing, +the clean bills) are deliberately absent. + +- **(a) VST placement fork — T4-18 vs T4-19.** *Recommend T4-18:* integrate `src/vst/` into the + one `core/`/`shell/` top split with `instrument/{engine,map,ui}` subsystem dirs — one rule + ("directory = may it touch a host type"), the artifact boundary is a link-graph fact the + sources already straddle, CMake impact is path-edits only. *Alternative T4-19:* artifact-first + subtree (`src/vst/core|shell` behind one SDK-gated `add_subdirectory`) — cleanest expression + of the Windows-only gate, at the cost of two parallel `core/` trees and a subtree whose + self-containment is cosmetic. Q-W1 executes whichever shape is chosen. +- **(b) DSP bounded fixes in Q-W0 — T1-01 and T1-03.** *Recommend fix-now for T1-01* (linked + L/R lag + splice schedule): High severity on the flagship stereo-Preserve path, standard + stereo-SOLA practice, no technique change. *Alternative:* defer and record as the known + stereo-Preserve limitation. *For T1-03* (prime bound + immediate `freezeTail()` on + sub-window spans): T1's own framing — fix-now **if the short-one-shot case matters** to you + (short drum one-shots are realistic content); else document-and-defer with the T1 note as the + record. T1-09 (`declickR_` dead state) rides whichever `sampler_core` edit happens. +- **(c) T2-01(a) provenance wire-cursor hardening backport in Q-W0.** *Recommend yes:* small, + pure, closes a concrete robustness gap on persisted user-editable input, covered by existing + `provenance_tests`. *Alternative:* wait for the Q-W1 wire-codec collapse to fix it + structurally — leaves the gap open through the gate for no saving. +- **(d) Env-constant fix-nows in Q-W0 — T3-01 and T3-03.** *Recommend fix-now for both:* each + is trivial, isolated, and a live hardcoded-rate residue Daniel's standing ruling forbids; + behavior-identical at the baked-in rates. *Alternative for T3-03 only* (T3's stated + fallback): document-and-defer with the comment amended to name the 44.1 k assumption, if zero + UI-feel change is preferred. (Folding either into Q-W2v instead is *not* recommended — it + would put behavior changes inside a mechanical-split wave; §2.5(8).) +- **(e) WAV/RIFF consolidation moment — the one true track disagreement (T2-08 vs T4-23/T4-10).** + T2 prefers the `core/wav` homing moment (the relocation wave); T4 prefers "the wave that opens + `ingest.cpp`" — which does not exist, and T4-10 points back circularly. *Recommend:* record + the consolidation (one pure `wav_codec` owner: walker + layout + build + patch, absorbing + T4-10's ingest extraction) as a named rider on **Q-W3** — the wave already opening the capture + family (`capture_realtime` finalize, T4-08) — with Q-W1-relocation as the alternative moment + if Daniel prefers T2's framing. Either way it must land somewhere named, or the + dedup-by-hash/null-test maintenance surface stays quadruplicated. +- **(f) Q-W2v scheduling.** *Recommend parallel with Q-W2* (different artifact, zero file + overlap — T4 §1.5). *Alternative:* sequence it serially after W5 as "Q-W7" if you want serial + waves throughout. + +--- + +## 5. Clean bills — surfaces audited and found clean + +Consolidated coverage evidence; details in the appendices' clean-surface sections. + +**DSP (T1):** `peaks` (bin partition exact, overflow-guarded, per-channel no-fold), `master_gain` +(taper math correct end to end), `velocity_curve` (Fritsch–Carlson monotonicity/no-overshoot +claims hold) — fully clean. Clean with only the noted findings: `wav_trim` (T1-10 metadata note), +offline capture path (T1-11; precision-invariant plumbing "disciplined"), realtime capture path +(T1-11 + already-in-code DAW-verify flags), `sampler_core`'s voice/steal/mono/panic machinery +(rate-free-seconds invariant honored; takeover blend mathematically sound), `pitch_shift`'s core +machinery (safe-band geometry, clamps, correlation, fades, `freezeTail` continuity all audit +sound; down-shift writer-lap unreachable above ≈ −109 st). + +**Architecture (T2):** pure-module include hygiene across both trees (zero host-type includes in +any claimed-pure module); `bank_sync`; `bridge_marshal`; the realtime record lifecycle (explicit +enum state machine, not implicit); project-identity transitions; `usage_scan` (decisions +delegated pure, depth-bounded recursion, protect-on-truncation); the three `catch (...)` sites +(documented, narrow, non-swallowing); `FxBypassGuard` vs `view.cpp` park/restore (duplication of +shape, not concept — correctly separate); path resolution (`resolveBankFile` is the single +resolver both sides); WAV decode (one decoder); the draw layer (no parallel vocabulary on the VST +side); interface cost (no hot-path virtual/`std::function` chains); boolean parameters (no +smell). Also T2-11's headline: the ComponentState v1→v11 lift chain is **functionally sound** — +the finding is shape, not correctness. + +**Env-coupled constants (T3):** `sample_map` v5+ persistence (the reference implementation); +ComponentState v6–v11 fields; Trigger fade/loop frames as source-file facts with the rate stored +alongside; `trigger_seam`; `kPreserveWindowMs` (the model pattern); `pitch_shift` internal +geometry; the tail system; `wav_trim`; `bank_model` persisted metadata; all ext-state wires; the +envelope schematic's param-domain scaling; every timer surveyed; `peaks` / `waveform_view` / +`master_gain` / `velocity_curve` / `keyboard_strip`. The persistence layer — the category's +highest-stakes surface — is clean end to end. + +**Sizing/indirection (T4):** borderline files needing no action: `capture.cpp` (549), +`reasampler_editor.h`/`reasampler_processor.h` (shrink with their TU splits), `bank_book.h`, +`pitch_shift.cpp` (371 — single responsibility, hot, leave), `persist.h`/`capture.h` (owned by +Q-W6). T4-30: no gratuitous indirection anywhere beyond the dead `ICaptureBackend` — VST-side +virtuals are SDK-mandated, and the layered pure→shell pairs are the load-bearing discipline, all +direct calls. Plan-confirmations: Q-W4 and Q-W5 seams land as planned (T4-03/T4-04); Q-W1's JSON +scope confirmed accurate modulo T2-02 (T4-25).