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
2 changes: 1 addition & 1 deletion .gittensory.yml.example
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ review:
# suggestions: false
# When true, inline findings with precise fixes render as one-click suggestion blocks (requires inline_comments).
# finding_categories: false
# When true, inline findings are tagged with a category label (requires inline_comments).
# inline_comments_per_category: 3

# Fix-handoff blocks (#2176). Bool | null. Default: null/false — byte-identical.
# Requires operator flag GITTENSORY_REVIEW_FIX_HANDOFF + cutover allowlist AND this toggle.
Expand Down
2 changes: 1 addition & 1 deletion config/examples/gittensory.full.yml
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ review:
# suggestions: false
# When true, inline findings with precise fixes render as one-click suggestion blocks (requires inline_comments).
# finding_categories: false
# When true, inline findings are tagged with a category label (requires inline_comments).
# inline_comments_per_category: 3

# Fix-handoff blocks (#2176). Bool | null. Default: null/false — byte-identical.
# Requires operator flag GITTENSORY_REVIEW_FIX_HANDOFF + cutover allowlist AND this toggle.
Expand Down
3 changes: 3 additions & 0 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8004,6 +8004,7 @@ async function maybePublishPrPublicSurface(
let reviewMemoryEnabledForReview = false;
let findingCategoriesEnabledForReview = false;
let minFindingSeverityForReview: ReviewFindingSeverity | null = null;
let inlineCommentsPerCategoryForReview: number | null = null;
let aiReviewExpected = false;
let aiReviewWasReused = false;
let gateFinalized = false;
Expand Down Expand Up @@ -8515,6 +8516,7 @@ async function maybePublishPrPublicSurface(
changedFilesSummaryEnabledForReview = deterministicReviewOverrides.changedFilesSummary;
effortScoreEnabledForReview = deterministicReviewOverrides.effortScore;
minFindingSeverityForReview = deterministicReviewOverrides.minFindingSeverity;
inlineCommentsPerCategoryForReview = deterministicReviewOverrides.inlineCommentsPerCategory;
// review.memory (#2179, part of #1964): deterministic, no-AI -- resolved the same unconditional way as
// changed_files_summary/effort_score above (must apply even when the AI review itself is skipped this
// pass). ANDed with the operator's GITTENSORY_REVIEW_MEMORY kill-switch at the actual apply site below
Expand Down Expand Up @@ -9884,6 +9886,7 @@ async function maybePublishPrPublicSurface(
suggestionsEnabled: suggestionsEnabledForReview,
categoriesEnabled: findingCategoriesEnabledForReview,
minFindingSeverity: minFindingSeverityForReview,
perCategoryCap: inlineCommentsPerCategoryForReview,
});
}
if (decision.willLabel) {
Expand Down
120 changes: 120 additions & 0 deletions src/review/inline-comments-select.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
/** Pure inline-comment selection with optional per-category caps (#2159). */

import { classifyFindingCategory, type FindingCategory } 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 { PullRequestFileRecord } from "../types";

export const DEFAULT_MAX_INLINE_COMMENTS = 10;

/** PURE: the set of NEW-file (RIGHT-side) line numbers a unified-diff patch makes commentable. */
export function rightSideLinesFromPatch(patch: string): Set<number> {
const lines = new Set<number>();
let right = 0;
for (const raw of patch.split("\n")) {
const header = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(raw);
if (header?.[1]) {
right = Number.parseInt(header[1], 10);
continue;
}
if (right === 0) continue;
const marker = raw[0];
if (marker === undefined || marker === "-" || marker === "\\") continue;
lines.add(right);
right += 1;
}
return lines;
}

/** Higher-priority categories survive per-category and total caps first (#2159). */
const INLINE_COMMENT_CATEGORY_PRIORITY: Record<FindingCategory, number> = {
security: 0,
correctness: 1,
performance: 2,
maintainability: 3,
tests: 4,
style: 5,
};

export function inlineFindingCategory(finding: InlineFinding): FindingCategory {
return finding.category ?? classifyFindingCategory(finding);
}

/** Lower rank sorts earlier. Blockers always beat nits; ties break on category priority. */
export function compareInlineFindingPriority(left: InlineFinding, right: InlineFinding): number {
const leftSeverity = left.severity === "blocker" ? 0 : 1;
const rightSeverity = right.severity === "blocker" ? 0 : 1;
if (leftSeverity !== rightSeverity) return leftSeverity - rightSeverity;
const leftCategory = INLINE_COMMENT_CATEGORY_PRIORITY[inlineFindingCategory(left)];
const rightCategory = INLINE_COMMENT_CATEGORY_PRIORITY[inlineFindingCategory(right)];
return leftCategory - rightCategory;
}

export type InlineCommentSelectOptions = {
suggestionsEnabled?: boolean | undefined;
categoriesEnabled?: boolean | undefined;
minFindingSeverity?: ReviewFindingSeverity | null | undefined;
/** When unset, preserve first-seen order with only the total cap (#2159 default-off). */
perCategoryCap?: number | null | undefined;
maxComments?: number | undefined;
};

type AnchoredInlineFinding = { finding: InlineFinding; index: number };

function anchorableInlineFindings(
findings: InlineFinding[],
files: Pick<PullRequestFileRecord, "path" | "payload">[],
minFindingSeverity: ReviewFindingSeverity | null | undefined,
): AnchoredInlineFinding[] {
const rightLinesByPath = new Map<string, Set<number>>();
for (const file of files) {
const patch = typeof file.payload?.patch === "string" ? file.payload.patch : "";
if (patch) rightLinesByPath.set(file.path, rightSideLinesFromPatch(patch));
}
const out: AnchoredInlineFinding[] = [];
const seen = new Set<string>();
for (let index = 0; index < findings.length; index++) {
const finding = findings[index]!;
if (!shouldShowInlineFinding(finding.severity, minFindingSeverity)) continue;
const validLines = rightLinesByPath.get(finding.path);
if (!validLines || !validLines.has(finding.line)) continue;
const key = `${finding.path}:${finding.line}`;
if (seen.has(key)) continue;
seen.add(key);
out.push({ finding, index });
}
return out;
}

/** Select anchorable inline findings, optionally applying a per-category sub-cap before the total cap. */
export function selectAnchoredInlineFindings(
findings: InlineFinding[],
files: Pick<PullRequestFileRecord, "path" | "payload">[],
options: InlineCommentSelectOptions,
): InlineFinding[] {
const anchored = anchorableInlineFindings(findings, files, options.minFindingSeverity);
const maxComments = options.maxComments ?? DEFAULT_MAX_INLINE_COMMENTS;
const perCategoryCap = options.perCategoryCap;
const ordered =
perCategoryCap == null
? anchored
: [...anchored].sort((left, right) => {
const byPriority = compareInlineFindingPriority(left.finding, right.finding);
if (byPriority !== 0) return byPriority;
return left.index - right.index;
});
const perCategoryCounts = new Map<FindingCategory, number>();
const out: InlineFinding[] = [];
for (const { finding } of ordered) {
if (out.length >= maxComments) break;
if (perCategoryCap != null) {
const category = inlineFindingCategory(finding);
const count = perCategoryCounts.get(category) ?? 0;
if (count >= perCategoryCap) continue;
perCategoryCounts.set(category, count + 1);
}
out.push(finding);
}
return out;
}
62 changes: 17 additions & 45 deletions src/review/inline-comments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
import { createPullRequestReviewComments } from "../github/pr-actions";
import { isConvergenceRepoAllowed } from "./cutover-gate";
import { classifyFindingCategory } from "./finding-category-classify";
import { shouldShowInlineFinding } from "./finding-severity-filter";
import { selectAnchoredInlineFindings } from "./inline-comments-select";
export { rightSideLinesFromPatch } from "./inline-comments-select";
import type { InlineFinding } from "../services/ai-review";
import type { ReviewFindingSeverity } from "../signals/focus-manifest";
import type { AgentActionMode } from "../settings/agent-execution";
Expand Down Expand Up @@ -63,32 +64,6 @@ export type ReviewInlineComment = { path: string; line: number; side: "RIGHT"; b

/** Hard cap on inline comments posted per PR review — a focused review leaves a handful of precise notes, not a
* wall (the model is also asked to be selective, and composeInlineFindings already caps at 10). */
const MAX_INLINE_COMMENTS = 10;

/** PURE: the set of NEW-file (RIGHT-side) line numbers a unified-diff patch makes commentable — every added
* ("+") and context (" ") line inside a hunk. GitHub 422s an inline comment whose line is NOT one of these, so
* {@link selectInlineComments} validates each finding against this set. Deleted ("-") lines are LEFT-side only
* and excluded; the "\ No newline at end of file" marker is skipped. Mirrors firstAddedLineFromPatch's
* hunk-header regex (advisory.ts). */
export function rightSideLinesFromPatch(patch: string): Set<number> {
const lines = new Set<number>();
let right = 0;
for (const raw of patch.split("\n")) {
const header = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(raw);
if (header?.[1]) {
right = Number.parseInt(header[1], 10);
continue;
}
if (right === 0) continue; // preamble before the first hunk header
const marker = raw[0];
// `undefined` ⇒ an empty "" element (a trailing-newline split artifact, NOT a real diff line — a blank
// context line is " ", a single space); "-" ⇒ deleted (LEFT side only); "\\" ⇒ the "no newline" marker.
if (marker === undefined || marker === "-" || marker === "\\") continue;
lines.add(right); // added ("+") or context (" ") line → occupies a RIGHT-side line number
right += 1;
}
return lines;
}

/** GitHub's suggested-change syntax requires the LITERAL ` ```suggestion ` fence; if the suggestion text itself
* contains a triple-backtick run, embedding it verbatim would prematurely close the fence and corrupt the
Expand Down Expand Up @@ -128,25 +103,18 @@ export function selectInlineComments(
suggestionsEnabled = false,
categoriesEnabled = false,
minFindingSeverity: ReviewFindingSeverity | null | undefined = null,
perCategoryCap: number | 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 : "";
if (patch) rightLinesByPath.set(file.path, rightSideLinesFromPatch(patch));
}
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)
const key = `${finding.path}:${finding.line}`;
if (seen.has(key)) continue;
seen.add(key);
out.push({ path: finding.path, line: finding.line, side: "RIGHT", body: formatInlineBody(finding, suggestionsEnabled, categoriesEnabled) });
}
return out;
const selected = selectAnchoredInlineFindings(findings, files, {
minFindingSeverity,
perCategoryCap,
});
return selected.map((finding) => ({
path: finding.path,
line: finding.line,
side: "RIGHT" as const,
body: formatInlineBody(finding, suggestionsEnabled, categoriesEnabled),
}));
}

/** Post the model's inline findings as ONE quiet, non-blocking review (`event: COMMENT`) on the PR. Fully
Expand All @@ -166,6 +134,7 @@ export async function postInlineReviewComments(
suggestionsEnabled?: boolean | undefined;
categoriesEnabled?: boolean | undefined;
minFindingSeverity?: ReviewFindingSeverity | null | undefined;
perCategoryCap?: number | null | undefined;
},
): Promise<{ posted: number }> {
const comments = selectInlineComments(
Expand All @@ -174,6 +143,7 @@ export async function postInlineReviewComments(
args.suggestionsEnabled,
args.categoriesEnabled,
args.minFindingSeverity,
args.perCategoryCap,
);
if (comments.length === 0 || !args.commitId) return { posted: 0 };
try {
Expand Down Expand Up @@ -205,6 +175,7 @@ export async function maybePostInlineComments(
suggestionsEnabled?: boolean | undefined;
categoriesEnabled?: boolean | undefined;
minFindingSeverity?: ReviewFindingSeverity | null | undefined;
perCategoryCap?: number | null | undefined;
},
): Promise<void> {
if (!args.inlineCommentsEnabled) return;
Expand All @@ -221,5 +192,6 @@ export async function maybePostInlineComments(
suggestionsEnabled: args.suggestionsEnabled,
categoriesEnabled: args.categoriesEnabled,
minFindingSeverity: args.minFindingSeverity,
perCategoryCap: args.perCategoryCap,
});
}
Loading