feat(charts): Add visualization blocks support #66
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #66 +/- ##
======================================
Coverage 71% 71%
======================================
Files 518 520 +2
Lines 38721 38865 +144
Branches 4884 4895 +11
======================================
+ Hits 27577 27705 +128
- Misses 9523 9537 +14
- Partials 1621 1623 +2
🚀 New features to boost your workflow:
|
|
@coderabbitai full review, highlight big issues re security, correctness, or performance |
|
@OlegWock: I'll conduct a full review of this PR, focusing specifically on security, correctness, and performance issues as requested. ✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughThis PR adds Vega chart rendering support to the VSCode Deepnote extension. It introduces a new notebook renderer ( Sequence DiagramsequenceDiagram
participant VSCode as VSCode Notebook
participant Converter as DeepnoteDataConverter
participant VizConverter as VisualizationBlockConverter
participant Renderer as VegaRenderer
participant Vega as Vega/Vega-Lite
rect rgb(230, 245, 255)
note right of VSCode: Block to Cell Conversion
VSCode->>Converter: convertCellsToBlocks(cells, index)
Converter->>Converter: convertCellToBlock(cell, index)
Converter->>VizConverter: canConvert(blockType)
alt blockType == 'visualization'
Converter->>VizConverter: applyChangesToBlock(cell, block)
VizConverter->>VizConverter: Parse JSON metadata
VizConverter-->>block: Update metadata (variable, spec, filters)
end
end
rect rgb(240, 255, 240)
note right of Vega: Output Rendering
VSCode->>Renderer: activate(context)
Renderer->>Renderer: Cache DOM roots
Renderer->>Vega: Convert output to spec
Renderer->>Vega: Register custom formatters & color schemes
Vega-->>Renderer: Render visualization
Renderer->>VSCode: Mount in notebook cell
end
rect rgb(255, 245, 230)
note right of VSCode: Cell to Block Conversion
VSCode->>VizConverter: convertToCell(block)
VizConverter-->>VSCode: Python cell with JSON metadata
end
Possibly related PRs
Suggested reviewers
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
2159-2162: Bump React & React-DOM to ^16.14.0
react-vega@7.7.1 declares peerDependencies react/react-dom ^16.8.0 || ^17 || ^18 || ^19; Hooks in VegaRenderer need ≥16.8.0.
Recommended minimal bump:- "react": "^16.5.2", - "react-dom": "^16.5.2", + "react": "^16.14.0", + "react-dom": "^16.14.0",Align types:
- "@types/react": "^16.4.14", - "@types/react-dom": "^16.0.8", + "@types/react": "^16.14.37", + "@types/react-dom": "^16.9.21",Optionally upgrade to React 18+ if feasible.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
build/esbuild/build.ts(1 hunks)package.json(5 hunks)src/kernels/execution/cellExecution.ts(2 hunks)src/notebooks/deepnote/converters/visualizationBlockConverter.ts(1 hunks)src/notebooks/deepnote/converters/visualizationBlockConverter.unit.test.ts(1 hunks)src/notebooks/deepnote/deepnoteDataConverter.ts(5 hunks)src/webviews/webview-side/vega-renderer/VegaRenderer.tsx(1 hunks)src/webviews/webview-side/vega-renderer/colors.ts(1 hunks)src/webviews/webview-side/vega-renderer/index.ts(1 hunks)src/webviews/webview-side/vega-renderer/number-formats.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 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/deepnoteDataConverter.ts : deepnoteDataConverter.ts performs Deepnote data transformations
Applied to files:
src/notebooks/deepnote/deepnoteDataConverter.tssrc/notebooks/deepnote/converters/visualizationBlockConverter.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/export/**/*.ts : Update FileConverter to handle the new export format
Applied to files:
src/notebooks/deepnote/deepnoteDataConverter.tssrc/notebooks/deepnote/converters/visualizationBlockConverter.ts
🧬 Code graph analysis (6)
src/kernels/execution/cellExecution.ts (1)
src/notebooks/deepnote/deepnoteDataConverter.ts (1)
DeepnoteDataConverter(17-403)
src/notebooks/deepnote/deepnoteDataConverter.ts (3)
src/notebooks/deepnote/converters/visualizationBlockConverter.ts (1)
VisualizationBlockConverter(49-97)src/notebooks/deepnote/converters/blockConverter.ts (1)
BlockConverter(5-10)src/notebooks/deepnote/pocket.ts (1)
createBlockFromPocket(48-80)
src/notebooks/deepnote/converters/visualizationBlockConverter.ts (2)
src/notebooks/deepnote/converters/blockConverter.ts (1)
BlockConverter(5-10)src/test/mocks/vsc/extHostedTypes.ts (1)
NotebookCellData(2523-2549)
src/webviews/webview-side/vega-renderer/VegaRenderer.tsx (2)
src/webviews/webview-side/vega-renderer/number-formats.ts (1)
numberFormats(3-17)src/webviews/webview-side/vega-renderer/colors.ts (3)
chartColors10(1-12)chartColors20(15-36)deepnoteBlues(38-38)
src/webviews/webview-side/vega-renderer/index.ts (1)
src/webviews/webview-side/vega-renderer/VegaRenderer.tsx (1)
VegaRenderer(13-48)
src/notebooks/deepnote/converters/visualizationBlockConverter.unit.test.ts (3)
src/notebooks/deepnote/converters/visualizationBlockConverter.ts (1)
VisualizationBlockConverter(49-97)src/notebooks/deepnote/deepnoteTypes.ts (1)
DeepnoteBlock(6-6)src/test/mocks/vsc/extHostedTypes.ts (1)
NotebookCellData(2523-2549)
🔇 Additional comments (11)
src/notebooks/deepnote/deepnoteDataConverter.ts (4)
24-25: Good addition: visualization converter registeredRegistration integrates cleanly with existing registry.
32-34: findConverter API is usefulPublic lookup improves testability and separation of concerns.
80-107: Solid extraction of per‑cell conversionNice reuse of pocket, non‑destructive output handling, and cleanup of internal flags.
289-296: Vega v5 output passthrough to VS Code looks correctJSON item with proper MIME and ordering preserved.
package.json (1)
1822-1830: Renderer contribution is wired correctlyRenderer id, entrypoint, MIME, and messaging settings are consistent with VS Code Notebook renderer expectations.
src/notebooks/deepnote/converters/visualizationBlockConverter.ts (1)
49-97: Converter logic is robustDefaults, parse guards, and round‑trip JSON shape look solid. Good case‑insensitive canConvert.
src/notebooks/deepnote/converters/visualizationBlockConverter.unit.test.ts (1)
1-311: Comprehensive testsCovers happy path, defaults, invalid JSON, and round‑trip. Nice.
build/esbuild/build.ts (1)
332-336: Vega renderer build step added correctlyEntry/source/target align with contributes.notebookRenderer entrypoint.
src/webviews/webview-side/vega-renderer/colors.ts (1)
1-38: Palettes look goodStatic, immutable schemes ready for registration.
src/webviews/webview-side/vega-renderer/VegaRenderer.tsx (1)
13-32: React Hooks usage requires React >=16.8 (see package.json)Component is fine; ensure the React bump lands or this will crash.
src/webviews/webview-side/vega-renderer/index.ts (1)
36-41: No change needed for ReactDOM.render: the project uses React 16.x, so this API is appropriate.
| }; | ||
|
|
||
| const jsonContent = JSON.stringify(config, null, 2); | ||
| const cell = new NotebookCellData(NotebookCellKind.Code, jsonContent, 'python'); |
There was a problem hiding this comment.
Setting Python here makes VSC show linting errors (e.g. false is unknown variable), but setting it as JSON blocks us from intercepting its execution, not sure why
OlegWock
requested review from
andyjakubowski,
jamesbhobbs and
saltenasl
as code owners
saltenasl
left a comment
saltenasl
left a comment
There was a problem hiding this comment.
It seems like the extension doesn't support legacy Vega-Lite outputs if i export an old project. Non-blocking for this PR though.
|
@coderabbitai approve pls |
|
@OlegWock: Approved! Note that this PR is currently marked as draft. Let me know if you need any further reviews. ✅ Actions performedComments resolved and changes approved. |
@coderabbitai ignore
Summary by CodeRabbit
Release Notes