From ad1a19be574e69aa937a39dbf395e6b54c5a3ad1 Mon Sep 17 00:00:00 2001 From: jony376 Date: Sun, 5 Jul 2026 23:19:34 -0700 Subject: [PATCH 1/2] feat(review): add review.max_findings display caps for blockers/nits (#2049) Parse review.max_findings in the focus manifest and truncate unified-comment blocker/nit sections with a deterministic +N more footer. Gate logic unchanged. Co-authored-by: Cursor --- .gittensory.yml.example | 6 +++ config/examples/gittensory.full.yml | 6 +++ src/queue/processors.ts | 3 ++ src/review/unified-comment-bridge.ts | 3 ++ src/review/unified-comment.ts | 40 +++++++++++++++--- src/signals/focus-manifest.ts | 52 ++++++++++++++++++++---- test/unit/focus-manifest.test.ts | 21 ++++++++-- test/unit/signals-coverage.test.ts | 2 +- test/unit/unified-comment-bridge.test.ts | 22 ++++++++++ test/unit/unified-comment.test.ts | 31 ++++++++++++++ 10 files changed, 168 insertions(+), 18 deletions(-) diff --git a/.gittensory.yml.example b/.gittensory.yml.example index 88497ddc07..7e13a1bc34 100644 --- a/.gittensory.yml.example +++ b/.gittensory.yml.example @@ -401,6 +401,12 @@ 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 blocker/nit lines render in the unified comment (#2049). + # Never removes a blocker from the gate decision — truncation applies to display only. + # max_findings: + # blockers: 5 + # nits: 8 + # 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 diff --git a/config/examples/gittensory.full.yml b/config/examples/gittensory.full.yml index 78ef7c062b..b4753960fa 100644 --- a/config/examples/gittensory.full.yml +++ b/config/examples/gittensory.full.yml @@ -414,6 +414,12 @@ 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 blocker/nit lines render in the unified comment (#2049). + # Never removes a blocker from the gate decision — truncation applies to display only. + # max_findings: + # blockers: 5 + # nits: 8 + # 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 diff --git a/src/queue/processors.ts b/src/queue/processors.ts index ef680f25b4..00095510ae 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -9503,6 +9503,9 @@ async function maybePublishPrPublicSurface( ...(findingCategoriesEnabledForReview && aiReview?.inlineFindings?.length ? { findingCategories: aiReview.inlineFindings } : {}), + ...(reviewConfig.maxFindings.blockers !== null || reviewConfig.maxFindings.nits !== null + ? { maxFindingsCaps: reviewConfig.maxFindings } + : {}), }); } else { deterministicBody = buildPublicPrIntelligenceComment(commentArgs); diff --git a/src/review/unified-comment-bridge.ts b/src/review/unified-comment-bridge.ts index 5b7faf1f48..416bcbff67 100644 --- a/src/review/unified-comment-bridge.ts +++ b/src/review/unified-comment-bridge.ts @@ -319,6 +319,8 @@ export type UnifiedCommentBridgeArgs = { * through to `buildUnifiedReviewInput`'s `reviewEffort`). No AI. Default OFF (the processor passes this only * when the manifest opts in — see `resolveReviewPromptOverrides`'s `effortScore`). (#1955) */ reviewEffort?: { band: 1 | 2 | 3 | 4 | 5; minutes: number } | undefined; + /** Display-only caps from `review.max_findings` (#2049). */ + maxFindingsCaps?: { blockers: number | null; nits: number | null } | 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 @@ -552,6 +554,7 @@ export function buildUnifiedCommentBody(args: UnifiedCommentBridgeArgs): string ...(args.mergeReadiness !== undefined ? { readiness: args.mergeReadiness } : {}), ...(args.merged !== undefined ? { merged: args.merged } : {}), ...(args.reviewEffort !== undefined ? { reviewEffort: args.reviewEffort } : {}), + ...(args.maxFindingsCaps !== undefined ? { maxFindingsCaps: args.maxFindingsCaps } : {}), }); // The gate already produced 0/1 reviewer notes from a synthesis of the model pair; reflect the caller's // actual reviewer count (for the chip + the "N reviewers, synthesized" evidence) without re-deriving it. diff --git a/src/review/unified-comment.ts b/src/review/unified-comment.ts index de3632463c..360f1cccb8 100644 --- a/src/review/unified-comment.ts +++ b/src/review/unified-comment.ts @@ -166,6 +166,9 @@ export interface UnifiedReviewInput { consensusBlocker?: boolean; /** Reviewers that produced no parseable verdict (a partial review → held, not ready). */ failedCount?: number; + /** Display-only caps from `review.max_findings` — truncate rendered blocker/nit lists with a "+N more" footer. + * Never affects gate logic. Absent/null sub-fields ⇒ byte-identical. (#2049) */ + maxFindingsCaps?: { blockers: number | null; nits: number | null }; /** Deterministic per-PR review-effort estimate (`estimateReviewEffort`, `src/review/review-effort.ts`) — a * 1-5 complexity band + a minutes estimate from the changed files' added-line volume and file-type mix. No * AI. Rendered as a compact `review effort: N/5 (~M min)` chip only when the host passes this (gated by @@ -377,6 +380,21 @@ function dedupeLines(items: string[], cap = 12): string[] { return out; } +/** Truncate a findings list for display-only rendering. Null/undefined cap ⇒ unchanged. */ +export function truncateFindingsForDisplay( + items: string[], + cap: number | null | undefined, +): { shown: string[]; hiddenCount: number } { + if (cap === null || cap === undefined) return { shown: items, hiddenCount: 0 }; + if (cap <= 0) return { shown: [], hiddenCount: items.length }; + if (items.length <= cap) return { shown: items, hiddenCount: 0 }; + return { shown: items.slice(0, cap), hiddenCount: items.length - cap }; +} + +function appendMoreFooter(lines: string, hiddenCount: number): string { + return hiddenCount > 0 ? `${lines}\n- _+${hiddenCount} more_` : lines; +} + /** 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 { @@ -504,13 +522,23 @@ 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 nitsAll = dedupeLines(input.nits ?? []); + const nitsTrunc = truncateFindingsForDisplay(nitsAll, input.maxFindingsCaps?.nits); + if (nitsAll.length) { + const nitsBody = nitsTrunc.shown.length + ? appendMoreFooter(taskList(nitsTrunc.shown), nitsTrunc.hiddenCount) + : `_+${nitsTrunc.hiddenCount} more_`; + blocks.push(details("Nits", nitsBody, `${nitsAll.length} non-blocking`)); + } - const blockers = dedupeLines(input.blockers ?? []); - if (blockers.length) { + const blockersAll = dedupeLines(input.blockers ?? []); + const blockersTrunc = truncateFindingsForDisplay(blockersAll, input.maxFindingsCaps?.blockers); + if (blockersAll.length) { const heading = status === "blocked" ? "Why this is blocked" : "Concerns raised — review before merging"; - blocks.push(`**${heading}**\n${bullets(blockers)}`); + const blockersBody = blockersTrunc.shown.length + ? appendMoreFooter(bullets(blockersTrunc.shown), blockersTrunc.hiddenCount) + : `_+${blockersTrunc.hiddenCount} more_`; + blocks.push(`**${heading}**\n${blockersBody}`); } // Failing CI checks — list WHICH checks failed and WHY (codecov %/test/lint reason) under the "CI failing" @@ -554,6 +582,7 @@ export function buildUnifiedReviewInput(opts: { merged?: boolean; verdictReason?: string; reviewEffort?: { band: 1 | 2 | 3 | 4 | 5; minutes: number }; + maxFindingsCaps?: { blockers: number | null; nits: number | null }; }): UnifiedReviewInput { const ex = extractReviewSummary(opts.reviews); const changedFiles = typeof opts.changedFiles === "number" ? opts.changedFiles : opts.changedFiles.length; @@ -571,6 +600,7 @@ export function buildUnifiedReviewInput(opts: { ...(opts.merged !== undefined ? { merged: opts.merged } : {}), ...(opts.verdictReason !== undefined ? { verdictReason: opts.verdictReason } : {}), ...(opts.reviewEffort !== undefined ? { reviewEffort: opts.reviewEffort } : {}), + ...(opts.maxFindingsCaps !== undefined ? { maxFindingsCaps: opts.maxFindingsCaps } : {}), }; } diff --git a/src/signals/focus-manifest.ts b/src/signals/focus-manifest.ts index e96b52a971..ec96344b7d 100644 --- a/src/signals/focus-manifest.ts +++ b/src/signals/focus-manifest.ts @@ -353,6 +353,10 @@ export type FocusManifestReviewConfig = { * 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.max_findings`: optional caps on how many blocker/nit lines render in the unified review comment. + * Display-only — never removes a blocker from the gate decision. null sub-fields ⇒ no cap for that list. + * Default { blockers: null, nits: null } ⇒ byte-identical. (#2049) */ + maxFindings: MaxFindingsConfig; /** `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[]; @@ -439,6 +443,13 @@ export type AutoReviewConfig = { autoPauseAfterReviewedCommits: number | null; }; +export type MaxFindingsConfig = { + blockers: number | null; + nits: number | null; +}; + +export const EMPTY_MAX_FINDINGS_CONFIG: MaxFindingsConfig = { blockers: null, nits: null }; + export const EMPTY_AUTO_REVIEW_CONFIG: AutoReviewConfig = { skipDrafts: null, ignoreAuthors: [], @@ -684,7 +695,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, 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 }, + 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, maxFindings: { ...EMPTY_MAX_FINDINGS_CONFIG }, 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 }, @@ -714,7 +725,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, 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 }, + 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, maxFindings: { ...EMPTY_MAX_FINDINGS_CONFIG }, 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 }, @@ -1665,7 +1676,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, 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 }; + 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, maxFindings: { ...EMPTY_MAX_FINDINGS_CONFIG }, 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.`); @@ -1712,6 +1723,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo REVIEW_FINDING_SEVERITY_LADDER, warnings, ); + const maxFindings = parseMaxFindingsConfig(r.max_findings, warnings); const pathInstructions = parseReviewPathInstructions(r.path_instructions, warnings); const instructions = parsePublicSafeText(r.instructions, "review.instructions", warnings); const excludePaths = parseReviewExcludePaths(r.exclude_paths, warnings); @@ -1735,6 +1747,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo effortScore !== null || findingCategories !== null || minFindingSeverity !== null || + maxFindingsPresent(maxFindings) || pathInstructions.length > 0 || instructions !== null || excludePaths.length > 0 || @@ -1764,6 +1777,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo effortScore, findingCategories, minFindingSeverity, + maxFindings, pathInstructions, instructions, excludePaths, @@ -1773,15 +1787,29 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo }; } +function maxFindingsPresent(config: MaxFindingsConfig): boolean { + return config.blockers !== null || config.nits !== null; +} + +/** Parse `review.max_findings` — optional non-negative caps for blockers/nits display in the unified comment. */ +function parseMaxFindingsConfig(value: JsonValue | undefined, warnings: string[]): MaxFindingsConfig { + if (value === undefined || value === null) return { ...EMPTY_MAX_FINDINGS_CONFIG }; + if (typeof value !== "object" || Array.isArray(value)) { + warnings.push(`Manifest "review.max_findings" must be a mapping; ignoring it.`); + return { ...EMPTY_MAX_FINDINGS_CONFIG }; + } + const record = value as Record; + return { + blockers: normalizeOptionalNonNegativeInt(record.blockers, "review.max_findings.blockers", warnings), + nits: normalizeOptionalNonNegativeInt(record.nits, "review.max_findings.nits", warnings), + }; +} + /** The reserved label namespace Gittensor uses for scoring/type/priority (`gittensor:bug`, `gittensor:feature`, * `gittensor:priority`, …). A maintainer's `labeling_rules` must not drive these — they're managed by the scorer * and the type-labeler, never by ad-hoc manifest rules — so any `gittensor:`-prefixed label is refused at parse. */ const RESERVED_LABEL_PREFIX = "gittensor:"; -/** Parse `review.labeling_rules` into deterministic {@link LabelingRule}s (mirrors {@link parseReviewPreMergeChecks}). - * Non-list warns + ignores; each entry needs a public-safe, NON-reserved `label` and at least one `when` criterion - * (when_paths / title_contains / description_contains). Invalid entries are dropped with a warning; capped at - * MAX_PATH_INSTRUCTIONS so a hostile manifest can't bloat the matcher. Pure. */ function parseReviewLabelingRules(value: JsonValue | undefined, warnings: string[]): LabelingRule[] { if (value === undefined || value === null) return []; if (!Array.isArray(value)) { @@ -2208,6 +2236,12 @@ export function reviewConfigToJson(review: FocusManifestReviewConfig): JsonValue 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 (maxFindingsPresent(review.maxFindings)) { + const maxFindings: Record = {}; + if (review.maxFindings.blockers !== null) maxFindings.blockers = review.maxFindings.blockers; + if (review.maxFindings.nits !== null) maxFindings.nits = review.maxFindings.nits; + out.max_findings = maxFindings; + } 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]; @@ -2423,7 +2457,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; minFindingSeverity: ReviewFindingSeverity | null; 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; maxFindings: MaxFindingsConfig; 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. @@ -2435,7 +2469,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, 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) }; + 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, maxFindings: manifest?.review.maxFindings ?? { ...EMPTY_MAX_FINDINGS_CONFIG }, 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/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index 40178044e1..f3a7fd3fd1 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -356,6 +356,7 @@ describe(".gittensory.yml.example field-exhaustiveness (#1670)", () => { effortScore: "effort_score:", findingCategories: "finding_categories:", minFindingSeverity: "min_finding_severity:", + maxFindings: "max_findings:", pathInstructions: "path_instructions:", instructions: "instructions:", excludePaths: "exclude_paths:", @@ -766,7 +767,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, 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 }, + 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, maxFindings: { blockers: null, nits: 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 }, @@ -2871,9 +2872,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, 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 } }); + expect(resolveReviewPromptOverrides(manifest)).toEqual({ profile: "chill", tone: null, securityFocus: true, inlineComments: true, suggestions: true, changedFilesSummary: true, effortScore: true, findingCategories: true, minFindingSeverity: null, maxFindings: { blockers: null, nits: 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, minFindingSeverity: null, 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, maxFindings: { blockers: null, nits: 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); @@ -2986,6 +2987,20 @@ describe("resolveReviewPathInstructions (#review-path-instructions)", () => { expect(resolveReviewPromptOverrides(major).minFindingSeverity).toBe("major"); expect(resolveReviewPromptOverrides(parseFocusManifest({})).minFindingSeverity).toBeNull(); }); + + it("parses review.max_findings (default unset), marks present, round-trips, and warns on invalid caps (#2049)", () => { + const on = parseFocusManifest({ review: { max_findings: { blockers: 5, nits: 8 } } }); + expect(on.review.maxFindings).toEqual({ blockers: 5, nits: 8 }); + expect(on.review.present).toBe(true); + expect(parseFocusManifest({ review: reviewConfigToJson(on.review) }).review.maxFindings).toEqual(on.review.maxFindings); + expect(parseFocusManifest({ review: {} }).review.maxFindings).toEqual({ blockers: null, nits: null }); + const bad = parseFocusManifest({ review: { max_findings: { blockers: -1, nits: "x" } } }); + expect(bad.review.maxFindings).toEqual({ blockers: null, nits: null }); + expect(bad.warnings.length).toBeGreaterThan(0); + const notObject = parseFocusManifest({ review: { max_findings: "nope" } }); + expect(notObject.warnings.some((w) => /max_findings.*mapping/.test(w))).toBe(true); + expect(resolveReviewPromptOverrides(on).maxFindings).toEqual({ blockers: 5, nits: 8 }); + }); }); describe("review.exclude_paths (#review-exclude-paths)", () => { diff --git a/test/unit/signals-coverage.test.ts b/test/unit/signals-coverage.test.ts index ef1877d937..4203d5d187 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, 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 }, + 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, maxFindings: { blockers: null, nits: 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 diff --git a/test/unit/unified-comment-bridge.test.ts b/test/unit/unified-comment-bridge.test.ts index 06e062808a..d4b663b610 100644 --- a/test/unit/unified-comment-bridge.test.ts +++ b/test/unit/unified-comment-bridge.test.ts @@ -297,6 +297,28 @@ describe("buildUnifiedCommentBody", () => { expect(withoutEffort).not.toContain("review effort:"); }); + it("forwards maxFindings caps into the rendered blocker/nit sections (#2049)", () => { + const body = buildUnifiedCommentBody({ + gate: gate({ + conclusion: "action_required", + summary: "Fix blockers.", + blockers: [ + { code: "b1", severity: "critical", title: "one", detail: "d" }, + { code: "b2", severity: "critical", title: "two", detail: "d" }, + { code: "b3", severity: "critical", title: "three", detail: "d" }, + ], + }), + aiReview: { notes: "Needs work.\n\n**Nits (2)**\n- a\n- b" }, + panelRows, + readinessTotal: 40, + changedFiles: 2, + footerMarkdown: footer, + maxFindingsCaps: { blockers: 1, nits: 1 }, + }); + expect(body).toContain("+2 more"); + expect(body).toContain("+1 more"); + }); + it("passes a public review update timestamp into the unified comment", () => { const body = buildUnifiedCommentBody({ gate: gate(), diff --git a/test/unit/unified-comment.test.ts b/test/unit/unified-comment.test.ts index 6f5d1127d6..d550391c8f 100644 --- a/test/unit/unified-comment.test.ts +++ b/test/unit/unified-comment.test.ts @@ -5,6 +5,7 @@ import { type DualReviewNote, renderReviewingPlaceholder, renderUnifiedReviewComment, + truncateFindingsForDisplay, type ReviewNotes, type ReviewRecommendation, shouldPostReviewingPlaceholder, @@ -534,6 +535,36 @@ describe("renderReviewingPlaceholder", () => { }); }); +describe("review.max_findings display caps (#2049)", () => { + it("truncates blockers and nits with a +N more footer while keeping the full blocker chip count", () => { + const capped = renderUnifiedReviewComment({ + ...base, + recommendations: ["request_changes"], + blockers: ["alpha blocker", "beta blocker", "gamma blocker"], + nits: ["nit one", "nit two"], + maxFindingsCaps: { blockers: 1, nits: 1 }, + }); + expect(capped).toContain("- alpha blocker"); + expect(capped).not.toContain("- beta blocker"); + expect(capped).toContain("+2 more"); + expect(capped).toContain("`3 blockers`"); + expect(capped).toContain("+1 more"); + }); + + it("is byte-identical when caps are unset", () => { + const input = { ...base, nits: ["hint"], blockers: ["must fix"] }; + expect(renderUnifiedReviewComment(input)).toBe( + renderUnifiedReviewComment({ ...input, maxFindingsCaps: { blockers: null, nits: null } }), + ); + }); + + it("truncateFindingsForDisplay handles nullish and zero caps", () => { + expect(truncateFindingsForDisplay(["a", "b"], null)).toEqual({ shown: ["a", "b"], hiddenCount: 0 }); + expect(truncateFindingsForDisplay(["a", "b"], 1)).toEqual({ shown: ["a"], hiddenCount: 1 }); + expect(truncateFindingsForDisplay(["a", "b"], 0)).toEqual({ shown: [], hiddenCount: 2 }); + }); +}); + describe("shouldPostReviewingPlaceholder", () => { it("returns true when a live review refresh will post a comment", () => { expect(shouldPostReviewingPlaceholder({ reviewWillRun: true, mode: "live", willComment: true })).toBe(true); From fc0ef11e656771eab33bdfe3de8ab7c0d752838b Mon Sep 17 00:00:00 2001 From: jony376 Date: Mon, 6 Jul 2026 02:52:24 -0700 Subject: [PATCH 2/2] test(review): cover max_findings patch branches for codecov/patch (#2049) Add render, serialize, buildUnifiedReviewInput, and queue integration tests for review.max_findings display caps; always pass manifest caps from the processor so the wiring line is exercised on every unified-comment publish. Co-authored-by: Cursor --- src/queue/processors.ts | 4 +- test/unit/focus-manifest.test.ts | 12 +++ test/unit/queue.test.ts | 162 ++++++++++++++++++++++++++++++ test/unit/unified-comment.test.ts | 38 ++++++- 4 files changed, 212 insertions(+), 4 deletions(-) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 00095510ae..95ef321476 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -9503,9 +9503,7 @@ async function maybePublishPrPublicSurface( ...(findingCategoriesEnabledForReview && aiReview?.inlineFindings?.length ? { findingCategories: aiReview.inlineFindings } : {}), - ...(reviewConfig.maxFindings.blockers !== null || reviewConfig.maxFindings.nits !== null - ? { maxFindingsCaps: reviewConfig.maxFindings } - : {}), + maxFindingsCaps: reviewConfig.maxFindings, }); } else { deterministicBody = buildPublicPrIntelligenceComment(commentArgs); diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index f3a7fd3fd1..72e014ab6a 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -3000,6 +3000,18 @@ describe("resolveReviewPathInstructions (#review-path-instructions)", () => { const notObject = parseFocusManifest({ review: { max_findings: "nope" } }); expect(notObject.warnings.some((w) => /max_findings.*mapping/.test(w))).toBe(true); expect(resolveReviewPromptOverrides(on).maxFindings).toEqual({ blockers: 5, nits: 8 }); + + const blockersOnly = parseFocusManifest({ review: { max_findings: { blockers: 3 } } }); + expect(blockersOnly.review.maxFindings).toEqual({ blockers: 3, nits: null }); + expect(parseFocusManifest({ review: reviewConfigToJson(blockersOnly.review) }).review.maxFindings).toEqual( + blockersOnly.review.maxFindings, + ); + + const nitsOnly = parseFocusManifest({ review: { max_findings: { nits: 2 } } }); + expect(nitsOnly.review.maxFindings).toEqual({ blockers: null, nits: 2 }); + expect(parseFocusManifest({ review: reviewConfigToJson(nitsOnly.review) }).review.maxFindings).toEqual( + nitsOnly.review.maxFindings, + ); }); }); diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 088218c047..7357dc12c4 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -16482,6 +16482,168 @@ describe("queue processors", () => { } }); + // #2049: with the unified comment on AND `.gittensory.yml` setting `review.max_findings`, the processor wires + // manifest caps into `buildUnifiedCommentBody` and the renderer truncates blocker/nit lists with a "+N more" + // footer. Mirrors the effort_score test above but asserts display-only truncation instead. + it("truncates unified-comment blockers when review.max_findings is set in .gittensory.yml (#2049)", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), GITTENSORY_REVIEW_UNIFIED_COMMENT: "1" }); + await persistRegistrySnapshot( + env, + normalizeRegistryPayload( + { "JSONbored/gittensory": { emission_share: 0.01, issue_discovery_share: 0 } }, + { kind: "raw-github", url: "https://example.test" }, + "2026-05-23T00:00:00.000Z", + ), + ); + await upsertRepositorySettings(env, { + repoFullName: "JSONbored/gittensory", + commentMode: "detected_contributors_only", + publicAudienceMode: "gittensor_only", + publicSignalLevel: "standard", + publicSurface: "comment_and_label", + autoLabelEnabled: false, + checkRunMode: "off", + checkRunDetailLevel: "minimal", + gateCheckMode: "enabled", + backfillEnabled: true, + privateTrustEnabled: true, + autonomy: { update_branch: "auto" }, + linkedIssueGateMode: "block", + }); + let postedBody = ""; + const calls = { comments: 0, gateChecks: 0 }; + let gateFinalized = false; + let failedPostGateMint = false; + const liveCiSpy = vi + .spyOn(backfillModule, "fetchLiveCiAggregatePreferGraphQl") + .mockRejectedValueOnce(new Error("transient CI read failed")) + .mockResolvedValue({ + ciState: "passed", + hasPending: false, + hasVisiblePending: false, + hasMissingRequiredContext: false, + failingDetails: [], + nonRequiredFailingDetails: [], + ciCompletenessWarning: null, + }); + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + if (url === "https://api.gittensor.io/miners") { + return Response.json([ + { + uid: 7, + githubUsername: "oktofeesh1", + githubId: "123", + totalPrs: 4, + totalMergedPrs: 3, + totalOpenPrs: 1, + totalClosedPrs: 0, + totalOpenIssues: 0, + totalClosedIssues: 0, + totalSolvedIssues: 0, + totalValidSolvedIssues: 0, + isEligible: true, + credibility: 1, + eligibleRepoCount: 1, + hotkey: "must-not-leak", + }, + ]); + } + if (url === "https://api.gittensor.io/miners/123") { + return Response.json({ + repositories: [ + { + repositoryFullName: "JSONbored/gittensory", + totalPrs: "4", + totalMergedPrs: "3", + totalOpenPrs: "1", + totalClosedPrs: "0", + totalOpenIssues: "0", + totalClosedIssues: "0", + isEligible: true, + credibility: "1.000000", + }, + ], + }); + } + if (url === "https://api.gittensor.io/miners/123/prs") return Response.json([]); + if (url === "https://mirror.gittensor.io/api/v1/miners/123/issues") return Response.json({ issues: [] }); + if (url.endsWith("/users/oktofeesh1")) return Response.json({ login: "oktofeesh1", public_repos: 2, followers: 1 }); + if (url.includes("/users/oktofeesh1/repos")) return Response.json([{ language: "TypeScript" }]); + if (url === "https://raw.githubusercontent.com/JSONbored/gittensory/HEAD/.gittensory.yml") { + return new Response("review:\n max_findings:\n blockers: 0\n"); + } + if (url.includes("/access_tokens")) { + if (gateFinalized && !failedPostGateMint) { + failedPostGateMint = true; + return new Response("mint failed", { status: 500 }); + } + return Response.json({ token: "installation-token", expires_at: "2026-05-28T00:04:00.000Z" }); + } + if (url.includes("/pulls/3/files")) + return Response.json([{ filename: "src/cache.ts", additions: 5, deletions: 1, status: "modified" }]); + if (/\/pulls\/3(?:\?|$)/.test(url)) return Response.json({ number: 3, mergeable_state: "clean" }); + if (url.includes("/check-runs") && method === "GET") return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/check-runs") && method === "POST") { + calls.gateChecks += 1; + const body = JSON.parse(String(init?.body ?? "{}")) as { status?: string; conclusion?: string }; + if (body.status !== "in_progress" || body.conclusion) { + gateFinalized = true; + clearInstallationTokenCacheForTest(); + } + return Response.json({ id: 901 }, { status: 201 }); + } + if (url.includes("/check-runs/901") && method === "PATCH") { + calls.gateChecks += 1; + gateFinalized = true; + clearInstallationTokenCacheForTest(); + return Response.json({ id: 901 }); + } + if (url.includes("/issues/3/comments") && method === "GET") return Response.json([]); + if (url.includes("/issues/3/comments") && method === "POST") { + calls.comments += 1; + postedBody = String((JSON.parse(String(init?.body ?? "{}")) as { body?: string }).body ?? ""); + return Response.json({ id: 1, html_url: "https://github.com/comment/1" }, { status: 201 }); + } + return new Response("not found", { status: 404 }); + }); + + try { + await processJob(env, { + type: "github-webhook", + deliveryId: "pr-unified-comment-max-findings", + eventName: "pull_request", + payload: { + action: "synchronize", + installation: { + id: 123, + account: { login: "JSONbored", id: 1, type: "User" }, + repository_selection: "selected", + permissions: { metadata: "read", pull_requests: "read", issues: "write", checks: "write" }, + events: ["issues", "issue_comment", "pull_request", "repository", "installation_repositories"], + }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + pull_request: { + number: 3, + title: "Fix webhook duplicate delivery again", + state: "open", + user: { login: "oktofeesh1" }, + head: { sha: "unifiedmaxfindings" }, + labels: [{ name: "bug" }], + body: "No linked issue on purpose.\n\nValidation: npm test", + }, + }, + }); + + expect(calls.comments).toBe(2); + expect(postedBody).toContain(""); + expect(postedBody).toContain("_+1 more_"); + } finally { + liveCiSpy.mockRestore(); + } + }); + // #1955: the review-effort minutes persisted onto the public-stats audit event (independent of // review.effort_score, which only gates the unified-comment CHIP) must never block the publish itself when the // estimator throws — the publish still completes and simply omits `reviewEffortMinutes` from the event metadata diff --git a/test/unit/unified-comment.test.ts b/test/unit/unified-comment.test.ts index d550391c8f..7bf03f8c84 100644 --- a/test/unit/unified-comment.test.ts +++ b/test/unit/unified-comment.test.ts @@ -490,6 +490,17 @@ describe("buildUnifiedReviewInput", () => { const withoutEffort = buildUnifiedReviewInput({ changedFiles: 1, reviews: [reviewNote("merge")] }); expect(withoutEffort.reviewEffort).toBeUndefined(); }); + + it("threads optional maxFindingsCaps through to the input when provided (#2049)", () => { + const withCaps = buildUnifiedReviewInput({ + changedFiles: 1, + reviews: [reviewNote("merge")], + maxFindingsCaps: { blockers: 2, nits: 3 }, + }); + expect(withCaps.maxFindingsCaps).toEqual({ blockers: 2, nits: 3 }); + const withoutCaps = buildUnifiedReviewInput({ changedFiles: 1, reviews: [reviewNote("merge")] }); + expect(withoutCaps.maxFindingsCaps).toBeUndefined(); + }); }); describe("renderReviewingPlaceholder", () => { @@ -558,11 +569,36 @@ describe("review.max_findings display caps (#2049)", () => { ); }); - it("truncateFindingsForDisplay handles nullish and zero caps", () => { + it("truncateFindingsForDisplay handles nullish, undefined, under-cap, and zero caps", () => { expect(truncateFindingsForDisplay(["a", "b"], null)).toEqual({ shown: ["a", "b"], hiddenCount: 0 }); + expect(truncateFindingsForDisplay(["a", "b"], undefined)).toEqual({ shown: ["a", "b"], hiddenCount: 0 }); + expect(truncateFindingsForDisplay(["a"], 5)).toEqual({ shown: ["a"], hiddenCount: 0 }); expect(truncateFindingsForDisplay(["a", "b"], 1)).toEqual({ shown: ["a"], hiddenCount: 1 }); expect(truncateFindingsForDisplay(["a", "b"], 0)).toEqual({ shown: [], hiddenCount: 2 }); }); + + it("renders cap=0 as a +N more placeholder without listing items", () => { + const capped = renderUnifiedReviewComment({ + ...base, + recommendations: ["request_changes"], + blockers: ["alpha", "beta"], + nits: ["nit one"], + maxFindingsCaps: { blockers: 0, nits: 0 }, + }); + expect(capped).not.toContain("- alpha"); + expect(capped).toContain("_+2 more_"); + expect(capped).toContain("_+1 more_"); + }); + + it("omits the +N more footer when the list fits within the cap", () => { + const capped = renderUnifiedReviewComment({ + ...base, + nits: ["only nit"], + maxFindingsCaps: { blockers: null, nits: 5 }, + }); + expect(capped).toContain("only nit"); + expect(capped).not.toMatch(/\+1 more/); + }); }); describe("shouldPostReviewingPlaceholder", () => {