agentsclimarketplace
Sign in

Pr review rubric

Skill atuljha23/holocron/skills/pr-review-rubric

Staff-level PR review rubric — correctness, contract, clarity, tests, blast radius. Use when reviewing code (yours or others') before merge.From its SKILL.md

Install
npx -y skills add atuljha23/holocron --skill pr-review-rubric

Assembled from the repository path, not quoted from the project. Check it against their README if it does not work.

2 things to look at

  • 2 stars2 stars. Stars are a popularity signal and not a quality one, but at this level it is likely that nobody has read this closely except its author, and you would be relying on your own review.
  • runs commandsInstructs the agent to run 1 command, including `git grep`.

SKILL.md

3.0 KB, 674 tokens by cl100k_base, as published. Nobody here has run it

PR review rubric

What a staff-plus engineer looks at when reviewing.

1. Understand before commenting

  • Read the PR description. If there isn't one, that's the first problem.
  • Skim commits. A clean story of atomic commits tells you what the author was thinking.
  • Read the tests first — they describe the contract.
  • Now read the code.

If you can't state what this PR does in one sentence in your own words, you can't review it yet.

2. Correctness

  • Does the happy path do what the description claims?
  • What are the boundary inputs? (empty, huge, zero, negative, unicode, null, undefined, missing)
  • What about concurrency? Two callers at once. Retries. Cancellation.
  • Error paths — is every throw handled, and is the handler the right place?
  • Off-by-one, inclusive/exclusive range, integer overflow on counters.

3. Contract

  • Public API change? Add-only? Breaking?
  • Response shape change — who depends on the old shape?
  • Database schema change — is the migration safe under traffic?
  • Removed a function — who called it? git grep before approving the delete.

4. Clarity

  • Names convey intent. getUser returns a user. getUserOrDefault returns a user or a default. resolve explains nothing.
  • Control flow reads top-to-bottom with minimal branching surprise.
  • The obvious case isn't buried under the edge case.
  • Comments explain why, not what. If there's a non-obvious constraint, it's documented.

5. Tests

  • Do they assert behavior the user cares about, or implementation detail?
  • Would they catch the next regression of this code?
  • Are the failing messages useful, or "expected true got false"?
  • Are there tests for every new branch?
  • Do the tests actually run? (Yes, this gets missed.)

6. Blast radius

  • What else does this code touch or get touched by?
  • Are those callers tested, or are we relying on this PR's tests alone?
  • Is there a way to stage the rollout?

7. Security

  • Only flag real risks. "You could add validation here" is noise unless there's an exploitable surface.
  • If this touches authn/authz, data egress, or user-controlled strings → apply the security checklist.

8. Perf

  • Loops inside loops on user data. Queries inside loops. New deps that balloon the bundle.
  • Do not optimize on feel. If you flag perf, propose a measurement.

Severity

Every comment has a severity:

  • Blocker — must fix before merge
  • Important — should fix, call out if intentional
  • Nit — optional polish
  • Question — explain the choice

Pick the right severity. "Blocker" for whitespace is how people learn to ignore you.

Respect the author

  • Point at the problem. Let them choose the fix.
  • Don't rewrite the PR in the comments.
  • Ask before assuming incompetence — sometimes there's context you don't have.
  • Approve with nits is a valid verdict. Don't chain-block on trivia.

What ships with it

Read from the repository

Just SKILL.md. No reference files, no scripts.

Gives 0 of the 12 instructions most pr commit review skills give in 674 tokens

Counted across 1,055 of the 1,911 authors here whose files we hold, read 2026-09-06

  • Use conventional commit message formatin 150 of 1055, across 145 files
  • Announce skill usage at startin 78 of 1055
  • Use imperative mood for commit descriptionsin 54 of 1055, across 51 files
  • Add directory to gitignore if not ignoredin 52 of 1055, across 41 files
  • Use imperative mood for commit subjectin 52 of 1055
  • Run tests to verify clean baselinein 42 of 1055, across 32 files
  • Push branch to originin 40 of 1055, across 38 files
  • Verify worktree directory is ignored before creationin 39 of 1055, across 32 files
  • Delete branches after mergingin 38 of 1055, across 30 files
  • Create worktree with new branchin 37 of 1055, across 32 files
  • Wrap body text at 72 charactersin 36 of 1055, across 34 files
  • Auto-detect and run project setupin 35 of 1055, across 27 files

Said here and by no other author read

  • Read the tests before the code
  • Summarize the PR in one sentence
  • Check boundary inputs and concurrency
  • Verify error handling for every throw
  • Check for breaking public API changes
  • Ensure comments explain why not what

Grouped from the skills themselves: near-identical wordings counted once, and counted by distinct author, so one author publishing three of these counts once. Length counted with cl100k_base; the agent that loads this file may tokenize it differently.

Keep looking

Skills are one crate of 325,949. Ordering is by how many stacks a row turns up in, so the top of any crate is what has actually been picked rather than what has the most stars.