From 4a81f0e740a7c58b25e1dc081aae1521147f6a1f Mon Sep 17 00:00:00 2001 From: afjcjsbx Date: Tue, 12 May 2026 18:06:47 +0200 Subject: [PATCH 1/4] feat(tools): show unified diff for edit_file edits --- pkg/tools/fs/edit.go | 17 +++-- pkg/tools/fs/edit_test.go | 38 +++++++++-- pkg/tools/fs/shared.go | 4 ++ pkg/tools/result_test.go | 41 ++++++++++++ pkg/tools/shared/diff_result.go | 52 +++++++++++++++ pkg/tools/shared/diff_result_test.go | 97 ++++++++++++++++++++++++++++ pkg/tools/shared_facade.go | 4 ++ 7 files changed, 240 insertions(+), 13 deletions(-) create mode 100644 pkg/tools/shared/diff_result.go create mode 100644 pkg/tools/shared/diff_result_test.go diff --git a/pkg/tools/fs/edit.go b/pkg/tools/fs/edit.go index 827ea50c8..7a54a1b01 100644 --- a/pkg/tools/fs/edit.go +++ b/pkg/tools/fs/edit.go @@ -69,10 +69,11 @@ func (t *EditFileTool) Execute(ctx context.Context, args map[string]any) *ToolRe return ErrorResult("new_text is required") } - if err := editFile(t.fs, path, oldText, newText); err != nil { + beforeContent, afterContent, err := editFile(t.fs, path, oldText, newText) + if err != nil { return ErrorResult(err.Error()) } - return SilentResult(fmt.Sprintf("File edited: %s", path)) + return DiffResult(path, beforeContent, afterContent) } type AppendFileTool struct { @@ -131,18 +132,22 @@ func (t *AppendFileTool) Execute(ctx context.Context, args map[string]any) *Tool // editFile reads the file via sysFs, performs the replacement, and writes back. // It uses a fileSystem interface, allowing the same logic for both restricted and unrestricted modes. -func editFile(sysFs fileSystem, path, oldText, newText string) error { +func editFile(sysFs fileSystem, path, oldText, newText string) ([]byte, []byte, error) { content, err := sysFs.ReadFile(path) if err != nil { - return err + return nil, nil, err } newContent, err := replaceEditContent(content, oldText, newText) if err != nil { - return err + return nil, nil, err } - return sysFs.WriteFile(path, newContent) + if err := sysFs.WriteFile(path, newContent); err != nil { + return nil, nil, err + } + + return content, newContent, nil } // appendFile reads the existing content (if any) via sysFs, appends new content, and writes back. diff --git a/pkg/tools/fs/edit_test.go b/pkg/tools/fs/edit_test.go index 4c25322ef..e94a896da 100644 --- a/pkg/tools/fs/edit_test.go +++ b/pkg/tools/fs/edit_test.go @@ -2,6 +2,7 @@ package fstools import ( "context" + "fmt" "os" "path/filepath" "strings" @@ -31,14 +32,31 @@ func TestEditTool_EditFile_Success(t *testing.T) { t.Errorf("Expected success, got IsError=true: %s", result.ForLLM) } - // Should return SilentResult - if !result.Silent { - t.Errorf("Expected Silent=true for EditFile, got false") + // Successful edits should surface a diff to the user. + if result.Silent { + t.Errorf("Expected Silent=false for EditFile, got true") } - // ForUser should be empty (silent result) - if result.ForUser != "" { - t.Errorf("Expected ForUser to be empty for SilentResult, got: %s", result.ForUser) + if result.ForUser == "" { + t.Fatal("Expected ForUser to contain the diff preview") + } + + if result.ForLLM != result.ForUser { + t.Errorf("Expected ForLLM and ForUser to match, got ForLLM=%q ForUser=%q", result.ForLLM, result.ForUser) + } + + diffPath := strings.TrimLeft(filepath.ToSlash(testFile), "/") + for _, want := range []string{ + fmt.Sprintf("File edited: %s", testFile), + "```diff", + "--- a/" + diffPath, + "+++ b/" + diffPath, + "-Hello World", + "+Hello Universe", + } { + if !strings.Contains(result.ForUser, want) { + t.Fatalf("Expected edit diff to contain %q, got:\n%s", want, result.ForUser) + } } // Verify file was actually edited @@ -412,7 +430,13 @@ func TestEditFileTool_Restricted_InPlaceEdit(t *testing.T) { result := tool.Execute(ctx, args) assert.False(t, result.IsError, "Expected success, got: %s", result.ForLLM) - assert.True(t, result.Silent) + assert.False(t, result.Silent) + assert.Equal(t, result.ForLLM, result.ForUser) + assert.Contains(t, result.ForUser, "```diff") + assert.Contains(t, result.ForUser, "--- a/edit_target.txt") + assert.Contains(t, result.ForUser, "+++ b/edit_target.txt") + assert.Contains(t, result.ForUser, "-Hello World") + assert.Contains(t, result.ForUser, "+Hello Go") data, err := os.ReadFile(filepath.Join(workspace, testFile)) assert.NoError(t, err) diff --git a/pkg/tools/fs/shared.go b/pkg/tools/fs/shared.go index 6d46e692b..acf14169e 100644 --- a/pkg/tools/fs/shared.go +++ b/pkg/tools/fs/shared.go @@ -32,6 +32,10 @@ func SilentResult(forLLM string) *ToolResult { return toolshared.SilentResult(forLLM) } +func DiffResult(path string, before, after []byte) *ToolResult { + return toolshared.DiffResult(path, before, after) +} + func MediaResult(forLLM string, mediaRefs []string) *ToolResult { return toolshared.MediaResult(forLLM, mediaRefs) } diff --git a/pkg/tools/result_test.go b/pkg/tools/result_test.go index 5f08cb4fa..fda7e69c0 100644 --- a/pkg/tools/result_test.go +++ b/pkg/tools/result_test.go @@ -41,6 +41,47 @@ func TestSilentResult(t *testing.T) { } } +func TestDiffResult(t *testing.T) { + result := DiffResult("pkg/tools/fs/edit.go", []byte("hello world\n"), []byte("hello universe\n")) + + if result.ForLLM != result.ForUser { + t.Fatalf("Expected ForLLM and ForUser to match, got %q vs %q", result.ForLLM, result.ForUser) + } + if result.Silent { + t.Error("Expected Silent to be false") + } + if result.IsError { + t.Error("Expected IsError to be false") + } + if result.Async { + t.Error("Expected Async to be false") + } + + for _, want := range []string{ + "File edited: pkg/tools/fs/edit.go", + "```diff", + "--- a/pkg/tools/fs/edit.go", + "+++ b/pkg/tools/fs/edit.go", + "-hello world", + "+hello universe", + } { + if !strings.Contains(result.ForUser, want) { + t.Fatalf("DiffResult output missing %q:\n%s", want, result.ForUser) + } + } +} + +func TestDiffResult_NormalizesAbsolutePathsAndHandlesNoOpChanges(t *testing.T) { + result := DiffResult("/tmp/test.txt", []byte("same\n"), []byte("same\n")) + + if !strings.Contains(result.ForUser, "File edited: /tmp/test.txt") { + t.Fatalf("Expected original path in output, got %q", result.ForUser) + } + if !strings.Contains(result.ForUser, "(no content change)") { + t.Fatalf("Expected no-content-change marker, got %q", result.ForUser) + } +} + func TestAsyncResult(t *testing.T) { result := AsyncResult("async task started") diff --git a/pkg/tools/shared/diff_result.go b/pkg/tools/shared/diff_result.go new file mode 100644 index 000000000..99f7e9793 --- /dev/null +++ b/pkg/tools/shared/diff_result.go @@ -0,0 +1,52 @@ +package toolshared + +import ( + "fmt" + "path/filepath" + "strings" + + "github.com/pmezard/go-difflib/difflib" +) + +const noContentChangeDiffMessage = "(no content change)" + +// DiffResult creates a user-visible tool result containing a unified diff for +// a successful file edit. The diff is included for both the LLM and the user so +// the follow-up assistant response can reason about the exact change set. +func DiffResult(path string, before, after []byte) *ToolResult { + diff, err := buildUnifiedDiff(path, before, after) + if err != nil { + return UserResult(fmt.Sprintf("File edited: %s\n[diff unavailable: %v]", path, err)) + } + + content := fmt.Sprintf("File edited: %s\n```diff\n%s\n```", path, diff) + return UserResult(content) +} + +func buildUnifiedDiff(path string, before, after []byte) (string, error) { + diff, err := difflib.GetUnifiedDiffString(difflib.UnifiedDiff{ + A: difflib.SplitLines(string(before)), + B: difflib.SplitLines(string(after)), + FromFile: "a/" + diffDisplayPath(path), + ToFile: "b/" + diffDisplayPath(path), + Context: 3, + }) + if err != nil { + return "", err + } + + diff = strings.TrimRight(diff, "\n") + if diff == "" { + return noContentChangeDiffMessage, nil + } + + return diff, nil +} + +func diffDisplayPath(path string) string { + displayPath := strings.TrimLeft(filepath.ToSlash(path), "/") + if displayPath == "" { + return "file" + } + return displayPath +} diff --git a/pkg/tools/shared/diff_result_test.go b/pkg/tools/shared/diff_result_test.go new file mode 100644 index 000000000..7c06d205a --- /dev/null +++ b/pkg/tools/shared/diff_result_test.go @@ -0,0 +1,97 @@ +package toolshared + +import ( + "strings" + "testing" +) + +func TestDiffResult_UserVisibleUnifiedDiff(t *testing.T) { + result := DiffResult("/tmp/example.txt", []byte("alpha\nbeta\ngamma\n"), []byte("alpha\nbeta 2\ngamma\n")) + + if result == nil { + t.Fatal("DiffResult() returned nil") + } + if result.ForLLM != result.ForUser { + t.Fatalf("expected ForLLM and ForUser to match, got %q vs %q", result.ForLLM, result.ForUser) + } + if result.Silent { + t.Fatal("expected DiffResult to be user-visible") + } + if result.IsError { + t.Fatal("expected DiffResult to be successful") + } + + for _, want := range []string{ + "File edited: /tmp/example.txt", + "```diff", + "--- a/tmp/example.txt", + "+++ b/tmp/example.txt", + "@@ -1,4 +1,4 @@", + " alpha", + "-beta", + "+beta 2", + " gamma", + } { + if !strings.Contains(result.ForUser, want) { + t.Fatalf("DiffResult output missing %q:\n%s", want, result.ForUser) + } + } +} + +func TestBuildUnifiedDiff_NoContentChange(t *testing.T) { + diff, err := buildUnifiedDiff("test.txt", []byte("same\n"), []byte("same\n")) + if err != nil { + t.Fatalf("buildUnifiedDiff() error = %v", err) + } + if diff != noContentChangeDiffMessage { + t.Fatalf("buildUnifiedDiff() = %q, want %q", diff, noContentChangeDiffMessage) + } +} + +func TestBuildUnifiedDiff_UsesNormalizedDisplayPaths(t *testing.T) { + diff, err := buildUnifiedDiff("/tmp/nested/example.txt", []byte("before\n"), []byte("after\n")) + if err != nil { + t.Fatalf("buildUnifiedDiff() error = %v", err) + } + + for _, want := range []string{ + "--- a/tmp/nested/example.txt", + "+++ b/tmp/nested/example.txt", + } { + if !strings.Contains(diff, want) { + t.Fatalf("buildUnifiedDiff() missing %q:\n%s", want, diff) + } + } +} + +func TestDiffDisplayPath(t *testing.T) { + tests := []struct { + name string + path string + want string + }{ + { + name: "absolute path", + path: "/tmp/example.txt", + want: "tmp/example.txt", + }, + { + name: "relative path", + path: "pkg/tools/fs/edit.go", + want: "pkg/tools/fs/edit.go", + }, + { + name: "empty path", + path: "", + want: "file", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := diffDisplayPath(tt.path); got != tt.want { + t.Fatalf("diffDisplayPath(%q) = %q, want %q", tt.path, got, tt.want) + } + }) + } +} diff --git a/pkg/tools/shared_facade.go b/pkg/tools/shared_facade.go index 8409ea060..85bac140a 100644 --- a/pkg/tools/shared_facade.go +++ b/pkg/tools/shared_facade.go @@ -101,6 +101,10 @@ func SilentResult(forLLM string) *ToolResult { return toolshared.SilentResult(forLLM) } +func DiffResult(path string, before, after []byte) *ToolResult { + return toolshared.DiffResult(path, before, after) +} + func AsyncResult(forLLM string) *ToolResult { return toolshared.AsyncResult(forLLM) } From 87048499fff62fd8249948c09aa2655bedf47a46 Mon Sep 17 00:00:00 2001 From: afjcjsbx Date: Tue, 12 May 2026 23:06:43 +0200 Subject: [PATCH 2/4] fix(tools) diff preview for files without trailing newline --- pkg/tools/shared/diff_result.go | 43 +++++++++++++++++++++++++--- pkg/tools/shared/diff_result_test.go | 38 +++++++++++++++++++++++- 2 files changed, 76 insertions(+), 5 deletions(-) diff --git a/pkg/tools/shared/diff_result.go b/pkg/tools/shared/diff_result.go index 99f7e9793..0f6166219 100644 --- a/pkg/tools/shared/diff_result.go +++ b/pkg/tools/shared/diff_result.go @@ -1,6 +1,7 @@ package toolshared import ( + "bytes" "fmt" "path/filepath" "strings" @@ -8,11 +9,15 @@ import ( "github.com/pmezard/go-difflib/difflib" ) -const noContentChangeDiffMessage = "(no content change)" +const ( + noContentChangeDiffMessage = "(no content change)" + noNewlineAtEOFMarker = `\ No newline at end of file` +) // DiffResult creates a user-visible tool result containing a unified diff for // a successful file edit. The diff is included for both the LLM and the user so -// the follow-up assistant response can reason about the exact change set. +// the follow-up assistant response can reason about the resulting change set, +// including EOF newline transitions. func DiffResult(path string, before, after []byte) *ToolResult { diff, err := buildUnifiedDiff(path, before, after) if err != nil { @@ -25,8 +30,8 @@ func DiffResult(path string, before, after []byte) *ToolResult { func buildUnifiedDiff(path string, before, after []byte) (string, error) { diff, err := difflib.GetUnifiedDiffString(difflib.UnifiedDiff{ - A: difflib.SplitLines(string(before)), - B: difflib.SplitLines(string(after)), + A: splitDiffLinesPreservingEOF(before), + B: splitDiffLinesPreservingEOF(after), FromFile: "a/" + diffDisplayPath(path), ToFile: "b/" + diffDisplayPath(path), Context: 3, @@ -43,6 +48,36 @@ func buildUnifiedDiff(path string, before, after []byte) (string, error) { return diff, nil } +func splitDiffLinesPreservingEOF(content []byte) []string { + if len(content) == 0 { + return nil + } + + lines := make([]string, 0, bytes.Count(content, []byte{'\n'})+1) + lineStart := 0 + for i, b := range content { + if b != '\n' { + continue + } + lines = append(lines, string(content[lineStart:i+1])) + lineStart = i + 1 + } + if lineStart < len(content) { + lines = append(lines, string(content[lineStart:])) + } + + if lacksTrailingNewline(content) { + lines[len(lines)-1] += "\n" + lines = append(lines, noNewlineAtEOFMarker+"\n") + } + + return lines +} + +func lacksTrailingNewline(content []byte) bool { + return len(content) > 0 && !bytes.HasSuffix(content, []byte("\n")) +} + func diffDisplayPath(path string) string { displayPath := strings.TrimLeft(filepath.ToSlash(path), "/") if displayPath == "" { diff --git a/pkg/tools/shared/diff_result_test.go b/pkg/tools/shared/diff_result_test.go index 7c06d205a..848bde221 100644 --- a/pkg/tools/shared/diff_result_test.go +++ b/pkg/tools/shared/diff_result_test.go @@ -26,7 +26,7 @@ func TestDiffResult_UserVisibleUnifiedDiff(t *testing.T) { "```diff", "--- a/tmp/example.txt", "+++ b/tmp/example.txt", - "@@ -1,4 +1,4 @@", + "@@ -1,3 +1,3 @@", " alpha", "-beta", "+beta 2", @@ -48,6 +48,42 @@ func TestBuildUnifiedDiff_NoContentChange(t *testing.T) { } } +func TestBuildUnifiedDiff_PreservesTrailingNewlineRemoval(t *testing.T) { + diff, err := buildUnifiedDiff("test.txt", []byte("same\n"), []byte("same")) + if err != nil { + t.Fatalf("buildUnifiedDiff() error = %v", err) + } + + for _, want := range []string{ + "--- a/test.txt", + "+++ b/test.txt", + " same", + "+" + noNewlineAtEOFMarker, + } { + if !strings.Contains(diff, want) { + t.Fatalf("buildUnifiedDiff() missing %q:\n%s", want, diff) + } + } +} + +func TestBuildUnifiedDiff_PreservesTrailingNewlineAddition(t *testing.T) { + diff, err := buildUnifiedDiff("test.txt", []byte("same"), []byte("same\n")) + if err != nil { + t.Fatalf("buildUnifiedDiff() error = %v", err) + } + + for _, want := range []string{ + "--- a/test.txt", + "+++ b/test.txt", + " same", + "-" + noNewlineAtEOFMarker, + } { + if !strings.Contains(diff, want) { + t.Fatalf("buildUnifiedDiff() missing %q:\n%s", want, diff) + } + } +} + func TestBuildUnifiedDiff_UsesNormalizedDisplayPaths(t *testing.T) { diff, err := buildUnifiedDiff("/tmp/nested/example.txt", []byte("before\n"), []byte("after\n")) if err != nil { From 56cca3f12fb0c462e9e83f51beabcd56d916c147 Mon Sep 17 00:00:00 2001 From: afjcjsbx Date: Tue, 12 May 2026 23:12:37 +0200 Subject: [PATCH 3/4] fix(tools) limit edit diff preview size for user and model --- pkg/tools/result_test.go | 12 +++- pkg/tools/shared/diff_result.go | 85 ++++++++++++++++++++++++++-- pkg/tools/shared/diff_result_test.go | 50 +++++++++++++++- 3 files changed, 136 insertions(+), 11 deletions(-) diff --git a/pkg/tools/result_test.go b/pkg/tools/result_test.go index fda7e69c0..3e7848687 100644 --- a/pkg/tools/result_test.go +++ b/pkg/tools/result_test.go @@ -44,9 +44,6 @@ func TestSilentResult(t *testing.T) { func TestDiffResult(t *testing.T) { result := DiffResult("pkg/tools/fs/edit.go", []byte("hello world\n"), []byte("hello universe\n")) - if result.ForLLM != result.ForUser { - t.Fatalf("Expected ForLLM and ForUser to match, got %q vs %q", result.ForLLM, result.ForUser) - } if result.Silent { t.Error("Expected Silent to be false") } @@ -56,6 +53,12 @@ func TestDiffResult(t *testing.T) { if result.Async { t.Error("Expected Async to be false") } + if result.ForLLM == result.ForUser { + t.Fatalf("Expected ForLLM to omit the full diff, got %q", result.ForLLM) + } + if len(result.ForLLM) >= len(result.ForUser) { + t.Fatalf("Expected ForLLM to stay smaller than ForUser, got %d vs %d", len(result.ForLLM), len(result.ForUser)) + } for _, want := range []string{ "File edited: pkg/tools/fs/edit.go", @@ -80,6 +83,9 @@ func TestDiffResult_NormalizesAbsolutePathsAndHandlesNoOpChanges(t *testing.T) { if !strings.Contains(result.ForUser, "(no content change)") { t.Fatalf("Expected no-content-change marker, got %q", result.ForUser) } + if !strings.Contains(result.ForLLM, "(no content change)") { + t.Fatalf("Expected compact no-op summary in ForLLM, got %q", result.ForLLM) + } } func TestAsyncResult(t *testing.T) { diff --git a/pkg/tools/shared/diff_result.go b/pkg/tools/shared/diff_result.go index 0f6166219..3ed7bdda1 100644 --- a/pkg/tools/shared/diff_result.go +++ b/pkg/tools/shared/diff_result.go @@ -5,6 +5,7 @@ import ( "fmt" "path/filepath" "strings" + "unicode/utf8" "github.com/pmezard/go-difflib/difflib" ) @@ -12,6 +13,11 @@ import ( const ( noContentChangeDiffMessage = "(no content change)" noNewlineAtEOFMarker = `\ No newline at end of file` + diffPreviewSkippedMessage = "[diff preview skipped: file too large for inline preview]" + diffPreviewTruncatedNote = "[diff preview truncated; call read_file for the full edited contents]" + maxDiffInputBytes = 64 * 1024 + maxDiffInputLines = 2000 + maxUserDiffPreviewBytes = 16 * 1024 ) // DiffResult creates a user-visible tool result containing a unified diff for @@ -19,13 +25,36 @@ const ( // the follow-up assistant response can reason about the resulting change set, // including EOF newline transitions. func DiffResult(path string, before, after []byte) *ToolResult { - diff, err := buildUnifiedDiff(path, before, after) - if err != nil { - return UserResult(fmt.Sprintf("File edited: %s\n[diff unavailable: %v]", path, err)) + summary := fmt.Sprintf("File edited: %s", path) + if exceedsDiffPreviewLimits(before, after) { + return SilentResult(summary + "\n" + diffPreviewSkippedMessage) } - content := fmt.Sprintf("File edited: %s\n```diff\n%s\n```", path, diff) - return UserResult(content) + diff, err := buildUnifiedDiff(path, before, after) + if err != nil { + return UserResult(fmt.Sprintf("%s\n[diff unavailable: %v]", summary, err)) + } + + userDiff, truncated := truncateDiffPreview(diff, maxUserDiffPreviewBytes) + userContent := fmt.Sprintf("%s\n```diff\n%s\n```", summary, userDiff) + if truncated { + userContent += "\n" + diffPreviewTruncatedNote + } + + llmContent := summary + if diff == noContentChangeDiffMessage { + llmContent = summary + "\n" + noContentChangeDiffMessage + } else if truncated { + llmContent = summary + "\n" + diffPreviewTruncatedNote + } + + return &ToolResult{ + ForLLM: llmContent, + ForUser: userContent, + Silent: false, + IsError: false, + Async: false, + } } func buildUnifiedDiff(path string, before, after []byte) (string, error) { @@ -78,6 +107,52 @@ func lacksTrailingNewline(content []byte) bool { return len(content) > 0 && !bytes.HasSuffix(content, []byte("\n")) } +func exceedsDiffPreviewLimits(before, after []byte) bool { + return len(before) > maxDiffInputBytes || + len(after) > maxDiffInputBytes || + countDiffLines(before) > maxDiffInputLines || + countDiffLines(after) > maxDiffInputLines +} + +func countDiffLines(content []byte) int { + if len(content) == 0 { + return 0 + } + + lines := bytes.Count(content, []byte{'\n'}) + if !bytes.HasSuffix(content, []byte("\n")) { + lines++ + } + return lines +} + +func truncateDiffPreview(diff string, maxBytes int) (string, bool) { + if maxBytes <= 0 || len(diff) <= maxBytes { + return diff, false + } + + truncated := diff[:maxBytes] + for len(truncated) > 0 && !utf8.ValidString(truncated) { + truncated = truncated[:len(truncated)-1] + } + + lastNewline := strings.LastIndexByte(truncated, '\n') + if lastNewline > 0 { + truncated = truncated[:lastNewline] + } + + truncated = strings.TrimRight(truncated, "\n") + if truncated == "" { + truncated = diff[:maxBytes] + for len(truncated) > 0 && !utf8.ValidString(truncated) { + truncated = truncated[:len(truncated)-1] + } + truncated = strings.TrimRight(truncated, "\n") + } + + return truncated, true +} + func diffDisplayPath(path string) string { displayPath := strings.TrimLeft(filepath.ToSlash(path), "/") if displayPath == "" { diff --git a/pkg/tools/shared/diff_result_test.go b/pkg/tools/shared/diff_result_test.go index 848bde221..9d4f38ea5 100644 --- a/pkg/tools/shared/diff_result_test.go +++ b/pkg/tools/shared/diff_result_test.go @@ -1,6 +1,7 @@ package toolshared import ( + "bytes" "strings" "testing" ) @@ -11,15 +12,21 @@ func TestDiffResult_UserVisibleUnifiedDiff(t *testing.T) { if result == nil { t.Fatal("DiffResult() returned nil") } - if result.ForLLM != result.ForUser { - t.Fatalf("expected ForLLM and ForUser to match, got %q vs %q", result.ForLLM, result.ForUser) - } if result.Silent { t.Fatal("expected DiffResult to be user-visible") } if result.IsError { t.Fatal("expected DiffResult to be successful") } + if result.ForLLM == result.ForUser { + t.Fatal("expected compact model context instead of duplicating the full diff") + } + if len(result.ForLLM) >= len(result.ForUser) { + t.Fatalf("expected ForLLM to stay smaller than ForUser, got %d vs %d", len(result.ForLLM), len(result.ForUser)) + } + if result.ForLLM != "File edited: /tmp/example.txt" { + t.Fatalf("expected compact summary in ForLLM, got %q", result.ForLLM) + } for _, want := range []string{ "File edited: /tmp/example.txt", @@ -100,6 +107,43 @@ func TestBuildUnifiedDiff_UsesNormalizedDisplayPaths(t *testing.T) { } } +func TestDiffResult_SkipsPreviewForLargeFiles(t *testing.T) { + before := bytes.Repeat([]byte("a"), maxDiffInputBytes+1) + after := bytes.Repeat([]byte("b"), maxDiffInputBytes+1) + + result := DiffResult("big.txt", before, after) + + if !result.Silent { + t.Fatal("expected large diff previews to be skipped silently") + } + if result.ForUser != "" { + t.Fatalf("expected no user-facing preview when skipped, got %q", result.ForUser) + } + if !strings.Contains(result.ForLLM, diffPreviewSkippedMessage) { + t.Fatalf("expected skipped-preview note, got %q", result.ForLLM) + } +} + +func TestDiffResult_TruncatesLargeUserPreview(t *testing.T) { + after := []byte(strings.Repeat("abcd", maxUserDiffPreviewBytes/4) + "\n") + + result := DiffResult("preview.txt", []byte("before\n"), after) + + if result.Silent { + t.Fatal("expected preview to remain user-visible below the input caps") + } + if !strings.Contains(result.ForUser, diffPreviewTruncatedNote) { + t.Fatalf("expected truncated preview note, got %q", result.ForUser) + } + if !strings.Contains(result.ForLLM, diffPreviewTruncatedNote) { + t.Fatalf("expected model summary to mention truncation, got %q", result.ForLLM) + } + if len(result.ForLLM) >= len(result.ForUser) { + t.Fatalf("expected ForLLM to remain smaller than ForUser, "+ + "got %d vs %d", len(result.ForLLM), len(result.ForUser)) + } +} + func TestDiffDisplayPath(t *testing.T) { tests := []struct { name string From e0370aafcc5df79c5345b48af79537d254cf16ac Mon Sep 17 00:00:00 2001 From: afjcjsbx Date: Tue, 12 May 2026 23:23:26 +0200 Subject: [PATCH 4/4] fix test --- pkg/tools/fs/edit_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/tools/fs/edit_test.go b/pkg/tools/fs/edit_test.go index e94a896da..b04c41fff 100644 --- a/pkg/tools/fs/edit_test.go +++ b/pkg/tools/fs/edit_test.go @@ -41,8 +41,11 @@ func TestEditTool_EditFile_Success(t *testing.T) { t.Fatal("Expected ForUser to contain the diff preview") } - if result.ForLLM != result.ForUser { - t.Errorf("Expected ForLLM and ForUser to match, got ForLLM=%q ForUser=%q", result.ForLLM, result.ForUser) + if result.ForLLM == result.ForUser { + t.Fatalf("Expected ForLLM to be a compact summary, got identical outputs %q", result.ForLLM) + } + if result.ForLLM != fmt.Sprintf("File edited: %s", testFile) { + t.Fatalf("Expected compact ForLLM summary, got %q", result.ForLLM) } diffPath := strings.TrimLeft(filepath.ToSlash(testFile), "/") @@ -431,7 +434,7 @@ func TestEditFileTool_Restricted_InPlaceEdit(t *testing.T) { result := tool.Execute(ctx, args) assert.False(t, result.IsError, "Expected success, got: %s", result.ForLLM) assert.False(t, result.Silent) - assert.Equal(t, result.ForLLM, result.ForUser) + assert.Equal(t, "File edited: edit_target.txt", result.ForLLM) assert.Contains(t, result.ForUser, "```diff") assert.Contains(t, result.ForUser, "--- a/edit_target.txt") assert.Contains(t, result.ForUser, "+++ b/edit_target.txt")