From 6d442d5ec98d153ab40cf9ea0e1026edd5b5a981 Mon Sep 17 00:00:00 2001
From: JSONbored <49853598+JSONbored@users.noreply.github.com>
Date: Wed, 15 Jul 2026 01:27:37 -0700
Subject: [PATCH] review: drop redundant banner + demote readiness chip
 off-ready

The 12x repeated-square banner row duplicated the color the alert
blockquote already renders; the readiness score (advisory-only, never
feeds the gate) read as contradictory next to a non-"ready" verdict.
Also gives the Suggested Action line its own nested-blockquote box so
it's visually distinct from the surrounding body text.

Closes #6066
---
 src/review/unified-comment.ts     | 44 ++++++++++++++++++++++---------
 test/unit/unified-comment.test.ts | 28 ++++++++++++++++++++
 2 files changed, 60 insertions(+), 12 deletions(-)

diff --git a/src/review/unified-comment.ts b/src/review/unified-comment.ts
index 9952f39ed1..6a8c6f4388 100644
--- a/src/review/unified-comment.ts
+++ b/src/review/unified-comment.ts
@@ -391,12 +391,15 @@ function plural(n: number, one: string): string {
   return `${n} ${one}${n === 1 ? "" : "s"}`;
 }
 
-function statusChips(input: UnifiedReviewInput, ctx: UnifiedCommentContext): string {
+function statusChips(input: UnifiedReviewInput, ctx: UnifiedCommentContext, status: UnifiedCommentStatus): string {
   const chips: string[] = [`\`${plural(input.changedFiles, "file")}\``];
   if (input.reviewerCount > 0) chips.push(`\`${plural(input.reviewerCount, "AI reviewer")}\``);
   const blockerCount = (input.blockers ?? []).length;
   chips.push(blockerCount ? `\`${plural(blockerCount, "blocker")}\`` : "`no blockers`");
-  if (typeof ctx.readinessScore === "number") chips.push(`\`readiness ${Math.round(ctx.readinessScore)}/100\``);
+  // The readiness score is advisory-only and NEVER feeds the gate (see deriveUnifiedStatus's own comments) —
+  // showing it next to a non-"ready" verdict reads as contradictory (e.g. "readiness 93/100" beside "fixes
+  // required"). Only surface the number when the verdict itself agrees with a high score.
+  if (status === "ready" && typeof ctx.readinessScore === "number") chips.push(`\`readiness ${Math.round(ctx.readinessScore)}/100\``);
   if (input.readiness) {
     const ci = input.readiness.ciState;
     chips.push(ci === "passed" ? "`CI green`" : ci === "failed" ? "`CI failing`" : "`CI pending`");
@@ -408,6 +411,20 @@ function statusChips(input: UnifiedReviewInput, ctx: UnifiedCommentContext): str
   return chips.join(" · ");
 }
 
+/** Nest a block one level deeper inside the outer alert blockquote (an extra `> ` per line). Gives the
+ *  Suggested Action verdict — the single most load-bearing line in the comment — its own visually distinct
+ *  bordered sub-block instead of a plain bold paragraph lost in the body flow. Pure markdown (a nested
+ *  blockquote), no custom HTML/CSS — `asAlert` re-prefixes every line with its own `> ` afterward, so this
+ *  produces ordinary two-deep blockquote nesting, which GitHub already renders with a second indent/border.
+ *  Unlike `asAlert`, every caller-supplied line here is always non-empty (a bold verdict line, optionally
+ *  followed by `- reason` bullets — never a blank separator), so no blank-line special case is needed. */
+function nestedBox(text: string): string {
+  return text
+    .split("\n")
+    .map((l) => `> ${l}`)
+    .join("\n");
+}
+
 function verdictLine(status: UnifiedCommentStatus, input: UnifiedReviewInput, ctx: UnifiedCommentContext): string {
   const icon = STATUS_META[status].icon;
   const reasons = (defaultReason?: string) => {
@@ -416,21 +433,23 @@ function verdictLine(status: UnifiedCommentStatus, input: UnifiedReviewInput, ct
   };
   switch (status) {
     case "ready":
-      return input.merged
-        ? `**${icon} Suggested Action - Approve/Merge**${reasons("auto-merged")}`
-        : `**${icon} Suggested Action - Approve/Merge**${reasons("safe to merge")}`;
+      return nestedBox(
+        input.merged
+          ? `**${icon} Suggested Action - Approve/Merge**${reasons("auto-merged")}`
+          : `**${icon} Suggested Action - Approve/Merge**${reasons("safe to merge")}`,
+      );
     case "advisory":
-      return `**${icon} Suggested Action - Advisory Only**${reasons("no action taken")}`;
+      return nestedBox(`**${icon} Suggested Action - Advisory Only**${reasons("no action taken")}`);
     case "held":
-      return `**${icon} Suggested Action - Manual Review**${reasons()}`;
+      return nestedBox(`**${icon} Suggested Action - Manual Review**${reasons()}`);
     case "blocked":
       if (ctx.neverClosed) {
-        return `**${icon} Suggested Action - Manual Review**${reasons()}`;
+        return nestedBox(`**${icon} Suggested Action - Manual Review**${reasons()}`);
       }
       if (input.decision === "close" && !ctx.neverClosed) {
-        return `**${icon} Suggested Action - Reject/Close**${reasons()}`;
+        return nestedBox(`**${icon} Suggested Action - Reject/Close**${reasons()}`);
       }
-      return `**${icon} Suggested Action - Fix Blockers**${reasons()}`;
+      return nestedBox(`**${icon} Suggested Action - Fix Blockers**${reasons()}`);
   }
 }
 
@@ -653,10 +672,11 @@ export function renderUnifiedReviewComment(input: UnifiedReviewInput, ctx: Unifi
   const collapsiblesOpen = verbosity === "detailed";
 
   const blocks: string[] = [
-    meta.square.repeat(12),
+    // No repeated-square banner row here (dropped, #6066) — the alert blockquote below already renders a
+    // colored border + icon for the same status; a 12x-emoji row on top of that was pure decoration.
     `### ${meta.icon} ${brand} result - ${headlineLabel(status, input, ctx)}${status === "ready" && input.merged ? " · auto-merged" : ""}`,
     ...(reviewTimestamp ? [`<sub>Review updated: ${reviewTimestamp}</sub>`] : []),
-    statusChips(input, ctx),
+    statusChips(input, ctx, status),
     verdictLine(status, input, ctx),
   ];
 
diff --git a/test/unit/unified-comment.test.ts b/test/unit/unified-comment.test.ts
index 102dc4f125..c23aa43be7 100644
--- a/test/unit/unified-comment.test.ts
+++ b/test/unit/unified-comment.test.ts
@@ -149,6 +149,9 @@ describe("renderUnifiedReviewComment", () => {
     expect(md).toContain("`no blockers`");
     expect(md).toContain("`readiness 93/100`");
     expect(md).toContain("`CI green`");
+    // The Suggested Action verdict is nested one level deeper than the rest of the alert body (#6066) — a
+    // second `> ` on top of asAlert's own per-line prefix, giving it a visually distinct bordered sub-block.
+    expect(md).toContain("> > **✅ Suggested Action - Approve/Merge**");
     expect(md).toContain("**Review summary**");
     expect(md).toContain("| **Code review** | ✅ No blockers | 2 reviewers, synthesized |");
     expect(md).toContain("| Linked issue | ✅ Linked | #1372 |");
@@ -226,6 +229,9 @@ describe("renderUnifiedReviewComment", () => {
     expect(md).toContain("Why this is blocked");
     expect(md).toContain("Introduces a hardcoded secret.");
     expect(md).toContain("| **Code review** | ❌ 1 blocker |");
+    // #6066: the advisory-only readiness score is hidden on a non-"ready" verdict — "readiness 93/100" next
+    // to "fixes required" reads as contradictory, since the score never feeds the gate either way.
+    expect(md).not.toContain("readiness 93/100");
   });
 
   it("held state uses the warning alert and amber bar", () => {
@@ -233,6 +239,7 @@ describe("renderUnifiedReviewComment", () => {
     expect(md).toContain("> [!WARNING]");
     expect(md).toContain("🟨");
     expect(md).toContain("Suggested Action - Manual Review");
+    expect(md).not.toContain("readiness 93/100"); // #6066: hidden outside the "ready" status
   });
 
   it("advisory state uses the note alert and blue bar", () => {
@@ -242,6 +249,27 @@ describe("renderUnifiedReviewComment", () => {
     expect(md).toContain("Suggested Action - Advisory Only");
   });
 
+  it("never renders the old repeated-square banner row (#6066 — redundant with the alert's own color/icon)", () => {
+    for (const decision of ["merge", "manual", "close", "comment"] as const) {
+      const md = renderUnifiedReviewComment({ ...base, decision }, ctx);
+      // Every STATUS_META square is exactly 2 code units (emoji + variation selector); a banner would show up
+      // as one square repeated 12x in a row with no other characters — the legend line below uses each square
+      // only ONCE per status name, so this pattern can only match a leftover banner.
+      expect(md).not.toMatch(/(🟩|🟦|🟨|🟥){12}/);
+    }
+  });
+
+  it("only shows the readiness-score chip when the verdict is ready, regardless of how high the score is", () => {
+    const readyMd = renderUnifiedReviewComment({ ...base, decision: "merge" }, ctx);
+    expect(readyMd).toContain("`readiness 93/100`");
+    const heldMd = renderUnifiedReviewComment({ ...base, decision: "manual" }, ctx);
+    expect(heldMd).not.toContain("readiness 93/100");
+    const blockedMd = renderUnifiedReviewComment({ ...base, decision: "close", blockers: ["x"] }, ctx);
+    expect(blockedMd).not.toContain("readiness 93/100");
+    const advisoryMd = renderUnifiedReviewComment({ ...base, decision: "comment", recommendations: [] }, ctx);
+    expect(advisoryMd).not.toContain("readiness 93/100");
+  });
+
   it("dedupes repeated blockers and nits", () => {
     const md = renderUnifiedReviewComment(
       { ...base, decision: "close", blockers: ["Same issue", "same issue", "Same issue"] },