Fix(tooling,#24434): write_notebook preserve metadata.papermill par defaut

scripts/notebook_tools/notebook_helpers.py

@@ -81,12 +81,31 @@ def _load(self) -> None:
         with open(self.path, 'r', encoding='utf-8', newline='') as f:
             self.notebook = json.load(f)
 
-    def save(self, path: Optional[str] = None) -> None:
-        """Save notebook JSON to file with LF line endings."""
+    def save(self, path: Optional[str] = None,
+             strip_stale_metadata: bool = False) -> None:
+        """Save notebook JSON to file with LF line endings.
+
+        Ne retire PAS ``metadata.papermill`` / ``metadata.execution`` par
+        defaut. Ce writer ne reexecute aucune cellule : il ne peut donc pas
+        fabriquer de bloc perime (#12722), il ne peut que detruire la
+        provenance d'un carnet deja execute. Mesure #24434 sur
+        ``SW-9-Python-JSONLD.ipynb`` : une edition d'une seule ligne de
+        source via ``CellIterator`` donnait 1 insertion et 984
+        suppressions -- 107 blocs ``metadata.papermill`` et 37 blocs
+        ``metadata.execution`` tombaient a 0 **sans qu'un seul
+        ``execution_count`` bouge**, donc les controles mecaniques de C.2
+        passaient et la perte restait invisible.
+
+        L'invariant #12722 reste tenu la ou il compte : les executeurs qui
+        reecrivent des sorties fraiches sans passer par Papermill appellent
+        ``strip_stale_papermill_metadata`` directement
+        (``dotnet_executor``, ``exec_single_cell``, ``exec_dotnet_persist``,
+        ``execute_qcpy_docker``). Passer ``strip_stale_metadata=True``
+        uniquement pour un appelant qui vient d'ecrire des sorties fraiches.
+        """
         save_path = Path(path) if path else self.path
-        # Les sorties fraîches décrivent CE run : un bloc papermill hérité
-        # d'une passe antérieure les daterait au mauvais run (#12722).
-        strip_stale_papermill_metadata(self.notebook)
+        if strip_stale_metadata:
+            strip_stale_papermill_metadata(self.notebook)
         with open(save_path, 'w', encoding='utf-8', newline='\n') as f:
             json.dump(self.notebook, f, indent=1, ensure_ascii=False)
             f.write('\n')
@@ -703,9 +722,16 @@ def apply_correction(self, new_source: str) -> None:
         """Apply a correction to the cell source."""
         self.helper.set_cell_source(self.cell_index, new_source)
 
-    def save(self) -> None:
-        """Save the notebook with corrections."""
-        self.helper.save()
+    def save(self, strip_stale_metadata: bool = False) -> None:
+        """Save the notebook with corrections.
+
+        Cet iterateur ne reexecute rien : il delegue a
+        ``NotebookHelper.save()``, dont le strip est opt-in depuis #24434
+        (une correction de source detruisait la provenance papermill du
+        carnet). Les executeurs, eux, appellent
+        ``strip_stale_papermill_metadata`` directement.
+        """
+        self.helper.save(strip_stale_metadata=strip_stale_metadata)
 
     def get_summary(self) -> Dict[str, Any]:
         """Get summary of the iteration process."""
@@ -1421,9 +1447,21 @@ def read_notebook(path: str) -> Dict[str, Any]:
     return helper.notebook
 
 
-def write_notebook(path: str, notebook: Dict[str, Any]) -> None:
-    """Write notebook JSON to file."""
-    strip_stale_papermill_metadata(notebook)
+def write_notebook(path: str, notebook: Dict[str, Any], strip_stale_metadata: bool = False) -> None:
+    """Write notebook JSON to file.
+
+    Ne retire PAS ``metadata.papermill`` / ``metadata.execution`` par defaut :
+    ce writer ne reexecute aucune cellule, donc il ne peut pas fabriquer de
+    bloc perime -- il ne peut que detruire la provenance d'un carnet deja
+    execute (mesure #24434 : une edition markdown-only de 2 phrases
+    supprimait 344 lignes de metadonnees papermill). Les executeurs qui
+    reecrivent des sorties sans passer par Papermill appellent
+    ``strip_stale_papermill_metadata`` directement (cf. ``_papermill_meta``).
+    Passer ``strip_stale_metadata=True`` uniquement si l'appelant vient
+    d'ecrire des sorties fraiches.
+    """
+    if strip_stale_metadata:
+        strip_stale_papermill_metadata(notebook)
     with open(path, 'w', encoding='utf-8', newline='\n') as f:
         json.dump(notebook, f, indent=1, ensure_ascii=False)
         f.write('\n')

scripts/notebook_tools/tests/test_write_notebook_preserves_papermill.py

@@ -0,0 +1,117 @@
+"""Tests for write_notebook papermill metadata preservation (#24434).
+
+``write_notebook`` est un writer generique : il ne reexecute aucune cellule et
+ne doit donc PAS retirer ``metadata.papermill`` / ``metadata.execution`` --
+seuls les executeurs qui reecrivent des sorties strippent (cf. ``_papermill_meta``,
+cable directement dans ``dotnet_executor``, ``exec_single_cell``, etc.).
+
+Cas fondateur : une edition markdown-only de 2 phrases via ``write_notebook``
+supprimait 344 lignes de metadonnees papermill (PR #24411), rendant la preuve
+d'execution invisible alors que ``execution_count`` et ``outputs`` restaient
+en place.
+"""
+
+import sys
+import unittest
+from pathlib import Path
+
+sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
+
+from notebook_helpers import read_notebook, write_notebook  # noqa: E402
+
+
+def _nb_with_papermill():
+    return {
+        "cells": [
+            {
+                "cell_type": "code",
+                "execution_count": 1,
+                "id": "aaa",
+                "metadata": {
+                    "execution": {
+                        "iopub.status.idle": "2026-11-03T08:36:38.519650Z",
+                        "shell.execute_reply": "2026-11-03T08:36:38.519003Z",
+                    },
+                    "papermill": {
+                        "duration": 0.009247,
+                        "end_time": "2026-11-03T08:36:38.520588+00:00",
+                        "exception": False,
+                        "start_time": "2026-11-03T08:36:38.511341+00:00",
+                        "status": "completed",
+                    },
+                },
+                "outputs": [{"name": "stdout", "output_type": "stream", "text": ["ok\n"]}],
+                "source": ["print('ok')\n"],
+            },
+            {
+                "cell_type": "markdown",
+                "id": "bbb",
+                "metadata": {
+                    "papermill": {
+                        "duration": 0.003066,
+                        "end_time": "2026-11-03T08:36:38.508118+00:00",
+                        "exception": False,
+                        "start_time": "2026-11-03T08:36:38.505052+00:00",
+                        "status": "completed",
+                    }
+                },
+                "source": ["### Interpretation\n"],
+            },
+        ],
+        "metadata": {"kernelspec": {"display_name": "Python 3", "language": "python", "name": "python3"}},
+        "nbformat": 4,
+        "nbformat_minor": 5,
+    }
+
+
+class TestWriteNotebookPreservesPapermill(unittest.TestCase):
+    def test_markdown_only_edit_preserves_metadata(self):
+        import tempfile
+
+        with tempfile.TemporaryDirectory() as td:
+            path = str(Path(td) / "nb.ipynb")
+            nb = _nb_with_papermill()
+            # edition markdown-only : on remplace la source de la cellule 1
+            nb["cells"][1]["source"] = ["### Interpretation corrigee\n"]
+            write_notebook(path, nb)
+
+            reread = read_notebook(path)
+            cell_code = reread["cells"][0]
+            cell_md = reread["cells"][1]
+            self.assertIn("papermill", cell_code["metadata"], "le bloc papermill du code doit survivre")
+            self.assertIn("execution", cell_code["metadata"], "le bloc execution du code doit survivre")
+            self.assertIn("papermill", cell_md["metadata"], "le bloc papermill du markdown doit survivre")
+            self.assertEqual(
+                cell_code["metadata"]["papermill"]["end_time"],
+                "2026-11-03T08:36:38.520588+00:00",
+            )
+            self.assertEqual("".join(cell_md["source"]), "### Interpretation corrigee\n")
+
+    def test_optin_strip_still_strips(self):
+        import tempfile
+
+        with tempfile.TemporaryDirectory() as td:
+            path = str(Path(td) / "nb.ipynb")
+            nb = _nb_with_papermill()
+            write_notebook(path, nb, strip_stale_metadata=True)
+
+            reread = read_notebook(path)
+            for cell in reread["cells"]:
+                self.assertNotIn("papermill", cell.get("metadata", {}))
+
+    def test_default_roundtrip_is_byte_stable(self):
+        import tempfile
+
+        with tempfile.TemporaryDirectory() as td:
+            path = str(Path(td) / "nb.ipynb")
+            nb = _nb_with_papermill()
+            write_notebook(path, nb)
+            first = Path(path).read_text(encoding="utf-8")
+            # relecture + reecriture sans edition : byte-identique
+            write_notebook(path, read_notebook(path))
+            second = Path(path).read_text(encoding="utf-8")
+            self.assertEqual(first, second)
+
+
+if __name__ == "__main__":
+    unittest.main()

scripts/tests/test_papermill_meta_strip.py

@@ -71,23 +71,65 @@ def _papermill_block_of(path: Path):
 
 # --- notebook_helpers : executeur de kernel generique (2 points d'ecriture) ---
 
-def test_notebook_helpers_save_strips_stale_block(tmp_path):
+def test_notebook_helpers_save_strip_is_optin(tmp_path):
+    """#24434, second site : le strip de save() est opt-in, comme celui de
+    write_notebook.
+
+    Ce test disait l'inverse jusqu'au 2026-11-04. Le contrat a change parce
+    que la mesure l'a contredit : ``save()`` n'a **aucun** appelant executeur
+    (mesure du 04/11 : seuls la docstring du module, la delegation de
+    ``NotebookCellCorrector.save()`` et les tests l'appellent), tandis que
+    les quatre executeurs qui reecrivent des sorties fraiches appellent
+    ``strip_stale_papermill_metadata`` directement (``dotnet_executor``,
+    ``exec_single_cell``, ``exec_dotnet_persist``, ``execute_qcpy_docker``).
+    Le strip inconditionnel ne protegeait donc aucun run reel, et detruisait
+    la provenance a chaque edition markdown-only : mesure sur
+    ``SW-9-Python-JSONLD.ipynb``, 1 insertion / 984 suppressions, 107 blocs
+    ``metadata.papermill`` et 37 blocs ``metadata.execution`` tombes a 0
+    sans qu'un seul ``execution_count`` bouge.
+    """
     from notebook_helpers import NotebookHelper
     p = _write_fake_nb(tmp_path / "nb.ipynb")
     helper = NotebookHelper(str(p))
     helper.notebook["cells"][0]["execution_count"] = 1
     helper.save()
+    assert _papermill_block_of(p) is not None, (
+        "NotebookHelper.save() sans re-execution ne doit PAS retirer le bloc "
+        "papermill (provenance du dernier run, #24434)")
+    helper.save(strip_stale_metadata=True)
     assert _papermill_block_of(p) is None, (
-        "NotebookHelper.save() doit retirer le bloc perime : les sorties "
-        "fraiches decrivent CE run, pas la passe papermill du 2026-05-31")
+        "l'opt-in strip_stale_metadata=True doit retirer le bloc perime : "
+        "c'est le chemin des executeurs qui ont reecrit des sorties")
+
+
+def test_cell_iterator_save_preserves_papermill(tmp_path):
+    """Le troisieme chemin (``CellIterator.save()``) heritait du strip via sa
+    delegation : il detruisait la provenance sur une simple correction de
+    source. Il doit desormais la preserver par defaut."""
+    from notebook_helpers import CellIterator
+    p = _write_fake_nb(tmp_path / "nb.ipynb")
+    iterator = CellIterator(str(p), cell_index=0, objective="noop")
+    iterator.save()
+    assert _papermill_block_of(p) is not None, (
+        "CellIterator.save() corrige une source, il ne reexecute rien : la "
+        "provenance papermill doit survivre (#24434)")
 
 
-def test_notebook_helpers_write_notebook_strips_stale_block(tmp_path):
+def test_notebook_helpers_write_notebook_strip_is_optin(tmp_path):
     from notebook_helpers import write_notebook
     p = _write_fake_nb(tmp_path / "nb.ipynb")
     nb = json.loads(p.read_text(encoding="utf-8"))
+    # Depuis #24434, write_notebook est un writer generique : le strip est
+    # opt-in (strip_stale_metadata=True). Les executeurs qui reecrivent les
+    # sorties strippent explicitement ; une ecriture sans re-execution
+    # preserve la provenance papermill du carnet.
     write_notebook(str(p), nb)
-    assert _papermill_block_of(p) is None
+    assert _papermill_block_of(p) is not None, (
+        "write_notebook sans re-execution ne doit PAS retirer le bloc "
+        "papermill (provenance du dernier run, #24434)")
+    write_notebook(str(p), nb, strip_stale_metadata=True)
+    assert _papermill_block_of(p) is None, (
+        "l'opt-in strip_stale_metadata=True doit retirer le bloc perime")
 
 
 # --- exec_dotnet_persist : point d'ecriture extrait (kernel .NET requis sinon) ---