Guide · FastAPI

How to review FastAPI pull requests: the 9 defect classes with examples

FastAPI handlers look safe because Pydantic validates the body. Validation covers types and constraints, not authorization or intent: it will not notice a missing ownership check, a blocking call in an async route or a sort order that contradicts the spec. This guide walks DiffDojo's nine defect classes as they show up in FastAPI services.

1. Security

Check every route that takes an id: the dependency that loads the current user is not an authorization check until the query is filtered by that user. Also look for SQL built with f-strings and secrets returned in a response model.

2. Requirement fidelity

Compare query parameters and defaults with the ticket. A sort that defaults to ascending when the spec says newest first, or a filter that matches one status when the spec lists three, passes every type check.

Worked example:

• 'Newest first' but sorted ascending: the order is reversed

3. Edge cases & error handling

Outbound HTTP calls need a timeout, and an async def route must not call a blocking client such as requests, which stalls the event loop. Look at what the route returns when an optional field is missing or a list is empty.

Worked example:

• requests.get with no timeout can hang the handler forever

4. Hallucinated APIs & dependencies

FastAPI, Starlette and Pydantic v1 and v2 have similar but incompatible APIs, which makes invented or outdated calls easy to miss: @field_validator in a project pinned to v1, .dict() and class Config where v2 expects model_dump() and model_config, a parameter a dependency never accepted. Check the version the project pins.

5. State & data flow

Module-level clients and caches are shared by every request a worker process handles, but not between workers. A dict used as a per-user store leaks state between callers and also disagrees from one worker to the next.

6. Performance

One database query per item in a list endpoint, or awaiting calls one by one when they could run concurrently with a bound, is the usual cost. With three items in a test neither may be noticeable unless the test counts queries or controls timing, so ask how the endpoint behaves at the page size the frontend actually requests.

7. Data correctness

Never mix aware and naive datetimes: datetime.now() without a timezone against a timestamp stored in UTC either raises or silently shifts by the server's offset. Use datetime.now(timezone.utc).

Worked example:

• datetime.now() is naive: comparing it to an aware datetime raises, and it's local time anyway

8. Deceptive tests

A TestClient test that overrides the dependency it is supposed to exercise, or asserts only the status code, can stay green while the route returns the wrong data. Check that at least one test exercises the real dependency, or a fake that behaves like it, and asserts on the response body.

9. Over-engineering

A repository layer, a service layer and an abstract base class for one CRUD route is a review comment, not a bug. Ask what the extra layers buy.

The checklist

1. Is every id-based query filtered by the current user?
2. Do defaults and filters match the ticket exactly?
3. Do outbound calls have a timeout, and no blocking call sits in an async route?
4. Do the Pydantic and FastAPI calls exist in the pinned versions?
5. Is any module-level object holding per-request data?
6. Any query per item, or sequential awaits that could be bounded-concurrent?
7. Are datetimes timezone-aware end to end?
8. Does the test exercise the real dependency and check the body?
9. Is every layer earning its place?

As of October 2026, the DiffDojo library has 14 live FastAPI 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