No longer instantiate recently opened agent threads on startup (#32285)

Michael Sloan created

This was causing a lot of work on startup, particularly due to
instantiating edit tool cards. The minor downside is that now these
threads don't open quite as fast.

Includes a few other improvements:

* On text thread rename, now immediately updates the metadata for
display in the UI instead of waiting for reload.

* On text thread rename, first renames the file before writing. Before
if the file removal failed you'd end up with a duplicate.

* Now only stores text thread file names instead of full paths. This is
more concise and allows for the app data dir changing location.

* Renames `ThreadStore::unordered_threads` to
`ThreadStore::reverse_chronological_threads` (and removes the old one
that sorted), since the recent change to use a SQL database queries them
in that order.

* Removes `ContextStore::reverse_chronological_contexts` since it was
only used in one location where it does sorting anyway - no need to sort
twice.

* `SavedContextMetadata::title` is now `SharedString` instead of
`String`.

Release Notes:

- Fixed regression in startup performance by not deserializing and
instantiating recently opened agent threads.

Change summary

crates/agent/src/agent_panel.rs                          | 198 +++---
crates/agent/src/context_picker/thread_context_picker.rs |  23 
crates/agent/src/history_store.rs                        | 273 +++++----
crates/agent/src/thread_history.rs                       |   2 
crates/agent/src/thread_store.rs                         |   9 
crates/assistant_context_editor/src/context.rs           |  36 
crates/assistant_context_editor/src/context_editor.rs    |   1 
crates/assistant_context_editor/src/context_store.rs     |  18 
8 files changed, 299 insertions(+), 261 deletions(-)

Detailed changes

crates/agent/src/agent_panel.rs 🔗

@@ -57,7 +57,7 @@ use zed_llm_client::{CompletionIntent, UsageLimit};
 use crate::active_thread::{self, ActiveThread, ActiveThreadEvent};
 use crate::agent_configuration::{AgentConfiguration, AssistantConfigurationEvent};
 use crate::agent_diff::AgentDiff;
-use crate::history_store::{HistoryStore, RecentEntry};
+use crate::history_store::{HistoryEntryId, HistoryStore};
 use crate::message_editor::{MessageEditor, MessageEditorEvent};
 use crate::thread::{Thread, ThreadError, ThreadId, ThreadSummary, TokenUsageRatio};
 use crate::thread_history::{HistoryEntryElement, ThreadHistory};
@@ -257,6 +257,7 @@ impl ActiveView {
 
     pub fn prompt_editor(
         context_editor: Entity<ContextEditor>,
+        history_store: Entity<HistoryStore>,
         language_registry: Arc<LanguageRegistry>,
         window: &mut Window,
         cx: &mut App,
@@ -322,6 +323,19 @@ impl ActiveView {
                             editor.set_text(summary, window, cx);
                         })
                     }
+                    ContextEvent::PathChanged { old_path, new_path } => {
+                        history_store.update(cx, |history_store, cx| {
+                            if let Some(old_path) = old_path {
+                                history_store
+                                    .replace_recently_opened_text_thread(old_path, new_path, cx);
+                            } else {
+                                history_store.push_recently_opened_entry(
+                                    HistoryEntryId::Context(new_path.clone()),
+                                    cx,
+                                );
+                            }
+                        });
+                    }
                     _ => {}
                 }
             }),
@@ -516,8 +530,7 @@ impl AgentPanel {
             HistoryStore::new(
                 thread_store.clone(),
                 context_store.clone(),
-                [RecentEntry::Thread(thread_id, thread.clone())],
-                window,
+                [HistoryEntryId::Thread(thread_id)],
                 cx,
             )
         });
@@ -544,7 +557,13 @@ impl AgentPanel {
                     editor.insert_default_prompt(window, cx);
                     editor
                 });
-                ActiveView::prompt_editor(context_editor, language_registry.clone(), window, cx)
+                ActiveView::prompt_editor(
+                    context_editor,
+                    history_store.clone(),
+                    language_registry.clone(),
+                    window,
+                    cx,
+                )
             }
         };
 
@@ -581,86 +600,9 @@ impl AgentPanel {
             let panel = weak_panel.clone();
             let assistant_navigation_menu =
                 ContextMenu::build_persistent(window, cx, move |mut menu, _window, cx| {
-                    let recently_opened = panel
-                        .update(cx, |this, cx| {
-                            this.history_store.update(cx, |history_store, cx| {
-                                history_store.recently_opened_entries(cx)
-                            })
-                        })
-                        .unwrap_or_default();
-
-                    if !recently_opened.is_empty() {
-                        menu = menu.header("Recently Opened");
-
-                        for entry in recently_opened.iter() {
-                            if let RecentEntry::Context(context) = entry {
-                                if context.read(cx).path().is_none() {
-                                    log::error!(
-                                        "bug: text thread in recent history list was never saved"
-                                    );
-                                    continue;
-                                }
-                            }
-
-                            let summary = entry.summary(cx);
-
-                            menu = menu.entry_with_end_slot_on_hover(
-                                summary,
-                                None,
-                                {
-                                    let panel = panel.clone();
-                                    let entry = entry.clone();
-                                    move |window, cx| {
-                                        panel
-                                            .update(cx, {
-                                                let entry = entry.clone();
-                                                move |this, cx| match entry {
-                                                    RecentEntry::Thread(_, thread) => {
-                                                        this.open_thread(thread, window, cx)
-                                                    }
-                                                    RecentEntry::Context(context) => {
-                                                        let Some(path) = context.read(cx).path()
-                                                        else {
-                                                            return;
-                                                        };
-                                                        this.open_saved_prompt_editor(
-                                                            path.clone(),
-                                                            window,
-                                                            cx,
-                                                        )
-                                                        .detach_and_log_err(cx)
-                                                    }
-                                                }
-                                            })
-                                            .ok();
-                                    }
-                                },
-                                IconName::Close,
-                                "Close Entry".into(),
-                                {
-                                    let panel = panel.clone();
-                                    let entry = entry.clone();
-                                    move |_window, cx| {
-                                        panel
-                                            .update(cx, |this, cx| {
-                                                this.history_store.update(
-                                                    cx,
-                                                    |history_store, cx| {
-                                                        history_store.remove_recently_opened_entry(
-                                                            &entry, cx,
-                                                        );
-                                                    },
-                                                );
-                                            })
-                                            .ok();
-                                    }
-                                },
-                            );
-                        }
-
-                        menu = menu.separator();
+                    if let Some(panel) = panel.upgrade() {
+                        menu = Self::populate_recently_opened_menu_section(menu, panel, cx);
                     }
-
                     menu.action("View All", Box::new(OpenHistory))
                         .end_slot_action(DeleteRecentlyOpenThread.boxed_clone())
                         .fixed_width(px(320.).into())
@@ -898,6 +840,7 @@ impl AgentPanel {
         self.set_active_view(
             ActiveView::prompt_editor(
                 context_editor.clone(),
+                self.history_store.clone(),
                 self.language_registry.clone(),
                 window,
                 cx,
@@ -984,7 +927,13 @@ impl AgentPanel {
             )
         });
         self.set_active_view(
-            ActiveView::prompt_editor(editor.clone(), self.language_registry.clone(), window, cx),
+            ActiveView::prompt_editor(
+                editor.clone(),
+                self.history_store.clone(),
+                self.language_registry.clone(),
+                window,
+                cx,
+            ),
             window,
             cx,
         );
@@ -1383,16 +1332,6 @@ impl AgentPanel {
                     }
                 }
             }
-            ActiveView::TextThread { context_editor, .. } => {
-                let context = context_editor.read(cx).context();
-                // When switching away from an unsaved text thread, delete its entry.
-                if context.read(cx).path().is_none() {
-                    let context = context.clone();
-                    self.history_store.update(cx, |store, cx| {
-                        store.remove_recently_opened_entry(&RecentEntry::Context(context), cx);
-                    });
-                }
-            }
             _ => {}
         }
 
@@ -1400,13 +1339,14 @@ impl AgentPanel {
             ActiveView::Thread { thread, .. } => self.history_store.update(cx, |store, cx| {
                 if let Some(thread) = thread.upgrade() {
                     let id = thread.read(cx).id().clone();
-                    store.push_recently_opened_entry(RecentEntry::Thread(id, thread), cx);
+                    store.push_recently_opened_entry(HistoryEntryId::Thread(id), cx);
                 }
             }),
             ActiveView::TextThread { context_editor, .. } => {
                 self.history_store.update(cx, |store, cx| {
-                    let context = context_editor.read(cx).context().clone();
-                    store.push_recently_opened_entry(RecentEntry::Context(context), cx)
+                    if let Some(path) = context_editor.read(cx).context().read(cx).path() {
+                        store.push_recently_opened_entry(HistoryEntryId::Context(path.clone()), cx)
+                    }
                 })
             }
             _ => {}
@@ -1425,6 +1365,70 @@ impl AgentPanel {
 
         self.focus_handle(cx).focus(window);
     }
+
+    fn populate_recently_opened_menu_section(
+        mut menu: ContextMenu,
+        panel: Entity<Self>,
+        cx: &mut Context<ContextMenu>,
+    ) -> ContextMenu {
+        let entries = panel
+            .read(cx)
+            .history_store
+            .read(cx)
+            .recently_opened_entries(cx);
+
+        if entries.is_empty() {
+            return menu;
+        }
+
+        menu = menu.header("Recently Opened");
+
+        for entry in entries {
+            let title = entry.title().clone();
+            let id = entry.id();
+
+            menu = menu.entry_with_end_slot_on_hover(
+                title,
+                None,
+                {
+                    let panel = panel.downgrade();
+                    let id = id.clone();
+                    move |window, cx| {
+                        let id = id.clone();
+                        panel
+                            .update(cx, move |this, cx| match id {
+                                HistoryEntryId::Thread(id) => this
+                                    .open_thread_by_id(&id, window, cx)
+                                    .detach_and_log_err(cx),
+                                HistoryEntryId::Context(path) => this
+                                    .open_saved_prompt_editor(path.clone(), window, cx)
+                                    .detach_and_log_err(cx),
+                            })
+                            .ok();
+                    }
+                },
+                IconName::Close,
+                "Close Entry".into(),
+                {
+                    let panel = panel.downgrade();
+                    let id = id.clone();
+                    move |_window, cx| {
+                        panel
+                            .update(cx, |this, cx| {
+                                this.history_store.update(cx, |history_store, cx| {
+                                    history_store.remove_recently_opened_entry(&id, cx);
+                                });
+                            })
+                            .ok();
+                    }
+                },
+            );
+        }
+
+        menu = menu.separator();
+
+        menu
+    }
 }
 
 impl Focusable for AgentPanel {

crates/agent/src/context_picker/thread_context_picker.rs 🔗

@@ -282,15 +282,18 @@ pub fn unordered_thread_entries(
     text_thread_store: Entity<TextThreadStore>,
     cx: &App,
 ) -> impl Iterator<Item = (DateTime<Utc>, ThreadContextEntry)> {
-    let threads = thread_store.read(cx).unordered_threads().map(|thread| {
-        (
-            thread.updated_at,
-            ThreadContextEntry::Thread {
-                id: thread.id.clone(),
-                title: thread.summary.clone(),
-            },
-        )
-    });
+    let threads = thread_store
+        .read(cx)
+        .reverse_chronological_threads()
+        .map(|thread| {
+            (
+                thread.updated_at,
+                ThreadContextEntry::Thread {
+                    id: thread.id.clone(),
+                    title: thread.summary.clone(),
+                },
+            )
+        });
 
     let text_threads = text_thread_store
         .read(cx)
@@ -300,7 +303,7 @@ pub fn unordered_thread_entries(
                 context.mtime.to_utc(),
                 ThreadContextEntry::Context {
                     path: context.path.clone(),
-                    title: context.title.clone().into(),
+                    title: context.title.clone(),
                 },
             )
         });

crates/agent/src/history_store.rs 🔗

@@ -1,18 +1,17 @@
 use std::{collections::VecDeque, path::Path, sync::Arc};
 
-use anyhow::Context as _;
-use assistant_context_editor::{AssistantContext, SavedContextMetadata};
+use anyhow::{Context as _, Result};
+use assistant_context_editor::SavedContextMetadata;
 use chrono::{DateTime, Utc};
-use futures::future::{TryFutureExt as _, join_all};
-use gpui::{Entity, Task, prelude::*};
+use gpui::{AsyncApp, Entity, SharedString, Task, prelude::*};
+use itertools::Itertools;
+use paths::contexts_dir;
 use serde::{Deserialize, Serialize};
-use smol::future::FutureExt;
 use std::time::Duration;
-use ui::{App, SharedString, Window};
+use ui::App;
 use util::ResultExt as _;
 
 use crate::{
-    Thread,
     thread::ThreadId,
     thread_store::{SerializedThreadMetadata, ThreadStore},
 };
@@ -41,52 +40,34 @@ impl HistoryEntry {
             HistoryEntry::Context(context) => HistoryEntryId::Context(context.path.clone()),
         }
     }
+
+    pub fn title(&self) -> &SharedString {
+        match self {
+            HistoryEntry::Thread(thread) => &thread.summary,
+            HistoryEntry::Context(context) => &context.title,
+        }
+    }
 }
 
 /// Generic identifier for a history entry.
-#[derive(Clone, PartialEq, Eq)]
+#[derive(Clone, PartialEq, Eq, Debug)]
 pub enum HistoryEntryId {
     Thread(ThreadId),
     Context(Arc<Path>),
 }
 
-#[derive(Clone, Debug)]
-pub(crate) enum RecentEntry {
-    Thread(ThreadId, Entity<Thread>),
-    Context(Entity<AssistantContext>),
-}
-
-impl PartialEq for RecentEntry {
-    fn eq(&self, other: &Self) -> bool {
-        match (self, other) {
-            (Self::Thread(l0, _), Self::Thread(r0, _)) => l0 == r0,
-            (Self::Context(l0), Self::Context(r0)) => l0 == r0,
-            _ => false,
-        }
-    }
-}
-
-impl Eq for RecentEntry {}
-
-impl RecentEntry {
-    pub(crate) fn summary(&self, cx: &App) -> SharedString {
-        match self {
-            RecentEntry::Thread(_, thread) => thread.read(cx).summary().or_default(),
-            RecentEntry::Context(context) => context.read(cx).summary().or_default(),
-        }
-    }
-}
-
 #[derive(Serialize, Deserialize)]
-enum SerializedRecentEntry {
+enum SerializedRecentOpen {
     Thread(String),
+    ContextName(String),
+    /// Old format which stores the full path
     Context(String),
 }
 
 pub struct HistoryStore {
     thread_store: Entity<ThreadStore>,
     context_store: Entity<assistant_context_editor::ContextStore>,
-    recently_opened_entries: VecDeque<RecentEntry>,
+    recently_opened_entries: VecDeque<HistoryEntryId>,
     _subscriptions: Vec<gpui::Subscription>,
     _save_recently_opened_entries_task: Task<()>,
 }
@@ -95,8 +76,7 @@ impl HistoryStore {
     pub fn new(
         thread_store: Entity<ThreadStore>,
         context_store: Entity<assistant_context_editor::ContextStore>,
-        initial_recent_entries: impl IntoIterator<Item = RecentEntry>,
-        window: &mut Window,
+        initial_recent_entries: impl IntoIterator<Item = HistoryEntryId>,
         cx: &mut Context<Self>,
     ) -> Self {
         let subscriptions = vec![
@@ -104,68 +84,20 @@ impl HistoryStore {
             cx.observe(&context_store, |_, _, cx| cx.notify()),
         ];
 
-        window
-            .spawn(cx, {
-                let thread_store = thread_store.downgrade();
-                let context_store = context_store.downgrade();
-                let this = cx.weak_entity();
-                async move |cx| {
-                    let path = paths::data_dir().join(NAVIGATION_HISTORY_PATH);
-                    let contents = cx
-                        .background_spawn(async move { std::fs::read_to_string(path) })
-                        .await
-                        .ok()?;
-                    let entries = serde_json::from_str::<Vec<SerializedRecentEntry>>(&contents)
-                        .context("deserializing persisted agent panel navigation history")
-                        .log_err()?
-                        .into_iter()
-                        .take(MAX_RECENTLY_OPENED_ENTRIES)
-                        .map(|serialized| match serialized {
-                            SerializedRecentEntry::Thread(id) => thread_store
-                                .update_in(cx, |thread_store, window, cx| {
-                                    let thread_id = ThreadId::from(id.as_str());
-                                    thread_store
-                                        .open_thread(&thread_id, window, cx)
-                                        .map_ok(|thread| RecentEntry::Thread(thread_id, thread))
-                                        .boxed()
-                                })
-                                .unwrap_or_else(|_| {
-                                    async {
-                                        anyhow::bail!("no thread store");
-                                    }
-                                    .boxed()
-                                }),
-                            SerializedRecentEntry::Context(id) => context_store
-                                .update(cx, |context_store, cx| {
-                                    context_store
-                                        .open_local_context(Path::new(&id).into(), cx)
-                                        .map_ok(RecentEntry::Context)
-                                        .boxed()
-                                })
-                                .unwrap_or_else(|_| {
-                                    async {
-                                        anyhow::bail!("no context store");
-                                    }
-                                    .boxed()
-                                }),
-                        });
-                    let entries = join_all(entries)
-                        .await
-                        .into_iter()
-                        .filter_map(|result| result.log_with_level(log::Level::Debug))
-                        .collect::<VecDeque<_>>();
-
-                    this.update(cx, |this, _| {
-                        this.recently_opened_entries.extend(entries);
-                        this.recently_opened_entries
-                            .truncate(MAX_RECENTLY_OPENED_ENTRIES);
-                    })
-                    .ok();
-
-                    Some(())
-                }
+        cx.spawn(async move |this, cx| {
+            let entries = Self::load_recently_opened_entries(cx).await.log_err()?;
+            this.update(cx, |this, _| {
+                this.recently_opened_entries
+                    .extend(
+                        entries.into_iter().take(
+                            MAX_RECENTLY_OPENED_ENTRIES
+                                .saturating_sub(this.recently_opened_entries.len()),
+                        ),
+                    );
             })
-            .detach();
+            .ok()
+        })
+        .detach();
 
         Self {
             thread_store,
@@ -184,19 +116,20 @@ impl HistoryStore {
             return history_entries;
         }
 
-        for thread in self
-            .thread_store
-            .update(cx, |this, _cx| this.reverse_chronological_threads())
-        {
-            history_entries.push(HistoryEntry::Thread(thread));
-        }
-
-        for context in self
-            .context_store
-            .update(cx, |this, _cx| this.reverse_chronological_contexts())
-        {
-            history_entries.push(HistoryEntry::Context(context));
-        }
+        history_entries.extend(
+            self.thread_store
+                .read(cx)
+                .reverse_chronological_threads()
+                .cloned()
+                .map(HistoryEntry::Thread),
+        );
+        history_entries.extend(
+            self.context_store
+                .read(cx)
+                .unordered_contexts()
+                .cloned()
+                .map(HistoryEntry::Context),
+        );
 
         history_entries.sort_unstable_by_key(|entry| std::cmp::Reverse(entry.updated_at()));
         history_entries
@@ -206,15 +139,62 @@ impl HistoryStore {
         self.entries(cx).into_iter().take(limit).collect()
     }
 
+    pub fn recently_opened_entries(&self, cx: &App) -> Vec<HistoryEntry> {
+        #[cfg(debug_assertions)]
+        if std::env::var("ZED_SIMULATE_NO_THREAD_HISTORY").is_ok() {
+            return Vec::new();
+        }
+
+        let thread_entries = self
+            .thread_store
+            .read(cx)
+            .reverse_chronological_threads()
+            .flat_map(|thread| {
+                self.recently_opened_entries
+                    .iter()
+                    .enumerate()
+                    .flat_map(|(index, entry)| match entry {
+                        HistoryEntryId::Thread(id) if &thread.id == id => {
+                            Some((index, HistoryEntry::Thread(thread.clone())))
+                        }
+                        _ => None,
+                    })
+            });
+
+        let context_entries =
+            self.context_store
+                .read(cx)
+                .unordered_contexts()
+                .flat_map(|context| {
+                    self.recently_opened_entries
+                        .iter()
+                        .enumerate()
+                        .flat_map(|(index, entry)| match entry {
+                            HistoryEntryId::Context(path) if &context.path == path => {
+                                Some((index, HistoryEntry::Context(context.clone())))
+                            }
+                            _ => None,
+                        })
+                });
+
+        thread_entries
+            .chain(context_entries)
+            // optimization to halt iteration early
+            .take(self.recently_opened_entries.len())
+            .sorted_unstable_by_key(|(index, _)| *index)
+            .map(|(_, entry)| entry)
+            .collect()
+    }
+
     fn save_recently_opened_entries(&mut self, cx: &mut Context<Self>) {
         let serialized_entries = self
             .recently_opened_entries
             .iter()
             .filter_map(|entry| match entry {
-                RecentEntry::Context(context) => Some(SerializedRecentEntry::Context(
-                    context.read(cx).path()?.to_str()?.to_owned(),
-                )),
-                RecentEntry::Thread(id, _) => Some(SerializedRecentEntry::Thread(id.to_string())),
+                HistoryEntryId::Context(path) => path.file_name().map(|file| {
+                    SerializedRecentOpen::ContextName(file.to_string_lossy().to_string())
+                }),
+                HistoryEntryId::Thread(id) => Some(SerializedRecentOpen::Thread(id.to_string())),
             })
             .collect::<Vec<_>>();
 
@@ -233,7 +213,33 @@ impl HistoryStore {
         });
     }
 
-    pub fn push_recently_opened_entry(&mut self, entry: RecentEntry, cx: &mut Context<Self>) {
+    fn load_recently_opened_entries(cx: &AsyncApp) -> Task<Result<Vec<HistoryEntryId>>> {
+        cx.background_spawn(async move {
+            let path = paths::data_dir().join(NAVIGATION_HISTORY_PATH);
+            let contents = smol::fs::read_to_string(path).await?;
+            let entries = serde_json::from_str::<Vec<SerializedRecentOpen>>(&contents)
+                .context("deserializing persisted agent panel navigation history")?
+                .into_iter()
+                .take(MAX_RECENTLY_OPENED_ENTRIES)
+                .flat_map(|entry| match entry {
+                    SerializedRecentOpen::Thread(id) => {
+                        Some(HistoryEntryId::Thread(id.as_str().into()))
+                    }
+                    SerializedRecentOpen::ContextName(file_name) => Some(HistoryEntryId::Context(
+                        contexts_dir().join(file_name).into(),
+                    )),
+                    SerializedRecentOpen::Context(path) => {
+                        Path::new(&path).file_name().map(|file_name| {
+                            HistoryEntryId::Context(contexts_dir().join(file_name).into())
+                        })
+                    }
+                })
+                .collect::<Vec<_>>();
+            Ok(entries)
+        })
+    }
+
+    pub fn push_recently_opened_entry(&mut self, entry: HistoryEntryId, cx: &mut Context<Self>) {
         self.recently_opened_entries
             .retain(|old_entry| old_entry != &entry);
         self.recently_opened_entries.push_front(entry);
@@ -244,24 +250,33 @@ impl HistoryStore {
 
     pub fn remove_recently_opened_thread(&mut self, id: ThreadId, cx: &mut Context<Self>) {
         self.recently_opened_entries.retain(|entry| match entry {
-            RecentEntry::Thread(thread_id, _) if thread_id == &id => false,
+            HistoryEntryId::Thread(thread_id) if thread_id == &id => false,
             _ => true,
         });
         self.save_recently_opened_entries(cx);
     }
 
-    pub fn remove_recently_opened_entry(&mut self, entry: &RecentEntry, cx: &mut Context<Self>) {
-        self.recently_opened_entries
-            .retain(|old_entry| old_entry != entry);
+    pub fn replace_recently_opened_text_thread(
+        &mut self,
+        old_path: &Path,
+        new_path: &Arc<Path>,
+        cx: &mut Context<Self>,
+    ) {
+        for entry in &mut self.recently_opened_entries {
+            match entry {
+                HistoryEntryId::Context(path) if path.as_ref() == old_path => {
+                    *entry = HistoryEntryId::Context(new_path.clone());
+                    break;
+                }
+                _ => {}
+            }
+        }
         self.save_recently_opened_entries(cx);
     }
 
-    pub fn recently_opened_entries(&self, _cx: &mut Context<Self>) -> VecDeque<RecentEntry> {
-        #[cfg(debug_assertions)]
-        if std::env::var("ZED_SIMULATE_NO_THREAD_HISTORY").is_ok() {
-            return VecDeque::new();
-        }
-
-        self.recently_opened_entries.clone()
+    pub fn remove_recently_opened_entry(&mut self, entry: &HistoryEntryId, cx: &mut Context<Self>) {
+        self.recently_opened_entries
+            .retain(|old_entry| old_entry != entry);
+        self.save_recently_opened_entries(cx);
     }
 }

crates/agent/src/thread_history.rs 🔗

@@ -671,7 +671,7 @@ impl RenderOnce for HistoryEntryElement {
             ),
             HistoryEntry::Context(context) => (
                 context.path.to_string_lossy().to_string(),
-                context.title.clone().into(),
+                context.title.clone(),
                 context.mtime.timestamp(),
             ),
         };

crates/agent/src/thread_store.rs 🔗

@@ -393,16 +393,11 @@ impl ThreadStore {
         self.threads.len()
     }
 
-    pub fn unordered_threads(&self) -> impl Iterator<Item = &SerializedThreadMetadata> {
+    pub fn reverse_chronological_threads(&self) -> impl Iterator<Item = &SerializedThreadMetadata> {
+        // ordering is from "ORDER BY" in `list_threads`
         self.threads.iter()
     }
 
-    pub fn reverse_chronological_threads(&self) -> Vec<SerializedThreadMetadata> {
-        let mut threads = self.threads.iter().cloned().collect::<Vec<_>>();
-        threads.sort_unstable_by_key(|thread| std::cmp::Reverse(thread.updated_at));
-        threads
-    }
-
     pub fn create_thread(&mut self, cx: &mut Context<Self>) -> Entity<Thread> {
         cx.new(|cx| {
             Thread::new(

crates/assistant_context_editor/src/context.rs 🔗

@@ -11,7 +11,7 @@ use assistant_slash_commands::FileCommandMetadata;
 use client::{self, proto, telemetry::Telemetry};
 use clock::ReplicaId;
 use collections::{HashMap, HashSet};
-use fs::{Fs, RemoveOptions};
+use fs::{Fs, RenameOptions};
 use futures::{FutureExt, StreamExt, future::Shared};
 use gpui::{
     App, AppContext as _, Context, Entity, EventEmitter, RenderImage, SharedString, Subscription,
@@ -452,6 +452,10 @@ pub enum ContextEvent {
     MessagesEdited,
     SummaryChanged,
     SummaryGenerated,
+    PathChanged {
+        old_path: Option<Arc<Path>>,
+        new_path: Arc<Path>,
+    },
     StreamedCompletion,
     StartedThoughtProcess(Range<language::Anchor>),
     EndedThoughtProcess(language::Anchor),
@@ -2894,22 +2898,34 @@ impl AssistantContext {
                 }
 
                 fs.create_dir(contexts_dir().as_ref()).await?;
-                fs.atomic_write(new_path.clone(), serde_json::to_string(&context).unwrap())
-                    .await?;
-                if let Some(old_path) = old_path {
+
+                // rename before write ensures that only one file exists
+                if let Some(old_path) = old_path.as_ref() {
                     if new_path.as_path() != old_path.as_ref() {
-                        fs.remove_file(
+                        fs.rename(
                             &old_path,
-                            RemoveOptions {
-                                recursive: false,
-                                ignore_if_not_exists: true,
+                            &new_path,
+                            RenameOptions {
+                                overwrite: true,
+                                ignore_if_exists: true,
                             },
                         )
                         .await?;
                     }
                 }
 
-                this.update(cx, |this, _| this.path = Some(new_path.into()))?;
+                // update path before write in case it fails
+                this.update(cx, {
+                    let new_path: Arc<Path> = new_path.clone().into();
+                    move |this, cx| {
+                        this.path = Some(new_path.clone());
+                        cx.emit(ContextEvent::PathChanged { old_path, new_path });
+                    }
+                })
+                .ok();
+
+                fs.atomic_write(new_path, serde_json::to_string(&context).unwrap())
+                    .await?;
             }
 
             Ok(())
@@ -3277,7 +3293,7 @@ impl SavedContextV0_1_0 {
 
 #[derive(Debug, Clone)]
 pub struct SavedContextMetadata {
-    pub title: String,
+    pub title: SharedString,
     pub path: Arc<Path>,
     pub mtime: chrono::DateTime<chrono::Local>,
 }

crates/assistant_context_editor/src/context_editor.rs 🔗

@@ -580,6 +580,7 @@ impl ContextEditor {
                 });
             }
             ContextEvent::SummaryGenerated => {}
+            ContextEvent::PathChanged { .. } => {}
             ContextEvent::StartedThoughtProcess(range) => {
                 let creases = self.insert_thought_process_output_sections(
                     [(

crates/assistant_context_editor/src/context_store.rs 🔗

@@ -347,12 +347,6 @@ impl ContextStore {
         self.contexts_metadata.iter()
     }
 
-    pub fn reverse_chronological_contexts(&self) -> Vec<SavedContextMetadata> {
-        let mut contexts = self.contexts_metadata.iter().cloned().collect::<Vec<_>>();
-        contexts.sort_unstable_by_key(|thread| std::cmp::Reverse(thread.mtime));
-        contexts
-    }
-
     pub fn create(&mut self, cx: &mut Context<Self>) -> Entity<AssistantContext> {
         let context = cx.new(|cx| {
             AssistantContext::local(
@@ -618,6 +612,16 @@ impl ContextStore {
             ContextEvent::SummaryChanged => {
                 self.advertise_contexts(cx);
             }
+            ContextEvent::PathChanged { old_path, new_path } => {
+                if let Some(old_path) = old_path.as_ref() {
+                    for metadata in &mut self.contexts_metadata {
+                        if &metadata.path == old_path {
+                            metadata.path = new_path.clone();
+                            break;
+                        }
+                    }
+                }
+            }
             ContextEvent::Operation(operation) => {
                 let context_id = context.read(cx).id().to_proto();
                 let operation = operation.to_proto();
@@ -792,7 +796,7 @@ impl ContextStore {
                         .next()
                     {
                         contexts.push(SavedContextMetadata {
-                            title: title.to_string(),
+                            title: title.to_string().into(),
                             path: path.into(),
                             mtime: metadata.mtime.timestamp_for_user().into(),
                         });