chore: converge AI review loop on accepted tradeoffs - #101
Merged
Merged
Conversation
The review loop never terminates on PRs containing a deliberate tradeoff: stateless reviews re-flag the same by-design property every round, so "zero findings" is unreachable. Give the loop a real stop condition and let it carry state across rounds. - claude-review.yml: read the PR's "Design decisions / Accepted tradeoffs" section + prior maintainer replies and do NOT re-raise settled decisions; "Found N issues" counts only new actionable findings; by-design/accepted items go under a separate "### Design notes" heading, excluded from the count. - resolve-pr-reviews.md: add a 🟰 Accepted/by-design classification that gets recorded in the PR's accepted-tradeoffs section (so the reviewer prompt and future rounds stop re-litigating it); define "unresolved" as new actionable findings only; add a convergence/stop condition (only LOW/accepted remain → done) with a 3-cycle hard cap. - fix-issue.md: seed a "Design decisions / Accepted tradeoffs" PR-body section when a change makes a deliberate tradeoff.
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.
What & why
PR #95 went ~10 review rounds without "consensus" — not because of open defects (the one real bug was fixed in round 1), but because the loop can't terminate on a deliberate, documented tradeoff. AI reviews are stateless: each round re-derives from scratch and re-flags the same by-design property (the exact-deposit coupling), so "zero findings" is unreachable, and every push (even a 1-line doc tweak) resets the gate. This gives the loop a real stop condition and lets it carry state across rounds.
Changes
.github/workflows/claude-review.yml— the reviewer now reads the PR's "Design decisions / Accepted tradeoffs" section + prior maintainer replies and must not re-raise settled decisions. "Found N issues" counts only new, actionable findings; by-design / already-accepted items go under a separate### Design notesheading, excluded from the count..claude/commands/resolve-pr-reviews.md— adds a 🟰 Accepted / by-design classification, recorded in the PR's "Design decisions / Accepted tradeoffs" section (fetch-amend-set) so the reviewer prompt and future rounds stop re-litigating it; defines "unresolved" as new actionable findings only; adds a convergence / stop condition (both reviewers on current head + CI green + only LOW/accepted remain → done) with a 3-cycle hard cap..claude/commands/fix-issue.md— seeds a "Design decisions / Accepted tradeoffs" PR-body section when a change makes a deliberate tradeoff.The accepted-tradeoffs section is the key lever: it is the shared state the (otherwise stateless) reviewer reads, so a decision made once isn't re-surfaced as a fresh finding every round.
Release impact
No release impact — CI workflow + command docs only, no published package touched.
🤖 Generated with Claude Code