diff --git a/.devcontainer/docker-compose.yml b/.devcontainer/docker-compose.yml index 96a9e7ff1..a82b5a2a6 100644 --- a/.devcontainer/docker-compose.yml +++ b/.devcontainer/docker-compose.yml @@ -27,6 +27,7 @@ services: REVIEW_AGENT_LOGGING_ENABLED: "true" REVIEW_AGENT_AUTO_REVIEW_ENABLED: "false" REVIEW_AGENT_REVIEW_COMMAND: review + REVIEW_AGENT_SUMMARY_ENABLED: "false" depends_on: - postgres - redis diff --git a/.env.development b/.env.development index 39b25125c..7db3da7ed 100644 --- a/.env.development +++ b/.env.development @@ -49,6 +49,7 @@ REDIS_URL="redis://localhost:6379" REVIEW_AGENT_LOGGING_ENABLED=true REVIEW_AGENT_AUTO_REVIEW_ENABLED=false REVIEW_AGENT_REVIEW_COMMAND=review +REVIEW_AGENT_SUMMARY_ENABLED=false # Misc diff --git a/CHANGELOG.md b/CHANGELOG.md index 44c52d5e5..3729ce6ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- [Experimental] Added support for generating a summary comment to the AI Code review agent. [#1175](https://github.com/sourcebot-dev/sourcebot/pull/1175) + ### Fixed - Add missing schema changes introduced in [#1170](https://github.com/sourcebot-dev/sourcebot/pull/1170). [#1176](https://github.com/sourcebot-dev/sourcebot/pull/1176) - Fixed blame gutter commit navigation to use the file path as it existed at the attributing commit, so clicking a blame line whose commit predates a rename resolves to the correct historical path. [#1178](https://github.com/sourcebot-dev/sourcebot/pull/1178) diff --git a/docs/docs/configuration/environment-variables.mdx b/docs/docs/configuration/environment-variables.mdx index 875a32dbe..464a06e6c 100644 --- a/docs/docs/configuration/environment-variables.mdx +++ b/docs/docs/configuration/environment-variables.mdx @@ -74,6 +74,8 @@ The following environment variables allow you to configure your Sourcebot deploy | `REVIEW_AGENT_AUTO_REVIEW_ENABLED` | `false` |

Enables/disables automatic code reviews by the review agent.

| | `REVIEW_AGENT_LOGGING_ENABLED` | `true` |

Enables/disables logging for the review agent. Logs are saved in `DATA_CACHE_DIR/review-agent`

| | `REVIEW_AGENT_REVIEW_COMMAND` | `review` |

The command used to trigger a code review by the review agent.

| +| `REVIEW_AGENT_SUMMARY_ENABLED` | `false` |

Enables/disables posting a concise summary comment on the PR/MR after each review.

| +| `REVIEW_AGENT_SUMMARY_MAX_LENGTH` | `250` |

Maximum character length of the summary comment posted by the review agent.

| ### Overriding environment variables from the config diff --git a/docs/docs/features/agents/review-agent.mdx b/docs/docs/features/agents/review-agent.mdx index b4dc9c2f6..dc775d462 100644 --- a/docs/docs/features/agents/review-agent.mdx +++ b/docs/docs/features/agents/review-agent.mdx @@ -145,3 +145,5 @@ You can also trigger a review manually by commenting `/review` on any PR or MR. | `REVIEW_AGENT_REVIEW_COMMAND` | `review` | Comment command that triggers a manual review (without the `/`) | | `REVIEW_AGENT_MODEL` | first configured model | `displayName` of the language model to use for reviews | | `REVIEW_AGENT_LOGGING_ENABLED` | unset | Write prompt and response logs to disk for debugging | +| `REVIEW_AGENT_SUMMARY_ENABLED` | `false` | Post a concise summary comment on the PR/MR after each review | +| `REVIEW_AGENT_SUMMARY_MAX_LENGTH` | `250` | Maximum character length of the summary comment | diff --git a/packages/shared/src/env.server.ts b/packages/shared/src/env.server.ts index 460afc29b..29a7a9d2c 100644 --- a/packages/shared/src/env.server.ts +++ b/packages/shared/src/env.server.ts @@ -240,6 +240,8 @@ const options = { REVIEW_AGENT_LOGGING_ENABLED: booleanSchema.default('true'), REVIEW_AGENT_AUTO_REVIEW_ENABLED: booleanSchema.default('false'), REVIEW_AGENT_REVIEW_COMMAND: z.string().default('review'), + REVIEW_AGENT_SUMMARY_ENABLED: booleanSchema.default('false'), + REVIEW_AGENT_SUMMARY_MAX_LENGTH: numberSchema.default(250), ANTHROPIC_API_KEY: z.string().optional(), ANTHROPIC_AUTH_TOKEN: z.string().optional(), diff --git a/packages/web/src/features/agents/review-agent/app.ts b/packages/web/src/features/agents/review-agent/app.ts index 043a80fad..1279fb7b3 100644 --- a/packages/web/src/features/agents/review-agent/app.ts +++ b/packages/web/src/features/agents/review-agent/app.ts @@ -1,12 +1,13 @@ import { Octokit } from "octokit"; import { Gitlab } from "@gitbeaker/rest"; import { generatePrReviews } from "@/features/agents/review-agent/nodes/generatePrReview"; +import { generatePrSummary } from "@/features/agents/review-agent/nodes/generatePrSummary"; import { githubPushPrReviews } from "@/features/agents/review-agent/nodes/githubPushPrReviews"; import { githubPrParser } from "@/features/agents/review-agent/nodes/githubPrParser"; import { getReviewAgentLogDir } from "@/features/agents/review-agent/nodes/invokeDiffReviewLlm"; import { gitlabMrParser } from "@/features/agents/review-agent/nodes/gitlabMrParser"; import { gitlabPushMrReviews } from "@/features/agents/review-agent/nodes/gitlabPushMrReviews"; -import { GitHubPullRequest, GitLabMergeRequestPayload } from "@/features/agents/review-agent/types"; +import { GitHubPullRequest, GitLabMergeRequestPayload, sourcebot_pr_payload } from "@/features/agents/review-agent/types"; import { env } from "@sourcebot/shared"; import path from "path"; import fs from "fs"; @@ -24,6 +25,18 @@ const rules = [ const logger = createLogger('review-agent'); +async function generateSummarySafely(payload: sourcebot_pr_payload, label: string): Promise { + if (!env.REVIEW_AGENT_SUMMARY_ENABLED) { + return undefined; + } + try { + return await generatePrSummary(payload); + } catch (error) { + logger.error(`Error generating ${label} summary: ${error}`); + return undefined; + } +} + function getReviewAgentLogPath(identifier: string): string | undefined { if (!env.REVIEW_AGENT_LOGGING_ENABLED) { return undefined; @@ -55,7 +68,9 @@ export async function processGitHubPullRequest(octokit: Octokit, pullRequest: Gi const prPayload = await githubPrParser(octokit, pullRequest); const fileDiffReviews = await generatePrReviews(reviewAgentLogPath, prPayload, rules); - await githubPushPrReviews(octokit, prPayload, fileDiffReviews); + + const summary = await generateSummarySafely(prPayload, "PR"); + await githubPushPrReviews(octokit, prPayload, fileDiffReviews, summary); } export async function processGitLabMergeRequest( @@ -70,5 +85,7 @@ export async function processGitLabMergeRequest( const prPayload = await gitlabMrParser(gitlabClient, mrPayload, hostDomain); const fileDiffReviews = await generatePrReviews(reviewAgentLogPath, prPayload, rules); - await gitlabPushMrReviews(gitlabClient, projectId, prPayload, fileDiffReviews); + + const summary = await generateSummarySafely(prPayload, "MR"); + await gitlabPushMrReviews(gitlabClient, projectId, prPayload, fileDiffReviews, summary); } diff --git a/packages/web/src/features/agents/review-agent/nodes/generatePrSummary.ts b/packages/web/src/features/agents/review-agent/nodes/generatePrSummary.ts new file mode 100644 index 000000000..21ba69ff5 --- /dev/null +++ b/packages/web/src/features/agents/review-agent/nodes/generatePrSummary.ts @@ -0,0 +1,51 @@ +import { sourcebot_pr_payload } from "@/features/agents/review-agent/types"; +import { getAISDKLanguageModelAndOptions, getConfiguredLanguageModels } from "@/features/chat/utils.server"; +import { env } from "@sourcebot/shared"; +import { generateText } from "ai"; +import { createLogger } from "@sourcebot/shared"; + +const logger = createLogger('generate-pr-summary'); + +export const generatePrSummary = async (prPayload: sourcebot_pr_payload): Promise => { + const maxSummaryLength = env.REVIEW_AGENT_SUMMARY_MAX_LENGTH; + logger.debug("Executing generate_pr_summary"); + + const models = await getConfiguredLanguageModels(); + if (models.length === 0) { + throw new Error("No language models are configured"); + } + + let selectedModel = models[0]; + if (env.REVIEW_AGENT_MODEL) { + const match = models.find((m) => m.displayName === env.REVIEW_AGENT_MODEL); + if (match) { + selectedModel = match; + } else { + logger.warn(`REVIEW_AGENT_MODEL="${env.REVIEW_AGENT_MODEL}" did not match any configured model displayName. Falling back to the first configured model.`); + } + } + + const { model, providerOptions, temperature } = await getAISDKLanguageModelAndOptions(selectedModel); + + const filesChanged = prPayload.file_diffs.map(f => f.to).join(", "); + + const prompt = `Summarize the following pull request changes in ${maxSummaryLength} characters or fewer. Be concise and focus on what changed and why. You may use inline markdown (e.g. \`code\`, **bold**) but avoid headers, bullet lists, and block-level formatting. + +PR Title: ${prPayload.title} +PR Description: ${prPayload.description} +Files changed: ${filesChanged} +`; + + const result = await generateText({ + model, + system: `You are a code review assistant. Generate a concise markdown-compatible summary of pull request changes. The summary must be ${maxSummaryLength} characters or fewer. Avoid headers, bullet lists, and block-level formatting.`, + prompt, + providerOptions, + temperature, + }); + + const summary = result.text.trim().slice(0, maxSummaryLength); + + logger.debug("Completed generate_pr_summary"); + return summary; +}; diff --git a/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.test.ts b/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.test.ts index 9e80d31a9..9e6011def 100644 --- a/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.test.ts +++ b/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.test.ts @@ -29,7 +29,7 @@ const SINGLE_REVIEW: sourcebot_file_diff_review[] = [ }, ]; -function makeMockOctokit(createReviewCommentResult: 'resolve' | 'reject' = 'resolve') { +function makeMockOctokit(createReviewCommentResult: 'resolve' | 'reject' = 'resolve', existingComments: { id: number; body: string }[] = []) { return { rest: { pulls: { @@ -37,6 +37,11 @@ function makeMockOctokit(createReviewCommentResult: 'resolve' | 'reject' = 'reso ? vi.fn().mockResolvedValue({}) : vi.fn().mockRejectedValue(new Error('Unprocessable Entity')), }, + issues: { + listComments: vi.fn().mockResolvedValue({ data: existingComments }), + createComment: vi.fn().mockResolvedValue({}), + updateComment: vi.fn().mockResolvedValue({}), + }, }, } as any; } @@ -146,3 +151,62 @@ describe('githubPushPrReviews', () => { expect(octokit.rest.pulls.createReviewComment).not.toHaveBeenCalled(); }); }); + +describe('githubPushPrReviews – summary comment', () => { + const SUMMARY_MARKER = ''; + + test('does not call issues API when summary is undefined', async () => { + const octokit = makeMockOctokit(); + + await githubPushPrReviews(octokit, MOCK_PAYLOAD, [], undefined); + + expect(octokit.rest.issues.listComments).not.toHaveBeenCalled(); + expect(octokit.rest.issues.createComment).not.toHaveBeenCalled(); + expect(octokit.rest.issues.updateComment).not.toHaveBeenCalled(); + }); + + test('creates a new comment including the marker when no existing comment found', async () => { + const octokit = makeMockOctokit('resolve', []); + + await githubPushPrReviews(octokit, MOCK_PAYLOAD, [], 'Summary text'); + + expect(octokit.rest.issues.listComments).toHaveBeenCalledWith({ + owner: 'my-org', + repo: 'my-repo', + issue_number: 7, + }); + expect(octokit.rest.issues.createComment).toHaveBeenCalledOnce(); + const body = octokit.rest.issues.createComment.mock.calls[0][0].body as string; + expect(body).toContain(SUMMARY_MARKER); + expect(body).toContain('Summary text'); + expect(body).toContain('Created:'); + expect(body).not.toContain('Updated:'); + expect(octokit.rest.issues.updateComment).not.toHaveBeenCalled(); + }); + + test('updates the existing comment when the marker is already present', async () => { + const existingComments = [{ id: 99, body: `${SUMMARY_MARKER}\nOld summary` }]; + const octokit = makeMockOctokit('resolve', existingComments); + + await githubPushPrReviews(octokit, MOCK_PAYLOAD, [], 'New summary'); + + expect(octokit.rest.issues.updateComment).toHaveBeenCalledOnce(); + expect(octokit.rest.issues.updateComment).toHaveBeenCalledWith( + expect.objectContaining({ comment_id: 99 }), + ); + const body = octokit.rest.issues.updateComment.mock.calls[0][0].body as string; + expect(body).toContain('New summary'); + expect(body).toContain('Updated:'); + expect(body).not.toContain('Created:'); + expect(octokit.rest.issues.createComment).not.toHaveBeenCalled(); + }); + + test('does not throw when listComments fails', async () => { + const octokit = makeMockOctokit(); + octokit.rest.issues.listComments = vi.fn().mockRejectedValue(new Error('403 Forbidden')); + + await expect( + githubPushPrReviews(octokit, MOCK_PAYLOAD, [], 'Summary text'), + ).resolves.not.toThrow(); + }); +}); diff --git a/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.ts b/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.ts index a6ef88d2b..a83d58bca 100644 --- a/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.ts +++ b/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.ts @@ -4,9 +4,40 @@ import { createLogger } from "@sourcebot/shared"; const logger = createLogger('github-push-pr-reviews'); -export const githubPushPrReviews = async (octokit: Octokit, pr_payload: sourcebot_pr_payload, file_diff_reviews: sourcebot_file_diff_review[]) => { +export const githubPushPrReviews = async (octokit: Octokit, pr_payload: sourcebot_pr_payload, file_diff_reviews: sourcebot_file_diff_review[], summary?: string) => { logger.info("Executing github_push_pr_reviews"); + if (summary) { + const SUMMARY_MARKER = ""; + try { + const { data: comments } = await octokit.rest.issues.listComments({ + owner: pr_payload.owner, + repo: pr_payload.repo, + issue_number: pr_payload.number, + }); + const existing = comments.find(c => c.body?.includes(SUMMARY_MARKER)); + const action = existing ? "Updated" : "Created"; + const body = `${SUMMARY_MARKER}\n${summary}\n\n---\n*${action}: ${new Date().toUTCString()}*`; + if (existing) { + await octokit.rest.issues.updateComment({ + owner: pr_payload.owner, + repo: pr_payload.repo, + comment_id: existing.id, + body, + }); + } else { + await octokit.rest.issues.createComment({ + owner: pr_payload.owner, + repo: pr_payload.repo, + issue_number: pr_payload.number, + body, + }); + } + } catch (error) { + logger.error(`Error posting PR summary comment: ${error}`); + } + } + try { for (const file_diff_review of file_diff_reviews) { for (const review of file_diff_review.reviews) { diff --git a/packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.test.ts b/packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.test.ts index b26ecc53b..dff3a74fa 100644 --- a/packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.test.ts +++ b/packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.test.ts @@ -34,7 +34,7 @@ const SINGLE_REVIEW: sourcebot_file_diff_review[] = [ }, ]; -function makeMockClient(discussionResult: 'resolve' | 'reject' = 'resolve') { +function makeMockClient(discussionResult: 'resolve' | 'reject' = 'resolve', existingNotes: { id: number; body: string }[] = []) { return { MergeRequestDiscussions: { create: discussionResult === 'resolve' @@ -42,7 +42,9 @@ function makeMockClient(discussionResult: 'resolve' | 'reject' = 'resolve') { : vi.fn().mockRejectedValue(new Error('400 Bad Request')), }, MergeRequestNotes: { + all: vi.fn().mockResolvedValue(existingNotes), create: vi.fn().mockResolvedValue({}), + edit: vi.fn().mockResolvedValue({}), }, } as any; } @@ -177,7 +179,7 @@ describe('gitlabPushMrReviews', () => { test('does not throw when both discussion and note creation fail', async () => { const client = { MergeRequestDiscussions: { create: vi.fn().mockRejectedValue(new Error('500')) }, - MergeRequestNotes: { create: vi.fn().mockRejectedValue(new Error('500')) }, + MergeRequestNotes: { all: vi.fn().mockResolvedValue([]), create: vi.fn().mockRejectedValue(new Error('500')), edit: vi.fn() }, } as any; await expect( @@ -185,3 +187,55 @@ describe('gitlabPushMrReviews', () => { ).resolves.not.toThrow(); }); }); + +describe('gitlabPushMrReviews – summary note', () => { + const SUMMARY_MARKER = ''; + + test('does not call MergeRequestNotes API when summary is undefined', async () => { + const client = makeMockClient(); + + await gitlabPushMrReviews(client, 101, MOCK_PAYLOAD, [], undefined); + + expect(client.MergeRequestNotes.all).not.toHaveBeenCalled(); + expect(client.MergeRequestNotes.create).not.toHaveBeenCalled(); + expect(client.MergeRequestNotes.edit).not.toHaveBeenCalled(); + }); + + test('creates a new note including the marker when no existing note found', async () => { + const client = makeMockClient('resolve', []); + + await gitlabPushMrReviews(client, 101, MOCK_PAYLOAD, [], 'Summary text'); + + expect(client.MergeRequestNotes.all).toHaveBeenCalledWith(101, 42); + expect(client.MergeRequestNotes.create).toHaveBeenCalledOnce(); + const body = client.MergeRequestNotes.create.mock.calls[0][2] as string; + expect(body).toContain(SUMMARY_MARKER); + expect(body).toContain('Summary text'); + expect(body).toContain('Created:'); + expect(body).not.toContain('Updated:'); + expect(client.MergeRequestNotes.edit).not.toHaveBeenCalled(); + }); + + test('updates the existing note when the marker is already present', async () => { + const existingNotes = [{ id: 55, body: `${SUMMARY_MARKER}\nOld summary` }]; + const client = makeMockClient('resolve', existingNotes); + + await gitlabPushMrReviews(client, 101, MOCK_PAYLOAD, [], 'New summary'); + + expect(client.MergeRequestNotes.edit).toHaveBeenCalledOnce(); + const editOptions = client.MergeRequestNotes.edit.mock.calls[0][3] as { body: string }; + expect(editOptions.body).toContain('New summary'); + expect(editOptions.body).toContain('Updated:'); + expect(editOptions.body).not.toContain('Created:'); + expect(client.MergeRequestNotes.create).not.toHaveBeenCalled(); + }); + + test('does not throw when MergeRequestNotes.all fails', async () => { + const client = makeMockClient(); + client.MergeRequestNotes.all = vi.fn().mockRejectedValue(new Error('403 Forbidden')); + + await expect( + gitlabPushMrReviews(client, 101, MOCK_PAYLOAD, [], 'Summary text'), + ).resolves.not.toThrow(); + }); +}); diff --git a/packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.ts b/packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.ts index ff98ecb3c..360bb8f4d 100644 --- a/packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.ts +++ b/packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.ts @@ -9,9 +9,36 @@ export const gitlabPushMrReviews = async ( projectId: number, prPayload: sourcebot_pr_payload, fileDiffReviews: sourcebot_file_diff_review[], + summary?: string, ): Promise => { logger.info("Executing gitlab_push_mr_reviews"); + if (summary) { + const SUMMARY_MARKER = ""; + try { + const notes = await gitlabClient.MergeRequestNotes.all(projectId, prPayload.number); + const existing = notes.find(n => n.body?.includes(SUMMARY_MARKER)); + const action = existing ? "Updated" : "Created"; + const body = `${SUMMARY_MARKER}\n${summary}\n\n---\n*${action}: ${new Date().toUTCString()}*`; + if (existing) { + await gitlabClient.MergeRequestNotes.edit( + projectId, + prPayload.number, + existing.id, + { body }, + ); + } else { + await gitlabClient.MergeRequestNotes.create( + projectId, + prPayload.number, + body, + ); + } + } catch (error) { + logger.error(`Error posting MR summary note: ${error}`); + } + } + if (!prPayload.diff_refs) { logger.error("diff_refs is missing from pr_payload, cannot post inline GitLab MR reviews"); return;