chore(review-loop): send fewer PRs to a human - #1734
Merged
Conversation
Every one of the first eleven Review Loop runs against this repository ended with needs-human-review. Two things produced that. The path list sent a dependency bump, a doc edit, an ADR and anything under scripts/, config/ or deployed/ to a maintainer regardless of what the reviewers found. And the reviewers set needsHuman on low and info findings (line wrapping, a condition written out four times, a missing cross-reference), which the host honours at any severity: it downgrades a low or info *defect* to nonblocking, but a low or info finding marked needsHuman or filed as a product_decision still ends the run. The path list is now the places where a wrong automatic approval is hard to undo or would weaken a gate: the policy and its instructions, the workflows, migrations, licensing, the release and deployment definitions, the SDK version files whose merge publishes, the gate thresholds and scripts, and the ADRs. The four reviewer files name the one case that always warrants needsHuman, a change that contradicts a decision an Accepted ADR records, reserve the rest for medium-or-higher findings a maintainer must choose and cannot easily reverse, and say a low or info finding is never needsHuman. Intent the approved base already records is settled when the PR follows it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015YbHtxQBZQ6pDHFsqumcuR
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Every one of the first eleven Review Loop runs against this repository ended with
needs-human-review(#1661, #1666, #1675, #1681, #1682, #1683, #1718, #1719, #1720, #1722, #1724, #1725, #1730). Reading the summaries, two things produced that:human_review_pathsnamedscripts/**,config/**,deployed/**,decisions/**, everymix.exs/mix.lock/package.json/go.mod, CLAUDE.md, CONTRIBUTING.md and more. A dependency bump (deps: upgrade managoat_acp to 0.2.3 #1725), a doc (docs: add Review Loop skill for PR contributors #1719) or a deployed-suite tweak (Wait for LiveView readiness in deployed browser checks #1661) went to a human before any reviewer had a say.needsHuman. The host downgrades a low or info defect to nonblocking, but a low or info finding withneedsHuman: trueorkind: product_decisionends the run at any severity (server/engine.tstriage). Reviewers used it for line wrapping past 80 columns (fix(review-loop): cover all core precommit stages #1720), a condition written out four times (feat(api): bulk apply reconciles Teammate, Schedule and Webhook documents #1675), a missing NOTICE cross-reference (docs: add Review Loop skill for PR contributors #1719) and similar.What changes
.github/review-loop.yml:human_review_pathsis now the places where a wrong automatic approval is hard to undo or would weaken a gate: this policy and.github/review/**, the workflows, migrations,LICENSE*/NOTICE,Dockerfile/rel/**/deploy/**, the three SDK version files whose merge publishes a package, the gate thresholds (coverage.exs,.credo.exs,.sobelow-conf) and gate scripts, anddecisions/**. Dropped:scripts/**(other than the gate scripts),config/**,deployed/**,images/**, the dependency manifests, the contract manifests and omissions, CLAUDE.md, CONTRIBUTING.md, AGENTS.md, and the compose/fly/render files (several of which do not exist in the tree).The four reviewer files: the
needsHumanguidance is rewritten with one rule in common. The one case that always warrants it is a change that contradicts a decision an Accepted ADR records, cited by ADR and sentence, and a PR that amends the ADR to fit counts the same. Beyond that it is reserved for a medium-or-higher finding whose remedy a maintainer must choose and cannot easily reverse (public contract, migration or retention, licensing, authorization model, a repair outside the fix policy). A low or info finding is neverneedsHumanand never aproduct_decision. Intent the approved base already records (an ADR, CONTRIBUTING.md, the CHANGELOG, the linked issue) is settled when the PR follows it..github/review/README.md: describes the two producers of a human decision and the narrowed rule.Notes
.github/**, so it goes to a human by its own policy; the base policy governs its run, as designed.mix precommitdoes not apply. The YAML parses and fits the service's policy schema (server/policy.ts:human_review_pathsis a plain string array).🤖 Generated with Claude Code
https://claude.ai/code/session_015YbHtxQBZQ6pDHFsqumcuR