github-copilot/.github/skills/python-review/references/review-checklist.md

Review Checklist

Full criteria for categorizing findings in the Python review skill.

🔴 Must Fix — Correctness & Type Safety

These block merge. Apply by default.

mypy strict violations

• error: any mypy error (all are must-fix under strict mode)
• Implicit Any — untyped function parameters or return values
• type: ignore without a specific error code (e.g. bare # type: ignore)
• Incompatible types in assignment or return
• Missing --strict compatibility: missing py.typed, missing stubs

Logic & Security (manual review)

• Mutable default arguments: def f(x: list = []) → use None + guard
• Shadowing built-ins: list, dict, id, type, input, etc.
• Uncaught exceptions from external I/O without explicit handling
• SQL/shell injection risks (string formatting into queries/commands)
• Hardcoded secrets or credentials
• assert used for runtime validation (stripped with -O)

---

🟡 Should Fix — Code Quality

These hurt maintainability. Fix unless explicitly skipped.

ruff violations (common)

• E501 — line too long (respect line-length from pyproject.toml)
• F401 — unused import
• F841 — local variable assigned but never used
• B006 — mutable argument default
• B007 — loop variable unused
• UP prefix — pyupgrade rules (use modern Python syntax)
• SIM prefix — simplifiable conditions
• RUF prefix — Ruff-specific best practices

Manual quality

• Missing return type annotation on public functions/methods
• Missing docstring on public API (classes, public methods, module)
• pass in except block without a comment explaining why
• print() left in production code (use logging)
• Overly broad except Exception without re-raise or specific handling
• Dead code: unreachable branches, commented-out code blocks

---

🟢 Consider — Optimization & Modern Python 3.13+

Suggestions only. Offer but don't apply without confirmation.

Python 3.13+ idioms

• Use type X = ... (PEP 695) instead of TypeAlias
• Use X | Y union syntax instead of Union[X, Y] or Optional[X]
• Use TypeVar with type statement (PEP 695) instead of old TypeVar()
• @override decorator (PEP 698) for overriding methods
• tomllib (stdlib) instead of third-party TOML parsers
• pathlib.Path instead of os.path string operations

Performance

• Generator expressions instead of list comprehensions when result is iterated once
• __slots__ on data-heavy classes to reduce memory
• functools.cache / functools.lru_cache for pure deterministic functions
• Avoid repeated attribute lookup in tight loops (local = self.attr)
• Prefer collections.deque over list for O(1) pop from front

Readability

• Replace long if/elif chains with match/case (structural pattern matching)
• Extract deeply nested logic into named helper functions
• Use dataclasses.dataclass or pydantic.BaseModel for plain data containers
• Named tuples or TypedDict instead of untyped dict returns