From a390bf1e63feb47ee1dd225f3fa1591bbf0b4616 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Tue, 21 Jul 2026 13:19:29 -0700 Subject: [PATCH] fix(review): stop the Code review row contradicting itself when AI never ran buildDualReviewNotes (FIX D1) deliberately folds the gate's own deterministic hard blockers into the same blocker list AI findings use, so a non-AI gate failure still populates "Why this is blocked" even with zero AI reviewers. But codeReviewRow's evidence text stayed "No AI review summary" unconditionally, so a PR blocked purely by a deterministic check (AI review inconclusive, e.g. a concurrent duplicate pass) rendered "1 blocker (No AI review summary)" -- reading as an AI pass that silently found something, not "AI review never ran; a separate check is what's blocking this." A blocker present with reviewerCount 0 can only be that deterministic fold-in (a fresh ai_consensus_defect needs an actual review pass to exist), so the evidence text now says so plainly in that case. --- src/review/unified-comment.ts | 20 ++++++++++++++++---- test/unit/unified-comment.test.ts | 18 ++++++++++++++++++ 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/review/unified-comment.ts b/src/review/unified-comment.ts index d9b751cbde..87d86a9e14 100644 --- a/src/review/unified-comment.ts +++ b/src/review/unified-comment.ts @@ -608,9 +608,19 @@ function nonRequiredFailingChecksBlock(readiness: MergeReadiness | undefined): s return lines.join("\n"); } -/** The synthetic "Code review" row -- derived from the AI reviewers' own blocker count, never from - * `ctx.signals` -- so it always exists and is never subject to `review.fields` visibility (matches its - * pre-#6067 behavior as the signal table's unconditional first row). */ +/** The synthetic "Code review" row -- its blocker count is NOT purely the AI reviewers' own findings: + * `buildDualReviewNotes` (FIX D1) folds the gate's own deterministic hard blockers in too, so a non-AI + * gate failure (a missing linked issue, a registry-deliverable check, a secret leak, ...) still populates + * a "Why this is blocked" list even when no AI reviewer ran at all. Never routed through `ctx.signals`, so + * this row always exists and is never subject to `review.fields` visibility (matches its pre-#6067 + * behavior as the signal table's unconditional first row). + * + * #7491-class fix: when `reviewerCount` is 0, ANY blocker present can only have come from that deterministic + * fold-in -- a fresh `ai_consensus_defect` (the other blocker source) requires an actual review pass to + * exist at all. The evidence text used to read "No AI review summary" regardless, which next to a nonzero + * blocker count ("1 blocker (No AI review summary)") looked self-contradictory: as if an AI pass ran, + * found something, but produced no write-up -- not "AI review never ran; a separate check is what's + * blocking this." */ function codeReviewRow(input: UnifiedReviewInput): UnifiedSignalRow { const blockerCount = (input.blockers ?? []).length; const reviewerEvidence = @@ -618,7 +628,9 @@ function codeReviewRow(input: UnifiedReviewInput): UnifiedSignalRow { ? `${input.reviewerCount} reviewers, synthesized` : input.reviewerCount === 1 ? "1 reviewer" - : "No AI review summary"; + : blockerCount > 0 + ? "no AI review ran — blocker is from a non-AI gate check" + : "No AI review summary"; return { label: "Code review", state: blockerCount ? "fail" : "ok", diff --git a/test/unit/unified-comment.test.ts b/test/unit/unified-comment.test.ts index 588d8f34ad..75c642cba7 100644 --- a/test/unit/unified-comment.test.ts +++ b/test/unit/unified-comment.test.ts @@ -238,6 +238,24 @@ describe("renderUnifiedReviewComment", () => { expect(md).not.toContain("readiness 93/100"); }); + // #7491-class fix: a blocker present with reviewerCount 0 can only be a deterministic gate blocker folded + // in by buildDualReviewNotes (a fresh ai_consensus_defect needs an actual review pass to exist) -- the old + // "No AI review summary" evidence text, unconditional on reviewerCount, made this read as a silent/ + // incomplete AI review rather than "AI review never ran; something else is blocking this." + it("does not claim a silent/incomplete AI review when a blocker is present but no reviewer ran", () => { + const md = renderUnifiedReviewComment( + { ...base, reviewerCount: 0, recommendations: [], decision: "close", blockers: ["This PR's linked issue names a registry path it never touches."] }, + ctx, + ); + expect(md).toContain("- ❌ Code review — 1 blocker (no AI review ran — blocker is from a non-AI gate check)"); + expect(md).not.toContain("1 blocker (No AI review summary)"); + }); + + it("still reports a clean, non-contradictory 'no blockers' row when no reviewer ran and nothing is blocking", () => { + const md = renderUnifiedReviewComment({ ...base, reviewerCount: 0, recommendations: [] }, ctx); + expect(md).toContain("- ✅ Code review — No blockers (No AI review summary)"); + }); + it("held state uses the warning alert and amber bar", () => { const md = renderUnifiedReviewComment({ ...base, decision: "manual", recommendations: ["manual_review"] }, ctx); expect(md).toContain("> [!WARNING]");