From c896ff292ce9a5f6cb8cb648105d552544742e35 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 15 Aug 2024 16:47:29 -0700 Subject: [PATCH] Remove workflow inspector, clean up workflow code (#16325) Now that there's a dedicated, user-facing view for each workflow step, we don't need the inspector functionality. This PR also cleans up some naming around workflow steps and step resolutions. Release Notes: - N/A --- crates/assistant/src/assistant.rs | 2 - crates/assistant/src/assistant_panel.rs | 281 ++++++--------------- crates/assistant/src/context.rs | 95 +++---- crates/assistant/src/context_inspector.rs | 241 ------------------ crates/assistant/src/workflow.rs | 117 +++------ crates/assistant/src/workflow/step_view.rs | 86 +++---- 6 files changed, 209 insertions(+), 613 deletions(-) delete mode 100644 crates/assistant/src/context_inspector.rs diff --git a/crates/assistant/src/assistant.rs b/crates/assistant/src/assistant.rs index db109e029d7d9e13f2837eb0507552a30b754539..7b26113ea9538b206203de72f22dda8ee7ccfa97 100644 --- a/crates/assistant/src/assistant.rs +++ b/crates/assistant/src/assistant.rs @@ -3,7 +3,6 @@ pub mod assistant_panel; pub mod assistant_settings; mod context; -pub(crate) mod context_inspector; pub mod context_store; mod inline_assistant; mod model_selector; @@ -65,7 +64,6 @@ actions!( DeployPromptLibrary, ConfirmCommand, ToggleModelSelector, - DebugWorkflowSteps ] ); diff --git a/crates/assistant/src/assistant_panel.rs b/crates/assistant/src/assistant_panel.rs index ce6273672afc289f24087b849baa30a5ab97b039..fb2fc805a3618174c69349e1168d77b446f1ce1b 100644 --- a/crates/assistant/src/assistant_panel.rs +++ b/crates/assistant/src/assistant_panel.rs @@ -1,6 +1,5 @@ use crate::{ assistant_settings::{AssistantDockPosition, AssistantSettings}, - context_inspector::ContextInspector, humanize_token_count, prompt_library::open_prompt_library, prompts::PromptBuilder, @@ -12,10 +11,10 @@ use crate::{ }, terminal_inline_assistant::TerminalInlineAssistant, Assist, ConfirmCommand, Context, ContextEvent, ContextId, ContextStore, CycleMessageRole, - DebugWorkflowSteps, DeployHistory, DeployPromptLibrary, InlineAssist, InlineAssistId, - InlineAssistant, InsertIntoEditor, MessageStatus, ModelSelector, PendingSlashCommand, - PendingSlashCommandStatus, QuoteSelection, RemoteContextMetadata, ResolvedWorkflowStep, - SavedContextMetadata, Split, ToggleFocus, ToggleModelSelector, WorkflowStepView, + DeployHistory, DeployPromptLibrary, InlineAssist, InlineAssistId, InlineAssistant, + InsertIntoEditor, MessageStatus, ModelSelector, PendingSlashCommand, PendingSlashCommandStatus, + QuoteSelection, RemoteContextMetadata, SavedContextMetadata, Split, ToggleFocus, + ToggleModelSelector, WorkflowStepResolution, WorkflowStepView, }; use crate::{ContextStoreEvent, ModelPickerDelegate, ShowConfiguration}; use anyhow::{anyhow, Result}; @@ -57,7 +56,7 @@ use settings::{update_settings_file, Settings}; use smol::stream::StreamExt; use std::{ borrow::Cow, - cmp::{self, Ordering}, + cmp, fmt::Write, ops::{DerefMut, Range}, path::PathBuf, @@ -65,7 +64,6 @@ use std::{ time::Duration, }; use terminal_view::{terminal_panel::TerminalPanel, TerminalView}; -use text::OffsetRangeExt; use ui::TintColor; use ui::{ prelude::*, @@ -77,7 +75,6 @@ use util::ResultExt; use workspace::{ dock::{DockPosition, Panel, PanelEvent}, item::{self, FollowableItem, Item, ItemHandle}, - notifications::NotifyTaskExt, pane::{self, SaveIntent}, searchable::{SearchEvent, SearchableItem}, Pane, Save, ToggleZoom, ToolbarItemEvent, ToolbarItemLocation, ToolbarItemView, Workspace, @@ -404,56 +401,13 @@ impl AssistantPanel { } else { "Zoom In" }; - let weak_pane = cx.view().downgrade(); let menu = ContextMenu::build(cx, |menu, cx| { - let menu = menu - .context(pane.focus_handle(cx)) + menu.context(pane.focus_handle(cx)) .action("New Context", Box::new(NewFile)) .action("History", Box::new(DeployHistory)) .action("Prompt Library", Box::new(DeployPromptLibrary)) .action("Configure", Box::new(ShowConfiguration)) - .action(zoom_label, Box::new(ToggleZoom)); - - if let Some(editor) = pane - .active_item() - .and_then(|e| e.downcast::()) - { - let is_enabled = editor.read(cx).debug_inspector.is_some(); - menu.separator().toggleable_entry( - "Debug Workflows", - is_enabled, - IconPosition::End, - None, - move |cx| { - weak_pane - .update(cx, |this, cx| { - if let Some(context_editor) = - this.active_item().and_then(|item| { - item.downcast::() - }) - { - context_editor.update(cx, |this, cx| { - if let Some(mut state) = - this.debug_inspector.take() - { - state.deactivate(cx); - } else { - this.debug_inspector = Some( - ContextInspector::new( - this.editor.clone(), - this.context.clone(), - ), - ); - } - }) - } - }) - .ok(); - }, - ) - } else { - menu - } + .action(zoom_label, Box::new(ToggleZoom)) }); cx.subscribe(&menu, |pane, _, _: &DismissEvent, _| { pane.new_item_menu = None; @@ -1380,7 +1334,7 @@ struct WorkflowStep { range: Range, header_block_id: CustomBlockId, footer_block_id: CustomBlockId, - resolved_step: Option>>, + resolved_step: Option>>, assist: Option, } @@ -1744,7 +1698,6 @@ pub struct ContextEditor { active_workflow_step: Option, assistant_panel: WeakView, error_message: Option, - debug_inspector: Option, show_accept_terms: bool, } @@ -1806,7 +1759,6 @@ impl ContextEditor { active_workflow_step: None, assistant_panel, error_message: None, - debug_inspector: None, show_accept_terms: false, }; this.update_message_headers(cx); @@ -2016,51 +1968,6 @@ impl ContextEditor { cx.propagate(); } - fn debug_workflow_steps(&mut self, _: &DebugWorkflowSteps, cx: &mut ViewContext) { - let mut output = String::new(); - for (i, step) in self.context.read(cx).workflow_steps().iter().enumerate() { - output.push_str(&format!("Step {}:\n", i + 1)); - output.push_str(&format!( - "Content: {}\n", - self.context - .read(cx) - .buffer() - .read(cx) - .text_for_range(step.tagged_range.clone()) - .collect::() - )); - match &step.resolution.read(cx).result { - Some(Ok(ResolvedWorkflowStep { - title, - suggestion_groups: suggestions, - })) => { - output.push_str("Resolution:\n"); - output.push_str(&format!(" {:?}\n", title)); - output.push_str(&format!(" {:?}\n", suggestions)); - } - None => { - output.push_str("Resolution: Pending\n"); - } - Some(Err(error)) => { - writeln!(output, "Resolution: Error\n{:?}", error).unwrap(); - } - } - output.push('\n'); - } - - let editor = self - .workspace - .update(cx, |workspace, cx| Editor::new_in_workspace(workspace, cx)); - - if let Ok(editor) = editor { - cx.spawn(|_, mut cx| async move { - let editor = editor.await?; - editor.update(&mut cx, |editor, cx| editor.set_text(output, cx)) - }) - .detach_and_notify_err(cx); - } - } - fn cycle_message_role(&mut self, _: &CycleMessageRole, cx: &mut ViewContext) { let cursors = self.cursors(cx); self.context.update(cx, |context, cx| { @@ -2482,9 +2389,6 @@ impl ContextEditor { blocks_to_remove.insert(step.header_block_id); blocks_to_remove.insert(step.footer_block_id); } - if let Some(debug) = self.debug_inspector.as_mut() { - debug.deactivate_for(step_range, cx); - } } self.editor.update(cx, |editor, cx| { editor.remove_blocks(blocks_to_remove, None, cx) @@ -2508,12 +2412,9 @@ impl ContextEditor { return; }; - let resolved_step = step.resolution.read(cx).result.clone(); + let resolved_step = step.read(cx).resolution.clone(); if let Some(existing_step) = self.workflow_steps.get_mut(&step_range) { existing_step.resolved_step = resolved_step; - if let Some(debug) = self.debug_inspector.as_mut() { - debug.refresh(&step_range, cx); - } } else { let start = buffer_snapshot .anchor_in_excerpt(excerpt_id, step_range.start) @@ -2553,51 +2454,80 @@ impl ContextEditor { } else { theme.info_border }; - let step_index = weak_self.update(&mut **cx, |this, cx| { - let snapshot = this.editor.read(cx).buffer().read(cx).as_singleton()?.read(cx).text_snapshot(); - let start_offset = step_range.start.to_offset(&snapshot); - let parent_message = this.context.read(cx).messages_for_offsets([start_offset], cx); - debug_assert_eq!(parent_message.len(), 1); - let parent_message = parent_message.first()?; - - let index_of_current_step = this.workflow_steps.keys().filter(|workflow_step_range| workflow_step_range.start.cmp(&parent_message.anchor, &snapshot).is_ge() && workflow_step_range.end.cmp(&step_range.end, &snapshot).is_le()).count(); - Some(index_of_current_step) - }).ok().flatten(); - - let debug_header = weak_self - .update(&mut **cx, |this, _| { - if let Some(inspector) = this.debug_inspector.as_mut() { - Some(inspector.is_active(&step_range)) - } else { - None - } + let step_index = weak_self + .update(&mut **cx, |this, cx| { + let snapshot = this + .editor + .read(cx) + .buffer() + .read(cx) + .as_singleton()? + .read(cx) + .text_snapshot(); + let start_offset = + step_range.start.to_offset(&snapshot); + let parent_message = this + .context + .read(cx) + .messages_for_offsets([start_offset], cx); + debug_assert_eq!(parent_message.len(), 1); + let parent_message = parent_message.first()?; + + let index_of_current_step = this + .workflow_steps + .keys() + .filter(|workflow_step_range| { + workflow_step_range + .start + .cmp(&parent_message.anchor, &snapshot) + .is_ge() + && workflow_step_range + .end + .cmp(&step_range.end, &snapshot) + .is_le() + }) + .count(); + Some(index_of_current_step) }) - .unwrap_or_default(); + .ok() + .flatten(); + let step_label = if let Some(index) = step_index { Label::new(format!("Step {index}")).size(LabelSize::Small) } else { Label::new("Step").size(LabelSize::Small) }; - let step_label = if current_status.as_ref().is_some_and(|status| status.is_confirmed()) { - h_flex().items_center().gap_2().child(step_label.strikethrough(true).color(Color::Muted)).child(Icon::new(IconName::Check).size(IconSize::Small).color(Color::Created)) + let step_label = if current_status + .as_ref() + .is_some_and(|status| status.is_confirmed()) + { + h_flex() + .items_center() + .gap_2() + .child( + step_label.strikethrough(true).color(Color::Muted), + ) + .child( + Icon::new(IconName::Check) + .size(IconSize::Small) + .color(Color::Created), + ) } else { div().child(step_label) }; - let step_label = step_label.id("step") + let step_label = step_label + .id("step") .cursor(CursorStyle::PointingHand) .on_click({ let this = weak_self.clone(); let step_range = step_range.clone(); move |_, cx| { - this - .update(cx, |this, cx| { - this.open_workflow_step( - step_range.clone(), cx, - ); - }) - .ok(); + this.update(cx, |this, cx| { + this.open_workflow_step(step_range.clone(), cx); + }) + .ok(); } }); @@ -2614,42 +2544,12 @@ impl ContextEditor { .items_center() .justify_between() .gap_2() - .child(h_flex().justify_start().gap_2().child(step_label).children( - debug_header.map(|is_active| { - - Button::new("debug-workflows-toggle", "Debug") - .icon_color(Color::Hidden) - .color(Color::Hidden) - .selected_icon_color(Color::Default) - .selected_label_color(Color::Default) - .icon(IconName::Microscope) - .icon_position(IconPosition::Start) - .icon_size(IconSize::Small) - .label_size(LabelSize::Small) - .selected(is_active) - .on_click({ - let weak_self = weak_self.clone(); - let step_range = step_range.clone(); - move |_, cx| { - weak_self - .update(cx, |this, cx| { - if let Some(inspector) = - this.debug_inspector - .as_mut() - { - if is_active { - - inspector.deactivate_for(&step_range, cx); - } else { - inspector.activate_for_step(step_range.clone(), cx); - } - } - }) - .ok(); - } - }) - }) - )) + .child( + h_flex() + .justify_start() + .gap_2() + .child(step_label), + ) .children(current_status.as_ref().map(|status| { h_flex().w_full().justify_end().child( status.into_element( @@ -2731,9 +2631,8 @@ impl ContextEditor { let context = self.context.read(cx); let language_registry = context.language_registry(); let step = context.workflow_step_for_range(step_range)?; - let resolution = step.resolution.clone(); let view = cx.new_view(|cx| { - WorkflowStepView::new(self.context.clone(), resolution, language_registry, cx) + WorkflowStepView::new(self.context.clone(), step, language_registry, cx) }); cx.deref_mut().defer(move |cx| { pane.update(cx, |pane, cx| { @@ -2857,7 +2756,7 @@ impl ContextEditor { } fn open_assists_for_step( - resolved_step: &ResolvedWorkflowStep, + resolved_step: &WorkflowStepResolution, project: &Model, assistant_panel: &WeakView, workspace: &WeakView, @@ -2961,7 +2860,7 @@ impl ContextEditor { let mut assist_ids = Vec::new(); for (excerpt_id, suggestion_group) in suggestion_groups { for suggestion in &suggestion_group.suggestions { - assist_ids.extend(suggestion.kind.show( + assist_ids.extend(suggestion.show( &editor, excerpt_id, workspace, @@ -3675,28 +3574,11 @@ impl ContextEditor { fn active_workflow_step_for_cursor(&self, cx: &AppContext) -> Option { let newest_cursor = self.editor.read(cx).selections.newest::(cx).head(); let context = self.context.read(cx); - let buffer = context.buffer().read(cx); - - let workflow_steps = context.workflow_steps(); - workflow_steps - .binary_search_by(|step| { - let step_range = step.tagged_range.to_offset(&buffer); - if newest_cursor < step_range.start { - Ordering::Greater - } else if newest_cursor > step_range.end { - Ordering::Less - } else { - Ordering::Equal - } - }) - .ok() - .and_then(|index| { - let range = workflow_steps[index].tagged_range.clone(); - Some(ActiveWorkflowStep { - resolved: self.workflow_steps.get(&range)?.resolved_step.is_some(), - range, - }) - }) + let (range, step) = context.workflow_step_containing(newest_cursor, cx)?; + Some(ActiveWorkflowStep { + resolved: step.read(cx).resolution.is_some(), + range, + }) } } @@ -3724,7 +3606,6 @@ impl Render for ContextEditor { .capture_action(cx.listener(ContextEditor::confirm_command)) .on_action(cx.listener(ContextEditor::assist)) .on_action(cx.listener(ContextEditor::split)) - .on_action(cx.listener(ContextEditor::debug_workflow_steps)) .size_full() .children(self.render_notice(cx)) .child( diff --git a/crates/assistant/src/context.rs b/crates/assistant/src/context.rs index d30ec47a65b00bfa40a7a32f3428d064a1c54b68..d08c468baf93db6891a17216b66472f05fa5fe6f 100644 --- a/crates/assistant/src/context.rs +++ b/crates/assistant/src/context.rs @@ -1,6 +1,6 @@ use crate::{ - prompts::PromptBuilder, slash_command::SlashCommandLine, workflow::WorkflowStepResolution, - MessageId, MessageStatus, + prompts::PromptBuilder, slash_command::SlashCommandLine, workflow::WorkflowStep, MessageId, + MessageStatus, }; use anyhow::{anyhow, Context as _, Result}; use assistant_slash_command::{ @@ -382,12 +382,6 @@ pub struct ImageAnchor { pub image: Shared>>, } -impl PartialEq for ImageAnchor { - fn eq(&self, other: &Self) -> bool { - self.image_id == other.image_id - } -} - struct PendingCompletion { id: usize, assistant_message_id: MessageId, @@ -397,18 +391,9 @@ struct PendingCompletion { #[derive(Copy, Clone, Debug, Hash, Eq, PartialEq)] pub struct SlashCommandId(clock::Lamport); -pub struct WorkflowStep { - pub tagged_range: Range, - pub resolution: Model, - pub _task: Option>, -} - -impl Debug for WorkflowStep { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("WorkflowStep") - .field("tagged_range", &self.tagged_range) - .finish_non_exhaustive() - } +struct WorkflowStepEntry { + range: Range, + step: Model, } pub struct Context { @@ -437,7 +422,7 @@ pub struct Context { _subscriptions: Vec, telemetry: Option>, language_registry: Arc, - workflow_steps: Vec, + workflow_steps: Vec, edits_since_last_workflow_step_prune: language::Subscription, project: Option>, prompt_builder: Arc, @@ -858,14 +843,40 @@ impl Context { self.summary.as_ref() } - pub fn workflow_steps(&self) -> &[WorkflowStep] { - &self.workflow_steps + pub fn workflow_step_containing( + &self, + offset: usize, + cx: &AppContext, + ) -> Option<(Range, Model)> { + let buffer = self.buffer.read(cx); + let index = self + .workflow_steps + .binary_search_by(|step| { + let step_range = step.range.to_offset(&buffer); + if offset < step_range.start { + Ordering::Greater + } else if offset > step_range.end { + Ordering::Less + } else { + Ordering::Equal + } + }) + .ok()?; + let step = &self.workflow_steps[index]; + Some((step.range.clone(), step.step.clone())) } - pub fn workflow_step_for_range(&self, range: Range) -> Option<&WorkflowStep> { - self.workflow_steps - .iter() - .find(|step| step.tagged_range == range) + pub fn workflow_step_for_range( + &self, + range: Range, + ) -> Option> { + Some( + self.workflow_steps + .iter() + .find(|step| step.range == range)? + .step + .clone(), + ) } pub fn pending_slash_commands(&self) -> &[PendingSlashCommand] { @@ -1028,7 +1039,7 @@ impl Context { removed.extend( self.workflow_steps .drain(intersecting_range) - .map(|step| step.tagged_range), + .map(|step| step.range), ); } @@ -1047,7 +1058,7 @@ impl Context { let buffer = self.buffer.read(cx); let start_ix = match self.workflow_steps.binary_search_by(|probe| { probe - .tagged_range + .range .end .to_offset(buffer) .cmp(&range.start) @@ -1061,7 +1072,7 @@ impl Context { }; let end_ix = match self.workflow_steps.binary_search_by(|probe| { probe - .tagged_range + .range .start .to_offset(buffer) .cmp(&range.end) @@ -1114,20 +1125,16 @@ impl Context { // Check if a step with the same range already exists let existing_step_index = self .workflow_steps - .binary_search_by(|probe| probe.tagged_range.cmp(&tagged_range, &buffer)); + .binary_search_by(|probe| probe.range.cmp(&tagged_range, &buffer)); if let Err(ix) = existing_step_index { new_edit_steps.push(( ix, - WorkflowStep { - resolution: cx.new_model(|_| { - WorkflowStepResolution::new( - tagged_range.clone(), - weak_self.clone(), - ) + WorkflowStepEntry { + step: cx.new_model(|_| { + WorkflowStep::new(tagged_range.clone(), weak_self.clone()) }), - tagged_range, - _task: None, + range: tagged_range, }, )); } @@ -1141,7 +1148,7 @@ impl Context { let mut updated = Vec::new(); for (index, step) in new_edit_steps.into_iter().rev() { - let step_range = step.tagged_range.clone(); + let step_range = step.range.clone(); updated.push(step_range.clone()); self.workflow_steps.insert(index, step); self.resolve_workflow_step(step_range, cx); @@ -1161,7 +1168,7 @@ impl Context { ) { let Ok(step_index) = self .workflow_steps - .binary_search_by(|step| step.tagged_range.cmp(&tagged_range, self.buffer.read(cx))) + .binary_search_by(|step| step.range.cmp(&tagged_range, self.buffer.read(cx))) else { return; }; @@ -1169,7 +1176,7 @@ impl Context { cx.emit(ContextEvent::WorkflowStepUpdated(tagged_range.clone())); cx.notify(); - let resolution = self.workflow_steps[step_index].resolution.clone(); + let resolution = self.workflow_steps[step_index].step.clone(); cx.defer(move |cx| { resolution.update(cx, |resolution, cx| resolution.resolve(cx)); }); @@ -3032,12 +3039,12 @@ mod tests { .iter() .map(|step| { let buffer = context.buffer.read(cx); - let status = match &step.resolution.read(cx).result { + let status = match &step.step.read(cx).resolution { None => WorkflowStepTestStatus::Pending, Some(Ok(_)) => WorkflowStepTestStatus::Resolved, Some(Err(_)) => WorkflowStepTestStatus::Error, }; - (step.tagged_range.to_point(buffer), status) + (step.range.to_point(buffer), status) }) .collect() } diff --git a/crates/assistant/src/context_inspector.rs b/crates/assistant/src/context_inspector.rs deleted file mode 100644 index ed1c22f1cd693e4339349ed81ab9204ffb3436a5..0000000000000000000000000000000000000000 --- a/crates/assistant/src/context_inspector.rs +++ /dev/null @@ -1,241 +0,0 @@ -use std::{ops::Range, sync::Arc}; - -use collections::{HashMap, HashSet}; -use editor::{ - display_map::{BlockDisposition, BlockProperties, BlockStyle, CustomBlockId}, - Editor, -}; -use gpui::{AppContext, Model, View}; -use text::{Bias, ToOffset, ToPoint}; -use ui::{ - div, h_flex, px, Color, Element as _, ParentElement as _, Styled, ViewContext, WindowContext, -}; - -use crate::{Context, ResolvedWorkflowStep, WorkflowSuggestion, WorkflowSuggestionKind}; - -type StepRange = Range; - -struct DebugInfo { - range: Range, - block_id: CustomBlockId, -} - -pub(crate) struct ContextInspector { - active_debug_views: HashMap, DebugInfo>, - context: Model, - editor: View, -} - -impl ContextInspector { - pub(crate) fn new(editor: View, context: Model) -> Self { - Self { - editor, - context, - active_debug_views: Default::default(), - } - } - - pub(crate) fn is_active(&self, range: &StepRange) -> bool { - self.active_debug_views.contains_key(range) - } - - pub(crate) fn refresh(&mut self, range: &StepRange, cx: &mut WindowContext<'_>) { - if self.deactivate_for(range, cx) { - self.activate_for_step(range.clone(), cx); - } - } - - fn crease_content( - context: &Model, - range: StepRange, - cx: &mut AppContext, - ) -> Option> { - use std::fmt::Write; - let step = context.read(cx).workflow_step_for_range(range)?; - let mut output = String::from("\n\n"); - match &step.resolution.read(cx).result { - Some(Ok(ResolvedWorkflowStep { - title, - suggestion_groups: suggestions, - })) => { - writeln!(output, "Resolution:").ok()?; - writeln!(output, " {title:?}").ok()?; - if suggestions.is_empty() { - writeln!(output, " No suggestions").ok()?; - } - - for (buffer, suggestion_groups) in suggestions { - let buffer = buffer.read(cx); - let buffer_path = buffer - .file() - .and_then(|file| file.path().to_str()) - .unwrap_or("untitled"); - let snapshot = buffer.text_snapshot(); - writeln!(output, "Path: {buffer_path}:").ok()?; - for group in suggestion_groups { - for suggestion in &group.suggestions { - pretty_print_workflow_suggestion(&mut output, suggestion, &snapshot); - } - } - } - } - Some(Err(error)) => { - writeln!(output, "Resolution: Error").ok()?; - writeln!(output, "{error:?}").ok()?; - } - None => { - writeln!(output, "Resolution: Pending").ok()?; - } - } - - Some(output.into()) - } - - pub(crate) fn activate_for_step(&mut self, range: StepRange, cx: &mut WindowContext<'_>) { - let text = Self::crease_content(&self.context, range.clone(), cx) - .unwrap_or_else(|| Arc::from("Error fetching debug info")); - self.editor.update(cx, |editor, cx| { - let buffer = editor.buffer().read(cx).as_singleton()?; - let snapshot = buffer.read(cx).text_snapshot(); - let start_offset = range.end.to_offset(&snapshot) + 1; - let start_offset = snapshot.clip_offset(start_offset, Bias::Right); - let text_len = text.len(); - buffer.update(cx, |this, cx| { - this.edit([(start_offset..start_offset, text)], None, cx); - }); - - let end_offset = start_offset + text_len; - let multibuffer_snapshot = editor.buffer().read(cx).snapshot(cx); - let anchor_before = multibuffer_snapshot.anchor_after(start_offset); - let anchor_after = multibuffer_snapshot.anchor_before(end_offset); - - let block_id = editor - .insert_blocks( - [BlockProperties { - position: anchor_after, - height: 0, - style: BlockStyle::Sticky, - render: Box::new(move |cx| { - div() - .w_full() - .px(cx.gutter_dimensions.full_width()) - .child(h_flex().h(px(1.)).bg(Color::Warning.color(cx))) - .into_any() - }), - disposition: BlockDisposition::Below, - priority: 0, - }], - None, - cx, - ) - .into_iter() - .next()?; - let info = DebugInfo { - range: anchor_before..anchor_after, - block_id, - }; - self.active_debug_views.insert(range, info); - Some(()) - }); - } - - fn deactivate_impl(editor: &mut Editor, debug_data: DebugInfo, cx: &mut ViewContext) { - editor.remove_blocks(HashSet::from_iter([debug_data.block_id]), None, cx); - editor.edit([(debug_data.range, Arc::::default())], cx) - } - pub(crate) fn deactivate_for(&mut self, range: &StepRange, cx: &mut WindowContext<'_>) -> bool { - if let Some(debug_data) = self.active_debug_views.remove(range) { - self.editor.update(cx, |this, cx| { - Self::deactivate_impl(this, debug_data, cx); - }); - true - } else { - false - } - } - - pub(crate) fn deactivate(&mut self, cx: &mut WindowContext<'_>) { - let steps_to_disable = std::mem::take(&mut self.active_debug_views); - - self.editor.update(cx, move |editor, cx| { - for (_, debug_data) in steps_to_disable { - Self::deactivate_impl(editor, debug_data, cx); - } - }); - } -} -fn pretty_print_anchor( - out: &mut String, - anchor: language::Anchor, - snapshot: &text::BufferSnapshot, -) { - use std::fmt::Write; - let point = anchor.to_point(snapshot); - write!(out, "{}:{}", point.row, point.column).ok(); -} -fn pretty_print_range( - out: &mut String, - range: &Range, - snapshot: &text::BufferSnapshot, -) { - use std::fmt::Write; - write!(out, " Range: ").ok(); - pretty_print_anchor(out, range.start, snapshot); - write!(out, "..").ok(); - pretty_print_anchor(out, range.end, snapshot); -} - -fn pretty_print_workflow_suggestion( - out: &mut String, - suggestion: &WorkflowSuggestion, - snapshot: &text::BufferSnapshot, -) { - use std::fmt::Write; - let (position, description, range) = match &suggestion.kind { - WorkflowSuggestionKind::Update { - range, description, .. - } => (None, Some(description), Some(range)), - WorkflowSuggestionKind::CreateFile { description } => (None, Some(description), None), - WorkflowSuggestionKind::AppendChild { - position, - description, - .. - } => (Some(position), Some(description), None), - WorkflowSuggestionKind::InsertSiblingBefore { - position, - description, - .. - } => (Some(position), Some(description), None), - WorkflowSuggestionKind::InsertSiblingAfter { - position, - description, - .. - } => (Some(position), Some(description), None), - WorkflowSuggestionKind::PrependChild { - position, - description, - .. - } => (Some(position), Some(description), None), - WorkflowSuggestionKind::Delete { range, .. } => (None, None, Some(range)), - }; - writeln!(out, " Tool input: {}", suggestion.tool_input).ok(); - writeln!( - out, - " Tool output: {}", - serde_json::to_string_pretty(&suggestion.tool_output) - .expect("Should not fail on valid struct serialization") - ) - .ok(); - if let Some(description) = description { - writeln!(out, " Description: {description}").ok(); - } - if let Some(range) = range { - pretty_print_range(out, &range, snapshot); - } - if let Some(position) = position { - write!(out, " Position: ").ok(); - pretty_print_anchor(out, *position, snapshot); - write!(out, "\n").ok(); - } - write!(out, "\n").ok(); -} diff --git a/crates/assistant/src/workflow.rs b/crates/assistant/src/workflow.rs index 55a85bb718c6009d95ef7c67f3ecd17e0fa8e170..f08456a35b840e3f49cc1f4e57c515d74ae2a9e2 100644 --- a/crates/assistant/src/workflow.rs +++ b/crates/assistant/src/workflow.rs @@ -8,8 +8,7 @@ use collections::HashMap; use editor::Editor; use futures::future; use gpui::{ - AppContext, Model, ModelContext, Task, UpdateGlobal as _, View, WeakModel, WeakView, - WindowContext, + Model, ModelContext, Task, UpdateGlobal as _, View, WeakModel, WeakView, WindowContext, }; use language::{Anchor, Buffer, BufferSnapshot, SymbolPath}; use language_model::{LanguageModelRegistry, LanguageModelRequestMessage, Role}; @@ -24,16 +23,16 @@ use workspace::Workspace; pub use step_view::WorkflowStepView; -pub struct WorkflowStepResolution { - tagged_range: Range, - output: String, +pub struct WorkflowStep { context: WeakModel, + context_buffer_range: Range, + tool_output: String, resolve_task: Option>, - pub result: Option>>, + pub resolution: Option>>, } #[derive(Clone, Debug, Eq, PartialEq)] -pub struct ResolvedWorkflowStep { +pub struct WorkflowStepResolution { pub title: String, pub suggestion_groups: HashMap, Vec>, } @@ -45,36 +44,7 @@ pub struct WorkflowSuggestionGroup { } #[derive(Clone, Debug, Eq, PartialEq)] -pub struct WorkflowSuggestion { - pub kind: WorkflowSuggestionKind, - pub tool_input: String, - pub tool_output: tool::WorkflowSuggestionTool, -} - -impl WorkflowSuggestion { - pub fn range(&self) -> Range { - self.kind.range() - } - - pub fn show( - &self, - editor: &View, - excerpt_id: editor::ExcerptId, - workspace: &WeakView, - assistant_panel: &View, - cx: &mut ui::ViewContext, - ) -> Option { - self.kind - .show(editor, excerpt_id, workspace, assistant_panel, cx) - } - - fn try_merge(&mut self, other: &mut WorkflowSuggestion, snapshot: &BufferSnapshot) -> bool { - self.kind.try_merge(&other.kind, snapshot) - } -} - -#[derive(Clone, Debug, Eq, PartialEq)] -pub enum WorkflowSuggestionKind { +pub enum WorkflowSuggestion { Update { symbol_path: SymbolPath, range: Range, @@ -109,28 +79,19 @@ pub enum WorkflowSuggestionKind { }, } -impl WorkflowStepResolution { +impl WorkflowStep { pub fn new(range: Range, context: WeakModel) -> Self { Self { - tagged_range: range, - output: String::new(), + context_buffer_range: range, + tool_output: String::new(), context, - result: None, + resolution: None, resolve_task: None, } } - pub fn step_text(&self, context: &Context, cx: &AppContext) -> String { - context - .buffer() - .clone() - .read(cx) - .text_for_range(self.tagged_range.clone()) - .collect::() - } - - pub fn resolve(&mut self, cx: &mut ModelContext) -> Option<()> { - let range = self.tagged_range.clone(); + pub fn resolve(&mut self, cx: &mut ModelContext) -> Option<()> { + let range = self.context_buffer_range.clone(); let context = self.context.upgrade()?; let context = context.read(cx); let project = context.project()?; @@ -159,8 +120,8 @@ impl WorkflowStepResolution { }; this.update(&mut cx, |this, cx| { - this.output.clear(); - this.result = None; + this.tool_output.clear(); + this.resolution = None; this.result_updated(cx); cx.notify(); })?; @@ -184,17 +145,17 @@ impl WorkflowStepResolution { while let Some(chunk) = stream.next().await { let chunk = chunk?; this.update(&mut cx, |this, cx| { - this.output.push_str(&chunk); + this.tool_output.push_str(&chunk); cx.notify(); })?; } let resolution = this.update(&mut cx, |this, _| { - serde_json::from_str::(&this.output) + serde_json::from_str::(&this.tool_output) })??; this.update(&mut cx, |this, cx| { - this.output = serde_json::to_string_pretty(&resolution).unwrap(); + this.tool_output = serde_json::to_string_pretty(&resolution).unwrap(); cx.notify(); })?; @@ -202,9 +163,7 @@ impl WorkflowStepResolution { let suggestion_tasks: Vec<_> = resolution .suggestions .iter() - .map(|suggestion| { - suggestion.resolve(step_text.clone(), project.clone(), cx.clone()) - }) + .map(|suggestion| suggestion.resolve(project.clone(), cx.clone())) .collect(); // Expand the context ranges of each suggestion and group suggestions with overlapping context ranges. @@ -281,8 +240,8 @@ impl WorkflowStepResolution { let result = result.await; this.update(&mut cx, |this, cx| { - this.result = Some(match result { - Ok((title, suggestion_groups)) => Ok(ResolvedWorkflowStep { + this.resolution = Some(match result { + Ok((title, suggestion_groups)) => Ok(WorkflowStepResolution { title, suggestion_groups, }), @@ -301,13 +260,13 @@ impl WorkflowStepResolution { fn result_updated(&mut self, cx: &mut ModelContext) { self.context .update(cx, |context, cx| { - context.workflow_step_updated(self.tagged_range.clone(), cx) + context.workflow_step_updated(self.context_buffer_range.clone(), cx) }) .ok(); } } -impl WorkflowSuggestionKind { +impl WorkflowSuggestion { pub fn range(&self) -> Range { match self { Self::Update { range, .. } => range.clone(), @@ -504,8 +463,6 @@ impl WorkflowSuggestionKind { } pub mod tool { - use std::path::Path; - use super::*; use anyhow::Context as _; use gpui::AsyncAppContext; @@ -513,6 +470,7 @@ pub mod tool { use language_model::LanguageModelTool; use project::ProjectPath; use schemars::JsonSchema; + use std::path::Path; #[derive(Debug, Serialize, Deserialize, JsonSchema)] pub struct WorkflowStepResolutionTool { @@ -562,7 +520,6 @@ pub mod tool { impl WorkflowSuggestionTool { pub(super) async fn resolve( &self, - tool_input: String, project: Model, mut cx: AsyncAppContext, ) -> Result<(Model, super::WorkflowSuggestion)> { @@ -599,7 +556,7 @@ pub mod tool { let snapshot = buffer.update(&mut cx, |buffer, _| buffer.snapshot())?; let outline = snapshot.outline(None).context("no outline for buffer")?; - let kind = match kind { + let suggestion = match kind { WorkflowSuggestionToolKind::Update { symbol, description, @@ -617,14 +574,14 @@ pub mod tool { snapshot.line_len(symbol.range.end.row), ); let range = snapshot.anchor_before(start)..snapshot.anchor_after(end); - WorkflowSuggestionKind::Update { + WorkflowSuggestion::Update { range, description, symbol_path, } } WorkflowSuggestionToolKind::Create { description } => { - WorkflowSuggestionKind::CreateFile { description } + WorkflowSuggestion::CreateFile { description } } WorkflowSuggestionToolKind::InsertSiblingBefore { symbol, @@ -641,7 +598,7 @@ pub mod tool { annotation_range.start }), ); - WorkflowSuggestionKind::InsertSiblingBefore { + WorkflowSuggestion::InsertSiblingBefore { position, description, symbol_path, @@ -656,7 +613,7 @@ pub mod tool { .with_context(|| format!("symbol not found: {:?}", symbol))?; let symbol = symbol.to_point(&snapshot); let position = snapshot.anchor_after(symbol.range.end); - WorkflowSuggestionKind::InsertSiblingAfter { + WorkflowSuggestion::InsertSiblingAfter { position, description, symbol_path, @@ -677,13 +634,13 @@ pub mod tool { .body_range .map_or(symbol.range.start, |body_range| body_range.start), ); - WorkflowSuggestionKind::PrependChild { + WorkflowSuggestion::PrependChild { position, description, symbol_path: Some(symbol_path), } } else { - WorkflowSuggestionKind::PrependChild { + WorkflowSuggestion::PrependChild { position: language::Anchor::MIN, description, symbol_path: None, @@ -705,13 +662,13 @@ pub mod tool { .body_range .map_or(symbol.range.end, |body_range| body_range.end), ); - WorkflowSuggestionKind::AppendChild { + WorkflowSuggestion::AppendChild { position, description, symbol_path: Some(symbol_path), } } else { - WorkflowSuggestionKind::PrependChild { + WorkflowSuggestion::PrependChild { position: language::Anchor::MAX, description, symbol_path: None, @@ -732,16 +689,10 @@ pub mod tool { snapshot.line_len(symbol.range.end.row), ); let range = snapshot.anchor_before(start)..snapshot.anchor_after(end); - WorkflowSuggestionKind::Delete { range, symbol_path } + WorkflowSuggestion::Delete { range, symbol_path } } }; - let suggestion = WorkflowSuggestion { - kind, - tool_output: self.clone(), - tool_input, - }; - Ok((buffer, suggestion)) } } diff --git a/crates/assistant/src/workflow/step_view.rs b/crates/assistant/src/workflow/step_view.rs index a1b3bd7ee2ae03385734e2a8819bee800185ff7e..96d2ab710b29ea0c1cd562b18182281df4fbf3ae 100644 --- a/crates/assistant/src/workflow/step_view.rs +++ b/crates/assistant/src/workflow/step_view.rs @@ -1,4 +1,4 @@ -use super::WorkflowStepResolution; +use super::WorkflowStep; use crate::{Assist, Context}; use editor::{ display_map::{BlockDisposition, BlockProperties, BlockStyle}, @@ -23,7 +23,7 @@ use workspace::{ }; pub struct WorkflowStepView { - step: WeakModel, + step: WeakModel, tool_output_buffer: Model, editor: View, } @@ -31,17 +31,18 @@ pub struct WorkflowStepView { impl WorkflowStepView { pub fn new( context: Model, - step: Model, + step: Model, language_registry: Arc, cx: &mut ViewContext, ) -> Self { - let tool_output_buffer = cx.new_model(|cx| Buffer::local(step.read(cx).output.clone(), cx)); + let tool_output_buffer = + cx.new_model(|cx| Buffer::local(step.read(cx).tool_output.clone(), cx)); let buffer = cx.new_model(|cx| { let mut buffer = MultiBuffer::without_headers(0, language::Capability::ReadWrite); buffer.push_excerpts( context.read(cx).buffer().clone(), [ExcerptRange { - context: step.read(cx).tagged_range.clone(), + context: step.read(cx).context_buffer_range.clone(), primary: None, }], cx, @@ -146,55 +147,54 @@ impl WorkflowStepView { fn render_result(&mut self, cx: &mut ViewContext) -> Option { let step = self.step.upgrade()?; - let result = step.read(cx).result.as_ref()?; + let result = step.read(cx).resolution.as_ref()?; match result { - Ok(result) => Some( - v_flex() - .child(result.title.clone()) - .children(result.suggestion_groups.iter().filter_map( - |(buffer, suggestion_groups)| { - let path = buffer.read(cx).file().map(|f| f.path()); - v_flex() - .mb_2() - .border_b_1() - .children(path.map(|path| format!("path: {}", path.display()))) - .children(suggestion_groups.iter().map(|group| { - v_flex().pl_2().children(group.suggestions.iter().map( - |suggestion| { - v_flex() - .children( - suggestion - .kind - .description() - .map(|desc| format!("description: {desc}")), - ) - .child(format!("kind: {}", suggestion.kind.kind())) - .children( - suggestion.kind.symbol_path().map(|path| { - format!("symbol path: {}", path.0) - }), - ) - }, - )) - })) - .into() - }, - )) - .into_any_element(), - ), + Ok(result) => { + Some( + v_flex() + .child(result.title.clone()) + .children(result.suggestion_groups.iter().filter_map( + |(buffer, suggestion_groups)| { + let path = buffer.read(cx).file().map(|f| f.path()); + v_flex() + .mb_2() + .border_b_1() + .children(path.map(|path| format!("path: {}", path.display()))) + .children(suggestion_groups.iter().map(|group| { + v_flex().pl_2().children(group.suggestions.iter().map( + |suggestion| { + v_flex() + .children( + suggestion.description().map(|desc| { + format!("description: {desc}") + }), + ) + .child(format!("kind: {}", suggestion.kind())) + .children(suggestion.symbol_path().map( + |path| format!("symbol path: {}", path.0), + )) + }, + )) + })) + .into() + }, + )) + .into_any_element(), + ) + } Err(error) => Some(format!("{:?}", error).into_any_element()), } } - fn step_updated(&mut self, step: Model, cx: &mut ViewContext) { + fn step_updated(&mut self, step: Model, cx: &mut ViewContext) { self.tool_output_buffer.update(cx, |buffer, cx| { - let text = step.read(cx).output.clone(); + let text = step.read(cx).tool_output.clone(); buffer.set_text(text, cx); }); cx.notify(); } - fn step_released(&mut self, _: &mut WorkflowStepResolution, cx: &mut ViewContext) { + fn step_released(&mut self, _: &mut WorkflowStep, cx: &mut ViewContext) { cx.emit(EditorEvent::Closed); }