Q-W0 code-review remediation: pitch_shift follower self-heal, T3-03 rate bail, doc/comment riders

docs/product/code-quality-audit.md

@@ -315,6 +315,14 @@ the clean bills) are deliberately absent.
   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

src/provenance.cpp

@@ -140,6 +140,9 @@ public:
 
 private:
     bool fail() { ok_ = false; return false; }
+    // TODO(Q-W1): strtol does not check errno/range here, so an out-of-range field narrows
+    // silently to LONG_MAX (then truncates into `int`) instead of failing parse. Flagged for
+    // the Q-W1 wire-codec collapse rather than fixed in place.
     static bool toInt(const std::string& f, int& out) {
         const char* b = f.c_str();
         char* end = nullptr;

src/vst/pitch_shift.cpp

@@ -201,8 +201,9 @@ void PitchShifter::splice(std::int64_t nominalJump, double delay) {
     // search (and the +/-1-lag parabolic refinement calls at bestLag ± 1, and the interpolator's
     // read-ahead) can touch is delay d + jump + maxLag + 2 (maxLag from the coarse/fine search,
     // +1 for the parabola's outer ± 1 probe, +1 for the interpolator's i1 = i0+1 read-ahead),
-    // so the tight cap is filled_ - d - maxLag_ - 2. The code uses - 1 here — one sample of
-    // conservative margin, never out-of-range. In steady state (filled_ == ringLen_) this is
+    // so the tight cap is filled_ - d - maxLag_ - 2. The code uses - 1 here — one sample LOOSER
+    // than that derived cap (not extra margin); ring indexing wraps via modulo everywhere, so
+    // this never runs off the physical ring_ array. In steady state (filled_ == ringLen_) this is
     // > window_ and the nominal jump is untouched; near a primed onset it shrinks the jump to
     // what real history exists (still many source periods with a full-window prime). The floor of
     // 1 is only reachable on the documented degenerate reset-without-prime path — garbage-tolerant.
@@ -375,7 +376,28 @@ AudioSample PitchShifter::processImpl(AudioSample in, const SpliceEvent* linked)
         //     master channel did this frame. Lockstep state means our own trigger would have
         //     fired on the same frame; applying the master's decision keeps the two rings
         //     sample-aligned (one shared lag, one shared schedule).
-        if (linkedEv.fired) applySplice(linkedEv);
+        if (linkedEv.fired) {
+            applySplice(linkedEv);
+        } else {
+            // Self-healing fallback (review rider): the master not firing normally means this
+            // channel's own trigger wouldn't fire either (lockstep). But if the processor ever
+            // renders a mono block mid-note, this follower channel is skipped for that block
+            // while the master keeps advancing — its writePos_/filled_ falls behind and, with
+            // only the `if (linkedEv.fired)` path above, could never resync. So check this
+            // follower's OWN tap distance against the safe band and splice via its own search
+            // when it has left [dLow_, dHigh_], exactly as the master would. Reuses splice() —
+            // no allocation, no new RT cost. In the normal (non-mono-block) case this branch
+            // never triggers: the master's trigger fires first and this whole `if` is false.
+            double d = static_cast<double>(writePos_) - posA_;
+            const double len = static_cast<double>(ringLen_);
+            while (d < 0.0) d += len;
+            while (d >= len) d -= len;
+            if (d <= static_cast<double>(dLow_)) {
+                splice(+window_, d);
+            } else if (d >= static_cast<double>(dHigh_)) {
+                splice(-window_, d);
+            }
+        }
     } else {
         // 3. Splice scheduling: relocate when the active tap's delay leaves the safe band.
         //    Up-shifts (ratio > 1) drain the delay toward 0 -> jump one window OLDER; down-

src/vst/reasampler_editor.cpp

@@ -405,10 +405,14 @@ double clamp01(double v) { return v < 0.0 ? 0.0 : (v > 1.0 ? 1.0 : v); }
 double ReaSamplerEditor::controlValue(int id, const ZonePlaySeconds& play) const {
     // Wall-clock seconds -> normalized over the seconds ceiling; source frames -> normalized over
     // the rate-resolved frames ceiling (T3-03). Two domains, kept explicit so neither leaks a rate.
+    // A stored fade exceeding fadeMaxFrames() at the current host rate reads as norm 1.0 (clamp01
+    // pins it) and gets rewritten down on the next knob touch — deliberate, matching the old
+    // fixed-ceiling clamp behavior in kind, just rate-dependent now instead of fixed at 88200.
     const double fadeMax = fadeMaxFrames();
     const auto secToNorm = [](double s) { return clamp01(s / kEnvTimeMaxSeconds); };
     const auto framesToNorm = [fadeMax](std::int64_t f) {
-        return clamp01(static_cast<double>(f) / fadeMax);
+        // Ceiling unavailable (rate not yet known): inert until fadeMaxFrames() resolves.
+        return fadeMax > 0.0 ? clamp01(static_cast<double>(f) / fadeMax) : 0.0;
     };
     switch (static_cast<ParamControl>(id)) {
         case ParamControl::kPlayMode:      return play.playMode == PlayMode::Trigger ? 1.0 : 0.0;
@@ -435,7 +439,9 @@ void ReaSamplerEditor::applyControl(int id, ZonePlaySeconds& play, double value,
                                     int segment) const {
     const double fadeMax = fadeMaxFrames();  // T3-03: rate-resolved knob full-scale
     const auto normToSec = [](double v) { return clamp01(v) * kEnvTimeMaxSeconds; };
-    const auto normToFrames = [fadeMax](double v) {
+    const auto normToFrames = [fadeMax](double v) -> std::int64_t {
+        // Ceiling unavailable (rate not yet known): inert until fadeMaxFrames() resolves.
+        if (fadeMax <= 0.0) return 0;
         return static_cast<std::int64_t>(clamp01(v) * fadeMax + 0.5);
     };
     switch (static_cast<ParamControl>(id)) {
@@ -476,12 +482,14 @@ double ReaSamplerEditor::fadeMaxFrames() const {
     // T3-03: the Trigger-fade knob's full-scale throw is kFadeMaxSeconds (2 s wall-clock)
     // resolved against the live rate — the SAME time base the envelope overlay already uses
     // to place these source-frame fades on screen (totalSeconds = frames / liveSampleRate()),
-    // and the rate captures are made at (the capture path renders at the project rate). The
-    // 44.1 kHz fallback covers the pre-setupProcessing window (rate still 0) and reproduces
-    // the legacy 88200-frame ceiling there. Storage stays SOURCE FRAMES — this resolves the
-    // UI ceiling only.
+    // and the rate captures are made at (the capture path renders at the project rate).
+    // Pre-setupProcessing the rate is still 0: rather than substitute a literal rate (the
+    // exact residue T3-03 removed), bail the same way paintEnvelopeOverlay does (~line 1396) —
+    // callers treat a <= 0 return as "ceiling unavailable yet" and degrade the knob to inert
+    // rather than guess a rate. Storage stays SOURCE FRAMES — this resolves the UI ceiling only.
     const double rate = liveSampleRate();
-    return kFadeMaxSeconds * (rate > 0.0 ? rate : 44100.0);
+    if (rate <= 0.0) return 0.0;
+    return kFadeMaxSeconds * rate;
 }
 
 double ReaSamplerEditor::previewVelocity01() const {

tests/test_pitch_shift.cpp

@@ -501,7 +501,19 @@ static void testFreezeTailContinuousTone() {
 //        mono-sum-combing mechanism). The divergence witness: an INDEPENDENT shifter fed the
 //        follower's content picks a different lag on the same schedule, proving the mirror
 //        assertion is not vacuous (the two channels' contents genuinely disagree on the best
-//        alignment). ---
+//        alignment). A third shifter (`mirror`), primed with the SAME content as the master
+//        and driven via processLinked() with the master's own decisions, must reproduce the
+//        master's output BIT-IDENTICALLY every frame — this is the review-rider strengthening:
+//        the `ef == em` mirror check above only proves lastSplice_ was copied verbatim (which
+//        applySplice() always does), not that applySplice() actually reproduces splice()'s
+//        effect on posA_/fadeLen_/audio output; a same-content bit-identical check catches a
+//        real divergence there (e.g. an asymmetry between applySplice()'s unconditional
+//        `max(1, ev.fadeLen)` and splice()'s own fadeLen_ assignment). This driven-every-frame
+//        setup keeps both master and follower in lockstep the whole run (posA_/writePos_ stay
+//        identical since jumps are geometric, not content-dependent), so it exercises
+//        applySplice() on every splice — never the Q-W0 remediation self-healing fallback
+//        (own-search splice on a stale follower), which only fires when a follower has been
+//        skipped a block relative to the master (mono-render-block starvation). ---
 static void testStereoLinkedLagSharedSchedule() {
     const std::int64_t w = 2205;                 // the product window
     const std::size_t n = 40000;                 // ~17 splice cycles at ratio 2
@@ -515,28 +527,34 @@ static void testStereoLinkedLagSharedSchedule() {
         srcL[i] = static_cast<AudioSample>(std::sin(2.0 * kPi * fL * static_cast<double>(i)));
         srcR[i] = static_cast<AudioSample>(std::sin(2.0 * kPi * fR * static_cast<double>(i)));
     }
-    PitchShifter master, follower, independent;
+    PitchShifter master, follower, independent, mirror;
     master.configure(w);
     follower.configure(w);
     independent.configure(w);
+    mirror.configure(w);
     master.prime(srcL.data(), w);
     follower.prime(srcR.data(), w);        // linked: R content, master's decisions
     independent.prime(srcR.data(), w);     // control: R content, OWN search (pre-fix behavior)
+    mirror.prime(srcL.data(), w);          // SAME content as master: bit-identical witness
     master.setShiftRatio(2.0);
     follower.setShiftRatio(2.0);
     independent.setShiftRatio(2.0);
+    mirror.setShiftRatio(2.0);
 
     int spliceCount = 0;
     bool followerDiverged = false;
     bool independentDiverged = false;
+    bool mirrorDiverged = false;
     for (std::size_t i = 0; i < n; ++i) {
         const std::size_t si = i + static_cast<std::size_t>(w);
-        (void)master.process(srcL[si]);
+        const AudioSample oM = master.process(srcL[si]);
         const SpliceEvent& em = master.lastSplice();
         const AudioSample oR = follower.processLinked(srcR[si], em);
         CHECK(std::isfinite(oR));
         // The follower mirrors the master's decision EXACTLY, every frame (fired == false
-        // frames included — a follower must never splice on its own).
+        // frames included). In this driven-every-frame lockstep run the follower never falls
+        // behind, so it never reaches the Q-W0 self-healing fallback — every splice here goes
+        // through applySplice(), same as the mirror check below.
         const SpliceEvent& ef = follower.lastSplice();
         if (ef.fired != em.fired || ef.jump != em.jump || ef.lag != em.lag ||
             ef.frac != em.frac || ef.fadeLen != em.fadeLen) {
@@ -550,10 +568,16 @@ static void testStereoLinkedLagSharedSchedule() {
         if (ei.fired != em.fired || ei.lag != em.lag || ei.frac != em.frac) {
             independentDiverged = true;
         }
+        // The bit-identical witness: same content as the master, master's decisions applied
+        // via applySplice() instead of computed via splice() — the two code paths must produce
+        // the exact same sample stream.
+        const AudioSample oMirror = mirror.processLinked(srcL[si], em);
+        if (oMirror != oM) mirrorDiverged = true;
     }
     CHECK(spliceCount >= 3);        // the run actually exercised several splices
     CHECK(!followerDiverged);       // linked lag: one decision, one schedule, both channels
     CHECK(independentDiverged);     // non-tautology witness: unlinked channels DO disagree
+    CHECK(!mirrorDiverged);         // applySplice() reproduces splice() bit-identically
 }
 
 int main() {