From a5c3ff6da83e706fed57e8ca169765075f04ccd2 Mon Sep 17 00:00:00 2001 From: Vasily Ilin Date: Sat, 12 Sep 2026 18:55:31 -0700 Subject: [PATCH] Keep accepted resolutions across review integration rounds --- python/azure-review.md | 4 +- python/lean_pool/review.py | 13 ++++- python/lean_pool/review_portions.py | 48 ++++++++++++++++++ python/tests/test_review_portions.py | 74 ++++++++++++++++++++++++++++ 4 files changed, 136 insertions(+), 3 deletions(-) diff --git a/python/azure-review.md b/python/azure-review.md index 3fd5772156..e7a4a11433 100644 --- a/python/azure-review.md +++ b/python/azure-review.md @@ -37,7 +37,9 @@ That review checks the headline contracts and dependencies across modules. It can request exact declaration or file excerpts from the original diff, with up to three source follow-ups when summaries leave a semantic question unanswered. It must explicitly resolve every non-passing portion, finding, and open question -before it can pass; unresolved obligations force discussion. Missing/malformed +before it can pass; unresolved obligations force discussion. Accepted resolutions +persist across follow-ups, and the reviewer can explicitly reopen an obligation +when later evidence changes its assessment. Missing/malformed portion results or integration evidence that cannot fit fail the run rather than silently reducing coverage. Small diffs still use one call per rubric. diff --git a/python/lean_pool/review.py b/python/lean_pool/review.py index f61c2721ad..040fbb4b2d 100644 --- a/python/lean_pool/review.py +++ b/python/lean_pool/review.py @@ -1225,6 +1225,8 @@ def _integrate_portions( bundle = json.loads(evidence) bundle["source_followups"] = [] bundle["integration_history"] = [] + accepted: dict[str, dict] = {} + bundle["accepted_resolutions"] = [] for iteration in range(1, 5): bundle["obligations"] = obligations material = json.dumps(bundle, ensure_ascii=False) @@ -1234,7 +1236,14 @@ def _integrate_portions( raise ValueError( "Integration evidence exceeds budget; no evidence was discarded" ) - final = send(messages) + response = send(messages) + final = replace( + response, + payload=review_portions.accumulate_resolutions( + response.payload, obligations, accepted + ), + ) + bundle["accepted_resolutions"] = list(accepted.values()) queries = final.payload.get("source_requests", []) if not isinstance(queries, list) or any( not isinstance(query, str) or not query.strip() for query in queries @@ -1244,7 +1253,7 @@ def _integrate_portions( ) if not queries: break - bundle["integration_history"].append(final.payload) + bundle["integration_history"].append(response.payload) obligations.update( review_portions.report_obligations( f"integration:{iteration}", final.payload diff --git a/python/lean_pool/review_portions.py b/python/lean_pool/review_portions.py index e8f8429c99..76ef952c0b 100644 --- a/python/lean_pool/review_portions.py +++ b/python/lean_pool/review_portions.py @@ -138,6 +138,12 @@ def integration_instructions() -> str: "definitions/arguments resolve each. Unresolved obligations prevent " "approval. You may retain block/discuss and findings without resolving " "them. Never discard a concrete finding just to obtain approval.\n" + "The `accepted_resolutions` ledger preserves earlier answers. Your " + "`resolutions` may contain only new or updated answers. To retract an " + "earlier answer, return `reopened_obligations`: an array of its IDs. " + "Reopened obligations need fresh evidence in a later round. Record " + "new doubts as findings or open questions, rather than silently " + "forgetting an earlier answer.\n" "When exact source is needed, also return `source_requests`: an array " "of declaration names (qualified names preferred) or exact repository-" "relative file paths. The orchestrator retrieves them from the complete " @@ -200,6 +206,48 @@ def validate_portion_payload(payload: dict) -> None: raise ValueError("Portion findings must be a list") +def accumulate_resolutions( + payload: dict, obligations: dict[str, str], accepted: dict[str, dict] +) -> dict: + """Keep valid earlier evidence until the reviewer explicitly reopens it.""" + resolutions = payload.get("resolutions", []) + reopened = payload.get("reopened_obligations", []) + if not isinstance(resolutions, list): + raise ValueError("Integration resolutions must be a list") + if not isinstance(reopened, list) or any( + not isinstance(item, str) for item in reopened + ): + raise ValueError("Reopened obligations must be a list of IDs") + for identifier in reopened: + accepted.pop(identifier, None) + unknown = [] + for item in resolutions: + if not ( + isinstance(item, dict) + and isinstance(item.get("id"), str) + and isinstance(item.get("evidence"), str) + and item["evidence"].strip() + ): + continue + if item["id"] not in obligations: + unknown.append(item) + elif item["id"] not in reopened: + accepted[item["id"]] = dict(item) + result = dict(payload, resolutions=[*accepted.values(), *unknown]) + for identifier in set(reopened) - obligations.keys(): + result["findings"] = [ + *result.get("findings", []), + { + "file": "", + "line": 0, + "rule": "unknown-review-obligation", + "comment": f"Integration reopened unknown obligation {identifier!r}.", + "evidence": "This ID does not identify any recorded review obligation.", + }, + ] + return result + + def enforce_resolutions(payload: dict, obligations: dict[str, str]) -> dict: """Prevent an integration pass from silently dropping an unresolved issue.""" resolutions = payload.get("resolutions", []) diff --git a/python/tests/test_review_portions.py b/python/tests/test_review_portions.py index f43358f2cf..b328fe83b0 100644 --- a/python/tests/test_review_portions.py +++ b/python/tests/test_review_portions.py @@ -644,3 +644,77 @@ def request_review(**kwargs): saved = json.loads(destination.read_text()) assert len(saved) == 1 assert saved[0]["payload"]["bottom_line"] == "completed faithfulness" + + +@pytest.mark.parametrize("reopen", [False, True]) +def test_integration_keeps_answers_across_followups_unless_reopened(reopen): + """A follow-up retains earlier answers and honors explicit retractions.""" + calls = [] + + def prepare(evidence, rules): + return [{"role": "user", "content": evidence}] + + def send(messages): + bundle = json.loads(messages[0]["content"]) + calls.append(bundle) + if len(calls) == 1: + return result( + { + "verdict": "pass", + "resolutions": [ + { + "id": "1:question:1", + "evidence": "The supplied definitions settle this.", + } + ], + "source_requests": ["Known"], + } + ) + assert bundle["accepted_resolutions"][0]["id"] == "1:question:1" + return result( + { + "verdict": "pass", + "resolutions": [ + { + "id": "integration:1:source:1", + "evidence": "The exact Known definition is now supplied.", + } + ], + "reopened_obligations": ["1:question:1"] if reopen else [], + "source_requests": [], + } + ) + + answer = review._integrate_portions( + "diff --git a/P.lean b/P.lean\n+def Known := 0\n", + "{}", + {"1:question:1": "Is the foundation standard?"}, + 10_000, + prepare, + send, + ) + assert answer.payload["verdict"] == ("discuss" if reopen else "pass") + if reopen: + assert "Is the foundation standard?" in str(answer.payload["findings"]) + else: + assert len(answer.payload["resolutions"]) == 2 + + +def test_unknown_resolution_does_not_enter_the_accepted_ledger(): + """An unrecognized ID cannot gain credibility by surviving a follow-up.""" + accepted = {} + payload = { + "verdict": "pass", + "resolutions": [{"id": "invented", "evidence": "claim"}], + } + updated = review_portions.accumulate_resolutions(payload, {}, accepted) + assert accepted == {} + assert review_portions.enforce_resolutions(updated, {})["verdict"] == "discuss" + + +def test_unknown_reopened_obligation_cannot_approve(): + """An invalid retraction remains visible as an integration concern.""" + updated = review_portions.accumulate_resolutions( + {"verdict": "pass", "reopened_obligations": ["invented"]}, {}, {} + ) + assert review_portions.enforce_resolutions(updated, {})["verdict"] == "discuss"