office_translator/_bmad-output/implementation-artifacts/code-review-1-1-to-1-8.md

Revue de code adverse — Stories 1-1 à 1-8 (Epic 1)

Workflow: code-review
Date: 2026-02-21
Revu par: Agent (adversarial senior developer)
Stories: 1-1, 1-2, 1-3, 1-4, 1-5, 1-6, 1-7, 1-8

Contexte: Comparaison Git vs File List des stories, validation des AC et des tâches [x], qualité de code, sécurité, perf, conformité architecture.

---

Discrepancies Git vs Story File List

• Fichiers modifiés (git) : .env.example, alembic/env.py, database/*, main.py, models/subscription.py, requirements.txt, routes/auth_routes.py, services/auth_service.py
• Fichiers non suivis (??) : alembic/versions/002_*, database/utils.py, middleware/tier_quota.py, tests/*, pytest.ini
• Constat : Les fichiers créés (migrations, utils, middleware, tests) ne sont pas commités — MEDIUM (transparence / traçabilité). Les File List des stories sont globalement alignées avec le code touché.

---

Story 1-1 : Setup base de données & modèle User

Statut story : in-progress
Sprint : in-progress

Problèmes trouvés (5)

Sévérité	Id	Description	Fichier:ligne / preuve
HIGH	1-1-H1	UserRepository.create() ne reçoit pas subscription_status — auth_service.create_user() appelle repo.create(..., subscription_status=SubscriptionStatus.ACTIVE.value) alors que la signature de UserRepository.create() n’a que email, name, password_hash, plan. → TypeError à l’inscription avec DB.	services/auth_service.py:285-291 vs database/repositories.py:41-56
MEDIUM	1-1-M1	datetime.utcnow() déprécié — L’architecture impose datetime.now(timezone.utc). Utilisé dans auth_service (create_access_token, create_refresh_token, _db_user_to_model, check_usage_limits, update_user JSON path) et dans repositories (increment_usage, update_status).	services/auth_service.py (multiples), database/repositories.py:99, 241
MEDIUM	1-1-M2	UserRepository.create(plan=PlanType.FREE.value) — On passe une str alors que la signature attend PlanType. Fonctionne avec l’Enum SQLAlchemy mais incohérent pour le typage et la maintenance.	services/auth_service.py:289
LOW	1-1-L1	Task 1.5 (UUID PK) reportée — La story le documente comme DEFERRED ; pas de preuve dans le code que la migration coordonnée est planifiée (lien vers une story/ticket).	Story 1-1, Task 1.5
LOW	1-1-L2	Modèle User — to_dict() n’expose pas tier ni daily_translation_count alors qu’ils sont centraux pour l’API (rate limit, billing). Risque d’incohérence si d’autres chemins s’appuient sur to_dict().	database/models.py:121-138

---

Story 1-2 : Inscription utilisateur

Statut story : done
Sprint : done

Problèmes trouvés (4)

Sévérité	Id	Description	Fichier:ligne / preuve
CRITICAL	1-2-C1	Même bug que 1-1-H1 — Inscription avec DB échoue à cause de subscription_status passé à repo.create(). Inscription en mode JSON fonctionne ; avec PostgreSQL/SQLite → crash.	services/auth_service.py:285-291
MEDIUM	1-2-M1	register_v1 retourne tier: user.plan.value — La story et l’architecture parlent de tier (free/pro). Si user vient de la DB, _db_user_to_model mappe plan ; le champ tier du modèle SQLAlchemy n’est pas exposé directement sur le modèle Pydantic User (subscription). Risque de divergence si plan et tier ne sont pas toujours synchronisés.	routes/auth_routes.py:382-388
LOW	1-2-L1	Réponse 201 — AC respectée (201 + user data). Pas de meta.rate_limit_remaining sur register ; acceptable car pas encore de traduction.	—
LOW	1-2-L2	File List story 1-2 — Liste des fichiers modifiés (dont requirements, database, alembic) reflète des changements “hors story” ; utile pour traçabilité mais à garder cohérent avec les stories 1-1 / 1-2.	Story 1-2, File List

---

Story 1-3 : Login utilisateur (JWT)

Statut story : done

Problèmes trouvés (4)

Sévérité	Id	Description	Fichier:ligne / preuve
MEDIUM	1-3-M1	login_v1 utilise user.password_hash — Propriété dépréciée (alias de hashed_password). Préférable d’utiliser user.hashed_password si le modèle subscription l’expose, sinon documenter la dépendance au legacy.	routes/auth_routes.py:419 (modèle subscription User)
MEDIUM	1-3-M2	Tokens JWT — create_access_token / create_refresh_token utilisent datetime.utcnow() au lieu de datetime.now(timezone.utc) (dépréciation Python 3.12+ et bonnes pratiques timezone).	services/auth_service.py:141-144, 165-168
LOW	1-3-L1	AC2 (JWT signing) — SECRET_KEY depuis l’env (JWT_SECRET / JWT_SECRET_KEY) ; OK. Pas de fallback en production à documenter.	—
LOW	1-3-L2	AC3/AC4 (INVALID_CREDENTIALS) — Même message pour “user not found” et “wrong password” ; conforme à la story (anti-énumération).	—

---

Story 1-4 : Logout utilisateur

Statut story : done

Problèmes trouvés (3)

Sévérité	Id	Description	Fichier:ligne / preuve
MEDIUM	1-4-M1	Blocklist JTI en mémoire — Documenté comme non thread-safe et perdu au redémarrage. Pour un déploiement multi-worker, la révocation ne sera pas partagée. Story 1.6 introduit Redis ; pas de migration de la blocklist vers Redis dans la codebase.	services/auth_service.py:76-94, Story 1-4 Dev Notes
LOW	1-4-L1	Corps logout optionnel — request.json() en cas d’exception → pass ; correct. Pas de validation stricte du body si présent.	routes/auth_routes.py:415-423
LOW	1-4-L2	Backward compat tokens sans JTI — Gérée (payload.get("jti") → pas de révocation). OK.	—

---

Story 1-5 : Refresh token

Statut story : done

Problèmes trouvés (3)

Sévérité	Id	Description	Fichier:ligne / preuve
MEDIUM	1-5-M1	refresh_v1 — Utilise user.plan.value pour create_access_token(..., tier=user.plan.value). Cohérent avec login_v1 ; même remarque que 1-2-M1 si un jour tier et plan divergent.	routes/auth_routes.py:431-432
LOW	1-5-L1	Guard isinstance(body, dict) — Présent ; évite 500 sur body non-objet. OK.	routes/auth_routes.py:401-409
LOW	1-5-L2	Rotation refresh token — Nouveau refresh_token émis à chaque refresh ; ancien non révoqué explicitement (pas demandé par la story). À considérer pour une future “rotation” sécurisée.	—

---

Story 1-6 : Middleware rate limiting par tier

Statut story : done

Problèmes trouvés (5)

Sévérité	Id	Description	Fichier:ligne / preuve
MEDIUM	1-6-M1	AC5 (meta.rate_limit_remaining) — L’architecture prévoit meta.rate_limit_remaining dans le body JSON. L’implémentation renvoie FileResponse pour /translate et met l’info dans les headers (X-Rate-Limit-Remaining, X-Rate-Limit-Reset-At). Pour un endpoint qui retourne du JSON, il faudrait meta dans le body ; pour un fichier, les headers sont une alternative raisonnable mais à documenter par rapport à l’AC.	main.py:621-678 ; Story 1-6 Completion Notes
MEDIUM	1-6-M2	Tier pour quota — _tier_for_quota(current_user.plan) utilise plan (PlanType). tier_quota accepte une string ("free"/"pro" etc.) ; cohérent avec tier en DB. Vérifier que tous les chemins (API key, admin) passent bien un tier aligné.	main.py:496, 612 ; middleware/tier_quota.py:129
LOW	1-6-L1	Sliding window — AC4 demande “sliding window”. Implémentation par clé Redis rate_limit:daily:{user_id}:{YYYY-MM-DD} = fenêtre par jour UTC, pas fenêtre glissante 24h. C’est un “fixed window” par jour. À aligner avec le libellé AC ou documenter le choix.	middleware/tier_quota.py, AC4
LOW	1-6-L2	Fallback in-memory — Par process ; documenté. OK pour MVP.	—
LOW	1-6-L3	auth_update_user en to_thread — Utilisation de asyncio.to_thread pour ne pas bloquer l’event loop ; OK.	main.py:632-636

---

Story 1-7 : Admin changement de tier manuel

Statut story : done

Problèmes trouvés (4)

Sévérité	Id	Description	Fichier:ligne / preuve
MEDIUM	1-7-M1	Préfixe endpoint — L’epic indique PATCH /api/v1/admin/users/{user_id}. Si l’app expose PATCH /admin/users/{user_id} (sans /api/v1), c’est une déviation par rapport à la convention du reste de l’API v1. À documenter ou aligner.	Story 1-7, main.py (route admin)
LOW	1-7-L1	Réponses 404/400 — Format { error, message, details? } sans data ; conforme à la revue précédente.	—
LOW	1-7-L2	Audit log — event admin_tier_change avec target_user_id, new_tier, timestamp ; pas de contenu document. OK.	—
LOW	1-7-L3	Literal plan — Schéma Pydantic avec Literal pour les plans ; 422 sur valeur invalide. OK.	—

---

Story 1-8 : Tracking usage pour billing

Statut story : review

Problèmes trouvés (5)

Sévérité	Id	Description	Fichier:ligne / preuve
MEDIUM	1-8-M1	create_completed dans to_thread — La création du log de traduction est dans un asyncio.to_thread(_create_translation_log). Si la session sync est utilisée ailleurs dans le même flux, attention aux risques de concurrence / fermeture de session. Le context manager get_sync_session() est bien utilisé dans la fonction passée à to_thread.	main.py:641-658
MEDIUM	1-8-M2	update_status(..., status="completed") — Utilise datetime.utcnow() pour completed_at au lieu de datetime.now(timezone.utc). Incohérent avec create_completed qui utilise timezone.utc.	database/repositories.py:241
LOW	1-8-L1	Champs translation_log — AC2 demande user_id, file_name, file_size, timestamp, status, provider_used. Le modèle Translation a original_filename, file_size_bytes, created_at, completed_at, status, provider. completed_at est renseigné dans create_completed ; OK.	database/models.py:141-184
LOW	1-8-L2	Pas de contenu fichier — Aucun champ ne stocke le contenu ; NFR11/NFR16 respectés.	—
LOW	1-8-L3	Skip si pas DB — Si USE_DATABASE ou DATABASE_AVAILABLE est False, pas d’entrée Translation ; documenté. OK.	main.py:643-644

---

Synthèse

Sévérité	Nombre
CRITICAL	1 (1-2-C1 = 1-1-H1, inscription DB)
HIGH	1 (1-1-H1)
MEDIUM	12+
LOW	15+

Problème bloquant commun aux stories 1-1 et 1-2 :
UserRepository.create() ne prend pas subscription_status ; l’appel dans auth_service.create_user() avec DB lève TypeError et casse l’inscription.

Recommandations prioritaires :

1. Corriger l’appel à UserRepository.create() : soit retirer l’argument subscription_status de l’appel dans auth_service.create_user() (le repo met déjà SubscriptionStatus.ACTIVE dans le modèle), soit ajouter le paramètre au repository et le faire passer jusqu’au modèle. Recommandation : retirer l’argument dans auth_service.
2. Remplacer datetime.utcnow() par datetime.now(timezone.utc) dans auth_service et repositories (migrations, completed_at, usage_reset, etc.).
3. Clarifier AC5 story 1-6 : documenter que pour POST /translate (FileResponse), le rate limit est exposé en headers et non dans un body meta.

---

Suite à donner

Conformément au workflow (étape 4) :

1. Corriger automatiquement — Appliquer les correctifs (au moins CRITICAL + HIGH + MEDIUM listés ci-dessus) dans le code et les tests.
2. Créer des action items — Ajouter une sous-section « Review Follow-ups (AI) » dans les tâches des stories concernées avec des items - [ ] [AI-Review][Severity] Description [file:line].
3. Détails — Approfondir un point précis (indiquer le numéro d’issue ou la story).

Réponse attendue : [1], [2] ou [3] (ou combinaison, ex. « 1 pour C1/H1, 2 pour le reste »).