add ~max=<n> to --review-with for a per-reviewer iteration cap - #129
Merged
Conversation
--review-iterations is global and only ever reached the copilot and @<login> loops; the local agents and ollama had a hardcoded MAX_ITERATIONS=3 with no way to move it. That made a slow local model expensive to include in a review list, and made it impossible to budget reviewers differently in one run. A per-entry ~max=<n> suffix now caps that one reviewer's review -> fix -> re-review cycles, so --review-with claude~max=2,ollama~max=1,codex~max=3 gives each its own budget. It chains with ~opt in either order, is not part of the dedup identity (first occurrence's cap wins on collapse), and rides through /do:config saved defaults. This needed a second change to be usable: the local-agent and ollama loops report `guardrail` on cap exhaustion, which is inconclusive and blocks the merge gate -- so ~max=1 would have blocked nearly every merge. All four loops now report `capped` (clean-equivalent) when the cap was user-configured, and keep `guardrail` for a built-in ceiling. The split is deliberate: a ceiling you chose is a budget, one you didn't is an unfinished review. Runs that never pass ~max are unaffected. Ollama's coverage-gap rule still outranks both -- a partially-reviewed diff reports `incomplete`, never `capped`. ~max is ignored (with a warning) in --review-mode parallel, where each reviewer runs a single review-only pass.
atomantic
force-pushed
the
feat/review-with-max-iterations
branch
from
July 26, 2026 21:39
595a17e to
53b1409
Compare
atomantic
added a commit
that referenced
this pull request
Jul 27, 2026
The ~max=<n> docs landed in #129 but didn't say the suffix only has meaning in series mode. In parallel mode each reviewer runs a single review-only pass and the orchestrator applies the union once, so multi-reviewer-loop.md warns and ignores the cap -- a reader budgeting reviewers with ~max in parallel mode would otherwise expect it to apply.
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
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.
Summary
--review-iterationsis global and only ever reached thecopilotand@<login>loops. The local agents (codex/claude/agy/grok) andollamahad a hardcodedMAX_ITERATIONS=3with no way to move it — so a slow local model was expensive to include, and there was no way to budget reviewers differently in one run.A per-entry
~max=<n>suffix now caps that one reviewer's review → fix → re-review cycles:<n>is a non-negative integer;0means loop-until-clean, bounded by the existing 10-iteration guardrail. The suffix chains with~optin either order, is not part of the dedup identity (first occurrence's cap wins on collapse), and rides through/do:configsaved defaults (stored canonically as~optthen~max=<n>).The
cappedvsguardrailsplitThis needed a second change to be usable. The local-agent and ollama loops report
guardrailon cap exhaustion, which is inconclusive in the aggregate and blocks the merge gate — so~max=1would have blocked nearly every merge.All four loops now report
capped(clean-equivalent, as copilot/@<login>already did) when the cap was user-configured, and keepguardrailfor a built-in ceiling. A ceiling you chose is a budget; one you didn't is an unfinished review. Runs that never pass~maxare unaffected — their built-in caps behave exactly as before. Ollama's coverage-gap rule still outranks both: a partially-reviewed diff reportsincomplete, nevercapped.~maxis ignored (with a warning, not an abort) in--review-mode parallel, where each reviewer runs a single review-only pass and there are no cycles to cap — matching how that mode already handles--reviewer-appliesand the stop-mode flags.Scope
multi-reviewer-loop(suffix parsing, dedup rules, cap resolution + precedence, dispatch forwarding, parallel warning,Iterationscolumn in the summary);local-agent-review-loopandollama-review-looptake{MAX_ITERATIONS}/{MAX_EXPLICIT}instead of hardcoding 3;copilot-review-loopandgithub-reviewer-loopnote where their{REVIEW_ITERATIONS}now comes from.--review-with, plusconfig.mdfor saved-default validation and round-trip./do:rprforwards~maxto the local-agent and Ollama loops it dispatches and documents that it's a no-op on its bespoke Copilot flow (it doesn't accept--review-iterationseither)..changelogs/NEXT.md.Test plan
npm test— 174 pass, 0 fail.config.test.jsround-trip case covering~maxon a bare slug, a bracketed model, an@<login>,~max=0, and chained after~opt, asserting it coexists with a savedreview-iterationskey (storage stays suffix-agnostic; precedence is the review commands' job).--review-with; the installer copies these files verbatim.