Skip to content

Commit aa7eff6

Browse files
committed
Address PR feedback: simplify conditional logic and add YRoom helper method
- Combine three similar else blocks in _updatePrompt method into one - Use optional chaining to simplify null checking in _getCellExecutionStateFromAwareness - Add set_cell_awareness_state method to YRoom for better encapsulation - Update kernel client to use new YRoom method for setting cell states
1 parent 1de776d commit aa7eff6

File tree

3 files changed

+44
-38
lines changed

3 files changed

+44
-38
lines changed

jupyter_server_documents/kernels/kernel_client.py

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -133,11 +133,7 @@ def handle_incoming_message(self, channel_name: str, msg: list[bytes]):
133133
# This ensures queued cells show '*' prompt even before kernel starts processing them
134134
if msg_type == "execute_request" and channel_name == "shell" and cell_id:
135135
for yroom in self._yrooms:
136-
awareness = yroom.get_awareness()
137-
if awareness is not None:
138-
cell_states = awareness.get_local_state().get("cell_execution_states", {})
139-
cell_states[cell_id] = "busy"
140-
awareness.set_local_state_field("cell_execution_states", cell_states)
136+
yroom.set_cell_awareness_state(cell_id, "busy")
141137

142138
self.message_cache.add({
143139
"msg_id": msg_id,
@@ -282,9 +278,8 @@ async def handle_document_related_message(self, msg: t.List[bytes]) -> t.Optiona
282278
# Store cell execution state for persistence across client connections
283279
# This ensures that cell execution states survive page refreshes
284280
if cell_id:
285-
cell_states = awareness.get_local_state().get("cell_execution_states", {})
286-
cell_states[cell_id] = execution_state
287-
awareness.set_local_state_field("cell_execution_states", cell_states)
281+
for yroom in self._yrooms:
282+
yroom.set_cell_awareness_state(cell_id, execution_state)
288283
break
289284

290285
case "execute_input":

jupyter_server_documents/rooms/yroom.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,17 @@ def set_cell_execution_state(self, cell_id: str, execution_state: str) -> None:
387387
self._cell_execution_states = {}
388388
self._cell_execution_states[cell_id] = execution_state
389389

390+
def set_cell_awareness_state(self, cell_id: str, execution_state: str) -> None:
391+
"""
392+
Sets the execution state for a specific cell in the awareness system.
393+
This provides real-time updates to all connected clients.
394+
"""
395+
awareness = self.get_awareness()
396+
if awareness is not None:
397+
cell_states = awareness.get_local_state().get("cell_execution_states", {})
398+
cell_states[cell_id] = execution_state
399+
awareness.set_local_state_field("cell_execution_states", cell_states)
400+
390401
def add_message(self, client_id: str, message: bytes) -> None:
391402
"""
392403
Adds new message to the message queue. Items placed in the message queue

src/notebook-factory/notebook-factory.ts

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ class RtcOutputAreaModel extends OutputAreaModel implements IOutputAreaModel {
170170
default:
171171
break;
172172
}
173-
173+
174174
// Always update prompt to check for awareness state on any state change
175175
this._updatePrompt();
176176
};
@@ -179,7 +179,7 @@ class RtcOutputAreaModel extends OutputAreaModel implements IOutputAreaModel {
179179
* Override the _updatePrompt method to check awareness execution state for real-time updates.
180180
* This method integrates with the server-side cell execution state tracking to provide
181181
* real-time visual feedback about cell execution status across collaborative sessions.
182-
*
182+
*
183183
* Key behaviors:
184184
* - Shows '*' for cells that are busy/running
185185
* - Shows execution count for idle cells
@@ -188,56 +188,53 @@ class RtcOutputAreaModel extends OutputAreaModel implements IOutputAreaModel {
188188
*/
189189
(CodeCell.prototype as any)._updatePrompt = function (): void {
190190
let prompt: string;
191-
191+
192192
// Get cell execution state from awareness (real-time)
193193
const cellExecutionState = this._getCellExecutionStateFromAwareness();
194-
194+
195195
// Check execution state from awareness
196196
if (cellExecutionState === 'busy') {
197197
// Cell is queued or actively executing - show spinning indicator
198198
prompt = '*';
199-
} else if (cellExecutionState === 'idle') {
200-
// Cell is idle, show execution count or blank if not provided
201-
prompt = `${this.model.executionCount || ''}`;
202-
} else if (cellExecutionState === undefined) {
203-
// Cell has never been executed - this is normal, don't trigger reconnection
204-
prompt = `${this.model.executionCount || ''}`;
205199
} else {
206-
// cellExecutionState === null - connection lost, show execution count as fallback
200+
// Cell is idle, never executed, or connection lost - show execution count as fallback
207201
prompt = `${this.model.executionCount || ''}`;
208202
}
209-
203+
210204
this._setPrompt(prompt);
211205
};
212206

213207
/**
214208
* Get execution state for this cell from awareness system.
215-
*
209+
*
216210
* This method queries the collaborative awareness state to determine the current
217211
* execution status of a cell. It distinguishes between three scenarios:
218-
*
212+
*
219213
* Returns:
220214
* - 'busy'|'idle'|'running': actual execution state from awareness
221215
* - null: awareness connection lost (should trigger reconnection)
222216
* - undefined: cell never executed (should not trigger reconnection)
223-
*
217+
*
224218
* The distinction between null and undefined is crucial for preventing
225219
* unnecessary reconnection attempts when cells have simply never been executed.
226220
*/
227-
(CodeCell.prototype as any)._getCellExecutionStateFromAwareness = function (): string | null | undefined {
221+
(CodeCell.prototype as any)._getCellExecutionStateFromAwareness = function ():
222+
| string
223+
| null
224+
| undefined {
228225
const notebook = this.parent?.parent;
229-
if (!notebook || !notebook.model || !notebook.model.sharedModel || !notebook.model.sharedModel.awareness) {
226+
if (!notebook?.model?.sharedModel?.awareness) {
230227
return null; // Connection lost
231228
}
232-
229+
233230
const awareness = notebook.model.sharedModel.awareness;
234231
const awarenessStates = awareness.getStates();
235-
232+
236233
// Check if awareness has any states at all
237234
if (awarenessStates.size === 0) {
238235
return null; // Connection lost
239236
}
240-
237+
241238
// Look through all client states for cell execution states
242239
let hasAnyExecutionStates = false;
243240
for (const [_, clientState] of awarenessStates) {
@@ -249,7 +246,7 @@ class RtcOutputAreaModel extends OutputAreaModel implements IOutputAreaModel {
249246
}
250247
}
251248
}
252-
249+
253250
if (hasAnyExecutionStates) {
254251
// We have execution states from server, but this cell is not in them
255252
// This means the cell has never been executed
@@ -262,7 +259,7 @@ class RtcOutputAreaModel extends OutputAreaModel implements IOutputAreaModel {
262259

263260
/**
264261
* Initialize CodeCell state including awareness listener setup.
265-
*
262+
*
266263
* This method is called once during cell creation to set up the awareness
267264
* listener that will track cell execution states in real-time across
268265
* collaborative sessions. It ensures that each cell has a properly
@@ -276,30 +273,33 @@ class RtcOutputAreaModel extends OutputAreaModel implements IOutputAreaModel {
276273

277274
/**
278275
* Set up awareness listener for prompt updates.
279-
*
276+
*
280277
* This method establishes a listener on the awareness system that will
281278
* automatically update the cell's prompt when execution states change.
282279
* It waits for the cell to be fully ready before attempting to access
283280
* awareness data, ensuring reliable setup.
284-
*
281+
*
285282
* The listener is stored for proper cleanup during cell disposal.
286283
*/
287284
(CodeCell.prototype as any)._setupAwarenessListener = function (): void {
288285
const updatePromptFromAwareness = () => {
289286
this._updatePrompt();
290287
};
291-
288+
292289
// The CodeCell instantiation needs to be fully ready before
293290
// attempting to fetch its awareness data.
294-
this.ready.then(() => {
291+
this.ready.then(() => {
295292
const notebook = this.parent?.parent;
296293
if (notebook?.model?.sharedModel?.awareness) {
297-
notebook.model.sharedModel.awareness.on('change', updatePromptFromAwareness);
298-
294+
notebook.model.sharedModel.awareness.on(
295+
'change',
296+
updatePromptFromAwareness
297+
);
298+
299299
// Store the listener for cleanup
300300
this._awarenessUpdateListener = updatePromptFromAwareness;
301301
this._awarenessInstance = notebook.model.sharedModel.awareness;
302-
302+
303303
// Perform initial prompt update
304304
this._updatePrompt();
305305
}
@@ -308,7 +308,7 @@ class RtcOutputAreaModel extends OutputAreaModel implements IOutputAreaModel {
308308

309309
/**
310310
* Override dispose to clean up awareness listener.
311-
*
311+
*
312312
* This ensures that when a cell is disposed, its awareness listener
313313
* is properly removed to prevent memory leaks and unexpected behavior.
314314
*/

0 commit comments

Comments
 (0)