Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .gittensory.yml.example
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions config/examples/gittensory.full.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,7 @@ import {
type FocusManifest,
type ReviewPathInstruction,
type ReviewProfile,
type ReviewFindingSeverity,
type SelfHostAiModelConfig,
type VisualConfig,
} from "../signals/focus-manifest";
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -8187,6 +8189,7 @@ async function maybePublishPrPublicSurface(
const deterministicReviewOverrides = resolveReviewPromptOverrides(reviewManifestForAutoReview);
changedFilesSummaryEnabledForReview = deterministicReviewOverrides.changedFilesSummary;
effortScoreEnabledForReview = deterministicReviewOverrides.effortScore;
minFindingSeverityForReview = deterministicReviewOverrides.minFindingSeverity;
const aiReviewWillRun =
!authorBlacklisted &&
!isFrozenForManualReview &&
Expand Down Expand Up @@ -9481,6 +9484,7 @@ async function maybePublishPrPublicSurface(
inlineCommentsEnabled: inlineCommentsEnabledForReview,
suggestionsEnabled: suggestionsEnabledForReview,
categoriesEnabled: findingCategoriesEnabledForReview,
minFindingSeverity: minFindingSeverityForReview,
});
}
if (decision.willLabel) {
Expand Down
29 changes: 29 additions & 0 deletions src/review/finding-severity-filter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import type { ReviewFindingSeverity } from "../signals/focus-manifest";

const SEVERITY_RANK: Record<ReviewFindingSeverity, number> = {
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);
}
22 changes: 20 additions & 2 deletions src/review/inline-comments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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<PullRequestFileRecord, "path" | "payload">[], suggestionsEnabled = false, categoriesEnabled = false): ReviewInlineComment[] {
export function selectInlineComments(
findings: InlineFinding[],
files: Pick<PullRequestFileRecord, "path" | "payload">[],
suggestionsEnabled = false,
categoriesEnabled = false,
minFindingSeverity: ReviewFindingSeverity | null | undefined = null,
): ReviewInlineComment[] {
const rightLinesByPath = new Map<string, Set<number>>();
for (const file of files) {
const patch = typeof file.payload?.patch === "string" ? file.payload.patch : "";
Expand All @@ -129,6 +137,7 @@ export function selectInlineComments(findings: InlineFinding[], files: Pick<Pull
const out: ReviewInlineComment[] = [];
const seen = new Set<string>();
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)
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -188,6 +204,7 @@ export async function maybePostInlineComments(
inlineCommentsEnabled: boolean;
suggestionsEnabled?: boolean | undefined;
categoriesEnabled?: boolean | undefined;
minFindingSeverity?: ReviewFindingSeverity | null | undefined;
},
): Promise<void> {
if (!args.inlineCommentsEnabled) return;
Expand All @@ -203,5 +220,6 @@ export async function maybePostInlineComments(
mode: args.mode,
suggestionsEnabled: args.suggestionsEnabled,
categoriesEnabled: args.categoriesEnabled,
minFindingSeverity: args.minFindingSeverity,
});
}
27 changes: 22 additions & 5 deletions src/signals/focus-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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[];
Expand Down Expand Up @@ -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 },
Expand Down Expand Up @@ -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 },
Expand Down Expand Up @@ -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.`);
Expand Down Expand Up @@ -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);
Expand All @@ -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 ||
Expand Down Expand Up @@ -1748,6 +1763,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo
changedFilesSummary,
effortScore,
findingCategories,
minFindingSeverity,
pathInstructions,
instructions,
excludePaths,
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/unit/config-templates.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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/);
Expand Down
Loading
Loading