code-review: make DOMAIN_CRITIC_CAP operator-tunable

[cchenault4]

Summary

Makes DOMAIN_CRITIC_CAP operator-tunable via .closedloop-ai/settings/code-review.json, the file that already hosts bha_unified_threshold_loc and out_of_hunk_confidence_floor.

The problem. The cap was a hardcoded 3 with no settings path, and it drops by (priority asc, reviewer asc). A repo whose critic-gates.json uses the legacy moduleCritics[] schema gets every entry migrated by _migrate_module_critics as required: False with no priority — so all of them sit at the default 2 and the tiebreak degenerates to alphabetical by reviewer name.

That is not a theoretical edge. On two symphony-alpha cost/metric PRs (#4330, #4349), the coverage critic proposed logical-metric-reconciliation-auditor specifically because of the diff's tally/cost semantics, consolidate accepted it, and the cap then dropped it — while api-architect, caching-strategist, and database-architect survived on their first letter. The reviewer best matched to the change was the one systematically excluded, on every such PR.

The change.

• _load_code_review_settings gains domain_critic_cap (int, ≥ 0) with the same validation discipline as its siblings: bool rejected (it is an int subclass), negative and wrong-type fall back to the default rather than crashing the stage.
• cmd_arbitrate_budget reads it with precedence mirroring bha_unified_threshold_loc: --domain-critic-cap flag → code-review.json → built-in DOMAIN_CRITIC_CAP.
• arbitrate-budget gains --settings (test isolation, mirroring validate) and --domain-critic-cap.
• 0 is a valid, documented kill switch — no domain critics spawn; source: "core" reviewers (Design Critic, Impact Analyzer) stay exempt as before.

Docs. README and commands/start.md no longer state the cap as a fixed 3. Both now explain when raising it is the right call — and note that raising the cap is the blunt remedy, while migrating rules to the canonical coverage[] schema (which supports explicit priority and required) is the precise one.

Test plan

• 9 new tests. Five cover loader validation (override honored, 0 kill switch, negative → default, wrong type → default, bool rejected). Four cover arbitrate-budget behavior: the settings file actually raising the cap, --domain-critic-cap beating the settings file (precedence pinned so a refactor cannot silently invert it), 0 spawning zero domain critics with all of them cap-deferred and no coverage-gap findings, and a malformed value leaving the fleet at the default 3.
• Test-isolation fix: the shared _run_arbitrate_budget harness now always points --settings at a test-local path, so a code-review.json in the CWD can never leak into a run.
• Snapshot updated deliberately: cli_parser_resolved.json regenerated. The diff is exactly the two new arbitrate-budget flags and nothing else — I diffed parser-by-parser before writing to confirm no collateral drift.
• Full suite: 1339 passed, 3 skipped, 0 failed across test_code_review_helpers.py, test_code_review_schema.py, test_golden_fixtures.py, test_prefix_golden.py — including the golden-fixture and prefix-parity harnesses.
• Regression check against baseline: stashed the change, captured the clean-HEAD failure set, restored, and diffed. Exactly one new failure (the CLI snapshot, expected and then updated) and zero others. (An earlier run showed 95 failures on both sides — that was my venv missing pyyaml, not the repo.)

What could still break: the cap remains tier-uniform, so --depth deep still does not widen it — deep's extra breadth continues to come from the cap-exempt core reviewers. Setting domain_critic_cap above the total reviewer --cap has no additional effect, since total-cap arbitration still runs first.

🤖 Generated with Claude Code

[shafty023]

Beginning a code-review. Please allow time for me to finish before merging the PR

[shafty023]

This makes review policy controllable by the PR being reviewed. In GitHub mode this settings path is read from the checked-out PR tree, so a contributor can add domain_critic_cap: 0 and suppress every rule/critic reviewer with no coverage-gap finding (or raise it to amplify review work). Can we load this budget from a trusted base/external operator source and add a test proving a head-only settings change cannot alter that PR’s fleet?

[shafty023]

This still says required critics dropped by this cap emit coverage-gap findings, but the new zero-cap test explicitly asserts gaps["findings"] == [], and arbitration records them only in deferred_for_budget. An operator lowering the cap could rely on a safety signal that never appears. Please make this line and commands/start.md match the implemented no-gap behavior, or change the implementation if gaps are intended.

[shafty023]

These tests bypass the production config path by always passing a test-local settings path into a hand-built namespace. The real stage omits --settings and relies on parser/default-path wiring, so that wiring can regress while this suite stays green. Can we add a run-prefix or real-parser fixture that creates .closedloop-ai/settings/code-review.json and proves the final spawn plan honors a raised or zero cap?

[shafty023]

This adds shipped plugin behavior, but plugins/code-review/.claude-plugin/plugin.json is unchanged at 3.7.0. The required Plugin Version Bump check is already failing on this exact head. Please bump the code-review plugin version once relative to main (the repo guidance classifies a backward-compatible new setting/CLI surface as a minor bump).

[shafty023]

The repo marks this README as generated and requires /update-documentation for plugin changes, including the root changelog entry. CHANGELOG.md is unchanged on this head, so the release docs are incomplete even though the README was edited. Please run the existing documentation workflow and commit its generated README/changelog output after the version bump.