Skip to content

add ~max=<n> to --review-with for a per-reviewer iteration cap - #129

Merged
atomantic merged 1 commit into
mainfrom
feat/review-with-max-iterations
Jul 26, 2026
Merged

add ~max=<n> to --review-with for a per-reviewer iteration cap#129
atomantic merged 1 commit into
mainfrom
feat/review-with-max-iterations

Conversation

@atomantic

Copy link
Copy Markdown
Owner

Summary

--review-iterations is global and only ever reached the copilot and @<login> loops. The local agents (codex/claude/agy/grok) and ollama had a hardcoded MAX_ITERATIONS=3 with 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:

--review-with claude~max=2,ollama[qwen3.6:35b]~opt~max=1,codex~max=3

entry := ( <agent> [ "[" <model> "]" ] | "@" <login> ) ( "~opt" | "~max=" <n> )*

<n> is a non-negative integer; 0 means loop-until-clean, bounded by the existing 10-iteration guardrail. The suffix 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 (stored canonically as ~opt then ~max=<n>).

The capped vs guardrail split

This needed a second change to be usable. The local-agent and ollama loops report guardrail on cap exhaustion, which is inconclusive in the aggregate and blocks the merge gate — so ~max=1 would 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 keep guardrail for a built-in ceiling. A ceiling you chose is a budget; one you didn't is an unfinished review. Runs that never pass ~max are unaffected — their built-in caps behave exactly as before. Ollama's coverage-gap rule still outranks both: a partially-reviewed diff reports incomplete, never capped.

~max is 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-applies and the stop-mode flags.

Scope

  • Loops: multi-reviewer-loop (suffix parsing, dedup rules, cap resolution + precedence, dispatch forwarding, parallel warning, Iterations column in the summary); local-agent-review-loop and ollama-review-loop take {MAX_ITERATIONS}/{MAX_EXPLICIT} instead of hardcoding 3; copilot-review-loop and github-reviewer-loop note where their {REVIEW_ITERATIONS} now comes from.
  • Commands: the 7 specs that parse --review-with, plus config.md for saved-default validation and round-trip. /do:rpr forwards ~max to 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-iterations either).
  • Docs: README suffix explainer, examples, and flag table; .changelogs/NEXT.md.

Test plan

  • npm test — 174 pass, 0 fail.
  • Added a config.test.js round-trip case covering ~max on a bare slug, a bracketed model, an @<login>, ~max=0, and chained after ~opt, asserting it coexists with a saved review-iterations key (storage stays suffix-agnostic; precedence is the review commands' job).
  • Spec-only otherwise — no runtime code parses --review-with; the installer copies these files verbatim.

--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
atomantic force-pushed the feat/review-with-max-iterations branch from 595a17e to 53b1409 Compare July 26, 2026 21:39
@atomantic
atomantic merged commit 45d3883 into main Jul 26, 2026
4 checks passed
@atomantic
atomantic deleted the feat/review-with-max-iterations branch July 26, 2026 21:40
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant