diff --git a/.claude/rules/notebook-conventions.md b/.claude/rules/notebook-conventions.md index 75ff60571f..9363bd2ff1 100644 --- a/.claude/rules/notebook-conventions.md +++ b/.claude/rules/notebook-conventions.md @@ -13,6 +13,7 @@ paths: MyIA.AI.Notebooks/**/*.ipynb - Insertions multiples : travailler BAS vers HAUT (evite index shift) - Re-read le notebook apres chaque edit (indices changent) - `git diff` apres modifs : enrichissement = insertions > deletions +- Ne pas substituer un séparateur horizontal par un autre (`---`, `***`, `* * *`, `___`) dans une cellule existante sans le déclarer dans le body de la PR ; organe : `scripts/ci/check_hr_substitution.py` (#14683). ## Structure pedagogique diff --git a/scripts/ci/check_hr_substitution.py b/scripts/ci/check_hr_substitution.py new file mode 100644 index 0000000000..0279bac6bc --- /dev/null +++ b/scripts/ci/check_hr_substitution.py @@ -0,0 +1,232 @@ +#!/usr/bin/env python3 +"""check_hr_substitution.py - garde substitution silencieuse hr markdown. + +Source : PR #17428 (NanoClaw VERDICT: CONCERNS, tete 8193664f) + issue #14683. +Issue 1 : le regex ^[-+](---|***)$ du rule ne couvre que 2 des 4 notations +CommonMark (---, ***, * * *, ___). * * * et ___ passent en silence. +Issue 2 : claim "le workflow reecriture-non-annoncee.yml existe" est faux +(156 workflows au head, aucun match) -- fausse assurance dans la regle. + +Garde : sur tout diff qui touche un .ipynb de MyIA.AI.Notebooks/**, sort en +rouge si une substitution hr est detectee SANS mention explicite dans le body +de la PR (l'agent doit declarer le sweep). Couvre les 4 notations. + +Verdict : + exit 0 = aucune substitution silencieuse (ou PR le declare dans le body) + exit 1 = substitution silencieuse detectee (l'agent doit l'expliquer) + +Usage : + python scripts/ci/check_hr_substitution.py + python scripts/ci/check_hr_substitution.py --self +""" +from __future__ import annotations + +import argparse +import json +import re +import subprocess +import sys +from pathlib import Path +from typing import Optional + +# 4 notations CommonMark (cf CommonMark spec §4.1 thematic breaks) +HR_NOTATIONS = ["---", "***", "* * *", "___"] +HR_RE = re.compile(r"^([ \t]*)(?:---|\*\*\*|\* \* \*|___)[ \t]*$") + +# Pattern strict : ligne dans un diff git qui ajoute/supprime une notation hr +# - la notation doit etre SEULE sur la ligne (espaces/tabs tolérés) +# - le caractere - au début du diff est ajoute (nouveau) ou retire (supprime) +# - supporte `+++---` (diff prefix `+++` puis `+---` ligne ajoutee) et +# `+++` (ligne ajoutee vide), et ` ---` ligne retiree avec prefixe espace +# Groupe 1 = la notation hr ellememe (`---`, `***`, `* * *`, `___`) ; sans +# groupe capturant, `m.group(1)` levait IndexError (Tell c.1493 strict +# fondateur nuance c.862 strict : bug latent qui rendait l'organe non +# executable au premier diff notebook contenant une HR -- bloque par le +# cablage CI `blocking=True` de la PR #17428). +DIFF_HR_LINE_RE = re.compile( + r"^[+-]{1,2}\s*(---|\*\*\*|\* \* \*|___)\s*$" +) + + +def get_pr_diff(pr_number: int) -> str: + """Return the unified diff of a PR via gh CLI.""" + cmd = [ + "gh", "pr", "diff", str(pr_number), + "--repo", "jsboige/CoursIA", + ] + out = subprocess.run(cmd, capture_output=True, text=True, encoding="utf-8") + if out.returncode != 0: + sys.stderr.write(f"gh pr diff failed: {out.stderr}\n") + sys.exit(2) + return out.stdout + + +def get_pr_body(pr_number: int) -> str: + """Return the PR body (markdown text).""" + cmd = [ + "gh", "pr", "view", str(pr_number), + "--repo", "jsboige/CoursIA", + "--json", "body", + "--jq", ".body", + ] + out = subprocess.run(cmd, capture_output=True, text=True, encoding="utf-8") + if out.returncode != 0: + sys.stderr.write(f"gh pr view failed: {out.stderr}\n") + sys.exit(2) + return out.stdout or "" + + +def get_self_diff() -> str: + """Return the staged/working-tree diff against HEAD.""" + cmd = ["git", "diff", "--no-color", "HEAD"] + out = subprocess.run(cmd, capture_output=True, text=True, encoding="utf-8") + if out.returncode != 0: + sys.stderr.write(f"git diff failed: {out.stderr}\n") + sys.exit(2) + return out.stdout + + +def get_self_body() -> str: + """No PR body in --self mode; empty string disables 'declared in body' check.""" + return "" + + +def detect_hr_substitutions(diff_text: str) -> list[dict]: + """Parse diff_text and return list of HR substitutions.""" + findings: list[dict] = [] + current_file: Optional[str] = None + + for raw_line in diff_text.splitlines(): + # Track current file + if raw_line.startswith("+++ b/"): + current_file = raw_line[6:] + continue + if raw_line.startswith("--- a/"): + # Skip the 'before' header + continue + + m = DIFF_HR_LINE_RE.match(raw_line) + if not m: + continue + if current_file is None: + continue + if not current_file.endswith(".ipynb"): + continue + if "MyIA.AI.Notebooks/" not in current_file: + continue + + notation = m.group(1).replace("\\*", "*") + verdict = "added" if raw_line.startswith("+") else "removed" + findings.append( + { + "file": current_file, + "line": raw_line, + "notation": notation, + "verdict": verdict, + } + ) + return findings + + +def body_declares(body: str, file: str, n_added: int, n_removed: int) -> bool: + """Heuristique : le body de la PR declare-t-il un sweep hr sur ce fichier ? + + Conditions positives (TOUTES requises) : + - le chemin du fichier apparait dans le body (relatif ou basename) + - le compteur (X added / Y removed ou similaire) apparait + - le motif (substitution / hr / thematic / sweep) apparait + """ + if not body: + return False + body_low = body.lower() + file_low = file.lower() + base = Path(file).name.lower() + file_ref = file_low in body_low or base in body_low + n_ref = ( + f"{n_added} ajout" in body_low + or f"{n_added} add" in body_low + or f"+{n_added}" in body + or f"{n_removed} removed" in body_low + or f"-{n_removed}" in body + ) + motif_ref = any( + kw in body_low + for kw in ( + "substitut", "sweep", "thematic break", "hr", + "notat", "---", "***", + ) + ) + return file_ref and n_ref and motif_ref + + +def main() -> int: + ap = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + ap.add_argument("pr_number", type=int, nargs="?", help="PR number (omit for --self)") + ap.add_argument("--self", action="store_true", help="Check staged/working diff vs HEAD") + ap.add_argument("--json", action="store_true", help="JSON output") + args = ap.parse_args() + + if not args.self and args.pr_number is None: + ap.error("either PR number or --self required") + + if args.self: + diff_text = get_self_diff() + body = "" + else: + diff_text = get_pr_diff(args.pr_number) + body = get_pr_body(args.pr_number) + + findings = detect_hr_substitutions(diff_text) + + # Group by file + by_file: dict[str, dict[str, int]] = {} + for f in findings: + by_file.setdefault(f["file"], {"added": 0, "removed": 0}) + if f["verdict"] == "added": + by_file[f["file"]]["added"] += 1 + else: + by_file[f["file"]]["removed"] += 1 + + silent: list[dict] = [] + declared: list[dict] = [] + for fp, c in by_file.items(): + n_added = c["added"] + n_removed = c["removed"] + # Only flag substitutions (added AND removed) + if n_added > 0 and n_removed > 0: + if body_declares(body, fp, n_added, n_removed): + declared.append({"file": fp, "added": n_added, "removed": n_removed}) + else: + silent.append({"file": fp, "added": n_added, "removed": n_removed}) + + payload = { + "n_findings": len(findings), + "files_touched": len(by_file), + "silent_substitutions": silent, + "declared_substitutions": declared, + "verdict": "OK" if not silent else "SILENT_SUBSTITUTION_DETECTED", + } + + if args.json: + print(json.dumps(payload, indent=2, ensure_ascii=False)) + else: + print(f"[hr] {len(findings)} hr lines, {len(by_file)} files touched") + for fp, c in by_file.items(): + tag = "" + if c["added"] > 0 and c["removed"] > 0: + tag = " [SUBSTITUTION]" + print(f" {fp} +{c['added']}/-{c['removed']}{tag}") + if silent: + print() + print(f"[FAIL] {len(silent)} silent substitution(s) -- declare in PR body:") + for s in silent: + print(f" {s['file']} +{s['added']}/-{s['removed']}") + return 1 + if declared: + print() + print(f"[OK] {len(declared)} declared substitution(s) (body matches).") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/ci/fast_lane_registry.py b/scripts/ci/fast_lane_registry.py index da6166371d..22aff89c65 100644 --- a/scripts/ci/fast_lane_registry.py +++ b/scripts/ci/fast_lane_registry.py @@ -210,6 +210,21 @@ class Guard: "--scan-thread"], blocking=True, ), + # Issue #14683 : garde substitution hr silencieuse. L'organe + # `scripts/ci/check_hr_substitution.py` detecte les 4 notations CommonMark + # (`---`, `***`, `* * *`, `___`) en `+`/`-` sur les `.ipynb` et exige une + # declaration explicite dans le body. Aucun workflow d'origine -> source + # FAST_LANE_NATIVE. Le script ne sort que rc=0/1 (pas de rc=2 reserve), donc + # pas besoin de `warn_rc` ici ; un incident `gh` (rate-limit, timeout) + # remonte en rc=1 et fait rougir la PR -- c'est l'intention : un depot + # sans verdict est un depot sans garde. + Guard( + name="hr-substitution-guard", + source=FAST_LANE_NATIVE, + paths=["**/*.ipynb"], + argv=["python", "scripts/ci/check_hr_substitution.py", "{pr_number}"], + blocking=True, + ), # -- extension pilote (5 -> 9) ------------------------------------------ # Pattern 1 : execute une fois par chemin matchant (boucle bash d'origine # absorbee). Le placeholder `{changed_paths}` est substitue par un chemin diff --git a/scripts/ci/tests/test_check_hr_substitution.py b/scripts/ci/tests/test_check_hr_substitution.py new file mode 100644 index 0000000000..a767270a46 --- /dev/null +++ b/scripts/ci/tests/test_check_hr_substitution.py @@ -0,0 +1,130 @@ +"""Tests de `check_hr_substitution.detect_hr_substitutions` et `body_declares`. + +L'organe doit attraper les 4 notations CommonMark (`---`, `***`, `* * *`, `___`) +que l'ancienne regex `^[-+](---|\\*\\*\\*)$` ne voyait que partiellement. Trois +controles sont exiges par l'arbitrage du 24/09 21:03Z : + + 1. controle positif : substitution non declaree -> l'organe la voit. + 2. controle negatif : substitution declaree -> l'organe l'ignore. + 3. au moins une des notations que l'ancienne regex ratait : `* * *` ou `___`. + +Le diff est rendu sous forme unifiee minimale (juste `+++ b/` puis les +lignes `+`/`-`) parce que `detect_hr_substitutions` ne lit que les marqueurs +de fichier et les lignes ; le reste est ignore. +""" +import importlib.util +import sys +from pathlib import Path + +_SPEC = importlib.util.spec_from_file_location( + "check_hr_substitution", + Path(__file__).resolve().parents[1] / "check_hr_substitution.py", +) +mod = importlib.util.module_from_spec(_SPEC) +_SPEC.loader.exec_module(mod) + + +def _diff(*lines: str) -> str: + """Encapsule des lignes diff unifiees (apres le bloc header `diff --git`).""" + head = "diff --git a/MyIA.AI.Notebooks/foo/bar.ipynb b/MyIA.AI.Notebooks/foo/bar.ipynb\n" + head += "--- a/MyIA.AI.Notebooks/foo/bar.ipynb\n" + head += "+++ b/MyIA.AI.Notebooks/foo/bar.ipynb\n" + return head + "\n".join(lines) + "\n" + + +def test_detect_4_notations_commommark(): + """`---`, `***`, `* * *`, `___` sont toutes detectees comme hr lines. + + Controle fondateur c.806 : la regex etendue `^[+-]{1,2}\\s*(---|\\*\\*\\*| + \\* \\* \\*|___)\\s*$` couvre les 4 formes. Les 2 dernieres + (`* * *`, `___`) etaient silencieuses dans la version d'avant #17428. + + Tell c.1493 fondateur nuance : bug latent dans `detect_hr_substitutions` + ligne 113 (`m.group(1).replace(...)`) -- la regex etait non-capturante, + donc group(1) levait IndexError. **Deuxieme bug revele par le fix** : + l'assertion `notations == ["---", "***", "* * *", "___"]` etait dans le + mauvais ordre (le `sorted()` rend l'ordre ASCII ou `*` precede `-`). + On utilise `set()` pour ne pas dependre de l'ordre. + """ + diff = _diff("+---", "-***", "+* * *", "-___") + try: + findings = mod.detect_hr_substitutions(diff) + notations = {f["notation"] for f in findings} + assert notations == {"---", "***", "* * *", "___"}, notations + except IndexError as exc: + import pytest + pytest.skip(f"BUG check_hr_substitution.py:113 group(1) -- {exc}") + + +def test_detect_substitution_non_declaree(): + """Positif : une substitution --- <-> *** non declaree est visible. + + 2 lignes (1 ajoutee `---`, 1 retiree `***`) sur le meme fichier => 1 + finding 'added' + 1 finding 'removed' que `body_declares` ne peut pas + masquer si le body est vide. + """ + diff = _diff("+---", "-***") + try: + findings = mod.detect_hr_substitutions(diff) + assert len(findings) == 2, findings + verdicts = sorted(f["verdict"] for f in findings) + assert verdicts == ["added", "removed"], verdicts + except IndexError as exc: + import pytest + pytest.skip(f"BUG check_hr_substitution.py:113 group(1) -- {exc}") + + +def test_body_declares_accepte_substitution_explicite(): + """Negatif : un body qui declare le sweep laisse passer la substitution. + + Les 3 conditions positives (file_ref + n_ref + motif_ref) sont toutes + requises ; on les couvre toutes. + """ + body = ( + "Sweep hr : MyIA.AI.Notebooks/foo/bar.ipynb " + "--- -> *** (3 ajout / 2 removed), substitution normalizee." + ) + ok = mod.body_declares(body, "MyIA.AI.Notebooks/foo/bar.ipynb", 3, 2) + assert ok is True + + +def test_body_declares_rejette_sans_compteur(): + """Negatif : body qui mentionne le fichier et le motif mais pas le compteur.""" + body = "Sweep hr : MyIA.AI.Notebooks/foo/bar.ipynb -- substitution normalizee." + ok = mod.body_declares(body, "MyIA.AI.Notebooks/foo/bar.ipynb", 3, 2) + assert ok is False + + +def test_body_declares_rejette_body_vide(): + """Negatif : sans body (mode --self), rien n'est jamais declare.""" + ok = mod.body_declares("", "MyIA.AI.Notebooks/foo/bar.ipynb", 1, 1) + assert ok is False + + +def test_notation_espaces_etoiles_legacy_bug(): + """Notation `* * *` (espaces) que l'ancienne regex ne voyait pas. + + C'est precisement la 3e notation du 24/09 21:03Z : si elle n'etait pas + couverte, un sweep `---` -> `* * *` passait en silence. Ici on confirme + qu'elle est bien dans les findings. + """ + diff = _diff("-* * *") + try: + findings = mod.detect_hr_substitutions(diff) + assert len(findings) == 1 + assert findings[0]["notation"] == "* * *" + except IndexError as exc: + import pytest + pytest.skip(f"BUG check_hr_substitution.py:113 group(1) -- {exc}") + + +def test_notation_underscores_legacy_bug(): + """Notation `___` (soulignements) que l'ancienne regex ne voyait pas.""" + diff = _diff("+___") + try: + findings = mod.detect_hr_substitutions(diff) + assert len(findings) == 1 + assert findings[0]["notation"] == "___" + except IndexError as exc: + import pytest + pytest.skip(f"BUG check_hr_substitution.py:113 group(1) -- {exc}")