[napetrov]

Summary

A zero delta with no p reads as "no effect". On a task set where both arms pass every paired task, it means the tasks had no room to move. On oneDAL a compile-check tool cut new build breaks from 5.9 to 0.4 per run with Haiku 4.5. With Haiku 5.5 and Sonnet 5.5, both arms passed 9/9, and repo compare would have shown that as no effect (#41, #65).

Real output for 3 tasks × 3 reps, where both arms pass every cell:

 guidance:raw vs guidance:none, model m-1
   measure   tasks   baseline  treatment  delta [95% CI]                   p  p_holm  up/down/tied
   gate          3      1.000      1.000  +0.000 [+0.000, +0.000]          -       -  0/0/3
   cost_usd      3      0.100      0.100  +0.000 [+0.000, +0.000]          -       -  0/0/3
   turns         3     10.000     10.000  +0.000 [+0.000, +0.000]          -       -  0/0/3
+  gate ceiling: both arms pass all 3 paired tasks; no headroom to show an effect

headroom(pairs)                      # gate and score rows only; others null
  both arms at top on every task  -> "ceiling"      top = 1.0 (gate) | task score.max (score)
  both arms at 0 on every task    -> "floor"
  >= 2 tasks, baseline at top on
    all but at most one           -> "near_ceiling" # a one-task gain has sign-flip p = 1
  else                            -> null

• p values and Holm's correction are unchanged; the flag is only a note.
• result.json now records score_max, so score rows know their top. Batches recorded before this change never get ceiling on score rows; they can still get floor.
• The rule is documented in the compare.py docstring and the contract's repo compare bullet. A changelog fragment is added.

Evidence

• Before: two arms that both pass every task show a zero delta, no p, and nothing else.
  After: tests/test_repo_compare.py covers the cases:

  • 6 parametrized gate cases: ceiling, floor, near_ceiling, a single task, mixed extremes and a normal case;
  • one score test: ceiling against score.max, an unknown max (cells disagree), and floor that includes a grader failure.

  64 passed. tests/test_repo_run.py asserts that result.json carries score_max.

• make ci-local: 0 failed, 19 passed, 4 skipped. The skipped jobs need docker or network, run only on PRs, or are semgrep.

Merge Danger

Door: two-way

The change adds one JSON field to the repo compare output and one to result.json; nothing is removed or renamed.

Blast Radius: small

Readers of repo compare --json see a new headroom key. Text output gains a line only under flagged comparisons.

🤖 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 91.68%. Comparing base (565f653) to head (050c642).
⚠️ Report is 1 commits behind head on main.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #114   +/-   ##
=======================================
  Coverage   91.67%   91.68%           
=======================================
  Files         223      223           
  Lines       27505    27562   +57     
  Branches     4525     4540   +15     
=======================================
+ Hits        25216    25269   +53     
- Misses       1620     1623    +3     
- Partials      669      670    +1     

Files with missing lines	Coverage Δ
dokimi/dokimi/repos/compare.py	99.68% <100.00%> (+0.04%)	⬆️
dokimi/dokimi/repos/runner.py	93.91% <ø> (ø)

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

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

[Copilot]

🟡 Changes recommended

Pooling legacy and current score cells can incorrectly report a ceiling despite unknown legacy maxima.

1 open issue

• Preserve unknown score maximums when pooling legacy and new batches · New

What changed in this PR

Adds headroom notes to repository comparisons to distinguish saturated task results from evidence of no effect.

Changes:

• Classifies gate and score comparisons as ceiling, floor, or near-ceiling.
• Records task score maxima for score comparisons.
• Adds tests, contract documentation, and a changelog entry.

File	Description
dokimi/​tests/​test_repo_run.py	Checks persisted score maxima.
dokimi/​tests/​test_repo_compare.py	Tests headroom classifications and text output.
dokimi/​dokimi/​repos/​runner.py	Records score_max in results.
dokimi/​dokimi/​repos/​compare.py	Computes and displays headroom flags.
dokimi/​docs/​repo-evaluation-contract.md	Documents headroom behavior.
dokimi/​changelog.d/​20261018_120000_napetrov_feat-repo-compare-ceiling.md	Announces comparison notes and metadata.

_{Review effort: Balanced}

[Copilot]

🟢 Approval recommended

The additive changes preserve statistical calculations, cover legacy-data handling, and have no identified blocking defects.

No open issues 🎉

1 resolved since last review

• Preserve unknown score maximums when pooling legacy and new batches

_{Review effort: Balanced}

[antonwolfy]

The rule is sound and the legacy-batch handling after c0c29af holds up, but the note misreads a regression, and the tests leave much of the new logic unpinned. I'd fix the inline risks before merge.

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

• make ci-local: 19 passed, 0 failed, 4 skipped; changed-line coverage gate passes.
• Reproductions for the code findings, with the LLM transport mocked (cells built with the _cell helper of test_repo_compare.py).
• A mutation run over headroom(), _task_values, _headroom_note, the _compare_model wiring and the runner.py score_max: 36 mutants, 16 of which pass test_repo_compare.py and test_repo_run.py (99 tests).

Test gaps (the mutants that survive):

1. The tolerance and the floor condition are unpinned. value >= top without EPS, value >= top - 0.5, and a floor that checks only the baseline, only the treatment, or uses or all pass. Every fixture uses exact 0, 1 or score_max values. Add a case with a mean of reps just under the top (for example 0.6 of 1) that must stay None, and a floor case where only one arm is at 0.
2. len(pairs) >= 2 is unpinned. Changing it to >= 1 or >= 3 passes. Add a one-task comparison with the baseline at the top and the treatment below: it must be None.
3. headroom on non-primary rows is unpinned. if True else None at compare.py:257 passes, because the only check is cost 0.1, where headroom() returns None anyway. Use a cost of 1.0 on every task.
4. Pooling cells with different score_max, and the treatment's own top, are unpinned. max(maxes) instead of None, and using the baseline's top for both arms, both pass. The new regression test covers only the unknown-max case.

Checked, no issues

• Holm and p: headroom is added after paired_summary; p and p_holm are unchanged on a ceiling row.
• JSON: headroom is null on cost, turns and metric rows; gate and score are flagged separately in a mixed task set.
• Legacy batches: cells with and without score_max pooled for one task give None, as do cells with different maxima.
• Repo rules: the changelog fragment follows changelog.d/README.md; no versioned schema or with_docs / without_docs key is touched.

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

3 open issues

• The tops and failed lists are computed for every measure, even though they’re only used when… · New
• Indexing task.score[\"max\"] will raise KeyError if a score task is missing the max field… · New
• The expression not any(failed or ()) is correct but hard to read, especially since failed is… · New

_{Review effort: Lite}

[napetrov]

No change. Tasks are validated before they run: manifest.py _check_task (repo validate) requires score to be {metric, max} with a numeric max, and grading.py already indexes task.score["metric"] on the same path. A malformed task is rejected at validate time rather than silently treated as an unknown top here.

[antonwolfy]

Review at fce566b. The five points from my earlier review are fixed: a collapse no longer reads as near_ceiling, grader-failed pairs stay out of floor, differing tops give null, and the runner and contract are pinned. Two things are left: the near_ceiling note still misreads a grader failure, and the new grader-failure and tops logic is only partly pinned by the tests.

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

• make ci-local: 19 passed, 0 failed, 4 skipped.
• Reproductions for the code findings, with the LLM transport mocked (cells built with the _cell helper of test_repo_compare.py).
• A mutation run over the new code in headroom(), _compare_model and _task_values: 20 mutants, 9 of which pass test_repo_compare.py and test_repo_run.py (102 tests).

Checked, no issues

• Holm and p: unchanged; headroom is still only a note.
• Mixed sets: a gate ceiling and a score floor in one batch are flagged separately, and the note wording matches the data.
• Legacy cells: no score_max is never counted as the top; two legacy arms at 0 still give floor.
• Runner: task.score["max"] cannot raise, because the manifest loader rejects a score without max.

[xaleryb]

• Ran on the head merged onto current main: the full suite (5317 passed, 22 skipped), ruff, mypy strict and bandit, plus targeted repros and mutants of headroom().
• Agent outage read as floor: 5 gate tasks, every cell in both arms has agent.is_error true, no turns and a graded fail -> headroom: "floor", "both arms fail all 5 paired tasks; no headroom" -> expected null, the same reasoning as the grader-failure exclusion. Counting every is_error as unmeasured would be too broad, because errors after an attempt are scored. is_error with num_turns None or <= 1 is one option; naming the error count in the note is another. Your call.
• Pushed:
• 261cfa5: any grader failure gives null. At score.max <= 0, an all-failed task read as ceiling.
• 2025887: a pair counts as at the top only if both arms tie. Before this, 90 vs 80 on max 80 (p 0.008, 8 up) was flagged ceiling.
• 9b0c90e: pins EPS, the treatment side of floor, the two-task minimum, a small loss, one known top per task, and no flag on cost/turns.

[antonwolfy]

Review at 9b0c90e. The three fixes since fce566b are right: any grader failure gives null, a pair counts as at the top only when both arms tie, and the new tests pin EPS, the treatment side of floor, and the two-task minimum. Two points inline: a test gap in the tops wiring, which I'd fix before merge, and a nit on the contract text.

What I ran, on a Linux review host at 9b0c90e:

• make ci-local: 19 passed, 0 failed, 4 skipped; changed-line coverage 100% (35 lines).
• A mutation run over headroom() and the _compare_model wiring: 30 mutants, 7 of which pass test_repo_compare.py and test_repo_run.py (121 tests). Most are equivalent or sub-EPS; the tops wiring below is the one that matters.

Checked, no issues

• Earlier points: any(failed) vs all(failed), failure on either side, >= 2, base_top.count(False) <= 1, the treatment-not-below check and the at_top EPS are all killed by the suite.
• Past score.max: 90 vs 80 on max 80 gives None; 90 vs 90 gives ceiling. NaN scores and a NaN or inf score_max give no flag.
• Runner: score_max is written from task.score["max"], which the manifest check requires to be numeric.
• Repo rules: the changelog fragment matches the code; no versioned schema covers result.json or the compare output.

[antonwolfy]

Review at 6ffb878. Both threads from my last review are fixed: the tops wiring is pinned by the new floor test, and the contract sentence reads correctly. One test gap is left, and one nit on the changelog.

What I ran, on a Linux review host at 6ffb878:

• make ci-local: 19 passed, 0 failed, 4 skipped; 5,210 tests; changed-line coverage gate passes.
• A mutation run over headroom(), the _compare_model wiring, _task_values and the runner.py score_max: 53 mutants, 10 of which pass test_repo_compare.py and test_repo_run.py (122 tests). Six are equivalent or differ only at float noise; the four that matter are inline.
• Offline reproductions of headroom() on unknown tops, with the LLM transport mocked.

Checked, no issues

• Earlier points: the treatment's top in floor, any(failed), the two-task minimum, count(False) <= 1, the tops-mismatch guard and the runner.py score_max link are all killed by the suite.
• Holm and p: unchanged; headroom is only appended to the row, and is null on cost, turns and metric rows.
• Repo rules: the changelog fragment is in the right place and format; no versioned schema covers result.json or the compare output.

[xaleryb]

• Re-ran every earlier point's input at c1413de, plus the full suite, ruff, mypy, bandit and the strict docs build.
• Still open: 5 gate tasks, every cell in both arms with agent.is_error true, num_turns None and a graded fail -> headroom: "floor", "both arms fail all 5 paired tasks; no headroom to show an effect" -> expected null. The agent never ran, the same reasoning as the grader-failure exclusion. Which errors count as unmeasured is your call.
• Pushed 40802bd: since 2025887, a pair is at the top only when both arms reach it and tie. The docstring, contract and changelog still said "at the top", so 1.0 vs 1.3 on max 1 read as ceiling/near_ceiling in the docs and gives null in the code.
• Static checks fail on the missing license headers in errors.py/host_spec.py, which come from main. fix: add the missing license header to two repos modules #144 fixes them; update the branch after it lands.

[antonwolfy]

LGTM. Both threads from my last review are fixed: the tolerances are pinned at their boundaries and the changelog states the tops rule correctly. One nit inline.

What I ran, on a Linux review host at 40802bd:

• make ci-local: 24 passed, 0 failed by the PR; changed-line coverage gate passes. The two gitleaks jobs only fail to download the tool; a direct gitleaks run over the PR's commits finds no leaks.
• A mutation run over headroom(), the _compare_model wiring, _task_values, _headroom_note and the runner.py score_max: 35 mutants, 3 of which pass test_repo_compare.py and test_repo_run.py (156 tests). The three are equivalent or differ only at the EPS float boundary. The four mutants from my thread (top - 0.1, baseline - 0.1, < 0.5, gate top 0.5) all fail now.
• Offline reproductions of headroom() and repo compare on pooled, legacy, mixed-top and multi-model cells, with the LLM transport mocked.

@xaleryb's question on agent-outage cells read as floor waits for the author.

Checked, no issues

• Holm and p: unchanged; headroom is null on cost, turns and metric rows and is the only new key on a compare row.
• Legacy cells: no score_max, or a mix of recorded and missing, or differing maxima, never give ceiling; no reader of result.json rejects the new key.
• Docs and rules: the docstring, contract bullet and changelog fragment match headroom() for ceiling, floor, near_ceiling and the null cases; the fragment follows changelog.d/README.md; no versioned schema is touched.

[napetrov]

On the agent-outage point: fixed in b8cbee4. headroom() now returns null when a paired cell is an agent outage (agent.is_error true and num_turns None or <= 1), the same unmeasured exclusion as a grader failure. test_an_agent_outage_is_unmeasured_not_a_floor (all cells is_error, turns None or 1, graded fail -> null; 5 turns still reads floor) fails on the old code. Contract doc and changelog fragment updated. This supersedes the earlier "left as decided" note.

[xaleryb]

• Re-ran every earlier input at b8cbee4 (head and head merged onto main), plus the full suite, ruff, mypy, bandit, end-to-end repo run cells with a fake runner, and mutants of _agent_outage.
• Still open: if the harness exits 1 with no stdout, the cell records is_error: false, num_turns: null and a graded fail -> headroom: "floor" on the gate and score rows -> expected null, because the agent never ran. _agent_outage needs is_error true, and dropping that check passes the suite. One option is num_turns None and (is_error or rc != 0) and not timed_out. That changes the documented rule, so it is your call.
• Pushed 04529dc: an error cell whose num_turns is not a number ("1") raised TypeError and aborted the whole repo compare. It now counts as no turn count, as in the turns row.
• Pushed 2b77ba4: the module docstring now names the agent-outage exclusion, as the contract and changelog do.

[napetrov]

On the exit-1 harness with no stdout (xaleryb): fixed in 0eb5eae, as decided by the owner. _agent_outage now also counts a cell whose harness exited non-zero (agent.rc) with no turn count and was not killed at its timeout, so headroom is null for gate and score rows instead of floor. A timed-out agent, an exit 0, a missing rc and a cell that reported turns keep their old reading. Test: test_a_harness_that_exits_non_zero_with_no_result_is_an_agent_outage (fails on the old code for rc 1 and 137). Docstring, contract and changelog fragment updated.

[antonwolfy]

Review at 0eb5eae. The agent-outage rule from b8cbee4 and 0eb5eae does what the contract says, and the earlier threads' fixes still hold. One test gap in the new rule is worth closing before merge, and one nit on wrapping.

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

• make ci-local: 25 passed, 1 failed, 5 skipped; 6,687 tests; changed-line coverage gate passes. The one failure is pip-audit, reporting a new gitpython 3.2.0 advisory (CVE-2026-114145, fix in 3.2.1) in the existing locks; it is not caused by the PR.
• A mutation run over _agent_outage() and its wiring in _task_values: 16 mutants, 3 of which pass test_repo_compare.py (107 tests). The three are inline.
• Reproductions of rc, turns and timed_out combinations through compare, with the LLM transport mocked.

Still open from my earlier review:

• dokimi/docs/repo-evaluation-contract.md:858-861: the paragraph was re-wrapped, but the outage clause added in b8cbee4 / 0eb5eae brought back lines of 86, 90 and 104 characters, against 70 to 79 around them. Re-wrap from "else null; a grader failure" to "A pair".

Checked, no issues

• Outage rule: an error result with no turn count or at most one turn, and a non-zero exit with no turn count that is not a timeout, both give null on gate and score rows. Killing the is_error check, the not turns checks, not timed_out, the or join, any over reps, and the _task_values wiring each fail the suite.
• Holm and p: unchanged; grader_failed feeds only headroom().
• Docs: the module docstring, contract bullet, changelog fragment and headroom() docstring all name the same outage rule.

[xaleryb]

Re-checked at e077e13: every earlier input passes (error cells with no turns, harness exit 1/137/-9 with no result end to end through repo run, a grader failure at score.max 0, 90 vs 80 on max 80, 1.0 vs 1.3 on max 1, a non-numeric num_turns). The mutants from the open rc/bool/turn-limit thread now fail. The full suite merged onto current main passes, and ruff, mypy and bandit are clean. The red Tests jobs ran before #183 landed on main; they go green once the branch is updated.

[antonwolfy]

LGTM. The outage rule, the tops rule and the unchanged p and Holm all hold at this head, and the three open threads are closed. One nit inline.

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

• make ci-local: 29 passed, 0 failed, 7 skipped; 7,295 tests; changed-line coverage gate passes.
• A mutation run over headroom(), _agent_outage(), _task_values, _headroom_note, the _compare_model and format_compare wiring and the runner.py score_max: 46 mutants, 5 of which pass test_repo_compare.py and test_repo_run.py (176 tests). The five are equivalent, differ only at the EPS float boundary, or sit behind a guard that is dead code (rc is always an int).
• Reproductions through compare() and format_compare() with the LLM transport mocked: odd rc, num_turns and score_max values (str, float, bool, NaN, negative) raise nothing and serialise to JSON.

Checked, no issues

• Holm and p: 30 random multi-model batches with grader failures, errors and mixed turns give byte-identical compare() output at the base and at the head, apart from the new headroom key.
• Docs: the module docstring, the contract bullet, the changelog fragment and the headroom() and _agent_outage() docstrings all state the same ceiling, floor, near_ceiling, tops, grader-failure and outage rules as the code.
• Repo rules: no versioned schema covers result.json or the compare output; no new doc page; no generated files; the tests are offline.
• Readers: no other reader of result.json or of the compare output is affected by the new keys.

Copilot's point on task.score["max"] raising KeyError does not apply: the manifest check requires a numeric score.max before repo run starts.

[antonwolfy]

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

• 050c642: dokimi/dokimi/repos/compare.py:518: replaced the three-line comment with the one-line wording from the suggestion, which names the grader failure and the agent outage (see _agent_outage).

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