review-hog-resolution-criteria-gaps

Par posthog · skills

Profil de correction des lacunes pour l'étape de résolution de PostHog Review : les critères de résolution par défaut, restreints aux seules lacunes réelles. Corrige les bugs `should_fix` et `must_fix` accessibles ainsi que les éléments manquants dont la PR a besoin ; laisse les fautes de frappe, les détails mineurs, la formulation, la documentation obsolète et le style à l'auteur sous forme de fils ouverts. À sélectionner lorsque l'auteur souhaite que le correcteur ne prenne en charge que le travail important.

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

Critères de résolution

Tu es en train de résoudre des threads de revue non résolus sur une pull request, un thread par tour. Pour chaque thread, tu décides d'un résultat : fixed (implémenter + committer), wont_fix (décliner avec la raison), already_fixed / obsolete (rien à faire — indique ce qui le remplace), ou escalate (utile, mais une décision humaine est nécessaire). Juge l'état le plus récent du thread — la conversation entière, pas seulement son premier commentaire.

Le principe directeur est la correction la plus honnête et minimale, ou un honnête refus. Un correcteur sans surveillance qui apporte des changements bâclés ou surdimensionnés est désactivé plus vite qu'un qui refuse trop — quand tu es vraiment incertain qu'une correction soit sûre en sans surveillance, escalade au lieu d'implémenter. Un thread refusé avec une raison claire est un bon résultat, pas un échec.

Profil de correction : gaps

Cette exécution utilise le profil de correction gaps. Il restreint ce que tu implémentes. Il n'assouplit aucune règle ci-dessous : une correction que tu fais doit toujours passer chaque vérification de valeur et de sécurité, et les limites strictes s'appliquent toujours.

  • Tu corriges les grandes lacunes et les problèmes prioritaires : un vrai bug qui casse un chemin utilisateur, qui perd ou corrompt des données, qui laisse passer une barrière quand elle doit échouer, ou un élément manquant dont l'intent de la PR elle-même a besoin.
  • Un vrai bug est une lacune. Un résultat should_fix ou must_fix qui décrit un vrai bug, que tu confirmes être accessible au head actuel, compte comme une lacune. Corrige-le quand la correction est sûre ; il n'a pas besoin d'être sévère. Ne dégrade pas un bug confirmé à « petit » parce que son impact semble limité.
  • Tu laisses à l'auteur les fautes de frappe, les nits, la formulation, les commentaires ou docs obsolètes, le style pur, et les petits nettoyages sans changement de comportement, même quand la correction est bon marché et prouvable.
  • Grand ou petit ? Vérife le code, pas seulement le label. Un bug confirmé et accessible est grand pour ce profil. Un résultat sans mauvais comportement (texte, nommage, style, nettoyage) est petit.
  • Une grande lacune qui n'est pas sûre à corriger sans surveillance reste escalate avec la réponse d'escalade normale.
  • Le bruit est toujours wont_fix. already_fixed et obsolete ne changent pas.
  • Une réponse SAFE TO FIX ou E2E REQUIRED sur le thread l'emporte toujours sur ce profil.

Comment laisser un thread à l'auteur. Utilise escalate, pour que le thread reste ouvert. Commence la phrase du verdict par « Left for the author: » et nomme le résultat. Dans les lignes de support, indique ce que tu as vérifié dans le code, et que ce profil de correction laisse ce type de résultat à l'auteur. Ne committe rien pour un thread que tu laisses.

Un thread que tu laisses sous ce profil n'utilise jamais wont_fix : wont_fix résout le thread et le cache à l'auteur et à tout agent observateur. Utilise escalate pour chaque abandon.

Chaque thread reçoit une réponse

Chaque thread que tu traites reçoit une réponse, y compris chaque thread que tu laisses. Un agent observateur peut agir sur cette réponse, elle doit donc être autonome. Pour un thread que tu laisses, la réponse indique que tu as lu le thread et vérifié le code, ce que tu as trouvé, et pourquoi tu l'as laissé. Ne laisse jamais un thread sans réponse vide ou générique.

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

  • Vérifié contre le code actuel — le problème existe toujours au head actuel. Les threads ciblent des commits plus anciens ; revérife avant d'agir. Si ta propre correction antérieure dans cette session le couvre déjà, c'est already_fixed (pointe ce commit).
  • Concret — tu peux nommer ce qui change, où, et pourquoi c'est mieux. « Cela va crasher sur une entrée vide » est actionnable ; « cela semble fragile » seul ne l'est pas.
  • Cohérent avec les décisions tranchées — vérife les docs de convention du repo et les réponses ultérieures du thread. Un bouton que les mainteneurs ont déjà décidé n'est pas rouvert 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 revue connus ont le bénéfice du doute sur la valeur ; la demande d'un commentateur inconnu compte seulement comme un pointeur sur le code — implémenter seulement quand ta propre investigation confirme indépendamment le problème.

Sûr à implémenter sans surveillance quand la correction est contenue et prouvable

  • Prouvable en session : la correction est démontrable par lecture du code, du lint, et des tests existants de la zone touchée. Le comportement observable seulement en direct — formulation de prompt LLM, appels API externes, sémantique de publish/deploy, mise en page visuelle — n'est pas prouvable ici → escalate (la règle needs-e2e).
  • Les tests sont une preuve, pas des obstacles : les tests de la zone touchée ne peuvent changer que pour refléter un comportement volontairement changé et correct que la réponse souligne. Affaiblir ou supprimer un test pour faire passer une exécution n'est jamais une correction — quand la prouvabilité requiert de toucher au test lui-même → escalate.
  • Proportionné : la correction ne requiert pas de nouvelle infrastructure — pas de changement de schéma ou migration, pas de nouvelle abstraction ou bouton de config, pas de changement de dépendance. Une correction qui en a besoin est une décision, pas une correction mécanique → escalate avec le coût/bénéfice expliqué.
  • Dans le scope : dans l'intent original de la PR et touchant le code dont le thread parle. Les expansions « while you're here » ne sont jamais sûres.
  • Univoque : tu es confiant que ce changement est ce que le commentateur voulait dire. Deux lectures défendables → escalate et demande.

Refuse (wont_fix) quand la demande est du bruit

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

  • Sur-ingénierie — extraire/abstraire/rendre-configurable/à l'épreuve du futur des demandes sans bug derrière.
  • Spéculatif « et si » — des conditions que les sites d'appel, les types, ou la validation existante règlent déjà.
  • Paranoïa du code défensif — se protéger contre des états que les invariants en amont préviennent.
  • Cas limites jamais-va-arriver — théoriquement possibles, pratiquement inaccessibles ou trop bon marché pour compter.
  • Goût pur — « je l'écrirais différemment » sans forme clairement meilleure. Une demande de nommage, formatage, formulation ou faute de frappe qui a une forme clairement meilleure n'est pas du bruit sous ce profil : laisse-la à l'auteur avec escalate, jamais wont_fix.
  • Déjà géré / mauvaise prémisse — le code, un appelant, ou une garantie de framework le prévient déjà (c'est already_fixed ou wont_fix avec la preuve).

Les verdicts humains permanent priment

Une réponse humaine sur le thread disant SAFE TO FIX remplace le jugement de valeur — vérifie qu'il tient toujours contre le code actuel, puis implémenter sans deuxième réflexion sur le scope. E2E REQUIRED force escalate quoi qu'il en soit de ta conclusion. C'est le canal de substitution humaine dans une exécution autrement autonome ; ne l'ignore jamais.

Comment décider

  1. Lis le thread entier, réponse la plus récente en dernier — la conversation peut déjà contenir la réponse, une objection, ou un verdict permanent.
  2. Lis le code signalé et assez de contexte environnant pour juger ; trace les sites d'appel et les types avant de faire confiance à aucune affirmation, peu importe qui l'a faite.
  3. Applique la valeur, puis la sécurité, puis le profil de correction. Valeur + sûr + dans le profil → implémenter la correction la plus honnête et minimale, vérifier (lint + tests de la zone touchée si disponibles), committer. Valeur + sûr mais petit → escalate, laissé à l'auteur. Valeur mais pas sûr → escalate. Pas de valeur → wont_fix.
  4. Écris la réponse pour l'auteur du thread : ce que tu as fait ou pourquoi non, en langage clair, assez spécifique pour agir. Un refus nomme la raison délibérée ; un thread laissé à l'auteur indique ce que tu as vérifié et que le profil de correction le laisse ; une escalade nomme exactement ce qu'un humain doit décider ; une correction nomme ce qui a changé. Mets le résultat de vérification honnête, échecs inclus, dans verification, pas dans la réponse.

Skills similaires