Skip to content

Commit 35c8bef

Browse files
authored
Merge pull request #199 from JiusiServe/codex/eval-archive-replay-20260926
Replay archived PR review-quality dataset without changing production eligibility
2 parents 5109fd2 + 0717a0e commit 35c8bef

8 files changed

Lines changed: 457 additions & 7 deletions

‎eval/dataset/REVIEWBOT_EVAL.md‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,31 @@ ONLY_ITEMS=4893 GEN_REPLICATES=1 bash eval/dataset/run_reviewbot_monthly.sh smok
3636
A non-month tag writes `results/reviewbot/REVIEWBOT_smoke.md` and never
3737
touches the month-over-month `INDEX.md`.
3838

39+
The 15 frozen train+val PRs are now merged. The monthly command explicitly
40+
uses `REVIEWBOT_EVAL_ARCHIVED_REPLAY=1`: an evaluation-only adapter presents
41+
their pinned historical open snapshot to the installed ReviewBot Direct
42+
pipeline. It requires `POST_MODE=shadow`, `REVIEW_CONTEXT_MODE=no_discussion`,
43+
and `post=False`, and rejects every non-GET GitHub request. ReviewBot's
44+
production rule remains open PRs only (drafts require an explicit request).
45+
For a future live open-PR dataset, invoke `run_reviewbot_arm.py` directly
46+
without the replay flag.
47+
48+
`goal-eval/expected_pr_bases.json` pins each historical merge base alongside
49+
the existing head pins. The adapter reads the frozen `gt/pr<N>.diff` for file
50+
metadata; GitHub's current file list has drifted for PRs 4816, 4926, and
51+
4977. Before changing any pin, verify all 15 patches against a local
52+
vllm-omni Git checkout:
53+
54+
```bash
55+
python3 eval/dataset/verify_reviewbot_archived_bases.py --repo /path/to/vllm-omni
56+
```
57+
58+
The verifier ignores only Git's display-only object IDs and optional hunk
59+
function names. Each normalized historical `base..head` patch must exactly
60+
equal its frozen ground-truth diff. The base-pin file, selected frozen diffs,
61+
adapter SHA-256s, and replay mode are recorded in each arm manifest, so a
62+
resumed campaign cannot silently mix historical snapshots or adapter versions.
63+
3964
`--dry-run` is deliberately planning-only: it does not require the release
4065
interpreter or manifest and does not probe installed packages.
4166

@@ -80,8 +105,10 @@ interpreter or manifest and does not probe installed packages.
80105
match them exactly. This strict envelope applies only to
81106
`reviewbot_YYYY-MM`; non-month smoke tags retain legacy-manifest compatibility.
82107
- Child processes receive no `PYTHONPATH`, `PYTHONHOME`, provider source-path
83-
override, or user site. The harness invokes only the selected installed
84-
`python -m omni_reviewbot`; it never imports a sibling ReviewBot checkout.
108+
override, or user site. The harness runs the selected installed ReviewBot
109+
wheel through the evaluation-only archived adapter for these merged PRs;
110+
direct live-PR runs use `python -m omni_reviewbot`. It never imports a
111+
sibling ReviewBot checkout.
85112
- `POST_MODE=shadow` asserted from CLI output — nothing can reach GitHub.
86113
- `REVIEW_CONTEXT_MODE=no_discussion` asserted (`review_context:
87114
no_discussion (0 threads)`): on these PRs the historical review

‎eval/dataset/build_reviewbot_report.py‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,9 @@
7171
"started_at",
7272
}
7373
_CONFIG_FIELDS = {"release", "env", "review_context_mode", "post_mode"}
74+
_ARCHIVED_REPLAY_FIELDS = {
75+
"mode", "bases_sha256", "diffs_sha256", "adapter_sha256",
76+
}
7477
_ENV_FIELDS = {
7578
"AGENT_PROVIDER",
7679
"REVIEW_MODEL",
@@ -195,8 +198,19 @@ def _monthly_provenance_problems(
195198
if not isinstance(config, dict):
196199
problems.append("config is not an object")
197200
return problems
198-
if set(config) != _CONFIG_FIELDS:
201+
if set(config) not in (_CONFIG_FIELDS, _CONFIG_FIELDS | {"archived_replay"}):
199202
problems.append("config fields do not match the runner config shape")
203+
if "archived_replay" in config:
204+
replay = config["archived_replay"]
205+
if not isinstance(replay, dict) or set(replay) != _ARCHIVED_REPLAY_FIELDS:
206+
problems.append("config.archived_replay fields do not match")
207+
else:
208+
if replay.get("mode") != "frozen_open_pr_v1":
209+
problems.append("config.archived_replay.mode is invalid")
210+
for name in ("bases_sha256", "diffs_sha256", "adapter_sha256"):
211+
digest = replay.get(name)
212+
if not isinstance(digest, str) or not SHA256_FINGERPRINT.fullmatch(digest):
213+
problems.append(f"config.archived_replay.{name} is invalid")
200214
if config.get("post_mode") != "shadow":
201215
problems.append("config.post_mode must be 'shadow'")
202216
if config.get("review_context_mode") != "no_discussion":
@@ -520,6 +534,14 @@ def render(tag, aggregate, item_rows, wins, gap_rows, config,
520534
"| dim | Δ | 95% CI | items +/− | n |",
521535
"|---|---|---|---|---|",
522536
]
537+
replay = config.get("archived_replay")
538+
if replay:
539+
lines[6:6] = [
540+
(
541+
"- Dataset: frozen merged-PR shadow replay, historical "
542+
f"base pins `{replay['bases_sha256']}`"
543+
),
544+
]
523545
for dim in DIMS:
524546
a = aggregate[dim]
525547
lines.append(
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
{
2+
"4804": "a25636d7803a2a5da22cb5beae2363eef1f09fad",
3+
"4810": "b443504d619b4a66bfcb5e6f5900e91458dbe11d",
4+
"4816": "a560ed184d9197855f1cccf46cb1cac87b0a7138",
5+
"4817": "e7b281799ec80cabc1493f3c8a623589a95cf1de",
6+
"4825": "d526dc93e89aa4d806a9111aab496b0bc04c8937",
7+
"4837": "431a7c71ac15fc824edfe9dd59c10f7566bde2ed",
8+
"4859": "a8b2bf11d2f2e9282e2182c2aad932d58777e3dc",
9+
"4870": "0798b0bf03cb6e7c4dc40ee58fe12ce492ce255f",
10+
"4893": "ddba6de2ab658a0fdb6f3f72cac9d2c3e2c19aea",
11+
"4923": "ca0ae7269ca3e9487645cf66088fdfc338951da9",
12+
"4926": "5c51782c6bec3c7976c528e5899e89452278c448",
13+
"4950": "4908b85731731da46c4560cbe2b567c94fb032a7",
14+
"4970": "da15a4cb1cfeae3ca02bc075bbe64fe64de59820",
15+
"4977": "b7d08fe5f6b1306eee44172c6589e3225de6897e",
16+
"5009": "cbd3939c3d5099a805397f0cc29a41aa3e9c3235"
17+
}
Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
#!/usr/bin/env python3
2+
"""Replay one frozen, now-merged PR through an installed ReviewBot release.
3+
4+
This evaluation-only adapter supplies the historical open snapshot and the
5+
frozen file list to the normal Direct pipeline. The GitHub transport is
6+
read-only, and the pipeline is always called with post=False. Production PR
7+
eligibility and publication code are neither imported from source nor changed.
8+
"""
9+
from __future__ import annotations
10+
11+
import argparse
12+
import os
13+
import re
14+
from dataclasses import replace
15+
from pathlib import Path
16+
17+
SHA = re.compile(r"[0-9a-f]{40}\Z")
18+
FILE_HEADER = re.compile(r"diff --git a/(.+) b/(.+)\Z")
19+
20+
21+
def frozen_files(diff: str) -> list[dict[str, object]]:
22+
"""Project the checked-in Git patch into ReviewBot's ChangedFile fields."""
23+
files: list[dict[str, object]] = []
24+
current: dict[str, object] | None = None
25+
patch: list[str] = []
26+
in_hunk = False
27+
28+
def finish() -> None:
29+
if current is not None:
30+
current["patch"] = "\n".join(patch) + "\n" if patch else None
31+
files.append(current)
32+
33+
for line in diff.splitlines():
34+
if line.startswith("diff --git "):
35+
finish()
36+
match = FILE_HEADER.fullmatch(line)
37+
if match is None or match.group(1) != match.group(2):
38+
raise ValueError(f"unsupported frozen file header: {line}")
39+
current = {
40+
"path": match.group(2), "status": "modified",
41+
"additions": 0, "deletions": 0,
42+
}
43+
patch = []
44+
in_hunk = False
45+
elif current is None:
46+
raise ValueError("frozen diff has content before its first file")
47+
elif line.startswith("new file mode "):
48+
current["status"] = "added"
49+
elif line.startswith("deleted file mode "):
50+
current["status"] = "removed"
51+
elif line.startswith("rename from ") or line.startswith("copy from "):
52+
raise ValueError("frozen diff rename/copy needs explicit projection")
53+
elif line.startswith("Binary files ") or line == "GIT binary patch":
54+
raise ValueError("binary frozen diffs need explicit projection")
55+
elif line.startswith("@@ "):
56+
in_hunk = True
57+
patch.append(line)
58+
elif in_hunk:
59+
patch.append(line)
60+
if line.startswith("+"):
61+
current["additions"] = int(current["additions"]) + 1
62+
elif line.startswith("-"):
63+
current["deletions"] = int(current["deletions"]) + 1
64+
finish()
65+
if not files:
66+
raise ValueError("frozen diff contains no files")
67+
paths = [str(item["path"]) for item in files]
68+
if len(paths) != len(set(paths)):
69+
raise ValueError("frozen diff repeats a file")
70+
return files
71+
72+
73+
def historical_pull(pull, *, pr: int, head: str, base: str):
74+
"""Verify the live archived identity before presenting the frozen view."""
75+
if pull.number != pr or pull.head_sha != head:
76+
raise ValueError(f"pr{pr} no longer has the pinned head")
77+
if pull.state != "closed" or not pull.merged_at:
78+
raise ValueError(f"pr{pr} is not a merged archived PR")
79+
return replace(
80+
pull, state="open", base_sha=base, merged_at="", merge_commit_sha="",
81+
)
82+
83+
84+
def main() -> int:
85+
parser = argparse.ArgumentParser(description=__doc__)
86+
parser.add_argument("--pr", type=int, required=True)
87+
parser.add_argument("--expected-head", required=True)
88+
parser.add_argument("--base", required=True)
89+
parser.add_argument("--diff", type=Path, required=True)
90+
args = parser.parse_args()
91+
if os.environ.get("REVIEWBOT_EVAL_ARCHIVED_REPLAY") != "1":
92+
parser.error("archived replay requires REVIEWBOT_EVAL_ARCHIVED_REPLAY=1")
93+
if os.environ.get("POST_MODE") != "shadow":
94+
parser.error("archived replay requires POST_MODE=shadow")
95+
if os.environ.get("REVIEW_CONTEXT_MODE") != "no_discussion":
96+
parser.error("archived replay requires REVIEW_CONTEXT_MODE=no_discussion")
97+
if args.pr <= 0 or not SHA.fullmatch(args.expected_head) or not SHA.fullmatch(args.base):
98+
parser.error("invalid PR number or pinned commit SHA")
99+
if args.diff.name != f"pr{args.pr}.diff" or not args.diff.is_file():
100+
parser.error("diff must be the frozen patch for the selected PR")
101+
102+
# Import only the paired wheel installed in the runner's private venv.
103+
from omni_reviewbot.cli import build_runtime
104+
from omni_reviewbot.config import Settings
105+
from omni_reviewbot.models import ChangedFile
106+
107+
settings = Settings.from_env()
108+
if settings.post or settings.review_context_mode != "no_discussion":
109+
parser.error("installed ReviewBot settings violate shadow replay")
110+
diff_files = [
111+
ChangedFile(**fields)
112+
for fields in frozen_files(args.diff.read_text(encoding="utf-8"))
113+
]
114+
runtime = build_runtime(settings, recover_running=False)
115+
github = runtime.github
116+
request = github._request
117+
118+
def read_only(method, path, payload=None):
119+
if method != "GET" or payload is not None:
120+
raise RuntimeError("archived evaluation rejects GitHub writes")
121+
return request(method, path, payload)
122+
123+
github._request = read_only
124+
original_get_pull = github.get_pull
125+
126+
def get_pull(number):
127+
if number != args.pr:
128+
raise RuntimeError("archived evaluation rejects another PR")
129+
return historical_pull(
130+
original_get_pull(number),
131+
pr=args.pr, head=args.expected_head, base=args.base,
132+
)
133+
134+
def list_files(number):
135+
if number != args.pr:
136+
raise RuntimeError("archived evaluation rejects another PR")
137+
return list(diff_files)
138+
139+
github.get_pull = get_pull
140+
github.list_files = list_files
141+
outcome = runtime.pipeline.run(
142+
pr_number=args.pr, expected_head=args.expected_head, post=False,
143+
)
144+
print(outcome.body)
145+
print(f"status: {outcome.status}")
146+
metrics = outcome.metrics or {}
147+
if "review_context_mode" in metrics:
148+
print(
149+
f"review_context: {metrics['review_context_mode']} "
150+
f"({metrics['review_context_count']} threads)"
151+
)
152+
if "changed_files" in metrics:
153+
print(f"changed_files: {metrics['changed_files']}")
154+
if outcome.artifact_path:
155+
print(f"artifact: {outcome.artifact_path}")
156+
return 0
157+
158+
159+
if __name__ == "__main__":
160+
raise SystemExit(main())

0 commit comments

Comments
 (0)