spike(rust/lag): port find_best_lag to Rust (#97) - #104
Merged
Conversation
Replicates findBestLag from AudioSyncer.js (L188-216). The candidate-reset logic (clear candidates list on each new maximum) must match the JS exactly; without it, near-equal peaks early in the array can survive as candidates relative to a lower intermediate maximum, breaking the tie-break. PEAK_NEARNESS_THRESHOLD (0.5) and SYNC_FRAME_RATE (30) are module-level constants — the baseline.json fixture was computed with these values so they must not be changed here. The negative-lag test uses index 724 (= N-300) rather than 924 (= N-100) because 100 samples at 8000 Hz rounds to 0 frames; 300 samples rounds to -1 frame and survives quantization to produce a verifiable negative result. AC: all acceptance criteria from #97
Rust f64::round() rounds away from zero (-1.5 → -2), but JavaScript Math.round(-1.5) = -1 (rounds toward +infinity). The divergence is only observable at exact half-frame boundaries, which AudioSyncer.js handles with JS semantics. Fix: replace .round() with (x + 0.5).floor(). Adds regression test: lag_samples = -400 at 8000 Hz → -1.5 frames → must return -1/30 s, not -2/30 s. Also adds find_best_lag_positive_lag_nonzero at index 267 (→ 1/30 s) so the positive-lag AC has a test that asserts a nonzero result rather than relying solely on the index-100 case that quantizes to 0.0.
find_best_lag_negative_lag and find_best_lag_tie_break_earliest_wins were computing their expected values with .round(), which agrees with the fix for all non-half-integer inputs but diverges at half-frame boundaries. Using the same formula as the implementation makes the tests self-documenting and safe if either test input is later modified.
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
1 participant
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
PEAK_NEARNESS_THRESHOLD = 0.5andSYNC_FRAME_RATE = 30.0as module-level constants (must not change — baseline.json was computed with these values)find_best_lag(correlation: &[f64], sample_rate: u32) -> f64replicatingAudioSyncer.jsL188–216, including the candidate-reset-on-new-maximum tie-break(x + 0.5).floor()to replicate JSMath.round(Rustf64::roundrounds away from zero; JS rounds toward +infinity at half-frames)spike/audio-sync/src/lib.rsKey behaviour preserved
The candidate list is cleared each time a new maximum is found, then rebuilt from scratch. Without this reset, candidates from an earlier (lower) maximum can persist and corrupt the tie-break — this mirrors the JS exactly.
Tests (8 inline
#[cfg(test)], 13 total in suite)find_best_lag_positive_lagfind_best_lag_positive_lag_nonzerofind_best_lag_negative_lagfind_best_lag_negative_half_frame_regressionfind_best_lag_tie_break_earliest_winsfind_best_lag_all_zeros_no_panicfind_best_lag_peak_at_n_over_2_is_positivefind_best_lag_one_frame_at_44100cargo test --manifest-path spike/audio-sync/Cargo.toml— 13/13 pass.Note on negative-lag test index
The AC example used index 924 (= N-100 samples), but 100 samples at 8000 Hz quantizes to 0 frames. Index 724 (= N-300 samples → -1 frame) is used instead to produce a verifiable negative result after quantization.
Closes #97
🤖 Generated with Claude Code