From 4a26987d8eb514ca69c57005a2c9123ae0232785 Mon Sep 17 00:00:00 2001 From: Zaki Date: Sun, 22 Mar 2026 23:26:31 -0700 Subject: [PATCH] perf(agent): eliminate redundant UUID parsing in approval path (#1488) Pass pre-parsed UUID through the approval resolution flow instead of parsing the same string twice. The new resolve_thread_with_parsed_uuid method accepts an optional pre-parsed UUID, and resolve_thread delegates to it with None for backward compatibility. Closes #1488 Co-Authored-By: Claude Opus 4.6 (1M context) --- src/agent/agent_loop.rs | 3 +- src/agent/session_manager.rs | 78 ++++++++++++++++++++++++++++++++++-- 2 files changed, 77 insertions(+), 4 deletions(-) diff --git a/src/agent/agent_loop.rs b/src/agent/agent_loop.rs index 5cbd8166..555ecfd9 100644 --- a/src/agent/agent_loop.rs +++ b/src/agent/agent_loop.rs @@ -1052,10 +1052,11 @@ impl Agent { } else { drop(sess); self.session_manager - .resolve_thread( + .resolve_thread_with_parsed_uuid( &message.user_id, &message.channel, message.conversation_scope(), + approval_thread_uuid, ) .await } diff --git a/src/agent/session_manager.rs b/src/agent/session_manager.rs index 3bf20697..1fb6bc57 100644 --- a/src/agent/session_manager.rs +++ b/src/agent/session_manager.rs @@ -107,6 +107,20 @@ impl SessionManager { user_id: &str, channel: &str, external_thread_id: Option<&str>, + ) -> (Arc>, Uuid) { + self.resolve_thread_with_parsed_uuid(user_id, channel, external_thread_id, None) + .await + } + + /// Like [`resolve_thread`](Self::resolve_thread), but accepts a pre-parsed + /// UUID to skip redundant parsing when the caller has already validated + /// the external thread ID as a UUID (e.g. the approval routing path). + pub async fn resolve_thread_with_parsed_uuid( + &self, + user_id: &str, + channel: &str, + external_thread_id: Option<&str>, + parsed_uuid: Option, ) -> (Arc>, Uuid) { let session = self.get_or_create_session(user_id).await; @@ -133,9 +147,12 @@ impl SessionManager { // (e.g. created by chat_new_thread_handler or hydrated from DB). // We only adopt it if no thread_map entry maps to this UUID — // otherwise it belongs to a different channel scope. - if let Some(ext_tid) = external_thread_id - && let Ok(ext_uuid) = Uuid::parse_str(ext_tid) - { + // Use pre-parsed UUID if available, otherwise parse from string. + let ext_uuid = parsed_uuid.or_else(|| { + external_thread_id.and_then(|ext_tid| Uuid::parse_str(ext_tid).ok()) + }); + + if let Some(ext_uuid) = ext_uuid { let thread_map = self.thread_map.read().await; let mapped_elsewhere = thread_map.values().any(|&v| v == ext_uuid); drop(thread_map); @@ -947,4 +964,59 @@ mod tests { "should have exactly 1 thread, not a duplicate" ); } + + #[tokio::test] + async fn test_resolve_thread_with_pre_parsed_uuid_adopts_thread() { + use crate::agent::session::Thread; + + let manager = SessionManager::new(); + let (session, _) = manager.resolve_thread("user1", "chan1", None).await; + + // Manually insert a thread with a known UUID + let known_id = Uuid::new_v4(); + { + let mut sess = session.lock().await; + let thread = Thread::with_id(known_id, sess.id); + sess.threads.insert(known_id, thread); + } + + // Resolve with pre-parsed UUID -- should adopt it without re-parsing + let (_, resolved) = manager + .resolve_thread_with_parsed_uuid( + "user1", + "chan1", + Some(&known_id.to_string()), + Some(known_id), + ) + .await; + assert_eq!(resolved, known_id); + } + + #[tokio::test] + async fn test_resolve_thread_with_parsed_uuid_none_delegates_to_parse() { + use crate::agent::session::Thread; + + let manager = SessionManager::new(); + let (session, _) = manager.resolve_thread("user2", "chan2", None).await; + + // Insert a thread with a known UUID + let known_id = Uuid::new_v4(); + { + let mut sess = session.lock().await; + let thread = Thread::with_id(known_id, sess.id); + sess.threads.insert(known_id, thread); + } + + // Resolve with parsed_uuid=None but a valid UUID string -- should + // fall back to parsing the string and still adopt the thread + let (_, resolved) = manager + .resolve_thread_with_parsed_uuid( + "user2", + "chan2", + Some(&known_id.to_string()), + None, + ) + .await; + assert_eq!(resolved, known_id); + } }