code-review

Original：🇺🇸 English
Translated

Senior code review pass. Use PROACTIVELY as the FINAL task after code changes. Reviews against project rules, fixes issues, runs quality gates. Focuses on logic and architecture while delegating style to automation.

2installs
Sourcecarvalab/k-skills
Added on

NPX Install

npx skill4agent add carvalab/k-skills code-review

Tags

Translated version includes tags in frontmatter
code-reviewquality-gatescode-architectureself-reviewautomated-linting

SKILL.md Content

View Translation Comparison →

Code Review

Perform a thorough senior-level code review on recent changes. Focus human review on complex logic, architecture, and edge cases. Delegate style enforcement to automated tools.
Related Skills:
  • kavak-documentation
    - Query for Kavak-specific patterns, logging/metrics standards, GitLab CI templates
  • Use 
    kavak-platform/plati_query
    MCP tool to verify Kavak best practices when reviewing

Quick Start

bash
# 1. Get review range
BASE_SHA=$(git rev-parse HEAD~1)  # or: git merge-base main HEAD
HEAD_SHA=$(git rev-parse HEAD)

# 2. See what changed
git diff --stat $BASE_SHA..$HEAD_SHA
git diff --name-only $BASE_SHA..$HEAD_SHA --diff-filter=ACMR

# 3. After review, run quality gates (detect project type)
Quality gate commands by language:
LanguageLintBuildTest
Go
golangci-lint run
go build ./...
go test ./...
Node/TS
npm run lint
npm run build
npm test
Python
ruff check .
python -m py_compile
pytest
Java
./mvnw checkstyle:check
./mvnw compile
./mvnw test

When to Self-Review (Automated)

Mandatory triggers:
  • After completing each task in multi-step development
  • After implementing a major feature
  • Before merging to main branch
Valuable triggers:
  • When stuck (fresh perspective on own code)
  • Before refactoring (baseline check)
  • After fixing complex bugs

Review Mindset

  • Thorough, not rushed - Read all related code before concluding
  • Evidence-based - Trace execution paths, don't assume bugs exist
  • Fix, don't just flag - Identify issues AND resolve them
  • Small scope - Review <400 lines at a time for effectiveness

Workflow

1. Scope the Diff

Identify all changed files:
bash
git diff --name-only HEAD~1 --diff-filter=ACMR
For each file in the list, skip any that produces no actual diff hunks.

2. Understand Intent & Requirements

Before reviewing code, understand what it should do:
  • Original task/issue: What was requested?
  • Product impact: What does this deliver for users?
  • Acceptance criteria: What defines "done"?
Ask: "Does the implementation satisfy the requirements?"

3. Review Each File

For each changed file and each diff hunk, evaluate in context of the existing codebase.
MANDATORY: Trace Complete Execution Before Flagging Critical/Major
Before classifying any issue as Critical or Major, you MUST:
  1. Read the complete code path - not just the suspicious line
  2. Trace what actually happens - follow execution from start to end
  3. Verify the issue exists - confirm there is no defensive code preventing it
  4. Consider all scenarios - including error paths, cancellation, edge cases

4. Review Categories

CategoryWhat to Check
Design & ArchitectureFits system patterns, avoids coupling, clear separation
Complexity & MaintainabilityFlat control flow, low complexity, DRY, no dead code
Functionality & CorrectnessValid/invalid inputs handled, edge cases covered
Readability & NamingIntent-revealing names, comments explain WHY
Best PracticesLanguage/framework idioms, SOLID principles
Test CoverageSuccess + failure paths tested
StandardizationStyle guide conformance, zero new lint warnings
SecurityInput validation, secrets management, OWASP Top 10
PerformanceNo N+1 queries, efficient I/O, no memory leaks
LoggingState machine pattern, proper tense, actionable context
MetricsNo cardinality explosions, proper naming, documented
Automation tip: Delegate style/formatting to linters (ESLint, Prettier). Focus human review on logic, architecture, and edge cases.

4.1 Logging Review

Check✓ Good✗ Bad
Length50-200 chars>200 (truncation)
MultilineSingle lineMultiple lines
Start"Syncing orders" (progressive)"Sync orders"
End OK"Orders synced" (past)"Syncing complete"
End Fail"Failed to sync orders""Error"
Context"DB timeout after 30s""Database error"

4.2 Metrics Review

Check✓ Good✗ Bad (CRITICAL)
Labels
status=2xx
, 
method=GET
user_id
, 
order_id
Naming
api.request.total
apiRequestTotal
TypeCounter (events), Histogram (latency)Gauge for everything
CRITICAL: High-cardinality labels (user_id, request_id, timestamp) cause monitoring system crashes.

5. Check Project Rules

MANDATORY: Check and enforce rules from:
  • .claude/CLAUDE.md
    or 
    CLAUDE.md
    (root)
  • .cursor/rules/
    folder
  • AGENTS.md
  • Follow ALL patterns and conventions defined there

6. Report & Fix Issues

For each validated issue:
  • File: 
    <path>:<line-range>
  • Issue: One-line summary
  • Fix: Concise suggested change
Severity levels:
  • Critical (P0): Security vulnerabilities, data loss, crashes
  • Major (P1): Significant bugs, performance issues, architectural violations
  • Minor (P2): Code style, minor improvements
  • Enhancement (P3): Nice-to-have improvements
MANDATORY: Fix all issues immediately after identifying them.
  1. Start with highest priority (Critical → Major → Minor)
  2. For each issue:
    • Fix the code
    • Verify fix doesn't break anything
    • CHECKPOINT COMMIT: 
      git add -A && git commit -m "fix: brief description"
  3. Continue until ALL issues are resolved

7. Run Quality Gates

After all issues are fixed, run lint → build → test for project type (see Quick Start table).
If any gate fails, fix immediately and commit the fix.

8. Final Report

Provide structured output:
markdown
### Strengths

[What's well done - be specific with file:line references]

### Issues Found & Fixed

- **Critical**: [count] - [brief list]
- **Major**: [count] - [brief list]
- **Minor**: [count] - [brief list]

### Quality Gates

- Lint: ✓/✗
- Typecheck: ✓/✗
- Build: ✓/✗
- Tests: ✓/✗

### Assessment

**Ready to proceed?** [Yes / Yes with notes / No - needs fixes]
**Reasoning:** [1-2 sentence technical assessment]
If no issues: "Code review complete. No issues found. All quality gates pass. Ready to proceed."

References

ReferencePurpose
references/checklist.md
Detailed review checklist
references/severity-guide.md
How to classify issue severity
references/common-issues.md
Common issues (TypeScript/Node)
references/common-issues-go.md
Common issues (Go)
references/security-owasp.md
OWASP Top 10 security checklist
references/feedback-guide.md
How to give constructive feedback
references/self-review-workflow.md
Automated self-review during development

Best Practices

  1. No assumptions - Read ALL related code before flagging issues
  2. Fix immediately - Don't just report, fix and commit
  3. Checkpoint commits - Commit each fix separately for easy rollback
  4. Verify first - Trace execution paths before claiming bugs exist
  5. Project rules priority - Always check 
    .cursor/rules/
    and 
    AGENTS.md
  6. Be timely - Review promptly to avoid blocking teammates
  7. Limit scope - Review <400 lines at a time for effectiveness
Principle: Review → Fix → Verify → Commit. A review that only identifies problems without fixing them is incomplete.