feat(ci,#19251): mesurer les carnets RENAMED dans le balayage credited (previous_filename par REST)

scripts/notebook_tools/check_pr_exercises.py

@@ -205,6 +205,7 @@ def check_notebooks(
     base_ref: str = "",
     head_ref: str = "",
     pr_body: str = "",
+    base_path_of: dict[str, str] | None = None,
 ) -> CheckResult:
     """Classify + count each path, bucketing by advisory status.
 
@@ -219,6 +220,12 @@ def check_notebooks(
     exemption markers from ``pr_body``. The default (``base_ref=""``) skips
     this check (preserves backward compatibility for callers that don't pass
     a base ref).
+
+    #19251 -- ``base_path_of`` maps a ``path`` to the location it had at the
+    base, for a notebook the PR **renamed**: the credited diff then reads
+    ``base:previousFilename`` against ``head:path`` instead of the same path on
+    both sides (which would find nothing at the base and read as a full loss).
+    Absent from the map, ``path`` is used for both sides -- the pre-rename case.
     """
     result = CheckResult()
     # Pré-calcul des exemptions une seule fois (cf. #18740).
@@ -283,7 +290,16 @@ def check_notebooks(
                     head_examples = count_credited_examples(_read_nb(head_blob))
                 else:
                     head_examples = count_credited_examples(_read_nb(path))
-                base_path = _read_git_blob(base_ref, str(path).replace("\\", "/"))
+                # #19251 -- un carnet RENOMME par la PR n'existe pas au meme
+                # chemin dans la base : sans la correspondance, `base:path`
+                # rendrait un blob absent et le diff lirait une perte totale.
+                # La carte est clee en POSIX (le chemin GraphQL) : on normalise
+                # la cle du `Path` avant de la consulter, sinon sous Windows
+                # (`str(path)` rend des `\`) la correspondance raterait en
+                # silence et le renommage redeviendrait non mesure.
+                posix_path = str(path).replace("\\", "/")
+                base_side = (base_path_of or {}).get(posix_path, posix_path)
+                base_path = _read_git_blob(base_ref, base_side)
                 base_examples = count_credited_examples(_read_nb(base_path))
                 diff = diff_examples(base_examples, head_examples)
                 nb_name = path.name

scripts/notebook_tools/credited_examples_sweep.py

@@ -15,7 +15,7 @@
 coup : il liste les PRs MERGEES de la fenetre et rejoue, pour chacune, le diff
 avec SA base et SON body -- c'est l'option 1 de #19101.
 
-## Deux sources de verite, chacune du bon cote (review 04:53Z)
+## Trois sources de verite, chacune du bon cote (review 04:53Z, #19251)
 
 - **`changeType` n'existe pas dans `gh pr list --json files`** (mesure gh
   2.83.2 : cette forme ne rend que `{additions, deletions, path}`). Le
@@ -24,6 +24,14 @@
   carnet supprime (#19040 a supprime GameTheory-18d) fait sinon planter le
   balayage en `FileNotFoundError` : le repli « tout MODIFIED » de la premiere
   version classait tout en modifie.
+- **le chemin de base d'un RENAMED n'existe PAS cote GraphQL** (#19251,
+  mesure : le serveur repond `Field 'previousFilename' doesn't exist on type
+  'PullRequestChangedFile'` ; ses champs sont `additions, changeType,
+  deletions, path, viewerViewedState`). Il vit cote **REST**, champ
+  `previous_filename` de `pulls/{n}/files` -- lu par `_previous_filenames`,
+  et **seulement** quand la PR porte un renommage. Avec ce chemin, le
+  renommage est **mesure** comme un MODIFIED ; sans lui, il reste nomme NON
+  MESURE.
 - **le cote « apres » est la tete de la PR, pas l'arbre de travail** :
   `check_notebooks` recoit `head_ref=headRefOid`, et le commit est amene
   localement s'il manque (PR squash-mergee : l'objet n'est pas dans `main`).
@@ -129,13 +137,52 @@ def merged_prs(repo: str, since: dt.datetime, run=_gh_json) -> list[dict]:
     return rows
 
 
+def _previous_filenames(repo: str, number: int, run=_gh_json) -> dict[str, str]:
+    """`{chemin_de_tete: chemin_de_base}` des renommages, par REST.
+
+    `previousFilename` n'existe **pas** sur le type GraphQL
+    `PullRequestChangedFile` (#19251, mesure : le serveur repond `Field
+    'previousFilename' doesn't exist on type 'PullRequestChangedFile'` ; ses
+    champs sont `additions, changeType, deletions, path, viewerViewedState`).
+    Le chemin de base d'un renommage vit cote REST, champ `previous_filename`
+    de `pulls/{n}/files` -- d'ou cette seconde source, appelee **seulement**
+    quand la PR porte un renommage. Pagement explicite (`per_page`/`page`) :
+    `per_page` en `-f` ferait basculer `gh api` en POST.
+    """
+    owner, name = repo.split("/", 1)
+    out: dict[str, str] = {}
+    page = 1
+    while True:
+        payload = run([
+            "api", (f"repos/{owner}/{name}/pulls/{number}/files"
+                    f"?per_page=100&page={page}"),
+        ])
+        if not isinstance(payload, list) or not payload:
+            return out
+        for f in payload:
+            head = f.get("filename") or ""
+            prev = f.get("previous_filename") or ""
+            if head and prev:
+                out[head] = prev
+        if len(payload) < 100:
+            return out
+        page += 1
+
+
 def pr_files(repo: str, number: int, run=_gh_json) -> list[dict]:
     """Fichiers de la PR avec `changeType`, par GraphQL, pagine au curseur.
 
     `gh pr view --json files` (REST/CLI) ne rend que des decomptes : la nature
     du changement n'existe que cote GraphQL. Pages de 100, boucle sur
     `pageInfo.endCursor` -- une PR de carnet peut deplacer plus de 100 fichiers
     (mesure : #19040 en deplace 2876+214 lignes sur plusieurs carnets).
+
+    #19251 -- le chemin de base d'un `RENAMED` n'est pas dans GraphQL : quand la
+    PR porte au moins un renommage, une seconde passe REST (`_previous_filenames`)
+    remplit `previousFilename` sur ces seuls noeuds, pour que le renommage soit
+    mesurable. Si cette passe echoue, les renommages restent sans chemin de base
+    (`""`) et seront nommes NON MESURES par `sweep` -- jamais mesures contre le
+    mauvais chemin.
     """
     owner, name = repo.split("/", 1)
     nodes: list[dict] = []
@@ -151,14 +198,25 @@ def pr_files(repo: str, number: int, run=_gh_json) -> list[dict]:
         nodes.extend(conn.get("nodes") or [])
         page = conn.get("pageInfo") or {}
         if not page.get("hasNextPage"):
-            return nodes
+            break
         cursor = page.get("endCursor")
+    if any(n.get("changeType") == "RENAMED" for n in nodes):
+        try:
+            previous = _previous_filenames(repo, number, run)
+        except RuntimeError:
+            previous = {}
+        for node in nodes:
+            if node.get("changeType") == "RENAMED":
+                node["previousFilename"] = previous.get(node.get("path") or "", "")
+    return nodes
 
 
-def _ipynb_by_change(files: list[dict]) -> tuple[list[str], list[str], list[str]]:
-    """`(modifies, ajoutes, renommes)` parmi les `.ipynb` de la PR.
+def _ipynb_by_change(
+    files: list[dict],
+) -> tuple[list[str], list[str], list[tuple[str, str]], list[str]]:
+    """`(modifies, ajoutes, renommes_mesurables, renommes_non_mesurables)`.
 
-    Deux exclusions et une separation, toutes structurelles, toutes lues de
+    Trois exclusions et une separation, toutes structurelles, toutes lues de
     `changeType` (GraphQL) :
 
       - `DELETED` : supprime, plus rien a compter (meme regle que le
@@ -167,17 +225,27 @@ def _ipynb_by_change(files: list[dict]) -> tuple[list[str], list[str], list[str]
         l'ouverture du carnet levait `FileNotFoundError` sur GameTheory-18d
         (supprime par #19040) ;
       - `ADDED` : neuf, donc **rien a perdre** par construction ;
-      - `RENAMED` : la version de base existe **sous un autre chemin**, que
-        cette source n'expose pas (`previousFilename` absent du jeu GraphQL
-        demande ici). On ne peut donc pas la comparer -- et contrairement a
-        `ADDED`, un renommage **peut** perdre des exemples. NON MESURE.
+      - `RENAMED` : la version de base existe **sous un autre chemin**. Depuis
+        #19251 ce chemin est rempli par `pr_files` (passe REST,
+        `previous_filename`) : quand il est present, le renommage est
+        **mesurable** et rendu comme la paire `(chemin_de_tete, chemin_de_base)`,
+        pour que le diff credite soit calcule entre `base:previous` et
+        `head:path` -- un `git mv` suivi d'une edition peut perdre des exemples
+        comme un `MODIFIED`.
+
+    `previousFilename` peut manquer (passe REST en echec, ou renommage hors du
+    champ REST) : ces renommages-la restent **non mesurables** et sont rendus a
+    part, pour etre **nommes** NON MESURES plutot que comptes comme zero perte.
 
     Pourquoi separer plutot que compter en erreur : `credited_diff_errors > 0`
     **bloque** la pose du label (#18761). Une erreur structurelle sur un carnet
     empechait donc la mesure reelle des carnets modifies de la meme PR -- un
     faux positif d'erreur produisait un faux zero de pertes.
     """
-    modified, added, renamed = [], [], []
+    modified: list[str] = []
+    added: list[str] = []
+    renamed_measured: list[tuple[str, str]] = []
+    renamed_unmeasured: list[str] = []
     for f in (files or []):
         path = f.get("path") or ""
         if not path.endswith(".ipynb"):
@@ -188,10 +256,14 @@ def _ipynb_by_change(files: list[dict]) -> tuple[list[str], list[str], list[str]
         if change == "ADDED":
             added.append(path)
         elif change == "RENAMED":
-            renamed.append(path)
+            previous = f.get("previousFilename") or ""
+            if previous:
+                renamed_measured.append((path, previous))
+            else:
+                renamed_unmeasured.append(path)
         else:
             modified.append(path)
-    return modified, added, renamed
+    return modified, added, renamed_measured, renamed_unmeasured
 
 
 def ipynb_paths(files: list[dict]) -> list[str]:
@@ -253,18 +325,27 @@ def sweep(repo: str, repo_dir: Path, hours: int, now: dt.datetime,
         except RuntimeError as exc:
             errors.append(f"#{number}: fichiers illisibles ({exc})")
             continue
-        paths, added, renamed = _ipynb_by_change(files)
+        modified, added, renamed_measured, renamed_unmeasured = \
+            _ipynb_by_change(files)
+        # #19251 -- un renommage dont `previousFilename` est connu est mesure
+        # comme un MODIFIED : sa tete se lit au nouveau chemin, sa base a
+        # l'ancien. `base_path_of` porte cette correspondance ; un renommage
+        # sans `previousFilename` reste NON MESURE et seulement nomme.
+        base_path_of = {head: prev for head, prev in renamed_measured}
+        paths = modified + [head for head, _ in renamed_measured]
+        renamed = len(renamed_measured) + len(renamed_unmeasured)
         if not paths and not added and not renamed:
             continue
         if not paths:
             # PR sans carnet mesurable : rien a perdre (ajouts) ou rien de
-            # comparable (renommages). On la NOMME plutot que de la faire
-            # disparaitre du rapport.
+            # comparable (renommages sans chemin de base). On la NOMME plutot
+            # que de la faire disparaitre du rapport.
             rows.append({
                 "number": number,
                 "notebooks": 0,
                 "added_notebooks": len(added),
-                "renamed_notebooks": len(renamed),
+                "renamed_notebooks": renamed,
+                "renamed_measured": len(renamed_measured),
                 "credited_lost_unexempted": 0,
                 "credited_diff_errors": 0,
                 "would_label": False,
@@ -298,7 +379,8 @@ def sweep(repo: str, repo_dir: Path, hours: int, now: dt.datetime,
             continue
         try:
             result = check([Path(p) for p in paths], base_ref=base,
-                           head_ref=head_ref, pr_body=body)
+                           head_ref=head_ref, pr_body=body,
+                           base_path_of=base_path_of)
         except (OSError, ValueError) as exc:
             # #19215 (review 5411248369, voie 2) : un carnet illisible depuis
             # l'arbre du jour ne doit pas emporter les AUTRES PR de la
@@ -317,7 +399,8 @@ def sweep(repo: str, repo_dir: Path, hours: int, now: dt.datetime,
             "number": number,
             "notebooks": len(paths),
             "added_notebooks": len(added),
-            "renamed_notebooks": len(renamed),
+            "renamed_notebooks": renamed,
+            "renamed_measured": len(renamed_measured),
             "credited_lost_unexempted": lost,
             "credited_diff_errors": diff_errors,
             # #18761 : le label ne se pose QUE si tous les diffs ont reussi.
@@ -370,11 +453,16 @@ def main(argv: list[str] | None = None) -> int:
         mark = "LABEL" if row["would_label"] else "  -  "
         added = row.get("added_notebooks", 0)
         renamed = row.get("renamed_notebooks", 0)
+        renamed_measured = row.get("renamed_measured", 0)
+        renamed_unmeasured = renamed - renamed_measured
         suffix = ""
         if added:
             suffix += f" (+{added} neuf(s), rien a perdre)"
-        if renamed:
-            suffix += f" ({renamed} renomme(s), NON MESURE(S) : base a un autre chemin)"
+        if renamed_measured:
+            suffix += f" ({renamed_measured} renomme(s) mesure(s) via previousFilename)"
+        if renamed_unmeasured:
+            suffix += (f" ({renamed_unmeasured} renomme(s) NON MESURE(S) : "
+                       "previousFilename absent)")
         print(f"  [{mark}] #{row['number']}: {row['notebooks']} carnet(s) modifie(s), "
               f"pertes non exemptees={row['credited_lost_unexempted']}, "
               f"erreurs de diff={row['credited_diff_errors']}{suffix}")