feat: enhance AI review process and configuration options: - #300
Conversation
- Updated .env.example to clarify embedding provider settings and added debounce configuration for PR review events. - Modified the AI review trigger route to support a blocking wait option for manual reviews. - Implemented stale pending review detection in the dashboard to improve user experience. - Enhanced the review generation process with timing measurements for better performance tracking. - Refactored embedding logic to ensure proper provider configuration and error handling. - Removed auto-queueing of unreviewed PRs to streamline review management.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (11)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🤖 Supercode AI ReviewSummaryThis PR refines the AI review workflow end-to-end: manual review requests can now “wait” for completion, background review runs debounce rapid GitHub synchronize events with a configurable period, and the dashboard treats long-stale Walkthrough
Changes table
Findings
Risk assessmentMedium — This touches core workflow control flow (blocking waits), removes auto-queueing (behavioral change), and changes embedding provider selection. Blast radius is mainly in review generation + dashboard UX + embeddings/RAG availability. Test plan
Suggested PR descriptionWhat
Why
How tested
Automated review by Supercode · leave a 👍/👎 reaction to rate this review |
![tembo[bot]](https://avatars.githubusercontent.com/in/1223475?s=60&v=4){kind=small}
| const result = await reviewPullRequest(owner, repo, prNumber, { | ||
| userId: session.user.id, | ||
| source: "api_trigger", | ||
| wait: true, |
There was a problem hiding this comment.
This now blocks the HTTP request until the full LLM pipeline finishes. Unlike the Inngest route, this route doesn't export maxDuration, so on Vercel the request can hit the function timeout and return a 500/504 even though the review eventually completes (the caller just never sees the result). Consider raising maxDuration here or making the blocking behavior opt-in via a query param.
| export function embeddingModel( | ||
| modelId: string, | ||
| provider: GatewayProviderName = activeGatewayProvider, | ||
| provider: "vercel" | "merge" | "openai" = embeddingProvider() ?? "openai", |
There was a problem hiding this comment.
The ?? "openai" fallback bypasses the key checks in embeddingProvider(), so any caller that omits provider will attempt direct OpenAI even with no key configured — exactly what the new guard is meant to prevent. The only caller passes it explicitly now, so the default can just go away:
| provider: "vercel" | "merge" | "openai" = embeddingProvider() ?? "openai", | |
| provider: "vercel" | "merge" | "openai", |
| } | ||
| } catch (error) { | ||
| const message = error instanceof Error ? error.message : "Unknown Error" | ||
| await markReviewFailed(payload, message) |
There was a problem hiding this comment.
If markReviewFailed itself throws (e.g. DB hiccup), it replaces the original review error and you lose the real failure reason. Keeping it best-effort preserves the root cause:
| await markReviewFailed(payload, message) | |
| await markReviewFailed(payload, message).catch(() => {}) |
| const result = await reviewPullRequest(owner, repo, prNumber, { | ||
| userId: session.user.id, | ||
| source: "dashboard", | ||
| wait: true, |
There was a problem hiding this comment.
Blocking the server action works, but if the platform kills it at the execution limit the client gets an error and the row sits pending until the 10-minute stale threshold flips it back — that's a long spinner for the user. Might be worth surfacing stale state sooner than 10 min, or letting the stale threshold account for reviews that are genuinely long-running.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const result = await reviewPullRequest(owner, repo, prNumber, { | ||
| userId: session.user.id, | ||
| source: "dashboard", | ||
| wait: true, |
There was a problem hiding this comment.
This makes the dashboard action wait for the full GitHub, Pinecone, model-generation, persistence, commenting, and notification pipeline. The same blocking behavior also occurs in apps/web/app/api/reviews/trigger/route.ts:52. If a review exceeds the hosting platform's request limit, the process can be terminated before markReviewFailed runs. The client then receives an error while the review remains pending. Keep this work in the background worker, or give every blocking entry point enough execution time and durable timeout recovery.
| !isGenerating && | ||
| (review.status === "unreviewed" || | ||
| review.status === "failed" || | ||
| stalePending || |
There was a problem hiding this comment.
A pending review older than ten minutes may still be running because there is no heartbeat. This condition enables another review without checking for an active run, and direct execution does not share Inngest's per-PR concurrency control. Both runs can then update the same database row and sticky GitHub comment, allowing an older run that finishes last to overwrite newer output. Recovery should verify that no run is active or reject results from obsolete runs or head SHAs.
| } | ||
| } catch (error) { | ||
| const message = error instanceof Error ? error.message : "Unknown Error" | ||
| await markReviewFailed(payload, message) | ||
| throw error |
There was a problem hiding this comment.
This catch awaits markReviewFailed before rethrowing the original generation error. If that database write fails, its error replaces the actual review failure. If it succeeds, the rethrown error reaches the existing outer catch, which records the same failure again. Preserve the original exception and use one guarded failure-recording path.
| // browser's current origin so local, preview, and production deployments do | ||
| // not depend on a separately configured public URL. | ||
| export const { signIn, signUp, useSession, signOut } = createAuthClient(); |
There was a problem hiding this comment.
The client no longer reads NEXT_PUBLIC_BETTER_AUTH_URL, but AGENTS.md:111 still lists that variable as required. Update the documented environment contract so developers are not instructed to configure an ineffective variable.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
- Added functionality to extract and update PR description summaries during review generation. - Implemented concurrent processing for posting review comments and updating PR summaries to improve efficiency. - Introduced new constants and helper functions for managing PR summary formatting and merging.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| const result = await reviewPullRequest(owner, repo, prNumber, { | ||
| userId: session.user.id, | ||
| source: "api_trigger", | ||
| wait: true, |
There was a problem hiding this comment.
The blocking /api/reviews/trigger route lacks export const maxDuration, so long reviews hit the Vercel default function timeout and return 500/504.
1 participant
Description
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
bun testpassesbun run typecheckpassesbun run lintpasses (if applicable)Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Performance