fix(source-control): babysit snapshot classifies Approve-with-nits bot review as non-blocking (#499)#567
Conversation
…cking
The babysit-prs snapshot classified a claude[bot] Approve verdict whose only
findings were self-deprioritized nits as a blocking bot-feedback item, because
the review body's prose contains the word "blocking" ("blocking criteria",
"blocking checks", "No blocking issues") which the text heuristic matched. The
snapshot disagreed with babysit-readiness-gate.sh, which reported findings=0 for
the same review.
Parse the verdict and severity markers: an explicit approval carrying no
CRITICAL/IMPORTANT or required-fix marker is downgraded structurally (for any
bot) to a non-blocking result, consistent with the gate. CRITICAL/IMPORTANT are
now recognized as blocking-severity markers and a Request-changes verdict is
detected, so genuinely blocking feedback still classifies as blocking. A login
named in approval_downgrade_logins opts its clean approvals into the material
bucket instead of ignored.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011V31qpAHP3jfs76B9d5Rfo
|
Claude finished @kyle-sexton's task in 2m 31s —— View job PR Review — fix(source-control): Approve-with-nits classification
The fix is well-reasoned and the implementation is clean. The behavioral change (structural approval downgrade for all bots, not just a configured allowlist) is the right direction and doesn't weaken blocker detection — it tightens it. A few notes: Core logic (
|
The comment-hygiene check flagged an owner/repo#N issue reference (melodic-software/claude-code-plugins-#492) in the test-fixture comment, which the org policy bans in code comments. The workflow run id already pins the fixture's provenance, so the issue reference was redundant coupling; reword to reference the run alone and reflow the comment. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
PR Review — fix(source-control): Approve-with-nits classification
The fix is correct and well-scoped. The behavioral change (structural approval downgrade for all bots) makes the snapshot agree with the gate on the exact input from the issue, and the Core logic (
|
| Scenario | Test | Asserts |
|---|---|---|
Approve + descriptive "blocking" prose → ignored |
test_bot_approval_with_descriptive_blocking_prose_is_ignored |
blocking=[], material=[], ignored[0]["downgrade"]="approval_verdict" |
Configured login → material |
test_configured_login_surfaces_clean_approval_as_material |
material[0]["downgrade"]="approval_verdict" |
| Approval + CRITICAL → stays blocking | test_approval_verdict_with_critical_marker_stays_blocking |
blocking has 1 item |
| "Request changes" verdict → stays blocking | test_request_changes_verdict_is_blocking |
blocking has 1 item |
has_blocking_severity case sensitivity |
test_has_blocking_severity_matches_only_uppercase_markers |
uppercase hits, lowercase doesn't, SUGGESTION excluded |
The ApproveWithNitsClassification class in test_pr_queue_snapshot.py drives the fix end-to-end through delta.classify_pr with APPROVE_WITH_NITS_BODY — a faithful abbreviation of the real #492 review preserving all four "blocking" occurrences that caused the misfiring. Asserting both needs_worker=False and pr_clean_ready_for_direct_gate=True directly encodes the gate/snapshot consistency invariant.
Minor gap: test_approve_with_only_nits_is_not_blocking asserts material=[] and blocking=[] but doesn't assert that the item lands in feedback["ignored"]. The unit test in test_babysit_feedback.py covers the ignored bucket, so it's not a correctness hole — but adding self.assertEqual(len(result["feedback"]["ignored"]), 1) to the integration test would tighten the end-to-end invariant. Fix this →
CLI help / docs / version
--approval-downgrade-loginshelp at line 512 accurately reflects the opt-in-to-material semantics.FeedbackConfigdocstring,README.md,SKILL.mdconfig table all updated consistently.- CHANGELOG entry is complete and accurate.
- Version bump
0.9.3 → 0.9.4is correct for a patch bug fix.
Summary: No blocking issues. The logic is correct, test coverage is strong, and docs are consistent. The one minor test gap (asserting the ignored bucket in the integration test) is worth a follow-up but not a blocker. Ready to merge.
Post-green review pass (work lane)
Bot review concluded "no blocking issues... ready to merge." Moving this out of draft. The pre-existing 🤖 Generated with Claude Code |
|
Claude finished @kyle-sexton's task in 2m 35s —— View job PR Review — fix(source-control): Approve-with-nits classification
Two prior bot reviews have already covered this in detail and both concluded "no blocking issues — ready to merge." I've read all eight changed files and the diff independently. My findings agree. Core logic (
|
| Scenario | File | Asserts |
|---|---|---|
Approve + descriptive "blocking" prose → ignored |
test_babysit_feedback.py |
blocking=[], material=[], ignored[0]["downgrade"]="approval_verdict" |
Configured login → material |
test_babysit_feedback.py |
material[0]["downgrade"]="approval_verdict" |
| Approval + CRITICAL → stays blocking | test_babysit_feedback.py |
len(blocking)=1 |
| "Request changes" verdict → stays blocking | test_babysit_feedback.py |
len(blocking)=1 |
has_blocking_severity case-sensitivity |
test_babysit_feedback.py |
uppercase hits, lowercase/SUGGESTION excluded |
End-to-end through classify_pr |
test_pr_queue_snapshot.py |
needs_worker=False, pr_clean_ready_for_direct_gate=True |
Minor gap (filed as #578): test_approve_with_only_nits_is_not_blocking asserts blocking=[] and material=[] but doesn't assert that the item lands in feedback["ignored"]. The unit test in test_babysit_feedback.py covers the ignored bucket directly, so there's no correctness hole — but the integration test could be tightened. Not a blocker. Fix this →
Docs / version
- CHANGELOG entry accurate and complete
FeedbackConfigdocstring, README.md, SKILL.md config table all consistent with the new structural-downgrade semantics- CLI help at line 512 reflects the opt-in-to-material semantics correctly
- Version bump
0.9.3 → 0.9.4correct for a patch bug fix
No blocking issues. The fix is correct, the logic is sound, test coverage is strong, and the docs are consistent. The integration-test gap is a nit filed as #578, not a blocker. Ready to merge.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 44ac67b66d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…y blocking The new structured-severity blocking path introduced in this PR (BLOCKING_SEVERITY_RE / has_blocking_severity) re-created a false blocker for a common clean-approval phrasing: a bot review concluding "No CRITICAL or IMPORTANT findings" survived redaction and classified as blocking, and approval_downgrade's guard then rejected the approval. This is the structured- marker analogue of the already-handled "no P1/P2 issues" case. Add NEGATED_SEVERITY_MARKER_RE (list-aware, severity tokens case-sensitive to match BLOCKING_SEVERITY_RE; negator and noun case-insensitive) and apply it in the redaction pipeline of both has_blocking_severity and approval_downgrade. A live CRITICAL/IMPORTANT marker not preceded by no/zero/without still blocks. Addresses chatgpt-codex-connector P2 "Redact negated severity markers before blocking" (#567 review). Full unit + integration suite green (227 tests). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 3m 32s —— View job PR Review — fix(source-control): Approve-with-nits classification (post-Codex pass)
This is a focused re-review covering the two Codex P2 findings and the latest commit ( Codex P2 #1 — "Redact negated severity markers before blocking" ✅ RESOLVED
CHANGELOG entry is accurate. This finding is closed. Codex P2 #2 — "Preserve approved P2 feedback as material"
|
Triage pass — reconciling the bot reviewsThe three
Correcting my earlier "Post-green review pass" note: the negated- |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c86fc59495
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Follow-up tracking for the two design-decision findings left open on this PR's live re-review pass (worker: babysit lane, 2026-07-20):
Both are author-owned policy/contract calls, not mechanical fixes — tracked issues filed, threads left open pending that decision. |
|
Config decision logged on #499: KEEP Remaining to unblock merge:
|
…arrow approval-downgrade docs; bump 0.12.0 Composes main's source-control 0.11.0 (#606 attribution_drift arm) with this PR's Approve-with-nits classification fix — both sides preserved. Conflict resolutions: - plugin.json: version 0.9.4/0.11.0 -> 0.12.0 (minor from current main; this PR changes a userConfig key's semantics, and repo convention treats behavior/contract changes as minor). userConfig kept both #606's babysit_intended_write_identity and this PR's approval-downgrade keys. - CHANGELOG.md: this PR's entry re-headed [0.9.4] -> [0.12.0], placed atop #606's [0.11.0]. Thread 2 (docs overstate the knob, #499 decision): narrowed all six doc surfaces for babysit_approval_downgrade_logins to the code's actual reach (README cell, SKILL cell, FeedbackConfig docstring, --approval-downgrade-logins help, CHANGELOG entry, plugin.json userConfig description). The knob only elevates to material an approval whose body carries blocking-looking prose that parses as an approval verdict — the one case the structural downgrade routes; a real APPROVED-state review or a plain clean approval is ignored regardless. No change to collect_feedback logic. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Eu6ZSEy8ptQjCXbhbjBKpo
|
Claude finished @kyle-sexton's task in 3m 6s —— View job PR Review — fix(source-control): Approve-with-nits classification (post-merge pass)
This pass covers the merge commit Attribution-drift arm
Docs narrowing —
|
| Surface | Status |
|---|---|
plugin.json userConfig description |
✅ Updated — correctly describes the structural downgrade, opt-in-to-material semantics, and both cases not affected (APPROVED-state reviews, plain clean approvals) |
README.md config table default cell |
✅ Updated — accurate one-liner |
SKILL.md config table default cell |
✅ Updated — matches README |
--approval-downgrade-logins CLI help |
✅ Updated — full accurate description |
FeedbackConfig docstring |
✅ Updated — correctly scopes the setting to the one case it reaches |
CHANGELOG.md |
✅ Updated — accurate description of the full fix including negation redaction |
Accuracy check against the code: Three distinct cases for a bot review with no state == "APPROVED":
- Plain clean approval (
has_blocking_textandhas_blocking_severityboth False): falls toelse: ignoredat line 421 — theelif approval_downgrade(...)branch is never reached. Docs now accurately state "plain clean approval with no blocking-looking prose — both are ignored regardless." - APPROVED-state review: short-circuits at line 377 before any login or text check. Docs now accurately state "Real APPROVED-state reviews… are ignored regardless."
- Blocking-looking prose + approval verdict: enters the structural downgrade at line 393 — the one case the configured-login opt-in reaches.
The docs are accurate against the code. The approve_with_only_nits_is_not_blocking integration test not asserting result["feedback"]["ignored"] is also confirmed correct: classify_pr returns "feedback" with only blocking/material/human_blocking/human — babysit_delta.py:1134–1139 shows ignored is deliberately omitted from the integration return. Prior reviews' #578 suggestion was not actionable; kyle-sexton's triage was correct.
NEGATED_SEVERITY_MARKER_RE
The regex introduced in c86fc59 is sound:
- Covers
"No CRITICAL or IMPORTANT findings","No CRITICAL issues found","No CRITICAL, IMPORTANT issues"(comma separator),"Without CRITICAL findings","Zero actionable IMPORTANT issues"— the cases likely to appear in clean-verdict prose. - Severity tokens stay case-sensitive to match
BLOCKING_SEVERITY_RE's rationale; negator and trailing noun are case-insensitive. - Applied in both
has_blocking_severity(line 267) andapproval_downgrade(line 285) — consistent pipeline. - A live
CRITICALwithout a negator still fires (test_approval_verdict_with_critical_marker_stays_blocking✓).
One narrow gap: "No CRITICAL" with no trailing noun (e.g. "No CRITICAL.") is NOT redacted by the regex (trailing noun is required). Such a review body would still have BLOCKING_SEVERITY_RE match CRITICAL, causing a false positive. This is extremely unlikely in practice — any well-formed clean verdict would say "No CRITICAL issues" — and the conservative direction (forces human review, not a false negative) is acceptable.
Version bump
0.9.3 → 0.12.0 (via 0.9.4 on the PR branch, composed with 0.11.0 already on main): the final merged version is 0.12.0, correctly reflecting a minor bump for a behavior/contract change (semantics of approval_downgrade_logins inverted; structural downgrade introduced for all bots).
Open deferred threads
Both Codex P2 threads in review comments have been replied to with author-owned policy decisions and corresponding follow-up issues:
babysit_feedback.py:410(P2 advisory under approval →ignored): Deferred → source-control:babysit-prs: advisory-P2-under-approval routing has no ignored-vs-material policy (babysit_feedback.py) #594babysit_feedback.py:379(configured-login plain clean approval → never reaches material): Deferred → source-control:babysit-prs: configured-login clean-approval material opt-in is unreachable (contradicts PR #567's own doc contract) #595; docs now accurately document this limitation rather than overclaiming the opt-in's reach
Threads remain open pending the author's design decisions, which is correct — the babysit gate counts any unresolved thread as a blocker, so the author will need to resolve or further defer these before the merge gate allows merge.
No blocking issues. The docs narrowing is accurate, the version bump is correct, the NEGATED_SEVERITY_MARKER_RE fix is sound, and the open threads are properly tracked. Ready to merge once the two deferred Codex threads are formally resolved (either closed-to-issue or acknowledged-and-accepted by the author).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07961c568f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ty markers A bot review in `skip_downgrade_logins` whose skip comment also carries a genuine CRITICAL/IMPORTANT marker (but no imperative blocking text) was downgraded to `material` as `review_skip` instead of staying `blocking`, because `skip_downgrade()` rechecked only `has_blocking_text` and the older text regex omits the structured severity markers this PR introduced. Add the sibling `not has_blocking_severity(remainder)` guard already used on the `approval_downgrade` path, so a live high-severity finding under a skipped review keeps blocking. Regression test covers the CRITICAL and IMPORTANT cases and confirms a negated severity conclusion still downgrades. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Eu6ZSEy8ptQjCXbhbjBKpo
|
Claude finished @kyle-sexton's task in 2m 17s —— View job PR Review — fix(source-control): Approve-with-nits classification (post-sync pass)
This pass covers the latest commit Codex P2 (line 380) — skip-downgrade severity guard ✅ RESOLVEDThe final commit correctly addresses the Codex finding. The Fix at # before
return not has_blocking_text(remainder)
# after
return not has_blocking_text(remainder) and not has_blocking_severity(remainder)Correctness trace for
The negated case is also handled correctly: The fix is minimal, mirrors the pattern already used in TestsThree regression tests in
Coverage is complete for the finding. Minor documentation gap (nit, not blocking)The Status of all prior findings
No blocking issues. The Codex P2 skip-downgrade finding is cleanly resolved. The fix is a one-line change that correctly mirrors the |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69a35602e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ehavior; defer APPROVED-state scan (#621) The CHANGELOG and approval_downgrade docstring stated an unqualified guarantee that CRITICAL/IMPORTANT findings stay blocking. A formal APPROVED/DISMISSED-state review short-circuits to `ignored` before the severity check (pre-existing behavior, unchanged by this PR), so the guarantee overstated the code for that case. Narrow both to the enforced scope — a comment or a non-APPROVED-state review — and cite the follow-up design issue (#621) that owns whether formal APPROVED-state reviews should be severity-scanned (bot and human paths). No classifier logic change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Eu6ZSEy8ptQjCXbhbjBKpo
|
Claude finished @kyle-sexton's task in 1m 41s —— View job PR Review — fix(source-control): Approve-with-nits classification (post-sync pass)
This pass covers the latest commit Latest commit (
|
| Finding | Source | Status |
|---|---|---|
Core fix: Approve-with-nits → ignored |
PR | ✅ Fixed |
| Negated CRITICAL/IMPORTANT → false blocker | Codex P2 (line 274) | ✅ Fixed in c86fc59 |
| Skip-downgrade severity guard | Codex P2 (line 384) | ✅ Fixed in 69a3560 |
| APPROVED-state review bypasses severity check | Codex P1 (line 384) | ✅ Docs narrowed; deferred → #621 |
Advisory P2 under approval → ignored |
Codex P2 (line 415) | 🔵 Deferred → #594 |
Configured-login plain clean approval never reaches material |
Codex P2 (line 384) | 🔵 Docs narrowed; deferred → #595 |
Integration test ignored bucket assertion (#578) |
Bot reviews | 🔵 Not actionable at integration layer |
No new issues introduced
The CHANGELOG line A negated severity conclusion — a clean approval stating 'No CRITICAL or IMPORTANT is long but is prose in a CHANGELOG entry, not code — not actionable.
No blocking issues. The latest commit correctly scopes the documented guarantee to match the enforced behavior, and the APPROVED-state scan decision is properly deferred to #621. All correctness issues raised across six review rounds have been fixed or formally tracked with follow-up issues. Ready to merge.
…val classifier + golden fixtures (#534) (#634) ## Summary Authorship (self/bot/human), finding (severity + lifetime-vs-open state), and approval-verdict detection were hand-rolled independently across the babysit **snapshot** (`babysit_delta`/`babysit_feedback`), the **merge gate** (`babysit_merge`), the **resolve-thread reporter** (`babysit_resolve_thread`), and the **readiness gate** (`babysit-readiness-gate.sh`). The surfaces disagreed on identical input — the six-issue misclassification class this umbrella closes (the smoking gun: #499, where the gate returned `findings=0` while the snapshot called the same review "1 blocking bot finding"). This extracts **one shared classifier** and migrates every surface to it, so the surfaces cannot diverge, and folds each member issue in as a golden fixture — regression-proof by construction. Implements the operator-**ACCEPTED Option A** from the decision brief verbatim. ## Fix - **New leaf module `babysit_classify.py`** (depends only on `babysit_util`) owns the three concern areas: authorship (`is_bot`, `actor_kind`, self-login normalization/membership, dependency-author test), finding (blocking/severity heuristics + the readiness gate's finding/classification **counting**), and approval-verdict (approval/skip downgrades). The primitives moved out of `babysit_feedback`, which re-exports them so existing consumers are unchanged. - **Migrations (import the shared classifier directly):** `babysit_delta` and `babysit_merge` replace hand-rolled `self_logins` casefold-compare (delta ×3 + merge) with the shared `normalize_self_logins`/`is_self_login`; `babysit_feedback` orchestrates the shared primitives; `babysit_resolve_thread` shares the same `is_bot`. **`babysit_merge` remains the merge-ready boolean owner** — it just stops carrying its own private authorship copy. - **Gate shells out (ratified, non-defaultable call):** `babysit-readiness-gate.sh` shells out to `babysit_findings.py` for finding-counting (mirroring the existing `source-control-babysit-merge` bash→Python wrapper) instead of re-implementing the severity vocabulary in bash grep. The **bash counting is retained only as the Python-free safe-tier degrade** (`reference/loop.md` is that path and it runs this gate; hard-requiring Python would regress that documented tier). A convergence test pins the two counts together on thread-state-free input. - **#465 (lifetime P-badges over-counted → false `READINESS_BLOCKED`):** the shared counter discounts a severity marker carried in a review thread GitHub reports **resolved or outdated**, counting currently-open findings only. **Scope:** this is the mechanical resolved/outdated discount the brief specs ("severity occurrence + lifetime-vs-open **state**"). De-duplicating the *same concern restated across re-review rounds within still-open threads* is deliberately **out of scope** — there is no reliable mechanical "same concern" signal — so restatements still count. - **#512 (`humanThreadsActed` reported for a Bot-authored thread):** count only threads whose **opening author** is human (via the shared `is_bot`), not `botOnly` (which mislabeled a bot-opened thread carrying a later human reply). - **orchestration doc:** there was no explicit "cross-check live thread state each cycle" workaround to delete — `orchestration.md` already mandates trusting the deterministic engine (the brief notes the re-derivation was "real but implicit"). Strengthened that trust instruction to name the classification as now one shared, fixture-locked classifier, so "don't re-derive by eye" has teeth. - Every surface stays a **pure predicate with no writes** (#440 `report-only` default preserved). ## Verification All commands run in the worktree with Python 3.14.6, ruff 0.15.20, jq 1.8.2. **Engine suite (unittest + ruff + guarded wrappers) — 256 tests (was 236; +17 classifier + 3 resolve-thread golden fixtures):** ``` == unittest suite == Ran 256 tests in 1.500s OK == ruff == All checks passed! == guarded-wrapper behavior == PASS: merge wrapper rejects --allow-unpinned-head PASS: merge wrapper reaches fail-closed CLI (no allowlist) PASS: resolve wrapper reaches fail-closed CLI (no allowlist) ``` **Readiness-gate bash suite — 53 cases, exit 0.** The 47 pre-existing fixtures still pass (now through the Python counter). New: the #465 lifetime discount end-to-end, plus an explicit **dual-path convergence** test — `BABYSIT_READINESS_BASH_ONLY=1` forces the degrade so both counts are observed in one run and asserted equal, so the severity vocabulary cannot drift between Python and bash silently: ``` PASS: [48] #465 lifetime discount -> findings=1 (only open) PASS: [49] #465 lifetime discount -> READINESS_OK PASS: [50] convergence [severity-words]: python == bash degrade (findings=3 classified=1) PASS: [51] convergence [codex-badges]: python == bash degrade (findings=2 classified=1) PASS: [52] convergence [plain-p-markers]: python == bash degrade (findings=2 classified=1) PASS: [53] convergence [self-row-exclusion]: python == bash degrade (findings=1 classified=1) ``` **#465 end-to-end (3 lifetime severity markers — 2 resolved/outdated, 1 open — fully classified):** ``` $ python babysit_findings.py --comments-json lifetime.json --self me[bot] findings=1 classified=1 $ bash babysit-readiness-gate.sh 377 --comments-json lifetime.json --self me[bot] READINESS_OK findings=1 classified=1 checklist=n/a (exit 0) ``` The bash degrade counts all 3 lifetime markers (old behavior, preserved for Python-free); the Python path discounts the 2 resolved/outdated → the false `READINESS_BLOCKED` is gone. **#512 regression (`tests/test_babysit_resolve_thread.py`):** bot-opened thread with a human reply, acted under `--include-human` → `humanThreadsActed: 0`; human-opened → `1`; mixed → `1`. Passing. **Lint:** `shellcheck --rcfile .shellcheckrc` clean on both gate scripts; `shfmt -d` (editorconfig-driven) clean; plugin manifest valid JSON at `0.13.0`. Closes #534 ## Related Member issues folded as golden fixtures / regression proofs: **#512** (bot thread counted as human — fixed here), **#499** (Approve-with-nits, already fixed by #567 — regression fixture), **#465** (lifetime over-count — fixed here), **#497** (single-`--pr` `self_logins` empty, already fixed on main — covered by `SinglePrScopeSelfLoginTests`), **#473** (CLOSED, self-reply — fixture-only). **#455** is a *different* classifier (auto-mode retry semantics, not authorship/finding/approval) and is referenced only, per the brief. - #637 — post-green review finding (`babysit_resolve_thread.py` two `is_bot` call sites omit `extra_bot_logins`), pre-existing gap surfaced but not introduced by this PR, deferred out of scope Deferred out of this PR's scope (arrived as research/triage notes, not operator-ACCEPTED like Option A; none trivially covered by the classifier built here): - **#618** advisory-thread fast-path — tag Codex `COMMENTED` threads as ADVISORY for a fast-path resolve ([#534 comment 5019177827](#534 (comment))). A natural follow-up: it would slot into this same classifier. - **#594 / #595** — P2-under-Approve material-vs-ignored routing and configured-login clean-approval opt-in reachability (`babysit_feedback` classification gaps found on PR #567). Same classifier home, non-trivial policy decisions of their own. - **#465 (b)** dedup-by-concern across re-review rounds — no reliable mechanical signal (see Fix). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01KyLj6oaFVTE6xFuoYCC2KC --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Summary
Babysit snapshot misclassified an Approve-with-nits bot review: a clean approval delivered as an issue comment (the #499 claude[bot] shape) was treated as blocking feedback, stalling merge-ready classification. This PR makes the approval downgrade structural and narrows every doc surface to the code's actual behavior. Composed atop the #606 attribution-drift arm (source-control 0.11.0) as 0.12.0.
Fix
ignoredfor ANY bot — no login configuration required.babysit_approval_downgrade_loginsKEPT with repurposed semantics (decision logged on source-control:babysit-prs: snapshot classifies an Approve-with-nits claude[bot] review as blocking bot feedback (gate correctly returns findings=0) #499): a configured login's approval-carrying-blocking-prose routes tomaterialinstead ofignored. Reach documented narrowly: fires only for an approval whose body carries blocking-looking prose; never for a formal APPROVED-state review (pre-existing short-circuit, see source-control:babysit-prs: should formal APPROVED-state reviews be severity-scanned before the ignored short-circuit (bot AND human paths) #621) or a plain clean approval.BLOCKING_SEVERITY_RE(case-sensitive CRITICAL|IMPORTANT) +has_blocking_severityon bothapproval_downgradeANDskip_downgrade(Codex P2 — a configured skip comment carryingCRITICAL:stays blocking); negated-marker handling; "request(s|ing) changes" added toBLOCKING_TEXT_RE.Verification
babysit_delta.py+ delta tests byte-identical to main's feat(source-control): surface babysit bot-to-personal identity fallback as an attribution-drift finding (#450) #606 arm; this PR stacks cleanly on 0.11.0.Closes #499
Related