From 49569dad3650093d27e78dc62890b4e81b498ca4 Mon Sep 17 00:00:00 2001 From: jsboige Date: Wed, 2 Sep 2026 22:06:07 +0200 Subject: [PATCH] feat(guards,#14297): content-based pairing fallback for no-id bases The id pairing of #14319 covers nbformat 4.5+ bases; legacy bases without cell ids still paired positionally, so an enrichment insertion shifted indices and confronted unrelated C.1 stubs whose uniform outputs are byte-identical - the exact 3/3 FP class the issue measured. Two passes over canonical sources: exact matches first (a moved-unmodified cell pairs with its own base copy -> UNCHANGED), then a difflib fuzzy pass (ratio >= 0.75, greedy descending) so a moved-AND-modified cell still confronts its own base version (STALE_OUTPUT). A head cell matching nothing stays UNPAIRED. Acceptance #14297 sec3 pinned: moved-only -> UNCHANGED, moved-and-modified -> STALE_OUTPUT, modified stub next to an identical -output sibling pairs to its own base. Founding #13550 positive control survives untouched (26/26). Co-Authored-By: Claude-Code --- .../check_source_output_ratchet.py | 70 ++++++++++++++++-- .../tests/test_check_source_output_ratchet.py | 73 ++++++++++++++++++- 2 files changed, 134 insertions(+), 9 deletions(-) diff --git a/scripts/notebook_tools/check_source_output_ratchet.py b/scripts/notebook_tools/check_source_output_ratchet.py index fdb4ad150f..4578ccae54 100644 --- a/scripts/notebook_tools/check_source_output_ratchet.py +++ b/scripts/notebook_tools/check_source_output_ratchet.py @@ -53,9 +53,11 @@ notebook carries ids, so inserted cells (fresh ids, unpaired) or shifted cells (stable ids, paired to their real base partner) no longer fabricate source-changed/outputs-identical pairs on enrichment PRs -(#14297). Legacy notebooks whose base carries no id pair by position -over the full cell list, as before - an insertion there still shifts -later indices, and the body door lifts any residual pair. +(#14297). Legacy notebooks whose base carries no id pair by CONTENT: +exact source matches first (a moved-unmodified cell pairs with its own +base copy), then a difflib fuzzy pass (ratio >= 0.75, greedy) so a +moved-AND-modified cell still confronts its own base version; a head +cell matching nothing stays UNPAIRED - never a fabricated stale pair. Usage: python check_source_output_ratchet.py [--json] [--body-file F] @@ -72,6 +74,7 @@ """ import argparse +import difflib import json import re import subprocess @@ -97,6 +100,13 @@ # deliberately absent - see module docstring. NON_EXECUTABLE_KERNELS = set(ALLOW_NULL_EXEC_COUNT_KERNELS) | {"lean"} +# Fuzzy-pairing floor for legacy bases without ids (#14297): below it a +# head code cell stays UNPAIRED rather than confronting a base cell it +# merely resembles. High on purpose - two distinct C.1 stubs share enough +# boilerplate that a lax floor would re-fabricate the exact stale pair +# the content pairing exists to kill. +FUZZY_PAIR_RATIO = 0.75 + # "Source-output ratchet: [12] exempte" / "Source-output ratchet: # Path/To.ipynb: [12] exempte" _EXEMPT_RE = re.compile( @@ -190,6 +200,48 @@ def parse_body_exemptions(body_text): return lifted +def _pair_by_content(base_cells, head_cells): + """Content-based code-cell pairing for legacy bases without ids. + + Two passes over canonical sources. Exact pass first: a cell that only + MOVED pairs with its own byte-identical base copy (the #14297 FP class + - enrichment insertions shift indices, positional pairing then + confronted unrelated C.1 stubs whose uniform outputs are identical). + Fuzzy pass second (difflib ratio, descending, greedy): a MOVED AND + MODIFIED cell still confronts its own base version - without it, the + fallback would disarm the guard on the very PRs it polices. A head + cell with no partner in either pass stays unpaired. + """ + base_code = [(j, canonical_source(c)) + for j, c in enumerate(base_cells) + if c.get("cell_type") == "code"] + head_code = [(i, canonical_source(c)) + for i, c in enumerate(head_cells) + if c.get("cell_type") == "code"] + pairs = {} + free_base = dict(base_code) + for i, src in head_code: + for j, bsrc in free_base.items(): + if src == bsrc: + pairs[i] = j + del free_base[j] + break + candidates = [] + for i, src in head_code: + if i in pairs: + continue + for j, bsrc in free_base.items(): + ratio = difflib.SequenceMatcher( + None, bsrc, src, autojunk=False).ratio() + if ratio >= FUZZY_PAIR_RATIO: + candidates.append((ratio, i, j)) + for _, i, j in sorted(candidates, key=lambda t: (-t[0], t[1], t[2])): + if i not in pairs and j in free_base: + pairs[i] = j + del free_base[j] + return pairs + + def classify_cells(base_nb, head_nb): """Per-code-cell records of one notebook pair, indexed over ALL cells. @@ -203,10 +255,14 @@ def classify_cells(base_nb, head_nb): # nbformat 4.5+ stamps every cell with a stable `id`: pair by id so an # insertion shifts indices but never the pairing itself (#14297 - the # positional pairing fabricated 3/3 STALE_OUTPUT on enrichment PRs). - # Legacy notebooks whose base carries no id keep the positional - # pairing, and a head cell with a fresh id (inserted) has no partner. + # A head cell with a fresh id (inserted) has no partner. Legacy + # notebooks whose base carries no id pair by CONTENT (#14297 fallback: + # exact then fuzzy) - positional pairing there re-fabricated stale + # pairs on every enrichment insertion. base_by_id = {c.get("id"): c for c in base_cells if c.get("id")} use_ids = bool(base_by_id) + content_pairs = (None if use_ids + else _pair_by_content(base_cells, head_cells)) records = [] for i, hcell in enumerate(head_cells): if hcell.get("cell_type") != "code": @@ -218,8 +274,8 @@ def classify_cells(base_nb, head_nb): bcell = base_by_id[hid] elif not hid: bcell = base_cells[i] if i < len(base_cells) else None - else: - bcell = base_cells[i] if i < len(base_cells) else None + elif content_pairs and i in content_pairs: + bcell = base_cells[content_pairs[i]] if bcell is None or bcell.get("cell_type") != "code": records.append({"index": i, "verdict": "UNPAIRED", "regression": False}) diff --git a/scripts/tests/test_check_source_output_ratchet.py b/scripts/tests/test_check_source_output_ratchet.py index 5b4b0813b1..7694d1b275 100644 --- a/scripts/tests/test_check_source_output_ratchet.py +++ b/scripts/tests/test_check_source_output_ratchet.py @@ -207,11 +207,25 @@ def test_all_cell_indexing_survives_markdown_edits(self): self.assertEqual(recs[-1]["index"], 26) def test_inserted_cell_is_unpaired_not_stale(self): - # A cell inserted before a code cell shifts it: no base partner at - # that index -> UNPAIRED (clean), never a fabricated stale pair. + # A cell inserted before a code cell shifts it. Without ids, the + # legacy content pairing (#14297 fallback) pairs the moved + # UNMODIFIED cell with its own base copy -> UNCHANGED (clean), + # never a fabricated stale pair. Same semantics as the id branch + # in test_shifted_cells_pair_by_id_after_conforming_insertion. base = nb([code("print(1)", OUT_BASE)]) head = nb([md("nouveau"), code("print(1)", OUT_BASE)]) recs = CSR.classify_cells(base, head) + self.assertEqual([r["verdict"] for r in recs], ["UNCHANGED"]) + self.assertFalse(any(r["regression"] for r in recs)) + + def test_new_legacy_cell_matching_nothing_stays_unpaired(self): + # A genuinely NEW code cell (no exact, no fuzzy base partner) + # stays UNPAIRED - content pairing must not fabricate pairs any + # more than positional pairing fabricated stale ones. + base = nb([code("print(1)", OUT_BASE)]) + head = nb([md("nouveau"), + code("import numpy as np\narr = np.zeros(3)", OUT_BASE)]) + recs = CSR.classify_cells(base, head) self.assertEqual([r["verdict"] for r in recs], ["UNPAIRED"]) self.assertFalse(any(r["regression"] for r in recs)) @@ -255,6 +269,61 @@ def test_id_pairing_still_flags_real_stale_output(self): self.assertTrue(recs[0]["regression"]) +class TestLegacyContentPairing(unittest.TestCase): + """#14297 residual tranche: no-id bases pair by content, not position. + + The three measured FPs were legacy-style notebooks where an enrichment + insertion shifted cells that positional pairing then confronted with + UNRELATED C.1 stubs whose uniform outputs are byte-identical. Id + pairing (#14319) covers nbformat 4.5+ bases; this class pins the + content fallback that covers the rest, including the two negative + controls the issue's acceptance demands. + """ + + def test_moved_only_cell_is_unchanged(self): + # Acceptance §3, first control: a cell only MOVED by an insertion + # above (the exact #14297 scenario - distinct stubs, identical + # outputs) -> UNCHANGED, never a fabricated STALE_OUTPUT. + stub_a = code("# Exercice 1\nprint('Exercice a completer')", + OUT_BASE) + stub_b = code("# Exercice 3\nprint('Exercice a completer')", + OUT_BASE) + base = nb([stub_a, stub_b]) + head = nb([md("## Lecture du resultat"), stub_a, stub_b]) + recs = CSR.classify_cells(base, head) + self.assertEqual([r["verdict"] for r in recs], + ["UNCHANGED", "UNCHANGED"]) + self.assertFalse(any(r["regression"] for r in recs)) + + def test_moved_and_modified_cell_is_stale(self): + # Acceptance §3, second control: without it, the content fallback + # would be indistinguishable from disarming the guard - a cell + # MOVED AND MODIFIED must still confront its own base version. + base = nb([code("resultat = 40 + 2\nprint(resultat)", OUT_BASE)]) + head = nb([md("## Introduction"), + code("resultat = 41 + 1 # reformule\nprint(resultat)", + OUT_BASE)]) + recs = CSR.classify_cells(base, head) + self.assertEqual([r["verdict"] for r in recs], ["STALE_OUTPUT"]) + self.assertTrue(all(r["regression"] for r in recs)) + + def test_modified_cell_pairs_to_own_base_desident_sibling(self): + # The FP-residual guard: when a modified stub and an untouched + # sibling stub share byte-identical outputs, the exact pass must + # consume the untouched one first, and the fuzzy pass must send + # the MODIFIED cell to its own base version (TRUE stale), not to + # the sibling (fabricated pair). + ex1 = "# Exercice 1\n# TODO\nprint('Exercice a completer')" + ex3 = "# Exercice 3\n# TODO\nprint('Exercice a completer')" + base = nb([code(ex1, OUT_BASE), code(ex3, OUT_BASE)]) + head = nb([code(ex3, OUT_BASE), + code(ex1 + "\n# indice: voir section 2", OUT_BASE)]) + recs = CSR.classify_cells(base, head) + self.assertEqual([r["verdict"] for r in recs], + ["UNCHANGED", "STALE_OUTPUT"]) + self.assertTrue(recs[1]["regression"]) + + class TestNotebookExemptions(unittest.TestCase): """validate_pr_notebooks' predicates, reused not duplicated."""