Skip to content

Conversation

@saltenasl
Copy link
Member

@saltenasl saltenasl commented Oct 31, 2025

Summary by CodeRabbit

  • New Features

    • Added "Open in Deepnote" action button in the notebook toolbar for Deepnote-type notebooks
    • Extended support to open Deepnote files from both notebook and text editor contexts
  • Improvements

    • Refined save workflow to handle VS Code notebook saves before upload
    • Improved progress messages and non-cancellable upload feedback
    • Better error messages for missing/unsupported files and upload failures

@linear
Copy link

linear bot commented Oct 31, 2025

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 31, 2025

📝 Walkthrough

Walkthrough

Adds a deepnote.openInDeepnote command and a notebook toolbar entry visible when notebookType == 'deepnote'. The handler now detects active notebook editors (type 'deepnote') or falls back to text editors, triggers appropriate save behavior (global save for notebooks, save active editor when dirty), validates .deepnote files and size, reads and uploads the file via the import client, then opens the resulting Deepnote URL. Comprehensive unit tests exercise notebook and text-editor flows, error paths, and file I/O behaviors.

Sequence Diagram

sequenceDiagram
    actor User
    participant VSCode
    participant Handler
    participant FS as FileSystem
    participant Import as ImportClient
    participant Browser

    User->>VSCode: Click "Open in Deepnote"
    VSCode->>Handler: handleOpenInDeepnote()

    rect rgb(230,240,255)
    Note over Handler: Determine context (notebook vs text editor)
    Handler->>Handler: inspect activeNotebookEditor / activeTextEditor
    end

    rect rgb(245,235,220)
    Note over Handler: Save before upload
    alt notebook active
        Handler->>VSCode: execute global save
    else text editor active and dirty
        Handler->>VSCode: save active editor
    end
    VSCode-->>Handler: save result
    end

    rect rgb(235,255,235)
    Note over Handler: Validate and read file
    Handler->>FS: stat URI
    FS-->>Handler: metadata
    Handler->>FS: readFile
    FS-->>Handler: contents
    end

    rect rgb(255,250,230)
    Note over Handler: Import and upload
    Handler->>Import: initImport()
    Import-->>Handler: upload token/info
    Handler->>Import: uploadFile(contents)
    Import-->>Handler: upload result with URL
    end

    Handler->>Browser: openExternal(URL)
    Browser-->>User: Deepnote opens in browser
Loading

Possibly related PRs

Suggested reviewers

  • Artmann
  • jamesbhobbs
  • andyjakubowski

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "feat(open-in-deepnote): button in toolbar" directly describes the primary user-facing change: adding a toolbar button for the open-in-deepnote feature. It's concise, uses proper conventional commit format, and is specific enough to communicate the main objective. While the PR includes supporting changes (handler refactoring and tests), the title appropriately focuses on the core feature being delivered.

📜 Recent 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 87c5210 and 4214d58.

📒 Files selected for processing (1)
  • src/notebooks/deepnote/openInDeepnoteHandler.node.unit.test.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/openInDeepnoteHandler.node.unit.test.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

Files:

  • src/notebooks/deepnote/openInDeepnoteHandler.node.unit.test.ts
**/*.unit.test.ts

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

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

Files:

  • src/notebooks/deepnote/openInDeepnoteHandler.node.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/openInDeepnoteHandler.node.unit.test.ts
🧬 Code graph analysis (1)
src/notebooks/deepnote/openInDeepnoteHandler.node.unit.test.ts (2)
src/test/vscode-mock.ts (2)
  • resetVSCodeMocks (60-79)
  • mockedVSCodeNamespaces (17-17)
src/test/datascience/mockDocument.ts (1)
  • isDirty (108-110)
🔇 Additional comments (1)
src/notebooks/deepnote/openInDeepnoteHandler.node.unit.test.ts (1)

58-410: Test matrix looks great. Every happy and unhappy path is exercised with clean sandboxing and clear assertions.


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

@codecov
Copy link

codecov bot commented Oct 31, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73%. Comparing base (c6640a4) to head (4214d58).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@          Coverage Diff           @@
##            main    #153    +/-   ##
======================================
  Coverage     72%     73%            
======================================
  Files        548     548            
  Lines      43061   43272   +211     
  Branches    5252    5256     +4     
======================================
+ Hits       31337   31590   +253     
+ Misses      9984    9942    -42     
  Partials    1740    1740            
Files with missing lines Coverage Δ
...c/notebooks/deepnote/openInDeepnoteHandler.node.ts 92% <100%> (+76%) ⬆️
...s/deepnote/openInDeepnoteHandler.node.unit.test.ts 100% <100%> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

coderabbitai[bot]
coderabbitai bot previously approved these changes Oct 31, 2025
@saltenasl saltenasl marked this pull request as ready for review October 31, 2025 16:14
@saltenasl saltenasl requested a review from a team as a code owner October 31, 2025 16:14
@saltenasl saltenasl merged commit 7cde864 into main Oct 31, 2025
13 checks passed
@saltenasl saltenasl deleted the lukas/grn-5041-add-open-in-deepnote-button-in-top-bar branch October 31, 2025 16:42
Artmann pushed a commit that referenced this pull request Oct 31, 2025
* feat(open-in-deepnote): button in toolbar

* chore: fix tests
saltenasl added a commit that referenced this pull request Nov 1, 2025
…lete) (#88)

* feat: Set up a custom renderer for data frames.

* wip

* feat: Implement deepnote big number chart support and renderer

* refactor: Clean up debug logs and improve big number block conversion handling

* feat: Pass block metadata to cell outputs for renderer to access

* feat: Set up a custom renderer for data frames.

* wip

* add the table state.

* page size handling

* add page navigation

* Generate Python code before executing the cell.

* clean up

* pr feedback.

* Update build/esbuild/build.ts

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* feat: Support Deepnote input blocks

* feat: Move DEEPNOTE_VSCODE_RAW_CONTENT_KEY into constants file

* feedback

* feat: Add Deepnote input blocks converters

* refactor: Remove debug logs and refine metadata parsing in Deepnote converters

* feat: Add chart big number converter tests

* Reformat test code

* Refactor ChartBigNumberBlockConverter tests to use deepStrictEqual for assertions

* Remove debug console.log

* docs and metadata changes.

* fix: Spread operator object fallback

* Merge remote-tracking branch 'origin/chris/display-data-frames' into tomaskislan/grn-4762-support-big-number-blocks

* use the latest blocks package.

* add the packages permission

* simplify execution flow.

* remove copyright header

* clean up the code

* revert controller changes

* pr feedback

* pr feedback

* fix the tests

* guard metadata spread against undefined.

* fix: Merge cleanup

* More merge cleanup

* Fix test

* feat: Add big number chart json config execution support

* feat: Add commands to add Deepnote SQL block and big number chart

* fix: Enhance error handling for big number metadata parsing and improve chart big number renderer logic

* refactor: Simplify chart big number renderer by directly rendering to the element and improve cleanup logic for unmounting components

* Fix import

* fix: Change deepnote_big_number_comparison_type to string type for better flexibility

* fix: Remove constants, accidentaly added to wrong branch

* Update package.json

* feat: Add new commands for SQL and Big Number Chart blocks

* feat: Update command titles and add SQL block functionality

* Fix imports

* fix: Fix imports

* fix: Remove unused code

* feat: Add support for new input blocks in Deepnote, including text, textarea, select, slider, checkbox, date, date range, file, and button blocks

* feat(big-number): Integrate react-error-boundary for error handling and enhance big number output rendering

* Update test snapshots

* feat: Update input create default values to match those in deepnote app

Signed-off-by: Tomas Kislan <tomas@kislan.sk>

* feat: Add support for project notebook management (create, rename, delete)

Signed-off-by: Tomas Kislan <tomas@kislan.sk>

* feat: Localize commands, extract notebook creation logic to reuse in methods

Signed-off-by: Tomas Kislan <tomas@kislan.sk>

* feat: Extract command ids into separate config files

Signed-off-by: Tomas Kislan <tomas@kislan.sk>

* refactor: Improve notebook renaming logic in DeepnoteExplorerView

Signed-off-by: Tomas Kislan <tomas@kislan.sk>

* refactor: Remove lowercase logic from notebook names checks

Signed-off-by: Tomas Kislan <tomas@kislan.sk>

* test: Implement DeepnoteExplorerView tests

Signed-off-by: Tomas Kislan <tomas@kislan.sk>

* fix: Fix spell check

Signed-off-by: Tomas Kislan <tomas@kislan.sk>

* test: Change assertion

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Revert "fix: Fix spell check"

This reverts commit 8becf4e.

* Revert "test: Change assertion"

This reverts commit fe81891.

* fix: Fix spellcheck

Signed-off-by: Tomas Kislan <tomas@kislan.sk>

* fix: Fix test yaml

Signed-off-by: Tomas Kislan <tomas@kislan.sk>

* test: Update assertions

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* fix: Change version assertion from string to number

* fix: Fix version type in deepnote project unit test

* refactor: Reuse promptForNotebookName in rename command, simplify createAndAddNotebookToProject function

Signed-off-by: Tomas Kislan <tomas@kislan.sk>

* fix: Exclude current name from validation when renaming notebook

Signed-off-by: Tomas Kislan <tomas@kislan.sk>

* fix: Sort imports

Signed-off-by: Tomas Kislan <tomas@kislan.sk>

* feat(open-in-deepnote): button in toolbar (#153)

* feat(open-in-deepnote): button in toolbar

* chore: fix tests

* fix(warn-on-startup): move `jupyterViewVariables` view into the existing deepnote container (#148)

* feat: Order projects and notebooks alphabetically.

* fix: Don't recreate the tree.

* fix test

* fix dumb tests.

* coderabbit feedback

* more feedback

---------

Signed-off-by: Tomas Kislan <tomas@kislan.sk>
Co-authored-by: Christoffer Artmann <artgaard@gmail.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Lukas Šaltėnas <lukas.saltenas@gmail.com>
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