Compare Vetted Reviewer Markers Without Regard to Letter Case

scripts/README.md

@@ -183,7 +183,7 @@ A partial round is reported and handed over rather than retried into. Measured o
 
 `status` and `wait` both exit `43` where the reviewer sent a **shape this script has no reader for**, and that outcome outranks every other reading here, because a reader that does not understand the output cannot be believed about what it found in it. Every rule in this script keys on a structural marker, so a marker that changes spelling is a section the reader stops finding and reports as absent. All three failures on record have that exact shape: the suppressed heading reworded and the count went to zero, the suppressed section moved inside another wrapper and the count went to zero again, and the coverage line was never parsed at all. Each reported a clean pass over a review it had misread, and each was found by the maintainer afterwards rather than by the gate. The digest carries `shapes=UNRECOGNIZED`, lists each shape it could not place, and names the remedy in two parts: **file an issue on the hub repository that hosts this script**, quoting the body the shape came from, and leave the merge decision to the maintainer, since an unrecognized shape does not say the pull request is bad, only that nothing here can vouch for the review of it.
 
-The vetted inventory is measured rather than imagined, and it is small because the output is regular. Across those 332 bodies, with fenced blocks dropped and text reduced to ASCII, the whole corpus is seven headings, six `<summary>` texts and three metadata labels. Counts are normalized to `(N)` and the verdict headings' colored circle is dropped before comparing, since both change on every review without the section having changed, and dropping the emoji is also what keeps this repository's charset rule satisfied. A body carrying **no** heading at all is itself unrecognized, which is what catches a rewrite that changes every marker at once, and a **refusal is exempt** because it is a bare paragraph by design and `REFUSAL` is its vetted spelling. That exemption is the pattern rather than a carve-out, so a refusal reworded stops being exempt and blocks, which is the refusal check's own failure mode caught one rewording later. The last reading is the quietest: a reviewer **login** that reads as this reviewer without being the spelling every query filters on, since a rename leaves every filter matching nothing and the digest then reports a review that landed as no review at all. A case runs the whole inventory over the measured corpus, where it raises nothing.
+The vetted inventory is measured rather than imagined, and it is small because the output is regular. Across those 332 bodies, with fenced blocks dropped and text reduced to ASCII, the whole corpus is nine headings, six `<summary>` texts and three metadata labels. Counts are normalized to `(N)` and the verdict headings' colored circle is dropped before comparing, since both change on every review without the section having changed, and dropping the emoji is also what keeps this repository's charset rule satisfied. Letter case is folded at the comparison rather than in the reduction, so a marker keeps the case the reviewer wrote while a drift of case alone stops blocking a loop, which it did once, on a heading already in the inventory. A body carrying **no** heading at all is itself unrecognized, which is what catches a rewrite that changes every marker at once, and a **refusal is exempt** because it is a bare paragraph by design and `REFUSAL` is its vetted spelling. That exemption is the pattern rather than a carve-out, so a refusal reworded stops being exempt and blocks, which is the refusal check's own failure mode caught one rewording later. The last reading is the quietest: a reviewer **login** that reads as this reviewer without being the spelling every query filters on, since a rename leaves every filter matching nothing and the digest then reports a review that landed as no review at all. A case runs the whole inventory over the measured corpus, where it raises nothing.
 `wait` exits `44` when the review loop has closed but a required check sits in a shape no wait clears. The digest carries `checks=N/M` beside the merge word and names the shape as `stuck=`, because `mergeStateStatus` reports one word, `BLOCKED`, for a red check, a check nothing is running, an unresolved thread, and a missing approval alike, and a run in this repository spent twenty-five minutes polling that word on a pull request whose only unfinished check was an aggregator job no runner ever took. The cause came from the maintainer rather than from any field here, which is the whole defect: the digest named the state it could not explain and stopped there. Four shapes are told apart because each wants a different response. `NOT_POSTED` is a required status whose poster has not spoken, which is a `StatusContext`'s `EXPECTED` and only ever that. It is deliberately not folded in with the starved shape, since no runner is owed a status nothing has posted, so re-running a workflow clears nothing and the starved wording would send a reader at the runner pool over a missing poster. `NOT_PICKED_UP` is a check GitHub dispatched and assigned no runner, read from the queued state rather than from a runner name GraphQL does not carry, and the state suffices because a job held behind a `needs:` dependency does not enter the rollup until that dependency finishes, so there is no dependency-blocked queue to mistake for a starved one. Nothing agent-side starts it, since the pool is GitHub-hosted, so the remedy is a re-run or that capacity. `RUNNING_LONG` is deliberately the weaker reading and its wording says so, because duration alone cannot separate a hung job from a slow one: this repository's lint job legitimately runs nine to eleven minutes while its aggregator is a single shell conditional, so the threshold is generous, the elapsed time prints for the reader to judge against what the job costs, and nothing asserts a fault. `FAILED` is a verdict rather than a stuck check, reported so no reader deduces a red check from `BLOCKED`.
 
 A check merely still running normally is **not** any of these and exits `0`. That boundary is the whole design, because `wait` returns the moment coverage lands and on almost every pull request the checks are still going at that instant, so taking `44` for a pending check would make `44` the ordinary outcome and a code that fires always carries nothing. `44` additionally requires `mergeStateStatus: BLOCKED`, which the module docstring's own list of shapes has to name rather than only implying, because a reader who sees `stuck=FAILED` and exit `0` on a merge that is `UNSTABLE` should find the condition written down rather than infer the field is unreliable. It is because a rollup carries checks the ruleset does not require and four of the six on a green pull request here are exactly that, so the code borrows GitHub's own reading of which checks gate a merge rather than reading the ruleset's contexts over another call. `CLEAN` proves no required gate is outstanding whatever else the rollup is doing, and without that condition a stuck check nothing requires returns `44` on a mergeable pull request. The digest names the check either way, so the narrower code costs the reader nothing. Both of those came out of this change's own review.

scripts/pr_review.py

@@ -297,7 +297,7 @@
 # A heading this script has no spelling for is a section it will not find, reported as absent.
 # That is the shape of all three failures already on record here, each caught after it landed.
 # The lists are small because the output is regular: 9 headings, 6 summaries and 3 labels.
-# Counts are normalized to `(N)` and non-ASCII is dropped before comparing.
+# Counts are normalized to `(N)` and non-ASCII is dropped before comparing, and `unvetted` folds letter case at the comparison itself.
 # The verdict headings carry a colored circle, so the emoji is what would drift most cheaply.
 # Dropping it also keeps this file inside the charset rule that governs the repository.
 VETTED_HEADINGS = {
@@ -1083,7 +1083,9 @@ def table_against_diff(pr: dict, counts: tuple[int, int] | None) -> str:
 
 
 def normal(text: str) -> str:
-    """A marker reduced to what a vetted list compares: ASCII, single spaces, counts as `(N)`.
+    """A marker reduced toward what a vetted list compares: ASCII, single spaces, counts as `(N)`.
+
+    Letter case survives, and `unvetted` folds it at the membership test instead.
 
     The verdict headings carry a colored circle and the suppressed heading carries its finding
     count, so both drift on every review without the section having changed at all.
@@ -1092,6 +1094,13 @@ def normal(text: str) -> str:
     return re.sub(r"\s+", " ", re.sub(r"\(\d+\)", "(N)", ascii_only)).strip()
 
 
+def unvetted(marker: str, vetted: set[str]) -> bool:
+    """Whether `marker` is absent from `vetted`, compared without regard to letter case."""
+    # Folded here rather than in `normal`, whose value the report strings also carry.
+    # A report naming a folded shape would not match the body it asks the reader to quote beside it.
+    return marker.casefold() not in {v.casefold() for v in vetted}
+
+
 def unrecognized_in(body: str) -> list[str]:
     """Every marker in one review body this script has no vetted spelling for.
 
@@ -1112,13 +1121,16 @@ def unrecognized_in(body: str) -> list[str]:
     plain = FENCE.sub("", body or "")
     headings = [normal(ln) for ln in plain.splitlines() if MARKDOWN_HEADING.match(ln)]
     labels = [normal(m.group(1)) for m in map(LABEL_LINE.match, plain.splitlines()) if m]
-    found = [f"heading: {h}" for h in dict.fromkeys(headings) if h not in VETTED_HEADINGS]
+    found = [f"heading: {h}" for h in dict.fromkeys(headings) if unvetted(h, VETTED_HEADINGS)]
+    found += [
+        f"summary: {marker}"
+        # Deduplicated on the raw text before normalizing, which is what it did before the hoist.
+        for marker in (normal(s) for s in dict.fromkeys(SUMMARY.findall(plain)))
+        if unvetted(marker, VETTED_SUMMARIES)
+    ]
     found += [
-        f"summary: {normal(s)}"
-        for s in dict.fromkeys(SUMMARY.findall(plain))
-        if normal(s) not in VETTED_SUMMARIES
+        f"metadata label: {la}" for la in dict.fromkeys(labels) if unvetted(la, VETTED_LABELS)
     ]
-    found += [f"metadata label: {la}" for la in dict.fromkeys(labels) if la not in VETTED_LABELS]
     found += [
         f"coverage line: {ln}" for ln in coverage_statements(body) if read_coverage(ln) is None
     ]

scripts/tests/test_pr_review.py

@@ -1594,6 +1594,42 @@ def digest_for(self, *reviews: dict) -> str:
         out, _ = pr_review.digest("o", "r", 7)
         return out
 
+    def test_a_vetted_marker_survives_a_change_of_letter_case(self) -> None:
+        """Case carries no meaning in these markers, and the reviewer has drifted one.
+
+        A body carrying `### Reviewed Changes` reported as a shape never seen while
+        `### Reviewed changes` sat in the vetted list, which blocked a review loop over one letter
+        on a round every reader had parsed correctly.
+        """
+        for vetted in ("### Reviewed changes", "## Pull request overview"):
+            drifted = vetted.title()
+            self.assertNotEqual(drifted, vetted, "this case pair no longer differs")
+            self.assertEqual(
+                [], pr_review.unrecognized_in(OVERVIEW + "\n" + drifted), f"blocked on {drifted}"
+            )
+        # One case per vetted list, since a one-arm revert would otherwise stay green.
+        # Not the files-reviewed label, which is also read as a coverage line and judged separately.
+        self.assertEqual([], pr_review.unrecognized_in(OVERVIEW + "\n- **comments generated:** 2"))
+        self.assertEqual(
+            [],
+            pr_review.unrecognized_in(OVERVIEW + "\n<details><summary>Review Details</summary>"),
+        )
+
+    def test_a_genuinely_unknown_marker_is_still_reported(self) -> None:
+        """The floor under the case above, since folding could have turned the check off.
+
+        A vetting list that stops reporting is indistinguishable from a body with nothing to
+        report, which is the failure the list exists to make loud.
+        """
+        for shape, unknown in (
+            ("heading", "### Reviewed Chances"),
+            ("metadata label", "- **Review depth level:** deep"),
+        ):
+            found = pr_review.unrecognized_in(OVERVIEW + "\n" + unknown)
+            self.assertTrue(
+                any(f.startswith(shape) for f in found), f"{unknown!r} passed as vetted: {found}"
+            )
+
     def test_every_vetted_marker_together_reads_as_recognized(self) -> None:
         """The corpus shape in one body, so the lists are held against what they were built from."""
         self.assertEqual([], pr_review.unrecognized_in(nested()))