Skip to content

Commit a7d7231

Browse files
ellisonbg3coins
andauthored
Fix real-time output clearing for collaborative editing (#150)
* Submits a delete request to clear outputs * lint * Fix real-time output clearing synchronization Improve output clearing to work properly in collaborative environments by: 1. Immediate YDoc clearing: Clear outputs from shared document first for instant real-time sync to all connected clients 2. Async disk cleanup: Make API call to clear disk storage as secondary operation 3. Better error handling: Add early returns and improved error messages 4. Enhanced reliability: Real-time collaboration works even if disk API fails This fixes the issue where one user clearing outputs wouldn't be visible to other users until they reloaded the notebook. --------- Co-authored-by: Piyush Jain <piyushjain@duck.com>
1 parent ec93b8b commit a7d7231

File tree

3 files changed

+68
-7
lines changed

3 files changed

+68
-7
lines changed

jupyter_server_documents/outputs/handlers.py

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
1-
import json
2-
31
from tornado import web
42

53
from jupyter_server.auth.decorator import authorized
64
from jupyter_server.base.handlers import APIHandler
7-
from jupyter_server.utils import url_path_join
85

96

107
class OutputsAPIHandler(APIHandler):
@@ -18,7 +15,7 @@ def outputs(self):
1815

1916
@web.authenticated
2017
@authorized
21-
async def get(self, file_id=None, cell_id=None, output_index=None):
18+
async def get(self, file_id, cell_id=None, output_index=None):
2219
try:
2320
if output_index:
2421
output = self.outputs.get_output(file_id, cell_id, output_index)
@@ -35,6 +32,19 @@ async def get(self, file_id=None, cell_id=None, output_index=None):
3532
self.write(output)
3633
self.finish(set_content_type=content_type)
3734

35+
@web.authenticated
36+
@authorized
37+
async def delete(self, file_id, cell_id=None, output_index=None):
38+
# output_index is accepted but ignored as we clear all cell outputs regardless
39+
try:
40+
self.outputs.clear(file_id, cell_id)
41+
except (FileNotFoundError):
42+
self.set_status(404)
43+
self.finish({"error": "Output not found."})
44+
else:
45+
self.set_status(200)
46+
self.finish()
47+
3848

3949
class StreamAPIHandler(APIHandler):
4050
"""An outputs service API handler."""
@@ -69,7 +79,7 @@ async def get(self, file_id=None, cell_id=None):
6979

7080
_file_id_regex = r"(?P<file_id>[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12})"
7181
# In nbformat, cell_ids follow this format, compatible with uuid4
72-
_cell_id_regex = rf"(?P<cell_id>[a-zA-Z0-9_-]+)"
82+
_cell_id_regex = r"(?P<cell_id>[a-zA-Z0-9_-]+)"
7383

7484
# non-negative integers
7585
_output_index_regex = r"(?P<output_index>0|[1-9]\d*)"

jupyter_server_documents/rooms/yroom.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,9 @@ def _init_awareness(self, ydoc: pycrdt.Doc) -> pycrdt.Awareness:
307307
`awareness.unobserve(self._awareness_subscription)`.
308308
"""
309309
self._awareness = pycrdt.Awareness(ydoc=ydoc)
310+
if self.room_id != "JupyterLab:globalAwareness":
311+
file_format, file_type, file_id = self.room_id.split(":")
312+
self._awareness.set_local_state_field("file_id", file_id)
310313
self._awareness_subscription = self._awareness.observe(
311314
self._on_awareness_update
312315
)
@@ -338,7 +341,6 @@ def _init_jupyter_ydoc(self, ydoc: pycrdt.Doc, awareness: pycrdt.Awareness) -> Y
338341
self._jupyter_ydoc.observe(self._on_jupyter_ydoc_update)
339342
return self._jupyter_ydoc
340343

341-
342344
@property
343345
def clients(self) -> YjsClientGroup:
344346
"""

src/notebook-factory/notebook-factory.ts

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {
55
ICodeCellModel
66
} from '@jupyterlab/cells';
77
import { IChangedArgs } from '@jupyterlab/coreutils';
8-
import { Notebook, NotebookPanel } from '@jupyterlab/notebook';
8+
import { Notebook, NotebookPanel, NotebookActions } from '@jupyterlab/notebook';
99
import { CellChange, createMutex, ISharedCodeCell } from '@jupyter/ydoc';
1010
import { IOutputAreaModel, OutputAreaModel } from '@jupyterlab/outputarea';
1111
import { requestAPI } from '../handler';
@@ -340,3 +340,52 @@ export class RtcNotebookContentFactory
340340
return new ResettableNotebook(options);
341341
}
342342
}
343+
344+
// Add a handler for the outputCleared signal
345+
NotebookActions.outputCleared.connect((sender, args) => {
346+
const { notebook, cell } = args;
347+
const cellId = cell.model.sharedModel.getId();
348+
const awareness = notebook.model?.sharedModel.awareness;
349+
const awarenessStates = awareness?.getStates();
350+
351+
// FIRST: Clear outputs in YDoc for immediate real-time sync to all clients
352+
try {
353+
const sharedCodeCell = cell.model.sharedModel as ISharedCodeCell;
354+
sharedCodeCell.setOutputs([]);
355+
console.debug(`Cleared outputs in YDoc for cell ${cellId}`);
356+
} catch (error: unknown) {
357+
console.error('Error clearing YDoc outputs:', error);
358+
}
359+
360+
if (awarenessStates?.size === 0) {
361+
console.log('Could not delete cell output, awareness is not present');
362+
return; // Early return since we can't get fileId without awareness
363+
}
364+
365+
let fileId = null;
366+
for (const [_, state] of awarenessStates || []) {
367+
if (state && 'file_id' in state) {
368+
fileId = state['file_id'];
369+
}
370+
}
371+
372+
if (fileId === null) {
373+
console.error('No fileId found in awareness');
374+
return; // Early return since we can't make API call without fileId
375+
}
376+
377+
// SECOND: Send API request to clear outputs from disk storage
378+
try {
379+
requestAPI(`/api/outputs/${fileId}/${cellId}`, {
380+
method: 'DELETE'
381+
})
382+
.then(() => {
383+
console.debug(`Successfully cleared outputs from disk for cell ${cellId}`);
384+
})
385+
.catch((error: Error) => {
386+
console.error(`Failed to clear outputs from disk for cell ${cellId}:`, error);
387+
});
388+
} catch (error: unknown) {
389+
console.error('Error in disk output clearing process:', error);
390+
}
391+
});

0 commit comments

Comments
 (0)