fix(check-pr-fully-clean): stop scoring non-blocking as a finding

scripts/check-pr-fully-clean.py

@@ -1232,20 +1232,45 @@ def strip_cited_finding_vocab(text: str) -> str:
 # `Block(?:ed|ing)?` needs lookbehinds because `\b` treats a hyphen as a
 # boundary, so "non-blocking" -- how a reviewer marks a nit as NOT blocking
 # -- read a Ready-for-merge review as not-clean (ai-config#2369, measured
-# 2026-08-26 on #2288). Only the `non-`/`non ` compounds are exempted.
-# "previously-blocking" is deliberately NOT exempted, although it produces a
-# safe-direction false positive when narrating a fixed finding: "the
-# previously-blocking finding remains open; do not merge" is a real
-# not-clean statement, and a lexical lookbehind cannot tell it from "the
-# previously-blocking error was fixed". Missing a not-clean is the dangerous
-# direction, so the narration form stays an over-flag -- as does any other
-# `-blocking` compound ("merge-blocking" is a real signal) and the
-# emphasized form ("non-**blocking**": the char before `blocking` is `*`,
-# which the lookbehind cannot see through).
+# 2026-08-26 on #2288). "non-"/"non " were the only compounds exempted at
+# first; "not blocking" and "no blocking findings remain" reproduced the
+# identical false positive on #3468 (ai-config#3487, measured 2026-09-09),
+# because a hyphen or space after "non" is not the only way a reviewer
+# negates the word -- "not " and "no " immediately before it are the same
+# statement in different words. Four prefixes are now guarded: `non-`,
+# `non `, `not `, `no `. The hyphenated forms of the latter two --
+# "not-blocking", "no-blocking" -- are deliberately NOT exempted and stay
+# flagged: this is the same over-flag-rather-than-swallow direction as
+# "previously-blocking" just below, not an oversight (adding them would mean
+# widening word-boundary lookbehinds the same blanket way that, on the
+# `NOT_CLEAN_NEGATION_PREFIX` guard a few hundred lines down, turned out to
+# swallow real not-clean statements like "Not-negligible changes requested."
+# -- see that guard's own comment). "previously-blocking" is deliberately NOT
+# exempted either, although it produces a safe-direction false positive when
+# narrating a fixed finding: "the previously-blocking finding remains open;
+# do not merge" is a real not-clean statement, and a lexical lookbehind
+# cannot tell it from "the previously-blocking error was fixed". Missing a
+# not-clean is the dangerous direction, so the narration form stays an
+# over-flag -- as does any other `-blocking` compound ("merge-blocking" is
+# a real signal) and the emphasized form ("non-**blocking**": the char
+# before `blocking` is `*`, which the lookbehind cannot see through). A
+# negator that is NOT immediately adjacent also stays flagged, on purpose:
+# "this is not a nit -- it is blocking" has "not" five words away from
+# "blocking", so none of the fixed-width lookbehinds below fire on it, and
+# the sentence is correctly read as a live finding.
+#
+# `Rejected` and `Unapproved` were checked for the same treatment
+# (ai-config#3487's own question) and deliberately left unguarded: unlike
+# "non-blocking", there is no established review idiom "non-rejected" or
+# "not unapproved" -- a grep of this corpus's own prose and test fixtures
+# turns up no such usage anywhere. Adding a speculative guard for a phrasing
+# that does not occur would only buy risk (silently swallowing a genuine
+# "Rejected" or "Unapproved" finding) for zero real benefit. Revisit if a
+# real occurrence ever surfaces.
 _BARE_REJECTION = (
     r"\b(?:Rejected|Unapproved|"
-    r"(?<!non-)(?<!non\s)Block(?:ed|ing)?"
-    r"|Impasse|Deadlock|Changes\s+requested|Actionable\s+findings"
+    r"(?<!\bnon-)(?<!\bnon\s)(?<!\bnot\s)(?<!\bno\s)Block(?:ed|ing)?"
+    r"|Impasse|Deadlock|(?<!\bno-)Changes\s+requested|Actionable\s+findings"
     r"|Partial\s+review)\b"
 )
 
@@ -1820,30 +1845,33 @@ def _is_exempt_findings_heading(
     # already existed for `no changes requested`.
     r"\bNeeds\s+(?:(?!no\b|nothing\b|none\b)\w+\s+){0,3}work\b",
     r"Verdict:\s*(?:Ready after addressing findings|Changes requested|Actionable findings|Block(?:ed|ing)?|Rejected|Unapproved|Impasse|Deadlock|Partial review)",
-    r"changes\s+requested\b",
+    r"(?<!\bno-)changes\s+requested\b",
     _BARE_REJECTION,
     r"\[FINDINGS_COUNT:\s*[1-9]\d*\]",  # Machine-readable finding count > 0
     r"\b(?:not|never|no|isn't|aren't|wasn't|cannot|can't|unapproved|rejected)\s+(?:\w+\s+){0,2}(?:clean|approved|ready|lgtm|approval)\b",
     r"\b(?:omitted\s+region\s+was\s+not\s+assessed|diff\s+was\s+truncated)\b",
     r"\bnot\s+(?:an\s+)?approval\s+of\s+the\s+(?:MR|PR)\s+as\s+a\s+whole\b",
 ]
 
-# Applies to EVERY not-clean pattern, not to one named member.
+# Space-only, deliberately. This guard is consulted for EVERY pattern in
+# `VERDICT_NOT_CLEAN_PATTERNS` / `FINDING_PATTERNS`, so a hyphen alternative
+# here does not stay near the phrase it was written for: granting `non-`/`no-`
+# silently read `non-rejected`, `no-rejected`, `non-unapproved`, `non-impasse`
+# and `non-deadlock` as clean, which is a swallowed not-clean and the
+# dangerous direction (ai-config#3497 review).
 #
-# This guard already existed, as an `if pat == r"changes\s+requested\b"` branch
-# inside the matching loop -- so a sibling pattern added to the list above got
-# no negation handling at all, which is precisely what happened. Enumerating
-# which patterns need the guard is the same failure this file has already lost
-# to twice on the clean side.
+# Hyphen handling belongs on the phrase that needs it. `no-changes requested`
+# is one phrase, so it carries its own `(?<!\bno-)`, in `_BARE_REJECTION` and
+# in both pattern lists. `non-blocking` -- the case ai-config#3487 is about --
+# needs nothing here either: `_BARE_REJECTION`'s own `(?<!\bnon-)` lookbehind
+# already covers it.
 #
-# Adjacency-anchored rather than a bare negator search anywhere in the prefix,
-# and that is what keeps it in the safe direction. Missing a not-clean signal
-# is the dangerous direction here, so the guard must not fire on a negator
-# belonging to an earlier clause: the `\w+\s+` filler cannot cross punctuation,
-# so `This is not done. Needs work` and `It is not ready; needs more work` both
-# stay not-clean.
+# The filler-word branch stays space-separated. `not-negligible` and
+# `no-nonsense` are compound adjectives whose hyphen does not negate what
+# follows, and reading them as negated filler swallows a real finding.
 NOT_CLEAN_NEGATION_PREFIX = re.compile(
-    r"\b(?:no|not|nothing|none|never)\s+(?:\w+\s+){0,2}$", re.IGNORECASE
+    r"\b(?:no|not|nothing|none|never)\s+(?:\w+\s+){0,2}$",
+    re.IGNORECASE,
 )
 # Two alternation groups on purpose. Emphasis markers are tolerated ONLY
 # before the alternatives that are unambiguous negations when they open the
@@ -1915,7 +1943,7 @@ def _is_exempt_findings_heading(
     r"\*\*Location:\*\*",
     r"Verdict:\s*(?:Ready after addressing findings|Needs work|Needs more work|Changes requested|Actionable findings|Block(?:ed|ing)?|Rejected|Unapproved|Impasse|Deadlock|Partial review)",
     r"\bNeeds\s+(?:(?!no\b|nothing\b|none\b)\w+\s+){0,3}work\b",
-    r"changes\s+requested\b",
+    r"(?<!\bno-)changes\s+requested\b",
     _BARE_REJECTION,
     r"\[FINDINGS_COUNT:\s*[1-9]\d*\]",  # Machine-readable finding count > 0
     r"\b(?:not|never|no|isn't|aren't|wasn't|cannot|can't|unapproved|rejected)\s+(?:\w+\s+){0,2}(?:clean|approved|ready|lgtm|approval)\b",
@@ -2340,11 +2368,6 @@ def _unresolved_finding_pattern(body: str) -> Optional[str]:
                 suffix = scan_body[match.end():match.end() + 60]
                 if NOT_CLEAN_NEGATION_SUFFIX.search(suffix):
                     continue
-            if pat == r"changes\s+requested\b":
-                start = match.start()
-                pfx = scan_body[max(0, start - 25):start].lower()
-                if re.search(r"\bno\s+(\w+\s+)?$", pfx):
-                    continue
             return pat
     return None
 

scripts/test_check_pr_fully_clean.py

@@ -1364,6 +1364,10 @@ def main() -> int:
         "Nothing here needs any further work.",
         "No changes requested.",
         "There are no changes requested on this round.",
+        # ai-config#3487: the guard's prefix test required a space between
+        # "no" and what follows, so a hyphenated negation slipped through
+        # even though it says the identical thing.
+        "No-changes requested.",
     ):
         check(
             f"classify_verdict: a NEGATED not-clean phrase is not a verdict -- {phrase!r}",
@@ -1854,6 +1858,147 @@ def main() -> int:
           checker.classify_verdict(
               "### Verdict\nNits below, plus the previously blocking crash which is NOT fixed.\n", "")
           == "not-clean")
+
+    # ai-config#3487: the #2369 fix above only exempted the "non-"/"non "
+    # compounds from the bare `_BARE_REJECTION` pattern itself. "not
+    # blocking" and "no blocking findings remain" say the exact same thing
+    # with a different negator immediately in front of the word, and the
+    # bare pattern matched both with no guard at all -- confirmed by
+    # reverting the `(?<!not\s)(?<!no\s)` lookbehinds and re-running these
+    # two checks, which then fail (match found instead of none).
+    #
+    # These two are asserted at the bare-pattern level, not through
+    # classify_verdict/`_unresolved_finding_pattern`: both of those already
+    # route every `_BARE_REJECTION` match through `NOT_CLEAN_NEGATION_PREFIX`
+    # (below), whose word list already includes "no"/"not" with a plain
+    # space -- so a classify_verdict-level check of the same two phrases
+    # passes identically with or without this fix and would not actually
+    # exercise it. The bare pattern is still worth guarding directly: other
+    # code (and this test file's own #2369 checks) matches `_BARE_REJECTION`
+    # standalone, with no downstream negation guard at all.
+    check("_BARE_REJECTION no longer matches inside 'not blocking'",
+          not _re.search(checker._BARE_REJECTION,
+                         "this is not blocking the merge", _re.I))
+    check("_BARE_REJECTION no longer matches inside 'no blocking findings remain'",
+          not _re.search(checker._BARE_REJECTION,
+                         "no blocking findings remain", _re.I))
+
+    # #3497 review: the hyphen branch of NOT_CLEAN_NEGATION_PREFIX was first
+    # written as `(?:no|not|nothing|none|never)(?:-|\s+...)`, which exempted
+    # `not-blocking` / `no-blocking` / `not-rejected` through the FULL
+    # pipeline -- undoing one layer up the narrowness the lookbehinds above
+    # preserve, in the dangerous direction. `non-X` is a negating compound;
+    # `not-X` is two words someone hyphenated.
+    #
+    # These are asserted through classify_verdict on purpose. The comment
+    # above explains why the bare level is the right place for the SPACE
+    # forms; it does not carry to the hyphen forms, whose whole failure mode
+    # lived in the generic guard rather than in the bare pattern. That gap is
+    # why the first version of this fix passed 797 tests with the regression
+    # in it.
+    # All five negators, including `no`. An earlier round exempted `no-`
+    # in the generic prefix guard to keep `no-changes requested` clean, and
+    # that swallowed `no-blocking` with it. The hyphen now lives on the
+    # `Changes\s+requested` alternative itself, so the generic guard stays
+    # space-only and every hyphenated negator before a not-clean phrase
+    # stays flagged.
+    for _neg in ("not", "no", "never", "nothing", "none"):
+        check(f"classify_verdict: '{_neg}-blocking' stays not-clean",
+              checker.classify_verdict(
+                  f"### Verdict\nReady for merge. {_neg}-blocking nit noted.\n",
+                  "") == "not-clean")
+    check("classify_verdict: 'not-rejected' stays not-clean",
+          checker.classify_verdict(
+              "### Verdict\nReady for merge. not-rejected nit noted.\n", "")
+          == "not-clean")
+    check("_unresolved_finding_pattern: 'not-blocking' still reports a finding",
+          checker._unresolved_finding_pattern(
+              "### Verdict\nReady for merge. not-blocking nit noted.\n")
+          is not None)
+    # The case the fix exists for must still pass, through the same pipeline.
+    # The five phrases the generic hyphen exemption silently swallowed
+    # before it was withdrawn (#3497 review): every alternative of
+    # `_BARE_REJECTION`, not just the one the fix was written for.
+    for _phrase in ("rejected", "unapproved", "impasse", "deadlock"):
+        check(f"classify_verdict: 'non-{_phrase}' stays not-clean",
+              checker.classify_verdict(
+                  f"### Verdict\nReady for merge. non-{_phrase} nit noted.\n",
+                  "") == "not-clean")
+    check("classify_verdict: 'no-rejected' stays not-clean",
+          checker.classify_verdict(
+              "### Verdict\nReady for merge. no-rejected nit noted.\n", "")
+          == "not-clean")
+    # #3497 review round 4: the lookbehinds had no word boundary, so any
+    # longer word ending in "no"/"not" swallowed the phrase after it.
+    for _word in ("volcano", "casino", "domino"):
+        check(f"classify_verdict: '{_word}-changes requested' stays not-clean",
+              checker.classify_verdict(
+                  f"### Verdict\n{_word.capitalize()}-changes requested.\n",
+                  "") == "not-clean")
+        check(f"classify_verdict: '{_word} blocking' stays not-clean",
+              checker.classify_verdict(
+                  f"### Verdict\nReady for merge. {_word} blocking issue "
+                  "remains.\n", "") == "not-clean")
+    check("classify_verdict: 'no-changes requested' stays exempt (#2369)",
+          checker.classify_verdict(
+              "### Verdict\nNo-changes requested.\n", "") != "not-clean")
+    check("classify_verdict: 'non-blocking' is still exempt",
+          checker.classify_verdict(
+              "### Verdict\nReady for merge. non-blocking nit noted.\n", "")
+          == "clean")
+    # The dangerous direction stays covered: a negator that is NOT
+    # immediately adjacent to "blocking" must still read as a live finding,
+    # because the fixed-width lookbehind only ever looks at the three or
+    # four characters right before the word. "it is not a nit -- it is
+    # blocking" and "there is no doubt this is blocking" both put other
+    # words between the negator and "blocking", so neither is swallowed --
+    # this is the guard against the "blanket negation window" anti-pattern
+    # the issue explicitly warns against. Checked at both the bare-pattern
+    # level and through the full classify_verdict pipeline.
+    check("_BARE_REJECTION still matches 'not a nit -- it is blocking' (negator not adjacent)",
+          bool(_re.search(checker._BARE_REJECTION,
+                          "this is not a nit -- it is blocking", _re.I)))
+    check("_BARE_REJECTION still matches 'no doubt this is blocking' (negator not adjacent)",
+          bool(_re.search(checker._BARE_REJECTION,
+                          "there is no doubt this is blocking", _re.I)))
+    check("classify_verdict: 'not a nit -- it is blocking' stays not-clean",
+          checker.classify_verdict(
+              "### Verdict\nThis is not a nit -- it is blocking.\n", "")
+          == "not-clean")
+    check("classify_verdict: 'no doubt this is blocking' stays not-clean",
+          checker.classify_verdict(
+              "### Verdict\nThere is no doubt this is blocking merge.\n", "")
+          == "not-clean")
+
+    # ai-config#3487: NOT_CLEAN_NEGATION_PREFIX's negator-to-phrase gap used
+    # a bare `\s`, so a hyphenated negation ("No-changes requested.") was
+    # NOT exempted even though the word-spaced form ("No changes
+    # requested.") already was -- the same hyphen-vs-space gap "non-blocking"
+    # hit, one guard over. Mutation check: reverting the hyphen branch back
+    # to a bare `\s` in NOT_CLEAN_NEGATION_PREFIX makes this fail (returns
+    # "not-clean" instead of "").
+    check("classify_verdict: 'No-changes requested.' (hyphenated) is not a verdict",
+          checker.classify_verdict("### Verdict\nNo-changes requested.\n", "") == "")
+    # Round-2 adversarial review of #3487: the FIRST attempt at the check
+    # above widened `\s` to `[\s-]` everywhere in NOT_CLEAN_NEGATION_PREFIX,
+    # including inside the `\w+\s+` filler -- which let a hyphen stand in
+    # for a space between the negator and an intervening word, not just
+    # between the negator and the phrase itself. That swallowed any
+    # hyphenated compound ADJECTIVE that happens to open with a negator
+    # word, which has nothing to do with negating what follows: reverting
+    # to that blanket `[\s-]` version makes every check below fail (each
+    # returns "" or None instead of "not-clean"/a real finding).
+    for phrase in (
+        "Not-negligible changes requested.",
+        "No-nonsense changes requested here.",
+        "Not-yet-addressed changes requested below.",
+        "Never-resolved changes requested.",
+    ):
+        check(
+            f"classify_verdict: hyphenated compound adjective before a real "
+            f"not-clean phrase still reads not-clean -- {phrase!r}",
+            checker.classify_verdict(f"### Verdict\n{phrase}\n", "") == "not-clean",
+        )
     check("a previously blocking failure explicitly fixed is not an active finding",
           checker._unresolved_finding_pattern(
               "### Verdict\n**Ready for merge.** The previously blocking "