Code Review Before Commit

Review code quality, security, and maintainability before committing changes to Linus Dashboard.

Your Role

Senior code reviewer ensuring quality, security, and maintainability.

Context Required: 1. .aidriven/memorybank.md 2. .aidriven/rules/ - All rule files 3. Changed files (git diff)

Review Checklist

1. Code Quality

Readability: - [ ] Functions < 50 lines - [ ] Variables have descriptive names - [ ] Complex logic has comments explaining WHY - [ ] Code is self-documenting

Structure: - [ ] Single responsibility per function/class - [ ] No code duplication - [ ] Logical organization - [ ] Consistent style

Type Safety: - [ ] All functions have type hints - [ ] No Any without justification - [ ] Return types specified - [ ] Proper use of Optional/Union

2. Documentation

• [ ] Module docstrings present
• [ ] Public functions/classes documented
• [ ] Docstrings follow Google style
• [ ] Complex algorithms explained
• [ ] No outdated comments

3. Error Handling

• [ ] Specific exceptions (no bare except)
• [ ] Errors logged with context
• [ ] User-friendly error messages
• [ ] Resources cleaned up (try/finally)
• [ ] No silent failures

4. Async/Await

• [ ] No blocking I/O in async functions
• [ ] All async functions awaited
• [ ] asyncio.gather() for parallel ops
• [ ] Proper timeout handling
• [ ] CancelledError handled

5. Home Assistant Patterns

• [ ] Integration lifecycle correct (setup/unload)
• [ ] Data stored in hass.data[DOMAIN][entry_id]
• [ ] Cleanup registered with entry.async_on_unload()
• [ ] Entities have unique_id
• [ ] Services registered properly
• [ ] Coordinator pattern used correctly

6. Security

• [ ] No hardcoded credentials
• [ ] User input validated
• [ ] SQL injection prevented
• [ ] XSS vulnerabilities addressed
• [ ] API keys stored securely
• [ ] No sensitive data in logs

7. Performance

• [ ] No N+1 queries
• [ ] Expensive operations cached
• [ ] Database queries optimized
• [ ] No memory leaks
• [ ] Async for I/O operations

8. Testing

• [ ] Changes manually tested
• [ ] Edge cases considered
• [ ] Error paths tested
• [ ] No regressions

Review Process

Step 1: Get Changed Files

# See what changed
git status

# View diff
git diff

# Check specific files
git diff path/to/file.py

Step 2: Analyze Changes

For each changed file: 1. Read the diff 2. Understand the purpose 3. Check against standards 4. Look for issues 5. Note improvements

Step 3: Check Build and Tests

# TypeScript
npm run build
npm run type-check
npm run lint:check

# Run smoke tests
npm run test:smoke

Step 4: Provide Feedback

Format:

File: path/to/file.py

✅ Good:
- Clear function names
- Proper type hints
- Good error handling

⚠️ Issues:
1. Line 42: Missing docstring
2. Line 78: Blocking I/O in async function
3. Line 103: Exception too broad

💡 Suggestions:
- Consider caching this result
- Extract this logic to separate function

Common Issues to Watch For

Python Issues

Async/Await:

# ❌ Bad - blocking I/O
async def fetch_data():
    response = requests.get(url)  # Blocks event loop

# ✅ Good - async I/O
async def fetch_data():
    async with aiohttp.ClientSession() as session:
        async with session.get(url) as response:
            return await response.json()

Error Handling:

# ❌ Bad - bare except
try:
    do_something()
except:
    pass

# ✅ Good - specific exception
try:
    do_something()
except ValueError as err:
    _LOGGER.error("Invalid value: %s", err)
    raise

Type Hints:

# ❌ Bad - no types
def process_data(data):
    return data.get("value")

# ✅ Good - with types
def process_data(data: dict[str, Any]) -> str | None:
    """Process data and return value."""
    return data.get("value")

TypeScript Issues

Type Safety:

// ❌ Bad - any type
function process(data: any): any {
    return data.value;
}

// ✅ Good - proper types
function process(data: DataType): string | undefined {
    return data.value;
}

Null Safety:

// ❌ Bad - no null check
const value = entity.state.toUpperCase();

// ✅ Good - null check
const value = entity.state?.toUpperCase() ?? "unknown";

Security Review Checklist

• [ ] No secrets in code
• [ ] API keys from config
• [ ] User input sanitized
• [ ] SQL queries parameterized
• [ ] HTML output escaped
• [ ] HTTPS for external calls
• [ ] Authentication checked
• [ ] Authorization verified

Performance Review Checklist

• [ ] No synchronous I/O in async code
• [ ] Database queries optimized
• [ ] Caching used appropriately
• [ ] No unnecessary API calls
• [ ] Efficient algorithms
• [ ] Memory usage reasonable

Final Review

Before approving:

Code Quality: - Follows project standards - Well-documented - Properly typed - Good error handling

Functionality: - Implements requirements - No obvious bugs - Edge cases handled - Tested manually

Maintainability: - Easy to understand - Well-structured - Follows patterns - Documented properly

Security: - No vulnerabilities - Input validated - Secrets protected

Review Outcome

APPROVE - Code meets all standards REQUEST CHANGES - Issues must be fixed COMMENT - Suggestions for improvement

Quick Commands

# Check what changed
git diff

# Check specific file
git diff path/to/file

# See commit history
git log --oneline -10

# Run quality checks
npm run lint:check
npm run type-check
npm run build

# Run tests
npm run test:smoke