From 4455e4d1b11a0649c281e49e792ea2d5dea46a6d Mon Sep 17 00:00:00 2001 From: Jake Date: Wed, 22 Apr 2026 07:59:27 +0100 Subject: [PATCH] fix(A.3 #24): clamp LocalAgreement slices against latest.len() to prevent panic on shrinking passes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback (CRITICAL): LocalAgreement::push could panic with an index OOB when a later pass arrived shorter than committed_count. Concrete case: commit [a, b], next pass arrives [a] — lcp_len=1, new_committed=max(1, 2)=2, then latest[2..] panicked because latest.len()==1. A Whisper re-transcription of an overlapping window can legitimately collapse repeated segments, or the user can stop mid-utterance after some tokens were already committed, both of which produce this shape. The committed_count invariant still holds (non-shrinkage) — it is the slicing that was unsafe. Clamps every latest[..] slice against latest.len() before indexing. committed_count stays at new_committed even when the pass is shorter: non-shrinkage is relative to what we have already emitted, not to the current pass length. newly_committed and tentative both return empty when the shorter pass has nothing past the committed prefix. Adds two regression tests: - shorter_pass_after_commit_does_not_panic (commit 2, push 1) - empty_pass_after_commit_does_not_panic (commit 1, push empty) --- .../src/streaming/commit_policy.rs | 56 +++++++++++++++++-- 1 file changed, 51 insertions(+), 5 deletions(-) diff --git a/crates/transcription/src/streaming/commit_policy.rs b/crates/transcription/src/streaming/commit_policy.rs index 9277f58..b573dab 100644 --- a/crates/transcription/src/streaming/commit_policy.rs +++ b/crates/transcription/src/streaming/commit_policy.rs @@ -124,11 +124,21 @@ impl LocalAgreement { // The agreed prefix can only grow — never shrink below what we // already committed. ufal's invariant: once committed, stay // committed. - let new_lcp_len = lcp_len.max(self.committed_count); + let new_committed = lcp_len.max(self.committed_count); let latest = self.history.back().expect("history is non-empty here"); - let newly_committed = if new_lcp_len > self.committed_count { - latest[self.committed_count..new_lcp_len].to_vec() + // Clamp every slice against `latest.len()` — a later pass can + // legitimately arrive shorter than `committed_count` (Whisper + // re-transcribing an overlapping window with fewer segments, + // or user stopping mid-word while the committer holds a longer + // history). Without the clamp, `latest[committed_count..]` + // panics with an index OOB. + let old_committed = self.committed_count; + let latest_len = latest.len(); + let emit_start = old_committed.min(latest_len); + let emit_end = new_committed.min(latest_len); + let newly_committed = if emit_end > emit_start { + latest[emit_start..emit_end].to_vec() } else { Vec::new() }; @@ -136,9 +146,14 @@ impl LocalAgreement { if let Some(last) = newly_committed.last() { self.last_committed_end_secs = last.end_secs; } - self.committed_count = new_lcp_len; + // `committed_count` stays at `new_committed` even when the + // latest pass is shorter — the non-shrinkage invariant holds + // relative to what we've already emitted, not to the current + // pass length. + self.committed_count = new_committed; - let tentative = latest[new_lcp_len..].to_vec(); + let tentative_start = new_committed.min(latest_len); + let tentative = latest[tentative_start..].to_vec(); CommitDecision { newly_committed, @@ -303,6 +318,37 @@ mod tests { assert!(decision.newly_committed.is_empty()); } + #[test] + fn shorter_pass_after_commit_does_not_panic() { + // Regression: committed_count = 2, then a pass arrives with + // only 1 token (Whisper re-transcribing an overlapping window + // that collapses repeated segments, or user stopping mid- + // utterance). `latest[committed_count..]` would index OOB. + let mut la = LocalAgreement::new(2); + let _ = la.push(vec![tok("a", 0.0, 0.1), tok("b", 0.1, 0.2)]); + let _ = la.push(vec![tok("a", 0.0, 0.1), tok("b", 0.1, 0.2)]); + assert_eq!(la.committed_count, 2); + + let decision = la.push(vec![tok("a", 0.0, 0.1)]); + // committed_count stays at 2 (non-shrinkage invariant). + assert_eq!(la.committed_count, 2); + // No new commit, no tentative (nothing past position 2 in the + // shorter pass). + assert!(decision.newly_committed.is_empty()); + assert!(decision.tentative.is_empty()); + } + + #[test] + fn empty_pass_after_commit_does_not_panic() { + let mut la = LocalAgreement::new(2); + let _ = la.push(vec![tok("a", 0.0, 0.1)]); + let _ = la.push(vec![tok("a", 0.0, 0.1)]); + let decision = la.push(vec![]); + assert_eq!(la.committed_count, 1); + assert!(decision.newly_committed.is_empty()); + assert!(decision.tentative.is_empty()); + } + #[test] fn flush_emits_remaining_tentative() { let mut la = LocalAgreement::new(2);