diff --git a/src/brigade/outcome.py b/src/brigade/outcome.py index 19e8be41..1b7e1ca7 100644 --- a/src/brigade/outcome.py +++ b/src/brigade/outcome.py @@ -400,6 +400,24 @@ def split_by_capability( return CapabilityCohorts(current_capability, pooled, capability, shrunk, off_capability, capability_legacy) +def candidate_regression_withheld(helped: int, hurt: int) -> bool: + """Return True when a candidate cohort should stay withheld for regressions. + + One verified hurt used to withhold the unchanged content-fingerprint cohort + forever. Later verified clears in that same cohort can release the hold once + they strictly outnumber hurts. Append-only evidence is never rewritten; + demotion of an already promoted artifact stays a separate policy. + """ + return hurt > 0 and helped <= hurt + + +def candidate_install_reason(helped: int, hurt: int) -> str: + """Reason string for a candidate install that cleared the hurt gate.""" + if hurt > 0: + return "verified helped outnumbers regressions" + return "verified helped, no regressions" + + def decide( score: OutcomeScore, *, @@ -418,10 +436,15 @@ def decide( return Decision(score.artifact_id, "hold", current_status, "cooldown active") if current_status == "candidate": - if score.hurt > 0: + if candidate_regression_withheld(score.helped, score.hurt): return Decision(score.artifact_id, "hold", "candidate", "withheld: verified regression present") - if score.helped >= config.install_min_helped: - return Decision(score.artifact_id, "install", "promoted", "verified helped, no regressions") + if score.helped >= config.install_min_helped and score.helped > score.hurt: + return Decision( + score.artifact_id, + "install", + "promoted", + candidate_install_reason(score.helped, score.hurt), + ) return Decision(score.artifact_id, "hold", "candidate", "insufficient verified evidence") if current_status == "promoted": diff --git a/src/brigade/scorecard.py b/src/brigade/scorecard.py index dca40ff0..255c4dc4 100644 --- a/src/brigade/scorecard.py +++ b/src/brigade/scorecard.py @@ -768,9 +768,9 @@ def effectiveness_gate_reason( hurt = int(effectiveness.get("hurt", 0)) trials = helped + hurt wilson = outcome_core.wilson_lower_bound(helped, trials, config.z) - if hurt > 0: + if outcome_core.candidate_regression_withheld(helped, hurt): return "withheld: verified regression present" - if helped < config.install_min_helped: + if helped < config.install_min_helped or helped <= hurt: return "insufficient verified evidence" wilson_min = config.effective_wilson_min if wilson < wilson_min: @@ -790,7 +790,10 @@ def dual_gate_passes( utility_reason = utility_gate_reason(card, min_passing_units=config.utility_min_passing_units) if utility_reason is not None: return False, utility_reason - return True, "verified helped, no regressions" + effectiveness = card.dimensions.get("effectiveness", {}) + helped = int(effectiveness.get("helped", 0)) + hurt = int(effectiveness.get("hurt", 0)) + return True, outcome_core.candidate_install_reason(helped, hurt) def decide_scorecard( @@ -826,7 +829,7 @@ def decide_scorecard( return outcome_core.Decision(artifact_id, "hold", current_status, "cooldown active") if current_status == "candidate": - if hurt > 0: + if outcome_core.candidate_regression_withheld(helped, hurt): return outcome_core.Decision(artifact_id, "hold", "candidate", "withheld: verified regression present") passes, reason = dual_gate_passes(card, config=config) if passes: diff --git a/tests/test_outcome.py b/tests/test_outcome.py index f4b502e7..d8ba4366 100644 --- a/tests/test_outcome.py +++ b/tests/test_outcome.py @@ -115,9 +115,9 @@ def test_decide_installs_a_clean_candidate(): assert decision.new_status == "promoted" -def test_decide_withholds_candidate_with_a_verified_regression(): +def test_decide_withholds_candidate_when_regressions_are_not_outnumbered(): decision = outcome.decide( - _score(3, 1), + _score(1, 1), current_status="candidate", last_action_ts=None, now=dt.datetime(2026, 6, 21), @@ -125,6 +125,21 @@ def test_decide_withholds_candidate_with_a_verified_regression(): ) assert decision.action == "hold" assert decision.new_status == "candidate" + assert decision.reason == "withheld: verified regression present" + + +def test_decide_installs_candidate_when_later_clears_outnumber_regression(): + """#629: helped=10 hurt=1 in one unchanged cohort must not withhold forever.""" + decision = outcome.decide( + _score(10, 1), + current_status="candidate", + last_action_ts=None, + now=dt.datetime(2026, 6, 21), + config=outcome.ReconcileConfig(), + ) + assert decision.action == "install" + assert decision.new_status == "promoted" + assert decision.reason == "verified helped outnumbers regressions" def test_decide_withholds_candidate_with_insufficient_evidence(): @@ -180,6 +195,39 @@ def _fp_record(evidence_ref, ts, fingerprint, signal=1): ) +def test_decide_scores_new_fingerprint_cohort_separately_from_regressed_cohort(): + """A content-fingerprint change starts a new cohort; old hurts do not follow.""" + records = [ + _fp_record("r1", "2026-07-01T00:00:00+00:00", "old-rev"), + _fp_record("r2", "2026-07-02T00:00:00+00:00", "old-rev", signal=-1), + _fp_record("r3", "2026-07-03T00:00:00+00:00", "new-rev"), + _fp_record("r4", "2026-07-04T00:00:00+00:00", "new-rev"), + ] + cohorts = outcome.split_by_fingerprint("skill-x", records, "new-rev") + assert (cohorts.current.helped, cohorts.current.hurt) == (2, 0) + assert (cohorts.lifetime.helped, cohorts.lifetime.hurt) == (3, 1) + + current_decision = outcome.decide( + cohorts.current, + current_status="candidate", + last_action_ts=None, + now=dt.datetime(2026, 7, 5), + config=outcome.ReconcileConfig(), + ) + assert current_decision.action == "install" + assert current_decision.reason == "verified helped, no regressions" + + lifetime_decision = outcome.decide( + cohorts.lifetime, + current_status="candidate", + last_action_ts=None, + now=dt.datetime(2026, 7, 5), + config=outcome.ReconcileConfig(), + ) + assert lifetime_decision.action == "install" + assert lifetime_decision.reason == "verified helped outnumbers regressions" + + def test_split_by_fingerprint_grandfathers_legacy_and_drops_proven_stale(): records = [ _fp_record("r1", "2026-07-01T00:00:00+00:00", "old-rev"),