diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml new file mode 100644 index 0000000..2c0a3ae --- /dev/null +++ b/.github/workflows/release.yml @@ -0,0 +1,100 @@ +name: Release + +on: + push: + tags: ["v*"] + +permissions: + contents: read + +concurrency: + group: release + cancel-in-progress: true + +jobs: + build-and-push: + runs-on: ubuntu-latest + permissions: + contents: read + packages: write + outputs: + digest: ${{ steps.build.outputs.digest }} + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + + - uses: docker/setup-buildx-action@8d2750c68a42422c14e847fe6c8ac0403b4cbd6f # v3 + + - uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 # v3 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + + - uses: docker/metadata-action@c299e40c65443455700f0fdfc63efafe5b349051 # v5 + id: meta + with: + images: ghcr.io/${{ github.repository }} + tags: | + type=semver,pattern={{version}} + type=raw,value=latest + + - uses: docker/build-push-action@10e90e3645eae34f1e60eeb005ba3a3d33f178e8 # v6 + id: build + with: + context: . + push: true + tags: ${{ steps.meta.outputs.tags }} + labels: ${{ steps.meta.outputs.labels }} + + release: + needs: build-and-push + runs-on: ubuntu-latest + permissions: + contents: write + steps: + - uses: actions/create-github-app-token@fee1f7d63c2ff003460e3d139729b119787bc349 # v2 + id: app-token + with: + app-id: ${{ secrets.RELEASE_APP_ID }} + private-key: ${{ secrets.RELEASE_PRIVATE_KEY }} + permission-contents: write + + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + token: ${{ steps.app-token.outputs.token }} + ref: ${{ github.ref }} + persist-credentials: true + + - name: Flip action.yml image to digest-pinned GHCR reference + env: + DIGEST: ${{ needs.build-and-push.outputs.digest }} + IMAGE: ghcr.io/${{ github.repository }} + run: | + sed -i "s|image: Dockerfile|image: docker://${IMAGE}@${DIGEST}|" action.yml + git config user.name "umm-actually[bot]" + git config user.email "umm-actually[bot]@users.noreply.github.com" + git add action.yml + git commit -m "chore(release): pin image to ${DIGEST}" + + - name: Force-move semver tag to the digest-flipped commit + env: + TAG: ${{ github.ref_name }} + run: | + git tag -f "$TAG" + git push origin "$TAG" --force + + - name: Force-move floating major tag + env: + TAG: ${{ github.ref_name }} + run: | + MAJOR="${TAG%%.*}" + git tag -f "$MAJOR" + git push origin "$MAJOR" --force + + - name: Create GitHub Release + env: + GH_TOKEN: ${{ steps.app-token.outputs.token }} + TAG: ${{ github.ref_name }} + run: gh release create "$TAG" --generate-notes diff --git a/.github/workflows/self_review.yml b/.github/workflows/self_review.yml new file mode 100644 index 0000000..5285a78 --- /dev/null +++ b/.github/workflows/self_review.yml @@ -0,0 +1,54 @@ +name: Self Review + +on: + pull_request: + types: [opened, synchronize, reopened, ready_for_review] + issue_comment: + types: [created] + +permissions: + contents: read + +concurrency: + group: self-review-${{ github.event.pull_request.number || github.event.issue.number }} + cancel-in-progress: true + +jobs: + review: + runs-on: ubuntu-latest + if: >- + github.event_name == 'pull_request' || + ( + github.event_name == 'issue_comment' && + github.event.issue.pull_request && + contains(github.event.comment.body, '@umm review') + ) + permissions: + contents: read + pull-requests: write + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + + - uses: actions/create-github-app-token@fee1f7d63c2ff003460e3d139729b119787bc349 # v2 + id: app-token + with: + app-id: ${{ secrets.UMM_APP_ID }} + private-key: ${{ secrets.UMM_PRIVATE_KEY }} + permission-contents: read + permission-pull-requests: write + + - name: Request review from umm-actually bot + env: + GH_TOKEN: ${{ steps.app-token.outputs.token }} + PR_NUMBER: ${{ github.event.pull_request.number || github.event.issue.number }} + run: | + gh api "repos/${{ github.repository }}/pulls/${PR_NUMBER}/requested_reviewers" \ + --method POST -f 'reviewers[]=umm-actually[bot]' || true + + - uses: ./ + with: + github_token: ${{ steps.app-token.outputs.token }} + openrouter_api_key: ${{ secrets.OPENROUTER_KEY }} + model: ${{ vars.OPENROUTER_MODEL || 'anthropic/claude-sonnet-4-6' }} diff --git a/AGENTS.md b/AGENTS.md index 1240026..41a2a55 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -16,7 +16,7 @@ action.yml # action metadata — inputs/outputs, runs.using: doc Dockerfile # multi-stage: build (tsc) → slim runtime fixtures/ # test fixtures (event payloads, sample diff, LLM responses) src/ - main.ts # entrypoint — collects/validates inputs; pipeline + outputs land next PR + main.ts # entrypoint — collects/validates inputs, wires clients into orchestrate, sets outputs config.ts # action inputs → validated ActionConfig logger.ts # structured JSON logger — levels, child contexts, lazy props github/ # GitHub I/O: event payload → PrContext, octokit wrappers (diff fetch, review posting) @@ -24,7 +24,7 @@ src/ diff/ # pure transforms over parse-diff output context/ # workspace I/O: conventions file, changed files, related-files reverse-import scan review/ # pure review logic: finding schema, phases, prompt, selection, comment mapping - orchestrate.ts # (planned — next PR) the pipeline — fully testable with stub clients + orchestrate.ts # pipeline + createPromptedGenerateFindings — fully testable with stub clients ``` ## Module layering @@ -90,7 +90,11 @@ files. Prefer SDK-provided types over redefining shapes. - `const` per test via factory helpers; `beforeEach` only when per-test creation is genuinely impractical. - Exact assertions over loose matchers; assert whole values over substrings - when output is deterministic. + when output is deterministic. When fixtures and stubs produce deterministic + results, assert the entire return value or call params — not just individual + fields. Asserting fragments is the cheap option; asserting the whole value + catches drift in formatting, structure, and attribution that field-level + checks miss. - Two-bar rule: a test must (1) fail when the behavior breaks and (2) pass only because the intended behavior occurred. Guard against silent no-op, wrong-error, and early-return passes. diff --git a/README.md b/README.md index b04c652..784d01a 100644 --- a/README.md +++ b/README.md @@ -2,8 +2,6 @@ LLM-powered pull request review as a GitHub Action. One consolidated review per PR with inline findings, powered by any model on [OpenRouter](https://openrouter.ai). -> Full documentation, inputs reference, and setup guide land with the first release. This repo is under active initial development. - ## What it does - Reviews the PR diff **and traces changed code into its callers** — regressions and pre-existing bugs in affected code are findings, not noise @@ -11,6 +9,127 @@ LLM-powered pull request review as a GitHub Action. One consolidated review per - Posts exactly **one** PR review with inline comments anchored to diff lines — no duplicate comments, no unrequested-reviewer badges - Structured output end to end: every finding carries a category, severity, confidence, and a concrete failure scenario - Model-agnostic via OpenRouter — pick your model, see your per-call costs +- Findings that can't be anchored to the diff (e.g. callers outside the changed files) render in the review body under "Findings beyond the diff" +- PRs with oversized diffs are skipped gracefully with a body-only review stating the reason + +## Setup + +umm-actually runs as a Docker-based action. It needs a GitHub token (for fetching the diff and posting the review) and an OpenRouter API key. + +For the best experience, use a [GitHub App](https://docs.github.com/en/apps/creating-github-apps) installation token so reviews are attributed to a bot identity rather than a personal account. + +## Usage + +```yaml +name: Review + +on: + pull_request: + types: [opened, synchronize, reopened, ready_for_review] + issue_comment: + types: [created] + +permissions: + contents: read + +concurrency: + group: review-${{ github.event.pull_request.number || github.event.issue.number }} + cancel-in-progress: true + +jobs: + review: + runs-on: ubuntu-latest + if: >- + github.event_name == 'pull_request' || + ( + github.event_name == 'issue_comment' && + github.event.issue.pull_request && + contains(github.event.comment.body, '@umm review') + ) + permissions: + contents: read + pull-requests: write + steps: + - uses: actions/checkout@v7 + with: + persist-credentials: false + + - uses: actions/create-github-app-token@v2 + id: app-token + with: + app-id: ${{ secrets.UMM_APP_ID }} + private-key: ${{ secrets.UMM_PRIVATE_KEY }} + + - uses: aliasunder/umm-actually@v0 + with: + github_token: ${{ steps.app-token.outputs.token }} + openrouter_api_key: ${{ secrets.OPENROUTER_KEY }} +``` + +The `@umm review` comment trigger lets you re-request a review on any PR by commenting. The `issue_comment` event fires for PR comments — the `if` condition filters to PRs only. + +## Inputs + +| Input | Default | Description | +| ----------------------- | ----------------------------- | ----------------------------------------------------------------------------------------------------------- | +| `github_token` | _(required)_ | Token for fetching the diff and posting the review. A GitHub App installation token keeps the bot identity. | +| `openrouter_api_key` | _(required)_ | OpenRouter API key | +| `model` | `anthropic/claude-sonnet-4-6` | OpenRouter model slug exactly as listed on openrouter.ai/models | +| `fallback_model` | `""` | Model to retry with if the primary model fails the structured-output ladder | +| `max_findings` | `""` _(uncapped)_ | Cap on posted findings, highest severity first. Empty = all validated findings post. | +| `severity_threshold` | `low` | Minimum severity to post: `low` \| `medium` \| `high` \| `critical` | +| `conventions_file` | `AGENTS.md` | Repo-relative path to the conventions file included in the prompt | +| `phases` | `combined` | Review phases to run. V1 supports: `combined` | +| `context_budget_tokens` | `80000` | Approximate token budget for prompt context (file contents + diff — conventions have a separate cap) | +| `trace_related_files` | `true` | Include files that reference changed files in the prompt so the model can trace regressions into callers | +| `cost_summary` | `true` | Write a per-run cost report (model, prompt/completion tokens, USD) to the workflow step summary | +| `pr_number` | `""` | PR number override — required only when the triggering event does not identify a PR directly | + +## Outputs + +| Output | Description | +| ---------------- | ------------------------------------------------------------ | +| `findings_count` | Number of findings posted (after threshold and cap) | +| `review_url` | URL of the submitted review; empty when no review was posted | +| `model_used` | Model that produced the accepted response | +| `skipped_reason` | Non-empty when the review was skipped (e.g. diff too large) | + +## How it works + +1. Resolves the PR from the triggering event (supports `pull_request`, `pull_request_target`, and `issue_comment` events) +2. Fetches the unified diff via the GitHub API — PRs that exceed the API's diff size limit are skipped +3. Reads the conventions file and changed source files (token-budgeted), then traces imports to find related files that reference the changes +4. Builds a structured prompt with randomized delimiter nonces (prompt injection defense) and sends it to OpenRouter +5. Validates the response against a strict Zod schema, retrying with a fallback model if the primary fails +6. Filters findings by severity threshold, deduplicates overlapping findings, and caps if configured +7. Maps findings to inline PR review comments anchored to diff lines, with a snap-to-nearest-hunk fallback +8. Posts one consolidated review — findings that can't be inlined render in the review body + +## Status + +umm-actually is in early development — the core review pipeline works but there's more to build. Here's what's shipped and what's in progress: + +**Shipped (V1)** + +- Single-pass review with inline findings anchored to diff lines +- Structured output with retry ladder and fallback model +- Import-tracing: changed code is traced into callers via reverse-import scan +- Token-budgeted context (changed files + related files + conventions) +- Prompt injection defense (randomized delimiter nonces) +- Skip-path handling with posted reasons (oversized diff, empty diff, API limits) +- Cost transparency (per-run model/token/USD report in workflow summary) +- `@umm review` comment trigger for on-demand re-reviews + +**In progress** + +- **Review dedup on re-runs** — currently each push posts a new review; working on deduplicating findings across runs and updating a single summary comment instead of creating new ones +- **Doc-staleness detection** — extending the workspace scan to doc files (`.md`, `.json`) so unchanged docs that describe changed code reach the prompt and staleness becomes a finding +- **Branded check run** — using the Checks API so the CI check shows the umm-actually avatar instead of the generic GitHub Actions logo + +**Planned** + +- V1.5: `read_file` verification tool — the model can read additional files before finalizing findings +- V2: bounded agentic exploration — multi-step investigation with tool use behind a `generateFindings` seam ## License diff --git a/action.yml b/action.yml index 4c9c45e..7fc1c02 100644 --- a/action.yml +++ b/action.yml @@ -37,7 +37,7 @@ inputs: required: false default: combined context_budget_tokens: - description: Approximate token budget for prompt context (conventions + file contents + diff) + description: Approximate token budget for prompt context (file contents + diff — conventions have a separate cap) required: false default: "80000" trace_related_files: diff --git a/src/__tests__/orchestrate.test.ts b/src/__tests__/orchestrate.test.ts new file mode 100644 index 0000000..4a93268 --- /dev/null +++ b/src/__tests__/orchestrate.test.ts @@ -0,0 +1,795 @@ +import { readFileSync } from "node:fs" +import parseDiff from "parse-diff" +import { describe, expect, it } from "vitest" +import type { ActionConfig } from "../config.js" +import type { GithubClient } from "../github/client.js" +import type { PrContext } from "../github/event.js" +import type { ContextReader } from "../context/workspace.js" +import type { + ModelAttempt, + OpenRouterClient, + StructuredReviewResult, +} from "../openrouter/client.js" +import { estimateTokens, type PromptFile } from "../review/prompt.js" +import { annotateDiff } from "../diff/annotate-diff.js" +import { computeCommentableLines } from "../diff/commentable-lines.js" +import type { ReviewResponse } from "../review/finding.js" +import { + buildReviewBody, + buildZeroFindingsBody, + mapFindingsToReview, + type ReviewComment, +} from "../review/comment-mapping.js" +import { selectFindings } from "../review/select-findings.js" +import { renderCostSummary } from "../openrouter/cost-summary.js" +import { + orchestrate, + createPromptedGenerateFindings, + type OrchestrateDeps, + type GenerateFindings, + type ReviewContext, +} from "../orchestrate.js" +import { createTestLogger } from "./test-logger.js" + +const sampleDiff = readFileSync( + new URL("../../fixtures/sample.diff", import.meta.url), + "utf8", +) + +const sampleDiffTokens = estimateTokens(annotateDiff(parseDiff(sampleDiff))) + +const pullRequestPayload: Record = JSON.parse( + readFileSync( + new URL("../../fixtures/pull_request.opened.json", import.meta.url), + "utf8", + ), +) + +const fixtureReviewResponse: ReviewResponse = JSON.parse( + readFileSync( + new URL("../../fixtures/openrouter.response.json", import.meta.url), + "utf8", + ), +) + +const fixturePrContext: PrContext = { + prNumber: 7, + title: "feat: trim names before greeting", + body: "Trims whitespace from names and validates registry keys.", + headSha: "abc123def456abc123def456abc123def456abc1", + headRef: "feat/trim-names", + baseRef: "main", +} + +const fixtureAttempt: ModelAttempt = { + model: "test/model", + outcome: "accepted", + promptTokens: 1000, + completionTokens: 500, + costUsd: 0.01, + errorSummary: null, +} + +const fixtureChangedFile: PromptFile = { + path: "src/greeter.ts", + content: "export const greet = (name: string) => name.trim()", + includedAs: "full", +} + +// Precomputed expected values from deterministic fixtures — used for exact +// assertions on the full result and submitReview params +const fixtureFiles = parseDiff(sampleDiff) +const fixtureCommentableByPath = computeCommentableLines(fixtureFiles) + +const expectedSelection = selectFindings({ + findings: fixtureReviewResponse.findings, + severityThreshold: "low", + maxFindings: undefined, +}) +const expectedMapped = mapFindingsToReview({ + findings: expectedSelection.selected, + commentableByPath: fixtureCommentableByPath, +}) +const expectedBody = buildReviewBody({ + bodyFindings: expectedMapped.bodyFindings, + droppedByCap: expectedSelection.droppedByCap, + model: "test/model", + inlineCommentCount: expectedMapped.comments.length, +}) +const expectedFallbackBody = buildReviewBody({ + bodyFindings: expectedSelection.selected, + droppedByCap: expectedSelection.droppedByCap, + model: "test/model", + bodyFindingsHeading: "Findings", + bodyFindingsDescription: + "Inline comments were unavailable; all findings are listed here:", +}) +const expectedCostSummary = renderCostSummary({ + attempts: [fixtureAttempt], + modelUsed: "test/model", +}) + +const expectedCappedSelection = selectFindings({ + findings: fixtureReviewResponse.findings, + severityThreshold: "low", + maxFindings: 1, +}) +const expectedCappedMapped = mapFindingsToReview({ + findings: expectedCappedSelection.selected, + commentableByPath: fixtureCommentableByPath, +}) +const expectedCappedBody = buildReviewBody({ + bodyFindings: expectedCappedMapped.bodyFindings, + droppedByCap: expectedCappedSelection.droppedByCap, + model: "test/model", + inlineCommentCount: expectedCappedMapped.comments.length, +}) + +const expectedZeroBody = buildZeroFindingsBody({ model: "test/model" }) + +const buildSkipBody = (reason: string): string => + `**umm-actually** — review skipped\n\n${reason}\n\n---\n*umm-actually*` + +const baseConfig: ActionConfig = { + githubToken: "ghp_test", + openrouterApiKey: "sk-test", + model: "test/model", + fallbackModel: "", + maxFindings: undefined, + severityThreshold: "low", + conventionsFile: "AGENTS.md", + phases: "combined", + contextBudgetTokens: 80_000, + traceRelatedFiles: true, + costSummary: true, + prNumberOverride: undefined, +} + +type SubmitReviewParams = { + prNumber: number + commitId: string + body: string + comments: ReviewComment[] + fallbackBody: string +} + +type ReadChangedFilesParams = { + changedPaths: string[] + budgetTokens: number +} + +type FindRelatedFilesParams = { + changedPaths: string[] + budgetTokens: number +} + +type RequestReviewParams = { + systemPrompt: string + userPrompt: string + model: string + fallbackModel: string | null +} + +const first = (array: T[]): T => { + const item = array[0] + if (item === undefined) throw new Error("expected at least one element") + return item +} + +type RecordingStubs = { + deps: OrchestrateDeps + fetchPullRequestCalls: { prNumber: number }[] + fetchDiffCalls: { prNumber: number }[] + submitReviewCalls: SubmitReviewParams[] + readConventionsCalls: { conventionsFile: string }[] + readChangedFilesCalls: ReadChangedFilesParams[] + findRelatedFilesCalls: FindRelatedFilesParams[] + generateFindingsCalls: ReviewContext[] +} + +const makeOrchestrateDeps = ( + overrides: { + config?: Partial + eventName?: string + payload?: unknown + githubClient?: Partial + contextReader?: Partial + generateFindings?: GenerateFindings + fixtureResult?: Partial + } = {}, +): RecordingStubs => { + const fetchPullRequestCalls: { prNumber: number }[] = [] + const fetchDiffCalls: { prNumber: number }[] = [] + const submitReviewCalls: SubmitReviewParams[] = [] + const readConventionsCalls: { conventionsFile: string }[] = [] + const readChangedFilesCalls: ReadChangedFilesParams[] = [] + const findRelatedFilesCalls: FindRelatedFilesParams[] = [] + const generateFindingsCalls: ReviewContext[] = [] + + const structuredResult: StructuredReviewResult = { + review: fixtureReviewResponse, + modelUsed: "test/model", + attempts: [fixtureAttempt], + ...overrides.fixtureResult, + } + + const githubClient: GithubClient = { + fetchPullRequest: async (params) => { + fetchPullRequestCalls.push(params) + return fixturePrContext + }, + fetchDiff: async (params) => { + fetchDiffCalls.push(params) + return { kind: "ok" as const, diff: sampleDiff } + }, + submitReview: async (params) => { + submitReviewCalls.push(params) + return { + url: "https://github.com/test/review/1", + usedFallbackBody: false, + } + }, + ...overrides.githubClient, + } + + const contextReader: ContextReader = { + readConventions: async (params) => { + readConventionsCalls.push(params) + return "# Test conventions" + }, + readChangedFiles: async (params) => { + readChangedFilesCalls.push(params) + return { files: [fixtureChangedFile], remainingTokens: 40_000 } + }, + findRelatedFiles: async (params) => { + findRelatedFilesCalls.push(params) + return [] + }, + ...overrides.contextReader, + } + + const defaultGenerateFindings: GenerateFindings = async (reviewContext) => { + generateFindingsCalls.push(reviewContext) + return structuredResult + } + + const deps: OrchestrateDeps = { + config: { ...baseConfig, ...overrides.config }, + eventName: overrides.eventName ?? "pull_request", + payload: overrides.payload ?? pullRequestPayload, + githubClient, + contextReader, + generateFindings: overrides.generateFindings ?? defaultGenerateFindings, + } + + return { + deps, + fetchPullRequestCalls, + fetchDiffCalls, + submitReviewCalls, + readConventionsCalls, + readChangedFilesCalls, + findRelatedFilesCalls, + generateFindingsCalls, + } +} + +describe("orchestrate", () => { + describe("startup validation", () => { + it("throws on invalid severity threshold before any network call", async () => { + const stubs = makeOrchestrateDeps({ + config: { severityThreshold: "invalid" }, + }) + const logger = createTestLogger() + + await expect(orchestrate(stubs.deps, logger)).rejects.toThrow("severity") + expect(stubs.fetchDiffCalls).toHaveLength(0) + expect(stubs.generateFindingsCalls).toHaveLength(0) + expect(stubs.submitReviewCalls).toHaveLength(0) + }) + + it("throws on invalid phases input before any network call", async () => { + const stubs = makeOrchestrateDeps({ + config: { phases: "invalid" }, + }) + const logger = createTestLogger() + + await expect(orchestrate(stubs.deps, logger)).rejects.toThrow("phases") + expect(stubs.fetchDiffCalls).toHaveLength(0) + expect(stubs.generateFindingsCalls).toHaveLength(0) + }) + }) + + describe("event resolution", () => { + it("returns skipped result for non-PR events without calling any stubs", async () => { + const stubs = makeOrchestrateDeps({ + eventName: "push", + payload: {}, + }) + const logger = createTestLogger() + + const result = await orchestrate(stubs.deps, logger) + + expect(result).toEqual({ + findingsCount: 0, + reviewUrl: "", + modelUsed: "", + skippedReason: "unsupported event: push", + costSummaryMarkdown: null, + }) + expect(stubs.generateFindingsCalls).toHaveLength(0) + expect(stubs.submitReviewCalls).toHaveLength(0) + }) + + it("fetches PR context when event needs fetch", async () => { + const stubs = makeOrchestrateDeps({ + config: { prNumberOverride: 42 }, + }) + const logger = createTestLogger() + + await orchestrate(stubs.deps, logger) + + expect(stubs.fetchPullRequestCalls).toHaveLength(1) + expect(stubs.fetchPullRequestCalls[0]).toEqual({ prNumber: 42 }) + }) + }) + + describe("skip paths — post body-only review", () => { + it("posts skip review when diff is too large", async () => { + const stubs = makeOrchestrateDeps({ + githubClient: { + fetchDiff: async () => ({ kind: "too_large" as const }), + }, + }) + const logger = createTestLogger() + + const result = await orchestrate(stubs.deps, logger) + + const skipReason = "diff exceeds GitHub's diff API limits" + expect(result).toEqual({ + findingsCount: 0, + reviewUrl: "https://github.com/test/review/1", + modelUsed: "", + skippedReason: skipReason, + costSummaryMarkdown: null, + }) + expect(stubs.generateFindingsCalls).toHaveLength(0) + expect(stubs.submitReviewCalls).toHaveLength(1) + expect(first(stubs.submitReviewCalls)).toEqual({ + prNumber: fixturePrContext.prNumber, + commitId: fixturePrContext.headSha, + body: buildSkipBody(skipReason), + comments: [], + fallbackBody: buildSkipBody(skipReason), + }) + }) + + it("posts skip review when diff parses to zero files", async () => { + const stubs = makeOrchestrateDeps({ + githubClient: { + fetchDiff: async () => ({ kind: "ok" as const, diff: "" }), + }, + }) + const logger = createTestLogger() + + const result = await orchestrate(stubs.deps, logger) + + const skipReason = "empty diff" + expect(result).toEqual({ + findingsCount: 0, + reviewUrl: "https://github.com/test/review/1", + modelUsed: "", + skippedReason: skipReason, + costSummaryMarkdown: null, + }) + expect(stubs.generateFindingsCalls).toHaveLength(0) + expect(stubs.submitReviewCalls).toHaveLength(1) + expect(first(stubs.submitReviewCalls)).toEqual({ + prNumber: fixturePrContext.prNumber, + commitId: fixturePrContext.headSha, + body: buildSkipBody(skipReason), + comments: [], + fallbackBody: buildSkipBody(skipReason), + }) + }) + + it("posts skip review when annotated diff exceeds budget", async () => { + const stubs = makeOrchestrateDeps({ + config: { contextBudgetTokens: 10 }, + }) + const logger = createTestLogger() + + const result = await orchestrate(stubs.deps, logger) + + const budgetHalf = Math.floor(10 / 2) + const skipReason = `diff too large for context budget (${sampleDiffTokens} tokens, limit ${budgetHalf} of 10)` + expect(result).toEqual({ + findingsCount: 0, + reviewUrl: "https://github.com/test/review/1", + modelUsed: "", + skippedReason: skipReason, + costSummaryMarkdown: null, + }) + expect(stubs.generateFindingsCalls).toHaveLength(0) + expect(stubs.submitReviewCalls).toHaveLength(1) + expect(first(stubs.submitReviewCalls)).toEqual({ + prNumber: fixturePrContext.prNumber, + commitId: fixturePrContext.headSha, + body: buildSkipBody(skipReason), + comments: [], + fallbackBody: buildSkipBody(skipReason), + }) + }) + + it("passes correct prNumber and commitId in skip reviews", async () => { + const stubs = makeOrchestrateDeps({ + githubClient: { + fetchDiff: async () => ({ kind: "too_large" as const }), + }, + }) + const logger = createTestLogger() + + await orchestrate(stubs.deps, logger) + + const reviewCall = first(stubs.submitReviewCalls) + expect(reviewCall.prNumber).toBe(fixturePrContext.prNumber) + expect(reviewCall.commitId).toBe(fixturePrContext.headSha) + }) + }) + + describe("happy path", () => { + it("posts review with correct params and returns expected result", async () => { + const stubs = makeOrchestrateDeps() + const logger = createTestLogger() + + const result = await orchestrate(stubs.deps, logger) + + expect(result).toEqual({ + findingsCount: expectedSelection.selected.length, + reviewUrl: "https://github.com/test/review/1", + modelUsed: "test/model", + skippedReason: "", + costSummaryMarkdown: expectedCostSummary, + }) + + expect(stubs.submitReviewCalls).toHaveLength(1) + expect(first(stubs.submitReviewCalls)).toEqual({ + prNumber: fixturePrContext.prNumber, + commitId: fixturePrContext.headSha, + body: expectedBody, + comments: expectedMapped.comments, + fallbackBody: expectedFallbackBody, + }) + }) + + it("passes changed files and conventions to generateFindings", async () => { + const stubs = makeOrchestrateDeps() + const logger = createTestLogger() + + await orchestrate(stubs.deps, logger) + + expect(stubs.generateFindingsCalls).toHaveLength(1) + const reviewContext = first(stubs.generateFindingsCalls) + expect(reviewContext.conventions).toBe("# Test conventions") + expect(reviewContext.changedFiles).toEqual([fixtureChangedFile]) + expect(reviewContext.prContext).toEqual(fixturePrContext) + }) + + it("includes annotated diff in review context", async () => { + const stubs = makeOrchestrateDeps() + const logger = createTestLogger() + + await orchestrate(stubs.deps, logger) + + const reviewContext = first(stubs.generateFindingsCalls) + expect(reviewContext.annotatedDiff).toContain("=== src/greeter.ts ===") + }) + }) + + describe("context wiring", () => { + it("passes budget minus diff tokens to readChangedFiles", async () => { + const localReadChangedFilesCalls: ReadChangedFilesParams[] = [] + const stubs = makeOrchestrateDeps({ + contextReader: { + readChangedFiles: async (params) => { + localReadChangedFilesCalls.push(params) + return { files: [fixtureChangedFile], remainingTokens: 10_000 } + }, + }, + }) + const logger = createTestLogger() + + await orchestrate(stubs.deps, logger) + + expect(localReadChangedFilesCalls).toHaveLength(1) + const call = first(localReadChangedFilesCalls) + expect(call.budgetTokens).toBe( + baseConfig.contextBudgetTokens - sampleDiffTokens, + ) + }) + + it("passes remainingTokens from readChangedFiles to findRelatedFiles", async () => { + const expectedRemainingTokens = 12_345 + const localFindRelatedFilesCalls: FindRelatedFilesParams[] = [] + const stubs = makeOrchestrateDeps({ + config: { traceRelatedFiles: true }, + contextReader: { + readChangedFiles: async () => ({ + files: [fixtureChangedFile], + remainingTokens: expectedRemainingTokens, + }), + findRelatedFiles: async (params) => { + localFindRelatedFilesCalls.push(params) + return [] + }, + }, + }) + const logger = createTestLogger() + + await orchestrate(stubs.deps, logger) + + expect(localFindRelatedFilesCalls).toHaveLength(1) + const call = first(localFindRelatedFilesCalls) + expect(call.budgetTokens).toBe(expectedRemainingTokens) + }) + + it("passes paths extracted from the diff to readChangedFiles", async () => { + const stubs = makeOrchestrateDeps() + const logger = createTestLogger() + + await orchestrate(stubs.deps, logger) + + expect(stubs.readChangedFilesCalls).toHaveLength(1) + const changedPaths = first(stubs.readChangedFilesCalls).changedPaths + // The fixture diff modifies src/greeter.ts, adds src/added-file.ts, + // renames to src/new-name.ts, modifies src/no-trailing-newline.ts, and + // deletes src/removed-file.ts (null newFilePath) + binary logo.png + expect(changedPaths).toContain("src/greeter.ts") + expect(changedPaths).toContain("src/added-file.ts") + expect(changedPaths).toContain("src/new-name.ts") + // Deleted file has no newFilePath — should NOT appear + expect(changedPaths).not.toContain("src/removed-file.ts") + }) + }) + + describe("conditional behaviors", () => { + it("does not call findRelatedFiles when traceRelatedFiles is false", async () => { + const stubs = makeOrchestrateDeps({ + config: { traceRelatedFiles: false }, + }) + const logger = createTestLogger() + + await orchestrate(stubs.deps, logger) + + expect(stubs.findRelatedFilesCalls).toHaveLength(0) + const reviewContext = first(stubs.generateFindingsCalls) + expect(reviewContext.relatedFiles).toEqual([]) + }) + + it("calls findRelatedFiles when traceRelatedFiles is true", async () => { + const stubs = makeOrchestrateDeps({ + config: { traceRelatedFiles: true }, + }) + const logger = createTestLogger() + + await orchestrate(stubs.deps, logger) + + expect(stubs.findRelatedFilesCalls).toHaveLength(1) + }) + + it("posts zero-findings body when all findings are below threshold", async () => { + const stubs = makeOrchestrateDeps({ + config: { severityThreshold: "critical" }, + }) + const logger = createTestLogger() + + const result = await orchestrate(stubs.deps, logger) + + expect(result.findingsCount).toBe(0) + expect(result.reviewUrl).toBe("https://github.com/test/review/1") + expect(result.skippedReason).toBe("") + expect(stubs.submitReviewCalls).toHaveLength(1) + expect(first(stubs.submitReviewCalls)).toEqual({ + prNumber: fixturePrContext.prNumber, + commitId: fixturePrContext.headSha, + body: expectedZeroBody, + comments: [], + fallbackBody: expectedZeroBody, + }) + }) + + it("respects maxFindings cap and includes dropped findings in body", async () => { + const stubs = makeOrchestrateDeps({ + config: { maxFindings: 1 }, + }) + const logger = createTestLogger() + + const result = await orchestrate(stubs.deps, logger) + + expect(result.findingsCount).toBe(1) + expect(stubs.submitReviewCalls).toHaveLength(1) + const reviewCall = first(stubs.submitReviewCalls) + expect(reviewCall.body).toBe(expectedCappedBody) + }) + + it("returns costSummaryMarkdown when LLM call happened", async () => { + const stubs = makeOrchestrateDeps() + const logger = createTestLogger() + + const result = await orchestrate(stubs.deps, logger) + + expect(result.costSummaryMarkdown).toBe(expectedCostSummary) + }) + + it("returns null costSummaryMarkdown when skipped before LLM call", async () => { + const stubs = makeOrchestrateDeps({ + githubClient: { + fetchDiff: async () => ({ kind: "too_large" as const }), + }, + }) + const logger = createTestLogger() + + const result = await orchestrate(stubs.deps, logger) + + expect(result.costSummaryMarkdown).toBeNull() + }) + + it("builds fallback body with all selected findings", async () => { + const stubs = makeOrchestrateDeps() + const logger = createTestLogger() + + await orchestrate(stubs.deps, logger) + + const reviewCall = first(stubs.submitReviewCalls) + expect(reviewCall.fallbackBody).toBe(expectedFallbackBody) + }) + + it("uses complete PrContext from pull_request event without fetching", async () => { + const stubs = makeOrchestrateDeps() + const logger = createTestLogger() + + await orchestrate(stubs.deps, logger) + + expect(stubs.fetchPullRequestCalls).toHaveLength(0) + }) + }) +}) + +describe("createPromptedGenerateFindings", () => { + it("passes model and fallbackModel to openrouterClient.requestReview", async () => { + const requestReviewCalls: RequestReviewParams[] = [] + const stubClient: OpenRouterClient = { + requestReview: async (params) => { + requestReviewCalls.push(params) + return { + review: fixtureReviewResponse, + modelUsed: "test/model", + attempts: [fixtureAttempt], + } + }, + } + + const generate = createPromptedGenerateFindings( + { + openrouterClient: stubClient, + model: "test/primary", + fallbackModel: "test/fallback", + }, + createTestLogger(), + ) + + const files = parseDiff(sampleDiff) + const { annotateDiff } = await import("../diff/annotate-diff.js") + const { resolvePhases } = await import("../review/phases.js") + const phases = resolvePhases("combined") + const phase = phases[0] + if (phase === undefined) throw new Error("expected a phase") + + await generate({ + prContext: fixturePrContext, + phase, + conventions: "test conventions", + changedFiles: [fixtureChangedFile], + relatedFiles: [], + annotatedDiff: annotateDiff(files), + priorFindings: [], + }) + + expect(requestReviewCalls).toHaveLength(1) + expect(requestReviewCalls[0]).toEqual( + expect.objectContaining({ + model: "test/primary", + fallbackModel: "test/fallback", + }), + ) + }) + + it("includes annotated diff in the user prompt", async () => { + const requestReviewCalls: RequestReviewParams[] = [] + const stubClient: OpenRouterClient = { + requestReview: async (params) => { + requestReviewCalls.push(params) + return { + review: fixtureReviewResponse, + modelUsed: "test/model", + attempts: [fixtureAttempt], + } + }, + } + + const generate = createPromptedGenerateFindings( + { openrouterClient: stubClient, model: "m", fallbackModel: null }, + createTestLogger(), + ) + + const files = parseDiff(sampleDiff) + const { annotateDiff } = await import("../diff/annotate-diff.js") + const { resolvePhases } = await import("../review/phases.js") + const phases = resolvePhases("combined") + const phase = phases[0] + if (phase === undefined) throw new Error("expected a phase") + const annotated = annotateDiff(files) + + await generate({ + prContext: fixturePrContext, + phase, + conventions: null, + changedFiles: [], + relatedFiles: [], + annotatedDiff: annotated, + priorFindings: [], + }) + + const call = first(requestReviewCalls) + expect(call.userPrompt).toContain("src/greeter.ts") + }) + + it("generates a unique nonce per call", async () => { + const userPrompts: string[] = [] + const stubClient: OpenRouterClient = { + requestReview: async (params) => { + userPrompts.push(params.userPrompt) + return { + review: { analysis: "", findings: [] }, + modelUsed: "m", + attempts: [], + } + }, + } + + const generate = createPromptedGenerateFindings( + { openrouterClient: stubClient, model: "m", fallbackModel: null }, + createTestLogger(), + ) + + const files = parseDiff(sampleDiff) + const { annotateDiff } = await import("../diff/annotate-diff.js") + const { resolvePhases } = await import("../review/phases.js") + const phases = resolvePhases("combined") + const phase = phases[0] + if (phase === undefined) throw new Error("expected a phase") + const annotated = annotateDiff(files) + + const context = { + prContext: fixturePrContext, + phase, + conventions: null, + changedFiles: [], + relatedFiles: [], + annotatedDiff: annotated, + priorFindings: [], + } + + await generate(context) + await generate(context) + + expect(userPrompts).toHaveLength(2) + // Nonce-suffixed tags should differ between calls + const noncePattern = / ({ prNumberOverride: core.getInput("pr_number"), }) -// Input collection and config validation are wired now so bad inputs fail -// loudly today; orchestrate.ts lands in the next PR and replaces the -// setFailed stub below. try { const config = parseConfig(collectRawInputs()) - // Register both credentials with the runner's masker before anything can - // log — our JSON logger writes raw to stdout, bypassing the masking the - // runner applies only to values passed through ::add-mask::. core.setSecret(config.githubToken) core.setSecret(config.openrouterApiKey) - core.setFailed( - "umm-actually: review pipeline not yet implemented (scaffold only)", - ) -} catch (configError) { - core.setFailed( - configError instanceof Error ? configError.message : String(configError), + + const workspaceRoot = envVar + .from(process.env) + .get("GITHUB_WORKSPACE") + .required() + .asString() + const octokit = getOctokit(config.githubToken) + const { owner, repo } = context.repo + + const result = await orchestrate( + { + config, + eventName: context.eventName, + payload: context.payload, + githubClient: createGithubClient({ octokit, owner, repo }, logger), + contextReader: createContextReader({ workspaceRoot }, logger), + generateFindings: createPromptedGenerateFindings( + { + openrouterClient: createOpenRouterClient( + { sdk: new OpenRouter({ apiKey: config.openrouterApiKey }) }, + logger, + ), + model: config.model, + fallbackModel: + config.fallbackModel === "" ? null : config.fallbackModel, + }, + logger, + ), + }, + logger, ) + + core.setOutput("findings_count", result.findingsCount) + core.setOutput("review_url", result.reviewUrl) + core.setOutput("model_used", result.modelUsed) + core.setOutput("skipped_reason", result.skippedReason) + if (config.costSummary && result.costSummaryMarkdown !== null) { + await core.summary.addRaw(result.costSummaryMarkdown).write() + } +} catch (error) { + core.setFailed(error instanceof Error ? error.message : String(error)) } diff --git a/src/orchestrate.ts b/src/orchestrate.ts new file mode 100644 index 0000000..f2b25e7 --- /dev/null +++ b/src/orchestrate.ts @@ -0,0 +1,329 @@ +import parseDiff from "parse-diff" +import type { ActionConfig } from "./config.js" +import { + computeCommentableLines, + newFilePath, + type CommentableFile, +} from "./diff/commentable-lines.js" +import { annotateDiff } from "./diff/annotate-diff.js" +import type { Logger } from "./logger.js" +import type { GithubClient } from "./github/client.js" +import { resolvePullRequestEvent, type PrContext } from "./github/event.js" +import type { + OpenRouterClient, + StructuredReviewResult, +} from "./openrouter/client.js" +import { renderCostSummary } from "./openrouter/cost-summary.js" +import type { ContextReader } from "./context/workspace.js" +import { + buildReviewBody, + buildZeroFindingsBody, + mapFindingsToReview, + type ReviewComment, +} from "./review/comment-mapping.js" +import { resolveSeverityThreshold, type Finding } from "./review/finding.js" +import { resolvePhases, type ReviewPhase } from "./review/phases.js" +import { + buildSystemPrompt, + buildUserPrompt, + estimateTokens, + generateDelimiterNonce, + type PromptFile, +} from "./review/prompt.js" +import { selectFindings } from "./review/select-findings.js" + +export type ReviewContext = { + prContext: PrContext + phase: ReviewPhase + conventions: string | null + changedFiles: PromptFile[] + relatedFiles: PromptFile[] + annotatedDiff: string + priorFindings: Finding[] +} + +export type GenerateFindings = ( + reviewContext: ReviewContext, +) => Promise + +export type OrchestrateResult = { + findingsCount: number + reviewUrl: string + modelUsed: string + skippedReason: string + costSummaryMarkdown: string | null +} + +export type OrchestrateDeps = { + config: ActionConfig + eventName: string + payload: unknown + githubClient: GithubClient + contextReader: ContextReader + generateFindings: GenerateFindings +} + +const buildSkipBody = (reason: string): string => + `**umm-actually** — review skipped\n\n${reason}\n\n---\n*umm-actually*` + +type ReviewPayload = { + body: string + comments: ReviewComment[] + fallbackBody: string +} + +/** Separates findings into inline comments and body-only findings, then + * builds a fallback body that includes all findings in case GitHub rejects + * the inline anchors. */ +const buildReviewPayload = ({ + findings, + commentableByPath, + droppedByCap, + modelUsed, +}: { + findings: Finding[] + commentableByPath: Map + droppedByCap: Finding[] + modelUsed: string +}): ReviewPayload => { + const { comments, bodyFindings } = mapFindingsToReview({ + findings, + commentableByPath, + }) + const body = buildReviewBody({ + bodyFindings, + droppedByCap, + model: modelUsed, + inlineCommentCount: comments.length, + }) + const fallbackBody = buildReviewBody({ + bodyFindings: findings, + droppedByCap, + model: modelUsed, + bodyFindingsHeading: "Findings", + bodyFindingsDescription: + "Inline comments were unavailable; all findings are listed here:", + }) + return { body, comments, fallbackBody } +} + +const SKIPPED_RESULT_BASE: Omit< + OrchestrateResult, + "reviewUrl" | "skippedReason" +> = { + findingsCount: 0, + modelUsed: "", + costSummaryMarkdown: null, +} + +/** Runs the full review pipeline — event resolution through review posting — + * with all I/O injected through deps so the pipeline is fully testable. */ +export const orchestrate = async ( + deps: OrchestrateDeps, + logger: Logger, +): Promise => { + const { config, githubClient, contextReader, generateFindings } = deps + + // Step 1: fail-fast validation — throws before any network call + const severityThreshold = resolveSeverityThreshold(config.severityThreshold) + const phases = resolvePhases(config.phases) + + // Step 2: event resolution + const resolvedEvent = resolvePullRequestEvent( + { + eventName: deps.eventName, + payload: deps.payload, + prNumberOverride: config.prNumberOverride, + }, + logger, + ) + + if (resolvedEvent.kind === "not_a_pr") { + logger.info("not a PR event — skipping", { reason: resolvedEvent.reason }) + return { + ...SKIPPED_RESULT_BASE, + reviewUrl: "", + skippedReason: resolvedEvent.reason, + } + } + + // Step 3: PR context + const prContext: PrContext = + resolvedEvent.kind === "complete" + ? resolvedEvent.context + : await githubClient.fetchPullRequest({ + prNumber: resolvedEvent.prNumber, + }) + + const postSkipReview = async (reason: string): Promise => { + const body = buildSkipBody(reason) + const { url } = await githubClient.submitReview({ + prNumber: prContext.prNumber, + commitId: prContext.headSha, + body, + comments: [], + fallbackBody: body, + }) + logger.info("posted skip review", { reason, reviewUrl: url }) + return { ...SKIPPED_RESULT_BASE, reviewUrl: url, skippedReason: reason } + } + + // Step 4: diff fetch + const diffResult = await githubClient.fetchDiff({ + prNumber: prContext.prNumber, + }) + if (diffResult.kind === "too_large") { + return postSkipReview("diff exceeds GitHub's diff API limits") + } + + // Step 5: parse diff + const files = parseDiff(diffResult.diff) + if (files.length === 0) { + return postSkipReview("empty diff") + } + + // Step 6: annotate + token check + const annotatedDiff = annotateDiff(files) + const diffTokens = estimateTokens(annotatedDiff) + const budgetHalf = Math.floor(config.contextBudgetTokens / 2) + if (diffTokens > budgetHalf) { + return postSkipReview( + `diff too large for context budget (${diffTokens} tokens, limit ${budgetHalf} of ${config.contextBudgetTokens})`, + ) + } + + // Step 7: commentable lines + const commentableByPath = computeCommentableLines(files) + + // Step 8: extract changed paths (includes old path for renames so the + // import scanner finds callers that still reference the pre-rename path) + const changedPaths = files + .flatMap((file) => { + const toPath = newFilePath(file) + const isRename = + toPath !== null && + file.from !== undefined && + file.from !== "/dev/null" && + file.from !== file.to + return isRename ? [toPath, file.from] : [toPath] + }) + .filter((path): path is string => path !== null) + + // Step 9: context reads + const conventions = await contextReader.readConventions({ + conventionsFile: config.conventionsFile, + }) + + const fileBudgetTokens = config.contextBudgetTokens - diffTokens + const { files: changedFiles, remainingTokens } = + await contextReader.readChangedFiles({ + changedPaths, + budgetTokens: fileBudgetTokens, + }) + + const relatedFiles = config.traceRelatedFiles + ? await contextReader.findRelatedFiles({ + changedPaths, + budgetTokens: remainingTokens, + }) + : [] + + // Step 10–11: generate findings (V1: single combined phase) + const phase = phases[0] + if (phase === undefined) { + throw new Error("resolvePhases returned no phases") + } + const structuredResult = await generateFindings({ + prContext, + phase, + conventions, + changedFiles, + relatedFiles, + annotatedDiff, + priorFindings: [], + }) + + const { modelUsed, attempts } = structuredResult + + // Step 12: select findings + const { selected, droppedByCap } = selectFindings({ + findings: structuredResult.review.findings, + severityThreshold, + maxFindings: config.maxFindings, + }) + + // Step 13: post review + const hasFindings = selected.length > 0 + + const reviewPayload: ReviewPayload = hasFindings + ? buildReviewPayload({ + findings: selected, + commentableByPath, + droppedByCap, + modelUsed, + }) + : { + body: buildZeroFindingsBody({ model: modelUsed }), + comments: [], + fallbackBody: buildZeroFindingsBody({ model: modelUsed }), + } + + const { url: reviewUrl } = await githubClient.submitReview({ + prNumber: prContext.prNumber, + commitId: prContext.headSha, + ...reviewPayload, + }) + + logger.info("review posted", { + reviewUrl, + findingsCount: selected.length, + modelUsed, + }) + + // Step 14: cost summary + const costSummaryMarkdown = renderCostSummary({ attempts, modelUsed }) + + return { + findingsCount: selected.length, + reviewUrl, + modelUsed, + skippedReason: "", + costSummaryMarkdown, + } +} + +/** V1 one-shot strategy — builds a prompt from the review context and sends + * it to OpenRouter. V1.5/V2 will swap in different strategies behind the + * same GenerateFindings interface. */ +export const createPromptedGenerateFindings = ( + { + openrouterClient, + model, + fallbackModel, + }: { + openrouterClient: OpenRouterClient + model: string + fallbackModel: string | null + }, + logger: Logger, +): GenerateFindings => { + const log = logger.child({ module: "generateFindings" }) + + return async (reviewContext) => { + const delimiterNonce = generateDelimiterNonce() + const systemPrompt = buildSystemPrompt({ phase: reviewContext.phase }) + const userPrompt = buildUserPrompt({ + ...reviewContext, + delimiterNonce, + }) + + log.info("requesting review", { model, fallbackModel }) + + return openrouterClient.requestReview({ + systemPrompt, + userPrompt, + model, + fallbackModel, + }) + } +} diff --git a/src/review/comment-mapping.ts b/src/review/comment-mapping.ts index bc6e658..1e43acf 100644 --- a/src/review/comment-mapping.ts +++ b/src/review/comment-mapping.ts @@ -185,20 +185,31 @@ const renderBodyFinding = (finding: Finding): string => ${finding.description} **Failure scenario:** ${finding.failure_scenario}${suggestionBlock(finding)}` -/** The review's top-level body: beyond-diff findings, cap note, attribution. */ +/** The review's top-level body: summary, body findings section, cap note, attribution. */ export const buildReviewBody = ({ bodyFindings, droppedByCap, model, + inlineCommentCount = 0, + bodyFindingsHeading = "Findings beyond the diff", + bodyFindingsDescription = "These are in code the changes touch or depend on, outside the diff's line ranges:", }: { bodyFindings: Finding[] droppedByCap: Finding[] model: string + inlineCommentCount?: number + bodyFindingsHeading?: string + bodyFindingsDescription?: string }): string => { + const summaryLine = + inlineCommentCount > 0 && bodyFindings.length === 0 + ? `Reviewed — ${inlineCommentCount} finding(s) posted as inline comments.` + : "" + const beyondDiffSection = bodyFindings.length === 0 ? "" - : `### Findings beyond the diff\n\nThese are in code the changes touch or depend on, outside the diff's line ranges:\n\n${bodyFindings.map(renderBodyFinding).join("\n\n")}` + : `### ${bodyFindingsHeading}\n\n${bodyFindingsDescription}\n\n${bodyFindings.map(renderBodyFinding).join("\n\n")}` const capNote = droppedByCap.length === 0 @@ -207,7 +218,7 @@ export const buildReviewBody = ({ const attribution = `---\n*umm-actually · ${model}*` - return [beyondDiffSection, capNote, attribution] + return [summaryLine, beyondDiffSection, capNote, attribution] .filter((section) => section !== "") .join("\n\n") }