AI-Assisted Code Review: Tools and Practices

// ❌ Issues AI catches reliably — humans shouldn't waste time on these
 
// 1. Unused variables
const result = await fetchUser(id);
const data = transformUser(result);
return result; // Should return data, not result
 
// 2. Missing error handling
async function getUser(id: string) {
  const response = await fetch(`/api/users/${id}`);
  return response.json(); // No status check
}
 
// 3. SQL injection
const query = `SELECT * FROM users WHERE id = '${userId}'`;
 
// 4. Hardcoded secrets
const API_KEY = 'sk-1234567890abcdef';
 
// 5. Performance anti-patterns
users.forEach(async (user) => {
  await sendEmail(user.email); // Sequential when parallel is safe
});

// ✅ AI-suggested fixes for the above
 
// 1. Correct variable usage
const result = await fetchUser(id);
const data = transformUser(result);
return data;
 
// 2. Proper error handling
async function getUser(id: string) {
  const response = await fetch(`/api/users/${id}`);
  if (!response.ok) {
    throw new Error(`Failed to fetch user: ${response.status}`);
  }
  return response.json();
}
 
// 3. Parameterized query
const query = 'SELECT * FROM users WHERE id = $1';
const result = await db.query(query, [userId]);
 
// 4. Environment variable
const API_KEY = process.env.API_KEY;
 
// 5. Parallel execution
await Promise.all(users.map(user => sendEmail(user.email)));

# .github/workflows/ai-review.yml
name: AI Code Review
 
on:
  pull_request:
    types: [opened, synchronize]
 
jobs:
  ai-review:
    runs-on: ubuntu-latest
    permissions:
      contents: read
      pull-requests: write
    steps:
      - uses: actions/checkout@v4
        with:
          fetch-depth: 0
 
      - name: Get changed files
        id: diff
        run: |
          FILES=$(git diff --name-only origin/main...HEAD -- '*.ts' '*.tsx')
          echo "files=$FILES" >> "$GITHUB_OUTPUT"
 
      - name: Run static analysis
        run: |
          npx eslint --format json ${{ steps.diff.outputs.files }} > lint-results.json
          npx tsc --noEmit 2> type-errors.txt || true
 
      - name: Security scan
        run: |
          npx audit-ci --high
          npx semgrep --config auto --json ${{ steps.diff.outputs.files }} > security.json
 
      - name: Complexity check
        run: |
          npx ts-complexity --threshold 15 ${{ steps.diff.outputs.files }} > complexity.json
 
      - name: Post review summary
        uses: actions/github-script@v7
        with:
          script: |
            const fs = require('fs');
            const lint = JSON.parse(fs.readFileSync('lint-results.json', 'utf8'));
            const security = JSON.parse(fs.readFileSync('security.json', 'utf8'));
 
            let body = '## AI Review Summary\n\n';
            const lintErrors = lint.filter(f => f.errorCount > 0);
            if (lintErrors.length > 0) {
              body += `### Lint Issues: ${lintErrors.length} files\n`;
            }
            if (security.results?.length > 0) {
              body += `### Security Findings: ${security.results.length}\n`;
            }
 
            await github.rest.issues.createComment({
              owner: context.repo.owner,
              repo: context.repo.repo,
              issue_number: context.issue.number,
              body: body || '## AI Review Summary\n\n✅ No issues found'
            });

interface PRRiskAssessment {
  riskLevel: 'low' | 'medium' | 'high' | 'critical';
  factors: string[];
  suggestedReviewers: string[];
  estimatedReviewTime: string;
}
 
function assessPRRisk(pr: PullRequestData): PRRiskAssessment {
  const factors: string[] = [];
  let riskScore = 0;
 
  // File change patterns
  if (pr.changedFiles.some(f => f.includes('migration'))) {
    factors.push('Database migration detected');
    riskScore += 3;
  }
 
  if (pr.changedFiles.some(f => f.includes('auth') || f.includes('security'))) {
    factors.push('Security-sensitive files modified');
    riskScore += 3;
  }
 
  if (pr.changedFiles.some(f => f.includes('.env') || f.includes('config'))) {
    factors.push('Configuration changes');
    riskScore += 2;
  }
 
  // Scale of change
  if (pr.additions + pr.deletions > 500) {
    factors.push(`Large change: ${pr.additions + pr.deletions} lines`);
    riskScore += 2;
  }
 
  // New dependencies
  if (pr.changedFiles.includes('package.json')) {
    factors.push('Dependency changes');
    riskScore += 1;
  }
 
  // API surface changes
  if (pr.changedFiles.some(f => f.match(/routes|controller|handler/))) {
    factors.push('API surface modified');
    riskScore += 2;
  }
 
  const riskLevel = riskScore >= 6 ? 'critical'
    : riskScore >= 4 ? 'high'
    : riskScore >= 2 ? 'medium'
    : 'low';
 
  return {
    riskLevel,
    factors,
    suggestedReviewers: getSuggestedReviewers(pr.changedFiles),
    estimatedReviewTime: riskScore >= 4 ? '30-60 min' : '10-20 min',
  };
}
 
function getSuggestedReviewers(files: string[]): string[] {
  // Map files to code owners or recent contributors
  const reviewers = new Set<string>();
  for (const file of files) {
    if (file.startsWith('src/auth/')) reviewers.add('security-team');
    if (file.startsWith('src/api/')) reviewers.add('api-team');
    if (file.includes('migration')) reviewers.add('dba-team');
  }
  return Array.from(reviewers);
}

## Human-Only Review Checklist
 
### Architecture & Design
- [ ] Does the approach make sense for this problem?
- [ ] Are there simpler alternatives that were not considered?
- [ ] Does this create technical debt we will regret?
- [ ] Does this align with the team's established patterns?
 
### Business Logic
- [ ] Does this correctly implement the requirements?
- [ ] Are edge cases from the domain handled?
- [ ] Will this break existing user workflows?
 
### Team Impact
- [ ] Will other team members understand this code?
- [ ] Does this increase or decrease on-call burden?
- [ ] Is the testing approach appropriate for the risk?
 
### Context
- [ ] Does the PR description explain WHY, not just WHAT?
- [ ] Are there related PRs that should be reviewed together?
- [ ] Does this need feature flag protection?

// ❌ AI would approve this — it's syntactically correct and has no bugs
async function processOrder(order: Order): Promise<void> {
  await chargePayment(order.total);
  await updateInventory(order.items);
  await sendConfirmation(order.customerEmail);
}
// But a human reviewer asks: What happens if chargePayment succeeds
// but updateInventory fails? The customer is charged but items aren't
// reserved. This needs a transaction or a saga pattern.
 
// ✅ Human insight leads to a more resilient design
async function processOrder(order: Order): Promise<void> {
  const paymentId = await chargePayment(order.total);
 
  try {
    await updateInventory(order.items);
  } catch (error) {
    await refundPayment(paymentId);
    throw new OrderProcessingError('Inventory update failed', { paymentId });
  }
 
  await sendConfirmation(order.customerEmail).catch(err => {
    // Email failure is non-critical — log and continue
    logger.error({ orderId: order.id, err }, 'Confirmation email failed');
  });
}