spike(rust/snr): port validate_peak to Rust - #105
Merged
Conversation
Ports validatePeak from scripts/sync/AudioSyncer.js L218-235 to a pub fn validate_peak(correlation, lag_seconds, sample_rate) -> (f64, bool) in spike/audio-sync/src/lib.rs. Replicates the JS SNR check exactly: biased (population) variance sumSq/N - mean², index reconstruction via (x + 0.5).floor() to match JS Math.round on both round operations, and modular wrap ((lagSamples % N) + N) % N for negative lags. Returns (0.0, false) on std = 0 without panicking, matching the JS `snr = std > 0 ? ... : 0` guard. Also adds RELIABILITY_SNR_THRESHOLD = 3.0 constant (was specified in the issue as already present from #97, but was not yet in lib.rs). 5 tests added: high-SNR reliable, flat/near-silence not-reliable, zero-std guard, negative lag modular wrap, and length-1 no-panic. AC: #1 (high-SNR reliable), #2 (near-silence not-reliable), #3 (zero-std guard)
An empty correlation slice causes lag_samples % n_i to be integer division by zero — unconditional panic in both debug and release. Mirrors the existing # Preconditions note on compute_cross_correlation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
validatePeakfromscripts/sync/AudioSyncer.jsL218–235 topub fn validate_peak(correlation: &[f64], lag_seconds: f64, sample_rate: u32) -> (f64, bool)inspike/audio-sync/src/lib.rsRELIABILITY_SNR_THRESHOLD = 3.0constant (specified in the issue as coming from spike(rust/lag): port find_best_lag to Rust #97 but was absent from the merged code)# Preconditionsdoc note for the empty-slice panic (mirrorscompute_cross_correlation)Implementation notes
The port is a faithful replica of the JS:
sumSq/N - mean²) matches the JS formula exactly — not Bessel-corrected(x + 0.5).floor()twice (once forlagFrames, once forlagSamples) to replicateMath.roundincluding negative half-frame tie-breakingi64arithmetic:((lag_samples % n_i) + n_i) % n_i— identical to the JS((lagSamples % N) + N) % N(0.0, false)without panicking, matchingsnr = std > 0 ? ... : 0Test plan
validate_peak_high_snr_returns_reliable— sharp peak at idx 0 with lag 0.0 →(snr ≥ 3.0, true)validate_peak_flat_array_returns_not_reliable— all-equal array →(0.0, false)validate_peak_zero_std_no_panic— all-equal values, explicit zero-std guard → no panic,(0.0, false)validate_peak_negative_lag_modular_wrap— symmetric correlation, ±1 frame lag yields identical SNRvalidate_peak_length_one_no_panic— single-element slice → no paniccargo test— 18/18 passnpm test— 363/363 passCloses #98
🤖 Generated with Claude Code