fix(campaign): compare optimization methods on untouched test data#355
Conversation
…ntamination # Conflicts: # CHANGELOG.md # examples/benchmarks/gsm8k/compare-proposers.ts # src/campaign/presets/compare-proposers.ts # src/campaign/presets/run-skill-opt.ts
…ntamination # Conflicts: # CHANGELOG.md # docs/concepts.md # docs/improvement-glossary.md # docs/self-improvement-map.md # examples/compare-proposers-canonical/README.md
…ntamination # Conflicts: # CHANGELOG.md # src/llm-client.test.ts # src/llm-client.ts
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 2b2df3c1
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-23T00:47:28Z
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 2b2df3c1
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-23T00:47:46Z
tangletools
left a comment
There was a problem hiding this comment.
🟢 Value Audit — sound
| Verdict | sound |
| Concerns | 2 (2 low) |
| Heuristic | 0.1s |
| Duplication | 0.0s |
| Interrogation | 228.9s (2 bridge agents) |
| Total | 229.0s |
💰 Value — sound
Replaces the methodologically flawed compareProposers (which let optimizers touch their own final-test data) with compareOptimizationMethods that structurally isolates train/selection/test partitions — the correct fix, built in the grain of the codebase.
- What it does: Introduces compareOptimizationMethods: methods receive only train+selection scenarios in a frozen OptimizationMethodInput (test data is physically absent), all methods finish before any final-test scoring begins, and partitions must be non-empty and pairwise-disjoint (enforced up front). Adds simultaneous Bonferroni-adjusted bootstrap CIs across all method-vs-baseline and method-vs-method contrast
- Goals it achieves: Eliminate optimistic ranking bias: the old compareProposers passed holdoutScenarios into ProposerEntry closures, and SkillOpt adaptively accepted edits on those same scenarios it was later ranked on — the final ranking was contaminated by construction. The new API makes the three-way train/selection/test split a structural invariant rather than a caller convention, so no optimizer can ever see the
- Assessment: This is the standard ML train/validation/test methodology enforced at the type and runtime level — the right design. It reuses existing infrastructure (runCampaign, runImprovementLoop, runSkillOpt, pairedBootstrap, cost-ledger, campaignBreakdown) rather than reinventing; the builtin methods correctly delegate selectionScenarios → the loop's holdoutScenarios gate axis. The structural isolation (tes
- Better / existing approach: none — this is the right approach. Searched for existing equivalents: runImprovementLoop is the single-method loop (complementary, not duplicative — the builtin methods compose it); the old compareProposers was the flawed predecessor being replaced. The train/val/test split with structural input isolation is the canonical correct design; no simpler alternative achieves the same guarantee.
- 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
Replaces the contaminated compareProposers with a train/selection/test-partitioned compareOptimizationMethods that structurally prevents the selection-on-test bias it fixes, wired through builtins, examples, tests, and the public surface.
- Integration: Fully reachable: exported from src/campaign/index.ts:196-208 and re-exported at root; four builtin method adapters (builtin-optimization-methods.ts:47-161) compose runImprovementLoop/runSkillOpt; three runnable examples (canonical, GSM8K, AppWorld) consume it; old compareProposers/*Entry factories fully removed and rejected with fail-closed errors; 722-line test file plus 289-line builtin test exe
- Fit with existing patterns: Matches the established pattern exactly — uses runCampaign as the measurement primitive, the existing CostLedger, pairedBootstrap, surfaceContentHash for dedup, and runImprovementLoop/runSkillOpt as the procedure bodies. No competing implementation; this IS the canonical comparison primitive, replacing the flawed predecessor.
- Real-world viability: Holds up: mapConcurrent bounds optimization concurrency; align() throws on missing scenario scores instead of fabricating zeros; structuredClone isolates scenario mutations between methods AND from caller-owned data (verified by tests at compare-optimization-methods.test.ts:429-517); cost completeness propagates through accountingComplete/incompleteReasons; Bonferroni-derived minimum resamples sca
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
🔎 Heuristic Signals
🟡 Cruft: console debug added examples/benchmarks/gsm8k/compare-optimization-methods.ts
- console.log('GSM8K: gepa-reflection vs gepa-pareto vs skill-opt')
🟡 Cruft: magic number added examples/compare-optimization-methods/index.ts
+const round = (n: number) => Math.round(n * 1000) / 1000
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.
What changed
compareProposersAPI withcompareOptimizationMethods, which compares complete optimization procedures rather than candidate generatorsOptimizationMethodInputand wait for every method to finish before final scoring startsWhy
The old comparison allowed SkillOpt to accept edits on
holdoutScenariosand then ranked methods on those same scenarios. That made the final ranking optimistically biased. The new API separates candidate generation, candidate selection, and final method comparison by construction.Breaking changes
compareProposersbecomescompareOptimizationMethodsproposersbecomesmethodsProposerEntrybecomesOptimizationMethod*Entryfactories become*MethodfactoriestrainScenarios,selectionScenarios, andtestScenariosrunSkillOpt({ holdoutScenarios })becomesrunSkillOpt({ selectionScenarios })Verification
pnpm test: 3,366 passed, 2 skippedpnpm lint: 529 source files checked, 0 errorspnpm typecheckpnpm typecheck:examplespnpm buildpnpm verify:package@tangle-network/agent-eval/contractgit merge-tree --write-tree origin/main HEADThe checked-in June result remains labeled as historical and selection-contaminated. This PR does not claim a fresh method ranking from that artifact.