fix(guard,#19475): etendre check-subprocess-encoding aux cellules .ipynb

.pre-commit-config.yaml

@@ -171,14 +171,19 @@ repos:
           universal_newlines=True) without encoding= in the same call. On a
           cp1252 host that raises UnicodeDecodeError on UTF-8 payloads — the
           exact class that killed the lane-claim guard in production (#12811).
-          Retroactively-clean-by-design: only files the commit touches are
-          scanned, so the gate is green on main while the historical sweep
-          lands tranche by tranche. No CI workflow counterpart on purpose:
-          #13097 CI famine — the check runs at commit time, zero queue slots.
+          Extended to .ipynb in #19475: the defect class lives almost
+          exclusively in code cells (19 sites fixed under the #15629 sweep —
+          Lean-03b, 12, 14, 15, 16a, 16b, 17c, 21c, 34, ...). Code cells are
+          extracted and scanned with the same predicate, reported as
+          ``path:cell_NN:line``. Retroactively-clean-by-design: only files
+          the commit touches are scanned, so the gate is green on main while
+          the historical sweep lands tranche by tranche. No CI workflow
+          counterpart on purpose: #13097 CI famine — the check runs at commit
+          time, zero queue slots.
         entry: python scripts/check_subprocess_encoding.py
         language: system
         pass_filenames: true
-        files: '\.py$'
+        files: '\.(py|ipynb)$'
 
       - id: cell-source-parses
         name: '#13326 — refuse un-compilable cell source (markdown-typed-code or syntax error)'

scripts/check_subprocess_encoding.py

@@ -21,17 +21,34 @@
 -- including when ``encoding=`` sits on a separate line of a multiline call.
 
 Usage (two modes):
-    python check_subprocess_encoding.py <file.py> [file2.py ...]
+    python check_subprocess_encoding.py <file.{py,ipynb}> [file2 ...]
         Scan the given files (pre-commit mode: pre-commit passes the staged
-        filenames).
+        filenames). For .ipynb inputs, every Python code cell whose source
+        contains a violating call is reported as ``path:cell_NN:line``.
 
     python check_subprocess_encoding.py --base origin/main
-        Scan every .py file changed between merge-base(REF, HEAD) and HEAD,
-        reading the working tree (in CI the checkout IS the head).
+        Scan every .py/.ipynb file changed between merge-base(REF, HEAD)
+        and HEAD, reading the working tree (in CI the checkout IS the head).
 
 Exit code 1 iff at least one violation is reported. Vendored / external
 subtrees (see EXCLUDE_MARKERS) are out of scope in both modes.
 
+# ``.ipynb`` rationale (#19475): the ratchet hook filters on
+# ``files: '\\.py$'``, but the defect class (cp1252 host decoding UTF-8
+# payload from a subprocess returning ``text=True`` without ``encoding=``)
+# lives almost exclusively in notebook code cells -- 19 sites were fixed
+# under the #15629 sweep (Lean-03b, 12, 14, 15, 16a, 16b, 17c, 21c, 34, ...),
+# every one in source of a cell. The extension reuses ``scan_source``
+# unchanged: a code cell's text is extracted
+# (``"".join(cell.get("source", []) or [])``), and any violation found
+# inside is reported with the cell index for the reviewer's eye. The
+# prose-suppression tokenize pass is reused as-is -- a markdown cell
+# carries no executable source, but if it were ever scanned the
+# suppression would still filter docstrings. The retroactive-clean
+# invariant is preserved: only the notebooks a commit touches are
+# scanned, so untouched notebooks with the historical defect stay
+# invisible to the gate.
+
 Known-good fix forms: ``encoding="utf-8", errors="replace"`` -- or the
 single-quote variant ``encoding='utf-8', errors='replace'`` when the call
 lives inside an f-string expression, where nested double quotes are a
@@ -143,44 +160,92 @@ def git_out(*args: str) -> str | None:
     return proc.stdout if proc.returncode == 0 else None
 
 
+def scan_ipynb(path: str) -> list[tuple[int, int, str]]:
+    """Return (cell_index, 1-based line, snippet) for each violating call found
+    inside any code-cell of a Jupyter notebook.
+
+    The notebook's ``cells`` list is walked; only cells with
+    ``cell_type == "code"`` are scanned (markdown cells carry prose the
+    prose-suppression pass would filter anyway, but we skip them to stay
+    consistent with the notebook's kernel contract). Each cell's ``source``
+    (a list of strings in nbformat) is joined and fed to ``scan_source``
+    unchanged -- the predicate is identical to the .py path. Findings are
+    reported with the cell index so the reviewer can navigate.
+
+    The retroactive invariant is the same as the .py ratchet: only notebooks
+    the commit touches are scanned, so untouched historical defects stay
+    invisible to the gate (#19475).
+    """
+    import json
+    findings: list[tuple[int, int, str]] = []
+    try:
+        nb = json.loads(Path(path).read_text(encoding="utf-8"))
+    except (OSError, UnicodeDecodeError, json.JSONDecodeError):
+        return findings  # unparseable notebook: skip silently (pre-commit path)
+    for idx, cell in enumerate(nb.get("cells", []) or []):
+        if cell.get("cell_type") != "code":
+            continue
+        src = "".join(cell.get("source", []) or [])
+        if not src.strip():
+            continue
+        for line, snippet in scan_source(src):
+            findings.append((idx, line, snippet))
+    return findings
+
+
 def changed_python_files(base: str) -> list[str]:
-    """Files changed between merge-base(base, HEAD) and HEAD (paths, .py only)."""
+    """Files changed between merge-base(base, HEAD) and HEAD
+    (paths for .py and .ipynb; other extensions are ignored)."""
     mb = git_out("merge-base", base, "HEAD")
     if mb is None:
         # No common ancestor (orphan branch): fall back to base tip.
         mb = base
     out = git_out("diff", "--name-only", "--diff-filter=AM", mb.strip(), "HEAD")
     if out is None:
         return []
+    keep = (".py", ".ipynb")
     return [l.strip() for l in out.splitlines()
-            if l.strip().endswith(".py") and not excluded(l.strip())]
+            if l.strip().endswith(keep) and not excluded(l.strip())]
 
 
 def main(argv: list[str] | None = None) -> int:
     p = argparse.ArgumentParser(
         description="Refuse NEW subprocess text=True calls without encoding=")
     p.add_argument("files", nargs="*", help="files to scan (pre-commit mode)")
     p.add_argument("--base", default=None, metavar="REF",
-                   help="scan .py files changed since merge-base(REF, HEAD)")
+                   help="scan .py/.ipynb files changed since merge-base(REF, HEAD)")
     args = p.parse_args(argv)
 
     if args.base:
         targets = changed_python_files(args.base)
     else:
-        targets = [f for f in args.files if f.endswith(".py") and not excluded(f)]
+        targets = [
+            f for f in args.files
+            if (f.endswith(".py") or f.endswith(".ipynb")) and not excluded(f)
+        ]
 
     violations = 0
+    py_count = 0
+    ipynb_count = 0
     for f in targets:
         path = Path(f)
         if not path.is_file():
             continue
         try:
-            src = path.read_text(encoding="utf-8")
+            text = path.read_text(encoding="utf-8")
         except (OSError, UnicodeDecodeError):
             continue
-        for line, snippet in scan_source(src):
-            violations += 1
-            print(f"{f}:{line}: text=True without encoding= :: {snippet}")
+        if f.endswith(".ipynb"):
+            ipynb_count += 1
+            for cell_idx, line, snippet in scan_ipynb(f):
+                violations += 1
+                print(f"{f}:cell_{cell_idx:02d}:{line}: "
+                      f"text=True without encoding= :: {snippet}")
+        else:
+            py_count += 1
+            for line, snippet in scan_source(text):
+                violations += 1
+                print(f"{f}:{line}: text=True without encoding= :: {snippet}")
 
     if violations:
         print(f"\n{violations} subprocess call(s) set text=True without "
@@ -189,7 +254,8 @@ def main(argv: list[str] | None = None) -> int:
               "inside f-string expressions).")
         return 1
     if args.base:
-        print(f"subprocess-encoding ratchet: {len(targets)} changed .py file(s), 0 violation(s)")
+        print(f"subprocess-encoding ratchet: {py_count} .py + {ipynb_count} "
+              f".ipynb = {len(targets)} changed file(s), 0 violation(s)")
     return 0
 
 

scripts/tests/test_check_subprocess_encoding.py

@@ -165,6 +165,105 @@ def test_main_base_mode(monkeypatch, capsys):
             "scripts/a.py\n_peters/b.py\nnotebook.ipynb\n",
     }[a])
     # No files on disk with those names -> 0 violations, exit 0, paths filtered.
+    # .ipynb now in scope (#19475): the diff yields 2 files (a.py + notebook.ipynb);
+    # both are non-existent on disk so neither is read -> 0 violations.
     assert cse.main(["--base", "origin/main"]) == 0
     out = capsys.readouterr().out
-    assert "1 changed .py file(s)" in out
+    assert "= 2 changed file(s)" in out
+    assert ".ipynb" in out
+
+
+# ---- #19475: ipynb code-cell extraction ----
+
+def _nb_for_write(cells, path):
+    """Tiny nbformat-shape writer for pre-commit testing. not Papermill 1:1,
+    only the keys scan_ipynb() reads (cells[*].cell_type and source list)."""
+    import json
+    path.write_text(json.dumps({"cells": cells}), encoding="utf-8")
+
+
+def test_scan_ipynb_clean_code_cell(tmp_path):
+    nb = tmp_path / "good.ipynb"
+    _nb_for_write([
+        {"cell_type": "markdown", "source": ["# prose about subprocess.run\n"]},
+        {"cell_type": "code",
+         "source": ["import subprocess\n",
+                    "subprocess.run(['x'], text=True, encoding='utf-8')\n"]},
+    ], nb)
+    assert cse.scan_ipynb(str(nb)) == []
+
+
+def test_scan_ipynb_violation_in_code_cell(tmp_path):
+    nb = tmp_path / "bad.ipynb"
+    _nb_for_write([
+        {"cell_type": "code",
+         "source": ["import subprocess\n",
+                    "subprocess.run(['x'], text=True)\n"]},
+    ], nb)
+    findings = cse.scan_ipynb(str(nb))
+    assert len(findings) == 1
+    cell_idx, line_no, _ = findings[0]
+    assert cell_idx == 0
+    assert line_no == 2  # the line of the call
+
+
+def test_scan_ipynb_skips_markdown_cells(tmp_path):
+    nb = tmp_path / "md.ipynb"
+    _nb_for_write([
+        {"cell_type": "markdown",
+         "source": ["def fake_subprocess_run(*a, text=True): pass\n",
+                    "fake_subprocess_run('x', text=True)\n"]},
+        {"cell_type": "code",
+         "source": ["# clean cell\n"]},
+    ], nb)
+    # The markdown cell's source IS valid Python in this test, but we don't
+    # care: scan_ipynb only walks code cells, so the markdown is skipped
+    # regardless of its content. The defect lives where it can be executed.
+    assert cse.scan_ipynb(str(nb)) == []
+
+
+def test_scan_ipynb_unparseable_is_skipped(tmp_path):
+    nb = tmp_path / "corrupt.ipynb"
+    nb.write_text("{not json", encoding="utf-8")
+    assert cse.scan_ipynb(str(nb)) == []
+
+
+def test_scan_ipynb_multiline_call_reports_call_line(tmp_path):
+    nb = tmp_path / "multi.ipynb"
+    _nb_for_write([
+        {"cell_type": "code",
+         "source": ["import subprocess\n",
+                    "proc = subprocess.run(\n",
+                    "    ['git', 'log'],\n",
+                    "    capture_output=True,\n",
+                    "    text=True,\n",
+                    ")\n"]},
+    ], nb)
+    findings = cse.scan_ipynb(str(nb))
+    assert len(findings) == 1
+    cell_idx, line_no, _ = findings[0]
+    assert cell_idx == 0
+    assert line_no == 2  # the line of the call, not of text=True
+
+
+def test_main_files_mode_mixed_py_ipynb(tmp_path, capsys):
+    bad_py = tmp_path / "bad.py"
+    bad_py.write_text("import subprocess\nsubprocess.run(['x'], text=True)\n",
+                      encoding="utf-8")
+    good_nb = tmp_path / "good.ipynb"
+    _nb_for_write([
+        {"cell_type": "code",
+         "source": ["import subprocess\n",
+                    "subprocess.run(['x'], text=True, encoding='utf-8')\n"]},
+    ], good_nb)
+    bad_nb = tmp_path / "bad.ipynb"
+    _nb_for_write([
+        {"cell_type": "code",
+         "source": ["import subprocess\n",
+                    "subprocess.run(['x'], text=True)\n"]},
+    ], bad_nb)
+    assert cse.main([str(bad_py), str(good_nb), str(bad_nb)]) == 1
+    out = capsys.readouterr().out
+    assert f"{bad_py}" in out
+    assert f"{good_nb}" not in out
+    assert f"{bad_nb}:cell_00:2" in out  # cell index in the report