Skip to content

Conversation

@Artmann
Copy link
Member

@Artmann Artmann commented Oct 16, 2025

CleanShot 2025-10-16 at 12 08 03

Summary by CodeRabbit

  • New Features

    • Create new Deepnote projects from the explorer
    • Import Deepnote and Jupyter notebooks into your workspace
    • Welcome view added with quick-access shortcuts to these commands
    • Explorer now appears even in empty workspaces
  • Bug Fixes

    • Prevents errors when opening notebooks with missing block data
    • Preserves empty outputs when converting between notebook formats
  • Tests

    • Extensive unit tests added for the new empty-state commands and import flows

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 16, 2025

📝 Walkthrough

Walkthrough

The PR adds three commands (deepnote.newProject, deepnote.importNotebook, deepnote.importJupyterNotebook), a welcome view for the Deepnote explorer with quick links, and removes the prior workspace-folder visibility gating for the explorer. It introduces @deepnote/convert as a dependency and integrates convertIpynbFilesToDeepnoteFile. Implementation adds project-creation, .deepnote and .ipynb import flows (including collision checks, YAML generation, UUIDs), improved error handling and localized messages, extensive unit tests for the new flows, a VSCode test mock for the convert module, and a defensive default for selectedNotebook.blocks in the serializer.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant VS as VS Code
    participant Cmd as Deepnote Command Handler
    participant FS as File System
    participant Convert as @deepnote/convert
    participant Explorer as Deepnote Explorer

    rect rgb(200,220,240)
    Note over User,Cmd: Create New Project
    User->>VS: invoke deepnote.newProject
    VS->>Cmd: execute newProject
    Cmd->>Cmd: validate & sanitize name
    Cmd->>FS: check collisions
    Cmd->>FS: write .deepnote (YAML with UUIDs)
    Cmd->>Explorer: refresh
    Cmd->>VS: open created notebook
    end

    rect rgb(240,230,210)
    Note over User,Cmd: Import Notebook (.deepnote or .ipynb)
    User->>VS: invoke deepnote.importNotebook / deepnote.importJupyterNotebook
    VS->>Cmd: execute import flow
    alt source is .deepnote
        Cmd->>FS: copy file into workspace (handle collisions)
    else source is .ipynb
        Cmd->>Convert: convertIpynbFilesToDeepnoteFile
        Convert-->>Cmd: deepnote-formatted file
        Cmd->>FS: write .deepnote (handle collisions)
    end
    Cmd->>Explorer: refresh
    Cmd->>VS: show summary or errors
    end
Loading

Possibly related PRs

Suggested reviewers

  • andyjakubowski
  • jankuca
  • saltenasl

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: Add a welcome screen to the extension" accurately refers to a real and significant part of the changeset—the new viewsWelcome section in package.json that displays a welcome message to users. However, this is only one aspect of a substantially larger feature set. The PR primarily implements three new commands (newProject, importNotebook, importJupyterNotebook) with full import and file-conversion logic, adds a new dependency (@deepnote/convert), and includes comprehensive test coverage. While the title correctly describes the welcome UI component, it omits the core functionality that enables this feature.
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 9d96c54 and c4b12c5.

📒 Files selected for processing (2)
  • build/esbuild/build.ts (1 hunks)
  • src/notebooks/deepnote/deepnoteDataConverter.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/!(*.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/deepnote/deepnoteDataConverter.ts
  • build/esbuild/build.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 (public/protected/private) and then alphabetically
Do not add the Microsoft copyright header to new files
Use Uri.joinPath() to construct file paths instead of manual string concatenation
Add a blank line after groups of const declarations and before return statements for readability
Separate third-party imports from local file imports

Files:

  • src/notebooks/deepnote/deepnoteDataConverter.ts
  • build/esbuild/build.ts
src/notebooks/deepnote/**

📄 CodeRabbit inference engine (CLAUDE.md)

Deepnote integration code resides under src/notebooks/deepnote/

Files:

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

📄 CodeRabbit inference engine (CLAUDE.md)

deepnoteDataConverter.ts performs Deepnote data transformations

Files:

  • src/notebooks/deepnote/deepnoteDataConverter.ts
🔇 Additional comments (2)
build/esbuild/build.ts (1)

51-52: No issues found—changes are safe.

Verification confirms node:fs/promises and node:path are not imported in web code, and @deepnote/convert (which likely needs these modules) is used only in desktop-specific code (src/notebooks/deepnote/deepnoteExplorerView.ts). The additions follow the existing node:crypto pattern and won't break web builds.

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

93-99: Defensive guard correctly preserves original output state and prevents overwrites.

The logic is sound. Execution state tracking relies on executionCount, not output presence—matches codebase patterns throughout. A cell with executionCount set but outputs: [] indicates execution with no output, which is the intended behavior. No fix needed.


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

@Artmann Artmann force-pushed the chris/new-user-experience branch from 42b76c4 to 6737ef7 Compare October 16, 2025 15:48
@codecov
Copy link

codecov bot commented Oct 16, 2025

Codecov Report

❌ Patch coverage is 92.54386% with 34 lines in your changes missing coverage. Please review.
✅ Project coverage is 71%. Comparing base (0ac510e) to head (c4b12c5).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/notebooks/deepnote/deepnoteExplorerView.ts 83% 10 Missing and 10 partials ⚠️
...tebooks/deepnote/deepnoteExplorerView.unit.test.ts 96% 8 Missing and 3 partials ⚠️
src/notebooks/deepnote/deepnoteDataConverter.ts 33% 0 Missing and 2 partials ⚠️
src/notebooks/deepnote/deepnoteSerializer.ts 0% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@          Coverage Diff           @@
##            main     #74    +/-   ##
======================================
  Coverage     70%     71%            
======================================
  Files        518     518            
  Lines      38270   38721   +451     
  Branches    4856    4884    +28     
======================================
+ Hits       27160   27577   +417     
- Misses      9505    9523    +18     
- Partials    1605    1621    +16     
Files with missing lines Coverage Δ
src/notebooks/deepnote/deepnoteSerializer.ts 73% <0%> (-2%) ⬇️
src/notebooks/deepnote/deepnoteDataConverter.ts 55% <33%> (-2%) ⬇️
...tebooks/deepnote/deepnoteExplorerView.unit.test.ts 95% <96%> (+6%) ⬆️
src/notebooks/deepnote/deepnoteExplorerView.ts 71% <83%> (+21%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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: 7

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0ac510e and 6737ef7.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (5)
  • package.json (3 hunks)
  • package.nls.json (1 hunks)
  • src/notebooks/deepnote/deepnoteExplorerView.ts (3 hunks)
  • src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts (1 hunks)
  • src/notebooks/deepnote/deepnoteSerializer.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/deepnote/deepnoteExplorerView.unit.test.ts
  • src/notebooks/deepnote/deepnoteSerializer.ts
  • src/notebooks/deepnote/deepnoteExplorerView.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 (public/protected/private) and then alphabetically
Do not add the Microsoft copyright header to new files
Use Uri.joinPath() to construct file paths instead of manual string concatenation
Add a blank line after groups of const declarations and before return statements for readability
Separate third-party imports from local file imports

Files:

  • src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts
  • src/notebooks/deepnote/deepnoteSerializer.ts
  • src/notebooks/deepnote/deepnoteExplorerView.ts
**/*.unit.test.ts

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

Place unit tests in files matching *.unit.test.ts

**/*.unit.test.ts: Unit tests must use Mocha/Chai and have the .unit.test.ts extension
Place unit test files alongside the source files they test
Use assert.deepStrictEqual() for object comparisons in tests instead of checking individual properties

Files:

  • src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts
**/*.{test,spec}.ts

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

In unit tests, when a mock is returned from a promise, ensure the mocked instance has an undefined then property to avoid hanging tests

Files:

  • src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts
src/notebooks/deepnote/**

📄 CodeRabbit inference engine (CLAUDE.md)

Deepnote integration code resides under src/notebooks/deepnote/

Files:

  • src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts
  • src/notebooks/deepnote/deepnoteSerializer.ts
  • src/notebooks/deepnote/deepnoteExplorerView.ts
src/notebooks/deepnote/deepnoteSerializer.ts

📄 CodeRabbit inference engine (CLAUDE.md)

deepnoteSerializer.ts is the main serializer orchestrating Deepnote integration

Files:

  • src/notebooks/deepnote/deepnoteSerializer.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-09T11:21:57.494Z
Learning: Project is a VS Code extension for Jupyter notebooks
📚 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 : Add unit and integration tests for new features and modifications; include performance and cross-platform test coverage

Applied to files:

  • src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts
📚 Learning: 2025-10-09T11:21:57.494Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-09T11:21:57.494Z
Learning: Applies to src/notebooks/deepnote/deepnoteSerializer.ts : deepnoteSerializer.ts is the main serializer orchestrating Deepnote integration

Applied to files:

  • src/notebooks/deepnote/deepnoteSerializer.ts
📚 Learning: 2025-10-09T11:21:57.494Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-09T11:21:57.494Z
Learning: Applies to src/notebooks/deepnote/deepnoteNotebookManager.ts : deepnoteNotebookManager.ts handles Deepnote notebook state management

Applied to files:

  • src/notebooks/deepnote/deepnoteExplorerView.ts
📚 Learning: 2025-09-09T13:50:51.073Z
Learnt from: Artmann
PR: deepnote/vscode-extension#11
File: package.json:320-325
Timestamp: 2025-09-09T13:50:51.073Z
Learning: The user prefers to have a dedicated "Deepnote" category in package.json commands rather than grouping Deepnote commands under the existing "Jupyter" category.

Applied to files:

  • package.json
🧬 Code graph analysis (1)
src/notebooks/deepnote/deepnoteExplorerView.ts (1)
src/platform/common/uuid.ts (1)
  • generateUuid (10-61)
🔇 Additional comments (3)
src/notebooks/deepnote/deepnoteSerializer.ts (1)

66-66: LGTM - defensive null handling.

The ?? [] prevents potential crashes if blocks is undefined, which can occur during import flows.

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

361-364: Verify edge-case handling in convertIpynbFilesToDeepnoteFile
At lines 361–364 and 433–436 the call assumes success—confirm the imported '@deepnote/convert' function handles empty notebooks, invalid JSON, missing cells, and malformed metadata, or add validation/error wrappers.


309-309: Use path utilities instead of string manipulation.

Parsing paths with split('/') is fragile. Use VS Code's Uri methods or Node's path.basename() for robust path handling.

Example:

const fileName = Uri.parse(deepnoteUri.path).path.split('/').pop() || 'imported.deepnote';
// Better:
const fileName = deepnoteUri.path.split('/').pop() || 'imported.deepnote';
// Or even better with a helper:
const getFileName = (uri: Uri) => uri.path.substring(uri.path.lastIndexOf('/') + 1) || 'imported.deepnote';

Also applies to: 325-325, 417-417

⛔ Skipped due to learnings
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-09T11:21:57.494Z
Learning: Applies to **/*.{ts,tsx} : Use Uri.joinPath() to construct file paths instead of manual string concatenation
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/platform.instructions.md:0-0
Timestamp: 2025-09-03T13:00:18.307Z
Learning: Applies to src/platform/**/{fileSystem.ts,fileSystem.node.ts,platformService.web.ts,platformService.node.ts} : Use URI-based file operations and vscode-path for cross-platform path handling in file system and platform service implementations

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: 2

♻️ Duplicate comments (3)
src/notebooks/deepnote/deepnoteExplorerView.ts (3)

200-202: Filename collision risk from basic sanitization.

The regex replacement can produce identical filenames for different inputs (e.g., "My Project" and "My_Project" both become "my-project"). Consider appending a timestamp or counter when collisions occur.

Based on past review comments.


216-226: Extract block template to constant.

The default block structure is hardcoded here and repeated logic exists elsewhere. Extract to a shared constant or factory function.

Based on past review comments.


271-380: Significant code duplication between import methods.

importNotebook and importJupyterNotebook share ~70% identical logic (workspace check, collision detection, success messages). Extract common logic to reduce maintenance burden.

Based on past review comments.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6737ef7 and 9d96c54.

📒 Files selected for processing (3)
  • src/notebooks/deepnote/deepnoteExplorerView.ts (3 hunks)
  • src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts (2 hunks)
  • src/test/vscode-mock.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/!(*.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/deepnote/deepnoteExplorerView.unit.test.ts
  • src/test/vscode-mock.ts
  • src/notebooks/deepnote/deepnoteExplorerView.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 (public/protected/private) and then alphabetically
Do not add the Microsoft copyright header to new files
Use Uri.joinPath() to construct file paths instead of manual string concatenation
Add a blank line after groups of const declarations and before return statements for readability
Separate third-party imports from local file imports

Files:

  • src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts
  • src/test/vscode-mock.ts
  • src/notebooks/deepnote/deepnoteExplorerView.ts
**/*.unit.test.ts

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

Place unit tests in files matching *.unit.test.ts

**/*.unit.test.ts: Unit tests must use Mocha/Chai and have the .unit.test.ts extension
Place unit test files alongside the source files they test
Use assert.deepStrictEqual() for object comparisons in tests instead of checking individual properties

Files:

  • src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts
**/*.{test,spec}.ts

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

In unit tests, when a mock is returned from a promise, ensure the mocked instance has an undefined then property to avoid hanging tests

Files:

  • src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts
src/notebooks/deepnote/**

📄 CodeRabbit inference engine (CLAUDE.md)

Deepnote integration code resides under src/notebooks/deepnote/

Files:

  • src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts
  • src/notebooks/deepnote/deepnoteExplorerView.ts
🧠 Learnings (4)
📚 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 : Add unit and integration tests for new features and modifications; include performance and cross-platform test coverage

Applied to files:

  • src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts
📚 Learning: 2025-10-14T13:43:36.364Z
Learnt from: CR
PR: deepnote/deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-14T13:43:36.364Z
Learning: Applies to packages/*/src/**/*.test.{ts,tsx} : Import { describe, expect, it } from 'vitest' in tests

Applied to files:

  • src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts
📚 Learning: 2025-10-09T11:21:57.494Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-09T11:21:57.494Z
Learning: Applies to src/notebooks/deepnote/deepnoteNotebookManager.ts : deepnoteNotebookManager.ts handles Deepnote notebook state management

Applied to files:

  • src/notebooks/deepnote/deepnoteExplorerView.ts
📚 Learning: 2025-10-09T11:21:57.494Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-09T11:21:57.494Z
Learning: Applies to src/notebooks/deepnote/deepnoteActivationService.ts : deepnoteActivationService.ts manages VS Code activation for Deepnote

Applied to files:

  • src/notebooks/deepnote/deepnoteExplorerView.ts
🧬 Code graph analysis (2)
src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts (1)
src/test/vscode-mock.ts (2)
  • resetVSCodeMocks (60-79)
  • mockedVSCodeNamespaces (17-17)
src/notebooks/deepnote/deepnoteExplorerView.ts (1)
src/platform/common/uuid.ts (1)
  • generateUuid (10-61)
🔇 Additional comments (3)
src/test/vscode-mock.ts (1)

92-98: LGTM!

Mock follows existing patterns and provides appropriate no-op for test isolation.

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

248-249: Good use of TextEncoder for cross-platform compatibility.

Using TextEncoder instead of Buffer.from ensures the code works in both Node and browser-based extensions.

src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts (1)

187-736: Excellent test coverage.

The test suite comprehensively covers all three new commands with appropriate edge cases, error scenarios, and user interactions. Good use of Mocha/Chai/Sinon/ts-mockito as per coding guidelines.

coderabbitai[bot]
coderabbitai bot previously approved these changes Oct 17, 2025
@Artmann Artmann marked this pull request as ready for review October 17, 2025 06:38
@Artmann Artmann enabled auto-merge (squash) October 17, 2025 06:40
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