[napetrov]

Summary

Follow-up promised in the review of #80 (thread #80 (comment), eval/judge.py:297): @antonwolfy noted that the judge paths disagree on a record whose question text is missing or empty. On main:

• Judge.evaluate_answers: a missing question_text key raised KeyError inside the per-answer try, so it became a per-question error row; an empty string went to the judge.
• degrade._answer_pool (judge-validity / judge-wording pairs): skipped the record silently.
• JudgePanel.evaluate_answers, ArmRunner.judge, run_arm_pairwise, run_checklist, score_measure: sent "" to the judge.

Rule chosen: refuse. Every path checks all its records before the first judge call and raises EmptyQuestionError (a ValueError) naming the record (question 'q2', or record #N without an id). The CLI prints Error: … and exits 1, no traceback.

Changes

• New dokimi/eval/question_text.py: require_question_text(record, index) / require_question_texts(records) and EmptyQuestionError. Accepts question_text, then question/text (the spellings the panel already accepted); blank/whitespace counts as empty.
• Applied in judge.py, panel.py, arm_runner.py (judge), pairwise.py, checklist.py, cross_judge.py (score_measure), degrade.py (refuses instead of skipping).
• dokimi/cli.py main() catches EmptyQuestionError → Error: …, exit 1.
• Changelog fragment changelog.d/20261016_190000_napetrov_empty-question-rule.md. No doc described the old behavior, so no doc change.

Risk

• A run over an answers file with a question-less record now stops up front instead of producing partial or meaningless scores. Answer files written by the framework always carry question_text.
• Overlap with refactor(eval): give answer and eval records one reader (eval/records.py) #80: refactor(eval): give answer and eval records one reader (eval/records.py) #80 adds its own RecordError in evaluate_answers. The helper here is small and self-contained; on rebase, refactor(eval): give answer and eval records one reader (eval/records.py) #80's check can call require_question_text (or EmptyQuestionError can subclass RecordError). Expect a small conflict around judge.py:297.

Testing

• New tests/test_empty_question_rule.py (29 tests): helper; each path with missing / "" / whitespace question, asserting the judge is never called; question still passed through; CLI eval pairwise exits 1 with Error:.
• python -m pytest -q: 4742 passed, 1 failure (sandbox-only IPv6 test).
• ruff check . clean; mypy: only the 2 known mcp_protocol.py errors (sandbox); config_check, schema_check pass.

Checklist

• Tests added/updated and pytest passes
• ruff, mypy, and schema checks pass
• Docs updated (or N/A) — N/A, no doc described this behavior
• No ad-hoc run artifacts committed
• New files carry the SPDX license header (or are exempt in .github/.licenserc.yaml)

🤖 Generated with Claude Code

https://claude.ai/code/session_01RW8mAicb3ibXdzHMV83wDn

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 97.11538% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.19%. Comparing base (9753cc7) to head (4794af9).

Files with missing lines	Patch %	Lines
dokimi/dokimi/commands/arms.py	75.00%	2 Missing ⚠️
dokimi/dokimi/eval/checklist.py	94.73%	0 Missing and 1 partial ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #81      +/-   ##
==========================================
+ Coverage   90.06%   90.19%   +0.13%     
==========================================
  Files         205      206       +1     
  Lines       24193    24275      +82     
  Branches     3892     3904      +12     
==========================================
+ Hits        21790    21896     +106     
+ Misses       1724     1708      -16     
+ Partials      679      671       -8     

Files with missing lines	Coverage Δ
dokimi/dokimi/commands/evaluate.py	76.22% <100.00%> (+2.86%)	⬆️
dokimi/dokimi/commands/judge_reliability.py	86.94% <100.00%> (+0.18%)	⬆️
dokimi/dokimi/eval/arm_runner.py	92.50% <100.00%> (+0.08%)	⬆️
dokimi/dokimi/eval/cross_judge.py	94.57% <100.00%> (+0.09%)	⬆️
dokimi/dokimi/eval/degrade.py	97.83% <100.00%> (+1.14%)	⬆️
dokimi/dokimi/eval/judge.py	94.90% <100.00%> (+1.13%)	⬆️
dokimi/dokimi/eval/pairwise.py	100.00% <100.00%> (+1.68%)	⬆️
dokimi/dokimi/eval/panel.py	97.19% <100.00%> (+0.02%)	⬆️
dokimi/dokimi/eval/question_text.py	100.00% <100.00%> (ø)
dokimi/dokimi/eval/ragas_eval.py	70.34% <100.00%> (+3.44%)	⬆️
... and 2 more

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[xaleryb]

Every judge path the fragment lists refuses before its first call, and each has a test that fails without the guard.

One gap: arms run --judge with a whitespace-only question ("question": " ", which load_questions accepts) answered every question on every arm, and only then did the judge refuse. The command exited 1 without writing its output, so the paid answers were lost; arms matrix-run behaved the same per cell. Pushed 6140ec4: _build_arms_output checks the questions before the sampling probe, with an end-to-end CLI test.

Not covered, and not claimed by the fragment: RAGAS (ragas_eval.py) still sends an empty question. This PR conflicts with #80 in seven eval modules, so whichever lands second re-applies the guard.

[xaleryb]

Approved at 6140ec4. Every listed judge path refuses a blank question before its first call, each with a test that fails without the guard; full suite on Linux 5041 passed. Whichever of this PR and #80 lands second re-applies the guard in the seven shared eval modules.

[antonwolfy]

Review at 6140ec4. The guard itself is right: all seven listed paths refuse before their first judge call, the new tests fail without the fix, and the behaviour is consistent across paths. Three things are worth fixing before merge: one judge path still sends an empty question, the up-front check refuses records that would never be judged, and the exit code collides with the verdict code of eval judge-validity.

What I ran, on a Linux review host at 6140ec4:

• make ci-local: 19 passed, 0 failed, 4 skipped; 5041 tests; changed-line coverage 100%.
• Reproductions for the code findings, with the LLM transport mocked.
• A mutation run over the new code and the new tests: 39 mutants, 17 of which pass the new tests plus 23 related test files (1179 tests). The survivors are noted in the findings below.

Findings that cannot be anchored inline:

1. RAGAS still scores an empty question, so the title's "every judge path" is not true yet. I agree with @xaleryb that this path is outside the fragment's list. One detail matters for the fix: _run_ragas_eval (commands/evaluate.py:66) wraps the call in except Exception and prints "RAGAS evaluation failed", so a guard added at eval/ragas_eval.py:112 would be reduced to a warning unless that wrapper lets EmptyQuestionError through. Suggestion: add the guard there in this PR, or narrow the title and the Summary to the listed paths.

2. Nit: eval pairwise, eval checklist and eval cross-judge --judges make a billed probe call before the refusal. With --judge-temperature 0.2 and a blank record, _check_judge_temperature (commands/judge_reliability.py:71, :120, :296) calls _probe_sampling first. Output: Error: question 'q2' has no question text ..., exit 1, probe calls before the refusal: 1. The cost is one 16-token call, but it contradicts "before the first judge call". arms run --judge avoids this because its check sits before _probe_sampling. The eval pairwise CLI test omits --judge-temperature, so it cannot see this.

Checked, no issues

• Order of checks: Judge.evaluate_answers, JudgePanel.evaluate_answers (after the limit slice) and ArmRunner.judge run the check before the pool starts. The re-checks inside except Exception (judge.py:313, arm_runner.py:458) cannot fire after it, so the refusal is never turned into a per-row error.
• Entry points: every command goes through dokimi.cli.main; the refusal is raised on the main thread, so the handler catches it. No traceback.
• Pins: the new tests fail against the base modules (23 of 31 fail; the 8 that pass are helper-only). Mutants that drop a guard in judge.py, panel.py, arm_runner.py, degrade.py or commands/arms.py, change the strip() check, drop the question/text fallbacks, or change the exit code or stream in cli.py are all killed.
• Repo rules: SPDX headers present, changelog fragment follows changelog.d/README.md, no schema or with_docs/without_docs change, no nav change, tests offline.

[antonwolfy]

The up-front check refuses records that would never reach a judge, so one bad record now blocks the whole file. The check runs over every record before any filtering. Before the PR these were skipped. Cases that now raise EmptyQuestionError (all reproduced, judge never called):

• run_arm_pairwise: the blank record has no baseline answer, or only arms excluded by arms=["skill"].
• run_checklist (checklist.py:283): the blank record has no expected points.
• score_measure (cross_judge.py:240): the blank record has only empty answers.
• build_pairs (degrade.py:603): the blank record has only an answer under the length floor, or arms: {}.
• Judge.evaluate_answers (judge.py:290): an answer-error record. answerer.py:168 builds it with q.get("question") or q.get("text", ""), which ignores question_text. With {"question_id": "q2", "question_text": "", "error": "timeout", "with_docs": None, "without_docs": None} plus one good record, the base returns rows for ['q1', 'q2'] after 2 judge calls. The PR head raises question 'q2' has no question text with 0 judge calls.

So one failed answer in a file blocks scoring of every other question.

Mutation check: deleting require_question_texts(...) from pairwise.py, checklist.py and cross_judge.py all pass the full related suite (1179 tests), because the per-record guard in the job loop still fires for every test record. No test pins which behaviour is intended.

Suggestion: decide the rule. If a blank question is only fatal when the record would be judged, drop the up-front call and check inside the job loops (still before the first judge call, since jobs are built first). If refusing everything is intended, say so in the fragment and add one test per path with a blank record that would otherwise be skipped.

[antonwolfy]

Exit 1 for a bad input record collides with the verdict code of the judge-reliability commands. eval judge-validity documents "Exit 1 on FAIL" (docs/benchmarking-and-comparison.md:422, help at commands/evaluate.py:461), and eval judge-wording --strict and eval cross-judge --strict use 1 for a failed gate. Their input errors use 2 (evaluate.py:258-329, judge_reliability.py:95, :133).

Reproduction: eval judge-validity --answers a.json with a record {"question_id": "q9", "arms": {"baseline": {"answer": "short"}}}:

Error: question 'q9' has no question text (question_text is missing or empty); ...
EXIT 1

CI that reads 1 as "the judge failed validity" now reports a malformed answers file as a failed gate.

Suggestion: exit 2 here, as the other input errors in those commands do. The test at test_empty_question_rule.py pins exit 1 for eval pairwise, so it needs the same change. If 1 is kept, the exit-code paragraph in the docs should say a malformed record exits 1 as well.

[antonwolfy]

The guard accepts the question and text spellings, but this method still reads rec["question_text"] two lines above, so such a record crashes after judge calls are paid. _score returns "question_text": rec["question_text"] at line 485. With a hand-built record that has only question: "How?", the check at 492 passes, score_answer is called once, then the call raises KeyError: 'question_text' and the whole judge() run fails. The refusal rule says "refuse before any call", so the guard and the reader must agree.

Suggestion: take the text once, e.g. text = require_question_text(rec), and use it for both the score_answer call and the returned row. The same applies to judge.py:313, where the error row now carries the text the guard found, but _evaluate_answer_pair was not exercised by a test on a question-only record (the Judge tests call evaluate_answers with a MagicMock as self).

[antonwolfy]

Nit: two small mismatches around this check.

• EmptyQuestionError is a ValueError, and cmd_arms_run (arms.py:84) and cmd_arms_matrix_run (arms.py:137) catch ValueError. The refusal prints Error building arms/plugins: question 'q2' has no question text ... (matrix: Error running matrix cell 'c1': ...), which blames arm setup for a bad input record. The handler in cli.main is never reached for these commands. The test only matches 'q2' in stderr, so it passes either way.
• The runner builds question_text as question or text or question_text (arm_runner.py:224, first truthy wins), while require_question_texts takes the first non-blank of question_text, question, text. For {"id": "q1", "question": " ", "question_text": "Real?"} the check passes, every arm is answered, the record gets question_text = ' ', and ArmRunner.judge refuses after the answers are paid for. This is the case this check was added for.

Suggestion: re-raise EmptyQuestionError ahead of the ValueError handlers, and let the runner use require_question_text to build the record's text.

[antonwolfy]

Nit: a record with no id is still skipped silently, so "refuses instead of skipping" and the record #N label do not apply on this path. The qid is None check above runs first, so {"question_text": " ", "arms": {...}} returns no pairs and no error (build_pairs(answers + [noid_blank]) returned without raising), and the record #N label can never come from here. Mutation check: removing the qid is None skip and changing i to i + 1 both pass the full suite, because every test record has an id. Suggestion: call require_question_text before the qid check, or say in the fragment that records without an id are still skipped.

[antonwolfy]

Nit: the docstring cites dokimi.eval.records.RecordError, which does not exist at this head (grep -rn RecordError dokimi/dokimi finds only this line), and the getattr(record, "question_text") and getattr(record, "question_id") branches (lines 37 and 48) are unreachable because every caller passes dicts. They are covered only with a SimpleNamespace. Suggestion: drop the branches and the reference until #80 lands, and let the second PR to land add them with the real class.