fix(promotion): report the interval used for decisions - #656
Conversation
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — fc4cb300
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-29T06:50:12Z
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 | 290.4s (2 bridge agents) |
| Total | 290.4s |
💰 Value — sound
Fixes the promotion gate to report the statistical interval that actually carried the ship decision (eval's new decision bounds) instead of always reporting descriptive bootstrap CI bounds — correct, minimal, in-grain.
- What it does: In src/runtime/promotion-gate.ts, the lift/latency/costSavings bound fields now read
sig.decision.{low,high}(the interval heldoutSignificance used to decide) instead ofsig.bootstrap.{low,high}(the descriptive bootstrap CI). For continuous outcomes the two are the same; for small-sample/binary outcomes eval decides via an exact sign test whose bounds differ from the bootstrap CI, so the old - Goals it achieves: Correctness and observability of the promotion verdict: the CI bounds a human reads in a PromotionVerdict must be the bounds that governed the promote/no-promote decision — not descriptive diagnostics that, on binary outcomes, contradict the decision. Secondary: bring the cohort's published versions into lockstep with the exact released eval/knowledge commits the gate now depends on.
- Assessment: Coherent and correct. The runtime correctly positions itself as a thin consumer of eval's statistics: it routes the center (mean/median, still from
sig.bootstrap) as diagnostics and the bounds (low/high, now fromsig.decision) as the decision interval, with doc comments at promotion-gate.ts:51-56 and :63 updated to explain the split. The fix touches exactly the three bound-passthrough sites (l - Better / existing approach: none — this is the right approach. Considered exposing BOTH bootstrap and decision bounds on PromotionVerdict for full transparency, but the type already separates diagnostic fields (mean/median) from decision fields (low/high) with explicit docs, so dual bounds would bloat the contract without value. Searched for an existing equivalent (grep'd PromotionVerdict/promotionGate consumers in src/): th
- 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
Fixes a real reported-CI ≠ deciding-CI mismatch in the canonical holdout promotion gate, on a code path already reached by strategy evolution and three display consumers.
- Integration: promotionGate is wired into runStrategyEvolution (src/runtime/strategy-evolution.ts:658), exported from the runtime index, and its lift.low/high output is consumed by three display paths (self-improving-coder.ts:339, strategy-evolution.ts:86, swe-self-improve.mts:73). The corrected interval is on the live promotion path.
- Fit with existing patterns: promotionGate is the documented canonical ship gate (docs/roadmap-rsi.md:71, docs/canonical-api.md:128); no competing capability. The fix reads heldoutSignificance's decision interval — the one that actually computes sig.significant — instead of the diagnostic bootstrap interval, aligning the reported bounds with the verdict.
- Real-world viability: Pure property-swap on an already-computed object: sig.bootstrap.{low,high} → sig.decision.{low,high} in three places (promotion-gate.ts:112-113, 128-129, 163-164). No new control flow. The new binary-outcome test proves the intervals diverge and the verdict now carries the deciding one; all existing edge-case reasons (few-tasks, no-margin, identical-champion) are unchanged.
- 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
Proof