diff --git a/crates/tui/src/tui/ui.rs b/crates/tui/src/tui/ui.rs index 3a4dd022..3a89a149 100644 --- a/crates/tui/src/tui/ui.rs +++ b/crates/tui/src/tui/ui.rs @@ -1430,6 +1430,11 @@ async fn run_event_loop( break; } }; + // #3033: remember whether an EARLIER event in this drain batch + // already requested a redraw. The AgentProgress throttle below + // may opt the current event out of repainting, but it must not + // cancel redraws owed to other events in the same batch. + let redraw_requested_before_event = received_engine_event; received_engine_event = true; if app.suppress_stream_events_until_turn_complete { if matches!(event, EngineEvent::TurnStarted { .. }) { @@ -2326,15 +2331,14 @@ async fn run_event_loop( // status-animation timer (80ms cadence) provides a guaranteed // floor for sidebar updates. Data is still recorded immediately; // the sidebar picks it up on the next permitted redraw. - let now = Instant::now(); - if let Some(last) = app.last_agent_progress_redraw { - if now.duration_since(last) < Duration::from_millis(100) { - received_engine_event = false; - } else { - app.last_agent_progress_redraw = Some(now); - } - } else { - app.last_agent_progress_redraw = Some(now); + if !agent_progress_redraw_permitted( + &mut app.last_agent_progress_redraw, + Instant::now(), + ) { + // Restore the pre-event accumulator value: a + // throttled progress event contributes no redraw of + // its own, but earlier events' redraws survive. + received_engine_event = redraw_requested_before_event; } } EngineEvent::AgentComplete { id, result } => { @@ -4668,6 +4672,21 @@ fn reconcile_turn_liveness(app: &mut App, now: Instant, has_running_agents: bool false } +/// #3033: gate progress-driven repaints to at most one per 100ms. +/// +/// Returns whether the current `AgentProgress` event may request a redraw, +/// updating the last-redraw timestamp when it may. Data updates are never +/// throttled — only the repaint request is. +fn agent_progress_redraw_permitted(last_redraw: &mut Option, now: Instant) -> bool { + match *last_redraw { + Some(last) if now.duration_since(last) < Duration::from_millis(100) => false, + _ => { + *last_redraw = Some(now); + true + } + } +} + fn recover_engine_event_disconnect(app: &mut App) -> bool { let had_live_work = app.is_loading || app.is_compacting diff --git a/crates/tui/src/tui/ui/tests.rs b/crates/tui/src/tui/ui/tests.rs index 832a8f9c..2172e432 100644 --- a/crates/tui/src/tui/ui/tests.rs +++ b/crates/tui/src/tui/ui/tests.rs @@ -9303,3 +9303,64 @@ mod work_sidebar_projection_tests { assert_ne!(entry.status, "running"); } } + +// ── #3033: AgentProgress redraw throttle ─────────────────────────────────── + +#[test] +fn agent_progress_redraw_throttle_permits_first_and_spaced_events() { + let mut last_redraw = None; + let t0 = Instant::now(); + + assert!( + agent_progress_redraw_permitted(&mut last_redraw, t0), + "first progress event always repaints" + ); + assert!( + !agent_progress_redraw_permitted(&mut last_redraw, t0 + Duration::from_millis(50)), + "events inside the 100ms window are throttled" + ); + assert!( + !agent_progress_redraw_permitted(&mut last_redraw, t0 + Duration::from_millis(99)), + "throttled events must not advance the window" + ); + assert!( + agent_progress_redraw_permitted(&mut last_redraw, t0 + Duration::from_millis(150)), + "events past the window repaint again" + ); +} + +#[test] +fn throttled_progress_event_does_not_cancel_other_events_redraw() { + // Repro for the #3033 audit finding: `received_engine_event` is a shared + // accumulator for the whole drain batch. A throttled AgentProgress event + // must restore the PRE-EVENT value instead of clearing the flag, so + // redraws owed to other events (AgentSpawned, AgentList, cross-agent + // AgentComplete...) survive. + let t0 = Instant::now(); + let mut last_redraw = Some(t0); + + // Batch: AgentSpawned (requests redraw), then a throttled AgentProgress. + let mut received_engine_event = true; // AgentSpawned drained + let redraw_requested_before_event = received_engine_event; + received_engine_event = true; // AgentProgress drained + if !agent_progress_redraw_permitted(&mut last_redraw, t0 + Duration::from_millis(10)) { + received_engine_event = redraw_requested_before_event; + } + assert!( + received_engine_event, + "redraw owed to AgentSpawned must survive a throttled progress event" + ); + + // Same batch shape but with NO earlier redraw-worthy event: the lone + // throttled progress event contributes nothing. + let mut received_engine_event = false; + let redraw_requested_before_event = received_engine_event; + received_engine_event = true; // AgentProgress drained + if !agent_progress_redraw_permitted(&mut last_redraw, t0 + Duration::from_millis(20)) { + received_engine_event = redraw_requested_before_event; + } + assert!( + !received_engine_event, + "a lone throttled progress event must not trigger a repaint" + ); +}