Add Deepnote Environments kernel management

[tkislan]

Fixes #

Summary by CodeRabbit

• New Features

  • Environment management view with create, delete, edit, and package management capabilities
  • Kernel status indicator displaying active environment information
  • Automatic kernel restart when integration configurations change
  • Environment-scoped server lifecycle management

• Documentation

  • Added comprehensive debugging guide for kernel management
  • Removed orphan process cleanup documentation

• Improvements

  • Enhanced port information display showing both Jupyter and LSP ports
  • Expanded spell-check word list

[tkislan]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

Actionable comments posted: 11

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/kernels/deepnote/deepnoteServerStarter.node.ts (1)

610-615: Await the pending operation, not the struct

Promise.race([op, …]) resolves immediately because op is the wrapper object, not the async work. We never wait for inflight start/stop during dispose, so background processes can outlive teardown. Pass op.promise to race instead.

-            await Promise.allSettled(pendingOps.map((op) => Promise.race([op, sleep(2000)])));
+            await Promise.allSettled(
+                pendingOps.map((op) => Promise.race([op.promise, sleep(2000)]))
+            );

♻️ Duplicate comments (11)

src/kernels/deepnote/environments/deepnoteEnvironmentStorage.unit.test.ts (1)

40-41: Fix assert.deepStrictEqual - method doesn't exist in Chai.

Chai's assert lacks deepStrictEqual. Use assert.deepEqual instead, or import Node's assert module.

-            assert.deepStrictEqual(configs, []);
+            assert.deepEqual(configs, []);

Also applies to line 117.

Also applies to: 116-117

src/notebooks/controllers/vscodeNotebookController.ts (1)

300-337: Deepnote swaps still miss kernel changes.

areKernelConnectionsEqual only checks id, so Deepnote environment switches (same id, new interpreter/server) leave hasChanged false and we never dispose the stale kernel. Restore the richer field comparison suggested earlier or bypass the helper for Deepnote.

DEBUGGING_KERNEL_MANAGEMENT.md (1)

22-262: Fix repeated markdownlint violations.

Code fences still sit flush against surrounding text and lack language tags (``` at Lines 23, 39, 59, etc.), so MD031/MD040 keep firing; several headings remain unspaced (MD022). Please add blank lines around every fence, supply appropriate languages, and space the headings, then rerun markdownlint.

src/kernels/deepnote/environments/deepnoteNotebookEnvironmentMapper.node.ts (1)

30-90: Stop keying by fsPath.
Remote and untitled notebooks share an empty fsPath, so mappings collide or disappear and environments can’t be restored. Serialize the full URI (without query/fragment) to key the map and parse it back when returning notebook URIs.

-    private mappings: Map<string, string>; // notebookUri.fsPath -> environmentId
+    private mappings: Map<string, string>; // serialized notebook URI -> environmentId
@@
-        const key = notebookUri.fsPath;
+        const key = DeepnoteNotebookEnvironmentMapper.serializeNotebookUri(notebookUri);
         return this.mappings.get(key);
@@
-        const key = notebookUri.fsPath;
+        const key = DeepnoteNotebookEnvironmentMapper.serializeNotebookUri(notebookUri);
         this.mappings.set(key, environmentId);
         await this.saveMappings();
-        logger.info(`Mapped notebook ${notebookUri.fsPath} to environment ${environmentId}`);
+        logger.info(`Mapped notebook ${key} to environment ${environmentId}`);
@@
-        const key = notebookUri.fsPath;
+        const key = DeepnoteNotebookEnvironmentMapper.serializeNotebookUri(notebookUri);
         this.mappings.delete(key);
         await this.saveMappings();
-        logger.info(`Removed environment mapping for notebook ${notebookUri.fsPath}`);
+        logger.info(`Removed environment mapping for notebook ${key}`);
@@
-        for (const [notebookPath, configId] of this.mappings.entries()) {
+        for (const [serializedNotebook, configId] of this.mappings.entries()) {
             if (configId === environmentId) {
-                notebooks.push(Uri.file(notebookPath));
+                notebooks.push(DeepnoteNotebookEnvironmentMapper.deserializeNotebookUri(serializedNotebook));
             }
         }
         return notebooks;
@@
     private async saveMappings(): Promise<void> {
         const obj = Object.fromEntries(this.mappings.entries());
         await this.workspaceState.update(DeepnoteNotebookEnvironmentMapper.STORAGE_KEY, obj);
     }
+
+    private static serializeNotebookUri(notebookUri: Uri): string {
+        return notebookUri.with({ query: '', fragment: '' }).toString(true);
+    }
+
+    private static deserializeNotebookUri(serialized: string): Uri {
+        try {
+            return Uri.parse(serialized, true);
+        } catch {
+            return Uri.file(serialized);
+        }
+    }

src/kernels/deepnote/environments/deepnoteEnvironmentManager.node.ts (2)

2-176: Delete venv on environment removal.
deleteEnvironment drops the in-memory record but leaves the venv directory on disk, so every removal leaks storage. Remove the venv before deleting the entry (log failures).

-import { CancellationToken, EventEmitter, l10n, Uri } from 'vscode';
+import { CancellationToken, EventEmitter, l10n, Uri, workspace } from 'vscode';
@@
         const config = this.environments.get(id);
         if (!config) {
             throw new Error(`Environment not found: ${id}`);
         }
@@
-        Cancellation.throwIfCanceled(token);
-
-        this.environments.delete(id);
+        Cancellation.throwIfCanceled(token);
+
+        try {
+            await workspace.fs.delete(config.venvPath, { recursive: true, useTrash: false });
+        } catch (err) {
+            logger.warn(`Failed to delete venv for environment ${config.name} (${id})`, err);
+        }
+
+        this.environments.delete(id);
         await this.persistEnvironments();
         this._onDidChangeEnvironments.fire();

---

203-206: Do not dispose shared output channel.
STANDARD_OUTPUT_CHANNEL is shared; disposing it here breaks other services still using it. Let the owner manage its lifecycle.

-        this.outputChannel.dispose();
         this._onDidChangeEnvironments.dispose();

src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts (1)

278-553: Remove placeholder “documentation” tests.

These cases still only call assert.ok(true, …) and never exercise the selector, so they add zero signal while masking real gaps. Please drop them or replace each with a meaningful test (or mark skipped with a TODO) rather than leaving them as passing no-ops.

src/kernels/deepnote/environments/deepnoteEnvironmentTreeItem.node.ts (1)

93-117: Return MarkdownString so the tooltip renders.

The tooltip still returns Markdown markup as a plain string, so VS Code shows the raw ** characters. Wrap the content in a MarkdownString (and import it) so formatting renders correctly.

-import { l10n, ThemeIcon, TreeItem, TreeItemCollapsibleState } from 'vscode';
+import { l10n, MarkdownString, ThemeIcon, TreeItem, TreeItemCollapsibleState } from 'vscode';
@@
-    private buildTooltip(): string {
+    private buildTooltip(): MarkdownString {
         if (!this.environment) {
-            return '';
+            return new MarkdownString('', true);
         }
 
         const lines: string[] = [];
@@
-        return lines.join('\n');
+        return new MarkdownString(lines.join('\n'), true);
     }

src/kernels/deepnote/environments/deepnoteEnvironmentTreeDataProvider.node.ts (1)

1-140: Localize environment info labels

Environment detail strings are still hard-coded, so the tree view ignores locale packs. Please wrap them with l10n.t(...) and add the missing import. As per coding guidelines.

-import { Disposable, Event, EventEmitter, TreeDataProvider, TreeItem } from 'vscode';
+import { Disposable, Event, EventEmitter, TreeDataProvider, TreeItem, l10n } from 'vscode';
@@
-                `Python: ${config.pythonInterpreter.uri.fsPath}`,
+                l10n.t('Python: {0}', config.pythonInterpreter.uri.fsPath),
@@
-            DeepnoteEnvironmentTreeItem.createInfoItem('venv', config.id, `Venv: ${config.venvPath.fsPath}`, 'folder')
+            DeepnoteEnvironmentTreeItem.createInfoItem(
+                'venv',
+                config.id,
+                l10n.t('Venv: {0}', config.venvPath.fsPath),
+                'folder'
+            )
@@
-                `Created: ${config.createdAt.toLocaleString()}`,
+                l10n.t('Created: {0}', config.createdAt.toLocaleString()),
@@
-                `Last used: ${config.lastUsedAt.toLocaleString()}`,
+                l10n.t('Last used: {0}', config.lastUsedAt.toLocaleString()),

src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts (1)

530-579: Allow clearing environment packages

Blank input should clear the package list, but the validator rejects it and the handler treats '' as cancel, so users can’t remove packages. Accept empty input and persist an empty array.

-            validateInput: (value: string) => {
-                if (!value || value.trim().length === 0) {
-                    return l10n.t('Please enter at least one package');
-                }
+            validateInput: (value: string) => {
+                const trimmed = value.trim();
+                if (trimmed.length === 0) {
+                    return undefined;
+                }
                 const packages = value.split(',').map((p: string) => p.trim());
@@
-        if (!packagesInput) {
+        if (packagesInput === undefined) {
             return;
         }
 
-        const packages = packagesInput
-            .split(',')
-            .map((p: string) => p.trim())
-            .filter((p: string) => p.length > 0);
+        const trimmed = packagesInput.trim();
+        const packages =
+            trimmed.length === 0
+                ? []
+                : trimmed.split(',').map((p: string) => p.trim()).filter((p: string) => p.length > 0);

src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts (1)

498-677: Localize progress notifications

Progress notifications shown to users must go through l10n.t. These literals remain untranslated.

-        progress.report({ message: `Using ${configuration.name}...` });
+        progress.report({ message: l10n.t('Using {0}...', configuration.name) });
…
-        progress.report({ message: 'Starting Deepnote server...' });
+        progress.report({ message: l10n.t('Starting Deepnote server...') });
…
-        progress.report({ message: 'Connecting to kernel...' });
+        progress.report({ message: l10n.t('Connecting to kernel...') });
…
-        progress.report({ message: 'Finalizing kernel setup...' });
+        progress.report({ message: l10n.t('Finalizing kernel setup...') });
…
-        progress.report({ message: 'Kernel ready!' });
+        progress.report({ message: l10n.t('Kernel ready!') });
…
-                progress.report({ message: 'Creating requirements.txt...' });
+                progress.report({ message: l10n.t('Creating requirements.txt...') });

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 90bfefd and 2989525.

⛔ Files ignored due to path filters (1)

• package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (53)

• .vscode/launch.json (1 hunks)
• CLAUDE.md (2 hunks)
• DEBUGGING_KERNEL_MANAGEMENT.md (1 hunks)
• KERNEL_RESTART_ON_INTEGRATION_CHANGE.md (1 hunks)
• ORPHAN_PROCESS_CLEANUP_IMPLEMENTATION.md (0 hunks)
• cspell.json (2 hunks)
• package.json (7 hunks)
• package.nls.json (1 hunks)
• src/kernels/deepnote/deepnoteServerProvider.node.ts (1 hunks)
• src/kernels/deepnote/deepnoteServerStarter.node.ts (20 hunks)
• src/kernels/deepnote/deepnoteSharedToolkitInstaller.node.ts (5 hunks)
• src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (13 hunks)
• src/kernels/deepnote/environments/deepnoteEnvironment.ts (1 hunks)
• src/kernels/deepnote/environments/deepnoteEnvironmentManager.node.ts (1 hunks)
• src/kernels/deepnote/environments/deepnoteEnvironmentManager.unit.test.ts (1 hunks)
• src/kernels/deepnote/environments/deepnoteEnvironmentStorage.node.ts (1 hunks)
• src/kernels/deepnote/environments/deepnoteEnvironmentStorage.unit.test.ts (1 hunks)
• src/kernels/deepnote/environments/deepnoteEnvironmentTreeDataProvider.node.ts (1 hunks)
• src/kernels/deepnote/environments/deepnoteEnvironmentTreeDataProvider.unit.test.ts (1 hunks)
• src/kernels/deepnote/environments/deepnoteEnvironmentTreeItem.node.ts (1 hunks)
• src/kernels/deepnote/environments/deepnoteEnvironmentTreeItem.unit.test.ts (1 hunks)
• src/kernels/deepnote/environments/deepnoteEnvironmentsActivationService.ts (1 hunks)
• src/kernels/deepnote/environments/deepnoteEnvironmentsActivationService.unit.test.ts (1 hunks)
• src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts (1 hunks)
• src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts (1 hunks)
• src/kernels/deepnote/environments/deepnoteNotebookEnvironmentMapper.node.ts (1 hunks)
• src/kernels/deepnote/types.ts (6 hunks)
• src/kernels/helpers.ts (2 hunks)
• src/kernels/kernel.ts (1 hunks)
• src/notebooks/controllers/vscodeNotebookController.ts (3 hunks)
• src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts (5 hunks)
• src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts (1 hunks)
• src/notebooks/deepnote/deepnoteKernelStatusIndicator.node.ts (1 hunks)
• src/notebooks/deepnote/deepnoteKernelStatusIndicator.ts (1 hunks)
• src/notebooks/deepnote/deepnoteProjectUtils.ts (1 hunks)
• src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts (5 hunks)
• src/notebooks/deepnote/integrations/integrationKernelRestartHandler.ts (1 hunks)
• src/notebooks/deepnote/integrations/integrationKernelRestartHandler.unit.test.ts (1 hunks)
• src/notebooks/serviceRegistry.node.ts (3 hunks)
• src/notebooks/serviceRegistry.web.ts (2 hunks)
• src/platform/common/application/extension.node.unit.test.ts (5 hunks)
• src/platform/common/cancellation.ts (2 hunks)
• src/platform/common/constants.ts (1 hunks)
• src/platform/common/utils.ts (1 hunks)
• src/platform/common/utils/localize.ts (2 hunks)
• src/platform/deepnote/deepnoteProjectUtils.ts (1 hunks)
• src/platform/deepnote/deepnoteServerUtils.node.ts (1 hunks)
• src/platform/interpreter/display/visibilityFilter.node.ts (2 hunks)
• src/platform/logging/index.ts (1 hunks)
• src/standalone/context/activeEditorContext.ts (3 hunks)
• src/standalone/import-export/importTracker.ts (2 hunks)
• src/test/constants.ts (1 hunks)
• src/webviews/extension-side/variablesView/notebookWatcher.ts (3 hunks)

💤 Files with no reviewable changes (1)

• ORPHAN_PROCESS_CLEANUP_IMPLEMENTATION.md

🧰 Additional context used

📓 Path-based instructions (8)

src/platform/**/*.ts

📄 CodeRabbit inference engine (.github/instructions/platform.instructions.md)

src/platform/**/*.ts: Use Inversify decorators for DI: annotate classes with @Injectable() and inject dependencies with @Inject(Token)
Use the centralized logger (import { logger } from '../platform/logging') instead of console.log for application logging

Files:

• src/platform/common/cancellation.ts
• src/platform/common/application/extension.node.unit.test.ts
• src/platform/deepnote/deepnoteServerUtils.node.ts
• src/platform/common/utils/localize.ts
• src/platform/interpreter/display/visibilityFilter.node.ts
• src/platform/common/utils.ts
• src/platform/deepnote/deepnoteProjectUtils.ts
• src/platform/common/constants.ts
• src/platform/logging/index.ts

src/platform/common/application/**/*.ts

📄 CodeRabbit inference engine (.github/instructions/platform.instructions.md)

Wrap VS Code APIs behind application service interfaces (e.g., IWorkspaceService, IDebugService, IApplicationEnvironment, IEncryptedStorage)

Files:

• src/platform/common/application/extension.node.unit.test.ts

**/*.{test,spec}.ts

📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)

In unit tests, when a mock is returned from a promise, ensure the mocked instance has an undefined then property to avoid hanging tests

Files:

• src/platform/common/application/extension.node.unit.test.ts
• src/notebooks/deepnote/integrations/integrationKernelRestartHandler.unit.test.ts
• src/kernels/deepnote/environments/deepnoteEnvironmentTreeDataProvider.unit.test.ts
• src/kernels/deepnote/environments/deepnoteEnvironmentsActivationService.unit.test.ts
• src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts
• src/kernels/deepnote/environments/deepnoteEnvironmentManager.unit.test.ts
• src/kernels/deepnote/environments/deepnoteEnvironmentStorage.unit.test.ts
• src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts
• src/kernels/deepnote/environments/deepnoteEnvironmentTreeItem.unit.test.ts

src/platform/**/*.node.ts

📄 CodeRabbit inference engine (.github/instructions/platform.instructions.md)

src/platform/**/*.node.ts: Provide Node.js-specific implementations in .node.ts files (e.g., FileSystem with fs-extra, PlatformService with OS detection)
Desktop implementations may use Node capabilities (fs-extra, process spawning, native modules) consistent with their responsibilities

Files:

• src/platform/deepnote/deepnoteServerUtils.node.ts
• src/platform/interpreter/display/visibilityFilter.node.ts

src/kernels/**/*.ts

📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)

src/kernels/**/*.ts: Use event-driven updates (EventEmitter) for state changes
Monitor and dispose pending promises to prevent leaks during teardown
Use WeakRef/weak references for notebook/object backreferences to avoid memory leaks
Respect CancellationToken in all async operations

Files:

• src/kernels/deepnote/environments/deepnoteEnvironmentTreeDataProvider.unit.test.ts
• src/kernels/helpers.ts
• src/kernels/deepnote/environments/deepnoteEnvironmentsActivationService.ts
• src/kernels/deepnote/environments/deepnoteNotebookEnvironmentMapper.node.ts
• src/kernels/deepnote/environments/deepnoteEnvironmentsActivationService.unit.test.ts
• src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts
• src/kernels/deepnote/environments/deepnoteEnvironmentTreeItem.node.ts
• src/kernels/deepnote/deepnoteSharedToolkitInstaller.node.ts
• src/kernels/deepnote/environments/deepnoteEnvironment.ts
• src/kernels/deepnote/environments/deepnoteEnvironmentManager.unit.test.ts
• src/kernels/deepnote/environments/deepnoteEnvironmentStorage.node.ts
• src/kernels/kernel.ts
• src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts
• src/kernels/deepnote/environments/deepnoteEnvironmentStorage.unit.test.ts
• src/kernels/deepnote/types.ts
• src/kernels/deepnote/environments/deepnoteEnvironmentManager.node.ts
• src/kernels/deepnote/deepnoteServerStarter.node.ts
• src/kernels/deepnote/deepnoteServerProvider.node.ts
• src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
• src/kernels/deepnote/environments/deepnoteEnvironmentTreeDataProvider.node.ts
• src/kernels/deepnote/environments/deepnoteEnvironmentTreeItem.unit.test.ts

src/platform/logging/**/*.ts

📄 CodeRabbit inference engine (.github/instructions/platform.instructions.md)

Implement logging using OutputChannelLogger/ConsoleLogger and honor LogLevel; ensure PII scrubbing in log output

Files:

• src/platform/logging/index.ts

src/kernels/kernel.ts

📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)

Kernel must manage sessions via IKernelSession and implement lifecycle hooks (willRestart, willInterrupt, etc.) with startup code execution (e.g., matplotlib, IPyWidgets)

Files:

• src/kernels/kernel.ts

src/kernels/{kernel.ts,kernelSessionFactory.ts}

📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)

Prefer session reuse to avoid unnecessary kernel restarts

Files:

• src/kernels/kernel.ts

🪛 markdownlint-cli2 (0.18.1)

DEBUGGING_KERNEL_MANAGEMENT.md

23-23: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)

---

23-23: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

---

39-39: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)

---

39-39: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

---

59-59: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)

---

59-59: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

---

86-86: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

---

101-101: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

---

119-119: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

---

138-138: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

---

156-156: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

---

180-180: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

---

199-199: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

---

226-226: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)

---

245-245: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)

---

254-254: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)

---

254-254: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

---

261-261: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)

---

445-445: Files should end with a single newline character

(MD047, single-trailing-newline)

KERNEL_RESTART_ON_INTEGRATION_CHANGE.md

1-1: First line in a file should be a top-level heading

(MD041, first-line-heading, first-line-h1)

🔇 Additional comments (43)

cspell.json (1)

48-48: LGTM. Three common Python packages added to the spell-check allowlist in alphabetical order. Contextually appropriate for environment and package management features.

Also applies to: 60-60, 61-61

src/kernels/kernel.ts (1)

488-488: LGTM – useful startup logging.

The added log captures session initialization. Logging both display path and raw URI provides full context, though consider whether both forms are needed.

src/platform/common/utils/localize.ts (2)

42-42: Output channel rebranded to Deepnote.

The change correctly updates the display string while preserving the export key.

---

709-709: Rebranding is intentional and correct.

The changes rebrand user-facing display strings (OutputChannelNames and console labels) to "Deepnote" while preserving technical references to Jupyter protocols and components—an appropriate selective pattern. No issues.

src/platform/deepnote/deepnoteServerUtils.node.ts (1)

1-2: LGTM.

Standard import, correctly used.

src/notebooks/deepnote/deepnoteProjectUtils.ts (1)

16-32: Solid implementation.

Input validation, type narrowing, and normalization logic are all correct. The function name uses "hash" for a string representation rather than a cryptographic hash, but the JSDoc clarifies this intent.

.vscode/launch.json (1)

12-12: LGTM: Extension-specific proposed API flag.

Correctly specifies the Deepnote extension identifier for proposed APIs.

src/test/constants.ts (1)

4-4: LGTM: Extension ID updated for tests.

Consistent with the extension rebranding.

package.nls.json (1)

278-289: LGTM: Environment localization keys added.

New localization strings follow established naming conventions and support the environment management UI.

src/kernels/deepnote/deepnoteSharedToolkitInstaller.node.ts (3)

5-5: LGTM: Localization support added.

---

101-101: LGTM: Structured logging improvements.

Passing error objects and stdout/stderr as structured data improves observability.

Also applies to: 105-105, 183-183

---

201-201: LGTM: User-facing messages localized.

Proper use of l10n.t() for internationalization. Error handling correctly separates user message from full error logging.

Also applies to: 243-243, 247-247, 251-253

CLAUDE.md (1)

6-6: LGTM: Development guidelines enhanced.

Useful conventions added for uuid imports and pre-commit formatting.

Also applies to: 11-11

src/platform/common/application/extension.node.unit.test.ts (1)

14-17: LGTM: Test fixtures updated for rebranding.

Test data consistently reflects the Deepnote extension identity. Test logic unchanged.

Also applies to: 27-29, 60-62, 74-75, 79-80, 85-86, 89-92

src/kernels/deepnote/environments/deepnoteEnvironment.ts (3)

9-60: LGTM: Well-designed runtime environment model.

Clear field documentation and appropriate use of rich types (Uri, PythonEnvironment, Date).

---

66-79: LGTM: Serializable state properly designed.

Correctly uses primitive types (strings) for JSON persistence while maintaining structure.

---

84-89: LGTM: Creation options interface.

Clean options interface for environment creation.

src/platform/deepnote/deepnoteProjectUtils.ts (1)

3-5: No issues found.

The function correctly removes query/fragment parameters from notebook URIs. The single call site at src/notebooks/controllers/vscodeNotebookController.ts:326 uses it appropriately—normalizing the path before comparing against the kernel connection's project file path. The transformation behavior matches expectations.

package.json (4)

102-136: LGTM: Environment commands properly structured.

All six new commands follow the existing patterns with appropriate icons and localization keys.

---

2244-2248: LGTM: Environment view properly configured.

View correctly shows only when workspace folders are present.

---

959-971: Verify toolbar reordering impact.

The notebook toolbar navigation groups have been renumbered (navigation@0 through navigation@12). The new environment selector is placed first at navigation@0. Ensure this reordering doesn't disrupt existing user workflows or conflict with other extensions that rely on specific toolbar positions.

---

2589-2589: UUID dependency added but not used in codebase.

The npm uuid package has been added to dependencies, but the codebase generates UUIDs via its own local implementation in src/platform/common/uuid.ts, which uses native crypto.randomUUID(). No TypeScript files import or use the npm uuid package directly. Either remove the unused dependency, or clarify if this is intentional for future use.

Likely an incorrect or invalid review comment.

src/kernels/deepnote/types.ts (6)

12-15: LGTM: New type exposes toolkit version.

VenvAndToolkitInstallation properly exposes both the Python interpreter and toolkit version, supporting version tracking for environments.

---

102-118: LGTM: Toolkit installer API refactored to environment-based model.

The method rename from ensureInstalled to ensureVenvAndToolkit and the shift from file-based to environment-based parameters (baseInterpreter + venvPath) align with the environment-centric architecture. The new installAdditionalPackages method enables dynamic package management.

---

170-175: LGTM: Server info extended with dual ports.

Replacing single port with jupyterPort and lspPort supports the dual-server model for Jupyter and LSP. This is a breaking change but aligns with the architecture.

---

227-288: LGTM: Environment manager interface is comprehensive.

The interface provides complete CRUD operations, event-driven updates (per coding guidelines), initialization lifecycle, and usage tracking. Well-designed API surface.

---

290-318: LGTM: Notebook-environment mapper interface is well-designed.

The bidirectional mapping (notebook→environment and environment→notebooks) enables both lookup and cleanup scenarios. URI normalization is properly documented in comments.

---

200-225: LGTM: Kernel auto-selector extended for environment lifecycle.

The addition of progress reporting, boolean return values, and new methods (rebuildController, clearControllerForEnvironment) properly support environment switching and cleanup operations.

src/platform/common/cancellation.ts (1)

9-9: LGTM: Cancellation logging improves debuggability.

Adding centralized logging before throwing CancellationError helps trace cancellation flows. Info level is appropriate.

Also applies to: 124-124

src/notebooks/serviceRegistry.web.ts (1)

54-54: LGTM: Integration kernel restart handler properly registered.

The new activation service follows existing patterns and is correctly registered as a singleton.

Also applies to: 125-128

src/kernels/deepnote/deepnoteServerProvider.node.ts (1)

88-88: LGTM: Server label now shows both ports.

The updated label format clearly displays both Jupyter and LSP ports, improving transparency about the server configuration.

src/standalone/import-export/importTracker.ts (1)

136-136: LGTM: Import tracking extended to Deepnote notebooks.

The conditional correctly includes Deepnote notebooks alongside Jupyter notebooks for import telemetry tracking.

src/webviews/extension-side/variablesView/notebookWatcher.ts (1)

12-12: LGTM!

The addition of isDeepnoteNotebook checks alongside existing Jupyter checks is consistent and correct.

Also applies to: 113-114, 177-177

src/standalone/context/activeEditorContext.ts (2)

122-122: LGTM!

Logging the active notebook URI aids debugging of editor context changes.

---

190-194: LGTM!

Deepnote notebook support correctly added to kernel selection context.

src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts (1)

34-34: LGTM!

Unified cancellation handling using Cancellation.throwIfCanceled is cleaner than scattered early-return checks. Well-placed at async boundaries.

Also applies to: 64-64, 115-115, 148-148

src/kernels/helpers.ts (1)

303-314: LGTM!

Display name hierarchy (projectName → environmentName → fallback) is logical and provides good UX.

src/kernels/deepnote/environments/deepnoteEnvironmentsActivationService.unit.test.ts (1)

38-48: LGTM!

Error handling test correctly verifies graceful degradation when initialization fails.

src/kernels/deepnote/environments/deepnoteEnvironmentStorage.unit.test.ts (2)

43-74: LGTM!

Serialization/deserialization test correctly validates round-trip persistence of environment state including URI conversions.

---

122-160: LGTM!

Save test correctly verifies serialization format with proper URI string conversion.

src/kernels/deepnote/environments/deepnoteEnvironmentTreeItem.unit.test.ts (3)

24-31: LGTM!

Environment item creation test validates correct initialization of tree item properties.

---

33-37: LGTM!

Description test ensures last-used timestamp is included in UI display.

---

113-175: LGTM!

Relative time formatting tests cover all relevant time ranges with appropriate assertions.

[saltenasl]

@coderabbitai resolve we're going to fix these in follow up PRs

[saltenasl]

@coderabbitai resolve

[coderabbitai]

✅ Actions performed

Comments resolved and changes approved.

[coderabbitai]

@saltenasl, understood. I'll resolve the comments. Would you like me to create GitHub issues for the items to be addressed in follow-up PRs?

✅ Actions performed

Comments resolved and changes approved.