Code Review Skill

Systematic code review patterns covering security, performance, accessibility, quality, and testing across languages and frameworks.

Security Review

Critical Checks:

• Authentication tokens validated; authorization on sensitive ops

• Session management secure (httpOnly, secure, sameSite)

• No hardcoded credentials/API keys

• Proper RBAC implementation

• JWT tokens with proper algorithms (not 'none')

• Password hashing: bcrypt/argon2 (not MD5/SHA1)

Input Validation:

• User inputs sanitized

• SQL injection prevention (parameterized queries)

• XSS prevention (escaping/sanitization)

• CSRF tokens on state-changing ops

• File upload validation (type, size, content)

• JSON/XML size limits

• URL validation for redirects

Data Protection:

• Sensitive data encrypted at rest

• TLS/HTTPS for transit

• No sensitive data in logs

• PII handling regulation-compliant

• Secure random (crypto.randomBytes)

• Secrets in env vars or secret managers

• Encrypted database connections

OWASP Top 10: No injection, broken auth, data exposure, XXE, broken access control, misconfiguration, XSS, insecure deserialization, vulnerable components, insufficient logging

Performance Review

Database & Queries:

• N+1 queries identified and fixed

• Proper indexing on WHERE/JOIN columns

• Query result sets limited (pagination)

• Connection pooling implemented

• Expensive queries cached

• Batch operations instead of loops

Frontend Performance:

• Code splitting for large bundles

• Lazy loading routes/components

• Images optimized and lazy loaded

• CSS/JS minified and compressed

• Memoization for expensive computations

• Virtual scrolling for long lists

API & Network:

• API responses paginated

• GraphQL queries optimized (no over-fetching)

• Response compression (gzip/brotli)

• CDN for static assets

• HTTP/2 or HTTP/3

• Proper caching headers

• Rate limiting implemented

Memory Management:

• Event listeners removed

• No memory leaks (closures, timers)

• Large objects disposed

• File streams closed

• WeakMap/WeakSet for caching

Accessibility Review (WCAG 2.1 AA)

Semantic HTML:

• Proper heading hierarchy (h1, h2, h3)

• Semantic elements (nav, main, article, aside)

• Form labels properly associated

• Button vs link used correctly

• Tables with proper headers

Keyboard Navigation:

• All interactive elements accessible

• Logical tab order

• Focus indicators visible

• No keyboard traps

• Skip links for navigation

Screen Reader Support:

• All images have alt text

• ARIA labels where needed

• Live regions for dynamic content

• Hidden content marked properly

Color & Contrast:

• Text contrast >= 4.5:1 (normal)

• Text contrast >= 3:1 (large)

• Info not conveyed by color alone

Forms:

• Error messages associated with fields

• Required fields clearly marked

• Validation accessible

• Autocomplete attributes set

Code Quality Standards

Readability:

• Descriptive variable/function names

• Functions < 50 lines

• Consistent naming (camelCase, PascalCase)

• No magic numbers

• Proper indentation

Maintainability:

• DRY principle followed

• SOLID principles applied

• Low coupling, high cohesion

• Proper separation of concerns

• Configuration externalized

• Feature flags for gradual rollout

Error Handling:

• All errors caught and handled

• User-friendly messages

• Errors logged with context

• Graceful degradation

• Retry logic for transient failures

• Circuit breakers for external services

Comments:

• Complex logic explained

• Why, not what (code self-explanatory)

• TODO/FIXME with issue tracking

• JSDoc/TSDoc for public APIs

• No commented-out code

Testing Requirements

Unit Tests:

• All business logic tested

• Edge cases covered

• Error conditions tested

• Mocks for external dependencies

• Test coverage >= 80%

• Deterministic tests (no flaky)

Integration Tests:

• API endpoints tested

• Database operations tested

• External integrations tested

• Auth flows tested

E2E Tests:

• Critical user flows covered

• Happy paths tested

• Error scenarios tested

Test Quality:

• Readable and maintainable

• AAA pattern (Arrange, Act, Assert)

• Meaningful descriptions

• No interdependencies

• Fast execution (< 10s unit tests)

Language-Specific Patterns

TypeScript/JavaScript

Type Safety:

• No any types (use unknown if needed)

• Strict mode enabled

• Union types over enums

• Proper generic constraints

• Type guards for narrowing

Modern Patterns:

• Destructuring and defaults

• Optional chaining (?.) and nullish coalescing (??)

• Async/await over promise chains

• Arrow functions for callbacks

React Components

Best Practices:

• Functional components with hooks

• Props properly typed

• Minimal, localized state

• Effect dependencies correct

• No inline function definitions

• Keys correct in lists

• useCallback/useMemo for optimization

Node.js

Server Setup:

• Error handling middleware

• Request validation

• Rate limiting

• Helmet for security headers

• CORS configured

• Graceful shutdown handling

Python

Best Practices:

• Type hints on functions

• PEP 8 compliance

• Context managers for resources

• List comprehensions over loops

• Dataclasses for data structures

SQL

Best Practices:

• Parameterized queries only

• Indexes on WHERE/JOIN columns

• LIMIT on large queries

• Transactions for consistency

• Specify columns (no SELECT *)

Review Process

File Change Analysis:

• High risk: auth, migrations, security config, payments, encryption

• Medium risk: business logic, API, queries, integrations

• Low risk: UI, tests, docs

Impact Assessment:

• Blast radius of change

• Backward compatibility

• Database migrations needed

• User impact

• Performance implications

• Security implications

Approval Criteria:

• No critical security issues

• Tests passing

• Code coverage maintained/improved

• Documentation updated

• No performance regressions

• Accessibility requirements met

• Code follows style guide

• Changes backward compatible (or migration plan exists)

Request Changes for:

• Critical security vulnerabilities

• Failing tests

• Missing test coverage

• Breaking changes without migration

• Performance regressions

• Accessibility violations

Common Issues Reference

Security: Hardcoded secrets, SQL injection, missing auth, weak random generation, PII in logs, missing CSRF

Performance: N+1 queries, missing indexes, unnecessary re-renders, large bundles, sync blocking ops, memory leaks

Code Smells: Functions > 50 lines, nesting > 3 levels, duplication, magic numbers, poor naming, god objects

Missing Error Handling: Unhandled rejections, missing try/catch, no input validation, silent failures, generic messages

Incomplete Tests: Missing edge cases, no error tests, flaky tests, no integration tests, no accessibility tests

Review Workflow

• Initial Scan (2 min): PR description, file changes, high-risk files

• Deep Dive (15-30 min): Review each file, apply checklists, note issues

• Testing Verification (5 min): Check coverage, test quality

• Documentation Check (3 min): Updated docs, breaking changes

• Feedback Generation (5 min): Organize by severity, code suggestions

• Decision (1 min): Approve, request changes, or comment

Tools Integration

Automated Checks:

• ESLint/Prettier: Code style

• SonarQube: Quality metrics

• Snyk/Dependabot: Security vulnerabilities

• Jest/Vitest: Test coverage

• Lighthouse: Performance/accessibility

• TypeScript: Type safety

Manual Review Focus:

• Business logic correctness

• Architecture decisions

• Security implications

• UX impact

• Maintainability

Best Practices Summary

• Security first

• Performance matters

• Accessibility is non-negotiable

• Quality over speed

• Constructive feedback

• Continuous improvement