cosmicstack-labs/code-review
Systematic code review methodology, PR checklist, feedback techniques, and review automation patterns
npx skills add https://github.com/cosmicstack-labs/mercury-agent-skills --skill code-reviewSystematic code review methodology for consistent, effective, and humane reviews that improve both code and team culture.
Every review is a human interaction. The goal is shared understanding and team growth, not ego or gatekeeping. Be constructive, specific, and kind.
A bug caught in review costs 10x less than one caught in production. Use each review as an opportunity to add automated checks so the same issue never needs a human review again.
Deep reviews catch more issues but slow delivery. Shallow reviews miss things. Adapt depth to risk: security-critical code gets exhaustive review; trivial config changes get a quick skim.
If a reviewer can point out a formatting issue, a lint violation, or a missing test — that check should be automated. Human attention is for design, logic, and tradeoffs.
| Dimension | 1 (Poor) | 3 (Adequate) | 5 (Excellent) |
|-----------|----------|---------------|----------------|
| Correctness | Obvious bugs missed | Catches logic errors | Identifies edge cases + security issues |
| Constructiveness | "This is wrong" comments | Points to specific lines | Suggests alternatives + explains reasoning |
| Speed | Reviews take >5 days | Reviews within 48 hours | Reviews within 4 hours (same day) |
| Depth | Skims only formatting | Checks logic + tests | Reviews design, security, performance, test coverage |
| Automation | No CI checks | Linting + basic tests | Pre-commit hooks, auto-review bots, coverage gates |
| Consistency | Every review is different | Team has some standards | Defined checklist, shared expectations, documented norms |
Target: 4+ in every dimension for a mature review culture.
Use this as a template. Adapt to your stack and team norms.
**Situation**: In the `calculateDiscount()` function (line 42-58)
**Behavior**: You're using floating-point arithmetic for currency values
**Impact**: This can cause precision errors — 0.1 + 0.2 !== 0.3 in IEEE 754
**Suggestion**: Consider using `Decimal` from the standard library instead
Prefix review comments for clarity:
| Prefix | Meaning | Example |
|--------|---------|---------|
| nit: | Minor preference, non-blocking | nit: trailing whitespace on line 12 |
| suggestion: | Alternative approach | suggestion: consider extracting this validation to a shared utility |
| blocking: | Must be resolved before merge | blocking: this SQL query is vulnerable to injection |
| question: | Seeking understanding | question: why is the timeout set to 30s here? |
| praise: | Positive reinforcement | praise: great use of the strategy pattern here — very extensible |
Praise: "Great approach using the observer pattern here."
Constructive: "One concern — the unsubscribe logic isn't cleaning up listeners."
Praise: "Overall this is solid. Thanks for the thorough test coverage."
Caution: The sandwich can feel manipulative. Sometimes direct feedback is better.
// Reviewer: "I'm worried about this early return — what happens if data is null?"
// Author: "Good catch — data shouldn't be null here but adding a guard makes it safer."
// Reviewer: "Yeah, and maybe log a warning so we can monitor it in prod?"
// Final code:
function processUserData(data) {
if (!data) {
logger.warn('processUserData called with null data');
return { error: 'No data provided' };
}
// ... rest of processing
}
# .github/workflows/review-checks.yml
name: Pre-review Checks
on: [pull_request]
jobs:
lint:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- run: npm ci
- run: npm run lint
- run: npm run type-check
test:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- run: npm ci
- run: npm run test -- --coverage
- uses: actions/comment-on-pr@v1
if: failure()
with:
message: '⚠️ Tests failed. Please check the CI logs.'
size:
runs-on: ubuntu-latest
steps:
- uses: actions/labeler@v4
- run: |
SIZE=$(git diff --stat main...HEAD | tail -1 | awk '{print $4}')
if [ $SIZE -gt 400 ]; then
echo "⚠️ PR exceeds 400 lines — consider splitting"
fi
Danger runs rules in CI to automate common review comments:
// dangerfile.js
import { danger, warn, fail, message } from 'danger'
const { modified, created } = danger.git
// Enforce PR description
if (!danger.github.pr.body || danger.github.pr.body.length < 10) {
warn('Please provide a more detailed PR description.')
}
// Enforce changelog entry
const hasChangelog = modified.includes('CHANGELOG.md')
if (!hasChangelog && !danger.github.pr.labels.includes('no-changelog')) {
warn('Changes should be documented in CHANGELOG.md')
}
// Warn on large PRs
const totalLines = danger.github.pr.additions + danger.github.pr.deletions
if (totalLines > 400) {
warn(`Large PR (${totalLines} lines). Consider splitting into smaller PRs.`)
}
// Check for test files
const hasTests = created.some(f => f.includes('.test.') || f.includes('.spec.'))
if (!hasTests && modified.some(f => f.endsWith('.ts') && !f.endsWith('.test.ts'))) {
warn('No test files found — consider adding tests for new/modified code')
}
// Flag debug code
const debugStatements = ['console.log', 'debugger', 'print(']
const changes = [...danger.git.created_files, ...danger.git.modified_files]
changes.forEach(file => {
// Check file content for debug statements
})
// review-bot-rules.json
{
"rules": [
{
"name": "no-debugger",
"pattern": "debugger;?",
"message": "🚫 Remove `debugger` statement before merging",
"severity": "error"
},
{
"name": "no-console-log",
"pattern": "console\\.(log|debug|info)\\(",
"message": "⚠️ Use a proper logger instead of console.log",
"severity": "warning",
"exceptions": ["src/cli/", "scripts/"]
},
{
"name": "todo-left",
"pattern": "TODO|FIXME|HACK",
"message": "📝 TODO/FIXME found — is this intentional?",
"severity": "warning"
},
{
"name": "hardcoded-secret",
"pattern": "(?:api[_-]?key|secret|password|token)\\s*[:=]\\s*['\"][A-Za-z0-9]{16,}",
"message": "🔑 Possible hardcoded secret detected",
"severity": "error"
}
]
}
# Our Team's Code Review Standards
## Expectations
- Review requests within 4 hours during working hours
- PRs < 400 lines get reviewed within 24 hours
- PRs < 100 lines get reviewed within 4 hours
- No PR merged without at least one approval
- Critical fixes can skip review but need post-merge review
## What We Review In Depth
- Security-sensitive code (auth, payments, PII)
- Public API changes
- Database schema changes
- Concurrency/async code
## What We Skim
- Generated code
- Configuration files
- Test-only changes (if trivial)
- Dependency bumps (if lockfile-only)
Track these to improve your team's review culture:
| Metric | Target | How to Measure |
|--------|--------|----------------|
| Time to first review | <4 hours | GitHub API: first review timestamp - PR open timestamp |
| Time to merge | <24 hours for small PRs | GitHub API: merge timestamp - PR open timestamp |
| Review depth | >2 comments per 100 lines | Count comments vs line count |
| Review latency | <1 hour for blocking comments | Time between blocking comment and response |
| Approval ratio | >80% PRs approved on first review | Count PRs approved vs PRs with revisions requested |
## When You Disagree With a Review
1. **Ask clarifying questions** — "I'm not sure I understand the concern, could you elaborate?"
2. **Explain your reasoning** — "I chose this approach because..."
3. **Acknowledge valid points** — "That's a good point about edge cases, I hadn't considered that."
4. **Propose compromises** — "How about I keep the current structure but add better documentation?"
5. **Escalate if needed** — "We have two valid approaches here. Let's get a third opinion or make a team decision."
## When An Author Disagrees With Your Review
1. **Re-evaluate** — Are you being too strict? Is this truly important?
2. **Accept good counter-arguments** — "You're right, the performance concern is negligible here."
3. **Agree to disagree with documentation** — "Let's document this tradeoff and move on."
4. **Use blocking sparingly** — Only block for correctness, security, or maintainability issues.
Take cosmicstack-labs/code-review from the repository into ~/.claude/skills for personal
use, or into .claude/skills inside a project.
The agent identifies a skill by the name field in its header. Two skills with the
same name cannot sit side by side — one of them will be ignored.