# Code Review Agents Course: [AI Agents](https://www.devtutorials.ai/learn/ai-agents) · Section: Agents for Development Workflows Source: https://www.devtutorials.ai/learn/ai-agents/dev-workflows/code-review-agents > How a review agent checks a pull request with best-practice and security rules, and how a bug agent goes from a log to a root cause, a failing test and a patch. Think about a teacher marking homework and a doctor treating a patient. The teacher follows a marking guide. They check the spelling, check that the working is shown, and check the final answer. A teacher with a good marking guide finds the same types of mistakes every time. A teacher who only checks spelling may miss a wrong answer, even if the work looks neat. A doctor works differently. Something is already wrong. They examine the symptoms, identify the cause, confirm it with a test, provide a treatment, and then check whether the treatment worked. This lesson introduces two agents that work in these two different ways. **The main idea:** A **review agent** checks a change against a set of rules before it is merged. A **bug agent** starts with a symptom from the logs, finds the root cause, reproduces the problem with a test, and then creates a fix. ## A Real-World Example **The review.** A teammate opens a pull request that adds ticket search. The changes include three intentional problems: 1. A `console.log` statement left over from debugging. 2. An API key written directly into the code. 3. A database query created by joining user input directly into SQL text. A review agent with rules for all three issues catches all three. A review agent that only checks for debug logs catches one problem and approves the rest. **The bug.** `logs/app.log` contains an error from production: ```text ERROR TypeError: Cannot read properties of undefined (reading 'toLocaleString') at formatRows (web/format.js:3) ``` The bug agent reads the log, finds the cause, writes a test that fails in the same way, patches the code, and runs the tests to prove the fix. **In simple words:** A review agent uses a checklist to review changes. A bug agent works like a doctor, starting from the symptom and finding the cause. ## What You Will Learn In this lesson, you will learn: 1. How a review agent checks a pull request using a set of rules. 2. The difference between best-practice rules and security rules. 3. What refactoring, modernization, and technical debt mean. 4. How a bug agent moves from a log message to the root cause. 5. Why a bug should be reproduced before it is fixed. The review agent has two tools, `read_file` and `review_diff`, and cannot make any changes. Its review rules are part of its configuration. The bug agent has `read_log`, `read_file`, `write_file`, and `run_tests`. ## Code Review ### Pull Request Review A **pull request** (PR) is a request to merge a change. Its **diff** shows added lines (starting with `+`) and removed lines (starting with `-`). A review agent reads the diff and adds review comments. Reviewing only the added lines keeps the review focused. The agent evaluates the change itself, not the entire codebase. ### Best Practices **Best-practice rules** identify habits that make code harder to maintain, such as debug logs left in the code, unclear names, or very long functions. These issues are important, but they usually do not cause direct harm by themselves. ### Security Review **Security rules** identify issues that can cause real harm: - **Secrets stored in code.** Anyone who can access the code can use the secret. - **SQL built from user input.** A user could enter SQL commands into the search field and change the query. This is called **SQL injection**. ```js runnable title="Review the added lines of a diff with rules" const diff = [ "+function searchTickets(db, term) {", '+ console.log("searching", term);', "+ return db.query(\"SELECT * FROM tickets WHERE title LIKE '%\" + term + \"%'\");", "+}", '+const API_KEY = "sk-live-4f9a8b7c6d5e";', ]; const rules = { "debug output": /console\.log\(/, "secret in code": /(API_KEY|SECRET|PASSWORD)\s*=\s*"/, "SQL built from input": /(SELECT|INSERT|UPDATE|DELETE)[^"]*"\s*\+/, }; for (const line of diff.filter((l) => l.startsWith("+")).map((l) => l.slice(1))) { for (const [name, re] of Object.entries(rules)) if (re.test(line)) console.log(name + ": " + line.trim()); } ``` Expected output: ```text debug output: console.log("searching", term); SQL built from input: return db.query("SELECT * FROM tickets WHERE title LIKE '%" + term + "%'"); secret in code: const API_KEY = "sk-live-4f9a8b7c6d5e"; ``` Each finding clearly identifies the rule and quotes the related line. This makes it easy for the author to understand exactly what needs to be fixed. All three issues are detected, so the agent requests changes and reports each finding. The same code change is reviewed again, but this time the agent has only one rule. It finds the debug log and approves the change with a single comment. The exposed API key and the SQL injection issue are missed. The agent did exactly what its rules told it to do. The weakness was in the review rules, not in the agent. A review agent is only as effective as its rules. "The agent approved the change" means "the change did not violate any of my rules." It does not automatically mean the change is safe. ```js runnable title="Must-fix or nice-to-have?" const findings = [ { rule: "debug output", line: 'console.log("searching", term);' }, { rule: "SQL built from input", line: "db.query(\"SELECT ... '\" + term)" }, { rule: "secret in code", line: 'API_KEY = "sk-live-…"' }, ]; const mustFix = ["SQL built from input", "secret in code"]; const sorted = [...findings].sort((a, b) => mustFix.includes(b.rule) - mustFix.includes(a.rule)); for (const f of sorted) console.log((mustFix.includes(f.rule) ? "MUST FIX " : "nice to have ") + f.rule); console.log(sorted.some((f) => mustFix.includes(f.rule)) ? "Verdict: request changes" : "Verdict: approve"); ``` Expected output: ```text MUST FIX SQL built from input MUST FIX secret in code nice to have debug output Verdict: request changes ``` Putting the must-fix items first helps the author see the most important issues before style or maintenance suggestions. ## Refactoring, Modernization, and Technical Debt A review agent can also suggest improvements that do not fix a bug: - **Refactoring** changes how code is written without changing its behavior. For example, splitting one long function into two smaller and clearer functions. - **Modernization** updates older coding patterns to newer ones. For example, replacing `var` with `let` and `const`, or replacing callbacks with `async`/`await`. - **Technical debt** is the cost of shortcuts taken in the past. A review agent can identify technical debt so the team can plan when to address it. ```js runnable title="Find modernisation suggestions" const code = [ "var total = 0;", "if (status == \"done\") total = rows;", "const label = `Done: ${rows}`;", ]; const suggestions = [ [/\bvar\s/, "use let or const instead of var"], [/[^=!]==[^=]/, "use === instead of =="], ]; code.forEach((line, i) => { for (const [re, tip] of suggestions) if (re.test(line)) console.log("line " + (i + 1) + ": " + tip); }); ``` Expected output: ```text line 1: use let or const instead of var line 2: use === instead of == ``` Suggestions like these should be marked as **"nice to have"** and kept separate from **"must-fix"** issues. Mixing them together can make serious problems harder to notice. ## Bug Investigation ### Log Analysis **Log analysis** means reading log entries to identify the error and understand what was happening when it occurred. The most useful parts of a log entry are the timestamp, the log level (such as INFO or ERROR), the message, and the location where the problem occurred. ```js runnable title="Pull the error out of a log" const log = [ "09:14:02 INFO GET /tickets/4410 200 12ms", "09:14:07 ERROR TypeError: Cannot read properties of undefined (reading 'toLocaleString')", " at formatRows (web/format.js:3)", " at renderTicket (ticket 4411, export status done, rows undefined)", "09:15:30 INFO GET /tickets/4412 200 11ms", ]; const i = log.findIndex((l) => l.includes("ERROR")); const where = /\((\S+):(\d+)\)/.exec(log[i + 1]); console.log("error: " + log[i].split("ERROR ")[1]); console.log("file: " + where[1] + ", line " + where[2]); console.log("context: " + log[i + 2].trim()); ``` Expected output: ```text error: TypeError: Cannot read properties of undefined (reading 'toLocaleString') file: web/format.js, line 3 context: at renderTicket (ticket 4411, export status done, rows undefined) ``` ### Root Cause The **root cause** is the actual reason the problem happened, not just the place where the application crashed. In this example, `formatRows` assumes that it will always receive a number. For ticket 4411, the row count was missing, so the function received `undefined` instead. ```js runnable title="Symptom versus root cause" function formatRows(n) { return n.toLocaleString("en-US") + " rows"; } for (const rows of [80000, 0, undefined]) { try { console.log(JSON.stringify(rows) + " -> " + formatRows(rows)); } catch (e) { console.log(JSON.stringify(rows) + " -> crash: " + e.constructor.name); } } ``` Expected output: ```text 80000 -> 80,000 rows 0 -> 0 rows undefined -> crash: TypeError ``` The symptom is a crash at line 3. The root cause is that the function was not designed to handle a missing count. Wrapping the crash in a `try/catch` block would only hide the symptom. Handling the missing count fixes the actual cause. ### Reproduce **Reproducing** a bug means making the same problem happen intentionally by writing a test. A test that fails in the same way as the log provides evidence that the agent has found the real cause. After the fix is applied, the same test proves that the bug has been fixed. The test then stays in the test suite so the bug cannot return without being detected. ### Patch A **patch** is the smallest possible change that fixes the root cause. In this example, a missing count is treated as 0 using `(n ?? 0)`. The agent starts with the log and then reads the related file. Next, it creates a test: `formatRows(undefined)` should return `"0 rows"`. The tests run and the new test fails, successfully reproducing the bug. The agent then applies the patch and runs the tests again. This time, all 9 tests pass. **Easy Way to Remember:** **No patch without a failing test.** If you cannot reproduce the bug, you cannot prove that you fixed it. ## Common Mistakes | Mistake | Wrong idea | Correct idea | What goes wrong | |---|---|---|---| | Few review rules | "The agent will notice anything bad." | Write rules for what matters, including security. | A leaked key passes review. | | Style mixed with safety | "All comments are equal." | Label must-fix and nice-to-have. | The SQL injection gets lost among style notes. | | Patch without reproducing | "I can see the problem." | Write a failing test first. | The "fix" doesn't fix the real cause. | | Fixing the symptom | "Wrap it in try/catch." | Fix the root cause. | The error hides, and wrong data shows. | | Throwing the test away | "The bug is fixed now." | Keep the test. | The bug comes back unnoticed. | ## Exercises Each exercise ends with a value on the last line. Press **Check** to compare it with the expected answer. ## What to Remember - Review code with a checklist. Investigate bugs like a doctor diagnosing a patient. - A review agent checks only the rules it has been given, so make sure those rules include important security checks. - Quote the relevant line in every review finding, and keep **must-fix** issues separate from **nice-to-have** suggestions. - A bug investigation follows a clear path: log → root cause → failing test → patch → passing test. - Never create a patch without first writing a failing test that reproduces the bug. Next: **Testing Agents**, the agents that measure coverage and check the dependencies.