Period detection: silence is not dissent but an absent period is — the agreement denominator is the probes that carried signal

src/core/instrument/engine/period_detect.h

@@ -21,8 +21,8 @@ using audio::AudioSample;
 struct PeriodEstimate {
     double frames = 0.0;      // 0 = no single period (inharmonic, polyphonic, percussive, noise)
     // 1 - the accepted dissimilarity, [0,1]; 0 when frames == 0. Diagnostic: the accept decision
-    // is `valid()` alone and the loader takes `.frames` without reading this — its consumers are
-    // the tests and the measurement harness. It is deliberately NOT a second accept gate: every
+    // is `valid()` alone and the loader takes `.frames` without reading this — its only reader is
+    // tests/test_period_detect.cpp. It is deliberately NOT a second accept gate: every
     // accepted probe already cleared kPeriodDetectThreshold, so confidence > 0.88 holds by
     // construction and any gate below that is a no-op while any gate above it is a tuned number
     // with nothing to derive it from.
@@ -44,6 +44,14 @@ inline constexpr double kPeriodDetectMaxHz = 2000.0;
 // global minimum — the difference between "quiet but real" and "the least bad of nothing".
 inline constexpr double kPeriodDetectThreshold = 0.12;
 
+// The longest lag searched, in frames — THE one derivation of it. A probe block is twice this,
+// and `periodAnalysisSpan`'s minimum is one block; both read this rather than re-deriving the
+// same expression, so "choosing the loop never narrows the search band" is a fact and not a
+// coincidence between two literals.
+inline std::size_t longestLagFrames(int sampleRate) {
+    return static_cast<std::size_t>(static_cast<double>(sampleRate) / kPeriodDetectMinHz);
+}
+
 // How many blocks across the sample are estimated independently, and how far apart two of them
 // may land and still be called the same period. Agreement is what separates a genuinely
 // periodic source from one whose opening happens to look periodic.
@@ -53,8 +61,6 @@ inline constexpr double kPeriodDetectThreshold = 0.12;
 // whole cycle of the lowest frequency in the band, so neither can be a trivially shifted copy
 // of the other at any period searched. Requiring DISJOINT blocks instead left every source
 // under ~4x the longest lag with a single probe and so with no agreement to check at all.
-// One probe survives as an irreducible case below `block + longest lag` frames and is accepted
-// on the absolute threshold alone — see detectPeriod's contract.
 inline constexpr int kPeriodDetectProbes = 4;
 inline constexpr double kPeriodDetectAgreeTolerance = 0.02;  // 2% of the median
 
@@ -66,10 +72,21 @@ inline constexpr double kPeriodDetectAgreeTolerance = 0.02;  // 2% of the median
 // Returns an invalid estimate (frames == 0) for silence, noise, and anything whose probes
 // disagree — the caller's documented fallback is the fixed-window splice geometry.
 //
-// Two probes or more must reach a STRICT MAJORITY agreement. A lone probe — which only happens
-// on a span too short to host a second probe position — is accepted on the absolute threshold
-// alone, because there is no second estimate for a majority rule to rule on and refusing would
-// deny every short one-shot a period.
+// A STRICT MAJORITY of the probes that CARRIED SIGNAL must agree. Silence is excluded from that
+// denominator and a failure to find a period is not: a silent block is no evidence either way,
+// whereas a block that carries signal and repeats at no lag is evidence against a single period.
+// A capture with a silent head or tail therefore still detects, while a mostly-noise source with
+// one pitched burst is refused rather than accepted on that burst alone. A LONE piece of
+// evidence — the whole span too short for a second probe position, or every other probe silent —
+// is accepted on the absolute threshold alone, because there is nothing to rule against it and
+// refusing would deny every short one-shot a period.
+//
+// The answer is NOT monotone in span length, and cannot be made so: no rule that refuses a
+// two-and-two split at four probes can also accept a lone probe unconditionally, and the probe
+// count steps at 3x, 4x, 5x and 6x the longest lag before saturating. What IS pinned, by a
+// length sweep in the tests, is that a STATIONARY source detects at every length — a source
+// whose period varies by more than kPeriodDetectAgreeTolerance is the only class that moves
+// with the count, and refusing it is this contract's own answer.
 PeriodEstimate detectPeriod(const std::vector<AudioSample>& pcm, int sampleRate,
                             std::size_t from, std::size_t count);
 
@@ -79,12 +96,20 @@ PeriodEstimate detectPeriod(const std::vector<AudioSample>& pcm, int sampleRate)
 // The frames detection should analyse for a capture that carries a sustain loop, and the reason
 // the answer is not simply "all of them": under Gate the loop region is asymptotically ALL the
 // splicer plays, so a phrase whose head is pitched differently from its sustain would otherwise
-// disagree its way to none over the whole source. `[loopStart, loopEnd)` is used only when it
-// is at least `2 * (sampleRate / kPeriodDetectMinHz)` frames — the span below which detectPeriod
-// starts shortening its own search band — so choosing the narrower span never costs search-band
-// width and so can never lose a low fundamental that the whole source would have found.
+// disagree its way to none over the whole source. `[loopStart, loopEnd)` is used only when it is
+// at least one full probe block — `2 * longestLagFrames(sampleRate)`, the span below which
+// detectPeriod starts shortening its own search band — so choosing the narrower span never costs
+// search-band WIDTH. It can still change the ANSWER: the agreement rule rules on content, so a
+// source periodic over most of its length whose loop region is noisy detects whole and refuses
+// over the loop. That is the intent — the loop is what a Gate voice plays.
 // Anything else (no loop, an out-of-range span, a short one) yields the whole source.
 //
+// It takes NO play mode, deliberately, even though loop_span's resolveLoop does and refuses the
+// loop outright under Trigger. A loop edit is structurally reload-bound — it moves the PCM span
+// this cache was derived from — whereas play mode's exclusion from live delivery is a listed,
+// reversible decision (deck_groups' isLiveDeckParam). Keying a load-time cache on it would work
+// today and silently serve a stale period the day that decision is revisited.
+//
 // The read path's loop-validity authority is loop_span's resolveLoop; the bounds check here is
 // on a cache input, not a second validity rule, and it refuses rather than repairs the same way.
 struct AnalysisSpan {