Skip to content

Conversation

@Artmann
Copy link
Member

@Artmann Artmann commented Nov 1, 2025

CleanShot 2025-11-01 at 14 35 47

Summary by CodeRabbit

  • New Features

    • Added support for Plotly v1 chart visualization in notebooks
    • Jupyter Renderers extension is now declared as a required extension
  • Tests

    • Added test coverage for Plotly output handling, including round‑trip conversion verification
  • Chores

    • Expanded internal word dictionary to recognize additional plotting-related terms

@Artmann Artmann requested a review from a team as a code owner November 1, 2025 13:39
@Artmann Artmann marked this pull request as draft November 1, 2025 13:39
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 1, 2025

📝 Walkthrough

Walkthrough

This PR adds Plotly v1 JSON chart output support to the Deepnote notebook converter, updates spell-check words for Plotly-related terms, declares a dependency/pack on ms-toolsai.jupyter-renderers in package.json, and adds unit tests validating Plotly output conversion and a round-trip conversion preserving payloads.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Deepnote as Deepnote blocks
    participant Converter as DeepnoteDataConverter
    participant VSCode as VS Code Notebook cells

    rect rgb(230,245,255)
    Note over Deepnote,Converter: Input parsing (convertBlocksToCells)
    Deepnote->>Converter: execute_result with application/vnd.plotly.v1+json
    Converter-->>VSCode: NotebookCell with output item mime: application/vnd.plotly.v1+json (json payload)
    end

    rect rgb(240,255,230)
    Note over VSCode,Converter: Output serialization (convertCellsToBlocks)
    VSCode->>Converter: NotebookCell containing NotebookCellOutputItem.json (plotly mime)
    Converter-->>Deepnote: execute_result block with application/vnd.plotly.v1+json (identical payload)
    end
Loading

Possibly related PRs

Suggested reviewers

  • saltenasl
  • jamesbhobbs

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 "fix: Add support for Plotly charts" directly aligns with the changeset's primary purpose. The modifications span spell-checker configuration, extension dependencies, MIME type handling for Plotly JSON, and corresponding test coverage—all focused on enabling Plotly chart rendering. The title is concise, specific, and accurately reflects the main change without vague language or unnecessary detail.

📜 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 93dc4c6 and d2aed58.

📒 Files selected for processing (1)
  • package.json (1 hunks)
🔇 Additional comments (1)
package.json (1)

28-33: Manifest additions for Plotly support are sound.

The extensionDependencies and extensionPack fields correctly declare ms-toolsai.jupyter-renderers. The data converter properly emits application/vnd.plotly.v1+json (verified at lines 255–256, 347–351) and tests confirm the MIME type handling is correct.


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

@codecov
Copy link

codecov bot commented Nov 1, 2025

Codecov Report

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

Additional details and impacted files
@@          Coverage Diff          @@
##            main    #155   +/-   ##
=====================================
  Coverage     73%     73%           
=====================================
  Files        550     550           
  Lines      44004   44030   +26     
  Branches    5312    5314    +2     
=====================================
+ Hits       32189   32215   +26     
  Misses     10048   10048           
  Partials    1767    1767           
Files with missing lines Coverage Δ
src/notebooks/deepnote/deepnoteDataConverter.ts 60% <100%> (+<1%) ⬆️
...ebooks/deepnote/deepnoteDataConverter.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.

@Artmann Artmann marked this pull request as ready for review November 1, 2025 13:50
@Artmann Artmann enabled auto-merge (squash) November 1, 2025 13:50
coderabbitai[bot]
coderabbitai bot previously approved these changes Nov 1, 2025
Copy link
Member

@saltenasl saltenasl left a comment

Choose a reason for hiding this comment

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

outputs are now rendered correctly after reloading the vscode window, thanks!

@Artmann Artmann merged commit 4e1645d into main Nov 1, 2025
13 checks passed
@Artmann Artmann deleted the chris/render-plotly-charts branch November 1, 2025 16:50
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