From 596e7e2f49acec14759ffaa356287b8b548f65ca Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 15:04:47 +0100 Subject: [PATCH] feat(maestro): debounce the gitroot probe in the TUI draft MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Typing in the raw path field fired one /api/gitroot probe per keystroke (#74). The draft now schedules the probe 250ms out — matching GITROOT_PROBE_DEBOUNCE_MS in web/src/spawnForm.ts — and every keystroke resets it; the 30Hz heartbeat polls for due probes so the burst's one request fires after typing stops. The seq guard in resolve_gitroot is untouched, so a late answer still cannot overwrite a newer input. --- crates/maestro-tui/src/app.rs | 55 ++++++++++++---- crates/maestro-tui/src/draft.rs | 113 ++++++++++++++++++++++++++++---- 2 files changed, 145 insertions(+), 23 deletions(-) diff --git a/crates/maestro-tui/src/app.rs b/crates/maestro-tui/src/app.rs index cecf43e..1894082 100644 --- a/crates/maestro-tui/src/app.rs +++ b/crates/maestro-tui/src/app.rs @@ -175,7 +175,8 @@ impl App { // Render on next loop iteration. } _ = tokio::time::sleep(Duration::from_millis(33)) => { - // 30 Hz heartbeat. + let effects = self.poll_due_probes_at(std::time::Instant::now()); + self.perform(effects).await; } } } @@ -439,7 +440,7 @@ impl App { if draft.take_name_refill() { effects.push(Effect::RefillNames(tab_idx)); } - if let Some((seq, path)) = draft.take_gitroot_probe() { + if let Some((seq, path)) = draft.take_gitroot_probe(std::time::Instant::now()) { effects.push(Effect::ProbeGitRoot { tab: tab_idx, seq, @@ -452,6 +453,26 @@ impl App { effects } + /// Fire any draft gitroot probes whose debounce window has elapsed (#74). + /// The 30 Hz heartbeat calls this so a burst of typing issues its one + /// probe even after the last keystroke. + fn poll_due_probes_at(&mut self, now: std::time::Instant) -> Vec { + let mut effects = Vec::new(); + for tab_idx in 0..self.state.tabs.len() { + let Some(PaneRef::Draft(draft)) = self.state.tabs[tab_idx].pane.as_mut() else { + continue; + }; + if let Some((seq, path)) = draft.take_gitroot_probe(now) { + effects.push(Effect::ProbeGitRoot { + tab: tab_idx, + seq, + path, + }); + } + } + effects + } + /// Populate a freshly opened draft: presets from `/api/info` (falling back /// to the built-ins already in place), projects from `/api/projects`, and /// a first name batch. Applies only if that tab still shows the draft — @@ -1347,10 +1368,11 @@ mod tests { } } - /// Editing the raw path with the worktree toggle on schedules a - /// `/api/gitroot` probe so the section learns its base repo (#65). + /// Editing the raw path schedules a `/api/gitroot` probe only once the + /// keystroke burst settles (#74): nothing fires inside the debounce + /// window; the heartbeat picks the due probe up afterwards. #[test] - fn typing_a_raw_path_schedules_a_gitroot_probe() { + fn typing_a_raw_path_schedules_a_debounced_gitroot_probe() { let mut app = make_app(); app.dispatch(Action::NewDraftPane); let PaneRef::Draft(draft) = app.state.active_tab_mut().pane.as_mut().unwrap() else { @@ -1359,13 +1381,24 @@ mod tests { draft.cwd_raw_visible = true; draft.focused_field = DraftField::CwdRaw; - let effects = app.handle_draft_input(&InputKey::char('/')); + // A burst of keystrokes fires nothing. + for c in "/opt/tooling".chars() { + assert!(app.handle_draft_input(&InputKey::char(c)).is_empty()); + assert!(app.poll_due_probes_at(std::time::Instant::now()).is_empty()); + } + + // Once the quiet period passes, exactly one probe is due. + let due = std::time::Instant::now() + + std::time::Duration::from_millis(crate::draft::GITROOT_PROBE_DEBOUNCE_MS) * 10; + match app.poll_due_probes_at(due).as_slice() { + [Effect::ProbeGitRoot { tab: 0, path, .. }] => { + assert_eq!(path, &std::path::PathBuf::from("/opt/tooling")); + } + other => panic!("expected one ProbeGitRoot effect, got {other:?}"), + } assert!( - matches!( - effects.as_slice(), - [Effect::ProbeGitRoot { tab: 0, seq: 2, .. }] - ), - "{effects:?}" + app.poll_due_probes_at(due).is_empty(), + "the probe is consumed once" ); } diff --git a/crates/maestro-tui/src/draft.rs b/crates/maestro-tui/src/draft.rs index 6604326..0224662 100644 --- a/crates/maestro-tui/src/draft.rs +++ b/crates/maestro-tui/src/draft.rs @@ -17,6 +17,7 @@ use std::collections::VecDeque; use std::path::{Path, PathBuf}; +use std::time::{Duration, Instant}; use chrono::{DateTime, Utc}; use maestro_config::{builtin_presets, custom_preset}; @@ -35,6 +36,12 @@ use crate::projects; /// dry. pub const NAME_BATCH_SIZE: usize = 20; +/// Quiet period after raw-path typing before a `/api/gitroot` probe fires +/// (#74). Every keystroke resets it, so a burst issues one request once the +/// operator pauses. Must stay equal to `GITROOT_PROBE_DEBOUNCE_MS` in +/// web/src/spawnForm.ts so both clients debounce alike. +pub const GITROOT_PROBE_DEBOUNCE_MS: u64 = 250; + /// Width of the fixed label gutter every form line starts with (`cmd: `, /// `dir: `, …). const GUTTER: usize = 5; @@ -149,6 +156,9 @@ pub struct DraftPane { /// Bumped on every target change; answers carrying an older seq are stale. probe_seq: u32, needs_probe: bool, + /// When the pending probe may fire (debounce, #74). Typing into the raw + /// path resets it; a discrete action like the toggle makes it due at once. + probe_due_at: Option, /// Set when a probe found no repository (drives the rendered hint). repo_missing: bool, /// New-branch name. Tracks [`Self::name`] until edited, like the web form. @@ -198,6 +208,7 @@ impl Default for DraftPane { probed_cwd: None, probe_seq: 0, needs_probe: false, + probe_due_at: None, repo_missing: false, branch: crate::text_input::TextInput::default(), branch_edited: false, @@ -320,18 +331,23 @@ impl DraftPane { self.repo_missing = false; self.probe_seq += 1; self.needs_probe = false; + self.probe_due_at = None; self.probed_cwd = self.cwd(); self.branches_for = None; self.schedule_branch_load(); } /// A target with an unknown repo (raw path edit or the Other… hatch). - /// Cancels any in-flight probe and requests a fresh one. + /// Cancels any in-flight probe and schedules a fresh one after the + /// debounce window (#74). fn note_unknown_repo(&mut self) { self.base_repo = None; self.repo_missing = false; self.probe_seq += 1; self.needs_probe = self.worktree_enabled && self.cwd().is_some(); + self.probe_due_at = self + .needs_probe + .then(|| Instant::now() + Duration::from_millis(GITROOT_PROBE_DEBOUNCE_MS)); self.branches_for = None; self.schedule_branch_load(); } @@ -342,14 +358,19 @@ impl DraftPane { } } - /// Consume the gitroot-probe request: the caller fetches + /// Consume the gitroot-probe request once its debounce window has passed + /// (`now` at or after [`Self::probe_due_at`]): the caller fetches /// `GET /api/gitroot?path=…` and reports back via [`Self::resolve_gitroot`] - /// with the same seq. - pub fn take_gitroot_probe(&mut self) -> Option<(u32, PathBuf)> { + /// with the same seq. Returns `None` while typing is still in flight. + pub fn take_gitroot_probe(&mut self, now: Instant) -> Option<(u32, PathBuf)> { if !self.needs_probe { return None; } + if now < self.probe_due_at? { + return None; + } self.needs_probe = false; + self.probe_due_at = None; let cwd = self.cwd()?; self.probe_seq += 1; Some((self.probe_seq, cwd)) @@ -363,6 +384,7 @@ impl DraftPane { return; } self.needs_probe = false; + self.probe_due_at = None; self.probed_cwd = self.cwd(); match root { Some(repo) => { @@ -434,7 +456,9 @@ impl DraftPane { } if self.base_repo.is_none() { self.probe_seq += 1; + // A discrete toggle, not a keystroke burst: due immediately. self.needs_probe = cwd.is_some(); + self.probe_due_at = self.needs_probe.then(Instant::now); } self.schedule_branch_load(); } @@ -1277,6 +1301,11 @@ mod tests { } } + /// A moment comfortably past the probe debounce window (#74). + fn probe_due() -> Instant { + Instant::now() + Duration::from_millis(GITROOT_PROBE_DEBOUNCE_MS) * 10 + } + #[test] fn defaults_fall_back_to_builtin_presets_with_custom_last() { let draft = DraftPane::default(); @@ -1464,7 +1493,7 @@ mod tests { typed(&mut draft, "/opt/tooling"); assert_eq!(draft.cwd(), Some(PathBuf::from("/home/u/opt/tooling"))); // The raw target probed clean, so the worktree spec carries its repo. - let (seq, _) = draft.take_gitroot_probe().unwrap(); + let (seq, _) = draft.take_gitroot_probe(probe_due()).unwrap(); draft.resolve_gitroot(seq, Some(PathBuf::from("/home/u/opt/tooling"))); draft.seed_names(vec!["amber-falcon".into()]); let req = draft.build_request().unwrap(); @@ -1685,7 +1714,7 @@ mod tests { Some(Path::new("/repos/maestro")), "a picked project resolves its base repo without a probe" ); - assert!(draft.take_gitroot_probe().is_none()); + assert!(draft.take_gitroot_probe(probe_due()).is_none()); draft.seed_names(vec!["crimson-otter".into()]); let wt = draft.build_request().unwrap().worktree.unwrap(); @@ -1725,7 +1754,7 @@ mod tests { draft.on_input(&InputKey::Enter); assert!(draft.worktree_enabled); assert_eq!(draft.base_repo.as_deref(), Some(Path::new("/repos/main"))); - assert!(draft.take_gitroot_probe().is_none()); + assert!(draft.take_gitroot_probe(probe_due()).is_none()); } #[test] @@ -1737,13 +1766,18 @@ mod tests { draft.on_input(&InputKey::Enter); // no projects → Other directory… typed(&mut draft, "/opt/tool"); - let (seq, path) = draft.take_gitroot_probe().expect("probe requested"); + let (seq, path) = draft + .take_gitroot_probe(probe_due()) + .expect("probe requested"); assert_eq!(path, PathBuf::from("/opt/tool")); - assert!(draft.take_gitroot_probe().is_none(), "one probe at a time"); + assert!( + draft.take_gitroot_probe(probe_due()).is_none(), + "one probe at a time" + ); // Further typing supersedes the in-flight probe… typed(&mut draft, "-x"); - let (seq2, _) = draft.take_gitroot_probe().unwrap(); + let (seq2, _) = draft.take_gitroot_probe(probe_due()).unwrap(); assert_ne!(seq2, seq); // …so a late answer to the old one must be ignored. draft.resolve_gitroot(seq, Some(PathBuf::from("/somewhere/else"))); @@ -1764,7 +1798,7 @@ mod tests { }; draft.on_input(&InputKey::Enter); typed(&mut draft, "/not/a/repo"); - let (seq, _) = draft.take_gitroot_probe().unwrap(); + let (seq, _) = draft.take_gitroot_probe(probe_due()).unwrap(); draft.resolve_gitroot(seq, None); assert!(!draft.worktree_enabled); @@ -1908,6 +1942,61 @@ mod tests { ); } + #[test] + fn a_keystroke_burst_schedules_one_debounced_gitroot_probe() { + let mut draft = DraftPane { + focused_field: DraftField::DirPicker, + ..DraftPane::default() + }; + draft.on_input(&InputKey::Enter); // no projects → Other directory… + + // Burst: every keystroke reschedules; nothing may fire inside the + // debounce window (#74). + for c in "/opt/tooling".chars() { + draft.on_input(&InputKey::char(c)); + assert!( + draft.take_gitroot_probe(Instant::now()).is_none(), + "no probe while typing is still in flight" + ); + } + + // Once the quiet period passes, exactly one probe fires. + let (seq, path) = draft + .take_gitroot_probe(probe_due()) + .expect("probe due after the quiet period"); + assert_eq!(path, PathBuf::from("/opt/tooling")); + assert!( + draft.take_gitroot_probe(probe_due()).is_none(), + "one probe total" + ); + + // The answer still lands through the unchanged seq guard. + draft.resolve_gitroot(seq, Some(PathBuf::from("/opt/tooling"))); + assert_eq!(draft.base_repo.as_deref(), Some(Path::new("/opt/tooling"))); + } + + #[test] + fn a_stale_probe_answer_cannot_overwrite_a_newer_target_after_the_debounce() { + let mut draft = DraftPane { + focused_field: DraftField::DirPicker, + ..DraftPane::default() + }; + draft.on_input(&InputKey::Enter); // Other directory… + + typed(&mut draft, "/repo/old"); + let (old_seq, _) = draft.take_gitroot_probe(probe_due()).unwrap(); + + // Retyping supersedes the in-flight probe and gets a newer seq… + typed(&mut draft, "/repo/new"); + let (new_seq, _) = draft.take_gitroot_probe(probe_due()).unwrap(); + assert_ne!(new_seq, old_seq); + + // …so the late answer to the old target must be ignored. + draft.resolve_gitroot(new_seq, Some(PathBuf::from("/repo/new"))); + draft.resolve_gitroot(old_seq, Some(PathBuf::from("/repo/old"))); + assert_eq!(draft.base_repo.as_deref(), Some(Path::new("/repo/new"))); + } + #[test] fn toggling_worktree_off_spawns_in_the_checkout_itself() { let mut draft = DraftPane::default(); @@ -1933,7 +2022,7 @@ mod tests { draft.on_input(&InputKey::Enter); assert!(draft.worktree_enabled); assert!( - draft.take_gitroot_probe().is_none(), + draft.take_gitroot_probe(probe_due()).is_none(), "re-enabling reuses the known repo for an unchanged target" ); }