docs(q-w0): T3 env-coupled-constant audit — 7 findings (2 fix-now: gain-ramp step, fade-ceiling frames); persistence surfaces clean

docs/product/audit-notes/q-w0-t3-env-constants.md

@@ -0,0 +1,220 @@
+# Q-W0 Track 3 — env-coupled-constant domain-modeling audit
+
+Static analysis, 2026-07-28, branch `pq-w0-audit`. Scope: any value stored in an
+environment-coupled domain — frames, sample rate, DPI, pixels, tick cadence — that should be
+stored **rate-free / device-free and resolved at the point of use** (PLAN.md §Q-W0 env-coupled
+bullet; `docs/product/code-organization.md` §2c.3; the load-bearing `sample_map` seconds
+invariant). Findings are domain-modeling calls, not "rescale by rate" patches. The judgment bar
+applied: a finding requires (a) an env-coupled *stored* domain AND (b) an environment that can
+actually change under it. Frame counts computed transiently from seconds at the use site are
+correct and are not reported.
+
+Waves referenced for disposition: Q-W1..Q-W6 open `bank_panel.cpp`, `main.cpp`, `actions.cpp`,
+`persist.cpp`, and relocate the clean pure libs — **no downstream wave opens
+`reasampler_processor.cpp` / `sampler_core.h` / `reasampler_editor.cpp` for logic change**, so
+fix-now findings in those files must be remediated in Q-W0 itself.
+
+---
+
+## Findings
+
+### T3-01 — master-gain ramp step is a frame-domain constant anchored to 48 kHz
+
+- **Location:** `src/vst/reasampler_processor.cpp:46-51` (`kGainRampRate = 1.0f / 960.0f`,
+  `kGainRampSnap`), applied per-sample at `:1069-1085` (stereo) and `:1114-1126` (mono).
+- **Stored vs. correct domain:** stored as a **per-sample linear step** — `960` is literally
+  20 ms × 48 000 Hz, and the comment says so ("960 samples @ 48 kHz ≈ 20 ms"). The intended
+  quantity is a **wall-clock ramp time** (~20 ms); the correct model is a seconds/ms constant
+  with the per-sample step derived from `sampleRate_` at `setupProcessing` — exactly the
+  pattern the same file already uses two paragraphs away for `kPreserveWindowMs`
+  (`:618-623`, `:733-735`).
+- **What breaks when the environment shifts:** the ramp's wall-clock length halves at 96 kHz
+  (~10 ms) and quarters at 192 kHz (~5 ms); at 44.1 kHz it stretches to ~21.8 ms. The FB1
+  "no zipper" contract degrades silently as the host rate rises. Not persisted, so no on-disk
+  breakage — but it is a live hardcoded-rate assumption in `src/`, which Daniel's standing
+  ruling forbids.
+- **Severity:** Med.
+- **Disposition:** **fix-now, remediated in Q-W0.** Rationale: trivial, isolated,
+  behavior-identical at 48 kHz; no downstream wave opens this file, so deferring means keeping
+  a named violation of the no-hardcoded-rate ruling through the whole reorg. Store
+  `kGainRampSeconds = 0.020`, derive the step from the live rate where `sampleRate_` is set.
+
+### T3-02 — takeover-declick decay is a per-frame coefficient (documented deliberate)
+
+- **Location:** `src/vst/sampler_core.h:409-410` (`kDeclickDecay = 0.95`,
+  `kDeclickFloor = 1e-4`), applied per output frame in `sampler_core.cpp:514-519`, `:643-645`.
+- **Stored vs. correct domain:** a per-output-frame exponential coefficient; the implied
+  wall-clock decay-to-floor is ~4.2 ms at 44.1 kHz and ~1.9 ms at 96 kHz. The strict
+  domain-model form would be a time constant in seconds resolved to a coefficient at engine
+  build.
+- **What breaks when the environment shifts:** the takeover-blend residue fades ~2× faster at
+  96 kHz. Audibly negligible for a declick micro-ramp — and the in-code comment
+  (`sampler_core.h:397-401`) **already documents this as a deliberate per-frame DSP micro-ramp,
+  "not a stored wall-clock quantity"**, with the 44.1–96 kHz variance stated and accepted.
+- **Severity:** Low.
+- **Disposition:** **document-and-defer.** Rationale: the coupling is already an explicit,
+  written, bounded design decision in the code; converting it buys no audible improvement.
+  Triage should ratify the in-code note as the record.
+
+### T3-03 — Trigger-fade UI throw ceiling hardcodes 2 s × 44 100 as `88200.0` frames
+
+- **Location:** `src/vst/reasampler_editor.cpp:395`
+  (`constexpr double kFadeMaxFrames = 88200.0`), used by `controlValue`/the commit path to
+  normalize the Trigger fade-in/out knobs.
+- **Stored vs. correct domain:** the fade **storage** domain (int64 SOURCE frames, persisted in
+  the zones payload) is settled and correct — a source-timeline fact, invariant under project-
+  rate change (PLAN.md §S15). The *UI ceiling*, however, encodes a wall-clock intent ("2-second
+  max fade throw") as a frame count at an assumed 44.1 kHz source. `88200` is a rate-derived
+  literal in `src/`, brushing the no-hardcoded-rate ruling even though it never touches disk.
+- **What breaks when the environment shifts:** the environment here is the **source file's
+  rate**: a 96 kHz capture's maximum fade throw is ~0.92 s; a 22.05 kHz file gets 4 s. The knob's
+  full-scale meaning silently varies per loaded sample.
+- **Severity:** Low (UI-only, not persisted, comment flags it as a "build-time residual — one
+  place to retune").
+- **Disposition:** **fix-now, remediated in Q-W0.** Rationale: small and contained — replace
+  with `kFadeMaxSeconds = 2.0` resolved against the loaded source's rate at the two normalize
+  sites (the editor already threads `frameCount + rate` through the pack/unpack path,
+  `envelope_edit.cpp:148`); storage domain unchanged. If triage prefers zero UI-feel change,
+  the fallback is document-and-defer with the comment amended to name the 44.1 k assumption.
+
+### T3-04 — drop-hint banner duration stored in sync-timer ticks
+
+- **Location:** `src/vst/reasampler_editor.cpp:2920-2923` (`dropHintTicks_ = 6`), decayed in
+  `onSyncTimer` (`:244-245`); field at `reasampler_editor.h:457`.
+- **Stored vs. correct domain:** a wall-clock intent ("a few seconds of banner") stored as a
+  **count of `kSyncTimerIntervalMs` ticks** (6 × 500 ms). Correct model: a duration in ms,
+  ticks derived — or a `GetTickCount`-style deadline like the bank panel's tooltip already
+  uses.
+- **What breaks when the environment shifts:** retuning the sync cadence (a plausible perf
+  tweak — the 500 ms value is itself a tuning constant) silently changes the banner duration.
+  The comment does state the coupling.
+- **Severity:** Low.
+- **Disposition:** **document-and-defer.** Rationale: cosmetic, self-documenting at the single
+  site, and the cadence and hint decay live three lines apart; a fix is fine to fold in
+  opportunistically if the file is ever opened, but does not justify a Q-W0 edit on its own.
+
+### T3-05 — systemic: no DPI/content-scale support in either UI surface
+
+- **Location:** systemic. VST3 editor: `reasampler_editor.cpp:150-153` (`ViewRect(0,0,840,620)`
+  default and the size floor at `:816`), all `editor_geometry` / `knob_deck` / `curve_popup` /
+  `envelope_overlay` px constants (e.g. the 8 px node min-separation, the 28×28 curve button),
+  cached font sizes in `draw_kit`. Extension side: the LICE-drawn `bank_panel` dock and its
+  geometry modules. No implementation of VST3's `IPlugViewContentScaleSupport` anywhere in
+  `src/vst/` (grep: zero hits for content-scale/DPI), no scale factor threaded through the
+  pure geometry modules.
+- **Stored vs. correct domain:** every layout constant is a **physical device pixel** that
+  silently assumes ~96 DPI. Correct model: logical units × one scale factor resolved at draw
+  time (the pure geometry modules take widths/heights as parameters already, so a scale factor
+  threads through cleanly — the constants are centralized, which is the good news).
+- **What breaks when the environment shifts:** on a 150–200 % Windows display the editor and
+  dock render physically small (or get bitmap-stretched by the host, blurring text); hit
+  targets like the 8 px min node separation shrink below comfortable pointer accuracy.
+  Usability, not correctness — nothing mis-plays and nothing persisted is wrong.
+- **Severity:** Med (usability on modern displays; Windows-only product makes high-DPI common).
+- **Disposition:** **document-and-defer.** Rationale: a proper UI-scaling pass is a feature
+  wave of its own (scale plumbing through ~15 geometry modules + font cache + both shells),
+  far outside Q-W0's remediation budget; deferral should be recorded as a named future phase,
+  and Q-W1's relocation of the geometry modules should keep the constants centralized so the
+  eventual scale factor lands in one place.
+
+### T3-06 — legacy v3 zone-payload lift divides by the *current* project rate
+
+- **Location:** `src/vst/sample_map.cpp:601-605` (v3 lift inside `readZonesPayload`), format
+  note at `sample_map.h:439-456`, `:510-513`.
+- **Stored vs. correct domain:** the v3 blobs (Daniel's beta projects) stored wall-clock times
+  as frames — **the prior incident itself**. The lift converts frames → seconds by dividing by
+  the live `projectRate` threaded in at read time. That is exact only if the project rate today
+  equals the rate in effect when the S15/S16 editor wrote the frames; the write-era rate was
+  never recorded, so a project whose rate changed since lifts skewed times (old/new ratio,
+  e.g. ~8.8 % for 44.1→48 k).
+- **What breaks when the environment shifts:** already broken by construction for
+  rate-changed-since-write projects; a one-time lift residue, after which v5+ re-saves in
+  seconds and the skew is frozen in, silently.
+- **Severity:** Low (legacy-only, beta-project blobs, envelope-time magnitudes; unrecoverable
+  in principle — the missing datum was never written).
+- **Disposition:** **document-and-defer.** Rationale: no better conversion exists; this is the
+  documented residue of the incident that motivated the seconds invariant. Worth one sentence
+  in the code-quality-audit report so the skew is a recorded known, not a mystery bug later.
+
+### T3-07 — SOLA correlation-segment cap of 512 frames (deliberate CPU bound; cross-ref T1)
+
+- **Location:** `src/vst/pitch_shift.cpp:72`
+  (`corrFrames_ = max(1, min(dLow_ - 1, 512))`; rationale comment at `:64-67`).
+- **Stored vs. correct domain:** borderline by design. The quantity being bounded is **work per
+  splice** (multiply-accumulates), which is genuinely frame-domain — a CPU bound *should* be in
+  frames. The side effect is that the correlation segment's wall-clock span halves at 96 kHz
+  (512 frames ≈ 11.6 ms at 44.1 k, ≈ 5.3 ms at 96 k), raising the lowest frequency the
+  alignment search can lock onto at high rates. All other shifter geometry correctly derives
+  from `kPreserveWindowMs` resolved at the live rate.
+- **What breaks when the environment shifts:** alignment quality for low-frequency content
+  degrades somewhat at high host rates; no correctness or persistence impact.
+- **Severity:** Low.
+- **Disposition:** **document-and-defer**, and hand to the T1 DSP audit for the quality call.
+  Rationale: the frame domain is arguably correct for a compute bound; whether 512 is the right
+  *number* is an algorithm-quality question (T1's territory), not a domain-modeling one.
+
+---
+
+## Surfaces checked clean
+
+- **`sample_map` v5+ persistence (the reference implementation):** AHDSR + pitch-env times as
+  SECONDS doubles; no rate constant anywhere in the read/write paths (`kLegacyV3NominalRate`
+  deliberately does not exist); legacy v3 lift takes the rate as a parameter. Clean.
+- **ComponentState envelope v6–v11 fields:** channel mode, assign generation, preview velocity,
+  voice count/mode/trigger, `masterGainLinear` (dimensionless linear), explicit flag,
+  `SampleRefs` (paths + root/loop/channels intrinsics), `instanceGuid` — all rate-free or
+  file-fact domains. Clean.
+- **Trigger `fadeInFrames`/`fadeOutFrames`/`startPoint`/`SampleLoop.start/end` persisted as
+  int64 SOURCE frames:** deliberate, settled source-timeline facts (PLAN.md §S15;
+  `bank_model.h:66-72` documents the loop rationale) — frames *of the file* are invariant under
+  project-rate change; the file's own rate is stored alongside and resolved at decode. Correct
+  domain, not a finding.
+- **`trigger_seam`:** frames↔fraction with `startFrame` threaded both directions; the overlay's
+  fraction domain is expressly rate-invariant. Clean.
+- **`kPreserveWindowMs` (50 ms):** resolved to frames against the live host rate at both call
+  sites (`reasampler_processor.cpp:618-623`, `:733-735`) — the correct pattern, cited here as
+  the model T3-01 should copy.
+- **`pitch_shift` internal geometry:** ring length, fade, lag band, delay band all derived from
+  the rate-resolved `window_`; ratio-scaled live fade length. Clean (T3-07 cap noted above).
+- **Tail system:** `TailSetting.manualMs` persisted in **ms**; `kMaxTailSeconds`/`kMaxTailMs`
+  wall-clock; trim threshold in **dB** with the linear ratio derived
+  (`render_settings.h:59-86`); the realtime decay scan resolves frames against the **file's own
+  authoritative rate** (`capture_realtime.cpp:384-408`). Clean.
+- **`wav_trim`:** frame counts are parsed file facts and transient truncate plans. Clean.
+- **`bank_model` persisted metadata:** source bounds in seconds + PPQ (both stored, each for
+  its consumer); loudness in dB; `sampleRate`/`channelCount` are *recorded facts about the
+  file*, not assumptions; capture tempo + meter stamped at capture time deliberately so the
+  bars.beats read-out is stable under later project meter changes (`card_meta`). Clean.
+- **Ext-state wires** (`banks` JSON, view-mode model, owned manifest, `assignment_request`,
+  `rsusage_*`): no frame-domain values; the rate field in the assign wire is a recorded fact.
+  Clean.
+- **Envelope schematic (`envelope_overlay`/`envelope_edit`):** param-domain px↔seconds scale
+  derived from the live rect (`gatePxPerSecond`), sample-length-free; editor time-slider
+  ceiling is `kEnvTimeMaxSeconds = 2.0` (seconds). Clean (pixel constants themselves fall under
+  the systemic T3-05).
+- **Timers:** bank_panel tooltip delay uses `GetTickCount()` ms against `kTooltipDelayMs = 500`
+  (wall-clock — the pattern T3-04 should copy); editor sync timer is a 500 ms `SetTimer`
+  interval (ms, not ticks); the new-content detector is an event diff per tick with no
+  wall-clock meaning encoded in tick counts; `retireIdleDrain` is idleness-driven, not
+  time-driven. Clean.
+- **`peaks` / `waveform_view` / `master_gain` / `velocity_curve` / `keyboard_strip`:** bins and
+  columns derived from rects at use; dB↔linear taper and curve math dimensionless; key rects
+  from the passed strip rect. Clean.
+
+## Summary
+
+| ID | Location | Stored domain | Severity | Disposition |
+|----|----------|---------------|----------|-------------|
+| T3-01 | `reasampler_processor.cpp:46-51` gain-ramp step | per-sample step (20 ms @ 48 k baked in) | Med | **Fix-now (Q-W0)** — store seconds, derive step from `sampleRate_` |
+| T3-02 | `sampler_core.h:409-410` declick decay | per-frame coefficient | Low | Document-and-defer — deliberate, already documented in-code |
+| T3-03 | `reasampler_editor.cpp:395` fade throw ceiling | 88200 source frames (2 s @ 44.1 k) | Low | **Fix-now (Q-W0)** — seconds ceiling resolved vs. source rate at use |
+| T3-04 | `reasampler_editor.cpp:2923` drop-hint duration | sync-timer ticks | Low | Document-and-defer — cosmetic, coupling stated in-code |
+| T3-05 | systemic (both UI surfaces) | physical px, ~96 DPI assumed; no content-scale | Med | Document-and-defer — a UI-scaling phase of its own; keep geometry constants centralized through Q-W1 |
+| T3-06 | `sample_map.cpp:601-605` v3 legacy lift | frames ÷ *current* project rate | Low | Document-and-defer — unrecoverable legacy residue; record as known skew |
+| T3-07 | `pitch_shift.cpp:72` correlation cap | 512 frames (CPU bound) | Low | Document-and-defer — frame domain arguably correct for a compute bound; hand to T1 for the quality call |
+
+Two fix-now findings (T3-01, T3-03), both assigned to **Q-W0 itself** — no downstream wave
+opens those files for logic change. Five deferrals, each with a recorded rationale. 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 frame-domain persisted value
+is a source-file fact whose rate travels with it.