Self Review Pr

Self-Review a PR You Just Opened

Review a PR before handing back to the user. The goal is to catch the things you'd be embarrassed about if a reviewer flagged them next.

Reading the diff alone is not enough. A clean diff can still miss the point of the bug.

Usage

/self-review-pr [pr-number]

If no PR number is given, default to the most recent PR you opened on the current branch.

Steps

1. Restate the goal. Read the linked issue (or PR description if no issue). Restate the problem in one sentence. If you can't restate it, re-read.

2. Diff correctness. For each modified file:

   • Verify the change does what the description claims
   • Check edge cases (empty input, null, single element, fallback paths)
   • Audit every deleted block, not just the added ones. For each deletion in the diff, name what coverage or behavior the block provided and whether anything else in the diff still covers it. A test file shrinking from N to N/2 lines in a PR titled "add X" is a red flag, not a neutral substitution. Deletion-without-replacement is the most common silent regression in self-review.
   • For any test you added: confirm it would fail without the fix. Run it against the prior commit if there's any doubt.

3. Plan-vs-execution check. Reread the plan you wrote at the start of this task (the bullet list of "what I'll do" or "things to keep"). Diff that plan against your actual diff:

   • Every item you said you'd KEEP should still appear in the new state.
   • Every item you said you'd MOVE should appear in its new home, with no facts lost in transit.
   • Every item you said you'd ADD should be in the diff.
   • Every item you said you'd REMOVE should be gone.
   • For prose edits specifically (SKILL.md, tool descriptions, docs): list the discrete facts in the old text and the discrete facts in the new text. Any fact in old-not-new must either appear elsewhere or be explicitly justified as obsolete. Drift between "what I planned" and "what I did" is the most common bug in prose refactors and is invisible to syntactic checks.

4. Breadth check — the most commonly missed step. A bug pattern almost always exists in more than one place. Before deciding the PR is complete:

   • Identify the predicate or pattern your fix targets (e.g. filter(t => t.category === 'domain'), await db.execute(insert(X)), a specific error path).
   • Grep the codebase for that pattern. For each hit not in your diff, ask: does the same bug exist here?
   • For every hit that shares the bug: either include it in this PR, or write down explicitly why it is out of scope.
   • "I'll do it in a follow-up" is fine. Silent omission is not. Name every site you considered and chose not to touch.

5. Test coverage check. For each modified source file:

   • Does a sibling test file exist (e.g. ToolsPage.tsx → ToolsPage.test.tsx)?
   • Did you add a test there for the new behavior?
   • If you modified two files and only tested one, the other is a gap. Either add a test or name the gap.
   • Do not dismiss with "uses the same logic, acceptable." Test gaps are gaps — name them, then decide.

6. Convention scan. Two passes:

   • Local style. Read 50 lines of context around each diff hunk. Does anything in your diff diverge from the surrounding style (naming, types, helper usage, error handling)? If something looks unusual, ask why.
   • Explicit project rules. Re-read the project's AGENTS.md / CLAUDE.md (and any sibling rules files the diff touches, e.g. a frontend AGENTS.md). For each explicit rule you can match to your diff (import placement, type-annotation requirements, async patterns, conventional-commit format, PII rules, naming, etc.), verify the diff respects it. Local style only catches "looks weird next to neighbors"; explicit rules catch "violates a written rule even if the neighbors don't show it" (e.g. an inline import inside a try/except, where the surrounding lines are all try:/except: rather than imports).
   • Metadata-staleness check. For each surface you extended (a module, class, registry entry, API route, CLI command), find the text that describes that surface — docstrings, summary fields, README sections, schema descriptions, in-line comments — and ask whether it still accurately describes the post-change state. Adding a third method to a class whose docstring lists two is silent drift; adding a new endpoint to a router whose section header says "two endpoints" is silent drift; adding a tool to a registry whose description= enumerates the old capabilities is silent drift. The diff is correct; the surface around it is wrong. This is a different question than steps 2-4 (which ask "is the diff correct"). This asks "does the surface around my diff still describe what is now true."
   • Sibling-pattern check. When your diff adds a new instance of a known class of code (a new admin endpoint, a new tool, a new handler, a new migration), look at the recently-merged siblings of that class and audit your new instance for shared traits. Did the last few similar instances acquire a new dependency, decorator, audit field, or guard? If so, your new instance should follow the same pattern unless you have a reason not to. Don't rely on the breadth check (step 4) for this: step 4 asks "does this bug exist elsewhere?", which finds gaps in existing code; this asks "does my new instance match the established pattern?", which catches the inverse direction. Quick recipe: git log --oneline -10 -- <directory-of-my-new-instance> to see what siblings recently changed, then diff one against the prior version to spot uplift patterns.

7. Mergeability check. Diff-clean is not merge-ready. Before any "ready to merge" verdict, run:

   gh pr view <N> --json mergeable,mergeStateStatus,statusCheckRollup
   

   • mergeable must be MERGEABLE. CONFLICTING means GitHub cannot merge this even if you click the button. Rebase on latest origin/main (or the PR's base branch), resolve conflicts, force-push, then re-run this check.
   • statusCheckRollup should not be empty when CI is configured on this repo. An empty array typically means CI never fired (e.g. conflicts blocked it, or a GITHUB_TOKEN-opened PR suppressed downstream workflows). Cause must be diagnosed before declaring done.
   • Any check with conclusion: FAILURE is a blocker. IN_PROGRESS means wait; do not call the PR ready until checks complete.

   This step exists because diff-only review hides the operational state. A PR can have a perfect diff and still be unmergeable.

8. PR hygiene. Confirm:

   • PR template followed end to end. Every [ ] checkbox must be either checked, or replaced with [ ] N/A (reason). Every (describe how) / (...) placeholder must be filled in. "Filled with Fixes #N" is not enough.
   • PR title follows conventional commit format. gh pr create --fill can produce a branch-style title (e.g. fix/audit-integration-absence-guards) even when the commit body is fine. The title is what shows on GitHub; check it separately from the commit message.
   • Fixes #N to auto-close the issue
   • Identification footer present per the global rule
   • Commit message follows conventional commit format
   • Commit has the Co-Authored-By trailer per the global rule

9. Write the verdict. Use these headings, in this order:

   • Plan drift — items you said you'd keep/move/add/remove that didn't end up that way in the diff. If none, write "none."
   • Completeness gaps — breadth misses, missing tests, missing follow-up notes. If none, write "none."
   • Bugs — incorrect logic, edge cases not handled
   • Convention concerns
   • Mergeability — current mergeable / mergeStateStatus / CI summary from step 7. Quote the values; do not paraphrase.
   • PR hygiene
   • Conclusion — one of:
     • "Clean, ready to merge" (only if all above are empty AND step 7 returned mergeable: MERGEABLE with all required checks green)
     • "Fix needed before merge: "
     • "Land as-is, follow-up needed for: "

   For any high-confidence small fix found in steps 2-6, stage it and force-push a fixup commit. Do not invent new requirements the issue did not ask for.

Honest self-review applies here

[[feedback-honest-self-review]] is the general rule. Specifically for PR self-review:

• Do not soften findings. "Test gap" is not "acceptable." "Missed a third call site" is not "out of scope."
• Lead with what's wrong. If completeness gaps or bugs exist, surface them before any "what works well" prose.
• If you ran the breadth check and found nothing, say so. Silence on an expected step reads as a skipped step.
• Pre-existing test failures are not yours to fix, but they ARE yours to surface — name them so the maintainer doesn't assume the suite is fully green.

What NOT to do

• Do not post the review to GitHub. The orchestrator or user decides whether and how to comment.
• Do not redo the work. The point is to review the PR, not to re-implement the feature.
• Do not redesign the change. If the design is wrong, name it as one bullet under Conclusion and stop.
• Do not add scope the issue did not ask for. Breadth check fixes related bugs of the same kind; scope creep adds unrelated improvements.