Γ-W1-T5 remediation: narrow the rate-bound claim, fix baseline/measurement provenance, correct §2.4 framing

Re-derives the splice-cadence inequality and adds a corner probe that FAILS at
P=500 by design, pending a ruling. Names the baseline commit and harness edit,
fixes measurement methodology, corrects the Trigger-AHD/rate coupling framing.

src/core/instrument/engine/time_stretch.h

@@ -11,13 +11,22 @@
 namespace reasampler::instrument::engine {
 
 // The playback rates the Preserve DSP is measured over, and therefore the only ones it
-// accepts. Two independent reasons they are here and not wider:
+// accepts. The ceiling also bounds a voice's per-output-frame feed loop (kMaxFeedPerFrame
-//  - the ceiling is what bounds a voice's per-output-frame feed loop (kMaxFeedPerFrame source
+// source frames) — the RT-safety argument for feeding a variable count at all.
-//    frames), which is the RT-safety argument for feeding a variable count at all;
+//
-//  - the splice search can only align a period it can see. The tap's delay drifts at
+// This range NARROWS the splice-cadence failure onto the source fundamental; it does not
-//    |rate - shift| per frame, so a wide rate over a deep DOWN-shift splices faster than one
+// eliminate it. A splice recurs every `window / |rate - shift|` output frames (the tap's
-//    period of the output tone and the correlation stops holding the pitch: measured at rate
+// delay drifts across one window at that per-frame rate); the shifted tone's own period is
-//    4.0 with -24 st, the observed period came out 539 frames against 785 wanted.
+// `sourcePeriod / shift` output frames. Whenever the recurrence interval is shorter than
+// that period, a splice lands inside a single perceived cycle and the correlation search
+// has less than one period to align against. Measured at rate 4.0, shift 0.25 (-24 st):
+// interval 2205/3.75 ~= 588 vs period ~4*P ~= 785 frames (P ~= 196) — matches the originally
+// observed 539-vs-785 failure. This range's ceiling (2.0, not 4.0) raises the safe floor, it
+// does not remove it: at rate 2.0, shift 0.25, interval = 2205/1.75 = 1260 still fails for
+// any source period P > 315 frames (~140 Hz at 44.1k) — inside bass/low-vocal material, and
+// -24 st is reachable from the Pitch knob alone. (The pre-stretch rate-1.0 engine's floor by
+// the same inequality is P > 735, ~60 Hz — what this range raises the floor from, not what
+// it removes.)
 inline constexpr double kStretchRateMin = 0.5;
 inline constexpr double kStretchRateMax = 2.0;
 inline constexpr int kMaxFeedPerFrame = 2;  // ceil(kStretchRateMax)

src/core/instrument/engine/voice.cpp

@@ -327,6 +327,8 @@ void Voice::retune(int note) {
     // Filter key-tracking follows the pitch: it is a function of the note, so a slide moves it
     // too. The velocity offset deliberately stays the first note's, matching velocityGain_.
     if (filterOn_) updateFilterCutoffBase(note);
+    // stretchRate_ (Preserve's duration control) is untouched here too — it is a note-on latch
+    // like velocityGain_, not a per-note property to re-resolve on a legato slide.
 }
 
 void Voice::release() {

src/core/instrument/engine/voice.h

@@ -192,9 +192,9 @@ private:
     // This frame's amplitude in [0,1] from the active envelope. Spline: the drawn contour read
     // at the normalized position (one cached-segment compare per frame). Gate: AHDSR ticks once
     // per output frame (envelope time is wall-clock, independent of read rate). Trigger: the AHD
-    // is evaluated at the source offset (readPos - startFrame) so its stages anchor to source
+    // is evaluated at the source offset (readPos - startFrame) — see the `ratio_ = stretchRate_`
-    // frames regardless of pitch engine. Sets amplitudeDone_ on finish so advanceFrame frees
+    // note below for what that means for Preserve's stage-time/rate coupling. Sets
-    // the voice.
+    // amplitudeDone_ on finish so advanceFrame frees the voice.
     double tickAmplitude() {
         double amp;
         // playMode_ is Trigger whenever a spline is genuinely reachable (resolvePlay forces it —
@@ -454,7 +454,10 @@ private:
             // 2^((note-root + pitchEnvSemis)/12) — the pitch envelope adds to the shift amount,
             // never to the read rate. The feed runs one window ahead of readPos_ (the rings were
             // primed with that window at start()), under the same sustain-loop wrap rule,
-            // reading integer source frames (nothing to interpolate).
+            // reading integer source frames into the ring — no RATE-DEPENDENT interpolation
+            // (unlike Varispeed's readPos_ below). The shifter's own read tap still carries a
+            // splice's sub-sample `frac` (pitch_shift.cpp), so it interpolates on every read,
+            // splice or no; that constant fractional delay is not a rate coupling.
             const bool stereoOut = stereo && haveR && shiftR_.configured();
             // The last real source frame is playEnd_-1 for Trigger or frameCount-1 for Gate.
             // Once the feed reaches that bound the source is exhausted — feeding the held last
@@ -528,6 +531,16 @@ private:
             // Preserve advances the read head at the STRETCH rate — the one duration control.
             // Everything downstream of it (the loop wrap, the Trigger span, the spline phase)
             // therefore stays a source-frame fact and scales by construction.
+            //
+            // Consequence (§2.4 of instrument-control-surface.md is explicit that staged
+            // envelopes' stage times are wall-clock and do NOT scale with rate): Trigger's amp
+            // AHD and filter AHD are both evaluated at sourceOffset() = readPos_ - startFrame_
+            // (tickAmplitude/tickFilterCutoff above), which now advances at stretchRate_ instead
+            // of always 1.0 — so those two envelopes will scale with a future non-unity Rate.
+            // This is NEW here: Preserve's ratio_ was pinned at 1.0 before this track, so those
+            // stage times were exact wall-clock. It is latent (nothing publishes a non-unity
+            // rate yet) and owned by the track that adds the Rate control, not this one — Gate's
+            // AHDSR (env_.tick(), per-output-frame) and every spline contour are unaffected.
             ratio_ = stretchRate_;
         } else {
             // VARISPEED: pitch and duration coupled. The read rate carries the repitch; the

tests/test_pitch_shift.cpp

@@ -737,6 +737,38 @@ static void testStretchAndShiftComposeSafely() {
     }
 }
 
+// The [0.5, 2.0] rate bound (time_stretch.h) narrows the splice-cadence failure onto the
+// source fundamental rather than eliminating it. At rate 2.0, shift 0.25 (-24 st) — both
+// inside the shipped range — the header's own derivation puts the safe-source floor at a
+// period of 315 frames (~140 Hz @ 44.1k): testStretchAndShiftComposeSafely's probe period of
+// 196.37 frames (~225 Hz) sits ABOVE that floor, so it passes because of the probe, not
+// because of headroom. This probe sits BELOW the floor on purpose, asserting the corner
+// rather than assuming it. A failure here is the inequality's PREDICTED outcome, not a
+// defect this test exists to chase — report it, don't retune the tolerance to hide it.
+static void testStretchCadenceBelowSafeFloorAtRate2ShiftQuarter() {
+    const std::int64_t w = 2205;
+    const double rate = 2.0;
+    const double shift = std::pow(2.0, -24.0 / 12.0);  // 0.25
+    for (double period : {500.0, 600.0, 700.0}) {
+        const double f0 = 1.0 / period;
+        const std::size_t srcLen = 400000;
+        std::vector<AudioSample> src(srcLen);
+        for (std::size_t i = 0; i < srcLen; ++i) {
+            src[i] = static_cast<AudioSample>(std::sin(2.0 * kPi * f0 * static_cast<double>(i)));
+        }
+        const std::size_t outFrames = 60000;
+        const std::vector<double> out = runStretch(src, w, rate, shift, outFrames, nullptr);
+        for (double v : out) CHECK(std::isfinite(v));
+        const double p = periodIn(out, 20000, 50000);
+        const double want = period / shift;
+        const bool ok = approx(p, want, want * 0.12);
+        std::printf("  [floor probe] period %.0f (rate 2.0, -24 st): observed %.2f want %.2f "
+                    "-> %s\n", period, p, want, ok ? "held" : "FAILED (predicted by the "
+                    "inequality in time_stretch.h)");
+        CHECK(ok);
+    }
+}
+
 // The two new entry points on a shifter that was never configured (a Varispeed voice's) —
 // neither may touch the empty ring.
 static void testStretchEntryPointsOnPassThrough() {
@@ -758,6 +790,7 @@ int main() {
     testStereoLinkedLagSharedSchedule();
     testStretchMovesDurationNotPitch();
     testStretchAndShiftComposeSafely();
+    testStretchCadenceBelowSafeFloorAtRate2ShiftQuarter();
     testStretchEntryPointsOnPassThrough();
 
     if (g_fail == 0) {

tests/test_sampler_core.cpp

@@ -20,11 +20,11 @@
 #include "../src/core/instrument/engine/voice_engine.h"
 
 #include <algorithm>
+#include <chrono>
 #include <cmath>
 #include <cstdint>
 #include <cstdio>
 #include <cstring>
-#include <ctime>
 #include <vector>
 
 using namespace reasampler;
@@ -2947,11 +2947,13 @@ static void renderVoice(const SampleData& s, int note, double rate, std::int64_t
 }
 
 // --- The null case, asserted against a baseline the SHIPPED engine produced. ---
-// The four constants below were captured by running this same function against the
+// The four constants below are a witness against `phase-g`'s tip, commit 0a7778b — the last
-// pre-stretch build (phase-g, before the rate seam existed) and printing the hashes; they are
+// commit before this track's rate seam — not a self-consistency check. To re-derive: check
-// therefore a witness that the generalized read path reproduces the shipped Preserve output
+// out 0a7778b, add this file's stretchProbeSample/hashStream/renderVoice/test body to it, and
-// bit for bit at rate 1.0, not a self-consistency check. A change here is a change to what
+// drop the trailing `, rate` argument from renderVoice's `v.start(...)` call (0a7778b's
-// every already-saved project sounds like — re-derive the cause before re-baselining.
+// Voice::start has no 5th parameter) — then build, run, and print the hashes. A change here is
+// a change to what every already-saved project sounds like — re-derive the cause before
+// re-baselining.
 static void testPreserveUnityRateIsBitIdenticalToTheShippedRead() {
     const std::int64_t w = 2205;   // the product window at 44.1k
     const std::size_t n = 6000;
@@ -3084,11 +3086,16 @@ static void testPreserveStretchSpeaksOnFrameZeroAtEveryRate() {
             // The primed ring parks the tap ON the start frame, so output frame 0 is source
             // frame `startFrame` exactly — at every rate and every transposition. A stretcher
             // that buffered a window before speaking would fail here, which is the whole point.
+            // This bit-exact check is what actually carries "no first-frame smear"; the loop
+            // below is a coarser, complementary DROPOUT detector (see its own comment).
             CHECK(first == s.frames[500]);
-            // ...and it keeps speaking: no first-window dip while the schedule settles. The
+            // ...and it keeps speaking: no first-window DROPOUT while the schedule settles.
-            // 256-frame measuring window spans most of a period even at the lowest note tested
+            // `lo > 0.5` over twenty 256-frame peak windows catches a gap of roughly a window,
-            // (-12 st stretches the probe's 196-frame period to 393), so a continuous tone
+            // but a smeared or phase-scrambled first window can still peak above 0.5 and pass
-            // peaks well above the floor in every one of them and only a real gap can sink it.
+            // here — it cannot see that; the CHECK above is what does. The 256-frame measuring
+            // window spans most of a period even at the lowest note tested (-12 st stretches
+            // the probe's 196-frame period to 393), so a continuous tone peaks well above the
+            // floor in every one of them and only a real gap can sink it.
             double lo = 1e9;
             for (int i = 0; i < 20; ++i) {
                 double peak = 0.0;
@@ -3130,14 +3137,84 @@ static void testPreserveStretchLoopsTheSourceSpan() {
             CHECK(approx(sum / 200.0, 0.5, 0.05));
         }
     }
+
+    // The two assertions above hold even if stretchRate_ were ignored outright — the loop's
+    // constant content proves nothing about cadence. A one-time marker AFTER the primed window
+    // but BEFORE the loop start is the source-frame witness that the feed genuinely consumes
+    // source AT THE RATE: note-on primes the ring with the first `window` source frames up
+    // front (played back at 1 frame/output-frame, independent of rate — a marker inside that
+    // span was measured landing at a FIXED output frame at every rate, confirming it is not a
+    // rate witness). Past it, new content only enters the ring via the ongoing due()-scheduled
+    // feed, at `rate` source frames per output frame on average: the marker's single output
+    // appearance lands at `window + (markerFrame - window) / rate` output frames. Note == root
+    // (shift == 1.0), isolating the rate's effect from the pitch engine's own transposition.
+    //
+    // Excludes rate 2.0: at shift 1.0 that is drift = |rate-shift| = 1.0 exactly, and this
+    // geometry's own splice trigger (0.75x window output frames from note-on, measured) fires
+    // BEFORE the primed span even finishes playing back (< window frames) whenever drift >=
+    // ~0.75 — so no marker placed "past the prime" can be reached before a splice relocates
+    // the tap first. Confirmed by measurement, not assumed: a rate-2.0 attempt at this marker
+    // came back with the tap having moved on (no witness value in the output at all). The
+    // write-side consumption-at-the-rate claim at every rate, splice-immune because it never
+    // goes through the shifter, is what test_time_stretch.cpp's StretchCursor tests assert.
+    //
+    // A marker placed INSIDE the steady-state loop instead would NOT show rate-dependence
+    // either: once ring-resident, the read tap's own pace is governed by SHIFT alone ("shift
+    // the output" — ratio_ advances posA_ every output frame unconditionally), so it revisits
+    // every loopLength ring slots at 1 slot/output-frame regardless of how fast the writer
+    // filled them — confirmed by measurement (median recurrence gap 200 frames at rate 0.5,
+    // 1.0 AND 2.0 alike, for a 200-frame loop). Rate governs the feed/splice cadence, not the
+    // loop's own output period, once its content is already in the ring.
+    for (double rate : {0.5, 1.0}) {
+        SampleData s;
+        s.frames.assign(1000, 0.0f);
+        for (int i = 300; i < 900; ++i) s.frames[i] = 0.5f;
+        s.frames[650] = 1.0f;  // past the 600-frame primed span, before the loop at 700
+        s.rootNote = 60;
+        s.sampleRate = 48000;
+        s.loop.hasLoop = true;
+        s.loop.start = 700;
+        s.loop.end = 900;
+        s.play.adsr = flatAdsr();
+        s.play.pitchEngine = PitchEngine::Preserve;
+        Voice v;
+        v.presizePreserveShifters(600);
+        v.start(60, 127, s, /*declickTakeover=*/false, rate);  // root note: shift == 1.0
+        const std::size_t total = 3000;
+        std::vector<AudioSample> out(total);
+        for (std::size_t i = 0; i < total; ++i) out[i] = v.renderFrame();
+        std::size_t hitAt = 0;
+        for (std::size_t i = 0; i < total; ++i) {
+            if (out[i] > 0.7f) { hitAt = i; break; }
+        }
+        CHECK(hitAt > 0);
+        const double want = 600.0 + (650.0 - 600.0) / rate;
+        if (!approx(static_cast<double>(hitAt), want, want * 0.15 + 5.0)) {
+            std::printf("  rate %.2f: marker at frame %zu want %.2f\n", rate, hitAt, want);
+        }
+        CHECK(approx(static_cast<double>(hitAt), want, want * 0.15 + 5.0));
+    }
 }
 
 // --- The 32-voice measurement gate. Asserts correctness; PRINTS the cost, which is the
-//     number reported for the algorithm decision (meaningful only in a Release build). ---
+//     number reported for the algorithm decision (meaningful only in a Release build).
+//
+// Methodology: std::chrono::steady_clock (not std::clock() — a single wall-clock diff has no
+// warm-up and no spread), kWarmupReps discarded, kTimedReps repetitions per rate, median +
+// [min, max] reported. secs is wall-clock for 1.0 s of audio on ONE thread with no other work
+// scheduled onto it, so 100*secs is % of REALTIME consumed — not "% of one core" (that would
+// additionally claim core-pinned exclusivity this benchmark never establishes).
+//
+// A separate, one-off Release A/B (unity-now vs the pre-stretch build at commit 0a7778b, same
+// methodology, standalone harness outside this tree) found the two statistically
+// indistinguishable at ~79-82 ns/voice/frame; that is a point-in-time finding to re-derive if
+// this path changes materially, not a hardcoded regression bound here. ---
 static void testPreserveStretchThirtyTwoVoicesHoldUp() {
     const std::int64_t w = 2205;                  // the product window at 44.1k
     const std::size_t blockFrames = 44100;        // one second of audio
     const std::size_t voiceCount = 32;
+    const int kWarmupReps = 2;
+    const int kTimedReps = 7;
     SampleData s = stretchProbeSample(200000, true);
     s.loop.hasLoop = true;                        // held notes: all 32 sound for the whole run
     s.loop.start = 40000;
@@ -3147,36 +3224,49 @@ static void testPreserveStretchThirtyTwoVoicesHoldUp() {
     // 1.0 is the reference: it is the cost the shipped Preserve read already carries, so the
     // two stretched rows are read as a delta against it rather than in isolation.
     for (double rate : {1.0, 0.5, 2.0}) {
-        std::vector<Voice> voices(voiceCount);
+        std::vector<double> nsPerVoiceFrame;
-        for (std::size_t i = 0; i < voiceCount; ++i) {
+        nsPerVoiceFrame.reserve(kTimedReps);
-            voices[i].presizePreserveShifters(w);
+        for (int rep = 0; rep < kWarmupReps + kTimedReps; ++rep) {
-            voices[i].start(48 + static_cast<int>(i), 100, s, /*declickTakeover=*/false, rate);
+            std::vector<Voice> voices(voiceCount);
-        }
-        const std::clock_t t0 = std::clock();
-        double guard = 0.0;
-        std::size_t sounding = 0;
-        for (std::size_t f = 0; f < blockFrames; ++f) {
-            AudioSample l = 0.0f, r = 0.0f;
             for (std::size_t i = 0; i < voiceCount; ++i) {
-                AudioSample a = 0.0f, b = 0.0f;
+                voices[i].presizePreserveShifters(w);
-                voices[i].renderFrameStereo(a, b);
+                voices[i].start(48 + static_cast<int>(i), 100, s, /*declickTakeover=*/false,
-                l += a;
+                                 rate);
-                r += b;
+            }
+            const auto t0 = std::chrono::steady_clock::now();
+            double guard = 0.0;
+            std::size_t sounding = 0;
+            for (std::size_t f = 0; f < blockFrames; ++f) {
+                AudioSample l = 0.0f, r = 0.0f;
+                for (std::size_t i = 0; i < voiceCount; ++i) {
+                    AudioSample a = 0.0f, b = 0.0f;
+                    voices[i].renderFrameStereo(a, b);
+                    l += a;
+                    r += b;
+                }
+                guard += static_cast<double>(l) + static_cast<double>(r);
+                CHECK(std::isfinite(l) && std::isfinite(r));
+            }
+            const double secs =
+                std::chrono::duration<double>(std::chrono::steady_clock::now() - t0).count();
+            for (std::size_t i = 0; i < voiceCount; ++i) {
+                if (voices[i].active()) ++sounding;
+            }
+            CHECK(sounding == voiceCount);  // all 32 held the whole second (the loop kept them up)
+            CHECK(std::fabs(guard) > 0.0);  // ...and genuinely produced audio
+            if (rep >= kWarmupReps) {
+                nsPerVoiceFrame.push_back(secs * 1e9 / (static_cast<double>(blockFrames) *
+                                                         static_cast<double>(voiceCount)));
             }
-            guard += static_cast<double>(l) + static_cast<double>(r);
-            CHECK(std::isfinite(l) && std::isfinite(r));
         }
-        const double secs = static_cast<double>(std::clock() - t0) / CLOCKS_PER_SEC;
+        std::sort(nsPerVoiceFrame.begin(), nsPerVoiceFrame.end());
-        for (std::size_t i = 0; i < voiceCount; ++i) {
+        const double medianNs = nsPerVoiceFrame[nsPerVoiceFrame.size() / 2];
-            if (voices[i].active()) ++sounding;
+        const double secsAtMedian =
-        }
+            medianNs * static_cast<double>(blockFrames) * static_cast<double>(voiceCount) / 1e9;
-        CHECK(sounding == voiceCount);   // all 32 held the whole second (the loop kept them up)
+        std::printf("  [measure] 32 stereo Preserve voices @ rate %.2f: median %.1f ns/voice/"
-        CHECK(std::fabs(guard) > 0.0);   // ...and genuinely produced audio
+                    "frame [%.1f .. %.1f] over %d reps (%.1f%% of realtime at the median)\n",
-        std::printf("  [measure] 32 stereo Preserve voices @ rate %.2f: %.3f s wall for 1.0 s "
+                    rate, medianNs, nsPerVoiceFrame.front(), nsPerVoiceFrame.back(), kTimedReps,
-                    "audio (%.1f%% of one core, %.1f ns/voice/frame)\n",
+                    100.0 * secsAtMedian);
-                    rate, secs, 100.0 * secs,
-                    secs * 1e9 / (static_cast<double>(blockFrames) *
-                                  static_cast<double>(voiceCount)));
     }
 }