Skip to content

Conversation

@FilipPyrek
Copy link
Member

@FilipPyrek FilipPyrek commented Oct 8, 2025

Summary by CodeRabbit

  • New Features

    • Init Notebooks: runs before user code (cached per project/session), blocks until completion with progress (“Running init notebook…”, “Kernel ready!”); failures don’t stop user code.
    • Project Requirements: generates requirements.txt from project settings with user prompt/overwrite handling.
  • Improvements

    • Event-driven kernel startup and kernel-selection reliability; better environment discovery and clearer progress/messaging.
  • Documentation

    • README updated to describe Init Notebooks and Project Requirements.

@linear
Copy link

linear bot commented Oct 8, 2025

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 8, 2025

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Adds an init-notebook execution flow for Deepnote projects: a new DeepnoteInitNotebookRunner runs a configured init notebook (cached per project/session) after a kernel starts; DeepnoteRequirementsHelper generates a workspace requirements.txt from project settings before user code runs; DeepnoteKernelAutoSelector becomes event-driven (uses toolkit installer's getVenvHash for venv kernel naming), schedules pending init-notebook runs on kernel start, and awaits their completion. DeepnoteNotebookManager tracks per-project init-notebook run state. Service registry and DI bindings register the new runner and requirements helper; several public types/interfaces were extended to support these features.

Sequence Diagram(s)

sequenceDiagram
    participant AutoSelector as DeepnoteKernelAutoSelector
    participant ReqHelper as DeepnoteRequirementsHelper
    participant Provider as KernelProvider
    participant Kernel as Kernel
    participant InitRunner as DeepnoteInitNotebookRunner
    participant NBManager as DeepnoteNotebookManager
    participant UserNB as UserNotebook

    Note right of AutoSelector #E8F1FF: Activation & pre-kernel setup
    AutoSelector->>ReqHelper: createRequirementsFile(project, token)
    alt requirements written
        ReqHelper-->>AutoSelector: wrote requirements.txt
    else skipped/no requirements
        ReqHelper-->>AutoSelector: no-op
    end

    AutoSelector->>Provider: ensureOrSelectKernel(specName=getVenvHash(...))
    Provider-->>AutoSelector: kernel instance (lazy)
    Kernel->>AutoSelector: onDidStartKernel(projectId, kernel)

    AutoSelector->>NBManager: hasInitNotebookBeenRun(projectId)?
    alt not run
        AutoSelector->>InitRunner: runInitNotebookIfNeeded(projectId, UserNB, token)
        InitRunner->>Kernel: execute init notebook blocks (sequential)
        Note right of InitRunner #E8F1FF: dual progress UI (Notification + Window)
        InitRunner-->>NBManager: markInitNotebookAsRun(projectId)
        InitRunner-->>AutoSelector: completed (success/failure cached)
    else already run
        AutoSelector-->>UserNB: proceed to user execution
    end

    Note right of Kernel #E8F1FF: Kernel ready -> user cells execute
    UserNB->>Kernel: execute user cells
Loading

Possibly related PRs

Suggested reviewers

  • saltenasl
  • Artmann
  • andyjakubowski

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title “feat: implement init notebook behavior” clearly and concisely describes the primary feature added by this changeset—the init notebook execution flow—without extraneous detail or vagueness. It focuses on the main enhancement and follows conventional commit style, making it immediately understandable to reviewers scanning PR history.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fe5819a and 8a9c59b.

📒 Files selected for processing (2)
  • src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts (9 hunks)
  • src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts (1 hunks)

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b495c47 and 8e4861d.

📒 Files selected for processing (11)
  • DEEPNOTE_KERNEL_IMPLEMENTATION.md (5 hunks)
  • README.md (1 hunks)
  • src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (1 hunks)
  • src/kernels/deepnote/types.ts (1 hunks)
  • src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts (1 hunks)
  • src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts (7 hunks)
  • src/notebooks/deepnote/deepnoteNotebookManager.ts (2 hunks)
  • src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts (1 hunks)
  • src/notebooks/deepnote/deepnoteTypes.ts (1 hunks)
  • src/notebooks/serviceRegistry.node.ts (2 hunks)
  • src/notebooks/types.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/!(*.node|*.web).ts

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Place shared cross-platform logic in common .ts files (not suffixed with .node or .web)

Files:

  • src/kernels/deepnote/types.ts
  • src/notebooks/deepnote/deepnoteTypes.ts
  • src/notebooks/deepnote/deepnoteNotebookManager.ts
  • src/notebooks/types.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Inject interfaces, not concrete classes
Avoid circular dependencies
Use l10n.t() for user-facing strings
Use typed error classes from src/platform/errors/ when throwing or handling errors
Use the ILogger service instead of console.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 with CancellationToken

**/*.{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/types.ts
  • src/notebooks/deepnote/deepnoteTypes.ts
  • src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts
  • src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
  • src/notebooks/deepnote/deepnoteNotebookManager.ts
  • src/notebooks/types.ts
  • src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
  • src/notebooks/serviceRegistry.node.ts
  • src/notebooks/deepnote/deepnoteInitNotebookRunner.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/types.ts
  • src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
src/notebooks/deepnote/**

📄 CodeRabbit inference engine (CLAUDE.md)

Deepnote integration code resides under src/notebooks/deepnote/

Files:

  • src/notebooks/deepnote/deepnoteTypes.ts
  • src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts
  • src/notebooks/deepnote/deepnoteNotebookManager.ts
  • src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
  • src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts
src/notebooks/deepnote/deepnoteTypes.ts

📄 CodeRabbit inference engine (CLAUDE.md)

deepnoteTypes.ts should contain only type definitions

Files:

  • src/notebooks/deepnote/deepnoteTypes.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/deepnoteTypes.ts
  • src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts
  • src/notebooks/deepnote/deepnoteNotebookManager.ts
  • src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
  • src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts
**/*.node.ts

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use *.node.ts for Desktop-specific implementations that require full file system access and Python environments

Files:

  • src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts
  • src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
  • src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
  • src/notebooks/serviceRegistry.node.ts
  • src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts
src/notebooks/deepnote/deepnoteNotebookManager.ts

📄 CodeRabbit inference engine (CLAUDE.md)

deepnoteNotebookManager.ts handles state management

Files:

  • src/notebooks/deepnote/deepnoteNotebookManager.ts
🧠 Learnings (9)
📚 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/deepnote/deepnoteTypes.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/deepnoteNotebookManager.ts : deepnoteNotebookManager.ts handles state management

Applied to files:

  • src/notebooks/deepnote/deepnoteNotebookManager.ts
  • src/notebooks/types.ts
  • src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-03T12:59:14.489Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/notebooks.instructions.md:0-0
Timestamp: 2025-09-03T12:59:14.489Z
Learning: Applies to src/notebooks/controllers/kernelSource/**/*.ts : For new kernel sources, implement ILocalNotebookKernelSourceSelector (or similar)

Applied to files:

  • src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-03T12:58:18.091Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel.instructions.md:0-0
Timestamp: 2025-09-03T12:58:18.091Z
Learning: Applies to src/kernels/kernelProvider.base.ts : KernelProviderBase must create/manage IKernel instances, track by NotebookDocument, manage lifecycle (start/restart/dispose), and expose INotebookKernelExecution

Applied to files:

  • src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
  • src/notebooks/deepnote/deepnoteInitNotebookRunner.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.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/jupyterKernelSpec.ts : Manage kernel spec registration and updates; create temporary kernel specs for non-standard kernels when needed

Applied to files:

  • src/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: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/serviceRegistry.node.ts
🧬 Code graph analysis (3)
src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts (1)
src/notebooks/deepnote/deepnoteTypes.ts (1)
  • DeepnoteProject (4-19)
src/notebooks/serviceRegistry.node.ts (1)
src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts (2)
  • IDeepnoteInitNotebookRunner (227-227)
  • IDeepnoteInitNotebookRunner (228-230)
src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts (3)
src/notebooks/deepnote/deepnoteNotebookManager.ts (1)
  • injectable (10-94)
src/notebooks/types.ts (2)
  • IDeepnoteNotebookManager (27-27)
  • IDeepnoteNotebookManager (28-37)
src/notebooks/deepnote/deepnoteTypes.ts (2)
  • DeepnoteProject (4-19)
  • DeepnoteNotebook (24-31)
🪛 GitHub Actions: CI
src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts

[error] 6-6: eslint: Do not import path builtin module. Use the custom vscode-path instead (local-rules/node-imports).

🪛 GitHub Check: Lint & Format
src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts

[failure] 6-6:
Do not import path builtin module. Use the custom vscode-path instead

🪛 markdownlint-cli2 (0.18.1)
DEEPNOTE_KERNEL_IMPLEMENTATION.md

216-216: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/1/1

(MD029, ol-prefix)


224-224: Ordered list item prefix
Expected: 1; Actual: 4; Style: 1/1/1

(MD029, ol-prefix)


371-371: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


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

(MD031, blanks-around-fences)


377-377: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


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

(MD031, blanks-around-fences)


384-384: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


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

(MD031, blanks-around-fences)


390-390: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


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

(MD031, blanks-around-fences)


397-397: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


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

(MD031, blanks-around-fences)

🔇 Additional comments (6)
src/notebooks/deepnote/deepnoteNotebookManager.ts (1)

15-15: LGTM!

Clean state tracking using Set. Implementation aligns with IDeepnoteNotebookManager interface and follows existing patterns in the class.

Also applies to: 78-93

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

14-14: LGTM!

Optional field addition is appropriate and follows type-only guideline for this file.

src/notebooks/types.ts (1)

35-36: LGTM!

Interface extension is clear and matches the implementation in deepnoteNotebookManager.ts.

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

265-265: LGTM!

Making getVenvHash public enables cross-component reuse. Method is pure and safe to expose.

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

89-94: LGTM!

Interface extension matches the implementation and includes clear documentation.

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

56-56: Verify activation service binding.

The new service is registered as a singleton, following existing patterns. However, verify whether DeepnoteInitNotebookRunner requires activation. If it implements IExtensionSyncActivationService, add the binding:

 serviceManager.addSingleton<IDeepnoteInitNotebookRunner>(IDeepnoteInitNotebookRunner, DeepnoteInitNotebookRunner);
+serviceManager.addBinding(IDeepnoteInitNotebookRunner, IExtensionSyncActivationService);

Otherwise, the current registration is correct.

Also applies to: 139-139

@FilipPyrek FilipPyrek force-pushed the fpyrek/grn-4761-load-and-install-requirementstxt-from-the-project-file branch from 8e4861d to 6b3b152 Compare October 8, 2025 10:59
@FilipPyrek FilipPyrek requested a review from Artmann October 8, 2025 11:00
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 9

Caution

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

⚠️ Outside diff range comments (1)
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (1)

265-281: Add getVenvHash to IDeepnoteToolkitInstaller
Declare getVenvHash(deepnoteFileUri: Uri): string; in src/kernels/deepnote/types.ts under IDeepnoteToolkitInstaller.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8e4861d and 6b3b152.

📒 Files selected for processing (12)
  • DEEPNOTE_KERNEL_IMPLEMENTATION.md (5 hunks)
  • README.md (1 hunks)
  • src/extension.common.ts (2 hunks)
  • src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (1 hunks)
  • src/kernels/deepnote/types.ts (1 hunks)
  • src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts (1 hunks)
  • src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts (7 hunks)
  • src/notebooks/deepnote/deepnoteNotebookManager.ts (2 hunks)
  • src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts (1 hunks)
  • src/notebooks/serviceRegistry.node.ts (2 hunks)
  • src/notebooks/types.ts (1 hunks)
  • src/platform/logging/types.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.node.ts

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use *.node.ts for Desktop-specific implementations that require full file system access and Python environments

Files:

  • src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
  • src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
  • src/notebooks/serviceRegistry.node.ts
  • src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts
  • src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Inject interfaces, not concrete classes
Avoid circular dependencies
Use l10n.t() for user-facing strings
Use typed error classes from src/platform/errors/ when throwing or handling errors
Use the ILogger service instead of console.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 with CancellationToken

**/*.{ts,tsx}: Order class members (methods, fields, properties) first by accessibility, then alphabetically
Separate third-party imports from local file imports

Files:

  • src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
  • src/platform/logging/types.ts
  • src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
  • src/kernels/deepnote/types.ts
  • src/notebooks/types.ts
  • src/notebooks/serviceRegistry.node.ts
  • src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts
  • src/notebooks/deepnote/deepnoteNotebookManager.ts
  • src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts
  • src/extension.common.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/deepnoteRequirementsHelper.node.ts
  • src/notebooks/deepnote/deepnoteNotebookManager.ts
  • src/notebooks/deepnote/deepnoteInitNotebookRunner.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/notebooks/deepnote/deepnoteRequirementsHelper.node.ts
  • src/notebooks/deepnote/deepnoteNotebookManager.ts
  • src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts
**/!(*.node|*.web).ts

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Place shared cross-platform logic in common .ts files (not suffixed with .node or .web)

Files:

  • src/platform/logging/types.ts
  • src/kernels/deepnote/types.ts
  • src/notebooks/types.ts
  • src/notebooks/deepnote/deepnoteNotebookManager.ts
  • src/extension.common.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/logging/types.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/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.ts
  • src/kernels/deepnote/types.ts
src/notebooks/deepnote/deepnoteNotebookManager.ts

📄 CodeRabbit inference engine (CLAUDE.md)

deepnoteNotebookManager.ts handles state management

Files:

  • src/notebooks/deepnote/deepnoteNotebookManager.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/kernelProvider.base.ts : KernelProviderBase must create/manage IKernel instances, track by NotebookDocument, manage lifecycle (start/restart/dispose), and expose INotebookKernelExecution

Applied to files:

  • src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
  • src/notebooks/deepnote/deepnoteInitNotebookRunner.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/deepnoteNotebookManager.ts : deepnoteNotebookManager.ts handles state management

Applied to files:

  • src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
  • src/notebooks/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/deepnoteActivationService.ts : deepnoteActivationService.ts wires VSCode activation

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/jupyterKernelSpec.ts : Manage kernel spec registration and updates; create temporary kernel specs for non-standard kernels when needed

Applied to files:

  • src/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-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-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: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/serviceRegistry.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/deepnoteRequirementsHelper.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/deepnoteRequirementsHelper.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/deepnoteRequirementsHelper.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/deepnoteRequirementsHelper.node.ts
📚 Learning: 2025-09-12T08:03:14.066Z
Learnt from: Artmann
PR: deepnote/vscode-deepnote#12
File: src/notebooks/deepnote/deepnoteExplorerView.ts:69-71
Timestamp: 2025-09-12T08:03:14.066Z
Learning: In the DeepnoteExplorerView class, ILogger needs to be properly injected via constructor dependency injection with inject(ILogger) decorator and added as a private field to replace console.log usage.

Applied to files:

  • src/notebooks/deepnote/deepnoteRequirementsHelper.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 the `ILogger` service instead of `console.log`

Applied to files:

  • src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts
  • src/extension.common.ts
🧬 Code graph analysis (5)
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts (5)
src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts (2)
  • IDeepnoteInitNotebookRunner (225-225)
  • IDeepnoteInitNotebookRunner (226-228)
src/notebooks/types.ts (2)
  • IDeepnoteNotebookManager (27-27)
  • IDeepnoteNotebookManager (28-37)
src/kernels/types.ts (3)
  • IKernelProvider (548-548)
  • IKernelProvider (549-564)
  • IKernel (439-446)
src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts (2)
  • IDeepnoteRequirementsHelper (72-72)
  • IDeepnoteRequirementsHelper (73-75)
src/kernels/deepnote/types.ts (1)
  • DEEPNOTE_NOTEBOOK_TYPE (160-160)
src/notebooks/serviceRegistry.node.ts (2)
src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts (2)
  • IDeepnoteInitNotebookRunner (225-225)
  • IDeepnoteInitNotebookRunner (226-228)
src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts (2)
  • IDeepnoteRequirementsHelper (72-72)
  • IDeepnoteRequirementsHelper (73-75)
src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts (2)
src/platform/logging/types.ts (2)
  • ILogger (9-9)
  • ILogger (10-19)
src/test/mocks/vsc/index.ts (1)
  • CancellationToken (100-112)
src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts (3)
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts (1)
  • injectable (46-519)
src/notebooks/deepnote/deepnoteNotebookManager.ts (1)
  • injectable (10-94)
src/notebooks/types.ts (2)
  • IDeepnoteNotebookManager (27-27)
  • IDeepnoteNotebookManager (28-37)
src/extension.common.ts (2)
src/platform/logging/types.ts (2)
  • ILogger (9-9)
  • ILogger (10-19)
src/platform/logging/index.ts (1)
  • logger (35-48)
🪛 markdownlint-cli2 (0.18.1)
DEEPNOTE_KERNEL_IMPLEMENTATION.md

216-216: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/1/1

(MD029, ol-prefix)


224-224: Ordered list item prefix
Expected: 1; Actual: 4; Style: 1/1/1

(MD029, ol-prefix)


371-371: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


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

(MD031, blanks-around-fences)


377-377: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


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

(MD031, blanks-around-fences)


384-384: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


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

(MD031, blanks-around-fences)


390-390: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


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

(MD031, blanks-around-fences)


397-397: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


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

(MD031, blanks-around-fences)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Lint & Format
  • GitHub Check: copilot-setup-steps
🔇 Additional comments (7)
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts (2)

96-99: LGTM: Event-driven init notebook execution.

Using onDidStartKernel is correct since kernels are created lazily when cells execute.


37-41: New dependencies are properly registered in serviceRegistry.node.ts.

README.md (1)

12-13: LGTM: Clear feature documentation.

Accurately describes the new init notebook and requirements.txt capabilities.

src/platform/logging/types.ts (1)

9-9: LGTM: DI symbol for logger.

Enables dependency injection of ILogger across services.

src/extension.common.ts (1)

40-40: LGTM: Logger service registration.

Registers the logger instance as ILogger for dependency injection.

Also applies to: 131-131

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

89-94: LGTM: Interface updated for public API.

Correctly adds getVenvHash to the interface with clear documentation.

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

56-57: Singleton scope is appropriate
Both DeepnoteInitNotebookRunner and DeepnoteRequirementsHelper hold no internal state beyond injected dependencies. No changes needed.

logger.info(`Kernel found for ${getDisplayPath(notebook.uri)}, starting init notebook execution`);

// Filter out non-code blocks
const codeBlocks = initNotebook.blocks.filter((block) => block.type === 'code');
Copy link
Member

Choose a reason for hiding this comment

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

We'll have other types of executable blocks here soon that we will want to support. Can you expand it to include SQL, input blocks, and chart types?

Copy link
Member Author

Choose a reason for hiding this comment

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

Since SQL blocks are not yet implemented in the extension, we agreed that it can be added to the init notebooks once implemented.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 9

♻️ Duplicate comments (9)
DEEPNOTE_KERNEL_IMPLEMENTATION.md (3)

216-223: Ordered-list style fixed — thanks

This matches the 1/1/1 style the linter expects.


224-228: Same here — numbering compliant

Good with MD029 1/1/1.


371-398: Silence markdownlint without changing the bold-label pattern

Maintainer chose to keep bold labels. Wrap this section with rule disables to appease MD036/MD031.

+<!-- markdownlint-disable MD036 MD031 -->
 **1. Package Installation**
 ```python
 %%bash
 pip install -r requirements.txt

2. Environment Setup

import os
os.environ['DATA_PATH'] = '/workspace/data'
os.environ['MODEL_PATH'] = '/workspace/models'

3. Database Connections

import sqlalchemy
engine = sqlalchemy.create_engine('postgresql://...')

4. Data Preprocessing

import pandas as pd
# Preload common datasets
df = pd.read_csv('data/master_dataset.csv')

5. Custom Imports

import sys
sys.path.append('./custom_modules')
from my_utils import helper_functions

+

Alternatively, insert a blank line between each bold line and its code fence to satisfy MD031 only.

</blockquote></details>
<details>
<summary>src/notebooks/types.ts (1)</summary><blockquote>

`35-36`: **Interface extension LGTM**

Methods read well and fit the manager’s role. Persistence across restarts can be tackled later (not a blocker here).

</blockquote></details>
<details>
<summary>src/notebooks/deepnote/deepnoteNotebookManager.ts (1)</summary><blockquote>

`15-15`: **In‑memory tracking is fine for now**

Adds clear APIs and a simple Set; note this won’t survive process restarts if persistence is desired later.




Also applies to: 83-93

</blockquote></details>
<details>
<summary>src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts (1)</summary><blockquote>

`203-208`: **Future: support more block types**

Currently executes only type === 'code'. If SQL/input/chart blocks are planned, extend filter/dispatch accordingly.

</blockquote></details>
<details>
<summary>src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts (1)</summary><blockquote>

`27-31`: **Add element type guard**

Validate requirements elements are strings before joining/logging.


```diff
-            const requirements = project.project.settings?.requirements;
-            if (!requirements || !Array.isArray(requirements) || requirements.length === 0) {
+            const requirements = project.project.settings?.requirements;
+            if (!requirements || !Array.isArray(requirements) || requirements.length === 0) {
                 this.logger.info(`No requirements found in project ${project.project.id}`);
                 return;
             }
+            if (!requirements.every((r: unknown): r is string => typeof r === 'string')) {
+                this.logger.warn(`Invalid requirements format in project ${project.project.id}`);
+                return;
+            }

Also applies to: 48-51

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

58-63: Keying pending init by projectId may leave stale entries

Multiple notebooks for the same project can conflict; closed notebooks may leave entries. Consider keying by notebookKey (fsPath) instead.


189-194: Cleanup keyed by projectId shares the same risk

Deleting by projectId can remove entries for other open notebooks of the same project. Prefer deleting by notebookKey.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6b3b152 and fe5819a.

📒 Files selected for processing (6)
  • DEEPNOTE_KERNEL_IMPLEMENTATION.md (5 hunks)
  • src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts (1 hunks)
  • src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts (9 hunks)
  • src/notebooks/deepnote/deepnoteNotebookManager.ts (2 hunks)
  • src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts (1 hunks)
  • src/notebooks/types.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/!(*.node|*.web).ts

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Place shared cross-platform logic in common .ts files (not suffixed with .node or .web)

Files:

  • src/notebooks/types.ts
  • src/notebooks/deepnote/deepnoteNotebookManager.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Inject interfaces, not concrete classes
Avoid circular dependencies
Use l10n.t() for user-facing strings
Use typed error classes from src/platform/errors/ when throwing or handling errors
Use the ILogger service instead of console.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 with CancellationToken

**/*.{ts,tsx}: Order class members (methods, fields, properties) first by accessibility, then alphabetically
Separate third-party imports from local file imports

Files:

  • src/notebooks/types.ts
  • src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts
  • src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts
  • src/notebooks/deepnote/deepnoteNotebookManager.ts
  • src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
**/*.node.ts

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use *.node.ts for Desktop-specific implementations that require full file system access and Python environments

Files:

  • src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts
  • src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts
  • src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
src/notebooks/deepnote/**

📄 CodeRabbit inference engine (CLAUDE.md)

Deepnote integration code resides under src/notebooks/deepnote/

Files:

  • src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts
  • src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts
  • src/notebooks/deepnote/deepnoteNotebookManager.ts
  • 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/deepnoteRequirementsHelper.node.ts
  • src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts
  • src/notebooks/deepnote/deepnoteNotebookManager.ts
  • src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
src/notebooks/deepnote/deepnoteNotebookManager.ts

📄 CodeRabbit inference engine (CLAUDE.md)

deepnoteNotebookManager.ts handles state management

Files:

  • src/notebooks/deepnote/deepnoteNotebookManager.ts
🧠 Learnings (18)
📚 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/deepnoteNotebookManager.ts : deepnoteNotebookManager.ts handles state management

Applied to files:

  • src/notebooks/types.ts
  • src/notebooks/deepnote/deepnoteKernelAutoSelector.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/deepnoteRequirementsHelper.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/deepnoteRequirementsHelper.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/deepnoteRequirementsHelper.node.ts
  • src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-03T12:58:18.091Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel.instructions.md:0-0
Timestamp: 2025-09-03T12:58:18.091Z
Learning: Applies to src/kernels/**/*.ts : Respect CancellationToken in all async operations

Applied to files:

  • src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts
  • src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-12T08:03:14.066Z
Learnt from: Artmann
PR: deepnote/vscode-deepnote#12
File: src/notebooks/deepnote/deepnoteExplorerView.ts:69-71
Timestamp: 2025-09-12T08:03:14.066Z
Learning: In the DeepnoteExplorerView class, ILogger needs to be properly injected via constructor dependency injection with inject(ILogger) decorator and added as a private field to replace console.log usage.

Applied to files:

  • src/notebooks/deepnote/deepnoteRequirementsHelper.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 the `ILogger` service instead of `console.log`

Applied to files:

  • src/notebooks/deepnote/deepnoteRequirementsHelper.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: Do not add the Microsoft copyright header to new files

Applied to files:

  • src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts
📚 Learning: 2025-09-09T11:31:30.479Z
Learnt from: Artmann
PR: deepnote/vscode-extension#11
File: src/notebooks/deepnote/deepnoteActivationService.ts:1-3
Timestamp: 2025-09-09T11:31:30.479Z
Learning: New files in the Deepnote VS Code extension project should not include copyright headers.

Applied to files:

  • src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts
📚 Learning: 2025-10-07T15:24:14.050Z
Learnt from: jankuca
PR: deepnote/vscode-deepnote#24
File: src/notebooks/deepnote/integrations/integrationTypes.ts:1-7
Timestamp: 2025-10-07T15:24:14.050Z
Learning: In the deepnote/vscode-deepnote repository, Microsoft copyright headers are not required for new files and should not be suggested.

Applied to files:

  • src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts
📚 Learning: 2025-09-03T12:59:14.489Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/notebooks.instructions.md:0-0
Timestamp: 2025-09-03T12:59:14.489Z
Learning: Applies to src/notebooks/controllers/kernelSource/**/*.ts : For new kernel sources, implement ILocalNotebookKernelSourceSelector (or similar)

Applied to files:

  • src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-03T12:58:18.091Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel.instructions.md:0-0
Timestamp: 2025-09-03T12:58:18.091Z
Learning: Applies to src/kernels/**/*.ts : Use WeakRef/weak references for notebook/object backreferences to avoid memory leaks

Applied to files:

  • src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-03T12:58:18.091Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel.instructions.md:0-0
Timestamp: 2025-09-03T12:58:18.091Z
Learning: Applies to src/kernels/kernelProvider.base.ts : KernelProviderBase must create/manage IKernel instances, track by NotebookDocument, manage lifecycle (start/restart/dispose), and expose INotebookKernelExecution

Applied to files:

  • src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-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.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/message/ipyWidgetMessageDispatcher.ts : On kernel restarts, clear pending state, resolve waiting promises, and re-establish hooks and comm targets

Applied to files:

  • src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-03T12:59:14.489Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/notebooks.instructions.md:0-0
Timestamp: 2025-09-03T12:59:14.489Z
Learning: Applies to src/notebooks/controllers/kernelSource/**/*.ts : Register the new kernel source in KernelSourceCommandHandler

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/jupyterKernelSpec.ts : Manage kernel spec registration and updates; create temporary kernel specs for non-standard kernels when needed

Applied to files:

  • src/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/notebookIPyWidgetCoordinator.ts : NotebookIPyWidgetCoordinator should manage per-notebook lifecycle, controller selection, webview attachment, and creation of CommonMessageCoordinator instances

Applied to files:

  • src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
🧬 Code graph analysis (2)
src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts (2)
src/platform/logging/types.ts (2)
  • ILogger (9-9)
  • ILogger (10-19)
src/test/mocks/vsc/index.ts (1)
  • CancellationToken (100-112)
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts (5)
src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts (2)
  • IDeepnoteInitNotebookRunner (283-283)
  • IDeepnoteInitNotebookRunner (284-286)
src/notebooks/types.ts (2)
  • IDeepnoteNotebookManager (27-27)
  • IDeepnoteNotebookManager (28-37)
src/kernels/types.ts (3)
  • IKernelProvider (548-548)
  • IKernelProvider (549-564)
  • IKernel (439-446)
src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts (2)
  • IDeepnoteRequirementsHelper (69-69)
  • IDeepnoteRequirementsHelper (70-72)
src/kernels/deepnote/types.ts (1)
  • DEEPNOTE_NOTEBOOK_TYPE (160-160)
🪛 markdownlint-cli2 (0.18.1)
DEEPNOTE_KERNEL_IMPLEMENTATION.md

371-371: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


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

(MD031, blanks-around-fences)


377-377: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


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

(MD031, blanks-around-fences)


384-384: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


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

(MD031, blanks-around-fences)


390-390: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


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

(MD031, blanks-around-fences)


397-397: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


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

(MD031, blanks-around-fences)

🔇 Additional comments (5)
DEEPNOTE_KERNEL_IMPLEMENTATION.md (1)

123-130: Event-driven wiring looks right

onDidStartKernel for init execution + shared hash via installer is consistent.

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

27-104: Good control‑flow, cancellation, and state marking

Solid checks (cancellation, missing project/notebook), and marking as run on success/miss/error prevents loops.

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

98-101: Hooking kernel start is correct

onDidStartKernel is the right place to trigger init‑notebook execution.


401-403: Correct: derive venv kernel name via getVenvHash

Using the installer’s hash ensures consistent kernel spec naming across sessions.


461-489: Race avoided by creating requirements and staging init before affinity

Good: requirements.txt is created and pending init stored before setting Preferred affinity.

Also applies to: 505-506

Comment on lines +78 to +87
#### 5. **Deepnote Init Notebook Runner** (`src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts`)

- Runs initialization notebooks automatically before user code executes
- Checks for `project.initNotebookId` in the Deepnote project YAML
- Executes init notebook code blocks sequentially in the kernel
- Shows progress notification to user
- Caches execution per project (runs only once per session)
- Logs errors but allows user to continue on failure
- Blocks user cell execution until init notebook completes

Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Clarify blocking semantics and user escape hatch

Docs say init “blocks user cell execution.” Recommend stating timeout/opt‑out behavior (e.g., cancel/cancel after N seconds) to avoid hard locks on long/failed inits.

Example edit:

- - Blocks user cell execution until init notebook completes
+ - Blocks user cell execution until init notebook completes
+ - If the run exceeds a reasonable timeout (e.g., 2–5 minutes) or the user cancels the progress,
+   the init is aborted and user code proceeds (with a warning)
📝 Committable suggestion

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

Suggested change
#### 5. **Deepnote Init Notebook Runner** (`src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts`)
- Runs initialization notebooks automatically before user code executes
- Checks for `project.initNotebookId` in the Deepnote project YAML
- Executes init notebook code blocks sequentially in the kernel
- Shows progress notification to user
- Caches execution per project (runs only once per session)
- Logs errors but allows user to continue on failure
- Blocks user cell execution until init notebook completes
#### 5. **Deepnote Init Notebook Runner** (`src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts`)
- Runs initialization notebooks automatically before user code executes
- Checks for `project.initNotebookId` in the Deepnote project YAML
- Executes init notebook code blocks sequentially in the kernel
- Shows progress notification to user
- Caches execution per project (runs only once per session)
- Logs errors but allows user to continue on failure
- Blocks user cell execution until init notebook completes
- If the run exceeds a reasonable timeout (e.g., 2–5 minutes) or the user cancels the progress,
the init is aborted and user code proceeds (with a warning)
🤖 Prompt for AI Agents
In DEEPNOTE_KERNEL_IMPLEMENTATION.md around lines 78 to 87, clarify the current
statement that the Deepnote Init Notebook Runner "blocks user cell execution" by
adding explicit blocking semantics and an escape hatch: describe whether the
block is bounded (specify default timeout duration if any), how users can opt
out (e.g., cancel button, CLI/metadata flag to skip init), and what happens on
timeout or manual cancellation (log, surface notification, and allow user cells
to proceed). Update the docs to state the default behavior, configuration
options to disable/skip init per-session or per-project, and error handling
semantics so readers know there is no permanent hard lock.

Comment on lines +108 to 112
- Queries the Deepnote server for available kernel specs using **matching hash function**
- **Selects a server-native Python kernel spec** (e.g., `deepnote-venv-{hash}` or fallback to any available Python kernel)
- The Deepnote server is started with the venv's Python interpreter, ensuring the kernel uses the venv environment
- Environment variables (`PATH`, `VIRTUAL_ENV`) are configured so the server and kernel use the venv's Python
- Environment variables (`PATH`, `VIRTUAL_ENV`, `JUPYTER_PATH`) are configured so the server and kernel use the venv's Python
- Registers the server with the server provider
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Cross‑platform note for PATH/JUPYTER_PATH

Add a platform note: venv bin vs Scripts on Windows and how JUPYTER_PATH is set/merged (don’t clobber existing).

- - Environment variables (`PATH`, `VIRTUAL_ENV`, `JUPYTER_PATH`) are configured so the server and kernel use the venv's Python
+ - Environment variables (`PATH`, `VIRTUAL_ENV`, `JUPYTER_PATH`) are configured so the server and kernel use the venv's Python
+   (on Windows, prepend `<venv>\\Scripts`; on POSIX, `<venv>/bin`). Existing `JUPYTER_PATH` is preserved and extended.
📝 Committable suggestion

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

Suggested change
- Queries the Deepnote server for available kernel specs using **matching hash function**
- **Selects a server-native Python kernel spec** (e.g., `deepnote-venv-{hash}` or fallback to any available Python kernel)
- The Deepnote server is started with the venv's Python interpreter, ensuring the kernel uses the venv environment
- Environment variables (`PATH`, `VIRTUAL_ENV`) are configured so the server and kernel use the venv's Python
- Environment variables (`PATH`, `VIRTUAL_ENV`, `JUPYTER_PATH`) are configured so the server and kernel use the venv's Python
- Registers the server with the server provider
- Queries the Deepnote server for available kernel specs using **matching hash function**
- **Selects a server-native Python kernel spec** (e.g., `deepnote-venv-{hash}` or fallback to any available Python kernel)
- The Deepnote server is started with the venv's Python interpreter, ensuring the kernel uses the venv environment
- Environment variables (`PATH`, `VIRTUAL_ENV`, `JUPYTER_PATH`) are configured so the server and kernel use the venv's Python
(on Windows, prepend `<venv>\\Scripts`; on POSIX, `<venv>/bin`). Existing `JUPYTER_PATH` is preserved and extended.
- Registers the server with the server provider
🤖 Prompt for AI Agents
In DEEPNOTE_KERNEL_IMPLEMENTATION.md around lines 108 to 112, add a short
cross‑platform note explaining that virtualenv executables live in "bin/" on
POSIX and "Scripts\" on Windows and that PATH and VIRTUAL_ENV must point to the
correct platform-specific executable directory when launching the server;
instruct to modify how JUPYTER_PATH is set so it is merged (prepend the venv's
jupyter paths) rather than clobbered, using the platform path separator (':' on
POSIX, ';' on Windows) and preserving any existing entries; keep the note brief
and include a recommended merge order (venv paths first, then existing
JUPYTER_PATH) and a reminder to normalize path separators on Windows.

Comment on lines +248 to +266
## Init Notebook Feature

### Overview

Deepnote projects can define an **initialization notebook** that runs automatically before any user code executes. This feature ensures that the environment is properly configured with required packages and setup code before the main notebook runs.

### How It Works

When you open a Deepnote notebook, the extension:

1. **Checks for `initNotebookId`** in the project YAML (`project.initNotebookId`)
2. **Creates `requirements.txt`** from `project.settings.requirements` array
3. **Finds the init notebook** in the project's notebooks array by ID
4. **Executes all code blocks** from the init notebook sequentially
5. **Shows progress** with a notification: "Running init notebook..."
6. **Caches the execution** so it only runs once per project per session
7. **Allows user code to run** after init notebook completes

### Example Project Structure
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Security note: executing init notebooks

Add a brief warning that init notebooks execute arbitrary project code with the user’s permissions; advise trusting the source before enabling and show how to disable per workspace.

 ### Overview
 
 Deepnote projects can define an **initialization notebook** that runs automatically before any user code executes.
+⚠️ Security: Init notebooks execute arbitrary code from the project with your user permissions.
+Only enable for trusted projects. Provide a setting to disable this per workspace.

Want a PR snippet to wire a setting like deepnote.initNotebook.enabled?

📝 Committable suggestion

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

Suggested change
## Init Notebook Feature
### Overview
Deepnote projects can define an **initialization notebook** that runs automatically before any user code executes. This feature ensures that the environment is properly configured with required packages and setup code before the main notebook runs.
### How It Works
When you open a Deepnote notebook, the extension:
1. **Checks for `initNotebookId`** in the project YAML (`project.initNotebookId`)
2. **Creates `requirements.txt`** from `project.settings.requirements` array
3. **Finds the init notebook** in the project's notebooks array by ID
4. **Executes all code blocks** from the init notebook sequentially
5. **Shows progress** with a notification: "Running init notebook..."
6. **Caches the execution** so it only runs once per project per session
7. **Allows user code to run** after init notebook completes
### Example Project Structure
## Init Notebook Feature
### Overview
Deepnote projects can define an **initialization notebook** that runs automatically before any user code executes. This feature ensures that the environment is properly configured with required packages and setup code before the main notebook runs.
⚠️ Security: Init notebooks execute arbitrary code from the project with your user permissions.
Only enable for trusted projects. Provide a setting to disable this per workspace.
### How It Works
When you open a Deepnote notebook, the extension:
1. **Checks for `initNotebookId`** in the project YAML (`project.initNotebookId`)
2. **Creates `requirements.txt`** from `project.settings.requirements` array
3. **Finds the init notebook** in the project's notebooks array by ID
4. **Executes all code blocks** from the init notebook sequentially
5. **Shows progress** with a notification: "Running init notebook..."
6. **Caches the execution** so it only runs once per project per session
7. **Allows user code to run** after init notebook completes
### Example Project Structure
🤖 Prompt for AI Agents
In DEEPNOTE_KERNEL_IMPLEMENTATION.md around lines 248 to 266, add a short
security warning immediately after the "How It Works" section stating that init
notebooks execute arbitrary project code with the user's permissions, advising
users to only enable them for trusted projects and explaining how to disable per
workspace; include an example configuration key (deepnote.initNotebook.enabled)
with its default (true) and show a one-line example of disabling it in workspace
or project settings (e.g., set deepnote.initNotebook.enabled: false in workspace
or project YAML), and mention that administrators can override or document
policy via workspace settings.

Comment on lines +332 to +344
**Caching:**
- Init notebook runs **once per project** per VS Code session
- If you open multiple notebooks from the same project, init runs only once
- Cache is cleared when VS Code restarts
- Only marks as "run" if execution actually succeeds

**Error Handling:**
- If init notebook fails, the error is logged but user can still run cells
- Progress continues even if some blocks fail
- User receives notification about any errors
- Returns `false` if kernel unavailable (won't mark as run, can retry)
- Returns `true` if execution completes (marks as run)

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Inconsistency: kernel “guaranteed” vs “kernel unavailable”

Earlier you state kernel is guaranteed on onDidStartKernel. Here you mention “Returns false if kernel unavailable.” Align these statements (drop the latter or scope it to rare disposal races).

- - Returns `false` if kernel unavailable (won't mark as run, can retry)
+ - In rare disposal races, if the kernel becomes unavailable mid-run, the run is aborted and can be retried
📝 Committable suggestion

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

Suggested change
**Caching:**
- Init notebook runs **once per project** per VS Code session
- If you open multiple notebooks from the same project, init runs only once
- Cache is cleared when VS Code restarts
- Only marks as "run" if execution actually succeeds
**Error Handling:**
- If init notebook fails, the error is logged but user can still run cells
- Progress continues even if some blocks fail
- User receives notification about any errors
- Returns `false` if kernel unavailable (won't mark as run, can retry)
- Returns `true` if execution completes (marks as run)
**Caching:**
- Init notebook runs **once per project** per VS Code session
- If you open multiple notebooks from the same project, init runs only once
- Cache is cleared when VS Code restarts
- Only marks as "run" if execution actually succeeds
**Error Handling:**
- If init notebook fails, the error is logged but user can still run cells
- Progress continues even if some blocks fail
- User receives notification about any errors
- In rare disposal races, if the kernel becomes unavailable mid-run, the run is aborted and can be retried
- Returns `true` if execution completes (marks as run)
🤖 Prompt for AI Agents
In DEEPNOTE_KERNEL_IMPLEMENTATION.md around lines 332 to 344, there is an
inconsistency between the earlier claim that the kernel is guaranteed on
onDidStartKernel and the current statement "Returns false if kernel
unavailable"; update the doc to align these by either removing the "kernel
unavailable" clause or explicitly scoping it to rare race/disposal cases: state
that onDidStartKernel guarantees a running kernel, then add a note that a very
rare race (kernel disposed immediately after start) may cause init to fail and
return false, or simply delete the ambiguous "Returns false if kernel
unavailable" line so all statements consistently reflect the guarantee.

@@ -0,0 +1,286 @@
import { inject, injectable } from 'inversify';
import { NotebookDocument, ProgressLocation, window, CancellationTokenSource, CancellationToken } from 'vscode';
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Localize user‑facing progress strings

Wrap titles with l10n.t() per guidelines.

As per coding guidelines

-import { NotebookDocument, ProgressLocation, window, CancellationTokenSource, CancellationToken } from 'vscode';
+import { NotebookDocument, ProgressLocation, window, CancellationTokenSource, CancellationToken, l10n } from 'vscode';
@@
-                    title: `🚀 Initializing project environment`,
+                    title: l10n.t('🚀 Initializing project environment'),
@@
-                            title: `Init: "${initNotebook.name}"`,
+                            title: l10n.t('Init: "{0}"', initNotebook.name),

Also applies to: 136-147

🤖 Prompt for AI Agents
In src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts around line 2 (and
also apply the same change to lines 136-147), user-facing progress titles are
not localized; update all ProgressLocation/window progress title strings to use
l10n.t(...)—import l10n if not present—and replace raw literal titles with
l10n.t('your.key', { default: 'Original Title' }) or appropriate keys, ensuring
each progress message uses a unique key and preserves existing
interpolation/formatting.

Comment on lines +196 to 250
private async onKernelStarted(kernel: IKernel) {
// Only handle deepnote notebooks
if (kernel.notebook?.notebookType !== DEEPNOTE_NOTEBOOK_TYPE) {
return;
}

const notebook = kernel.notebook;
const projectId = notebook.metadata?.deepnoteProjectId;

if (!projectId) {
return;
}

// Check if we have a pending init notebook for this project
const pendingInit = this.projectsPendingInitNotebook.get(projectId);
if (!pendingInit) {
return; // No init notebook to run
}

logger.info(`Kernel started for Deepnote notebook, running init notebook for project ${projectId}`);

// Remove from pending list
this.projectsPendingInitNotebook.delete(projectId);

// Create a CancellationTokenSource tied to the notebook lifecycle
const cts = new CancellationTokenSource();
const disposables: Disposable[] = [];

try {
// Register handler to cancel the token if the notebook is closed
// Note: We check the URI to ensure we only cancel for the specific notebook that closed
const closeListener = workspace.onDidCloseNotebookDocument((closedNotebook) => {
if (closedNotebook.uri.toString() === notebook.uri.toString()) {
logger.info(`Notebook closed while init notebook was running, cancelling for project ${projectId}`);
cts.cancel();
}
});
disposables.push(closeListener);

// Run init notebook with cancellation support
await this.initNotebookRunner.runInitNotebookIfNeeded(projectId, notebook, cts.token);
} catch (error) {
// Check if this is a cancellation error - if so, just log and continue
if (error instanceof Error && error.message === 'Cancelled') {
logger.info(`Init notebook cancelled for project ${projectId}`);
return;
}
logger.error(`Error running init notebook: ${error}`);
// Continue anyway - don't block user if init fails
} finally {
// Always clean up the CTS and event listeners
cts.dispose();
disposables.forEach((d) => d.dispose());
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Also cancel on kernel dispose

Tie the CTS to kernel.onDisposed so restarts/dispose cancel init promptly.

As per coding guidelines

-        // Create a CancellationTokenSource tied to the notebook lifecycle
+        // Create a CancellationTokenSource tied to the notebook/kernel lifecycle
         const cts = new CancellationTokenSource();
         const disposables: Disposable[] = [];
@@
-            // Register handler to cancel the token if the notebook is closed
+            // Cancel if the notebook is closed
             const closeListener = workspace.onDidCloseNotebookDocument((closedNotebook) => {
                 if (closedNotebook.uri.toString() === notebook.uri.toString()) {
                     logger.info(`Notebook closed while init notebook was running, cancelling for project ${projectId}`);
                     cts.cancel();
                 }
             });
             disposables.push(closeListener);
+
+            // Cancel if the kernel is disposed (e.g., restart or shutdown)
+            const kernelDisposed = kernel.onDisposed(() => {
+                logger.info(`Kernel disposed while init notebook was running, cancelling for project ${projectId}`);
+                cts.cancel();
+            });
+            disposables.push(kernelDisposed);
📝 Committable suggestion

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

Suggested change
private async onKernelStarted(kernel: IKernel) {
// Only handle deepnote notebooks
if (kernel.notebook?.notebookType !== DEEPNOTE_NOTEBOOK_TYPE) {
return;
}
const notebook = kernel.notebook;
const projectId = notebook.metadata?.deepnoteProjectId;
if (!projectId) {
return;
}
// Check if we have a pending init notebook for this project
const pendingInit = this.projectsPendingInitNotebook.get(projectId);
if (!pendingInit) {
return; // No init notebook to run
}
logger.info(`Kernel started for Deepnote notebook, running init notebook for project ${projectId}`);
// Remove from pending list
this.projectsPendingInitNotebook.delete(projectId);
// Create a CancellationTokenSource tied to the notebook lifecycle
const cts = new CancellationTokenSource();
const disposables: Disposable[] = [];
try {
// Register handler to cancel the token if the notebook is closed
// Note: We check the URI to ensure we only cancel for the specific notebook that closed
const closeListener = workspace.onDidCloseNotebookDocument((closedNotebook) => {
if (closedNotebook.uri.toString() === notebook.uri.toString()) {
logger.info(`Notebook closed while init notebook was running, cancelling for project ${projectId}`);
cts.cancel();
}
});
disposables.push(closeListener);
// Run init notebook with cancellation support
await this.initNotebookRunner.runInitNotebookIfNeeded(projectId, notebook, cts.token);
} catch (error) {
// Check if this is a cancellation error - if so, just log and continue
if (error instanceof Error && error.message === 'Cancelled') {
logger.info(`Init notebook cancelled for project ${projectId}`);
return;
}
logger.error(`Error running init notebook: ${error}`);
// Continue anyway - don't block user if init fails
} finally {
// Always clean up the CTS and event listeners
cts.dispose();
disposables.forEach((d) => d.dispose());
}
}
private async onKernelStarted(kernel: IKernel) {
// Only handle deepnote notebooks
if (kernel.notebook?.notebookType !== DEEPNOTE_NOTEBOOK_TYPE) {
return;
}
const notebook = kernel.notebook;
const projectId = notebook.metadata?.deepnoteProjectId;
if (!projectId) {
return;
}
// Check if we have a pending init notebook for this project
const pendingInit = this.projectsPendingInitNotebook.get(projectId);
if (!pendingInit) {
return; // No init notebook to run
}
logger.info(`Kernel started for Deepnote notebook, running init notebook for project ${projectId}`);
// Remove from pending list
this.projectsPendingInitNotebook.delete(projectId);
// Create a CancellationTokenSource tied to the notebook/kernel lifecycle
const cts = new CancellationTokenSource();
const disposables: Disposable[] = [];
try {
// Cancel if the notebook is closed
const closeListener = workspace.onDidCloseNotebookDocument((closedNotebook) => {
if (closedNotebook.uri.toString() === notebook.uri.toString()) {
logger.info(
`Notebook closed while init notebook was running, cancelling for project ${projectId}`
);
cts.cancel();
}
});
disposables.push(closeListener);
// Cancel if the kernel is disposed (e.g., restart or shutdown)
const kernelDisposed = kernel.onDisposed(() => {
logger.info(
`Kernel disposed while init notebook was running, cancelling for project ${projectId}`
);
cts.cancel();
});
disposables.push(kernelDisposed);
// Run init notebook with cancellation support
await this.initNotebookRunner.runInitNotebookIfNeeded(projectId, notebook, cts.token);
} catch (error) {
if (error instanceof Error && error.message === 'Cancelled') {
logger.info(`Init notebook cancelled for project ${projectId}`);
return;
}
logger.error(`Error running init notebook: ${error}`);
} finally {
cts.dispose();
disposables.forEach((d) => d.dispose());
}
}
🤖 Prompt for AI Agents
In src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts around lines 196 to
250, the CancellationTokenSource is tied to notebook close but not to kernel
disposal/restarts; register a handler on the kernel disposal event (e.g.,
kernel.onDisposed or the correct IKernel disposal event) that calls cts.cancel()
and add that disposable to the disposables array so the CTS is cancelled
promptly on kernel dispose/restart and cleaned up in the finally block.

@FilipPyrek FilipPyrek requested a review from Artmann October 8, 2025 15:45
@FilipPyrek FilipPyrek merged commit b2649a7 into main Oct 8, 2025
3 of 4 checks passed
@FilipPyrek FilipPyrek deleted the fpyrek/grn-4761-load-and-install-requirementstxt-from-the-project-file branch October 8, 2025 15:47
Artmann pushed a commit that referenced this pull request Oct 13, 2025
* feat: implement init notebook behavior

* fix: coderabbit feedback

* fix: address code review feedback

* feat: ask about overriding requirements.txt first
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants