Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 32 additions & 15 deletions crates/forge_app/src/fmt/fmt_input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,20 @@ impl FormatContent for ToolCatalog {
let path = PathBuf::from(&input.file_path);
let display_path = display_path_for(&input.file_path);
let title = match (path.exists(), input.overwrite) {
(true, true) => "Overwrite",
(true, true) => format!("Overwrite: {display_path}"),
(true, false) => {
// Case: file exists but overwrite is false then we throw error from tool,
// so it's good idea to not print anything on CLI.
return None;
}
(false, _) => "Create",
(false, _) => format!("Create: {display_path}"),
};
Some(TitleFormat::debug(title).sub_title(display_path).into())
// Show the content inline so the user has context about what will be
// written before any permission prompt
let body = if input.content.is_empty() {
"[empty content]".to_string()
} else {
format!("{title}\n\n{}", input.content)
};
Some(ChatResponseContent::ToolOutput(body))
}
ToolCatalog::FsSearch(input) => {
let formatted_dir = input.path.as_deref().unwrap_or(".");
Expand Down Expand Up @@ -90,19 +95,31 @@ impl FormatContent for ToolCatalog {
} else {
"Replace"
};
Some(
TitleFormat::debug(operation_name)
.sub_title(display_path)
.into(),
)
let body = format!(
"{operation_name}: {display_path}\n\n--- old string ---\n{}\n--- new string ---\n{}",
input.old_string, input.new_string
);
Some(ChatResponseContent::ToolOutput(body))
}
ToolCatalog::MultiPatch(input) => {
let display_path = display_path_for(&input.file_path);
Some(
TitleFormat::debug("Replace")
.sub_title(format!("{} ({} edits)", display_path, input.edits.len()))
.into(),
)
let edits: Vec<String> = input
.edits
.iter()
.map(|e| {
format!(
"- Replace \"{}…\" → \"{}…\"{}",
&e.old_string[..e.old_string.len().min(60)],
&e.new_string[..e.new_string.len().min(60)],
if e.replace_all { " (all occurrences)" } else { "" }
)
})
.collect();
Some(ChatResponseContent::ToolOutput(format!(
"Replace: {display_path} ({} edit(s))\n\n{}",
input.edits.len(),
edits.join("\n")
)))
}
ToolCatalog::Undo(input) => {
let display_path = display_path_for(&input.path);
Expand Down
5 changes: 5 additions & 0 deletions crates/forge_app/src/fmt/fmt_output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ mod tests {
input: forge_domain::FSWrite {
file_path: "/home/user/project/new_file.txt".to_string(),
content: content.to_string(),
context: String::new(),
overwrite: false,
},
output: FsWriteOutput {
Expand All @@ -157,6 +158,7 @@ mod tests {
input: forge_domain::FSWrite {
file_path: "/home/user/project/existing_file.txt".to_string(),
content: content.to_string(),
context: String::new(),
overwrite: true,
},
output: FsWriteOutput {
Expand Down Expand Up @@ -185,6 +187,7 @@ mod tests {
input: forge_domain::FSWrite {
file_path: "/home/user/project/file.txt".to_string(),
content: content.to_string(),
context: String::new(),
overwrite: false,
},
output: FsWriteOutput {
Expand Down Expand Up @@ -304,6 +307,7 @@ mod tests {
file_path: "/home/user/project/test.txt".to_string(),
old_string: "Hello world".to_string(),
new_string: "Hello universe".to_string(),
context: String::new(),
replace_all: false,
},
output: PatchOutput {
Expand All @@ -327,6 +331,7 @@ mod tests {
file_path: "/home/user/project/large_file.txt".to_string(),
old_string: "line2".to_string(),
new_string: "new line\nline2".to_string(),
context: String::new(),
replace_all: false,
},
output: PatchOutput {
Expand Down
7 changes: 7 additions & 0 deletions crates/forge_app/src/operation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -982,6 +982,7 @@ mod tests {
input: forge_domain::FSWrite {
file_path: "/home/user/new_file.txt".to_string(),
content: content.to_string(),
context: String::new(),
overwrite: false,
},
output: FsWriteOutput {
Expand Down Expand Up @@ -1013,6 +1014,7 @@ mod tests {
input: forge_domain::FSWrite {
file_path: "/home/user/existing_file.txt".to_string(),
content: content.to_string(),
context: String::new(),
overwrite: true,
},
output: FsWriteOutput {
Expand Down Expand Up @@ -1518,6 +1520,7 @@ mod tests {
input: forge_domain::FSWrite {
file_path: "/home/user/file_with_warning.txt".to_string(),
content: content.to_string(),
context: String::new(),
overwrite: false,
},
output: FsWriteOutput {
Expand Down Expand Up @@ -1549,6 +1552,7 @@ mod tests {
input: forge_domain::FSWrite {
file_path: "/home/user/file_with_warning.txt".to_string(),
content: content.to_string(),
context: String::new(),
overwrite: false,
},
output: FsWriteOutput {
Expand Down Expand Up @@ -2083,6 +2087,7 @@ mod tests {
file_path: "/home/user/test.txt".to_string(),
old_string: "world".to_string(),
new_string: "universe".to_string(),
context: String::new(),
replace_all: false,
},
output: PatchOutput {
Expand Down Expand Up @@ -2115,6 +2120,7 @@ mod tests {
file_path: "/home/user/large_file.txt".to_string(),
old_string: "line1".to_string(),
new_string: "\nnew line".to_string(),
context: String::new(),
replace_all: false,
},
output: PatchOutput {
Expand Down Expand Up @@ -2147,6 +2153,7 @@ mod tests {
file_path: "/home/user/test.zsh".to_string(),
old_string: "line1".to_string(),
new_string: "\nnew line".to_string(),
context: String::new(),
replace_all: false,
},
output: PatchOutput {
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@ Use the following summary frames as the authoritative reference for all coding s
````
You're right - the call_id should be optional. Let me fix that:
````
**Update:** `crates/forge_domain/src/compact/summary.rs`
**MCP:** `patch`
````
Now update the helper methods to accept optional call_id:
````
**Update:** `crates/forge_domain/src/compact/summary.rs`
**MCP:** `patch`
````
Now update the `From<&TextMessage>` implementation to handle optional call_id:
````
**Update:** `crates/forge_domain/src/compact/summary.rs`
**MCP:** `patch`
````
Now let's run the tests to verify everything works:
````
Expand All @@ -45,11 +45,11 @@ I see the problem. The tests are still using the old API. Let me read the file f
````
Now I need to fix the HashMap lookup issue on line 129. The problem is that `tool_call_id` is `Option<ToolCallId>` but the HashMap key is `&ToolCallId`. Let me fix that:
````
**Update:** `crates/forge_domain/src/compact/summary.rs`
**MCP:** `patch`
````
Now I need to fix the helper functions and tests. Let me update them to work with the new enum-based API:
````
**Update:** `crates/forge_domain/src/compact/summary.rs`
**MCP:** `patch`
````
Now let me read the test section and fix the remaining tests:
````
Expand All @@ -64,7 +64,7 @@ wc -l crates/forge_domain/src/compact/summary.rs
````
Let me remove the old test functions that use the old API and keep only the ones we need:
````
**Update:** `crates/forge_domain/src/compact/summary.rs`
**MCP:** `patch`
````
Now let me read more of the test file to see what else needs fixing:
````
Expand All @@ -76,35 +76,35 @@ I see there are many tests that use the old struct-based API. Let me fix all the
````
I see the issue - all these tests are using the old struct-based API with `.default()` and setters. Since we changed to an enum-based design, these tests need to be completely rewritten. Let me replace all the problematic tests:
````
**Update:** `crates/forge_domain/src/compact/summary.rs`
**MCP:** `patch`
````
Now let me fix the remaining tests. Let me continue with the next batch:
````
**Update:** `crates/forge_domain/src/compact/summary.rs`
**MCP:** `patch`
````
Now let me fix the tests that use `block_read_with_content` and `block_update_with_content`:
````
**Update:** `crates/forge_domain/src/compact/summary.rs`
**MCP:** `patch`
````
Now let me fix the next test with multiple tool results:
````
**Update:** `crates/forge_domain/src/compact/summary.rs`
**MCP:** `patch`
````
Now let me fix the next test:
````
**Update:** `crates/forge_domain/src/compact/summary.rs`
**MCP:** `patch`
````
Now let me fix the complex conversation test:
````
**Update:** `crates/forge_domain/src/compact/summary.rs`
**MCP:** `patch`
````
Now let me remove the last test that uses the old API:
````
**Read:** `crates/forge_domain/src/compact/summary.rs`
````
Let me remove the last test that uses the old struct-based API:
````
**Update:** `crates/forge_domain/src/compact/summary.rs`
**MCP:** `patch`
````
Now let me run the tests again to see if there are any remaining issues:
````
Expand All @@ -119,7 +119,7 @@ There are still compilation errors in other files that use the old API. Let me c
````
I see - there are other transformer files that also use the old API. Let me fix this file:
````
**Update:** `crates/forge_domain/src/compact/transformers/drop_role.rs`
**MCP:** `patch`
````
Good! Now let me check the other files that have compilation errors. Let me search for files that use `SummaryMessageBlock`:
````
Expand All @@ -131,7 +131,7 @@ Let me check and fix those files:
````
Let me fix this file:
````
**Update:** `crates/forge_domain/src/compact/transformers/keep_first_user_message.rs`
**MCP:** `patch`
````
Now let me check the trim_context_summary.rs file:
````
Expand All @@ -151,11 +151,11 @@ This is a complex refactoring. Let me systematically fix the `trim_context_summa
````
This file needs significant refactoring. Let me create a comprehensive fix. I'll need to update the implementation to work with the enum-based API:
````
**Update:** `crates/forge_domain/src/compact/transformers/trim_context_summary.rs`
**MCP:** `patch`
````
Good! Now let me fix the test helpers:
````
**Update:** `crates/forge_domain/src/compact/transformers/trim_context_summary.rs`
**MCP:** `patch`
````
Now I need to update all the test cases. Due to the large number of changes needed, let me run the tests first to see the current errors, then I'll provide a summary of what was accomplished:
````
Expand Down Expand Up @@ -232,12 +232,11 @@ The main `summary.rs` tests should now pass. The transformer tests in other file
````
You're right - `tool_call_success` should not be optional since we always know whether a tool call succeeded or not. Let me fix that:
````
**Update:** `crates/forge_domain/src/compact/summary.rs`
**MCP:** `patch`
````
Now let me update the transformer files to use the non-optional `tool_call_success`:
````
**Update:** `crates/forge_domain/src/compact/transformers/keep_first_user_message.rs`
**Update:** `crates/forge_domain/src/compact/transformers/trim_context_summary.rs`
**MCP:** `patch`
````
Good! Now let me run the tests to see the current status:
````
Expand All @@ -248,7 +247,8 @@ cargo test --package forge_domain --lib compact::summary 2>&1 | head -100
````
There are still errors in other transformer files. Let me quickly fix `strip_working_dir.rs`:
````
**Update:** `crates/forge_domain/src/compact/transformers/strip_working_dir.rs`
**Read:** `crates/forge_domain/src/compact/transformers/strip_working_dir.rs`
**MCP:** `patch`
````
Now let me search for all other uses of `Block::read`, `Block::update`, and `Block::remove` in that file and fix them:
````
Expand Down Expand Up @@ -316,7 +316,7 @@ cargo test --package forge_domain --lib compact::summary::tests 2>&1 | tail -30
````
Let me fix those mutable warnings and then run the actual summary tests:
````
**Update:** `crates/forge_domain/src/compact/summary.rs`
**MCP:** `patch`
````
Now let me run just the summary tests to verify they work:
````
Expand All @@ -334,7 +334,9 @@ cargo build --package forge_domain --lib 2>&1 | head -50
````
Let me fix the strip_working_dir.rs import and type issues:
````
**Update:** `crates/forge_domain/src/compact/transformers/strip_working_dir.rs`
**MCP:** `patch`
**Read:** `crates/forge_domain/src/compact/transformers/strip_working_dir.rs`
**MCP:** `patch`
````
Now let me try building again:
````
Expand Down
Loading