feat: default to using a single function for both pr review and summa…#50
feat: default to using a single function for both pr review and summa…#50
Conversation
…ry in the webhook
📝 WalkthroughWalkthroughThe changes consolidate PR processing from a two-step workflow (separate summary and review steps) into a single unified review operation. The Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/ai/actions/index.ts (1)
131-131: Fix typo in return object.The property name is misspelled as
sucessinstead ofsuccess.🔎 Proposed fix
return { - sucess: true, + success: true, message: "Review Queued", runId: run.id, };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/api/webhooks/github/route.tslib/ai/actions/index.ts
🧰 Additional context used
🧬 Code graph analysis (2)
lib/ai/actions/index.ts (1)
lib/inngest.ts (1)
inngest(3-6)
app/api/webhooks/github/route.ts (1)
lib/ai/actions/index.ts (1)
reviewPullRequest(7-163)
🔇 Additional comments (1)
app/api/webhooks/github/route.ts (1)
2-2: LGTM: Import updated correctly.The import statement correctly reflects the consolidated workflow, importing only
reviewPullRequestas thegeneratePullRequestSummaryfunction has been removed.
| await reviewPullRequest( | ||
| owner, | ||
| repoName, | ||
| prNumber, | ||
| pull_request?.title, | ||
| pull_request?.body, | ||
| body?.installation?.id, | ||
| pull_request?.base?.sha, | ||
| pull_request?.head?.sha, | ||
| pull_request?.changed_files, | ||
| pull_request?.additions, | ||
| pull_request?.deletions | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the files mentioned in the review
git ls-files | grep -E "(webhook|route)" | head -20Repository: afuhflynn/code-reverb
Length of output: 719
🏁 Script executed:
# Check the structure of the repository
git ls-files | grep -E "^app/|^lib/" | head -30Repository: afuhflynn/code-reverb
Length of output: 938
🏁 Script executed:
# Read the webhook route file around the mentioned lines
cat app/api/webhooks/github/route.ts | head -150Repository: afuhflynn/code-reverb
Length of output: 4512
🏁 Script executed:
# Check the lib/ai/actions file mentioned
cat -n lib/ai/actions/index.ts | head -50Repository: afuhflynn/code-reverb
Length of output: 1608
🏁 Script executed:
# Look for the reviewPullRequest function to see its signature
grep -r "reviewPullRequest" --include="*.ts" | head -20Repository: afuhflynn/code-reverb
Length of output: 284
🏁 Script executed:
# Check the actual TypeScript configuration and any type assertions
head -20 tsconfig.json 2>/dev/null || echo "No tsconfig.json in root"Repository: afuhflynn/code-reverb
Length of output: 506
🏁 Script executed:
# Look for any error handling or guards around the reviewPullRequest call
sed -n '100,135p' app/api/webhooks/github/route.tsRepository: afuhflynn/code-reverb
Length of output: 1139
🏁 Script executed:
# Check if reviewPullRequest has error handling for undefined values
sed -n '1,80p' lib/ai/actions/index.tsRepository: afuhflynn/code-reverb
Length of output: 2026
🏁 Script executed:
# Verify GitHub webhook documentation specifics by checking if there are any comments or guards in the code
grep -A 10 "pull_request" app/api/webhooks/github/route.ts | head -30Repository: afuhflynn/code-reverb
Length of output: 943
🏁 Script executed:
# Check if TypeScript compilation has errors with strict mode
# Look for any type assertions or non-null assertions in the code
grep -n "!" app/api/webhooks/github/route.ts | head -20Repository: afuhflynn/code-reverb
Length of output: 190
🏁 Script executed:
# Check if there's a build/compilation output or error log
find . -name "*.log" -o -name ".next" -type d 2>/dev/null | head -10Repository: afuhflynn/code-reverb
Length of output: 47
🏁 Script executed:
# Look for any type casting or validation before the function call
sed -n '95,125p' app/api/webhooks/github/route.tsRepository: afuhflynn/code-reverb
Length of output: 994
Fix TypeScript type safety: parameters passed to reviewPullRequest may be undefined.
The function signature in lib/ai/actions/index.ts (lines 7-16) requires non-optional parameters (installationId: number, baseSha: string, etc.), but the webhook call passes potentially undefined values via optional chaining. While GitHub webhook payloads guarantee these fields exist for opened and synchronized pull_request events, TypeScript's strict mode will reject this mismatch.
Either add non-null assertions (!), validate before the call, or update the function signature to accept optional parameters with appropriate fallbacks.
🤖 Prompt for AI Agents
In app/api/webhooks/github/route.ts around lines 110 to 120, the call to
reviewPullRequest passes values that may be undefined (via optional chaining)
while the function signature in lib/ai/actions/index.ts requires non-optional
parameters; fix by either (A) asserting non-null for the guaranteed fields at
call site (use ! on installation id, base.sha, head.sha,
changed_files/additions/deletions) or (B) add runtime validation before calling
reviewPullRequest and return or throw a clear error if any required field is
missing, or (C) change the reviewPullRequest signature to accept optional
parameters and handle defaults inside the function—apply one consistent approach
and update types accordingly.
| export async function reviewPullRequest( | ||
| owner: string, | ||
| repo: string, | ||
| prNumber: number | ||
| prNumber: number, | ||
| installationId: number, | ||
| baseSha: string, | ||
| headSha: string, | ||
| changedFiles: number, | ||
| additions: number, | ||
| deletions: number | ||
| ) { |
There was a problem hiding this comment.
Type safety issue: function signature doesn't match actual usage.
The function signature declares installationId, baseSha, headSha, changedFiles, additions, and deletions as required parameters with non-nullable types. However, the caller in app/api/webhooks/github/route.ts (lines 114-119) passes potentially undefined values using optional chaining (body?.installation?.id, pull_request?.base?.sha, etc.). Additionally, the null coalescing operators used in the event payload (lines 109, 112-114) suggest these values can indeed be null or undefined.
🔎 Proposed fix: Make parameters optional
export async function reviewPullRequest(
owner: string,
repo: string,
prNumber: number,
- installationId: number,
- baseSha: string,
- headSha: string,
- changedFiles: number,
- additions: number,
- deletions: number
+ installationId?: number,
+ baseSha?: string,
+ headSha?: string,
+ changedFiles?: number,
+ additions?: number,
+ deletions?: number
) {| await inngest.send({ | ||
| name: "pr.summary.requested", | ||
| id: `review-${repository.id}-${prNumber}`, | ||
| data: { | ||
| owner, | ||
| repo, | ||
| prNumber, | ||
| title: title ?? "", | ||
| description: description ?? "", | ||
| accountId: githubAccount.accountId, | ||
| installationId: installationId ?? null, | ||
| baseSha, | ||
| headSha, | ||
| changedFiles: changedFiles ?? 0, | ||
| additions: additions ?? 0, | ||
| deletions: deletions ?? 0, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Inngest event ID deduplication idempotency behavior
💡 Result:
Summary — Inngest event-ID deduplication / idempotency
-
Producer-level dedupe: if you send an event with an explicit id, Inngest uses that id as an idempotency key and will ignore any subsequently received events with the same id for 24 hours (duplicates are stored in the event history but will not trigger functions). The id is global across event types, so make it unique to the event type + resource. [1]
-
Consumer-level idempotency: you can set a function-level idempotency CEL expression (idempotency key derived from event data). That key also prevents duplicate function runs for 24 hours (one unique expression → one execution per 24h). Useful when you can’t control producer ids or when different functions should dedupe differently. [2]
-
Important caveats: event-idempotency is ignored for debouncing, event batching, and while a function is paused (replays after unpausing may “skip” idempotency). Choose ids/expressions that are specific enough to avoid collisions across event types/functions. [2]
Sources: Inngest docs — Sending events / Deduplication; Handling idempotency (function-level). [1][2]
🏁 Script executed:
# Search for pr.review.requested event and its ID in the codebase
rg "pr\.review\.requested" -A 5 -B 5Repository: afuhflynn/code-reverb
Length of output: 1165
Event ID collision: both events use the same ID causing deduplication.
Both the pr.summary.requested event (line 101) and the pr.review.requested event use identical IDs: review-${repository.id}-${prNumber}. Inngest treats event IDs as global idempotency keys across all event types and will silently drop any duplicate events with the same ID for 24 hours. This means one of these events will never be processed.
Use distinct event IDs for each event type:
await inngest.send({
name: "pr.summary.requested",
- id: `review-${repository.id}-${prNumber}`,
+ id: `summary-${repository.id}-${prNumber}`,
data: {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await inngest.send({ | |
| name: "pr.summary.requested", | |
| id: `review-${repository.id}-${prNumber}`, | |
| data: { | |
| owner, | |
| repo, | |
| prNumber, | |
| title: title ?? "", | |
| description: description ?? "", | |
| accountId: githubAccount.accountId, | |
| installationId: installationId ?? null, | |
| baseSha, | |
| headSha, | |
| changedFiles: changedFiles ?? 0, | |
| additions: additions ?? 0, | |
| deletions: deletions ?? 0, | |
| }, | |
| }); | |
| await inngest.send({ | |
| name: "pr.summary.requested", | |
| id: `summary-${repository.id}-${prNumber}`, | |
| data: { | |
| owner, | |
| repo, | |
| prNumber, | |
| title: title ?? "", | |
| description: description ?? "", | |
| accountId: githubAccount.accountId, | |
| installationId: installationId ?? null, | |
| baseSha, | |
| headSha, | |
| changedFiles: changedFiles ?? 0, | |
| additions: additions ?? 0, | |
| deletions: deletions ?? 0, | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In lib/ai/actions/index.ts around lines 99 to 116, the event ID uses a generic
key `review-${repository.id}-${prNumber}` which collides with another event and
causes Inngest to dedupe one of them; change the sent event ID to include the
event type (for example append or prepend a unique token like `summary` or the
event name) so each event has a distinct idempotency key (e.g.,
`pr-summary-${repository.id}-${prNumber}`) and ensure the other event uses a
different type-specific ID as well.
1 participant
…ry in the webhook
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.