diff --git a/.gittensory.yml.example b/.gittensory.yml.example index a3d69525d9..59b4cd9210 100644 --- a/.gittensory.yml.example +++ b/.gittensory.yml.example @@ -397,6 +397,10 @@ review: # effort_score: false # When true, the unified review comment gains a compact "review effort: N/5 (~M min)" chip. + # Display-only floor for inline AI findings (`critical` | `major` | `minor` | `nitpick`). Findings below the + # configured level are suppressed from inline comments — never from gate blockers. Default: null (show all). + # min_finding_severity: major + # Deterministic label suggestions (#2045). Each rule SUGGESTS a non-scoring label when a PR matches ALL of the # `when` criteria it sets (at least one is required): when_paths (any changed path matches a glob), title_contains, # description_contains (both case-insensitive). Suggestions are advisory; they are auto-applied only when the repo's diff --git a/config/examples/gittensory.full.yml b/config/examples/gittensory.full.yml index e23613877c..03b1def04f 100644 --- a/config/examples/gittensory.full.yml +++ b/config/examples/gittensory.full.yml @@ -410,6 +410,10 @@ review: # effort_score: false # When true, the unified review comment gains a compact "review effort: N/5 (~M min)" chip. + # Display-only floor for inline AI findings (`critical` | `major` | `minor` | `nitpick`). Findings below the + # configured level are suppressed from inline comments — never from gate blockers. Default: null (show all). + # min_finding_severity: major + # Deterministic label suggestions (#2045). Each rule SUGGESTS a non-scoring label when a PR matches ALL of the # `when` criteria it sets (at least one is required): when_paths (any changed path matches a glob), title_contains, # description_contains (both case-insensitive). Suggestions are advisory; they are auto-applied only when the repo's diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 7e004a092b..790da73e32 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -373,6 +373,7 @@ import { type FocusManifest, type ReviewPathInstruction, type ReviewProfile, + type ReviewFindingSeverity, type SelfHostAiModelConfig, type VisualConfig, } from "../signals/focus-manifest"; @@ -7684,6 +7685,7 @@ async function maybePublishPrPublicSurface( let changedFilesSummaryEnabledForReview = false; let effortScoreEnabledForReview = false; let findingCategoriesEnabledForReview = false; + let minFindingSeverityForReview: ReviewFindingSeverity | null = null; let aiReviewExpected = false; let aiReviewWasReused = false; let gateFinalized = false; @@ -8187,6 +8189,7 @@ async function maybePublishPrPublicSurface( const deterministicReviewOverrides = resolveReviewPromptOverrides(reviewManifestForAutoReview); changedFilesSummaryEnabledForReview = deterministicReviewOverrides.changedFilesSummary; effortScoreEnabledForReview = deterministicReviewOverrides.effortScore; + minFindingSeverityForReview = deterministicReviewOverrides.minFindingSeverity; const aiReviewWillRun = !authorBlacklisted && !isFrozenForManualReview && @@ -9481,6 +9484,7 @@ async function maybePublishPrPublicSurface( inlineCommentsEnabled: inlineCommentsEnabledForReview, suggestionsEnabled: suggestionsEnabledForReview, categoriesEnabled: findingCategoriesEnabledForReview, + minFindingSeverity: minFindingSeverityForReview, }); } if (decision.willLabel) { diff --git a/src/review/finding-severity-filter.ts b/src/review/finding-severity-filter.ts new file mode 100644 index 0000000000..912ef9c065 --- /dev/null +++ b/src/review/finding-severity-filter.ts @@ -0,0 +1,29 @@ +import type { ReviewFindingSeverity } from "../signals/focus-manifest"; + +const SEVERITY_RANK: Record = { + critical: 0, + major: 1, + minor: 2, + nitpick: 3, +}; + +/** True when `findingSeverity` is at or above the configured floor (critical is highest). null min ⇒ always true. */ +export function meetsMinFindingSeverity( + findingSeverity: ReviewFindingSeverity, + minSeverity: ReviewFindingSeverity | null | undefined, +): boolean { + if (!minSeverity) return true; + return SEVERITY_RANK[findingSeverity] <= SEVERITY_RANK[minSeverity]; +} + +/** Map inline-comment severities onto the unified review finding ladder for threshold checks. */ +export function inlineFindingSeverityTier(severity: "blocker" | "nit"): ReviewFindingSeverity { + return severity === "blocker" ? "critical" : "nitpick"; +} + +export function shouldShowInlineFinding( + severity: "blocker" | "nit", + minSeverity: ReviewFindingSeverity | null | undefined, +): boolean { + return meetsMinFindingSeverity(inlineFindingSeverityTier(severity), minSeverity); +} diff --git a/src/review/inline-comments.ts b/src/review/inline-comments.ts index 04d8f8d062..cda032cb5d 100644 --- a/src/review/inline-comments.ts +++ b/src/review/inline-comments.ts @@ -11,7 +11,9 @@ import { createPullRequestReviewComments } from "../github/pr-actions"; import { isConvergenceRepoAllowed } from "./cutover-gate"; import { classifyFindingCategory } from "./finding-category-classify"; +import { shouldShowInlineFinding } from "./finding-severity-filter"; import type { InlineFinding } from "../services/ai-review"; +import type { ReviewFindingSeverity } from "../signals/focus-manifest"; import type { AgentActionMode } from "../settings/agent-execution"; import type { PullRequestFileRecord } from "../types"; import { errorMessage } from "../utils/json"; @@ -120,7 +122,13 @@ function formatInlineBody(finding: InlineFinding, suggestionsEnabled: boolean, c * 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". `categoriesEnabled` * (#1958) gates whether the label carries a category tag. */ -export function selectInlineComments(findings: InlineFinding[], files: Pick[], suggestionsEnabled = false, categoriesEnabled = false): ReviewInlineComment[] { +export function selectInlineComments( + findings: InlineFinding[], + files: Pick[], + suggestionsEnabled = false, + categoriesEnabled = false, + minFindingSeverity: ReviewFindingSeverity | null | undefined = null, +): ReviewInlineComment[] { const rightLinesByPath = new Map>(); for (const file of files) { const patch = typeof file.payload?.patch === "string" ? file.payload.patch : ""; @@ -129,6 +137,7 @@ export function selectInlineComments(findings: InlineFinding[], files: Pick(); for (const finding of findings) { + if (!shouldShowInlineFinding(finding.severity, minFindingSeverity)) continue; if (out.length >= MAX_INLINE_COMMENTS) break; const validLines = rightLinesByPath.get(finding.path); if (!validLines || !validLines.has(finding.line)) continue; // not a commentable diff line → drop (no 422) @@ -156,9 +165,16 @@ export async function postInlineReviewComments( mode: AgentActionMode; suggestionsEnabled?: boolean | undefined; categoriesEnabled?: boolean | undefined; + minFindingSeverity?: ReviewFindingSeverity | null | undefined; }, ): Promise<{ posted: number }> { - const comments = selectInlineComments(args.findings, args.files, args.suggestionsEnabled, args.categoriesEnabled); + const comments = selectInlineComments( + args.findings, + args.files, + args.suggestionsEnabled, + args.categoriesEnabled, + args.minFindingSeverity, + ); if (comments.length === 0 || !args.commitId) return { posted: 0 }; try { await createPullRequestReviewComments(env, args.installationId, args.repoFullName, args.pullNumber, args.commitId, comments, args.mode); @@ -188,6 +204,7 @@ export async function maybePostInlineComments( inlineCommentsEnabled: boolean; suggestionsEnabled?: boolean | undefined; categoriesEnabled?: boolean | undefined; + minFindingSeverity?: ReviewFindingSeverity | null | undefined; }, ): Promise { if (!args.inlineCommentsEnabled) return; @@ -203,5 +220,6 @@ export async function maybePostInlineComments( mode: args.mode, suggestionsEnabled: args.suggestionsEnabled, categoriesEnabled: args.categoriesEnabled, + minFindingSeverity: args.minFindingSeverity, }); } diff --git a/src/signals/focus-manifest.ts b/src/signals/focus-manifest.ts index 3c54768edf..e96b52a971 100644 --- a/src/signals/focus-manifest.ts +++ b/src/signals/focus-manifest.ts @@ -287,6 +287,10 @@ export type ReviewFieldKey = (typeof REVIEW_FIELD_KEYS)[number]; export const REVIEW_PROFILES = ["chill", "balanced", "assertive"] as const; export type ReviewProfile = (typeof REVIEW_PROFILES)[number]; +export type ReviewFindingSeverity = "critical" | "major" | "minor" | "nitpick"; + +export const REVIEW_FINDING_SEVERITY_LADDER = ["critical", "major", "minor", "nitpick"] as const; + /** * Maintainer overrides for the public review-panel CONTENT, declared under `review:`. Customizes the * panel without changing what gittensory measures: a custom public-safe footer lead line, a custom intro @@ -345,6 +349,10 @@ export type FocusManifestReviewConfig = { * 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.min_finding_severity`: display-only floor for AI findings with a severity tier. Findings below the + * configured level are suppressed from inline comments — never from gate blockers. null (default, absent) ⇒ every + * finding shown = byte-identical behavior. (#2048) */ + minFindingSeverity: ReviewFindingSeverity | 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[]; @@ -676,7 +684,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, effortScore: null, findingCategories: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null }, + review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, effortScore: null, findingCategories: null, minFindingSeverity: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null }, features: { ...EMPTY_FEATURES_CONFIG }, contentLane: { ...EMPTY_CONTENT_LANE_CONFIG }, repoDocGeneration: { ...EMPTY_REPO_DOC_GENERATION_CONFIG }, @@ -706,7 +714,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, effortScore: null, findingCategories: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null }, + review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, effortScore: null, findingCategories: null, minFindingSeverity: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null }, features: { ...EMPTY_FEATURES_CONFIG }, contentLane: { ...EMPTY_CONTENT_LANE_CONFIG }, repoDocGeneration: { ...EMPTY_REPO_DOC_GENERATION_CONFIG }, @@ -1657,7 +1665,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, effortScore: null, findingCategories: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null }; + const empty: FocusManifestReviewConfig = { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, effortScore: null, findingCategories: null, minFindingSeverity: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null }; 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.`); @@ -1698,6 +1706,12 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo const changedFilesSummary = normalizeOptionalBoolean(r.changed_files_summary, "review.changed_files_summary", warnings); const effortScore = normalizeOptionalBoolean(r.effort_score, "review.effort_score", warnings); const findingCategories = normalizeOptionalBoolean(r.finding_categories, "review.finding_categories", warnings); + const minFindingSeverity = normalizeOptionalEnum( + r.min_finding_severity, + "review.min_finding_severity", + REVIEW_FINDING_SEVERITY_LADDER, + warnings, + ); const pathInstructions = parseReviewPathInstructions(r.path_instructions, warnings); const instructions = parsePublicSafeText(r.instructions, "review.instructions", warnings); const excludePaths = parseReviewExcludePaths(r.exclude_paths, warnings); @@ -1720,6 +1734,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo changedFilesSummary !== null || effortScore !== null || findingCategories !== null || + minFindingSeverity !== null || pathInstructions.length > 0 || instructions !== null || excludePaths.length > 0 || @@ -1748,6 +1763,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo changedFilesSummary, effortScore, findingCategories, + minFindingSeverity, pathInstructions, instructions, excludePaths, @@ -2191,6 +2207,7 @@ export function reviewConfigToJson(review: FocusManifestReviewConfig): JsonValue if (review.changedFilesSummary !== null) out.changed_files_summary = review.changedFilesSummary; if (review.effortScore !== null) out.effort_score = review.effortScore; if (review.findingCategories !== null) out.finding_categories = review.findingCategories; + if (review.minFindingSeverity !== null) out.min_finding_severity = review.minFindingSeverity; 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]; @@ -2406,7 +2423,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; effortScore: boolean; findingCategories: 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; effortScore: boolean; findingCategories: boolean; minFindingSeverity: ReviewFindingSeverity | null; 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. @@ -2418,7 +2435,7 @@ export function resolveReviewPromptOverrides(manifest: FocusManifest | null): { // (never touches the AI prompt) and only needs the unified-comment convergence feature to be on. // 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, effortScore: manifest?.review.effortScore === 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) }; + 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, effortScore: manifest?.review.effortScore === true, findingCategories: manifest?.review.findingCategories === true, minFindingSeverity: manifest?.review.minFindingSeverity ?? null, 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/config-templates.test.ts b/test/unit/config-templates.test.ts index 5ae5136361..5b2bda0146 100644 --- a/test/unit/config-templates.test.ts +++ b/test/unit/config-templates.test.ts @@ -58,7 +58,7 @@ describe("config/examples review templates (#1682)", () => { it("documents shipped unified-comment display toggles in gittensory.full.yml (#2069)", () => { const full = readConfigExample("gittensory.full.yml"); - for (const field of ["changed_files_summary", "effort_score"]) { + for (const field of ["changed_files_summary", "effort_score", "min_finding_severity"]) { expect(full, `missing review field ${field}`).toMatch(new RegExp(`# ${field}:`)); } expect(full).not.toMatch(/Planned display toggle/); diff --git a/test/unit/finding-severity-filter.test.ts b/test/unit/finding-severity-filter.test.ts new file mode 100644 index 0000000000..f284848232 --- /dev/null +++ b/test/unit/finding-severity-filter.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from "vitest"; +import { + inlineFindingSeverityTier, + meetsMinFindingSeverity, + shouldShowInlineFinding, +} from "../../src/review/finding-severity-filter"; + +describe("finding-severity-filter (#2048)", () => { + it("maps inline blocker/nit severities onto the unified ladder", () => { + expect(inlineFindingSeverityTier("blocker")).toBe("critical"); + expect(inlineFindingSeverityTier("nit")).toBe("nitpick"); + }); + + it("keeps findings at or above the configured floor", () => { + expect(meetsMinFindingSeverity("critical", "major")).toBe(true); + expect(meetsMinFindingSeverity("major", "major")).toBe(true); + expect(meetsMinFindingSeverity("minor", "major")).toBe(false); + expect(meetsMinFindingSeverity("nitpick", "major")).toBe(false); + expect(meetsMinFindingSeverity("nitpick", null)).toBe(true); + }); + + it("filters inline findings without changing gate blockers", () => { + expect(shouldShowInlineFinding("blocker", "major")).toBe(true); + expect(shouldShowInlineFinding("nit", "major")).toBe(false); + expect(shouldShowInlineFinding("nit", null)).toBe(true); + }); +}); diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index 9833eb065c..40178044e1 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -355,6 +355,7 @@ describe(".gittensory.yml.example field-exhaustiveness (#1670)", () => { changedFilesSummary: "changed_files_summary:", effortScore: "effort_score:", findingCategories: "finding_categories:", + minFindingSeverity: "min_finding_severity:", pathInstructions: "path_instructions:", instructions: "instructions:", excludePaths: "exclude_paths:", @@ -765,7 +766,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, effortScore: null, findingCategories: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null }, + review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, changedFilesSummary: null, effortScore: null, findingCategories: null, minFindingSeverity: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null }, 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 }, @@ -2870,9 +2871,9 @@ 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, effort_score: 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, effortScore: 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 } }); + expect(resolveReviewPromptOverrides(manifest)).toEqual({ profile: "chill", tone: null, securityFocus: true, inlineComments: true, suggestions: true, changedFilesSummary: true, effortScore: true, findingCategories: true, minFindingSeverity: null, 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 + effort score + finding categories + security focus default OFF. - expect(resolveReviewPromptOverrides(null)).toEqual({ profile: null, tone: null, securityFocus: false, inlineComments: false, suggestions: false, changedFilesSummary: false, effortScore: false, findingCategories: false, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], selfHostAiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG } }); + expect(resolveReviewPromptOverrides(null)).toEqual({ profile: null, tone: null, securityFocus: false, inlineComments: false, suggestions: false, changedFilesSummary: false, effortScore: false, findingCategories: false, minFindingSeverity: null, 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); @@ -2972,6 +2973,19 @@ describe("resolveReviewPathInstructions (#review-path-instructions)", () => { expect(bad.review.findingCategories).toBeNull(); expect(bad.warnings.some((w) => /review\.finding_categories.*must be a boolean/.test(w))).toBe(true); }); + + it("parses review.min_finding_severity, round-trips, and warns on invalid values (#2048)", () => { + const major = parseFocusManifest({ review: { min_finding_severity: "major" } }); + expect(major.review.minFindingSeverity).toBe("major"); + expect(major.review.present).toBe(true); + expect(parseFocusManifest({ review: reviewConfigToJson(major.review) }).review.minFindingSeverity).toBe("major"); + expect(parseFocusManifest({ review: {} }).review.minFindingSeverity).toBeNull(); + const bad = parseFocusManifest({ review: { min_finding_severity: "urgent" } }); + expect(bad.review.minFindingSeverity).toBeNull(); + expect(bad.warnings.some((w) => /review\.min_finding_severity/.test(w))).toBe(true); + expect(resolveReviewPromptOverrides(major).minFindingSeverity).toBe("major"); + expect(resolveReviewPromptOverrides(parseFocusManifest({})).minFindingSeverity).toBeNull(); + }); }); describe("review.exclude_paths (#review-exclude-paths)", () => { diff --git a/test/unit/inline-comments.test.ts b/test/unit/inline-comments.test.ts index 4a72281f66..f3451880a1 100644 --- a/test/unit/inline-comments.test.ts +++ b/test/unit/inline-comments.test.ts @@ -100,6 +100,20 @@ describe("selectInlineComments (#inline-comments)", () => { expect(out).toEqual([{ path: "src/a.ts", line: 1, side: "RIGHT", body: "**Nit:** First." }]); }); + it("drops inline nits below review.min_finding_severity while keeping blockers (#2048)", () => { + const out = selectInlineComments( + [ + { path: "src/a.ts", line: 2, severity: "blocker", body: "Must fix." }, + { path: "src/a.ts", line: 2, severity: "nit", body: "Style only." }, + ], + files, + false, + false, + "major", + ); + expect(out).toEqual([{ path: "src/a.ts", line: 2, side: "RIGHT", body: "**Blocker:** Must fix." }]); + }); + it("caps the output at 10 comments", () => { const bigPatch = "@@ -1,0 +1,12 @@\n" + Array.from({ length: 12 }, (_, i) => `+line${i + 1}`).join("\n"); const bigFiles = [{ path: "src/big.ts", payload: { patch: bigPatch } }]; diff --git a/test/unit/signals-coverage.test.ts b/test/unit/signals-coverage.test.ts index 4a052340ca..ef1877d937 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, 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: [], gif: false }, 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, minFindingSeverity: 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: [], gif: false }, 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