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
17 changes: 17 additions & 0 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9756,6 +9756,7 @@ async function maybePublishPrPublicSurface(
let suggestionsEnabledForReview = false;
let changedFilesSummaryEnabledForReview = false;
let effortScoreEnabledForReview = false;
let autoMergeSummaryEnabledForReview = false;
let reviewMemoryEnabledForReview = false;
let findingCategoriesEnabledForReview = false;
let fixHandoffEnabledForReview = false;
Expand Down Expand Up @@ -10340,6 +10341,7 @@ async function maybePublishPrPublicSurface(
const deterministicReviewOverrides = resolveReviewPromptOverrides(reviewManifestForAutoReview);
changedFilesSummaryEnabledForReview = deterministicReviewOverrides.changedFilesSummary;
effortScoreEnabledForReview = deterministicReviewOverrides.effortScore;
autoMergeSummaryEnabledForReview = deterministicReviewOverrides.autoMergeSummary;
minFindingSeverityForReview = deterministicReviewOverrides.minFindingSeverity;
inlineCommentsPerCategoryForReview = deterministicReviewOverrides.inlineCommentsPerCategory;
// review.memory (#2179, part of #1964): deterministic, no-AI -- resolved the same unconditional way as
Expand Down Expand Up @@ -11831,6 +11833,21 @@ async function maybePublishPrPublicSurface(
...(aiReview !== undefined ? { aiReview } : {}),
advisoryFindings: advisory.findings,
...(linkedIssueSatisfaction !== null ? { linkedIssueSatisfaction } : {}),
// review.auto_merge_summary (#2051/#4147): deterministic, no-AI — reuses the SAME ciState/
// mergeStateLabel/gate/linkedIssues facts this pass already resolved for mergeReadiness and the gate
// verdict above, no extra fetch. gatePassing mirrors the gate's own "no hard blocker" definition
// (conclusion === "success"); linkedIssueValid mirrors missing_linked_issue's own "has at least one
// linked issue reference" check (pr.linkedIssues.length > 0).
...(autoMergeSummaryEnabledForReview
? {
autoMergeSummary: {
ciGreen: ciState === "passed",
gatePassing: renderedGate.conclusion === "success",
mergeableClean: mergeStateLabel === "clean",
linkedIssueValid: pr.linkedIssues.length > 0,
},
}
: {}),
panelRows: rows,
...(reviewConfig?.fields !== undefined
? { reviewFields: reviewConfig.fields }
Expand Down
17 changes: 16 additions & 1 deletion src/review/unified-comment-bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,10 @@ import { githubPrFileDiffUrl } from "./changed-files-diff-link";
import { classifyFindingCategory, FINDING_CATEGORIES, type FindingCategory } from "./finding-category-classify";
import type { FixHandoffBlock } from "./fix-handoff-render";
import {
buildAutoMergeSummaryCollapsible,
buildUnifiedReviewInput,
renderUnifiedReviewComment,
type AutoMergeSummarySignals,
type DualReviewNote,
type MergeReadiness,
type ReviewNotes,
Expand Down Expand Up @@ -346,6 +348,12 @@ 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;
/** Read-only "auto-merge readiness" conditions table (review.auto_merge_summary port, #2051/#4147). When
* present, an "Auto-merge readiness" collapsible listing which auto-merge conditions currently pass/fail
* is appended — informational only, never a decision or a promise to merge (the gate/status chip above it
* remains the actual verdict). No AI, no network. Default OFF (the processor passes this only when the
* manifest opts in — see `resolveReviewPromptOverrides`'s `autoMergeSummary`). */
autoMergeSummary?: AutoMergeSummarySignals | undefined;
/** Display-only caps from `review.max_findings` (#2049). */
maxFindingsCaps?: { blockers: number | null; nits: number | null } | undefined;
/** `review.comment_verbosity` port (#2047): how much collapsible detail renders — `quiet` drops the Nits
Expand Down Expand Up @@ -762,6 +770,13 @@ export function buildUnifiedCommentBody(args: UnifiedCommentBridgeArgs): string
args.manifestWarnings && args.manifestWarnings.length > 0 ? buildManifestValidationCollapsible(args.manifestWarnings) : null;
const withManifestValidation =
manifestValidationCollapsible !== null ? [manifestValidationCollapsible, ...(args.extraCollapsibles ?? [])] : args.extraCollapsibles;
// review.auto_merge_summary port (#2051/#4147): when the manifest opts in, the processor hands us the
// already-computed auto-merge condition signals here; append the read-only "Auto-merge readiness"
// collapsible right after manifest validation (decision-relevant, so ahead of the structural/visual
// summaries below). Flag-OFF (the processor passes undefined) ⇒ extraCollapsibles is unchanged.
const autoMergeSummaryCollapsible = args.autoMergeSummary !== undefined ? buildAutoMergeSummaryCollapsible(args.autoMergeSummary) : null;
const withAutoMergeSummary =
autoMergeSummaryCollapsible !== null ? [...(withManifestValidation ?? []), autoMergeSummaryCollapsible] : withManifestValidation;
// review.changed_files_summary port: when the manifest opts in, the processor hands us every changed file's
// path + deltas here; append the grouped "Changed files" collapsible ahead of the visual preview (structure
// before pixels). Flag-OFF (the processor passes undefined) ⇒ extraCollapsibles is unchanged. (#1957)
Expand All @@ -770,7 +785,7 @@ export function buildUnifiedCommentBody(args: UnifiedCommentBridgeArgs): string
? buildChangedFilesSummaryCollapsible(args.changedFilesSummary, args.changedFilesSummaryContext)
: null;
const withChangedFiles =
changedFilesCollapsible !== null ? [...(withManifestValidation ?? []), changedFilesCollapsible] : withManifestValidation;
changedFilesCollapsible !== null ? [...(withAutoMergeSummary ?? []), changedFilesCollapsible] : withAutoMergeSummary;
// review.finding_categories port: when the manifest opts in, the processor hands us this review's line-anchored
// AI findings here; append the "Finding categories" collapsible right after Changed files (both are structural
// review-shape summaries, ahead of the visual preview). Flag-OFF (the processor passes undefined) ⇒
Expand Down
8 changes: 6 additions & 2 deletions src/signals/focus-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,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; impactMap: boolean; cultureProfile: boolean; findingCategories: boolean; inlineCommentsPerCategory: number | null; minFindingSeverity: ReviewFindingSeverity | null; maxFindings: MaxFindingsConfig; commentVerbosity: CommentVerbosity | null; e2eTestDelivery: E2eTestDeliveryMode | 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; autoMergeSummary: boolean; impactMap: boolean; cultureProfile: boolean; findingCategories: boolean; inlineCommentsPerCategory: number | null; minFindingSeverity: ReviewFindingSeverity | null; maxFindings: MaxFindingsConfig; commentVerbosity: CommentVerbosity | null; e2eTestDelivery: E2eTestDeliveryMode | 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. `shouldRequestInlineFindings` (#4099) only ever checks `=== true`, so null
// and false are functionally identical to it — collapsing here (matching every sibling field below) is simpler
Expand All @@ -286,7 +286,11 @@ export function resolveReviewPromptOverrides(manifest: FocusManifest | null): {
// cultureProfile resolves the same way (#2995) — true ONLY when the manifest explicitly set
// review.culture_profile: true. The caller ANDs this per-repo opt-in with the GITTENSORY_REVIEW_CULTURE_PROFILE
// global kill-switch (mirrors how RAG/reputation/grounding compose a global flag with a per-repo override).
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, impactMap: manifest?.review.impactMap === true, cultureProfile: manifest?.review.cultureProfile === true, findingCategories: manifest?.review.findingCategories === true, inlineCommentsPerCategory: manifest?.review.inlineCommentsPerCategory ?? null, minFindingSeverity: manifest?.review.minFindingSeverity ?? null, maxFindings: manifest?.review.maxFindings ?? { ...EMPTY_MAX_FINDINGS_CONFIG }, commentVerbosity: manifest?.review.commentVerbosity ?? null, e2eTestDelivery: manifest?.review.e2eTestDelivery ?? null, pathInstructions: manifest?.review.pathInstructions ?? [], instructions: manifest?.review.instructions ?? null, excludePaths: manifest?.review.excludePaths ?? [], pathFilters: manifest?.review.pathFilters ?? [], selfHostAiModel: resolveReviewSelfHostAiModel(manifest) };
// autoMergeSummary resolves the same way (#2051/#4147) — like changedFilesSummary/effortScore, it is
// deterministic/display-only (never touches the AI prompt) and only needs the unified-comment convergence
// feature itself to be on; the caller supplies the already-computed AutoMergeSummarySignals unconditionally
// once this is true (no separate global kill-switch, matching changedFilesSummary/effortScore's shape).
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, autoMergeSummary: manifest?.review.autoMergeSummary === true, impactMap: manifest?.review.impactMap === true, cultureProfile: manifest?.review.cultureProfile === true, findingCategories: manifest?.review.findingCategories === true, inlineCommentsPerCategory: manifest?.review.inlineCommentsPerCategory ?? null, minFindingSeverity: manifest?.review.minFindingSeverity ?? null, maxFindings: manifest?.review.maxFindings ?? { ...EMPTY_MAX_FINDINGS_CONFIG }, commentVerbosity: manifest?.review.commentVerbosity ?? null, e2eTestDelivery: manifest?.review.e2eTestDelivery ?? null, pathInstructions: manifest?.review.pathInstructions ?? [], instructions: manifest?.review.instructions ?? null, excludePaths: manifest?.review.excludePaths ?? [], pathFilters: manifest?.review.pathFilters ?? [], selfHostAiModel: resolveReviewSelfHostAiModel(manifest) };
}

/** Resolve `review.memory` (#2179, config slice of #1964) from a possibly-null manifest (null = load failure ⇒
Expand Down
16 changes: 9 additions & 7 deletions test/unit/focus-manifest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3388,14 +3388,14 @@ 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, impact_map: true, culture_profile: true, finding_categories: true, comment_verbosity: "detailed", 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, impactMap: true, cultureProfile: true, findingCategories: true, inlineCommentsPerCategory: null, minFindingSeverity: null, maxFindings: { blockers: null, nits: null }, commentVerbosity: "detailed", e2eTestDelivery: 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 } });
const manifest = parseFocusManifest({ review: { profile: "chill", security_focus: true, inline_comments: true, suggestions: true, changed_files_summary: true, effort_score: true, auto_merge_summary: true, impact_map: true, culture_profile: true, finding_categories: true, comment_verbosity: "detailed", 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, autoMergeSummary: true, impactMap: true, cultureProfile: true, findingCategories: true, inlineCommentsPerCategory: null, minFindingSeverity: null, maxFindings: { blockers: null, nits: null }, commentVerbosity: "detailed", e2eTestDelivery: 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 + impact map + culture profile + finding categories + security focus
// all default OFF (strict false) — inlineComments collapses the same way as every sibling flag on this
// object (#4099: shouldRequestInlineFindings only ever checks `=== true`, so null/false/absent are
// functionally identical to it; no tri-state needed here).
expect(resolveReviewPromptOverrides(null)).toEqual({ profile: null, tone: null, securityFocus: false, inlineComments: false, suggestions: false, changedFilesSummary: false, effortScore: false, impactMap: false, cultureProfile: false, findingCategories: false, inlineCommentsPerCategory: null, minFindingSeverity: null, maxFindings: { blockers: null, nits: null }, commentVerbosity: null, e2eTestDelivery: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], selfHostAiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG } });
// changed-files summary + effort score + auto-merge summary + impact map + culture profile + finding
// categories + security focus all default OFF (strict false) — inlineComments collapses the same way as
// every sibling flag on this object (#4099: shouldRequestInlineFindings only ever checks `=== true`, so
// null/false/absent are functionally identical to it; no tri-state needed here).
expect(resolveReviewPromptOverrides(null)).toEqual({ profile: null, tone: null, securityFocus: false, inlineComments: false, suggestions: false, changedFilesSummary: false, effortScore: false, autoMergeSummary: false, impactMap: false, cultureProfile: false, findingCategories: false, inlineCommentsPerCategory: null, minFindingSeverity: null, maxFindings: { blockers: null, nits: null }, commentVerbosity: null, e2eTestDelivery: 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);
Expand All @@ -3405,6 +3405,8 @@ describe("resolveReviewPathInstructions (#review-path-instructions)", () => {
expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { profile: "chill" } })).changedFilesSummary).toBe(false);
expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { effort_score: false } })).effortScore).toBe(false);
expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { profile: "chill" } })).effortScore).toBe(false);
expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { auto_merge_summary: false } })).autoMergeSummary).toBe(false);
expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { profile: "chill" } })).autoMergeSummary).toBe(false);
expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { impact_map: false } })).impactMap).toBe(false);
expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { profile: "chill" } })).impactMap).toBe(false);
expect(resolveReviewPromptOverrides(parseFocusManifest({ review: { finding_categories: false } })).findingCategories).toBe(false);
Expand Down
Loading