Skip to content
Closed
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
7 changes: 7 additions & 0 deletions .gittensory.yml.example
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,13 @@ review:
# configured level are suppressed from inline comments — never from gate blockers. Default: null (show all).
# min_finding_severity: major

# Display-only caps on how many blockers/nits render in the unified comment. Each field is a non-negative integer;
# unset within the object ⇒ no cap for that list. Whole object absent ⇒ legacy 12-item cap (byte-identical).
# Never affects gate decisions. Default: absent.
# max_findings:
# blockers: 5
# nits: 10

# Inline-comment layer toggles (#1956 / #1958). Bool | null. Default: null/false — byte-identical.
# Requires operator flag GITTENSORY_REVIEW_INLINE_COMMENTS + cutover allowlist + review.inline_comments: true.
# inline_comments: false
Expand Down
7 changes: 7 additions & 0 deletions config/examples/gittensory.full.yml
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,13 @@ review:
# configured level are suppressed from inline comments — never from gate blockers. Default: null (show all).
# min_finding_severity: major

# Display-only caps on how many blockers/nits render in the unified comment. Each field is a non-negative integer;
# unset within the object ⇒ no cap for that list. Whole object absent ⇒ legacy 12-item cap (byte-identical).
# Never affects gate decisions. Default: absent.
# max_findings:
# blockers: 5
# nits: 10

# Inline-comment layer toggles (#1956 / #1958). Bool | null. Default: null/false — byte-identical.
# Requires operator flag GITTENSORY_REVIEW_INLINE_COMMENTS + cutover allowlist + review.inline_comments: true.
# inline_comments: false
Expand Down
7 changes: 7 additions & 0 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,8 @@ import {
type ReviewPathInstruction,
type ReviewProfile,
type ReviewFindingSeverity,
type MaxFindingsConfig,
maxFindingsPresent,
type SelfHostAiModelConfig,
type VisualConfig,
} from "../signals/focus-manifest";
Expand Down Expand Up @@ -7754,6 +7756,7 @@ async function maybePublishPrPublicSurface(
let effortScoreEnabledForReview = false;
let findingCategoriesEnabledForReview = false;
let minFindingSeverityForReview: ReviewFindingSeverity | null = null;
let maxFindingsForReview: MaxFindingsConfig | undefined;
let aiReviewExpected = false;
let aiReviewWasReused = false;
let gateFinalized = false;
Expand Down Expand Up @@ -8258,6 +8261,9 @@ async function maybePublishPrPublicSurface(
changedFilesSummaryEnabledForReview = deterministicReviewOverrides.changedFilesSummary;
effortScoreEnabledForReview = deterministicReviewOverrides.effortScore;
minFindingSeverityForReview = deterministicReviewOverrides.minFindingSeverity;
if (maxFindingsPresent(deterministicReviewOverrides.maxFindings)) {
maxFindingsForReview = deterministicReviewOverrides.maxFindings;
}
const aiReviewWillRun =
!authorBlacklisted &&
!isFrozenForManualReview &&
Expand Down Expand Up @@ -9496,6 +9502,7 @@ async function maybePublishPrPublicSurface(
...(findingCategoriesEnabledForReview && aiReview?.inlineFindings?.length
? { findingCategories: aiReview.inlineFindings }
: {}),
...(maxFindingsForReview !== undefined ? { maxFindings: maxFindingsForReview } : {}),
});
} else {
deterministicBody = buildPublicPrIntelligenceComment(commentArgs);
Expand Down
3 changes: 3 additions & 0 deletions src/review/unified-comment-bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,8 @@ export type UnifiedCommentBridgeArgs = {
* `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;
/** Display-only caps on rendered blockers/nits (`review.max_findings` port). Omitted ⇒ legacy 12-item cap. (#2049) */
maxFindings?: { blockers: number | null; nits: number | null } | 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;
Expand Down Expand Up @@ -608,6 +610,7 @@ export function buildUnifiedCommentBody(args: UnifiedCommentBridgeArgs): string
...(args.heldForReview ? { heldForReview: true } : {}),
...(args.neverClosed ? { neverClosed: true } : {}),
...(args.preflightHeld ? { preflightHeld: true } : {}),
...(args.maxFindings !== undefined ? { maxFindings: args.maxFindings } : {}),
});

// Prepend the marker verbatim (matching the legacy body, which leads with the marker then a blank line)
Expand Down
68 changes: 54 additions & 14 deletions src/review/unified-comment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,8 @@ export interface UnifiedCommentContext {
preflightHeld?: boolean;
/** Public freshness marker for the posted/updated review comment. Rendered as UTC when provided. */
reviewedAt?: string | number | Date | undefined;
/** Display-only caps on rendered blockers/nits (`review.max_findings`). Omitted ⇒ legacy 12-item cap per list. (#2049) */
maxFindings?: { blockers: number | null; nits: number | null } | undefined;
}

const STATUS_META: Record<UnifiedCommentStatus, { alert: string; square: string; icon: string }> = {
Expand Down Expand Up @@ -361,8 +363,36 @@ function verdictLine(status: UnifiedCommentStatus, input: UnifiedReviewInput, ct
}
}

/** Dedupe + cap a list of lines (case-insensitive), so blockers/nits never balloon the comment. */
function dedupeLines(items: string[], cap = 12): string[] {
/** Legacy unified-comment display cap when `review.max_findings` is absent (byte-identical). */
export const LEGACY_FINDINGS_DISPLAY_CAP = 12;

/** Truncate a deduped findings list for display. `cap === null` ⇒ show all lines. */
export function truncateDisplayedFindingLines(
lines: readonly string[],
cap: number | null,
): { visible: string[]; omitted: number } {
if (cap === null || lines.length <= cap) {
return { visible: [...lines], omitted: 0 };
}
return { visible: lines.slice(0, cap), omitted: lines.length - cap };
}

function resolveFindingsDisplayCap(
maxFindings: UnifiedCommentContext["maxFindings"],
kind: "blockers" | "nits",
): number | null {
if (!maxFindings) return LEGACY_FINDINGS_DISPLAY_CAP;
return maxFindings[kind];
}

/** Escape angle brackets in caller-provided public text so raw HTML, HTML comments,
* or stray closing tags cannot change the GitHub comment structure. */
function escapePublicHtmlAngles(text: string): string {
return text.replace(/[<>]/g, (char) => (char === "<" ? "&lt;" : "&gt;"));
}

/** Dedupe a list of lines (case-insensitive) so blockers/nits never repeat in the comment. */
function dedupeLines(items: string[], cap?: number): string[] {
const seen = new Set<string>();
const out: string[] = [];
for (const raw of items) {
Expand All @@ -372,17 +402,11 @@ function dedupeLines(items: string[], cap = 12): string[] {
if (seen.has(key)) continue;
seen.add(key);
out.push(line);
if (out.length >= cap) break;
if (cap !== undefined && out.length >= cap) break;
}
return out;
}

/** Escape angle brackets in caller-provided public text so raw HTML, HTML comments,
* or stray closing tags cannot change the GitHub comment structure. */
function escapePublicHtmlAngles(text: string): string {
return text.replace(/[<>]/g, (char) => (char === "<" ? "&lt;" : "&gt;"));
}

function bullets(items: string[]): string {
return dedupeLines(items)
.map((i) => `- ${escapePublicHtmlAngles(i)}`)
Expand Down Expand Up @@ -504,13 +528,29 @@ export function renderUnifiedReviewComment(input: UnifiedReviewInput, ctx: Unifi

if (input.summary.trim()) blocks.push(`**Review summary**\n${escapePublicHtmlAngles(input.summary.trim())}`);

const nits = dedupeLines(input.nits ?? []);
if (nits.length) blocks.push(details("Nits", taskList(nits), `${nits.length} non-blocking`));
const nitsDeduped = dedupeLines(input.nits ?? []);
if (nitsDeduped.length) {
const nitsTrunc = truncateDisplayedFindingLines(nitsDeduped, resolveFindingsDisplayCap(ctx.maxFindings, "nits"));
const nitsBody =
taskList(nitsTrunc.visible) + (nitsTrunc.omitted > 0 ? `\n\n_+${nitsTrunc.omitted} more nit(s) not shown._` : "");
const nitsSub =
nitsTrunc.omitted > 0
? `${nitsTrunc.visible.length} non-blocking (+${nitsTrunc.omitted} more)`
: `${nitsDeduped.length} non-blocking`;
blocks.push(details("Nits", nitsBody, nitsSub));
}

const blockers = dedupeLines(input.blockers ?? []);
if (blockers.length) {
const blockersDeduped = dedupeLines(input.blockers ?? []);
if (blockersDeduped.length) {
const blockersTrunc = truncateDisplayedFindingLines(
blockersDeduped,
resolveFindingsDisplayCap(ctx.maxFindings, "blockers"),
);
const heading = status === "blocked" ? "Why this is blocked" : "Concerns raised — review before merging";
blocks.push(`**${heading}**\n${bullets(blockers)}`);
const blockersBody =
bullets(blockersTrunc.visible) +
(blockersTrunc.omitted > 0 ? `\n\n_+${blockersTrunc.omitted} more blocker(s) not shown._` : "");
blocks.push(`**${heading}**\n${blockersBody}`);
}

// Failing CI checks — list WHICH checks failed and WHY (codecov %/test/lint reason) under the "CI failing"
Expand Down
Loading
Loading