Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion eval/dataset/REVIEWBOT_EVAL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
63 changes: 60 additions & 3 deletions eval/dataset/reviewbot_archived_replay.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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":
Expand Down Expand Up @@ -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 {}
Expand Down
43 changes: 43 additions & 0 deletions test/test_reviewbot_eval.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import zipfile
from dataclasses import replace
from pathlib import Path
from types import SimpleNamespace

import pytest

Expand Down Expand Up @@ -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(
Expand Down
Loading