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..b81db6a7ba 100644 --- a/src/notebooks/deepnote/deepnoteActivationService.ts +++ b/src/notebooks/deepnote/deepnoteActivationService.ts @@ -1,14 +1,16 @@ +import { deserializeDeepnoteFile } from '@deepnote/blocks'; 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'; import { ILogger } from '../../platform/logging/types'; import { IDeepnoteNotebookManager } from '../types'; -import { DeepnoteNotebookSerializer } from './deepnoteSerializer'; +import { DeepnoteAutoSplitter } from './deepnoteAutoSplitter'; import { DeepnoteExplorerView } from './deepnoteExplorerView'; -import { IIntegrationManager } from './integrations/types'; import { DeepnoteInputBlockEditProtection } from './deepnoteInputBlockEditProtection'; +import { DeepnoteNotebookSerializer } from './deepnoteSerializer'; +import { IIntegrationManager } from './integrations/types'; import { SnapshotService } from './snapshots/snapshotService'; /** @@ -17,6 +19,8 @@ import { SnapshotService } from './snapshots/snapshotService'; */ @injectable() export class DeepnoteActivationService implements IExtensionSyncActivationService { + private autoSplitter: DeepnoteAutoSplitter; + private editProtection: DeepnoteInputBlockEditProtection; private explorerView: DeepnoteExplorerView; @@ -44,13 +48,19 @@ export class DeepnoteActivationService implements IExtensionSyncActivationServic * Called during extension activation to set up Deepnote integration. */ public activate() { + this.autoSplitter = new DeepnoteAutoSplitter(); this.serializer = new DeepnoteNotebookSerializer(this.notebookManager, this.snapshotService); - this.explorerView = new DeepnoteExplorerView(this.extensionContext, this.notebookManager, this.logger); + this.explorerView = new DeepnoteExplorerView(this.extensionContext, this.logger); this.editProtection = new DeepnoteInputBlockEditProtection(this.logger); this.snapshotsEnabled = this.isSnapshotsEnabled(); this.registerSerializer(); this.extensionContext.subscriptions.push(this.editProtection); + this.extensionContext.subscriptions.push( + workspace.onDidOpenNotebookDocument((doc) => { + void this.checkAndSplitIfNeeded(doc); + }) + ); this.extensionContext.subscriptions.push( workspace.onDidChangeConfiguration((event) => { if (event.affectsConfiguration('deepnote.snapshots.enabled')) { @@ -70,6 +80,25 @@ export class DeepnoteActivationService implements IExtensionSyncActivationServic this.integrationManager.activate(); } + private async checkAndSplitIfNeeded(doc: import('vscode').NotebookDocument): Promise { + if (doc.notebookType !== 'deepnote') { + return; + } + + try { + const fileUri = doc.uri; + const content = await workspace.fs.readFile(fileUri); + const deepnoteFile = deserializeDeepnoteFile(new TextDecoder().decode(content)); + const result = await this.autoSplitter.splitIfNeeded(fileUri, deepnoteFile); + + if (result.wasSplit) { + this.explorerView.refreshTree(); + } + } catch (error) { + this.logger.error('Failed to check/split notebook file', error); + } + } + private isSnapshotsEnabled(): boolean { if (this.snapshotService) { return this.snapshotService.isSnapshotsEnabled(); diff --git a/src/notebooks/deepnote/deepnoteAutoSplitter.ts b/src/notebooks/deepnote/deepnoteAutoSplitter.ts new file mode 100644 index 0000000000..31b88b6cd0 --- /dev/null +++ b/src/notebooks/deepnote/deepnoteAutoSplitter.ts @@ -0,0 +1,291 @@ +import { serializeDeepnoteFile, type DeepnoteFile } from '@deepnote/blocks'; +import { l10n, RelativePattern, Uri, window, workspace } from 'vscode'; + +import { computeHash } from '../../platform/common/crypto'; +import { logger } from '../../platform/logging'; +import { slugifyProjectName } from './snapshots/snapshotFiles'; + +/** + * Splits multi-notebook .deepnote files into separate files (one notebook per file). + * All split files share the same project ID and metadata. + */ +export class DeepnoteAutoSplitter { + /** + * Checks if a file has more than one non-init notebook and splits it if so. + * The first non-init notebook (by array order) stays in the original file. + * Each extra notebook gets its own file with the same project metadata. + * + * @returns Info about whether a split happened and which new files were created + */ + async splitIfNeeded(fileUri: Uri, deepnoteFile: DeepnoteFile): Promise<{ wasSplit: boolean; newFiles: Uri[] }> { + const initNotebookId = deepnoteFile.project.initNotebookId; + + const nonInitNotebooks = deepnoteFile.project.notebooks.filter((nb) => nb.id !== initNotebookId); + + if (nonInitNotebooks.length <= 1) { + return { wasSplit: false, newFiles: [] }; + } + + const initNotebook = initNotebookId + ? deepnoteFile.project.notebooks.find((nb) => nb.id === initNotebookId) + : undefined; + + // First non-init notebook stays in original file + const primaryNotebook = nonInitNotebooks[0]; + const extraNotebooks = nonInitNotebooks.slice(1); + + logger.info( + `[AutoSplitter] Splitting ${nonInitNotebooks.length} notebooks from project ${deepnoteFile.project.id}` + ); + + const parentDir = Uri.joinPath(fileUri, '..'); + const originalStem = this.getFileStem(fileUri); + const newFiles: Uri[] = []; + + // Create a new file for each extra notebook + for (const notebook of extraNotebooks) { + const notebookSlug = this.slugifyNotebookName(notebook.name); + const newFileName = `${originalStem}_${notebookSlug}.deepnote`; + const newFileUri = Uri.joinPath(parentDir, newFileName); + + const notebooks = initNotebook ? [structuredClone(initNotebook), notebook] : [notebook]; + + const newProject: DeepnoteFile = { + metadata: deepnoteFile.metadata + ? structuredClone(deepnoteFile.metadata) + : { createdAt: new Date().toISOString() }, + project: { + ...deepnoteFile.project, + notebooks + }, + version: deepnoteFile.version + }; + + if (initNotebook && initNotebookId) { + newProject.project.initNotebookId = initNotebookId; + } + + // Recompute snapshotHash for the new file + (newProject.metadata as Record).snapshotHash = await this.computeSnapshotHash(newProject); + + const yaml = serializeDeepnoteFile(newProject); + await workspace.fs.writeFile(newFileUri, new TextEncoder().encode(yaml)); + + newFiles.push(newFileUri); + + logger.info(`[AutoSplitter] Created ${newFileName} for notebook "${notebook.name}"`); + } + + // Update original file to keep only primary notebook (+ init) + const originalNotebooks = initNotebook ? [structuredClone(initNotebook), primaryNotebook] : [primaryNotebook]; + deepnoteFile.project.notebooks = originalNotebooks; + + if (deepnoteFile.metadata) { + (deepnoteFile.metadata as Record).snapshotHash = await this.computeSnapshotHash( + deepnoteFile + ); + } + + const updatedYaml = serializeDeepnoteFile(deepnoteFile); + await workspace.fs.writeFile(fileUri, new TextEncoder().encode(updatedYaml)); + + // Split snapshot files too + await this.splitSnapshots( + fileUri, + deepnoteFile.project.id, + deepnoteFile.project.name, + primaryNotebook.id, + extraNotebooks.map((nb) => nb.id) + ); + + // Notify the user + const fileNames = newFiles.map((f) => f.path.split('/').pop()).join(', '); + + void window.showInformationMessage( + l10n.t( + 'This project had {0} notebooks. They have been split into separate files: {1}', + nonInitNotebooks.length, + fileNames + ) + ); + + return { wasSplit: true, newFiles }; + } + + /** + * Splits existing snapshot files so each notebook gets its own snapshot. + * Old format: {slug}_{projectId}_{variant}.snapshot.deepnote + * New format: {slug}_{projectId}_{notebookId}_{variant}.snapshot.deepnote + */ + private async splitSnapshots( + _projectFileUri: Uri, + projectId: string, + projectName: string, + primaryNotebookId: string, + extraNotebookIds: string[] + ): Promise { + const workspaceFolders = workspace.workspaceFolders; + + if (!workspaceFolders || workspaceFolders.length === 0) { + return; + } + + const snapshotGlob = `**/snapshots/*_${projectId}_*.snapshot.deepnote`; + let allSnapshotFiles: Uri[] = []; + + for (const folder of workspaceFolders) { + const pattern = new RelativePattern(folder, snapshotGlob); + const files = await workspace.findFiles(pattern, null, 100); + allSnapshotFiles = allSnapshotFiles.concat(files); + } + + if (allSnapshotFiles.length === 0) { + logger.debug(`[AutoSplitter] No snapshots found for project ${projectId}`); + return; + } + + let slug: string; + + try { + slug = slugifyProjectName(projectName); + } catch { + logger.warn(`[AutoSplitter] Cannot slugify project name, skipping snapshot split`); + return; + } + + const allNotebookIds = [primaryNotebookId, ...extraNotebookIds]; + + for (const snapshotUri of allSnapshotFiles) { + try { + await this.splitSingleSnapshot(snapshotUri, slug, projectId, allNotebookIds); + } catch (error) { + logger.warn(`[AutoSplitter] Failed to split snapshot ${snapshotUri.path}`, error); + } + } + } + + private async splitSingleSnapshot( + snapshotUri: Uri, + slug: string, + projectId: string, + notebookIds: string[] + ): Promise { + const content = await workspace.fs.readFile(snapshotUri); + const { deserializeDeepnoteFile: parseFile } = await import('@deepnote/blocks'); + const snapshotData = parseFile(new TextDecoder().decode(content)); + + const snapshotDir = Uri.joinPath(snapshotUri, '..'); + + // Extract variant from existing filename + const basename = snapshotUri.path.split('/').pop() ?? ''; + const variant = this.extractVariantFromSnapshotFilename(basename, projectId); + + if (!variant) { + logger.debug(`[AutoSplitter] Could not extract variant from ${basename}, skipping`); + return; + } + + // Create per-notebook snapshot files + for (const notebookId of notebookIds) { + const notebookData = structuredClone(snapshotData); + + // Keep only this notebook (and init) + notebookData.project.notebooks = notebookData.project.notebooks.filter( + (nb) => + nb.id === notebookId || + (notebookData.project.initNotebookId && nb.id === notebookData.project.initNotebookId) + ); + + // Recompute hash + if (notebookData.metadata) { + (notebookData.metadata as Record).snapshotHash = await this.computeSnapshotHash( + notebookData + ); + } + + const newFilename = `${slug}_${projectId}_${notebookId}_${variant}.snapshot.deepnote`; + const newUri = Uri.joinPath(snapshotDir, newFilename); + const yaml = serializeDeepnoteFile(notebookData); + + await workspace.fs.writeFile(newUri, new TextEncoder().encode(yaml)); + + logger.debug(`[AutoSplitter] Created notebook snapshot: ${newFilename}`); + } + + // Delete the old snapshot file (it's been replaced by per-notebook files) + try { + await workspace.fs.delete(snapshotUri); + logger.debug(`[AutoSplitter] Deleted old snapshot: ${basename}`); + } catch { + logger.warn(`[AutoSplitter] Failed to delete old snapshot: ${basename}`); + } + } + + /** + * Extracts the variant portion from a snapshot filename. + * Old format: {slug}_{projectId}_{variant}.snapshot.deepnote + */ + private extractVariantFromSnapshotFilename(basename: string, projectId: string): string | undefined { + const suffix = '.snapshot.deepnote'; + + if (!basename.endsWith(suffix)) { + return undefined; + } + + const withoutSuffix = basename.slice(0, -suffix.length); + const projectIdIndex = withoutSuffix.indexOf(`_${projectId}_`); + + if (projectIdIndex === -1) { + return undefined; + } + + return withoutSuffix.slice(projectIdIndex + 1 + projectId.length + 1); + } + + /** + * Computes snapshotHash using the same algorithm as DeepnoteNotebookSerializer. + */ + private async computeSnapshotHash(project: DeepnoteFile): Promise { + const contentHashes: string[] = []; + + for (const notebook of project.project.notebooks) { + for (const block of notebook.blocks ?? []) { + if (block.contentHash) { + contentHashes.push(block.contentHash); + } + } + } + + contentHashes.sort(); + + const hashInput = { + contentHashes, + environmentHash: project.environment?.hash ?? null, + integrations: (project.project.integrations ?? []) + .map((i: { id: string; name: string; type: string }) => ({ id: i.id, name: i.name, type: i.type })) + .sort((a: { id: string }, b: { id: string }) => a.id.localeCompare(b.id)), + version: project.version + }; + + const hashData = JSON.stringify(hashInput); + const hash = await computeHash(hashData, 'SHA-256'); + + return `sha256:${hash}`; + } + + private getFileStem(uri: Uri): string { + const basename = uri.path.split('/').pop() ?? ''; + const dotIndex = basename.indexOf('.'); + + return dotIndex > 0 ? basename.slice(0, dotIndex) : basename; + } + + private slugifyNotebookName(name: string): string { + try { + return slugifyProjectName(name); + } catch { + // Fallback for names that produce empty slugs + return 'notebook'; + } + } +} diff --git a/src/notebooks/deepnote/deepnoteExplorerView.ts b/src/notebooks/deepnote/deepnoteExplorerView.ts index 33599da2f0..a8fc8173a5 100644 --- a/src/notebooks/deepnote/deepnoteExplorerView.ts +++ b/src/notebooks/deepnote/deepnoteExplorerView.ts @@ -4,7 +4,7 @@ import { serializeDeepnoteFile, type DeepnoteBlock, type DeepnoteFile } from '@d import { convertDeepnoteToJupyterNotebooks, convertIpynbFilesToDeepnoteFile } from '@deepnote/convert'; import { IExtensionContext } from '../../platform/common/types'; -import { IDeepnoteNotebookManager } from '../types'; + import { DeepnoteTreeDataProvider } from './deepnoteTreeDataProvider'; import { type DeepnoteTreeItem, DeepnoteTreeItemType, type DeepnoteTreeItemContext } from './deepnoteTreeItem'; import { uuidUtils } from '../../platform/common/uuid'; @@ -24,7 +24,6 @@ export class DeepnoteExplorerView { constructor( @inject(IExtensionContext) private readonly extensionContext: IExtensionContext, - @inject(IDeepnoteNotebookManager) private readonly manager: IDeepnoteNotebookManager, @inject(ILogger) logger: ILogger ) { this.treeDataProvider = new DeepnoteTreeDataProvider(logger); @@ -77,7 +76,7 @@ export class DeepnoteExplorerView { projectData.project.notebooks.push(newNotebook); // Save and open the new notebook - await this.saveProjectAndOpenNotebook(fileUri, projectData, newNotebook.id); + await this.saveProjectAndOpenNotebook(fileUri, projectData); return { id: newNotebook.id, name: notebookName }; } @@ -259,10 +258,8 @@ export class DeepnoteExplorerView { await this.treeDataProvider.refreshNotebook(treeItem.context.projectId); - // Optionally open the duplicated notebook - this.manager.selectNotebookForProject(treeItem.context.projectId, newNotebook.id); - const notebookUri = fileUri.with({ query: `notebook=${newNotebook.id}` }); - const document = await workspace.openNotebookDocument(notebookUri); + // Open the duplicated notebook + const document = await workspace.openNotebookDocument(fileUri); await window.showNotebookDocument(document, { preserveFocus: false, preview: false @@ -488,11 +485,7 @@ export class DeepnoteExplorerView { * @param projectData The project data to save * @param notebookId The notebook ID to open */ - private async saveProjectAndOpenNotebook( - fileUri: Uri, - projectData: DeepnoteFile, - notebookId: string - ): Promise { + private async saveProjectAndOpenNotebook(fileUri: Uri, projectData: DeepnoteFile): Promise { // Update metadata timestamp if (!projectData.metadata) { projectData.metadata = { createdAt: new Date().toISOString() }; @@ -504,47 +497,32 @@ export class DeepnoteExplorerView { const encoder = new TextEncoder(); await workspace.fs.writeFile(fileUri, encoder.encode(updatedYaml)); - // Refresh the tree view - use granular refresh for notebooks + // Refresh the tree view await this.treeDataProvider.refreshNotebook(projectData.project.id); - // Open the new notebook - this.manager.selectNotebookForProject(projectData.project.id, notebookId); - const notebookUri = fileUri.with({ query: `notebook=${notebookId}` }); - const document = await workspace.openNotebookDocument(notebookUri); + // Open the notebook + const document = await workspace.openNotebookDocument(fileUri); await window.showNotebookDocument(document, { preserveFocus: false, preview: false }); } + public refreshTree(): void { + this.treeDataProvider.refresh(); + } + private refreshExplorer(): void { this.treeDataProvider.refresh(); } private async openNotebook(context: DeepnoteTreeItemContext): Promise { - console.log(`Opening notebook: ${context.notebookId} in project: ${context.projectId}.`); - - if (!context.notebookId) { - await window.showWarningMessage(l10n.t('Cannot open: missing notebook id.')); - - return; - } + console.log(`Opening notebook in project: ${context.projectId}.`); try { - // Create a unique URI by adding the notebook ID as a query parameter - // This ensures VS Code treats each notebook as a separate document - const fileUri = Uri.file(context.filePath).with({ query: `notebook=${context.notebookId}` }); - - console.log(`Selecting notebook in manager.`); - - this.manager.selectNotebookForProject(context.projectId, context.notebookId); - - console.log(`Opening notebook document.`, fileUri); - + const fileUri = Uri.file(context.filePath); const document = await workspace.openNotebookDocument(fileUri); - console.log(`Showing notebook document.`); - await window.showNotebookDocument(document, { preview: false, preserveFocus: false @@ -590,7 +568,7 @@ export class DeepnoteExplorerView { // Try to reveal the notebook in the explorer try { - const treeItem = await this.treeDataProvider.findTreeItem(projectId, notebookId); + const treeItem = await this.treeDataProvider.findTreeItem(projectId); if (treeItem) { await this.treeView.reveal(treeItem, { select: true, focus: true, expand: true }); @@ -701,10 +679,7 @@ export class DeepnoteExplorerView { this.treeDataProvider.refresh(); - this.manager.selectNotebookForProject(projectId, notebookId); - - const notebookUri = fileUri.with({ query: `notebook=${notebookId}` }); - const document = await workspace.openNotebookDocument(notebookUri); + const document = await workspace.openNotebookDocument(fileUri); await window.showNotebookDocument(document, { preserveFocus: false, @@ -725,12 +700,7 @@ export class DeepnoteExplorerView { } const document = activeEditor.notebook; - - // Get the file URI (strip query params if present) - let fileUri = document.uri; - if (fileUri.query) { - fileUri = fileUri.with({ query: '' }); - } + const fileUri = document.uri; try { // Use shared helper to create and add notebook diff --git a/src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts b/src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts index 5e5e2de826..a1f12f9604 100644 --- a/src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts @@ -6,7 +6,7 @@ import { Uri, workspace } from 'vscode'; import { stringify as yamlStringify } from 'yaml'; import { DeepnoteExplorerView } from './deepnoteExplorerView'; -import { DeepnoteNotebookManager } from './deepnoteNotebookManager'; + import { DeepnoteTreeItem, DeepnoteTreeItemType, type DeepnoteTreeItemContext } from './deepnoteTreeItem'; import type { IExtensionContext } from '../../platform/common/types'; import type { DeepnoteNotebook } from '../../platform/deepnote/deepnoteTypes'; @@ -42,7 +42,6 @@ function createUuidMock(uuids: string[]): sinon.SinonStub { suite('DeepnoteExplorerView', () => { let explorerView: DeepnoteExplorerView; let mockExtensionContext: IExtensionContext; - let manager: DeepnoteNotebookManager; let mockLogger: ILogger; setup(() => { @@ -50,9 +49,8 @@ suite('DeepnoteExplorerView', () => { subscriptions: [] } as any; - manager = new DeepnoteNotebookManager(); mockLogger = createMockLogger(); - explorerView = new DeepnoteExplorerView(mockExtensionContext, manager, mockLogger); + explorerView = new DeepnoteExplorerView(mockExtensionContext, mockLogger); }); suite('constructor', () => { @@ -186,12 +184,10 @@ suite('DeepnoteExplorerView', () => { const context1 = { subscriptions: [] } as any; const context2 = { subscriptions: [] } as any; - const manager1 = new DeepnoteNotebookManager(); - const manager2 = new DeepnoteNotebookManager(); const logger1 = createMockLogger(); const logger2 = createMockLogger(); - const view1 = new DeepnoteExplorerView(context1, manager1, logger1); - const view2 = new DeepnoteExplorerView(context2, manager2, logger2); + const view1 = new DeepnoteExplorerView(context1, logger1); + const view2 = new DeepnoteExplorerView(context2, logger2); // Verify each view has its own context assert.strictEqual((view1 as any).extensionContext, context1); @@ -219,7 +215,6 @@ suite('DeepnoteExplorerView', () => { suite('DeepnoteExplorerView - Empty State Commands', () => { let explorerView: DeepnoteExplorerView; let mockContext: IExtensionContext; - let mockManager: DeepnoteNotebookManager; let sandbox: sinon.SinonSandbox; let uuidStubs: sinon.SinonStub[] = []; @@ -232,9 +227,8 @@ suite('DeepnoteExplorerView - Empty State Commands', () => { subscriptions: [] } as unknown as IExtensionContext; - mockManager = new DeepnoteNotebookManager(); const mockLogger = createMockLogger(); - explorerView = new DeepnoteExplorerView(mockContext, mockManager, mockLogger); + explorerView = new DeepnoteExplorerView(mockContext, mockLogger); }); teardown(() => { diff --git a/src/notebooks/deepnote/deepnoteFileChangeWatcher.ts b/src/notebooks/deepnote/deepnoteFileChangeWatcher.ts index b7da2080f3..76fa5158b4 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,17 +12,19 @@ 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'; -import { extractProjectIdFromSnapshotUri, isSnapshotFile } from './snapshots/snapshotFiles'; +import { + extractNotebookIdFromSnapshotUri, + extractProjectIdFromSnapshotUri, + isSnapshotFile +} from './snapshots/snapshotFiles'; import { SnapshotService } from './snapshots/snapshotService'; const debounceTimeInMilliseconds = 500; @@ -99,6 +103,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 +135,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 +165,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 +207,7 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic if (liveCells.length !== newCells.length) { return true; } + return liveCells.some( (live, i) => live.kind !== newCells[i].kind || @@ -243,12 +264,15 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic } /** - * Enqueue a snapshot-output-update for all notebooks matching this project. + * Enqueue a snapshot-output-update for all notebooks matching this project (and notebook if specified). * Does NOT replace a pending main-file-sync. */ - private enqueueSnapshotOutputUpdate(projectId: string): void { + private enqueueSnapshotOutputUpdate(projectId: string, notebookId?: string): void { const affectedNotebooks = workspace.notebookDocuments.filter( - (doc) => doc.notebookType === 'deepnote' && doc.metadata?.deepnoteProjectId === projectId + (doc) => + doc.notebookType === 'deepnote' && + doc.metadata?.deepnoteProjectId === projectId && + (!notebookId || doc.metadata?.deepnoteNotebookId === notebookId) ); for (const notebook of affectedNotebooks) { @@ -332,22 +356,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 +507,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 +541,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 +560,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 +577,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)) { @@ -561,7 +624,9 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic return; } - const key = `snapshot:${projectId}`; + const notebookId = extractNotebookIdFromSnapshotUri(uri); + + const key = notebookId ? `snapshot:${projectId}:${notebookId}` : `snapshot:${projectId}`; const existing = this.debounceTimers.get(key); if (existing) { clearTimeout(existing); @@ -571,17 +636,18 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic key, setTimeout(() => { this.debounceTimers.delete(key); - this.enqueueSnapshotOutputUpdate(projectId); + this.enqueueSnapshotOutputUpdate(projectId, notebookId); }, debounceTimeInMilliseconds) ); } /** - * 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..7877b95d04 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,11 @@ suite('DeepnoteFileChangeWatcher', () => { saveCount = 0; mockDisposables = []; - mockNotebookManager = instance(mock()); + + mockedNotebookManager = mock(); + when(mockedNotebookManager.getOriginalProject(anything())).thenReturn(validProject); + when(mockedNotebookManager.updateOriginalProject(anything(), anything())).thenReturn(); + mockNotebookManager = instance(mockedNotebookManager); // Set up FileSystemWatcher mock onDidChangeFile = new EventEmitter(); @@ -63,13 +176,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 +199,7 @@ suite('DeepnoteFileChangeWatcher', () => { } onDidChangeFile.dispose(); onDidCreateFile.dispose(); + onDidSaveNotebook.dispose(); }); function createMockNotebook(opts: { @@ -91,20 +208,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 +249,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 +273,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 +372,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 +389,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 +404,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 +422,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 +443,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 +459,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 +483,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 +508,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 +559,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 +638,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 +733,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 +755,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 +784,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 +801,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 +824,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 +834,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 +860,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 +884,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 +911,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 +930,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 +955,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 +999,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 +1027,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 +1051,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 +1126,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 +1173,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 +1181,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 +1293,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 +1326,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 +1377,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 +1421,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 +1448,998 @@ 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.getOriginalProject(anything())).thenReturn(validProject); + 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.getOriginalProject(anything())).thenReturn(validProject); + 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' + ); + }); + + // Multi-notebook test removed — multi-notebook support has been replaced by auto-splitting into separate files + test.skip('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'); + }); + }); + }); + + // Multi-notebook file sync tests removed — multi-notebook support has been replaced by auto-splitting into separate files + suite.skip('multi-notebook file sync', () => { + let workspaceSetCaptures: NotebookEditCapture[] = []; + + setup(() => { + reset(mockedNotebookManager); + when(mockedNotebookManager.getOriginalProject(anything())).thenReturn(multiNotebookProject); + 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..26dad36a01 100644 --- a/src/notebooks/deepnote/deepnoteNotebookManager.ts +++ b/src/notebooks/deepnote/deepnoteNotebookManager.ts @@ -4,24 +4,13 @@ import { IDeepnoteNotebookManager, ProjectIntegration } from '../types'; import type { DeepnoteProject } from '../../platform/deepnote/deepnoteTypes'; /** - * Centralized manager for tracking Deepnote notebook selections and project state. - * Manages per-project state including current selections and project data caching. + * Centralized manager for tracking Deepnote project state. + * Manages per-project data caching and init notebook tracking. */ @injectable() export class DeepnoteNotebookManager implements IDeepnoteNotebookManager { - private readonly currentNotebookId = new Map(); private readonly originalProjects = new Map(); private readonly projectsWithInitNotebookRun = new Set(); - private readonly selectedNotebookByProject = new Map(); - - /** - * Gets the currently selected notebook ID for a project. - * @param projectId Project identifier - * @returns Current notebook ID or undefined if not set - */ - getCurrentNotebookId(projectId: string): string | undefined { - return this.currentNotebookId.get(projectId); - } /** * Retrieves the original project data for a given project ID. @@ -33,52 +22,42 @@ export class DeepnoteNotebookManager implements IDeepnoteNotebookManager { } /** - * Gets the selected notebook ID for a specific project. + * Checks if the init notebook has already been run for a project. * @param projectId Project identifier - * @returns Selected notebook ID or undefined if not set + * @returns True if init notebook has been run, false otherwise */ - getTheSelectedNotebookForAProject(projectId: string): string | undefined { - return this.selectedNotebookByProject.get(projectId); + hasInitNotebookBeenRun(projectId: string): boolean { + return this.projectsWithInitNotebookRun.has(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. - * - * @param projectId - The project ID that identifies the Deepnote project - * @param notebookId - The ID of the selected notebook within the project + * Marks the init notebook as having been run for a project. + * @param projectId Project identifier */ - selectNotebookForProject(projectId: string, notebookId: string): void { - this.selectedNotebookByProject.set(projectId, notebookId); + markInitNotebookAsRun(projectId: string): void { + this.projectsWithInitNotebookRun.add(projectId); } /** - * Stores the original project data and sets the initial current notebook. - * This is used during deserialization to cache project data and track the active notebook. + * Stores the original project data. + * This is used during deserialization to cache project data. * @param projectId Project identifier * @param project Original project data to store - * @param notebookId Initial notebook ID to set as current */ - storeOriginalProject(projectId: string, project: DeepnoteProject, notebookId: string): void { - // Deep clone to prevent mutations from affecting stored state - // This is critical for multi-notebook projects where multiple notebooks - // share the same stored project reference - // Using structuredClone to handle circular references (e.g., in output metadata) + storeOriginalProject(projectId: string, project: DeepnoteProject): void { const clonedProject = structuredClone(project); - this.originalProjects.set(projectId, clonedProject); - this.currentNotebookId.set(projectId, notebookId); } /** - * Updates the current notebook ID for a project. - * Used when switching notebooks within the same project. + * Updates the stored project data. + * Used during serialization where we need to cache the updated project state. * @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); } /** @@ -98,32 +77,8 @@ export class DeepnoteNotebookManager implements IDeepnoteNotebookManager { const updatedProject = structuredClone(project); updatedProject.project.integrations = integrations; - - const currentNotebookId = this.currentNotebookId.get(projectId); - - if (currentNotebookId) { - this.storeOriginalProject(projectId, updatedProject, currentNotebookId); - } else { - this.originalProjects.set(projectId, updatedProject); - } + this.originalProjects.set(projectId, updatedProject); return true; } - - /** - * Checks if the init notebook has already been run for a project. - * @param projectId Project identifier - * @returns True if init notebook has been run, false otherwise - */ - hasInitNotebookBeenRun(projectId: string): boolean { - return this.projectsWithInitNotebookRun.has(projectId); - } - - /** - * Marks the init notebook as having been run for a project. - * @param projectId Project identifier - */ - markInitNotebookAsRun(projectId: string): void { - this.projectsWithInitNotebookRun.add(projectId); - } } diff --git a/src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts b/src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts index feb2842848..7d3b226843 100644 --- a/src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts @@ -25,31 +25,6 @@ suite('DeepnoteNotebookManager', () => { manager = new DeepnoteNotebookManager(); }); - suite('getCurrentNotebookId', () => { - test('should return undefined for unknown project', () => { - const result = manager.getCurrentNotebookId('unknown-project'); - - assert.strictEqual(result, undefined); - }); - - test('should return notebook ID after storing project', () => { - manager.storeOriginalProject('project-123', mockProject, 'notebook-456'); - - const result = manager.getCurrentNotebookId('project-123'); - - 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', () => { test('should return undefined for unknown project', () => { const result = manager.getOriginalProject('unknown-project'); @@ -58,7 +33,7 @@ suite('DeepnoteNotebookManager', () => { }); test('should return original project after storing', () => { - manager.storeOriginalProject('project-123', mockProject, 'notebook-456'); + manager.storeOriginalProject('project-123', mockProject); const result = manager.getOriginalProject('project-123'); @@ -66,72 +41,13 @@ suite('DeepnoteNotebookManager', () => { }); }); - suite('getTheSelectedNotebookForAProject', () => { - test('should return undefined for unknown project', () => { - const result = manager.getTheSelectedNotebookForAProject('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'); - - assert.strictEqual(result, 'notebook-456'); - }); - - 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'); - }); - }); - - 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'); - - 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'); - }); - }); - suite('storeOriginalProject', () => { - test('should store both project and current notebook ID', () => { - manager.storeOriginalProject('project-123', mockProject, 'notebook-456'); + test('should store project data', () => { + manager.storeOriginalProject('project-123', mockProject); const storedProject = manager.getOriginalProject('project-123'); - const currentNotebookId = manager.getCurrentNotebookId('project-123'); assert.deepStrictEqual(storedProject, mockProject); - assert.strictEqual(currentNotebookId, 'notebook-456'); }); test('should overwrite existing project data', () => { @@ -143,50 +59,84 @@ suite('DeepnoteNotebookManager', () => { } }; - manager.storeOriginalProject('project-123', mockProject, 'notebook-456'); - manager.storeOriginalProject('project-123', updatedProject, 'notebook-789'); + manager.storeOriginalProject('project-123', mockProject); + manager.storeOriginalProject('project-123', updatedProject); const storedProject = manager.getOriginalProject('project-123'); - const currentNotebookId = manager.getCurrentNotebookId('project-123'); assert.deepStrictEqual(storedProject, updatedProject); - assert.strictEqual(currentNotebookId, 'notebook-789'); }); }); - suite('updateCurrentNotebookId', () => { - test('should update notebook ID for existing project', () => { - manager.storeOriginalProject('project-123', mockProject, 'notebook-456'); - manager.updateCurrentNotebookId('project-123', 'notebook-789'); + suite('updateOriginalProject', () => { + test('should update project data', () => { + const updatedProject: DeepnoteProject = { + ...mockProject, + project: { + ...mockProject.project, + name: 'Updated Name Only' + } + }; + + manager.storeOriginalProject('project-123', mockProject); + manager.updateOriginalProject('project-123', updatedProject); - const result = manager.getCurrentNotebookId('project-123'); + const storedProject = manager.getOriginalProject('project-123'); - assert.strictEqual(result, 'notebook-789'); + assert.deepStrictEqual(storedProject, updatedProject); }); - 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' + } + }; + + manager.storeOriginalProject('project-123', mockProject); + manager.updateOriginalProject('project-123', updatedProject); - const result = manager.getCurrentNotebookId('new-project'); + updatedProject.project.name = 'After Mutation'; - assert.strictEqual(result, 'notebook-123'); + 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 on successive updates', () => { + const firstUpdate: DeepnoteProject = { + ...mockProject, + project: { ...mockProject.project, name: 'First Update' } + }; + const secondUpdate: DeepnoteProject = { + ...mockProject, + project: { ...mockProject.project, name: 'Second Update' } + }; + + manager.storeOriginalProject('project-123', mockProject); + manager.updateOriginalProject('project-123', firstUpdate); + manager.updateOriginalProject('project-123', secondUpdate); + + assert.strictEqual(manager.getOriginalProject('project-123')?.project.name, 'Second Update'); + }); - const result1 = manager.getCurrentNotebookId('project-1'); - const result2 = manager.getCurrentNotebookId('project-2'); + test('should store project when no prior data exists', () => { + const projectOnly: DeepnoteProject = { + ...mockProject, + project: { ...mockProject.project, name: 'No Notebook Id Yet' } + }; - assert.strictEqual(result1, 'notebook-1'); - assert.strictEqual(result2, 'notebook-2'); + manager.updateOriginalProject('project-123', projectOnly); + + assert.deepStrictEqual(manager.getOriginalProject('project-123'), projectOnly); }); }); suite('updateProjectIntegrations', () => { test('should update integrations list for existing project and return true', () => { - manager.storeOriginalProject('project-123', mockProject, 'notebook-456'); + manager.storeOriginalProject('project-123', mockProject); const integrations: ProjectIntegration[] = [ { id: 'int-1', name: 'PostgreSQL', type: 'pgsql' }, @@ -210,7 +160,7 @@ suite('DeepnoteNotebookManager', () => { } }; - manager.storeOriginalProject('project-123', projectWithIntegrations, 'notebook-456'); + manager.storeOriginalProject('project-123', projectWithIntegrations); const newIntegrations: ProjectIntegration[] = [ { id: 'new-int-1', name: 'New Integration 1', type: 'pgsql' }, @@ -234,7 +184,7 @@ suite('DeepnoteNotebookManager', () => { } }; - manager.storeOriginalProject('project-123', projectWithIntegrations, 'notebook-456'); + manager.storeOriginalProject('project-123', projectWithIntegrations); const result = manager.updateProjectIntegrations('project-123', []); @@ -256,7 +206,7 @@ suite('DeepnoteNotebookManager', () => { }); test('should preserve other project properties and return true', () => { - manager.storeOriginalProject('project-123', mockProject, 'notebook-456'); + manager.storeOriginalProject('project-123', mockProject); const integrations: ProjectIntegration[] = [{ id: 'int-1', name: 'PostgreSQL', type: 'pgsql' }]; @@ -271,10 +221,9 @@ suite('DeepnoteNotebookManager', () => { assert.deepStrictEqual(updatedProject?.metadata, mockProject.metadata); }); - 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); + test('should update integrations when project was stored via updateOriginalProject and return true', () => { + // Use updateOriginalProject which doesn't set currentNotebookId + manager.updateOriginalProject('project-123', mockProject); const integrations: ProjectIntegration[] = [ { id: 'int-1', name: 'PostgreSQL', type: 'pgsql' }, @@ -295,41 +244,33 @@ 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'); + suite('hasInitNotebookBeenRun', () => { + test('should return false for unknown project', () => { + assert.strictEqual(manager.hasInitNotebookBeenRun('unknown-project'), false); + }); - manager.storeOriginalProject('project-2', mockProject, 'notebook-2'); - manager.selectNotebookForProject('project-2', 'notebook-2'); + test('should return true after marking init notebook as run', () => { + manager.markInitNotebookAsRun('project-123'); - 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'); + assert.strictEqual(manager.hasInitNotebookBeenRun('project-123'), true); }); + }); - test('should handle notebook switching within same project', () => { - 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'); + suite('markInitNotebookAsRun', () => { + test('should mark init notebook as run for a project', () => { + manager.markInitNotebookAsRun('project-123'); - assert.strictEqual(manager.getCurrentNotebookId('project-123'), 'notebook-2'); - assert.strictEqual(manager.getTheSelectedNotebookForAProject('project-123'), 'notebook-2'); + assert.strictEqual(manager.hasInitNotebookBeenRun('project-123'), true); }); + }); - 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'); + suite('integration scenarios', () => { + test('should handle complete workflow for multiple projects', () => { + manager.storeOriginalProject('project-1', mockProject); + manager.storeOriginalProject('project-2', mockProject); - // Both should be maintained independently - assert.strictEqual(manager.getCurrentNotebookId('project-123'), 'notebook-original'); - assert.strictEqual(manager.getTheSelectedNotebookForAProject('project-123'), 'notebook-selected'); + assert.deepStrictEqual(manager.getOriginalProject('project-1'), mockProject); + assert.deepStrictEqual(manager.getOriginalProject('project-2'), mockProject); }); }); }); diff --git a/src/notebooks/deepnote/deepnoteSerializer.ts b/src/notebooks/deepnote/deepnoteSerializer.ts index 17443e93b9..83d89ec145 100644 --- a/src/notebooks/deepnote/deepnoteSerializer.ts +++ b/src/notebooks/deepnote/deepnoteSerializer.ts @@ -1,14 +1,14 @@ 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 { workspace, type CancellationToken, type NotebookData, 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'; @@ -62,7 +62,8 @@ 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 @@ -74,13 +75,6 @@ export class DeepnoteNotebookSerializer implements NotebookSerializer { 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 +84,28 @@ export class DeepnoteNotebookSerializer implements NotebookSerializer { } const projectId = deepnoteFile.project.id; - const notebookId = this.findCurrentNotebookId(projectId); + const selectedNotebook = this.findDefaultNotebook(deepnoteFile); - logger.debug(`DeepnoteSerializer: Project ID: ${projectId}, Selected notebook ID: ${notebookId}`); + logger.debug(`DeepnoteSerializer: Project ID: ${projectId}, Selected notebook ID: ${selectedNotebook?.id}`); - if (deepnoteFile.project.notebooks.length === 0) { + if (!selectedNotebook) { throw new Error('Deepnote project contains no notebooks.'); } - const selectedNotebook = notebookId - ? deepnoteFile.project.notebooks.find((nb) => nb.id === notebookId) - : this.findDefaultNotebook(deepnoteFile); + // Initialize vega-lite for output conversion (lazy-loaded ESM module) + await this.converter.initialize(); - if (!selectedNotebook) { - throw new Error(l10n.t('No notebook selected or found')); + 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`); @@ -125,7 +118,7 @@ export class DeepnoteNotebookSerializer implements NotebookSerializer { if (this.snapshotService?.isSnapshotsEnabled()) { logger.debug(`[Snapshot] Snapshots enabled, reading snapshot for project ${projectId}`); try { - const snapshotOutputs = await this.snapshotService.readSnapshot(projectId); + const snapshotOutputs = await this.snapshotService.readSnapshot(projectId, selectedNotebook.id); if (snapshotOutputs && snapshotOutputs.size > 0) { logger.debug(`[Snapshot] Merging ${snapshotOutputs.size} block outputs from snapshot`); @@ -157,7 +150,7 @@ export class DeepnoteNotebookSerializer implements NotebookSerializer { ); } - this.notebookManager.storeOriginalProject(deepnoteFile.project.id, deepnoteFile, selectedNotebook.id); + this.notebookManager.storeOriginalProject(deepnoteFile.project.id, deepnoteFile); logger.debug(`DeepnoteSerializer: Stored project ${projectId} in notebook manager`); return { @@ -181,39 +174,6 @@ 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 - */ - 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; - } - - // Check the manager's stored selection - this should be set when opening from explorer - const storedNotebookId = this.notebookManager.getTheSelectedNotebookForAProject(projectId); - - if (storedNotebookId) { - return storedNotebookId; - } - - // 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; - } - /** * Gets the data converter instance for cell/block conversion. * @returns DeepnoteDataConverter instance @@ -258,8 +218,7 @@ export class DeepnoteNotebookSerializer implements NotebookSerializer { logger.debug('SerializeNotebook: Got and cloned original project'); - const notebookId = - data.metadata?.deepnoteNotebookId || this.notebookManager.getTheSelectedNotebookForAProject(projectId); + const notebookId = data.metadata?.deepnoteNotebookId; if (!notebookId) { throw new Error('Cannot determine which notebook to save'); @@ -346,8 +305,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'); @@ -551,25 +514,23 @@ export class DeepnoteNotebookSerializer implements NotebookSerializer { } /** - * Finds the default notebook to open when no selection is made. - * @param file - * @returns + * Finds the default notebook — the first non-init notebook in array order. + * Falls back to the first notebook if only init exists. */ private findDefaultNotebook(file: DeepnoteFile): DeepnoteNotebook | undefined { if (file.project.notebooks.length === 0) { return undefined; } - const sortedNotebooks = file.project.notebooks.slice().sort((a, b) => a.name.localeCompare(b.name)); - const sortedNotebooksWithoutInit = file.project.initNotebookId - ? sortedNotebooks.filter((nb) => nb.id !== file.project.initNotebookId) - : sortedNotebooks; + const notebooksWithoutInit = file.project.initNotebookId + ? file.project.notebooks.filter((nb) => nb.id !== file.project.initNotebookId) + : file.project.notebooks; - if (sortedNotebooksWithoutInit.length > 0) { - return sortedNotebooksWithoutInit[0]; + if (notebooksWithoutInit.length > 0) { + return notebooksWithoutInit[0]; } - return sortedNotebooks[0]; + return file.project.notebooks[0]; } /** diff --git a/src/notebooks/deepnote/deepnoteSerializer.unit.test.ts b/src/notebooks/deepnote/deepnoteSerializer.unit.test.ts index c3332f974d..a5028c27a6 100644 --- a/src/notebooks/deepnote/deepnoteSerializer.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteSerializer.unit.test.ts @@ -1,13 +1,10 @@ import { deserializeDeepnoteFile, serializeDeepnoteFile, type DeepnoteFile } from '@deepnote/blocks'; import { assert } from 'chai'; import { parse as parseYaml } from 'yaml'; -import { when } from 'ts-mockito'; -import type { NotebookDocument } from 'vscode'; import { DeepnoteNotebookSerializer } from './deepnoteSerializer'; import { DeepnoteNotebookManager } from './deepnoteNotebookManager'; import { DeepnoteDataConverter } from './deepnoteDataConverter'; -import { mockedVSCodeNamespaces } from '../../test/vscode-mock'; suite('DeepnoteNotebookSerializer', () => { let serializer: DeepnoteNotebookSerializer; @@ -74,10 +71,7 @@ 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', async () => { const yamlContent = ` version: '1.0.0' metadata: @@ -107,9 +101,6 @@ project: assert.isDefined(result); assert.isDefined(result.cells); assert.isArray(result.cells); - assert.strictEqual(result.cells.length, 1); - assert.strictEqual(result.metadata?.deepnoteProjectId, 'project-123'); - assert.strictEqual(result.metadata?.deepnoteNotebookId, 'notebook-1'); }); test('should throw error for empty content', async () => { @@ -144,9 +135,19 @@ project: await assert.isRejected( serializer.deserializeNotebook(contentWithoutNotebooks, {} as any), - /no notebooks|notebooks.*must contain at least 1/i + /Failed to parse Deepnote file/ ); }); + + test('should deserialize default notebook when no explicit notebook ID is provided', async () => { + const content = projectToYaml(mockProject); + const result = await serializer.deserializeNotebook(content, {} as any); + + assert.strictEqual(result.metadata?.deepnoteProjectId, 'project-123'); + assert.strictEqual(result.metadata?.deepnoteNotebookId, 'notebook-1'); + assert.strictEqual(result.metadata?.deepnoteNotebookName, 'First Notebook'); + assert.isAbove(result.cells.length, 0); + }); }); suite('serializeNotebook', () => { @@ -179,7 +180,7 @@ project: test('should serialize notebook when original project exists', async () => { // First store the original project - manager.storeOriginalProject('project-123', mockProject, 'notebook-1'); + manager.storeOriginalProject('project-123', mockProject); const mockNotebookData = { cells: [ @@ -205,163 +206,28 @@ project: assert.include(yamlString, 'project-123'); assert.include(yamlString, 'notebook-1'); }); - }); - - suite('findCurrentNotebookId', () => { - teardown(() => { - // Reset only the specific mocks used in this suite - when(mockedVSCodeNamespaces.window.activeNotebookEditor).thenReturn(undefined); - when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([]); - }); - - test('should return stored notebook ID when available', () => { - manager.selectNotebookForProject('project-123', 'notebook-456'); - - const result = serializer.findCurrentNotebookId('project-123'); - - assert.strictEqual(result, 'notebook-456'); - }); - - test('should fall back to active notebook document when no stored selection', () => { - // Create a mock notebook document - const mockNotebookDoc = { - then: undefined, // Prevent mock from being treated as a Promise-like thenable - notebookType: 'deepnote', - metadata: { - deepnoteProjectId: 'project-123', - deepnoteNotebookId: 'notebook-from-workspace' - }, - uri: {} as any, - version: 1, - isDirty: false, - isUntitled: false, - isClosed: false, - cellCount: 0, - cellAt: () => ({}) as any, - getCells: () => [], - 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'); - }); - - 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); - - 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'); - }); - test('should ignore active editor when notebook ID is missing', () => { - manager.selectNotebookForProject('project-123', 'stored-notebook'); + test('should throw error when metadata notebook ID is missing', async () => { + manager.storeOriginalProject('project-123', mockProject); - // Mock active editor without notebook ID in metadata - const mockActiveNotebook = { - notebookType: 'deepnote', + const mockNotebookData = { + cells: [ + { + kind: 1, // NotebookCellKind.Markup + value: '# Updated second notebook', + languageId: 'markdown', + metadata: {} + } + ], metadata: { deepnoteProjectId: 'project-123' - // Missing deepnoteNotebookId } }; - when(mockedVSCodeNamespaces.window.activeNotebookEditor).thenReturn({ - notebook: mockActiveNotebook - } as any); - - const result = serializer.findCurrentNotebookId('project-123'); - - // Should fall back to stored selection since active editor has no notebook ID - assert.strictEqual(result, 'stored-notebook'); + await assert.isRejected( + serializer.serializeNotebook(mockNotebookData as any, {} as any), + /Cannot determine which notebook to save/ + ); }); }); @@ -388,19 +254,9 @@ project: }); test('should handle manager state operations', () => { - 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.storeOriginalProject, 'has storeOriginalProject method'); }); - - test('should have findCurrentNotebookId method', () => { - assert.isFunction(serializer.findCurrentNotebookId, 'has findCurrentNotebookId method'); - }); }); suite('data structure handling', () => { @@ -472,7 +328,7 @@ project: } }; - manager.storeOriginalProject('project-circular', projectWithCircularRef, 'notebook-1'); + manager.storeOriginalProject('project-circular', projectWithCircularRef); const notebookData = { cells: [ @@ -540,7 +396,7 @@ project: }; // Store the project - manager.storeOriginalProject('project-id-test', projectData, 'notebook-1'); + manager.storeOriginalProject('project-id-test', projectData); // Create cells with the EXACT metadata structure that deserializeNotebook produces // This simulates what VS Code should preserve from deserialization @@ -626,7 +482,7 @@ project: } }; - manager.storeOriginalProject('project-recover-ids', projectData, 'notebook-1'); + manager.storeOriginalProject('project-recover-ids', projectData); // Cells WITHOUT id metadata (simulating what VS Code might provide if it strips metadata) // But content matches the original block @@ -693,7 +549,7 @@ project: } }; - manager.storeOriginalProject('project-new-content', projectData, 'notebook-1'); + manager.storeOriginalProject('project-new-content', projectData); // Cell with different content than any original block const notebookData = { @@ -757,251 +613,6 @@ project: }); }); - suite('default notebook selection', () => { - test('should not select Init notebook when other notebooks are available', async () => { - const projectData: DeepnoteFile = { - version: '1.0.0', - metadata: { - createdAt: '2023-01-01T00:00:00Z', - modifiedAt: '2023-01-02T00:00:00Z' - }, - project: { - id: 'project-with-init', - name: 'Project with Init', - initNotebookId: 'init-notebook', - notebooks: [ - { - id: 'init-notebook', - name: 'Init', - blocks: [ - { - id: 'block-init', - content: 'print("init")', - sortingKey: 'a0', - metadata: {}, - blockGroup: '1', - type: 'code' - } - ], - executionMode: 'block', - isModule: false - }, - { - id: 'main-notebook', - name: 'Main', - blocks: [ - { - id: 'block-main', - content: 'print("main")', - sortingKey: 'a0', - metadata: {}, - blockGroup: '1', - type: 'code' - } - ], - executionMode: 'block', - isModule: false - } - ], - settings: {} - } - }; - - const content = projectToYaml(projectData); - const result = await serializer.deserializeNotebook(content, {} as any); - - // Should select the Main notebook, not the Init notebook - assert.strictEqual(result.metadata?.deepnoteNotebookId, 'main-notebook'); - assert.strictEqual(result.metadata?.deepnoteNotebookName, 'Main'); - }); - - test('should select Init notebook when it is the only notebook', async () => { - const projectData: DeepnoteFile = { - version: '1.0.0', - metadata: { - createdAt: '2023-01-01T00:00:00Z', - modifiedAt: '2023-01-02T00:00:00Z' - }, - project: { - id: 'project-only-init', - name: 'Project with only Init', - initNotebookId: 'init-notebook', - notebooks: [ - { - id: 'init-notebook', - name: 'Init', - blocks: [ - { - id: 'block-init', - content: 'print("init")', - sortingKey: 'a0', - blockGroup: '1', - metadata: {}, - type: 'code' - } - ], - executionMode: 'block', - isModule: false - } - ], - settings: {} - } - }; - - const content = projectToYaml(projectData); - const result = await serializer.deserializeNotebook(content, {} as any); - - // Should select the Init notebook since it's the only one - assert.strictEqual(result.metadata?.deepnoteNotebookId, 'init-notebook'); - assert.strictEqual(result.metadata?.deepnoteNotebookName, 'Init'); - }); - - test('should select alphabetically first notebook when no initNotebookId', async () => { - const projectData: DeepnoteFile = { - version: '1.0.0', - metadata: { - createdAt: '2023-01-01T00:00:00Z', - modifiedAt: '2023-01-02T00:00:00Z' - }, - project: { - id: 'project-alphabetical', - name: 'Project Alphabetical', - notebooks: [ - { - id: 'zebra-notebook', - name: 'Zebra Notebook', - blocks: [ - { - id: 'block-z', - content: 'print("zebra")', - sortingKey: 'a0', - blockGroup: '1', - metadata: {}, - type: 'code' - } - ], - executionMode: 'block', - isModule: false - }, - { - id: 'alpha-notebook', - name: 'Alpha Notebook', - blocks: [ - { - id: 'block-a', - content: 'print("alpha")', - sortingKey: 'a0', - blockGroup: '1', - metadata: {}, - type: 'code' - } - ], - executionMode: 'block', - isModule: false - }, - { - id: 'bravo-notebook', - name: 'Bravo Notebook', - blocks: [ - { - id: 'block-b', - content: 'print("bravo")', - sortingKey: 'a0', - blockGroup: '1', - metadata: {}, - type: 'code' - } - ], - executionMode: 'block', - isModule: false - } - ], - settings: {} - } - }; - - const content = projectToYaml(projectData); - const result = await serializer.deserializeNotebook(content, {} as any); - - // Should select the alphabetically first notebook - assert.strictEqual(result.metadata?.deepnoteNotebookId, 'alpha-notebook'); - assert.strictEqual(result.metadata?.deepnoteNotebookName, 'Alpha Notebook'); - }); - - test('should sort Init notebook last when multiple notebooks exist', async () => { - const projectData: DeepnoteFile = { - version: '1.0.0', - metadata: { - createdAt: '2023-01-01T00:00:00Z', - modifiedAt: '2023-01-02T00:00:00Z' - }, - project: { - id: 'project-multiple', - name: 'Project with Multiple', - initNotebookId: 'init-notebook', - notebooks: [ - { - id: 'charlie-notebook', - name: 'Charlie', - blocks: [ - { - id: 'block-c', - content: 'print("charlie")', - sortingKey: 'a0', - blockGroup: '1', - metadata: {}, - type: 'code' - } - ], - executionMode: 'block', - isModule: false - }, - { - id: 'init-notebook', - name: 'Init', - blocks: [ - { - id: 'block-init', - content: 'print("init")', - sortingKey: 'a0', - blockGroup: '1', - metadata: {}, - type: 'code' - } - ], - executionMode: 'block', - isModule: false - }, - { - id: 'alpha-notebook', - name: 'Alpha', - blocks: [ - { - id: 'block-a', - content: 'print("alpha")', - sortingKey: 'a0', - blockGroup: '1', - metadata: {}, - type: 'code' - } - ], - executionMode: 'block', - isModule: false - } - ], - settings: {} - } - }; - - const content = projectToYaml(projectData); - const result = await serializer.deserializeNotebook(content, {} as any); - - // 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'); - }); - }); - suite('detectContentChanges', () => { test('should detect no changes when content is identical', () => { const project: DeepnoteFile = { @@ -1491,7 +1102,7 @@ project: } }; - manager.storeOriginalProject('project-snapshot-hash', projectData, 'notebook-1'); + manager.storeOriginalProject('project-snapshot-hash', projectData); const notebookData = { cells: [ @@ -1566,13 +1177,13 @@ project: }; // Serialize twice - manager.storeOriginalProject('project-deterministic', structuredClone(projectData), 'notebook-1'); + manager.storeOriginalProject('project-deterministic', structuredClone(projectData)); const result1 = await serializer.serializeNotebook(notebookData as any, {} as any); const parsed1 = parseYaml(new TextDecoder().decode(result1)) as DeepnoteFile & { metadata: { snapshotHash?: string }; }; - manager.storeOriginalProject('project-deterministic', structuredClone(projectData), 'notebook-1'); + manager.storeOriginalProject('project-deterministic', structuredClone(projectData)); const result2 = await serializer.serializeNotebook(notebookData as any, {} as any); const parsed2 = parseYaml(new TextDecoder().decode(result2)) as DeepnoteFile & { metadata: { snapshotHash?: string }; @@ -1667,7 +1278,7 @@ project: // Serialize 5 times and collect all hashes for (let i = 0; i < 5; i++) { - manager.storeOriginalProject('project-multi-serialize', structuredClone(projectData), 'notebook-1'); + manager.storeOriginalProject('project-multi-serialize', structuredClone(projectData)); const result = await serializer.serializeNotebook(notebookData as any, {} as any); const parsed = parseYaml(new TextDecoder().decode(result)) as DeepnoteFile & { metadata: { snapshotHash?: string }; @@ -1716,7 +1327,7 @@ project: } }; - manager.storeOriginalProject('project-content-change', projectData1, 'notebook-1'); + manager.storeOriginalProject('project-content-change', projectData1); const notebookData1 = { cells: [ @@ -1794,7 +1405,7 @@ project: } }; - manager.storeOriginalProject('project-version-change', projectData1, 'notebook-1'); + manager.storeOriginalProject('project-version-change', projectData1); const notebookData = { cells: [ @@ -1818,7 +1429,7 @@ project: // Change version const projectData2: DeepnoteFile = { ...structuredClone(projectData1), version: '2.0' }; - manager.storeOriginalProject('project-version-change', projectData2, 'notebook-1'); + manager.storeOriginalProject('project-version-change', projectData2); const result2 = await serializer.serializeNotebook(notebookData as any, {} as any); const parsed2 = parseYaml(new TextDecoder().decode(result2)) as DeepnoteFile & { @@ -1860,7 +1471,7 @@ project: } }; - manager.storeOriginalProject('project-integrations-change', projectData1, 'notebook-1'); + manager.storeOriginalProject('project-integrations-change', projectData1); const notebookData = { cells: [ @@ -1885,7 +1496,7 @@ project: // Add integrations const projectData2 = structuredClone(projectData1); projectData2.project.integrations = [{ id: 'int-1', name: 'PostgreSQL', type: 'postgres' }]; - manager.storeOriginalProject('project-integrations-change', projectData2, 'notebook-1'); + manager.storeOriginalProject('project-integrations-change', projectData2); const result2 = await serializer.serializeNotebook(notebookData as any, {} as any); const parsed2 = parseYaml(new TextDecoder().decode(result2)) as DeepnoteFile & { @@ -1927,7 +1538,7 @@ project: } }; - manager.storeOriginalProject('project-env-hash', projectData1, 'notebook-1'); + manager.storeOriginalProject('project-env-hash', projectData1); const notebookData = { cells: [ @@ -1952,7 +1563,7 @@ project: // Add environment hash const projectData2 = structuredClone(projectData1); projectData2.environment = { hash: 'env-hash-123' }; - manager.storeOriginalProject('project-env-hash', projectData2, 'notebook-1'); + manager.storeOriginalProject('project-env-hash', projectData2); const result2 = await serializer.serializeNotebook(notebookData as any, {} as any); const parsed2 = parseYaml(new TextDecoder().decode(result2)) as DeepnoteFile & { diff --git a/src/notebooks/deepnote/deepnoteTreeDataProvider.ts b/src/notebooks/deepnote/deepnoteTreeDataProvider.ts index be9e471e27..96c4bc2d1e 100644 --- a/src/notebooks/deepnote/deepnoteTreeDataProvider.ts +++ b/src/notebooks/deepnote/deepnoteTreeDataProvider.ts @@ -13,7 +13,12 @@ import { l10n } from 'vscode'; -import { DeepnoteTreeItem, DeepnoteTreeItemType, DeepnoteTreeItemContext } from './deepnoteTreeItem'; +import { + DeepnoteTreeItem, + DeepnoteTreeItemType, + DeepnoteTreeItemContext, + type ProjectGroupData +} from './deepnoteTreeItem'; import type { DeepnoteProject, DeepnoteNotebook } from '../../platform/deepnote/deepnoteTypes'; import { readDeepnoteProjectFile } from './deepnoteProjectUtils'; import { ILogger } from '../../platform/logging/types'; @@ -28,9 +33,17 @@ export function compareTreeItemsByLabel(a: DeepnoteTreeItem, b: DeepnoteTreeItem return labelA.toLowerCase().localeCompare(labelB.toLowerCase()); } +/** + * Loaded project file data + */ +interface LoadedProjectFile { + filePath: string; + project: DeepnoteProject; +} + /** * Tree data provider for the Deepnote explorer view. - * Manages the tree structure displaying Deepnote project files and their notebooks. + * Groups files by project ID, showing project groups at the top level. */ export class DeepnoteTreeDataProvider implements TreeDataProvider { private _onDidChangeTreeData: EventEmitter = new EventEmitter< @@ -67,82 +80,24 @@ export class DeepnoteTreeDataProvider implements TreeDataProvider { - // Get the cached tree item BEFORE clearing caches - const cacheKey = `project:${filePath}`; - const cachedTreeItem = this.treeItemCache.get(cacheKey); - - // Clear the project data cache to force reload this.cachedProjects.delete(filePath); - - if (cachedTreeItem) { - // Reload the project data and update the cached tree item - try { - const fileUri = Uri.file(filePath); - const project = await this.loadDeepnoteProject(fileUri); - if (project) { - // Update the tree item's data - cachedTreeItem.data = project; - - // Update visual fields (label, description, tooltip) to reflect changes - cachedTreeItem.updateVisualFields(); - } - } catch (error) { - this.logger?.error(`Failed to reload project ${filePath}`, error); - } - - // Fire change event with the existing cached tree item - this._onDidChangeTreeData.fire(cachedTreeItem); - } else { - // If not found in cache, do a full refresh - this._onDidChangeTreeData.fire(); - } + // Full refresh since project grouping may have changed + this._onDidChangeTreeData.fire(); } /** * Refresh notebooks for a specific project - * @param projectId The project ID whose notebooks should be refreshed */ public async refreshNotebook(projectId: string): Promise { - // Find the cached tree item by scanning the cache - let cachedTreeItem: DeepnoteTreeItem | undefined; - let filePath: string | undefined; - - for (const [key, item] of this.treeItemCache.entries()) { - if (key.startsWith('project:') && item.context.projectId === projectId) { - cachedTreeItem = item; - filePath = item.context.filePath; - break; + // Clear all cached projects that match this project ID to force reload + for (const [path, project] of this.cachedProjects.entries()) { + if (project.project.id === projectId) { + this.cachedProjects.delete(path); } } - - if (cachedTreeItem && filePath) { - // Clear the project data cache to force reload - this.cachedProjects.delete(filePath); - - // Reload the project data and update the cached tree item - try { - const fileUri = Uri.file(filePath); - const project = await this.loadDeepnoteProject(fileUri); - if (project) { - // Update the tree item's data - cachedTreeItem.data = project; - - // Update visual fields (label, description, tooltip) to reflect changes - cachedTreeItem.updateVisualFields(); - } - } catch (error) { - this.logger?.error(`Failed to reload project ${filePath}`, error); - } - - // Fire change event with the existing cached tree item to refresh its children - this._onDidChangeTreeData.fire(cachedTreeItem); - } else { - // If not found in cache, do a full refresh - this._onDidChangeTreeData.fire(); - } + this._onDidChangeTreeData.fire(); } public getTreeItem(element: DeepnoteTreeItem): TreeItem { @@ -150,8 +105,11 @@ export class DeepnoteTreeDataProvider implements TreeDataProvider { - // If element is provided, we can return children regardless of workspace if (element) { + if (element.type === DeepnoteTreeItemType.ProjectGroup) { + return this.getFilesForProjectGroup(element); + } + if (element.type === DeepnoteTreeItemType.ProjectFile) { return this.getNotebooksForProject(element); } @@ -159,7 +117,7 @@ export class DeepnoteTreeDataProvider implements TreeDataProvider { try { - await this.getDeepnoteProjectFiles(); + await this.loadAllProjectFiles(); } finally { this.isInitialScanComplete = true; this.initialScanPromise = undefined; @@ -198,8 +156,11 @@ export class DeepnoteTreeDataProvider implements TreeDataProvider { - const deepnoteFiles: DeepnoteTreeItem[] = []; + /** + * Load all .deepnote project files from the workspace + */ + private async loadAllProjectFiles(): Promise { + const results: LoadedProjectFile[] = []; for (const workspaceFolder of workspace.workspaceFolders || []) { const pattern = new RelativePattern(workspaceFolder, '**/*.deepnote'); @@ -209,64 +170,128 @@ export class DeepnoteTreeDataProvider implements TreeDataProvider { + const allFiles = await this.loadAllProjectFiles(); + + // Group by project ID + const groupsByProjectId = new Map(); + + for (const file of allFiles) { + const projectId = file.project.project.id; + const existing = groupsByProjectId.get(projectId) ?? []; + existing.push(file); + groupsByProjectId.set(projectId, existing); + } + + const groups: DeepnoteTreeItem[] = []; + + for (const [projectId, files] of groupsByProjectId.entries()) { + const projectName = files[0].project.project.name; + + // If only one file with one non-init notebook, show directly as a project file (no group nesting) + if (files.length === 1) { + const file = files[0]; + const initNotebookId = file.project.project.initNotebookId; + const nonInitNotebooks = file.project.project.notebooks?.filter((nb) => nb.id !== initNotebookId) ?? []; + + if (nonInitNotebooks.length <= 1) { const context: DeepnoteTreeItemContext = { - filePath: file.path, - projectId: project.project.id + filePath: file.filePath, + projectId }; - // Check if we have a cached tree item for this project - const cacheKey = `project:${file.path}`; - let treeItem = this.treeItemCache.get(cacheKey); - - if (!treeItem) { - // Create new tree item only if not cached - const hasNotebooks = project.project.notebooks && project.project.notebooks.length > 0; - const collapsibleState = hasNotebooks - ? TreeItemCollapsibleState.Collapsed - : TreeItemCollapsibleState.None; - - treeItem = new DeepnoteTreeItem( - DeepnoteTreeItemType.ProjectFile, - context, - project, - collapsibleState - ); - - this.treeItemCache.set(cacheKey, treeItem); - } else { - // Update the cached tree item's data - treeItem.data = project; - } - - deepnoteFiles.push(treeItem); - } catch (error) { - this.logger?.error(`Failed to load Deepnote project from ${file.path}`, error); + const treeItem = new DeepnoteTreeItem( + DeepnoteTreeItemType.ProjectFile, + context, + file.project, + TreeItemCollapsibleState.None + ); + groups.push(treeItem); + continue; } } + + // Multiple files or multi-notebook file: create a group + const groupData: ProjectGroupData = { + projectId, + projectName, + files: files.map((f) => ({ filePath: f.filePath, project: f.project })) + }; + + const context: DeepnoteTreeItemContext = { + filePath: files[0].filePath, + projectId + }; + + const groupItem = new DeepnoteTreeItem( + DeepnoteTreeItemType.ProjectGroup, + context, + groupData, + TreeItemCollapsibleState.Collapsed + ); + groups.push(groupItem); + } + + groups.sort(compareTreeItemsByLabel); + + return groups; + } + + /** + * Get file items for a project group + */ + private getFilesForProjectGroup(groupItem: DeepnoteTreeItem): DeepnoteTreeItem[] { + const groupData = groupItem.data as ProjectGroupData; + const fileItems: DeepnoteTreeItem[] = []; + + for (const file of groupData.files) { + const initNotebookId = file.project.project.initNotebookId; + const nonInitNotebooks = file.project.project.notebooks?.filter((nb) => nb.id !== initNotebookId) ?? []; + const hasMultipleNotebooks = nonInitNotebooks.length > 1; + + const context: DeepnoteTreeItemContext = { + filePath: file.filePath, + projectId: groupData.projectId + }; + + const treeItem = new DeepnoteTreeItem( + DeepnoteTreeItemType.ProjectFile, + context, + file.project, + hasMultipleNotebooks ? TreeItemCollapsibleState.Collapsed : TreeItemCollapsibleState.None + ); + fileItems.push(treeItem); } - // Sort projects alphabetically by name (case-insensitive) - deepnoteFiles.sort(compareTreeItemsByLabel); + fileItems.sort(compareTreeItemsByLabel); - return deepnoteFiles; + return fileItems; } - private async getNotebooksForProject(projectItem: DeepnoteTreeItem): Promise { + /** + * Get notebook items for a project file (shown for multi-notebook files before splitting) + */ + private getNotebooksForProject(projectItem: DeepnoteTreeItem): DeepnoteTreeItem[] { const project = projectItem.data as DeepnoteProject; const notebooks = project.project.notebooks || []; - // Sort notebooks alphabetically by name (case-insensitive) - const sortedNotebooks = [...notebooks].sort((a, b) => { - const nameA = a.name || ''; - const nameB = b.name || ''; - return nameA.toLowerCase().localeCompare(nameB.toLowerCase()); - }); - - return sortedNotebooks.map((notebook: DeepnoteNotebook) => { + return notebooks.map((notebook: DeepnoteNotebook) => { const context: DeepnoteTreeItemContext = { filePath: projectItem.context.filePath, projectId: projectItem.context.projectId, @@ -312,7 +337,6 @@ export class DeepnoteTreeDataProvider implements TreeDataProvider { - const projectFiles = await this.getDeepnoteProjectFiles(); + public async findTreeItem(projectId: string): Promise { + const groups = await this.getProjectGroups(); - for (const projectItem of projectFiles) { - if (projectItem.context.projectId === projectId) { - if (!notebookId) { - return projectItem; - } - - const notebooks = await this.getNotebooksForProject(projectItem); - for (const notebookItem of notebooks) { - if (notebookItem.context.notebookId === notebookId) { - return notebookItem; - } - } + for (const item of groups) { + if (item.context.projectId === projectId) { + return item; } } diff --git a/src/notebooks/deepnote/deepnoteTreeDataProvider.unit.test.ts b/src/notebooks/deepnote/deepnoteTreeDataProvider.unit.test.ts index 4944422590..4c10e5408b 100644 --- a/src/notebooks/deepnote/deepnoteTreeDataProvider.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteTreeDataProvider.unit.test.ts @@ -379,8 +379,8 @@ suite('DeepnoteTreeDataProvider', () => { treeItemCache.set(cacheKey, mockTreeItem); // Verify initial state - assert.strictEqual(mockTreeItem.label, 'Original Name'); - assert.strictEqual(mockTreeItem.description, '1 notebook'); + assert.strictEqual(mockTreeItem.label, 'test-project.deepnote'); + assert.strictEqual(mockTreeItem.description, '0 cells'); // Update the project data (simulating rename and adding notebooks) const updatedProject: DeepnoteProject = { @@ -407,19 +407,20 @@ suite('DeepnoteTreeDataProvider', () => { mockTreeItem.updateVisualFields(); } else { // Manually update visual fields for testing purposes - mockTreeItem.label = updatedProject.project.name || 'Untitled Project'; + const fileName = mockTreeItem.context.filePath.split('/').pop() ?? ''; + mockTreeItem.label = fileName || updatedProject.project.name || 'Untitled Project'; mockTreeItem.tooltip = `Deepnote Project: ${updatedProject.project.name}\nFile: ${mockTreeItem.context.filePath}`; - const notebookCount = updatedProject.project.notebooks?.length || 0; - mockTreeItem.description = `${notebookCount} notebook${notebookCount !== 1 ? 's' : ''}`; + const blockCount = + updatedProject.project.notebooks?.reduce( + (sum: number, nb: any) => sum + (nb.blocks?.length ?? 0), + 0 + ) ?? 0; + mockTreeItem.description = `${blockCount} cell${blockCount !== 1 ? 's' : ''}`; } // Verify visual fields were updated - assert.strictEqual(mockTreeItem.label, 'Renamed Project', 'Label should reflect new project name'); - assert.strictEqual( - mockTreeItem.description, - '2 notebooks', - 'Description should reflect new notebook count' - ); + assert.strictEqual(mockTreeItem.label, 'test-project.deepnote', 'Label should reflect filename'); + assert.strictEqual(mockTreeItem.description, '0 cells', 'Description should reflect cell count'); assert.include( mockTreeItem.tooltip as string, 'Renamed Project', @@ -522,16 +523,16 @@ suite('DeepnoteTreeDataProvider', () => { ) ); - // Verify items are initially unsorted - assert.strictEqual(treeItems[0].label, 'Zebra Project'); + // Verify items are initially unsorted (label is filename derived from project name) + assert.strictEqual(treeItems[0].label, 'Zebra Project.deepnote'); // Sort using the exported comparator const sortedItems = [...treeItems].sort(compareTreeItemsByLabel); // Verify alphabetical order - assert.strictEqual(sortedItems[0].label, 'Apple Project'); - assert.strictEqual(sortedItems[1].label, 'Middle Project'); - assert.strictEqual(sortedItems[2].label, 'Zebra Project'); + assert.strictEqual(sortedItems[0].label, 'Apple Project.deepnote'); + assert.strictEqual(sortedItems[1].label, 'Middle Project.deepnote'); + assert.strictEqual(sortedItems[2].label, 'Zebra Project.deepnote'); }); test('should sort notebooks alphabetically by name within a project', async () => { @@ -584,11 +585,11 @@ suite('DeepnoteTreeDataProvider', () => { const notebookItems = await provider.getChildren(mockProjectItem); - // Verify notebooks are sorted alphabetically + // Verify notebooks are returned in original order assert.strictEqual(notebookItems.length, 3, 'Should have 3 notebooks'); - assert.strictEqual(notebookItems[0].label, 'Apple Notebook', 'First notebook should be Apple Notebook'); - assert.strictEqual(notebookItems[1].label, 'Middle Notebook', 'Second notebook should be Middle Notebook'); - assert.strictEqual(notebookItems[2].label, 'Zebra Notebook', 'Third notebook should be Zebra Notebook'); + assert.strictEqual(notebookItems[0].label, 'Zebra Notebook', 'First notebook should be Zebra Notebook'); + assert.strictEqual(notebookItems[1].label, 'Apple Notebook', 'Second notebook should be Apple Notebook'); + assert.strictEqual(notebookItems[2].label, 'Middle Notebook', 'Third notebook should be Middle Notebook'); }); test('should sort notebooks case-insensitively', async () => { @@ -641,11 +642,11 @@ suite('DeepnoteTreeDataProvider', () => { const notebookItems = await provider.getChildren(mockProjectItem); - // Verify case-insensitive sorting + // Verify notebooks are returned in original order assert.strictEqual(notebookItems.length, 3, 'Should have 3 notebooks'); - assert.strictEqual(notebookItems[0].label, 'Apple Notebook', 'First should be Apple Notebook'); - assert.strictEqual(notebookItems[1].label, 'MIDDLE Notebook', 'Second should be MIDDLE Notebook'); - assert.strictEqual(notebookItems[2].label, 'zebra notebook', 'Third should be zebra notebook'); + assert.strictEqual(notebookItems[0].label, 'zebra notebook', 'First should be zebra notebook'); + assert.strictEqual(notebookItems[1].label, 'Apple Notebook', 'Second should be Apple Notebook'); + assert.strictEqual(notebookItems[2].label, 'MIDDLE Notebook', 'Third should be MIDDLE Notebook'); }); }); }); diff --git a/src/notebooks/deepnote/deepnoteTreeItem.ts b/src/notebooks/deepnote/deepnoteTreeItem.ts index 0763f0e1bb..15577142da 100644 --- a/src/notebooks/deepnote/deepnoteTreeItem.ts +++ b/src/notebooks/deepnote/deepnoteTreeItem.ts @@ -1,10 +1,12 @@ -import { TreeItem, TreeItemCollapsibleState, Uri, ThemeIcon } from 'vscode'; +import { TreeItem, TreeItemCollapsibleState, ThemeIcon } from 'vscode'; + import type { DeepnoteProject, DeepnoteNotebook } from '../../platform/deepnote/deepnoteTypes'; /** * Represents different types of items in the Deepnote tree view */ export enum DeepnoteTreeItemType { + ProjectGroup = 'projectGroup', ProjectFile = 'projectFile', Notebook = 'notebook', Loading = 'loading' @@ -20,25 +22,39 @@ export interface DeepnoteTreeItemContext { } /** - * Tree item representing a Deepnote project file or notebook in the explorer view + * Data associated with a ProjectGroup tree item + */ +export interface ProjectGroupData { + readonly projectId: string; + readonly projectName: string; + readonly files: Array<{ filePath: string; project: DeepnoteProject }>; +} + +/** + * Tree item representing a Deepnote project group, file, or notebook in the explorer view */ export class DeepnoteTreeItem extends TreeItem { constructor( public readonly type: DeepnoteTreeItemType, public readonly context: DeepnoteTreeItemContext, - public data: DeepnoteProject | DeepnoteNotebook | null, + public data: DeepnoteProject | DeepnoteNotebook | ProjectGroupData | null, collapsibleState: TreeItemCollapsibleState ) { super('', collapsibleState); this.contextValue = this.type; - // Inline method calls to avoid ES module TreeItem extension issues if (this.type === DeepnoteTreeItemType.Loading) { this.label = 'Loading…'; this.tooltip = 'Loading…'; this.description = ''; this.iconPath = new ThemeIcon('loading~spin'); + } else if (this.type === DeepnoteTreeItemType.ProjectGroup) { + const groupData = this.data as ProjectGroupData; + this.label = groupData.projectName || 'Untitled Project'; + this.tooltip = `Deepnote Project: ${groupData.projectName}\n${groupData.files.length} file(s)`; + this.description = `${groupData.files.length} file${groupData.files.length !== 1 ? 's' : ''}`; + this.iconPath = new ThemeIcon('notebook'); } else { // getTooltip() inline if (this.type === DeepnoteTreeItemType.ProjectFile) { @@ -51,7 +67,7 @@ export class DeepnoteTreeItem extends TreeItem { // getIcon() inline if (this.type === DeepnoteTreeItemType.ProjectFile) { - this.iconPath = new ThemeIcon('notebook'); + this.iconPath = new ThemeIcon('file-code'); } else { this.iconPath = new ThemeIcon('file-code'); } @@ -59,7 +75,8 @@ export class DeepnoteTreeItem extends TreeItem { // getLabel() inline if (this.type === DeepnoteTreeItemType.ProjectFile) { const project = this.data as DeepnoteProject; - this.label = project.project.name || 'Untitled Project'; + const fileName = this.context.filePath.split('/').pop() ?? ''; + this.label = fileName || project.project.name || 'Untitled Project'; } else { const notebook = this.data as DeepnoteNotebook; this.label = notebook.name || 'Untitled Notebook'; @@ -68,8 +85,10 @@ export class DeepnoteTreeItem extends TreeItem { // getDescription() inline if (this.type === DeepnoteTreeItemType.ProjectFile) { const project = this.data as DeepnoteProject; - const notebookCount = project.project.notebooks?.length || 0; - this.description = `${notebookCount} notebook${notebookCount !== 1 ? 's' : ''}`; + const initNotebookId = project.project.initNotebookId; + const nonInitNotebooks = project.project.notebooks?.filter((nb) => nb.id !== initNotebookId) ?? []; + const blockCount = nonInitNotebooks.reduce((sum, nb) => sum + (nb.blocks?.length ?? 0), 0); + this.description = `${blockCount} cell${blockCount !== 1 ? 's' : ''}`; } else { const notebook = this.data as DeepnoteNotebook; const blockCount = notebook.blocks?.length || 0; @@ -77,11 +96,17 @@ export class DeepnoteTreeItem extends TreeItem { } } + // ProjectFile items open the file directly (no query param) + if (this.type === DeepnoteTreeItemType.ProjectFile) { + this.command = { + command: 'deepnote.openNotebook', + title: 'Open Notebook', + arguments: [this.context] + }; + } + + // Notebook items also open the file directly (no query param) if (this.type === DeepnoteTreeItemType.Notebook) { - // getNotebookUri() inline - if (this.context.notebookId) { - this.resourceUri = Uri.parse(`deepnote-notebook://${this.context.filePath}#${this.context.notebookId}`); - } this.command = { command: 'deepnote.openNotebook', title: 'Open Notebook', @@ -103,15 +128,24 @@ export class DeepnoteTreeItem extends TreeItem { return; } + if (this.type === DeepnoteTreeItemType.ProjectGroup) { + const groupData = this.data as ProjectGroupData; + this.label = groupData.projectName || 'Untitled Project'; + this.tooltip = `Deepnote Project: ${groupData.projectName}\n${groupData.files.length} file(s)`; + this.description = `${groupData.files.length} file${groupData.files.length !== 1 ? 's' : ''}`; + return; + } + if (this.type === DeepnoteTreeItemType.ProjectFile) { const project = this.data as DeepnoteProject; - - this.label = project.project.name || 'Untitled Project'; + const fileName = this.context.filePath.split('/').pop() ?? ''; + this.label = fileName || project.project.name || 'Untitled Project'; this.tooltip = `Deepnote Project: ${project.project.name}\nFile: ${this.context.filePath}`; - const notebookCount = project.project.notebooks?.length || 0; - - this.description = `${notebookCount} notebook${notebookCount !== 1 ? 's' : ''}`; + const initNotebookId = project.project.initNotebookId; + const nonInitNotebooks = project.project.notebooks?.filter((nb) => nb.id !== initNotebookId) ?? []; + const blockCount = nonInitNotebooks.reduce((sum, nb) => sum + (nb.blocks?.length ?? 0), 0); + this.description = `${blockCount} cell${blockCount !== 1 ? 's' : ''}`; } else { const notebook = this.data as DeepnoteNotebook; diff --git a/src/notebooks/deepnote/deepnoteTreeItem.unit.test.ts b/src/notebooks/deepnote/deepnoteTreeItem.unit.test.ts index bbadeceee4..cbc2a99bf3 100644 --- a/src/notebooks/deepnote/deepnoteTreeItem.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteTreeItem.unit.test.ts @@ -61,8 +61,8 @@ suite('DeepnoteTreeItem', () => { assert.strictEqual(item.type, DeepnoteTreeItemType.ProjectFile); assert.deepStrictEqual(item.context, context); assert.strictEqual(item.collapsibleState, TreeItemCollapsibleState.Collapsed); - assert.strictEqual(item.label, 'Test Project'); - assert.strictEqual(item.description, '1 notebook'); + assert.strictEqual(item.label, 'project.deepnote'); + assert.strictEqual(item.description, '0 cells'); }); test('should create notebook item with basic properties', () => { @@ -117,19 +117,20 @@ suite('DeepnoteTreeItem', () => { TreeItemCollapsibleState.Collapsed ); - assert.strictEqual(item.label, 'Test Project'); + assert.strictEqual(item.label, 'my-project.deepnote'); assert.strictEqual(item.type, DeepnoteTreeItemType.ProjectFile); assert.strictEqual(item.collapsibleState, TreeItemCollapsibleState.Collapsed); assert.strictEqual(item.contextValue, 'projectFile'); assert.strictEqual(item.tooltip, 'Deepnote Project: Test Project\nFile: /workspace/my-project.deepnote'); - assert.strictEqual(item.description, '1 notebook'); + assert.strictEqual(item.description, '0 cells'); - // Should have notebook icon for project files + // Should have file-code icon for project files assert.instanceOf(item.iconPath, ThemeIcon); - assert.strictEqual((item.iconPath as ThemeIcon).id, 'notebook'); + assert.strictEqual((item.iconPath as ThemeIcon).id, 'file-code'); - // Should not have command for project files - assert.isUndefined(item.command); + // Should have command for project files + assert.isDefined(item.command); + assert.strictEqual(item.command!.command, 'deepnote.openNotebook'); }); test('should handle project with multiple notebooks', () => { @@ -175,7 +176,7 @@ suite('DeepnoteTreeItem', () => { TreeItemCollapsibleState.Collapsed ); - assert.strictEqual(item.description, '3 notebooks'); + assert.strictEqual(item.description, '0 cells'); }); test('should handle project with no notebooks', () => { @@ -199,7 +200,7 @@ suite('DeepnoteTreeItem', () => { TreeItemCollapsibleState.Collapsed ); - assert.strictEqual(item.description, '0 notebooks'); + assert.strictEqual(item.description, '0 cells'); }); test('should handle unnamed project', () => { @@ -223,7 +224,7 @@ suite('DeepnoteTreeItem', () => { TreeItemCollapsibleState.Collapsed ); - assert.strictEqual(item.label, 'Untitled Project'); + assert.strictEqual(item.label, 'project.deepnote'); }); }); @@ -259,12 +260,8 @@ suite('DeepnoteTreeItem', () => { assert.strictEqual(item.command!.title, 'Open Notebook'); assert.deepStrictEqual(item.command!.arguments, [context]); - // Should have resource URI - assert.isDefined(item.resourceUri); - assert.strictEqual( - item.resourceUri!.toString(), - 'deepnote-notebook:/workspace/project.deepnote#notebook-789' - ); + // Should not have resource URI + assert.isUndefined(item.resourceUri); }); test('should handle notebook with multiple blocks', () => { @@ -418,7 +415,7 @@ suite('DeepnoteTreeItem', () => { }); suite('command configuration', () => { - test('should not create command for project files', () => { + test('should create command for project files', () => { const context: DeepnoteTreeItemContext = { filePath: '/test/project.deepnote', projectId: 'project-123' @@ -431,7 +428,10 @@ suite('DeepnoteTreeItem', () => { TreeItemCollapsibleState.Collapsed ); - assert.isUndefined(item.command); + assert.isDefined(item.command); + assert.strictEqual(item.command!.command, 'deepnote.openNotebook'); + assert.strictEqual(item.command!.title, 'Open Notebook'); + assert.deepStrictEqual(item.command!.arguments, [context]); }); test('should create correct command for notebooks', () => { @@ -457,7 +457,7 @@ suite('DeepnoteTreeItem', () => { }); suite('icon configuration', () => { - test('should use notebook icon for project files', () => { + test('should use file-code icon for project files', () => { const context: DeepnoteTreeItemContext = { filePath: '/test/project.deepnote', projectId: 'project-123' @@ -471,7 +471,7 @@ suite('DeepnoteTreeItem', () => { ); assert.instanceOf(item.iconPath, ThemeIcon); - assert.strictEqual((item.iconPath as ThemeIcon).id, 'notebook'); + assert.strictEqual((item.iconPath as ThemeIcon).id, 'file-code'); }); test('should use file-code icon for notebooks', () => { @@ -746,7 +746,8 @@ suite('DeepnoteTreeItem', () => { items.forEach((item, index) => { assert.strictEqual(item.context.filePath, contexts[index].filePath); assert.strictEqual(item.context.projectId, contexts[index].projectId); - assert.isUndefined(item.command); // Project files don't have commands + assert.isDefined(item.command); // Project files have commands + assert.strictEqual(item.command!.command, 'deepnote.openNotebook'); }); }); }); diff --git a/src/notebooks/deepnote/snapshots/snapshotFiles.ts b/src/notebooks/deepnote/snapshots/snapshotFiles.ts index 6f6ff9f5fd..109411da9f 100644 --- a/src/notebooks/deepnote/snapshots/snapshotFiles.ts +++ b/src/notebooks/deepnote/snapshots/snapshotFiles.ts @@ -5,8 +5,21 @@ import { InvalidProjectNameError } from '../../../platform/errors/invalidProject /** File suffix for snapshot files */ export const SNAPSHOT_FILE_SUFFIX = '.snapshot.deepnote'; -/** Regex pattern for extracting project ID from snapshot filenames. */ -const SNAPSHOT_FILENAME_PATTERN = new RegExp(`^[a-z0-9-]+_(.+)_[^_]+${SNAPSHOT_FILE_SUFFIX.replace(/\./g, '\\.')}$`); +/** + * Regex pattern for extracting project ID and notebook ID from snapshot filenames. + * New format: {slug}_{projectId}_{notebookId}_{variant}.snapshot.deepnote + */ +const SNAPSHOT_FILENAME_PATTERN = new RegExp( + `^[a-z0-9-]+_(.+)_([a-f0-9-]+)_[^_]+${SNAPSHOT_FILE_SUFFIX.replace(/\./g, '\\.')}$` +); + +/** + * Legacy pattern for old-format snapshots without notebook ID. + * Old format: {slug}_{projectId}_{variant}.snapshot.deepnote + */ +const LEGACY_SNAPSHOT_FILENAME_PATTERN = new RegExp( + `^[a-z0-9-]+_(.+)_[^_]+${SNAPSHOT_FILE_SUFFIX.replace(/\./g, '\\.')}$` +); /** * Checks if a URI represents a snapshot file @@ -17,14 +30,34 @@ export function isSnapshotFile(uri: Uri): boolean { /** * Extracts the project ID from a snapshot file URI. - * Snapshot filenames follow: `${slug}_${projectId}_${variant}.snapshot.deepnote` + * Supports both new format ({slug}_{projectId}_{notebookId}_{variant}) and + * legacy format ({slug}_{projectId}_{variant}). * @returns The project ID, or undefined if the URI is not a valid snapshot file */ export function extractProjectIdFromSnapshotUri(uri: Uri): string | undefined { const basename = uri.path.split('/').pop() ?? ''; + + // Try new format first + const newMatch = basename.match(SNAPSHOT_FILENAME_PATTERN); + if (newMatch) { + return newMatch[1]; + } + + // Fall back to legacy format + const legacyMatch = basename.match(LEGACY_SNAPSHOT_FILENAME_PATTERN); + return legacyMatch?.[1]; +} + +/** + * Extracts the notebook ID from a snapshot file URI. + * Only works with new format: {slug}_{projectId}_{notebookId}_{variant}.snapshot.deepnote + * @returns The notebook ID, or undefined if the URI uses legacy format or is invalid + */ +export function extractNotebookIdFromSnapshotUri(uri: Uri): string | undefined { + const basename = uri.path.split('/').pop() ?? ''; const match = basename.match(SNAPSHOT_FILENAME_PATTERN); - return match?.[1]; + return match?.[2]; } /** diff --git a/src/notebooks/deepnote/snapshots/snapshotService.ts b/src/notebooks/deepnote/snapshots/snapshotService.ts index 6e4b99e96e..4ed8095628 100644 --- a/src/notebooks/deepnote/snapshots/snapshotService.ts +++ b/src/notebooks/deepnote/snapshots/snapshotService.ts @@ -202,9 +202,17 @@ export class SnapshotService implements ISnapshotMetadataService, IExtensionSync projectId: string, projectName: string, projectData: DeepnoteFile, - notebookUri?: string + notebookUri?: string, + notebookId?: string ): Promise { - const prepared = await this.prepareSnapshotData(projectUri, projectId, projectName, projectData, notebookUri); + const prepared = await this.prepareSnapshotData( + projectUri, + projectId, + projectName, + projectData, + notebookUri, + notebookId + ); if (!prepared) { logger.debug(`[Snapshot] No changes detected, skipping snapshot creation`); @@ -214,7 +222,7 @@ export class SnapshotService implements ISnapshotMetadataService, IExtensionSync const { latestPath, content } = prepared; const timestamp = generateTimestamp(); - const timestampedPath = this.buildSnapshotPath(projectUri, projectId, projectName, timestamp); + const timestampedPath = this.buildSnapshotPath(projectUri, projectId, projectName, timestamp, notebookId); // Write to timestamped file first (safe - doesn't touch existing files) try { @@ -390,8 +398,8 @@ export class SnapshotService implements ISnapshotMetadataService, IExtensionSync }; } - async readSnapshot(projectId: string): Promise | undefined> { - logger.debug(`[Snapshot] readSnapshot called for projectId=${projectId}`); + async readSnapshot(projectId: string, notebookId?: string): Promise | undefined> { + logger.debug(`[Snapshot] readSnapshot called for projectId=${projectId}, notebookId=${notebookId}`); const workspaceFolders = workspace.workspaceFolders; if (!workspaceFolders || workspaceFolders.length === 0) { @@ -404,8 +412,10 @@ export class SnapshotService implements ISnapshotMetadataService, IExtensionSync logger.debug(`[Snapshot] Searching ${workspaceFolders.length} workspace folder(s) for snapshots`); - // 1. Try to find a 'latest' snapshot file - const latestGlob = `**/snapshots/*_${projectId}_latest.snapshot.deepnote`; + // 1. Try to find a 'latest' snapshot file (new format with notebookId first, then legacy) + const latestGlob = notebookId + ? `**/snapshots/*_${projectId}_${notebookId}_latest.snapshot.deepnote` + : `**/snapshots/*_${projectId}_latest.snapshot.deepnote`; for (const folder of workspaceFolders) { logger.debug(`[Snapshot] Searching for latest snapshot with glob: ${latestGlob} in ${folder.uri.path}`); @@ -430,7 +440,9 @@ export class SnapshotService implements ISnapshotMetadataService, IExtensionSync logger.debug(`[Snapshot] No latest snapshot found, looking for timestamped files`); // 2. Find timestamped snapshots across all workspace folders - const timestampedGlob = `**/snapshots/*_${projectId}_*.snapshot.deepnote`; + const timestampedGlob = notebookId + ? `**/snapshots/*_${projectId}_${notebookId}_*.snapshot.deepnote` + : `**/snapshots/*_${projectId}_*.snapshot.deepnote`; let allTimestampedFiles: Uri[] = []; for (const folder of workspaceFolders) { @@ -494,11 +506,14 @@ export class SnapshotService implements ISnapshotMetadataService, IExtensionSync projectUri: Uri, projectId: string, projectName: string, - variant: 'latest' | string + variant: 'latest' | string, + notebookId?: string ): Uri { const parentDir = Uri.joinPath(projectUri, '..'); const slug = slugifyProjectName(projectName); - const filename = `${slug}_${projectId}_${variant}.snapshot.deepnote`; + const filename = notebookId + ? `${slug}_${projectId}_${notebookId}_${variant}.snapshot.deepnote` + : `${slug}_${projectId}_${variant}.snapshot.deepnote`; return Uri.joinPath(parentDir, 'snapshots', filename); } @@ -800,7 +815,8 @@ export class SnapshotService implements ISnapshotMetadataService, IExtensionSync projectId, originalProject.project.name, snapshotProject, - notebookUri + notebookUri, + notebookId ); if (snapshotUri) { @@ -814,7 +830,8 @@ export class SnapshotService implements ISnapshotMetadataService, IExtensionSync projectId, originalProject.project.name, snapshotProject, - notebookUri + notebookUri, + notebookId ); if (snapshotUri) { @@ -866,12 +883,13 @@ export class SnapshotService implements ISnapshotMetadataService, IExtensionSync projectId: string, projectName: string, projectData: DeepnoteFile, - notebookUri?: string + notebookUri?: string, + notebookId?: string ): Promise<{ latestPath: Uri; content: Uint8Array } | undefined> { let latestPath: Uri; try { - latestPath = this.buildSnapshotPath(projectUri, projectId, projectName, 'latest'); + latestPath = this.buildSnapshotPath(projectUri, projectId, projectName, 'latest', notebookId); } catch (error) { if (error instanceof InvalidProjectNameError) { logger.warn('[Snapshot] Skipping snapshots due to invalid project name', error); @@ -1011,9 +1029,17 @@ export class SnapshotService implements ISnapshotMetadataService, IExtensionSync projectId: string, projectName: string, projectData: DeepnoteFile, - notebookUri?: string + notebookUri?: string, + notebookId?: string ): Promise { - const prepared = await this.prepareSnapshotData(projectUri, projectId, projectName, projectData, notebookUri); + const prepared = await this.prepareSnapshotData( + projectUri, + projectId, + projectName, + projectData, + notebookUri, + notebookId + ); if (!prepared) { logger.debug(`[Snapshot] No changes detected, skipping latest snapshot update`); 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..de3c3e664b 100644 --- a/src/notebooks/types.ts +++ b/src/notebooks/types.ts @@ -37,12 +37,11 @@ export interface ProjectIntegration { export const IDeepnoteNotebookManager = Symbol('IDeepnoteNotebookManager'); export interface IDeepnoteNotebookManager { - getCurrentNotebookId(projectId: string): string | undefined; getOriginalProject(projectId: string): DeepnoteProject | undefined; - getTheSelectedNotebookForAProject(projectId: string): string | undefined; - selectNotebookForProject(projectId: string, notebookId: string): void; - storeOriginalProject(projectId: string, project: DeepnoteProject, notebookId: string): void; - updateCurrentNotebookId(projectId: string, notebookId: string): void; + hasInitNotebookBeenRun(projectId: string): boolean; + markInitNotebookAsRun(projectId: string): void; + storeOriginalProject(projectId: string, project: DeepnoteProject): void; + updateOriginalProject(projectId: string, project: DeepnoteProject): void; /** * Updates the integrations list in the project data. @@ -53,7 +52,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; }