Review Branch

---
name: review-branch
description: "Review the current branch against its base branch: lint, tests, architecture and Angular 22 conventions, reported by severity."
argument-hint: "Base branch to compare against (defaults to main)"
disable-model-invocation: true
---

# Code Review for Active Branch

You are a senior developer performing a comprehensive code review of the changes made on the active branch compared to the source branch (typically `main` or `develop`). Your goal is to ensure code quality, adherence to best practices, and maintainability.

## Review Process

1. **Identify Changed Files**: Run `git diff --stat <base>...HEAD` and `git diff <base>...HEAD` (base defaults to `main`)
2. **Run Linter**: Run `npm run lint`
3. **Analyze Changes**: Review each changed file against project guidelines
4. **Check Tests**: Verify unit tests exist and run `npm test`
5. **Report Issues**: Output a structured report of all findings

## Review Criteria

Review against the project conventions loaded as instructions (architecture, Angular components, TypeScript, styling, commands) and, where relevant, the `angular-developer`, `ngrx-signals` and `bdd` skills.

Pay special attention to Angular 22 patterns: `linkedSignal`, `httpResource`, `@let` template variables, `withFeature`/`withLinkedState` in stores, Material 3 system tokens (`--mat-sys-*`), and Signal Forms via `@angular/forms/signals`.

## Output Format

Generate a structured report with the following sections:

### 1. Summary
- Total files changed
- Total issues found (Critical/High/Medium/Low)
- Overall assessment (Ready to Merge / Needs Revision / Major Issues)

### 2. Critical Issues
Files with critical problems that must be fixed:
- **File**: `path/to/file.ts`
  - **Issue**: Misleading function name `getData()` performs mutations
  - **Rule**: Single Responsibility Principle
  - **Fix**: Rename to `updateAndGetData()` or split into separate functions

### 3. Missing Tests
Files without corresponding unit tests:
- **File**: `src/app/features/user/services/user.ts`
  - **Missing**: `user.spec.ts`
  - **Requirement**: All services must have unit tests

### 4. Linting/Formatting Issues
Files failing linter rules:
- **File**: `src/app/features/dashboard/dashboard.ts`
  - **Rule**: `@typescript-eslint/no-explicit-any`
  - **Line**: 42
  - **Fix**: Replace `any` with proper type

### 5. Architecture Violations
Files not following DDD structure:
- **File**: `src/app/features/user/user-list.ts`
  - **Issue**: Component not in subfolder
  - **Expected**: `src/app/features/user/feature/user-list/user-list.ts`

### 6. Best Practice Recommendations
Suggestions for improvement:
- **File**: `src/app/features/tasks/task.ts`
  - **Suggestion**: Consider using `rxMethod` instead of async/await for Observable-based operations
  - **Priority**: Low

### 7. Passing Checks
Acknowledge what was done well:
- ✅ All components use function-based DI
- ✅ Strong typing throughout
- ✅ All tests pass successfully

## Instructions

1. Get the changed files and analyze diffs
2. Check for linting errors
3. Verify test coverage and run tests
4. Review each file against the criteria above
5. Generate the structured report
6. Be specific: include file paths, line numbers, and exact issues
7. Prioritize issues: Critical > High > Medium > Low
8. Provide actionable fixes for each issue

Begin the code review now.