From 174d174713d69e32ad6a0ff849fd84bd9acf5ef0 Mon Sep 17 00:00:00 2001 From: jsboige Date: Mon, 5 Oct 2026 05:13:04 +0200 Subject: [PATCH 1/3] feat(ci,#19101): reveiller la branche credited-examples de exercises-advisory (balayage post-mortem) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit La branche « exemples credites » posee par #18761 etait **dormante**. Le diff exige `--base` ET `--pr-body-file` ; sous `schedule` -- seul declencheur qui subsiste apres la tranche 1 de #12817 -- il n'y a pas de contexte PR, donc `PR_NUMBER` est vide, donc `PR_BODY_FILE` reste vide, donc le test de `exercises-advisory.yml` prend la branche sans `--base` et **saute le diff**. Le label `credited-examples-lost` ne pouvait structurellement pas etre pose. ## Ce que livre la PR `scripts/notebook_tools/credited_examples_sweep.py` : balayage **post-mortem** des PRs mergees de la fenetre, rejouant le diff pour chacune avec SA base et SON body (option 1 de #19101). Cable comme etape du workflow ; l'etape existante n'est pas touchee. **Aucun `pull_request` n'est reintroduit** sur ce workflow : c'est la contrainte explicite de l'issue (le cout du clone par PR est la motivation d'origine de #12817). Le compromis -- detecter les pertes passees, pas proteger le merge -- est ecrit dans le module, pas seulement ici. ## Le piege trouve en mesurant Premiere mesure sur 24 h : **6 « erreurs de diff »** sur 38 PRs. Instruites, elles venaient toutes du meme cas : `git show :` sort en **128** parce que le carnet n'est pas a ce chemin dans la base. Ce n'etait pas cosmetique. `credited_diff_errors > 0` **bloque** la pose du label (#18761) : une erreur structurelle sur un carnet empechait la mesure reelle des carnets modifies de la meme PR. Un faux positif d'erreur produisait un faux zero de pertes. Trois cas separes, qui ne disent pas la meme chose : - `ADDED` : neuf, **rien a perdre** par construction (6 des 6 erreurs) ; - `RENAMED` : la base est a un **autre chemin**, que `gh pr view --json files` n'expose pas (`previousFilename` absent). Non comparable -- et contrairement a `ADDED`, un renommage **peut** perdre des exemples : declare NON MESURE, jamais « sans perte » ; - `DELETED` : exclu, comme le `--diff-filter=d` du workflow. Les carnets non mesurables sont **nommes** dans le rapport au lieu de disparaitre : un carnet tu se lirait comme un carnet conforme. ## Mesure Fenetre de 24 h sur `main`, `--json`, sans `--apply`. Un diff en erreur n'est PAS un zero mesure (#18761) : le rapport separe « pertes non exemptees = 0 » de « carnets non mesurables », et n'affirme jamais une couverture que la mesure ne porte pas. ## Tests `test_credited_examples_sweep.py` : tout hors ligne (reseau et git injectes). Le lot de l'API de recherche **au plafond** leve au lieu de passer pour un compte ; le label ne se pose que si **tous** les diffs ont reussi ; un `merge-base` indisponible est nomme ; un body illisible est nomme et la PR ecartee ; un `ADDED` n'est pas mesure ; un `RENAMED` est declare non mesure ET ne bloque plus les carnets modifies de la meme PR. Co-Authored-By: Claude Sonnet 5.5 --- .github/workflows/exercises-advisory.yml | 23 ++ .../notebook_tools/credited_examples_sweep.py | 300 ++++++++++++++++++ scripts/tests/test_credited_examples_sweep.py | 238 ++++++++++++++ 3 files changed, 561 insertions(+) create mode 100644 scripts/notebook_tools/credited_examples_sweep.py create mode 100644 scripts/tests/test_credited_examples_sweep.py diff --git a/.github/workflows/exercises-advisory.yml b/.github/workflows/exercises-advisory.yml index fbb9adc9f3..517a956658 100644 --- a/.github/workflows/exercises-advisory.yml +++ b/.github/workflows/exercises-advisory.yml @@ -252,3 +252,26 @@ jobs: echo "RESULT: all in-corpus modified notebooks meet their threshold (verified)." fi exit 0 + + - name: Credited-examples sweep over the day's merged PRs (nocturne) + # #19101 -- l'etape precedente porte la branche « exemples credites » + # de #18761, mais elle est DORMANTE sous `schedule` : le diff exige + # `--base` ET `--pr-body-file`, et le mode nocturne n'a pas de contexte + # PR, donc pas de body. Le test de l'etape (l.136) echouait, la branche + # `else` sans `--base` etait prise, et le diff n'etait **jamais** + # mesure : le label `credited-examples-lost` ne pouvait pas etre pose. + # + # Ce balayage le reveille SANS reintroduire `pull_request` sur ce + # workflow (interdit : le clone par PR est la motivation de #12817 + # tranche 1). Il rejoue le diff apres coup, PR par PR, avec SA base et + # SON body -- option 1 de #19101, assumee post-mortem. + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_REPO: ${{ github.repository }} + run: | + set -uo pipefail + # Advisory : un balayage qui echoue NOMME sa panne et ne rougit pas + # le run (le contrat de cet organe porte sur le verdict). Le repli et + # les erreurs de diff sont imprimes, jamais tus (#18761). + python scripts/notebook_tools/credited_examples_sweep.py \ + --hours 24 --apply || echo "::warning::credited sweep failed -- nothing labelled" diff --git a/scripts/notebook_tools/credited_examples_sweep.py b/scripts/notebook_tools/credited_examples_sweep.py new file mode 100644 index 0000000000..d2aba25408 --- /dev/null +++ b/scripts/notebook_tools/credited_examples_sweep.py @@ -0,0 +1,300 @@ +#!/usr/bin/env python3 +r"""credited_examples_sweep.py -- pose POST-MORTEM du label `credited-examples-lost`. + +## Pourquoi (#19101) + +`exercises-advisory.yml` porte une branche « exemples credites » posee par +#18761, mais **dormante** : elle exige `--base` ET `--pr-body-file`. Sous +`schedule` -- le seul declencheur qui subsiste apres la tranche 1 de #12817 -- +il n'y a pas de contexte PR, donc pas de body, donc pas de `--base` : le diff +des exemples credites n'etait **jamais** mesure et le label ne pouvait pas +etre pose. + +Ce balayage le reveille **sans reintroduire `pull_request`** sur ce workflow +(interdit : le clone par PR etait la motivation de #12817). Il tourne apres +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. + +## Portee honnete + +Post-mortem : il detecte les pertes **passees**, il ne protege pas le merge. +C'est le compromis assume de l'option 1 ; l'option 2 (un declencheur PR leger) +reste ouverte et n'est pas traitee ici. + +Deux limites de la mesure, nommees plutot que tues : + + - la version HEAD est lue dans l'**arbre de travail** (defaut de + `check_notebooks`), donc une PR dont un carnet a ete re-touche par une PR + ulterieure est mesuree contre l'etat courant, pas contre son propre head. + Le chiffre reste une borne basse exploitable ; il n'est pas presente comme + « exactement l'apport de cette PR ». + - quand `headRefOid` n'est plus atteignable (ref supprimee au merge), le + merge-base est indisponible : la PR est alors mesuree **contre sa base + declaree** et le repli est imprime en `[WARN]`, jamais silencieux. + +## Usage + + python scripts/notebook_tools/credited_examples_sweep.py --hours 24 + python scripts/notebook_tools/credited_examples_sweep.py --hours 24 --apply + +Sans `--apply`, rien n'est pose : le rapport dit ce qui **serait** labellise. +""" +from __future__ import annotations + +import argparse +import datetime as dt +import json +import subprocess +import sys +from pathlib import Path + +_TOOLS_DIR = Path(__file__).resolve().parent +if str(_TOOLS_DIR) not in sys.path: + sys.path.insert(0, str(_TOOLS_DIR)) + +from check_pr_exercises import LABEL_NAME, check_notebooks # noqa: E402 + +REPO_DEFAULT = "jsboige/CoursIA" +LABEL_CREDITED_LOST = "credited-examples-lost" + +# Le plafond de l'API de recherche. Un lot qui l'atteint n'est pas un compte : +# c'est le plafond, et il cache les plus anciens (#19209, meme classe). Une +# fenetre qui sature est une erreur, jamais un corpus tronque qui a l'air +# complet. +SEARCH_RESULT_CAP = 1000 + + +def _run(argv: list[str], **kw) -> subprocess.CompletedProcess: + return subprocess.run( + argv, capture_output=True, text=True, encoding="utf-8", + errors="replace", check=False, **kw, + ) + + +def _gh_json(argv: list[str]) -> object: + proc = _run(["gh", *argv]) + if proc.returncode != 0: + raise RuntimeError( + f"gh failed ({proc.returncode}): " + f"{proc.stderr.strip() or proc.stdout.strip()}" + ) + if not proc.stdout.strip(): + return None + return json.loads(proc.stdout) + + +def merged_prs(repo: str, since: dt.datetime, run=_gh_json) -> list[dict]: + """PRs mergees depuis `since`, avec leurs fichiers et leurs deux refs. + + Leve si le lot atteint le plafond de l'API de recherche : un corpus + tronque se lirait comme un corpus complet, et les PRs perdues sont les + plus anciennes de la fenetre. + """ + stamp = since.strftime("%Y-%m-%dT%H:%M:%SZ") + rows = run([ + "pr", "list", "--repo", repo, "--state", "merged", + "--limit", str(SEARCH_RESULT_CAP), + "--search", f"merged:>={stamp}", + "--json", "number,baseRefOid,headRefOid,files,mergedAt", + ]) or [] + if len(rows) >= SEARCH_RESULT_CAP: + raise RuntimeError( + f"la fenetre depuis {stamp} rend {len(rows)} PRs, au plafond de " + f"l'API de recherche ({SEARCH_RESULT_CAP}) : le corpus serait " + "tronque par les plus anciennes. Reduire --hours." + ) + return rows + + +def _ipynb_by_change(pr: dict) -> tuple[list[str], list[str], list[str]]: + """`(modifies, ajoutes, renommes)` parmi les `.ipynb` de la PR. + + Deux exclusions et une separation, toutes structurelles. Toutes viennent + d'un `git show :` qui sort en **128** parce que le carnet + n'est pas a ce chemin dans la base -- ce qui n'est PAS une erreur de + mesure, mais ne veut pas dire la meme chose selon le cas : + + - `DELETED` : supprime, plus rien a compter (meme regle que le + `--diff-filter=d` du workflow) ; + - `ADDED` : neuf, donc **rien a perdre** par construction. Mesure du + 2026-10-05 : 6 des 6 « erreurs de diff » du corpus de 24 h venaient de + la (#18968, #19037, #19020, #19007, #18986) ; + - `RENAMED` : la version de base existe **sous un autre chemin**, que + `gh pr view --json files` n'expose pas ici (`previousFilename` absent). + On ne peut donc pas la comparer -- et contrairement a `ADDED`, un + renommage **peut** perdre des exemples. On le declare NON MESURE. + + 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 = [], [], [] + for f in (pr.get("files") or []): + path = f.get("path") or "" + if not path.endswith(".ipynb"): + continue + change = f.get("changeType") or "MODIFIED" + if change == "DELETED": + continue + if change == "ADDED": + added.append(path) + elif change == "RENAMED": + renamed.append(path) + else: + modified.append(path) + return modified, added, renamed + + +def ipynb_paths(pr: dict) -> list[str]: + """Chemins `.ipynb` **modifies** par la PR (cf. `_ipynb_by_change`).""" + return _ipynb_by_change(pr)[0] + + +def merge_base(repo_dir: Path, base_ref: str, head_ref: str) -> str | None: + """`git merge-base` des deux refs, ou None si l'une n'est pas atteignable. + + Pour une PR deja mergee et squash-mergee, `headRefOid` peut avoir disparu + du depot (ref supprimee). On rend None et l'appelant NOMME le repli -- + jamais un diff silencieusement mesure contre la mauvaise base. + """ + proc = _run(["git", "-C", str(repo_dir), "merge-base", base_ref, head_ref]) + if proc.returncode != 0: + return None + return proc.stdout.strip() or None + + +def pr_body(repo: str, number: int, run=_gh_json) -> str: + payload = run(["pr", "view", str(number), "--repo", repo, "--json", "body"]) + return ((payload or {}).get("body") or "") if isinstance(payload, dict) else "" + + +def sweep(repo: str, repo_dir: Path, hours: int, now: dt.datetime, + *, fetch_merged=merged_prs, fetch_body=pr_body, + base_of=merge_base, check=check_notebooks) -> tuple[list[dict], list[str]]: + """Rejoue le diff des exemples credites pour chaque PR mergee de la fenetre. + + Rend `(lignes, erreurs)`. Une ligne porte, par PR : le nombre de pertes non + exemptees et si le label serait pose. Un diff en erreur n'est JAMAIS + presente comme un zero mesure (#18761) : il est compte a part. + """ + since = now - dt.timedelta(hours=hours) + rows: list[dict] = [] + errors: list[str] = [] + for pr in fetch_merged(repo, since): + number = pr.get("number") + paths, added, renamed = _ipynb_by_change(pr) + 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. + rows.append({ + "number": number, + "notebooks": 0, + "added_notebooks": len(added), + "renamed_notebooks": len(renamed), + "credited_lost_unexempted": 0, + "credited_diff_errors": 0, + "would_label": False, + "paths": [], + }) + continue + base_ref = pr.get("baseRefOid") or "" + head_ref = pr.get("headRefOid") or "" + base = base_of(repo_dir, base_ref, head_ref) if base_ref and head_ref else None + if base is None: + # Repli nomme : on mesure contre la ref de base declaree, sans + # merge-base. Le chiffre reste exploitable, sa provenance est dite. + errors.append( + f"#{number}: merge-base indisponible pour " + f"{base_ref[:8]}...{head_ref[:8]} -- diff mesure contre la base declaree" + ) + base = base_ref + try: + body = fetch_body(repo, number) + except RuntimeError as exc: + errors.append(f"#{number}: body illisible ({exc})") + continue + result = check([Path(p) for p in paths], base_ref=base, pr_body=body) + summary = result.as_payload().get("summary", {}) + lost = int(summary.get("credited_lost_unexempted", 0) or 0) + diff_errors = int(summary.get("credited_diff_errors", 0) or 0) + rows.append({ + "number": number, + "notebooks": len(paths), + "added_notebooks": len(added), + "renamed_notebooks": len(renamed), + "credited_lost_unexempted": lost, + "credited_diff_errors": diff_errors, + # #18761 : le label ne se pose QUE si tous les diffs ont reussi. + "would_label": lost > 0 and diff_errors == 0, + "paths": paths, + }) + return rows, errors + + +def apply_label(repo: str, number: int, run=_run) -> bool: + """Pose le label sur la PR (idempotent cote GitHub). Rend True si pose.""" + proc = run([ + "gh", "pr", "edit", str(number), "--repo", repo, + "--add-label", LABEL_CREDITED_LOST, + ]) + return proc.returncode == 0 + + +def main(argv: list[str] | None = None) -> int: + ap = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + ap.add_argument("--repo", default=REPO_DEFAULT) + ap.add_argument("--repo-dir", default=".") + ap.add_argument("--hours", type=int, default=24) + ap.add_argument("--apply", action="store_true", + help="poser reellement le label (defaut: rapport seul)") + ap.add_argument("--json", action="store_true") + args = ap.parse_args(argv) + + now = dt.datetime.now(dt.timezone.utc) + try: + rows, errors = sweep(args.repo, Path(args.repo_dir), args.hours, now) + except RuntimeError as exc: + print(f"credited_examples_sweep: {exc}", file=sys.stderr) + return 2 + + labelled: list[int] = [] + if args.apply: + for row in rows: + if row["would_label"] and apply_label(args.repo, row["number"]): + labelled.append(row["number"]) + + if args.json: + json.dump({"prs": rows, "errors": errors, "labelled": labelled}, + sys.stdout, ensure_ascii=False) + print() + return 0 + + print(f"Fenetre : {args.hours} h -- {len(rows)} PR(s) mergee(s) touchant un .ipynb") + for row in rows: + mark = "LABEL" if row["would_label"] else " - " + added = row.get("added_notebooks", 0) + renamed = row.get("renamed_notebooks", 0) + 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)" + 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}") + for err in errors: + print(f" [WARN] {err}", file=sys.stderr) + total_lost = sum(r["credited_lost_unexempted"] for r in rows if r["would_label"]) + total_err = sum(r["credited_diff_errors"] for r in rows) + print(f"RESULT: {len(labelled) if args.apply else sum(1 for r in rows if r['would_label'])} " + f"PR(s) a labelliser, {total_lost} perte(s) non exemptee(s), " + f"{total_err} erreur(s) de diff, {len(errors)} repli(s) nomme(s)") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/tests/test_credited_examples_sweep.py b/scripts/tests/test_credited_examples_sweep.py new file mode 100644 index 0000000000..189f2e48f7 --- /dev/null +++ b/scripts/tests/test_credited_examples_sweep.py @@ -0,0 +1,238 @@ +#!/usr/bin/env python3 +"""Tests for `scripts/notebook_tools/credited_examples_sweep.py` (#19101). + +Le balayage est concu pour etre injectable : `sweep()` prend ses appels +reseau et git en parametres, donc tout se teste hors ligne. Les controles +epinglent les trois pieges qui rendraient le balayage trompeur : + + - un lot qui ATTEINT le plafond de l'API de recherche n'est pas un compte ; + - le label ne se pose que si TOUS les diffs ont reussi (#18761) ; + - un merge-base indisponible est NOMME, jamais un diff silencieux contre la + mauvaise base. +""" +from __future__ import annotations + +import datetime as dt +import importlib.util +import sys +from pathlib import Path + +_SCRIPT = Path(__file__).resolve().parent.parent / "notebook_tools" / "credited_examples_sweep.py" +_spec = importlib.util.spec_from_file_location("credited_examples_sweep", _SCRIPT) +assert _spec and _spec.loader, f"could not load {_SCRIPT}" +_mod = importlib.util.module_from_spec(_spec) +sys.modules["credited_examples_sweep"] = _mod +_spec.loader.exec_module(_mod) + +NOW = dt.datetime(2026, 10, 5, 12, 0, tzinfo=dt.timezone.utc) + + +class _FakeResult: + def __init__(self, lost=0, diff_errors=0): + self._s = { + "credited_lost_unexempted": lost, + "credited_diff_errors": diff_errors, + } + + def as_payload(self): + return {"summary": self._s} + + +def _pr(number, files=None, base="a" * 40, head="b" * 40): + return { + "number": number, + "baseRefOid": base, + "headRefOid": head, + "files": files or [], + "mergedAt": NOW.isoformat(), + } + + +def _nb(path, change="MODIFIED"): + return {"path": path, "changeType": change} + + +IPY = "MyIA.AI.Notebooks/Search/Part1/x.ipynb" +MD = "MyIA.AI.Notebooks/Search/Part1/README.md" + + +def _run_sweep(prs): + """Drive `sweep` with the network and git fully injected (offline).""" + return _mod.sweep( + "o/r", Path("."), 24, NOW, + fetch_merged=lambda repo, since: prs, + fetch_body=lambda repo, number: "", + base_of=lambda repo_dir, b, h: "deadbeef", + check=lambda paths, base_ref="", head_ref="", pr_body="": _FakeResult(), + ) + + +class TestFiltering: + def test_only_ipynb_paths_are_kept(self): + pr = _pr(1, [_nb(IPY), _nb(MD)]) + assert _mod.ipynb_paths(pr) == [IPY] + + def test_deleted_notebooks_are_excluded(self): + """Un carnet supprime n'a plus rien a compter (--diff-filter=d).""" + pr = _pr(1, [_nb(IPY, "DELETED")]) + assert _mod.ipynb_paths(pr) == [] + + def test_added_notebooks_are_not_measured(self): + """Un carnet neuf n'existe pas dans la base : `git show` sort en 128. + + Ce n'est PAS une erreur de mesure (#18761) -- et la compter comme telle + bloquait le label pour les carnets MODIFIES de la meme PR. + """ + pr = _pr(1, [_nb(IPY, "ADDED")]) + assert _mod.ipynb_paths(pr) == [] + modified, added, renamed = _mod._ipynb_by_change(pr) + assert modified == [] and added == [IPY] and renamed == [] + + def test_added_and_modified_split_in_the_same_pr(self): + other = "MyIA.AI.Notebooks/Search/Part1/y.ipynb" + pr = _pr(1, [_nb(other, "ADDED"), _nb(IPY, "MODIFIED")]) + modified, added, renamed = _mod._ipynb_by_change(pr) + assert modified == [IPY] and added == [other] and renamed == [] + + def test_a_purely_additive_pr_is_named_not_dropped(self): + rows, errs = _run_sweep([_pr(1, [_nb(IPY, "ADDED")])]) + assert len(rows) == 1 and rows[0]["notebooks"] == 0 + assert rows[0]["added_notebooks"] == 1 + assert rows[0]["would_label"] is False + assert errs == [] + + def test_a_renamed_notebook_is_declared_unmeasured_not_lost_free(self): + """Un renommage PEUT perdre des exemples : on ne le dit pas « sans perte ». + + La base est a un autre chemin (`previousFilename` non expose par + `gh pr view --json files`), donc la comparaison est impossible. Le + carnet ne doit ni etre mesure contre le mauvais chemin, ni etre tu. + """ + pr = _pr(1, [_nb(IPY, "RENAMED")]) + assert _mod.ipynb_paths(pr) == [] + modified, added, renamed = _mod._ipynb_by_change(pr) + assert modified == [] and added == [] and renamed == [IPY] + rows, errs = _run_sweep([pr]) + assert len(rows) == 1 and rows[0]["renamed_notebooks"] == 1 + assert rows[0]["notebooks"] == 0 + assert rows[0]["would_label"] is False + + def test_a_renamed_notebook_does_not_block_the_other_notebooks(self): + """Le faux positif d'erreur produisait un faux zero de pertes (#18761). + + Un carnet renomme dans la meme PR ne doit plus empecher la mesure des + carnets modifies -- c'est le defaut que la separation corrige. + """ + other = "MyIA.AI.Notebooks/Search/Part1/y.ipynb" + pr = _pr(1, [_nb(other, "RENAMED"), _nb(IPY, "MODIFIED")]) + + def check(paths, base_ref="", head_ref="", pr_body=""): + assert [str(p).endswith("x.ipynb") for p in paths] == [True], paths + return _FakeResult(lost=2, diff_errors=0) + + rows, errs = _mod.sweep( + "o/r", Path("."), 24, NOW, + fetch_merged=lambda repo, since: [pr], + fetch_body=lambda repo, number: "", + base_of=lambda *a: "deadbeef", + check=check, + ) + assert rows[0]["would_label"] is True, rows + assert rows[0]["renamed_notebooks"] == 1 + assert errs == [] + + def test_a_pr_without_notebook_is_skipped_by_the_sweep(self): + rows, errs = _run_sweep([_pr(1, [_nb(MD)])]) + assert rows == [] and errs == [] + + def test_modified_notebook_is_kept(self): + pr = _pr(1, [_nb(IPY, "MODIFIED")]) + assert _mod.ipynb_paths(pr) == [IPY] + + +class TestSearchCapIsNotACount: + def test_a_batch_at_the_cap_raises(self): + def run(argv): + return [{} for _ in range(_mod.SEARCH_RESULT_CAP)] + + try: + _mod.merged_prs("o/r", NOW - dt.timedelta(hours=24), run=run) + except RuntimeError as exc: + assert "plafond" in str(exc) + else: + raise AssertionError("un lot au plafond doit lever, pas passer") + + def test_a_batch_under_the_cap_passes(self): + rows = _mod.merged_prs("o/r", NOW - dt.timedelta(hours=24), + run=lambda argv: [{"number": 1}]) + assert rows == [{"number": 1}] + + def test_the_window_is_asked_on_merge_time(self): + seen = {} + + def run(argv): + seen["argv"] = argv + return [] + + _mod.merged_prs("o/r", NOW - dt.timedelta(hours=24), run=run) + argv = seen["argv"] + assert "--search" in argv + assert any(a.startswith("merged:>=") for a in argv), argv + assert argv[argv.index("--limit") + 1] == str(_mod.SEARCH_RESULT_CAP) + + +class TestSweepVerdicts: + def _one(self, lost, diff_errors, *, bases="ok"): + pr = _pr(1, [_nb(IPY)]) + + def fetch_merged(repo, since): + return [pr] + + def fetch_body(repo, number): + return "" + + def base_of(repo_dir, b, h): + return "deadbeef" if bases == "ok" else None + + def check(paths, base_ref="", head_ref="", pr_body=""): + return _FakeResult(lost=lost, diff_errors=diff_errors) + + return _mod.sweep("o/r", Path("."), 24, NOW, + fetch_merged=fetch_merged, fetch_body=fetch_body, + base_of=base_of, check=check) + + def test_a_loss_with_clean_diffs_is_labelled(self): + rows, errs = self._one(2, 0) + assert rows[0]["would_label"] is True + assert errs == [] + + def test_a_broken_diff_never_labels_even_with_a_loss(self): + """#18761 : un diff casse n'est pas un zero mesure, ni un label.""" + rows, errs = self._one(3, 1) + assert rows[0]["would_label"] is False + assert rows[0]["credited_lost_unexempted"] == 3 + + def test_zero_loss_does_not_label(self): + rows, _ = self._one(0, 0) + assert rows[0]["would_label"] is False + + def test_a_missing_merge_base_is_named_and_the_pr_is_still_measured(self): + rows, errs = self._one(1, 0, bases="missing") + assert len(errs) == 1 and "merge-base" in errs[0] + assert rows[0]["would_label"] is True + + def test_an_unreadable_body_is_named_and_the_pr_is_skipped(self): + pr = _pr(1, [_nb(IPY)]) + + def boom(repo, number): + raise RuntimeError("gh failed (1): not found") + + rows, errs = _mod.sweep( + "o/r", Path("."), 24, NOW, + fetch_merged=lambda repo, since: [pr], + fetch_body=boom, + base_of=lambda *a: "deadbeef", + check=lambda *a, **k: _FakeResult(1, 0), + ) + assert rows == [] + assert len(errs) == 1 and "body illisible" in errs[0] From 5cdd314bcee8d89a2959da2599ba308a378dc2e6 Mon Sep 17 00:00:00 2001 From: jsboige Date: Mon, 5 Oct 2026 08:58:21 +0200 Subject: [PATCH 2/3] =?UTF-8?q?fix(ci,#19101):=20sweep=20credited=20?= =?UTF-8?q?=E2=80=94=20changeType=20GraphQL,=20tete=20de=20PR,=20echec=20n?= =?UTF-8?q?on=20masque?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reponse a la review CHANGES_REQUESTED du 05/10 (5 conditions) : 1. changeType n'existe pas dans gh pr list --json files (mesure gh 2.83.2 sur #19040 : {additions, deletions, path} seulement). Les fichiers et leur nature viennent maintenant de GraphQL (pullRequest.files.nodes {path changeType}), pagines au curseur -- le repli « tout MODIFIED » classait GameTheory-18d (supprime par #19040) en modifie et levait FileNotFoundError, hors de tout garde. 2. check_notebooks recoit head_ref=headRefOid : le cote « apres » est la tete de la PR, pas l'arbre du moment du balayage. Le commit est amene par fetch-by-SHA s'il manque (PR squash-mergee) ; inatteignable = erreur nommee, PR ecartee -- jamais mesuree contre l'arbre du jour. 3. Tests : 17 -> 27. La forme reelle du lot (sans files/changeType) est desormais un fixture ; carnet supprime sans crash + carnets modifies de la meme PR toujours mesures ; pagination >100 fichiers ; tete inatteignable ; propagation de head_ref au check. Falsification : 3 mutants (branche DELETED retiree, head_ref non passe, pagination coupee) -> chacun cuche par au moins un test ; restore vert 27/27. 4. Le masque || echo "::warning::..." du workflow est retire : un plantage permanent laissait le run vert chaque nuit (classe de defaut #19214). set -euo pipefail ; sans effet merge (schedule/dispatch seulement). 5. Rejou de la mesure 24 h sur le corpus reel : dans le corps de la PR. Co-Authored-By: Claude Sonnet 5.5 --- .github/workflows/exercises-advisory.yml | 16 +- .../notebook_tools/credited_examples_sweep.py | 156 +++++++--- scripts/tests/test_credited_examples_sweep.py | 279 +++++++++++++----- 3 files changed, 337 insertions(+), 114 deletions(-) diff --git a/.github/workflows/exercises-advisory.yml b/.github/workflows/exercises-advisory.yml index 517a956658..bdf12d4ae3 100644 --- a/.github/workflows/exercises-advisory.yml +++ b/.github/workflows/exercises-advisory.yml @@ -269,9 +269,15 @@ jobs: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} GH_REPO: ${{ github.repository }} run: | - set -uo pipefail - # Advisory : un balayage qui echoue NOMME sa panne et ne rougit pas - # le run (le contrat de cet organe porte sur le verdict). Le repli et - # les erreurs de diff sont imprimes, jamais tus (#18761). + set -euo pipefail + # Le masque `|| echo "::warning::..."` est RETIRE (review #19215, + # point 3) : un plantage permanent laissait le run vert chaque nuit + # -- exactement la classe de defaut de #19214 (35 jours verts sans + # rapport). Le contrat de l'organe reste : ses pannes ATTENDUES + # (refs absentes, body illisible, plafond atteint) sont nommees en + # [WARN] et le script sort 0 ; ce qui ROUGIT ici est l'imprevu + # (traceback, crash) -- jamais masque. Sans effet merge : la mesure + # du body de #19214 vaut aussi pour ce workflow (schedule/dispatch + # seulement, hors du perimetre `merge_dwell`). python scripts/notebook_tools/credited_examples_sweep.py \ - --hours 24 --apply || echo "::warning::credited sweep failed -- nothing labelled" + --hours 24 --apply diff --git a/scripts/notebook_tools/credited_examples_sweep.py b/scripts/notebook_tools/credited_examples_sweep.py index d2aba25408..2eee6a01f2 100644 --- a/scripts/notebook_tools/credited_examples_sweep.py +++ b/scripts/notebook_tools/credited_examples_sweep.py @@ -7,30 +7,38 @@ #18761, mais **dormante** : elle exige `--base` ET `--pr-body-file`. Sous `schedule` -- le seul declencheur qui subsiste apres la tranche 1 de #12817 -- il n'y a pas de contexte PR, donc pas de body, donc pas de `--base` : le diff -des exemples credites n'etait **jamais** mesure et le label ne pouvait pas -etre pose. +des exemples credites n'etait **jamais** mesure et le label ne pouvait pas etre +pose. Ce balayage le reveille **sans reintroduire `pull_request`** sur ce workflow (interdit : le clone par PR etait la motivation de #12817). Il tourne apres 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) + +- **`changeType` n'existe pas dans `gh pr list --json files`** (mesure gh + 2.83.2 : cette forme ne rend que `{additions, deletions, path}`). Le + classifieur ADDED/DELETED/RENAMED se nourrit donc de **GraphQL** + (`pullRequest.files.nodes { path changeType }`), pagine au curseur. Un + 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 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`). + Un commit de tete inatteignable est **nomme**, jamais remplace par l'etat + du jour. + ## Portee honnete Post-mortem : il detecte les pertes **passees**, il ne protege pas le merge. C'est le compromis assume de l'option 1 ; l'option 2 (un declencheur PR leger) reste ouverte et n'est pas traitee ici. -Deux limites de la mesure, nommees plutot que tues : - - - la version HEAD est lue dans l'**arbre de travail** (defaut de - `check_notebooks`), donc une PR dont un carnet a ete re-touche par une PR - ulterieure est mesuree contre l'etat courant, pas contre son propre head. - Le chiffre reste une borne basse exploitable ; il n'est pas presente comme - « exactement l'apport de cette PR ». - - quand `headRefOid` n'est plus atteignable (ref supprimee au merge), le - merge-base est indisponible : la PR est alors mesuree **contre sa base - declaree** et le repli est imprime en `[WARN]`, jamais silencieux. +Quand `headRefOid` n'est plus atteignable (ref supprimee au merge), le +merge-base est indisponible : la PR est alors mesuree **contre sa base +declaree** et le repli est imprime en `[WARN]`, jamais silencieux. ## Usage @@ -63,6 +71,20 @@ # complet. SEARCH_RESULT_CAP = 1000 +# La liste des fichiers d'une PR AVEC leur nature de changement. Cette +# information n'existe pas dans `gh pr list --json files` (review 04:53Z, +# mesure gh 2.83.2) : elle est lue par GraphQL, page par 100. +GRAPHQL_FILES = """query($owner: String!, $name: String!, $num: Int!, $cursor: String) { + repository(owner: $owner, name: $name) { + pullRequest(number: $num) { + files(first: 100, after: $cursor) { + nodes { path changeType } + pageInfo { hasNextPage endCursor } + } + } + } +}""" + def _run(argv: list[str], **kw) -> subprocess.CompletedProcess: return subprocess.run( @@ -84,18 +106,19 @@ def _gh_json(argv: list[str]) -> object: def merged_prs(repo: str, since: dt.datetime, run=_gh_json) -> list[dict]: - """PRs mergees depuis `since`, avec leurs fichiers et leurs deux refs. + """PRs mergees depuis `since`, avec leurs deux refs -- PAS leurs fichiers. - Leve si le lot atteint le plafond de l'API de recherche : un corpus - tronque se lirait comme un corpus complet, et les PRs perdues sont les - plus anciennes de la fenetre. + La forme `gh pr list --json files` ne rend pas `changeType` : les fichiers + (et leur nature) sont lus par `pr_files`, par PR, en GraphQL. Leve si le + lot atteint le plafond de l'API de recherche : un corpus tronque se + lirait comme un corpus complet. """ stamp = since.strftime("%Y-%m-%dT%H:%M:%SZ") rows = run([ "pr", "list", "--repo", repo, "--state", "merged", "--limit", str(SEARCH_RESULT_CAP), "--search", f"merged:>={stamp}", - "--json", "number,baseRefOid,headRefOid,files,mergedAt", + "--json", "number,baseRefOid,headRefOid,mergedAt", ]) or [] if len(rows) >= SEARCH_RESULT_CAP: raise RuntimeError( @@ -106,23 +129,48 @@ def merged_prs(repo: str, since: dt.datetime, run=_gh_json) -> list[dict]: return rows -def _ipynb_by_change(pr: dict) -> tuple[list[str], list[str], list[str]]: +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). + """ + owner, name = repo.split("/", 1) + nodes: list[dict] = [] + cursor = None + while True: + payload = run([ + "api", "graphql", "-f", f"query={GRAPHQL_FILES}", + "-F", f"owner={owner}", "-F", f"name={name}", + "-F", f"num={number}", + ] + (["-f", f"cursor={cursor}"] if cursor else [])) + conn = (payload or {}).get("data", {}).get("repository", {}) \ + .get("pullRequest", {}).get("files", {}) + nodes.extend(conn.get("nodes") or []) + page = conn.get("pageInfo") or {} + if not page.get("hasNextPage"): + return nodes + cursor = page.get("endCursor") + + +def _ipynb_by_change(files: list[dict]) -> tuple[list[str], list[str], list[str]]: """`(modifies, ajoutes, renommes)` parmi les `.ipynb` de la PR. - Deux exclusions et une separation, toutes structurelles. Toutes viennent - d'un `git show :` qui sort en **128** parce que le carnet - n'est pas a ce chemin dans la base -- ce qui n'est PAS une erreur de - mesure, mais ne veut pas dire la meme chose selon le cas : + Deux exclusions et une separation, toutes structurelles, toutes lues de + `changeType` (GraphQL) : - `DELETED` : supprime, plus rien a compter (meme regle que le - `--diff-filter=d` du workflow) ; - - `ADDED` : neuf, donc **rien a perdre** par construction. Mesure du - 2026-10-05 : 6 des 6 « erreurs de diff » du corpus de 24 h venaient de - la (#18968, #19037, #19020, #19007, #18986) ; + `--diff-filter=d` du workflow). C'est le cas qui FAIT PLANTER le + balayage quand on le croit modifie : `git show :` puis + 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 - `gh pr view --json files` n'expose pas ici (`previousFilename` absent). - On ne peut donc pas la comparer -- et contrairement a `ADDED`, un - renommage **peut** perdre des exemples. On le declare NON MESURE. + 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. Pourquoi separer plutot que compter en erreur : `credited_diff_errors > 0` **bloque** la pose du label (#18761). Une erreur structurelle sur un carnet @@ -130,7 +178,7 @@ def _ipynb_by_change(pr: dict) -> tuple[list[str], list[str], list[str]]: faux positif d'erreur produisait un faux zero de pertes. """ modified, added, renamed = [], [], [] - for f in (pr.get("files") or []): + for f in (files or []): path = f.get("path") or "" if not path.endswith(".ipynb"): continue @@ -146,15 +194,15 @@ def _ipynb_by_change(pr: dict) -> tuple[list[str], list[str], list[str]]: return modified, added, renamed -def ipynb_paths(pr: dict) -> list[str]: +def ipynb_paths(files: list[dict]) -> list[str]: """Chemins `.ipynb` **modifies** par la PR (cf. `_ipynb_by_change`).""" - return _ipynb_by_change(pr)[0] + return _ipynb_by_change(files)[0] def merge_base(repo_dir: Path, base_ref: str, head_ref: str) -> str | None: """`git merge-base` des deux refs, ou None si l'une n'est pas atteignable. - Pour une PR deja mergee et squash-mergee, `headRefOid` peut avoir disparu + Pour une PR deja mergeee et squash-mergee, `headRefOid` peut avoir disparu du depot (ref supprimee). On rend None et l'appelant NOMME le repli -- jamais un diff silencieusement mesure contre la mauvaise base. """ @@ -164,14 +212,31 @@ def merge_base(repo_dir: Path, base_ref: str, head_ref: str) -> str | None: return proc.stdout.strip() or None +def ensure_commit(repo_dir: Path, sha: str, run=_run) -> bool: + """Ameine localement le commit de tete s'il manque, puis confirme. + + Une PR squash-mergee n'est pas un ancetre de `main` : son `headRefOid` + n'est pas dans un clone frais. GitHub autorise le fetch d'un SHA atteignable + depuis une ref de PR (`git fetch origin `). Rend False si le commit + reste inatteignable -- l'appelant NOMME l'echec, il ne mesure pas contre + un autre arbre. + """ + probe = ["git", "-C", str(repo_dir), "cat-file", "-e", f"{sha}^{{commit}}"] + if run(probe).returncode == 0: + return True + run(["git", "-C", str(repo_dir), "fetch", "--quiet", "origin", sha]) + return run(probe).returncode == 0 + + def pr_body(repo: str, number: int, run=_gh_json) -> str: payload = run(["pr", "view", str(number), "--repo", repo, "--json", "body"]) return ((payload or {}).get("body") or "") if isinstance(payload, dict) else "" def sweep(repo: str, repo_dir: Path, hours: int, now: dt.datetime, - *, fetch_merged=merged_prs, fetch_body=pr_body, - base_of=merge_base, check=check_notebooks) -> tuple[list[dict], list[str]]: + *, fetch_merged=merged_prs, fetch_files=pr_files, + fetch_body=pr_body, base_of=merge_base, ensure=ensure_commit, + check=check_notebooks) -> tuple[list[dict], list[str]]: """Rejoue le diff des exemples credites pour chaque PR mergee de la fenetre. Rend `(lignes, erreurs)`. Une ligne porte, par PR : le nombre de pertes non @@ -183,7 +248,12 @@ def sweep(repo: str, repo_dir: Path, hours: int, now: dt.datetime, errors: list[str] = [] for pr in fetch_merged(repo, since): number = pr.get("number") - paths, added, renamed = _ipynb_by_change(pr) + try: + files = fetch_files(repo, number) + except RuntimeError as exc: + errors.append(f"#{number}: fichiers illisibles ({exc})") + continue + paths, added, renamed = _ipynb_by_change(files) if not paths and not added and not renamed: continue if not paths: @@ -203,7 +273,16 @@ def sweep(repo: str, repo_dir: Path, hours: int, now: dt.datetime, continue base_ref = pr.get("baseRefOid") or "" head_ref = pr.get("headRefOid") or "" - base = base_of(repo_dir, base_ref, head_ref) if base_ref and head_ref else None + if not base_ref or not head_ref: + errors.append(f"#{number}: refs absentes (base={base_ref!r}, head={head_ref!r})") + continue + if not ensure(repo_dir, head_ref): + errors.append( + f"#{number}: commit de tete {head_ref[:8]} inatteignable -- " + "PR ecartee, pas mesuree contre l'arbre du jour" + ) + continue + base = base_of(repo_dir, base_ref, head_ref) if base is None: # Repli nomme : on mesure contre la ref de base declaree, sans # merge-base. Le chiffre reste exploitable, sa provenance est dite. @@ -217,7 +296,8 @@ def sweep(repo: str, repo_dir: Path, hours: int, now: dt.datetime, except RuntimeError as exc: errors.append(f"#{number}: body illisible ({exc})") continue - result = check([Path(p) for p in paths], base_ref=base, pr_body=body) + result = check([Path(p) for p in paths], base_ref=base, + head_ref=head_ref, pr_body=body) summary = result.as_payload().get("summary", {}) lost = int(summary.get("credited_lost_unexempted", 0) or 0) diff_errors = int(summary.get("credited_diff_errors", 0) or 0) diff --git a/scripts/tests/test_credited_examples_sweep.py b/scripts/tests/test_credited_examples_sweep.py index 189f2e48f7..bcac7db0fe 100644 --- a/scripts/tests/test_credited_examples_sweep.py +++ b/scripts/tests/test_credited_examples_sweep.py @@ -3,12 +3,20 @@ Le balayage est concu pour etre injectable : `sweep()` prend ses appels reseau et git en parametres, donc tout se teste hors ligne. Les controles -epinglent les trois pieges qui rendraient le balayage trompeur : - +epinglent les pieges qui rendraient le balayage trompeur, dont les trois +mesures de la review #19215 : + + - la forme REELLE de `gh pr list --json` ne porte NI `files` NI + `changeType` (mesure gh 2.83.2) : la nature des fichiers vient de + `pr_files`, par GraphQL pagine -- jamais du lot ; + - un carnet supprime (#19040 a supprime GameTheory-18d) est ecarte sans + planter, et n'empeche pas la mesure des carnets modifies de la PR ; + - le cote « apres » est la tete de la PR (`headRefOid`), pas l'arbre du + moment ; un commit de tete inatteignable est NOMME, la PR ecartee ; - un lot qui ATTEINT le plafond de l'API de recherche n'est pas un compte ; - le label ne se pose que si TOUS les diffs ont reussi (#18761) ; - - un merge-base indisponible est NOMME, jamais un diff silencieux contre la - mauvaise base. + - un merge-base indisponible est NOMME, jamais un diff silencieux contre + la mauvaise base. """ from __future__ import annotations @@ -26,6 +34,9 @@ NOW = dt.datetime(2026, 10, 5, 12, 0, tzinfo=dt.timezone.utc) +IPY = "MyIA.AI.Notebooks/Search/Part1/x.ipynb" +MD = "MyIA.AI.Notebooks/Search/Part1/README.md" + class _FakeResult: def __init__(self, lost=0, diff_errors=0): @@ -38,44 +49,132 @@ def as_payload(self): return {"summary": self._s} -def _pr(number, files=None, base="a" * 40, head="b" * 40): +class _Proc: + def __init__(self, rc): + self.returncode = rc + + +def _pr(number, base="a" * 40, head="b" * 40): + """La forme EXACTE rendue par `gh pr list --json number,baseRefOid,headRefOid,mergedAt` : + pas de cle `files` -- les fichiers (et leur `changeType`) viennent de + `pr_files`, en GraphQL. C'est l'ecart que masquaient les tests d'avant, + qui injectaient des lots deja classes.""" return { "number": number, "baseRefOid": base, "headRefOid": head, - "files": files or [], "mergedAt": NOW.isoformat(), } def _nb(path, change="MODIFIED"): + """Un noeud GraphQL `pullRequest.files.nodes { path changeType }`.""" return {"path": path, "changeType": change} -IPY = "MyIA.AI.Notebooks/Search/Part1/x.ipynb" -MD = "MyIA.AI.Notebooks/Search/Part1/README.md" +def _files_of(mapping): + def fetch_files(repo, number): + return mapping[number] + return fetch_files -def _run_sweep(prs): +def _sweep(prs, *, files=None, body=None, base_of=None, ensure=None, check=None): """Drive `sweep` with the network and git fully injected (offline).""" + if files is None: + files = {} + fetch_files = _files_of(files) if isinstance(files, dict) else files return _mod.sweep( "o/r", Path("."), 24, NOW, fetch_merged=lambda repo, since: prs, - fetch_body=lambda repo, number: "", - base_of=lambda repo_dir, b, h: "deadbeef", - check=lambda paths, base_ref="", head_ref="", pr_body="": _FakeResult(), + fetch_files=fetch_files, + fetch_body=body or (lambda repo, number: ""), + base_of=base_of or (lambda repo_dir, b, h: "deadbeef"), + ensure=ensure or (lambda repo_dir, sha: True), + check=check or (lambda paths, base_ref="", head_ref="", pr_body="": + _FakeResult()), ) +class TestGhFormIsNotTheFilesSource: + """Review #19215, point 1 : `gh pr list --json files` ne rend pas + `changeType` -- tout le classement ADDED/DELETED/RENAMED etait mort en + production (repli « tout MODIFIED »), et les tests d'avant l'ignoraient + en injectant des dicts deja classes.""" + + def test_the_real_gh_pr_list_form_has_no_files_and_still_measures(self): + pr = _pr(1) + assert "files" not in pr, "la forme reelle de gh pr list ne porte pas files" + rows, errs = _sweep([pr], files={1: [_nb(IPY)]}) + assert len(rows) == 1 and rows[0]["notebooks"] == 1 and errs == [] + + def test_merged_prs_asks_exactly_the_four_fields(self): + seen = {} + + def run(argv): + seen["argv"] = argv + return [] + + _mod.merged_prs("o/r", NOW - dt.timedelta(hours=24), run=run) + argv = seen["argv"] + assert argv[argv.index("--json") + 1] == "number,baseRefOid,headRefOid,mergedAt" + assert "files" not in argv[argv.index("--json") + 1] + + def test_an_unreadable_files_query_names_the_pr_and_continues(self): + def fetch_files(repo, number): + if number == 1: + raise RuntimeError("gh failed (1): graphql timeout") + return [_nb(IPY)] + + rows, errs = _sweep([_pr(1), _pr(2)], files=fetch_files) + assert [r["number"] for r in rows] == [2] + assert len(errs) == 1 and "fichiers illisibles" in errs[0] + + def test_pages_are_followed_to_the_cursor(self): + """Une PR de carnet peut deplacer plus de 100 fichiers : la requete + GraphQL pagine au curseur, et le lot est la concatenation des pages.""" + pages = [ + {"data": {"repository": {"pullRequest": {"files": { + "nodes": [{"path": f"n{i}.ipynb", "changeType": "MODIFIED"} + for i in range(100)], + "pageInfo": {"hasNextPage": True, "endCursor": "CUR1"}}}}}}, + {"data": {"repository": {"pullRequest": {"files": { + "nodes": [{"path": "last.ipynb", "changeType": "DELETED"}], + "pageInfo": {"hasNextPage": False}}}}}}, + ] + calls = [] + + def run(argv): + calls.append(argv) + return pages[len(calls) - 1] + + nodes = _mod.pr_files("o/r", 7, run=run) + assert len(nodes) == 101 + assert nodes[-1]["changeType"] == "DELETED" + assert "cursor=CUR1" in calls[1], f"le curseur de la 1re page doit servir : {calls[1]}" + assert "cursor=CUR1" not in calls[0] + + class TestFiltering: def test_only_ipynb_paths_are_kept(self): - pr = _pr(1, [_nb(IPY), _nb(MD)]) - assert _mod.ipynb_paths(pr) == [IPY] + assert _mod.ipynb_paths([_nb(IPY), _nb(MD)]) == [IPY] def test_deleted_notebooks_are_excluded(self): """Un carnet supprime n'a plus rien a compter (--diff-filter=d).""" - pr = _pr(1, [_nb(IPY, "DELETED")]) - assert _mod.ipynb_paths(pr) == [] + assert _mod.ipynb_paths([_nb(IPY, "DELETED")]) == [] + + def test_a_deleted_notebook_no_longer_crashes_the_sweep(self): + """Regression #19215 (cause 1 de la review) : GameTheory-18d, + supprime par #19040, etait classe MODIFIED (changeType absent du lot) + puis ouvert -> FileNotFoundError, hors de tout garde. Avec la source + GraphQL il est ecarte ET les carnets modifies de la meme PR restent + mesures.""" + gone = "MyIA.AI.Notebooks/GameTheory/GameTheory-18d-Humour-Banc-Dur-Python.ipynb" + rows, errs = _sweep( + [_pr(19040)], + files={19040: [_nb(gone, "DELETED"), _nb(IPY, "MODIFIED")]}, + ) + assert errs == [] + assert rows[0]["notebooks"] == 1 and rows[0]["paths"] == [IPY] def test_added_notebooks_are_not_measured(self): """Un carnet neuf n'existe pas dans la base : `git show` sort en 128. @@ -83,19 +182,17 @@ def test_added_notebooks_are_not_measured(self): Ce n'est PAS une erreur de mesure (#18761) -- et la compter comme telle bloquait le label pour les carnets MODIFIES de la meme PR. """ - pr = _pr(1, [_nb(IPY, "ADDED")]) - assert _mod.ipynb_paths(pr) == [] - modified, added, renamed = _mod._ipynb_by_change(pr) + modified, added, renamed = _mod._ipynb_by_change([_nb(IPY, "ADDED")]) assert modified == [] and added == [IPY] and renamed == [] def test_added_and_modified_split_in_the_same_pr(self): other = "MyIA.AI.Notebooks/Search/Part1/y.ipynb" - pr = _pr(1, [_nb(other, "ADDED"), _nb(IPY, "MODIFIED")]) - modified, added, renamed = _mod._ipynb_by_change(pr) + modified, added, renamed = _mod._ipynb_by_change( + [_nb(other, "ADDED"), _nb(IPY, "MODIFIED")]) assert modified == [IPY] and added == [other] and renamed == [] def test_a_purely_additive_pr_is_named_not_dropped(self): - rows, errs = _run_sweep([_pr(1, [_nb(IPY, "ADDED")])]) + rows, errs = _sweep([_pr(1)], files={1: [_nb(IPY, "ADDED")]}) assert len(rows) == 1 and rows[0]["notebooks"] == 0 assert rows[0]["added_notebooks"] == 1 assert rows[0]["would_label"] is False @@ -104,15 +201,13 @@ def test_a_purely_additive_pr_is_named_not_dropped(self): def test_a_renamed_notebook_is_declared_unmeasured_not_lost_free(self): """Un renommage PEUT perdre des exemples : on ne le dit pas « sans perte ». - La base est a un autre chemin (`previousFilename` non expose par - `gh pr view --json files`), donc la comparaison est impossible. Le - carnet ne doit ni etre mesure contre le mauvais chemin, ni etre tu. + La base est a un autre chemin (`previousFilename` hors du jeu GraphQL + demande ici), donc la comparaison est impossible. Le carnet ne doit ni + etre mesure contre le mauvais chemin, ni etre tu. """ - pr = _pr(1, [_nb(IPY, "RENAMED")]) - assert _mod.ipynb_paths(pr) == [] - modified, added, renamed = _mod._ipynb_by_change(pr) + modified, added, renamed = _mod._ipynb_by_change([_nb(IPY, "RENAMED")]) assert modified == [] and added == [] and renamed == [IPY] - rows, errs = _run_sweep([pr]) + rows, errs = _sweep([_pr(1)], files={1: [_nb(IPY, "RENAMED")]}) assert len(rows) == 1 and rows[0]["renamed_notebooks"] == 1 assert rows[0]["notebooks"] == 0 assert rows[0]["would_label"] is False @@ -124,30 +219,98 @@ def test_a_renamed_notebook_does_not_block_the_other_notebooks(self): carnets modifies -- c'est le defaut que la separation corrige. """ other = "MyIA.AI.Notebooks/Search/Part1/y.ipynb" - pr = _pr(1, [_nb(other, "RENAMED"), _nb(IPY, "MODIFIED")]) + seen = [] def check(paths, base_ref="", head_ref="", pr_body=""): - assert [str(p).endswith("x.ipynb") for p in paths] == [True], paths + seen.append([str(p) for p in paths]) return _FakeResult(lost=2, diff_errors=0) - rows, errs = _mod.sweep( - "o/r", Path("."), 24, NOW, - fetch_merged=lambda repo, since: [pr], - fetch_body=lambda repo, number: "", - base_of=lambda *a: "deadbeef", + rows, errs = _sweep( + [_pr(1)], + files={1: [_nb(other, "RENAMED"), _nb(IPY, "MODIFIED")]}, check=check, ) assert rows[0]["would_label"] is True, rows assert rows[0]["renamed_notebooks"] == 1 + assert seen == [[str(Path(IPY))]] assert errs == [] def test_a_pr_without_notebook_is_skipped_by_the_sweep(self): - rows, errs = _run_sweep([_pr(1, [_nb(MD)])]) + rows, errs = _sweep([_pr(1)], files={1: [_nb(MD)]}) assert rows == [] and errs == [] def test_modified_notebook_is_kept(self): - pr = _pr(1, [_nb(IPY, "MODIFIED")]) - assert _mod.ipynb_paths(pr) == [IPY] + assert _mod.ipynb_paths([_nb(IPY, "MODIFIED")]) == [IPY] + + +class TestHeadIsThePrHead: + """Review #19215, point 2 : le cote « apres » est la tete de la PR, pas + l'arbre de travail du moment du balayage.""" + + def test_the_check_receives_the_head_of_the_pr(self): + captured = {} + + def check(paths, base_ref="", head_ref="", pr_body=""): + captured["base_ref"] = base_ref + captured["head_ref"] = head_ref + return _FakeResult(1, 0) + + _sweep([_pr(1, base="a" * 40, head="c" * 40)], + files={1: [_nb(IPY)]}, check=check) + # cote « apres » : la TETE de la PR, pas l'arbre du moment + assert captured["head_ref"] == "c" * 40 + # cote « avant » : le merge-base rendu par base_of, pas la base brute + assert captured["base_ref"] == "deadbeef" + + def test_a_local_commit_short_circuits_without_fetch(self): + calls = [] + + def run(argv): + calls.append(argv) + return _Proc(0) + + assert _mod.ensure_commit(Path("."), "b" * 40, run=run) is True + assert len(calls) == 1, "le probe suffit, pas de fetch" + assert "cat-file" in calls[0] + + def test_a_missing_commit_is_fetched_then_reconfirmed(self): + seq = [_Proc(1), _Proc(0), _Proc(0)] # probe manque, fetch, probe ok + + def run(argv): + return seq.pop(0) + + assert _mod.ensure_commit(Path("."), "b" * 40, run=run) is True + assert not seq + + def test_a_still_missing_commit_returns_false(self): + seq = [_Proc(1), _Proc(0), _Proc(1)] # meme apres fetch, introuvable + + def run(argv): + return seq.pop(0) + + assert _mod.ensure_commit(Path("."), "b" * 40, run=run) is False + assert not seq + + def test_an_unreachable_head_names_the_pr_and_skips_the_measurement(self): + """Une PR squash-mergee dont le headRefOid a disparu n'est JAMAIS + mesuree contre l'arbre du jour -- elle est ecartee, en erreur nommee.""" + seen = [] + + def check(*a, **k): + seen.append(a) + return _FakeResult(9, 0) + + rows, errs = _sweep([_pr(1)], files={1: [_nb(IPY)]}, + ensure=lambda repo_dir, sha: False, check=check) + assert rows == [] and seen == [] + assert len(errs) == 1 and "inatteignable" in errs[0] + + def test_a_missing_merge_base_is_named_and_the_pr_is_still_measured(self): + rows, errs = _sweep([_pr(1)], files={1: [_nb(IPY)]}, + base_of=lambda repo_dir, b, h: None, + check=lambda *a, **k: _FakeResult(1, 0)) + assert len(errs) == 1 and "merge-base" in errs[0] + assert rows[0]["would_label"] is True class TestSearchCapIsNotACount: @@ -182,24 +345,11 @@ def run(argv): class TestSweepVerdicts: - def _one(self, lost, diff_errors, *, bases="ok"): - pr = _pr(1, [_nb(IPY)]) - - def fetch_merged(repo, since): - return [pr] - - def fetch_body(repo, number): - return "" - - def base_of(repo_dir, b, h): - return "deadbeef" if bases == "ok" else None - - def check(paths, base_ref="", head_ref="", pr_body=""): - return _FakeResult(lost=lost, diff_errors=diff_errors) - - return _mod.sweep("o/r", Path("."), 24, NOW, - fetch_merged=fetch_merged, fetch_body=fetch_body, - base_of=base_of, check=check) + def _one(self, lost, diff_errors): + return _sweep( + [_pr(1)], files={1: [_nb(IPY)]}, + check=lambda *a, **k: _FakeResult(lost, diff_errors), + ) def test_a_loss_with_clean_diffs_is_labelled(self): rows, errs = self._one(2, 0) @@ -216,23 +366,10 @@ def test_zero_loss_does_not_label(self): rows, _ = self._one(0, 0) assert rows[0]["would_label"] is False - def test_a_missing_merge_base_is_named_and_the_pr_is_still_measured(self): - rows, errs = self._one(1, 0, bases="missing") - assert len(errs) == 1 and "merge-base" in errs[0] - assert rows[0]["would_label"] is True - def test_an_unreadable_body_is_named_and_the_pr_is_skipped(self): - pr = _pr(1, [_nb(IPY)]) - def boom(repo, number): raise RuntimeError("gh failed (1): not found") - rows, errs = _mod.sweep( - "o/r", Path("."), 24, NOW, - fetch_merged=lambda repo, since: [pr], - fetch_body=boom, - base_of=lambda *a: "deadbeef", - check=lambda *a, **k: _FakeResult(1, 0), - ) + rows, errs = _sweep([_pr(1)], files={1: [_nb(IPY)]}, body=boom) assert rows == [] assert len(errs) == 1 and "body illisible" in errs[0] From 90b0435f1e942fe7dd96b99c6c201daf737b76c7 Mon Sep 17 00:00:00 2001 From: jsboige Date: Mon, 5 Oct 2026 09:40:12 +0200 Subject: [PATCH 3/3] fix(ci,#19215): le comptage lit la revision de la PR, et un carnet illisible n'emporte plus le balayage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review 5411248369, deux voies demandees, les deux faites. 1. `check_notebooks` comptait les exercices sur l'ARBRE DU JOUR (`count_exercises_in_notebook(path)`) alors que le diff credite lit deja le blob de `head_ref`. Consequence mesuree : #18788 MODIFIE ICT-45-InoculationBifurcation-9B, #19153 le RENOMME ensuite -> le chemin est MODIFIED mais absent de l'arbre, et le comptage levait un FileNotFoundError qui emportait TOUT le balayage (`--hours 72`, rc=1 : les autres PR de la fenetre n'etaient pas mesurees). Le comptage porte desormais sur le blob de tete, qui existe par construction pour un chemin MODIFIED. La classification reste sur le chemin d'origine : `classify_notebook` lit les regles de REPERTOIRE, que le fichier temporaire du blob ne porte pas. Effet de bord voulu, sur le meme chemin : meme quand le carnet existe dans l'arbre, c'est la revision de la PR qui est comptee -- l'ancien code pouvait mesurer un arbre different de celui qu'il comparait. 2. Le repli par PR : un echec de `check` est NOMME (« carnet illisible depuis l'arbre du jour (FileNotFoundError: ...) -- renomme ou supprime apres merge, PR ecartee ») et la suite de la fenetre est mesuree. La portee du `except` est etroite (`OSError`, `ValueError`) : un autre type d'echec remonte, pour qu'un bug du compteur ne se derobe pas en « carnet renomme ». Tests : 27 -> 32. Les nouveaux portent sur un depot git REEL (deux commits, renommage effectif), pas sur un dict injecte : carnet MODIFIED absent de l'arbre compte depuis le blob (3 exercices) ; l'arbre porte une autre version (1 exercice) et la tete gagne ; sans `head_ref` l'arbre sert encore (retro-compatibilite) ; un `check` qui leve est nomme et les autres PR sont mesurees ; un `RuntimeError` remonte au lieu d'etre absorbe. Falsification : 3 mutants, 3 rouges -- comptage remis sur l'arbre (2 tests), repli par PR retire (1), `except` elargi a `Exception` (1). Source restauree, 32 passed. Suivi RENAMED : issue #19251 ouverte avant merge (mesurer les renommages, `previousFilename` absent du jeu GraphQL). Co-Authored-By: Claude Sonnet 5.5 --- scripts/notebook_tools/check_pr_exercises.py | 23 ++- .../notebook_tools/credited_examples_sweep.py | 16 +- scripts/tests/test_credited_examples_sweep.py | 140 ++++++++++++++++++ 3 files changed, 176 insertions(+), 3 deletions(-) diff --git a/scripts/notebook_tools/check_pr_exercises.py b/scripts/notebook_tools/check_pr_exercises.py index 47486909fd..37a4e7d1b7 100644 --- a/scripts/notebook_tools/check_pr_exercises.py +++ b/scripts/notebook_tools/check_pr_exercises.py @@ -53,6 +53,7 @@ import argparse import json +import subprocess import sys from dataclasses import asdict, dataclass, field from pathlib import Path @@ -238,7 +239,27 @@ def check_notebooks( ) continue - cnt = count_exercises_in_notebook(path) + # #19215 (review 5411248369) : le comptage doit porter sur la MEME + # revision que le diff credite, pas sur l'arbre du jour. Un carnet + # MODIFIE par une PR puis RENOMME (ou supprime) par une PR suivante + # est absent de l'arbre : `count_exercises_in_notebook` levait un + # FileNotFoundError qui emportait tout le balayage -- les autres PR + # de la fenetre n'etaient pas mesurees. Le blob de tete, lui, existe + # par construction pour un chemin MODIFIED. + # + # La classification, elle, reste sur `path` : `classify_notebook` lit + # les regles de REPERTOIRE (`IIT/`, `groupe-`, `_`) et le fichier + # temporaire du blob ne les porte pas -- classify sur le blob + # reclasserait le carnet en `archive`/`tooling` sur son seul prefixe. + count_path = path + if head_ref and _HAS_CREDITED: + try: + count_path = _read_git_blob(head_ref, str(path).replace("\\", "/")) + except (subprocess.CalledProcessError, OSError): + # Blob absent de la tete : on garde l'arbre du jour. Si lui + # aussi manque, l'echec est nomme par l'appelant (sweep). + count_path = path + cnt = count_exercises_in_notebook(count_path) if cnt.parse_error is not None: result.parse_errors.append( NotebookVerdict( diff --git a/scripts/notebook_tools/credited_examples_sweep.py b/scripts/notebook_tools/credited_examples_sweep.py index 2eee6a01f2..ee723722f1 100644 --- a/scripts/notebook_tools/credited_examples_sweep.py +++ b/scripts/notebook_tools/credited_examples_sweep.py @@ -296,8 +296,20 @@ def sweep(repo: str, repo_dir: Path, hours: int, now: dt.datetime, except RuntimeError as exc: errors.append(f"#{number}: body illisible ({exc})") continue - result = check([Path(p) for p in paths], base_ref=base, - head_ref=head_ref, pr_body=body) + try: + result = check([Path(p) for p in paths], base_ref=base, + head_ref=head_ref, pr_body=body) + 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 + # fenetre. Nommee, la PR est ecartee et la suite est mesuree -- + # l'inverse d'un masque : le repli est compte et imprime. + errors.append( + f"#{number}: carnet illisible depuis l'arbre du jour " + f"({exc.__class__.__name__}: {exc}) -- renomme ou supprime " + "apres merge, PR ecartee" + ) + continue summary = result.as_payload().get("summary", {}) lost = int(summary.get("credited_lost_unexempted", 0) or 0) diff_errors = int(summary.get("credited_diff_errors", 0) or 0) diff --git a/scripts/tests/test_credited_examples_sweep.py b/scripts/tests/test_credited_examples_sweep.py index bcac7db0fe..b5e3c668e5 100644 --- a/scripts/tests/test_credited_examples_sweep.py +++ b/scripts/tests/test_credited_examples_sweep.py @@ -22,9 +22,17 @@ import datetime as dt import importlib.util +import json +import subprocess import sys + +import pytest from pathlib import Path +sys.path.insert(0, str(Path(__file__).resolve().parent.parent / "notebook_tools")) + +import check_pr_exercises as _cpe # noqa: E402 + _SCRIPT = Path(__file__).resolve().parent.parent / "notebook_tools" / "credited_examples_sweep.py" _spec = importlib.util.spec_from_file_location("credited_examples_sweep", _SCRIPT) assert _spec and _spec.loader, f"could not load {_SCRIPT}" @@ -373,3 +381,135 @@ def boom(repo, number): rows, errs = _sweep([_pr(1)], files={1: [_nb(IPY)]}, body=boom) assert rows == [] assert len(errs) == 1 and "body illisible" in errs[0] + + +class TestARenamedNotebookNoLongerTakesDownTheSweep: + """Review #19215 (5411248369), la mesure qui a fait tomber `--hours 72`. + + #18788 a MODIFIE ICT-45-InoculationBifurcation-9B ; #19153 l'a ensuite + RENOMME en ICT-42b. Pour #18788 le chemin est donc MODIFIED, mais il + n'existe plus dans l'arbre du jour : `check_notebooks` comptait les + exercices sur cet arbre et levait un FileNotFoundError, ce qui emportait + TOUT le balayage -- les autres PR de la fenetre n'etaient pas mesurees. + """ + + def test_a_failing_check_names_the_pr_and_the_others_are_still_measured(self): + def check(paths, base_ref="", head_ref="", pr_body=""): + if paths and "ICT-45" in str(paths[0]): + raise FileNotFoundError( + "MyIA.AI.Notebooks/IIT/ICT-Series/ICT-45-...ipynb") + return _FakeResult() + + rows, errs = _sweep( + [_pr(18788), _pr(18789)], + files={18788: [_nb("MyIA.AI.Notebooks/IIT/ICT-Series/ICT-45-x.ipynb")], + 18789: [_nb(IPY)]}, + check=check, + ) + assert [r["number"] for r in rows] == [18789], \ + "un carnet illisible ne doit pas emporter les autres PR" + assert len(errs) == 1 + assert errs[0].startswith("#18788:") + assert "carnet illisible" in errs[0] + assert "renomme ou supprime apres merge" in errs[0] + assert "FileNotFoundError" in errs[0], \ + "le nom de l'exception doit rester lisible dans le repli" + + def test_only_a_missing_notebook_is_named_other_errors_still_propagate(self): + """La portee du `except` est etroite : un bug du compteur ne doit pas + se derober en « carnet renomme ». Un repli trop large remplacerait une + panne par un chiffre manquant, ce que la fenetre ne distinguerait pas + d'une PR sans perte.""" + def check(paths, base_ref="", head_ref="", pr_body=""): + raise RuntimeError("bug du compteur") + + with pytest.raises(RuntimeError): + _sweep([_pr(7)], files={7: [_nb(IPY)]}, check=check) + + +def _notebook_json(n_exercises): + """Un carnet minimal : N paires (en-tete `### Exercice i` + stub TODO).""" + cells = [] + for i in range(1, n_exercises + 1): + cells.append({"cell_type": "markdown", "metadata": {}, + "source": [f"### Exercice {i}\n", "A completer.\n"]}) + cells.append({"cell_type": "code", "metadata": {}, "execution_count": None, + "outputs": [], "source": ["# TODO etudiant\n", + "result = None\n"]}) + return {"cells": cells, "metadata": {}, "nbformat": 4, "nbformat_minor": 5} + + +def _git(repo, *args): + subprocess.run( + ["git", "-C", str(repo), "-c", "user.name=t", "-c", "user.email=t@t", + "-c", "core.autocrlf=false", *args], + check=True, capture_output=True, text=True, + ) + + +class TestTheCountReadsTheRevisionNotTheTree: + """Review #19215 : `check_notebooks` comptait les exercices sur l'ARBRE DU + JOUR (`count_exercises_in_notebook(path)`), alors que le diff credite lit + deja le blob de `head_ref`. Deux consequences, une panne et un chiffre + faux -- les deux sont epinglees ici sur un depot git reel.""" + + REL = "MyIA.AI.Notebooks/Search/Part1/x.ipynb" + + def _repo_with_head(self, tmp_path, *, tree): + """Depot a deux commits ; la tete porte 3 exercices. + + ``tree`` : ce que l'arbre de travail contient a la fin -- ``None`` + (carnet renomme/supprime depuis), ou un carnet d'un autre nombre + d'exercices (l'arbre a bouge depuis le merge).""" + repo = tmp_path / "repo" + (repo / Path(self.REL).parent).mkdir(parents=True) + target = repo / self.REL + _git(repo, "init", "-q") + target.write_text(json.dumps(_notebook_json(1)), encoding="utf-8") + _git(repo, "add", "-A") + _git(repo, "commit", "-q", "-m", "base") + base = subprocess.run(["git", "-C", str(repo), "rev-parse", "HEAD"], + capture_output=True, text=True, check=True).stdout.strip() + target.write_text(json.dumps(_notebook_json(3)), encoding="utf-8") + _git(repo, "add", "-A") + _git(repo, "commit", "-q", "-m", "head") + head = subprocess.run(["git", "-C", str(repo), "rev-parse", "HEAD"], + capture_output=True, text=True, check=True).stdout.strip() + if tree is None: + target.unlink() + else: + target.write_text(json.dumps(_notebook_json(tree)), encoding="utf-8") + return repo, base, head + + def _count(self, result): + """Le compte du carnet mesure, quel que soit son seau.""" + for bucket in (result.ok, result.sub_threshold, result.parse_errors, + result.out_of_corpus): + for verdict in bucket: + if verdict.path.endswith("x.ipynb"): + return verdict.count + raise AssertionError("le carnet n'apparait dans aucun seau du resultat") + + def test_a_modified_notebook_absent_from_the_tree_is_counted_from_the_blob( + self, tmp_path, monkeypatch): + """Le cas #18788 puis #19153 : ICT-45 modifie, puis renomme.""" + repo, base, head = self._repo_with_head(tmp_path, tree=None) + monkeypatch.chdir(repo) + result = _cpe.check_notebooks([Path(self.REL)], base_ref=base, head_ref=head) + assert self._count(result) == 3, \ + "le compte doit venir du blob de tete, pas de l'arbre (absent)" + + def test_the_tree_version_never_wins_over_the_pr_head(self, tmp_path, monkeypatch): + """Meme quand le chemin existe, c'est la revision de la PR qui compte.""" + repo, base, head = self._repo_with_head(tmp_path, tree=1) + monkeypatch.chdir(repo) + result = _cpe.check_notebooks([Path(self.REL)], base_ref=base, head_ref=head) + assert self._count(result) == 3, \ + "1 exercice dans l'arbre, 3 dans la tete : la tete est la mesure" + + def test_without_a_head_ref_the_tree_still_serves(self, tmp_path, monkeypatch): + """Retro-compatibilite : l'appel sans tete (CLI locale) lit l'arbre.""" + repo, _base, _head = self._repo_with_head(tmp_path, tree=2) + monkeypatch.chdir(repo) + result = _cpe.check_notebooks([Path(self.REL)]) + assert self._count(result) == 2