diff --git a/pkg/tools/filesystem.go b/pkg/tools/filesystem.go index c206e8ad2..7e43440ac 100644 --- a/pkg/tools/filesystem.go +++ b/pkg/tools/filesystem.go @@ -540,6 +540,9 @@ func (t *ReadFileLinesTool) Execute(ctx context.Context, args map[string]any) *T if _, exists := args["length"]; exists { return ErrorResult("length is not supported in line mode; use max_lines") } + if _, exists := args["limit"]; exists { + return ErrorResult("limit is not supported in line mode; use max_lines") + } limit := int64(-1) if raw, exists := args["max_lines"]; exists && raw != nil { @@ -577,7 +580,8 @@ func (t *ReadFileLinesTool) Execute(ctx context.Context, args map[string]any) *T var content strings.Builder lineIndex := int64(1) var linesRead int64 - var bytesRead int64 + var fileBytesRead int64 + var outputBytesRead int64 var reachedEOF bool var byteBudgetTruncated bool var lineTruncated bool @@ -596,7 +600,7 @@ func (t *ReadFileLinesTool) Execute(ctx context.Context, args map[string]any) *T for !reachedEOF && (limit < 0 || linesRead < limit) { prefix := formatReadFileLinePrefix(lineIndex) - remaining := t.maxSize - bytesRead - int64(len(prefix)) + remaining := t.maxSize - outputBytesRead - int64(len(prefix)) if remaining <= 0 { byteBudgetTruncated = true break @@ -613,7 +617,8 @@ func (t *ReadFileLinesTool) Execute(ctx context.Context, args map[string]any) *T content.WriteString(prefix) content.Write(line) - bytesRead += int64(len(prefix) + len(line)) + fileBytesRead += int64(len(line)) + outputBytesRead += int64(len(prefix) + len(line)) linesRead++ lineIndex++ @@ -643,8 +648,8 @@ func (t *ReadFileLinesTool) Execute(ctx context.Context, args map[string]any) *T endLine := startLine + linesRead - 1 displayPath := filepath.Base(path) header := fmt.Sprintf( - "[file: %s | read: lines %d-%d (1-indexed) | bytes: %d]", - displayPath, start, endLine, bytesRead, + "[file: %s | read: lines %d-%d (1-indexed) | file_bytes: %d | output_bytes: %d]", + displayPath, start, endLine, fileBytesRead, outputBytesRead, ) switch { @@ -679,11 +684,12 @@ func (t *ReadFileLinesTool) Execute(ctx context.Context, args map[string]any) *T logger.DebugCF("tool", "ReadFileTool execution completed successfully", map[string]any{ - "path": path, - "lines_read": linesRead, - "bytes_read": bytesRead, - "truncated": byteBudgetTruncated, - "tool": t.Name(), + "path": path, + "lines_read": linesRead, + "file_bytes_read": fileBytesRead, + "output_bytes_read": outputBytesRead, + "truncated": byteBudgetTruncated, + "tool": t.Name(), }) return NewToolResult(header + "\n\n" + content.String()) diff --git a/pkg/tools/filesystem_test.go b/pkg/tools/filesystem_test.go index 0249bdd86..341b04b18 100644 --- a/pkg/tools/filesystem_test.go +++ b/pkg/tools/filesystem_test.go @@ -1094,6 +1094,29 @@ func TestReadFileLinesTool_RejectsLength(t *testing.T) { } } +func TestReadFileLinesTool_RejectsLimit(t *testing.T) { + tmpDir := t.TempDir() + testFile := filepath.Join(tmpDir, "legacy_limit.txt") + + err := os.WriteFile(testFile, []byte("line 1\nline 2\n"), 0o644) + if err != nil { + t.Fatalf("Failed to write test file: %v", err) + } + + tool := NewReadFileLinesTool(tmpDir, false, MaxReadFileSize) + result := tool.Execute(context.Background(), map[string]any{ + "path": testFile, + "start_line": 1, + "limit": 1, + }) + if !result.IsError { + t.Fatalf("expected limit to be rejected, got success: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "limit is not supported in line mode; use max_lines") { + t.Fatalf("unexpected error for limit in line mode: %s", result.ForLLM) + } +} + func TestReadFileLinesTool_BinaryFileRejected(t *testing.T) { tmpDir := t.TempDir() testFile := filepath.Join(tmpDir, "binary.dat") @@ -1148,3 +1171,59 @@ func TestReadFileLinesTool_TruncatesSingleLongLineAtByteBudget(t *testing.T) { t.Fatalf("expected line prefix for the truncated line, got: %s", result.ForLLM) } } + +func TestReadFileLinesTool_NoTrailingNewline(t *testing.T) { + tmpDir := t.TempDir() + testFile := filepath.Join(tmpDir, "no_trailing_newline.txt") + + err := os.WriteFile(testFile, []byte("line 1\nline 2"), 0o644) + if err != nil { + t.Fatalf("Failed to write test file: %v", err) + } + + tool := NewReadFileLinesTool(tmpDir, false, MaxReadFileSize) + result := tool.Execute(context.Background(), map[string]any{ + "path": testFile, + "start_line": 1, + }) + if result.IsError { + t.Fatalf("Execute() error = %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "1|line 1\n2|line 2") { + t.Fatalf("expected final line without trailing newline to be preserved, got: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "[END OF FILE - no further content.]") { + t.Fatalf("expected EOF marker, got: %s", result.ForLLM) + } +} + +func TestReadFileLinesTool_ExactByteBudgetBoundaryIncludesPrefix(t *testing.T) { + tmpDir := t.TempDir() + testFile := filepath.Join(tmpDir, "exact_boundary.txt") + + err := os.WriteFile(testFile, []byte("1234567\nsecond line\n"), 0o644) + if err != nil { + t.Fatalf("Failed to write test file: %v", err) + } + + tool := NewReadFileLinesTool(tmpDir, false, 10) + result := tool.Execute(context.Background(), map[string]any{ + "path": testFile, + "start_line": 1, + }) + if result.IsError { + t.Fatalf("Execute() error = %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "1|1234567\n") { + t.Fatalf("expected first line to fit exactly in the byte budget with its prefix, got: %s", result.ForLLM) + } + if strings.Contains(result.ForLLM, "2|") { + t.Fatalf("expected second line to be excluded once the exact output byte budget was reached, got: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "file_bytes: 8 | output_bytes: 10") { + t.Fatalf("expected separate file/output byte counters, got: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "start_line=2") { + t.Fatalf("expected continuation at line 2, got: %s", result.ForLLM) + } +}