Skip to content

fix: nothing measured is not a pass, second batch (tasks run, novelty, run, compares) - #181

Merged
antonwolfy merged 9 commits into
intel:mainfrom
napetrov:fix/outcome-completeness-2
Oct 23, 2026
Merged

antonwolfy merged 9 commits into
intel:mainfrom
napetrov:fix/outcome-completeness-2

Conversation

@napetrov

Copy link
Copy Markdown
Contributor

Follow-up to #176 (independent of it: based on main, no shared code, merges cleanly in either order).

Summary

More places where nothing measured read as green. Each now says so, with counts, using the incomplete / NOT MEASURED / "not measured" wording from #176. An exit code is added only where the command already ends a run with a verdict; report-only lines get new wording and no exit code.

                          input                          main                                    now
tasks run                 every cell errored             "0/0 pass (n/a), N errored", exit 0     artifact + report written, then incomplete (0 scored, N errored of N cells), exit 1
tasks run                 some cells errored             counts, exit 0                          unchanged
novelty gate status       no check measured ({} record)  PASS (10 of 10 not measured)            NOT MEASURED (soft: still no exit code anywhere)
run                       selected doc not in load_docs  KeyError traceback                      one ❌ line, exit 1, no snapshot
run                       selected doc unreadable        PermissionError traceback               one ❌ line, exit 1, no snapshot
baseline compare          current eval empty/unscored    ✅ No notable changes vs baseline.      ⚪ Nothing to compare (0 scored) + record counts
baseline compare          scored, nothing moved          ✅ No notable changes vs baseline.      ✅ … (k of N questions compared)
compare (snapshots, .md)  base or candidate has 0 docs   ✅ No regressions                       ⚪ Nothing to compare (0 scored): no document in base/candidate

The changes are in commands/tasks.cmd_tasks_run, gate/novelty_gate.NoveltyVerdict.status, runner/run._load_found_docs (new DocLoadError, caught in commands/run.cmd_run), commands/baseline.cmd_baseline_compare and report/markdown_report.write_compare_report. Docs: coding-harnesses.md (the tasks run exit) and skill-novelty.md (the new status).

Still deferred: the run report's "minimal impact" on empty input, and the trust gate not counting unscored rows on partial input.

Evidence

Each new test fails with the source changes stashed and passes now:

  • tests/test_tasks_command.py::test_tasks_run_with_every_cell_errored_is_incomplete: every harness raises. Main exits 0; now exits 1 and still writes runs.json. …_with_some_cells_errored_keeps_exit_0_and_counts covers the partial case, which is unchanged (it passes on main too).
  • tests/test_novelty_gate.py::test_an_artifact_with_no_check_measured_is_not_a_pass: evaluate_novelty({}) is PASS on main and NOT MEASURED now; one measured check still gives PASS.
  • tests/test_runner_run.py::test_a_found_doc_that_did_not_load_is_a_clean_error: a KeyError and a PermissionError on main, DocLoadError now. …::test_run_command_exits_1_without_a_traceback_when_a_doc_does_not_load checks the CLI: exit 1, one line, no snapshot.
  • tests/test_baseline_compare.py::test_compare_with_nothing_scored_on_both_sides_is_not_no_change[unscored|empty], plus …_with_a_scored_pair_and_no_move_says_how_many_it_compared.
  • tests/test_runner_and_report.py::test_write_compare_report_with_no_scored_doc_is_not_no_regressions.
main:  8 failed, 110 passed   (the five files above)
now:   7097 passed, 2 skipped (full suite)

One existing fixture changed: the toy task in tests/test_tasks_command.py had no environment/ or tests/test.sh, so every dry-run cell errored and the two dry-run tests passed only because an all-errored run exited 0. The fixture now makes a task Harbor can run, so those tests still exit 0, for the right reason.

Local CI: 17 passed, 0 failed, 12 skipped (network or docker). That covers ruff check, mypy (222 files), deptry, bandit, zizmor, schema and registry checks, the import-architecture test, audit_test_quality.py, the changelog fragment, docs review triggers and mkdocs. The CLI surface diff is empty, and changed-line coverage is 100% (35 lines).

Merge Danger

Door: two-way. Every change is an exit code or a wording change for input where nothing was measured; a revert restores the old behaviour.

Blast Radius: gates

  • A job that runs tasks run where every cell errors (missing credentials, a broken image) now fails; before, it passed with nothing scored. Runs with some cells scored are unchanged.
  • Persisted value: the status of the novelty gate verdict (novelty.cells[].gate.status in subject_scorecard.v2, which does not enumerate it) can now be NOT MEASURED. No field is added or removed. A reader that switches on PASS/WARN/FAIL needs a case for it.
  • run over a tree where a selected doc cannot be read now exits 1 without writing a snapshot, instead of crashing with a traceback.
  • baseline compare and the snapshot compare Markdown change wording only; their exit codes are unchanged. A scraper of "No notable changes vs baseline." must allow the new (k of N questions compared) suffix.
  • Next to fix(gates): nothing measured is not a pass #176: both change commands/run.cmd_run, in separate hunks. git merge-tree shows no conflict either way.

Checklist

  • Tests added/updated and pytest passes
  • ruff, mypy, and schema checks pass
  • Docs updated (or N/A) — see the docs-drift checklist in CONTRIBUTING.md
  • No ad-hoc run artifacts committed
  • New files carry the SPDX license header (or are exempt in .github/.licenserc.yaml)

🤖 Generated with Claude Code

…, run, compares)

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
@codecov-commenter

codecov-commenter commented Oct 22, 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 92.70833% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.72%. Comparing base (5064dc2) to head (cc7397f).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
dokimi/dokimi/commands/baseline.py 83.33% 5 Missing and 2 partials ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #181      +/-   ##
==========================================
+ Coverage   91.64%   91.72%   +0.07%     
==========================================
  Files         222      223       +1     
  Lines       27409    27565     +156     
  Branches     4508     4536      +28     
==========================================
+ Hits        25118    25283     +165     
+ Misses       1620     1616       -4     
+ Partials      671      666       -5     
Files with missing lines Coverage Δ
dokimi/dokimi/commands/run.py 68.05% <100.00%> (+1.87%) ⬆️
dokimi/dokimi/commands/tasks.py 91.33% <100.00%> (+0.90%) ⬆️
dokimi/dokimi/gate/novelty_gate.py 98.93% <100.00%> (+0.01%) ⬆️
dokimi/dokimi/report/markdown_report.py 95.00% <100.00%> (+0.88%) ⬆️
dokimi/dokimi/runner/run.py 95.83% <100.00%> (+0.48%) ⬆️
dokimi/dokimi/commands/baseline.py 81.02% <83.33%> (+4.93%) ⬆️

... and 7 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 a1d4436. The tasks run guard, the NOT MEASURED status and the DocLoadError path hold up. Three things inline: one wrong headline in the compare report that I'd fix before merge, one test gap, and one nit.

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

  • make ci-local: the full pytest run passed (7078 passed, 21 skipped). The lint, type and docs legs (ruff, mypy, deptry, bandit, zizmor, mkdocs, pip-audit) did not run in my venv: make dev stops at the tools install on this base, which #179 fixed on main. Static checks, mypy and the strict docs build pass on GitHub's CI at this head.
  • Reproduction of the compare-report finding with the real compare_snapshots and write_compare_report, no LLM involved.
  • Mutations of the new code: the tasks run guard, the any in NoveltyVerdict.status, the not n_compared branch, both sides of the unscored filter and the DocLoadError handler are each caught by the new tests. The count in the baseline compare line is not (see inline).

Checked, no issues

  • Novelty status consumers: format_novelty_block, report/subject_report.py:_gate_status and subjects/novelty_track.py treat the status as an opaque string, and subject_scorecard.v2.json has no enum for it, so NOT MEASURED renders as NOT MEASURED (k not measured) and breaks nothing.
  • run on an unreadable doc: a doc with mode 000 gives one ❌ could not read ...: Permission denied line on stderr, exit 1, and no snapshot. Invalid UTF-8 does not raise (load_text replaces), and run_benchmark has no other caller.
  • tasks run counts: n and errored in per_harness match summarize_task_results, so errored == n means no cell has a verdict.

Comment thread dokimi/dokimi/report/markdown_report.py Outdated
Comment thread dokimi/dokimi/commands/baseline.py
Comment thread dokimi/docs/skill-novelty.md Outdated
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 5410417, which fix the nit from my review:

  • 5410417: dokimi/docs/skill-novelty.md:757: ended the sentence after "cleared one." and put the NOT MEASURED sentence and the warnings sentence on their own lines, as suggested.

make ci-local at 5410417: 19 passed, 9 failed (host), 7 skipped.

Not approving yet. Still open: the bug and the risk from my review.

The compare report put 'Nothing to compare' before the CRITICAL/WARN
headline, so a candidate that lost every document read as no regression.
The regression headline now comes first and the unscored note follows.
Also pins the 'N of M questions compared' count in baseline compare.

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…_tasks_run to hold the complexity ratchet

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
… the baseline

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…ss-2

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.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.

Review at 0d84e46. Both inline points from my earlier review are fixed. One test gap is left in the compare report, inline.

What I ran, on a Linux review host at 0d84e46:

  • make ci-local: 28 passed, 1 failed, 7 skipped; 7218 tests passed. The failure is pip-audit reporting gitpython 3.2.0 (CVE-2026-114145) from the lock files, which this PR does not touch.
  • Reproductions with the LLM transport mocked; the baseline compare refactor diffed against the base over 11 edge inputs (empty, unscored on either side, question missing from the baseline, duplicate ids, no evaluations key, malformed evaluations): exit codes and output are identical except for the two intended wording lines.
  • A mutation run of the new report and compare code: 12 mutants, one passes the full suite (inline).

Checked, no issues

  • Headline order: the CRITICAL/WARN line now comes first and the Nothing to compare line follows it; the empty-candidate case is pinned by test_write_compare_report_empty_candidate_keeps_the_critical_headline.
  • Question count: n_compared is pinned by the 1-of-3 test; using len(current_evals) as the numerator fails it.
  • Novelty status: format_novelty_block and subject_report print the status as an opaque string, and subject_scorecard.v2.json has no enum for it.

Comment thread dokimi/dokimi/report/markdown_report.py
The empty-candidate test covered only the CRITICAL headline, so narrowing
the guard to has_critical stayed green and dropped the "no document in
candidate" line on a WARN-level diff. Parametrize over both severities.

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

xaleryb commented Oct 23, 2026

Copy link
Copy Markdown
Contributor
  • Ran: the real CLI (tasks run, run, compare, baseline compare) on head and main, the full suite merged with current main, and mutants over the new guards.
  • Residual of the CRITICAL-headline thread: an empty candidate against a WARN-level diff was untested. Input: change if unscored and (has_critical or has_warnings) to if unscored and has_critical. Observed: all tests stay green, and compare drops the "no document in candidate" line under "Warnings detected". Expected: a test fails.
  • Commit we pushed: 99feff2 parametrizes test_write_compare_report_empty_candidate_keeps_the_headline over critical and warn, so that mutant now fails the [warn] case.
  • Left for the author: resolve the two answered threads on the compare headline and n_compared. Both fixes hold at the head.

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

Approved at 99feff2. Every earlier point re-run at this head passes and no must-fix or mid item is open; full suite, ruff and mypy clean on Linux; CI green.

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

LGTM. The warn-level empty-side test closes my earlier gap, and the rest of the PR holds up. One nit inline.

What I ran, on a Linux review host at 99feff2:

  • make ci-local: 28 passed, 1 failed, 7 skipped; 7219 tests passed. The failure is pip-audit reporting gitpython 3.2.0 from the lock files, which this PR does not touch.
  • Mutants of the new compare-report branch: dropping has_warnings, dropping has_critical, or to and, removing the unscored line, putting the elif before the headline, and wrong side lists are all caught. Two survive (inline, and the line order).
  • The new tests against the base modules: the five baseline compare and tasks run tests fail there and pass at the head.

Checked, no issues

  • baseline compare refactor: 12 edge inputs (empty, unscored on either side, disjoint ids, duplicate ids, non-list evaluations, delta without an arm score) give the same exit codes, stderr and tables as the base, except the two intended wording lines.
  • tasks run: the artifact and the report are written before the exit 1, and partial errors still exit 0.
  • Docs and changelog: NOT MEASURED and the tasks run exit match the code; no schema or nav change is needed.

Comment thread dokimi/dokimi/report/markdown_report.py
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.

LGTM. I pushed a commit per nit, up to cc7397f, which fix the nit from my review:

  • cc7397f: dokimi/tests/test_runner_and_report.py:278: the critical and warn compare-report tests now assert Nothing to compare is absent when both snapshots have documents.

make ci-local at cc7397f: 28 passed, 1 failed (pip-audit, same as at the reviewed head), 7 skipped.

@antonwolfy
antonwolfy merged commit 85afd57 into intel:main Oct 23, 2026
24 checks passed
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.

4 participants