Remove duplicate cells on document change#555
Conversation
|
|
|
@davidbrochart @krassowski another couple of changes related to cell duplication (also #556). unfortunately this has been a bit of a thorn in our sides but with the amazing help of @pablogsal we've been able to reproduce a situation where concurrent out-of-band updates cause things to blow up so these two commit will guard against that |
davidbrochart
left a comment
davidbrochart
left a comment
There was a problem hiding this comment.
Should this change be in jupyter_ydoc? I don't think jupyter-server-ydoc should be aware of the types of shared documents.
Call `remove_duplicate_cells()` (added in jupyter-ydoc) from `_on_document_change` when cells change, preventing duplicate cells from accumulating when multiple clients sync notebook content concurrently. Uses `getattr` to duck-type the check, so `rooms.py` remains unaware of notebook-specific document structure — all cell-level knowledge stays encapsulated in `jupyter_ydoc.YNotebook`. Requires jupyter-ydoc with `YNotebook.remove_duplicate_cells()`. PY-1190 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
jordanhboxer
force-pushed
the
jupyter-server-ydoc-2.2.1+hrt1
branch
from
194810b to
e84bfba
Compare
|
good call! i created a public method on |
| remove_duplicate_cells = getattr(self._document, "remove_duplicate_cells", None) | ||
| if target == "cells" and remove_duplicate_cells is not None: | ||
| asyncio.get_running_loop().call_soon(remove_duplicate_cells) | ||
|
|
There was a problem hiding this comment.
Hmm I still see that as an abstraction leak.
There was a problem hiding this comment.
fair point. adding an observe cells callback to handle in jupyter-server/jupyter_ydoc#397
|
abandoning in favor of jupyter-server/jupyter_ydoc#397 |
3 participants
Summary
remove_duplicate_cells()(added in jupyter-ydoc#397) from_on_document_changewhen cells change, preventing duplicate cells from accumulating when multiple clients sync notebook content concurrentlygetattrsorooms.pyremains unaware of notebook-specific document structure — all cell-level knowledge stays encapsulated injupyter_ydoc.YNotebookDependencies
YNotebook.remove_duplicate_cells()Test plan
Related
remove_duplicate_cells()public method to YNotebook jupyter-server/jupyter_ydoc#397YNotebook.get()level too jupyter-server/jupyter_ydoc#385