Repository navigation
docs(adr): apply the code-review ADR challenge amendments #7642
Description
Refs: #7576
Summary
The review and merge ADRs were challenged against the Anthropic article "How the Claude Code team does code review" (https://claude.dev/blog/solving-code-review-at-anthropic, 2026-10-15) and external practice. Each amendment below survived a defended consensus that checked it against the article, the owner record (poteto-review Q and D ids, GUIDE rows) and github-iac ADRs 0007 and 0011. Owner-ratified decisions are not overturned here; where the article disagrees with one, it is recorded as a departure in the issues linked below. Each section names the exact amendment and its basis. Digest ids (for example "05 C18") name verified claims in the digest of the article; figures from it are vendor-claimed.
ADR 0002 (advisory review with earned promotion)
- Record that thread resolution already gates merges through the live ruleset (
required_review_thread_resolution: trueon main), so an open lane finding blocks until resolved without any lane check becoming required. - Record that github-iac ADR 0011 (Decision and its 2026-10-04 amendment) rules out promotion: "no organization ruleset may make their status a merge or deployment prerequisite" for agentic reviews, and an AI finding blocks only once a deterministic reproducer replaces it. 0002:15-18 still says "earned promotion still apply".
- Close the merge-queue revisit trigger.
- State that precision measurement tunes the reviewer (prompt, model, effort, what each lane reports) rather than promoting it. feat(review): measure review-lane precision from how findings were answered #7638 carries the measurement.
- Not added: nits out of the blocking path, already decided by Q500 (blocking-only findings after the first review) and Q512 (a nit that breaks nothing becomes a follow-up issue).
ADR 0037 (seat layer)
- Status header: the surviving ready step (draft-first, security review at ready) is to be superseded by Q500/Q501. With a CI lane, review drops out of prep and ready, and
pr_open_statedefaults toready(0037:9-10, 0037:174-178; GUIDE 73, 75).
ADR 0038 (review lanes on every push)
- Cite github-iac ADR 0011's 2026-09-05 amendment, which says a per-push re-review "only ever earned its cost against a gate", and state that 0038 departs from it for this repository on the local merge-conflict evidence (0038:42-48) without creating a gate.
- Add rounds-to-pass measures to the revisit triggers (05 C17), fed from the Q521 lifecycle-metrics work. The round cap is 3 (Q500; 05 C18).
- Background only: update the sentence at 0038:17-18 ("the product docs set no default trigger") to what the current Code Review docs page says, with a fetch date. Managed Code Review is not used here (GUIDE 122, Q594), so this changes no decision; 0038:22 already names that page change as a recheck trigger.
- Not added: the lane resolving its own threads. The Q512 findings lane and review-discipline D7.5 own thread clearance.
ADR 0003 (verification guards earn default-on by measured precision)
- When the reader of a check's output is an agent, measure precision by adjudication of each finding, not by how often the agent acted on it.
- Name
skill-reference-verify, measured at 57%, under the D64 inline exception (reason, expiry, approval; D84 90-day expiry), or re-measure it. - State that hook-precision's per-incident tests are the only post-ship check until guardrails: grade every default-on guard's precision from the per-session hook event log (ADR 0003) #3756 lands.
- Basis: 0003:45-49; owner tier table (0 effective false positives when blocking, under 10% when advisory; D29-D31, GUIDE 116); Google CACM 2018 ("produce no effective false positives"; its 10% threshold is set "somewhat arbitrarily"); 05 C10.
ADR 0072 (check placement)
- A check placed where it blocks, such as a
pre-commithook exiting 1, meets the blocking precision bar (zero effective false positives; D29, D84), and installing one follows D81/Q95 (ask first). Basis: docs/conventions/check-placement/README.md:65-66, 143; Google CACM 2018. - Add a model-change recheck to rule 4: the pilot's own confound note (check-placement/README.md:289-294) ties its result to the model it ran on.
ADR 0050 (merge authority reaches C4 by evidence)
- Mark Decision 2 "not in force; superseded when Phase 9 lands". Q505 drops the work classes, and Q567 makes the non-author verifier verdict the merge criterion ("no report-only phase", "promotion bar superseded"). The current text has drifted from docs/conventions/loop-lane/README.md:50-53 and docs/conventions/pr-pipeline/README.md:341-343.
- Keep Decision 1 (vendor-hosted cap). Q581 (the lane takes only agent-ready items) now covers Decision 3's intent.
- Not added: a C4 gate built on months of C3 data, which contradicts Q567.
- ADR 0050 belongs to the Automation Lanes owner; this text goes to that owner first.
review-discipline (plugins/source-control/reference/review-discipline.md)
- Add
INTENTIONALto the D5 outcomes, resolved with a reason. The article sorts reviewer comments by whether the person "said it was intentional" (05 C6-C8). - No resolve-and-drop disposition is added.
VALID (defer)is widened to cover a nit that breaks nothing, which becomes a follow-up issue blocked by the PR (Q512). Today review-discipline.md:142 limitsVALID (defer)to structural, urgent-but-cannot-land or research-blocked findings and sends small ones toVALID (fix now). - Keep D3 validation (Q512: findings are "never taken at face value"). The round cap already exists (Q500). Basis: review-discipline.md:293-305.
Acceptance criteria
- ADR 0002 records ruleset thread resolution as the existing gate, cites github-iac ADR 0011 as ruling out promotion, drops "earned promotion still apply", closes the merge-queue revisit trigger, and names precision as a tuning measure.
- ADR 0037's status header says the ready step and draft-first are superseded by Q500/Q501.
- ADR 0038 cites github-iac ADR 0011's 2026-09-05 per-push reasoning it departs from, lists rounds-to-pass among its revisit triggers, and its default-trigger sentence matches the current docs page, marked as background.
- ADR 0003 states adjudicated precision for agent-read checks, names
skill-reference-verifyat 57% under D64 (or records a re-measure), and names hook-precision as the only post-ship check until guardrails: grade every default-on guard's precision from the per-session hook event log (ADR 0003) #3756. - ADR 0072 states that a check placed where it blocks meets the blocking precision bar, and rule 4 carries a model-change recheck.
- ADR 0050 marks Decision 2 not in force and superseded when Phase 9 lands, keeps Decision 1, and the Automation Lanes owner has seen the text before merge.
- review-discipline lists
INTENTIONAL(resolved with a reason), itsVALID (defer)rule covers a nit that breaks nothing (filed as a follow-up issue, Q512), and it adds no disposition that resolves a finding without a fix or a cited follow-up.
Activity
github-actions commentedon Oct 16, 2026
Proposed work class: work-class: scoped
Basis: automated intake triage; the lanes App applied this class.
Size: Medium
Complexity: Medium
Uncertainty: Known
Flags: none
Blocked by: none
Understood: this applies the agreed code-review ADR amendments to ADRs 0002, 0003, 0037, 0038, 0050 and 0072 and to review-discipline.md. The acceptance criteria are specific, and the change is docs only. Nothing is missing that would block starting.
One note: the issue says the ADR 0050 text goes to the Automation Lanes owner before merge. That is a review step on the PR, not an open question on this issue.
No likely duplicates found among the open issues. Related issues already linked: #7576, #7638, #7641.
Automated triage, applied by the lanes App.
kyle-sexton commentedon Oct 24, 2026
Released for re-triage under the owner's 2026-10-24 rule: an issue stops for a person only for an intent or vision question, a security call, a host or attended run, or spend outside prepaid credit. The hold label came off and intake triage runs again now; it re-applies a hold, with a numbered question, when one of those applies.
To hold this issue again, add
needs-human(orstatus: needs-decision) and say why in a comment.melodic-automation-lanes commentedon Oct 24, 2026
The work-resolve-issue lane stopped on its usage limit, not on this issue. It keeps
agent-ready, and work-unblock starts it again after 2026-10-25T16:00:00Z.Run: https://github.com/melodic-software/claude-code-plugins/actions/runs/40144409141
melodic-automation-lanes commentedon Oct 25, 2026
The work-resolve-issue lane stopped partway through this issue and needs a person.
Why, in the run's own words:
What the run says it did not do:
Partial work is on the branch
work-resolve-issue/issue-7642-40187897719-1. A retry continues from it while it is not behind the default branch, and otherwise starts fresh from the default branch; delete the branch first to always start fresh.Run: https://github.com/melodic-software/claude-code-plugins/actions/runs/40187897719
To retry, remove
needs-humanand addagent-readyagain.kyle-sexton commentedon Oct 25, 2026
Re-checked on main (2d9ac7f). The ADR 0050 item is done: Decision 2 is marked superseded by ADR 0082, so that criterion drops. The premise is only partly false: no ruleset requires a review check, but ci-status requires both review lanes on the head and fails while a lane hold stands (
.github/workflows/pr-require-checks.yml:4122-4159,docs/conventions/pr-pipeline/README.md:1731), so ADR 0002's "precision tunes, not promotes" and ADR 0038's "without creating a gate" need an owner reframe. Still unapplied: ADR 0002:19,24(earned promotion) and the open merge-queue trigger:98,421; noskill-reference-verify/D64 entry in ADR 0003; no model-change recheck in ADR 0072 rule 4; no rounds-to-pass trigger in ADR 0038; no Q500/Q501 line in ADR 0037; noINTENTIONALoutcome in review-discipline.md. This stays held for the owner's framing decision.kyle-sexton commentedon Oct 25, 2026
Decision recorded.
The owner's recorded direction, 2026-10-24, settles the framing question this issue left open: "the review process should be the gate that flags this stuff (central place to flag something as mergeable or not)" and "meaning the REVIEW lanes (code review + security review) are what determine if a PR requires a human" (comment on #9595). On main the review lanes already gate merges through ci-status (
.github/workflows/pr-require-checks.yml:4122-4159,docs/conventions/pr-pipeline/README.md:1731), so the ADRs that say otherwise get amended to match. The ADR 0050 item is done (Decision 2 is marked superseded by ADR 0082) and drops.Acceptance criteria:
:19,:24). It says the review lanes gate merges through ci-status, and cites the 2026-10-24 direction above. Its open merge-queue trigger (:98,:421) is resolved or restated against that gate.skill-reference-verify(D64) entry.plugins/source-control/reference/review-discipline.mdgains anINTENTIONALoutcome.pr-require-checks.ymldoes not implement.