[IMPORTANT] Use TaskCreate to break ALL work into small tasks BEFORE starting — including tasks for each file read. This prevents context loss from long files. For simple tasks, AI MUST ask user whether to skip.

Prerequisites: MUST READ .claude/skills/shared/evidence-based-reasoning-protocol.md before executing.

Critical Purpose: Ensure quality — no flaws, no bugs, no missing updates, no stale content. Verify both code AND documentation.

Quick Summary

Goal: Autonomous comprehensive code review with structured checklists for architecture, patterns, quality, security, and performance (subagent variant of code-review).

Workflow:

Understand Context — Get changed files, full diff, commit messages
Categorize Changes — Group by layer (Domain, Application, Persistence, Frontend)
Review Each Category — Backend checklist (Entity, Command/Query, Repository, EventHandler), Frontend checklist (Component, Store, Form, API Service)
Security Review — Authorization, input validation, sensitive data
Performance Review — Database (indexes, eager loading, paging), API (parallel ops, caching), Frontend (lazy loading, track-by, OnPush)

Key Rules:

Autonomous: Use for comprehensive reviews without user feedback loop
Structured Checklists: Follow category-specific checklists for complete coverage
Report Format: Critical/Major/Minor issues with file:line references
Anti-Patterns: Flag side effects in handlers, wrong repository types, DTO mapping in handlers, N+1 queries

Skill Variant: Use this skill for autonomous, comprehensive code reviews with structured checklists. For interactive code review discussions with user feedback, use code-review instead.

Code Review Workflow

Review Mindset (NON-NEGOTIABLE)

Be skeptical. Apply critical thinking, sequential thinking. Every claim needs traced proof.

Do NOT accept code correctness at face value — verify by reading actual implementations
Every finding must include file:line evidence (grep results, read confirmations)
If you cannot prove a claim with a code trace, do NOT include it in the report
Question assumptions: "Does this actually work?" → trace the call path to confirm
Challenge completeness: "Is this all?" → grep for related usages across services
Verify side effects: "What else does this change break?" → check consumers and dependents
No "looks fine" without proof — state what you verified and how

Core Principles (ENFORCE ALL)

YAGNI — Flag code solving hypothetical future problems (unused params, speculative interfaces, premature abstractions) KISS — Flag unnecessarily complex solutions. Ask: "Is there a simpler way that meets the same requirement?" DRY — Actively grep for similar/duplicate code across the codebase. If 3+ similar patterns exist, flag for extraction. Clean Code — Readable > clever. Names reveal intent. Functions do one thing. No deep nesting (≤3 levels). Methods <30 lines. Follow Convention — Before flagging ANY pattern violation, grep for 3+ existing examples in the codebase. Codebase convention wins over textbook rules. No Flaws/No Bugs — Trace logic paths. Verify edge cases (null, empty, boundary values). Check error handling covers failure modes. Proof Required — Every claim backed by file:line evidence or grep results. Speculation is forbidden. Doc Staleness — Cross-reference changed files against related docs (feature docs, test specs, READMEs). Flag any doc that is stale or missing updates to reflect current code changes.

When to Use This Skill

Reviewing pull requests
Analyzing code for refactoring
Pre-commit code quality check
Security and performance audit

Review Dimensions

1. Architecture Compliance

Follows Clean Architecture layers
Uses correct repository pattern
No cross-service boundary violations
Proper separation of concerns

2. Pattern Adherence

CQRS patterns followed (Command/Query/Handler in ONE file)
Entity patterns correct (expressions, computed properties)
Frontend component hierarchy respected
Project base classes used correctly

3. Code Quality & Clean Code

4. Security

No SQL injection vulnerabilities
Authorization checks present
Sensitive data handling
Input validation

5. Performance

N+1 query prevention (eager loading)
Proper paging for large datasets
Parallel operations where applicable
Caching considerations

Review Process

Step 1: Understand Context

# Get changed files
git diff --name-only main...HEAD

# Get full diff
git diff main...HEAD

# Check commit messages
git log main...HEAD --oneline

Step 2: Categorize Changes

## Files Changed

### Domain Layer

- `Entity.cs` - New entity

### Application Layer

- `SaveEntityCommand.cs` - New command

### Persistence Layer

- `EntityConfiguration.cs` - EF configuration

### Frontend

- `entity-list.component.ts` - List component

Step 3: Review Each Category

Backend Review Checklist

## Entity Review

- [ ] Inherits from correct base (RootEntity/RootAuditedEntity)
- [ ] Static expressions for queries
- [ ] Computed properties have empty `set { }`
- [ ] Navigation properties have `[JsonIgnore]`
- [ ] `[TrackFieldUpdatedDomainEvent]` on tracked fields

## Command/Query Review

- [ ] Command + Handler + Result in ONE file
- [ ] Uses service-specific repository
- [ ] Validation uses fluent API
- [ ] No side effects in command handler
- [ ] DTO mapping in DTO class, not handler

## Repository Usage Review

- [ ] Uses `GetQueryBuilder` for reusable queries
- [ ] Uses `WhereIf` for optional filters
- [ ] Parallel tuple queries for count + data
- [ ] Proper eager loading

## Event Handler Review

- [ ] In `UseCaseEvents/` folder
- [ ] Uses project entity event handler base class (see docs/backend-patterns-reference.md)
- [ ] `HandleWhen` is `public override async Task<bool>`
- [ ] Filters by `CrudAction` appropriately

Frontend Review Checklist

## Component Review

- [ ] Correct base class for use case
- [ ] Store provided at component level
- [ ] Loading/error states handled
- [ ] `untilDestroyed()` on subscriptions
- [ ] Track-by in `@for` loops

## Store Review

- [ ] State interface defined
- [ ] `vmConstructor` provides defaults
- [ ] Effects use `observerLoadingErrorState`
- [ ] Immutable state updates

## Form Review

- [ ] `validateForm()` before submit
- [ ] Async validators conditional
- [ ] Dependent validations configured
- [ ] Error messages for all rules

## API Service Review

- [ ] Extends project API service base class (see docs/frontend-patterns-reference.md)
- [ ] Typed responses
- [ ] Caching where appropriate

Step 4: Security Review

## Security Checklist

### Authorization

- [ ] Authorization attributes on controllers (see docs/backend-patterns-reference.md)
- [ ] Role checks in handlers
- [ ] Data filtered by company/user context

### Input Validation

- [ ] All inputs validated
- [ ] No raw SQL strings
- [ ] File upload validation

### Sensitive Data

- [ ] No secrets in code
- [ ] Passwords hashed
- [ ] PII handled correctly

Step 5: Performance Review

## Performance Checklist

### Database

- [ ] Indexes on filtered columns
- [ ] Eager loading for N+1 prevention
- [ ] Paging for large datasets

### API

- [ ] Response size reasonable
- [ ] Parallel operations used
- [ ] Caching for static data

### Frontend

- [ ] Lazy loading for routes
- [ ] Track-by for lists
- [ ] OnPush change detection

Common Issues to Flag

:x: Anti-Patterns

// Issue: Side effect in command handler
await notificationService.SendAsync(...);

// Issue: Wrong repository type
GenericRootRepository<Entity>  // Should be service-specific

// Issue: DTO mapping in handler
var entity = new Entity { Name = request.Name };  // Should use DTO.MapToEntity()

// Issue: Missing eager loading
var items = await repo.GetAllAsync(...);  // Missing relations
items.ForEach(i => Console.WriteLine(i.Related.Name));  // N+1!

// Issue: Seed data in migration executor — lost after DB reset, won't run on new environments
class SeedDefaults : DataMigrationExecutor { ... }  // Should be in ApplicationDataSeeder

// Issue: No loading state
this.api.getItems().subscribe(items => this.items = items);

// Issue: Direct mutation
this.state.items.push(newItem);

// Issue: Missing cleanup
this.data$.subscribe(...);  // Missing untilDestroyed()

Review Report Template

# Code Review Report

## Summary

- **PR/Changes**: [Description]
- **Reviewer**: AI
- **Date**: [Date]

## Overall Assessment

[APPROVED | APPROVED WITH COMMENTS | CHANGES REQUESTED]

## Strengths

1. [Positive point 1]
2. [Positive point 2]

## Issues Found

### Critical (Must Fix)

1. **[File:Line]**: [Description]
    - Problem: [Explanation]
    - Suggestion: [Fix]

### Major (Should Fix)

1. **[File:Line]**: [Description]

### Minor (Consider Fixing)

1. **[File:Line]**: [Description]

## Recommendations

1. [Recommendation 1]
2. [Recommendation 2]

Review Commands

# Find potential issues
grep -r "new Entity {" --include="*.cs"  # DTO mapping in handler
grep -r "SendAsync\|NotifyAsync" --include="*CommandHandler.cs"  # Side effects
grep -r "GenericRootRepository" --include="*.cs"  # Generic repository (should be service-specific)

# Check patterns
grep -r "observerLoadingErrorState" --include="*.ts"  # Loading tracking
grep -r "untilDestroyed" --include="*.ts"  # Subscription cleanup

Verification Checklist

All changed files reviewed
Architecture compliance verified
Project patterns followed
Security concerns addressed
Performance considered
Documentation staleness checked (changed files cross-referenced against related docs)
Review report generated

code-review
tasks-test-generation

IMPORTANT Task Planning Notes (MUST FOLLOW)

Always plan and break work into many small todo tasks
Always add a final review todo task to verify work quality and identify fixes/enhancements

tasks-code-review