From b725beb3270b4d25fd491f963ac50c216c1d843d Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Mon, 16 Feb 2026 15:19:10 +0100 Subject: [PATCH] acp: Refresh history less often (#49263) Changes the AcpThreadHistory behavior to only load the full paginated list if we are in the history view, and only reloads the first page otherwise. And it also updates the first page in way fewer cases to reduce load on the agents. Release Notes: - N/A --- crates/agent_ui/src/acp/thread_history.rs | 349 +++++++++++++++++++++- crates/agent_ui/src/acp/thread_view.rs | 3 - crates/agent_ui/src/agent_panel.rs | 35 ++- 3 files changed, 355 insertions(+), 32 deletions(-) diff --git a/crates/agent_ui/src/acp/thread_history.rs b/crates/agent_ui/src/acp/thread_history.rs index 16108e599e31f8d56f1b04e9f2e74bebff9ecee1..76f981b8847a191d66c173df000fdbf619d62239 100644 --- a/crates/agent_ui/src/acp/thread_history.rs +++ b/crates/agent_ui/src/acp/thread_history.rs @@ -38,7 +38,8 @@ pub struct AcpThreadHistory { visible_items: Vec, local_timezone: UtcOffset, confirming_delete_history: bool, - _update_task: Task<()>, + _visible_items_task: Task<()>, + _refresh_task: Task<()>, _watch_task: Option>, _subscriptions: Vec, } @@ -111,7 +112,8 @@ impl AcpThreadHistory { search_query: SharedString::default(), confirming_delete_history: false, _subscriptions: vec![search_editor_subscription], - _update_task: Task::ready(()), + _visible_items_task: Task::ready(()), + _refresh_task: Task::ready(()), _watch_task: None, }; this.set_session_list(session_list, cx); @@ -131,7 +133,7 @@ impl AcpThreadHistory { None }; - self._update_task = cx.spawn(async move |this, cx| { + self._visible_items_task = cx.spawn(async move |this, cx| { let new_visible_items = new_list_items.await; this.update(cx, |this, cx| { let new_selected_index = if let Some(history_entry) = selected_history_entry { @@ -170,6 +172,8 @@ impl AcpThreadHistory { self.sessions.clear(); self.visible_items.clear(); self.selected_index = 0; + self._visible_items_task = Task::ready(()); + self._refresh_task = Task::ready(()); let Some(session_list) = self.session_list.as_ref() else { self._watch_task = None; @@ -179,7 +183,7 @@ impl AcpThreadHistory { let Some(rx) = session_list.watch(cx) else { // No watch support - do a one-time refresh self._watch_task = None; - self.refresh_sessions(false, cx); + self.refresh_sessions(false, false, cx); return; }; session_list.notify_refresh(); @@ -192,14 +196,13 @@ impl AcpThreadHistory { updates.push(update); } - let needs_refresh = updates - .iter() - .any(|u| matches!(u, SessionListUpdate::Refresh)); - this.update(cx, |this, cx| { - // We will refresh the whole list anyway, so no need to apply incremental updates or do several refreshes + let needs_refresh = updates + .iter() + .any(|u| matches!(u, SessionListUpdate::Refresh)); + if needs_refresh { - this.refresh_sessions(true, cx); + this.refresh_sessions(true, false, cx); } else { for update in updates { if let SessionListUpdate::SessionInfo { session_id, update } = update { @@ -213,6 +216,10 @@ impl AcpThreadHistory { })); } + pub(crate) fn refresh_full_history(&mut self, cx: &mut Context) { + self.refresh_sessions(true, true, cx); + } + fn apply_info_update( &mut self, session_id: acp::SessionId, @@ -254,13 +261,21 @@ impl AcpThreadHistory { self.update_visible_items(true, cx); } - fn refresh_sessions(&mut self, preserve_selected_item: bool, cx: &mut Context) { + fn refresh_sessions( + &mut self, + preserve_selected_item: bool, + load_all_pages: bool, + cx: &mut Context, + ) { let Some(session_list) = self.session_list.clone() else { self.update_visible_items(preserve_selected_item, cx); return; }; - self._update_task = cx.spawn(async move |this, cx| { + // If a new refresh arrives while pagination is in progress, the previous + // `_refresh_task` is cancelled. This is intentional (latest refresh wins), + // but means sessions may be in a partial state until the new refresh completes. + self._refresh_task = cx.spawn(async move |this, cx| { let mut cursor: Option = None; let mut is_first_page = true; @@ -295,6 +310,10 @@ impl AcpThreadHistory { .ok(); is_first_page = false; + if !load_all_pages { + break; + } + match next_cursor { Some(next_cursor) => { if cursor.as_ref() == Some(&next_cursor) { @@ -1050,7 +1069,10 @@ mod tests { use acp_thread::AgentSessionListResponse; use chrono::NaiveDate; use gpui::TestAppContext; - use std::any::Any; + use std::{ + any::Any, + sync::{Arc, Mutex}, + }; fn init_test(cx: &mut TestAppContext) { cx.update(|cx| { @@ -1104,6 +1126,307 @@ mod tests { } } + #[derive(Clone)] + struct PaginatedTestSessionList { + first_page_sessions: Vec, + second_page_sessions: Vec, + requested_cursors: Arc>>>, + async_responses: bool, + updates_tx: smol::channel::Sender, + updates_rx: smol::channel::Receiver, + } + + impl PaginatedTestSessionList { + fn new( + first_page_sessions: Vec, + second_page_sessions: Vec, + ) -> Self { + let (tx, rx) = smol::channel::unbounded(); + Self { + first_page_sessions, + second_page_sessions, + requested_cursors: Arc::new(Mutex::new(Vec::new())), + async_responses: false, + updates_tx: tx, + updates_rx: rx, + } + } + + fn with_async_responses(mut self) -> Self { + self.async_responses = true; + self + } + + fn requested_cursors(&self) -> Vec> { + self.requested_cursors.lock().unwrap().clone() + } + + fn clear_requested_cursors(&self) { + self.requested_cursors.lock().unwrap().clear() + } + + fn send_update(&self, update: SessionListUpdate) { + self.updates_tx.try_send(update).ok(); + } + } + + impl AgentSessionList for PaginatedTestSessionList { + fn list_sessions( + &self, + request: AgentSessionListRequest, + cx: &mut App, + ) -> Task> { + let requested_cursors = self.requested_cursors.clone(); + let first_page_sessions = self.first_page_sessions.clone(); + let second_page_sessions = self.second_page_sessions.clone(); + + let respond = move || { + requested_cursors + .lock() + .unwrap() + .push(request.cursor.clone()); + + match request.cursor.as_deref() { + None => AgentSessionListResponse { + sessions: first_page_sessions, + next_cursor: Some("page-2".to_string()), + meta: None, + }, + Some("page-2") => AgentSessionListResponse::new(second_page_sessions), + _ => AgentSessionListResponse::new(Vec::new()), + } + }; + + if self.async_responses { + cx.foreground_executor().spawn(async move { + smol::future::yield_now().await; + Ok(respond()) + }) + } else { + Task::ready(Ok(respond())) + } + } + + fn watch(&self, _cx: &mut App) -> Option> { + Some(self.updates_rx.clone()) + } + + fn notify_refresh(&self) { + self.send_update(SessionListUpdate::Refresh); + } + + fn into_any(self: Rc) -> Rc { + self + } + } + + fn test_session(session_id: &str, title: &str) -> AgentSessionInfo { + AgentSessionInfo { + session_id: acp::SessionId::new(session_id), + cwd: None, + title: Some(title.to_string().into()), + updated_at: None, + meta: None, + } + } + + #[gpui::test] + async fn test_refresh_only_loads_first_page_by_default(cx: &mut TestAppContext) { + init_test(cx); + + let session_list = Rc::new(PaginatedTestSessionList::new( + vec![test_session("session-1", "First")], + vec![test_session("session-2", "Second")], + )); + + let (history, cx) = cx.add_window_view(|window, cx| { + AcpThreadHistory::new(Some(session_list.clone()), window, cx) + }); + cx.run_until_parked(); + + history.update(cx, |history, _cx| { + assert_eq!(history.sessions.len(), 1); + assert_eq!( + history.sessions[0].session_id, + acp::SessionId::new("session-1") + ); + }); + assert_eq!(session_list.requested_cursors(), vec![None]); + } + + #[gpui::test] + async fn test_enabling_full_pagination_loads_all_pages(cx: &mut TestAppContext) { + init_test(cx); + + let session_list = Rc::new(PaginatedTestSessionList::new( + vec![test_session("session-1", "First")], + vec![test_session("session-2", "Second")], + )); + + let (history, cx) = cx.add_window_view(|window, cx| { + AcpThreadHistory::new(Some(session_list.clone()), window, cx) + }); + cx.run_until_parked(); + session_list.clear_requested_cursors(); + + history.update(cx, |history, cx| history.refresh_full_history(cx)); + cx.run_until_parked(); + + history.update(cx, |history, _cx| { + assert_eq!(history.sessions.len(), 2); + assert_eq!( + history.sessions[0].session_id, + acp::SessionId::new("session-1") + ); + assert_eq!( + history.sessions[1].session_id, + acp::SessionId::new("session-2") + ); + }); + assert_eq!( + session_list.requested_cursors(), + vec![None, Some("page-2".to_string())] + ); + } + + #[gpui::test] + async fn test_standard_refresh_replaces_with_first_page_after_full_history_refresh( + cx: &mut TestAppContext, + ) { + init_test(cx); + + let session_list = Rc::new(PaginatedTestSessionList::new( + vec![test_session("session-1", "First")], + vec![test_session("session-2", "Second")], + )); + + let (history, cx) = cx.add_window_view(|window, cx| { + AcpThreadHistory::new(Some(session_list.clone()), window, cx) + }); + cx.run_until_parked(); + + history.update(cx, |history, cx| history.refresh_full_history(cx)); + cx.run_until_parked(); + session_list.clear_requested_cursors(); + + history.update(cx, |history, cx| { + history.refresh(cx); + }); + cx.run_until_parked(); + + history.update(cx, |history, _cx| { + assert_eq!(history.sessions.len(), 1); + assert_eq!( + history.sessions[0].session_id, + acp::SessionId::new("session-1") + ); + }); + assert_eq!(session_list.requested_cursors(), vec![None]); + } + + #[gpui::test] + async fn test_re_entering_full_pagination_reloads_all_pages(cx: &mut TestAppContext) { + init_test(cx); + + let session_list = Rc::new(PaginatedTestSessionList::new( + vec![test_session("session-1", "First")], + vec![test_session("session-2", "Second")], + )); + + let (history, cx) = cx.add_window_view(|window, cx| { + AcpThreadHistory::new(Some(session_list.clone()), window, cx) + }); + cx.run_until_parked(); + + history.update(cx, |history, cx| history.refresh_full_history(cx)); + cx.run_until_parked(); + session_list.clear_requested_cursors(); + + history.update(cx, |history, cx| history.refresh_full_history(cx)); + cx.run_until_parked(); + + history.update(cx, |history, _cx| { + assert_eq!(history.sessions.len(), 2); + }); + assert_eq!( + session_list.requested_cursors(), + vec![None, Some("page-2".to_string())] + ); + } + + #[gpui::test] + async fn test_partial_refresh_batch_drops_non_first_page_sessions(cx: &mut TestAppContext) { + init_test(cx); + + let second_page_session_id = acp::SessionId::new("session-2"); + let session_list = Rc::new(PaginatedTestSessionList::new( + vec![test_session("session-1", "First")], + vec![test_session("session-2", "Second")], + )); + + let (history, cx) = cx.add_window_view(|window, cx| { + AcpThreadHistory::new(Some(session_list.clone()), window, cx) + }); + cx.run_until_parked(); + + history.update(cx, |history, cx| history.refresh_full_history(cx)); + cx.run_until_parked(); + + session_list.clear_requested_cursors(); + + session_list.send_update(SessionListUpdate::SessionInfo { + session_id: second_page_session_id.clone(), + update: acp::SessionInfoUpdate::new().title("Updated Second"), + }); + session_list.send_update(SessionListUpdate::Refresh); + cx.run_until_parked(); + + history.update(cx, |history, _cx| { + assert_eq!(history.sessions.len(), 1); + assert_eq!( + history.sessions[0].session_id, + acp::SessionId::new("session-1") + ); + assert!( + history + .sessions + .iter() + .all(|session| session.session_id != second_page_session_id) + ); + }); + assert_eq!(session_list.requested_cursors(), vec![None]); + } + + #[gpui::test] + async fn test_full_pagination_works_with_async_page_fetches(cx: &mut TestAppContext) { + init_test(cx); + + let session_list = Rc::new( + PaginatedTestSessionList::new( + vec![test_session("session-1", "First")], + vec![test_session("session-2", "Second")], + ) + .with_async_responses(), + ); + + let (history, cx) = cx.add_window_view(|window, cx| { + AcpThreadHistory::new(Some(session_list.clone()), window, cx) + }); + cx.run_until_parked(); + session_list.clear_requested_cursors(); + + history.update(cx, |history, cx| history.refresh_full_history(cx)); + cx.run_until_parked(); + + history.update(cx, |history, _cx| { + assert_eq!(history.sessions.len(), 2); + }); + assert_eq!( + session_list.requested_cursors(), + vec![None, Some("page-2".to_string())] + ); + } + #[gpui::test] async fn test_apply_info_update_title(cx: &mut TestAppContext) { init_test(cx); diff --git a/crates/agent_ui/src/acp/thread_view.rs b/crates/agent_ui/src/acp/thread_view.rs index 6e7ff8486bda34cb017b98374acabc81b1f614fc..e939f053314582a42d6a009c155b243ad235802a 100644 --- a/crates/agent_ui/src/acp/thread_view.rs +++ b/crates/agent_ui/src/acp/thread_view.rs @@ -1106,8 +1106,6 @@ impl AcpServerView { if should_send_queued { self.send_queued_message_at_index(0, false, window, cx); } - - self.history.update(cx, |history, cx| history.refresh(cx)); } AcpThreadEvent::Refusal => { let error = ThreadError::Refusal; @@ -1162,7 +1160,6 @@ impl AcpServerView { } }); } - self.history.update(cx, |history, cx| history.refresh(cx)); } AcpThreadEvent::PromptCapabilitiesUpdated => { if let Some(active) = self.thread_view(&thread_id) { diff --git a/crates/agent_ui/src/agent_panel.rs b/crates/agent_ui/src/agent_panel.rs index 91b8f29638f12aaef4f7da22646b8d84dd84657a..35cc8103acab805b878e5545d7191149a69b7a63 100644 --- a/crates/agent_ui/src/agent_panel.rs +++ b/crates/agent_ui/src/agent_panel.rs @@ -1124,22 +1124,7 @@ impl AgentPanel { match self.active_view { ActiveView::Configuration | ActiveView::History { .. } => { if let Some(previous_view) = self.previous_view.take() { - self.active_view = previous_view; - cx.emit(AgentPanelEvent::ActiveViewChanged); - - match &self.active_view { - ActiveView::AgentThread { thread_view } => { - thread_view.focus_handle(cx).focus(window, cx); - } - ActiveView::TextThread { - text_thread_editor, .. - } => { - text_thread_editor.focus_handle(cx).focus(window, cx); - } - ActiveView::Uninitialized - | ActiveView::History { .. } - | ActiveView::Configuration => {} - } + self.set_active_view(previous_view, true, window, cx); } cx.notify(); } @@ -1561,6 +1546,12 @@ impl AgentPanel { window: &mut Window, cx: &mut Context, ) { + let was_in_agent_history = matches!( + self.active_view, + ActiveView::History { + kind: HistoryKind::AgentThreads + } + ); let current_is_uninitialized = matches!(self.active_view, ActiveView::Uninitialized); let current_is_history = matches!(self.active_view, ActiveView::History { .. }); let new_is_history = matches!(new_view, ActiveView::History { .. }); @@ -1593,6 +1584,18 @@ impl AgentPanel { _ => None, }; + let is_in_agent_history = matches!( + self.active_view, + ActiveView::History { + kind: HistoryKind::AgentThreads + } + ); + + if !was_in_agent_history && is_in_agent_history { + self.acp_history + .update(cx, |history, cx| history.refresh_full_history(cx)); + } + if focus { self.focus_handle(cx).focus(window, cx); }