feat: catch potential errors and fix them

[afuhflynn]

Summary by CodeRabbit

• Chores
  • Refactored review operations to use async queuing model with improved error handling and logging.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR refactors the pull request review flow by simplifying method signatures and enhancing error handling. Changes include removing redundant parameters from the reviewPullRequest function, updating the getPullRequestDiff return value to include diff metrics, wrapping enqueue operations with error handling, and updating messaging from completion/failure semantics to asynchronous queuing semantics.

Changes

Cohort / File(s)	Change Summary
Webhook handler updates app/api/webhooks/github/route.ts	Removed changed_files, additions, and deletions arguments from reviewPullRequest call; updated log messages from "Review completed" to "Review queued" and error messages from "Review failed" to "Review queue failed"
AI actions refactoring lib/ai/actions/index.ts	Simplified reviewPullRequest signature by removing changedFiles, additions, and deletions parameters; updated PR diff field extraction mapping; replaced direct inngest.send calls with try-catch error handling blocks; changed missing persona behavior from throwing error to logging and returning { success: false }; updated event enqueue logic with error handling for both summary and review events
GitHub utilities enhancement lib/github-utils/actions/index.ts	Extended getPullRequestDiff return object to include three new fields: changed_files: number, additions: number, and deletions: number, populated from corresponding PR metadata

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• feat: add some new changes #49: Modifies event-sending and enqueue logic in lib/ai/actions/index.ts, including changes to event ID fields and error handling patterns
• feat: change inngest functions positions #48: Updates both webhook handler and AI actions invocation, including argument and signature changes to reviewPullRequest
• feat: update hooks and inngest functions #40: Refactors reviewPullRequest signature and webhook handler integration, directly affecting the same public API surface

Poem

🐰 The rabbit hops with glee so bright,
Trimmed the params, made the code tight,
Wrapped each enqueue in safe, sure care,
Now the reviews queue through the air! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check	❓ Inconclusive	The title is vague and generic; it uses non-descriptive terms like 'catch potential errors and fix them' that don't convey meaningful information about the specific changes.	Revise the title to specifically describe the main change, such as 'refactor: remove diff metrics from reviewPullRequest and add error handling' or 'refactor: extract diff metrics and add async queue error handling'.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch chore/octokit-app-upgrade

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

app/api/webhooks/github/route.ts (1)

110-124: Missing return value check could mask failures.

The function reviewPullRequest can now return { success: false } without throwing (see line 79 in lib/ai/actions/index.ts when no persona is found). However, this webhook handler doesn't check the return value, so it will log "Review queued" even when the review was never actually queued.

🔎 Proposed fix to check return value

       if (action === "opened" || action === "synchronized") {
         try {
-          await reviewPullRequest(
+          const result = await reviewPullRequest(
             owner,
             repoName,
             prNumber,
             body?.installation?.id,
             pull_request?.base?.sha,
             pull_request?.head?.sha
           );
-          console.log(`Review queued for: ${repo} #${prNumber}`);
+          if (result?.success !== false) {
+            console.log(`Review queued for: ${repo} #${prNumber}`);
+          } else {
+            console.error(`Review queue failed for repo: ${repo} #${prNumber}: returned success=false`);
+          }
         } catch (error) {
           console.error(
             `Review queue failed for repo: ${repo} #${prNumber}: `,
             error
           );
         }
       }

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1117d85 and fae6309.

📒 Files selected for processing (3)

• app/api/webhooks/github/route.ts
• lib/ai/actions/index.ts
• lib/github-utils/actions/index.ts

🧰 Additional context used

🧬 Code graph analysis (1)

lib/ai/actions/index.ts (2)

lib/github-utils/actions/index.ts (1)

• getPullRequestDiff (205-237)

lib/inngest.ts (1)

• inngest (3-6)

🔇 Additional comments (6)

lib/github-utils/actions/index.ts (1)

233-235: LGTM! Backward-compatible API enhancement.

The addition of changed_files, additions, and deletions to the return value provides useful PR metrics without breaking existing consumers. The fields are properly sourced from the GitHub API response.

app/api/webhooks/github/route.ts (1)

118-118: LGTM! Log messages correctly reflect queuing semantics.

The updated messages from "completed"/"failed" to "queued"/"queue failed" accurately convey that the operation is asynchronous.

Also applies to: 121-121

lib/ai/actions/index.ts (4)

13-13: LGTM! Simplified function signature.

Removing changedFiles, additions, and deletions from the parameters is good refactoring since these values are now obtained directly from getPullRequestDiff (lines 46-47), eliminating redundant parameter passing.

---

46-47: LGTM! Correctly extracts new fields from getPullRequestDiff.

The destructuring properly retrieves the newly added changed_files, deletions, and additions fields that were added to the return value of getPullRequestDiff.

---

91-112: LGTM! Proper error handling for summary enqueue.

Wrapping the inngest.send call in a try-catch prevents summary enqueue failures from blocking the review enqueue operation, improving resilience. The error logging provides visibility into failures.

---

114-128: LGTM! Proper error handling for review enqueue.

The try-catch wrapper ensures that review enqueue failures are logged without propagating exceptions, maintaining a fire-and-forget pattern for the queue operations.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Behavior change: returning false instead of throwing.

This changes error propagation semantics. Previously, missing persona would throw an exception caught by the webhook handler. Now it returns { success: false }, but the webhook handler (app/api/webhooks/github/route.ts lines 110-124) doesn't check the return value, causing it to log "Review queued" even when no review was actually queued.

Note: There's also an existing typo at line 131: sucess instead of success, which means successful returns have a different key spelling than this failure return.

Consider either:

1. Keep throwing an error here (preserve existing behavior), OR
2. Update the webhook handler to check and handle the { success: false } return value (see my comment on the webhook file)

Also consider fixing the typo at line 131: sucess → success for consistency.

🤖 Prompt for AI Agents

In lib/ai/actions/index.ts around lines 78-79, the function currently logs "No
persona found" and returns { success: false } which changes error propagation
and breaks the webhook flow; revert to throwing an error instead of returning a
failure object so upstream webhook error handling works as before, or if you
prefer the new return-style contract, update the webhook handler to check the
returned value and handle failure cases (do not assume success), and fix the
typo at line 131 by changing "sucess" to "success" so returned objects use a
consistent key.