diff --git a/python/lean_pool/review.py b/python/lean_pool/review.py index b6aefb741a..f61c2721ad 100644 --- a/python/lean_pool/review.py +++ b/python/lean_pool/review.py @@ -1453,6 +1453,7 @@ def run_project_rubrics( verdict = normalize_rubric_verdict(spec, result.payload) print(f"Rubric {spec.key}: {verdict}.", file=sys.stderr) outcomes.append(RubricOutcome(spec=spec, result=result, verdict=verdict)) + _write_review_evidence([outcome.result for outcome in outcomes]) return outcomes diff --git a/python/lean_pool/review_portions.py b/python/lean_pool/review_portions.py index b478be4128..e8f8429c99 100644 --- a/python/lean_pool/review_portions.py +++ b/python/lean_pool/review_portions.py @@ -213,8 +213,13 @@ def enforce_resolutions(payload: dict, obligations: dict[str, str]) -> dict: and item["evidence"].strip() and isinstance(item.get("id"), str) } - if resolved - obligations.keys(): - raise ValueError("Integration cites an unknown review obligation") + unknown = { + f"integration:unknown-resolution:{index}": ( + f"Integration cited unknown obligation {identifier!r}; " + "its resolution was not accepted." + ) + for index, identifier in enumerate(sorted(resolved - obligations.keys()), 1) + } missing = obligations.keys() - resolved final_concerns = { identifier: concern @@ -223,8 +228,8 @@ def enforce_resolutions(payload: dict, obligations: dict[str, str]) -> dict: ).items() if not identifier.endswith(":verdict") } - obligations = obligations | final_concerns - missing |= final_concerns.keys() + obligations = obligations | final_concerns | unknown + missing |= final_concerns.keys() | unknown.keys() if missing: payload = dict(payload) findings = list(payload.get("findings") or []) diff --git a/python/tests/test_review_portions.py b/python/tests/test_review_portions.py index 0c34c7edc5..f43358f2cf 100644 --- a/python/tests/test_review_portions.py +++ b/python/tests/test_review_portions.py @@ -165,8 +165,9 @@ def test_integration_cannot_drop_findings_or_questions(): review_portions.enforce_resolutions(resolved, obligations)["verdict"] == "pass" ) resolved["resolutions"].append({"id": "invented", "evidence": "irrelevant"}) - with pytest.raises(ValueError, match="unknown"): - review_portions.enforce_resolutions(resolved, obligations) + answer = review_portions.enforce_resolutions(resolved, obligations) + assert answer["verdict"] == "discuss" + assert "unknown obligation 'invented'" in str(answer["findings"]) def test_oversized_integration_fails_without_discarding_evidence(monkeypatch): @@ -610,3 +611,36 @@ def test_final_findings_are_reported_once(): ) assert answer["verdict"] == "discuss" assert answer["findings"] == [finding] + + +def test_unknown_resolution_cannot_resolve_its_own_diagnostic(): + """An invented diagnostic ID cannot bypass the unknown-ID safeguard.""" + payload = { + "verdict": "approve", + "resolutions": [ + {"id": "integration:unknown-resolution:1", "evidence": "invented"} + ], + } + answer = review_portions.enforce_resolutions(payload, {}) + assert answer["verdict"] == "needs_discussion" + assert "unknown obligation" in str(answer["findings"]) + + +def test_completed_rubrics_survive_a_later_failure(monkeypatch, tmp_path): + """The evidence artifact retains complete results before the final rubric.""" + destination = tmp_path / "review-evidence.json" + monkeypatch.setenv("REVIEW_EVIDENCE_PATH", str(destination)) + calls = [] + + def request_review(**kwargs): + calls.append(kwargs) + if len(calls) == 2: + raise RuntimeError("later transport failure") + return result({"verdict": "pass", "bottom_line": "completed faithfulness"}) + + monkeypatch.setattr(review, "request_review", request_review) + with pytest.raises(RuntimeError, match="later transport"): + review.run_project_rubrics(review.DEFAULT_MODEL, "source", "xhigh", None, None) + saved = json.loads(destination.read_text()) + assert len(saved) == 1 + assert saved[0]["payload"]["bottom_line"] == "completed faithfulness"