feat(kernels-management): select env, get jupyter server per project and kernel per notebook #162
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add core infrastructure for managing Deepnote kernel configurations, enabling future user-controlled kernel lifecycle management. ## What's New ### Core Models & Storage - Create type definitions for kernel configurations with UUID-based identity - Implement persistent storage layer using VS Code globalState - Build configuration manager with full CRUD operations - Add event system for configuration change notifications ### Components Added - `deepnoteKernelConfiguration.ts`: Type definitions and interfaces - `deepnoteConfigurationStorage.ts`: Serialization and persistence - `deepnoteConfigurationManager.ts`: Business logic and lifecycle management ### API Updates - Extend `IDeepnoteToolkitInstaller` with configuration-based methods - Extend `IDeepnoteServerStarter` with configuration-based methods - Maintain backward compatibility with file-based APIs ### Service Registration - Register DeepnoteConfigurationStorage as singleton - Register DeepnoteConfigurationManager with auto-activation - Integrate with existing dependency injection system ## Testing - Add comprehensive unit tests for storage layer (11 tests, all passing) - Add unit tests for configuration manager (29 tests, 22 passing) - 7 tests intentionally failing pending Phase 2 service refactoring ## Documentation - Create KERNEL_MANAGEMENT_VIEW_IMPLEMENTATION.md with complete architecture - Document 8-phase implementation plan - Define migration strategy from file-based to configuration-based system ## Dependencies - Add uuid package for configuration ID generation ## Status Phase 1 complete. Ready for Phase 2 (refactoring existing services). Related: #4913
…epnoteServerStarter Refactor DeepnoteToolkitInstaller and DeepnoteServerStarter to support configuration-based kernel management alongside existing file-based workflow. DeepnoteToolkitInstaller changes: - Add ensureVenvAndToolkit() method for direct venv path management - Add installAdditionalPackages() for installing packages in existing venvs - Add getVenvInterpreterByPath() helper for venv path-based interpreter lookup - Add getKernelSpecName() and getKernelDisplayName() for venv-based naming - Refactor installImpl() to installVenvAndToolkit() using venv paths - Update kernel spec installation to use venv directory names instead of file hashes - Mark ensureInstalled() as deprecated, now delegates to ensureVenvAndToolkit() DeepnoteServerStarter changes: - Add startServer() method accepting venv path and configuration ID - Add stopServer() method accepting configuration ID instead of file URI - Add startServerForConfiguration() implementation for config-based lifecycle - Add stopServerForConfiguration() implementation for config-based cleanup - Mark getOrStartServer() as deprecated for backward compatibility - Update server process tracking to work with configuration IDs as keys These changes enable the kernel configuration manager to create and manage isolated Python environments without requiring .deepnote file associations. Legacy file-based methods are preserved for backward compatibility. Part of Phase 2: Refactoring Existing Services
Add VS Code tree view interface for managing Deepnote kernel configurations. Changes: - Created DeepnoteConfigurationTreeItem with status-based icons and context values - Created DeepnoteConfigurationTreeDataProvider implementing TreeDataProvider - Created DeepnoteConfigurationsView handling all UI commands: * create: Multi-step wizard for new configurations * start/stop/restart: Server lifecycle management * delete: Configuration removal with confirmation * editName: Rename configurations * managePackages: Update package lists * refresh: Manual tree refresh - Created DeepnoteConfigurationsActivationService for initialization - Registered all services in serviceRegistry.node.ts - Added deepnoteKernelConfigurations view to package.json - Added 8 commands with icons and context menus - Added view/title and view/item/context menu contributions Technical details: - Uses Python API to enumerate available interpreters - Implements progress notifications for long-running operations - Provides input validation for names and package lists - Shows status indicators (Running/Starting/Stopped) with appropriate colors - Displays configuration details (Python path, venv, packages, timestamps) Fixes: - Made DeepnoteConfigurationManager.initialize() public to match interface - Removed unused getDisplayName() method from DeepnoteToolkitInstaller - Added type annotations to all lambda parameters
Add VS Code tree view interface for managing Deepnote kernel configurations. Changes: - Created DeepnoteConfigurationTreeItem with status-based icons and context values - Created DeepnoteConfigurationTreeDataProvider implementing TreeDataProvider - Created DeepnoteConfigurationsView handling all UI commands: * create: Multi-step wizard for new configurations * start/stop/restart: Server lifecycle management * delete: Configuration removal with confirmation * editName: Rename configurations * managePackages: Update package lists * refresh: Manual tree refresh - Created DeepnoteConfigurationsActivationService for initialization - Registered all services in serviceRegistry.node.ts - Added deepnoteKernelConfigurations view to package.json - Added 8 commands with icons and context menus - Added view/title and view/item/context menu contributions Technical details: - Uses Python API to enumerate available interpreters - Implements progress notifications for long-running operations - Provides input validation for names and package lists - Shows status indicators (Running/Starting/Stopped) with appropriate colors - Displays configuration details (Python path, venv, packages, timestamps) Fixes: - Made DeepnoteConfigurationManager.initialize() public to match interface - Removed unused getDisplayName() method from DeepnoteToolkitInstaller - Added type annotations to all lambda parameters
Add infrastructure for notebook-to-configuration mapping and selection UI. Changes: - Created DeepnoteConfigurationPicker for showing configuration selection UI - Created DeepnoteNotebookConfigurationMapper for tracking notebook→config mappings - Added interfaces to types.ts for new services - Registered new services in serviceRegistry.node.ts - Updated KERNEL_MANAGEMENT_VIEW_IMPLEMENTATION.md with: * New components documentation (picker and mapper) * Updated file structure with implementation status * Comprehensive implementation status section Technical details: - Picker shows configurations with status indicators (running/stopped) - Picker includes "Create new" option - Mapper stores selections in workspace state (per-workspace persistence) - Mapper provides bidirectional lookups (notebook→config and config→notebooks) Next steps: - Integrate picker and mapper with DeepnoteKernelAutoSelector - Show picker when notebook opens without selected configuration - Use selected configuration's venv and server instead of auto-creating
This refactoring improves terminology clarity across the Deepnote kernel management system by renaming "Kernel Configuration" to "Environment". Rationale: - "Configuration" implies kernel settings, but the system manages Python virtual environments (venv + packages + Jupyter server) - "Environment" is more accurate and familiar to developers - Reduces confusion between "configuration" (settings) and "environment" (Python venv) Changes: - Renamed directory: configurations/ → environments/ - Renamed 15 files (classes, interfaces, tests) - Updated types.ts: 6 interface names, 6 symbol constants - Updated package.json: 9 commands, 1 view ID, titles/labels - Updated all import paths and references across codebase - Updated documentation in KERNEL_MANAGEMENT_VIEW_IMPLEMENTATION.md Terminology mapping: - Kernel Configuration → Environment - IDeepnoteConfigurationManager → IDeepnoteEnvironmentManager - DeepnoteConfigurationsView → DeepnoteEnvironmentsView - deepnote.configurations.* → deepnote.environments.* - deepnoteKernelConfigurations view → deepnoteEnvironments view All tests pass, TypeScript compilation successful.
this was leading to many error logs in the "Jupyter Outputs"
Added comprehensive UI for selecting and switching environments for Deepnote notebooks, making it easy for users to choose which kernel environment to use. New Features: - Added "Select Environment for Notebook" command in notebook toolbar that shows a quick pick with all available environments - Environment picker now directly triggers the create environment dialog when "Create New" is selected, then automatically re-shows the picker - Environment names are now displayed in kernel connection labels for better visibility (e.g., "Deepnote: Python 3.10") - Shows current environment selection with checkmark in the picker - Displays environment status (Running/Stopped) with icons in picker - Warns users before switching if cells are currently executing Improvements: - Environment picker integration: Clicking "Create New" now launches the full creation dialog instead of just showing an info message - Added rebuildController() to IDeepnoteKernelAutoSelector interface for switching environments - Updated test mocks to include new dependencies UI/UX: - Added notebook toolbar button with server-environment icon - Quick pick shows environment details: interpreter path, packages, status - Graceful handling of edge cases (no environments, same environment selected) - Clear progress notifications during environment switching
Fixed three critical bugs that prevented proper environment switching: 1. Controller disposal race condition: Old controller was disposed before the new controller was ready, causing "DISPOSED" errors during cell execution. Fixed by deferring disposal until after new controller is fully registered. 2. Stale configuration caching: Configuration object wasn't refreshed after startServer(), so we connected with outdated serverInfo. Fixed by always calling getEnvironment() after startServer() to get current server info. 3. Environment manager early return: startServer() had an early return when config.serverInfo was set, preventing verification that the server was actually running. This caused connections to wrong/stale servers when switching TO a previously-used environment. Fixed by always calling serverStarter.startServer() (which is idempotent) to ensure current info. Additional improvements: - Made kernel spec installation idempotent and ensured it runs when reusing existing venvs - Removed legacy auto-create fallback path that's no longer needed - Added proper TypeScript non-null assertions after server info validation
This commit implements environment switching but with a known limitation regarding controller disposal and queued cell executions. ## Changes ### Port Allocation Refactoring - Refactored DeepnoteServerProvider to handle both jupyterPort and lspPort - Updated DeepnoteServerStarter to allocate both ports - Updated DeepnoteEnvironmentManager to store complete serverInfo - All port-related code now consistently uses the dual-port model ### Kernel Selection Logic Extraction - Extracted kernel selection logic into public `selectKernelSpec()` method - Added 4 unit tests for kernel selection (all passing) - Method is now testable and validates environment-specific kernel preference - Falls back to generic Python kernels when env-specific kernel not found ### Environment Switching Implementation - Added environment switching via tree view context menu - Switching calls `rebuildController()` to create new controller - Shows warning dialog if cells are currently executing - Updates notebook-to-environment mapping on switch ## Known Issue: Controller Disposal Problem When switching environments, we encountered a critical "notebook controller is DISPOSED" error. The issue occurs because: 1. User queues cell execution (VS Code references current controller) 2. User switches environments 3. New controller created and marked as preferred 4. Old controller disposed 5. Queued execution tries to run 5+ seconds later → DISPOSED error ### Current Workaround (Implemented) We do NOT dispose old controllers at all. Instead: - Old controllers stay alive to handle queued executions - New controller is marked as "Preferred" for new executions - Garbage collection cleans up eventually ### Why This Is Not Ideal - Potential memory leak with many switches - No guarantee new executions use new controller - Users might execute on wrong environment after switch - VS Code controller selection not fully deterministic ### What We Tried 1. Adding delay before disposal → Failed (timing unpredictable) 2. Disposing after setting preference → Failed 3. Never disposing → Prevents error but suboptimal ### Proper Solution Needed May require VS Code API changes to: - Force controller selection immediately - Cancel/migrate queued executions - Query if controller has pending executions See TODO.md for full analysis and discussion. ## Testing - ✅ All 40+ unit tests passing - ✅ Kernel selection tests passing (4 new tests) - ✅ Port allocation working correctly -⚠️ Environment switching works but with limitations above Related files: - src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts:306-315 - src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts:599-635 - src/kernels/deepnote/environments/deepnoteEnvironmentsView.ts:542-561
…nt management This commit removes the deprecated `ensureInstalled` method from the `DeepnoteToolkitInstaller` class, streamlining the installation process by encouraging the use of `ensureVenvAndToolkit`. Additionally, the `logs.txt` file has been deleted as it is no longer necessary for the current implementation. Changes include: - Removal of the legacy method to enhance code clarity and maintainability. - Deletion of the logs file to reduce clutter in the repository. These changes contribute to a cleaner codebase and improved user experience when managing Deepnote environments. Signed-off-by: Tomas Kislan <tomas@kislan.sk>
…ling This commit refines the Deepnote environment management by updating the structure of the `DeepnoteEnvironmentState` to encapsulate the `pythonInterpreterPath` as an object with `id` and `uri`. Additionally, the `venvBinDir` in `DeepnoteServerStarter` is now derived using `path.dirname()` for better path handling. Changes include: - Refactored `DeepnoteEnvironmentState` to use an object for `pythonInterpreterPath`. - Improved path handling for virtual environment binaries in `DeepnoteServerStarter`. - Updated related tests to reflect the new structure and ensure correctness. These changes enhance code clarity and maintainability while ensuring accurate environment configurations. Signed-off-by: Tomas Kislan <tomas@kislan.sk>
…e-deepnote-kernels Signed-off-by: Tomas Kislan <tomas@kislan.sk>
Signed-off-by: Tomas Kislan <tomas@kislan.sk>
Signed-off-by: Tomas Kislan <tomas@kislan.sk>
…ion and improve logging
…e-deepnote-kernels Signed-off-by: Tomas Kislan <tomas@kislan.sk>
…g messages Signed-off-by: Tomas Kislan <tomas@kislan.sk>
Signed-off-by: Tomas Kislan <tomas@kislan.sk>
…ed accessibility - Updated error, information, and prompt messages to use localization functions for better internationalization support. - Enhanced user experience by ensuring all relevant messages are translated, including environment creation, deletion, and package management prompts. Signed-off-by: Tomas Kislan <tomas@kislan.sk>
…nmentName functionality - Added comprehensive tests for the editEnvironmentName method, covering scenarios such as early returns for non-existent environments, user cancellations, and input validation. - Implemented checks for successful renaming of environments, ensuring existing configurations are preserved. - Updated the environment status icon in the UI to reflect error states accurately. Signed-off-by: Tomas Kislan <tomas@kislan.sk>
- Removed outdated `@types/uuid` dependency from `package.json` and `package-lock.json`. - Localized command titles and error messages in the Deepnote environment management components for improved accessibility. - Enhanced error handling in the environment manager and server starter to provide clearer feedback to users. Signed-off-by: Tomas Kislan <tomas@kislan.sk>
…e environment manager, fix some tests - Updated the `startServer` method in `IDeepnoteEnvironmentManager` to accept an optional cancellation token for better control over server startup processes. - Implemented the cancellation token in the `DeepnoteEnvironmentsView` to handle user cancellations during server operations. - Refactored tests to ensure proper handling of dependencies and cancellation scenarios. Signed-off-by: Tomas Kislan <tomas@kislan.sk>
…ger and improve localization - Replaced manual cancellation checks in the `stopServer` method with a centralized `Cancellation.throwIfCanceled` method for cleaner code. - Enhanced localization in the `DeepnoteEnvironmentTreeItem` class to provide more accurate time-related messages. - Removed unnecessary binding of `IDeepnoteEnvironmentManager` to `IExtensionSyncActivationService` in the service registry. Signed-off-by: Tomas Kislan <tomas@kislan.sk>
…tegration and streamlined cancellation handling - Added `IOutputChannel` integration to the `DeepnoteEnvironmentManager` for improved logging of activation errors. - Replaced manual cancellation checks with `Cancellation.throwIfCanceled` for cleaner and more consistent cancellation handling across methods. - Ensured proper disposal of the output channel during resource cleanup. Signed-off-by: Tomas Kislan <tomas@kislan.sk>
…gEnvironment logic - Introduced clearControllerForEnvironment method in IDeepnoteKernelAutoSelector to unselect the controller for a notebook when an environment is deleted. - Implemented disposeKernelsUsingEnvironment method in DeepnoteEnvironmentsView to dispose of kernels from open notebooks using the deleted environment. - Enhanced unit tests for DeepnoteEnvironmentsView to verify kernel disposal behavior when an environment is deleted. Signed-off-by: Tomas Kislan <tomas@kislan.sk>
Signed-off-by: Tomas Kislan <tomas@kislan.sk>
…e-deepnote-kernels Signed-off-by: Tomas Kislan <tomas@kislan.sk>
Signed-off-by: Tomas Kislan <tomas@kislan.sk>
Signed-off-by: Tomas Kislan <tomas@kislan.sk>
Signed-off-by: Tomas Kislan <tomas@kislan.sk>
Signed-off-by: Tomas Kislan <tomas@kislan.sk>
Signed-off-by: Tomas Kislan <tomas@kislan.sk>
…View Signed-off-by: Tomas Kislan <tomas@kislan.sk>
…e-deepnote-kernels Signed-off-by: Tomas Kislan <tomas@kislan.sk>
…Picker and DeepnoteEnvironmentsView - Removed redundant checks for existing environments and streamlined the user prompts for creating new environments. - Updated the createEnvironmentCommand to return the created environment for better handling in the environment selection process. - Enhanced logging for environment switching and user interactions. This refactor improves the clarity and efficiency of the environment management workflow. Signed-off-by: Tomas Kislan <tomas@kislan.sk>
Signed-off-by: Tomas Kislan <tomas@kislan.sk>
Signed-off-by: Tomas Kislan <tomas@kislan.sk>
Signed-off-by: Tomas Kislan <tomas@kislan.sk>
Signed-off-by: Tomas Kislan <tomas@kislan.sk>
…e-deepnote-kernels Signed-off-by: Tomas Kislan <tomas@kislan.sk>
- Changed return type of `ensureKernelSelected` method to Promise<boolean> for better handling of kernel selection status. - Refactored imports in `deepnoteEnvironmentManager.node.ts` to remove unused dependencies and improve clarity. - Updated environment selection logic in `deepnoteKernelAutoSelector` to include user prompts for environment creation. - Removed deprecated UI handling code from `deepnoteEnvironmentUi.ts`. - Cleaned up test files by removing unnecessary tests and assertions related to server management. This commit enhances the overall structure and functionality of the Deepnote kernel management system. Signed-off-by: Tomas Kislan <tomas@kislan.sk>
- Added `additionalPackages` parameter to `startServer` and `startServerForEnvironment` methods in `DeepnoteServerStarter` to allow installation of extra packages. - Updated the `createDeepnoteServerConfigHandle` utility function for consistent server handle creation. - Refactored kernel connection metadata handling in `DeepnoteKernelAutoSelector` to utilize the new server config handle. - Cleaned up test cases to reflect changes in method signatures and ensure proper functionality. This commit improves the flexibility of the Deepnote server setup by enabling the installation of additional packages during server startup. Signed-off-by: Tomas Kislan <tomas@kislan.sk>
- Modified the `ensureKernelSelected` method to include progress reporting and cancellation token parameters for improved user feedback during kernel selection. - Enhanced the `selectEnvironmentForNotebook` method to accept a notebook parameter, ensuring the correct notebook context is used when selecting environments. - Updated the environment selection command to handle active notebooks more robustly, providing warnings when no Deepnote notebook is found. - Refactored logging to provide clearer insights into notebook state changes and environment selection processes. These changes improve the user experience by providing better feedback and ensuring that the correct notebook context is maintained during operations. Signed-off-by: Tomas Kislan <tomas@kislan.sk>
Member
Author
|
@coderabbitai ignore |
Contributor
|
Note Reviews pausedUse the following commands to manage reviews:
📝 WalkthroughWalkthroughThis PR introduces a multi-environment architecture for Deepnote kernel management, replacing file-key-based server lifecycle with environment-scoped operations. It adds environment creation, persistence, and UI management; refactors server startup to allocate dedicated ports per environment; updates toolkit installation to support per-environment venvs and kernel specs; and adds notebook-to-environment mapping. Supporting changes include new tree views, commands, localization keys, and documentation. The orphan process cleanup feature is removed. Sequence Diagram(s)sequenceDiagram
actor User
participant View as DeepnoteEnvironmentsView
participant Manager as DeepnoteEnvironmentManager
participant Storage as DeepnoteEnvironmentStorage
participant Installer as DeepnoteToolkitInstaller
participant Starter as DeepnoteServerStarter
User->>View: Create Environment
View->>Manager: createEnvironment(options)
Manager->>Manager: Generate ID, compute venvPath
Manager->>Installer: ensureVenvAndToolkit(interpreter, venvPath)
Installer->>Installer: Create venv, install toolkit
Installer-->>Manager: VenvAndToolkitInstallation
Manager->>Storage: saveEnvironments()
Storage-->>Manager: Persisted
Manager->>Manager: Emit onDidChangeEnvironments
Manager-->>View: DeepnoteEnvironment
User->>View: Select Environment for Notebook
View->>Manager: getEnvironment(id)
Manager-->>View: DeepnoteEnvironment
View->>Starter: startServer(interpreter, venvPath, ...)
Starter->>Starter: Allocate ports (jupyter, lsp)
Starter->>Starter: Start server with venv Python
Starter-->>View: DeepnoteServerInfo
View->>View: Map notebook to environment
Possibly related PRs
Suggested reviewers
Pre-merge checks✅ Passed checks (2 passed)
Comment |
❌ 3 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (39)
.vscode/launch.json(1 hunks)CLAUDE.md(2 hunks)DEBUGGING_KERNEL_MANAGEMENT.md(1 hunks)ORPHAN_PROCESS_CLEANUP_IMPLEMENTATION.md(0 hunks)cspell.json(2 hunks)package.json(7 hunks)package.nls.json(1 hunks)src/kernels/deepnote/deepnoteServerProvider.node.ts(1 hunks)src/kernels/deepnote/deepnoteServerStarter.node.ts(21 hunks)src/kernels/deepnote/deepnoteSharedToolkitInstaller.node.ts(5 hunks)src/kernels/deepnote/deepnoteToolkitInstaller.node.ts(13 hunks)src/kernels/deepnote/environments/deepnoteEnvironment.ts(1 hunks)src/kernels/deepnote/environments/deepnoteEnvironmentManager.node.ts(1 hunks)src/kernels/deepnote/environments/deepnoteEnvironmentManager.unit.test.ts(1 hunks)src/kernels/deepnote/environments/deepnoteEnvironmentStorage.node.ts(1 hunks)src/kernels/deepnote/environments/deepnoteEnvironmentStorage.unit.test.ts(1 hunks)src/kernels/deepnote/environments/deepnoteEnvironmentTreeDataProvider.node.ts(1 hunks)src/kernels/deepnote/environments/deepnoteEnvironmentTreeDataProvider.unit.test.ts(1 hunks)src/kernels/deepnote/environments/deepnoteEnvironmentTreeItem.node.ts(1 hunks)src/kernels/deepnote/environments/deepnoteEnvironmentTreeItem.unit.test.ts(1 hunks)src/kernels/deepnote/environments/deepnoteEnvironmentsActivationService.ts(1 hunks)src/kernels/deepnote/environments/deepnoteEnvironmentsActivationService.unit.test.ts(1 hunks)src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts(1 hunks)src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts(1 hunks)src/kernels/deepnote/environments/deepnoteNotebookEnvironmentMapper.node.ts(1 hunks)src/kernels/deepnote/environments/deepnoteNotebookEnvironmentMapper.unit.test.ts(1 hunks)src/kernels/deepnote/types.ts(8 hunks)src/kernels/helpers.ts(2 hunks)src/kernels/kernel.ts(1 hunks)src/notebooks/controllers/vscodeNotebookController.ts(3 hunks)src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts(6 hunks)src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts(1 hunks)src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts(3 hunks)src/notebooks/serviceRegistry.node.ts(2 hunks)src/platform/common/utils/localize.ts(2 hunks)src/platform/deepnote/deepnoteServerUtils.node.ts(1 hunks)src/platform/deepnote/deepnoteUriUtils.node.ts(1 hunks)src/platform/deepnote/deepnoteUriUtils.ts(1 hunks)src/platform/logging/index.ts(1 hunks)
💤 Files with no reviewable changes (1)
- ORPHAN_PROCESS_CLEANUP_IMPLEMENTATION.md
🧰 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/platform/common/utils/localize.tssrc/kernels/deepnote/environments/deepnoteEnvironmentManager.unit.test.tssrc/kernels/deepnote/environments/deepnoteEnvironmentsActivationService.tssrc/platform/logging/index.tssrc/kernels/deepnote/environments/deepnoteEnvironment.tssrc/kernels/helpers.tssrc/kernels/kernel.tssrc/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.tssrc/kernels/deepnote/environments/deepnoteEnvironmentsActivationService.unit.test.tssrc/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.tssrc/kernels/deepnote/environments/deepnoteEnvironmentTreeDataProvider.unit.test.tssrc/platform/deepnote/deepnoteUriUtils.tssrc/kernels/deepnote/environments/deepnoteEnvironmentStorage.unit.test.tssrc/kernels/deepnote/environments/deepnoteEnvironmentTreeItem.unit.test.tssrc/kernels/deepnote/environments/deepnoteNotebookEnvironmentMapper.unit.test.tssrc/kernels/deepnote/types.tssrc/notebooks/controllers/vscodeNotebookController.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
Files:
src/platform/common/utils/localize.tssrc/kernels/deepnote/environments/deepnoteEnvironmentManager.unit.test.tssrc/kernels/deepnote/environments/deepnoteEnvironmentsActivationService.tssrc/platform/logging/index.tssrc/platform/deepnote/deepnoteServerUtils.node.tssrc/kernels/deepnote/environments/deepnoteEnvironment.tssrc/kernels/helpers.tssrc/kernels/kernel.tssrc/kernels/deepnote/environments/deepnoteEnvironmentTreeDataProvider.node.tssrc/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.tssrc/kernels/deepnote/deepnoteServerProvider.node.tssrc/kernels/deepnote/environments/deepnoteNotebookEnvironmentMapper.node.tssrc/kernels/deepnote/environments/deepnoteEnvironmentsActivationService.unit.test.tssrc/kernels/deepnote/deepnoteSharedToolkitInstaller.node.tssrc/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.tssrc/kernels/deepnote/environments/deepnoteEnvironmentTreeDataProvider.unit.test.tssrc/platform/deepnote/deepnoteUriUtils.tssrc/kernels/deepnote/environments/deepnoteEnvironmentStorage.unit.test.tssrc/kernels/deepnote/environments/deepnoteEnvironmentTreeItem.unit.test.tssrc/platform/deepnote/deepnoteUriUtils.node.tssrc/kernels/deepnote/environments/deepnoteEnvironmentStorage.node.tssrc/kernels/deepnote/environments/deepnoteEnvironmentManager.node.tssrc/kernels/deepnote/environments/deepnoteNotebookEnvironmentMapper.unit.test.tssrc/notebooks/deepnote/deepnoteRequirementsHelper.node.tssrc/kernels/deepnote/types.tssrc/kernels/deepnote/deepnoteServerStarter.node.tssrc/notebooks/deepnote/deepnoteKernelAutoSelector.node.tssrc/kernels/deepnote/environments/deepnoteEnvironmentTreeItem.node.tssrc/kernels/deepnote/environments/deepnoteEnvironmentsView.node.tssrc/notebooks/serviceRegistry.node.tssrc/kernels/deepnote/deepnoteToolkitInstaller.node.tssrc/notebooks/controllers/vscodeNotebookController.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/utils/localize.tssrc/platform/logging/index.tssrc/platform/deepnote/deepnoteServerUtils.node.tssrc/platform/deepnote/deepnoteUriUtils.tssrc/platform/deepnote/deepnoteUriUtils.node.ts
**/*.unit.test.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place unit tests in files matching
*.unit.test.ts
Files:
src/kernels/deepnote/environments/deepnoteEnvironmentManager.unit.test.tssrc/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.tssrc/kernels/deepnote/environments/deepnoteEnvironmentsActivationService.unit.test.tssrc/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.tssrc/kernels/deepnote/environments/deepnoteEnvironmentTreeDataProvider.unit.test.tssrc/kernels/deepnote/environments/deepnoteEnvironmentStorage.unit.test.tssrc/kernels/deepnote/environments/deepnoteEnvironmentTreeItem.unit.test.tssrc/kernels/deepnote/environments/deepnoteNotebookEnvironmentMapper.unit.test.ts
src/kernels/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)
src/kernels/**/*.ts: Use event-driven updates (EventEmitter) for state changes
Monitor and dispose pending promises to prevent leaks during teardown
Use WeakRef/weak references for notebook/object backreferences to avoid memory leaks
Respect CancellationToken in all async operations
Files:
src/kernels/deepnote/environments/deepnoteEnvironmentManager.unit.test.tssrc/kernels/deepnote/environments/deepnoteEnvironmentsActivationService.tssrc/kernels/deepnote/environments/deepnoteEnvironment.tssrc/kernels/helpers.tssrc/kernels/kernel.tssrc/kernels/deepnote/environments/deepnoteEnvironmentTreeDataProvider.node.tssrc/kernels/deepnote/deepnoteServerProvider.node.tssrc/kernels/deepnote/environments/deepnoteNotebookEnvironmentMapper.node.tssrc/kernels/deepnote/environments/deepnoteEnvironmentsActivationService.unit.test.tssrc/kernels/deepnote/deepnoteSharedToolkitInstaller.node.tssrc/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.tssrc/kernels/deepnote/environments/deepnoteEnvironmentTreeDataProvider.unit.test.tssrc/kernels/deepnote/environments/deepnoteEnvironmentStorage.unit.test.tssrc/kernels/deepnote/environments/deepnoteEnvironmentTreeItem.unit.test.tssrc/kernels/deepnote/environments/deepnoteEnvironmentStorage.node.tssrc/kernels/deepnote/environments/deepnoteEnvironmentManager.node.tssrc/kernels/deepnote/environments/deepnoteNotebookEnvironmentMapper.unit.test.tssrc/kernels/deepnote/types.tssrc/kernels/deepnote/deepnoteServerStarter.node.tssrc/kernels/deepnote/environments/deepnoteEnvironmentTreeItem.node.tssrc/kernels/deepnote/environments/deepnoteEnvironmentsView.node.tssrc/kernels/deepnote/deepnoteToolkitInstaller.node.ts
**/*.{test,spec}.ts
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
In unit tests, when a mock is returned from a promise, ensure the mocked instance has an undefined
thenproperty to avoid hanging tests
Files:
src/kernels/deepnote/environments/deepnoteEnvironmentManager.unit.test.tssrc/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.tssrc/kernels/deepnote/environments/deepnoteEnvironmentsActivationService.unit.test.tssrc/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.tssrc/kernels/deepnote/environments/deepnoteEnvironmentTreeDataProvider.unit.test.tssrc/kernels/deepnote/environments/deepnoteEnvironmentStorage.unit.test.tssrc/kernels/deepnote/environments/deepnoteEnvironmentTreeItem.unit.test.tssrc/kernels/deepnote/environments/deepnoteNotebookEnvironmentMapper.unit.test.ts
src/platform/logging/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/platform.instructions.md)
Implement logging using OutputChannelLogger/ConsoleLogger and honor LogLevel; ensure PII scrubbing in log output
Files:
src/platform/logging/index.ts
**/*.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/platform/deepnote/deepnoteServerUtils.node.tssrc/kernels/deepnote/environments/deepnoteEnvironmentTreeDataProvider.node.tssrc/kernels/deepnote/deepnoteServerProvider.node.tssrc/kernels/deepnote/environments/deepnoteNotebookEnvironmentMapper.node.tssrc/kernels/deepnote/deepnoteSharedToolkitInstaller.node.tssrc/platform/deepnote/deepnoteUriUtils.node.tssrc/kernels/deepnote/environments/deepnoteEnvironmentStorage.node.tssrc/kernels/deepnote/environments/deepnoteEnvironmentManager.node.tssrc/notebooks/deepnote/deepnoteRequirementsHelper.node.tssrc/kernels/deepnote/deepnoteServerStarter.node.tssrc/notebooks/deepnote/deepnoteKernelAutoSelector.node.tssrc/kernels/deepnote/environments/deepnoteEnvironmentTreeItem.node.tssrc/kernels/deepnote/environments/deepnoteEnvironmentsView.node.tssrc/notebooks/serviceRegistry.node.tssrc/kernels/deepnote/deepnoteToolkitInstaller.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/deepnote/deepnoteServerUtils.node.tssrc/platform/deepnote/deepnoteUriUtils.node.ts
src/kernels/kernel.ts
📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)
Kernel must manage sessions via IKernelSession and implement lifecycle hooks (willRestart, willInterrupt, etc.) with startup code execution (e.g., matplotlib, IPyWidgets)
Files:
src/kernels/kernel.ts
src/kernels/{kernel.ts,kernelSessionFactory.ts}
📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)
Prefer session reuse to avoid unnecessary kernel restarts
Files:
src/kernels/kernel.ts
🧬 Code graph analysis (26)
src/kernels/deepnote/environments/deepnoteEnvironmentManager.unit.test.ts (1)
src/kernels/deepnote/types.ts (2)
IDeepnoteServerStarter(126-126)IDeepnoteServerStarter(127-162)
src/kernels/deepnote/environments/deepnoteEnvironmentsActivationService.ts (5)
src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts (1)
injectable(39-584)src/kernels/deepnote/environments/deepnoteEnvironmentManager.node.ts (1)
injectable(17-210)src/kernels/deepnote/types.ts (2)
IDeepnoteEnvironmentManager(217-217)IDeepnoteEnvironmentManager(218-278)src/platform/common/constants.ts (1)
STANDARD_OUTPUT_CHANNEL(32-32)src/platform/logging/index.ts (1)
logger(35-48)
src/platform/deepnote/deepnoteServerUtils.node.ts (1)
src/platform/common/crypto.ts (1)
createHash(9-11)
src/kernels/deepnote/environments/deepnoteEnvironment.ts (1)
src/kernels/deepnote/types.ts (1)
DeepnoteServerInfo(164-169)
src/kernels/kernel.ts (2)
src/platform/logging/index.ts (1)
logger(35-48)src/platform/common/platform/fs-paths.ts (1)
getDisplayPath(33-74)
src/kernels/deepnote/environments/deepnoteEnvironmentTreeDataProvider.node.ts (3)
src/kernels/deepnote/environments/deepnoteEnvironmentManager.node.ts (1)
injectable(17-210)src/kernels/deepnote/environments/deepnoteEnvironmentTreeItem.node.ts (1)
DeepnoteEnvironmentTreeItem(19-139)src/kernels/deepnote/types.ts (2)
IDeepnoteEnvironmentManager(217-217)IDeepnoteEnvironmentManager(218-278)
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts (6)
src/kernels/deepnote/types.ts (8)
IDeepnoteServerProvider(171-171)IDeepnoteServerProvider(172-185)IDeepnoteEnvironmentManager(217-217)IDeepnoteEnvironmentManager(218-278)IDeepnoteServerStarter(126-126)IDeepnoteServerStarter(127-162)IDeepnoteNotebookEnvironmentMapper(280-280)IDeepnoteNotebookEnvironmentMapper(281-308)src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts (2)
IDeepnoteRequirementsHelper(8-8)IDeepnoteRequirementsHelper(9-11)src/test/mocks/vsc/index.ts (1)
CancellationToken(100-112)src/notebooks/controllers/types.ts (1)
IVSCodeNotebookController(23-44)src/kernels/types.ts (1)
IKernel(439-446)src/kernels/deepnote/environments/deepnoteEnvironment.ts (1)
DeepnoteEnvironment(9-60)
src/kernels/deepnote/environments/deepnoteNotebookEnvironmentMapper.node.ts (3)
src/platform/deepnote/deepnoteUriUtils.node.ts (2)
getDeepnoteNotebookStorageKey(31-31)getLegacyDeepnoteNotebookStorageKey(10-13)src/platform/deepnote/deepnoteUriUtils.ts (1)
getDeepnoteNotebookStorageKey(8-11)src/platform/logging/index.ts (1)
logger(35-48)
src/kernels/deepnote/environments/deepnoteEnvironmentsActivationService.unit.test.ts (1)
src/kernels/deepnote/types.ts (2)
IDeepnoteEnvironmentManager(217-217)IDeepnoteEnvironmentManager(218-278)
src/kernels/deepnote/deepnoteSharedToolkitInstaller.node.ts (1)
src/platform/logging/index.ts (1)
logger(35-48)
src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts (6)
src/kernels/deepnote/types.ts (6)
IDeepnoteEnvironmentManager(217-217)IDeepnoteEnvironmentManager(218-278)IDeepnoteKernelAutoSelector(187-187)IDeepnoteKernelAutoSelector(188-215)IDeepnoteNotebookEnvironmentMapper(280-280)IDeepnoteNotebookEnvironmentMapper(281-308)src/test/vscode-mock.ts (2)
resetVSCodeMocks(60-79)mockedVSCodeNamespaces(17-17)src/kernels/deepnote/environments/deepnoteEnvironment.ts (1)
DeepnoteEnvironment(9-60)src/test/mocks/vsc/index.ts (1)
CancellationToken(100-112)src/platform/deepnote/deepnoteUriUtils.ts (1)
getDeepnoteProjectStorageKey(17-23)src/platform/deepnote/deepnoteServerUtils.node.ts (1)
createDeepnoteServerConfigHandle(3-6)
src/kernels/deepnote/environments/deepnoteEnvironmentTreeDataProvider.unit.test.ts (3)
src/kernels/deepnote/types.ts (2)
IDeepnoteEnvironmentManager(217-217)IDeepnoteEnvironmentManager(218-278)src/test/mocks/vsc/index.ts (1)
EventEmitter(65-89)src/kernels/deepnote/environments/deepnoteEnvironment.ts (1)
DeepnoteEnvironment(9-60)
src/kernels/deepnote/environments/deepnoteEnvironmentStorage.unit.test.ts (1)
src/kernels/deepnote/environments/deepnoteEnvironment.ts (1)
DeepnoteEnvironmentState(66-79)
src/kernels/deepnote/environments/deepnoteEnvironmentTreeItem.unit.test.ts (3)
src/kernels/deepnote/environments/deepnoteEnvironment.ts (1)
DeepnoteEnvironment(9-60)src/kernels/deepnote/environments/deepnoteEnvironmentTreeItem.node.ts (1)
DeepnoteEnvironmentTreeItem(19-139)src/test/mocks/vsc/extHostedTypes.ts (1)
ThemeIcon(2094-2104)
src/platform/deepnote/deepnoteUriUtils.node.ts (1)
src/platform/deepnote/deepnoteUriUtils.ts (2)
getDeepnoteNotebookStorageKey(8-11)getDeepnoteProjectStorageKey(17-23)
src/kernels/deepnote/environments/deepnoteEnvironmentStorage.node.ts (1)
src/kernels/deepnote/environments/deepnoteEnvironment.ts (2)
DeepnoteEnvironment(9-60)DeepnoteEnvironmentState(66-79)
src/kernels/deepnote/environments/deepnoteEnvironmentManager.node.ts (1)
src/kernels/deepnote/types.ts (4)
IDeepnoteEnvironmentManager(217-217)IDeepnoteEnvironmentManager(218-278)IDeepnoteServerStarter(126-126)IDeepnoteServerStarter(127-162)
src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts (2)
src/platform/logging/types.ts (2)
ILogger(9-9)ILogger(10-19)src/platform/logging/index.ts (1)
logger(35-48)
src/kernels/deepnote/types.ts (1)
src/kernels/deepnote/environments/deepnoteEnvironment.ts (2)
CreateDeepnoteEnvironmentOptions(84-89)DeepnoteEnvironment(9-60)
src/kernels/deepnote/deepnoteServerStarter.node.ts (4)
src/kernels/deepnote/types.ts (2)
DeepnoteServerInfo(164-169)DEEPNOTE_DEFAULT_PORT(311-311)src/platform/logging/index.ts (1)
logger(35-48)src/platform/deepnote/deepnoteUriUtils.ts (1)
getDeepnoteNotebookStorageKey(8-11)src/platform/errors/deepnoteKernelErrors.ts (2)
DeepnoteServerTimeoutError(252-287)DeepnoteServerStartupError(174-247)
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts (7)
src/notebooks/controllers/types.ts (1)
IVSCodeNotebookController(23-44)src/kernels/deepnote/types.ts (7)
IDeepnoteEnvironmentManager(217-217)IDeepnoteEnvironmentManager(218-278)IDeepnoteServerStarter(126-126)IDeepnoteServerStarter(127-162)IDeepnoteNotebookEnvironmentMapper(280-280)IDeepnoteNotebookEnvironmentMapper(281-308)DeepnoteKernelConnectionMetadata(21-80)src/kernels/deepnote/environments/deepnoteEnvironment.ts (1)
DeepnoteEnvironment(9-60)src/platform/deepnote/deepnoteUriUtils.node.ts (3)
getDeepnoteNotebookStorageKey(31-31)getDeepnoteNotebookKeyHash(18-21)getDeepnoteProjectStorageKey(31-31)src/platform/deepnote/deepnoteServerUtils.node.ts (1)
createDeepnoteServerConfigHandle(3-6)src/kernels/jupyter/jupyterUtils.ts (1)
createJupyterConnectionInfo(83-179)src/platform/common/utils.ts (1)
disposeAsync(458-465)
src/kernels/deepnote/environments/deepnoteEnvironmentTreeItem.node.ts (2)
src/test/mocks/vsc/extHostedTypes.ts (2)
TreeItem(2066-2086)ThemeIcon(2094-2104)src/kernels/deepnote/environments/deepnoteEnvironment.ts (1)
DeepnoteEnvironment(9-60)
src/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts (6)
src/kernels/deepnote/environments/deepnoteNotebookEnvironmentMapper.node.ts (1)
injectable(17-129)src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts (1)
injectable(62-821)src/kernels/deepnote/environments/deepnoteEnvironmentManager.node.ts (1)
injectable(17-210)src/platform/interpreter/helpers.ts (3)
resolvedPythonEnvToJupyterEnv(229-254)getCachedEnvironment(148-162)getPythonEnvironmentName(65-74)src/platform/deepnote/deepnoteUriUtils.ts (1)
getDeepnoteProjectStorageKey(17-23)src/platform/deepnote/deepnoteServerUtils.node.ts (1)
createDeepnoteServerConfigHandle(3-6)
src/notebooks/serviceRegistry.node.ts (1)
src/kernels/deepnote/types.ts (4)
IDeepnoteEnvironmentManager(217-217)IDeepnoteEnvironmentManager(218-278)IDeepnoteNotebookEnvironmentMapper(280-280)IDeepnoteNotebookEnvironmentMapper(281-308)
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (1)
src/kernels/deepnote/types.ts (2)
VenvAndToolkitInstallation(12-15)DEEPNOTE_TOOLKIT_VERSION(310-310)
src/notebooks/controllers/vscodeNotebookController.ts (4)
src/kernels/helpers.ts (2)
areKernelConnectionsEqual(592-614)getDisplayNameOrNameOfKernelConnection(250-319)src/platform/logging/index.ts (1)
logger(35-48)src/platform/common/platform/fs-paths.ts (1)
getDisplayPath(33-74)src/platform/common/utils/misc.ts (1)
noop(11-11)
🪛 GitHub Actions: CI
src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts
[warning] 1-1: Code style issues found in the file. Run Prettier to fix.
🪛 markdownlint-cli2 (0.18.1)
DEBUGGING_KERNEL_MANAGEMENT.md
23-23: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
23-23: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
39-39: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
39-39: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
59-59: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
59-59: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
86-86: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
101-101: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
119-119: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
138-138: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
156-156: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
180-180: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
199-199: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
226-226: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
245-245: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
254-254: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
254-254: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
261-261: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
445-445: Files should end with a single newline character
(MD047, single-trailing-newline)
🔇 Additional comments (12)
cspell.json (1)
44-44: LGTM!Adding
matplotlib,scipy, andsklearnto the spell-check dictionary is appropriate for a data science extension. These libraries are likely referenced in kernel/environment management code elsewhere in this PR.Also applies to: 55-55, 56-56
.vscode/launch.json (1)
12-12: Extension ID verified—no issues found.The proposed API flag correctly references
Deepnote.vscode-deepnote, matching the publisher (Deepnote) and name (vscode-deepnote) fields in package.json.src/platform/logging/index.ts (1)
67-67: Verify: Auto-show output channel is intentional.Displaying the log channel by default on every extension activation may clutter the UI for users who don't need logs. Confirm this aligns with the desired UX—especially since the PR emphasizes Deepnote branding updates that would make this more prominent.
src/kernels/kernel.ts (1)
488-488: LGTM: Enhanced observability.The additional log statement provides useful context for debugging session startup. Logging both display path and raw URI is helpful.
src/platform/common/utils/localize.ts (1)
42-42: LGTM: Consistent branding update.The Jupyter → Deepnote rebranding is applied consistently to user-facing channel names.
Also applies to: 709-709
src/kernels/deepnote/deepnoteServerProvider.node.ts (1)
88-88: LGTM: Improved server labeling.Exposing jupyter and lsp ports in the server label aids debugging in multi-environment setups.
CLAUDE.md (1)
1-42: LGTM: Enhanced developer documentation.The expanded guidelines for imports, testing, and architecture align well with the new environment-management features introduced in this PR.
src/kernels/deepnote/deepnoteSharedToolkitInstaller.node.ts (1)
5-5: LGTM: Improved logging and localization.Structured logging (passing context objects rather than concatenating strings) and localized user-facing messages enhance maintainability and internationalization. Good adherence to coding guidelines.
Also applies to: 101-101, 105-105, 201-201, 243-243, 247-247, 252-253
package.nls.json (1)
278-288: LGTM: Localization for new environment features.The added localization keys support the new environment management UI and commands introduced in this PR. Naming is consistent with existing patterns.
src/kernels/deepnote/environments/deepnoteEnvironmentStorage.node.ts (1)
83-108: No issues found. Code correctly implements PythonEnvironment.The
PythonEnvironmentinterface requires onlyidandurifields. The deserialized object provides exactly these fields and fully satisfies the type contract. Noversionfield exists in the interface definition.src/kernels/deepnote/deepnoteServerStarter.node.ts (1)
208-241: Do not kill shared servers still in use.When notebook A (projectKey P) is closed,
stopServertears downserverKey = Punconditionally. If notebook B from the same project is still running, its kernel is terminated mid-execution. Please keep reference counts (e.g., detect otherlegacyServerKeyMapentries that still containserverKey) and only stop when this was the last consumer, otherwise skip the shutdown. raise_critical_issuesrc/kernels/deepnote/environments/deepnoteEnvironmentsView.node.ts (1)
531-573: Allow clearing the package list.
validateInputrejects empty input, so once packages are set there’s no way to remove them (only cancel). Let empty/whitespace mean “clear packages” and persist[]; otherwise users are stuck with stale installs. raise_major_issue
Comment on lines
+1
to
+445
| # Debugging Kernel Configuration Management | ||
|
|
||
| ## Quick Start: See It In Action | ||
|
|
||
| ### 1. Launch Extension in Debug Mode | ||
|
|
||
| 1. Open this project in VS Code | ||
| 2. Press **F5** (or Run > Start Debugging) | ||
| 3. Select **"Extension"** configuration | ||
| 4. A new **Extension Development Host** window opens | ||
|
|
||
| ### 2. Find the Kernel Management UI | ||
|
|
||
| **In the Extension Development Host window:** | ||
|
|
||
| 1. Look for the **Deepnote icon** in the Activity Bar (left sidebar) | ||
| 2. Click it to open the Deepnote view | ||
| 3. You should see two sections: | ||
| - **DEEPNOTE EXPLORER** (existing notebook browser) | ||
| - **DEEPNOTE KERNEL CONFIGURATIONS** ⬅️ **NEW!** | ||
|
|
||
| **Initial State:** | ||
| ``` | ||
| DEEPNOTE KERNEL CONFIGURATIONS | ||
| └─ [+] Create New Configuration | ||
| ``` | ||
|
|
||
| ### 3. Create Your First Configuration | ||
|
|
||
| 1. Click **"Create New Configuration"** button | ||
| 2. Follow the wizard: | ||
| - **Select Python interpreter** (choose any available) | ||
| - **Enter name**: e.g., "My Test Config" | ||
| - **Enter packages** (optional): e.g., "pandas, numpy" | ||
| 3. Watch the progress notification | ||
| 4. Configuration appears in the tree! | ||
|
|
||
| **After Creation:** | ||
| ``` | ||
| DEEPNOTE KERNEL CONFIGURATIONS | ||
| ├─ ⚪ My Test Config [Stopped] | ||
| │ ├─ Python: /usr/bin/python3.11 | ||
| │ ├─ Venv: .../deepnote-venvs/{uuid} | ||
| │ ├─ Packages: pandas, numpy | ||
| │ ├─ Created: 1/15/2025, 10:00:00 AM | ||
| │ └─ Last used: 1/15/2025, 10:00:00 AM | ||
| └─ [+] Create New Configuration | ||
| ``` | ||
|
|
||
| ### 4. Start the Server | ||
|
|
||
| 1. Right-click the configuration | ||
| 2. Select **"Start Server"** (or click the play button ▶️) | ||
| 3. Watch the output channel for logs | ||
| 4. Icon changes to 🟢 **[Running]** | ||
| 5. Port and URL appear in the tree | ||
|
|
||
| **Running State:** | ||
| ``` | ||
| DEEPNOTE KERNEL CONFIGURATIONS | ||
| ├─ 🟢 My Test Config [Running] | ||
| │ ├─ Port: 8888 | ||
| │ ├─ URL: http://localhost:8888 | ||
| │ ├─ Python: /usr/bin/python3.11 | ||
| │ ├─ Venv: .../deepnote-venvs/{uuid} | ||
| │ ├─ Packages: pandas, numpy | ||
| │ ├─ Created: 1/15/2025, 10:00:00 AM | ||
| │ └─ Last used: 1/15/2025, 10:05:23 AM | ||
| └─ [+] Create New Configuration | ||
| ``` | ||
|
|
||
| ### 5. Use Configuration with a Notebook | ||
|
|
||
| 1. Open (or create) a `.deepnote` file | ||
| 2. A **picker dialog** appears automatically | ||
| 3. Select your configuration from the list | ||
| 4. Notebook connects to the running server | ||
| 5. Execute cells - they run in your configured environment! | ||
|
|
||
| --- | ||
|
|
||
| ## Key Debugging Locations | ||
|
|
||
| ### Files to Set Breakpoints In | ||
|
|
||
| #### **1. Activation & Initialization** | ||
| **File:** `src/kernels/deepnote/configurations/deepnoteConfigurationsActivationService.ts` | ||
|
|
||
| **Key Lines:** | ||
| - Line 27: `activate()` - Entry point | ||
| - Line 30: `configurationManager.initialize()` - Load configs from storage | ||
|
|
||
| ```typescript | ||
| // Set breakpoint here to see extension activation | ||
| public activate(): void { | ||
| logger.info('Activating Deepnote kernel configurations view'); | ||
| // ... | ||
| } | ||
| ``` | ||
|
|
||
| #### **2. Creating Configurations** | ||
| **File:** `src/kernels/deepnote/configurations/deepnoteConfigurationsView.ts` | ||
|
|
||
| **Key Lines:** | ||
| - Line 64: `createConfiguration()` command handler | ||
| - Line 238: `configurationManager.createConfiguration()` call | ||
|
|
||
| ```typescript | ||
| // Set breakpoint here to see configuration creation | ||
| private async createConfiguration(): Promise<void> { | ||
| try { | ||
| // Step 1: Select Python interpreter | ||
| const api = await this.pythonApiProvider.getNewApi(); | ||
| // ... | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| #### **3. Configuration Manager (Business Logic)** | ||
| **File:** `src/kernels/deepnote/configurations/deepnoteConfigurationManager.ts` | ||
|
|
||
| **Key Lines:** | ||
| - Line 45: `initialize()` - Load from storage | ||
| - Line 63: `createConfiguration()` - Create new config | ||
| - Line 174: `startServer()` - Start Jupyter server | ||
| - Line 215: `stopServer()` - Stop server | ||
|
|
||
| ```typescript | ||
| // Set breakpoint here to see config creation | ||
| public async createConfiguration(options: CreateKernelConfigurationOptions): Promise<DeepnoteKernelConfiguration> { | ||
| const id = uuid(); | ||
| const venvPath = Uri.joinPath(this.context.globalStorageUri, 'deepnote-venvs', id); | ||
| // ... | ||
| this._onDidChangeConfigurations.fire(); // ← Watch this fire! | ||
| } | ||
| ``` | ||
|
|
||
| #### **4. TreeDataProvider (UI Updates)** | ||
| **File:** `src/kernels/deepnote/configurations/deepnoteConfigurationTreeDataProvider.ts` | ||
|
|
||
| **Key Lines:** | ||
| - Line 19-21: Event listener setup | ||
| - Line 25: `refresh()` - Triggers tree update | ||
| - Line 32: `getChildren()` - VS Code calls this to refresh | ||
|
|
||
| ```typescript | ||
| // Set breakpoint here to see event propagation | ||
| constructor(private readonly configurationManager: IDeepnoteConfigurationManager) { | ||
| // Listen to configuration changes and refresh the tree | ||
| this.configurationManager.onDidChangeConfigurations(() => { | ||
| this.refresh(); // ← Breakpoint here! | ||
| }); | ||
| } | ||
| ``` | ||
|
|
||
| #### **5. Server Lifecycle** | ||
| **File:** `src/kernels/deepnote/configurations/deepnoteConfigurationManager.ts` | ||
|
|
||
| **Key Lines:** | ||
| - Line 189: `ensureVenvAndToolkit()` - Create venv | ||
| - Line 192: `installAdditionalPackages()` - Install packages | ||
| - Line 197: `serverStarter.startServer()` - Launch Jupyter | ||
|
|
||
| ```typescript | ||
| // Set breakpoint here to see server startup | ||
| public async startServer(id: string): Promise<void> { | ||
| const config = this.configurations.get(id); | ||
| if (!config) { | ||
| throw new Error(`Configuration not found: ${id}`); | ||
| } | ||
|
|
||
| // First ensure venv is created and toolkit is installed | ||
| await this.toolkitInstaller.ensureVenvAndToolkit(config.pythonInterpreter, config.venvPath); | ||
| // ... | ||
| // Start the Jupyter server | ||
| const serverInfo = await this.serverStarter.startServer(config.pythonInterpreter, config.venvPath, id); | ||
| } | ||
| ``` | ||
|
|
||
| #### **6. Notebook Integration (Configuration Picker)** | ||
| **File:** `src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts` | ||
|
|
||
| **Key Lines:** | ||
| - Look for `ensureKernelSelected()` method | ||
| - Look for calls to `configurationPicker.pickConfiguration()` | ||
| - Look for calls to `mapper.getConfigurationForNotebook()` | ||
|
|
||
| --- | ||
|
|
||
| ## Visual Debugging Tips | ||
|
|
||
| ### **1. Watch the Output Channel** | ||
|
|
||
| When the extension is running, look for: | ||
| - **Output Panel** (View > Output) | ||
| - Select **"Deepnote"** from the dropdown | ||
| - You'll see logs like: | ||
|
|
||
| ``` | ||
| [info] Activating Deepnote kernel configurations view | ||
| [info] Initialized configuration manager with 0 configurations | ||
| [info] Creating virtual environment at /path/to/venv | ||
| [info] Installing deepnote-toolkit and ipykernel in venv from https://... | ||
| [info] Created new kernel configuration: My Test Config (uuid-123) | ||
| [info] Starting server for configuration: My Test Config (uuid-123) | ||
| [info] Deepnote server started successfully at http://localhost:8888 | ||
| ``` | ||
|
|
||
| ### **2. Watch VS Code's Developer Tools** | ||
|
|
||
| Open Developer Tools: | ||
| - **Help > Toggle Developer Tools** | ||
| - **Console tab**: See any JavaScript errors | ||
| - **Network tab**: See server requests (when executing cells) | ||
|
|
||
| ### **3. Watch Global State (Persistence)** | ||
|
|
||
| To see what's stored: | ||
|
|
||
| 1. Set breakpoint in `deepnoteConfigurationStorage.ts` | ||
| 2. At line 56: `await this.globalState.update(STORAGE_KEY, states)` | ||
| 3. Inspect the `states` variable | ||
| 4. You'll see the JSON being persisted | ||
|
|
||
| **Example stored data:** | ||
| ```json | ||
| { | ||
| "deepnote.kernelConfigurations": [ | ||
| { | ||
| "id": "abc-123-def", | ||
| "name": "My Test Config", | ||
| "pythonInterpreterPath": "/usr/bin/python3.11", | ||
| "venvPath": "/Users/.../deepnote-venvs/abc-123-def", | ||
| "createdAt": "2025-01-15T10:00:00.000Z", | ||
| "lastUsedAt": "2025-01-15T10:00:00.000Z", | ||
| "packages": ["pandas", "numpy"] | ||
| } | ||
| ] | ||
| } | ||
| ``` | ||
|
|
||
| ### **4. Watch Server Processes** | ||
|
|
||
| In your terminal: | ||
| ```bash | ||
| # Find running deepnote-toolkit servers | ||
| ps aux | grep deepnote_toolkit | ||
|
|
||
| # Or on Windows | ||
| tasklist | findstr python | ||
| ``` | ||
|
|
||
| You should see processes like: | ||
| ``` | ||
| python -m deepnote_toolkit server --jupyter-port 8888 | ||
| ``` | ||
|
|
||
| ### **5. Check Venv Directories** | ||
|
|
||
| Navigate to: | ||
| ```bash | ||
| # macOS/Linux | ||
| cd ~/.vscode/extensions/.../globalStorage/deepnote-venvs/ | ||
|
|
||
| # Windows | ||
| cd %APPDATA%\Code\User\globalStorage\...\deepnote-venvs\ | ||
| ``` | ||
|
|
||
| You'll see directories named with UUIDs, each containing a Python venv. | ||
|
|
||
| --- | ||
|
|
||
| ## Common Debugging Scenarios | ||
|
|
||
| ### **Scenario 1: Configuration Not Appearing in Tree** | ||
|
|
||
| **Set breakpoints:** | ||
| 1. `deepnoteConfigurationManager.ts:80` - Check if `_onDidChangeConfigurations.fire()` is called | ||
| 2. `deepnoteConfigurationTreeDataProvider.ts:20` - Check if event listener is triggered | ||
| 3. `deepnoteConfigurationTreeDataProvider.ts:25` - Check if `refresh()` is called | ||
| 4. `deepnoteConfigurationTreeDataProvider.ts:46` - Check if `getRootItems()` returns the config | ||
|
|
||
| **Debug steps:** | ||
| - Verify `this.configurations` Map contains the config | ||
| - Verify event propagation chain | ||
| - Check if tree view is registered with VS Code | ||
|
|
||
| ### **Scenario 2: Server Won't Start** | ||
|
|
||
| **Set breakpoints:** | ||
| 1. `deepnoteConfigurationManager.ts:174` - `startServer()` entry | ||
| 2. `deepnoteToolkitInstaller.node.ts:74` - `ensureVenvAndToolkit()` entry | ||
| 3. `deepnoteServerStarter.node.ts:76` - `startServer()` entry | ||
|
|
||
| **Check:** | ||
| - Output channel for error messages | ||
| - Venv creation succeeded | ||
| - Python interpreter is valid | ||
| - Port is not already in use | ||
|
|
||
| ### **Scenario 3: Notebook Picker Not Appearing** | ||
|
|
||
| **Set breakpoints:** | ||
| 1. `deepnoteKernelAutoSelector.node.ts` - `ensureKernelSelected()` method | ||
| 2. Check if `mapper.getConfigurationForNotebook()` returns undefined | ||
| 3. Check if `picker.pickConfiguration()` is called | ||
|
|
||
| **Verify:** | ||
| - Picker service is registered in DI container | ||
| - Mapper service is registered in DI container | ||
| - Notebook URI is being normalized correctly | ||
|
|
||
| --- | ||
|
|
||
| ## EventEmitter Pattern Debugging | ||
|
|
||
| To trace the event flow (from my earlier explanation): | ||
|
|
||
| ### **1. Set Breakpoints in Sequence:** | ||
|
|
||
| ```typescript | ||
| // 1. Manager fires event | ||
| // deepnoteConfigurationManager.ts:80 | ||
| this._onDidChangeConfigurations.fire(); | ||
|
|
||
| // 2. TreeProvider receives event | ||
| // deepnoteConfigurationTreeDataProvider.ts:20 | ||
| this.configurationManager.onDidChangeConfigurations(() => { | ||
| this.refresh(); // ← Breakpoint | ||
|
|
||
| // 3. TreeProvider fires its event | ||
| // deepnoteConfigurationTreeDataProvider.ts:25 | ||
| public refresh(): void { | ||
| this._onDidChangeTreeData.fire(); // ← Breakpoint | ||
|
|
||
| // 4. VS Code calls getChildren | ||
| // deepnoteConfigurationTreeDataProvider.ts:32 | ||
| public async getChildren(element?: DeepnoteConfigurationTreeItem): Promise<DeepnoteConfigurationTreeItem[]> { | ||
| // ← Breakpoint | ||
| ``` | ||
| ### **2. Watch Variables:** | ||
| - In Manager: `this.configurations` - See all configs | ||
| - In TreeProvider: `this._onDidChangeTreeData` - See EventEmitter | ||
| - In `getChildren`: `element` parameter - See what VS Code is requesting | ||
| --- | ||
| ## Testing the Complete Flow | ||
| ### **End-to-End Test:** | ||
| 1. **Create Configuration** | ||
| - Set breakpoint: `deepnoteConfigurationsView.ts:238` | ||
| - Click "Create New Configuration" | ||
| - Step through: select interpreter → enter name → create venv | ||
| 2. **Start Server** | ||
| - Set breakpoint: `deepnoteConfigurationManager.ts:197` | ||
| - Right-click config → "Start Server" | ||
| - Step through: install toolkit → start server → update state | ||
| 3. **Open Notebook** | ||
| - Set breakpoint: `deepnoteKernelAutoSelector.node.ts` (ensureKernelSelected) | ||
| - Open a `.deepnote` file | ||
| - Step through: check mapper → show picker → save selection | ||
| 4. **Execute Cell** | ||
| - Open notebook with selected configuration | ||
| - Execute a cell (e.g., `print("Hello")`) | ||
| - Watch server logs in Output channel | ||
| --- | ||
| ## Quick Test Commands | ||
| ```bash | ||
| # 1. Build the extension | ||
| npm run compile | ||
|
|
||
| # 2. Run unit tests | ||
| npm test | ||
|
|
||
| # Or run specific test file | ||
| npx mocha --config ./build/.mocha.unittests.js.json \ | ||
| ./out/src/kernels/deepnote/configurations/deepnoteConfigurationManager.unit.test.js | ||
|
|
||
| # 3. Check for TypeScript errors | ||
| npm run compile:check | ||
|
|
||
| # 4. Format code | ||
| npx prettier --write . | ||
| ``` | ||
| --- | ||
| ## Useful VS Code Commands (in Extension Development Host) | ||
| Press `Cmd+Shift+P` (Mac) or `Ctrl+Shift+P` (Windows/Linux): | ||
| - **Developer: Reload Window** - Reload extension after code changes | ||
| - **Developer: Open Webview Developer Tools** - Debug webviews | ||
| - **Developer: Show Running Extensions** - See if your extension loaded | ||
| - **Deepnote: Create Kernel Configuration** - Test command directly | ||
| - **Deepnote: Refresh** - Manually refresh tree view | ||
| --- | ||
| ## Summary: Where to Look | ||
| | What You Want to See | Where to Look | File/Line | | ||
| |---------------------|---------------|-----------| | ||
| | **UI Tree View** | Sidebar → Deepnote → Kernel Configurations | Activity Bar | | ||
| | **Create Configuration** | Click "+ Create New Configuration" | View controller | | ||
| | **Configuration State** | Expand config item in tree | Tree data provider | | ||
| | **Server Logs** | Output panel → "Deepnote" channel | Output channel | | ||
| | **Persisted Data** | Inspect `globalState` in debugger | Storage layer | | ||
| | **Running Processes** | Terminal: `ps aux \| grep deepnote` | System | | ||
| | **Event Flow** | Breakpoints in Manager → Provider → getChildren | Event chain | | ||
| | **Notebook Picker** | Opens when you open a `.deepnote` file | Auto-selector | | ||
| --- | ||
| ## Pro Tips | ||
| 1. **Use Logpoints** instead of console.log: | ||
| - Right-click in gutter → Add Logpoint | ||
| - Logs appear in Debug Console without modifying code | ||
| 2. **Watch Expressions:** | ||
| - Add to Watch panel: `this.configurations.size` | ||
| - See live count of configurations | ||
| 3. **Conditional Breakpoints:** | ||
| - Right-click breakpoint → Edit Breakpoint → Add condition | ||
| - Example: `config.id === "specific-uuid"` | ||
| 4. **Call Stack Navigation:** | ||
| - When breakpoint hits, examine Call Stack panel | ||
| - See the entire event propagation path | ||
| 5. **Restart Extension Fast:** | ||
| - In Debug toolbar, click restart button (circular arrow) | ||
| - Or use `Cmd+Shift+F5` / `Ctrl+Shift+F5` No newline at end of file |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
LGTM: Comprehensive debugging guide.
This documentation provides valuable debugging guidance for the new environment-management features. The static analysis hints about markdown formatting (missing blank lines around fences, missing language specifiers) are cosmetic—consider addressing them for consistency, but they don't block functionality.
Optional: Address markdown linting issues flagged by static analysis (add blank lines around code fences, specify language for fenced blocks, add trailing newline at EOF).
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
23-23: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
23-23: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
39-39: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
39-39: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
59-59: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
59-59: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
86-86: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
101-101: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
119-119: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
138-138: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
156-156: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
180-180: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
199-199: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
226-226: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
245-245: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
254-254: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
254-254: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
261-261: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
445-445: Files should end with a single newline character
(MD047, single-trailing-newline)
🤖 Prompt for AI Agents
DEBUGGING_KERNEL_MANAGEMENT.md lines 1-445: The review notes cosmetic Markdown
linting issues; add blank lines before and after all fenced code blocks, add
appropriate language specifiers (e.g., ```typescript, ```bash, ```json) to
fenced blocks, and ensure the file ends with a single trailing newline; make
these small formatting edits throughout the document to satisfy static analysis
while preserving existing content.
Comment on lines
+206
to
+208
| public dispose(): void { | ||
| this.outputChannel.dispose(); | ||
| this._onDidChangeEnvironments.dispose(); |
Contributor
There was a problem hiding this comment.
Don’t dispose the shared output channel
The STANDARD_OUTPUT_CHANNEL instance is shared across the extension. Disposing it here tears down the channel for everyone else. Please drop the this.outputChannel.dispose() call (or switch to a dedicated per-manager channel) so other services keep their log sink.
🤖 Prompt for AI Agents
In src/kernels/deepnote/environments/deepnoteEnvironmentManager.node.ts around
lines 206 to 208, the dispose() method is disposing the shared
STANDARD_OUTPUT_CHANNEL via this.outputChannel.dispose(), which tears down a
channel used by other parts of the extension; remove the
this.outputChannel.dispose() call (or alternatively create and use a dedicated
output channel per manager and dispose that instead) so the shared channel
remains available to other services.
Comment on lines
+20
to
+24
| const testInterpreter: PythonEnvironment = { | ||
| id: 'test-python-id', | ||
| uri: Uri.file('/usr/bin/python3'), | ||
| version: { major: 3, minor: 11, patch: 0, raw: '3.11.0' } | ||
| } as PythonEnvironment; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Type assertion may hide incomplete mock.
The as PythonEnvironment assertion bypasses type checking. Consider creating a complete mock or using a factory function to ensure all required fields are present.
🤖 Prompt for AI Agents
In src/kernels/deepnote/environments/deepnoteEnvironmentManager.unit.test.ts
around lines 20 to 24, the test uses a blanket "as PythonEnvironment" type
assertion which can hide missing required fields; replace the assertion by
creating a complete mock or a small factory function that returns a fully
populated PythonEnvironment (inspect the PythonEnvironment interface and include
all required properties with sensible defaults), then use that factory in the
test instead of the cast so the compiler enforces completeness and the mock
accurately represents the real type.
|
|
||
| const updated = manager.getEnvironment(config.id); | ||
| assert.strictEqual(updated?.name, 'Updated Name'); | ||
| verify(mockStorage.saveEnvironments(anything())).atLeast(1); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Prefer exact call count in unit tests.
Using atLeast(1) is imprecise. Consider .once() or .times(n) to verify exact behavior.
🤖 Prompt for AI Agents
In src/kernels/deepnote/environments/deepnoteEnvironmentManager.unit.test.ts
around line 174, the test currently uses
verify(mockStorage.saveEnvironments(anything())).atLeast(1) which is imprecise;
change this to an exact verification such as .once() or .times(n) that matches
the intended behavior (e.g.,
verify(mockStorage.saveEnvironments(anything())).once()), and if necessary
update the test setup or assertions to ensure the method is called exactly that
many times so the test reflects precise expected behavior.
|
|
||
| const updated = manager.getEnvironment(config.id); | ||
| assert.deepStrictEqual(updated?.packages, ['numpy', 'pandas']); | ||
| verify(mockStorage.saveEnvironments(anything())).atLeast(1); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Prefer exact call count in unit tests.
Using atLeast(1) is imprecise. Consider .once() or .times(n) to verify exact behavior.
Comment on lines
+201
to
+221
| const items: (QuickPickItem & { environment?: DeepnoteEnvironment })[] = environments.map((env) => { | ||
| return { | ||
| label: env.name, | ||
| description: getDisplayPath(env.pythonInterpreter.uri), | ||
| detail: env.packages?.length | ||
| ? l10n.t('Packages: {0}', env.packages.join(', ')) | ||
| : l10n.t('No additional packages'), | ||
| environment: env | ||
| }; | ||
| }); | ||
| // Add "Create new" option at the end | ||
| items.push({ | ||
| label: '$(add) Create New Environment', | ||
| description: 'Set up a new kernel environment', | ||
| alwaysShow: true | ||
| }); | ||
| const selected = await window.showQuickPick(items, { | ||
| placeHolder: `Select an environment for ${getDisplayPath(notebookUri)}`, | ||
| matchOnDescription: true, | ||
| matchOnDetail: true | ||
| }); |
Contributor
There was a problem hiding this comment.
Localize the quick pick strings
User-facing copy in the quick pick ('$(add) Create New Environment', 'Set up a new kernel environment', and the placeholder string) must go through l10n.t(...) per our guidelines. Please wrap these literals so the picker is localizable.
🤖 Prompt for AI Agents
In src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts around lines
201–221, the quick-pick label, description, and placeholder are hard-coded and
must be localized; change the "Create New Environment" label to use l10n.t
(preserving the "$(add)" icon prefix), wrap the description "Set up a new kernel
environment" with l10n.t, and replace the placeholder string with l10n.t using a
parameter for the notebook display path (e.g. l10n.t('Select an environment for
{0}', getDisplayPath(notebookUri))); ensure string interpolation uses l10n.t so
these UI strings are localizable.
Comment on lines
445
to
456
| const environmentId = this.notebookEnvironmentMapper.getEnvironmentForNotebook(notebook.uri); | ||
|
|
||
| const sessionManager = JupyterLabHelper.create(connectionInfo.settings); | ||
| let kernelSpec; | ||
| try { | ||
| const kernelSpecs = await sessionManager.getKernelSpecs(); | ||
| logger.info( | ||
| `Available kernel specs on Deepnote server: ${kernelSpecs.map((s) => s.name).join(', ')}` | ||
| ); | ||
| if (environmentId == null) { | ||
| return false; | ||
| } | ||
|
|
||
| // Create expected kernel name based on file path (uses installer's hash logic) | ||
| const venvHash = this.toolkitInstaller.getVenvHash(baseFileUri); | ||
| const expectedKernelName = `deepnote-venv-${venvHash}`; | ||
| logger.info(`Looking for venv kernel spec: ${expectedKernelName}`); | ||
|
|
||
| // Prefer the venv kernel spec that uses the venv's Python interpreter | ||
| // This ensures packages installed via pip are available to the kernel | ||
| kernelSpec = kernelSpecs.find((s) => s.name === expectedKernelName); | ||
|
|
||
| if (!kernelSpec) { | ||
| logger.warn( | ||
| l10n.t( | ||
| "⚠️ Venv kernel spec '{expectedKernelName}' not found! Falling back to generic Python kernel.", | ||
| { expectedKernelName } | ||
| ) | ||
| ); | ||
| logger.warn( | ||
| l10n.t( | ||
| 'This may cause import errors if packages are installed to the venv but kernel uses system Python.' | ||
| ) | ||
| ); | ||
| kernelSpec = | ||
| kernelSpecs.find((s) => s.language === 'python') || | ||
| kernelSpecs.find((s) => s.name === 'python3-venv') || | ||
| kernelSpecs[0]; | ||
| } | ||
| const environment = environmentId ? this.environmentManager.getEnvironment(environmentId) : undefined; | ||
| if (environment == null) { | ||
| logger.info(`No environment found for notebook ${getDisplayPath(notebook.uri)}`); | ||
| await this.notebookEnvironmentMapper.removeEnvironmentForNotebook(notebook.uri); | ||
| return false; | ||
| } |
Contributor
There was a problem hiding this comment.
Wait for environment initialization before lookup
We call getEnvironment() immediately. If the storage load is still in-flight, this returns undefined, we drop the mapping, and users lose their environment selection on startup. Please await this.environmentManager.waitForInitialization() before reading so we don’t race the load.
🤖 Prompt for AI Agents
In src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts around lines 445 to
456, we call getEnvironment() immediately which can race with storage loading
and return undefined; await this.environmentManager.waitForInitialization()
before calling getEnvironment() so the manager has finished loading, then call
getEnvironment(environmentId) and only call
notebookEnvironmentMapper.removeEnvironmentForNotebook(...) if, after waiting,
environment is still null; keep the existing environmentId null check and ensure
the wait is awaited (await this.environmentManager.waitForInitialization())
before the lookup/removal logic.
Comment on lines
+91
to
98
| this.logger.info('requirements.txt exists with different content; overriding with Deepnote requirements'); | ||
| } else { | ||
| this.logger.info('Creating requirements.txt from Deepnote project requirements'); | ||
| } | ||
|
|
||
| // Write the requirements.txt file | ||
| // Write the requirements.txt file (overwrites if it already exists) | ||
| await fs.promises.writeFile(requirementsPath, requirementsText, 'utf8'); | ||
|
|
Contributor
There was a problem hiding this comment.
Don't overwrite user content without consent.
Dropping the confirmation means any existing requirements.txt with local edits is silently clobbered, which is a regression into data loss; we still need an explicit user opt-in before overwriting divergent content.
🤖 Prompt for AI Agents
In src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts around lines 91 to
98, the code unconditionally writes requirements.txt and silently overwrites any
existing local edits; change this so we do not clobber user content without
explicit opt-in: detect if requirementsPath exists and its contents differ from
requirementsText, and if so either (a) abort the write and throw or return a
clear error indicating a divergent local file and instructing the caller to pass
an explicit flag to confirm overwrite, or (b) if interactive flows are
supported, prompt the user to confirm overwrite before writing; implement a new
boolean option (e.g., forceOverwrite or confirmOverwrite) on the helper and only
perform fs.promises.writeFile when that flag is true (log appropriate info when
skipping/aborting). Ensure logs clearly state when the write was skipped due to
divergence and what caller action is required.
Comment on lines
+195
to
+214
| // Deepnote configuration services | ||
| serviceManager.addSingleton<DeepnoteEnvironmentStorage>(DeepnoteEnvironmentStorage, DeepnoteEnvironmentStorage); | ||
| serviceManager.addSingleton<IDeepnoteEnvironmentManager>(IDeepnoteEnvironmentManager, DeepnoteEnvironmentManager); | ||
| serviceManager.addSingleton<DeepnoteEnvironmentTreeDataProvider>( | ||
| DeepnoteEnvironmentTreeDataProvider, | ||
| DeepnoteEnvironmentTreeDataProvider | ||
| ); | ||
|
|
||
| // Deepnote configuration view | ||
| serviceManager.addSingleton<DeepnoteEnvironmentsView>(DeepnoteEnvironmentsView, DeepnoteEnvironmentsView); | ||
| serviceManager.addSingleton<IExtensionSyncActivationService>( | ||
| IExtensionSyncActivationService, | ||
| DeepnoteEnvironmentsActivationService | ||
| ); | ||
|
|
||
| // Deepnote configuration selection | ||
| serviceManager.addSingleton<IDeepnoteNotebookEnvironmentMapper>( | ||
| IDeepnoteNotebookEnvironmentMapper, | ||
| DeepnoteNotebookEnvironmentMapper | ||
| ); |
Contributor
There was a problem hiding this comment.
Bind DeepnoteEnvironmentManager for activation
DeepnoteEnvironmentManager implements IExtensionSyncActivationService, but we never bind it to that contract. Without the binding, VS Code never calls activate(), so persisted environments are never loaded and every dependent flow sees an empty list. Please add the usual addSingleton<IExtensionSyncActivationService>(..., DeepnoteEnvironmentManager) (or an explicit binding) so initialization actually runs before other services query the manager.
🤖 Prompt for AI Agents
In src/notebooks/serviceRegistry.node.ts around lines 195 to 214,
DeepnoteEnvironmentManager implements IExtensionSyncActivationService but is not
bound to that contract, so its activate() is never invoked and persisted
environments aren't loaded; bind it by adding a singleton registration for
IExtensionSyncActivationService with DeepnoteEnvironmentManager (e.g., call
serviceManager.addSingleton<IExtensionSyncActivationService>(IExtensionSyncActivationService,
DeepnoteEnvironmentManager)) alongside the other Deepnote activation/service
registrations so initialization runs before other services query the manager.
Comment on lines
+18
to
+21
| export function getDeepnoteNotebookKeyHash(uri: Uri, length = 16): string { | ||
| const key = getDeepnoteNotebookStorageKey(uri); | ||
| return createHash('sha256').update(key).digest('hex').slice(0, length); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider collision risk with 16-character hash.
A 64-bit hash (16 hex chars) may experience collisions with ~4 billion items (birthday paradox). For environment/notebook identifiers, this is likely sufficient, but consider increasing if collision detection/handling isn't implemented.
🤖 Prompt for AI Agents
In src/platform/deepnote/deepnoteUriUtils.node.ts around lines 18 to 21, the
function currently slices the sha256 hex digest to 16 chars (64 bits) which
raises collision risk; change the default length to a larger value (e.g. 32 hex
chars for 128 bits) or make the length a named constant/config option, update
the function signature default to that larger value, and add a short comment
that callers can override the length or implement duplicate checks if needed.
Contributor
✅ Actions performedReviews paused. |
Member
Author
|
closing in favor of #152 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
My own spin on top of @tkislan and @hc2p work
Summary by CodeRabbit
New Features
Documentation
Changes
Chores