| name | review-python-code |
| description | Review Python code for quality, readability, correctness, and project conventions. Use when: code review, review this file, check code quality, Python review, review PR changes, review my code. |
Python Code Review — SegmentationService
Quick-checklist review for Python code in this repository. Scan the provided code against each section below and report findings grouped by category. Skip categories with no findings.
Procedure
- Read the file(s) or selection the user provides.
- Run through each checklist section below.
- Report findings as a markdown table:
| Line(s) | Category | Finding | Suggestion |.
- Summarize with a one-line verdict: LGTM, minor issues, or needs rework.
Checklist
1 — Correctness & Logic
- Off-by-one, wrong comparison, unreachable code, swallowed exceptions.
- Mutable default arguments (
def f(x=[])).
- Resource leaks (open files, DB sessions, Dask futures not released).
- Correct use of
db_session inside Flask context; use SessionHelper outside it.
- Task state transitions follow
TaskState enum (ready → running → completed | error).
2 — Security (OWASP Top 10)
- No unsanitized user input in SQL, shell commands, or file paths.
- No hardcoded secrets, tokens, or credentials.
- No
eval(), exec(), pickle.loads() on untrusted data.
- Log files: no PII or secrets written via
lg.get_log().
3 — Project Conventions
- Logging: use
lg.get_log(__name__) with LogMessage enum values — never print().
- Config: access via
cfg.current_config.<ATTR> — never hardcode paths or env vars.
- Errors: raise
SegmentationError; format with create_error_response(ex) in views.
- API responses: business endpoints use
create_response() (JSON:API v1.0). Operational endpoints (/health, /gpu, /logs) use flask.jsonify().
- ONNX-first: use
deep_extractors_onnx.py — never import from archived deep_extractors.py. DnnClassifier tries OnnxInferencer first; TF is lazy-imported fallback only.
- Singletons:
FeatureExtractorFactory, Scheduler, FutureStore use @Singleton.
- Type hints: required on all public function signatures.
- Import order:
E402 is suppressed only in runserver.py, runscheduler.py, scripts/, tests/ — everywhere else imports must be at the top.
- TF isolation: TensorFlow imports must be lazy (
import tensorflow as tf inside functions) to preserve ONNX-only deployability.
- GPU detection: use
onnxruntime.get_available_providers(), never TF-based detection.
4 — Readability & Style
- PEP 8 compliance (Ruff rules: E4, E7, E9, F; E501 not enforced).
- Meaningful names; no single-letter variables outside tight loops.
- Functions ≤ 50 lines; classes have a single clear responsibility.
- No dead code, commented-out blocks, or TODO/FIXME without a tracking reference.
5 — Performance & Resource Usage
- NumPy/OpenCV vectorized ops instead of Python loops on image data.
- ONNX sessions cached via
@lru_cache — no redundant session creation.
- Dask futures properly tracked in
FutureStore and released after use.
- Avoid loading large arrays into memory when tiling/streaming is available.
6 — Testability
- Pure functions preferred; side effects isolated behind interfaces.
- New logic should be testable without GPU (
IS_GPU flag, CPU fallback).
- Mocking guidance: use
mock_helpers.py patterns; mock at the boundary, not deep internals.
- keep in in mind that the test also need to be able to run in a CPU-only environment, so any GPU-specific code should be abstracted behind interfaces that can be mocked or have CPU fallbacks.
Correctness of Doc
- check for a README.md file in the repository root that provides an overview of the project, installation instructions, usage examples, and contribution guidelines.
- ensure that the README.md file is well-structured, easy to read, and free of spelling and grammatical errors.
- check for its internal consistency, ensuring that the information provided is accurate and up-to-date with the codebase and the project's current state.
