Code review prompts: 16 ready-made checks
Sixteen snippets with a real defect in each and the question a reviewer would put about it. Every card already sets the language, the focus of the AI code review and where the code runs, so drop your own code in place of the sample.
Bugs and logic
Code that compiles and still lies, and the bugs no test caught: a Python mutable default argument that remembers the last call, a zero read as nothing, a timestamp with no zone, a regression shipped in a pull request.
Mutable default argument
The classic Python trap: the list in the signature outlives the call and collects other people’s data.
Review this cart function: customers report finding items in their order that they never added.def add_item(item, cart=[]): cart.append(item) return cart
Line 1 · critical · bugcart=[] is created once, when the module is imported, not on every call: the second customer keeps filling the first one’s cart, and so on until the process restarts.Fix: cart: list | None = None, and as the first line of the body, if cart is None: cart = [].
A zero read as nothing
A truthiness check hides a legitimate zero, so the customer with an empty balance sees a placeholder.
Look at this check: does it really tell a missing value from a zero?def render_balance(user): balance = user.get("balance") if not balance: return "no data" return format_money(balance)
Line 3 · major · bugif not balance matches None, 0 and 0.0 alike: a customer who has spent every last cent sees "no data" instead of an honest zero.Fix: if balance is None — test for absence explicitly, and let a real zero go through the formatter like any other number.
Time compared without a zone
datetime.now() returns the server’s local time while the token expiry is stored in UTC.
Check how token expiry is calculated. The service runs on servers in several time zones.def is_expired(token): return datetime.now() > token.expires_at
Line 2 · critical · bugdatetime.now() is naive local time and expires_at comes out of the database in UTC: on a European server tokens live two hours too long, and against an aware value the comparison fails outright with TypeError.Fix: datetime.now(timezone.utc), plus one rule across the project — everything stored is aware and in UTC.
A regression in a pull request
A review of the change rather than the file: what ships to production with this commit.
Review this pull-request diff: what exactly goes to production along with it?@@ -12,8 +12,7 @@ def apply_discount(order, percent): if order.paid: raise ValueError("order already paid")- if percent > 90:- raise ValueError("discount too large") order.total = order.total * (100 - percent) / 100+ order.discount_percent = percent
Lines 15-16 · critical · bugThe deleted guard was the only cap on percent: at 150 the order total goes negative, and checkout turns that into a refund.Fix: put the check back or move it into request validation, and add a percent=150 case to the tests.
Security
Every place input from outside reaches the database or the disk. Check code for vulnerabilities like SQL injection in a lookup, a live token left inside the script, and an upload path that walks out of its own folder.
SQL injection in a lookup
The address from the form is concatenated into the SQL, so whoever filled in the form now drives the database.
Review this user lookup. The email arrives straight from a form on the site.def find_user(conn, email): query = "SELECT * FROM users WHERE email = '" + email + "'" return conn.execute(query).fetchone()
Line 2 · critical · securityemail is glued into the SQL text: a value containing a quote closes the condition and everything after it runs as your own query — DROP TABLE included.Fix: a parameter instead of concatenation — conn.execute("SELECT id, email FROM users WHERE email = %s", [email]); that also retires the SELECT * currently fetching the password hash.
A token inside the script
A live key sits in the repository and gets printed into the build log on the way past.
Review this deploy script: is there anything dangerous here security-wise?#!/usr/bin/env bashAPI_TOKEN="sk_live_9f3c1ad84b22"echo "deploying with $API_TOKEN"curl -H "Authorization: Bearer $API_TOKEN" -X POST https://api.example.com/deploy
Lines 2-3 · critical · securityA production token is committed to a file in git: everyone who ever cloned the repo has it in their history, and echo copies it into the build log, which a much wider group reads.Fix: read it from the environment ($API_TOKEN with no default), drop the echo, and treat the key itself as leaked — rotate it.
Escaping the upload folder
The filename is taken from the request as it is, and a couple of dot-dot-slashes reach any file on the server.
Review this download endpoint: the filename arrives in the query string.def download(request): name = request.args.get("file") path = os.path.join("/var/app/uploads", name) return send_file(path)
Line 3 · critical · securityos.path.join walks straight out of /var/app/uploads when the name is ../../etc/passwd, and an absolute path discards the first argument altogether: any file the process can read is downloadable.Fix: os.path.basename(name), then compare os.path.realpath of the result against the uploads directory and serve the file only when it sits inside.
Performance
Three reasons for slow code that a review catches without a profiler: N+1 queries fired inside a loop, a statement that misses the index and scans the whole table, and a log file read into memory in one go.
Queries inside a loop
The report goes to the database twice per user: a thousand rows become two thousand queries.
Look at this report: it takes a minute for a thousand users. Where does the time go?def orders_report(user_ids): rows = [] for user_id in user_ids: user = db.query("SELECT name FROM users WHERE id = %s", user_id) orders = db.query("SELECT total FROM orders WHERE user_id = %s", user_id) rows.append((user.name, sum(o.total for o in orders))) return rows
Lines 3-5 · major · performanceTwo queries per user: a thousand users means two thousand round trips, and the time goes into the network, not the arithmetic.Fix: one query with a JOIN and GROUP BY users.id, or two queries with IN and the join done in memory.
A query that misses the index
A function on the column and a leading percent in LIKE switch the indexes off: the database reads the whole table.
Review this query: on a table of ten million rows it takes twenty seconds.SELECT *FROM ordersWHERE date_trunc('day', created_at) = '2026-09-01' AND lower(email) LIKE '%@example.com'ORDER BY created_at DESC
Lines 3-4 · major · performancedate_trunc on created_at makes the index useless, and a LIKE with a leading percent cannot use one at all: what is left is a seq scan over the entire table, and SELECT * drags in columns nobody reads.Fix: compare created_at against a range (from midnight on 1 September, and less than midnight on the 2nd), index the domain separately or keep it in a column of its own, and select only the fields you use.
The whole log in memory
The file is read in one go, so the size of the log becomes the size of the process.
Review this error counter for a log file. The files run to several gigabytes.def count_errors(path): lines = open(path).read().split("\n") return len([line for line in lines if "ERROR" in line])
Line 2 · major · performanceread() lifts the entire file into memory, split doubles that, and the list inside len() holds a third copy: on an eight-gigabyte log the OOM killer arrives before the answer does. The file is never closed either.Fix: with open(path) as f and sum(1 for line in f if "ERROR" in line) — line by line, with constant memory.
Readability and standards
Clean code principles applied to what is already written: four nested if statements waiting to be flattened, one-letter names against the style guide, and a VAT rate sitting as a magic number in two places.
Four nested conditions
The rule for sending the email hides at the fifth level of indentation and can only be read whole.
Review this function for readability: you have to read it to the end before you know the condition.def notify(user): if user is not None: if user.email is not None: if user.subscribed: if not user.banned: send_email(user.email) return True return False
Lines 2-5 · minor · readabilityFour nested ifs are one rule spread out as a staircase: to see who gets the email you have to hold all four conditions at once, while PEP 8 asks for the flat shape.Fix: early returns — if user is None: return False and so on — which leaves the body at a single level of indentation.
Names against the style guide
A capitalised method name and one-letter variables: RuboCop complains, and so does the next reader.
Review this method against RuboCop: what here goes against normal Ruby style?def CalcTotal(o) t = 0 o.each do |i| t = t + i.price * i.qty end tend
Lines 1-4 · minor · standardsNaming/MethodName: Ruby method names are snake_case, and CamelCase reads here as a constant. The names o, t and i say nothing about what they hold, and accumulating by hand is exactly the job of sum.Fix: def calc_total(items) and items.sum do |item| item.price * item.qty end — four lines collapse into one.
A VAT rate in two places
One rule copied into two functions, and it has already drifted: 20 per cent on the invoice, 19 on the receipt.
Review these two functions: are they calculating the same thing?def invoice_total(order): return round(order.subtotal * 1.2, 2)def receipt_total(order): return round(order.subtotal * 1.19, 2)
Lines 2 and 5 · major · architectureOne business rule is written twice and has already diverged: the invoice charges 20 per cent, the receipt 19, and the customer sees two different totals for one order.Fix: one VAT_RATE constant and one function both of them call, so the rate changes in exactly one place.
Tests and reliability
What happens on the day something fails: test coverage that stops at the happy path, an exception swallowed by an empty catch, and a backup script that never once checks whether the file it wrote can be read.
Tests for the happy path only
A single passing case creates the impression of coverage that is not there.
Review these tests for apply_discount: what is missing from them?def test_apply_discount(): order = Order(total=100) apply_discount(order, 10) assert order.total == 90
Line 1 · major · testsOne case is covered: an ordinary discount on an unpaid order. Nothing for zero, a hundred per cent, a negative value, an already paid order or the rounding on 33.33 — any of those branches can break unnoticed.Fix: parametrize the boundary values and add a separate pytest.raises test for percent=150 and for the paid order.
A swallowed exception
except Exception: pass turns a failure into "ok" and wipes the trail out of the logs.
Review this profile save: users say their changes sometimes disappear.def save_profile(user, data): try: db.update(user.id, data) search.reindex(user.id) except Exception: pass return "ok"
Lines 5-6 · critical · bugexcept Exception: pass swallows everything and still returns "ok" when the write never happened: the user sees success, the data is gone and the log is empty. Two operations under one try also make a failing reindex indistinguishable from a failing database.Fix: catch the specific exceptions, log with logger.exception and return an honest status; move the reindex out so its failure cannot cancel the save.
A backup that never checks
The script ignores exit codes and deletes the old copies before anyone finds out the new one is empty.
Review this nightly backup script: a restore from the latest copy did not work.#!/usr/bin/env bashpg_dump "$DATABASE_URL" > /backup/db.sqlgzip -f /backup/db.sqlfind /backup -name "db.sql.gz" -mtime +7 -delete
Lines 2-4 · critical · bugThere is no set -euo pipefail: if pg_dump fails the file is created anyway — empty — gzip compresses it happily, and find deletes the working copies older than a week; after seven days not one intact backup is left. An unset $DATABASE_URL goes unnoticed the same way.Fix: set -euo pipefail on the first line, a size check on the dump after pg_dump, and deleting old copies only once the new one has succeeded.