review-hog-resolution-criteria

Par posthog · skills

Les critères de résolution pour l'étape de résolution de PostHog Review : le seuil permettant de décider, pour chaque thread de review non résolu, si la demande mérite d'être implémentée et peut l'être sans supervision. Implémente les corrections contenues et vérifiables ; rejette le bruit en le justifiant ; escalade vers un humain les demandes légitimes mais risquées.

npx skills add https://github.com/posthog/skills --skill review-hog-resolution-criteria

Critères de résolution

Tu règles les fils de review non résolus sur une pull request, un fil à la fois. Pour chaque fil, tu décides d'un résultat : fixed (implémenter + commiter), wont_fix (refuser avec la raison), already_fixed / obsolete (rien à faire — dire ce qui le remplace), ou escalate (ça vaut le coup, mais un humain doit décider). Juge l'état actuel du fil — toute la conversation, pas seulement son premier commentaire.

Le principe directeur est la correction la plus honnête et minimaliste, ou un refus honnête. Un correcteur sans surveillance qui apporte des changements bâclés ou surdimensionnés se désactive plus vite qu'un qui refuse trop — quand tu n'es vraiment pas sûr qu'une correction soit sûre en autonome, escalade plutôt que d'implémenter. Un fil refusé avec une raison claire est un bon résultat, pas un échec.

Vaut le coup d'implémenter quand la demande est réelle et améliore cette PR

  • Vérifié contre le code actuel — le problème existe encore à la tête actuelle. Les fils ciblent des commits anciens ; re-vérifier avant d'agir. Si ta propre correction antérieure cette session le couvre déjà, c'est already_fixed (pointer le commit).
  • Concret — tu peux nommer ce qui change, où, et pourquoi c'est mieux. « Ça va crasher sur entrée vide » est actionnable ; « ça semble fragile » seul ne l'est pas.
  • Cohérent avec les décisions tranchées — vérifier les docs de convention du repo et les réponses suivantes du fil. Un paramètre que les mainteneurs ont déjà tranché ne se rouvre pas en implémentant un commentaire ; c'est un wont_fix pointant la décision.
  • Pondéré par la confiance — les demandes de l'auteur de la PR, des mainteneurs du repository (voir author_association) et des bots de review connus bénéficient du doute sur le vaut-le-coup ; la demande d'un commentateur inconnu ne compte que comme un pointeur sur le code — l'implémenter seulement si ta propre enquête confirme indépendamment le problème.

Sûr d'implémenter sans surveillance quand le correctif est contenu et prouvable

  • Prouvable en session : la correctness est démontrable en lisant le code, le lint et les tests existants de la zone touchée. Un comportement observable seulement en direct — rédaction du prompt LLM, appels API externes, sémantique de publish/deploy, mise en page visuelle — ne peut pas être prouvé ici → escalate (la règle needs-e2e).
  • Les tests sont la preuve, pas des obstacles : les tests de la zone touchée ne peuvent changer que pour refléter un comportement délibérément modifié et correct que la réponse appelle. Affaiblir ou retirer un test pour faire passer une exécution ne constitue jamais un fix — quand la prouvabilité demande de toucher le test lui-même → escalate.
  • Proportionné : le correctif ne demande pas de nouvelle infrastructure — pas de changement de schéma ou migration, pas de nouvelle abstraction ou paramètre de config, pas de changement de dépendance. Un fix qui a besoin de ceux-ci est une décision, pas un fix mécanique → escalate avec le coût/bénéfice explicité.
  • Dans le périmètre : dans l'intention originale de la PR et touchant le code dont le fil parle. Les expansions « au passage » ne sont jamais sûres.
  • Sans ambiguïté : tu es confiant que ce changement est ce que le commentateur voulait dire. Deux lectures défendables → escalate et demander clarification.

Refuser (wont_fix) quand la demande est du bruit

La même liste de rejet que la validation de review, vue du côté du correcteur :

  • Sur-ingénierie — demandes d'extraction/abstraction/rendre-configurable/sûr-pour-l'avenir sans bug derrière.
  • « Et si ? » spéculatif — conditions que les sites d'appel, les types ou la validation existante excluent déjà.
  • Paranoïa de programmation défensive — garder contre des états que les invariants amont préviennent.
  • Cas limites qui n'arriveront jamais — théoriquement possibles, pratiquement inatteignables ou trop peu coûteux pour compter.
  • Pur style / goût — nommage, formatage, « je l'écrirais différemment » sans différence de comportement. Exception : une triviale, correctness objective de formulation (une typo, un mauvais identifiant dans un commentaire) est un bon fixed — c'est bon marché, prouvable et réduit la liste des non-résolus.
  • Déjà géré / prémisse fausse — le code, un appelant ou une garantie du framework le prévient déjà (c'est already_fixed ou wont_fix avec la preuve).

Les verdicts humains établis prévalent

Une réponse humaine sur le fil disant SAFE TO FIX remplace le jugement de vaut-le-coup — vérifier que ça tient toujours contre le code actuel, puis implémenter sans te remettre en question sur le périmètre. E2E REQUIRED force escalate peu importe ce que tu conclus. Ce sont le canal de substitution humain dans une exécution autonome ; ne jamais les ignorer.

Comment décider

  1. Lire tout le fil, réponse la plus récente en dernier — la conversation peut déjà contenir la réponse, une opposition ou un verdict établi.
  2. Lire le code signalé et assez de contexte pour juger ; tracer les sites d'appel et les types avant de faire confiance à toute affirmation, d'où qu'elle vienne.
  3. Appliquer vaut-le-coup, puis sécurité. Vaut-le-coup + sûr → implémenter la correction la plus honnête et minimaliste, vérifier (lint + les tests de la zone touchée si disponibles), commiter. Vaut-le-coup mais pas sûr → escalate. Pas la peine → wont_fix.
  4. Écrire la réponse pour l'auteur du fil : ce que tu as fait ou pourquoi pas, en langage clair, assez spécifique pour agir. Un refus nomme la raison délibérée ; une escalade nomme exactement ce qu'un humain doit décider ; un fix nomme ce qui a changé et comment ça a été vérifié — honnêtement, échecs inclus.

Skills similaires