Skip to content
Closed
Show file tree
Hide file tree
Changes from 45 commits
Commits
Show all changes
52 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
477b790
feat(deepnote-file): Migration to single notebook deepnote file
tkislan Apr 13, 2026
1140fa0
feat(deepnote): Enhance snapshot output update handling with notebook…
tkislan Apr 16, 2026
966bf96
feat(deepnote): Implement notebook ID support in project management
tkislan Apr 17, 2026
367db44
Fix test
tkislan Apr 17, 2026
b73d313
feat(deepnote): Refactor notebook file handling and introduce new fac…
tkislan Apr 17, 2026
cea5ecd
Rename test
tkislan Apr 17, 2026
a47d889
fix(deepnote): Update context handling for project files and notebooks
tkislan Apr 20, 2026
c872be8
refactor(deepnote): Key operations by projectId for improved server m…
tkislan Apr 20, 2026
b415441
feat(deepnote): Improve snapshot handling with output settling logic
tkislan Apr 23, 2026
32671f0
feat(deepnote): Enhance environment switching and notebook rebuilding…
tkislan Apr 23, 2026
bfd0cc3
feat(deepnote): Process already-open notebooks during activation
tkislan Apr 23, 2026
b3278e9
Cleanup tests
tkislan Apr 24, 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
1 change: 1 addition & 0 deletions build/mocha-esm-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
Expand Down
6 changes: 6 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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%",
Expand Down
1 change: 1 addition & 0 deletions package.nls.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
34 changes: 22 additions & 12 deletions src/kernels/deepnote/deepnoteServerStarter.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 || '',
Expand Down Expand Up @@ -343,11 +343,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension

this.serverOutputByFile.delete(fileKey);

const disposables = this.disposablesByFile.get(fileKey);
if (disposables) {
disposables.forEach((d) => d.dispose());
this.disposablesByFile.delete(fileKey);
}
this.disposeOutputListeners(fileKey);
}

/**
Expand Down Expand Up @@ -402,6 +398,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
*/
private monitorServerOutput(fileKey: string, serverInfo: DeepnoteServerInfo): void {
const proc = serverInfo.process;
this.disposeOutputListeners(fileKey);
const disposables: IDisposable[] = [];
this.disposablesByFile.set(fileKey, disposables);

Expand Down Expand Up @@ -446,6 +443,22 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
}
}

/**
* Dispose all output listeners registered for a given file key.
* Per-listener try/catch ensures one failure doesn't prevent others from being disposed.
*/
private disposeOutputListeners(fileKey: string): void {
const disposables = this.disposablesByFile.get(fileKey) ?? [];
for (const d of disposables) {
try {
d.dispose();
} catch (ex) {
logger.warn(`Error disposing output listener for ${fileKey}`, ex);
}
}
this.disposablesByFile.delete(fileKey);
}

public async dispose(): Promise<void> {
Comment on lines +444 to 460

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Reorder members to match required class ordering.

private disposeOutputListeners is introduced immediately before public dispose(), which breaks the required accessibility/alphabetical ordering in this section. Please reorder this block to align with the class member ordering rule. As per coding guidelines, "Order method, fields and properties, first by accessibility and then by alphabetical order".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/kernels/deepnote/deepnoteServerStarter.node.ts` around lines 446 - 462,
The private method disposeOutputListeners is placed immediately before the
public dispose method, violating the rule to order class members first by
accessibility then alphabetically; move disposeOutputListeners so that all
public members (including dispose) appear before private members, or otherwise
reorder members so that public dispose() comes before private
disposeOutputListeners and ensure disposablesByFile usage remains correct;
update the class member sequence so accessibility groups (public then private)
and alphabetical ordering within each group are respected for dispose() and
disposeOutputListeners.

logger.info('Disposing DeepnoteServerStarter - stopping all servers...');

Expand Down Expand Up @@ -485,12 +498,9 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension
await this.deleteLockFile(pid);
}

for (const [fileKey, disposables] of this.disposablesByFile.entries()) {
try {
disposables.forEach((d) => d.dispose());
} catch (ex) {
logger.error(`Error disposing resources for ${fileKey}`, ex);
}
// Snapshot the keys first: disposeOutputListeners mutates disposablesByFile via .delete().
for (const fileKey of Array.from(this.disposablesByFile.keys())) {
this.disposeOutputListeners(fileKey);
}

this.disposablesByFile.clear();
Expand Down
59 changes: 57 additions & 2 deletions src/kernels/deepnote/deepnoteTestHelpers.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>): 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,
Comment on lines +15 to +66

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

usage_files="$(rg -l --type=ts '\bcreateMockChildProcess\s*\(' || true)"
if [ -z "$usage_files" ]; then
  echo "No call sites found."
  exit 0
fi

echo "createMockChildProcess call sites:"
printf '%s\n' "$usage_files"

echo
echo "Potential event/state-dependent usages to inspect:"
while IFS= read -r file; do
  rg -n -C2 '\bcreateMockChildProcess\s*\(|\.(on|once|emit|kill|disconnect)\s*\(|\.(killed|connected)\b' "$file"
done <<< "$usage_files"

Repository: deepnote/vscode-deepnote

Length of output: 2366


🏁 Script executed:

# Check how mockProcess is actually used in the test files
for file in src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts \
            src/kernels/deepnote/environments/deepnoteEnvironmentTreeDataProvider.unit.test.ts \
            src/kernels/deepnote/deepnoteLspClientManager.node.vscode.test.ts; do
  if [ -f "$file" ]; then
    echo "=== $file ==="
    # Look for any usage of killed, connected, on, once, emit, kill, disconnect on the process mock
    rg -n '(\.killed|\.connected|\.on\(|\.once\(|\.emit\(|\.kill\(|\.disconnect\()\b' "$file" || echo "  (no state/event usage found)"
  fi
done

Repository: deepnote/vscode-deepnote

Length of output: 401


Refactor mock to use real EventEmitter and mutable state.

The mock's listener stubs and hardcoded killed/connected flags don't match ChildProcess semantics. While current tests don't use these features, a test that awaits process events or checks post-call state will get a lying mock. Use EventEmitter as a base and expose state via getters instead.

Suggested fix
+import { EventEmitter } from 'node:events';
 import type { ChildProcess } from 'node:child_process';
 
 export function createMockChildProcess(overrides?: Partial<ChildProcess>): ChildProcess {
-    const mockProcess: ChildProcess = {
+    const emitter = new EventEmitter();
+    let killed = false;
+    let connected = false;
+
+    const mockProcess = Object.assign(emitter, {
         pid: undefined,
         stdio: [null, null, null, null, null],
         stdin: null,
         stdout: null,
         stderr: null,
         exitCode: null,
-        killed: false,
-        connected: false,
+        get killed() {
+            return killed;
+        },
+        get connected() {
+            return connected;
+        },
         signalCode: null,
         spawnargs: [],
         spawnfile: '',
-        kill: () => true,
+        kill: () => {
+            killed = true;
+            return 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;
+        disconnect: () => {
+            connected = false;
         },
         [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,
+            killed = true;
+        },
         ...overrides
-    };
+    }) as ChildProcess;
     return mockProcess;
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
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,
import { EventEmitter } from 'node:events';
import type { ChildProcess } from 'node:child_process';
export function createMockChildProcess(overrides?: Partial<ChildProcess>): ChildProcess {
const emitter = new EventEmitter();
let killed = false;
let connected = false;
const mockProcess = Object.assign(emitter, {
pid: undefined,
stdio: [null, null, null, null, null],
stdin: null,
stdout: null,
stderr: null,
exitCode: null,
get killed() {
return killed;
},
get connected() {
return connected;
},
signalCode: null,
spawnargs: [],
spawnfile: '',
kill: () => {
killed = true;
return true;
},
send: () => true,
disconnect: () => {
connected = false;
},
[Symbol.dispose]: () => {
killed = true;
},
...overrides
}) as ChildProcess;
return mockProcess;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/kernels/deepnote/deepnoteTestHelpers.node.ts` around lines 15 - 66,
Replace the hand-rolled mock object with one that extends Node's EventEmitter
and uses real emitter methods (emit/on/once/removeListener/listeners/etc.) and
mutable internal state exposed via getters for properties like killed and
connected; specifically update the mock that currently defines killed,
connected, signalCode, spawnargs, spawnfile and methods kill, send, disconnect,
unref, ref and all the listener stubs so the mock class (or factory) wraps
EventEmitter, maintains internal booleans (e.g., _killed, _connected) that are
updated by kill/disconnect/ref/unref and exposed as getters killed and
connected, forward listener methods to the EventEmitter instance (or inherit
them) and remove the hardcoded stub implementations and the Symbol.dispose no-op
in favor of proper EventEmitter behavior.

...overrides
} as ChildProcess;
};
return mockProcess;
}
37 changes: 33 additions & 4 deletions src/notebooks/deepnote/deepnoteActivationService.ts
Original file line number Diff line number Diff line change
@@ -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';

/**
Expand All @@ -17,6 +19,8 @@ import { SnapshotService } from './snapshots/snapshotService';
*/
@injectable()
export class DeepnoteActivationService implements IExtensionSyncActivationService {
private autoSplitter: DeepnoteAutoSplitter;

private editProtection: DeepnoteInputBlockEditProtection;

private explorerView: DeepnoteExplorerView;
Expand Down Expand Up @@ -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);
})
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
this.extensionContext.subscriptions.push(
workspace.onDidChangeConfiguration((event) => {
if (event.affectsConfiguration('deepnote.snapshots.enabled')) {
Expand All @@ -70,6 +80,25 @@ export class DeepnoteActivationService implements IExtensionSyncActivationServic
this.integrationManager.activate();
}

private async checkAndSplitIfNeeded(doc: import('vscode').NotebookDocument): Promise<void> {
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);
Comment on lines +89 to +98

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Skip dirty notebooks before auto-splitting.

This path reads from disk and can rewrite doc.uri, so an already-open dirty notebook gets split from stale bytes instead of the in-memory document. That can leave the open editor and the saved .deepnote file out of sync.

Possible fix
     private async checkAndSplitIfNeeded(doc: import('vscode').NotebookDocument): Promise<void> {
         if (doc.notebookType !== 'deepnote') {
             return;
         }
+
+        if (doc.isDirty) {
+            this.logger.debug(`Skipping auto-split for dirty notebook: ${doc.uri.path}`);
+            return;
+        }
 
         try {
             const fileUri = doc.uri;
             const content = await workspace.fs.readFile(fileUri);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/notebooks/deepnote/deepnoteActivationService.ts` around lines 89 - 98,
The checkAndSplitIfNeeded path reads the on-disk file and can overwrite doc.uri
contents, so avoid operating on an in-memory dirty notebook: inside
checkAndSplitIfNeeded, add an early return when the NotebookDocument is dirty
(check doc.isDirty) before calling workspace.fs.readFile /
deserializeDeepnoteFile / autoSplitter.splitIfNeeded; this ensures you only read
from disk and auto-split for clean documents and prevents the editor and saved
.deepnote file from getting out of sync.


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();
Expand Down
Loading
Loading