diff --git a/build/mocha-esm-loader.js b/build/mocha-esm-loader.js index 7a45afa78b..f948d8aae6 100644 --- a/build/mocha-esm-loader.js +++ b/build/mocha-esm-loader.js @@ -335,6 +335,7 @@ export async function load(url, context, nextLoad) { export const EndOfLine = createClassProxy('EndOfLine'); export const PortAutoForwardAction = createClassProxy('PortAutoForwardAction'); export const PortAttributes = createClassProxy('PortAttributes'); + export const TabInputNotebook = createClassProxy('TabInputNotebook'); `, shortCircuit: true }; diff --git a/package.json b/package.json index dfba8b252b..c9f71a0d14 100644 --- a/package.json +++ b/package.json @@ -93,6 +93,12 @@ "category": "Deepnote", "icon": "$(reveal)" }, + { + "command": "deepnote.copyNotebookDetails", + "title": "%deepnote.commands.copyNotebookDetails.title%", + "category": "Deepnote", + "icon": "$(copy)" + }, { "command": "deepnote.enableSnapshots", "title": "%deepnote.commands.enableSnapshots.title%", diff --git a/package.nls.json b/package.nls.json index 35ee95ae66..d13aed9b17 100644 --- a/package.nls.json +++ b/package.nls.json @@ -250,6 +250,7 @@ "deepnote.commands.openNotebook.title": "Open Notebook", "deepnote.commands.openFile.title": "Open File", "deepnote.commands.revealInExplorer.title": "Reveal in Explorer", + "deepnote.commands.copyNotebookDetails.title": "Copy Active Deepnote Notebook Details", "deepnote.commands.enableSnapshots.title": "Enable Snapshots", "deepnote.commands.disableSnapshots.title": "Disable Snapshots", "deepnote.commands.manageIntegrations.title": "Manage Integrations", diff --git a/src/kernels/deepnote/deepnoteServerStarter.node.ts b/src/kernels/deepnote/deepnoteServerStarter.node.ts index 112f7f0e09..0ea58022cd 100644 --- a/src/kernels/deepnote/deepnoteServerStarter.node.ts +++ b/src/kernels/deepnote/deepnoteServerStarter.node.ts @@ -280,7 +280,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension throw new DeepnoteServerStartupError( interpreter.uri.fsPath, - serverInfo?.jupyterPort ?? 0, + 0, 'unknown', capturedOutput?.stdout || '', capturedOutput?.stderr || '', @@ -402,6 +402,16 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension */ private monitorServerOutput(fileKey: string, serverInfo: DeepnoteServerInfo): void { const proc = serverInfo.process; + const existing = this.disposablesByFile.get(fileKey); + if (existing) { + for (const d of existing) { + try { + d.dispose(); + } catch (ex) { + logger.warn(`Error disposing listener for ${fileKey}`, ex); + } + } + } const disposables: IDisposable[] = []; this.disposablesByFile.set(fileKey, disposables); diff --git a/src/kernels/deepnote/deepnoteTestHelpers.node.ts b/src/kernels/deepnote/deepnoteTestHelpers.node.ts index 524c6e0e47..42d1ae19ec 100644 --- a/src/kernels/deepnote/deepnoteTestHelpers.node.ts +++ b/src/kernels/deepnote/deepnoteTestHelpers.node.ts @@ -5,11 +5,66 @@ import type { ChildProcess } from 'node:child_process'; * Satisfies the ChildProcess interface with minimal stub values. */ export function createMockChildProcess(overrides?: Partial): ChildProcess { - return { + const mockProcess: ChildProcess = { pid: undefined, + stdio: [null, null, null, null, null], + stdin: null, stdout: null, stderr: null, exitCode: null, + killed: false, + connected: false, + signalCode: null, + spawnargs: [], + spawnfile: '', + kill: () => true, + send: () => true, + disconnect: () => true, + unref: () => true, + ref: () => true, + addListener: function () { + return this; + }, + emit: () => true, + on: function () { + return this; + }, + once: function () { + return this; + }, + removeListener: function () { + return this; + }, + removeAllListeners: function () { + return this; + }, + prependListener: function () { + return this; + }, + prependOnceListener: function () { + return this; + }, + [Symbol.dispose]: () => { + return undefined; + }, + off: function () { + return this; + }, + setMaxListeners: function () { + return this; + }, + getMaxListeners: () => 10, + listeners: function () { + return []; + }, + rawListeners: function () { + return []; + }, + eventNames: function () { + return []; + }, + listenerCount: () => 0, ...overrides - } as ChildProcess; + }; + return mockProcess; } diff --git a/src/notebooks/deepnote/deepnoteActivationService.ts b/src/notebooks/deepnote/deepnoteActivationService.ts index 96c35288cd..edd3474a00 100644 --- a/src/notebooks/deepnote/deepnoteActivationService.ts +++ b/src/notebooks/deepnote/deepnoteActivationService.ts @@ -1,5 +1,5 @@ import { inject, injectable, optional } from 'inversify'; -import { commands, l10n, workspace, window, type Disposable, type NotebookDocumentContentOptions } from 'vscode'; +import { commands, l10n, window, workspace, type Disposable, type NotebookDocumentContentOptions } from 'vscode'; import { IExtensionSyncActivationService } from '../../platform/activation/types'; import { IExtensionContext } from '../../platform/common/types'; @@ -51,6 +51,11 @@ export class DeepnoteActivationService implements IExtensionSyncActivationServic this.registerSerializer(); this.extensionContext.subscriptions.push(this.editProtection); + this.extensionContext.subscriptions.push( + workspace.onDidOpenNotebookDocument((doc) => { + void this.serializer.verifyDeserializedNotebook(doc); + }) + ); this.extensionContext.subscriptions.push( workspace.onDidChangeConfiguration((event) => { if (event.affectsConfiguration('deepnote.snapshots.enabled')) { diff --git a/src/notebooks/deepnote/deepnoteExplorerView.ts b/src/notebooks/deepnote/deepnoteExplorerView.ts index 33599da2f0..9c41083e5f 100644 --- a/src/notebooks/deepnote/deepnoteExplorerView.ts +++ b/src/notebooks/deepnote/deepnoteExplorerView.ts @@ -260,7 +260,7 @@ export class DeepnoteExplorerView { await this.treeDataProvider.refreshNotebook(treeItem.context.projectId); // Optionally open the duplicated notebook - this.manager.selectNotebookForProject(treeItem.context.projectId, newNotebook.id); + this.registerNotebookOpenIntent(treeItem.context.projectId, newNotebook.id); const notebookUri = fileUri.with({ query: `notebook=${newNotebook.id}` }); const document = await workspace.openNotebookDocument(notebookUri); await window.showNotebookDocument(document, { @@ -508,7 +508,7 @@ export class DeepnoteExplorerView { await this.treeDataProvider.refreshNotebook(projectData.project.id); // Open the new notebook - this.manager.selectNotebookForProject(projectData.project.id, notebookId); + this.registerNotebookOpenIntent(projectData.project.id, notebookId); const notebookUri = fileUri.with({ query: `notebook=${notebookId}` }); const document = await workspace.openNotebookDocument(notebookUri); await window.showNotebookDocument(document, { @@ -517,6 +517,10 @@ export class DeepnoteExplorerView { }); } + private registerNotebookOpenIntent(projectId: string, notebookId: string): void { + this.manager.queueNotebookResolution(projectId, notebookId); + } + private refreshExplorer(): void { this.treeDataProvider.refresh(); } @@ -537,7 +541,7 @@ export class DeepnoteExplorerView { console.log(`Selecting notebook in manager.`); - this.manager.selectNotebookForProject(context.projectId, context.notebookId); + this.registerNotebookOpenIntent(context.projectId, context.notebookId); console.log(`Opening notebook document.`, fileUri); @@ -701,7 +705,7 @@ export class DeepnoteExplorerView { this.treeDataProvider.refresh(); - this.manager.selectNotebookForProject(projectId, notebookId); + this.registerNotebookOpenIntent(projectId, notebookId); const notebookUri = fileUri.with({ query: `notebook=${notebookId}` }); const document = await workspace.openNotebookDocument(notebookUri); diff --git a/src/notebooks/deepnote/deepnoteFileChangeWatcher.ts b/src/notebooks/deepnote/deepnoteFileChangeWatcher.ts index b7da2080f3..8e54997f2e 100644 --- a/src/notebooks/deepnote/deepnoteFileChangeWatcher.ts +++ b/src/notebooks/deepnote/deepnoteFileChangeWatcher.ts @@ -1,3 +1,5 @@ +import type { DeepnoteBlock } from '@deepnote/blocks'; +import { inject, injectable, optional } from 'inversify'; import { CancellationTokenSource, NotebookCell, @@ -10,13 +12,11 @@ import { WorkspaceEdit, workspace } from 'vscode'; -import { inject, injectable, optional } from 'inversify'; -import type { DeepnoteBlock } from '@deepnote/blocks'; -import { IControllerRegistration } from '../controllers/types'; import { IExtensionSyncActivationService } from '../../platform/activation/types'; import { IDisposableRegistry } from '../../platform/common/types'; import { logger } from '../../platform/logging'; +import { IControllerRegistration } from '../controllers/types'; import { IDeepnoteNotebookManager } from '../types'; import { DeepnoteDataConverter } from './deepnoteDataConverter'; import { DeepnoteNotebookSerializer } from './deepnoteSerializer'; @@ -99,6 +99,15 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic this.disposables.push(watcher.onDidCreate((uri) => this.handleFileChange(uri))); this.disposables.push({ dispose: () => this.clearAllTimers() }); + this.disposables.push( + workspace.onDidSaveNotebookDocument((notebook) => { + if (notebook.notebookType === 'deepnote') { + const fileUri = notebook.uri.with({ query: '', fragment: '' }); + this.markSelfWrite(fileUri); + } + }) + ); + if (this.snapshotService) { this.disposables.push( this.snapshotService.onFileWritten((uri) => { @@ -122,6 +131,13 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic } } + public async applyNotebookEdits(uri: Uri, edits: NotebookEdit[]): Promise { + const wsEdit = new WorkspaceEdit(); + wsEdit.set(uri, edits); + + return workspace.applyEdit(wsEdit); + } + private clearAllTimers(): void { for (const timer of this.debounceTimers.values()) { clearTimeout(timer); @@ -145,7 +161,7 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic * Consumes a self-write marker. Returns true if the fs event was self-triggered. */ private consumeSelfWrite(uri: Uri): boolean { - const key = uri.toString(); + const key = this.normalizeFileUri(uri); // Check snapshot self-writes first if (this.snapshotSelfWriteUris.has(key)) { @@ -187,6 +203,7 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic if (liveCells.length !== newCells.length) { return true; } + return liveCells.some( (live, i) => live.kind !== newCells[i].kind || @@ -275,10 +292,12 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic return; } + const targetNotebookId = notebook.metadata?.deepnoteNotebookId as string | undefined; + const tokenSource = new CancellationTokenSource(); let newData; try { - newData = await this.serializer.deserializeNotebook(content, tokenSource.token); + newData = await this.serializer.deserializeNotebook(content, tokenSource.token, targetNotebookId); } catch (error) { logger.warn(`[FileChangeWatcher] Failed to parse changed file: ${fileUri.path}`, error); return; @@ -332,22 +351,53 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic } } - const wsEdit = new WorkspaceEdit(); - wsEdit.set(notebook.uri, edits); - const applied = await workspace.applyEdit(wsEdit); + // Apply the edit to update in-memory cells immediately (responsive UX). + const applied = await this.applyNotebookEdits(notebook.uri, edits); if (!applied) { logger.warn(`[FileChangeWatcher] Failed to apply edit: ${notebook.uri.path}`); return; } - // Save to sync mtime — mark as self-write first - this.markSelfWrite(notebook.uri); + // Serialize the notebook and write canonical bytes to disk. This ensures + // the file on disk matches what VS Code's serializer would produce. + // Then save via workspace.save() to clear dirty state and update VS Code's + // internal mtime tracker. Since WE just wrote the file, its mtime is from + // our write (not the external change), avoiding the "content is newer" conflict. + const serializeTokenSource = new CancellationTokenSource(); try { - await workspace.save(notebook.uri); - } catch (error) { - this.consumeSelfWrite(notebook.uri); - throw error; + const serializedBytes = await this.serializer.serializeNotebook(newData, serializeTokenSource.token); + + // Write to disk first — this updates the file mtime to "now" + this.markSelfWrite(fileUri); + try { + await workspace.fs.writeFile(fileUri, serializedBytes); + } catch (writeError) { + this.consumeSelfWrite(fileUri); + logger.warn(`[FileChangeWatcher] Failed to write synced file: ${fileUri.path}`, writeError); + + // Bail out — without a successful write, workspace.save() would hit + // the stale mtime and show a "content is newer" conflict dialog. + // The notebook stays dirty but cells are already up-to-date from applyEdit. + return; + } + + // Save to clear dirty state. VS Code serializes (same bytes) and sees the + // mtime from our recent write, so no "content is newer" conflict. + // NOTE: onDidSaveNotebookDocument handles the self-write mark for this save. + try { + const saved = await workspace.save(notebook.uri); + if (!saved) { + logger.warn(`[FileChangeWatcher] Save after sync write returned undefined: ${notebook.uri.path}`); + return; + } + } catch (saveError) { + logger.warn(`[FileChangeWatcher] Save after sync write failed: ${notebook.uri.path}`, saveError); + } + } catch (serializeError) { + logger.warn(`[FileChangeWatcher] Failed to serialize for sync write: ${fileUri.path}`, serializeError); + } finally { + serializeTokenSource.dispose(); } logger.info(`[FileChangeWatcher] Reloaded notebook from external change: ${notebook.uri.path}`); @@ -452,9 +502,13 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic } } if (metadataEdits.length > 0) { - const wsEdit = new WorkspaceEdit(); - wsEdit.set(notebook.uri, metadataEdits); - await workspace.applyEdit(wsEdit); + const metadataApplied = await this.applyNotebookEdits(notebook.uri, metadataEdits); + if (!metadataApplied) { + logger.warn( + `[FileChangeWatcher] Failed to restore block IDs via execution path: ${notebook.uri.path}` + ); + return; + } } logger.info(`[FileChangeWatcher] Updated notebook outputs via execution API: ${notebook.uri.path}`); @@ -482,9 +536,7 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic ); } - const wsEdit = new WorkspaceEdit(); - wsEdit.set(notebook.uri, replaceEdits); - const applied = await workspace.applyEdit(wsEdit); + const applied = await this.applyNotebookEdits(notebook.uri, replaceEdits); if (!applied) { logger.warn(`[FileChangeWatcher] Failed to apply snapshot outputs: ${notebook.uri.path}`); @@ -503,19 +555,16 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic }) ); } - const metaEdit = new WorkspaceEdit(); - metaEdit.set(notebook.uri, metadataEdits); - await workspace.applyEdit(metaEdit); - - // Save to sync mtime — mark as self-write first - this.markSelfWrite(notebook.uri); - try { - await workspace.save(notebook.uri); - } catch (error) { - this.consumeSelfWrite(notebook.uri); - throw error; + const metadataApplied = await this.applyNotebookEdits(notebook.uri, metadataEdits); + if (!metadataApplied) { + logger.warn(`[FileChangeWatcher] Failed to restore block IDs after replaceCells: ${notebook.uri.path}`); + return; } + // Save to sync mtime. + // NOTE: onDidSaveNotebookDocument handles the self-write mark for this save. + await workspace.save(notebook.uri); + logger.info(`[FileChangeWatcher] Updated notebook outputs from external snapshot: ${notebook.uri.path}`); } @@ -523,6 +572,15 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic return (metadata?.__deepnoteBlockId ?? metadata?.id) as string | undefined; } + /** + * Normalizes a URI to the underlying file path by stripping query and fragment. + * Notebook URIs include query params (e.g., ?notebook=id) but the filesystem + * watcher fires with the raw file URI — keys must match for self-write detection. + */ + private normalizeFileUri(uri: Uri): string { + return uri.with({ query: '', fragment: '' }).toString(); + } + private handleFileChange(uri: Uri): void { // Deterministic self-write check — no timers involved if (this.consumeSelfWrite(uri)) { @@ -577,11 +635,12 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic } /** - * Marks a URI as about to be written by us (workspace.save). - * Call before workspace.save() to prevent the resulting fs event from triggering a reload. + * Marks a URI as about to be written by us. + * For workspace.fs.writeFile(): call this before the write. + * For workspace.save(): do NOT call this — onDidSaveNotebookDocument handles it. */ private markSelfWrite(uri: Uri): void { - const key = uri.toString(); + const key = this.normalizeFileUri(uri); const count = this.selfWriteCounts.get(key) ?? 0; this.selfWriteCounts.set(key, count + 1); diff --git a/src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts b/src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts index c9229820e2..be90fbfa65 100644 --- a/src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts @@ -1,21 +1,114 @@ +import { DeepnoteFile, serializeDeepnoteFile } from '@deepnote/blocks'; import { assert } from 'chai'; +import { mkdtempSync, rmSync } from 'fs'; +import { tmpdir } from 'os'; import * as sinon from 'sinon'; -import { anything, instance, mock, when } from 'ts-mockito'; -import { Disposable, EventEmitter, FileSystemWatcher, NotebookCellKind, NotebookDocument, Uri } from 'vscode'; +import { anything, instance, mock, reset, resetCalls, when } from 'ts-mockito'; +import { + Disposable, + EventEmitter, + FileSystemWatcher, + NotebookCellData, + NotebookCellKind, + NotebookDocument, + NotebookEdit, + Uri +} from 'vscode'; +import { join } from '../../platform/vscode-path/path'; +import { logger } from '../../platform/logging'; -import type { IControllerRegistration } from '../controllers/types'; import type { IDisposableRegistry } from '../../platform/common/types'; import type { DeepnoteOutput, DeepnoteProject } from '../../platform/deepnote/deepnoteTypes'; import { mockedVSCodeNamespaces, resetVSCodeMocks } from '../../test/vscode-mock'; +import type { IControllerRegistration } from '../controllers/types'; import { IDeepnoteNotebookManager } from '../types'; +import { DeepnoteDataConverter } from './deepnoteDataConverter'; import { DeepnoteFileChangeWatcher } from './deepnoteFileChangeWatcher'; import { SnapshotService } from './snapshots/snapshotService'; +const validProject: DeepnoteFile = { + version: '1.0.0', + metadata: { createdAt: '2025-01-01T00:00:00Z' }, + project: { + id: 'project-1', + name: 'Test Project', + notebooks: [ + { + id: 'notebook-1', + name: 'Notebook 1', + blocks: [ + { + id: 'block-1', + type: 'code', + sortingKey: 'a0', + blockGroup: '1', + content: 'print("hello")', + metadata: {} + } + ] + } + ] + } +}; + +const multiNotebookProject: DeepnoteFile = { + version: '1.0.0', + metadata: { createdAt: '2025-01-01T00:00:00Z' }, + project: { + id: 'project-1', + name: 'Multi Notebook Project', + notebooks: [ + { + id: 'notebook-1', + name: 'Notebook 1', + blocks: [ + { + id: 'block-nb1', + type: 'code', + sortingKey: 'a0', + blockGroup: '1', + content: 'print("nb1-new")', + metadata: {} + } + ] + }, + { + id: 'notebook-2', + name: 'Notebook 2', + blocks: [ + { + id: 'block-nb2', + type: 'code', + sortingKey: 'a0', + blockGroup: '1', + content: 'print("nb2-new")', + metadata: {} + } + ] + } + ] + } +}; + +const multiNotebookYaml = serializeDeepnoteFile(multiNotebookProject); + const waitForTimeoutMs = 5000; const waitForIntervalMs = 50; const debounceWaitMs = 800; const rapidChangeIntervalMs = 100; const autoSaveGraceMs = 200; +const postSnapshotReadGraceMs = 100; + +interface NotebookEditCapture { + uriKey: string; + cellSourceJoined: string; +} + +interface SnapshotInteractionCapture { + cellSourcesJoined: string; + outputPlainJoined: string; + uriKey: string; +} /** * Polls until a condition is met or a timeout is reached. @@ -35,11 +128,27 @@ async function waitFor( } suite('DeepnoteFileChangeWatcher', () => { + let testFixturesDir: string; + + suiteSetup(() => { + testFixturesDir = mkdtempSync(join(tmpdir(), 'deepnote-fcw-')); + }); + + suiteTeardown(() => { + rmSync(testFixturesDir, { recursive: true, force: true }); + }); + + function testFileUri(...pathSegments: string[]): Uri { + return Uri.joinPath(Uri.file(testFixturesDir), ...pathSegments); + } + let watcher: DeepnoteFileChangeWatcher; let mockDisposables: IDisposableRegistry; + let mockedNotebookManager: IDeepnoteNotebookManager; let mockNotebookManager: IDeepnoteNotebookManager; let onDidChangeFile: EventEmitter; let onDidCreateFile: EventEmitter; + let onDidSaveNotebook: EventEmitter; let readFileCalls: number; let applyEditCount: number; let saveCount: number; @@ -51,7 +160,13 @@ suite('DeepnoteFileChangeWatcher', () => { saveCount = 0; mockDisposables = []; - mockNotebookManager = instance(mock()); + + mockedNotebookManager = mock(); + when(mockedNotebookManager.consumePendingNotebookResolution(anything())).thenReturn(undefined); + when(mockedNotebookManager.getOriginalProject(anything())).thenReturn(validProject); + when(mockedNotebookManager.queueNotebookResolution(anything(), anything())).thenReturn(); + when(mockedNotebookManager.updateOriginalProject(anything(), anything())).thenReturn(); + mockNotebookManager = instance(mockedNotebookManager); // Set up FileSystemWatcher mock onDidChangeFile = new EventEmitter(); @@ -63,13 +178,16 @@ suite('DeepnoteFileChangeWatcher', () => { when(mockedVSCodeNamespaces.workspace.createFileSystemWatcher(anything())).thenReturn(instance(fsWatcher)); + onDidSaveNotebook = new EventEmitter(); + when(mockedVSCodeNamespaces.workspace.onDidSaveNotebookDocument).thenReturn(onDidSaveNotebook.event); + when(mockedVSCodeNamespaces.workspace.applyEdit(anything())).thenCall(() => { applyEditCount++; return Promise.resolve(true); }); - when(mockedVSCodeNamespaces.workspace.save(anything())).thenCall(() => { + when(mockedVSCodeNamespaces.workspace.save(anything())).thenCall((uri: Uri) => { saveCount++; - return Promise.resolve(Uri.file('/workspace/test.deepnote')); + return Promise.resolve(uri); }); watcher = new DeepnoteFileChangeWatcher(mockDisposables, mockNotebookManager); @@ -83,6 +201,7 @@ suite('DeepnoteFileChangeWatcher', () => { } onDidChangeFile.dispose(); onDidCreateFile.dispose(); + onDidSaveNotebook.dispose(); }); function createMockNotebook(opts: { @@ -91,20 +210,26 @@ suite('DeepnoteFileChangeWatcher', () => { notebookType?: string; cellCount?: number; metadata?: Record; - cells?: Array<{ - metadata?: Record; - outputs: any[]; - kind?: number; - document?: { getText: () => string; languageId?: string }; - }>; + cells?: Array< + | { + metadata?: Record; + outputs: any[]; + kind?: number; + document?: { getText: () => string; languageId?: string }; + } + | NotebookCellData + >; }): NotebookDocument { const cells = (opts.cells ?? []).map((c) => ({ ...c, kind: c.kind ?? NotebookCellKind.Code, - document: { - getText: c.document?.getText ?? (() => ''), - languageId: c.document?.languageId ?? 'python' - } + document: + 'document' in c && c.document + ? { getText: c.document.getText ?? (() => ''), languageId: c.document.languageId ?? 'python' } + : { + getText: () => ('value' in c ? (c.value as string) : ''), + languageId: 'languageId' in c ? (c.languageId as string) : 'python' + } })); return { @@ -126,6 +251,7 @@ suite('DeepnoteFileChangeWatcher', () => { readFileCalls++; return Promise.resolve(new TextEncoder().encode(yamlContent)); }); + when(mockFs.writeFile(anything(), anything())).thenReturn(Promise.resolve()); when(mockedVSCodeNamespaces.workspace.fs).thenReturn(instance(mockFs)); return mockFs; @@ -149,8 +275,78 @@ project: content: print("hello") `; + suite('save-triggered self-write detection', () => { + test('saving a deepnote notebook should suppress the next FS change event', async () => { + const uri = testFileUri('self-write.deepnote'); + const notebook = createMockNotebook({ + notebookType: 'deepnote', + uri + }); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + setupMockFs(validYaml); + + onDidSaveNotebook.fire(notebook); + onDidChangeFile.fire(uri); + + await new Promise((resolve) => setTimeout(resolve, debounceWaitMs)); + + assert.strictEqual(applyEditCount, 0, 'FS change after deepnote save should be treated as self-write'); + }); + + test('saving a non-deepnote notebook should not suppress FS change events', async function () { + this.timeout(8000); + const fileUri = testFileUri('jupyter-save.deepnote'); + const jupyterNotebook = createMockNotebook({ + cellCount: 0, + notebookType: 'jupyter-notebook', + uri: fileUri + }); + const deepnoteNotebook = createMockNotebook({ + cellCount: 0, + uri: fileUri.with({ query: 'view=1' }) + }); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([jupyterNotebook, deepnoteNotebook]); + setupMockFs(validYaml); + + onDidSaveNotebook.fire(jupyterNotebook); + onDidChangeFile.fire(fileUri); + + await waitFor(() => applyEditCount >= 1); + + assert.isAtLeast(applyEditCount, 1); + }); + + test('self-write is consumed exactly once', async function () { + this.timeout(8000); + const uri = testFileUri('self-write-once.deepnote'); + const notebook = createMockNotebook({ + notebookType: 'deepnote', + uri, + cellCount: 0 + }); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + setupMockFs(validYaml); + + onDidSaveNotebook.fire(notebook); + onDidChangeFile.fire(uri); + + await new Promise((resolve) => setTimeout(resolve, debounceWaitMs)); + + assert.strictEqual(applyEditCount, 0, 'first FS event after save should be skipped'); + + onDidChangeFile.fire(uri); + + await waitFor(() => applyEditCount >= 1); + + assert.isAtLeast(applyEditCount, 1); + }); + }); + test('should skip reload when content matches notebook cells', async () => { - const uri = Uri.file('/workspace/test.deepnote'); + const uri = testFileUri('test.deepnote'); // Create a notebook whose cell content already matches validYaml const notebook = createMockNotebook({ uri, @@ -178,7 +374,7 @@ project: }); test('should reload on external change', async () => { - const uri = Uri.file('/workspace/test.deepnote'); + const uri = testFileUri('test.deepnote'); const notebook = createMockNotebook({ uri, cellCount: 0 }); when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); @@ -195,7 +391,7 @@ project: }); test('should skip snapshot files when no SnapshotService', async () => { - const snapshotUri = Uri.file('/workspace/snapshots/project_abc_latest.snapshot.deepnote'); + const snapshotUri = testFileUri('snapshots', 'project_abc_latest.snapshot.deepnote'); setupMockFs(validYaml); when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([]); @@ -210,7 +406,7 @@ project: }); test('should reload dirty notebooks', async () => { - const uri = Uri.file('/workspace/test.deepnote'); + const uri = testFileUri('test.deepnote'); const notebook = createMockNotebook({ uri, isDirty: true }); when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); @@ -228,7 +424,7 @@ project: }); test('should debounce rapid changes', async () => { - const uri = Uri.file('/workspace/test.deepnote'); + const uri = testFileUri('test.deepnote'); const notebook = createMockNotebook({ uri, cellCount: 0 }); when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); @@ -249,7 +445,7 @@ project: }); test('should handle parse errors gracefully', async () => { - const uri = Uri.file('/workspace/test.deepnote'); + const uri = testFileUri('test.deepnote'); const notebook = createMockNotebook({ uri, cellCount: 0 }); when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); @@ -265,7 +461,7 @@ project: }); test('should preserve live cell outputs during reload', async () => { - const uri = Uri.file('/workspace/test.deepnote'); + const uri = testFileUri('test.deepnote'); const fakeOutput = { items: [{ mime: 'text/plain', data: new Uint8Array([72]) }] }; const notebook = createMockNotebook({ uri, @@ -289,7 +485,7 @@ project: }); test('should reload dirty notebooks and preserve outputs', async () => { - const uri = Uri.file('/workspace/test.deepnote'); + const uri = testFileUri('test.deepnote'); const fakeOutput = { items: [{ mime: 'text/plain', data: new Uint8Array([72]) }] }; const notebook = createMockNotebook({ uri, @@ -314,19 +510,30 @@ project: }); test('should not suppress real changes after auto-save', async function () { - this.timeout(5000); - const uri = Uri.file('/workspace/test.deepnote'); + this.timeout(10_000); + const uri = testFileUri('test.deepnote'); // First change: notebook has no cells, YAML has one cell -> different -> reload const notebook = createMockNotebook({ uri, cellCount: 0, cells: [] }); when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + + // Override save mock to fire onDidSaveNotebook (matching real VS Code behavior). + // The onDidSaveNotebookDocument handler calls markSelfWrite, producing the + // second self-write marker that corresponds to the serializer's save-triggered write. + when(mockedVSCodeNamespaces.workspace.save(anything())).thenCall((saveUri: Uri) => { + saveCount++; + onDidSaveNotebook.fire(notebook); + return Promise.resolve(saveUri); + }); + setupMockFs(validYaml); onDidChangeFile.fire(uri); await waitFor(() => saveCount >= 1); - // The save from the first reload set a self-write marker. - // Simulate the auto-save fs event being consumed (as it would in real VS Code). + // The first reload sets 2 self-write markers (writeFile + save). + // Consume them both with simulated fs events. + onDidChangeFile.fire(uri); onDidChangeFile.fire(uri); // Second real external change: use different YAML content @@ -354,8 +561,78 @@ project: assert.isAtLeast(applyEditCount, 2, 'applyEdit should be called for both external changes'); }); + test('editor→external→editor→external: second external edit must reload (self-write leak regression)', async function () { + this.timeout(15_000); + const uri = testFileUri('self-write-leak.deepnote'); + + when(mockedNotebookManager.getOriginalProject(anything())).thenReturn(multiNotebookProject); + + // Initial state: editor content matches disk — use the real converter + const converter = new DeepnoteDataConverter(); + const nb1 = multiNotebookProject.project.notebooks[0]; + const notebook = createMockNotebook({ + uri, + metadata: { deepnoteProjectId: multiNotebookProject.project.id, deepnoteNotebookId: nb1.id }, + cells: converter.convertBlocksToCells(nb1.blocks) + }); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + setupMockFs(multiNotebookYaml); + + // Real VS Code behavior: workspace.save() fires onDidSaveNotebookDocument. + // executeMainFileSync calls markSelfWrite before workspace.save, AND the + // onDidSaveNotebookDocument handler also calls markSelfWrite — two marks + // for one FS event. This leaks a phantom self-write count. + when(mockedVSCodeNamespaces.workspace.save(anything())).thenCall((saveUri: Uri) => { + saveCount++; + onDidSaveNotebook.fire(notebook); + return Promise.resolve(saveUri); + }); + + // Step 1: editor edit → user saves notebook 1 + onDidSaveNotebook.fire(notebook); + onDidChangeFile.fire(uri); + await new Promise((resolve) => setTimeout(resolve, debounceWaitMs)); + assert.strictEqual(applyEditCount, 0, 'Step 1: editor save FS event should be suppressed'); + + // Step 2: first external edit → triggers executeMainFileSync (reload works) + const externalProject1 = structuredClone(multiNotebookProject); + externalProject1.project.notebooks[0].blocks![0].content = 'print("external-1")'; + setupMockFs(serializeDeepnoteFile(externalProject1)); + + onDidChangeFile.fire(uri); + await waitFor(() => saveCount >= 1); + assert.isAtLeast(applyEditCount, 1, 'Step 2: first external edit should reload'); + + const applyCountAfterFirstReload = applyEditCount; + + // Consume FS events from executeMainFileSync's writeFile + save + onDidChangeFile.fire(uri); + onDidChangeFile.fire(uri); + await new Promise((resolve) => setTimeout(resolve, debounceWaitMs)); + + // Step 3: editor edit → user saves again + onDidSaveNotebook.fire(notebook); + onDidChangeFile.fire(uri); + await new Promise((resolve) => setTimeout(resolve, debounceWaitMs)); + + // Step 4: second external edit → should reload but phantom self-write blocks it + const externalProject2 = structuredClone(multiNotebookProject); + externalProject2.project.notebooks[0].blocks![0].content = 'print("external-2")'; + setupMockFs(serializeDeepnoteFile(externalProject2)); + + onDidChangeFile.fire(uri); + await new Promise((resolve) => setTimeout(resolve, debounceWaitMs + 500)); + + assert.isAbove( + applyEditCount, + applyCountAfterFirstReload, + 'Step 4: second external edit should reload, but phantom self-write from executeMainFileSync leaks and suppresses it' + ); + }); + test('should use atomic edit (single applyEdit for replaceCells + metadata)', async () => { - const uri = Uri.file('/workspace/test.deepnote'); + const uri = testFileUri('test.deepnote'); const notebook = createMockNotebook({ uri, cellCount: 0 }); when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); @@ -363,14 +640,14 @@ project: onDidChangeFile.fire(uri); - await waitFor(() => saveCount > 0); + await waitFor(() => applyEditCount > 0); // Only ONE applyEdit call (atomic: replaceCells + metadata in single WorkspaceEdit) assert.strictEqual(applyEditCount, 1, 'applyEdit should be called exactly once (atomic edit)'); }); test('should skip auto-save-triggered changes via content comparison', async () => { - const uri = Uri.file('/workspace/test.deepnote'); + const uri = testFileUri('test.deepnote'); // Notebook already has the same source as validYaml but with outputs const fakeOutput = { items: [{ mime: 'text/plain', data: new Uint8Array([72]) }] }; const notebook = createMockNotebook({ @@ -458,9 +735,9 @@ project: snapshotApplyEditCount++; return Promise.resolve(true); }); - when(mockedVSCodeNamespaces.workspace.save(anything())).thenCall(() => { + when(mockedVSCodeNamespaces.workspace.save(anything())).thenCall((uri: Uri) => { snapshotSaveCount++; - return Promise.resolve(Uri.file('/workspace/test.deepnote')); + return Promise.resolve(uri); }); snapshotWatcher = new DeepnoteFileChangeWatcher( @@ -480,9 +757,9 @@ project: }); test('should update outputs when snapshot file changes', async () => { - const snapshotUri = Uri.file('/workspace/snapshots/my-project_project-1_latest.snapshot.deepnote'); + const snapshotUri = testFileUri('snapshots', 'my-project_project-1_latest.snapshot.deepnote'); const notebook = createMockNotebook({ - uri: Uri.file('/workspace/test.deepnote'), + uri: testFileUri('test.deepnote'), cells: [ { metadata: { id: 'block-1', type: 'code' }, @@ -509,7 +786,7 @@ project: const noSnapshotWatcher = new DeepnoteFileChangeWatcher(noSnapshotDisposables, mockNotebookManager); noSnapshotWatcher.activate(); - const snapshotUri = Uri.file('/workspace/snapshots/my-project_project-1_latest.snapshot.deepnote'); + const snapshotUri = testFileUri('snapshots', 'my-project_project-1_latest.snapshot.deepnote'); when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([]); @@ -526,9 +803,9 @@ project: }); test('should skip self-triggered snapshot writes via onFileWritten', async () => { - const snapshotUri = Uri.file('/workspace/snapshots/my-project_project-1_latest.snapshot.deepnote'); + const snapshotUri = testFileUri('snapshots', 'my-project_project-1_latest.snapshot.deepnote'); const notebook = createMockNotebook({ - uri: Uri.file('/workspace/test.deepnote'), + uri: testFileUri('test.deepnote'), cells: [{ metadata: { id: 'block-1', type: 'code' }, outputs: [] }] }); @@ -549,7 +826,7 @@ project: test('should skip when snapshots are disabled', async () => { when(mockSnapshotService.isSnapshotsEnabled()).thenReturn(false); - const snapshotUri = Uri.file('/workspace/snapshots/my-project_project-1_latest.snapshot.deepnote'); + const snapshotUri = testFileUri('snapshots', 'my-project_project-1_latest.snapshot.deepnote'); snapshotOnDidChange.fire(snapshotUri); @@ -559,12 +836,10 @@ project: }); test('should debounce rapid snapshot changes for same project', async () => { - const snapshotUri1 = Uri.file( - '/workspace/snapshots/my-project_project-1_2025-01-15T10-31-48.snapshot.deepnote' - ); - const snapshotUri2 = Uri.file('/workspace/snapshots/my-project_project-1_latest.snapshot.deepnote'); + const snapshotUri1 = testFileUri('snapshots', 'my-project_project-1_2025-01-15T10-31-48.snapshot.deepnote'); + const snapshotUri2 = testFileUri('snapshots', 'my-project_project-1_latest.snapshot.deepnote'); const notebook = createMockNotebook({ - uri: Uri.file('/workspace/test.deepnote'), + uri: testFileUri('test.deepnote'), cells: [ { metadata: { id: 'block-1', type: 'code' }, @@ -587,9 +862,9 @@ project: }); test('should handle onDidCreate for new snapshot files', async () => { - const snapshotUri = Uri.file('/workspace/snapshots/my-project_project-1_latest.snapshot.deepnote'); + const snapshotUri = testFileUri('snapshots', 'my-project_project-1_latest.snapshot.deepnote'); const notebook = createMockNotebook({ - uri: Uri.file('/workspace/test.deepnote'), + uri: testFileUri('test.deepnote'), cells: [ { metadata: { id: 'block-1', type: 'code' }, @@ -611,9 +886,9 @@ project: }); test('should skip update when snapshot outputs match live state', async () => { - const snapshotUri = Uri.file('/workspace/snapshots/my-project_project-1_latest.snapshot.deepnote'); + const snapshotUri = testFileUri('snapshots', 'my-project_project-1_latest.snapshot.deepnote'); const notebook = createMockNotebook({ - uri: Uri.file('/workspace/test.deepnote'), + uri: testFileUri('test.deepnote'), cells: [ { metadata: { id: 'block-1', type: 'code' }, @@ -638,7 +913,7 @@ project: data: new TextEncoder().encode('Hello World') }; const notebookWithOutputs = createMockNotebook({ - uri: Uri.file('/workspace/test.deepnote'), + uri: testFileUri('test.deepnote'), cells: [ { metadata: { id: 'block-1', type: 'code' }, @@ -657,10 +932,10 @@ project: }); test('should update outputs when content changed but count is the same', async () => { - const snapshotUri = Uri.file('/workspace/snapshots/my-project_project-1_latest.snapshot.deepnote'); + const snapshotUri = testFileUri('snapshots', 'my-project_project-1_latest.snapshot.deepnote'); const existingOutput = { items: [{ mime: 'text/plain', data: new Uint8Array([72]) }] }; const notebook = createMockNotebook({ - uri: Uri.file('/workspace/test.deepnote'), + uri: testFileUri('test.deepnote'), cells: [ { metadata: { id: 'block-1', type: 'code' }, @@ -682,8 +957,8 @@ project: }); test('should skip main-file reload after snapshot update via self-write tracking', async () => { - const snapshotUri = Uri.file('/workspace/snapshots/my-project_project-1_latest.snapshot.deepnote'); - const notebookUri = Uri.file('/workspace/test.deepnote'); + const snapshotUri = testFileUri('snapshots', 'my-project_project-1_latest.snapshot.deepnote'); + const notebookUri = testFileUri('test.deepnote'); const notebook = createMockNotebook({ uri: notebookUri, cells: [ @@ -726,9 +1001,9 @@ project: }); test('should use two-phase edit for snapshot updates (replaceCells + metadata restore)', async () => { - const snapshotUri = Uri.file('/workspace/snapshots/my-project_project-1_latest.snapshot.deepnote'); + const snapshotUri = testFileUri('snapshots', 'my-project_project-1_latest.snapshot.deepnote'); const notebook = createMockNotebook({ - uri: Uri.file('/workspace/test.deepnote'), + uri: testFileUri('test.deepnote'), cells: [ { metadata: { id: 'block-1', type: 'code' }, @@ -754,9 +1029,9 @@ project: }); test('should call workspace.save after snapshot fallback output update', async () => { - const snapshotUri = Uri.file('/workspace/snapshots/my-project_project-1_latest.snapshot.deepnote'); + const snapshotUri = testFileUri('snapshots', 'my-project_project-1_latest.snapshot.deepnote'); const notebook = createMockNotebook({ - uri: Uri.file('/workspace/test.deepnote'), + uri: testFileUri('test.deepnote'), cells: [ { metadata: { id: 'block-1', type: 'code' }, @@ -778,10 +1053,10 @@ project: }); test('should preserve outputs for cells not covered by snapshot', async () => { - const snapshotUri = Uri.file('/workspace/snapshots/my-project_project-1_latest.snapshot.deepnote'); + const snapshotUri = testFileUri('snapshots', 'my-project_project-1_latest.snapshot.deepnote'); const existingOutput = { items: [{ mime: 'text/plain', data: new Uint8Array([72]) }] }; const notebook = createMockNotebook({ - uri: Uri.file('/workspace/test.deepnote'), + uri: testFileUri('test.deepnote'), cells: [ { metadata: { id: 'block-1', type: 'code' }, @@ -853,10 +1128,10 @@ project: ); fallbackWatcher.activate(); - const snapshotUri = Uri.file('/workspace/snapshots/my-project_project-1_latest.snapshot.deepnote'); + const snapshotUri = testFileUri('snapshots', 'my-project_project-1_latest.snapshot.deepnote'); // Cell has NO id in metadata — simulates VS Code losing metadata after replaceCells const notebook = createMockNotebook({ - uri: Uri.file('/workspace/test.deepnote'), + uri: testFileUri('test.deepnote'), metadata: { deepnoteProjectId: 'project-1', deepnoteNotebookId: 'notebook-1' }, cells: [ { @@ -900,7 +1175,7 @@ project: }); test('should only update cells whose outputs changed (per-cell updates)', async () => { - const snapshotUri = Uri.file('/workspace/snapshots/my-project_project-1_latest.snapshot.deepnote'); + const snapshotUri = testFileUri('snapshots', 'my-project_project-1_latest.snapshot.deepnote'); // Two cells: block-1 has no outputs (will get updated), block-2 already has matching outputs const outputItem = { @@ -908,7 +1183,7 @@ project: data: new TextEncoder().encode('Existing output') }; const notebook = createMockNotebook({ - uri: Uri.file('/workspace/test.deepnote'), + uri: testFileUri('test.deepnote'), cells: [ { metadata: { id: 'block-1', type: 'code' }, @@ -1020,9 +1295,9 @@ project: ); execWatcher.activate(); - const snapshotUri = Uri.file('/workspace/snapshots/my-project_project-1_latest.snapshot.deepnote'); + const snapshotUri = testFileUri('snapshots', 'my-project_project-1_latest.snapshot.deepnote'); const notebook = createMockNotebook({ - uri: Uri.file('/workspace/test.deepnote'), + uri: testFileUri('test.deepnote'), cells: [ { metadata: { id: 'block-1', type: 'code' }, @@ -1053,9 +1328,45 @@ project: }); test('should not apply updates when cells have no block IDs and no fallback', async () => { - const snapshotUri = Uri.file('/workspace/snapshots/my-project_project-1_latest.snapshot.deepnote'); + const noFallbackDisposables: IDisposableRegistry = []; + const noFallbackOnDidChange = new EventEmitter(); + const noFallbackOnDidCreate = new EventEmitter(); + const fsWatcherNf = mock(); + when(fsWatcherNf.onDidChange).thenReturn(noFallbackOnDidChange.event); + when(fsWatcherNf.onDidCreate).thenReturn(noFallbackOnDidCreate.event); + when(fsWatcherNf.dispose()).thenReturn(); + when(mockedVSCodeNamespaces.workspace.createFileSystemWatcher(anything())).thenReturn( + instance(fsWatcherNf) + ); + + let nfApplyEditCount = 0; + when(mockedVSCodeNamespaces.workspace.applyEdit(anything())).thenCall(() => { + nfApplyEditCount++; + return Promise.resolve(true); + }); + + let nfReadSnapshotCount = 0; + const nfSnapshotService = mock(); + when(nfSnapshotService.isSnapshotsEnabled()).thenReturn(true); + when(nfSnapshotService.readSnapshot(anything())).thenCall(() => { + nfReadSnapshotCount++; + return Promise.resolve(snapshotOutputs); + }); + when(nfSnapshotService.onFileWritten(anything())).thenReturn({ dispose: () => {} } as Disposable); + + const nfManager = mock(); + when(nfManager.getOriginalProject(anything())).thenReturn(undefined); + + const nfWatcher = new DeepnoteFileChangeWatcher( + noFallbackDisposables, + instance(nfManager), + instance(nfSnapshotService) + ); + nfWatcher.activate(); + + const snapshotUri = testFileUri('snapshots', 'my-project_project-1_latest.snapshot.deepnote'); const notebook = createMockNotebook({ - uri: Uri.file('/workspace/test.deepnote'), + uri: testFileUri('test.deepnote'), cells: [ { metadata: {}, @@ -1068,16 +1379,19 @@ project: when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); - snapshotOnDidChange.fire(snapshotUri); + noFallbackOnDidChange.fire(snapshotUri); - await waitFor(() => readSnapshotCallCount >= 1); + await waitFor(() => nfReadSnapshotCount >= 1); + await new Promise((resolve) => setTimeout(resolve, postSnapshotReadGraceMs)); - assert.isAtLeast(readSnapshotCallCount, 1, 'readSnapshot should be called'); - assert.strictEqual( - snapshotApplyEditCount, - 0, - 'applyEdit should NOT be called when no block IDs can be resolved' - ); + assert.isAtLeast(nfReadSnapshotCount, 1, 'readSnapshot should be called'); + assert.strictEqual(nfApplyEditCount, 0, 'applyEdit should NOT be called when no block IDs can be resolved'); + + for (const d of noFallbackDisposables) { + d.dispose(); + } + noFallbackOnDidChange.dispose(); + noFallbackOnDidCreate.dispose(); }); test('should fall back to replaceCells when no kernel is active', async () => { @@ -1109,9 +1423,9 @@ project: ); fbWatcher.activate(); - const snapshotUri = Uri.file('/workspace/snapshots/my-project_project-1_latest.snapshot.deepnote'); + const snapshotUri = testFileUri('snapshots', 'my-project_project-1_latest.snapshot.deepnote'); const notebook = createMockNotebook({ - uri: Uri.file('/workspace/test.deepnote'), + uri: testFileUri('test.deepnote'), cells: [ { metadata: { id: 'block-1', type: 'code' }, @@ -1136,5 +1450,1002 @@ project: fbOnDidChange.dispose(); fbOnDidCreate.dispose(); }); + + test('should not save when metadata restore fails after replaceCells fallback', async () => { + const mdDisposables: IDisposableRegistry = []; + const mdOnDidChange = new EventEmitter(); + const mdOnDidCreate = new EventEmitter(); + const fsWatcherMd = mock(); + when(fsWatcherMd.onDidChange).thenReturn(mdOnDidChange.event); + when(fsWatcherMd.onDidCreate).thenReturn(mdOnDidCreate.event); + when(fsWatcherMd.dispose()).thenReturn(); + when(mockedVSCodeNamespaces.workspace.createFileSystemWatcher(anything())).thenReturn( + instance(fsWatcherMd) + ); + + let mdApplyEditInvocation = 0; + let mdSaveCount = 0; + when(mockedVSCodeNamespaces.workspace.applyEdit(anything())).thenCall(() => { + mdApplyEditInvocation++; + return Promise.resolve(mdApplyEditInvocation === 1); + }); + when(mockedVSCodeNamespaces.workspace.save(anything())).thenCall((uri: Uri) => { + mdSaveCount++; + return Promise.resolve(uri); + }); + + const mockedControllerRegistration = mock(); + when(mockedControllerRegistration.getSelected(anything())).thenReturn(undefined); + + const mdWatcher = new DeepnoteFileChangeWatcher( + mdDisposables, + mockNotebookManager, + instance(mockSnapshotService), + instance(mockedControllerRegistration) + ); + mdWatcher.activate(); + + const snapshotUri = testFileUri('snapshots', 'my-project_project-1_latest.snapshot.deepnote'); + const notebook = createMockNotebook({ + uri: testFileUri('metadata-fail-replace.deepnote'), + cells: [ + { + metadata: { id: 'block-1', type: 'code' }, + outputs: [], + kind: NotebookCellKind.Code, + document: { getText: () => 'print("hello")' } + } + ] + }); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + + mdOnDidChange.fire(snapshotUri); + + await waitFor(() => mdApplyEditInvocation >= 2); + + assert.strictEqual( + mdApplyEditInvocation, + 2, + 'replaceCells and metadata restore should each invoke applyEdit once' + ); + assert.strictEqual( + mdSaveCount, + 0, + 'workspace.save must not run when metadata restore fails (would persist wrong cell IDs)' + ); + + for (const d of mdDisposables) { + d.dispose(); + } + mdOnDidChange.dispose(); + mdOnDidCreate.dispose(); + }); + + test('should warn and return when metadata restore fails after execution API with block ID fallback', async () => { + const warnStub = sinon.stub(logger, 'warn'); + + try { + const exMdDisposables: IDisposableRegistry = []; + const exMdOnDidChange = new EventEmitter(); + const exMdOnDidCreate = new EventEmitter(); + const fsWatcherExMd = mock(); + when(fsWatcherExMd.onDidChange).thenReturn(exMdOnDidChange.event); + when(fsWatcherExMd.onDidCreate).thenReturn(exMdOnDidCreate.event); + when(fsWatcherExMd.dispose()).thenReturn(); + when(mockedVSCodeNamespaces.workspace.createFileSystemWatcher(anything())).thenReturn( + instance(fsWatcherExMd) + ); + + let exMdSaveCount = 0; + when(mockedVSCodeNamespaces.workspace.applyEdit(anything())).thenCall(() => Promise.resolve(false)); + when(mockedVSCodeNamespaces.workspace.save(anything())).thenCall((uri: Uri) => { + exMdSaveCount++; + return Promise.resolve(uri); + }); + + const mockVSCodeController = { + createNotebookCellExecution: () => ({ + start: () => {}, + replaceOutput: () => Promise.resolve(), + end: () => {} + }) + }; + const mockedControllerRegistration = mock(); + when(mockedControllerRegistration.getSelected(anything())).thenReturn({ + controller: mockVSCodeController + } as any); + + const mockedManagerEx = mock(); + when(mockedManagerEx.consumePendingNotebookResolution(anything())).thenReturn(undefined); + when(mockedManagerEx.getOriginalProject(anything())).thenReturn(validProject); + when(mockedManagerEx.queueNotebookResolution(anything(), anything())).thenReturn(); + when(mockedManagerEx.updateOriginalProject(anything(), anything())).thenReturn(); + + const exMdWatcher = new DeepnoteFileChangeWatcher( + exMdDisposables, + instance(mockedManagerEx), + instance(mockSnapshotService), + instance(mockedControllerRegistration) + ); + exMdWatcher.activate(); + + const snapshotUri = testFileUri('snapshots', 'my-project_project-1_latest.snapshot.deepnote'); + const notebook = createMockNotebook({ + uri: testFileUri('metadata-fail-exec.deepnote'), + cells: [ + { + metadata: { type: 'code' }, + outputs: [], + kind: NotebookCellKind.Code, + document: { getText: () => 'print("hello")' } + } + ] + }); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + + exMdOnDidChange.fire(snapshotUri); + + await waitFor(() => warnStub.called); + + assert.include( + warnStub.firstCall.args[0] as string, + 'Failed to restore block IDs via execution path', + 'should log when metadata restore fails after execution API' + ); + assert.strictEqual( + exMdSaveCount, + 0, + 'execution API path should not save after failed metadata restore' + ); + + for (const d of exMdDisposables) { + d.dispose(); + } + exMdOnDidChange.dispose(); + exMdOnDidCreate.dispose(); + } finally { + warnStub.restore(); + } + }); + + suite('snapshot and deserialization interaction', () => { + let interactionCaptures: SnapshotInteractionCapture[]; + let snapshotApplyEditStub: sinon.SinonStub; + + setup(() => { + interactionCaptures = []; + + reset(mockedNotebookManager); + when(mockedNotebookManager.consumePendingNotebookResolution(anything())).thenReturn(undefined); + when(mockedNotebookManager.getOriginalProject(anything())).thenReturn(validProject); + when(mockedNotebookManager.queueNotebookResolution(anything(), anything())).thenReturn(); + when(mockedNotebookManager.updateOriginalProject(anything(), anything())).thenReturn(); + resetCalls(mockedNotebookManager); + + snapshotApplyEditStub = sinon.stub(snapshotWatcher, 'applyNotebookEdits').callsFake(async function ( + this: DeepnoteFileChangeWatcher, + ...args: unknown[] + ) { + const uri = args[0] as Uri; + const edits = args[1] as NotebookEdit[]; + + const replaceCellsEdit = edits.find((e) => (e as { newCells?: unknown[] }).newCells?.length); + if (replaceCellsEdit) { + const newCells = ( + replaceCellsEdit as { + newCells: Array<{ + outputs?: Array<{ items: Array<{ data?: Uint8Array }> }>; + value: string; + }>; + } + ).newCells; + const outputPlainJoined = newCells + .map((c) => { + const data = c.outputs?.[0]?.items?.[0]?.data; + + return data ? new TextDecoder().decode(data) : ''; + }) + .filter(Boolean) + .join(';'); + + interactionCaptures.push({ + uriKey: uri.toString(), + cellSourcesJoined: newCells.map((c) => c.value).join('\n'), + outputPlainJoined + }); + } + + return DeepnoteFileChangeWatcher.prototype.applyNotebookEdits.apply(this, [uri, edits]); + }); + }); + + teardown(() => { + snapshotApplyEditStub.restore(); + }); + + test('snapshot change with multi-notebook project applies only matching block outputs per notebook', async () => { + when(mockedNotebookManager.getOriginalProject(anything())).thenReturn(multiNotebookProject); + + const multiOutputs = new Map([ + [ + 'block-nb1', + [ + { + output_type: 'execute_result', + data: { 'text/plain': 'OutputForNb1Only' }, + execution_count: 1 + } as DeepnoteOutput + ] + ], + [ + 'block-nb2', + [ + { + output_type: 'execute_result', + data: { 'text/plain': 'OutputForNb2Only' }, + execution_count: 1 + } as DeepnoteOutput + ] + ] + ]); + when(mockSnapshotService.readSnapshot(anything())).thenCall(() => { + readSnapshotCallCount++; + + return Promise.resolve(multiOutputs); + }); + + const basePath = testFileUri('multi-snap.deepnote'); + const uriNb1 = basePath.with({ query: 'view=1' }); + const uriNb2 = basePath.with({ query: 'view=2' }); + + const notebook1 = createMockNotebook({ + uri: uriNb1, + metadata: { + deepnoteProjectId: 'project-1', + deepnoteNotebookId: 'notebook-1' + }, + cells: [ + { + metadata: { id: 'block-nb1', type: 'code' }, + outputs: [], + kind: NotebookCellKind.Code, + document: { getText: () => 'print("nb1-old")', languageId: 'python' } + } + ] + }); + + const notebook2 = createMockNotebook({ + uri: uriNb2, + metadata: { + deepnoteProjectId: 'project-1', + deepnoteNotebookId: 'notebook-2' + }, + cells: [ + { + metadata: { id: 'block-nb2', type: 'code' }, + outputs: [], + kind: NotebookCellKind.Code, + document: { getText: () => 'print("nb2-old")', languageId: 'python' } + } + ] + }); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook1, notebook2]); + + const snapshotUri = testFileUri('snapshots', 'my-project_project-1_latest.snapshot.deepnote'); + snapshotOnDidChange.fire(snapshotUri); + + await waitFor(() => snapshotApplyEditCount >= 4); + + const byUri = new Map(interactionCaptures.map((c) => [c.uriKey, c])); + + assert.include(byUri.get(uriNb1.toString())?.outputPlainJoined ?? '', 'OutputForNb1Only'); + assert.notInclude(byUri.get(uriNb1.toString())?.outputPlainJoined ?? '', 'OutputForNb2Only'); + + assert.include(byUri.get(uriNb2.toString())?.outputPlainJoined ?? '', 'OutputForNb2Only'); + assert.notInclude(byUri.get(uriNb2.toString())?.outputPlainJoined ?? '', 'OutputForNb1Only'); + }); + + test('main file change after snapshot update deserializes updated cell source', async function () { + this.timeout(10_000); + const notebookUri = testFileUri('after-snap.deepnote'); + const notebook = createMockNotebook({ + uri: notebookUri, + cells: [ + { + metadata: { id: 'block-1', type: 'code' }, + outputs: [], + kind: NotebookCellKind.Code, + document: { getText: () => 'print("hello")', languageId: 'python' } + } + ] + }); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + + const snapshotUri = testFileUri('snapshots', 'my-project_project-1_latest.snapshot.deepnote'); + when(mockSnapshotService.readSnapshot(anything())).thenCall(() => { + readSnapshotCallCount++; + + return Promise.resolve(snapshotOutputs); + }); + + snapshotOnDidChange.fire(snapshotUri); + await waitFor(() => snapshotApplyEditCount >= 2); + + const changedYaml = ` +version: '1.0.0' +metadata: + createdAt: '2025-01-01T00:00:00Z' +project: + id: project-1 + name: Test Project + notebooks: + - id: notebook-1 + name: Notebook 1 + blocks: + - id: block-1 + type: code + sortingKey: a0 + blockGroup: '1' + content: print("after-snapshot-main-sync") +`; + + const mockFs = mock(); + when(mockFs.readFile(anything())).thenCall(() => { + return Promise.resolve(new TextEncoder().encode(changedYaml)); + }); + when(mockFs.writeFile(anything(), anything())).thenReturn(Promise.resolve()); + when(mockedVSCodeNamespaces.workspace.fs).thenReturn(instance(mockFs)); + + // Snapshot save marks self-write; first FS event consumes it without reloading. + snapshotOnDidChange.fire(notebookUri); + await new Promise((resolve) => setTimeout(resolve, debounceWaitMs)); + + snapshotOnDidChange.fire(notebookUri); + + await waitFor(() => + interactionCaptures.some((c) => c.cellSourcesJoined.includes('after-snapshot-main-sync')) + ); + + const mainSyncCapture = interactionCaptures.find((c) => + c.cellSourcesJoined.includes('after-snapshot-main-sync') + ); + + assert.isDefined(mainSyncCapture); + assert.include( + mainSyncCapture!.cellSourcesJoined, + 'after-snapshot-main-sync', + 'main-file sync should deserialize new source after snapshot outputs were applied' + ); + }); + + test('snapshot save self-write is consumed once then external main-file change applies', async function () { + this.timeout(10_000); + const baseUri = testFileUri('snap-self-write.deepnote'); + const notebook = createMockNotebook({ + uri: baseUri, + cells: [ + { + metadata: { id: 'block-1', type: 'code' }, + outputs: [], + kind: NotebookCellKind.Code, + document: { getText: () => 'print("hello")', languageId: 'python' } + } + ] + }); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + + const snapshotUri = testFileUri('snapshots', 'my-project_project-1_latest.snapshot.deepnote'); + snapshotOnDidChange.fire(snapshotUri); + await waitFor(() => snapshotSaveCount >= 1); + + const editsBefore = snapshotApplyEditCount; + + snapshotOnDidChange.fire(baseUri); + await new Promise((resolve) => setTimeout(resolve, debounceWaitMs)); + + assert.strictEqual( + snapshotApplyEditCount, + editsBefore, + 'first main-file FS event after snapshot save should be consumed as self-write' + ); + + const externalYaml = ` +version: '1.0.0' +metadata: + createdAt: '2025-01-01T00:00:00Z' +project: + id: project-1 + name: Test Project + notebooks: + - id: notebook-1 + name: Notebook 1 + blocks: + - id: block-1 + type: code + sortingKey: a0 + blockGroup: '1' + content: print("external-after-self-write") +`; + + const mockFs = mock(); + when(mockFs.readFile(anything())).thenCall(() => { + return Promise.resolve(new TextEncoder().encode(externalYaml)); + }); + when(mockFs.writeFile(anything(), anything())).thenReturn(Promise.resolve()); + when(mockedVSCodeNamespaces.workspace.fs).thenReturn(instance(mockFs)); + + snapshotOnDidChange.fire(baseUri); + + await waitFor(() => + interactionCaptures.some((c) => c.cellSourcesJoined.includes('external-after-self-write')) + ); + + assert.isTrue( + interactionCaptures.some((c) => c.cellSourcesJoined.includes('external-after-self-write')), + 'second main-file change should deserialize external content' + ); + }); + + test('main-file sync runs after in-flight snapshot when both are triggered close together', async function () { + this.timeout(12_000); + let releaseSnapshot!: () => void; + const snapshotGate = new Promise((resolve) => { + releaseSnapshot = resolve; + }); + + when(mockSnapshotService.readSnapshot(anything())).thenCall(() => { + readSnapshotCallCount++; + + return snapshotGate.then(() => snapshotOutputs); + }); + + const baseUri = testFileUri('coalesce.deepnote'); + const notebook = createMockNotebook({ + uri: baseUri, + cells: [ + { + metadata: { id: 'block-1', type: 'code' }, + outputs: [], + kind: NotebookCellKind.Code, + document: { getText: () => 'print("hello")', languageId: 'python' } + } + ] + }); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + + const snapshotUri = testFileUri('snapshots', 'my-project_project-1_latest.snapshot.deepnote'); + snapshotOnDidChange.fire(snapshotUri); + + await waitFor(() => readSnapshotCallCount >= 1); + + const coalescedYaml = ` +version: '1.0.0' +metadata: + createdAt: '2025-01-01T00:00:00Z' +project: + id: project-1 + name: Test Project + notebooks: + - id: notebook-1 + name: Notebook 1 + blocks: + - id: block-1 + type: code + sortingKey: a0 + blockGroup: '1' + content: print("main-wins-after-snapshot") +`; + + const mockFs = mock(); + when(mockFs.readFile(anything())).thenCall(() => { + return Promise.resolve(new TextEncoder().encode(coalescedYaml)); + }); + when(mockFs.writeFile(anything(), anything())).thenReturn(Promise.resolve()); + when(mockedVSCodeNamespaces.workspace.fs).thenReturn(instance(mockFs)); + + snapshotOnDidChange.fire(baseUri); + await new Promise((resolve) => setTimeout(resolve, debounceWaitMs)); + + releaseSnapshot(); + + await waitFor(() => + interactionCaptures.some((c) => c.cellSourcesJoined.includes('main-wins-after-snapshot')) + ); + + assert.isTrue( + interactionCaptures.some((c) => c.cellSourcesJoined.includes('main-wins-after-snapshot')), + 'main-file sync should deserialize disk YAML after snapshot operation completes' + ); + }); + + test('multi-notebook: snapshot outputs then external YAML update keeps per-notebook sources', async function () { + this.timeout(12_000); + when(mockedNotebookManager.getOriginalProject(anything())).thenReturn(multiNotebookProject); + + const multiOutputs = new Map([ + [ + 'block-nb1', + [ + { + output_type: 'execute_result', + data: { 'text/plain': 'SnapNb1' }, + execution_count: 1 + } as DeepnoteOutput + ] + ], + [ + 'block-nb2', + [ + { + output_type: 'execute_result', + data: { 'text/plain': 'SnapNb2' }, + execution_count: 1 + } as DeepnoteOutput + ] + ] + ]); + when(mockSnapshotService.readSnapshot(anything())).thenCall(() => { + readSnapshotCallCount++; + + return Promise.resolve(multiOutputs); + }); + + const basePath = testFileUri('multi-snap-then-yaml.deepnote'); + const uriNb1 = basePath.with({ query: 'view=1' }); + const uriNb2 = basePath.with({ query: 'view=2' }); + + const notebook1 = createMockNotebook({ + uri: uriNb1, + metadata: { + deepnoteProjectId: 'project-1', + deepnoteNotebookId: 'notebook-1' + }, + cells: [ + { + metadata: { id: 'block-nb1', type: 'code' }, + outputs: [], + kind: NotebookCellKind.Code, + document: { getText: () => 'print("nb1-old")', languageId: 'python' } + } + ] + }); + + const notebook2 = createMockNotebook({ + uri: uriNb2, + metadata: { + deepnoteProjectId: 'project-1', + deepnoteNotebookId: 'notebook-2' + }, + cells: [ + { + metadata: { id: 'block-nb2', type: 'code' }, + outputs: [], + kind: NotebookCellKind.Code, + document: { getText: () => 'print("nb2-old")', languageId: 'python' } + } + ] + }); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook1, notebook2]); + + const snapshotUri = testFileUri('snapshots', 'my-project_project-1_latest.snapshot.deepnote'); + snapshotOnDidChange.fire(snapshotUri); + await waitFor(() => snapshotApplyEditCount >= 4); + + const round2Project = structuredClone(multiNotebookProject); + round2Project.project.notebooks[0].blocks![0].content = 'print("nb1-round2")'; + round2Project.project.notebooks[1].blocks![0].content = 'print("nb2-round2")'; + const yamlRound2 = serializeDeepnoteFile(round2Project); + + const mockFs = mock(); + when(mockFs.readFile(anything())).thenCall(() => { + return Promise.resolve(new TextEncoder().encode(yamlRound2)); + }); + when(mockFs.writeFile(anything(), anything())).thenReturn(Promise.resolve()); + when(mockedVSCodeNamespaces.workspace.fs).thenReturn(instance(mockFs)); + + // Two snapshot saves increment self-write count to 2 for the shared base file URI. + snapshotOnDidChange.fire(basePath); + await new Promise((resolve) => setTimeout(resolve, debounceWaitMs)); + snapshotOnDidChange.fire(basePath); + await new Promise((resolve) => setTimeout(resolve, debounceWaitMs)); + + snapshotOnDidChange.fire(basePath); + + await waitFor(() => + interactionCaptures.some( + (c) => c.uriKey === uriNb1.toString() && c.cellSourcesJoined.includes('nb1-round2') + ) + ); + + assert.isTrue( + interactionCaptures.some( + (c) => c.uriKey === uriNb1.toString() && c.outputPlainJoined.includes('SnapNb1') + ), + 'snapshot phase should apply SnapNb1 output to notebook-1' + ); + assert.isTrue( + interactionCaptures.some( + (c) => c.uriKey === uriNb2.toString() && c.outputPlainJoined.includes('SnapNb2') + ), + 'snapshot phase should apply SnapNb2 output to notebook-2' + ); + + const nb1Main = interactionCaptures.find( + (c) => c.uriKey === uriNb1.toString() && c.cellSourcesJoined.includes('nb1-round2') + ); + const nb2Main = interactionCaptures.find( + (c) => c.uriKey === uriNb2.toString() && c.cellSourcesJoined.includes('nb2-round2') + ); + + assert.isDefined(nb1Main); + assert.include(nb1Main!.cellSourcesJoined, 'nb1-round2'); + assert.notInclude(nb1Main!.cellSourcesJoined, 'nb2-round2'); + + assert.isDefined(nb2Main); + assert.include(nb2Main!.cellSourcesJoined, 'nb2-round2'); + assert.notInclude(nb2Main!.cellSourcesJoined, 'nb1-round2'); + }); + + test('snapshot outputs for sibling notebook blocks do not leak into a single open notebook', async () => { + when(mockedNotebookManager.getOriginalProject(anything())).thenReturn(multiNotebookProject); + + const multiOutputs = new Map([ + [ + 'block-nb1', + [ + { + output_type: 'execute_result', + data: { 'text/plain': 'OnlyNb1' }, + execution_count: 1 + } as DeepnoteOutput + ] + ], + [ + 'block-nb2', + [ + { + output_type: 'execute_result', + data: { 'text/plain': 'LeakIfApplied' }, + execution_count: 1 + } as DeepnoteOutput + ] + ] + ]); + when(mockSnapshotService.readSnapshot(anything())).thenCall(() => { + readSnapshotCallCount++; + + return Promise.resolve(multiOutputs); + }); + + const uriNb1 = testFileUri('only-nb1.deepnote'); + const notebook1Only = createMockNotebook({ + uri: uriNb1, + metadata: { + deepnoteProjectId: 'project-1', + deepnoteNotebookId: 'notebook-1' + }, + cells: [ + { + metadata: { id: 'block-nb1', type: 'code' }, + outputs: [], + kind: NotebookCellKind.Code, + document: { getText: () => 'print("nb1-only")', languageId: 'python' } + } + ] + }); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook1Only]); + + const snapshotUri = testFileUri('snapshots', 'my-project_project-1_latest.snapshot.deepnote'); + snapshotOnDidChange.fire(snapshotUri); + + await waitFor(() => snapshotApplyEditCount >= 2); + + const cap = interactionCaptures.find((c) => c.uriKey === uriNb1.toString()); + + assert.isDefined(cap); + assert.include(cap!.outputPlainJoined, 'OnlyNb1'); + assert.notInclude(cap!.outputPlainJoined, 'LeakIfApplied'); + }); + }); + }); + + suite('multi-notebook file sync', () => { + let workspaceSetCaptures: NotebookEditCapture[] = []; + + setup(() => { + reset(mockedNotebookManager); + when(mockedNotebookManager.consumePendingNotebookResolution(anything())).thenReturn(undefined); + when(mockedNotebookManager.getOriginalProject(anything())).thenReturn(multiNotebookProject); + when(mockedNotebookManager.queueNotebookResolution(anything(), anything())).thenReturn(); + when(mockedNotebookManager.updateOriginalProject(anything(), anything())).thenReturn(); + resetCalls(mockedNotebookManager); + workspaceSetCaptures = []; + sinon.stub(watcher, 'applyNotebookEdits' as any).callsFake(async (...args: unknown[]) => { + const uri = args[0] as Uri; + const edits = args[1] as NotebookEdit[]; + + applyEditCount++; + + const replaceCellsEdit = edits.find((e) => e.newCells?.length > 0); + if (replaceCellsEdit) { + workspaceSetCaptures.push({ + uriKey: uri.toString(), + cellSourceJoined: replaceCellsEdit.newCells.map((c: any) => c.value).join('\n') + }); + } + + return true; + }); + }); + + test('should reload each notebook with its own content when multiple notebooks are open', async () => { + const basePath = testFileUri('multi.deepnote'); + const uriNb1 = basePath.with({ query: 'view=1' }); + const uriNb2 = basePath.with({ query: 'view=2' }); + + const notebook1 = createMockNotebook({ + uri: uriNb1, + metadata: { + deepnoteProjectId: 'project-1', + deepnoteNotebookId: 'notebook-1' + }, + cells: [ + { + metadata: { id: 'block-nb1', type: 'code' }, + outputs: [], + kind: NotebookCellKind.Code, + document: { getText: () => 'print("nb1-old")', languageId: 'python' } + } + ] + }); + + const notebook2 = createMockNotebook({ + uri: uriNb2, + metadata: { + deepnoteProjectId: 'project-1', + deepnoteNotebookId: 'notebook-2' + }, + cells: [ + { + metadata: { id: 'block-nb2', type: 'code' }, + outputs: [], + kind: NotebookCellKind.Code, + document: { getText: () => 'print("nb2-old")', languageId: 'python' } + } + ] + }); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook1, notebook2]); + setupMockFs(multiNotebookYaml); + + onDidChangeFile.fire(basePath); + + await waitFor(() => applyEditCount >= 2); + + assert.strictEqual(applyEditCount, 2, 'applyEdit should run once per open notebook'); + assert.strictEqual(workspaceSetCaptures.length, 2, 'each notebook should get a replaceCells edit'); + + const byUri = new Map(workspaceSetCaptures.map((c) => [c.uriKey, c.cellSourceJoined])); + + assert.include(byUri.get(uriNb1.toString()) ?? '', 'nb1-new'); + assert.notInclude(byUri.get(uriNb1.toString()) ?? '', 'nb2-new'); + assert.include(byUri.get(uriNb2.toString()) ?? '', 'nb2-new'); + assert.notInclude(byUri.get(uriNb2.toString()) ?? '', 'nb1-new'); + }); + + test('should not clear notebook selection before processing file change', async () => { + const basePath = testFileUri('multi.deepnote'); + const uriNb1 = basePath.with({ query: 'a=1' }); + const uriNb2 = basePath.with({ query: 'b=2' }); + + const notebook1 = createMockNotebook({ + uri: uriNb1, + metadata: { + deepnoteProjectId: 'project-1', + deepnoteNotebookId: 'notebook-1' + }, + cells: [ + { + metadata: { id: 'block-nb1' }, + outputs: [], + kind: NotebookCellKind.Code, + document: { getText: () => 'print("nb1-old")' } + } + ] + }); + + const notebook2 = createMockNotebook({ + uri: uriNb2, + metadata: { + deepnoteProjectId: 'project-1', + deepnoteNotebookId: 'notebook-2' + }, + cells: [ + { + metadata: { id: 'block-nb2' }, + outputs: [], + kind: NotebookCellKind.Code, + document: { getText: () => 'print("nb2-old")' } + } + ] + }); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook1, notebook2]); + setupMockFs(multiNotebookYaml); + + onDidChangeFile.fire(basePath); + + await waitFor(() => applyEditCount >= 2); + }); + + test('should not corrupt other notebooks when one notebook triggers a file change', async () => { + const basePath = testFileUri('multi.deepnote'); + const uriNb1 = basePath.with({ query: 'n=1' }); + const uriNb2 = basePath.with({ query: 'n=2' }); + + const notebook1 = createMockNotebook({ + uri: uriNb1, + metadata: { + deepnoteProjectId: 'project-1', + deepnoteNotebookId: 'notebook-1' + }, + cells: [ + { + metadata: { id: 'block-nb1' }, + outputs: [], + kind: NotebookCellKind.Code, + document: { getText: () => 'print("nb1-old")' } + } + ] + }); + + const notebook2 = createMockNotebook({ + uri: uriNb2, + metadata: { + deepnoteProjectId: 'project-1', + deepnoteNotebookId: 'notebook-2' + }, + cells: [ + { + metadata: { id: 'block-nb2' }, + outputs: [], + kind: NotebookCellKind.Code, + document: { getText: () => 'print("nb2-old")' } + } + ] + }); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook1, notebook2]); + setupMockFs(multiNotebookYaml); + + onDidChangeFile.fire(basePath); + + await waitFor(() => applyEditCount >= 2); + + const nb1Cells = workspaceSetCaptures.find((c) => c.uriKey === uriNb1.toString())?.cellSourceJoined; + const nb2Cells = workspaceSetCaptures.find((c) => c.uriKey === uriNb2.toString())?.cellSourceJoined; + + assert.isDefined(nb1Cells); + assert.isDefined(nb2Cells); + assert.notStrictEqual(nb1Cells, nb2Cells, 'each open notebook must receive distinct deserialized content'); + + assert.include(nb1Cells!, 'nb1-new'); + assert.include(nb2Cells!, 'nb2-new'); + assert.notInclude(nb1Cells!, 'nb2-new', 'notebook-1 must not receive notebook-2 block content'); + assert.notInclude(nb2Cells!, 'nb1-new', 'notebook-2 must not receive notebook-1 block content'); + }); + + test('external edit to disk should update each open notebook and not be suppressed as self-write', async () => { + const basePath = testFileUri('multi-external.deepnote'); + const uriNb1 = basePath.with({ query: 'view=1' }); + const uriNb2 = basePath.with({ query: 'view=2' }); + + const notebook1 = createMockNotebook({ + uri: uriNb1, + metadata: { + deepnoteProjectId: 'project-1', + deepnoteNotebookId: 'notebook-1' + }, + cells: [ + { + metadata: { id: 'block-nb1', type: 'code' }, + outputs: [], + kind: NotebookCellKind.Code, + document: { getText: () => 'print("nb1-old")', languageId: 'python' } + } + ] + }); + + const notebook2 = createMockNotebook({ + uri: uriNb2, + metadata: { + deepnoteProjectId: 'project-1', + deepnoteNotebookId: 'notebook-2' + }, + cells: [ + { + metadata: { id: 'block-nb2', type: 'code' }, + outputs: [], + kind: NotebookCellKind.Code, + document: { getText: () => 'print("nb2-old")', languageId: 'python' } + } + ] + }); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook1, notebook2]); + setupMockFs(multiNotebookYaml); + + onDidChangeFile.fire(basePath); + + await waitFor(() => applyEditCount >= 2); + + const byUri = new Map(workspaceSetCaptures.map((c) => [c.uriKey, c.cellSourceJoined])); + + assert.include(byUri.get(uriNb1.toString()) ?? '', 'nb1-new'); + assert.include(byUri.get(uriNb2.toString()) ?? '', 'nb2-new'); + }); + + test('external edit after a user save should still be processed', async function () { + this.timeout(8000); + const basePath = testFileUri('multi-save-then-external.deepnote'); + const uriNb1 = basePath.with({ query: 'view=1' }); + const uriNb2 = basePath.with({ query: 'view=2' }); + + const notebook1 = createMockNotebook({ + uri: uriNb1, + metadata: { + deepnoteProjectId: 'project-1', + deepnoteNotebookId: 'notebook-1' + }, + cells: [ + { + metadata: { id: 'block-nb1' }, + outputs: [], + kind: NotebookCellKind.Code, + document: { getText: () => 'print("nb1-old")' } + } + ] + }); + + const notebook2 = createMockNotebook({ + uri: uriNb2, + metadata: { + deepnoteProjectId: 'project-1', + deepnoteNotebookId: 'notebook-2' + }, + cells: [ + { + metadata: { id: 'block-nb2' }, + outputs: [], + kind: NotebookCellKind.Code, + document: { getText: () => 'print("nb2-old")' } + } + ] + }); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook1, notebook2]); + setupMockFs(multiNotebookYaml); + + onDidSaveNotebook.fire(notebook1); + onDidChangeFile.fire(basePath); + + await new Promise((resolve) => setTimeout(resolve, debounceWaitMs)); + + assert.strictEqual(applyEditCount, 0, 'first FS event after save should be suppressed as self-write'); + + onDidChangeFile.fire(basePath); + + await waitFor(() => applyEditCount >= 1); + + assert.isAtLeast(applyEditCount, 1); + }); }); }); diff --git a/src/notebooks/deepnote/deepnoteNotebookInfoStatusBar.ts b/src/notebooks/deepnote/deepnoteNotebookInfoStatusBar.ts new file mode 100644 index 0000000000..3959cf6f91 --- /dev/null +++ b/src/notebooks/deepnote/deepnoteNotebookInfoStatusBar.ts @@ -0,0 +1,154 @@ +import { inject, injectable } from 'inversify'; +import { + commands, + Disposable, + env, + l10n, + NotebookDocument, + StatusBarAlignment, + StatusBarItem, + window, + workspace +} from 'vscode'; + +import { DEEPNOTE_NOTEBOOK_TYPE } from '../../kernels/deepnote/types'; +import { IExtensionSyncActivationService } from '../../platform/activation/types'; +import { Commands } from '../../platform/common/constants'; +import { IDisposableRegistry } from '../../platform/common/types'; + +/** + * Shows the active Deepnote notebook name in the status bar; tooltip and copy action include full debug details (metadata and URI). + */ +@injectable() +export class DeepnoteNotebookInfoStatusBar implements IExtensionSyncActivationService, Disposable { + private readonly disposables: Disposable[] = []; + + private disposed = false; + + private statusBarItem: StatusBarItem | undefined; + + constructor(@inject(IDisposableRegistry) disposableRegistry: IDisposableRegistry) { + disposableRegistry.push(this); + } + + public activate(): void { + this.statusBarItem = window.createStatusBarItem('deepnote.notebookInfo', StatusBarAlignment.Left, 99); + this.statusBarItem.name = l10n.t('Deepnote Notebook Info'); + this.statusBarItem.command = Commands.CopyNotebookDetails; + this.disposables.push(this.statusBarItem); + + this.disposables.push( + commands.registerCommand(Commands.CopyNotebookDetails, () => { + this.copyActiveNotebookDetails(); + }) + ); + + this.disposables.push(window.onDidChangeActiveNotebookEditor(() => this.updateStatusBar())); + + this.disposables.push( + workspace.onDidChangeNotebookDocument((e) => { + if (e.notebook === window.activeNotebookEditor?.notebook) { + this.updateStatusBar(); + } + }) + ); + + this.updateStatusBar(); + } + + public dispose(): void { + if (this.disposed) { + return; + } + + this.disposed = true; + + while (this.disposables.length) { + const disposable = this.disposables.pop(); + + try { + disposable?.dispose(); + } catch { + // ignore + } + } + } + + private copyActiveNotebookDetails(): void { + const notebook = window.activeNotebookEditor?.notebook; + + if (!notebook) { + return; + } + + const info = this.getNotebookDebugInfo(notebook); + + if (!info) { + return; + } + + void env.clipboard.writeText(info.detailsText); + void window.showInformationMessage(l10n.t('Copied notebook details to clipboard.')); + } + + private getNotebookDebugInfo(notebook: NotebookDocument): { detailsText: string; displayName: string } | undefined { + if (notebook.notebookType !== DEEPNOTE_NOTEBOOK_TYPE) { + return undefined; + } + + const metadata = notebook.metadata as { + deepnoteNotebookId?: string; + deepnoteNotebookName?: string; + deepnoteProjectId?: string; + deepnoteProjectName?: string; + deepnoteVersion?: string; + name?: string; + }; + + const displayName = metadata.deepnoteNotebookName ?? metadata.name ?? l10n.t('Deepnote notebook'); + const uriString = notebook.uri.toString(true); + + const lines: string[] = [ + l10n.t('Notebook: {0}', displayName), + l10n.t('Notebook ID: {0}', metadata.deepnoteNotebookId ?? l10n.t('(unknown)')), + l10n.t('Project: {0}', metadata.deepnoteProjectName ?? l10n.t('(unknown)')), + l10n.t('Project ID: {0}', metadata.deepnoteProjectId ?? l10n.t('(unknown)')) + ]; + + if (metadata.deepnoteVersion !== undefined) { + lines.push(l10n.t('Deepnote version: {0}', String(metadata.deepnoteVersion))); + } + + lines.push(l10n.t('URI: {0}', uriString)); + + return { detailsText: lines.join('\n'), displayName }; + } + + private updateStatusBar(): void { + const item = this.statusBarItem; + + if (!item) { + return; + } + + const editor = window.activeNotebookEditor; + + if (!editor) { + item.hide(); + + return; + } + + const info = this.getNotebookDebugInfo(editor.notebook); + + if (!info) { + item.hide(); + + return; + } + + item.text = `$(notebook) ${info.displayName}`; + item.tooltip = [info.detailsText, l10n.t('Click to copy details')].join('\n'); + item.show(); + } +} diff --git a/src/notebooks/deepnote/deepnoteNotebookManager.ts b/src/notebooks/deepnote/deepnoteNotebookManager.ts index 069f3a570c..7d80ce3391 100644 --- a/src/notebooks/deepnote/deepnoteNotebookManager.ts +++ b/src/notebooks/deepnote/deepnoteNotebookManager.ts @@ -2,6 +2,14 @@ import { injectable } from 'inversify'; import { IDeepnoteNotebookManager, ProjectIntegration } from '../types'; import type { DeepnoteProject } from '../../platform/deepnote/deepnoteTypes'; +import { logger } from '../../platform/logging'; + +const pendingNotebookResolutionTtlMs = 60_000; + +interface PendingNotebookResolution { + notebookId: string; + queuedAt: number; +} /** * Centralized manager for tracking Deepnote notebook selections and project state. @@ -11,8 +19,26 @@ import type { DeepnoteProject } from '../../platform/deepnote/deepnoteTypes'; export class DeepnoteNotebookManager implements IDeepnoteNotebookManager { private readonly currentNotebookId = new Map(); private readonly originalProjects = new Map(); + private readonly pendingNotebookResolutions = new Map(); private readonly projectsWithInitNotebookRun = new Set(); - private readonly selectedNotebookByProject = new Map(); + + /** + * Consumes the next short-lived notebook resolution hint for a project. + * These hints are queued immediately before operations that trigger a + * deserialize without explicit URI context. + */ + consumePendingNotebookResolution(projectId: string): string | undefined { + const pendingResolutions = this.getValidPendingNotebookResolutions(projectId); + const nextResolution = pendingResolutions.shift(); + + if (pendingResolutions.length > 0) { + this.pendingNotebookResolutions.set(projectId, pendingResolutions); + } else { + this.pendingNotebookResolutions.delete(projectId); + } + + return nextResolution?.notebookId; + } /** * Gets the currently selected notebook ID for a project. @@ -33,24 +59,20 @@ export class DeepnoteNotebookManager implements IDeepnoteNotebookManager { } /** - * Gets the selected notebook ID for a specific project. - * @param projectId Project identifier - * @returns Selected notebook ID or undefined if not set - */ - getTheSelectedNotebookForAProject(projectId: string): string | undefined { - return this.selectedNotebookByProject.get(projectId); - } - - /** - * Associates a notebook ID with a project to remember user's notebook selection. - * When a Deepnote project contains multiple notebooks, this mapping persists the user's - * choice so we can automatically open the same notebook on subsequent file opens. + * Queues a short-lived notebook resolution hint for the next deserialize. * * @param projectId - The project ID that identifies the Deepnote project - * @param notebookId - The ID of the selected notebook within the project + * @param notebookId - The notebook ID the next deserialize should resolve to */ - selectNotebookForProject(projectId: string, notebookId: string): void { - this.selectedNotebookByProject.set(projectId, notebookId); + queueNotebookResolution(projectId: string, notebookId: string): void { + const pendingResolutions = this.getValidPendingNotebookResolutions(projectId); + + pendingResolutions.push({ + notebookId, + queuedAt: Date.now() + }); + + this.pendingNotebookResolutions.set(projectId, pendingResolutions); } /** @@ -72,13 +94,15 @@ export class DeepnoteNotebookManager implements IDeepnoteNotebookManager { } /** - * Updates the current notebook ID for a project. - * Used when switching notebooks within the same project. + * Updates the stored project data without changing the current notebook selection. + * Used during serialization where we need to cache the updated project state + * but must not alter notebook routing for other open notebooks. * @param projectId Project identifier - * @param notebookId New current notebook ID + * @param project Updated project data to store */ - updateCurrentNotebookId(projectId: string, notebookId: string): void { - this.currentNotebookId.set(projectId, notebookId); + updateOriginalProject(projectId: string, project: DeepnoteProject): void { + const clonedProject = structuredClone(project); + this.originalProjects.set(projectId, clonedProject); } /** @@ -126,4 +150,23 @@ export class DeepnoteNotebookManager implements IDeepnoteNotebookManager { markInitNotebookAsRun(projectId: string): void { this.projectsWithInitNotebookRun.add(projectId); } + + private getValidPendingNotebookResolutions(projectId: string): PendingNotebookResolution[] { + const cutoffTime = Date.now() - pendingNotebookResolutionTtlMs; + const allPendingResolutions = this.pendingNotebookResolutions.get(projectId) ?? []; + logger.debug( + `DeepnoteNotebookManager: getValidPendingNotebookResolutions: projectId=${projectId}, allPendingResolutions=${allPendingResolutions.length}` + ); + logger.debug(JSON.stringify(allPendingResolutions, null, 2)); + const pendingResolutions = (this.pendingNotebookResolutions.get(projectId) ?? []).filter( + (resolution) => resolution.queuedAt >= cutoffTime + ); + + if (pendingResolutions.length === 0) { + this.pendingNotebookResolutions.delete(projectId); + return []; + } + + return pendingResolutions; + } } diff --git a/src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts b/src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts index feb2842848..8a7de52200 100644 --- a/src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts @@ -39,15 +39,6 @@ suite('DeepnoteNotebookManager', () => { assert.strictEqual(result, 'notebook-456'); }); - - test('should return updated notebook ID', () => { - manager.storeOriginalProject('project-123', mockProject, 'notebook-456'); - manager.updateCurrentNotebookId('project-123', 'notebook-789'); - - const result = manager.getCurrentNotebookId('project-123'); - - assert.strictEqual(result, 'notebook-789'); - }); }); suite('getOriginalProject', () => { @@ -66,60 +57,36 @@ suite('DeepnoteNotebookManager', () => { }); }); - suite('getTheSelectedNotebookForAProject', () => { - test('should return undefined for unknown project', () => { - const result = manager.getTheSelectedNotebookForAProject('unknown-project'); + suite('consumePendingNotebookResolution', () => { + test('should return undefined when no pending resolution exists', () => { + const result = manager.consumePendingNotebookResolution('unknown-project'); assert.strictEqual(result, undefined); }); - test('should return notebook ID after setting', () => { - manager.selectNotebookForProject('project-123', 'notebook-456'); - - const result = manager.getTheSelectedNotebookForAProject('project-123'); + test('should consume queued notebook resolutions in order', () => { + manager.queueNotebookResolution('project-123', 'notebook-1'); + manager.queueNotebookResolution('project-123', 'notebook-2'); - assert.strictEqual(result, 'notebook-456'); + assert.strictEqual(manager.consumePendingNotebookResolution('project-123'), 'notebook-1'); + assert.strictEqual(manager.consumePendingNotebookResolution('project-123'), 'notebook-2'); + assert.strictEqual(manager.consumePendingNotebookResolution('project-123'), undefined); }); - test('should handle multiple projects independently', () => { - manager.selectNotebookForProject('project-1', 'notebook-1'); - manager.selectNotebookForProject('project-2', 'notebook-2'); - - const result1 = manager.getTheSelectedNotebookForAProject('project-1'); - const result2 = manager.getTheSelectedNotebookForAProject('project-2'); + test('should keep pending resolutions isolated per project', () => { + manager.queueNotebookResolution('project-1', 'notebook-1'); + manager.queueNotebookResolution('project-2', 'notebook-2'); - assert.strictEqual(result1, 'notebook-1'); - assert.strictEqual(result2, 'notebook-2'); + assert.strictEqual(manager.consumePendingNotebookResolution('project-1'), 'notebook-1'); + assert.strictEqual(manager.consumePendingNotebookResolution('project-2'), 'notebook-2'); }); }); - suite('selectNotebookForProject', () => { - test('should store notebook selection for project', () => { - manager.selectNotebookForProject('project-123', 'notebook-456'); - - const selectedNotebook = manager.getTheSelectedNotebookForAProject('project-123'); - - assert.strictEqual(selectedNotebook, 'notebook-456'); - }); - - test('should overwrite existing selection', () => { - manager.selectNotebookForProject('project-123', 'notebook-456'); - manager.selectNotebookForProject('project-123', 'notebook-789'); - - const result = manager.getTheSelectedNotebookForAProject('project-123'); + suite('queueNotebookResolution', () => { + test('should queue a notebook resolution for later consumption', () => { + manager.queueNotebookResolution('project-123', 'notebook-456'); - assert.strictEqual(result, 'notebook-789'); - }); - - test('should handle multiple projects independently', () => { - manager.selectNotebookForProject('project-1', 'notebook-1'); - manager.selectNotebookForProject('project-2', 'notebook-2'); - - const result1 = manager.getTheSelectedNotebookForAProject('project-1'); - const result2 = manager.getTheSelectedNotebookForAProject('project-2'); - - assert.strictEqual(result1, 'notebook-1'); - assert.strictEqual(result2, 'notebook-2'); + assert.strictEqual(manager.consumePendingNotebookResolution('project-123'), 'notebook-456'); }); }); @@ -154,33 +121,73 @@ suite('DeepnoteNotebookManager', () => { }); }); - suite('updateCurrentNotebookId', () => { - test('should update notebook ID for existing project', () => { + suite('updateOriginalProject', () => { + test('should update project data without changing currentNotebookId', () => { + const updatedProject: DeepnoteProject = { + ...mockProject, + project: { + ...mockProject.project, + name: 'Updated Name Only' + } + }; + manager.storeOriginalProject('project-123', mockProject, 'notebook-456'); - manager.updateCurrentNotebookId('project-123', 'notebook-789'); + manager.updateOriginalProject('project-123', updatedProject); - const result = manager.getCurrentNotebookId('project-123'); + const storedProject = manager.getOriginalProject('project-123'); + const currentNotebookId = manager.getCurrentNotebookId('project-123'); - assert.strictEqual(result, 'notebook-789'); + assert.deepStrictEqual(storedProject, updatedProject); + assert.strictEqual(currentNotebookId, 'notebook-456'); }); - test('should set notebook ID for new project', () => { - manager.updateCurrentNotebookId('new-project', 'notebook-123'); + test('should deep-clone project data so mutations to input do not affect stored state', () => { + const updatedProject: DeepnoteProject = { + ...mockProject, + project: { + ...mockProject.project, + name: 'Before Mutation' + } + }; - const result = manager.getCurrentNotebookId('new-project'); + manager.storeOriginalProject('project-123', mockProject, 'notebook-456'); + manager.updateOriginalProject('project-123', updatedProject); - assert.strictEqual(result, 'notebook-123'); + updatedProject.project.name = 'After Mutation'; + + const storedProject = manager.getOriginalProject('project-123'); + + assert.strictEqual(storedProject?.project.name, 'Before Mutation'); }); - test('should handle multiple projects independently', () => { - manager.updateCurrentNotebookId('project-1', 'notebook-1'); - manager.updateCurrentNotebookId('project-2', 'notebook-2'); + test('should overwrite existing project data while preserving currentNotebookId', () => { + const firstUpdate: DeepnoteProject = { + ...mockProject, + project: { ...mockProject.project, name: 'First Update' } + }; + const secondUpdate: DeepnoteProject = { + ...mockProject, + project: { ...mockProject.project, name: 'Second Update' } + }; - const result1 = manager.getCurrentNotebookId('project-1'); - const result2 = manager.getCurrentNotebookId('project-2'); + manager.storeOriginalProject('project-123', mockProject, 'notebook-456'); + manager.updateOriginalProject('project-123', firstUpdate); + manager.updateOriginalProject('project-123', secondUpdate); + + assert.strictEqual(manager.getCurrentNotebookId('project-123'), 'notebook-456'); + assert.strictEqual(manager.getOriginalProject('project-123')?.project.name, 'Second Update'); + }); + + test('should store project when no currentNotebookId has been set', () => { + const projectOnly: DeepnoteProject = { + ...mockProject, + project: { ...mockProject.project, name: 'No Notebook Id Yet' } + }; + + manager.updateOriginalProject('project-123', projectOnly); - assert.strictEqual(result1, 'notebook-1'); - assert.strictEqual(result2, 'notebook-2'); + assert.strictEqual(manager.getCurrentNotebookId('project-123'), undefined); + assert.deepStrictEqual(manager.getOriginalProject('project-123'), projectOnly); }); }); @@ -272,9 +279,8 @@ suite('DeepnoteNotebookManager', () => { }); test('should update integrations when currentNotebookId is undefined and return true', () => { - // Store project with a notebook ID, then clear it to simulate the edge case - manager.storeOriginalProject('project-123', mockProject, 'notebook-456'); - manager.updateCurrentNotebookId('project-123', undefined as any); + // Use updateOriginalProject which doesn't set currentNotebookId + manager.updateOriginalProject('project-123', mockProject); const integrations: ProjectIntegration[] = [ { id: 'int-1', name: 'PostgreSQL', type: 'pgsql' }, @@ -298,38 +304,17 @@ suite('DeepnoteNotebookManager', () => { suite('integration scenarios', () => { test('should handle complete workflow for multiple projects', () => { manager.storeOriginalProject('project-1', mockProject, 'notebook-1'); - manager.selectNotebookForProject('project-1', 'notebook-1'); - manager.storeOriginalProject('project-2', mockProject, 'notebook-2'); - manager.selectNotebookForProject('project-2', 'notebook-2'); assert.strictEqual(manager.getCurrentNotebookId('project-1'), 'notebook-1'); assert.strictEqual(manager.getCurrentNotebookId('project-2'), 'notebook-2'); - assert.strictEqual(manager.getTheSelectedNotebookForAProject('project-1'), 'notebook-1'); - assert.strictEqual(manager.getTheSelectedNotebookForAProject('project-2'), 'notebook-2'); }); - test('should handle notebook switching within same project', () => { + test('should handle notebook switching within same project via storeOriginalProject', () => { manager.storeOriginalProject('project-123', mockProject, 'notebook-1'); - manager.selectNotebookForProject('project-123', 'notebook-1'); - - manager.updateCurrentNotebookId('project-123', 'notebook-2'); - manager.selectNotebookForProject('project-123', 'notebook-2'); + manager.storeOriginalProject('project-123', mockProject, 'notebook-2'); assert.strictEqual(manager.getCurrentNotebookId('project-123'), 'notebook-2'); - assert.strictEqual(manager.getTheSelectedNotebookForAProject('project-123'), 'notebook-2'); - }); - - test('should maintain separation between current and selected notebook IDs', () => { - // Store original project sets current notebook - manager.storeOriginalProject('project-123', mockProject, 'notebook-original'); - - // Selecting a different notebook for the project - manager.selectNotebookForProject('project-123', 'notebook-selected'); - - // Both should be maintained independently - assert.strictEqual(manager.getCurrentNotebookId('project-123'), 'notebook-original'); - assert.strictEqual(manager.getTheSelectedNotebookForAProject('project-123'), 'notebook-selected'); }); }); }); diff --git a/src/notebooks/deepnote/deepnoteSerializer.ts b/src/notebooks/deepnote/deepnoteSerializer.ts index 17443e93b9..531c645957 100644 --- a/src/notebooks/deepnote/deepnoteSerializer.ts +++ b/src/notebooks/deepnote/deepnoteSerializer.ts @@ -1,14 +1,25 @@ import type { DeepnoteBlock, DeepnoteFile, DeepnoteSnapshot } from '@deepnote/blocks'; import { deserializeDeepnoteFile, isExecutableBlock, serializeDeepnoteSnapshot } from '@deepnote/blocks'; import { inject, injectable, optional } from 'inversify'; -import { l10n, window, workspace, type CancellationToken, type NotebookData, type NotebookSerializer } from 'vscode'; +import { + CancellationTokenSource, + l10n, + NotebookEdit, + NotebookRange, + workspace, + WorkspaceEdit, + type CancellationToken, + type NotebookData, + type NotebookDocument, + type NotebookSerializer +} from 'vscode'; +import { computeHash } from '../../platform/common/crypto'; +import type { DeepnoteNotebook } from '../../platform/deepnote/deepnoteTypes'; import { logger } from '../../platform/logging'; import { IDeepnoteNotebookManager } from '../types'; import { DeepnoteDataConverter } from './deepnoteDataConverter'; -import type { DeepnoteNotebook } from '../../platform/deepnote/deepnoteTypes'; import { SnapshotService } from './snapshots/snapshotService'; -import { computeHash } from '../../platform/common/crypto'; export type { DeepnoteBlock, DeepnoteFile } from '@deepnote/blocks'; export { DeepnoteNotebook, DeepnoteOutput } from '../../platform/deepnote/deepnoteTypes'; @@ -46,6 +57,8 @@ function cloneWithoutCircularRefs(obj: T, seen = new WeakSet()): T { } } +const LAST_SERIALIZED_TTL_MS = 10_000; + /** * Serializer for converting between Deepnote YAML files and VS Code notebook format. * Handles reading/writing .deepnote files and manages project state persistence. @@ -53,6 +66,8 @@ function cloneWithoutCircularRefs(obj: T, seen = new WeakSet()): T { @injectable() export class DeepnoteNotebookSerializer implements NotebookSerializer { private converter = new DeepnoteDataConverter(); + private lastSerializedNotebookId: string | undefined; + private lastSerializedTimestamp = 0; constructor( @inject(IDeepnoteNotebookManager) private readonly notebookManager: IDeepnoteNotebookManager, @@ -62,25 +77,23 @@ export class DeepnoteNotebookSerializer implements NotebookSerializer { /** * Deserializes a Deepnote YAML file into VS Code notebook format. * Parses YAML and converts the selected notebook's blocks to cells. - * The notebook to deserialize must be pre-selected and stored in the manager. + * Notebook resolution prefers an explicit notebook ID, then transient + * resolver state, and finally a deterministic default notebook. * @param content Raw file content as bytes * @param token Cancellation token (unused) * @returns Promise resolving to notebook data */ - async deserializeNotebook(content: Uint8Array, token: CancellationToken): Promise { + async deserializeNotebook( + content: Uint8Array, + token: CancellationToken, + notebookId?: string + ): Promise { logger.debug('DeepnoteSerializer: Deserializing Deepnote notebook'); if (token?.isCancellationRequested) { throw new Error('Serialization cancelled'); } - // Initialize vega-lite for output conversion (lazy-loaded ESM module) - await this.converter.initialize(); - - if (token?.isCancellationRequested) { - throw new Error('Serialization cancelled'); - } - try { const contentString = new TextDecoder('utf-8').decode(content); const deepnoteFile = deserializeDeepnoteFile(contentString); @@ -90,29 +103,49 @@ export class DeepnoteNotebookSerializer implements NotebookSerializer { } const projectId = deepnoteFile.project.id; - const notebookId = this.findCurrentNotebookId(projectId); + const resolvedNotebookId = notebookId ?? this.findCurrentNotebookId(projectId); - logger.debug(`DeepnoteSerializer: Project ID: ${projectId}, Selected notebook ID: ${notebookId}`); + logger.debug(`DeepnoteSerializer: Project ID: ${projectId}, Selected notebook ID: ${resolvedNotebookId}`); + + if (!resolvedNotebookId) { + logger.debug( + 'DeepnoteSerializer: No notebook ID resolved, returning empty state for post-open verification' + ); + + return { + cells: [], + metadata: { + deepnoteProjectId: projectId, + deepnoteProjectName: deepnoteFile.project.name, + deepnoteVersion: deepnoteFile.version + } + }; + } if (deepnoteFile.project.notebooks.length === 0) { throw new Error('Deepnote project contains no notebooks.'); } - const selectedNotebook = notebookId - ? deepnoteFile.project.notebooks.find((nb) => nb.id === notebookId) - : this.findDefaultNotebook(deepnoteFile); + const selectedNotebook = deepnoteFile.project.notebooks.find((nb) => nb.id === resolvedNotebookId); if (!selectedNotebook) { throw new Error(l10n.t('No notebook selected or found')); } + // Initialize vega-lite for output conversion (lazy-loaded ESM module) + await this.converter.initialize(); + + if (token?.isCancellationRequested) { + throw new Error('Serialization cancelled'); + } + // Log block IDs from source file - for (let i = 0; i < (selectedNotebook.blocks ?? []).length; i++) { - const block = selectedNotebook.blocks![i]; + for (let i = 0; i < selectedNotebook.blocks.length; i++) { + const block = selectedNotebook.blocks[i]; logger.trace(`DeserializeNotebook: block[${i}] id=${block.id} from source file`); } - let cells = this.converter.convertBlocksToCells(selectedNotebook.blocks ?? []); + let cells = this.converter.convertBlocksToCells(selectedNotebook.blocks); logger.debug(`DeepnoteSerializer: Converted ${cells.length} cells from notebook blocks`); @@ -182,36 +215,24 @@ export class DeepnoteNotebookSerializer implements NotebookSerializer { } /** - * Finds the notebook ID to deserialize by checking the manager's stored selection. - * The notebook ID should be set via selectNotebookForProject before opening the document. - * @param projectId The project ID to find a notebook for - * @returns The notebook ID to deserialize, or undefined if none found + * Resolves the notebook ID for a deserialization call. + * + * Priority: + * 1. Pending resolution hint (explicit intent from explorer view) + * 2. Already-open document metadata (re-deserialization after file change) + * — skips the notebook that was just serialized, since that's the one + * that triggered the file change, not the one being re-deserialized. + * 3. undefined — initial open; verifyDeserializedNotebook will resolve + * from the document URI after open. */ findCurrentNotebookId(projectId: string): string | undefined { - // Prefer the active notebook editor when it matches the project - const activeEditorNotebook = window.activeNotebookEditor?.notebook; - - if ( - activeEditorNotebook?.notebookType === 'deepnote' && - activeEditorNotebook.metadata?.deepnoteProjectId === projectId && - activeEditorNotebook.metadata?.deepnoteNotebookId - ) { - return activeEditorNotebook.metadata.deepnoteNotebookId; - } + const pendingNotebookId = this.notebookManager.consumePendingNotebookResolution(projectId); - // Check the manager's stored selection - this should be set when opening from explorer - const storedNotebookId = this.notebookManager.getTheSelectedNotebookForAProject(projectId); - - if (storedNotebookId) { - return storedNotebookId; + if (pendingNotebookId) { + return pendingNotebookId; } - // Fallback: Check if there's an active notebook document for this project - const openNotebook = workspace.notebookDocuments.find( - (doc) => doc.notebookType === 'deepnote' && doc.metadata?.deepnoteProjectId === projectId - ); - - return openNotebook?.metadata?.deepnoteNotebookId; + return this.findNotebookIdFromOpenDocuments(projectId); } /** @@ -222,6 +243,81 @@ export class DeepnoteNotebookSerializer implements NotebookSerializer { return this.converter; } + /** + * Parses the file content and returns the ID of the default notebook + * (alphabetically first, excluding Init when other notebooks exist). + * Used by the post-open verification handler for direct file opens + * that have no `?notebook=` query param. + */ + resolveDefaultNotebookId(content: Uint8Array): string | undefined { + try { + const contentString = new TextDecoder('utf-8').decode(content); + const deepnoteFile = deserializeDeepnoteFile(contentString); + + return this.findDefaultNotebook(deepnoteFile)?.id; + } catch { + return undefined; + } + } + + /** + * Ensures an opened notebook document shows the correct notebook. + * + * The serializer returns an empty state when it cannot determine which + * notebook to display (no pending resolution). This method is called + * after the document is created, reads the real notebook ID from + * the URI `?notebook=` query param (or picks the default notebook + * for direct file opens), and patches the document in place. + */ + async verifyDeserializedNotebook(doc: NotebookDocument): Promise { + if (doc.notebookType !== 'deepnote') { + return; + } + + const expectedNotebookId = new URLSearchParams(doc.uri.query).get('notebook'); + const actualNotebookId = doc.metadata?.deepnoteNotebookId as string | undefined; + + if (actualNotebookId && (!expectedNotebookId || expectedNotebookId === actualNotebookId)) { + return; + } + + const cts = new CancellationTokenSource(); + + try { + const fileUri = doc.uri.with({ query: '', fragment: '' }); + const content = await workspace.fs.readFile(fileUri); + + const targetNotebookId = expectedNotebookId ?? this.resolveDefaultNotebookId(content); + + if (!targetNotebookId || targetNotebookId === actualNotebookId) { + return; + } + + logger.info( + `Notebook verification: resolving notebook ${targetNotebookId} (was ${actualNotebookId ?? 'empty'}).` + ); + + const correctData = await this.deserializeNotebook(content, cts.token, targetNotebookId); + + const edit = new WorkspaceEdit(); + + edit.set(doc.uri, [ + NotebookEdit.replaceCells(new NotebookRange(0, doc.cellCount), correctData.cells), + NotebookEdit.updateNotebookMetadata(correctData.metadata ?? {}) + ]); + + const applied = await workspace.applyEdit(edit); + + if (applied) { + await doc.save(); + } + } catch (error) { + logger.error('Failed to verify/correct notebook content', error); + } finally { + cts.dispose(); + } + } + /** * Serializes VS Code notebook data back to Deepnote YAML format. * Converts cells to blocks, updates project data, and generates YAML. @@ -259,12 +355,15 @@ export class DeepnoteNotebookSerializer implements NotebookSerializer { logger.debug('SerializeNotebook: Got and cloned original project'); const notebookId = - data.metadata?.deepnoteNotebookId || this.notebookManager.getTheSelectedNotebookForAProject(projectId); + data.metadata?.deepnoteNotebookId || this.notebookManager.getCurrentNotebookId(projectId); if (!notebookId) { throw new Error('Cannot determine which notebook to save'); } + this.lastSerializedNotebookId = notebookId; + this.lastSerializedTimestamp = Date.now(); + logger.debug(`SerializeNotebook: Notebook ID: ${notebookId}`); const notebook = originalProject.project.notebooks.find((nb: { id: string }) => nb.id === notebookId); @@ -346,8 +445,12 @@ export class DeepnoteNotebookSerializer implements NotebookSerializer { originalProject.metadata.modifiedAt = new Date().toISOString(); } - // Store the updated project back so subsequent saves start from correct state - this.notebookManager.storeOriginalProject(projectId, originalProject, notebookId); + // Store the updated project back so subsequent saves start from correct state. + // Use updateOriginalProject (not storeOriginalProject) to avoid overwriting + // currentNotebookId — when multiple notebooks share the same file, changing + // currentNotebookId here would cause VS Code's follow-up deserialize calls + // for other open notebooks to resolve to the wrong notebook. + this.notebookManager.updateOriginalProject(projectId, originalProject); logger.debug('SerializeNotebook: Serializing to YAML'); @@ -550,6 +653,42 @@ export class DeepnoteNotebookSerializer implements NotebookSerializer { return false; } + /** + * Returns a notebook ID from an already-open document for re-deserialization. + * + * When VS Code re-reads a file (e.g. after save or external change), it + * calls deserializeNotebook again for each open document backed by that + * file. The document already exists in workspace.notebookDocuments with + * the correct deepnoteNotebookId in its metadata. + * + * Skips the notebook that was most recently serialized — that document + * triggered the file change and is not the one being re-deserialized. + */ + private findNotebookIdFromOpenDocuments(projectId: string): string | undefined { + const recentSerialize = + this.lastSerializedNotebookId && Date.now() - this.lastSerializedTimestamp < LAST_SERIALIZED_TTL_MS; + + for (const doc of workspace.notebookDocuments) { + if (doc.notebookType !== 'deepnote' || doc.metadata?.deepnoteProjectId !== projectId) { + continue; + } + + const notebookId = doc.metadata?.deepnoteNotebookId as string | undefined; + + if (!notebookId) { + continue; + } + + if (recentSerialize && notebookId === this.lastSerializedNotebookId) { + continue; + } + + return notebookId; + } + + return undefined; + } + /** * Finds the default notebook to open when no selection is made. * @param file diff --git a/src/notebooks/deepnote/deepnoteSerializer.unit.test.ts b/src/notebooks/deepnote/deepnoteSerializer.unit.test.ts index c3332f974d..02b6c369ce 100644 --- a/src/notebooks/deepnote/deepnoteSerializer.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteSerializer.unit.test.ts @@ -74,9 +74,8 @@ suite('DeepnoteNotebookSerializer', () => { } suite('deserializeNotebook', () => { - test('should deserialize valid project with selected notebook', async () => { - // Set up the manager to select the first notebook - manager.selectNotebookForProject('project-123', 'notebook-1'); + test('should deserialize valid project with queued notebook resolution', async () => { + manager.queueNotebookResolution('project-123', 'notebook-1'); const yamlContent = ` version: '1.0.0' @@ -130,7 +129,7 @@ project: ); }); - test('should throw error when no notebooks found', async () => { + test('should return empty state when no notebooks found and no ID resolved', async () => { const contentWithoutNotebooks = new TextEncoder().encode(` version: '1.0.0' metadata: @@ -142,10 +141,39 @@ project: settings: {} `); - await assert.isRejected( - serializer.deserializeNotebook(contentWithoutNotebooks, {} as any), - /no notebooks|notebooks.*must contain at least 1/i - ); + const result = await serializer.deserializeNotebook(contentWithoutNotebooks, {} as any); + + assert.deepStrictEqual(result.cells, []); + assert.strictEqual(result.metadata?.deepnoteProjectId, 'project-123'); + assert.strictEqual(result.metadata?.deepnoteNotebookId, undefined); + }); + + test('should deserialize the specified notebook when notebookId is passed', async () => { + const content = projectToYaml(mockProject); + const result = await serializer.deserializeNotebook(content, {} as any, 'notebook-2'); + + assert.strictEqual(result.metadata?.deepnoteNotebookId, 'notebook-2'); + assert.strictEqual(result.metadata?.deepnoteNotebookName, 'Second Notebook'); + assert.strictEqual(result.cells.length, 1); + assert.include(result.cells[0].value, 'Title'); + }); + + test('should ignore queued resolution when explicit notebookId is provided', async () => { + manager.queueNotebookResolution('project-123', 'notebook-1'); + const content = projectToYaml(mockProject); + const result = await serializer.deserializeNotebook(content, {} as any, 'notebook-2'); + + assert.strictEqual(result.metadata?.deepnoteNotebookId, 'notebook-2'); + assert.strictEqual(result.metadata?.deepnoteNotebookName, 'Second Notebook'); + }); + + test('should fall back to findCurrentNotebookId when notebookId is undefined', async () => { + manager.queueNotebookResolution('project-123', 'notebook-1'); + const content = projectToYaml(mockProject); + const result = await serializer.deserializeNotebook(content, {} as any); + + assert.strictEqual(result.metadata?.deepnoteNotebookId, 'notebook-1'); + assert.strictEqual(result.metadata?.deepnoteNotebookName, 'First Notebook'); }); }); @@ -205,6 +233,65 @@ project: assert.include(yamlString, 'project-123'); assert.include(yamlString, 'notebook-1'); }); + + test('should use current notebook ID when metadata notebook ID is missing', async () => { + manager.storeOriginalProject('project-123', mockProject, 'notebook-2'); + + const mockNotebookData = { + cells: [ + { + kind: 1, // NotebookCellKind.Markup + value: '# Updated second notebook', + languageId: 'markdown', + metadata: {} + } + ], + metadata: { + deepnoteProjectId: 'project-123' + } + }; + + const result = await serializer.serializeNotebook(mockNotebookData as any, {} as any); + const serializedProject = deserializeDeepnoteFile(new TextDecoder().decode(result)); + + assert.strictEqual(serializedProject.project.notebooks[0].id, 'notebook-1'); + assert.strictEqual(serializedProject.project.notebooks[1].id, 'notebook-2'); + assert.strictEqual(serializedProject.project.notebooks[0].blocks?.[0].content, 'print("hello")'); + assert.strictEqual(serializedProject.project.notebooks[1].blocks?.[0].content, '# Updated second notebook'); + }); + + suite('multi-notebook save scenarios', () => { + teardown(() => { + when(mockedVSCodeNamespaces.window.activeNotebookEditor).thenReturn(undefined); + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([]); + }); + + test('serializing notebook-1 should not change currentNotebookId', async () => { + manager.queueNotebookResolution('project-123', 'notebook-2'); + await serializer.deserializeNotebook(projectToYaml(mockProject), {} as any); + + assert.strictEqual(manager.getCurrentNotebookId('project-123'), 'notebook-2'); + + const mockNotebookData = { + cells: [ + { + kind: 2, + languageId: 'python', + metadata: {}, + value: 'print("edited nb1")' + } + ], + metadata: { + deepnoteNotebookId: 'notebook-1', + deepnoteProjectId: 'project-123' + } + }; + + await serializer.serializeNotebook(mockNotebookData as any, {} as any); + + assert.strictEqual(manager.getCurrentNotebookId('project-123'), 'notebook-2'); + }); + }); }); suite('findCurrentNotebookId', () => { @@ -212,24 +299,34 @@ project: // Reset only the specific mocks used in this suite when(mockedVSCodeNamespaces.window.activeNotebookEditor).thenReturn(undefined); when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([]); + when(mockedVSCodeNamespaces.window.tabGroups).thenReturn({ all: [] } as any); }); - test('should return stored notebook ID when available', () => { - manager.selectNotebookForProject('project-123', 'notebook-456'); + test('should return queued notebook resolution when available', () => { + manager.queueNotebookResolution('project-123', 'queued-notebook'); const result = serializer.findCurrentNotebookId('project-123'); - assert.strictEqual(result, 'notebook-456'); + assert.strictEqual(result, 'queued-notebook'); }); - test('should fall back to active notebook document when no stored selection', () => { - // Create a mock notebook document + test('should consume queued notebook resolution only once', () => { + manager.queueNotebookResolution('project-123', 'queued-notebook'); + + assert.strictEqual(serializer.findCurrentNotebookId('project-123'), 'queued-notebook'); + assert.strictEqual(serializer.findCurrentNotebookId('project-123'), undefined); + }); + + test('should prioritize queued notebook resolution over current notebook and open documents', () => { + manager.queueNotebookResolution('project-123', 'queued-notebook'); + manager.storeOriginalProject('project-123', mockProject, 'current-notebook'); + const mockNotebookDoc = { - then: undefined, // Prevent mock from being treated as a Promise-like thenable + then: undefined, notebookType: 'deepnote', metadata: { deepnoteProjectId: 'project-123', - deepnoteNotebookId: 'notebook-from-workspace' + deepnoteNotebookId: 'open-notebook' }, uri: {} as any, version: 1, @@ -242,126 +339,25 @@ project: save: async () => true } as NotebookDocument; - // Configure the mocked workspace.notebookDocuments (same pattern as other tests) when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([mockNotebookDoc]); const result = serializer.findCurrentNotebookId('project-123'); - assert.strictEqual(result, 'notebook-from-workspace'); - }); - - test('should return undefined for unknown project', () => { - const result = serializer.findCurrentNotebookId('unknown-project'); - - assert.strictEqual(result, undefined); - }); - - test('should prioritize stored selection over fallback', () => { - manager.selectNotebookForProject('project-123', 'stored-notebook'); - - const result = serializer.findCurrentNotebookId('project-123'); - - assert.strictEqual(result, 'stored-notebook'); - }); - - test('should handle multiple projects independently', () => { - manager.selectNotebookForProject('project-1', 'notebook-1'); - manager.selectNotebookForProject('project-2', 'notebook-2'); - - const result1 = serializer.findCurrentNotebookId('project-1'); - const result2 = serializer.findCurrentNotebookId('project-2'); - - assert.strictEqual(result1, 'notebook-1'); - assert.strictEqual(result2, 'notebook-2'); - }); - - test('should prioritize active notebook editor over stored selection', () => { - // Store a selection for the project - manager.selectNotebookForProject('project-123', 'stored-notebook'); - - // Mock the active notebook editor to return a different notebook - const mockActiveNotebook = { - notebookType: 'deepnote', - metadata: { - deepnoteProjectId: 'project-123', - deepnoteNotebookId: 'active-editor-notebook' - } - }; - - when(mockedVSCodeNamespaces.window.activeNotebookEditor).thenReturn({ - notebook: mockActiveNotebook - } as any); - - const result = serializer.findCurrentNotebookId('project-123'); - - // Should return the active editor's notebook, not the stored one - assert.strictEqual(result, 'active-editor-notebook'); + assert.strictEqual(result, 'queued-notebook'); }); - test('should ignore active editor when project ID does not match', () => { - manager.selectNotebookForProject('project-123', 'stored-notebook'); - - // Mock active editor with a different project - const mockActiveNotebook = { - notebookType: 'deepnote', - metadata: { - deepnoteProjectId: 'different-project', - deepnoteNotebookId: 'active-editor-notebook' - } - }; - - when(mockedVSCodeNamespaces.window.activeNotebookEditor).thenReturn({ - notebook: mockActiveNotebook - } as any); + test('should return undefined when no pending resolution or active tab exists', () => { + manager.storeOriginalProject('project-123', mockProject, 'current-notebook'); const result = serializer.findCurrentNotebookId('project-123'); - // Should fall back to stored selection since active editor is for different project - assert.strictEqual(result, 'stored-notebook'); - }); - - test('should ignore active editor when notebook type is not deepnote', () => { - manager.selectNotebookForProject('project-123', 'stored-notebook'); - - // Mock active editor with non-deepnote notebook type - const mockActiveNotebook = { - notebookType: 'jupyter-notebook', - metadata: { - deepnoteProjectId: 'project-123', - deepnoteNotebookId: 'active-editor-notebook' - } - }; - - when(mockedVSCodeNamespaces.window.activeNotebookEditor).thenReturn({ - notebook: mockActiveNotebook - } as any); - - const result = serializer.findCurrentNotebookId('project-123'); - - // Should fall back to stored selection since active editor is not a deepnote notebook - assert.strictEqual(result, 'stored-notebook'); + assert.strictEqual(result, undefined); }); - test('should ignore active editor when notebook ID is missing', () => { - manager.selectNotebookForProject('project-123', 'stored-notebook'); - - // Mock active editor without notebook ID in metadata - const mockActiveNotebook = { - notebookType: 'deepnote', - metadata: { - deepnoteProjectId: 'project-123' - // Missing deepnoteNotebookId - } - }; - - when(mockedVSCodeNamespaces.window.activeNotebookEditor).thenReturn({ - notebook: mockActiveNotebook - } as any); - - const result = serializer.findCurrentNotebookId('project-123'); + test('should return undefined for unknown project', () => { + const result = serializer.findCurrentNotebookId('unknown-project'); - // Should fall back to stored selection since active editor has no notebook ID - assert.strictEqual(result, 'stored-notebook'); + assert.strictEqual(result, undefined); }); }); @@ -388,13 +384,10 @@ project: }); test('should handle manager state operations', () => { + assert.isFunction(manager.consumePendingNotebookResolution, 'has consumePendingNotebookResolution method'); assert.isFunction(manager.getCurrentNotebookId, 'has getCurrentNotebookId method'); assert.isFunction(manager.getOriginalProject, 'has getOriginalProject method'); - assert.isFunction( - manager.getTheSelectedNotebookForAProject, - 'has getTheSelectedNotebookForAProject method' - ); - assert.isFunction(manager.selectNotebookForProject, 'has selectNotebookForProject method'); + assert.isFunction(manager.queueNotebookResolution, 'has queueNotebookResolution method'); assert.isFunction(manager.storeOriginalProject, 'has storeOriginalProject method'); }); @@ -758,7 +751,7 @@ project: }); suite('default notebook selection', () => { - test('should not select Init notebook when other notebooks are available', async () => { + test('should not select Init notebook when other notebooks are available', () => { const projectData: DeepnoteFile = { version: '1.0.0', metadata: { @@ -808,14 +801,12 @@ project: }; const content = projectToYaml(projectData); - const result = await serializer.deserializeNotebook(content, {} as any); + const result = serializer.resolveDefaultNotebookId(content); - // Should select the Main notebook, not the Init notebook - assert.strictEqual(result.metadata?.deepnoteNotebookId, 'main-notebook'); - assert.strictEqual(result.metadata?.deepnoteNotebookName, 'Main'); + assert.strictEqual(result, 'main-notebook'); }); - test('should select Init notebook when it is the only notebook', async () => { + test('should select Init notebook when it is the only notebook', () => { const projectData: DeepnoteFile = { version: '1.0.0', metadata: { @@ -849,14 +840,12 @@ project: }; const content = projectToYaml(projectData); - const result = await serializer.deserializeNotebook(content, {} as any); + const result = serializer.resolveDefaultNotebookId(content); - // Should select the Init notebook since it's the only one - assert.strictEqual(result.metadata?.deepnoteNotebookId, 'init-notebook'); - assert.strictEqual(result.metadata?.deepnoteNotebookName, 'Init'); + assert.strictEqual(result, 'init-notebook'); }); - test('should select alphabetically first notebook when no initNotebookId', async () => { + test('should select alphabetically first notebook when no initNotebookId', () => { const projectData: DeepnoteFile = { version: '1.0.0', metadata: { @@ -921,14 +910,12 @@ project: }; const content = projectToYaml(projectData); - const result = await serializer.deserializeNotebook(content, {} as any); + const result = serializer.resolveDefaultNotebookId(content); - // Should select the alphabetically first notebook - assert.strictEqual(result.metadata?.deepnoteNotebookId, 'alpha-notebook'); - assert.strictEqual(result.metadata?.deepnoteNotebookName, 'Alpha Notebook'); + assert.strictEqual(result, 'alpha-notebook'); }); - test('should sort Init notebook last when multiple notebooks exist', async () => { + test('should sort Init notebook last when multiple notebooks exist', () => { const projectData: DeepnoteFile = { version: '1.0.0', metadata: { @@ -994,11 +981,19 @@ project: }; const content = projectToYaml(projectData); - const result = await serializer.deserializeNotebook(content, {} as any); + const result = serializer.resolveDefaultNotebookId(content); // Should select Alpha, not Init even though "Init" comes before "Alpha" alphabetically when in upper case - assert.strictEqual(result.metadata?.deepnoteNotebookId, 'alpha-notebook'); - assert.strictEqual(result.metadata?.deepnoteNotebookName, 'Alpha'); + assert.strictEqual(result, 'alpha-notebook'); + }); + + test('should return empty state from deserializeNotebook when no notebook ID is resolved', async () => { + const content = projectToYaml(mockProject); + const result = await serializer.deserializeNotebook(content, {} as any); + + assert.deepStrictEqual(result.cells, []); + assert.strictEqual(result.metadata?.deepnoteProjectId, 'project-123'); + assert.strictEqual(result.metadata?.deepnoteNotebookId, undefined); }); }); diff --git a/src/notebooks/serviceRegistry.node.ts b/src/notebooks/serviceRegistry.node.ts index cbd8b860fe..f4ea82b469 100644 --- a/src/notebooks/serviceRegistry.node.ts +++ b/src/notebooks/serviceRegistry.node.ts @@ -41,6 +41,7 @@ import { NotebookTracebackFormatter } from './outputs/tracebackFormatter'; import { InterpreterPackageTracker } from './telemetry/interpreterPackageTracker.node'; import { INotebookEditorProvider, INotebookPythonEnvironmentService } from './types'; import { DeepnoteActivationService } from './deepnote/deepnoteActivationService'; +import { DeepnoteNotebookInfoStatusBar } from './deepnote/deepnoteNotebookInfoStatusBar'; import { DeepnoteNotebookManager } from './deepnote/deepnoteNotebookManager'; import { IDeepnoteNotebookManager } from './types'; import { IntegrationStorage } from '../platform/notebooks/deepnote/integrationStorage'; @@ -169,6 +170,10 @@ export function registerTypes(serviceManager: IServiceManager, isDevMode: boolea IExtensionSyncActivationService, DeepnoteActivationService ); + serviceManager.addSingleton( + IExtensionSyncActivationService, + DeepnoteNotebookInfoStatusBar + ); serviceManager.addSingleton( IExtensionSyncActivationService, DeepnoteNotebookCommandListener diff --git a/src/notebooks/serviceRegistry.web.ts b/src/notebooks/serviceRegistry.web.ts index 2488ff73d7..51ea5e2b88 100644 --- a/src/notebooks/serviceRegistry.web.ts +++ b/src/notebooks/serviceRegistry.web.ts @@ -36,6 +36,7 @@ import { CellOutputMimeTypeTracker } from './outputs/cellOutputMimeTypeTracker'; import { NotebookTracebackFormatter } from './outputs/tracebackFormatter'; import { INotebookEditorProvider, INotebookPythonEnvironmentService } from './types'; import { DeepnoteActivationService } from './deepnote/deepnoteActivationService'; +import { DeepnoteNotebookInfoStatusBar } from './deepnote/deepnoteNotebookInfoStatusBar'; import { DeepnoteNotebookManager } from './deepnote/deepnoteNotebookManager'; import { IDeepnoteNotebookManager } from './types'; import { DeepnoteNotebookCommandListener } from './deepnote/deepnoteNotebookCommandListener'; @@ -108,6 +109,10 @@ export function registerTypes(serviceManager: IServiceManager, isDevMode: boolea IExtensionSyncActivationService, DeepnoteActivationService ); + serviceManager.addSingleton( + IExtensionSyncActivationService, + DeepnoteNotebookInfoStatusBar + ); serviceManager.addSingleton( IExtensionSyncActivationService, DeepnoteNotebookCommandListener diff --git a/src/notebooks/types.ts b/src/notebooks/types.ts index a12dacf52a..2da356ee25 100644 --- a/src/notebooks/types.ts +++ b/src/notebooks/types.ts @@ -37,12 +37,14 @@ export interface ProjectIntegration { export const IDeepnoteNotebookManager = Symbol('IDeepnoteNotebookManager'); export interface IDeepnoteNotebookManager { + consumePendingNotebookResolution(projectId: string): string | undefined; getCurrentNotebookId(projectId: string): string | undefined; getOriginalProject(projectId: string): DeepnoteProject | undefined; - getTheSelectedNotebookForAProject(projectId: string): string | undefined; - selectNotebookForProject(projectId: string, notebookId: string): void; + hasInitNotebookBeenRun(projectId: string): boolean; + markInitNotebookAsRun(projectId: string): void; + queueNotebookResolution(projectId: string, notebookId: string): void; storeOriginalProject(projectId: string, project: DeepnoteProject, notebookId: string): void; - updateCurrentNotebookId(projectId: string, notebookId: string): void; + updateOriginalProject(projectId: string, project: DeepnoteProject): void; /** * Updates the integrations list in the project data. @@ -53,7 +55,4 @@ export interface IDeepnoteNotebookManager { * @returns `true` if the project was found and updated successfully, `false` if the project does not exist */ updateProjectIntegrations(projectId: string, integrations: ProjectIntegration[]): boolean; - - hasInitNotebookBeenRun(projectId: string): boolean; - markInitNotebookAsRun(projectId: string): void; } diff --git a/src/platform/common/constants.ts b/src/platform/common/constants.ts index c85719faec..d60321f1cf 100644 --- a/src/platform/common/constants.ts +++ b/src/platform/common/constants.ts @@ -223,6 +223,7 @@ export namespace Commands { export const OpenDeepnoteNotebook = 'deepnote.openNotebook'; export const OpenDeepnoteFile = 'deepnote.openFile'; export const RevealInDeepnoteExplorer = 'deepnote.revealInExplorer'; + export const CopyNotebookDetails = 'deepnote.copyNotebookDetails'; export const EnableSnapshots = 'deepnote.enableSnapshots'; export const DisableSnapshots = 'deepnote.disableSnapshots'; export const ManageIntegrations = 'deepnote.manageIntegrations'; diff --git a/src/test/mocks/vsc/extHostedTypes.ts b/src/test/mocks/vsc/extHostedTypes.ts index 4a58f1c077..ab429319c4 100644 --- a/src/test/mocks/vsc/extHostedTypes.ts +++ b/src/test/mocks/vsc/extHostedTypes.ts @@ -2544,6 +2544,16 @@ export namespace vscMockExtHostedTypes { */ Default = 0 } + export class TabInputNotebook { + readonly uri: vscUri.URI; + readonly notebookType: string; + + constructor(uri: vscUri.URI, notebookType: string) { + this.uri = uri; + this.notebookType = notebookType; + } + } + export class NotebookCellData { kind: NotebookCellKind; value: string; diff --git a/src/test/vscode-mock.ts b/src/test/vscode-mock.ts index b6b0f5371f..d413837dc7 100644 --- a/src/test/vscode-mock.ts +++ b/src/test/vscode-mock.ts @@ -83,6 +83,7 @@ export function resetVSCodeMocks() { when(mockedVSCodeNamespaces.window.visibleNotebookEditors).thenReturn([]); when(mockedVSCodeNamespaces.window.activeTextEditor).thenReturn(undefined); when(mockedVSCodeNamespaces.window.activeNotebookEditor).thenReturn(undefined); + when(mockedVSCodeNamespaces.window.tabGroups).thenReturn({ all: [] } as any); // Window dialog methods with overloads (1-5 parameters) // showInformationMessage @@ -300,6 +301,7 @@ export function resetVSCodeMocks() { (mockedVSCode as any).NotebookCellExecutionState = vscodeMocks.vscMockExtHostedTypes.NotebookCellExecutionState; (mockedVSCode as any).NotebookEditorRevealType = vscodeMocks.vscMockExtHostedTypes.NotebookEditorRevealType; // Mock ColorThemeKind enum + (mockedVSCode as any).TabInputNotebook = vscodeMocks.vscMockExtHostedTypes.TabInputNotebook; (mockedVSCode as any).ColorThemeKind = { Light: 1, Dark: 2, HighContrast: 3, HighContrastLight: 4 }; mockedVSCode.EndOfLine = vscodeMocks.vscMockExtHostedTypes.EndOfLine; }