Code review
Reviews a diff for correctness first, style last.
Engineering · produces a Review · built in, ready to run
What you give it
Something like this:
public function applyDiscount(Order $order, string $code)
{
$discount = Discount::where('code', $code)->first();
if ($discount->expires_at < now()) {
return false;
}
$total = 0;
foreach ($order->items as $item) {
$total += $item->price * $item->qty;
}
$order->total = $total - ($total * $discount->percent / 100);
$order->discount_code = $code;
$order->save();
DB::statement("UPDATE discounts SET used = used + 1 WHERE code = '$code'");
return true;
}
What you get back
This is a real run of the skill on the input above, not a mock-up:
# Code Review: `applyDiscount`
## Correctness & Bugs
**Line 5: Null pointer on missing discount**
The code calls `$discount->expires_at` without checking if `$discount` is null. If the code doesn't exist in the database, `first()` returns null and the comparison crashes.
- Trigger: Any call with a non-existent discount code.
- Fix: Add `if (!$discount) return false;` after line 4.
**Line 8–11: Discount calculation ignores existing order state**
The code recalculates `$total` from scratch but doesn't account for items already removed or quantity changes since the order was created. If `$order->items` has been modified but not saved, the discount applies to stale data.
- Trigger: Calling this after `$order->items` has been modified in memory but `Order::refresh()` was never called.
- This is a design issue, not strictly a bug in this function, but worth noting.
---
## Security
**Line 17: SQL injection vulnerability**
The discount code is interpolated directly into a raw SQL statement. A code like `"; DROP TABLE orders; --` will execute arbitrary SQL.
- Trigger: Any discount code containing SQL metacharacters.
- Fix: Use parameterized query: `DB::update('UPDATE discounts SET used = used + 1 WHERE code = ?', [$code]);`
---
## Scale & Performance
**Line 8–11: N+1 query pattern (mild)**
If `$order->items` is not eager-loaded, this loops through items and may trigger a query per item. Not severe if items are already loaded, but worth ensuring the relation is eager-loaded before calling this method.
- Trigger: Calling with an order fetched via `Order::find()` without `->with('items')`.
**Line 17: Race condition on discount usage counter**
Two concurrent requests can read `used`, increment it, and write back the same value. The counter becomes inaccurate under load.
- Trigger: Two simultaneous `applyDiscount()` calls with the same code.
- Fix: Use `DB::update('UPDATE discounts SET used = used + 1 WHERE code = ?', [$code]);` (atomic increment, not read-modify-write).
---
## Readability
No issues that genuinely hurt clarity here. The logic is straightforward.
---
## Summary
**Must fix:** Null check (line 5), SQL injection (line 17).
**Should fix:** Race condition on counter (line 17).
**Consider:** Ensure items are eager-loaded to avoid N+1.
The wording changes a little every run. The shape does not — that is what the procedure below is for.
Tools it uses
None of its own. A skill that only writes needs no tools, so this one inherits whatever your assistant can normally reach.
The procedure it follows
This is the skill, in full. Add it to your own skills to change any of it.
Review the code below.
Order: correctness bugs, then security, then things that will break at scale, then readability. Style last, and only if it genuinely hurts.
For each finding: the line, what goes wrong, and the input or state that triggers it. If you cannot describe how it fails, it is not a finding — leave it out.
Say plainly when the change looks correct. A review that always finds something teaches people to ignore reviews.
How to run it
| Where | How |
|---|---|
| Chat | "Run my Code review skill", or pick it from the + menu |
| Skills page | Find Code review under Library and press Add & Use |
| Automations | Invoke it on every trigger |
| API | POST /v1/skills/{id}/run — add it to your own skills first, then take the id from GET /v1/skills |
In chat, on the Skills page and in automations you do not have to add it first — a library skill runs as it is. Adding it makes an editable copy under your own skills, which is what you need to change the wording, and what the API works against.
New to skills? Start with what a skill is.
Open App