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 e9029f539..d0ca5ef2c 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 8ab886a30..40be407a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - [EE] Added prompt caching for Ask Sourcebot. For Anthropic models, the static prompt prefix (tool definitions, system prompt, and conversation history) is marked with a cache breakpoint so it is billed at the provider's discounted cache-read rate on subsequent agent steps and follow-up turns. Toggle with `SOURCEBOT_CHAT_PROMPT_CACHING_ENABLED` (default `true`). [#1278](https://github.com/sourcebot-dev/sourcebot/pull/1278) - [EE] Added a cached-token breakdown to the Ask Sourcebot message details, showing what share of the input tokens were served from the model provider's prompt cache. [#1278](https://github.com/sourcebot-dev/sourcebot/pull/1278) +- [Experimental] Added support for generating a summary comment to the AI Code review agent. [#1175](https://github.com/sourcebot-dev/sourcebot/pull/1175) ### Fixed - Upgraded `protobufjs` to `^7.6.2`. [#1281](https://github.com/sourcebot-dev/sourcebot/pull/1281) diff --git a/docs/docs/configuration/environment-variables.mdx b/docs/docs/configuration/environment-variables.mdx index 8c0415fc4..e3b6a52bc 100644 --- a/docs/docs/configuration/environment-variables.mdx +++ b/docs/docs/configuration/environment-variables.mdx @@ -72,6 +72,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 712ac9d41..e140c5a02 100644 --- a/packages/shared/src/env.server.ts +++ b/packages/shared/src/env.server.ts @@ -245,6 +245,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 210fffb44..db961a945 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 @@ -30,16 +30,23 @@ const SINGLE_REVIEW: sourcebot_file_diff_review[] = [ }, ]; -function makeMockOctokit(createReviewCommentResult: 'resolve' | 'reject' = 'resolve') { - return { +function makeMockOctokit(createReviewCommentResult: 'resolve' | 'reject' = 'resolve', existingComments: { id: number; body: string }[] = []) { + const octokit = { + paginate: vi.fn().mockResolvedValue(existingComments), rest: { pulls: { createReviewComment: createReviewCommentResult === 'resolve' ? vi.fn().mockResolvedValue({}) : vi.fn().mockRejectedValue(new Error('Unprocessable Entity')), }, + issues: { + listComments: vi.fn(), + createComment: vi.fn().mockResolvedValue({}), + updateComment: vi.fn().mockResolvedValue({}), + }, }, } as unknown as Octokit; + return octokit; } describe('githubPushPrReviews', () => { @@ -147,3 +154,61 @@ 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.paginate).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.paginate).toHaveBeenCalledWith( + octokit.rest.issues.listComments, + { 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 paginate fails', async () => { + const octokit = makeMockOctokit(); + octokit.paginate = 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..8e0c74762 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 comments = await octokit.paginate(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 98caad2d9..03606bc0e 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 @@ -37,7 +37,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' @@ -45,7 +45,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 unknown as GitlabClient; } @@ -180,7 +182,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 unknown as GitlabClient; await expect( @@ -188,3 +190,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;