Benchmark: the held-out set was the tuning set, and the summary JSON asserts otherwise

[adamjohnwright]

From an adversarial review of main after #11. Reported factually; the harness is careful work and bench/README.md is unusually candid — several of these are failures to follow its own stated guidance rather than blind spots.

1. Held-out was used for tuning (HIGH)

bench/tune_phase1.sh runs a 3 AND-modes × 3 threshold grid with DOWN_LOOSE="0.85"; UP_LOOSE="1.15" — the harness's exact default cutoffs (benchmark_mpbiopath_cases.py:82-83) — and names TP53, WNT, ERBB2 among the pathways kept in by MAX_EDGES=80000. bench/phase2_holdout.sh:2-3 states the ten were the tuning set.

Four of the seven "held-out" pathways (TP53, WNT, ERBB2, S Phase) are 328 of 404 held-out scored cases (81.2%). Meanwhile benchmark_mpbiopath_cases.py:814 writes "held_out_was_used_for_configuration": False into every summary, and bench/README.md:47-49 says "Do not change that split after looking at held-out results."

The ten pathways are the entire experimental ground truth (847 of 847 cases), so no unseen holdout exists.

Fix: set the field honestly or delete it, and describe the split as a replication split.

2. McNemar treats correlated readouts as independent (HIGH)

generate_evaluation_report.py:223-231 pairs at case level, but 847 cases come from 194 unique perturbation solves — the harness itself caches one solve per perturbation (:757-764). The 20 discordant cases come from 9 independent experiments.

clustering	p
per case (reported)	0.0118
per perturbation solve	0.0703
per pathway	0.625
converged-only pairs	0.344

Nine of the 13 changed cases with a non-converged arm are gains, and the report's own Convergence Gate section warns those should not count as equal evidence. paired_bootstrap (benchmark_mpbiopath_cases.py:571-587) resamples cases too, so both CIs are narrower than they should be.

3. The strongest baseline is computed, stored, then omitted (HIGH)

shortest_signed_path scores 443/627 (70.7%, macro-F1 0.63) on exactly the same cases; DeltaSignal is 456/627 (72.7%). A 13-case, 2.1pp advantage over sign-of-shortest-path traversal — the same magnitude as the diagram effect the report does test. Present in diagram_on_summary.json, absent from evaluation_report.md, scorecard.tsv and both figures; the Comparator Context section names only MP-BioPath and curator.

Also: baselines use two denominators (847 vs 627), so their accuracy columns are not mutually comparable.

4. Pinned observations are not excluded from the readout (HIGH)

load_dbid_to_uuids (:228-235) registers a node's UUID under its own stable id and every member_leaves entry, so a complex containing gene X is a "gene-X node". The perturbation pins all of them (:760) and the readout is taken with no disjointness check (:769-793). Verified live:

CHEK2 dir=0 -> readout uuid is PINNED, value 0.0,  class 0  ("correct")
CCNE1       -> readout set is a SUBSET of the pinned set

21 of 223 dev cases intersect; 14 are trivially correct from the pinned value alone.

Fix: record pinned_readout_uuids per case; exclude or report as a separate stratum.

5. Per-split/per-pathway convergence is fabricated (MEDIUM-HIGH)

metric_summary_for_field hardcodes "converged": True (:515), and grouped_summaries/split_summaries call it with the default field. The committed artefact reports 133 non-converged cases at top level and 0 for every split and pathway, including TP53 which the report names as the sole source of failures. run_mpbiopath_factorial.py:81,98 reads from by_split, so the factorial TSV reports 0 unconditionally.

6. Not reproducible as committed (MEDIUM-HIGH)

• benchmark_cases.tsv is not committed for either arm, so the confusion matrix, per-class F1 and the McNemar pairing cannot be recomputed.
• All 40 recorded network sha256s match nothing on disk, and bench/README.md:273-277 notes LNG emits UUID4 ids — so they cannot be byte-regenerated even at the pinned LNG commit.
• --lng-commit and --reactome-release are unvalidated free text (generate_evaluation_report.py:35-36). Given that a finding in this repo was already retracted for validating a v96 artefact against v97, an unverified release string is a live hazard.
• Nothing records which catalog was diagram-on; sanitized_manifest strips DS_PATHWAY_CATALOG (:66-69).

7. Report pins a superseded commit (MEDIUM)

provenance.json pins 59e1757; bd2b533 ("honest convergence") is not an ancestor. Its "133 non-converged / 9 of 171 solves" describes a convergence definition that no longer exists — and 694643e then retracted the replacement metric too.

Verified CORRECT (do not re-check)

• 16 + 4 ≠ 25 is not an inconsistency: 5 changed cases were wrong in both arms; 16+4+5 = 25, net 456−444 = 12 = 16−4.
• exact_mcnemar_p is a correct exact two-sided binomial and reproduces 0.011818; the discordant-pair definition (on correctness) is the right pairing.
• Denominators identical across arms (627/847, byte-identical unscored_reasons); main:220-221 raises if the case sets differ.
• The 0.85/1.15 cutoffs are legitimate: a live unperturbed control gives 1276/1276 nodes at exactly 1.0, so no control solve is needed.
• Repeated identical solves are bitwise identical; case keys unique 847/847; the class-frequency baseline is fitted on development pathways only; structural baselines are cycle-safe; per-class metrics verified by hand against the committed confusion matrix.

Separately for the TCGA analysis: no leakage found on the central question (CV refits Cox inside each fold, folds event-stratified and shared across specs), but clinical_plus_mki67 (CV median 0.6763, beating DeltaSignal on two of three pathways) is filtered out of every figure by a display dict, and the headline adjusted HR comes from a near-collinear model (Spearman 0.95–0.96, VIF 9.0/12.4) whose sibling pathway shows the textbook see-saw. Happy to open that separately if useful.