perf: use keyed terminal lookup for named targets (#4669)

justfile

@@ -87,6 +87,10 @@ build:
 bench-render-scale:
     cargo test --release --locked --bin herdr render_scale_profile -- --ignored --nocapture --test-threads=1
 
+# Profile terminal target name resolution at increasing pane counts.
+bench-terminal-targets:
+    cargo test --release --locked --bin herdr terminal_target_lookup_profile -- --ignored --nocapture --test-threads=1
+
 # ~3-5 minute CPU comparison; downloads stable unless HERDR_PERF_BASELINE_BIN is set
 bench-release-smoke:
     cargo build --release --locked

src/app/terminal_targets.rs

@@ -55,8 +55,7 @@ impl App {
             .filter(|candidate| {
                 self.state
                     .terminals
-                    .values()
-                    .find(|terminal| terminal.id.to_string() == candidate.terminal_id)
+                    .get(candidate.terminal_id.as_str())
                     .is_some_and(|terminal| {
                         terminal.agent_name.as_deref() == Some(target)
                             || terminal.effective_agent_label() == Some(target)
@@ -91,8 +90,7 @@ impl App {
             .filter(|candidate| {
                 self.state
                     .terminals
-                    .values()
-                    .find(|terminal| terminal.id.to_string() == candidate.terminal_id)
+                    .get(candidate.terminal_id.as_str())
                     .is_some_and(|terminal| terminal.agent_name.as_deref() == Some(target))
             })
             .collect();
@@ -108,8 +106,7 @@ impl App {
     fn target_is_agent(&self, target: &TerminalTarget) -> bool {
         self.state
             .terminals
-            .values()
-            .find(|terminal| terminal.id.to_string() == target.terminal_id)
+            .get(target.terminal_id.as_str())
             .is_some_and(|terminal| terminal.is_agent_terminal())
     }
 
@@ -193,3 +190,140 @@ impl App {
         })
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::app::{AppPolicy, AppState};
+
+    fn test_app() -> App {
+        let (_, api_rx) = tokio::sync::mpsc::unbounded_channel();
+        App::new(
+            &crate::config::Config::default(),
+            AppPolicy::TEST,
+            None,
+            api_rx,
+            crate::api::EventHub::default(),
+        )
+    }
+
+    #[test]
+    fn named_targets_follow_terminal_identity_after_pane_and_tab_reordering() {
+        let mut app = test_app();
+        app.state = AppState::test_with_adversarial_identity_state();
+        let targets = app.terminal_targets();
+        for (index, target) in targets.iter().enumerate() {
+            app.state
+                .terminals
+                .get_mut(target.terminal_id.as_str())
+                .unwrap()
+                .set_agent_name(format!("worker-{index}"));
+        }
+        for (index, target) in targets.iter().enumerate() {
+            let name = format!("worker-{index}");
+            assert_eq!(app.resolve_terminal_target(&name).unwrap(), *target);
+            assert_eq!(app.resolve_agent_target(&name).unwrap(), *target);
+            let pane = app.public_pane_id(target.ws_idx, target.pane_id).unwrap();
+            assert_eq!(app.resolve_agent_target(&pane).unwrap(), *target);
+        }
+        app.state.assert_invariants_for_test();
+    }
+
+    #[test]
+    fn name_lookup_keeps_pane_order_and_ignores_detached_terminals() {
+        let mut app = test_app();
+        app.state = AppState::test_with_adversarial_identity_state();
+        let targets = app.terminal_targets();
+        for target in &targets {
+            app.state
+                .terminals
+                .get_mut(target.terminal_id.as_str())
+                .unwrap()
+                .set_agent_name("shared".into());
+        }
+        let detached_id = crate::terminal::TerminalId::alloc();
+        let mut detached =
+            crate::terminal::TerminalState::new(detached_id.clone(), std::env::temp_dir());
+        detached.set_agent_name("detached".into());
+        app.state.terminals.insert(detached_id, detached);
+        assert!(matches!(
+            app.resolve_agent_target("detached"),
+            Err(TerminalTargetError::NotFound { .. })
+        ));
+        for result in [
+            app.resolve_terminal_target("shared"),
+            app.resolve_agent_target("shared"),
+        ] {
+            let Err(TerminalTargetError::Ambiguous { candidates, .. }) = result else {
+                panic!("expected all attached panes to remain ambiguous");
+            };
+            assert_eq!(
+                candidates
+                    .iter()
+                    .map(|candidate| candidate.terminal_id.as_str())
+                    .collect::<Vec<_>>(),
+                targets
+                    .iter()
+                    .map(|target| target.terminal_id.as_str())
+                    .collect::<Vec<_>>(),
+            );
+        }
+    }
+
+    #[test]
+    #[ignore = "manual terminal target lookup scaling profile"]
+    fn terminal_target_lookup_profile() {
+        use std::hint::black_box;
+        use std::time::{Duration, Instant};
+
+        for count in [1, 15, 128, 512] {
+            let mut app = test_app();
+            let mut workspace = crate::workspace::Workspace::test_new("lookup-profile");
+            for _ in 1..count {
+                workspace.test_split(ratatui::layout::Direction::Horizontal);
+            }
+            app.state.workspaces = vec![workspace];
+            app.state.ensure_test_terminals();
+            let id = app.state.workspaces[0]
+                .terminal_id(app.state.workspaces[0].tabs[0].root_pane)
+                .unwrap()
+                .clone();
+            app.state
+                .terminals
+                .get_mut(&id)
+                .unwrap()
+                .set_agent_name("profile-target".into());
+            for (label, agent, target) in [
+                ("terminal-name", false, "profile-target"),
+                ("agent-name", true, "profile-target"),
+                ("missing", false, "missing-target"),
+            ] {
+                let lookup = || {
+                    if agent {
+                        app.resolve_agent_target(target)
+                    } else {
+                        app.resolve_terminal_target(target)
+                    }
+                };
+                for _ in 0..32 {
+                    black_box(lookup()).ok();
+                }
+                let mut samples = Vec::new();
+                for _ in 0..7 {
+                    let start = Instant::now();
+                    let mut iterations = 0;
+                    while start.elapsed() < Duration::from_millis(20) {
+                        black_box(lookup()).ok();
+                        iterations += 1;
+                    }
+                    samples.push(start.elapsed().as_secs_f64() * 1e6 / f64::from(iterations));
+                }
+                samples.sort_by(f64::total_cmp);
+                println!(
+                    "terminal-target panes={count} case={label} median_us={:.3}",
+                    samples[3]
+                );
+            }
+        }
+    }
+}

src/terminal/id.rs

@@ -1,3 +1,4 @@
+use std::borrow::Borrow;
 use std::fmt;
 use std::sync::atomic::{AtomicU64, Ordering};
 use std::time::{SystemTime, UNIX_EPOCH};
@@ -31,3 +32,29 @@ impl fmt::Display for TerminalId {
         f.write_str(&self.0)
     }
 }
+
+impl Borrow<str> for TerminalId {
+    fn borrow(&self) -> &str {
+        self.as_str()
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::collections::HashMap;
+
+    #[test]
+    fn terminal_ids_support_borrowed_lookup_without_changing_identity() {
+        let id = TerminalId::alloc();
+        let serialized = serde_json::to_string(&id).unwrap();
+        let restored: TerminalId = serde_json::from_str(&serialized).unwrap();
+        let mut terminals = HashMap::from([(id.clone(), 1)]);
+
+        assert_eq!(terminals.get(restored.as_str()), Some(&1));
+        assert_eq!(terminals.get("missing-terminal"), None);
+        *terminals.get_mut(restored.as_str()).unwrap() = 2;
+        assert_eq!(terminals.remove(id.as_str()), Some(2));
+        assert!(terminals.is_empty());
+    }
+}