# 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