[vlad-perevezentsev]

Summary

arms run, arms matrix-run and subjects run pass an arms run its configuration as an argparse.Namespace. matrix-run copies the Namespace and mutates it for each cell. subjects run builds one by hand from FORWARDED_RUN_CONFIG_FIELDS. _build_arms_output reads most fields with getattr(args, ..., default), so when a caller drops a field, the run looks the same as if the user had chosen the default.

This PR replaces that hand-off with one frozen dataclass and moves the run out of commands/. It is the second half of the #4 Wave 5 item, as described in this #4 comment. No flag changes; stdout, exit codes and arms.v1 stay the same.

• dokimi/eval/arms_run.py (new; no existing eval/* file changes):
  • ArmsRunConfig: 24 fields, named after the flags' current dest. Each default is what arms run runs with when the flag is left out, so the getattr defaults are gone. A left-out judge_model is the default judge for judge_provider, resolved as parse_args does.
  • run_arms is the body of _build_arms_output, split to stay under the complexity cap.
  • cell_config builds a matrix cell's config with dataclasses.replace(...).
  • run_matrix(config, cells, specs, questions, on_cell) checks every cell before the first paid call, then runs the cells in turn and hands each finished one to on_cell.
  • The run helpers move here under public names (probe_sampling is also used by judge_reliability).
• commands/arms.py keeps argument parsing, output paths, saving, printing and exit codes. run_config_from_args is the one place where flags become a config. The commands' run_matrix writes the cells, the plugin deltas and the rollup.
• subjects run builds the config directly. FORWARDED_RUN_CONFIG_FIELDS, forwarded_run_config() and the regex test over getattr(args, ...) are removed.
• Errors inside the run are now exceptions. LibraryIdError replaces a SystemExit raised inside _build_arms_output, and MatrixCellError names the failing cell. Both commands print the same messages and exit 1, as before.

There is one order change in matrix-run: the check that cell names give distinct file names now runs before the harness check, so both cheap checks come before the paid sampling probe.

Commits: (1) move the shared checks to the new module; (2) ArmsRunConfig + run_arms for arms run and matrix-run; (3) subjects run builds the config; (4) the matrix loop moves to the module; (5) @xaleryb's pin of exit 2 on a blank question in matrix-run. Then main is merged in, followed by (6) #253's test importing the renamed helper, (7) the config's judge model following judge_provider, and (8) #264's docstring. Commits 1-4 pass the tests on their own.

#229, "the arms Namespace" (not assessed yet). Three duplicates went away:

• the field list FORWARDED_RUN_CONFIG_FIELDS, now the dataclass fields;
• the 9 getattr defaults, now the dataclass defaults, which a test checks against the arms run flags;
• the per-cell Namespace copy, now dataclasses.replace.

The remaining duplicates are the flag definitions themselves (#94 F1a/F1b). F1b can now rename fields to the registry's api_kwarg in one place.

#264 and #238

• test(arms): pin what arms run, matrix-run and subjects run hand the arms runner #264 (tests only) is merged and merged in here; its parity tests pass with no edit. The last commit rewrites that file's docstring, which described the hand-off removed here.
• fix(cli): run, compare, arms, tasks, explore and repo exit with a message on bad input or output #238 edits cmd_arms_matrix_run. Merging it gives 4 mechanical conflict hunks in commands/arms.py; I merge main in after it lands.

Evidence

• New tests/test_arms_run_config.py (7 tests):

  • the config defaults equal the arms run flag defaults, also with --judge-provider openai;
  • a judge provider with no default judge needs a judge model, on the CLI and in the config;
  • a doc arm with no library id prints Error: ... and exits 1, for arms run and matrix-run;
  • a failing cell is named in the error, and the cells before it are kept;
  • run_matrix refuses a bad cell before any provider call.

  The three CLI tests also pass on main.

• 10 other test files change only their imports and monkeypatch targets (one of them, test(answers): characterize the two answering stacks (#228 step 1) #253's test_answering_stack_parity.py, came in from main).

• Mutation sweep: 21 single edits (fields dropped on the way to the runner, defaults changed, the judge model not resolved for its provider, cells not applied, errors misrouted). Each one fails a test.

• make ci-local: pytest 10133 passed, 57 skipped. The one failure besides the fragment step (green with skip-changelog) is pip-audit: filelock advisories in the lock files, which this PR does not touch.

• Growth: commands/arms.py goes from 772 to 513 lines. The _build_arms_output (13) and _validate_expected_skills (11) baseline entries are removed.

Merge Danger

Door: two-way. This is an internal refactor; the CLI, stdout, exit codes and artifacts do not change.

Blast Radius: the three commands that run arms, guarded by #264's parity tests.

Overlap: #238 (see above). #255 changes --provider choices in lines this PR does not touch.

Not in this PR:

• Flags (CLI API: the evaluation loop at the top level, subject and eval groups below, one enforced flag vocabulary (proposal v2.1) #94 F1b), including the flags that subjects run and matrix-run lack today: adding them changes behaviour.
• Typed plugins/judge_panel: they stay comma-separated strings, because tuples would change how a cell's plugin list is split.

Checklist

• Tests added and pytest passes
• ruff, mypy, and schema checks pass (make ci-local)
• Docs: evaluating-treatments.md depends_on and the architecture note name the new module. No changelog fragment (internal refactor): skip-changelog is set
• Self-reviewed per "Before opening a PR" in AGENTS.md
• No ad-hoc run artifacts committed
• New files carry the SPDX license header

Refs #4
Refs #229

[xaleryb]

Ran: the full suite, lint and types on the head; the head merged into current main; old-vs-new CLI runs of arms run, matrix-run and subjects run.

• Merge with main breaks a test. Input: merge the head into current main. Observed: tests/test_answering_stack_parity.py::test_both_stacks_match_pinned_values fails with ImportError: cannot import name '_resolve_doc_library_id' from 'dokimi.commands.arms'. Expected: green. Fix: rebase, then import resolve_doc_library_id from dokimi.eval.arms_run at :265/:275. No alias.
• Input: ArmsRunConfig(product=..., judge=True, judge_provider='openai'). Observed: judge claude-sonnet-5-5 on openai, while arms run --judge --judge-provider openai resolves gpt-6.1-sol. With openrouter the config runs and the CLI refuses. Expected: what the docstring promises. Proposal: resolve via default_judge_model(judge_provider), using either a None default or the JUDGE_MODEL_FOR_PROVIDER sentinel in __post_init__, and add a non-default provider case to the defaults test.
• 4d7ca1d: pins that matrix-run exits 2 on a blank question. Before it, nothing failed if that branch was dropped.
• Left: rebase; skip-changelog label or a fragment, so the Tests jobs run.

[antonwolfy]

The refactor holds up, with two open points from @xaleryb that I confirm and that should be fixed before merge. I found nothing new.

I agree with @xaleryb that the head merged into current main fails test_answering_stack_parity.py, and that ArmsRunConfig keeps the Claude judge for a non-default judge_provider. New evidence: ArmsRunConfig(product="p", judge=True, judge_provider="openai").judge_model is claude-sonnet-5-5; the CLI paths are fine only because cli.py resolves the sentinel first.

What I ran, on a Linux review host at 4d7ca1d:

• make ci-local: 25 passed, 3 failed, 7 skipped. 9302 tests passed; changed-line coverage 95% (232 lines measured).
• Fragment for package changes fails because the PR changes dokimi/dokimi/ with no changelog fragment and no skip-changelog label, as @xaleryb noted; either one turns it green. The other two failures are not the PR's: pip-audit lists filelock advisories in the lock files, which this PR does not touch. test_unbalanced_quote_in_hook_completes_quickly took 1.04s against a 1s limit, and passes on its own (508 passed in test_repo_environment.py).
• Mutation run: 119 single-point mutants of arms_run.py, commands/arms.py and commands/subjects.py (fields dropped on the way to ArmRunner, run_config_from_args fields, defaults, cell_config, exit codes, error routing). The related test files kill all 119.

Checked, no issues

• Parity: every flag the base passed to ArmRunner, Judge and JudgePanel still reaches them; exit codes and messages for LibraryIdError, MatrixCellError and EmptyQuestionError match the base.
• Description: ArmsRunConfig has 24 fields named after the arms run flag dests, and the defaults match the flags apart from the judge model above.
• Growth baseline: the removed entries and the new line counts match; test_growth_ratchet.py passes.
• Leftovers: no references to the removed helpers remain in code, tests or docs. All five commits carry Signed-off-by.

[vlad-perevezentsev]

@xaleryb @antonwolfy thanks. I merged main into the branch, on top of your 4d7ca1d, and added three commits:

• Merge with main: b11aa61 imports resolve_doc_library_id from dokimi.eval.arms_run in test_answering_stack_parity.py.
• Judge model: 4f41612. judge_model now defaults to the JUDGE_MODEL_FOR_PROVIDER sentinel, the --judge-model default, and __post_init__ resolves it with default_judge_model(judge_provider), like parse_args. ArmsRunConfig(product="p", judge=True, judge_provider="openai").judge_model is gpt-6.1-sol. With openrouter and no judge_model, it raises ValueError, the way the CLI exits 2. An explicit judge_model is kept. The defaults test now also runs with --judge-provider openai. A new test covers openrouter on both the CLI and the config. Removing the resolution fails the first test, and removing the None check fails the second.
• test(arms): pin what arms run, matrix-run and subjects run hand the arms runner #264's docstring: 299f31b rewrites the module docstring of test_arms_run_config_parity.py, which described the Namespace hand-off this PR removes. Its tests are unchanged.
• skip-changelog is set.

make ci-local at 299f31b: 26 passed, 2 failed, 7 skipped (pytest 10133 passed). The two failures are the fragment step, which the label fixes, and pip-audit (filelock in the lock files, as on main).

[codecov-commenter]

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

Codecov Report

❌ Patch coverage is 96.73469% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.53%. Comparing base (3cfe49c) to head (dc01659).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
dokimi/dokimi/eval/arms_run.py	96.92%	4 Missing and 2 partials ⚠️
dokimi/dokimi/commands/arms.py	95.65%	1 Missing and 1 partial ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #270      +/-   ##
==========================================
+ Coverage   93.50%   93.53%   +0.03%     
==========================================
  Files         273      274       +1     
  Lines       35569    35643      +74     
  Branches     6040     6041       +1     
==========================================
+ Hits        33260    33340      +80     
+ Misses       1492     1486       -6     
  Partials      817      817              

Files with missing lines	Coverage Δ
dokimi/dokimi/commands/judge_reliability.py	92.93% <100.00%> (ø)
dokimi/dokimi/commands/subjects.py	86.57% <100.00%> (-0.07%)	⬇️
dokimi/dokimi/commands/arms.py	97.84% <95.65%> (+2.36%)	⬆️
dokimi/dokimi/eval/arms_run.py	96.92% <96.92%> (ø)

🚀 New features to boost your workflow:

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

[vchamarthi]

✅ OpenCodeReview: Review complete: 0 finding(s) across 18 selected item(s).

[exai-bot]

✅ OpenCodeReview: Review complete: 0 finding(s) across 20 selected item(s).

Review details

• Commit reviewed: dc01659 against merge base 3cfe49c
• Coverage: 20 files in 5 groups
• Engine: OpenCodeReview v1.13.3, us.anthropic.claude-sonnet-5-5
• Usage: 636k tokens (446k from cache), 56s
• Tool calls: 32 (19 code_search, 11 file_read, 2 file_find)
• Extract arms run logic into eval/arms_run module (core code): 4 files
• Docs and growth baseline updates: 3 files
• Arms run config tests (new and parity): 4 files
• Subjects and session tests updated for refactor: 3 files
• Other tests adjusted for refactor (parity, judge, question ids, skills, treatments, empty question): 6 files

[antonwolfy]

LGTM. Both points from my earlier review are fixed: ArmsRunConfig(product="p", judge=True, judge_provider="openai") now resolves its judge model through default_judge_model, and test_answering_stack_parity.py imports resolve_doc_library_id from dokimi.eval.arms_run. I found nothing new.

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

• make ci-local: 28 passed, 1 failed, 7 skipped. 10631 tests passed; changed-line coverage 97% (245 lines measured). The one failure is Fragment for package changes; the skip-changelog label on the PR covers it.
• Mutation run: 115 hand-picked single-point mutants of arms_run.py, commands/arms.py and commands/subjects.py (fields dropped on the way to ArmRunner, Judge, JudgePanel; defaults; cell_config; judge-model resolution; check order; caught exception classes; exit codes). The related test files kill all of them.

Checked, no issues

• Defaults: all 24 ArmsRunConfig fields compared with the argparse defaults of arms run, matrix-run and subjects run. The only difference is judge_model, which is the --judge-model sentinel resolved in __post_init__.
• Parity: every argument the base passed to ArmRunner, Judge and JudgePanel still reaches them. Exit codes and messages for LibraryIdError, MatrixCellError and EmptyQuestionError match the base. The one stderr change is the matrix cell '<name>': prefix on a matrix-run sampling refusal, which the last commit adds on purpose and a test pins.
• Leftovers: no reference to FORWARDED_RUN_CONFIG_FIELDS, forwarded_run_config, _build_arms_output, _validate_expected_skills, _probe_sampling or _resolve_doc_library_id remains in code, tests, docs or scripts. The growth baseline matches the new line counts.
• Repo rules: SPDX headers on the new files, docs build, docs-to-review check and comment-block check pass, no generated artifacts, and every non-merge commit carries Signed-off-by.