fix(tools) limit edit diff preview size for user and model
This commit is contained in:
parent
87048499ff
commit
56cca3f12f
3 changed files with 136 additions and 11 deletions
|
|
@ -44,9 +44,6 @@ func TestSilentResult(t *testing.T) {
|
||||||
func TestDiffResult(t *testing.T) {
|
func TestDiffResult(t *testing.T) {
|
||||||
result := DiffResult("pkg/tools/fs/edit.go", []byte("hello world\n"), []byte("hello universe\n"))
|
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 {
|
if result.Silent {
|
||||||
t.Error("Expected Silent to be false")
|
t.Error("Expected Silent to be false")
|
||||||
}
|
}
|
||||||
|
|
@ -56,6 +53,12 @@ func TestDiffResult(t *testing.T) {
|
||||||
if result.Async {
|
if result.Async {
|
||||||
t.Error("Expected Async to be false")
|
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{
|
for _, want := range []string{
|
||||||
"File edited: pkg/tools/fs/edit.go",
|
"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)") {
|
if !strings.Contains(result.ForUser, "(no content change)") {
|
||||||
t.Fatalf("Expected no-content-change marker, got %q", result.ForUser)
|
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) {
|
func TestAsyncResult(t *testing.T) {
|
||||||
|
|
|
||||||
|
|
@ -5,6 +5,7 @@ import (
|
||||||
"fmt"
|
"fmt"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
|
"unicode/utf8"
|
||||||
|
|
||||||
"github.com/pmezard/go-difflib/difflib"
|
"github.com/pmezard/go-difflib/difflib"
|
||||||
)
|
)
|
||||||
|
|
@ -12,6 +13,11 @@ import (
|
||||||
const (
|
const (
|
||||||
noContentChangeDiffMessage = "(no content change)"
|
noContentChangeDiffMessage = "(no content change)"
|
||||||
noNewlineAtEOFMarker = `\ No newline at end of file`
|
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
|
// 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,
|
// the follow-up assistant response can reason about the resulting change set,
|
||||||
// including EOF newline transitions.
|
// including EOF newline transitions.
|
||||||
func DiffResult(path string, before, after []byte) *ToolResult {
|
func DiffResult(path string, before, after []byte) *ToolResult {
|
||||||
diff, err := buildUnifiedDiff(path, before, after)
|
summary := fmt.Sprintf("File edited: %s", path)
|
||||||
if err != nil {
|
if exceedsDiffPreviewLimits(before, after) {
|
||||||
return UserResult(fmt.Sprintf("File edited: %s\n[diff unavailable: %v]", path, err))
|
return SilentResult(summary + "\n" + diffPreviewSkippedMessage)
|
||||||
}
|
}
|
||||||
|
|
||||||
content := fmt.Sprintf("File edited: %s\n```diff\n%s\n```", path, diff)
|
diff, err := buildUnifiedDiff(path, before, after)
|
||||||
return UserResult(content)
|
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) {
|
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"))
|
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 {
|
func diffDisplayPath(path string) string {
|
||||||
displayPath := strings.TrimLeft(filepath.ToSlash(path), "/")
|
displayPath := strings.TrimLeft(filepath.ToSlash(path), "/")
|
||||||
if displayPath == "" {
|
if displayPath == "" {
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,7 @@
|
||||||
package toolshared
|
package toolshared
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"bytes"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
)
|
)
|
||||||
|
|
@ -11,15 +12,21 @@ func TestDiffResult_UserVisibleUnifiedDiff(t *testing.T) {
|
||||||
if result == nil {
|
if result == nil {
|
||||||
t.Fatal("DiffResult() returned 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 {
|
if result.Silent {
|
||||||
t.Fatal("expected DiffResult to be user-visible")
|
t.Fatal("expected DiffResult to be user-visible")
|
||||||
}
|
}
|
||||||
if result.IsError {
|
if result.IsError {
|
||||||
t.Fatal("expected DiffResult to be successful")
|
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{
|
for _, want := range []string{
|
||||||
"File edited: /tmp/example.txt",
|
"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) {
|
func TestDiffDisplayPath(t *testing.T) {
|
||||||
tests := []struct {
|
tests := []struct {
|
||||||
name string
|
name string
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue