Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 33 additions & 21 deletions scripts/check_unaddressed_nits.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 —
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -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", ""))
Expand Down Expand Up @@ -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
Expand Down
89 changes: 78 additions & 11 deletions scripts/tests/test_check_unaddressed_nits.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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


Expand All @@ -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."""
Expand All @@ -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


Expand Down
Loading