diff --git a/eval/dataset/REVIEWBOT_EVAL.md b/eval/dataset/REVIEWBOT_EVAL.md index 95720588..3c4d1f5a 100644 --- a/eval/dataset/REVIEWBOT_EVAL.md +++ b/eval/dataset/REVIEWBOT_EVAL.md @@ -117,7 +117,9 @@ interpreter or manifest and does not probe installed packages. `changed_files` == the frozen GT diff's file count. - Sanitized artifacts (markers stripped, neutral heading) must fit the judge's 24k cap — over-cap is a campaign failure, not a silent - truncation. + truncation. The archived adapter captures the actual inline comment payload + from the ReviewBot publisher and appends it to the evaluation artifact; + ReviewBot's ordinary review-body artifact omits placed inline findings. - Before reporting, `build_reviewbot_report.py --verify` audits exact denominators, pinned judge identity, and fresh candidate sha256s in every verdict (judge_val skips existing files, so its exit code is not diff --git a/eval/dataset/reviewbot_archived_replay.py b/eval/dataset/reviewbot_archived_replay.py index 42cd5959..7d075190 100644 --- a/eval/dataset/reviewbot_archived_replay.py +++ b/eval/dataset/reviewbot_archived_replay.py @@ -81,6 +81,48 @@ def historical_pull(pull, *, pr: int, head: str, base: str): ) +def shadow_candidate(body: str, inline_comments: list[dict]) -> str: + """Include the inline payload that a COMMENT review would publish. + + The normal review-body artifact intentionally omits placed findings. A + judge reading that artifact alone would score a review with inline + findings as though it made none. + """ + if not inline_comments: + return body + if "No actionable findings." in body: + raise ValueError("review body contradicts its inline findings") + parts = [body.rstrip(), "", "### Inline findings", ""] + for comment in inline_comments: + parts.extend([ + f"#### {comment['path']}:{comment['line']} ({comment['side']})", + "", + str(comment["body"]).strip(), + "", + ]) + return "\n".join(parts).rstrip() + "\n" + + +def write_shadow_artifact(outcome, inline_comments: list[dict] | None, state_dir: Path) -> None: + """Complete only this evaluation's shadow artifact with placed comments.""" + if outcome.status != "shadow": + return + metrics = outcome.metrics or {} + if inline_comments is None or metrics.get("inline_placed") != len(inline_comments): + raise RuntimeError("shadow inline payload differs from publication metrics") + if "See inline comments below." in outcome.body and not inline_comments: + raise RuntimeError("review body references missing inline findings") + if outcome.artifact_path is None: + raise RuntimeError("shadow review has no artifact") + artifact = Path(outcome.artifact_path).resolve(strict=True) + if artifact.parent != (state_dir / "artifacts").resolve(): + raise RuntimeError("shadow artifact escaped the evaluation state") + artifact.write_text( + shadow_candidate(artifact.read_text(encoding="utf-8"), inline_comments), + encoding="utf-8", + ) + + def main() -> int: parser = argparse.ArgumentParser(description=__doc__) parser.add_argument("--pr", type=int, required=True) @@ -103,6 +145,7 @@ def main() -> int: from omni_reviewbot.cli import build_runtime from omni_reviewbot.config import Settings from omni_reviewbot.models import ChangedFile + from omni_reviewbot import publication settings = Settings.from_env() if settings.post or settings.review_context_mode != "no_discussion": @@ -138,9 +181,23 @@ def list_files(number): github.get_pull = get_pull github.list_files = list_files - outcome = runtime.pipeline.run( - pr_number=args.pr, expected_head=args.expected_head, post=False, - ) + original_inline = publication.inline_review_comments + captured: list[dict] | None = None + + def capture_inline(*call_args, **call_kwargs): + nonlocal captured + comments, indexes = original_inline(*call_args, **call_kwargs) + captured = list(comments) + return comments, indexes + + publication.inline_review_comments = capture_inline + try: + outcome = runtime.pipeline.run( + pr_number=args.pr, expected_head=args.expected_head, post=False, + ) + finally: + publication.inline_review_comments = original_inline + write_shadow_artifact(outcome, captured, settings.state_dir) print(outcome.body) print(f"status: {outcome.status}") metrics = outcome.metrics or {} diff --git a/test/test_reviewbot_eval.py b/test/test_reviewbot_eval.py index 7ddcbfca..08b72b8b 100644 --- a/test/test_reviewbot_eval.py +++ b/test/test_reviewbot_eval.py @@ -11,6 +11,7 @@ import zipfile from dataclasses import replace from pathlib import Path +from types import SimpleNamespace import pytest @@ -100,6 +101,48 @@ class Pull: archive.historical_pull(stale, pr=4893, head="a" * 40, base="d" * 40) +def test_shadow_candidate_includes_the_posted_inline_payload(): + body = "## Omni ReviewBot review\n\nSummary\n\nSee inline comments below.\n" + comment = { + "path": "models/foo.py", "line": 42, "side": "RIGHT", + "body": "**[major] Missing guard**\n\nThis path can fail.", + } + rendered = archive.shadow_candidate(body, [comment]) + assert "### Inline findings" in rendered + assert "models/foo.py:42 (RIGHT)" in rendered + assert comment["body"] in rendered + assert archive.shadow_candidate(body, []) == body + with pytest.raises(ValueError, match="contradicts"): + archive.shadow_candidate("No actionable findings.\n", [comment]) + + +def test_shadow_artifact_requires_publication_parity_and_evaluation_path(tmp_path): + state = tmp_path / "state" + artifacts = state / "artifacts" + artifacts.mkdir(parents=True) + artifact = artifacts / "pr-4810-head.md" + artifact.write_text("See inline comments below.\n") + comment = { + "path": "foo.py", "line": 7, "side": "RIGHT", + "body": "**[P2] Missing guard**\n\nReason", + } + outcome = SimpleNamespace( + status="shadow", metrics={"inline_placed": 1}, + body="See inline comments below.\n", artifact_path=artifact, + ) + with pytest.raises(RuntimeError, match="differs"): + archive.write_shadow_artifact(outcome, [], state) + assert artifact.read_text() == outcome.body + outcome.artifact_path = tmp_path / "outside.md" + outcome.artifact_path.write_text(outcome.body) + with pytest.raises(RuntimeError, match="escaped"): + archive.write_shadow_artifact(outcome, [comment], state) + assert outcome.artifact_path.read_text() == outcome.body + outcome.artifact_path = artifact + archive.write_shadow_artifact(outcome, [comment], state) + assert comment["body"] in artifact.read_text() + + def test_archived_runner_identity_and_command_are_explicit(): release = {"paired": True} config = arm.build_config(