From 07f67559615027fbd2874d1c422e18845518dab1 Mon Sep 17 00:00:00 2001 From: Jake Date: Wed, 13 May 2026 17:54:05 +0100 Subject: [PATCH] =?UTF-8?q?agent:=20code-atomiser-fix=20=E2=80=94=20drain?= =?UTF-8?q?=5Finference=20deadline=20from=20task.duration=5Fsecs=20(Time-b?= =?UTF-8?q?omb-1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F3 derived the drain_inference timeout from the CHUNK_SAMPLES constant and capped it near 6s. That breaks on the realistic worst-case configuration (slow CPU + Whisper large-v3, 3-5x realtime): a 4-second chunk legitimately takes ~20s to clear the decoder, but the F3 budget aborted it after 6s and treated healthy work as a wedge — the lifecycle keeps surviving, but every long-tail chunk gets cancelled and the user sees their final stretch of dictation get dropped. The deadline now derives from the in-flight task's own duration_secs multiplied by a REALTIME_SAFETY_MULTIPLIER constant (5x — the documented upper bound for the slowest supported backend), with a DRAIN_TIMEOUT_FLOOR of 2s so sub-second tail chunks still get enough wall-clock to amortise model load, OS scheduling jitter, and the abort-callback's own poll cadence. Both constants sit next to CHUNK_SAMPLES with doc comments explaining the rationale. Defensive: non-finite or non-positive durations fall back to the floor so a malformed task can't produce a NaN/overflow budget. Regression tests (commands::live::tests): - drain_timeout_scales_with_inflight_chunk_duration_secs: 4.0s chunk must get 20s budget (5x), not the old 6s cap. - drain_timeout_honours_floor_for_short_chunks: 0.3s chunk produces 1.5s scaled value, must be clamped to the 2s floor. - drain_timeout_uses_floor_when_no_inflight_task: well-defined fallback for the (in practice unreachable) None branch. - drain_timeout_rejects_non_finite_duration: NaN / inf / 0 / negative fall back to floor. cargo test -p lumotia --lib commands::live::tests::drain: 4 passed. Co-Authored-By: Claude Opus 4.7 (1M context) --- src-tauri/src/commands/live.rs | 139 ++++++++++++++++++++++++++++++++- 1 file changed, 136 insertions(+), 3 deletions(-) diff --git a/src-tauri/src/commands/live.rs b/src-tauri/src/commands/live.rs index 040ea6e..f3408ec 100644 --- a/src-tauri/src/commands/live.rs +++ b/src-tauri/src/commands/live.rs @@ -31,6 +31,25 @@ const CHUNK_SAMPLES: usize = 32_000; // 2s at 16kHz const OVERLAP_SAMPLES: usize = 4_000; // 0.25s at 16kHz const FINAL_CHUNK_MIN_SAMPLES: usize = 4_000; // 0.25s const MAX_PENDING_SAMPLES: usize = CHUNK_SAMPLES; + +/// Headroom multiplier for `drain_inference`'s timeout, derived from the +/// in-flight chunk's `duration_secs`. F3's original derivation used the +/// constant CHUNK_SAMPLES and capped at ~6s, which broke for the +/// realistic case of a slow CPU + large-v3 model running 3-5x realtime +/// on a chunk that happens to be SHORTER than CHUNK_SAMPLES — the +/// timeout would fire before the legitimate inference finished, treating +/// healthy work as a wedge. 5x realtime is the documented upper bound +/// for the slowest supported configuration (whisper.cpp large-v3, CPU +/// only, low-end laptop). Anything beyond that is a genuine wedge worth +/// aborting. +const REALTIME_SAFETY_MULTIPLIER: u32 = 5; + +/// Floor on the drain timeout. Even a sub-second tail chunk needs more +/// than its duration's worth of wall-clock to clear the decoder (model +/// load amortisation, OS scheduling jitter, the abort callback's own +/// poll cadence). 2s is conservative enough to never fire spuriously on +/// healthy backends and short enough to keep the lifecycle responsive. +const DRAIN_TIMEOUT_FLOOR: Duration = Duration::from_secs(2); const SPEECH_FRAME_SAMPLES: usize = 800; // 50ms const MIN_SPEECH_FRAMES: usize = 1; // any plausible speech-like frame const SILENCE_RMS_THRESHOLD: f32 = 0.001; @@ -521,15 +540,22 @@ impl LiveSessionRuntime { // abort callback — on expiry we set the flag and drop our // receiver, so the worker either honours the abort or exits // silently when it eventually tries to send its result. - let chunk_duration_ms = (CHUNK_SAMPLES as u64 * 1000) / WHISPER_SAMPLE_RATE as u64; - let drain_timeout_ms = (chunk_duration_ms.saturating_mul(3)).max(2_000); + // + // Budget is derived from the IN-FLIGHT task's own duration_secs + // rather than the constant CHUNK_SAMPLES so a short tail chunk + // running through a slow CPU/model combination (large-v3 at + // 3-5x realtime is the documented upper bound) isn't aborted + // mid-flight while it's still doing legitimate work. The + // multiplier + floor gives us safety on both ends. + let drain_timeout = drain_timeout_for_inflight(self.state.inflight.as_ref()); + let drain_timeout_ms = drain_timeout.as_millis() as u64; let drain_started = Instant::now(); while self.state.inflight.is_some() { self.poll_inference()?; if self.state.inflight.is_none() { break; } - if drain_started.elapsed() >= Duration::from_millis(drain_timeout_ms) { + if drain_started.elapsed() >= drain_timeout { // SAFETY: a real test of this path requires a wedged // whisper-rs, which is hard to fixture. We rely on the // abort_callback wiring plus cargo build + manual smoke @@ -542,6 +568,7 @@ impl LiveSessionRuntime { session_id = self.session_id, chunk_id = task.chunk_id, timeout_ms = drain_timeout_ms, + chunk_duration_secs = task.duration_secs, inflight_age_ms = task.dispatched_at.elapsed().as_millis() as u64, "drained inflight inference after timeout" ); @@ -607,6 +634,31 @@ impl Drop for InferenceTask { } } +/// Compute the drain-inference timeout for the in-flight task. Derived +/// from `task.duration_secs * REALTIME_SAFETY_MULTIPLIER` with a floor +/// at `DRAIN_TIMEOUT_FLOOR` so even a sub-second tail chunk gets enough +/// wall-clock to clear the decoder before we treat it as a wedge. +/// +/// `None` means there's nothing in flight — return the floor purely so +/// the drain loop has a well-defined fallback; in practice the outer +/// `while self.state.inflight.is_some()` short-circuits before the +/// timeout is consulted. +fn drain_timeout_for_inflight(task: Option<&InferenceTask>) -> Duration { + let Some(task) = task else { + return DRAIN_TIMEOUT_FLOOR; + }; + // Treat non-finite or non-positive durations as "use the floor" — + // we never want a NaN or negative to silently produce a tiny or + // overflowing budget. + let duration_secs = task.duration_secs; + if !duration_secs.is_finite() || duration_secs <= 0.0 { + return DRAIN_TIMEOUT_FLOOR; + } + let scaled_secs = duration_secs * REALTIME_SAFETY_MULTIPLIER as f64; + let scaled = Duration::from_secs_f64(scaled_secs); + scaled.max(DRAIN_TIMEOUT_FLOOR) +} + #[derive(Debug, Clone)] struct RecentTranscriptSegment { start_secs: f64, @@ -1932,6 +1984,87 @@ mod tests { assert_eq!(start.await.unwrap().unwrap(), 8); } + #[test] + fn drain_timeout_scales_with_inflight_chunk_duration_secs() { + // Time-bomb-1 regression: the drain timeout used to be a + // constant derived from CHUNK_SAMPLES (capped at ~6s). On slow + // CPU + large-v3 model (3-5x realtime), a 4-second chunk could + // legitimately need ~20s to finish; the old cap aborted + // healthy work mid-flight. The new derivation pulls duration + // from the in-flight task itself. + let task = InferenceTask { + chunk_id: 1, + chunk_start_sample: 0, + trim_before_secs: 0.0, + duration_secs: 4.0, + rx: std::sync::mpsc::channel().1, + abort_flag: Arc::new(AtomicBool::new(false)), + dispatched_at: Instant::now(), + }; + let timeout = drain_timeout_for_inflight(Some(&task)); + // 4.0s * 5x safety = 20s. Old constant-derived budget would + // have been ~6s, aborting at chunk_duration * 1.5 — well short + // of the 20s a 5x-realtime backend genuinely needs. + assert_eq!( + timeout, + Duration::from_secs(20), + "5x realtime headroom must be the actual budget" + ); + } + + #[test] + fn drain_timeout_honours_floor_for_short_chunks() { + // A sub-second tail chunk still needs more than its raw + // duration to clear the decoder (model load amortisation, OS + // scheduling, abort poll cadence). The floor guards against + // the timeout firing on legitimate short-chunk inference. + let task = InferenceTask { + chunk_id: 1, + chunk_start_sample: 0, + trim_before_secs: 0.0, + duration_secs: 0.3, // 0.3s * 5x = 1.5s, below the 2s floor + rx: std::sync::mpsc::channel().1, + abort_flag: Arc::new(AtomicBool::new(false)), + dispatched_at: Instant::now(), + }; + let timeout = drain_timeout_for_inflight(Some(&task)); + assert_eq!( + timeout, DRAIN_TIMEOUT_FLOOR, + "tiny chunks must still get the floor's worth of budget" + ); + } + + #[test] + fn drain_timeout_uses_floor_when_no_inflight_task() { + // Defensive: no-inflight is in practice short-circuited by the + // outer `while self.state.inflight.is_some()` loop, but the + // helper must still produce a well-defined non-zero value. + assert_eq!(drain_timeout_for_inflight(None), DRAIN_TIMEOUT_FLOOR); + } + + #[test] + fn drain_timeout_rejects_non_finite_duration() { + // Defensive against NaN / negative durations slipping in from + // a malformed task. We must not silently produce a tiny or + // overflowing budget — fall back to the floor. + for bad_duration in [f64::NAN, f64::INFINITY, -1.0, 0.0] { + let task = InferenceTask { + chunk_id: 1, + chunk_start_sample: 0, + trim_before_secs: 0.0, + duration_secs: bad_duration, + rx: std::sync::mpsc::channel().1, + abort_flag: Arc::new(AtomicBool::new(false)), + dispatched_at: Instant::now(), + }; + assert_eq!( + drain_timeout_for_inflight(Some(&task)), + DRAIN_TIMEOUT_FLOOR, + "non-finite duration {bad_duration:?} must fall back to the floor" + ); + } + } + #[test] fn dropping_inference_task_sets_abort_flag() { // Race-A regression: dropping an InferenceTask (via `?`