source-control:babysit-prs: snapshot classifies an Approve-with-nits claude[bot] review as blocking bot feedback (gate correctly returns findings=0)

[kyle-sexton]

Lane: babysit (worker tier)

Triggering example: PR #492, cycle start snapshot.

The dispatch snapshot reported: 1 blocking bot feedback item — a claude[bot] PR Review just posted (run 29694104425) and characterized it as a genuinely fresh finding, not noise.

Fetching ground truth, that claude[bot] review was an Approve verdict whose only findings were two 🟡 nits, each explicitly self-deprioritized by the reviewer ("Not worth a change on its own, but worth noting" / "low impact"). Correspondingly:

• babysit-readiness-gate.sh 492 returned READINESS_OK findings=0 classified=0
• gh pr view showed mergeStateStatus: CLEAN, mergeable: MERGEABLE, all 15 checks SUCCESS
• Zero review threads existed (the comment is issue-level type: general, reviews nodes empty)

Observed: The snapshot's readiness classifier appears to treat the mere presence of a fresh claude[bot] PR Review run as "blocking bot feedback" without parsing the verdict/severity — a fresh Approve with only non-blocking nits was surfaced as a blocking, genuinely-fresh finding.

Expected: An Approve verdict carrying only non-blocking nits (no CRITICAL/IMPORTANT/severity markers) should classify as non-blocking / readiness-OK, consistent with the readiness gate's findings=0. The snapshot classifier and the gate disagreed on the same input.

Category: behavioral

Note: Distinct from the known gate bugs #484 (resolve-thread wrapper PATH) and #465, and from #475 (gate env-var gap) / #497 (single---pr snapshot self_logins empty) / #473 (self-reply false positive). This is a snapshot-side verdict/severity-parsing gap, not a gate over-count.

[kyle-sexton]

This was generated by AI during triage. · Category: behavioral (bug) · Priority: priority: high (source-control north-star; false-blocking classification wastes babysit cycles and can stall a mergeable PR). Routed status: ready.

Plan

The dispatch snapshot's readiness classifier treats the presence of a fresh claude[bot] PR Review run as "blocking bot feedback" without parsing the review's verdict + severity. Ground truth on PR #492: an Approve verdict whose only findings were two 🟡 nits, each self-deprioritized — yet surfaced as "1 blocking bot feedback item … a genuinely fresh finding, not noise." The readiness gate disagreed on the same input (READINESS_OK findings=0 classified=0; mergeStateStatus: CLEAN; zero review threads).

Fix (behavioral contract): the snapshot classifier parses the bot review's verdict and per-finding severity, matching the readiness gate's logic:

• APPROVE verdict with only non-blocking findings (🟡 nits / no CRITICAL/IMPORTANT/severity markers) → non-blocking / readiness-OK.
• Only CHANGES_REQUESTED, or findings carrying blocking-severity markers, → blocking.
• The snapshot and babysit-readiness-gate.sh must not disagree on the same input — align the snapshot classifier to the gate's classification (ideally sharing the classification logic so they cannot drift).

Acceptance criteria

• A fresh claude[bot] Approve review whose findings are all non-blocking nits classifies as non-blocking in the snapshot, agreeing with the gate's findings=0.
• A CHANGES_REQUESTED or blocking-severity finding still classifies as blocking.
• Snapshot classifier and readiness gate return consistent blocking/non-blocking verdicts on identical input (shared logic preferred over parallel re-implementation).

Distinct from #465/#475/#484/#497/#473 (gate over-count / env / wrapper-PATH / self_logins / self-reply) — this is a snapshot-side verdict-parsing gap.

Decision defaulted: align the snapshot classifier to the readiness gate's verdict+severity parsing (Approve-with-only-non-blocking-nits = non-blocking), sharing the classification logic rather than re-implementing it — veto before merge

[kyle-sexton]

Parking a config-semantics decision the worker made unilaterally while fixing this issue (advisor was rate-limited for them; per this lane's rule 7, a subagent's own decision question parks the same as the orchestrator's — flagging it now rather than letting it ship silently).

PR #567 fixes the root bug (verdict/severity-aware classification: clean approvals with no CRITICAL/IMPORTANT marker downgrade to non-blocking for any bot). Fixing that required deciding what to do with the existing babysit_approval_downgrade_logins user config, which previously had no real effect (every bot's approval was already treated as blocking regardless). The worker's choice: repurpose it so named logins route their clean approvals to a material bucket (surfaced but non-blocking), while every other bot defaults to ignored.

Question: keep that repurposed meaning for babysit_approval_downgrade_logins, or remove the config entirely now that the underlying bug it was working around is fixed by default?

RECOMMENDED: keep the repurposed meaning (the worker's choice). It preserves a documented user-facing config instead of a breaking removal, gives operators an opt-in "still show me this bot's clean approvals" knob, and the new default (ignored) already fixes the reported bug for everyone who never touched the config.

Options:

1. (RECOMMENDED) Keep babysit_approval_downgrade_logins with its new "route clean approvals to material" meaning, as shipped in PR fix(source-control): babysit snapshot classifies Approve-with-nits bot review as non-blocking (#499) #567.
2. Remove the config entirely (CLI flag + FeedbackConfig + userConfig schema) since the bug it addressed no longer exists by default — larger diff, breaking change for anyone who set it.
3. Keep the config's literal old meaning (fully suppress the bot's approvals from any lane visibility) as a separate opt-in flag, additive to the new default, at the cost of two config knobs doing adjacent things.

PR #567 is in DRAFT pending this decision; the code as shipped implements option 1.