Code Review

CaseMark-specific code review guidelines. We build legal AI products where correctness matters—bugs can affect court proceedings, attorney work product, and client trust.

Review Philosophy

Legal-grade quality: Our code handles legal documents, transcripts, and sensitive data. Review with the rigor of someone whose work product will be used in court.

Ship working code, not perfect code: We're pre-PMF. Push back on gold-plating, but never on correctness.

Small, focused PRs: Jumbo garbage PRs are unacceptable. If the PR is too big, ask for it to be broken up.

Severity Levels

Level	Meaning	Action
🔴 Critical	Bugs, security issues, data loss risks	Must fix before merge
🟡 Warning	Logic concerns, missing edge cases, performance	Should fix or explicitly acknowledge
🟢 Suggestion	Style, readability, minor improvements	Optional, author's discretion

Review Checklist

1. Correctness (Critical)

- [ ] Logic is correct and handles edge cases
- [ ] Error handling is comprehensive
- [ ] Async operations handle failures gracefully
- [ ] Data transformations preserve integrity
- [ ] No race conditions in concurrent code

CaseMark-specific:

• Transcript processing handles malformed input
• Document IDs are validated before operations
• API responses match expected schemas
• File operations handle encoding correctly (legal docs often have special characters)

2. Security (Critical)

- [ ] No secrets in code (API keys, credentials)
- [ ] Input validation on all user data
- [ ] SQL injection prevention (parameterized queries)
- [ ] XSS prevention (proper escaping)
- [ ] Authentication checked on protected routes
- [ ] Authorization verified for data access

CaseMark-specific:

• User can only access their own cases/transcripts
• PII is handled according to data retention policies
• Audit logs capture sensitive operations
• Clerk auth properly validated in server actions

3. Performance (Warning)

- [ ] No N+1 queries
- [ ] Large datasets paginated
- [ ] Expensive operations cached appropriately
- [ ] No blocking operations in hot paths
- [ ] Bundle size impact considered

React/Next.js specific:

• No barrel imports (direct imports only)
• Dynamic imports for heavy components
• Server components used where possible
• Proper Suspense boundaries for streaming

4. Code Quality (Suggestion)

- [ ] Functions are focused and appropriately sized
- [ ] Names are clear and descriptive
- [ ] No dead code or commented-out blocks
- [ ] Consistent with existing patterns
- [ ] Types are specific (no `any` without justification)

CaseMark Standards

TypeScript

// ✅ Good: Explicit return types, specific types
export async function getCase(id: string): Promise<Case | null> {
  const result = await db.cases.findUnique({ where: { id } });
  return result;
}

// ❌ Bad: Implicit return, any types
export async function getCase(id: any) {
  return await db.cases.findUnique({ where: { id } });
}

React Components

// ✅ Good: Explicit Props type, function keyword
interface CaseCardProps {
  case: Case;
  onSelect?: (id: string) => void;
}

export function CaseCard({ case, onSelect }: CaseCardProps) {
  return (/* ... */);
}

// ❌ Bad: Inline types, arrow function component
export const CaseCard = ({ case, onSelect }: { case: any; onSelect?: Function }) => {
  return (/* ... */);
}

Error Handling

// ✅ Good: Specific error handling, user-friendly messages
export async function processTranscript(id: string): Promise<Result<Transcript, ProcessingError>> {
  const transcript = await db.transcripts.findUnique({ where: { id } });
  
  if (!transcript) {
    return { success: false, error: { code: 'NOT_FOUND', message: 'Transcript not found' } };
  }
  
  if (transcript.status !== 'pending') {
    return { success: false, error: { code: 'INVALID_STATE', message: 'Transcript already processed' } };
  }
  
  // Process...
  return { success: true, data: processedTranscript };
}

// ❌ Bad: Generic try/catch, swallowed errors
export async function processTranscript(id: string) {
  try {
    const transcript = await db.transcripts.findUnique({ where: { id } });
    // Process...
    return transcript;
  } catch (e) {
    console.log(e);
    return null;
  }
}

API Routes (Next.js 15+)

// ✅ Good: Proper param handling, typed response
interface RouteContext {
  params: Promise<{ id: string }>;
}

export async function GET(request: Request, context: RouteContext) {
  const { id } = await context.params;
  const session = await getSession();
  
  if (!session) {
    return NextResponse.json({ error: 'Unauthorized' }, { status: 401 });
  }
  
  const data = await getData(id, session.user.id);
  
  if (!data) {
    return NextResponse.json({ error: 'Not found' }, { status: 404 });
  }
  
  return NextResponse.json(data);
}

Server Actions

// ✅ Good: Input validation, proper revalidation
'use server';

import { z } from 'zod';
import { revalidatePath } from 'next/cache';

const CreateCaseSchema = z.object({
  title: z.string().min(1).max(200),
  clientId: z.string().uuid(),
});

export async function createCase(formData: FormData) {
  const session = await getSession();
  if (!session) throw new Error('Unauthorized');
  
  const parsed = CreateCaseSchema.safeParse({
    title: formData.get('title'),
    clientId: formData.get('clientId'),
  });
  
  if (!parsed.success) {
    return { error: parsed.error.flatten() };
  }
  
  const newCase = await db.cases.create({
    data: { ...parsed.data, userId: session.user.id },
  });
  
  revalidatePath('/cases');
  return { success: true, caseId: newCase.id };
}

Common Issues to Flag

Always Flag (Critical)

1. Unvalidated user input going to database or external services
2. Missing auth checks on API routes or server actions
3. Secrets or credentials hardcoded or logged
4. Unbounded queries that could return massive datasets
5. Silent failures where errors are caught but not handled

Usually Flag (Warning)

1. Missing loading/error states in UI components
2. Duplicate data fetching that should be deduplicated
3. Over-fetching data that isn't used
4. Missing TypeScript types or excessive any usage
5. Inconsistent patterns with rest of codebase

Sometimes Flag (Suggestion)

1. Naming that could be clearer
2. Code organization that could be improved
3. Comments that are outdated or obvious
4. Abstractions that are premature or unnecessary

Review Format

Structure feedback clearly:

## Summary
Brief overview of the changes and overall assessment.

## Critical Issues
🔴 **[file:line]** Issue description
- Why it matters
- Suggested fix

## Warnings
🟡 **[file:line]** Issue description
- Concern
- Recommendation

## Suggestions
🟢 **[file:line]** Suggestion
- Rationale

## What's Good
Acknowledge what's done well (keep brief).

Context to Gather

Before reviewing, understand:

1. What's the ticket? Check Linear for requirements and context
2. What's the scope? Is this a quick fix or major feature?
3. What's the risk? Production code? User-facing? Data migration?
4. What changed recently? Look at recent commits for context

When to Approve

✅ Approve when:

• No critical issues
• Warnings are acknowledged or addressed
• Code follows CaseMark standards
• Tests pass (if applicable)
• PR is appropriately scoped

⏸️ Request changes when:

• Critical issues exist
• Significant warnings unaddressed
• PR is too large to review effectively
• Missing required tests or documentation

Push Back On

• Vague PRs without clear description of what/why
• Jumbo PRs that should be split up
• Scope creep beyond the ticket
• Premature optimization that adds complexity
• Gold-plating when shipping matters more

Related Skills

Skill	Use When
simplify-code	Refactoring after review
building-frontend	Reviewing UI components
api	Reviewing API routes
vercel-react-best-practices	Performance concerns
using-linear	Checking ticket context