feat: Implement deepnote big number chart support and renderer #41
Conversation
…tomaskislan/grn-4762-support-big-number-blocks
📝 WalkthroughWalkthroughThis PR introduces a new Deepnote chart big-number renderer for VSCode notebooks. It adds a ChartBigNumberBlockConverter to handle bidirectional conversion between Deepnote big-number blocks and VSCode cells, defines Zod schemas for validation, creates React webview components for rendering, and adds corresponding build configuration and package.json registry entry. The PR also refactors type imports across the codebase, moving DeepnoteBlock type resolution from local modules to a centralized platform location, and integrates big-number output handling into cell execution for MIME type remapping. Sequence Diagram(s)sequenceDiagram
participant Deepnote Block
participant ChartBigNumberBlockConverter
participant ZodSchema
participant VSCode Cell
rect rgb(200, 220, 240)
note over Deepnote Block,VSCode Cell: convertToCell (Deepnote → VSCode)
Deepnote Block->>ChartBigNumberBlockConverter: pass block with metadata
ChartBigNumberBlockConverter->>ZodSchema: validate metadata
ZodSchema-->>ChartBigNumberBlockConverter: validation result
ChartBigNumberBlockConverter->>ChartBigNumberBlockConverter: serialize to JSON config
ChartBigNumberBlockConverter-->>VSCode Cell: return JSON NotebookCellData
end
rect rgb(220, 240, 200)
note over Deepnote Block,VSCode Cell: applyChangesToBlock (VSCode → Deepnote)
VSCode Cell->>ChartBigNumberBlockConverter: pass cell with JSON value
ChartBigNumberBlockConverter->>ChartBigNumberBlockConverter: parse JSON value
ChartBigNumberBlockConverter->>ZodSchema: validate parsed data
alt valid
ZodSchema-->>ChartBigNumberBlockConverter: ✓ passes validation
ChartBigNumberBlockConverter->>Deepnote Block: merge into metadata
else invalid
ZodSchema-->>ChartBigNumberBlockConverter: ✗ fails validation
ChartBigNumberBlockConverter->>Deepnote Block: store as raw content
end
end
sequenceDiagram
participant Webview
participant ChartBigNumberOutputRendererContainer
participant ZodSchema
participant ChartBigNumberOutputRenderer
participant Browser
rect rgb(200, 220, 240)
note over Webview,Browser: Rendering Output in Webview
Webview->>ChartBigNumberOutputRendererContainer: pass outputText & outputMetadata
ChartBigNumberOutputRendererContainer->>ChartBigNumberOutputRendererContainer: parse JSON from text
ChartBigNumberOutputRendererContainer->>ZodSchema: validate output & metadata
alt valid
ZodSchema-->>ChartBigNumberOutputRendererContainer: ✓ validation passes
ChartBigNumberOutputRendererContainer->>ChartBigNumberOutputRenderer: render with parsed data
ChartBigNumberOutputRenderer->>ChartBigNumberOutputRenderer: format values using formatValue()
ChartBigNumberOutputRenderer->>Browser: display big-number with comparison
else invalid
ZodSchema-->>ChartBigNumberOutputRendererContainer: ✗ validation fails
ChartBigNumberOutputRendererContainer->>Browser: error boundary fallback
end
end
Possibly related PRs
Suggested reviewers
Pre-merge checks✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (3)**/*.node.ts📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
src/notebooks/deepnote/**📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-10-09T11:21:57.494ZApplied to files:
📚 Learning: 2025-10-09T11:21:57.494ZApplied to files:
⏰ 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). (3)
🔇 Additional comments (1)
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: 14
📜 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 (12)
build/esbuild/build.ts(1 hunks)package.json(2 hunks)src/notebooks/deepnote/converters/chartBigNumberBlockConverter.ts(1 hunks)src/notebooks/deepnote/dataConversionUtils.ts(1 hunks)src/notebooks/deepnote/deepnoteConstants.ts(1 hunks)src/notebooks/deepnote/deepnoteDataConverter.ts(5 hunks)src/notebooks/deepnote/deepnoteSchemas.ts(1 hunks)src/notebooks/deepnote/deepnoteSerializer.ts(2 hunks)src/webviews/deepnote-utils/format-value.ts(1 hunks)src/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.tsx(1 hunks)src/webviews/webview-side/chart-big-number-renderer/index.ts(1 hunks)src/webviews/webview-side/chart-big-number-renderer/styles.css(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
.tsfiles (not suffixed with.nodeor.web)
Files:
build/esbuild/build.tssrc/webviews/deepnote-utils/format-value.tssrc/notebooks/deepnote/deepnoteSchemas.tssrc/notebooks/deepnote/deepnoteConstants.tssrc/notebooks/deepnote/dataConversionUtils.tssrc/webviews/webview-side/chart-big-number-renderer/index.tssrc/notebooks/deepnote/deepnoteSerializer.tssrc/notebooks/deepnote/deepnoteDataConverter.tssrc/notebooks/deepnote/converters/chartBigNumberBlockConverter.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:
build/esbuild/build.tssrc/webviews/deepnote-utils/format-value.tssrc/notebooks/deepnote/deepnoteSchemas.tssrc/notebooks/deepnote/deepnoteConstants.tssrc/notebooks/deepnote/dataConversionUtils.tssrc/webviews/webview-side/chart-big-number-renderer/index.tssrc/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.tsxsrc/notebooks/deepnote/deepnoteSerializer.tssrc/notebooks/deepnote/deepnoteDataConverter.tssrc/notebooks/deepnote/converters/chartBigNumberBlockConverter.ts
src/notebooks/deepnote/**
📄 CodeRabbit inference engine (CLAUDE.md)
Deepnote integration code resides under src/notebooks/deepnote/
Files:
src/notebooks/deepnote/deepnoteSchemas.tssrc/notebooks/deepnote/deepnoteConstants.tssrc/notebooks/deepnote/dataConversionUtils.tssrc/notebooks/deepnote/deepnoteSerializer.tssrc/notebooks/deepnote/deepnoteDataConverter.tssrc/notebooks/deepnote/converters/chartBigNumberBlockConverter.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
src/notebooks/deepnote/deepnoteDataConverter.ts
📄 CodeRabbit inference engine (CLAUDE.md)
deepnoteDataConverter.ts performs Deepnote data transformations
Files:
src/notebooks/deepnote/deepnoteDataConverter.ts
🧠 Learnings (6)
📚 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/deepnoteSchemas.tssrc/notebooks/deepnote/deepnoteSerializer.tssrc/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 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/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/deepnoteActivationService.ts : deepnoteActivationService.ts manages VS Code activation for Deepnote
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/deepnoteDataConverter.ts : deepnoteDataConverter.ts performs Deepnote data transformations
Applied to files:
src/notebooks/deepnote/deepnoteDataConverter.tssrc/notebooks/deepnote/converters/chartBigNumberBlockConverter.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/chartBigNumberBlockConverter.ts
🧬 Code graph analysis (5)
build/esbuild/build.ts (5)
gulpfile.js (1)
path(14-14)build/preLaunchWebTest.js (1)
path(4-4)build/launchWebUtils.js (1)
path(4-4)build/ci/postInstall.js (1)
path(8-8)build/webpack/common.js (1)
path(6-6)
src/webviews/webview-side/chart-big-number-renderer/index.ts (2)
src/notebooks/deepnote/deepnoteSchemas.ts (2)
DeepnoteBigNumberMetadataSchema(11-44)DeepnoteChartBigNumberOutputSchema(3-9)src/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.tsx (1)
ChartBigNumberOutputRenderer(5-145)
src/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.tsx (2)
src/notebooks/deepnote/deepnoteSchemas.ts (2)
DeepnoteChartBigNumberOutput(46-46)DeepnoteBigNumberMetadata(47-47)src/webviews/deepnote-utils/format-value.ts (1)
formatValue(1-36)
src/notebooks/deepnote/deepnoteDataConverter.ts (3)
src/notebooks/deepnote/converters/chartBigNumberBlockConverter.ts (1)
ChartBigNumberBlockConverter(11-64)src/notebooks/deepnote/deepnoteTypes.ts (2)
DeepnoteBlock(6-6)DeepnoteOutput(23-51)src/notebooks/deepnote/deepnoteConstants.ts (1)
CHART_BIG_NUMBER_MIME_TYPE(1-1)
src/notebooks/deepnote/converters/chartBigNumberBlockConverter.ts (5)
src/notebooks/deepnote/converters/blockConverter.ts (1)
BlockConverter(5-10)src/notebooks/deepnote/deepnoteSerializer.ts (1)
DeepnoteBlock(9-9)src/test/mocks/vsc/extHostedTypes.ts (1)
NotebookCellData(2523-2549)src/notebooks/deepnote/deepnoteSchemas.ts (1)
DeepnoteBigNumberMetadataSchema(11-44)src/notebooks/deepnote/dataConversionUtils.ts (1)
parseJsonWithFallback(5-11)
🔇 Additional comments (14)
package.json (2)
1822-1829: LGTM: Renderer configuration is correct.The notebookRenderer entry properly registers the big-number chart renderer with the correct MIME type and entrypoint.
2182-2183: Confirm zod@^4.1.12 availability
npm’s main registry still shows 4.1.11 as the latest stable; if 4.1.12 isn’t published in your registry, change to ^4.1.11.build/esbuild/build.ts (1)
295-306: LGTM: Build configuration follows established pattern.The chart-big-number-renderer build step correctly mirrors the dataframe-renderer configuration above it.
src/notebooks/deepnote/dataConversionUtils.ts (1)
5-11: LGTM: Simple, correct JSON parsing utility.The function appropriately handles parsing failures with a fallback mechanism.
src/notebooks/deepnote/deepnoteSerializer.ts (1)
1-9: LGTM: Export surface updated to include DeepnoteBlock and DeepnoteFile.The changes expose necessary types while removing DeepnoteProject from public exports.
src/webviews/webview-side/chart-big-number-renderer/styles.css (1)
1-54: LGTM: CSS properly uses VS Code theme variables.The styles correctly reference VS Code's theming system and provide appropriate layout for big-number charts.
src/notebooks/deepnote/deepnoteConstants.ts (1)
1-1: LGTM: MIME type constant follows conventions.The constant correctly defines the custom MIME type for Deepnote big-number charts.
src/notebooks/deepnote/deepnoteDataConverter.ts (3)
10-11: LGTM: Converter registration follows established pattern.The ChartBigNumberBlockConverter is properly imported and registered in the constructor.
Also applies to: 24-24
60-60: LGTM: Method signature updated to support block-type-aware output transformation.Passing blockType enables MIME type selection based on block type, which is necessary for big-number rendering.
Also applies to: 208-211
277-293: Guard against duplicate big-number outputs
ForblockType === 'big-number}, both thedata['text/plain']fallback (overriding to CHART_BIG_NUMBER_MIME_TYPE) and the explicitdata[CHART_BIG_NUMBER_MIME_TYPE]branch will fire if both keys are present, producing duplicate items. Restrict the fallback to non-big-number blocks or guard the CHART_BIG_NUMBER_MIME_TYPE branch byblockType, or confirm that Jupyter never emits both.src/notebooks/deepnote/deepnoteSchemas.ts (1)
11-44: LGTM!Metadata schema correctly normalizes all optional fields to
null, provides appropriate enum constraints for comparison type, and handles all expected configuration options.src/notebooks/deepnote/converters/chartBigNumberBlockConverter.ts (3)
35-37: LGTM!Case-insensitive block type matching is appropriate.
61-63: LGTM!Correctly returns supported types.
13-13: Unconditional clearing ofblock.contentis intentional. chartBigNumberBlockConverter uses structured properties for parsed JSON and only assignsblock.contentfromcell.valueon parse failure.
src/notebooks/deepnote/converters/chartBigNumberBlockConverter.ts
Outdated
Show resolved
Hide resolved
src/notebooks/deepnote/converters/chartBigNumberBlockConverter.ts
Outdated
Show resolved
Hide resolved
src/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.tsx
Outdated
Show resolved
Hide resolved
src/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.tsx
Outdated
Show resolved
Hide resolved
|
The comparison type supports a few different options, such as |
|
Even if the comparison option is turned off, it's displayed as a |
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (5)
src/notebooks/deepnote/converters/chartBigNumberBlockConverter.ts (2)
45-46: Use ILogger instead of console methods.Per coding guidelines, use the
ILoggerservice for logging.As per coding guidelines.
Also applies to: 56-56
10-10: Pre-stringify the default config constant.The constant addresses the past review but isn't fully optimized. Pre-stringify it with formatting to avoid repeated
JSON.stringifycalls at line 53.Apply this diff:
-const DEFAULT_BIG_NUMBER_CONFIG = DeepnoteBigNumberMetadataSchema.parse({}); +const DEFAULT_BIG_NUMBER_CONFIG = JSON.stringify(DeepnoteBigNumberMetadataSchema.parse({}), null, 2);Then update line 53:
- : JSON.stringify(DEFAULT_BIG_NUMBER_CONFIG); + : DEFAULT_BIG_NUMBER_CONFIG;src/webviews/webview-side/chart-big-number-renderer/index.ts (1)
31-37: Use ILogger or forward to host; avoid console.*.Replace console.error with ILogger or post a message to the extension host.
As per coding guidelines.
src/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.tsx (2)
12-13: Remove dead hasErrors code or implement real error state.Currently always false; branches are unreachable.
54-60: Simplify numeric parsing.Always use parseFloat; supports ints, decimals, scientific notation.
- const isFloat = output.value.includes('.') || output.comparisonValue.includes('.'); - - const parsedValue = isFloat ? parseFloat(output.value) : parseInt(output.value, 10); - const parsedComparisonValue = isFloat - ? parseFloat(output.comparisonValue) - : parseInt(output.comparisonValue, 10); + const parsedValue = parseFloat(output.value); + const parsedComparisonValue = parseFloat(output.comparisonValue);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
src/notebooks/deepnote/converters/chartBigNumberBlockConverter.ts(1 hunks)src/notebooks/deepnote/deepnoteSerializer.ts(1 hunks)src/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.tsx(1 hunks)src/webviews/webview-side/chart-big-number-renderer/index.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
.tsfiles (not suffixed with.nodeor.web)
Files:
src/webviews/webview-side/chart-big-number-renderer/index.tssrc/notebooks/deepnote/converters/chartBigNumberBlockConverter.tssrc/notebooks/deepnote/deepnoteSerializer.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/webviews/webview-side/chart-big-number-renderer/index.tssrc/notebooks/deepnote/converters/chartBigNumberBlockConverter.tssrc/notebooks/deepnote/deepnoteSerializer.tssrc/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.tsx
src/notebooks/deepnote/**
📄 CodeRabbit inference engine (CLAUDE.md)
Deepnote integration code resides under src/notebooks/deepnote/
Files:
src/notebooks/deepnote/converters/chartBigNumberBlockConverter.tssrc/notebooks/deepnote/deepnoteSerializer.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 (6)
📚 Learning: 2025-09-03T12:53:28.421Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-03T12:53:28.421Z
Learning: Applies to **/*.{ts,tsx} : Use the `ILogger` service instead of `console.log`
Applied to files:
src/webviews/webview-side/chart-big-number-renderer/index.tssrc/notebooks/deepnote/converters/chartBigNumberBlockConverter.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/webviews/webview-side/chart-big-number-renderer/index.tssrc/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/deepnoteDataConverter.ts : deepnoteDataConverter.ts performs Deepnote data transformations
Applied to files:
src/notebooks/deepnote/converters/chartBigNumberBlockConverter.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/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/deepnoteTypes.ts : deepnoteTypes.ts contains Deepnote-related type definitions
Applied to files:
src/notebooks/deepnote/deepnoteSerializer.ts
🧬 Code graph analysis (3)
src/webviews/webview-side/chart-big-number-renderer/index.ts (2)
src/notebooks/deepnote/deepnoteSchemas.ts (2)
DeepnoteBigNumberMetadataSchema(11-44)DeepnoteChartBigNumberOutputSchema(3-9)src/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.tsx (1)
ChartBigNumberOutputRenderer(5-147)
src/notebooks/deepnote/converters/chartBigNumberBlockConverter.ts (5)
src/notebooks/deepnote/deepnoteSchemas.ts (1)
DeepnoteBigNumberMetadataSchema(11-44)src/notebooks/deepnote/converters/blockConverter.ts (1)
BlockConverter(5-10)src/notebooks/deepnote/deepnoteSerializer.ts (1)
DeepnoteBlock(9-9)src/test/mocks/vsc/extHostedTypes.ts (1)
NotebookCellData(2523-2549)src/notebooks/deepnote/dataConversionUtils.ts (1)
parseJsonWithFallback(5-11)
src/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.tsx (2)
src/notebooks/deepnote/deepnoteSchemas.ts (2)
DeepnoteChartBigNumberOutput(46-46)DeepnoteBigNumberMetadata(47-47)src/webviews/deepnote-utils/format-value.ts (1)
formatValue(1-36)
🔇 Additional comments (3)
src/notebooks/deepnote/deepnoteSerializer.ts (3)
1-3: LGTM: Import organization follows guidelines.Imports are properly separated and ordered.
7-7: LGTM: Type-only import.Using
typekeyword is good practice for imports used only at compile time.
9-9: Safe to remove DeepnoteProject export
No imports ofDeepnoteProjectfromdeepnoteSerializer;DeepnoteBlockandDeepnoteFileare present indeepnoteTypes.
src/notebooks/deepnote/converters/chartBigNumberBlockConverter.ts
Outdated
Show resolved
Hide resolved
src/notebooks/deepnote/converters/chartBigNumberBlockConverter.ts
Outdated
Show resolved
Hide resolved
src/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.tsx
Outdated
Show resolved
Hide resolved
src/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.tsx
Outdated
Show resolved
Hide resolved
src/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.tsx
Outdated
Show resolved
Hide resolved
Artmann
force-pushed
the
chris/display-data-frames
branch
from
930dcde to
1ba9ab4
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (13)
src/notebooks/deepnote/deepnoteSchemas.ts (1)
5-11: ** Inconsistent null handling already flagged.**Past review noted:
DeepnoteChartBigNumberOutputSchemauses.nullish()without.transform(), whileDeepnoteBigNumberMetadataSchematransformsundefinedtonull. Consumers must handle bothundefinedandnullfor output fields.src/webviews/webview-side/chart-big-number-renderer/index.ts (5)
19-20: ** Fragile JSON parsing already flagged.**Past review noted: regex-based quote stripping breaks on edge cases. Should handle both plain JSON and JSON-stringified JSON robustly.
38-38: ** Use ILogger instead of console.**Past review noted: coding guidelines require
ILoggerservice instead ofconsole.error.
27-36: ** Memory leak and duplicate mounts already flagged.**Past review noted: need to clear element, track mounted roots, and unmount React components properly to prevent memory leaks and duplicate mounts.
47-49: ** Cleanup not implemented.**Past review noted:
disposeOutputItemshould unmount React components to prevent memory leaks.
42-42: ** Localize error message.**Past review noted: "Error rendering chart big number" must use
l10n.t()per coding guidelines.src/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.tsx (7)
13-13: ** Dead code: hasErrors hardcoded.**Past review noted:
hasErrorsis alwaysfalse, making error branches unreachable.
15-21: ** Missing localization.**Past review noted: user-facing strings ('Not defined', 'Title') must use
l10n.t().
23-39: ** Missing localization.**Past review noted: user-facing strings ('N/A', 'Value', 'NaN') must use
l10n.t().
54-59: ** Unnecessary parseInt/parseFloat branching.**Past review noted:
parseFloathandles both integers and decimals correctly. Theincludes('.')check is unnecessary complexity.
73-78: ** Missing absolute-change support.**Past review noted: schema allows 'absolute-change', but implementation only handles 'absolute-value' and default difference.
89-93: ** Fix percent formatting.**Past review noted: pre-multiplying by 100 before calling
formatValue('percent')causes double-scaling.formatValuealready handles the conversion.
121-123: ** Hide comparison UI when format is 'off'.**Past review noted:
showComparisonshould also checkmetadata.deepnote_big_number_comparison_format !== 'off'to prevent displaying "-" when comparison is turned off (per PR comment).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
src/notebooks/deepnote/deepnoteConstants.ts(1 hunks)src/notebooks/deepnote/deepnoteDataConverter.ts(5 hunks)src/notebooks/deepnote/deepnoteSchemas.ts(1 hunks)src/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.tsx(1 hunks)src/webviews/webview-side/chart-big-number-renderer/index.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
.tsfiles (not suffixed with.nodeor.web)
Files:
src/notebooks/deepnote/deepnoteConstants.tssrc/webviews/webview-side/chart-big-number-renderer/index.tssrc/notebooks/deepnote/deepnoteSchemas.tssrc/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/deepnoteConstants.tssrc/webviews/webview-side/chart-big-number-renderer/index.tssrc/notebooks/deepnote/deepnoteSchemas.tssrc/notebooks/deepnote/deepnoteDataConverter.tssrc/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.tsx
src/notebooks/deepnote/**
📄 CodeRabbit inference engine (CLAUDE.md)
Deepnote integration code resides under src/notebooks/deepnote/
Files:
src/notebooks/deepnote/deepnoteConstants.tssrc/notebooks/deepnote/deepnoteSchemas.tssrc/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 (9)
📚 Learning: 2025-09-03T12:53:28.421Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-03T12:53:28.421Z
Learning: Applies to **/*.{ts,tsx} : Use the `ILogger` service instead of `console.log`
Applied to files:
src/webviews/webview-side/chart-big-number-renderer/index.ts
📚 Learning: 2025-09-03T12:53:28.421Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-03T12:53:28.421Z
Learning: Applies to **/*.{ts,tsx} : Use `l10n.t()` for user-facing strings
Applied to files:
src/webviews/webview-side/chart-big-number-renderer/index.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/webviews/webview-side/chart-big-number-renderer/index.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/deepnoteSchemas.ts
📚 Learning: 2025-08-18T23:41:22.727Z
Learnt from: CR
PR: deepnote/deepnote#0
File: CONVENTIONS.md:0-0
Timestamp: 2025-08-18T23:41:22.727Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Prefer null over undefined for intentional absence of values
Applied to files:
src/notebooks/deepnote/deepnoteSchemas.ts
📚 Learning: 2025-09-30T12:33:33.332Z
Learnt from: CR
PR: deepnote/deepnote-internal#0
File: CONVENTIONS.md:0-0
Timestamp: 2025-09-30T12:33:33.332Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Prefer null over undefined for intentional absence of value
Applied to files:
src/notebooks/deepnote/deepnoteSchemas.ts
📚 Learning: 2025-09-30T12:34:31.202Z
Learnt from: CR
PR: deepnote/deepnote-internal#0
File: .cursor/rules/base.mdc:0-0
Timestamp: 2025-09-30T12:34:31.202Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Prefer null over undefined as deliberate empty value
Applied to files:
src/notebooks/deepnote/deepnoteSchemas.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 (4)
src/webviews/webview-side/chart-big-number-renderer/index.ts (2)
src/notebooks/deepnote/deepnoteSchemas.ts (3)
getDeepnoteBlockMetadataSchema(48-52)DeepnoteBigNumberMetadataSchema(13-46)DeepnoteChartBigNumberOutputSchema(5-11)src/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.tsx (1)
ChartBigNumberOutputRenderer(5-153)
src/notebooks/deepnote/deepnoteSchemas.ts (2)
gulpfile.js (1)
schema(38-65)src/notebooks/deepnote/deepnoteConstants.ts (1)
OUTPUT_BLOCK_METADATA_KEY(1-1)
src/notebooks/deepnote/deepnoteDataConverter.ts (4)
src/notebooks/deepnote/converters/chartBigNumberBlockConverter.ts (1)
ChartBigNumberBlockConverter(12-63)src/notebooks/deepnote/deepnoteConstants.ts (2)
OUTPUT_BLOCK_METADATA_KEY(1-1)CHART_BIG_NUMBER_MIME_TYPE(3-3)src/notebooks/deepnote/deepnoteTypes.ts (2)
DeepnoteBlock(6-6)DeepnoteOutput(23-51)src/test/mocks/vsc/extHostedTypes.ts (2)
NotebookCellOutput(71-103)NotebookCellOutputItem(16-69)
src/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.tsx (2)
src/notebooks/deepnote/deepnoteSchemas.ts (2)
DeepnoteChartBigNumberOutput(54-54)DeepnoteBigNumberMetadata(55-55)src/webviews/deepnote-utils/format-value.ts (1)
formatValue(1-36)
🔇 Additional comments (13)
src/notebooks/deepnote/deepnoteSchemas.ts (2)
48-52: LGTM!Helper correctly wraps schema under
OUTPUT_BLOCK_METADATA_KEY.
54-55: LGTM!Type inference follows standard Zod pattern.
src/notebooks/deepnote/deepnoteConstants.ts (1)
1-3: LGTM!Constants correctly define metadata key and MIME type.
src/notebooks/deepnote/deepnoteDataConverter.ts (6)
4-4: LGTM!Type import correctly added.
10-11: LGTM!New imports for chart big number support.
24-24: LGTM!Converter correctly registered.
60-60: LGTM!Call site correctly updated with new parameters.
306-316: LGTM!Metadata correctly assembled with
blockMetadata,executionCount, and output metadata.
216-220: AlltransformOutputsForVsCodecall sites have been updated. No remaining references to the old signature.src/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.tsx (4)
65-71: LGTM!Division by zero correctly handled (lines 66-68).
85-87: LGTM!Null check correctly uses
== nullto allow zero values.
98-104: LGTM!Direction calculation correctly uses
== nullcheck.
125-152: LGTM!Component structure and rendering logic correct (aside from flagged issues).
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/notebooks/deepnote/converters/chartBigNumberBlockConverter.ts (2)
46-47: Use ILogger instead of console methods.Per coding guidelines and previous review feedback, use the
ILoggerservice for logging instead of console methods.As per coding guidelines.
Inject
ILoggerinto the class constructor and replace:-console.error('Error parsing deepnote big number metadata:', deepnoteBigNumberMetadataResult.error); -console.debug('Metadata:', JSON.stringify(block.metadata)); +this.logger.error('Error parsing deepnote big number metadata:', deepnoteBigNumberMetadataResult.error); +this.logger.debug('Metadata:', JSON.stringify(block.metadata));
10-10: Pre-stringify the default config.The constant is stringified on every call to
convertToCell(line 54). Stringify it once at declaration to avoid repeated serialization.Apply this diff:
-const DEFAULT_BIG_NUMBER_CONFIG = DeepnoteBigNumberMetadataSchema.parse({}); +const DEFAULT_BIG_NUMBER_CONFIG = JSON.stringify(DeepnoteBigNumberMetadataSchema.parse({}), null, 2);Then at line 54:
const configStr = deepnoteJupyterRawContentResult.success ? deepnoteJupyterRawContentResult.data : deepnoteBigNumberMetadataResult.success ? JSON.stringify(deepnoteBigNumberMetadataResult.data, null, 2) - : JSON.stringify(DEFAULT_BIG_NUMBER_CONFIG, null, 2); + : DEFAULT_BIG_NUMBER_CONFIG;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/notebooks/deepnote/converters/chartBigNumberBlockConverter.ts(1 hunks)src/notebooks/deepnote/converters/chartBigNumberBlockConverter.unit.test.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
.tsfiles (not suffixed with.nodeor.web)
Files:
src/notebooks/deepnote/converters/chartBigNumberBlockConverter.unit.test.tssrc/notebooks/deepnote/converters/chartBigNumberBlockConverter.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/converters/chartBigNumberBlockConverter.unit.test.tssrc/notebooks/deepnote/converters/chartBigNumberBlockConverter.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/converters/chartBigNumberBlockConverter.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/converters/chartBigNumberBlockConverter.unit.test.ts
src/notebooks/deepnote/**
📄 CodeRabbit inference engine (CLAUDE.md)
Deepnote integration code resides under src/notebooks/deepnote/
Files:
src/notebooks/deepnote/converters/chartBigNumberBlockConverter.unit.test.tssrc/notebooks/deepnote/converters/chartBigNumberBlockConverter.ts
🧠 Learnings (4)
📚 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/converters/chartBigNumberBlockConverter.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/converters/chartBigNumberBlockConverter.ts
📚 Learning: 2025-09-03T12:53:28.421Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-03T12:53:28.421Z
Learning: Applies to **/*.{ts,tsx} : Use the `ILogger` service instead of `console.log`
Applied to files:
src/notebooks/deepnote/converters/chartBigNumberBlockConverter.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 **/*.{ts,tsx} : Add a blank line after groups of const declarations and before return statements for readability
Applied to files:
src/notebooks/deepnote/converters/chartBigNumberBlockConverter.ts
🧬 Code graph analysis (2)
src/notebooks/deepnote/converters/chartBigNumberBlockConverter.unit.test.ts (3)
src/notebooks/deepnote/converters/chartBigNumberBlockConverter.ts (1)
ChartBigNumberBlockConverter(12-64)src/notebooks/deepnote/converters/constants.ts (1)
DEEPNOTE_VSCODE_RAW_CONTENT_KEY(1-1)src/test/mocks/vsc/extHostedTypes.ts (1)
NotebookCellData(2523-2549)
src/notebooks/deepnote/converters/chartBigNumberBlockConverter.ts (4)
src/notebooks/deepnote/deepnoteSchemas.ts (1)
DeepnoteBigNumberMetadataSchema(13-46)src/notebooks/deepnote/converters/blockConverter.ts (1)
BlockConverter(5-10)src/notebooks/deepnote/dataConversionUtils.ts (1)
parseJsonWithFallback(5-11)src/notebooks/deepnote/converters/constants.ts (1)
DEEPNOTE_VSCODE_RAW_CONTENT_KEY(1-1)
🔇 Additional comments (3)
src/notebooks/deepnote/converters/chartBigNumberBlockConverter.ts (3)
13-35: LGTM!The method correctly handles both valid and invalid config, storing raw content as fallback and cleaning up when valid config is applied.
37-39: LGTM!Case-insensitive type matching is appropriate.
61-63: LGTM!Correctly returns the supported block type.
src/notebooks/deepnote/converters/chartBigNumberBlockConverter.unit.test.ts
Outdated
Show resolved
Hide resolved
OlegWock
left a comment
OlegWock
left a comment
There was a problem hiding this comment.
Couple of comments, but I don't think they should be blocking
| if (outputToBeUpdated.cell.metadata['__deepnotePocket']?.['type'] === 'big-number') { | ||
| data[CHART_BIG_NUMBER_MIME_TYPE] = data['text/plain']; | ||
| delete data['text/plain']; | ||
| } |
There was a problem hiding this comment.
Please add comment why we need this
src/kernels/execution/helpers.ts
Outdated
| } | ||
| } | ||
| */ | ||
| // TODO - add DeepnotePocket zod schema validation |
There was a problem hiding this comment.
Please create ticket and put its number here (or address directly in this PR if that's appropiate)
There was a problem hiding this comment.
Same for todo couple lines below
| if (data['text/plain']) { | ||
| items.push(NotebookCellOutputItem.text(data['text/plain'] as string)); | ||
| let mimeType = 'text/plain'; | ||
| if (blockType === 'big-number' && !(CHART_BIG_NUMBER_MIME_TYPE in data)) { |
There was a problem hiding this comment.
Could you add comment what is happening here? Also comment above is not outdated
| output: DeepnoteChartBigNumberOutput; | ||
| metadata: DeepnoteBigNumberMetadata; | ||
| }) { | ||
| // TODO: either remove or handle here .. currently handled in the parent |
There was a problem hiding this comment.
Is this still relevant? Please address or add Linear ticket id here
| ReactDOM.render( | ||
| React.createElement(ChartBigNumberOutputRenderer, { | ||
| output: chartBigNumberOutput, | ||
| metadata: blockMetadata | ||
| }), | ||
| element | ||
| ); |
There was a problem hiding this comment.
Suggestion: in charts PR I added error boundary, you could use it here without much extra work (maybe with some minimal adjustments)
…nd enhance big number output rendering
tkislan
dismissed stale reviews from OlegWock and coderabbitai[bot]
via
95fd687
There was a problem hiding this comment.
Actionable comments posted: 2
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/helpers.ts (1)
361-379: Stop emitting duplicate big-number itemsWhen a big-number block carries both
text/plainandapplication/vnd.deepnote.chart.big-number+json, this loop now pushes twoNotebookCellOutputItems with the same MIME type. VS Code renders both, so users see duplicate big-number widgets. Skip the fallback when the custom payload already exists.- if (data['text/plain']) { - let mimeType = 'text/plain'; - // deepnote-toolkit returns the text/plain mime type for big number outputs - // and for the custom renderer to be used, we need to return the application/vnd.deepnote.chart.big-number+json mime type - if (blockType === 'big-number' && !(CHART_BIG_NUMBER_MIME_TYPE in data)) { - mimeType = CHART_BIG_NUMBER_MIME_TYPE; - } - items.push(NotebookCellOutputItem.text(data['text/plain'] as string, mimeType)); + if (data['text/plain']) { + if (blockType === 'big-number' && CHART_BIG_NUMBER_MIME_TYPE in data) { + continue; + } + const mimeType = + blockType === 'big-number' + ? CHART_BIG_NUMBER_MIME_TYPE + : 'text/plain'; + items.push(NotebookCellOutputItem.text(data['text/plain'] as string, mimeType));
📜 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)
.eslintrc.js(1 hunks)package.json(3 hunks)src/kernels/execution/cellExecutionMessageHandler.ts(3 hunks)src/kernels/execution/helpers.ts(3 hunks)src/notebooks/deepnote/deepnoteDataConverter.ts(6 hunks)src/notebooks/deepnote/deepnoteSchemas.ts(1 hunks)src/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.tsx(1 hunks)src/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRendererContainer.tsx(1 hunks)src/webviews/webview-side/chart-big-number-renderer/ErrorBoundary.tsx(1 hunks)src/webviews/webview-side/chart-big-number-renderer/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{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/chart-big-number-renderer/ErrorBoundary.tsxsrc/notebooks/deepnote/deepnoteDataConverter.tssrc/webviews/webview-side/chart-big-number-renderer/index.tssrc/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRendererContainer.tsxsrc/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.tsxsrc/kernels/execution/helpers.tssrc/notebooks/deepnote/deepnoteSchemas.tssrc/kernels/execution/cellExecutionMessageHandler.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/webviews/webview-side/chart-big-number-renderer/index.tssrc/kernels/execution/helpers.tssrc/notebooks/deepnote/deepnoteSchemas.tssrc/kernels/execution/cellExecutionMessageHandler.ts
src/notebooks/deepnote/**
📄 CodeRabbit inference engine (CLAUDE.md)
Deepnote integration code resides under src/notebooks/deepnote/
Files:
src/notebooks/deepnote/deepnoteDataConverter.tssrc/notebooks/deepnote/deepnoteSchemas.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/helpers.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/helpers.tssrc/kernels/execution/cellExecutionMessageHandler.ts
🧠 Learnings (13)
📓 Common learnings
Learnt from: tkislan
PR: deepnote/vscode-deepnote#41
File: src/notebooks/deepnote/deepnoteDataConverter.ts:296-314
Timestamp: 2025-10-16T13:09:32.543Z
Learning: For big-number blocks in src/notebooks/deepnote/deepnoteDataConverter.ts, invalid JSON validation and error handling is performed in the renderer (src/webviews/webview-side/chart-big-number-renderer) rather than in the converter layer. The converter can safely coerce text/plain to CHART_BIG_NUMBER_MIME_TYPE for big-number blocks without pre-validation.
📚 Learning: 2025-10-16T13:09:32.543Z
Learnt from: tkislan
PR: deepnote/vscode-deepnote#41
File: src/notebooks/deepnote/deepnoteDataConverter.ts:296-314
Timestamp: 2025-10-16T13:09:32.543Z
Learning: For big-number blocks in src/notebooks/deepnote/deepnoteDataConverter.ts, invalid JSON validation and error handling is performed in the renderer (src/webviews/webview-side/chart-big-number-renderer) rather than in the converter layer. The converter can safely coerce text/plain to CHART_BIG_NUMBER_MIME_TYPE for big-number blocks without pre-validation.
Applied to files:
src/notebooks/deepnote/deepnoteDataConverter.tssrc/webviews/webview-side/chart-big-number-renderer/index.tssrc/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRendererContainer.tsxsrc/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.tsxpackage.jsonsrc/kernels/execution/cellExecutionMessageHandler.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-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/deepnoteDataConverter.ts
📚 Learning: 2025-09-03T12:53:28.421Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-03T12:53:28.421Z
Learning: Applies to **/*.{ts,tsx} : Use the `ILogger` service instead of `console.log`
Applied to files:
src/webviews/webview-side/chart-big-number-renderer/index.ts
📚 Learning: 2025-09-03T12:53:28.421Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-03T12:53:28.421Z
Learning: Applies to **/*.{ts,tsx} : Use `l10n.t()` for user-facing strings
Applied to files:
src/webviews/webview-side/chart-big-number-renderer/index.ts
📚 Learning: 2025-10-16T13:14:27.505Z
Learnt from: tkislan
PR: deepnote/vscode-deepnote#41
File: src/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.tsx:73-78
Timestamp: 2025-10-16T13:14:27.505Z
Learning: In the vscode-deepnote repository, code in `src/webviews/webview-side/chart-big-number-renderer/` is a direct copy from the main Deepnote app and should be kept in sync with it, even if issues are identified. This code should eventually be abstracted into the deepnote/deepnote package.
Applied to files:
src/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.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/** : Deepnote integration code resides under src/notebooks/deepnote/
Applied to files:
src/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.tsx
📚 Learning: 2025-10-16T09:28:01.616Z
Learnt from: OlegWock
PR: deepnote/vscode-deepnote#66
File: src/webviews/webview-side/vega-renderer/number-formats.ts:11-16
Timestamp: 2025-10-16T09:28:01.616Z
Learning: In the vscode-deepnote repository, code in `src/webviews/webview-side/vega-renderer/` is a direct copy from the main Deepnote app and should be kept in sync with it, even if issues are identified.
Applied to files:
src/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.tsx
📚 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/helpers.ts
📚 Learning: 2025-08-18T23:41:22.727Z
Learnt from: CR
PR: deepnote/deepnote#0
File: CONVENTIONS.md:0-0
Timestamp: 2025-08-18T23:41:22.727Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Prefer null over undefined for intentional absence of values
Applied to files:
src/notebooks/deepnote/deepnoteSchemas.ts
📚 Learning: 2025-09-30T12:33:33.332Z
Learnt from: CR
PR: deepnote/deepnote-internal#0
File: CONVENTIONS.md:0-0
Timestamp: 2025-09-30T12:33:33.332Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Prefer null over undefined for intentional absence of value
Applied to files:
src/notebooks/deepnote/deepnoteSchemas.ts
📚 Learning: 2025-09-30T12:34:31.202Z
Learnt from: CR
PR: deepnote/deepnote-internal#0
File: .cursor/rules/base.mdc:0-0
Timestamp: 2025-09-30T12:34:31.202Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Prefer null over undefined as deliberate empty value
Applied to files:
src/notebooks/deepnote/deepnoteSchemas.ts
🧬 Code graph analysis (6)
src/notebooks/deepnote/deepnoteDataConverter.ts (2)
src/notebooks/deepnote/converters/chartBigNumberBlockConverter.ts (1)
ChartBigNumberBlockConverter(12-64)src/platform/deepnote/deepnoteConstants.ts (1)
CHART_BIG_NUMBER_MIME_TYPE(1-1)
src/webviews/webview-side/chart-big-number-renderer/index.ts (2)
src/webviews/webview-side/chart-big-number-renderer/ErrorBoundary.tsx (1)
ErrorFallback(4-37)src/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRendererContainer.tsx (1)
ChartBigNumberOutputRendererContainer(9-23)
src/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRendererContainer.tsx (1)
src/notebooks/deepnote/deepnoteSchemas.ts (2)
DeepnoteBigNumberMetadataSchema(11-44)DeepnoteChartBigNumberOutputSchema(3-9)
src/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.tsx (2)
src/notebooks/deepnote/deepnoteSchemas.ts (2)
DeepnoteChartBigNumberOutput(46-46)DeepnoteBigNumberMetadata(47-47)src/webviews/deepnote-utils/format-value.ts (1)
formatValue(1-36)
src/kernels/execution/helpers.ts (2)
src/platform/deepnote/pocket.ts (1)
src/platform/deepnote/deepnoteConstants.ts (1)
CHART_BIG_NUMBER_MIME_TYPE(1-1)
src/kernels/execution/cellExecutionMessageHandler.ts (1)
src/platform/deepnote/deepnoteConstants.ts (1)
CHART_BIG_NUMBER_MIME_TYPE(1-1)
⏰ 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). (1)
- GitHub Check: Build & Test
🔇 Additional comments (1)
src/webviews/webview-side/chart-big-number-renderer/ChartBigNumberOutputRenderer.tsx (1)
1-134: Upstream synchronization acknowledged.This renderer is copied from the main Deepnote app. Per previous discussion, keeping it in sync with upstream is the priority until abstracted into
@deepnote/deepnotepackage.Based on learnings.
Signed-off-by: Tomas Kislan <tomas@kislan.sk>
…support-big-number-blocks
tkislan
dismissed stale reviews from OlegWock and coderabbitai[bot]
via
60442ec
jankuca
left a comment
jankuca
left a comment
There was a problem hiding this comment.
I see this was previously approved and Oleh is off today, approving the latest push too
tkislan
dismissed stale reviews from jankuca and coderabbitai[bot]
via
236cccd
jamesbhobbs
deleted the
tomaskislan/grn-4762-support-big-number-blocks
branch
Summary by CodeRabbit
New Features
Tests