Skip to content

fix: fail incomplete task runs and add opt-in baseline regression gates - #300

Merged
vchamarthi merged 26 commits into
intel:mainfrom
vchamarthi:fix/task-completion-baseline-gates
Nov 2, 2026
Merged

vchamarthi merged 26 commits into
intel:mainfrom
vchamarthi:fix/task-completion-baseline-gates

Conversation

@vchamarthi

Copy link
Copy Markdown
Contributor

tasks run exits 0 when some cells errored, were never run, or have no score, so a partly broken grid reads as a result. baseline compare can check that the models match (--strict) but has no way to fail on a score regression.

Changes

  • tasks run checks the result grid against the planned task x harness x repeat cells after writing runs.json and the report. Any errored, unscored (non-finite or missing reward), missing, duplicate or unplanned cell prints an incomplete line with counts and lists the missing cells, then exits 1. A verifier failure (reward 0.0) is a scored outcome and keeps exit 0. A supported --dry-run prints the plan and exits 0.
  • baseline compare takes opt-in gates: --max-context-drop and --max-lift-drop (score points, finite and >= 0, equality passes) and --min-matched (default 1). With a gate on, every scored question must match on both sides, and both answer and judge models must be recorded and equal. Otherwise it exits 1. --strict alone is unchanged: an identity check only.
  • An undecided judge-validity check reads INCONCLUSIVE in the result line and in evaluate's warnings. Exit codes are unchanged.

Behaviour change

This reverses part of #181. That PR made tasks run exit 1 only when every cell errored and kept partly errored runs at exit 0. Here a partly errored run also exits 1. test_tasks_run_with_some_cells_errored_keeps_exit_0_and_counts becomes test_tasks_run_with_some_cells_errored_is_incomplete. A CI job that runs tasks run and tolerates some errored cells now fails. The artifacts are still written.

Testing

  • Breaking the code on purpose fails the tests each time: drop check > to >= (7 fail), --min-matched check removed (1 fails), unscored cells not counted (1 fails).
  • Two oneTBB tasks run in real Docker against the oracle: 4/4 cells completed, exit 0. A run with a missing harness wrote its results and exited 1.

Not run: a Docker run on this rebased branch (the Docker results above are from before the rebase onto main), and a full native Windows suite.

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>
Copilot AI balanced review requested due to automatic review settings October 30, 2026 02:55
@vchamarthi
vchamarthi requested a review from a team as a code owner October 30, 2026 02:55
@github-actions github-actions Bot added the cli-interface-change Changes dokimi's user-facing CLI surface label Oct 30, 2026
@github-actions

Copy link
Copy Markdown

CLI interface change detected

This PR changes dokimi's user-facing CLI surface (commands and/or options):

~ `dokimi baseline compare` parameters changed:
    + added option `--max-context-drop`
    + added option `--max-lift-drop`
    + added option `--min-matched`

Updated on every push; the cli-interface-change label is on while the diff is non-empty.

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.

🟡 Changes recommended

Incomplete panel votes can falsely pass baseline regression gates, while command harness runs with valid pass/fail verdicts are incorrectly rejected as unscored.

2 open issues
What changed in this PR

2 new findings.

This PR strengthens evaluation gates: incomplete task grids fail after saving artifacts, baseline comparisons gain opt-in regression thresholds, and undecided judge-validity results are labeled clearly.

Changes:

  • Reject errored, unscored, missing, duplicate, or unplanned task cells while preserving supported dry runs.
  • Add context-score and paired-lift regression gates requiring complete matched scores and known matching models.
  • Display INCONCLUSIVE judge-validity results without changing exit policies, and update tests and documentation.
File Description
dokimi/​tests/​test_tasks_command.py Tests completion checks and artifact preservation.
dokimi/​tests/​test_judge_validity.py Updates inconclusive-result assertions.
dokimi/​tests/​test_judge_reliability.py Checks the explicit inconclusive verdict.
dokimi/​tests/​test_baseline_compare.py Tests regression thresholds and evidence requirements.
dokimi/​tests/​outcomes/​journeys.yaml Updates capability status and outcome evidence.
dokimi/​scripts/​growth_baseline.json Lowers the reliability test file’s size baseline.
dokimi/​dokimi/​eval/​judge_validity.py Labels undecided results INCONCLUSIVE.
dokimi/​dokimi/​commands/​tasks.py Checks task-grid completeness after saving artifacts.
dokimi/​dokimi/​commands/​evaluate.py Updates inconclusive warnings and help text.
dokimi/​dokimi/​commands/​baseline.py Registers regression flags and enforces model checks.
dokimi/​dokimi/​commands/​baseline_regression.py Implements matched-score regression gates.
dokimi/​docs/​user-journeys/​journeys.md Updates gating capabilities and acceptance criteria.
dokimi/​docs/​coding-harnesses.md Documents stricter task completion behavior.
dokimi/​docs/​benchmarking-and-comparison.md Explains regression gates and inconclusive exit policies.
dokimi/​changelog.d/​20261026_evaluation_completion.md Records completion, verdict, and regression-gate changes.

Review effort: Balanced

Comment thread dokimi/dokimi/commands/tasks.py Resolved
@exai-bot

exai-bot commented Oct 30, 2026 •

Copy link
Copy Markdown

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

Review details
  • Commit reviewed: 3d13b56 against merge base 5cbf0c2
  • Coverage: 1 files in 1 groups
  • Engine: OpenCodeReview v1.13.3, us.openai.gpt-6.1-sol
  • Usage: 15k tokens (8k from cache), 5s
  • Tool calls: 0
  • dokimi/docs/reference/cli-interface.json: 1 file

Comment thread dokimi/dokimi/commands/baseline.py Resolved
Comment thread dokimi/dokimi/commands/baseline.py Resolved
Comment thread dokimi/dokimi/eval/judge_validity.py Resolved
Comment thread dokimi/tests/outcomes/journeys.yaml Resolved
Comment thread dokimi/tests/test_tasks_command.py Resolved
@codecov-commenter

codecov-commenter commented Oct 30, 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 98.50746% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.30%. Comparing base (63c9cdb) to head (3d13b56).
⚠️ Report is 18 commits behind head on main.

Files with missing lines Patch % Lines
dokimi/dokimi/harnesses/command.py 91.30% 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     #300      +/-   ##
==========================================
+ Coverage   94.04%   94.30%   +0.26%     
==========================================
  Files         294      309      +15     
  Lines       38011    39437    +1426     
  Branches     6432     6672     +240     
==========================================
+ Hits        35747    37192    +1445     
+ Misses       1454     1433      -21     
- Partials      810      812       +2     
Files with missing lines Coverage Δ
dokimi/dokimi/commands/baseline.py 94.75% <100.00%> (+0.54%) ⬆️
dokimi/dokimi/commands/baseline_regression.py 100.00% <100.00%> (ø)
dokimi/dokimi/commands/evaluate.py 90.23% <100.00%> (+0.29%) ⬆️
dokimi/dokimi/commands/tasks.py 93.66% <100.00%> (+0.91%) ⬆️
dokimi/dokimi/eval/judge_validity.py 98.33% <100.00%> (ø)
dokimi/dokimi/harnesses/runner.py 99.19% <100.00%> (+0.01%) ⬆️
dokimi/dokimi/records.py 100.00% <100.00%> (ø)
dokimi/dokimi/harnesses/command.py 97.10% <91.30%> (-0.19%) ⬇️

... and 57 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Reject panel scores with a failed vote in the regression gate, score the
command harness exit-code fallback, print the gate verdict after the
comparison output, keep FAIL visible on an undecided judge-validity failure,
document the stricter model parsing, and tie the journey evidence to the exit
codes.

Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
Comment thread dokimi/dokimi/commands/baseline.py Resolved

@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 003edba. The completion check and the baseline gates behave correctly in every case I ran, and the earlier bot points are fixed. Two of the new decisions are not pinned by any test, and there are three doc nits.

What I ran, on a Linux review host at 003edba:

  • make ci-local (with Docker): 34 passed, 0 failed, 6 skipped.
  • Mutation runs on a copy of the head against the new test files, plus hand-built baseline compare and tasks run cases with the LLM transport mocked. 28 of 35 mutants of the gate code were killed; the survivors that decide a verdict or exit code are in the two inline comments.

Nits outside this diff:

  1. Nit: .github/workflows/agent-quality.yml:42 and dokimi/scripts/check_smoke_runs.py:7 still say tasks run's exit code "does not reflect pass/fail" and that it is "used by callers that want partial results". Since this PR any errored, unscored or missing cell exits 1. Suggestion: reword both comments to say the exit code now flags incomplete runs and the explicit pass-rate check covers failed verification.
  2. Nit: changelog.d/20261021_180000_napetrov_outcome-completeness-2.md:3 (unreleased) still says "partly errored runs keep exit 0". Both fragments land in one release, so the notes would state both rules. Suggestion: drop that clause from the older fragment. This PR's bullet also changes behaviour that CI jobs depend on, so ### Changed fits better than ### Fixed.

Checked, no issues

  • Completion check: every non-error reward=None row in harnesses/* is a dry-run row or carries metrics.error, so real runs never hit "unscored" by accident. A nonexistent binary exits 1, exit code 3 is a scored 0 with exit 0, and --repeats 2 with a command harness exits 0.
  • Gate logic: > vs >=, lift vs context index, completeness on each side, --min-matched, Decimal handling at the boundary (6.9 vs 0.1) and duplicate question ids are all pinned by killed mutants.
  • Earlier bot points: Copilot's panel-vote and command-fallback points are fixed at this head and covered by tests.
  • Repo rules: the schema, nav and growth-baseline checks pass, the commits all carry Signed-off-by, and journeys.yaml evidence ids exist.

Comment thread dokimi/dokimi/commands/tasks.py Resolved
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
…hanged

Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
Comment thread dokimi/tests/test_tasks_command.py Resolved

@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 1de60da, which fix the three nits from my review:

  • 1fc52af: dokimi/docs/benchmarking-and-comparison.md:487: states that an undecided result prints RESULT: INCONCLUSIVE above the gate and RESULT: FAIL (INCONCLUSIVE, see above) below it.
  • 57e06f2: dokimi/changelog.d/20261021_180000_napetrov_outcome-completeness-2.md:3: dropped the stale "partly errored runs keep exit 0" clause, and moved this PR's task-run exit bullet under ### Changed.
  • 1de60da: dokimi/scripts/check_smoke_runs.py:7: docstring now says tasks run exits 1 for an incomplete run, not for failed verification, which this gate adds.

make ci-local at 1de60da: 31 passed, 3 failed (host: gitleaks download, gitleaks scan, one timing-flaky test), 6 skipped.

Tests (py3.11, py3.14) fail in test_cli_flags.py::test_every_spelling_on_the_tree_is_canonical_or_allowlisted, and the mypy job fails at CLI snapshot is current. Both checks came with #254, which merged into main after my review: the new baseline compare flags (--max-context-drop, --max-lift-drop) are not in cli_flags.py or tests/data/cli_flags_allowlist.txt, and cli-interface.json lacks them. The fix commits touch only docs. Fix: allowlist the new flags with a reason (or use the registry spelling), then make cli-snapshot.

Not approving yet. Still open: the two risks from my review.

@vlad-perevezentsev vlad-perevezentsev 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 1de60da. The completion check and the baseline gates work in the cases I ran. Two things need a change before merge: on current main the three new baseline compare flags fail the CLI flag registry check, and the command-harness exit-code fallback also turns an unreadable reward file into a scored 1.0. The rest is small and in the line comments.

What I ran, at 1de60da:

  • make ci-local --offline: 18 passed, 0 failed, 13 skipped (network-only jobs). It runs against the PR base, so it does not see #254.
  • This head merged with main (e97ff7a): tests/test_cli_flags.py fails 2 tests, the registry check (on --max-context-drop, --max-lift-drop and --min-matched) and the CLI snapshot.
  • The external-catalog smoke (docker-oracle + docker-probe on the pinned task): 2/2 cells measured and passed, the isolation probe is closed.
  • tasks run repros with a command: harness: a reward.txt holding 2.0 gives reward 1.0, passed, exit 0; a repeated harness or task runs both cells, then exits 1 with "1 duplicate".

Checked, no issues

  • Gate logic: _scored drops errored rows, errored scores and incomplete panel votes on both sides. Coverage has to be complete and unique, the model check turns strict under a gate, and the gate runs after the comparison tables.
  • Judge validity: the RESULT: line matches the exit code: INCONCLUSIVE (see above) only for a passing undecided result, and FAIL (INCONCLUSIVE, see above) otherwise.
  • Callers of tasks run: the only one in CI is agent-quality.yml, which checks the pass rate in its own step, so an exit 1 on an incomplete run does not hide anything.
  • Earlier review points: Copilot's panel-vote and command-fallback threads and the exai-bot threads resolved in 003edba are fixed in the code.

Comment thread dokimi/dokimi/harnesses/command.py Resolved
Comment thread dokimi/dokimi/commands/baseline.py Resolved
Comment thread dokimi/dokimi/commands/tasks.py Resolved
Comment thread dokimi/dokimi/commands/baseline.py Resolved
Comment thread dokimi/scripts/check_smoke_runs.py Resolved
Comment thread dokimi/dokimi/commands/tasks.py Resolved

@napetrov napetrov 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 of #300 at head 1de60da (base 3cfe49c).

Provenance: one changed finding (inline); one changed nit (docs, below). Other rows are questions, see below.

Checks: CI is red on Tests (py3.11), Tests (py3.14) and mypy at head 1de60da. I could not read the job logs with gh from this box, so I did not confirm the failing tests.

Nit (docs, no line here): .github/workflows/agent-quality.yml lines 41-44 still say tasks run's exit code "does not reflect pass/fail" and serve "callers that want partial results". This PR makes that step exit 1 on incomplete cells, so the comment is now wrong.

Question: --harnesses codex,codex or --tasks a,a now exit 1 with invalid identities: N duplicate instead of running twice. The PR body says duplicates count as incomplete, so this looks intended; it is a behaviour change for existing scripts. Probe from the edge pass: probes/completion_edges.py.

Open threads from other reviewers that I confirmed or did not repeat: the duplicate-identity run-then-reject at tasks.py:211 (vlad-perevezentsev), --min-matched silently ignored without a drop gate at baseline.py:352 (vlad-perevezentsev), the no-test mutants in baseline_regression.py:61 and tasks.py:197 (antonwolfy), and the CLI flag registry failure on main after #254 (vlad-perevezentsev). I did not re-probe those.

Coverage: read the diff and the probes from the plain and edge passes. Did not run the test suite (the sandbox pytest plugins do not load).

Maintainer view on the scoring question: for command-harness cells, treat exit 126/127 and signal exits as no verdict (an infra error, not counted) and keep every other exit code scored. Please add a test for that and a changelog line saying avg_reward changes for command-harness runs.

Comment thread dokimi/dokimi/harnesses/command.py Resolved

@napetrov napetrov 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 of #300 at 1de60da against intel main 72d7add.

Merge: clean. git merge-tree with main has no conflicts (only growth_baseline.json auto-merges). No command is added, so no route-table row is needed. baseline compare stays free, which is right: the gate reads saved files.

Mismatches with the new conventions:

  1. Flag registry (still open, already raised). dokimi/dokimi/commands/baseline.py:348, :350, :352 add --max-context-drop, --max-lift-drop and --min-matched. None is in cli_flags.py or tests/data/cli_flags_allowlist.txt, so test_every_spelling_on_the_tree_is_canonical_or_allowlisted fails. Fix: add three rows to dokimi/tests/data/cli_flags_allowlist.txt with a reason, e.g. --min-matched # baseline compare gate: no registry concept yet, then run make cli-snapshot. I checked this on a merge with main: test_cli_flags.py, test_cli_routes.py, test_baseline_compare.py and test_tasks_command.py pass (123). If you want the gate flags canonical now, add a concept to cli_flags.py instead.
  2. CLI snapshot (same root cause). dokimi/docs/reference/cli-interface.json lacks the three flags, so test_the_cli_interface_snapshot_is_current and the mypy job fail. Fix: the regeneration in item 1.
  3. Exit rule, duplicate identities (the run-then-reject point is already raised; this adds the exit code). --harnesses codex,codex or --tasks a,a is a usage error, so under the rule it exits 2 and nothing runs. Now the cells run, then _check_completion calls sys.exit(1) at dokimi/dokimi/commands/tasks.py:223. Fix: reject duplicates where the harness list is built (tasks.py:231-233) and where tasks are selected, with SystemExit(2). Keep exit 1 at :223 for errored, unscored or missing cells, which are runtime results.
  4. Exit rule, partial evidence (decide). dokimi/dokimi/commands/baseline_regression.py:54 exits 1 when matched evidence is below --min-matched or is not complete and unique. This PR already exits 2 for a malformed file (baseline.py:281). Recommendation: exit 2 for partial or duplicate baseline and current evidence, since the user can fix the input. Keep exit 1 for REGRESSION at baseline_regression.py:66, so CI can tell the two apart. Your call.

Not repeated here: --min-matched ignored without a drop flag, the command-harness reward fallback, the mutant gaps, the doc nits and the agent-quality.yml comment. Those are already open in the threads.

Pre-existing on main, not this PR: tasks.py:131, :162 and :233 ("No tasks/harnesses selected") and baseline.py:210 ("Baseline not found") exit 1. Under the rule these are usage errors and should be 2. Not blocking for #300; a follow-up.

@napetrov napetrov 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.

CLI API check against #94 v2.1, which was accepted on 10-28. The goal fits: an opt-in regression gate, and "not measured is never pass" (G3). The placement and exit codes conflict with the accepted design.

  1. The thresholds belong to gate, not compare. In §2.1, compare does the paired comparison and gate <comparison> --policy applies the thresholds and sets the exit code. The §2.9 pr-check recipe is compare $RUN baseline:main → gate --policy policy.yaml. With --max-context-drop / --max-lift-drop / --min-matched, baseline compare becomes a second gate.
  2. Exit codes (§2.6). The PR exits 1 in every case. Under the table, a regression beyond the threshold exits 1. Not comparable (models differ or unrecorded, questions unmatched) is invalid and exits 4. Below --min-matched is inconclusive and exits 3. As written, CI cannot tell "regressed" from "could not compare". A2 #285 is setting up this split.
  3. baseline compare is being moved. §2.10 maps it to compare $RUN baseline:NAME, so three new flags here mean three more migrations or tombstones.
  4. Flag registry. cli_flags.py and the ratchet contract test (F1a, #254) are on main, but this PR does not register the new flags. They may be why Tests is failing; I have not checked the logs.
  5. Minor: the --max-* stem means a spend cap elsewhere (--max-usd). These flags are score tolerances, so check the name against the F1c stem rule.

Suggestion: keep this as a stopgap if (a) the names and meanings match the policy.v1 fields that gate will read, so the later move is mechanical; (b) invalid exits 4 and inconclusive exits 3, with 1 only for a real regression; (c) the flags are registered in cli_flags.py; and (d) a line in the changelog or docs says that #94 moves these to gate.

The tasks run incomplete-grid half fits G3, subject to the same 1 vs 3/4 question for errored or missing cells.

vchamarthi and others added 6 commits October 30, 2026 23:02
Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
A command that exited 126 or 127, or was killed by a signal, never produced a
verdict, and a reward file that cannot be read is not an absent one. Both stay
unscored so tasks run reports them instead of reading them as a pass or a fail.

Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
A task or harness listed twice used to run both cells and then fail the run as
incomplete. It now exits 2 before anything runs. The cell state reuses the
runner's number check, so the exit code and the summary agree on what is scored.

Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
… evidence exits 2

--max-context-drop, --max-lift-drop and --min-matched join the CLI flag
allowlist. --min-matched alone is a usage error instead of a silent no-op.
Partial or insufficient evidence is an input the user can fix, so it exits 2;
a regression and a model mismatch keep exit 1. The fragment and docs say so,
and avg_reward of command harnesses changing is noted.

Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
The two functions were over the complexity baseline. The smoke job comment is
cut to two lines as the comment-block check requires.

Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
The repeat check compared --tasks strings as typed, while cells and
output dirs are keyed by the loaded task name. A name plus its path, a
trailing slash, or two tasks sharing a directory name ran every cell,
then exited 1. Check the loaded names too, before anything runs.

Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
Exit 126/127, a signal exit or an unreadable reward file left reward
None but passed from the exit code, so task_runs counted the cell as a
scored pass or failure in pass_rate and the paired test. Mark it
solver_error, which the summary already leaves out.

Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
@xaleryb

xaleryb commented Oct 31, 2026

Copy link
Copy Markdown
Contributor
  • Ran: the test suite, lint, types, security and the benchmark gate on the head merged with current main, plus the CLI inputs from the open threads.
  • tasks run --tasks toy-task,<root>/toy-task (also toy-task,toy-task/, and two different */toy-task dirs) -> every cell runs, then exit 1 1 duplicate; the shared-basename case writes both cells into one output dir -> expected: exit 2 before anything runs, the same as toy-task,toy-task.
  • command:sh -c "exit 127", a signal exit, or an unreadable reward.txt -> stderr says unscored, but task_runs.json counts the cell as scored (pass_rate 0.0, or 1.0 when the unreadable file comes with exit 0) -> expected: errored, out of pass_rate and the paired test.
  • Pushed: 592dd42 checks the loaded task names for repeats as well as the raw strings; 97e5dfd marks a no-verdict command cell solver_error, with docs and changelog updated.
  • Nothing is left for the author.

Comment thread dokimi/dokimi/commands/baseline.py Resolved
Comment thread dokimi/dokimi/harnesses/command.py Resolved
A reward file that failed to parse fell through to a reward: x marker on
stdout or stderr, so the cell passed instead of being errored as documented.

Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
pair() stringified numbers, so a schema-invalid identity of 123/456 on both
sides verified the model and let the regression gate print PASS.

Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
@xaleryb

xaleryb commented Oct 31, 2026

Copy link
Copy Markdown
Contributor

Two new claims on 97e5dfd, both reproduced and fixed:

  • d010a4d: a reward file that fails to parse no longer falls through to a reward: x marker on stdout/stderr; an empty or out-of-range reward.txt next to reward: 1 output is now an errored cell, not a pass. Docs and changelog say so.
  • f19aff1: a non-string provider or model counts as not recorded, so 123/456 on both sides no longer verifies the model; the regression gate and --strict exit 1 with "cannot verify". Covers both the answer and judge roles.

Comment thread dokimi/dokimi/harnesses/command.py Resolved
…illed

With no reward file, a `reward: 1` on stdout or stderr scored exit 126,
127 or a signal exit as a pass. Read output markers only after an exit
that finished; a readable reward file still outranks such an exit.

Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
Conflicts in commands/baseline.py: keep intel#273's same-model rule (raw
panel strings, model_identity) with this PR's regression-gate policy
and non-string identity rule; growth_baseline.json keeps both caps.

Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
@xaleryb

xaleryb commented Oct 31, 2026

Copy link
Copy Markdown
Contributor

Pushed 3af1472 for the exai-bot finding on command.py:161: with no reward file, a reward: x on stdout or stderr is no longer read after exit 126, 127 or a signal exit, so the cell is errored as the docs say. A readable reward file still decides the cell, and a printed reward followed by exit 1 still scores. Then merged main (#273, #301) in 7b6da2e: in baseline.py the same-model rule and the regression gates both stay.

Comment thread dokimi/dokimi/harnesses/command.py Resolved
@napetrov

Copy link
Copy Markdown
Contributor

Review: re-review since 1de60da (head 7b6da2e)

This PR makes tasks run and baseline compare refuse incomplete evidence. A command cell with no verdict (exit 126/127, a signal exit, or an unreadable reward file) is errored instead of scored, a task or harness listed twice exits 2 before running, and baseline compare gets opt-in score-drop gates where a regression exits 1 and insufficient evidence exits 2. Since 1de60da it adds: a printed reward: no longer counts for a command that never ran or was killed, a reward file that yields no number blocks the output fallback, repeated tasks are caught after name resolution, --min-matched needs a gate, and the CI smoke comment is shortened.

Findings:

  • nit: dokimi/docs/coding-harnesses.md (command harness rule) and the changelog say "a reward file it cannot read". The code also errors a readable file that holds no number: probe with reward.txt = PASS at exit 0 gives reward None and solver_error: no verdict: exit 0, reward file unreadable (intel/main gave passed=True with reward None). Fix: say "a reward file with no reward in it" in both places, and reword the solver_error text to match.

Coverage: checked the 10 commits since 1de60da (round diff, main merges excluded), a reward-path probe on head and on intel/main, the 6 touched test files (384 passed), and ruff F-rules on changed .py files. Not checked: the full test suite, Docker and native Windows runs. test_agent_runner.py and test_arm_runner.py fail with ModuleNotFoundError: litellm on both head and intel/main in this environment, so they are not a signal about this PR.

No blocking findings.

@vlad-perevezentsev vlad-perevezentsev 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 7b6da2e. Every point from my first round is fixed. The reward-file rule, the repeated-task refusal, --min-matched, the flag registry and the changelog all hold, and the new tests catch the bugs they cover. What is left is on Windows: a crashed command still counts as a verdict there, and the new CLI test fails there. Both are in the line comments.

Agreed with @napetrov on the doc wording: coding-harnesses.md and the fragment say "a reward file it cannot read", but a readable reward.txt that holds PASS is no verdict too. On the 3/4 exit codes from #94 I disagree for this PR: exit_codes.py on main has only 1 and 2, and the PR follows it (2 for partial evidence, 1 for a regression or a model mismatch). The move to gate is noted at benchmarking-and-comparison.md:838.

Agreed with @exai-bot on the Windows exit statuses (I added a repro in its thread) and on the changelog wording for the exit-code fallback.

What I ran, at 7b6da2e:

  • make ci-local --offline (lint, types, tests, coverage of changed lines, docs): 19 passed, 0 failed, 13 skipped.
  • This head merged with current main: the PR's test files plus test_cli_flags.py and test_growth_ratchet.py, 700 passed.
  • Mutation: with the reward-file check removed from _exit_code_reward, 3 tests in test_harbor_results.py fail.
  • On Windows (Server 2025, Python 3.13): pytest tests/test_harbor_results.py tests/test_tasks_command.py tests/test_baseline_compare.py: 181 passed, 2 failed. Both failures are cases of test_command_harness_exit_code_verdict_is_a_complete_run, a test this PR adds. A real os.abort() there exits 0xC0000409 and is scored, while on Linux it is no verdict: exit -6.

Checked, no issues

  • No-verdict rule: exit 126, 127 and signal exits ignore a printed reward, a readable reward file still decides, and an unreadable one blocks both the exit code and the output marker. On Linux, a real segfault and a real abort are errored cells.
  • Repeated tasks: tasks run --all cannot repeat a name, since task names are directory names one level under the root. --tasks checks both the spelling and the loaded name.
  • Gate exits: partial evidence and --min-matched without a threshold exit 2, a regression exits 1. Merging #273 kept the same-model rule and the stricter pair().

Comment thread dokimi/tests/test_tasks_command.py Resolved

@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 7b6da2e. The delta fixes everything raised in the last round, but one command-harness hole is still open: a crash on Windows is scored as a verdict. One nit inline, and two small ones below.

Open bot points I confirm:

  • I agree with exai-bot and @vlad-perevezentsev that a Windows crash status still counts as a verdict: _never_finished stops at 192, so an exit such as 3221226505 (0xC0000409) is recorded as reward 0.0, or as a pass with reward: 1 printed, and tasks run can exit 0. New evidence: with the fake runner, exits 3221226505, 3221225786 and 3221225477 each give solver_error=None, while 192 gives no verdict: exit 192.

Other points:

  1. Nit: tests/test_tasks_command.py:249 and three neighbours pass no --output-dir. I agree with @vlad-perevezentsev on the quoted-harness test. New evidence: test_unsupported_dry_run_preserves_artifacts, test_partial_suite_writes_remaining_results_before_refusing and test_native_completion_preserves_grade_failures also leave results/task-runs/toy-task/... in the working directory.
  2. Nit: docs/coding-harnesses.md and the changelog fragment say "a reward file it cannot read". A readable reward.txt holding PASS at exit 0 also gives reward=None and solver_error: no verdict: exit 0, reward file unreadable. I agree with @napetrov and @vlad-perevezentsev on the wording; say "a reward file with no reward in it" and reword the solver_error text.

What I ran, on a Linux review host at 7b6da2e:

  • make ci-local (with Docker): 35 passed, 0 failed, 7 skipped.
  • Mutation run of the new tests over command.py, tasks.py and baseline_regression.py: 91 mutants, 25 survived the three new test files. The survivors that decide a verdict are in the inline comment and in the exit-code edges below.
  • Reproductions with the LLM transport mocked.

Checked, no issues

  • Completion check: every _check_completion mutant is killed (missing, duplicate, unplanned, errored, unscored cells, empty plan, repeats).
  • No-verdict rule: 126, 127, -1, 129 and 192 are errored cells; 125, 128, 193 and 255 stay scored. A readable reward file still decides, and an unreadable one blocks the output marker.
  • Repo rules: all 16 non-merge commits carry Signed-off-by; cli-interface.json, the flag allowlist and the CLI snapshot test agree with the parser.
  • Gate exits: regression and model mismatch exit 1, partial evidence and --min-matched without a gate exit 2, matching the docs.
Bot points I checked that no longer apply at this head (1)
  • exai-bot's points on non-string identities, on an unreadable reward file with a printed marker, and on exit 126, 127 or a signal with a printed marker do not apply: each is fixed at this head and pinned by a test.

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

@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 3d350ed, which fix the three nits from my review:

  • 4fff339: tests/test_baseline_compare.py:375: added tests for the mean drop (two questions down 1), an improvement larger than the limit, and an empty baseline and current exiting 2.
  • b002f48: tests/test_tasks_command.py:190: the quoted-harness test, _run_tasks (used by the native-completion and partial-suite tests) and the unsupported dry-run test now pass --output-dir under tmp_path.
  • 3d350ed: docs/coding-harnesses.md:611: the docs, the changelog fragment and the solver_error text now say "a reward file with no reward in it" / "reward file has no reward".

make ci-local at 3d350ed: 35 passed, 0 failed, 7 skipped.

Not approving yet. Still open: @vlad-perevezentsev on Windows NTSTATUS exits (0xC0000409, 0xC000013A) are scored as a verdict by _never_finished (command.py:404); also raised by exai-bot.

@vlad-perevezentsev vlad-perevezentsev 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 3d350ed. The new commits fix the wording of the reward-file rule, keep the tests out of the cwd and pin the gate's mean and sign. Both Windows points from my last round are still open: an NTSTATUS crash exit is scored, and the quoted-harness test fails because _safe_id keeps the quote.

Agreed with @exai-bot on openai/unknown passing as a known model (_recorded_models returns it as recorded); on the float threshold I disagree and replied in its thread.

What I ran, at 3d350ed:

  • make ci-local --offline (lint, types, tests, coverage of changed lines, docs): 18 passed, 1 failed, 13 skipped. The failure is schema_compat, which reports schema files removed that this PR does not touch: drift between main and the PR base, not from this PR.
  • This head merged with current main: the PR's test files plus test_cli_flags.py, test_cli_surface.py and test_growth_ratchet.py, 768 passed.
  • On Windows (Server 2025, Python 3.13): pytest tests/test_tasks_command.py tests/test_harbor_results.py: 112 passed, 2 failed. Both failures are cases of test_command_harness_exit_code_verdict_is_a_complete_run, still with WinError 123 on the quote in the cell directory name.

A command that exits with an NTSTATUS error code (0xC0000005, 0xC000013A, 0xC0000409) is an errored cell, as a signal exit is on Linux, instead of a scored 0.0 or a pass.

Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
…d cell directory

_safe_id also replaces the double quote, backslash, <, >, |, ? and * so a quoted command: harness can run on Windows.

Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
The --max-context-drop and --max-lift-drop values are parsed as Decimal, so a drop equal to the limit passes at any precision. A model id that is only unknown after its route (openai/unknown) is not recorded.

Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
Comment thread dokimi/dokimi/harnesses/runner.py Resolved
…s in separate directories

_safe_id appends a digest of the id when it replaces a character Windows refuses, so command:echo "a?" and command:echo "a*" no longer share and clear one cell directory.

Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>
Signed-off-by: vchamarthi <123653452+vchamarthi@users.noreply.github.com>

@vlad-perevezentsev vlad-perevezentsev 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.

LGTM

I checked 3d13b56. Every point from my earlier rounds is fixed. On Windows, a crash status is no verdict and a quoted command: harness gets a valid cell directory; harness ids that differ only in characters Windows refuses get a digest suffix, so they keep separate directories. openai/unknown is no longer a known model, the threshold is exact, and the CLI snapshot is current.

Merge: safe now: it merges cleanly into current main, 18 commits behind, and its tests pass on the merge. Once it is in, #325 conflicts with it (journeys.md, journeys.yaml) and needs a rebase.

What I ran, at 3d13b56:

  • This head merged with current main: the PR's test files plus the CLI flag, CLI surface, docs-refs, growth and runner tests, 849 passed.
  • At the head: test_cli_flags.py, test_tasks_command.py, test_runner_run.py, 66 passed.
  • On Windows (Server 2025, Python 3.13), at 1e4cc90: pytest tests/test_tasks_command.py tests/test_harbor_results.py tests/test_baseline_compare.py: 204 passed. A real os.abort() with reward: 1 printed is no verdict: exit 3221226505.

Checked, no issues

  • Cell directories: an id without "<>|?*\ keeps its old name, so existing results paths do not move; an id with them gets an 8-character digest of the original id.
  • Exit codes: >= 0xC0000000 never matches on POSIX, where codes are 0-255 or negative, so Linux behaviour is unchanged.

@vchamarthi
vchamarthi merged commit d32c4d6 into intel:main Nov 2, 2026
18 checks passed
@vchamarthi
vchamarthi deleted the fix/task-completion-baseline-gates branch November 2, 2026 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cli-interface-change Changes dokimi's user-facing CLI surface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants