acp: Eagerly load all kinds of mentions (#36741)

This PR makes it so that all kinds of @-mentions start loading their
context as soon as they are confirmed. Previously, we were waiting to
load the context for file, symbol, selection, and rule mentions until
the user's message was sent. By kicking off loading immediately for
these kinds of context, we can support adding selections from unsaved
buffers, and we make the semantics of @-mentions more consistent.

Loading all kinds of context eagerly also makes it possible to simplify
the structure of the MentionSet and the code around it. Now MentionSet
is just a single hash map, all the management of creases happens in a
uniform way in `MessageEditor::confirm_completion`, and the helper
methods for loading different kinds of context are much more focused and
orthogonal.

Release Notes:

- N/A

---------

Co-authored-by: Conrad <[email protected]>

Author: 2 people

crates/acp_thread/src/mention.rs

@@ -5,7 +5,7 @@ use prompt_store::{PromptId, UserPromptId};
 use serde::{Deserialize, Serialize};
 use std::{
     fmt,
-    ops::Range,
+    ops::RangeInclusive,
     path::{Path, PathBuf},
     str::FromStr,
 };
@@ -17,13 +17,14 @@ pub enum MentionUri {
     File {
         abs_path: PathBuf,
     },
+    PastedImage,
     Directory {
         abs_path: PathBuf,
     },
     Symbol {
-        path: PathBuf,
+        abs_path: PathBuf,
         name: String,
-        line_range: Range<u32>,
+        line_range: RangeInclusive<u32>,
     },
     Thread {
         id: acp::SessionId,
@@ -38,8 +39,9 @@ pub enum MentionUri {
         name: String,
     },
     Selection {
-        path: PathBuf,
-        line_range: Range<u32>,
+        #[serde(default, skip_serializing_if = "Option::is_none")]
+        abs_path: Option<PathBuf>,
+        line_range: RangeInclusive<u32>,
     },
     Fetch {
         url: Url,
@@ -48,36 +50,44 @@ pub enum MentionUri {
 
 impl MentionUri {
     pub fn parse(input: &str) -> Result<Self> {
+        fn parse_line_range(fragment: &str) -> Result<RangeInclusive<u32>> {
+            let range = fragment
+                .strip_prefix("L")
+                .context("Line range must start with \"L\"")?;
+            let (start, end) = range
+                .split_once(":")
+                .context("Line range must use colon as separator")?;
+            let range = start
+                .parse::<u32>()
+                .context("Parsing line range start")?
+                .checked_sub(1)
+                .context("Line numbers should be 1-based")?
+                ..=end
+                    .parse::<u32>()
+                    .context("Parsing line range end")?
+                    .checked_sub(1)
+                    .context("Line numbers should be 1-based")?;
+            Ok(range)
+        }
+
         let url = url::Url::parse(input)?;
         let path = url.path();
         match url.scheme() {
             "file" => {
                 let path = url.to_file_path().ok().context("Extracting file path")?;
                 if let Some(fragment) = url.fragment() {
-                    let range = fragment
-                        .strip_prefix("L")
-                        .context("Line range must start with \"L\"")?;
-                    let (start, end) = range
-                        .split_once(":")
-                        .context("Line range must use colon as separator")?;
-                    let line_range = start
-                        .parse::<u32>()
-                        .context("Parsing line range start")?
-                        .checked_sub(1)
-                        .context("Line numbers should be 1-based")?
-                        ..end
-                            .parse::<u32>()
-                            .context("Parsing line range end")?
-                            .checked_sub(1)
-                            .context("Line numbers should be 1-based")?;
+                    let line_range = parse_line_range(fragment)?;
                     if let Some(name) = single_query_param(&url, "symbol")? {
                         Ok(Self::Symbol {
                             name,
-                            path,
+                            abs_path: path,
                             line_range,
                         })
                     } else {
-                        Ok(Self::Selection { path, line_range })
+                        Ok(Self::Selection {
+                            abs_path: Some(path),
+                            line_range,
+                        })
                     }
                 } else if input.ends_with("/") {
                     Ok(Self::Directory { abs_path: path })
@@ -105,6 +115,17 @@ impl MentionUri {
                         id: rule_id.into(),
                         name,
                     })
+                } else if path.starts_with("/agent/pasted-image") {
+                    Ok(Self::PastedImage)
+                } else if path.starts_with("/agent/untitled-buffer") {
+                    let fragment = url
+                        .fragment()
+                        .context("Missing fragment for untitled buffer selection")?;
+                    let line_range = parse_line_range(fragment)?;
+                    Ok(Self::Selection {
+                        abs_path: None,
+                        line_range,
+                    })
                 } else {
                     bail!("invalid zed url: {:?}", input);
                 }
@@ -121,13 +142,16 @@ impl MentionUri {
                 .unwrap_or_default()
                 .to_string_lossy()
                 .into_owned(),
+            MentionUri::PastedImage => "Image".to_string(),
             MentionUri::Symbol { name, .. } => name.clone(),
             MentionUri::Thread { name, .. } => name.clone(),
             MentionUri::TextThread { name, .. } => name.clone(),
             MentionUri::Rule { name, .. } => name.clone(),
             MentionUri::Selection {
-                path, line_range, ..
-            } => selection_name(path, line_range),
+                abs_path: path,
+                line_range,
+                ..
+            } => selection_name(path.as_deref(), line_range),
             MentionUri::Fetch { url } => url.to_string(),
         }
     }
@@ -137,6 +161,7 @@ impl MentionUri {
             MentionUri::File { abs_path } => {
                 FileIcons::get_icon(abs_path, cx).unwrap_or_else(|| IconName::File.path().into())
             }
+            MentionUri::PastedImage => IconName::Image.path().into(),
             MentionUri::Directory { .. } => FileIcons::get_folder_icon(false, cx)
                 .unwrap_or_else(|| IconName::Folder.path().into()),
             MentionUri::Symbol { .. } => IconName::Code.path().into(),
@@ -157,29 +182,40 @@ impl MentionUri {
             MentionUri::File { abs_path } => {
                 Url::from_file_path(abs_path).expect("mention path should be absolute")
             }
+            MentionUri::PastedImage => Url::parse("zed:///agent/pasted-image").unwrap(),
             MentionUri::Directory { abs_path } => {
                 Url::from_directory_path(abs_path).expect("mention path should be absolute")
             }
             MentionUri::Symbol {
-                path,
+                abs_path,
                 name,
                 line_range,
             } => {
-                let mut url = Url::from_file_path(path).expect("mention path should be absolute");
+                let mut url =
+                    Url::from_file_path(abs_path).expect("mention path should be absolute");
                 url.query_pairs_mut().append_pair("symbol", name);
                 url.set_fragment(Some(&format!(
                     "L{}:{}",
-                    line_range.start + 1,
-                    line_range.end + 1
+                    line_range.start() + 1,
+                    line_range.end() + 1
                 )));
                 url
             }
-            MentionUri::Selection { path, line_range } => {
-                let mut url = Url::from_file_path(path).expect("mention path should be absolute");
+            MentionUri::Selection {
+                abs_path: path,
+                line_range,
+            } => {
+                let mut url = if let Some(path) = path {
+                    Url::from_file_path(path).expect("mention path should be absolute")
+                } else {
+                    let mut url = Url::parse("zed:///").unwrap();
+                    url.set_path("/agent/untitled-buffer");
+                    url
+                };
                 url.set_fragment(Some(&format!(
                     "L{}:{}",
-                    line_range.start + 1,
-                    line_range.end + 1
+                    line_range.start() + 1,
+                    line_range.end() + 1
                 )));
                 url
             }
@@ -191,7 +227,10 @@ impl MentionUri {
             }
             MentionUri::TextThread { path, name } => {
                 let mut url = Url::parse("zed:///").unwrap();
-                url.set_path(&format!("/agent/text-thread/{}", path.to_string_lossy()));
+                url.set_path(&format!(
+                    "/agent/text-thread/{}",
+                    path.to_string_lossy().trim_start_matches('/')
+                ));
                 url.query_pairs_mut().append_pair("name", name);
                 url
             }
@@ -237,12 +276,14 @@ fn single_query_param(url: &Url, name: &'static str) -> Result<Option<String>> {
     }
 }
 
-pub fn selection_name(path: &Path, line_range: &Range<u32>) -> String {
+pub fn selection_name(path: Option<&Path>, line_range: &RangeInclusive<u32>) -> String {
     format!(
         "{} ({}:{})",
-        path.file_name().unwrap_or_default().display(),
-        line_range.start + 1,
-        line_range.end + 1
+        path.and_then(|path| path.file_name())
+            .unwrap_or("Untitled".as_ref())
+            .display(),
+        *line_range.start() + 1,
+        *line_range.end() + 1
     )
 }
 
@@ -302,14 +343,14 @@ mod tests {
         let parsed = MentionUri::parse(symbol_uri).unwrap();
         match &parsed {
             MentionUri::Symbol {
-                path,
+                abs_path: path,
                 name,
                 line_range,
             } => {
                 assert_eq!(path.to_str().unwrap(), path!("/path/to/file.rs"));
                 assert_eq!(name, "MySymbol");
-                assert_eq!(line_range.start, 9);
-                assert_eq!(line_range.end, 19);
+                assert_eq!(line_range.start(), &9);
+                assert_eq!(line_range.end(), &19);
             }
             _ => panic!("Expected Symbol variant"),
         }
@@ -321,16 +362,39 @@ mod tests {
         let selection_uri = uri!("file:///path/to/file.rs#L5:15");
         let parsed = MentionUri::parse(selection_uri).unwrap();
         match &parsed {
-            MentionUri::Selection { path, line_range } => {
-                assert_eq!(path.to_str().unwrap(), path!("/path/to/file.rs"));
-                assert_eq!(line_range.start, 4);
-                assert_eq!(line_range.end, 14);
+            MentionUri::Selection {
+                abs_path: path,
+                line_range,
+            } => {
+                assert_eq!(
+                    path.as_ref().unwrap().to_str().unwrap(),
+                    path!("/path/to/file.rs")
+                );
+                assert_eq!(line_range.start(), &4);
+                assert_eq!(line_range.end(), &14);
             }
             _ => panic!("Expected Selection variant"),
         }
         assert_eq!(parsed.to_uri().to_string(), selection_uri);
     }
 
+    #[test]
+    fn test_parse_untitled_selection_uri() {
+        let selection_uri = uri!("zed:///agent/untitled-buffer#L1:10");
+        let parsed = MentionUri::parse(selection_uri).unwrap();
+        match &parsed {
+            MentionUri::Selection {
+                abs_path: None,
+                line_range,
+            } => {
+                assert_eq!(line_range.start(), &0);
+                assert_eq!(line_range.end(), &9);
+            }
+            _ => panic!("Expected Selection variant without path"),
+        }
+        assert_eq!(parsed.to_uri().to_string(), selection_uri);
+    }
+
     #[test]
     fn test_parse_thread_uri() {
         let thread_uri = "zed:///agent/thread/session123?name=Thread+name";

crates/agent/src/thread_store.rs

@@ -893,8 +893,19 @@ impl ThreadsDatabase {
 
         let needs_migration_from_heed = mdb_path.exists();
 
-        let connection = if *ZED_STATELESS || cfg!(any(feature = "test-support", test)) {
+        let connection = if *ZED_STATELESS {
             Connection::open_memory(Some("THREAD_FALLBACK_DB"))
+        } else if cfg!(any(feature = "test-support", test)) {
+            // rust stores the name of the test on the current thread.
+            // We use this to automatically create a database that will
+            // be shared within the test (for the test_retrieve_old_thread)
+            // but not with concurrent tests.
+            let thread = std::thread::current();
+            let test_name = thread.name();
+            Connection::open_memory(Some(&format!(
+                "THREAD_FALLBACK_{}",
+                test_name.unwrap_or_default()
+            )))
         } else {
             Connection::open_file(&sqlite_path.to_string_lossy())
         };

crates/agent2/src/db.rs

@@ -266,8 +266,19 @@ impl ThreadsDatabase {
     }
 
     pub fn new(executor: BackgroundExecutor) -> Result<Self> {
-        let connection = if *ZED_STATELESS || cfg!(any(feature = "test-support", test)) {
+        let connection = if *ZED_STATELESS {
             Connection::open_memory(Some("THREAD_FALLBACK_DB"))
+        } else if cfg!(any(feature = "test-support", test)) {
+            // rust stores the name of the test on the current thread.
+            // We use this to automatically create a database that will
+            // be shared within the test (for the test_retrieve_old_thread)
+            // but not with concurrent tests.
+            let thread = std::thread::current();
+            let test_name = thread.name();
+            Connection::open_memory(Some(&format!(
+                "THREAD_FALLBACK_{}",
+                test_name.unwrap_or_default()
+            )))
         } else {
             let threads_dir = paths::data_dir().join("threads");
             std::fs::create_dir_all(&threads_dir)?;