feat(statistics): add repository-clustered paired inference

[drewstone]

What changed

• add clusteredPairedBinary for paired binary outcomes nested inside independent repositories
• return every matched and unmatched source row unchanged
• resample whole repositories for a task-weighted confidence interval
• sign-flip whole-repository totals for the same task-weighted effect
• add Holm step-down adjustment for multiple benchmark arms

Checked before building: pairArms, seeded RNG, binary power helpers, and existing multiple-comparison utilities already existed. This composes those primitives and adds only the missing repository-clustered inference layer.

Correctness properties

• one repository returns no cluster interval
• too few bootstrap draws fail before sampling
• exact and Monte Carlo sign flips are deterministic
• a 40-repository unequal-size counterexample cannot report significance for a different estimand
• consumers receive unmatched rows and must reject them before promotion

Verification

• typecheck
• 270 test files: 2,750 passed, 2 skipped
• build + OpenAPI generation
• packed-package export verification
• lint: 0 errors, 4 pre-existing warnings
• two persisted critical audits: initial REQUEST_CHANGES, then APPROVE after all 3 medium findings were fixed
• branch merges cleanly into current origin/main

[tangletools]

✅ Auto-approved drewstone PR — 0837b244

This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

_{tangletools · auto-approval · reason: drewstone_author · 2026-07-09T22:07:54Z}

[tangletools]

🟢 Value Audit — sound

Verdict	sound
Concerns	1 (1 low)
Heuristic	0.0s
Duplication	0.0s
Interrogation	98.8s (2 bridge agents)
Total	98.8s

💰 Value — sound

Adds repository-clustered paired binary inference (whole-cluster bootstrap + cluster-aligned sign-flip) plus a Holm FWER helper — a genuine, in-grain gap-fill with no existing equivalent to reuse.

• What it does: New clusteredPairedBinary (src/clustered-paired-binary.ts) compares pass/fail outcomes on matched work items where the independent unit is a cluster (repository/subject), not the task. It delegates pairing to the existing pairArms, then reports both a task-weighted and an equal-cluster-mean risk difference, builds a CI by resampling WHOLE clusters and recomputing the task-weighted RD (src/clus
• Goals it achieves: Make pass/fail promotion decisions honest on repository-disjoint coding benchmarks (SWE-bench-style), where tasks nested in the same repo are correlated. The existing task-level estimators — mcnemar/pairedRiskDifference (statistics.ts:1080,1127) and pairedBootstrap (statistics.ts:946) — treat every task as independent and are anti-conservative under within-repo correlation; the PR's 40-repo
• Assessment: Coherent and worthwhile. It fills a real hole — a repo-wide grep for cluster/sign-flip/whole-cluster shows only unrelated failure-mode clustering (src/pipelines/failure-cluster.ts), and nothing in statistics.ts does whole-cluster resampling or cluster-aligned randomization. It is squarely in the codebase grain: composes pairArms and the shared mulberry32 PRNG rather than reimplementing them, m
• Better / existing approach: None found — this is the right seam. I checked statistics.ts (mcnemar/pairedRiskDifference/pairedBootstrap all assume independent pairs), paired-arms.ts (pairing only), and a repo-wide cluster/sign-flip grep (only failure-clustering). pairedBootstrap has a clean single-resampling-unit (independent pair) contract; bolting a grouping dimension onto it would muddy the common independent-pair case,
• Model: opencode/kimi-for-coding/k2p7
• Bridge attempts: 1

🎯 Usefulness — sound

A coherent cluster-aware paired-binary inference layer that composes the existing pairArms/mulberry32 primitives and fills a real gap (whole-cluster bootstrap + sign-flip + Holm FWER) with no duplication and correct reachability through the public barrel.

• Integration: Exported from the root barrel at src/index.ts:183 alongside every other substrate primitive, and documented in the SKILL.md production-rigor surface menu (line 148) next to HeldOutGate, pairedBootstrap, and researchReport. No internal caller exists in this repo, but that is expected and correct: per CLAUDE.md this package is the bottom of the layering (zero upward deps) and every peer primitive (p
• Fit with existing patterns: Excellent fit. It delegates pairing to the existing pairArms primitive (clustered-paired-binary.ts:145) rather than reimplementing it — exactly the composition pattern paired-arms.ts documents ('feeding the matched pairs to the correct paired estimators'). It reuses the single shared mulberry32 PRNG from statistics.ts:1391 rather than rolling its own randomness. It does NOT compete with comparePai
• Real-world viability: Built to hold up under realistic use. The three failure modes that the persisted critical-audit caught and fixed (sign-flip/interval estimand mismatch, one-cluster zero-width interval, insufficient-bootstrap-draws collapsing to a nominal [1,1]) are precisely the subtle statistical traps that bite in production eval interpretation, and all three are now gated with explicit tests (tests/clustered-pa
• Model: opencode/zai-coding-plan/glm-5.2
• Bridge attempts: 1

🔎 Heuristic Signals

🟡 Cruft: magic number added tests/clustered-paired-binary.test.ts

• seed: 1337,

---

What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass	What it asks
Heuristic	Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication	Do added function/class names already exist elsewhere in the repo?
Value Audit	What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit	Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

_{value-audit · 20260709T221004Z}

[tangletools]

❌ Needs Work — 0837b244

Review health 100/100 · Reviewer score 45/100 · Confidence 95/100 · 22 findings (1 high, 5 medium, 16 low)

	opencode-kimi	glm	deepseek	aggregate
Readiness	45	54	63	45
Confidence	95	95	95	95
Correctness	45	54	63	45
Security	45	54	63	45
Testing	45	54	63	45
Architecture	45	54	63	45

Reviewer score is advisory once the run is complete and the verdict has no blockers.

Full multi-shot audit completed 8/8 planned shots over 17 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 8/8 planned shots over 17 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 8/8 planned shots over 17 changed files. Global verifier still owns final merge decision.

Blocking

🔴 HIGH Manifest base/head provenance predates the audited code — .evolve/critical-audit/2026-07-09T21-47-00Z/manifest.json

Both manifests (this one and 2026-07-09T21-54-37Z/manifest.json:5-6) record base 9c7fc30 / head cf51ffd and scope src/clustered-paired-binary.ts + tests/clustered-paired-binary.test.ts. git cat-file -e shows those two files are absent at 9c7fc30, cf51ffd, and PR base 39e8002; they exist only at PR head 0837b24, introduced by descendant commit eeac9c2, and cf51ffd is an ancestor of eeac9c2 (verified via merge-base --is-ancestor). So the recorded head cannot be the commit that was audited — the subject did not exist there — yet the artifacts carry line-precise findings (180/228/366; 180/236/374) and an APPROVE verdict attributed to that commit pair. Impact: any merge gate or reviewer trusting m

Other

🟠 MEDIUM Both manifests reference base/head commits where the audited file does not exist; range is inverted — .evolve/critical-audit/2026-07-09T21-47-00Z/manifest.json

Both manifests declare base=9c7fc3056b9effddda6a5b91f1467fd3dc1af225, head=cf51ffd63b53daa66e2d8d8d8003ceec5123231d. Verified: git cat-file confirms src/clustered-paired-binary.ts does NOT exist at either commit (both predate eeac9c2 which adds it). Furthermore git merge-base --is-ancestor proves head (cf51ffd) is an ANCESTOR of base (9c7fc30) — the range is inverted. The line numbers in findings.jsonl (366, 374, 180, 228, 236) actually match the real PR head 0837b24, not cf51ffd. Additionally, the two runs cite different line numbers (366→374, 228→236) while claiming the SAME head commit, which is only possible if the head is stale. Impact: anyone checking out base..head to reproduce finds an empty diff and a missing file. Fix: set base/head to the actual commits audited (base=39e8002

🟠 MEDIUM Re-audit manifest claims medium:0 but its findings.jsonl lists 3 medium entries — .evolve/critical-audit/2026-07-09T21-54-37Z/manifest.json

manifest.json line 14 declares "medium": 0, but the same run's findings.jsonl contains 3 entries each with "severity":"medium". Prior convention (2026-06-21 run) counted ALL entries in the manifest (1 MEDIUM + 9 LOW = 10, matching 10 JSONL lines) and tracked resolution via a separate "status":"fixed" field. An automated consumer parsing findings_count_by_severity would believe this re-audit is spotless, yet three findings are recorded. Fix: either set medium to 3 (counting entries as prior runs did) and add "status":"resolved" to each JSONL entry, or document that the count reflects open-only findings.

🟠 MEDIUM Reaudit count bucket disagrees with findings.jsonl rows; no disposition field — .evolve/critical-audit/2026-07-09T21-54-37Z/manifest.json

manifest.json reports findings_count_by_severity {medium:0} but the sibling findings.jsonl contains 3 rows, each still "severity":"medium" (lines 1-3), with resolution encoded only in free-text action ('Resolved by...') / verification ('resolved: ...'). There is no machine-readable status/disposition field, so a gate keyed on the count (0 -> pass) and a gate keyed on rows-with-severity-medium (3 -> block) reach opposite verdicts on the same artifact, and no structured consumer can distinguish open vs resolved without parsing prose. Impact: silent gate disagreement; the reaudit looks clean by count while still enumerating three medium

🟠 MEDIUM holm throws raw Error instead of ValidationError — src/index.ts

src/index.ts:416 re-exports holm from statistics.ts. The function throws new Error(...) for invalid inputs (lines 861, 865 of statistics.ts) despite statistics.ts:1 importing ValidationError from './errors'. The repo CLAUDE.md mandates 'every error this package throws as part of its public contract extends AgentEvalError.' ValidationError extends AgentEvalError; raw Error does not. Net-new code should follow the existing error taxonomy. Fix: replace throw new Error(...) with throw new ValidationError(...) in the holm function (statistics.ts:861,865).

🟠 MEDIUM Significance boundary inconsistent with bonferroni — src/statistics.ts

holm uses pValue <= alpha for significance while bonferroni (line 844) uses p < alpha. This creates an asymmetry where a p-value exactly equal to alpha is significant under Holm but NOT under Bonferroni. Standard convention is p < alpha (strict) and Holm-Bonferroni uniformly dominates Bonferroni in statistical power under the same boundary. Fix: change to pValue < alpha to match bonferroni and standard convention.

🟡 LOW Schema regression: manifest drops score and verdict fields present in all prior manifests — .evolve/critical-audit/2026-07-09T21-47-00Z/manifest.json

Prior manifests (2026-05-27: "score":"7.5/10"; 2026-06-21: "score":"8/10","verdict":"APPROVE_AFTER_FIXES") include machine-readable score and verdict. Both new manifests omit these; the verdict (REQUEST_CHANGES / APPROVE) and score (6/10, 8/10) exist only in summary.md prose. A structured consumer can no longer read the verdict from manifest.json. Fix: add "score":"6/10","verdict":"REQUEST_CHANGES" (and 8/10, APPROVE for the re-audit) to the manifest root.

🟡 LOW Severity casing and status-field inconsistencies break prior convention — .evolve/critical-audit/2026-07-09T21-54-37Z/findings.jsonl

Prior findings.jsonl (2026-06-21) uses uppercase severity ("MEDIUM","LOW") matching uppercase manifest keys, and a structured "status":"fixed" field for resolution. New artifacts use lowercase ("medium") in both findings.jsonl and manifest keys, and the re-audit entries embed resolution only inside the verification string ("resolved: ...") with no status field. If any downstream consumer matches on uppercase severity or filters on status, it will silently miss these findings. Fix: align casing with prior convention and add "status":"resolved" to re-audit entries.

🟡 LOW Re-audit manifest head commit may be stale — .evolve/critical-audit/2026-07-09T21-54-37Z/manifest.json

The re-audit manifest carries head: cf51ffd63b53daa66e2d8d8d8003ceec5123231d — the same commit as the first audit that reported the 3 bugs. The re-audit claims all 3 are resolved with specific post-fix verification (p=0.1413, bootstrap-null, min-40). This implies the re-audit ran against a later commit than the manifest states. The findings are verified correct against the PR head (0837b24), so this is a metadata accuracy nit, not a correctness issue. Fix: update the head field to the commit actually audited, or add an auditedHead field for re-audits.

🟡 LOW No upper bound on bootstrapResamples/signFlipResamples allows resource exhaustion — src/clustered-paired-binary.ts

assertPositiveInteger (line 272) validates these are positive integers but imposes no upper bound. clusterBootstrap (line 340) pre-allocates new Array<number>(config.bootstrapResamples), so a caller passing bootstrapResamples=1e9 would OOM. The Monte Carlo sign-flip path doesn't pre-allocate but would run for a very long time. Defaults (10K/100K) are reasonable and exactClusterLimit is correctly capped at 20, so this is a hardening concern for adversarial/untrusted callers, not a correctness bug. Consider an upper guard (e.g., 1e6 o

🟡 LOW arm accessor validated before the other-arm ignore filter — src/clustered-paired-binary.ts

Line 287 calls assertNonEmptyString('arm', arm) for EVERY row, before the line-288 if (arm !== baselineArm && arm !== treatmentArm) continue filter. The option doc (line 25) says "Rows from other arms are ignored", but a row in an unrelated arm whose arm() returns ''/whitespace throws at 287 instead of being ignored at 288. In practice only triggers on a genuinely broken accessor, so it is defensible under the repo's fail-loud doctrine and the other accessors (pairKey/clusterKey/pass) are correctly validated only post-filter. Reconci

🟡 LOW equalClusterMean naming is ambiguous — src/clustered-paired-binary.ts

The field name equalClusterMean doesn't communicate that it's the mean of per-cluster mean differences (each cluster weighted equally regardless of pair count). The docstring line 108 clarifies it ('so every cluster has equal weight'), but the name alone reads like it might be a cluster-weighted mean of the risk difference. Consider clusterWeightedMean or perClusterMean.

🟡 LOW pass! non-null assertion relies on non-local invariant from projectSelectedRows — src/clustered-paired-binary.ts

Lines 165-166 use baseline.pass! / treatment.pass! to read the projected boolean. This is safe today because projectSelectedRows (line 302) always sets pass on every ProjectedRow, and the runtime check at line 296-300 validates it's a boolean. However, the ! is a non-local assertion: if projectSelectedRows is later modified to omit pass on some path, these reads silently produce `undefin

🟡 LOW Significance comparison convention inconsistent with sibling adjusters (<= vs <) — src/statistics.ts

holm computes significant: adjusted.map((pValue) => pValue <= alpha) (inclusive, line 882), while the sibling bonferroni uses strict p < alpha (line 844) and benjaminiHochberg uses strict v < fdr (line 906). At the exact boundary p==alpha the three functions in the same module disagree: bonferroni([0.025,0.025]) at alpha=0.05 yields adjusted=[0.05,0.05] significant=[false,false], whereas holm([0.025,0.025]) yields the same adjust

🟡 LOW Significance uses <= alpha while sibling corrections use strict < — src/statistics.ts

significant: adjusted.map((pValue) => pValue <= alpha) uses non-strict <=, but bonferroni (src/statistics.ts:844) uses p < alpha and benjaminiHochberg (src/statistics.ts:906) uses v < fdr. The <= choice matches the standard Holm rejection rule and is locked in by the [0.025,0.025] boundary test (tests/clustered-paired-binary.test.ts:290-293), so it is not a correctness bug — but the three family-wise/FDR helpers now disagree on the boundary with no comment explaining why. Either align all three to one convention or add a one-line note that Holm rejects on the boundary by definition so a future reader does not "fix" it and silently break the tested behavior.

🟡 LOW 'less' alternative never exercised end-to-end — tests/clustered-paired-binary.test.ts

The only 'alternative' coverage is the invalid-value throw (alternative:'up') plus 'greater' (lines 92/110/157/205) and two-sided defaults. No test pins a pValue for alternative:'less'. The isExtreme 'less' branch (clustered-paired-binary.ts:427) is symmetric to 'greater' so risk is low, but a single pinned 'less' case would close the branch and lock the direction. Fix: add one small exact-enumeration case (e.g. 2 clusters) asserting a known pValue under alternative:'less'.

🟡 LOW Missing less alternative coverage in sign-flip tests — tests/clustered-paired-binary.test.ts

Tests 2-5 only exercise the 'greater' and default 'two-sided' alternatives. The 'less' branch in isExtreme (src/clustered-paired-binary.ts:427) has no test coverage. A regression here would silently produce wrong one-sided p-values.

🟡 LOW No fully-empty input case — tests/clustered-paired-binary.test.ts

Test at line 175 covers 'rows present but none pair' (statistics:null). A clusteredPairedBinary([], options()) case would also assert matchedPairs=[]/unpaired empty/statistics:null. The code path is identical (matchedPairs.length===0 at clustered-paired-binary.ts:177), so this is a completeness nit, not a defect.

🟡 LOW No multi-rep / repKey path through the clustered wrapper — tests/clustered-paired-binary.test.ts

options() wires repKey (line 28) but every test uses at most one row per (pairKey, arm), so the multi-rep branch of pairArms (paired-arms.ts:152-166) is never reached through clusteredPairedBinary. The clustered-specific rep behavior — repIndex preserved on ClusteredMatchedPair and the repIndex embedded in the cross-cluster error (clustered-paired-binary.ts:155) — is therefore untested in this file. Pairing correctness itself is covered in paired-arms' own tests (out of shot), but a single two-rep case here would prove the wrapper forwards reps and reports repIndex correctly. Low; non-blocking.

🟡 LOW No test for repKey with multiple replicates per work item — tests/clustered-paired-binary.test.ts

The repKey accessor is wired in the options helper (line 27) and passed to projectSelectedRows/pairArms, but no test supplies multiple rows per arm per pairKey. The test suite can't detect whether repKey handling in pairArms mis-pairs replicates.

🟡 LOW No test for the 'less' alternative — tests/clustered-paired-binary.test.ts

The alternative enum has three values; tests cover 'two-sided' (default) and 'greater' (lines 91, 110, 175). The 'less' branch of isExtreme (clustered-paired-binary.ts:427) is never asserted. isExtreme is a 3-line branch but it is load-bearing for the p-value, so a one-line mirrored case (regressing treatment, alternative:'less') would close the matrix. Not blocking — the 'greater' path exercises the same one-sided comparison logic.

🟡 LOW repKey accessor is wired in the options helper but never exercised with multi-rep rows — tests/clustered-paired-binary.test.ts

options() sets repKey: (value) => value.rep and BinaryRow declares rep?: string, but every test row omits rep, so the multi-rep pairing path (indexByRepKey in paired-arms.ts) and its interaction with cluster summarization is never driven through clusteredPairedBinary in this file. The multi-rep path itself is covered in paired-arms tests, and the cluster math is independent of rep count, so this is a coverage nit not a correctness gap. If desired, add one case with 2 reps in one cluster to confirm repIndex propagation through ClusteredMatchedPair.

---

_{tangletools · 2026-07-09T22:29:55Z · trace}

[tangletools]

❌ 1 Blocking Finding — 0837b244

Full multi-shot audit completed 8/8 planned shots over 17 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 8/8 planned shots over 17 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 8/8 planned shots over 17 changed files. Global verifier still owns final merge decision.

Full immutable report for this review: trace

Summary comment for this run: full summary

---

_{tangletools · 2026-07-09T22:29:55Z · immutable trace}

[tangletools]

✅ Auto-approved drewstone PR — 7dad8824

This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

_{tangletools · auto-approval · reason: drewstone_author · 2026-07-09T22:51:15Z}

[tangletools]

🟢 Value Audit — sound

Verdict	sound
Concerns	1 (1 low)
Heuristic	0.0s
Duplication	0.0s
Interrogation	79.3s (2 bridge agents)
Total	79.3s

💰 Value — sound

Adds the missing cluster-level paired-binary inference (whole-repository bootstrap CI + whole-cluster sign-flip test + Holm step-down) by composing existing pairArms/mulberry32 primitives rather than reinventing them, and no equivalent already exists — ship it.

• What it does: New exported clusteredPairedBinary<TRow> (src/clustered-paired-binary.ts:140) pairs baseline/treatment rows via the existing pairArms, rejects pairs that cross clusters (src/clustered-paired-binary.ts:154), summarizes per-cluster discordant counts, returns a task-weighted risk difference with a whole-cluster percentile bootstrap CI (null when <2 clusters, src/clustered-paired-binary.ts:187,341
• Goals it achieves: Enables trustworthy paired A/B on coding-benchmark data where tasks nest inside repositories: resampling individual tasks (what comparePairedArms/pairedBootstrap already do) violates cluster independence and can declare a different estimand significant, which the tests pin with a 40-repo unequal-size counterexample (tests/clustered-paired-binary.test.ts:92,191). It achieves cluster-aware CI +
• Assessment: Good on its merits and in-grain: it delegates pairing to pairArms and RNG to exported mulberry32 (src/statistics.ts:1392), deliberately does NOT reimplement mcnemar/pairedRiskDifference/pairedBootstrap (those are task-level and cannot express this estimand), validates inputs with ValidationError, returns statistics:null on no matches, and reports unpaired rows — all matching existing
• Better / existing approach: None — this is the right layer and the right composition. Searched src/, tests/, examples/ for holm/clusterBootstrap/clusterSignFlip/whole-cluster/repository-cluster resampling: the only Holm is the new one (src/statistics.ts:856) and no cluster-aware inference exists outside the new file; the existing pairedBootstrap/mcnemar in paired-arms.ts:262 are explicitly task-level, so there is nothing
• Model: opencode/kimi-for-coding/k2p7
• Bridge attempts: 1

🎯 Usefulness — sound

A cluster-aware paired-binary inference primitive that composes the existing pairArms/mulberry32 primitives and fills a genuine statistical gap (whole-repository resampling) the task-level estimators cannot express; exported and documented in the established substrate pattern.

• Integration: Exported on the root barrel at src/index.ts:174-183 (types + function), documented in the SKILL.md surface menu (line 148), CHANGELOG entry, and version-bumped as an additive minor (0.110.1 -> 0.111.0, Python client synced). No in-repo caller beyond its own tests, but this is a substrate package where that is the rule, not the exception: the closest sibling comparePairedArms (src/paired-arms.ts:26
• Fit with existing patterns: Composes the two primitives it names — pairArms (src/paired-arms.ts:103) for pairing and mulberry32 (src/statistics.ts:1392) for deterministic resampling — without reimplementing either. Adds holm (src/statistics.ts:856) as the missing standard multiple-comparison sibling to the already-present bonferroni and benjaminiHochberg. Does not compete with comparePairedArms: that function operates on Pai
• Real-world viability: Matches the package's rigor doctrine: deterministic seed is required (no unseeded randomness in verdict-feeding code), one cluster yields null bootstrap (cannot estimate cluster uncertainty from a single cluster), bootstrapResamples below the minimum for both tails throws before sampling, the bootstrap and sign-flip test the SAME task-weighted estimand (the estimand-mismatch failure mode was caugh
• Model: opencode/zai-coding-plan/glm-5.2
• Bridge attempts: 1

🔎 Heuristic Signals

🟡 Cruft: magic number added tests/clustered-paired-binary.test.ts

• seed: 1337,

---

What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass	What it asks
Heuristic	Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication	Do added function/class names already exist elsewhere in the repo?
Value Audit	What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit	Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

_{value-audit · 20260709T225301Z}

[tangletools]

✅ No Blockers — 7dad8824

Review health 100/100 · Reviewer score 64/100 · Confidence 95/100 · 12 findings (1 medium, 11 low)

	deepseek	kimi-code	aggregate
Readiness	64	83	64
Confidence	95	95	95
Correctness	64	83	64
Security	64	83	64
Testing	64	83	64
Architecture	64	83	64

Reviewer score is advisory once the run is complete and the verdict has no blockers.

Full multi-shot audit completed 8/8 planned shots over 14 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 8/8 planned shots over 14 changed files. Global verifier still owns final merge decision.

🟠 MEDIUM Significance boundary operator inconsistency: holm uses <=, bonferroni uses < — src/statistics.ts

holm rejects at adjusted p <= alpha (line 883: pValue <= alpha), while bonferroni rejects at adjusted p < alpha (line 844: p < alpha). This means a single p-value of 0.05 at default alpha=0.05 yields significant=true in holm but significant=false in bonferroni. The comment on line 882 acknowledges holm's inclusive rule, but the discrepancy between the two sibling family-wise-error functions in the same module is a reproducibility risk for ca

🟡 LOW Audit finding line reference is imprecise — .evolve/critical-audit/2026-07-09T22-35-41Z/findings.jsonl

The finding references line 134 of src/clustered-paired-binary.ts, which is a JSDoc comment (/** Compare binary outcomes... */). The target function clusteredPairedBinary is declared at line 140. A reader tracing this finding to the file would land on the comment rather than the function. Move the line reference to 140.

🟡 LOW No execution environment recorded in manifest — .evolve/critical-audit/2026-07-09T22-35-41Z/manifest.json

The manifest claims 'pnpm test: 270 files, 2752 passed, 2 skipped' but records no node version, pnpm version, or platform. In the current worktree vitest hangs on pnpm test, so the test claim cannot be independently verified. Adding runtime provenance (node --version, pnpm --version, uname -a) would make the claim reproducible.

🟡 LOW Score rationale is implicit — .evolve/critical-audit/2026-07-09T22-35-41Z/manifest.json

The manifest records score: '8/10' with a single LOW finding. The 2-point deduction is not explained — it's unclear whether this is a fixed rubric penalty for missing parity vectors or an aggregate judgment. The summary.md says 'APPROVE' and lists the finding but doesn't tie it to the score. Adding a score_rationale field or inline explanation would clarify what remains to reach 10/10.

🟡 LOW manifest.scope omits 6 of 11 files changed in base..head — .evolve/critical-audit/2026-07-09T22-35-41Z/manifest.json

git diff --name-status 39e8002..abe1d89 shows 11 changed files, but the scope array (L2-8) lists only 5. Omitted: .evolve/skill-runs.jsonl, CHANGELOG.md, clients/python/pyproject.toml, clients/python/src/agent_eval_rpc/init.py, clients/python/uv.lock, package.json (release-prep from 0837b24 + provenance churn). flags=['--diff-only'] plus an APPROVE verdict can be read by a downstream gate as covering the whole base..head change while release-affecting files (version bumps to 0.111.0, CHANGELOG release notes) were never in scope. Given summary.md explicitly frames this record as replacing 'untrustworthy audit records,' the scope should be exhaustive or explicit. Impact: provenance/coverage transparency, not a code defect. Fix: either enumerate every base..head file in scope with per-fil

🟡 LOW PackageNotFoundError fallback for version has no test coverage — clients/python/src/agent_eval_rpc/__init__.py

The except PackageNotFoundError path hardcoding __version__ = '0.111.0' is never exercised by any test — all tests import the installed package so importlib.metadata.version() succeeds. If the fallback value drifts out of sync with pyproject.toml in a future release, no automated check catches it. Low risk: the installed path dominates real usage, and the two values are currently identical. A one-line unit test importing init after temporarily clearing sys.modules would close the gap.

🟡 LOW Non-null assertion on pass! weakens type safety despite runtime correctness — src/clustered-paired-binary.ts

baselinePass: baseline.pass! and treatmentPass: treatment.pass! use non-null assertions because PairedArmRow declares pass as pass?: boolean. projectSelectedRows validates pass is boolean before projection, so runtime undefined is impossible, but the ! escape hatch means a future refactor that weakens projectSelectedRows validation would silently produce undefined instead of a type error. Consider a projected-row type with required pass, or an explicit validation before destructuring.

🟡 LOW as ProjectedRow casts bypass type-level pairing guarantees — src/clustered-paired-binary.ts

pairArms returns PairedArmRow[], then three call sites (lines 152, 172, 175) cast to ProjectedRow to access .clusterKey and .original. Runtime safe because projectSelectedRows always creates ProjectedRow instances, but a future change that feeds unprojected rows to pairArms would produce runtime field-access failures rather than compile errors. Could be hardened by making pairArms generic over a row subtype extending PairedArmRow, but the current pattern matches the existing paired-arms design.

🟡 LOW Inclusive rejection in holm diverges from strict < in sibling bonferroni — src/statistics.ts

holm marks significance with adjusted.map((pValue) => pValue <= alpha) (inclusive), while the adjacent bonferroni (same file, line 844) uses adjusted.map((p) => p < alpha) (strict). For the identical adjusted p-value 0.05 at alpha=0.05 — e.g. input [0.025, 0.025], which both methods adjust to [0.05, 0.05] — holm reports significant=[true,true] but bonferroni reports [false,false]. The inclusive rule is the textbook-correct Holm decision and is explicitly documented and tested here ("handles ties, the alpha boundary"), so this is not a defect in holm itself; but this PR introduces a behavior where two same-module FWER methods disagree at the exact boundary for e

🟡 LOW No non-trivial assertion of the two-sided sign-flip branch — tests/clustered-paired-binary.test.ts

The two-sided alternative is the default, but no test asserts a computed two-sided p-value on discordant data: test 1 asserts no p-value, and test 9's pValue=1 holds for any alternative because the single pair is concordant (statistic 0). The greater (1/8) and less (1/8) branches of isExtreme are pinned, but the Math.abs(candidate) >= Math.abs(observed) - tolerance two-sided branch is only exercised trivially. A regression that breaks the absolute-value comparison would not be caught. Fix: add one small exact case (e.g. 2-3 non-zero clusters with mixed deltas) under the default alternative and assert the exact two-sided p-value by hand, mirroring the existing greater/less exact tests.

🟡 LOW holm tests in unexpected file, missing edge cases — tests/clustered-paired-binary.test.ts

holm tests live in clustered-paired-binary.test.ts (line 327) rather than the natural home in tests/statistics.test.ts alongside bonferroni and benjaminiHochberg tests (line 437+). Coverage is thin: 3 cases cover basic ordering, ties+empty, and input validation, but miss single-p-value, all-significant, all-non-significant families, the monotonicity guarantee, and floating-point behavior. These gaps won't block correctness but reduce regression protection.

🟡 LOW pValue>0.05 pins exact RNG output instead of the estimand property — tests/clustered-paired-binary.test.ts

expect(result.statistics!.signFlip.pValue).toBeGreaterThan(0.05) (with seed 36020260709, signFlipResamples 200000) is a deterministic-but-magic threshold whose truth depends entirely on mulberry32 + SIGN_FLIP_SEED_SALT producing a Monte Carlo p above 0.05. The strong, meaningful assertion here is the line just above it (signFlip.statistic ≈ taskWeightedRiskDifference), which already proves the task-weighted estimand is tested. The p-value gate adds brittleness: any legitimate change to the RNG or seed salt can flip it across 0.05 even though the feature is unchanged. Fix: drop the 0.05 gate or replace it with a property check that does not depend on a fixed seed (e.g. assert the monte-carlo p-value is computed from the task-weighted statistic and is in (0,1)), keeping the statistic-e

---

_{tangletools · 2026-07-09T23:09:16Z · trace}

Superseded by re-review — no blocking findings on latest commit.

[tangletools]

✅ Approved — 12 non-blocking findings — 7dad8824

Full multi-shot audit completed 8/8 planned shots over 14 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 8/8 planned shots over 14 changed files. Global verifier still owns final merge decision.

Full immutable report for this review: trace

Summary comment for this run: full summary

---

_{tangletools · 2026-07-09T23:09:16Z · immutable trace}