fix(campaign): keep profile matrix evidence exact - #545
Conversation
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — b8582b8f
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-08-04T04:25:05Z
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 | 151.6s (2 bridge agents) |
| Total | 151.6s |
💰 Value — sound
Hardens profile-matrix model evidence (records every paid-call model, rejects mixed/mismatched snapshots, cancels siblings on failure, adds commit+dispatchRef to cache identity) by correctly extending existing dispatchRef/signal/mapConcurrent primitives — ship.
- What it does: Six integrity tightenings on the profile-matrix path: (1) CampaignCellResult now exposes resolvedModels[] (every distinct receipt model) and only exposes resolvedModel when all receipts agree; (2) run-campaign's modelHasSnapshot is rewritten to accept Router -MMDD snapshots while rejecting @preset/ routing selectors and invalid calendar dates; (3) runProfileMatrix rejects duplicate profile ids pre
- Goals it achieves: Guarantee that every RunRecord's pinned model is the exact model that served every paid call in that cell, so profile comparisons and promotion gates cannot be poisoned by mixed, mislabeled, or silently-relabelled model evidence. This is the integrity follow-up to the concurrent-profile-matrix feature landed in 9b7c5f2 — concurrency made the old 'last receipt wins' relabeling dangerous, and this c
- Assessment: Sound and well-grained. The change correctly reuses three pre-existing primitives rather than inventing new ones: dispatchRef already lived on RunCampaignOptions (run-campaign.ts:55) and is used by benchmarks, fixtures, gepa/skillopt/external-text optimization methods; signal already lived on RunCampaignOptions (run-campaign.ts:49) and was already chained campaignAbort→cellAbort→ctx.signal; mapCon
- Better / existing approach: none — this is the right approach. Searched for existing equivalents: dispatchRef (42 matches across the campaign/contract/benchmarks surfaces) is correctly extended, not reinvented; signal plumbing reuses the existing run-campaign abort chain; no pre-existing duplicate-profile-id validator existed (grep found zero matches before this PR); modelHasSnapshot had exactly two copies (run-record.ts + o
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 2
- Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error
🎯 Usefulness — sound
A coherent evidence-integrity hardening of runProfileMatrix that closes real mixed-model relabeling, false-aggregation, and cross-commit cache-reuse holes, threaded through established primitives with no dead surface.
- Integration: Fully wired. runProfileMatrix is the keystone exported from src/campaign/index.ts:259 and consumed by tests/rl/corus-from-matrix.test.ts and the playback adapter. The new resolvedModels field on CampaignCellResult (types.ts:589) is consumed by campaignMeasurementDigest (provenance.ts:580), selfImprove (self-improve.ts:1027), and validated in runProfileMatrix. dispatchRef flows into runCampaign's e
- Fit with existing patterns: Fits the codebase grain exactly. resolvedModels sits alongside the existing resolvedModel on CampaignCellResult and the digest already tracked resolvedModel. dispatchRef mirrors runCampaign.dispatchRef and compareOptimizationMethods.dispatchRef. The signal chain reuses the existing runCampaign→executeCell→ctx.signal wiring rather than inventing a new cancellation path. Duplicate-profile-id and mul
- Real-world viability: Edge paths are covered by new tests and the logic is sound. Mixed models in one cell produce resolvedModels[] with no false resolvedModel (run-campaign.ts:1098-1110); cached-cell resume recomputes evidence from receipts via withCurrentAgentModelEvidence so stale cached models don't leak; moving-alias profiles resolving to different snapshots across cells are rejected; duplicate profile ids rejecte
- 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.
What changed
Direct proof