feat(tools): inline unified-diff in edit_file / write_file results (#505)
`edit_file` and `write_file` now capture the file contents before and after the mutation, generate a unified diff with `similar`, and emit it at the head of the `ToolResult` body. The TUI's existing `output_looks_like_diff` detector (history.rs:1335) sees the `@@` header in the first 5 lines and routes the payload through `diff_render::render_diff`, which already renders unified diffs with line numbers and coloured `+`/`-` gutters. The model also benefits — it sees exactly which lines changed instead of just `Replaced N occurrence(s)` or `Wrote N bytes`. Identical content produces an empty diff, in which case the body falls back to `<summary>\n(no changes)`. ### What's wired - New `crates/tui/src/tools/diff_format.rs` exposes `make_unified_diff(path, old, new) -> String` using `similar::TextDiff::from_lines(...).unified_diff().context_radius(3)`. - `WriteFileTool::execute` snapshots prior contents (or empty for new files), writes, then emits `<diff>\n<summary>` where summary is `Wrote N bytes to PATH` for existing files and `Created PATH (N bytes)` for new ones. - `EditFileTool::execute` snapshots, replaces, writes, emits `<diff>\nReplaced N occurrence(s) in PATH`. - `similar = "2"` added to `crates/tui/Cargo.toml`. Pure-Rust, no C deps; v2.7.0 in Cargo.lock. ### Tests - 4 unit tests in `diff_format::tests` covering identical inputs, replacement, new-file (against empty), and presence of the `@@` header in the first 5 lines (so the TUI detector trips). - Existing `test_write_file_tool` / `test_edit_file_tool` updated to assert both the summary line and the unified-diff body (`--- a/`, `-old`, `+new`). ### Verification cargo fmt --all -- --check ✓ cargo clippy --workspace --all-targets --all-features --locked -- -D warnings ✓ cargo test --workspace --all-features --locked ✓ (1824 + supporting; was 1820) Closes #505 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Generated
+7
@@ -1269,6 +1269,7 @@ dependencies = [
|
||||
"sha2 0.10.9",
|
||||
"shellexpand",
|
||||
"shlex",
|
||||
"similar",
|
||||
"starlark",
|
||||
"tar",
|
||||
"tempfile",
|
||||
@@ -4607,6 +4608,12 @@ version = "0.3.8"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "e320a6c5ad31d271ad523dcf3ad13e2767ad8b1cb8f047f75a8aeaf8da139da2"
|
||||
|
||||
[[package]]
|
||||
name = "similar"
|
||||
version = "2.7.0"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "bbbb5d9659141646ae647b42fe094daf6c6192d1620870b449d9557f748b2daa"
|
||||
|
||||
[[package]]
|
||||
name = "siphasher"
|
||||
version = "1.0.1"
|
||||
|
||||
@@ -39,6 +39,7 @@ indicatif = "0.18.0"
|
||||
ratatui = "0.29"
|
||||
regex = "1.11"
|
||||
reqwest = { version = "0.13.1", default-features = false, features = ["blocking", "json", "stream", "multipart", "native-tls", "http2"] }
|
||||
similar = "2"
|
||||
rustyline = "15.0.0"
|
||||
serde = { version = "1.0.228", features = ["derive"] }
|
||||
serde_json = { version = "1.0.149", features = ["preserve_order"] }
|
||||
|
||||
@@ -0,0 +1,77 @@
|
||||
//! Build unified-diff strings for tool results.
|
||||
//!
|
||||
//! `edit_file` and `write_file` capture the file contents before and after
|
||||
//! the mutation and emit a unified diff at the head of their `ToolResult`
|
||||
//! output. The TUI's `output_looks_like_diff` detector then routes the
|
||||
//! payload through `diff_render::render_diff`, which renders it with line
|
||||
//! numbers and coloured `+`/`-` gutters (#505).
|
||||
//!
|
||||
//! The diff is also a strict UX upgrade for the model — it sees exactly
|
||||
//! which lines changed instead of a one-line summary.
|
||||
|
||||
use similar::TextDiff;
|
||||
|
||||
/// Build a unified diff between `old` and `new` keyed at `path`.
|
||||
///
|
||||
/// Returns an empty string when the inputs are byte-identical so callers
|
||||
/// can skip the "no changes" header. The output uses git-style `--- a/...`
|
||||
/// / `+++ b/...` headers and three lines of context — matching the format
|
||||
/// the TUI's `diff_render::render_diff` already understands.
|
||||
#[must_use]
|
||||
pub fn make_unified_diff(path: &str, old: &str, new: &str) -> String {
|
||||
if old == new {
|
||||
return String::new();
|
||||
}
|
||||
let a = format!("a/{path}");
|
||||
let b = format!("b/{path}");
|
||||
let diff = TextDiff::from_lines(old, new);
|
||||
diff.unified_diff()
|
||||
.context_radius(3)
|
||||
.header(&a, &b)
|
||||
.to_string()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn identical_inputs_emit_empty_diff() {
|
||||
let s = "hello\nworld\n";
|
||||
assert!(make_unified_diff("foo.txt", s, s).is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn replacement_emits_minus_plus_pair() {
|
||||
let old = "alpha\nbeta\ngamma\n";
|
||||
let new = "alpha\nBETA\ngamma\n";
|
||||
let diff = make_unified_diff("foo.txt", old, new);
|
||||
assert!(diff.contains("--- a/foo.txt"), "{diff}");
|
||||
assert!(diff.contains("+++ b/foo.txt"), "{diff}");
|
||||
assert!(diff.contains("-beta"), "{diff}");
|
||||
assert!(diff.contains("+BETA"), "{diff}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn new_file_renders_against_empty_old() {
|
||||
let new = "first line\nsecond line\n";
|
||||
let diff = make_unified_diff("new.txt", "", new);
|
||||
assert!(diff.contains("--- a/new.txt"), "{diff}");
|
||||
assert!(diff.contains("+++ b/new.txt"), "{diff}");
|
||||
assert!(diff.contains("+first line"), "{diff}");
|
||||
assert!(diff.contains("+second line"), "{diff}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn diff_contains_hunk_header_so_tui_renders_it() {
|
||||
// The TUI detector scans the first 5 lines for `@@`. Make sure the
|
||||
// unified diff puts a hunk header within that window so the
|
||||
// diff-aware renderer kicks in (#505).
|
||||
let diff = make_unified_diff("foo.txt", "a\n", "b\n");
|
||||
let head: Vec<&str> = diff.lines().take(5).collect();
|
||||
assert!(
|
||||
head.iter().any(|line| line.starts_with("@@")),
|
||||
"expected hunk header in first 5 lines; got {head:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -3,6 +3,7 @@
|
||||
//! These tools provide safe file system operations within the workspace,
|
||||
//! with path validation to prevent escaping the workspace boundary.
|
||||
|
||||
use super::diff_format::make_unified_diff;
|
||||
use super::spec::{
|
||||
ApprovalRequirement, ToolCapability, ToolContext, ToolError, ToolResult, ToolSpec,
|
||||
optional_str, required_str,
|
||||
@@ -228,6 +229,15 @@ impl ToolSpec for WriteFileTool {
|
||||
|
||||
let file_path = context.resolve_path(path_str)?;
|
||||
|
||||
// Snapshot the existing contents (if any) before we overwrite — used
|
||||
// to render an inline diff in the tool result.
|
||||
let existed_before = file_path.exists();
|
||||
let prior_contents = if existed_before {
|
||||
fs::read_to_string(&file_path).unwrap_or_default()
|
||||
} else {
|
||||
String::new()
|
||||
};
|
||||
|
||||
// Create parent directories if needed
|
||||
if let Some(parent) = file_path.parent() {
|
||||
fs::create_dir_all(parent).map_err(|e| {
|
||||
@@ -243,11 +253,20 @@ impl ToolSpec for WriteFileTool {
|
||||
ToolError::execution_failed(format!("Failed to write {}: {}", file_path.display(), e))
|
||||
})?;
|
||||
|
||||
Ok(ToolResult::success(format!(
|
||||
"Wrote {} bytes to {}",
|
||||
file_content.len(),
|
||||
file_path.display()
|
||||
)))
|
||||
let display = file_path.display().to_string();
|
||||
let diff = make_unified_diff(&display, &prior_contents, file_content);
|
||||
let summary = if existed_before {
|
||||
format!("Wrote {} bytes to {}", file_content.len(), display)
|
||||
} else {
|
||||
format!("Created {} ({} bytes)", display, file_content.len())
|
||||
};
|
||||
let body = if diff.is_empty() {
|
||||
format!("{summary}\n(no changes)")
|
||||
} else {
|
||||
format!("{diff}\n{summary}")
|
||||
};
|
||||
|
||||
Ok(ToolResult::success(body))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -324,11 +343,16 @@ impl ToolSpec for EditFileTool {
|
||||
ToolError::execution_failed(format!("Failed to write {}: {}", file_path.display(), e))
|
||||
})?;
|
||||
|
||||
Ok(ToolResult::success(format!(
|
||||
"Replaced {} occurrence(s) in {}",
|
||||
count,
|
||||
file_path.display()
|
||||
)))
|
||||
let display = file_path.display().to_string();
|
||||
let diff = make_unified_diff(&display, &contents, &updated);
|
||||
let summary = format!("Replaced {count} occurrence(s) in {display}");
|
||||
let body = if diff.is_empty() {
|
||||
format!("{summary}\n(no textual changes)")
|
||||
} else {
|
||||
format!("{diff}\n{summary}")
|
||||
};
|
||||
|
||||
Ok(ToolResult::success(body))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -527,7 +551,15 @@ mod tests {
|
||||
.expect("execute");
|
||||
|
||||
assert!(result.success);
|
||||
assert!(result.content.contains("Wrote"));
|
||||
// New file → "Created …" summary; the unified diff above the summary
|
||||
// primes the TUI's diff-aware renderer (#505).
|
||||
assert!(result.content.contains("Created"), "{}", result.content);
|
||||
assert!(result.content.contains("--- a/"), "{}", result.content);
|
||||
assert!(
|
||||
result.content.contains("+test content"),
|
||||
"{}",
|
||||
result.content
|
||||
);
|
||||
|
||||
// Verify file was written
|
||||
let written = fs::read_to_string(tmp.path().join("output.txt")).expect("read");
|
||||
@@ -575,6 +607,19 @@ mod tests {
|
||||
|
||||
assert!(result.success);
|
||||
assert!(result.content.contains("2 occurrence(s)"));
|
||||
// Inline diff (#505) — the unified diff lands above the summary
|
||||
// line so the TUI's diff-aware renderer kicks in.
|
||||
assert!(result.content.contains("--- a/"), "{}", result.content);
|
||||
assert!(
|
||||
result.content.contains("-hello world hello"),
|
||||
"{}",
|
||||
result.content
|
||||
);
|
||||
assert!(
|
||||
result.content.contains("+hi world hi"),
|
||||
"{}",
|
||||
result.content
|
||||
);
|
||||
|
||||
// Verify edit was applied
|
||||
let edited = fs::read_to_string(&test_file).expect("read");
|
||||
|
||||
@@ -4,6 +4,7 @@ pub mod apply_patch;
|
||||
pub mod approval_cache;
|
||||
pub mod automation;
|
||||
pub mod diagnostics;
|
||||
pub mod diff_format;
|
||||
pub mod file;
|
||||
pub mod file_search;
|
||||
pub mod finance;
|
||||
|
||||
Reference in New Issue
Block a user