Skill de Révision

Objectif

Révision finale du code de tous les changements non validés avant de pousser. Détecte les problèmes faciles à introduire lors du développement itératif : commentaires obsolètes référençant du code supprimé, code mort, violations de règles et incohérences.

Comment Réviser

Étape 1 : Identifier les fichiers modifiés

Exécutez git diff --name-only et git diff --cached --name-only pour obtenir la liste complète des fichiers modifiés (staged + unstaged). Si l'utilisateur a fourni un argument de chemin, filtrez pour ne garder que les fichiers sous ce chemin.

Étape 2 : Lire tous les fichiers modifiés

Lisez chaque fichier modifié en entier. Pour chaque fichier, vérifiez les éléments ci-dessous.

Étape 3 : Exécuter les vérifications

Pour chaque fichier modifié, vérifiez :

Code Mort

Imports inutilisés (import non référencé ailleurs dans le fichier)
Champs, méthodes ou getters privés inutilisés
Code inaccessible après des early returns
Blocs de code commentés (doivent être supprimés, non commentés)

Références Obsolètes

Commentaires ou docstrings référençant des méthodes, classes ou variables qui n'existent plus dans le codebase (utilisez Grep pour vérifier que les références sont toujours valides)
Commentaires ABOUTME qui ne décrivent plus précisément le fichier
Commentaires TODO pour du travail déjà complété dans cette session

Docs Brainstorm

Tout fichier sous mobile/docs/brainstorm/ (typiquement datés comme YYYY-MM-DD-issueNNNN-…-brainstorm.md) est un artefact de travail, non un livrable. Au moment où la PR est prête, la justification doit être dans la description de la PR ; le fichier ne doit pas être livré avec le commit de fusion.

Signalez chaque fichier staged sous ce chemin et demandez à l'utilisateur si le brainstorm doit être supprimé ou converti sur place :

Le supprimer (par défaut) — git rm le fichier. Utilisez un commit distinct docs: drop … brainstorm doc pour que la suppression soit auditable.
Le convertir en decision record — seulement si le doc a une valeur durable au-delà de la PR (par exemple, capture une approche rejetée que les lecteurs futurs continueront de proposer). Renommez, déplacez hors de brainstorm/, et réduisez aux parties durables. Précédent : cd723075e docs(notifications): convert badge-desync brainstorm to decision record.

Précédents de suppression pure : 380bf50c1 docs: drop PR #4229 brainstorm doc, et le suivi de la révision PR #4234.

Violations de Règles AGENTS.md

Lisez et appliquez TOUTES les règles de AGENTS.md et .claude/rules/. Ne codifiez pas de règles spécifiques ici — consultez toujours la source de vérité dans ces fichiers.

Patterns Flutter — Résultats de Révisions Connus

Ces patterns ont émergé des cycles de révision passés sur ce repo et ne sont pas dans les règles toujours chargées (pour garder la context window globale légère). Cherchez-les explicitement lors de la révision de code.

Design System

Widget sur mesure diverge de divine_ui sans note de docstring. Tout widget qui diffère délibérément d'un composant divine_ui (taille différente, structure différente, variante contournée) doit dire pourquoi dans son docstring de classe — quel composant du système de design s'en rapproche et ce qui a forcé la divergence. Sans cela, les reviewers relèveront « pourquoi ne pas utiliser DivineIconButton ? »
```
// Bon
/// Visuellement équivalent à un [DivineIconButton] en style ghost mais dimensionné 64×64
/// avec une icône de 32 px au lieu des présets 40×40/56×56 de DivineIconButton,
/// parce que le spec Figma (node 15314:53971) demande une tap target plus grande
/// que n'importe quelle taille standard de DivineIconButton.
class CenterPlaybackControl extends StatelessWidget { ... }
```

Widget API Design

Paramètres spéculatifs sur des widgets réutilisables. Avant d'ajouter un paramètre à quoi que ce soit dans divine_ui ou lib/utils/, confirmez que la branche qu'il déverrouille est atteignable depuis au moins un appelant. Un paramètre Builder/Callback/Resolver avec exactement un appelant qui le fournit toujours est une branche morte. Supprimez-le — l'ajouter plus tard est bon marché ; les branches mortes confondent les lecteurs et gonflent la surface d'API.
```
// Mauvais — initialChildSizeBuilder est seulement appelé depuis une surface où
// le clavier n'est jamais ouvert, la branche est donc inaccessible.
static Future<T?> show<T>({
  double initialChildSize = 0.6,
  double Function(BuildContext)? initialChildSizeBuilder,
}) { ... }

// Bon — branche spéculative supprimée.
static Future<T?> show<T>({double initialChildSize = 0.6}) { ... }
```
Injection d'ancêtre via une fermeture builder one-off. Quand un site d'appel a besoin de placer un BlocProvider / InheritedWidget au-dessus de chaque slot d'une modal/sheet/route, ajoutez un paramètre contentWrapper à la cible au lieu de passer une fermeture Widget Function(BuildContext, Widget) au site d'appel. La fermeture est un _buildFoo en déguisement et manque silencieusement les nouveaux slots ajoutés plus tard.
```
// Mauvais — fermeture builder baked dans le site d'appel.
showMySheet(context, builder: (ctx, child) => BlocProvider<MyBloc>(
  create: (_) => MyBloc(), child: child));

// Bon — la cible expose contentWrapper ; le provider couvre chaque slot.
showMySheet(context,
  contentWrapper: (ctx, child) => BlocProvider<MyBloc>(
    create: (_) => MyBloc(), child: child));
```

State Management

Bloc limité à la modal instancié au site d'appel avec try/finally close(). Placez le BlocProvider à l'intérieur de la subtree de la modal (via contentWrapper ou équivalent). BlocProvider gère la fermeture automatiquement au démontage — toute route qui pop via un chemin autre que await retour fuite silencieusement le bloc dans le pattern try/finally.

// Mauvais — cycle de vie géré au site d'appel.
final bloc = CommentsBloc()..add(const CommentsLoadRequested());
try {
  await showSheet(title: BlocProvider.value(value: bloc, child: _Title()));
} finally {
  await bloc.close();
}

// Bon — BlocProvider possède la fermeture au démontage.
return showSheet(
  contentWrapper: (ctx, child) => BlocProvider<CommentsBloc>(
    create: (_) => CommentsBloc()..add(const CommentsLoadRequested()),
    child: child),
  title: _Title());

Commentaires

Commentaires multi-lignes de rationale de conception pour le bien de Codex. Les commentaires inline de longueur paragraphe dérivent au moment où le code change, et les LLMs les citent comme autoritaires même quand obsolètes. Si l'explication est plus d'une phrase, déplacez-la dans la description de la PR ou un fichier de règles et laissez un pointeur // See PR #N inline — pas le paragraphe entier.

// Mauvais — paragraphe au-dessus de ClipRRect qui dérirera silencieusement.
// Le shell extérieur utilise des coins de 30 px en bas et le conteneur de tabs interne
// utilise des coins de 32 px en haut pour que la surface interne soit visiblement imbriquée ...
ClipRRect(borderRadius: BorderRadius.vertical(bottom: Radius.circular(30)));

// Bon — la rationale vit dans la constante nommée.
// Le rayon interne est 2 px plus grand pour que le conteneur de tabs soit visiblement imbriqué.
ClipRRect(borderRadius: BorderRadius.vertical(
  bottom: Radius.circular(VineTheme.shellCornerRadius)));

UI / Animations

ValueKey redondant sur les branches AnimatedSwitcher qui sont déjà de types runtime différents. AnimatedSwitcher compare runtimeType + key. Les branches qui retournent déjà des types de widget différents (Center, IgnorePointer, SizedBox, …) sont déjà distinguables — ajouter ValueKey est redondant et risque des assertions « duplicate GlobalKey » quand le widget est réutilisé. Ajoutez un ValueKey seulement quand (a) deux branches sont du même type runtime, ou (b) un test s'ancre sur la clé via find.byKey(...) (laissez un commentaire le disant).
Timer (ou Future.delayed) pour le timing d'UI. Timer continue de tirer après que le widget soit disposé sauf s'il est manuellement annulé ; il ne met pas en pause avec la route ; il ne respecte pas MediaQuery.disableAnimations ; et il peut envoyer un setState mid-frame. Utilisez AnimationController + FadeTransition / AnimatedBuilder pour les flashs transitoires, badge pulses, ou overlays de type snackbar. N'atteignez Timer que quand l'effet est genuinely hors du pipeline de rendu (par exemple, network debounce, analytics delay).
```
// Bon
late final AnimationController _feedbackController = AnimationController(
  vsync: this, duration: const Duration(milliseconds: 550));
void _triggerFeedback() => _feedbackController.forward(from: 0);
// In build:
FadeTransition(
  opacity: Tween(begin: 1.0, end: 0.0).animate(_feedbackController),
  child: const FeedbackIcon());
```

Tests

Bornes absolues de wall-clock timing (<100 ns, <100 ms). Les runners CI partagés sont bruyants et substantially plus lents que les laptops de dev — ceux-ci flakent. Utilisez une comparaison relative (expect(fastMs, lessThan(slowMs))), enveloppez dans fakeAsync pour les vérifications de durée logique, ou skippez avec skip: true + un commentaire TODO(any):. Ne relevez jamais le seuil — cela juste retarde le prochain flake.
expect(tester.takeException(), isNotNull) comme signal d'effet secondaire. very_good test --optimization fusionne les fichiers de test ; l'état Riverpod keepAlive fuitée d'un test antérieur peut silencieusement résoudre la dépendance, supprimant l'exception. Affirmez seulement le contrat nommé du test ; videz les erreurs incidentelles sans assertion :
```
tester.takeException(); // drain erreur incidentelle — pas d'assertion
```
tester.tapAt(Offset(...)) pour l'interaction modale. Les taps basés sur coordonnées sont sensibles au flag expand de la modal : DraggableScrollableSheet(expand: false) a un espace vide transparent au-dessus de son contenu, tandis que expand: true remplit ce rectangle avec le hit-testing de la sheet. Préférez find.text(...) / find.byType(...) / find.bySemanticsIdentifier(...). Quand tapAt est inévitable, documentez l'hypothèse de layout inline et gardez l'offset bien à l'intérieur de la région visée.
DateTime.now() dans le code comparé contre tester.pump(Duration). L'horloge de test Flutter avance via pump ; DateTime.now() lit l'horloge murale hôte — elles ne s'accordent pas. Utilisez clock.now() de package:clock dans le code de production et withClock(Clock(() => now), () async { ... }) dans le test pour que les deux horloges avancent ensemble. (clock est déjà une dépendance transitive — aucun changement pubspec.yaml nécessaire.)
```
// Code sous test :
import 'package:clock/clock.dart';
if (clock.now().difference(_pausedAt!) >= _minPauseForFeedback) {
  _triggerUnpauseFeedback();
}

// Test :
var now = DateTime(2026);
await withClock(Clock(() => now), () async {
  now = now.add(const Duration(milliseconds: 220));
  await tester.pump(const Duration(milliseconds: 220));
  expect(find.byType(UnpauseFeedback), findsOneWidget);
});
```

Cohérence des Tests

Si le code de production a changé, vérifiez que les tests correspondants existent et matchent toujours
Cherchez les assertions de test qui référencent des champs ou méthodes supprimés
Vérifiez que les setups de mock matchent les signatures de méthodes actuelles

Étape 4 : Exécuter l'analyseur et les tests

Exécutez mcp__dart__analyze_files sur tous les fichiers modifiés
Exécutez mcp__dart__run_tests sur tous les fichiers de test modifiés
Rapportez tout échec

Étape 5 : Rapporter les résultats

Présentez les résultats groupés par sévérité :

## Résumé de la Révision

### Doit Être Corrigé
- [file.dart:42](path/to/file.dart#L42) - Description du problème

### Devrait Être Corrigé
- [file.dart:15](path/to/file.dart#L15) - Description du problème

### Détail
- [file.dart:7](path/to/file.dart#L7) - Description du problème

### Tout Est Bon
Si aucun problème trouvé, confirmez : "Aucun problème trouvé. Prêt à pousser."

Définitions de sévérité :

Doit Être Corrigé : Causera des bugs, échecs de tests, ou échecs de CI
Devrait Être Corrigé : Viole les règles du projet, code mort, commentaires obsolètes
Détail : Préférences de style, améliorations mineures

Après rapportage, corrigez automatiquement tous les éléments "Doit Être Corrigé" et "Devrait Être Corrigé". Demandez la permission à l'utilisateur avant de corriger les éléments "Détail".

review-before-commit