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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .claude/rules/notebook-conventions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
232 changes: 232 additions & 0 deletions scripts/ci/check_hr_substitution.py
Original file line number Diff line number Diff line change
@@ -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 <PR_NUMBER>
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())
15 changes: 15 additions & 0 deletions scripts/ci/fast_lane_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
130 changes: 130 additions & 0 deletions scripts/ci/tests/test_check_hr_substitution.py
Original file line number Diff line number Diff line change
@@ -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/<f>` 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}")
Loading