Code Review Plus

Dev Tools

Enhanced code review with AI/ML-specific checks, multi-language support, security vulnerability patterns, and integration with automated testing. Covers security, performance, correctness, maintainability, testing, AI/ML, and language-specific best practices.

Install

openclaw skills install code-review-plus

Code Review Plus

Enhanced code review covering security, performance, correctness, maintainability, testing, AI/ML, and language-specific best practices.

Review Dimensions

DimensionFocusPriority
SecurityVulnerabilities, auth, data exposureCritical
PerformanceSpeed, memory, scalability bottlenecksHigh
CorrectnessLogic errors, edge cases, data integrityHigh
MaintainabilityReadability, structure, future-proofingMedium
TestingCoverage, quality, reliability of testsMedium
AI/MLModel safety, data leakage, biasHigh
LanguageIdiomatic patterns, best practicesMedium

Security Checklist

  • SQL Injection — All queries use parameterized statements or ORM
  • XSS — User content escaped/sanitized before rendering
  • CSRF Protection — State-changing requests require valid CSRF tokens
  • Authentication — Protected endpoints verify authentication
  • Authorization — Resource access scoped to requesting user
  • Input Validation — All external input validated server-side
  • Secrets Management — No credentials in source code
  • Dependency Safety — Dependencies from trusted sources, no known CVEs
  • Sensitive Data — PII/tokens never logged or in error messages
  • Rate Limiting — Public/auth endpoints have rate limits
  • File Upload Safety — Files validated for type/size, stored outside webroot
  • HTTP Security Headers — CSP, X-Content-Type-Options, HSTS set

AI/ML-Specific Checklist

  • Data Leakage — Training data not exposed in predictions
  • Prompt Injection — User input not injected into prompts without sanitization
  • Model Safety — Output filtering for harmful content
  • Bias Detection — Model outputs checked for demographic bias
  • Cost Control — API calls budgeted, loops prevented
  • Fallback Handling — Graceful degradation when AI services fail
  • Token Limits — Input/output token limits enforced
  • Caching — Expensive AI calls cached where appropriate
  • Rate Limiting — AI API calls rate-limited
  • Logging — AI interactions logged for debugging/audit

Performance Checklist

  • N+1 Queries — Database access patterns batched/joined
  • Unnecessary Re-renders — Components re-render only when needed
  • Memory Leaks — Event listeners, subscriptions cleaned up
  • Bundle Size — Dependencies tree-shakeable, large libs loaded dynamically
  • Lazy Loading — Heavy components use code splitting
  • Caching Strategy — Expensive computations use caching
  • Database Indexing — Queries filter/sort on indexed columns
  • Pagination — List endpoints use pagination
  • Async Operations — Long tasks offloaded to background jobs
  • Image Optimization — Proper sizing, modern formats, CDN

Correctness Checklist

  • Edge Cases — Empty arrays, zero values, max values handled
  • Null/Undefined — Nullable values checked before access
  • Off-by-One — Loop bounds, pagination offsets verified
  • Race Conditions — Concurrent access uses locks/transactions
  • Timezone Handling — Dates stored in UTC
  • Unicode — String ops handle multi-byte characters
  • Integer Overflow — Large number arithmetic uses BigInt/Decimal
  • Error Propagation — Async errors caught and handled
  • State Consistency — Multi-step mutations transactional
  • Boundary Validation — Boundary values tested

Maintainability Checklist

  • Naming Clarity — Descriptive names revealing intent
  • Single Responsibility — Each function/class does one thing
  • DRY — Duplicated logic extracted to utilities
  • Cyclomatic Complexity — Low branching complexity
  • Error Handling — Errors caught, logged, surfaced meaningfully
  • Dead Code — Commented code, unused imports removed
  • Magic Numbers — Literals extracted to named constants
  • Consistent Patterns — New code follows codebase conventions
  • Function Length — Short enough to understand at a glance
  • Dependency Direction — Dependencies point inward

Testing Checklist

  • Test Coverage — New logic paths have tests
  • Edge Case Tests — Boundary values, empty inputs tested
  • No Flaky Tests — Deterministic, no timing reliance
  • Test Independence — Tests set up/tear down own state
  • Meaningful Assertions — Assert on behavior, not implementation
  • Test Readability — Arrange-Act-Assert pattern
  • Mocking Discipline — Only external boundaries mocked
  • Regression Tests — Bug fixes include reproducing tests

Language-Specific Patterns

Python

  • Type hints for public functions
  • Context managers for resource cleanup
  • Virtual environments for dependency isolation
  • Black/flake8 formatting
  • Docstrings for public APIs

JavaScript/TypeScript

  • ESLint/Prettier formatting
  • TypeScript strict mode
  • Async/await over raw promises
  • Optional chaining for null safety
  • Proper error boundaries in React

Go

  • Error handling with errors.Is/As
  • Context propagation
  • Proper goroutine lifecycle
  • gofmt/goimports formatting
  • Interface design patterns

Review Process

PassFocusTimeWhat to Look For
FirstHigh-level structure2-5 minArchitecture, file organization, API design
SecondLine-by-line detailBulkLogic, security, performance, edge cases
ThirdEdge cases & hardening5 minFailure modes, concurrency, boundary values

Severity Levels

LevelLabelMeaningBlocks Merge?
Critical[CRITICAL]Security vulnerability, data loss, crashYes
Major[MAJOR]Bug, logic error, performance regressionYes
Minor[MINOR]Future maintenance cost reductionNo
Nitpick[NIT]Style preference, naming suggestionNo

Giving Feedback

Principles

  • Be specific — Point to exact line and explain issue
  • Explain why — State risk or consequence
  • Suggest a fix — Offer concrete alternative or code snippet
  • Ask, don't demand — Use questions for subjective points
  • Acknowledge good work — Call out clean solutions
  • Separate blocking from non-blocking — Use severity labels

Example Comments

Bad:

This is wrong. Fix it.

Good:

[MAJOR] This query interpolates user input directly into SQL (line 42), vulnerable to SQL injection. Consider:

SELECT * FROM users WHERE id = $1

Anti-Patterns

Anti-PatternDescription
Rubber-StampingApproving without reading
BikesheddingDebating variable names while ignoring bugs
Blocking on StyleRefusing over formatting a linter should enforce
GatekeepingRequiring personal preferred approach
Drive-by ReviewsOne vague comment and disappearing
Scope CreepRequesting unrelated refactors
Stale ReviewsLetting PRs sit for days
Emotional Language"This is terrible" or "obviously wrong"

NEVER Do

  1. NEVER approve without reading every changed line
  2. NEVER block a PR solely for style preferences
  3. NEVER leave feedback without severity level
  4. NEVER request changes without explaining why
  5. NEVER review more than 400 lines in one sitting
  6. NEVER skip the security checklist
  7. NEVER make it personal