Code Review: Uncommitted Changes

Perform a comprehensive code review of all uncommitted git changes following EasyPlatform standards.

Summary

Goal: Review all uncommitted changes via a report-driven four-phase process before commit.

Phase	Action	Key Notes
0	Collect changes & create report	`git status` , `git diff HEAD` , create `plans/reports/code-review-*.md`
1	File-by-file review	Read each diff, update report with summary/purpose/issues
2	Holistic review	Re-read accumulated report for architecture coherence
3	Finalize findings	Critical issues, recommendations, suggestions, commit message

Key Principles:

Be skeptical. Critical thinking. Everything needs traced proof. — Never accept code at face value; verify claims against actual behavior, trace data flow end-to-end, and demand evidence (file:line references, grep results, runtime confirmation) for every finding
Ensure code quality: no flaws, no bugs — Verify correctness of logic, data flow, edge cases, and error handling. Flag anything that could fail at runtime
Clean code and DRY — No duplication, clear naming, single responsibility, early returns. Code should be self-documenting
Follow existing conventions — Match project patterns, naming style, file organization, and architectural decisions already established in the codebase. Grep for similar implementations before flagging deviations
Docs must match code — If changes affect behavior, APIs, or features, verify related docs are updated: feature docs (
```
docs/business-features/
```
), test specs (
```
docs/test-specs/
```
), CHANGELOG, README, architecture docs, and inline code comments. Flag any doc that describes old behavior
Build report incrementally — update after EACH file, not at the end
Check logic placement in lowest layer (Entity > Service > Component)
Always suggest conventional commit message based on changes

Review Approach (Report-Driven Four-Phase - CRITICAL)

⛔ MANDATORY FIRST: Create Todo Tasks for Review Phases Before starting, call TodoWrite with:

[Review Phase 0] Get git changes and create report file

- in_progress

[Review Phase 1] Review file-by-file and update report

- pending

[Review Phase 2] Re-read report for holistic assessment

- pending

[Review Phase 3] Generate final review findings

- pending

Update todo status as each phase completes. This ensures review is tracked.

Phase 0: Get Changes & Create Report

0.1 Get Change Summary

bash

# See all changed files
git status

# See actual changes (staged and unstaged)
git diff HEAD

0.2 Create Report File

Create

plans/reports/code-review-{date}-{slug}.md

Initialize with Scope (list of changed files), Change Type (feature/bugfix/refactor)

Phase 1: File-by-File Review (Build Report Incrementally)

For EACH changed file, read the diff and immediately update report with:

File path and change type (added/modified/deleted)
Change Summary: what was modified/added/deleted
Purpose: why this change exists (infer from context)
Issues Found: naming, typing, responsibility, patterns
Continue to next file, repeat

Review Checklist Per File

Architecture Compliance

Follows Clean Architecture layers (Domain, Application, Persistence, Service)
Uses correct repository pattern (I{Service}RootRepository<T>)
CQRS pattern: Command/Query + Handler + Result in ONE file
No cross-service direct database access

Code Quality

Single Responsibility Principle
No code duplication (DRY)
Appropriate error handling with PlatformValidationResult
No magic numbers/strings (extract to named constants)
Type annotations on all functions
No implicit any types
Early returns/guard clauses used

Naming Conventions

Names reveal intent (WHAT not HOW)
Specific names, not generic (
```
employeeRecords
```
not
```
data
```
)
Methods: Verb + Noun (
```
getEmployee
```
,
```
validateInput
```
)
Booleans: is/has/can/should prefix (
```
isActive
```
,
```
hasPermission
```
)
No cryptic abbreviations (
```
employeeCount
```
not
```
empCnt
```
)

Platform Patterns

Uses platform validation fluent API (.And(), .AndAsync())
No direct side effects in command handlers (use entity events)
DTO mapping in DTO classes, not handlers
Static expressions for entity queries

Security

No hardcoded credentials or secrets
Proper authorization checks
Input validation at boundaries
No SQL injection risks

Performance

No O(n²) complexity (use dictionary for lookups)
No N+1 query patterns (batch load related entities)
Project only needed properties (don't load all then select one)
Pagination for all list queries (never get all without paging)
Parallel queries for independent operations
Appropriate use of async/await
Entity query expressions have database indexes configured
MongoDB collections have
```
Ensure*IndexesAsync()
```
methods
EF Core migrations include indexes for WHERE clause columns

Backend-Specific Checks

CQRS patterns followed correctly
Repository usage (no direct DbContext access)
Entity DTO mapping patterns
Validation using PlatformValidationResult

Frontend-Specific Checks

Component base class inheritance correct (AppBase*)
State management via PlatformVmStore
Memory leaks (missing .pipe(this.untilDestroyed()))
Template binding issues
BEM classes on ALL template elements

Common Anti-Patterns to Flag

Unused imports or variables
Console.log/Debug.WriteLine left in code
Hardcoded values that should be configuration
Missing async/await keywords
Incorrect exception handling
Missing validation

Test-Specific Checks (When Diff Includes Test Files)

Apply these checks ONLY to files matching

*.Tests.*/*.cs

e2e/tests/**/*.spec.ts

Assertion Quality

Every mutation test (create/update/delete) asserts at least one domain field, not just HTTP status
Preferred: follow-up query verification after mutations (proves DB round-trip)
Validation error tests parse response body and inspect error content (not just
```
IsSuccessStatusCode.Should().BeFalse()
```
)
Setup steps (arrange phase HTTP calls) have status assertion with descriptive
```
because
```
string
Domain boolean flags verified where applicable (
```
wasCreated
```
,
```
wasSoftDeleted
```
,
```
wasRestored
```
)
E2E update tests include post-mutation field re-read (not just
```
waitForLoading()
```
)

Data Verification

After create: asserts
```
id
```
not null + at least 1 domain field matches input
After update: asserts same
```
id
```
retained + at least 1 changed field
After delete: asserts domain flag or absence from list
Search tests: verify matched item contains search term (not just count > 0)

Test Structure

TC-ID annotation present (
```
[Trait("TestCase", "...")]
```
or TC-ID in test title)
No hardcoded test data (uses
```
TestDataHelper
```
/
```
createTestSnippet()
```
)
Cleanup tracked in
```
currentTestData
```
(E2E) or UUID-isolated (integration)
```
because
```
strings on FluentAssertions calls (C#) are descriptive and unique

Anti-Patterns to Flag in Tests

Status-only assertions:
```
response.StatusCode.Should().Be(OK)
```
with no body check or follow-up query
Missing intermediate assertions: setup calls without status verification
E2E tests ending at
```
waitForLoading()
```
with no field verification
Validation tests checking only
```
IsSuccessStatusCode.Should().BeFalse()
```
Identical
```
because
```
strings across multiple assertions

Phase 2: Holistic Review (Review the Accumulated Report)

After ALL files reviewed, re-read the report to see big picture:

Overall technical approach makes sense?
Solution architecture coherent as unified plan?
New files in correct layers (Domain/Application/Presentation)?
Logic in LOWEST appropriate layer?
Backend: mapping in Command/DTO (not Handler)?
Frontend: constants/columns in Model (not Component)?
No duplicated logic across changes?
Service boundaries respected?
No circular dependencies?
Test assertion density adequate? (each test verifies meaningful domain state, not just HTTP status)
No "placeholder" tests that only check HTTP status and defer real assertions?
Assertion patterns consistent across test files in the same feature area?
Related documentation updated? (feature docs, test specs, CHANGELOG, README, architecture docs)
No docs describe old/removed behavior that conflicts with current changes?
New APIs, entities, or features have corresponding doc entries?

Phase 3: Generate Final Review Result

Update report with final sections:

Output Format

Summary: Brief overall assessment of the changes

Critical Issues: (Must fix before commit)

Issue 1: Description and suggested fix
Issue 2: Description and suggested fix

High Priority: (Should fix)

Issue 1: Description
Issue 2: Description

Suggestions: (Nice to have)

Suggestion 1
Suggestion 2

Positive Notes:

What was done well

Architecture Recommendations: (If applicable)

Recommendation 1

Suggested Commit Message: Based on changes (conventional commit format)

<type>(<scope>): <description>

<body - what and why>

IMPORTANT Task Planning Notes

Always plan and break many small todo tasks
Always add a final review todo task to review the works done at the end to find any fix or enhancement needed

review-changes

NPX Install

Tags

SKILL.md Content

Code Review: Uncommitted Changes

Summary

Review Approach (Report-Driven Four-Phase - CRITICAL)

Phase 0: Get Changes & Create Report

0.1 Get Change Summary

0.2 Create Report File

Phase 1: File-by-File Review (Build Report Incrementally)

Review Checklist Per File

Architecture Compliance

Code Quality

Naming Conventions

Platform Patterns

Security

Performance

Backend-Specific Checks

Frontend-Specific Checks

Common Anti-Patterns to Flag

Test-Specific Checks (When Diff Includes Test Files)

Assertion Quality

Data Verification

Test Structure

Anti-Patterns to Flag in Tests

Phase 2: Holistic Review (Review the Accumulated Report)

Phase 3: Generate Final Review Result

Output Format

IMPORTANT Task Planning Notes