From eef5bbf34df0e5367a1bcb6b38533e13db03e48a Mon Sep 17 00:00:00 2001 From: jsboige Date: Tue, 3 Nov 2026 12:13:07 +0100 Subject: [PATCH 1/3] Fix(tooling,#24434): write_notebook preserve metadata.papermill par defaut MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit write_notebook est un writer generique (docstring : « Write notebook JSON to file ») mais appelait strip_stale_papermill_metadata sans condition : toute edition de source — markdown compris — effaçait les blocs metadata.papermill et metadata.execution de toutes les cellules, laissant execution_count et outputs en place. Le defaut etait silencieux : les controles mecaniques de C.2 passaient, et le diff ressemblait a une remise en forme. Cas fondateur mesure sur #24411 : une edition de deux phrases supprimait 344 lignes de metadonnees. Le strip reste correct pour son public documente (_papermill_meta : « partage par executeurs ») — un executeur qui reecrit les sorties doit retirer le bloc de la passe anterieure. Un writer qui ne reexecute rien ne peut pas fabriquer de bloc perime ; il ne peut que detruire la provenance d'un carnet deja execute. Le strip devient opt-in (strip_stale_metadata=False par defaut). Aucun executeur ne depend du chemin retire : dotnet_executor:360, exec_dotnet_persist:25, exec_single_cell:172, execute_qcpy_docker:46 et NotebookExecutor:88 appellent strip_stale_papermill_metadata directement. write_notebook n'a aucun appelant de production dans scripts/ (seul _archive/_fix_gt15b_compilation.py). Tests : 3 nouveaux (edition markdown-only preserve les metadonnees ; opt-in strip toujours actif ; roundtrip byte-stable), 158 existants helpers+strip et 44 dotnet_executor rejoues verts. Co-Authored-By: Claude Sonnet 5.5 --- scripts/notebook_tools/notebook_helpers.py | 18 ++- ...test_write_notebook_preserves_papermill.py | 117 ++++++++++++++++++ 2 files changed, 132 insertions(+), 3 deletions(-) create mode 100644 scripts/notebook_tools/tests/test_write_notebook_preserves_papermill.py diff --git a/scripts/notebook_tools/notebook_helpers.py b/scripts/notebook_tools/notebook_helpers.py index cee77eca646..72c3afb4add 100644 --- a/scripts/notebook_tools/notebook_helpers.py +++ b/scripts/notebook_tools/notebook_helpers.py @@ -1421,9 +1421,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') diff --git a/scripts/notebook_tools/tests/test_write_notebook_preserves_papermill.py b/scripts/notebook_tools/tests/test_write_notebook_preserves_papermill.py new file mode 100644 index 00000000000..4575b529116 --- /dev/null +++ b/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() From 65c24dc467967b6397c176348e4e55bbc0f62437 Mon Sep 17 00:00:00 2001 From: jsboige Date: Tue, 3 Nov 2026 13:02:51 +0100 Subject: [PATCH 2/3] Fix(tooling,#24434): le test legacy suit le contrat opt-in de write_notebook test_notebook_helpers_write_notebook_strips_stale_block codifiait le defaut lui-meme : il exigeait le strip inconditionnel que #24434 retire. Repointe sur le nouveau contrat, les deux sens verifies : sans re-execution le bloc papermill survit (provenance du dernier run), l'opt-in strip_stale_metadata=True le retire. Renomme ..._strip_is_optin pour que le nom dise le contrat. Rejoue localement : test_papermill_meta_strip.py 7 passed ; helpers+strip+preserves 161 passed. Co-Authored-By: Claude Sonnet 5.5 --- scripts/tests/test_papermill_meta_strip.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/scripts/tests/test_papermill_meta_strip.py b/scripts/tests/test_papermill_meta_strip.py index daf869ef4bd..d75ead39a9e 100644 --- a/scripts/tests/test_papermill_meta_strip.py +++ b/scripts/tests/test_papermill_meta_strip.py @@ -82,12 +82,21 @@ def test_notebook_helpers_save_strips_stale_block(tmp_path): "fraiches decrivent CE run, pas la passe papermill du 2026-05-31") -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) --- From c3b6c9eb6a805bbefd729fea2bd64878a5b3496c Mon Sep 17 00:00:00 2001 From: jsboige Date: Wed, 4 Nov 2026 13:36:22 +0100 Subject: [PATCH 3/3] Fix(tooling,#24434): strip papermill opt-in sur NotebookHelper.save() (second site) Le premier correctif de #24434 avait rendu le strip opt-in dans write_notebook(). Le meme defaut vivait un etage plus bas, dans NotebookHelper.save(), et c'est le chemin que les lanes prennent pour une edition markdown-only -- donc celui qui detruit le plus souvent. Mesure (rapportee par ai-01:CoursIA-2 sur SW-9-Python-JSONLD.ipynb, et reproduite ici) : une edition d'une seule ligne de source donnait 1 insertion et 984 suppressions ; 107 blocs metadata.papermill et 37 blocs metadata.execution tombaient a 0 alors que les 37 execution_count non nuls ne bougeaient pas -- les controles mecaniques de C.2 passaient, la perte restait invisible. Ce que la mesure a etabli avant de changer le contrat : save() n'a AUCUN appelant executeur dans le depot (seuls la docstring du module, la delegation de CellIterator.save() et les tests l'appellent), tandis que les quatre executeurs qui reecrivent des sorties fraiches appellent deja strip_stale_papermill_metadata directement -- dotnet_executor, exec_single_cell, exec_dotnet_persist, execute_qcpy_docker. L'invariant #12722 n'etait donc protege par ce strip nulle part, et il le reste : les executeurs continuent de stripper explicitement. - NotebookHelper.save(path=None, strip_stale_metadata=False) - CellIterator.save(strip_stale_metadata=False), qui delegue - test_papermill_meta_strip.py : le garde qui affirmait l'inverse est retourne en connaissance de cause, avec la mesure qui le contredit, et deux tests remplacent l'ancien (save() preserve ; l'opt-in strippe ; CellIterator.save() preserve). Verification : 158 tests verts sur test_papermill_meta_strip.py + test_write_notebook_preserves_papermill.py + test_notebook_helpers.py. Sonde empirique sur une copie de SW-9 : avant/apres identiques (107 blocs papermill, 37 execution_count non nuls), la ou le defaut donnait 107 -> 0. Co-Authored-By: Claude Sonnet 5.5 --- scripts/notebook_tools/notebook_helpers.py | 42 +++++++++++++++++----- scripts/tests/test_papermill_meta_strip.py | 39 ++++++++++++++++++-- 2 files changed, 70 insertions(+), 11 deletions(-) diff --git a/scripts/notebook_tools/notebook_helpers.py b/scripts/notebook_tools/notebook_helpers.py index 72c3afb4add..f6fab79cb14 100644 --- a/scripts/notebook_tools/notebook_helpers.py +++ b/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.""" diff --git a/scripts/tests/test_papermill_meta_strip.py b/scripts/tests/test_papermill_meta_strip.py index d75ead39a9e..af68b1913b2 100644 --- a/scripts/tests/test_papermill_meta_strip.py +++ b/scripts/tests/test_papermill_meta_strip.py @@ -71,15 +71,48 @@ 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_strip_is_optin(tmp_path):