Review branch changes for logic correctness. This skill finds what breaks, not what looks bad — use cmd-sculpt-code for code quality.
Determine Scope
Default (no scope specified): diff the current branch against the repo's base branch.
Detect the base branch in order — stop at the first success:
gh repo view --json defaultBranchRef -q '.defaultBranchRef.name' 2>/dev/nullgit remote show origin 2>/dev/null | grep "HEAD branch" | cut -d: -f2 | xargsgit symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@'
Do not assume main or master. If all methods fail, ask the user.
Once resolved, run:
git diff <base>...HEAD -- ":(exclude)*.lock" ":(exclude)package-lock.json" ":(exclude)pnpm-lock.yaml" ":(exclude)package.json"
If the user specifies a scope, use the corresponding command instead:
| Scope | Command | What it covers |
|---|---|---|
unstaged |
git diff HEAD -- <excludes> |
All uncommitted changes (staged + unstaged) |
last commit / last 1 commit |
git diff HEAD~1...HEAD -- <excludes> |
Changes in the most recent commit |
last N commits |
git diff HEAD~N...HEAD -- <excludes> |
Changes in the last N commits |
entire repo |
git ls-files | grep -vE "\.(lock|snap)$|package-lock\.json|pnpm-lock\.yaml" |
All tracked source files; no diff — apply all dimensions to the full current state of the codebase |
For all diff commands, apply: -- ":(exclude)*.lock" ":(exclude)package-lock.json" ":(exclude)pnpm-lock.yaml" ":(exclude)package.json"
Instructions
- Determine scope using Determine Scope above — ask if still unclear after applying those rules.
- Read all changed files in full before reviewing — understand the context around the diff, not just the changed lines.
- Review each dimension below. For each finding, cite
file_path:line_number.
Dimensions
1. Test Gaps
- New code paths without corresponding tests
- Missing edge case coverage: empty inputs, zero values, nil/null, boundary values
- Error paths that aren't tested (what happens when the DB call fails? the API returns 500?)
- Testable helpers or utilities that lack unit tests
- Existing tests that may be invalidated by the changes
2. Logic Simplification
- Conditional branches that can be collapsed or are mutually exclusive
- Defensive code that can't trigger (checking for nil after a constructor that never returns nil)
- Redundant validation (checking the same condition at multiple layers)
- Complex boolean expressions that could be a single predicate
- Switch/match statements missing cases or with unreachable default branches
3. Edge Cases & Failure Modes
- What happens with empty collections, zero-length strings, negative numbers?
- Concurrent access: race conditions, double-submission, stale reads
- Partial failures: what if step 2 of 3 fails? Is state left consistent?
- Resource exhaustion: unbounded loops, unlimited retries, growing memory
- Time-dependent behavior: timezone issues, DST, clock skew, expiry edge cases
- Integer overflow, floating-point precision, string encoding issues
- Off-by-one errors in loops, pagination, slicing
4. Integration Risks
- Breaking changes to public APIs, function signatures, or data formats
- Database migration safety: can it be rolled back? Does it lock tables?
- Dependency version changes: breaking updates, deprecation warnings
- Configuration changes that could affect other environments
- Feature flags or environment checks that may behave differently in prod
- Backward compatibility: will existing clients/callers still work?
Output Format
For each dimension, output:
### [Dimension Name]
- **[file:line]** — Finding description
Risk: [what could go wrong]
Suggestion: [how to address it]
If a dimension has no findings, output: No issues found.
Final Summary
End with:
- Top 3-5 risks ranked by severity (what is most likely to cause a production incident?)
- Recommended test cases to add before merging