Files
reasampler/docs/product/code-quality-audit.md
T

379 lines
39 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 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).)
**Recorded deviation (Q-W0 remediation, code review):** T3-03 as implemented resolves
`fadeMaxFrames()` against `liveSampleRate()` (the host/project rate), not the per-file rate
this section's text literally suggests ("the loaded source's rate"). Reviewer verified this
is the more correct choice: no resample path exists anywhere in `src/`, the engine advances
one source frame per host frame, and this matches the time base `paintEnvelopeOverlay`
already uses for the same fades (`totalSeconds = frames / liveSampleRate()`). No further
action — recorded here so the audit text and the shipped behavior don't read as diverged.
- **(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` (FritschCarlson 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 v6v11 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).