Review Rust code changes for unsafe correctness, security and robustness, documentation quality, and C-to-Rust porting fidelity. Use this when you want to review Rust changes before merging.
Systematically bisect an end-to-end performance regression between two refs (tags/branches/commits) down to the exact offending commit(s). Use this when CI/customer benchmarks show one version is measurably slower than another and you need to attribute the cost to specific PRs.
Run full verification before committing or creating a PR. Use this when you want to create a PR.
Review C code changes for correctness, safety, and style. Use this when you want to review C code changes or PRs.
Compile the project to verify changes build successfully. Use this to verify your changes build properly together with the complete project and dependencies, and make sure to use it before running end to end tests.
Investigate a RediSearch flaky-test report from a Jira key, CI failure, test id, or local logs. Use this to collect evidence, identify a supported root cause, and propose a real fix; if the evidence is insufficient, say so and ask for the missing data instead of suggesting workaround-only fixes or skip_until.
Fix merge conflicts in a jj change and its ancestors, starting from the oldest conflict. Use this when a jj change or its ancestors have conflicts that need resolving.
| name | rust-review |
| description | Review Rust code changes for unsafe correctness, security and robustness, documentation quality, and C-to-Rust porting fidelity. Use this when you want to review Rust changes before merging. |
Review Rust code changes for unsafe correctness, security and robustness risks, documentation quality, and (when applicable) C-to-Rust porting fidelity.
The input specifies what to review. Exactly one of the following forms:
Changesets:
<revset>: A jj revset (when .jj/ is present) — uses jj diff -r <revset>.
Examples: slrzwyul, slrzwyul::vlrzmzvm, @-.<commit> or <commit1>..<commit2>: Git commit(s) (when .jj/ is absent) — uses git diff / git show.pr:<number>: GitHub pull request — fetches the PR branch and reviews locally.Source files or directories:
<path>: Path to a Rust file or directory.<path1> <path2>: Multiple files or directories.If a path doesn't include src/, assume it to be in the src/redisearch_rs directory.
E.g. trie_rs becomes src/redisearch_rs/trie_rs.
If a path points to a directory, review all .rs files in that directory (recursively).
No argument: default to reviewing the uncommitted working-tree changes
(jj diff if .jj/ is present, git diff otherwise).
When reviewing a GitHub PR:
When reviewing a changeset (revset, commits, or PR), obtain the full diff of the
Rust files (.rs):
# Jujutsu changes (when .jj/ is present)
jj diff -r <revset> --git -- glob:'**/*.rs'
# Git commits (when .jj/ is absent)
git diff <commit1>..<commit2> -- '*.rs'
# or for a single commit
git show <commit> -- '*.rs'
For a GitHub PR (pr:<number>), fetch the PR head ref and diff against master:
# When .jj/ is absent:
git fetch origin refs/pull/<number>/head
git diff origin/master...FETCH_HEAD -- '*.rs'
# When .jj/ is present:
git fetch origin refs/pull/<number>/head
jj git import
# Use the fetched SHA directly in the revset
jj diff -r 'master@origin..<sha>' --git -- glob:'**/*.rs'
Read the full source of every Rust file that was added or modified so that you have complete context (not just the diff hunks).
When reviewing source files or directories, there is no diff — read the full source
of every .rs file at the given path(s) and review them in their entirety.
Scan the diff and commit messages for signals that the change re-implements existing C code:
c_entrypoint/*_ffi/If detected, set porting mode = true and identify the original C module(s) by reading
them with /read-unmodified-c-module.
Run every check below on the changed Rust code. For each violation found, record:
Every unsafe fn must have a # Safety section in its doc comment that documents
all pre-conditions the caller must uphold.
Violations:
unsafe fn with no doc comment at all.unsafe fn with a doc comment but no # Safety section.# Safety section that omits a pre-condition required for soundness
(e.g. pointer validity, alignment, aliasing, lifetime, initialized memory).Every unsafe block (or unsafe call inside an unsafe fn) must have a
// SAFETY: comment immediately preceding the unsafe block or call that
explains why every pre-condition of the called function / accessed operation
is satisfied at that call site.
Violations:
// SAFETY: comment.# Safety
section (or the standard library's documented safety requirements).// SAFETY: safe to call) that do not reference
the specific pre-conditions.When a rustdoc comment mentions a Rust symbol (type, function, constant, trait, module, etc.),
it must use an intra-doc link
([Symbol] or [Symbol::method]).
Violations:
`Foo`) inside a doc comment but is not an intra-doc link.Exceptions: symbols that are not Rust items (e.g. C function names, Redis command names, field names used in prose) do not need intra-doc links.
Treat security-sensitive Rust issues as in scope for automated review. Prioritize findings that can lead to panics, undefined behavior, memory unsoundness, data exposure, unauthorized access, or denial of service.
Check especially for:
unsafe and FFI soundness bugs, including invalid pointer, NULL, lifetime, aliasing,
initialization, and ownership assumptions.rm_malloc /
rm_free, while Rust values must be released through the allocator and ownership
path that created them.Box::into_raw,
must have a clear path back to Rust and be converted with Box::from_raw exactly
once so they are cleaned up correctly.String / str or
CString / CStr.mem::transmute and from_raw_parts must validate size,
alignment, initialized memory, lifetime, and valid-value requirements.unwrap / expect,
unsafe conversions, and unchecked UTF-8 or slice assumptions.For any security-sensitive finding, state the concrete impact and the input or code path that can trigger it.
Compare the new Rust implementation against the original C code and verify:
Option, Result).Violations: any semantic divergence that could change observable behavior.
Identify all C/C++ tests that exercise the ported module (look under tests/ for files
that reference the module's functions or types).
For each C/C++ test, verify that an equivalent Rust test exists that covers the same
scenario. Use /check-rust-coverage to confirm
line-level coverage of the new Rust code.
Test placement rules:
pub) functions must be tested in tests/integration/ (integration tests).pub(crate) and private functions should be tested in #[cfg(test)] mod test
(unit tests inside the source file), since integration tests cannot access them.Violations:
mod test instead of tests/integration/.Report only actionable, non-duplicate findings.
For each finding include:
Omit checklist sections with no findings. Do not include "No issues found" for every section. If there are no findings at all, it is fine to give the normal approval, thumbs-up, or no-findings signal.
At the end, provide a short summary:
Blocking violations: any issue in 3a, 3b, 4a, or 4b, plus any 3d issue that can cause memory unsoundness, crashes, data exposure, unauthorized access, or denial of service. Suggestions: issues in 3c (intra-doc links) and low-risk robustness improvements in 3d.