Add Claude Code review skills - #5292
Open
pbrubeck wants to merge 3 commits into
Open
Conversation
pbrubeck
changed the base branch from
release
to
pbrubeck/agents-md-prose-rules
August 8, 2026 18:45
pbrubeck
force-pushed
the
pbrubeck/ai-review-skills
branch
5 times, most recently
from
August 9, 2026 14:01
eefabc6 to
9d0c081
Compare
pbrubeck
marked this pull request as ready for review
August 9, 2026 14:04
Three skills under .claude/skills/, modelled on the equivalent setup in PETSc (.claude/skills/review-mr, review-branch, review-mr-post): * review-pr -- review a GitHub PR, report to stdout * review-branch -- review a local branch against origin/main or origin/release * review-brief -- write a reviewer's brief for a mathematically heavy PR AGENTS.md is the rubric; its Anti-Patterns section is already written to be applied at review time. The review procedure adds Firedrake-specific weighting (parallel-correctness failures as CRITICAL, discretization branching, missing parallel test coverage) and requires each finding to be re-verified against the cited code before it is reported. Stacked PRs are reviewed against their own base, never against main, so the parent's changes stay out of scope. review-brief exists because finding bugs and making a change reviewable are different problems. A numerically heavy PR carries context that exists only in the author's head -- which formulation was chosen and why, what it assumes, what must hold if it is right -- and review stalls when that was never written down. The brief maps the mathematics onto the diff, lists the invariants, and lists the claims that no test pins, so a specific test can be asked for without re-deriving anything. It is barred from asserting that the mathematics is correct, and it interviews the author rather than inventing rationale. None of these post to GitHub. Publishing a brief is a separate, explicit step subject to the AI contribution policy. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Split the sentences that stack clauses, and name the subject before the verb. The skills tell a reader to write this way, so they have to read this way. Rewrite the assumption lists as sentences, rather than as a parenthesis inside a longer sentence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
pbrubeck
force-pushed
the
pbrubeck/ai-review-skills
branch
from
August 9, 2026 16:37
9d0c081 to
a19a1dd
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds three Claude Code skills under
.claude/skills/, following the equivalent setup in PETSc (.claude/skills/{review-mr,review-branch,review-mr-post}).review-prreview-branchorigin/mainororigin/releasereview-briefAGENTS.mdis the rubric — its Anti-Patterns section is already written to be applied at review time. The review procedure adds:returnor a rank guard), rank-local decisions feeding code generation or cache keys, and star-forest/halo assumptions.main, so a parent's changes stay out of scope. A PR based on a feature branch without abase:main/base:releaselabel is reported, since CI cannot resolve its base build.@pytest.mark.paralleltest, is reported rather than assumed covered.review-briefaddresses a different problem from bug-finding. A numerically heavy PR carries context that exists only in the author's head — which formulation was chosen and why, what it assumes, what must hold if it is right — and review stalls when that was never written down. The brief writes it down: it maps each step of the method ontofile:line, lists the invariants that must hold, tabulates which tests pin them, and lists the claims that no test pins. It is barred from asserting that the mathematics is correct, and it interviews the author rather than inventing rationale.None of the skills post to GitHub. Publishing a brief is a separate, explicit step, subject to the AI contribution policy.
No changes to Firedrake source, tests, or CI. No CI integration is proposed here — PETSc drives the equivalent from a manual
.gitlab-ci.ymljob over a model matrix, which would be a separate discussion.AI was used to draft these files (Claude Code, Opus 5).
🤖 Generated with Claude Code