From 6d251474941c3630b8d00039e291c14b1802f596 Mon Sep 17 00:00:00 2001 From: zbl94 Date: Tue, 14 Jul 2026 18:07:30 +0000 Subject: [PATCH 1/2] Honor view_file line range and cap result size execViewFile read the whole file and ignored the agent's StartLine/EndLine, so viewing a large file returned its entire content as the tool result -- a ~900KB blob for a 3k-line CSV stalled the following turn. Implement the Antigravity view_file contract (1-indexed inclusive line range with slice-notation windowing) plus defensive caps: - StartLine/EndLine window (neither=first N lines; start-only=next N forward; end-only=previous N backward; both=precise range, capped to N). - viewFileMaxLines / viewFileMaxBytes caps so a large file can never blob. - ContentOffset honored as the read position within the windowed content. The result payload is just {"content": ...}; the view_file result schema is defined server-side, so no extra fields are added. Adds intArg/intArgOK helpers and tests for windowing, byte cap, offset read, and range honoring. --- .../antigravityinteractions_tools.go | 125 +++++++++- .../antigravityinteractions_tools_test.go | 217 ++++++++++++++++++ 2 files changed, 341 insertions(+), 1 deletion(-) create mode 100644 internal/harness/antigravityinteractions/antigravityinteractions_tools_test.go diff --git a/internal/harness/antigravityinteractions/antigravityinteractions_tools.go b/internal/harness/antigravityinteractions/antigravityinteractions_tools.go index e84f2019..d61267fe 100644 --- a/internal/harness/antigravityinteractions/antigravityinteractions_tools.go +++ b/internal/harness/antigravityinteractions/antigravityinteractions_tools.go @@ -21,6 +21,7 @@ import ( "os/exec" "path/filepath" "sort" + "strconv" "strings" "time" ) @@ -75,6 +76,16 @@ func (h *AntigravityInteractionsHarness) executeTool(ctx context.Context, call c // the step's own output (content, Output/ExitCode, results). // --------------------------------------------------------------------------- +// view_file result caps. Mirrors the Antigravity view_file contract: +// StartLine/EndLine are 1-indexed inclusive, at most viewFileMaxLines lines are +// returned per view, and content is byte-capped with a ContentOffset +// continuation so a large file can't blob a multi-hundred-KB tool result that +// stalls the turn / blows past API context limits. +const ( + viewFileMaxLines = 2000 + viewFileMaxBytes = 256 * 1024 // 256 KiB +) + func execViewFile(call capturedToolCall) any { path := stringArg(call.arguments, "AbsolutePath") if path == "" { @@ -84,7 +95,90 @@ func execViewFile(call capturedToolCall) any { if err != nil { return map[string]any{"error": fmt.Sprintf("view_file: %v", err)} } - return map[string]any{"content": string(data)} + + // Resolve the 1-indexed inclusive line window using slice notation (see + // resolveLineWindow), then apply the byte cap, honoring ContentOffset as the + // read position within the windowed content. The caps bound the result size so + // a large file can't blob; the returned payload is just the content slice (the + // result schema for view_file is defined server-side, so we don't add fields). + start, startSet := intArgOK(call.arguments, "StartLine") + end, endSet := intArgOK(call.arguments, "EndLine") + offset := intArg(call.arguments, "ContentOffset") + + windowed := resolveLineWindow(string(data), start, startSet, end, endSet) + content := applyByteWindow(windowed, offset) + + return map[string]any{"content": content} +} + +// resolveLineWindow returns the requested line window of content, following the +// Antigravity view_file slice notation over 1-indexed inclusive [start, end]: +// +// - neither set: the first viewFileMaxLines lines (or the whole file if smaller) +// - start only: viewFileMaxLines lines starting at start (forward window) +// - end only: viewFileMaxLines lines ending at end (backward window) +// - both set: lines [start, end], capped to viewFileMaxLines from start +// +// It returns the joined lines for the window. +func resolveLineWindow(content string, start int, startSet bool, end int, endSet bool) string { + lines := strings.Split(content, "\n") + // strings.Split on a trailing newline yields a final empty element; drop it so + // line counts match the file's logical lines. + if n := len(lines); n > 0 && lines[n-1] == "" { + lines = lines[:n-1] + } + total := len(lines) + if total == 0 { + return "" + } + + // Compute a 1-indexed inclusive [lo, hi] window per slice notation. + var lo, hi int + switch { + case !startSet && !endSet: + lo, hi = 1, viewFileMaxLines + case startSet && !endSet: + lo = start + hi = start + viewFileMaxLines - 1 + case !startSet && endSet: + hi = end + lo = end - viewFileMaxLines + 1 + default: // both set + lo, hi = start, end + if hi-lo+1 > viewFileMaxLines { + hi = lo + viewFileMaxLines - 1 + } + } + + // Clamp to the file bounds. + if lo < 1 { + lo = 1 + } + if hi > total { + hi = total + } + if lo > total || hi < lo { + // Window is entirely past EOF (or inverted after clamping): nothing to show. + return "" + } + + return strings.Join(lines[lo-1:hi], "\n") +} + +// applyByteWindow returns the slice of content starting at offset (the agent's +// ContentOffset), capped to viewFileMaxBytes so a large window can't blob. +func applyByteWindow(content string, offset int) string { + if offset < 0 { + offset = 0 + } + if offset > len(content) { + offset = len(content) + } + remaining := content[offset:] + if len(remaining) > viewFileMaxBytes { + return remaining[:viewFileMaxBytes] + } + return remaining } // runCommandTimeout bounds how long a single run_command may take, so a runaway @@ -352,3 +446,32 @@ func boolArg(args map[string]any, name string) bool { } return false } + +// intArg reads an integer argument. JSON numbers decode to float64, but tolerate +// int and numeric strings too. Returns 0 when absent or unparseable. +func intArg(args map[string]any, name string) int { + n, _ := intArgOK(args, name) + return n +} + +// intArgOK is like intArg but also reports whether the argument was present and +// parseable. Callers use the ok flag to distinguish "unset" from an explicit 0 +// (needed for view_file slice notation, where start-only vs end-only differ). +func intArgOK(args map[string]any, name string) (int, bool) { + if args == nil { + return 0, false + } + switch v := args[name].(type) { + case float64: + return int(v), true + case int: + return v, true + case int64: + return int(v), true + case string: + if n, err := strconv.Atoi(strings.TrimSpace(v)); err == nil { + return n, true + } + } + return 0, false +} diff --git a/internal/harness/antigravityinteractions/antigravityinteractions_tools_test.go b/internal/harness/antigravityinteractions/antigravityinteractions_tools_test.go new file mode 100644 index 00000000..989d161f --- /dev/null +++ b/internal/harness/antigravityinteractions/antigravityinteractions_tools_test.go @@ -0,0 +1,217 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package antigravityinteractions + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" +) + +// numberedLines returns "1\n2\n...\nN\n". +func numberedLines(n int) string { + var b strings.Builder + for i := 1; i <= n; i++ { + fmt.Fprintf(&b, "%d\n", i) + } + return b.String() +} + +func TestResolveLineWindow(t *testing.T) { + content := "l1\nl2\nl3\nl4\nl5\n" + tests := []struct { + name string + start int + startSet bool + end int + endSet bool + want string + }{ + {"neither set (small file)", 0, false, 0, false, "l1\nl2\nl3\nl4\nl5"}, + {"both set middle", 2, true, 4, true, "l2\nl3\nl4"}, + {"both set whole", 1, true, 5, true, "l1\nl2\nl3\nl4\nl5"}, + {"start only", 3, true, 0, false, "l3\nl4\nl5"}, + {"end only", 0, false, 3, true, "l1\nl2\nl3"}, + {"end past EOF clamps", 3, true, 999, true, "l3\nl4\nl5"}, + {"start past EOF empty", 99, true, 0, false, ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := resolveLineWindow(content, tt.start, tt.startSet, tt.end, tt.endSet) + if got != tt.want { + t.Errorf("content = %q, want %q", got, tt.want) + } + }) + } +} + +func TestResolveLineWindow_StartOnlyForwardWindow(t *testing.T) { + // start only => viewFileMaxLines lines starting at start (forward window). + content := numberedLines(viewFileMaxLines + 500) + got := resolveLineWindow(content, 10, true, 0, false) + lines := strings.Split(got, "\n") + if len(lines) != viewFileMaxLines { + t.Fatalf("got %d lines, want forward window of %d", len(lines), viewFileMaxLines) + } + if lines[0] != "10" { + t.Errorf("first line = %q, want %q (window starts at start)", lines[0], "10") + } +} + +func TestResolveLineWindow_EndOnlyBackwardWindow(t *testing.T) { + // end only => viewFileMaxLines lines ending at end (backward window). + total := viewFileMaxLines + 500 + content := numberedLines(total) + end := total // last line + got := resolveLineWindow(content, 0, false, end, true) + lines := strings.Split(got, "\n") + if len(lines) != viewFileMaxLines { + t.Fatalf("got %d lines, want backward window of %d", len(lines), viewFileMaxLines) + } + if lines[len(lines)-1] != fmt.Sprintf("%d", end) { + t.Errorf("last line = %q, want %q (window ends at end)", lines[len(lines)-1], fmt.Sprintf("%d", end)) + } +} + +func TestResolveLineWindow_NeitherSetCapsToMaxLines(t *testing.T) { + content := numberedLines(viewFileMaxLines + 100) + got := resolveLineWindow(content, 0, false, 0, false) + if n := len(strings.Split(got, "\n")); n != viewFileMaxLines { + t.Errorf("got %d lines, want cap %d", n, viewFileMaxLines) + } +} + +func TestApplyByteWindow(t *testing.T) { + t.Run("under cap", func(t *testing.T) { + if got := applyByteWindow("hello", 0); got != "hello" { + t.Errorf("got %q, want hello", got) + } + }) + t.Run("over cap truncates to viewFileMaxBytes", func(t *testing.T) { + big := strings.Repeat("x", viewFileMaxBytes+100) + if got := applyByteWindow(big, 0); len(got) != viewFileMaxBytes { + t.Errorf("got len %d, want %d", len(got), viewFileMaxBytes) + } + }) + t.Run("resume via offset", func(t *testing.T) { + big := strings.Repeat("x", viewFileMaxBytes+100) + if got := applyByteWindow(big, viewFileMaxBytes); len(got) != 100 { + t.Errorf("got len %d, want 100", len(got)) + } + }) + t.Run("offset past end is empty", func(t *testing.T) { + if got := applyByteWindow("abc", 999); got != "" { + t.Errorf("got %q, want empty", got) + } + }) +} + +func TestExecViewFile_HonorsRange(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "f.txt") + if err := os.WriteFile(path, []byte("a\nb\nc\nd\ne\n"), 0o644); err != nil { + t.Fatal(err) + } + res := execViewFile(capturedToolCall{arguments: map[string]any{ + "AbsolutePath": path, + "StartLine": float64(2), // JSON numbers arrive as float64 + "EndLine": float64(3), + }}) + m := res.(map[string]any) + if m["content"] != "b\nc" { + t.Errorf("content = %q, want %q", m["content"], "b\nc") + } +} + +func TestExecViewFile_LargeFileIsCapped(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "big.csv") + var b strings.Builder + for i := 0; i < 5000; i++ { + fmt.Fprintf(&b, "row-%d-with-some-padding\n", i) + } + if err := os.WriteFile(path, []byte(b.String()), 0o644); err != nil { + t.Fatal(err) + } + // No range requested (the regression: previously returned the whole file). + res := execViewFile(capturedToolCall{arguments: map[string]any{"AbsolutePath": path}}) + content := res.(map[string]any)["content"].(string) + if len(content) > viewFileMaxBytes { + t.Errorf("content = %d bytes, exceeds cap %d", len(content), viewFileMaxBytes) + } + if lines := len(strings.Split(content, "\n")); lines > viewFileMaxLines { + t.Errorf("content = %d lines, exceeds cap %d", lines, viewFileMaxLines) + } +} + +func TestExecViewFile_ContentOffsetReadsFromOffset(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "wide.txt") + // One long single line exceeding the byte cap. + if err := os.WriteFile(path, []byte(strings.Repeat("z", viewFileMaxBytes+50)), 0o644); err != nil { + t.Fatal(err) + } + // Default read is capped at viewFileMaxBytes. + first := execViewFile(capturedToolCall{arguments: map[string]any{"AbsolutePath": path}}) + if got := first.(map[string]any)["content"].(string); len(got) != viewFileMaxBytes { + t.Fatalf("first read = %d bytes, want cap %d", len(got), viewFileMaxBytes) + } + // Reading from ContentOffset returns the remaining bytes. + second := execViewFile(capturedToolCall{arguments: map[string]any{ + "AbsolutePath": path, + "ContentOffset": float64(viewFileMaxBytes), + }}) + if got := second.(map[string]any)["content"].(string); len(got) != 50 { + t.Errorf("offset read = %d bytes, want 50", len(got)) + } +} + +func TestExecViewFile_MissingPath(t *testing.T) { + res := execViewFile(capturedToolCall{arguments: map[string]any{}}) + m := res.(map[string]any) + if _, ok := m["error"]; !ok { + t.Errorf("expected error for missing AbsolutePath, got %+v", m) + } +} + +func TestIntArgOK(t *testing.T) { + args := map[string]any{ + "f": float64(7), + "i": 9, + "s": "12", + "z": float64(0), + "bad": "nope", + } + cases := []struct { + name string + wantN int + wantOK bool + }{ + {"f", 7, true}, + {"i", 9, true}, + {"s", 12, true}, + {"z", 0, true}, // explicit 0 is present + {"bad", 0, false}, + {"absent", 0, false}, + } + for _, c := range cases { + n, ok := intArgOK(args, c.name) + if n != c.wantN || ok != c.wantOK { + t.Errorf("intArgOK(%q) = (%d,%v), want (%d,%v)", c.name, n, ok, c.wantN, c.wantOK) + } + } +} From 1d442296a67160a7544b5b7bb55cfa36c1cf395a Mon Sep 17 00:00:00 2001 From: zbl94 Date: Wed, 15 Jul 2026 00:48:48 +0000 Subject: [PATCH 2/2] view_file: UTF-8-safe byte cap + pagination metadata Address review feedback on the byte cap: - applyByteWindow backs the cut off to the last complete UTF-8 rune so a multi-byte character straddling viewFileMaxBytes is never split (raw byte slicing could produce invalid UTF-8, corrupting the last char and JSON serialization). - execViewFile returns the metadata the server needs to distinguish a complete read from a paginated/byte-truncated one: content, start_line/ end_line (0-indexed inclusive served range), content_offset, line_range_bytes (total bytes of the line range pre-byte-cap), and num_lines/num_bytes. The server detects truncation via content_offset+len(content) < line_range_bytes and prompts the model to resume from that offset. A paired server change reads these fields instead of hardcoding start_line=0/end_line=last. Adds tests for UTF-8 boundary capping, pagination metadata, and resume. --- .../antigravityinteractions_tools.go | 74 ++++++-- .../antigravityinteractions_tools_test.go | 174 +++++++++++++++--- 2 files changed, 209 insertions(+), 39 deletions(-) diff --git a/internal/harness/antigravityinteractions/antigravityinteractions_tools.go b/internal/harness/antigravityinteractions/antigravityinteractions_tools.go index d61267fe..ad4b979f 100644 --- a/internal/harness/antigravityinteractions/antigravityinteractions_tools.go +++ b/internal/harness/antigravityinteractions/antigravityinteractions_tools.go @@ -24,6 +24,7 @@ import ( "strconv" "strings" "time" + "unicode/utf8" ) // executeTool runs a tool call the agent yielded and returns the result value to @@ -99,16 +100,39 @@ func execViewFile(call capturedToolCall) any { // Resolve the 1-indexed inclusive line window using slice notation (see // resolveLineWindow), then apply the byte cap, honoring ContentOffset as the // read position within the windowed content. The caps bound the result size so - // a large file can't blob; the returned payload is just the content slice (the - // result schema for view_file is defined server-side, so we don't add fields). + // a large file can't blob. start, startSet := intArgOK(call.arguments, "StartLine") end, endSet := intArgOK(call.arguments, "EndLine") offset := intArg(call.arguments, "ContentOffset") - windowed := resolveLineWindow(string(data), start, startSet, end, endSet) + whole := string(data) + windowed, lo, hi, totalLines := resolveLineWindow(whole, start, startSet, end, endSet) content := applyByteWindow(windowed, offset) - return map[string]any{"content": content} + // Return the metadata the server needs to distinguish a complete read from a + // paginated/byte-truncated one: + // - start_line/end_line: 0-indexed inclusive served line range (the result + // is 0-indexed; the tool-call StartLine/EndLine args are 1-indexed). + // - content_offset: byte offset within the line-range content this slice + // starts at (for pagination continuation). + // - line_range_bytes: total bytes of the selected line range *before* byte + // truncation; the server compares content_offset+len(content) against + // this to detect byte truncation. + // - num_lines/num_bytes: whole-file totals. + result := map[string]any{ + "content": content, + "content_offset": offset, + "line_range_bytes": len(windowed), + "num_lines": totalLines, + "num_bytes": len(whole), + } + // Only report a concrete served range when there was content to serve; + // otherwise leave the (0,0) default. + if lo > 0 && hi >= lo { + result["start_line"] = lo - 1 // 1-indexed -> 0-indexed + result["end_line"] = hi - 1 + } + return result } // resolveLineWindow returns the requested line window of content, following the @@ -119,21 +143,21 @@ func execViewFile(call capturedToolCall) any { // - end only: viewFileMaxLines lines ending at end (backward window) // - both set: lines [start, end], capped to viewFileMaxLines from start // -// It returns the joined lines for the window. -func resolveLineWindow(content string, start int, startSet bool, end int, endSet bool) string { +// It returns the joined window text, the served 1-indexed inclusive [lo, hi] +// range (0, 0 when nothing is served), and the whole-file total line count. +func resolveLineWindow(content string, start int, startSet bool, end int, endSet bool) (windowed string, lo, hi, total int) { lines := strings.Split(content, "\n") // strings.Split on a trailing newline yields a final empty element; drop it so // line counts match the file's logical lines. if n := len(lines); n > 0 && lines[n-1] == "" { lines = lines[:n-1] } - total := len(lines) + total = len(lines) if total == 0 { - return "" + return "", 0, 0, 0 } // Compute a 1-indexed inclusive [lo, hi] window per slice notation. - var lo, hi int switch { case !startSet && !endSet: lo, hi = 1, viewFileMaxLines @@ -159,14 +183,20 @@ func resolveLineWindow(content string, start int, startSet bool, end int, endSet } if lo > total || hi < lo { // Window is entirely past EOF (or inverted after clamping): nothing to show. - return "" + return "", 0, 0, total } - return strings.Join(lines[lo-1:hi], "\n") + return strings.Join(lines[lo-1:hi], "\n"), lo, hi, total } // applyByteWindow returns the slice of content starting at offset (the agent's -// ContentOffset), capped to viewFileMaxBytes so a large window can't blob. +// ContentOffset), capped to viewFileMaxBytes so a large window can't blob. When +// it truncates, the cut is backed off to the last complete UTF-8 rune so the +// returned string is always valid UTF-8 (never a split multi-byte character). +// +// Callers detect truncation and the resume point from the returned slice: the +// window was truncated when offset+len(out) < len(content), and the agent +// resumes by re-reading with ContentOffset == offset+len(out). func applyByteWindow(content string, offset int) string { if offset < 0 { offset = 0 @@ -175,10 +205,24 @@ func applyByteWindow(content string, offset int) string { offset = len(content) } remaining := content[offset:] - if len(remaining) > viewFileMaxBytes { - return remaining[:viewFileMaxBytes] + if len(remaining) <= viewFileMaxBytes { + return remaining + } + + // Cap at viewFileMaxBytes, then back off to a valid UTF-8 boundary so we + // don't split a multi-byte rune across the cut. + cut := viewFileMaxBytes + for cut > 0 && !utf8.RuneStart(remaining[cut]) { + cut-- + } + // remaining[cut] is now the start of a rune (or cut == 0). Guard the + // pathological case of a single rune larger than the cap: emit at least that + // rune so we always make forward progress. + if cut == 0 { + _, size := utf8.DecodeRuneInString(remaining) + cut = size } - return remaining + return remaining[:cut] } // runCommandTimeout bounds how long a single run_command may take, so a runaway diff --git a/internal/harness/antigravityinteractions/antigravityinteractions_tools_test.go b/internal/harness/antigravityinteractions/antigravityinteractions_tools_test.go index 989d161f..e38f3a36 100644 --- a/internal/harness/antigravityinteractions/antigravityinteractions_tools_test.go +++ b/internal/harness/antigravityinteractions/antigravityinteractions_tools_test.go @@ -20,6 +20,7 @@ import ( "path/filepath" "strings" "testing" + "unicode/utf8" ) // numberedLines returns "1\n2\n...\nN\n". @@ -51,7 +52,7 @@ func TestResolveLineWindow(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := resolveLineWindow(content, tt.start, tt.startSet, tt.end, tt.endSet) + got, _, _, _ := resolveLineWindow(content, tt.start, tt.startSet, tt.end, tt.endSet) if got != tt.want { t.Errorf("content = %q, want %q", got, tt.want) } @@ -62,7 +63,7 @@ func TestResolveLineWindow(t *testing.T) { func TestResolveLineWindow_StartOnlyForwardWindow(t *testing.T) { // start only => viewFileMaxLines lines starting at start (forward window). content := numberedLines(viewFileMaxLines + 500) - got := resolveLineWindow(content, 10, true, 0, false) + got, _, _, _ := resolveLineWindow(content, 10, true, 0, false) lines := strings.Split(got, "\n") if len(lines) != viewFileMaxLines { t.Fatalf("got %d lines, want forward window of %d", len(lines), viewFileMaxLines) @@ -77,7 +78,7 @@ func TestResolveLineWindow_EndOnlyBackwardWindow(t *testing.T) { total := viewFileMaxLines + 500 content := numberedLines(total) end := total // last line - got := resolveLineWindow(content, 0, false, end, true) + got, _, _, _ := resolveLineWindow(content, 0, false, end, true) lines := strings.Split(got, "\n") if len(lines) != viewFileMaxLines { t.Fatalf("got %d lines, want backward window of %d", len(lines), viewFileMaxLines) @@ -89,35 +90,74 @@ func TestResolveLineWindow_EndOnlyBackwardWindow(t *testing.T) { func TestResolveLineWindow_NeitherSetCapsToMaxLines(t *testing.T) { content := numberedLines(viewFileMaxLines + 100) - got := resolveLineWindow(content, 0, false, 0, false) + got, _, _, _ := resolveLineWindow(content, 0, false, 0, false) if n := len(strings.Split(got, "\n")); n != viewFileMaxLines { t.Errorf("got %d lines, want cap %d", n, viewFileMaxLines) } } func TestApplyByteWindow(t *testing.T) { - t.Run("under cap", func(t *testing.T) { - if got := applyByteWindow("hello", 0); got != "hello" { - t.Errorf("got %q, want hello", got) + // truncated reports whether applyByteWindow(content, offset)==out capped the + // window, i.e. there is still content past the returned slice. Callers derive + // this the same way from the wire fields (offset+len(content) < + // line_range_bytes). + truncated := func(content string, offset, gotLen int) bool { + return offset+gotLen < len(content) + } + + t.Run("under cap: not truncated", func(t *testing.T) { + got := applyByteWindow("hello", 0) + if got != "hello" || truncated("hello", 0, len(got)) { + t.Errorf("got %q (truncated=%v), want hello (false)", got, truncated("hello", 0, len(got))) } }) - t.Run("over cap truncates to viewFileMaxBytes", func(t *testing.T) { + t.Run("over cap truncates to viewFileMaxBytes + resumes at next", func(t *testing.T) { big := strings.Repeat("x", viewFileMaxBytes+100) - if got := applyByteWindow(big, 0); len(got) != viewFileMaxBytes { - t.Errorf("got len %d, want %d", len(got), viewFileMaxBytes) + got := applyByteWindow(big, 0) + if len(got) != viewFileMaxBytes || !truncated(big, 0, len(got)) { + t.Errorf("got len %d (truncated=%v), want %d (true)", len(got), truncated(big, 0, len(got)), viewFileMaxBytes) + } + // Resume offset is offset+len(got); the remainder is the trailing 100 bytes. + if next := len(got); next != viewFileMaxBytes { + t.Errorf("resume offset = %d, want %d", next, viewFileMaxBytes) } }) - t.Run("resume via offset", func(t *testing.T) { + t.Run("resume via offset returns remainder", func(t *testing.T) { big := strings.Repeat("x", viewFileMaxBytes+100) - if got := applyByteWindow(big, viewFileMaxBytes); len(got) != 100 { - t.Errorf("got len %d, want 100", len(got)) + got := applyByteWindow(big, viewFileMaxBytes) + if len(got) != 100 || truncated(big, viewFileMaxBytes, len(got)) { + t.Errorf("got len %d (truncated=%v), want 100 (false)", len(got), truncated(big, viewFileMaxBytes, len(got))) } }) t.Run("offset past end is empty", func(t *testing.T) { - if got := applyByteWindow("abc", 999); got != "" { + got := applyByteWindow("abc", 999) + if got != "" { t.Errorf("got %q, want empty", got) } }) + t.Run("caps at valid UTF-8 boundary (no split rune)", func(t *testing.T) { + // Fill just under the cap with ASCII, then place a 3-byte rune ("€" is + // U+20AC, 3 bytes) straddling the cap so a naive byte cut would split it. + const r = "€" // 3 bytes: E2 82 AC + pad := viewFileMaxBytes - 1 // cap lands 1 byte into the rune + content := strings.Repeat("a", pad) + r + "tail" + got := applyByteWindow(content, 0) + if !truncated(content, 0, len(got)) { + t.Fatal("expected truncation") + } + if !utf8.ValidString(got) { + t.Errorf("returned content is not valid UTF-8 (split rune): last bytes %x", got[len(got)-3:]) + } + // The rune must not be partially included: length backs off to pad. + if len(got) != pad { + t.Errorf("got len %d, want %d (backed off before the straddling rune)", len(got), pad) + } + // Resuming from offset+len(got) yields the rune intact at the front. + rest := applyByteWindow(content, len(got)) + if !strings.HasPrefix(rest, r) { + t.Errorf("resume did not start at the rune boundary: %.6q", rest) + } + }) } func TestExecViewFile_HonorsRange(t *testing.T) { @@ -158,25 +198,111 @@ func TestExecViewFile_LargeFileIsCapped(t *testing.T) { } } -func TestExecViewFile_ContentOffsetReadsFromOffset(t *testing.T) { +func TestExecViewFile_PaginationMetadataAndResume(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "wide.txt") + totalBytes := viewFileMaxBytes + 50 // One long single line exceeding the byte cap. - if err := os.WriteFile(path, []byte(strings.Repeat("z", viewFileMaxBytes+50)), 0o644); err != nil { + if err := os.WriteFile(path, []byte(strings.Repeat("z", totalBytes)), 0o644); err != nil { t.Fatal(err) } - // Default read is capped at viewFileMaxBytes. - first := execViewFile(capturedToolCall{arguments: map[string]any{"AbsolutePath": path}}) - if got := first.(map[string]any)["content"].(string); len(got) != viewFileMaxBytes { + // First read: capped at viewFileMaxBytes; reports pagination metadata so the + // converter can detect byte truncation (content_offset+len(content) < + // line_range_bytes). + first := execViewFile(capturedToolCall{arguments: map[string]any{"AbsolutePath": path}}).(map[string]any) + if got := first["content"].(string); len(got) != viewFileMaxBytes { t.Fatalf("first read = %d bytes, want cap %d", len(got), viewFileMaxBytes) } - // Reading from ContentOffset returns the remaining bytes. + if first["content_offset"] != 0 { + t.Errorf("content_offset = %v, want 0", first["content_offset"]) + } + if first["line_range_bytes"] != totalBytes { + t.Errorf("line_range_bytes = %v, want %d (pre-truncation total)", first["line_range_bytes"], totalBytes) + } + // Byte-truncation detectable: offset + len(content) < line_range_bytes. + if got := first["content_offset"].(int) + len(first["content"].(string)); got >= first["line_range_bytes"].(int) { + t.Errorf("offset+len = %d, want < line_range_bytes %d (should look truncated)", got, first["line_range_bytes"]) + } + + // Resume from where the first slice ended: returns the remaining 50 bytes. + resumeOffset := len(first["content"].(string)) // 0 + viewFileMaxBytes second := execViewFile(capturedToolCall{arguments: map[string]any{ "AbsolutePath": path, - "ContentOffset": float64(viewFileMaxBytes), - }}) - if got := second.(map[string]any)["content"].(string); len(got) != 50 { - t.Errorf("offset read = %d bytes, want 50", len(got)) + "ContentOffset": float64(resumeOffset), + }}).(map[string]any) + if got := second["content"].(string); len(got) != 50 { + t.Errorf("resume read = %d bytes, want 50", len(got)) + } + // Now complete: offset + len == line_range_bytes. + if got := second["content_offset"].(int) + len(second["content"].(string)); got != second["line_range_bytes"].(int) { + t.Errorf("resume offset+len = %d, want == line_range_bytes %d (complete)", got, second["line_range_bytes"]) + } +} + +func TestExecViewFile_MultiByteRuneStraddlesByteCap(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "wide_utf8.txt") + // One long single line: ASCII padding so the byte cap lands 1 byte into a + // 3-byte rune ("€" is U+20AC, E2 82 AC), followed by more content so the read + // is byte-truncated and must continue. + const r = "€" + pad := viewFileMaxBytes - 1 + content := strings.Repeat("a", pad) + r + strings.Repeat("b", 40) + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + + // First page: capped, backed off before the straddling rune -> valid UTF-8. + first := execViewFile(capturedToolCall{arguments: map[string]any{"AbsolutePath": path}}).(map[string]any) + firstContent := first["content"].(string) + if !utf8.ValidString(firstContent) { + t.Errorf("first page is not valid UTF-8 (split rune): last bytes %x", firstContent[len(firstContent)-3:]) + } + if len(firstContent) != pad { + t.Errorf("first page = %d bytes, want %d (backed off before the straddling rune)", len(firstContent), pad) + } + // Byte-truncation detectable: content_offset+len(content) < line_range_bytes. + if got := first["content_offset"].(int) + len(firstContent); got >= first["line_range_bytes"].(int) { + t.Errorf("offset+len = %d, want < line_range_bytes %d (should look truncated)", got, first["line_range_bytes"]) + } + + // Second page: resume from where the first ended. The rune must appear intact + // at the front and the page must be valid UTF-8. + resumeOffset := first["content_offset"].(int) + len(firstContent) + second := execViewFile(capturedToolCall{arguments: map[string]any{ + "AbsolutePath": path, + "ContentOffset": float64(resumeOffset), + }}).(map[string]any) + secondContent := second["content"].(string) + if !utf8.ValidString(secondContent) { + t.Errorf("resume page is not valid UTF-8: first bytes %x", secondContent[:3]) + } + if !strings.HasPrefix(secondContent, r) { + t.Errorf("resume page did not start at the rune boundary: %.6q", secondContent) + } + // Now complete: offset + len == line_range_bytes. + if got := second["content_offset"].(int) + len(secondContent); got != second["line_range_bytes"].(int) { + t.Errorf("resume offset+len = %d, want == line_range_bytes %d (complete)", got, second["line_range_bytes"]) + } +} + +func TestExecViewFile_CompleteReadMetadata(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "small.txt") + if err := os.WriteFile(path, []byte("a\nb\nc\n"), 0o644); err != nil { + t.Fatal(err) + } + res := execViewFile(capturedToolCall{arguments: map[string]any{"AbsolutePath": path}}).(map[string]any) + // 3 lines, served whole: start_line=0, end_line=2 (0-indexed inclusive). + if res["start_line"] != 0 || res["end_line"] != 2 { + t.Errorf("served range = (%v,%v), want (0,2)", res["start_line"], res["end_line"]) + } + if res["num_lines"] != 3 { + t.Errorf("num_lines = %v, want 3", res["num_lines"]) + } + // Complete (not byte-truncated): content_offset+len(content) == line_range_bytes. + if got := res["content_offset"].(int) + len(res["content"].(string)); got != res["line_range_bytes"].(int) { + t.Errorf("offset+len = %d, want == line_range_bytes %d (complete read)", got, res["line_range_bytes"]) } }