From 2fa0aa028db99998bfa9ee56e4c0c5cfb667ddfd Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sun, 5 Jul 2026 13:09:36 -0700 Subject: [PATCH] feat(review): tag AI review findings with a category taxonomy (#1958) Adds a category field (security/correctness/performance/ maintainability/tests/style) to each inline finding: the model self-categorizes when asked, and a deterministic path/keyword classifier (classifyFindingCategory) supplies a safe default for whatever it omits, so a caller with the feature on never sees a partial tag. Surfaced in the inline-comment label and a new "Finding categories" collapsible in the unified comment. Gated by review.finding_categories in .gittensory.yml (default off, layered on review.inline_comments like review.suggestions) -- no new global env var, no DB/OpenAPI surface. --- README.md | 2 +- src/queue/processors.ts | 32 ++++- src/review/finding-category-classify.ts | 42 ++++++ src/review/inline-comments.ts | 39 ++++-- src/review/unified-comment-bridge.ts | 56 +++++++- src/services/ai-review.ts | 33 ++++- src/signals/focus-manifest.ts | 23 +++- test/unit/ai-review.test.ts | 60 +++++++++ test/unit/finding-category-classify.test.ts | 72 ++++++++++ .../unit/finding-category-collapsible.test.ts | 125 ++++++++++++++++++ test/unit/focus-manifest.test.ts | 29 +++- test/unit/inline-comments.test.ts | 73 +++++++++- test/unit/queue.test.ts | 89 +++++++++++++ test/unit/signals-coverage.test.ts | 2 +- 14 files changed, 647 insertions(+), 30 deletions(-) create mode 100644 src/review/finding-category-classify.ts create mode 100644 test/unit/finding-category-classify.test.ts create mode 100644 test/unit/finding-category-collapsible.test.ts diff --git a/README.md b/README.md index cb56e9ac86..6ce5300725 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,7 @@ Gittensory CI and gittensory review score, gate, and comment on pull requests. T - **`Gittensory Orb Review Agent`** (`gate.*` / `settings.gateCheckMode` / `settings.reviewCheckMode`, off by default) — the authoritative GitHub Check Run carrying the gate's pass/fail verdict. This is the one worth making a required status check. - **`Gittensory Context`** (`settings.checkRunMode` / `settings.checkRunDetailLevel`, off by default) — a separate, purely advisory Check Run. At its default `checkRunDetailLevel: minimal` it publishes no findings at all; even at `standard`/`deep` it only re-renders content already shown elsewhere. Never make this one required. -- **Inline review comments** (`GITTENSORY_REVIEW_INLINE_COMMENTS` + `.gittensory.yml`'s `review.inline_comments`, off by both by default) — real, reply-able line-anchored PR review comment threads (CodeRabbit-style). This is the ONLY one of the three that posts an interactive per-line thread; the two check runs above never do. With `.gittensory.yml`'s `review.suggestions` also on, a precise line-anchored fix is additionally rendered as a one-click, committable GitHub suggested-change block. +- **Inline review comments** (`GITTENSORY_REVIEW_INLINE_COMMENTS` + `.gittensory.yml`'s `review.inline_comments`, off by both by default) — real, reply-able line-anchored PR review comment threads (CodeRabbit-style). This is the ONLY one of the three that posts an interactive per-line thread; the two check runs above never do. With `.gittensory.yml`'s `review.suggestions` also on, a precise line-anchored fix is additionally rendered as a one-click, committable GitHub suggested-change block. With `review.finding_categories` also on (off by default), each finding is additionally tagged with a category — security/correctness/performance/maintainability/tests/style — in both the inline comment label and the unified comment's "Finding categories" collapsible; a deterministic path/keyword fallback covers whatever the model omits. See [Tuning your reviews](https://gittensory.aethereal.dev/docs/tuning) for the full flag, setting, and `.gittensory.yml` reference. diff --git a/src/queue/processors.ts b/src/queue/processors.ts index c56688becd..774a995475 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -390,6 +390,7 @@ import { } from "../services/ai-review"; import { maybePostInlineComments, + shouldRenderFindingCategories, shouldRenderSuggestions, shouldRequestInlineFindings, } from "../review/inline-comments"; @@ -6426,6 +6427,10 @@ export async function runAiReviewForAdvisory( // (the per-repo toggle). ANDed here with the operator flag + cutover allowlist to decide whether to ASK the // model for line-anchored inline findings. Absent/false ⇒ the reviewer prompt is byte-identical (no findings). reviewInlineComments?: boolean | undefined; + // `.gittensory.yml` review.finding_categories (#1958), resolved by the caller from the cached manifest. ANDed + // here with reviewInlineComments (a category has nothing to categorize without an inline finding) to decide + // whether to ASK the model to self-categorize each inlineFindings item. Absent/false ⇒ byte-identical prompt. + reviewFindingCategories?: boolean | undefined; // `.gittensory.yml` review.ai_model (#selfhost-ai-model-override), resolved by the caller from the cached // manifest. Self-host only — overrides that repo's claude-code/codex model+effort, taking priority over the // operator's global env vars. Absent/all-null ⇒ byte-identical (global env var, then provider default). @@ -6654,6 +6659,13 @@ export async function runAiReviewForAdvisory( diff: enrichmentDiff, }) : undefined; + // Resolved once and reused for BOTH inlineFindings itself and the finding-categories opt-in layered on top + // of it (#1958) — a category has nothing to categorize without an inline finding to attach it to. + const inlineFindingsRequested = shouldRequestInlineFindings( + env, + args.repoFullName, + args.reviewInlineComments, + ); const result = await runGittensoryAiReview(env, { repoFullName: args.repoFullName, prNumber: args.pr.number, @@ -6686,11 +6698,10 @@ export async function runAiReviewForAdvisory( codexEffort: args.reviewSelfHostAiModel?.codexEffort ?? null, // Inline comments (#inline-comments): ask the model for line-anchored findings only when the operator flag, // the cutover allowlist, AND the per-repo manifest toggle all pass. Otherwise the prompt is byte-identical. - inlineFindings: shouldRequestInlineFindings( - env, - args.repoFullName, - args.reviewInlineComments, - ), + inlineFindings: inlineFindingsRequested, + // review.finding_categories (#1958): ask the model to ALSO self-categorize each inlineFindings item, only + // when inline findings themselves are being requested (a category has nothing to categorize otherwise). + findingCategories: shouldRenderFindingCategories(inlineFindingsRequested, args.reviewFindingCategories), pathGuidance: resolveReviewPathInstructions( args.reviewPathInstructions ?? [], files.map((file) => file.path), @@ -7640,6 +7651,7 @@ async function maybePublishPrPublicSurface( let inlineCommentsEnabledForReview = false; let suggestionsEnabledForReview = false; let changedFilesSummaryEnabledForReview = false; + let findingCategoriesEnabledForReview = false; let aiReviewExpected = false; let aiReviewWasReused = false; let gateFinalized = false; @@ -8247,6 +8259,7 @@ async function maybePublishPrPublicSurface( securityFocus: reviewSecurityFocus, inlineComments: reviewInlineComments, suggestions: reviewSuggestions, + findingCategories: reviewFindingCategories, pathInstructions: reviewPathInstructions, instructions: manifestReviewInstructions, tone: reviewTone, @@ -8263,6 +8276,10 @@ async function maybePublishPrPublicSurface( inlineCommentsEnabledForReview, reviewSuggestions, ); + findingCategoriesEnabledForReview = shouldRenderFindingCategories( + inlineCommentsEnabledForReview, + reviewFindingCategories, + ); const reviewFilesForAi = await getReviewFiles(); const changedPaths = reviewFilesForAi.map((file) => file.path); // Per-repo review CONTEXT (#review-skills): fold the container-private review/AGENTS.md (or legacy @@ -8453,6 +8470,7 @@ async function maybePublishPrPublicSurface( reviewExcludePaths, reviewPathFilters, reviewInlineComments, + reviewFindingCategories, reviewSelfHostAiModel, deliveryId: webhook.deliveryId, }); @@ -9311,6 +9329,9 @@ async function maybePublishPrPublicSurface( })), } : {}), + ...(findingCategoriesEnabledForReview && aiReview?.inlineFindings?.length + ? { findingCategories: aiReview.inlineFindings } + : {}), }); } else { deterministicBody = buildPublicPrIntelligenceComment(commentArgs); @@ -9366,6 +9387,7 @@ async function maybePublishPrPublicSurface( mode, inlineCommentsEnabled: inlineCommentsEnabledForReview, suggestionsEnabled: suggestionsEnabledForReview, + categoriesEnabled: findingCategoriesEnabledForReview, }); } if (decision.willLabel) { diff --git a/src/review/finding-category-classify.ts b/src/review/finding-category-classify.ts new file mode 100644 index 0000000000..d13b649ad0 --- /dev/null +++ b/src/review/finding-category-classify.ts @@ -0,0 +1,42 @@ +import { isTestFile } from "../signals/local-branch"; +import { isTestPath } from "../signals/test-evidence"; + +// Deterministic category taxonomy for AI review findings (#1958). The model is asked to self-categorize each +// inlineFinding when review.finding_categories is on; `classifyFindingCategory` supplies the SAFE DEFAULT for +// whatever it omits or mis-emits, so a caller with the feature on always has a category to render — never a +// sometimes-present field. Pure, path/keyword-only — no diff content, no IO. + +export const FINDING_CATEGORIES = ["security", "correctness", "performance", "maintainability", "tests", "style"] as const; + +export type FindingCategory = (typeof FINDING_CATEGORIES)[number]; + +/** Type guard for a model-provided `category` value — anything outside the fixed enum (wrong case, a made-up + * category, a non-string) is rejected so the caller falls back to {@link classifyFindingCategory}. */ +export function isFindingCategory(value: unknown): value is FindingCategory { + return typeof value === "string" && (FINDING_CATEGORIES as readonly string[]).includes(value); +} + +const SECURITY_KEYWORDS = + /\b(?:sql injection|xss|cross-site scripting|csrf|authentication|authorization|secret|credential|vulnerab\w*|sanitiz\w*|command injection|path traversal|ssrf|deserializ\w*|hardcoded (?:password|key|token)|insecure)\b/i; +const PERFORMANCE_KEYWORDS = + /\b(?:performance|\bslow\b|n\+1|memory leak|inefficient|redundant (?:call|fetch|query)|unnecessary re-?render|blocking call|latency|throughput)\b/i; +const TEST_KEYWORDS = /\b(?:test coverage|missing test|flaky test|test case|assertion)\b/i; +const STYLE_KEYWORDS = /\b(?:naming|formatting|whitespace|indentation|lint\w*|style guide|typo)\b/i; +const MAINTAINABILITY_KEYWORDS = + /\b(?:duplicat\w*|refactor\w*|readability|overly complex|magic number|dead code|unused (?:variable|import|function))\b/i; + +/** + * Deterministic fallback categorization (#1958): PATH first (a finding anchored to a test file is a "tests" + * finding regardless of wording), then keyword sniffing over the finding's own body text, ordered so the + * costliest miscategorization (missing a real security defect) is checked first. Falls through to + * "correctness" — the general "this is a bug" bucket — when nothing else matches. Pure. + */ +export function classifyFindingCategory(finding: { path: string; body: string }): FindingCategory { + if (isTestPath(finding.path) || isTestFile(finding.path)) return "tests"; + if (SECURITY_KEYWORDS.test(finding.body)) return "security"; + if (PERFORMANCE_KEYWORDS.test(finding.body)) return "performance"; + if (TEST_KEYWORDS.test(finding.body)) return "tests"; + if (STYLE_KEYWORDS.test(finding.body)) return "style"; + if (MAINTAINABILITY_KEYWORDS.test(finding.body)) return "maintainability"; + return "correctness"; +} diff --git a/src/review/inline-comments.ts b/src/review/inline-comments.ts index 48e8df4dd7..04d8f8d062 100644 --- a/src/review/inline-comments.ts +++ b/src/review/inline-comments.ts @@ -10,6 +10,7 @@ import { createPullRequestReviewComments } from "../github/pr-actions"; import { isConvergenceRepoAllowed } from "./cutover-gate"; +import { classifyFindingCategory } from "./finding-category-classify"; import type { InlineFinding } from "../services/ai-review"; import type { AgentActionMode } from "../settings/agent-execution"; import type { PullRequestFileRecord } from "../types"; @@ -45,6 +46,16 @@ export function shouldRenderSuggestions( return inlineCommentsEnabled && manifestToggle === true; } +/** PURE (#1958): should an inline finding's `category` be rendered? An ADDITIONAL opt-in (`review.finding_categories`) + * layered on top of inline comments being enabled at all — mirrors {@link shouldRenderSuggestions} exactly, since + * a category has nothing to categorize without the inline comment it rides on. */ +export function shouldRenderFindingCategories( + inlineCommentsEnabled: boolean, + manifestToggle: boolean | undefined, +): boolean { + return inlineCommentsEnabled && manifestToggle === true; +} + /** A GitHub inline review comment anchored to a line on the RIGHT (added/context) side of the PR diff. */ export type ReviewInlineComment = { path: string; line: number; side: "RIGHT"; body: string }; @@ -88,14 +99,18 @@ function safeSuggestionBlock(suggestion: string | undefined): string { return `\n\n\`\`\`suggestion\n${suggestion}\n\`\`\``; } -/** The inline comment body: a compact severity label + the finding, plus a one-click GitHub suggested-change - * block when the finding carries a `suggestion` AND the caller has suggestions enabled (#1956). Public-safe by - * construction — both the body and the suggestion were already run through the public-safe filter by - * composeInlineFindings before they reached here. */ -function formatInlineBody(finding: InlineFinding, suggestionsEnabled: boolean): string { +/** The inline comment body: a compact severity (+ optional category) label + the finding, plus a one-click GitHub + * suggested-change block when the finding carries a `suggestion` AND the caller has suggestions enabled (#1956). + * When `categoriesEnabled` (#1958), the label gets a parenthetical category tag — the model's own `category` when + * it emitted one in the fixed enum, else the deterministic fallback (`classifyFindingCategory`), so the tag is + * never sometimes-present. Public-safe by construction — both the body and the suggestion were already run + * through the public-safe filter by composeInlineFindings before they reached here; `category` is a fixed enum + * literal, never free text. */ +function formatInlineBody(finding: InlineFinding, suggestionsEnabled: boolean, categoriesEnabled = false): string { const label = finding.severity === "blocker" ? "Blocker" : "Nit"; + const categoryTag = categoriesEnabled ? ` (${finding.category ?? classifyFindingCategory(finding)})` : ""; const suggestionBlock = suggestionsEnabled ? safeSuggestionBlock(finding.suggestion) : ""; - return `**${label}:** ${finding.body}${suggestionBlock}`; + return `**${label}${categoryTag}:** ${finding.body}${suggestionBlock}`; } /** PURE: turn the model's line-anchored findings into GitHub inline review comments, dropping any whose @@ -103,8 +118,9 @@ function formatInlineBody(finding: InlineFinding, suggestionsEnabled: boolean): * no usable patch. Dedupes by path+line (first wins) and caps the total. Empty in / nothing anchorable ⇒ []. * `suggestionsEnabled` (#1956) gates whether a finding's `suggestion` is rendered as a committable GitHub * suggested-change block — a suggestion is anchored to the SAME single line as its parent finding, so the - * existing line-validity check above already covers "drop it if the range can't be anchored". */ -export function selectInlineComments(findings: InlineFinding[], files: Pick[], suggestionsEnabled = false): ReviewInlineComment[] { + * existing line-validity check above already covers "drop it if the range can't be anchored". `categoriesEnabled` + * (#1958) gates whether the label carries a category tag. */ +export function selectInlineComments(findings: InlineFinding[], files: Pick[], suggestionsEnabled = false, categoriesEnabled = false): ReviewInlineComment[] { const rightLinesByPath = new Map>(); for (const file of files) { const patch = typeof file.payload?.patch === "string" ? file.payload.patch : ""; @@ -119,7 +135,7 @@ export function selectInlineComments(findings: InlineFinding[], files: Pick[]; mode: AgentActionMode; suggestionsEnabled?: boolean | undefined; + categoriesEnabled?: boolean | undefined; }, ): Promise<{ posted: number }> { - const comments = selectInlineComments(args.findings, args.files, args.suggestionsEnabled); + const comments = selectInlineComments(args.findings, args.files, args.suggestionsEnabled, args.categoriesEnabled); if (comments.length === 0 || !args.commitId) return { posted: 0 }; try { await createPullRequestReviewComments(env, args.installationId, args.repoFullName, args.pullNumber, args.commitId, comments, args.mode); @@ -170,6 +187,7 @@ export async function maybePostInlineComments( mode: AgentActionMode; inlineCommentsEnabled: boolean; suggestionsEnabled?: boolean | undefined; + categoriesEnabled?: boolean | undefined; }, ): Promise { if (!args.inlineCommentsEnabled) return; @@ -184,5 +202,6 @@ export async function maybePostInlineComments( files: await args.getFiles(), mode: args.mode, suggestionsEnabled: args.suggestionsEnabled, + categoriesEnabled: args.categoriesEnabled, }); } diff --git a/src/review/unified-comment-bridge.ts b/src/review/unified-comment-bridge.ts index 162613de2c..272a041e77 100644 --- a/src/review/unified-comment-bridge.ts +++ b/src/review/unified-comment-bridge.ts @@ -28,6 +28,7 @@ import type { CaptureRoute } from "./visual/capture"; import { PR_PANEL_COMMENT_MARKER } from "../github/comments"; import { GITTENSORY_GATE_CHECK_NAME } from "./check-names"; import { classifyChangedFile, type ReviewFileClass } from "./changed-files-classify"; +import { classifyFindingCategory, FINDING_CATEGORIES, type FindingCategory } from "./finding-category-classify"; import { buildUnifiedReviewInput, renderUnifiedReviewComment, @@ -313,6 +314,12 @@ export type UnifiedCommentBridgeArgs = { * passes this only when the manifest opts in — see `resolveReviewPromptOverrides`'s `changedFilesSummary`). * (#1957) */ changedFilesSummary?: ChangedFileSummaryInput[] | undefined; + /** Line-anchored AI findings, one entry per inline finding (review.finding_categories port). When present + + * non-empty, a "Finding categories" collapsible (a count per security/correctness/performance/maintainability/ + * tests/style category) is appended. A finding missing its own `category` falls back to + * `classifyFindingCategory` — never omitted from the count. Default OFF (the processor passes this only when + * the manifest opts in — see `resolveReviewPromptOverrides`'s `findingCategories`). (#1958) */ + findingCategories?: FindingCategoryInput[] | undefined; /** The disposition holds this PR for owner review because its diff touches a hard-guardrail path — so an * otherwise-ready comment renders "held for review" instead of "safe to merge". (#guarded-hold-comment) */ heldForReview?: boolean | undefined; @@ -417,6 +424,43 @@ export function buildChangedFilesSummaryCollapsible(files: ChangedFileSummaryInp return { title: "Changed files", body }; } +/** A finding's path + body — everything `buildFindingCategoryCollapsible` needs to use the finding's own + * `category` when present, or fall back to `classifyFindingCategory` when it isn't. Deliberately narrower than + * `InlineFinding` (no line/severity/suggestion) so the bridge's pure-rendering surface stays minimal. */ +export type FindingCategoryInput = { path: string; body: string; category?: FindingCategory | undefined }; + +const FINDING_CATEGORY_LABEL: Record = { + security: "Security", + correctness: "Correctness", + performance: "Performance", + maintainability: "Maintainability", + tests: "Tests", + style: "Style", +}; + +/** + * Build the "Finding categories" collapsible: a count per category (security/correctness/performance/ + * maintainability/tests/style) across this review's line-anchored AI findings. A finding missing its own + * `category` (the model omitted it) falls back to the deterministic `classifyFindingCategory` — every finding + * is counted exactly once, never dropped. No AI, no network. Returns null when there are no findings, so the + * caller can unconditionally chain this alongside the other optional collapsibles. + */ +export function buildFindingCategoryCollapsible(findings: FindingCategoryInput[]): UnifiedCollapsible | null { + if (findings.length === 0) return null; + const counts = new Map(); + for (const finding of findings) { + const category = finding.category ?? classifyFindingCategory(finding); + counts.set(category, (counts.get(category) ?? 0) + 1); + } + const rows = FINDING_CATEGORIES.flatMap((category) => { + const count = counts.get(category); + if (!count) return []; + return [`| ${FINDING_CATEGORY_LABEL[category]} | ${count} |`]; + }); + const body = ["| Category | Findings |", "| --- | --- |", ...rows].join("\n"); + return { title: "Finding categories", body }; +} + /** * Build the unified PR-review comment body from gittensory's live data. Returns a string that STARTS with * the panel marker (so the existing upsert updates in place) followed by the rendered unified comment. @@ -479,11 +523,21 @@ export function buildUnifiedCommentBody(args: UnifiedCommentBridgeArgs): string : null; const withChangedFiles = changedFilesCollapsible !== null ? [...(args.extraCollapsibles ?? []), changedFilesCollapsible] : args.extraCollapsibles; + // review.finding_categories port: when the manifest opts in, the processor hands us this review's line-anchored + // AI findings here; append the "Finding categories" collapsible right after Changed files (both are structural + // review-shape summaries, ahead of the visual preview). Flag-OFF (the processor passes undefined) ⇒ + // extraCollapsibles is unchanged. (#1958) + const findingCategoryCollapsible = + args.findingCategories && args.findingCategories.length > 0 + ? buildFindingCategoryCollapsible(args.findingCategories) + : null; + const withFindingCategories = + findingCategoryCollapsible !== null ? [...(withChangedFiles ?? []), findingCategoryCollapsible] : withChangedFiles; // Visual-capture port: when before/after routes are present, append a "Visual preview" collapsible to the // extra sections. Flag-OFF (the processor passes no beforeAfter) ⇒ extraCollapsibles is unchanged. const visualCollapsible = args.beforeAfter && args.beforeAfter.length > 0 ? buildBeforeAfterCollapsible(args.beforeAfter) : null; const extraCollapsibles = - visualCollapsible !== null ? [...(withChangedFiles ?? []), visualCollapsible] : withChangedFiles; + visualCollapsible !== null ? [...(withFindingCategories ?? []), visualCollapsible] : withFindingCategories; const body = renderUnifiedReviewComment(input, { brand: args.brand ?? "Gittensory review", diff --git a/src/services/ai-review.ts b/src/services/ai-review.ts index 6bf9bee237..aa1510c013 100644 --- a/src/services/ai-review.ts +++ b/src/services/ai-review.ts @@ -33,6 +33,7 @@ import { errorMessage } from "../utils/json"; import type { ReviewProfile } from "../signals/focus-manifest"; import { isCodeFile } from "../signals/local-branch"; import { isTestPath } from "../signals/test-evidence"; +import { isFindingCategory, type FindingCategory } from "../review/finding-category-classify"; import type { CombineStrategy, OnMerge } from "../types"; /** @@ -281,6 +282,13 @@ export type GittensoryAiReviewInput = { * (the default) ⇒ no instruction is appended, so the prompt is byte-identical and the model emits none. */ inlineFindings?: boolean | undefined; + /** + * `.gittensory.yml` `review.finding_categories` (#1958) — when true (the caller has already ANDed this with + * `inlineFindings` being requested, since a category has nothing to categorize otherwise), the reviewer is + * additionally asked to tag each `inlineFindings` item with a `category`. Absent/false (the default) ⇒ no + * instruction is appended, so the prompt is byte-identical and the model emits no category. + */ + findingCategories?: boolean | undefined; /** * This PR's changed file paths (#2558) — reused to splice a concise "changed code files with zero * test-path evidence" section into the user prompt via the engine's own deterministic classifier @@ -337,6 +345,11 @@ export type InlineFinding = { severity: "blocker" | "nit"; body: string; suggestion?: string | undefined; + /** `.gittensory.yml` `review.finding_categories` (#1958): the kind of issue (security/correctness/performance/ + * maintainability/tests/style), when the model was asked to self-categorize and emitted a value in the fixed + * enum. Absent when the feature is off (the model was never asked) OR the model's value didn't parse — callers + * that render categories fall back to `classifyFindingCategory` in that case rather than treating it as absent. */ + category?: FindingCategory | undefined; }; export type ModelReview = { @@ -583,6 +596,9 @@ export function parseModelReview(text: string): ModelReview | null { // Fail-safe: a malformed/absent inlineFindings field degrades to []; each item missing a usable path / a // positive line / a body is skipped, never partial. Severity defaults to "nit" unless it's exactly "blocker"; // a bad/blank suggestion is simply dropped while keeping the finding itself. (#2138) + // `category` (#1958) is parsed ONLY when it's one of the fixed enum values — an absent/mis-emitted category + // is left OFF the finding (not defaulted here) so a caller that didn't ask for categories at all sees no + // field, and a caller that DID ask can apply its own deterministic fallback (classifyFindingCategory). const toInlineFindings = (value: unknown): InlineFinding[] => Array.isArray(value) ? value @@ -598,6 +614,7 @@ export function parseModelReview(text: string): ModelReview | null { typeof o.suggestion === "string" ? o.suggestion.trim() : ""; const severity: "blocker" | "nit" = o.severity === "blocker" ? "blocker" : "nit"; + const category = isFindingCategory(o.category) ? o.category : undefined; return path && line > 0 && body ? [ { @@ -606,6 +623,7 @@ export function parseModelReview(text: string): ModelReview | null { severity, body, ...(suggestion ? { suggestion } : {}), + ...(category ? { category } : {}), }, ] : []; @@ -705,6 +723,14 @@ const SECURITY_FOCUS_SUFFIX = const INLINE_FINDINGS_SUFFIX = '\n\nINLINE FINDINGS: ALSO include an additional top-level field "inlineFindings" in the SAME JSON object — an array (possibly empty) of your most important findings, each anchored to a specific changed line, for inline PR comments. Each item: {"path": the changed file path EXACTLY as shown in the diff, "line": the 1-based line number in the NEW file (count forward from the "+" start in the nearest "@@ -old +new @@" hunk header) of an ADDED ("+") line you are commenting on, "severity": "blocker" or "nit", "body": the one-sentence finding, "suggestion": optional replacement text for that line}. Include ONLY findings you can place on a specific added line; OMIT any you cannot anchor precisely (a wrong line is worse than none). If a suggestion is blank or you are not confident in an exact replacement, omit the suggestion field and keep the finding. At most ~10 items.'; +// `.gittensory.yml` review.finding_categories → an appended instruction that ALSO asks for a `category` on each +// inlineFindings item (#1958). Only meaningful once INLINE_FINDINGS_SUFFIX is already appended (a category has +// nothing to categorize otherwise) — the caller ANDs this with inlineFindings before setting the input flag. +// Absent/off appends nothing (byte-identical); a parser-side fallback (classifyFindingCategory) covers whatever +// the model omits or mis-emits, so this suffix only needs to ask, never enforce. +const FINDING_CATEGORY_SUFFIX = + ' Each inlineFindings item must ALSO include "category": one of exactly "security", "correctness", "performance", "maintainability", "tests", "style" — the KIND of issue, not its severity.'; + /** The effective reviewer SYSTEM prompt. Appends the grounding-discipline suffix when the caller supplied one * (flag GITTENSORY_REVIEW_GROUNDING on), the `review.profile` tone suffix when set, the `review.security_focus` * prioritization suffix when on, then the inline-findings instruction when the caller asked for them; all absent @@ -727,7 +753,9 @@ function buildSystemPrompt(input: GittensoryAiReviewInput): string { const repoInstructionsAppend = buildRepoInstructionsSystemAppend(input.repoInstructions); const repoInstructionsSuffix = repoInstructionsAppend ? ` ${repoInstructionsAppend}` : ""; const inlineSuffix = input.inlineFindings ? INLINE_FINDINGS_SUFFIX : ""; - return `${REVIEW_SYSTEM_PROMPT}${groundingSuffix}${enrichmentSuffix}${profileSuffix}${securityFocusSuffix}${pathSuffix}${repoInstructionsSuffix}${inlineSuffix}`; + // review.finding_categories (#1958) only makes sense layered on top of inlineFindings itself being requested. + const categorySuffix = input.inlineFindings && input.findingCategories ? FINDING_CATEGORY_SUFFIX : ""; + return `${REVIEW_SYSTEM_PROMPT}${groundingSuffix}${enrichmentSuffix}${profileSuffix}${securityFocusSuffix}${pathSuffix}${repoInstructionsSuffix}${inlineSuffix}${categorySuffix}`; } function buildRepoInstructionsSystemAppend(repoInstructions: string | null | undefined): string { @@ -1190,6 +1218,9 @@ export function composeInlineFindings(reviews: ModelReview[]): InlineFinding[] { severity: finding.severity, body: safeBody, ...(safeSuggestion ? { suggestion: safeSuggestion } : {}), + // `category` is a fixed enum literal (never free text), so it carries through as-is — no public-safe + // scrubbing needed, unlike body/suggestion. + ...(finding.category ? { category: finding.category } : {}), }); } return out; diff --git a/src/signals/focus-manifest.ts b/src/signals/focus-manifest.ts index e9b86655d2..bcdef38e95 100644 --- a/src/signals/focus-manifest.ts +++ b/src/signals/focus-manifest.ts @@ -328,6 +328,13 @@ export type FocusManifestReviewConfig = { * existing `classifyChangedFile` classifier (`src/review/changed-files-classify.ts`, built for this table * under #2143). null/false (default, absent) = no changed-files section = byte-identical behavior. (#1957) */ changedFilesSummary: boolean | null; + /** `review.finding_categories`: when true, an inline finding is ALSO tagged with a category (security/ + * correctness/performance/maintainability/tests/style) — the AI reviewer is asked to self-categorize, with a + * deterministic path/keyword fallback (`classifyFindingCategory`) covering whatever it omits. Only takes + * effect when inline comments are already on (a category has nothing to categorize otherwise) — this is an + * ADDITIONAL opt-in on top of `review.inline_comments`, not a replacement gate, mirroring `review.suggestions`. + * null/false (default, absent) = no category tagging = byte-identical behavior. (#1958) */ + findingCategories: boolean | null; /** `review.path_instructions`: per-path natural-language guidance handed to the AI reviewer when the PR's * changed files match the glob. Empty (default) ⇒ byte-identical reviewer prompt. (#review-path-instructions) */ pathInstructions: ReviewPathInstruction[]; @@ -580,7 +587,7 @@ const EMPTY_MANIFEST: FocusManifest = { publicNotes: [], gate: { ...EMPTY_GATE_CONFIG }, settings: {}, - review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG } }, + review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, findingCategories: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG } }, features: { ...EMPTY_FEATURES_CONFIG }, contentLane: { ...EMPTY_CONTENT_LANE_CONFIG }, repoDocGeneration: { ...EMPTY_REPO_DOC_GENERATION_CONFIG }, @@ -610,7 +617,7 @@ function emptyManifest(source: FocusManifestSource, warnings: string[] = []): Fo warnings, gate: { ...EMPTY_GATE_CONFIG }, settings: {}, - review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG } }, + review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, findingCategories: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG } }, features: { ...EMPTY_FEATURES_CONFIG }, contentLane: { ...EMPTY_CONTENT_LANE_CONFIG }, repoDocGeneration: { ...EMPTY_REPO_DOC_GENERATION_CONFIG }, @@ -1541,7 +1548,7 @@ function parsePublicSafeText(value: JsonValue | undefined, field: string, warnin * throws; invalid/unsafe values are dropped with warnings. */ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): FocusManifestReviewConfig { - const empty: FocusManifestReviewConfig = { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG } }; + const empty: FocusManifestReviewConfig = { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, findingCategories: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG } }; if (value === undefined || value === null) return empty; if (typeof value !== "object" || Array.isArray(value)) { warnings.push(`Manifest field "review" must be a mapping; ignoring it.`); @@ -1580,6 +1587,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo const inlineComments = normalizeOptionalBoolean(r.inline_comments, "review.inline_comments", warnings); const suggestions = normalizeOptionalBoolean(r.suggestions, "review.suggestions", warnings); const changedFilesSummary = normalizeOptionalBoolean(r.changed_files_summary, "review.changed_files_summary", warnings); + const findingCategories = normalizeOptionalBoolean(r.finding_categories, "review.finding_categories", warnings); const pathInstructions = parseReviewPathInstructions(r.path_instructions, warnings); const instructions = parsePublicSafeText(r.instructions, "review.instructions", warnings); const excludePaths = parseReviewExcludePaths(r.exclude_paths, warnings); @@ -1598,6 +1606,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo inlineComments !== null || suggestions !== null || changedFilesSummary !== null || + findingCategories !== null || pathInstructions.length > 0 || instructions !== null || excludePaths.length > 0 || @@ -1620,6 +1629,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo inlineComments, suggestions, changedFilesSummary, + findingCategories, pathInstructions, instructions, excludePaths, @@ -1939,6 +1949,7 @@ export function reviewConfigToJson(review: FocusManifestReviewConfig): JsonValue if (review.inlineComments !== null) out.inline_comments = review.inlineComments; if (review.suggestions !== null) out.suggestions = review.suggestions; if (review.changedFilesSummary !== null) out.changed_files_summary = review.changedFilesSummary; + if (review.findingCategories !== null) out.finding_categories = review.findingCategories; if (review.instructions !== null) out.instructions = review.instructions; if (review.pathInstructions.length > 0) out.path_instructions = review.pathInstructions.map((entry) => ({ path: entry.path, instructions: entry.instructions })); if (review.excludePaths.length > 0) out.exclude_paths = [...review.excludePaths]; @@ -2077,7 +2088,7 @@ export function composeManifestReviewInstructions(instructions: string | null, t * failure). A null manifest yields the byte-identical defaults. Centralized so the AI-review caller threads them * in one place with the null-manifest branch covered here (unit-tested) rather than inline in the processor. * (#review-profile / #review-tone / #review-security-focus / #review-path-instructions / #review-exclude-paths / #2043 / #selfhost-ai-model-override / #1956) */ -export function resolveReviewPromptOverrides(manifest: FocusManifest | null): { profile: ReviewProfile | null; tone: string | null; securityFocus: boolean; inlineComments: boolean; suggestions: boolean; changedFilesSummary: boolean; pathInstructions: ReviewPathInstruction[]; instructions: string | null; excludePaths: string[]; pathFilters: string[]; selfHostAiModel: SelfHostAiModelConfig } { +export function resolveReviewPromptOverrides(manifest: FocusManifest | null): { profile: ReviewProfile | null; tone: string | null; securityFocus: boolean; inlineComments: boolean; suggestions: boolean; changedFilesSummary: boolean; findingCategories: boolean; pathInstructions: ReviewPathInstruction[]; instructions: string | null; excludePaths: string[]; pathFilters: string[]; selfHostAiModel: SelfHostAiModelConfig } { // inlineComments resolves to a strict boolean — true ONLY when the manifest explicitly set review.inline_comments: // true; null/false/absent ⇒ false. The caller ANDs this per-repo toggle with the operator flag + cutover allowlist. // securityFocus resolves the same way — true ONLY when the manifest explicitly set review.security_focus: true. @@ -2085,7 +2096,9 @@ export function resolveReviewPromptOverrides(manifest: FocusManifest | null): { // inlineComments gate, since a suggestion has nothing to attach to without an inline comment. // changedFilesSummary resolves the same way (#1957) — independent of inlineComments/suggestions; it only // needs the unified-comment convergence feature itself to be on (the caller's own outer gate). - return { profile: manifest?.review.profile ?? null, tone: manifest?.review.tone ?? null, securityFocus: manifest?.review.securityFocus === true, inlineComments: manifest?.review.inlineComments === true, suggestions: manifest?.review.suggestions === true, changedFilesSummary: manifest?.review.changedFilesSummary === true, pathInstructions: manifest?.review.pathInstructions ?? [], instructions: manifest?.review.instructions ?? null, excludePaths: manifest?.review.excludePaths ?? [], pathFilters: manifest?.review.pathFilters ?? [], selfHostAiModel: resolveReviewSelfHostAiModel(manifest) }; + // findingCategories resolves the same way (#1958) — like suggestions, the caller further ANDs it with the + // already-resolved inlineComments gate, since a category has nothing to categorize without an inline finding. + return { profile: manifest?.review.profile ?? null, tone: manifest?.review.tone ?? null, securityFocus: manifest?.review.securityFocus === true, inlineComments: manifest?.review.inlineComments === true, suggestions: manifest?.review.suggestions === true, changedFilesSummary: manifest?.review.changedFilesSummary === true, findingCategories: manifest?.review.findingCategories === true, pathInstructions: manifest?.review.pathInstructions ?? [], instructions: manifest?.review.instructions ?? null, excludePaths: manifest?.review.excludePaths ?? [], pathFilters: manifest?.review.pathFilters ?? [], selfHostAiModel: resolveReviewSelfHostAiModel(manifest) }; } /** Resolve `review.pre_merge_checks` from a possibly-null manifest (null = load failure ⇒ no checks). Centralized diff --git a/test/unit/ai-review.test.ts b/test/unit/ai-review.test.ts index d149b65bfc..927a0da54c 100644 --- a/test/unit/ai-review.test.ts +++ b/test/unit/ai-review.test.ts @@ -40,6 +40,7 @@ type InlineFinding = { severity: "blocker" | "nit"; body: string; suggestion?: string; + category?: "security" | "correctness" | "performance" | "maintainability" | "tests" | "style"; }; type ModelReviewShape = { assessment: string; @@ -467,6 +468,33 @@ describe("review.profile shapes the reviewer system prompt (#review-profile)", ( expect(await runInline(false)).not.toContain("INLINE FINDINGS"); expect(await runInline(undefined)).not.toContain("INLINE FINDINGS"); }); + + it("the finding-category instruction is appended to the system prompt ONLY when BOTH inlineFindings and findingCategories are requested (#1958)", async () => { + const systemPromptOf = (run: ReturnType): string => + (run.mock.calls[0]?.[1] as { messages?: Array<{ content?: string }> }) + ?.messages?.[0]?.content ?? ""; + const runWith = async (inlineFindings: boolean | undefined, findingCategories: boolean | undefined) => { + const run = vi.fn(async () => ({ response: reviewJson() })); + const env = createTestEnv({ + AI: { run } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await runGittensoryAiReview(env, { ...baseInput, inlineFindings, findingCategories }); + return systemPromptOf(run); + }; + const withBoth = await runWith(true, true); + expect(withBoth).toContain("INLINE FINDINGS"); + expect(withBoth).toContain('"category"'); + // findingCategories alone (inlineFindings off) has nothing to categorize — byte-identical, no category text. + expect(await runWith(false, true)).not.toContain('"category"'); + // inlineFindings on but findingCategories absent/false ⇒ byte-identical (no category instruction). + const inlineOnly = await runWith(true, false); + expect(inlineOnly).toContain("INLINE FINDINGS"); + expect(inlineOnly).not.toContain('"category"'); + expect(await runWith(true, undefined)).not.toContain('"category"'); + }); }); describe("review.security_focus shapes the reviewer system prompt (#review-security-focus)", () => { @@ -2887,6 +2915,25 @@ describe("pure helpers", () => { ]); }); + it("parseModelReview parses a valid category, drops one outside the fixed enum, and leaves it absent when omitted (#1958)", () => { + const json = JSON.stringify({ + assessment: "ok", + blockers: [], + nits: [], + suggestions: [], + inlineFindings: [ + { path: "src/a.ts", line: 2, severity: "nit", body: "SQL injection risk.", category: "security" }, + { path: "src/b.ts", line: 4, severity: "nit", body: "Made up category.", category: "readability" }, + { path: "src/c.ts", line: 6, severity: "nit", body: "No category at all." }, + ], + }); + expect(parseModelReview(json)?.inlineFindings).toEqual([ + { path: "src/a.ts", line: 2, severity: "nit", body: "SQL injection risk.", category: "security" }, + { path: "src/b.ts", line: 4, severity: "nit", body: "Made up category." }, + { path: "src/c.ts", line: 6, severity: "nit", body: "No category at all." }, + ]); + }); + it("parseModelReview keeps findings but drops empty, whitespace-only, and malformed suggestions (#2138)", () => { const json = JSON.stringify({ assessment: "ok", @@ -2994,6 +3041,19 @@ describe("pure helpers", () => { ]); }); + it("composeInlineFindings carries a finding's category through verbatim (a fixed enum literal, not scrubbed like body/suggestion) (#1958)", () => { + const out = composeInlineFindings([ + reviewWithFindings([ + { path: "src/a.ts", line: 1, severity: "nit", body: "SQL injection risk.", category: "security" }, + { path: "src/b.ts", line: 2, severity: "nit", body: "No category on this one." }, + ]), + ]); + expect(out).toEqual([ + { path: "src/a.ts", line: 1, severity: "nit", body: "SQL injection risk.", category: "security" }, + { path: "src/b.ts", line: 2, severity: "nit", body: "No category on this one." }, + ]); + }); + it("composeInlineFindings drops blank or public-unsafe suggestions while keeping safe findings (#2138)", () => { const out = composeInlineFindings([ reviewWithFindings([ diff --git a/test/unit/finding-category-classify.test.ts b/test/unit/finding-category-classify.test.ts new file mode 100644 index 0000000000..0394854d76 --- /dev/null +++ b/test/unit/finding-category-classify.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from "vitest"; +import { classifyFindingCategory, FINDING_CATEGORIES, isFindingCategory } from "../../src/review/finding-category-classify"; + +describe("isFindingCategory (#1958)", () => { + it("accepts every value in the fixed enum", () => { + for (const category of FINDING_CATEGORIES) { + expect(isFindingCategory(category)).toBe(true); + } + }); + + it("rejects a value outside the fixed enum", () => { + expect(isFindingCategory("readability")).toBe(false); + }); + + it("rejects the wrong case (case-sensitive)", () => { + expect(isFindingCategory("Security")).toBe(false); + }); + + it("rejects a non-string", () => { + expect(isFindingCategory(1)).toBe(false); + expect(isFindingCategory(null)).toBe(false); + expect(isFindingCategory(undefined)).toBe(false); + expect(isFindingCategory({})).toBe(false); + }); +}); + +describe("classifyFindingCategory (#1958)", () => { + it("tests: a finding anchored to a test file, regardless of body wording", () => { + expect(classifyFindingCategory({ path: "src/app.test.ts", body: "This looks fine." })).toBe("tests"); + }); + + it("security: SQL injection wording", () => { + expect(classifyFindingCategory({ path: "src/db.ts", body: "This query is vulnerable to SQL injection." })).toBe("security"); + }); + + it("security: hardcoded credential wording", () => { + expect(classifyFindingCategory({ path: "src/config.ts", body: "This hardcoded password should be a secret." })).toBe("security"); + }); + + it("performance: N+1 wording", () => { + expect(classifyFindingCategory({ path: "src/api.ts", body: "This introduces an N+1 query inside the loop." })).toBe("performance"); + }); + + it("tests: missing-test wording on a non-test file", () => { + expect(classifyFindingCategory({ path: "src/util.ts", body: "This branch has no test coverage." })).toBe("tests"); + }); + + it("style: naming/formatting wording", () => { + expect(classifyFindingCategory({ path: "src/util.ts", body: "This variable naming is inconsistent." })).toBe("style"); + }); + + it("maintainability: duplication wording", () => { + expect(classifyFindingCategory({ path: "src/util.ts", body: "This duplicates logic already in helpers.ts." })).toBe("maintainability"); + }); + + it("falls through to correctness when nothing matches", () => { + expect(classifyFindingCategory({ path: "src/util.ts", body: "This will throw when the array is empty." })).toBe("correctness"); + }); + + it("precedence: a test-path finding wins over security wording in the body (path checked first)", () => { + expect(classifyFindingCategory({ path: "test/unit/auth.test.ts", body: "This test bypasses authentication entirely." })).toBe("tests"); + }); + + it("precedence: security wording wins over performance wording in the same body", () => { + expect( + classifyFindingCategory({ + path: "src/api.ts", + body: "This SQL injection risk also causes a slow N+1 query.", + }), + ).toBe("security"); + }); +}); diff --git a/test/unit/finding-category-collapsible.test.ts b/test/unit/finding-category-collapsible.test.ts new file mode 100644 index 0000000000..7fffcd3a4c --- /dev/null +++ b/test/unit/finding-category-collapsible.test.ts @@ -0,0 +1,125 @@ +import { describe, expect, it } from "vitest"; +import { buildFindingCategoryCollapsible, buildUnifiedCommentBody, type FindingCategoryInput } from "../../src/review/unified-comment-bridge"; +import type { GateCheckEvaluation } from "../../src/rules/advisory"; +import type { PublicPrPanelSignalRow } from "../../src/signals/engine"; + +function gate(over: Partial = {}): GateCheckEvaluation { + return { + enabled: true, + conclusion: "success", + title: "Gittensory Orb Review Agent passed", + summary: "No configured hard blocker was found.", + blockers: [], + warnings: [], + ...over, + }; +} + +const panelRows: PublicPrPanelSignalRow[] = [ + { key: "gateResult", cells: ["Gate result", "✅ Passing", "No configured blocker found.", "No action."] }, +]; +const footer = "💰 Earn for open-source contributions. Checked by Gittensory."; + +const findings: FindingCategoryInput[] = [ + { path: "src/db.ts", body: "This is vulnerable to SQL injection.", category: "security" }, + { path: "src/util.ts", body: "This will throw on an empty array." }, // no category — falls back to classifyFindingCategory + { path: "src/app.test.ts", body: "Assert the right value here." }, +]; + +describe("buildFindingCategoryCollapsible (#1958)", () => { + it("counts findings by category, using the finding's own category when present", () => { + const c = buildFindingCategoryCollapsible(findings); + expect(c).not.toBeNull(); + expect(c?.title).toBe("Finding categories"); + expect(c?.body).toContain("| Category | Findings |"); + expect(c?.body).toContain("| Security | 1 |"); + }); + + it("falls back to classifyFindingCategory for a finding missing its own category (never dropped)", () => { + const c = buildFindingCategoryCollapsible(findings); + // src/util.ts's body matches no keyword bucket and isn't a test path → correctness. + expect(c?.body).toContain("| Correctness | 1 |"); + // src/app.test.ts is a test path → tests, regardless of its body wording. + expect(c?.body).toContain("| Tests | 1 |"); + }); + + it("collapses multiple findings of the same category into one row with a summed count", () => { + const c = buildFindingCategoryCollapsible([ + { path: "src/a.ts", body: "Fix this bug.", category: "correctness" }, + { path: "src/b.ts", body: "Fix that bug too.", category: "correctness" }, + ]); + expect(c?.body).toContain("| Correctness | 2 |"); + }); + + it("orders rows security-first, matching the fixed FINDING_CATEGORIES order", () => { + const c = buildFindingCategoryCollapsible([ + { path: "src/a.ts", body: "style nit: naming", category: "style" }, + { path: "src/b.ts", body: "sql injection risk", category: "security" }, + ]); + const body = c?.body ?? ""; + expect(body.indexOf("| Security")).toBeLessThan(body.indexOf("| Style")); + }); + + it("omits a category with no findings (no zero rows)", () => { + const c = buildFindingCategoryCollapsible([{ path: "src/a.ts", body: "Fix this.", category: "correctness" }]); + expect(c?.body).toContain("| Correctness | 1 |"); + expect(c?.body).not.toContain("Security"); + expect(c?.body).not.toContain("Performance"); + }); + + it("returns null for an empty finding list (no empty table)", () => { + expect(buildFindingCategoryCollapsible([])).toBeNull(); + }); + + it("is not marked as raw HTML (plain markdown table)", () => { + const c = buildFindingCategoryCollapsible(findings); + expect(c?.rawHtml).toBeUndefined(); + }); +}); + +describe("buildUnifiedCommentBody findingCategories wiring (#1958)", () => { + const base = { + gate: gate(), + panelRows, + readinessTotal: 90, + changedFiles: 3, + footerMarkdown: footer, + }; + + it("appends the Finding categories section when findingCategories is present + non-empty", () => { + const body = buildUnifiedCommentBody({ ...base, findingCategories: findings }); + expect(body).toContain("Finding categories"); + expect(body).toContain("| Security | 1 |"); + }); + + it("does NOT add a Finding categories section when findingCategories is absent (flag-OFF parity)", () => { + const body = buildUnifiedCommentBody(base); + expect(body).not.toContain("Finding categories"); + }); + + it("does NOT add a Finding categories section when findingCategories is empty", () => { + const body = buildUnifiedCommentBody({ ...base, findingCategories: [] }); + expect(body).not.toContain("Finding categories"); + }); + + it("preserves pre-existing extraCollapsibles alongside the Finding categories section", () => { + const body = buildUnifiedCommentBody({ + ...base, + extraCollapsibles: [{ title: "Signal definitions", body: "what each row means" }], + findingCategories: findings, + }); + expect(body).toContain("Signal definitions"); + expect(body).toContain("Finding categories"); + }); + + it("coexists with the Changed files section (both collapsibles render, in order)", () => { + const body = buildUnifiedCommentBody({ + ...base, + changedFilesSummary: [{ path: "src/app.ts", additions: 5, deletions: 1 }], + findingCategories: findings, + }); + expect(body).toContain("Changed files"); + expect(body).toContain("Finding categories"); + expect(body.indexOf("Changed files")).toBeLessThan(body.indexOf("Finding categories")); + }); +}); diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index db5c71305c..6d976a5f4e 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -566,7 +566,7 @@ describe("compileFocusManifestPolicy", () => { publicNotes: ["Keep PRs focused.", "Maximize your reward payout"], gate: { present: false, enabled: null, checkMode: null, pack: null, linkedIssue: null, duplicates: null, readinessMode: null, readinessMinScore: null, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, lockfileIntegrityMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, manifestPolicy: null, dryRun: null, firstTimeContributorGrace: null, premergeContentRecheck: null, requireFreshRebaseWindowMinutes: null, claMode: null, claConsentPhrase: null, claCheckRunName: null, claCheckRunAppSlug: null, expectedCiContexts: null }, settings: {}, - review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG } }, + review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, findingCategories: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG } }, features: { present: false, rag: null, reputation: null, unifiedComment: null, safety: null }, contentLane: { present: false, entryFileGlob: null, providerFileGlob: null, artifactGlob: null, collectionField: null, maxAppendedEntries: null, duplicateKeyFields: [], validatorId: null }, repoDocGeneration: { present: false, enabled: false, scope: ["agents"], allowOverwriteExisting: false, refreshIntervalDays: 7 }, @@ -2670,10 +2670,10 @@ describe("resolveReviewPathInstructions (#review-path-instructions)", () => { }); it("resolveReviewPromptOverrides: non-null manifest passes the config through; null manifest → defaults", () => { - const manifest = parseFocusManifest({ review: { profile: "chill", security_focus: true, inline_comments: true, suggestions: true, changed_files_summary: true, path_instructions: [{ path: "src/**", instructions: "be strict" }], instructions: "Follow our async-error conventions.", exclude_paths: ["**/*.lock"], path_filters: ["src/**", "!src/generated/**"] } }); - expect(resolveReviewPromptOverrides(manifest)).toEqual({ profile: "chill", tone: null, securityFocus: true, inlineComments: true, suggestions: true, changedFilesSummary: true, pathInstructions: [{ path: "src/**", instructions: "be strict" }], instructions: "Follow our async-error conventions.", excludePaths: ["**/*.lock"], pathFilters: ["src/**", "!src/generated/**"], selfHostAiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG } }); - // A null manifest (load failure) yields the byte-identical defaults; inline comments + suggestions + changed-files summary + security focus default OFF. - expect(resolveReviewPromptOverrides(null)).toEqual({ profile: null, tone: null, securityFocus: false, inlineComments: false, suggestions: false, changedFilesSummary: false, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], selfHostAiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG } }); + const manifest = parseFocusManifest({ review: { profile: "chill", security_focus: true, inline_comments: true, suggestions: true, changed_files_summary: true, finding_categories: true, path_instructions: [{ path: "src/**", instructions: "be strict" }], instructions: "Follow our async-error conventions.", exclude_paths: ["**/*.lock"], path_filters: ["src/**", "!src/generated/**"] } }); + expect(resolveReviewPromptOverrides(manifest)).toEqual({ profile: "chill", tone: null, securityFocus: true, inlineComments: true, suggestions: true, changedFilesSummary: true, findingCategories: true, pathInstructions: [{ path: "src/**", instructions: "be strict" }], instructions: "Follow our async-error conventions.", excludePaths: ["**/*.lock"], pathFilters: ["src/**", "!src/generated/**"], selfHostAiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG } }); + // A null manifest (load failure) yields the byte-identical defaults; inline comments + suggestions + changed-files summary + finding categories + security focus default OFF. + expect(resolveReviewPromptOverrides(null)).toEqual({ profile: null, tone: null, securityFocus: false, inlineComments: false, suggestions: false, changedFilesSummary: false, findingCategories: false, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], selfHostAiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG } }); // An explicit false / absent toggle both resolve to the strict-boolean false. expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { inline_comments: false } })).inlineComments).toBe(false); expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { profile: "chill" } })).inlineComments).toBe(false); @@ -2681,6 +2681,8 @@ describe("resolveReviewPathInstructions (#review-path-instructions)", () => { expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { profile: "chill" } })).suggestions).toBe(false); expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { changed_files_summary: false } })).changedFilesSummary).toBe(false); expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { profile: "chill" } })).changedFilesSummary).toBe(false); + expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { finding_categories: false } })).findingCategories).toBe(false); + expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { profile: "chill" } })).findingCategories).toBe(false); expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { security_focus: false } })).securityFocus).toBe(false); expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { profile: "chill" } })).securityFocus).toBe(false); }); @@ -2735,6 +2737,23 @@ describe("resolveReviewPathInstructions (#review-path-instructions)", () => { expect(bad.review.changedFilesSummary).toBeNull(); expect(bad.warnings.some((w) => /review\.changed_files_summary.*must be a boolean/.test(w))).toBe(true); }); + + it("parses review.finding_categories (default OFF), marks present, round-trips, and warns on a non-boolean (#1958)", () => { + expect(parseFocusManifest({ review: { finding_categories: true } }).review.findingCategories).toBe(true); + const on = parseFocusManifest({ review: { finding_categories: true } }); + expect(on.review.present).toBe(true); // a finding-categories-only manifest IS present + expect(parseFocusManifest({ review: reviewConfigToJson(on.review) }).review).toEqual(on.review); // survives round-trip + // Explicit false is retained (and marks present, since the maintainer set it). + const off = parseFocusManifest({ review: { finding_categories: false } }); + expect(off.review.findingCategories).toBe(false); + expect(off.review.present).toBe(true); + // Absent ⇒ null (the byte-identical default), config not present. + expect(parseFocusManifest({ review: {} }).review.findingCategories).toBeNull(); + // A non-boolean is ignored with a warning. + const bad = parseFocusManifest({ review: { finding_categories: "yes" } }); + expect(bad.review.findingCategories).toBeNull(); + expect(bad.warnings.some((w) => /review\.finding_categories.*must be a boolean/.test(w))).toBe(true); + }); }); describe("review.exclude_paths (#review-exclude-paths)", () => { diff --git a/test/unit/inline-comments.test.ts b/test/unit/inline-comments.test.ts index 0cd115c533..4a72281f66 100644 --- a/test/unit/inline-comments.test.ts +++ b/test/unit/inline-comments.test.ts @@ -1,7 +1,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { generateKeyPairSync } from "node:crypto"; import type { InlineFinding } from "../../src/services/ai-review"; -import { isInlineCommentsEnabled, maybePostInlineComments, postInlineReviewComments, rightSideLinesFromPatch, selectInlineComments, shouldRenderSuggestions, shouldRequestInlineFindings } from "../../src/review/inline-comments"; +import { isInlineCommentsEnabled, maybePostInlineComments, postInlineReviewComments, rightSideLinesFromPatch, selectInlineComments, shouldRenderFindingCategories, shouldRenderSuggestions, shouldRequestInlineFindings } from "../../src/review/inline-comments"; import { createTestEnv } from "../helpers/d1"; function envWithKey() { @@ -41,6 +41,16 @@ describe("shouldRenderSuggestions (#1956)", () => { }); }); +describe("shouldRenderFindingCategories (#1958)", () => { + it("requires the manifest toggle AND inline comments already being enabled — a category has nothing to categorize otherwise", () => { + expect(shouldRenderFindingCategories(true, true)).toBe(true); + expect(shouldRenderFindingCategories(true, false)).toBe(false); // manifest toggle off + expect(shouldRenderFindingCategories(true, undefined)).toBe(false); // manifest toggle absent + expect(shouldRenderFindingCategories(false, true)).toBe(false); // inline comments themselves are off + expect(shouldRenderFindingCategories(false, false)).toBe(false); + }); +}); + describe("rightSideLinesFromPatch (#inline-comments)", () => { it("returns RIGHT-side line numbers for added + context lines, excluding deleted lines and the no-newline marker", () => { const patch = "@@ -1,3 +1,4 @@\n ctx1\n-removed\n+added2\n+added3\n ctx4\n\\ No newline at end of file"; @@ -128,6 +138,37 @@ describe("selectInlineComments (#inline-comments)", () => { expect(out[0]?.body).not.toContain("escape attempt"); }); }); + + describe("category tags (#1958)", () => { + const withCategory: InlineFinding = { path: "src/a.ts", line: 2, severity: "nit", body: "Use const.", category: "style" }; + + it("defaults to OFF (backward compatible) — no category tag when the fourth argument is omitted", () => { + const out = selectInlineComments([withCategory], files); + expect(out).toEqual([{ path: "src/a.ts", line: 2, side: "RIGHT", body: "**Nit:** Use const." }]); + }); + + it("does not render a category tag when explicitly disabled, even if the finding carries one", () => { + const out = selectInlineComments([withCategory], files, false, false); + expect(out[0]?.body).not.toContain("(style)"); + }); + + it("renders the model's own category when enabled and the finding carries one", () => { + const out = selectInlineComments([withCategory], files, false, true); + expect(out[0]?.body).toBe("**Nit (style):** Use const."); + }); + + it("falls back to the deterministic classifier when enabled but the finding has no category (safe default, never omitted)", () => { + const noCategory: InlineFinding = { path: "src/app.test.ts", line: 2, severity: "nit", body: "Use const." }; + const out = selectInlineComments([noCategory], [fileWith("src/app.test.ts", "@@ -1,1 +1,2 @@\n ctx\n+added2")], false, true); + expect(out[0]?.body).toBe("**Nit (tests):** Use const."); + }); + + it("composes with a suggestion block — both the category tag and the suggestion render together", () => { + const both: InlineFinding = { path: "src/a.ts", line: 2, severity: "blocker", body: "Missing null check.", category: "correctness", suggestion: "if (!x) return;" }; + const out = selectInlineComments([both], files, true, true); + expect(out[0]?.body).toBe("**Blocker (correctness):** Missing null check.\n\n```suggestion\nif (!x) return;\n```"); + }); + }); }); describe("postInlineReviewComments (#inline-comments, fail-safe)", () => { @@ -255,4 +296,34 @@ describe("maybePostInlineComments (#inline-comments, review-path entry)", () => await maybePostInlineComments(envWithKey(), { ...base, aiReview: { inlineFindings: withSuggestion }, getFiles }); expect(calls[0]?.body).toMatchObject({ comments: [{ path: "src/a.ts", line: 2, side: "RIGHT", body: "**Nit:** guard this" }] }); }); + + it("renders a category tag end-to-end when categoriesEnabled is threaded through (#1958)", async () => { + const getFiles = vi.fn(async () => files); + const withCategory: InlineFinding[] = [{ path: "src/a.ts", line: 2, severity: "nit", body: "guard this", category: "maintainability" }]; + const calls: Array<{ url: string; body: unknown }> = []; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "t" }); + calls.push({ url, body: init?.body ? JSON.parse(String(init.body)) : null }); + if (url.endsWith("/pulls/3/reviews")) return Response.json({ id: 12 }); + return new Response("unexpected", { status: 500 }); + }); + await maybePostInlineComments(envWithKey(), { ...base, aiReview: { inlineFindings: withCategory }, getFiles, categoriesEnabled: true }); + expect(calls[0]?.body).toMatchObject({ comments: [{ path: "src/a.ts", line: 2, side: "RIGHT", body: "**Nit (maintainability):** guard this" }] }); + }); + + it("omits the category tag end-to-end when categoriesEnabled is not passed (default off, backward compatible)", async () => { + const getFiles = vi.fn(async () => files); + const withCategory: InlineFinding[] = [{ path: "src/a.ts", line: 2, severity: "nit", body: "guard this", category: "maintainability" }]; + const calls: Array<{ url: string; body: unknown }> = []; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "t" }); + calls.push({ url, body: init?.body ? JSON.parse(String(init.body)) : null }); + if (url.endsWith("/pulls/3/reviews")) return Response.json({ id: 13 }); + return new Response("unexpected", { status: 500 }); + }); + await maybePostInlineComments(envWithKey(), { ...base, aiReview: { inlineFindings: withCategory }, getFiles }); + expect(calls[0]?.body).toMatchObject({ comments: [{ path: "src/a.ts", line: 2, side: "RIGHT", body: "**Nit:** guard this" }] }); + }); }); diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 085f5d24ff..340b34a112 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -15680,6 +15680,95 @@ describe("queue processors", () => { } }); + // #1958: with inline comments AND finding categories both on in .gittensory.yml (finding_categories rides on + // inline_comments, exactly like suggestions did for #1956), the model is asked to self-categorize each + // inlineFindings item, and BOTH surfaces render it — the posted inline review comment label AND the unified + // comment's new "Finding categories" collapsible. + it("renders finding categories in the inline comment label and the unified comment's Finding categories section when review.finding_categories is on", async () => { + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + GITTENSORY_REVIEW_UNIFIED_COMMENT: "1", + GITTENSORY_REVIEW_INLINE_COMMENTS: "true", + GITTENSORY_REVIEW_REPOS: "JSONbored/gittensory", + AI: { + run: async () => + ({ + response: JSON.stringify({ + assessment: "Looks fine overall.", + blockers: [], + nits: [], + suggestions: [], + inlineFindings: [ + { path: "src/db.ts", line: 2, severity: "nit", body: "This query is vulnerable to SQL injection.", category: "security" }, + ], + }), + }) as { response: string }, + } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 123); + await upsertRepositorySettings(env, { + repoFullName: "JSONbored/gittensory", + commentMode: "all_prs", + publicSurface: "comment_only", + autoLabelEnabled: false, + checkRunMode: "off", + gateCheckMode: "enabled", + aiReviewMode: "block", + gatePack: "oss-anti-slop", + }); + let inlineReviewComments: Array<{ body: string }> = []; + let unifiedCommentBody = ""; + 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: "installation-token" }); + // .gittensory.yml opts into inline comments AND finding categories together. + if (url === "https://raw.githubusercontent.com/JSONbored/gittensory/HEAD/.gittensory.yml") { + return new Response("review:\n inline_comments: true\n finding_categories: true\n"); + } + if (url.includes("/pulls/8/files")) + return Response.json([{ filename: "src/db.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@ -1,1 +1,2 @@\n ctx\n+export const ok = true;" }]); + if (url.endsWith("/pulls/8")) return Response.json({ number: 8, title: "Add query helper", state: "open", user: { login: "contributor" }, head: { sha: "a8" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/commits/a8/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/a8/status")) return Response.json({ state: "success", statuses: [] }); + 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: [] } } }); + // The separate, quiet inline-review post (event: COMMENT) — distinct from the sticky unified issue comment. + if (url.endsWith("/pulls/8/reviews") && method === "POST") { + const body = JSON.parse(String(init?.body ?? "{}")) as { comments?: Array<{ body: string }> }; + inlineReviewComments = body.comments ?? []; + return Response.json({ id: 55 }); + } + if (url.includes("/issues/8/comments") && method === "GET") return Response.json([]); + if (url.includes("/issues/8/comments") && method === "POST") { + unifiedCommentBody = String((JSON.parse(String(init?.body ?? "{}")) as { body?: string }).body ?? ""); + return Response.json({ id: 1 }, { status: 201 }); + } + return Response.json({}); + }); + + await processJob(env, { + type: "github-webhook", + deliveryId: "pr-finding-categories", + eventName: "pull_request", + payload: { + action: "opened", + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + pull_request: { number: 8, title: "Add query helper", state: "open", user: { login: "contributor" }, head: { sha: "a8" }, labels: [], body: "Closes #1" }, + }, + }); + + // The inline PR-review comment label carries the category tag. + expect(inlineReviewComments[0]?.body).toBe("**Nit (security):** This query is vulnerable to SQL injection."); + // The unified comment's new collapsible counts it too. + expect(unifiedCommentBody).toContain("Finding categories"); + expect(unifiedCommentBody).toContain("| Security | 1 |"); + }); + // FIX B + FIX D3 at the processor call site: a unified comment for a PR whose CI has a FAILED check, with the // PR's files only available from GitHub (stored rows empty) — proves (B) the inline file fetch populates the // real diff/changed-file count on the first review, and (D3) the failing check name + its per-check WHY render diff --git a/test/unit/signals-coverage.test.ts b/test/unit/signals-coverage.test.ts index 9cf9fc9232..b91fe6177b 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, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { skipDrafts: null, ignoreAuthors: [], ignoreTitleKeywords: [], baseBranches: [], autoPauseAfterReviewedCommits: null }, labelingRules: [], aiModel: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: 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, findingCategories: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { skipDrafts: null, ignoreAuthors: [], ignoreTitleKeywords: [], baseBranches: [], autoPauseAfterReviewedCommits: null }, labelingRules: [], aiModel: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: 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