code-review-assistant

# Code Review Assistant

## Overview

Perform thorough, constructive code reviews that identify issues, suggest improvements, and ensure code quality, security, and maintainability.

## Review Categories

Examine code across these dimensions:

### 1. 🐛 Correctness & Bugs
- Logic errors
- Edge case handling
- Null/undefined checks
- Type mismatches
- Off-by-one errors
- Race conditions

### 2. 🔒 Security
- Input validation
- SQL injection risks
- XSS vulnerabilities
- Authentication/authorization flaws
- Sensitive data exposure
- Insecure dependencies

### 3. ⚡ Performance
- Algorithm efficiency (O(n) complexity)
- Memory leaks
- Unnecessary computations
- Database query optimization
- Caching opportunities

### 4. 🏗️ Code Quality
- Readability and clarity
- Naming conventions
- Code duplication (DRY principle)
- Function/method length
- Complexity (cyclomatic)
- SOLID principles

### 5. ✅ Testing
- Test coverage
- Edge case testing
- Unit vs integration tests
- Test quality and clarity
- Mock usage appropriateness

### 6. 📚 Documentation
- Code comments quality
- API documentation
- Function/method docstrings
- Complex logic explanation
- README updates

## Review Workflow

### Step 1: Understand Context

**Gather information:**
- What's the purpose of this code?
- What problem does it solve?
- What's the scope of changes?
- Are there related files to consider?

**For PRs:**
```bash
# View PR diff
gh pr diff <PR-NUMBER>

# View PR description
gh pr view <PR-NUMBER>

# See changed files
git diff --name-only main..HEAD
```

### Step 2: Read the Code

**First pass - high level:**
- Overall structure and organization
- Naming consistency
- Code patterns used
- Separation of concerns

**Second pass - detailed:**
- Line-by-line logic verification
- Edge cases and error handling
- Performance considerations
- Security implications

### Step 3: Identify Issues

Categorize findings by severity:

**🔴 Critical:** Must fix before merge
- Security vulnerabilities
- Data loss risks
- System crashes
- Breaking changes

**🟡 Important:** Should fix before merge
- Logic bugs
- Performance issues
- Poor error handling
- Missing tests for critical paths

**🔵 Minor:** Nice to have
- Code style inconsistencies
- Missing comments
- Minor optimizations
- Naming improvements

**💡 Suggestion:** Optional improvements
- Refactoring opportunities
- Alternative approaches
- Future considerations

### Step 4: Provide Feedback

Structure feedback constructively:

**For each issue:**
1. **Location:** File and line number
2. **Issue:** What's wrong
3. **Impact:** Why it matters
4. **Recommendation:** How to fix
5. **Example:** Code suggestion if helpful

**Tone guidelines:**
- Be specific and objective
- Focus on code, not the person
- Explain the "why" behind suggestions
- Acknowledge good practices
- Ask questions for clarification when unsure

## Review Template

```markdown
# Code Review: [PR Title / Code Description]

## Summary
- **Files Reviewed:** X files, Y lines changed
- **Overall Assessment:** [Approve/Request Changes/Comment]
- **Critical Issues:** N
- **Important Issues:** M
- **Minor Issues:** K

---

## 🔴 Critical Issues

### Issue 1: [Title]
**Location:** `path/to/file.py:42`

**Problem:**
[Clear description of the issue]

**Impact:**
[Why this is critical - security, data loss, crashes, etc.]

**Recommendation:**
[Specific fix needed]

**Example:**
```python
# Current (problematic)
user_input = request.GET['id']
query = f"SELECT * FROM users WHERE id = {user_input}"

# Suggested (fixed)
user_input = request.GET.get('id')
if user_input and user_input.isdigit():
    query = "SELECT * FROM users WHERE id = %s"
    cursor.execute(query, (user_input,))
```

---

## 🟡 Important Issues

### Issue 2: [Title]
[Same structure as above]

---

## 🔵 Minor Issues

### Issue 3: [Title]
[Shorter format acceptable for minor issues]

---

## 💡 Suggestions

- [Optional improvement 1]
- [Optional improvement 2]

---

## ✅ Positive Observations

- [Good practice 1]
- [Well-implemented feature]

---

## Questions for Author

- [Clarifying question about design decision]
- [Question about intended behavior]

---

## Recommendations

1. Fix all critical issues before merge
2. Address important issues or provide justification
3. Consider minor improvements where feasible
4. Add tests for edge cases X, Y, Z

**Overall:** [Approve with suggestions / Request changes / Needs discussion]
```

## Common Issues by Language

### Python
```python
# ❌ Mutable default argument
def append_to(element, to=[]):  # Bug: to persists across calls
    to.append(element)
    return to

# ✅ Correct
def append_to(element, to=None):
    if to is None:
        to = []
    to.append(element)
    return to

# ❌ Catching bare exceptions
try:
    risky_operation()
except:  # Too broad, masks errors
    pass

# ✅ Specific exception handling
try:
    risky_operation()
except ValueError as e:
    logger.error(f"Invalid value: {e}")
    raise

# ❌ String concatenation in loops
result = ""
for item in items:
    result += str(item)  # Creates new string each iteration

# ✅ Use join
result = "".join(str(item) for item in items)
```

### JavaScript/TypeScript
```javascript
// ❌ == instead of ===
if (value == null) {  // Loose equality
    // ...
}

// ✅ Strict equality
if (value === null || value === undefined) {
    // ...
}

// ❌ Unhandled promise rejection
fetchData().then(data => process(data));

// ✅ Error handling
fetchData()
    .then(data => process(data))
    .catch(error => console.error('Error:', error));

// ❌ Variable shadowing
const name = "Global";
function greet() {
    const name = "Local";  // Shadows outer name
    console.log(name);
}

// ✅ Distinct names
const globalName = "Global";
function greet() {
    const userName = "User";
    console.log(userName);
}
```

### Java
```java
// ❌ Resource leak
public void readFile(String path) {
    FileInputStream fis = new FileInputStream(path);
    // ... use fis
    // Missing close(),