Skip to content

feat: enhance AI review process and configuration options: #300

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yashdev9274 merged 2 commits into from
Sep 17, 2026
Merged

feat: enhance AI review process and configuration options: #300

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9d66735
feat: enhance AI review process and configuration options:
yashdev9274 Sep 17, 2026
57f2a46
feat: enhance PR review process with summary updates:
yashdev9274 Sep 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions apps/web/.env.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ MERGE_GATEWAY_API_KEY=
# OPENAI_API_KEY=
# ANTHROPIC_API_KEY=
# GOOGLE_GENERATIVE_AI_API_KEY=
# Embeddings default to direct OpenAI when OPENAI_API_KEY is set. Vercel's
# free tier rejects this model; opt into a paid gateway explicitly if available.
# EMBEDDING_PROVIDER=openai # openai | vercel | merge
PINECONE_DB_API_KEY=

# ─── Inngest (background AI reviews) ───
Expand Down Expand Up @@ -96,6 +99,8 @@ PINECONE_DB_API_KEY=
# INNGEST_DEV=1
# INNGEST_EVENT_KEY=
# INNGEST_SIGNING_KEY=
# Delay used to collapse rapid PR synchronize events (1–30 seconds; default 5).
# PR_REVIEW_DEBOUNCE_SECONDS=5

# ─── Composio (Slack + Linear integrations) ───
# OAuth for Slack/Linear is hosted by Composio. Tokens never hit our DB.
Expand Down
3 changes: 2 additions & 1 deletion apps/web/app/api/reviews/trigger/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import prisma from "@super/db"
/**
* POST /api/reviews/trigger
* Body: { owner, repo, prNumber }
* Manually queue an AI review for a connected repo (useful when webhooks can't reach localhost).
* Manually run an AI review for a connected repo (useful when webhooks can't reach localhost).
*/
export async function POST(req: NextRequest) {
try {
Expand Down Expand Up @@ -49,6 +49,7 @@ export async function POST(req: NextRequest) {
const result = await reviewPullRequest(owner, repo, prNumber, {
userId: session.user.id,
source: "api_trigger",
wait: true,

@tembo tembo Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@vercel vercel Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The blocking /api/reviews/trigger route lacks export const maxDuration, so long reviews hit the Vercel default function timeout and return 500/504.

Fix on Vercel

})
return NextResponse.json(result)
} catch (error) {
Expand Down
29 changes: 19 additions & 10 deletions apps/web/app/dashboard/pull-requests/[id]/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,16 @@ import {
} from "@/modules/pull-requests/components/pr-workspace"
import { toast } from "sonner"

const STALE_PENDING_MS = 10 * 60 * 1000

function isStalePending(review: {
status: string
updatedAt?: Date | string
} | null | undefined) {
if (review?.status !== "pending" || !review.updatedAt) return false
return Date.now() - new Date(review.updatedAt).getTime() >= STALE_PENDING_MS
}

function hasCompletedReview(review: {
status: string
review?: string
Expand Down Expand Up @@ -73,15 +83,12 @@ export default function ReviewDetailPage(props: {

const queueMutation = useMutation({
mutationFn: () => queueReview(id),
onSuccess: (result) => {
toast.success(result.message || "AI review queued")
queryClient.setQueryData(["review", id], (prev: unknown) =>
prev && typeof prev === "object"
? { ...(prev as object), status: "pending" }
: prev,
)
queryClient.invalidateQueries({ queryKey: ["review", id] })
queryClient.invalidateQueries({ queryKey: ["reviews"] })
onSuccess: async (result) => {
toast.success(result.message || "AI review completed")
await Promise.all([
queryClient.invalidateQueries({ queryKey: ["review", id] }),
queryClient.invalidateQueries({ queryKey: ["reviews"] }),
])
},
onError: (error) => {
toast.error(
Expand All @@ -103,15 +110,17 @@ export default function ReviewDetailPage(props: {
// eslint-disable-next-line react-hooks/exhaustive-deps -- intentionally once when review first loads
}, [isLoading, review?.status, review?.prState, review?.review])

const stalePending = isStalePending(review)
const isGenerating =
queueMutation.isPending || review?.status === "pending"
queueMutation.isPending || (review?.status === "pending" && !stalePending)
const completed = hasCompletedReview(review)
const showGenerate =
!!review &&
!completed &&
!isGenerating &&
(review.status === "unreviewed" ||
review.status === "failed" ||
stalePending ||
Comment on lines 120 to +123

@greptile-apps greptile-apps Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Stale retries can overwrite reviews

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.

!review.review?.trim())

if (!isLoading && !review) {
Expand Down
13 changes: 11 additions & 2 deletions apps/web/inngest/functions/ai-review.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@ import {
runGeneratePrReview,
} from "@/modules/ai/lib/generate-pr-review"

function reviewDebouncePeriod(): `${number}s` {
const configured = Number.parseInt(process.env.PR_REVIEW_DEBOUNCE_SECONDS ?? "5", 10)
const seconds = Number.isFinite(configured)
? Math.min(Math.max(configured, 1), 30)
: 5
return `${seconds}s`
}

/**
* Background AI PR review worker.
* Triggered by `pr.review.requested` from GitHub webhooks, dashboard, or
Expand All @@ -20,10 +28,10 @@ export const generateReview = inngest.createFunction(
},
{ limit: 5 },
],
// Rapid synchronize events → one review on the latest head.
// Collapse synchronize bursts without imposing a fixed 30-second wait.
debounce: {
key: "event.data.owner + '/' + event.data.repo + '#' + event.data.prNumber",
period: "30s",
period: reviewDebouncePeriod(),
},
retries: 2,
onFailure: async ({ event, error }) => {
Expand Down Expand Up @@ -115,6 +123,7 @@ export const generateReview = inngest.createFunction(
prNumber: result.prNumber,
files: result.files,
commentPosted: result.commentPosted,
descriptionUpdated: result.descriptionUpdated ?? false,
linearNotified: result.linearNotified ?? false,
linearIssueId: result.linearIssueId ?? null,
linearSkippedReason: result.linearSkippedReason ?? null,
Expand Down
15 changes: 14 additions & 1 deletion apps/web/lib/gateway.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,22 @@ export function chatModel(
return googleDirect(bareModelId(modelId))
}

export function embeddingProvider(): "vercel" | "merge" | "openai" | null {
const configured = (process.env.EMBEDDING_PROVIDER ?? "").trim().toLowerCase()
if (configured === "vercel" && hasVercelKey()) return "vercel"
if (configured === "merge" && hasMergeKey()) return "merge"
if (configured === "openai" && hasOpenAIKey()) return "openai"

// Vercel's free tier currently rejects OpenAI embeddings. Prefer a direct
// key automatically; gateways must be explicitly enabled once paid access
// is available.
if (hasOpenAIKey()) return "openai"
return null
}

export function embeddingModel(
modelId: string,
provider: GatewayProviderName = activeGatewayProvider,
provider: "vercel" | "merge" | "openai" = embeddingProvider() ?? "openai",

@tembo tembo Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

Suggested change
provider: "vercel" | "merge" | "openai" = embeddingProvider() ?? "openai",
provider: "vercel" | "merge" | "openai",

): EmbeddingModel {
if (provider === "vercel") {
return vercelGateway.embeddingModel(modelId)
Expand Down
22 changes: 14 additions & 8 deletions apps/web/modules/ai/action/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,14 +158,20 @@ export async function reviewPullRequest(
payload.source === "manual"

if (options?.wait) {
await runGeneratePrReview(payload)
console.log(
`[reviewPullRequest] completed blocking in-process review for ${owner}/${repo}#${prNumber}`,
)
return {
success: true,
message: "Review completed",
mode: "blocking" as const,
try {
await runGeneratePrReview(payload)
console.log(
`[reviewPullRequest] completed blocking in-process review for ${owner}/${repo}#${prNumber}`,
)
return {
success: true,
message: "Review completed",
mode: "blocking" as const,
}
} catch (error) {
const message = error instanceof Error ? error.message : "Unknown Error"
await markReviewFailed(payload, message)

@tembo tembo Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

Suggested change
await markReviewFailed(payload, message)
await markReviewFailed(payload, message).catch(() => {})

throw error
Comment on lines +170 to +174

@greptile-apps greptile-apps Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Failure handling masks errors

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.

}
}

Expand Down
Loading
Loading