Repository navigation
fix(cli): raise InputError for input refusals; run failures exit 1 (part of #275) - #393
Conversation
…art of intel#275) Input refusals in the eval, headroom, judge, redteam, answers, answer-formats, dataset, run and baseline commands now raise InputError instead of calling sys.exit(2). cli.main prints each one as "Error: <message>" on stderr and exits 2, so there is one path for usage errors. The exit-site ratchet (EXIT2_BASELINE) only lists the sites that still call sys.exit(2) directly. Run failures now exit 1 after writing their report: - eval score pairwise, when every comparison failed - a --judges judge that scored none of its answers (judge reliability and cross-judge runs) Left as they are, tracked in intel#275: - about 62 usage_exit() sites and exit codes 3, 4 and 5 - ingest "No supported documents found" stays exit 1 - the typed-scorer ImportError and the missing-Docling refusal stay exit 2 Test helper tests/cli_refusal.py runs a command in-process and reports the exit code and output the way cli.main would. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
|
✅ OpenCodeReview: Review complete: 0 finding(s) across 35 selected item(s). CI at Review limit reached. This PR has had 6 reviews, so it will only be reviewed again when you ask. Review details
|
There was a problem hiding this comment.
🟡 Changes recommended
The changelog’s migration notices name two invalid commands, obscuring which runnable commands have changed output or exit codes.
2 open issues
What changed in this PR
2 new findings.
This PR advances #275 by centralizing input refusals through InputError and distinguishing bad input from runtime failures in dokimi’s CLI.
Changes:
- Routes converted input refusals through
cli.mainfor consistent error output and exit code 2. - Changes all-failed pairwise runs and judges that score nothing to exit 1 after saving reports.
- Updates command tests, adds a refusal helper and exit-site ratchet, and documents the interface changes.
| File | Description |
|---|---|
dokimi/tests/test_typed_scorer.py |
Adopts refusal helper and explicit exit assertions. |
dokimi/tests/test_run_failure_exit_codes.py |
Checks all-failed pairwise runs save reports and exit 1. |
dokimi/tests/test_redteam.py |
Updates refusal and verdict assertions. |
dokimi/tests/test_record_readers.py |
Updates malformed-record refusal checks. |
dokimi/tests/test_judge_validity.py |
Adapts validity refusal tests. |
dokimi/tests/test_judge_sequential_design.py |
Updates undecidable-gate refusal assertion. |
dokimi/tests/test_judge_reliability.py |
Updates refusal handling and pairwise failure expectations. |
dokimi/tests/test_judge_not_measured.py |
Expects exit 1 for a failed judge. |
dokimi/tests/test_input_refusal_exit_sites.py |
Adds a ratchet for direct exit-2 sites. |
dokimi/tests/test_ingest_cli.py |
Adapts ingest error checks. |
dokimi/tests/test_headroom_command.py |
Adapts headroom refusal checks. |
dokimi/tests/test_eval_output_write_errors.py |
Updates output-write error assertions. |
dokimi/tests/test_datasets_export.py |
Adapts export refusal checks. |
dokimi/tests/test_cross_judge_judge_errors.py |
Pins exit 1 for judges that score nothing. |
dokimi/tests/test_baseline_unreadable_input.py |
Checks centralized malformed-record error output. |
dokimi/tests/test_baseline_compare.py |
Updates comparison refusal and verdict assertions. |
dokimi/tests/test_answers_command.py |
Adapts configuration, input, and write-error checks. |
dokimi/tests/cli_refusal.py |
Adds in-process exit and output capture helper. |
dokimi/dokimi/commands/run.py |
Raises InputError for unreadable gate bands. |
dokimi/dokimi/commands/redteam.py |
Converts input and artifact-write refusals. |
dokimi/dokimi/commands/judge_reliability.py |
Centralizes refusals and changes runtime failure exits. |
dokimi/dokimi/commands/ingest.py |
Converts materialization errors to InputError. |
dokimi/dokimi/commands/headroom.py |
Centralizes validation refusals. |
dokimi/dokimi/commands/evaluate.py |
Converts scoring and validity input refusals. |
dokimi/dokimi/commands/dataset.py |
Converts export refusals to InputError. |
dokimi/dokimi/commands/baseline.py |
Lets malformed-record errors reach the central handler. |
dokimi/dokimi/commands/answers.py |
Converts answer-write failures to InputError. |
dokimi/dokimi/commands/answer_formats.py |
Converts import and reference-scoring refusals. |
dokimi/changelog.d/20261104_120000_napetrov_input-refusal-exit-codes.md |
Documents error-output and exit-code changes. |
Review effort: Balanced
| `run` and `baseline` now raise `InputError`, which `cli.main` turns into the same | ||
| `Error: ...` line and exit 2, so a message that used to be printed and exited with | ||
| `sys.exit(2)` reads the same and no traceback escapes. A cross-judge run with several | ||
| stored-material problems prints one `Error:` line, with the problems below it. `eval baseline compare` on an eval |
There was a problem hiding this comment.
Fixed in 9059ad1: the notice now names baseline compare and eval judge agreement --judges, both checked against the registered routes.
|
|
||
| - **Run failures exit 1, not 2** — `eval score pairwise` in which every comparison failed | ||
| (the errors are still written to `--output`), and a `--judges` judge that scored none of its | ||
| answers in `eval judge reliability` / cross-judge runs, now exit 1 like other run failures |
There was a problem hiding this comment.
Fixed in 9059ad1: the notice now names baseline compare and eval judge agreement --judges, both checked against the registered routes.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #393 +/- ##
==========================================
+ Coverage 94.78% 94.86% +0.07%
==========================================
Files 353 354 +1
Lines 46575 46539 -36
Branches 7974 7974
==========================================
+ Hits 44146 44147 +1
+ Misses 1541 1513 -28
+ Partials 888 879 -9
🚀 New features to boost your workflow:
|
baseline compare is a top-level command, and the --judges exit change is in eval judge agreement; the notices named commands that do not run. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…() checked against cli.main The ratchet read only literal sys.exit(2)/exit(2)/SystemExit(2). It now also counts os._exit, quit, and a code that is a module constant holding 2 (EXIT_USAGE, or a same-file NAME = 2). The EXIT_USAGE sites that are not yet InputError are frozen in EXIT2_BASELINE (intel#275 follow-up). tests/test_cli_refusal_parity.py runs two converted commands through refusal() and through cli.main and requires the same code, stdout and stderr. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…ing states its limits Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
antonwolfy
left a comment
There was a problem hiding this comment.
Review at 2d2f745. The conversion itself holds up: every converted site keeps its message and exit 2 through cli.main, and no raise InputError sits inside a try that would catch it as a ValueError. Three things are worth fixing before merge, one inline and two below.
What I ran, on a Linux review host at 2d2f745:
make ci-local: 29 passed, 1 failed, 7 skipped; 15,694 tests; changed-line coverage 84%. The one failure istest_unbalanced_quote_in_hook_completes_quickly(2.6 s against a 1 s limit), plus two 60 s timeouts intest_audit_test_quality.py. They are timing failures under load and pass when run alone (173 passed); the PR touches none of that code.- Mutation runs on the changed commands: 26 hand-written mutants and one per converted
raise InputError. Both moved exits (pairwise all-failed, dead--judgesjudge) are pinned: mutants to exit 2 or 0 fail the tests. - All commits carry
Signed-off-by.
Findings that cannot be anchored inline:
-
benchmarking-and-comparison.md:555-557still says exit 2 for both run failures this PR moves to exit 1 (outside this diff). It reads: a judge that "scored none of its answers exits 2 once the report is written, with or without--strict, aseval score pairwisedoes when every comparison fails". Both now exit 1. A script that follows this page and checks for 2 never fires. The older fragmentchangelog.d/20261028_120000_vlad-perevezentsev_cross-judge-judge-errors.mdsays the same, so it now contradicts the new fragment. Suggestion: change both to exit 1 in this PR; the exit-code change is the point of the PR. -
Most converted refusals in
judge_reliability.py,answer_formats.pyandevaluate.pyhave no test that reaches them. Replacing eachraise InputError(...)withpasssurvives the related test files for:judge_reliability.py:80(cannot read questions),:213(checklist withoutexpected_topics),:237(unknown wording),:288(--groundedwithout--questions),:395(bad--judgesentry);answer_formats.py:28,:52,:54,:74;evaluate.py:302(unknown degradation). The exit code is what these lines decide. Atanswer_formats.py:54, losing the raise would save an artifact with no evaluations and exit 0. Run through the CLI today, that case printsError: no answered question has a grading block in --questionsand exits 2, so the behaviour is right and only the pin is missing. Suggestion: add onerefusal()case per site; the helper already makes it a few lines each.
…parable pair Without the raise, such a file gives an empty report and exits 1 as a run failure. The test fails when the raise is replaced with pass. Thread: PRRT_kwDOVBg7g860C5a6 Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Each refusal is run through refusal() and must exit 2 with its message. Replacing any one of them with pass fails its test (checked one site at a time). Covers judge_reliability.py (cannot read questions, no expected_topics, unknown wording, --grounded without --questions, bad --judges entry), answer_formats.py (import, reference scoring, no grading block, native import) and evaluate.py (unknown degradation). Thread: PRR_kwDOVBg7g88AAAABWMPbtQ#2 Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…odes-275 Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The docs page and the cross-judge changelog fragment still said exit 2 for a judge that scored none of its answers (and for pairwise with every comparison failed). Those are run failures, exit 1, since the input refusal change in this PR. Thread: PRR_kwDOVBg7g88AAAABWMPbtQ#1 Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
vlad-perevezentsev
left a comment
There was a problem hiding this comment.
Review at ad3ae50. The conversion is right: every converted refusal keeps its message and exit 2 through cli.main, and the two moved run failures exit 1 after their report is written. Two small gaps are in the line comments: the exit-code table for a judge with no key, and what the ratchet counts.
What I ran, at ad3ae50:
make ci-local --offline: 0 failed, 19 passed, 13 skipped (network and Docker jobs); full test suite, changed-line coverage, the changelog fragment check and the strict docs build pass.literal_exit_2_lineson ausage_exit(...)call and onsys.exit(code=2): both return[].
Checked, no issues
- Callers: every changed
cmd_*is reached only throughargs.funcincli.main, so no in-process caller catches the oldSystemExit. No newraise InputErrorsits inside atrythat would catch it as aValueError. Inheadroom, the helpers run outside theexcept ValueErroraroundheadroom(). - Pins: each of the ten sites @antonwolfy listed, and the pairwise "no comparable pair" refusal, now has a
refusal()test that asserts exit 2 and the message, so dropping theraisefails it.test_run_failure_exit_codes.pyasserts exit 1, so it fails on the base, which exits 2. - Docs and changelog:
benchmarking-and-comparison.mdand the older cross-judge fragment now say exit 1. The new fragment namesbaseline compareandeval judge agreement --judges(both registered) and tells scripts to check for 1. - New files: all carry the SPDX header.
RecordError is already an InputError; cli.main prints it and exits 2. Thread: PRRT_kwDOVBg7g860L0CF Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
usage_exit() prints by hand and exits 2, like a literal exit 2; its 61 remaining calls go into EXIT2_BASELINE as intel#275 follow-up sites. Thread: PRRT_kwDOVBg7g860L0CA Review: PRR_kwDOVBg7g88AAAABWQ7gaA Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
eval judge agreement --judges and eval score pairwise report a judge that scored nothing as a failed run (exit 1), a missing credential too. Thread: PRRT_kwDOVBg7g860L0B6 Review: PRR_kwDOVBg7g88AAAABWQ7gaA Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Thread: PRRT_kwDOVBg7g860L0CF Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Self-review nits on the ratchet change. Thread: PRRT_kwDOVBg7g860L0CA Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The refusal-parity test calls `data export`, the path intel#396 moved `dataset export` to, and the changelog fragment names the commands as main registers them. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
|
vlad-perevezentsev
left a comment
There was a problem hiding this comment.
LGTM
I checked e03d0d3. All three points from my last round are fixed. After the merge of #396, the refusal-parity test and the changelog fragment use the commands main registers now.
Merge: safe now: it merges cleanly into current main, 0 commits behind, and its tests pass there. Once it is in, #408 conflicts with it (evaluate.py, judge_reliability.py) and needs a rebase.
What I ran, at e03d0d3:
make ci-local --offline: 0 failed, 19 passed, 13 skipped (network and Docker jobs); 15,898 tests passed, 63 skipped.--helpon every command named in the changelog fragment and the quickstart line (eval score pairwise,eval judge agreement,data export,data import docling,baseline compare, ...): all are registered.
Checked, no issues
- Missing key: the quickstart exceptions paragraph now says
eval judge agreement --judgesandeval score pairwiseexit 1 when every judge call fails, a missing key included (4646df5). - Ratchet:
usage_exit(...)calls now count, by plain name and as an attribute, and the 61 sites are inEXIT2_BASELINEwith their reason (e81c4a1). eval judge agreement: theRecordErrorwrapper is gone, and the complexity baseline ofcmd_eval_cross_judgewent down to 29 (308882c).
antonwolfy
left a comment
There was a problem hiding this comment.
LGTM. All three points from my last review are fixed: the docs and the older cross-judge fragment now say exit 1, the untested refusals have refusal() pins, and the no-comparable-pair refusal in eval score pairwise is pinned. No new findings.
What I ran, on a Linux review host at e03d0d3:
make ci-local: 29 passed, 1 failed, 7 skipped; 15,950 tests passed; changed-line coverage passes. The one failure istest_the_interval_excludes_zero_exactly_when_p_is_below_005hitting the 60 s timeout during the full run. It passes alone (21 passed in 3.6 s), and it belongs to thecompare_intervalcode that came in with the merge of main, not to this PR.- Mutation run on the changed commands: each of the 45 converted
raise InputErrorinjudge_reliability,answer_formats,evaluate,headroom,redteam,ingest,answers,datasetandbaselinereplaced bypass, and both exit-1 moves changed to exit 2 and to exit 0. All are caught by the tests. - All 12 non-merge commits carry
Signed-off-by.
Checked, no issues
- Swallowed errors: no
raise InputErrororRecordErrorsits inside atrywhose handler catchesValueErrororException. No caller of a changedcmd_*catchesSystemExit. - Exit-1 moves: pairwise all-failed and the dead
--judgesjudge write the report first, then exit 1. The docs and both changelog fragments agree. - Ratchet:
EXIT2_BASELINEis enforced in both directions and countsusage_exit(...)calls, as asked in the earlier round.

Summary
Second half of the exit-code work in #275 (stage A2). Input refusals now raise
InputError, socli.mainprints oneError: <message>line and exits 2 from a single place. Two run failures move from exit 2 to exit 1, the code every other runtime failure uses.raise InputError:commands/answer_formats (4), answers (1), evaluate (13), headroom (16), ingest (out-dir write), judge_reliability (8), redteam (2), run (1), dataset (export), baseline (malformed records now propagate;RecordErrorsubclassesInputError).tests/test_input_refusal_exit_sites.py) counts every exit 2 in the frontend: a literal,os._exit,quit, and a module-level constant holding 2 (EXIT_USAGE). The 138EXIT_USAGEsites not converted yet are frozen per module inEXIT2_BASELINE(143 with the 5 literal exits that stay); each count drops as its site converts.tests/cli_refusal.py::refusal()runs a command in-process and reports exit code and output ascli.mainwould. About 12 test files now use it.tests/test_cli_refusal_parity.pyruns two converted commands through the helper and throughcli.mainand requires the same code, stdout and stderr.Part of #275. Not
Closes: the items listed under "Left out" remain open.Evidence
intel/main(5ce3207) in this environment:tests/test_repo_typecheck_grader.py(_image_providesand the shadowing cases). No new failures.tests/test_run_failure_exit_codes.py(pairwise all-failed writes the report and exits 1),tests/test_cross_judge_judge_errors.pyandtests/test_judge_not_measured.py(dead judge exits 1),tests/test_input_refusal_exit_sites.py(ratchet: a fixture module with each spelling of exit 2 is flagged),tests/test_cli_refusal_parity.py(helper vscli.main),tests/test_baseline_unreadable_input.py(unreadable record printsError:and exits 2).ruff check,mypy dokimi, andmake ci-local CI_LOCAL_ARGS="--offline --jobs static,typecheck"pass (rc 0). Two non-blocking docs-review warnings:docs/answer-formats.mdanddocs/ingestion-docling.mdare not updated for the exit-code change.Merge Danger
eval score pairwisewhen every comparison fails (the errors are still written to--output), and a--judgesjudge that scores none of its answers (eval judge agreement --judges). Scripts that treat 2 as "bad input" for these must also check 1. Labelledcli-interface-change.baseline compareon an unreadable eval record printsError: <message>instead of❌ <message>. Exit stays 2.Error: <message>throughcli.main. A cross-judge run with several stored-material problems used to print oneError:line per problem; it now prints oneError:line, with the problems on the lines after it.Left out (still open under #275)
usage_exit()sites, and exit codes 3, 4, 5.EXIT_USAGErefusals still written by hand in 23 frontend modules: frozen in the ratchet baseline, to convert in follow-up PRs.ingest"No supported documents found" stays exit 1 (a run outcome, not a refusal of the input shape); the reason is in the code.ImportErrorand the missing-Docling refusal stay exit 2 (recorded in the ratchet baseline).cli.py/cli_routes.pyroute rows are not touched (owned by the M2 change).🤖 Generated with Claude Code