Compare commits

..

13 Commits

Author SHA1 Message Date
claw 93d5aa942c fix(cmd): remove duplicate doc comment and double blank line
PR Ready Gate / clear-labels (pull_request) Successful in 2s
CI / test (pull_request) Successful in 24s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 39s
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m39s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m59s
2026-05-13 13:19:43 +00:00
claw 7af82d895c docs(cmd,github): clarify type assertion and parameter usage in review superseding
Address sonnet-review feedback on PR #106:

- Document that the type assertion in supersedeOldReviews is guaranteed to
  succeed given the caller's provider switch, with the !ok branch guarding
  against future refactors (comment 18889).
- Clarify that vcsURL is only used in the Gitea path for constructing
  review permalink URLs (comment 18890).
- Add note explaining why the page-limit warning in ListReviews only fires
  when the final page is full, confirming the logic is intentional
  (comment 18891).
2026-05-13 13:19:43 +00:00
claw 0c73dfab60 fix: address self-review findings
- Remove dead code: findOwnReview (replaced by findAllOwnReviews)
- Check SetRetryBackoff return value in doJSONRequest tests
- Extract doWithRetry shared helper to eliminate ~100 lines of
  duplicated 429-retry/backoff/Retry-After logic between doRequest
  and doJSONRequest
- Fix import order: context before encoding/json (goimports)
- Add slog.Warn when ListReviews hits maxReviewPages limit
2026-05-13 13:19:43 +00:00
claw 002b60d438 fix: address review feedback on PR #106
- Add 429 rate-limit retry logic to doJSONRequest (matching doRequest
  behavior) so write operations (PostReview, DismissReview) properly
  retry when rate-limited by GitHub
- Remove redundant explicit case for ReviewEventComment in
  translateReviewEvent (default already handles it)
- Add ordering comment on --gitea-url alias registration explaining
  the dependency on registration-before-parse evaluation order
- Add tests for doJSONRequest retry/exhaust behavior
2026-05-13 13:19:43 +00:00
claw c4d1631242 fix(review): address bot review feedback on PR #106
- Document --gitea-url/--vcs-url last-one-wins behavior when both flags
  are passed simultaneously (sonnet MINOR #1)
- Move doJSONRequest from github/reviews.go to github/client.go where
  other HTTP helpers live (sonnet MINOR #2)
- Return joined error from supersedeOldReviews GitHub case instead of
  silently swallowing DismissReview failures (sonnet MINOR #3)
- Fix evaluateCIStatus to distinguish 'all checks passed' from 'no
  failures (N pending)' to avoid misleading status (gpt MINOR #2)
- Extract reviewsPerPage and maxReviewPages named constants for
  ListReviews pagination (gpt NIT #3)
2026-05-13 13:19:43 +00:00
claw 511e32463b fix(review): address inline review feedback on PR #106
- Reword misleading 'Fall through' comment to 'Continue to' in
  supersedeOldReviews (comment #18704)
- Add shared-pointer explanation comment for --gitea-url alias
  registration (comment #18703)
- Add comment clarifying CommitID same-commit expectation in
  PostReview (comment #18705)
- Rename 'hidden alias' to 'backward-compatible alias' in flag
  comment (comment #18708)
2026-05-13 13:19:43 +00:00
claw f1ad1dd15a fix(cmd): clarify empty gitea case control flow in supersedeOldReviews
The empty case "gitea": body exits the switch and continues to the
Gitea-specific logic below. Replace the vague comment with an explicit
note about the fall-through intent, per self-review feedback.
2026-05-13 13:19:43 +00:00
claw db62d71fdf fix(cmd,github): address review feedback on PR #106
- Replace panic() with fmt.Fprintf+os.Exit(1) in provider switch default
  (repo convention: never panic)
- Remove spurious 'event' field from DismissReview payload (GitHub dismiss
  endpoint only documents 'message')
- Change translateReviewEvent default to return 'COMMENT' as canonical
  fallback instead of passing unknown events through to GitHub API
- Refactor supersedeOldReviews to use explicit switch/case with default
  error for exhaustiveness
2026-05-13 13:19:43 +00:00
claw 24e3ab105a fix: address review feedback on PR #106
- Replace interface{} with any in github/reviews.go (Go 1.18+ idiom)
- Add default panic case to VCS client init switch
- Refactor supersedeOldReviews to return error instead of os.Exit(1)
- Remove spurious blank lines in formatter.go and formatter_test.go
- Add doc comment to DeleteReview explaining when to use vs DismissReview
- Sanitize extractSentinelName output to prevent log injection
2026-05-13 13:19:43 +00:00
claw aa41934e74 feat(cmd): wire --provider and --base-url flags into CLI
- Add --provider flag (gitea|github) for VCS backend selection
- Add --base-url flag for GitHub API endpoint configuration
- Rename --gitea-url to --vcs-url with backward-compatible alias
- Replace direct gitea.Client usage with vcs.Client interface
- Create vcs.Client via factory switch based on --provider value
- Implement Reviewer + Identity interfaces on github.Client
- Add verdictToEvent() using canonical vcs.ReviewEvent types
- Remove review.GiteaEvent() (replaced by verdictToEvent)
- GitHub supersede uses DismissReview; Gitea keeps EditComment flow
- Add VCS_PROVIDER, VCS_BASE_URL, VCS_URL env var support

Closes #82
2026-05-13 13:19:43 +00:00
aweiker 65ba8af244 Merge pull request 'fix(gitea): map hunk-header positions in BuildPositionToLineMap' (#104) from review-bot-issue-97 into feature/github-support
Reviewed-on: #104
Reviewed-by: security-review-bot <10+security-review-bot@noreply.gitea.weiker.me>
Reviewed-by: Aaron Weiker <aaron@weiker.org>
2026-05-13 13:15:30 +00:00
claw 02bdd701a5 test(gitea): add hunk-header-at-end error path test
PR Ready Gate / clear-labels (pull_request) Successful in 2s
CI / test (pull_request) Successful in 20s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 23s
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 34s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m39s
Adds TestTranslate_HunkHeaderAtEnd covering the edge case where a
hunk-header is the last position in the file with no subsequent
new-file line. Mirrors TestBuildPositionToLineMap_DeletionAtEnd for
the hunk-header code path.

Addresses NIT from sonnet-review-bot on PR #104 (comment 18412).
2026-05-12 23:32:22 -07:00
claw 23dc781908 fix(gitea): map hunk-header positions in BuildPositionToLineMap
CI / test (pull_request) Successful in 28s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 27s
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 44s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m49s
BuildPositionToLineMap incremented position and updated maxPositions for
@@ hunk-header lines but did not store a map entry, causing Translate()
to return a hard error for any comment positioned at a hunk header.

Store sentinel value 0 for hunk-header positions (analogous to -1 for
deletions) and extend Translate() to fall through to the nearest
context/addition line below, matching the existing deletion-line
behavior.

Fixes #97
2026-05-12 23:13:28 -07:00
2 changed files with 120 additions and 4 deletions
+11 -4
View File
@@ -11,6 +11,7 @@ import (
type PositionMap struct {
// files maps filename → (position → new-file line number).
// Deletion lines are mapped to -1 (no new-file line).
// Hunk-header lines are mapped to 0 (no new-file line).
files map[string]map[int]int
// maxPositions caches the highest position number per file,
// tracked during construction to avoid O(n) scans at translate time.
@@ -19,8 +20,8 @@ type PositionMap struct {
// Translate converts a GitHub diff-position to a new-file line number for a given file.
// Returns an error if the file is not in the diff or the position is out of range.
// If the position targets a deletion line, it maps to the nearest non-deletion line below;
// if no such line exists, returns an error.
// If the position targets a deletion or hunk-header line, it maps to the nearest
// context/addition line below; if no such line exists, returns an error.
func (pm *PositionMap) Translate(file string, position int) (int, error) {
if pm == nil || pm.files == nil {
return 0, fmt.Errorf("empty position map")
@@ -41,14 +42,18 @@ func (pm *PositionMap) Translate(file string, position int) (int, error) {
}
// lineNum == -1 means this position is a deletion line.
// Map to the nearest non-deletion line below.
if lineNum == -1 {
// lineNum == 0 means this position is a hunk-header line.
// Both map to the nearest context/addition line below.
if lineNum <= 0 {
maxPos := pm.maxPosition(file)
for p := position + 1; p <= maxPos; p++ {
if ln, exists := fileMap[p]; exists && ln > 0 {
return ln, nil
}
}
if lineNum == 0 {
return 0, fmt.Errorf("position %d targets a hunk-header line with no subsequent new-file line in %q", position, file)
}
return 0, fmt.Errorf("position %d targets a deletion line with no subsequent new-file line in %q", position, file)
}
@@ -70,6 +75,7 @@ func (pm *PositionMap) maxPosition(file string) int {
// - A new @@ hunk within the same file continues incrementing (does not reset)
// - Position maps to the new file line number for additions and context lines
// - Deletion lines have a position but no new-file line number (stored as -1)
// - Hunk-header lines have a position but no new-file line number (stored as 0)
func BuildPositionToLineMap(diff string) *PositionMap {
pm := &PositionMap{
files: make(map[string]map[int]int),
@@ -126,6 +132,7 @@ func BuildPositionToLineMap(diff string) *PositionMap {
// Parse hunk headers
if strings.HasPrefix(line, "@@") && currentFile != "" {
position++
pm.files[currentFile][position] = 0 // sentinel: hunk-header has no new-file line
pm.maxPositions[currentFile] = position
newLine = parseHunkStart(line)
continue
+109
View File
@@ -272,3 +272,112 @@ diff --git a/b.go b/b.go
t.Errorf("Translate(b.go, 3) = %d, want 2", got)
}
}
func TestTranslate_HunkHeaderPosition_SingleHunk(t *testing.T) {
// Position 1 is the @@ hunk-header line.
// It should resolve to the first context/addition line below (new line 16).
diff := `diff --git a/file.go b/file.go
index abc..def 100644
--- a/file.go
+++ b/file.go
@@ -16,4 +16,5 @@ func example() {
context line
-deleted line
+added line
context after
`
pm := BuildPositionToLineMap(diff)
got, err := pm.Translate("file.go", 1)
if err != nil {
t.Fatalf("Translate(file.go, 1): unexpected error: %v", err)
}
if got != 16 {
t.Errorf("Translate(file.go, 1) = %d, want 16 (first context/addition line in hunk)", got)
}
}
func TestTranslate_HunkHeaderPosition_MultiHunk(t *testing.T) {
// First hunk: @@ is pos 1, then " line1" (pos 2), "-old" (pos 3), "+new" (pos 4)
// Second hunk: @@ is pos 5, then " func foo() {" (pos 6), "+// added" (pos 7), etc.
// Translating position 5 (second @@) should resolve to new line 10.
diff := `diff --git a/file.go b/file.go
--- a/file.go
+++ b/file.go
@@ -1,3 +1,3 @@ package main
line1
-old
+new
@@ -10,3 +10,4 @@ func foo() {
func foo() {
+ // added
return
}
`
pm := BuildPositionToLineMap(diff)
// Position 5 is the second @@ hunk-header — should resolve to new line 10
got, err := pm.Translate("file.go", 5)
if err != nil {
t.Fatalf("Translate(file.go, 5): unexpected error: %v", err)
}
if got != 10 {
t.Errorf("Translate(file.go, 5) = %d, want 10 (first context/addition line in second hunk)", got)
}
// Also verify first hunk header at position 1 resolves to new line 1
got, err = pm.Translate("file.go", 1)
if err != nil {
t.Fatalf("Translate(file.go, 1): unexpected error: %v", err)
}
if got != 1 {
t.Errorf("Translate(file.go, 1) = %d, want 1 (first context/addition line in first hunk)", got)
}
}
func TestTranslate_HunkHeaderPosition_NewFile(t *testing.T) {
// New file: @@ -0,0 +1,3 @@ is position 1.
// Should resolve to new line 1 (the first addition).
diff := `diff --git a/new.go b/new.go
new file mode 100644
--- /dev/null
+++ b/new.go
@@ -0,0 +1,3 @@
+package main
+
+func init() {}
`
pm := BuildPositionToLineMap(diff)
got, err := pm.Translate("new.go", 1)
if err != nil {
t.Fatalf("Translate(new.go, 1): unexpected error: %v", err)
}
if got != 1 {
t.Errorf("Translate(new.go, 1) = %d, want 1 (first addition line)", got)
}
}
func TestTranslate_HunkHeaderAtEnd(t *testing.T) {
// A hunk-header at the last position with no subsequent new-file line should error.
// This is the hunk-header equivalent of TestBuildPositionToLineMap_DeletionAtEnd.
diff := `diff --git a/file.go b/file.go
--- a/file.go
+++ b/file.go
@@ -1,2 +1,2 @@ package main
line1
-old
+new
@@ -10,2 +10,1 @@ func foo() {
-removed
`
pm := BuildPositionToLineMap(diff)
// Position 5 is the second @@ hunk-header; the only line after it (pos 6) is a
// deletion (lineNum == -1), so there's no positive new-file line to resolve to.
// The hunk-header lookup should fail.
_, err := pm.Translate("file.go", 5)
if err == nil {
t.Error("expected error for hunk-header at end with no subsequent new-file line")
}
}