fix(tools/view): fix view paging, test for edge cases

Christian Rocha created 2 weeks ago

Change summary

internal/agent/tools/view.go      |  6 +-
internal/agent/tools/view_test.go | 87 +++++++++++++++++++++++++++++++++
2 files changed, 90 insertions(+), 3 deletions(-)

Detailed changes

internal/agent/tools/view.go

@@ -272,7 +272,7 @@ func readTextFile(filePath string, offset, limit int) (string, bool, error) {
 	// Pre-allocate slice with expected capacity.
 	lines := make([]string, 0, limit)
 
-	for scanner.Scan() && len(lines) < limit {
+	for len(lines) < limit && scanner.Scan() {
 		lineText := scanner.Text()
 		if len(lineText) > MaxLineLength {
 			lineText = lineText[:MaxLineLength] + "..."
@@ -280,8 +280,8 @@ func readTextFile(filePath string, offset, limit int) (string, bool, error) {
 		lines = append(lines, lineText)
 	}
 
-	// Peek one more line to determine if the file has more content.
-	hasMore := scanner.Scan()
+	// Peek one more line only when we filled the limit.
+	hasMore := len(lines) == limit && scanner.Scan()
 
 	if err := scanner.Err(); err != nil {
 		return "", false, err

internal/agent/tools/view_test.go

@@ -0,0 +1,87 @@
+package tools
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestReadTextFileBoundaryCases(t *testing.T) {
+	t.Parallel()
+
+	tmpDir := t.TempDir()
+	filePath := filepath.Join(tmpDir, "sample.txt")
+
+	var allLines []string
+	for i := range 5 {
+		allLines = append(allLines, fmt.Sprintf("line %d", i+1))
+	}
+	require.NoError(t, os.WriteFile(filePath, []byte(strings.Join(allLines, "\n")), 0o644))
+
+	tests := []struct {
+		name        string
+		offset      int
+		limit       int
+		wantContent string
+		wantHasMore bool
+	}{
+		{
+			name:        "exactly limit lines remaining",
+			offset:      0,
+			limit:       5,
+			wantContent: "line 1\nline 2\nline 3\nline 4\nline 5",
+			wantHasMore: false,
+		},
+		{
+			name:        "limit plus one line remaining",
+			offset:      0,
+			limit:       4,
+			wantContent: "line 1\nline 2\nline 3\nline 4",
+			wantHasMore: true,
+		},
+		{
+			name:        "offset at last line",
+			offset:      4,
+			limit:       3,
+			wantContent: "line 5",
+			wantHasMore: false,
+		},
+		{
+			name:        "offset beyond eof",
+			offset:      10,
+			limit:       3,
+			wantContent: "",
+			wantHasMore: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			gotContent, gotHasMore, err := readTextFile(filePath, tt.offset, tt.limit)
+			require.NoError(t, err)
+			require.Equal(t, tt.wantContent, gotContent)
+			require.Equal(t, tt.wantHasMore, gotHasMore)
+		})
+	}
+}
+
+func TestReadTextFileTruncatesLongLines(t *testing.T) {
+	t.Parallel()
+
+	tmpDir := t.TempDir()
+	filePath := filepath.Join(tmpDir, "longline.txt")
+
+	longLine := strings.Repeat("a", MaxLineLength+10)
+	require.NoError(t, os.WriteFile(filePath, []byte(longLine), 0o644))
+
+	content, hasMore, err := readTextFile(filePath, 0, 1)
+	require.NoError(t, err)
+	require.False(t, hasMore)
+	require.Equal(t, strings.Repeat("a", MaxLineLength)+"...", content)
+}