diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 661cf7e..e8e74c3 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -2,7 +2,9 @@ ## Overview -REST API for managing football players built with Python and FastAPI. Implements async CRUD operations with SQLAlchemy 2.0 (async), SQLite, Pydantic validation, and in-memory caching. Part of a cross-language comparison study (.NET, Go, Java, Rust, TypeScript). +REST API for managing football players built with Python and FastAPI. Implements +async CRUD operations with SQLAlchemy 2.0 (async), SQLite, Pydantic validation, +and in-memory caching. ## Tech Stack @@ -31,23 +33,49 @@ tools/ — standalone seed scripts (run manually, not via Alembic) tests/ — pytest integration tests ``` -**Layer rule**: `Routes → Services → SQLAlchemy → SQLite`. Routes handle HTTP concerns only; business logic belongs in services. +**Layer rule**: `Routes → Services → SQLAlchemy → SQLite`. Routes handle HTTP +concerns only; business logic belongs in services. Never skip a layer. ## Coding Guidelines - **Naming**: snake_case (files, functions, variables), PascalCase (classes) - **Type hints**: Required everywhere — functions, variables, return types -- **Async**: All routes and service functions must be `async def`; use `AsyncSession` (never `Session`); use `aiosqlite` (never `sqlite3`); use SQLAlchemy 2.0 `select()` (never `session.query()`) -- **API contract**: camelCase JSON via Pydantic `alias_generator=to_camel`; Python internals stay snake_case -- **Models**: `PlayerRequestModel` (no `id`, used for POST/PUT) and `PlayerResponseModel` (includes `id: UUID`, used for GET/POST responses); never use the removed `PlayerModel` -- **Primary key**: UUID surrogate key (`id`) — opaque, internal, used for GET by id only. UUID v4 for API-created records; UUID v5 (deterministic) for migration-seeded records. `squad_number` is the natural key — human-readable, domain-meaningful, used for all mutation endpoints (PUT, DELETE) and preferred for all external consumers -- **Caching**: cache key `"players"` (hardcoded); clear on POST/PUT/DELETE; `X-Cache` header (HIT/MISS) -- **Errors**: Catch specific exceptions with rollback in services; Pydantic validation returns 422 (not 400) +- **Async**: All routes and service functions must be `async def`; use + `AsyncSession` (never `Session`); use `aiosqlite` (never `sqlite3`); use + SQLAlchemy 2.0 `select()` (never `session.query()`) +- **API contract**: camelCase JSON via Pydantic `alias_generator=to_camel`; + Python internals stay snake_case +- **Models**: `PlayerRequestModel` (no `id`, used for POST/PUT) and + `PlayerResponseModel` (includes `id: UUID`, used for GET/POST responses). + One request model intentionally covers both POST and PUT — per-operation + differences (conflict check on POST, mismatch guard on PUT) are handled at + the route layer, not by duplicating the model. Never reintroduce the removed + `PlayerModel`; it was removed because a single flat model conflated ORM, + request, and response concerns. +- **Primary key**: UUID surrogate key (`id`) — opaque, internal, used for GET + by id only. UUID v4 for API-created records; UUID v5 (deterministic) for + migration-seeded records. `squad_number` is the natural key — + human-readable, domain-meaningful, used for all mutation endpoints (PUT, + DELETE) and preferred for all external consumers +- **Caching**: cache key `"players"` (hardcoded); clear on POST/PUT/DELETE; + `X-Cache` header (HIT/MISS) +- **Errors**: Catch specific exceptions with rollback in services; Pydantic + validation returns 422 (not 400); squad number mismatch on PUT returns 400 + (not 422 — it is a semantic error, not a validation failure) - **Logging**: `logging` module only; never `print()` - **Line length**: 88; complexity ≤ 10 - **Import order**: stdlib → third-party → local -- **Tests**: naming pattern `test_request_{method}_{resource}_{context}_response_{outcome}`; docstrings single-line, concise; `tests/player_stub.py` for test data; `tests/test_main.py` excluded from Black -- **Avoid**: sync DB access, mixing sync/async, `print()`, missing type hints, unhandled exceptions +- **Tests**: integration tests against the real pre-seeded SQLite DB via + `TestClient` — no mocking. Naming pattern + `test_request_{method}_{resource}_{context}_response_{outcome}`; + docstrings single-line, concise; `tests/player_stub.py` for test data; + `tests/conftest.py` provides a `function`-scoped `client` fixture for + isolation; `tests/test_main.py` excluded from Black +- **Decisions**: justify every decision on its own technical merits; never use + "another project does it this way" as a reason — that explains nothing and + may mean replicating a mistake +- **Avoid**: sync DB access, mixing sync/async, `print()`, missing type hints, + unhandled exceptions ## Commands @@ -77,11 +105,12 @@ docker compose down -v ### Pre-commit Checks -1. Update `CHANGELOG.md` `[Unreleased]` section (Added / Changed / Fixed / Removed) +1. Update `CHANGELOG.md` `[Unreleased]` section (Added / Changed / Fixed / + Removed) 2. `uv run flake8 .` — must pass 3. `uv run black --check .` — must pass 4. `uv run pytest` — all tests must pass -5. `uv run pytest --cov=./ --cov-report=term` — coverage must be >=80% +5. `uv run pytest --cov=./ --cov-report=term` — coverage must be ≥80% 6. Commit message follows Conventional Commits format (enforced by commitlint) ### Commits @@ -90,45 +119,73 @@ Format: `type(scope): description (#issue)` — max 80 chars Types: `feat` `fix` `chore` `docs` `test` `refactor` `ci` `perf` Example: `feat(api): add player stats endpoint (#42)` +### Releases + +Tags follow the format `v{MAJOR}.{MINOR}.{PATCH}-{COACH}` (e.g. +`v2.0.0-capello`). The CD pipeline validates the coach name against a fixed +list (A–Z): + +``` +ancelotti bielsa capello delbosque eriksson ferguson guardiola heynckes +inzaghi klopp kovac low mourinho nagelsmann ottmar pochettino queiroz +ranieri simeone tuchel unai vangaal wenger xavi yozhef zeman +``` + +Never suggest a release tag with a coach name not on this list. + ## Agent Mode ### Proceed freely -- Add/modify routes and endpoints -- Service layer logic and cache management -- Tests (maintain async patterns and naming convention) +- Add/modify routes in `routes/player_route.py` and `routes/health_route.py` +- Add/modify service methods in `services/player_service.py` +- Add/modify Pydantic models in `models/player_model.py` (field additions or + docstring updates that don't change the API contract) +- Tests in `tests/` — maintain async patterns, naming convention, and + integration-test approach (no mocking) - Documentation and docstring updates - Lint/format fixes - Refactoring within existing architectural patterns ### Ask before changing -- Database schema (`schemas/player_schema.py` — no Alembic, use tools/ seed scripts manually) +- Database schema (`schemas/player_schema.py` — no Alembic; changes require + manually updating `storage/players-sqlite3.db` and the seed scripts in + `tools/`) +- `models/player_model.py` design decisions — especially splitting or merging + request/response models; discuss the rationale before restructuring - Dependencies (`pyproject.toml` with PEP 735 dependency groups) - CI/CD configuration (`.github/workflows/`) -- Docker setup -- API contracts (breaking Pydantic model changes) -- Global error handling +- Docker setup (`Dockerfile`, `docker-compose.yml`, `scripts/`) +- Breaking API contract changes (field renames, type changes, removing fields) +- Global error handling middleware +- HTTP status codes assigned to existing error conditions ### Never modify - `.env` files (secrets) +- `storage/players-sqlite3.db` directly — schema changes go through + `schemas/player_schema.py` and `tools/` seed scripts - Production configurations -- Async/await patterns (mandatory throughout) -- Type hints (mandatory throughout) -- Core layered architecture ### Key workflows -**Add an endpoint**: Add Pydantic model in `models/` → add async service method in `services/` with error handling → add route in `routes/` with `Depends(generate_async_session)` → add tests following naming pattern → run pre-commit checks. +**Add an endpoint**: Add Pydantic model in `models/` if the request/response +shape is new → add async service method in `services/` with error handling and +rollback → add route in `routes/` with `Depends(generate_async_session)` → +add tests following the naming pattern → run pre-commit checks. -**Modify schema**: Update `schemas/player_schema.py` → manually update `storage/players-sqlite3.db` (preserve 26 players) → update `models/player_model.py` if API changes → update services and tests → run `pytest`. +**Modify schema**: Update `schemas/player_schema.py` → manually update +`storage/players-sqlite3.db` (preserve all 26 seeded players) → update +`models/player_model.py` if the API shape changes → update services and tests +→ run `pytest`. -**After completing work**: Suggest a branch name (e.g. `feat/add-player-stats`) and a commit message following Conventional Commits including co-author line: +**After completing work**: Propose a branch name and commit message for user +approval. Do not create a branch, commit, or push until the user explicitly +confirms. Commit message format: ```text feat(scope): description (#issue) -Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> -Co-authored-by: Claude +Co-authored-by: Claude Sonnet 4.6 ```