feat(block-execution): use deepnote kernel in venv #16
Conversation
📝 WalkthroughWalkthroughImplements automatic Deepnote kernel startup for .deepnote notebooks: per-file venv creation and toolkit installation (with optional shared install), per-notebook Deepnote toolkit Jupyter server lifecycle and provider, and an auto-selector that registers controllers and auto-selects Deepnote kernels. Adds Deepnote-specific kernel connection metadata and deserialization (kind Sequence Diagram(s)sequenceDiagram
autonumber
participant VSCode as VS Code
participant NB as Notebook (.deepnote)
participant Auto as DeepnoteKernelAutoSelector
participant PyExt as Python Extension / InterpreterService
participant SharedInst as DeepnoteSharedToolkitInstaller
participant Installer as DeepnoteToolkitInstaller
participant Starter as DeepnoteServerStarter
participant Prov as DeepnoteServerProvider
participant Jup as Deepnote Toolkit Jupyter Server
participant Req as Jupyter Request Creator
participant Ctrl as ControllerRegistration
VSCode->>Auto: onDidOpenNotebook(NB)
Auto->>Auto: filter DEEPNOTE_NOTEBOOK_TYPE & no kernel selected?
Auto->>PyExt: getActiveInterpreter()
alt interpreter found
Auto->>SharedInst: ensureSharedInstallation(baseInterpreter)
SharedInst-->>Auto: sharedPath / ok
Auto->>Installer: ensureInstalled(baseInterpreter, NB.uri)
Installer-->>Auto: venvInterpreter
Auto->>Starter: getOrStartServer(venvInterpreter, NB.uri)
Starter->>Jup: spawn deepnote_toolkit server --jupyter-port
Jup-->>Starter: ready (url, port[, token])
Starter-->>Auto: DeepnoteServerInfo
Auto->>Prov: registerServer(handle, DeepnoteServerInfo)
Auto->>Req: request kernelSpecs from server URL
Req-->>Auto: kernelSpec list
Auto->>Ctrl: addOrUpdate(DeepnoteKernelConnectionMetadata, ['deepnote'])
Ctrl-->>Auto: controllerId
Auto->>VSCode: set NotebookControllerAffinity.Preferred (auto-select)
else missing interpreter / failures
Auto-->>VSCode: log and abort gracefully
end
VSCode->>Auto: onDidCloseNotebook(NB)
Auto->>Prov: unregisterServer(handle) and cleanup
Possibly related PRs
Suggested reviewers
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 37
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
DEEPNOTE_KERNEL_IMPLEMENTATION.md(1 hunks)src/kernels/deepnote/deepnoteServerProvider.node.ts(1 hunks)src/kernels/deepnote/deepnoteServerStarter.node.ts(1 hunks)src/kernels/deepnote/deepnoteToolkitInstaller.node.ts(1 hunks)src/kernels/deepnote/types.ts(1 hunks)src/kernels/jupyter/finder/remoteKernelFinder.ts(1 hunks)src/kernels/types.ts(2 hunks)src/notebooks/controllers/controllerRegistration.ts(2 hunks)src/notebooks/controllers/types.ts(2 hunks)src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts(1 hunks)src/notebooks/serviceRegistry.node.ts(2 hunks)src/telemetry.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
**/!(*.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/types.tssrc/kernels/jupyter/finder/remoteKernelFinder.tssrc/telemetry.tssrc/notebooks/controllers/types.tssrc/kernels/deepnote/types.tssrc/notebooks/controllers/controllerRegistration.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, then alphabetically
Separate third-party imports from local file imports
Files:
src/kernels/types.tssrc/kernels/jupyter/finder/remoteKernelFinder.tssrc/notebooks/deepnote/deepnoteKernelAutoSelector.node.tssrc/telemetry.tssrc/kernels/deepnote/deepnoteToolkitInstaller.node.tssrc/notebooks/controllers/types.tssrc/kernels/deepnote/deepnoteServerProvider.node.tssrc/kernels/deepnote/types.tssrc/kernels/deepnote/deepnoteServerStarter.node.tssrc/notebooks/serviceRegistry.node.tssrc/notebooks/controllers/controllerRegistration.ts
src/kernels/types.ts
📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)
All kernel connection metadata types must inherit from BaseKernelConnectionMetadata and include required connection fields (ports, executable paths, auth tokens, etc.)
Files:
src/kernels/types.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/types.tssrc/kernels/jupyter/finder/remoteKernelFinder.tssrc/kernels/deepnote/deepnoteToolkitInstaller.node.tssrc/kernels/deepnote/deepnoteServerProvider.node.tssrc/kernels/deepnote/types.tssrc/kernels/deepnote/deepnoteServerStarter.node.ts
src/kernels/jupyter/{connection,launcher,session,finder}/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/kernel-jupyter.instructions.md)
src/kernels/jupyter/{connection,launcher,session,finder}/**/*.ts: Wrap all @jupyterlab/services calls and network interactions with robust error handling and meaningful error types
Support cancellation via CancellationToken (or equivalent) for long-running async operations (network calls, startup waits)
Implement automatic retry with exponential backoff for transient connection/session errors
Provide fallbacks or graceful degradation when services are unavailable and surface actionable, user-friendly error messages
Files:
src/kernels/jupyter/finder/remoteKernelFinder.ts
src/kernels/jupyter/finder/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/kernel-jupyter.instructions.md)
Discover remote kernels via /api/kernelspecs and /api/kernels endpoints and cache results; handle server failures gracefully
Remote kernel discovery must use the Jupyter REST API
Files:
src/kernels/jupyter/finder/remoteKernelFinder.ts
src/kernels/jupyter/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/kernel-jupyter.instructions.md)
src/kernels/jupyter/**/*.ts: Avoid blocking operations; use async/await consistently for asynchronous flows
Avoid shared global state for services; prefer instance-scoped or injected dependencies
Files:
src/kernels/jupyter/finder/remoteKernelFinder.ts
src/kernels/**/finder/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)
src/kernels/**/finder/**/*.ts: All finder implementations must implement the IContributedKernelFinder interface
Finder implementations should use caching to avoid redundant discovery work
Activate finder scans lazily (only when needed) to reduce startup cost
Files:
src/kernels/jupyter/finder/remoteKernelFinder.ts
**/*.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/deepnoteServerProvider.node.tssrc/kernels/deepnote/deepnoteServerStarter.node.tssrc/notebooks/serviceRegistry.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/notebooks/deepnote/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Add a blank line after const groups and before return statements (whitespace for readability)
Files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
🧠 Learnings (14)
📚 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/types.ts : All kernel connection metadata types must inherit from BaseKernelConnectionMetadata and include required connection fields (ports, executable paths, auth tokens, etc.)
Applied to files:
src/kernels/types.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/finder/**/*.ts : Discover remote kernels via /api/kernelspecs and /api/kernels endpoints and cache results; handle server failures gracefully
Applied to files:
src/kernels/jupyter/finder/remoteKernelFinder.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/jupyter/finder/**/*.ts : Remote kernel discovery must use the Jupyter REST API
Applied to files:
src/kernels/jupyter/finder/remoteKernelFinder.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 : For new kernel sources, implement ILocalNotebookKernelSourceSelector (or similar)
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-12T12:52:09.726Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T12:52:09.726Z
Learning: Applies to src/notebooks/deepnote/deepnoteNotebookSelector.ts : deepnoteNotebookSelector.ts implements UI selection logic
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-12T12:52:09.726Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T12:52:09.726Z
Learning: Applies to src/notebooks/deepnote/deepnoteActivationService.ts : deepnoteActivationService.ts wires VSCode activation
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.tssrc/notebooks/serviceRegistry.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/telemetry/**/*.ts : New features should emit telemetry for usage tracking
Applied to files:
src/telemetry.ts
📚 Learning: 2025-09-12T12:52:09.726Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T12:52:09.726Z
Learning: Applies to src/notebooks/deepnote/deepnoteTypes.ts : deepnoteTypes.ts should contain only type definitions
Applied to files:
src/notebooks/controllers/types.tssrc/notebooks/controllers/controllerRegistration.ts
📚 Learning: 2025-09-03T12:55:29.175Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/ipywidgets.instructions.md:0-0
Timestamp: 2025-09-03T12:55:29.175Z
Learning: Applies to src/notebooks/controllers/ipywidgets/types.ts : Use the shared core interfaces/types from types.ts across the IPyWidgets controller code
Applied to files:
src/notebooks/controllers/types.ts
📚 Learning: 2025-09-03T12:55:29.175Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/ipywidgets.instructions.md:0-0
Timestamp: 2025-09-03T12:55:29.175Z
Learning: Applies to src/notebooks/controllers/ipywidgets/serviceRegistry.{node,web}.ts : Register DI services in platform-specific registries (serviceRegistry.node.ts vs serviceRegistry.web.ts)
Applied to files:
src/notebooks/serviceRegistry.node.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/serviceRegistry.{node,web}.ts : Use dependency injection via the service registry for component creation and registration
Applied to files:
src/notebooks/serviceRegistry.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/serviceRegistry.{node,web}.ts : Use dependency injection for services to enable testability and platform variance
Applied to files:
src/notebooks/serviceRegistry.node.ts
📚 Learning: 2025-09-03T13:01:10.017Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/standalone.instructions.md:0-0
Timestamp: 2025-09-03T13:01:10.017Z
Learning: Applies to src/standalone/serviceRegistry.{node,web}.ts : Register new services for both platforms in serviceRegistry.node.ts and serviceRegistry.web.ts as appropriate
Applied to files:
src/notebooks/serviceRegistry.node.ts
📚 Learning: 2025-09-03T13:00:18.307Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/platform.instructions.md:0-0
Timestamp: 2025-09-03T13:00:18.307Z
Learning: Applies to src/platform/**/ioc/**/*.ts : Register platform services in the IoC container using ServiceManager (e.g., serviceManager.addSingleton<Token>(Token, Impl)) within a registerTypes function
Applied to files:
src/notebooks/serviceRegistry.node.ts
🧬 Code graph analysis (7)
src/kernels/types.ts (1)
src/kernels/deepnote/types.ts (1)
DeepnoteKernelConnectionMetadata(15-66)
src/notebooks/controllers/types.ts (1)
src/platform/common/constants.ts (2)
JupyterNotebookView(13-13)InteractiveWindowView(14-14)
src/kernels/deepnote/deepnoteServerProvider.node.ts (3)
src/kernels/deepnote/types.ts (1)
DeepnoteServerInfo(105-109)src/platform/common/constants.ts (1)
JVSC_EXTENSION_ID(24-24)src/platform/logging/index.ts (1)
logger(35-48)
src/kernels/deepnote/types.ts (4)
src/kernels/types.ts (1)
IJupyterKernelSpec(626-698)src/kernels/jupyter/types.ts (1)
JupyterServerProviderHandle(117-130)src/platform/telemetry/helpers.ts (1)
getTelemetrySafeHashedString(37-39)src/platform/api/pythonApi.ts (1)
serializePythonEnvironment(91-102)
src/kernels/deepnote/deepnoteServerStarter.node.ts (4)
src/kernels/deepnote/types.ts (4)
IDeepnoteServerStarter(88-88)IDeepnoteServerStarter(89-103)DeepnoteServerInfo(105-109)DEEPNOTE_DEFAULT_PORT(121-121)src/platform/common/types.ts (1)
IDisposable(210-212)src/platform/common/net/httpClient.ts (1)
HttpClient(12-36)src/platform/common/constants.ts (1)
STANDARD_OUTPUT_CHANNEL(32-32)
src/notebooks/serviceRegistry.node.ts (1)
src/kernels/deepnote/types.ts (6)
IDeepnoteToolkitInstaller(68-68)IDeepnoteToolkitInstaller(69-86)IDeepnoteServerStarter(88-88)IDeepnoteServerStarter(89-103)IDeepnoteKernelAutoSelector(111-111)IDeepnoteKernelAutoSelector(112-117)
src/notebooks/controllers/controllerRegistration.ts (3)
src/notebooks/controllers/types.ts (1)
IVSCodeNotebookController(22-43)src/kernels/types.ts (1)
KernelConnectionMetadata(305-305)src/platform/common/constants.ts (2)
JupyterNotebookView(13-13)InteractiveWindowView(14-14)
🪛 Biome (2.1.2)
src/kernels/types.ts
[error] 84-84: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🪛 markdownlint-cli2 (0.18.1)
DEEPNOTE_KERNEL_IMPLEMENTATION.md
17-17: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
25-25: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
37-37: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
51-51: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
63-63: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
78-78: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
83-83: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
89-89: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
137-137: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
190-190: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
197-197: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🔇 Additional comments (13)
src/kernels/jupyter/finder/remoteKernelFinder.ts (1)
460-463: Do not cache Deepnote kernels — LGTMReturning false for startUsingDeepnoteKernel is correct to keep cache clean.
src/kernels/types.ts (1)
318-320: Union extension — LGTMAdding DeepnoteKernelConnectionMetadata to RemoteKernelConnectionMetadata aligns type flow across the codebase.
src/telemetry.ts (1)
893-895: Telemetry union updates — LGTMIncluding 'startUsingDeepnoteKernel' in kernelConnectionType payloads is correct and consistent.
Also applies to: 3537-3539, 3658-3660
src/notebooks/serviceRegistry.node.ts (1)
46-55: Deepnote DI registrations LGTM. Both DeepnoteServerProvider and DeepnoteKernelAutoSelector implement IExtensionSyncActivationService.src/notebooks/controllers/controllerRegistration.ts (3)
237-243: LGTM – Type extension for Deepnote support.The addition of 'deepnote' to the type union is consistent and aligns with the PR objective.
244-350: LGTM – Consistent type extension in implementation.The internal addImpl method correctly extends the type union to include 'deepnote'.
351-357: LGTM – Controller lookup extended for Deepnote.The get method now accepts 'deepnote' as a valid notebook type for lookups.
src/kernels/deepnote/deepnoteServerProvider.node.ts (3)
18-31: LGTM – Clean DI setup.Class structure and dependency injection follow best practices.
33-43: LGTM – Proper provider registration.Activation correctly registers the provider and tracks disposal.
55-71: LGTM – Straightforward server enumeration.Method correctly enumerates registered servers for provider interface.
src/kernels/deepnote/types.ts (1)
56-65: JSON excludes serverInfo/token. Good.Not serializing serverInfo avoids leaking tokens in persisted state. Keep it this way.
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts (2)
186-187: Remove redundant rethrow.Catching and immediately rethrowing adds no value. Either handle the error or remove the try/catch.
Apply this diff:
- } catch (ex) { - logger.error(`Failed to auto-select Deepnote kernel: ${ex}`); - throw ex; - } + } catch (ex) { + logger.error(`Failed to auto-select Deepnote kernel: ${ex}`); + throw; + }Or remove the try/catch entirely if no additional handling is needed.
Likely an incorrect or invalid review comment.
63-63: getDisplayPath already scrubs PII by replacing the user’s home directory with~. No further action needed.
|
@coderabbitai no nitpicks or minor comments please |
|
Tip For best results, initiate chat on the files or code changes. @saltenasl, understood. I'll focus on significant issues only and skip nitpicks or minor comments in my reviews. ✏️ Learnings added
|
|
@coderabbitai only major issues going forward please |
|
Tip For best results, initiate chat on the files or code changes. @saltenasl, understood. I'll focus exclusively on major issues in my reviews going forward. 🧠 Learnings used |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
src/notebooks/controllers/types.ts (1)
28-28: Add DEEPNOTE_NOTEBOOK_TYPE to viewType union.Line 28's viewType excludes DEEPNOTE_NOTEBOOK_TYPE, but lines 87 & 96 accept it in method signatures. This type inconsistency will break at runtime when Deepnote controllers are created.
Apply:
- readonly viewType: typeof JupyterNotebookView | typeof InteractiveWindowView; + readonly viewType: typeof JupyterNotebookView | typeof InteractiveWindowView | typeof DEEPNOTE_NOTEBOOK_TYPE;src/kernels/types.ts (1)
82-85: Wrap case in block to fix lint/noSwitchDeclarations.Biome flags the require declaration inside the switch-case. Wrap in braces to scope it.
Based on static analysis hints.
Apply:
- case 'startUsingDeepnoteKernel': - // eslint-disable-next-line @typescript-eslint/no-use-before-define - const { DeepnoteKernelConnectionMetadata } = require('./deepnote/types'); - return DeepnoteKernelConnectionMetadata.create(clone); + case 'startUsingDeepnoteKernel': { + // eslint-disable-next-line @typescript-eslint/no-use-before-define + const { DeepnoteKernelConnectionMetadata } = require('./deepnote/types'); + return DeepnoteKernelConnectionMetadata.create(clone); + }src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (1)
31-35: Hash venv folder names to avoid PII and long paths.Using the sanitized full fsPath leaks user directories into globalStorage and can blow past Windows path limits. Hash the path (keep a short label for readability) before joining the storage URI.
+import { createHash } from 'crypto'; +import * as path from 'path'; @@ - const safePath = deepnoteFileUri.fsPath.replace(/[^a-zA-Z0-9]/g, '_'); - return Uri.joinPath(this.context.globalStorageUri, 'deepnote-venvs', safePath); + const fsPath = deepnoteFileUri.fsPath; + const hash = createHash('sha256').update(fsPath).digest('hex').slice(0, 16); + const label = path.basename(fsPath).replace(/[^a-zA-Z0-9]/g, '_').slice(0, 32) || 'deepnote'; + const folder = `${label}-${hash}`; + return Uri.joinPath(this.context.globalStorageUri, 'deepnote-venvs', folder);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (17)
DEEPNOTE_KERNEL_IMPLEMENTATION.md(1 hunks)src/kernels/deepnote/deepnoteServerProvider.node.ts(1 hunks)src/kernels/deepnote/deepnoteServerStarter.node.ts(1 hunks)src/kernels/deepnote/deepnoteToolkitInstaller.node.ts(1 hunks)src/kernels/deepnote/types.ts(1 hunks)src/kernels/jupyter/finder/remoteKernelFinder.ts(1 hunks)src/kernels/types.ts(2 hunks)src/notebooks/controllers/controllerRegistration.ts(3 hunks)src/notebooks/controllers/types.ts(3 hunks)src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts(1 hunks)src/notebooks/serviceRegistry.node.ts(2 hunks)src/platform/common/net/httpClient.ts(2 hunks)src/platform/common/serviceRegistry.node.ts(3 hunks)src/platform/common/types.ts(1 hunks)src/platform/errors/deepnoteServerNotFoundError.ts(1 hunks)src/platform/errors/types.ts(1 hunks)src/telemetry.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
**/!(*.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/common/net/httpClient.tssrc/platform/errors/types.tssrc/telemetry.tssrc/notebooks/controllers/types.tssrc/notebooks/controllers/controllerRegistration.tssrc/kernels/types.tssrc/platform/errors/deepnoteServerNotFoundError.tssrc/platform/common/types.tssrc/kernels/jupyter/finder/remoteKernelFinder.tssrc/kernels/deepnote/types.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, then alphabetically
Separate third-party imports from local file imports
Files:
src/platform/common/net/httpClient.tssrc/platform/errors/types.tssrc/telemetry.tssrc/notebooks/controllers/types.tssrc/notebooks/controllers/controllerRegistration.tssrc/notebooks/serviceRegistry.node.tssrc/kernels/types.tssrc/platform/errors/deepnoteServerNotFoundError.tssrc/platform/common/serviceRegistry.node.tssrc/platform/common/types.tssrc/notebooks/deepnote/deepnoteKernelAutoSelector.node.tssrc/kernels/jupyter/finder/remoteKernelFinder.tssrc/kernels/deepnote/deepnoteToolkitInstaller.node.tssrc/kernels/deepnote/types.tssrc/kernels/deepnote/deepnoteServerProvider.node.tssrc/kernels/deepnote/deepnoteServerStarter.node.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/common/net/httpClient.tssrc/platform/errors/types.tssrc/platform/errors/deepnoteServerNotFoundError.tssrc/platform/common/serviceRegistry.node.tssrc/platform/common/types.ts
**/*.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/serviceRegistry.node.tssrc/platform/common/serviceRegistry.node.tssrc/notebooks/deepnote/deepnoteKernelAutoSelector.node.tssrc/kernels/deepnote/deepnoteToolkitInstaller.node.tssrc/kernels/deepnote/deepnoteServerProvider.node.tssrc/kernels/deepnote/deepnoteServerStarter.node.ts
src/kernels/types.ts
📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)
All kernel connection metadata types must inherit from BaseKernelConnectionMetadata and include required connection fields (ports, executable paths, auth tokens, etc.)
Files:
src/kernels/types.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/types.tssrc/kernels/jupyter/finder/remoteKernelFinder.tssrc/kernels/deepnote/deepnoteToolkitInstaller.node.tssrc/kernels/deepnote/types.tssrc/kernels/deepnote/deepnoteServerProvider.node.tssrc/kernels/deepnote/deepnoteServerStarter.node.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/common/serviceRegistry.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/notebooks/deepnote/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Add a blank line after const groups and before return statements (whitespace for readability)
Files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
src/kernels/jupyter/{connection,launcher,session,finder}/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/kernel-jupyter.instructions.md)
src/kernels/jupyter/{connection,launcher,session,finder}/**/*.ts: Wrap all @jupyterlab/services calls and network interactions with robust error handling and meaningful error types
Support cancellation via CancellationToken (or equivalent) for long-running async operations (network calls, startup waits)
Implement automatic retry with exponential backoff for transient connection/session errors
Provide fallbacks or graceful degradation when services are unavailable and surface actionable, user-friendly error messages
Files:
src/kernels/jupyter/finder/remoteKernelFinder.ts
src/kernels/jupyter/finder/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/kernel-jupyter.instructions.md)
Discover remote kernels via /api/kernelspecs and /api/kernels endpoints and cache results; handle server failures gracefully
Remote kernel discovery must use the Jupyter REST API
Files:
src/kernels/jupyter/finder/remoteKernelFinder.ts
src/kernels/jupyter/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/kernel-jupyter.instructions.md)
src/kernels/jupyter/**/*.ts: Avoid blocking operations; use async/await consistently for asynchronous flows
Avoid shared global state for services; prefer instance-scoped or injected dependencies
Files:
src/kernels/jupyter/finder/remoteKernelFinder.ts
src/kernels/**/finder/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)
src/kernels/**/finder/**/*.ts: All finder implementations must implement the IContributedKernelFinder interface
Finder implementations should use caching to avoid redundant discovery work
Activate finder scans lazily (only when needed) to reduce startup cost
Files:
src/kernels/jupyter/finder/remoteKernelFinder.ts
🧠 Learnings (30)
📓 Common learnings
Learnt from: saltenasl
PR: deepnote/vscode-deepnote#0
File: :0-0
Timestamp: 2025-10-01T20:18:36.813Z
Learning: For the repository deepnote/vscode-deepnote, user saltenasl prefers reviews without nitpicks or minor comments. Focus only on significant issues.
📚 Learning: 2025-09-03T13:00:18.307Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/platform.instructions.md:0-0
Timestamp: 2025-09-03T13:00:18.307Z
Learning: Applies to src/platform/**/*.ts : Use Inversify decorators for DI: annotate classes with injectable() and inject dependencies with inject(Token)
Applied to files:
src/platform/common/net/httpClient.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/telemetry/**/*.ts : New features should emit telemetry for usage tracking
Applied to files:
src/telemetry.ts
📚 Learning: 2025-09-12T12:52:09.726Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T12:52:09.726Z
Learning: Applies to src/notebooks/deepnote/deepnoteTypes.ts : deepnoteTypes.ts should contain only type definitions
Applied to files:
src/notebooks/controllers/types.tssrc/notebooks/controllers/controllerRegistration.ts
📚 Learning: 2025-09-03T12:55:29.175Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/ipywidgets.instructions.md:0-0
Timestamp: 2025-09-03T12:55:29.175Z
Learning: Applies to src/notebooks/controllers/ipywidgets/types.ts : Use the shared core interfaces/types from types.ts across the IPyWidgets controller code
Applied to files:
src/notebooks/controllers/types.tssrc/notebooks/controllers/controllerRegistration.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/controllers/controllerRegistration.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 : Register the new kernel source in KernelSourceCommandHandler
Applied to files:
src/notebooks/controllers/controllerRegistration.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 : For new kernel sources, implement ILocalNotebookKernelSourceSelector (or similar)
Applied to files:
src/notebooks/controllers/controllerRegistration.tssrc/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-03T12:55:29.175Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/ipywidgets.instructions.md:0-0
Timestamp: 2025-09-03T12:55:29.175Z
Learning: Applies to src/notebooks/controllers/ipywidgets/serviceRegistry.{node,web}.ts : Register DI services in platform-specific registries (serviceRegistry.node.ts vs serviceRegistry.web.ts)
Applied to files:
src/notebooks/serviceRegistry.node.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/serviceRegistry.{node,web}.ts : Use dependency injection via the service registry for component creation and registration
Applied to files:
src/notebooks/serviceRegistry.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/serviceRegistry.{node,web}.ts : Use dependency injection for services to enable testability and platform variance
Applied to files:
src/notebooks/serviceRegistry.node.tssrc/platform/common/serviceRegistry.node.ts
📚 Learning: 2025-09-12T12:52:09.726Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T12:52:09.726Z
Learning: Applies to src/notebooks/deepnote/deepnoteActivationService.ts : deepnoteActivationService.ts wires VSCode activation
Applied to files:
src/notebooks/serviceRegistry.node.tssrc/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-03T13:00:18.307Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/platform.instructions.md:0-0
Timestamp: 2025-09-03T13:00:18.307Z
Learning: Applies to src/platform/**/ioc/**/*.ts : Register platform services in the IoC container using ServiceManager (e.g., serviceManager.addSingleton<Token>(Token, Impl)) within a registerTypes function
Applied to files:
src/notebooks/serviceRegistry.node.tssrc/platform/common/serviceRegistry.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/types.ts : All kernel connection metadata types must inherit from BaseKernelConnectionMetadata and include required connection fields (ports, executable paths, auth tokens, etc.)
Applied to files:
src/kernels/types.ts
📚 Learning: 2025-09-03T13:01:10.017Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/standalone.instructions.md:0-0
Timestamp: 2025-09-03T13:01:10.017Z
Learning: Applies to src/standalone/serviceRegistry.{node,web}.ts : Register new services for both platforms in serviceRegistry.node.ts and serviceRegistry.web.ts as appropriate
Applied to files:
src/platform/common/serviceRegistry.node.ts
📚 Learning: 2025-09-12T12:52:09.726Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T12:52:09.726Z
Learning: Applies to src/notebooks/deepnote/deepnoteNotebookSelector.ts : deepnoteNotebookSelector.ts implements UI selection logic
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-12T12:52:09.726Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T12:52:09.726Z
Learning: Applies to **/*.{ts,tsx} : Separate third-party imports from local file imports
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-12T12:52:09.726Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T12:52:09.726Z
Learning: Applies to src/notebooks/deepnote/**/*.ts : Add a blank line after const groups and before return statements (whitespace for readability)
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/**/*.ts : Avoid blocking operations; use async/await consistently for asynchronous flows
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/{connection,launcher,session,finder}/**/*.ts : Support cancellation via CancellationToken (or equivalent) for long-running async operations (network calls, startup waits)
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.tssrc/kernels/deepnote/types.tssrc/kernels/deepnote/deepnoteServerStarter.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/**/*.ts : Respect CancellationToken in all async operations
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.tssrc/kernels/deepnote/types.tssrc/kernels/deepnote/deepnoteServerStarter.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/**/*.ts : Respect CancellationToken in async operations and support cancellation in long-running tasks
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.tssrc/kernels/deepnote/types.tssrc/kernels/deepnote/deepnoteServerStarter.node.ts
📚 Learning: 2025-09-03T12:53:28.421Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-03T12:53:28.421Z
Learning: Applies to **/*.{ts,tsx} : Prefer async/await and handle cancellation with `CancellationToken`
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.tssrc/kernels/deepnote/types.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/session/**/*.ts : Call dispose() on all jupyterlab/services objects (managers, sessions, kernels, contents) to prevent resource leaks
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/finder/**/*.ts : Discover remote kernels via /api/kernelspecs and /api/kernels endpoints and cache results; handle server failures gracefully
Applied to files:
src/kernels/jupyter/finder/remoteKernelFinder.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/jupyter/finder/**/*.ts : Remote kernel discovery must use the Jupyter REST API
Applied to files:
src/kernels/jupyter/finder/remoteKernelFinder.ts
📚 Learning: 2025-09-03T13:00:18.307Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/platform.instructions.md:0-0
Timestamp: 2025-09-03T13:00:18.307Z
Learning: Applies to src/platform/logging/**/*.ts : Implement logging using OutputChannelLogger/ConsoleLogger and honor LogLevel; ensure PII scrubbing in log output
Applied to files:
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
📚 Learning: 2025-09-03T12:53:28.421Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-03T12:53:28.421Z
Learning: Applies to **/*.{ts,tsx} : Use typed error classes from `src/platform/errors/` when throwing or handling errors
Applied to files:
src/kernels/deepnote/deepnoteServerProvider.node.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/launcher/**/*.ts : Wait for server readiness before creating connections (poll, handle token retrieval, enforce timeouts)
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.node.ts
📚 Learning: 2025-09-03T12:53:28.421Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-03T12:53:28.421Z
Learning: Applies to **/*.{ts,tsx} : Preserve error details while scrubbing PII in messages and telemetry
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.node.ts
🧬 Code graph analysis (10)
src/platform/common/net/httpClient.ts (2)
src/notebooks/controllers/ipywidgets/scriptSourceProvider/cdnWidgetScriptSourceProvider.ts (1)
injectable(92-342)src/platform/common/platform/fileSystem.ts (1)
injectable(18-105)
src/notebooks/controllers/types.ts (2)
src/platform/common/constants.ts (2)
JupyterNotebookView(13-13)InteractiveWindowView(14-14)src/kernels/deepnote/types.ts (1)
DEEPNOTE_NOTEBOOK_TYPE(147-147)
src/notebooks/controllers/controllerRegistration.ts (4)
src/platform/common/constants.ts (2)
JupyterNotebookView(13-13)InteractiveWindowView(14-14)src/kernels/deepnote/types.ts (1)
DEEPNOTE_NOTEBOOK_TYPE(147-147)src/notebooks/controllers/types.ts (1)
IVSCodeNotebookController(23-44)src/kernels/types.ts (1)
KernelConnectionMetadata(305-305)
src/notebooks/serviceRegistry.node.ts (1)
src/kernels/deepnote/types.ts (8)
IDeepnoteToolkitInstaller(68-68)IDeepnoteToolkitInstaller(69-88)IDeepnoteServerStarter(90-90)IDeepnoteServerStarter(91-110)IDeepnoteServerProvider(118-118)IDeepnoteServerProvider(119-132)IDeepnoteKernelAutoSelector(134-134)IDeepnoteKernelAutoSelector(135-142)
src/kernels/types.ts (1)
src/kernels/deepnote/types.ts (1)
DeepnoteKernelConnectionMetadata(15-66)
src/platform/common/serviceRegistry.node.ts (1)
src/platform/common/types.ts (2)
IHttpClient(164-164)IHttpClient(165-171)
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (2)
src/kernels/deepnote/types.ts (3)
IDeepnoteToolkitInstaller(68-68)IDeepnoteToolkitInstaller(69-88)DEEPNOTE_TOOLKIT_WHEEL_URL(144-145)src/platform/common/constants.ts (1)
STANDARD_OUTPUT_CHANNEL(32-32)
src/kernels/deepnote/types.ts (4)
src/kernels/types.ts (1)
IJupyterKernelSpec(626-698)src/kernels/jupyter/types.ts (1)
JupyterServerProviderHandle(117-130)src/platform/telemetry/helpers.ts (1)
getTelemetrySafeHashedString(37-39)src/platform/api/pythonApi.ts (1)
serializePythonEnvironment(91-102)
src/kernels/deepnote/deepnoteServerProvider.node.ts (5)
src/kernels/deepnote/types.ts (3)
IDeepnoteServerProvider(118-118)IDeepnoteServerProvider(119-132)DeepnoteServerInfo(112-116)src/platform/common/types.ts (2)
IDisposableRegistry(13-13)IDisposableRegistry(14-14)src/platform/common/constants.ts (1)
JVSC_EXTENSION_ID(24-24)src/platform/logging/index.ts (1)
logger(35-48)src/platform/errors/deepnoteServerNotFoundError.ts (1)
DeepnoteServerNotFoundError(18-22)
src/kernels/deepnote/deepnoteServerStarter.node.ts (5)
src/platform/common/net/httpClient.ts (2)
injectable(13-38)exists(28-37)src/kernels/deepnote/types.ts (1)
DEEPNOTE_DEFAULT_PORT(146-146)src/platform/common/types.ts (5)
IDisposable(211-213)IOutputChannel(10-10)IOutputChannel(11-11)IHttpClient(164-164)IHttpClient(165-171)src/platform/common/constants.ts (1)
STANDARD_OUTPUT_CHANNEL(32-32)src/platform/common/cancellation.ts (1)
raceCancellationError(71-91)
🪛 Biome (2.1.2)
src/kernels/types.ts
[error] 84-84: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🪛 markdownlint-cli2 (0.18.1)
DEEPNOTE_KERNEL_IMPLEMENTATION.md
17-17: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
25-25: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
37-37: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
51-51: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
63-63: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
78-78: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
83-83: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
89-89: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
137-137: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
190-190: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
197-197: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🔇 Additional comments (7)
src/platform/errors/types.ts (1)
107-108: LGTM!New error category added cleanly.
src/platform/errors/deepnoteServerNotFoundError.ts (1)
18-22: LGTM!Error class follows established patterns, message clear.
src/platform/common/serviceRegistry.node.ts (1)
50-50: LGTM!IHttpClient singleton registration follows DI patterns.
src/platform/common/net/httpClient.ts (1)
4-4: LGTM!DI decorator applied correctly per guidelines.
Also applies to: 13-13
src/platform/common/types.ts (1)
164-164: LGTM!IHttpClient symbol export correct for DI.
src/kernels/jupyter/finder/remoteKernelFinder.ts (1)
460-462: LGTM!Deepnote kernels correctly treated as non-cacheable.
src/kernels/types.ts (1)
318-319: LGTM!DeepnoteKernelConnectionMetadata correctly added to RemoteKernelConnectionMetadata union.
|
@coderabbitai can you also update the description please |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 147 @saltenasl, I've updated the PR description with a comprehensive summary covering the key features, implementation details, new components, integration points, and technical notes. You can view it here: #16 🧠 Learnings used |
|
@coderabbitai i believe i resolved all of your comments now |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 68040 @saltenasl, I've checked the review comments. While some issues have been addressed, several major concerns remain unresolved: Critical (Security & Correctness):
Important (API Design): If you've addressed these in recent commits not yet reflected in my view, please let me know and I can verify. Otherwise, would you like me to mark the remaining comments as resolved, or would you prefer to address the major issues first? 🧠 Learnings used |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/kernels/deepnote/deepnoteServerStarter.node.ts(1 hunks)src/kernels/deepnote/deepnoteSharedToolkitInstaller.node.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/deepnoteSharedToolkitInstaller.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, then alphabetically
Separate third-party imports from local file imports
Files:
src/kernels/deepnote/deepnoteSharedToolkitInstaller.node.tssrc/kernels/deepnote/deepnoteServerStarter.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/deepnoteSharedToolkitInstaller.node.tssrc/kernels/deepnote/deepnoteServerStarter.node.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: saltenasl
PR: deepnote/vscode-deepnote#0
File: :0-0
Timestamp: 2025-10-01T20:18:36.813Z
Learning: For the repository deepnote/vscode-deepnote, user saltenasl prefers reviews without nitpicks or minor comments. Focus only on significant issues.
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/{connection,launcher,session,finder}/**/*.ts : Support cancellation via CancellationToken (or equivalent) for long-running async operations (network calls, startup waits)
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.node.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/launcher/**/*.ts : Wait for server readiness before creating connections (poll, handle token retrieval, enforce timeouts)
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.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/**/*.ts : Respect CancellationToken in all async operations
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.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/**/*.ts : Respect CancellationToken in async operations and support cancellation in long-running tasks
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.node.ts
📚 Learning: 2025-09-03T12:53:28.421Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-03T12:53:28.421Z
Learning: Applies to **/*.{ts,tsx} : Preserve error details while scrubbing PII in messages and telemetry
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.node.ts
🧬 Code graph analysis (2)
src/kernels/deepnote/deepnoteSharedToolkitInstaller.node.ts (2)
src/platform/common/constants.ts (1)
STANDARD_OUTPUT_CHANNEL(32-32)src/kernels/deepnote/types.ts (1)
DEEPNOTE_TOOLKIT_WHEEL_URL(144-145)
src/kernels/deepnote/deepnoteServerStarter.node.ts (4)
src/platform/common/net/httpClient.ts (2)
injectable(13-38)exists(28-37)src/kernels/deepnote/types.ts (6)
IDeepnoteServerStarter(90-90)IDeepnoteServerStarter(91-110)DeepnoteServerInfo(112-116)IDeepnoteToolkitInstaller(68-68)IDeepnoteToolkitInstaller(69-88)DEEPNOTE_DEFAULT_PORT(146-146)src/platform/common/constants.ts (1)
STANDARD_OUTPUT_CHANNEL(32-32)src/platform/common/cancellation.ts (1)
raceCancellationError(71-91)
🪛 GitHub Actions: CI
src/kernels/deepnote/deepnoteSharedToolkitInstaller.node.ts
[error] 13-13: compile-tsc failed: TS2305: Module "'./types'" has no exported member 'DEEPNOTE_TOOLKIT_VERSION'.
|
@coderabbitai full review (take my previous comments into consideration) and only major issues |
|
@saltenasl, I'll conduct a full review focusing exclusively on major issues, taking into account our previous discussion and your resolutions. 🧠 Learnings used🧠 Learnings used✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (4)
DEEPNOTE_KERNEL_IMPLEMENTATION.md (1)
154-162: Do not ship a hardcoded staging wheel; require hash verification + config.Shipping a direct S3 wheel without SHA256 pinning is a supply‑chain risk. Make the URL configurable and verify integrity before install (or use PyPI with pinned version + hashes).
Apply doc change (example):
-### Hardcoded Values (as requested) -- **Wheel URL**: `https://.../deepnote_toolkit-0.2.30.post20-py3-none-any.whl` +### Config (secure defaults) +- DEEPNOTE_TOOLKIT_WHEEL_URL (or PyPI name/version) +- DEEPNOTE_TOOLKIT_WHEEL_SHA256 +Install via either: +- PyPI: `pip install deepnote-toolkit==X.Y.Z --require-hashes -r <reqs>` +- Direct URL with hash: `deepnote-toolkit @ <URL>#sha256=<HASH>` +If using URL: download to temp, verify SHA256, then `pip install <file>`; fail closed on mismatch.src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (2)
164-176: Pin wheel with SHA256 and add pip safety flags (supply‑chain risk).Installing from a direct URL without hash verification is a high‑risk supply‑chain gap. Require a SHA256 pin, add timeout, disable cache/version check, and pass the cancellation token.
Apply:
- const installResult = await venvProcessService.exec( - venvInterpreter.uri.fsPath, - [ - '-m', - 'pip', - 'install', - '--upgrade', - `deepnote-toolkit[server] @ ${DEEPNOTE_TOOLKIT_WHEEL_URL}`, - 'ipykernel' - ], - { throwOnStdErr: false } - ); + const pipArgs = [ + '-m', 'pip', 'install', + '--upgrade', + '--no-cache-dir', + '--disable-pip-version-check', + '--timeout', '30', + // Enforce hash pinning once SHA is configured: + // ...(DEEPNOTE_TOOLKIT_WHEEL_SHA256 ? ['--require-hashes'] : []), + `deepnote-toolkit[server] @ ${DEEPNOTE_TOOLKIT_WHEEL_URL}` // + (DEEPNOTE_TOOLKIT_WHEEL_SHA256 ? `#sha256=${DEEPNOTE_TOOLKIT_WHEEL_SHA256}` : '') + , + 'ipykernel' + ]; + const installResult = await venvProcessService.exec( + venvInterpreter.uri.fsPath, + pipArgs, + { throwOnStdErr: false, token } + );Also add and import a SHA constant:
- import { IDeepnoteToolkitInstaller, DEEPNOTE_TOOLKIT_WHEEL_URL } from './types'; + import { IDeepnoteToolkitInstaller, DEEPNOTE_TOOLKIT_WHEEL_URL, DEEPNOTE_TOOLKIT_WHEEL_SHA256 } from './types';And in src/kernels/deepnote/types.ts:
export const DEEPNOTE_TOOLKIT_WHEEL_SHA256: string | undefined = undefined;
137-141: Cancellation doesn’t stop child processes (long‑running venv/pip).Token is checked, but exec() ignores it; cancellations won’t terminate subprocesses.
Pass the token to exec() (if supported) across all calls:
-const venvResult = await processService.exec(baseInterpreter.uri.fsPath, ['-m', 'venv', venvPath.fsPath], { - throwOnStdErr: false -}); +const venvResult = await processService.exec( + baseInterpreter.uri.fsPath, + ['-m', 'venv', venvPath.fsPath], + { throwOnStdErr: false, token } +);- await venvProcessService.exec( + await venvProcessService.exec( venvInterpreter.uri.fsPath, [ '-m', 'ipykernel', 'install', '--user', '--name', `deepnote-venv-${this.getVenvHash(deepnoteFileUri)}`, '--display-name', `Deepnote (${this.getDisplayName(deepnoteFileUri)})` ], - { throwOnStdErr: false } + { throwOnStdErr: false, token } );Please confirm IProcessService.exec accepts a
tokenoption; if not, wire token cancellation to kill the spawned process in the process factory or here.Also applies to: 194-207
src/kernels/deepnote/types.ts (1)
144-148: Don’t embed a staging wheel URL; add SHA pin and move to config.Hardcoding a staging URL in the public API and installing it without a hash is a supply‑chain risk. Read the URL from config (or env) and export a SHA256 to enforce pip hash checking.
Apply:
-export const DEEPNOTE_TOOLKIT_WHEEL_URL = - 'https://deepnote-staging-runtime-artifactory.s3.amazonaws.com/deepnote-toolkit-packages/0.2.30.post20/deepnote_toolkit-0.2.30.post20-py3-none-any.whl'; -export const DEEPNOTE_TOOLKIT_VERSION = '0.2.30.post20'; +// Read at runtime from settings (e.g., `deepnote.toolkitWheelUrl`) or environment. +export const DEEPNOTE_TOOLKIT_WHEEL_URL: string = '<configured at runtime>'; +export const DEEPNOTE_TOOLKIT_WHEEL_SHA256: string | undefined = undefined; // Optional pin for pip --require-hashes +export const DEEPNOTE_TOOLKIT_VERSION = '0.2.30.post20';Then adjust the installer to append “#sha256=…” and pass “--require-hashes” when SHA is set (see installer comment).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
DEEPNOTE_KERNEL_IMPLEMENTATION.md(1 hunks)src/kernels/deepnote/deepnoteServerStarter.node.ts(1 hunks)src/kernels/deepnote/deepnoteToolkitInstaller.node.ts(1 hunks)src/kernels/deepnote/types.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.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/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, then alphabetically
Separate third-party imports from local file imports
Files:
src/kernels/deepnote/deepnoteToolkitInstaller.node.tssrc/kernels/deepnote/deepnoteServerStarter.node.tssrc/kernels/deepnote/types.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.tssrc/kernels/deepnote/types.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 (10)
📚 Learning: 2025-09-03T13:00:18.307Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/platform.instructions.md:0-0
Timestamp: 2025-09-03T13:00:18.307Z
Learning: Applies to src/platform/logging/**/*.ts : Implement logging using OutputChannelLogger/ConsoleLogger and honor LogLevel; ensure PII scrubbing in log output
Applied to files:
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
📚 Learning: 2025-10-01T20:18:36.813Z
Learnt from: saltenasl
PR: deepnote/vscode-deepnote#0
File: :0-0
Timestamp: 2025-10-01T20:18:36.813Z
Learning: For the repository deepnote/vscode-deepnote, user saltenasl prefers reviews without nitpicks or minor comments. Focus only on significant issues.
Applied to files:
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/{connection,launcher,session,finder}/**/*.ts : Support cancellation via CancellationToken (or equivalent) for long-running async operations (network calls, startup waits)
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.node.tssrc/kernels/deepnote/types.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/launcher/**/*.ts : Wait for server readiness before creating connections (poll, handle token retrieval, enforce timeouts)
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.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/**/*.ts : Respect CancellationToken in all async operations
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.node.tssrc/kernels/deepnote/types.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/**/*.ts : Respect CancellationToken in async operations and support cancellation in long-running tasks
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.node.tssrc/kernels/deepnote/types.ts
📚 Learning: 2025-09-03T12:53:28.421Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-03T12:53:28.421Z
Learning: Applies to **/*.{ts,tsx} : Preserve error details while scrubbing PII in messages and telemetry
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.node.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/launcher/**/*.ts : Validate the Python environment and required Jupyter packages before starting a local server; handle startup timeouts and errors
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.node.ts
📚 Learning: 2025-09-25T13:16:13.796Z
Learnt from: hc2p
PR: deepnote/environments#62
File: gpu/Dockerfile:0-0
Timestamp: 2025-09-25T13:16:13.796Z
Learning: In Deepnote's Docker environment setup, packages installed in the underlying Docker image should be in system site packages, not in virtual environments. Server/kernels are launched in a virtual environment created by the installer, which creates a pth file where system site packages are appended first, giving them higher precedence and making them first in the search path. This is expected behavior for the deepnote-toolkit setup.
Applied to files:
DEEPNOTE_KERNEL_IMPLEMENTATION.md
📚 Learning: 2025-09-03T12:53:28.421Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-03T12:53:28.421Z
Learning: Applies to **/*.{ts,tsx} : Prefer async/await and handle cancellation with `CancellationToken`
Applied to files:
src/kernels/deepnote/types.ts
🧬 Code graph analysis (2)
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (2)
src/kernels/deepnote/types.ts (3)
IDeepnoteToolkitInstaller(68-68)IDeepnoteToolkitInstaller(69-88)DEEPNOTE_TOOLKIT_WHEEL_URL(144-145)src/platform/common/constants.ts (1)
STANDARD_OUTPUT_CHANNEL(32-32)
src/kernels/deepnote/types.ts (4)
src/kernels/types.ts (1)
IJupyterKernelSpec(626-698)src/kernels/jupyter/types.ts (1)
JupyterServerProviderHandle(117-130)src/platform/telemetry/helpers.ts (1)
getTelemetrySafeHashedString(37-39)src/platform/api/pythonApi.ts (1)
serializePythonEnvironment(91-102)
🪛 markdownlint-cli2 (0.18.1)
DEEPNOTE_KERNEL_IMPLEMENTATION.md
17-17: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
25-25: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
41-41: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
59-59: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
71-71: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
93-93: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
98-98: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
104-104: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
154-154: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
263-263: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
270-270: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
390-390: Trailing spaces
Expected: 0 or 2; Actual: 3
(MD009, no-trailing-spaces)
🔇 Additional comments (1)
DEEPNOTE_KERNEL_IMPLEMENTATION.md (1)
17-24: Handle server tokens securely.Do not persist tokens to disk or emit them in logs/telemetry. Keep tokens in memory, scope them to the server instance, and rotate/regenerate on restart.
⛔ Skipped due to learnings
Learnt from: CR PR: deepnote/vscode-deepnote#0 File: CLAUDE.md:0-0 Timestamp: 2025-09-12T12:52:09.726Z Learning: Applies to src/notebooks/deepnote/deepnoteTypes.ts : deepnoteTypes.ts should contain only type definitionsLearnt 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/types.ts : All kernel connection metadata types must inherit from BaseKernelConnectionMetadata and include required connection fields (ports, executable paths, auth tokens, etc.)
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (10)
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (3)
123-124: Still logging PII.Lines 123-124 log full file paths (deepnoteFileUri.fsPath, venvPath.fsPath). Use basename or hash.
164-177: Supply-chain risk: no SHA256 pin.Line 172 installs from URL without hash verification. Add "#sha256=..." and "--require-hashes".
136-158: Check venv creation exit code.Lines 138-158 verify venv by checking if interpreter exists, but don't check process exit code. Add exitCode check for clarity:
const venvResult = await processService.exec(baseInterpreter.uri.fsPath, ['-m', 'venv', venvPath.fsPath], { throwOnStdErr: false }); +// Check exit code first +if ((venvResult as any).exitCode && (venvResult as any).exitCode !== 0) { + logger.error(`venv creation failed with exit code ${(venvResult as any).exitCode}`); + this.outputChannel.appendLine('Error creating virtual environment'); + return undefined; +} + // Log any stderr output (warnings, etc.) but don't fail on itsrc/kernels/deepnote/types.ts (3)
52-54: Missing return type.getHashId() should explicitly return Promise.
77-81: CancellationToken missing from APIs.ensureInstalled, getOrStartServer, and ensureKernelSelected should accept CancellationToken for long-running ops.
Also applies to: 99-103, 141-141
144-148: Staging URL hardcoded.DEEPNOTE_TOOLKIT_WHEEL_URL is a staging URL. Move to config with env override and add SHA256 constant.
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts (1)
4-36: Import ordering.Separate third-party imports from local imports with blank line.
src/kernels/deepnote/deepnoteServerProvider.node.ts (1)
52-56: No duplicate handle check.registerServer silently overwrites. Log warning if handle exists.
src/kernels/types.ts (1)
82-85: Wrap the Deepnote switch clause in a blockBiome fails this file (
noSwitchDeclarations) because theconstdeclared in the Deepnote case leaks to sibling cases, so the branch never passes lint. Enclose the clause in braces:- case 'startUsingDeepnoteKernel': - // eslint-disable-next-line @typescript-eslint/no-use-before-define - const { DeepnoteKernelConnectionMetadata } = require('./deepnote/types'); - return DeepnoteKernelConnectionMetadata.create(clone); + case 'startUsingDeepnoteKernel': { + // eslint-disable-next-line @typescript-eslint/no-use-before-define + const { DeepnoteKernelConnectionMetadata } = require('./deepnote/types'); + return DeepnoteKernelConnectionMetadata.create(clone); + }src/kernels/deepnote/deepnoteServerStarter.node.ts (1)
83-125: Use the venv interpreter returned by the installer.
ensureInstalledreturns the venv interpreter (seetypes.ts), but the code keeps using the original interpreter to build PATH and launch the server. The toolkit lives in the venv, so this launches with the wrong Python and the module import fails.Apply:
- const installed = await this.toolkitInstaller.ensureInstalled(interpreter, deepnoteFileUri, token); - if (!installed) { + const venvInterpreter = await this.toolkitInstaller.ensureInstalled(interpreter, deepnoteFileUri, token); + if (!venvInterpreter) { throw new Error('Failed to install deepnote-toolkit. Please check the output for details.'); } @@ - const processService = await this.processServiceFactory.create(interpreter.uri); + const processService = await this.processServiceFactory.create(venvInterpreter.uri); @@ - const venvBinDir = interpreter.uri.fsPath.replace(/\/python$/, '').replace(/\\python\.exe$/, ''); + const venvBinDir = venvInterpreter.uri.fsPath.replace(/\/python$/, '').replace(/\\python\.exe$/, ''); @@ - const serverProcess = processService.execObservable( - interpreter.uri.fsPath, + const serverProcess = processService.execObservable( + venvInterpreter.uri.fsPath,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (19)
DEEPNOTE_KERNEL_IMPLEMENTATION.md(1 hunks)package.json(1 hunks)src/kernels/deepnote/deepnoteServerProvider.node.ts(1 hunks)src/kernels/deepnote/deepnoteServerStarter.node.ts(1 hunks)src/kernels/deepnote/deepnoteSharedToolkitInstaller.node.ts(1 hunks)src/kernels/deepnote/deepnoteToolkitInstaller.node.ts(1 hunks)src/kernels/deepnote/types.ts(1 hunks)src/kernels/jupyter/finder/remoteKernelFinder.ts(1 hunks)src/kernels/types.ts(2 hunks)src/notebooks/controllers/controllerRegistration.ts(3 hunks)src/notebooks/controllers/types.ts(3 hunks)src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts(1 hunks)src/notebooks/serviceRegistry.node.ts(2 hunks)src/platform/common/net/httpClient.ts(2 hunks)src/platform/common/serviceRegistry.node.ts(3 hunks)src/platform/common/types.ts(1 hunks)src/platform/errors/deepnoteServerNotFoundError.ts(1 hunks)src/platform/errors/types.ts(1 hunks)src/telemetry.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (33)
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/{connection,launcher,session,finder}/**/*.ts : Support cancellation via CancellationToken (or equivalent) for long-running async operations (network calls, startup waits)
Applied to files:
src/kernels/deepnote/types.tssrc/notebooks/deepnote/deepnoteKernelAutoSelector.node.tssrc/kernels/deepnote/deepnoteServerStarter.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/**/*.ts : Respect CancellationToken in all async operations
Applied to files:
src/kernels/deepnote/types.tssrc/notebooks/deepnote/deepnoteKernelAutoSelector.node.tssrc/kernels/deepnote/deepnoteServerStarter.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/**/*.ts : Respect CancellationToken in async operations and support cancellation in long-running tasks
Applied to files:
src/kernels/deepnote/types.tssrc/notebooks/deepnote/deepnoteKernelAutoSelector.node.tssrc/kernels/deepnote/deepnoteServerStarter.node.ts
📚 Learning: 2025-09-03T12:53:28.421Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-03T12:53:28.421Z
Learning: Applies to **/*.{ts,tsx} : Prefer async/await and handle cancellation with `CancellationToken`
Applied to files:
src/kernels/deepnote/types.ts
📚 Learning: 2025-09-12T12:52:09.726Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T12:52:09.726Z
Learning: Applies to **/*.{ts,tsx} : Separate third-party imports from local file imports
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-12T12:52:09.726Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T12:52:09.726Z
Learning: Applies to src/notebooks/deepnote/**/*.ts : Add a blank line after const groups and before return statements (whitespace for readability)
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/**/*.ts : Avoid blocking operations; use async/await consistently for asynchronous flows
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/session/**/*.ts : Call dispose() on all jupyterlab/services objects (managers, sessions, kernels, contents) to prevent resource leaks
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-12T12:52:09.726Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T12:52:09.726Z
Learning: Applies to src/notebooks/deepnote/deepnoteActivationService.ts : deepnoteActivationService.ts wires VSCode activation
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.tssrc/notebooks/serviceRegistry.node.ts
📚 Learning: 2025-09-25T13:16:13.796Z
Learnt from: hc2p
PR: deepnote/environments#62
File: gpu/Dockerfile:0-0
Timestamp: 2025-09-25T13:16:13.796Z
Learning: In Deepnote's Docker environment setup, packages installed in the underlying Docker image should be in system site packages, not in virtual environments. Server/kernels are launched in a virtual environment created by the installer, which creates a pth file where system site packages are appended first, giving them higher precedence and making them first in the search path. This is expected behavior for the deepnote-toolkit setup.
Applied to files:
DEEPNOTE_KERNEL_IMPLEMENTATION.md
📚 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/telemetry/**/*.ts : New features should emit telemetry for usage tracking
Applied to files:
src/telemetry.ts
📚 Learning: 2025-09-03T13:00:18.307Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/platform.instructions.md:0-0
Timestamp: 2025-09-03T13:00:18.307Z
Learning: Applies to src/platform/logging/**/*.ts : Implement logging using OutputChannelLogger/ConsoleLogger and honor LogLevel; ensure PII scrubbing in log output
Applied to files:
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
📚 Learning: 2025-10-01T20:18:36.836Z
Learnt from: saltenasl
PR: deepnote/vscode-deepnote#0
File: :0-0
Timestamp: 2025-10-01T20:18:36.836Z
Learning: For the repository deepnote/vscode-deepnote, user saltenasl prefers reviews without nitpicks or minor comments. Focus only on significant issues.
Applied to files:
src/kernels/deepnote/deepnoteToolkitInstaller.node.tssrc/notebooks/controllers/controllerRegistration.ts
📚 Learning: 2025-09-03T12:55:29.175Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/ipywidgets.instructions.md:0-0
Timestamp: 2025-09-03T12:55:29.175Z
Learning: Applies to src/notebooks/controllers/ipywidgets/serviceRegistry.{node,web}.ts : Register DI services in platform-specific registries (serviceRegistry.node.ts vs serviceRegistry.web.ts)
Applied to files:
src/notebooks/serviceRegistry.node.tssrc/platform/common/serviceRegistry.node.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/serviceRegistry.{node,web}.ts : Use dependency injection via the service registry for component creation and registration
Applied to files:
src/notebooks/serviceRegistry.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/serviceRegistry.{node,web}.ts : Use dependency injection for services to enable testability and platform variance
Applied to files:
src/notebooks/serviceRegistry.node.tssrc/platform/common/serviceRegistry.node.ts
📚 Learning: 2025-09-03T13:01:10.017Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/standalone.instructions.md:0-0
Timestamp: 2025-09-03T13:01:10.017Z
Learning: Applies to src/standalone/serviceRegistry.{node,web}.ts : Register new services for both platforms in serviceRegistry.node.ts and serviceRegistry.web.ts as appropriate
Applied to files:
src/notebooks/serviceRegistry.node.tssrc/platform/common/serviceRegistry.node.ts
📚 Learning: 2025-09-03T13:00:18.307Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/platform.instructions.md:0-0
Timestamp: 2025-09-03T13:00:18.307Z
Learning: Applies to src/platform/**/ioc/**/*.ts : Register platform services in the IoC container using ServiceManager (e.g., serviceManager.addSingleton<Token>(Token, Impl)) within a registerTypes function
Applied to files:
src/notebooks/serviceRegistry.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/serviceRegistry.{node,web}.ts : Register dependencies via DI with platform-appropriate services for Node and Web builds
Applied to files:
src/platform/common/serviceRegistry.node.ts
📚 Learning: 2025-09-03T12:55:29.175Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/ipywidgets.instructions.md:0-0
Timestamp: 2025-09-03T12:55:29.175Z
Learning: Applies to src/notebooks/controllers/ipywidgets/types.ts : Use the shared core interfaces/types from types.ts across the IPyWidgets controller code
Applied to files:
src/notebooks/controllers/controllerRegistration.tssrc/notebooks/controllers/types.ts
📚 Learning: 2025-09-12T12:52:09.726Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T12:52:09.726Z
Learning: Applies to src/notebooks/deepnote/deepnoteTypes.ts : deepnoteTypes.ts should contain only type definitions
Applied to files:
src/notebooks/controllers/controllerRegistration.tssrc/notebooks/controllers/types.ts
📚 Learning: 2025-09-12T12:52:09.726Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T12:52:09.726Z
Learning: This is a VSCode extension for Jupyter notebooks
Applied to files:
src/notebooks/controllers/controllerRegistration.ts
📚 Learning: 2025-10-02T08:12:46.572Z
Learnt from: saltenasl
PR: deepnote/vscode-deepnote#16
File: src/notebooks/controllers/controllerRegistration.ts:360-367
Timestamp: 2025-10-02T08:12:46.572Z
Learning: The vscode-deepnote extension does not and will not support Jupyter notebooks; it is exclusively for Deepnote notebooks.
Applied to files:
src/notebooks/controllers/controllerRegistration.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/controllers/controllerRegistration.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 : Register the new kernel source in KernelSourceCommandHandler
Applied to files:
src/notebooks/controllers/controllerRegistration.tssrc/notebooks/controllers/types.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 : For new kernel sources, implement ILocalNotebookKernelSourceSelector (or similar)
Applied to files:
src/notebooks/controllers/controllerRegistration.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/launcher/**/*.ts : Wait for server readiness before creating connections (poll, handle token retrieval, enforce timeouts)
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.node.ts
📚 Learning: 2025-09-03T12:53:28.421Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-03T12:53:28.421Z
Learning: Applies to **/*.{ts,tsx} : Preserve error details while scrubbing PII in messages and telemetry
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.node.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/launcher/**/*.ts : Validate the Python environment and required Jupyter packages before starting a local server; handle startup timeouts and errors
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.node.ts
📚 Learning: 2025-09-03T12:53:28.421Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-03T12:53:28.421Z
Learning: Applies to **/*.{ts,tsx} : Use typed error classes from `src/platform/errors/` when throwing or handling errors
Applied to files:
src/kernels/deepnote/deepnoteServerProvider.node.ts
📚 Learning: 2025-09-03T13:00:18.307Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/platform.instructions.md:0-0
Timestamp: 2025-09-03T13:00:18.307Z
Learning: Applies to src/platform/**/*.ts : Use Inversify decorators for DI: annotate classes with injectable() and inject dependencies with inject(Token)
Applied to files:
src/platform/common/net/httpClient.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/finder/**/*.ts : Discover remote kernels via /api/kernelspecs and /api/kernels endpoints and cache results; handle server failures gracefully
Applied to files:
src/kernels/jupyter/finder/remoteKernelFinder.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/jupyter/finder/**/*.ts : Remote kernel discovery must use the Jupyter REST API
Applied to files:
src/kernels/jupyter/finder/remoteKernelFinder.ts
🧬 Code graph analysis (8)
src/kernels/deepnote/types.ts (4)
src/kernels/types.ts (1)
IJupyterKernelSpec(626-698)src/kernels/jupyter/types.ts (1)
JupyterServerProviderHandle(117-130)src/platform/telemetry/helpers.ts (1)
getTelemetrySafeHashedString(37-39)src/platform/api/pythonApi.ts (1)
serializePythonEnvironment(91-102)
src/kernels/types.ts (1)
src/kernels/deepnote/types.ts (1)
DeepnoteKernelConnectionMetadata(15-66)
src/notebooks/serviceRegistry.node.ts (1)
src/kernels/deepnote/types.ts (8)
IDeepnoteToolkitInstaller(68-68)IDeepnoteToolkitInstaller(69-88)IDeepnoteServerStarter(90-90)IDeepnoteServerStarter(91-110)IDeepnoteServerProvider(118-118)IDeepnoteServerProvider(119-132)IDeepnoteKernelAutoSelector(134-134)IDeepnoteKernelAutoSelector(135-142)
src/platform/common/serviceRegistry.node.ts (1)
src/platform/common/types.ts (2)
IHttpClient(164-164)IHttpClient(165-171)
src/notebooks/controllers/controllerRegistration.ts (3)
src/platform/common/constants.ts (2)
JupyterNotebookView(13-13)InteractiveWindowView(14-14)src/kernels/deepnote/types.ts (1)
DEEPNOTE_NOTEBOOK_TYPE(148-148)src/notebooks/controllers/types.ts (1)
IVSCodeNotebookController(23-44)
src/kernels/deepnote/deepnoteServerStarter.node.ts (4)
src/platform/common/net/httpClient.ts (2)
injectable(13-38)exists(28-37)src/kernels/deepnote/types.ts (4)
DeepnoteServerInfo(112-116)IDeepnoteToolkitInstaller(68-68)IDeepnoteToolkitInstaller(69-88)DEEPNOTE_DEFAULT_PORT(147-147)src/platform/common/types.ts (4)
IOutputChannel(10-10)IOutputChannel(11-11)IHttpClient(164-164)IHttpClient(165-171)src/platform/common/constants.ts (1)
STANDARD_OUTPUT_CHANNEL(32-32)
src/kernels/deepnote/deepnoteSharedToolkitInstaller.node.ts (2)
src/platform/common/constants.ts (1)
STANDARD_OUTPUT_CHANNEL(32-32)src/kernels/deepnote/types.ts (2)
DEEPNOTE_TOOLKIT_VERSION(146-146)DEEPNOTE_TOOLKIT_WHEEL_URL(144-145)
src/notebooks/controllers/types.ts (2)
src/platform/common/constants.ts (2)
JupyterNotebookView(13-13)InteractiveWindowView(14-14)src/kernels/deepnote/types.ts (1)
DEEPNOTE_NOTEBOOK_TYPE(148-148)
🪛 Biome (2.1.2)
src/kernels/types.ts
[error] 84-84: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🪛 markdownlint-cli2 (0.18.1)
DEEPNOTE_KERNEL_IMPLEMENTATION.md
17-17: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
25-25: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
41-41: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
59-59: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
71-71: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
93-93: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
98-98: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
104-104: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
154-154: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
263-263: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
270-270: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
390-390: Trailing spaces
Expected: 0 or 2; Actual: 3
(MD009, no-trailing-spaces)
🔇 Additional comments (6)
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (1)
228-240: LGTM: toolkit verification.Simple import check is appropriate for fast verification.
package.json (1)
713-786: LGTM: UI command exclusions.Correctly excludes Deepnote notebooks from Jupyter-specific commands.
src/kernels/deepnote/deepnoteServerProvider.node.ts (1)
102-118: Use typed error.Line 107 throws generic Error. Use DeepnoteServerNotFoundError from line 12.
Wait—line 12 already imports DeepnoteServerNotFoundError and line 107 uses it. This comment is obsolete.
src/kernels/jupyter/finder/remoteKernelFinder.ts (1)
460-462: LGTM: Deepnote kernel exclusion.Correctly excludes dynamically-created Deepnote kernels from cache.
src/platform/common/net/httpClient.ts (1)
4-4: LGTM: DI annotation.Correctly adds @Injectable() for DI container.
Also applies to: 13-13
src/platform/common/serviceRegistry.node.ts (1)
23-23: LGTM: IHttpClient binding.Correctly registers HttpClient as singleton.
Also applies to: 35-35, 50-50
|
I get two loading messages in the kernel selection dropdown. |
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 (2)
DEEPNOTE_KERNEL_IMPLEMENTATION.md(1 hunks)src/kernels/deepnote/deepnoteServerStarter.node.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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.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, then alphabetically
Separate third-party imports from local file imports
Files:
src/kernels/deepnote/deepnoteServerStarter.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.ts
🧠 Learnings (7)
📚 Learning: 2025-09-25T13:16:13.796Z
Learnt from: hc2p
PR: deepnote/environments#62
File: gpu/Dockerfile:0-0
Timestamp: 2025-09-25T13:16:13.796Z
Learning: In Deepnote's Docker environment setup, packages installed in the underlying Docker image should be in system site packages, not in virtual environments. Server/kernels are launched in a virtual environment created by the installer, which creates a pth file where system site packages are appended first, giving them higher precedence and making them first in the search path. This is expected behavior for the deepnote-toolkit setup.
Applied to files:
DEEPNOTE_KERNEL_IMPLEMENTATION.md
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/{connection,launcher,session,finder}/**/*.ts : Support cancellation via CancellationToken (or equivalent) for long-running async operations (network calls, startup waits)
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.node.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/launcher/**/*.ts : Wait for server readiness before creating connections (poll, handle token retrieval, enforce timeouts)
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.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/**/*.ts : Respect CancellationToken in all async operations
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.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/**/*.ts : Respect CancellationToken in async operations and support cancellation in long-running tasks
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.node.ts
📚 Learning: 2025-09-03T12:53:28.421Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-03T12:53:28.421Z
Learning: Applies to **/*.{ts,tsx} : Preserve error details while scrubbing PII in messages and telemetry
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.node.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/launcher/**/*.ts : Validate the Python environment and required Jupyter packages before starting a local server; handle startup timeouts and errors
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.node.ts
🪛 markdownlint-cli2 (0.18.1)
DEEPNOTE_KERNEL_IMPLEMENTATION.md
17-17: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
25-25: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
41-41: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
59-59: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
71-71: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
93-93: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
98-98: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
104-104: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
154-154: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
263-263: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
270-270: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
390-390: Trailing spaces
Expected: 0 or 2; Actual: 3
(MD009, no-trailing-spaces)
🔇 Additional comments (6)
src/kernels/deepnote/deepnoteServerStarter.node.ts (6)
34-72: LGTM—serialization and cancellation support are correct.The pendingOperations map prevents concurrent start/stop races per file, and CancellationToken is threaded through. Error suppression at line 47 is acceptable for operation ordering.
169-179: LGTM—error path cleanup is correct.Server cleanup on
waitForServerfailure or timeout prevents leaked processes.
187-236: LGTM—stop logic is correctly serialized and cleans up resources.Consistent pendingOperations pattern and thorough cleanup in
stopServerImpl.
238-252: LGTM—cancellation and timeout are properly handled.Polling loop respects
CancellationTokenand enforces timeout.
264-294: LGTM—dispose thoroughly cleans up all resources.All maps cleared, processes killed, disposables disposed with error handling.
134-141: Critical: verify and secure Deepnote server binding
Confirm ifdeepnote_toolkit serverdefaults to127.0.0.1or supports--ip/token flags; if not, pass--ip 127.0.0.1(and generate a token) or wrap the server in a localhost-only, authenticated proxy.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (2)
123-124: Scrub PII from logs.Full user file paths leaked to logger and output channel. Use basename or hash label.
As per coding guidelines.
165-176: Pin toolkit wheel hash.Installing from a direct URL without SHA256 verification is a supply-chain risk. Append
#sha256=<HASH>to the URL and add--require-hashes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/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, then alphabetically
Separate third-party imports from local file imports
Files:
src/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/deepnoteToolkitInstaller.node.ts
🧠 Learnings (2)
📚 Learning: 2025-09-03T13:00:18.307Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/platform.instructions.md:0-0
Timestamp: 2025-09-03T13:00:18.307Z
Learning: Applies to src/platform/logging/**/*.ts : Implement logging using OutputChannelLogger/ConsoleLogger and honor LogLevel; ensure PII scrubbing in log output
Applied to files:
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
📚 Learning: 2025-10-01T20:18:36.836Z
Learnt from: saltenasl
PR: deepnote/vscode-deepnote#0
File: :0-0
Timestamp: 2025-10-01T20:18:36.836Z
Learning: For the repository deepnote/vscode-deepnote, user saltenasl prefers reviews without nitpicks or minor comments. Focus only on significant issues.
Applied to files:
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
🧬 Code graph analysis (1)
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (2)
src/kernels/deepnote/types.ts (3)
IDeepnoteToolkitInstaller(68-68)IDeepnoteToolkitInstaller(69-88)DEEPNOTE_TOOLKIT_WHEEL_URL(144-145)src/platform/common/constants.ts (1)
STANDARD_OUTPUT_CHANNEL(32-32)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (4)
114-226: Propagate cancellation token to all process executions.The
tokenparameter is accepted but never passed toprocessService.execcalls at lines 138, 165, 194, and 231 (viaisToolkitInstalledat line 188). Long-running venv creation, pip installs, and import checks remain non-cancellable.As per coding guidelines.
If
IProcessServiceFactory.createorexecAPI supports passing a token (e.g., as an option{ token }), add it to all exec calls. Otherwise, implement cancellation by killing the child process whentoken.isCancellationRequested.
123-124: Scrub PII from log messages.Lines 123-124 log full
deepnoteFileUri.fsPath, leaking user directory structure. UsegetDisplayNameor a hash identifier instead.As per coding guidelines.
Apply:
-logger.info(`Creating virtual environment at ${venvPath.fsPath} for ${deepnoteFileUri.fsPath}`); -this.outputChannel.appendLine(`Setting up Deepnote toolkit environment for ${deepnoteFileUri.fsPath}...`); +const displayName = this.getDisplayName(deepnoteFileUri); +logger.info(`Creating virtual environment at ${venvPath.fsPath} for ${displayName}`); +this.outputChannel.appendLine(`Setting up Deepnote toolkit environment for ${displayName}...`);
221-223: Use typed error classes and scrub PII in error output.Line 223 logs raw exception
${ex}which may contain stack traces with user paths. Import and throw typed error classes fromsrc/platform/errors/and scrub before logging.As per coding guidelines.
Example:
+import { BaseError } from '../../platform/errors/types'; @@ } catch (ex) { - logger.error(`Failed to set up deepnote-toolkit: ${ex}`); - this.outputChannel.appendLine(`Error setting up deepnote-toolkit: ${ex}`); + const message = ex instanceof Error ? ex.message : String(ex); + logger.error(`Failed to set up deepnote-toolkit: ${message}`); + this.outputChannel.appendLine(`Error setting up deepnote-toolkit. See logs for details.`); return undefined; }
164-177: Pin SHA256 hash for wheel URL to prevent supply-chain attacks.Pip installs from
DEEPNOTE_TOOLKIT_WHEEL_URLwithout hash verification. An attacker who compromises the S3 bucket can serve a malicious wheel.Add a constant
DEEPNOTE_TOOLKIT_WHEEL_SHA256intypes.tsand update the install args:[ '-m', 'pip', 'install', '--upgrade', - `deepnote-toolkit[server] @ ${DEEPNOTE_TOOLKIT_WHEEL_URL}`, + '--require-hashes', + `deepnote-toolkit[server] @ ${DEEPNOTE_TOOLKIT_WHEEL_URL}#sha256=${DEEPNOTE_TOOLKIT_WHEEL_SHA256}`, 'ipykernel' ],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/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, then alphabetically
Separate third-party imports from local file imports
Files:
src/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/deepnoteToolkitInstaller.node.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: hc2p
PR: deepnote/environments#62
File: gpu/Dockerfile:0-0
Timestamp: 2025-09-25T13:16:13.796Z
Learning: In Deepnote's Docker environment setup, packages installed in the underlying Docker image should be in system site packages, not in virtual environments. Server/kernels are launched in a virtual environment created by the installer, which creates a pth file where system site packages are appended first, giving them higher precedence and making them first in the search path. This is expected behavior for the deepnote-toolkit setup.
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T12:52:09.726Z
Learning: Applies to src/notebooks/deepnote/** : Deepnote integration code resides under src/notebooks/deepnote/
📚 Learning: 2025-09-03T13:00:18.307Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/platform.instructions.md:0-0
Timestamp: 2025-09-03T13:00:18.307Z
Learning: Applies to src/platform/logging/**/*.ts : Implement logging using OutputChannelLogger/ConsoleLogger and honor LogLevel; ensure PII scrubbing in log output
Applied to files:
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
📚 Learning: 2025-10-01T20:18:36.836Z
Learnt from: saltenasl
PR: deepnote/vscode-deepnote#0
File: :0-0
Timestamp: 2025-10-01T20:18:36.836Z
Learning: For the repository deepnote/vscode-deepnote, user saltenasl prefers reviews without nitpicks or minor comments. Focus only on significant issues.
Applied to files:
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
🧬 Code graph analysis (1)
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (2)
src/kernels/deepnote/types.ts (3)
IDeepnoteToolkitInstaller(68-68)IDeepnoteToolkitInstaller(69-88)DEEPNOTE_TOOLKIT_WHEEL_URL(144-145)src/platform/common/constants.ts (1)
STANDARD_OUTPUT_CHANNEL(32-32)
⏰ 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: Lint & Format
This reverts commit d54e69e.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (11)
src/kernels/deepnote/deepnoteServerStarter.node.ts (2)
83-88: Critical: capture and use venv interpreter from ensureInstalled.
ensureInstalledreturnsPythonEnvironment | undefined(per past review and types), but you treat it as boolean and continue using the baseinterpreterparam at lines 99, 120. The server must run with the venv interpreter.Apply:
- const installed = await this.toolkitInstaller.ensureInstalled(interpreter, deepnoteFileUri, token); - if (!installed) { + const venvInterpreter = await this.toolkitInstaller.ensureInstalled(interpreter, deepnoteFileUri, token); + if (!venvInterpreter) { throw new Error('Failed to install deepnote-toolkit. Please check the output for details.'); }Then use
venvInterpreterinstead ofinterpreterat lines 99, 102, 120:- const processService = await this.processServiceFactory.create(interpreter.uri); + const processService = await this.processServiceFactory.create(venvInterpreter.uri); // Set up environment to ensure the venv's Python is used for shell commands - const venvBinDir = interpreter.uri.fsPath.replace(/\/python$/, '').replace(/\\python\.exe$/, ''); + const venvBinDir = venvInterpreter.uri.fsPath.replace(/\/python$/, '').replace(/\\python\.exe$/, '');const serverProcess = processService.execObservable( - interpreter.uri.fsPath, + venvInterpreter.uri.fsPath, ['-m', 'deepnote_toolkit', 'server', '--jupyter-port', port.toString()],
239-247: Remove redundant try-catch.
.catch(() => false)already handles errors. Outer try-catch is unnecessary.Apply:
private async isServerRunning(serverInfo: DeepnoteServerInfo): Promise<boolean> { - try { - // Try to connect to the Jupyter API endpoint - const exists = await this.httpClient.exists(`${serverInfo.url}/api`).catch(() => false); - return exists; - } catch { - return false; - } + // Try to connect to the Jupyter API endpoint + return await this.httpClient.exists(`${serverInfo.url}/api`).catch(() => false); }DEEPNOTE_KERNEL_IMPLEMENTATION.md (9)
28-31: Hardcoded wheel (no integrity) and user-level kernel pollution.Parameterize the wheel (env/setting), require a pinned SHA256 verification, and avoid
ipykernel install --user(or manage full uninstall lifecycle). Shipping a staging S3 URL without integrity checks is a supply-chain risk.
43-47: Close the port race and restrict exposure.
- Prefer
--jupyter-port 0and parse the actual bound port; else implement retry on EADDRINUSE.- If binding address isn’t supported, document exposure, ensure strong tokens, and consider a localhost proxy/wrapper.
Also applies to: 131-135
49-50: Redact secrets and user paths in logs.Redact tokens and home-paths before writing to Output/telemetry.
54-58: Serialize getOrStartServer to prevent duplicate starts.De‑dupe concurrent starts per file via a pending‑promise map or mutex.
75-77: Kernel-spec strategy is inconsistent—pick one.Doc mixes “register per‑venv specs (prefer deepnote‑venv‑)” with “use server‑native specs instead.” Choose one approach and update all sections accordingly to avoid mismatch/duplication and user kernel pollution.
Also applies to: 297-304, 330-334, 379-383
51-53: Cap concurrent servers and add idle shutdown.Add a max concurrent servers limit, idle timeout (e.g., 10–15 min), and LRU eviction to prevent resource exhaustion.
165-173: Gate first‑run install/start behind user consent.Prompt once (persist via setting/workspace trust) before downloading, creating venvs, or starting servers; abort cleanly if declined.
Also applies to: 175-179
220-228: Make “Cancellable: Yes” real.Plumb cancellation through venv creation, pip installs, and spawned processes; delete partial venvs and kill children on cancel/error.
354-368: Avoid permanent controller protection → leaks.Don’t keep controllers “forever.” Dispose on notebook close or after idle TTL, and ensure associated server/process cleanup runs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
DEEPNOTE_KERNEL_IMPLEMENTATION.md(1 hunks)src/kernels/deepnote/deepnoteServerStarter.node.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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.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, then alphabetically
Separate third-party imports from local file imports
Files:
src/kernels/deepnote/deepnoteServerStarter.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.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: hc2p
PR: deepnote/environments#62
File: gpu/Dockerfile:0-0
Timestamp: 2025-09-25T13:16:13.796Z
Learning: In Deepnote's Docker environment setup, packages installed in the underlying Docker image should be in system site packages, not in virtual environments. Server/kernels are launched in a virtual environment created by the installer, which creates a pth file where system site packages are appended first, giving them higher precedence and making them first in the search path. This is expected behavior for the deepnote-toolkit setup.
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T12:52:09.726Z
Learning: Applies to src/notebooks/deepnote/** : Deepnote integration code resides under src/notebooks/deepnote/
📚 Learning: 2025-09-25T13:16:13.796Z
Learnt from: hc2p
PR: deepnote/environments#62
File: gpu/Dockerfile:0-0
Timestamp: 2025-09-25T13:16:13.796Z
Learning: In Deepnote's Docker environment setup, packages installed in the underlying Docker image should be in system site packages, not in virtual environments. Server/kernels are launched in a virtual environment created by the installer, which creates a pth file where system site packages are appended first, giving them higher precedence and making them first in the search path. This is expected behavior for the deepnote-toolkit setup.
Applied to files:
DEEPNOTE_KERNEL_IMPLEMENTATION.md
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/{connection,launcher,session,finder}/**/*.ts : Support cancellation via CancellationToken (or equivalent) for long-running async operations (network calls, startup waits)
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.node.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/launcher/**/*.ts : Wait for server readiness before creating connections (poll, handle token retrieval, enforce timeouts)
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.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/**/*.ts : Respect CancellationToken in all async operations
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.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/**/*.ts : Respect CancellationToken in async operations and support cancellation in long-running tasks
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.node.ts
📚 Learning: 2025-09-03T12:53:28.421Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-03T12:53:28.421Z
Learning: Applies to **/*.{ts,tsx} : Preserve error details while scrubbing PII in messages and telemetry
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.node.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/launcher/**/*.ts : Validate the Python environment and required Jupyter packages before starting a local server; handle startup timeouts and errors
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.node.ts
🧬 Code graph analysis (1)
src/kernels/deepnote/deepnoteServerStarter.node.ts (3)
src/kernels/deepnote/types.ts (4)
IDeepnoteServerStarter(90-90)IDeepnoteServerStarter(91-110)DeepnoteServerInfo(112-116)DEEPNOTE_DEFAULT_PORT(147-147)src/platform/common/types.ts (5)
IDisposable(211-213)IOutputChannel(10-10)IOutputChannel(11-11)IHttpClient(164-164)IHttpClient(165-171)src/platform/common/constants.ts (1)
STANDARD_OUTPUT_CHANNEL(32-32)
🪛 markdownlint-cli2 (0.18.1)
DEEPNOTE_KERNEL_IMPLEMENTATION.md
17-17: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
25-25: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
41-41: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
59-59: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
71-71: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
93-93: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
98-98: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
104-104: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
154-154: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
263-263: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
270-270: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
390-390: Trailing spaces
Expected: 0 or 2; Actual: 3
(MD009, no-trailing-spaces)
⏰ 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: Lint & Format
🔇 Additional comments (4)
src/kernels/deepnote/deepnoteServerStarter.node.ts (4)
34-72: LGTM!Debouncing via
pendingOperationsand cancellation support added. Residual race betweenisServerRunningcheck and return is narrow and acceptable.
172-221: LGTM!Debouncing added for
stopServer. Cleanup logic instopServerImplcorrectly disposes resources and clears state.
223-237: LGTM!
CancellationTokensupport added. Polling loop respects cancellation correctly.
249-278: LGTM!Comprehensive cleanup: stops servers, disposes tracked resources, clears all state.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (10)
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (5)
18-29: Still missing IDisposable for resource cleanup.Maps
venvPythonPathsandpendingInstallationsaccumulate entries without cleanup. On extension deactivation, this leaks memory and leaves pending promises untracked.As per coding guidelines.
114-229: Cancellation token not propagated to process.exec calls.Token is checked at lines 121, 148, 180 but never passed to
processService.execorvenvProcessService.execat lines 139, 167, 197, 235. Long-running venv creation and pip installs remain non-cancellable.As per coding guidelines.
123-124: PII exposure: full file paths in logs.Lines 123-124 log
deepnoteFileUri.fsPathdirectly, leaking user directory structure. Use a scrubbed identifier (hash or basename).As per coding guidelines.
167-177: Supply-chain risk: wheel URL without hash verification.Installing from
DEEPNOTE_TOOLKIT_WHEEL_URLwithout SHA256 verification exposes users to man-in-the-middle attacks and compromised artifacts. Add#sha256=<hash>to the URL and use--require-hashes.
224-228: Use typed error classes; scrub PII from error output.Catch block uses untyped error (
ex), and line 226 outputs raw exception which may contain stack traces with user paths. Use typed errors fromsrc/platform/errors/and scrub before logging.As per coding guidelines.
src/kernels/deepnote/deepnoteServerStarter.node.ts (5)
19-20: Add IDisposable to implements clause.dispose() is implemented (line 250), but the class doesn't declare
implements IDisposable. Add it for clarity and type safety.Apply:
-export class DeepnoteServerStarter implements IDeepnoteServerStarter { +export class DeepnoteServerStarter implements IDeepnoteServerStarter, IDisposable {
83-88: Critical: Use returned venv interpreter to start the server.ensureInstalled returns
PythonEnvironment | undefined, not a boolean. The code treats it as boolean (line 86) and still uses the baseinterpreterto launch the server (line 121), so deepnote_toolkit won't be found in the venv.Assign the return value to
venvInterpreter, throw if undefined, and usevenvInterpreter.uri.fsPathwhen creating the process service (line 100) and launching the server (line 121).- const installed = await this.toolkitInstaller.ensureInstalled(interpreter, deepnoteFileUri, token); - if (!installed) { + const venvInterpreter = await this.toolkitInstaller.ensureInstalled(interpreter, deepnoteFileUri, token); + if (!venvInterpreter) { throw new Error('Failed to install deepnote-toolkit. Please check the output for details.'); }- const processService = await this.processServiceFactory.create(undefined); + const processService = await this.processServiceFactory.create(venvInterpreter.uri);- const venvBinDir = interpreter.uri.fsPath.replace(/\/python$/, '').replace(/\\python\.exe$/, ''); + const venvBinDir = venvInterpreter.uri.fsPath.replace(/\/python$/, '').replace(/\\python\.exe$/, '');- const serverProcess = processService.execObservable( - interpreter.uri.fsPath, + const serverProcess = processService.execObservable( + venvInterpreter.uri.fsPath, ['-m', 'deepnote_toolkit', 'server', '--jupyter-port', port.toString()],
120-127: Critical: Enforce localhost binding and auth.deepnote_toolkit server provides no
--ipor token flags (per web search). Server may bind to 0.0.0.0 and lack auth, exposing Jupyter API on LAN.Wrap the launch in a proxy or Jupyter CLI that sets
--ip=127.0.0.1and a token, or file an upstream feature request for these flags.
93-93: Remove 'host' parameter from getPort.Specifying
host: 'localhost'may cause IPv4/IPv6 resolution issues on some platforms. Use the default.- const port = await getPort({ host: 'localhost', port: DEEPNOTE_DEFAULT_PORT }); + const port = await getPort({ port: DEEPNOTE_DEFAULT_PORT });
240-248: Remove redundant try-catch.Line 243's
.catch(() => false)already handles errors; the outer try-catch (lines 241-247) is redundant.private async isServerRunning(serverInfo: DeepnoteServerInfo): Promise<boolean> { - try { - // Try to connect to the Jupyter API endpoint - const exists = await this.httpClient.exists(`${serverInfo.url}/api`).catch(() => false); - return exists; - } catch { - return false; - } + // Try to connect to the Jupyter API endpoint + return await this.httpClient.exists(`${serverInfo.url}/api`).catch(() => false); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/kernels/deepnote/deepnoteServerStarter.node.ts(1 hunks)src/kernels/deepnote/deepnoteToolkitInstaller.node.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/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, then alphabetically
Separate third-party imports from local file imports
Files:
src/kernels/deepnote/deepnoteToolkitInstaller.node.tssrc/kernels/deepnote/deepnoteServerStarter.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
🧠 Learnings (10)
📓 Common learnings
Learnt from: hc2p
PR: deepnote/environments#62
File: gpu/Dockerfile:0-0
Timestamp: 2025-09-25T13:16:13.796Z
Learning: In Deepnote's Docker environment setup, packages installed in the underlying Docker image should be in system site packages, not in virtual environments. Server/kernels are launched in a virtual environment created by the installer, which creates a pth file where system site packages are appended first, giving them higher precedence and making them first in the search path. This is expected behavior for the deepnote-toolkit setup.
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T12:52:09.726Z
Learning: Applies to src/notebooks/deepnote/** : Deepnote integration code resides under src/notebooks/deepnote/
📚 Learning: 2025-09-03T13:00:18.307Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/platform.instructions.md:0-0
Timestamp: 2025-09-03T13:00:18.307Z
Learning: Applies to src/platform/logging/**/*.ts : Implement logging using OutputChannelLogger/ConsoleLogger and honor LogLevel; ensure PII scrubbing in log output
Applied to files:
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
📚 Learning: 2025-10-01T20:18:36.836Z
Learnt from: saltenasl
PR: deepnote/vscode-deepnote#0
File: :0-0
Timestamp: 2025-10-01T20:18:36.836Z
Learning: For the repository deepnote/vscode-deepnote, user saltenasl prefers reviews without nitpicks or minor comments. Focus only on significant issues.
Applied to files:
src/kernels/deepnote/deepnoteToolkitInstaller.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/**/*.ts : Monitor and dispose pending promises to prevent leaks during teardown
Applied to files:
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/{connection,launcher,session,finder}/**/*.ts : Support cancellation via CancellationToken (or equivalent) for long-running async operations (network calls, startup waits)
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.node.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/launcher/**/*.ts : Wait for server readiness before creating connections (poll, handle token retrieval, enforce timeouts)
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.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/**/*.ts : Respect CancellationToken in all async operations
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.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/**/*.ts : Respect CancellationToken in async operations and support cancellation in long-running tasks
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.node.ts
📚 Learning: 2025-09-03T12:53:28.421Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-03T12:53:28.421Z
Learning: Applies to **/*.{ts,tsx} : Preserve error details while scrubbing PII in messages and telemetry
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.node.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/launcher/**/*.ts : Validate the Python environment and required Jupyter packages before starting a local server; handle startup timeouts and errors
Applied to files:
src/kernels/deepnote/deepnoteServerStarter.node.ts
🧬 Code graph analysis (2)
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (2)
src/kernels/deepnote/types.ts (3)
IDeepnoteToolkitInstaller(68-68)IDeepnoteToolkitInstaller(69-88)DEEPNOTE_TOOLKIT_WHEEL_URL(144-145)src/platform/common/constants.ts (1)
STANDARD_OUTPUT_CHANNEL(32-32)
src/kernels/deepnote/deepnoteServerStarter.node.ts (5)
src/platform/common/net/httpClient.ts (2)
injectable(13-38)exists(28-37)src/kernels/deepnote/types.ts (2)
DeepnoteServerInfo(112-116)DEEPNOTE_DEFAULT_PORT(147-147)src/platform/common/constants.ts (1)
STANDARD_OUTPUT_CHANNEL(32-32)src/platform/logging/index.ts (1)
logger(35-48)src/platform/common/cancellation.ts (1)
raceCancellationError(71-91)
🔇 Additional comments (2)
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (2)
31-36: Hash-based venv path resolves PII concerns.Using
getVenvHashfor the directory name avoids path-length issues and prevents leaking user directory structure.
60-112: In-flight deduplication resolves race condition.The
pendingInstallationsmap with double-check pattern (lines 68-97) prevents concurrent installs for the same venv.
|
@coderabbitai resolve |
✅ Actions performedComments resolved and changes approved. |
saltenasl
deleted the
lukas/oss-53-run-notebooks-in-a-managed-deepnote-kernel
branch
Summary
This PR implements automatic Deepnote kernel startup for notebooks in VS Code, enabling seamless execution using managed Deepnote kernels.
Key Features
deepnote-toolkitfrom a hardcoded wheel URL with server extras.deepnotenotebooksImplementation Details
New Components
Integration Points
RemoteKernelConnectionMetadatato include Deepnote-specific metadatastartUsingDeepnoteKerneldeepnotenotebook typeTechnical Notes
/apiendpointSummary by CodeRabbit
New Features
Improvements
Chores