fix: Use ESM internally but bundle the extension as CommonJS.

[Artmann]

The VS Code ecosystem is not ready for ESM, so we need to make sure we bundle everything as CommonJS.
microsoft/vscode#130367

I've used https://github.com/mike-lischke/vscode-antlr4 as a refence.

Fixes #246

Summary by CodeRabbit

• New Features

  • Lazy-load Vega‑Lite support to improve startup and skip conversions gracefully if unavailable.

• Bug Fixes

  • Added safety checks in completion parsing to prevent crashes when parse data is missing.

• Chores

  • Updated jsonc-parser dependency to 3.3.1.
  • Removed several obsolete build helper scripts and an unused build proxy; simplified runtime module/loading behavior and package entry configuration.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Removes several Node ESM helper scripts and a runtime proxy entry, changes build outputs/externals (adds vega‑lite external, removes createRequire banner, drops proxy build), removes "type":"module" and updates package.json main and jsonc-parser, replaces esmUtils helpers with __filename/require usage, adds a dynamic vega‑lite wrapper (isVegaLiteAvailable/compile/ensureVegaLiteLoaded), adds DeepnoteDataConverter.initialize() and awaits it during notebook deserialization; multiple tests updated to match the new runtime setup.

Sequence Diagram(s)

sequenceDiagram
  participant Serializer as DeepnoteNotebookSerializer
  participant Converter as DeepnoteDataConverter
  participant Wrapper as vegaLiteWrapper
  participant Vega as vega-lite

  Serializer->>Converter: deserializeNotebook(...)
  Converter->>Converter: check cancellation
  Converter->>Converter: await initialize()
  Converter->>Wrapper: ensureVegaLiteLoaded()
  activate Wrapper
  Wrapper->>Vega: dynamic import("vega-lite")
  Vega-->>Wrapper: compile function (or throw)
  Wrapper-->>Converter: load resolved (or mark failure)
  deactivate Wrapper
  Converter->>Wrapper: compile(spec) [if available]
  alt vega conversion available
    Wrapper-->>Converter: { spec: VegaSpec }
    Converter-->>Serializer: include Vega output
  else vega unavailable
    Converter-->>Serializer: skip Vega conversion
  end

Loading

Possibly related PRs

• Migrate to ESM & update the Vega libraries #214 — Touches the same ESM/vega-lite migration surface and related build scripts.

Suggested reviewers

• saltenasl

Pre-merge checks

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 27.27% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	Title clearly describes the main change: switching to ESM internally with CommonJS bundling. Directly addresses the PR's core objective.
Linked Issues check	✅ Passed	Changes comprehensively address issue #246: removed ESM proxy entry point, switched to CommonJS bundling, fixed module resolution, and enabled proper notebook activation.
Out of Scope Changes check	✅ Passed	All changes are in scope: build config updates, ESM/CommonJS refactoring, removal of obsolete scripts and proxy layer, dependency adjustments, and mocha loader enhancements for test support.

---

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0%. Comparing base (722b206) to head (ca4354d).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@     Coverage Diff     @@
##   main   #247   +/-   ##
===========================
===========================

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

Actionable comments posted: 7

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 722b206 and 514f1de.

⛔ Files ignored due to path filters (1)

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

📒 Files selected for processing (35)

• build/add-js-extensions.mjs (0 hunks)
• build/esbuild/build.ts (3 hunks)
• build/fix-directory-imports.mjs (0 hunks)
• build/mocha-esm-loader.js (2 hunks)
• build/remove-js-extensions.mjs (0 hunks)
• package.json (2 hunks)
• src/extension.node.proxy.ts (0 hunks)
• src/kernels/deepnote/deepnoteLspClientManager.node.ts (2 hunks)
• src/kernels/raw/finder/jupyterPaths.node.unit.test.ts (4 hunks)
• src/kernels/raw/launcher/kernelLauncher.unit.test.ts (0 hunks)
• src/kernels/raw/launcher/kernelProcess.node.unit.test.ts (0 hunks)
• src/kernels/raw/session/rawSessionConnection.node.unit.test.ts (1 hunks)
• src/kernels/raw/session/zeromq.node.ts (1 hunks)
• src/notebooks/controllers/ipywidgets/scriptSourceProvider/localWidgetScriptSourceProvider.unit.test.ts (0 hunks)
• src/notebooks/controllers/ipywidgets/scriptSourceProvider/nbExtensionsPathProvider.unit.test.ts (0 hunks)
• src/notebooks/deepnote/deepnoteDataConverter.ts (3 hunks)
• src/notebooks/deepnote/deepnoteSerializer.ts (1 hunks)
• src/notebooks/deepnote/vegaLiteWrapper.ts (1 hunks)
• src/notebooks/notebookEnvironmentService.node.ts (1 hunks)
• src/platform/common/esmUtils.node.ts (0 hunks)
• src/platform/common/variables/systemVariables.node.ts (1 hunks)
• src/platform/constants.node.ts (1 hunks)
• src/platform/interpreter/filter/completionProvider.node.ts (1 hunks)
• src/test/analysisEngineTest.node.ts (0 hunks)
• src/test/common.node.ts (0 hunks)
• src/test/common/variables/envVarsService.vscode.test.ts (0 hunks)
• src/test/constants.node.ts (0 hunks)
• src/test/debuggerTest.node.ts (0 hunks)
• src/test/extension.serviceRegistry.vscode.test.ts (0 hunks)
• src/test/index.node.ts (0 hunks)
• src/test/pythonEnvironments/constants.ts (0 hunks)
• src/test/smokeTest.node.ts (0 hunks)
• src/test/standardTest.node.ts (0 hunks)
• src/test/testRunner.ts (1 hunks)
• src/test/unittests.ts (1 hunks)

💤 Files with no reviewable changes (19)

• src/platform/common/esmUtils.node.ts
• src/test/smokeTest.node.ts
• src/extension.node.proxy.ts
• src/kernels/raw/launcher/kernelLauncher.unit.test.ts
• src/test/analysisEngineTest.node.ts
• src/test/index.node.ts
• build/add-js-extensions.mjs
• src/test/pythonEnvironments/constants.ts
• src/test/extension.serviceRegistry.vscode.test.ts
• src/test/constants.node.ts
• build/fix-directory-imports.mjs
• src/test/common/variables/envVarsService.vscode.test.ts
• src/notebooks/controllers/ipywidgets/scriptSourceProvider/nbExtensionsPathProvider.unit.test.ts
• src/test/debuggerTest.node.ts
• build/remove-js-extensions.mjs
• src/test/common.node.ts
• src/notebooks/controllers/ipywidgets/scriptSourceProvider/localWidgetScriptSourceProvider.unit.test.ts
• src/kernels/raw/launcher/kernelProcess.node.unit.test.ts
• src/test/standardTest.node.ts

🧰 Additional context used

📓 Path-based instructions (10)

**/*.node.ts

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

Platform Implementation - Desktop: Use .node.ts files for full file system access and Python environments

Files:

• src/platform/interpreter/filter/completionProvider.node.ts
• src/kernels/deepnote/deepnoteLspClientManager.node.ts
• src/notebooks/notebookEnvironmentService.node.ts
• src/platform/common/variables/systemVariables.node.ts
• src/kernels/raw/session/zeromq.node.ts
• src/platform/constants.node.ts

**/*.{ts,tsx}

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

**/*.{ts,tsx}: Use l10n.t() for all user-facing strings in TypeScript files
Use typed error classes from src/platform/errors/ instead of generic errors
Use ILogger service instead of console.log for logging
Preserve error details in error messages while scrubbing personally identifiable information (PII)
Prefer async/await over promise chains
Handle cancellation with CancellationToken

Order method, fields and properties first by accessibility (public/private/protected) and then by alphabetical order

Files:

• src/platform/interpreter/filter/completionProvider.node.ts
• src/test/unittests.ts
• src/kernels/raw/finder/jupyterPaths.node.unit.test.ts
• src/kernels/raw/session/rawSessionConnection.node.unit.test.ts
• src/notebooks/deepnote/deepnoteSerializer.ts
• src/notebooks/deepnote/deepnoteDataConverter.ts
• src/kernels/deepnote/deepnoteLspClientManager.node.ts
• src/notebooks/deepnote/vegaLiteWrapper.ts
• src/notebooks/notebookEnvironmentService.node.ts
• src/test/testRunner.ts
• src/platform/common/variables/systemVariables.node.ts
• build/esbuild/build.ts
• src/kernels/raw/session/zeromq.node.ts
• src/platform/constants.node.ts

src/platform/**/*.ts

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

src/platform/**/*.ts: Use Inversify-based dependency injection with IServiceManager and IServiceContainer for managing service relationships and dependencies
Define service interfaces using Symbol-based exports (e.g., export const IFileSystem = Symbol('IFileSystem')) paired with corresponding TypeScript interfaces
Register types using the pattern serviceManager.addSingleton<IService>(IService, ServiceImplementation) in registerTypes functions
Use @Injectable() and @Inject() decorators from Inversify for class dependency injection configuration
Implement unified interfaces for cross-platform services (IFileSystem, IPlatformService, etc.) with graceful degradation for web environment limitations
Use structured logging with context via logger.info, logger.error, etc. rather than console.log
Provide platform capability checks and feature detection before using advanced platform-specific features
Register separate service implementations for desktop and web environments, allowing the DI container to resolve the correct platform implementation

Files:

• src/platform/interpreter/filter/completionProvider.node.ts
• src/platform/common/variables/systemVariables.node.ts
• src/platform/constants.node.ts

src/platform/**/*.{node,web}.ts

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

Create separate implementations with .node.ts and .web.ts file suffixes for platform-specific code (e.g., fileSystem.node.ts for Node.js, fileSystem.web.ts for browser)

Files:

• src/platform/interpreter/filter/completionProvider.node.ts
• src/platform/common/variables/systemVariables.node.ts
• src/platform/constants.node.ts

**/*.ts

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

**/*.ts: ALWAYS check the 'Core - Build' task output for TypeScript compilation errors before running any script or declaring work complete
ALWAYS run npm run format-fix before committing changes to ensure proper code formatting
FIX all TypeScript compilation errors before moving forward with development
Use npm run format-fix to auto-fix TypeScript formatting issues before committing
Use npm run lint to check for linter issues in TypeScript files and attempt to fix before committing

Files:

• src/platform/interpreter/filter/completionProvider.node.ts
• src/test/unittests.ts
• src/kernels/raw/finder/jupyterPaths.node.unit.test.ts
• src/kernels/raw/session/rawSessionConnection.node.unit.test.ts
• src/notebooks/deepnote/deepnoteSerializer.ts
• src/notebooks/deepnote/deepnoteDataConverter.ts
• src/kernels/deepnote/deepnoteLspClientManager.node.ts
• src/notebooks/deepnote/vegaLiteWrapper.ts
• src/notebooks/notebookEnvironmentService.node.ts
• src/test/testRunner.ts
• src/platform/common/variables/systemVariables.node.ts
• build/esbuild/build.ts
• src/kernels/raw/session/zeromq.node.ts
• src/platform/constants.node.ts

src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.{ts,tsx}: Don't add the Microsoft copyright header to new files
Use Uri.joinPath() for constructing file paths instead of string concatenation with / to ensure platform-correct path separators
Follow established patterns when importing new packages, using helper imports rather than direct imports (e.g., use import { generateUuid } from '../platform/common/uuid' instead of importing uuid directly)
Add blank lines after const groups and before return statements for readability
Separate third-party and local file imports

Files:

• src/platform/interpreter/filter/completionProvider.node.ts
• src/test/unittests.ts
• src/kernels/raw/finder/jupyterPaths.node.unit.test.ts
• src/kernels/raw/session/rawSessionConnection.node.unit.test.ts
• src/notebooks/deepnote/deepnoteSerializer.ts
• src/notebooks/deepnote/deepnoteDataConverter.ts
• src/kernels/deepnote/deepnoteLspClientManager.node.ts
• src/notebooks/deepnote/vegaLiteWrapper.ts
• src/notebooks/notebookEnvironmentService.node.ts
• src/test/testRunner.ts
• src/platform/common/variables/systemVariables.node.ts
• src/kernels/raw/session/zeromq.node.ts
• src/platform/constants.node.ts

**/*.unit.test.ts

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

Create unit tests in *.unit.test.ts files

**/*.unit.test.ts: Unit tests use Mocha/Chai framework with .unit.test.ts extension
Test files should be placed alongside the source files they test
Use assert.deepStrictEqual() for object comparisons instead of checking individual properties

Files:

• src/kernels/raw/finder/jupyterPaths.node.unit.test.ts
• src/kernels/raw/session/rawSessionConnection.node.unit.test.ts

**/*.test.ts

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

Create integration tests in *.test.ts files (not *.unit.test.ts)

**/*.test.ts: ALWAYS check the 'Unittest - Build' task output for TypeScript compilation errors before running any script or declaring work complete
When a mock is returned from a promise in unit tests, ensure the mocked instance has an undefined then property to avoid hanging tests
Use npm run test:unittests for TypeScript unit tests before committing changes

Files:

• src/kernels/raw/finder/jupyterPaths.node.unit.test.ts
• src/kernels/raw/session/rawSessionConnection.node.unit.test.ts

src/kernels/**/*.ts

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

src/kernels/**/*.ts: Respect CancellationToken in all async operations across kernel execution, session communication, and finder operations
Implement proper error handling in files using custom error types from errors/ directory (KernelDeadError, KernelConnectionTimeoutError, KernelInterruptTimeoutError, JupyterInvalidKernelError, KernelDependencyError)

Files:

• src/kernels/raw/finder/jupyterPaths.node.unit.test.ts
• src/kernels/raw/session/rawSessionConnection.node.unit.test.ts
• src/kernels/deepnote/deepnoteLspClientManager.node.ts
• src/kernels/raw/session/zeromq.node.ts

src/notebooks/deepnote/**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Deepnote integration located in src/notebooks/deepnote/ with refactored architecture: deepnoteTypes.ts (type definitions), deepnoteNotebookManager.ts (state management), deepnoteNotebookSelector.ts (UI selection logic), deepnoteDataConverter.ts (data transformations), deepnoteSerializer.ts (main serializer/orchestration), deepnoteActivationService.ts (VSCode activation)

Files:

• src/notebooks/deepnote/deepnoteSerializer.ts
• src/notebooks/deepnote/deepnoteDataConverter.ts
• src/notebooks/deepnote/vegaLiteWrapper.ts

🧬 Code graph analysis (4)

src/notebooks/deepnote/deepnoteDataConverter.ts (2)

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

• ensureVegaLiteLoaded (50-52)

src/test/mocks/vsc/extHostedTypes.ts (1)

• NotebookCellOutputItem (16-69)

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

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

• logger (35-48)

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

src/platform/common/utils/encoder.ts (1)

• toPythonSafePath (4-6)

src/platform/common/variables/systemVariables.node.ts (2)

src/platform/common/variables/systemVariables.web.ts (1)

• workspaceFolder (41-43)

build/mocha-esm-loader.js (1)

• __dirname (11-11)

⏰ 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). (3)

• GitHub Check: Lint & Format
• GitHub Check: Build & Test
• GitHub Check: Build & Package Extension

🔇 Additional comments (15)

src/platform/common/variables/systemVariables.node.ts (1)

26-26: LGTM! Correct migration to CommonJS global.

The switch from getDirname(import.meta.url) to __dirname is appropriate for CommonJS bundling. Both resolve to the same location (the compiled module's directory), maintaining equivalent behavior.

src/platform/interpreter/filter/completionProvider.node.ts (1)

78-80: LGTM! Good defensive coding.

Prevents NPE when parseTree returns undefined. Consistent with the existing error-handling pattern.

src/kernels/raw/session/zeromq.node.ts (3)

42-142: Helper functions properly implemented.

The telemetry and platform detection helpers correctly use logger for diagnostics and handle errors gracefully. The once-only telemetry flag prevents duplicates.

---

24-25: The fallback path to dist/node_modules/zeromqold is not an assumption. The esbuild configuration (build/esbuild/build.ts lines 512–513) explicitly copies zeromqold from node_modules to dist/node_modules during the build process. Additionally, zeromqold is declared as a dependency in package.json, ensuring it's installed. This fallback mechanism is intentional and properly configured.

---

16-17: No issues found. The build configuration confirms that zeromq and zeromqold are externalized (copied to dist/node_modules, not bundled), and .node.ts files are compiled to CommonJS format at runtime. The bare require() calls are correct for this setup, and the eslint-disable comments are appropriate.

src/kernels/raw/session/rawSessionConnection.node.unit.test.ts (1)

36-38: Direct require for JupyterLab kernel default is reasonable here

Using a single, scoped CommonJS require to grab KernelConnection for stubbing fits the test’s needs, and the eslint disable is correctly limited to that one line. Only caveat: if you later bump @jupyterlab/services or its entrypoints, please re‑verify that @jupyterlab/services/lib/kernel/default still resolves and exposes KernelConnection as expected.

src/test/unittests.ts (1)

8-9: LGTM. Direct require('module') is cleaner for CommonJS context.

src/kernels/raw/finder/jupyterPaths.node.unit.test.ts (2)

340-350: LGTM. Using __filename as test path input is valid. The ESM loader shim injects this global.

---

362-373: Consistent with previous test. Same pattern applied correctly.

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

151-160: Remove or reconsider the __filename path check. The code executes Python on a remote kernel but gates the sys.executable output on whether the local extension file exists remotely—this will fail on all truly remote kernels (GitHub Codespaces, remote servers, etc.). Either remove the conditional or replace it with a path that actually exists in the remote environment.

src/platform/constants.node.ts (1)

16-17: Node‑specific require override looks fine.

Using a direct require with the lint suppression here is appropriate for this .node.ts CJS entry; no changes needed.

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

13-16: Direct require usage in the desktop LSP manager is appropriate.

Given this file compiles into the CJS desktop bundle and vscode-languageclient/node is handled via esbuild externals, switching from createRequire to plain require/require.resolve with lint suppression is a reasonable simplification.

Also applies to: 541-546

package.json (1)

64-64: Desktop entrypoint + jsonc‑parser bump look consistent; please verify the ESM path.

Pointing "main" at ./dist/extension.node.js matches the new esbuild desktop build target and removal of the proxy entry. For jsonc-parser@3.3.1, the build now relies on its ESM entry via lib/esm/main.js; double‑check that this file exists in that version so the desktop alias used in build/esbuild/build.ts doesn’t hit a module‑not‑found at bundle or runtime.

Also applies to: 2535-2535

build/esbuild/build.ts (2)

60-67: Additional externals look reasonable.

Pulling @azure/functions-core, the node: built‑ins, and ansi-regex into commonExternals keeps the desktop/web configs consistent and avoids duplicated polyfills; no changes needed here.

---

242-259: Alias consolidation and explicit CJS/ESM formats align with the PR’s goals.

Centralizing the alias map (including the desktop‑only override) and setting format to CJS for desktop and ESM for web makes the build behavior much clearer and matches the “CJS desktop / ESM web” objective; I don’t see issues in this block.

[coderabbitai]

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 514f1de and 9d5174c.

📒 Files selected for processing (4)

• build/mocha-esm-loader.js (3 hunks)
• src/notebooks/deepnote/deepnoteSerializer.ts (1 hunks)
• src/notebooks/deepnote/vegaLiteWrapper.ts (1 hunks)
• src/test/testRunner.ts (0 hunks)

💤 Files with no reviewable changes (1)

• src/test/testRunner.ts

🧰 Additional context used

📓 Path-based instructions (4)

**/*.{ts,tsx}

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

**/*.{ts,tsx}: Use l10n.t() for all user-facing strings in TypeScript files
Use typed error classes from src/platform/errors/ instead of generic errors
Use ILogger service instead of console.log for logging
Preserve error details in error messages while scrubbing personally identifiable information (PII)
Prefer async/await over promise chains
Handle cancellation with CancellationToken

Order method, fields and properties first by accessibility (public/private/protected) and then by alphabetical order

Files:

• src/notebooks/deepnote/deepnoteSerializer.ts
• src/notebooks/deepnote/vegaLiteWrapper.ts

**/*.ts

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

**/*.ts: ALWAYS check the 'Core - Build' task output for TypeScript compilation errors before running any script or declaring work complete
ALWAYS run npm run format-fix before committing changes to ensure proper code formatting
FIX all TypeScript compilation errors before moving forward with development
Use npm run format-fix to auto-fix TypeScript formatting issues before committing
Use npm run lint to check for linter issues in TypeScript files and attempt to fix before committing

Files:

• src/notebooks/deepnote/deepnoteSerializer.ts
• src/notebooks/deepnote/vegaLiteWrapper.ts

src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.{ts,tsx}: Don't add the Microsoft copyright header to new files
Use Uri.joinPath() for constructing file paths instead of string concatenation with / to ensure platform-correct path separators
Follow established patterns when importing new packages, using helper imports rather than direct imports (e.g., use import { generateUuid } from '../platform/common/uuid' instead of importing uuid directly)
Add blank lines after const groups and before return statements for readability
Separate third-party and local file imports

Files:

• src/notebooks/deepnote/deepnoteSerializer.ts
• src/notebooks/deepnote/vegaLiteWrapper.ts

src/notebooks/deepnote/**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Deepnote integration located in src/notebooks/deepnote/ with refactored architecture: deepnoteTypes.ts (type definitions), deepnoteNotebookManager.ts (state management), deepnoteNotebookSelector.ts (UI selection logic), deepnoteDataConverter.ts (data transformations), deepnoteSerializer.ts (main serializer/orchestration), deepnoteActivationService.ts (VSCode activation)

Files:

• src/notebooks/deepnote/deepnoteSerializer.ts
• src/notebooks/deepnote/vegaLiteWrapper.ts

🧬 Code graph analysis (2)

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

src/kernels/execution/cellExecutionCreator.ts (1)

• token (41-43)

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

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

• logger (35-48)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Build & Test

🔇 Additional comments (9)

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

78-83: LGTM! Past review feedback addressed.

The cancellation check after initialize() ensures we don't proceed with parsing if cancelled during the async load. Implementation is clean.

src/notebooks/deepnote/vegaLiteWrapper.ts (5)

1-12: Clean module setup.

Type-only imports and module-level state properly initialized. Comment explains the ESM/CJS interop issue clearly.

---

14-40: Robust loading with proper guards.

Past review feedback implemented: typeof === 'function' checks prevent invalid states. Concurrent load prevention via loadPromise pattern is correct. Graceful degradation on failure.

---

45-47: LGTM!

Consistent with the typeof === 'function' guards used throughout. Past review feedback addressed.

---

53-59: Proper guard and graceful degradation.

Returns undefined when unavailable. Consistent with other guards in file.

---

61-63: Simple and correct.

Clean public API for triggering the load.

build/mocha-esm-loader.js (3)

16-48: Centralized Python extension mock looks good; just ensure it covers your real usage.

Nice consolidation into PYTHON_EXTENSION_MOCK, removing duplication and keeping the mock shape in one place. Please double‑check that your code/tests only rely on the PythonExtension.api().environments surface and the exported constants/types you’ve stubbed here, so there’s no hidden dependency on unmocked behavior.

---

391-398: Reuse of PYTHON_EXTENSION_MOCK for the vscode-mock URL path is correct.

Hooking moduleName === 'python-extension' here and returning the shared PYTHON_EXTENSION_MOCK keeps the mock consistent between specifier‑based resolution and direct vscode-mock:///python-extension loads; no issues from my side.

---

414-432: CJS globals shim for /out/*.js is reasonable; scope and semantics look fine.

Injecting __filename, __dirname, and require for file:// URLs under /out/ lines up with the rest of the PR (ESM build but CJS‑style globals in compiled output). The hook is narrowly scoped and shouldn’t affect non‑out modules; only side effect is added imports and a few extra top‑of‑file lines in stack traces, which seems acceptable for tests.

[OlegWock]

Tested and everything works (but please update package command)

[OlegWock]

In which cases this could happen?

[Artmann]

It shouldn't, but as its loaded at runtime, it can fail.

[OlegWock]

But then it would go into catch block?

[Artmann]

Yeah its probably cleaner to throw an error here and let the catch handle it 👍

[OlegWock]

npm run package fails with sh: vsce: command not found, can be fixed by running vsce with npx

[Artmann]

Added @vscode/vsce to the dev dependencies.

[Artmann]

That doesn't work with the licenses so we can't have it as a direct dependency :(

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 5d69e3f and ca4354d.

⛔ Files ignored due to path filters (1)

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

📒 Files selected for processing (2)

• package.json (2 hunks)
• src/notebooks/deepnote/vegaLiteWrapper.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (4)

**/*.{ts,tsx}

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

**/*.{ts,tsx}: Use l10n.t() for all user-facing strings in TypeScript files
Use typed error classes from src/platform/errors/ instead of generic errors
Use ILogger service instead of console.log for logging
Preserve error details in error messages while scrubbing personally identifiable information (PII)
Prefer async/await over promise chains
Handle cancellation with CancellationToken

Order method, fields and properties first by accessibility (public/private/protected) and then by alphabetical order

Files:

• src/notebooks/deepnote/vegaLiteWrapper.ts

**/*.ts

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

**/*.ts: ALWAYS check the 'Core - Build' task output for TypeScript compilation errors before running any script or declaring work complete
ALWAYS run npm run format-fix before committing changes to ensure proper code formatting
FIX all TypeScript compilation errors before moving forward with development
Use npm run format-fix to auto-fix TypeScript formatting issues before committing
Use npm run lint to check for linter issues in TypeScript files and attempt to fix before committing

Files:

• src/notebooks/deepnote/vegaLiteWrapper.ts

src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.{ts,tsx}: Don't add the Microsoft copyright header to new files
Use Uri.joinPath() for constructing file paths instead of string concatenation with / to ensure platform-correct path separators
Follow established patterns when importing new packages, using helper imports rather than direct imports (e.g., use import { generateUuid } from '../platform/common/uuid' instead of importing uuid directly)
Add blank lines after const groups and before return statements for readability
Separate third-party and local file imports

Files:

• src/notebooks/deepnote/vegaLiteWrapper.ts

src/notebooks/deepnote/**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Deepnote integration located in src/notebooks/deepnote/ with refactored architecture: deepnoteTypes.ts (type definitions), deepnoteNotebookManager.ts (state management), deepnoteNotebookSelector.ts (UI selection logic), deepnoteDataConverter.ts (data transformations), deepnoteSerializer.ts (main serializer/orchestration), deepnoteActivationService.ts (VSCode activation)

Files:

• src/notebooks/deepnote/vegaLiteWrapper.ts

🧬 Code graph analysis (1)

src/notebooks/deepnote/vegaLiteWrapper.ts (2)

src/platform/webviews/webview.ts (1)

• loadFailed (21-23)

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

• logger (35-48)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Build & Test

🔇 Additional comments (5)

package.json (1)

64-64: Main entry change aligns with CommonJS bundling strategy.

Removing the proxy layer is consistent with the PR's goal to bundle as CommonJS internally.

src/notebooks/deepnote/vegaLiteWrapper.ts (4)

1-13: LGTM! Clear explanation and appropriate state management.

The comment explains why this wrapper exists, and the state variables support proper loading semantics.

---

14-37: Solid error handling and race-condition prevention.

The loadPromise pattern prevents concurrent loads, and the early-return guards are correct. The fail-fast approach (no retry on loadFailed) is appropriate for bundled modules.

---

39-56: Consistent guard checks ensure safe access.

Both functions properly check typeof compileFunction === 'function', addressing the earlier review feedback.

---

58-60: LGTM! Clean public API.

Simple and effective interface for triggering the load.