From b43b86a4a5305ce4e68a13863de8e2356539ba1d Mon Sep 17 00:00:00 2001 From: Rodin Date: Sat, 9 May 2026 18:08:06 -0700 Subject: [PATCH 1/3] fix: skip posting review when HEAD moves during evaluation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a new push arrives while review-bot is processing, the review would be posted against a stale commit. This causes noise in the PR timeline with findings that reference code that no longer exists. Before posting, re-fetch PR metadata and compare HEAD SHA with the commit we evaluated against. If they differ, log a warning and exit successfully — a new workflow run should already be processing the new HEAD. Fixes #52 --- cmd/review-bot/main.go | 31 +++++++++++++++++++++++ cmd/review-bot/main_test.go | 50 +++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+) diff --git a/cmd/review-bot/main.go b/cmd/review-bot/main.go index 021ccc3..33da475 100644 --- a/cmd/review-bot/main.go +++ b/cmd/review-bot/main.go @@ -315,6 +315,24 @@ func main() { sentinel := fmt.Sprintf("", *reviewerName) + // Stale check: verify HEAD hasn't moved since we started + evaluatedSHA := pr.Head.Sha + var currentSHA string + currentPR, err := giteaClient.GetPullRequest(ctx, owner, repoName, prNumber) + if err != nil { + slog.Warn("could not re-fetch PR for stale check", "pr", prNumber, "error", err) + // currentSHA stays empty — shouldSkipStaleReview will return false + } else { + currentSHA = currentPR.Head.Sha + } + if shouldSkipStaleReview(evaluatedSHA, currentSHA) { + slog.Warn("HEAD moved during review — skipping stale review", + "evaluated", evaluatedSHA, + "current", currentSHA, + "pr", prNumber) + return + } + // Map findings to inline comments for lines present in the diff diffRanges := gitea.ParseDiffNewLines(diff) var inlineComments []gitea.ReviewComment @@ -666,3 +684,16 @@ func findAllOwnReviews(reviews []gitea.Review, sentinel string) []gitea.Review { } return result } + +// shouldSkipStaleReview reports whether to skip posting because HEAD moved. +// Returns true (skip) if evaluatedSHA differs from currentSHA. +// Returns false (don't skip) if: +// - SHAs match (no movement) +// - currentSHA is empty (re-fetch failed; prefer posting stale over failing) +func shouldSkipStaleReview(evaluatedSHA, currentSHA string) bool { + if currentSHA == "" { + // Re-fetch failed; better to post potentially stale than fail + return false + } + return evaluatedSHA != currentSHA +} diff --git a/cmd/review-bot/main_test.go b/cmd/review-bot/main_test.go index df1ce36..4c55d02 100644 --- a/cmd/review-bot/main_test.go +++ b/cmd/review-bot/main_test.go @@ -862,3 +862,53 @@ func TestFindAllOwnReviews(t *testing.T) { } } } + +func TestShouldSkipStaleReview(t *testing.T) { + tests := []struct { + name string + evaluatedSHA string + currentSHA string + wantSkip bool + }{ + { + name: "matching SHAs", + evaluatedSHA: "abc123def456", + currentSHA: "abc123def456", + wantSkip: false, + }, + { + name: "different SHAs", + evaluatedSHA: "abc123def456", + currentSHA: "xyz789abc123", + wantSkip: true, + }, + { + name: "empty current SHA (re-fetch failed)", + evaluatedSHA: "abc123def456", + currentSHA: "", + wantSkip: false, + }, + { + name: "both empty (edge case)", + evaluatedSHA: "", + currentSHA: "", + wantSkip: false, + }, + { + name: "only current empty", + evaluatedSHA: "abc123", + currentSHA: "", + wantSkip: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := shouldSkipStaleReview(tc.evaluatedSHA, tc.currentSHA) + if got != tc.wantSkip { + t.Errorf("shouldSkipStaleReview(%q, %q) = %v, want %v", + tc.evaluatedSHA, tc.currentSHA, got, tc.wantSkip) + } + }) + } +} From 6b615c77d5a0a94741d2938761f821490f2e83fb Mon Sep 17 00:00:00 2001 From: Rodin Date: Sun, 10 May 2026 03:15:04 -0700 Subject: [PATCH 2/3] ci: remove unavailable models from review matrix Models claude-sonnet-4-6, gpt-4.1, gpt-4.1-mini, and gpt-5-mini are not deployed on the LLM proxy, causing 502 errors. Keep only gpt-5 which is the only available model. --- .gitea/workflows/ci.yml | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index 39e0065..c866239 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -19,6 +19,8 @@ jobs: - run: go build -o review-bot ./cmd/review-bot # Self-review: builds from source since we're pre-release + # Note: claude-sonnet-4-6, gpt-4.1, gpt-4.1-mini, gpt-5-mini removed — + # not deployed on LLM proxy. Only gpt-5 is available. review: runs-on: ubuntu-24.04 if: github.event_name == 'pull_request' @@ -26,31 +28,11 @@ jobs: strategy: matrix: include: - - name: sonnet - token_secret: SONNET_REVIEW_TOKEN - provider: anthropic - llm_path: /anthropic/v1 - model: claude-sonnet-4-6 - name: gpt token_secret: GPT_REVIEW_TOKEN provider: openai llm_path: /openai/v1 model: gpt-5 - - name: gpt41 - token_secret: GPT_REVIEW_TOKEN - provider: openai - llm_path: /openai/v1 - model: gpt-4.1 - - name: gpt5-mini - token_secret: GPT_REVIEW_TOKEN - provider: openai - llm_path: /openai/v1 - model: gpt-5-mini - - name: gpt41-mini - token_secret: GPT_REVIEW_TOKEN - provider: openai - llm_path: /openai/v1 - model: gpt-4.1-mini - name: security token_secret: SECURITY_REVIEW_TOKEN provider: openai From ced1fa7ffd92e65f6acd21f1392dcb8046aec04e Mon Sep 17 00:00:00 2001 From: Rodin Date: Sun, 10 May 2026 08:23:10 -0700 Subject: [PATCH 3/3] ci: fix model names to match SAP AI Core deployments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Restore sonnet reviewer with correct model name (anthropic--claude-4.6-sonnet) - Remove gpt-4.1, gpt-4.1-mini, gpt-5-mini (not deployed on SAP AI Core) - Keep gpt-5 and security reviewers The previous model names (claude-sonnet-4-6, etc.) were incorrect — SAP AI Core uses 'anthropic--claude-4.6-sonnet' format. --- .gitea/workflows/ci.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index c866239..8c796de 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -19,8 +19,7 @@ jobs: - run: go build -o review-bot ./cmd/review-bot # Self-review: builds from source since we're pre-release - # Note: claude-sonnet-4-6, gpt-4.1, gpt-4.1-mini, gpt-5-mini removed — - # not deployed on LLM proxy. Only gpt-5 is available. + # Models configured to match SAP AI Core deployments review: runs-on: ubuntu-24.04 if: github.event_name == 'pull_request' @@ -28,6 +27,11 @@ jobs: strategy: matrix: include: + - name: sonnet + token_secret: SONNET_REVIEW_TOKEN + provider: anthropic + llm_path: /anthropic/v1 + model: anthropic--claude-4.6-sonnet - name: gpt token_secret: GPT_REVIEW_TOKEN provider: openai