diff --git a/dokimi/changelog.d/20261026_120000_antonwolfy_cross-judge-verdict-shape.md b/dokimi/changelog.d/20261026_120000_antonwolfy_cross-judge-verdict-shape.md new file mode 100644 index 000000000..e9af067ea --- /dev/null +++ b/dokimi/changelog.d/20261026_120000_antonwolfy_cross-judge-verdict-shape.md @@ -0,0 +1,11 @@ +### Fixed + +- **`eval cross-judge` exits 2 on a `--pairwise` or `--checklist` entry of the wrong shape, + before any judge call** — a verdict whose `score` is not a finite number (missing, null, + a string, a bool, NaN, infinite or past a float's range), an arm that is not an object of + verdicts, or a checklist question without a `question_id` (or with a null or bool one), + without an `arms` object, or with a `coverage` that is neither a finite number nor null + now prints `Error: : ...` and exits 2 instead of ending in a Python + traceback or passing a bad value on. The entries are read with the file, before + `--judges` makes its first paid call; until now a wrong shape ended the run after those + calls. diff --git a/dokimi/dokimi/commands/judge_reliability.py b/dokimi/dokimi/commands/judge_reliability.py index 762da883b..5eb95837e 100644 --- a/dokimi/dokimi/commands/judge_reliability.py +++ b/dokimi/dokimi/commands/judge_reliability.py @@ -51,6 +51,19 @@ def _judge_name(doc: dict, path: str) -> str: return judge if isinstance(judge, str) and judge else Path(path).stem +def _result_measure(path: str, key: str, kind: type, build): + """The measure an `eval pairwise` or `eval checklist` file gives, or exit 2 naming the file. + + *build* is ``(doc, name) -> Measure``; it raises ValueError on an entry of the wrong shape. + """ + doc = _load_json_object(path, key, kind) + try: + return build(doc, _judge_name(doc, path)) + except ValueError as exc: + print(f"Error: {path}: {exc}", file=sys.stderr) + sys.exit(2) + + def _load_answers(path: str): """The parsed file and its answers records (dokimi/records.py), or a clean exit.""" from dokimi.records import RecordError, read @@ -361,9 +374,10 @@ def score(library, question, answer, ground_truth, context): sys.exit(2) report.measures.append(m) - # Read every result file before the first judge call, so a bad path costs no paid calls. - pairwise = [(path, _load_json_object(path, "verdicts")) for path in args.pairwise or []] - checklists = [(path, _load_json_object(path, "questions", list)) for path in args.checklist or []] + # Read every result file, and its entries, before the first judge call, so a bad file costs no paid calls. + from_files = [_result_measure(p, "verdicts", dict, lambda doc, name: pairwise_measure(doc, name, baseline)) + for p in args.pairwise or []] + from_files += [_result_measure(p, "questions", list, coverage_measure) for p in args.checklist or []] validity = [_load_json_object(v, "summary")["summary"] for v in args.validity or []] specs = [j.strip() for j in (args.judges or "").split(",") if j.strip()] if specs: @@ -385,10 +399,7 @@ def score(library, question, answer, ground_truth, context): concurrency=args.concurrency, progress=_progress(), ground_truth=grounding, store=materials, )) - for path, pw in pairwise: - report.measures.append(pairwise_measure(pw, _judge_name(pw, path), baseline)) - for path, ck in checklists: - report.measures.append(coverage_measure(ck, _judge_name(ck, path))) + report.measures += from_files answers_model = data.get("model") if isinstance(data, dict) else None for v in validity: if v.get("answers_model") and answers_model and v["answers_model"] != answers_model: diff --git a/dokimi/dokimi/eval/cross_judge.py b/dokimi/dokimi/eval/cross_judge.py index df9c18ae2..967de4a00 100644 --- a/dokimi/dokimi/eval/cross_judge.py +++ b/dokimi/dokimi/eval/cross_judge.py @@ -46,6 +46,7 @@ from __future__ import annotations +import math import re from concurrent.futures import ThreadPoolExecutor from dataclasses import dataclass, field @@ -229,20 +230,55 @@ def stored_measure(data: Dict[str, Any], name: Optional[str] = None) -> Optional return m if m.scores else None +def _number(x: Any) -> float: + """A score or coverage from a result file: a finite JSON number, not a bool or a string. + + Raises TypeError, ValueError, or OverflowError for an integer past a float's range.""" + if isinstance(x, bool) or not isinstance(x, (int, float)): + raise TypeError(x) + value = float(x) + if not math.isfinite(value): + raise ValueError(x) + return value + + def coverage_measure(checklist: Dict[str, Any], name: str) -> Measure: + """An `eval checklist` report as a measure: each arm scores its coverage. + + Raises ValueError naming the first question that is not ``{question_id, arms: {arm: + {coverage}}}`` with a finite number or null as the coverage. + """ m = Measure(name=name, kind="coverage") - for q in checklist.get("questions") or []: - for arm, v in (q.get("arms") or {}).items(): - m.scores[(q["question_id"], arm)] = v.get("coverage") + for i, q in enumerate(checklist.get("questions") or []): + try: + qid, arms = q["question_id"], q["arms"] + if qid is None or isinstance(qid, bool): + raise TypeError(qid) + for arm, v in arms.items(): + coverage = v.get("coverage") + m.scores[(qid, arm)] = None if coverage is None else _number(coverage) + except (AttributeError, KeyError, TypeError, ValueError, OverflowError): + raise ValueError(f"questions[{i}] is not a checklist question: a question_id and an arms " + "object of {arm: {coverage: a finite number or null}}") from None return m def pairwise_measure(pairwise: Dict[str, Any], name: str, baseline_arm: str) -> Measure: - """An `eval pairwise` report as a measure: the arm scores its verdict, the baseline 0.""" + """An `eval pairwise` report as a measure: the arm scores its verdict, the baseline 0. + + Raises ValueError naming the first verdict that is not an object with a finite number as + its ``score``. + """ m = Measure(name=name, kind="pairwise") for arm, by_q in (pairwise.get("verdicts") or {}).items(): + if not isinstance(by_q, dict): + raise ValueError(f"verdicts.{arm} is not an object of per-question verdicts") for qid, v in by_q.items(): - m.scores[(qid, arm)] = float(v["score"]) + try: + score = _number(v["score"]) + except (KeyError, TypeError, ValueError, OverflowError): + raise ValueError(f"verdicts.{arm}.{qid} has no score that is a finite number") from None + m.scores[(qid, arm)] = score m.scores[(qid, baseline_arm)] = 0.0 return m diff --git a/dokimi/scripts/growth_baseline.json b/dokimi/scripts/growth_baseline.json index d0d35275e..631cc9a53 100644 --- a/dokimi/scripts/growth_baseline.json +++ b/dokimi/scripts/growth_baseline.json @@ -22,7 +22,7 @@ "dokimi/commands/benchmark.py::cmd_benchmark_preflight": {"complexity": 22}, "dokimi/commands/catalog.py::cmd_catalog_trigger_set": {"complexity": 16}, "dokimi/commands/evaluate.py::cmd_eval_judge_validity": {"complexity": 29, "rationale": "grew on main in #73 (judge-validity --context stored) from 25 to 29 before the ratchet landed; split it in its own PR"}, - "dokimi/commands/judge_reliability.py::cmd_eval_cross_judge": {"complexity": 32, "rationale": "pre-existing when the gate was introduced"}, + "dokimi/commands/judge_reliability.py::cmd_eval_cross_judge": {"complexity": 30, "rationale": "pre-existing when the gate was introduced"}, "dokimi/commands/novelty.py::cmd_novelty_routing": {"complexity": 13, "rationale": "W8 adds the --repeats guard after main's no_queries handling raised the file; the command is split with the rest of novelty.py in a separate PR"}, "dokimi/commands/repo.py::cmd_repo_scan": {"complexity": 13}, "dokimi/commands/repo.py::cmd_repo_validate": {"complexity": 11, "rationale": "grew on main in #150 (repo validate --fail-on-warning) from 10 to 11 before the ratchet landed; split it in its own PR"}, diff --git a/dokimi/tests/test_record_readers.py b/dokimi/tests/test_record_readers.py index b67d45160..c6948d8a6 100644 --- a/dokimi/tests/test_record_readers.py +++ b/dokimi/tests/test_record_readers.py @@ -659,3 +659,82 @@ def test_cross_judge_names_a_result_file_measure_after_its_summary_judge(tmp_pat cli.main() md = out.with_suffix(".md").read_text(encoding="utf-8") assert " · judge-a |" in md and " · result |" not in md + + +_GOOD_QUESTION = {"question_id": "q0", "arms": {"skill": {"coverage": 1}}} + + +def _pairwise(verdict): + return {"verdicts": {"skill": {"q1": verdict}}} + + +def _checklist(question): + return {"questions": [question]} + + +@pytest.mark.parametrize("flag, content, where", [ + pytest.param("--pairwise", _pairwise({}), "verdicts.skill.q1", id="no-score"), + pytest.param("--pairwise", _pairwise(5), "verdicts.skill.q1", id="verdict-not-an-object"), + pytest.param("--pairwise", _pairwise({"score": "x"}), "verdicts.skill.q1", id="score-not-a-number"), + pytest.param("--pairwise", _pairwise({"score": "1"}), "verdicts.skill.q1", id="score-a-numeric-string"), + pytest.param("--pairwise", _pairwise({"score": None}), "verdicts.skill.q1", id="null-score"), + pytest.param("--pairwise", _pairwise({"score": True}), "verdicts.skill.q1", id="bool-score"), + pytest.param("--pairwise", _pairwise({"score": float("nan")}), "verdicts.skill.q1", id="nan-score"), + pytest.param("--pairwise", _pairwise({"score": float("inf")}), "verdicts.skill.q1", id="infinite-score"), + pytest.param("--pairwise", _pairwise({"score": 10 ** 400}), "verdicts.skill.q1", id="score-past-float-range"), + pytest.param("--pairwise", {"verdicts": {"skill": 5}}, "verdicts.skill", id="arm-not-an-object"), + pytest.param("--checklist", {"questions": [5]}, "questions[0]", id="question-not-an-object"), + pytest.param("--checklist", {"questions": [_GOOD_QUESTION, {"arms": {"skill": {"coverage": 1}}}]}, + "questions[1]", id="no-question-id"), + pytest.param("--checklist", _checklist({"question_id": None, "arms": {"skill": {"coverage": 1}}}), + "questions[0]", id="null-question-id"), + pytest.param("--checklist", _checklist({"question_id": True, "arms": {"skill": {"coverage": 1}}}), + "questions[0]", id="bool-question-id"), + pytest.param("--checklist", _checklist({"question_id": ["q1"], "arms": {"skill": {"coverage": 1}}}), + "questions[0]", id="unhashable-question-id"), + pytest.param("--checklist", _checklist({"question_id": "q1"}), "questions[0]", id="no-arms"), + pytest.param("--checklist", _checklist({"question_id": "q1", "arms": None}), "questions[0]", id="null-arms"), + pytest.param("--checklist", _checklist({"question_id": "q1", "arms": [1]}), "questions[0]", id="arms-not-an-object"), + pytest.param("--checklist", _checklist({"question_id": "q1", "arms": {"skill": 5}}), "questions[0]", + id="arm-entry-not-an-object"), + pytest.param("--checklist", _checklist({"question_id": "q1", "arms": {"skill": {"coverage": "x"}}}), + "questions[0]", id="coverage-not-a-number"), + pytest.param("--checklist", _checklist({"question_id": "q1", "arms": {"skill": {"coverage": True}}}), + "questions[0]", id="bool-coverage"), + pytest.param("--checklist", _checklist({"question_id": "q1", "arms": {"skill": {"coverage": float("nan")}}}), + "questions[0]", id="nan-coverage"), + pytest.param("--checklist", _checklist({"question_id": "q1", "arms": {"skill": {"coverage": 10 ** 400}}}), + "questions[0]", id="coverage-past-float-range"), +]) +def test_cross_judge_names_a_result_entry_of_the_wrong_shape_before_any_judge_call( + tmp_path, capsys, monkeypatch, flag, content, where): + from tests.test_judge_reliability import _cross_judge_args, _material_arms_run, _recording_judge + + seen: dict = {} + _recording_judge(monkeypatch, seen) + wrong = tmp_path / "wrong.json" + wrong.write_text(json.dumps(content), encoding="utf-8") + args = _cross_judge_args(tmp_path, _material_arms_run(), "--no-stored", "--judges", "amazon-bedrock/m", + flag, str(wrong)) + with pytest.raises(SystemExit) as exc: + args.func(args) + assert exc.value.code == 2 + assert capsys.readouterr().err.startswith(f"Error: {wrong}: {where} ") + assert seen == {} # no paid judge call was made + + +def test_cross_judge_takes_a_checklist_arm_with_a_null_coverage(tmp_path, monkeypatch): + """`eval checklist` writes a null coverage for an arm with no scored point.""" + from dokimi import cli + + arms = tmp_path / "arms.json" + arms.write_text(json.dumps({"answers": [{"question_id": "q1", "arms": {"skill": {"answer": "s"}}}]}), + encoding="utf-8") + result = tmp_path / "result.json" + result.write_text(json.dumps(_checklist({"question_id": "q1", "arms": {"skill": {"coverage": None}}})), + encoding="utf-8") + out = tmp_path / "out.json" + monkeypatch.setattr("sys.argv", ["dokimi", "eval", "cross-judge", "--answers", str(arms), "--no-stored", + "--checklist", str(result), "--output", str(out)]) + cli.main() + assert " · result |" in out.with_suffix(".md").read_text(encoding="utf-8")