From 56ae05462708fe282e8940f6366714439a2e6a53 Mon Sep 17 00:00:00 2001 From: Christoffer Artmann Date: Tue, 10 Feb 2026 10:44:15 +0100 Subject: [PATCH 1/6] fix: Detect external changes to notebooks Fixes #315 --- .../deepnote/deepnoteFileChangeWatcher.ts | 153 ++++++++ .../deepnoteFileChangeWatcher.unit.test.ts | 343 ++++++++++++++++++ src/notebooks/serviceRegistry.node.ts | 7 + src/notebooks/serviceRegistry.web.ts | 5 + 4 files changed, 508 insertions(+) create mode 100644 src/notebooks/deepnote/deepnoteFileChangeWatcher.ts create mode 100644 src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts diff --git a/src/notebooks/deepnote/deepnoteFileChangeWatcher.ts b/src/notebooks/deepnote/deepnoteFileChangeWatcher.ts new file mode 100644 index 000000000..e08c276cf --- /dev/null +++ b/src/notebooks/deepnote/deepnoteFileChangeWatcher.ts @@ -0,0 +1,153 @@ +import { + CancellationTokenSource, + NotebookCellData, + NotebookCellOutput, + NotebookDocument, + NotebookEdit, + NotebookRange, + Uri, + WorkspaceEdit, + workspace +} from 'vscode'; +import { inject, injectable, optional } from 'inversify'; + +import { IExtensionSyncActivationService } from '../../platform/activation/types'; +import { IDisposableRegistry } from '../../platform/common/types'; +import { logger } from '../../platform/logging'; +import { IDeepnoteNotebookManager } from '../types'; +import { DeepnoteNotebookSerializer } from './deepnoteSerializer'; +import { isSnapshotFile } from './snapshots/snapshotFiles'; +import { SnapshotService } from './snapshots/snapshotService'; + +const DEBOUNCE_MS = 500; + +/** + * Watches .deepnote files for external changes and reloads open notebook editors. + * + * When AI agents (Cursor, Claude Code) modify a .deepnote file on disk, + * VS Code's NotebookSerializer does not reliably detect and reload the notebook. + * This service bridges that gap by watching the filesystem and applying edits + * to open notebook documents when their underlying files change externally. + */ +@injectable() +export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationService { + private readonly serializer: DeepnoteNotebookSerializer; + private readonly debounceTimers = new Map>(); + + constructor( + @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, + @inject(IDeepnoteNotebookManager) private readonly notebookManager: IDeepnoteNotebookManager, + @inject(SnapshotService) @optional() private readonly snapshotService?: SnapshotService + ) { + this.serializer = new DeepnoteNotebookSerializer(this.notebookManager, this.snapshotService); + } + + public activate(): void { + const watcher = workspace.createFileSystemWatcher('**/*.deepnote'); + + this.disposables.push(watcher); + this.disposables.push(watcher.onDidChange((uri) => this.handleFileChange(uri))); + } + + private handleFileChange(uri: Uri): void { + if (isSnapshotFile(uri)) { + return; + } + + const key = uri.toString(); + + const existing = this.debounceTimers.get(key); + if (existing) { + clearTimeout(existing); + } + + this.debounceTimers.set( + key, + setTimeout(() => { + this.debounceTimers.delete(key); + void this.reloadNotebooksForFile(uri); + }, DEBOUNCE_MS) + ); + } + + private cellsMatchNotebook(notebook: NotebookDocument, newCells: NotebookCellData[]): boolean { + const liveCells = notebook.getCells(); + if (liveCells.length !== newCells.length) { + return false; + } + return liveCells.every( + (live, i) => live.document.getText() === newCells[i].value && live.kind === newCells[i].kind + ); + } + + private async reloadNotebooksForFile(uri: Uri): Promise { + const uriString = uri.toString(); + const affectedNotebooks = workspace.notebookDocuments.filter( + (doc) => + doc.notebookType === 'deepnote' && doc.uri.with({ query: '', fragment: '' }).toString() === uriString + ); + + if (affectedNotebooks.length === 0) { + return; + } + + let content: Uint8Array; + try { + content = await workspace.fs.readFile(uri); + } catch (error) { + logger.warn(`[FileChangeWatcher] Failed to read changed file: ${uri.path}`, error); + return; + } + + for (const notebook of affectedNotebooks) { + try { + const tokenSource = new CancellationTokenSource(); + const newData = await this.serializer.deserializeNotebook(content, tokenSource.token); + tokenSource.dispose(); + + if (this.cellsMatchNotebook(notebook, newData.cells)) { + return; + } + + // Preserve outputs from live cells that the deserialized data may lack. + // In snapshot mode the main file has outputs stripped; AI agents + // typically don't preserve outputs when editing code. + const liveCells = notebook.getCells(); + const liveOutputsByBlockId = new Map(); + for (const liveCell of liveCells) { + const blockId = (liveCell.metadata?.id ?? liveCell.metadata?.__deepnoteBlockId) as + | string + | undefined; + if (blockId && liveCell.outputs.length > 0) { + liveOutputsByBlockId.set(blockId, liveCell.outputs); + } + } + + for (const cell of newData.cells) { + const blockId = (cell.metadata?.id ?? cell.metadata?.__deepnoteBlockId) as string | undefined; + if (blockId && (!cell.outputs || cell.outputs.length === 0)) { + const liveOutputs = liveOutputsByBlockId.get(blockId); + if (liveOutputs) { + cell.outputs = [...liveOutputs]; + } + } + } + + const edit = new WorkspaceEdit(); + edit.set(notebook.uri, [ + NotebookEdit.replaceCells(new NotebookRange(0, notebook.cellCount), newData.cells) + ]); + await workspace.applyEdit(edit); + + // Save immediately so VS Code updates its internal mtime for the file. + // Without this, the user gets a "content is newer" conflict dialog on + // their next manual save because VS Code still remembers the old mtime. + await workspace.save(notebook.uri); + + logger.info(`[FileChangeWatcher] Reloaded notebook from external change: ${notebook.uri.path}`); + } catch (error) { + logger.error(`[FileChangeWatcher] Failed to reload notebook: ${notebook.uri.path}`, error); + } + } + } +} diff --git a/src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts b/src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts new file mode 100644 index 000000000..343b89357 --- /dev/null +++ b/src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts @@ -0,0 +1,343 @@ +import { assert } from 'chai'; +import * as sinon from 'sinon'; +import { anything, instance, mock, verify, when } from 'ts-mockito'; +import { EventEmitter, FileSystemWatcher, NotebookCellKind, NotebookDocument, Uri } from 'vscode'; + +import type { IDisposableRegistry } from '../../platform/common/types'; +import { mockedVSCodeNamespaces, resetVSCodeMocks } from '../../test/vscode-mock'; +import { IDeepnoteNotebookManager } from '../types'; +import { DeepnoteFileChangeWatcher } from './deepnoteFileChangeWatcher'; + +/** + * Polls until a condition is met or a timeout is reached. + */ +async function waitFor(condition: () => boolean, timeoutMs = 5000, intervalMs = 50): Promise { + const start = Date.now(); + while (!condition()) { + if (Date.now() - start > timeoutMs) { + throw new Error(`waitFor timed out after ${timeoutMs}ms`); + } + await new Promise((resolve) => setTimeout(resolve, intervalMs)); + } +} + +suite('DeepnoteFileChangeWatcher', () => { + let watcher: DeepnoteFileChangeWatcher; + let mockDisposables: IDisposableRegistry; + let mockNotebookManager: IDeepnoteNotebookManager; + let onDidChangeFile: EventEmitter; + let readFileCalls: number; + + setup(() => { + resetVSCodeMocks(); + readFileCalls = 0; + + mockDisposables = []; + mockNotebookManager = instance(mock()); + + // Set up FileSystemWatcher mock + onDidChangeFile = new EventEmitter(); + const fsWatcher = mock(); + when(fsWatcher.onDidChange).thenReturn(onDidChangeFile.event); + when(fsWatcher.dispose()).thenReturn(); + + when(mockedVSCodeNamespaces.workspace.createFileSystemWatcher(anything())).thenReturn(instance(fsWatcher)); + + when(mockedVSCodeNamespaces.workspace.save(anything())).thenReturn( + Promise.resolve(Uri.file('/workspace/test.deepnote')) + ); + + watcher = new DeepnoteFileChangeWatcher(mockDisposables, mockNotebookManager); + watcher.activate(); + }); + + teardown(() => { + sinon.restore(); + onDidChangeFile.dispose(); + }); + + function createMockNotebook(opts: { + uri: Uri; + isDirty?: boolean; + notebookType?: string; + cellCount?: number; + metadata?: Record; + cells?: Array<{ + metadata?: Record; + outputs: any[]; + kind?: number; + document?: { getText: () => string }; + }>; + }): NotebookDocument { + const cells = (opts.cells ?? []).map((c) => ({ + ...c, + kind: c.kind ?? NotebookCellKind.Code, + document: c.document ?? { getText: () => '' } + })); + return { + uri: opts.uri, + isDirty: opts.isDirty ?? false, + notebookType: opts.notebookType ?? 'deepnote', + cellCount: opts.cellCount ?? (cells.length || 1), + metadata: opts.metadata ?? { + deepnoteProjectId: 'project-1', + deepnoteNotebookId: 'notebook-1' + }, + getCells: () => cells + } as unknown as NotebookDocument; + } + + function setupMockFs(yamlContent: string) { + const mockFs = mock(); + when(mockFs.readFile(anything())).thenCall(() => { + readFileCalls++; + return Promise.resolve(new TextEncoder().encode(yamlContent)); + }); + when(mockedVSCodeNamespaces.workspace.fs).thenReturn(instance(mockFs)); + return mockFs; + } + + const validYaml = ` +version: '1.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 + content: print("hello") +`; + + const emptyBlocksYaml = ` +version: '1.0' +metadata: + createdAt: '2025-01-01T00:00:00Z' +project: + id: project-1 + name: Test Project + notebooks: + - id: notebook-1 + name: Notebook 1 + blocks: [] +`; + + test('should skip reload when content matches notebook cells', async () => { + const uri = Uri.file('/workspace/test.deepnote'); + // Create a notebook whose cell content already matches validYaml + const notebook = createMockNotebook({ + uri, + cells: [ + { + metadata: { id: 'block-1' }, + outputs: [], + kind: NotebookCellKind.Code, + document: { getText: () => 'print("hello")' } + } + ] + }); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + setupMockFs(validYaml); + + onDidChangeFile.fire(uri); + + // Wait for debounce + deserialization + await waitFor(() => readFileCalls > 0); + await new Promise((resolve) => setTimeout(resolve, 200)); + + // File was read, but applyEdit should NOT be called because cells match + assert.isAtLeast(readFileCalls, 1, 'readFile should be called'); + verify(mockedVSCodeNamespaces.workspace.applyEdit(anything())).never(); + }); + + test('should reload on external change', async () => { + const uri = Uri.file('/workspace/test.deepnote'); + const notebook = createMockNotebook({ uri, cellCount: 0 }); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + setupMockFs(validYaml); + + // Fire file change without a preceding save + onDidChangeFile.fire(uri); + + // Wait for the debounce + async deserialization to complete + await waitFor(() => readFileCalls > 0); + + // Wait a bit more for the async chain to finish (deserialize + applyEdit + save) + await new Promise((resolve) => setTimeout(resolve, 200)); + + verify(mockedVSCodeNamespaces.workspace.applyEdit(anything())).atLeast(1); + // After applyEdit, the document should be saved to update VS Code's mtime + verify(mockedVSCodeNamespaces.workspace.save(anything())).atLeast(1); + }); + + test('should skip snapshot files', async () => { + const snapshotUri = Uri.file('/workspace/snapshots/project_abc_latest.snapshot.deepnote'); + setupMockFs(validYaml); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([]); + + onDidChangeFile.fire(snapshotUri); + + // Wait well past debounce + await new Promise((resolve) => setTimeout(resolve, 800)); + + // Should not attempt to read the file at all + assert.strictEqual(readFileCalls, 0, 'readFile should not be called for snapshot files'); + }); + + test('should reload dirty notebooks', async () => { + const uri = Uri.file('/workspace/test.deepnote'); + const notebook = createMockNotebook({ uri, isDirty: true }); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + setupMockFs(validYaml); + + onDidChangeFile.fire(uri); + + // Wait for debounce + readFile to happen + await waitFor(() => readFileCalls > 0); + await new Promise((resolve) => setTimeout(resolve, 200)); + + // Dirty notebooks should now be reloaded and saved to prevent mtime conflicts + assert.isAtLeast(readFileCalls, 1, 'readFile should be called'); + verify(mockedVSCodeNamespaces.workspace.applyEdit(anything())).atLeast(1); + verify(mockedVSCodeNamespaces.workspace.save(anything())).atLeast(1); + }); + + test('should debounce rapid changes', async () => { + const uri = Uri.file('/workspace/test.deepnote'); + const notebook = createMockNotebook({ uri, cellCount: 0 }); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + setupMockFs(emptyBlocksYaml); + + // Fire multiple changes rapidly + onDidChangeFile.fire(uri); + await new Promise((resolve) => setTimeout(resolve, 100)); + onDidChangeFile.fire(uri); + await new Promise((resolve) => setTimeout(resolve, 100)); + onDidChangeFile.fire(uri); + + // Wait for debounce from the last event + processing + await waitFor(() => readFileCalls > 0); + await new Promise((resolve) => setTimeout(resolve, 200)); + + // readFile should only be called once (debounced) + assert.strictEqual(readFileCalls, 1, 'readFile should be called exactly once after debounce'); + }); + + test('should handle parse errors gracefully', async () => { + const uri = Uri.file('/workspace/test.deepnote'); + const notebook = createMockNotebook({ uri, cellCount: 0 }); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + setupMockFs('this is: [invalid: yaml: content'); + + onDidChangeFile.fire(uri); + + // Wait for debounce + processing + await waitFor(() => readFileCalls > 0); + await new Promise((resolve) => setTimeout(resolve, 200)); + + // Should not throw - errors are caught and logged + assert.ok(true, 'Parse error was handled gracefully'); + }); + + test('should preserve live cell outputs during reload', async () => { + const uri = Uri.file('/workspace/test.deepnote'); + const fakeOutput = { items: [{ mime: 'text/plain', data: new Uint8Array([72]) }] }; + const notebook = createMockNotebook({ + uri, + cells: [ + { + metadata: { id: 'block-1' }, + outputs: [fakeOutput] + } + ] + }); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + setupMockFs(validYaml); + + onDidChangeFile.fire(uri); + + await waitFor(() => readFileCalls > 0); + await new Promise((resolve) => setTimeout(resolve, 200)); + + // applyEdit should be called — the output preservation runs before it + verify(mockedVSCodeNamespaces.workspace.applyEdit(anything())).atLeast(1); + }); + + test('should reload dirty notebooks and preserve outputs', async () => { + const uri = Uri.file('/workspace/test.deepnote'); + const fakeOutput = { items: [{ mime: 'text/plain', data: new Uint8Array([72]) }] }; + const notebook = createMockNotebook({ + uri, + isDirty: true, + cells: [ + { + metadata: { id: 'block-1' }, + outputs: [fakeOutput] + } + ] + }); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + setupMockFs(validYaml); + + onDidChangeFile.fire(uri); + + await waitFor(() => readFileCalls > 0); + await new Promise((resolve) => setTimeout(resolve, 200)); + + // Dirty notebook should still be reloaded with outputs preserved + verify(mockedVSCodeNamespaces.workspace.applyEdit(anything())).atLeast(1); + }); + + test('should not suppress real changes after auto-save', async () => { + const uri = Uri.file('/workspace/test.deepnote'); + let applyEditCount = 0; + when(mockedVSCodeNamespaces.workspace.applyEdit(anything())).thenCall(() => { + applyEditCount++; + return Promise.resolve(true); + }); + + // 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]); + setupMockFs(validYaml); + + onDidChangeFile.fire(uri); + await waitFor(() => applyEditCount >= 1); + + // Second change: use different YAML content + const changedYaml = ` +version: '1.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 + content: print("world") +`; + setupMockFs(changedYaml); + onDidChangeFile.fire(uri); + await waitFor(() => applyEditCount >= 2, 5000); + + assert.isAtLeast(applyEditCount, 2, 'applyEdit should be called for both external changes'); + }); +}); diff --git a/src/notebooks/serviceRegistry.node.ts b/src/notebooks/serviceRegistry.node.ts index e61383207..991f5b4fe 100644 --- a/src/notebooks/serviceRegistry.node.ts +++ b/src/notebooks/serviceRegistry.node.ts @@ -92,6 +92,7 @@ import { OpenInDeepnoteHandler } from './deepnote/openInDeepnoteHandler.node'; import { IntegrationKernelRestartHandler } from './deepnote/integrations/integrationKernelRestartHandler'; import { ISnapshotMetadataService, SnapshotService } from './deepnote/snapshots/snapshotService'; import { EnvironmentCapture, IEnvironmentCapture } from './deepnote/snapshots/environmentCapture.node'; +import { DeepnoteFileChangeWatcher } from './deepnote/deepnoteFileChangeWatcher'; export function registerTypes(serviceManager: IServiceManager, isDevMode: boolean) { registerControllerTypes(serviceManager, isDevMode); @@ -258,6 +259,12 @@ export function registerTypes(serviceManager: IServiceManager, isDevMode: boolea serviceManager.addBinding(SnapshotService, IExtensionSyncActivationService); serviceManager.addBinding(SnapshotService, ISnapshotMetadataService); + // File change watcher for external edits + serviceManager.addSingleton( + IExtensionSyncActivationService, + DeepnoteFileChangeWatcher + ); + // File export/import serviceManager.addSingleton(IFileConverter, FileConverter); serviceManager.addSingleton(ExportInterpreterFinder, ExportInterpreterFinder); diff --git a/src/notebooks/serviceRegistry.web.ts b/src/notebooks/serviceRegistry.web.ts index 708e65787..2488ff73d 100644 --- a/src/notebooks/serviceRegistry.web.ts +++ b/src/notebooks/serviceRegistry.web.ts @@ -54,6 +54,7 @@ import { DeepnoteBigNumberCellStatusBarProvider } from './deepnote/deepnoteBigNu import { DeepnoteNewCellLanguageService } from './deepnote/deepnoteNewCellLanguageService'; import { SqlCellStatusBarProvider } from './deepnote/sqlCellStatusBarProvider'; import { IntegrationKernelRestartHandler } from './deepnote/integrations/integrationKernelRestartHandler'; +import { DeepnoteFileChangeWatcher } from './deepnote/deepnoteFileChangeWatcher'; export function registerTypes(serviceManager: IServiceManager, isDevMode: boolean) { registerControllerTypes(serviceManager, isDevMode); @@ -136,6 +137,10 @@ export function registerTypes(serviceManager: IServiceManager, isDevMode: boolea IExtensionSyncActivationService, IntegrationKernelRestartHandler ); + serviceManager.addSingleton( + IExtensionSyncActivationService, + DeepnoteFileChangeWatcher + ); serviceManager.addSingleton(IExportBase, ExportBase); serviceManager.addSingleton(IFileConverter, FileConverter); From 1edb1ec6b9b92e113a3548cd41e05c454e258566 Mon Sep 17 00:00:00 2001 From: Christoffer Artmann Date: Tue, 10 Feb 2026 11:28:13 +0100 Subject: [PATCH 2/6] Improvements --- .../deepnote/deepnoteFileChangeWatcher.ts | 30 ++++++--- .../deepnoteFileChangeWatcher.unit.test.ts | 62 +++++++++---------- 2 files changed, 49 insertions(+), 43 deletions(-) diff --git a/src/notebooks/deepnote/deepnoteFileChangeWatcher.ts b/src/notebooks/deepnote/deepnoteFileChangeWatcher.ts index e08c276cf..4b1a0f467 100644 --- a/src/notebooks/deepnote/deepnoteFileChangeWatcher.ts +++ b/src/notebooks/deepnote/deepnoteFileChangeWatcher.ts @@ -47,6 +47,14 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic this.disposables.push(watcher); this.disposables.push(watcher.onDidChange((uri) => this.handleFileChange(uri))); + this.disposables.push({ dispose: () => this.clearAllTimers() }); + } + + private clearAllTimers(): void { + for (const timer of this.debounceTimers.values()) { + clearTimeout(timer); + } + this.debounceTimers.clear(); } private handleFileChange(uri: Uri): void { @@ -99,14 +107,20 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic return; } + const tokenSource = new CancellationTokenSource(); + let newData; + try { + newData = await this.serializer.deserializeNotebook(content, tokenSource.token); + } finally { + tokenSource.dispose(); + } + for (const notebook of affectedNotebooks) { try { - const tokenSource = new CancellationTokenSource(); - const newData = await this.serializer.deserializeNotebook(content, tokenSource.token); - tokenSource.dispose(); + const newCells = newData.cells.map((cell) => ({ ...cell })); - if (this.cellsMatchNotebook(notebook, newData.cells)) { - return; + if (this.cellsMatchNotebook(notebook, newCells)) { + continue; } // Preserve outputs from live cells that the deserialized data may lack. @@ -123,7 +137,7 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic } } - for (const cell of newData.cells) { + for (const cell of newCells) { const blockId = (cell.metadata?.id ?? cell.metadata?.__deepnoteBlockId) as string | undefined; if (blockId && (!cell.outputs || cell.outputs.length === 0)) { const liveOutputs = liveOutputsByBlockId.get(blockId); @@ -134,9 +148,7 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic } const edit = new WorkspaceEdit(); - edit.set(notebook.uri, [ - NotebookEdit.replaceCells(new NotebookRange(0, notebook.cellCount), newData.cells) - ]); + edit.set(notebook.uri, [NotebookEdit.replaceCells(new NotebookRange(0, notebook.cellCount), newCells)]); await workspace.applyEdit(edit); // Save immediately so VS Code updates its internal mtime for the file. diff --git a/src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts b/src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts index 343b89357..d2e57125b 100644 --- a/src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts @@ -1,6 +1,6 @@ import { assert } from 'chai'; import * as sinon from 'sinon'; -import { anything, instance, mock, verify, when } from 'ts-mockito'; +import { anything, instance, mock, when } from 'ts-mockito'; import { EventEmitter, FileSystemWatcher, NotebookCellKind, NotebookDocument, Uri } from 'vscode'; import type { IDisposableRegistry } from '../../platform/common/types'; @@ -27,10 +27,14 @@ suite('DeepnoteFileChangeWatcher', () => { let mockNotebookManager: IDeepnoteNotebookManager; let onDidChangeFile: EventEmitter; let readFileCalls: number; + let applyEditCount: number; + let saveCount: number; setup(() => { resetVSCodeMocks(); readFileCalls = 0; + applyEditCount = 0; + saveCount = 0; mockDisposables = []; mockNotebookManager = instance(mock()); @@ -43,9 +47,14 @@ suite('DeepnoteFileChangeWatcher', () => { when(mockedVSCodeNamespaces.workspace.createFileSystemWatcher(anything())).thenReturn(instance(fsWatcher)); - when(mockedVSCodeNamespaces.workspace.save(anything())).thenReturn( - Promise.resolve(Uri.file('/workspace/test.deepnote')) - ); + when(mockedVSCodeNamespaces.workspace.applyEdit(anything())).thenCall(() => { + applyEditCount++; + return Promise.resolve(true); + }); + when(mockedVSCodeNamespaces.workspace.save(anything())).thenCall(() => { + saveCount++; + return Promise.resolve(Uri.file('/workspace/test.deepnote')); + }); watcher = new DeepnoteFileChangeWatcher(mockDisposables, mockNotebookManager); watcher.activate(); @@ -149,11 +158,10 @@ project: // Wait for debounce + deserialization await waitFor(() => readFileCalls > 0); - await new Promise((resolve) => setTimeout(resolve, 200)); // File was read, but applyEdit should NOT be called because cells match assert.isAtLeast(readFileCalls, 1, 'readFile should be called'); - verify(mockedVSCodeNamespaces.workspace.applyEdit(anything())).never(); + assert.strictEqual(applyEditCount, 0, 'applyEdit should not be called when cells match'); }); test('should reload on external change', async () => { @@ -166,15 +174,11 @@ project: // Fire file change without a preceding save onDidChangeFile.fire(uri); - // Wait for the debounce + async deserialization to complete - await waitFor(() => readFileCalls > 0); - - // Wait a bit more for the async chain to finish (deserialize + applyEdit + save) - await new Promise((resolve) => setTimeout(resolve, 200)); + // Wait for the full async chain: debounce + deserialize + applyEdit + save + await waitFor(() => saveCount > 0); - verify(mockedVSCodeNamespaces.workspace.applyEdit(anything())).atLeast(1); - // After applyEdit, the document should be saved to update VS Code's mtime - verify(mockedVSCodeNamespaces.workspace.save(anything())).atLeast(1); + assert.isAtLeast(applyEditCount, 1, 'applyEdit should be called'); + assert.isAtLeast(saveCount, 1, 'save should be called after applyEdit'); }); test('should skip snapshot files', async () => { @@ -201,14 +205,13 @@ project: onDidChangeFile.fire(uri); - // Wait for debounce + readFile to happen - await waitFor(() => readFileCalls > 0); - await new Promise((resolve) => setTimeout(resolve, 200)); + // Wait for the full async chain to finish + await waitFor(() => saveCount > 0); // Dirty notebooks should now be reloaded and saved to prevent mtime conflicts assert.isAtLeast(readFileCalls, 1, 'readFile should be called'); - verify(mockedVSCodeNamespaces.workspace.applyEdit(anything())).atLeast(1); - verify(mockedVSCodeNamespaces.workspace.save(anything())).atLeast(1); + assert.isAtLeast(applyEditCount, 1, 'applyEdit should be called'); + assert.isAtLeast(saveCount, 1, 'save should be called'); }); test('should debounce rapid changes', async () => { @@ -226,8 +229,7 @@ project: onDidChangeFile.fire(uri); // Wait for debounce from the last event + processing - await waitFor(() => readFileCalls > 0); - await new Promise((resolve) => setTimeout(resolve, 200)); + await waitFor(() => applyEditCount > 0); // readFile should only be called once (debounced) assert.strictEqual(readFileCalls, 1, 'readFile should be called exactly once after debounce'); @@ -244,10 +246,9 @@ project: // Wait for debounce + processing await waitFor(() => readFileCalls > 0); - await new Promise((resolve) => setTimeout(resolve, 200)); // Should not throw - errors are caught and logged - assert.ok(true, 'Parse error was handled gracefully'); + assert.strictEqual(applyEditCount, 0, 'applyEdit should not be called on parse error'); }); test('should preserve live cell outputs during reload', async () => { @@ -268,11 +269,10 @@ project: onDidChangeFile.fire(uri); - await waitFor(() => readFileCalls > 0); - await new Promise((resolve) => setTimeout(resolve, 200)); + await waitFor(() => applyEditCount > 0); // applyEdit should be called — the output preservation runs before it - verify(mockedVSCodeNamespaces.workspace.applyEdit(anything())).atLeast(1); + assert.isAtLeast(applyEditCount, 1, 'applyEdit should be called'); }); test('should reload dirty notebooks and preserve outputs', async () => { @@ -294,20 +294,14 @@ project: onDidChangeFile.fire(uri); - await waitFor(() => readFileCalls > 0); - await new Promise((resolve) => setTimeout(resolve, 200)); + await waitFor(() => applyEditCount > 0); // Dirty notebook should still be reloaded with outputs preserved - verify(mockedVSCodeNamespaces.workspace.applyEdit(anything())).atLeast(1); + assert.isAtLeast(applyEditCount, 1, 'applyEdit should be called'); }); test('should not suppress real changes after auto-save', async () => { const uri = Uri.file('/workspace/test.deepnote'); - let applyEditCount = 0; - when(mockedVSCodeNamespaces.workspace.applyEdit(anything())).thenCall(() => { - applyEditCount++; - return Promise.resolve(true); - }); // First change: notebook has no cells, YAML has one cell -> different -> reload const notebook = createMockNotebook({ uri, cellCount: 0, cells: [] }); From 72ed472e5644acebecfeec8055cffd8bb5d7c4ff Mon Sep 17 00:00:00 2001 From: Christoffer Artmann Date: Tue, 10 Feb 2026 15:11:50 +0100 Subject: [PATCH 3/6] format --- .../deepnote/deepnoteFileChangeWatcher.ts | 20 ++++++++++++++++--- .../deepnoteFileChangeWatcher.unit.test.ts | 2 +- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/notebooks/deepnote/deepnoteFileChangeWatcher.ts b/src/notebooks/deepnote/deepnoteFileChangeWatcher.ts index 4b1a0f467..cde8df019 100644 --- a/src/notebooks/deepnote/deepnoteFileChangeWatcher.ts +++ b/src/notebooks/deepnote/deepnoteFileChangeWatcher.ts @@ -19,7 +19,7 @@ import { DeepnoteNotebookSerializer } from './deepnoteSerializer'; import { isSnapshotFile } from './snapshots/snapshotFiles'; import { SnapshotService } from './snapshots/snapshotService'; -const DEBOUNCE_MS = 500; +const debounceTimeInMilliseconds = 500; /** * Watches .deepnote files for external changes and reloads open notebook editors. @@ -54,6 +54,7 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic for (const timer of this.debounceTimers.values()) { clearTimeout(timer); } + this.debounceTimers.clear(); } @@ -65,6 +66,7 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic const key = uri.toString(); const existing = this.debounceTimers.get(key); + if (existing) { clearTimeout(existing); } @@ -73,16 +75,19 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic key, setTimeout(() => { this.debounceTimers.delete(key); + void this.reloadNotebooksForFile(uri); - }, DEBOUNCE_MS) + }, debounceTimeInMilliseconds) ); } private cellsMatchNotebook(notebook: NotebookDocument, newCells: NotebookCellData[]): boolean { const liveCells = notebook.getCells(); + if (liveCells.length !== newCells.length) { return false; } + return liveCells.every( (live, i) => live.document.getText() === newCells[i].value && live.kind === newCells[i].kind ); @@ -90,6 +95,7 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic private async reloadNotebooksForFile(uri: Uri): Promise { const uriString = uri.toString(); + const affectedNotebooks = workspace.notebookDocuments.filter( (doc) => doc.notebookType === 'deepnote' && doc.uri.with({ query: '', fragment: '' }).toString() === uriString @@ -100,6 +106,7 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic } let content: Uint8Array; + try { content = await workspace.fs.readFile(uri); } catch (error) { @@ -111,6 +118,9 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic let newData; try { newData = await this.serializer.deserializeNotebook(content, tokenSource.token); + } catch (error) { + logger.warn(`[FileChangeWatcher] Failed to parse changed file: ${uri.path}`, error); + return; } finally { tokenSource.dispose(); } @@ -149,7 +159,11 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic const edit = new WorkspaceEdit(); edit.set(notebook.uri, [NotebookEdit.replaceCells(new NotebookRange(0, notebook.cellCount), newCells)]); - await workspace.applyEdit(edit); + const applied = await workspace.applyEdit(edit); + if (!applied) { + logger.warn(`[FileChangeWatcher] Failed to apply edit: ${notebook.uri.path}`); + continue; + } // Save immediately so VS Code updates its internal mtime for the file. // Without this, the user gets a "content is newer" conflict dialog on diff --git a/src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts b/src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts index d2e57125b..024afd45b 100644 --- a/src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts @@ -247,7 +247,7 @@ project: // Wait for debounce + processing await waitFor(() => readFileCalls > 0); - // Should not throw - errors are caught and logged + // Parse errors should be caught and logged without calling applyEdit assert.strictEqual(applyEditCount, 0, 'applyEdit should not be called on parse error'); }); From be9c94ef5a7ad1a7f35355dbd487c010cb46953b Mon Sep 17 00:00:00 2001 From: Christoffer Artmann Date: Tue, 10 Feb 2026 15:17:04 +0100 Subject: [PATCH 4/6] fix: Use validYaml in debounce test to avoid false cell match The debounce test used emptyBlocksYaml against a notebook with 0 cells, so cellsMatchNotebook returned true and applyEdit was never called, causing a timeout in CI. --- src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts b/src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts index 024afd45b..cf79db1f3 100644 --- a/src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts @@ -219,7 +219,7 @@ project: const notebook = createMockNotebook({ uri, cellCount: 0 }); when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); - setupMockFs(emptyBlocksYaml); + setupMockFs(validYaml); // Fire multiple changes rapidly onDidChangeFile.fire(uri); From efa5666bcc06a874948833dc83e4fe5a743508cf Mon Sep 17 00:00:00 2001 From: Christoffer Artmann Date: Tue, 10 Feb 2026 15:20:02 +0100 Subject: [PATCH 5/6] refactor: Extract block ID helper and fix test teardown cleanup --- .../deepnote/deepnoteFileChangeWatcher.ts | 10 ++++++---- .../deepnoteFileChangeWatcher.unit.test.ts | 14 +------------- 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/src/notebooks/deepnote/deepnoteFileChangeWatcher.ts b/src/notebooks/deepnote/deepnoteFileChangeWatcher.ts index cde8df019..1941bd762 100644 --- a/src/notebooks/deepnote/deepnoteFileChangeWatcher.ts +++ b/src/notebooks/deepnote/deepnoteFileChangeWatcher.ts @@ -50,6 +50,10 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic this.disposables.push({ dispose: () => this.clearAllTimers() }); } + private getBlockIdFromMetadata(metadata: Record | undefined): string | undefined { + return (metadata?.id ?? metadata?.__deepnoteBlockId) as string | undefined; + } + private clearAllTimers(): void { for (const timer of this.debounceTimers.values()) { clearTimeout(timer); @@ -139,16 +143,14 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic const liveCells = notebook.getCells(); const liveOutputsByBlockId = new Map(); for (const liveCell of liveCells) { - const blockId = (liveCell.metadata?.id ?? liveCell.metadata?.__deepnoteBlockId) as - | string - | undefined; + const blockId = this.getBlockIdFromMetadata(liveCell.metadata); if (blockId && liveCell.outputs.length > 0) { liveOutputsByBlockId.set(blockId, liveCell.outputs); } } for (const cell of newCells) { - const blockId = (cell.metadata?.id ?? cell.metadata?.__deepnoteBlockId) as string | undefined; + const blockId = this.getBlockIdFromMetadata(cell.metadata); if (blockId && (!cell.outputs || cell.outputs.length === 0)) { const liveOutputs = liveOutputsByBlockId.get(blockId); if (liveOutputs) { diff --git a/src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts b/src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts index cf79db1f3..94db24078 100644 --- a/src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts @@ -62,6 +62,7 @@ suite('DeepnoteFileChangeWatcher', () => { teardown(() => { sinon.restore(); + mockDisposables.forEach((d) => d.dispose()); onDidChangeFile.dispose(); }); @@ -123,19 +124,6 @@ project: content: print("hello") `; - const emptyBlocksYaml = ` -version: '1.0' -metadata: - createdAt: '2025-01-01T00:00:00Z' -project: - id: project-1 - name: Test Project - notebooks: - - id: notebook-1 - name: Notebook 1 - blocks: [] -`; - test('should skip reload when content matches notebook cells', async () => { const uri = Uri.file('/workspace/test.deepnote'); // Create a notebook whose cell content already matches validYaml From d6d36ccfcc381a41bcfa528af6ff649c4f72f9ef Mon Sep 17 00:00:00 2001 From: Christoffer Artmann Date: Tue, 10 Feb 2026 15:44:11 +0100 Subject: [PATCH 6/6] fix: Address CodeRabbit review comments - Alphabetize private fields and methods - Add blank lines before return statements - Extract timing magic numbers into named constants in tests - Use block-body arrow in forEach to avoid implicit return - Update misleading test comment --- .../deepnote/deepnoteFileChangeWatcher.ts | 34 +++++++++---------- .../deepnoteFileChangeWatcher.unit.test.ts | 27 +++++++++++---- 2 files changed, 37 insertions(+), 24 deletions(-) diff --git a/src/notebooks/deepnote/deepnoteFileChangeWatcher.ts b/src/notebooks/deepnote/deepnoteFileChangeWatcher.ts index 1941bd762..4e6393bc1 100644 --- a/src/notebooks/deepnote/deepnoteFileChangeWatcher.ts +++ b/src/notebooks/deepnote/deepnoteFileChangeWatcher.ts @@ -31,8 +31,8 @@ const debounceTimeInMilliseconds = 500; */ @injectable() export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationService { - private readonly serializer: DeepnoteNotebookSerializer; private readonly debounceTimers = new Map>(); + private readonly serializer: DeepnoteNotebookSerializer; constructor( @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, @@ -50,8 +50,16 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic this.disposables.push({ dispose: () => this.clearAllTimers() }); } - private getBlockIdFromMetadata(metadata: Record | undefined): string | undefined { - return (metadata?.id ?? metadata?.__deepnoteBlockId) as string | undefined; + private cellsMatchNotebook(notebook: NotebookDocument, newCells: NotebookCellData[]): boolean { + const liveCells = notebook.getCells(); + + if (liveCells.length !== newCells.length) { + return false; + } + + return liveCells.every( + (live, i) => live.document.getText() === newCells[i].value && live.kind === newCells[i].kind + ); } private clearAllTimers(): void { @@ -62,13 +70,16 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic this.debounceTimers.clear(); } + private getBlockIdFromMetadata(metadata: Record | undefined): string | undefined { + return (metadata?.id ?? metadata?.__deepnoteBlockId) as string | undefined; + } + private handleFileChange(uri: Uri): void { if (isSnapshotFile(uri)) { return; } const key = uri.toString(); - const existing = this.debounceTimers.get(key); if (existing) { @@ -85,21 +96,8 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic ); } - private cellsMatchNotebook(notebook: NotebookDocument, newCells: NotebookCellData[]): boolean { - const liveCells = notebook.getCells(); - - if (liveCells.length !== newCells.length) { - return false; - } - - return liveCells.every( - (live, i) => live.document.getText() === newCells[i].value && live.kind === newCells[i].kind - ); - } - private async reloadNotebooksForFile(uri: Uri): Promise { const uriString = uri.toString(); - const affectedNotebooks = workspace.notebookDocuments.filter( (doc) => doc.notebookType === 'deepnote' && doc.uri.with({ query: '', fragment: '' }).toString() === uriString @@ -115,6 +113,7 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic content = await workspace.fs.readFile(uri); } catch (error) { logger.warn(`[FileChangeWatcher] Failed to read changed file: ${uri.path}`, error); + return; } @@ -124,6 +123,7 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic newData = await this.serializer.deserializeNotebook(content, tokenSource.token); } catch (error) { logger.warn(`[FileChangeWatcher] Failed to parse changed file: ${uri.path}`, error); + return; } finally { tokenSource.dispose(); diff --git a/src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts b/src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts index 94db24078..189c2f56f 100644 --- a/src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts @@ -8,10 +8,19 @@ import { mockedVSCodeNamespaces, resetVSCodeMocks } from '../../test/vscode-mock import { IDeepnoteNotebookManager } from '../types'; import { DeepnoteFileChangeWatcher } from './deepnoteFileChangeWatcher'; +const waitForTimeoutMs = 5000; +const waitForIntervalMs = 50; +const debounceWaitMs = 800; +const rapidChangeIntervalMs = 100; + /** * Polls until a condition is met or a timeout is reached. */ -async function waitFor(condition: () => boolean, timeoutMs = 5000, intervalMs = 50): Promise { +async function waitFor( + condition: () => boolean, + timeoutMs = waitForTimeoutMs, + intervalMs = waitForIntervalMs +): Promise { const start = Date.now(); while (!condition()) { if (Date.now() - start > timeoutMs) { @@ -62,7 +71,9 @@ suite('DeepnoteFileChangeWatcher', () => { teardown(() => { sinon.restore(); - mockDisposables.forEach((d) => d.dispose()); + for (const d of mockDisposables) { + d.dispose(); + } onDidChangeFile.dispose(); }); @@ -84,6 +95,7 @@ suite('DeepnoteFileChangeWatcher', () => { kind: c.kind ?? NotebookCellKind.Code, document: c.document ?? { getText: () => '' } })); + return { uri: opts.uri, isDirty: opts.isDirty ?? false, @@ -104,6 +116,7 @@ suite('DeepnoteFileChangeWatcher', () => { return Promise.resolve(new TextEncoder().encode(yamlContent)); }); when(mockedVSCodeNamespaces.workspace.fs).thenReturn(instance(mockFs)); + return mockFs; } @@ -178,7 +191,7 @@ project: onDidChangeFile.fire(snapshotUri); // Wait well past debounce - await new Promise((resolve) => setTimeout(resolve, 800)); + await new Promise((resolve) => setTimeout(resolve, debounceWaitMs)); // Should not attempt to read the file at all assert.strictEqual(readFileCalls, 0, 'readFile should not be called for snapshot files'); @@ -211,9 +224,9 @@ project: // Fire multiple changes rapidly onDidChangeFile.fire(uri); - await new Promise((resolve) => setTimeout(resolve, 100)); + await new Promise((resolve) => setTimeout(resolve, rapidChangeIntervalMs)); onDidChangeFile.fire(uri); - await new Promise((resolve) => setTimeout(resolve, 100)); + await new Promise((resolve) => setTimeout(resolve, rapidChangeIntervalMs)); onDidChangeFile.fire(uri); // Wait for debounce from the last event + processing @@ -235,7 +248,7 @@ project: // Wait for debounce + processing await waitFor(() => readFileCalls > 0); - // Parse errors should be caught and logged without calling applyEdit + // Parse errors are caught and logged; applyEdit should not be called assert.strictEqual(applyEditCount, 0, 'applyEdit should not be called on parse error'); }); @@ -318,7 +331,7 @@ project: `; setupMockFs(changedYaml); onDidChangeFile.fire(uri); - await waitFor(() => applyEditCount >= 2, 5000); + await waitFor(() => applyEditCount >= 2, waitForTimeoutMs); assert.isAtLeast(applyEditCount, 2, 'applyEdit should be called for both external changes'); });