Code Reviewer

Overview

This skill provides a structured code review process following SF-Bench quality standards. It focuses on production-readiness, error handling, and adherence to project conventions.

When This Skill Applies

• •Reviewing code changes or pull requests
• •Assessing code quality before merging
• •Evaluating production-readiness
• •Checking adherence to project standards

Review Checklist

Critical (Must Fix)

Error Handling

• • Retry logic for transient operations (API calls, org creation)
• • Exponential backoff implemented correctly (2^n pattern)
• • Errors logged with full context (task ID, attempt number, error details)
• • No silent failures (all exceptions handled or propagated)

Resource Management

• • Scratch orgs deleted in finally blocks
• • Context managers used for file/connection handling
• • No resource leaks (unclosed connections, orphaned processes)
• • Timeouts set for external operations

Security

• • No hardcoded secrets or credentials
• • API keys read from environment variables
• • Sensitive data not logged
• • Input validation for user-provided data

Important (Should Fix)

Code Quality

• • PEP 8 compliance (line length 100)
• • Type hints on all function signatures
• • Docstrings follow Google style format
• • Functions focused and small (< 50 lines)

Logging

• • Structured logging with appropriate levels
• • Context included (task ID, operation name)
• • Debug logs for troubleshooting
• • No excessive logging in hot paths

Testing

• • Unit tests for new functionality
• • Edge cases covered
• • Error conditions tested
• • Test coverage meets threshold (≥80%)

Suggestions (Nice to Have)

Performance

• • Parallel processing where safe
• • Caching for expensive operations
• • Batch API calls when possible
• • No unnecessary iterations

Documentation

• • Complex logic explained in comments
• • README updated if needed
• • Docstrings accurate and complete
• • Examples provided for non-obvious usage

Review Process

Step 1: Understand the Change

1. •Read the PR description or commit message
2. •Identify the scope (files changed, components affected)
3. •Understand the intended behavior

Step 2: Check Critical Issues

1. •Scan for error handling patterns
2. •Verify resource cleanup
3. •Check for security issues
4. •Identify any breaking changes

Step 3: Review Code Quality

1. •Check adherence to style guidelines
2. •Verify type hints and docstrings
3. •Assess test coverage
4. •Review logging practices

Step 4: Validate Functionality

1. •Trace the code path mentally
2. •Identify edge cases
3. •Check error scenarios
4. •Verify business logic correctness

Step 5: Provide Feedback

1. •Prioritize issues (Critical > Important > Suggestions)
2. •Provide specific line references
3. •Suggest alternatives when rejecting
4. •Acknowledge good patterns

Feedback Format

For Issues

**[CRITICAL/IMPORTANT/SUGGESTION]** Line XX-YY

**Issue**: [Brief description of the problem]

**Why**: [Explanation of why this is problematic]

**Suggestion**: [Recommended fix or alternative]

For Approval

**Approved**

Changes look good. Key observations:
- [Positive observation 1]
- [Positive observation 2]

Minor suggestions (non-blocking):
- [Optional improvement 1]

SF-Bench Specific Patterns

Retry Pattern (Required for transient operations)

# Expected pattern
for attempt in range(max_retries):
    try:
        result = operation()
        return result
    except TransientError as e:
        if attempt < max_retries - 1:
            delay = initial_delay * (2 ** attempt)
            logger.warning(f"Attempt {attempt + 1} failed, retrying in {delay}s...")
            time.sleep(delay)
            continue
        raise

Cleanup Pattern (Required for scratch orgs)

# Expected pattern
try:
    org_alias = create_scratch_org()
    # ... operations ...
finally:
    delete_scratch_org(org_alias)  # Always executes

Logging Pattern (Required for operations)

# Expected pattern
logger.info(f"Starting operation for task {task_id}")
logger.debug(f"Input preview: {input[:500]}...")
logger.error(f"Operation failed: {error}", exc_info=True)

Anti-Patterns to Flag

Silent Failures

# BAD
try:
    operation()
except Exception:
    pass  # Silent failure

# GOOD
try:
    operation()
except Exception as e:
    logger.error(f"Operation failed: {e}", exc_info=True)
    raise

Resource Leaks

# BAD
org = create_scratch_org()
deploy(org)
# org never deleted if deploy fails

# GOOD
org = create_scratch_org()
try:
    deploy(org)
finally:
    delete_scratch_org(org)

Hardcoded Secrets

# BAD
api_key = "sk-12345abcde"

# GOOD
api_key = os.environ.get("API_KEY")

Missing Type Hints

# BAD
def process(data):
    return result

# GOOD
def process(data: Dict[str, Any]) -> List[Result]:
    return result