Search across all documentation pages
Code review is the team's main teaching loop for Python craft: types, async pitfalls, SQL safety, and test gaps. High-signal culture means clear standards, respectful feedback, and predictable turnaround - not perfectionism that blocks delivery.
Quick-reference recipe card - copy-paste ready.
## PR description template
**Intent:** one sentence
**Risk:** migration / perf / security flag
**Test evidence:** pytest path, screenshot, load note
**Rollback:** flag or revert plan
## Review comment tiers
🔴 Must fix - correctness, security, data loss
🟡 Should fix - clarity, missing test (author may defer with ticket)
🟢 Nit - style; author discretionWhen to reach for this:
# Author addresses review with explicit trade-off comment
# REVIEW: kept sync ORM here intentionally - RFC-042 phase 1 read-only.
# Async migration tracked in ENG-4412.
def get_catalog_item(item_id: str) -> dict[str, object]:
...## Reviewer checklist (Python)
- [ ] Types on public functions / Pydantic models validated
- [ ] No blocking I/O in async def without asyncio.to_thread
- [ ] DB queries parameterized; no string SQL concat
- [ ] Tests cover happy path + one failure mode
- [ ] Logging structured; no PII in messagesWhat this demonstrates:
uv run pytest.| PR size | Target first response |
|---|---|
| < 200 LOC | Same business day |
| 200-500 LOC | 24 hours |
| > 500 LOC | Author should split |
# pyproject.toml - let CI enforce style so review focuses on design
[tool.ruff]
line-length = 100
select = ["E", "F", "I", "UP", "B", "ASYNC"]| Alternative | Use When | Don't Use When |
|---|---|---|
| Pair programming | Complex novel code | Simple CRUD with tests |
| Mob review session | Architecture shift | Daily small PRs |
| Auto-approve bots | Docs-only typo | Application logic |
| Formal inspection | Regulated safety | Normal product velocity |
One for routine; two for security, migrations, or shared libraries.
For risky changes yes; otherwise trust CI if tests comprehensive.
Author replies with rationale; escalate to tech lead if stalemate.
Yes - check loop blocking, pool settings, and rollback flag.
Review generator inputs and templates, not every line of output.
Focus on data leakage, non-determinism, and production promotion path.
Soft cap 400 LOC changed; exceptions need RFC link.
Never sole approver; author can merge after other approval only.
Time to first review, comment tier ratio, post-merge defect rate.
Use for checklist pass; human owns security and domain correctness.
Stack versions: This page was written for Python 3.14.0 (stable 3.14, maintenance 3.13), FastAPI 0.115+, Django 5.2, Flask 3.1, Pydantic 2, PyTorch 2.6+, pandas 2.2+, Polars 1.x, ruff 0.9+, and uv 0.6+.
Reviewed by Chris St. John·Last updated Jul 16, 2026