diff --git a/crates/kebab-tui/src/app.rs b/crates/kebab-tui/src/app.rs index 809697e..c9164b0 100644 --- a/crates/kebab-tui/src/app.rs +++ b/crates/kebab-tui/src/app.rs @@ -83,12 +83,17 @@ pub struct SearchState { /// (the user kept typing and a newer query is already in /// flight). Wraps at u64::MAX which is unreachable in practice. pub generation: u64, - /// p9-fb-08: in-flight worker thread. Held so the worker can be - /// joined / observed; results stream back via `worker_rx`. - pub worker_thread: Option>, - /// p9-fb-08: receiver for `(generation, Result>)` - /// messages from the in-flight worker. Drained every tick by + /// p9-fb-08: receiver for the in-flight worker's + /// `SearchWorkerMessage::Done`. Drained every tick by /// `crate::search::poll_worker`. `None` between runs. + /// + /// Workers are fire-and-forget (no join) — search is a pure + /// read with no cleanup obligation, and dropping the receiver + /// makes the worker's `tx.send` no-op (worker exits after). + /// We don't store the `JoinHandle` because nothing observes it + /// (cf. `AskState.thread` which is `take().join()`'d on + /// `Ctrl-L`); the previous draft kept one for "symmetry" but + /// it was dead code. pub worker_rx: Option>, } @@ -115,7 +120,6 @@ impl Default for SearchState { searching: false, preview: None, generation: 0, - worker_thread: None, worker_rx: None, } } diff --git a/crates/kebab-tui/src/lib.rs b/crates/kebab-tui/src/lib.rs index 22bb495..a99652d 100644 --- a/crates/kebab-tui/src/lib.rs +++ b/crates/kebab-tui/src/lib.rs @@ -43,8 +43,10 @@ pub use library::{handle_key_library, render_library}; // only safe constructor path for raw mode + alt-screen). External // callers stage editor spawns via `App.pending_editor` instead. pub use search::{build_jump_command, handle_key_search, render_search}; -// p9-fb-08: expose `poll_worker` so integration tests can drive the -// stale-result drop / fresh-result apply paths without spawning the -// real thread (they inject a `SearchWorkerMessage` directly via a -// channel they construct in the test). +// p9-fb-08: expose `poll_worker` + `debounce_due` so integration +// tests can drive the stale-result drop / fresh-result apply paths +// without spawning the real thread (they inject a +// `SearchWorkerMessage` directly via a channel they construct in +// the test) and can pin the in-flight-skip invariant of debounce. pub use search::poll_worker as poll_search_worker; +pub use search::debounce_due as search_debounce_due; diff --git a/crates/kebab-tui/src/search.rs b/crates/kebab-tui/src/search.rs index 57a8510..dbf31e3 100644 --- a/crates/kebab-tui/src/search.rs +++ b/crates/kebab-tui/src/search.rs @@ -431,7 +431,7 @@ fn parse_editor_env(env: &str) -> (String, Vec) { /// - if a worker is already in flight for the *same* `(input, mode)` /// the spawn is redundant — wait for the result. /// - dedupe against `last_query` (was already there pre-fb-08, kept). -pub(crate) fn debounce_due(s: &SearchState) -> bool { +pub fn debounce_due(s: &SearchState) -> bool { let Some(at) = s.input_dirty_at else { return false }; let elapsed = (time::OffsetDateTime::now_utc() - at) .try_into() @@ -488,7 +488,12 @@ pub(crate) fn fire_search(state: &mut App) -> anyhow::Result<()> { }; let (tx, rx) = std::sync::mpsc::channel(); - let handle = std::thread::Builder::new() + // Fire-and-forget — `JoinHandle` is dropped immediately so the + // OS detaches the thread. Search is a pure read with no + // cleanup obligation; if the receiver is replaced (next + // keystroke spawns a fresh worker), the old worker's + // `tx.send` no-ops and it exits silently. + std::thread::Builder::new() .name(format!("kebab-tui-search-gen{generation}")) .spawn(move || { let query = SearchQuery { @@ -498,9 +503,6 @@ pub(crate) fn fire_search(state: &mut App) -> anyhow::Result<()> { filters: kebab_core::SearchFilters::default(), }; let result = kebab_app::search_with_config(cfg, query); - // Send result back. If the receiver dropped (UI closed, - // or — in tests — state torn down), the result is - // discarded; that's fine since search is a pure read. let _ = tx.send(crate::app::SearchWorkerMessage::Done { generation, result, @@ -509,7 +511,6 @@ pub(crate) fn fire_search(state: &mut App) -> anyhow::Result<()> { .map_err(|e| anyhow::anyhow!("spawn search worker: {e}"))?; let s = state.search.as_mut().expect("Search slot must exist"); - s.worker_thread = Some(handle); s.worker_rx = Some(rx); Ok(()) } @@ -526,15 +527,13 @@ pub fn poll_worker(state: &mut App) { Err(std::sync::mpsc::TryRecvError::Empty) => return, Err(std::sync::mpsc::TryRecvError::Disconnected) => { // Worker panicked or dropped tx without sending. Clear - // the worker handles + searching flag so the next debounce - // tick can re-fire if needed. - s.worker_thread = None; + // the rx + searching flag so the next debounce tick can + // re-fire if needed. s.worker_rx = None; s.searching = false; return; } }; - s.worker_thread = None; s.worker_rx = None; match msg { crate::app::SearchWorkerMessage::Done { generation, result } => { diff --git a/crates/kebab-tui/tests/search.rs b/crates/kebab-tui/tests/search.rs index 03257e8..349856d 100644 --- a/crates/kebab-tui/tests/search.rs +++ b/crates/kebab-tui/tests/search.rs @@ -8,7 +8,7 @@ use kebab_core::{ }; use kebab_tui::{ App, KeyOutcome, Pane, SearchState, SearchWorkerMessage, build_jump_command, - handle_key_search, poll_search_worker, render_search, + handle_key_search, poll_search_worker, render_search, search_debounce_due, }; use ratatui::Terminal; use ratatui::backend::TestBackend; @@ -410,6 +410,56 @@ fn poll_worker_noop_when_no_rx() { assert!(s.worker_rx.is_none()); } +/// Helper for the debounce_due tests — build a state with the four +/// fields the test cares about set, others default. +#[allow(clippy::field_reassign_with_default)] +fn search_state_with(input: &str, mode: SearchMode, searching: bool, last_query: Option<(String, SearchMode)>) -> SearchState { + let mut s = SearchState::default(); + s.input = input.into(); + s.mode = mode; + s.searching = searching; + s.last_query = last_query; + s.input_dirty_at = Some( + time::OffsetDateTime::now_utc() - time::Duration::seconds(1), + ); + s +} + +/// p9-fb-08 — `debounce_due` skips when an in-flight worker is +/// already running for the same `(input, mode)` pair. Without this +/// guard, a "phantom keystroke" (re-typing the same chars) would +/// pile up workers and burn CPU. +#[test] +fn debounce_due_skips_when_in_flight_for_same_query() { + let s = search_state_with( + "hello", + SearchMode::Hybrid, + true, + Some(("hello".into(), SearchMode::Hybrid)), + ); + assert!( + !search_debounce_due(&s), + "in-flight worker for same query → debounce must skip" + ); +} + +/// p9-fb-08 — `debounce_due` still fires when a different query is +/// in flight (user typed past the in-flight one). The new spawn +/// makes the prior result stale (handled by `poll_worker`). +#[test] +fn debounce_due_fires_when_in_flight_for_different_query() { + let s = search_state_with( + "hello world", + SearchMode::Hybrid, + true, + Some(("hello".into(), SearchMode::Hybrid)), + ); + assert!( + search_debounce_due(&s), + "in-flight worker for old query → new query still spawns" + ); +} + /// p9-fb-08 — disconnected channel (worker panicked) clears the rx /// + searching flag so the next debounce tick can re-fire cleanly. #[test]