Repository navigation
fix(cli): run, compare, arms, tasks, explore and repo exit with a message on bad input or output - #238
Conversation
…shot Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
…le that is not UTF-8 Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
…file Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
…--matrix-cells file Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
…nnot be written Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
…h file Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
…cannot be written Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
… non-UTF-8 --harness file Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
…be written Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
There was a problem hiding this comment.
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
4 open issues
_pin_batchstill allowsOSErrorfrompath.read_text(...)(e.g., permission denied, transient… · Newcommands/run.pyimports and relies on a private function (_load_spec) fromdokimi.runner.run.… · New The emitted CLI error prefix here iserror:(lowercase), while other updated commands/tests in… · New The emitted CLI error prefix here iserror:(lowercase), while other updated commands/tests in… · New
What changed in this PR
Improves CLI robustness by converting common file read/write/decode failures across multiple commands into concise, user-facing error messages with deterministic exit codes (notably exit code 2 for run/compare input issues).
Changes:
- Add structured handling for unreadable/malformed input files (YAML/JSON/UTF-8) across
run,compare,repo,arms,explore, andtasks. - Add handling for unwritable output paths for JSON/Markdown report generation.
- Add/extend tests covering the new error paths and ensure tracebacks are not emitted.
| File | Description |
|---|---|
| dokimi/tests/test_tasks_command.py | Adds coverage for unwritable tasks run outputs and artifact preservation behavior. |
| dokimi/tests/test_run_compare_errors.py | New tests asserting run/compare exit 2 with path+reason on bad inputs. |
| dokimi/tests/test_repo_run.py | Adds coverage for unreadable batch pin handling with a user-facing message. |
| dokimi/tests/test_repo_report_unreadable_cell.py | New tests for repo report/compare behavior on corrupt cell files. |
| dokimi/tests/test_repo_l0_cases.py | Adds coverage for non-UTF-8 inputs in repo scan paths. |
| dokimi/tests/test_repo_harness_profile_unreadable.py | New tests ensuring --harness load errors name the file and exit cleanly. |
| dokimi/tests/test_record_readers.py | Adds coverage for unreadable arms inputs exiting cleanly. |
| dokimi/tests/test_explore_task_files.py | New tests for malformed/unreadable explore task YAML inputs. |
| dokimi/tests/test_explore_run_outputs.py | New tests for unwritable explore run outputs. |
| dokimi/tests/test_arms_output_errors.py | New tests for arms plugin-delta unwritable output handling. |
| dokimi/tests/test_arms_matrix_cells_input.py | New tests for unreadable/invalid --matrix-cells handling. |
| dokimi/dokimi/runner/spec.py | Treats decode failures as readable errors when loading spec YAML. |
| dokimi/dokimi/runner/compare.py | Unifies snapshot read/parse/decode failures into ValueError with path context. |
| dokimi/dokimi/repos/report.py | Adds CellFileError and centralized JSON object reading/validation for batch files. |
| dokimi/dokimi/repos/harness_profile.py | Converts unreadable/non-UTF-8 harness profiles into ProfileError with path. |
| dokimi/dokimi/explore/tasks.py | Centralizes task YAML reading with path-aware errors; strengthens task schema checks. |
| dokimi/dokimi/eval/cells.py | Adds path-aware errors for unreadable/invalid matrix-cells JSON files. |
| dokimi/dokimi/commands/tasks.py | Converts output write failures into one-line SystemExit messages. |
| dokimi/dokimi/commands/run.py | Adds early spec validation + exit 2 on bad spec/snapshot; improves compare spec parsing errors. |
| dokimi/dokimi/commands/repo_run.py | Adds detection of unreadable batch pin JSON and emits actionable error. |
| dokimi/dokimi/commands/repo.py | Treats decode/JSON parse issues in repo scan inputs as clean errors. |
| dokimi/dokimi/commands/explore.py | Handles explore task parse/read failures and output write failures without tracebacks. |
| dokimi/dokimi/commands/arms.py | Adds shared helpers to write reports/save artifacts with clean errors; broadens read error handling. |
| dokimi/changelog.d/20261025_120000_vchamarthi_cli-input-run-arms-repo.md | Changelog entry describing the improved error handling and exit codes. |
Review effort: Lite
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #238 +/- ##
==========================================
+ Coverage 93.26% 93.48% +0.21%
==========================================
Files 272 272
Lines 35117 35363 +246
Branches 5964 6001 +37
==========================================
+ Hits 32752 33058 +306
+ Misses 1547 1491 -56
+ Partials 818 814 -4
🚀 New features to boost your workflow:
|
antonwolfy
left a comment
There was a problem hiding this comment.
Review at 4eaef5b. The error handling that is there is right and the new tests fail against the base modules. But the title says these commands exit with a message on bad input or output, and several paths of the same kind still end in a traceback. Five of those are worth fixing before merge, or the title and changelog should be narrowed.
Copilot's point on _pin_batch does not apply: its only caller sits inside try ... except _errors(), and _errors() includes OSError, so a permission error on batch.json already exits 1 with a message.
What I ran, on a Linux review host at 4eaef5b:
make ci-local: 29 passed, 0 failed, 7 skipped (docker, network and org-level checks).- Reproductions for the findings, in process through
cli.main(), with no model calls. - The new tests run against the base modules: the tests of the new behaviour fail there.
Checked, no issues
_pin_batch: a decode or JSON error inbatch.jsongives the message naming the batch and file.- Explore tasks: the six tasks under
data/explore_tasksand the task-format fixture still load with the new validation. - Repo rules: changelog fragment format, SPDX headers on the new test files, no generated files. The last commit's subject says "growth baseline", but it only adds the fragment.
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
antonwolfy
left a comment
There was a problem hiding this comment.
I pushed a commit per nit, up to 51a65ee, which fix the nit from my review:
51a65ee:dokimi/dokimi/commands/run.py:16: imports the new publicload_benchmark_spec(an alias of_load_specinrunner/run.py) instead of the private name.
make ci-local at 51a65ee: 29 passed, 0 failed, 7 skipped.
Not approving yet. Still open: the five risks from my review.
|
@exai-bot /review |
exai-bot
left a comment
There was a problem hiding this comment.
Note
AI-generated review of 51a65ee93b59, 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, 12 of 95 turns, 0.9 of 48 minutes, 383k in / 5k out tokens.
The PR wraps the input reads and output writes of run, compare, arms, tasks, explore and repo so a bad file ends in a one-line message and a non-zero exit. The input handling is careful and well tested. The output-write handling is incomplete in the places that matter most. run, compare and arms run/matrix-run still write their outputs unguarded, although the title and description say output errors are covered.
3 findings are on the changed lines, as comments below.
For a human reviewer
- Decide whether
run/compareoutput-write errors andarms run/matrix-runwrites belong in this PR or in a follow-up; the description currently says they are covered. - Check that exit 2 for a bad spec or snapshot in
run/compare, versus exit 1 elsewhere, is the intended CLI contract, since argparse errors also exit 2.
…n bad snapshot values run and compare exit 1 naming the path when an output cannot be written, and run exits 2 when the manifest's min_docs is not met. compare rejects non-numeric summary values. The arms output, matrix rollup and plugin delta writes go through the shared write helpers. repo report and compare reject a result file without a usable grade. tasks error messages use the Error: prefix. Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
…rror load_matrix_cells and the compare snapshot readers now catch RecursionError next to ValueError, so a pathologically nested file exits with a message naming it instead of a traceback. Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
exai-bot
left a comment
There was a problem hiding this comment.
Note
AI-generated review of 345757bee1b7, 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, 11 of 94 turns, 0.8 of 48 minutes, 442k in / 4k out tokens.
Re-review of 345757bee1b7: 2 new findings.
2 findings are on the changed lines, as comments below.
Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
…e with a bad timestamp Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
… stored unmeasurable score as a grader failure Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
|
@napetrov, on your review:
@xaleryb, on batches already stored with a NaN, infinite or >1e150 score: Normalized on read: |
…ut of the means Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
…valid Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
… RecursionError from a task load Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
Resolve the conflicts with intel#238: cmd_repo_compare passes --task and catches CellFileError too; _task_values reads first_prompt_tokens inside the new reading_cells block; compare.py keeps `import math` for min_detectable. Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
Resolve the conflicts with intel#238: summarize keeps the parse_errors count inside the new reading_cells block, and cmd_repo_report runs --hits inside the same CellFileError handling as the table. Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
After intel#238, repo report refuses a result.json of the wrong shape with a CellFileError naming it (exit 1). load_cells checks task, arm and model; --hits also sorts by rep, which the table never reads, so a cell without it stopped --hits with a KeyError traceback. load_hits now refuses it the same way: "<cell>/result.json has no usable `rep`". Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
…sage on bad input or output (intel#238) * fix(run): run and compare exit 2 with a message on a bad spec or snapshot Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * fix(repo): repo run exits 1 with a message on an unreadable batch pin Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * fix(repo): scan exits 1 with a message on a --static or --manifest-file that is not UTF-8 Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * fix(arms): plugin-delta exits 1 with a message on an unreadable arms file Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * fix(explore): list and run exit with a message on a malformed task file Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * fix(arms): run and matrix-run exit 1 with a message on an unreadable --matrix-cells file Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * fix(tasks): run exits 1 with a message when --out-json or --out-md cannot be written Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * fix(repo): report and compare exit 1 with a message on a corrupt batch file Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * fix(explore): run exits 1 with a message when --out-json or --out-md cannot be written Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * fix(repo): budget and reach exit 1 with a message on an unreadable or non-UTF-8 --harness file Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * fix(arms): plugin-delta exits 1 with a message when an output cannot be written Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * chore: changelog fragment and growth baseline Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * Import a public load_benchmark_spec in the run command Signed-off-by: Anton Volkov <antonwolfy@gmail.com> * Exit with a message on unwritable run, compare and arms outputs and on bad snapshot values run and compare exit 1 naming the path when an output cannot be written, and run exits 2 when the manifest's min_docs is not met. compare rejects non-numeric summary values. The arms output, matrix rollup and plugin delta writes go through the shared write helpers. repo report and compare reject a result file without a usable grade. tasks error messages use the Error: prefix. Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * Report deeply nested JSON in matrix cells and snapshots as an input error load_matrix_cells and the compare snapshot readers now catch RecursionError next to ValueError, so a pathologically nested file exits with a message naming it instead of a traceback. Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * Split compare_snapshots and match the Error: prefix in benchflow tests compare_snapshots went over the complexity cap, so snapshot loading and summary validation move into helpers. The tasks CLI now prints Error: for a failed preflight, so the two benchflow tests expect that prefix. Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * fix(repo): repo run exits 1 on a deeply nested batch pin A batch.json nested past the recursion limit raised RecursionError, which neither _pin_batch nor _errors() caught. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> * fix(repo): repo report and compare exit 1 on a result file without a usable agent, task, arm or model load_cells now checks the fields summarize and compare read directly and raises CellFileError naming the file, as it does for a missing grade. Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * Type _load_snapshot as Any and match the lowercase error: prefix in the subject preflight test Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * Print the benchflow preflight failure with an Error: prefix in subjects Signed-off-by: Anton Volkov <antonwolfy@gmail.com> * fix(cli): validate compare spec shape, finite snapshot numbers and cell grade/cost fields compare --spec rejects a non-mapping thresholds or thresholds.regressions and a deeply nested spec with exit 2. Snapshot summary values must be finite numbers and docs an integer. repo report requires grade.score_only (unless score_task is set), grade.success and a numeric or null agent.cost_usd. A deeply nested explore task file or arms plugin-delta artifact is reported instead of raising RecursionError. Add tests for the matrix-run delta and rollup writes and the report loader error branches. Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * fix(cli): reject non-finite cell costs, missing score and bad score_task, huge ints and bad regression thresholds repo report and compare require grade.score for a score task, a boolean score_task and a finite cost_usd. compare and the snapshot check treat an int too large for a float as not finite. compare --spec requires every thresholds.regressions value to be a finite non-negative number. The changelog names the readers that still raise. Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * fix(cli): reject malformed cell fields and check regression threshold keys repo report and repo compare exit 1, naming the file, when a cell has a non-numeric, bool or overflowing grade.score, score_max, agent.num_turns or token count, or a skills block without the keys the summary reads. A null stays valid where a run leaves a value unknown. A non-finite cost_usd now rejects its cell instead of being skipped, so the test covering it expects the exit 1 message. The compare --spec threshold tests use the keys the regression gate reads, and cover accepted zero and integer values. Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * fix(cli): treat an unreadable batch pin as a message, not a traceback _pin_batch reads _eval/batch.json whenever the path exists and reports any read failure, including a directory or a permission error, as an unreadable pin naming the file. The agent-key test lists the keys it checks directly, so removing one from the report's constant fails the test. The grade check gets a status-only case. Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * fix(repos): bound numeric cell fields and cover accepted skills blocks Numbers larger than 1e300 are rejected so sums over a batch stay finite. The same check now covers num_turns, token counts and score. The tests cover accepted skills blocks through repo report, a skills block with an expected skill but no skill_selected_correct, and a skills block without skill_invoked. Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * fix(repos): lower the cell magnitude bound so variance stays finite Signed-off-by: Anton Volkov <antonwolfy@gmail.com> * fix(repos): reject score-only cells without a score and pin the bound at 1e150 A cell with score_only true and no score is now rejected with a message instead of raising KeyError in repo report and repo compare. The magnitude bound is 1e150, so squared per-task deltas in the variance stay finite. Tests cover the bound on both sides, each token key, an empty skills block and a skills block without measured. Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * test(repos): build the bound-edge cells from _SAFE_MAGNITUDE Signed-off-by: Anton Volkov <antonwolfy@gmail.com> * fix(cli): report unexpected cell shapes and unreadable question sets without a traceback repo report and repo compare exit 1 with a message when a cell field has a shape the field checks do not cover. arms run exits 1 when --questions is a directory or deeply nested JSON. A fixed-value case pins the magnitude bound from above. Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * test(repo): pin each cell-shape error caught by repo report and compare Parametrize over the five exceptions in _MALFORMED_CELL, and drop two readers from the changelog's list of those that still raise. Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * fix(repo): record a non-integer agent.num_turns as null - repo run stores num_turns only when the harness reports an integer, so repo report and repo compare accept the cell Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * fix(repo): count a finished run as started whatever its num_turns holds repo run tested num_turns for None to tell that the agent started. A result record with a non-integer num_turns now reads as None, so a finished run with no tool events skipped the skill uptake measurement. Transcript.result_seen carries the started signal. Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * fix(cli): catch deeply nested and non-UTF-8 repo inputs and a missing tasks run task repo scan, validate, reach, budget and run exit 1 with a message on a deeply nested --manifest-file, --static or --harness file and on a non-UTF-8 or deeply nested .agent-eval/repo-eval.yaml. tasks run exits with an Error: message on a task that is missing or unreadable. The changelog fragment names what is covered. Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * fix(repo): record a non-finite grader score as a grader failure score_value passed NaN, inf and 1e200 from grade.json into result.json, and the reader check this PR adds then refused the whole batch in repo report and repo compare. A score the reader would refuse is now no measurement: the cell is a grader failure, counted as 0. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com> * test(repo): pin that a finished run with no tool call is counted The started check sat on two lines and only the skills one was tested: putting the process line back to num_turns left result.json process null with the suite green. Compute it once and assert process too. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com> * docs(run): say which exit code an empty root gives with --strict The README promised exit 1 under --strict when no document was measured, but with a golden_manifest (spec.v1.yaml has one) that is a min_docs violation and now exits 2. Say so, and pin 0 docs and --strict. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com> * Test that the grader score bound is inclusive at its edge Signed-off-by: Anton Volkov <antonwolfy@gmail.com> * Word the score-bound grader failure for non-finite and huge scores Signed-off-by: Anton Volkov <antonwolfy@gmail.com> * fix(arms): matrix run exits 1 when a cell cannot read its inputs Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * fix(repo): name an unreadable manifest and a bad --static file Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * fix(explore): reject a falsy non-string repo_root and name a task file with a bad timestamp Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * fix(repo): report a cell field of the wrong shape per cell and read a stored unmeasurable score as a grader failure Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * fix(run): exit 2 only on a bad spec, snapshot or document selection Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * fix(repo): note a non-integer num_turns in agent.trace_errors Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * test(repo): compare the batch path in a platform-independent form Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * docs(changelog): list the fixed bad-input cases Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * fix(repo): leave a process counter or grade metric beyond the bound out of the means Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * fix(explore): name the task file when a reference's lines range is invalid Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> * fix(tasks): prefix the harness validation exit with Error: and test a RecursionError from a task load Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> --------- Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com> Signed-off-by: Anton Volkov <antonwolfy@gmail.com> Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com> Co-authored-by: Anton Volkov <antonwolfy@gmail.com> Co-authored-by: Nikolay Petrov <nikolay.a.petrov@intel.com> Co-authored-by: Alexander Rybkin <alexander.rybkin@intel.com>
…rsed Namespace (#270) * refactor(arms): move the run's shared checks to dokimi/eval/arms_run.py resolve_doc_library_id, validate_expected_skills, probe_sampling and validate_supported_matrix_harnesses read no parsed arguments, and judge_reliability imported the probe from another command. Move them, unchanged, into a new evaluation-layer module under public names; the commands and the tests import them from there. validate_expected_skills collects the offered skill names in a helper of its own, so the moved function stays under the complexity cap and its growth baseline entry goes. Refs #4 Signed-off-by: Vladislav Perevezentsev <vladislav.perevezentsev@intel.com> Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> * refactor(arms): run arms run and matrix-run from a typed ArmsRunConfig _build_arms_output read the run configuration off the parsed Namespace, most fields with a getattr default, and matrix-run copied the Namespace per cell and mutated the copy. Add ArmsRunConfig to dokimi/eval/arms_run.py: frozen, the field names are the flags' dest names, and each default is what arms run runs with when the flag is left out. run_arms is the body of _build_arms_output, split under the complexity cap, and cell_config puts a matrix cell into a config with dataclasses.replace. run_config_from_args is the one place parsed flags become a config. A doc arm with no library id raises LibraryIdError instead of exiting inside the run; both commands print the same message and exit 1. The regex test over getattr(args, ...) goes with the getattr defaults. subjects run still hands matrix-run a Namespace; the next commit changes that. No flag changes; stdout, exit codes and arms.v1 are unchanged. Refs #4 Refs #229 Signed-off-by: Vladislav Perevezentsev <vladislav.perevezentsev@intel.com> Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> * refactor(subjects): build the arms run config directly in subjects run subjects run built a Namespace by hand from FORWARDED_RUN_CONFIG_FIELDS and called cmd_arms_matrix_run with it. Split the matrix run out of that handler into run_matrix(config, ...), which takes the config and the input and output paths, and have subjects run call it with run_config_from_args(args, product=..., baseline_arm=...). A field subjects run has no flag for takes the config's default, as before. FORWARDED_RUN_CONFIG_FIELDS and forwarded_run_config have no caller left and are removed. Refs #4 Refs #229 Signed-off-by: Vladislav Perevezentsev <vladislav.perevezentsev@intel.com> Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> * refactor(arms): run the matrix cells in dokimi/eval/arms_run.py The loop over the cells still lived in the command. Move it into the run module: check_matrix checks every cell's harness and sampling, and run_matrix calls it before the first cell is paid for, then runs the cells in turn and hands each finished cell and its arms.v1 output to a callback, so a later cell's failure keeps the earlier ones. A failing cell raises MatrixCellError naming it; a bad question record and LibraryIdError pass through. The command keeps the files: it checks the matrix before it makes the output dir, makes the dirs before the first paid call, saves each cell from the callback, and writes the plugin deltas and the rollup. The messages and exit codes are unchanged; the cell-name collision check now runs before the harness check, ahead of the paid sampling probe. Refs #4 Signed-off-by: Vladislav Perevezentsev <vladislav.perevezentsev@intel.com> Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> * test(arms): pin that matrix-run reports a blank question with exit 2 run_matrix re-raises EmptyQuestionError past the MatrixCellError wrapper, so a blank question is bad input (exit 2), not a failed cell (exit 1). Nothing pinned it: dropping it from the tuple left the suite green. Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com> Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> * test(answers): import resolve_doc_library_id from dokimi.eval.arms_run #253 came in from main with the old dokimi.commands.arms._resolve_doc_library_id, which this branch moved and renamed; the merge failed test_both_stacks_match_pinned_values with an ImportError. Signed-off-by: Vladislav Perevezentsev <vladislav.perevezentsev@intel.com> Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> * fix(arms): ArmsRunConfig picks the default judge for its judge_provider judge_model defaulted to the Claude judge whatever judge_provider said, so ArmsRunConfig(judge=True, judge_provider="openai") judged with claude-sonnet-5-5 on openai, while arms run --judge-provider openai runs gpt-6.1-sol. The CLI paths were right only because parse_args resolves the --judge-model sentinel first. The field now defaults to that sentinel and resolves it the way parse_args does; a provider with no default judge (openrouter) needs judge_model, as on the command line. Signed-off-by: Vladislav Perevezentsev <vladislav.perevezentsev@intel.com> Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> * test(arms): describe what the parity tests pin, not the hand-off they were written against Signed-off-by: Vladislav Perevezentsev <vladislav.perevezentsev@intel.com> Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> * chore(growth): lower the test_arm_runner baseline after the rebase onto main check_growth.py --update, once the arms split is on top of main. Refs #4 Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> * test(arms): point the output-error tests at the moved run_arms _build_arms_output moved to dokimi/eval/arms_run.py as run_arms, so the matrix-run tests from #238 patch the new name. Found while rebasing #270 onto main. Refs #4 Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> * test(arms): keep the config-defaults test's comment block to two lines The Comment blocks check (#298, on main since the rebase) fails a block of three lines that a PR adds. The rest of the reason is in the PR. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> * fix(arms): check_matrix names the cell a refusal is about The docstring promises a ValueError naming the first bad cell, but a harness or sampling refusal did not say which cell it came from. Each cell's refusal now starts with its name. Harness checks still run for every cell before the first sampling probe is paid for. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> --------- Signed-off-by: Vladislav Perevezentsev <vladislav.perevezentsev@intel.com> Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com> Co-authored-by: Alexander Rybkin <alexander.rybkin@intel.com> Co-authored-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…s lists them (#297) * feat(repo): replay grades record each credited hit; report --hits A dokimi:review_replay grade kept only the ids of the threads it credited (details.threads_hit), not which finding hit each one. To check a hit, the evaluation of IntelPython/dpnp#3099 had to rerun the location matching by hand next to gold.json and answer.txt, and that is how it found credits by chance: a finding on the right lines about something else. Separately, an answer whose findings JSON does not parse is graded parse_error with every recall 0, but no summary showed it, so an arm that makes answers harder to parse read as a recall loss. - grade_replay adds details.matches: per credited pair, in threads_hit order, the thread (id, file, line, comment cut to 200 characters), the finding (its index, repository-relative file, line as matched, severity, message cut to 200) and their distance. The matching and the metrics are unchanged, so VERSION stays 1; the graders digest a batch is pinned to changes with the source as before. - repo report --hits lists those pairs by task, arm, model and rep, two lines each (--json: records with task, arm, model, rep and cell), counts the cells graded before matches existed, and says so when the batch has no review_replay cell. The default report is unchanged. - repo report counts, per row, the cells whose grade.json has parse_error, as a note under the row ("agent answer(s) whose findings JSON did not parse") and as parse_errors in --json. - Contract §4.5 and §9, the grader docstring and a changelog fragment. Signed-off-by: Anton Volkov <antonwolfy@gmail.com> * docs(repo): say why the replay grader version stays 1 with details.matches A field added to details that leaves every metric and pass as they were does not bump VERSION; a reader tells such a grade by the field being present. Signed-off-by: Anton Volkov <antonwolfy@gmail.com> * test(repo): report --hits and the parse-error count on unreadable grades Covers a grade.json that is not JSON or not an object, details that is not an object, non-object and textless entries in matches, metrics that is not an object and a non-boolean parse_error. Signed-off-by: Anton Volkov <antonwolfy@gmail.com> * fix(repo): report --hits refuses a cell with no rep, as the table does After #238, repo report refuses a result.json of the wrong shape with a CellFileError naming it (exit 1). load_cells checks task, arm and model; --hits also sorts by rep, which the table never reads, so a cell without it stopped --hits with a KeyError traceback. load_hits now refuses it the same way: "<cell>/result.json has no usable `rep`". Signed-off-by: Anton Volkov <antonwolfy@gmail.com> * test(repo): pin "" for missing match text and rep order of --hits Review comment 4416180649: test_replay_matches_record_each_credited_pair_ nearest_first now grades a thread with no `comment` against a finding with no `severity` or `message` and compares the match with a literal whose three fields are "". Before, every input supplied those strings, so `_cut` could turn None into "None" and the suite still passed. Review comment 4416180698: test_hits_list_each_credited_pair_by_task_ arm_and_rep adds a replay cell with rep 10 next to rep 2. By directory name r10 sorts before r2, so the expected listing, rep 1, 2, then 10, now fails if load_hits drops its (task, arm, model, rep) sort or sorts the rep as a string. Signed-off-by: Anton Volkov <antonwolfy@gmail.com> * fix(repo): report reads grade.json as the host does; --hits skips grader failures `repo report` reads every cell's grade.json for parse_errors, and --hits for the matches. _grade_doc read it with a plain read_text, so a FIFO blocked the whole table, a link out of the batch was followed, and a deeply nested file raised RecursionError. It now goes through grading.read_grade_json, the reader read_grade uses (lstat, O_NOFOLLOW, and RecursionError as no grade). read_grade gets the RecursionError case too: it used to crash the grading of such a cell. --hits also credits no match of a cell whose result says grader_failure, which the table scores as 0. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> * fix(repo): parse_errors skips grader failures; score rows say scored 0 Review comment 4418950465 (the parse_errors half): a cell whose result.json says grader_failure was counted both in the row's grader_failures and, when its grade.json had metrics.parse_error, in parse_errors. summarize now leaves it out of parse_errors, as 9092ed4 leaves it out of --hits. Review comment 4418950480: the parse-error note said "(counted as failed)" on every row, but a score-only row (every dokimi:review_replay row) has no failure count; the cell is in the mean as a score of 0. The note now says "(scored 0)" on a score row and keeps "(counted as failed)" on a gate row. The module docstring, contract §9 and the changelog fragment say both. Signed-off-by: Anton Volkov <antonwolfy@gmail.com> * docs(cli): repo report --hits help says by task, arm, model and rep Review comment 4418950473: the --hits help said the pairs are listed "by task, arm and rep", but load_hits sorts and format_hits groups them by task, arm, model and rep, as contract §9 and the changelog fragment already say. The help now says so too. Signed-off-by: Anton Volkov <antonwolfy@gmail.com> * test(repo): FIFO grade test skips without mkfifo and fails on a block Review comment 4420760178: the test called os.mkfifo unguarded, an AttributeError on Windows. It is now skipped where os.mkfifo is missing, as tests/test_task_format_detect.py does. Review comment 4420760207: a report that opened the FIFO could still pass, because the cleanup opened the write end, the blocked read got EOF and the empty file read as no grade. The test now records whether the first join timed out, then opens the write end (non-blocking, a bounded number of times) until every blocked read has ended, and only then asserts that it did not time out. Signed-off-by: Anton Volkov <antonwolfy@gmail.com> * test(cli): allowlist repo report --hits and refresh the CLI snapshot The merge with main brings the flag registry of #254: its ratchet test fails a spelling that is neither canonical in dokimi/cli_flags.py nor on tests/data/cli_flags_allowlist.txt, and CI fails a stale docs/reference/cli-interface.json. `--hits` switches repo report from the table to the list of credited replay hits; no registry concept names a view switch, so it gets an allowlist row with that reason, and the snapshot is regenerated with what `make cli-snapshot` runs (the only change is the --hits option). #298's scripts/check_comment_blocks.py --base origin/main --head HEAD finds no comment block over two lines among the lines this PR adds or edits, so no comment changes. Signed-off-by: Anton Volkov <antonwolfy@gmail.com> * fix(repo): repo report --hits keeps the cell's key over a match's Review comment 4422546939: load_hits built each hit as the cell's task, arm, model, rep and cell followed by **match, so a details.matches entry with a key of one of those names moved the hit to another task or repetition in both --hits and --hits --json. The match is now spread first and the result.json values last. format_hits only reads these records, so it needs no change. A new test grades a cell whose match carries all five colliding keys and checks that the JSON records and the text listing show the cell's own task, arm, model, rep and cell. The contract (§9) and the changelog fragment say the five come from result.json. Signed-off-by: Anton Volkov <antonwolfy@gmail.com> --------- Signed-off-by: Anton Volkov <antonwolfy@gmail.com> Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com> Co-authored-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
#304) * fix(repos): an unreadable result.json is that cell's error in repo run Settles the open part of review point B3 item 2 on #238: `repo run --batch X` on a batch whose finished cell holds a truncated result.json ended the whole batch with a bare JSONDecodeError, because run_cell read the file with json.loads and one() does not catch ValueError. #238 narrowed the changelog; this changes the behaviour. run_cell now reads a finished cell through _finished_result, which raises UnreadableResult (a PrepError, so one() already reports it per cell) when the file is not UTF-8, not JSON or not a JSON object. The message names the file and says how to recover: remove it to run the cell again, or rerun with --force. The cell is logged as `✗ <cell>: <path>: ...` and counted in the summary's crashed cells, the other cells still run or are reused, and the command exits 1 as for any other crashed cell. The file is neither changed nor removed, and the cell is not run again on its own. check_batch_executions already skipped an unparsable result.json; it now also skips one that is not an object, which it used to count as an `unrecorded` execution, refusing the whole batch up front. A readable result is reused as before. The contract's paragraph on rerunning a batch gains a sentence on this case, and a Fixed changelog fragment describes it. Signed-off-by: Anton Volkov <antonwolfy@gmail.com> * fix(repos): report an unreadable kept result of an unsupported task Answers the review on #304: run_batch counted a finished cell of a task this host cannot run as already done as soon as its result.json existed, without reading it. check_batch_executions skips an unreadable file, so a result.json replaced with [] or cut in half was never reported there, even with --force, which keeps those cells. The unsupported-task branch of run_batch moves into _unsupported_cell. A kept result.json is now read through _finished_result; when it is not UTF-8, not JSON or not a JSON object (or cannot be read at all), one() records it in the run's errors and logs `✗ <cell>: <path>: ...` like any other per-cell error, and the command exits 1. The file is left as it is, and the cell is neither rerun nor marked unsupported. The message says that --force keeps the file on this host and that removing it lets the cell run on a host that meets the task's requirements. A readable kept result is counted as already done, as before. The comment on the check_batch_executions catch now names both places that report the cell, and the contract and changelog fragment cover the unsupported-task case. Signed-off-by: Anton Volkov <antonwolfy@gmail.com> --------- Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
Settles three review points that merged PRs left open. #254 (review body B1, item 3): `dokimi.cli_flags.CANONICAL` and `BANNED` were built at import by `_index()` and read by nothing; the flag contract test builds its own index from `CONCEPTS`, so a mutant that emptied them survived. The two tables, `_index()` and their `__all__` entries are removed, with a Removed changelog fragment. #155 (review body B3, item 1): no test had an Edit matcher meet a `perl -pi` Bash step, so dropping `perl` from `_SHELL_WRITERS` left the suite green. `test_perl_in_place_is_an_edit_of_its_file` checks that `perl -pi -e 's/a/b/' src/a.py` satisfies an Edit action on `src/a.py` and that a `perl` run without `-i` does not. #238 (thread T22): the accepted costs in `test_a_cost_at_the_bound_is_accepted` were built from `_SAFE_MAGNITUDE` and moved with it, so lowering the bound to 1e6 passed. A fixed cost of 1e100 is now accepted too. Signed-off-by: Anton Volkov <antonwolfy@gmail.com>



Summary
run,compare,arms,tasks,exploreandreporead user-supplied files (spec, snapshot, arms, task, batch, manifest,--matrix-cells,--harness,--static) and write--out-json/--out-mdwithout handling OS, decode or nesting errors, so a bad file or path ends in a traceback.Changes
runandcompareexit 2 on a bad spec, snapshot or document selection (DocSelectionError,CompareInputError) and 1 on an unreadable document or an unwritable output. 1 is also the--strictverdict code; the README says so. Errors raised inside metrics or the regression check keep their traceback.repo reportandrepo comparecheck every field they read inresult.jsonand name the file when one is unusable. A field of the wrong shape that the checks miss is reported per cell, naming its files; errors in the statistics code are not caught.explorerejects a non-stringrepo_root(including0and[]) and names the task file on an invalid YAML timestamp.Risk
Mostly error paths. Three changes affect recorded or read data:
repo runrecords a non-integeragent.num_turnsas null and notes it inagent.trace_errors.repo rungrades and whenrepo report/repo compareread a batch that already stores one.repo reportandrepo compareleave aprocesscounter orgrade.jsonmetric that is beyond 1e150 (or an integer too large for a float) out of the means, as they already did for NaN and infinity.arms runandarms matrix runcatchValueErroraround the whole arm build, as onmain; this PR addsOSErrorandRecursionErrorto that catch.Testing
make ci-localon Linux, Python 3.13: every row passes exceptpip-audit(filelock advisories, failing onmaintoo).test_unbalanced_quote_in_hook_completes_quickly(a 1 s timing bound, code this PR does not touch) fails intermittently in the full parallel run (1.03 to 1.04 s on a loaded host) and passed alone 3 times on this branch and onmain.Checklist
pytestpassesruff,mypy, and schema checks passrunexit codes.github/.licenserc.yaml)