docs(state): Phase D complete (D1-D3 + test-fakes refactor), Phase E scoping

This commit is contained in:
2026-05-17 01:08:04 +10:00
parent cbe9ae5d9c
commit 173032c12e
+44 -21
View File
@@ -1,6 +1,6 @@
# Premium Mechanic PWA — Implementation State # Premium Mechanic PWA — Implementation State
> Last updated: 2026-05-17 (end of session 2 — Phase B + Phase C done) > Last updated: 2026-05-17 (end of session 2 — Phase B + Phase C + Phase D done)
## What is done ## What is done
@@ -19,10 +19,23 @@
| C | C2: `GET /vehicles` list with `q`+`limit` search | ✅ DONE | commit `7be8783` — new `backend/app/api/mechanic_service.py` with `search_vehicles_with_last_inspection` (single aggregated subquery + outerjoin to avoid N+1 for last completed inspection per vehicle); endpoint in `mechanic.py`; new `VehicleListResponse` schema; 8 tests (6 endpoint + 2 service-stmt) with `_FakeSession.execute()` returning canned rows. Search hits plate (`CarV2.number`) + VIN only. | | C | C2: `GET /vehicles` list with `q`+`limit` search | ✅ DONE | commit `7be8783` — new `backend/app/api/mechanic_service.py` with `search_vehicles_with_last_inspection` (single aggregated subquery + outerjoin to avoid N+1 for last completed inspection per vehicle); endpoint in `mechanic.py`; new `VehicleListResponse` schema; 8 tests (6 endpoint + 2 service-stmt) with `_FakeSession.execute()` returning canned rows. Search hits plate (`CarV2.number`) + VIN only. |
| C | C3: `GET /vehicles/{id}` detail + bound /vehicles limit | ✅ DONE | commit `9109383` + style fix `90b8c66` — new `VehicleDetail(VehicleSummary)` + `InspectionSummary(_Base)` schemas; service `get_vehicle_by_id` + `recent_mechanic_inspections`; endpoint raises 404 when missing, `last_inspection_at` derived from first completed inspection in the list; applied `Query(20, ge=1, le=200)` bounds to C2's `/vehicles` limit param. 7 new tests (5 detail + 2 bounds). Service is now FastAPI-free (404 raise lives in endpoint, matching `automations.py` convention). | | C | C3: `GET /vehicles/{id}` detail + bound /vehicles limit | ✅ DONE | commit `9109383` + style fix `90b8c66` — new `VehicleDetail(VehicleSummary)` + `InspectionSummary(_Base)` schemas; service `get_vehicle_by_id` + `recent_mechanic_inspections`; endpoint raises 404 when missing, `last_inspection_at` derived from first completed inspection in the list; applied `Query(20, ge=1, le=200)` bounds to C2's `/vehicles` limit param. 7 new tests (5 detail + 2 bounds). Service is now FastAPI-free (404 raise lives in endpoint, matching `automations.py` convention). |
| D-pre | B3 schemas widened from vendor recon (`a187a92`) | ✅ DONE | After read-only recon of `car_inspections` (14k rows, 2yr): `InspectionType` extended 4→9 (added `initial`, `seizure`, `equipment-change`, `pre-repair`, `post-repair`); `DamageType` fully replaced 7→16 (removed `rust`/`glass`/`paint` which had 0 vendor occurrences; added real-world types from vendor distribution: `damaged`, `chip`, `scuff`, `burn`, `bent`, `puncture`, `bulge`, `not-working`, etc.). `Severity` unchanged (4 values, optional — vendor populates only 3%). 2 new positive-coverage tests. Stale comment in `mechanic_damage_marker.py:54` references old DamageType values — fix in Phase F when touching that model. | | D-pre | B3 schemas widened from vendor recon (`a187a92`) | ✅ DONE | After read-only recon of `car_inspections` (14k rows, 2yr): `InspectionType` extended 4→9 (added `initial`, `seizure`, `equipment-change`, `pre-repair`, `post-repair`); `DamageType` fully replaced 7→16 (removed `rust`/`glass`/`paint` which had 0 vendor occurrences; added real-world types from vendor distribution: `damaged`, `chip`, `scuff`, `burn`, `bent`, `puncture`, `bulge`, `not-working`, etc.). `Severity` unchanged (4 values, optional — vendor populates only 3%). 2 new positive-coverage tests. Stale comment in `mechanic_damage_marker.py:54` references old DamageType values — fix in Phase F when touching that model. |
| D | D1: `POST /inspections` with inline carry-over markers | ✅ DONE | commits `39a6579` + correctness fix `ee3d370` — service `start_inspection` + `latest_completed_inspection` (with `NULLS LAST` ordering); endpoint returns 201 with full InspectionDetail via re-query with `selectinload(photos, markers)`. Carry-over: copies all **unresolved** markers from prev completed inspection (`resolved=False` filter — closed defects don't reopen), each new marker has `carried_over_from_id=source.id`, `photo_id=None`, `resolved=False`. `_FakeSession` extended with `add`/`flush`/`commit`/`refresh` write-path methods + `rows_per_call` queue for multi-SELECT service flows. 9 new tests (6 endpoint + 3 service). |
| D | D2: `GET /inspections/{id}` with photos+markers | ✅ DONE | commit `1776a81` — service `get_inspection_by_id_with_relations` (selectinload both relationships, `scalar_one_or_none`); endpoint raises 404 with `"inspection not found"` parallel to vehicle's 404. D1's inline re-query block refactored to call the same helper (DRY). 6 new tests (4 endpoint + 2 service introspecting `stmt._with_options`). |
| D | D3: `PATCH /inspections/{id}` status/notes/finished_at | ✅ DONE | commit `aefe117` — service `get_inspection_by_id` (thin `session.get` wrapper) + `patch_inspection` (auto-sets `finished_at=now(UTC)` when status→completed AND finished_at not in payload; `setattr` loop applies all `model_dump(exclude_unset=True)` fields). Endpoint re-loads via `get_inspection_by_id_with_relations` for the response. 8 new tests (6 endpoint + 2 service). Endpoint and service both named `patch_inspection` — disambiguated by module path in code, by `as svc_patch` alias in tests. |
| D | Housekeeping: extract test fakes to `mechanic_fakes.py` | ✅ DONE | commit `eedbf23` — pure refactor. Moved `_FakeResult`, `_FakeSession`, `_fake_user`, `_fake_car`, `_fake_inspection`, `_fake_inspection_orm`, `_fake_marker_orm` from `test_mechanic_endpoints.py` (was 899 lines) to new `backend/tests/mechanic_fakes.py` (207 lines). Test file dropped to 697 lines. Pytest fixtures (`client`, `override_mechanic`, `override_db`) stayed in the test file. Phase E tests will import from `mechanic_fakes` directly. |
**Branch state:** `feat/mechanic-mvp` at `a187a92`. Full stack: `be0553e` (A4) → `d2113a0` (B1) → `2739cc8` + `2fb978d` (B3) → `f0d7e83` (B4) → `a0bdcaa` + `1e5b822` (C1) → `7be8783` (C2) → `9109383` + `90b8c66` (C3) → `a187a92` (B3-vendor-align). All pushed to `origin` (Gitea). **Branch state:** `feat/mechanic-mvp` at `eedbf23`. Full stack: `be0553e` (A4) → `d2113a0` (B1) → `2739cc8` + `2fb978d` (B3) → `f0d7e83` (B4) → `a0bdcaa` + `1e5b822` (C1) → `7be8783` (C2) → `9109383` + `90b8c66` (C3) → `a187a92` (B3-vendor-align)`39a6579` + `ee3d370` (D1) → `1776a81` (D2) → `aefe117` (D3) → `eedbf23` (test-fakes-refactor). All pushed to `origin` (Gitea).
**Test count on branch:** **85 mechanic tests pass** (38 models + 17 schemas + 9 deps + 21 endpoints across C1/C2/C3) in ~2s, no real DB needed. **Test count on branch:** **108 mechanic tests pass** (38 models + 17 schemas + 9 deps + 44 endpoints across C1-C3 + D1-D3) in ~2s, no real DB needed.
**Live mechanic API surface (7 routes):**
- `GET /api/v1/mechanic/healthz`
- `GET /api/v1/mechanic/me`
- `GET /api/v1/mechanic/vehicles?q=&limit=`
- `GET /api/v1/mechanic/vehicles/{vehicle_id}`
- `POST /api/v1/mechanic/inspections` — create with carry-over for type=return
- `GET /api/v1/mechanic/inspections/{inspection_id}` — full detail with photos+markers
- `PATCH /api/v1/mechanic/inspections/{inspection_id}` — partial update + auto finished_at
### Vendor data reality (from recon 2026-05-17 — 14,013 inspections, 2 years) ### Vendor data reality (from recon 2026-05-17 — 14,013 inspections, 2 years)
@@ -98,17 +111,25 @@ If Phase D/E/F needs a more complex query shape (e.g. `.scalars().one()` / `.fir
## Next task ## Next task
### **Task D1 — inspections CRUD foundation (create + list)** ### **Task E1 — MinIO async client wrapper** (Phase E starts)
(From `docs/superpowers/plans/2026-05-16-premium-mechanic-phase1.md` Phase D.) (From `docs/superpowers/plans/2026-05-16-premium-mechanic-phase1.md` Phase E, 5 tasks total: E1 MinIO wrapper, E2 presigned upload generation, E3 photo confirm endpoint, E4 photo-read 302 redirect, E5 celery thumbnail.)
**Working dir:** `c:\NewProject\taxi-dashboard` (branch `feat/mechanic-mvp`, HEAD `90b8c66`) **Working dir:** `c:\NewProject\taxi-dashboard` (branch `feat/mechanic-mvp`, HEAD `eedbf23`)
⚠️ **Heads up** — Phase D introduces **write operations** + the **carry-over markers** logic, which is qualitatively different from Phase C's read-only endpoints: ⚠️ **Heads up — Phase E introduces a new external dependency layer (MinIO/S3) and possibly Celery.** Qualitatively new domain:
- POST/PATCH endpoints need request-body validation (Pydantic schemas exist; verify they match the plan's body shapes) - **S3 client (`aioboto3` or `boto3` async wrapper)** — needs a thin wrapper module so endpoints don't import the SDK directly. Check what's already in `requirements.txt` (production has `media_proxy.py` and the existing `taxi-minio` container — there may be a usable abstraction already).
- Carry-over: when a new inspection is created for a vehicle, copy unresolved markers from the previous inspection (`prev_inspection_id` self-reference). Needs careful design. - **Presigned URL generation** for client-side direct uploads — credentials, expiration, content-type contract.
- Idempotency / race conditions: two mechanics opening the same vehicle simultaneously - **Celery for thumbnail generation** — this is a **major new dependency**. The TaxiDashboard backend may already have Celery (check `app/services/` or `app/tasks/`). If not, adding it is a big infrastructure step that warrants brainstorming + maybe a separate prep task.
- Real persistence with `_FakeSession`: tests need to verify the right `add()` / `flush()` / `refresh()` calls happened. The current `_FakeSession` only mocks read operations — needs extension for write paths (`add(obj)`, `flush()`, `refresh(obj)` to seed an `id`). - **Photo confirm flow** — client uploads to presigned URL → calls our `POST /photos/{id}/confirm` → backend updates `status='ready'` (matching the `MechanicInspectionPhoto.status` enum from B3).
- **302 redirect for reads** — `GET /photos/{id}` issues a 302 with a presigned download URL (avoids streaming binary through FastAPI).
**Recommendation for the next session:** before dispatching E1 implementer, **brainstorm**:
1. **Existing MinIO integration audit** — check `app/api/media_proxy.py`, look for existing S3 client setup, env vars (`MINIO_*` likely already in `.env`), bucket naming (the `pp-inspections` bucket is already set up per Phase A3).
2. **Celery audit** — is Celery already running? If yes, where; if no, do we want it for Phase 1 or can thumbnails be sync-on-confirm (faster ship, slightly slower confirm endpoint).
3. **Auth model for presigned URLs** — they're time-bounded but anyone with the URL can hit MinIO directly. Are we OK with that for inspection photos? Or do we want signed URLs to also encode a JWT claim?
Read the E1 task definition in the plan before deciding. The B3 `PresignedUploadRequest` / `PresignedUploadResponse` schemas are already shipped and define the API contract — the endpoints just need to be wired up.
### Phase D architecture decisions (RESOLVED 2026-05-17) ### Phase D architecture decisions (RESOLVED 2026-05-17)
@@ -158,32 +179,34 @@ In a new Claude Code chat in `c:\NewProject\`, paste this prompt:
Прочитай PremiumDriverApp/docs/superpowers/STATE.md — там полное состояние и следующая задача. Прочитай PremiumDriverApp/docs/superpowers/STATE.md — там полное состояние и следующая задача.
Phase C полностью завершён (C1C3, последний коммит 90b8c66, 83/83 тестов зелёных). Phase D полностью завершён (D1D3, последний коммит eedbf23, 108/108 тестов зелёных).
Следующая фаза — Phase D (inspections CRUD с carry-over markers) из плана Backend read+write API готов: 7 routes. Следующая фаза — Phase E (MinIO storage layer:
presigned upload + confirm + 302 read + celery thumbnail) из плана
PremiumDriverApp/docs/superpowers/plans/2026-05-16-premium-mechanic-phase1.md. PremiumDriverApp/docs/superpowers/plans/2026-05-16-premium-mechanic-phase1.md.
⚠️ Перед спавном D1 implementer-subagent сделай brainstorming pass по трём решениям ⚠️ Перед спавном E1 implementer сделай 3 проверки:
из секции "Next task" в STATE.md: (1) carry-over inline-vs-async, (2) _FakeSession 1. Audit existing MinIO integration: app/api/media_proxy.py, env vars, S3 client.
write-path extension, (3) пересмотр Option C для real-DB conftest в Phase D. 2. Audit Celery: уже установлен? Где? Иначе — sync thumbnails в Phase 1.
3. Решить про auth для presigned URLs (см. секцию "Next task" в STATE.md).
Используй superpowers:brainstorming, потом superpowers:subagent-driven-development. Используй superpowers:brainstorming для решений, потом superpowers:subagent-driven-development.
``` ```
That prompt is the **single entry point** for the next session — Claude will read STATE.md, ground itself, brainstorm the three Phase D architecture decisions, then dispatch the D1 implementer. That prompt is the **single entry point** for the next session — Claude will read STATE.md, audit existing MinIO/Celery state, resolve the auth-for-URLs decision, then dispatch the E1 implementer.
## Remaining roadmap (Phase 1 MVP) ## Remaining roadmap (Phase 1 MVP)
- ~~**Phase B** (4 tasks)~~ ✅ done — module skeleton, models, schemas, auth deps - ~~**Phase B** (4 tasks)~~ ✅ done — module skeleton, models, schemas, auth deps
- ~~**Phase C** (3 tasks)~~ ✅ done — GET /me, GET /vehicles, GET /vehicles/{id} - ~~**Phase C** (3 tasks)~~ ✅ done — GET /me, GET /vehicles, GET /vehicles/{id}
- **Phase D** (3 tasks): inspections CRUD with carry-over markers logic ← **NEXT** - ~~**Phase D** (3 tasks)~~ ✅ done — POST /inspections (+ carry-over), GET /inspections/{id}, PATCH /inspections/{id}
- **Phase E** (5 tasks): MinIO storage layer + presigned upload/confirm + 302 read + celery thumbnail - **Phase E** (5 tasks): MinIO storage layer + presigned upload/confirm + 302 read + celery thumbnail**NEXT**
- **Phase F** (2 tasks): markers create/update/delete - **Phase F** (2 tasks): markers create/update/delete
- **Phase G** (5 tasks): Vite + React + Tailwind + shadcn + PWA scaffold - **Phase G** (5 tasks): Vite + React + Tailwind + shadcn + PWA scaffold
- **Phase H** (3 tasks): Login + Home + VehicleCard pages - **Phase H** (3 tasks): Login + Home + VehicleCard pages
- **Phase I** (6 tasks): CameraCapture + PhotoSlot + VehicleScheme + InspectionEditor + InspectionReview - **Phase I** (6 tasks): CameraCapture + PhotoSlot + VehicleScheme + InspectionEditor + InspectionReview
- **Phase J** (4 tasks): e2e test + deploy backend + deploy frontend + smoke test - **Phase J** (4 tasks): e2e test + deploy backend + deploy frontend + smoke test
≈ 28 tasks left until MVP live on `mechanic.pptaxi.ru`. ≈ 25 tasks left until MVP live on `mechanic.pptaxi.ru`.
## Known external dependencies ## Known external dependencies