[vchamarthi]

Summary

eval, report, answers, benchmark and evaluate read user-supplied files (questions, eval and run files, runtime config) and write --output without handling OS or decode errors, so a bad file or path ends in a traceback.

Changes

Each affected read or write now reports the path and reason in a one-line message and exits. One commit per command.

Risk

Error paths only.

Testing

make ci-local (offline), Python 3.13: everything passes except mypy (1 error) and the 10 tests that also fail on main (process, sandbox, container probes; macOS). Load-flaky tests pass alone.

Checklist

• Tests added/updated and pytest passes (apart from the 10 pre-existing failures)
• ruff, mypy, and schema checks pass (mypy: the one pre-existing error)
• Docs updated (or N/A): N/A
• No ad-hoc run artifacts committed
• New files carry the SPDX license header (or are exempt in .github/.licenserc.yaml)

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

3 open issues

• except Exception is too broad here and will also swallow unexpected programming errors (and… · New
• load_questions_payload appears unused in cmd_benchmark_preflight after switching to… · New
• This assertion relies on the OS-specific strerror text Not a directory, which can differ across… · New

What changed in this PR

Improves CLI robustness by converting file/path OS and decoding failures into concise, user-facing error messages with appropriate exit codes across eval, report, answers, benchmark, and evaluate.

Changes:

• Add/centralize error handling for reading user-supplied JSON/YAML inputs and writing --output / output dirs, exiting with one-line messages (no tracebacks).
• Refactor some repeated I/O logic into helpers (_load_eval, _write_json_or_exit, _check_questions_from, _load_points).
• Add targeted tests for unreadable/invalid inputs and unwritable outputs; update growth baseline.

File	Description
dokimi/​tests/​test_report_eval_input_errors.py	New coverage for report eval failing cleanly on bad --eval.
dokimi/​tests/​test_report_cli_inputs.py	Adds coverage for unwritable report generate --output.
dokimi/​tests/​test_model_compare.py	Adds coverage for load_run() non-UTF8 and directory-path handling.
dokimi/​tests/​test_evaluate_cli_inputs.py	New coverage for evaluate bad --custom-questions and --output-dir.
dokimi/​tests/​test_eval_questions_file_errors.py	New coverage for eval checklist/cross-judge --grounded bad --questions handling.
dokimi/​tests/​test_eval_grounding_output_errors.py	New coverage for eval grounding/judge-validity unwritable --output.
dokimi/​tests/​test_benchmark_command_question_set.py	Adds coverage ensuring benchmark run rejects unusable --questions-from before pipeline starts.
dokimi/​tests/​test_answers_command.py	Adds coverage for unreadable/non-UTF8 runtime config handling.
dokimi/​scripts/​growth_baseline.json	Updates complexity/line baselines impacted by refactors.
dokimi/​dokimi/​report/​model_compare.py	Makes load_run() emit clearer errors on read/decode failures.
dokimi/​dokimi/​report/​eval_report.py	Adds _load_eval() to validate/normalize eval JSON input errors.
dokimi/​dokimi/​commands/​report.py	Wraps report write path creation/write with OSError handling and exit 1.
dokimi/​dokimi/​commands/​orchestrate.py	Wraps EvaluationPipeline creation to catch input/output-dir OS failures.
dokimi/​dokimi/​commands/​judge_reliability.py	Adds _load_points() to ensure bad --questions exits 2 with a message.
dokimi/​dokimi/​commands/​evaluate.py	Adds _write_json_or_exit() and uses it for command outputs.
dokimi/​dokimi/​commands/​benchmark.py	Adds _check_questions_from() for preflight/run validation of --questions-from.
dokimi/​dokimi/​commands/​answers.py	Handles unreadable/non-UTF8 YAML config with clean exit 1 + message.
dokimi/​changelog.d/​20261025_120000_vchamarthi_cli-input-eval-report.md	Changelog entry for the improved CLI error reporting.

_{Review effort: Lite}

[codecov-commenter]

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

Codecov Report

❌ Patch coverage is 99.15254% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 92.92%. Comparing base (dc141f0) to head (eeebd1b).
⚠️ Report is 12 commits behind head on main.

Files with missing lines	Patch %	Lines
dokimi/dokimi/commands/benchmark.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     #236      +/-   ##
==========================================
+ Coverage   92.29%   92.92%   +0.62%     
==========================================
  Files         239      241       +2     
  Lines       29969    30582     +613     
  Branches     4929     5043     +114     
==========================================
+ Hits        27661    28417     +756     
+ Misses       1627     1462     -165     
- Partials      681      703      +22     

Files with missing lines	Coverage Δ
dokimi/dokimi/commands/answers.py	95.04% <100.00%> (+0.86%)	⬆️
dokimi/dokimi/commands/evaluate.py	89.81% <100.00%> (+6.01%)	⬆️
dokimi/dokimi/commands/judge_reliability.py	92.71% <100.00%> (+0.65%)	⬆️
dokimi/dokimi/commands/orchestrate.py	84.50% <100.00%> (+3.91%)	⬆️
dokimi/dokimi/commands/report.py	78.51% <100.00%> (+3.09%)	⬆️
dokimi/dokimi/report/eval_report.py	85.44% <100.00%> (+3.12%)	⬆️
dokimi/dokimi/report/model_compare.py	93.03% <100.00%> (+0.19%)	⬆️
dokimi/dokimi/utils.py	98.14% <100.00%> (+0.14%)	⬆️
dokimi/dokimi/commands/benchmark.py	81.97% <94.73%> (+0.35%)	⬆️

... and 26 files with indirect coverage changes

🚀 New features to boost your workflow:

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

[antonwolfy]

The fixes that are in place hold up, but the PR leaves sibling paths in the same commands with the traceback it sets out to remove, and one new exit code contradicts the docs. Four points inline.

What I ran, on a Linux review host at cbb1fc1:

• make ci-local: 29 passed, 0 failed, 7 skipped; 8241 tests passed, 2 skipped.
• Reproductions for the code findings, with the LLM transport mocked.
• Mutation run on the new tests: 29 mutants, 5 of which pass the full suite. Restoring any changed module to the base fails at least 2 new tests.

Copilot's point on except Exception in _check_questions_from does not apply: it is the same catch that cmd_benchmark_preflight had at the base, and SystemExit is not an Exception. Copilot's point on the unused load_questions_payload import does not apply: cmd_benchmark_preflight still calls it for the golden set at benchmark.py:181.

Checked, no issues

• Exit codes: evaluate, report, benchmark and the grounding write exit 1, the questions loaders exit 2; each is pinned by a test.
• Behaviour: _load_eval rejects no input that worked before; cmd_evaluate's except OSError covers only the constructor.
• Repo rules: changelog fragment, growth baseline, ruff and mypy are clean.

[antonwolfy]

I pushed a commit per nit, up to 0644d19, which fix the nit from my review:

• 0644d19: dokimi/tests/test_evaluate_cli_inputs.py:35: the assertion now checks that stderr starts with Error: and names the blocker path, not the OS message.

make ci-local at 0644d19: 29 passed, 0 failed, 7 skipped.

Not approving yet. Still open: the three risks from my review.

[vchamarthi]

@exai-bot /review

[exai-bot]

Note

AI-generated review of 0644d19d8bc5, requested by @vchamarthi, using the default review skill. It can be wrong: verify a finding before acting on it. React with a thumbs up or down on a finding to tell the bot whether it helped.
Model us.anthropic.claude-sonnet-5-5, effort default, 13 of 94 turns, 1.0 of 48 minutes, 463k in / 6k out tokens.

The change wraps the read and write paths of eval, report, answers, benchmark and evaluate so OS and decode errors exit with an Error: … line naming the path. The new tests cover each path. I read the diff and the code around it but could not run the tests, because pytest is not installed in the container. The input-side handling looks sound, but two output paths that the changelog's "unwritable --output" wording suggests are covered still end in a traceback.

1 finding is on the changed lines, as comments below.

Findings outside the diff

1. Minor (confirmed, correctness): report eval --out and report model-compare --out still traceback when unwritable

dokimi/dokimi/report/eval_report.py:220

• The PR and the changelog say a command exits with a message on "an unwritable --output". That holds for report generate, eval grounding and eval judge-validity, but not for two other report subcommands.
• generate_report in eval_report.py still calls os.makedirs(dir_path), open(out_path, "w") and open(qa_json_out, "w") unguarded. cmd_report_eval only catches RecordError, so report eval --out <dir or path under a file> raises a raw OSError traceback. The new _load_eval covers only the read side.
• report model-compare --out writes through Path(out_path).parent.mkdir and write_text in model_compare.py (lines 736-737), also unguarded.
• The standalone main() in eval_report.py (python -m dokimi.report.eval_report) does not catch the RecordError that _load_eval now raises, so a bad --eval there still shows a traceback.

Suggested fix:

Wrap the output writes in generate_report and generate_combined_report so they raise RecordError or SystemExit with cannot write <path>: <reason>. Alternatively, catch OSError in cmd_report_eval and cmd_report_model_compare. Add a test for each, like the existing report generate one.

For a human reviewer

• Is exit code 1 right for evaluate and benchmark run input errors, when eval checklist and cross-judge use 2 for bad input?

[exai-bot]

Note

AI-generated review of 8137a3c95996, requested by @vchamarthi, using the default review skill. It can be wrong: verify a finding before acting on it. React with a thumbs up or down on a finding to tell the bot whether it helped.
Model us.anthropic.claude-sonnet-5-5, effort default, 13 of 96 turns, 1.4 of 48 minutes, 725k in / 8k out tokens.

Re-review of 8137a3c95996: 4 new findings.

4 findings are on the changed lines, as comments below.

[exai-bot]

Nit (plausible, maintainability): Questions file is parsed twice per benchmark run

cmd_benchmark_run now loads and validates the --questions-from set via _check_questions_from, and the pipeline reads it again in _read_question_inputs (and again for every run in a multi-run). It is cheap for normal sets and gives a clean error, so this is acceptable. The pipeline's own FileNotFoundError and ValueError for the same input are now partly redundant.

[vchamarthi]

Acceptable as is: the pre-flight check gives a clean exit-2 message before any run starts, and the pipeline reads stay the authoritative path for other callers. Leaving the double parse; no change.

🤖 Addressed by Claude Code

[napetrov]

Review at 11f9e77. Earlier rounds covered the code; this note is only about landing this PR next to the others in the set.

What I ran: the eight touched test files on 11f9e77: 143 passed. ruff check: clean. git merge-tree against current main: no conflict.

Conflict with #237 in dokimi/commands/answers.py. Both PRs change the same except line in _load_runtime_config: this PR has except (OSError, UnicodeDecodeError), #237 has except (OSError, ValueError). git merge-tree refs/pull/236/head refs/pull/237/head reports a content conflict on that one line. The two versions act the same here, because UnicodeDecodeError is a subclass of ValueError. Whichever PR lands second should keep #237's ValueError form, which also covers the shape checks that follow. tests/test_answers_command.py merges cleanly.

scripts/growth_baseline.json is also changed by #233, #235 and #239. Regenerate it on the final base when this lands. Do not merge the committed copy as it is.

Still open from the bot review at 8137a3c: four findings: other eval subcommands still traceback on an unwritable --output, report eval blames --out for a sidecar failure, exit codes differ between input and output failures, and the questions file is parsed twice. None of them has a reply yet. Either fix each one or say why it can wait.

[antonwolfy]

Review at 3331949. The new guards are right and every open point from earlier rounds is fixed. One test gap is left before merge: most of the write guards added in the last commits are not pinned by a behavioural test.

What I ran, on a Linux review host at 3331949:

• make ci-local: 29 passed, 0 failed, 7 skipped; 8255 tests passed, 2 skipped.
• Mutation run on the new code: 27 mutants, 14 of which pass the new test files. Those 14 also pass the nine existing test files that exercise these commands (231 tests).
• Reproductions for the code paths, with the LLM transport mocked.

Checked, no issues

• Judge errors vs. the new except OSError: panel.evaluate_answers and judge.evaluate_answers catch provider failures per answer, so the new catch in eval panel-score and eval score sees only write errors.
• Exit codes: eval writers exit 2, matching the documented malformed-input code; report and benchmark exit 1. Each is pinned for grounding, judge-validity, score and the three report paths.
• Sidecar name: report eval --out without .md no longer overwrites the report with its own sidecar; the test fails when the old replace is restored.

[antonwolfy]

Review at 5ebaeca. The tests added since 3331949 pin every write guard I mutated, and the comment change in answers.py is right. One gap is left in the command the PR names: answers generate --output still ends in a traceback.

What I ran, on a Linux review host at 5ebaeca:

• make ci-local: 29 passed, 0 failed, 7 skipped; 8263 tests passed, 2 skipped.
• Mutation run on the new write-guard tests: 22 mutants (exit code 2 to 1, except OSError removed, guarded write replaced by a bare write_text, .md sidecar guard removed) across pairwise, checklist, judge-wording, cross-judge, ragas, panel-score, score, grounding and judge-validity. 21 fail a test. The survivor changes exc.filename or output_path to output_path in cmd_eval_panel_score, which only changes which path the message names.
• A reproduction for the finding, with the answerer and the doc client stubbed.

Checked, no issues

• except OSError in eval score and eval panel-score: Judge.evaluate_answers and the panel catch provider errors per question, so the new catch sees only the incremental and final saves.
• Sidecar name: os.path.splitext(out_path)[0] + ".qa.json" no longer collides with an --out that lacks .md.
• Repo rules: growth ratchet tests pass, changelog fragment and SPDX headers are present, schema-compat is OK.

[napetrov]

Delta review of 11f9e77..34f8e41, plus one push of mine (5ebaeca).

Resolved at 3331949/34f8e41: the four open findings from the 8137a3c bot review.

• Every eval output write now goes through write_text_or_exit (pairwise, checklist, judge-wording, cross-judge, ragas, grounding), and score/panel-score catch OSError from the save.
• report eval now names the .qa.json path when that sidecar fails, not --out. splitext also fixes the .md-less --out case.
• Output failures now all exit 2.
• The double parse of the questions file is not touched in this delta. Fine to leave it for later.

Pushed 5ebaeca: the change is one comment in answers.py, now # ValueError covers bad UTF-8, the same text as #237. Your switch to ValueError left that comment as the only difference on the line. With it aligned, git merge-tree --write-tree of this PR and #237 (b42ded3) is clean.

Ran: ruff clean. 159 passed across the touched test files. CI 24/24 green on 5ebaeca.

Still to do: regenerate scripts/growth_baseline.json on the final base, because it is shared with #233/#235/#239.

[antonwolfy]

LGTM. The answers generate write guard is right: the incremental save and the final save both exit 2 with Error: cannot write ..., matching eval score, and both are pinned by a test. One nit inline.

What I ran, on a Linux review host at 33bb01e:

• make ci-local: 29 passed, 0 failed, 7 skipped.
• Mutation run on the new guard: 8 mutants (exit 2 to 1, except OSError removed or narrowed to FileNotFoundError, either save moved outside the try, strerror formatting changed, message path changed). 6 fail a test. The 2 survivors only change which path the message names.
• Reproductions with the answerer and doc client stubbed: --output <regular file>/a.json and --output <existing directory> both exit 2 with an Error: line and no traceback.

Checked, no issues

• Provider errors: Answerer.generate_answers catches per-question failures (answerer.py:164), so a ConnectionError from retrieval is recorded as an error record and never reaches the new except OSError.
• Save failure in the pool: the incremental save sets stop and re-raises (answerer.py:190-193); the OSError leaves f.result() and the queued questions return at once. Exit 2 at concurrency 1 and 2.
• --ragas step: its sidecar write already goes through _write_json_or_exit(..., code=2).
• Repo rules: changelog fragment and SPDX headers are present, growth ratchet tests pass, no docs state an exit code for answers generate.

[antonwolfy]

Review at fabe4be. The answers generate message now names the --output path as passed, and the write guard holds. One risk is left in the same command, and two nits are inline.

What I ran, on a Linux review host at fabe4be:

• make ci-local: 29 passed, 0 failed, 7 skipped; 8265 tests passed, 2 skipped.
• Reproductions with the answerer and doc client stubbed: --output <existing directory> exits 2 with Error: cannot write <dir>: Is a directory; --output <regular file>/a.json exits 2 with an Error: line and no traceback.
• Mutation check on the new answers generate tests: changing {output_path} back to {exc.filename or output_path} passes all 14 tests in test_answers_command.py.

Still open in answers generate:

1. answers generate --questions <missing file or directory> still ends in a traceback. _load_questions (answers.py:46-53, outside this diff) catches only ValueError. --questions nope.json gives FileNotFoundError: [Errno 2] No such file or directory: '.../nope.json', and --questions <dir> gives IsADirectoryError: [Errno 21]. A non-UTF-8 file already exits 1 with a message. This predates the PR, but the PR text names answers and its questions input, and eval checklist and eval cross-judge get the same fix here. Suggestion: catch OSError next to ValueError in _load_questions and exit 1 with Error: cannot read {path}: {exc.strerror or exc}, as _load_runtime_config does, and add a test for a missing path.

Checked, no issues

• Provider errors: Answerer.generate_answers records per-question failures, so the new except OSError sees only saves.
• Repo rules: changelog fragment, SPDX headers, growth ratchet and schema-compat are clean.

[antonwolfy]

LGTM. The three points from my last review are fixed: eval score and eval panel-score now name the --output path as passed, the tests now fail on a message that names the temp file, and answers generate --questions <missing file or directory> exits 1 with Error: cannot read <path>: <reason> instead of a traceback.

What I ran, on a Linux review host at eeebd1b:

• make ci-local: 29 passed, 0 failed, 7 skipped.
• Mutation run on the delta: 16 mutants ({output_path} reverted to {exc.filename or output_path} at each of the three sites, OSError handler in _load_questions removed or narrowed to FileNotFoundError, exit codes changed, strerror dropped, either save moved out of the try). 15 fail a test.
• answers generate --questions on a missing path and on a directory through the CLI entry point: both print Error: cannot read <path>: ... and exit 1, no traceback.

Checked, no issues

• Handler order: ValueError (which covers UnicodeDecodeError and JSONDecodeError) is caught before OSError, and the two do not overlap.
• Temp-file names: the updated tests raise PermissionError on a .123.0.tmp sibling, so a message that names the temp file fails them.
• Provider errors: Answerer.generate_answers records per-question failures, so the new except OSError sees only saves.