From fd8597c6963aecbb9c94da3440b691a4884c8c73 Mon Sep 17 00:00:00 2001 From: altair823 Date: Sun, 3 May 2026 03:50:56 +0000 Subject: [PATCH 1/2] feat(kebab-tui): p9-fb-08 async search worker + generation counter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 도그푸딩 item 6 — TUI search 의 200ms debounce 후 동기 호출이 vector / hybrid 모드에서 50-200ms 동안 UI 를 freeze 시키던 문제 해소. 별 thread 에서 search 돌리고 결과 mpsc 로 받음. 사용자가 계속 타이핑하면 stale 결과 자동 폐기 (generation counter pattern, ask.rs 의 worker 패턴과 동일). ## 핵심 변경 - **`SearchState` 필드 3 개 신규**: - `generation: u64` — 각 spawn 마다 increment, worker 가 carry - `worker_thread: Option>` - `worker_rx: Option>` - **`SearchWorkerMessage`** (`pub enum`) — 단일 변종 `Done { generation, result }`. ask.rs 의 token stream 과 달리 search 는 최종 결과만 한 번 send, 그래서 enum 으로 추후 확장 여지 둠. - **`fire_search`** rewrite: generation+1 → debounce snapshot 갱신 → `std::thread::Builder::spawn` 으로 별 thread, `kebab_app::search_ with_config(cfg, query)` 호출, channel 로 `(gen, result)` post. return 은 즉시 — event loop 안 막힘. - **`poll_worker`** 신규 (`pub`, integration test 위해 노출): tick 마다 try_recv. `gen != s.generation` 이면 stale → silently drop + `searching` 그대로 (newer worker 가 처리). 일치하면 hits 적용 + `searching=false`. Disconnect 면 worker 패닉 처리 — searching clear, 다음 tick 의 debounce_due 가 재 spawn. - **`debounce_due`** 강화: `searching && last_query == 현 input/mode` 케이스 skip — 같은 query 재 spawn 방지. 기존 dedupe 도 유지. - **run loop** 의 `Pane::Search` 분기에 `poll_worker(app)` 한 줄 추가 (debounce_due 호출 직전). 매 tick drain. ## 테스트 (tests/search.rs 신규 4 개) - `poll_worker_applies_fresh_result_to_hits` — gen 일치 시 hits 적용 + searching clear + rx drain - `poll_worker_drops_stale_result` — gen 불일치 시 hits 비어 있음 + searching 유지 (newer worker 기다림) - `poll_worker_noop_when_no_rx` — 평상시 tick 에 noop, 기존 hits 보존 - `poll_worker_handles_disconnected_channel` — 워커 panic (tx drop) 복구 — searching clear, rx 비움 기존 17 search + 35 lib + 18 ask + 12 inspect + 10 library = 92 통과. clippy clean. ## 문서 - README `kebab tui` 행: "Search 패널은 200ms debounce 후 background worker, stale 결과 자동 폐기" 한 줄 추가 - HANDOFF: 2026-05-03 entry - spec status planned → in_progress ## Out of scope - 캐시 (p9-fb-19 별도) - 동일 query 의 inflight worker 합치기 — 현재는 dedupe + 가장 최근 spawn 만 살아남는 fire-and-forget. 합치는 건 mpsc multiplexing 로직 필요해 P+ 로 미룸. Co-Authored-By: Claude Opus 4.7 (1M context) --- HANDOFF.md | 1 + README.md | 2 +- crates/kebab-tui/src/app.rs | 33 ++++++- crates/kebab-tui/src/lib.rs | 7 +- crates/kebab-tui/src/run.rs | 5 + crates/kebab-tui/src/search.rs | 136 ++++++++++++++++++++++----- crates/kebab-tui/tests/search.rs | 96 ++++++++++++++++++- tasks/p9/p9-fb-08-search-debounce.md | 2 +- 8 files changed, 252 insertions(+), 30 deletions(-) diff --git a/HANDOFF.md b/HANDOFF.md index 00ea0ca..54c5163 100644 --- a/HANDOFF.md +++ b/HANDOFF.md @@ -51,6 +51,7 @@ P0~P5 직렬. P6~P9 P5 이후 병렬 가능. - **2026-05-03 P9 도그푸딩 후속 (p9-fb-09)** — TUI external editor return restore. Search `g` 키 (citation jump) 후 TUI 화면이 깨지는 버그 수정. `kebab-tui::editor::with_external_program(&mut TuiTerminal, Command)` helper 가 suspend (LeaveAlternateScreen + Show cursor + disable_raw_mode) → spawn → restore (enable_raw_mode + EnterAlternateScreen + Hide cursor + `terminal.clear()`) 시퀀스를 RAII guard 로 atomic 하게 묶음. `App.pending_editor: Option` + `App.force_redraw: bool` 추가 — 키 핸들러는 EditorRequest enqueue 만, 실제 spawn 은 run loop 가 `TuiTerminal` 핸들 들고 처리. 후속 task (p9-fb-20 의 citation jump 등) 가 같은 helper 위에 build. spec: `tasks/p9/p9-fb-09-tui-editor-restore.md`. - **2026-05-03 P9 도그푸딩 후속 (p9-fb-14)** — TUI color theme module. `kebab-tui::theme::{Theme, Role, Palette}` 신규 — 16 개 Role (BorderActive/Title/Path/ModeLexical/ModeVector/ModeHybrid/Selected/Hint/Heading/Warning/Error/Success/CitationMarker/Bullet/Body/BorderInactive) 을 dark + light 두 팔레트가 exhaustive match 로 매핑. 모든 Pane (library/search/ask/inspect/run/error_popup) 의 inline `Style::default().fg(Color::*)` 호출이 `theme.style(Role::X)` 로 격리됨. `Config.ui.theme: String` (default `"dark"`) 신규. `App.theme: Theme` 가 `App::new` 에서 `Theme::from_name(&config.ui.theme)` 로 build — 알 수 없는 값은 dark fallback (config 가 typo 로 죽지 않음). `T` 키 runtime toggle 은 mode machine (p9-fb-12) 미진행이라 skip — config 만으로 결정. p9-fb-11 (ask markdown render) 의 Theme 의존성 unblock. spec: `tasks/p9/p9-fb-14-tui-color-theme.md`. - **2026-05-03 P9 도그푸딩 후속 (p9-fb-11)** — TUI Ask 답변 본문 markdown 렌더. `kebab-tui::markdown::render(text, &Theme) -> Vec>` 신규 — `pulldown-cmark = "0.13"` 위에서 inline (bold/italic/strikethrough/inline code/link)·block (heading H1-H6, ordered/unordered list with nesting, fenced code block, table, blockquote `▎`, horizontal rule) 변환. heading H1/H2 = `Role::Heading`, H3+ = `Role::Title`, link = `Role::CitationMarker + UNDERLINE`, code = `Role::Hint`. ask `push_turn_lines` 가 grounded 답변에서만 markdown 렌더; refusal (`Role::Warning`) / streaming (`Role::Hint`) 은 raw 로 두어 role color 시그널 보존. CLI `kebab ask` 출력은 raw markdown 그대로 (terminal 호환성). 매 frame 재 parse — pulldown 토크나이저가 µs/KB 라 비용 무시. spec: `tasks/p9/p9-fb-11-ask-markdown-render.md`. +- **2026-05-03 P9 도그푸딩 후속 (p9-fb-08)** — TUI search async worker + generation counter. 기존 200ms debounce 후 `kebab_app::search_with_config` 동기 호출이 vector/hybrid 모드 50-200ms 동안 UI freeze 시키던 문제 해소. `SearchState` 에 `generation: u64` + `worker_thread: Option` + `worker_rx: Option>` 신규. `fire_search` 가 spawn 만 하고 즉시 return — worker 가 별 thread 에서 검색 후 `(generation, Result)` 를 channel 로 post. run loop 가 매 tick `poll_worker` 로 try_recv, generation 일치 시 hits 적용 / 불일치 시 silently 폐기 (사용자가 더 빠르게 타이핑하면 stale 결과 자동 drop). debounce_due 가 `searching && last_query == 현 input` 케이스 추가 skip — in-flight worker 의 결과 기다리는 동안 동일 query 재 spawn 안 함. spec: `tasks/p9/p9-fb-08-search-debounce.md`. ## 다음 task 후보 diff --git a/README.md b/README.md index 77ca4df..948f5ec 100644 --- a/README.md +++ b/README.md @@ -76,7 +76,7 @@ kebab doctor | `kebab inspect doc ` / `kebab inspect chunk ` | raw record 보기 | | `kebab ask "" [--show-citations / --hide-citations]` | RAG 답변 + 근거 인용. 답변 후 `근거:` block 으로 full path / line range / score 한 줄씩 (default ON — `--hide-citations` 로 끄기, pipe 시 유용). 근거 부족 시 거절. Ollama 필요 | | `kebab doctor` | 설정/모델/DB 헬스 체크 | -| `kebab tui` | Ratatui 셸 (Library + Search + Ask + Inspect 패널, desktop 진행 중). Library 에서 `r` 키로 background ingest 시작 — 화면 하단 status bar 가 진행 표시, 완료/abort 시 final 라인 잠시 유지 후 자동 hide. ingest 진행 중 `Esc` / `Ctrl-C` 가 cancel signal (그 외에는 quit). Ask 패널은 multi-turn — 같은 conversation 안에서 Q1/A1, Q2/A2 transcript 누적, 다음 질문이 이전 턴을 history 로 받아 답변. 답변 본문은 markdown 렌더 (bold/italic/inline code/heading/list/code fence/table/blockquote, raw `**bold**` 가 실제 굵게 표시). `Ctrl-L` 로 새 conversation 시작. Search 의 `g` 키가 `$EDITOR` (기본 `vi`) 로 hit 의 citation 위치 열기 — 종료 후 TUI 화면이 자동으로 깨끗이 redraw. CLI `kebab ask` 는 raw markdown 그대로 (terminal 호환성 위해) | +| `kebab tui` | Ratatui 셸 (Library + Search + Ask + Inspect 패널, desktop 진행 중). Library 에서 `r` 키로 background ingest 시작 — 화면 하단 status bar 가 진행 표시, 완료/abort 시 final 라인 잠시 유지 후 자동 hide. ingest 진행 중 `Esc` / `Ctrl-C` 가 cancel signal (그 외에는 quit). Search 패널은 200ms debounce 후 background worker 가 검색 — 키 입력으로 UI freeze 안 됨, 사용자가 계속 타이핑하면 stale 결과 자동 폐기 (generation counter). Ask 패널은 multi-turn — 같은 conversation 안에서 Q1/A1, Q2/A2 transcript 누적, 다음 질문이 이전 턴을 history 로 받아 답변. 답변 본문은 markdown 렌더 (bold/italic/inline code/heading/list/code fence/table/blockquote, raw `**bold**` 가 실제 굵게 표시). `Ctrl-L` 로 새 conversation 시작. Search 의 `g` 키가 `$EDITOR` (기본 `vi`) 로 hit 의 citation 위치 열기 — 종료 후 TUI 화면이 자동으로 깨끗이 redraw. CLI `kebab ask` 는 raw markdown 그대로 (terminal 호환성 위해) | | `kebab reset [--all / --data-only / --vector-only / --config-only] [--yes]` | XDG 데이터 wipe. **Irreversible.** TTY 면 confirm prompt, 아니면 `--yes` 필수. `--vector-only` 는 SQLite `embedding_records` 도 함께 truncate (orphan 방지) | | `kebab eval run / compare` | golden query 회귀 측정 | diff --git a/crates/kebab-tui/src/app.rs b/crates/kebab-tui/src/app.rs index 48c7eee..809697e 100644 --- a/crates/kebab-tui/src/app.rs +++ b/crates/kebab-tui/src/app.rs @@ -69,12 +69,38 @@ pub struct SearchState { /// Snapshot of `(input, mode)` at the moment the last search /// fired. The debounce skips re-searches when nothing changed. pub last_query: Option<(String, kebab_core::SearchMode)>, - /// True while a synchronous search call is in flight. The run - /// loop uses this to overlay a "searching…" hint. + /// True while a search worker is in flight. The run loop uses + /// this to overlay a "searching…" hint and to dedupe rapid + /// keystroke spawns. pub searching: bool, /// Cached preview text for the currently-selected hit (lazily /// fetched via `kebab-app::inspect_chunk_with_config`). pub preview: Option, + /// p9-fb-08: monotonic counter incremented every time + /// `fire_search` spawns a worker. Each worker carries its + /// generation back via the channel; if it doesn't match the + /// current value at receive time, the result is silently dropped + /// (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 + /// `crate::search::poll_worker`. `None` between runs. + pub worker_rx: Option>, +} + +/// p9-fb-08: payload posted by the search worker on completion. +/// `generation` matches the value of `SearchState.generation` at the +/// moment the worker was spawned; the run loop drops the message if +/// `generation` no longer matches (a newer query is in flight). +pub enum SearchWorkerMessage { + Done { + generation: u64, + result: anyhow::Result>, + }, } impl Default for SearchState { @@ -88,6 +114,9 @@ impl Default for SearchState { last_query: None, 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 a28a3e8..22bb495 100644 --- a/crates/kebab-tui/src/lib.rs +++ b/crates/kebab-tui/src/lib.rs @@ -28,7 +28,7 @@ mod theme; pub use theme::{Palette, Role, Theme}; pub use app::{ App, AskState, IngestState, InspectState, InspectTarget, KeyOutcome, LibraryState, Pane, - SearchState, TERMINAL_LINE_HOLD_SECS, + SearchState, SearchWorkerMessage, TERMINAL_LINE_HOLD_SECS, }; pub use ask::{handle_key_ask, render_ask}; pub use error_popup::{ErrorOverlay, render_error_overlay}; @@ -43,3 +43,8 @@ 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). +pub use search::poll_worker as poll_search_worker; diff --git a/crates/kebab-tui/src/run.rs b/crates/kebab-tui/src/run.rs index 1d1665f..b9e103b 100644 --- a/crates/kebab-tui/src/run.rs +++ b/crates/kebab-tui/src/run.rs @@ -65,6 +65,11 @@ pub(crate) fn run_loop(app: &mut App) -> Result<()> { } } Pane::Search => { + // p9-fb-08: drain the async search worker first. + // Stale generations are silently dropped; the + // current generation's result populates `hits` + // / clears `searching` here. + crate::search::poll_worker(app); let due = app .search .as_ref() diff --git a/crates/kebab-tui/src/search.rs b/crates/kebab-tui/src/search.rs index 71092a4..57a8510 100644 --- a/crates/kebab-tui/src/search.rs +++ b/crates/kebab-tui/src/search.rs @@ -427,7 +427,10 @@ fn parse_editor_env(env: &str) -> (String, Vec) { /// Run-loop hook: tick called every poll cycle. Returns `true` if a /// search should fire this tick (debounce expired and query -/// changed). +/// changed). p9-fb-08 adds two skip cases: +/// - 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 { let Some(at) = s.input_dirty_at else { return false }; let elapsed = (time::OffsetDateTime::now_utc() - at) @@ -440,6 +443,16 @@ pub(crate) fn debounce_due(s: &SearchState) -> bool { if q.is_empty() { return false; } + // p9-fb-08: if the most-recent in-flight query is identical to + // the current input/mode pair, don't spawn another worker — the + // existing result will land via `poll_worker`. + if s.searching { + if let Some((prev_input, prev_mode)) = &s.last_query { + if prev_input == &s.input && *prev_mode == s.mode { + return false; + } + } + } !matches!( &s.last_query, Some((prev_input, prev_mode)) @@ -447,38 +460,113 @@ pub(crate) fn debounce_due(s: &SearchState) -> bool { ) } -/// Run-loop hook: actually perform the search, populate `hits`. The -/// state's `input_dirty_at` is cleared, `last_query` snapshots, and -/// `searching` flag toggles around the call. +/// Run-loop hook: spawn an asynchronous search worker. Returns +/// immediately so the event loop keeps polling — the result lands in +/// `state.search.worker_rx` and is applied by `poll_worker` on a +/// later tick. p9-fb-08 deviation from the original synchronous +/// design (the user typed faster than vector search could complete, +/// freezing the UI for 50-200 ms per keystroke under hybrid mode). +/// +/// Behavior: +/// 1. Increment `generation` so any in-flight result becomes stale +/// on receive (`poll_worker` drops it). +/// 2. Drop the prior `worker_rx` (the old worker keeps running and +/// its result is silently discarded — search is a pure read with +/// no cleanup obligation). +/// 3. Snapshot `last_query` + clear `input_dirty_at` for the +/// debounce machinery (so a no-op keystroke doesn't re-spawn). +/// 4. Spawn a fresh worker carrying its generation token. pub(crate) fn fire_search(state: &mut App) -> anyhow::Result<()> { let cfg = state.config.clone(); - let (q_text, mode) = { + let (q_text, mode, generation) = { let s = state.search.as_mut().expect("Search slot must exist"); + s.generation = s.generation.wrapping_add(1); s.searching = true; s.input_dirty_at = None; s.last_query = Some((s.input.clone(), s.mode)); - (s.input.clone(), s.mode) + (s.input.clone(), s.mode, s.generation) }; - let query = SearchQuery { - text: q_text, - mode, - k: SEARCH_K, - filters: kebab_core::SearchFilters::default(), - }; - let result = kebab_app::search_with_config(cfg, query); + + let (tx, rx) = std::sync::mpsc::channel(); + let handle = std::thread::Builder::new() + .name(format!("kebab-tui-search-gen{generation}")) + .spawn(move || { + let query = SearchQuery { + text: q_text, + mode, + k: SEARCH_K, + 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, + }); + }) + .map_err(|e| anyhow::anyhow!("spawn search worker: {e}"))?; + let s = state.search.as_mut().expect("Search slot must exist"); - s.searching = false; - match result { - Ok(hits) => { - s.hits = hits; - s.selected_hit = 0; - s.preview = None; - Ok(()) + s.worker_thread = Some(handle); + s.worker_rx = Some(rx); + Ok(()) +} + +/// Run-loop hook: drain any pending message from the search worker. +/// Stale results (newer query already in flight) are silently +/// dropped per the generation-counter contract. `pub` so integration +/// tests can drive the stale-result paths by injecting a channel. +pub fn poll_worker(state: &mut App) { + let Some(s) = state.search.as_mut() else { return }; + let Some(rx) = s.worker_rx.as_ref() else { return }; + let msg = match rx.try_recv() { + Ok(m) => m, + 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; + s.worker_rx = None; + s.searching = false; + return; } - Err(e) => { - s.hits.clear(); - s.selected_hit = 0; - Err(e) + }; + s.worker_thread = None; + s.worker_rx = None; + match msg { + crate::app::SearchWorkerMessage::Done { generation, result } => { + // p9-fb-08: stale guard. The user kept typing after this + // worker spawned and a newer query is in flight — drop + // the result. Don't clear `searching` because the newer + // worker (if any) is still running; if there's no newer + // worker (rare race), the next debounce_due tick will + // re-fire `fire_search` and reset everything. + if generation != s.generation { + tracing::debug!( + target: "kebab-tui", + stale_gen = generation, + current_gen = s.generation, + "dropping stale search result" + ); + return; + } + s.searching = false; + match result { + Ok(hits) => { + s.hits = hits; + s.selected_hit = 0; + s.preview = None; + } + Err(e) => { + s.hits.clear(); + s.selected_hit = 0; + state.error_overlay = + Some(crate::error_popup::ErrorOverlay::from_anyhow(&e)); + } + } } } } diff --git a/crates/kebab-tui/tests/search.rs b/crates/kebab-tui/tests/search.rs index 358dc04..03257e8 100644 --- a/crates/kebab-tui/tests/search.rs +++ b/crates/kebab-tui/tests/search.rs @@ -7,7 +7,8 @@ use kebab_core::{ RetrievalDetail, SearchHit, SearchMode, WorkspacePath, }; use kebab_tui::{ - App, KeyOutcome, Pane, SearchState, build_jump_command, handle_key_search, render_search, + App, KeyOutcome, Pane, SearchState, SearchWorkerMessage, build_jump_command, + handle_key_search, poll_search_worker, render_search, }; use ratatui::Terminal; use ratatui::backend::TestBackend; @@ -334,6 +335,99 @@ fn g_key_with_no_hits_does_not_enqueue() { ); } +// ── p9-fb-08: async search worker + generation counter ──────────── + +/// `poll_search_worker` applies a fresh result (matching generation) +/// to `state.search.hits` and clears `searching`. +#[test] +fn poll_worker_applies_fresh_result_to_hits() { + let mut app = fresh_app(); + let (tx, rx) = std::sync::mpsc::channel(); + { + let s = app.search.as_mut().unwrap(); + s.generation = 5; + s.searching = true; + s.worker_rx = Some(rx); + } + let hit = make_hit(1, "a.md", "snip", line_citation("a.md", 1)); + tx.send(SearchWorkerMessage::Done { + generation: 5, + result: Ok(vec![hit]), + }) + .unwrap(); + poll_search_worker(&mut app); + let s = app.search.as_ref().unwrap(); + assert_eq!(s.hits.len(), 1, "fresh result populates hits"); + assert!(!s.searching, "searching cleared"); + assert!(s.worker_rx.is_none(), "rx drained"); +} + +/// p9-fb-08 — a stale result (generation mismatch) is silently +/// dropped. `searching` remains true since a newer worker is +/// (presumed) still in flight. +#[test] +fn poll_worker_drops_stale_result() { + let mut app = fresh_app(); + let (tx, rx) = std::sync::mpsc::channel(); + { + let s = app.search.as_mut().unwrap(); + s.generation = 7; + s.searching = true; + s.worker_rx = Some(rx); + } + let hit = make_hit(1, "stale.md", "snip", line_citation("stale.md", 1)); + // generation 3 < current 7 → stale. + tx.send(SearchWorkerMessage::Done { + generation: 3, + result: Ok(vec![hit]), + }) + .unwrap(); + poll_search_worker(&mut app); + let s = app.search.as_ref().unwrap(); + assert!(s.hits.is_empty(), "stale result must not populate hits"); + assert!( + s.searching, + "searching stays true so newer worker can resolve it" + ); + assert!( + s.worker_rx.is_none(), + "stale message still drains the rx slot — worker is one-shot" + ); +} + +/// p9-fb-08 — `poll_search_worker` is a no-op when no worker is in +/// flight (no rx). Common case on every tick the user isn't typing. +#[test] +fn poll_worker_noop_when_no_rx() { + let mut app = fresh_app(); + { + let s = app.search.as_mut().unwrap(); + s.hits = vec![make_hit(1, "x.md", "snip", line_citation("x.md", 1))]; + } + poll_search_worker(&mut app); + let s = app.search.as_ref().unwrap(); + assert_eq!(s.hits.len(), 1, "existing hits preserved"); + assert!(s.worker_rx.is_none()); +} + +/// p9-fb-08 — disconnected channel (worker panicked) clears the rx +/// + searching flag so the next debounce tick can re-fire cleanly. +#[test] +fn poll_worker_handles_disconnected_channel() { + let mut app = fresh_app(); + let (tx, rx) = std::sync::mpsc::channel::(); + { + let s = app.search.as_mut().unwrap(); + s.searching = true; + s.worker_rx = Some(rx); + } + drop(tx); // simulate worker panic before send + poll_search_worker(&mut app); + let s = app.search.as_ref().unwrap(); + assert!(!s.searching, "searching cleared on disconnect"); + assert!(s.worker_rx.is_none()); +} + #[test] fn no_search_state_returns_to_library() { let mut config = Config::defaults(); diff --git a/tasks/p9/p9-fb-08-search-debounce.md b/tasks/p9/p9-fb-08-search-debounce.md index 48319e3..4e18121 100644 --- a/tasks/p9/p9-fb-08-search-debounce.md +++ b/tasks/p9/p9-fb-08-search-debounce.md @@ -3,7 +3,7 @@ phase: P9 component: kebab-tui (search pane) task_id: p9-fb-08 title: "Search debounce + Enter-immediate trigger" -status: planned +status: in_progress depends_on: [] unblocks: [] contract_source: ../../docs/superpowers/specs/2026-04-27-kebab-final-form-design.md From a99f81398c39949292486bb473b63d7d50e880c7 Mon Sep 17 00:00:00 2001 From: altair823 Date: Sun, 3 May 2026 03:55:27 +0000 Subject: [PATCH 2/2] =?UTF-8?q?review(p9-fb-08):=20=ED=9A=8C=EC=B0=A8=201?= =?UTF-8?q?=20nit=20=EB=B0=98=EC=98=81?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `SearchState.worker_thread` 필드 제거 — `JoinHandle` 을 저장만 하고 어디서도 join 안 했음. fire_search 가 spawn 후 handle 을 즉시 drop 하면 OS 가 thread 를 detach (search 는 pure read 라 cleanup 의무 없음). YAGNI — ask.rs 의 thread 와 달리 cancel/observe 수요가 없는 fire-and-forget. doc 으로 의도 명시. - `debounce_due` 가 `pub` 으로 노출 — 새 skip 분기 (`searching && 같은 query`) 회귀 테스트 추가: - `debounce_due_skips_when_in_flight_for_same_query`: 같은 input/mode 재입력 시 spawn 안 함 (worker 누적 방지) - `debounce_due_fires_when_in_flight_for_different_query`: 사용자가 in-flight 보다 빠르게 새 query 입력하면 정상 spawn (poll_worker 의 stale guard 가 이전 결과 처리) - `search_state_with` 헬퍼: `SearchState::default()` + field 재할당 패턴이 clippy `field_reassign_with_default` 위반 → `#[allow(...)]` 로 lint 무시 (테스트 helper 의 가독성 우선). 23 tests/search.rs + 35 lib + 18 ask + 12 inspect + 10 library = 98 통과. clippy clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/kebab-tui/src/app.rs | 16 ++++++---- crates/kebab-tui/src/lib.rs | 10 +++--- crates/kebab-tui/src/search.rs | 19 ++++++------ crates/kebab-tui/tests/search.rs | 52 +++++++++++++++++++++++++++++++- 4 files changed, 76 insertions(+), 21 deletions(-) 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]