371 lines
38 KiB
Markdown
371 lines
38 KiB
Markdown
# 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).
|