fix: address review feedback on PR #102

- Separate maxPages into maxFilesPages and maxCheckRunPages constants
  for clarity (sonnet MINOR #1)
- Add parallel to CheckRunConclusions subtests (sonnet MINOR #2)
- Add TestGetCommitStatuses_CheckRunsErrorAfterStatusesSucceed test
  covering check-runs 500 after statuses succeed (sonnet MINOR #2)
- Expand mapCheckRunStatus doc comment with full mapping rules including
  cancelled/skipped/neutral rationale and unknown value behavior
  (sonnet MINOR #3, gpt MINOR #1)
- Expand GetPullRequest doc comment to mention error types returned
  (sonnet NIT #4)
- Add inline comment on Description field clarifying it holds raw
  conclusion value (gpt NIT #3)

github/pr.go

@@ -51,7 +51,10 @@ type checkRunsResponse struct {
 	} `json:"check_runs"`
 }
 
-// GetPullRequest fetches PR metadata.
+// GetPullRequest fetches PR metadata from the GitHub API.
+// Returns an *APIError wrapping the HTTP status on non-2xx responses (e.g.
+// IsNotFound for 404, IsUnauthorized for 401). Network and context errors
+// are wrapped but not typed as *APIError.
 func (c *Client) GetPullRequest(ctx context.Context, owner, repo string, number int) (*vcs.PullRequest, error) {
 	reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d", c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number)
 	body, err := c.doGet(ctx, reqURL)
@@ -82,9 +85,15 @@ func (c *Client) GetPullRequestDiff(ctx context.Context, owner, repo string, num
 	return string(body), nil
 }
 
-// maxPages is the upper bound on pagination loops to prevent unbounded iteration
-// in case the server returns a full page indefinitely.
-const maxPages = 100
+const (
+	// maxFilesPages is the upper bound on pagination loops for PR file listing,
+	// preventing unbounded iteration if the server always returns a full page.
+	maxFilesPages = 100
+
+	// maxCheckRunPages is the upper bound on pagination loops for check-run listing,
+	// preventing unbounded iteration if the server always returns a full page.
+	maxCheckRunPages = 100
+)
 
 // GetPullRequestFiles fetches the list of files changed in a PR.
 // Paginates through all pages (100 per page) to collect all files.
@@ -93,7 +102,7 @@ const maxPages = 100
 func (c *Client) GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]vcs.ChangedFile, error) {
 	var allFiles []vcs.ChangedFile
 
-	for page := 1; page <= maxPages; page++ {
+	for page := 1; page <= maxFilesPages; page++ {
 		reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/files?per_page=100&page=%d",
 			c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number, page)
 		body, err := c.doGet(ctx, reqURL)
@@ -154,7 +163,7 @@ func (c *Client) GetCommitStatuses(ctx context.Context, owner, repo, sha string)
 	}
 
 	// Fetch check runs (paginated)
-	for checkPage := 1; checkPage <= maxPages; checkPage++ {
+	for checkPage := 1; checkPage <= maxCheckRunPages; checkPage++ {
 		checkURL := fmt.Sprintf("%s/repos/%s/%s/commits/%s/check-runs?per_page=100&page=%d",
 			c.baseURL, url.PathEscape(owner), url.PathEscape(repo), url.PathEscape(sha), checkPage)
 		checkBody, err := c.doGet(ctx, checkURL)
@@ -169,7 +178,7 @@ func (c *Client) GetCommitStatuses(ctx context.Context, owner, repo, sha string)
 			result = append(result, vcs.CommitStatus{
 				Context:     cr.Name,
 				Status:      mapCheckRunStatus(cr.Conclusion),
-				Description: derefString(cr.Conclusion),
+				Description: derefString(cr.Conclusion), // raw conclusion value (e.g. "success", "failure", "skipped")
 				TargetURL:   cr.HTMLURL,
 			})
 		}
@@ -181,9 +190,17 @@ func (c *Client) GetCommitStatuses(ctx context.Context, owner, repo, sha string)
 	return result, nil
 }
 
-// mapCheckRunStatus maps a check run conclusion to a vcs.CommitStatus status string.
+// mapCheckRunStatus maps a GitHub check run conclusion to a vcs.CommitStatus status string.
 // Conclusion alone determines the mapped state: nil conclusion means the run is
 // still in progress (pending), regardless of the status field value.
+//
+// Mapping rules:
+//   - nil → "pending" (run still in progress or queued)
+//   - "success" → "success"
+//   - "failure", "action_required", "timed_out" → "failure"
+//   - "cancelled", "skipped", "neutral" → "success" (non-blocking per GitHub check suite semantics)
+//   - "stale", "waiting" → "pending"
+//   - unknown values → "pending" (conservative: treat unrecognized conclusions as incomplete)
 func mapCheckRunStatus(conclusion *string) string {
 	if conclusion == nil {
 		// Still running or queued