Skill d'examen du code
Objectif
Examen de code final de tous les changements non commités avant de pousser. Capture les problèmes qui sont 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 examiner
Étape 1 : Identifier les fichiers modifiés
Exécute 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, filtre pour ne garder que les fichiers sous ce chemin.
Étape 2 : Lire tous les fichiers modifiés
Lis chaque fichier modifié en entier. Pour chaque fichier, vérifie les éléments ci-dessous.
Étape 3 : Exécuter les vérifications
Pour chaque fichier modifié, vérifie :
Code mort
- Imports inutilisés (import non référencé nulle part dans le fichier)
- Champs privés, méthodes ou getters inutilisés
- Code inaccessible après des retours précoces
- Blocs de code commentés (doivent être supprimés, pas 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 (utilise 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 de réflexion
Tout fichier sous mobile/docs/brainstorm/ (généralement datés comme YYYY-MM-DD-issueNNNN-…-brainstorm.md) est un artefact de travail, pas un livrable. Au moment où la PR est prête, la justification doit figurer dans la description de la PR ; le fichier ne doit pas être livré avec le commit de fusion.
Signale chaque fichier staged sous ce chemin et demande à l'utilisateur si le document de réflexion doit être supprimé ou converti sur place :
- Le supprimer (par défaut) —
git rmle fichier. Utilise un commitdocs: drop … brainstorm docséparé pour que la suppression soit auditable. - Le convertir en décision record — uniquement quand le doc a une valeur durable au-delà de la PR (p.ex. capture une approche rejetée que les lecteurs futurs proposeront continuellement). Renomme, déplace hors de
brainstorm/, et réduis aux parties durables. Précédent :cd723075e docs(notifications): convert badge-desync brainstorm to decision record.
Précédents pour suppression pure et simple : 380bf50c1 docs: drop PR #4229 brainstorm doc, et le suivi de l'examen de la PR #4234.
Violations des règles CLAUDE.md
Lis et applique TOUTES les règles de .claude/CLAUDE.md et .claude/rules/. Ne les code pas en dur ici — consulte toujours la source de vérité dans ces fichiers.
Patterns Flutter — Découvertes d'examen connues
Ces patterns ont émergé des cycles d'examen précédents sur ce repo et ne figurent pas dans les règles toujours chargées (pour garder la fenêtre de contexte global allégée). Vérifie-les explicitement lors de l'examen du code.
Design System
-
Widget sur mesure s'écarte de
divine_uisans note de docstring. Tout widget qui s'écarte délibérément d'un composantdivine_ui(taille différente, structure différente, variante contournée) doit dire pourquoi dans sa docstring de classe — quel composant du design system s'en rapproche et qu'est-ce qui a forcé la divergence. Sans cela, les relecteurs relèveront « pourquoi ne pas simplement utiliserDivineIconButton? »// Bon /// Visuellement équivalent à [DivineIconButton] en style ghost mais dimensionné 64×64 /// avec une icône 32 px au lieu des présets 40×40/56×56 de DivineIconButton, /// car le spec Figma (node 15314:53971) demande une cible tactile plus grande /// que n'importe quelle taille standard de DivineIconButton. class CenterPlaybackControl extends StatelessWidget { ... }
Widget API Design
-
Paramètres spéculatifs sur les widgets réutilisables. Avant d'ajouter un paramètre à quoi que ce soit dans
divine_uioulib/utils/, confirme que la branche qu'il déverrouille est accessible depuis au moins un appelant. Un paramètreBuilder/Callback/Resolveravec exactement un appelant qui le fournit toujours est une branche morte. Supprime-la — l'ajouter à nouveau plus tard est bon marché ; les branches mortes confondent les lecteurs et gonflent la surface API.// Mauvais — initialChildSizeBuilder n'est appelé que depuis une surface où // le clavier n'est jamais ouvert, donc la branche est 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 unique. Quand un site d'appel doit mettre un
BlocProvider/InheritedWidgetau-dessus de chaque slot d'une modal/sheet/route, ajoute un paramètrecontentWrapperà la cible au lieu de passer une fermetureWidget Function(BuildContext, Widget)au site d'appel. La fermeture est un_buildFooen disguise et manque silencieusement les nouveaux slots ajoutés plus tard.// Mauvais — fermeture builder cuite au 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(). Mets leBlocProviderà l'intérieur du sous-arbre de la modal (viacontentWrapperou équivalent).BlocProvidergère la fermeture automatiquement au démontage — toute route qui dépile via un chemin autre que le retour d'awaitfuit silencieusement le bloc dans le patterntry/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 de rationale de conception multilignes pour le bénéfice de Claude. Les commentaires inline de longueur paragraphe dérivent dès que le code change, et les LLMs les citent comme faisant autorité même quand ils sont obsolètes. Si l'explication dépasse une phrase, déplace-la vers la description de la PR ou un fichier de règles et laisse un pointeur
// See PR #Ninline — pas le paragraphe complet.// Mauvais — paragraphe au-dessus de ClipRRect qui dérivera silencieusement. // La coquille extérieure utilise des coins inférieurs 30 px et le conteneur // onglets interne utilise des coins supérieurs 32 px pour que la surface // interne s'imbrique visiblement ... 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 onglets s'imbriques visiblement. ClipRRect(borderRadius: BorderRadius.vertical( bottom: Radius.circular(VineTheme.shellCornerRadius)));
UI / Animations
-
ValueKeyredondant sur les branchesAnimatedSwitcherqui sont déjà de types runtime différents.AnimatedSwitchercompareruntimeType + key. Les branches qui retournent déjà des types de widget différents (Center,IgnorePointer,SizedBox, …) sont déjà distinguables — ajouterValueKeyest redondant et risque les assertions « duplicate GlobalKey » quand le widget est réutilisé. Ajoute unValueKeyuniquement quand (a) deux branches sont du même type runtime, ou (b) un test s'ancre sur la clé viafind.byKey(...)(laisse un commentaire le disant). -
Timer(ouFuture.delayed) pour les timages UI.Timercontinue de s'exécuter après le démontage du widget sauf annulation manuelle ; il ne s'interrompt pas avec la route ; il ne respecte pasMediaQuery.disableAnimations; et peut déclencher unsetStateen milieu de frame. UtiliseAnimationController+FadeTransition/AnimatedBuilderpour les flashes transitoires, pulsations de badge ou overlays style snackbar. AtteinsTimeruniquement quand l'effet est véritablement hors du pipeline de rendu (p.ex. debounce réseau, délai analytique).// 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 temps mur (
<100 ns,<100 ms). Les runners CI partagés sont bruyants et substantiellement plus lents que les laptops de dev — ceux-ci deviennent flaky. Utilise une comparaison relative (expect(fastMs, lessThan(slowMs))), enveloppe dansfakeAsyncpour les vérifications de durée logique, ou skip avecskip: true+ un commentaireTODO(any):. Ne relève jamais le seuil — cela retarde juste le prochain flake. -
expect(tester.takeException(), isNotNull)comme signal d'effet de bord.very_good test --optimizationfusionne les fichiers de test ; l'étatkeepAliveRiverpod fuyant d'un test antérieur peut silencieusement résoudre la dépendance, supprimant l'exception. Affirme uniquement le contrat nommé du test ; vide les erreurs incidentes sans assertion :tester.takeException(); // vide erreur incidente — pas d'assertion -
tester.tapAt(Offset(...))pour interaction modale. Les taps basés sur les coordonnées sont sensibles au flagexpandde la modal :DraggableScrollableSheet(expand: false)a un espace vide transparent au-dessus de son contenu, tandis queexpand: trueremplit ce rectangle avec le hit-testing de la sheet. Préfèrefind.text(...)/find.byType(...)/find.bySemanticsIdentifier(...). QuandtapAtest inévitable, documente l'hypothèse de layout inline et garde l'offset bien à l'intérieur de la région prévue. -
DateTime.now()dans le code comparé àtester.pump(Duration). L'horloge du test Flutter avance viapump;DateTime.now()lit l'horloge murale du host — ils ne concordent pas. Utiliseclock.now()depackage:clockdans le code de production etwithClock(Clock(() => now), () async { ... })dans le test pour que les deux horloges avancent ensemble. (clockest déjà une dépendance transitive — aucun changementpubspec.yamlnécessaire.)// Code 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); });
Test Consistency
- Si le code de production a changé, vérifie que les tests correspondants existent et correspondent toujours
- Vérifie les assertions de test qui référencent des champs ou méthodes supprimés
- Vérifie que les configurations de mock correspondent aux signatures de méthode actuelles
Étape 4 : Exécuter l'analyseur et les tests
- Exécute
mcp__dart__analyze_filessur tous les fichiers modifiés - Exécute
mcp__dart__run_testssur tous les fichiers de test modifiés - Signale les défaillances
Étape 5 : Rapporter les découvertes
Présente les découvertes groupées par sévérité :
## Résumé de l'examen
### À corriger obligatoirement
- [file.dart:42](path/to/file.dart#L42) - Description du problème
### À corriger de préférence
- [file.dart:15](path/to/file.dart#L15) - Description du problème
### Détail mineur
- [file.dart:7](path/to/file.dart#L7) - Description du problème
### Aucun problème
Si aucun problème trouvé, confirme : « Aucun problème trouvé. Prêt à pousser. »
Définitions de sévérité :
- À corriger obligatoirement : Causera des bugs, défaillances de test ou défaillances CI
- À corriger de préférence : Viole les règles du projet, code mort, commentaires obsolètes
- Détail mineur : Préférences de style, améliorations mineures
Après signalement, corrige automatiquement tous les éléments « À corriger obligatoirement » et « À corriger de préférence ». Demande à l'utilisateur avant de corriger les éléments « Détail mineur ».