diff --git a/benches/_panel.py b/benches/_panel.py index 24a1ada..7a03a0c 100644 --- a/benches/_panel.py +++ b/benches/_panel.py @@ -243,6 +243,8 @@ def validate_r_result( raw_sha256: str, family: str, endpoint_ids: set[str] | None = None, + endpoint_roles: dict[str, str] | None = None, + gating: bool | None = None, analyzer_sha256: str | None = None, ) -> None: if result.get("analysis_schema_version") != 1: @@ -257,12 +259,88 @@ def validate_r_result( raise ValueError("performance inference did not come from R stats") if result.get("adjustment") != "simultaneous Bonferroni intervals across the declared family": raise ValueError("R analysis did not use the declared family-wise adjustment") - if result.get("gate_decision") not in {"PASS", "FAIL", "EXPLORATORY"}: + if result.get("gate_decision") not in { + "PASS", + "INCONCLUSIVE", + "FAIL", + "EXPLORATORY", + }: raise ValueError("R analysis emitted an invalid gate decision") - if endpoint_ids is not None: - analyzed_ids = {endpoint["id"] for endpoint in result.get("endpoints", [])} - if analyzed_ids != endpoint_ids: - raise ValueError("R analysis did not decide the complete endpoint family") + + endpoints = result.get("endpoints") + if not isinstance(endpoints, list) or not endpoints: + raise ValueError("R analysis did not emit endpoint decisions") + allowed_statuses = { + "improvement": {"improved", "inconclusive", "regressed"}, + "non_inferiority": {"non_inferior", "inconclusive", "regressed"}, + "exploratory": {"exploratory"}, + } + analyzed_ids: list[str] = [] + analyzed_roles: dict[str, str] = {} + for endpoint in endpoints: + if not isinstance(endpoint, dict): + raise ValueError( # noqa: TRY004 + "R analysis emitted an invalid endpoint decision" + ) + endpoint_id = endpoint.get("id") + role = endpoint.get("role") + status = endpoint.get("status") + if not isinstance(endpoint_id, str) or not endpoint_id: + raise ValueError("R analysis emitted an invalid endpoint ID") + if ( + not isinstance(role, str) + or not isinstance(status, str) + or role not in allowed_statuses + or status not in allowed_statuses[role] + ): + raise ValueError(f"R analysis emitted an invalid decision for endpoint {endpoint_id}") + analyzed_ids.append(endpoint_id) + analyzed_roles[endpoint_id] = role + if len(analyzed_ids) != len(set(analyzed_ids)): + raise ValueError("R analysis emitted duplicate endpoint decisions") + if endpoint_ids is not None and set(analyzed_ids) != endpoint_ids: + raise ValueError("R analysis did not decide the complete endpoint family") + if endpoint_roles is not None and analyzed_roles != endpoint_roles: + raise ValueError("R analysis changed the declared endpoint roles") + + if gating is None: + gating = result["gate_decision"] != "EXPLORATORY" + if not isinstance(gating, bool): + raise ValueError("R analysis has an invalid gating mode") # noqa: TRY004 + if gating and any(role == "exploratory" for role in analyzed_roles.values()): + raise ValueError("gating R analysis contains an exploratory endpoint") + + expected_failures: list[str] = [] + expected_inconclusive: list[str] = [] + if gating: + expected_failures = [ + endpoint["id"] + for endpoint in endpoints + if endpoint["role"] == "improvement" and endpoint["status"] != "improved" + ] + expected_failures.extend( + endpoint["id"] + for endpoint in endpoints + if endpoint["status"] == "regressed" and endpoint["id"] not in expected_failures + ) + for endpoint in endpoints: + if endpoint["role"] == "non_inferiority" and endpoint["status"] == "inconclusive": + expected_inconclusive.append(endpoint["id"]) + if result.get("failure_endpoints") != expected_failures: + raise ValueError("R analysis failure endpoints disagree with endpoint decisions") + if result.get("inconclusive_endpoints") != expected_inconclusive: + raise ValueError("R analysis inconclusive endpoints disagree with endpoint decisions") + + expected_gate_decision = "EXPLORATORY" + if gating: + if expected_failures: + expected_gate_decision = "FAIL" + elif expected_inconclusive: + expected_gate_decision = "INCONCLUSIVE" + else: + expected_gate_decision = "PASS" + if result["gate_decision"] != expected_gate_decision: + raise ValueError("R gate decision disagrees with endpoint decisions") def validate_absolute_raw(evidence: dict[str, Any]) -> None: diff --git a/benches/ab.py b/benches/ab.py index 0e58305..f56e3ec 100644 --- a/benches/ab.py +++ b/benches/ab.py @@ -287,6 +287,8 @@ def analyze(raw_path: Path, output_path: Path, *, family: str) -> dict[str, Any] raw_sha256=raw_sha256, family=family, endpoint_ids={endpoint["id"] for endpoint in raw["endpoints"]}, + endpoint_roles={endpoint["id"]: endpoint["role"] for endpoint in raw["endpoints"]}, + gating=raw["family"]["gating"], analyzer_sha256=analyzer_sha256, ) return result diff --git a/benches/analyze_ab.R b/benches/analyze_ab.R index d2ef943..7a2cf4a 100644 --- a/benches/analyze_ab.R +++ b/benches/analyze_ab.R @@ -327,16 +327,23 @@ if (length(confirmatory_rows)) { if (!isTRUE(family$gating)) { gate_decision <- "EXPLORATORY" failures <- character() + inconclusive <- character() } else { improvement_failures <- results$id[ results$role == "improvement" & results$status != "improved" ] - noninferiority_failures <- results$id[ - results$role == "non_inferiority" & - !(results$status %in% c("non_inferior", "improved")) + regression_failures <- results$id[results$status == "regressed"] + failures <- unique(c(improvement_failures, regression_failures)) + inconclusive <- results$id[ + results$role == "non_inferiority" & results$status == "inconclusive" ] - failures <- c(improvement_failures, noninferiority_failures) - gate_decision <- if (length(failures)) "FAIL" else "PASS" + gate_decision <- if (length(failures)) { + "FAIL" + } else if (length(inconclusive)) { + "INCONCLUSIVE" + } else { + "PASS" + } } output <- list( @@ -351,7 +358,8 @@ output <- list( adjustment = "simultaneous Bonferroni intervals across the declared family", model = "paired log process means with balanced order term", gate_decision = gate_decision, - failure_endpoints = failures, + failure_endpoints = unname(as.list(failures)), + inconclusive_endpoints = unname(as.list(inconclusive)), planning = list( pairs = family$pairs, interval_family_size = interval_family_size, diff --git a/docs/releasing.md b/docs/releasing.md index 4347b9f..c30fb79 100644 --- a/docs/releasing.md +++ b/docs/releasing.md @@ -9,6 +9,13 @@ R owns the estimates, simultaneous Bonferroni intervals, classifications, and ga R also plans power for the complete family of confirmatory endpoints. A gating family must meet its declared familywise power target. +For a non-inferiority endpoint with regression margin `M`, R classifies an established regression +only when the simultaneous interval's lower bound is greater than `M`. It classifies established +non-inferiority only when the upper bound is less than `M`. An interval that contains `M` is +`INCONCLUSIVE`: it is not a regression, does not block a release, and must not be reported as proof +of non-inferiority. A gating improvement endpoint still fails when it does not establish its +predeclared improvement. + ## Required repository configuration Configure a protected GitHub environment named `pypi`. Limit deployment to release maintainers and @@ -44,9 +51,11 @@ which creates PyPI publish attestations by default. The workflow does not read a 5. Create the matching version tag only after the candidate is approved for publication. The publish job consumes `verified-release` without running a build tool. A failed validation, -build, verification, R-owned guard, absolute-report release gate, evidence, or collection job -prevents publication. Run the guard and absolute report serially on one runner. Parallel execution -creates measurement contention. +build, verification, R-owned guard `FAIL` (an established regression or an unmet improvement +endpoint), absolute-report release gate, evidence, or collection job prevents publication. An +`INCONCLUSIVE` non-inferiority result remains visible in the evidence but is not an established +regression. Run the guard and absolute report serially on one runner. Parallel execution creates +measurement contention. For a version tag, the GitHub release contains both raw files and both R result files. It also contains the benchmark-wheel verification record and the combined release manifest. diff --git a/scripts/release-report.py b/scripts/release-report.py index c0a56d5..24078f0 100644 --- a/scripts/release-report.py +++ b/scripts/release-report.py @@ -126,6 +126,8 @@ def release_guard(path: Path | None = None, raw_path: Path | None = None) -> dic raw_sha256=file_sha256(paired_raw_path), family="release-guard", endpoint_ids={endpoint["id"] for endpoint in raw["endpoints"]}, + endpoint_roles={endpoint["id"]: endpoint["role"] for endpoint in raw["endpoints"]}, + gating=raw["family"]["gating"], analyzer_sha256=file_sha256(ROOT / "benches" / "analyze_ab.R"), ) _validate_release_guard_identity(raw) @@ -135,6 +137,7 @@ def release_guard(path: Path | None = None, raw_path: Path | None = None) -> dic return {"status": f"SUPERSEDED OR INVALID — regenerate with make ab: {error}"} if raw["family"]["name"] != "release-guard" or result["gate_decision"] not in { "PASS", + "INCONCLUSIVE", "FAIL", }: raise SystemExit("release guard does not contain the declared R release decision") diff --git a/tests/test_performance_evidence.py b/tests/test_performance_evidence.py index a57f9de..fd33ef0 100644 --- a/tests/test_performance_evidence.py +++ b/tests/test_performance_evidence.py @@ -468,6 +468,8 @@ def test_r_owns_directional_decisions_order_term_and_insufficient_precision( assert result["engine"] == "R stats" assert result["adjustment"] == ("simultaneous Bonferroni intervals across the declared family") assert result["gate_decision"] == "FAIL" + assert result["failure_endpoints"] == ["slow"] + assert result["inconclusive_endpoints"] == ["noisy"] assert rows["fast"]["status"] == "improved" assert rows["safe"]["status"] == "non_inferior" assert rows["slow"]["status"] == "regressed" @@ -477,6 +479,56 @@ def test_r_owns_directional_decisions_order_term_and_insufficient_precision( assert "neutral" not in json.dumps(result) +def test_r_does_not_treat_an_inconclusive_noninferiority_interval_as_a_regression( + tmp_path: Path, +) -> None: + endpoints = [{"id": "uncertain", "label": "uncertain", "role": "non_inferiority"}] + effects = {"uncertain": (math.log1p(0.031), 0.0, 0.065)} + raw_path, result_path, manifest_path = _synthetic_files(tmp_path, endpoints, effects) + + result = _run_r(raw_path, result_path, manifest_path) + row = result["endpoints"][0] + + assert row["simultaneous_ci_lower_pct"] < 3.0 + assert row["simultaneous_ci_upper_pct"] > 3.0 + assert row["status"] == "inconclusive" + assert result["gate_decision"] == "INCONCLUSIVE" + assert result["failure_endpoints"] == [] + assert result["inconclusive_endpoints"] == ["uncertain"] + + +def test_ab_cli_blocks_only_an_r_fail_decision( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + decisions = iter(("INCONCLUSIVE", "FAIL")) + monkeypatch.setattr(ab, "collect", lambda **kwargs: {}) + monkeypatch.setattr( + ab, + "analyze", + lambda *args, **kwargs: {"gate_decision": next(decisions)}, + ) + monkeypatch.setattr(ab, "_print_result", lambda result: None) + monkeypatch.setattr( + sys, + "argv", + [ + "ab.py", + "--baseline-venv", + "baseline", + "--current-venv", + "current", + "--raw-output", + str(tmp_path / "raw.json"), + "--output", + str(tmp_path / "result.json"), + ], + ) + + ab.main() + with pytest.raises(SystemExit, match="R performance qualification failed"): + ab.main() + + def test_simultaneous_family_interval_blocks_a_nominal_only_improvement( tmp_path: Path, ) -> None: @@ -560,6 +612,9 @@ def test_r_reports_familywise_power_not_single_endpoint_power(tmp_path: Path) -> raw_path, result_path, manifest_path = _synthetic_files(tmp_path, endpoints, effects, pairs=36) result = _run_r(raw_path, result_path, manifest_path) planning = result["planning"] + assert result["gate_decision"] == "PASS" + assert result["failure_endpoints"] == [] + assert result["inconclusive_endpoints"] == [] assert planning["confirmatory_family_size"] == 22 assert planning["family_target_power"] == 0.8 assert planning["per_endpoint_power_target"] == pytest.approx(1 - 0.2 / 22) @@ -780,11 +835,38 @@ def test_r_result_digest_and_engine_fail_closed() -> None: "family": "focused", "adjustment": "simultaneous Bonferroni intervals across the declared family", "gate_decision": "PASS", + "failure_endpoints": [], + "inconclusive_endpoints": [], + "endpoints": [ + {"id": "faster", "role": "improvement", "status": "improved"}, + { + "id": "stable", + "role": "non_inferiority", + "status": "non_inferior", + }, + ], } + endpoint_roles = {"faster": "improvement", "stable": "non_inferiority"} validate_r_result( result, raw_sha256="abc", family="focused", + endpoint_ids=set(endpoint_roles), + endpoint_roles=endpoint_roles, + gating=True, + analyzer_sha256="analyzer-abc", + ) + inconclusive = copy.deepcopy(result) + inconclusive["gate_decision"] = "INCONCLUSIVE" + inconclusive["inconclusive_endpoints"] = ["stable"] + inconclusive["endpoints"][1]["status"] = "inconclusive" + validate_r_result( + inconclusive, + raw_sha256="abc", + family="focused", + endpoint_ids=set(endpoint_roles), + endpoint_roles=endpoint_roles, + gating=True, analyzer_sha256="analyzer-abc", ) for key, value, message in ( @@ -800,10 +882,70 @@ def test_r_result_digest_and_engine_fail_closed() -> None: changed, raw_sha256="abc", family="focused", + endpoint_ids=set(endpoint_roles), + endpoint_roles=endpoint_roles, + gating=True, analyzer_sha256="analyzer-abc", ) +def test_r_result_rejects_disagreement_between_endpoint_and_gate_decisions() -> None: + result = { + "analysis_schema_version": 1, + "engine": "R stats", + "raw_sha256": "abc", + "analyzer_sha256": "analyzer-abc", + "family": "focused", + "adjustment": "simultaneous Bonferroni intervals across the declared family", + "gate_decision": "INCONCLUSIVE", + "failure_endpoints": ["stable"], + "inconclusive_endpoints": [], + "endpoints": [ + { + "id": "stable", + "role": "non_inferiority", + "status": "regressed", + } + ], + } + with pytest.raises(ValueError, match="gate decision disagrees"): + validate_r_result( + result, + raw_sha256="abc", + family="focused", + endpoint_ids={"stable"}, + endpoint_roles={"stable": "non_inferiority"}, + gating=True, + analyzer_sha256="analyzer-abc", + ) + + result["gate_decision"] = "FAIL" + result["failure_endpoints"] = [] + with pytest.raises(ValueError, match="failure endpoints disagree"): + validate_r_result( + result, + raw_sha256="abc", + family="focused", + endpoint_ids={"stable"}, + endpoint_roles={"stable": "non_inferiority"}, + gating=True, + analyzer_sha256="analyzer-abc", + ) + + result["failure_endpoints"] = ["stable"] + result["inconclusive_endpoints"] = ["stable"] + with pytest.raises(ValueError, match="inconclusive endpoints disagree"): + validate_r_result( + result, + raw_sha256="abc", + family="focused", + endpoint_ids={"stable"}, + endpoint_roles={"stable": "non_inferiority"}, + gating=True, + analyzer_sha256="analyzer-abc", + ) + + def test_missing_r_fails_closed_without_python_inference( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: diff --git a/tests/test_release_report.py b/tests/test_release_report.py index f33f37e..a940a73 100644 --- a/tests/test_release_report.py +++ b/tests/test_release_report.py @@ -200,8 +200,8 @@ def test_release_guard_requires_nested_and_irregular_untyped_shapes( raw_path.write_text( json.dumps( { - "family": {"name": "release-guard"}, - "endpoints": [{"id": "untyped-decode@4096"}], + "family": {"name": "release-guard", "gating": True}, + "endpoints": [{"id": "untyped-decode@4096", "role": "non_inferiority"}], } ) ) @@ -216,7 +216,15 @@ def test_release_guard_requires_nested_and_irregular_untyped_shapes( "family": "release-guard", "adjustment": "simultaneous Bonferroni intervals across the declared family", "gate_decision": "PASS", - "endpoints": [{"id": "untyped-decode@4096"}], + "failure_endpoints": [], + "inconclusive_endpoints": [], + "endpoints": [ + { + "id": "untyped-decode@4096", + "role": "non_inferiority", + "status": "non_inferior", + } + ], } ) ) @@ -234,9 +242,9 @@ def test_release_guard_preserves_each_required_untyped_shape( raw_path.write_text( json.dumps( { - "family": {"name": "release-guard"}, + "family": {"name": "release-guard", "gating": True}, "endpoints": [ - {"id": metric} + {"id": metric, "role": "non_inferiority"} for metric in sorted(report_module.REQUIRED_UNTYPED_GUARD_METRICS) ], } @@ -251,8 +259,11 @@ def test_release_guard_preserves_each_required_untyped_shape( "family": "release-guard", "adjustment": "simultaneous Bonferroni intervals across the declared family", "gate_decision": "PASS", + "failure_endpoints": [], + "inconclusive_endpoints": [], "endpoints": [ - {"id": metric} for metric in sorted(report_module.REQUIRED_UNTYPED_GUARD_METRICS) + {"id": metric, "role": "non_inferiority", "status": "non_inferior"} + for metric in sorted(report_module.REQUIRED_UNTYPED_GUARD_METRICS) ], } path.write_text(json.dumps(result)) @@ -264,6 +275,13 @@ def test_release_guard_preserves_each_required_untyped_shape( report_module.REQUIRED_UNTYPED_GUARD_METRICS ) + result["gate_decision"] = "INCONCLUSIVE" + result["endpoints"][0]["status"] = "inconclusive" + result["inconclusive_endpoints"] = [result["endpoints"][0]["id"]] + path.write_text(json.dumps(result)) + evidence = report_module.release_guard(path, raw_path) + assert evidence["analysis"]["gate_decision"] == "INCONCLUSIVE" + def test_release_guard_rejects_an_exploratory_r_decision( report_module: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch @@ -272,9 +290,9 @@ def test_release_guard_rejects_an_exploratory_r_decision( raw_path.write_text( json.dumps( { - "family": {"name": "release-guard"}, + "family": {"name": "release-guard", "gating": True}, "endpoints": [ - {"id": metric} + {"id": metric, "role": "non_inferiority"} for metric in sorted(report_module.REQUIRED_UNTYPED_GUARD_METRICS) ], } @@ -291,8 +309,10 @@ def test_release_guard_rejects_an_exploratory_r_decision( "family": "release-guard", "adjustment": "simultaneous Bonferroni intervals across the declared family", "gate_decision": "EXPLORATORY", + "failure_endpoints": [], + "inconclusive_endpoints": [], "endpoints": [ - {"id": metric} + {"id": metric, "role": "non_inferiority", "status": "non_inferior"} for metric in sorted(report_module.REQUIRED_UNTYPED_GUARD_METRICS) ], } @@ -300,7 +320,8 @@ def test_release_guard_rejects_an_exploratory_r_decision( ) monkeypatch.setattr(report_module, "validate_ab_raw", lambda raw: None) monkeypatch.setattr(report_module, "_validate_release_guard_identity", lambda raw: None) - with pytest.raises(SystemExit, match="declared R release decision"): + monkeypatch.setattr(report_module, "REQUIRE_RELEASE_EVIDENCE", True) + with pytest.raises(SystemExit, match="invalid R-owned release guard evidence"): report_module.release_guard(result_path, raw_path)