From c7671811ad923572ebc209e1b7967687efba80ee Mon Sep 17 00:00:00 2001 From: Rob Morgan Date: Thu, 6 Aug 2026 12:18:37 +0800 Subject: [PATCH 1/2] fix: per-sample fade ramp, modulation-hold contract, analysis version bump Three items from the 2026-08-05 quality review (D6, D7) plus the artifact-invalidation footgun from the beat-grid diagnosis: - Keylock extreme-rate correction weight now chases its per-block target per sample at the toggle-fade slew bound. The fade crossfades two differently-pitched copies of the high band, so the old per-block step spliced between unrelated waveforms under fast tempo gestures. New fade-band rate-step click test: per-block steps measured ~3.5x the tone-slew bound, the chase ~0.9x. - modulation_hold's doc (stage.rs) now matches the code: the wide keylock stage is its sole consumer. SOLA deliberately does not read it - its discretionary work is gated by stricter rest-dwell machinery, and suppressing opportunistic splices during rides would push drift into forced onset-unprotected splices. Wiring it into SOLA stays a ROADMAP Stage 15 experiment, gated on seam/click measurements. - PREANALYSIS_VERSION and MIN_COMPATIBLE_VERSION -> 8: the rigid-grid fit (v0.10.0) materially changed beat positions without a bump, so v7 artifacts are ambiguous between old wandering and new rigid grids; forcing regeneration disambiguates. The invalidation policy is now documented on the constants and in RELEASE_CHECKLIST.md (rebuilt - it still referenced the pre-cutover engine surface). Co-Authored-By: Claude Fable 5 --- RELEASE_CHECKLIST.md | 33 +++++++++----- ROADMAP.md | 22 ++++++--- src/core/preanalysis.rs | 35 +++++++++++---- src/engine/stage.rs | 14 ++++-- src/engine/stages/keylock.rs | 87 ++++++++++++++++++++++++++++++++---- 5 files changed, 153 insertions(+), 38 deletions(-) diff --git a/RELEASE_CHECKLIST.md b/RELEASE_CHECKLIST.md index 96a5aa7..1252735 100644 --- a/RELEASE_CHECKLIST.md +++ b/RELEASE_CHECKLIST.md @@ -1,15 +1,24 @@ # Release Checklist -## Streaming Runtime +Complements the `/release` flow (CHANGELOG fold, version bump, CI checks, +signed tag, publish). Human judgment items that the automation cannot +decide: -Use this checklist when cutting a release that verifies the -streaming backend is working correctly. - -- [ ] Verify CI quality gates are green for the shipping streaming route: - - `cargo test --test realtime_allocations -- --nocapture` - - `cargo test --features qa-harnesses --release --test streaming_quality -- --nocapture` -- [ ] Re-run API docs/examples to ensure they reflect the plain `StreamProcessor` behavior. -- [ ] Record benchmark deltas versus previous release baseline: - - callback p99/p999 latency - - streaming artifact percentiles on modulation-heavy material - - end-of-stream length drift +- [ ] **Analysis version policy** (`src/core/preanalysis.rs`): if any + analysis output changed materially this release (beat grids, onsets, + key, tempo candidates — not just schema), bump `PREANALYSIS_VERSION`; + if cached artifacts from the previous version would now be *worse* than + re-analysis, raise `MIN_COMPATIBLE_VERSION` too so sidecars regenerate. + (Learned from v0.10.0 shipping the rigid-grid fit without a bump — + LEARNINGS.md.) +- [ ] CI green on the shipping surface: `cargo test --all-targets`, + clippy `-D warnings`, `cargo fmt --check`, docs with + `RUSTDOCFLAGS="-D warnings"`, and the desktop crate checks. +- [ ] Quality gates: the CI quality-gates job (`engine_ab_matrix`, + `engine_wcet`) and public-corpus job (`bpm_accuracy`, + `rubberband_reference_gate`) are green on the release commit. +- [ ] If DSP changed audibly this release: owner listen recorded in the + relevant ROADMAP stage note before tagging, with the implementation + state (commit) noted alongside the verdict. +- [ ] README latency table and RT-contract claims still match the code + (profile latencies, tempo range, MSRV/toolchain pins). diff --git a/ROADMAP.md b/ROADMAP.md index 9e6f0a4..12c60d9 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -157,11 +157,10 @@ intervals draws instead). whether the competitor phase sits specifically at a half/quarter-period offset before counting it against decisiveness. Do **not** lower the 0.3 gate to chase these tracks (recorded lesson). -- **Artifact invalidation policy**: bump `PREANALYSIS_VERSION` / - `MIN_COMPATIBLE_VERSION` whenever analysis output changes materially - (the rigid-grid fix shipped without a bump, so pre-v0.10 sidecars keep - serving worse grids until deleted). Write the policy into - `src/core/preanalysis.rs` docs and the release checklist. +- ~~Artifact invalidation policy~~ **done 2026-08-06**: versions bumped + to 8/8 so ambiguous v4–v7 sidecars regenerate; policy written into the + `src/core/preanalysis.rs` constant docs and RELEASE_CHECKLIST.md (which + was also rebuilt — it still referenced the pre-cutover engine). - Desktop owner check: grids visually aligned on real non-EDM tracks. ### Exit Criteria @@ -325,6 +324,19 @@ broken `LR8Crossover` kept only for baselines that no longer exist. Automation: auto +> **Status (2026-08-06): fade smoothing and the modulation-hold +> contradiction resolved** (branch `fix/fade-ramp-hold-doc-artifact-version`): +> the extreme-rate correction weight now chases its target per sample at +> the toggle-fade slew bound, gated by a fade-band rate-step click test +> (per-block steps measured ~3.5× the tone-slew bound; the chase ~0.9×). +> `modulation_hold` resolved as a doc fix: `stage.rs` now records that the +> wide stage is its sole consumer and WHY SOLA deliberately does not read +> it (suppressing opportunistic splices during rides would push drift into +> forced onset-unprotected splices) — wiring it remains the evidence-gated +> experiment below. Remaining: seam recovery under continuous rides, the +> optional correlation-reference/strength items, and promoting the two QA +> harnesses into CI. + ### Why Three review findings degrade exactly the gesture DJs perform most — diff --git a/src/core/preanalysis.rs b/src/core/preanalysis.rs index 1885f4c..77bc013 100644 --- a/src/core/preanalysis.rs +++ b/src/core/preanalysis.rs @@ -19,12 +19,27 @@ use std::path::Path; /// /// v7: adds [`tempo_candidates`](PreAnalysisArtifact::tempo_candidates). /// Purely additive — older sidecars stay compatible with an empty list. -pub const PREANALYSIS_VERSION: u32 = 7; - -/// Oldest schema version whose positions match the current analysis. -/// Artifacts below this carry the pre-v4 window-start bias and fail -/// [`PreAnalysisArtifact::matches_source`], so cached sidecars regenerate. -const MIN_COMPATIBLE_VERSION: u32 = 4; +/// +/// v8: no schema change — bumped (with `MIN_COMPATIBLE_VERSION`) because +/// the rigid-grid beat fit (v0.10.0) materially changed beat positions on +/// quantized material without a version bump, so v7 artifacts are +/// ambiguous: they may carry either the old wandering grids or the new +/// rigid ones. Forcing regeneration disambiguates. +/// +/// POLICY (learned from the v7 ambiguity, see LEARNINGS.md): bump +/// `PREANALYSIS_VERSION` on any release whose analysis output changes +/// materially, and raise `MIN_COMPATIBLE_VERSION` with it whenever cached +/// results from the previous version would be *worse* than re-analysis — +/// schema compatibility alone is not the bar. Checked at release time via +/// RELEASE_CHECKLIST.md. +pub const PREANALYSIS_VERSION: u32 = 8; + +/// Oldest schema version whose *analysis results* match the current +/// detector. Artifacts below this fail +/// [`PreAnalysisArtifact::matches_source`], so cached sidecars regenerate: +/// pre-v4 carried the window-start bias; v4–v7 predate (or are ambiguous +/// about) the rigid-grid beat fit. +const MIN_COMPATIBLE_VERSION: u32 = 8; fn default_artifact_version() -> u32 { 1 @@ -580,7 +595,10 @@ mod tests { #[test] fn test_v5_json_without_loudness_parses_as_none() { - // A v5 sidecar (has key, predates loudness) must stay readable. + // A v5 sidecar (has key, predates loudness) must stay READABLE — + // but as of v8 it is no longer cache-valid: v4–v7 artifacts are + // ambiguous about the rigid-grid beat fit, so `matches_source` + // rejects them and they regenerate. let mut artifact = test_artifact(); artifact.version = 5; artifact.loudness = None; @@ -590,7 +608,8 @@ mod tests { assert_eq!(parsed.loudness, None); assert!(parsed.tempo_candidates.is_empty()); assert_eq!(parsed.key, test_artifact().key); - assert!(parsed.version >= MIN_COMPATIBLE_VERSION); + assert!(parsed.version < MIN_COMPATIBLE_VERSION); + assert!(!parsed.matches_source(&[0.0; 4], 44100)); } #[test] diff --git a/src/engine/stage.rs b/src/engine/stage.rs index 993a324..5dd9b80 100644 --- a/src/engine/stage.rs +++ b/src/engine/stage.rs @@ -127,10 +127,16 @@ pub struct StageCtx<'a> { /// by up to ~2k), in stage-timeline coordinates. Empty when no /// artifact is attached — stages fall back to online heuristics. pub onsets: &'a [OnsetEvent], - /// True while a fast control gesture is in flight: stages suppress - /// disruptive maintenance (low-band phase resets, discretionary - /// splices) until the ride settles — the graph-level re-expression of - /// the old engine's modulation-hold latches. + /// True while a fast control gesture is in flight — the graph-level + /// re-expression of the old engine's modulation-hold latches. Sole + /// consumer today: the wide keylock stage, which holds LOW-BAND phase + /// resets until the ride settles (delay-matched through its latency). + /// The narrow keylock chain deliberately does NOT read it: SOLA's + /// discretionary work is already gated by its own stricter rest-dwell + /// machinery, and suppressing its opportunistic splices during rides + /// would push drift into forced (onset-unprotected) splices — wiring + /// it there is a ROADMAP Stage 15 experiment, gated on seam/click + /// measurements, not a given. pub modulation_hold: bool, /// Whether a pre-analysis artifact is attached at all. Distinguishes /// "no events nearby" from "no artifact" so stages know when to run diff --git a/src/engine/stages/keylock.rs b/src/engine/stages/keylock.rs index f915a6c..c265265 100644 --- a/src/engine/stages/keylock.rs +++ b/src/engine/stages/keylock.rs @@ -72,6 +72,12 @@ pub(crate) struct KeylockStage { /// [`KEYLOCK_TOGGLE_FADE_FRAMES`]. NaN = snap to the target on the next /// block (stream start / post-reset: no fade-in from stale state). enable: f32, + /// Smoothed extreme-rate correction weight chasing the per-block + /// deviation target at the same slew bound as `enable`. The fade + /// crossfades two DIFFERENTLY-PITCHED copies of the high band, so a + /// per-block step here is a click source under fast tempo gestures + /// (Stage 13 review, finding D6). NaN = snap, as `enable`. + correction: f32, } impl KeylockStage { @@ -87,6 +93,7 @@ impl KeylockStage { high: vec![[0.0; BLOCK_FRAMES]; channels], high_raw: vec![[0.0; BLOCK_FRAMES]; channels], enable: f32::NAN, + correction: f32::NAN, } } } @@ -122,31 +129,40 @@ impl Stage for KeylockStage { // Extreme-rate correction weight: 1 inside the DJ range, fading to // plain varispeed (pitch follows tempo) beyond it. let deviation = (ctx.embedded_rate - 1.0).abs(); - let correction = ((CORRECTION_FADE_END_DEV - deviation) + let correction_target = ((CORRECTION_FADE_END_DEV - deviation) / (CORRECTION_FADE_END_DEV - CORRECTION_FADE_START_DEV)) .clamp(0.0, 1.0) as f32; - // Live keylock toggle: chase the control target per sample so a - // mid-play switch is a click-free crossfade. Composes with the - // extreme-rate fade multiplicatively; the per-frame weights are - // shared across channels so the image stays stable through a fade. + // Live keylock toggle and extreme-rate fade: chase both targets per + // sample so a mid-play switch OR a fast tempo gesture through the + // fade band is a click-free crossfade (the fade blends two + // differently-pitched copies, so a per-block weight step is a + // discontinuity between unrelated waveforms). The per-frame weights + // are shared across channels so the image stays stable through a + // fade. let target = (ctx.keylock.clamp(0.0, 1.0)) as f32; if self.enable.is_nan() { self.enable = target; } + if self.correction.is_nan() { + self.correction = correction_target; + } let step = 1.0 / KEYLOCK_TOGGLE_FADE_FRAMES as f32; - let mut enable_w = [0.0f32; BLOCK_FRAMES]; + let mut weight_w = [0.0f32; BLOCK_FRAMES]; let mut enable = self.enable; - for w in &mut enable_w { + let mut correction = self.correction; + for w in &mut weight_w { enable += (target - enable).clamp(-step, step); - *w = enable; + correction += (correction_target - correction).clamp(-step, step); + *w = correction * enable; } self.enable = enable; + self.correction = correction; for ch in 0..block.channels() { let out = block.channel_mut(ch); for (i, sample) in out.iter_mut().enumerate() { - let weight = correction * enable_w[i]; + let weight = weight_w[i]; let high = weight * self.high[ch][i] + (1.0 - weight) * self.high_raw[ch][i]; *sample = self.low[ch][i] + high; } @@ -166,6 +182,7 @@ impl Stage for KeylockStage { self.raw_high_delay.reset(); self.sola.reset(); self.enable = f32::NAN; + self.correction = f32::NAN; } } @@ -209,6 +226,58 @@ mod tests { .collect() } + #[test] + fn fade_band_rate_steps_are_click_free() { + // The extreme-rate fade crossfades the corrected and raw high + // bands — two DIFFERENTLY-PITCHED copies. Rate gestures that jump + // across the fade band (dev 0.205→0.35) used to step the fade + // weight once per 32-frame block, splicing between unrelated + // waveforms mid-tone (Stage 13 review, finding D6). Toggle the + // rate between the band's edges repeatedly (so some step lands at + // adverse phase) and bound the output's sample-to-sample delta by + // the tone's own slew: measured ~0.9x the bound with the + // per-sample chase, up to ~3.5x with per-block steps. + let freq = 2_000.0; + let amp = 0.5f32; + let mut stage = KeylockStage::new(SR, 1); + let input = sine(freq, 4 * SR as usize, amp); + let mut block = BlockBuf::new(1); + let mut out = Vec::with_capacity(input.len()); + for (bi, chunk) in input.chunks_exact(BLOCK_FRAMES).enumerate() { + let t = (bi * BLOCK_FRAMES) as f64 / SR as f64; + // Warm up inside the corrected range, then square-wave across + // the fade band every ~15 ms. + let rate = if t < 1.0 || (t / 0.015) as usize % 2 == 0 { + 1.22 + } else { + 1.34 + }; + let ctx = StageCtx { + embedded_rate: rate, + embedded_rate_slope: 0.0, + onsets: &[], + modulation_hold: false, + has_artifact: false, + keylock: 1.0, + }; + block.channel_mut(0).copy_from_slice(chunk); + stage.process(&mut block, &ctx); + out.extend_from_slice(block.channel(0)); + } + // Skip warm-up + latency; scan the toggling region. + let start = SR as usize + KEYLOCK_LATENCY_FRAMES; + let max_delta = out[start..] + .windows(2) + .map(|w| (w[1] - w[0]).abs()) + .fold(0.0f32, f32::max); + let tone_slew = amp * 2.0 * std::f32::consts::PI * freq as f32 / SR as f32; + assert!( + max_delta < 1.5 * tone_slew, + "fade-band rate steps click: max sample delta {max_delta:.4} vs tone slew bound \ + {tone_slew:.4}" + ); + } + #[test] fn rest_recenter_never_degenerates_into_noop_splices() { // Regression (autoresearch #62): on highly periodic content the From 0692856999e90f35a6d1cacbbbf57c83ed2498ad Mon Sep 17 00:00:00 2001 From: Rob Morgan Date: Thu, 6 Aug 2026 13:37:41 +0800 Subject: [PATCH 2/2] docs: move the analysis version policy to CLAUDE.md The bump-when rules live in CLAUDE.md ("Analysis Version Policy") so they load into every session; the preanalysis.rs constant docs and the release checklist now point there instead of restating them. Co-Authored-By: Claude Fable 5 --- CLAUDE.md | 16 ++++++++++++++++ RELEASE_CHECKLIST.md | 12 +++++------- src/core/preanalysis.rs | 7 ++----- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 651eda0..52e9234 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,5 +1,21 @@ # CLAUDE.md +## Analysis Version Policy + +`src/core/preanalysis.rs` carries two constants that control cached +analysis artifacts (`.tsa` sidecars and Halo's stored blobs): + +- Bump `PREANALYSIS_VERSION` on any release whose **analysis output** + changes materially (beat grids, onsets, key, tempo candidates) — not + just on schema changes. +- Raise `MIN_COMPATIBLE_VERSION` with it whenever cached results from the + previous version would be *worse* than re-analysis, so stale sidecars + regenerate. Schema compatibility alone is not the bar. + +Learned from v0.10.0 shipping the rigid-grid beat fit without a bump, +which left v7 artifacts ambiguous between old wandering and new rigid +grids (LEARNINGS.md). Checked at release time via RELEASE_CHECKLIST.md. + ## CI Checks The following checks run on every push to `main` and on every pull request. All must pass. diff --git a/RELEASE_CHECKLIST.md b/RELEASE_CHECKLIST.md index 1252735..85b4cf4 100644 --- a/RELEASE_CHECKLIST.md +++ b/RELEASE_CHECKLIST.md @@ -4,13 +4,11 @@ Complements the `/release` flow (CHANGELOG fold, version bump, CI checks, signed tag, publish). Human judgment items that the automation cannot decide: -- [ ] **Analysis version policy** (`src/core/preanalysis.rs`): if any - analysis output changed materially this release (beat grids, onsets, - key, tempo candidates — not just schema), bump `PREANALYSIS_VERSION`; - if cached artifacts from the previous version would now be *worse* than - re-analysis, raise `MIN_COMPATIBLE_VERSION` too so sidecars regenerate. - (Learned from v0.10.0 shipping the rigid-grid fit without a bump — - LEARNINGS.md.) +- [ ] **Analysis version policy** (CLAUDE.md "Analysis Version Policy"): + if any analysis output changed materially this release, bump + `PREANALYSIS_VERSION` in `src/core/preanalysis.rs`; if cached artifacts + from the previous version would now be *worse* than re-analysis, raise + `MIN_COMPATIBLE_VERSION` too so sidecars regenerate. - [ ] CI green on the shipping surface: `cargo test --all-targets`, clippy `-D warnings`, `cargo fmt --check`, docs with `RUSTDOCFLAGS="-D warnings"`, and the desktop crate checks. diff --git a/src/core/preanalysis.rs b/src/core/preanalysis.rs index 77bc013..0e3cf17 100644 --- a/src/core/preanalysis.rs +++ b/src/core/preanalysis.rs @@ -26,11 +26,8 @@ use std::path::Path; /// ambiguous: they may carry either the old wandering grids or the new /// rigid ones. Forcing regeneration disambiguates. /// -/// POLICY (learned from the v7 ambiguity, see LEARNINGS.md): bump -/// `PREANALYSIS_VERSION` on any release whose analysis output changes -/// materially, and raise `MIN_COMPATIBLE_VERSION` with it whenever cached -/// results from the previous version would be *worse* than re-analysis — -/// schema compatibility alone is not the bar. Checked at release time via +/// The bump-when policy for these two constants lives in CLAUDE.md +/// ("Analysis Version Policy") and is checked at release time via /// RELEASE_CHECKLIST.md. pub const PREANALYSIS_VERSION: u32 = 8;