Add 'collect codereview' command: build dataset entries from real PRs

src/bcbench/collection/__init__.py

@@ -1,5 +1,11 @@
 """Collection module for gathering dataset entries from GitHub."""
 
+from bcbench.collection.collect_codereview import collect_codereview_entry
 from bcbench.collection.collect_gh import ScreeningResult, collect_gh_entry, screen_gh_candidate
 
-__all__ = ["ScreeningResult", "collect_gh_entry", "screen_gh_candidate"]
+__all__ = [
+    "ScreeningResult",
+    "collect_codereview_entry",
+    "collect_gh_entry",
+    "screen_gh_candidate",
+]

src/bcbench/collection/collect_codereview.py

@@ -0,0 +1,214 @@
+"""Build a code-review dataset entry from a real GitHub pull request.
+
+Turns a PR (its diff) plus a set of expected review comments into a CodeReviewEntry.
+Two ways to decide which of the PR's inline comments are the *expected* findings:
+
+- ``reviewer``: keep comments authored by a given login (e.g. a human reviewer who
+  wrote the gold findings directly on the PR).
+- ``reacted``: keep comments that received a positive reaction (thumbs-up / heart
+  / ...) from any author; combine with ``reviewer`` to restrict to the review bot.
+  Comments with only a thumbs-down are dropped, which turns them into false-positive
+  guards by omission (the construct stays in the patch but is absent from
+  ``expected_comments``, so flagging it costs precision).
+
+If neither is given, every top-level inline comment on the PR is used.
+"""
+
+from __future__ import annotations
+
+from pathlib import Path
+from typing import Any
+
+from bcbench.collection.gh_client import GHClient
+from bcbench.dataset import CodeReviewEntry, ReviewComment, Severity
+from bcbench.dataset.dataset_entry import EntryMetadata
+from bcbench.exceptions import CollectionError
+from bcbench.logger import get_logger
+
+logger = get_logger(__name__)
+
+# GitHub reaction contents that mark a comment as a confirmed (good) finding.
+POSITIVE_REACTIONS = frozenset({"+1", "heart", "hooray", "rocket"})
+
+_SEVERITY_WORDS = ("critical", "high", "medium", "low", "error", "warning", "suggestion", "info", "major", "minor")
+# Words the ReviewComment severity enum doesn't know, mapped to ones it does.
+_SEVERITY_WORD_ALIASES = {"major": "high", "minor": "low"}
+
+
+def parse_domain_severity(body: str) -> tuple[str | None, Severity | None]:
+    """Best-effort extraction of (domain, severity) from a review-comment body.
+
+    Recognises both the reviewer-bot header (``... Medium Severity - Performance ...``)
+    and the ``**Domain - title** _(major)_`` shape humans write. Both fields are
+    optional on ReviewComment, so anything unparsed simply stays ``None``.
+    """
+    import re
+
+    domain: str | None = None
+    # Preferred: the explicit metadata marker the review bot appends. It survives
+    # LaTeX-escaped headers (e.g. "Severity\ \u2014\ Performance") that the header
+    # regex below cannot read.
+    marker = re.search(r"<!--\s*agent_domain:\s*([A-Za-z][\w /-]*?)\s*-->", body)
+    if marker:
+        domain = marker.group(1).strip()
+    else:
+        # Bot header: "<severity> Severity - <Domain>"
+        match = re.search(r"Severity\s*[\u2014-]\s*([A-Za-z][A-Za-z /]+)", body)
+        if match:
+            domain = match.group(1).strip()
+        else:
+            # Human header: leading "**<Domain> - ...**"
+            match = re.match(r"\s*\*\*\s*([A-Za-z][A-Za-z ]+?)\s*[\u2014-]", body)
+            if match:
+                domain = match.group(1).strip()
+
+    severity: Severity | None = None
+    words = "|".join(_SEVERITY_WORDS)
+    # Prefer an explicit annotation over a bare severity-like word that may appear in
+    # prose (e.g. "high" inside "high-impact" in a title). Order: an explicit "(minor)"
+    # tag, then the bot "<severity> Severity" header, then a broad first-word fallback.
+    sev_match = (
+        re.search(r"\(\s*(" + words + r")\s*\)", body, re.IGNORECASE) or re.search(r"\b(" + words + r")\s+severity\b", body, re.IGNORECASE) or re.search(r"\b(" + words + r")\b", body, re.IGNORECASE)
+    )
+    if sev_match:
+        word = sev_match.group(1).lower()
+        word = _SEVERITY_WORD_ALIASES.get(word, word)
+        try:
+            severity = Severity.from_input(word)
+        except ValueError:
+            severity = None
+
+    return domain, severity
+
+
+def _comment_line_span(comment: dict[str, Any]) -> tuple[int, int | None] | None:
+    """Return (line_start, line_end) for a RIGHT-side comment, or None if unplaceable."""
+    if comment.get("in_reply_to_id"):
+        return None  # thread reply, not a standalone finding
+    if (comment.get("side") or "RIGHT") != "RIGHT":
+        return None  # LEFT-side comments reference base lines, not the reviewed diff
+    line = comment.get("line")
+    if not line:
+        return None  # outdated / unanchored: current line no longer exists in the patch
+    start = comment.get("start_line")
+    if start and start != line:
+        return int(start), int(line)
+    return int(line), None
+
+
+def build_expected_comments(
+    comments: list[dict[str, Any]],
+    *,
+    reviewer: str | None,
+    reacted: bool,
+    reactions_by_id: dict[int, list[dict[str, Any]]] | None = None,
+) -> list[ReviewComment]:
+    """Select and convert PR inline comments into expected ReviewComments."""
+    reactions_by_id = reactions_by_id or {}
+    selected: list[ReviewComment] = []
+
+    for comment in comments:
+        if reviewer is not None and ((comment.get("user") or {}).get("login") or "").casefold() != reviewer.casefold():
+            continue
+        if reacted:
+            cid = comment.get("id")
+            reactions = reactions_by_id.get(cid, []) if cid is not None else []
+            contents = {r.get("content") for r in reactions}
+            if not (contents & POSITIVE_REACTIONS):
+                continue
+
+        span = _comment_line_span(comment)
+        if span is None:
+            logger.debug("Skipping comment %s (unplaceable / reply / left-side)", comment.get("id"))
+            continue
+
+        body = (comment.get("body") or "").strip()
+        if not body:
+            continue
+
+        domain, severity = parse_domain_severity(body)
+        selected.append(
+            ReviewComment(
+                file=comment["path"],
+                line_start=span[0],
+                line_end=span[1],
+                body=body,
+                domain=domain,
+                severity=severity,
+            )
+        )
+
+    return selected
+
+
+def collect_codereview_entry(
+    pr_number: int,
+    output: Path,
+    environment_setup_version: str,
+    repo: str = "microsoft/BCApps",
+    reviewer: str | None = None,
+    reacted: bool = False,
+    area: str | None = None,
+) -> CodeReviewEntry:
+    gh_client = GHClient(repo)
+
+    try:
+        logger.info("Collecting code-review entry for PR #%s from %s", pr_number, repo)
+
+        pr_data: dict[str, Any] = gh_client.get_pr_info(pr_number)
+        base_ref = pr_data.get("baseRefOid", "")
+        head_ref = pr_data.get("headRefOid", "")
+        if not base_ref or not head_ref:
+            raise CollectionError("Unable to determine base/head commit from PR data")
+
+        base_commit = gh_client.get_merge_base(base_ref, head_ref)
+        if not base_commit:
+            raise CollectionError("Unable to determine merge-base commit for PR")
+
+        patch = gh_client.get_pr_diff(pr_number)
+        if not patch.strip():
+            raise CollectionError("PR diff is empty")
+
+        comments = gh_client.get_pr_review_comments(pr_number)
+        reactions_by_id: dict[int, list[dict[str, Any]]] = {}
+        if reacted:
+            for comment in comments:
+                cid = comment.get("id")
+                if cid is None:
+                    continue
+                # Only spend a reactions API call on comments that can actually become a
+                # finding: skip other reviewers, replies, LEFT-side/unplaceable and empty
+                # bodies (build_expected_comments applies the same filters again).
+                if reviewer is not None and ((comment.get("user") or {}).get("login") or "").casefold() != reviewer.casefold():
+                    continue
+                if _comment_line_span(comment) is None:
+                    continue
+                if not (comment.get("body") or "").strip():
+                    continue
+                reactions_by_id[cid] = gh_client.get_review_comment_reactions(cid)
+
+        expected_comments = build_expected_comments(comments, reviewer=reviewer, reacted=reacted, reactions_by_id=reactions_by_id)
+        logger.info("Selected %d expected comment(s) from %d PR comment(s)", len(expected_comments), len(comments))
+
+        entry = CodeReviewEntry(
+            repo=repo,
+            instance_id=f"{repo.replace('/', '__')}-{pr_number}",
+            base_commit=base_commit,
+            created_at=pr_data.get("createdAt", ""),
+            environment_setup_version=environment_setup_version,
+            patch=patch,
+            metadata=EntryMetadata(area=area),
+            expected_comments=expected_comments,
+        )
+    except CollectionError:
+        raise
+    except Exception as exc:
+        raise CollectionError(f"Failed to collect code-review entry for PR #{pr_number}: {exc}") from exc
+
+    try:
+        entry.save_to_file(output)
+    except OSError as exc:
+        raise CollectionError(f"Failed to write dataset entry to {output}: {exc}") from exc
+
+    logger.info("Saved code-review entry %s to %s", entry.instance_id, output)
+    return entry

src/bcbench/collection/gh_client.py

@@ -58,6 +58,62 @@ def get_pr_diff(self, pr_number: int) -> str:
         )
         return result.stdout
 
+    def get_merge_base(self, base: str, head: str) -> str:
+        """Return the merge-base SHA of `base` and `head`.
+
+        This is the commit `gh pr diff` (a three-dot diff) is computed against, so a
+        code-review entry that stores it as base_commit can apply the PR patch cleanly.
+        """
+        result = subprocess.run(
+            [
+                "gh",
+                "api",
+                f"/repos/{self.repo}/compare/{base}...{head}",
+                "--jq",
+                ".merge_base_commit.sha",
+            ],
+            capture_output=True,
+            text=True,
+            encoding="utf-8",
+            check=True,
+        )
+        return result.stdout.strip()
+
+    def get_pr_review_comments(self, pr_number: int) -> list[dict[str, Any]]:
+        """Return all inline review comments on a PR (paginated)."""
+        result = subprocess.run(
+            [
+                "gh",
+                "api",
+                f"/repos/{self.repo}/pulls/{pr_number}/comments",
+                "--paginate",
+                "--slurp",
+            ],
+            capture_output=True,
+            text=True,
+            encoding="utf-8",
+            check=True,
+        )
+        # --paginate --slurp wraps each page (itself an array) into an outer array.
+        return [item for page in json.loads(result.stdout) for item in page]
+
+    def get_review_comment_reactions(self, comment_id: int) -> list[dict[str, Any]]:
+        """Return reactions on a single inline review comment."""
+        result = subprocess.run(
+            [
+                "gh",
+                "api",
+                f"/repos/{self.repo}/pulls/comments/{comment_id}/reactions",
+                "--paginate",
+                "--slurp",
+            ],
+            capture_output=True,
+            text=True,
+            encoding="utf-8",
+            check=True,
+        )
+        return [item for page in json.loads(result.stdout) for item in page]
+
     def get_file_content(self, file_path: str, ref: str) -> str:
         # URL-encode the file path to handle spaces and special characters
         encoded_path = quote(file_path, safe="/")

src/bcbench/commands/collect.py

@@ -5,7 +5,7 @@
 import typer
 from typing_extensions import Annotated
 
-from bcbench.collection import ScreeningResult, collect_gh_entry, screen_gh_candidate
+from bcbench.collection import ScreeningResult, collect_codereview_entry, collect_gh_entry, screen_gh_candidate
 from bcbench.config import get_config
 from bcbench.exceptions import CollectionError
 
@@ -38,6 +38,57 @@ def collect_gh(
     collect_gh_entry(pr_number=pr_number, output=output, repo=repo, environment_setup_version=environment_setup_version)
 
 
+@collect_app.command("codereview")
+def collect_codereview(
+    pr_number: Annotated[int, typer.Argument(help="Pull request number to collect")],
+    environment_setup_version: Annotated[
+        str,
+        typer.Option("--environment-setup-version", help="BC environment version to record on the entry (e.g. 27.0)"),
+    ],
+    output: Annotated[Path, typer.Option(help="Path to output dataset file")] = _config.paths.dataset_dir / "codereview.jsonl",
+    repo: Annotated[str, typer.Option(help="GitHub repository in OWNER/REPO format")] = "microsoft/BCApps",
+    reviewer: Annotated[
+        str | None,
+        typer.Option(help="Only use inline comments authored by this GitHub login as expected findings"),
+    ] = None,
+    reacted: Annotated[
+        bool,
+        typer.Option("--reacted", help="Only use comments with a positive reaction (thumbs-up / heart / ...) as expected findings"),
+    ] = False,
+    area: Annotated[str | None, typer.Option(help="Value to record in metadata.area")] = None,
+) -> None:
+    """
+    Build a code-review dataset entry from a real GitHub pull request.
+
+    The PR diff becomes the review patch; its inline comments become the expected
+    findings. Choose which comments count as expected with --reviewer or --reacted
+    (default: all top-level inline comments).
+
+    Example usage:
+
+    # A PR whose expected findings a reviewer wrote directly as inline comments
+    bcbench collect codereview 9553 --reviewer WaelAbuSeada --environment-setup-version 27.0 --area privacy
+
+    # Harvest online-eval reactions: any comment that got a thumbs-up (add
+    # --reviewer <bot-login> to restrict to the review bot)
+    bcbench collect codereview 9315 --reacted --environment-setup-version 27.0
+    """
+    try:
+        entry = collect_codereview_entry(
+            pr_number=pr_number,
+            output=output,
+            environment_setup_version=environment_setup_version,
+            repo=repo,
+            reviewer=reviewer,
+            reacted=reacted,
+            area=area,
+        )
+    except CollectionError as exc:
+        typer.echo(f"Error: {exc}", err=True)
+        raise typer.Exit(code=1) from exc
+    typer.echo(f"Saved {entry.instance_id} with {len(entry.expected_comments)} expected comment(s) to {output}")
+
+
 @collect_app.command("screen")
 def screen(
     pr_number: Annotated[int, typer.Argument(help="Pull request number to screen")],

src/bcbench/dataset/codereview.py

@@ -47,7 +47,7 @@ def from_input(cls, value: str) -> Severity:
 class ReviewComment(BaseModel):
     model_config = ConfigDict(frozen=True)
 
-    file: Annotated[str, Field(pattern=r"^[A-Za-z0-9][A-Za-z0-9./_-]*\.(al|json)$")]
+    file: Annotated[str, Field(pattern=r"^[A-Za-z0-9][A-Za-z0-9 ./_-]*\.(al|json)$")]
     line_start: Annotated[int, Field(ge=1)]
     line_end: Annotated[int, Field(ge=1)] | None = None
     domain: str | None = None