---
name: code-review
description: Structured code review against implementation plan.
---

# Skill: code-review

Structured code review against implementation plan.

## Invoked by

🔍 Reviewer mode

## Required Inputs

| Input | Source | Example |
|-------|--------|---------|
| `TICKET_KEY` | Jira issue key | `PROJECT-123` |
| `MODULE` | Module/component name | `auth`, `api`, `core` |

## Output

Markdown file: `docs/<MODULE>/<TICKET_KEY>/<TICKET_KEY>-review.md`

## Steps

### 1. Read the plan document

```bash
cat docs/<MODULE>/<TICKET_KEY>/<TICKET_KEY>-plan.md
```

Extract: planned changes, affected files, expected patterns, acceptance criteria.

### 2. Read the test plan (if exists)

```bash
cat docs/<MODULE>/<TICKET_KEY>/<TICKET_KEY>-testplan.md
```

Cross-reference: are all planned test cases implemented?

### 3. Get the diff

```bash
cd <your-repo-path>-<TICKET_KEY>
git diff origin/main --name-only
git diff origin/main --stat
git diff origin/main
```

### 4. Read changed files

For each changed file, read the full file to understand context — not just the diff hunks.

### 5. Run the review checklist

For each changed file, verify:

| # | Check | What to look for |
|---|-------|-----------------|
| 1 | Plan compliance | All plan items implemented? Nothing missing, nothing extra? |
| 2 | Pattern correctness | Correct project patterns used? |
| 3 | No generated source changes | Generated sources must never be modified manually |
| 4 | Logging | Parameterized messages (`log.debug("x: {}", v)`) — no string concatenation |
| 5 | Error handling | Proper error responses checked before parsing? Null-safe patterns? |
| 6 | Test coverage | Every new/modified public method has a test? Edge cases covered? |
| 7 | Database migrations | Correct naming convention? Dual DB dialect support? |
| 8 | No hardcoded values | No hardcoded IDs, instance names, or secrets? |
| 9 | Field visibility | Appropriate access modifiers? |
| 10 | Annotations | Correct use of framework annotations? |

### 6. Check test quality

For each test file:
- Meaningful assertions (not just `assertNotNull`)?
- Edge cases covered?
- Mocking done correctly?
- Test naming convention: `test<What>_<Scenario>_<Expected>()`?

### 7. Run tests

```bash
cd <your-repo-path>-<TICKET_KEY>
mvn test -pl <module-path> -f pom.xml
```

### 8. Generate review document

```markdown
# Code Review: <TICKET_KEY> — <Summary>

**Date:** <today>
**Module:** <MODULE>
**Reviewer:** Lumen (Reviewer)
**Branch:** <branch name>
**Status:** ✅ Approved / ⚠️ Approved with comments / ❌ Changes requested

---

## Summary

<1-2 sentence summary of the review outcome>

## Reviewed Files

| File | Change | Assessment |
|------|--------|-----------|
| `<path>` | New/Modified | ✅ / ⚠️ / ❌ |

## Checklist

| # | Check | Result | Note |
|---|-------|--------|------|
| 1 | Plan compliance | ✅ | All planned changes implemented |
| ... | ... | ... | ... |

## Findings

### ⚠️ Warnings (non-blocking)

1. **<file>:<line>** — <description>
   - Recommendation: <suggested fix>

### ❌ Blockers (must fix)

1. **<file>:<line>** — <description>
   - Reason: <why this must be fixed>

## Tests

- **Executed:** <N> tests
- **Passed:** <N> ✅
- **Failed:** <N> ❌
- **Build:** ✅ Green / ❌ Red

## Recommendation

<Final recommendation: merge / fix and re-review / reject>
```

### 9. Store in BigMind

```python
memory_store_fact(
    category="codebase",
    fact=f"{TICKET_KEY}: Code review completed — {status}. {findings_count} findings ({blockers} blockers)."
)
```

## Severity Levels

| Level | Symbol | Meaning | Action |
|-------|--------|---------|--------|
| Blocker | ❌ | Must fix before merge | Changes requested |
| Warning | ⚠️ | Should fix, not blocking | Approved with comments |
| Info | ℹ️ | Suggestion for improvement | Approved |
| OK | ✅ | No issues | — |