Skip to content

Commit c193794

Browse files
fix: various issues with display and bugs in chat (#663)
1 parent 1016ed2 commit c193794

File tree

7 files changed

+98
-25
lines changed

7 files changed

+98
-25
lines changed

crates/q_cli/src/cli/chat/mod.rs

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -514,6 +514,7 @@ Hi, I'm <g>Amazon Q</g>. Ask me anything.
514514
style::SetForegroundColor(Color::Reset),
515515
style::SetForegroundColor(Color::DarkGrey),
516516
style::Print(format!("{}\n", "▔".repeat(terminal_width))),
517+
style::SetForegroundColor(Color::Reset),
517518
)?;
518519
tool.queue_description(&self.ctx, self.output)?;
519520
queue!(self.output, style::Print("\n"))?;
@@ -551,6 +552,7 @@ Hi, I'm <g>Amazon Q</g>. Ask me anything.
551552
// Tool execution.
552553
c if c.to_lowercase() == "y" && !queued_tools.is_empty() => {
553554
// Execute the requested tools.
555+
let terminal_width = self.terminal_width();
554556
let mut tool_results = vec![];
555557
for tool in queued_tools.drain(..) {
556558
let corresponding_builder = self.tool_use_events.iter_mut().find(|v| {
@@ -560,9 +562,31 @@ Hi, I'm <g>Amazon Q</g>. Ask me anything.
560562
false
561563
}
562564
});
563-
match tool.1.invoke(&self.ctx, self.output).await {
565+
566+
let tool_start = std::time::Instant::now();
567+
queue!(
568+
self.output,
569+
style::Print("\n"),
570+
style::Print("▔".repeat(terminal_width)),
571+
style::Print("\nExecuting "),
572+
style::SetForegroundColor(Color::Cyan),
573+
style::Print(format!("{}...\n\n", tool.1.display_name())),
574+
style::SetForegroundColor(Color::Reset),
575+
)?;
576+
let invoke_result = tool.1.invoke(&self.ctx, self.output).await;
577+
let tool_time = std::time::Instant::now().duration_since(tool_start);
578+
let tool_time = format!("{}.{}", tool_time.as_secs(), tool_time.subsec_millis());
579+
580+
match invoke_result {
564581
Ok(result) => {
565582
debug!("tool result output: {:#?}", result);
583+
queue!(
584+
self.output,
585+
style::SetForegroundColor(Color::Green),
586+
style::Print(format!("🟢 Completed in {}s", tool_time)),
587+
style::SetForegroundColor(Color::Reset),
588+
style::Print("\n\n"),
589+
)?;
566590
if let Some(builder) = corresponding_builder {
567591
builder.is_success = Some(true);
568592
}
@@ -587,11 +611,13 @@ Hi, I'm <g>Amazon Q</g>. Ask me anything.
587611
execute!(
588612
self.output,
589613
style::SetAttribute(Attribute::Bold),
590-
style::Print("Tool execution failed: "),
614+
style::SetForegroundColor(Color::Red),
615+
style::Print(format!("🔴 Execution failed after {}s:\n", tool_time)),
591616
style::SetAttribute(Attribute::Reset),
592617
style::SetForegroundColor(Color::Red),
593618
style::Print(err),
594-
style::SetForegroundColor(Color::Reset)
619+
style::SetAttribute(Attribute::Reset),
620+
style::Print("\n\n"),
595621
)?;
596622

597623
if let Some(builder) = corresponding_builder {

crates/q_cli/src/cli/chat/tools/execute_bash.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -72,19 +72,19 @@ impl ExecuteBash {
7272
}
7373

7474
pub fn queue_description(&self, updates: &mut impl Write) -> Result<()> {
75-
queue!(
76-
updates,
77-
style::Print("I will run the following shell command: "),
78-
style::SetForegroundColor(Color::Green),
79-
style::Print(&self.command),
80-
)?;
75+
queue!(updates, style::Print("I will run the following shell command: "),)?;
8176

8277
// TODO: Could use graphemes for a better heuristic
8378
if self.command.len() > 20 {
8479
queue!(updates, style::Print("\n"),)?;
8580
}
8681

87-
Ok(queue!(updates, style::Print(&self.command), style::ResetColor)?)
82+
Ok(queue!(
83+
updates,
84+
style::SetForegroundColor(Color::Green),
85+
style::Print(&self.command),
86+
style::ResetColor
87+
)?)
8888
}
8989

9090
pub async fn validate(&mut self, _ctx: &Context) -> Result<()> {

crates/q_cli/src/cli/chat/tools/fs_read.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,15 +88,15 @@ impl FsRead {
8888
style::Print("\n"),
8989
)?;
9090

91-
let formatted = stylize_output_if_able(&path, file_contents.as_str(), Some(start + 1), None);
91+
let formatted = stylize_output_if_able(ctx, &path, file_contents.as_str(), Some(start + 1), None);
9292
queue!(updates, style::Print(formatted), style::ResetColor, style::Print("\n"))?;
9393
return Ok(InvokeOutput {
9494
output: OutputKind::Text(file_contents),
9595
});
9696
}
9797

9898
let file = ctx.fs().read_to_string(&path).await?;
99-
let file_text = stylize_output_if_able(path, file.as_str(), None, None);
99+
let file_text = stylize_output_if_able(ctx, path, file.as_str(), None, None);
100100
queue!(
101101
updates,
102102
style::SetForegroundColor(Color::Green),

crates/q_cli/src/cli/chat/tools/fs_write.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,8 @@ impl FsWrite {
131131
)?;
132132
if ctx.fs().exists(path) {
133133
let prev = ctx.fs().read_to_string_sync(path)?;
134-
let prev = stylize_output_if_able(&relative_path, prev.as_str(), None, Some("-"));
135-
let new = stylize_output_if_able(&relative_path, file_text, None, Some("+"));
134+
let prev = stylize_output_if_able(ctx, &relative_path, prev.as_str(), None, Some("-"));
135+
let new = stylize_output_if_able(ctx, &relative_path, file_text, None, Some("+"));
136136
queue!(
137137
updates,
138138
style::Print("Replacing:\n"),
@@ -145,7 +145,7 @@ impl FsWrite {
145145
style::Print("\n\n")
146146
)?;
147147
} else {
148-
let file = stylize_output_if_able(&relative_path, file_text, None, None);
148+
let file = stylize_output_if_able(ctx, &relative_path, file_text, None, None);
149149
queue!(
150150
updates,
151151
style::Print("Contents:\n"),
@@ -161,7 +161,7 @@ impl FsWrite {
161161
new_str,
162162
} => {
163163
let path = format_path(cwd, path);
164-
let file = stylize_output_if_able(&path, new_str, Some(*insert_line), Some("+"));
164+
let file = stylize_output_if_able(ctx, &path, new_str, Some(*insert_line), Some("+"));
165165
queue!(
166166
updates,
167167
style::Print("Path: "),
@@ -182,8 +182,8 @@ impl FsWrite {
182182
Some((start_line, end_line)) => (Some(start_line), Some(end_line)),
183183
_ => (None, None),
184184
};
185-
let old_str = stylize_output_if_able(&path, old_str, start_line, Some("-"));
186-
let new_str = stylize_output_if_able(&path, new_str, start_line, Some("+"));
185+
let old_str = stylize_output_if_able(ctx, &path, old_str, start_line, Some("-"));
186+
let new_str = stylize_output_if_able(ctx, &path, new_str, start_line, Some("+"));
187187
queue!(
188188
updates,
189189
style::Print("Path: "),

crates/q_cli/src/cli/chat/tools/mod.rs

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,10 @@ use syntect::util::{
3636
LinesWithEndings,
3737
as_24_bit_terminal_escaped,
3838
};
39-
use tracing::error;
39+
use tracing::{
40+
error,
41+
warn,
42+
};
4043
use use_aws::UseAws;
4144

4245
use super::parser::ToolUse;
@@ -222,18 +225,24 @@ fn terminal_width(line_count: usize) -> usize {
222225
}
223226

224227
fn stylize_output_if_able(
228+
ctx: &Context,
225229
path: impl AsRef<Path>,
226230
file_text: &str,
227231
starting_line: Option<usize>,
228232
gutter_prefix: Option<&str>,
229233
) -> String {
230-
match stylized_file(path, file_text, starting_line, gutter_prefix) {
231-
Ok(s) => s,
232-
Err(err) => {
233-
error!(?err, "unable to syntax highlight the output");
234-
format!("\n{}", nonstylized_file(file_text))
234+
match ctx.env().get("COLORTERM") {
235+
Ok(s) if s == "truecolor" => match stylized_file(path, file_text, starting_line, gutter_prefix) {
236+
Ok(s) => return s,
237+
Err(err) => {
238+
error!(?err, "unable to syntax highlight the output");
239+
},
240+
},
241+
_ => {
242+
warn!("24bit color is not supported, falling back to nonstylized syntax highlighting");
235243
},
236244
}
245+
format!("\n{}", nonstylized_file(file_text))
237246
}
238247

239248
fn nonstylized_file(file_text: impl AsRef<str>) -> String {

crates/q_cli/src/cli/chat/tools/tool_index.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
"description": "Whether or not the command is interactive. Interactive commands like nano will overtake our conversation until exited. On exit, they will have produced no stderr or stdout."
1515
}
1616
},
17-
"required": ["command"]
17+
"required": ["command", "interactive"]
1818
}
1919
},
2020
{
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
[
2+
[
3+
"I will use the `fs_write` tool.",
4+
{
5+
"tool_use_id": "1",
6+
"name": "fs_write",
7+
"args": {
8+
"command": "create",
9+
"path": "stub_output/unknown",
10+
"file_text": "#[derive(Debug, Clone, Copy)]\npub struct Vec3 {\n pub x: f32,\n pub y: f32,\n pub z: f32,\n}\n\nimpl Vec3 {\n pub fn new(x: f32, y: f32, z: f32) -> Self {\n Vec3 { x, y, z }\n }\n\n pub fn min(&self, other: &Vec3) -> Vec3 {\n Vec3 {\n x: self.x.min(other.x),\n y: self.y.min(other.y),\n z: self.z.min(other.z),\n }\n }\n\n pub fn max(&self, other: &Vec3) -> Vec3 {\n Vec3 {\n x: self.x.max(other.x),\n y: self.y.max(other.y),\n z: self.z.max(other.z),\n }\n }\n}\n\n#[derive(Debug)]\npub struct Ray {\n pub origin: Vec3,\n pub direction: Vec3,\n}\n\nimpl Ray {\n pub fn new(origin: Vec3, direction: Vec3) -> Self {\n Ray { origin, direction }\n }\n}\n\n#[derive(Debug)]\npub struct Cube {\n pub min: Vec3,\n pub max: Vec3,\n}\n\nimpl Cube {\n pub fn new(min: Vec3, max: Vec3) -> Self {\n Cube { min, max }\n }\n\n // Returns (t_min, t_max) for intersection. If t_min > t_max, there is no intersection\n pub fn intersect(&self, ray: &Ray) -> (f32, f32) {\n let mut t_min = f32::NEG_INFINITY;\n let mut t_max = f32::INFINITY;\n\n // Check intersection with all three slabs\n for i in 0..3 {\n let (origin, direction) = match i {\n 0 => (ray.origin.x, ray.direction.x),\n 1 => (ray.origin.y, ray.direction.y),\n 2 => (ray.origin.z, ray.direction.z),\n _ => unreachable!(),\n };\n\n let (min_val, max_val) = match i {\n 0 => (self.min.x, self.max.x),\n 1 => (self.min.y, self.max.y),\n 2 => (self.min.z, self.max.z),\n _ => unreachable!(),\n };\n\n if direction.abs() < f32::EPSILON {\n // Ray is parallel to slab. No hit if origin not within slab\n if origin < min_val || origin > max_val {\n return (1.0, -1.0); // No intersection\n }\n } else {\n // Compute intersection t value of ray with near and far plane of slab\n let t1 = (min_val - origin) / direction;\n let t2 = (max_val - origin) / direction;\n\n // Make t1 be intersection with near plane, t2 with far plane\n let (t1, t2) = if t1 > t2 { (t2, t1) } else { (t1, t2) };\n\n // Update t_min and t_max\n t_min = t_min.max(t1);\n t_max = t_max.min(t2);\n\n // Exit with no collision as soon as slab intersection becomes empty\n if t_min > t_max {\n return (1.0, -1.0);\n }\n }\n }\n\n (t_min, t_max)\n }\n}\n\n#[cfg(test)]\nmod tests {\n use super::*;\n\n #[test]\n fn test_ray_cube_intersection() {\n // Create a cube centered at origin with size 2\n let cube = Cube::new(\n Vec3::new(-1.0, -1.0, -1.0),\n Vec3::new(1.0, 1.0, 1.0),\n );\n\n // Ray starting outside the cube and pointing towards it\n let ray = Ray::new(\n Vec3::new(-2.0, 0.0, 0.0),\n Vec3::new(1.0, 0.0, 0.0),\n );\n\n let (t_min, t_max) = cube.intersect(&ray);\n assert!(t_min <= t_max);\n assert_eq!(t_min, 1.0); // Should hit at x = -1\n assert_eq!(t_max, 3.0); // Should exit at x = 1\n\n // Ray missing the cube\n let ray_miss = Ray::new(\n Vec3::new(-2.0, 2.0, 0.0),\n Vec3::new(1.0, 0.0, 0.0),\n );\n\n let (t_min, t_max) = cube.intersect(&ray_miss);\n assert!(t_min > t_max); // Indicates no intersection\n }\n}\n\n"
11+
}
12+
}
13+
],
14+
[
15+
{
16+
"tool_use_id": "2",
17+
"name": "fs_write",
18+
"args": {
19+
"command": "insert",
20+
"path": "stub_output/unknown",
21+
"insert_line": 1,
22+
"new_str": "use std::os::env;\n\n"
23+
}
24+
}
25+
],
26+
[
27+
{
28+
"tool_use_id": "3",
29+
"name": "fs_write",
30+
"args": {
31+
"command": "str_replace",
32+
"path": "stub_output/unknown",
33+
"old_str": " // Ray missing the cube\n let ray_miss = Ray::new(\n Vec3::new(-2.0, 2.0, 0.0),\n Vec3::new(1.0, 0.0, 0.0),\n );\n\n",
34+
"new_str": " if ray.has_more_data() {\n bail!(\"Unexpected more data\");\n }\n\n\n"
35+
}
36+
}
37+
]
38+
]

0 commit comments

Comments
 (0)