alibaba / alibaba/open-code-review
file_read_diff output is unbounded and can exhaust the LLM context window
- Dominant language
- Go
- Stars
- 24.4k
- Forks
- 1.8k
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 105
Description
## OpenCodeReview version
Built from source at `ec5f6851d2fb363ec36b9f283910145ef87cf5df` (`upstream/main` on 2026-08-17). The installed CLI used to inspect the environment is `v1.9.3 (c35ddd72)`.
## Operating system
macOS (Apple Silicon), Darwin 25.5.0 arm64.
## Installation method
Built from source.
## LLM provider
Provider-independent. This report covers the size of the tool result before it is sent to an LLM.
## Bug description
`file_read_diff` concatenates the complete diff for every requested path without any output budget, truncation marker, or continuation mechanism. A sufficiently large result can consume or exceed the context available to the next LLM request and interrupt the review loop.
The current implementation appends each complete diff to one `strings.Builder` and returns it directly:
- [`internal/tool/file_read_diff.go`](https://github.com/alibaba/open-code-review/blob/ec5f6851d2fb363ec36b9f283910145ef87cf5df/internal/tool/file_read_diff.go#L54-L73)
- [`file_read_diff` tool schema](https://github.com/alibaba/open-code-review/blob/ec5f6851d2fb363ec36b9f283910145ef87cf5df/internal/config/toolsconfig/tools.json#L160-L182), which exposes only `path_array` and no pagination parameter
This differs from other read-oriented tools such as `file_read`, which limits output and reports `IS_TRUNCATED`.
## Steps to reproduce
1. Save the following temporary test as `internal/tool/file_read_diff_repro_test.go` at commit `ec5f6851d2fb363ec36b9f283910145ef87cf5df`:
```go
// SPDX-License-Identifier: Apache-2.0
// Copyright 2026 alibaba/open-code-review Contributors
package tool
import (
"context"
"fmt"
"strings"
"testing"
)
func TestReproFileReadDiffUnboundedOutput(t *testing.T) {
const (
files = 10
changedPairs = 10000
)
diffs := make(map[string]string, files)
paths := make([]any, 0, files)
for fileIndex := 1; fileIndex <= files; fileIndex++ {
path := fmt.Sprintf("file-%02d.go", fileIndex)
var diff strings.Builder
fmt.Fprintf(&diff, "--- a/%s\n+++ b/%s\n@@ -1,%d +1,%d @@\n", path, path, changedPairs, changedPairs)
for line := 1; line <= changedPairs; line++ {
fmt.Fprintf(&diff, "-old line %06d from file %02d\n+new line %06d from file %02d\n", line, fileIndex, line, fileIndex)
}
diffs[path] = diff.String()
paths = append(paths, path)
}
provider := NewFileReadDiff(NewDiffMap(diffs))
result, err := provider.Execute(context.Background(), map[string]any{"path_array": paths})
if err != nil {
t.Fatal(err)
}
t.Logf("output_bytes=%d output_lines=%d", len(result), 1+strings.Count(result, "\n"))
if strings.Contains(result, "IS_TRUNCATED") || strings.Contains(result, "NEXT_OFFSET") {
t.Fatal("unexpected truncation or continuation metadata")
}
}
```
2. Run the repository-required unit-test entry point:
```shell
make test
```
3. Observe that `TestReproFileReadDiffUnboundedOutput` reports `output_bytes=6000860 output_lines=200051`, with neither truncation nor continuation metadata.
4. Remove the temporary test file after reproducing the issue.
Each synthetic changed-line pair has one removed line and one added line. The following table reports raw bytes and lines; it does not estimate tokens.
| Files | Changed line pairs per file | Input bytes | Output bytes | Output lines | Truncation marker | Paging metadata |
|---:|---:|---:|---:|---:|:---:|:---:|
| 1 | 10,000 | 600,058 | 600,086 | 20,006 | No | No |
| 3 | 10,000 | 1,800,174 | 1,800,258 | 60,016 | No | No |
| 10 | 10,000 | 6,000,580 | 6,000,860 | 200,051 | No | No |
| 10 | 50,000 | 30,000,580 | 30,000,860 | 1,000,051 | No | No |
All requested file markers were present in every result. The output grew linearly with the complete requested diff size; the only overhead was the per-file header and trailing newline.
This is a deterministic source-level reproduction. It does not claim that a specific LLM provider was observed returning a context-overflow error, and byte size is not treated as token count.
## Logs / Error output
N/A. This is a deterministic source-level reproduction and does not call an LLM provider.
## Expected behavior
`file_read_diff` should have a deterministic response budget. When the complete result does not fit:
1. the response should explicitly state that it is partial;
2. omitted content should remain retrievable through a follow-up call; and
3. tests should verify the output bound and continuation behavior for both a single large diff and multiple requested files.
## Proposed direction
Keep the existing `path_array` behavior compatible, add a bounded response with explicit partial-result metadata, and expose a cursor or offset that lets the model retrieve the remainder. The exact budget unit, limit, and continuation schema can be agreed before implementation rather than hard-coded in this report.
## Additional context
The existing `file_read_diff` tests cover small single-file and multi-file results but do not assert any response-size bound. I am happy to implement the agreed behavior and add focused unit tests.
Contributor guide
Research direction
Start with internal/tool/file_read_diff.go and the existing file_read_diff tests; run make test and the reproduction described in the issue to confirm current unbounded output. Review internal/config/toolsconfig/tools.json alongside the implementation before agreeing the response budget and continuation schema. Done means bounded output with explicit partial metadata, retrievable remainder, and focused tests for one large diff and multiple paths.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100