Improve thread message history (#34299)

Agus Zubiaga and Ben Brandt created

- Keep history across threads
- Reset position when edited

Release Notes:

- N/A

---------

Co-authored-by: Ben Brandt <benjamin.j.brandt@gmail.com>

Change summary

crates/agent_ui/src/acp.rs                 |  1 
crates/agent_ui/src/acp/message_history.rs | 12 ++
crates/agent_ui/src/acp/thread_view.rs     | 95 +++++++++++++----------
crates/agent_ui/src/agent_panel.rs         | 15 +++
4 files changed, 79 insertions(+), 44 deletions(-)

Detailed changes

crates/agent_ui/src/acp.rs 🔗

@@ -2,4 +2,5 @@ mod completion_provider;
 mod message_history;
 mod thread_view;
 
+pub use message_history::MessageHistory;
 pub use thread_view::AcpThreadView;

crates/agent_ui/src/acp/message_history.rs 🔗

@@ -3,19 +3,25 @@ pub struct MessageHistory<T> {
     current: Option<usize>,
 }
 
-impl<T> MessageHistory<T> {
-    pub fn new() -> Self {
+impl<T> Default for MessageHistory<T> {
+    fn default() -> Self {
         MessageHistory {
             items: Vec::new(),
             current: None,
         }
     }
+}
 
+impl<T> MessageHistory<T> {
     pub fn push(&mut self, message: T) {
         self.current.take();
         self.items.push(message);
     }
 
+    pub fn reset_position(&mut self) {
+        self.current.take();
+    }
+
     pub fn prev(&mut self) -> Option<&T> {
         if self.items.is_empty() {
             return None;
@@ -46,7 +52,7 @@ mod tests {
 
     #[test]
     fn test_prev_next() {
-        let mut history = MessageHistory::new();
+        let mut history = MessageHistory::default();
 
         // Test empty history
         assert_eq!(history.prev(), None);

crates/agent_ui/src/acp/thread_view.rs 🔗

@@ -1,3 +1,4 @@
+use std::cell::RefCell;
 use std::collections::BTreeMap;
 use std::path::Path;
 use std::rc::Rc;
@@ -53,6 +54,8 @@ pub struct AcpThreadView {
     thread_state: ThreadState,
     diff_editors: HashMap<EntityId, Entity<Editor>>,
     message_editor: Entity<Editor>,
+    message_set_from_history: bool,
+    _message_editor_subscription: Subscription,
     mention_set: Arc<Mutex<MentionSet>>,
     last_error: Option<Entity<Markdown>>,
     list_state: ListState,
@@ -60,7 +63,7 @@ pub struct AcpThreadView {
     expanded_tool_calls: HashSet<ToolCallId>,
     expanded_thinking_blocks: HashSet<(usize, usize)>,
     edits_expanded: bool,
-    message_history: MessageHistory<acp::SendUserMessageParams>,
+    message_history: Rc<RefCell<MessageHistory<acp::SendUserMessageParams>>>,
 }
 
 enum ThreadState {
@@ -81,6 +84,7 @@ impl AcpThreadView {
     pub fn new(
         workspace: WeakEntity<Workspace>,
         project: Entity<Project>,
+        message_history: Rc<RefCell<MessageHistory<acp::SendUserMessageParams>>>,
         window: &mut Window,
         cx: &mut Context<Self>,
     ) -> Self {
@@ -125,6 +129,17 @@ impl AcpThreadView {
             editor
         });
 
+        let message_editor_subscription = cx.subscribe(&message_editor, |this, _, event, _| {
+            if let editor::EditorEvent::BufferEdited = &event {
+                if !this.message_set_from_history {
+                    this.message_history.borrow_mut().reset_position();
+                }
+                this.message_set_from_history = false;
+            }
+        });
+
+        let mention_set = mention_set.clone();
+
         let list_state = ListState::new(
             0,
             gpui::ListAlignment::Bottom,
@@ -147,6 +162,8 @@ impl AcpThreadView {
             project: project.clone(),
             thread_state: Self::initial_state(workspace, project, window, cx),
             message_editor,
+            message_set_from_history: false,
+            _message_editor_subscription: message_editor_subscription,
             mention_set,
             diff_editors: Default::default(),
             list_state: list_state,
@@ -155,7 +172,7 @@ impl AcpThreadView {
             expanded_tool_calls: HashSet::default(),
             expanded_thinking_blocks: HashSet::default(),
             edits_expanded: false,
-            message_history: MessageHistory::new(),
+            message_history,
         }
     }
 
@@ -358,7 +375,7 @@ impl AcpThreadView {
             editor.remove_creases(mention_set.lock().drain(), cx)
         });
 
-        self.message_history.push(message);
+        self.message_history.borrow_mut().push(message);
     }
 
     fn previous_history_message(
@@ -367,11 +384,11 @@ impl AcpThreadView {
         window: &mut Window,
         cx: &mut Context<Self>,
     ) {
-        Self::set_draft_message(
+        self.message_set_from_history = Self::set_draft_message(
             self.message_editor.clone(),
             self.mention_set.clone(),
             self.project.clone(),
-            self.message_history.prev(),
+            self.message_history.borrow_mut().prev(),
             window,
             cx,
         );
@@ -383,43 +400,16 @@ impl AcpThreadView {
         window: &mut Window,
         cx: &mut Context<Self>,
     ) {
-        Self::set_draft_message(
+        self.message_set_from_history = Self::set_draft_message(
             self.message_editor.clone(),
             self.mention_set.clone(),
             self.project.clone(),
-            self.message_history.next(),
+            self.message_history.borrow_mut().next(),
             window,
             cx,
         );
     }
 
-    fn open_agent_diff(&mut self, _: &OpenAgentDiff, window: &mut Window, cx: &mut Context<Self>) {
-        if let Some(thread) = self.thread() {
-            AgentDiffPane::deploy(thread.clone(), self.workspace.clone(), window, cx).log_err();
-        }
-    }
-
-    fn open_edited_buffer(
-        &mut self,
-        buffer: &Entity<Buffer>,
-        window: &mut Window,
-        cx: &mut Context<Self>,
-    ) {
-        let Some(thread) = self.thread() else {
-            return;
-        };
-
-        let Some(diff) =
-            AgentDiffPane::deploy(thread.clone(), self.workspace.clone(), window, cx).log_err()
-        else {
-            return;
-        };
-
-        diff.update(cx, |diff, cx| {
-            diff.move_to_path(PathKey::for_buffer(&buffer, cx), window, cx)
-        })
-    }
-
     fn set_draft_message(
         message_editor: Entity<Editor>,
         mention_set: Arc<Mutex<MentionSet>>,
@@ -427,15 +417,11 @@ impl AcpThreadView {
         message: Option<&acp::SendUserMessageParams>,
         window: &mut Window,
         cx: &mut Context<Self>,
-    ) {
+    ) -> bool {
         cx.notify();
 
         let Some(message) = message else {
-            message_editor.update(cx, |editor, cx| {
-                editor.clear(window, cx);
-                editor.remove_creases(mention_set.lock().drain(), cx)
-            });
-            return;
+            return false;
         };
 
         let mut text = String::new();
@@ -495,6 +481,35 @@ impl AcpThreadView {
                 mention_set.lock().insert(crease_id, project_path);
             }
         }
+
+        true
+    }
+
+    fn open_agent_diff(&mut self, _: &OpenAgentDiff, window: &mut Window, cx: &mut Context<Self>) {
+        if let Some(thread) = self.thread() {
+            AgentDiffPane::deploy(thread.clone(), self.workspace.clone(), window, cx).log_err();
+        }
+    }
+
+    fn open_edited_buffer(
+        &mut self,
+        buffer: &Entity<Buffer>,
+        window: &mut Window,
+        cx: &mut Context<Self>,
+    ) {
+        let Some(thread) = self.thread() else {
+            return;
+        };
+
+        let Some(diff) =
+            AgentDiffPane::deploy(thread.clone(), self.workspace.clone(), window, cx).log_err()
+        else {
+            return;
+        };
+
+        diff.update(cx, |diff, cx| {
+            diff.move_to_path(PathKey::for_buffer(&buffer, cx), window, cx)
+        })
     }
 
     fn handle_thread_event(

crates/agent_ui/src/agent_panel.rs 🔗

@@ -1,3 +1,4 @@
+use std::cell::RefCell;
 use std::ops::Range;
 use std::path::Path;
 use std::rc::Rc;
@@ -433,6 +434,8 @@ pub struct AgentPanel {
     configuration_subscription: Option<Subscription>,
     local_timezone: UtcOffset,
     active_view: ActiveView,
+    acp_message_history:
+        Rc<RefCell<crate::acp::MessageHistory<agentic_coding_protocol::SendUserMessageParams>>>,
     previous_view: Option<ActiveView>,
     history_store: Entity<HistoryStore>,
     history: Entity<ThreadHistory>,
@@ -699,6 +702,7 @@ impl AgentPanel {
             .unwrap(),
             inline_assist_context_store,
             previous_view: None,
+            acp_message_history: Default::default(),
             history_store: history_store.clone(),
             history: cx.new(|cx| ThreadHistory::new(weak_self, history_store, window, cx)),
             hovered_recent_history_item: None,
@@ -888,10 +892,17 @@ impl AgentPanel {
     fn new_gemini_thread(&mut self, window: &mut Window, cx: &mut Context<Self>) {
         let workspace = self.workspace.clone();
         let project = self.project.clone();
+        let message_history = self.acp_message_history.clone();
 
         cx.spawn_in(window, async move |this, cx| {
             let thread_view = cx.new_window_entity(|window, cx| {
-                crate::acp::AcpThreadView::new(workspace.clone(), project, window, cx)
+                crate::acp::AcpThreadView::new(
+                    workspace.clone(),
+                    project,
+                    message_history,
+                    window,
+                    cx,
+                )
             })?;
             this.update_in(cx, |this, window, cx| {
                 this.set_active_view(
@@ -1432,6 +1443,8 @@ impl AgentPanel {
             self.active_view = new_view;
         }
 
+        self.acp_message_history.borrow_mut().reset_position();
+
         self.focus_handle(cx).focus(window);
     }