Skip to content
Draft
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
6802960
fix: Fix deepnote notebook deserializer
tkislan Mar 18, 2026
2de7885
Add tests
tkislan Mar 18, 2026
bc2de06
Merge branch 'main' into tk/fix-project-notebook-picker
tkislan Mar 19, 2026
3a418c1
Fix file change watcher
tkislan Mar 19, 2026
9b5c107
Merge remote-tracking branch 'origin/main' into tk/fix-project-notebo…
tkislan Mar 19, 2026
77c0347
Improve error handling in DeepnoteFileChangeWatcher to prevent stale …
tkislan Mar 20, 2026
6dc77a0
Add unit tests for DeepnoteFileChangeWatcher to handle scenarios with…
tkislan Mar 20, 2026
8898be9
Format code
tkislan Mar 20, 2026
c74ffa7
Enhance error handling in DeepnoteFileChangeWatcher to check save ope…
tkislan Mar 23, 2026
8d061ac
Add post-snapshot read grace period in unit tests for DeepnoteFileCha…
tkislan Mar 23, 2026
5388ead
Remove the second markSelfWrite() on the workspace.save() path.
tkislan Mar 26, 2026
6d05f22
Merge branch 'main' into tk/fix-project-notebook-picker
tkislan Mar 26, 2026
c5342a3
Merge branch 'main' into tk/fix-project-notebook-picker
tkislan Mar 26, 2026
3e7dfc9
feat(deepnote): Add clearNotebookSelection method and update notebook…
tkislan Mar 27, 2026
c160ae3
Update test
tkislan Mar 27, 2026
0b2504a
refactor(deepnote): Improve mock child process and server output mana…
tkislan Mar 27, 2026
9d5c6c0
refactor(deepnote): Simplify notebook edit application logic
tkislan Mar 27, 2026
44bd482
feat(deepnote): Enhance notebook resolution management and error hand…
tkislan Mar 30, 2026
36cc1e6
refactor(deepnote): Improve notebook ID retrieval with zod validation
tkislan Mar 31, 2026
71579cf
refactor(deepnote): Improve variable naming and import organization i…
tkislan Mar 31, 2026
23fdcca
Fix notebook deserialization race conditions
tkislan Mar 31, 2026
824f471
feat(deepnote): Enhance testing for DeepnoteFileChangeWatcher and Dee…
tkislan Mar 31, 2026
c6d21e6
feat(deepnote): Add snapshot interaction tests for DeepnoteFileChange…
tkislan Apr 1, 2026
5d9d6e2
Fix cspell
tkislan Apr 1, 2026
4962616
Fix tests
tkislan Apr 1, 2026
552366b
Minor improvements
tkislan Apr 1, 2026
bd4d5fd
fix(deepnote): Enhance error handling for metadata restoration in Dee…
tkislan Apr 1, 2026
cb4644f
Merge remote-tracking branch 'origin/main' into tk/fix-project-notebo…
tkislan Apr 2, 2026
ec96f53
feat(deepnote): Add DeepnoteNotebookInfoStatusBar for displaying note…
tkislan Apr 2, 2026
d44c1aa
feat(deepnote): Add command to copy active notebook details
tkislan Apr 2, 2026
7815f42
Add a failing test for external change rerender
tkislan Apr 2, 2026
855f38c
Refactor self write mark handling
tkislan Apr 2, 2026
f72b0fa
feat(deepnote): Enhance notebook ID resolution with tab-based logic
tkislan Apr 2, 2026
8e6f679
feat(deepnote): Refactor notebook selection handling and enhance mism…
tkislan Apr 7, 2026
0396a2c
refactor(deepnote): Simplify DeepnoteActivationService and enhance no…
tkislan Apr 7, 2026
06d3197
refactor(deepnote): Streamline DeepnoteNotebookSerializer and remove …
tkislan Apr 7, 2026
f4b79d3
feat(deepnote): Add logging for pending notebook resolutions in Deepn…
tkislan Apr 7, 2026
c83af55
Reformat code
tkislan Apr 7, 2026
40f6c67
Fix test
tkislan Apr 7, 2026
46cbcda
feat(deepnote): Enhance notebook deserialization and verification logic
tkislan Apr 9, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 50 additions & 8 deletions src/notebooks/deepnote/deepnoteFileChangeWatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down Expand Up @@ -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 ||
Expand Down Expand Up @@ -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);
Expand All @@ -341,13 +343,44 @@ 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.
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);
Comment thread
tkislan marked this conversation as resolved.
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
} 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}`);
Expand Down Expand Up @@ -523,6 +556,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();
}
Comment thread
tkislan marked this conversation as resolved.

private handleFileChange(uri: Uri): void {
// Deterministic self-write check — no timers involved
if (this.consumeSelfWrite(uri)) {
Expand Down Expand Up @@ -581,7 +623,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);

Expand Down
85 changes: 74 additions & 11 deletions src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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>;
Expand All @@ -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>();
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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'),
Expand All @@ -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 () => {
Expand Down
19 changes: 10 additions & 9 deletions src/notebooks/deepnote/deepnoteSerializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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
);
Expand Down
Loading
Loading