fix(A.3 #24): clamp LocalAgreement slices against latest.len() to prevent panic on shrinking passes
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)
This commit is contained in:
@@ -124,11 +124,21 @@ impl LocalAgreement {
|
|||||||
// The agreed prefix can only grow — never shrink below what we
|
// The agreed prefix can only grow — never shrink below what we
|
||||||
// already committed. ufal's invariant: once committed, stay
|
// already committed. ufal's invariant: once committed, stay
|
||||||
// committed.
|
// 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 latest = self.history.back().expect("history is non-empty here");
|
||||||
let newly_committed = if new_lcp_len > self.committed_count {
|
// Clamp every slice against `latest.len()` — a later pass can
|
||||||
latest[self.committed_count..new_lcp_len].to_vec()
|
// 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 {
|
} else {
|
||||||
Vec::new()
|
Vec::new()
|
||||||
};
|
};
|
||||||
@@ -136,9 +146,14 @@ impl LocalAgreement {
|
|||||||
if let Some(last) = newly_committed.last() {
|
if let Some(last) = newly_committed.last() {
|
||||||
self.last_committed_end_secs = last.end_secs;
|
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 {
|
CommitDecision {
|
||||||
newly_committed,
|
newly_committed,
|
||||||
@@ -303,6 +318,37 @@ mod tests {
|
|||||||
assert!(decision.newly_committed.is_empty());
|
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]
|
#[test]
|
||||||
fn flush_emits_remaining_tentative() {
|
fn flush_emits_remaining_tentative() {
|
||||||
let mut la = LocalAgreement::new(2);
|
let mut la = LocalAgreement::new(2);
|
||||||
|
|||||||
Reference in New Issue
Block a user