Skip to content

fix(cli): run, compare, arms, tasks, explore and repo exit with a message on bad input or output - #238

Merged
napetrov merged 52 commits into
intel:mainfrom
vchamarthi:fix/cli-input-run-arms-repo
Oct 29, 2026
Merged

napetrov merged 52 commits into
intel:mainfrom
vchamarthi:fix/cli-input-run-arms-repo

Conversation

@vchamarthi

@vchamarthi vchamarthi commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

run, compare, arms, tasks, explore and repo read user-supplied files (spec, snapshot, arms, task, batch, manifest, --matrix-cells, --harness, --static) and write --out-json/--out-md without handling OS, decode or nesting errors, so a bad file or path ends in a traceback.

Changes

  • Each affected read or write reports the path and reason in a one-line message and exits.
  • run and compare exit 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 --strict verdict code; the README says so. Errors raised inside metrics or the regression check keep their traceback.
  • repo report and repo compare check every field they read in result.json and 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.
  • explore rejects a non-string repo_root (including 0 and []) and names the task file on an invalid YAML timestamp.

Risk

Mostly error paths. Three changes affect recorded or read data:

  • repo run records a non-integer agent.num_turns as null and notes it in agent.trace_errors.
  • A grader score that is NaN, infinite or beyond 1e150 is a grader failure, both when repo run grades and when repo report/repo compare read a batch that already stores one.
  • repo report and repo compare leave a process counter or grade.json metric 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 run and arms matrix run catch ValueError around the whole arm build, as on main; this PR adds OSError and RecursionError to that catch.

Testing

make ci-local on Linux, Python 3.13: every row passes except pip-audit (filelock advisories, failing on main too). 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 on main.

Checklist

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

…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>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
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, and tasks.
  • 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

Comment thread dokimi/dokimi/commands/repo_run.py Resolved
Comment thread dokimi/dokimi/commands/run.py Resolved
Comment thread dokimi/dokimi/commands/tasks.py Resolved
Comment thread dokimi/dokimi/commands/tasks.py Resolved
@codecov-commenter

codecov-commenter commented Oct 26, 2026 •

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 99.20844% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.48%. Comparing base (0a63f4e) to head (9e8067f).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
dokimi/dokimi/repos/report.py 98.19% 1 Missing and 1 partial ⚠️
dokimi/dokimi/runner/compare.py 97.14% 1 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
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     
Files with missing lines Coverage Δ
dokimi/dokimi/commands/arms.py 95.48% <100.00%> (+1.75%) ⬆️
dokimi/dokimi/commands/explore.py 81.25% <100.00%> (+25.20%) ⬆️
dokimi/dokimi/commands/repo.py 96.48% <100.00%> (+0.08%) ⬆️
dokimi/dokimi/commands/repo_run.py 93.96% <100.00%> (+0.16%) ⬆️
dokimi/dokimi/commands/run.py 91.52% <100.00%> (+22.19%) ⬆️
dokimi/dokimi/commands/subjects.py 86.63% <100.00%> (ø)
dokimi/dokimi/commands/tasks.py 92.70% <100.00%> (+0.23%) ⬆️
dokimi/dokimi/eval/cells.py 82.50% <100.00%> (+0.76%) ⬆️
dokimi/dokimi/explore/tasks.py 95.04% <100.00%> (+0.75%) ⬆️
dokimi/dokimi/repos/compare.py 99.48% <100.00%> (ø)
... and 10 more

... and 6 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 antonwolfy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 in batch.json gives the message naming the batch and file.
  • Explore tasks: the six tasks under data/explore_tasks and 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.

Comment thread dokimi/dokimi/commands/run.py Resolved
Comment thread dokimi/dokimi/commands/arms.py Resolved
Comment thread dokimi/dokimi/repos/report.py Resolved
Comment thread dokimi/dokimi/commands/run.py Resolved
Comment thread dokimi/dokimi/commands/run.py Resolved
Comment thread dokimi/dokimi/commands/run.py Resolved
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>

@antonwolfy antonwolfy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 public load_benchmark_spec (an alias of _load_spec in runner/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.

@vchamarthi

Copy link
Copy Markdown
Contributor Author

@exai-bot /review

@exai-bot exai-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/compare output-write errors and arms run/matrix-run writes 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.

Comment thread dokimi/dokimi/commands/arms.py Resolved
Comment thread dokimi/dokimi/commands/run.py Resolved
Comment thread dokimi/dokimi/eval/cells.py Resolved
…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>
@vchamarthi
vchamarthi requested a review from exai-bot October 26, 2026 16:24

@exai-bot exai-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread dokimi/dokimi/commands/repo_run.py Resolved
Comment thread dokimi/dokimi/repos/report.py Resolved
Comment thread dokimi/tests/test_repo_run.py Resolved
Comment thread dokimi/tests/test_run_compare_errors.py Resolved
Comment thread dokimi/tests/test_run_compare_errors.py Resolved
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>
@vchamarthi

Copy link
Copy Markdown
Contributor Author

@napetrov, on your review:

  1. Title and body refreshed; the body now lists the recorded-data changes (num_turns, the grader-failure reading of a stored bad score) under Risk.
  2. Changelog line reworded: it is a tasks run --tasks name that matches no task, which exits with a message.
  • repo scan --manifest-file catches RecursionError (commands/repo.py:321); the 100000-deep case is in test_a_deeply_nested_manifest_file_exits_1_naming_it.
  • run ValueError: see the run.py 34 thread.
  • num_turns: a nulled value is now noted in agent.trace_errors (result num_turns '3' is not an integer; recorded as null).
  • parse_constant: the bound test parses the output with a parse_constant that raises, so a NaN or Infinity in the output fails it.

@xaleryb, on batches already stored with a NaN, infinite or >1e150 score: Normalized on read: load_cells turns such a stored grade.score into a grader failure (status: grader_failure, success: false, score: null, with a reason) before the field checks, the same record repo run now writes. test_a_stored_unmeasurable_score_is_read_as_a_grader_failure covers NaN, Infinity, -Infinity, 1e200 and a 400-digit integer through repo report and repo compare.

…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>
@napetrov
napetrov merged commit f0f95a2 into intel:main Oct 29, 2026
21 checks passed
antonwolfy added a commit to antonwolfy/dokimi that referenced this pull request Oct 29, 2026
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>
antonwolfy added a commit to antonwolfy/dokimi that referenced this pull request Oct 29, 2026
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>
antonwolfy added a commit to antonwolfy/dokimi that referenced this pull request Oct 29, 2026
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>
napetrov added a commit to napetrov/dokimi that referenced this pull request Oct 29, 2026
…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>
napetrov added a commit to vlad-perevezentsev/dokimi that referenced this pull request Oct 30, 2026
_build_arms_output moved to dokimi/eval/arms_run.py as run_arms, so the
matrix-run tests from intel#238 patch the new name. Found while rebasing intel#270
onto main.
Refs intel#4

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
antonwolfy pushed a commit that referenced this pull request Oct 30, 2026
…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>
napetrov added a commit that referenced this pull request Oct 30, 2026
…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>
antonwolfy added a commit that referenced this pull request Oct 30, 2026
#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>
antonwolfy added a commit that referenced this pull request Oct 30, 2026
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants