From 4fb8fb21c5f77d03aa4d35db52aebf3b8211bb20 Mon Sep 17 00:00:00 2001 From: Vasily Ilin Date: Sat, 12 Sep 2026 16:30:10 -0700 Subject: [PATCH 1/4] Reject unresolved final integration concerns and correct excerpt units --- python/lean_pool/review.py | 4 +++- python/lean_pool/review_portions.py | 9 ++++++++ python/tests/test_review_portions.py | 32 ++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/python/lean_pool/review.py b/python/lean_pool/review.py index 9af4fc0f72..0adc591326 100644 --- a/python/lean_pool/review.py +++ b/python/lean_pool/review.py @@ -1255,7 +1255,9 @@ def _integrate_portions( review_portions.integration_instructions(), ) available = budget - _message_tokens(updated) - allowance = max(0, (available - 512) // len(queries)) + allowance = max( + 0, int((available - 512) * CHARS_PER_TOKEN_ESTIMATE) // len(queries) + ) bundle["source_followups"].extend( review_portions.source_excerpts(diff, query, allowance) for query in queries ) diff --git a/python/lean_pool/review_portions.py b/python/lean_pool/review_portions.py index 2f3a14bb8f..37a5544199 100644 --- a/python/lean_pool/review_portions.py +++ b/python/lean_pool/review_portions.py @@ -216,6 +216,15 @@ def enforce_resolutions(payload: dict, obligations: dict[str, str]) -> dict: if resolved - obligations.keys(): raise ValueError("Integration cites an unknown review obligation") missing = obligations.keys() - resolved + final_concerns = { + identifier: concern + for identifier, concern in report_obligations( + "integration:final", payload + ).items() + if not identifier.endswith(":verdict") + } + obligations = obligations | final_concerns + missing |= final_concerns.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 9ef524c7f1..b4e0edcc30 100644 --- a/python/tests/test_review_portions.py +++ b/python/tests/test_review_portions.py @@ -496,3 +496,35 @@ def send(messages): if not resolve: assert "Potentially vacuous definition" in str(answer.payload["findings"]) assert "MissingDefinition" in str(answer.payload["findings"]) + + +@pytest.mark.parametrize("field", ["findings", "open_questions", "source_requests"]) +def test_final_integration_concerns_prevent_approval(field): + """A final pass cannot override unresolved concerns in that same report.""" + payload = {"verdict": "pass", field: ["Unresolved semantic question"]} + answer = review_portions.enforce_resolutions(payload, {}) + assert answer["verdict"] == "discuss" + assert "Unresolved semantic question" in str(answer["findings"]) + + +def test_source_excerpt_allowance_uses_characters(monkeypatch): + """An excerpt can use the remaining token budget at the configured ratio.""" + calls = [] + allowances = [] + + def prepare(evidence, rules): + return [{"role": "user", "content": evidence}] + + def send(messages): + calls.append(messages) + return result( + {"verdict": "pass", "source_requests": ["Known"] if len(calls) == 1 else []} + ) + + def excerpt(diff, query, allowance): + allowances.append(allowance) + return {"source": "def Known := 0"} + + monkeypatch.setattr(review_portions, "source_excerpts", excerpt) + review._integrate_portions("def Known := 0", "{}", {}, 10_000, prepare, send) + assert 15_000 < allowances[0] < 20_000 From 7e66d7005bc8aadb0b564c18cc13f5d70c812cac Mon Sep 17 00:00:00 2001 From: Vasily Ilin Date: Sat, 12 Sep 2026 16:34:02 -0700 Subject: [PATCH 2/4] Keep source evidence in portion order for artifact inspection --- python/lean_pool/review.py | 5 ++++- python/tests/test_review_portions.py | 6 ++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/python/lean_pool/review.py b/python/lean_pool/review.py index 0adc591326..dc069e154a 100644 --- a/python/lean_pool/review.py +++ b/python/lean_pool/review.py @@ -1087,6 +1087,7 @@ class ReviewResult: calls: int = 1 coverage: str | None = None requests: tuple[ReviewResult, ...] = () + source_reviews: tuple[ReviewResult, ...] = () def _review_messages( @@ -1207,7 +1208,9 @@ def _review_in_portions( manifest, [r.payload for r in results], shared ) final = _integrate_portions(diff, evidence, obligations, budget, prepare, send) - return replace(final, portions=len(results), coverage=manifest) + return replace( + final, portions=len(results), coverage=manifest, source_reviews=tuple(results) + ) def _integrate_portions( diff --git a/python/tests/test_review_portions.py b/python/tests/test_review_portions.py index b4e0edcc30..15a08a741e 100644 --- a/python/tests/test_review_portions.py +++ b/python/tests/test_review_portions.py @@ -87,6 +87,9 @@ def send(model, messages, effort): "findings": [], "open_questions": [], "evidence_summary": "Reviewed unique_2_499 and local definitions.", + "source_number": int( + re.search(r"source portion (\d+)/", user).group(1) + ), } ) @@ -94,6 +97,9 @@ def send(model, messages, effort): answer = review.request_review(review.DEFAULT_MODEL, "rules", diff, "review") assert answer.payload["verdict"] == "pass" assert answer.portions > 1 + assert [item.payload["source_number"] for item in answer.source_reviews] == list( + range(1, answer.portions + 1) + ) assert answer.calls == answer.portions + 1 == len(calls) assert answer.usage.prompt_tokens == len(calls) * 10 assert answer.usage.completion_tokens == len(calls) * 3 From 37f1ec8060e38762c2f99a9e8daf86a5744d17de Mon Sep 17 00:00:00 2001 From: Vasily Ilin Date: Sat, 12 Sep 2026 16:36:07 -0700 Subject: [PATCH 3/4] Retain failed-review evidence and allow budgeted source request lists --- .github/workflows/llm-review.yml | 4 +-- python/lean_pool/review.py | 11 ++++---- python/tests/test_review_portions.py | 38 ++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/.github/workflows/llm-review.yml b/.github/workflows/llm-review.yml index 444146405f..555c1e3127 100644 --- a/.github/workflows/llm-review.yml +++ b/.github/workflows/llm-review.yml @@ -225,11 +225,11 @@ jobs: run: uv run python -m lean_pool.review - name: Upload review evidence - if: success() && steps.ci.outputs.conclusion == 'success' + if: always() && steps.ci.outputs.conclusion == 'success' uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 with: name: review-evidence - path: ${{ runner.temp }}/review-evidence.json + path: ${{ runner.temp }}/review-evidence* if-no-files-found: ignore retention-days: 30 diff --git a/python/lean_pool/review.py b/python/lean_pool/review.py index dc069e154a..45e32b9770 100644 --- a/python/lean_pool/review.py +++ b/python/lean_pool/review.py @@ -1236,13 +1236,11 @@ def _integrate_portions( ) final = send(messages) queries = final.payload.get("source_requests", []) - if ( - not isinstance(queries, list) - or len(queries) > 20 - or any(not isinstance(query, str) or not query.strip() for query in queries) + if not isinstance(queries, list) or any( + not isinstance(query, str) or not query.strip() for query in queries ): raise ValueError( - "Integration source requests must be at most 20 nonempty strings" + f"Integration source requests must be nonempty strings: {queries!r}" ) if not queries: break @@ -1284,6 +1282,9 @@ def send(self, messages: list[dict[str, str]]) -> ReviewResult: result = _send_review(self.model, messages, self.effort) with self.lock: self.completed.append(result) + if destination := os.environ.get("REVIEW_EVIDENCE_PATH"): + with Path(destination).with_suffix(".calls.jsonl").open("a") as stream: + stream.write(json.dumps(asdict(result), default=vars) + "\n") return result def accounted(self, result: ReviewResult) -> ReviewResult: diff --git a/python/tests/test_review_portions.py b/python/tests/test_review_portions.py index 15a08a741e..820d1b2e25 100644 --- a/python/tests/test_review_portions.py +++ b/python/tests/test_review_portions.py @@ -534,3 +534,41 @@ def excerpt(diff, query, allowance): monkeypatch.setattr(review_portions, "source_excerpts", excerpt) review._integrate_portions("def Known := 0", "{}", {}, 10_000, prepare, send) assert 15_000 < allowances[0] < 20_000 + + +def test_many_source_requests_are_not_discarded(monkeypatch): + """The input budget, rather than an arbitrary request count, bounds retrieval.""" + requested = [f"Definition{index}" for index in range(25)] + retrieved = [] + calls = [] + + def prepare(evidence, rules): + return [{"role": "user", "content": evidence}] + + def send(messages): + calls.append(messages) + return result( + {"verdict": "pass", "source_requests": requested if len(calls) == 1 else []} + ) + + def excerpt(diff, query, allowance): + retrieved.append(query) + return {"query": query, "status": "No matching source"} + + monkeypatch.setattr(review_portions, "source_excerpts", excerpt) + answer = review._integrate_portions("", "{}", {}, 10_000, prepare, send) + assert retrieved == requested + assert answer.payload["verdict"] == "discuss" + + +def test_completed_calls_are_saved_before_later_failure(monkeypatch, tmp_path): + """Successful source work survives an integration or transport failure.""" + destination = tmp_path / "review-evidence.json" + monkeypatch.setenv("REVIEW_EVIDENCE_PATH", str(destination)) + monkeypatch.setattr( + review, "_send_review", lambda *args: result({"verdict": "pass"}) + ) + session = review._ReviewSession(review.DEFAULT_MODEL, "xhigh") + session.send([]) + saved = json.loads(destination.with_suffix(".calls.jsonl").read_text()) + assert saved["payload"]["verdict"] == "pass" From 57f1f0b9b2bdd9fbab7906b60ccd4d26c16ba453 Mon Sep 17 00:00:00 2001 From: Vasily Ilin Date: Sat, 12 Sep 2026 16:38:39 -0700 Subject: [PATCH 4/4] Fit serialized follow-up evidence and avoid duplicate findings --- python/lean_pool/review.py | 39 ++++++++++++++++++++-------- python/lean_pool/review_portions.py | 2 ++ python/tests/test_review_portions.py | 38 +++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 11 deletions(-) diff --git a/python/lean_pool/review.py b/python/lean_pool/review.py index 45e32b9770..b6aefb741a 100644 --- a/python/lean_pool/review.py +++ b/python/lean_pool/review.py @@ -1251,21 +1251,38 @@ def _integrate_portions( ) ) bundle["obligations"] = obligations - updated = prepare( - json.dumps(bundle, ensure_ascii=False), - review_portions.integration_instructions(), - ) - available = budget - _message_tokens(updated) - allowance = max( - 0, int((available - 512) * CHARS_PER_TOKEN_ESTIMATE) // len(queries) - ) - bundle["source_followups"].extend( - review_portions.source_excerpts(diff, query, allowance) for query in queries - ) + if not _fit_source_followups(diff, queries, bundle, budget, prepare): + break payload = review_portions.enforce_resolutions(final.payload, obligations) return replace(final, payload=payload) +def _fit_source_followups( + diff: str, queries: list[str], bundle: dict, budget: int, prepare: Callable +) -> bool: + """Fit the serialized excerpts and their metadata within the next call.""" + rules = review_portions.integration_instructions() + available = budget - _message_tokens( + prepare(json.dumps(bundle, ensure_ascii=False), rules) + ) + allowance = max( + 0, int((available - 512) * CHARS_PER_TOKEN_ESTIMATE) // len(queries) + ) + for _ in range(20): + excerpts = [ + review_portions.source_excerpts(diff, query, allowance) for query in queries + ] + candidate = bundle | {"source_followups": bundle["source_followups"] + excerpts} + messages = prepare(json.dumps(candidate, ensure_ascii=False), rules) + if _message_tokens(messages) <= budget: + bundle["source_followups"] = candidate["source_followups"] + return True + if allowance == 0: + break + allowance //= 2 + return False + + class _ReviewSession: """Account for every call across concurrent portions and context retries.""" diff --git a/python/lean_pool/review_portions.py b/python/lean_pool/review_portions.py index 37a5544199..b478be4128 100644 --- a/python/lean_pool/review_portions.py +++ b/python/lean_pool/review_portions.py @@ -229,6 +229,8 @@ def enforce_resolutions(payload: dict, obligations: dict[str, str]) -> dict: payload = dict(payload) findings = list(payload.get("findings") or []) for identifier in sorted(missing): + if identifier.startswith("integration:final:finding:"): + continue # The concrete finding already appears in this final report. findings.append( { "file": "", diff --git a/python/tests/test_review_portions.py b/python/tests/test_review_portions.py index 820d1b2e25..0c34c7edc5 100644 --- a/python/tests/test_review_portions.py +++ b/python/tests/test_review_portions.py @@ -572,3 +572,41 @@ def test_completed_calls_are_saved_before_later_failure(monkeypatch, tmp_path): session.send([]) saved = json.loads(destination.with_suffix(".calls.jsonl").read_text()) assert saved["payload"]["verdict"] == "pass" + + +def test_followup_budget_includes_serialized_metadata_and_escaping(monkeypatch): + """Escaping and many per-query envelopes cannot overflow the next call.""" + calls = [] + + def prepare(evidence, rules): + return [{"role": "user", "content": evidence}] + + def send(messages): + assert review._message_tokens(messages) <= 10_000 + calls.append(messages) + return result( + { + "verdict": "pass", + "source_requests": [f"D{i}" for i in range(20)] + if len(calls) == 1 + else [], + } + ) + + def excerpt(diff, query, allowance): + return {"query": query, "source": "\\" * allowance, "status": "metadata " * 50} + + monkeypatch.setattr(review_portions, "source_excerpts", excerpt) + answer = review._integrate_portions("", "{}", {}, 10_000, prepare, send) + assert len(calls) == 2 + assert answer.payload["verdict"] == "discuss" + + +def test_final_findings_are_reported_once(): + """A retained finding blocks approval without a duplicate synthetic entry.""" + finding = {"comment": "Unresolved semantic concern"} + answer = review_portions.enforce_resolutions( + {"verdict": "pass", "findings": [finding]}, {} + ) + assert answer["verdict"] == "discuss" + assert answer["findings"] == [finding]