Repository navigation
Conversation
The exit-code table (CLI API v2 §2.6) gives 2 to a missing, unreadable or malformed input, a flag combination a command refuses, an output file that cannot be written, and a benchmark run or preflight with no provider credential. Exit 1 stays for a verdict or a gate that ran and failed. Refusals raised as SystemExit(<message>) now go through usage_exit, which prints the message to stderr and exits 2. Messages are unchanged. Output-write failures (exit_on_unwritable, write_text_or_exit) exit 2. Benchmark preflight exits 2 on a missing credential and keeps 1 for its question-set checks. Verdict, gate, infra and tool-output sites are not changed; see exit_audit for the open questions. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Drop the try/except around _role_models in novelty; the callers already handle ValueError. Tighten the exit-2 tests: assert on stderr instead of falling back to str(code), give the report-generate case a readable eval file, rename the non-list evaluations test to exit_2. Narrow the changelog credential wording to "any missing credential". Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
benchmark preflight exits 2 for a missing credential, an unwritable output dir, or a missing or invalid question or golden set, the same inputs benchmark run refuses. A failed check alone (doc source) stays 1. The golden-set check moves to a helper to keep preflight under its complexity baseline; growth_baseline.json is lowered for novelty.py and preflight. The catalog trigger-set @field refusal (a path typo) goes through usage_exit, so it exits 2. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Each try block that maps to exit 2 now covers only the input validation step. A failure while the operation runs (model call, git fetch, harness run, --rewrite, sampling probe) keeps exit 1, as on main. - arms run: setup (_prepare_arms) exits 2; run (_run_arms) exits 1. The sampling probe runs per matrix cell inside the run loop (arms_sampling.py) - repo fetch and reach: parse, pin, profile, manifest and scenario selection exit 2; git fetch and harness run exit 1 - benchmark batch with no libraries exits 2 - benchmark preflight: an invalid doc source exits 2, an unreachable one 1 - catalog trigger-set: duplicate id and no-text question (TriggerSetInputError) exit 2; a --rewrite that returns nothing exits 1; the --strict refusal exits 2 - evaluate: missing ragas extra back to main's exit 1 (open question) Tests: tests/test_exit_codes_boundaries.py (15 cases; 11 red on the round-2 head, all green), updated strict and preflight expectations. scripts/growth_baseline.json lowered entries only. Refs intel#94 Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
- repo validate, scan, inspect, budget: parse and pin, manifest and profile are usage errors (2); the clone or fetch is a runtime failure (1), split out in _source_tree as fetch already was. - catalog trigger-set --strict is a lint gate: back to 1, as on main. - trigger-set refuses a question without text before any --rewrite call. - arms matrix-run sets up and probes every cell before the first answer (arms_matrix.prepare_matrix_cells): a refused probe exits 1 with no answer run and no cell artifact, as on main. - Tests: boundary cases for the four repo commands, the matrix probe ordering, the no-text rewrite refusal, and the strict exit. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Pin-resolution failures (pinned local non-git source, pin absent locally, remote pin missing from origin) stay 1 as on main. The repo.py comment and the changelog fragment now say: 2 for a malformed source string, an unpinned remote, manifest or profile errors; 1 for a clone, fetch or pin resolution. Probe calls are model calls and can fail, so "no spend" applies to answers only. Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #285 +/- ##
==========================================
+ Coverage 93.48% 93.58% +0.09%
==========================================
Files 272 275 +3
Lines 35363 35513 +150
Branches 6001 6002 +1
==========================================
+ Hits 33060 33235 +175
+ Misses 1489 1471 -18
+ Partials 814 807 -7
🚀 New features to boost your workflow:
|
| probe(setup) | ||
| except ValueError as exc: | ||
| # A model call refused: a runtime failure, so the matrix stops with 1 and no answer was paid for. | ||
| print(f"Error probing matrix cell {cell.name!r}: {exc}", file=sys.stderr) | ||
| sys.exit(1) |
There was a problem hiding this comment.
The same sampling probe refusal now exits 1 in arms but 2 in eval.
judge_reliability._check_judge_temperature calls the same _probe_sampling and exits 2 when the judge rejects --judge-temperature (as docs/benchmarking-and-comparison.md:392 says). Here, and in cmd_arms_run (the probe runs at the top of _run_arms, so it lands in the "Error running arms" branch), the same refusal exits 1. So eval cross-judge --judge-temperature 0.5 with a gpt-5 judge exits 2, and arms run --judge --judge-temperature 0.5 exits 1. A script that reads 2 as "fix your flags" treats the arms case as a run failure.
I see the reason: the probe catches any exception, so a network error also lands here. Could the two paths agree? Either both 2 (the value is the problem in the common case), or both 1 with the eval docs changed.
There was a problem hiding this comment.
Waiting on a decision before I change this: should the sampling-probe refusal give the same exit code in arms and eval? Context: eval (_check_judge_temperature) exits 2 when the judge rejects --judge-temperature, arms run / arms matrix-run exit 1, and _probe_sampling catches every exception, so a network error lands in the same branch. Options: (A) both 2, and the message says the probe failed and that the setting or the provider may be the cause; (B) both 1, and docs/benchmarking-and-comparison.md:392 is changed to say 1; (C) split by cause, 2 for a provider rejection of the setting and 1 for a transport error, in both paths, which needs the probe to tell the two apart. My recommendation is (A): it makes arms agree with what the docs already promise for eval, and in the common case the value is the problem. No code change until you pick. The thread stays open.
|
Review at 0cdea25. The split of 2 (validation) from 1 (a run or a gate that failed) is consistent across the commands I checked. One regression needs a fix: What I ran, at 0cdea25:
Checked, no issues
Comments on lines outside the diff:
Suggestion: "exit 2 naming it when it cannot be written" (and say that a schema rejection in |
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
Review at 0cdea25. The 2-for-validation, 1-for-run split holds in the commands I checked, and the new boundary tests pin most of it. The problems are at the edges: a narrowed try now lets an error escape as a traceback, and some of the new exit codes are not pinned by any test. I'd fix the inline comments and the changelog claim below before merge.
I agree with @vlad-perevezentsev on the repo reach traceback with a missing --bin and on the stale exit codes in two docs. Their questions on the sampling-probe codes, the @field check after --rewrite and a local source inside a repository wait for the author.
What I ran, on a Linux review host at 0cdea25:
make ci-local: 26 passed, 2 failed, 7 skipped. 9,753 tests passed and 1 failed; changed-line coverage passes. The test failure istest_reference_checks.py::test_the_regex_limit_is_shared_by_every_pattern_of_one_check, a wall-clock ratio assertion on a loaded machine; the file passes on its own (62 passed).pip-auditfails on the lock files, which this PR does not touch.- Reproductions for the code findings, with the LLM transport mocked.
- Mutation runs of the new exit codes: 4 of the repo mutants and 1 matrix mutant pass the full targeted suite (see the inline comments).
- All six commits carry a
Signed-off-by.
Findings outside the diff or across files:
- The changelog and docs claim more than the code does. The fragment lists
repo runamong the commands whose input refusals exit 2. Inrepo_run.pyonlyrepo reportandrepo comparechanged.repo runwith an unknown arm still prints✗ unknown arm 'none'and exits 1, andrepo tasks checkwith no manifest exits 1 whilerepo validateon the same missing manifest exits 2. Also outside the diff:docs/repo-evaluation-contract.md:955and the--statichelp text (repo.py:422) say an invalid--staticdocument makesrepo scanexit 1. A file that is not JSON or cannot be read exits 2 (repo.py:104); only a schema-invalid one exits 1. A script that reads 1 as "bad static output" misclassifies a truncated file. Suggestion: either convert the remainingrepo run/repo tasks checkrefusals or drop them from the fragment, and say in the docs which--staticfailures give 2 and which 1.
Checked, no issues
usage_exit: prints once and exits 2; the message text is unchanged at the convertedSystemExitsites I compared.- Matrix ordering: every cell is set up and probed before
out_diris created and before the first answer; a probe-order mutant fails the boundary tests. - Gates:
baseline compare --strict,catalog trigger-set --strict, and a--rewritethat returns nothing stay at 1. - Preflight: a refusal gives 2, a failed check alone gives 1; mutants of each branch fail.
- Repo rules: no generated artifact or
exit_audit.mdcommitted, growth baseline only lowered, a changelog fragment is present, no schema changed.
Nit: stale docstrings still say "exit 1" for paths that now exit 2: repo.py:25, questions.py:20, questions.py:44, questions.py:53, library.py:19, library.py:44, catalog.py:142, novelty_view_inputs.py:4, novelty_view_inputs.py:20, subjects.py:279 (with the two in tools.py that @vlad-perevezentsev named). Suggestion: change each to "exit 2".
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
I pushed a commit per nit, up to 5612e8d, which fix the nit from my review:
5612e8d:repo.py:25,questions.py:20,44,53,library.py:19,44,catalog.py:142,novelty_view_inputs.py:4,20,subjects.py:279,tools.py:99,108: the docstrings now say exit 2 where the code exits with EXIT_USAGE.
make ci-local at 5612e8d: 25 passed, 3 failed (host: gitleaks download, gitleaks scan, pip-audit), 7 skipped.
GitHub CI has not run on these commits: the branch conflicts with main, and Actions runs no workflow on a conflicting PR. CI will check them once main is merged in.
Not approving yet. Still open: the bug and the three risks from my review; @vlad-perevezentsev on repo reach: ProfileError/SourceError from run_reach (missing --bin) escapes as a traceback; the except at repo.py:271 catches only ReachError (code shows run_reach calls profile.version(), reach.py:311); @vlad-perevezentsev on docs still give exit 1 for baseline save/list bad input (benchmarking-and-comparison.md:793) and benchmark preflight missing golden set (external-content.md:189); the changelog cites an exit-code table that is not in the docs.
|
🔍 OpenCodeReview found 7 issue(s) in this PR. By severity: 2 medium, 5 low
Review details
📄
|
…model exit 2 check_answer_key's refusal (atoms that do not fit --artifact) and SelfJudgeError (--judge-model equal to --model) are input refusals, like the other check_* refusals; both still exited 1 with SystemExit(message). Model failures under way (ExtractionError, an empty reply) keep 1. novelty probe now checks --arms and --placebo-text before it builds a client, as analyze does, so an unknown arm exits 2 before any model call. Thread: PRRT_kwDOVBg7g86xlNZ0 Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…exit 2 The six consistency refusals (mixed or differing judge_context, differing baseline_arm, differing treatment arms, differing prompt digests, duplicate replicates without --pool-replicates) raised SystemExit(message), exit 1, while a missing --treatment-arm in the same module already exited 2. Thread: PRRT_kwDOVBg7g86xlNu- Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…al refusal next to a failed check Mutants checked: dropping the ValueError branch, turning a failed doc-source check into a refusal (exit 2), and exiting 1 or always 2 whatever the refusals are, each fail a test. Thread: PRRT_kwDOVBg7g86xlN2F Thread: PRRT_kwDOVBg7g86xlOKM Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The check moves from the command into build_trigger_set, right after the per-question validation and before the rewrite loop, so a mistyped @field path exits 2 without spending a model call per question. Tests: the refusal sends no rewrite call and writes no file; "has no text" is pinned for missing, empty, whitespace, null and non-string text; the --strict refusal asserts the label_unresolved summary; the arm-spec test passes explicit argv; the unreachable-source test stubs open_source and asserts the git fetch message. Mutants checked: dropping .strip(), dropping the isinstance check and dropping label_unresolved from the --strict condition each fail a test. Thread: PRRT_kwDOVBg7g86xgoYp Thread: PRRT_kwDOVBg7g86xlN81 Thread: PRRT_kwDOVBg7g86xlOC8 Thread: PRRT_kwDOVBg7g86xlOQ9 Thread: PRRT_kwDOVBg7g86xlOae Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…the repository root open_source raised a plain SourceError before any git operation ran, and _source_tree mapped every SourceError to exit 1. A new SourceArgumentError subclass marks the one refusal that is a mistyped argument, and _source_tree exits 2 for it. Clone, fetch and pin failures keep exit 1. Thread: PRRT_kwDOVBg7g86xgoaW Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…that is not the repository root Same refusal as the other source commands (a33186e): the mistyped argument is a usage error here too, and a failed clone or fetch keeps exit 1. Thread: PRRT_kwDOVBg7g86xgoaW Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…; fix the fragment's claims docs/quickstart.md gets an "Exit codes" table (0, 1, 2), which the changelog fragment and exit_codes.py now cite instead of an unlinked "CLI API v2 §2.6". baseline save/list and benchmark preflight (missing golden set) say exit 2 in the docs. The --static help and the repo contract say which failures exit 2 (unreadable, not JSON) and which 1 (schema-invalid). The fragment no longer lists `repo run`, whose input refusals still exit 1, says so, and names the pinned local path that is not a repository root. Thread: PRRT_kwDOVBg7g86xgoV9 Review: PRR_kwDOVBg7g88AAAABU5N1-w Review: PRR_kwDOVBg7g88AAAABU5fRKA Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Same status as before; the three sites now read the constant like the neighbouring _write_items and _save_result. Comment: IC_kwDOVBg7g88AAAABg1eeQg Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…ite_items and _save_result docstrings say exit 2) Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…es to their own file The round's changes grew novelty.py, test_subjects.py and test_model_compare.py past their baselines. The model-compare refusals are pinned by one parametrized test in test_model_compare_exit_codes.py (every case fails when its site goes back to SystemExit), test_model_compare.py is back to its size, the forwarded-fields guard scans the arms module's functions in fewer lines, novelty.py joins message literals that fit one line, and an unused import in catalog.py is removed. Thread: PRRT_kwDOVBg7g86xlNu- Thread: PRRT_kwDOVBg7g86xlNTa Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…ad of the manifest The manifest handler caught every SourceError as exit 2, which also covered a failed `git show` (a blob fetched lazily from origin that is gone). Only the path refusals of the tree (a path that leaves it, a link that goes nowhere, too many links) raise the SourceArgumentError subclass and exit 2; any other SourceError is a fetch failure and exits 1, as on main. The two handlers move into _manifest_or_exit, which also lowers the complexity baselines of cmd_repo_validate and cmd_repo_scan. Thread: PRRT_kwDOVBg7g86xiOS9 Thread: PRRT_kwDOVBg7g86xlNfy Thread: PRRT_kwDOVBg7g86xlNlc Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…ong the exceptions Docstrings and test comments cite docs/quickstart.md instead of "CLI API v2 §2.6", the quickstart no longer says every command follows the table, and the TriggerSetInputError and placebo_text_for_arms_or_exit docstrings say what they raise for and who calls them. Thread: PRRT_kwDOVBg7g86xgoV9 Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…st (intel#94 A2) repo reach caught every SourceError around the manifest load and the probe as a usage error, so a failed `git show` (a lazily fetched blob that is gone) exited 2 where main exits 1. Only a path that leaves the tree (SourceArgumentError), a bad manifest and a bad scenario selection are usage errors now; a plain SourceError is a fetch failure. The boundary test for validate and scan covers reach too. Thread: PRRT_kwDOVBg7g86xiOS9 Thread: PRRT_kwDOVBg7g86xlNfy Thread: PRRT_kwDOVBg7g86xlNlc Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
… names its sources Moving the @field check into build_trigger_set made the library refuse an @field rule that labels none of a source's questions even when the caller (the readers' round-trip fixtures) means those questions to stay unlabelled. The check now runs only when `specs` is passed, as `catalog trigger-set` does, so the refusal still comes before any --rewrite call and the library default is unchanged. Thread: PRRT_kwDOVBg7g86xgoYp Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…t and the changelog fragment Thread: PRRT_kwDOVBg7g86xiOS9 Thread: PRRT_kwDOVBg7g86xgoYp Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
|
"The changelog and docs claim more than the code does": fixed in 98608b1. The fragment no longer lists |
|
The report is the patch coverage at the earlier head. The repo's gate is changed lines at 80%; |
|
Fixed in f3a6722: the three literal |
|
Round 1 on the open items of the "Not approving yet" review (5612e8d). Each is answered in its own thread; here is where each landed.
One item waits for the author: whether the sampling-probe refusal should give the same exit code in |
|
|
||
| `benchmark run` logs a warning and continues with generated questions only. | ||
| `benchmark preflight` treats a missing golden set as an error and exits 1. | ||
| `benchmark preflight` refuses a missing golden set and exits 2. |
There was a problem hiding this comment.
This change makes the preflight refusals exit 2. The transcript note a few lines below (line 216) still says "It exited 1 here because no API keys were set". A missing credential is a refusal (refuse(...) in cmd_benchmark_preflight), so that run now exits 2. Update that sentence.
| print(f"Error probing matrix cell {cell.name!r}: {exc}", file=sys.stderr) | ||
| sys.exit(1) |
There was a problem hiding this comment.
Nit: the probe-refusal exit uses a literal 1, while every other exit in this module uses a named constant. exit_codes.py defines only EXIT_USAGE, so the failure code is easy to change by accident. Consider adding an EXIT_FAILURE = 1 constant and using it here and in cmd_arms_run / cmd_arms_matrix_run (sys.exit(1)).
| chunk = out[first:] | ||
| if specs is not None and rule.startswith("@") and chunk and not any(tq.labelled for tq in chunk): | ||
| # A typo in the path, not a set of negatives. | ||
| raise TriggerSetInputError(f"{specs[index]}: {rule} resolves for none of {len(chunk)} questions") |
There was a problem hiding this comment.
Nit: this check also fires for --trigger-set sources, which _trigger_set_sources always passes with the rule @expected_skill. A questions-format trigger set in which no question carries expected_skill is valid. Before this change it was read as all unlabelled and only produced a warning. For example, a --format questions output from an all-unlabelled run, fed back in with --trigger-set, now exits 2. The "typo in the path" reasoning does not apply to that case, and the message is misleading. Consider restricting the refusal to --questions sources, or saying in the message that the file has no labels.
| for args in (["init", "-q"], ["add", "-A"], | ||
| ["-c", "user.name=t", "-c", "user.email=t@t", "commit", "-q", "-m", "init"]): |
There was a problem hiding this comment.
This test shells out to the real git. It has no skip when git is missing, so it fails with FileNotFoundError on a host without git. A global commit.gpgsign=true also makes the commit step fail, because only the identity is overridden. Both make the result depend on the host. Add pytest.mark.skipif(shutil.which("git") is None, ...) and pass -c commit.gpgsign=false.
The Windows case is also left out. The source spec C:\...\sub@HEAD goes through rpartition("@") and the location check in RepoSource.parse. Nothing here shows that a drive-letter path is parsed as local on Windows.
Suggestion:
| for args in (["init", "-q"], ["add", "-A"], | |
| ["-c", "user.name=t", "-c", "user.email=t@t", "commit", "-q", "-m", "init"]): | |
| for args in (["init", "-q"], ["add", "-A"], | |
| ["-c", "user.name=t", "-c", "user.email=t@t", "-c", "commit.gpgsign=false", | |
| "commit", "-q", "-m", "init"]): |
| assert err.strip(), "a refusal must say what is wrong" | ||
| assert exc.value.code == 2 |
There was a problem hiding this comment.
This assertion cannot tell the intended refusal from any other exit 2. It only checks that stderr is non-empty and has no usage:. If library list --domain or personas discover started exiting 2 for an unrelated reason, such as a different missing input, every case would still pass. Parametrize each case with a command-specific expected substring, for example the missing file path or the unknown domain name, and assert it appears in err.
The comment above says a refusal may be raised as a string. A string SystemExit has code equal to that string, so == 2 would fail. Either drop that remark or handle it.
| assert "is the answering model" in capsys.readouterr().err | ||
|
|
||
|
|
||
| def test_probe_refuses_an_unknown_arm_before_any_model_call(tmp_path, skill, stub_models, capsys): |
There was a problem hiding this comment.
Nit: the name promises no model call, but nothing asserts it. If the arm check moved back after the client build or the probe wordings, the refusal would still exit 2 with the same message and this test would pass. Assert the scripted transport recorded no call, for example by comparing its call count before and after, or make the stub transport raise.
|
🔍 OpenCodeReview found 3 issue(s) in this PR. By severity: 3 low
Review details
📄
|
| except ValueError as exc: # ArtifactValidationError: the input set cannot be gap-filled | ||
| print(f"Error: {exc}", file=sys.stderr) | ||
| sys.exit(1) | ||
| sys.exit(EXIT_USAGE) |
There was a problem hiding this comment.
This try wraps all of refiner.refine(...), including the gap-filler model calls. Any ValueError raised by model or runtime code (for example, parsing a model response) now exits 2 (usage) instead of 1 (runtime or model failure). That goes against the PR's rule of narrowing each try to its validation step. Catch ArtifactValidationError specifically, or validate the input set before the model calls start.
| print(f"✗ {exc}", file=sys.stderr) | ||
| sys.exit(1) | ||
| _, tree = _source_tree(args) | ||
| manifest = _manifest_or_exit(tree, args, guidance_default=True, also_usage=(OSError, json.JSONDecodeError)) |
There was a problem hiding this comment.
json.JSONDecodeError here does nothing. _load_manifest parses YAML and wraps its own failures in ManifestError, so it never raises a JSON error, and --static is read separately in _read_static. The OSError entry is the only live one. The old handler also caught ValueError and RecursionError, and those are no longer caught here. Drop the json.JSONDecodeError entry, or catch what can actually be raised.
Suggestion:
| manifest = _manifest_or_exit(tree, args, guidance_default=True, also_usage=(OSError, json.JSONDecodeError)) | |
| manifest = _manifest_or_exit(tree, args, guidance_default=True, also_usage=(OSError,)) |
Summary
Input and usage refusals exit 2, not 1, per the exit-code table in CLI API v2 §2.6. Exit 1 is kept for a verdict or gate that ran and failed, and for a runtime or model failure. Messages are unchanged. Round 3 narrows each
tryblock to its validation step, so a failure while the operation runs stays 1. Round 4 splits the clone out ofrepo validate,scan,inspectandbudget(malformed source, unpinned remote and manifest 2; clone, fetch or pin resolution 1), keepscatalog trigger-set --strictat 1 as a lint gate, refuses a no-text question before any--rewritecall, and restores main's up-front matrix probe.Counts by class: 291 table rows on main 114cb6c (284 before round 3, plus 3 round-3 splits and 4 round-4 clone splits:
repo.pyvalidate, scan, inspect, budget). Changed 173 rows: IN 171 (of 179), CRED 1, O4 1 (benchmark.py:306, batch). Kept 118: IN 8 (the arms run, rewrite, the five fetch halves and evaluate ragas, all 1 as on main); K2 48 already 2; VER 17 (includingcatalog.py:224,--strict, which stays 1 as on main); O3 6; O4 12; O5 12; MIX 7; K2O3/K2O4/K2O5 8. The full table is inexit_audit.md.Test expectations that pinned exit 1 for input paths now pin 2 (exact-line changes plus renames). The two
--strictassertions intest_skill_selection.pystay at 1 as on main;test_trigger_set_strict_refusal_exits_1pins it on the boundary file. Preflight check-failure tests now use a real failure (RuntimeError("doc source unreachable")) so they test the failed-check path. No verdict or gate test changed its code.Evidence
Round 1, before: new parametrized tests on main, 10 failing (all exit 1 or a string code where 2 is required):
Round 2 (preflight input refusals,
@fieldrule): the updated expectations and new cases fail first on the round-1 head (12 failed;test_benchmark_command_question_set.py,test_skill_selection.py). A failed check alone stays 1, pinned by a guard test.Round 3, red: the new file
tests/test_exit_codes_boundaries.py(15 cases: one per boundary above, plus the matrix probe and ragas checks) on the round-2 head: 11 failed, 4 passed (logs/red_round3.log).Round 3, green: targeted run over the new file and the changed test modules: 126 passed (
logs/green_round3_targeted.log).Round 4, red: the round-4 tests run against the round-3 head
7b938bc7(extracted to a scratch copy,PYTHONPATHon that tree): 8 failed, 9 passed in the selected set (logs/r4_red_on_7b938bc7.log). The 8 are the matrix probe ordering, the no-text rewrite call, the four repo reach-split cases, and the strict exit. The four unpinned-remote guards pass on both heads: they pin input behaviour that was already 2.Round 4, green: targeted run over the boundary file and the changed test modules: 220 passed (
logs/r3fix_targeted.log).After, all checks on the branch head:
Merge Danger
Door: two-way. Reverting restores exit 1 on every path; no data or state changes.
Behaviour changes to review:
arms matrix runsets up and probes every cell before the first answer, as main does. A refused probe exits 1 (main also exits 1) before any answer call: no answer run and no cell artifact. The probe calls are model calls and can still fail (exit 1). Probe calls are one per distinct key (cached per process), no more than main.arms_sampling.pyis a new module;arms.pyno longer holds the probe.TriggerSetInputErroris a new class; duplicate ids and no-text questions exit 2, and a no-text question is refused before any--rewritecall. A--rewritethat returns nothing exits 1 (judgement call, recorded).--strictstays 1 (lint gate).benchmark batchwith nothing to run was a question (4) in the audit; it now exits 2 as a usage refusal.novelty._refuse_no_queriesexits 2 but writes theno_queriesartifact before the refusal, so a refused run leaves a file behind. Kept; open whether the artifact should be written only on success.novelty._role_modelsraisesValueError, notSystemExit; its only callers are the USD estimate path and_clients, both updated.Open PRs that touch the same files and need a rebase or a merge order (recorded, not blocking):
commands/)fix/cli-hint-leftovers,feat/cli-routes-table(local branches, same command modules)Found, not fixed (out of §2.6):
explore listwith a missing--tasks-rootwhile the bundled root exists returns an empty list with exit 0 (dokimi/explore/tasks.py,_task_files). Follow-up.Open questions (not changed):
novelty.py:122/161/214,repo_run.py:198/271/279,arms.py:148,repo.py:241): mapping them needs a decision on infra vs tool-output failures.cli.py:115still exits 1; §2.6 has no code for it.baseline.py:263"No baselines found" stays 1 (nothing selected); §2.6 gives 4 or 2.evaluate.py:64) stays 1 as on main; §2.6 has no row for a missing extra (2 or 5).open_source"inside a repository; give its root" stays 1 (runtime fetch failure)._probe_samplingwith user-supplied unsupported values is a real model call, so a rejected parameter there exits 1, though it is an input problem.repo compareCompareError (repo_run.py:370-372): a bad--baseline, arm selector,--metricor--modelstays 1 as on main. It is an input refusal in §2.6 terms, so 2 is the candidate. Open, not changed in this PR.@fieldrule resolves for no question (catalog.py:213): the refusal (exit 2) comes after the--rewritecalls, so those calls are spent before it. Open.novelty._refuse_no_queriesartifact-before-refusal (see above).answers.py:138,evaluate.py:234,judge_reliability.py:125-128. Their exit codes predate this audit; left as they are for a follow-up.Blast Radius: CLI exit codes for input refusals across about 25 commands. Scripts that branch on
$? -eq 1for these cases must also accept 2 (called out in the changelog).Refs #94. Not closing #275: the audit is not complete (3/4/5 and MIX sites are open questions).
🤖 Generated with Claude Code