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