Guide · Django

How to review Django pull requests: the 9 defect classes

Django pull requests borrow a lot of safety from the framework: the ORM parameterizes values, templates escape output, forms validate input. Each of those has an exit (`raw()`, `mark_safe`, a lookup by primary key alone), and AI-written code takes the exits without saying so. This guide walks the nine defect classes DiffDojo trains, in the order a reviewer should weigh them, with what each one looks like in Django code.

1. Security

Look for the places where the framework's protection is switched off: SQL passed to raw(), extra() or cursor.execute() with values formatted into the string instead of passed as parameters, mark_safe or the safe filter on user content, and csrf_exempt on a view that changes data. Then check object-level authorization: get_object_or_404(Model, pk=pk) loads any user's row unless the query is also filtered by the current user, and a DRF viewset whose get_queryset returns Model.objects.all() serves every user's rows.

2. Requirement fidelity

Read the ticket before the code. The default ordering, which statuses a queryset includes and the exact filter the ticket describes are where Django PRs most often do something reasonable that is not what was asked.

3. Edge cases & error handling

Ask what happens on missing and malformed input. Typical misses: Model.objects.get() raising DoesNotExist or MultipleObjectsReturned straight out of a view as a 500, int(request.GET[...]) on a missing or non-numeric parameter, and an outbound requests call in a view without a timeout.

4. Hallucinated APIs & dependencies

Models invent plausible ORM methods and lookups, such as a get_or_none() manager method that Django does not ship or a field lookup that does not exist. A misspelled setting is worse: Django reads settings as plain module attributes, so an invented setting name is silently ignored and the default stays in force.

5. State & data flow

A read followed by a write is a race under concurrent requests: reading a counter or a stock level, changing it in Python and calling save() loses updates unless the row is locked with select_for_update() inside a transaction or the write uses an F() expression. Also check side effects inside transaction.atomic(): they belong in transaction.on_commit.

6. Performance

Look for one query per row: a foreign key read in a loop or a template without select_related, or a reverse or many-to-many relation without prefetch_related. Also len(queryset) where count() was enough, and a whole table loaded into memory to process it row by row. With a few rows of test data none of this shows, so ask what the code does at the production size the ticket implies.

7. Data correctness

Money and time are the usual suspects. A FloatField for currency instead of DecimalField, rounding at the wrong step, and naive and aware datetimes mixed in one comparison all produce code that runs and returns the wrong number.

8. Deceptive tests

A green test proves only what it asserts. Check for tests that patch the queryset or the model method under test, assert only status_code == 200, or compute the expected value with the same expression as the implementation.

9. Over-engineering

Not a bug, but a review comment: a custom manager, an abstract base model or a service layer for a single call site, a signal where a direct call would be clearer, or a new dependency for what Django already ships (pagination, caching, signing). Ask for the simple version.

The checklist

1. Does any request value reach raw SQL or `mark_safe`, and is any data-changing view `csrf_exempt`?
2. Does the change do exactly what the ticket says, field by field?
3. Missing, malformed, empty input: what does the view return, and do outbound calls have timeouts?
4. Does every unfamiliar ORM method, lookup and setting actually exist?
5. Is a read-then-write protected by a lock or an F() expression?
6. Is there a query inside a loop or a template?
7. Are money and time handled with DecimalField and aware datetimes?
8. Would the tests fail if the code were wrong?
9. Is there a simpler way to write this?

As of October 2026, the DiffDojo library has 30 live Django PRs to practise on, each graded on whether you catch the planted bug or approve the clean one.

All review guides · All code review examples