// Review Velox pull requests for code quality, memory safety, performance, and correctness. Use when reviewing PRs, when asked to review code changes, or when the user mentions "/pr-review".
Draft a commit message for a Velox commit. Use when the user asks to write, draft, or compose a commit message for a Velox change. Encodes the project's content rules so the draft is showable without a separate review pass.
Answer questions about the Velox codebase or pull requests. Use when asked a question via "/query" or when the user wants to understand code, architecture, or implementation details.
| name | pr-review |
| description | Review Velox pull requests for code quality, memory safety, performance, and correctness. Use when reviewing PRs, when asked to review code changes, or when the user mentions "/pr-review". |
Review Velox pull requests focusing on what CI cannot check: code quality, memory safety, concurrency, performance, and correctness. This is performance-critical C++ code for a database execution engine where bugs can cause data corruption, crashes, or security vulnerabilities.
When invoked via /pr-review [additional context] on a GitHub PR, the action
pre-fetches PR metadata and injects it into the prompt. Detect this mode by the
presence of <formatted_context>, <pr_or_issue_body>, and <comments> tags in
the prompt.
The prompt already contains:
Use git commands to get the diff and commit history. The base branch name is in the prompt context (look for PR Branch: -> or the baseBranch field).
# Get the full diff against the base branch
git diff origin/<baseBranch>...HEAD
# Get diff stats
git diff --stat origin/<baseBranch>...HEAD
# Get commit history for this PR
git log origin/<baseBranch>..HEAD --oneline
# If the base branch ref is not available, fetch it first
git fetch origin <baseBranch> --depth=1
Do NOT use gh CLI commands in this mode -- only git commands are available.
All PR metadata, comments, and reviews are already in the prompt context;
only the diff and commit log need to be fetched via git.
If the reviewer provided additional context or instructions after the /pr-review
command, incorporate those into your review focus.
The user provides a PR number or URL:
/pr-review 12345
/pr-review https://github.com/facebookincubator/velox/pull/12345
Use gh CLI to fetch PR data:
gh pr view <PR_NUMBER> --json title,body,author,baseRefName,headRefName,files,additions,deletions,commits
gh pr diff <PR_NUMBER>
gh pr view <PR_NUMBER> --json comments,reviews
Before reviewing, you MUST Read CODING_STYLE.md at the repo root in
full. Do not skim, do not skip — every modified line in the diff must be
checked against it.
The "Common Mistakes" section is the authoritative checklist for the
highest-volume real review hits (/// vs //, abbreviations, *Utils,
undocumented headers, header-body weight, goto, test-first for bug
fixes, naming conventions, assert forms, etc.).
This skill does not maintain a duplicate checklist — CODING_STYLE.md
is the single source of truth. If anything in this skill ever appears to
contradict CODING_STYLE.md, prefer CODING_STYLE.md.
Read through the diff systematically:
The <comments> block in the prompt context contains all prior review
comments — including any from earlier /pr-review invocations on this PR.
Read them before reviewing:
/pr-review was invoked in reply to a specific comment thread, focus
the review on that thread's concerns instead of re-reviewing the whole PR.Read the PR title and body, and walk the self-check in
.claude/skills/write-commit-message/SKILL.md. If 2 or more items fail
(e.g., long lists embedded in sentences, function-by-function walkthroughs,
restating the diff, jargon nouns, long inline code/error strings), include a
single short note in the summary comment asking the author to tighten the
description. Template:
The PR description would read more clearly with a rewrite. Specifically:
- <issue 1, one short sentence>
- <issue 2, one short sentence>
The `write-commit-message` skill at `.claude/skills/write-commit-message/`
can help (it has a self-check + drafting workflow), but any path is fine
as long as the result is clear.
One short paragraph in the summary comment — do not file an inline comment per issue, and do not nag on 0–1 minor issues.
Trace the logic step by step. For each change, consider boundary conditions (empty, null, max size, first/last iteration), failure modes (allocation failures, exceptions, partial state), concurrency (race conditions, lock ordering), and memory safety (ownership, lifetimes, dangling references). Be strict — better to flag a potential issue than miss a real bug. The Review Areas table below enumerates what to check; do not duplicate it as narrative.
Analyze each of these areas thoroughly:
| Area | Focus |
|---|---|
| Correctness & Edge Cases | Logic errors, off-by-one, null/empty handling, boundary conditions, integer overflow, floating point edge cases (NaN, Inf, negative zero) |
| Memory Safety | Use-after-free, double-free, leaks, dangling pointers/references, buffer overflows, ownership/lifetime issues, exception safety |
| Concurrency | Race conditions, data races, deadlocks, lock ordering, thread-safety of shared state |
| Performance | Unnecessary copies (move semantics?), inefficient algorithms, cache-unfriendly access, excessive allocations in hot paths |
| Error Handling | All error paths handled? Exceptions caught appropriately? Informative error messages? Correct use of VELOX_CHECK_* vs VELOX_USER_CHECK_*? |
| Code Quality | RAII, const-correctness, smart pointers, naming conventions, clear structure |
| Testing | Sufficient tests? Edge cases covered? Error paths tested? Using gtest matchers? Bug-fix PRs: does the diff add a test that would fail without the fix? Flag bug fixes that ship code-only. |
The output should be a markdown-formatted summary and should follow the following markdown format exactly:
### Summary
Brief overall assessment (1-2 sentences)
### Issues Found
List any issue, categorized by severity:
- 🔴 **Critical**: Must fix before merge
- 🟡 **Suggestion**: Should consider
- 🟢 **Nitpick**: Minor style issues
Each issue should also include:
- File and line reference
- Description of the issue
- Suggested fix if applicable
### Positive Observations
Note any particularly good patterns or improvements.
Use the mcp__github_inline_comment__create_inline_comment tool to post
comments directly on specific lines in the PR diff. Inline comments should
be used whenever pointing at the exact line adds clarity beyond the summary
comment.
Use inline comments for:
Do NOT use inline comments for:
Always post a summary comment with the overall review. Inline comments supplement the summary — they do not replace it.
When reviewing, consult these project files for context:
CLAUDE.md - Coding style and project guidelinesCODING_STYLE.md - Complete coding style guide