diff --git a/src/queue/processors.ts b/src/queue/processors.ts index b630b62717..b44982356c 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -334,7 +334,7 @@ import { type ContributorProfile, } from "../signals/engine"; import { isDuplicateClusterWinnerByClaim } from "../signals/duplicate-winner"; -import { buildUnifiedReviewDiff } from "../review/review-diff"; +import { buildUnifiedReviewDiff, totalAddedLineCount } from "../review/review-diff"; import { estimateReviewEffort } from "../review/review-effort"; import { buildUnifiedCommentBody } from "../review/unified-comment-bridge"; import { randomUUID } from "node:crypto"; @@ -6334,6 +6334,8 @@ export async function resolveAutoReviewSkipForPullRequest( deliveryId: string; headSha: string | null | undefined; changedPaths?: readonly string[] | undefined; + addedLineCount?: number | undefined; + changedFileCount?: number | undefined; }, ): Promise<{ skipReason: string | null; reviewManifest: FocusManifest | null }> { if (args.authorBlacklisted || args.isFrozenForManualReview) { @@ -6349,6 +6351,8 @@ export async function resolveAutoReviewSkipForPullRequest( title: args.pr.title, labels: args.pr.labels ?? [], changedPaths: args.changedPaths ?? [], + addedLineCount: args.addedLineCount ?? 0, + changedFileCount: args.changedFileCount ?? 0, baseRef: args.pr.baseRef ?? null, reviewedCommitCount, }); @@ -8143,7 +8147,10 @@ async function maybePublishPrPublicSurface( pr.labels.some((label) => label.toLowerCase() === manualReviewLabel.toLowerCase()); let reviewManifestForAutoReview: FocusManifest | null = null; let autoReviewSkipReason: string | null = null; - const autoReviewChangedPaths = (await getReviewFiles()).map((file) => file.path); + const autoReviewFiles = await getReviewFiles(); + const autoReviewChangedPaths = autoReviewFiles.map((file) => file.path); + const autoReviewAddedLineCount = totalAddedLineCount(autoReviewFiles); + const autoReviewChangedFileCount = autoReviewFiles.length; ({ skipReason: autoReviewSkipReason, reviewManifest: reviewManifestForAutoReview, @@ -8157,6 +8164,8 @@ async function maybePublishPrPublicSurface( deliveryId: webhook.deliveryId, headSha: advisory.headSha ?? null, changedPaths: autoReviewChangedPaths, + addedLineCount: autoReviewAddedLineCount, + changedFileCount: autoReviewChangedFileCount, })); // review.changed_files_summary (#1957) + review.effort_score (#1955): both deterministic, no-AI — resolve // them here, UNCONDITIONALLY, rather than inside the aiReviewWillRun-gated closure below. These sections diff --git a/src/review/review-diff.ts b/src/review/review-diff.ts index e3ebd8d211..02661d3ca2 100644 --- a/src/review/review-diff.ts +++ b/src/review/review-diff.ts @@ -35,6 +35,19 @@ export function addedLineCount(patch: string | undefined): number { return n; } +/** Sum added-line counts across a PR file list — used by auto-review size-cap eligibility (#2065). */ +export function totalAddedLineCount( + files: readonly { patch?: string | null | undefined; payload?: { patch?: unknown } | null | undefined }[], +): number { + let total = 0; + for (const file of files) { + const patch = + file.patch ?? (typeof file.payload?.patch === "string" ? file.payload.patch : undefined); + total += addedLineCount(patch); + } + return total; +} + /** Split a unified patch into hunks (each starting at an `@@` header); any preamble stays as hunk 0. */ function splitHunks(patch: string): string[] { const hunks: string[] = []; diff --git a/src/signals/focus-manifest.ts b/src/signals/focus-manifest.ts index 57e0b444b0..0b4b757832 100644 --- a/src/signals/focus-manifest.ts +++ b/src/signals/focus-manifest.ts @@ -420,6 +420,10 @@ export type AutoReviewConfig = { /** `review.auto_review.skip_docs_only`: when true, PRs whose every changed file classifies as docs skip AI review. * null (default) ⇒ docs PRs reviewed as today. Empty changed-file list ⇒ NOT docs-only (fail-safe eligible). (#2063) */ skipDocsOnly: boolean | null; + /** `review.auto_review.max_added_lines`: skip AI review when total added lines exceed this cap. 0 (default) ⇒ no cap. (#2065) */ + maxAddedLines: number; + /** `review.auto_review.max_files`: skip AI review when changed-file count exceeds this cap. 0 (default) ⇒ no cap. (#2065) */ + maxFiles: number; /** `review.auto_review.base_branches`: base-ref globs whose PRs ARE reviewed; empty/unset ⇒ every base. (#2041) */ baseBranches: string[]; /** `review.auto_review.auto_pause_after_reviewed_commits`: after N published AI reviews on this PR, pause further @@ -433,6 +437,8 @@ export const EMPTY_AUTO_REVIEW_CONFIG: AutoReviewConfig = { ignoreTitleKeywords: [], skipLabels: [], skipDocsOnly: null, + maxAddedLines: 0, + maxFiles: 0, baseBranches: [], autoPauseAfterReviewedCommits: null, }; @@ -804,6 +810,16 @@ function normalizeOptionalNonNegativeInt(value: JsonValue | undefined, field: st return value; } +/** Parse auto-review size caps where 0 means disabled (byte-identical default). (#2065) */ +function normalizeAutoReviewSizeCap(value: JsonValue | undefined, field: string, warnings: string[]): number { + if (value === undefined || value === null) return 0; + if (typeof value !== "number" || !Number.isFinite(value) || !Number.isInteger(value) || value < 0) { + warnings.push(`Manifest field "${field}" must be a non-negative integer; ignoring it.`); + return 0; + } + return value; +} + /** Normalize an optional confidence threshold in [0,1] (#7) — a fractional value (NOT a 0-100 score), so it is * clamped into range WITHOUT rounding. Absent/null ⇒ null (the resolver leaves the gate's 0.93 default in place); * a non-finite/non-number value is ignored with a warning. */ @@ -1778,6 +1794,8 @@ function autoReviewPresent(config: AutoReviewConfig): boolean { config.ignoreTitleKeywords.length > 0 || config.skipLabels.length > 0 || config.skipDocsOnly !== null || + config.maxAddedLines > 0 || + config.maxFiles > 0 || config.baseBranches.length > 0 || config.autoPauseAfterReviewedCommits !== null ); @@ -1797,6 +1815,8 @@ function parseAutoReviewConfig(value: JsonValue | undefined, warnings: string[]) ignoreTitleKeywords: parseAutoReviewTitleKeywords(record.ignore_title_keywords, warnings), skipLabels: parseAutoReviewSkipLabels(record.skip_labels, warnings), skipDocsOnly: normalizeOptionalBoolean(record.skip_docs_only, "review.auto_review.skip_docs_only", warnings), + maxAddedLines: normalizeAutoReviewSizeCap(record.max_added_lines, "review.auto_review.max_added_lines", warnings), + maxFiles: normalizeAutoReviewSizeCap(record.max_files, "review.auto_review.max_files", warnings), baseBranches: parseManifestGlobList(record.base_branches, "review.auto_review.base_branches", warnings), autoPauseAfterReviewedCommits: normalizeOptionalNonNegativeInt( record.auto_pause_after_reviewed_commits, @@ -2164,6 +2184,8 @@ export function reviewConfigToJson(review: FocusManifestReviewConfig): JsonValue if (review.autoReview.ignoreTitleKeywords.length > 0) autoReview.ignore_title_keywords = [...review.autoReview.ignoreTitleKeywords]; if (review.autoReview.skipLabels.length > 0) autoReview.skip_labels = [...review.autoReview.skipLabels]; if (review.autoReview.skipDocsOnly !== null) autoReview.skip_docs_only = review.autoReview.skipDocsOnly; + if (review.autoReview.maxAddedLines > 0) autoReview.max_added_lines = review.autoReview.maxAddedLines; + if (review.autoReview.maxFiles > 0) autoReview.max_files = review.autoReview.maxFiles; if (review.autoReview.baseBranches.length > 0) autoReview.base_branches = [...review.autoReview.baseBranches]; if (review.autoReview.autoPauseAfterReviewedCommits !== null) { autoReview.auto_pause_after_reviewed_commits = review.autoReview.autoPauseAfterReviewedCommits; @@ -2240,6 +2262,8 @@ export type AutoReviewEligibilityInput = { title: string; labels: readonly string[]; changedPaths: readonly string[]; + addedLineCount: number; + changedFileCount: number; baseRef: string | null; reviewedCommitCount: number; }; @@ -2270,6 +2294,12 @@ export function evaluateAutoReviewSkipReason(config: AutoReviewConfig, input: Au return "review skipped (docs only)"; } } + if (config.maxAddedLines > 0 && input.addedLineCount > config.maxAddedLines) { + return "review skipped (too large)"; + } + if (config.maxFiles > 0 && input.changedFileCount > config.maxFiles) { + return "review skipped (too large)"; + } if (config.baseBranches.length > 0) { const baseRef = input.baseRef?.trim() ?? ""; if (!baseRef || !config.baseBranches.some((glob) => matchesManifestPath(baseRef, glob))) { @@ -2292,6 +2322,8 @@ export function resolvePullRequestAutoReviewSkipReason(args: { title: string; labels?: readonly string[] | undefined; changedPaths?: readonly string[] | undefined; + addedLineCount?: number | undefined; + changedFileCount?: number | undefined; baseRef: string | null; reviewedCommitCount?: number | undefined; }): string | null { @@ -2302,6 +2334,8 @@ export function resolvePullRequestAutoReviewSkipReason(args: { title: args.title, labels: args.labels ?? [], changedPaths: args.changedPaths ?? [], + addedLineCount: args.addedLineCount ?? 0, + changedFileCount: args.changedFileCount ?? 0, baseRef: args.baseRef, reviewedCommitCount: args.reviewedCommitCount ?? 0, }); diff --git a/test/unit/auto-review-config-matrix.test.ts b/test/unit/auto-review-config-matrix.test.ts index 54faa72f43..5166491c88 100644 --- a/test/unit/auto-review-config-matrix.test.ts +++ b/test/unit/auto-review-config-matrix.test.ts @@ -24,6 +24,8 @@ describe("review.auto_review parse ↔ reviewConfigToJson round-trip (#2071)", ( { name: "ignore_title_keywords", autoReview: { ignore_title_keywords: ["WIP", "draft"] } }, { name: "skip_docs_only: true", autoReview: { skip_docs_only: true } }, { name: "skip_docs_only: false", autoReview: { skip_docs_only: false } }, + { name: "max_added_lines", autoReview: { max_added_lines: 500 } }, + { name: "max_files", autoReview: { max_files: 25 } }, { name: "base_branches", autoReview: { base_branches: ["main", "release/**"] } }, { name: "auto_pause_after_reviewed_commits", autoReview: { auto_pause_after_reviewed_commits: 3 } }, { @@ -82,6 +84,8 @@ describe("evaluateAutoReviewSkipReason predicate precedence (#2071)", () => { title: "WIP: bump deps", labels: [], changedPaths: [], + addedLineCount: 0, + changedFileCount: 0, baseRef: "develop", reviewedCommitCount: 5, }; @@ -125,6 +129,12 @@ describe("evaluateAutoReviewSkipReason predicate precedence (#2071)", () => { input: { ...allTriggers, isDraft: false, author: "alice", title: "docs: readme", changedPaths: ["README.md"] }, reason: "review skipped (docs only)", }, + { + name: "too large when added-line cap exceeded and earlier filters are off", + config: { ...allConfigured, skipDrafts: false, ignoreAuthors: [], ignoreTitleKeywords: [], skipDocsOnly: false, maxAddedLines: 10 }, + input: { ...allTriggers, isDraft: false, author: "alice", title: "feat", baseRef: "main", addedLineCount: 11 }, + reason: "review skipped (too large)", + }, { name: "base branch when earlier filters are off", config: { ...allConfigured, skipDrafts: false, ignoreAuthors: [], ignoreTitleKeywords: [] }, @@ -149,6 +159,8 @@ describe("evaluateAutoReviewSkipReason predicate precedence (#2071)", () => { title: "feat: add widget", labels: [], changedPaths: [], + addedLineCount: 0, + changedFileCount: 0, baseRef: "main", reviewedCommitCount: 0, }, diff --git a/test/unit/auto-review-wiring.test.ts b/test/unit/auto-review-wiring.test.ts index 39c16736bd..621e06cde1 100644 --- a/test/unit/auto-review-wiring.test.ts +++ b/test/unit/auto-review-wiring.test.ts @@ -100,6 +100,65 @@ describe("review.auto_review wiring (#1954)", () => { ).toBeNull(); }); + it("resolvePullRequestAutoReviewSkipReason: skips oversized PRs when size caps are configured", () => { + const linesManifest = parseFocusManifest({ review: { auto_review: { max_added_lines: 10 } } }); + expect( + resolvePullRequestAutoReviewSkipReason({ + manifest: linesManifest, + isDraft: false, + author: "alice", + title: "feat: big change", + addedLineCount: 11, + changedFileCount: 1, + baseRef: "main", + }), + ).toBe("review skipped (too large)"); + expect( + resolvePullRequestAutoReviewSkipReason({ + manifest: linesManifest, + isDraft: false, + author: "alice", + title: "feat: big change", + addedLineCount: 10, + changedFileCount: 1, + baseRef: "main", + }), + ).toBeNull(); + + const filesManifest = parseFocusManifest({ review: { auto_review: { max_files: 2 } } }); + expect( + resolvePullRequestAutoReviewSkipReason({ + manifest: filesManifest, + isDraft: false, + author: "alice", + title: "feat: wide change", + addedLineCount: 1, + changedFileCount: 3, + baseRef: "main", + }), + ).toBe("review skipped (too large)"); + expect( + resolvePullRequestAutoReviewSkipReason({ + manifest: filesManifest, + isDraft: false, + author: "alice", + title: "feat: wide change", + addedLineCount: 1, + changedFileCount: 2, + baseRef: "main", + }), + ).toBeNull(); + expect( + resolvePullRequestAutoReviewSkipReason({ + manifest: filesManifest, + isDraft: false, + author: "alice", + title: "feat: wide change", + baseRef: "main", + }), + ).toBeNull(); + }); + it("resolvePullRequestAutoReviewSkipReason: matches the documented *[bot] author glob", () => { const manifest = parseFocusManifest({ review: { auto_review: { ignore_authors: ["*[bot]"] } } }); expect( @@ -221,6 +280,23 @@ describe("review.auto_review wiring (#1954)", () => { }), ).resolves.toEqual({ skipReason: "review skipped (docs only)", reviewManifest: docsManifest }); + const sizeManifest = parseFocusManifest({ review: { auto_review: { max_added_lines: 1 } } }); + loadSpy.mockResolvedValueOnce(sizeManifest); + await expect( + resolveAutoReviewSkipForPullRequest({} as Env, { + authorBlacklisted: false, + isFrozenForManualReview: false, + repoFullName: "acme/widgets", + pr: { number: 9, title: "feat", baseRef: "main", isDraft: false, labels: [] }, + author: "alice", + deliveryId: "d9", + headSha: "sha9", + changedPaths: ["src/a.ts"], + addedLineCount: 2, + changedFileCount: 1, + }), + ).resolves.toEqual({ skipReason: "review skipped (too large)", reviewManifest: sizeManifest }); + loadSpy.mockRejectedValueOnce(new Error("manifest unavailable")); await expect( resolveAutoReviewSkipForPullRequest({} as Env, { diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index 928daadc46..7aab87f9e4 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -3091,6 +3091,8 @@ describe("review.auto_review (#1954 / #2038–#2041)", () => { ignoreTitleKeywords: ["WIP", "draft"], skipLabels: ["do-not-review", "wip"], skipDocsOnly: null, + maxAddedLines: 0, + maxFiles: 0, baseBranches: ["main", "release/**"], autoPauseAfterReviewedCommits: null, }); @@ -3122,7 +3124,7 @@ describe("review.auto_review (#1954 / #2038–#2041)", () => { it("evaluateAutoReviewSkipReason: byte-identical when unset; skips with deterministic reasons when configured", () => { const empty = { ...EMPTY_AUTO_REVIEW_CONFIG }; - const input = { isDraft: true, author: "dependabot[bot]", title: "WIP: bump deps", labels: [] as string[], changedPaths: [] as string[], baseRef: "develop", reviewedCommitCount: 0 }; + const input = { isDraft: true, author: "dependabot[bot]", title: "WIP: bump deps", labels: [] as string[], changedPaths: [] as string[], addedLineCount: 0, changedFileCount: 0, baseRef: "develop", reviewedCommitCount: 0 }; expect(evaluateAutoReviewSkipReason(empty, input)).toBeNull(); expect(evaluateAutoReviewSkipReason({ ...empty, skipDrafts: true }, { ...input, isDraft: true })).toBe("review skipped (draft)"); expect(evaluateAutoReviewSkipReason({ ...empty, skipDrafts: true }, { ...input, isDraft: false })).toBeNull(); @@ -3142,6 +3144,12 @@ describe("review.auto_review (#1954 / #2038–#2041)", () => { expect(evaluateAutoReviewSkipReason({ ...empty, skipDocsOnly: true }, { ...input, changedPaths: ["README.md", "src/a.ts"] })).toBeNull(); expect(evaluateAutoReviewSkipReason({ ...empty, skipDocsOnly: true }, { ...input, changedPaths: [] })).toBeNull(); expect(evaluateAutoReviewSkipReason({ ...empty, skipDocsOnly: false }, { ...input, changedPaths: ["README.md"] })).toBeNull(); + expect(evaluateAutoReviewSkipReason({ ...empty, maxAddedLines: 10 }, { ...input, addedLineCount: 11 })).toBe("review skipped (too large)"); + expect(evaluateAutoReviewSkipReason({ ...empty, maxAddedLines: 10 }, { ...input, addedLineCount: 10 })).toBeNull(); + expect(evaluateAutoReviewSkipReason({ ...empty, maxAddedLines: 0 }, { ...input, addedLineCount: 999 })).toBeNull(); + expect(evaluateAutoReviewSkipReason({ ...empty, maxFiles: 3 }, { ...input, changedFileCount: 4 })).toBe("review skipped (too large)"); + expect(evaluateAutoReviewSkipReason({ ...empty, maxFiles: 3 }, { ...input, changedFileCount: 3 })).toBeNull(); + expect(evaluateAutoReviewSkipReason({ ...empty, maxFiles: 0 }, { ...input, changedFileCount: 99 })).toBeNull(); expect(evaluateAutoReviewSkipReason({ ...empty, baseBranches: ["main"] }, { ...input, baseRef: "develop" })).toBe( "review skipped (base branch out of scope)", ); @@ -3194,10 +3202,27 @@ describe("review.auto_review (#1954 / #2038–#2041)", () => { expect(reviewConfigToJson(labelsOnly.review)).toEqual({ auto_review: { skip_labels: ["do-not-review"] } }); const docsOnly = parseFocusManifest({ review: { auto_review: { skip_docs_only: true } } }); expect(reviewConfigToJson(docsOnly.review)).toEqual({ auto_review: { skip_docs_only: true } }); + const linesCap = parseFocusManifest({ review: { auto_review: { max_added_lines: 500 } } }); + expect(reviewConfigToJson(linesCap.review)).toEqual({ auto_review: { max_added_lines: 500 } }); + const filesCap = parseFocusManifest({ review: { auto_review: { max_files: 25 } } }); + expect(reviewConfigToJson(filesCap.review)).toEqual({ auto_review: { max_files: 25 } }); const basesOnly = parseFocusManifest({ review: { auto_review: { base_branches: ["main"] } } }); expect(reviewConfigToJson(basesOnly.review)).toEqual({ auto_review: { base_branches: ["main"] } }); }); + it("warns on invalid max_added_lines and max_files values", () => { + const badLines = parseFocusManifest({ review: { auto_review: { max_added_lines: -1 } } }); + expect(badLines.review.autoReview.maxAddedLines).toBe(0); + expect(badLines.warnings.some((w) => /max_added_lines.*non-negative integer/.test(w))).toBe(true); + const badFiles = parseFocusManifest({ review: { auto_review: { max_files: "many" } } }); + expect(badFiles.review.autoReview.maxFiles).toBe(0); + expect(badFiles.warnings.some((w) => /max_files.*non-negative integer/.test(w))).toBe(true); + const explicitZero = parseFocusManifest({ review: { auto_review: { max_added_lines: 0, max_files: 0 } } }); + expect(explicitZero.review.autoReview.maxAddedLines).toBe(0); + expect(explicitZero.review.autoReview.maxFiles).toBe(0); + expect(reviewConfigToJson(explicitZero.review)).toBeNull(); + }); + it("warns on invalid skip_docs_only values and round-trips explicit false", () => { const bad = parseFocusManifest({ review: { auto_review: { skip_docs_only: "yes" } } }); expect(bad.review.autoReview.skipDocsOnly).toBeNull(); diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 688ec6740c..43dc6979ba 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -3339,6 +3339,54 @@ describe("queue processors", () => { expect(audit?.detail).toBe("review skipped (docs only)"); }); + it("skips AI review when review.auto_review.max_added_lines is exceeded (#2065)", async () => { + let aiCalls = 0; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => { aiCalls += 1; return { response: JSON.stringify({ assessment: "Looks fine.", blockers: [], nits: [], suggestions: [] }) }; } } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await seedRegateChurnRepo(env); + await upsertRepoFocusManifest(env, "JSONbored/gittensory", { review: { auto_review: { max_added_lines: 1 } } }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { + number: 80, + title: "feat: large change", + state: "open", + draft: false, + user: { login: "contributor" }, + head: { sha: "a80" }, + labels: [], + body: "Closes #1", + } as never); + await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 80, status: "complete", reviewsSyncedAt: new Date().toISOString() }); + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + if (url.includes("/access_tokens")) return Response.json({ token: "fake-installation-token" }); + if (url.includes("/pulls/80/files")) return Response.json([ + { filename: "src/a.ts", status: "modified", additions: 2, deletions: 0, changes: 2, patch: "@@\n+a\n+b" }, + ]); + if (url.endsWith("/pulls/80")) return Response.json({ number: 80, title: "feat: large change", state: "open", draft: false, user: { login: "contributor" }, head: { sha: "a80" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/commits/a80/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/a80/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/80/comments")) return method === "POST" ? Response.json({ id: 80 }, { status: 201 }) : Response.json([]); + if (url.includes("/issues/1")) return Response.json({ number: 1, title: "Issue", state: "open", labels: [], user: { login: "reporter" } }); + if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } }); + return Response.json({}); + }); + + await expect( + processJob(env, { type: "agent-regate-pr", deliveryId: "auto-review-skip-too-large", repoFullName: "JSONbored/gittensory", prNumber: 80, installationId: 123 }), + ).resolves.toBeUndefined(); + expect(aiCalls).toBe(0); + const audit = await env.DB.prepare("select detail from audit_events where event_type = ? and target_key = ?") + .bind("github_app.ai_review_auto_review_skipped", "JSONbored/gittensory#80") + .first<{ detail: string }>(); + expect(audit?.detail).toBe("review skipped (too large)"); + }); + it("runs AI review with cached manifest when auto_review eligibility passes (#1954)", async () => { let aiCalls = 0; const env = createTestEnv({ diff --git a/test/unit/review-diff.test.ts b/test/unit/review-diff.test.ts index 31b35b8746..7acd142b49 100644 --- a/test/unit/review-diff.test.ts +++ b/test/unit/review-diff.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { addedLineCount, buildUnifiedReviewDiff, diffFilePriority, keepHighSignalHunks } from "../../src/review/review-diff"; +import { addedLineCount, buildUnifiedReviewDiff, diffFilePriority, keepHighSignalHunks, totalAddedLineCount } from "../../src/review/review-diff"; describe("diffFilePriority — source survives, noise drops first", () => { it("ranks source(0) < tests(1) < docs(2) < lockfiles/generated(4)", () => { @@ -57,6 +57,20 @@ describe("addedLineCount — counts +lines, ignores +++ header", () => { }); }); +describe("totalAddedLineCount — sums added lines across PR files (#2065)", () => { + it("aggregates patch counts and treats missing patches as zero", () => { + expect(totalAddedLineCount([ + { patch: "@@\n+a\n+b" }, + { patch: "@@\n+c" }, + { patch: null }, + { payload: { patch: "@@\n+d" } }, + { payload: {} }, + {}, + ])).toBe(4); + expect(totalAddedLineCount([])).toBe(0); + }); +}); + describe("buildUnifiedReviewDiff — the #1528 fix: never silently drop the file defining a symbol", () => { it("orders SOURCE before a lockfile, so under a tight budget source survives and the lockfile drops", () => { const bigLock = `@@\n${"+x\n".repeat(400)}`; // large, low-priority diff --git a/test/unit/signals-coverage.test.ts b/test/unit/signals-coverage.test.ts index b3efb40960..7d7eaa879c 100644 --- a/test/unit/signals-coverage.test.ts +++ b/test/unit/signals-coverage.test.ts @@ -1127,7 +1127,7 @@ describe("signal coverage edge cases", () => { collisions: buildCollisionReport(directRepo.fullName, [], [currentPr]), preflight: buildPreflightResult({ repoFullName: directRepo.fullName, title: "Fix isolated issue", body: "Fixes #99", linkedIssues: [99] }, directRepo, [], [currentPr]), settings: gateSettings, - review: { present: true, footerText: "Reviewed by the Acme maintainer bot.", note: "Run npm test before pushing.", fields: { relatedWork: false }, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, effortScore: null, findingCategories: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { skipDrafts: null, ignoreAuthors: [], ignoreTitleKeywords: [], skipLabels: [], skipDocsOnly: null, baseBranches: [], autoPauseAfterReviewedCommits: null }, labelingRules: [], aiModel: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null }, visual: { preview: { urlTemplate: null }, routes: { paths: [], maxRoutes: null }, themes: [] }, linkedIssueSatisfaction: null }, + review: { present: true, footerText: "Reviewed by the Acme maintainer bot.", note: "Run npm test before pushing.", fields: { relatedWork: false }, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, effortScore: null, findingCategories: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { skipDrafts: null, ignoreAuthors: [], ignoreTitleKeywords: [], skipLabels: [], skipDocsOnly: null, maxAddedLines: 0, maxFiles: 0, baseBranches: [], autoPauseAfterReviewedCommits: null }, labelingRules: [], aiModel: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null }, visual: { preview: { urlTemplate: null }, routes: { paths: [], maxRoutes: null }, themes: [] }, linkedIssueSatisfaction: null }, aiReview: { notes: "The change is focused.\n\n**Nits (2)**\n- Add a test for the edge case.\n- Keep the validator helper scoped." }, }); expect(customizedComment).toContain("Reviewed by the Acme maintainer bot."); // custom footer lead