cmd/gadfly/consensus.go

@@ -382,6 +382,15 @@ func mergeIntoCluster(c *cluster, f outFinding, model string) {
 	if f.Line > c.maxLine {
 		c.maxLine = f.Line
 	}
+	// Backfill an empty title/detail from any report, regardless of severity, so a
+	// higher-severity-but-terse finding doesn't leave the cluster without context.
+	if strings.TrimSpace(c.title) == "" && strings.TrimSpace(f.Title) != "" {
+		c.title = f.Title
+	}
+	if strings.TrimSpace(c.detail) == "" && strings.TrimSpace(f.Detail) != "" {
+		c.detail = f.Detail
+	}
+	// A strictly-higher-severity report takes over the title/detail.
 	if sevRank(f.Severity) > sevRank(c.severity) {
 		c.severity = f.Severity
 		if strings.TrimSpace(f.Title) != "" {

cmd/gadfly/review.go

@@ -33,8 +33,8 @@ const (
 	// maxInlineComments caps how many inline comments one review carries, so a
 	// huge diff can't produce a wall of annotations. Clusters are pre-sorted by
 	// agreement×severity, so the cap keeps the most important.
-	maxInlineComments    = 25
-	inlineReviewHTTPTime = 20 * time.Second
+	maxInlineComments       = 25
+	inlineReviewHTTPTimeout = 20 * time.Second
 )
 
 // reviewComment is one inline comment in a Gitea pull review. Field names match
@@ -76,7 +76,7 @@ func postInlineReview(clusters []cluster) {
 		return // nothing anchors to a changed line; the consensus comment covers it
 	}
 
-	client := &http.Client{Timeout: inlineReviewHTTPTime}
+	client := &http.Client{Timeout: inlineReviewHTTPTimeout}
 	base := fmt.Sprintf("%s/pulls/%s/reviews", api, pr)
 	deletePriorReviews(client, base, token) // avoid stacking on re-runs
 
@@ -92,11 +92,12 @@ func postInlineReview(clusters []cluster) {
 func inlineComments(clusters []cluster, addable map[string]map[int]bool) []reviewComment {
 	var out []reviewComment
 	for _, c := range clusters {
-		lines := addable[c.file]
-		if lines == nil || c.line <= 0 || !lines[c.line] {
+		path := normPath(c.file)
+		anchor := anchorLine(addable[path], c.line, c.maxLine)
+		if anchor == 0 {
 			continue
 		}
-		out = append(out, reviewComment{Path: c.file, NewPosition: c.line, Body: inlineBody(c)})
+		out = append(out, reviewComment{Path: path, NewPosition: anchor, Body: inlineBody(c)})
 		if len(out) >= maxInlineComments {
 			break
 		}
@@ -104,6 +105,25 @@ func inlineComments(clusters []cluster, addable map[string]map[int]bool) []revie
 	return out
 }
 
+// anchorLine returns the first line in [lo,hi] that is an added line in the diff,
+// or 0 if none. Scanning the cluster's whole span (not just its representative
+// line) anchors a finding whose min line is just outside the diff but whose span
+// still overlaps a changed line.
+func anchorLine(added map[int]bool, lo, hi int) int {
+	if added == nil || lo <= 0 {
+		return 0
+	}
+	if hi < lo { // single-line cluster (maxLine unset)
+		hi = lo
+	}
+	for ln := lo; ln <= hi; ln++ {
+		if added[ln] {
+			return ln
+		}
+	}
+	return 0
+}
+
 // inlineBody renders one inline comment: severity + title, who flagged it, detail.
 func inlineBody(c cluster) string {
 	var b strings.Builder
@@ -116,11 +136,13 @@ func inlineBody(c cluster) string {
 	return b.String()
 }
 
-// parseDiffNewLines returns, per file, the set of NEW-file line numbers that are
-// present in the unified diff (added '+' or context ' ' lines) — the lines an
-// inline comment can anchor to. Hunk lengths from the @@ header bound each hunk,
-// so a content line that happens to start with "+++ " or "@@" is still read as
-// content, not mistaken for a header.
+// parseDiffNewLines returns, per file, the set of NEW-file line numbers that were
+// ADDED in the unified diff — the safest lines for an inline comment to anchor to
+// (Gitea reliably accepts comments on added lines). Context lines are walked to
+// keep the line counter correct but are NOT recorded: anchoring only to added
+// lines avoids the all-or-nothing review POST being rejected for an off-change
+// anchor. Hunk lengths from the @@ header bound each hunk, so a content line that
+// happens to start with "+++ " or "@@" is still read as content, not a header.
 func parseDiffNewLines(diff string) map[string]map[int]bool {
 	out := map[string]map[int]bool{}
 	var file string
@@ -130,14 +152,13 @@ func parseDiffNewLines(diff string) map[string]map[int]bool {
 		if inHunk && (newRem > 0 || oldRem > 0) {
 			switch {
 			case strings.HasPrefix(line, "+"):
-				record(out, file, newLine)
+				record(out, file, newLine) // added line — anchorable
 				newLine++
 				newRem--
 			case strings.HasPrefix(line, "-"):
 				oldRem--
 			case strings.HasPrefix(line, "\\"): // "\ No newline at end of file"
-			default: // context line (leading space, or an empty line)
-				record(out, file, newLine)
+			default: // context line (leading space, or an empty line): advance, don't record
 				newLine++
 				newRem--
 				oldRem--
@@ -149,8 +170,7 @@ func parseDiffNewLines(diff string) map[string]map[int]bool {
 		}
 		switch {
 		case strings.HasPrefix(line, "+++ "):
-			p := strings.TrimPrefix(strings.TrimPrefix(line, "+++ "), "b/")
-			file = strings.TrimSpace(p)
+			file = normPath(strings.TrimPrefix(line, "+++ "))
 			if file == "/dev/null" {
 				file = ""
 			}
@@ -170,6 +190,19 @@ func parseDiffNewLines(diff string) map[string]map[int]bool {
 // @@ -<oldStart>[,<oldLen>] +<newStart>[,<newLen>] @@
 var hunkRe = regexp.MustCompile(`^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@`)
 
+// normPath trims a unified-diff path down to a repo-relative one: strip a single
+// leading "a/" or "b/" prefix (and any "./"), and surrounding whitespace. Applied
+// to BOTH the diff paths and a finding's file so they match even when a model
+// writes "./pkg/x.go" or the diff carries the "b/" prefix.
+func normPath(p string) string {
+	p = strings.TrimSpace(p)
+	p = strings.TrimPrefix(p, "./")
+	if strings.HasPrefix(p, "a/") || strings.HasPrefix(p, "b/") {
+		p = p[2:]
+	}
+	return p
+}
+
 func record(out map[string]map[int]bool, file string, line int) {
 	if file == "" {
 		return
@@ -195,33 +228,44 @@ func atoiOr(s string, def int) int {
 // deletePriorReviews removes our previous inline reviews (matched by the body
 // marker) so a re-run replaces rather than stacks. Best-effort and quiet.
 func deletePriorReviews(client *http.Client, base, token string) {
-	req, err := http.NewRequest(http.MethodGet, base+"?limit=50", nil)
-	if err != nil {
-		return
-	}
-	req.Header.Set("Authorization", "token "+token)
-	resp, err := client.Do(req)
-	if err != nil {
-		return
-	}
-	var reviews []struct {
-		ID   int    `json:"id"`
-		Body string `json:"body"`
-	}
-	_ = json.NewDecoder(resp.Body).Decode(&reviews)
-	resp.Body.Close()
-	for _, r := range reviews {
-		if !strings.Contains(r.Body, inlineReviewMarker) {
-			continue
-		}
-		dreq, err := http.NewRequest(http.MethodDelete, fmt.Sprintf("%s/%d", base, r.ID), nil)
+	const perPage = 50
+	for page := 1; page <= 10; page++ { // bound the scan, but page past 50 so a stale marked review isn't missed
+		req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("%s?limit=%d&page=%d", base, perPage, page), nil)
 		if err != nil {
-			continue
+			return
 		}
-		dreq.Header.Set("Authorization", "token "+token)
-		if dresp, err := client.Do(dreq); err == nil {
-			io.Copy(io.Discard, dresp.Body)
-			dresp.Body.Close()
+		req.Header.Set("Authorization", "token "+token)
+		resp, err := client.Do(req)
+		if err != nil {
+			return
+		}
+		if resp.StatusCode < 200 || resp.StatusCode >= 300 {
+			io.Copy(io.Discard, resp.Body)
+			resp.Body.Close()
+			return
+		}
+		var reviews []struct {
+			ID   int    `json:"id"`
+			Body string `json:"body"`
+		}
+		_ = json.NewDecoder(resp.Body).Decode(&reviews)
+		resp.Body.Close()
+		for _, r := range reviews {
+			if !strings.Contains(r.Body, inlineReviewMarker) {
+				continue
+			}
+			dreq, err := http.NewRequest(http.MethodDelete, fmt.Sprintf("%s/%d", base, r.ID), nil)
+			if err != nil {
+				continue
+			}
+			dreq.Header.Set("Authorization", "token "+token)
+			if dresp, err := client.Do(dreq); err == nil {
+				io.Copy(io.Discard, dresp.Body)
+				dresp.Body.Close()
+			}
+		}
+		if len(reviews) < perPage {
+			return // last page
 		}
 	}
 }

cmd/gadfly/review_test.go

@@ -1,6 +1,9 @@
 package main
 
-import "testing"
+import (
+	"strings"
+	"testing"
+)
 
 func TestParseDiffNewLines(t *testing.T) {
 	diff := "diff --git a/a.go b/a.go\n" +
@@ -14,18 +17,19 @@ func TestParseDiffNewLines(t *testing.T) {
 		"+new3\n" +
 		" line4\n"
 	got := parseDiffNewLines(diff)
-	want := map[int]bool{1: true, 2: true, 3: true, 4: true} // context+added new-file lines
+	// Only ADDED lines anchor: new2 (line 2) and new3 (line 3). Context lines 1
+	// and 4 are walked for counting but not recorded.
+	want := map[int]bool{2: true, 3: true}
 	if len(got["a.go"]) != len(want) {
-		t.Fatalf("a.go lines = %v, want %v", got["a.go"], want)
+		t.Fatalf("a.go lines = %v, want %v (added-only)", got["a.go"], want)
 	}
 	for ln := range want {
 		if !got["a.go"][ln] {
 			t.Errorf("expected a.go line %d anchorable", ln)
 		}
 	}
-	// The deleted line's old number must NOT be an anchor target beyond the new set.
-	if got["a.go"][5] {
-		t.Error("line 5 should not be anchorable")
+	if got["a.go"][1] || got["a.go"][4] {
+		t.Error("context lines 1/4 should not be anchorable (added-only)")
 	}
 }
 
@@ -34,8 +38,22 @@ func TestParseDiffNewLinesContentLooksLikeHeader(t *testing.T) {
 	// diff. Hunk-length tracking must read it as content, not a file header.
 	diff := "--- a/x\n+++ b/x\n@@ -1,1 +1,2 @@\n ctx\n+++ weird\n"
 	got := parseDiffNewLines(diff)
-	if !got["x"][1] || !got["x"][2] {
-		t.Errorf("want lines 1 (ctx) and 2 (added) anchorable, got %v", got["x"])
+	if got["x"][1] { // context line, not recorded (added-only)
+		t.Errorf("line 1 is context, should not anchor: %v", got["x"])
+	}
+	if !got["x"][2] { // the "+++ weird" added line
+		t.Errorf("want added line 2 anchorable, got %v", got["x"])
+	}
+}
+
+func TestAnchorLineScansSpan(t *testing.T) {
+	// A cluster spanning 10..14 whose min line (10) isn't in the diff but whose
+	// span includes added line 14 must anchor to 14, not be dropped.
+	added := map[string]map[int]bool{"a.go": {14: true}}
+	clusters := []cluster{{file: "a.go", line: 10, maxLine: 14, severity: "high", title: "t", models: set("m1"), lenses: set("x")}}
+	cs := inlineComments(clusters, added)
+	if len(cs) != 1 || cs[0].NewPosition != 14 {
+		t.Fatalf("want anchor at 14 via span scan, got %+v", cs)
 	}
 }
 
@@ -51,6 +69,16 @@ func TestParseDiffNewLinesMultiFile(t *testing.T) {
 	}
 }
 
+func TestInlineCommentsNormalizesPaths(t *testing.T) {
+	// Diff path "b/pkg/x.go" -> "pkg/x.go"; a finding written as "./pkg/x.go" must
+	// still anchor (both normalize to the same repo-relative path).
+	addable := parseDiffNewLines("--- a/pkg/x.go\n+++ b/pkg/x.go\n@@ -0,0 +1,1 @@\n+bug\n")
+	clusters := []cluster{{file: "./pkg/x.go", line: 1, severity: "high", title: "t", models: set("m1"), lenses: set("x")}}
+	if cs := inlineComments(clusters, addable); len(cs) != 1 || cs[0].Path != "pkg/x.go" {
+		t.Errorf("path normalization failed to anchor: %+v", cs)
+	}
+}
+
 func TestInlineCommentsFiltersToDiffLines(t *testing.T) {
 	addable := map[string]map[int]bool{"a.go": {10: true, 11: true}}
 	clusters := []cluster{
@@ -65,7 +93,7 @@ func TestInlineCommentsFiltersToDiffLines(t *testing.T) {
 	if cs[0].Path != "a.go" || cs[0].NewPosition != 10 {
 		t.Errorf("wrong anchor: %+v", cs[0])
 	}
-	if !containsStr(cs[0].Body, "anchored") || !containsStr(cs[0].Body, "2 model") {
+	if !strings.Contains(cs[0].Body, "anchored") || !strings.Contains(cs[0].Body, "2 model") {
 		t.Errorf("inline body missing title/agreement: %q", cs[0].Body)
 	}
 }
@@ -77,12 +105,3 @@ func set(xs ...string) map[string]bool {
 	}
 	return m
 }
-
-func containsStr(s, sub string) bool {
-	for i := 0; i+len(sub) <= len(s); i++ {
-		if s[i:i+len(sub)] == sub {
-			return true
-		}
-	}
-	return false
-}