Load/auto-save document from the back-end using y-py

[davidbrochart]

References

Follow-up of #11599.

Code changes

We don't fetch a file content through HTTP anymore, the file is loaded when the Yjs websocket is opened.
The front-end doesn't request or put an initial content through the Yjs websocket anymore, it just syncs the Y document directly.
Pressing Ctrl-S in collaborative mode doesn't send an HTTP request, as every change to the document is auto-saved in the back-end.

User-facing changes

When a user changes a document, it is marked as dirty for one second then automatically saved.
A change made to a file in the back-end is automatically propagated to every user.

Backwards-incompatible changes

requestInitialContent and putInitializedState have been removed from the IDocumentProvider interface in the @jupyterlab/docprovider package.

[jupyterlab-probot]

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link:

[davidbrochart]

cc @dmonad @hbcarlos

[davidbrochart]

@hbcarlos with your changes, I get in this error in the browser while opening a notebook:

sync.jss:86 Caught error while handling a Yjs update TypeError: Cannot read properties of undefined (reading 'get')
    at createCellModelFromSharedType (ymodels.tss:490:16)
    at ymodels.tss:399:38
    at Set.forEach (<anonymous>)
    at Array.YNotebook._onYCellsChanged (ymodels.tss:396:25)
    at Module.callAll (function.jss:19:1)
    at callEventHandlerListeners (yjs.mjss:1885:3)
    at callTypeObservers (yjs.mjss:4628:1)
    at YArray._callObserver (yjs.mjss:5387:1)
    at Array.<anonymous> (yjs.mjss:3097:1)
    at callAll (function.jss:19:1)

[davidbrochart]

I also get this error in the back-end, so it might be related.

[davidbrochart]

Now that we watch file changes and update the Y document with the saved content, I don't think it makes sense for the user to be able to e.g. press Ctrl-S to save. We basically guarantee that what the user sees in the front-end is in sync with every other client, including the back-end.
In the context of a Y document, the back-end is a client just as any other front-end client. The only difference is that the back-end is also responsible for syncing the Y document with the file. With the current state of this PR, it does it in the "file -> Y document" direction. To sync in the other direction ("Y document -> file"), we need #12306, that I will include in this PR.

[hbcarlos]

You are right, we are storing nbformat and nbformat_minor on the state because we can not create an integer from the root of a Y.Doc.

I think we should move the nbformat and nbformat_minor attributes to the YType called "meta", this way we keep the content in one place and the state to store state-related attributes like dirty.

[davidbrochart]

That sound good, the notebook's metadata is actually stored in meta["metadata"] so we can move nbformat and nbformat_minor to meta["nbformat"] and meta["nbformat_minor"].
I'll make the changes in jupyter_ydoc, we also need to change the front-end.

[hbcarlos]

I'll make the changes in the frontend.

[davidbrochart]

Thanks, the problem is that we would need @jupyterlab/shared-models to be released, so this has to be done in a separate PR.

[davidbrochart]

Thanks, the problem is that we would need @jupyterlab/shared-models to be released, so this has to be done in a separate PR.

Actually, it's only to have tests pass in jupyter-server/jupyter_ydoc#21, so maybe we don't need a release.

[dmonad]

We also want to observe the dirty flag, because for instance a notebook opened as a text document will have its dirty indicator set from the front-end when the notebook opened as such changes. In that case we don't want to save the notebook again from the text document but we want to clear its dirty flag.

It might make sense to only modify the shared dirty property on the backend. If server observes a change (content, metadata, or otherwise), then the document state is dirty. The server can also set a variable "last_modified" that it maintains locally. If the document state has been dirty for more than X seconds, the server can store the document and set dirty=false again.

A potential problem is that a client modifies a document (setting dirty=true flag) at the same time as the server stores a document and sets dirty=false. These kinds of problems can be avoided if dirty is only modified by the server.

[davidbrochart]

I was thinking about that too, but @afshin mentioned that we still want the dirty flag to work when the front-end looses connection, and this is a good point.
But I agree things would be simpler if the dirty flag was only set/cleared from the back-end.

[dmonad]

Hmm.. that makes sense. There could be a local dirty variable on the client as well that is set to true in anticipation that the server will soon set the real dirty flag to true. This way the client can still set a local dirty flag, but prefers the remote state. The getter for this function could look like this:

class SharedModel {
  constructor () {
    ..
    this._localDirty = false
    this._ymeta = ydoc.getMap('meta')
    this._ymeta.observe(event => {
      if (event.changed.has('dirty')) {
        this._localDirty = this._ymeta.get('dirty')
        // potentially fire dirty-changed observer here
      }
    })
    ..
  }

  get dirty () {
    return this._localDirty
  }

  set dirty () {
    this._localDirty = true
  }
}

[hbcarlos]

Yep, I agree we should only modify it on the backend and keep a local variable in the frontend.
I'll make the changes

[davidbrochart]

@hbcarlos I thought that when the front-end changes the document, it would only set a local dirty flag, and let the back-end set the shared dirty flag when the document changes, but with your changes I see that the dirty flag is seen by all front-ends, and I've not changed the back-end yet to set the dirty flag.
Am I missing something?

[hbcarlos]

Yep, my bad. I added the local dirty attribute in the NotebookModel but I forgot the DocumentModel...

I'm on it

[davidbrochart]

Mmmm I'm seeing the same thing after this new commit @hbcarlos.

[hbcarlos]

Yes, it is. the sharedModel is still initializing the dirty property.

jupyterlab/packages/shared-models/src/ymodels.ts

Line 155 in d8ee1cb

model.dirty = false;

jupyterlab/packages/shared-models/src/ymodels.ts

Line 377 in d8ee1cb

model.dirty = false;

I'm on it 🤣

[davidbrochart]

This is ready to be merged!

[fcollonval]

Let's move on with this.

[damianavila]

@davidbrochart, just to confirm, you need to have collaborative mode enabled to make this work as described in the top message, right?

[davidbrochart]

@damianavila yes, this only works in collaborative mode.