ColaFlow/.claude/skills/code-reviewer.md

Code Reviewer Skill

This skill ensures all frontend and backend code follows proper coding standards, best practices, and maintains high quality.

Purpose

Automatically review code for:

• Coding Standards: Naming conventions, formatting, structure
• Best Practices: Design patterns, error handling, security
• Code Quality: Readability, maintainability, performance
• Common Issues: Anti-patterns, code smells, potential bugs

When to Use

This skill is automatically applied when:

• Backend agent generates code
• Frontend agent generates code
• Any code modifications are proposed
• Code refactoring is performed

Review Checklist

Backend Code (TypeScript/NestJS)

1. Naming Conventions

// ✅ CORRECT
export class UserService {
  async getUserById(userId: string): Promise<User> { }
}

const MAX_RETRY_ATTEMPTS = 3;

// ❌ INCORRECT
export class userservice {
  async getuser(id) { }
}

const max_retry = 3;

Rules:

• Classes: PascalCase
• Functions/variables: camelCase
• Constants: UPPER_SNAKE_CASE
• Files: kebab-case.ts
• Interfaces: IPascalCase or PascalCase

2. TypeScript Best Practices

// ✅ CORRECT: Strong typing
interface CreateUserDto {
  email: string;
  name: string;
  age?: number;
}

async function createUser(dto: CreateUserDto): Promise<User> {
  // Implementation
}

// ❌ INCORRECT: Using 'any'
async function createUser(dto: any): Promise<any> {
  // Don't use 'any'
}

Rules:

• ❌ Never use any type
• ✅ Use proper interfaces/types
• ✅ Use readonly where appropriate
• ✅ Use generics for reusable code

3. Error Handling

// ✅ CORRECT: Proper error handling
export class IssueService {
  async getIssueById(id: string): Promise<Issue> {
    try {
      const issue = await this.issueRepository.findOne({ where: { id } });

      if (!issue) {
        throw new NotFoundException(`Issue not found: ${id}`);
      }

      return issue;
    } catch (error) {
      this.logger.error(`Failed to get issue ${id}`, error);
      throw error;
    }
  }
}

// ❌ INCORRECT: Silent failures
async getIssueById(id: string) {
  const issue = await this.issueRepository.findOne({ where: { id } });
  return issue; // Returns null/undefined without error
}

Rules:

• ✅ Use custom error classes
• ✅ Log errors with context
• ✅ Throw descriptive errors
• ❌ Don't swallow errors silently
• ✅ Use try-catch for async operations

4. Dependency Injection (NestJS)

// ✅ CORRECT: Constructor injection
@Injectable()
export class IssueService {
  constructor(
    @InjectRepository(Issue)
    private readonly issueRepository: Repository<Issue>,
    private readonly auditService: AuditService,
    private readonly logger: Logger,
  ) {}
}

// ❌ INCORRECT: Direct instantiation
export class IssueService {
  private issueRepository = new IssueRepository();
  private auditService = new AuditService();
}

Rules:

• ✅ Use constructor injection
• ✅ Mark dependencies as private readonly
• ✅ Use @Injectable() decorator
• ❌ Don't create instances manually

5. Database Operations

// ✅ CORRECT: Parameterized queries, proper error handling
async findByEmail(email: string): Promise<User | null> {
  return this.userRepository.findOne({
    where: { email },
    select: ['id', 'email', 'name'] // Only select needed fields
  });
}

// ❌ INCORRECT: SQL injection risk, selecting all fields
async findByEmail(email: string) {
  return this.connection.query(`SELECT * FROM users WHERE email = '${email}'`);
}

Rules:

• ✅ Use ORM (TypeORM/Prisma)
• ✅ Parameterized queries only
• ✅ Select only needed fields
• ✅ Use transactions for multi-step operations
• ❌ Never concatenate SQL strings

6. Service Layer Structure

// ✅ CORRECT: Clean service structure
@Injectable()
export class IssueService {
  constructor(
    private readonly issueRepository: IssueRepository,
    private readonly auditService: AuditService,
  ) {}

  // Public API
  async create(dto: CreateIssueDto, userId: string): Promise<Issue> {
    const validated = this.validateDto(dto);
    const issue = await this.createIssue(validated, userId);
    await this.logAudit(issue, userId);
    return issue;
  }

  // Private helper methods
  private validateDto(dto: CreateIssueDto): CreateIssueDto {
    // Validation logic
    return dto;
  }

  private async createIssue(dto: CreateIssueDto, userId: string): Promise<Issue> {
    // Creation logic
  }

  private async logAudit(issue: Issue, userId: string): Promise<void> {
    // Audit logging
  }
}

Rules:

• ✅ Single Responsibility Principle
• ✅ Public methods for API, private for helpers
• ✅ Keep methods small and focused
• ✅ Extract complex logic to helper methods

Frontend Code (React/TypeScript)

1. Component Structure

// ✅ CORRECT: Functional component with TypeScript
import { FC, useState, useEffect } from 'react';
import styles from './IssueCard.module.css';

interface IssueCardProps {
  issueId: string;
  onUpdate?: (issue: Issue) => void;
}

export const IssueCard: FC<IssueCardProps> = ({ issueId, onUpdate }) => {
  const [issue, setIssue] = useState<Issue | null>(null);
  const [loading, setLoading] = useState(false);
  const [error, setError] = useState<string | null>(null);

  useEffect(() => {
    fetchIssue();
  }, [issueId]);

  const fetchIssue = async () => {
    // Implementation
  };

  if (loading) return <LoadingSpinner />;
  if (error) return <ErrorMessage message={error} />;
  if (!issue) return null;

  return (
    <div className={styles.card}>
      <h3>{issue.title}</h3>
    </div>
  );
};

// ❌ INCORRECT: Class component, no types
export default function issuecard(props) {
  const [data, setdata] = useState();

  useEffect(() => {
    fetch('/api/issues/' + props.id)
      .then(r => r.json())
      .then(d => setdata(d));
  });

  return <div>{data?.title}</div>;
}

Rules:

• ✅ Use functional components with hooks
• ✅ Define prop interfaces
• ✅ Use FC<Props> type
• ✅ Handle loading/error states
• ✅ Use CSS modules or styled-components
• ❌ Don't use default exports
• ❌ Don't use inline styles (except dynamic)

2. State Management

// ✅ CORRECT: Zustand store with TypeScript
import { create } from 'zustand';

interface ProjectStore {
  projects: Project[];
  loading: boolean;
  error: string | null;

  // Actions
  fetchProjects: () => Promise<void>;
  addProject: (project: Project) => void;
}

export const useProjectStore = create<ProjectStore>((set, get) => ({
  projects: [],
  loading: false,
  error: null,

  fetchProjects: async () => {
    set({ loading: true, error: null });
    try {
      const projects = await ProjectService.getAll();
      set({ projects, loading: false });
    } catch (error) {
      set({
        error: error instanceof Error ? error.message : 'Unknown error',
        loading: false
      });
    }
  },

  addProject: (project) => {
    set(state => ({ projects: [...state.projects, project] }));
  },
}));

// ❌ INCORRECT: No types, mutating state
const useProjectStore = create((set) => ({
  projects: [],
  addProject: (project) => {
    set(state => {
      state.projects.push(project); // Mutation!
      return state;
    });
  },
}));

Rules:

• ✅ Define store interface
• ✅ Immutable updates
• ✅ Handle loading/error states
• ✅ Use TypeScript
• ❌ Don't mutate state directly

3. Custom Hooks

// ✅ CORRECT: Proper custom hook
import { useState, useEffect } from 'react';

interface UseIssueResult {
  issue: Issue | null;
  loading: boolean;
  error: string | null;
  refetch: () => Promise<void>;
}

export const useIssue = (issueId: string): UseIssueResult => {
  const [issue, setIssue] = useState<Issue | null>(null);
  const [loading, setLoading] = useState(false);
  const [error, setError] = useState<string | null>(null);

  const fetchIssue = async () => {
    try {
      setLoading(true);
      setError(null);
      const data = await IssueService.getById(issueId);
      setIssue(data);
    } catch (err) {
      setError(err instanceof Error ? err.message : 'Failed to fetch');
    } finally {
      setLoading(false);
    }
  };

  useEffect(() => {
    fetchIssue();
  }, [issueId]);

  return { issue, loading, error, refetch: fetchIssue };
};

// Usage
const { issue, loading, error, refetch } = useIssue('123');

// ❌ INCORRECT: No error handling, no types
function useIssue(id) {
  const [data, setData] = useState();

  useEffect(() => {
    fetch(`/api/issues/${id}`)
      .then(r => r.json())
      .then(setData);
  }, [id]);

  return data;
}

Rules:

• ✅ Name starts with "use"
• ✅ Return object with named properties
• ✅ Include loading/error states
• ✅ Provide refetch capability
• ✅ Define return type interface

4. Event Handlers

// ✅ CORRECT: Typed event handlers
import { ChangeEvent, FormEvent } from 'react';

export const IssueForm: FC = () => {
  const [title, setTitle] = useState('');

  const handleTitleChange = (e: ChangeEvent<HTMLInputElement>) => {
    setTitle(e.target.value);
  };

  const handleSubmit = async (e: FormEvent<HTMLFormElement>) => {
    e.preventDefault();
    await createIssue({ title });
  };

  return (
    <form onSubmit={handleSubmit}>
      <input
        value={title}
        onChange={handleTitleChange}
      />
      <button type="submit">Create</button>
    </form>
  );
};

// ❌ INCORRECT: No types, inline functions
export const IssueForm = () => {
  const [title, setTitle] = useState('');

  return (
    <form onSubmit={(e) => {
      e.preventDefault();
      createIssue({ title });
    }}>
      <input
        value={title}
        onChange={(e) => setTitle(e.target.value)}
      />
    </form>
  );
};

Rules:

• ✅ Type event parameters
• ✅ Extract handlers to named functions
• ✅ Use preventDefault() for forms
• ❌ Avoid inline arrow functions in JSX (performance)

5. Performance Optimization

// ✅ CORRECT: Memoization
import { memo, useMemo, useCallback } from 'react';

export const IssueCard = memo<IssueCardProps>(({ issue, onUpdate }) => {
  const formattedDate = useMemo(
    () => new Date(issue.createdAt).toLocaleDateString(),
    [issue.createdAt]
  );

  const handleClick = useCallback(() => {
    onUpdate?.(issue);
  }, [issue, onUpdate]);

  return (
    <div onClick={handleClick}>
      <h3>{issue.title}</h3>
      <span>{formattedDate}</span>
    </div>
  );
});

// ❌ INCORRECT: Re-computing on every render
export const IssueCard = ({ issue, onUpdate }) => {
  const formattedDate = new Date(issue.createdAt).toLocaleDateString();

  return (
    <div onClick={() => onUpdate(issue)}>
      <h3>{issue.title}</h3>
      <span>{formattedDate}</span>
    </div>
  );
};

Rules:

• ✅ Use memo for expensive components
• ✅ Use useMemo for expensive computations
• ✅ Use useCallback for callback props
• ❌ Don't over-optimize (measure first)

Common Anti-Patterns to Avoid

Backend

❌ God Classes: Classes with too many responsibilities ❌ Magic Numbers: Use named constants instead ❌ Callback Hell: Use async/await ❌ N+1 Queries: Use eager loading or joins ❌ Ignoring Errors: Always handle errors ❌ Hardcoded Values: Use config/environment variables

Frontend

❌ Prop Drilling: Use Context or state management ❌ Inline Styles: Use CSS modules or styled-components ❌ Large Components: Break into smaller components ❌ Missing Keys: Always provide keys in lists ❌ Premature Optimization: Measure before optimizing ❌ Missing Error Boundaries: Wrap components with error boundaries

Security Checklist

Backend

• Validate all input
• Sanitize user data
• Use parameterized queries
• Implement authentication/authorization
• Hash passwords (bcrypt)
• Use HTTPS
• Set security headers
• Rate limiting on APIs
• Log security events

Frontend

• Sanitize user input (XSS prevention)
• Validate before sending to backend
• Secure token storage (httpOnly cookies)
• CSRF protection
• Content Security Policy
• No sensitive data in localStorage
• Validate API responses

Code Review Process

When reviewing code:

1. First Pass: Architecture & Design

   • Does it follow SOLID principles?
   • Is the structure logical?
   • Are there clear separation of concerns?

2. Second Pass: Implementation

   • Correct naming conventions?
   • Proper error handling?
   • Type safety?
   • Performance considerations?

3. Third Pass: Testing & Edge Cases

   • Are there unit tests?
   • Edge cases handled?
   • Error scenarios covered?

4. Fourth Pass: Security & Best Practices

   • Any security vulnerabilities?
   • Follows best practices?
   • Documentation adequate?

Automated Checks

Recommended tools:

• Backend: ESLint, Prettier, TypeScript compiler
• Frontend: ESLint, Prettier, TypeScript compiler
• Both: Husky (pre-commit hooks), SonarQube

Review Feedback Format

## Code Review: [File Name]

### ✅ Good Practices
- [What was done well]

### ⚠️ Issues Found

#### Critical (Must Fix)
- [ ] Issue 1: [Description]
  - Location: `file.ts:123`
  - Fix: [Suggested fix]

#### Moderate (Should Fix)
- [ ] Issue 2: [Description]

#### Minor (Consider)
- [ ] Issue 3: [Description]

### 💡 Suggestions
- [Improvement suggestions]

### Overall Rating: [Approved / Needs Changes / Rejected]

Usage

This skill is automatically invoked by the main coordinator whenever backend or frontend agents generate code. The coordinator will:

1. Receive code from agent
2. Apply code-reviewer skill
3. Report any issues found
4. Request fixes if needed
5. Approve once standards are met

---

Remember: The goal is not perfection, but maintainability, reliability, and consistency.