fix(review): trim the Improvement signal row to a concise value rating

src/signals/engine.ts

@@ -4735,22 +4735,6 @@ const IMPROVEMENT_BAND_LABELS: Record<ImprovementBand, string> = {
  *  (see the module comment above for why a full collapsible isn't wired up in this PR: at most one
  *  deterministic finding can fire today, since REES's complexity/duplication analyzers and a parsed Codecov
  *  number have no caller yet -- see `signals/improvement.ts`'s own header comment). */
-function improvementEvidenceText(
-  band: ImprovementBand,
-  safeFindings: SignalFinding[],
-  safeValueAssessment: { magnitude: ImprovementMagnitude; rationale: string } | undefined,
-): string {
-  const findingSentences = safeFindings.map((finding) => finding.publicText ?? finding.detail);
-  const deterministicPart =
-    band === "insufficient-signal"
-      ? "Nothing measurable for the structural-improvement analyzers on this PR (e.g. no code files changed)."
-      : findingSentences.length > 0
-        ? findingSentences.slice(0, 2).join(" ") + (findingSentences.length > 2 ? ` (+${findingSentences.length - 2} more.)` : "")
-        : "No structural-improvement signals were detected for this PR.";
-  const valuePart = safeValueAssessment ? ` LLM value judgment: ${safeValueAssessment.magnitude} — ${safeValueAssessment.rationale}` : "";
-  return `${deterministicPart}${valuePart}`;
-}
-
 /** The risk × value quadrant label (#4745): crosses the existing `SlopBand` (risk axis, `src/signals/slop.ts`)
  *  with the deterministic `ImprovementBand` (value axis, #4742) into one compact string, e.g.
  *  `"risk: low · value: moderate"` -- exactly the issue's own example wording. Both band types are closed
@@ -4789,22 +4773,23 @@ function buildImprovementSignalRow(
   slopBand?: SlopBand | undefined,
 ): PublicPrPanelSignalRow | null {
   if (!assessment) return null;
-  const safeFindings = assessment.findings.filter(
-    (finding) => !containsPrivatePublicTerm([finding.title, finding.detail, finding.publicText].filter(Boolean).join(" ")),
-  );
-  const safeValueAssessment = valueAssessment && !containsPrivatePublicTerm(valueAssessment.rationale) ? valueAssessment : undefined;
-  const evidence = improvementEvidenceText(assessment.band, safeFindings, safeValueAssessment);
-  // #4745: prefixes the risk × value quadrant onto the SAME Evidence cell instead of a new row/column --
-  // `assessment.band` is always defined here (the row already bailed out above when `assessment` is absent),
-  // so this only ever needs `slopBand` to produce the full quadrant; absent slopBand (slop wasn't computed
-  // this pass) leaves the evidence text exactly as it was before this PR, never a fabricated risk reading.
+  // #5101: the Evidence cell is a quick rating, not a paragraph. The risk × value quadrant (#4745) IS the
+  // score; it is tagged with only the LLM's one-word magnitude when present. The raw finding sentences and the
+  // full LLM rationale the old cell concatenated — a wall of text by construction — are intentionally dropped
+  // per maintainer feedback ("a quick, clean, simple score/rating ... not a paragraph"). Only closed-enum band
+  // names + the magnitude enum are interpolated, so the cell is public-safe with no free-text leak surface —
+  // the `containsPrivatePublicTerm` filtering the old free-text evidence needed is no longer required here.
   const quadrant = formatRiskValueQuadrant(slopBand, assessment.band);
+  const magnitudeTag = valueAssessment ? `LLM: ${valueAssessment.magnitude}` : undefined;
+  // Quadrant is absent only when slop wasn't computed this pass; fall back to the value band alone so the cell
+  // is never empty, still never fabricating a risk reading.
+  const rating = [quadrant ?? `value: ${assessment.band}`, magnitudeTag].filter(Boolean).join(" · ");
   return {
     key: "improvementSignal",
     cells: [
       "Improvement",
       IMPROVEMENT_BAND_LABELS[assessment.band],
-      quadrant ? `${quadrant} — ${evidence}` : evidence,
+      rating,
       "Advisory only — never blocks merge.",
     ],
   };

test/unit/queue-4.test.ts

@@ -3016,7 +3016,7 @@ describe("queue processors", () => {
       // unified renderer's table only surfaces the first 3 of each row's 4 cells (Label/Result/Evidence, not
       // Action) — same as the adjacent "Gate result" row, which also never shows its own 4th cell here — so this
       // asserts against the 3 columns this renderer actually prints, not the row's full cells array.
-      expect(postedBody).toContain("| Improvement | ⚠️ ℹ️ None detected | No structural-improvement signals were detected for this PR. |");
+      expect(postedBody).toContain("| Improvement | ⚠️ ℹ️ None detected | value: none |");
       // Public-safe regardless: no internal trust/economics fields leak through this new row either.
       expect(postedBody).not.toMatch(/wallet|hotkey|coldkey|reward|trust score/i);
     } finally {
@@ -3191,11 +3191,10 @@ describe("queue processors", () => {
 
       expect(calls.comments).toBeGreaterThan(0);
       expect(postedBody).toContain("<!-- gittensory-pr-panel:v1 -->");
-      // The quadrant prefix ("risk: low · value: none") threaded from the REAL slopBand computed this pass
-      // (missingTestEvidence only, slopRisk 15 -> band "low") ahead of the SAME evidence text #4744 already
-      // asserts verbatim -- proving processors.ts's new hoisted slopBand reaches the rendered comment, not
-      // just computed and discarded.
-      expect(postedBody).toContain("| Improvement | ⚠️ ℹ️ None detected | risk: low · value: none — No structural-improvement signals were detected for this PR. |");
+      // The quadrant rating ("risk: low · value: none") threaded from the REAL slopBand computed this pass
+      // (missingTestEvidence only, slopRisk 15 -> band "low") IS the concise Evidence cell (#5101) -- proving
+      // processors.ts's hoisted slopBand reaches the rendered comment, not just computed and discarded.
+      expect(postedBody).toContain("| Improvement | ⚠️ ℹ️ None detected | risk: low · value: none |");
       // Public-safe regardless: no internal trust/economics fields leak through the new quadrant clause either.
       expect(postedBody).not.toMatch(/wallet|hotkey|coldkey|reward|trust score/i);
     } finally {

test/unit/signals-coverage.test.ts

@@ -930,11 +930,11 @@ describe("signal coverage edge cases", () => {
       expect(panel.rows).toHaveLength(7);
     });
 
-    it("renders the deterministic band as a static template label when only the deterministic tier is available", () => {
+    it("renders a concise value rating (#5101) and never dumps the finding sentences into the cell", () => {
       const comment = buildPublicPrIntelligenceComment({ ...improvementBaseArgs, improvementSignal: minorAssessment, env: {} });
-      expect(comment).toContain("| Improvement | ✅ Minor |");
-      expect(comment).toContain("Code changes are accompanied by test evidence.");
-      expect(comment).not.toContain("Value judgment");
+      expect(comment).toContain("| Improvement | ✅ Minor | value: minor |");
+      // #5101: the raw finding sentence is intentionally no longer rendered — the cell is a quick rating.
+      expect(comment).not.toContain("Code changes are accompanied by test evidence.");
       expect(comment).not.toContain("LLM value judgment");
 
       const panel = buildPublicPrPanelSignalRows({ ...improvementBaseArgs, improvementSignal: minorAssessment });
@@ -943,12 +943,12 @@ describe("signal coverage edge cases", () => {
       expect(row.cells).toEqual([
         "Improvement",
         "✅ Minor",
-        "Code changes are accompanied by test evidence.",
+        "value: minor",
         "Advisory only — never blocks merge.",
       ]);
     });
 
-    it("renders the LLM tier's magnitude + rationale alongside the deterministic band when both tiers are available", () => {
+    it("tags the concise rating with the LLM's one-word magnitude (not the full rationale) when both tiers are available (#5101)", () => {
       const noneAssessment = { improvementScore: 0, band: "none" as const, findings: [] };
       const valueAssessment = { magnitude: "significant" as const, rationale: "This removes a whole class of retry bugs." };
       const comment = buildPublicPrIntelligenceComment({
@@ -959,21 +959,26 @@ describe("signal coverage edge cases", () => {
       });
       // The Result cell reflects the DETERMINISTIC band ("none"), never the LLM magnitude -- the two tiers are
       // deliberately never blended into one number/label (epic #4737 design constraint 1).
-      expect(comment).toContain("| Improvement | ℹ️ None detected |");
-      expect(comment).toContain("LLM value judgment: significant — This removes a whole class of retry bugs.");
+      expect(comment).toContain("| Improvement | ℹ️ None detected | value: none · LLM: significant |");
+      // #5101: only the one-word magnitude, never the full rationale paragraph, is surfaced.
+      expect(comment).toContain("· LLM: significant");
+      expect(comment).not.toContain("This removes a whole class of retry bugs.");
+      expect(comment).not.toContain("LLM value judgment");
 
       const panel = buildPublicPrPanelSignalRows({ ...improvementBaseArgs, improvementSignal: noneAssessment, valueAssessment });
       const row = panel.rows.find((r) => r.key === "improvementSignal")!;
       expect(row.cells[1]).toBe("ℹ️ None detected");
-      expect(row.cells[2]).toBe("No structural-improvement signals were detected for this PR. LLM value judgment: significant — This removes a whole class of retry bugs.");
+      expect(row.cells[2]).toBe("value: none · LLM: significant");
     });
 
-    it("renders the insufficient-signal band and caps inline findings at 2 with a '+N more' summary beyond that", () => {
+    it("renders insufficient-signal as a concise value rating and drops all finding sentences, however many (#5101)", () => {
       const insufficientAssessment = { improvementScore: 0, band: "insufficient-signal" as const, findings: [] };
       const insufficientPanel = buildPublicPrPanelSignalRows({ ...improvementBaseArgs, improvementSignal: insufficientAssessment });
       const insufficientRow = insufficientPanel.rows.find((r) => r.key === "improvementSignal")!;
       expect(insufficientRow.cells[1]).toBe("ℹ️ Insufficient signal");
-      expect(insufficientRow.cells[2]).toContain("Nothing measurable");
+      expect(insufficientRow.cells[2]).toBe("value: insufficient-signal");
+      // #5101: the old "Nothing measurable ..." paragraph is gone.
+      expect(insufficientRow.cells[2]).not.toContain("Nothing measurable");
 
       const manyFindingsAssessment = {
         improvementScore: 100,
@@ -987,12 +992,10 @@ describe("signal coverage edge cases", () => {
       };
       const manyFindingsPanel = buildPublicPrPanelSignalRows({ ...improvementBaseArgs, improvementSignal: manyFindingsAssessment });
       const manyFindingsRow = manyFindingsPanel.rows.find((r) => r.key === "improvementSignal")!;
-      // Only the first two finding sentences render inline (none of the four fixtures above set `publicText`, so
-      // this also exercises the `finding.publicText ?? finding.detail` fallback); the remaining two are
-      // summarized by count rather than dumped inline (mirrors the "Nits"-style non-inline-dump convention).
-      expect(manyFindingsRow.cells[2]).toBe(
-        "2 function(s) have lower cyclomatic complexity after this pull request. 1 previously-duplicated code block(s) were consolidated or removed by this pull request. (+2 more.)",
-      );
+      // #5101: no matter how many findings, the cell is the concise value rating — finding sentences are never
+      // concatenated inline (the whole point of this issue's simplification).
+      expect(manyFindingsRow.cells[2]).toBe("value: significant");
+      expect(manyFindingsRow.cells[2]).not.toContain("cyclomatic complexity");
     });
 
     it("hides the row via review.fields.improvementSignal: false, exactly like its seven siblings", () => {
@@ -1037,9 +1040,10 @@ describe("signal coverage edge cases", () => {
       const panel = buildPublicPrPanelSignalRows({ ...improvementBaseArgs, improvementSignal: unsafeAssessment, valueAssessment: unsafeValueAssessment });
       const row = panel.rows.find((r) => r.key === "improvementSignal")!;
       expect(JSON.stringify(row)).not.toMatch(forbidden);
-      // With the one (unsafe) finding filtered out and the (unsafe) valueAssessment dropped, the deterministic
-      // "no signals" fallback text is what's left -- never an empty cell.
-      expect(row.cells[2]).toBe("No structural-improvement signals were detected for this PR.");
+      // #5101 makes this structurally leak-proof: the cell renders only closed-enum band/magnitude names, never
+      // the free-text finding detail or LLM rationale, so no sanitizer pass is even needed. The unsafe finding
+      // sentence and unsafe rationale simply never reach the cell.
+      expect(row.cells[2]).toBe("value: moderate · LLM: significant");
     });
   });
 
@@ -1087,21 +1091,22 @@ describe("signal coverage edge cases", () => {
     };
     const quadrantAssessment = { improvementScore: 10, band: "minor" as const, findings: [] };
 
-    it("prefixes the quadrant label onto the Improvement row's Evidence cell when slopBand is threaded alongside improvementSignal", () => {
+    it("uses the quadrant label as the Improvement row's Evidence rating when slopBand is threaded (#5101)", () => {
       const panel = buildPublicPrPanelSignalRows({ ...quadrantBaseArgs, improvementSignal: quadrantAssessment, slopBand: "low" });
       const row = panel.rows.find((r) => r.key === "improvementSignal")!;
-      expect(row.cells[2]).toBe("risk: low · value: minor — No structural-improvement signals were detected for this PR.");
+      // #5101: the quadrant IS the rating now — no trailing finding/rationale paragraph.
+      expect(row.cells[2]).toBe("risk: low · value: minor");
       // The Result cell (the deterministic band label) is untouched by the quadrant -- the two tiers never blend.
       expect(row.cells[1]).toBe("✅ Minor");
 
       const comment = buildPublicPrIntelligenceComment({ ...quadrantBaseArgs, improvementSignal: quadrantAssessment, slopBand: "low", env: {} });
-      expect(comment).toContain("risk: low · value: minor — No structural-improvement signals were detected for this PR.");
+      expect(comment).toContain("| Improvement | ✅ Minor | risk: low · value: minor |");
     });
 
-    it("leaves the Evidence cell exactly as #4744 shipped it when slopBand is omitted -- byte-identical for every caller that hasn't threaded it yet", () => {
+    it("falls back to the value band alone when slopBand is omitted -- never a fabricated risk reading (#5101)", () => {
       const panel = buildPublicPrPanelSignalRows({ ...quadrantBaseArgs, improvementSignal: quadrantAssessment });
       const row = panel.rows.find((r) => r.key === "improvementSignal")!;
-      expect(row.cells[2]).toBe("No structural-improvement signals were detected for this PR.");
+      expect(row.cells[2]).toBe("value: minor");
       expect(row.cells[2]).not.toContain("risk:");
     });