feat(contract): share paired measurement evaluation#432
Conversation
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — daeacd51
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-24T20:15:32Z
tangletools
left a comment
There was a problem hiding this comment.
🟢 Value Audit — sound
| Verdict | sound |
| Concerns | 1 (1 weak-concern) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 164.1s (2 bridge agents) |
| Total | 164.1s |
💰 Value — sound
Cleanly extracts the paired-measurement statistical-decision core into a generic, adapter-driven public evaluator so ordinary profile runs reuse the same ship/hold logic as sealed candidate experiments — behavior-preserving (pinned digest) and in the substrate grain.
- What it does: Extracts the paired-decision computation (significance test, power preflight, cost/latency/objective estimation, contributing checks, ship/hold/need_more_work verdict, execution cost+duration+total) out of the sealed-candidate-only
measuredComparisonFromCandidateExperimentinto a new public generic functionevaluatePairedMeasurements<TRun>(src/contract/measured-comparison.ts:214). The new func - Goals it achieves: Let ordinary (non-sealed) profile-improvement runs feed their own receipt shapes into the exact same paired quality/regression/cost/latency/uncertainty decision that sealed candidate experiments use, without reimplementing the statistics or depending on the candidate-sealing machinery. This is the substrate-reuse doctrine from CLAUDE.md in action: one shared decision primitive at the bottom, two c
- Assessment: A well-scoped, behavior-preserving extraction. The seam (generic run type + adapter) is the right abstraction — it keeps the evaluator free of
CandidateExecutionEvidenceknowledge while the candidate path stays responsible for sealing and provenance. Validation that previously lived in the candidate builder (policy parse, finite measurements, dimension-set consistency) generalized correctly into - Better / existing approach: none — this is the right approach. I grepped src/ for every existing paired evaluator: the only paired machinery is (a) low-level statistical primitives
pairedBootstrap/pairedTTest/wilcoxonSignedRank/heldoutSignificance/powerPreflight, which the new function correctly composes rather than reimplements; (b) RL preference-pairing (paired-by-scenario,pairedEvalueSequence) — a different - Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 2
- Bridge warning: opencode/kimi-for-coding/k2p7: bridge stream ended without value-audit content
🎯 Usefulness — sound
Clean behavior-preserving extraction of the paired ship/hold decision out of the sealed-candidate record builder, exposing it via a product-owned adapter that the repo's own adoption doctrine already calls for.
- Integration: Exported from src/contract/index.ts:178-183 and immediately consumed by the refactored measuredComparisonFromCandidateExperiment (measured-comparison.ts:449) and verifyCandidateExperimentComparison (:507), which themselves back the sealed-candidate record/re-verify path. So the new function is live the moment this merges, not waiting on a caller. The PR-body 'connected-profile execution path' is a
- Fit with existing patterns: Fits the grain. The decision logic (significance + power + regression + cost/latency + budget -> ship/hold/need_more_work) was previously trapped inside the sealed-experiment record builder; extracting it behind PairedMeasurementAdapter is the right factoring. I grepped all sibling paired surfaces (pairedBootstrap in src/statistics.ts:947, heldoutSignificance in campaign/gates/statistical-he
- Real-world viability: Validation is thorough: non-empty measurements, schema-parsed policy (test confirms resamples:0 throws at measured-comparison.test.ts:463), unique cellIds, finite scores, non-negative cost/latency, dimension consistency across all arms. The digest-stability test (measured-comparison.test.ts:529) pins the exact output sha, proving the refactor didn't drift the sealed path. The 6-method adapter is e
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
💰 Value Audit
🟡 Mixed policy-field access style inside evaluatePairedMeasurements [maintenance] ``
measured-comparison.ts:235-242 destructures confidenceLevel/deltaThreshold/minProductiveRuns/budgetUsd/criticalDimensions/regressionTolerance off
policy, but then readspolicy.resamplesandpolicy.bootstrapSeedinline at lines 247-248, 256-257, 280-281, 291-292, 299-300. Purely cosmetic inconsistency; either destructure all or access all. Does not gate shipping.
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.
✅ No Blockers —
|
tangletools
left a comment
There was a problem hiding this comment.
✅ Approved — 4 non-blocking findings — daeacd51
Full multi-shot audit completed 1/1 planned shots over 3 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-24T20:23:57Z · immutable trace
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — cebe2e5a
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-24T20:48:00Z
tangletools
left a comment
There was a problem hiding this comment.
🟢 Value Audit — sound
| Verdict | sound |
| Concerns | 0 (none) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 120.8s (2 bridge agents) |
| Total | 120.8s |
💰 Value — sound
Extracts the sealed-candidate paired-decision computation into a reusable, adapter-driven evaluator so ordinary profile runs reuse the same statistics+checks+outcome logic; clean extraction, no existing equivalent found.
- What it does: Lifts the paired-decision core (composite/dimension/cost/latency bootstrap via measuredEstimate, heldoutSignificance, powerPreflight, budget + critical-dimension + completion/task-pass checks, and the ship/hold/need_more_work outcome) out of measuredComparisonFromCandidateExperiment into a new generic evaluatePairedMeasurements (src/contract/measured-comparison.ts:214). It takes any run shap
- Goals it achieves: Before this, the only place that turned paired baseline/candidate receipts into a ship/hold decision was buried inside the sealed-candidate record builder, so any non-sealed profile-improvement flow would have had to copy ~150 lines of statistics+policy code to get the same quality, regression, cost, latency, power, and budget rigor. After merging, the decision policy is one reusable dependency: c
- Assessment: Good change, executed in the grain of the substrate. The extraction is behavior-preserving: the test pins canonicalCandidateDigest(comparison) to a fixed sha256 (measured-comparison.test.ts:529) and the new PlatformProfileRun test asserts the adapter path produces the same overall/decision/cost numbers as the sealed path for equivalent inputs (test lines 413-467). The sealed builder stays thin and
- Better / existing approach: I searched for an existing equivalent before answering. src/paired-arms.ts (comparePairedArms/pairArms) is the closest peer: it also projects a caller-owned record into a structural shape and runs paired statistics. But it solves a DIFFERENT problem — it does the PAIRING (matching baseline rows to candidate rows by pairKey/repKey, reporting leftovers) and emits raw statistics (McNemar, Wilcoxon, b
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 2
- Bridge warning: opencode/kimi-for-coding/k2p7: bridge stream ended without value-audit content
🎯 Usefulness — sound
Clean extraction of the one paired-decision calculation out of the sealed-candidate builder, generic over the receipt shape via an adapter that matches an established codebase convention; immediately exercised by the existing path and aimed at a plausible imminent external caller.
- Integration: evaluatePairedMeasurements is exported (src/contract/index.ts:179) and already has one real in-repo caller: measuredComparisonFromCandidateExperiment delegates to it via candidateExecutionEvidenceAdapter (src/contract/measured-comparison.ts:451,891). That builder is itself called by verifyCandidateExperimentComparison (line 509) and ~10 test sites, so the extraction is live, not dead. The new gene
- Fit with existing patterns: Fits the grain. The adapter pattern is the repo convention (BenchmarkAdapter src/benchmarks/types.ts:81, HarnessAdapter src/harness-optimizer.ts:54, ReferenceReplayAdapter src/reference-replay.ts:64, WorktreeAdapter, PairedArmRow projection in src/paired-arms.ts). I checked for a competing implementation: src/paired-arms.ts comparePairedArms is the closest neighbor but is a LOWER-level primitive (
- Real-world viability: Validation covers happy and error paths: empty measurements (line 218), non-finite/negative additionalCost (225), duplicate cellIds (229), dimension name/shape mismatch (sharedProjectedDimensions, 826), non-boolean completed/passed (813), non-finite scores (finiteMeasurement, 882). The frozen-policy invariant is preserved on the generic path via agentCandidateEvaluationPolicySchema.parse (217). By
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
No concerns — sound change, no better or existing approach found. ✅
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.
✅ No Blockers —
|
tangletools
left a comment
There was a problem hiding this comment.
✅ Approved — 5 non-blocking findings — cebe2e5a
Full multi-shot audit completed 1/1 planned shots over 3 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-24T20:57:46Z · immutable trace
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — d3b9410f
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-24T21:27:48Z
tangletools
left a comment
There was a problem hiding this comment.
🟢 Value Audit — sound
| Verdict | sound |
| Concerns | 0 (none) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 101.7s (2 bridge agents) |
| Total | 101.7s |
💰 Value — sound
Extracts the sealed-candidate paired-decision (significance+power+objectives+cost+budget+regression → ship/hold/need_more_work) into a generic evaluatePairedMeasurements with a 6-method adapter; clean, behavior-preserving extraction in the codebase's grain.
- What it does: Pulls the statistical decision core out of measuredComparisonFromCandidateExperiment (src/contract/measured-comparison.ts:219) into a new public evaluatePairedMeasurements that takes any baseline/candidate receipt shape through a PairedMeasurementAdapter (score/dimensions/costUsd/latencyMs/completed/passed). The sealed path now delegates via a candidateExecutionEvidenceAdapter (line 90
- Goals it achieves: Make the ONE paired decision calculation reusable for ordinary profile improvements (receipts that aren't sealed candidate experiments) so they get the same quality/regression/cost/latency/uncertainty verdict. Single source of truth for the paired decision; the sealed builder keeps only sealing + record construction. Fits the substrate doctrine in CLAUDE.md (agent-eval exports primitives; the conn
- Assessment: Good change on its merits. The extraction is provably behavior-preserving: the digest-pinning test (measured-comparison.test.ts:643) locks canonicalCandidateDigest(comparison) to a fixed sha256, so the sealed path emits byte-identical output post-refactor. The adapter is narrow (6 accessors), heavily validated (empty/negative/non-finite/duplicate-cell/mismatched-dimension guards in projectRun + th
- Better / existing approach: none — this is the right approach. I checked the three nearest existing paired mechanisms and none is a substitute: (1) campaign/gates/default-production-gate.ts produces a lean GateResult from a GateContext (judgeScores + artifacts), adds red-team/reward-hacking/canary, and cannot emit the rich overall|objectives|decision|power shape the contract layer requires; (2) campaign/gates/promotion-polic
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 2
- Bridge warning: opencode/kimi-for-coding/k2p7: bridge stream ended without value-audit content
🎯 Usefulness — sound
Clean extraction of the paired-decision calculation into a receipt-agnostic, adapter-driven core that the sealed-candidate path already delegates to; matches the codebase's adapter grain and is justified by internal dedup alone.
- Integration: Exported from the /contract barrel (src/contract/index.ts:178-183) and immediately consumed by the sealed-candidate path it was extracted from (measured-comparison.ts:460 delegates via candidateExecutionEvidenceAdapter). No external caller exists yet, but that is expected: per CLAUDE.md this repo is the bottom substrate that agent-runtime/agent-knowledge depend on, and the PR body states the conne
- Fit with existing patterns: Fits the established pattern precisely. The codebase ships multiple receipt->canonical-shape adapters (BenchmarkAdapter at src/benchmarks/types.ts:81, ReferenceReplayAdapter at src/reference-replay.ts:64, VerifierAdapter at src/analyst/adapters.ts:70, and the fromXSession intake adapters); PairedMeasurementAdapter is the same idea. Layering is correct: it sits ABOVE the statistical primitive
- Real-world viability: Pure synchronous function over already-collected measurements, so concurrency is the caller's concern (runCandidateExperiment owns execution concurrency). Validation is thorough and tested: empty matrix, non-finite/negative additional cost, non-boolean scorer-channel, blank cell ids, non-finite scores, non-array/unnamed/repeated dimensions, dimension mismatch across arms, negative cost/latency, no
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
No concerns — sound change, no better or existing approach found. ✅
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.
✅ No Blockers —
|
tangletools
left a comment
There was a problem hiding this comment.
✅ Approved — 2 non-blocking findings — d3b9410f
Full multi-shot audit completed 1/1 planned shots over 3 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-24T21:32:55Z · immutable trace
Problem
Normal profile improvements need the same paired quality, regression, cost, latency, and uncertainty decision as sealed candidate experiments. The only existing calculation was embedded in the sealed-candidate record builder.
Change
Extract
evaluatePairedMeasurementsas the one public paired-decision calculation. It accepts product-owned receipts through a small adapter, validates every measurement and frozen policy, returns execution and total cost, and leaves sealed candidates responsible only for sealing and record construction.Evidence
pnpm install --frozen-lockfilepnpm test— 297 files passed, 2 skipped; 3,381 tests passed, 3 skippedpnpm typecheckpnpm buildpnpm verify:packageScope
No customer run, profile mutation, billing, or activation is included. This is the reusable evaluator dependency for the connected-profile execution path.