feat: Better error messages when installing the Deepnote kernel. #77
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a structured Deepnote kernel error hierarchy and uses those error types across installer and server code. Toolkit installer methods now throw typed errors (DeepnoteVenvCreationError, DeepnoteToolkitInstallError) instead of returning undefined. Server starter tracks per-file stdout/stderr (keeps last 5000 chars), includes captured output in DeepnoteServerTimeoutError and DeepnoteServerStartupError, and cleans up per-file tracking on stop/dispose. DeepnoteKernelAutoSelector now depends on an output channel and handles kernel-selection failures by logging and offering “Show Output” / “Copy Error Details” flows. Sequence Diagram(s)sequenceDiagram
participant Caller
participant Installer as ToolkitInstaller
participant Errors as DeepnoteErrors
Caller->>Installer: ensureInstalled(baseInterpreter, fileUri)
Installer->>Installer: create venv
alt venv creation fails
Installer->>Errors: throw DeepnoteVenvCreationError(pythonPath, venvPath, stderr, cause)
Errors-->>Caller: DeepnoteVenvCreationError
else install toolkit
Installer->>Installer: run pip install
alt install fails
Installer->>Errors: throw DeepnoteToolkitInstallError(pythonPath, venvPath, packageUrl, stdout, stderr, cause)
Errors-->>Caller: DeepnoteToolkitInstallError
else success
Installer-->>Caller: PythonEnvironment
end
end
sequenceDiagram
participant Caller
participant ServerStarter
participant Process as ServerProcess
Caller->>ServerStarter: startServer(file)
ServerStarter->>ServerStarter: init serverOutputByFile[file]={stdout:'',stderr:''}
ServerStarter->>Process: spawn server
Process-->>ServerStarter: stdout/stderr
ServerStarter->>ServerStarter: append & trim outputs (last 5000 chars)
alt health check succeeds
ServerStarter-->>Caller: server ready
else timeout/startup error
ServerStarter->>ServerStarter: build error with captured output
ServerStarter-->>Caller: throw DeepnoteServerTimeoutError / DeepnoteServerStartupError
end
Caller->>ServerStarter: stopServer(file)
ServerStarter->>ServerStarter: remove serverOutputByFile[file]
sequenceDiagram
participant User
participant AutoSelector
participant Output as OutputChannel
participant Clipboard
participant UI as VSCode UI
User->>AutoSelector: trigger kernel selection
AutoSelector->>AutoSelector: attempt load
alt success
AutoSelector-->>User: kernel ready
else failure
AutoSelector->>AutoSelector: handleKernelSelectionError(err)
alt err is DeepnoteKernelError
AutoSelector->>Output: append(err.getErrorReport())
AutoSelector->>UI: show options [Show Output, Copy Error Details]
alt Show Output
AutoSelector->>Output: reveal()
else Copy Error Details
AutoSelector->>Clipboard: writeText(err.getErrorReport())
end
else generic error
AutoSelector->>Output: append(err.toString())
AutoSelector->>UI: show message [Show Output]
alt Show Output
AutoSelector->>Output: reveal()
end
end
end
Possibly related PRs
Suggested reviewers
Pre-merge checks✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #77 +/- ##
=====================================
Coverage 71% 71%
=====================================
Files 527 527
Lines 39474 39474
Branches 4933 4933
=====================================
Hits 28277 28277
Misses 9563 9563
Partials 1634 1634 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
src/kernels/deepnote/deepnoteServerStarter.node.ts(6 hunks)src/kernels/deepnote/deepnoteToolkitInstaller.node.ts(3 hunks)src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts(6 hunks)src/platform/errors/deepnoteKernelErrors.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.node.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
*.node.tsfor Desktop-specific implementations that require full file system access and Python environments
Files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.tssrc/kernels/deepnote/deepnoteToolkitInstaller.node.tssrc/kernels/deepnote/deepnoteServerStarter.node.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Inject interfaces, not concrete classes
Avoid circular dependencies
Usel10n.t()for user-facing strings
Use typed error classes fromsrc/platform/errors/when throwing or handling errors
Use theILoggerservice instead ofconsole.log
Preserve error details while scrubbing PII in messages and telemetry
Include the Microsoft copyright header in source files
Prefer async/await and handle cancellation withCancellationToken
**/*.{ts,tsx}: Order class members (methods, fields, properties) first by accessibility (public/protected/private) and then alphabetically
Do not add the Microsoft copyright header to new files
Use Uri.joinPath() to construct file paths instead of manual string concatenation
Add a blank line after groups of const declarations and before return statements for readability
Separate third-party imports from local file imports
Files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.tssrc/kernels/deepnote/deepnoteToolkitInstaller.node.tssrc/platform/errors/deepnoteKernelErrors.tssrc/kernels/deepnote/deepnoteServerStarter.node.ts
src/notebooks/deepnote/**
📄 CodeRabbit inference engine (CLAUDE.md)
Deepnote integration code resides under src/notebooks/deepnote/
Files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.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/deepnoteToolkitInstaller.node.tssrc/kernels/deepnote/deepnoteServerStarter.node.ts
**/!(*.node|*.web).ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place shared cross-platform logic in common
.tsfiles (not suffixed with.nodeor.web)
Files:
src/platform/errors/deepnoteKernelErrors.ts
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/errors/deepnoteKernelErrors.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/interactiveWindow.instructions.md:0-0
Timestamp: 2025-09-03T12:54:35.368Z
Learning: Applies to src/interactive-window/{interactiveWindow.ts,interactiveWindowController.ts} : Implement robust kernel error handling: automatic restart, clear error display in system info cells, graceful degradation when kernels are unavailable, and user notifications for critical failures
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel.instructions.md:0-0
Timestamp: 2025-09-03T12:58:18.091Z
Learning: Applies to src/kernels/errors/**/*.ts : Use specific error types (e.g., KernelDeadError, KernelConnectionTimeoutError, KernelInterruptTimeoutError, JupyterInvalidKernelError, KernelDependencyError) for failure cases
Learnt from: dinohamzic
PR: deepnote/deepnote#18378
File: apps/webapp/server/api/landing.ts:130-136
Timestamp: 2025-09-10T08:51:54.455Z
Learning: In the Deepnote codebase, prefer explicit error messages over using raw error messages (like err.message) when handling errors that are exposed to customers, for security and user experience reasons.
📚 Learning: 2025-09-03T12:59:14.489Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/notebooks.instructions.md:0-0
Timestamp: 2025-09-03T12:59:14.489Z
Learning: Applies to src/notebooks/controllers/kernelSource/**/*.ts : For new kernel sources, implement ILocalNotebookKernelSourceSelector (or similar)
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-10-09T11:21:57.494Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-09T11:21:57.494Z
Learning: Applies to src/notebooks/deepnote/deepnoteActivationService.ts : deepnoteActivationService.ts manages VS Code activation for Deepnote
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-03T12:58:18.091Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel.instructions.md:0-0
Timestamp: 2025-09-03T12:58:18.091Z
Learning: Applies to src/kernels/kernelProvider.base.ts : KernelProviderBase must create/manage IKernel instances, track by NotebookDocument, manage lifecycle (start/restart/dispose), and expose INotebookKernelExecution
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-03T12:59:14.489Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/notebooks.instructions.md:0-0
Timestamp: 2025-09-03T12:59:14.489Z
Learning: Applies to src/notebooks/controllers/kernelSource/**/*.ts : Create a UI provider for kernel selection for the new kernel source
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-03T12:59:14.489Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/notebooks.instructions.md:0-0
Timestamp: 2025-09-03T12:59:14.489Z
Learning: Applies to src/notebooks/controllers/**/*KernelConnector*.ts : KernelConnector must present user-friendly error messages for connection failures
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-03T12:54:35.368Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/interactiveWindow.instructions.md:0-0
Timestamp: 2025-09-03T12:54:35.368Z
Learning: Applies to src/interactive-window/{interactiveWindow.ts,interactiveWindowController.ts} : Implement robust kernel error handling: automatic restart, clear error display in system info cells, graceful degradation when kernels are unavailable, and user notifications for critical failures
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-03T12:58:18.091Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel.instructions.md:0-0
Timestamp: 2025-09-03T12:58:18.091Z
Learning: Applies to src/kernels/errors/**/*.ts : Use specific error types (e.g., KernelDeadError, KernelConnectionTimeoutError, KernelInterruptTimeoutError, JupyterInvalidKernelError, KernelDependencyError) for failure cases
Applied to files:
src/platform/errors/deepnoteKernelErrors.ts
🧬 Code graph analysis (3)
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts (2)
src/platform/common/types.ts (2)
IOutputChannel(10-10)IOutputChannel(11-11)src/platform/common/constants.ts (1)
STANDARD_OUTPUT_CHANNEL(32-32)
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (2)
src/platform/errors/deepnoteKernelErrors.ts (2)
DeepnoteVenvCreationError(50-89)DeepnoteToolkitInstallError(94-169)src/kernels/deepnote/types.ts (1)
DEEPNOTE_TOOLKIT_WHEEL_URL(158-158)
src/kernels/deepnote/deepnoteServerStarter.node.ts (1)
src/platform/errors/deepnoteKernelErrors.ts (2)
DeepnoteServerTimeoutError(252-287)DeepnoteServerStartupError(174-247)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Lint & Format
- GitHub Check: Build & Test
🔇 Additional comments (8)
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (2)
163-167: Good: move to typed venv creation error.Clearer upstream handling and better diagnostics.
260-266: Good: install failure is now a typed error.Enables standardized reporting and UX.
src/kernels/deepnote/deepnoteServerStarter.node.ts (3)
45-46: Nice: per-file stdout/stderr ring buffer for diagnostics.This will materially improve error context.
181-183: Good: initialize and trim output buffers.Bounded to last 5k chars; low risk of memory growth.
Also applies to: 187-206
223-228: Timeout path correctly captures output before teardown.The timeout error includes stderr; good UX.
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts (2)
137-138: Good: route selection failures through a single handler.Centralized UX keeps errors consistent.
85-87: DI binding for STANDARD_OUTPUT_CHANNEL is properly configured; no changes needed.The container binding exists in
src/extension.common.ts:125asserviceManager.addSingletonInstance<OutputChannel>(IOutputChannel, standardOutputChannel, STANDARD_OUTPUT_CHANNEL). This identical injection pattern is already used successfully in multiple classes throughout the codebase (productInstaller.node.ts,deepnoteServerStarter.node.ts,deepnoteToolkitInstaller.node.ts, and others). The code is correct.Likely an incorrect or invalid review comment.
src/platform/errors/deepnoteKernelErrors.ts (1)
1-2: Confirm header policy for new files.This is a new file with a Microsoft header. Our guidelines elsewhere say not to add the header to new files. Confirm repo policy and adjust if needed.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
src/kernels/deepnote/deepnoteServerStarter.node.ts (3)
137-146: Incorrect venv PATH/VIRTUAL_ENV derivation (breaks on python3, Windows).Regex trimming ‘/python’ or ‘\python.exe’ misses common paths (e.g., bin/python3), yielding bad PATH/VIRTUAL_ENV. Derive via path utilities.
Apply:
- const venvBinDir = interpreter.uri.fsPath.replace(/\/python$/, '').replace(/\\python\.exe$/, ''); - const env = { ...process.env }; - - // Prepend venv bin directory to PATH so shell commands use venv's Python - env.PATH = `${venvBinDir}${process.platform === 'win32' ? ';' : ':'}${env.PATH || ''}`; - - // Also set VIRTUAL_ENV to indicate we're in a venv - const venvPath = venvBinDir.replace(/\/bin$/, '').replace(/\\Scripts$/, ''); - env.VIRTUAL_ENV = venvPath; + const env = { ...process.env }; + const pythonPath = interpreter.uri.fsPath; + const binDir = path.dirname(pythonPath); + const leaf = path.basename(binDir).toLowerCase(); + const venvRoot = + leaf === 'scripts' || leaf === 'bin' ? path.dirname(binDir) : binDir; + const delim = process.platform === 'win32' ? ';' : ':'; + // Ensure shell commands use the venv's Python + env.PATH = `${binDir}${delim}${env.PATH || ''}`; + // Advertise virtualenv root (used by many tools) + env.VIRTUAL_ENV = venvRoot;Also applies to: 140-146
129-170: Localize OutputChannel strings.User-facing text should use l10n.t(). Convert appendLine messages (start/stop status, warnings) to localized strings.
Example pattern:
- this.outputChannel.appendLine(`Starting Deepnote server on port ${port} for ${deepnoteFileUri.fsPath}...`); + this.outputChannel.appendLine(l10n.t('Starting Deepnote server on port {0} for {1}...', port, deepnoteFileUri.fsPath));Add
l10nimport.
161-162: Compute working directory robustly.Using Uri.joinPath(fileUri, '..') is brittle. Prefer dirname of the file path.
Apply:
- const notebookDir = Uri.joinPath(deepnoteFileUri, '..').fsPath; + const notebookDir = path.dirname(deepnoteFileUri.fsPath);src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (3)
128-129: Localize OutputChannel messages.Wrap user-facing strings with l10n.t() throughout installer output for consistency with guidelines.
Example edits:
- this.outputChannel.appendLine(`Setting up Deepnote toolkit environment for ${deepnoteFileUri.fsPath}...`); + this.outputChannel.appendLine(l10n.t('Setting up Deepnote toolkit environment for {0}...', deepnoteFileUri.fsPath)); - this.outputChannel.appendLine('Upgrading pip...'); + this.outputChannel.appendLine(l10n.t('Upgrading pip...')); - this.outputChannel.appendLine('Installing deepnote-toolkit and ipykernel...'); + this.outputChannel.appendLine(l10n.t('Installing deepnote-toolkit and ipykernel...')); - this.outputChannel.appendLine('✓ Deepnote toolkit ready'); + this.outputChannel.appendLine(l10n.t('✓ Deepnote toolkit ready')); - this.outputChannel.appendLine('✗ deepnote-toolkit installation failed'); + this.outputChannel.appendLine(l10n.t('✗ deepnote-toolkit installation failed')); - this.outputChannel.appendLine('Failed to set up deepnote-toolkit; see logs for details'); + this.outputChannel.appendLine(l10n.t('Failed to set up deepnote-toolkit; see logs for details'));Add
l10nto the vscode import list.Also applies to: 175-176, 193-215, 254-259, 276-277
322-326: Windows path bug in display name.Splitting by '/' fails on Windows. Use path.basename.
Apply:
- const parts = deepnoteFileUri.fsPath.split('/'); - return parts[parts.length - 1] || 'notebook'; + return path.basename(deepnoteFileUri.fsPath) || 'notebook';Import the existing
pathhelper as in server starter.
176-206: Pass CancellationToken to exec calls and handle process termination on cancellation.The concern is valid. The
tokenparameter is accepted (line 122) but never passed to anyexec()call (lines 176, 195, 226). When a user cancels during a pip operation, theCancellation.throwIfCanceled(token)checks at lines 189 and 217 only fire after the operation completes—if cancellation happens during execution, the pip/venv process continues uninterrupted, leaving orphans.Use
execObservable()to access the child process and kill it on cancellation:const upgrade = venvProcessService.execObservable( venvInterpreter.uri.fsPath, ['-m', 'pip', 'install', '--upgrade', 'pip'], { throwOnStdErr: false } ); const sub = token?.onCancellationRequested(() => upgrade.proc?.kill()); // Await result; check API for exact property (.result, .completed, or wrap exit event) sub?.dispose();Apply the same pattern to lines 195 and 226.
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts (2)
367-373: Avoid fsPath in provider handle; use a safe hash.Handles with raw paths can include separators and be long. Use the existing venv hash.
Apply:
- const serverProviderHandle: JupyterServerProviderHandle = { + const hash = this.toolkitInstaller.getVenvHash(baseFileUri); + const serverProviderHandle: JupyterServerProviderHandle = { extensionId: JVSC_EXTENSION_ID, id: 'deepnote-server', - handle: `deepnote-toolkit-server-${baseFileUri.fsPath}` + handle: `deepnote-toolkit-server-${hash}` };
535-538: Sanitize controller IDs; avoid embedding full paths.Use a short hash to form the temporary controller ID.
Apply:
- const loadingController = notebooks.createNotebookController( - `deepnote-loading-${notebookKey}`, + const safeId = this.toolkitInstaller.getVenvHash(notebook.uri.with({ query: '', fragment: '' })); + const loadingController = notebooks.createNotebookController( + `deepnote-loading-${safeId}`, DEEPNOTE_NOTEBOOK_TYPE, l10n.t('Loading Deepnote Kernel...') );Also applies to: 549-555
♻️ Duplicate comments (3)
src/kernels/deepnote/deepnoteServerStarter.node.ts (1)
218-250: Bugfix verified: capture output before cleanup.You now capture stdout/stderr before stop; timeout and unknown-failure paths preserve diagnostics. Good.
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (1)
269-286: Output sanitization fix acknowledged.You no longer echo raw exceptions to the Output channel; good alignment with learnings.
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts (1)
557-612: UI strings localized and clipboard hardened.Prior feedback addressed: l10n for messages/actions and clipboard write wrapped in try/catch.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
src/kernels/deepnote/deepnoteServerStarter.node.ts(7 hunks)src/kernels/deepnote/deepnoteToolkitInstaller.node.ts(6 hunks)src/kernels/deepnote/types.ts(1 hunks)src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts(7 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.node.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
*.node.tsfor Desktop-specific implementations that require full file system access and Python environments
Files:
src/kernels/deepnote/deepnoteServerStarter.node.tssrc/notebooks/deepnote/deepnoteKernelAutoSelector.node.tssrc/kernels/deepnote/deepnoteToolkitInstaller.node.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Inject interfaces, not concrete classes
Avoid circular dependencies
Usel10n.t()for user-facing strings
Use typed error classes fromsrc/platform/errors/when throwing or handling errors
Use theILoggerservice instead ofconsole.log
Preserve error details while scrubbing PII in messages and telemetry
Include the Microsoft copyright header in source files
Prefer async/await and handle cancellation withCancellationToken
**/*.{ts,tsx}: Order class members (methods, fields, properties) first by accessibility (public/protected/private) and then alphabetically
Do not add the Microsoft copyright header to new files
Use Uri.joinPath() to construct file paths instead of manual string concatenation
Add a blank line after groups of const declarations and before return statements for readability
Separate third-party imports from local file imports
Files:
src/kernels/deepnote/deepnoteServerStarter.node.tssrc/notebooks/deepnote/deepnoteKernelAutoSelector.node.tssrc/kernels/deepnote/types.tssrc/kernels/deepnote/deepnoteToolkitInstaller.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/deepnoteServerStarter.node.tssrc/kernels/deepnote/types.tssrc/kernels/deepnote/deepnoteToolkitInstaller.node.ts
src/notebooks/deepnote/**
📄 CodeRabbit inference engine (CLAUDE.md)
Deepnote integration code resides under src/notebooks/deepnote/
Files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
**/!(*.node|*.web).ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place shared cross-platform logic in common
.tsfiles (not suffixed with.nodeor.web)
Files:
src/kernels/deepnote/types.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel.instructions.md:0-0
Timestamp: 2025-09-03T12:58:18.091Z
Learning: Applies to src/kernels/errors/**/*.ts : Use specific error types (e.g., KernelDeadError, KernelConnectionTimeoutError, KernelInterruptTimeoutError, JupyterInvalidKernelError, KernelDependencyError) for failure cases
📚 Learning: 2025-09-03T12:59:14.489Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/notebooks.instructions.md:0-0
Timestamp: 2025-09-03T12:59:14.489Z
Learning: Applies to src/notebooks/controllers/kernelSource/**/*.ts : For new kernel sources, implement ILocalNotebookKernelSourceSelector (or similar)
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-10-09T11:21:57.494Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-09T11:21:57.494Z
Learning: Applies to src/notebooks/deepnote/deepnoteActivationService.ts : deepnoteActivationService.ts manages VS Code activation for Deepnote
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-03T12:58:18.091Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel.instructions.md:0-0
Timestamp: 2025-09-03T12:58:18.091Z
Learning: Applies to src/kernels/kernelProvider.base.ts : KernelProviderBase must create/manage IKernel instances, track by NotebookDocument, manage lifecycle (start/restart/dispose), and expose INotebookKernelExecution
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-03T12:59:14.489Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/notebooks.instructions.md:0-0
Timestamp: 2025-09-03T12:59:14.489Z
Learning: Applies to src/notebooks/controllers/kernelSource/**/*.ts : Create a UI provider for kernel selection for the new kernel source
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-03T12:59:14.489Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/notebooks.instructions.md:0-0
Timestamp: 2025-09-03T12:59:14.489Z
Learning: Applies to src/notebooks/controllers/**/*KernelConnector*.ts : KernelConnector must present user-friendly error messages for connection failures
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-03T12:54:35.368Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/interactiveWindow.instructions.md:0-0
Timestamp: 2025-09-03T12:54:35.368Z
Learning: Applies to src/interactive-window/{interactiveWindow.ts,interactiveWindowController.ts} : Implement robust kernel error handling: automatic restart, clear error display in system info cells, graceful degradation when kernels are unavailable, and user notifications for critical failures
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-10T08:51:54.455Z
Learnt from: dinohamzic
PR: deepnote/deepnote#18378
File: apps/webapp/server/api/landing.ts:130-136
Timestamp: 2025-09-10T08:51:54.455Z
Learning: In the Deepnote codebase, prefer explicit error messages over using raw error messages (like err.message) when handling errors that are exposed to customers, for security and user experience reasons.
Applied to files:
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
🧬 Code graph analysis (3)
src/kernels/deepnote/deepnoteServerStarter.node.ts (1)
src/platform/errors/deepnoteKernelErrors.ts (2)
DeepnoteServerTimeoutError(252-287)DeepnoteServerStartupError(174-247)
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts (2)
src/platform/common/types.ts (2)
IOutputChannel(10-10)IOutputChannel(11-11)src/platform/common/constants.ts (1)
STANDARD_OUTPUT_CHANNEL(32-32)
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (2)
src/platform/errors/deepnoteKernelErrors.ts (2)
DeepnoteVenvCreationError(50-89)DeepnoteToolkitInstallError(94-169)src/kernels/deepnote/types.ts (1)
DEEPNOTE_TOOLKIT_WHEEL_URL(160-160)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build & Test
🔇 Additional comments (3)
src/kernels/deepnote/types.ts (2)
159-161: Staging wheel URL hardcoded. Make configurable or gated.The wheel points to a staging S3 bucket. Consider a config flag or environment-based selection to avoid shipping staging endpoints by mistake.
Would you like a quick patch to switch via package.json setting or environment variable?
75-84: Contract verified: ensureInstalled callers properly handle the tightened contract.Both call sites avoid undefined checks:
deepnoteKernelAutoSelector.node.ts:350–356directly accessesvenvInterpreter.uriwithout null-checkingdeepnoteServerStarter.node.ts:122relies on exception propagationNo stale guards exist. Code is compliant.
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts (1)
347-355: Change is correct—single call site properly passes venv interpreter.Scan confirms only one deepnote
getOrStartServercall site (line 360 in deepnoteKernelAutoSelector.node.ts) and it passesvenvInterpreter. No alternative call sites pass a base interpreter, so the removal of installer logic from serverStarter is safe.
Artmann
requested review from
andyjakubowski,
jamesbhobbs and
saltenasl
as code owners
Fixes #
Summary by CodeRabbit
Bug Fixes
New Features