TodyAI Learning

Giao diện

Buổi 15: AI Code Review Tự Động Trên PR ​

AI Code Review Tự Động

Mục Tiêu Buổi Học ​

Bạn sẽ xây dựng GitHub Action tự động review code trên PR, phát hiện code smells (magic numbers, bad naming, DRY violations) và bắt buộc dev fix bằng evidence — không phải chỉ đồng ý.

Phần 1: Lý Thuyết (20 phút) ​

cm-code-review: Full Review Lifecycle (3 Parts) ​

Code review không phải task một chiều. Nó là interactive workflow với 3 phases rõ ràng:

Part A: Request Review ​

  • Dev hoàn thành feature → push PR
  • Gọi reviewer (AI hoặc human)
  • Reviewer phải check đầu vào: có test không, architecture có hợp lý không

Part B: Receive Review ​

  • Reviewer tìm issues: bugs, code smells, violations
  • Post comments trực tiếp trên PR với evidence (line number, code snippet, reason)
  • Dev phải respond — không phải chỉ đồng ý mà phải verify

Part C: Finish Branch ​

  • Dev fix issues
  • Dev re-request review
  • Reviewer approve
  • Merge PR
  • Clean branch

Diagram Lifecycle:

┌─────────────────┐
│  Dev push PR    │
└────────┬────────┘
         │
    Part A: Request
         │
         ▼
┌─────────────────┐
│  AI Review      │
│  Post comments  │
└────────┬────────┘
         │
    Part B: Receive
         │
         ▼
┌─────────────────┐
│  Dev responds   │
│  with evidence  │
└────────┬────────┘
         │
    Part C: Finish
         │
         ▼
┌─────────────────┐
│  Approve +      │
│  Merge          │
└─────────────────┘

Anti-Pattern: Performative Agreement ❌ ​

BAD RESPONSE (Performative Agreement):

Reviewer: "This validates email without checking for SQL injection."

Dev: "Good catch! Fixed." ❌

Why bad?

  • Dev didn't actually verify
  • Might not have fixed correctly
  • Creates false sense of security
  • No learning happens

GOOD RESPONSE (Evidence-Based):

Reviewer: "This validates email without checking for SQL injection."

Dev: "I verified this: [evidence]" ✅
- Added SQLite parameterized query (line 47)
- Changed from: user.insert(email) 
- To: db.prepare('INSERT INTO users (email) VALUES (?)').run(email)
- The suggestion is correct because SQL injection on email field could expose user_id column
- Added unit test: test/security/sql-injection.test.js
- Fixed." ✅

Why good?

  • Dev showed they understand the problem
  • Dev verified the fix with evidence
  • Dev explained the risk
  • Dev added test coverage
  • Creates learning opportunity

Response Framework: 3 Types ​

Khi reviewer comment, dev respond theo framework:

SituationResponse
Technically CorrectFix it immediately. Acknowledge the issue.
Unclear IntentAsk clarification: "Do you mean...? Can you point to example?"
Technically QuestionableChallenge with evidence: "I think this is safe because... Can you explain the risk?"
Stylistic PreferenceDiscuss trade-offs: "This is more readable but slower by 5%. Trade-off worth it?"

cm-clean-code: 7-Point Hygiene Checklist ​

AI reviewer will scan for:

1. Dead Code → DELETE don't comment ​

javascript
// ❌ BAD
// const oldHandler = (req, res) => { ... }  // Not used anymore

// ✅ GOOD
// Removed oldHandler. Logic moved to newHandler (commit xyz)

2. Unused Imports → REMOVE ​

javascript
// ❌ BAD
import axios from 'axios';
import { readFile } from 'fs';
import moment from 'moment';

export const getUser = (id) => {
  // Only uses fetch, never uses axios, readFile, or moment
}

// ✅ GOOD
import { readFile } from 'fs';

export const getUser = (id) => {
  // Removed unused axios, moment
}

3. Magic Numbers & Strings → EXTRACT to named const ​

javascript
// ❌ BAD
if (email.length > 255) throw new Error('Email too long');
if (password.length < 8) throw new Error('Password too short');
if (attempts > 5) lockAccount();

// ✅ GOOD
const VALIDATION = {
  EMAIL_MAX_LENGTH: 255,
  PASSWORD_MIN_LENGTH: 8,
  LOGIN_MAX_ATTEMPTS: 5
};

if (email.length > VALIDATION.EMAIL_MAX_LENGTH) throw new Error(...);
if (password.length < VALIDATION.PASSWORD_MIN_LENGTH) throw new Error(...);
if (attempts > VALIDATION.LOGIN_MAX_ATTEMPTS) lockAccount();

4. Naming → RENAME to describe intent ​

javascript
// ❌ BAD
const x = userData.filter(u => u.age > 18);
const p = x.map(u => u.password);

// ✅ GOOD
const adultsOnly = userData.filter(user => user.age > 18);
const passwordsOfAdults = adultsOnly.map(user => user.password);

5. Single Responsibility Principle (SRP) → EXTRACT into separate units ​

javascript
// ❌ BAD — validateUser does 4 things
const validateUser = (user) => {
  // 1. Check email format
  // 2. Query database
  // 3. Log to file
  // 4. Send email notification
}

// ✅ GOOD — Separate concerns
const isValidEmail = (email) => { /* check format */ };
const userExists = (email) => { /* query database */ };
const logValidation = (email, result) => { /* log to file */ };
const notifyUser = (email) => { /* send email */ };

6. DRY — EXTRACT shared logic ​

javascript
// ❌ BAD — Duplicate validation in 3 routes
app.post('/register', (req, res) => {
  if (!req.body.email) return res.status(400).json({ error: 'Email required' });
  if (req.body.email.length > 255) return res.status(400).json({ error: 'Email too long' });
  // ...
});

app.put('/users/:id', (req, res) => {
  if (!req.body.email) return res.status(400).json({ error: 'Email required' });
  if (req.body.email.length > 255) return res.status(400).json({ error: 'Email too long' });
  // ...
});

// ✅ GOOD — Extract to middleware
const validateEmail = (req, res, next) => {
  if (!req.body.email) return res.status(400).json({ error: 'Email required' });
  if (req.body.email.length > 255) return res.status(400).json({ error: 'Email too long' });
  next();
};

app.post('/register', validateEmail, (req, res) => { /* ... */ });
app.put('/users/:id', validateEmail, (req, res) => { /* ... */ });

7. Nesting Depth > 3 → EXTRACT, early return, guard clauses ​

javascript
// ❌ BAD — 5 levels of nesting
function processOrder(order) {
  if (order) {
    if (order.items) {
      if (order.items.length > 0) {
        if (order.customer) {
          if (order.customer.verified) {
            // Finally do real work at depth 5
          }
        }
      }
    }
  }
}

// ✅ GOOD — Guard clauses, early return
function processOrder(order) {
  if (!order) return;
  if (!order.items || order.items.length === 0) return;
  if (!order.customer || !order.customer.verified) return;
  
  // Real logic at depth 1
  chargeCustomer(order);
  shipItems(order.items);
}

Phần 2: Thực Hành (60 phút) ​

Task 1: Tạo GitHub Action Cho AI Review ​

File: .github/workflows/ai-code-review.yml

yaml
name: AI Code Review

on:
  pull_request:
    types: [opened, synchronize]

jobs:
  review:
    runs-on: ubuntu-latest
    permissions:
      pull-requests: write
      contents: read

    steps:
      - uses: actions/checkout@v3
        with:
          fetch-depth: 0

      - name: Setup Node.js
        uses: actions/setup-node@v3
        with:
          node-version: '18'

      - name: Install dependencies
        run: npm install

      - name: Get changed files
        id: changed-files
        uses: tj-actions/changed-files@v44
        with:
          files: |
            src/**/*.js
            !src/**/*.test.js

      - name: Run AI Code Review
        if: steps.changed-files.outputs.any_changed == 'true'
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
          PR_NUMBER: ${{ github.event.pull_request.number }}
        run: |
          node scripts/code-review.js \
            --files="${{ steps.changed-files.outputs.all_changed_files }}" \
            --pr=$PR_NUMBER

Task 2: Viết scripts/code-review.js ​

File này sẽ scan code violations từ 7-point checklist → post review comments trên PR.

Tạo file: scripts/code-review.js

javascript
const fs = require('fs');
const path = require('path');
const https = require('https');

/**
 * AI Code Review for TaskFlow QA
 * Scans for 7-point clean code violations
 * Posts review comments to GitHub PR
 * 
 * Usage: node scripts/code-review.js --files="src/api.js src/db.js" --pr=42
 */

class CodeReviewer {
  constructor(config = {}) {
    this.files = (config.files || '').split(' ').filter(Boolean);
    this.prNumber = config.pr;
    this.githubToken = process.env.GITHUB_TOKEN;
    this.githubOwner = process.env.GITHUB_REPOSITORY_OWNER || 'your-org';
    this.githubRepo = process.env.GITHUB_REPOSITORY?.split('/')[1] || 'taskflow';
    this.comments = [];
    this.violations = {
      deadCode: 0,
      unusedImports: 0,
      magicNumbers: 0,
      badNaming: 0,
      srpViolation: 0,
      dryViolation: 0,
      nestingDepth: 0
    };
  }

  /**
   * Check 1: Dead Code — Comments that are NOT meaningful
   */
  checkDeadCode(file, content) {
    const lines = content.split('\n');
    
    lines.forEach((line, idx) => {
      const lineNum = idx + 1;
      
      // Pattern: const x = ... // Not used
      if (/^\s*\/\/\s*(const|let|var|function|class)/.test(line)) {
        this.comments.push({
          path: file,
          line: lineNum,
          side: 'RIGHT',
          body: `🧹 **Dead Code** (Point 1)
This looks like commented-out code. Please delete it instead of commenting.
If you need to preserve history, git log has it.

✅ What to do:
- Delete the commented line(s)
- Or add a comment explaining WHY (e.g., "// TODO: remove after migration")

${line.trim()}`
        });
        this.violations.deadCode++;
      }
    });
  }

  /**
   * Check 2: Unused Imports
   */
  checkUnusedImports(file, content) {
    const lines = content.split('\n');
    const imports = new Map(); // { name: lineNum }
    const usage = new Set(); // { name }

    // Find all imports
    lines.forEach((line, idx) => {
      const importMatch = line.match(/import\s+(?:{([\s\S]+?)}\s+from|(\w+)\s+from)/);
      if (importMatch) {
        const imported = importMatch[1] || importMatch[2];
        if (importMatch[1]) {
          // Named imports: import { a, b } from 'x'
          imported.split(',').forEach(name => {
            const cleanName = name.trim().split(' as ')[0];
            imports.set(cleanName, idx + 1);
          });
        } else {
          // Default import: import a from 'x'
          imports.set(imported, idx + 1);
        }
      }
    });

    // Find usage of imports
    const importedNames = Array.from(imports.keys());
    importedNames.forEach(name => {
      const importLine = imports.get(name);
      // Count usage outside import line
      const usageCount = lines.reduce((count, line, idx) => {
        if (idx + 1 === importLine) return count; // Skip import line itself
        return count + (line.includes(name) ? 1 : 0);
      }, 0);

      if (usageCount === 0) {
        this.comments.push({
          path: file,
          line: imports.get(name),
          side: 'RIGHT',
          body: `🧹 **Unused Import** (Point 2)
\`${name}\` is imported but never used.

✅ Remove this import:
\`\`\`diff
- import { ${name} } from '...'
\`\`\``
        });
        this.violations.unusedImports++;
      }
    });
  }

  /**
   * Check 3: Magic Numbers & Strings
   */
  checkMagicNumbers(file, content) {
    const lines = content.split('\n');
    
    // Pattern: if (x > 255) or if (attempts > 5) etc
    const magicPatterns = [
      { pattern: />\s*(\d{2,})/, type: 'number' }, // > 100, > 255
      { pattern: /<\s*(\d+)/, type: 'number' },     // < 10
      { pattern: /(===|==)\s*['"](\w+)['"]/, type: 'string' } // == 'status'
    ];

    lines.forEach((line, idx) => {
      magicPatterns.forEach(({ pattern, type }) => {
        if (pattern.test(line) && line.includes('if') || line.includes('while')) {
          const magic = line.match(pattern)?.[1] || line.match(pattern)?.[0];
          
          if (type === 'number' && magic && magic.length > 1) {
            this.comments.push({
              path: file,
              line: idx + 1,
              side: 'RIGHT',
              body: `✨ **Magic Number** (Point 3)
The number \`${magic}\` should be extracted to a named constant.

✅ What to do:
At top of file, create:
\`\`\`javascript
const LIMITS = {
  EMAIL_MAX_LENGTH: 255,
  PASSWORD_MIN_LENGTH: 8,
  LOGIN_MAX_ATTEMPTS: 5
};
\`\`\`

Then use:
\`\`\`javascript
if (email.length > LIMITS.EMAIL_MAX_LENGTH) { ... }
\`\`\``
            });
            this.violations.magicNumbers++;
          }
        }
      });
    });
  }

  /**
   * Check 4: Bad Variable Naming
   */
  checkBadNaming(file, content) {
    const lines = content.split('\n');
    
    // Single letter variables (except in loops)
    lines.forEach((line, idx) => {
      // Pattern: const x = or let d = or var p =
      const badNameMatch = line.match(/(?:const|let|var)\s+([a-z])\s*=/);
      
      if (badNameMatch && !line.includes('for')) {
        const varName = badNameMatch[1];
        this.comments.push({
          path: file,
          line: idx + 1,
          side: 'RIGHT',
          body: `📝 **Poor Variable Name** (Point 4)
Variable \`${varName}\` is too short. What does it represent?

✅ Better names:
- \`${varName}\` → \`users\` or \`data\` or \`timestamp\`?
- Use: \`const ${varName}Data = ...\` or \`const ${varName}List = ...\`
- Describe the **intent**, not the **type**`
        });
        this.violations.badNaming++;
      }
    });
  }

  /**
   * Check 5: Single Responsibility Principle
   */
  checkSingleResponsibility(file, content) {
    const lines = content.split('\n');
    
    // Look for functions that do too many things
    // Pattern: long function with multiple unrelated operations
    lines.forEach((line, idx) => {
      if (line.includes('const ') && line.includes('=') && line.includes('=>')) {
        // Get function body
        let functionStart = idx;
        let braceCount = 0;
        let functionBody = '';
        
        for (let i = idx; i < Math.min(idx + 20, lines.length); i++) {
          functionBody += lines[i];
          braceCount += (lines[i].match(/{/g) || []).length;
          braceCount -= (lines[i].match(/}/g) || []).length;
          
          if (braceCount === 0 && i > idx) break;
        }

        // Check for multiple operations (heuristic: many if/else, many function calls)
        const ifCount = (functionBody.match(/if\s*\(/g) || []).length;
        const callCount = (functionBody.match(/\.\w+\(/g) || []).length;

        if (ifCount > 3 && callCount > 3 && functionBody.length > 500) {
          this.comments.push({
            path: file,
            line: idx + 1,
            side: 'RIGHT',
            body: `🎯 **SRP Violation** (Point 5)
This function seems to do multiple things (${ifCount} conditionals, ${callCount} calls). Extract responsibilities.

✅ What to do:
1. **Identify responsibilities**: validation, database query, logging, response
2. **Extract**: Create separate functions
3. **Reuse**: Compose small functions

Example:
\`\`\`javascript
// Before: validateAndSave does 4 things
const validateAndSave = (user) => { /* 30 lines */ };

// After: Each function has ONE job
const isValidUser = (user) => { /* 5 lines */ };
const saveUser = (user) => { /* 3 lines */ };
const logUser = (user) => { /* 3 lines */ };
const notifyUser = (user) => { /* 3 lines */ };
\`\`\``
          });
          this.violations.srpViolation++;
        }
      }
    });
  }

  /**
   * Check 6: DRY — Don't Repeat Yourself
   */
  checkDRY(file, content) {
    const lines = content.split('\n');
    const codeBlocks = [];

    // Extract meaningful code lines (skip comments, blanks)
    lines.forEach((line, idx) => {
      const clean = line.trim();
      if (clean && !clean.startsWith('//') && clean.length > 20) {
        codeBlocks.push({ line: clean, num: idx + 1 });
      }
    });

    // Find duplicates
    for (let i = 0; i < codeBlocks.length; i++) {
      for (let j = i + 1; j < codeBlocks.length; j++) {
        const similarity = this.stringSimilarity(codeBlocks[i].line, codeBlocks[j].line);
        if (similarity > 0.85) { // 85% similar
          this.comments.push({
            path: file,
            line: codeBlocks[j].num,
            side: 'RIGHT',
            body: `🔄 **DRY Violation** (Point 6)
This code is similar to line ${codeBlocks[i].num}. Extract to shared function.

✅ What to do:
1. Create utility function
2. Call from both places

Line ${codeBlocks[i].num}:
\`\`\`javascript
${codeBlocks[i].line}
\`\`\`

Line ${codeBlocks[j].num}:
\`\`\`javascript
${codeBlocks[j].line}
\`\`\``
          });
          this.violations.dryViolation++;
          break; // Only report first duplicate
        }
      }
    }
  }

  /**
   * Check 7: Nesting Depth > 3
   */
  checkNestingDepth(file, content) {
    const lines = content.split('\n');
    let maxDepth = 0;
    let maxDepthLine = 0;

    lines.forEach((line, idx) => {
      const depth = (line.match(/{/g) || []).length - (line.match(/}/g) || []).length;
      
      // Cumulative depth
      let currentDepth = 0;
      for (let i = 0; i <= idx; i++) {
        currentDepth += (lines[i].match(/{/g) || []).length;
        currentDepth -= (lines[i].match(/}/g) || []).length;
      }

      if (currentDepth > 3) {
        if (currentDepth > maxDepth) {
          maxDepth = currentDepth;
          maxDepthLine = idx + 1;
        }
      }
    });

    if (maxDepth > 3) {
      this.comments.push({
        path: file,
        line: maxDepthLine,
        side: 'RIGHT',
        body: `📍 **Nesting Too Deep** (Point 7)
Nesting depth is ${maxDepth} levels. Keep it ≤ 3.

✅ Refactoring strategies:
1. **Guard Clauses** (early return):
\`\`\`javascript
// Before (depth 5)
if (x) {
  if (y) {
    if (z) {
      doWork();
    }
  }
}

// After (depth 1)
if (!x) return;
if (!y) return;
if (!z) return;
doWork();
\`\`\`

2. **Extract Helper Functions**:
\`\`\`javascript
const isValidInput = (x, y, z) => x && y && z;
if (!isValidInput(x, y, z)) return;
doWork();
\`\`\``
      });
      this.violations.nestingDepth++;
    }
  }

  /**
   * Simple string similarity (Levenshtein-like)
   */
  stringSimilarity(a, b) {
    const longer = a.length > b.length ? a : b;
    const shorter = a.length > b.length ? b : a;
    
    if (longer.length === 0) return 1.0;
    
    const editDistance = this.levenshteinDistance(longer, shorter);
    return (longer.length - editDistance) / longer.length;
  }

  levenshteinDistance(a, b) {
    const matrix = [];

    for (let i = 0; i <= b.length; i++) {
      matrix[i] = [i];
    }

    for (let j = 0; j <= a.length; j++) {
      matrix[0][j] = j;
    }

    for (let i = 1; i <= b.length; i++) {
      for (let j = 1; j <= a.length; j++) {
        if (b.charAt(i - 1) === a.charAt(j - 1)) {
          matrix[i][j] = matrix[i - 1][j - 1];
        } else {
          matrix[i][j] = Math.min(
            matrix[i - 1][j - 1] + 1,
            matrix[i][j - 1] + 1,
            matrix[i - 1][j] + 1
          );
        }
      }
    }

    return matrix[b.length][a.length];
  }

  /**
   * Post all review comments to GitHub PR
   */
  async postReviewComments() {
    if (this.comments.length === 0) {
      console.log('✅ No violations found! Code is clean.');
      return;
    }

    const summary = `
## 🔍 AI Code Review Results

Found **${this.comments.length} issues** across 7-point hygiene checklist:

| Check | Count |
|-------|-------|
| 🧹 Dead Code | ${this.violations.deadCode} |
| 🧹 Unused Imports | ${this.violations.unusedImports} |
| ✨ Magic Numbers | ${this.violations.magicNumbers} |
| 📝 Bad Naming | ${this.violations.badNaming} |
| 🎯 SRP Violation | ${this.violations.srpViolation} |
| 🔄 DRY Violation | ${this.violations.dryViolation} |
| 📍 Nesting Depth | ${this.violations.nestingDepth} |

👉 **Fix these issues** and **re-request review** for approval.

---
`;

    // Post summary as PR comment
    await this.postPRComment(summary);

    // Post individual review comments
    for (const comment of this.comments) {
      await this.postReviewComment(comment);
    }

    console.log(`\n✅ Posted ${this.comments.length} review comments to PR #${this.prNumber}`);
  }

  /**
   * POST to GitHub API: Create PR comment
   */
  postPRComment(body) {
    return new Promise((resolve, reject) => {
      const data = JSON.stringify({ body });
      
      const options = {
        hostname: 'api.github.com',
        path: `/repos/${this.githubOwner}/${this.githubRepo}/issues/${this.prNumber}/comments`,
        method: 'POST',
        headers: {
          'Authorization': `token ${this.githubToken}`,
          'Accept': 'application/vnd.github.v3+json',
          'Content-Type': 'application/json',
          'Content-Length': Buffer.byteLength(data)
        }
      };

      const req = https.request(options, (res) => {
        if (res.statusCode === 201) {
          resolve();
        } else {
          reject(new Error(`Failed to post comment: ${res.statusCode}`));
        }
        res.on('data', () => {});
        res.on('end', () => resolve());
      });

      req.write(data);
      req.end();
    });
  }

  /**
   * POST to GitHub API: Create review comment on specific line
   */
  postReviewComment(comment) {
    return new Promise((resolve, reject) => {
      const data = JSON.stringify({
        body: comment.body,
        path: comment.path,
        line: comment.line,
        side: comment.side
      });

      const options = {
        hostname: 'api.github.com',
        path: `/repos/${this.githubOwner}/${this.githubRepo}/pulls/${this.prNumber}/comments`,
        method: 'POST',
        headers: {
          'Authorization': `token ${this.githubToken}`,
          'Accept': 'application/vnd.github.v3+json',
          'Content-Type': 'application/json',
          'Content-Length': Buffer.byteLength(data)
        }
      };

      const req = https.request(options, (res) => {
        res.on('data', () => {});
        res.on('end', () => {
          if (res.statusCode === 201) {
            resolve();
          } else {
            resolve(); // Don't fail, continue to next
          }
        });
      });

      req.on('error', () => resolve());
      req.write(data);
      req.end();
    });
  }

  /**
   * Main workflow
   */
  async run() {
    console.log(`\n🔍 AI Code Review started...`);
    console.log(`PR #${this.prNumber}`);
    console.log(`Files to review: ${this.files.length}\n`);

    for (const file of this.files) {
      if (!fs.existsSync(file)) {
        console.log(`⚠️  File not found: ${file}`);
        continue;
      }

      const content = fs.readFileSync(file, 'utf-8');
      console.log(`Checking ${file}...`);

      this.checkDeadCode(file, content);
      this.checkUnusedImports(file, content);
      this.checkMagicNumbers(file, content);
      this.checkBadNaming(file, content);
      this.checkSingleResponsibility(file, content);
      this.checkDRY(file, content);
      this.checkNestingDepth(file, content);
    }

    await this.postReviewComments();
  }
}

// CLI
if (require.main === module) {
  const args = process.argv.slice(2).reduce((acc, arg) => {
    const [key, value] = arg.split('=');
    acc[key.replace('--', '')] = value;
    return acc;
  }, {});

  const reviewer = new CodeReviewer(args);
  reviewer.run().catch(err => {
    console.error('❌ Error:', err);
    process.exit(1);
  });
}

module.exports = CodeReviewer;

Task 3: Demo — Create PR with Code Violations ​

Step 1: Create feature branch

bash
git checkout -b feature/user-validation

Step 2: Write code with violations

File: src/api.js (with intentional violations)

javascript
// ❌ Violation 1: Unused import
import axios from 'axios';
import moment from 'moment';  // unused

// ❌ Violation 2: Bad naming
const x = [];

// ❌ Violation 3: Magic numbers
const validateUser = (user) => {
  if (user.email.length > 255) throw new Error('Too long');
  if (user.password.length < 8) throw new Error('Too short');
  
  // ❌ Violation 4: SRP — does validation + logging + notification
  if (user.age > 18) {
    console.log('User age: ' + user.age);
    sendEmail(user.email);
  }
};

// ❌ Violation 5: Deep nesting
const processOrder = (order) => {
  if (order) {
    if (order.items) {
      if (order.items.length > 0) {
        if (order.customer) {
          if (order.customer.verified) {
            // Finally do work at depth 5
          }
        }
      }
    }
  }
};

Step 3: Commit & Push

bash
git add .
git commit -m "feat: add user validation"
git push origin feature/user-validation

Step 4: Create PR on GitHub

Go to GitHub → Create Pull Request

Step 5: Watch AI Review

  • GitHub Actions runs
  • AI Code Review workflow triggers
  • Posts violations as comments:
    • Line 2: "Unused import axios"
    • Line 3: "Unused import moment"
    • Line 5: "Bad variable name x"
    • Line 8: "Magic number 255"
    • Line 14: "SRP violation — does 3 things"
    • Line 25: "Nesting depth 5 — too deep"

Task 4: Respond with Evidence ​

Dev Response to Comment #1 (Unused Import):

I verified this: [evidence]
✅ Fixed. Removed axios import at line 2.
- axios was added for future plan to call external API
- But we're using native fetch() instead
- Removed unused import
- Added comment in ROADMAP.md about why fetch not axios

The suggestion is correct because unused imports increase bundle size and reduce readability.

Dev Response to Comment #5 (SRP):

I verified this: [evidence]
✅ Fixed. Split into 3 functions:
- isValidUser() — only validation (line 8)
- logValidation() — only logging (line 15)
- notifyNewUser() — only notification (line 19)

The suggestion is correct because:
1. Easier to test each function independently
2. Can reuse validation without logging
3. Can notify user without validation

Added unit tests: src/__tests__/api.test.js
- test/validation/valid-user → passes ✓
- test/validation/invalid-email → passes ✓
- test/notification/triggers-email → passes ✓

📝 Quiz (5 câu) ​

  1. Full Lifecycle: Nêu 3 phases của code review workflow (Part A, B, C)
  2. Anti-Pattern: Tại sao "Good catch! Fixed." là performative agreement? Cách đúng là gì?
  3. Response Framework: Có 4 loại situation. Khi nào dev nên "challenge with evidence"?
  4. 7-Point Checklist: Nêu 4 violations trong code và cách fix
  5. GitHub API: Comment review dùng endpoint nào? (path, method)

🏠 Homework (30 phút) ​

  1. Create PR: Commit code với 3-4 violations
  2. Watch Review: AI posts comments
  3. Respond: Fix violations + respond with evidence (not just "fixed")
  4. Re-request: Re-request review
  5. Approve: Merge when approved

📚 Resources ​

Powered by CodyMaster × VitePress

© 2026 Todyle. All rights reserved.