From 5e0f0f235e5fa02cefc936db77686e3a606fe4b1 Mon Sep 17 00:00:00 2001
From: groom auto-builder <groom-auto-builder@users.noreply.github.com>
Date: Thu, 27 Aug 2026 13:33:33 +0000
Subject: [PATCH 1/3] groom: cursor-review's severity badge is authored from
 two tables in post-review.py and re-parsed by a hardcoded vocabulary regex in
 build-ledger.py
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Auto-built groom refactor (github-workflows:whole-repo:github-cursor-review-build-ledger-py).
Review required — do not auto-merge.
---
 .github/cursor-review/build-ledger.py         |  8 ++++
 .../cursor-review/tests/test_build_ledger.py  | 46 +++++++++++++++++++
 2 files changed, 54 insertions(+)
diff --git a/.github/cursor-review/build-ledger.py b/.github/cursor-review/build-ledger.py
index 5d74f3c..b7f8b56 100644
--- a/.github/cursor-review/build-ledger.py
+++ b/.github/cursor-review/build-ledger.py
@@ -104,6 +104,14 @@ def _load_gate_unresolved():
 # post-review.py renders every inline finding as "<emoji> **<Label>** — <body>".
 # Recovering the severity from that badge is reading back our own structured
 # rendering, not inferring a disposition from author prose.
+#
+# The vocabulary and the badge's shape are re-typed here as literals, NOT shared with
+# post-review.py's SEVERITY_LABEL/SEVERITY_EMOJI: `^\S*\s*` accepts any emoji or none
+# on purpose, so the ledger keeps parsing bodies written by an OLDER post-review.py
+# still live on a pinned caller ref, and sharing the strict formatter would lose that.
+# The duplication is pinned instead — test_build_ledger.py drives post-review.py's real
+# renderer through this pattern for EVERY severity, so adding one, renaming a label or
+# changing the dash/spacing fails there rather than silently stopping the strip.
 _BADGE_RE = re.compile(r"^\S*\s*\*\*(Critical|High|Medium|Low|Nit)\*\*\s*—\s*", re.UNICODE)
 
 # Every character that STARTS A NEW LINE for `str.splitlines()` — and so, plausibly,
diff --git a/.github/cursor-review/tests/test_build_ledger.py b/.github/cursor-review/tests/test_build_ledger.py
index 6715596..5608a08 100644
--- a/.github/cursor-review/tests/test_build_ledger.py
+++ b/.github/cursor-review/tests/test_build_ledger.py
@@ -1410,5 +1410,51 @@ def test_the_prose_fallback_marker_still_matches_what_post_review_renders(self):
         )
 
 
+# --------------------------------------------------------------------------- #
+# The severity badge is WRITTEN in post-review.py and READ BACK by _BADGE_RE    #
+# --------------------------------------------------------------------------- #
+
+
+class TestSeverityBadgeContract(unittest.TestCase):
+    """`_BADGE_RE` re-types post-review.py's severity vocabulary as a literal
+    alternation, and its `**Label** — ` shape as a literal too. post-review.py's own
+    `strip_severity_badge` avoids that by rebuilding the badge from the tables that
+    printed it; the ledger deliberately does not, because its tolerant `^\\S*\\s*`
+    prefix has to keep parsing bodies written by an OLDER post-review.py still live on
+    a pinned caller ref. So pin the two here instead: adding a severity, renaming a
+    label, or changing the dash/spacing would leave post-review.py working while the
+    ledger silently stopped stripping, carrying badge-prefixed bodies into the
+    prior-review context and degrading the repeat matching the ledger exists for.
+
+    The badge under test comes from `normalize_comments` — the real renderer — not
+    from a format string re-typed here, which would drift with it.
+    """
+
+    PROSE = "the finding's own prose"
+
+    def _rendered_body(self, severity: str) -> str:
+        comments = pr.normalize_comments(
+            [{"file": "app.py", "line": 12, "severity": severity, "body": self.PROSE}]
+        )
+        self.assertEqual(len(comments), 1)
+        return comments[0]["comment"]["body"]
+
+    def test_every_severity_post_review_renders_is_stripped_by_the_ledger(self):
+        for severity in pr.SEVERITY_ORDER:
+            with self.subTest(severity=severity):
+                body = self._rendered_body(severity)
+                self.assertNotEqual(body, self.PROSE, "post-review.py prefixed a badge")
+                self.assertEqual(bl._strip_badge(body), (severity, self.PROSE))
+
+    def test_the_ledger_recovers_the_severity_of_an_older_or_emojiless_badge(self):
+        """The tolerant prefix is the point of not sharing the formatter — hold it."""
+        for prefix in ("", "🔴 ", "🆕 "):
+            with self.subTest(prefix=prefix):
+                self.assertEqual(
+                    bl._strip_badge(f"{prefix}**Critical** — {self.PROSE}"),
+                    ("critical", self.PROSE),
+                )
+
+
 if __name__ == "__main__":
     unittest.main()

From 493a0e708ab3ceda6c4b24723f9ba2b61ca00bbd Mon Sep 17 00:00:00 2001
From: Matt Miller <mattmiller@comfy.org>
Date: Thu, 27 Aug 2026 07:00:32 -0700
Subject: [PATCH 2/3] test(cursor-review): pin the badge contract on the table
 that gates rendering
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Addresses the cursor-review panel's findings on this PR.

- The contract loop iterated SEVERITY_ORDER, the sort-order table. The
  renderable vocabulary is gated on SEVERITY_EMOJI (normalize_severity) and
  the badge is built from SEVERITY_EMOJI/SEVERITY_LABEL, so a severity added
  to those two but not to SEVERITY_ORDER rendered a badge _BADGE_RE cannot
  strip while the test stayed green — the exact silent strip-stop the pin
  claims to make impossible. Iterate SEVERITY_EMOJI, and assert the three
  tables carry the same keys (which also stops the loop passing vacuously).
- Correct the pin's advertised scope in both the comment and the docstring:
  `\s*` absorbs any spacing change, so the LABELS and the dash CHARACTER are
  pinned, not the whitespace. Verified by mutation: a spacing-only edit still
  strips and stays green; a dash or label change fails.
- Record that `^\S*` swallows any non-whitespace run, not "any emoji or
  none" — a forged prefix strips too — and the caller-trust precondition
  that makes it safe (thread roots of Bot-authored consolidated reviews,
  bodies this workflow wrote), so nobody reuses it on imported PR text.
- Record that a renamed label must be ADDED to the alternation with the old
  one KEPT, since rounds posted by a caller on an older ref still carry it.
- Drop the assertNotEqual whose message stated the passing condition and
  which the following assertEqual subsumes with a better diff.
---
 .github/cursor-review/build-ledger.py         | 22 ++++++++++----
 .../cursor-review/tests/test_build_ledger.py  | 29 ++++++++++++++-----
 2 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/.github/cursor-review/build-ledger.py b/.github/cursor-review/build-ledger.py
index b7f8b56..2c09dc7 100644
--- a/.github/cursor-review/build-ledger.py
+++ b/.github/cursor-review/build-ledger.py
@@ -106,12 +106,22 @@ def _load_gate_unresolved():
 # rendering, not inferring a disposition from author prose.
 #
 # The vocabulary and the badge's shape are re-typed here as literals, NOT shared with
-# post-review.py's SEVERITY_LABEL/SEVERITY_EMOJI: `^\S*\s*` accepts any emoji or none
-# on purpose, so the ledger keeps parsing bodies written by an OLDER post-review.py
-# still live on a pinned caller ref, and sharing the strict formatter would lose that.
-# The duplication is pinned instead — test_build_ledger.py drives post-review.py's real
-# renderer through this pattern for EVERY severity, so adding one, renaming a label or
-# changing the dash/spacing fails there rather than silently stopping the strip.
+# post-review.py's SEVERITY_LABEL/SEVERITY_EMOJI. `^\S*` swallows ANY leading run of
+# non-whitespace — today's emoji, one this version has never seen, or none at all — so
+# the ledger keeps parsing bodies written by an OLDER post-review.py still live on a
+# pinned caller ref; sharing the strict formatter would lose that. The tolerance is not
+# a check: a forged `<anything>**Critical** — ` prefix strips just as happily, which is
+# safe only because the one caller reads thread roots of Bot-authored consolidated
+# reviews — bodies this workflow itself wrote. Narrow it before pointing _strip_badge at
+# imported PR text.
+#
+# The literals pin the LABELS and the dash CHARACTER only; the `\s*` runs absorb any
+# spacing change. The duplication is pinned by test, not shared: test_build_ledger.py
+# drives post-review.py's real renderer through this pattern for every key of
+# SEVERITY_EMOJI — the table `normalize_severity` actually gates the renderable
+# vocabulary on — so adding a severity, renaming a label or changing the dash fails
+# there rather than silently stopping the strip. On a rename, ADD the new label and
+# KEEP the old: rounds already posted by a caller on an older ref still carry it.
 _BADGE_RE = re.compile(r"^\S*\s*\*\*(Critical|High|Medium|Low|Nit)\*\*\s*—\s*", re.UNICODE)
 
 # Every character that STARTS A NEW LINE for `str.splitlines()` — and so, plausibly,
diff --git a/.github/cursor-review/tests/test_build_ledger.py b/.github/cursor-review/tests/test_build_ledger.py
index 5608a08..4c5f871 100644
--- a/.github/cursor-review/tests/test_build_ledger.py
+++ b/.github/cursor-review/tests/test_build_ledger.py
@@ -1421,13 +1421,20 @@ class TestSeverityBadgeContract(unittest.TestCase):
     `strip_severity_badge` avoids that by rebuilding the badge from the tables that
     printed it; the ledger deliberately does not, because its tolerant `^\\S*\\s*`
     prefix has to keep parsing bodies written by an OLDER post-review.py still live on
-    a pinned caller ref. So pin the two here instead: adding a severity, renaming a
-    label, or changing the dash/spacing would leave post-review.py working while the
-    ledger silently stopped stripping, carrying badge-prefixed bodies into the
-    prior-review context and degrading the repeat matching the ledger exists for.
+    a pinned caller ref. So pin the two here instead: adding a severity or renaming a
+    label would leave post-review.py working while the ledger silently stopped
+    stripping, carrying badge-prefixed bodies into the prior-review context and
+    degrading the repeat matching the ledger exists for.
+
+    Scope of the pin, so nobody expects more of it than it gives: the LABELS and the
+    dash CHARACTER are pinned; `_BADGE_RE`'s `\\s*` runs absorb any spacing change
+    around the dash, so a spacing-only edit stays green here and strips fine.
 
     The badge under test comes from `normalize_comments` — the real renderer — not
-    from a format string re-typed here, which would drift with it.
+    from a format string re-typed here, which would drift with it. The loop runs over
+    `SEVERITY_EMOJI`, not `SEVERITY_ORDER`: `normalize_severity` gates the renderable
+    vocabulary on `SEVERITY_EMOJI` membership, so that table — not the sort order — is
+    what decides which badges can ever reach the ledger.
     """
 
     PROSE = "the finding's own prose"
@@ -1439,11 +1446,19 @@ def _rendered_body(self, severity: str) -> str:
         self.assertEqual(len(comments), 1)
         return comments[0]["comment"]["body"]
 
+    def test_the_three_severity_tables_carry_the_same_keys(self):
+        """Guards the loop below, which would pass vacuously over an emptied table —
+        and catches a severity added to EMOJI/LABEL (so post-review.py renders it)
+        but left out of ORDER, which the sort-order loop would never have exercised.
+        """
+        self.assertTrue(pr.SEVERITY_EMOJI)
+        self.assertEqual(set(pr.SEVERITY_EMOJI), set(pr.SEVERITY_LABEL))
+        self.assertEqual(set(pr.SEVERITY_EMOJI), set(pr.SEVERITY_ORDER))
+
     def test_every_severity_post_review_renders_is_stripped_by_the_ledger(self):
-        for severity in pr.SEVERITY_ORDER:
+        for severity in pr.SEVERITY_EMOJI:
             with self.subTest(severity=severity):
                 body = self._rendered_body(severity)
-                self.assertNotEqual(body, self.PROSE, "post-review.py prefixed a badge")
                 self.assertEqual(bl._strip_badge(body), (severity, self.PROSE))
 
     def test_the_ledger_recovers_the_severity_of_an_older_or_emojiless_badge(self):

From 9a2fb6b1dc80d586ddf354b61c16e6d843f195f1 Mon Sep 17 00:00:00 2001
From: Matt Miller <mattmiller@comfy.org>
Date: Thu, 27 Aug 2026 07:53:06 -0700
Subject: [PATCH 3/3] test(cursor-review): pin the ingress vocabulary and the
 default severity too
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Round 2 of the review panel. All six findings were valid; none needed a
runtime change.

Tests:
- `review-output-mcp.py`'s `SEVERITIES` joins the key-set equality. It is the
  fourth copy of the vocabulary and the ingress schema gating what a model may
  submit at all, so a severity added to post-review.py's three tables and to
  `_BADGE_RE` was still rejected at submission with this contract green.
- New `test_the_default_severity_is_one_post_review_can_render`. The equality
  above is satisfied by a key renamed CONSISTENTLY across all three tables,
  which leaves `DEFAULT_SEVERITY` dangling — and `normalize_comments` indexes
  `SEVERITY_EMOJI[severity]` unguarded after `normalize_severity` falls back to
  it, so the first model-supplied unknown severity would `KeyError` and kill
  consolidate after the panel and judge had already run. Degradation elsewhere
  in this class; a crash here.
- A spaceless prefix (`🔴**Critical** — `) joins the tolerant-prefix loop. Every
  other case ended in whitespace, so nothing covered the greedy `^\S*` swallowing
  the label and backtracking — the behaviour build-ledger.py's comment leans on.

Comment above `_BADGE_RE`, corrected where it claimed more than the code does:
- "bodies this workflow itself wrote" overstated the gate. `_consolidated_reviews`
  requires author type Bot plus a marker this public repo puts within anyone's
  reach, so the guarantee is "some Bot wrote it"; the conclusion survives because
  clearing that gate already hands the forger the whole prose.
- The keep-the-old rename rule now covers the em dash, which carries the identical
  old-ref exposure: change it to a class holding both characters, never a swap.
- States that a new label's lowercase must equal its severity key, since
  `_strip_badge` derives the ledger's severity from `match.group(1).lower()`.

Mutation-checked, not argued: dropping `nit` from the ingress tuple fails 1 test,
renaming the `medium` key across all three tables fails 3 (was 1, and none of them
the default-severity relationship), requiring whitespace after the emoji run fails
the spaceless and empty prefixes. Suite: 364 tests OK; 373 on the merged result
against main, alongside agents-md (46), groom (365), shellcheck and bump-callers
(437). No runtime code touched.
---
 .github/cursor-review/build-ledger.py         | 25 ++++++++++----
 .../cursor-review/tests/test_build_ledger.py  | 34 +++++++++++++++++--
 2 files changed, 50 insertions(+), 9 deletions(-)

diff --git a/.github/cursor-review/build-ledger.py b/.github/cursor-review/build-ledger.py
index 2c09dc7..03953a0 100644
--- a/.github/cursor-review/build-ledger.py
+++ b/.github/cursor-review/build-ledger.py
@@ -110,18 +110,31 @@ def _load_gate_unresolved():
 # non-whitespace — today's emoji, one this version has never seen, or none at all — so
 # the ledger keeps parsing bodies written by an OLDER post-review.py still live on a
 # pinned caller ref; sharing the strict formatter would lose that. The tolerance is not
-# a check: a forged `<anything>**Critical** — ` prefix strips just as happily, which is
-# safe only because the one caller reads thread roots of Bot-authored consolidated
-# reviews — bodies this workflow itself wrote. Narrow it before pointing _strip_badge at
-# imported PR text.
+# a check: a forged `<anything>**Critical** — ` prefix strips just as happily. What the
+# one caller actually enforces on the bodies it feeds here is _consolidated_reviews'
+# gate — author type Bot, PLUS a marker that this public repo puts within anyone's
+# reach — so the guarantee is "some Bot wrote it", not "this workflow wrote it". That
+# is enough only because clearing that gate already hands the forger the entry's whole
+# prose, badge or no badge. Narrow the prefix before pointing _strip_badge at imported
+# PR text, where the author is not gated at all.
 #
 # The literals pin the LABELS and the dash CHARACTER only; the `\s*` runs absorb any
 # spacing change. The duplication is pinned by test, not shared: test_build_ledger.py
 # drives post-review.py's real renderer through this pattern for every key of
 # SEVERITY_EMOJI — the table `normalize_severity` actually gates the renderable
 # vocabulary on — so adding a severity, renaming a label or changing the dash fails
-# there rather than silently stopping the strip. On a rename, ADD the new label and
-# KEEP the old: rounds already posted by a caller on an older ref still carry it.
+# there rather than silently stopping the strip.
+#
+# Two editing rules that red test cannot state for you:
+#   * ADD, never replace. On a label rename, keep the old label in the alternation
+#     beside the new one — rounds already posted by a caller pinned to an older ref
+#     still carry it. The dash has the IDENTICAL old-ref exposure, so a dash change
+#     means widening `—` to a character class holding both, not swapping it: replace
+#     it and those rounds stop stripping silently, with nothing left to fail.
+#   * A new label's lowercase MUST equal its severity key. _strip_badge derives the
+#     ledger's severity from `match.group(1).lower()`, so a label-only rename (`Nit`
+#     -> `Nitpick`, key still `nit`) files entries under a severity string none of
+#     post-review.py's tables use. Rename both, or map label -> key here.
 _BADGE_RE = re.compile(r"^\S*\s*\*\*(Critical|High|Medium|Low|Nit)\*\*\s*—\s*", re.UNICODE)
 
 # Every character that STARTS A NEW LINE for `str.splitlines()` — and so, plausibly,
diff --git a/.github/cursor-review/tests/test_build_ledger.py b/.github/cursor-review/tests/test_build_ledger.py
index 4c5f871..a11c1d2 100644
--- a/.github/cursor-review/tests/test_build_ledger.py
+++ b/.github/cursor-review/tests/test_build_ledger.py
@@ -41,6 +41,7 @@ def _load(name, filename):
 
 bl = _load("build_ledger", "build-ledger.py")
 pr = _load("post_review", "post-review.py")
+mcp = _load("review_output_mcp", "review-output-mcp.py")
 
 MARKER = bl.CONSOLIDATED_MARKER
 
@@ -1446,14 +1447,35 @@ def _rendered_body(self, severity: str) -> str:
         self.assertEqual(len(comments), 1)
         return comments[0]["comment"]["body"]
 
-    def test_the_three_severity_tables_carry_the_same_keys(self):
+    def test_every_copy_of_the_severity_vocabulary_carries_the_same_keys(self):
         """Guards the loop below, which would pass vacuously over an emptied table —
         and catches a severity added to EMOJI/LABEL (so post-review.py renders it)
         but left out of ORDER, which the sort-order loop would never have exercised.
+
+        `review-output-mcp.py`'s SEVERITIES is in here as the fourth copy: it is the
+        ingress schema deciding what a model may submit at all, so a severity added to
+        post-review.py's tables and to `_BADGE_RE` but not to it is rejected at
+        submission and never reaches either end of the badge contract.
         """
         self.assertTrue(pr.SEVERITY_EMOJI)
         self.assertEqual(set(pr.SEVERITY_EMOJI), set(pr.SEVERITY_LABEL))
         self.assertEqual(set(pr.SEVERITY_EMOJI), set(pr.SEVERITY_ORDER))
+        self.assertEqual(set(pr.SEVERITY_EMOJI), set(mcp.SEVERITIES))
+
+    def test_the_default_severity_is_one_post_review_can_render(self):
+        """The one relationship here whose breach CRASHES rather than degrades.
+
+        `normalize_severity` maps every unknown/missing severity to DEFAULT_SEVERITY
+        and `normalize_comments` then indexes `SEVERITY_EMOJI[severity]` unguarded, so
+        a severity key renamed consistently across all three tables leaves the equality
+        above green while the first model-supplied unknown severity raises KeyError —
+        killing the consolidate job after the whole panel and the judge have run.
+        """
+        self.assertIn(pr.DEFAULT_SEVERITY, pr.SEVERITY_EMOJI)
+        self.assertEqual(
+            bl._strip_badge(self._rendered_body("not-a-severity")),
+            (pr.DEFAULT_SEVERITY, self.PROSE),
+        )
 
     def test_every_severity_post_review_renders_is_stripped_by_the_ledger(self):
         for severity in pr.SEVERITY_EMOJI:
@@ -1462,8 +1484,14 @@ def test_every_severity_post_review_renders_is_stripped_by_the_ledger(self):
                 self.assertEqual(bl._strip_badge(body), (severity, self.PROSE))
 
     def test_the_ledger_recovers_the_severity_of_an_older_or_emojiless_badge(self):
-        """The tolerant prefix is the point of not sharing the formatter — hold it."""
-        for prefix in ("", "🔴 ", "🆕 "):
+        """The tolerant prefix is the point of not sharing the formatter — hold it.
+
+        The spaceless entry is not padding: `^\\S*` is greedy, so on `🔴**Critical** — `
+        it first swallows the label too and only backtracks to the emoji. That is the
+        behaviour the comment in build-ledger.py leans on, and no other case here
+        exercises it — every other prefix ends in the whitespace `\\s*` consumes.
+        """
+        for prefix in ("", "🔴 ", "🆕 ", "🔴"):
             with self.subTest(prefix=prefix):
                 self.assertEqual(
                     bl._strip_badge(f"{prefix}**Critical** — {self.PROSE}"),
