From 5c715ddb3ad174bbd3d3bc795941833d867870f2 Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Thu, 20 Nov 2025 13:18:14 +0000 Subject: [PATCH 1/6] Add a failing test case for granular modifications Update tests to expect specific events --- tests/test_ynotebook.py | 65 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 64 insertions(+), 1 deletion(-) diff --git a/tests/test_ynotebook.py b/tests/test_ynotebook.py index f7619a1..bec5b4d 100644 --- a/tests/test_ynotebook.py +++ b/tests/test_ynotebook.py @@ -1,7 +1,9 @@ # Copyright (c) Jupyter Development Team. # Distributed under the terms of the Modified BSD License. -from pycrdt import Map + +from pycrdt import ArrayEvent, Map, MapEvent, TextEvent +from pytest import mark from jupyter_ydoc import YNotebook @@ -114,3 +116,64 @@ def record_changes(topic, event): {"delete": 1}, {"insert": [AnyInstanceOf(Map)]}, ] + + +@mark.parametrize( + "modifications, expected_events", + [ + # modifications of single attributes + ([["source", "'b'"]], {TextEvent}), + ([["outputs", []]], {ArrayEvent}), + ([["execution_count", 2]], {MapEvent}), + # multi-attribute modifications + ([["source", "10"], ["execution_count", 10]], {TextEvent, MapEvent}), + ], +) +def test_modify_single_cell(modifications, expected_events): + nb = YNotebook() + nb.set( + { + "cells": [ + { + "id": "8800f7d8-6cad-42ef-a339-a9c185ffdd54", + "cell_type": "code", + "source": "'a'", + "metadata": {"tags": ["test-tag"]}, + "outputs": [{"name": "stdout", "output_type": "stream", "text": ["a\n"]}], + "execution_count": 1, + }, + ] + } + ) + + # Get the model as Python object + model = nb.get() + + # Make changes + for modification in modifications: + key, new_value = modification + model["cells"][0][key] = new_value + + changes = [] + + def record_changes(topic, event): + changes.append((topic, event)) + + nb.observe(record_changes) + nb.set(model) + + for modification in modifications: + key, new_value = modification + after = nb.ycells[0][key] + after_py = after.to_py() if hasattr(after, "to_py") else after + assert after_py == new_value + + # there should be only one change + assert len(changes) == 1 + cell_events = [e for t, e in changes if t == "cells"] + # and it should be a cell change + assert len(cell_events) == 1 + # but it should be a change to cell data, not a change to the cell list + events = cell_events[0] + assert len(events) == len(expected_events) + assert {type(e) for e in events} == expected_events From ab37f2c2534f395722ed293744881c48b7beebcf Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Thu, 20 Nov 2025 13:29:36 +0000 Subject: [PATCH 2/6] Implement granular cell updates on Python side Fix --- jupyter_ydoc/ynotebook.py | 54 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 52 insertions(+), 2 deletions(-) diff --git a/jupyter_ydoc/ynotebook.py b/jupyter_ydoc/ynotebook.py index 9067740..ad1edcd 100644 --- a/jupyter_ydoc/ynotebook.py +++ b/jupyter_ydoc/ynotebook.py @@ -17,6 +17,8 @@ # The default minor version of the notebook format. NBFORMAT_MINOR_VERSION = 5 +_CELL_KEY_TYPE_MAP = {"metadata": Map, "source": Text, "outputs": Array} + class YNotebook(YBaseDoc): """ @@ -249,7 +251,7 @@ def set(self, value: dict) -> None: "id": str(uuid4()), } ] - old_ycells_by_id = {ycell["id"]: ycell for ycell in self._ycells} + old_ycells_by_id: dict[str, Map] = {ycell["id"]: ycell for ycell in self._ycells} with self._ydoc.transaction(): new_cell_list: list[dict] = [] @@ -260,7 +262,55 @@ def set(self, value: dict) -> None: cell_id = new_cell.get("id") if cell_id and (old_ycell := old_ycells_by_id.get(cell_id)): old_cell = self._cell_to_py(old_ycell) - if old_cell == new_cell: + updated_granularly = True + if old_cell != new_cell: + # attempt to update cell granularly + old_keys = set(old_cell.keys()) + new_keys = set(new_cell.keys()) + + shared_keys = old_keys & new_keys + removed_keys = old_keys - new_keys + added_keys = new_keys - old_keys + + for key in shared_keys: + if old_cell[key] != new_cell[key]: + if key == "output" and value: + # outputs require complex handling - some have Text type nested; + # for now skip creating them; clearing all outputs is fine + updated_granularly = False + + if key in _CELL_KEY_TYPE_MAP: + kind = _CELL_KEY_TYPE_MAP[key] + value = new_cell[key] + if kind == Text: + old: Text = old_ycell[key] + old.clear() + old.insert(0, value) + elif kind == Array: + old: Array = old_ycell[key] + old.clear() + old.extend(value) + elif kind == Map: + old: Map = old_ycell[key] + old.clear() + for k, v in value.items(): + old[k] = v + else: + old_ycell[key] = new_cell[key] + + for key in removed_keys: + del old_ycell[key] + + for key in added_keys: + if key in _CELL_KEY_TYPE_MAP: + # we hard-reload cells when keys that require nested types get added + # to allow the frontend to connect observers; this could be changed + # in the future, once frontends learn how to observe all changes + updated_granularly = False + else: + old_ycell[key] = new_cell[key] + + if updated_granularly: new_cell_list.append(old_cell) retained_cells.add(cell_id) continue From 675da6f7de04ce3e25f84a55884b63c16e7f609b Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Thu, 20 Nov 2025 14:44:59 +0000 Subject: [PATCH 3/6] Move code to a private method --- jupyter_ydoc/ynotebook.py | 100 ++++++++++++++++++++------------------ 1 file changed, 53 insertions(+), 47 deletions(-) diff --git a/jupyter_ydoc/ynotebook.py b/jupyter_ydoc/ynotebook.py index ad1edcd..87c8690 100644 --- a/jupyter_ydoc/ynotebook.py +++ b/jupyter_ydoc/ynotebook.py @@ -262,53 +262,9 @@ def set(self, value: dict) -> None: cell_id = new_cell.get("id") if cell_id and (old_ycell := old_ycells_by_id.get(cell_id)): old_cell = self._cell_to_py(old_ycell) - updated_granularly = True - if old_cell != new_cell: - # attempt to update cell granularly - old_keys = set(old_cell.keys()) - new_keys = set(new_cell.keys()) - - shared_keys = old_keys & new_keys - removed_keys = old_keys - new_keys - added_keys = new_keys - old_keys - - for key in shared_keys: - if old_cell[key] != new_cell[key]: - if key == "output" and value: - # outputs require complex handling - some have Text type nested; - # for now skip creating them; clearing all outputs is fine - updated_granularly = False - - if key in _CELL_KEY_TYPE_MAP: - kind = _CELL_KEY_TYPE_MAP[key] - value = new_cell[key] - if kind == Text: - old: Text = old_ycell[key] - old.clear() - old.insert(0, value) - elif kind == Array: - old: Array = old_ycell[key] - old.clear() - old.extend(value) - elif kind == Map: - old: Map = old_ycell[key] - old.clear() - for k, v in value.items(): - old[k] = v - else: - old_ycell[key] = new_cell[key] - - for key in removed_keys: - del old_ycell[key] - - for key in added_keys: - if key in _CELL_KEY_TYPE_MAP: - # we hard-reload cells when keys that require nested types get added - # to allow the frontend to connect observers; this could be changed - # in the future, once frontends learn how to observe all changes - updated_granularly = False - else: - old_ycell[key] = new_cell[key] + updated_granularly = self._update_cell( + old_cell=old_cell, new_cell=new_cell, old_ycell=old_ycell + ) if updated_granularly: new_cell_list.append(old_cell) @@ -374,3 +330,53 @@ def observe(self, callback: Callable[[str, Any], None]) -> None: self._subscriptions[self._ystate] = self._ystate.observe(partial(callback, "state")) self._subscriptions[self._ymeta] = self._ymeta.observe_deep(partial(callback, "meta")) self._subscriptions[self._ycells] = self._ycells.observe_deep(partial(callback, "cells")) + + def _update_cell(self, old_cell: dict, new_cell: dict, old_ycell: Map) -> bool: + if old_cell == new_cell: + return True + # attempt to update cell granularly + old_keys = set(old_cell.keys()) + new_keys = set(new_cell.keys()) + + shared_keys = old_keys & new_keys + removed_keys = old_keys - new_keys + added_keys = new_keys - old_keys + + for key in shared_keys: + if old_cell[key] != new_cell[key]: + value = new_cell[key] + if key == "output" and value: + # outputs require complex handling - some have Text type nested; + # for now skip creating them; clearing all outputs is fine + return False + + if key in _CELL_KEY_TYPE_MAP: + kind = _CELL_KEY_TYPE_MAP[key] + if kind == Text and value: + old: Text = old_ycell[key] + old.clear() + old.insert(0, value) + elif kind == Array: + old: Array = old_ycell[key] + old.clear() + old.extend(value) + elif kind == Map: + old: Map = old_ycell[key] + old.clear() + for k, v in value.items(): + old[k] = v + else: + old_ycell[key] = new_cell[key] + + for key in removed_keys: + del old_ycell[key] + + for key in added_keys: + if key in _CELL_KEY_TYPE_MAP: + # we hard-reload cells when keys that require nested types get added + # to allow the frontend to connect observers; this could be changed + # in the future, once frontends learn how to observe all changes + return False + else: + old_ycell[key] = new_cell[key] + return True From e59b4e6b58b2632e028bb602f1fc9dd825ac1c0e Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Thu, 20 Nov 2025 14:49:11 +0000 Subject: [PATCH 4/6] Add type guard, fix text removal --- jupyter_ydoc/ynotebook.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/jupyter_ydoc/ynotebook.py b/jupyter_ydoc/ynotebook.py index 87c8690..0471e1b 100644 --- a/jupyter_ydoc/ynotebook.py +++ b/jupyter_ydoc/ynotebook.py @@ -352,7 +352,12 @@ def _update_cell(self, old_cell: dict, new_cell: dict, old_ycell: Map) -> bool: if key in _CELL_KEY_TYPE_MAP: kind = _CELL_KEY_TYPE_MAP[key] - if kind == Text and value: + + if not isinstance(old_ycell[key], kind): + # if our assumptions about types do not, fall back to hard update + return False + + if kind == Text: old: Text = old_ycell[key] old.clear() old.insert(0, value) From e826aa49977b72aa3a8c5e69e71ae62cbcf710b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Krassowski?= <5832902+krassowski@users.noreply.github.com> Date: Thu, 20 Nov 2025 16:01:35 +0000 Subject: [PATCH 5/6] Apply suggestions from code review Co-authored-by: David Brochart --- jupyter_ydoc/ynotebook.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/jupyter_ydoc/ynotebook.py b/jupyter_ydoc/ynotebook.py index 0471e1b..b6886a7 100644 --- a/jupyter_ydoc/ynotebook.py +++ b/jupyter_ydoc/ynotebook.py @@ -354,13 +354,13 @@ def _update_cell(self, old_cell: dict, new_cell: dict, old_ycell: Map) -> bool: kind = _CELL_KEY_TYPE_MAP[key] if not isinstance(old_ycell[key], kind): - # if our assumptions about types do not, fall back to hard update + # if our assumptions about types do not hold, fall back to hard update return False if kind == Text: old: Text = old_ycell[key] old.clear() - old.insert(0, value) + old += value elif kind == Array: old: Array = old_ycell[key] old.clear() @@ -368,8 +368,7 @@ def _update_cell(self, old_cell: dict, new_cell: dict, old_ycell: Map) -> bool: elif kind == Map: old: Map = old_ycell[key] old.clear() - for k, v in value.items(): - old[k] = v + old.update(value) else: old_ycell[key] = new_cell[key] From 9ccb5a8061527616eb68d3c705f9c4050b783063 Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Thu, 20 Nov 2025 20:38:43 +0000 Subject: [PATCH 6/6] Increase test coverage --- tests/test_ynotebook.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_ynotebook.py b/tests/test_ynotebook.py index bec5b4d..868e9c1 100644 --- a/tests/test_ynotebook.py +++ b/tests/test_ynotebook.py @@ -125,6 +125,8 @@ def record_changes(topic, event): ([["source", "'b'"]], {TextEvent}), ([["outputs", []]], {ArrayEvent}), ([["execution_count", 2]], {MapEvent}), + ([["metadata", {"tags": []}]], {MapEvent}), + ([["new_key", "test"]], {MapEvent}), # multi-attribute modifications ([["source", "10"], ["execution_count", 10]], {TextEvent, MapEvent}), ],