Close Γ-W1-T7 re-review: pitch-sync cadence math, floor-model regression check, evidence-count fix, one-home comments

New cadence-collapse-band test at P=1470 shows PSOLA eliminates the corner rather than regressing it (18.52% -> 0.00%).

src/core/instrument/CLAUDE.md

@@ -289,13 +289,13 @@ anything for a trigger shape.
 - `engine/loop/` — the sustain loop's ONE validity/clamp fold (`resolveLoop`) plus its pre-seam crossfade geometry and the editor's default handle span; see `engine/loop/CLAUDE.md`. The voice folds it once at note-on; the crossfade weight is header-inline because it rides the per-sample read.
 - `pitch_shift` — hand-rolled **correlation-aligned SOLA** (splice-overlap-add) pitch shifter AND time-stretcher for the Preserve playback mode: one active read tap chases the write head at the shift ratio; each splice jump is refined by a cross-correlation search so the new read point is waveform-aligned, then old and new taps are crossfaded (raised-cosine, amplitude-complementary). Replaces the prior dual-tap OLA whose fixed half-window tap offset caused anti-phase cancellation on many source frequencies. **GA2:** ring buffer **primed with the actual upcoming source** at note-on (was zero-filled) → gap-free frame-0 onset, ~25 ms Preserve onset latency eliminated (Preserve now speaks on frame 0, matching Varispeed), and real-content-bounded tail (last-window tail-truncation gone). No third-party dependencies; RT-discipline: no allocation in `process()`.
   - **The WRITE rate (duration) and the TAP rate (pitch) are independent, and that is the whole time-stretcher** — `writeFrame` for a surplus source frame, `processNoInput` for a starved output frame, plain `process` for the 1:1 case, `setShiftRatio` for pitch, and `setFeedRate` so the splice crossfade is sized against the real drain rate. The header owns the argument, including why this is not the resampled-read-with-a-cancelling-shift the `WDL_Resampler` invariant above forbids.
-  - **Splices are PITCH-SYNCHRONOUS when the source's period is known** (`setSourcePeriod`, fed from `period_detect` via the loader): the nominal jump becomes the multiple of that period nearest the window that still fits the ring's jump bound (~1.25 windows), so an aligned landing point sits at the CENTRE of the correlation search instead of possibly not existing inside it at all. The search is unchanged and still earns its keep — it absorbs the jump's rounding to whole frames and tracks a source whose period drifts. **An unknown period restores the fixed-window geometry byte for byte**; do not "simplify" that fallback into an approximation of it.
+  - **Splices are PITCH-SYNCHRONOUS when the source's period is known** (`setSourcePeriod`, fed from `period_detect` via the loader): the nominal jump becomes the multiple of that period nearest the window that still fits the ring's jump bound (~1.25 windows), so an aligned landing point sits at the CENTRE of the correlation search instead of possibly not existing inside it at all. The search is unchanged and still earns its keep — it absorbs the jump's rounding to whole frames and tracks a source whose period drifts. **An unknown period restores the fixed-window geometry exactly** (`periodAlignedJump`, `pitch_shift.h`); do not "simplify" that fallback into an approximation of it.
 - `period_detect` — the source's own fundamental period, estimated ONCE per load (two-pass YIN:
   a decimated cumulative-mean-normalized difference picks the period, the full-rate difference
   function refines it to a fraction of a frame), so `pitch_shift`'s splice jump can be a whole
-  number of it. **It runs off the audio thread BY LINK GRAPH: `sampler_core` does not link it**,
-  so no TU on the render path can name `detectPeriod` — the same shape as the extension's link
-  graph not gaining the voice engine. Its one caller is the loader (`map/sample_map`'s
+  number of it. **Runs off the audio thread by link graph** (`period_detect.h` is the one home
+  for that invariant) — the same shape as the extension's link graph not gaining the voice
+  engine. Its one caller is the loader (`map/sample_map`'s
   `buildSampleData`), which hands the answer down on `SampleData::sourcePeriodFrames`. A period
   is DERIVED from the audio, so it is cache and not state: nothing persists it, and it takes no
   rung of the payload ladder. **Answering "none" is a first-class result** — noise, polyphony,

src/core/instrument/engine/CMakeLists.txt

@@ -5,10 +5,9 @@ reasampler_pure_library(pitch_shift SOURCES pitch_shift.cpp LINK PUBLIC peaks)
 # specifically the compile-time proof it does not drag in the WDL <windows.h> chain.
 reasampler_test(pitch_shift LINK pitch_shift)
 
-# Deliberately NOT linked by sampler_core, and that omission is the structural proof the
-# detector cannot run on the audio thread: no TU on the render path can name detectPeriod
-# without failing to link in sampler_core_tests, which links sampler_core and nothing else.
-# Its one caller is the loader (map/sample_map), which runs off-thread by construction.
+# Deliberately NOT linked by sampler_core, enforcing period_detect.h's off-audio-thread
+# invariant at build time: sampler_core_tests links sampler_core and nothing else, so no TU
+# on the render path can name detectPeriod without failing to link.
 reasampler_pure_library(period_detect SOURCES period_detect.cpp LINK PUBLIC peaks)
 reasampler_test(period_detect LINK period_detect)
 
@@ -69,6 +68,11 @@ add_executable(preserve_low_frequency_tests
 # does, which is exactly the seam under measurement.
 target_link_libraries(preserve_low_frequency_tests PRIVATE sampler_core period_detect)
 
+# Bridges the two structural proofs above (sample_map never links the voice engine;
+# sampler_core never links period_detect) for the one case that needs both: a REAL detected
+# period reaching a real Preserve render. Its own target rather than extending either.
+reasampler_test(period_render_integration LINK sample_map sampler_core)
+
 # The Preserve read's source-feed schedule — the TIME half beside pitch_shift's PITCH half.
 # Header-only (it sits on the per-sample feed), hence INTERFACE.
 add_library(time_stretch INTERFACE)

src/core/instrument/engine/period_detect.cpp

@@ -145,25 +145,27 @@ PeriodEstimate detectPeriod(const std::vector<AudioSample>& pcm, int sampleRate,
 
     std::vector<double> periods;
     std::vector<double> confidences;
-    // Probes that carried signal — the agreement denominator. A silent block is no evidence
-    // either way and is excluded; every other outcome, a period found or not, is evidence.
+    // Probes that carried signal AND ran the real dip search — the agreement denominator. A
+    // silent block is no evidence either way; a block whose decimated search band or full-rate
+    // refine bracket collapsed to nothing (the two geometry continues below) never ran that
+    // search either, so it is excluded on the same footing as silence, not counted as if it had.
     std::size_t evidence = 0;
     for (std::size_t p = 0; p < probes; ++p) {
         // (probes - 1) * stride <= room by construction, so the last block always fits.
         const std::size_t from = spanFrom + p * stride;
         if (blockRms(pcm, from, block) < kSilenceRms) continue;
-        ++evidence;
 
         const std::vector<double> small = decimate(pcm, from, block);
         const std::size_t smallHi = lagHi / kDecimate;
         const std::size_t smallW = small.size() - smallHi;
-        if (smallHi <= lagLo / kDecimate + 2 || smallW == 0) continue;
+        if (smallHi <= lagLo / kDecimate + 2 || smallW == 0) continue;  // degenerate geometry
         const std::vector<double> dp = cmndf(small, smallW, smallHi);
 
         double coarseTau = 0.0, dissimilarity = 1.0;
         if (!pickPeriod(dp, std::max<std::size_t>(2, lagLo / kDecimate), coarseTau,
                         dissimilarity)) {
-            continue;  // no dip below threshold: this block has no single period
+            ++evidence;  // the search ran and found no dip: real evidence against a period
+            continue;
         }
 
         // Bracket the full-rate refinement at +/- 2 decimated samples around the coarse pick:
@@ -174,7 +176,8 @@ PeriodEstimate detectPeriod(const std::vector<AudioSample>& pcm, int sampleRate,
             std::max(static_cast<double>(lagLo), centre - 2.0 * kDecimate));
         const std::size_t hi = static_cast<std::size_t>(
             std::min(static_cast<double>(lagHi), centre + 2.0 * kDecimate));
-        if (hi <= lo) continue;
+        if (hi <= lo) continue;  // degenerate refine bracket
+        ++evidence;  // the search ran and found a period: real evidence for one
         periods.push_back(refineFullRate(pcm, from, block - hi, lo, hi));
         confidences.push_back(1.0 - dissimilarity);
     }

src/core/instrument/engine/pitch_shift.h

@@ -119,14 +119,15 @@ public:
     void setFeedRate(double rate);
 
     // The period of the source being fed, in SOURCE frames, making every splice jump a whole
-    // number of it (see periodAlignedJump). <= 0 means "unknown" and restores the fixed-window
-    // geometry byte for byte — the default, so a caller that never calls this sees no change.
+    // number of it — <= 0 means "unknown" (see periodAlignedJump for the exact fallback); the
+    // default, so a caller that never calls this sees no change.
     // Detection itself is off-thread and elsewhere (period_detect, which the engine deliberately
     // does not link); this is a couple of divisions and is safe to call at note-on.
     // Cleared by configure()/reset(); NOT by prime()/warm(), which do not change the source.
     void setSourcePeriod(double periodFrames);
 
-    // The nominal jump splices currently use — window() unless a source period narrowed it.
+    // The nominal jump splices currently use — window() unless a source period retuned it to
+    // the nearest whole-period multiple, which can land either narrower or wider than window().
     std::int64_t spliceJump() const { return jump_; }
 
     // Transforms one input frame into one output frame (1 in, 1 out). RT-safe: reads/writes the
@@ -209,11 +210,18 @@ private:
                                      // tap can never drain into the writer mid-fade
     std::int64_t maxLag_ = 0;        // correlation search half-range (window_/4)
     double period_ = 0.0;            // source period in frames, 0 = unknown (fixed-window)
-    std::int64_t jump_ = 0;          // nominal splice jump; window_ unless period_ narrows it
+    std::int64_t jump_ = 0;          // nominal splice jump; window_ unless period_ retunes it
     std::int64_t jumpMax_ = 0;       // largest jump whose post-splice delay stays STRICTLY
                                      // inside [dLow_, dHigh_] at the worst search lag, so a
                                      // period-sized jump can never land back on a trigger and
-                                     // thrash (dHigh_-dLow_-maxLag_-1, i.e. 1.25*window_)
+                                     // thrash (dHigh_-dLow_-maxLag_-1, i.e. 1.25*window_). At
+                                     // jump_==jumpMax_ a DOWN-splice's correlation read comes
+                                     // within ~41 frames of the write head (measured: the
+                                     // exact ring/lag/corrFrames_ geometry at the product
+                                     // window, worst case over every lag the search reaches) —
+                                     // real margin, not zero, but tight enough that widening
+                                     // maxLag_, corrFrames_ or jumpMax_ without re-deriving
+                                     // this bound risks reading unwritten ring content.
     std::int64_t corrFrames_ = 0;    // correlation segment length (dLow_-1, capped at 512, so
                                      // the reference read forward from the tap stays behind
                                      // the writer by construction at an up-splice)

src/core/instrument/engine/time_stretch.h

@@ -15,22 +15,42 @@ namespace reasampler::instrument::engine {
 // source frames) — the RT-safety argument for feeding a variable count at all.
 //
 // This range NARROWS the splice-cadence failure onto the source fundamental; it does not
-// eliminate it. A splice recurs every `window / |rate - shift|` output frames (the tap's
-// delay drifts across one window at that per-frame rate); the shifted tone's own period is
-// `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 produces
-// measurable splice debris 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. pitch_shift_tests
-// (testStretchCadenceCornerArtifactEnergyAtRate2ShiftQuarter) asserts this corner directly at
-// P=500/600/700: energy outside the fundamental runs 7-21% there against ~0% on an aligned
-// control at the same rate/shift — zero-crossing period is NOT what it checks, since splice
-// debris fools that estimator into reading the wrong period on a render whose fundamental is
-// actually fine. (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.)
+// eliminate it. A splice recurs every `pitch_shift.h`'s spliceJump() / |rate - shift| output
+// frames (the tap's delay drifts across one nominal jump at that per-frame rate); the shifted
+// tone's own period is `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), fixed-window jump (2205): 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 produces measurable splice debris 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. pitch_shift_tests (testStretchCadenceCornerArtifactEnergyAtRate2ShiftQuarter)
+// asserts this corner directly at P=500/600/700: energy outside the fundamental runs 7-21% there
+// against ~0% on an aligned control at the same rate/shift — zero-crossing period is NOT what it
+// checks, since splice debris fools that estimator into reading the wrong period on a render
+// whose fundamental is provably correct. (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.)
+//
+// The above derives the floor with jump == window(), which is only the FIXED-WINDOW half of
+// the story. Once a source period is known, spliceJump() is periodAlignedJump's answer instead
+// (pitch_shift.h), and that answer can land NARROWER than window() — as low as ~0.63*window for
+// some periods — which SHRINKS the interval and moves the failure threshold EARLIER, not later.
+// There is no single closed-form floor for this case (the jump is itself a function of P), so
+// read it at the concrete corner instead: at P=1470 (30 Hz at 44.1k) the same rate 2.0/shift
+// 0.25 corner's jump narrows from window (2205) to 1470, and its interval from 1260 to
+// 1470/1.75 = 840. Independently, at the plain (no time-stretch) rate 1.0 case, solving this
+// same inequality for shift at P=1470 puts the failure threshold at shift = P/(jump+P): 0.4
+// (-16 st) at the fixed-window jump (2205), 0.5 (-12 st) at the pitch-synchronous jump (1470) —
+// the geometry fix that lets 30 Hz align AT ALL moves this unrelated cadence inequality's own
+// trip point from roughly -16 st to roughly -12 st for the same source. Do NOT read this as a
+// proven regression: the inequality above was calibrated for RANDOM-PHASE (unaligned) splices,
+// and a pitch-synchronous splice is waveform-aligned by construction, which the inequality does
+// not model — whether the shorter interval still produces audible debris once every splice
+// lands in phase is what pitch_shift_tests' own P=1470 cadence-collapse-band measurement
+// answers, not this derivation. Do not narrow kStretchRateMin/kStretchRateMax in response to
+// this: sub-50 Hz sine material is first-class product material, not an edge case, and a
+// narrower range does not fix a floor it does not reach.
 //
 // A SECOND, INDEPENDENT limit bound the same material, and no rate bound touched it. It is now
 // CLOSED for any source whose period is detected, but the geometry is worth keeping because it
@@ -56,7 +76,7 @@ namespace reasampler::instrument::engine {
 // pitch_shift_tests' testThirtyHertzSplicesAlignOnceTheSourcePeriodIsKnown, a different
 // quantity from the raw percentages here. What survives: a period longer than the reachable
 // jump (~1.25 windows, so below ~16 Hz at 50 ms) still cannot align, and a source with no
-// single period falls back to this fixed-window geometry by design.
+// single period falls back to it by design (periodAlignedJump, pitch_shift.h).
 inline constexpr double kStretchRateMin = 0.5;
 inline constexpr double kStretchRateMax = 2.0;
 inline constexpr int kMaxFeedPerFrame = 2;  // ceil(kStretchRateMax)