Ffp Code Review

FFP Code Review Skill

Capabilities

Senior-level code review specialising in:

• Multi-tenant security: RLS enforcement, tenant isolation, JWT validation
• Healthcare data compliance: OWASP Top 10, sensitive data protection
• FFP architecture patterns: Domain-organised structure, Handler → Service → Repository flow
• British English enforcement: Spelling, grammar, naming conventions
• TypeScript best practices: Strict mode, type safety, explicit signatures

Review Process

1. Gather Context

Check for review context (ALWAYS DO THIS FIRST):

• Try to read .claude/review-context.md using Read tool
• If exists: Extract goals, requirements, changes made, focus areas, known limitations
• If missing: Note "No review context provided" and proceed with general review

Get git changes:

git diff main...HEAD              # All changes on current branch
git log main..HEAD --oneline      # Recent commits
git branch --show-current         # Current branch name

Load project documentation:

• CLAUDE.md - Team-wide project standards
• CLAUDE.local.md - Personal preferences and FFP-specific gotchas
• project-documentation/project-state.md - Current sprint and task context
• project-documentation/architecture.md - Architecture patterns and decisions
• project-documentation/security.md - Security requirements and RLS patterns

2. Analyse Changes

Search for critical patterns using Grep:

• SQL queries: query|execute|transaction
• Tenant checks: tenant_id|tenantId
• Auth claims: claims\[|custom:
• Error handling: throw|catch|Error
• Database access: db\.|repository|Repository
• Raw HTML elements: <h[1-5]|<p>|<p |<span>|<span |<button>|<button
• Hard-coded colours: text-(gray|red|blue|green|yellow|purple|pink|indigo)-(100|200|300|400|500|600|700|800|900)|bg-(gray|red|blue|green|yellow|purple|pink|indigo)-(100|200|300|400|500|600|700|800|900)

Read modified files using Read tool:

• Focus on changed lines from git diff
• Check surrounding context for proper patterns
• Verify imports and dependencies

Find related files using Glob:

• Test files: **/*.test.ts, **/*.spec.ts
• Type definitions: **/*.d.ts, **/types/*.ts
• Related domain files: packages/core/{domain}/**/*.ts

3. Prioritise Feedback

[CRITICAL] Security & Data Safety

• Missing RLS context in database transactions
• Unvalidated tenant_id in queries
• SQL injection vulnerabilities (string concatenation)
• Exposed secrets or credentials
• Missing input validation
• Error messages leaking sensitive data
• Improper Cognito claim access (missing custom: prefix)

[HIGH] Architecture & Type Safety

• Business logic in handlers
• Direct data access in services (should use repositories)
• Missing error handling
• any types or missing type annotations
• Incorrect domain organisation
• Missing or improper entity usage

[MEDIUM] Code Quality

• American spelling in FFP-specific code (optimize, color, behavior)
  • Exception: Framework/package integrations (TailwindCSS classes, library APIs)
• Emojis in code, comments, or user-facing strings
• Raw HTML elements instead of components (h1-h5, p, span, button)
• Hard-coded colours instead of theme variables
• Poor naming conventions
• Missing comments for complex logic
• Inconsistent formatting (not 2-space indentation)

[LOW] Style Preferences

• Minor code organisation improvements
• Variable naming suggestions
• Performance micro-optimisations

4. Format Output

Provide structured feedback:

# Code Review Summary

**Branch**: [branch-name]
**Files Changed**: X files, +Y/-Z lines
**Review Context**: [Yes/No] - [If yes, summarise key goals and focus areas]
**Review Focus**: [Security | Architecture | Quality]

## [CRITICAL] Issues (Must Fix Before Merge)

[Specific file:line references with remediation examples]

## [HIGH] Priority (Should Fix)

[Architecture violations, type safety issues]

## [MEDIUM] Suggestions (Consider)

[Code quality improvements with trade-offs]

## Positive Observations

[What was done well - reinforce good practices]

FFP-Specific Security Patterns

Correct RLS Pattern

// Set RLS context in transaction
await db.transaction(async (tx) => {
  await setRLSContext(tx, context.tenantId);
  return await tx.query.users.findMany();
});

Wrong RLS Pattern

// WRONG: Direct query without RLS context - LEAKS DATA!
await db.query.users.findMany();

Correct Cognito Claims

// Use custom: prefix
const tenantId = claims['custom:tenantId'];
const role = claims['custom:role'];

Wrong Cognito Claims

// WRONG: Missing custom: prefix - won't work!
const tenantId = claims.tenantId; // undefined

Correct Tenant Validation

// Always validate tenant_id in queries
const user = await tx.query.users.findFirst({
  where: and(
    eq(users.id, userId),
    eq(users.tenant_id, tenantId) // CRITICAL
  ),
});

Wrong Tenant Validation

// WRONG: Missing tenant check - DATA LEAK!
const user = await tx.query.users.findFirst({
  where: eq(users.id, userId),
});

Architecture Flow Validation

Correct Handler Pattern

// Handler = HTTP interface only, NO business logic
export const handler = async (event: APIGatewayProxyEvent) => {
  const context = extractUserContext(event);
  const input = parseInput(event.body);

  const result = await userService.createUser(context, input);

  return { statusCode: 201, body: JSON.stringify(result) };
};

Wrong Handler Pattern

// WRONG: Handler with business logic
export const handler = async (event: APIGatewayProxyEvent) => {
  const data = JSON.parse(event.body);

  // WRONG: Business logic in handler
  if (data.age < 18) {
    throw new Error('Must be 18+');
  }

  // WRONG: Direct data access in handler
  const user = await db.query.users.create({ ... });

  return { statusCode: 201, body: JSON.stringify(user) };
};

Component Usage Enforcement

Use themed components instead of raw HTML elements:

Text Elements

// WRONG: Raw HTML elements
<h1 className="text-3xl font-bold text-gray-900">Dashboard</h1>
<h2 className="text-xl text-gray-600">Welcome</h2>
<p className="text-sm text-gray-500">Description text</p>
<span className="text-red-600">Error message</span>

// CORRECT: Use Title and Text components
<Title as="h1" colour="foreground">Dashboard</Title>
<Title as="h2" colour="muted-foreground">Welcome</Title>
<Text as="p" styleProps={{ size: 'sm', colour: 'muted-foreground' }}>Description text</Text>
<Text styleProps={{ colour: 'destructive' }}>Error message</Text>

Buttons

// WRONG: Raw button element
<button className="bg-blue-600 text-white px-4 py-2 rounded-md">
  Click me
</button>

// CORRECT: Use Button component
<Button variant="primary">Click me</Button>

When raw HTML is acceptable:

• Form inputs (input, textarea, select) - use FormTextInput instead
• Structural elements (div, section, nav, header, footer)
• Semantic elements (ul, ol, li, table, tr, td)
• Special cases where component doesn't exist (rare)

Colour Theme Enforcement

Use theme colours instead of hard-coded values:

Hard-coded Colours to Avoid

// WRONG: Hard-coded colour classes
className="text-gray-900"
className="text-gray-600"
className="text-red-600"
className="bg-blue-50"
className="border-green-200"

// CORRECT: Use theme colours via components
<Text styleProps={{ colour: 'foreground' }} />
<Text styleProps={{ colour: 'muted-foreground' }} />
<Text styleProps={{ colour: 'destructive' }} />
className="bg-info/10"
className="border-success/20"

Available Theme Colours

Text colours (use via Text or Title components):

• foreground - Primary text (dark)
• muted-foreground - Secondary text (medium gray)
• primary - FFP primary blue
• secondary - FFP light purple
• success - Green
• destructive - Red (errors)
• warning - Yellow/amber
• info - Blue (informational)

Background/border colours (use via Tailwind classes):

• bg-background, bg-foreground
• bg-primary, bg-secondary, bg-success, bg-destructive, bg-warning, bg-info
• bg-muted, bg-accent, bg-card
• Use opacity for lighter shades: bg-primary/10, border-destructive/20

When Hard-coded Colours Are Acceptable

• Gradients or complex visual effects (e.g., bg-gradient-to-r from-success to-info)
• Temporary dev-only components
• Very specific brand colours not in theme (must be rare)

Check files in scope:

• All .tsx files in packages/web/src/pages/
• All .tsx files in packages/web/src/components/
• Focus on production components (not dev pages)

British English Enforcement

Important: British English applies to FFP-specific code only. Framework/package integrations (e.g., TailwindCSS classes, library APIs) should use the framework's expected spelling.

Common mistakes to catch in FFP code:

Check in:

• FFP variable/function names: optimizeWorkout → optimiseWorkout
• FFP comments: "// Optimize the query" → "// Optimise the query"
• FFP string literals: "Customization" → "Customisation"
• User-facing messages: "Authorization failed" → "Authorisation failed"

DO NOT flag as errors:

• TailwindCSS classes: text-center, bg-color-blue-500 (framework convention)
• Library APIs: color, initialize, center (when part of framework interface)
• Package configuration: color, behavior (when required by package)

Review Philosophy

1. Security first: Healthcare data = zero tolerance for vulnerabilities
2. Constructive feedback: Explain why, show how to fix
3. Specific examples: Provide remediation code, not just descriptions
4. Positive reinforcement: Acknowledge good practices
5. Phase 1 context: Don't over-engineer; ship fast, iterate on feedback
6. Senior perspective: Mentor, don't just criticise

Usage Examples

Invoke directly:

Use the ffp-code-review skill to review my changes

Focus on specific area:

Use ffp-code-review to check security in my authentication code

After making changes:

Review my latest commit with ffp-code-review before I push