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
31 changes: 29 additions & 2 deletions eval/dataset/REVIEWBOT_EVAL.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,31 @@ ONLY_ITEMS=4893 GEN_REPLICATES=1 bash eval/dataset/run_reviewbot_monthly.sh smok
A non-month tag writes `results/reviewbot/REVIEWBOT_smoke.md` and never
touches the month-over-month `INDEX.md`.

The 15 frozen train+val PRs are now merged. The monthly command explicitly
uses `REVIEWBOT_EVAL_ARCHIVED_REPLAY=1`: an evaluation-only adapter presents
their pinned historical open snapshot to the installed ReviewBot Direct
pipeline. It requires `POST_MODE=shadow`, `REVIEW_CONTEXT_MODE=no_discussion`,
and `post=False`, and rejects every non-GET GitHub request. ReviewBot's
production rule remains open PRs only (drafts require an explicit request).
For a future live open-PR dataset, invoke `run_reviewbot_arm.py` directly
without the replay flag.

`goal-eval/expected_pr_bases.json` pins each historical merge base alongside
the existing head pins. The adapter reads the frozen `gt/pr<N>.diff` for file
metadata; GitHub's current file list has drifted for PRs 4816, 4926, and
4977. Before changing any pin, verify all 15 patches against a local
vllm-omni Git checkout:

```bash
python3 eval/dataset/verify_reviewbot_archived_bases.py --repo /path/to/vllm-omni
```

The verifier ignores only Git's display-only object IDs and optional hunk
function names. Each normalized historical `base..head` patch must exactly
equal its frozen ground-truth diff. The base-pin file, selected frozen diffs,
adapter SHA-256s, and replay mode are recorded in each arm manifest, so a
resumed campaign cannot silently mix historical snapshots or adapter versions.

`--dry-run` is deliberately planning-only: it does not require the release
interpreter or manifest and does not probe installed packages.

Expand Down Expand Up @@ -80,8 +105,10 @@ interpreter or manifest and does not probe installed packages.
match them exactly. This strict envelope applies only to
`reviewbot_YYYY-MM`; non-month smoke tags retain legacy-manifest compatibility.
- Child processes receive no `PYTHONPATH`, `PYTHONHOME`, provider source-path
override, or user site. The harness invokes only the selected installed
`python -m omni_reviewbot`; it never imports a sibling ReviewBot checkout.
override, or user site. The harness runs the selected installed ReviewBot
wheel through the evaluation-only archived adapter for these merged PRs;
direct live-PR runs use `python -m omni_reviewbot`. It never imports a
sibling ReviewBot checkout.
- `POST_MODE=shadow` asserted from CLI output — nothing can reach GitHub.
- `REVIEW_CONTEXT_MODE=no_discussion` asserted (`review_context:
no_discussion (0 threads)`): on these PRs the historical review
Expand Down
24 changes: 23 additions & 1 deletion eval/dataset/build_reviewbot_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@
"started_at",
}
_CONFIG_FIELDS = {"release", "env", "review_context_mode", "post_mode"}
_ARCHIVED_REPLAY_FIELDS = {
"mode", "bases_sha256", "diffs_sha256", "adapter_sha256",
}
_ENV_FIELDS = {
"AGENT_PROVIDER",
"REVIEW_MODEL",
Expand Down Expand Up @@ -195,8 +198,19 @@ def _monthly_provenance_problems(
if not isinstance(config, dict):
problems.append("config is not an object")
return problems
if set(config) != _CONFIG_FIELDS:
if set(config) not in (_CONFIG_FIELDS, _CONFIG_FIELDS | {"archived_replay"}):
problems.append("config fields do not match the runner config shape")
if "archived_replay" in config:
replay = config["archived_replay"]
if not isinstance(replay, dict) or set(replay) != _ARCHIVED_REPLAY_FIELDS:
problems.append("config.archived_replay fields do not match")
else:
if replay.get("mode") != "frozen_open_pr_v1":
problems.append("config.archived_replay.mode is invalid")
for name in ("bases_sha256", "diffs_sha256", "adapter_sha256"):
digest = replay.get(name)
if not isinstance(digest, str) or not SHA256_FINGERPRINT.fullmatch(digest):
problems.append(f"config.archived_replay.{name} is invalid")
if config.get("post_mode") != "shadow":
problems.append("config.post_mode must be 'shadow'")
if config.get("review_context_mode") != "no_discussion":
Expand Down Expand Up @@ -520,6 +534,14 @@ def render(tag, aggregate, item_rows, wins, gap_rows, config,
"| dim | Δ | 95% CI | items +/− | n |",
"|---|---|---|---|---|",
]
replay = config.get("archived_replay")
if replay:
lines[6:6] = [
(
"- Dataset: frozen merged-PR shadow replay, historical "
f"base pins `{replay['bases_sha256']}`"
),
]
for dim in DIMS:
a = aggregate[dim]
lines.append(
Expand Down
17 changes: 17 additions & 0 deletions eval/dataset/goal-eval/expected_pr_bases.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"4804": "a25636d7803a2a5da22cb5beae2363eef1f09fad",
"4810": "b443504d619b4a66bfcb5e6f5900e91458dbe11d",
"4816": "a560ed184d9197855f1cccf46cb1cac87b0a7138",
"4817": "e7b281799ec80cabc1493f3c8a623589a95cf1de",
"4825": "d526dc93e89aa4d806a9111aab496b0bc04c8937",
"4837": "431a7c71ac15fc824edfe9dd59c10f7566bde2ed",
"4859": "a8b2bf11d2f2e9282e2182c2aad932d58777e3dc",
"4870": "0798b0bf03cb6e7c4dc40ee58fe12ce492ce255f",
"4893": "ddba6de2ab658a0fdb6f3f72cac9d2c3e2c19aea",
"4923": "ca0ae7269ca3e9487645cf66088fdfc338951da9",
"4926": "5c51782c6bec3c7976c528e5899e89452278c448",
"4950": "4908b85731731da46c4560cbe2b567c94fb032a7",
"4970": "da15a4cb1cfeae3ca02bc075bbe64fe64de59820",
"4977": "b7d08fe5f6b1306eee44172c6589e3225de6897e",
"5009": "cbd3939c3d5099a805397f0cc29a41aa3e9c3235"
}
160 changes: 160 additions & 0 deletions eval/dataset/reviewbot_archived_replay.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
#!/usr/bin/env python3
"""Replay one frozen, now-merged PR through an installed ReviewBot release.

This evaluation-only adapter supplies the historical open snapshot and the
frozen file list to the normal Direct pipeline. The GitHub transport is
read-only, and the pipeline is always called with post=False. Production PR
eligibility and publication code are neither imported from source nor changed.
"""
from __future__ import annotations

import argparse
import os
import re
from dataclasses import replace
from pathlib import Path

SHA = re.compile(r"[0-9a-f]{40}\Z")
FILE_HEADER = re.compile(r"diff --git a/(.+) b/(.+)\Z")


def frozen_files(diff: str) -> list[dict[str, object]]:
"""Project the checked-in Git patch into ReviewBot's ChangedFile fields."""
files: list[dict[str, object]] = []
current: dict[str, object] | None = None
patch: list[str] = []
in_hunk = False

def finish() -> None:
if current is not None:
current["patch"] = "\n".join(patch) + "\n" if patch else None
files.append(current)

for line in diff.splitlines():
if line.startswith("diff --git "):
finish()
match = FILE_HEADER.fullmatch(line)
if match is None or match.group(1) != match.group(2):
raise ValueError(f"unsupported frozen file header: {line}")
current = {
"path": match.group(2), "status": "modified",
"additions": 0, "deletions": 0,
}
patch = []
in_hunk = False
elif current is None:
raise ValueError("frozen diff has content before its first file")
elif line.startswith("new file mode "):
current["status"] = "added"
elif line.startswith("deleted file mode "):
current["status"] = "removed"
elif line.startswith("rename from ") or line.startswith("copy from "):
raise ValueError("frozen diff rename/copy needs explicit projection")
elif line.startswith("Binary files ") or line == "GIT binary patch":
raise ValueError("binary frozen diffs need explicit projection")
elif line.startswith("@@ "):
in_hunk = True
patch.append(line)
elif in_hunk:
patch.append(line)
if line.startswith("+"):
current["additions"] = int(current["additions"]) + 1
elif line.startswith("-"):
current["deletions"] = int(current["deletions"]) + 1
finish()
if not files:
raise ValueError("frozen diff contains no files")
paths = [str(item["path"]) for item in files]
if len(paths) != len(set(paths)):
raise ValueError("frozen diff repeats a file")
return files


def historical_pull(pull, *, pr: int, head: str, base: str):
"""Verify the live archived identity before presenting the frozen view."""
if pull.number != pr or pull.head_sha != head:
raise ValueError(f"pr{pr} no longer has the pinned head")
if pull.state != "closed" or not pull.merged_at:
raise ValueError(f"pr{pr} is not a merged archived PR")
return replace(
pull, state="open", base_sha=base, merged_at="", merge_commit_sha="",
)


def main() -> int:
parser = argparse.ArgumentParser(description=__doc__)
parser.add_argument("--pr", type=int, required=True)
parser.add_argument("--expected-head", required=True)
parser.add_argument("--base", required=True)
parser.add_argument("--diff", type=Path, required=True)
args = parser.parse_args()
if os.environ.get("REVIEWBOT_EVAL_ARCHIVED_REPLAY") != "1":
parser.error("archived replay requires REVIEWBOT_EVAL_ARCHIVED_REPLAY=1")
if os.environ.get("POST_MODE") != "shadow":
parser.error("archived replay requires POST_MODE=shadow")
if os.environ.get("REVIEW_CONTEXT_MODE") != "no_discussion":
parser.error("archived replay requires REVIEW_CONTEXT_MODE=no_discussion")
if args.pr <= 0 or not SHA.fullmatch(args.expected_head) or not SHA.fullmatch(args.base):
parser.error("invalid PR number or pinned commit SHA")
if args.diff.name != f"pr{args.pr}.diff" or not args.diff.is_file():
parser.error("diff must be the frozen patch for the selected PR")

# Import only the paired wheel installed in the runner's private venv.
from omni_reviewbot.cli import build_runtime
from omni_reviewbot.config import Settings
from omni_reviewbot.models import ChangedFile

settings = Settings.from_env()
if settings.post or settings.review_context_mode != "no_discussion":
parser.error("installed ReviewBot settings violate shadow replay")
diff_files = [
ChangedFile(**fields)
for fields in frozen_files(args.diff.read_text(encoding="utf-8"))
]
runtime = build_runtime(settings, recover_running=False)
github = runtime.github
request = github._request

def read_only(method, path, payload=None):
if method != "GET" or payload is not None:
raise RuntimeError("archived evaluation rejects GitHub writes")
return request(method, path, payload)

github._request = read_only
original_get_pull = github.get_pull

def get_pull(number):
if number != args.pr:
raise RuntimeError("archived evaluation rejects another PR")
return historical_pull(
original_get_pull(number),
pr=args.pr, head=args.expected_head, base=args.base,
)

def list_files(number):
if number != args.pr:
raise RuntimeError("archived evaluation rejects another PR")
return list(diff_files)

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,
)
print(outcome.body)
print(f"status: {outcome.status}")
metrics = outcome.metrics or {}
if "review_context_mode" in metrics:
print(
f"review_context: {metrics['review_context_mode']} "
f"({metrics['review_context_count']} threads)"
)
if "changed_files" in metrics:
print(f"changed_files: {metrics['changed_files']}")
if outcome.artifact_path:
print(f"artifact: {outcome.artifact_path}")
return 0


if __name__ == "__main__":
raise SystemExit(main())
Loading
Loading