Skip to content

Commit 01814b3

Browse files
author
kiran-garre
committed
fixed most clippy warnings, added tool-level snapshot display
Updates: - Snapshot indices are displayed when modifying tools finish running, e.g. `Completed in 0.2s [2.1]` - Snapshot timestamps are now displayed as y-m-d h:m:s
1 parent d5595a1 commit 01814b3

File tree

4 files changed

+29
-31
lines changed

4 files changed

+29
-31
lines changed

‎crates/chat-cli/src/cli/chat/cli/snapshot.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ impl SnapshotSubcommand {
7070
Ok(_) => {
7171
execute!(
7272
session.stderr,
73-
style::Print(format!("Created initial snapshot: 1\n").blue().bold())
73+
style::Print("Created initial snapshot: 1\n".blue().bold())
7474
)?;
7575
},
7676
Err(e) => {
@@ -134,7 +134,7 @@ impl SnapshotSubcommand {
134134
match SnapshotManager::clean_all(os).await {
135135
Ok(_) => execute!(
136136
session.stderr,
137-
style::Print(format!("Deleted shadow repository\n").blue().bold())
137+
style::Print("Deleted shadow repository\n".blue().bold())
138138
)?,
139139
Err(e) => {
140140
return Err(ChatError::Custom(
@@ -178,8 +178,8 @@ pub fn list_snapshots(manager: &SnapshotManager, output: &mut impl Write, limit:
178178
execute!(
179179
output,
180180
style::Print(format!("[{}]", i + 1).blue()),
181-
style::Print(format!(" {} - {}\n", snapshot.timestamp, snapshot.message))
182-
)?
181+
style::Print(format!(" {} - {}\n", snapshot.timestamp.format("%Y-%m-%d %H:%M:%S"), snapshot.message)),
182+
)?;
183183
}
184184
Ok(())
185185
}
@@ -221,7 +221,7 @@ impl FromStr for Index {
221221
let outer_idx = outer.parse::<usize>()
222222
.map_err(|_| ChatError::Custom(format!("Invalid checkpoint idx: {outer}").into()))?;
223223
let inner_idx = inner.parse::<usize>()
224-
.map_err(|_| ChatError::Custom(format!("Invalid checkpoint idx: {outer}").into()))?;
224+
.map_err(|_| ChatError::Custom(format!("Invalid checkpoint idx: {inner}").into()))?;
225225

226226
// Convert from 1-based to 0-based indexing
227227
Ok(Self::Nested(outer_idx - 1, inner_idx - 1))

‎crates/chat-cli/src/cli/chat/conversation.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -515,11 +515,11 @@ impl ConversationState {
515515
FILTER OUT CHAT CONVENTIONS (greetings, offers to help, etc)."
516516
.to_string();
517517

518-
let conv_state = self.backend_conversation_state(os, false, &mut vec![]).await?;
519-
let summary_message = Some(UserMessage::new_prompt(summary_content.clone()));
518+
let mut conv_state = self.backend_conversation_state(os, false, &mut vec![]).await?;
519+
let summary_message = UserMessage::new_prompt(summary_content.clone());
520520

521521
// Create the history according to the passed compact strategy.
522-
let history = conv_state.history.cloned().last();
522+
let history = conv_state.history.next_back().cloned();
523523

524524
// Only send the dummy tool spec in order to prevent the model from ever attempting a tool
525525
// use.
@@ -537,7 +537,6 @@ impl ConversationState {
537537
Ok(FigConversationState {
538538
conversation_id: Some(self.conversation_id.clone()),
539539
user_input_message: summary_message
540-
.unwrap_or(UserMessage::new_prompt(summary_content)) // should not happen
541540
.into_user_input_message(self.model.clone(), &tools),
542541
history: Some(flatten_history(history.iter())),
543542
})

‎crates/chat-cli/src/cli/chat/mod.rs

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1742,7 +1742,7 @@ impl ChatSession {
17421742
let tool_time = format!("{}.{}", tool_time.as_secs(), tool_time.subsec_millis());
17431743
match invoke_result {
17441744
Ok(result) => {
1745-
let mut tracked_tool_use: Option<bool> = Some(false);
1745+
let mut tracked_tool_use: Option<(usize, usize)> = None;
17461746
// Track tool uses for snapshots
17471747
match tool.tool {
17481748
Tool::FsWrite(_) | Tool::ExecuteCommand(_) => {
@@ -1757,7 +1757,7 @@ impl ChatSession {
17571757
if let Some(manager) = &mut self.conversation.snapshot_manager {
17581758
if manager.any_modified(os).await.unwrap_or(false) {
17591759
match manager.track_tool_use(os, &tool.tool.display_name(), purpose, history_index).await {
1760-
Ok(_) => tracked_tool_use = Some(true),
1760+
Ok(_) => tracked_tool_use = Some((manager.snapshot_count + 1, manager.tool_use_buffer.len())),
17611761
Err(_) => tracked_tool_use = None,
17621762
};
17631763
}
@@ -1789,17 +1789,16 @@ impl ChatSession {
17891789
style::Print("\n"),
17901790
style::SetForegroundColor(Color::Green),
17911791
style::SetAttribute(Attribute::Bold),
1792-
style::Print(format!(" ● Completed in {}s", tool_time)),
1792+
style::Print(format!(" ● Completed in {}s ", tool_time)),
17931793
style::SetForegroundColor(Color::Reset),
1794-
style::Print("\n\n"),
17951794
)?;
17961795

1797-
match tracked_tool_use {
1798-
Some(true) => execute!(self.stderr, style::Print("Tracked tool use!\n".blue()),)?,
1799-
Some(false) => (),
1800-
None => execute!(self.stderr, style::Print("Could not track tool use\n".blue()),)?,
1796+
if let Some((outer, inner)) = tracked_tool_use {
1797+
execute!(self.stderr, style::Print(format!("[{outer}.{inner}]\n").blue()))?;
18011798
};
18021799

1800+
execute!(self.stdout, style::Print("\n\n"))?;
1801+
18031802
tool_telemetry = tool_telemetry.and_modify(|ev| ev.is_success = Some(true));
18041803
if let Tool::Custom(_) = &tool.tool {
18051804
tool_telemetry

‎crates/chat-cli/src/cli/chat/snapshot.rs

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::collections::HashMap;
22
use std::fmt::Debug;
3-
use std::mem::replace;
3+
use std::mem::take;
44
use std::path::{
55
Path,
66
PathBuf,
@@ -153,7 +153,7 @@ impl SnapshotManager {
153153
let mut index = self.repo.index()?;
154154
let cwd = os.env.current_dir()?;
155155

156-
let ignores = RegexSet::new(&[r".git"])?;
156+
let ignores = RegexSet::new([r".git"])?;
157157
let comparison = Comparison::new(ignores);
158158
let res = comparison.compare(SHADOW_REPO_DIR, os.env.current_dir()?.to_str().unwrap())?;
159159

@@ -162,11 +162,11 @@ impl SnapshotManager {
162162
if shadow_path.is_file() {
163163
let cwd_path = convert_path(SHADOW_REPO_DIR, shadow_path, &cwd)?;
164164
self.modified_map
165-
.insert(cwd_path.to_path_buf(), get_modified_timestamp(os, &cwd_path).await?);
165+
.insert(cwd_path.clone(), get_modified_timestamp(os, &cwd_path).await?);
166166
copy_file_to_dir(os, &cwd, cwd_path, SHADOW_REPO_DIR).await?;
167167

168168
// Staging requires relative paths
169-
index.add_path(&shadow_path.strip_prefix(SHADOW_REPO_DIR)?)?;
169+
index.add_path(shadow_path.strip_prefix(SHADOW_REPO_DIR)?)?;
170170
}
171171
}
172172

@@ -175,10 +175,10 @@ impl SnapshotManager {
175175
copy_file_to_dir(os, &cwd, cwd_path, SHADOW_REPO_DIR).await?;
176176
if cwd_path.is_file() {
177177
self.modified_map
178-
.insert(cwd_path.to_path_buf(), get_modified_timestamp(os, cwd_path).await?);
178+
.insert(cwd_path.clone(), get_modified_timestamp(os, cwd_path).await?);
179179

180180
// Staging requires relative paths
181-
index.add_path(&cwd_path.strip_prefix(&cwd)?)?;
181+
index.add_path(cwd_path.strip_prefix(&cwd)?)?;
182182
}
183183
}
184184

@@ -195,7 +195,7 @@ impl SnapshotManager {
195195

196196
// Update table and shadow repo if deleted
197197
// FIX: removing the entry is probably not the best choice?
198-
self.modified_map.remove(&shadow_path.to_path_buf());
198+
self.modified_map.remove(&shadow_path.clone());
199199
os.fs.remove_file(shadow_path).await?;
200200

201201
// Staging requires relative paths
@@ -228,21 +228,21 @@ impl SnapshotManager {
228228
&signature,
229229
message,
230230
&tree,
231-
&parents.iter().map(|c| c).collect::<Vec<_>>(),
231+
&parents.iter().collect::<Vec<_>>(),
232232
)?;
233233

234234
if turn {
235235
// Assign tool uses to the turn snapshot they belong to
236236
let tool_snapshots = if !self.tool_use_buffer.is_empty() {
237-
replace(&mut self.tool_use_buffer, Vec::new())
237+
take(&mut self.tool_use_buffer)
238238
} else {
239239
Vec::new()
240240
};
241241

242242
self.snapshot_table.push(Snapshot {
243243
timestamp: Local::now(),
244244
message: message.to_string(),
245-
history_index: history_index,
245+
history_index,
246246
tool_snapshots,
247247
});
248248
self.oid_table.push(oid);
@@ -283,7 +283,7 @@ impl SnapshotManager {
283283
self.reset_hard(&oid.to_string()).await?;
284284

285285
let cwd = os.env.current_dir()?;
286-
let ignores = RegexSet::new(&[r".git"])?;
286+
let ignores = RegexSet::new([r".git"])?;
287287
let comparison = Comparison::new(ignores);
288288
let res = comparison.compare(SHADOW_REPO_DIR, cwd.to_str().unwrap())?;
289289

@@ -293,7 +293,7 @@ impl SnapshotManager {
293293
let cwd_path = convert_path(SHADOW_REPO_DIR, shadow_path, &cwd)?;
294294
copy_file_to_dir(os, SHADOW_REPO_DIR, shadow_path, &cwd).await?;
295295
self.modified_map
296-
.insert(cwd_path.to_path_buf(), get_modified_timestamp(os, &cwd_path).await?);
296+
.insert(cwd_path.clone(), get_modified_timestamp(os, &cwd_path).await?);
297297
}
298298
}
299299

@@ -302,7 +302,7 @@ impl SnapshotManager {
302302
let cwd_path = convert_path(SHADOW_REPO_DIR, shadow_path, &cwd)?;
303303
copy_file_to_dir(os, SHADOW_REPO_DIR, shadow_path, &cwd).await?;
304304
self.modified_map
305-
.insert(cwd_path.to_path_buf(), get_modified_timestamp(os, &cwd_path).await?);
305+
.insert(cwd_path.clone(), get_modified_timestamp(os, &cwd_path).await?);
306306
}
307307

308308
// Delete extra files
@@ -377,7 +377,7 @@ async fn copy_file_to_dir(
377377
destination: impl AsRef<Path>,
378378
) -> Result<()> {
379379
let path = path.as_ref();
380-
let target_path = convert_path(prefix, &path, destination)?;
380+
let target_path = convert_path(prefix, path, destination)?;
381381
if path.is_dir() && !os.fs.exists(&target_path) {
382382
os.fs.create_dir_all(target_path).await?;
383383
} else if path.is_file() {

0 commit comments

Comments
 (0)