fix(source-control): babysit snapshot classifies Approve-with-nits bot review as non-blocking (#499)

plugins/source-control/.claude-plugin/plugin.json

@@ -1,7 +1,7 @@
 {
   "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json",
   "name": "source-control",
-  "version": "0.11.0",
+  "version": "0.12.0",
   "description": "Git and GitHub delivery workflow: /commit (Conventional Commits + Co-Authored-By trailer via safe heredoc mechanics), /pull-request (prep, create, CI monitoring, review-comment triage, merge, CI-log fetch), /babysit-prs (self-pacing fleet loop — safe by default; opt-in worker/autopilot tiers add gate-checked merge and thread resolution behind a deterministic Python engine), /worktree (create, status, cleanup, audit for parallel-session isolation), /setup (check the effective commit-subject / PR-title convention and babysit-prs config, or apply — interview the repo and write the tracked convention config), and /resolve-conflicts (intent-first merge/rebase conflict resolution with a semantic-conflict sweep — never --abort). The commit-subject / PR-title convention is configurable per repo via a tracked .claude/source-control.md config written by a re-runnable setup skill; Conventional Commits is the default when no convention is declared.",
   "author": {
     "name": "Melodic Software",
@@ -82,7 +82,7 @@
       "type": "string",
       "multiple": true,
       "title": "Babysit approval-downgrade reviewer logins",
-      "description": "AI reviewer logins whose APPROVED verdict is downgraded when the review body still carries blocking language. Absent: the downgrade heuristic stays dormant."
+      "description": "AI reviewer logins whose approval is surfaced as a `material` finding instead of `ignored` in the one case the structural approval-downgrade reaches: a review body carrying blocking-looking prose that still parses as an approval verdict (no CRITICAL/IMPORTANT or required-fix marker). Every bot's such approval is downgraded to non-blocking regardless; naming a login opts its own into the more-conservative `material` bucket rather than being ignored. Does not affect a review already in the APPROVED state or a plain clean approval with no blocking-looking prose — both are ignored regardless. Absent: such approvals are ignored for every bot."
     },
     "babysit_skip_downgrade_logins": {
       "type": "string",

plugins/source-control/CHANGELOG.md

@@ -3,6 +3,35 @@
 All notable changes to the `source-control` plugin are documented here. Format follows
 [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning.
 
+## [0.12.0]
+
+### Fixed
+
+- **`babysit-prs` snapshot no longer classifies an Approve-with-nits bot review as blocking bot
+  feedback.** A `claude[bot]` PR review posted as an issue-level comment with an explicit
+  **Approve** verdict and only 🟡-nit findings (no `CRITICAL`/`IMPORTANT` or other severity
+  marker) was surfaced as a blocking, genuinely-fresh finding because the body's prose contained
+  the word "blocking" ("blocking criteria", "blocking checks", "No blocking issues"), which the
+  text heuristic matched. The classifier now parses the verdict and severity markers: an explicit
+  approval carrying no genuine severity marker is downgraded structurally (for any bot, not only a
+  configured login) to a non-blocking result, consistent with `babysit-readiness-gate.sh`
+  reporting `findings=0` for the same review. Detection of genuinely blocking feedback is
+  unweakened — in a comment or a non-`APPROVED`-state review, a `CRITICAL`/`IMPORTANT` finding or
+  a Request-changes verdict still classifies as blocking, and `CRITICAL`/`IMPORTANT` are now
+  recognized as blocking-severity markers in their own right. (A review submitted in the formal
+  `APPROVED`/`DISMISSED` state is routed to `ignored` before the severity check — pre-existing
+  behavior this change does not alter; whether such reviews should be severity-scanned first is
+  tracked as a follow-up in #621.) A negated severity conclusion — a clean approval stating `No CRITICAL or IMPORTANT
+  findings` — is redacted before the severity check, the structured-marker analogue of the
+  existing `no P1/P2 issues` redaction, so introducing severity-marker detection does not itself
+  re-create a false blocker for that common clean-verdict phrasing. A login named in
+  `babysit_approval_downgrade_logins` opts that bot's approval into the more-conservative
+  `material` bucket (surfaced but non-blocking) instead of `ignored` in the one case the
+  structural downgrade reaches — a review body carrying blocking-looking prose that still parses
+  as an approval verdict. It does not affect a review already in the APPROVED state or a plain
+  clean approval whose body carries no blocking-looking prose: both are ignored regardless of the
+  setting, since neither reaches the downgrade branch.
+
 ## [0.11.0]
 
 ### Added

plugins/source-control/README.md

@@ -158,7 +158,7 @@ repo's owner.
 | `babysit_review_gate_context` | string | review gate treated as absent |
 | `babysit_ci_gateway_context` | string | gateway check unused |
 | `babysit_extra_bot_logins` | string (multiple) | structural bot detection only |
-| `babysit_approval_downgrade_logins` | string (multiple) | downgrade heuristic dormant |
+| `babysit_approval_downgrade_logins` | string (multiple) | an approval carrying blocking-looking prose is downgraded to ignored structurally (every bot); a named login instead surfaces its own as material. Real APPROVED-state reviews and plain clean approvals are ignored regardless. |
 | `babysit_skip_downgrade_logins` | string (multiple) | downgrade heuristic dormant |
 | `babysit_max_quiet_recheck_seconds` | number | 14400 |
 | `babysit_advisory_fix_round_cap` | number | 100 |

plugins/source-control/skills/babysit-prs/SKILL.md

@@ -293,7 +293,7 @@ tier authority.
 | `babysit_review_gate_context` | `${user_config.babysit_review_gate_context}` | `--review-gate-context` (snapshot) | gate treated as absent |
 | `babysit_ci_gateway_context` | `${user_config.babysit_ci_gateway_context}` | `--ci-gateway-context` (snapshot) | gateway check unused |
 | `babysit_extra_bot_logins` | `${user_config.babysit_extra_bot_logins}` | `--extra-bot-logins` (snapshot) | structural bot detection only |
-| `babysit_approval_downgrade_logins` | `${user_config.babysit_approval_downgrade_logins}` | `--approval-downgrade-logins` (snapshot) | downgrade heuristic dormant |
+| `babysit_approval_downgrade_logins` | `${user_config.babysit_approval_downgrade_logins}` | `--approval-downgrade-logins` (snapshot) | an approval carrying blocking-looking prose is downgraded to ignored structurally (every bot); a named login instead surfaces its own as material. Real APPROVED-state reviews and plain clean approvals are ignored regardless. |
 | `babysit_skip_downgrade_logins` | `${user_config.babysit_skip_downgrade_logins}` | `--skip-downgrade-logins` (snapshot) | downgrade heuristic dormant |
 | `babysit_max_quiet_recheck_seconds` | `${user_config.babysit_max_quiet_recheck_seconds}` | `--max-quiet-recheck-seconds` (snapshot) | `14400` |
 | `babysit_advisory_fix_round_cap` | `${user_config.babysit_advisory_fix_round_cap}` | `--fix-round-cap` (snapshot, ledger) | `100` |

plugins/source-control/skills/babysit-prs/scripts/babysit_feedback.py

@@ -23,6 +23,7 @@
 
 BLOCKING_TEXT_RE = re.compile(
     r"\b(p0|p1|p2|high[- ]severity|not approving|changes requested|"
+    + r"request(?:s|ing)? changes|"
     + r"required fix|must fix|blocking|regression|vulnerability)\b",
     re.I,
 )
@@ -35,6 +36,19 @@
     + r"(?:issues?|findings?|defects?|problems?|regressions?|vulnerabilities?)\b",
     re.I,
 )
+# Negated CRITICAL/IMPORTANT conclusions ("No CRITICAL or IMPORTANT findings",
+# "No CRITICAL issues found") are the structured-severity analogue of
+# NEGATED_SEVERITY_LIST_RE: a clean approval stating the absence of high-severity
+# findings, not a live one. The severity tokens stay case-sensitive (uppercase
+# only) for the same reason BLOCKING_SEVERITY_RE is -- lowercase "critical"/
+# "important" are ordinary prose -- while the negator and trailing noun are not.
+NEGATED_SEVERITY_MARKER_RE = re.compile(
+    r"(?i:\b(?:no|zero|without)\s+(?:actionable\s+)?)"
+    + r"(?:CRITICAL|IMPORTANT)"
+    + r"(?:(?i:\s*,?\s*(?:(?:and|or)\s+)?)(?:CRITICAL|IMPORTANT))*"
+    + r"(?i:\s+(?:issues?|findings?|defects?|problems?|regressions?|"
+    + r"vulnerabilities?))\b"
+)
 NEGATED_BLOCKING_TERM_RE = re.compile(
     r"\b(?:no|zero|without)\s+(?:actionable\s+)?(?:p[012]|high[- ]severity|"
     + r"blocking|regressions?|vulnerabilities?|required fixes?|changes requested)\b|"
@@ -61,6 +75,15 @@
     + r"request(?:s|ing)? changes|regression|vulnerability|high[- ]severity)\b",
     re.I,
 )
+# Blocking-severity markers, case-sensitive and whole-word, matching the
+# vocabulary babysit-readiness-gate.sh counts as findings (CRITICAL/IMPORTANT).
+# Deliberately NOT case-insensitive: lowercase "critical"/"important" occur
+# constantly in ordinary review prose ("it is important to note", "critical
+# path"), whereas the uppercase tokens are the reviewer's structured severity
+# labels. SUGGESTION is intentionally excluded here -- like a 🟡 nit it is a
+# non-blocking marker -- so an approval carrying only suggestions/nits stays
+# non-blocking, consistent with the issue's "CRITICAL/IMPORTANT vs nits" split.
+BLOCKING_SEVERITY_RE = re.compile(r"\b(?:CRITICAL|IMPORTANT)\b")
 REVIEW_SKIP_RE = re.compile(
     r"\bbugbot\b[^\n.]{0,80}?\b(?:skipped|did(?:n't| not) run|"
     + r"could(?:n't| not) run|was not run|unable to run|usage limit)\b"
@@ -85,9 +108,18 @@ class FeedbackConfig:
     """Caller-supplied identity configuration; every set ships empty.
 
     `extra_bot_logins` supplements structural bot detection for accounts whose
-    metadata misreports them as users. `approval_downgrade_logins` and
-    `skip_downgrade_logins` name the reviewer logins whose blocking-looking
-    text may be downgraded by the approval-verdict and review-skip heuristics.
+    metadata misreports them as users. A clean approval (explicit approval
+    verdict, no CRITICAL/IMPORTANT or required-fix marker) is treated as
+    non-blocking for every bot structurally. `approval_downgrade_logins` names
+    the reviewer logins whose approval is surfaced as a material finding rather
+    than ignored in the one case the structural downgrade reaches: a review body
+    carrying blocking-looking prose that still parses as an approval verdict. It
+    does not affect a review already in the APPROVED state or a plain clean
+    approval whose body carries no blocking-looking prose -- both are ignored
+    regardless, since neither reaches the downgrade branch.
+    `skip_downgrade_logins` names the reviewer logins
+    whose not-approving text may be downgraded to material when their review
+    provably could not run.
     """
 
     extra_bot_logins: frozenset[str] = field(default_factory=frozenset)
@@ -228,19 +260,41 @@ def has_blocking_text(text: str) -> bool:
     return bool(BLOCKING_TEXT_RE.search(redacted))
 
 
+def has_blocking_severity(text: str) -> bool:
+    """True when a CRITICAL/IMPORTANT severity marker survives negation redaction.
+
+    A companion to `has_blocking_text` for the structured severity vocabulary
+    the readiness gate counts as findings. A bot review that raises a genuine
+    high-severity finding is blocking even when its prose contains none of
+    `BLOCKING_TEXT_RE`'s imperative terms.
+    """
+    redacted = NEGATED_SEVERITY_LIST_RE.sub("", text)
+    redacted = NEGATED_SEVERITY_MARKER_RE.sub("", redacted)
+    redacted = NEGATED_BLOCKING_TERM_RE.sub("", redacted)
+    return bool(BLOCKING_SEVERITY_RE.search(redacted))
+
+
 def approval_downgrade(text: str) -> bool:
     """True when a reviewer bot states an explicit approval verdict.
 
     Requires a clear approval/non-blocking conclusion, no negated approval
-    language, and no required-fix language surviving negation redaction.
-    Anything ambiguous stays blocking.
+    language, and neither a required-fix term nor a CRITICAL/IMPORTANT severity
+    marker surviving negation redaction. Anything ambiguous -- and any genuine
+    high-severity finding carried in an approval-verdict body that reaches this
+    check -- stays blocking. A review submitted in the formal APPROVED/DISMISSED
+    state is routed to `ignored` upstream, before this predicate; whether that
+    short-circuit should be severity-scanned first is tracked as a follow-up in
+    issue #621.
     """
     if NON_APPROVAL_RE.search(text):
         return False
     if not APPROVAL_VERDICT_RE.search(text):
         return False
     redacted = NEGATED_SEVERITY_LIST_RE.sub("", text)
+    redacted = NEGATED_SEVERITY_MARKER_RE.sub("", redacted)
     redacted = NEGATED_BLOCKING_TERM_RE.sub("", redacted)
+    if BLOCKING_SEVERITY_RE.search(redacted):
+        return False
     return not REQUIRED_FIX_RE.search(redacted)
 
 
@@ -250,14 +304,15 @@ def skip_downgrade(text: str) -> bool:
 
     A not-approving comment whose stated reason is a skipped review run (for
     example a usage limit) and which carries no findings of its own is a
-    user-triage item, not a code blocker. Any residual blocking language keeps
-    it blocking.
+    user-triage item, not a code blocker. Any residual blocking language --
+    imperative blocking text or a surviving CRITICAL/IMPORTANT severity marker
+    -- keeps it blocking.
     """
     if not REVIEW_SKIP_RE.search(text):
         return False
     remainder = NOT_APPROVING_RE.sub("", text)
     remainder = REVIEW_SKIP_RE.sub("", remainder)
-    return not has_blocking_text(remainder)
+    return not has_blocking_text(remainder) and not has_blocking_severity(remainder)
 
 
 def collect_feedback(
@@ -326,7 +381,7 @@ def collect_feedback(
             blocking.append(record)
         elif state in {"APPROVED", "DISMISSED"}:
             ignored.append(record)
-        elif has_blocking_text(text):
+        elif has_blocking_text(text) or has_blocking_severity(text):
             disposition = json_object((dispositions or {}).get(fid))
             current_head = str(pr.get("headRefOid") or "")
             # Only honor a disposition recorded at the current head. If the PR has
@@ -340,11 +395,24 @@ def collect_feedback(
             if disposition_applies:
                 record["disposed_reason"] = str(disposition.get("reason") or "")
                 material.append(record)
-            elif normalized_bot_login(
-                item
-            ) in approval_downgrade_logins and approval_downgrade(text):
+            elif approval_downgrade(text):
+                # An explicit approval verdict with no genuine severity marker
+                # (no CRITICAL/IMPORTANT, no required-fix term surviving negation
+                # redaction): the blocking-looking text is descriptive prose --
+                # e.g. "blocking criteria", "blocking checks", "no blocking
+                # issues" -- not a live finding. This is structural, not
+                # login-gated: a clean approval reads the same from any bot, and
+                # this keeps the snapshot consistent with
+                # babysit-readiness-gate.sh reporting findings=0 for the very
+                # same review. A login named in `approval_downgrade_logins` opts
+                # that bot's clean approvals into the more-conservative `material`
+                # bucket (surfaced but non-blocking) instead of being fully
+                # ignored; the safe default for every other bot is `ignored`.
                 record["downgrade"] = "approval_verdict"
-                material.append(record)
+                if normalized_bot_login(item) in approval_downgrade_logins:
+                    material.append(record)
+                else:
+                    ignored.append(record)
             elif normalized_bot_login(
                 item
             ) in skip_downgrade_logins and skip_downgrade(text):

plugins/source-control/skills/babysit-prs/scripts/pr_queue_snapshot.py

@@ -518,8 +518,15 @@ def main() -> int:
         "--approval-downgrade-logins",
         default=None,
         help=(
-            "Comma-separated reviewer-bot logins whose blocking-looking text "
-            "may downgrade on an explicit approval verdict (ships empty)."
+            "Comma-separated reviewer-bot logins whose approval is surfaced as a "
+            "material finding rather than ignored in the one case the structural "
+            "downgrade reaches: a review body carrying blocking-looking prose that "
+            "still parses as an approval verdict (no CRITICAL/IMPORTANT or "
+            "required-fix marker). Every bot's such approval is downgraded to "
+            "non-blocking by default; naming a login opts its own into the "
+            "more-conservative material bucket instead of ignored. Does not affect "
+            "a review already in the APPROVED state or a plain clean approval with "
+            "no blocking-looking prose -- both are ignored regardless (ships empty)."
         ),
     )
     parser.add_argument(