fix: Fix deepnote notebook deserializer and file change watcher

src/notebooks/deepnote/deepnoteFileChangeWatcher.ts

@@ -145,7 +145,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 +187,7 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic
         if (liveCells.length !== newCells.length) {
             return true;
         }
+
         return liveCells.some(
             (live, i) =>
                 live.kind !== newCells[i].kind ||
@@ -332,6 +333,7 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic
             }
         }
 
+        // Apply the edit to update in-memory cells immediately (responsive UX).
         const wsEdit = new WorkspaceEdit();
         wsEdit.set(notebook.uri, edits);
         const applied = await workspace.applyEdit(wsEdit);
@@ -341,13 +343,47 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic
             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.
+            this.markSelfWrite(fileUri);
+            try {
+                const saved = await workspace.save(notebook.uri);
+                if (!saved) {
+                    this.consumeSelfWrite(fileUri);
+                    logger.warn(`[FileChangeWatcher] Save after sync write returned undefined: ${notebook.uri.path}`);
+                    return;
+                }
+            } catch (saveError) {
+                this.consumeSelfWrite(fileUri);
+                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}`);
@@ -523,6 +559,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)) {
@@ -581,7 +626,7 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic
      * Call before workspace.save() to prevent the resulting fs event from triggering a reload.
      */
     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);
 

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

@@ -11,11 +11,28 @@ import { IDeepnoteNotebookManager } from '../types';
 import { DeepnoteFileChangeWatcher } from './deepnoteFileChangeWatcher';
 import { SnapshotService } from './snapshots/snapshotService';
 
+const validProject = {
+    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")' }]
+            }
+        ]
+    }
+} as DeepnoteProject;
+
 const waitForTimeoutMs = 5000;
 const waitForIntervalMs = 50;
 const debounceWaitMs = 800;
 const rapidChangeIntervalMs = 100;
 const autoSaveGraceMs = 200;
+const postSnapshotReadGraceMs = 100;
 
 /**
  * Polls until a condition is met or a timeout is reached.
@@ -37,6 +54,7 @@ async function waitFor(
 suite('DeepnoteFileChangeWatcher', () => {
     let watcher: DeepnoteFileChangeWatcher;
     let mockDisposables: IDisposableRegistry;
+    let mockedNotebookManager: IDeepnoteNotebookManager;
     let mockNotebookManager: IDeepnoteNotebookManager;
     let onDidChangeFile: EventEmitter<Uri>;
     let onDidCreateFile: EventEmitter<Uri>;
@@ -51,7 +69,11 @@ suite('DeepnoteFileChangeWatcher', () => {
         saveCount = 0;
 
         mockDisposables = [];
-        mockNotebookManager = instance(mock<IDeepnoteNotebookManager>());
+
+        mockedNotebookManager = mock<IDeepnoteNotebookManager>();
+        when(mockedNotebookManager.getOriginalProject(anything())).thenReturn(validProject);
+        when(mockedNotebookManager.getTheSelectedNotebookForAProject(anything())).thenReturn('notebook-1');
+        mockNotebookManager = instance(mockedNotebookManager);
 
         // Set up FileSystemWatcher mock
         onDidChangeFile = new EventEmitter<Uri>();
@@ -126,6 +148,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;
@@ -325,8 +348,9 @@ project:
         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
@@ -1053,6 +1077,42 @@ project:
         });
 
         test('should not apply updates when cells have no block IDs and no fallback', async () => {
+            const noFallbackDisposables: IDisposableRegistry = [];
+            const noFallbackOnDidChange = new EventEmitter<Uri>();
+            const noFallbackOnDidCreate = new EventEmitter<Uri>();
+            const fsWatcherNf = mock<FileSystemWatcher>();
+            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<SnapshotService>();
+            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<IDeepnoteNotebookManager>();
+            when(nfManager.getOriginalProject(anything())).thenReturn(undefined);
+
+            const nfWatcher = new DeepnoteFileChangeWatcher(
+                noFallbackDisposables,
+                instance(nfManager),
+                instance(nfSnapshotService)
+            );
+            nfWatcher.activate();
+
             const snapshotUri = Uri.file('/workspace/snapshots/my-project_project-1_latest.snapshot.deepnote');
             const notebook = createMockNotebook({
                 uri: Uri.file('/workspace/test.deepnote'),
@@ -1068,16 +1128,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 () => {

src/notebooks/deepnote/deepnoteSerializer.ts

@@ -188,7 +188,15 @@ export class DeepnoteNotebookSerializer implements NotebookSerializer {
      * @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
+        // Check the manager's stored selection first - this is set explicitly when the user
+        // picks a notebook from the explorer, and must take priority over the active editor
+        const storedNotebookId = this.notebookManager.getTheSelectedNotebookForAProject(projectId);
+
+        if (storedNotebookId) {
+            return storedNotebookId;
+        }
+
+        // Fallback: prefer the active notebook editor when it matches the project
         const activeEditorNotebook = window.activeNotebookEditor?.notebook;
 
         if (
@@ -199,14 +207,7 @@ export class DeepnoteNotebookSerializer implements NotebookSerializer {
             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
+        // Last 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
         );