Skip to content

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

Merged
gggdttt merged 6 commits into
mainfrom
feature/collect-codereview-from-pr
Jul 24, 2026
Merged

Add 'collect codereview' command: build dataset entries from real PRs#749
gggdttt merged 6 commits into
mainfrom
feature/collect-codereview-from-pr

Conversation

@gggdttt

@gggdttt gggdttt commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

What

New bcbench collect codereview <pr> command that turns a real GitHub PR into a CodeReviewEntry: the PR diff becomes the review patch, and the PR's inline comments become the expected_comments.

Why

Today code-review dataset entries are hand-authored synthetic JSONL. Two workflows need real PRs as entries:

  • Real-PR gold answers — a reviewer writes the expected findings directly as inline comments on a PR (e.g. microsoft/BCApps#9553). --reviewer <login> harvests exactly those.
  • Online-eval reaction harvesting — the reviewer bot's findings on a real PR carry 👍 / 👎 reactions (e.g. microsoft/BCApps#9315). --reacted keeps the positively-reacted findings as gold; a 👎-only comment is dropped, which makes it a false-positive guard by omission (the construct stays in the patch but is absent from expected_comments, so flagging it costs precision).

The two flags compose: --reacted --reviewer "github-actions[bot]" = only the bot's positively-reacted findings.

How

  • collection/collect_codereview.py — mirrors the existing collect_gh (bug-fix) collector but builds a CodeReviewEntry.
    • base_commit is the merge-base of base/head (via the compare API), i.e. exactly what gh pr diff's three-dot diff is computed against, so the patch applies cleanly.
    • Comment selection + (domain, severity) best-effort parsing are pure, unit-tested functions. Unplaceable / reply / LEFT-side comments are skipped.
  • GHClient gains get_merge_base, get_pr_review_comments, get_review_comment_reactions.
  • Schema: ReviewComment.file pattern relaxed to allow spaces — real BC paths contain them (e.g. .../1.Setup Data/...) and the judge matches on the full normalized path, so the exact path must be storable. Matches how project_paths already allows spaces. No other model change; synthetic entries are unaffected (test_dataset_integrity green).
  • CLI: collect codereview subcommand.

Tested

  • tests/test_collect_codereview.py (new, 11 cases): header parsing, reviewer/reacted/compose filtering, reply/side/unanchored skipping, multi-line spans, spaced paths.
  • End-to-end against both real PRs, validated through the real CodeReviewEntry.load:
    • #9553 --reviewer WaelAbuSeada → 16 expected comments, base_commit=e597a54….
    • #9315 --reacted → 2 positively-reacted findings, the 👎 Style comment correctly excluded.
  • test_collect_codereview + test_codereview + test_collect_gh + test_dataset_integrity green; ruff check / ruff format clean.

Depends on the same category as #748 (fetch base_commit on demand) — that PR lets the fetched real-PR base be checked out at eval time. First helper for Accept real PRs as BC Bench code-review dataset entries (ADO 643360); the --reacted path also serves Auto-feed online-eval reactions (ADO 643361).

Harvest a real PR diff as the patch and its inline comments as expected_comments; --reviewer keeps one author's findings, --reacted keeps positively-reacted ones (drops thumbs-down as FP guards by omission). Relax ReviewComment.file to allow spaces in paths. Serves ADO 643360 + 643361.
Copilot AI review requested due to automatic review settings July 23, 2026 09:38
Comment thread src/bcbench/collection/collect_codereview.py Fixed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds collection of code-review dataset entries from real GitHub pull requests.

Changes:

  • Adds reviewer and reaction-based comment harvesting.
  • Adds GitHub APIs for merge bases, comments, and reactions.
  • Supports spaces in reviewed file paths.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/test_collect_codereview.py Tests parsing and comment selection.
src/bcbench/dataset/codereview.py Allows spaces in review paths.
src/bcbench/commands/collect.py Registers the new CLI command.
src/bcbench/collection/gh_client.py Adds required GitHub API operations.
src/bcbench/collection/collect_codereview.py Builds and saves code-review entries.
src/bcbench/collection/__init__.py Exports collection helpers.

Comment thread src/bcbench/collection/gh_client.py Outdated
Comment thread src/bcbench/collection/gh_client.py Outdated
Comment thread src/bcbench/collection/collect_codereview.py Outdated
Comment thread src/bcbench/collection/collect_codereview.py Outdated
Real reviewer-bot comments escape the severity/domain header (Severity\ -\ Performance), so the header regex missed domain. Prefer the <!-- agent_domain: X --> marker the bot always appends; fall back to header parsing for human comments.
Copilot AI review requested due to automatic review settings July 23, 2026 12:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (3)

src/bcbench/collection/gh_client.py:89

  • gh api --paginate emits one JSON array per page, so json.loads(result.stdout) raises JSONDecodeError: Extra data as soon as a PR has more than one page of review comments. Slurp the pages and flatten the outer array before parsing so this method actually returns all comments as documented.
                "--paginate",

src/bcbench/collection/gh_client.py:105

  • This has the same pagination parsing failure as the review-comments endpoint: comments with more than one page of reactions produce concatenated JSON arrays, which json.loads cannot parse. Flatten the paginated output into one array.
                "--paginate",

src/bcbench/collection/collect_codereview.py:90

  • Falling back to original_line re-admits exactly the outdated comments this function says it skips. GitHub keeps original_line populated when the current line becomes null, but that old location is relative to an earlier head and may no longer identify an issue in the collected final patch. Use only the current line fields so outdated findings cannot corrupt the gold set.
    line = comment.get("line") or comment.get("original_line")
    if not line:
        return None  # outdated / unanchored comment
    start = comment.get("start_line") or comment.get("original_start_line")

Comment thread src/bcbench/collection/collect_codereview.py Outdated
Comment thread src/bcbench/commands/collect.py Outdated
…se-insensitive reviewer, remove unused global

- gh_client: add --slurp so multi-page comment/reaction results parse (json.loads previously failed on concatenated page arrays).
- _comment_line_span: use only current line/start_line; drop original_line fallback that re-admitted outdated comments at stale coordinates.
- build_expected_comments: case-insensitive reviewer login match.
- Remove unused module-level _config (CodeQL); clarify --reacted is any-author (combine with --reviewer for bot-only).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 74500e3e-0905-4f26-a2f4-3e6208c4fec2
Copilot AI review requested due to automatic review settings July 23, 2026 13:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

Comment thread src/bcbench/collection/collect_codereview.py Outdated
Comment thread src/bcbench/commands/collect.py Outdated
build_expected_comments and parse_domain_severity are internal helpers (unit-tested directly via the submodule, no package-level consumer). Match the collect_gh convention of exporting only public entry-points.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 74500e3e-0905-4f26-a2f4-3e6208c4fec2
Copilot AI review requested due to automatic review settings July 23, 2026 14:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.

Comment thread src/bcbench/collection/collect_codereview.py
Comment thread src/bcbench/collection/collect_codereview.py Outdated
Comment thread src/bcbench/collection/collect_codereview.py
- parse_domain_severity: prefer an explicit (minor)/(major) tag or a '<sev> Severity' header over the first severity-like word in prose, so a word inside a title (e.g. 'high' in 'high-impact') no longer overrides the annotated severity.

- collect_codereview_entry: in --reacted mode, pre-filter comments (reviewer, placeable line span, non-empty body) before calling the reactions API, so review-heavy PRs no longer spend a request per comment and risk rate limits.

- collect codereview command: catch CollectionError and exit(1) with a clean message instead of a traceback, matching the screen command.

- tests: cover collect_codereview_entry orchestration with a mocked GHClient (call sequence + reload) and the explicit-tag severity case.
Copilot AI review requested due to automatic review settings July 24, 2026 10:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.

Comment thread src/bcbench/collection/collect_codereview.py
Comment thread src/bcbench/collection/collect_codereview.py Outdated
Comment thread src/bcbench/collection/collect_codereview.py Outdated
- Return Severity (not str) from parse_domain_severity and annotate the local, resolving the ty mismatch against ReviewComment.severity.

- Guard the reactions lookup against a None comment id.

- Raise CollectionError instead of typer.Exit from collect_codereview_entry so the CLI command formats the error consistently; drop the now-unused typer import.
Copilot AI review requested due to automatic review settings July 24, 2026 11:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

Comment on lines +168 to +172
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)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good catch. I shall create another bug and then check with Wael how do we want to solve it

Comment thread src/bcbench/collection/collect_codereview.py
@gggdttt
gggdttt merged commit e6c04fe into main Jul 24, 2026
14 checks passed
@gggdttt
gggdttt deleted the feature/collect-codereview-from-pr branch July 24, 2026 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants