---
name: code-review-excellence
description: Systematic code review for correctness, security, performance and maintainability. Produces structured review reports.
source: community (pinned 2026-03-19)
---

# Code Review Excellence

## Overview
Transform code reviews from rubber-stamping into knowledge sharing through constructive feedback, systematic analysis, and collaborative improvement.

## When to Use
- Reviewing pull requests
- Establishing code review standards
- Mentoring developers through review
- Auditing for correctness, security, or performance
- Pre-delivery validation against architectural patterns

## Instructions

### 1. Read Context First
Before touching code:
- Read the PR description, linked issues, and requirements
- Understand the architectural pattern being followed
- Check if there's a reference implementation to compare against
- Read test signals (CI status, coverage reports)

### 2. Review Systematically
Check in this order:

**Correctness:**
- Does the code do what the requirements ask?
- Are edge cases handled?
- Are error paths covered?
- Does it match the reference implementation pattern?

**Security:**
- Input validation present? (`@Valid`, `@NotNull`, `@Pattern`)
- Sensitive data masked in logs? (`authorization`, `customerId`)
- No secrets hardcoded?
- Authentication/authorization checks present?

**Performance:**
- No blocking I/O in hot paths (check `@PostConstruct` usage)
- No N+1 queries or repeated file reads
- Appropriate timeouts on external calls
- Async operations use proper context propagation (MDC)

**Maintainability:**
- Clear naming conventions
- Single responsibility per class
- Dependencies injected, not constructed
- Proper layer separation (no business logic in adapters)

### 3. Provide Actionable Feedback
For each issue:
- State the severity: BLOCKING | IMPORTANT | MINOR
- Explain WHY it's a problem, not just WHAT is wrong
- Suggest a concrete fix or alternative
- Ask clarifying questions when intent is unclear

### 4. Acknowledge Good Patterns
- Call out well-implemented patterns explicitly
- Recognize clean abstractions and good test coverage
- Note when the code follows architectural guidelines correctly

## Output Format

```markdown
## Code Review Summary

### Overall Assessment
[1-2 sentence summary of the change quality]

### Blocking Issues (must fix before merge)
- **[FILE:LINE]** — [Description] → [Suggested fix]

### Important Issues (should fix)
- **[FILE:LINE]** — [Description] → [Suggested fix]

### Minor Issues (nice to have)
- **[FILE:LINE]** — [Description] → [Suggested fix]

### Questions
- [Clarifying questions about intent or requirements]

### Positive Patterns Noted
- [Good patterns worth highlighting]

### Test Coverage
- [Coverage assessment and gaps]
```

## Review Checklist for V4 BIAN APIs

When reviewing an API implementation against `guia-4-implemented-pattern.md`:

### REC Layer
- [ ] Resource only has `@Valid` and delegates to UseCase — no logic
- [ ] Request DTOs use `@NotNull(message = "VDE01")` and `@Valid` on nested objects
- [ ] BusXxxClient is an **interface** with `@RegisterRestClient(configKey = "...")`
- [ ] BusXxxAdapter injects with `@RestClient` and reads `agencyCode` from MDC
- [ ] `ValidationExceptionMapper` present in `common/infrastructure/interceptor/`
- [ ] `RestClientExceptionMapper` present and registered via `@RegisterProvider`
- [ ] `InboundLoggingFilter` uses `SequenceInputStream` to reconstruct the stream

### BUS Layer
- [ ] Service orchestrates Ports — does not implement HTTP directly
- [ ] Service availability checked **before** business logic
- [ ] Security trace executes in `finally` using `AsyncMdcRunner.run()`
- [ ] DomXxxClient has `@RegisterProvider(RestClientLoggingFilter.class)` and `@RegisterProvider(RestClientExceptionMapper.class)`
- [ ] Mapper uses `@Mapper(componentModel = "cdi")` with `@AfterMapping` for complex fields
- [ ] `MessageHelper` uses `@PostConstruct` — no file reading in constructor
- [ ] Error responses always include `status = "error"` and `traceId`

## Common Review Anti-Patterns
| Anti-Pattern | Better Approach |
|---|---|
| "Looks good to me" (LGTM without review) | Run through the checklist systematically |
| Nitpicking style while ignoring logic bugs | Prioritize correctness over formatting |
| Rewriting the PR in comments | Suggest one concrete alternative, not a full rewrite |
| Blocking on subjective preferences | Only block on correctness, security, performance |