acp: Hide loading diff animation for external agents and update in place (#36699)

The loading diff animation can be jarring for external agents because
they stream the diff at the same time the tool call is pushed, so it's
only displayed while we're asynchronously calculating the diff. We'll
now only show it for the native agent.

Also, we'll now only update the diff when it changes, which avoids
unnecessarily hiding it for a few frames.

Release Notes:

- N/A

Co-authored-by: Bennet Bo Fenner <bennetbo@gmx.de>

crates/acp_thread/src/acp_thread.rs

@@ -238,10 +238,21 @@ impl ToolCall {
         }
 
         if let Some(content) = content {
-            self.content = content
-                .into_iter()
-                .map(|chunk| ToolCallContent::from_acp(chunk, language_registry.clone(), cx))
-                .collect();
+            let new_content_len = content.len();
+            let mut content = content.into_iter();
+
+            // Reuse existing content if we can
+            for (old, new) in self.content.iter_mut().zip(content.by_ref()) {
+                old.update_from_acp(new, language_registry.clone(), cx);
+            }
+            for new in content {
+                self.content.push(ToolCallContent::from_acp(
+                    new,
+                    language_registry.clone(),
+                    cx,
+                ))
+            }
+            self.content.truncate(new_content_len);
         }
 
         if let Some(locations) = locations {
@@ -551,6 +562,28 @@ impl ToolCallContent {
         }
     }
 
+    pub fn update_from_acp(
+        &mut self,
+        new: acp::ToolCallContent,
+        language_registry: Arc<LanguageRegistry>,
+        cx: &mut App,
+    ) {
+        let needs_update = match (&self, &new) {
+            (Self::Diff(old_diff), acp::ToolCallContent::Diff { diff: new_diff }) => {
+                old_diff.read(cx).needs_update(
+                    new_diff.old_text.as_deref().unwrap_or(""),
+                    &new_diff.new_text,
+                    cx,
+                )
+            }
+            _ => true,
+        };
+
+        if needs_update {
+            *self = Self::from_acp(new, language_registry, cx);
+        }
+    }
+
     pub fn to_markdown(&self, cx: &App) -> String {
         match self {
             Self::ContentBlock(content) => content.to_markdown(cx).to_string(),

crates/acp_thread/src/diff.rs

@@ -28,10 +28,12 @@ impl Diff {
         cx: &mut Context<Self>,
     ) -> Self {
         let multibuffer = cx.new(|_cx| MultiBuffer::without_headers(Capability::ReadOnly));
-        let buffer = cx.new(|cx| Buffer::local(new_text, cx));
+        let new_buffer = cx.new(|cx| Buffer::local(new_text, cx));
+        let base_text = old_text.clone().unwrap_or(String::new()).into();
         let task = cx.spawn({
             let multibuffer = multibuffer.clone();
             let path = path.clone();
+            let buffer = new_buffer.clone();
             async move |_, cx| {
                 let language = language_registry
                     .language_for_file_path(&path)
@@ -76,6 +78,8 @@ impl Diff {
         Self::Finalized(FinalizedDiff {
             multibuffer,
             path,
+            base_text,
+            new_buffer,
             _update_diff: task,
         })
     }
@@ -119,7 +123,7 @@ impl Diff {
                     diff.update(cx);
                 }
             }),
-            buffer,
+            new_buffer: buffer,
             diff: buffer_diff,
             revealed_ranges: Vec::new(),
             update_diff: Task::ready(Ok(())),
@@ -154,9 +158,9 @@ impl Diff {
             .map(|buffer| buffer.read(cx).text())
             .join("\n");
         let path = match self {
-            Diff::Pending(PendingDiff { buffer, .. }) => {
-                buffer.read(cx).file().map(|file| file.path().as_ref())
-            }
+            Diff::Pending(PendingDiff {
+                new_buffer: buffer, ..
+            }) => buffer.read(cx).file().map(|file| file.path().as_ref()),
             Diff::Finalized(FinalizedDiff { path, .. }) => Some(path.as_path()),
         };
         format!(
@@ -169,12 +173,33 @@ impl Diff {
     pub fn has_revealed_range(&self, cx: &App) -> bool {
         self.multibuffer().read(cx).excerpt_paths().next().is_some()
     }
+
+    pub fn needs_update(&self, old_text: &str, new_text: &str, cx: &App) -> bool {
+        match self {
+            Diff::Pending(PendingDiff {
+                base_text,
+                new_buffer,
+                ..
+            }) => {
+                base_text.as_str() != old_text
+                    || !new_buffer.read(cx).as_rope().chunks().equals_str(new_text)
+            }
+            Diff::Finalized(FinalizedDiff {
+                base_text,
+                new_buffer,
+                ..
+            }) => {
+                base_text.as_str() != old_text
+                    || !new_buffer.read(cx).as_rope().chunks().equals_str(new_text)
+            }
+        }
+    }
 }
 
 pub struct PendingDiff {
     multibuffer: Entity<MultiBuffer>,
     base_text: Arc<String>,
-    buffer: Entity<Buffer>,
+    new_buffer: Entity<Buffer>,
     diff: Entity<BufferDiff>,
     revealed_ranges: Vec<Range<Anchor>>,
     _subscription: Subscription,
@@ -183,7 +208,7 @@ pub struct PendingDiff {
 
 impl PendingDiff {
     pub fn update(&mut self, cx: &mut Context<Diff>) {
-        let buffer = self.buffer.clone();
+        let buffer = self.new_buffer.clone();
         let buffer_diff = self.diff.clone();
         let base_text = self.base_text.clone();
         self.update_diff = cx.spawn(async move |diff, cx| {
@@ -221,10 +246,10 @@ impl PendingDiff {
     fn finalize(&self, cx: &mut Context<Diff>) -> FinalizedDiff {
         let ranges = self.excerpt_ranges(cx);
         let base_text = self.base_text.clone();
-        let language_registry = self.buffer.read(cx).language_registry();
+        let language_registry = self.new_buffer.read(cx).language_registry();
 
         let path = self
-            .buffer
+            .new_buffer
             .read(cx)
             .file()
             .map(|file| file.path().as_ref())
@@ -233,12 +258,12 @@ impl PendingDiff {
 
         // Replace the buffer in the multibuffer with the snapshot
         let buffer = cx.new(|cx| {
-            let language = self.buffer.read(cx).language().cloned();
+            let language = self.new_buffer.read(cx).language().cloned();
             let buffer = TextBuffer::new_normalized(
                 0,
                 cx.entity_id().as_non_zero_u64().into(),
-                self.buffer.read(cx).line_ending(),
-                self.buffer.read(cx).as_rope().clone(),
+                self.new_buffer.read(cx).line_ending(),
+                self.new_buffer.read(cx).as_rope().clone(),
             );
             let mut buffer = Buffer::build(buffer, None, Capability::ReadWrite);
             buffer.set_language(language, cx);
@@ -274,7 +299,9 @@ impl PendingDiff {
 
         FinalizedDiff {
             path,
+            base_text: self.base_text.clone(),
             multibuffer: self.multibuffer.clone(),
+            new_buffer: self.new_buffer.clone(),
             _update_diff: update_diff,
         }
     }
@@ -283,8 +310,8 @@ impl PendingDiff {
         let ranges = self.excerpt_ranges(cx);
         self.multibuffer.update(cx, |multibuffer, cx| {
             multibuffer.set_excerpts_for_path(
-                PathKey::for_buffer(&self.buffer, cx),
-                self.buffer.clone(),
+                PathKey::for_buffer(&self.new_buffer, cx),
+                self.new_buffer.clone(),
                 ranges,
                 editor::DEFAULT_MULTIBUFFER_CONTEXT,
                 cx,
@@ -296,7 +323,7 @@ impl PendingDiff {
     }
 
     fn excerpt_ranges(&self, cx: &App) -> Vec<Range<Point>> {
-        let buffer = self.buffer.read(cx);
+        let buffer = self.new_buffer.read(cx);
         let diff = self.diff.read(cx);
         let mut ranges = diff
             .hunks_intersecting_range(Anchor::MIN..Anchor::MAX, buffer, cx)
@@ -330,6 +357,8 @@ impl PendingDiff {
 
 pub struct FinalizedDiff {
     path: PathBuf,
+    base_text: Arc<String>,
+    new_buffer: Entity<Buffer>,
     multibuffer: Entity<MultiBuffer>,
     _update_diff: Task<Result<()>>,
 }

crates/agent_ui/src/acp/thread_view.rs

@@ -1625,7 +1625,9 @@ impl AcpThreadView {
                         .into_any()
                 }
                 ToolCallStatus::Pending | ToolCallStatus::InProgress
-                    if is_edit && tool_call.content.is_empty() =>
+                    if is_edit
+                        && tool_call.content.is_empty()
+                        && self.as_native_connection(cx).is_some() =>
                 {
                     self.render_diff_loading(cx).into_any()
                 }
@@ -1981,7 +1983,7 @@ impl AcpThreadView {
                     && diff.read(cx).has_revealed_range(cx)
                 {
                     editor.into_any_element()
-                } else if tool_progress {
+                } else if tool_progress && self.as_native_connection(cx).is_some() {
                     self.render_diff_loading(cx)
                 } else {
                     Empty.into_any()