fix(diff,tool): read file content at reviewed ref in range/commit mode

file_find and finalizeDiff always read from the working tree, even in
range/commit mode where the review targets a specific git ref. This
caused inconsistent file versions compared to file_read and code_search
which correctly used git show. Fix file_find to use git ls-tree and
finalizeDiff to use git show when a ref is specified.
This commit is contained in:
kite 2026-06-04 15:20:14 +08:00
parent a53b16cff5
commit 99e6709603
3 changed files with 43 additions and 7 deletions

View file

@ -139,7 +139,15 @@ func (p *Provider) GetDiff() ([]model.Diff, error) {
}
}
diffs, err := ParseDiffText(combined.String(), p.repoDir)
var ref string
switch p.mode {
case ModeRange:
ref = p.to
case ModeCommit:
ref = p.commit
}
diffs, err := ParseDiffText(combined.String(), p.repoDir, ref)
if err != nil {
return nil, err
}

View file

@ -2,11 +2,14 @@
package diff
import (
"context"
"fmt"
"os"
"os/exec"
"path/filepath"
"regexp"
"strings"
"time"
"github.com/open-code-review/open-code-review/internal/model"
)
@ -19,18 +22,23 @@ var (
)
// ParseDiffText splits the unified diff text into per-file Diff structs.
func ParseDiffText(diffText string, repoDir string) ([]model.Diff, error) {
// ref, if non-empty, is a git ref used to read new-file content via
// git show instead of reading from the working tree.
func ParseDiffText(diffText string, repoDir string, ref string) ([]model.Diff, error) {
lines := strings.Split(diffText, "\n")
var diffs []model.Diff
var current *model.Diff
var buf strings.Builder
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
defer cancel()
for _, line := range lines {
if m := diffHeaderRe.FindStringSubmatch(line); m != nil {
// Flush previous diff
if current != nil {
current.Diff = strings.TrimSuffix(buf.String(), "\n")
finalizeDiff(current, repoDir)
finalizeDiff(ctx, current, repoDir, ref)
diffs = append(diffs, *current)
buf.Reset()
}
@ -66,19 +74,33 @@ func ParseDiffText(diffText string, repoDir string) ([]model.Diff, error) {
// Flush last diff
if current != nil {
current.Diff = strings.TrimSuffix(buf.String(), "\n")
finalizeDiff(current, repoDir)
finalizeDiff(ctx, current, repoDir, ref)
diffs = append(diffs, *current)
}
return diffs, nil
}
// finalizeDiff attempts to read the new file content from disk.
func finalizeDiff(d *model.Diff, repoDir string) {
// finalizeDiff reads the new file content. When ref is non-empty it uses
// git show to read the file at that ref; otherwise it reads from disk.
func finalizeDiff(ctx context.Context, d *model.Diff, repoDir string, ref string) {
if d.IsDeleted || d.NewPath == "/dev/null" {
d.NewPath = "/dev/null"
return
}
if ref != "" {
cmd := exec.CommandContext(ctx, "git", "-c", "core.quotepath=false",
"show", ref+":"+d.NewPath)
cmd.Dir = repoDir
output, err := cmd.Output()
if err != nil {
fmt.Fprintf(os.Stderr, "[ocr] WARNING: cannot read file %s at ref %s: %v\n",
d.NewPath, ref, err)
return
}
d.NewFileContent = string(output)
return
}
fullPath := filepath.Join(repoDir, d.NewPath)
content, err := os.ReadFile(fullPath)
if err != nil {

View file

@ -57,8 +57,14 @@ func (p *FileFindProvider) Execute(args map[string]any) (string, error) {
}
// listGitFiles returns tracked and untracked files (respecting .gitignore) via git ls-files.
// In range/commit mode it uses git ls-tree to list files at the reviewed ref.
func (p *FileFindProvider) listGitFiles() ([]string, error) {
cmd := exec.Command("git", "ls-files", "--cached", "--others", "--exclude-standard")
var cmd *exec.Cmd
if ref := p.FileReader.Ref; ref != "" {
cmd = exec.Command("git", "ls-tree", "-r", "--name-only", ref)
} else {
cmd = exec.Command("git", "ls-files", "--cached", "--others", "--exclude-standard")
}
cmd.Dir = p.FileReader.RepoDir
output, err := cmd.Output()
if err != nil {