diff --git a/scripts/coordination/merge_ready.py b/scripts/coordination/merge_ready.py index 3dac9959d6..c3391afba9 100644 --- a/scripts/coordination/merge_ready.py +++ b/scripts/coordination/merge_ready.py @@ -80,8 +80,15 @@ (``hold:``) AVANT tout appel gh. Fichier absent = aucune retenue ; fichier illisible ou ligne malformee -> exit 2 (on ne merge pas sans savoir ce qui est retenu). +- Review : la disposition de review est CLASSEE a la tete que la ligne + declare (``approved-exact-head`` / ``approval-not-on-head`` / + ``no-approval``, point 1 de #17672) depuis les ``reviews`` de la meme + vue -- l'oid de review y figure, aucun appel supplementaire. Le + dossier hache cet oid sans le comparer a la tete ; c'est cette + comparaison qui manquait. Elle informe, elle ne bloque pas : le + dossier READY a la tete exacte reste le contrat d'entree. - Journal : une ligne JSON par PR evaluee (ts UTC en Z, pr, head, - verdict, reason, merged) dans + verdict, reason, merged, review) dans ``%LOCALAPPDATA%/CoursIA/merge_ready/journal.jsonl`` (surchargeable ``--journal``), plus un resume humain sur stdout ; ``--json`` pour la sortie machine. @@ -128,6 +135,17 @@ frozen_umbrella_exclusion, ) +# Le canon d'emission du verdict de review vit dans scripts/ci/ : le jeton de +# review du cluster ne peut poster que des COMMENT (#16926), donc le verdict +# reel s'ecrit ``VERDICT: `` dans le CORPS de la voix. Importe, jamais +# recopie -- une regex locale divergerait en silence du canon qui gouverne le +# triage du pool. +CI_DIR = SCRIPTS_DIR / "ci" +if str(CI_DIR) not in sys.path: + sys.path.insert(0, str(CI_DIR)) + +import pool_review_verdicts as review_canon # noqa: E402 + REPO = "jsboige/CoursIA" COORDINATOR_USER = "myia-ai-01" GATE_PATH = SCRIPTS_DIR / "check_adjoint_prevalidation.py" @@ -147,6 +165,31 @@ GATE_RC_REASONS = {1: "gate:no-dossier", 2: "gate:unknown", 3: "gate:blocked"} NITS_DOCUMENTED_RC = frozenset({0, 1}) # clear / blocked +# --- disposition de review classsee a la tete evaluee (point 1 de #17672) ------ +# Le dossier HACHE l'oid de chaque review dans son empreinte +# (``check_adjoint_prevalidation._fingerprint_payload``) sans le CLASSER : rien +# ne dit si l'approbation porte sur le commit qui va etre merge. Trois etats, +# toujours lus a la tete que la ligne de journal declare : +# ``approved-exact-head`` une voix approbatrice gouverne CETTE tete ; +# ``approval-not-on-head`` une approbation existe dans la fenetre lue, mais +# elle ne couvre pas la tete evaluee (tete avancee +# depuis l'approbation, ou voix posterieure qui n'approuve +# pas) ; +# ``no-approval`` aucune approbation lue. +# Les deux surfaces du canon sont lues : l'etat REEL de l'API (APPROVED, quand un +# humain review) et le verdict type du CORPS en COMMENT -- la seule surface dont +# dispose le jeton du cluster. Lire la seule premiere classerait « sans +# approbation » des PR revues, le faux compte que #16926 a deja puni deux fois. +APPROVED_EXACT_HEAD = "approved-exact-head" +APPROVAL_NOT_ON_HEAD = "approval-not-on-head" +NO_APPROVAL = "no-approval" +NOT_EVALUATED = "not-evaluated" # ligne d'un run interrompu avant evaluation + +#: Ce qui vaut approbation : l'etat REEL ``APPROVED``, ou le verdict type +#: ``LGTM`` emis en corps de voix (``VERDICT_RE`` du canon ne type que LGTM et +#: CONCERNS -- les deux etats reels suffisent au reste). +APPROVING_VOICES = frozenset({"APPROVED", "LGTM"}) + MAX_MERGES_DEFAULT = 15 PR_LIST_LIMIT = 500 # le pool ouvert mesure ~220 PRs ; au-dela, ordre ancien d'abord # Mesure du 2026-09-22 (22:15Z-22:45Z), merges en rafale sur la file vivante : @@ -259,6 +302,9 @@ class PRVerdict: verdict: str # skipped | would-merge | merged | merge-failed | run-error reason: str | None merged: bool + #: Disposition de review classee a ``head`` (point 1 de #17672) ; une ligne + #: ecrite avant evaluation (erreur inattendue) porte ``not-evaluated``. + review: str = NOT_EVALUATED def journal_dict(self) -> dict: return { @@ -268,6 +314,7 @@ def journal_dict(self) -> dict: "verdict": self.verdict, "reason": self.reason, "merged": self.merged, + "review": self.review, } @@ -430,11 +477,18 @@ def list_open_prs(runner: Runner, gh_env: dict[str, str]) -> list[int]: return numbers -PR_VIEW_FIELDS = "number,title,isDraft,body,headRefName,headRefOid,files,changedFiles,comments" +#: ``reviews`` est lu par la MEME commande que le reste : la disposition de +#: review se classe sans appel supplementaire (l'oid de review y figure, mesure +#: du 2026-09-25 : ``gh pr view --json reviews`` rend ``commit.oid``). +PR_VIEW_FIELDS = ( + "number,title,isDraft,body,headRefName,headRefOid,files,changedFiles," + "comments,reviews" +) def fetch_pr_view(runner: Runner, pr: int, gh_env: dict[str, str]) -> dict: - """Une vue par PR : brouillon, body (tag Grain), tete, fichiers, commentaires.""" + """Une vue par PR : brouillon, body (tag Grain), tete, fichiers, commentaires, + reviews (disposition de review classee a la tete, point 1 de #17672).""" res = runner.run( ["gh", "pr", "view", str(pr), "--repo", REPO, "--json", PR_VIEW_FIELDS], env=gh_env, @@ -449,6 +503,60 @@ def fetch_pr_view(runner: Runner, pr: int, gh_env: dict[str, str]) -> dict: return view +def _review_voice_state(review: dict) -> str | None: + """L'etat REEL d'une review, ou le verdict type de son corps (canal COMMENT). + + Meme lecture a deux surfaces que le canon (``REAL_STATES`` puis + ``VERDICT_RE``), pour la meme raison : le jeton du cluster ne peut poster que + des COMMENT, son verdict vit donc dans le corps (#16926). Une voix sans + verdict type n'est pas une approbation -- un commentaire de lane, de CI ou + un ``COMMENTED`` muet ne dit rien de la disposition. + + ``DISMISSED`` n'est jamais approbateur : une approbation ANNULEE ne + gouverne plus, meme si son corps porte encore le jeton type (reserve 2 + Hermes 2026-09-26 : le croisement des deux surfaces manquait). + """ + state = str(review.get("state") or "") + if state == "DISMISSED": + return None + if state in review_canon.REAL_STATES: + return state + match = review_canon.VERDICT_RE.search(str(review.get("body") or "")) + return match.group(1) if match else None + + +def review_disposition(view: dict, head: str) -> str: + """La disposition de review, CLASSEE a la tete evaluee (point 1 de #17672). + + La tete gouverne : une approbation posee sur un commit anterieur ne couvre + pas le commit qui va etre merge, et c'est cette difference que le dossier + n'exprime pas (il hache l'oid de review sans le comparer). La voix qui + gouverne une tete est la PLUS RECENTE des VOIX posees sur cette tete + (latest-wins, la discipline du canon) : une approbation suivie, sur la + meme tete, d'une voix qui n'approuve pas, n'est plus une approbation. + Une ligne qui n'est pas une voix au sens du canon (pas d'etat REEL ni de + ``VERDICT`` type en corps -- commentaire de lane, ``[OVERRIDE]``, CI) ne + detrone rien : le latest-wins porte sur les voix, pas sur les lignes + ``reviews[]`` (reserve 1 Hermes 2026-09-26). + + Fonction pure : la vue est deja fetchee, aucun appel supplementaire. + """ + reviews = [row for row in (view.get("reviews") or []) if isinstance(row, dict)] + at_head = [ + row + for row in reviews + if str(((row.get("commit") or {}).get("oid")) or "") == head + ] + voices_at_head = [row for row in at_head if _review_voice_state(row)] + if voices_at_head: + latest = max(voices_at_head, key=lambda row: str(row.get("submittedAt") or "")) + if _review_voice_state(latest) in APPROVING_VOICES: + return APPROVED_EXACT_HEAD + if any(_review_voice_state(row) in APPROVING_VOICES for row in reviews): + return APPROVAL_NOT_ON_HEAD + return NO_APPROVAL + + def precheck_dossier(view: dict) -> str | None: """Pre-controle bon marche, AVANT le gate : le dernier dossier visible dans la vue est-il a la tete courante, et declare-t-il ``b0: clear`` ? @@ -660,10 +768,18 @@ def evaluate_pr( Les controles bon marche (brouillon, commentaire, perimetre, tag) passent AVANT le gate couteux ; le gate avant les organes B.0 ; le REST en dernier, juste avant le merge, pour minimiser la fenetre de course. + + La disposition de review (point 1 de #17672) est classee pour TOUTE PR + evaluee, skip compris, et reportee a la tete que la ligne declare : la tete + de la vue pour un skip, la tete du gate pour un would-merge -- celle que le + merge epingle, l'etape 6 ayant deja refuse une tete qui a bouge depuis. """ + view_head = str(view.get("headRefOid") or "") + disposition = review_disposition(view, view_head) + def skip(reason: str) -> PRVerdict: - return PRVerdict(pr, str(view.get("headRefOid") or ""), "skipped", reason, False) + return PRVerdict(pr, view_head, "skipped", reason, False, disposition) # 1. prefiltre bon marche. if view.get("isDraft"): @@ -713,7 +829,9 @@ def skip(reason: str) -> PRVerdict: return skip(f"mergeable-state:{state or 'absent'}") if live_head != gate_head: return skip("head-moved") - return PRVerdict(pr, gate_head, "would-merge", None, False) + return PRVerdict( + pr, gate_head, "would-merge", None, False, review_disposition(view, gate_head) + ) # --- journal ------------------------------------------------------------------- @@ -734,9 +852,12 @@ def append_journal(path: Path, verdict: PRVerdict) -> None: def _describe(verdict: PRVerdict) -> str: if verdict.verdict == "merged": - return f"PR #{verdict.pr} MERGED ({verdict.head})" + return f"PR #{verdict.pr} MERGED ({verdict.head}) [review: {verdict.review}]" if verdict.verdict == "would-merge": - return f"PR #{verdict.pr} WOULD MERGE ({verdict.head}) [dry-run]" + return ( + f"PR #{verdict.pr} WOULD MERGE ({verdict.head}) [dry-run] " + f"[review: {verdict.review}]" + ) if verdict.verdict == "merge-failed": return f"PR #{verdict.pr} MERGE ECHOUE : {verdict.reason}" if verdict.verdict == "run-error": @@ -769,6 +890,11 @@ def emit_output( would = sum(1 for v in results if v.verdict == "would-merge") skipped = sum(1 for v in results if v.verdict == "skipped") errored = sum(1 for v in results if v.verdict == "run-error") + # La disposition de review ne se compte que sur les candidates au merge : + # c'est la que « approbation a la tete exacte » ou « ailleurs » decide. + candidates = [v for v in results if v.verdict in ("merged", "would-merge")] + at_head = sum(1 for v in candidates if v.review == APPROVED_EXACT_HEAD) + off_head = sum(1 for v in candidates if v.review == APPROVAL_NOT_ON_HEAD) print( f"bilan : {len(results)} evaluee(s), {merged} merge(s), " f"{would} would-merge, {skipped} skip(s)" @@ -776,6 +902,12 @@ def emit_output( # compte, un run qui a continue malgre une PR en erreur se lirait comme # un run propre (#17672 point 3). + (f", {errored} erreur(s) isolee(s)" if errored else "") + + ( + f", candidates : {at_head} approved-exact-head, " + f"{off_head} approval-not-on-head" + if candidates + else "" + ) ) if stopped_reason: print(f"arret : {stopped_reason}") @@ -841,14 +973,17 @@ def run(argv: list[str] | None = None, runner: Runner | None = None) -> int: merge_pr(active_runner, pr, decision.head or "", gh_env) except MergeFailedError as exc: failed = PRVerdict( - pr, decision.head, "merge-failed", str(exc), False + pr, decision.head, "merge-failed", str(exc), False, + decision.review, ) results.append(failed) append_journal(journal_path, failed) stopped_reason = f"merge-failed:PR-{pr}" exit_code = 1 break - decision = PRVerdict(pr, decision.head, "merged", None, True) + decision = PRVerdict( + pr, decision.head, "merged", None, True, decision.review + ) merged_count += 1 results.append(decision) append_journal(journal_path, decision) diff --git a/scripts/tests/test_merge_ready.py b/scripts/tests/test_merge_ready.py index 5e8505d658..4a7299ba45 100644 --- a/scripts/tests/test_merge_ready.py +++ b/scripts/tests/test_merge_ready.py @@ -67,6 +67,7 @@ def default_view( body: str | None = None, comments: list[dict] | None = None, title: str = "fix(x): une PR ordinaire", + reviews: list[dict] | None = None, ) -> dict: return { "number": pr, @@ -79,6 +80,25 @@ def default_view( "comments": comments if comments is not None else [{"body": dossier_body()}], + "reviews": reviews if reviews is not None else [], + } + + +def review_row( + *, + state: str = "APPROVED", + oid: str = HEAD, + submitted: str = "2026-09-25T03:00:00Z", + body: str = "", + login: str = "clusterManager-Myia", +) -> dict: + """Forme de ``gh pr view --json reviews`` (l'oid de review est sur ``commit``).""" + return { + "author": {"login": login}, + "state": state, + "body": body, + "submittedAt": submitted, + "commit": {"oid": oid}, } @@ -505,11 +525,17 @@ def test_journal_ligne_par_pr(tmp_path): assert journal.is_file() assert len(lines) == 2 for row in lines: - assert set(row.keys()) == {"ts", "pr", "head", "verdict", "reason", "merged"} + assert set(row.keys()) == { + "ts", "pr", "head", "verdict", "reason", "merged", "review", + } assert re.fullmatch(r"\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z", row["ts"]) assert isinstance(row["pr"], int) assert row["merged"] is True and row["verdict"] == "merged" assert row["reason"] is None + # Une ligne mergee porte la disposition CLASSEE, pas « non evaluee » : + # le verdict terminal est reconstruit apres le merge, il doit heriter de + # la classification faite avant. + assert row["review"] == mr.NO_APPROVAL assert [row["pr"] for row in lines] == [401, 402] # ancienne d'abord @@ -685,6 +711,136 @@ def test_hold_file_override(tmp_path): assert lines[-1]["reason"] == "hold:ordre de stack" +# --- disposition de review classee a la tete evaluee (point 1 de #17672) --------- + + +def test_disposition_approbation_a_la_tete(): + view = default_view(reviews=[review_row(oid=HEAD)]) + assert mr.review_disposition(view, HEAD) == mr.APPROVED_EXACT_HEAD + + +def test_disposition_approbation_sur_une_tete_ancienne(): + # L'approbation existe, mais elle porte sur un commit anterieur : elle ne + # couvre pas le commit qui va etre merge. + view = default_view(reviews=[review_row(oid=HEAD_MOVED)]) + assert mr.review_disposition(view, HEAD) == mr.APPROVAL_NOT_ON_HEAD + + +def test_disposition_verdict_en_corps_compte_a_la_tete(): + # Le jeton de review du cluster ne peut poster que des COMMENT : son + # approbation vit dans le CORPS de la voix, pas dans l'etat de l'API (#16926). + view = default_view( + reviews=[ + review_row( + state="COMMENTED", + oid=HEAD, + body=( + "**[Hermes]** — VERDICT: LGTM " + "(contrainte token CoursIA : COMMENT only, #15511)" + ), + ) + ] + ) + assert mr.review_disposition(view, HEAD) == mr.APPROVED_EXACT_HEAD + + +def test_disposition_voix_posterieure_non_approbatrice_retire_l_approbation(): + # Latest-wins sur la tete : une approbation suivie, sur la MEME tete, d'une + # voix qui n'approuve pas ne gouverne plus. + view = default_view( + reviews=[ + review_row(oid=HEAD, submitted="2026-09-25T03:00:00Z"), + review_row( + state="COMMENTED", + oid=HEAD, + submitted="2026-09-25T04:00:00Z", + body="VERDICT: CONCERNS (test rouge depuis le dernier push)", + ), + ] + ) + assert mr.review_disposition(view, HEAD) == mr.APPROVAL_NOT_ON_HEAD + + +def test_disposition_ligne_non_voix_ne_detronne_pas_l_approbation(): + # Reserve 1 Hermes (2026-09-26) : le latest-wins porte sur les VOIX du + # canon, pas sur les lignes reviews[]. Un COMMENTED SANS verdict type -- + # la forme reelle des [OVERRIDE] de lane -- n'est pas une voix : il ne + # detrone pas une approbation posee sur la meme tete. Avant le filtre, + # cette vue rendait approval-not-on-head alors que l'approbation gouverne. + view = default_view( + reviews=[ + review_row(oid=HEAD, submitted="2026-09-25T03:00:00Z"), + review_row( + state="COMMENTED", + oid=HEAD, + submitted="2026-09-25T04:00:00Z", + body="[OVERRIDE] lane myia-ai-01:CoursIA -- reserve G-VAR-3 levee", + ), + ] + ) + assert mr.review_disposition(view, HEAD) == mr.APPROVED_EXACT_HEAD + + +def test_disposition_review_dismissed_n_est_jamais_approbatrice(): + # Reserve 2 Hermes (2026-09-26) : une approbation ANNULEE ne gouverne + # plus, meme si son corps porte encore le jeton type. Le croisement des + # deux surfaces (etat DISMISSED + VERDICT en corps) manquait : cette vue + # rendait approved-exact-head avant le traitement explicite de DISMISSED. + view = default_view( + reviews=[ + review_row( + state="DISMISSED", + oid=HEAD, + body="VERDICT: LGTM (annule apres relecture du diff)", + ) + ] + ) + assert mr.review_disposition(view, HEAD) == mr.NO_APPROVAL + + +def test_disposition_sans_approbation_lue(): + assert mr.review_disposition(default_view(), HEAD) == mr.NO_APPROVAL + # Une voix qui ne type pas de verdict n'est pas une approbation. + view = default_view(reviews=[review_row(state="COMMENTED")]) + assert mr.review_disposition(view, HEAD) == mr.NO_APPROVAL + + +def test_deux_prs_qui_ne_different_que_par_la_tete_de_l_approbation(tmp_path): + """Le defaut vise : a tout le reste egal, l'organe ne distinguait pas une PR + approuvee a la tete de la PR approuvee sur un commit anterieur.""" + views = { + 201: default_view(pr=201, reviews=[review_row(oid=HEAD)]), + 202: default_view(pr=202, reviews=[review_row(oid=HEAD_MOVED)]), + } + rc, lines, _ = run_organ(tmp_path, ScriptedRunner(prs=(201, 202), views=views)) + assert rc == 0 + assert [row["verdict"] for row in lines] == ["would-merge", "would-merge"] + reste = [ + {k: row[k] for k in ("head", "verdict", "reason", "merged")} for row in lines + ] + assert reste[0] == reste[1] + assert lines[0]["review"] == mr.APPROVED_EXACT_HEAD + assert lines[1]["review"] == mr.APPROVAL_NOT_ON_HEAD + + +def test_un_skip_porte_la_disposition_de_la_tete_evaluee(tmp_path): + views = {201: default_view(pr=201, draft=True, reviews=[review_row(oid=HEAD)])} + rc, lines, _ = run_organ(tmp_path, ScriptedRunner(prs=(201,), views=views)) + assert rc == 0 + assert lines[-1]["reason"] == "draft" + assert lines[-1]["review"] == mr.APPROVED_EXACT_HEAD + + +def test_le_bilan_compte_les_candidates_par_disposition(tmp_path, capsys): + views = { + 201: default_view(pr=201, reviews=[review_row(oid=HEAD)]), + 202: default_view(pr=202, reviews=[review_row(oid=HEAD_MOVED)]), + } + run_organ(tmp_path, ScriptedRunner(prs=(201, 202), views=views)) + out = capsys.readouterr().out + assert "candidates : 1 approved-exact-head, 1 approval-not-on-head" in out + assert "[review: approved-exact-head]" in out + # --- 5bis. collision d'index twin-pairs ----------------------------------------- TWIN_FILE = "scripts/notebook_tools/twin_pairs.d/sw-5-linked-data/0012-2026-09-25-lane.yaml"