reviewer-eval: fail closed on malformed empty CLI selectors (CI review P1)

CI Codex flagged a fail-open input gap: malformed selectors silently ran a different
matrix than intended.
- _resolve_configs dropped empty comma segments, so "A,", ",A", "A,,B" resolved to a
  narrower-than-typed config set instead of erroring. It now rejects any empty segment.
- --strata is nargs="*", and load_cases treated [] as falsy ("no filter"), so a bare
  --strata (or empty shell expansion) silently ran the WHOLE corpus. load_cases now
  distinguishes None (no flag -> all strata) from [] (explicit empty -> match nothing),
  so a bare --strata fails closed (0 cases selected -> non-zero exit).

Tests (+2, 80 total): _resolve_configs rejects "A,"/",A"/"A,,B"/""/","/"A, ,B" and still
accepts A,B and A; load_cases(None) loads all, load_cases([]) selects nothing, and a
single stratum still filters. (Sanity: `run --configs A --strata` now exits "no cases
selected".)

Verified: ruff clean (changed), black clean, 80 eval tests pass, verify-corpus 2/2.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: igerber

tests/test_evals_runtime.py

@@ -1614,3 +1614,25 @@ def _fake_mat(case_id, fixture, repo_root, worktrees_root, case_dir="", worktree
     r2.prompt_sha_for(case)
     assert seen[0] != seen[1], "same case, different invocations -> distinct worktree keys"
     assert r1._wt_namespace in seen[0] and r2._wt_namespace in seen[1]
+
+
+def test_resolve_configs_rejects_empty_selectors():
+    """Malformed comma selectors must fail closed, not silently drop empty segments and
+    run a narrower matrix than intended."""
+    import run_eval
+
+    for bad in ("A,", ",A", "A,,B", "", ",", "A, ,B"):
+        assert run_eval._resolve_configs(bad) is None, f"{bad!r} must fail closed"
+    assert run_eval._resolve_configs("A,B") is not None, "valid A,B still resolves"
+    assert run_eval._resolve_configs("A") is not None, "valid single arm still resolves"
+
+
+def test_load_cases_distinguishes_none_from_empty_strata():
+    """No --strata (None) loads all; a bare --strata ([]) selects NOTHING (fail closed),
+    not the whole corpus."""
+    from adapters.corpus_loader import CorpusLoader
+
+    loader = CorpusLoader(str(_EVAL_ROOT / "corpus"), str(_REPO))
+    assert len(loader.load_cases(None)) >= 2, "None (no flag) loads all strata"
+    assert loader.load_cases([]) == [], "bare --strata ([]) must select nothing"
+    assert {c.id for c in loader.load_cases(["s3_negative"])} == {"s3-changelog-prose"}

tools/reviewer-eval/adapters/corpus_loader.py

@@ -137,7 +137,10 @@ def load_cases(self, strata: Optional[list[str]] = None) -> list[Case]:
         if not os.path.isdir(self.cases_dir):
             return cases
         for stratum in sorted(os.listdir(self.cases_dir)):
-            if strata and stratum not in strata:
+            # Distinguish None (no --strata -> all strata) from [] (a bare --strata with
+            # no values -> an explicit empty selection that must match NOTHING, so a typo
+            # / empty shell expansion fails closed instead of silently running everything).
+            if strata is not None and stratum not in strata:
                 continue
             stratum_dir = os.path.join(self.cases_dir, stratum)
             if not os.path.isdir(stratum_dir):

tools/reviewer-eval/run_eval.py

@@ -116,7 +116,11 @@ def _resolve_configs(arg: str):
     Fail-closed: a typo like ``--configs Z`` (or ``A,Z``) returns None so the
     caller can abort, rather than silently running a partial/empty matrix.
     """
-    requested = [c for c in (arg or "").split(",") if c]
+    requested = (arg or "").split(",")
+    # Reject malformed/empty selectors ("", "A,", ",A", "A,,B") rather than silently
+    # dropping empty segments and running a narrower matrix than the operator intended.
+    if any(c == "" for c in requested):
+        return None
     # Reject duplicate ids ("A,A"): run identity, artifacts, and bundle columns key
     # off config_id, so a repeat would alias both arms onto one identity and collapse
     # the comparison rather than running two arms.