Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 9 additions & 3 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,19 @@ files. Prefer SDK-provided types over redefining shapes.
cohesive service surfaces (`githubClient.submitReview(…)`).
- `type` over `interface`. TypeScript strict mode. `node:` prefix for built-ins.
- Explicit return types on exports. No `any`. No `as` or `!` — use runtime
guards or schema validation to narrow.
guards or schema validation to narrow. Truthy/falsy checks over explicit
`!== undefined` comparisons — use `if (value)` not `if (value !== undefined)`
unless distinguishing `undefined` from other falsy values (`null`, `0`, `""`,
`false`) actually matters for correctness.
- Immutable by default; avoid `let`. A `reduce` must return a new accumulator
each step — never mutate-and-return. When mutation is genuinely needed,
add a comment justifying it.
- Explicit names over abbreviations, everywhere — params, callbacks, locals.
- Early returns over nested `if/else`. Extract multi-clause conditionals into
named booleans. Name booleans for the affirmative state.
- Early returns over nested `if/else`. When a function has a primary path and
a secondary path (e.g. first-run vs re-run), return early from the simpler
branch so the remaining code flows linearly without nesting. Extract
multi-clause conditionals into named booleans. Name booleans for the
affirmative state.
- Block bodies `{}` for any multiline function response — expression bodies
only for one-liners. A multi-clause boolean spanning lines gets
`{ return (...) }`; guard chains get explicit early returns, never a
Expand Down
19 changes: 10 additions & 9 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,12 @@ The `@umm review` comment trigger lets you re-request a review on any PR by comm

## 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) |
| Output | Description |
| ---------------- | ------------------------------------------------------------------------- |
| `findings_count` | Number of new findings posted (after threshold, cap, and cross-run dedup) |
| `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

Expand All @@ -102,8 +102,9 @@ The `@umm review` comment trigger lets you re-request a review on any PR by comm
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
7. On re-runs, compares findings against previously posted inline comments (by hidden HTML anchor) and filters out duplicates
8. Maps findings to inline PR review comments anchored to diff lines, with a snap-to-nearest-hunk fallback
9. Posts one consolidated review — findings that can't be inlined render in the review body; re-runs upsert a summary comment with totals

## Status

Expand All @@ -119,10 +120,10 @@ umm-actually is in early development — the core review pipeline works but ther
- 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
- Cross-run finding dedup — re-runs detect previously posted inline findings via hidden HTML anchors and post only new ones, with an updatable summary comment tracking totals

**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

Expand Down
2 changes: 1 addition & 1 deletion action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ inputs:

outputs:
findings_count:
description: Number of findings posted (after threshold and cap)
description: Number of new findings posted (after threshold, cap, and cross-run dedup)
Comment thread
aliasunder marked this conversation as resolved.
review_url:
description: URL of the submitted review; empty when no review was posted
model_used:
Expand Down
197 changes: 197 additions & 0 deletions src/__tests__/orchestrate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,12 @@ import { annotateDiff } from "../diff/annotate-diff.js"
import { computeCommentableLines } from "../diff/commentable-lines.js"
import type { ReviewResponse } from "../review/finding.js"
import {
buildRerunSummary,
buildReviewBody,
buildZeroFindingsBody,
computeAnchorKey,
mapFindingsToReview,
RERUN_ANCHOR,
type ReviewComment,
} from "../review/comment-mapping.js"
import { selectFindings } from "../review/select-findings.js"
Expand Down Expand Up @@ -176,11 +179,19 @@ const first = <T>(array: T[]): T => {
return item
}

type UpsertSummaryCommentParams = {
prNumber: number
body: string
anchor: string
}

type RecordingStubs = {
deps: OrchestrateDeps
fetchPullRequestCalls: { prNumber: number }[]
fetchDiffCalls: { prNumber: number }[]
submitReviewCalls: SubmitReviewParams[]
fetchReviewCommentsCalls: { prNumber: number }[]
upsertSummaryCommentCalls: UpsertSummaryCommentParams[]
readConventionsCalls: { conventionsFile: string }[]
readChangedFilesCalls: ReadChangedFilesParams[]
findRelatedFilesCalls: FindRelatedFilesParams[]
Expand All @@ -201,6 +212,8 @@ const makeOrchestrateDeps = (
const fetchPullRequestCalls: { prNumber: number }[] = []
const fetchDiffCalls: { prNumber: number }[] = []
const submitReviewCalls: SubmitReviewParams[] = []
const fetchReviewCommentsCalls: { prNumber: number }[] = []
const upsertSummaryCommentCalls: UpsertSummaryCommentParams[] = []
const readConventionsCalls: { conventionsFile: string }[] = []
const readChangedFilesCalls: ReadChangedFilesParams[] = []
const findRelatedFilesCalls: FindRelatedFilesParams[] = []
Expand Down Expand Up @@ -229,6 +242,17 @@ const makeOrchestrateDeps = (
usedFallbackBody: false,
}
},
fetchReviewComments: async (params) => {
fetchReviewCommentsCalls.push(params)
return []
},
upsertSummaryComment: async (params) => {
upsertSummaryCommentCalls.push(params)
return {
url: "https://github.com/test/comment/1",
created: true,
}
},
...overrides.githubClient,
}

Expand Down Expand Up @@ -267,6 +291,8 @@ const makeOrchestrateDeps = (
fetchPullRequestCalls,
fetchDiffCalls,
submitReviewCalls,
fetchReviewCommentsCalls,
upsertSummaryCommentCalls,
readConventionsCalls,
readChangedFilesCalls,
findRelatedFilesCalls,
Expand Down Expand Up @@ -653,6 +679,177 @@ describe("orchestrate", () => {
expect(stubs.fetchPullRequestCalls).toHaveLength(0)
})
})

describe("cross-run dedup", () => {
it("first run — posts normal review, no summary comment", async () => {
Comment thread
sourcery-ai[bot] marked this conversation as resolved.
const stubs = makeOrchestrateDeps()
const logger = createTestLogger()

const result = await orchestrate(stubs.deps, logger)

expect(result.findingsCount).toBe(expectedSelection.selected.length)
expect(stubs.fetchReviewCommentsCalls).toHaveLength(1)
expect(stubs.submitReviewCalls).toHaveLength(1)
expect(stubs.upsertSummaryCommentCalls).toHaveLength(0)
})
Comment thread
aliasunder marked this conversation as resolved.

it("re-run — filters duplicate findings and posts only new ones", async () => {
const findings = fixtureReviewResponse.findings
const duplicateFinding = findings[0]
if (duplicateFinding === undefined) {
throw new Error("expected at least one fixture finding")
}
const duplicateAnchor = computeAnchorKey(duplicateFinding)

const stubs = makeOrchestrateDeps({
githubClient: {
fetchReviewComments: async () => [
{
path: duplicateFinding.file,
body: `some comment\n\n<!-- umm-actually:${duplicateAnchor} -->`,
},
],
},
})
const logger = createTestLogger()

const result = await orchestrate(stubs.deps, logger)

const newFindingCount = findings.length - 1
expect(result.findingsCount).toBe(newFindingCount)
expect(stubs.submitReviewCalls).toHaveLength(1)
expect(stubs.upsertSummaryCommentCalls).toHaveLength(1)

const summaryCall = first(stubs.upsertSummaryCommentCalls)
expect(summaryCall.anchor).toBe(RERUN_ANCHOR)
expect(summaryCall.body).toBe(
buildRerunSummary({
sha: fixturePrContext.headSha,
newCount: newFindingCount,
totalCount: 1 + newFindingCount,
model: "test/model",
}),
)
})

it("re-run — skips review when all findings are duplicates", async () => {
const findings = fixtureReviewResponse.findings
const existingComments = findings.map((finding) => ({
path: finding.file,
body: `comment\n\n<!-- umm-actually:${computeAnchorKey(finding)} -->`,
}))

const stubs = makeOrchestrateDeps({
githubClient: {
fetchReviewComments: async () => existingComments,
},
})
const logger = createTestLogger()

const result = await orchestrate(stubs.deps, logger)

expect(result.findingsCount).toBe(0)
expect(result.reviewUrl).toBe("")
expect(stubs.submitReviewCalls).toHaveLength(0)
expect(stubs.upsertSummaryCommentCalls).toHaveLength(1)

const summaryCall = first(stubs.upsertSummaryCommentCalls)
expect(summaryCall.body).toBe(
buildRerunSummary({
sha: fixturePrContext.headSha,
newCount: 0,
totalCount: findings.length,
model: "test/model",
}),
)
})

it("re-run — zero findings from LLM posts summary only", async () => {
const stubs = makeOrchestrateDeps({
fixtureResult: {
review: { analysis: "clean", findings: [] },
},
githubClient: {
fetchReviewComments: async () => [
{
path: "src/a.ts",
body: "body\n\n<!-- umm-actually:src/a.ts:correctness:aaaaaaaa -->",
},
],
},
})
const logger = createTestLogger()

const result = await orchestrate(stubs.deps, logger)

expect(result.findingsCount).toBe(0)
expect(stubs.submitReviewCalls).toHaveLength(0)
expect(stubs.upsertSummaryCommentCalls).toHaveLength(1)

const summaryCall = first(stubs.upsertSummaryCommentCalls)
expect(summaryCall.body).toBe(
buildRerunSummary({
sha: fixturePrContext.headSha,
newCount: 0,
totalCount: 1,
model: "test/model",
}),
)
})

it("does not fetch review comments on skip paths", async () => {
const stubs = makeOrchestrateDeps({
githubClient: {
fetchDiff: async () => ({ kind: "too_large" as const }),
},
})
const logger = createTestLogger()

await orchestrate(stubs.deps, logger)

expect(stubs.fetchReviewCommentsCalls).toHaveLength(0)
})

it("continues without throwing when upsertSummaryComment fails", async () => {
const stubs = makeOrchestrateDeps({
githubClient: {
fetchReviewComments: async () => [
{
path: "src/a.ts",
body: "body\n\n<!-- umm-actually:src/a.ts:correctness:aaaaaaaa -->",
},
],
upsertSummaryComment: async () => {
throw new Error("API rate limit")
},
},
})
const logger = createTestLogger()

const result = await orchestrate(stubs.deps, logger)

expect(result.findingsCount).toBe(expectedSelection.selected.length)
expect(result.reviewUrl).toBe("https://github.com/test/review/1")
expect(stubs.submitReviewCalls).toHaveLength(1)
})

it("degrades to first run when fetching review comments fails", async () => {
const stubs = makeOrchestrateDeps({
githubClient: {
fetchReviewComments: async () => {
throw new Error("network error")
},
},
})
const logger = createTestLogger()

const result = await orchestrate(stubs.deps, logger)

expect(result.findingsCount).toBe(expectedSelection.selected.length)
expect(stubs.submitReviewCalls).toHaveLength(1)
expect(stubs.upsertSummaryCommentCalls).toHaveLength(0)
})
})
})

describe("createPromptedGenerateFindings", () => {
Expand Down
Loading
Loading