Skip to content

Commit 3e7dfc9

Browse files
committed
feat(deepnote): Add clearNotebookSelection method and update notebook deserialization logic
- Introduced `clearNotebookSelection` method in `DeepnoteNotebookManager` to reset notebook selection for a project. - Updated `DeepnoteFileChangeWatcher` to call `clearNotebookSelection` during file change events, ensuring the active editor is prioritized during re-deserialization. - Modified `deserializeNotebook` method in `DeepnoteNotebookSerializer` to accept an optional `notebookId` parameter, preventing race conditions when multiple notebooks from the same project are open.
1 parent c5342a3 commit 3e7dfc9

6 files changed

Lines changed: 120 additions & 6 deletions

File tree

src/notebooks/deepnote/deepnoteFileChangeWatcher.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,17 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic
235235
doc.notebookType === 'deepnote' && doc.uri.with({ query: '', fragment: '' }).toString() === uriString
236236
);
237237

238+
// Clear the global notebook selection so that any VS Code-triggered
239+
// re-deserialization (e.g. from workspace.save) falls back to the
240+
// active editor rather than a stale global selection.
241+
for (const notebook of affectedNotebooks) {
242+
const projectId = notebook.metadata?.deepnoteProjectId as string | undefined;
243+
if (projectId) {
244+
this.notebookManager.clearNotebookSelection(projectId);
245+
break;
246+
}
247+
}
248+
238249
for (const notebook of affectedNotebooks) {
239250
const nbKey = notebook.uri.toString();
240251
// main-file-sync always replaces any pending operation
@@ -276,10 +287,15 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic
276287
return;
277288
}
278289

290+
// Pass the notebook ID explicitly to avoid mutating the global selection state.
291+
// Multiple notebooks from the same project may be open simultaneously, and
292+
// mutating selectedNotebookByProject would cause race conditions.
293+
const notebookNotebookId = notebook.metadata?.deepnoteNotebookId as string | undefined;
294+
279295
const tokenSource = new CancellationTokenSource();
280296
let newData;
281297
try {
282-
newData = await this.serializer.deserializeNotebook(content, tokenSource.token);
298+
newData = await this.serializer.deserializeNotebook(content, tokenSource.token, notebookNotebookId);
283299
} catch (error) {
284300
logger.warn(`[FileChangeWatcher] Failed to parse changed file: ${fileUri.path}`, error);
285301
return;

src/notebooks/deepnote/deepnoteNotebookManager.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,14 @@ export class DeepnoteNotebookManager implements IDeepnoteNotebookManager {
1414
private readonly projectsWithInitNotebookRun = new Set<string>();
1515
private readonly selectedNotebookByProject = new Map<string, string>();
1616

17+
/**
18+
* Clears the notebook selection for a project so that subsequent
19+
* deserializations fall back to the active editor or open documents.
20+
*/
21+
clearNotebookSelection(projectId: string): void {
22+
this.selectedNotebookByProject.delete(projectId);
23+
}
24+
1725
/**
1826
* Gets the currently selected notebook ID for a project.
1927
* @param projectId Project identifier

src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,33 @@ suite('DeepnoteNotebookManager', () => {
123123
});
124124
});
125125

126+
suite('clearNotebookSelection', () => {
127+
test('should clear selection for a project', () => {
128+
manager.selectNotebookForProject('project-123', 'notebook-456');
129+
manager.clearNotebookSelection('project-123');
130+
131+
const selectedNotebook = manager.getTheSelectedNotebookForAProject('project-123');
132+
133+
assert.strictEqual(selectedNotebook, undefined);
134+
});
135+
136+
test('should not affect other projects', () => {
137+
manager.selectNotebookForProject('project-1', 'notebook-1');
138+
manager.selectNotebookForProject('project-2', 'notebook-2');
139+
manager.clearNotebookSelection('project-1');
140+
141+
assert.strictEqual(manager.getTheSelectedNotebookForAProject('project-1'), undefined);
142+
assert.strictEqual(manager.getTheSelectedNotebookForAProject('project-2'), 'notebook-2');
143+
});
144+
145+
test('should be idempotent for unknown project', () => {
146+
assert.doesNotThrow(() => {
147+
manager.clearNotebookSelection('unknown-project');
148+
manager.clearNotebookSelection('unknown-project');
149+
});
150+
});
151+
});
152+
126153
suite('storeOriginalProject', () => {
127154
test('should store both project and current notebook ID', () => {
128155
manager.storeOriginalProject('project-123', mockProject, 'notebook-456');

src/notebooks/deepnote/deepnoteSerializer.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,11 @@ export class DeepnoteNotebookSerializer implements NotebookSerializer {
6767
* @param token Cancellation token (unused)
6868
* @returns Promise resolving to notebook data
6969
*/
70-
async deserializeNotebook(content: Uint8Array, token: CancellationToken): Promise<NotebookData> {
70+
async deserializeNotebook(
71+
content: Uint8Array,
72+
token: CancellationToken,
73+
notebookId?: string
74+
): Promise<NotebookData> {
7175
logger.debug('DeepnoteSerializer: Deserializing Deepnote notebook');
7276

7377
if (token?.isCancellationRequested) {
@@ -90,16 +94,16 @@ export class DeepnoteNotebookSerializer implements NotebookSerializer {
9094
}
9195

9296
const projectId = deepnoteFile.project.id;
93-
const notebookId = this.findCurrentNotebookId(projectId);
97+
const resolvedNotebookId = notebookId ?? this.findCurrentNotebookId(projectId);
9498

95-
logger.debug(`DeepnoteSerializer: Project ID: ${projectId}, Selected notebook ID: ${notebookId}`);
99+
logger.debug(`DeepnoteSerializer: Project ID: ${projectId}, Selected notebook ID: ${resolvedNotebookId}`);
96100

97101
if (deepnoteFile.project.notebooks.length === 0) {
98102
throw new Error('Deepnote project contains no notebooks.');
99103
}
100104

101-
const selectedNotebook = notebookId
102-
? deepnoteFile.project.notebooks.find((nb) => nb.id === notebookId)
105+
const selectedNotebook = resolvedNotebookId
106+
? deepnoteFile.project.notebooks.find((nb) => nb.id === resolvedNotebookId)
103107
: this.findDefaultNotebook(deepnoteFile);
104108

105109
if (!selectedNotebook) {

src/notebooks/deepnote/deepnoteSerializer.unit.test.ts

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,34 @@ project:
147147
/no notebooks|notebooks.*must contain at least 1/i
148148
);
149149
});
150+
151+
test('should deserialize the specified notebook when notebookId is passed', async () => {
152+
const content = projectToYaml(mockProject);
153+
const result = await serializer.deserializeNotebook(content, {} as any, 'notebook-2');
154+
155+
assert.strictEqual(result.metadata?.deepnoteNotebookId, 'notebook-2');
156+
assert.strictEqual(result.metadata?.deepnoteNotebookName, 'Second Notebook');
157+
assert.strictEqual(result.cells.length, 1);
158+
assert.include(result.cells[0].value, 'Title');
159+
});
160+
161+
test('should ignore stored selection when explicit notebookId is provided', async () => {
162+
manager.selectNotebookForProject('project-123', 'notebook-1');
163+
const content = projectToYaml(mockProject);
164+
const result = await serializer.deserializeNotebook(content, {} as any, 'notebook-2');
165+
166+
assert.strictEqual(result.metadata?.deepnoteNotebookId, 'notebook-2');
167+
assert.strictEqual(result.metadata?.deepnoteNotebookName, 'Second Notebook');
168+
});
169+
170+
test('should fall back to findCurrentNotebookId when notebookId is undefined', async () => {
171+
manager.selectNotebookForProject('project-123', 'notebook-1');
172+
const content = projectToYaml(mockProject);
173+
const result = await serializer.deserializeNotebook(content, {} as any);
174+
175+
assert.strictEqual(result.metadata?.deepnoteNotebookId, 'notebook-1');
176+
assert.strictEqual(result.metadata?.deepnoteNotebookName, 'First Notebook');
177+
});
150178
});
151179

152180
suite('serializeNotebook', () => {
@@ -420,6 +448,36 @@ project:
420448
manager.selectNotebookForProject('project-123', 'notebook-3');
421449
assert.strictEqual(serializer.findCurrentNotebookId('project-123'), 'notebook-3');
422450
});
451+
452+
test('should return undefined when selection is cleared and no active editor', () => {
453+
manager.selectNotebookForProject('project-123', 'notebook-456');
454+
manager.clearNotebookSelection('project-123');
455+
456+
const result = serializer.findCurrentNotebookId('project-123');
457+
458+
assert.strictEqual(result, undefined);
459+
});
460+
461+
test('should fall back to active editor after selection is cleared', () => {
462+
manager.selectNotebookForProject('project-123', 'stored-wrong');
463+
manager.clearNotebookSelection('project-123');
464+
465+
const mockActiveNotebook = {
466+
notebookType: 'deepnote',
467+
metadata: {
468+
deepnoteProjectId: 'project-123',
469+
deepnoteNotebookId: 'active-editor-notebook'
470+
}
471+
};
472+
473+
when(mockedVSCodeNamespaces.window.activeNotebookEditor).thenReturn({
474+
notebook: mockActiveNotebook
475+
} as any);
476+
477+
const result = serializer.findCurrentNotebookId('project-123');
478+
479+
assert.strictEqual(result, 'active-editor-notebook');
480+
});
423481
});
424482

425483
suite('component integration', () => {

src/notebooks/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ export interface ProjectIntegration {
3737

3838
export const IDeepnoteNotebookManager = Symbol('IDeepnoteNotebookManager');
3939
export interface IDeepnoteNotebookManager {
40+
clearNotebookSelection(projectId: string): void;
4041
getCurrentNotebookId(projectId: string): string | undefined;
4142
getOriginalProject(projectId: string): DeepnoteProject | undefined;
4243
getTheSelectedNotebookForAProject(projectId: string): string | undefined;

0 commit comments

Comments
 (0)