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