mirror of
https://github.com/alibaba/open-code-review.git
synced 2026-07-09 17:28:58 +00:00
refactor(diff): Simplify hunk resolution logic with helper functions
This change refactors the `resolveFromHunk` function by introducing several helper functions (`extractSideLines`, `matchConsecutive`) to improve readability and maintainability. The new implementation attempts to match against both new and old sides of the diff hunk, making comment resolution more robust.
This commit is contained in:
kite
parent
35424d18bf
commit
29564a891b
2 changed files with 606 additions and 42 deletions
|
|
@ -54,11 +54,17 @@ func ResolveLineNumbers(comments []model.LlmComment, diffs []model.Diff) []model
|
|||
return result
|
||||
}
|
||||
|
||||
// resolveFromHunk tries to find the startLine/endLine by matching ExistingCode
|
||||
// against "from" side lines (context + deleted) in the diff hunks.
|
||||
// Returns true on success (comments fields are mutated in place).
|
||||
// indexedLine pairs a normalized line with its absolute file line number.
|
||||
type indexedLine struct {
|
||||
lineNum int
|
||||
content string
|
||||
}
|
||||
|
||||
// resolveFromHunk tries to find startLine/endLine by matching ExistingCode
|
||||
// against hunk lines. It tries the new-side first (context + added lines →
|
||||
// new-file line numbers), then falls back to old-side (context + deleted →
|
||||
// old-file line numbers).
|
||||
func resolveFromHunk(d *model.Diff, cm *model.LlmComment) bool {
|
||||
// TODO: re-track with llm
|
||||
hunks := ParseHunks(d.Diff)
|
||||
if len(hunks) == 0 {
|
||||
return false
|
||||
|
|
@ -70,53 +76,77 @@ func resolveFromHunk(d *model.Diff, cm *model.LlmComment) bool {
|
|||
}
|
||||
|
||||
for i := range hunks {
|
||||
hunk := &hunks[i]
|
||||
offset := 0 // tracks position within old-file range
|
||||
newSide := extractSideLines(&hunks[i], true)
|
||||
if start, end, ok := matchConsecutive(newSide, targetLines); ok {
|
||||
cm.StartLine = start
|
||||
cm.EndLine = end
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
||||
lines := hunk.Lines
|
||||
for j := 0; j < len(lines); j++ {
|
||||
line := lines[j]
|
||||
|
||||
switch line.Type {
|
||||
case HunkAdded:
|
||||
// Added lines are TO-side only; don't affect old-file offset
|
||||
continue
|
||||
case HunkContext, HunkDeleted:
|
||||
// Both are FROM-side candidates
|
||||
if matchAt(lines, j, targetLines) {
|
||||
startLine := hunk.OldStart + offset
|
||||
endLine := startLine + len(targetLines) - 1
|
||||
cm.StartLine = startLine
|
||||
cm.EndLine = endLine
|
||||
return true
|
||||
}
|
||||
offset++
|
||||
}
|
||||
for i := range hunks {
|
||||
oldSide := extractSideLines(&hunks[i], false)
|
||||
if start, end, ok := matchConsecutive(oldSide, targetLines); ok {
|
||||
cm.StartLine = start
|
||||
cm.EndLine = end
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
// matchAt checks whether targetLines[i] matches the normalized content of
|
||||
// lines[startIndex+i] for all i, where each matched line must be a "from" side
|
||||
// line (context or deleted).
|
||||
func matchAt(lines []HunkLine, startIndex int, targetLines []string) bool {
|
||||
for i, target := range targetLines {
|
||||
idx := startIndex + i
|
||||
if idx >= len(lines) {
|
||||
return false
|
||||
}
|
||||
l := lines[idx]
|
||||
// Must be a "from" side line
|
||||
if l.Type != HunkContext && l.Type != HunkDeleted {
|
||||
return false
|
||||
}
|
||||
if normalizeLine(l.Content) != target {
|
||||
return false
|
||||
// extractSideLines extracts one side of the diff from a hunk.
|
||||
// When newSide is true, returns context+added lines with new-file line numbers.
|
||||
// When newSide is false, returns context+deleted lines with old-file line numbers.
|
||||
func extractSideLines(hunk *Hunk, newSide bool) []indexedLine {
|
||||
var result []indexedLine
|
||||
oldLine := hunk.OldStart
|
||||
newLine := hunk.NewStart
|
||||
|
||||
for _, l := range hunk.Lines {
|
||||
switch l.Type {
|
||||
case HunkContext:
|
||||
if newSide {
|
||||
result = append(result, indexedLine{newLine, normalizeLine(l.Content)})
|
||||
} else {
|
||||
result = append(result, indexedLine{oldLine, normalizeLine(l.Content)})
|
||||
}
|
||||
oldLine++
|
||||
newLine++
|
||||
case HunkAdded:
|
||||
if newSide {
|
||||
result = append(result, indexedLine{newLine, normalizeLine(l.Content)})
|
||||
}
|
||||
newLine++
|
||||
case HunkDeleted:
|
||||
if !newSide {
|
||||
result = append(result, indexedLine{oldLine, normalizeLine(l.Content)})
|
||||
}
|
||||
oldLine++
|
||||
}
|
||||
}
|
||||
return true
|
||||
return result
|
||||
}
|
||||
|
||||
// matchConsecutive scans sideLines for a consecutive run matching all targetLines.
|
||||
func matchConsecutive(sideLines []indexedLine, targetLines []string) (startLine, endLine int, found bool) {
|
||||
if len(targetLines) == 0 || len(sideLines) < len(targetLines) {
|
||||
return 0, 0, false
|
||||
}
|
||||
for i := 0; i <= len(sideLines)-len(targetLines); i++ {
|
||||
matched := true
|
||||
for j, target := range targetLines {
|
||||
if sideLines[i+j].content != target {
|
||||
matched = false
|
||||
break
|
||||
}
|
||||
}
|
||||
if matched {
|
||||
return sideLines[i].lineNum, sideLines[i+len(targetLines)-1].lineNum, true
|
||||
}
|
||||
}
|
||||
return 0, 0, false
|
||||
}
|
||||
|
||||
// resolveFromFileContent scans the new file content line-by-line for consecutive
|
||||
|
|
|
|||
|
|
@ -207,6 +207,280 @@ line2`)
|
|||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// extractSideLines unit tests
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
func TestExtractSideLines_NewSide(t *testing.T) {
|
||||
hunk := Hunk{
|
||||
OldStart: 10, OldCount: 3,
|
||||
NewStart: 10, NewCount: 4,
|
||||
Lines: []HunkLine{
|
||||
{HunkContext, " ctx := r.Context()"},
|
||||
{HunkDeleted, ` log.Print("old")`},
|
||||
{HunkAdded, ` log.Printf("new: %s", r.URL)`},
|
||||
{HunkContext, " err := process(ctx)"},
|
||||
},
|
||||
}
|
||||
|
||||
got := extractSideLines(&hunk, true)
|
||||
|
||||
want := []indexedLine{
|
||||
{10, `ctx := r.Context()`},
|
||||
{11, `log.Printf("new: %s", r.URL)`},
|
||||
{12, `err := process(ctx)`},
|
||||
}
|
||||
|
||||
if len(got) != len(want) {
|
||||
t.Fatalf("new-side: expected %d lines, got %d", len(want), len(got))
|
||||
}
|
||||
for i := range want {
|
||||
if got[i].lineNum != want[i].lineNum || got[i].content != want[i].content {
|
||||
t.Errorf("new-side[%d]: got {%d, %q}, want {%d, %q}",
|
||||
i, got[i].lineNum, got[i].content, want[i].lineNum, want[i].content)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestExtractSideLines_OldSide(t *testing.T) {
|
||||
hunk := Hunk{
|
||||
OldStart: 10, OldCount: 3,
|
||||
NewStart: 10, NewCount: 4,
|
||||
Lines: []HunkLine{
|
||||
{HunkContext, " ctx := r.Context()"},
|
||||
{HunkDeleted, ` log.Print("old")`},
|
||||
{HunkAdded, ` log.Printf("new: %s", r.URL)`},
|
||||
{HunkContext, " err := process(ctx)"},
|
||||
},
|
||||
}
|
||||
|
||||
got := extractSideLines(&hunk, false)
|
||||
|
||||
want := []indexedLine{
|
||||
{10, `ctx := r.Context()`},
|
||||
{11, `log.Print("old")`},
|
||||
{12, `err := process(ctx)`},
|
||||
}
|
||||
|
||||
if len(got) != len(want) {
|
||||
t.Fatalf("old-side: expected %d lines, got %d", len(want), len(got))
|
||||
}
|
||||
for i := range want {
|
||||
if got[i].lineNum != want[i].lineNum || got[i].content != want[i].content {
|
||||
t.Errorf("old-side[%d]: got {%d, %q}, want {%d, %q}",
|
||||
i, got[i].lineNum, got[i].content, want[i].lineNum, want[i].content)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestExtractSideLines_DivergentStartLines(t *testing.T) {
|
||||
hunk := Hunk{
|
||||
OldStart: 5, OldCount: 2,
|
||||
NewStart: 8, NewCount: 3,
|
||||
Lines: []HunkLine{
|
||||
{HunkContext, "A"},
|
||||
{HunkAdded, "B"},
|
||||
{HunkContext, "C"},
|
||||
},
|
||||
}
|
||||
|
||||
newSide := extractSideLines(&hunk, true)
|
||||
if len(newSide) != 3 {
|
||||
t.Fatalf("expected 3 new-side lines, got %d", len(newSide))
|
||||
}
|
||||
// new-side: A(8), B(9), C(10)
|
||||
wantNew := []int{8, 9, 10}
|
||||
for i, w := range wantNew {
|
||||
if newSide[i].lineNum != w {
|
||||
t.Errorf("new-side[%d].lineNum = %d, want %d", i, newSide[i].lineNum, w)
|
||||
}
|
||||
}
|
||||
|
||||
oldSide := extractSideLines(&hunk, false)
|
||||
if len(oldSide) != 2 {
|
||||
t.Fatalf("expected 2 old-side lines, got %d", len(oldSide))
|
||||
}
|
||||
// old-side: A(5), C(6)
|
||||
wantOld := []int{5, 6}
|
||||
for i, w := range wantOld {
|
||||
if oldSide[i].lineNum != w {
|
||||
t.Errorf("old-side[%d].lineNum = %d, want %d", i, oldSide[i].lineNum, w)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestExtractSideLines_OnlyAdded(t *testing.T) {
|
||||
hunk := Hunk{
|
||||
OldStart: 1, OldCount: 0,
|
||||
NewStart: 1, NewCount: 2,
|
||||
Lines: []HunkLine{
|
||||
{HunkAdded, "line1"},
|
||||
{HunkAdded, "line2"},
|
||||
},
|
||||
}
|
||||
|
||||
newSide := extractSideLines(&hunk, true)
|
||||
if len(newSide) != 2 {
|
||||
t.Fatalf("expected 2 new-side lines, got %d", len(newSide))
|
||||
}
|
||||
|
||||
oldSide := extractSideLines(&hunk, false)
|
||||
if len(oldSide) != 0 {
|
||||
t.Errorf("expected 0 old-side lines, got %d", len(oldSide))
|
||||
}
|
||||
}
|
||||
|
||||
func TestExtractSideLines_OnlyDeleted(t *testing.T) {
|
||||
hunk := Hunk{
|
||||
OldStart: 3, OldCount: 2,
|
||||
NewStart: 3, NewCount: 0,
|
||||
Lines: []HunkLine{
|
||||
{HunkDeleted, "old1"},
|
||||
{HunkDeleted, "old2"},
|
||||
},
|
||||
}
|
||||
|
||||
oldSide := extractSideLines(&hunk, false)
|
||||
if len(oldSide) != 2 {
|
||||
t.Fatalf("expected 2 old-side lines, got %d", len(oldSide))
|
||||
}
|
||||
if oldSide[0].lineNum != 3 || oldSide[1].lineNum != 4 {
|
||||
t.Errorf("expected line nums 3,4, got %d,%d", oldSide[0].lineNum, oldSide[1].lineNum)
|
||||
}
|
||||
|
||||
newSide := extractSideLines(&hunk, true)
|
||||
if len(newSide) != 0 {
|
||||
t.Errorf("expected 0 new-side lines, got %d", len(newSide))
|
||||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// matchConsecutive unit tests
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
func TestMatchConsecutive_SingleLine(t *testing.T) {
|
||||
lines := []indexedLine{{5, "hello"}, {6, "world"}, {7, "foo"}}
|
||||
start, end, ok := matchConsecutive(lines, []string{"world"})
|
||||
if !ok || start != 6 || end != 6 {
|
||||
t.Errorf("single-line: got (%d, %d, %v), want (6, 6, true)", start, end, ok)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMatchConsecutive_MultiLine(t *testing.T) {
|
||||
lines := []indexedLine{{1, "a"}, {2, "b"}, {3, "c"}, {4, "d"}}
|
||||
start, end, ok := matchConsecutive(lines, []string{"b", "c"})
|
||||
if !ok || start != 2 || end != 3 {
|
||||
t.Errorf("multi-line: got (%d, %d, %v), want (2, 3, true)", start, end, ok)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMatchConsecutive_NoMatch(t *testing.T) {
|
||||
lines := []indexedLine{{1, "a"}, {2, "b"}}
|
||||
_, _, ok := matchConsecutive(lines, []string{"x"})
|
||||
if ok {
|
||||
t.Errorf("expected no match")
|
||||
}
|
||||
}
|
||||
|
||||
func TestMatchConsecutive_FirstMatchWins(t *testing.T) {
|
||||
lines := []indexedLine{{10, "x"}, {11, "y"}, {20, "x"}, {21, "y"}}
|
||||
start, end, ok := matchConsecutive(lines, []string{"x", "y"})
|
||||
if !ok || start != 10 || end != 11 {
|
||||
t.Errorf("first match: got (%d, %d, %v), want (10, 11, true)", start, end, ok)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMatchConsecutive_TargetLongerThanLines(t *testing.T) {
|
||||
lines := []indexedLine{{1, "a"}}
|
||||
_, _, ok := matchConsecutive(lines, []string{"a", "b"})
|
||||
if ok {
|
||||
t.Errorf("expected no match when target is longer")
|
||||
}
|
||||
}
|
||||
|
||||
func TestMatchConsecutive_EmptySideLines(t *testing.T) {
|
||||
_, _, ok := matchConsecutive(nil, []string{"a"})
|
||||
if ok {
|
||||
t.Errorf("expected no match on empty side lines")
|
||||
}
|
||||
}
|
||||
|
||||
func TestMatchConsecutive_MatchAtEnd(t *testing.T) {
|
||||
lines := []indexedLine{{1, "a"}, {2, "b"}, {3, "c"}}
|
||||
start, end, ok := matchConsecutive(lines, []string{"b", "c"})
|
||||
if !ok || start != 2 || end != 3 {
|
||||
t.Errorf("match-at-end: got (%d, %d, %v), want (2, 3, true)", start, end, ok)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMatchConsecutive_MatchAtStart(t *testing.T) {
|
||||
lines := []indexedLine{{1, "a"}, {2, "b"}, {3, "c"}}
|
||||
start, end, ok := matchConsecutive(lines, []string{"a", "b"})
|
||||
if !ok || start != 1 || end != 2 {
|
||||
t.Errorf("match-at-start: got (%d, %d, %v), want (1, 2, true)", start, end, ok)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMatchConsecutive_ExactFull(t *testing.T) {
|
||||
lines := []indexedLine{{1, "a"}, {2, "b"}}
|
||||
start, end, ok := matchConsecutive(lines, []string{"a", "b"})
|
||||
if !ok || start != 1 || end != 2 {
|
||||
t.Errorf("exact-full: got (%d, %d, %v), want (1, 2, true)", start, end, ok)
|
||||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// resolveFromHunk integration tests
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
func TestResolveFromHunk_AddedLines(t *testing.T) {
|
||||
raw := `diff --git a/test.go b/test.go
|
||||
--- a/test.go
|
||||
+++ b/test.go
|
||||
@@ -3,3 +3,5 @@
|
||||
func main() {
|
||||
+ x := 1
|
||||
+ y := 2
|
||||
fmt.Println("hello")
|
||||
}`
|
||||
|
||||
diffs := []model.Diff{{NewPath: "test.go", Diff: raw}}
|
||||
comments := []model.LlmComment{
|
||||
{Path: "test.go", ExistingCode: ` x := 1
|
||||
y := 2`},
|
||||
}
|
||||
|
||||
result := ResolveLineNumbers(comments, diffs)
|
||||
cm := result[0]
|
||||
if cm.StartLine != 4 || cm.EndLine != 5 {
|
||||
t.Errorf("added lines: expected 4..5, got %d..%d", cm.StartLine, cm.EndLine)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveFromHunk_OldSideAcrossAddedLines(t *testing.T) {
|
||||
raw := `diff --git a/test.go b/test.go
|
||||
--- a/test.go
|
||||
+++ b/test.go
|
||||
@@ -5,3 +5,4 @@
|
||||
x := 1
|
||||
+ z := 99
|
||||
y := 2
|
||||
}`
|
||||
|
||||
diffs := []model.Diff{{NewPath: "test.go", Diff: raw}}
|
||||
comments := []model.LlmComment{
|
||||
{Path: "test.go", ExistingCode: ` x := 1
|
||||
y := 2`},
|
||||
}
|
||||
|
||||
result := ResolveLineNumbers(comments, diffs)
|
||||
cm := result[0]
|
||||
// Old-side extracts [x:=1(5), y:=2(6), }(7)] — consecutive after skipping added line
|
||||
if cm.StartLine != 5 || cm.EndLine != 6 {
|
||||
t.Errorf("old-side across added: expected 5..6, got %d..%d", cm.StartLine, cm.EndLine)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveFromHunk_ContextLinesOnly(t *testing.T) {
|
||||
// When existing_code matches context (unchanged) lines rather than deleted ones
|
||||
raw := `diff --git a/test.go b/test.go
|
||||
|
|
@ -232,3 +506,263 @@ func TestResolveFromHunk_ContextLinesOnly(t *testing.T) {
|
|||
t.Errorf("expected line 4, got %d", cm.StartLine)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveFromHunk_SingleAddedLine(t *testing.T) {
|
||||
raw := `diff --git a/test.go b/test.go
|
||||
--- a/test.go
|
||||
+++ b/test.go
|
||||
@@ -1,2 +1,3 @@
|
||||
package main
|
||||
+import "fmt"
|
||||
func main() {}`
|
||||
|
||||
diffs := []model.Diff{{NewPath: "test.go", Diff: raw}}
|
||||
comments := []model.LlmComment{
|
||||
{Path: "test.go", ExistingCode: `import "fmt"`},
|
||||
}
|
||||
|
||||
result := ResolveLineNumbers(comments, diffs)
|
||||
cm := result[0]
|
||||
if cm.StartLine != 2 || cm.EndLine != 2 {
|
||||
t.Errorf("single added line: expected 2..2, got %d..%d", cm.StartLine, cm.EndLine)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveFromHunk_NewSidePriority(t *testing.T) {
|
||||
// "fmt.Println" appears on both sides (context line).
|
||||
// OldStart differs from NewStart → verifies new-side wins with new-file line number.
|
||||
raw := `diff --git a/test.go b/test.go
|
||||
--- a/test.go
|
||||
+++ b/test.go
|
||||
@@ -5,3 +8,4 @@
|
||||
func main() {
|
||||
fmt.Println("hello")
|
||||
+ fmt.Println("world")
|
||||
}`
|
||||
|
||||
diffs := []model.Diff{{NewPath: "test.go", Diff: raw}}
|
||||
comments := []model.LlmComment{
|
||||
{Path: "test.go", ExistingCode: ` fmt.Println("hello")`},
|
||||
}
|
||||
|
||||
result := ResolveLineNumbers(comments, diffs)
|
||||
cm := result[0]
|
||||
// new-side: func main(8), fmt.Println("hello")(9), fmt.Println("world")(10), }(11)
|
||||
// old-side: func main(5), fmt.Println("hello")(6), }(7)
|
||||
// new-side wins → line 9
|
||||
if cm.StartLine != 9 {
|
||||
t.Errorf("new-side priority: expected 9, got %d", cm.StartLine)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveFromHunk_MultiHunkMatchInSecond(t *testing.T) {
|
||||
raw := `diff --git a/test.go b/test.go
|
||||
--- a/test.go
|
||||
+++ b/test.go
|
||||
@@ -2,3 +2,3 @@
|
||||
func foo() {
|
||||
- old1()
|
||||
+ new1()
|
||||
}
|
||||
@@ -20,3 +20,4 @@
|
||||
func bar() {
|
||||
+ added_in_bar()
|
||||
existing()
|
||||
}`
|
||||
|
||||
diffs := []model.Diff{{NewPath: "test.go", Diff: raw}}
|
||||
comments := []model.LlmComment{
|
||||
{Path: "test.go", ExistingCode: " added_in_bar()"},
|
||||
}
|
||||
|
||||
result := ResolveLineNumbers(comments, diffs)
|
||||
cm := result[0]
|
||||
// Second hunk @@ -20,3 +20,4: new-side → func bar(20), added_in_bar(21), existing(22), }(23)
|
||||
if cm.StartLine != 21 || cm.EndLine != 21 {
|
||||
t.Errorf("multi-hunk second: expected 21..21, got %d..%d", cm.StartLine, cm.EndLine)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveFromHunk_AddedWithContext(t *testing.T) {
|
||||
raw := `diff --git a/test.go b/test.go
|
||||
--- a/test.go
|
||||
+++ b/test.go
|
||||
@@ -10,3 +10,5 @@
|
||||
func process() {
|
||||
+ validate()
|
||||
+ transform()
|
||||
save()
|
||||
}`
|
||||
|
||||
diffs := []model.Diff{{NewPath: "test.go", Diff: raw}}
|
||||
// LLM provides added line + surrounding context
|
||||
comments := []model.LlmComment{
|
||||
{Path: "test.go", ExistingCode: ` validate()
|
||||
transform()
|
||||
save()`},
|
||||
}
|
||||
|
||||
result := ResolveLineNumbers(comments, diffs)
|
||||
cm := result[0]
|
||||
// new-side: process(10), validate(11), transform(12), save(13), }(14)
|
||||
if cm.StartLine != 11 || cm.EndLine != 13 {
|
||||
t.Errorf("added+context: expected 11..13, got %d..%d", cm.StartLine, cm.EndLine)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveFromHunk_NewSideAcrossDeletedLines(t *testing.T) {
|
||||
raw := `diff --git a/test.go b/test.go
|
||||
--- a/test.go
|
||||
+++ b/test.go
|
||||
@@ -5,4 +5,3 @@
|
||||
a := 1
|
||||
- unused := 0
|
||||
b := 2
|
||||
}`
|
||||
|
||||
diffs := []model.Diff{{NewPath: "test.go", Diff: raw}}
|
||||
// Target spans two context lines that have a deleted line between them in the hunk.
|
||||
// In the new file they are consecutive.
|
||||
comments := []model.LlmComment{
|
||||
{Path: "test.go", ExistingCode: ` a := 1
|
||||
b := 2`},
|
||||
}
|
||||
|
||||
result := ResolveLineNumbers(comments, diffs)
|
||||
cm := result[0]
|
||||
// new-side: a:=1(5), b:=2(6), }(7) — deleted line skipped, consecutive
|
||||
if cm.StartLine != 5 || cm.EndLine != 6 {
|
||||
t.Errorf("new-side across deleted: expected 5..6, got %d..%d", cm.StartLine, cm.EndLine)
|
||||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// ResolveLineNumbers integration tests (additional scenarios)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
func TestResolveLineNumbers_AlreadyResolved(t *testing.T) {
|
||||
diffs := []model.Diff{{NewPath: "test.go", Diff: testDiff}}
|
||||
comments := []model.LlmComment{
|
||||
{Path: "test.go", ExistingCode: `log.Print("handling request")`, StartLine: 99, EndLine: 99},
|
||||
}
|
||||
|
||||
result := ResolveLineNumbers(comments, diffs)
|
||||
if result[0].StartLine != 99 || result[0].EndLine != 99 {
|
||||
t.Errorf("already resolved: expected 99..99, got %d..%d", result[0].StartLine, result[0].EndLine)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveLineNumbers_MultipleCommentsOnSameFile(t *testing.T) {
|
||||
raw := `diff --git a/test.go b/test.go
|
||||
--- a/test.go
|
||||
+++ b/test.go
|
||||
@@ -1,4 +1,6 @@
|
||||
package main
|
||||
+import "fmt"
|
||||
+import "os"
|
||||
func main() {
|
||||
- old()
|
||||
+ new()
|
||||
}`
|
||||
|
||||
diffs := []model.Diff{{NewPath: "test.go", Diff: raw}}
|
||||
comments := []model.LlmComment{
|
||||
{Path: "test.go", ExistingCode: `import "fmt"`},
|
||||
{Path: "test.go", ExistingCode: `import "os"`},
|
||||
{Path: "test.go", ExistingCode: " old()"},
|
||||
}
|
||||
|
||||
result := ResolveLineNumbers(comments, diffs)
|
||||
// comment 0: added import "fmt" → new-side line 2
|
||||
if result[0].StartLine != 2 || result[0].EndLine != 2 {
|
||||
t.Errorf("comment[0]: expected 2..2, got %d..%d", result[0].StartLine, result[0].EndLine)
|
||||
}
|
||||
// comment 1: added import "os" → new-side line 3
|
||||
if result[1].StartLine != 3 || result[1].EndLine != 3 {
|
||||
t.Errorf("comment[1]: expected 3..3, got %d..%d", result[1].StartLine, result[1].EndLine)
|
||||
}
|
||||
// comment 2: deleted old() → old-side; @@ -1,4: old-side lines: package(1), main(2), old(3), }(4)
|
||||
if result[2].StartLine != 3 || result[2].EndLine != 3 {
|
||||
t.Errorf("comment[2]: expected 3..3, got %d..%d", result[2].StartLine, result[2].EndLine)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveLineNumbers_OldPathMapping(t *testing.T) {
|
||||
raw := `diff --git a/old_name.go b/new_name.go
|
||||
--- a/old_name.go
|
||||
+++ b/new_name.go
|
||||
@@ -1,3 +1,3 @@
|
||||
package main
|
||||
-func oldFunc() {}
|
||||
+func newFunc() {}`
|
||||
|
||||
diffs := []model.Diff{{OldPath: "old_name.go", NewPath: "new_name.go", Diff: raw}}
|
||||
// Comment references old path — should still resolve via diffByPath[oldPath]
|
||||
comments := []model.LlmComment{
|
||||
{Path: "old_name.go", ExistingCode: "func oldFunc() {}"},
|
||||
}
|
||||
|
||||
result := ResolveLineNumbers(comments, diffs)
|
||||
cm := result[0]
|
||||
if cm.StartLine != 2 || cm.EndLine != 2 {
|
||||
t.Errorf("old path mapping: expected 2..2, got %d..%d", cm.StartLine, cm.EndLine)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveLineNumbers_MixedStrategies(t *testing.T) {
|
||||
raw := `diff --git a/test.go b/test.go
|
||||
--- a/test.go
|
||||
+++ b/test.go
|
||||
@@ -5,3 +5,4 @@
|
||||
func foo() {
|
||||
+ newLine()
|
||||
bar()
|
||||
}`
|
||||
|
||||
diffs := []model.Diff{
|
||||
{
|
||||
NewPath: "test.go", Diff: raw,
|
||||
NewFileContent: "package main\nimport \"fmt\"\n\nfunc helper() {}\nfunc foo() {\n newLine()\n bar()\n}",
|
||||
},
|
||||
}
|
||||
comments := []model.LlmComment{
|
||||
{Path: "test.go", ExistingCode: " newLine()"}, // hunk new-side
|
||||
{Path: "test.go", ExistingCode: "func helper() {}"}, // not in hunk, file content fallback
|
||||
{Path: "test.go", ExistingCode: "this_does_not_exist_anywhere()"}, // no match
|
||||
}
|
||||
|
||||
result := ResolveLineNumbers(comments, diffs)
|
||||
|
||||
if result[0].StartLine != 6 || result[0].EndLine != 6 {
|
||||
t.Errorf("hunk match: expected 6..6, got %d..%d", result[0].StartLine, result[0].EndLine)
|
||||
}
|
||||
if result[1].StartLine != 4 || result[1].EndLine != 4 {
|
||||
t.Errorf("file content fallback: expected 4..4, got %d..%d", result[1].StartLine, result[1].EndLine)
|
||||
}
|
||||
if result[2].StartLine != 0 || result[2].EndLine != 0 {
|
||||
t.Errorf("no match: expected 0..0, got %d..%d", result[2].StartLine, result[2].EndLine)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveLineNumbers_DiffMarkerInExistingCode(t *testing.T) {
|
||||
raw := `diff --git a/test.go b/test.go
|
||||
--- a/test.go
|
||||
+++ b/test.go
|
||||
@@ -1,2 +1,3 @@
|
||||
x := 1
|
||||
+y := 2
|
||||
z := 3`
|
||||
|
||||
diffs := []model.Diff{{NewPath: "test.go", Diff: raw}}
|
||||
// LLM may include the '+' marker from diff output
|
||||
comments := []model.LlmComment{
|
||||
{Path: "test.go", ExistingCode: "+y := 2"},
|
||||
}
|
||||
|
||||
result := ResolveLineNumbers(comments, diffs)
|
||||
cm := result[0]
|
||||
// normalizeLine strips leading '+', so "+y := 2" → "y := 2" matches
|
||||
if cm.StartLine != 2 || cm.EndLine != 2 {
|
||||
t.Errorf("diff marker in existing_code: expected 2..2, got %d..%d", cm.StartLine, cm.EndLine)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue