From 20cfb33cdd2ab973031230d2184f690484285e8e Mon Sep 17 00:00:00 2001
From: claw <claw@weiker.me>
Date: Wed, 13 May 2026 00:54:21 -0700
Subject: [PATCH] address review feedback: wrap ErrCannotDeleteSubmittedReview,
 fix nits

- Wrap ErrCannotDeleteSubmittedReview with operation context via fmt.Errorf
  so callers get both sentinel identity and context (MINOR fix)
- Combine double iteration in PostReview into single loop (NIT)
- Remove extra trailing blank line in review_test.go (NIT)
- Clarify translateGitHubReviewState comment re: PENDING state (NIT)
- Update requestOptions.bodyFn comment to mention DELETE-with-body (NIT)
---
 github/client.go      |  3 ++-
 github/review.go      | 13 +++++--------
 github/review_test.go |  2 --
 3 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/github/client.go b/github/client.go
index 2d5204e..f958bfa 100644
--- a/github/client.go
+++ b/github/client.go
@@ -196,7 +196,8 @@ func (c *Client) SetRetryBackoff(d []time.Duration) error {
 // requestOptions holds per-request configuration for doRequestCore.
 type requestOptions struct {
 	// bodyFn returns a fresh io.Reader for the request body on each attempt.
-	// Must be non-nil for requests that carry a body (POST, PUT, PATCH).
+	// Must be non-nil for any request that carries a body (POST, PUT, PATCH,
+	// or DELETE when a body is required by the API).
 	// Returning a fresh reader on each call allows retries to re-send the body.
 	bodyFn func() io.Reader
 
diff --git a/github/review.go b/github/review.go
index 785175b..7502fc7 100644
--- a/github/review.go
+++ b/github/review.go
@@ -59,7 +59,8 @@ func translateGitHubReviewState(state string) string {
 		return "COMMENT"
 	default:
 		// States like APPROVED, DISMISSED, and PENDING pass through unchanged
-		// as they already match the canonical vcs representation.
+		// as they already match the canonical vcs representation. PENDING appears
+		// on draft reviews that have not yet been submitted via the GitHub UI or API.
 		return state
 	}
 }
@@ -77,16 +78,12 @@ func (c *Client) PostReview(ctx context.Context, owner, repo string, number int,
 		Event: string(req.Event),
 	}
 
-	// Populate CommitID from the first comment if available.
+	// Populate CommitID from the first comment and build the payload in one pass.
 	// All comments in a single review share the same commit_id.
 	for _, comment := range req.Comments {
-		if comment.CommitID != "" {
+		if payload.CommitID == "" && comment.CommitID != "" {
 			payload.CommitID = comment.CommitID
-			break
 		}
-	}
-
-	for _, comment := range req.Comments {
 		payload.Comments = append(payload.Comments, reviewCommentEntry{
 			Path:     comment.Path,
 			Position: comment.Position,
@@ -159,7 +156,7 @@ func (c *Client) DeleteReview(ctx context.Context, owner, repo string, number in
 	if err != nil {
 		var apiErr *APIError
 		if errors.As(err, &apiErr) && apiErr.StatusCode == 422 {
-			return ErrCannotDeleteSubmittedReview
+			return fmt.Errorf("delete review: %w", ErrCannotDeleteSubmittedReview)
 		}
 		return fmt.Errorf("delete review: %w", err)
 	}
diff --git a/github/review_test.go b/github/review_test.go
index 1c3d0ac..da6535d 100644
--- a/github/review_test.go
+++ b/github/review_test.go
@@ -379,5 +379,3 @@ func TestTranslateGitHubReviewState(t *testing.T) {
 		}
 	}
 }
-
-