From f937c1931fe382ad08f0d96b529f8a0428166d7c Mon Sep 17 00:00:00 2001 From: Bennet Bo Fenner Date: Thu, 18 Dec 2025 17:21:41 +0100 Subject: [PATCH] rules_library: Only store built-in prompts when they are customized (#45112) Follow up to #45004 Release Notes: - N/A --- Cargo.lock | 2 + crates/agent/src/agent.rs | 2 +- crates/agent_ui/src/completion_provider.rs | 2 +- crates/git_ui/src/git_panel.rs | 13 +- crates/prompt_store/Cargo.toml | 5 + crates/prompt_store/src/prompt_store.rs | 257 ++++++++++++++++++--- crates/rules_library/src/rules_library.rs | 47 +--- 7 files changed, 249 insertions(+), 79 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1ec640d49c2135d35442f0bf23047be7991427eb..0d83b2b9b912ab112d9b38fd1ef1d5ff21f9049c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -12648,6 +12648,8 @@ dependencies = [ "paths", "rope", "serde", + "strum 0.27.2", + "tempfile", "text", "util", "uuid", diff --git a/crates/agent/src/agent.rs b/crates/agent/src/agent.rs index 5e16f74682ef95a4e990ed5a124a0d6031acfb0e..43ed3b90f3556eb24e45440a7fe0038e7a1b9535 100644 --- a/crates/agent/src/agent.rs +++ b/crates/agent/src/agent.rs @@ -426,7 +426,7 @@ impl NativeAgent { .into_iter() .flat_map(|(contents, prompt_metadata)| match contents { Ok(contents) => Some(UserRulesContext { - uuid: prompt_metadata.id.user_id()?, + uuid: prompt_metadata.id.as_user()?, title: prompt_metadata.title.map(|title| title.to_string()), contents, }), diff --git a/crates/agent_ui/src/completion_provider.rs b/crates/agent_ui/src/completion_provider.rs index 206a2b3282b5471e8d5e8d18788519c3853dca55..a7b955b81ef3a7edccca98f15fa73bb40787a2c9 100644 --- a/crates/agent_ui/src/completion_provider.rs +++ b/crates/agent_ui/src/completion_provider.rs @@ -1586,7 +1586,7 @@ pub(crate) fn search_rules( None } else { Some(RulesContextEntry { - prompt_id: metadata.id.user_id()?, + prompt_id: metadata.id.as_user()?, title: metadata.title?, }) } diff --git a/crates/git_ui/src/git_panel.rs b/crates/git_ui/src/git_panel.rs index 4e94a811510ee07707bf729040d41fc8b1eb922c..0f967e68d1fab829fb37b626c23ecfebe69fb5dd 100644 --- a/crates/git_ui/src/git_panel.rs +++ b/crates/git_ui/src/git_panel.rs @@ -58,7 +58,7 @@ use project::{ git_store::{GitStoreEvent, Repository, RepositoryEvent, RepositoryId, pending_op}, project_settings::{GitPathStyle, ProjectSettings}, }; -use prompt_store::{PromptId, PromptStore, RULES_FILE_NAMES}; +use prompt_store::{BuiltInPrompt, PromptId, PromptStore, RULES_FILE_NAMES}; use serde::{Deserialize, Serialize}; use settings::{Settings, SettingsStore, StatusStyle}; use std::future::Future; @@ -2579,25 +2579,26 @@ impl GitPanel { is_using_legacy_zed_pro: bool, cx: &mut AsyncApp, ) -> String { - const DEFAULT_PROMPT: &str = include_str!("commit_message_prompt.txt"); - // Remove this once we stop supporting legacy Zed Pro // In legacy Zed Pro, Git commit summary generation did not count as a // prompt. If the user changes the prompt, our classification will fail, // meaning that users will be charged for generating commit messages. if is_using_legacy_zed_pro { - return DEFAULT_PROMPT.to_string(); + return BuiltInPrompt::CommitMessage.default_content().to_string(); } let load = async { let store = cx.update(|cx| PromptStore::global(cx)).ok()?.await.ok()?; store - .update(cx, |s, cx| s.load(PromptId::CommitMessage, cx)) + .update(cx, |s, cx| { + s.load(PromptId::BuiltIn(BuiltInPrompt::CommitMessage), cx) + }) .ok()? .await .ok() }; - load.await.unwrap_or_else(|| DEFAULT_PROMPT.to_string()) + load.await + .unwrap_or_else(|| BuiltInPrompt::CommitMessage.default_content().to_string()) } /// Generates a commit message using an LLM. diff --git a/crates/prompt_store/Cargo.toml b/crates/prompt_store/Cargo.toml index 13bacbfad3bf2b5deb4a20af866f37dad47288ff..a7df9d13ee82da62838175029b9bdfd7c9375508 100644 --- a/crates/prompt_store/Cargo.toml +++ b/crates/prompt_store/Cargo.toml @@ -28,6 +28,11 @@ parking_lot.workspace = true paths.workspace = true rope.workspace = true serde.workspace = true +strum.workspace = true text.workspace = true util.workspace = true uuid.workspace = true + +[dev-dependencies] +gpui = { workspace = true, features = ["test-support"] } +tempfile.workspace = true diff --git a/crates/prompt_store/src/prompt_store.rs b/crates/prompt_store/src/prompt_store.rs index 7823f7a6957caf282f4ad7f1d6f884971364518e..2c45410c2aa172c8a4f7118a914cacca69ea7ca8 100644 --- a/crates/prompt_store/src/prompt_store.rs +++ b/crates/prompt_store/src/prompt_store.rs @@ -1,6 +1,6 @@ mod prompts; -use anyhow::{Context as _, Result, anyhow}; +use anyhow::{Result, anyhow}; use chrono::{DateTime, Utc}; use collections::HashMap; use futures::FutureExt as _; @@ -23,6 +23,7 @@ use std::{ path::PathBuf, sync::{Arc, atomic::AtomicBool}, }; +use strum::{EnumIter, IntoEnumIterator as _}; use text::LineEnding; use util::ResultExt; use uuid::Uuid; @@ -51,11 +52,51 @@ pub struct PromptMetadata { pub saved_at: DateTime, } +impl PromptMetadata { + fn builtin(builtin: BuiltInPrompt) -> Self { + Self { + id: PromptId::BuiltIn(builtin), + title: Some(builtin.title().into()), + default: false, + saved_at: DateTime::default(), + } + } +} + +/// Built-in prompts that have default content and can be customized by users. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Deserialize, EnumIter)] +pub enum BuiltInPrompt { + CommitMessage, +} + +impl BuiltInPrompt { + pub fn title(&self) -> &'static str { + match self { + Self::CommitMessage => "Commit message", + } + } + + /// Returns the default content for this built-in prompt. + pub fn default_content(&self) -> &'static str { + match self { + Self::CommitMessage => include_str!("../../git_ui/src/commit_message_prompt.txt"), + } + } +} + +impl std::fmt::Display for BuiltInPrompt { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::CommitMessage => write!(f, "Commit message"), + } + } +} + #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] #[serde(tag = "kind")] pub enum PromptId { User { uuid: UserPromptId }, - CommitMessage, + BuiltIn(BuiltInPrompt), } impl PromptId { @@ -63,31 +104,37 @@ impl PromptId { UserPromptId::new().into() } - pub fn user_id(&self) -> Option { + pub fn as_user(&self) -> Option { match self { Self::User { uuid } => Some(*uuid), - _ => None, + Self::BuiltIn { .. } => None, } } - pub fn is_built_in(&self) -> bool { + pub fn as_built_in(&self) -> Option { match self { - Self::User { .. } => false, - Self::CommitMessage => true, + Self::User { .. } => None, + Self::BuiltIn(builtin) => Some(*builtin), } } + pub fn is_built_in(&self) -> bool { + matches!(self, Self::BuiltIn { .. }) + } + pub fn can_edit(&self) -> bool { match self { - Self::User { .. } | Self::CommitMessage => true, + Self::User { .. } => true, + Self::BuiltIn(builtin) => match builtin { + BuiltInPrompt::CommitMessage => true, + }, } } +} - pub fn default_content(&self) -> Option<&'static str> { - match self { - Self::User { .. } => None, - Self::CommitMessage => Some(include_str!("../../git_ui/src/commit_message_prompt.txt")), - } +impl From for PromptId { + fn from(builtin: BuiltInPrompt) -> Self { + PromptId::BuiltIn(builtin) } } @@ -117,7 +164,7 @@ impl std::fmt::Display for PromptId { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { PromptId::User { uuid } => write!(f, "{}", uuid.0), - PromptId::CommitMessage => write!(f, "Commit message"), + PromptId::BuiltIn(builtin) => write!(f, "{}", builtin), } } } @@ -150,6 +197,16 @@ impl MetadataCache { cache.metadata.push(metadata.clone()); cache.metadata_by_id.insert(prompt_id, metadata); } + + // Insert all the built-in prompts that were not customized by the user + for builtin in BuiltInPrompt::iter() { + let builtin_id = PromptId::BuiltIn(builtin); + if !cache.metadata_by_id.contains_key(&builtin_id) { + let metadata = PromptMetadata::builtin(builtin); + cache.metadata.push(metadata.clone()); + cache.metadata_by_id.insert(builtin_id, metadata); + } + } cache.sort(); Ok(cache) } @@ -198,10 +255,6 @@ impl PromptStore { let mut txn = db_env.write_txn()?; let metadata = db_env.create_database(&mut txn, Some("metadata.v2"))?; let bodies = db_env.create_database(&mut txn, Some("bodies.v2"))?; - - metadata.delete(&mut txn, &PromptId::CommitMessage)?; - bodies.delete(&mut txn, &PromptId::CommitMessage)?; - txn.commit()?; Self::upgrade_dbs(&db_env, metadata, bodies).log_err(); @@ -294,7 +347,16 @@ impl PromptStore { let bodies = self.bodies; cx.background_spawn(async move { let txn = env.read_txn()?; - let mut prompt = bodies.get(&txn, &id)?.context("prompt not found")?.into(); + let mut prompt: String = match bodies.get(&txn, &id)? { + Some(body) => body.into(), + None => { + if let Some(built_in) = id.as_built_in() { + built_in.default_content().into() + } else { + anyhow::bail!("prompt not found") + } + } + }; LineEnding::normalize(&mut prompt); Ok(prompt) }) @@ -339,11 +401,6 @@ impl PromptStore { }) } - /// Returns the number of prompts in the store. - pub fn prompt_count(&self) -> usize { - self.metadata_cache.read().metadata.len() - } - pub fn metadata(&self, id: PromptId) -> Option { self.metadata_cache.read().metadata_by_id.get(&id).cloned() } @@ -412,23 +469,38 @@ impl PromptStore { return Task::ready(Err(anyhow!("this prompt cannot be edited"))); } - let prompt_metadata = PromptMetadata { - id, - title, - default, - saved_at: Utc::now(), + let body = body.to_string(); + let is_default_content = id + .as_built_in() + .is_some_and(|builtin| body.trim() == builtin.default_content().trim()); + + let metadata = if let Some(builtin) = id.as_built_in() { + PromptMetadata::builtin(builtin) + } else { + PromptMetadata { + id, + title, + default, + saved_at: Utc::now(), + } }; - self.metadata_cache.write().insert(prompt_metadata.clone()); + + self.metadata_cache.write().insert(metadata.clone()); let db_connection = self.env.clone(); let bodies = self.bodies; - let metadata = self.metadata; + let metadata_db = self.metadata; let task = cx.background_spawn(async move { let mut txn = db_connection.write_txn()?; - metadata.put(&mut txn, &id, &prompt_metadata)?; - bodies.put(&mut txn, &id, &body.to_string())?; + if is_default_content { + metadata_db.delete(&mut txn, &id)?; + bodies.delete(&mut txn, &id)?; + } else { + metadata_db.put(&mut txn, &id, &metadata)?; + bodies.put(&mut txn, &id, &body)?; + } txn.commit()?; @@ -490,3 +562,122 @@ impl PromptStore { pub struct GlobalPromptStore(Shared, Arc>>>); impl Global for GlobalPromptStore {} + +#[cfg(test)] +mod tests { + use super::*; + use gpui::TestAppContext; + + #[gpui::test] + async fn test_built_in_prompt_load_save(cx: &mut TestAppContext) { + cx.executor().allow_parking(); + + let temp_dir = tempfile::tempdir().unwrap(); + let db_path = temp_dir.path().join("prompts-db"); + + let store = cx.update(|cx| PromptStore::new(db_path, cx)).await.unwrap(); + let store = cx.new(|_cx| store); + + let commit_message_id = PromptId::BuiltIn(BuiltInPrompt::CommitMessage); + + let loaded_content = store + .update(cx, |store, cx| store.load(commit_message_id, cx)) + .await + .unwrap(); + + let mut expected_content = BuiltInPrompt::CommitMessage.default_content().to_string(); + LineEnding::normalize(&mut expected_content); + assert_eq!( + loaded_content.trim(), + expected_content.trim(), + "Loading a built-in prompt not in DB should return default content" + ); + + let metadata = store.read_with(cx, |store, _| store.metadata(commit_message_id)); + assert!( + metadata.is_some(), + "Built-in prompt should always have metadata" + ); + assert!( + store.read_with(cx, |store, _| { + store + .metadata_cache + .read() + .metadata_by_id + .contains_key(&commit_message_id) + }), + "Built-in prompt should always be in cache" + ); + + let custom_content = "Custom commit message prompt"; + store + .update(cx, |store, cx| { + store.save( + commit_message_id, + Some("Commit message".into()), + false, + Rope::from(custom_content), + cx, + ) + }) + .await + .unwrap(); + + let loaded_custom = store + .update(cx, |store, cx| store.load(commit_message_id, cx)) + .await + .unwrap(); + assert_eq!( + loaded_custom.trim(), + custom_content.trim(), + "Custom content should be loaded after saving" + ); + + assert!( + store + .read_with(cx, |store, _| store.metadata(commit_message_id)) + .is_some(), + "Built-in prompt should have metadata after customization" + ); + + store + .update(cx, |store, cx| { + store.save( + commit_message_id, + Some("Commit message".into()), + false, + Rope::from(BuiltInPrompt::CommitMessage.default_content()), + cx, + ) + }) + .await + .unwrap(); + + let metadata_after_reset = + store.read_with(cx, |store, _| store.metadata(commit_message_id)); + assert!( + metadata_after_reset.is_some(), + "Built-in prompt should still have metadata after reset" + ); + assert_eq!( + metadata_after_reset + .as_ref() + .and_then(|m| m.title.as_ref().map(|t| t.as_ref())), + Some("Commit message"), + "Built-in prompt should have default title after reset" + ); + + let loaded_after_reset = store + .update(cx, |store, cx| store.load(commit_message_id, cx)) + .await + .unwrap(); + let mut expected_content_after_reset = + BuiltInPrompt::CommitMessage.default_content().to_string(); + LineEnding::normalize(&mut expected_content_after_reset); + assert_eq!( + loaded_after_reset.trim(), + expected_content_after_reset.trim(), + "After saving default content, load should return default" + ); + } +} diff --git a/crates/rules_library/src/rules_library.rs b/crates/rules_library/src/rules_library.rs index 642d6b64f79ed0f52b9cdb7feee900cf87af83cc..fc6af46782f26615aa0f5faeb7062ca03181ab9b 100644 --- a/crates/rules_library/src/rules_library.rs +++ b/crates/rules_library/src/rules_library.rs @@ -3,9 +3,9 @@ use collections::{HashMap, HashSet}; use editor::{CompletionProvider, SelectionEffects}; use editor::{CurrentLineHighlight, Editor, EditorElement, EditorEvent, EditorStyle, actions::Tab}; use gpui::{ - Action, App, Bounds, DEFAULT_ADDITIONAL_WINDOW_SIZE, Entity, EventEmitter, Focusable, - PromptLevel, Subscription, Task, TextStyle, TitlebarOptions, WindowBounds, WindowHandle, - WindowOptions, actions, point, size, transparent_black, + App, Bounds, DEFAULT_ADDITIONAL_WINDOW_SIZE, Entity, EventEmitter, Focusable, PromptLevel, + Subscription, Task, TextStyle, TitlebarOptions, WindowBounds, WindowHandle, WindowOptions, + actions, point, size, transparent_black, }; use language::{Buffer, LanguageRegistry, language_settings::SoftWrap}; use language_model::{ @@ -21,7 +21,7 @@ use std::sync::atomic::AtomicBool; use std::time::Duration; use theme::ThemeSettings; use title_bar::platform_title_bar::PlatformTitleBar; -use ui::{Divider, KeyBinding, ListItem, ListItemSpacing, ListSubHeader, Tooltip, prelude::*}; +use ui::{Divider, ListItem, ListItemSpacing, ListSubHeader, Tooltip, prelude::*}; use util::{ResultExt, TryFutureExt}; use workspace::{Workspace, WorkspaceSettings, client_side_decorations}; use zed_actions::assistant::InlineAssist; @@ -206,13 +206,8 @@ impl PickerDelegate for RulePickerDelegate { self.filtered_entries.len() } - fn no_matches_text(&self, _window: &mut Window, cx: &mut App) -> Option { - let text = if self.store.read(cx).prompt_count() == 0 { - "No rules.".into() - } else { - "No rules found matching your search.".into() - }; - Some(text) + fn no_matches_text(&self, _window: &mut Window, _cx: &mut App) -> Option { + Some("No rules found matching your search.".into()) } fn selected_index(&self) -> usize { @@ -680,13 +675,13 @@ impl RulesLibrary { window: &mut Window, cx: &mut Context, ) { - let Some(default_content) = prompt_id.default_content() else { + let Some(built_in) = prompt_id.as_built_in() else { return; }; if let Some(rule_editor) = self.rule_editors.get(&prompt_id) { rule_editor.body_editor.update(cx, |editor, cx| { - editor.set_text(default_content, window, cx); + editor.set_text(built_in.default_content(), window, cx); }); } } @@ -1428,31 +1423,7 @@ impl Render for RulesLibrary { this.border_t_1().border_color(cx.theme().colors().border) }) .child(self.render_rule_list(cx)) - .map(|el| { - if self.store.read(cx).prompt_count() == 0 { - el.child( - v_flex() - .h_full() - .flex_1() - .items_center() - .justify_center() - .border_l_1() - .border_color(cx.theme().colors().border) - .bg(cx.theme().colors().editor_background) - .child( - Button::new("create-rule", "New Rule") - .style(ButtonStyle::Outlined) - .key_binding(KeyBinding::for_action(&NewRule, cx)) - .on_click(|_, window, cx| { - window - .dispatch_action(NewRule.boxed_clone(), cx) - }), - ), - ) - } else { - el.child(self.render_active_rule(cx)) - } - }), + .child(self.render_active_rule(cx)), ), window, cx,