refactor(tools): simplify tool display names and improve output formatting (#999)

GoodluckH · web-flow · commit 7cafb71b5299 · 2025-04-16T15:23:46.000-07:00
diff --git a/crates/q_cli/src/cli/chat/mod.rs b/crates/q_cli/src/cli/chat/mod.rs
@@ -2080,7 +2080,7 @@ where
                 play_notification_bell(!allowed);
             }
 
-            self.print_tool_description(tool, allowed).await?;
+            self.print_tool_descriptions(tool, allowed).await?;
 
             if allowed {
                 tool.accepted = true;
@@ -2101,22 +2101,13 @@ where
         }
 
         // Execute the requested tools.
-        let terminal_width = self.terminal_width();
         let mut tool_results = vec![];
 
         for tool in tool_uses {
             let mut tool_telemetry = self.tool_use_telemetry_events.entry(tool.id.clone());
             tool_telemetry = tool_telemetry.and_modify(|ev| ev.is_accepted = true);
 
             let tool_start = std::time::Instant::now();
-            queue!(
-                self.output,
-                style::SetForegroundColor(Color::Cyan),
-                style::Print(format!("\n{}...\n", tool.tool.display_name_action())),
-                style::SetForegroundColor(Color::DarkGrey),
-                style::Print(format!("{}\n", "▔".repeat(terminal_width))),
-                style::SetForegroundColor(Color::Reset),
-            )?;
             let invoke_result = tool.tool.invoke(&self.ctx, &mut self.output).await;
 
             if self.interactive && self.spinner.is_some() {
@@ -2131,14 +2122,18 @@ where
 
             let tool_time = std::time::Instant::now().duration_since(tool_start);
             let tool_time = format!("{}.{}", tool_time.as_secs(), tool_time.subsec_millis());
+            const CONTINUATION_LINE: &str = " ⋮ ";
 
             match invoke_result {
                 Ok(result) => {
                     debug!("tool result output: {:#?}", result);
                     execute!(
                         self.output,
+                        style::Print(CONTINUATION_LINE),
+                        style::Print("\n"),
                         style::SetForegroundColor(Color::Green),
-                        style::Print(format!("🟢 Completed in {}s", tool_time)),
+                        style::SetAttribute(Attribute::Bold),
+                        style::Print(format!(" ● Completed in {}s", tool_time)),
                         style::SetForegroundColor(Color::Reset),
                         style::Print("\n"),
                     )?;
@@ -2154,9 +2149,11 @@ where
                     error!(?err, "An error occurred processing the tool");
                     execute!(
                         self.output,
+                        style::Print(CONTINUATION_LINE),
+                        style::Print("\n"),
                         style::SetAttribute(Attribute::Bold),
                         style::SetForegroundColor(Color::Red),
-                        style::Print(format!("🔴 Execution failed after {}s:\n", tool_time)),
+                        style::Print(format!(" ● Execution failed after {}s:\n", tool_time)),
                         style::SetAttribute(Attribute::Reset),
                         style::SetForegroundColor(Color::Red),
                         style::Print(&err),
@@ -2644,25 +2641,32 @@ where
         };
     }
 
-    async fn print_tool_description(&mut self, tool_use: &QueuedTool, trusted: bool) -> Result<(), ChatError> {
-        let terminal_width = self.terminal_width();
+    async fn print_tool_descriptions(&mut self, tool_use: &QueuedTool, trusted: bool) -> Result<(), ChatError> {
+        const TOOL_BULLET: &str = " ● ";
+        const CONTINUATION_LINE: &str = " ⋮ ";
+
         queue!(
             self.output,
-            style::SetForegroundColor(Color::Green),
-            style::Print(format!("[Tool Request{}] ", if trusted { " - Trusted" } else { "" })),
-            style::SetForegroundColor(Color::Cyan),
-            style::Print(format!("{}\n", tool_use.tool.display_name())),
-            style::SetForegroundColor(Color::Reset),
-            style::SetForegroundColor(Color::DarkGrey),
-            style::Print(format!("{}\n", "▔".repeat(terminal_width))),
-            style::SetForegroundColor(Color::Reset),
+            style::SetForegroundColor(Color::Magenta),
+            style::Print(format!(
+                "🛠️  Using tool: {} {}\n",
+                tool_use.tool.display_name(),
+                if trusted { "(trusted)".dark_green() } else { "".reset() }
+            )),
+            style::SetForegroundColor(Color::Reset)
         )?;
+        queue!(self.output, style::Print(CONTINUATION_LINE))?;
+        queue!(self.output, style::Print("\n"))?;
+        queue!(self.output, style::Print(TOOL_BULLET))?;
+
+        self.output.flush()?;
+
         tool_use
             .tool
             .queue_description(&self.ctx, &mut self.output)
             .await
             .map_err(|e| ChatError::Custom(format!("failed to print tool: {}", e).into()))?;
-        queue!(self.output, style::Print("\n"))?;
+
         Ok(())
     }
 
diff --git a/crates/q_cli/src/cli/chat/tools/execute_bash.rs b/crates/q_cli/src/cli/chat/tools/execute_bash.rs
@@ -114,6 +114,7 @@ impl ExecuteBash {
             updates,
             style::SetForegroundColor(Color::Green),
             style::Print(&self.command),
+            style::Print("\n\n"),
             style::ResetColor
         )?)
     }
diff --git a/crates/q_cli/src/cli/chat/tools/fs_read.rs b/crates/q_cli/src/cli/chat/tools/fs_read.rs
@@ -341,14 +341,16 @@ impl FsDirectory {
                 break;
             }
             let relative_path = format_path(&cwd, &path);
-            queue!(
-                updates,
-                style::Print("Reading: "),
-                style::SetForegroundColor(Color::Green),
-                style::Print(&relative_path),
-                style::ResetColor,
-                style::Print("\n"),
-            )?;
+            if !relative_path.is_empty() {
+                queue!(
+                    updates,
+                    style::Print("Reading: "),
+                    style::SetForegroundColor(Color::Green),
+                    style::Print(&relative_path),
+                    style::ResetColor,
+                    style::Print("\n"),
+                )?;
+            }
             let mut read_dir = ctx.fs().read_dir(path).await?;
             while let Some(ent) = read_dir.next_entry().await? {
                 use std::os::unix::fs::MetadataExt;
diff --git a/crates/q_cli/src/cli/chat/tools/gh_issue.rs b/crates/q_cli/src/cli/chat/tools/gh_issue.rs
@@ -199,7 +199,7 @@ impl GhIssue {
     pub fn queue_description(&self, updates: &mut impl Write) -> Result<()> {
         Ok(queue!(
             updates,
-            style::Print("I will prepare a github issue with our conversation history.\n"),
+            style::Print("I will prepare a github issue with our conversation history.\n\n"),
             style::SetForegroundColor(Color::Green),
             style::Print(format!("Title: {}\n", &self.title)),
             style::ResetColor
diff --git a/crates/q_cli/src/cli/chat/tools/mod.rs b/crates/q_cli/src/cli/chat/tools/mod.rs
@@ -48,26 +48,14 @@ impl Tool {
     /// The display name of a tool
     pub fn display_name(&self) -> &'static str {
         match self {
-            Tool::FsRead(_) => "Read from filesystem",
-            Tool::FsWrite(_) => "Write to filesystem",
-            Tool::ExecuteBash(_) => "Execute shell command",
-            Tool::UseAws(_) => "Use AWS CLI",
-            Tool::GhIssue(_) => "Prepare GitHub issue",
+            Tool::FsRead(_) => "fs_read",
+            Tool::FsWrite(_) => "fs_write",
+            Tool::ExecuteBash(_) => "execute_bash",
+            Tool::UseAws(_) => "use_aws",
+            Tool::GhIssue(_) => "gh_issue",
         }
     }
 
-    // TODO: Remove, just roll with it for now ya?
-    pub fn display_name_action(&self) -> String {
-        match self {
-            Tool::FsRead(_) => "Reading from filesystem",
-            Tool::FsWrite(_) => "Writing to filesystem",
-            Tool::ExecuteBash(execute_bash) => return format!("Executing `{}`", execute_bash.command),
-            Tool::UseAws(_) => "Using AWS CLI",
-            Tool::GhIssue(_) => "Preparing GitHub issue",
-        }
-        .to_owned()
-    }
-
     /// Whether or not the tool should prompt the user to accept before [Self::invoke] is called.
     pub fn requires_acceptance(&self, _ctx: &Context) -> bool {
         match self {
diff --git a/crates/q_cli/src/cli/chat/tools/use_aws.rs b/crates/q_cli/src/cli/chat/tools/use_aws.rs
@@ -106,7 +106,7 @@ impl UseAws {
     pub fn queue_description(&self, updates: &mut impl Write) -> Result<()> {
         queue!(
             updates,
-            style::Print("Running aws cli command:\n"),
+            style::Print("Running aws cli command:\n\n"),
             style::Print(format!("Service name: {}\n", self.service_name)),
             style::Print(format!("Operation name: {}\n", self.operation_name)),
         )?;
diff --git a/crates/q_cli/src/cli/issue.rs b/crates/q_cli/src/cli/issue.rs
@@ -32,6 +32,8 @@ pub struct IssueCreator {
 
 impl IssueCreator {
     pub async fn create_url(&self) -> Result<url::Url> {
+        println!("Heading over to GitHub...");
+
         let warning = |text: &String| {
             format!("<This will be visible to anyone. Do not include personal or sensitive information>\n\n{text}")
         };
@@ -78,7 +80,6 @@ impl IssueCreator {
             params.iter(),
         )?;
 
-        println!("Heading over to GitHub...");
         if is_remote() || fig_util::open_url_async(url.as_str()).await.is_err() {
             println!("Issue Url: {}", url.as_str().underlined());
         }

Original file line number	Diff line number	Diff line change
`@@ -114,6 +114,7 @@ impl ExecuteBash {`
`114`	`114`	`updates,`
`115`	`115`	`style::SetForegroundColor(Color::Green),`
`116`	`116`	`style::Print(&self.command),`
	`117`	`+ style::Print("\n\n"),`
`117`	`118`	`style::ResetColor`
`118`	`119`	`)?)`
`119`	`120`	`}`