From 09b038831235ee38f84d235cdba53452c3a29535 Mon Sep 17 00:00:00 2001 From: Bennet Bo Fenner Date: Thu, 19 Mar 2026 14:18:11 +0100 Subject: [PATCH] Maintain cache in `ThreadMetadataStore` (#51923) ## Context This makes it so that we maintain a cached state on the thread metadata store itself, rather than storing it at other locations. Which is similar to how `ThreadStore` works. ## How to Review ## Self-Review Checklist - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - N/A --- crates/agent_ui/src/thread_metadata_store.rs | 384 +++++++++++++++---- crates/sidebar/src/sidebar.rs | 206 +++++----- 2 files changed, 406 insertions(+), 184 deletions(-) diff --git a/crates/agent_ui/src/thread_metadata_store.rs b/crates/agent_ui/src/thread_metadata_store.rs index 7b20ac268271bae977ccae57b84c4401d3c5fc94..63914d1e9a8f32b4a4258109b29e8f35787f05d8 100644 --- a/crates/agent_ui/src/thread_metadata_store.rs +++ b/crates/agent_ui/src/thread_metadata_store.rs @@ -3,7 +3,7 @@ use std::{path::Path, sync::Arc}; use acp_thread::AgentSessionInfo; use agent::{ThreadStore, ZED_AGENT_ID}; use agent_client_protocol as acp; -use anyhow::Result; +use anyhow::{Context as _, Result}; use chrono::{DateTime, Utc}; use collections::HashMap; use db::{ @@ -14,9 +14,11 @@ use db::{ sqlez_macros::sql, }; use feature_flags::{AgentV2FeatureFlag, FeatureFlagAppExt}; +use futures::{FutureExt as _, future::Shared}; use gpui::{AppContext as _, Entity, Global, Subscription, Task}; use project::AgentId; use ui::{App, Context, SharedString}; +use util::ResultExt as _; use workspace::PathList; pub fn init(cx: &mut App) { @@ -37,35 +39,39 @@ pub fn init(cx: &mut App) { /// /// TODO: Remove this after N weeks of shipping the sidebar fn migrate_thread_metadata(cx: &mut App) { - SidebarThreadMetadataStore::global(cx).update(cx, |store, cx| { - let list = store.list(cx); - cx.spawn(async move |this, cx| { - let Ok(list) = list.await else { - return; - }; - if list.is_empty() { - this.update(cx, |this, cx| { - let metadata = ThreadStore::global(cx) - .read(cx) - .entries() - .map(|entry| ThreadMetadata { - session_id: entry.id, - agent_id: None, - title: entry.title, - updated_at: entry.updated_at, - created_at: entry.created_at, - folder_paths: entry.folder_paths, - }) - .collect::>(); - for entry in metadata { - this.save(entry, cx).detach_and_log_err(cx); - } + let store = SidebarThreadMetadataStore::global(cx); + let db = store.read(cx).db.clone(); + + cx.spawn(async move |cx| { + if !db.is_empty()? { + return Ok::<(), anyhow::Error>(()); + } + + let metadata = store.read_with(cx, |_store, app| { + ThreadStore::global(app) + .read(app) + .entries() + .map(|entry| ThreadMetadata { + session_id: entry.id, + agent_id: None, + title: entry.title, + updated_at: entry.updated_at, + created_at: entry.created_at, + folder_paths: entry.folder_paths, }) - .ok(); - } - }) - .detach(); - }); + .collect::>() + }); + + // Manually save each entry to the database and call reload, otherwise + // we'll end up triggering lots of reloads after each save + for entry in metadata { + db.save(entry).await?; + } + + let _ = store.update(cx, |store, cx| store.reload(cx)); + Ok(()) + }) + .detach_and_log_err(cx); } struct GlobalThreadMetadataStore(Entity); @@ -146,6 +152,9 @@ impl ThreadMetadata { /// Automatically listens to AcpThread events and updates metadata if it has changed. pub struct SidebarThreadMetadataStore { db: ThreadMetadataDb, + threads: Vec, + threads_by_paths: HashMap>, + reload_task: Option>>, session_subscriptions: HashMap, } @@ -180,20 +189,61 @@ impl SidebarThreadMetadataStore { cx.global::().0.clone() } - pub fn list_ids(&self, cx: &App) -> Task>> { - let db = self.db.clone(); - cx.background_spawn(async move { - let s = db.list_ids()?; - Ok(s) - }) + pub fn is_empty(&self) -> bool { + self.threads.is_empty() + } + + pub fn entries(&self) -> impl Iterator + '_ { + self.threads.iter().cloned() + } + + pub fn entry_ids(&self) -> impl Iterator + '_ { + self.threads.iter().map(|thread| thread.session_id.clone()) + } + + pub fn entries_for_path( + &self, + path_list: &PathList, + ) -> impl Iterator + '_ { + self.threads_by_paths + .get(path_list) + .into_iter() + .flatten() + .cloned() } - pub fn list(&self, cx: &App) -> Task>> { + fn reload(&mut self, cx: &mut Context) -> Shared> { let db = self.db.clone(); - cx.background_spawn(async move { - let s = db.list()?; - Ok(s) - }) + self.reload_task.take(); + + let list_task = cx + .background_spawn(async move { db.list().context("Failed to fetch sidebar metadata") }); + + let reload_task = cx + .spawn(async move |this, cx| { + let Some(rows) = list_task.await.log_err() else { + return; + }; + + this.update(cx, |this, cx| { + this.threads.clear(); + this.threads_by_paths.clear(); + + for row in rows { + this.threads_by_paths + .entry(row.folder_paths.clone()) + .or_default() + .push(row.clone()); + this.threads.push(row); + } + + cx.notify(); + }) + .ok(); + }) + .shared(); + self.reload_task = Some(reload_task.clone()); + reload_task } pub fn save(&mut self, metadata: ThreadMetadata, cx: &mut Context) -> Task> { @@ -204,7 +254,9 @@ impl SidebarThreadMetadataStore { let db = self.db.clone(); cx.spawn(async move |this, cx| { db.save(metadata).await?; - this.update(cx, |_this, cx| cx.notify()) + let reload_task = this.update(cx, |this, cx| this.reload(cx))?; + reload_task.await; + Ok(()) }) } @@ -220,7 +272,9 @@ impl SidebarThreadMetadataStore { let db = self.db.clone(); cx.spawn(async move |this, cx| { db.delete(session_id).await?; - this.update(cx, |_this, cx| cx.notify()) + let reload_task = this.update(cx, |this, cx| this.reload(cx))?; + reload_task.await; + Ok(()) }) } @@ -257,10 +311,15 @@ impl SidebarThreadMetadataStore { }) .detach(); - Self { + let mut this = Self { db, + threads: Vec::new(), + threads_by_paths: HashMap::default(), + reload_task: None, session_subscriptions: HashMap::default(), - } + }; + let _ = this.reload(cx); + this } fn handle_thread_update( @@ -309,10 +368,9 @@ impl Domain for ThreadMetadataDb { db::static_connection!(ThreadMetadataDb, []); impl ThreadMetadataDb { - /// List all sidebar thread session IDs. - pub fn list_ids(&self) -> anyhow::Result> { - self.select::>("SELECT session_id FROM sidebar_threads")?() - .map(|ids| ids.into_iter().map(|id| acp::SessionId::new(id)).collect()) + pub fn is_empty(&self) -> anyhow::Result { + self.select::("SELECT COUNT(*) FROM sidebar_threads")?() + .map(|counts| counts.into_iter().next().unwrap_or_default() == 0) } /// List all sidebar thread metadata, ordered by updated_at descending. @@ -427,7 +485,6 @@ mod tests { use project::Project; use std::path::Path; use std::rc::Rc; - use util::path_list::PathList; fn make_db_thread(title: &str, updated_at: DateTime) -> DbThread { DbThread { @@ -450,6 +507,207 @@ mod tests { } } + fn make_metadata( + session_id: &str, + title: &str, + updated_at: DateTime, + folder_paths: PathList, + ) -> ThreadMetadata { + ThreadMetadata { + session_id: acp::SessionId::new(session_id), + agent_id: None, + title: title.to_string().into(), + updated_at, + created_at: Some(updated_at), + folder_paths, + } + } + + #[gpui::test] + async fn test_store_initializes_cache_from_database(cx: &mut TestAppContext) { + let first_paths = PathList::new(&[Path::new("/project-a")]); + let second_paths = PathList::new(&[Path::new("/project-b")]); + let now = Utc::now(); + let older = now - chrono::Duration::seconds(1); + + let thread = std::thread::current(); + let test_name = thread.name().unwrap_or("unknown_test"); + let db_name = format!("THREAD_METADATA_DB_{}", test_name); + let db = ThreadMetadataDb(smol::block_on(db::open_test_db::( + &db_name, + ))); + + db.save(make_metadata( + "session-1", + "First Thread", + now, + first_paths.clone(), + )) + .await + .unwrap(); + db.save(make_metadata( + "session-2", + "Second Thread", + older, + second_paths.clone(), + )) + .await + .unwrap(); + + cx.update(|cx| { + let settings_store = settings::SettingsStore::test(cx); + cx.set_global(settings_store); + cx.update_flags(true, vec!["agent-v2".to_string()]); + SidebarThreadMetadataStore::init_global(cx); + }); + + cx.run_until_parked(); + + cx.update(|cx| { + let store = SidebarThreadMetadataStore::global(cx); + let store = store.read(cx); + + let entry_ids = store + .entry_ids() + .map(|session_id| session_id.0.to_string()) + .collect::>(); + assert_eq!(entry_ids, vec!["session-1", "session-2"]); + + let first_path_entries = store + .entries_for_path(&first_paths) + .map(|entry| entry.session_id.0.to_string()) + .collect::>(); + assert_eq!(first_path_entries, vec!["session-1"]); + + let second_path_entries = store + .entries_for_path(&second_paths) + .map(|entry| entry.session_id.0.to_string()) + .collect::>(); + assert_eq!(second_path_entries, vec!["session-2"]); + }); + } + + #[gpui::test] + async fn test_store_cache_updates_after_save_and_delete(cx: &mut TestAppContext) { + cx.update(|cx| { + let settings_store = settings::SettingsStore::test(cx); + cx.set_global(settings_store); + cx.update_flags(true, vec!["agent-v2".to_string()]); + SidebarThreadMetadataStore::init_global(cx); + }); + + let first_paths = PathList::new(&[Path::new("/project-a")]); + let second_paths = PathList::new(&[Path::new("/project-b")]); + let initial_time = Utc::now(); + let updated_time = initial_time + chrono::Duration::seconds(1); + + let initial_metadata = make_metadata( + "session-1", + "First Thread", + initial_time, + first_paths.clone(), + ); + + let second_metadata = make_metadata( + "session-2", + "Second Thread", + initial_time, + second_paths.clone(), + ); + + cx.update(|cx| { + let store = SidebarThreadMetadataStore::global(cx); + store.update(cx, |store, cx| { + store.save(initial_metadata, cx).detach(); + store.save(second_metadata, cx).detach(); + }); + }); + + cx.run_until_parked(); + + cx.update(|cx| { + let store = SidebarThreadMetadataStore::global(cx); + let store = store.read(cx); + + let first_path_entries = store + .entries_for_path(&first_paths) + .map(|entry| entry.session_id.0.to_string()) + .collect::>(); + assert_eq!(first_path_entries, vec!["session-1"]); + + let second_path_entries = store + .entries_for_path(&second_paths) + .map(|entry| entry.session_id.0.to_string()) + .collect::>(); + assert_eq!(second_path_entries, vec!["session-2"]); + }); + + let moved_metadata = make_metadata( + "session-1", + "First Thread", + updated_time, + second_paths.clone(), + ); + + cx.update(|cx| { + let store = SidebarThreadMetadataStore::global(cx); + store.update(cx, |store, cx| { + store.save(moved_metadata, cx).detach(); + }); + }); + + cx.run_until_parked(); + + cx.update(|cx| { + let store = SidebarThreadMetadataStore::global(cx); + let store = store.read(cx); + + let entry_ids = store + .entry_ids() + .map(|session_id| session_id.0.to_string()) + .collect::>(); + assert_eq!(entry_ids, vec!["session-1", "session-2"]); + + let first_path_entries = store + .entries_for_path(&first_paths) + .map(|entry| entry.session_id.0.to_string()) + .collect::>(); + assert!(first_path_entries.is_empty()); + + let second_path_entries = store + .entries_for_path(&second_paths) + .map(|entry| entry.session_id.0.to_string()) + .collect::>(); + assert_eq!(second_path_entries, vec!["session-1", "session-2"]); + }); + + cx.update(|cx| { + let store = SidebarThreadMetadataStore::global(cx); + store.update(cx, |store, cx| { + store.delete(acp::SessionId::new("session-2"), cx).detach(); + }); + }); + + cx.run_until_parked(); + + cx.update(|cx| { + let store = SidebarThreadMetadataStore::global(cx); + let store = store.read(cx); + + let entry_ids = store + .entry_ids() + .map(|session_id| session_id.0.to_string()) + .collect::>(); + assert_eq!(entry_ids, vec!["session-1"]); + + let second_path_entries = store + .entries_for_path(&second_paths) + .map(|entry| entry.session_id.0.to_string()) + .collect::>(); + assert_eq!(second_path_entries, vec!["session-1"]); + }); + } + #[gpui::test] async fn test_migrate_thread_metadata(cx: &mut TestAppContext) { cx.update(|cx| { @@ -457,13 +715,11 @@ mod tests { SidebarThreadMetadataStore::init_global(cx); }); - // Verify the list is empty before migration - let metadata_list = cx.update(|cx| { + // Verify the cache is empty before migration + let list = cx.update(|cx| { let store = SidebarThreadMetadataStore::global(cx); - store.read(cx).list(cx) + store.read(cx).entries().collect::>() }); - - let list = metadata_list.await.unwrap(); assert_eq!(list.len(), 0); let now = Utc::now(); @@ -505,12 +761,10 @@ mod tests { cx.run_until_parked(); // Verify the metadata was migrated - let metadata_list = cx.update(|cx| { + let list = cx.update(|cx| { let store = SidebarThreadMetadataStore::global(cx); - store.read(cx).list(cx) + store.read(cx).entries().collect::>() }); - - let list = metadata_list.await.unwrap(); assert_eq!(list.len(), 2); let metadata1 = list @@ -577,12 +831,10 @@ mod tests { cx.run_until_parked(); // Verify only the existing metadata is present (migration was skipped) - let metadata_list = cx.update(|cx| { + let list = cx.update(|cx| { let store = SidebarThreadMetadataStore::global(cx); - store.read(cx).list(cx) + store.read(cx).entries().collect::>() }); - - let list = metadata_list.await.unwrap(); assert_eq!(list.len(), 1); assert_eq!(list[0].session_id.0.as_ref(), "existing-session"); } @@ -650,14 +902,12 @@ mod tests { }); cx.run_until_parked(); - // List all metadata from the store. - let metadata_list = cx.update(|cx| { + // List all metadata from the store cache. + let list = cx.update(|cx| { let store = SidebarThreadMetadataStore::global(cx); - store.read(cx).list(cx) + store.read(cx).entries().collect::>() }); - let list = metadata_list.await.unwrap(); - // The subagent thread should NOT appear in the sidebar metadata. // Only the regular thread should be listed. assert_eq!( diff --git a/crates/sidebar/src/sidebar.rs b/crates/sidebar/src/sidebar.rs index 7236e1e5e85f8c1a78638d27975e9dae69b0e31d..d30b8e4ee8db38704a2a8e307edeba4b73f7f850 100644 --- a/crates/sidebar/src/sidebar.rs +++ b/crates/sidebar/src/sidebar.rs @@ -242,11 +242,9 @@ pub struct Sidebar { hovered_thread_index: Option, collapsed_groups: HashSet, expanded_groups: HashMap, - threads_by_paths: HashMap>, view: SidebarView, recent_projects_popover_handle: PopoverMenuHandle, _subscriptions: Vec, - _list_threads_task: Option>, _draft_observation: Option, } @@ -303,7 +301,7 @@ impl Sidebar { cx.observe( &SidebarThreadMetadataStore::global(cx), |this, _store, cx| { - this.list_threads(cx); + this.update_entries(cx); }, ) .detach(); @@ -321,8 +319,7 @@ impl Sidebar { this.update_entries(cx); }); - let mut this = Self { - _list_threads_task: None, + Self { multi_workspace: multi_workspace.downgrade(), width: DEFAULT_WIDTH, focus_handle, @@ -335,14 +332,11 @@ impl Sidebar { hovered_thread_index: None, collapsed_groups: HashSet::new(), expanded_groups: HashMap::new(), - threads_by_paths: HashMap::new(), view: SidebarView::default(), recent_projects_popover_handle: PopoverMenuHandle::default(), _subscriptions: Vec::new(), _draft_observation: None, - }; - this.list_threads(cx); - this + } } fn subscribe_to_workspace( @@ -687,46 +681,47 @@ impl Sidebar { if should_load_threads { let mut seen_session_ids: HashSet = HashSet::new(); - // Read threads from SidebarDb for this workspace's path list. - if let Some(rows) = self.threads_by_paths.get(&path_list) { - for row in rows { - seen_session_ids.insert(row.session_id.clone()); - let (agent, icon, icon_from_external_svg) = match &row.agent_id { - None => (Agent::NativeAgent, IconName::ZedAgent, None), - Some(id) => { - let custom_icon = agent_server_store - .as_ref() - .and_then(|store| store.read(cx).agent_icon(&id)); - ( - Agent::Custom { id: id.clone() }, - IconName::Terminal, - custom_icon, - ) - } - }; - threads.push(ThreadEntry { - agent, - session_info: acp_thread::AgentSessionInfo { - session_id: row.session_id.clone(), - work_dirs: None, - title: Some(row.title.clone()), - updated_at: Some(row.updated_at), - created_at: row.created_at, - meta: None, - }, - icon, - icon_from_external_svg, - status: AgentThreadStatus::default(), - workspace: ThreadEntryWorkspace::Open(workspace.clone()), - is_live: false, - is_background: false, - is_title_generating: false, - highlight_positions: Vec::new(), - worktree_name: None, - worktree_highlight_positions: Vec::new(), - diff_stats: DiffStats::default(), - }); - } + // Read threads from the store cache for this workspace's path list. + let thread_store = SidebarThreadMetadataStore::global(cx); + let workspace_rows: Vec<_> = + thread_store.read(cx).entries_for_path(&path_list).collect(); + for row in workspace_rows { + seen_session_ids.insert(row.session_id.clone()); + let (agent, icon, icon_from_external_svg) = match &row.agent_id { + None => (Agent::NativeAgent, IconName::ZedAgent, None), + Some(id) => { + let custom_icon = agent_server_store + .as_ref() + .and_then(|store| store.read(cx).agent_icon(&id)); + ( + Agent::Custom { id: id.clone() }, + IconName::Terminal, + custom_icon, + ) + } + }; + threads.push(ThreadEntry { + agent, + session_info: acp_thread::AgentSessionInfo { + session_id: row.session_id.clone(), + work_dirs: None, + title: Some(row.title.clone()), + updated_at: Some(row.updated_at), + created_at: row.created_at, + meta: None, + }, + icon, + icon_from_external_svg, + status: AgentThreadStatus::default(), + workspace: ThreadEntryWorkspace::Open(workspace.clone()), + is_live: false, + is_background: false, + is_title_generating: false, + highlight_positions: Vec::new(), + worktree_name: None, + worktree_highlight_positions: Vec::new(), + diff_stats: DiffStats::default(), + }); } // Load threads from linked git worktrees of this workspace's repos. @@ -767,52 +762,52 @@ impl Sidebar { None => ThreadEntryWorkspace::Closed(worktree_path_list.clone()), }; - if let Some(rows) = self.threads_by_paths.get(worktree_path_list) { - for row in rows { - if !seen_session_ids.insert(row.session_id.clone()) { - continue; - } - let (agent, icon, icon_from_external_svg) = match &row.agent_id { - None => (Agent::NativeAgent, IconName::ZedAgent, None), - Some(name) => { - let custom_icon = - agent_server_store.as_ref().and_then(|store| { - store - .read(cx) - .agent_icon(&AgentId(name.clone().into())) - }); - ( - Agent::Custom { - id: AgentId::new(name.clone()), - }, - IconName::Terminal, - custom_icon, - ) - } - }; - threads.push(ThreadEntry { - agent, - session_info: acp_thread::AgentSessionInfo { - session_id: row.session_id.clone(), - work_dirs: None, - title: Some(row.title.clone()), - updated_at: Some(row.updated_at), - created_at: row.created_at, - meta: None, - }, - icon, - icon_from_external_svg, - status: AgentThreadStatus::default(), - workspace: target_workspace.clone(), - is_live: false, - is_background: false, - is_title_generating: false, - highlight_positions: Vec::new(), - worktree_name: Some(worktree_name.clone()), - worktree_highlight_positions: Vec::new(), - diff_stats: DiffStats::default(), - }); + let worktree_rows: Vec<_> = thread_store + .read(cx) + .entries_for_path(worktree_path_list) + .collect(); + for row in worktree_rows { + if !seen_session_ids.insert(row.session_id.clone()) { + continue; } + let (agent, icon, icon_from_external_svg) = match &row.agent_id { + None => (Agent::NativeAgent, IconName::ZedAgent, None), + Some(name) => { + let custom_icon = + agent_server_store.as_ref().and_then(|store| { + store.read(cx).agent_icon(&AgentId(name.clone().into())) + }); + ( + Agent::Custom { + id: AgentId::new(name.clone()), + }, + IconName::Terminal, + custom_icon, + ) + } + }; + threads.push(ThreadEntry { + agent, + session_info: acp_thread::AgentSessionInfo { + session_id: row.session_id.clone(), + work_dirs: None, + title: Some(row.title.clone()), + updated_at: Some(row.updated_at), + created_at: row.created_at, + meta: None, + }, + icon, + icon_from_external_svg, + status: AgentThreadStatus::default(), + workspace: target_workspace.clone(), + is_live: false, + is_background: false, + is_title_generating: false, + highlight_positions: Vec::new(), + worktree_name: Some(worktree_name.clone()), + worktree_highlight_positions: Vec::new(), + diff_stats: DiffStats::default(), + }); } } } @@ -1010,30 +1005,7 @@ impl Sidebar { }; } - fn list_threads(&mut self, cx: &mut Context) { - let list_task = SidebarThreadMetadataStore::global(cx).read(cx).list(cx); - self._list_threads_task = Some(cx.spawn(async move |this, cx| { - let Some(thread_entries) = list_task.await.log_err() else { - return; - }; - this.update(cx, |this, cx| { - let mut threads_by_paths: HashMap> = HashMap::new(); - for row in thread_entries { - threads_by_paths - .entry(row.folder_paths.clone()) - .or_default() - .push(row); - } - this.threads_by_paths = threads_by_paths; - this.update_entries(cx); - }) - .ok(); - })); - } - /// Rebuilds the sidebar's visible entries from already-cached state. - /// Data fetching happens elsewhere (e.g. `list_threads`); this just - /// re-derives the entry list, list state, and notifications. fn update_entries(&mut self, cx: &mut Context) { let Some(multi_workspace) = self.multi_workspace.upgrade() else { return;