diff --git a/pilot/ai_review.py b/pilot/ai_review.py index 37f5d50..b03d135 100644 --- a/pilot/ai_review.py +++ b/pilot/ai_review.py @@ -1277,6 +1277,35 @@ def parse_repo_config(raw: str) -> dict: if tr is not None: out["triage"] = tr + # Repo-level kill-switch: `enabled: false` lets a maintainer pause the bot + # for this repo without removing the file (handy during a flaky provider + # outage). Always written so callers can do `cfg.get("enabled") is False` + # without a separate default — the file itself is committed, so we treat + # absent / wrong-type as an explicit off rather than as "config missing". + en = data.get("enabled") + out["enabled"] = en if isinstance(en, bool) else False + + # Compare-against roster: list of `cost_model.PRICES` keys the render layer + # uses to print equivalent-cost lines (one per key) for maintainer + # budgeting. Unknown keys are dropped with a stderr line so a typo is loud. + # Lazy import: `cost_model` has no dep on `ai_review`, and the ollama + # fallback path never hits this branch — keep import-time cost low there. + from cost_model import PRICES as _PRICES + ca = data.get("compare_against") + if isinstance(ca, list): + cleaned: list[str] = [] + for x in ca: + if isinstance(x, str) and x.strip() in _PRICES: + cleaned.append(x.strip()) + elif isinstance(x, str): + print( + f"pragent: ignoring compare_against entry {x!r} " + f"(not in cost_model.PRICES); valid: {', '.join(sorted(_PRICES))}", + file=sys.stderr, flush=True, + ) + if cleaned: + out["compare_against"] = cleaned[:12] + return out diff --git a/tests/pilot/test_ai_review.py b/tests/pilot/test_ai_review.py index d5e02dd..c7490f7 100644 --- a/tests/pilot/test_ai_review.py +++ b/tests/pilot/test_ai_review.py @@ -400,11 +400,11 @@ def test_parse_repo_config_full(): def test_parse_repo_config_partial_and_bad(): - assert parse_repo_config('{"focus":"not-a-list"}') == {} - assert parse_repo_config('{"focus":["ok"]}') == {"focus": ["ok"]} + assert parse_repo_config('{"focus":"not-a-list"}') == {"enabled": False} + assert parse_repo_config('{"focus":["ok"]}') == {"focus": ["ok"], "enabled": False} assert parse_repo_config("") == {} assert parse_repo_config("not json") == {} - assert parse_repo_config('{"instructions":" "}') == {} + assert parse_repo_config('{"instructions":" "}') == {"enabled": False} # --------------------------------------------------------------------------- @@ -895,7 +895,10 @@ def test_parse_repo_config_still_accepts_normal_config(): cfg = parse_repo_config(json.dumps({ "focus": ["security"], "languages": ["go"], "instructions": "No bare throw.", })) - assert cfg == {"focus": ["security"], "languages": ["go"], "instructions": "No bare throw."} + assert cfg == { + "focus": ["security"], "languages": ["go"], "instructions": "No bare throw.", + "enabled": False, + } # --------------------------------------------------------------------------- @@ -917,7 +920,7 @@ def test_fetch_repo_config_uses_given_base_ref(monkeypatch): monkeypatch.setattr(ai_review, "gitea_get", fake_get) cfg = ai_review.fetch_repo_config("http://g", "o/r", "tok", ref="main") - assert cfg == {"focus": ["security"]} + assert cfg == {"focus": ["security"], "enabled": False} assert seen["path"] == "contents/.pr-review.json?ref=main" @@ -1755,3 +1758,56 @@ def test_severity_badge_labels_each_known_severity(): badge = _severity_badge(sev) assert f"[{sev.upper()}]" in badge, (sev, badge) + +# --------------------------------------------------------------------------- +# parse_repo_config — `enabled` (kill-switch) + `compare_against` (cost roster) +# --------------------------------------------------------------------------- + + +def test_parse_repo_config_enabled_true(): + cfg = parse_repo_config('{"enabled": true}') + assert cfg.get("enabled") is True + + +def test_parse_repo_config_enabled_false_explicit(): + cfg = parse_repo_config('{"enabled": false}') + assert cfg.get("enabled") is False + + +def test_parse_repo_config_enabled_missing_defaults_false(): + cfg = parse_repo_config('{}') + assert cfg.get("enabled") is False + + +def test_parse_repo_config_enabled_wrong_type_ignored(): + cfg = parse_repo_config('{"enabled": "yes"}') + assert cfg.get("enabled") is False + + +def test_parse_repo_config_compare_against_default_absent(): + # absent in returned cfg; defaults applied in render, not parse_repo_config + cfg = parse_repo_config('{}') + assert "compare_against" not in cfg + + +def test_parse_repo_config_compare_against_valid(): + cfg = parse_repo_config( + '{"compare_against": ["claude-sonnet-5", "gpt-5", "gemini-2.5-pro"]}') + assert cfg["compare_against"] == ["claude-sonnet-5", "gpt-5", "gemini-2.5-pro"] + + +def test_parse_repo_config_compare_against_drops_unknown_keys(capfd): + cfg = parse_repo_config( + '{"compare_against": ["claude-sonnet-5", "bogus-1", "gpt-5"]}') + assert "bogus-1" not in cfg["compare_against"] + assert "claude-sonnet-5" in cfg["compare_against"] + captured = capfd.readouterr() + assert "bogus-1" in captured.err + + +def test_parse_repo_config_compare_against_caps_at_12(): + raw_keys = ["claude-sonnet-5"] + [f"bogus-{i}" for i in range(20)] + cfg = parse_repo_config(json.dumps({"compare_against": raw_keys})) + # Only claude-sonnet-5 is valid; rest dropped; net result is 1 entry. + assert len(cfg["compare_against"]) == 1 +