[napetrov]

Summary

generate_section_report (model-compare report) was one 556-line function with cyclomatic complexity 53, the highest in the repo. Each report section is now a helper. The builder works out the shared inputs once (common questions, scored arm, has_treatment, per-run summary) and joins the helpers' lines. The output does not change.

 generate_section_report                     53 -> 4
+  _pool_section                                  5   replicate pooling + note
+  _common_scores                                 1   common-question cut
+  _run_summary                                   1   per-run means, paired deltas
+  _pooling_lines                                 2
+  _models_compared_lines                         2
+  _resource_usage_lines / _cost_cell           4 / 3
+  _overall_summary_lines                         2
+  _significance_lines                            2
+    _adjusted_significance / _significance_row 4 / 2
+  _difficulty_lines / _difficulty_cells        6 / 4
+  _head_to_head_lines / _winner_counts         3 / 5
+  _benefit_lines                                 4
+  _ranking_lines / _rank_table                 2 / 1

dokimi/report/model_compare.py goes from 1296 to 1209 lines. Its other functions are unchanged (extract_scores 16 and check_run_consistency 13 are still over 10).

#93 interaction: #93's growth_baseline.json entries for this file drop. dokimi/report/model_compare.py::generate_section_report (53) goes away, and the file's lines entry goes from 1296 to 1209. Whichever of #93 and this PR merges second runs python scripts/check_growth.py --update to lower them. This PR does not touch #93's branch.

Evidence

Two commits. The first adds only the characterization tests: tests/test_model_compare_sections.py with goldens in tests/fixtures/model_compare_sections.json. Its 11 cases each render a whole section and compare it byte for byte with the golden. Together they cover every line and branch of the old function (coverage.py --branch). The cases are: treatment, Holm and BH, no SciPy, baseline-only, a treatment run beside a baseline-only one, two scored arms (Context arm), a pinned arm, cost rows with and without a summary, pooled replicates with a label clash and a run left out of run_ids, singleton pooling, and an empty section.

test_section_report_matches_golden[*]   on the old function (commit 1):   12 passed
                                        on the refactor   (commit 2):   12 passed
mutation check: one extra space in a head-to-head row  ->  8 failed

Complexity: ruff check --select C901 --config 'lint.mccabe.max-complexity=10' reports only the untouched extract_scores (16) and check_run_consistency (13), and check_growth.py's function_complexities gives the same numbers as above.

Local: full pytest 6911 passed, 2 skipped; ruff check . clean; mypy clean (222 files).

Merge Danger

Door: two-way. This is an internal refactor with no change to the public API, and a revert restores the old function.

Blast Radius: none

generate_section_report keeps its signature and output. The new helpers are private. There are no persisted-format, schema or flag changes. Two odd outputs from the old code are kept on purpose and are pinned by the goldens. First, the baseline-only "By Difficulty" separator is |---|---:||, with a doubled pipe. Second, a difficulty row's N counts the first run that has that bucket, not every run. Fixing either would change the output, so each belongs in its own PR.

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

[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.83%. Comparing base (92d92d3) to head (e7c2507).
⚠️ 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     #169      +/-   ##
==========================================
+ Coverage   91.78%   91.83%   +0.05%     
==========================================
  Files         223      223              
  Lines       27623    27599      -24     
  Branches     4549     4535      -14     
==========================================
- Hits        25354    25346       -8     
+ Misses       1606     1597       -9     
+ Partials      663      656       -7     

Files with missing lines	Coverage Δ
dokimi/dokimi/report/model_compare.py	92.84% <100.00%> (+3.17%)	⬆️

... 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 d139a2a. The refactor itself holds up: the output is unchanged. The goldens are narrower than the PR claims, so there are five test gaps: two inline and three below. I'd close the two inline ones before merge.

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

• make ci-local: 19 passed, 9 failed, 7 skipped. The suite passed (6892 passed, 21 skipped) and changed-line coverage is 100% (196 of 196). Eight failures are tools missing from my venv (ruff, zizmor, pre-commit, bandit, mypy, deptry, mkdocs, pip-audit; exit 127): 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. The changelog failure is covered by skip-changelog.
• A differential run of the old and new generate_section_report on 3000 seeded inputs (0 to 5 runs, missing, NaN, tied and zero scores, odd labels, one or two treatment arms, pinned arm, baseline-only runs, Holm and BH, SciPy on and off, pooling on and off), with the LLM transport mocked. Output or exception (type and message) was identical every time.
• The new test file against the base module: 12 passed. Branch coverage of generate_section_report and every new helper by the new tests alone is complete.
• A mutation run: 92 mutants of the new helpers, 35 of which pass the new tests (31 also pass the existing report tests). The ones that matter are listed below.

More test gaps from the same run (all pass the new tests):

1. _cost_cell is only tested with 3 < 8. cost_known < total_calls to <= passes the new tests, and the cs.get("n") fallback for total_calls is never reached (model_compare.py:908-909).
2. The pooled footnote flag is untested. _resource_usage_lines(runs, bool(replicate_note)) (model_compare.py:781) passes with pool_replicates in its place, and no fixture uses a baseline_arm other than the default.
3. Unscored count and arm list. The unscored count is only 0, 2 or 9, so if st["unpaired"] to > 1 passes. Each fixture run has one arm, so the sorted, joined arm cell is untested.

Checked, no issues

• Complexity: an AST branch count gives 53 on the base and 4 at the head for generate_section_report; the helper numbers match the table; extract_scores (16) and check_run_consistency (13) are unchanged. Line counts are 1296 and 1209.
• Kept quirks: the |---|---:|| separator and the first-run N in difficulty rows are as in the base.
• Repo rules: SPDX header on the test file, .json fixture exempt, no generated artifacts, tests offline, signature unchanged, helpers private, no docs mention the function.

[napetrov]

Review-body items, all in 10b4bbe (tests only, no package change): 1. _cost_cell: golden unscored_and_arms has 4/5 with the count read from n (fallback) and 5/5 shown without a ratio, so <-><= and dropping the n fallback both fail. 2. pooled flag: golden pooled_singletons_cost (pool_replicates on, nothing pooled, no footnote) with a baseline_arm of control and a cost summary keyed by it; passing pool_replicates instead of bool(replicate_note) fails. 3. unscored count and arm list: the same golden has exactly 1 unscored question and a run scored in two arms (docs, skill:x); > 1 and a single-arm join both fail. Each mutant was applied and reverted. Local: tests/test_model_compare_sections.py 22 passed.

[antonwolfy]

Review at 12a5836. The refactor holds up and 10b4bbe closes both of my threads. One test gap is left: zero values in the cost cell and the benefit row are not pinned.

What I ran, on a Linux review host at 12a5836:

• make ci-local: 27 passed, 2 failed, 7 skipped. The suite passed (7229 passed, 1 skipped) and the changed-line coverage check passes. The changelog failure is covered by skip-changelog. pip-audit fails on the lock files, which this PR does not touch.
• A differential run of the old and new generate_section_report on 2500 seeded inputs (0 to 5 runs, missing, NaN, tied and zero scores, one or two treatment arms, pinned arm, baseline-only runs, Holm and BH, SciPy on and off, pooling on and off, cost summaries with and without n), with the LLM transport mocked. 1726 renders matched byte for byte, and the 774 inputs that raise gave the same exception type and message in both. Since d139a2a, model_compare.py changed in one place only: the _check_judge_context error text.
• A mutation run: 72 mutants of the new helpers against the new tests and test_model_compare.py. All four significance mutants from my thread and all the truthiness mutants of _run_summary and _difficulty_cells now fail. So do cost_known < total_calls to <=, dropping the n fallback, pool_replicates in place of bool(replicate_note), and if st["unpaired"] to > 1.

Checked, no issues

• Growth baseline: model_compare.py has 1210 lines, matching the entry. Only extract_scores (16) and check_run_consistency (13) are over 10 under ruff --select C901, and both are still listed.
• Goldens: the fixture has 17 keys and the case list has the same 17. Output does not depend on dict or set order. The sorted arm cell is caught on about half the hash seeds, because the unscored_and_arms golden has docs before skill:x.
• Repo rules: SPDX header on the test file, tests offline, no generated artifacts, signature unchanged.

[xaleryb]

• Ran: a differential fuzz of the old and new generate_section_report on several thousand seeded inputs (output bytes or exception compared), the CLI end to end on the repo fixture, the full suite on head merged with main, and the earlier reviewers' mutants re-run at the head.
• Output is the same for every valid input, and every earlier gap is now pinned. The p_adj <=, Wilcoxon is not None, and->or and sig['significant'] mutants fail. So do the zero-score truthiness filters, _cost_cell <= and the n fallback, pool_replicates used in place of bool(replicate_note), and unscored > 1.
• No new must-fix or mid findings. No commits from us.
• For the author: this PR and ci(growth): take #153's growth into the baseline; name the PR behind each post-#93 entry #183 both edit adjacent lines of growth_baseline.json, so whichever merges second needs a rebase. The two inline threads (model_compare.py:937 and :1022) are addressed by 10b4bbe and can be resolved.

[xaleryb]

Approved at 12a5836. 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]

LGTM. 08a8f5d closes my last thread: the zero_values golden pins the zero cost, latency, cache-hit, improved-count and baseline-mean cells. One nit inline.

What I ran, on a Linux review host at 08a8f5d:

• make ci-local: 27 passed, 2 failed, 7 skipped. The suite passed (7230 passed, 1 skipped) and the changed-line coverage check passes. The changelog failure is covered by skip-changelog. pip-audit fails on the lock files, which this PR does not touch.
• The five mutants from my thread against the new tests and test_model_compare.py (91 tests): not cost_usd, not st["improved"], not cs.get(f) for latency and cache hit, not wo in the treatment layout, and cost_known and total_calls all fail on the new golden. Four more truthiness mutants of _run_summary and _difficulty_cells fail too.
• Since 12a5836 the diff is test-only: one fixture key, the zero_values case and its expected-substring entry.

Checked, no issues

• Goldens: zero_values renders $0.0000 (0/5), 0.00s, 0.0%, 0 (0%) and 0.0 (baseline), as the thread asked.
• Repo rules: no package change since the last review, no generated artifacts, tests offline.

[antonwolfy]

LGTM. 56b041c closes my last nit: _FREE_COST now carries zero token counts and the zero_values golden prints 0 | 0 | 0. Nothing else is open.

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

• make ci-local: 28 passed, 1 failed, 7 skipped. The suite passed (7430 passed, 1 skipped). The changelog failure is covered by skip-changelog.
• A differential run of the old and new generate_section_report on 1500 seeded inputs, each with SciPy on and off, with the LLM transport mocked. 2008 renders matched byte for byte, and the 992 inputs that raise gave the same exception type and message in both.
• A mutation run: 20 mutants of _resource_usage_lines, _cost_cell, _benefit_lines and _difficulty_cells against the new tests and test_model_compare.py (91 tests). 18 fail. The mutant that prints — for a zero token count fails on the new golden.

Checked, no issues

• Since 12a5836: the PR's own changes are test-only: the _FREE_COST fields, one expected-substring entry and one fixture key. The rest of the diff is merges from main.
• Growth baseline: model_compare.py has 1210 lines, matching the entry. Only extract_scores (16) and check_run_consistency (13) are over 10 under ruff --select C901, and both are still listed.
• Repo rules: SPDX header on the test file, tests offline, DOKIMI_UPDATE_GOLDEN only rewrites the fixture when set, no generated artifacts, signature unchanged.