feat: Set up a custom renderer for data frames. #60
Conversation
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
📝 WalkthroughWalkthroughAdds a dataframe notebook renderer and editor-side controller, wires Tailwind/PostCSS into the web build for the renderer, and registers a new notebookRenderer in package.json. Enriches notebook outputs with cellId/cellIndex metadata, changes execution wrapper lifecycle to replace-and-start semantics, and generates Python for execution via @deepnote/blocks createPythonCode. Moves block id to top-level cell metadata and adjusts pocket/convertor logic and tests. Adds localization, a requirements helper interface, unit tests for execution wrappers, and related wiring in service registries. Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Renderer as Webview Renderer\n(DataframeRenderer)
participant Ext as Extension Host\n(DataframeController)
participant Workspace as VS Code Workspace
participant Kernel
rect rgb(245,250,255)
note over Renderer: Render dataframe preview\n(from OutputItem + metadata)
User->>Renderer: change page / pageSize
Renderer->>Ext: postMessage { command, cellId, value }
end
rect rgb(250,245,240)
Ext->>Workspace: find notebook cell by metadata.id (cellId)
Ext->>Workspace: apply WorkspaceEdit (update deepnote_table_state)
Ext->>Workspace: request cell execution
Workspace->>Kernel: execute generated Python (createPythonCode)
Kernel-->>Workspace: outputs (include cellId/cellIndex)
Workspace-->>Renderer: deliver output for renderer MIME
Renderer->>Renderer: re-render with updated state
end
sequenceDiagram
autonumber
participant Cell as NotebookCell
participant Creator as CellExecutionCreator
participant Blocks as @deepnote/blocks
participant Kernel
Cell->>Blocks: createBlockFromPocket(cell metadata)
Blocks-->>Cell: DeepnoteBlock
Cell->>Blocks: createPythonCode(DeepnoteBlock)
Creator->>Creator: ensure start() then clearOutput()
Creator->>Kernel: execute generated code
Kernel-->>Creator: outputs (enriched with cellId/cellIndex)
Creator-->>Cell: update outputs (translated with cell context)
Possibly related PRs
Suggested reviewers
Pre-merge checks✅ Passed checks (3 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: 15
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (27)
.github/workflows/ci.yml(1 hunks)build/esbuild/build.ts(6 hunks)package.json(6 hunks)postcss.config.js(1 hunks)src/kernels/deepnote/deepnoteToolkitInstaller.node.ts(2 hunks)src/kernels/execution/cellExecution.ts(2 hunks)src/kernels/execution/cellExecutionCreator.ts(3 hunks)src/kernels/execution/cellExecutionCreator.unit.test.ts(1 hunks)src/kernels/execution/cellExecutionMessageHandler.ts(4 hunks)src/kernels/execution/helpers.ts(10 hunks)src/notebooks/deepnote/blocks.md(1 hunks)src/notebooks/deepnote/deepnoteDataConverter.ts(9 hunks)src/notebooks/deepnote/deepnoteDataConverter.unit.test.ts(3 hunks)src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts(3 hunks)src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts(16 hunks)src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts(6 hunks)src/notebooks/deepnote/pocket.ts(3 hunks)src/notebooks/deepnote/pocket.unit.test.ts(7 hunks)src/notebooks/deepnote/serialization.md(0 hunks)src/standalone/intellisense/resolveCompletionItem.unit.test.ts(1 hunks)src/webviews/extension-side/dataframe/dataframeController.ts(1 hunks)src/webviews/extension-side/serviceRegistry.node.ts(2 hunks)src/webviews/extension-side/serviceRegistry.web.ts(2 hunks)src/webviews/webview-side/dataframe-renderer/DataframeRenderer.tsx(1 hunks)src/webviews/webview-side/dataframe-renderer/index.ts(1 hunks)src/webviews/webview-side/dataframe-renderer/tailwind.css(1 hunks)tailwind.config.js(1 hunks)
💤 Files with no reviewable changes (1)
- src/notebooks/deepnote/serialization.md
🧰 Additional context used
📓 Path-based instructions (10)
**/*.node.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
*.node.tsfor Desktop-specific implementations that require full file system access and Python environments
Files:
src/notebooks/deepnote/deepnoteInitNotebookRunner.node.tssrc/kernels/deepnote/deepnoteToolkitInstaller.node.tssrc/notebooks/deepnote/deepnoteKernelAutoSelector.node.tssrc/notebooks/deepnote/deepnoteRequirementsHelper.node.tssrc/webviews/extension-side/serviceRegistry.node.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Inject interfaces, not concrete classes
Avoid circular dependencies
Usel10n.t()for user-facing strings
Use typed error classes fromsrc/platform/errors/when throwing or handling errors
Use theILoggerservice instead ofconsole.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 withCancellationToken
**/*.{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/deepnoteInitNotebookRunner.node.tssrc/kernels/execution/cellExecutionMessageHandler.tssrc/kernels/deepnote/deepnoteToolkitInstaller.node.tssrc/notebooks/deepnote/pocket.tssrc/notebooks/deepnote/pocket.unit.test.tssrc/notebooks/deepnote/deepnoteKernelAutoSelector.node.tssrc/standalone/intellisense/resolveCompletionItem.unit.test.tssrc/notebooks/deepnote/deepnoteRequirementsHelper.node.tssrc/kernels/execution/cellExecution.tssrc/notebooks/deepnote/deepnoteDataConverter.unit.test.tsbuild/esbuild/build.tssrc/webviews/extension-side/serviceRegistry.node.tssrc/notebooks/deepnote/deepnoteDataConverter.tssrc/webviews/extension-side/serviceRegistry.web.tssrc/webviews/webview-side/dataframe-renderer/index.tssrc/webviews/extension-side/dataframe/dataframeController.tssrc/webviews/webview-side/dataframe-renderer/DataframeRenderer.tsxsrc/kernels/execution/cellExecutionCreator.unit.test.tssrc/kernels/execution/cellExecutionCreator.tssrc/kernels/execution/helpers.ts
src/notebooks/deepnote/**
📄 CodeRabbit inference engine (CLAUDE.md)
Deepnote integration code resides under src/notebooks/deepnote/
Files:
src/notebooks/deepnote/deepnoteInitNotebookRunner.node.tssrc/notebooks/deepnote/pocket.tssrc/notebooks/deepnote/pocket.unit.test.tssrc/notebooks/deepnote/deepnoteKernelAutoSelector.node.tssrc/notebooks/deepnote/deepnoteRequirementsHelper.node.tssrc/notebooks/deepnote/deepnoteDataConverter.unit.test.tssrc/notebooks/deepnote/deepnoteDataConverter.tssrc/notebooks/deepnote/blocks.md
**/!(*.node|*.web).ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place shared cross-platform logic in common
.tsfiles (not suffixed with.nodeor.web)
Files:
src/kernels/execution/cellExecutionMessageHandler.tssrc/notebooks/deepnote/pocket.tssrc/notebooks/deepnote/pocket.unit.test.tssrc/standalone/intellisense/resolveCompletionItem.unit.test.tssrc/kernels/execution/cellExecution.tssrc/notebooks/deepnote/deepnoteDataConverter.unit.test.tsbuild/esbuild/build.tssrc/notebooks/deepnote/deepnoteDataConverter.tssrc/webviews/webview-side/dataframe-renderer/index.tssrc/webviews/extension-side/dataframe/dataframeController.tssrc/kernels/execution/cellExecutionCreator.unit.test.tssrc/kernels/execution/cellExecutionCreator.tssrc/kernels/execution/helpers.ts
src/kernels/execution/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)
Notebook-level execution must enqueue cells via CellExecutionQueue and manage per-cell lifecycles through CellExecution
Files:
src/kernels/execution/cellExecutionMessageHandler.tssrc/kernels/execution/cellExecution.tssrc/kernels/execution/cellExecutionCreator.unit.test.tssrc/kernels/execution/cellExecutionCreator.tssrc/kernels/execution/helpers.ts
src/kernels/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)
src/kernels/**/*.ts: Use event-driven updates (EventEmitter) for state changes
Monitor and dispose pending promises to prevent leaks during teardown
Use WeakRef/weak references for notebook/object backreferences to avoid memory leaks
Respect CancellationToken in all async operations
Files:
src/kernels/execution/cellExecutionMessageHandler.tssrc/kernels/deepnote/deepnoteToolkitInstaller.node.tssrc/kernels/execution/cellExecution.tssrc/kernels/execution/cellExecutionCreator.unit.test.tssrc/kernels/execution/cellExecutionCreator.tssrc/kernels/execution/helpers.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/pocket.unit.test.tssrc/standalone/intellisense/resolveCompletionItem.unit.test.tssrc/notebooks/deepnote/deepnoteDataConverter.unit.test.tssrc/kernels/execution/cellExecutionCreator.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
thenproperty to avoid hanging tests
Files:
src/notebooks/deepnote/pocket.unit.test.tssrc/standalone/intellisense/resolveCompletionItem.unit.test.tssrc/notebooks/deepnote/deepnoteDataConverter.unit.test.tssrc/kernels/execution/cellExecutionCreator.unit.test.ts
src/notebooks/deepnote/deepnoteDataConverter.ts
📄 CodeRabbit inference engine (CLAUDE.md)
deepnoteDataConverter.ts performs Deepnote data transformations
Files:
src/notebooks/deepnote/deepnoteDataConverter.ts
**/*.web.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
*.web.tsfor Web-specific implementations using browser-compatible APIs
Files:
src/webviews/extension-side/serviceRegistry.web.ts
🧠 Learnings (10)
📚 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/deepnoteInitNotebookRunner.node.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/deepnoteInitNotebookRunner.node.tssrc/kernels/deepnote/deepnoteToolkitInstaller.node.tssrc/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-03T12:58:18.091Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel.instructions.md:0-0
Timestamp: 2025-09-03T12:58:18.091Z
Learning: Applies to src/kernels/execution/CellExecutionMessageHandler.ts : CellExecutionMessageHandler must process kernel messages (iopub, execute_reply) and update VS Code cell outputs
Applied to files:
src/kernels/execution/cellExecutionMessageHandler.ts
📚 Learning: 2025-10-07T19:51:40.201Z
Learnt from: m1so
PR: deepnote/vscode-deepnote#25
File: src/kernels/deepnote/deepnoteServerStarter.node.ts:150-154
Timestamp: 2025-10-07T19:51:40.201Z
Learning: The deepnote-toolkit server is a separate Python package maintained in a different repository from the VS Code extension (vscode-deepnote). The VS Code extension launches the toolkit as a subprocess and communicates via environment variables and HTTP. The toolkit implementation cannot be verified from the extension codebase.
Applied to files:
src/kernels/deepnote/deepnoteToolkitInstaller.node.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/deepnoteTypes.ts : deepnoteTypes.ts contains Deepnote-related type definitions
Applied to files:
src/notebooks/deepnote/pocket.ts
📚 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/{interactiveWindow.ts,interactiveWindowController.ts} : Implement robust kernel error handling: automatic restart, clear error display in system info cells, graceful degradation when kernels are unavailable, and user notifications for critical failures
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-09-03T12:55:29.175Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/ipywidgets.instructions.md:0-0
Timestamp: 2025-09-03T12:55:29.175Z
Learning: Applies to src/notebooks/controllers/ipywidgets/serviceRegistry.{node,web}.ts : Register DI services in platform-specific registries (serviceRegistry.node.ts vs serviceRegistry.web.ts)
Applied to files:
src/webviews/extension-side/serviceRegistry.node.ts
📚 Learning: 2025-09-03T12:55:29.175Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/ipywidgets.instructions.md:0-0
Timestamp: 2025-09-03T12:55:29.175Z
Learning: Applies to src/notebooks/controllers/ipywidgets/types.ts : Use the shared core interfaces/types from types.ts across the IPyWidgets controller code
Applied to files:
src/webviews/extension-side/serviceRegistry.node.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/deepnoteDataConverter.ts : deepnoteDataConverter.ts performs Deepnote data transformations
Applied to files:
src/notebooks/deepnote/deepnoteDataConverter.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.ts
🧬 Code graph analysis (14)
src/kernels/execution/cellExecutionMessageHandler.ts (1)
src/kernels/execution/helpers.ts (1)
cellOutputToVSCCellOutput(137-176)
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (1)
src/kernels/deepnote/types.ts (1)
DEEPNOTE_TOOLKIT_VERSION(157-157)
src/notebooks/deepnote/pocket.ts (1)
src/notebooks/deepnote/dataConversionUtils.ts (1)
generateBlockId(8-15)
src/notebooks/deepnote/pocket.unit.test.ts (1)
src/notebooks/deepnote/pocket.ts (2)
extractPocketFromCellMetadata(44-46)createBlockFromPocket(48-80)
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts (1)
src/platform/logging/index.ts (1)
logger(35-48)
src/standalone/intellisense/resolveCompletionItem.unit.test.ts (1)
src/kernels/execution/helpers.ts (1)
cellOutputToVSCCellOutput(137-176)
src/kernels/execution/cellExecution.ts (1)
src/notebooks/deepnote/pocket.ts (1)
createBlockFromPocket(48-80)
src/webviews/extension-side/serviceRegistry.node.ts (1)
src/platform/activation/types.ts (2)
IExtensionSyncActivationService(15-15)IExtensionSyncActivationService(22-24)
src/notebooks/deepnote/deepnoteDataConverter.ts (3)
src/notebooks/deepnote/pocket.ts (1)
addPocketToCellMetadata(20-42)src/notebooks/deepnote/deepnoteTypes.ts (1)
DeepnoteOutput(23-51)src/test/mocks/vsc/extHostedTypes.ts (2)
NotebookCellOutput(71-103)NotebookCellOutputItem(16-69)
src/webviews/extension-side/serviceRegistry.web.ts (1)
src/platform/activation/types.ts (2)
IExtensionSyncActivationService(15-15)IExtensionSyncActivationService(22-24)
src/webviews/webview-side/dataframe-renderer/index.ts (1)
src/webviews/webview-side/dataframe-renderer/DataframeRenderer.tsx (2)
DataframeMetadata(6-8)DataframeRenderer(54-210)
src/webviews/extension-side/dataframe/dataframeController.ts (1)
src/platform/common/types.ts (1)
IDisposable(211-213)
src/kernels/execution/cellExecutionCreator.ts (3)
src/platform/logging/index.ts (1)
logger(35-48)src/platform/common/utils/misc.ts (1)
noop(11-11)src/notebooks/controllers/notebookIPyWidgetCoordinator.ts (1)
controller(27-32)
src/kernels/execution/helpers.ts (1)
src/test/mocks/vsc/extHostedTypes.ts (2)
NotebookCellOutput(71-103)NotebookCellOutputItem(16-69)
🪛 markdownlint-cli2 (0.18.1)
src/notebooks/deepnote/blocks.md
15-15: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (2)
- GitHub Check: Build & Test
- GitHub Check: Lint & Format
🔇 Additional comments (38)
src/standalone/intellisense/resolveCompletionItem.unit.test.ts (1)
549-549: Correct fix: prevents unintended arguments.Previously, passing
cellOutputToVSCCellOutputdirectly tomapcaused it to receive the array index ascellIndexand the array itself ascellId. The arrow wrapper ensures only the output is passed, leaving optional metadata params correctly undefined.src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (1)
7-7: LGTM: version isolation is correct.Including
DEEPNOTE_TOOLKIT_VERSIONin the venv path ensures clean isolation when the toolkit is upgraded.Also applies to: 32-36
.github/workflows/ci.yml (1)
96-99: LGTM!Permission needed for accessing
@deepnote/blocksfrom GitHub Package Registry.src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts (4)
15-16: LGTM!Imports support localization and lifecycle management.
131-134: LGTM!Localized error message and proper error parameter passing.
259-537: LGTM!Comprehensive localization of user-facing progress messages.
415-424: LGTM!Localized warnings with proper parameter interpolation.
src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts (3)
2-13: LGTM!Type-only imports optimize bundle size.
145-152: LGTM!Progress titles properly localized.
245-245: LGTM!Progress calculation fix: uses
(i+1)to show 100% on final block instead of stopping short.package.json (3)
1812-1822: LGTM!Dataframe renderer configuration is properly structured.
2113-2113: LGTM!Minor version bump for
@deepnote/blocks.
2183-2299: LGTM!Tailwind/PostCSS toolchain dependencies properly added.
src/webviews/webview-side/dataframe-renderer/DataframeRenderer.tsx (3)
1-8: LGTM!Clean imports with proper React optimization patterns.
10-53: LGTM!Type definitions are comprehensive and clear.
54-78: LGTM!State initialization and column filtering logic are sound.
src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts (4)
11-14: LGTM!Interface formalization enables proper DI and follows coding guidelines.
44-57: LGTM!Input normalization is thorough and handles edge cases properly.
72-90: LGTM!Proper use of
Uri.joinPathand line-ending normalization prevents cross-platform issues.
98-148: LGTM!User preference flow is well-implemented with proper state persistence.
src/webviews/extension-side/serviceRegistry.node.ts (2)
20-20: LGTM!Import follows established patterns.
69-69: LGTM!Registration follows DI patterns and aligns with learnings about platform-specific service registries.
Based on learnings.
src/webviews/webview-side/dataframe-renderer/tailwind.css (1)
1-3: LGTM!Standard Tailwind CSS setup with base, components, and utilities directives. Integrates correctly with the PostCSS pipeline configured in
postcss.config.jsand build setup.postcss.config.js (1)
1-6: LGTM!Standard PostCSS configuration enabling Tailwind CSS processing and vendor prefixing. Aligns with the build pipeline changes in
build/esbuild/build.ts.src/kernels/execution/cellExecution.ts (1)
411-423: Verify that code replacement is the intended behavior.The original cell code is replaced with code generated by
createPythonCode(deepnoteBlock). Users might expect their exact code to execute. Ensure this transformation is intentional and that it's surfaced to users (e.g., via logging, documentation, or UI feedback) if the generated code differs significantly from the input.The logging at line 422 provides visibility, but consider whether additional user-facing feedback is needed, especially during development when users might be debugging their cell code.
tailwind.config.js (1)
1-24: LGTM!Tailwind configuration correctly targets the dataframe renderer and maps VSCode CSS variables to Tailwind tokens. Disabling
preflight(line 22) is essential to prevent conflicts with VSCode's native styling.src/notebooks/deepnote/pocket.ts (1)
48-79: LGTM!The refactoring correctly moves
idfrom the pocket to top-level metadata. The logic is clear:
- Extract
idfrommetadata.id(line 53)- Clean it from metadata (line 59)
- Use it for
block.id(line 69)Comments (lines 52, 58) effectively document the rationale. The fallback to
generateBlockId()ensures blocks always have an ID.src/notebooks/deepnote/deepnoteDataConverter.unit.test.ts (1)
33-34: LGTM!Test expectations correctly updated to verify
idis stored atmetadata?.idrather than in the pocket. Consistent with the refactoring insrc/notebooks/deepnote/pocket.ts.Also applies to: 57-58, 116-116
src/webviews/extension-side/serviceRegistry.web.ts (1)
30-30: LGTM!Standard service registration following the existing pattern.
DataframeControlleris correctly registered as anIExtensionSyncActivationServicesingleton.Also applies to: 69-69
src/notebooks/deepnote/pocket.unit.test.ts (1)
21-27: LGTM!Tests comprehensively verify the new
idhandling:
idremains at top-levelmetadata.idafter pocket operations- Extracted pocket does not contain
id- Block metadata has
idremoved during conversionConsistent with the refactoring in
src/notebooks/deepnote/pocket.ts.Also applies to: 61-65, 85-90, 163-163
src/kernels/execution/cellExecutionMessageHandler.ts (1)
645-649: Good: propagate cell context into outputs.Passing
cellIndex,cellId, andcellMetadataaligns output metadata with renderer needs.src/kernels/execution/cellExecutionCreator.ts (1)
4-11: Type‑only imports: LGTM.Reduces emitted JS and clarifies intent.
build/esbuild/build.ts (2)
196-199: Targeted Tailwind enablement: LGTM.Scoped to dataframe renderer entrypoint.
327-331: New dataframe renderer build: LGTM.Output path and watch config look consistent with web bundles.
src/kernels/execution/cellExecutionCreator.unit.test.ts (4)
33-63: Call-order assertion: LGTM.Validates start-before-clear semantics.
64-77: Negative path for clear on start: LGTM.Ensures no clear when disabled.
118-154: Idempotent start: LGTM.Ensures single invocation.
209-293: Re-execution behavior: LGTM with new replacement semantics.Covers end/replace/start flow.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #60 +/- ##
======================================
Coverage 70% 70%
======================================
Files 512 513 +1
Lines 37506 37701 +195
Branches 4811 4822 +11
======================================
+ Hits 26473 26648 +175
- Misses 9441 9456 +15
- Partials 1592 1597 +5
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/kernels/execution/cellExecutionCreator.ts (1)
30-37: Update the flag descriptionThe flag name/comment still implies clearing only happens when
startTimeis provided, but the implementation always clears. Please adjust the doc comment (or rename the flag) so the contract matches behavior.- * @param {boolean} [clearOutputOnStartWithTime=false] If true, clear the output when start is called with a time. + * @param {boolean} [clearOutputOnStartWithTime=false] If true, clear outputs immediately after every start().
♻️ Duplicate comments (2)
src/webviews/extension-side/dataframe/dataframeController.ts (1)
49-52: Dispose the renderer messaging disposable.
createRendererMessagingreturns a disposable; only the subscription is tracked. Without pushingcommsintothis.disposables, the channel leaks after deactivation. Add it to the bucket right after creation.public activate() { const comms = notebooks.createRendererMessaging('deepnote-dataframe-renderer'); + this.disposables.push(comms); const messageDisposable = comms.onDidReceiveMessage(this.onDidReceiveMessage.bind(this, comms), this); this.disposables.push(messageDisposable); }src/kernels/execution/cellExecutionMessageHandler.ts (1)
86-88: Drop URI fallback fromgetCellIdFalling back to
cell.document.uriexposes full workspace paths to the renderer and breaks any code that relies on Deepnote’s metadata IDs. Returnundefinedwhen no metadata id exists and make call sites tolerate the absent id instead of leaking URIs.-function getCellId(cell: NotebookCell): string { - return (cell.metadata?.id as string | undefined) || cell.document.uri.toString(); -} +function getCellId(cell: NotebookCell): string | undefined { + const id = cell.metadata?.id; + return typeof id === 'string' ? id : undefined; +}Callers should bail out or skip metadata enrichment when this returns
undefined.
As per coding guidelines
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
src/kernels/execution/cellExecutionCreator.ts(3 hunks)src/kernels/execution/cellExecutionCreator.unit.test.ts(1 hunks)src/kernels/execution/cellExecutionMessageHandler.ts(5 hunks)src/notebooks/deepnote/blocks.md(1 hunks)src/notebooks/deepnote/deepnoteDataConverter.ts(9 hunks)src/webviews/extension-side/dataframe/dataframeController.ts(1 hunks)src/webviews/webview-side/dataframe-renderer/DataframeRenderer.tsx(1 hunks)src/webviews/webview-side/dataframe-renderer/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Inject interfaces, not concrete classes
Avoid circular dependencies
Usel10n.t()for user-facing strings
Use typed error classes fromsrc/platform/errors/when throwing or handling errors
Use theILoggerservice instead ofconsole.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 withCancellationToken
**/*.{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/webviews/webview-side/dataframe-renderer/DataframeRenderer.tsxsrc/notebooks/deepnote/deepnoteDataConverter.tssrc/kernels/execution/cellExecutionCreator.tssrc/webviews/webview-side/dataframe-renderer/index.tssrc/kernels/execution/cellExecutionCreator.unit.test.tssrc/kernels/execution/cellExecutionMessageHandler.tssrc/webviews/extension-side/dataframe/dataframeController.ts
src/notebooks/deepnote/**
📄 CodeRabbit inference engine (CLAUDE.md)
Deepnote integration code resides under src/notebooks/deepnote/
Files:
src/notebooks/deepnote/blocks.mdsrc/notebooks/deepnote/deepnoteDataConverter.ts
**/!(*.node|*.web).ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place shared cross-platform logic in common
.tsfiles (not suffixed with.nodeor.web)
Files:
src/notebooks/deepnote/deepnoteDataConverter.tssrc/kernels/execution/cellExecutionCreator.tssrc/webviews/webview-side/dataframe-renderer/index.tssrc/kernels/execution/cellExecutionCreator.unit.test.tssrc/kernels/execution/cellExecutionMessageHandler.tssrc/webviews/extension-side/dataframe/dataframeController.ts
src/notebooks/deepnote/deepnoteDataConverter.ts
📄 CodeRabbit inference engine (CLAUDE.md)
deepnoteDataConverter.ts performs Deepnote data transformations
Files:
src/notebooks/deepnote/deepnoteDataConverter.ts
src/kernels/execution/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)
Notebook-level execution must enqueue cells via CellExecutionQueue and manage per-cell lifecycles through CellExecution
Files:
src/kernels/execution/cellExecutionCreator.tssrc/kernels/execution/cellExecutionCreator.unit.test.tssrc/kernels/execution/cellExecutionMessageHandler.ts
src/kernels/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)
src/kernels/**/*.ts: Use event-driven updates (EventEmitter) for state changes
Monitor and dispose pending promises to prevent leaks during teardown
Use WeakRef/weak references for notebook/object backreferences to avoid memory leaks
Respect CancellationToken in all async operations
Files:
src/kernels/execution/cellExecutionCreator.tssrc/kernels/execution/cellExecutionCreator.unit.test.tssrc/kernels/execution/cellExecutionMessageHandler.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/kernels/execution/cellExecutionCreator.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
thenproperty to avoid hanging tests
Files:
src/kernels/execution/cellExecutionCreator.unit.test.ts
🧠 Learnings (7)
📚 Learning: 2025-10-15T07:00:04.988Z
Learnt from: Artmann
PR: deepnote/vscode-deepnote#60
File: src/webviews/webview-side/dataframe-renderer/DataframeRenderer.tsx:80-112
Timestamp: 2025-10-15T07:00:04.988Z
Learning: In the vscode-deepnote project, console.log statements are acceptable in webview code (files under src/webviews/webview-side/).
Applied to files:
src/webviews/webview-side/dataframe-renderer/DataframeRenderer.tsxsrc/webviews/webview-side/dataframe-renderer/index.ts
📚 Learning: 2025-10-14T14:15:55.626Z
Learnt from: Artmann
PR: deepnote/vscode-deepnote#28
File: src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts:93-99
Timestamp: 2025-10-14T14:15:55.626Z
Learning: For user Artmann in the deepnote/vscode-deepnote repository: avoid making nitpick or trivial-level review comments. Focus only on substantial issues like bugs, security concerns, performance problems, and significant architectural or design issues.
Applied to files:
src/webviews/webview-side/dataframe-renderer/DataframeRenderer.tsx
📚 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.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.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 **/*.unit.test.ts : Use assert.deepStrictEqual() for object comparisons in tests instead of checking individual properties
Applied to files:
src/kernels/execution/cellExecutionCreator.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 **/*.unit.test.ts : Unit tests must use Mocha/Chai and have the .unit.test.ts extension
Applied to files:
src/kernels/execution/cellExecutionCreator.unit.test.ts
📚 Learning: 2025-09-03T12:58:18.091Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel.instructions.md:0-0
Timestamp: 2025-09-03T12:58:18.091Z
Learning: Applies to src/kernels/execution/CellExecutionMessageHandler.ts : CellExecutionMessageHandler must process kernel messages (iopub, execute_reply) and update VS Code cell outputs
Applied to files:
src/kernels/execution/cellExecutionMessageHandler.ts
🧬 Code graph analysis (5)
src/notebooks/deepnote/deepnoteDataConverter.ts (5)
src/notebooks/deepnote/pocket.ts (1)
addPocketToCellMetadata(20-42)src/kernels/execution/cellExecutionCreator.ts (1)
cell(38-40)src/notebooks/deepnote/deepnoteSerializer.ts (1)
DeepnoteOutput(9-9)src/notebooks/deepnote/deepnoteTypes.ts (1)
DeepnoteOutput(23-51)src/test/mocks/vsc/extHostedTypes.ts (2)
NotebookCellOutput(71-103)NotebookCellOutputItem(16-69)
src/kernels/execution/cellExecutionCreator.ts (2)
src/platform/logging/index.ts (1)
logger(35-48)src/platform/common/utils/misc.ts (1)
noop(11-11)
src/webviews/webview-side/dataframe-renderer/index.ts (1)
src/webviews/webview-side/dataframe-renderer/DataframeRenderer.tsx (2)
DataframeMetadata(7-9)DataframeRenderer(55-213)
src/kernels/execution/cellExecutionMessageHandler.ts (2)
src/kernels/execution/helpers.ts (1)
cellOutputToVSCCellOutput(137-176)src/test/mocks/vsc/extHostedTypes.ts (1)
NotebookCellOutput(71-103)
src/webviews/extension-side/dataframe/dataframeController.ts (2)
src/platform/common/types.ts (1)
IDisposable(211-213)src/test/mocks/vsc/extHostedTypes.ts (2)
WorkspaceEdit(649-833)NotebookEdit(2444-2507)
🔇 Additional comments (1)
src/webviews/webview-side/dataframe-renderer/DataframeRenderer.tsx (1)
81-113: Receiver already handles missing cellId
DataframeController’s handlers guard on!message.cellId, show an error, and throw. No further changes needed.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/notebooks/deepnote/deepnoteDataConverter.ts(9 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
.tsfiles (not suffixed with.nodeor.web)
Files:
src/notebooks/deepnote/deepnoteDataConverter.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Inject interfaces, not concrete classes
Avoid circular dependencies
Usel10n.t()for user-facing strings
Use typed error classes fromsrc/platform/errors/when throwing or handling errors
Use theILoggerservice instead ofconsole.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 withCancellationToken
**/*.{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
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
🧠 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.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.ts
🧬 Code graph analysis (1)
src/notebooks/deepnote/deepnoteDataConverter.ts (4)
src/notebooks/deepnote/pocket.ts (1)
addPocketToCellMetadata(20-42)src/notebooks/deepnote/deepnoteSerializer.ts (1)
DeepnoteOutput(9-9)src/notebooks/deepnote/deepnoteTypes.ts (1)
DeepnoteOutput(23-51)src/test/mocks/vsc/extHostedTypes.ts (2)
NotebookCellOutput(71-103)NotebookCellOutputItem(16-69)
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/notebooks/deepnote/deepnoteDataConverter.ts (2)
36-36: Replaceconsole.warnwithILogger.Per coding guidelines, use the
ILoggerservice instead ofconsole.warn.- console.warn(`Unknown block type: ${block.type}, converting to markdown`);Inject
ILoggervia constructor and log appropriately.
105-114: Guardatobagainst malformed base64.If Deepnote image outputs contain invalid base64,
atobthrows an unhandled exception that breaks notebook rendering.private base64ToUint8Array(base64: string): Uint8Array { + try { const binaryString = atob(base64); const bytes = new Uint8Array(binaryString.length); for (let i = 0; i < binaryString.length; i++) { bytes[i] = binaryString.charCodeAt(i); } return bytes; + } catch { + // Return empty array for malformed base64 + return new Uint8Array(0); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/notebooks/deepnote/deepnoteDataConverter.ts(9 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
.tsfiles (not suffixed with.nodeor.web)
Files:
src/notebooks/deepnote/deepnoteDataConverter.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Inject interfaces, not concrete classes
Avoid circular dependencies
Usel10n.t()for user-facing strings
Use typed error classes fromsrc/platform/errors/when throwing or handling errors
Use theILoggerservice instead ofconsole.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 withCancellationToken
**/*.{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
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
🧠 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.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.ts
🧬 Code graph analysis (1)
src/notebooks/deepnote/deepnoteDataConverter.ts (5)
src/notebooks/deepnote/pocket.ts (1)
addPocketToCellMetadata(20-42)src/kernels/execution/cellExecutionCreator.ts (1)
cell(38-40)src/notebooks/deepnote/deepnoteSerializer.ts (1)
DeepnoteOutput(9-9)src/notebooks/deepnote/deepnoteTypes.ts (1)
DeepnoteOutput(23-51)src/test/mocks/vsc/extHostedTypes.ts (2)
NotebookCellOutput(71-103)NotebookCellOutputItem(16-69)
Artmann
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.
Works great! Thanks for also making the datatable accessible!
This PR creates a paginated data frame similar to the one on deepnote.com.
blocks.mdthat explains how blocks work in the extension.Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores