Files
Claude-Code-Workflow/.claude/skills/code-reviewer/specs/best-practices-requirements.md
catlog22 ef770ff29b Add comprehensive code review specifications and templates
- Introduced best practices requirements specification covering code quality, performance, maintainability, error handling, and documentation standards.
- Established quality standards with overall quality metrics and mandatory checks for security, code quality, performance, and maintainability.
- Created security requirements specification aligned with OWASP Top 10 and CWE Top 25, detailing checks and patterns for common vulnerabilities.
- Developed templates for documenting best practice findings, security findings, and generating reports, including structured markdown and JSON formats.
- Updated dependencies in the project, ensuring compatibility and stability.
- Added test files and README documentation for vector indexing tests.
2026-01-06 23:11:15 +08:00

7.9 KiB
Raw Blame History

Best Practices Requirements Specification

Code Quality Standards

Naming Conventions

TypeScript/JavaScript:

  • Classes/Interfaces: PascalCase (UserService, IUserRepository)
  • Functions/Methods: camelCase (getUserById, validateEmail)
  • Constants: UPPER_SNAKE_CASE (MAX_RETRY_COUNT, API_BASE_URL)
  • Private properties: prefix with _ or # (_cache, #secretKey)

Python:

  • Classes: PascalCase (UserService, DatabaseConnection)
  • Functions: snake_case (get_user_by_id, validate_email)
  • Constants: UPPER_SNAKE_CASE (MAX_RETRY_COUNT)
  • Private: prefix with _ (_internal_cache)

Java:

  • Classes/Interfaces: PascalCase (UserService, IUserRepository)
  • Methods: camelCase (getUserById, validateEmail)
  • Constants: UPPER_SNAKE_CASE (MAX_RETRY_COUNT)
  • Packages: lowercase (com.example.service)

Function Complexity

Cyclomatic Complexity Thresholds:

  • Low: 1-5 (simple functions, easy to test)
  • Medium: 6-10 (acceptable, well-structured)
  • High: 11-20 (needs refactoring)
  • Very High: 21+ (critical, must refactor)

Calculation:

Complexity = 1 (base) 
  + count(if)
  + count(else if)
  + count(while)
  + count(for)
  + count(case)
  + count(catch)
  + count(&&)
  + count(||)
  + count(? :)

Code Duplication

Thresholds:

  • Acceptable: < 3% duplication
  • Warning: 3-5% duplication
  • Critical: > 5% duplication

Detection:

  • Minimum block size: 5 lines
  • Similarity threshold: 85%
  • Ignore: Comments, imports, trivial getters/setters

Dead Code Detection

Targets:

  • Unused imports
  • Unused variables/functions (not exported)
  • Unreachable code (after return/throw)
  • Commented-out code blocks (> 5 lines)

Performance Standards

N+1 Query Prevention

Anti-patterns:

// ❌ N+1 Query
for (const order of orders) {
  const user = await User.findById(order.userId);
}

// ✅ Batch Query
const userIds = orders.map(o => o.userId);
const users = await User.findByIds(userIds);

Algorithm Efficiency

Common Issues:

  • Nested loops (O(n²)) when O(n) possible
  • Array.indexOf in loop → use Set.has()
  • Array.filter().length → use Array.some()
  • Multiple array iterations → combine into one pass

Acceptable Complexity:

  • O(1): Ideal for lookups
  • O(log n): Good for search
  • O(n): Acceptable for linear scan
  • O(n log n): Acceptable for sorting
  • O(n²): Avoid if possible, document if necessary

Memory Leak Prevention

Common Issues:

  • Event listeners without cleanup
  • setInterval without clearInterval
  • Global variable accumulation
  • Circular references
  • Large array/object allocations

Patterns:

// ❌ Memory Leak
element.addEventListener('click', handler);
// No cleanup

// ✅ Proper Cleanup
useEffect(() => {
  element.addEventListener('click', handler);
  return () => element.removeEventListener('click', handler);
}, []);

Resource Cleanup

Required Cleanup:

  • Database connections
  • File handles
  • Network sockets
  • Timers (setTimeout, setInterval)
  • Event listeners

Maintainability Standards

Documentation Requirements

Required for:

  • All exported functions/classes
  • Public APIs
  • Complex algorithms
  • Non-obvious business logic

JSDoc Format:

/**
 * Validates user credentials and generates JWT token
 * 
 * @param {string} username - User's username or email
 * @param {string} password - Plain text password
 * @returns {Promise<{token: string, expiresAt: Date}>} JWT token and expiration
 * @throws {AuthenticationError} If credentials are invalid
 * 
 * @example
 * const {token} = await authenticateUser('john@example.com', 'secret123');
 */
async function authenticateUser(username, password) {
  // ...
}

Coverage Targets:

  • Critical modules: 100%
  • High priority: 90%
  • Medium priority: 70%
  • Low priority: 50%

Test Coverage Requirements

Coverage Targets:

  • Unit tests: 80% line coverage
  • Integration tests: Key workflows covered
  • E2E tests: Critical user paths covered

Required Tests:

  • All exported functions
  • All public methods
  • Error handling paths
  • Edge cases

Test File Convention:

src/auth/login.ts
  → src/auth/login.test.ts (unit)
  → src/auth/login.integration.test.ts (integration)

Dependency Management

Best Practices:

  • Pin major versions ("^1.2.3" not "*")
  • Avoid 0.x versions in production
  • Regular security audits (npm audit, snyk)
  • Keep dependencies up-to-date
  • Minimize dependency count

Version Pinning:

{
  "dependencies": {
    "express": "^4.18.0",    // ✅ Pinned major version
    "lodash": "*",            // ❌ Wildcard
    "legacy-lib": "^0.5.0"   // ⚠️ Unstable 0.x
  }
}

Magic Numbers

Definition: Numeric literals without clear meaning

Anti-patterns:

// ❌ Magic numbers
if (user.age > 18) { }
setTimeout(() => {}, 5000);
buffer = new Array(1048576);

// ✅ Named constants
const LEGAL_AGE = 18;
const RETRY_DELAY_MS = 5000;
const BUFFER_SIZE_1MB = 1024 * 1024;

if (user.age > LEGAL_AGE) { }
setTimeout(() => {}, RETRY_DELAY_MS);
buffer = new Array(BUFFER_SIZE_1MB);

Exceptions (acceptable magic numbers):

  • 0, 1, -1 (common values)
  • 100, 1000 (obvious scaling factors in context)
  • HTTP status codes (200, 404, 500)

Error Handling Standards

Required Error Handling

Categories:

  • Network errors (timeout, connection failure)
  • Database errors (query failure, constraint violation)
  • Validation errors (invalid input)
  • Authentication/Authorization errors

Anti-patterns:

// ❌ Silent failure
try {
  await saveUser(user);
} catch (err) {
  // Empty catch
}

// ❌ Generic catch
try {
  await processPayment(order);
} catch (err) {
  console.log('Error');  // No details
}

// ✅ Proper handling
try {
  await processPayment(order);
} catch (err) {
  logger.error('Payment processing failed', { orderId: order.id, error: err });
  throw new PaymentError('Failed to process payment', { cause: err });
}

Logging Standards

Required Logs:

  • Authentication attempts (success/failure)
  • Authorization failures
  • Data modifications (create/update/delete)
  • External API calls
  • Errors and exceptions

Log Levels:

  • ERROR: System errors, exceptions
  • WARN: Recoverable issues, deprecations
  • INFO: Business events, state changes
  • DEBUG: Detailed troubleshooting info

Sensitive Data:

  • Never log: passwords, tokens, credit cards, SSNs
  • Hash/mask: emails, IPs, usernames (in production)

Code Structure Standards

File Organization

Max File Size: 300 lines (excluding tests) Max Function Size: 50 lines

Module Structure:

module/
  ├── index.ts          # Public exports
  ├── types.ts          # Type definitions
  ├── constants.ts      # Constants
  ├── utils.ts          # Utilities
  ├── service.ts        # Business logic
  └── service.test.ts   # Tests

Import Organization

Order:

  1. External dependencies
  2. Internal modules (absolute imports)
  3. Relative imports
  4. Type imports (TypeScript)
// ✅ Organized imports
import express from 'express';
import { Logger } from 'winston';

import { UserService } from '@/services/user';
import { config } from '@/config';

import { validateEmail } from './utils';
import { UserRepository } from './repository';

import type { User, UserCreateInput } from './types';

Scoring System

Overall Score Calculation

Overall Score = (
  Security Score × 0.4 +
  Code Quality Score × 0.25 +
  Performance Score × 0.2 +
  Maintainability Score × 0.15
)

Security = 100 - (Critical × 30 + High × 2 + Medium × 0.5)
Code Quality = 100 - (violations / total_checks × 100)
Performance = 100 - (issues / potential_issues × 100)
Maintainability = (doc_coverage × 0.4 + test_coverage × 0.4 + dependency_health × 0.2)

Risk Levels

  • LOW: Score 90-100
  • MEDIUM: Score 70-89
  • HIGH: Score 50-69
  • CRITICAL: Score < 50