>.prompt.md — Python Code Review

1---
2title: "Python Code Review"
3description: "Structured code review for Python — types, safety, patterns, tests"
4---
5 
6# Python Code Review
7 
8Review the provided Python code systematically using the checks below. For each section, report findings as **🔴 Must Fix**, **🟡 Should Fix**, or **💡 Suggestion**. If a section has no findings, skip it entirely — don't report clean passes.
9 
10Focus on bugs, security holes, and maintainability problems that actually ship to production. Ignore style nitpicks that ruff or black would catch automatically.
11 
12---
13 
14## 1. Type Safety & Signatures
15 
16Check every function signature and return type. Flag these specific patterns:
17 
18**Flag as 🔴:**
19- Functions missing return type annotations
20- `Any` used where a concrete type or generic would work
21- `dict` or `list` without type parameters (`dict[str, int]`, not `dict`)
22- Mutable default arguments: `def f(items: list[str] = [])` → use `None` sentinel
23- `Optional[X]` without a `None` check before use (potential `AttributeError`)
24 
25**Flag as 🟡:**
26- `Union` types that could be narrowed with `@overload`
27- Missing `TypeVar` bounds on generic functions
28- `cast()` calls — each one should have a comment justifying why
29- `# type: ignore` without an error code (`# type: ignore[assignment]` is fine)
30 
31**Check:** Would `mypy --strict` pass on this code? If not, what specific errors would it raise?
32 
33```
34# Bad: silent None bug
35def get_user(id: int) -> User | None:
36    ...
37 
38def process(id: int):
39    user = get_user(id)
40    user.name  # 🔴 no None check — AttributeError at runtime
41 
42# Good
43def process(id: int):
44    user = get_user(id)
45    if user is None:
46        raise ValueError(f"User {id} not found")
47    user.name  # safe — type narrowed
48```
49 
50---
51 
52## 2. Error Handling & Failure Modes
53 
54Trace every code path that can raise. Flag these patterns:
55 
56**Flag as 🔴:**
57- Bare `except:` or `except Exception:` that swallows errors silently
58- `except` blocks that `pass` without logging or re-raising
59- Missing error handling on I/O: file ops, network calls, DB queries
60- `KeyError`/`IndexError` risks on unvalidated external input (API params, env vars, config)
61 
62**Flag as 🟡:**
63- Catching too broadly: `except Exception` when `except (ValueError, TypeError)` would be precise
64- Missing `finally` or context manager for cleanup (file handles, DB connections, locks)
65- Error messages that don't include the failing value: `raise ValueError("invalid input")` → `raise ValueError(f"invalid input: {value!r}")`
66- Retryable operations (HTTP, DB) without retry logic
67 
68**Check for this anti-pattern specifically:**
69```
70# Bad: swallowed error, invisible failure
71try:
72    result = api_client.fetch(url)
73except Exception:
74    result = None  # 🔴 caller has no idea this failed
75 
76# Good: explicit, logged, typed
77try:
78    result = api_client.fetch(url)
79except httpx.HTTPStatusError as exc:
80    logger.warning("Fetch failed for %s: %s", url, exc)
81    raise
82```
83 
84---
85 
86## 3. Security & Input Validation
87 
88Scan for these specific vulnerability patterns. Any finding here is 🔴 unless noted.
89 
90**SQL injection:**
91- Raw string formatting in queries: `f"SELECT * FROM users WHERE id = {user_id}"`
92- String concatenation with `.format()` or `%` in SQL
93- Flag even if using an ORM — check raw query escapes (`RawSQL`, `extra()`, `.raw()`)
94 
95**Command injection:**
96- `os.system()`, `subprocess.run(shell=True)` with user input
97- `eval()`, `exec()` on any external input
98 
99**Path traversal:**
100- User-controlled file paths without sanitization
101- Missing `Path.resolve()` + prefix check for uploaded file names
102 
103**Secrets & credentials:**
104- Hardcoded API keys, passwords, tokens, DSNs
105- Secrets in default argument values
106- Credentials logged at any level (even DEBUG)
107 
108**Deserialization:**
109- `pickle.loads()` on untrusted data
110- `yaml.load()` without `Loader=SafeLoader`
111- `json.loads()` on user input without schema validation (🟡)
112 
113**Django-specific:**
114- `mark_safe()` on user-controlled content
115- `CSRF_COOKIE_HTTPONLY = False` or missing CSRF middleware
116- `DEBUG = True` without environment gate
117 
118**FastAPI/Flask-specific:**
119- Missing request body validation (Pydantic model vs raw dict)
120- `response.set_cookie()` without `httponly=True, secure=True, samesite="lax"`
121 
122---
123 
124## 4. Resource Management & Concurrency
125 
126**Flag as 🔴:**
127- File handles opened without `with` statement (resource leak)
128- DB connections not closed in error paths
129- `threading.Lock` acquired without `with lock:` pattern
130- Shared mutable state in async code without synchronization
131 
132**Flag as 🟡:**
133- Missing `async with` for async context managers (httpx.AsyncClient, aiofiles)
134- `time.sleep()` in async code (blocks the event loop — use `asyncio.sleep()`)
135- Unbounded queues or caches (`dict` used as cache without size limit → use `functools.lru_cache` or `cachetools.TTLCache`)
136- N+1 query patterns in Django: `for obj in queryset:` then `obj.related.field` without `select_related()`
137 
138```
139# Bad: blocks event loop
140async def fetch_all(urls: list[str]):
141    for url in urls:
142        time.sleep(1)  # 🟡 use asyncio.sleep
143        resp = httpx.get(url)  # 🔴 sync HTTP in async — use httpx.AsyncClient
144 
145# Good
146async def fetch_all(urls: list[str]):
147    async with httpx.AsyncClient() as client:
148        tasks = [client.get(url) for url in urls]
149        return await asyncio.gather(*tasks)
150```
151 
152---
153 
154## 5. Data Modeling & Validation
155 
156**Flag as 🔴:**
157- User/external input consumed without validation (raw `request.json`, `sys.argv`, env vars parsed manually)
158- Pydantic models with `model_config = ConfigDict(extra="allow")` in API boundaries (lets garbage through)
159- Django model fields missing `blank`/`null` constraints that match business rules
160 
161**Flag as 🟡:**
162- Stringly-typed data: status fields as `str` instead of `enum.StrEnum`
163- Business logic in serializers/schemas — push it to the model or service layer
164- Missing `__str__` and `__repr__` on domain objects (makes debugging painful)
165- Dataclasses where `frozen=True` would enforce immutability
166 
167```
168# Bad: stringly typed, no validation
169def create_order(status: str, priority: str): ...
170 
171# Good: enum-typed, validated
172class Status(enum.StrEnum):
173    PENDING = "pending"
174    SHIPPED = "shipped"
175    DELIVERED = "delivered"
176 
177class Priority(enum.IntEnum):
178    LOW = 1
179    MEDIUM = 2
180    HIGH = 3
181 
182def create_order(status: Status, priority: Priority): ...
183```
184 
185---
186 
187## 6. API Contract & Interface Design
188 
189**Flag as 🔴:**
190- Public function signatures that accept `**kwargs` and forward blindly (invisible API)
191- Breaking changes to public interfaces without deprecation path
192- Return type inconsistency: sometimes returns `dict`, sometimes `None`, sometimes raises
193 
194**Flag as 🟡:**
195- Functions with more than 5 positional parameters (use a dataclass/Pydantic model)
196- Boolean trap: `process(data, True, False)` — use keyword-only args or enums
197- Missing `__all__` in `__init__.py` for public packages
198 
199```
200# Bad: boolean trap, positional overload
201def send_email(to, subject, body, True, False, True, None): ...
202 
203# Good: keyword-only after required args
204def send_email(
205    to: str,
206    subject: str,
207    body: str,
208    *,
209    html: bool = False,
210    track_opens: bool = False,
211    reply_to: str | None = None,
212) -> None: ...
213```
214 
215---
216 
217## 7. Test Quality
218 
219If tests are included in the review, or if the code under review lacks tests:
220 
221**Flag as 🔴:**
222- Public functions/endpoints with zero test coverage
223- Tests that mock the thing they're testing (mock the dependency, not the subject)
224- Tests with no assertions (test runs but proves nothing)
225- `@pytest.mark.skip` without a linked issue or TODO
226 
227**Flag as 🟡:**
228- Tests that depend on execution order or shared mutable state
229- Missing edge case tests: empty input, `None`, boundary values, Unicode
230- Integration tests hitting real external services without `responses`, `respx`, or `vcr`
231- Fixtures that do too much — a fixture with 50 lines of setup is a missing factory
232 
233**Check:** Can you run `pytest --co -q` mentally on these tests and predict what they cover? If not, the test names need work.
234 
235```
236# Bad: mocks the subject, tests nothing
237def test_process_order(mocker):
238    mocker.patch("app.orders.process_order", return_value=True)
239    assert process_order(order) is True  # 🔴 you tested the mock
240 
241# Good: mocks the dependency
242def test_process_order(mocker):
243    mocker.patch("app.payments.charge", return_value=Receipt(id="r1"))
244    result = process_order(order)
245    assert result.status == Status.PAID
246    assert result.receipt_id == "r1"
247```
248 
249---
250 
251## 8. Performance & Scalability Flags
252 
253Only flag performance issues that would cause real production problems. Skip micro-optimizations.
254 
255**Flag as 🔴:**
256- Unbounded queries: `Model.objects.all()` without pagination in an API endpoint
257- `O(n²)` loops on collections that could grow: nested `for x in list: for y in list: if x == y`
258- Loading entire files into memory: `f.read()` on user-uploaded files (use chunked reading)
259 
260**Flag as 🟡:**
261- Missing database indexes on fields used in `filter()`, `WHERE`, or `ORDER BY`
262- Repeated identical queries in a request cycle (missing `select_related`/`prefetch_related`)
263- Sync I/O in hot paths that could use `asyncio` or background tasks
264- String concatenation in loops (`+=`) instead of `"".join()` or list building
265 
266---
267 
268## 9. Logging, Observability & Debugging
269 
270**Flag as 🟡:**
271- `print()` statements that should be `logger.info()` / `logger.debug()`
272- Missing structured context in log messages: `logger.error("Failed")` → `logger.error("Payment failed", extra={"order_id": order.id, "amount": amount})`
273- Caught exceptions without `logger.exception()` or `exc_info=True` (loses traceback)
274- No logging on error recovery paths (silent retries, fallbacks)
275 
276**Flag as 💡:**
277- Opportunities for `structlog` or JSON logging in services that feed into log aggregators
278- Missing request ID / correlation ID propagation in multi-service architectures
279- Endpoints without timing/metrics instrumentation
280 
281---
282 
283## 10. Python-Specific Gotchas
284 
285Flag these known footguns when you see them:
286 
287| Pattern | Problem | Fix |
288|---------|---------|-----|
289| `datetime.now()` | No timezone — ambiguous | `datetime.now(tz=UTC)` |
290| `== None` / `!= None` | Wrong comparison | `is None` / `is not None` |
291| `isinstance(x, str | int)` | Python <3.10 compat | `isinstance(x, (str, int))` |
292| `from module import *` | Namespace pollution | Explicit imports |
293| Class-level mutable: `items = []` | Shared across instances | Use `field(default_factory=list)` or `__init__` |
294| `os.environ["KEY"]` | Crashes if missing | `os.environ.get("KEY")` or `os.getenv("KEY", default)` |
295| `json.dumps(obj)` with dates | `TypeError` at runtime | Custom encoder or `.isoformat()` |
296| `hashlib.md5(password)` | Insecure hashing | `bcrypt`, `argon2`, or `hashlib.scrypt` |
297 
298---
299 
300## Output Format
301 
302Structure your review as:
303 
304```markdown
305## Review Summary
306 
307**Risk Level:** [Low / Medium / High / Critical]
308**Estimated Issues:** X must-fix, Y should-fix, Z suggestions
309 
310## Findings
311 
312### 🔴 [Section Name]: Brief description
313**Location:** `file.py:42` / `ClassName.method_name`
314**Problem:** What's wrong and why it matters
315**Fix:** Concrete code suggestion
316 
317### 🟡 [Section Name]: Brief description
318...
319 
320### 💡 [Section Name]: Brief description
321...
322 
323## What Looks Good
324 
325(2-3 bullets on what the code does well — don't skip this)
326```
327 
328Prioritize findings by severity. Lead with 🔴, then 🟡, then 💡. If there are more than 15 findings, group the 💡 suggestions into a summary list instead of expanding each one.
329