diff --git a/scripts/check_unaddressed_nits.py b/scripts/check_unaddressed_nits.py index 97ea9d1df71..2f33fa35da5 100644 --- a/scripts/check_unaddressed_nits.py +++ b/scripts/check_unaddressed_nits.py @@ -206,7 +206,14 @@ # effet quand la construction est conditionnelle. CONDITIONAL_LIFT = re.compile( r"(et je merge|puis je merge|ensuite je merge" - r"|je merge (?:dès|des|une fois|quand|après|apres|si\b))", re.I) + r"|je merge (?:dès|des|une fois|quand|après|apres|si\b)" + # #12319 — miroir exact des constructions « je merge » pour « je leve » : + # « corrige X et je leve » est l'enonce de la condition, pas une levee + # acquise. Le commentaire de LIFT_MARKERS le revendiquait deja ; le regex + # ne couvrait que « je merge » — mesure par le test de regression + # test_12319_levee_conditionnelle_ne_leve_pas_nit_commentaire. + r"|et je l[èe]ve|puis je l[èe]ve|ensuite je l[èe]ve" + r"|je l[èe]ve (?:dès|des|une fois|quand|après|apres|si\b))", re.I) # #12074 — marqueurs de levee EXPLICITE par son auteur : quand l'un d'eux # PRECEDE le match CONDITIONAL_LIFT, la construction n'est pas une condition — @@ -800,19 +807,6 @@ def analyse(pr_data: dict, threads: list[dict], cutoff: datetime) -> dict: # ou reponse — n'eteint pas une reserve posee par un autre). pr_author = (pr_data.get("author") or {}).get("login", "") - # Seuls les commentaires capables de LEVER comptent (cf can_lift) : un - # commentaire de bot CI ou un tag de protocole nu n'a jamais repondu a rien. - # On porte l'AUTEUR de chaque evenement de levee : la borne #11145 en a - # besoin (auteur seul = les temps plats de #11222 ne suffisaient pas). - # Le body est porte pour la trappe OVERRIDE de `_lift_eligible` (#11639) : - # elle doit voir le marqueur `[OVERRIDE] lane …` de la levee, pas - # seulement son auteur. - lift_events = [ - (ts(c["createdAt"]), (c.get("author") or {}).get("login", ""), - c.get("body", "") or "") - for c in (pr_data.get("comments") or []) if can_lift(c) - ] - # Fenetre 05-29..06-04 (#1958) : la RE-REVIEW APPROVED. Le reviewer qui # revient approuver apres sa demande de changements A dit que la reserve # est levee — le state GitHub natif porte plus de sens que le body (une @@ -828,8 +822,6 @@ def analyse(pr_data: dict, threads: list[dict], cutoff: datetime) -> dict: and (r.get("author") or {}).get("login", "") not in BOT_LOGINS ] approved_rereviews = [x for x in approved_rereviews if x[0] is not None] - lift_events += approved_rereviews - lift_events = [x for x in lift_events if x[0] is not None] def _lift_eligible(lift_author: str, nit_author: str, lift_body: str = "") -> bool: @@ -849,6 +841,14 @@ def _lift_eligible(lift_author: str, nit_author: str, # conditionnel) dans un commentaire qui peut lever reste une levee pour # l'etat de review — c'est la reponse ecrite que B.0 exige. On garde # l'auteur pour la branche dedicated ci-dessous. + # #12319 : explicit_lifts est desormais LE regime des nits portes par un + # COMMENTAIRE ou une review COMMENTED aussi — l'ancienne branche elif + # consultait lift_events (tout commentaire can_lift), et la borne d'auteur + # #11145 est vacue sur ce depot : Hermès poste sous jsboige (self-review + # cap) et la lane pousse sous jsboige, donc nit_author == pr_author == + # "jsboige" sur presque chaque PR — n'importe quel commentaire posterieur + # de la lane eteignait la reserve de son propre reviewer. B.0 : ce qui + # leve une remarque est une PHRASE, pas un commentaire de protocole. explicit_lifts = [ (ts(c["createdAt"]), (c.get("author") or {}).get("login", ""), c.get("body", "")) @@ -922,11 +922,23 @@ def _lift_eligible(lift_author: str, nit_author: str, ) if lifted: continue - elif any( - when < t < cutoff and _lift_eligible(lift_author, login, lift_body) - for (t, lift_author, lift_body) in lift_events - ): - continue # reponse de l'auteur du nit ou de l'auteur de la PR + # #12319 : meme regime pour un nit porte par un commentaire ou une + # review COMMENTED (dont chaque reserve Hermes, self-review cap). + # Avant : n'importe quel commentaire posterieur de la lane levait + # (nit_author == pr_author == "jsboige" flotte-wide rendait la borne + # #11145 vacue — un [READY-FOR-MERGE SELF attestation] qui ne nomme + # pas la reserve l'eteignait). Apres : une PHRASE de levee + # (LIFT_MARKER, borne d'auteur #11145 preservee via _lift_eligible) + # OU une re-review APPROVED de l'auteur de la reserve (etat natif + # GitHub, phrase de levee au sens fort). + elif (any( + when < t < cutoff and _lift_eligible(lift_author, login, lift_body) + for (t, lift_author, lift_body) in explicit_lifts + ) or any( + when < t < cutoff and author == login + for (t, author, _) in approved_rereviews + )): + continue # Un commit poussé après le nit ne le lève PAS à lui seul : sur #10761, # le « traitement » était un rebase à 19:41 qui n'adressait aucun des # deux nits de 11:07. Le push est reporté comme contexte, pas comme levée diff --git a/scripts/tests/test_check_unaddressed_nits.py b/scripts/tests/test_check_unaddressed_nits.py index 6394794fcc7..13f969a6342 100644 --- a/scripts/tests/test_check_unaddressed_nits.py +++ b/scripts/tests/test_check_unaddressed_nits.py @@ -97,8 +97,12 @@ def test_le_bruit_ne_leve_pas_un_nit(noise): def test_vraie_reponse_humaine_leve(): + """#12319 : la vraie reponse qui LEVE porte une phrase de levee (B.0 : + ce qui leve une remarque est une phrase) — une reponse substantive sans + marqueur ne leve plus (cf test_12319_reponse_nue_ne_leve_plus).""" reply = {"author": {"login": "jsboige"}, "createdAt": at(12), - "body": "Bien vu, l'attribution est corrigee en cellule 0 et 2."} + "body": "Bien vu, l'attribution est corrigee en cellule 0 et 2 — " + "les 2 nits sont adresses."} assert run([USER_NIT, reply])["blocked"] is False @@ -813,11 +817,12 @@ def test_cr_levee_conditionnelle_ne_leve_pas(): def test_nit_commentaire_garde_le_regime_general(): - """Non-regression : le durcissement ne touche QUE l'etat de review. Un nit - porte par un COMMENTAIRE reste leve par une reponse humaine (regime - general, limite NLP de can_lift).""" + """#12319 : le regime general d'un nit porte par un COMMENTAIRE est + aligne sur celui de l'etat de review — il faut une PHRASE de levee + (LIFT_MARKER) ou une re-review APPROVED de l'auteur. Une reponse humaine + PORTANT la phrase leve toujours (regime general, limite NLP de can_lift).""" reply = {"author": {"login": "jsboige"}, "createdAt": at(12), - "body": "Bien vu, corrige."} + "body": "Bien vu, corrige — les 2 nits sont levées."} assert run([USER_NIT, reply])["blocked"] is False @@ -842,6 +847,64 @@ def test_bystander_commentaire_ne_leve_pas_un_nit(): assert run([USER_NIT, bystander])["blocked"] is True +# --- #12319 : la collision d'identite flotte-wide. Hermès poste sous jsboige +# (self-review cap) et chaque lane pousse sous jsboige, donc +# nit_author == pr_author == "jsboige" sur presque chaque PR du depot : la +# borne d'auteur #11145 ne borne plus rien, et l'ancienne branche elif +# (tout commentaire can_lift leve) laissait une lane eteindre la reserve de +# son propre reviewer en postant une attestation de protocole. Le regime est +# desormais : PHRASE de levee (LIFT_MARKER, borne d'auteur preservee) OU +# re-review APPROVED de l'auteur de la reserve. + +def test_12319_reponse_nue_ne_leve_plus(): + """Le defaut fondateur : USER_NIT (auteur jsboige) sur une PR d'auteur + jsboige — une reponse posterieure de jsboige SANS phrase de levee + n'eteint plus la reserve (avant : levait via lift_events).""" + reply = {"author": {"login": "jsboige"}, "createdAt": at(12), + "body": "Bien vu, l'attribution est corrigee en cellule 0 et 2."} + assert run([USER_NIT, reply])["blocked"] is True + + +def test_12319_attestation_self_ne_leve_pas(): + """Le litteral de l'issue : une lane eteint la reserve de son reviewer en + postant un [READY-FOR-MERGE SELF attestation] qui ne la mentionne pas.""" + attestation = {"author": {"login": "jsboige"}, "createdAt": at(12), + "body": "[READY-FOR-MERGE SELF attestation] checks verts, " + "body complet, preuve d'execution dans le body."} + assert run([USER_NIT, attestation])["blocked"] is True + + +def test_12319_reponse_nue_sur_reserve_hermes_ne_leve_plus(): + """Meme regime pour la reserve Hermes (review COMMENTED, src non-CR) : + la lane pousse une reponse sans phrase de levee — reste bloquante.""" + reply = {"author": {"login": "jsboige"}, "createdAt": at(12), + "body": "Les 2 cellules d'interp sont ajoutees, commit abc."} + assert run([HERMES_NIT, reply])["blocked"] is True + + +def test_12319_rereview_approved_de_l_auteur_leve_nit_commentaire(): + """L'etat natif garde son role de phrase de levee au sens fort : Hermes + (auteur de la reserve) revient APPROVED — la reserve s'eteint, meme sans + marqueur textuel.""" + data = { + "number": 0, "title": "t", "author": {"login": "jsboige"}, + "comments": [HERMES_NIT], + "reviews": [{"author": {"login": "clusterManager-Myia"}, + "state": "APPROVED", "submittedAt": at(15), + "body": ""}], + "commits": [{"committedDate": at(19)}], + } + assert mod.analyse(data, [], MERGED)["blocked"] is False + + +def test_12319_levee_conditionnelle_ne_leve_pas_nit_commentaire(): + """#11201 preserve sur le nouveau chemin : « corrige X et je leve » est + l'enonce de la condition, pas une levee acquise.""" + cond = {"author": {"login": "jsboige"}, "createdAt": at(12), + "body": "Corrige la cellule 19 et je leve ma reserve."} + assert run([USER_NIT, cond])["blocked"] is True + + def test_bystander_approved_ne_leve_pas_un_nit(): """La classe mesuree #11494 (4 cas Hermes) : un APPROVED d'un reviewer tiers n'eteint pas une reserve posee par un autre.""" @@ -857,18 +920,22 @@ def test_bystander_approved_ne_leve_pas_un_nit(): def test_auteur_du_nit_leve_son_nit(): - """SELF (59,5 %) : l'auteur de la reserve revient repondre — le nit se - leve par le regime general borne (auteur == auteur du nit).""" + """SELF (59,5 %) : l'auteur de la reserve revient repondre AVEC une + phrase de levee — le nit se leve par le regime general borne + (auteur == auteur du nit, #12319 : phrase exigee).""" reply = {"author": {"login": "clusterManager-Myia"}, "createdAt": at(12), - "body": "Les 2 cellules d'interp sont ajoutees, commit abc."} + "body": "Les 2 cellules d'interp sont ajoutees, commit abc — " + "reserve levee."} assert run([HERMES_NIT, reply])["blocked"] is False def test_auteur_pr_repond_a_son_reviewer_leve(): - """PR_AUTHOR (16,2 %) : le worker (auteur de la PR) repond au reviewer — - le flux sain que la borne preserve.""" + """PR_AUTHOR (16,2 %) : le worker (auteur de la PR) repond au reviewer + AVEC une phrase de levee — le flux sain que la borne preserve + (#12319 : la phrase est desormais exigee, l'auteur seul ne suffit plus).""" reply = {"author": {"login": "jsboige"}, "createdAt": at(12), - "body": "Les 2 cellules d'interp sont ajoutees, commit abc."} + "body": "Les 2 cellules d'interp sont ajoutees, commit abc — " + "les points sont adresses."} assert run([HERMES_NIT, reply])["blocked"] is False