From c512d43e8cb3ad2933972b33f3f247476132de2b Mon Sep 17 00:00:00 2001 From: Agus Zubiaga Date: Thu, 8 May 2025 22:18:52 -0300 Subject: [PATCH] agent: Render edit tool error as markdown (#30325) Release Notes: - agent: Render edit tool error as markdown and allow selecting it --- Cargo.lock | 1 + crates/assistant_tools/Cargo.toml | 7 +- crates/assistant_tools/src/edit_file_tool.rs | 158 +++++++++++-------- 3 files changed, 98 insertions(+), 68 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6dc7dd1ce7..b6b6d19327 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -676,6 +676,7 @@ dependencies = [ "language_models", "linkme", "log", + "markdown", "open", "paths", "portable-pty", diff --git a/crates/assistant_tools/Cargo.toml b/crates/assistant_tools/Cargo.toml index 12f0b2685a..5f57d530c0 100644 --- a/crates/assistant_tools/Cargo.toml +++ b/crates/assistant_tools/Cargo.toml @@ -17,14 +17,14 @@ eval = [] [dependencies] aho-corasick.workspace = true anyhow.workspace = true -assistant_tool.workspace = true assistant_settings.workspace = true +assistant_tool.workspace = true buffer_diff.workspace = true chrono.workspace = true collections.workspace = true component.workspace = true -editor.workspace = true derive_more.workspace = true +editor.workspace = true feature_flags.workspace = true futures.workspace = true gpui.workspace = true @@ -35,8 +35,9 @@ indoc.workspace = true itertools.workspace = true language.workspace = true language_model.workspace = true -log.workspace = true linkme.workspace = true +log.workspace = true +markdown.workspace = true open.workspace = true paths.workspace = true portable-pty.workspace = true diff --git a/crates/assistant_tools/src/edit_file_tool.rs b/crates/assistant_tools/src/edit_file_tool.rs index acad0133ed..845b531081 100644 --- a/crates/assistant_tools/src/edit_file_tool.rs +++ b/crates/assistant_tools/src/edit_file_tool.rs @@ -20,6 +20,7 @@ use language::{ language_settings::SoftWrap, }; use language_model::{LanguageModel, LanguageModelRequest, LanguageModelToolSchemaFormat}; +use markdown::{Markdown, MarkdownElement, MarkdownStyle}; use project::Project; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -335,7 +336,7 @@ pub struct EditFileToolCard { project: Entity, diff_task: Option>>, preview_expanded: bool, - error_expanded: bool, + error_expanded: Option>, full_height_expanded: bool, total_lines: Option, editor_unique_id: EntityId, @@ -378,7 +379,7 @@ impl EditFileToolCard { multibuffer, diff_task: None, preview_expanded: true, - error_expanded: false, + error_expanded: None, full_height_expanded: false, total_lines: None, } @@ -435,9 +436,9 @@ impl ToolCard for EditFileToolCard { workspace: WeakEntity, cx: &mut Context, ) -> impl IntoElement { - let (failed, error_message) = match status { - ToolUseStatus::Error(err) => (true, Some(err.to_string())), - _ => (false, None), + let error_message = match status { + ToolUseStatus::Error(err) => Some(err), + _ => None, }; let path_label_button = h_flex() @@ -525,9 +526,11 @@ impl ToolCard for EditFileToolCard { .gap_1() .justify_between() .rounded_t_md() - .when(!failed, |header| header.bg(codeblock_header_bg)) + .when(error_message.is_none(), |header| { + header.bg(codeblock_header_bg) + }) .child(path_label_button) - .when(failed, |header| { + .when_some(error_message, |header, error_message| { header.child( h_flex() .gap_1() @@ -539,19 +542,28 @@ impl ToolCard for EditFileToolCard { .child( Disclosure::new( ("edit-file-error-disclosure", self.editor_unique_id), - self.error_expanded, + self.error_expanded.is_some(), ) .opened_icon(IconName::ChevronUp) .closed_icon(IconName::ChevronDown) - .on_click(cx.listener( - move |this, _event, _window, _cx| { - this.error_expanded = !this.error_expanded; - }, - )), + .on_click(cx.listener({ + let error_message = error_message.clone(); + + move |this, _event, _window, cx| { + if this.error_expanded.is_some() { + this.error_expanded.take(); + } else { + this.error_expanded = Some(cx.new(|cx| { + Markdown::new(error_message.clone(), None, None, cx) + })) + } + cx.notify(); + } + })), ), ) }) - .when(!failed && self.has_diff(), |header| { + .when(error_message.is_none() && self.has_diff(), |header| { header.child( Disclosure::new( ("edit-file-disclosure", self.editor_unique_id), @@ -658,12 +670,12 @@ impl ToolCard for EditFileToolCard { v_flex() .mb_2() .border_1() - .when(failed, |card| card.border_dashed()) + .when(error_message.is_some(), |card| card.border_dashed()) .border_color(border_color) .rounded_md() .overflow_hidden() .child(codeblock_header) - .when(failed && self.error_expanded, |card| { + .when_some(self.error_expanded.as_ref(), |card, error_markdown| { card.child( v_flex() .p_2() @@ -683,65 +695,81 @@ impl ToolCard for EditFileToolCard { .rounded_md() .text_ui_sm(cx) .bg(cx.theme().colors().editor_background) - .children( - error_message - .map(|error| div().child(error).into_any_element()), - ), + .child(MarkdownElement::new( + error_markdown.clone(), + markdown_style(window, cx), + )), ), ) }) - .when(!self.has_diff() && !failed, |card| { + .when(!self.has_diff() && error_message.is_none(), |card| { card.child(waiting_for_diff) }) - .when( - !failed && self.preview_expanded && self.has_diff(), - |card| { + .when(self.preview_expanded && self.has_diff(), |card| { + card.child( + v_flex() + .relative() + .h_full() + .when(!self.full_height_expanded, |editor_container| { + editor_container + .max_h(DEFAULT_COLLAPSED_LINES as f32 * editor_line_height) + }) + .overflow_hidden() + .border_t_1() + .border_color(border_color) + .bg(cx.theme().colors().editor_background) + .child(editor) + .when( + !self.full_height_expanded && is_collapsible, + |editor_container| editor_container.child(gradient_overlay), + ), + ) + .when(is_collapsible, |card| { card.child( - v_flex() - .relative() - .h_full() - .when(!self.full_height_expanded, |editor_container| { - editor_container - .max_h(DEFAULT_COLLAPSED_LINES as f32 * editor_line_height) - }) - .overflow_hidden() + h_flex() + .id(("expand-button", self.editor_unique_id)) + .flex_none() + .cursor_pointer() + .h_5() + .justify_center() .border_t_1() + .rounded_b_md() .border_color(border_color) .bg(cx.theme().colors().editor_background) - .child(editor) - .when( - !self.full_height_expanded && is_collapsible, - |editor_container| editor_container.child(gradient_overlay), - ), + .hover(|style| style.bg(cx.theme().colors().element_hover.opacity(0.1))) + .child( + Icon::new(full_height_icon) + .size(IconSize::Small) + .color(Color::Muted), + ) + .tooltip(Tooltip::text(full_height_tooltip_label)) + .on_click(cx.listener(move |this, _event, _window, _cx| { + this.full_height_expanded = !this.full_height_expanded; + })), ) - .when(is_collapsible, |card| { - card.child( - h_flex() - .id(("expand-button", self.editor_unique_id)) - .flex_none() - .cursor_pointer() - .h_5() - .justify_center() - .border_t_1() - .rounded_b_md() - .border_color(border_color) - .bg(cx.theme().colors().editor_background) - .hover(|style| { - style.bg(cx.theme().colors().element_hover.opacity(0.1)) - }) - .child( - Icon::new(full_height_icon) - .size(IconSize::Small) - .color(Color::Muted), - ) - .tooltip(Tooltip::text(full_height_tooltip_label)) - .on_click(cx.listener(move |this, _event, _window, _cx| { - this.full_height_expanded = !this.full_height_expanded; - })), - ) - }) - }, - ) + }) + }) + } +} + +fn markdown_style(window: &Window, cx: &App) -> MarkdownStyle { + let theme_settings = ThemeSettings::get_global(cx); + let ui_font_size = TextSize::Default.rems(cx); + let mut text_style = window.text_style(); + + text_style.refine(&TextStyleRefinement { + font_family: Some(theme_settings.ui_font.family.clone()), + font_fallbacks: theme_settings.ui_font.fallbacks.clone(), + font_features: Some(theme_settings.ui_font.features.clone()), + font_size: Some(ui_font_size.into()), + color: Some(cx.theme().colors().text), + ..Default::default() + }); + + MarkdownStyle { + base_text_style: text_style.clone(), + selection_background_color: cx.theme().players().local().selection, + ..Default::default() } }