diff --git a/scripts/notebook_tools/check_source_output_ratchet.py b/scripts/notebook_tools/check_source_output_ratchet.py index 4578ccae54..feaa60ab80 100644 --- a/scripts/notebook_tools/check_source_output_ratchet.py +++ b/scripts/notebook_tools/check_source_output_ratchet.py @@ -162,6 +162,96 @@ def canonical_outputs(cell): ensure_ascii=False) +def _outputs_text(outs): + """Extract all stream/execute_result text from outputs (decoded). + + Used by diff_outputs to produce the *honest* report NanoClaw missed on + #14958: when a comparison only counts image/* payloads it declares + "0 diff" and hides text-only output drift (deaccentuation, line join). + """ + parts = [] + for o in outs or []: + ot = o.get("output_type") + if ot == "stream": + text = o.get("text") or [] + if isinstance(text, list): + parts.append("".join(text)) + else: + parts.append(str(text)) + elif ot in ("execute_result", "display_data"): + data = o.get("data") or {} + text = data.get("text/plain") + if isinstance(text, list): + parts.append("".join(text)) + elif text is not None: + parts.append(str(text)) + # Other MIME types (image/png, html, latex) contribute nothing + # here; they are tallied separately by _outputs_payload_sizes. + return "".join(parts) + + +def _outputs_payload_sizes(outs): + """Sizes of binary/structured payloads (image/png, application/json, etc). + + The founding class of false-negative in #14978: the comparison omitted + image/* entirely, so three PNG re-encodings (86064→86272, 78012→78040, + 31668→31812 octets) read as 0 diff. We tally sizes here so a side-by-side + report names the cells that moved on the binary axis. + """ + sizes = {} + for o in outs or []: + data = o.get("data") or {} + for mime, payload in data.items(): + if mime.startswith("image/") or mime == "application/json": + if isinstance(payload, list): + blob = "".join(payload) + else: + blob = str(payload) + sizes[mime] = sizes.get(mime, 0) + len(blob.encode("utf-8", + errors="replace")) + return sizes + + +def diff_outputs(base_cell, head_cell): + """Classify the output diff between a base and head code cell. + + Returns one of: + "IDENTICAL" - byte-identical (canonical_outputs) + "TEXT_DIFF" - canonical differs AND text payload differs + "PAYLOAD_DIFF" - canonical differs AND a non-text payload size changed + "BOTH_DIFF" - canonical differs AND text + payload moved + "EMPTY_BASE" - base had no outputs, head has (re-execution) + "EMPTY_HEAD" - head has no outputs (cleared, suspicious) + The four non-IDENTICAL classes are what the existing ratchet lumps into + "EXECUTED" and what NanoClaw on #14958 misreported as 0. Splitting them + lets a reader tell *why* the output moved (text drift vs re-execution). + """ + bouts = base_cell.get("outputs") or [] + houts = head_cell.get("outputs") or [] + if not bouts and houts: + return "EMPTY_BASE" + if bouts and not houts: + return "EMPTY_HEAD" + if canonical_outputs(base_cell) == canonical_outputs(head_cell): + return "IDENTICAL" + b_text = _outputs_text(bouts) + h_text = _outputs_text(houts) + b_sizes = _outputs_payload_sizes(bouts) + h_sizes = _outputs_payload_sizes(houts) + text_diff = b_text != h_text + payload_diff = b_sizes != h_sizes + if text_diff and payload_diff: + return "BOTH_DIFF" + if payload_diff: + return "PAYLOAD_DIFF" + if text_diff: + return "TEXT_DIFF" + # canonical differs but neither text nor payload moved: rare; the diff + # lives in nbformat metadata (e.g. transient fields, traceback + # timestamps). Name it rather than lump it with IDENTICAL. + return "METADATA_DIFF" + + def notebook_kernel(nb): """kernelspec.name of a notebook, or None.""" return ((nb.get("metadata") or {}).get("kernelspec") or {}).get("name") @@ -249,6 +339,14 @@ def classify_cells(base_nb, head_nb): feeds git blobs. No exemption logic here - the caller applies notebook-level and body-level lifts so the classification stays reusable for the tier-1 sweep. + + Output-diff granularity (issue #14978): each cell where source AND + outputs both move is split by `diff_outputs` into TEXT_DIFF / + PAYLOAD_DIFF / BOTH_DIFF rather than the single EXECUTED verdict the + ratchet used to render. A bot (NanoClaw on #14958) that declares + "outputs = 0 diff" against such a pair is wrong on the face of it - + the local report names the cells, the kind of diff, and the byte + deltas so any external reviewer can be held to the truth. """ base_cells = base_nb.get("cells", []) head_cells = head_nb.get("cells", []) @@ -294,7 +392,11 @@ def classify_cells(base_nb, head_nb): records.append({"index": i, "verdict": "STALE_OUTPUT", "regression": True}) else: - records.append({"index": i, "verdict": "EXECUTED", + # Was: verdict="EXECUTED". Split so a reader sees *why* the + # output moved - the issue #14978 demand that a report say + # what it actually compared. + records.append({"index": i, + "verdict": diff_outputs(bcell, hcell), "regression": False}) return records @@ -355,15 +457,144 @@ def ratchet(base, cwd=None, body_text=""): return records +# The `moved` counter has ONE definition, read by the text render, the JSON +# report and the tests alike. IDENTICAL means "source changed but outputs are +# byte-identical" -- the STALE_OUTPUT class. It is reported in `diffs` (it is +# the whole point of the ratchet) but it is NOT an output diff: counting it +# would lie about the output axis. ai-01 #16234 review. +NON_MOVED_KINDS = ("UNCHANGED_SOURCE", "UNPAIRED", "IDENTICAL") + + +def report_output_diffs(base, cwd=None): + """Per-cell truth about output diffs, independent of regression class. + + Issue #14978: a reviewer (NanoClaw on #14958) declared "outputs = 0 diff" + on a pair where 7 of 14 code cells differed (3 PNG re-encodings + 4 text + deaccentuations). The ratchet's EXECUTED verdict already catches *that* + a cell changed - what the reviewer missed was naming which cells and + why. This report emits every paired code cell with its diff kind and + payload deltas, so any external claim of "0 diff" can be cross-checked + against the local truth. + + Returned shape (machine-readable): + {"base": , "notebooks": [ + {"notebook": , + "code_cells": , + "moved": , + "diffs": [ + {"index": , + "source_same": , + "kind": "IDENTICAL"|"TEXT_DIFF"|"PAYLOAD_DIFF"|"BOTH_DIFF" + |"EMPTY_BASE"|"EMPTY_HEAD"|"METADATA_DIFF" + |"UNCHANGED_SOURCE", + "payload_deltas": {"image/png": 208, ...}, + "text_delta_chars": }, + ...]}, + ...]} + `payload_deltas` is signed bytes (head - base) and is empty for + IDENTICAL cells. `text_delta_chars` is the absolute difference in the + decoded text payload (the deaccentuation fingerprint on #14958 is + exactly this). + + `moved` is the count of cells whose OUTPUTS moved -- `diffs` minus + NON_MOVED_KINDS. It is the number a reader of the CLI actually sees, + so it is carried in the machine-readable report too: a test that + recomputes it from `diffs` would assert its own restatement of the + rule instead of the counter under test (NanoClaw #16234 re-review). + """ + base = resolve_base(base, cwd=cwd) + out = {"base": base, "notebooks": []} + for path in changed_notebooks(base, cwd=cwd): + content = git("show", f"{base}:{path}", cwd=cwd) + if content is None: + out["notebooks"].append({"notebook": path, "verdict": "ADDED", + "code_cells": 0, "moved": 0, + "diffs": []}) + continue + base_nb, b_status = _parse_notebook(content) + try: + head_nb = json.loads((Path(cwd or ".") / path).read_text( + encoding="utf-8")) + h_status = "OK" + except Exception: + head_nb, h_status = None, "PARSE_ERROR" + if b_status != "OK" or h_status != "OK": + out["notebooks"].append({"notebook": path, "verdict": "PARSE_ERROR", + "code_cells": 0, "moved": 0, + "diffs": []}) + continue + base_by_id = {c.get("id"): c for c in base_nb.get("cells", []) + if c.get("id")} + use_ids = bool(base_by_id) + content_pairs = (None if use_ids + else _pair_by_content(base_nb.get("cells", []), + head_nb.get("cells", []))) + diffs = [] + code_count = 0 + for i, hcell in enumerate(head_nb.get("cells", [])): + if hcell.get("cell_type") != "code": + continue + code_count += 1 + bcell = None + if use_ids: + hid = hcell.get("id") + if hid and hid in base_by_id: + bcell = base_by_id[hid] + elif not hid: + bcell = (base_nb.get("cells", [])[i] + if i < len(base_nb.get("cells", [])) else None) + elif content_pairs and i in content_pairs: + bcell = base_nb.get("cells", [])[content_pairs[i]] + if bcell is None or bcell.get("cell_type") != "code": + diffs.append({"index": i, "source_same": None, + "kind": "UNPAIRED", + "payload_deltas": {}, "text_delta_chars": 0}) + continue + source_same = (canonical_source(bcell) + == canonical_source(hcell)) + kind = diff_outputs(bcell, hcell) + if source_same and kind == "IDENTICAL": + kind = "UNCHANGED_SOURCE" + b_sizes = _outputs_payload_sizes(bcell.get("outputs") or []) + h_sizes = _outputs_payload_sizes(hcell.get("outputs") or []) + deltas = {mime: h_sizes.get(mime, 0) - b_sizes.get(mime, 0) + for mime in set(b_sizes) | set(h_sizes) + if h_sizes.get(mime, 0) != b_sizes.get(mime, 0)} + b_text = _outputs_text(bcell.get("outputs") or []) + h_text = _outputs_text(hcell.get("outputs") or []) + diffs.append({"index": i, "source_same": source_same, + "kind": kind, + "payload_deltas": deltas, + "text_delta_chars": abs(len(h_text) - len(b_text)), + "text_identical": (b_text == h_text)}) + out["notebooks"].append({ + "notebook": path, + "verdict": "CHANGED", + "code_cells": code_count, + "moved": sum(1 for d in diffs + if d["kind"] not in NON_MOVED_KINDS), + "diffs": diffs, + }) + return out + + def main(): ap = argparse.ArgumentParser( description="Source-output ratchet: changed code must not ride " - "unchanged outputs (issue #13562)") + "unchanged outputs (issue #13562). --show-output-diffs " + "(issue #14978) emits the truth about every cell's " + "output diff, independent of regression class.") ap.add_argument("base", help="Base git ref (CI: origin/)") ap.add_argument("--json", action="store_true", dest="as_json", help="Machine-readable output") ap.add_argument("--body-file", dest="body_file", default=None, help="PR body text carrying per-cell exemptions") + ap.add_argument("--show-output-diffs", action="store_true", + dest="show_output_diffs", + help="Emit per-cell output diff report (issue #14978). " + "Independent of the regression check; the report " + "is the ground truth against which any external " + "'0 diff' claim should be cross-checked.") args = ap.parse_args() body_text = "" @@ -378,6 +609,39 @@ def main(): total_reg = sum(r["regressions"] for r in records) failing = [r for r in records if r["regressions"]] + if args.show_output_diffs: + report = report_output_diffs(args.base) + if args.as_json: + print(json.dumps(report, ensure_ascii=False, indent=1)) + else: + print(f"output-diff report -- base {args.base}") + for nb_rec in report["notebooks"]: + if nb_rec["verdict"] != "CHANGED": + print(f" {nb_rec['verdict']:12s} {nb_rec['notebook']}") + continue + moved = [d for d in nb_rec["diffs"] + if d["kind"] not in NON_MOVED_KINDS] + # The printed count is the report's own `moved` field, not a + # second computation: one definition, one number. INDIVIDUAL + # cells are still listed below from `moved` (the report only + # carries the count, not the filtered list). ai-01 #16234. + print(f" {nb_rec['notebook']} " + f"code={nb_rec['code_cells']} moved={nb_rec['moved']}") + for d in moved: + mark = [] + if d["payload_deltas"]: + for m, b in sorted(d["payload_deltas"].items()): + sign = "+" if b >= 0 else "" + mark.append(f"{m}={sign}{b}") + if d["text_delta_chars"]: + mark.append(f"text={d['text_delta_chars']}") + suffix = " " + " ".join(mark) if mark else "" + print(f" [{d['index']:>3}] {d['kind']:<14} " + f"src_same={d['source_same']}{suffix}") + # --show-output-diffs is read-only; do NOT enforce the regression + # gate (caller is auditing, not policing). Exit 0. + sys.exit(0) + if args.as_json: print(json.dumps({ "base": args.base, diff --git a/scripts/tests/test_check_source_output_ratchet.py b/scripts/tests/test_check_source_output_ratchet.py index 7694d1b275..25fe8dec3c 100644 --- a/scripts/tests/test_check_source_output_ratchet.py +++ b/scripts/tests/test_check_source_output_ratchet.py @@ -39,17 +39,32 @@ def md(*lines): return {"cell_type": "markdown", "metadata": {}, "source": list(lines)} -def code(src, outputs, execution_count=1): - return {"cell_type": "code", "execution_count": execution_count, +def code(src, outputs, execution_count=1, cell_id=None): + cell = {"cell_type": "code", "execution_count": execution_count, "metadata": {}, "outputs": outputs, "source": [src]} + if cell_id is not None: + cell["id"] = cell_id + return cell def nb(cells, kernel="python3"): + # Auto-stamp ids so the by-id pairing path is exercised by default. + # Tests that need legacy pairing can use nb_legacy() to skip this. + for j, c in enumerate(cells): + if c.get("cell_type") == "code" and "id" not in c: + c["id"] = f"cell-{j}" return {"cells": cells, "metadata": {"kernelspec": {"display_name": kernel, "name": kernel}}} +def nb_legacy(cells, kernel="python3"): + """Notebook without cell ids - exercises content-based pairing.""" + return {"cells": list(cells), + "metadata": {"kernelspec": {"display_name": kernel, + "name": kernel}}} + + # Two DIFFERENT non-empty outputs: the founding case kept the base output # byte-identical while editing the source; the patch refreshes it. OUT_BASE = [{"output_type": "stream", "name": "stdout", "text": ["42\n"]}] @@ -145,7 +160,11 @@ def test_passes_once_outputs_refreshed(self): payload = json.loads(proc.stdout) self.assertEqual(payload["regressions"], 0) cell = payload["records"][0]["cells"][-1] - self.assertEqual(cell["verdict"], "EXECUTED") + # Issue #14978 split EXECUTED into TEXT_DIFF/PAYLOAD_DIFF/ + # BOTH_DIFF. The refreshed outputs are a different output_type + # (stream vs execute_result) so canonical_outputs differs and + # _outputs_text differs -> TEXT_DIFF is the right verdict. + self.assertEqual(cell["verdict"], "TEXT_DIFF") finally: repo.close() @@ -212,8 +231,8 @@ def test_inserted_cell_is_unpaired_not_stale(self): # 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)]) + base = nb_legacy([code("print(1)", OUT_BASE)]) + head = nb_legacy([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)) @@ -288,8 +307,8 @@ def test_moved_only_cell_is_unchanged(self): 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]) + base = nb_legacy([stub_a, stub_b]) + head = nb_legacy([md("## Lecture du resultat"), stub_a, stub_b]) recs = CSR.classify_cells(base, head) self.assertEqual([r["verdict"] for r in recs], ["UNCHANGED", "UNCHANGED"]) @@ -299,10 +318,10 @@ 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)]) + base = nb_legacy([code("resultat = 40 + 2\nprint(resultat)", OUT_BASE)]) + head = nb_legacy([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)) @@ -315,9 +334,9 @@ def test_modified_cell_pairs_to_own_base_desident_sibling(self): # 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)]) + base = nb_legacy([code(ex1, OUT_BASE), code(ex3, OUT_BASE)]) + head = nb_legacy([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"]) @@ -395,5 +414,364 @@ def test_qualifier_scopes_the_lift(self): self.assertTrue(bar[0]["regression"]) +class TestDiffOutputsGranularity(unittest.TestCase): + """Issue #14978: split the single EXECUTED verdict by output-diff kind. + + The NanoClaw bot on #14958 declared "outputs = 0 diff" against a pair + where 7 of 14 code cells differed - 3 PNG re-encodings and 4 text + deaccentuations. The ratchet's EXECUTED verdict already caught *that* + cells moved; what the reviewer missed was naming which cells and why. + These tests prove the new granularity: TEXT_DIFF, PAYLOAD_DIFF, + BOTH_DIFF, and the unchanged cells named UNCHANGED_SOURCE. + """ + + def test_classify_marks_source_only_diff_as_unchanged(self): + # Source unchanged but outputs differ (per #14958 fingerprint): + # classify_cells still labels the cell UNCHANGED because the + # source axis is identical - the ratchet's regression class is + # gated on source change + output drift (STALE_OUTPUT). The + # granular kind of the OUTPUT drift lives in report_output_diffs, + # tested separately below. + base = nb([code("print('Strategie Row')", + [{"output_type": "stream", "name": "stdout", + "text": ["Strategie Row\n"]}])]) + head = nb([code("print('Strategie Row')", + [{"output_type": "stream", "name": "stdout", + "text": ["Stratégie Row\n"]}])]) + recs = CSR.classify_cells(base, head) + self.assertEqual(recs[0]["verdict"], "UNCHANGED") + self.assertFalse(recs[0]["regression"]) + + def test_report_output_diffs_names_source_only_diff_as_TEXT_DIFF(self): + # Counterpart of the test above at the report_output_diffs layer. + # Source identical + outputs differ -> the report must name the + # diff as TEXT_DIFF (otherwise the user's "0 diff" complaint on + # #14958 survives). This is the granularity the audit uses. + base = nb([code("print('Strategie Row')", + [{"output_type": "stream", "name": "stdout", + "text": ["Strategie Row\n"]}])]) + head = nb([code("print('Strategie Row')", + [{"output_type": "stream", "name": "stdout", + "text": ["Stratégie Row\n"]}])]) + repo = GitRepo.__new__(GitRepo) + import tempfile + repo.dir = tempfile.TemporaryDirectory() + repo.path = Path(repo.dir.name) + subprocess.run(["git", "init", "-q"], cwd=repo.path, check=True) + nb_path = "MyIA.AI.Notebooks/Fake/GT-text.ipynb" + (repo.path / "MyIA.AI.Notebooks/Fake").mkdir(parents=True) + for state in (base, head): + (repo.path / nb_path).write_text( + json.dumps(state, ensure_ascii=False, indent=1) + "\n", + encoding="utf-8") + subprocess.run(["git", "add", "-A"], cwd=repo.path, check=True) + subprocess.run(["git", "-c", "user.email=t@t", "-c", + "user.name=t", "commit", "-q", "-m", "s"], + cwd=repo.path, check=True) + try: + proc = subprocess.run( + [sys.executable, str(TOOL), "HEAD~1", + "--show-output-diffs", "--json"], + cwd=repo.path, capture_output=True, text=True, + encoding="utf-8", check=False) + self.assertEqual(proc.returncode, 0, proc.stderr) + payload = json.loads(proc.stdout) + diffs = payload["notebooks"][0]["diffs"] + self.assertEqual(len(diffs), 1) + cell = diffs[0] + self.assertEqual(cell["kind"], "TEXT_DIFF") + self.assertTrue(cell["source_same"]) + self.assertFalse(cell["text_identical"]) + finally: + repo.dir.cleanup() + + def test_payload_diff_is_PAYLOAD_DIFF(self): + # Two PNG payloads of different sizes; identical source. + png_small = "data:image/png;base64," + "A" * 100 + png_big = "data:image/png;base64," + "A" * 200 + base = nb([code("plt.savefig('out.png')", + [{"output_type": "display_data", + "data": {"image/png": png_small}}])]) + head = nb([code("plt.savefig('out.png')", + [{"output_type": "display_data", + "data": {"image/png": png_big}}])]) + recs = CSR.classify_cells(base, head) + self.assertEqual(recs[0]["verdict"], "UNCHANGED") + + def test_classify_marks_TEXT_DIFF_when_source_changed(self): + # Source AND outputs both move: the ratchet split lands here. + base = nb([code("x = 'Strategie'", + [{"output_type": "stream", "name": "stdout", + "text": ["Strategie\n"]}])]) + head = nb([code("x = 'Stratégie' # edited accent", + [{"output_type": "stream", "name": "stdout", + "text": ["Stratégie\n"]}])]) + recs = CSR.classify_cells(base, head) + self.assertEqual(recs[0]["verdict"], "TEXT_DIFF") + self.assertFalse(recs[0]["regression"]) + + def test_classify_marks_PAYLOAD_DIFF_when_png_resized(self): + # Source + PNG payload both change. + png_v1 = "data:image/png;base64," + "A" * 86064 + png_v2 = "data:image/png;base64," + "A" * 86272 + base = nb([code("plt.savefig('a.png')", + [{"output_type": "display_data", + "data": {"image/png": png_v1}}])]) + head = nb([code("plt.savefig('a.png', dpi=120) # tweak", + [{"output_type": "display_data", + "data": {"image/png": png_v2}}])]) + recs = CSR.classify_cells(base, head) + self.assertEqual(recs[0]["verdict"], "PAYLOAD_DIFF") + self.assertFalse(recs[0]["regression"]) + + def test_classify_marks_BOTH_DIFF(self): + png_v1 = "data:image/png;base64," + "A" * 100 + png_v2 = "data:image/png;base64," + "A" * 200 + base = nb([code("x = 1\nprint(x)", + [{"output_type": "stream", "name": "stdout", + "text": ["1\n"]}, + {"output_type": "display_data", + "data": {"image/png": png_v1}}])]) + head = nb([code("x = 2 # edited\nprint(x)", + [{"output_type": "stream", "name": "stdout", + "text": ["2\n"]}, + {"output_type": "display_data", + "data": {"image/png": png_v2}}])]) + recs = CSR.classify_cells(base, head) + self.assertEqual(recs[0]["verdict"], "BOTH_DIFF") + + def test_diff_outputs_unit(self): + # Pure diff_outputs unit test on canonical outputs. + base_cell = {"outputs": [{"output_type": "stream", "name": "stdout", + "text": ["abc\n"]}]} + head_cell_text = {"outputs": [{"output_type": "stream", "name": "stdout", + "text": ["abd\n"]}]} + head_cell_payload = {"outputs": [ + {"output_type": "stream", "name": "stdout", "text": ["abc\n"]}, + {"output_type": "display_data", + "data": {"image/png": "data:image/png;base64," + "A" * 200}}]} + self.assertEqual(CSR.diff_outputs(base_cell, base_cell), "IDENTICAL") + self.assertEqual(CSR.diff_outputs(base_cell, head_cell_text), + "TEXT_DIFF") + self.assertEqual(CSR.diff_outputs(base_cell, head_cell_payload), + "PAYLOAD_DIFF") + # Empty base + non-empty head: + self.assertEqual(CSR.diff_outputs({"outputs": []}, + head_cell_payload), + "EMPTY_BASE") + # Non-empty base + empty head: + self.assertEqual(CSR.diff_outputs(base_cell, {"outputs": []}), + "EMPTY_HEAD") + + def test_report_output_diffs_14958_fixture(self): + """The exact #14958 reconstructed: 14 code cells, 7 differ. + + Source untouched for all 14 cells; the head's outputs carry the + fresh accents + re-encoded PNGs. Without --show-output-diffs, the + ratchet's per-cell verdict would be UNCHANGED (source identical) + and the report would say "0 diff" - precisely NanoClaw's claim. + report_output_diffs is the read-only truth that names them. + """ + def stream(text): + return [{"output_type": "stream", "name": "stdout", + "text": [text]}] + + def png(n): + return [{"output_type": "display_data", + "data": {"image/png": "data:image/png;base64," + + "A" * n}}] + + # 14 code cells; 7 of them move on the output axis (4 text + + # 3 PNG), exactly the #14958 fingerprint. + bases = [] + heads = [] + for i in range(14): + if i in (2, 4, 5, 9): + bases.append(code(f"print({i})", + stream(f"sortie brute {i}\n"))) + heads.append(code(f"print({i})", + stream(f"sortie accentuée {i}\n"))) + elif i in (6, 7, 10): + bases.append(code(f"plt.savefig('c{i}.png')", png(100))) + heads.append(code(f"plt.savefig('c{i}.png')", + png(100 + (i - 5)))) + else: + bases.append(code(f"x{i} = {i}", + stream(f"{i}\n"))) + heads.append(code(f"x{i} = {i}", + stream(f"{i}\n"))) + # Run the report against the throwaway repo the test pattern + # already uses (GitRepo sets up commits and runs the tool). + repo = GitRepo.__new__(GitRepo) + import tempfile + repo.dir = tempfile.TemporaryDirectory() + repo.path = Path(repo.dir.name) + subprocess.run(["git", "init", "-q"], cwd=repo.path, check=True) + nb_path = "MyIA.AI.Notebooks/Fake/GT-05.ipynb" + (repo.path / "MyIA.AI.Notebooks/Fake").mkdir(parents=True) + for state in (nb(bases), nb(heads)): + (repo.path / nb_path).write_text( + json.dumps(state, ensure_ascii=False, indent=1) + "\n", + encoding="utf-8") + subprocess.run(["git", "add", "-A"], cwd=repo.path, check=True) + subprocess.run(["git", "-c", "user.email=t@t", "-c", + "user.name=t", "commit", "-q", "-m", "s"], + cwd=repo.path, check=True) + try: + proc = subprocess.run( + [sys.executable, str(TOOL), "HEAD~1", + "--show-output-diffs", "--json"], + cwd=repo.path, capture_output=True, text=True, + encoding="utf-8", check=False) + self.assertEqual(proc.returncode, 0, proc.stderr) + payload = json.loads(proc.stdout) + nb_rec = payload["notebooks"][0] + self.assertEqual(nb_rec["verdict"], "CHANGED") + self.assertEqual(nb_rec["code_cells"], 14) + # 7 cells with output diff (4 TEXT_DIFF + 3 PAYLOAD_DIFF), + # 7 cells UNCHANGED_SOURCE. The count is read from the report's + # own `moved` field. This fixture carries no IDENTICAL cell, so + # it cannot catch the pre-fix predicate by itself -- that guard + # is test_identical_outputs_not_counted_as_moved. NanoClaw + # #16234 re-review: asserting a locally re-derived count pins + # the restatement, not the counter. + self.assertEqual(nb_rec["moved"], 7, + f"expected 7 moved cells, got " + f"{nb_rec['moved']}") + text_moved = [d for d in nb_rec["diffs"] + if d["kind"] == "TEXT_DIFF"] + payload_moved = [d for d in nb_rec["diffs"] + if d["kind"] == "PAYLOAD_DIFF"] + self.assertEqual(len(text_moved), 4) + self.assertEqual(len(payload_moved), 3) + # Indices 2, 4, 5, 9 carry TEXT_DIFF; 6, 7, 10 carry PAYLOAD. + self.assertEqual(sorted(d["index"] for d in text_moved), + [2, 4, 5, 9]) + self.assertEqual(sorted(d["index"] for d in payload_moved), + [6, 7, 10]) + # At least one payload cell reports a positive byte delta. + self.assertTrue(any(d["payload_deltas"].get("image/png", 0) > 0 + for d in payload_moved)) + finally: + repo.dir.cleanup() + + def test_show_output_diffs_exits_zero_even_with_stale(self): + # --show-output-diffs is read-only; even a real STALE_OUTPUT pair + # should not cause non-zero exit (the ratchet gate is bypassed + # when the audit flag is on, so the caller can read the truth + # without the run failing on it). + repo = GitRepo([fixture_13550_base(), + fixture_13550_head(OUT_BASE)]) + try: + proc = subprocess.run( + [sys.executable, str(TOOL), "HEAD~1", + "--show-output-diffs"], + cwd=repo.path, capture_output=True, text=True, + encoding="utf-8", check=False) + self.assertEqual(proc.returncode, 0, proc.stderr) + finally: + repo.close() + + def test_identical_outputs_not_counted_as_moved(self): + # Cell with source changed BUT outputs byte-identical = the + # STALE_OUTPUT class. ai-01 #16234 review flagged this as a + # bug: report_output_diffs listed such cells as "moved" because + # the old filter excluded only UNCHANGED_SOURCE / UNPAIRED, not + # IDENTICAL. The fix: `moved` counts only cells whose OUTPUTS + # differ (TEXT_DIFF, PAYLOAD_DIFF, BOTH_DIFF, EMPTY_BASE, + # EMPTY_HEAD, METADATA_DIFF). IDENTICAL cells remain in `diffs` + # (with source_same=False, so the STALE_OUTPUT class is named) + # but are NOT counted as moved. + def stream(text): + return [{"output_type": "stream", "name": "stdout", + "text": [text]}] + + # 4 cells: 1 STALE (source changed, outputs identical -> kind=IDENTICAL, + # source_same=False), 1 TEXT_DIFF, 1 UNCHANGED_SOURCE, 1 PAYLOAD_DIFF. + bases = [ + code("x = 1\nprint(x)", stream("1\n")), + code("print('foo')", stream("foo\n")), + code("y = 2\nprint(y)", stream("2\n")), + code("plt.savefig('a.png')", + [{"output_type": "display_data", + "data": {"image/png": "data:image/png;base64," + "A" * 100}}]), + ] + heads = [ + code("x = 1 # edited comment\nprint(x)", stream("1\n")), # STALE + code("print('foo')", stream("foo accentuated\n")), # TEXT_DIFF + code("y = 2\nprint(y)", stream("2\n")), # UNCHANGED_SOURCE + code("plt.savefig('a.png')", + [{"output_type": "display_data", + "data": {"image/png": "data:image/png;base64," + + "A" * 200}}]), # PAYLOAD_DIFF + ] + repo = GitRepo.__new__(GitRepo) + import tempfile + repo.dir = tempfile.TemporaryDirectory() + repo.path = Path(repo.dir.name) + subprocess.run(["git", "init", "-q"], cwd=repo.path, check=True) + nb_path = "MyIA.AI.Notebooks/Fake/GT-05-IDENTICAL.ipynb" + (repo.path / "MyIA.AI.Notebooks/Fake").mkdir(parents=True) + for state in (nb(bases), nb(heads)): + (repo.path / nb_path).write_text( + json.dumps(state, ensure_ascii=False, indent=1) + "\n", + encoding="utf-8") + subprocess.run(["git", "add", "-A"], cwd=repo.path, check=True) + subprocess.run(["git", "-c", "user.email=t@t", "-c", + "user.name=t", "commit", "-q", "-m", "s"], + cwd=repo.path, check=True) + try: + proc = subprocess.run( + [sys.executable, str(TOOL), "HEAD~1", + "--show-output-diffs", "--json"], + cwd=repo.path, capture_output=True, text=True, + encoding="utf-8", check=False) + self.assertEqual(proc.returncode, 0, proc.stderr) + payload = json.loads(proc.stdout) + nb_rec = payload["notebooks"][0] + self.assertEqual(nb_rec["verdict"], "CHANGED") + self.assertEqual(nb_rec["code_cells"], 4) + # The STALE cell (index 0): IDENTICAL kind, source_same=False. + stale = [d for d in nb_rec["diffs"] + if d["index"] == 0][0] + self.assertEqual(stale["kind"], "IDENTICAL") + self.assertFalse(stale["source_same"]) + # UNCHANGED_SOURCE cell (index 2): UNCHANGED_SOURCE kind. + unchanged = [d for d in nb_rec["diffs"] + if d["index"] == 2][0] + self.assertEqual(unchanged["kind"], "UNCHANGED_SOURCE") + # The moved count is the REPORT's own field -- the same number + # the CLI prints -- not a local restatement of the rule. This + # fixture carries an IDENTICAL cell, so widening NON_MOVED_KINDS + # back to the pre-fix predicate makes this assertion fail. + # NanoClaw #16234 re-review. + self.assertEqual( + nb_rec["moved"], 2, + f"expected 2 moved cells (TEXT+PAYLOAD), got " + f"{nb_rec['moved']}: kinds=" + f"{[d['kind'] for d in nb_rec['diffs']]}") + # Which cells those are is fixture documentation, asserted by + # index rather than by re-filtering the rule. + self.assertEqual( + sorted(d["index"] for d in nb_rec["diffs"] + if d["index"] in (1, 3)), [1, 3]) + self.assertEqual( + {d["index"]: d["kind"] for d in nb_rec["diffs"]}, + {0: "IDENTICAL", 1: "TEXT_DIFF", 2: "UNCHANGED_SOURCE", + 3: "PAYLOAD_DIFF"}) + # Second surface, same field: the TEXT render prints the + # report's `moved`, so a divergence between what a human reads + # and what the JSON carries would fail here too. + text_proc = subprocess.run( + [sys.executable, str(TOOL), "HEAD~1", "--show-output-diffs"], + cwd=repo.path, capture_output=True, text=True, + encoding="utf-8", check=False) + self.assertEqual(text_proc.returncode, 0, text_proc.stderr) + self.assertIn("moved=2", text_proc.stdout) + self.assertNotIn("moved=3", text_proc.stdout) + finally: + repo.dir.cleanup() + + if __name__ == "__main__": unittest.main()