diff --git a/src/review/unified-comment.ts b/src/review/unified-comment.ts index 328647faad..6fb6ea2b2a 100644 --- a/src/review/unified-comment.ts +++ b/src/review/unified-comment.ts @@ -182,6 +182,11 @@ export interface UnifiedReviewInput { * (default; the host only resolves this when `review.linkedIssueSatisfaction` is on) ⇒ no section is * rendered, byte-identical to today. */ linkedIssueSatisfaction?: { status: "addressed" | "partial" | "unaddressed"; rationale: string }; + /** The review's line-anchored inline findings (only their `category` is read), used ONLY to render a compact + * category-tally line (#2150). Structural shape so an `InlineFinding[]` is assignable without importing it — + * this renderer stays self-contained. Absent/empty (default; the host passes them only when it produced + * categorized inline findings) ⇒ no tally line, byte-identical. Presentation only — never affects the verdict. */ + inlineFindings?: ReadonlyArray<{ category?: UnifiedFindingCategory | undefined }>; } /** One row of the readiness signal table (gittensory side, host-provided; the engine adds Code review). */ @@ -573,6 +578,25 @@ function asAlert(alert: string, inner: string): string { * (it only emits the fields passed in; no guardrail paths / thresholds / rubric). The host applies * its redactor to the result before posting, exactly as the runtime does for the legacy comment. */ +/** The finding-category names (#1958 / #2150) — mirrors the fixed enum in finding-category-classify.ts, inlined + * here to keep this renderer self-contained (no cross-module imports). */ +export type UnifiedFindingCategory = "security" | "correctness" | "performance" | "maintainability" | "tests" | "style"; + +/** Tally CATEGORIZED inline findings by category (#2150). Uncategorized findings are ignored; ordered by count + * desc, then category name asc, so the rendered line is deterministic. Pure — no IO, no gate impact. */ +export function tallyFindingCategories( + findings: ReadonlyArray<{ category?: UnifiedFindingCategory | undefined }>, +): Array<{ category: UnifiedFindingCategory; count: number }> { + const counts = new Map(); + for (const finding of findings) { + if (!finding.category) continue; + counts.set(finding.category, (counts.get(finding.category) ?? 0) + 1); + } + return [...counts.entries()] + .map(([category, count]) => ({ category, count })) + .sort((a, b) => b.count - a.count || a.category.localeCompare(b.category)); +} + export function renderUnifiedReviewComment(input: UnifiedReviewInput, ctx: UnifiedCommentContext = {}): string { const status = deriveUnifiedStatus(input, ctx); const meta = STATUS_META[status]; @@ -613,6 +637,14 @@ export function renderUnifiedReviewComment(input: UnifiedReviewInput, ctx: Unifi blocks.push(`**${heading}**\n${blockersBody}`); } + // Category breakdown (#2150): a compact, deterministic one-liner of the finding mix (e.g. "2 correctness · + // 1 security"). Omitted entirely when no finding carries a category (default) ⇒ byte-identical. Pure tally, no + // AI, no gate impact. + const categoryTally = tallyFindingCategories(input.inlineFindings ?? []); + if (categoryTally.length) { + blocks.push(`**Findings by category:** ${categoryTally.map(({ category, count }) => `${count} ${category}`).join(" · ")}`); + } + // Failing CI checks — list WHICH checks failed and WHY (codecov %/test/lint reason) under the "CI failing" // chip, instead of leaving the chip as the only signal. Only when CI actually failed (failingChecksBlock // guards on ciState === "failed"); public-safe (names + short reasons only). diff --git a/test/unit/finding-category-tally.test.ts b/test/unit/finding-category-tally.test.ts new file mode 100644 index 0000000000..6e60c58867 --- /dev/null +++ b/test/unit/finding-category-tally.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, it } from "vitest"; +import { tallyFindingCategories, renderUnifiedReviewComment, type UnifiedReviewInput } from "../../src/review/unified-comment"; + +const baseInput: UnifiedReviewInput = { + changedFiles: 1, + reviewerCount: 1, + recommendations: ["merge"], + summary: "Looks good.", +}; + +describe("tallyFindingCategories (#2150)", () => { + it("tallies by category, ordered by count desc then category name asc", () => { + expect( + tallyFindingCategories([ + { category: "style" }, + { category: "correctness" }, + { category: "security" }, + { category: "correctness" }, + { category: "security" }, + { category: "correctness" }, + ]), + ).toEqual([ + { category: "correctness", count: 3 }, + { category: "security", count: 2 }, + { category: "style", count: 1 }, + ]); + }); + + it("breaks a count tie alphabetically by category name", () => { + expect(tallyFindingCategories([{ category: "tests" }, { category: "performance" }])).toEqual([ + { category: "performance", count: 1 }, + { category: "tests", count: 1 }, + ]); + }); + + it("ignores uncategorized findings; all-uncategorized ⇒ empty", () => { + expect(tallyFindingCategories([{ category: "security" }, {}, { category: undefined }])).toEqual([ + { category: "security", count: 1 }, + ]); + expect(tallyFindingCategories([{}, { category: undefined }])).toEqual([]); + expect(tallyFindingCategories([])).toEqual([]); + }); +}); + +describe("renderUnifiedReviewComment category breakdown line (#2150)", () => { + it("renders the one-line tally when categorized findings are present", () => { + const out = renderUnifiedReviewComment({ + ...baseInput, + inlineFindings: [{ category: "correctness" }, { category: "correctness" }, { category: "security" }], + }); + expect(out).toContain("**Findings by category:** 2 correctness · 1 security"); + }); + + it("omits the line entirely when no finding is categorized (byte-identical to absent)", () => { + const withEmpty = renderUnifiedReviewComment({ ...baseInput, inlineFindings: [{}, { category: undefined }] }); + const withAbsent = renderUnifiedReviewComment(baseInput); + expect(withEmpty).not.toContain("Findings by category"); + expect(withEmpty).toBe(withAbsent); // byte-identical + }); +});