diff --git a/scripts/guardian_bench.py b/scripts/guardian_bench.py index 1bb0deee..af75cc9a 100644 --- a/scripts/guardian_bench.py +++ b/scripts/guardian_bench.py @@ -33,6 +33,7 @@ ) from cgis.guardian.chunked import run_review_routed from cgis.guardian.collector import ContextCollector, parse_features +from cgis.guardian.evidence import Evidence, collect_evidence from cgis.guardian.findings import ReviewResult from cgis.guardian.providers.base import BaseProvider from cgis.guardian.review_fingerprint import ( @@ -51,6 +52,11 @@ log = structlog.getLogger(__name__) _REPO_ROOT = Path(__file__).parent.parent.absolute() + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from guardian_replay_skeptic import changed_files, worktree_at # noqa: E402 + _BENCH_DIR = _REPO_ROOT / "benchmarks" / "guardian" @@ -103,7 +109,10 @@ async def _run_one( With ``replay_finder`` the finder is skipped entirely and its recorded findings are judged instead, so a skeptic variant is measured against a - frozen set (spec §4.1) — no worktree, no ingest, no finder tokens. + frozen set (spec §4.1) — no ingest and no finder tokens. There *is* a + worktree: the checkers have to report on the code the recording was taken + from, and this line claimed there was none for as long as the evidence was + silently skipped. With ``record_finder`` the finder pass is written to disk and the skeptic is skipped, producing exactly that frozen set. """ @@ -122,7 +131,17 @@ async def _run_one( if replay_finder is not None: recording = load_finder_recording(replay_finder) - result = await _judge_recording(recording, skeptic) + # Evidence at the reviewed commit, not None. The replay path used to + # hardcode None on the reasoning that a recording is not a checkout — + # true, and beside the point: the commit is named by the fixture, so a + # worktree gets one, which is exactly what `guardian_replay_skeptic` + # does. Production runs with GUARDIAN_EVIDENCE=1, so judging without it + # measured a configuration nobody runs and quietly attributed the + # difference to whatever else the arm changed (#246). + evidence = await asyncio.to_thread( + _evidence_at_head, truth.head, recording.diff, _REPO_ROOT + ) + result = await _judge_recording(recording, skeptic, evidence=evidence) await _score_and_record( truth, run_idx, results_path, result, model, provider, skeptic, chunks=None ) @@ -159,17 +178,33 @@ async def _run_one( ) +def _evidence_at_head(head: str, diff: str, repo_root: Path) -> Evidence | None: + """The checkers' output at the commit the recording was taken from. + + A worktree, because the recording is not a checkout — which is the fact the + old `evidence=None` was justified by, and it argues for building one rather + than for going without. + """ + with worktree_at(head, repo_root) as tree: + return collect_evidence(tree, changed_files(diff)) + + async def _judge_recording( - recording: FinderRecording, skeptic: tuple[BaseProvider, str] | None + recording: FinderRecording, + skeptic: tuple[BaseProvider, str] | None, + *, + evidence: Evidence | None, ) -> ReviewResult: - """Run ONLY the skeptic over a frozen finder pass (spec §4.1).""" + """Run ONLY the skeptic over a frozen finder pass (spec §4.1). + + `evidence` is keyword-only with no default, matching `judge_all` and for the + same reason: this function spent a year passing None, and a default would + let the next caller do it again without saying so. + """ result = recording.result if skeptic is None or not result.findings: return result - # The replay path holds a recording, not a checkout: there is no project - # root, so no checkers to run. Stated as None rather than defaulted — which - # is the point of the argument being required (#401). - judgements = await judge_all(skeptic[0], result.findings, recording.diff, evidence=None) + judgements = await judge_all(skeptic[0], result.findings, recording.diff, evidence=evidence) judged = sum(1 for j in judgements if j is not None) return result.model_copy( update={ diff --git a/scripts/skeptic_arms.py b/scripts/skeptic_arms.py new file mode 100644 index 00000000..cec3f1b0 --- /dev/null +++ b/scripts/skeptic_arms.py @@ -0,0 +1,322 @@ +"""Judge the same frozen finder passes with two skeptics, and score both (#246). + +#246 asks whether a **cross-model** skeptic cuts noise where a same-model one +cannot: a mistral skeptic over a mistral finder was measured as binary — it +refuted everything or nothing — and the designed answer was a different vendor. + +That answer has since shipped: production runs `GUARDIAN_PROVIDER=mistral` with +`GUARDIAN_SKEPTIC=gemini`. So running the issue as written would measure the +configuration already in use and could not say *why* it still produces 19 +findings with 1 real. What was never measured is the comparison itself — the +same finder output judged by both. + +The corpus makes a stronger question answerable than the one asked. Of the 37 +frozen passes carrying findings, 30 came from a gemini finder and 7 from a +mistral finder, so **both orientations are present**. Judging every one with +each skeptic separates two hypotheses the issue conflates: + +* cross-vendor refutes more in *both* orientations → the mechanism is the vendor + difference, as #246 supposes; +* gemini refutes more whichever finder produced the findings → the mechanism is + gemini's disposition, and "cross-model" was never the operative word. + +The original single-orientation design cannot tell those apart. + +**Evidence is collected, not skipped.** `guardian_bench`'s replay path hardcodes +`evidence=None`, which would measure a configuration production does not run +(`GUARDIAN_EVIDENCE=1`). Here it is collected once per PR in a worktree at the +fixture head — the same mechanism `guardian_replay_skeptic` uses. Four of the six +fixtures touch Python and yield evidence, covering 96 of 135 findings; the other +two touch none, so their findings are judged with `evidence=None` because that is +also what production would do for them. + +No finder call is made. The cost is one skeptic call per finding per arm. +""" + +import argparse +import asyncio +import json +import os +import sys +import tempfile +from collections import defaultdict +from collections.abc import Mapping +from datetime import UTC, datetime +from pathlib import Path +from typing import Any + +from cgis.guardian.bench import ( + GroundTruth, + killed_ground_truth, + load_ground_truth, + match_findings, + score, +) +from cgis.guardian.evidence import Evidence, collect_evidence +from cgis.guardian.providers.base import BaseProvider +from cgis.guardian.recording import load_finder_recording +from cgis.guardian.runner import build_skeptic_provider +from cgis.guardian.skeptic import apply_judgements, judge_all, visible_findings + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from guardian_replay_skeptic import changed_files, worktree_at +from recordings_from_corpus import build, is_frozen_pass, row_key + +REPO_ROOT = Path(__file__).resolve().parent.parent +BENCH_DIR = REPO_ROOT / "benchmarks" / "guardian" + +#: The two arms. Named rather than derived, because "the opposite of the primary" +#: is exactly the rule under test — deriving the arm from the finder would build +#: the hypothesis into the instrument. +ARMS = ("gemini", "mistral") + + +class NoArmError(RuntimeError): + """Raised when an arm's provider cannot be built. + + Refused before any call is spent. Half an experiment costs the same as none + and reads like a result, which is worse. + """ + + +def arm_provider(name: str, env: Mapping[str, str]) -> tuple[BaseProvider, str]: + """The skeptic for one arm, or a refusal naming the missing key. + + `GUARDIAN_SKEPTIC_MODEL` is dropped rather than passed through. + `build_skeptic_provider` applies it to whichever provider it builds, so an + environment holding the production value (`gemini-2.5-flash`) would hand + that model name to the mistral arm — one arm running a model that does not + exist, while the other ran the intended one. Each arm takes its provider's + own default, and the models used are recorded on every row. + """ + per_arm = {k: v for k, v in env.items() if k != "GUARDIAN_SKEPTIC_MODEL"} + built = build_skeptic_provider({**per_arm, "GUARDIAN_SKEPTIC": name}, primary="none") + if built is None: + _msg = ( + f"Arm {name!r} could not be built — its API key is missing, or the model was " + f"rejected. Both arms must exist before either is run: a single-arm result " + f"answers nothing this experiment asks." + ) + raise NoArmError(_msg) + return built + + +def finder_models(results: Path) -> dict[str, str]: + """`row_key` → the model that produced that pass. + + The recording carries findings and a diff and nothing about who wrote them, + so the vendor split — the whole reason both orientations are separable — has + to come back from the corpus row. + """ + models: dict[str, str] = {} + for line in results.read_text(encoding="utf-8").splitlines(): + if not line.strip(): + continue + row = json.loads(line) + if is_frozen_pass(row): + models[row_key(row).replace(":", "-")] = str(row.get("model") or "unknown") + return models + + +def evidence_for_pr(truth: GroundTruth, diff: str, repo_root: Path) -> Evidence | None: + """Checker output at the reviewed commit, or None when there is nothing to check. + + Once per PR rather than once per pass: every pass of a PR was shown the same + diff, so the checkers would report the same thing, and a worktree costs a + `uv sync`. None here is not a degradation — a fixture whose diff touches no + Python has nothing for the checkers to say, and production would collect + nothing for it either. + """ + with worktree_at(truth.head, repo_root) as tree: + return collect_evidence(tree, changed_files(diff)) + + +async def judge_one( + provider: BaseProvider, + recording_path: Path, + truth: GroundTruth, + evidence: Evidence | None, +) -> dict[str, Any]: + """One recording under one arm, scored the way `guardian_bench` scores a run.""" + recording = load_finder_recording(recording_path) + findings = recording.result.findings + judgements = await judge_all(provider, findings, recording.diff, evidence=evidence) + judged = apply_judgements(findings, judgements) + visible = visible_findings(judged) + matches = match_findings(visible, truth) + bench_score = score(matches, truth) + return { + "findings": len(findings), + "refuted": sum(1 for f in judged if f.verdict == "refuted"), + "uncertain": sum(1 for f in judged if f.verdict == "uncertain"), + "confirmed": sum(1 for f in judged if f.verdict == "confirmed"), + "unruled": sum(1 for j in judgements if j is None), + "recall": bench_score.recall, + "precision": bench_score.precision, + "noise": bench_score.noise, + # The recall guard, and the reason `missed` is not enough: it cannot tell + # "the finder never found it" from "the finder found it and the skeptic + # killed it" (#270), and only the second is this experiment's doing. + "killed_gt": killed_ground_truth(judged, truth), + } + + +async def collect_rows(repo_root: Path, limit: int | None) -> list[dict[str, Any]]: + """Both arms over every frozen pass; one row per (pass, arm). + + Returns the rows rather than writing them. The write is the caller's, and + synchronous: a blocking file write inside the event loop is the same defect + the worktree collection above avoids with `to_thread`, and here there is + nothing to gain by being in the loop at all. + """ + arms = {name: arm_provider(name, os.environ) for name in ARMS} + models = finder_models(BENCH_DIR / "results.jsonl") + + with tempfile.TemporaryDirectory(prefix="arms-") as tmp: + written = build(BENCH_DIR / "results.jsonl", Path(tmp), BENCH_DIR, repo_root) + # Passes with no findings cost nothing to judge and contribute nothing to + # a comparison of judgements, so they are dropped before any worktree is + # built rather than filtered out of the numbers afterwards. + recordings = [p for p in written if load_finder_recording(p).result.findings] + recordings.sort(key=lambda p: p.stem) + if limit is not None: + recordings = recordings[:limit] + + by_pr: dict[int, list[Path]] = defaultdict(list) + for path in recordings: + by_pr[int(path.stem.split("@")[0])].append(path) + + rows: list[dict[str, Any]] = [] + for pr in sorted(by_pr): + truth = load_ground_truth(BENCH_DIR / f"pr-{pr}.yaml") + sample = load_finder_recording(by_pr[pr][0]) + evidence = await asyncio.to_thread(evidence_for_pr, truth, sample.diff, repo_root) + print( + f"pr-{pr}: {len(by_pr[pr])} passes, " + f"evidence {'collected' if evidence else 'unavailable'}", + file=sys.stderr, + ) + for path in by_pr[pr]: + for name, (provider, model) in arms.items(): + scored = await judge_one(provider, path, truth, evidence) + rows.append( + { + "timestamp": datetime.now(UTC).isoformat(), + "pr": pr, + "pass": path.stem, + "arm": name, + "skeptic_model": model, + "finder_model": models.get(path.stem, "unknown"), + "evidence": evidence is not None, + **scored, + } + ) + print( + f" {path.stem} [{name}] {scored['refuted']}/{scored['findings']} refuted", + file=sys.stderr, + ) + + return rows + + +#: Substrings that identify a model's vendor, lower-cased. +#: +#: Explicit rather than "does the name contain the vendor's name", because +#: Mistral ships families that do not: `codestral-latest` is a Mistral model and +#: was the arm proposed for #246's first draft. Raised in review of #411. +_VENDOR_MARKERS: dict[str, tuple[str, ...]] = { + "gemini": ("gemini",), + "mistral": ("mistral", "codestral", "ministral", "magistral"), +} + +#: A model this table does not recognise. **Not** a third vendor — a refusal to +#: guess, and it has to travel as far as the report. +#: +#: The failure it replaces was silent and pointed one way. An unrecognised model +#: used to become "other", "other" never equals an arm name, so every one of its +#: rows read `cross` — in the exact variable this experiment measures. A new +#: vendor tomorrow would not have produced an error; it would have produced a +#: wrong answer that looked like the expected one. +UNKNOWN_VENDOR = "unknown" + + +def _vendor(model: str) -> str: + """The vendor behind a model name, or `UNKNOWN_VENDOR`. + + Lower-cased first: the corpus holds names as the provider reported them, and + a capitalised variant would fall through to unknown for no reason anyone + chose. + """ + lowered = model.lower() + for vendor, markers in _VENDOR_MARKERS.items(): + if any(marker in lowered for marker in markers): + return vendor + return UNKNOWN_VENDOR + + +def pairing_kind(finder: str, arm: str) -> str: + """How a (finder, skeptic) pair relates: `same`, `cross`, or `unknown`. + + Its own function rather than a conditional inside the report loop, because + it is the experiment's independent variable and deserves to be read and + tested on its own (raised by SonarCloud on #411 as a nested conditional). + + `unknown` is never folded into `cross`. For a model nobody has attributed, + same-vs-cross is not merely unmeasured but unknowable, and answering `cross` + would be inventing the very thing under test — which is exactly what the + previous version did, silently, to every unrecognised model. + """ + if finder == UNKNOWN_VENDOR: + return "unknown" + return "same" if finder == arm else "cross" + + +def report(rows: list[dict[str, Any]]) -> None: + """The comparison the issue asks for, split by which vendor found the findings.""" + print(f"\n{len(rows)} (pass, arm) results\n") + print( + f"{'finder':9} {'skeptic':9} {'kind':7} {'passes':>6} {'refuted':>8} " + f"{'of':>5} {'killed GT':>10}" + ) + groups: dict[tuple[str, str], list[dict[str, Any]]] = defaultdict(list) + for row in rows: + groups[(_vendor(str(row["finder_model"])), str(row["arm"]))].append(row) + for (finder, arm), group in sorted(groups.items()): + kind = pairing_kind(finder, arm) + refuted = sum(int(r["refuted"]) for r in group) + total = sum(int(r["findings"]) for r in group) + killed = sum(len(r["killed_gt"]) for r in group) + print(f"{finder:9} {arm:9} {kind:7} {len(group):6} {refuted:8} {total:5} {killed:10}") + unattributed = sorted( + {str(r["finder_model"]) for r in rows if _vendor(str(r["finder_model"])) == UNKNOWN_VENDOR} + ) + if unattributed: + print( + f"\n{len(unattributed)} model name(s) match no vendor and are excluded from the " + f"same/cross split: {', '.join(unattributed)}. Add a marker to _VENDOR_MARKERS." + ) + print( + "\n'killed GT' is ground truth the finder matched and the skeptic then hid — the " + "recall cost, which `missed` cannot show." + ) + + +def main() -> int: + """Run both arms and write the rows.""" + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--out", type=Path, default=REPO_ROOT / ".guardian-arms.jsonl") + parser.add_argument("--repo-root", type=Path, default=REPO_ROOT) + parser.add_argument( + "--limit", type=int, default=None, help="judge only the first N passes (a smoke run)" + ) + args = parser.parse_args() + rows = asyncio.run(collect_rows(args.repo_root, args.limit)) + args.out.parent.mkdir(parents=True, exist_ok=True) + args.out.write_text("".join(json.dumps(r) + "\n" for r in rows), encoding="utf-8") + report(rows) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/unit/test_skeptic_arms.py b/tests/unit/test_skeptic_arms.py new file mode 100644 index 00000000..ff20fb21 --- /dev/null +++ b/tests/unit/test_skeptic_arms.py @@ -0,0 +1,355 @@ +"""Both arms over one frozen pass, with the model calls stubbed (#246). + +The experiment itself needs two providers; these tests need none. What they +cover is everything that decides whether the numbers mean anything: which +skeptic each arm builds, that neither arm can inherit the other's model, and +that a same/cross classification is derived from the finder rather than assumed. +""" + +import json +import sys +from pathlib import Path +from typing import Any + +import pytest +from guardian_stubs import StubProvider + +from cgis.guardian.bench import load_ground_truth + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent.parent / "scripts")) + +import skeptic_arms +from skeptic_arms import ARMS, NoArmError, _vendor, arm_provider, finder_models, judge_one, report + +REPO_ROOT = Path(__file__).resolve().parent.parent.parent +BENCH_DIR = REPO_ROOT / "benchmarks" / "guardian" + + +class TestBuildingAnArm: + """Which provider each arm gets, and what it must not inherit.""" + + def test_a_missing_provider_refuses_before_any_call( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Half an experiment costs the same as none and reads like a result. + + This refusal is not hypothetical: it is what stopped the first real run + after 2 calls instead of 135, when the mistral key returned HTTP 402. + """ + monkeypatch.setattr(skeptic_arms, "build_skeptic_provider", lambda _e, **_k: None) + with pytest.raises(NoArmError, match="answers nothing this experiment asks"): + arm_provider("mistral", {}) + + def test_the_arm_name_is_what_selects_the_provider( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + seen: list[str] = [] + + def _capture(env: dict[str, str], **_kw: object) -> tuple[StubProvider, str]: + seen.append(env["GUARDIAN_SKEPTIC"]) + return StubProvider([]), "m" + + monkeypatch.setattr(skeptic_arms, "build_skeptic_provider", _capture) + for name in ARMS: + arm_provider(name, {}) + assert seen == list(ARMS) + + def test_a_skeptic_model_in_the_environment_reaches_neither_arm( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """The cross-arm contamination that would have run one arm on a fake model. + + `build_skeptic_provider` applies `GUARDIAN_SKEPTIC_MODEL` to whichever + provider it builds. An environment holding the production value + (`gemini-2.5-flash`) would hand that name to the mistral arm, so one arm + would run a model that does not exist while the other ran the intended + one — and the comparison would be between a working skeptic and a broken + one, reported as a comparison between vendors. + """ + envs: list[dict[str, str]] = [] + + def _capture(env: dict[str, str], **_kw: object) -> tuple[StubProvider, str]: + envs.append(dict(env)) + return StubProvider([]), "m" + + monkeypatch.setattr(skeptic_arms, "build_skeptic_provider", _capture) + arm_provider("mistral", {"GUARDIAN_SKEPTIC_MODEL": "gemini-2.5-flash", "X": "keep"}) + assert "GUARDIAN_SKEPTIC_MODEL" not in envs[0] + assert envs[0]["X"] == "keep" + + +class TestTheVendorSplit: + """Both orientations are in the corpus, and that is what the split rests on.""" + + @pytest.mark.parametrize( + ("model", "vendor"), + [ + ("gemini-2.5-flash", "gemini"), + ("gemini-3.5-flash", "gemini"), + ("mistral-medium-latest", "mistral"), + # Mistral families whose names do not contain "mistral". codestral + # was the arm proposed in #246's first draft, so this is not + # hypothetical. Raised in review of #411. + ("codestral-latest", "mistral"), + ("ministral-8b-latest", "mistral"), + ("Gemini-2.5-Flash", "gemini"), + ("gpt-5", "unknown"), + ("", "unknown"), + ], + ) + def test_a_model_name_maps_to_its_vendor(self, model: str, vendor: str) -> None: + assert _vendor(model) == vendor + + def test_the_corpus_really_carries_both_orientations(self) -> None: + """The premise of the whole design, asserted rather than assumed. + + If every frozen pass came from one vendor there would be one + orientation, and the experiment could not tell "cross-vendor refutes + more" from "gemini refutes more" — which is the distinction #246's + original single-orientation design could not make. + """ + vendors = {_vendor(m) for m in finder_models(BENCH_DIR / "results.jsonl").values()} + assert {"gemini", "mistral"} <= vendors + + def test_only_frozen_passes_are_mapped(self, tmp_path: Path) -> None: + """A judged row's findings carry rewritten confidences (#279), so it is not a pass.""" + rows = [ + { + "pr": 1, + "timestamp": "t1", + "matched": [], + "precision": 1.0, + "model": "m", + "findings": [], + }, + {"pr": 2, "timestamp": "t2", "matched": [], "precision": 1.0, "skeptic_model": "s"}, + ] + path = tmp_path / "results.jsonl" + path.write_text("".join(json.dumps(r) + "\n" for r in rows), encoding="utf-8") + assert finder_models(path) == {"1@t1": "m"} + + +@pytest.mark.asyncio +async def test_judge_one_scores_a_pass_and_reports_what_the_skeptic_killed( + tmp_path: Path, +) -> None: + """The row a single (pass, arm) produces, including the recall guard. + + `killed_gt` is carried because `missed` cannot tell "the finder never found + it" from "the finder found it and the skeptic hid it" (#270), and only the + second is something an arm did. + """ + recording = json.loads( + (BENCH_DIR / "experiments" / "401-evidence" / "pr-399-recording.json").read_text( + encoding="utf-8" + ) + ) + findings = recording["result"]["findings"][:2] + for finding in findings: + finding.pop("verdict", None) + finding.pop("skeptic_note", None) + path = tmp_path / "rec.json" + path.write_text( + json.dumps({"result": {"findings": findings, "summary": ""}, "diff": recording["diff"]}), + encoding="utf-8", + ) + truth = load_ground_truth(BENCH_DIR / "pr-143.yaml") + provider = StubProvider( + ['{"verdict": "refuted", "impact_score": 0, "rationale": "r"}'] * len(findings) + ) + + row = await judge_one(provider, path, truth, None) + + assert row["findings"] == len(findings) + assert row["refuted"] == len(findings) + assert row["confirmed"] == 0 + assert isinstance(row["killed_gt"], list) + + +class _Rec: + """A recording stand-in: findings and the diff they were found in.""" + + def __init__(self, findings: list[object]) -> None: + self.result = type("R", (), {"findings": findings})() + self.diff = "diff --git a/x.py b/x.py\n" + + +class TestTheRunItself: + """Orchestration, with every call stubbed: what runs, how often, and on what.""" + + def _wire( + self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path, passes: list[str] + ) -> list[str]: + """Stub every outside edge; return the list evidence collection appends to.""" + paths = [tmp_path / f"{name}.json" for name in passes] + for path in paths: + path.write_text("{}", encoding="utf-8") + collected: list[str] = [] + monkeypatch.setattr( + skeptic_arms, "arm_provider", lambda name, _env: (StubProvider([]), f"{name}-model") + ) + monkeypatch.setattr(skeptic_arms, "finder_models", lambda _p: {}) + monkeypatch.setattr(skeptic_arms, "build", lambda *_a: paths) + monkeypatch.setattr(skeptic_arms, "load_finder_recording", lambda _p: _Rec(["f"])) + monkeypatch.setattr(skeptic_arms, "load_ground_truth", lambda _p: object()) + + def _evidence(_truth: object, _diff: str, _root: Path) -> object: + collected.append("once") + return object() + + monkeypatch.setattr(skeptic_arms, "evidence_for_pr", _evidence) + + async def _judge(*_a: object, **_k: object) -> dict[str, Any]: + return {"findings": 1, "refuted": 0, "killed_gt": []} + + monkeypatch.setattr(skeptic_arms, "judge_one", _judge) + return collected + + @pytest.mark.asyncio + async def test_evidence_is_collected_once_per_pr_not_once_per_pass( + self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path + ) -> None: + """Three passes of one PR share a diff, so the checkers would say the same thing. + + Per pass it would be three worktrees and three `uv sync`s for one answer. + Asserted by counting, because a per-pass version returns identical rows + and differs only in how long it takes — invisible in the output. + """ + collected = self._wire(monkeypatch, tmp_path, ["143@a", "143@b", "143@c"]) + rows = await skeptic_arms.collect_rows(REPO_ROOT, None) + assert len(collected) == 1 + assert len(rows) == 3 * len(ARMS) + + @pytest.mark.asyncio + async def test_each_pass_is_judged_by_every_arm( + self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path + ) -> None: + """Pairing is the whole design: the same findings, both skeptics.""" + self._wire(monkeypatch, tmp_path, ["143@a", "144@b"]) + rows = await skeptic_arms.collect_rows(REPO_ROOT, None) + by_pass: dict[str, set[str]] = {} + for row in rows: + by_pass.setdefault(str(row["pass"]), set()).add(str(row["arm"])) + assert by_pass == {"143@a": set(ARMS), "144@b": set(ARMS)} + + @pytest.mark.asyncio + async def test_limit_trims_the_work_before_a_worktree_is_built( + self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path + ) -> None: + """`--limit` is the smoke run; it must cost one PR, not all of them.""" + collected = self._wire(monkeypatch, tmp_path, ["143@a", "144@b"]) + rows = await skeptic_arms.collect_rows(REPO_ROOT, 1) + assert len(collected) == 1 + assert len(rows) == len(ARMS) + + @pytest.mark.asyncio + async def test_a_pass_with_no_findings_is_dropped_before_any_worktree( + self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path + ) -> None: + """Nothing to judge means nothing to compare, and a worktree costs a uv sync.""" + self._wire(monkeypatch, tmp_path, ["143@a"]) + monkeypatch.setattr(skeptic_arms, "load_finder_recording", lambda _p: _Rec([])) + collected: list[str] = [] + monkeypatch.setattr( + skeptic_arms, "evidence_for_pr", lambda *_a: collected.append("x") or object() + ) + rows = await skeptic_arms.collect_rows(REPO_ROOT, None) + assert rows == [] + assert collected == [] + + +def test_main_writes_the_rows_it_collected( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """The CLI wiring, and the reason the write is not inside the coroutine. + + `collect_rows` returns rows and `main` writes them, so the blocking file + write never happens on the event loop — the same rule the worktree + collection follows with `to_thread`. + """ + row = { + "finder_model": "gemini-2.5-flash", + "arm": "mistral", + "refuted": 1, + "findings": 2, + "killed_gt": [], + } + + async def _rows(_root: Path, _limit: int | None) -> list[dict[str, Any]]: + return [row] + + out = tmp_path / "nested" / "arms.jsonl" + monkeypatch.setattr(skeptic_arms, "collect_rows", _rows) + monkeypatch.setattr(sys, "argv", ["skeptic_arms.py", "--out", str(out)]) + assert skeptic_arms.main() == 0 + assert [json.loads(line) for line in out.read_text(encoding="utf-8").splitlines()] == [row] + assert "cross" in capsys.readouterr().out + + +def test_report_labels_same_and_cross_from_the_finder(capsys: pytest.CaptureFixture[str]) -> None: + """The label is derived, not stored — an arm is 'same' only against its own vendor.""" + rows: list[dict[str, Any]] = [ + { + "finder_model": "mistral-medium-latest", + "arm": "gemini", + "refuted": 3, + "findings": 5, + "killed_gt": [], + }, + { + "finder_model": "mistral-medium-latest", + "arm": "mistral", + "refuted": 0, + "findings": 5, + "killed_gt": ["a"], + }, + ] + report(rows) + out = capsys.readouterr().out + assert "cross" in out + assert "same" in out + + +@pytest.mark.parametrize( + ("finder", "arm", "kind"), + [ + ("gemini", "gemini", "same"), + ("mistral", "mistral", "same"), + ("gemini", "mistral", "cross"), + ("mistral", "gemini", "cross"), + # Both orderings, because `unknown` must not depend on which arm it met. + ("unknown", "gemini", "unknown"), + ("unknown", "mistral", "unknown"), + ], +) +def test_the_independent_variable_is_classified_on_its_own( + finder: str, arm: str, kind: str +) -> None: + """same / cross / unknown, read and checked apart from the report that prints it.""" + assert skeptic_arms.pairing_kind(finder, arm) == kind + + +def test_an_unattributed_finder_is_never_reported_as_cross( + capsys: pytest.CaptureFixture[str], +) -> None: + """The silent failure behind the codestral case, and the general answer to it. + + An unrecognised model used to become "other"; "other" equals no arm name, so + every one of its rows read `cross` — in the exact variable this experiment + measures. A new vendor would not have raised anything; it would have + produced a wrong answer shaped like the expected one. So `unknown` is its + own kind and the model is named, because the fix is a one-line marker and + nobody can apply it to a number they never saw. + """ + rows: list[dict[str, Any]] = [ + {"finder_model": "gpt-5", "arm": "gemini", "refuted": 1, "findings": 4, "killed_gt": []} + ] + report(rows) + out = capsys.readouterr().out + # The table row, not the whole blob: the sentence naming the omission says + # "same/cross split", so a substring check over the output would pass on the + # explanation while the row said `cross`. + row_line = next(line for line in out.splitlines() if line.startswith("unknown")) + assert row_line.split()[:3] == ["unknown", "gemini", "unknown"] + assert "gpt-5" in out + assert "_VENDOR_MARKERS" in out