gh-cliftonc-claude-plugins-plugins-ts-quality/commands/ts-review.md

name, description

name	description
ts-review	Perform architectural review of uncommitted TypeScript code with quality checks and pragmatic improvement suggestions

TypeScript Architectural Review

This command performs a comprehensive architectural review of uncommitted TypeScript code, combining quality checks with architectural analysis to provide concrete, pragmatic suggestions for improvement.

What This Command Does

When invoked, this command:

1. Identifies Uncommitted Changes - Uses git diff to find modified/new .ts/.tsx files
2. Runs Quality Checks - Same criteria as typescript-quality skill:
   • Type checking (pnpm typecheck)
   • Linting (pnpm lint)
   • Building (pnpm build)
3. Performs Architectural Review - Analyzes code structure, design patterns, and provides concrete improvements
4. Delivers Pragmatic Suggestions - Actionable recommendations, not theoretical perfection

Usage

/ts-review

Run this command before committing TypeScript changes to ensure both quality and good architectural design.

Instructions

When this command is invoked, follow these steps:

1. Identify Changed Files and Check Size

Step 1: Find changed TypeScript files

# Get list of modified TypeScript files (both staged and unstaged)
{ git diff --name-only; git diff --cached --name-only; } | sort -u | grep -E '\.(ts|tsx)$'

# Also get untracked TypeScript files
git ls-files --others --exclude-standard | grep -E '\.(ts|tsx)$'

If no TypeScript files found:

• Report: "No uncommitted TypeScript files found. Nothing to review."
• Exit

Step 2: Check total diff size (for planning purposes)

# Count total lines changed (gives us a sense of changeset size)
{ git diff; git diff --cached; } | wc -l

If diff is larger than 1000 lines:

• Report: "Large changeset detected (X lines across Y TypeScript files). This review may take a while."
• List all changed TypeScript files
• Use the AskUserQuestion tool to let the user choose:
  • Option 1: "Review all files now"
  • Option 2: "Let me select specific files to review"
  • Option 3: "Review files one at a time"
• Based on user's choice, proceed accordingly

If diff is 1000 lines or less:

• Proceed with full review of all changed TypeScript files

2. Run Quality Checks (ZERO TOLERANCE)

Execute quality checks sequentially, stopping at first failure:

pnpm typecheck && pnpm lint && pnpm build

If any check fails:

• Report the specific errors with file paths and line numbers
• DO NOT proceed to architectural review
• Exit with clear instructions to fix errors first

If all checks pass:

• Report: "✓ All quality checks passed"
• Proceed to architectural review

3. Read Changed Files

Use the Read tool to analyze all changed TypeScript files from step 1.

For each file in the list:

• Use the Read tool to get the current file contents
• Analyze the code structure, patterns, and potential issues
• Focus your review on understanding what changed and why

Build understanding:

• Understand what functionality is being added/modified in each file
• Identify architectural patterns in the code
• Look for potential issues in the implementation
• Consider how changes fit into the broader codebase structure

4. Perform Architectural Review

Analyze the uncommitted code for:

A. Design Patterns & Principles

SOLID Principles:

• Single Responsibility: Does each class/function have one clear purpose?
• Open/Closed: Is code open for extension, closed for modification?
• Liskov Substitution: Can subclasses replace parent classes without breaking?
• Interface Segregation: Are interfaces focused and not bloated?
• Dependency Inversion: Does code depend on abstractions, not concretions?

Common Issues to Flag:

• God classes/functions doing too much
• Tight coupling between modules
• Circular dependencies
• Missing abstractions
• Violation of separation of concerns

B. Code Structure & Organization

File Organization:

• Are related functions/classes grouped logically?
• Is the file size reasonable (< 500 lines generally)?
• Should code be split into multiple files?
• Are imports organized and minimal?

Function/Method Design:

• Are functions focused on a single task?
• Is complexity reasonable (cyclomatic complexity < 10)?
• Are parameter counts reasonable (< 5 parameters)?
• Are functions too long (> 50 lines warrants review)?

Class Design:

• Are classes cohesive (methods work with shared state)?
• Is inheritance used appropriately (favor composition)?
• Are class responsibilities clear?

C. Type Safety & TypeScript Usage

Type Quality:

• Are types explicit where clarity is needed?
• Is any being abused? (should use unknown or proper types)
• Are union types used effectively?
• Are generics used where appropriate?
• Are type guards used for runtime safety?

Type Organization:

• Should types be extracted to separate files?
• Are complex types well-documented?
• Are types reusable across the codebase?

D. Error Handling & Resilience

Error Handling:

• Are errors handled appropriately?
• Are error messages informative?
• Are errors typed (custom error classes)?
• Is error propagation clear?

Edge Cases:

• Are null/undefined cases handled?
• Are boundary conditions considered?
• Is input validation present?

E. Testability & Maintainability

Testability:

• Can the code be easily unit tested?
• Are dependencies injectable?
• Is business logic separated from infrastructure?
• Are side effects isolated?

Maintainability:

• Is code self-documenting with clear names?
• Are complex sections commented?
• Is there duplication that should be extracted?
• Can code be understood by other developers?

F. Performance & Efficiency

Common Performance Issues:

• Unnecessary re-renders (React components)
• N+1 query problems
• Missing memoization for expensive operations
• Inefficient algorithms (O(n²) when O(n) exists)
• Large object copying

Note: Only flag performance issues that are clearly problematic, not micro-optimizations.

5. Generate Pragmatic Suggestions

For each issue found, provide:

1. Location: File path and line numbers (e.g., src/auth/login.ts:45-67)
2. Issue: Clear description of the problem
3. Impact: Why this matters (coupling, testing, performance, etc.)
4. Suggestion: Concrete, actionable fix (not theoretical)
5. Example: Show before/after code when helpful

Format each suggestion as:

### Issue: [Brief Title]

**Location:** `file/path.ts:line-range`

**Problem:**
[Clear description of what's wrong]

**Impact:**
[Why this matters - coupling, maintainability, bugs, performance]

**Suggestion:**
[Concrete steps to improve]

**Example:**
[Optional: before/after code snippet]

6. Prioritize Suggestions

Organize suggestions by priority:

🔴 Critical (Fix Now):

• Severe SOLID violations
• Major coupling issues
• Security concerns
• Clear bug risks

🟡 Important (Fix Soon):

• Moderate design issues
• Testability problems
• Maintainability concerns
• Performance issues

🟢 Nice to Have (Consider):

• Minor improvements
• Consistency issues
• Optimization opportunities

7. Provide Summary

End with a summary:

## Summary

**Files Reviewed:** [count]
**Quality Checks:** ✓ All passed
**Issues Found:**
- 🔴 Critical: [count]
- 🟡 Important: [count]
- 🟢 Nice to Have: [count]

**Overall Assessment:**
[1-2 sentence summary of code quality]

**Top Priority:**
[Most important thing to address]

Best Practices for Reviews

Be Pragmatic, Not Pedantic

• Focus on issues that genuinely impact quality
• Don't flag style issues already caught by linting
• Avoid theoretical concerns without real impact
• Prioritize maintainability and clarity

Be Concrete, Not Abstract

• ❌ "This violates separation of concerns"
• ✅ "Extract database logic from this component into a repository class - see example below"

Be Helpful, Not Critical

• Frame suggestions as improvements, not criticisms
• Acknowledge good patterns when you see them
• Explain the "why" behind suggestions

Be Realistic About Refactoring

• Don't suggest massive refactors for small changes
• Consider the context and scope of the commit
• Focus on incremental improvements
• Note when bigger refactors might be valuable "in the future"

Edge Cases

No Issues Found

If the code is genuinely good:

## Review Complete

**Files Reviewed:** [count]
**Quality Checks:** ✓ All passed
**Architectural Issues:** None found

**Assessment:**
The uncommitted TypeScript code follows good architectural practices. The code is:
- Well-structured and organized
- Properly typed with good TypeScript usage
- Testable and maintainable
- Following SOLID principles appropriately

No changes recommended. Code is ready to commit.

Only Quality Check Failures

If quality checks fail before architectural review:

## Quality Checks Failed

**Files Reviewed:** [count]
**Quality Check Status:** ✗ Failed

The following quality checks must pass before architectural review:

[Error output from failed checks]

Please fix these errors and run `/ts-review` again.

Mixed Results

Some files good, some with issues - provide granular feedback per file.

Example Output

# TypeScript Architectural Review

**Date:** 2025-01-15
**Uncommitted Files:** 3 TypeScript files

## Quality Checks

✓ Type checking passed
✓ Linting passed
✓ Build passed
✓ Tests passed (12/12)

---

## Architectural Review

### 🔴 Critical: God Class with Multiple Responsibilities

**Location:** `src/services/UserService.ts:15-250`

**Problem:**
The `UserService` class handles authentication, profile management, email notifications, and database operations. This violates the Single Responsibility Principle.

**Impact:**
- Difficult to test (must mock database, email, auth)
- Changes to email logic risk breaking authentication
- Class has 250 lines and growing

**Suggestion:**
Split into focused services:
1. `AuthService` - handles login/logout/tokens
2. `UserProfileService` - manages user profiles
3. `UserNotificationService` - sends user emails
4. `UserRepository` - database operations

Keep `UserService` as a facade if needed, delegating to specialized services.

**Example:**
```typescript
// Before: One class doing everything
class UserService {
  login() { /* auth logic */ }
  updateProfile() { /* profile logic */ }
  sendEmail() { /* email logic */ }
  saveToDb() { /* db logic */ }
}

// After: Focused services
class AuthService {
  constructor(private userRepo: UserRepository) {}
  login() { /* auth logic */ }
}

class UserProfileService {
  constructor(
    private userRepo: UserRepository,
    private notificationService: UserNotificationService
  ) {}
  updateProfile() { /* profile logic */ }
}

---

🟡 Important: Missing Abstraction for Third-Party API

Location: src/api/PaymentProcessor.ts:45-89

Problem: Stripe API calls are made directly throughout the code with hardcoded endpoints and no abstraction layer.

Impact:

• Cannot easily switch payment providers
• Difficult to test (must mock Stripe SDK directly)
• Stripe-specific logic scattered across files

Suggestion: Create a PaymentGateway interface and StripePaymentGateway implementation:

interface PaymentGateway {
  processPayment(amount: number, token: string): Promise<PaymentResult>
  refund(transactionId: string): Promise<RefundResult>
}

class StripePaymentGateway implements PaymentGateway {
  // Stripe-specific implementation
}

// Easy to test with a mock
class MockPaymentGateway implements PaymentGateway {
  // Test implementation
}

---

🟢 Nice to Have: Extract Complex Type

Location: src/types/api.ts:120-145

Problem: Inline type definition for API response is complex and reused in 3 places with slight variations.

Suggestion: Extract to a named type with generic parameter for variations:

type ApiResponse<T> = {
  data: T
  status: 'success' | 'error'
  metadata: {
    timestamp: number
    requestId: string
  }
}

---

Summary

Files Reviewed: 3 Quality Checks: ✓ All passed Issues Found:

• 🔴 Critical: 1
• 🟡 Important: 1
• 🟢 Nice to Have: 1

Overall Assessment: Code quality is good, but the UserService class needs immediate refactoring to improve testability and maintainability.

Top Priority: Split UserService into focused services (AuthService, UserProfileService, etc.)

## Requirements

This command requires:

- **Git repository** - Uses git to identify uncommitted files
- **pnpm** - For running quality checks
- **TypeScript** - Configured in the project
- **Linter, build, test** - Same requirements as typescript-quality skill

## Related

- **typescript-quality skill** - Automatically enforces quality checks when modifying TypeScript files
- Use `/ts-review` for deeper architectural analysis before commits