[napetrov]

Part of #121 (item 1).

Summary

Before: a run's completeness came from whatever rows ended up in its artifact. A run that finished 2 of its 6 planned cells could still pass a gate built on per-harness counts.

After: completeness is computed from what was planned.

plan.v1   cells: [arm x model x task x rep, stable cell_id] + subjects, suites, grader, varied_variable, budget
   │  plan_digest (sha256 of the canonical plan; generated_at is left out)
   ▼
result.v1 cells: [{cell_id, status ∈ pass|fail|inconclusive|invalid|infra_error|not_run, model}]
   │
   ▼
runner.plan.completeness(plan, result) -> complete | incomplete (missing ids) | invalid (problems)

• plan.v1 records subject SHA and fingerprint, suites (hash plus selection reasons), requested models and harnesses, grader id and digest, one varied_variable (a string, or a list for a 2×2 design), planned cells, and a budget (cells, which must equal the number of planned cells, plus estimates agent_calls, cost_usd, wall_sec). A cell's arm carries the harness when a plan crosses harnesses. completeness also refuses a plan cell whose id does not match its coordinates, and an invalid result reports no statuses.
• result.v1 has plan_digest, an optional execution_id, and one status per cell. A cell can also record the resolved model (the feat(repo): record the model that answered each cell #119 per-cell record).
• completeness only counts cells. A cell is measured only when its status is pass, fail or inconclusive; not_run, infra_error, invalid and absent cells are listed in missing. The result is invalid when its digest does not match the plan, when it has a cell the plan does not, or when it gives one cell two statuses. Whether a complete run passes is left to the gate (J11/J12: distinct inconclusive verdict and blocking policy (AC-J11-002, AC-J12-002) #99).
• scripts/check_smoke_runs.py declares its planned cells (both harnesses on the one workflow task) and runs them through completeness. A missing or errored cell now fails the gate even when the summary counts are clean.

Not in this PR:

• --plan on repo run / arms / run, which would write result.v1. The execution id is still open in feat(repo): --delegation and an execution id for repo run cells #107, and repo run would need its cell loop reworked. That wiring is a follow-up once feat(repo): --delegation and an execution id for repo run cells #107 lands.
• Compare and gate policy (J11/J12: distinct inconclusive verdict and blocking policy (AC-J11-002, AC-J12-002) #99).

Evidence

• make ci-local: 25 pass, 1 FAIL, 6 skip. The one failure is pip-audit (diskcache 5.6.3 PYSEC-2026-2447, ragas 0.3.9 PYSEC-2026-3046). It fails the same way on requirements.lock.txt with this change stashed, and this PR does not touch any lock file.
• pytest tests/test_run_plan.py tests/test_check_smoke_runs.py tests/test_schema_compat.py tests/test_import_architecture.py tests/test_audit_test_quality.py: 208 passed.
• New tests that fail on wrong behaviour:
  • 2 of 6 cells passing gives incomplete, with the 4 missing ids listed.
  • A not_run, infra_error or invalid cell keeps the run incomplete.
  • Editing the grader digest after the run gives invalid.
  • Unplanned or doubled cells give invalid.
  • An unknown status is rejected by the schema.
  • A smoke artifact that lacks the oracle row fails the gate with 1/2 measured (incomplete).
• The smoke script run directly on an empty artifact: cells: 0/2 measured (incomplete), both cells listed as not measured, rc=1.
• schema_compat vs the PR base: pass. The fixtures plan.v1.json and result.v1.json were written through save_artifact.

Merge danger

• Low. Two new artifact kinds, a pure module in dokimi.runner (it imports only the standard library), and a stricter smoke gate.
• The external-catalog-smoke job now fails when runs.json has a row for a task or rep outside its plan. The workflow runs one task once, so that is expected not to happen. The job needs Docker and was not run locally.
• plan_digest hashes the plan as written, including schema_version. A producer has to compute the digest from the saved (stamped) plan, not from the dict it passed to save_artifact.

🤖 Generated with Claude Code

[codecov-commenter]

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

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.77%. Comparing base (b226dba) to head (71fc9a6).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #161      +/-   ##
==========================================
+ Coverage   92.76%   92.77%   +0.01%     
==========================================
  Files         256      257       +1     
  Lines       32414    32515     +101     
  Branches     5421     5439      +18     
==========================================
+ Hits        30068    30167      +99     
- Misses       1561     1562       +1     
- Partials      785      786       +1     

Files with missing lines	Coverage Δ
dokimi/dokimi/artifacts.py	95.12% <ø> (ø)
dokimi/dokimi/runner/plan.py	100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

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

[antonwolfy]

Review at 67975e9. The completeness computation is sound for well-formed input, and the smoke gate maps real task_runs rows onto the plan correctly. Three things are worth fixing before merge, and there is one nit.

What I ran, on a Linux review host at 67975e9:

• make ci-local: 25 passed, 1 failed, 6 skipped. The failure is pip-audit; the PR touches no lock file, so it is not caused by this change. The Docker smoke job was skipped.
• Reproductions for the completeness findings, run directly against the module; the smoke-gate reproduction and a 27-mutant run against the new tests (6 survived; the three that matter are in the inline comments).

Checked, no issues

• Smoke gate mapping: a healthy artifact (repeat absent for one repetition, model=None, task name onetbb-histogram-local-aggregation) passes. Missing, extra, duplicated and wrong-rep rows each fail with a specific message.
• Schema versioning: both kinds are new, each has a fixture, SCHEMA_VERSIONS is updated, and schema_compat passes. Fixtures load through load_artifact, their cell_ids match cell_id() and the result digest matches the plan.
• Digest: generated_at and key order do not change it; a changed grader digest gives invalid.

[antonwolfy]

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

• 7852366: dokimi/dokimi/schemas/plan.v1.json:16: added additionalProperties: false to subject, suite, grader and budget objects, and to result cells in result.v1.json.

make ci-local at 7852366: 25 passed, 1 failed (pip-audit, same as at the reviewed head), 6 skipped.

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

[xaleryb]

• Ran: the head merged with current main, full suite, ruff, mypy strict, plus mutation of the malformed-cell guards in completeness.
• Growth ratchet: merging this head into main -> plan.py::completeness: 13 complexity is over the 10 cap and test_growth_ratchet fails -> expected green. This PR's CI ran on a base older than the ratchet.
• Digest trap: save_artifact("plan", plan, p), then result["plan_digest"] = plan_digest(plan) with every cell pass, then reload both -> invalid (digest mismatch) -> expected complete. plan_digest hashes schema_version, which save_artifact adds only to the copy it writes.
• Commits pushed:
  • 266a0aa: moves the status loop into _statuses(), so completeness is under the cap. No behaviour change.
  • 043179c: puts bad statuses on a planned cell and asserts the problem text. Covers a non-object cell and cells: None. Reverting any guard now fails a test.
• Left for the author: rebase onto main; choose a digest fix (default schema_version before hashing, or leave it out and regenerate result.v1.json); add a save-then-digest test.

[antonwolfy]

The three threads from my earlier review are fixed, and the malformed-cell handling in completeness now holds up. Two test gaps are worth closing before merge, both inline.

What I ran, on a Linux review host at 043179c:

• make ci-local: 25 passed, 1 failed, 6 skipped. The failure is pip-audit; the PR touches no lock file. The Docker smoke job was skipped. 6,748 tests passed.
• A mutation run of the new tests (about 40 mutants; the two that matter are inline).
• Reproductions for the digest point below, with the LLM transport mocked.

I agree with @xaleryb on the digest trap. New evidence: save_artifact("plan", plan, p), then result["plan_digest"] = plan_digest(plan) with six pass cells, reload both gives invalid (result is for plan sha256:7cd6c1…, not sha256:9ffebb…), and no test digests the pre-save dict. The digest choice waits for the author.

Checked, no issues

• Smoke gate mapping: task, harness and model come from TaskRunResult.as_dict; repeat is written only above 1, so absent means 1. A healthy artifact gives 2/2 measured (complete); missing, extra, duplicated and errored rows each fail. The script imports when run by path as CI does.
• Schema versioning: both kinds are new, SCHEMA_VERSIONS is updated, fixtures load and their digests match.

[antonwolfy]

Review at 19835fc. Both test gaps from my last review are closed: the list-valued cell_id case now fails when the continue in _statuses is removed, and test_a_probe_whose_connect_was_not_denied_fails fails when the :443 half of the egress check is dropped. Two nits inline, both test gaps with no effect on the gate's verdict.

I agree with @xaleryb's point on plan_digest hashing schema_version, which save_artifact adds only on save: it still reproduces at 19835fc, and the fix waits for the author.

What I ran, on a Linux review host at 19835fc:

• make ci-local: 29 passed, 0 failed, 7 skipped (the Docker smoke job among them). 8,230 tests passed, changed-line coverage check passed.
• A mutation run of the two test files against the new code (48 mutants, 11 of which pass both files; the two that matter are inline).
• Probes of completeness, plan_digest and the smoke gate with malformed input.

Checked, no issues

• Threads: the two earlier mutants (continue -> pass, :443 dropped) are both caught now.
• Schema versioning: plan and result are new kinds, SCHEMA_VERSIONS is updated, fixtures load, schema_compat passes.
• State logic: the digest check, duplicate and unplanned ids, the empty plan, measured=0 on invalid, and the MEASURED set are each pinned by a test that fails when the logic is changed.

[antonwolfy]

I pushed a commit per nit, up to aafab7c, which fix the two nits from my review:

• d9c464e: dokimi/tests/test_run_plan.py:84: asserts that dropping the last cell changes plan_digest.
• aafab7c: dokimi/tests/test_check_smoke_runs.py:110: asserts the printed output holds the problem line result has cells the plan does not.

make ci-local at aafab7c: 29 passed, 0 failed, 7 skipped.

Not approving yet. Still open: @xaleryb on plan_digest hashes schema_version, which save_artifact adds only on save, so a saved plan's digest differs from the one computed before saving.

[vlad-perevezentsev]

Adding to @xaleryb's point, which still reproduces at aafab7c: d = plan_digest(plan), then save_artifact("plan", plan, p), then completeness(load_artifact("plan", p), {"plan_digest": d, ...}) gives invalid even with every cell pass. The tests miss it because _plan() digests the dict that save_artifact returned. A fix that keeps the fixture digests valid is to hash the plan as it will be saved:

body = {k: v for k, v in plan.items() if k != "generated_at"}
body.setdefault("schema_version", SCHEMA_VERSIONS["plan"])

plus a test that digests the dict before save_artifact and checks the reloaded plan against it.

[napetrov]

Fixed in 935f6c0: plan_digest fills a missing schema_version (plan.v1) before hashing, so a digest of the pre-save dict matches the saved plan. Test: test_a_plan_digested_before_saving_matches_the_saved_plan digests the raw dict, saves and reloads both, and expects complete; test_the_plan_schema_constant_matches_artifacts pins the constant to SCHEMA_VERSIONS.

[vlad-perevezentsev]

The plan lists several harnesses, but a cell has no harness and cell_id hashes only (arm, model, task, rep). A plan that runs the arms skill and no-skill under both docker-claude and codex gets the same two ids twice, and completeness says plan repeats cell ids. The smoke gate gets around this by putting the harness into arm, which then no longer means the varied variable. Since plan.v1 cannot be narrowed after it ships, could you decide now: add harness to the cell and to cell_id, or document that arm includes the harness?

[napetrov]

Decided in 935f6c0: the arm carries the harness. plan.v1 arm is now described as the full condition the cell runs under; when a plan crosses harnesses the harness is part of the arm (e.g. docker-claude/skill), and there is no separate harness key on a cell. This keeps cell_id and the result fixtures as they are and matches what the smoke gate already does. A separate harness coordinate can come in plan.v2 if a consumer needs to group by it.

[vlad-perevezentsev]

An invalid result reports measured=0, but statuses and missing are still computed from its cells. A result for another plan with one pass cell gives {'state': 'invalid', 'measured': 0, 'statuses': {'pass': 1, 'not_run': 1}, ...}, so a report that shows statuses shows a pass from a run we rejected. Please return statuses={} (and maybe every planned id in missing) when there are problems, and pin it in test_an_invalid_result_reports_no_measured_cells.

[napetrov]

Fixed in 935f6c0: an invalid result now returns statuses={} and every planned id in missing. Pinned in test_an_invalid_result_reports_no_measured_cells.

[vlad-perevezentsev]

Nothing checks that a plan cell's cell_id equals cell_id(arm, model, task, rep). A plan edited by hand (arm renamed, old id kept) can have two cells with the same coordinates and different ids, and completeness calls it complete; a result keyed by recomputed ids, as check_smoke_runs.result_for builds them, is then invalid. Please add a problem when a plan cell's id does not match its coordinates.

[napetrov]

Fixed in 935f6c0: a plan cell whose cell_id is not cell_id(arm, model, task, rep) (or whose coordinates cannot be hashed) is a problem, state invalid. Test: test_a_plan_cell_whose_id_does_not_match_its_coordinates_is_invalid.

[vlad-perevezentsev]

budget.cells repeats the number of cells, but nothing checks that it matches: a plan with 36 cells and budget.cells: 6 validates and completeness ignores it. Either check it in completeness (a problem when it differs from the number of distinct planned ids) or say in the description that it is only an estimate.

[napetrov]

Fixed in 935f6c0: completeness checks it; a budget.cells that differs from the number of distinct planned ids is a problem (invalid), and the schema description says so. Test: test_a_budget_that_miscounts_the_cells_is_invalid.

[antonwolfy]

Review at 2949419. The two new commits are right: a plan or result whose cells is not a list is now invalid instead of reading as zero cells, and the closed-object typo test fails against open schemas. No new findings from me, but the points other reviewers raised are still open at this head, so this is not ready to merge.

I agree with @xaleryb and @vlad-perevezentsev on the plan_digest trap, and with @vlad-perevezentsev on the cell_id coordinate check and statuses on an invalid result. All three reproduce at this head.

@vlad-perevezentsev's questions on harness in the cell and on budget.cells wait for the author.

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

• make ci-local: 29 passed, 0 failed, 7 skipped (the Docker smoke job among them). Changed-line coverage check passed.
• Reproductions for plan_digest, completeness and budget with malformed plans, no model or network involved.
• A mutation run of the new tests (63 mutants against plan.py, the smoke script and both schemas); none of the survivors changes completeness verdicts.

Checked, no issues

• Last two commits: a non-list cells on either side gives invalid with a problem line; {}, "", 0, 5 and False do not raise.
• Schema versioning: plan and result are new kinds, SCHEMA_VERSIONS is updated, fixtures load, schema_compat passes.
• Smoke gate: a missing, errored, extra or wrong-rep row each fails; the three new gate tests fail on the PR's base.

[napetrov]

Updated at 586031d (merged current main in 66a7620).

• @xaleryb digest trap (save then digest): fixed in 935f6c0, plan_digest defaults schema_version before hashing; save-then-digest test added. Your other two points (rebase onto main, save-then-digest test) are done in the same round.
• @vlad-perevezentsev: statuses on invalid, cell_id coordinate check and budget.cells check fixed in 935f6c0; harness decided as part of the arm (documented in plan.v1). Details in the threads.
• @antonwolfy: all your threads were addressed earlier; nothing new open from you.

make ci-local: 27 passed, 1 failed, 7 skipped; the failure is test_probe_reports_a_writable_docker_socket (AF_UNIX path too long from the long worktree path on this host, not this change).

[antonwolfy]

Review at 586031d. The new commits hold up: plan_digest now matches a plan digested before save_artifact, an invalid result reports no statuses with every planned id in missing, and the coordinate and budget.cells checks work. One test gap in that new code is worth fixing before merge, and there is one nit; both inline.

What I ran, on Linux at 586031d:

• make ci-local: 28 passed, 0 failed, 7 skipped (the Docker smoke job among them). Changed-line coverage check passed.
• A mutation run of plan.py against test_run_plan.py and test_check_smoke_runs.py (41 mutants, 8 survived; the ones that change a verdict or a returned field are inline).
• Probes of completeness and plan_digest with malformed plans, and of the smoke gate with missing, extra, duplicated and wrong-rep rows, with no model or network involved.

Checked, no issues

• Digest: a dict digested before save_artifact and the reloaded plan give the same digest; generated_at and key order do not change it.
• Invalid result: statuses={}, measured=0 and every distinct planned id in missing hold for a repeated-id plan and for a bad cell id.
• Schema versioning: plan and result are new kinds, SCHEMA_VERSIONS is updated, fixtures load and their digests match, schema_compat passes.
• Smoke gate: a healthy artifact gives 2/2 measured (complete); missing, extra and duplicate rows each fail.

[vlad-perevezentsev]

Review at 021ad8f. The plan_digest, statuses, coordinate and budget.cells fixes from the last round hold up. I found three more problems, one in the code and two test gaps:

• completeness raises on a plan whose cells is a number. The coordinate check added in 935f6c0 loops over plan.get("cells") or [], so completeness({"cells": 5}, ...) raises TypeError: 'int' object is not iterable (also for 1.5 and True) instead of returning invalid. At 2949419 this was invalid.
• The status enum in result.v1.json is a second copy of plan.STATUSES. Nothing keeps the two in step, so a status added on one side only makes save_artifact and completeness disagree.
• No test covers an unknown egress on a non-probe row. Since the unreadable-sidecar test moved the unknown status onto the docker-probe row, limiting the check to probe rows by mistake would still pass every test.

I pushed a commit per problem, up to 6390266:

• 3535d62: dokimi/tests/test_run_plan.py: a test that the status enum in result.v1.json equals plan.STATUSES.
• 29b27c8: dokimi/tests/test_check_smoke_runs.py: the unreadable-sidecar test also puts an unknown egress on the docker-oracle row and checks that the gate fails and prints docker-oracle: egress unknown.
• 6390266: dokimi/dokimi/runner/plan.py: _plan_problems loops over the cells only when they are a list, as _ids does, so such a plan is invalid; the not-a-list test now also covers 5, 1.5 and True.

Each new test fails when the code it covers is broken. make ci-local --offline at 6390266: 17 passed, 0 failed, 13 skipped.

I agree with @antonwolfy on the rep gap in the coordinate check (dokimi/tests/test_run_plan.py:209), which is not fixed yet: with cell["rep"] replaced by 1 in _plan_problems, all 57 tests in test_run_plan.py and test_check_smoke_runs.py still pass at 6390266, because every plan in the tests has reps=1. I left that one for the author.

[antonwolfy]

Review at 6390266. The three new commits are right: a plan whose cells is a number is now invalid, the repeated-cell test pins planned, measured and missing, and the status-enum and unknown-egress tests fail when the code they cover is broken. One test gap from my earlier review is still open, so this is not ready to merge.

What I ran, on Linux at 6390266:

• make ci-local: 28 passed, 0 failed, 7 skipped (the Docker smoke job among them). Changed-line coverage check passed.
• A mutation run of plan.py and the smoke script against test_run_plan.py and test_check_smoke_runs.py (15 mutants on the changed code; the one that matters is below).
• Probes of completeness and the smoke gate with non-list cells on both sides and malformed cells, with no model or network involved.

Still open from my earlier review:

• dokimi/tests/test_run_plan.py:217: no test edits rep of a cell on a plan with reps > 1. Replacing cell["rep"] with 1 in _plan_problems (plan.py:126) still passes all 57 tests in the two files at this head. A cell renumbered to another rep with its old id kept would read complete. The rep = "x" assertion there only reaches the except branch, on a rep-1 cell.

Checked, no issues

• Non-list cells: 5, 1.5, True, None, a dict and a string on either side give invalid with a problem line and do not raise; the guard in _plan_problems is pinned by the parametrised test.
• Invalid result: the repeated-cell plan reports planned=6, measured=0 and each planned id once in missing; dropping the dedup in measured or missing fails the test.
• Smoke gate: limiting the unknown-egress check to probe rows, or dropping it, fails the unreadable-sidecar test.
• Schema versioning: plan and result are new kinds, SCHEMA_VERSIONS is updated, fixtures load, schema_compat passes.

[xaleryb]

• Ran: the head merged with current main (full suite, ruff, mypy, bandit), the earlier digest-trap input, and mutants of plan.py.
• DCO: 3535d62, 29b27c8 and 6390266 have no Signed-off-by, so the DCO check fails. Their author needs to re-sign them (git rebase --signoff on the branch).
• rep coordinate gap (open thread at test_run_plan.py:219): with cell["rep"] replaced by 1 in _plan_problems, all 57 tests in the two test files still pass. They should fail.
• Commit we pushed:
  • 5bd6906: a cell renumbered to rep 2 with its old id kept must be invalid. The test fails on that mutant.
• Left for the author: the sign-off on the three commits above.

[antonwolfy]

LGTM. The one commit since my last review closes the rep gap: a cell renumbered to rep 2 with its old id kept is now invalid, and the test fails when the check ignores rep. No new findings. As @xaleryb noted, 3535d62, 29b27c8 and 6390266 have no Signed-off-by, which the repo's DCO needs before merge.

What I ran, on Linux at 5bd6906:

• make ci-local: 28 passed, 0 failed, 7 skipped (the Docker smoke job among them). Changed-line coverage check passed.
• A mutation run of the new lines: replacing cell["rep"] with 1 in _plan_problems fails test_a_plan_cell_whose_id_does_not_match_its_coordinates_is_invalid at the new assertion (complete instead of invalid). Replacing it with 0, or dropping the id comparison, also fails. On unmodified code the only problem reported is does not match its coordinates.

Checked, no issues

• Schema versioning: plan and result are new kinds, SCHEMA_VERSIONS is updated, fixtures load, schema_compat passes.
• Delta: the commit touches only tests/test_run_plan.py (3 lines); no generated artifacts or growth-baseline files change.