Skip to content

feat(repo): compare records the start context, filters tasks, and flags inconclusive rows - #296

Open
antonwolfy wants to merge 9 commits into
intel:mainfrom
antonwolfy:feat/repo-compare-reading
Open

antonwolfy wants to merge 9 commits into
intel:mainfrom
antonwolfy:feat/repo-compare-reading

Conversation

@antonwolfy

@antonwolfy antonwolfy commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Found while evaluating IntelPython/dpnp#3099 (AGENTS.md guidance) with dokimi: 26 review_replay tasks, two arms (guidance:none and guidance:rev:<sha>), 3 reps, Claude Code, 156 cells. Reading that batch with repo compare ran into three gaps.

What the reader needed What main gives
Whether the guidance arm delivered its files Nothing per cell. The only evidence was the size of each run's first prompt, read by hand from every agent.stdout: median 19,380 tokens against 16,315 for guidance:none (each ±20 over 78 cells), +3,065. That matches the root AGENTS.md (7,015 chars) and none of the 7 nested files (21,199 chars), which load only lazily. result.json agent has whole-run token totals only, and repos/events.py reads the assistant messages but not their usage.
The 17 tasks reviewed before the first guidance commit vs the 9 after repo run has --task; repo compare has none, so every statistic of the split was recomputed by hand.
How to read recall +0.022 [-0.066, +0.100], p 0.62, 26 tasks Just the row. The report wrote "no measurable recall gain". With SD 0.222 for the per-task deltas, 80% power at a two-sided 0.05 detects only about ±0.12, (1.96 + 0.84) * 0.222 / sqrt(26): the result is inconclusive.
How to read an interval and a p that disagree Just the row. metric:n_findings +0.89, t interval [-0.091, +1.860], sign-flip p 0.0497: the report quoted "p = 0.049" for a descriptive row outside Holm's family, one of about a dozen comparisons. The score row over the 6 post-cut tasks no guidance commit named: +0.139, bootstrap [+0.028, +0.250], which excludes 0, while the exact sign-flip p is 0.25 (three non-zero deltas, all positive, so p cannot go below 0.25).

Related: #292 asks for an interval that agrees with the sign-flip p by construction; the disagreement note here is the stopgap until then. #290 asks repo compare to flag tasks a guidance revision may have seen; --task is how the rest get compared on their own.

Changes

  • repos/events.py: Transcript.first_prompt_tokens, the input + cache_creation + cache_read tokens of the first main-thread call (parent_tool_use_id null) of a Claude Code stream-json trace. It is read with dokimi.context.trace.parse_trace, so a call is counted once by message id and its blocks merge as context trace does; there is no second implementation. null when the trace has no such call, and for the text format.
  • repos/runner.py: result.json agent.first_prompt_tokens (null until set, like the other token fields).
  • repos/compare.py: first_prompt_tokens is a descriptive measure next to cost_usd and turns (per-task mean over the cells that have it). Like a process count it is compared only when both arms report it for some shared task: a batch written before the field has no row, not a row of zeros, and when only one arm has it a warning names it.
  • repo compare --task ID, repeatable (commands/repo_run.py, compare._select_tasks): compares only those task ids. An id no finished cell of the batches has is refused (exit 1, ✗ no finished cell of the compared batches has --task t9). The tasks left out are not listed as unpaired and their cells not as missing. --json records the selection as task_filter ({"tasks": [...], "of": <task count of the batches>}, null without --task), and the text header prints tasks: N of M (--task).
  • Reading notes under each comparison (compare._reading_notes), after the changed: and headroom notes. No number and no table column changes:
    • a gate or score row of 2+ tasks whose interval includes 0: score inconclusive: the interval includes 0; with this spread the smallest effect 80% power detects is about ±0.639 (4 tasks). mde = (1.959964 + 0.841621) * sd / sqrt(n) over the per-task deltas (sample SD) is in every --json row, null for one task or deltas with no spread;
    • a row whose interval and p disagree gets interval_p_disagree: true and a note. On the 6-delta case above it prints score: the interval [+0.028, +0.250] excludes 0 but p 0.25 is ≥ 0.05; they come from two methods, a percentile bootstrap, too narrow over few tasks, and a sign-flip test; read p. A descriptive row names the t interval instead and does not say "read p". On a gate or score row that has this note, the inconclusive note is not printed too;
    • descriptive rows with p < 0.05 are named on one line: p < 0.05 on turns: descriptive rows, their p uncorrected and outside Holm's family, which covers gate and score only.
  • Docs: contract §4 (agent.first_prompt_tokens) and §9 (the measure, --task, the three notes and mde), a line in docs/context-window.md, and the compare.py module docstring.
  • Tests: new tests/test_repo_compare_reading.py (test_repo_compare.py is 955 lines, near the 1000-line limit for a test file), one trace test in tests/test_repo_reach.py, and the agent-record tests in tests/test_repo_run.py. The fake harness in tests/repo_eval_fixture.py puts usage on its FAKE_USAGE assistant message.
  • Changelog fragment (Added).

Risk

  • result.json agent gains one key; readers that take the dict as it is see one more field. No schema covers this file.
  • repo compare --json rows gain mde and interval_p_disagree, and the summary gains task_filter. Every existing key and value is unchanged.
  • The text output gains note lines under some comparisons, and a first_prompt_tokens row for batches run from this change on. Older batches print as before apart from the notes.
  • repo run parses each Claude Code trace a second time (in context.trace) to get the first call. That is linear in the trace size and runs once per cell.
  • repos/compare.py grows from 696 to 769 lines (limit 800). Open PR fix(model): one same-model rule for baseline, self-judge and repo compare #273 also edits this file. This change edits only the parts above and reformats nothing around them; merged together, the file may need a split to stay under the limit.

Testing

On a Linux review host, at 30030ed (on main e966422):

  • make ci-local: 27 passed, 1 failed, 7 skipped. Skipped: external-catalog-smoke (needs docker), test-latest, docs deploy and pip-audit report (job conditions false on a PR), cli-surface packaging (unknown expression locally), conda-forge-deps (no micromamba), semgrep secrets (not installed). The failure is pip-audit: Found 10 known vulnerabilities in filelock 3.32.7 (fixes in 4.0.9 / 4.2.1). That version is pinned in main's lock, which this branch does not touch; ci(deps): document and ignore ten more filelock 3.32.7 advisories #288 documents and ignores those advisories. 10196 tests passed and 4 were skipped; changed-line coverage 100% (42 lines). The cli-surface step reports the one interface change: dokimi repo compare, added option --task.
  • Touched and related test files (test_repo_compare_reading.py, test_repo_compare.py, test_repo_compare_paths.py, test_repo_reach.py, test_repo_run.py, test_repo_process_counts.py, test_repo_uptake.py, test_repo_skill_arms.py, test_context_trace.py): 456 passed. Guard tests (test_docs_code_refs.py, test_growth_ratchet.py, test_audit_test_quality.py, test_import_architecture.py): 290 passed. ruff check on the changed files and mypy on the whole package (278 files) are clean.
  • At main, the new tests fail: test_repo_compare_reading.py stops at import (min_detectable does not exist), and test_first_prompt_tokens_is_the_first_main_thread_call_counted_once and three test_repo_run.py agent-record tests fail. All of them pass here.
  • Mutants: each was run on a copy of the commit against the test that pins it, and all 17 failed. An unmutated copy passed (179 tests). The mutants:
    • the trace read from every call instead of the main thread's;
    • context.trace's block merge dropped;
    • the runner not writing the field, or not defaulting it to null;
    • first_prompt_tokens dropped from _task_values, or from the both-arms rule;
    • for --task: the unknown-id check off, every cell kept, missing cells not filtered, the header line dropped, the CLI not passing the flag;
    • Z_POWER set to 0, or stdev swapped for pstdev;
    • the inconclusive condition inverted;
    • the disagreement test flipped;
    • "read p" dropped;
    • the descriptive-p test flipped.
  • Not run: a real repo run against a model, since this host makes no model calls. The dpnp numbers above come from that evaluation's batch, which is not on this host.
  • main merged in after fix(cli): run, compare, arms, tasks, explore and repo exit with a message on bad input or output #238 (cad52dd). The conflicts were in cmd_repo_compare, which now passes --task and also catches fix(cli): run, compare, arms, tasks, explore and repo exit with a message on bad input or output #238's CellFileError, and in _task_values, where the first_prompt_tokens list moved into fix(cli): run, compare, arms, tasks, explore and repo exit with a message on bad input or output #238's reading_cells block. compare.py keeps import math for min_detectable and is 770 lines. On that commit, the compare, run, reach and context-trace tests with the guard tests: 644 passed; ruff and mypy are clean on the merged files.
  • Merged together on main f0f95a2 with the other PRs from this evaluation (fix(repo): scan says why it is incomplete #295 at 4fa7783, feat(repo): compare records the start context, filters tasks, and flags inconclusive rows #296 at cad52dd, feat(repo): replay grades record each credited hit; repo report --hits lists them #297 at 0a7539a, fix(novelty): a capped run says how far the cap reached #299 at fa453d8), on a workstation: the full suite has 10482 passed and 57 skipped, and the same 5 failures and 1 collection error as main alone there (10456 passed). All six come from that machine's setup: no docker, no hypothesis, and two tests that depend on its git and shell settings.
  • Review round, 00f9b40 (test only): test_first_prompt_tokens_of_an_older_batch_is_no_row_not_zero now also checks a batch where only the baseline reports first_prompt_tokens, with the same warning and no unpaired entries. test_first_prompt_tokens_is_the_first_main_thread_call_counted_once gives the main call a third, sparser usage line after the full one. Before the change, two mutants passed these tests: the one-arm skip checking only the baseline arm (if not has[0]), and "last line wins" in context.trace's merge. Each now fails, and "first line wins" fails as well: 3 of 3 caught. An unmutated copy passes both files (100 tests). make ci-local: 25 passed, 3 failed, 7 skipped. 10528 tests passed and 4 were skipped; changed-line coverage 100% (43 lines). Two of the failures are gitleaks: its download got HTTP 504 four times, so Scan commits had no binary (exit 127). The third is pip-audit, the same filelock 3.32.7 finding as before; that version comes from main's lock. The skips are the same 7 as before.

Revert

git revert of the commit. Cells written in the meantime keep agent.first_prompt_tokens in result.json; readers before this change ignore the key.

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
  • Self-reviewed per "Before opening a PR" in AGENTS.md
  • No ad-hoc run artifacts committed
  • New files carry the SPDX license header (or are exempt in .github/.licenserc.yaml)

An evaluation of AGENTS.md guidance files (26 review_replay tasks, two
arms, three reps) showed three gaps in reading a batch with repo compare.

Start context: a batch did not say what each cell started with, so
whether an arm's guidance was delivered had to be read by hand from
every agent.stdout. The Transcript now carries first_prompt_tokens, the
input + cache_creation + cache_read tokens of the first main-thread call,
read with dokimi.context.trace (one call per message id, subagent calls
apart). repo run writes it as result.json agent.first_prompt_tokens (null
until set, and for a trace without such a call), and repo compare
compares it as a descriptive measure next to cost_usd and turns. Like a
process count, it is compared only when both arms report it, so a batch
written before the field has no row rather than a row of zeros.

Task selection: repo compare --task ID (repeatable) compares only those
task ids. An id no finished cell has is refused through CompareError;
the tasks left out are not listed as unpaired nor their cells as
missing; the summary records the selection as task_filter and the text
header prints "tasks: N of M (--task)".

Reading notes, with no number changed: a gate or score row of 2+ tasks
whose interval includes 0 gets an "inconclusive" note with mde, the
smallest effect 80% power detects at that spread,
(1.959964 + 0.841621) * sd / sqrt(n) (mde is in every row of --json);
a row whose interval and sign-flip p disagree gets interval_p_disagree
and a note naming the two methods, saying for gate and score that p is
the one to read; descriptive rows with p < 0.05 are named on one line as
uncorrected and outside Holm's family.

Contract sections 4 and 9, docs/context-window.md and the compare
module docstring describe the field, the flag and the notes.

Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
@github-actions github-actions Bot added the cli-interface-change Changes dokimi's user-facing CLI surface label Oct 29, 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 repo compare` parameters changed:
    + added option `--task`

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

@antonwolfy antonwolfy self-assigned this Oct 29, 2026
@antonwolfy

Copy link
Copy Markdown
Contributor Author

@exai-bot /review

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

Note

AI-generated review of 30030ed1d020, requested by @antonwolfy, using the default review skill. It can be wrong: verify a finding before acting on it. React with a thumbs up or down on a finding to tell the bot whether it helped.
CI on this commit, at review start and at posting: 13 passed, 3 running, 2 skipped.
Model us.anthropic.claude-sonnet-5-5, effort default, 12 of 162 turns, 1.3 of 120 minutes, 528k in / 6k out tokens.

The change records first_prompt_tokens per cell and compares it as a descriptive measure, adds repo compare --task, and prints reading notes (mde, interval/p disagreement, uncorrected descriptive p) without changing any number. I read the diff against context/trace.py, compare.py and the test helpers and found no defects. The parse_trace reuse, the one-arm-reports skip rule, the task-filter handling of missing and unpaired, and the interval_p_disagree condition all behave as the description says. I could not run the tests, because pytest, numpy, ruff and mypy are not installed in the container and there is no network.

No issues found.

For a human reviewer

  • Design call: the notes label a gate or score row inconclusive and give an mde computed from the observed spread of the per-task deltas. Confirm that framing is what you want users to read.
  • Whether the interval/p disagreement note should fire on descriptive rows, since the t interval and the sign-flip p will often differ at small task counts and could add noise.

Repository checks

Each ran as the head of this pull request defines it.

Check From Result Note
python scripts/check_growth.py AGENTS.md not run no shell call ran this command
pytest tests/test_repo_compare_reading.py tests/test_repo_compare.py tests/test_import_architecture.py tests/test_repo_reach.py AGENTS.md not run pytest is not installed in the container and there is no network
ruff check / mypy dokimi .github/workflows/ci.yml not run tools are not installed in the container and there is no network

Not verified

I did not run the new tests (test_repo_compare_reading.py, test_repo_reach.py, test_repo_run.py) or the CI checks. I checked the bootstrap-interval expectations (lo > 0, hi = +0.250) against the code by reading only. python scripts/check_growth.py ran and printed only a note that origin/main has no growth baseline to compare against. I did not confirm compare.py stays under the complexity cap for _reading_notes.

@codecov-commenter

codecov-commenter commented Oct 29, 2026 •

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.52%. Comparing base (f0f95a2) to head (525e9d2).
⚠️ Report is 7 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     #296      +/-   ##
==========================================
+ Coverage   93.48%   93.52%   +0.03%     
==========================================
  Files         272      273       +1     
  Lines       35363    35616     +253     
  Branches     6001     6047      +46     
==========================================
+ Hits        33060    33310     +250     
- Misses       1489     1492       +3     
  Partials      814      814              
Files with missing lines Coverage Δ
dokimi/dokimi/commands/repo_run.py 93.97% <100.00%> (+0.01%) ⬆️
dokimi/dokimi/repos/compare.py 99.76% <100.00%> (+0.28%) ⬆️
dokimi/dokimi/repos/events.py 92.93% <100.00%> (+0.15%) ⬆️
dokimi/dokimi/repos/runner.py 95.26% <ø> (ø)

... and 5 files with indirect coverage changes

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

@vchamarthi

vchamarthi commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

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

Review details
  • Commit reviewed: 00f9b40 against merge base f0f95a2
  • Coverage: 11 files in 3 groups
  • Engine: OpenCodeReview v1.13.3, us.anthropic.claude-sonnet-5-5
  • Usage: 950k tokens (792k from cache), 1m1s
  • Tool calls: 43 (25 code_search, 16 file_read, 1 file_find, 1 file_read_diff)
  • Docs and changelog for compare reading feature: 3 files
  • Repo compare/events/runner/command implementation: 4 files
  • Tests and fixtures for repo compare reading and run: 4 files

Comment thread dokimi/tests/test_repo_compare_reading.py
Resolve the conflicts with intel#238: cmd_repo_compare passes --task and
catches CellFileError too; _task_values reads first_prompt_tokens inside
the new reading_cells block; compare.py keeps `import math` for
min_detectable.

Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
Comment thread dokimi/tests/test_repo_reach.py
Review comment 4415626666: the one-arm skip of first_prompt_tokens was
tested only with the baseline lacking it. The test now also compares a
batch whose treatment lacks it and asserts the same warning and no
unpaired entries, so a skip that checks only the baseline arm fails.

Review comment 4416413441: the main call's message id now has a third,
sparser usage line after the full one, so the first prompt is 19_380
only when the lines merge by maximum; "last line wins" gives 16_319
and fails the test, as "first line wins" already did.

Signed-off-by: Anton Volkov <antonwolfy@gmail.com>
@antonwolfy

Copy link
Copy Markdown
Contributor Author

Review round in 00f9b40, tests only. Both OpenCodeReview points held: a mutant survived each at cad52dd, and both are caught now.

  • The one-arm skip of first_prompt_tokens is tested in both directions.
  • The trace test pins the field-by-field merge of one message's usage lines.

On the two design points exai-bot left for a human reviewer:

  • mde and "inconclusive". This is the framing intended. The evaluation that prompted this PR read recall +0.022 [-0.066, +0.100] over 26 tasks as "no measurable gain". At that spread, 80% power reaches only about ±0.12, so the honest reading is inconclusive. mde uses the observed spread of the per-task deltas, as a planning figure for the next run's task count. It is not a post-hoc power claim about this result, and the note prints only when the interval includes 0.
  • The disagreement note on descriptive rows. It fires only when the interval and p fall on opposite sides of 0.05, not whenever they differ. That case is the one that misled the same report: n_findings +0.89, t interval [-0.091, +1.860], sign-flip p 0.0497, quoted as a finding. When the two agree, nothing is printed.

@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 00f9b40. The first_prompt_tokens record and --task are right. The reading notes can still mislead in two cases: mde at task counts where the sign-flip test cannot reach 0.05, and gate and score rows read against the raw p instead of p_holm. Both are in the line comments.

What I ran, at 00f9b40:

  • make ci-local --offline: 17 checks passed, 0 failed, 13 skipped (network, docker and the other Pythons).
  • paired_summary on small gate sets: [1, 0] gives mde 1.401 at p 1.0, and [1, 1, 0] gives 0.934 at p 0.5.
  • compare with two treatments and 6 of 7 gate tasks up: p 0.031, p_holm 0.062, interval [+0.571, +1.000], and no note.
  • compare --task fix on a batch with tasks fix and fix__a: a missing fix__a cell is still listed.

Checked, no issues

  • Start context: _claude_stream_json takes the first main-thread call from parse_trace, so a message id's usage lines are merged field by field, as context trace does. A broken line goes into errors there and does not lose the cell's record. A killed cell writes null.
  • Older batches: first_prompt_tokens goes through the same one-arm rule as the process counts. The tests now cover both directions and the sparse-later-line merge, as the resolved threads asked.
  • Numbers: no existing value changes; mde and interval_p_disagree are new keys, and nothing in the schemas covers the compare JSON.
  • Docs and changelog: the contract, context-window.md and the fragment describe the new fields and --task.

"p": sign_flip_p(deltas, seed=seed),
"p": p,
"p_holm": None,
"mde": _round(min_detectable(sd, len(deltas))) if sd > EPS else None,

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.

mde comes from a normal-approximation power formula, but the p on the row is an exact sign-flip test, which cannot go below 0.05 with 5 or fewer changed tasks.

The smallest p the exact test can give is 2 / 2**k for k changed tasks, so for k <= 5 no effect of any size can reach p < 0.05. The note still names an effect that 80% power would detect. In the slot venv, paired_summary gives:

  • gate deltas [1, 0]: interval [+0.000, +1.000], p 1.0, mde 1.401. The note says the smallest detectable effect is about ±1.401 (2 tasks), which is more than any gate delta can be.
  • gate deltas [1, 1, 0]: p 0.5, mde 0.934. The best p any result could reach here is 0.25.

A reader takes "±0.934 would have been detected" as a planning figure, but at this task count nothing can be detected.

Suggestion: when 2 / 2**k >= ALPHA (or n_tasks is so small that the exact test cannot reach it), say that in the note instead of an mde, e.g. "with 3 changed tasks the sign-flip p cannot go below 0.25; no effect can reach p < 0.05", and leave mde null there (or cap it at the measure's range for gate). A test with [1, 1, 0] would pin it.

"p": p,
"p_holm": None,
"mde": _round(min_detectable(sd, len(deltas))) if sd > EPS else None,
"interval_p_disagree": lo is not None and hi is not None and p is not None and (lo <= 0 <= hi) == (p < ALPHA),

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.

For gate and score, interval_p_disagree and the "read p" note use the raw p, but the p that decides those rows is p_holm.

paired_summary sets the flag before _holm runs, so Holm never enters it. With two treatments and the same gate deltas (6 of 7 tasks up, one tied), each row prints +0.857 [+0.571, +1.000], p 0.031, p_holm 0.062. The interval excludes 0 and the corrected p is not below 0.05, which is exactly the disagreement the note is meant to flag ("for gate and score p is the one to read"), but no note is printed. In the other direction (interval includes 0, raw p < 0.05, p_holm >= 0.05), the note tells the reader to "read p" and quotes the uncorrected p.

Suggestion: set interval_p_disagree for the gate and score rows after _holm(rows), comparing the interval with p_holm, and print p_holm in their note; descriptive rows keep the raw p. A test with two treatments like the one above would pin it.

Comment thread dokimi/dokimi/repos/compare.py Outdated
unknown = [t for t in dict.fromkeys(tasks) if t not in known]
if unknown:
raise CompareError(f"no finished cell of the compared batches has --task {', '.join(unknown)}")
kept = [m for m in missing if any(m.partition("/")[2].startswith(f"{t}__") for t in tasks)] # BATCH/<task>__...

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.

Missing cells are matched to --task by the prefix <task>__ of the cell name, and a task id may itself contain __.

The manifest only rejects whitespace and commas in task ids. With tasks fix and fix__a, and an unfinished cell fix__a__guidance-raw__m-1__r1, compare([b], "guidance:none", tasks=["fix"]) returns missing == ['b/fix__a__guidance-raw__m-1__r1'], and the header says "1 missing", though fix__a was left out.

Suggestion: give each missing cell the longest known task id it starts with (<id>__), and keep it only if that id was selected. Or read the task from the cell's meta.json or the plan when there is one.

if pass_k is not None and pass_k < 1:
raise CompareError(f"--pass-k must be at least 1, got {pass_k}")
cells, missing = load_batches(batch_dirs, metrics, path_stability)
cells, missing, task_filter = _select_tasks(cells, missing, tasks)

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.

Nit: --task filters the cells before the baseline, treatment and --metric checks, so their errors no longer say why nothing matched.

repo compare b --baseline guidance:none --arm guidance:raw --task t3, where guidance:raw ran only t1 and t2, exits with "no finished cell matches the treatment 'guidance:raw'", which reads as a wrong arm name. Without --arm, that arm drops out of the comparison with no word.

Suggestion: add "among the --task selection" to those messages when tasks is set.

if unknown:
raise CompareError(f"no finished cell of the compared batches has --task {', '.join(unknown)}")
kept = [m for m in missing if any(m.partition("/")[2].startswith(f"{t}__") for t in tasks)] # BATCH/<task>__...
return [c for c in cells if c["task"] in tasks], kept, {"tasks": sorted(set(tasks)), "of": len(known)}

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.

Nit: task_filter.of counts only the tasks that have a finished cell, while the contract calls it "the task count of the batches".

A task whose cells all crashed before result.json is left out of of, so tasks: 1 of 8 (--task) can be printed for a batch of 10 tasks. Either count the planned tasks too, or say "tasks with a finished cell" in repo-evaluation-contract.md and the changelog.

@exai-bot

exai-bot commented Oct 30, 2026 •

Copy link
Copy Markdown

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

Review details
  • Commit reviewed: 525e9d2 against merge base f0f95a2
  • Coverage: 11 files in 4 groups
  • Engine: OpenCodeReview v1.13.3, us.anthropic.claude-sonnet-5-5
  • Usage: 785k tokens (603k from cache), 54s
  • Tool calls: 26 (15 code_search, 10 file_read, 1 file_read_diff)
  • Compare reading feature: core implementation, events, and changelog: 3 files
  • Runner and run command integration: 3 files
  • Documentation updates for repo evaluation contract and context window: 2 files
  • Compare reading tests and fixtures: 3 files

The inconclusive note gave mde with normal quantiles (1.96 + 0.84), but the
interval beside it is a t interval at n-1 degrees of freedom. At 4 tasks the
note said +-0.639 where the t quantiles give +-0.950; at 26 the gap is small.
The contract's formula is updated to match.

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

A task id may contain '__', so a missing cell of fix__a starts with fix__ and
was counted as a cell of fix. The task is now split off the cell name from the
right, as the arm, model and rep are the last three parts.

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
With --task the arm and metric checks run on the selected cells, so an arm
that ran only other tasks read as a wrong name. Those errors now add 'among the
--task selection' when a selection is set.

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The sentence bolded an orphan 'first'. It now names the first prompt.

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
of is the tasks with a finished cell, the only ids --task can name, not the
task count of the batches: a task whose cells all crashed before result.json
is not in it.

Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Copilot AI balanced review requested due to automatic review settings October 30, 2026 05:29
@napetrov
napetrov requested a review from a team as a code owner October 30, 2026 05:29

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

The detection-limit estimate misrepresents the exact sign-flip test’s small-sample capabilities, and ambiguous cell-name parsing silently omits unfinished cells when filtering by task.

2 open issues
What changed in this PR

2 new must-fix findings.

This PR improves dokimi’s repository A/B comparisons by recording initial prompt size, supporting task subsets, and adding guidance for interpreting uncertain results.

Changes:

  • Records and compares first-main-thread prompt tokens, while preserving missing values for older batches.
  • Adds repeatable repo compare --task filtering with selection details in text and JSON.
  • Adds detection-limit estimates and notes for inconclusive results, interval/p-value disagreements, and uncorrected descriptive comparisons.
File Description
dokimi/​tests/​test_repo_run.py Checks recorded prompt tokens and null defaults.
dokimi/​tests/​test_repo_reach.py Tests first-call selection and merged usage records.
dokimi/​tests/​test_repo_compare_reading.py Tests task filtering, token comparisons, and statistical notes.
dokimi/​tests/​repo_eval_fixture.py Adds assistant-call usage to the fake harness.
dokimi/​dokimi/​repos/​runner.py Persists first-prompt token counts.
dokimi/​dokimi/​repos/​events.py Extracts initial prompt size through the shared trace parser.
dokimi/​dokimi/​repos/​compare.py Implements task filtering, token comparisons, and reading notes.
dokimi/​dokimi/​commands/​repo_run.py Exposes and forwards the repeatable --task option.
dokimi/​docs/​repo-evaluation-contract.md Documents the new fields, filtering, and statistical guidance.
dokimi/​docs/​context-window.md Documents first-prompt measurement in repository evaluations.
dokimi/​changelog.d/​20261029_150000_antonwolfy_compare-reading.md Announces the new recording and comparison features.

Review effort: Balanced

Comment on lines +239 to +240
parts = cell.rsplit("__", 3)
return parts[0] if len(parts) == 4 else None
Comment on lines +564 to +568
def min_detectable(sd: float, n: int) -> float:
"""The smallest mean delta a two-sided test at :data:`ALPHA` finds with :data:`POWER` over ``n`` tasks of
spread ``sd``. The quantiles are t at ``n - 1`` degrees of freedom, as the interval of ``mean_ci`` is."""
df = n - 1
return (sp_stats.t.ppf(1 - ALPHA / 2, df) + sp_stats.t.ppf(POWER, df)) * sd / math.sqrt(n)
@napetrov

Copy link
Copy Markdown
Contributor

Head at review: 00f9b40 (unchanged on fetch, 10-29). Pushed sha: 525e9d2.

Anton, the small fixes for Vlad's threads are on top of 00f9b40 and pushed as 525e9d2 (6 commits, signed off). Each fix has a test that fails on 00f9b40. Two of the threads are design questions, listed at the end, and are not changed.

Fixed, with the commits (pushed shas):

  • compare.py _select_tasks, the --task prefix thread: a missing cell is matched to its task by splitting the cell name from the right (<task>__<arm>__<model>__r<rep>), not by prefix. fix no longer counts fix__a's unfinished cell. Test: test_task_matches_a_missing_cell_by_its_task_not_by_a_prefix. (349d544)
  • Error text, the arm and metric thread: with --task, the baseline, treatment and --metric errors add "among the --task selection". Test: test_an_arm_or_metric_error_under_task_says_the_selection_is_the_reason. (d5f2d7c)
  • The of count thread: I took the wording option. of is the number of tasks with a finished cell, the only ids --task can name. The contract now says so. Counting the planned tasks instead is a bigger change; say if you want it. (e00edbc)
  • The mde nit (my review): min_detectable uses t quantiles at n-1 df, like the interval, so the note at 4 tasks says ±0.950, not ±0.639. The contract formula follows. Test: test_min_detectable_uses_the_t_quantiles_of_the_interval_at_small_n. (0c7d61f, 525e9d2)
  • docs/context-window.md: the sentence now names the first prompt ("records the size of each cell's first prompt, in tokens, ..."). (5fc5198)

Not changed, for you to decide:

  1. Sign-flip p vs mde (thread at compare.py:539). With k <= 5 changed tasks the exact p cannot go below 2/2^k, so no effect can reach p < 0.05, yet the note still names an mde. Options: (a) mde null and a note saying the sign-flip p cannot go below 2/2^k at this count; (b) keep mde and say it is not reachable by the test; (c) cap it. Recommendation: (a). It changes the mde JSON value at small k, and the module docstring says no number changes, so the docstring would need an update too.
  2. Holm p in the disagreement check (thread at compare.py:540). interval_p_disagree is set before _holm, so gate and score use the raw p. Options: (a) compare with p_holm and print it in the gate and score note, as suggested; (b) keep raw p. Recommendation: (a), but it compares an unadjusted 95% interval with a Holm-adjusted p. Decide whether the interval should be adjusted too, or the check should stay on raw p.
  3. Without --arm, an arm that ran only other tasks drops out of the comparison with no word (the nit at compare.py:180). Options: name the dropped arms in the header, or leave it. Recommendation: name them.
  4. _cell_task assumes the arm and model slugs carry no __. A slug can produce __ from a run like _!. Not reachable with today's arm and model names; a comment in the code says so.

Test note for you: dokimi/tests on Python 3.14 hits a DeprecationWarning from langsmith (asyncio.iscoroutinefunction) at collection once litellm is installed. It is not from this PR. The compare tests pass with that warning filtered.

This branch has not been deployed

No deployments
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.

7 participants