Open deepnote file #11
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds full Deepnote notebook support: types, serializer, data converter, MIME/output handlers, output-type detector, data conversion utilities, notebook selector, notebook manager, activation service, service registrations, and unit tests. Updates package manifest (new notebook type, activation events, commands, notebook preload, dependencies, localization). Adds CLAUDE.md and adjusts issue templates. Restructures CI by deleting several legacy workflows and adding consolidated Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant VSCode as VS Code
participant Ext as Extension
participant Serializer as DeepnoteNotebookSerializer
participant Manager as DeepnoteNotebookManager
participant Selector as DeepnoteNotebookSelector
participant Converter as DeepnoteDataConverter
participant YAML as js-yaml
Note over VSCode,Ext: Activation & registration
VSCode->>Ext: Activate onNotebook:deepnote
Ext->>VSCode: register serializer & command
Note over User,VSCode: Open *.deepnote
User->>VSCode: open file
VSCode->>Serializer: deserialize(content)
Serializer->>YAML: parse(content)
YAML-->>Serializer: DeepnoteProject
Serializer->>Manager: storeOriginalProject(...)
alt selection available or single notebook or skip
Serializer->>Manager: getSelectedNotebookForUri(...)
else prompt needed
Serializer->>Selector: selectNotebook(...)
Selector-->>Serializer: chosenNotebook
Serializer->>Manager: setSelectedNotebookForUri(...)
end
Serializer->>Converter: convertBlocksToCells(chosen.blocks)
Converter-->>Serializer: NotebookCellData[]
Serializer-->>VSCode: return NotebookData(cells, metadata)
sequenceDiagram
autonumber
actor User
participant VSCode as VS Code
participant Ext as Extension
participant Selector as DeepnoteNotebookSelector
participant Converter as DeepnoteDataConverter
participant Manager as DeepnoteNotebookManager
Note over User,VSCode: Switch Deepnote notebook
User->>VSCode: run jupyter.selectDeepnoteNotebook
VSCode->>Ext: executeCommand
Ext->>Manager: getOriginalProject / currentNotebookId
Ext->>Selector: selectNotebook(notebooks, currentId)
Selector-->>Ext: selectedNotebook
alt selected differs
Ext->>Converter: convertBlocksToCells(selected.blocks)
Converter-->>Ext: NotebookCellData[]
Ext->>VSCode: apply WorkspaceEdit (replace cells, update metadata)
VSCode-->>Ext: edit result
Ext->>Manager: setSelectedNotebookForUri(uri, selected.id)
Ext->>Manager: updateCurrentNotebookId(projectId, selected.id)
else canceled/no-op
Note over Ext: no changes
end
sequenceDiagram
autonumber
participant VSCode as VS Code
participant Serializer as DeepnoteNotebookSerializer
participant Manager as DeepnoteNotebookManager
participant Converter as DeepnoteDataConverter
participant YAML as js-yaml
Note over VSCode,Serializer: Save .deepnote file
VSCode->>Serializer: serialize(NotebookData)
Serializer->>Manager: getOriginalProject(projectId)
Serializer->>Converter: convertCellsToBlocks(cells)
Converter-->>Serializer: DeepnoteBlock[]
Serializer->>Serializer: update project.notebooks[target].blocks & modifiedAt
Serializer->>YAML: dump(project)
YAML-->>Serializer: YAML buffer
Serializer-->>VSCode: Uint8Array (file contents)
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (13)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 44
📜 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 (23)
.github/ISSUE_TEMPLATE/2_bug_form.yml(1 hunks).github/ISSUE_TEMPLATE/config.yml(1 hunks).github/workflows/aggregate-test-results.yml(0 hunks).github/workflows/build-test.yml(0 hunks).github/workflows/ci.yml(1 hunks).github/workflows/codeql-analysis.yml(0 hunks).github/workflows/deps.yml(1 hunks).github/workflows/lock.yml(0 hunks)CLAUDE.md(1 hunks)package.json(8 hunks)src/notebooks/deepnote/deepnoteActivationService.ts(1 hunks)src/notebooks/deepnote/deepnoteDataConverter.ts(1 hunks)src/notebooks/deepnote/deepnoteDataConverter.unit.test.ts(1 hunks)src/notebooks/deepnote/deepnoteNotebookManager.ts(1 hunks)src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts(1 hunks)src/notebooks/deepnote/deepnoteNotebookSelector.ts(1 hunks)src/notebooks/deepnote/deepnoteNotebookSelector.unit.test.ts(1 hunks)src/notebooks/deepnote/deepnoteSerializer.ts(1 hunks)src/notebooks/deepnote/deepnoteTypes.ts(1 hunks)src/notebooks/serviceRegistry.node.ts(2 hunks)src/notebooks/serviceRegistry.web.ts(2 hunks)src/platform/common/constants.ts(1 hunks)src/standalone/api/kernels/kernel.unit.test.ts(1 hunks)
💤 Files with no reviewable changes (4)
- .github/workflows/aggregate-test-results.yml
- .github/workflows/lock.yml
- .github/workflows/codeql-analysis.yml
- .github/workflows/build-test.yml
🧰 Additional context used
📓 Path-based instructions (8)
**/!(*.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/platform/common/constants.tssrc/standalone/api/kernels/kernel.unit.test.tssrc/notebooks/deepnote/deepnoteActivationService.tssrc/notebooks/deepnote/deepnoteNotebookSelector.tssrc/notebooks/deepnote/deepnoteNotebookSelector.unit.test.tssrc/notebooks/deepnote/deepnoteDataConverter.unit.test.tssrc/notebooks/deepnote/deepnoteNotebookManager.unit.test.tssrc/notebooks/deepnote/deepnoteNotebookManager.tssrc/notebooks/deepnote/deepnoteTypes.tssrc/notebooks/deepnote/deepnoteSerializer.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
Files:
src/platform/common/constants.tssrc/notebooks/serviceRegistry.node.tssrc/standalone/api/kernels/kernel.unit.test.tssrc/notebooks/serviceRegistry.web.tssrc/notebooks/deepnote/deepnoteActivationService.tssrc/notebooks/deepnote/deepnoteNotebookSelector.tssrc/notebooks/deepnote/deepnoteNotebookSelector.unit.test.tssrc/notebooks/deepnote/deepnoteDataConverter.unit.test.tssrc/notebooks/deepnote/deepnoteNotebookManager.unit.test.tssrc/notebooks/deepnote/deepnoteNotebookManager.tssrc/notebooks/deepnote/deepnoteTypes.tssrc/notebooks/deepnote/deepnoteSerializer.tssrc/notebooks/deepnote/deepnoteDataConverter.ts
src/platform/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/platform.instructions.md)
src/platform/**/*.ts: Use Inversify decorators for DI: annotate classes with @Injectable() and inject dependencies with @Inject(Token)
Use the centralized logger (import { logger } from '../platform/logging') instead of console.log for application logging
Files:
src/platform/common/constants.ts
**/*.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/serviceRegistry.node.ts
**/*.unit.test.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place unit tests in files matching
*.unit.test.ts
Files:
src/standalone/api/kernels/kernel.unit.test.tssrc/notebooks/deepnote/deepnoteNotebookSelector.unit.test.tssrc/notebooks/deepnote/deepnoteDataConverter.unit.test.tssrc/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts
src/standalone/api/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/standalone.instructions.md)
Define new API endpoint interfaces within the appropriate subdirectory under src/standalone/api/
Files:
src/standalone/api/kernels/kernel.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/standalone/api/kernels/kernel.unit.test.tssrc/notebooks/deepnote/deepnoteNotebookSelector.unit.test.tssrc/notebooks/deepnote/deepnoteDataConverter.unit.test.tssrc/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts
**/*.web.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
*.web.tsfor Web-specific implementations using browser-compatible APIs
Files:
src/notebooks/serviceRegistry.web.ts
🧠 Learnings (18)
📚 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 : Add UI elements for the new format to the export quick pick
Applied to files:
src/platform/common/constants.tssrc/notebooks/deepnote/deepnoteNotebookSelector.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/notebooks/serviceRegistry.node.tssrc/notebooks/serviceRegistry.web.ts
📚 Learning: 2025-09-03T13:01:10.017Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/standalone.instructions.md:0-0
Timestamp: 2025-09-03T13:01:10.017Z
Learning: Applies to src/standalone/serviceRegistry.{node,web}.ts : Register new services for both platforms in serviceRegistry.node.ts and serviceRegistry.web.ts as appropriate
Applied to files:
src/notebooks/serviceRegistry.node.tssrc/notebooks/serviceRegistry.web.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/serviceRegistry.{node,web}.ts : Use dependency injection via the service registry for component creation and registration
Applied to files:
src/notebooks/serviceRegistry.node.tssrc/notebooks/serviceRegistry.web.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/serviceRegistry.{node,web}.ts : Use dependency injection for services to enable testability and platform variance
Applied to files:
src/notebooks/serviceRegistry.node.tssrc/notebooks/serviceRegistry.web.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/serviceRegistry.{node,web}.ts : Register dependencies via DI with platform-appropriate services for Node and Web builds
Applied to files:
src/notebooks/serviceRegistry.node.tssrc/notebooks/serviceRegistry.web.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/**/*.ts : Monitor and dispose pending promises to prevent leaks during teardown
Applied to files:
src/standalone/api/kernels/kernel.unit.test.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/webviews/webview-side/ipywidgets/kernel/index.ts : Create a proxy kernel in the webview to manage widget lifecycle and relay comms with the extension
Applied to files:
src/standalone/api/kernels/kernel.unit.test.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} : Include the Microsoft copyright header in source files
Applied to files:
CLAUDE.md
📚 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/**/*.ts : Add unit and integration tests for new features and modifications; include performance and cross-platform test coverage
Applied to files:
CLAUDE.mdsrc/notebooks/deepnote/deepnoteNotebookSelector.unit.test.tssrc/notebooks/deepnote/deepnoteDataConverter.unit.test.tssrc/notebooks/deepnote/deepnoteNotebookManager.unit.test.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/controllers/kernelSource/**/*.ts : For new kernel sources, implement ILocalNotebookKernelSourceSelector (or similar)
Applied to files:
src/notebooks/deepnote/deepnoteNotebookSelector.tssrc/notebooks/deepnote/deepnoteNotebookSelector.unit.test.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/controllers/kernelSource/**/*.ts : Create a UI provider for kernel selection for the new kernel source
Applied to files:
src/notebooks/deepnote/deepnoteNotebookSelector.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 **/*.unit.test.ts : Place unit tests in files matching `*.unit.test.ts`
Applied to files:
src/notebooks/deepnote/deepnoteNotebookSelector.unit.test.tssrc/notebooks/deepnote/deepnoteDataConverter.unit.test.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 **/!(*.unit).test.ts : Place integration tests in `*.test.ts` files that are not `*.unit.test.ts`
Applied to files:
src/notebooks/deepnote/deepnoteNotebookSelector.unit.test.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.unit.test.tssrc/notebooks/deepnote/deepnoteSerializer.tssrc/notebooks/deepnote/deepnoteDataConverter.ts
📚 Learning: 2025-09-03T13:01:49.096Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-09-03T13:01:49.096Z
Learning: Applies to **/*.{test,spec}.ts : In unit tests, when a mock is returned from a promise, ensure the mocked instance has an undefined `then` property to avoid hanging tests
Applied to files:
src/notebooks/deepnote/deepnoteNotebookManager.unit.test.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/debugger/**/*.ts : Add the new debugging controller to DebuggingManager creation logic
Applied to files:
src/notebooks/deepnote/deepnoteNotebookManager.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 : On conversion failures, provide detailed error reporting with suggested fixes
Applied to files:
src/notebooks/deepnote/deepnoteDataConverter.ts
🧬 Code graph analysis (9)
src/notebooks/deepnote/deepnoteActivationService.ts (4)
src/notebooks/deepnote/deepnoteSerializer.ts (2)
DeepnoteNotebookSerializer(10-187)DeepnoteProject(8-8)src/notebooks/deepnote/deepnoteNotebookSelector.ts (1)
DeepnoteNotebookSelector(10-72)src/notebooks/deepnote/deepnoteTypes.ts (1)
DeepnoteProject(1-13)src/test/mocks/vsc/extHostedTypes.ts (3)
WorkspaceEdit(649-833)NotebookEdit(2444-2507)NotebookRange(2413-2443)
src/notebooks/deepnote/deepnoteNotebookSelector.ts (3)
src/notebooks/deepnote/deepnoteTypes.ts (1)
DeepnoteNotebook(15-22)src/platform/common/utils/events.ts (1)
toPromise(44-46)src/test/datascience/notebook/executionHelper.ts (1)
cellCount(101-103)
src/notebooks/deepnote/deepnoteNotebookSelector.unit.test.ts (2)
src/notebooks/deepnote/deepnoteNotebookSelector.ts (1)
DeepnoteNotebookSelector(10-72)src/notebooks/deepnote/deepnoteTypes.ts (1)
DeepnoteNotebook(15-22)
src/notebooks/deepnote/deepnoteDataConverter.unit.test.ts (3)
src/notebooks/deepnote/deepnoteDataConverter.ts (1)
DeepnoteDataConverter(5-202)src/notebooks/deepnote/deepnoteTypes.ts (2)
DeepnoteBlock(24-33)DeepnoteOutput(35-42)src/test/mocks/vsc/extHostedTypes.ts (1)
NotebookCellData(2523-2549)
src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts (2)
src/notebooks/deepnote/deepnoteNotebookManager.ts (1)
DeepnoteNotebookManager(3-53)src/notebooks/deepnote/deepnoteTypes.ts (1)
DeepnoteProject(1-13)
src/notebooks/deepnote/deepnoteNotebookManager.ts (1)
src/notebooks/deepnote/deepnoteTypes.ts (1)
DeepnoteProject(1-13)
src/notebooks/deepnote/deepnoteTypes.ts (1)
src/notebooks/deepnote/deepnoteSerializer.ts (4)
DeepnoteProject(8-8)DeepnoteNotebook(8-8)DeepnoteBlock(8-8)DeepnoteOutput(8-8)
src/notebooks/deepnote/deepnoteSerializer.ts (5)
src/notebooks/deepnote/deepnoteNotebookManager.ts (1)
DeepnoteNotebookManager(3-53)src/notebooks/deepnote/deepnoteNotebookSelector.ts (1)
DeepnoteNotebookSelector(10-72)src/notebooks/deepnote/deepnoteDataConverter.ts (1)
DeepnoteDataConverter(5-202)src/notebooks/deepnote/deepnoteTypes.ts (2)
DeepnoteProject(1-13)DeepnoteNotebook(15-22)src/test/mocks/vsc/index.ts (1)
CancellationToken(100-112)
src/notebooks/deepnote/deepnoteDataConverter.ts (2)
src/notebooks/deepnote/deepnoteTypes.ts (2)
DeepnoteBlock(24-33)DeepnoteOutput(35-42)src/test/mocks/vsc/extHostedTypes.ts (3)
NotebookCellData(2523-2549)NotebookCellOutput(71-103)NotebookCellOutputItem(16-69)
🪛 markdownlint-cli2 (0.17.2)
CLAUDE.md
1-1: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
2-2: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
5-5: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
6-6: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
13-13: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
14-14: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
19-19: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
20-20: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
29-29: Files should end with a single newline character
(MD047, single-trailing-newline)
⏰ 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 (18)
src/platform/common/constants.ts (1)
221-221: New Deepnote command constant — LGTM.Constant name and value look consistent with the manifest.
.github/ISSUE_TEMPLATE/2_bug_form.yml (1)
11-12: Verify new support links resolve correctly.Double-check that the wiki and discussions URLs exist and are publicly accessible.
.github/workflows/ci.yml (1)
11-12: Confirm Node/NPM versions across toolchain.Node 22.x and npm 10.x can break older tooling (native deps, vscode-test). Verify builds/tests pass on CI and locally.
src/notebooks/deepnote/deepnoteTypes.ts (1)
15-33: Schema fidelity: confirm fields match Deepnote’s .deepnote spec.Validate
executionMode,sortingKey, andoutput*fields naming and optionality against real files.package.json (6)
32-32: Engine downgrade — verify compatibility.Lowering to "vscode": "^1.103.0" can exclude APIs you rely on. Ensure all used proposals/features are supported in 1.103.
59-64: Activation for Deepnote — LGTM.onNotebook:deepnote is correctly added alongside existing events.
894-898: Toolbar button for Deepnote — LGTM.Placement under navigation is reasonable; icon optional.
1971-1977: Preload for Deepnote — verify necessity and bundle path.Confirm the ipywidgetsKernel entrypoint works for Deepnote notebooks and the path exists in dist for both node/web builds.
2305-2305: Add js-yaml — LGTM.Appropriate choice for YAML parsing.
2364-2364: Types for js-yaml — LGTM.Dev dependency is correct.
src/notebooks/serviceRegistry.web.ts (2)
38-38: Deepnote activation service registration (Web) — LGTM.Import and DI registration pattern matches existing activation services. No issues spotted.
Also applies to: 88-91
40-91: All Deepnote registrations verified
DeepnoteActivationService implements IExtensionSyncActivationService, is registered in both web and node service registries, and the ‘deepnote’ notebook serializer is properly registered.src/notebooks/serviceRegistry.node.ts (1)
43-43: Deepnote activation service registration (Node) — LGTM.Mirrors the web registry; consistent with DI patterns here.
Also applies to: 112-115
src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts (1)
1-230: State-management tests — LGTM.Good coverage of per-URI flags, overwrite semantics, and multi-project isolation.
src/notebooks/deepnote/deepnoteDataConverter.unit.test.ts (3)
14-36: Good coverage for basic block→cell mapping.Solid assertions on kind, languageId, and metadata propagation.
58-86: Ordering test is precise and valuable.Sorting by sortingKey is exercised well.
170-192: ID and sortingKey generation checks are good.The regex + uniqueness assertions are appropriate.
src/notebooks/deepnote/deepnoteSerializer.ts (1)
128-136: Web-friendly YAML emission and localization unaffected.No functional issue; LGTM.
.github/workflows/deps.yml
Outdated
Comment on lines
21
to
22
| - name: Checkout | ||
| uses: actions/checkout@v5 |
Contributor
There was a problem hiding this comment.
actions/checkout@v5 likely invalid — pin to a released major.
Align with CI and use @v4 (or a commit SHA).
Apply this diff in both jobs:
- uses: actions/checkout@v5
+ uses: actions/checkout@v4Also applies to: 44-45
🤖 Prompt for AI Agents
In .github/workflows/deps.yml around lines 21-22 (and also lines 44-45), the
workflow references actions/checkout@v5 which is likely invalid; update the
checkout action to a released major like actions/checkout@v4 (or pin to a
specific commit SHA) in both job steps. Replace the uses value for both
occurrences with the chosen stable reference and ensure the workflow passes by
committing that change.
Comment on lines
10
to
14
| export class DeepnoteNotebookSerializer implements NotebookSerializer { | ||
| private static manager = new DeepnoteNotebookManager(); | ||
| private static selector = new DeepnoteNotebookSelector(); | ||
| private static converter = new DeepnoteDataConverter(); | ||
|
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Inject interfaces instead of concrete singletons.
Prefer DI with interfaces for manager/selector/converter to avoid tight coupling and ease testing.
🤖 Prompt for AI Agents
In src/notebooks/deepnote/deepnoteSerializer.ts around lines 10 to 14, replace
the hard-coded static singletons with constructor-injected interfaces: remove
the private static manager/selector/converter singletons and add instance fields
typed to the manager/selector/converter interfaces, provide a constructor that
accepts those interfaces (with optional defaults that instantiate the existing
concrete classes for backwards compatibility), update usages to reference the
instance fields (non-static), and adjust any call sites/tests to pass mocks or
the defaults; this decouples the serializer for DI and unit testing without
changing external behavior.
Comment on lines
93
to
132
| async serializeNotebook(data: NotebookData, _token: CancellationToken): Promise<Uint8Array> { | ||
| try { | ||
| const projectId = data.metadata?.deepnoteProjectId; | ||
| if (!projectId) { | ||
| throw new Error('Missing Deepnote project ID in notebook metadata'); | ||
| } | ||
|
|
||
| const originalProject = DeepnoteNotebookSerializer.manager.getOriginalProject(projectId); | ||
| if (!originalProject) { | ||
| throw new Error('Original Deepnote project not found. Cannot save changes.'); | ||
| } | ||
|
|
||
| // Get the current notebook ID (may have changed due to switching) | ||
| const notebookId = | ||
| data.metadata?.deepnoteNotebookId || DeepnoteNotebookSerializer.manager.getCurrentNotebookId(projectId); | ||
| if (!notebookId) { | ||
| throw new Error('Cannot determine which notebook to save'); | ||
| } | ||
|
|
||
| // Find the notebook to update | ||
| const notebookIndex = originalProject.project.notebooks.findIndex((nb) => nb.id === notebookId); | ||
| if (notebookIndex === -1) { | ||
| throw new Error(`Notebook with ID ${notebookId} not found in project`); | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use typed errors and localized messages.
Localize user-facing strings and throw typed errors.
- const projectId = data.metadata?.deepnoteProjectId;
+ const projectId = data.metadata?.deepnoteProjectId;
if (!projectId) {
- throw new Error('Missing Deepnote project ID in notebook metadata');
+ throw new FileFormatError(l10n.t('Missing Deepnote project ID in notebook metadata'));
}
@@
- const originalProject = DeepnoteNotebookSerializer.manager.getOriginalProject(projectId);
+ const originalProject = DeepnoteNotebookSerializer.manager.getOriginalProject(projectId);
if (!originalProject) {
- throw new Error('Original Deepnote project not found. Cannot save changes.');
+ throw new NotFoundError(l10n.t('Original Deepnote project not found. Cannot save changes.'));
}
@@
- const notebookId =
+ const notebookId =
data.metadata?.deepnoteNotebookId || DeepnoteNotebookSerializer.manager.getCurrentNotebookId(projectId);
if (!notebookId) {
- throw new Error('Cannot determine which notebook to save');
+ throw new FileFormatError(l10n.t('Cannot determine which notebook to save'));
}
@@
- if (notebookIndex === -1) {
- throw new Error(`Notebook with ID ${notebookId} not found in project`);
+ if (notebookIndex === -1) {
+ throw new NotFoundError(l10n.t('Notebook with ID {0} not found in project', notebookId));
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/notebooks/deepnote/deepnoteSerializer.ts around lines 93 to 116, the code
throws plain Error with hard-coded English strings; replace these with typed
error classes and localized messages: create or reuse specific error types
(e.g., DeepnoteNotFoundError, DeepnoteValidationError) and throw instances of
those types instead of new Error(...), and obtain user-facing text via the
project's i18n/localize helper (e.g.,
localize('deepnote.error.missingProjectId')) passing any variables as
translation parameters; import the error classes and the localization function
at the top of the file, replace each throw with a localized message and the
appropriate typed error, and ensure any callers that catch these errors still
handle the new error types or map them to existing error handling paths.
Comment on lines
1
to
7
| export interface DeepnoteProject { | ||
| metadata: { | ||
| createdAt: string; |
Contributor
There was a problem hiding this comment.
Add required copyright header.
All TS files must start with the Microsoft copyright header.
Apply this diff:
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
export interface DeepnoteProject {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export interface DeepnoteProject { | |
| metadata: { | |
| createdAt: string; | |
| // Copyright (c) Microsoft Corporation. | |
| // Licensed under the MIT License. | |
| export interface DeepnoteProject { | |
| metadata: { | |
| createdAt: string; |
🤖 Prompt for AI Agents
In src/notebooks/deepnote/deepnoteTypes.ts around lines 1 to 3, the file is
missing the required Microsoft copyright header; add the standard TypeScript
file header (the Microsoft copyright/license comment block used across the repo)
at the very top of the file before any code, making sure to preserve existing
imports/exports and formatting.
Comment on lines
+131
to
134
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| for await (const _ of api.executeCode('bogus', token)) { | ||
| // | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Avoid disabling ESLint; drain the async iterator instead.
Suppressing the rule is fine, but a tiny helper keeps lint clean and intent explicit.
Apply:
- // eslint-disable-next-line @typescript-eslint/no-unused-vars
- for await (const _ of api.executeCode('bogus', token)) {
- //
- }
+ await drain(api.executeCode('bogus', token));Add once in this file (or a shared test util):
async function drain<T>(iter: AsyncIterable<T>) {
for await (const _ of iter) { /* no-op */ }
}🤖 Prompt for AI Agents
In src/standalone/api/kernels/kernel.unit.test.ts around lines 131 to 134,
remove the eslint-disable comment and the manual for-await loop; add a small
async drain helper (either in this test file near the top or in a shared test
util) with signature async function drain<T>(iter: AsyncIterable<T>) that
iterates and no-ops, then call await drain(api.executeCode('bogus', token)) to
consume the async iterator and keep linting and intent explicit.
Comment on lines
47
to
74
| async deserializeNotebook(content: Uint8Array, _token: CancellationToken): Promise<NotebookData> { | ||
| try { | ||
| const contentString = Buffer.from(content).toString('utf8'); | ||
| const deepnoteProject = yaml.load(contentString) as DeepnoteProject; | ||
|
|
||
| if (!deepnoteProject.project?.notebooks) { | ||
| throw new Error('Invalid Deepnote file: no notebooks found'); | ||
| } | ||
|
|
||
| // Select the notebook to open | ||
| const selectedNotebook = await this.selectNotebookForOpen( | ||
| deepnoteProject.project.id, | ||
| deepnoteProject.project.notebooks | ||
| ); | ||
|
|
Contributor
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Honor CancellationToken.
Abort early if cancelled; propagate a typed cancellation error.
- async deserializeNotebook(content: Uint8Array, _token: CancellationToken): Promise<NotebookData> {
+ async deserializeNotebook(content: Uint8Array, token: CancellationToken): Promise<NotebookData> {
try {
+ if (token?.isCancellationRequested) {
+ throw new OperationCancelledError();
+ }
const contentString = new TextDecoder('utf-8').decode(content);
const deepnoteProject = yaml.load(contentString) as DeepnoteProject;
@@
- const selectedNotebook = await this.selectNotebookForOpen(
+ const selectedNotebook = await this.selectNotebookForOpen(
deepnoteProject.project.id,
deepnoteProject.project.notebooks
);
+ if (token?.isCancellationRequested) {
+ throw new OperationCancelledError();
+ }Please confirm the exact class name exported from src/platform/errors for cancellation (e.g., OperationCancelledError). See script below to list available error classes.
🏁 Script executed:
#!/bin/bash
fd -t f src/platform/errors | xargs -I{} rg -nP 'class\s+\w+Error\b' -n {}Length of output: 381
🏁 Script executed:
#!/usr/bin/env bash
# List directories under src/platform
fd -t d src/platform
# Search for all error classes under src/platform
rg -nP 'class\s+\w+Error\b' -n src/platformLength of output: 2665
Fix missing cancellation error
OperationCancelledError isn’t defined in src/platform/errors. Import and use the existing CancellationError (e.g. from vscode-jsonrpc) or add an OperationCancelledError class in src/platform/errors.
🤖 Prompt for AI Agents
In src/notebooks/deepnote/deepnoteSerializer.ts around lines 47 to 61, the
catch/throw uses an undefined OperationCancelledError; replace it by importing
and using the existing CancellationError (for example import { CancellationError
} from 'vscode-jsonrpc') or alternatively add an OperationCancelledError class
to src/platform/errors and import that; update the throw/import so the code
throws a defined cancellation error type and remove references to the undefined
OperationCancelledError.
Comment on lines
85
to
100
| } catch (error) { | ||
| console.error('Error deserializing Deepnote notebook:', error); | ||
| throw new Error( | ||
| `Failed to parse Deepnote file: ${error instanceof Error ? error.message : 'Unknown error'}` | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Replace console.error with ILogger and preserve error cause.
Use the ILogger service and wrap errors with cause for fidelity.
- console.error('Error deserializing Deepnote notebook:', error);
- throw new Error(
- `Failed to parse Deepnote file: ${error instanceof Error ? error.message : 'Unknown error'}`
- );
+ this.logger?.error('Deepnote: deserialize failed', { error });
+ throw new DeserializationError(l10n.t('Failed to parse Deepnote file'), { cause: error });🏁 Script executed:
#!/bin/bash
# Verify ILogger and DeserializationError availability/paths
rg -nP 'interface\s+ILogger\b|class\s+DeserializationError\b' -C2Length of output: 331
🏁 Script executed:
#!/bin/bash
# Search for DeserializationError usage/definition
rg -nP 'DeserializationError' -C2
# Search for any Error subclasses in TypeScript to locate similar patterns
rg -nP 'class\s+\w*Error\b' -C2 --type tsLength of output: 20605
Use ILogger and wrap errors with WrappedError
Replace the console.error and generic Error throw with:
- console.error('Error deserializing Deepnote notebook:', error);
- throw new Error(
- `Failed to parse Deepnote file: ${error instanceof Error ? error.message : 'Unknown error'}`
- );
+ this.logger.error('Deepnote: deserialize failed', { error });
+ throw new WrappedError(l10n.t('Failed to parse Deepnote file'), error);Import WrappedError from src/platform/errors/types.ts (defined at lines 23–25) and ILogger is available in src/platform/logging/types.ts (lines 9–11).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| } catch (error) { | |
| console.error('Error deserializing Deepnote notebook:', error); | |
| throw new Error( | |
| `Failed to parse Deepnote file: ${error instanceof Error ? error.message : 'Unknown error'}` | |
| ); | |
| } | |
| } catch (error) { | |
| - console.error('Error deserializing Deepnote notebook:', error); | |
| - throw new Error( | |
| - `Failed to parse Deepnote file: ${error instanceof Error ? error.message : 'Unknown error'}` | |
| this.logger.error('Deepnote: deserialize failed', { error }); | |
| throw new WrappedError(l10n.t('Failed to parse Deepnote file'), error); | |
| } |
🤖 Prompt for AI Agents
In src/notebooks/deepnote/deepnoteSerializer.ts around lines 85 to 90, remove
the console.error and the generic Error throw and instead log via an ILogger
instance and rethrow a WrappedError that preserves the original error; import
WrappedError from src/platform/errors/types.ts and ensure an ILogger (from
src/platform/logging/types.ts) is available in scope (add as a function
parameter or use the existing logger variable), call logger.error with a
descriptive message and the original error, and then throw new
WrappedError('Failed to parse Deepnote file', error) so the original error is
wrapped and retained.
Comment on lines
154
to
182
| const fileId = projectId; | ||
| const skipPrompt = DeepnoteNotebookSerializer.manager.shouldSkipPrompt(fileId); | ||
| const storedNotebookId = DeepnoteNotebookSerializer.manager.getSelectedNotebookForUri(fileId); | ||
|
|
||
| if (notebooks.length === 1) { | ||
| return notebooks[0]; | ||
| } | ||
|
|
||
| if (skipPrompt && storedNotebookId) { | ||
| // Use the stored selection when triggered by command | ||
| const preSelected = notebooks.find((nb) => nb.id === storedNotebookId); | ||
| return preSelected || notebooks[0]; | ||
| } |
Contributor
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify using projectId as the URI key.
You’re storing selections under projectId (fileId = projectId). Ensure this won’t collide for different files sharing a project or drift across renames.
🏁 Script executed:
#!/bin/bash
# Trace callers of setSelectedNotebookForUri/getSelectedNotebookForUri and how keys are formed.
rg -nP 'setSelectedNotebookForUri\(|getSelectedNotebookForUri\(' -C2Length of output: 10547
Use the notebook’s URI as the key instead of the project ID
In deepnoteSerializer.ts (around line 154) you assign fileId = projectId and pass that into getSelectedNotebookForUri/setSelectedNotebookForUri, but the manager indexes selections by URI string—using projectId will collide across multiple files in the same project and persist across renames. Switch to the actual file URI when storing and retrieving the selected notebook.
🤖 Prompt for AI Agents
In src/notebooks/deepnote/deepnoteSerializer.ts around lines 154 to 166, the
code uses projectId as fileId when calling
DeepnoteNotebookSerializer.manager.getSelectedNotebookForUri/setSelectedNotebookForUri
which is wrong because the manager keys selections by the notebook URI; change
the key to the notebook's actual URI string (e.g., the document/uri.toString()
or the variable holding the file URI) and pass that into
getSelectedNotebookForUri and setSelectedNotebookForUri so selections are scoped
per-file (and not shared across a project or persisted across renames); also
update any corresponding stored selection lookup/assignment sites in this block
to use the same URI string.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
♻️ Duplicate comments (21)
.github/ISSUE_TEMPLATE/config.yml (1)
5-5: Grammar fixed (“worth submitting”) — resolved.
This addresses the earlier nit..github/workflows/deps.yml (2)
21-22: Pin checkout to a released major (v4).- - name: Checkout - uses: actions/checkout@v5 + - name: Checkout + uses: actions/checkout@v4
50-52: Pin checkout to a released major (v4).- - name: Checkout - uses: actions/checkout@v5 + - name: Checkout + uses: actions/checkout@v4src/notebooks/deepnote/deepnoteDataConverter.ts (8)
8-9: Fix sort stability (numeric).Lexicographic misorders a10 < a2.
- .sort((a, b) => a.sortingKey.localeCompare(b.sortingKey)) + .sort((a, b) => a.sortingKey.localeCompare(b.sortingKey, undefined, { numeric: true }))
49-53: Use nullish coalescing for executionCount.Preserves 0.
- const executionCount = cell.metadata?.executionCount || cell.executionSummary?.executionOrder; + const executionCount = cell.metadata?.executionCount ?? cell.executionSummary?.executionOrder;
73-76: Flatten output metadata; don’t nest metadata.metadata.- return new NotebookCellOutput(outputItems, { - ...(output.metadata && { metadata: output.metadata }), - ...(output.execution_count && { executionCount: output.execution_count }) - }); + return new NotebookCellOutput( + outputItems, + { + ...(output.metadata ?? {}), + ...(output.execution_count !== undefined ? { executionCount: output.execution_count } : {}) + } + );
119-123: Localize error text.- const errorText = output.text || 'Error occurred'; + const errorText = output.text || l10n.t('An error occurred while rendering output');
145-160: Support application/json and image/*; avoid forcing JSON to text.Also use web-safe base64 decode.
- if (mimeType.startsWith('application/')) { - const jsonContent = typeof content === 'string' ? content : JSON.stringify(content, null, 2); - return NotebookCellOutputItem.text(jsonContent, mimeType); - } + if (mimeType === 'application/json' || /\+json$/.test(mimeType)) { + return NotebookCellOutputItem.json(content, mimeType); + } + if (mimeType.startsWith('image/')) { + const b64 = typeof content === 'string' ? content : ''; + const bytes = this.base64ToBytes(b64); + return NotebookCellOutputItem.bytes(bytes, mimeType); + } + if (mimeType.startsWith('application/')) { + const jsonContent = typeof content === 'string' ? content : JSON.stringify(content, null, 2); + return NotebookCellOutputItem.text(jsonContent, mimeType); + }
179-184: Emit stdout/stderr mimes for streams.- if (!output.text) { - return []; - } - return [NotebookCellOutputItem.text(output.text)]; + if (!output.text) { + return []; + } + if (output.name === 'stderr') { + return [NotebookCellOutputItem.stderr(output.text)]; + } + return [NotebookCellOutputItem.stdout(output.text)];
195-201: SortingKey generator collides/misorders; use zero-padded numeric.Stable, lexicographically sortable, collision-free.
- private generateSortingKey(index: number): string { - const alphabet = 'abcdefghijklmnopqrstuvwxyz'; - const letterIndex = Math.floor(index / 100); - const letter = letterIndex < alphabet.length ? alphabet[letterIndex] : 'z'; - const number = index % 100; - return `${letter}${number}`; - } + private generateSortingKey(index: number): string { + return index.toString().padStart(8, '0'); + }
79-117: Back-conversion misses stdout/stderr/error and mishandles multi-item bundles.Preserve metadata, support VS Code mimes, aggregate items, avoid Buffer for web.
- return vscodeOutputs.map((output) => { - const deepnoteOutput: DeepnoteOutput = { - output_type: 'execute_result' - }; - - if (output.metadata?.executionCount) { - deepnoteOutput.execution_count = output.metadata.executionCount as number; - } - - if (output.items.length > 0) { - const firstItem = output.items[0]; - - if (firstItem.mime === 'text/plain') { - deepnoteOutput.output_type = 'stream'; - deepnoteOutput.text = new TextDecoder().decode(firstItem.data); - } else if (firstItem.mime === 'text/html') { - deepnoteOutput.output_type = 'execute_result'; - deepnoteOutput.data = { - 'text/html': new TextDecoder().decode(firstItem.data) - }; - } else if (firstItem.mime.startsWith('application/')) { - deepnoteOutput.output_type = 'execute_result'; - deepnoteOutput.data = { - [firstItem.mime]: new TextDecoder().decode(firstItem.data) - }; - } - - if (output.items.length > 1) { - deepnoteOutput.data = {}; - for (const item of output.items) { - deepnoteOutput.data[item.mime] = new TextDecoder().decode(item.data); - } - } - } - - return deepnoteOutput; - }); + return vscodeOutputs.map((output) => { + const deepnote: DeepnoteOutput = { output_type: 'execute_result' }; + if (output.metadata) { + deepnote.metadata = { ...output.metadata }; + if (typeof output.metadata.executionCount === 'number') { + deepnote.execution_count = output.metadata.executionCount; + delete (deepnote.metadata as any).executionCount; + } + } + if (output.items.length === 0) { + return deepnote; + } + const td = new TextDecoder(); + if (output.items.length === 1) { + const [item] = output.items; + if (item.mime === 'application/vnd.code.notebook.stdout') { + deepnote.output_type = 'stream'; + deepnote.name = 'stdout'; + deepnote.text = td.decode(item.data); + return deepnote; + } + if (item.mime === 'application/vnd.code.notebook.stderr') { + deepnote.output_type = 'stream'; + deepnote.name = 'stderr'; + deepnote.text = td.decode(item.data); + return deepnote; + } + if (item.mime === 'application/vnd.code.notebook.error') { + deepnote.output_type = 'error'; + try { + const err = JSON.parse(td.decode(item.data)); + deepnote.text = err?.message ?? td.decode(item.data); + deepnote.data = { name: err?.name, stack: err?.stack }; + } catch { + deepnote.text = td.decode(item.data); + } + return deepnote; + } + } + deepnote.output_type = output.items.length > 1 ? 'display_data' : 'execute_result'; + deepnote.data = {}; + for (const item of output.items) { + const mime = item.mime; + const isText = /^text\/|\/json$|^application\/(json|.+\+json)$/.test(mime); + if (isText) { + deepnote.data[mime] = td.decode(item.data); + } else { + deepnote.data[mime] = this.bytesToBase64(item.data); + } + } + return deepnote; + });Additional helpers to add in this class:
private bytesToBase64(bytes: Uint8Array): string { if (typeof btoa === 'function') { let bin = ''; for (let i = 0; i < bytes.length; i++) bin += String.fromCharCode(bytes[i]); return btoa(bin); } // Node fallback // eslint-disable-next-line @typescript-eslint/no-var-requires const { Buffer } = require('buffer'); return Buffer.from(bytes).toString('base64'); } private base64ToBytes(b64: string): Uint8Array { if (typeof atob === 'function') { const bin = atob(b64); const out = new Uint8Array(bin.length); for (let i = 0; i < bin.length; i++) out[i] = bin.charCodeAt(i); return out; } // Node fallback // eslint-disable-next-line @typescript-eslint/no-var-requires const { Buffer } = require('buffer'); return new Uint8Array(Buffer.from(b64, 'base64')); }src/notebooks/deepnote/deepnoteActivationService.ts (4)
40-47: Avoid YAML read/parse when project is cached.Grab projectId from notebook metadata first.
- const notebookUri = activeEditor.notebook.uri; - const rawContent = await workspace.fs.readFile(notebookUri); - const contentString = Buffer.from(rawContent).toString('utf8'); + const notebookUri = activeEditor.notebook.uri; + const projectIdFromMetadata = activeEditor.notebook.metadata?.deepnoteProjectId as string | undefined; + let deepnoteProject: DeepnoteProject | undefined; + if (projectIdFromMetadata) { + deepnoteProject = this.serializer.getManager().getOriginalProject(projectIdFromMetadata); + } + const contentString = + deepnoteProject ? '' : new TextDecoder('utf-8').decode(await workspace.fs.readFile(notebookUri));And below reuse deepnoteProject if present; otherwise parse YAML as you do now.
71-113: Key consistency: persist selection under projectId (serializer looks up by projectId).- const fileUri = notebookUri.toString(); const projectId = deepnoteProject.project.id; const manager = this.serializer.getManager(); - manager.setSelectedNotebookForUri(fileUri, selectedNotebook.id); + manager.setSelectedNotebookForUri(projectId, selectedNotebook.id); // Update the current notebook ID for serialization manager.storeOriginalProject( projectId, - manager.getOriginalProject(projectId) || deepnoteProject, + manager.getOriginalProject(projectId) ?? deepnoteProject, selectedNotebook.id );
17-20: Prefer DI over new for serializer/selector.Improves testability and adheres to “inject interfaces, not concretes.”
- public activate() { - this.serializer = new DeepnoteNotebookSerializer(); - this.selector = new DeepnoteNotebookSelector(); + public activate() { + this.serializer = this.serializer ?? new DeepnoteNotebookSerializer(); + this.selector = this.selector ?? new DeepnoteNotebookSelector();If desired, I can sketch minimal interfaces and container bindings.
31-38: Use real NotebookCellData in the converter
In src/notebooks/deepnote/deepnoteDataConverter.ts, update convertBlocksToCells to instantiate each cell withnew NotebookCellData(...)instead of returning plain objects.src/notebooks/deepnote/deepnoteSerializer.ts (6)
33-37: Use TextDecoder; Buffer breaks on web.- const contentString = Buffer.from(content).toString('utf8'); + const contentString = new TextDecoder('utf-8').decode(content);
92-96: Localize and type the “notebook not found” error.- if (notebookIndex === -1) { - throw new Error(`Notebook with ID ${notebookId} not found in project`); - } + if (notebookIndex === -1) { + throw new NotFoundError(l10n.t('Notebook with ID {0} not found in project', notebookId)); + }
76-90: Localize and type thrown errors in serialize preconditions.- if (!projectId) { - throw new Error('Missing Deepnote project ID in notebook metadata'); - } + if (!projectId) { + throw new FileFormatError(l10n.t('Missing Deepnote project ID in notebook metadata')); + } @@ - if (!originalProject) { - throw new Error('Original Deepnote project not found. Cannot save changes.'); - } + if (!originalProject) { + throw new NotFoundError(l10n.t('Original Deepnote project not found. Cannot save changes.')); + } @@ - if (!notebookId) { - throw new Error('Cannot determine which notebook to save'); - } + if (!notebookId) { + throw new FileFormatError(l10n.t('Cannot determine which notebook to save')); + }
123-127: Replace console.error with ILogger and throw typed SerializationError.- console.error('Error serializing Deepnote notebook:', error); - throw new Error( - `Failed to save Deepnote file: ${error instanceof Error ? error.message : 'Unknown error'}` - ); + this.logger?.error('Deepnote: serialize failed', { error }); + throw new SerializationError(l10n.t('Failed to save Deepnote file'), { cause: error });
33-71: Use platform errors and ILogger
- Replace generic
Errorthrows with a domain error (e.g.WrappedErrororBaseErrorfromsrc/platform/errors/types.ts).- Inject and use
ILogger(import fromsrc/platform/logging/types.ts) instead ofconsole.error.
66-71: Replace console.error with ILogger and preserve cause.- console.error('Error deserializing Deepnote notebook:', error); - - throw new Error( - `Failed to parse Deepnote file: ${error instanceof Error ? error.message : 'Unknown error'}` - ); + this.logger?.error('Deepnote: deserialize failed', { error }); + throw new DeserializationError(l10n.t('Failed to parse Deepnote file'), { cause: error });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
.github/ISSUE_TEMPLATE/config.yml(1 hunks).github/workflows/ci.yml(1 hunks).github/workflows/deps.yml(1 hunks)src/notebooks/deepnote/deepnoteActivationService.ts(1 hunks)src/notebooks/deepnote/deepnoteDataConverter.ts(1 hunks)src/notebooks/deepnote/deepnoteSerializer.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/!(*.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/notebooks/deepnote/deepnoteActivationService.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
Files:
src/notebooks/deepnote/deepnoteDataConverter.tssrc/notebooks/deepnote/deepnoteActivationService.tssrc/notebooks/deepnote/deepnoteSerializer.ts
🧠 Learnings (15)
📚 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/deepnoteSerializer.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 : On conversion failures, provide detailed error reporting with suggested fixes
Applied to files:
src/notebooks/deepnote/deepnoteDataConverter.tssrc/notebooks/deepnote/deepnoteSerializer.ts
📚 Learning: 2025-09-09T11:31:30.442Z
Learnt from: Artmann
PR: deepnote/vscode-extension#11
File: src/notebooks/deepnote/deepnoteActivationService.ts:1-3
Timestamp: 2025-09-09T11:31:30.442Z
Learning: New files in the Deepnote VS Code extension project should not include copyright headers.
Applied to files:
src/notebooks/deepnote/deepnoteDataConverter.tssrc/notebooks/deepnote/deepnoteActivationService.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/**/*.ts : Notebook-level execution must enqueue cells via CellExecutionQueue and manage per-cell lifecycles through CellExecution
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 `l10n.t()` for user-facing strings
Applied to files:
src/notebooks/deepnote/deepnoteDataConverter.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} : Prefer interface over type where possible; reserve type for unions/literals
Applied to files:
src/notebooks/deepnote/deepnoteActivationService.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/serviceRegistry.{node,web}.ts : Use dependency injection for services to enable testability and platform variance
Applied to files:
src/notebooks/deepnote/deepnoteActivationService.tssrc/notebooks/deepnote/deepnoteSerializer.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/serviceRegistry.{node,web}.ts : Use dependency injection via the service registry for component creation and registration
Applied to files:
src/notebooks/deepnote/deepnoteActivationService.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/**/*.ts : Use dependency injection to keep components testable
Applied to files:
src/notebooks/deepnote/deepnoteActivationService.tssrc/notebooks/deepnote/deepnoteSerializer.ts
📚 Learning: 2025-09-03T13:00:18.307Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/platform.instructions.md:0-0
Timestamp: 2025-09-03T13:00:18.307Z
Learning: Applies to src/platform/**/*.ts : Use Inversify decorators for DI: annotate classes with injectable() and inject dependencies with inject(Token)
Applied to files:
src/notebooks/deepnote/deepnoteActivationService.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} : Inject interfaces, not concrete classes
Applied to files:
src/notebooks/deepnote/deepnoteSerializer.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/{connection,launcher,session,finder}/**/*.ts : Support cancellation via CancellationToken (or equivalent) for long-running async operations (network calls, startup waits)
Applied to files:
src/notebooks/deepnote/deepnoteSerializer.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} : Prefer async/await and handle cancellation with `CancellationToken`
Applied to files:
src/notebooks/deepnote/deepnoteSerializer.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/**/*.ts : Respect CancellationToken in async operations and support cancellation in long-running tasks
Applied to files:
src/notebooks/deepnote/deepnoteSerializer.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/**/*.ts : Respect CancellationToken in all async operations
Applied to files:
src/notebooks/deepnote/deepnoteSerializer.ts
🧬 Code graph analysis (3)
src/notebooks/deepnote/deepnoteDataConverter.ts (2)
src/notebooks/deepnote/deepnoteTypes.ts (2)
DeepnoteBlock(24-33)DeepnoteOutput(35-42)src/test/mocks/vsc/extHostedTypes.ts (3)
NotebookCellData(2523-2549)NotebookCellOutput(71-103)NotebookCellOutputItem(16-69)
src/notebooks/deepnote/deepnoteActivationService.ts (4)
src/notebooks/deepnote/deepnoteSerializer.ts (3)
DeepnoteNotebookSerializer(15-167)DeepnoteProject(8-8)DeepnoteNotebook(8-8)src/notebooks/deepnote/deepnoteNotebookSelector.ts (1)
DeepnoteNotebookSelector(10-72)src/notebooks/deepnote/deepnoteTypes.ts (2)
DeepnoteProject(1-13)DeepnoteNotebook(15-22)src/test/mocks/vsc/extHostedTypes.ts (3)
WorkspaceEdit(649-833)NotebookEdit(2444-2507)NotebookRange(2413-2443)
src/notebooks/deepnote/deepnoteSerializer.ts (4)
src/notebooks/deepnote/deepnoteTypes.ts (2)
DeepnoteNotebook(15-22)DeepnoteProject(1-13)src/notebooks/deepnote/deepnoteNotebookManager.ts (1)
DeepnoteNotebookManager(3-53)src/notebooks/deepnote/deepnoteNotebookSelector.ts (1)
DeepnoteNotebookSelector(10-72)src/notebooks/deepnote/deepnoteDataConverter.ts (1)
DeepnoteDataConverter(5-202)
⏰ 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: Lint & Format
🔇 Additional comments (4)
.github/ISSUE_TEMPLATE/config.yml (1)
4-4: Help link now points to repo Discussions — LGTM.
Accurate destination for support..github/workflows/ci.yml (1)
18-20: Good: workflow concurrency added.Prevents wasted CI on rapid pushes.
src/notebooks/deepnote/deepnoteDataConverter.ts (1)
16-33: Sanity check passed: no callers depend on plain-object cells. All sites consume the returned NotebookCellData shape without relying on plain-object semantics.src/notebooks/deepnote/deepnoteSerializer.ts (1)
130-166: No action needed;projectIdis consistently used as the selection key in bothdeepnoteSerializeranddeepnoteActivationService.
Comment on lines
+59
to
+64
| - name: Install dependencies | ||
| run: npm ci --prefer-offline --no-audit | ||
|
|
||
| - name: Run postinstall | ||
| run: npm run postinstall | ||
|
|
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid double-running postinstall here too.
Either remove the step or skip scripts during npm ci.
Option A (remove):
- name: Install dependencies
- run: npm ci --prefer-offline --no-audit
+ run: npm ci --prefer-offline --no-audit
-
- - name: Run postinstall
- run: npm run postinstallOption B (skip scripts, keep explicit):
- - name: Install dependencies
- run: npm ci --prefer-offline --no-audit
+ - name: Install dependencies (no scripts)
+ run: npm ci --prefer-offline --no-audit --ignore-scripts🤖 Prompt for AI Agents
In .github/workflows/deps.yml around lines 59-64 the workflow runs postinstall
twice (npm ci runs lifecycle scripts and then there is an explicit npm run
postinstall step); either remove the explicit "Run postinstall" step entirely,
or change the "Install dependencies" step to skip lifecycle scripts by adding
the ignore-scripts flag (use npm ci --ignore-scripts --prefer-offline
--no-audit) and keep the explicit postinstall step — pick one option and apply
the corresponding change so postinstall only runs once.
Comment on lines
15
to
45
| export class DeepnoteNotebookSerializer implements NotebookSerializer { | ||
| private manager = new DeepnoteNotebookManager(); | ||
| private selector = new DeepnoteNotebookSelector(); | ||
| private converter = new DeepnoteDataConverter(); | ||
| private notebookSelectionCallback?: NotebookSelectionCallback; | ||
|
|
||
| getManager(): DeepnoteNotebookManager { | ||
| return this.manager; | ||
| } | ||
|
|
||
| getConverter(): DeepnoteDataConverter { | ||
| return this.converter; | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Inject interfaces for manager/selector/converter (avoid hard-coded singletons).
Eases testing and platform variance.
I can provide interfaces and bindings if you want to proceed.
🤖 Prompt for AI Agents
In src/notebooks/deepnote/deepnoteSerializer.ts around lines 15 to 27, the class
currently instantiates concrete DeepnoteNotebookManager/Selector/DataConverter
directly which makes testing and platform swaps hard; change the class to accept
manager, selector and converter via constructor parameters typed to their
interfaces (provide defaults to the existing concrete classes to preserve
current behavior), store them on the instance, and update
getManager/getConverter to return the injected instances; ensure types use
interfaces (not concrete classes) and update any instantiation sites to pass in
dependencies or rely on defaults.
Comment on lines
+38
to
+70
| if (!deepnoteProject.project?.notebooks) { | ||
| throw new Error('Invalid Deepnote file: no notebooks found'); | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Localize and type errors.
- if (!deepnoteProject.project?.notebooks) {
- throw new Error('Invalid Deepnote file: no notebooks found');
- }
+ if (!deepnoteProject.project?.notebooks) {
+ throw new FileFormatError(l10n.t('Invalid Deepnote file: no notebooks found'));
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (!deepnoteProject.project?.notebooks) { | |
| throw new Error('Invalid Deepnote file: no notebooks found'); | |
| } | |
| if (!deepnoteProject.project?.notebooks) { | |
| throw new FileFormatError(l10n.t('Invalid Deepnote file: no notebooks found')); | |
| } |
🤖 Prompt for AI Agents
In src/notebooks/deepnote/deepnoteSerializer.ts around lines 38–41, replace the
raw throw new Error('Invalid Deepnote file: no notebooks found') with a typed,
localizable error: perform a strict runtime check (e.g., ensure project exists
and notebooks is an Array and has length), then throw a domain-specific error
class (e.g., InvalidDeepnoteError) instantiated with a localized message from
your i18n utility (e.g., t('deepnote.errors.noNotebooks') or localize(...)); if
the InvalidDeepnoteError class does not exist, add it to your errors module
extending Error (including a stable error code property) and update types so the
check narrows notebooks to the expected Notebook[] before proceeding.
Comment on lines
46
to
77
| if (!selectedNotebook) { | ||
| throw new Error('No notebook selected'); | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid bare “No notebook selected”; localize and type.
- if (!selectedNotebook) {
- throw new Error('No notebook selected');
- }
+ if (!selectedNotebook) {
+ throw new OperationCancelledError(l10n.t('No notebook selected'));
+ }If OperationCancelledError isn’t available, use the project’s cancellation error type.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/notebooks/deepnote/deepnoteSerializer.ts around lines 46 to 48, replace
the bare throw new Error('No notebook selected') with the project’s cancellation
error type (e.g., OperationCancelledError or the repo-specific cancellation
error) and use a localized message key rather than a hard-coded string; import
the cancellation error type if missing and throw new
OperationCancelledError(localize('noNotebookSelected')) or the equivalent
constructor for the project error type so the error is strongly typed and the
message is localizable.
| } | ||
| } | ||
|
|
||
| async serializeNotebook(data: NotebookData, _token: CancellationToken): Promise<Uint8Array> { |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Honor CancellationToken in (de)serialize.
- async serializeNotebook(data: NotebookData, _token: CancellationToken): Promise<Uint8Array> {
+ async serializeNotebook(data: NotebookData, token: CancellationToken): Promise<Uint8Array> {
+ if (token?.isCancellationRequested) {
+ throw new OperationCancelledError();
+ }Make the same change in deserialize and add mid-operation checks before costly work.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/notebooks/deepnote/deepnoteSerializer.ts around line 74, the
serializeNotebook method (and similarly deserialize) currently ignores the
CancellationToken; update both methods to check the token before starting and at
key mid-operation points (especially before any expensive conversions or I/O) by
testing token.isCancellationRequested and aborting promptly (throw a
CancellationError or return a rejected Promise with an appropriate cancellation
signal). Add these checks immediately on method entry and right before any
costly loops, async calls or data transformations so the operation can be
cancelled mid-work.
Artmann
force-pushed
the
open-deepnote-file
branch
from
7079885 to
5bc6bf1
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
♻️ Duplicate comments (31)
src/standalone/api/kernels/kernel.unit.test.ts (1)
131-134: Avoid disabling ESLint; drain the async iterator instead.Replace the loop with a small drain helper to keep lint clean and intent explicit.
- // eslint-disable-next-line @typescript-eslint/no-unused-vars - for await (const _ of api.executeCode('bogus', token)) { - // - } + await drain(api.executeCode('bogus', token));Add once in this file (or a shared test util):
async function drain<T>(iter: AsyncIterable<T>) { for await (const _ of iter) { /* no-op */ } }CLAUDE.md (1)
1-3: Fix markdownlint and align header policy
- Add H1, required blank lines, trailing newline.
- Update guidance to require the Microsoft header in TS files (per repo standards).
+# Project Conventions + ## Code Style & Organization -- Order method, fields and properties, first by accessibility and then by alphabetical order. -- Don't add the Microsoft copyright header to new files. +- Order methods, fields and properties first by accessibility and then alphabetically. +- Add the Microsoft copyright header to new TypeScript source files. ## Testing + ## Project Structure + ## Deepnote Integration +Also applies to: 5-5, 13-13, 19-19, 29-29
src/notebooks/deepnote/deepnoteTypes.ts (1)
1-3: Add required copyright headerAll TS files should start with the Microsoft header.
+// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + export interface DeepnoteProject {package.json (1)
320-325: Localize command title; keep “Deepnote” category (per preference)Use an l10n key for the command title and add the string in package.nls.json.
{ "command": "jupyter.selectDeepnoteNotebook", - "title": "Select Notebook", + "title": "%deepnote.command.selectNotebook.title%", "category": "Deepnote", "enablement": "notebookType == 'deepnote'" }.github/workflows/ci.yml (3)
10-12: Remove unused NPM_VERSION or activate itCurrently dead config.
env: NODE_VERSION: 22.x - NPM_VERSION: 10.x
39-41: Don’t run postinstall twicenpm ci already runs lifecycle scripts; drop the extra steps.
- - name: Run postinstall - run: npm run postinstall @@ - - name: Run postinstall - run: npm run postinstallAlso applies to: 64-66
23-25: Add job timeoutsPrevents hung runs.
lint: name: Lint & Format runs-on: ubuntu-latest + timeout-minutes: 15 @@ build: name: Build & Test runs-on: ubuntu-latest + timeout-minutes: 30Also applies to: 48-51
.github/workflows/deps.yml (4)
21-22: Pin checkout to a released major.actions/checkout@v5 isn’t a published major. Use @v4 (or a SHA) in both jobs.
- - name: Checkout - uses: actions/checkout@v5 + - name: Checkout + uses: actions/checkout@v4Also applies to: 51-52
12-15: Add workflow concurrency to cancel superseded runs.Prevents overlapping scheduled/dispatch runs.
permissions: contents: read actions: read +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true
59-64: Avoid double-running postinstall.Either skip scripts during ci or drop the explicit postinstall. Prefer skipping here.
- - name: Install dependencies - run: npm ci --prefer-offline --no-audit + - name: Install dependencies (no scripts) + run: npm ci --prefer-offline --no-audit --ignore-scripts
65-66: Make dependency check non-blocking on schedule.Only soften failures for scheduled runs.
- - name: Check dependencies for issues - run: npm run checkDependencies + - name: Check dependencies for issues + run: npm run checkDependencies + continue-on-error: ${{ github.event_name == 'schedule' }}src/notebooks/deepnote/deepnoteNotebookSelector.unit.test.ts (6)
1-1: Localize expected strings in tests.Hard-coded English will break under non-English locales. Build expectations with l10n.t like production.
import * as assert from 'assert'; +import { l10n } from 'vscode';
35-46: Localize description assertions (current vs non-current).- assert.strictEqual(description, '1 cells (current)'); + assert.strictEqual(description, l10n.t('{0} cells (current)', 1)); @@ - assert.strictEqual(description, '2 cells'); + assert.strictEqual(description, l10n.t('{0} cells', 2));
48-60: Localize zero-cells case.- assert.strictEqual(description, '0 cells'); + assert.strictEqual(description, l10n.t('{0} cells', 0));
62-66: Localize generic cell-count case.- assert.strictEqual(description, '2 cells'); + assert.strictEqual(description, l10n.t('{0} cells', 2));
70-99: Localize detail assertions.- assert.strictEqual(detail, 'ID: notebook-1 | Working Directory: /home/user'); + assert.strictEqual(detail, l10n.t('ID: {0} | Working Directory: {1}', 'notebook-1', '/home/user')); @@ - assert.strictEqual(detail, 'ID: notebook-2'); + assert.strictEqual(detail, l10n.t('ID: {0}', 'notebook-2')); @@ - assert.strictEqual(detail, 'ID: notebook-1'); + assert.strictEqual(detail, l10n.t('ID: {0}', 'notebook-1')); @@ - assert.strictEqual(detail1, 'ID: notebook-1 | Working Directory: /home/user'); - assert.strictEqual(detail2, 'ID: notebook-2'); + assert.strictEqual(detail1, l10n.t('ID: {0} | Working Directory: {1}', 'notebook-1', '/home/user')); + assert.strictEqual(detail2, l10n.t('ID: {0}', 'notebook-2'));
34-67: Tests reach into privates via any-cast.Prefer exercising public behavior (e.g., build items through selectNotebook with a stubbed QuickPick) to avoid brittle private API coupling.
Also applies to: 69-101, 102-146
src/notebooks/deepnote/deepnoteDataConverter.unit.test.ts (2)
130-230: Add reverse output conversion tests (cells ➜ blocks).Current suite doesn’t validate outputs in convertCellsToBlocks. Add cases for stream, multi-mime execute_result, and error.
@@ suite('convertCellsToBlocks', () => { + test('converts stream/plain-text output back to block', () => { + const cells: any[] = [ + { + kind: NotebookCellKind.Code, + value: 'print("x")', + languageId: 'python', + outputs: [ + { + items: [ + { mime: 'application/vnd.code.notebook.stdout', data: new TextEncoder().encode('hi\n') } + ] + } + ] + } + ]; + const blocks = converter.convertCellsToBlocks(cells as unknown as NotebookCellData[]); + assert.strictEqual(blocks[0].outputs?.[0].output_type, 'stream'); + assert.strictEqual(blocks[0].outputs?.[0].text, 'hi\n'); + }); + + test('converts multi-mime execute_result back to block', () => { + const cells: any[] = [ + { + kind: NotebookCellKind.Code, + value: 'x', + languageId: 'python', + outputs: [ + { + metadata: { executionCount: 1, custom: 'metadata' }, + items: [ + { mime: 'text/plain', data: new TextEncoder().encode('42') }, + { mime: 'text/html', data: new TextEncoder().encode('<div>42</div>') }, + { mime: 'application/json', data: new TextEncoder().encode(JSON.stringify({ value: 42 })) } + ] + } + ] + } + ]; + const blocks = converter.convertCellsToBlocks(cells as unknown as NotebookCellData[]); + const out = blocks[0].outputs?.[0]!; + assert.strictEqual(out.output_type, 'execute_result'); + assert.strictEqual(out.execution_count, 1); + assert.deepStrictEqual(out.data && Object.keys(out.data).sort(), ['application/json', 'text/html', 'text/plain']); + }); + + test('converts error output back to block', () => { + const error = { name: 'NameError', message: "name 'x' is not defined", stack: ['Traceback...'] }; + const cells: any[] = [ + { + kind: NotebookCellKind.Code, + value: 'x', + languageId: 'python', + outputs: [ + { + items: [ + { mime: 'application/vnd.code.notebook.error', data: new TextEncoder().encode(JSON.stringify(error)) } + ] + } + ] + } + ]; + const blocks = converter.convertCellsToBlocks(cells as unknown as NotebookCellData[]); + assert.strictEqual(blocks[0].outputs?.[0].output_type, 'error'); + });
425-456: Add round-trip with rich outputs.Extend round-trip to include multi-mime display/execute_result to catch asymmetries.
@@ suite('round trip conversion', () => { - test('blocks -> cells -> blocks preserves data', () => { + test('blocks -> cells -> blocks preserves data (stream)', () => { @@ }); + + test('blocks -> cells -> blocks preserves data (rich multi-mime)', () => { + const originalBlocks: DeepnoteBlock[] = [ + { + id: 'b3', + type: 'code', + content: 'x', + sortingKey: 'a2', + outputs: [ + { + output_type: 'execute_result', + execution_count: 7, + metadata: { custom: 'm' }, + data: { + 'text/plain': '42', + 'text/html': '<b>42</b>' + } + } + ] + } + ]; + const cells = converter.convertBlocksToCells(originalBlocks); + const roundTripBlocks = converter.convertCellsToBlocks(cells); + assert.deepStrictEqual(roundTripBlocks, originalBlocks); + });src/notebooks/deepnote/deepnoteNotebookSelector.ts (3)
10-18: Support cancellation.Accept a CancellationToken and hide picker when canceled.
-import { l10n, type QuickPickItem, window } from 'vscode'; +import { l10n, type QuickPickItem, window, type CancellationToken } from 'vscode'; @@ - options?: { + options?: { title?: string; placeHolder?: string; + token?: CancellationToken; } @@ - let accepted = false; + let accepted = false; + const cancelSub = options?.token?.onCancellationRequested(() => quickPick.hide()); quickPick.show(); @@ - const selectedItem = accepted ? quickPick.selectedItems[0] : undefined; + const selectedItem = accepted ? quickPick.selectedItems[0] : undefined; quickPick.dispose(); + cancelSub?.dispose();Also applies to: 41-51
26-33: Improve QuickPick search UX.Enable matching on description/detail.
const quickPick = window.createQuickPick<NotebookQuickPickItem>(); quickPick.items = items; + quickPick.matchOnDescription = true; + quickPick.matchOnDetail = true;
55-63: Pluralize “cell(s)”.Use singular for 1.
- const cellCount = notebook.blocks.length; - - if (notebook.id === currentNotebookId) { - return l10n.t('{0} cells (current)', cellCount); - } - - return l10n.t('{0} cells', cellCount); + const cellCount = notebook.blocks.length; + const base = cellCount === 1 ? l10n.t('{0} cell', cellCount) : l10n.t('{0} cells', cellCount); + return notebook.id === currentNotebookId ? l10n.t('{0} (current)', base) : base;src/notebooks/deepnote/deepnoteDataConverter.ts (2)
8-9: Stable, collision-free ordering for sortingKey.Current scheme misorders (a10 < a2) and collides after ~2700. Use numeric sort + zero-padded keys.
- .sort((a, b) => a.sortingKey.localeCompare(b.sortingKey)) + .sort((a, b) => a.sortingKey.localeCompare(b.sortingKey, undefined, { numeric: true }))- private generateSortingKey(index: number): string { - const alphabet = 'abcdefghijklmnopqrstuvwxyz'; - const letterIndex = Math.floor(index / 100); - const letter = letterIndex < alphabet.length ? alphabet[letterIndex] : 'z'; - const number = index % 100; - return `${letter}${number}`; - } + private generateSortingKey(index: number): string { + return index.toString().padStart(8, '0'); + }Also applies to: 385-391
50-53: executionCount truthiness bug (0 lost).0 is valid; use nullish coalescing.
- const executionCount = cell.metadata?.executionCount || cell.executionSummary?.executionOrder; + const executionCount = cell.metadata?.executionCount ?? cell.executionSummary?.executionOrder;src/notebooks/deepnote/deepnoteSerializer.ts (6)
33-37: Web-safe decoding: use TextDecoder (Buffer not available in web).- const contentString = Buffer.from(content).toString('utf8'); + const contentString = new TextDecoder('utf-8').decode(content);
15-27: Inject interfaces, not concrete classes (manager/selector/converter).Aids testing and platform variance.
33-33: Honor CancellationToken.Check token early and mid-operation.
- async deserializeNotebook(content: Uint8Array, _token: CancellationToken): Promise<NotebookData> { + async deserializeNotebook(content: Uint8Array, token: CancellationToken): Promise<NotebookData> { try { + if (token?.isCancellationRequested) throw new OperationCancelledError();- async serializeNotebook(data: NotebookData, _token: CancellationToken): Promise<Uint8Array> { + async serializeNotebook(data: NotebookData, token: CancellationToken): Promise<Uint8Array> { try { + if (token?.isCancellationRequested) throw new OperationCancelledError();Also applies to: 74-74
1-3: Type-only import for interfaces.-import { DeepnoteProject, DeepnoteNotebook } from './deepnoteTypes'; +import type { DeepnoteProject, DeepnoteNotebook } from './deepnoteTypes';
38-41: Use typed, localized errors (no raw Error).- if (!deepnoteProject.project?.notebooks) { - throw new Error('Invalid Deepnote file: no notebooks found'); - } + if (!deepnoteProject.project?.notebooks) { + throw new FileFormatError(l10n.t('Invalid Deepnote file: no notebooks found')); + } @@ - if (!selectedNotebook) { - throw new Error('No notebook selected'); - } + if (!selectedNotebook) { + throw new OperationCancelledError(l10n.t('No notebook selected')); + }Note: import FileFormatError/OperationCancelledError from src/platform/errors and l10n if not already.
Also applies to: 46-48
66-71: Replace console.error and wrap cause in typed error.- } catch (error) { - console.error('Error deserializing Deepnote notebook:', error); - - throw new Error( - `Failed to parse Deepnote file: ${error instanceof Error ? error.message : 'Unknown error'}` - ); + } catch (error) { + this.logger?.error('Deepnote: deserialize failed', { error }); + throw new DeserializationError(l10n.t('Failed to parse Deepnote file'), { cause: error }); }Inject ILogger (this.logger) via constructor/DI.
src/notebooks/deepnote/deepnoteActivationService.ts (1)
41-45: Web-safe decoding: use TextDecoder (Buffer not available in web).- const rawContent = await workspace.fs.readFile(notebookUri); - const contentString = Buffer.from(rawContent).toString('utf8'); + const rawContent = await workspace.fs.readFile(notebookUri); + const contentString = new TextDecoder('utf-8').decode(rawContent);
📜 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 (24)
.github/ISSUE_TEMPLATE/2_bug_form.yml(1 hunks).github/ISSUE_TEMPLATE/config.yml(1 hunks).github/workflows/aggregate-test-results.yml(0 hunks).github/workflows/build-test.yml(0 hunks).github/workflows/ci.yml(1 hunks).github/workflows/codeql-analysis.yml(0 hunks).github/workflows/deps.yml(1 hunks).github/workflows/lock.yml(0 hunks)CLAUDE.md(1 hunks)package.json(8 hunks)package.nls.json(1 hunks)src/notebooks/deepnote/deepnoteActivationService.ts(1 hunks)src/notebooks/deepnote/deepnoteDataConverter.ts(1 hunks)src/notebooks/deepnote/deepnoteDataConverter.unit.test.ts(1 hunks)src/notebooks/deepnote/deepnoteNotebookManager.ts(1 hunks)src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts(1 hunks)src/notebooks/deepnote/deepnoteNotebookSelector.ts(1 hunks)src/notebooks/deepnote/deepnoteNotebookSelector.unit.test.ts(1 hunks)src/notebooks/deepnote/deepnoteSerializer.ts(1 hunks)src/notebooks/deepnote/deepnoteTypes.ts(1 hunks)src/notebooks/serviceRegistry.node.ts(2 hunks)src/notebooks/serviceRegistry.web.ts(2 hunks)src/platform/common/constants.ts(1 hunks)src/standalone/api/kernels/kernel.unit.test.ts(1 hunks)
💤 Files with no reviewable changes (4)
- .github/workflows/lock.yml
- .github/workflows/aggregate-test-results.yml
- .github/workflows/codeql-analysis.yml
- .github/workflows/build-test.yml
🧰 Additional context used
📓 Path-based instructions (8)
**/!(*.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/platform/common/constants.tssrc/notebooks/deepnote/deepnoteNotebookSelector.unit.test.tssrc/notebooks/deepnote/deepnoteNotebookManager.unit.test.tssrc/notebooks/deepnote/deepnoteNotebookManager.tssrc/notebooks/deepnote/deepnoteTypes.tssrc/notebooks/deepnote/deepnoteDataConverter.unit.test.tssrc/notebooks/deepnote/deepnoteSerializer.tssrc/notebooks/deepnote/deepnoteNotebookSelector.tssrc/standalone/api/kernels/kernel.unit.test.tssrc/notebooks/deepnote/deepnoteActivationService.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
Files:
src/platform/common/constants.tssrc/notebooks/deepnote/deepnoteNotebookSelector.unit.test.tssrc/notebooks/deepnote/deepnoteNotebookManager.unit.test.tssrc/notebooks/deepnote/deepnoteNotebookManager.tssrc/notebooks/deepnote/deepnoteTypes.tssrc/notebooks/serviceRegistry.node.tssrc/notebooks/deepnote/deepnoteDataConverter.unit.test.tssrc/notebooks/deepnote/deepnoteSerializer.tssrc/notebooks/deepnote/deepnoteNotebookSelector.tssrc/standalone/api/kernels/kernel.unit.test.tssrc/notebooks/serviceRegistry.web.tssrc/notebooks/deepnote/deepnoteActivationService.tssrc/notebooks/deepnote/deepnoteDataConverter.ts
src/platform/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/platform.instructions.md)
src/platform/**/*.ts: Use Inversify decorators for DI: annotate classes with @Injectable() and inject dependencies with @Inject(Token)
Use the centralized logger (import { logger } from '../platform/logging') instead of console.log for application logging
Files:
src/platform/common/constants.ts
**/*.unit.test.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place unit tests in files matching
*.unit.test.ts
Files:
src/notebooks/deepnote/deepnoteNotebookSelector.unit.test.tssrc/notebooks/deepnote/deepnoteNotebookManager.unit.test.tssrc/notebooks/deepnote/deepnoteDataConverter.unit.test.tssrc/standalone/api/kernels/kernel.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/deepnoteNotebookSelector.unit.test.tssrc/notebooks/deepnote/deepnoteNotebookManager.unit.test.tssrc/notebooks/deepnote/deepnoteDataConverter.unit.test.tssrc/standalone/api/kernels/kernel.unit.test.ts
**/*.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/serviceRegistry.node.ts
src/standalone/api/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/standalone.instructions.md)
Define new API endpoint interfaces within the appropriate subdirectory under src/standalone/api/
Files:
src/standalone/api/kernels/kernel.unit.test.ts
**/*.web.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
*.web.tsfor Web-specific implementations using browser-compatible APIs
Files:
src/notebooks/serviceRegistry.web.ts
🧠 Learnings (39)
📓 Common learnings
Learnt from: Artmann
PR: deepnote/vscode-extension#11
File: package.json:320-325
Timestamp: 2025-09-09T13:50:51.031Z
Learning: The user prefers to have a dedicated "Deepnote" category in package.json commands rather than grouping Deepnote commands under the existing "Jupyter" category.
📚 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 : Add UI elements for the new format to the export quick pick
Applied to files:
src/platform/common/constants.tssrc/notebooks/deepnote/deepnoteNotebookSelector.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/**/*.ts : Add unit and integration tests for new features and modifications; include performance and cross-platform test coverage
Applied to files:
src/notebooks/deepnote/deepnoteNotebookSelector.unit.test.tssrc/notebooks/deepnote/deepnoteNotebookManager.unit.test.tsCLAUDE.mdsrc/notebooks/deepnote/deepnoteDataConverter.unit.test.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 **/*.unit.test.ts : Place unit tests in files matching `*.unit.test.ts`
Applied to files:
src/notebooks/deepnote/deepnoteNotebookSelector.unit.test.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/controllers/kernelSource/**/*.ts : For new kernel sources, implement ILocalNotebookKernelSourceSelector (or similar)
Applied to files:
src/notebooks/deepnote/deepnoteNotebookSelector.unit.test.tssrc/notebooks/deepnote/deepnoteNotebookSelector.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 **/!(*.unit).test.ts : Place integration tests in `*.test.ts` files that are not `*.unit.test.ts`
Applied to files:
src/notebooks/deepnote/deepnoteNotebookSelector.unit.test.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/notebooks/deepnote/deepnoteNotebookSelector.unit.test.tssrc/notebooks/deepnote/deepnoteDataConverter.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 **/*.{spec,e2e}.{ts,tsx} : Avoid defining test-specific values outside individual test cases
Applied to files:
src/notebooks/deepnote/deepnoteNotebookSelector.unit.test.ts
📚 Learning: 2025-09-03T13:01:49.096Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-09-03T13:01:49.096Z
Learning: Applies to **/*.{test,spec}.ts : In unit tests, when a mock is returned from a promise, ensure the mocked instance has an undefined `then` property to avoid hanging tests
Applied to files:
src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts
📚 Learning: 2025-08-25T14:42:02.289Z
Learnt from: CR
PR: deepnote/deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-25T14:42:02.289Z
Learning: Follow the codebase conventions documented in CONVENTIONS.md
Applied to files:
CLAUDE.md
📚 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/debugger/**/*.ts : Add the new debugging controller to DebuggingManager creation logic
Applied to files:
src/notebooks/deepnote/deepnoteNotebookManager.ts
📚 Learning: 2025-09-09T11:31:30.442Z
Learnt from: Artmann
PR: deepnote/vscode-extension#11
File: src/notebooks/deepnote/deepnoteActivationService.ts:1-3
Timestamp: 2025-09-09T11:31:30.442Z
Learning: New files in the Deepnote VS Code extension project should not include copyright headers.
Applied to files:
src/notebooks/deepnote/deepnoteNotebookManager.tssrc/notebooks/deepnote/deepnoteTypes.tssrc/notebooks/deepnote/deepnoteNotebookSelector.tssrc/notebooks/deepnote/deepnoteActivationService.tssrc/notebooks/deepnote/deepnoteDataConverter.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/notebooks/serviceRegistry.node.tssrc/notebooks/serviceRegistry.web.ts
📚 Learning: 2025-09-03T13:01:10.017Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/standalone.instructions.md:0-0
Timestamp: 2025-09-03T13:01:10.017Z
Learning: Applies to src/standalone/serviceRegistry.{node,web}.ts : Register new services for both platforms in serviceRegistry.node.ts and serviceRegistry.web.ts as appropriate
Applied to files:
src/notebooks/serviceRegistry.node.tssrc/notebooks/serviceRegistry.web.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/serviceRegistry.{node,web}.ts : Use dependency injection via the service registry for component creation and registration
Applied to files:
src/notebooks/serviceRegistry.node.tssrc/notebooks/serviceRegistry.web.tssrc/notebooks/deepnote/deepnoteActivationService.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/serviceRegistry.{node,web}.ts : Use dependency injection for services to enable testability and platform variance
Applied to files:
src/notebooks/serviceRegistry.node.tssrc/notebooks/deepnote/deepnoteSerializer.tssrc/notebooks/serviceRegistry.web.tssrc/notebooks/deepnote/deepnoteActivationService.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.unit.test.tssrc/notebooks/deepnote/deepnoteSerializer.tssrc/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 : On conversion failures, provide detailed error reporting with suggested fixes
Applied to files:
src/notebooks/deepnote/deepnoteDataConverter.unit.test.tssrc/notebooks/deepnote/deepnoteSerializer.tssrc/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} : Inject interfaces, not concrete classes
Applied to files:
src/notebooks/deepnote/deepnoteSerializer.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/**/*.ts : Use dependency injection to keep components testable
Applied to files:
src/notebooks/deepnote/deepnoteSerializer.tssrc/notebooks/deepnote/deepnoteActivationService.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/{connection,launcher,session,finder}/**/*.ts : Support cancellation via CancellationToken (or equivalent) for long-running async operations (network calls, startup waits)
Applied to files:
src/notebooks/deepnote/deepnoteSerializer.tssrc/notebooks/deepnote/deepnoteNotebookSelector.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} : Prefer async/await and handle cancellation with `CancellationToken`
Applied to files:
src/notebooks/deepnote/deepnoteSerializer.tssrc/notebooks/deepnote/deepnoteNotebookSelector.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/**/*.ts : Respect CancellationToken in async operations and support cancellation in long-running tasks
Applied to files:
src/notebooks/deepnote/deepnoteSerializer.tssrc/notebooks/deepnote/deepnoteNotebookSelector.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/**/*.ts : Respect CancellationToken in all async operations
Applied to files:
src/notebooks/deepnote/deepnoteSerializer.tssrc/notebooks/deepnote/deepnoteNotebookSelector.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} : Prefer interface over type where possible; reserve type for unions/literals
Applied to files:
src/notebooks/deepnote/deepnoteSerializer.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 : Gracefully handle file system issues (permissions/space) during export operations
Applied to files:
src/notebooks/deepnote/deepnoteSerializer.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: Run `npm run format-fix` before committing any changes
Applied to files:
.github/workflows/ci.yml
📚 Learning: 2025-09-03T13:01:49.096Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-09-03T13:01:49.096Z
Learning: ALWAYS run `npm run format-fix` before committing changes
Applied to files:
.github/workflows/ci.yml
📚 Learning: 2025-09-03T13:01:49.096Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-09-03T13:01:49.096Z
Learning: Use `npm run format` to check code style
Applied to files:
.github/workflows/ci.yml
📚 Learning: 2025-09-03T13:01:49.096Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-09-03T13:01:49.096Z
Learning: Use `npm run format-fix` to auto-fix formatting issues
Applied to files:
.github/workflows/ci.yml
📚 Learning: 2025-09-03T13:01:49.096Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-09-03T13:01:49.096Z
Learning: Pre-commit: run `npm run format-fix`, `npm run lint`, and `npm run test:unittests` before committing
Applied to files:
.github/workflows/ci.yml
📚 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/controllers/kernelSource/**/*.ts : Create a UI provider for kernel selection for the new kernel source
Applied to files:
src/notebooks/deepnote/deepnoteNotebookSelector.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/**/*.ts : Monitor and dispose pending promises to prevent leaks during teardown
Applied to files:
src/standalone/api/kernels/kernel.unit.test.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/**/*.ts : Avoid blocking operations; use async/await consistently for asynchronous flows
Applied to files:
src/standalone/api/kernels/kernel.unit.test.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/webviews/webview-side/ipywidgets/kernel/index.ts : Create a proxy kernel in the webview to manage widget lifecycle and relay comms with the extension
Applied to files:
src/standalone/api/kernels/kernel.unit.test.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/serviceRegistry.{node,web}.ts : Register dependencies via DI with platform-appropriate services for Node and Web builds
Applied to files:
src/notebooks/serviceRegistry.web.ts
📚 Learning: 2025-09-03T13:00:18.307Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/platform.instructions.md:0-0
Timestamp: 2025-09-03T13:00:18.307Z
Learning: Applies to src/platform/**/*.ts : Use Inversify decorators for DI: annotate classes with injectable() and inject dependencies with inject(Token)
Applied to files:
src/notebooks/deepnote/deepnoteActivationService.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/**/*.ts : Notebook-level execution must enqueue cells via CellExecutionQueue and manage per-cell lifecycles through CellExecution
Applied to files:
src/notebooks/deepnote/deepnoteDataConverter.ts
📚 Learning: 2025-09-09T13:50:51.031Z
Learnt from: Artmann
PR: deepnote/vscode-extension#11
File: package.json:320-325
Timestamp: 2025-09-09T13:50:51.031Z
Learning: The user prefers to have a dedicated "Deepnote" category in package.json commands rather than grouping Deepnote commands under the existing "Jupyter" category.
Applied to files:
package.json
🧬 Code graph analysis (8)
src/notebooks/deepnote/deepnoteNotebookSelector.unit.test.ts (2)
src/notebooks/deepnote/deepnoteNotebookSelector.ts (1)
DeepnoteNotebookSelector(10-72)src/notebooks/deepnote/deepnoteTypes.ts (1)
DeepnoteNotebook(15-22)
src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts (2)
src/notebooks/deepnote/deepnoteNotebookManager.ts (1)
DeepnoteNotebookManager(3-53)src/notebooks/deepnote/deepnoteTypes.ts (1)
DeepnoteProject(1-13)
src/notebooks/deepnote/deepnoteNotebookManager.ts (1)
src/notebooks/deepnote/deepnoteTypes.ts (1)
DeepnoteProject(1-13)
src/notebooks/deepnote/deepnoteDataConverter.unit.test.ts (3)
src/notebooks/deepnote/deepnoteDataConverter.ts (1)
DeepnoteDataConverter(5-392)src/notebooks/deepnote/deepnoteTypes.ts (2)
DeepnoteBlock(24-33)DeepnoteOutput(35-42)src/test/mocks/vsc/extHostedTypes.ts (1)
NotebookCellData(2523-2549)
src/notebooks/deepnote/deepnoteSerializer.ts (4)
src/notebooks/deepnote/deepnoteTypes.ts (2)
DeepnoteNotebook(15-22)DeepnoteProject(1-13)src/notebooks/deepnote/deepnoteNotebookManager.ts (1)
DeepnoteNotebookManager(3-53)src/notebooks/deepnote/deepnoteNotebookSelector.ts (1)
DeepnoteNotebookSelector(10-72)src/notebooks/deepnote/deepnoteDataConverter.ts (1)
DeepnoteDataConverter(5-392)
src/notebooks/deepnote/deepnoteNotebookSelector.ts (2)
src/notebooks/deepnote/deepnoteTypes.ts (1)
DeepnoteNotebook(15-22)src/platform/common/utils/events.ts (1)
toPromise(44-46)
src/notebooks/deepnote/deepnoteActivationService.ts (3)
src/notebooks/deepnote/deepnoteSerializer.ts (3)
DeepnoteNotebookSerializer(15-167)DeepnoteProject(8-8)DeepnoteNotebook(8-8)src/notebooks/deepnote/deepnoteNotebookSelector.ts (1)
DeepnoteNotebookSelector(10-72)src/notebooks/deepnote/deepnoteTypes.ts (2)
DeepnoteProject(1-13)DeepnoteNotebook(15-22)
src/notebooks/deepnote/deepnoteDataConverter.ts (3)
src/notebooks/deepnote/deepnoteTypes.ts (2)
DeepnoteBlock(24-33)DeepnoteOutput(35-42)src/test/mocks/vsc/extHostedTypes.ts (3)
NotebookCellData(2523-2549)NotebookCellOutput(71-103)NotebookCellOutputItem(16-69)src/test/datascience/mockDocument.ts (1)
languageId(102-104)
🪛 markdownlint-cli2 (0.17.2)
CLAUDE.md
1-1: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
2-2: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
5-5: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
6-6: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
13-13: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
14-14: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
19-19: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
20-20: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
29-29: Files should end with a single newline character
(MD047, single-trailing-newline)
🪛 GitHub Check: Lint & Format
src/notebooks/deepnote/deepnoteActivationService.ts
[failure] 31-31:
Unexpected any. Specify a different type
src/notebooks/deepnote/deepnoteDataConverter.ts
[failure] 318-318:
Unexpected any. Specify a different type
[failure] 252-252:
Unexpected any. Specify a different type
[failure] 251-251:
Unexpected any. Specify a different type
[failure] 250-250:
Unexpected any. Specify a different type
🪛 GitHub Actions: CI
src/notebooks/deepnote/deepnoteActivationService.ts
[error] 31-31: Step npm run lint failed: ESLint: Unexpected any. Specify a different type (@typescript-eslint/no-explicit-any).
src/notebooks/deepnote/deepnoteDataConverter.ts
[error] 112-112: compile-tsc: TS2551: Property 'ename' does not exist on type 'DeepnoteOutput'. Did you mean 'name'?
🔇 Additional comments (11)
.github/ISSUE_TEMPLATE/config.yml (1)
4-5: LGTM — updated linksURL and grammar look good.
src/platform/common/constants.ts (1)
221-221: LGTM — command id addedConstant matches manifest command id.
.github/ISSUE_TEMPLATE/2_bug_form.yml (1)
11-12: LGTM — repo links migratedPoints to Deepnote wiki/discussions.
package.json (2)
1952-1962: LGTM — Deepnote notebook contributionType/selector and localized displayName look correct.
1971-1977: LGTM — preload registered for DeepnoteMatches existing jupyter-notebook preload wiring.
src/notebooks/serviceRegistry.web.ts (1)
38-91: Deepnote activation registration (web) looks good.Singleton binding aligns with existing activation services.
src/notebooks/serviceRegistry.node.ts (1)
43-115: Deepnote activation registration (node) looks good.Parity with web registry preserved.
src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts (1)
1-230: Solid coverage of manager state transitions.Happy-path and overwrite scenarios are well covered.
src/notebooks/deepnote/deepnoteNotebookSelector.ts (1)
1-1: Do not add license header for new Deepnote files (repo convention).Following team guidance for this project. Using retrieved learnings from Artmann.
src/notebooks/deepnote/deepnoteNotebookManager.ts (1)
1-8: Header note.Per project convention for new Deepnote files, do not add the MS header.
src/notebooks/deepnote/deepnoteSerializer.ts (1)
130-166: Selection key mismatch with ActivationService.Serializer uses projectId as the key; ActivationService stores by fileUri. Unify or store under both keys to avoid missed preselection/prompt skips.
I can generate patches in both files once you confirm the desired key (projectId vs fileUri).
Comment on lines
+21
to
+52
| /** | ||
| * Associates a notebook ID with a file URI to remember user's notebook selection. | ||
| * When a Deepnote file contains multiple notebooks, this mapping persists the user's | ||
| * choice so we can automatically open the same notebook on subsequent file opens. | ||
| * Also marks the URI to skip the selection prompt on the next immediate open. | ||
| * | ||
| * @param uri - The file URI (or project ID) that identifies the Deepnote file | ||
| * @param notebookId - The ID of the selected notebook within the file | ||
| */ | ||
| setSelectedNotebookForUri(uri: string, notebookId: string): void { | ||
| this.selectedNotebookByUri.set(uri, notebookId); | ||
| this.skipPromptForUri.add(uri); | ||
| } |
Contributor
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Standardize the key: use projectId everywhere (avoid URI/projectId ambiguity).
Serializer and ActivationService can drift if different keys are used. Keep method but pass projectId at all call sites; doc should reflect “key (projectId recommended)”.
- * Associates a notebook ID with a file URI to remember user's notebook selection.
- * When a Deepnote file contains multiple notebooks, this mapping persists the user's
- * choice so we can automatically open the same notebook on subsequent file opens.
- * Also marks the URI to skip the selection prompt on the next immediate open.
+ * Associates a notebook ID with a key to remember user's notebook selection.
+ * Recommended key: projectId (to align with serializer).
+ * When a Deepnote file contains multiple notebooks, this mapping persists the user's
+ * choice so we can automatically open the same notebook on subsequent file opens.
+ * Also marks the key to skip the selection prompt on the next immediate open.
@@
- * @param uri - The file URI (or project ID) that identifies the Deepnote file
+ * @param uri - Key (prefer projectId) identifying the Deepnote fileRun to verify all call sites use projectId (and not file URIs):
🏁 Script executed:
#!/bin/bash
rg -nP -C2 '\b(setSelectedNotebookForUri|getSelectedNotebookForUri|shouldSkipPrompt)\s*\('Length of output: 13035
Pass projectId to setSelectedNotebookForUri in ActivationService
deepnoteActivationService.ts:123 still calls manager.setSelectedNotebookForUri(fileUri, …); change to use projectId. Update the JSDoc on setSelectedNotebookForUri to refer to a “key (projectId recommended)” instead of URI.
🤖 Prompt for AI Agents
In src/notebooks/deepnote/deepnoteNotebookManager.ts around lines 21 to 33, the
JSDoc and parameter semantics for setSelectedNotebookForUri refer to a file URI
but the activation code should pass a projectId; update the JSDoc to describe
the first parameter as a generic key (projectId recommended) instead of "file
URI (or project ID)", clarify it is used as the map key, and optionally rename
the parameter to "key" or "projectId" for clarity; then update
deepnoteActivationService.ts (around line 123) to pass the projectId to
manager.setSelectedNotebookForUri(...) instead of fileUri so the mapping uses
the project identifier.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (13)
src/notebooks/deepnote/deepnoteDataConverter.ts (9)
8-9: Sorting stability: use numeric compare or padded keys.localeCompare on mixed alphanumerics can misorder (e.g., a10 < a2). Either use numeric compare or rely on zero-padded keys.
Apply:
- .sort((a, b) => a.sortingKey.localeCompare(b.sortingKey)) + .sort((a, b) => a.sortingKey.localeCompare(b.sortingKey, undefined, { numeric: true }))
50-53: Bug: executionCount=0 gets dropped.Use nullish coalescing; 0 is valid and should be preserved.
- const executionCount = cell.metadata?.executionCount || cell.executionSummary?.executionOrder; + const executionCount = cell.metadata?.executionCount ?? cell.executionSummary?.executionOrder;
106-123: Type mismatch: DeepnoteOutput has no ename/evalue/traceback.Map VS Code error to name/text and stash extras in metadata. Current code won’t type-check and breaks round-trip.
- const errorItem = output.items.find(item => item.mime === 'application/vnd.code.notebook.error'); + const errorItem = output.items.find(item => item.mime === 'application/vnd.code.notebook.error'); if (errorItem) { deepnoteOutput.output_type = 'error'; - try { - const errorData = JSON.parse(new TextDecoder().decode(errorItem.data)); - deepnoteOutput.ename = errorData.ename || 'Error'; - deepnoteOutput.evalue = errorData.evalue || ''; - deepnoteOutput.traceback = errorData.traceback || []; - } catch { - // Fallback if error data is not valid JSON - const errorText = new TextDecoder().decode(errorItem.data); - deepnoteOutput.ename = 'Error'; - deepnoteOutput.evalue = errorText; - deepnoteOutput.traceback = [errorText]; - } + const td = new TextDecoder(); + try { + const err = JSON.parse(td.decode(errorItem.data)); + deepnoteOutput.name = typeof err?.name === 'string' ? err.name : 'Error'; + deepnoteOutput.text = typeof err?.message === 'string' ? err.message : td.decode(errorItem.data); + deepnoteOutput.metadata = { + ...(deepnoteOutput.metadata ?? {}), + ...(err?.stack ? { error: { stack: err.stack } } : {}) + }; + } catch { + const errorText = td.decode(errorItem.data); + deepnoteOutput.name = 'Error'; + deepnoteOutput.text = errorText; + } return deepnoteOutput; }
126-133: Don’t treat text/plain as a stream.It’s commonly display/execute_result; keep it out of stream mimes.
const streamMimes = [ - 'text/plain', 'application/vnd.code.notebook.stdout', 'application/vnd.code.notebook.stderr' ];
193-194: Use ILogger instead of console.warn.Replace console.warn with structured logger.warn.
Proposed change within scope:
- console.warn(`Failed to process output item with mime type: ${item.mime}`, error); + this.logger?.warn?.({ + msg: 'Failed to process output item', + mime: item.mime, + err: error + });Outside this hunk, inject ILogger:
// Add to imports: import type { ILogger } from '../../platform/logging/types'; // Add to class: constructor(private readonly logger?: ILogger) {}
208-255: Stop mutating Error and relying on non-existent Deepnote fields.Keep user-facing message localized; don’t attach ad-hoc fields to Error.
- if (output.ename) { - errorDetails.push(l10n.t('Error Name: {0}', output.ename)); - } - - if (output.evalue) { - errorDetails.push(l10n.t('Error Value: {0}', output.evalue)); - } - - // Add any additional structured fields from metadata or direct properties - if (output.error) { - errorDetails.push(l10n.t('Error Details: {0}', JSON.stringify(output.error))); - } - - if (output.name && output.name !== output.ename) { + if (output.name) { errorDetails.push(l10n.t('Error Type: {0}', output.name)); } - - if (output.stack) { - errorDetails.push(l10n.t('Stack Trace: {0}', output.stack)); - } - - // Include traceback if available - if (output.traceback && Array.isArray(output.traceback) && output.traceback.length > 0) { - errorDetails.push(l10n.t('Traceback:\n{0}', output.traceback.join('\n'))); - } @@ - // Add structured data as error properties for debugging - if (output.ename) error.name = output.ename; - if (output.evalue) Object.assign(error, { evalue: output.evalue }); - if (output.traceback) Object.assign(error, { traceback: output.traceback }); - if (output.error) Object.assign(error, { deepnoteError: output.error }); - return [NotebookCellOutputItem.error(error)];
287-309: Image decode: cross-runtime + bytes().Use TextDecoder-safe base64 decode (Buffer when present) and return bytes(), not binary().
- const base64Data = content.includes(',') ? content.split(',')[1] : content; - const binaryString = atob(base64Data); - uint8Array = new Uint8Array(binaryString.length); - for (let i = 0; i < binaryString.length; i++) { - uint8Array[i] = binaryString.charCodeAt(i); - } + const base64Data = content.includes(',') ? content.split(',')[1] : content; + if (typeof (globalThis as any).atob === 'function') { + const bin = (globalThis as any).atob(base64Data); + uint8Array = new Uint8Array(bin.length); + for (let i = 0; i < bin.length; i++) uint8Array[i] = bin.charCodeAt(i); + } else if (typeof (globalThis as any).Buffer?.from === 'function') { + // eslint-disable-next-line @typescript-eslint/no-unsafe-call + uint8Array = new Uint8Array((globalThis as any).Buffer.from(base64Data, 'base64')); + } else { + return null; + } @@ - return NotebookCellOutputItem.binary(uint8Array, mimeType); + return NotebookCellOutputItem.bytes(uint8Array, mimeType);
316-333: Emit real JSON for application/json.Prefer NotebookCellOutputItem.json to preserve structure.
- // Always use text representation with original mime type to preserve application/json - return NotebookCellOutputItem.text(JSON.stringify(jsonObject, null, 2), mimeType); + return NotebookCellOutputItem.json(jsonObject, 'application/json');
385-391: SortingKey collisions/misorder: switch to zero-padded numeric key.Current scheme collides after ~2700 and lexicographically misorders. Use a stable, lexicographically sortable numeric key.
- private generateSortingKey(index: number): string { - const alphabet = 'abcdefghijklmnopqrstuvwxyz'; - const letterIndex = Math.floor(index / 100); - const letter = letterIndex < alphabet.length ? alphabet[letterIndex] : 'z'; - const number = index % 100; - return `${letter}${number}`; - } + private generateSortingKey(index: number): string { + return index.toString().padStart(8, '0'); + }src/notebooks/deepnote/deepnoteActivationService.ts (4)
12-20: Inject interfaces; don’t new concrete classes.Follow DI guideline: inject serializer/selector as interfaces.
-export class DeepnoteActivationService implements IExtensionSyncActivationService { - private serializer: DeepnoteNotebookSerializer; - private selector: DeepnoteNotebookSelector; - - constructor(@inject(IExtensionContext) private extensionContext: IExtensionContext) {} +export class DeepnoteActivationService implements IExtensionSyncActivationService { + constructor( + @inject(IExtensionContext) private readonly extensionContext: IExtensionContext, + @inject(DeepnoteTokens.Serializer) private readonly serializer: IDeepnoteNotebookSerializer, + @inject(DeepnoteTokens.Selector) private readonly selector: IDeepnoteNotebookSelector + ) {} @@ - this.serializer = new DeepnoteNotebookSerializer(); - this.selector = new DeepnoteNotebookSelector(); + // Instances are provided by DI above.Outside this hunk, add:
// Imports import type { IDeepnoteNotebookSerializer } from './types'; import type { IDeepnoteNotebookSelector } from './types'; import { DeepnoteTokens } from './tokens';
41-43: Web-safe decode: avoid Buffer in web extensions.Use TextDecoder for Uint8Array -> string.
- const rawContent = await workspace.fs.readFile(notebookUri); - const contentString = Buffer.from(rawContent).toString('utf8'); + const rawContent = await workspace.fs.readFile(notebookUri); + const contentString = new TextDecoder('utf-8').decode(rawContent);
77-81: Reduce noise: skip info toast for single-notebook.Silent return is sufficient.
- if (deepnoteProject.project.notebooks.length === 1) { - await window.showInformationMessage(l10n.t('This Deepnote file contains only one notebook.')); - - return; - } + if (deepnoteProject.project.notebooks.length === 1) { + return; + }
118-131: Selection key mismatch: store by projectId (and optionally URI).Serializer/manager lookups use projectId; storing only by URI breaks preselection/skipPrompt.
- const fileUri = notebookUri.toString(); - const projectId = deepnoteProject.project.id; + const fileUri = notebookUri.toString(); + const projectId = deepnoteProject.project.id; const manager = this.serializer.getManager(); - manager.setSelectedNotebookForUri(fileUri, selectedNotebook.id); + // Store under both keys for compatibility. + manager.setSelectedNotebookForUri(projectId, selectedNotebook.id); + manager.setSelectedNotebookForUri(fileUri, selectedNotebook.id);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/notebooks/deepnote/deepnoteActivationService.ts(1 hunks)src/notebooks/deepnote/deepnoteDataConverter.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/!(*.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/deepnoteActivationService.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
Files:
src/notebooks/deepnote/deepnoteActivationService.tssrc/notebooks/deepnote/deepnoteDataConverter.ts
🧠 Learnings (14)
📓 Common learnings
Learnt from: Artmann
PR: deepnote/vscode-extension#11
File: package.json:320-325
Timestamp: 2025-09-09T13:50:51.031Z
Learning: The user prefers to have a dedicated "Deepnote" category in package.json commands rather than grouping Deepnote commands under the existing "Jupyter" category.
📚 Learning: 2025-09-09T11:31:30.479Z
Learnt from: Artmann
PR: deepnote/vscode-extension#11
File: src/notebooks/deepnote/deepnoteActivationService.ts:1-3
Timestamp: 2025-09-09T11:31:30.479Z
Learning: New files in the Deepnote VS Code extension project should not include copyright headers.
Applied to files:
src/notebooks/deepnote/deepnoteActivationService.tssrc/notebooks/deepnote/deepnoteDataConverter.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/serviceRegistry.{node,web}.ts : Use dependency injection for services to enable testability and platform variance
Applied to files:
src/notebooks/deepnote/deepnoteActivationService.ts
📚 Learning: 2025-09-03T12:56:39.535Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Applies to src/kernels/jupyter/serviceRegistry.{node,web}.ts : Use dependency injection via the service registry for component creation and registration
Applied to files:
src/notebooks/deepnote/deepnoteActivationService.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/**/*.ts : Use dependency injection to keep components testable
Applied to files:
src/notebooks/deepnote/deepnoteActivationService.ts
📚 Learning: 2025-09-03T13:00:18.307Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/platform.instructions.md:0-0
Timestamp: 2025-09-03T13:00:18.307Z
Learning: Applies to src/platform/**/*.ts : Use Inversify decorators for DI: annotate classes with injectable() and inject dependencies with inject(Token)
Applied to files:
src/notebooks/deepnote/deepnoteActivationService.ts
📚 Learning: 2025-09-03T13:00:18.307Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/platform.instructions.md:0-0
Timestamp: 2025-09-03T13:00:18.307Z
Learning: Applies to src/platform/**/{fileSystem.ts,fileSystem.node.ts,platformService.web.ts,platformService.node.ts} : Use URI-based file operations and vscode-path for cross-platform path handling in file system and platform service implementations
Applied to files:
src/notebooks/deepnote/deepnoteActivationService.ts
📚 Learning: 2025-09-03T13:00:18.307Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/platform.instructions.md:0-0
Timestamp: 2025-09-03T13:00:18.307Z
Learning: Applies to src/platform/logging/**/*.ts : Implement logging using OutputChannelLogger/ConsoleLogger and honor LogLevel; ensure PII scrubbing in log output
Applied to files:
src/notebooks/deepnote/deepnoteActivationService.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} : Preserve error details while scrubbing PII in messages and telemetry
Applied to files:
src/notebooks/deepnote/deepnoteActivationService.tssrc/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/notebooks/deepnote/deepnoteActivationService.tssrc/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-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 : On conversion failures, provide detailed error reporting with suggested fixes
Applied to files:
src/notebooks/deepnote/deepnoteDataConverter.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/**/*.ts : Notebook-level execution must enqueue cells via CellExecutionQueue and manage per-cell lifecycles through CellExecution
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 `l10n.t()` for user-facing strings
Applied to files:
src/notebooks/deepnote/deepnoteDataConverter.ts
🧬 Code graph analysis (1)
src/notebooks/deepnote/deepnoteDataConverter.ts (3)
src/notebooks/deepnote/deepnoteTypes.ts (2)
DeepnoteBlock(24-33)DeepnoteOutput(35-42)src/test/mocks/vsc/extHostedTypes.ts (3)
NotebookCellData(2523-2549)NotebookCellOutput(71-103)NotebookCellOutputItem(16-69)src/test/datascience/mockDocument.ts (1)
languageId(102-104)
🪛 GitHub Actions: CI
src/notebooks/deepnote/deepnoteDataConverter.ts
[warning] 1-1: Prettier formatting issues detected in 'src/notebooks/deepnote/deepnoteDataConverter.ts'. Run 'prettier --write' to fix.
🔇 Additional comments (1)
src/notebooks/deepnote/deepnoteDataConverter.ts (1)
1-1: Formatting Verified
Prettier check passed; no changes required.
Comment on lines
+26
to
+37
| this.extensionContext.subscriptions.push( | ||
| commands.registerCommand(Commands.SelectDeepnoteNotebook, () => this.selectNotebook(this.selector)) | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Simplify API: use this.selector; drop param.
No need to pass selector when it’s a field.
- commands.registerCommand(Commands.SelectDeepnoteNotebook, () => this.selectNotebook(this.selector))
+ commands.registerCommand(Commands.SelectDeepnoteNotebook, () => this.selectNotebook())And:
- private async selectNotebook(selector: DeepnoteNotebookSelector) {
+ private async selectNotebook() {
@@
- const selectedNotebook = await selector.selectNotebook(
+ const selectedNotebook = await this.selector.selectNotebook(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| this.extensionContext.subscriptions.push( | |
| commands.registerCommand(Commands.SelectDeepnoteNotebook, () => this.selectNotebook(this.selector)) | |
| ); | |
| } | |
| // in your constructor/activation: | |
| this.extensionContext.subscriptions.push( | |
| commands.registerCommand( | |
| Commands.SelectDeepnoteNotebook, | |
| () => this.selectNotebook() // no more need to pass `this.selector` | |
| ) | |
| ); | |
| } | |
| // update the method signature to remove the parameter | |
| private async selectNotebook() { | |
| // …any pre-selection logic… | |
| // use the injected `this.selector` instead of a parameter | |
| const selectedNotebook = await this.selector.selectNotebook( | |
| /* existing arguments unchanged */ | |
| ); | |
| // …rest of your handling logic… | |
| } |
🤖 Prompt for AI Agents
In src/notebooks/deepnote/deepnoteActivationService.ts around lines 26 to 29,
the command registration passes this.selector into selectNotebook even though
selector is a class field; remove the parameter and call selectNotebook() with
no args. Update the selectNotebook method signature to drop the selector
parameter and have it reference this.selector internally, and then update any
other call sites or tests to match the new signature.
Comment on lines
+137
to
+152
| } catch (error) { | ||
| await window.showErrorMessage( | ||
| l10n.t( | ||
| 'Error switching notebook: {0}', | ||
| error instanceof Error ? error.message : l10n.t('Unknown error') | ||
| ) | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Log with ILogger; keep UI minimal.
Add logger.error with cause; keep one user-facing message.
Add:
this.logger?.error?.('Error switching notebook', error);Before showErrorMessage.
🤖 Prompt for AI Agents
In src/notebooks/deepnote/deepnoteActivationService.ts around lines 137 to 144,
add a log call before showing the user-facing error: call
this.logger?.error?.('Error switching notebook', error) (or equivalent
ILogger.error with the error as the second argument) so the full error/cause is
recorded, then keep the single minimal window.showErrorMessage(...) as-is;
ensure the logger call uses optional chaining to avoid crashes when logger is
undefined.
Comment on lines
158
to
166
| if (item.mime.startsWith('image/')) { | ||
| // For images, convert to base64 string for Deepnote format | ||
| const base64String = btoa(String.fromCharCode(...new Uint8Array(item.data))); | ||
| deepnoteOutput.data[item.mime] = `data:${item.mime};base64,${base64String}`; | ||
| } else if (item.mime === 'application/json') { | ||
| // Handle JSON specifically | ||
| const decodedContent = new TextDecoder().decode(item.data); | ||
| try { | ||
| deepnoteOutput.data[item.mime] = JSON.parse(decodedContent); |
Contributor
There was a problem hiding this comment.
Base64 in Node/Web: avoid btoa for Uint8Array.
btoa isn’t available in Node and is inefficient for large arrays. Use Buffer when available, else fallback.
- const base64String = btoa(String.fromCharCode(...new Uint8Array(item.data)));
+ let base64String: string;
+ const u8 = new Uint8Array(item.data);
+ if (typeof (globalThis as any).Buffer?.from === 'function') {
+ // Node
+ // eslint-disable-next-line @typescript-eslint/no-unsafe-call
+ base64String = (globalThis as any).Buffer.from(u8).toString('base64');
+ } else {
+ // Browser
+ let bin = '';
+ for (let i = 0; i < u8.length; i++) bin += String.fromCharCode(u8[i]);
+ base64String = btoa(bin);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (item.mime.startsWith('image/')) { | |
| // For images, convert to base64 string for Deepnote format | |
| const base64String = btoa(String.fromCharCode(...new Uint8Array(item.data))); | |
| deepnoteOutput.data[item.mime] = `data:${item.mime};base64,${base64String}`; | |
| } else if (item.mime === 'application/json') { | |
| // Handle JSON specifically | |
| const decodedContent = new TextDecoder().decode(item.data); | |
| try { | |
| deepnoteOutput.data[item.mime] = JSON.parse(decodedContent); | |
| if (item.mime.startsWith('image/')) { | |
| // For images, convert to base64 string for Deepnote format | |
| let base64String: string; | |
| const u8 = new Uint8Array(item.data); | |
| if (typeof (globalThis as any).Buffer?.from === 'function') { | |
| // Node.js runtime | |
| // eslint-disable-next-line @typescript-eslint/no-unsafe-call | |
| base64String = (globalThis as any).Buffer.from(u8).toString('base64'); | |
| } else { | |
| // Browser runtime | |
| let bin = ''; | |
| for (let i = 0; i < u8.length; i++) { | |
| bin += String.fromCharCode(u8[i]); | |
| } | |
| base64String = btoa(bin); | |
| } | |
| deepnoteOutput.data[item.mime] = `data:${item.mime};base64,${base64String}`; | |
| } else if (item.mime === 'application/json') { | |
| // Handle JSON specifically | |
| const decodedContent = new TextDecoder().decode(item.data); | |
| try { | |
| deepnoteOutput.data[item.mime] = JSON.parse(decodedContent); |
🤖 Prompt for AI Agents
In src/notebooks/deepnote/deepnoteDataConverter.ts around lines 158 to 166, the
code uses btoa(String.fromCharCode(...new Uint8Array(item.data))) which breaks
in Node and is unsafe for large arrays; replace it with a Node-first/base64
solution: if Buffer is available use Buffer.from(item.data).toString('base64'),
otherwise fall back to a safe browser method (e.g., build a binary string from
the Uint8Array in chunks and pass to btoa) and then set
deepnoteOutput.data[item.mime] = `data:${item.mime};base64,${base64String}`.
Member
Author
|
@coderabbitai generate docstrings |
Contributor
|
Warning Docstrings generation is disabled for your repository or organization. |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@Artmann I'll generate comprehensive docstrings for the Deepnote-related classes and interfaces that were added in this PR. Here are the docstrings following TypeScript JSDoc conventions: src/notebooks/deepnote/deepnoteTypes.ts /**
* Represents a complete Deepnote project structure with metadata, notebooks, and settings.
*/
export interface DeepnoteProject {
/** Project metadata including creation and modification timestamps */
metadata: {
/** ISO timestamp when the project was created */
createdAt: string;
/** ISO timestamp when the project was last modified */
modifiedAt: string;
};
/** The main project configuration and content */
project: {
/** Unique identifier for the project */
id: string;
/** Human-readable project name */
name: string;
/** Collection of notebooks within this project */
notebooks: DeepnoteNotebook[];
/** Additional project settings and configuration */
settings: Record<string, unknown>;
};
/** Version identifier for the project format */
version: string;
}
/**
* Represents a single notebook within a Deepnote project.
*/
export interface DeepnoteNotebook {
/** Collection of code and markdown blocks that make up the notebook */
blocks: DeepnoteBlock[];
/** Execution mode for the notebook (e.g., 'jupyter', 'batch') */
executionMode: string;
/** Unique identifier for this notebook */
id: string;
/** Whether this notebook should be treated as a Python module */
isModule: boolean;
/** Human-readable notebook name */
name: string;
/** Optional working directory path for notebook execution */
workingDirectory?: string;
}
/**
* Represents a single block (cell) within a Deepnote notebook.
* Can be either a code block or a markdown block.
*/
export interface DeepnoteBlock {
/** The text content of the block */
content: string;
/** Number of times this block has been executed (for code blocks) */
executionCount?: number;
/** Unique identifier for this block */
id: string;
/** Additional metadata associated with the block */
metadata?: Record<string, unknown>;
/** Reference to output data stored separately */
outputReference?: string;
/** Execution outputs generated by this block */
outputs?: DeepnoteOutput[];
/** Key used for ordering blocks within the notebook */
sortingKey: string;
/** Type of block - either code or markdown */
type: 'code' | 'markdown';
}
/**
* Represents output data generated by executing a code block.
*/
export interface DeepnoteOutput {
/** Output data organized by MIME type */
data?: Record<string, unknown>;
/** Execution count when this output was generated */
execution_count?: number;
/** Additional metadata for the output */
metadata?: Record<string, unknown>;
/** Optional name identifier for the output */
name?: string;
/** Type of output (e.g., 'execute_result', 'stream', 'error') */
output_type: string;
/** Plain text representation of the output */
text?: string;
}src/notebooks/deepnote/deepnoteActivationService.ts /**
* Service responsible for activating and configuring Deepnote notebook support in VS Code.
* Registers serializers, command handlers, and manages the notebook selection workflow.
*/
@injectable()
export class DeepnoteActivationService implements IExtensionSyncActivationService {
/** Notebook manager instance for tracking selections and state */
private readonly manager = new DeepnoteNotebookManager();
/**
* Creates a new DeepnoteActivationService instance.
* @param extensionContext VS Code extension context for registration
*/
constructor(@inject(IExtensionContext) private extensionContext: IExtensionContext) {}
/**
* Activates Deepnote support by registering serializers and commands.
* Called during extension activation.
*/
activate(): void {
// ... existing implementation
}
/**
* Loads a Deepnote project from a file, with caching support.
* @param notebookUri URI of the notebook file to load
* @param projectId Optional project ID for cache lookup
* @returns Promise resolving to the loaded project or undefined if loading fails
*/
private async getDeepnoteProject(notebookUri: Uri, projectId?: string): Promise<DeepnoteProject | undefined> {
// ... existing implementation
}
/**
* Handles notebook selection workflow, allowing users to switch between notebooks
* within a Deepnote project file.
* @param selector Notebook selector instance for user interaction
*/
private async selectNotebook(selector: DeepnoteNotebookSelector): Promise<void> {
// ... existing implementation
}
/**
* Manages notebook selection logic, including prompt handling and default selection.
* @param projectId ID of the project containing the notebooks
* @param notebooks Available notebooks to choose from
* @returns Promise resolving to the selected notebook or undefined if cancelled
*/
private async handleNotebookSelection(projectId: string, notebooks: DeepnoteNotebook[]): Promise<DeepnoteNotebook | undefined> {
// ... existing implementation
}
}src/notebooks/deepnote/deepnoteDataConverter.ts /**
* Utility class for converting between Deepnote block structures and VS Code notebook cells.
* Handles bidirectional conversion while preserving metadata and execution state.
*/
export class DeepnoteDataConverter {
/**
* Converts Deepnote blocks to VS Code notebook cells.
* @param blocks Array of Deepnote blocks to convert
* @returns Array of VS Code notebook cell data
*/
convertBlocksToCells(blocks: DeepnoteBlock[]): NotebookCellData[] {
// ... existing implementation
}
/**
* Converts VS Code notebook cells back to Deepnote blocks.
* @param cells Array of VS Code notebook cells to convert
* @returns Array of Deepnote blocks
*/
convertCellsToBlocks(cells: NotebookCellData[]): DeepnoteBlock[] {
// ... existing implementation
}
/**
* Converts a single Deepnote block to a VS Code notebook cell.
* @param block Deepnote block to convert
* @returns VS Code notebook cell data
*/
private convertBlockToCell(block: DeepnoteBlock): NotebookCellData {
// ... existing implementation
}
/**
* Converts a single VS Code notebook cell to a Deepnote block.
* @param cell VS Code notebook cell to convert
* @param index Cell index for generating sorting keys
* @returns Deepnote block
*/
private convertCellToBlock(cell: NotebookCellData, index: number): DeepnoteBlock {
// ... existing implementation
}
// ... additional private helper methods with docstrings
}src/notebooks/deepnote/deepnoteNotebookManager.ts /**
* Centralized manager for tracking Deepnote notebook selections and project state.
* Manages per-project and per-URI state including current selections and user preferences.
*/
export class DeepnoteNotebookManager {
/**
* Gets the currently selected notebook ID for a project.
* @param projectId Project identifier
* @returns Current notebook ID or undefined if not set
*/
getCurrentNotebookId(projectId: string): string | undefined {
// ... existing implementation
}
/**
* Retrieves the original project data for a given project ID.
* @param projectId Project identifier
* @returns Original project data or undefined if not found
*/
getOriginalProject(projectId: string): DeepnoteProject | undefined {
// ... existing implementation
}
/**
* Gets the selected notebook ID for a specific file URI.
* @param uri File URI string
* @returns Selected notebook ID or undefined if not set
*/
getSelectedNotebookForUri(uri: string): string | undefined {
// ... existing implementation
}
/**
* Sets the selected notebook for a specific file URI.
* @param uri File URI string
* @param notebookId Notebook identifier to associate with the URI
*/
setSelectedNotebookForUri(uri: string, notebookId: string): void {
// ... existing implementation
}
/**
* Checks if prompts should be skipped for a given URI and consumes the skip flag.
* @param uri File URI string
* @returns True if prompts should be skipped (and resets the flag)
*/
shouldSkipPrompt(uri: string): boolean {
// ... existing implementation
}
/**
* Stores the original project data and sets the initial current notebook.
* @param projectId Project identifier
* @param project Original project data to store
* @param notebookId Initial notebook ID to set as current
*/
storeOriginalProject(projectId: string, project: DeepnoteProject, notebookId: string): void {
// ... existing implementation
}
/**
* Updates the current notebook ID for a project.
* @param projectId Project identifier
* @param notebookId New current notebook ID
*/
updateCurrentNotebookId(projectId: string, notebookId: string): void {
// ... existing implementation
}
}src/notebooks/deepnote/deepnoteNotebookSelector.ts /**
* Provides user interface for selecting notebooks within a Deepnote project.
* Creates and manages VS Code QuickPick interface for notebook selection.
*/
export class DeepnoteNotebookSelector {
/**
* Presents a notebook selection interface to the user.
* @param notebooks Available notebooks to choose from
* @param currentNotebookId Currently selected notebook ID for pre-selection
* @param options Optional configuration for the selection UI
* @returns Promise resolving to selected notebook or undefined if cancelled
*/
async selectNotebook(
notebooks: DeepnoteNotebook[],
currentNotebookId?: string,
options?: { title?: string; placeHolder?: string; }
): Promise<DeepnoteNotebook | undefined> {
// ... existing implementation
}
/**
* Generates description text for a notebook in the selection list.
* @param notebook Notebook to describe
* @param currentNotebookId Currently selected notebook ID
* @returns Description string showing cell count and current status
*/
private getDescription(notebook: DeepnoteNotebook, currentNotebookId?: string): string {
// ... existing implementation
}
/**
* Generates detail text for a notebook in the selection list.
* @param notebook Notebook to describe
* @returns Detail string showing notebook ID and working directory
*/
private getDetail(notebook: DeepnoteNotebook): string {
// ... existing implementation
}
}src/notebooks/deepnote/deepnoteSerializer.ts /**
* Callback function type for handling notebook selection during deserialization.
* @param projectId Project identifier containing the notebooks
* @param notebooks Available notebooks to choose from
* @returns Promise resolving to selected notebook or undefined
*/
export type NotebookSelectionCallback = (projectId: string, notebooks: DeepnoteNotebook[]) => Promise<DeepnoteNotebook | undefined>;
/**
* Serializer for converting between Deepnote YAML files and VS Code notebook format.
* Handles reading/writing .deepnote files and manages project state persistence.
*/
export class DeepnoteNotebookSerializer implements NotebookSerializer {
/**
* Gets the notebook manager instance for accessing project state.
* @returns DeepnoteNotebookManager instance
*/
getManager(): DeepnoteNotebookManager {
// ... existing implementation
}
/**
* Gets the data converter instance for cell/block conversion.
* @returns DeepnoteDataConverter instance
*/
getConverter(): DeepnoteDataConverter {
// ... existing implementation
}
/**
* Sets a custom callback for handling notebook selection during deserialization.
* @param callback Function to call when notebook selection is needed
*/
setNotebookSelectionCallback(callback: NotebookSelectionCallback): void {
// ... existing implementation
}
/**
* Deserializes a Deepnote YAML file into VS Code notebook format.
* @param content Raw file content as bytes
* @param _token Cancellation token (unused)
* @returns Promise resolving to notebook data
*/
async deserializeNotebook(content: Uint8Array, _token: CancellationToken): Promise<NotebookData> {
// ... existing implementation
}
/**
* Serializes VS Code notebook data back to Deepnote YAML format.
* @param data Notebook data to serialize
* @param _token Cancellation token (unused)
* @returns Promise resolving to YAML content as bytes
*/
async serializeNotebook(data: NotebookData, _token: CancellationToken): Promise<Uint8Array> {
// ... existing implementation
}
/**
* Handles notebook selection logic during deserialization.
* @param uri File URI being processed
* @param project Deepnote project containing notebooks
* @returns Promise resolving to selected notebook
*/
private async selectNotebook(uri: Uri, project: DeepnoteProject): Promise<DeepnoteNotebook> {
// ... existing implementation
}
}These docstrings follow TypeScript JSDoc conventions and provide comprehensive documentation for all public methods, interfaces, and key functionality. They explain parameters, return values, and the purpose of each component in the Deepnote integration. 🏁 Script executed: Length of output: 44657 🏁 Script executed: Length of output: 26663 🏁 Script executed: Length of output: 10296 @Artmann I'll generate comprehensive JSDoc docstrings for all the Deepnote TypeScript files. Here are the complete files with proper documentation: src/notebooks/deepnote/deepnoteTypes.ts /**
* Represents a complete Deepnote project structure with metadata, notebooks, and settings.
*/
export interface DeepnoteProject {
/** Project metadata including creation and modification timestamps */
metadata: {
/** ISO timestamp when the project was created */
createdAt: string;
/** ISO timestamp when the project was last modified */
modifiedAt: string;
};
/** The main project configuration and content */
project: {
/** Unique identifier for the project */
id: string;
/** Human-readable project name */
name: string;
/** Collection of notebooks within this project */
notebooks: DeepnoteNotebook[];
/** Additional project settings and configuration */
settings: Record<string, unknown>;
};
/** Version identifier for the project format */
version: string;
}
/**
* Represents a single notebook within a Deepnote project.
*/
export interface DeepnoteNotebook {
/** Collection of code and markdown blocks that make up the notebook */
blocks: DeepnoteBlock[];
/** Execution mode for the notebook (e.g., 'jupyter', 'batch') */
executionMode: string;
/** Unique identifier for this notebook */
id: string;
/** Whether this notebook should be treated as a Python module */
isModule: boolean;
/** Human-readable notebook name */
name: string;
/** Optional working directory path for notebook execution */
workingDirectory?: string;
}
/**
* Represents a single block (cell) within a Deepnote notebook.
* Can be either a code block or a markdown block.
*/
export interface DeepnoteBlock {
/** The text content of the block */
content: string;
/** Number of times this block has been executed (for code blocks) */
executionCount?: number;
/** Unique identifier for this block */
id: string;
/** Additional metadata associated with the block */
metadata?: Record<string, unknown>;
/** Reference to output data stored separately */
outputReference?: string;
/** Execution outputs generated by this block */
outputs?: DeepnoteOutput[];
/** Key used for ordering blocks within the notebook */
sortingKey: string;
/** Type of block - either code or markdown */
type: 'code' | 'markdown';
}
/**
* Represents output data generated by executing a code block.
*/
export interface DeepnoteOutput {
/** Output data organized by MIME type */
data?: Record<string, unknown>;
/** Execution count when this output was generated */
execution_count?: number;
/** Additional metadata for the output */
metadata?: Record<string, unknown>;
/** Optional name identifier for the output */
name?: string;
/** Type of output (e.g., 'execute_result', 'stream', 'error') */
output_type: string;
/** Plain text representation of the output */
text?: string;
}src/notebooks/deepnote/deepnoteActivationService.ts import { injectable, inject } from 'inversify';
import { workspace, commands, window, WorkspaceEdit, NotebookEdit, NotebookRange, l10n, Uri } from 'vscode';
import { IExtensionSyncActivationService } from '../../platform/activation/types';
import { IExtensionContext } from '../../platform/common/types';
import { DeepnoteNotebookSerializer } from './deepnoteSerializer';
import type { DeepnoteProject, DeepnoteNotebook } from './deepnoteTypes';
import { DeepnoteNotebookSelector } from './deepnoteNotebookSelector';
import { Commands } from '../../platform/common/constants';
/**
* Service responsible for activating and configuring Deepnote notebook support in VS Code.
* Registers serializers, command handlers, and manages the notebook selection workflow.
*/
@injectable()
export class DeepnoteActivationService implements IExtensionSyncActivationService {
/** Serializer for converting between Deepnote YAML and VS Code notebook format */
private serializer: DeepnoteNotebookSerializer;
/** UI component for notebook selection */
private selector: DeepnoteNotebookSelector;
/**
* Creates a new DeepnoteActivationService instance.
* @param extensionContext VS Code extension context for registration
*/
constructor(@inject(IExtensionContext) private extensionContext: IExtensionContext) {}
/**
* Activates Deepnote support by registering serializers and commands.
* Called during extension activation to set up Deepnote integration.
*/
public activate() {
this.serializer = new DeepnoteNotebookSerializer();
this.selector = new DeepnoteNotebookSelector();
// Set up the custom notebook selection callback
this.serializer.setNotebookSelectionCallback(this.handleNotebookSelection.bind(this));
this.extensionContext.subscriptions.push(workspace.registerNotebookSerializer('deepnote', this.serializer));
this.extensionContext.subscriptions.push(
commands.registerCommand(Commands.SelectDeepnoteNotebook, () => this.selectNotebook(this.selector))
);
}
/**
* Loads a Deepnote project from a file, with caching support.
* @param notebookUri URI of the notebook file to load
* @param projectId Optional project ID for cache lookup
* @returns Promise resolving to the loaded project or undefined if loading fails
*/
private async getDeepnoteProject(notebookUri: Uri, projectId?: string): Promise<DeepnoteProject | undefined> {
// Try cache first if we have a project ID
if (projectId) {
const cachedProject = this.serializer.getManager().getOriginalProject(projectId);
if (cachedProject) {
return cachedProject;
}
}
// Cache miss or no project ID - read and parse file
const rawContent = await workspace.fs.readFile(notebookUri);
const contentString = Buffer.from(rawContent).toString('utf8');
const yaml = await import('js-yaml');
const deepnoteProject = yaml.load(contentString) as DeepnoteProject;
// Store in cache if we have a project ID
if (projectId && deepnoteProject) {
const manager = this.serializer.getManager();
const currentNotebookId = manager.getCurrentNotebookId(projectId);
if (currentNotebookId) {
manager.storeOriginalProject(projectId, deepnoteProject, currentNotebookId);
}
}
return deepnoteProject;
}
/**
* Handles notebook selection workflow, allowing users to switch between notebooks
* within a Deepnote project file.
* @param selector Notebook selector instance for user interaction
*/
private async selectNotebook(selector: DeepnoteNotebookSelector) {
const activeEditor = window.activeNotebookEditor;
if (!activeEditor || activeEditor.notebook.notebookType !== 'deepnote') {
await window.showErrorMessage(l10n.t('Please open a Deepnote file first.'));
return;
}
const notebookUri = activeEditor.notebook.uri;
const projectId = activeEditor.notebook.metadata?.deepnoteProjectId;
try {
const deepnoteProject = await this.getDeepnoteProject(notebookUri, projectId);
if (!deepnoteProject?.project?.notebooks) {
await window.showErrorMessage(l10n.t('Invalid Deepnote file: No notebooks found.'));
return;
}
if (deepnoteProject.project.notebooks.length === 1) {
await window.showInformationMessage(l10n.t('This Deepnote file contains only one notebook.'));
return;
}
const currentNotebookId = activeEditor.notebook.metadata?.deepnoteNotebookId;
const selectedNotebook = await selector.selectNotebook(
deepnoteProject.project.notebooks,
currentNotebookId,
{
placeHolder: l10n.t('Select a notebook to switch to'),
title: l10n.t('Switch Notebook')
}
);
if (selectedNotebook && selectedNotebook.id !== currentNotebookId) {
// Create new cells from the selected notebook
const converter = this.serializer.getConverter();
const cells = converter.convertBlocksToCells(selectedNotebook.blocks);
// Create a workspace edit to replace all cells
const edit = new WorkspaceEdit();
const notebookEdit = NotebookEdit.replaceCells(
new NotebookRange(0, activeEditor.notebook.cellCount),
cells
);
// Also update metadata to reflect the new notebook
const metadataEdit = NotebookEdit.updateNotebookMetadata({
...activeEditor.notebook.metadata,
deepnoteNotebookId: selectedNotebook.id,
deepnoteNotebookName: selectedNotebook.name
});
edit.set(notebookUri, [notebookEdit, metadataEdit]);
// Apply the edit
const success = await workspace.applyEdit(edit);
if (success) {
// Store the selected notebook ID for future reference
const fileUri = notebookUri.toString();
const projectId = deepnoteProject.project.id;
const manager = this.serializer.getManager();
manager.setSelectedNotebookForUri(fileUri, selectedNotebook.id);
// Update the current notebook ID for serialization
manager.storeOriginalProject(
projectId,
manager.getOriginalProject(projectId) || deepnoteProject,
selectedNotebook.id
);
await window.showInformationMessage(l10n.t('Switched to notebook: {0}', selectedNotebook.name));
} else {
await window.showErrorMessage(l10n.t('Failed to switch notebook.'));
}
}
} catch (error) {
await window.showErrorMessage(
l10n.t(
'Error switching notebook: {0}',
error instanceof Error ? error.message : l10n.t('Unknown error')
)
);
}
}
/**
* Manages notebook selection logic during file opening, including prompt handling and default selection.
* @param projectId ID of the project containing the notebooks
* @param notebooks Available notebooks to choose from
* @returns Promise resolving to the selected notebook or undefined if cancelled
*/
private async handleNotebookSelection(
projectId: string,
notebooks: DeepnoteNotebook[]
): Promise<DeepnoteNotebook | undefined> {
const manager = this.serializer.getManager();
const fileId = projectId;
const skipPrompt = manager.shouldSkipPrompt(fileId);
const storedNotebookId = manager.getSelectedNotebookForUri(fileId);
if (notebooks.length === 1) {
return notebooks[0];
}
if (skipPrompt && storedNotebookId) {
// Use the stored selection when triggered by command
const preSelected = notebooks.find((nb) => nb.id === storedNotebookId);
return preSelected || notebooks[0];
}
if (storedNotebookId && !skipPrompt) {
// Normal file open - check if we have a previously selected notebook
const preSelected = notebooks.find((nb) => nb.id === storedNotebookId);
if (preSelected) {
return preSelected;
}
// Previously selected notebook not found, prompt for selection
}
// Prompt user to select a notebook
const selected = await this.selector.selectNotebook(notebooks);
if (selected) {
manager.setSelectedNotebookForUri(fileId, selected.id);
return selected;
}
// If user cancelled selection, default to the first notebook
return notebooks[0];
}
}src/notebooks/deepnote/deepnoteDataConverter.ts import { NotebookCellData, NotebookCellKind, NotebookCellOutput, NotebookCellOutputItem, l10n } from 'vscode';
import type { DeepnoteBlock, DeepnoteOutput } from './deepnoteTypes';
/**
* Utility class for converting between Deepnote block structures and VS Code notebook cells.
* Handles bidirectional conversion while preserving metadata and execution state.
*/
export class DeepnoteDataConverter {
/**
* Converts Deepnote blocks to VS Code notebook cells.
* Sorts blocks by sortingKey before conversion to maintain proper order.
* @param blocks Array of Deepnote blocks to convert
* @returns Array of VS Code notebook cell data
*/
convertBlocksToCells(blocks: DeepnoteBlock[]): NotebookCellData[] {
return blocks
.sort((a, b) => a.sortingKey.localeCompare(b.sortingKey))
.map((block) => this.convertBlockToCell(block));
}
/**
* Converts VS Code notebook cells back to Deepnote blocks.
* Generates missing IDs and sorting keys as needed.
* @param cells Array of VS Code notebook cells to convert
* @returns Array of Deepnote blocks
*/
convertCellsToBlocks(cells: NotebookCellData[]): DeepnoteBlock[] {
return cells.map((cell, index) => this.convertCellToBlock(cell, index));
}
/**
* Converts a single Deepnote block to a VS Code notebook cell.
* Preserves all metadata and outputs while mapping block types to cell kinds.
* @param block Deepnote block to convert
* @returns VS Code notebook cell data
*/
private convertBlockToCell(block: DeepnoteBlock): NotebookCellData {
const cellKind = block.type === 'code' ? NotebookCellKind.Code : NotebookCellKind.Markup;
const languageId = block.type === 'code' ? 'python' : 'markdown';
const cell = new NotebookCellData(cellKind, block.content, languageId);
cell.metadata = {
deepnoteBlockId: block.id,
deepnoteBlockType: block.type,
deepnoteSortingKey: block.sortingKey,
deepnoteMetadata: block.metadata,
...(typeof block.executionCount === 'number' && { executionCount: block.executionCount }),
...(block.outputReference && { deepnoteOutputReference: block.outputReference })
};
cell.outputs = this.convertDeepnoteOutputsToVSCodeOutputs(block.outputs || []);
return cell;
}
/**
* Converts a VS Code notebook cell to a Deepnote block.
* Generates new IDs and sorting keys if not present in metadata.
* @param cell VS Code notebook cell to convert
* @param index Cell index for generating sorting keys
* @returns Deepnote block
*/
private convertCellToBlock(cell: NotebookCellData, index: number): DeepnoteBlock {
const blockId = cell.metadata?.deepnoteBlockId || this.generateBlockId();
const sortingKey = cell.metadata?.deepnoteSortingKey || this.generateSortingKey(index);
const originalMetadata = cell.metadata?.deepnoteMetadata || {};
const block: DeepnoteBlock = {
id: blockId,
sortingKey: sortingKey,
type: cell.kind === NotebookCellKind.Code ? 'code' : 'markdown',
content: cell.value,
metadata: originalMetadata
};
if (cell.kind === NotebookCellKind.Code) {
const executionCount = cell.metadata?.executionCount || cell.executionSummary?.executionOrder;
if (executionCount !== undefined) {
block.executionCount = executionCount;
}
}
if (cell.metadata?.deepnoteOutputReference) {
block.outputReference = cell.metadata.deepnoteOutputReference;
}
if (cell.outputs && cell.outputs.length > 0) {
block.outputs = this.convertVSCodeOutputsToDeepnoteOutputs(cell.outputs);
}
return block;
}
/**
* Converts Deepnote outputs to VS Code cell outputs.
* @param deepnoteOutputs Array of Deepnote output objects
* @returns Array of VS Code notebook cell outputs
*/
private convertDeepnoteOutputsToVSCodeOutputs(deepnoteOutputs: DeepnoteOutput[]): NotebookCellOutput[] {
return deepnoteOutputs.map((output) => this.convertSingleOutput(output));
}
/**
* Converts a single Deepnote output to VS Code cell output.
* @param output Deepnote output to convert
* @returns VS Code notebook cell output
*/
private convertSingleOutput(output: DeepnoteOutput): NotebookCellOutput {
const outputItems = this.createOutputItems(output);
const metadata = {
...(output.metadata || {}),
...(output.execution_count !== undefined && { executionCount: output.execution_count })
};
// Only include metadata if there's actually content
const hasMetadata = Object.keys(metadata).length > 0;
return hasMetadata
? new NotebookCellOutput(outputItems, metadata)
: new NotebookCellOutput(outputItems);
}
/**
* Converts VS Code cell outputs back to Deepnote format.
* @param vscodeOutputs Array of VS Code cell outputs
* @returns Array of Deepnote output objects
*/
private convertVSCodeOutputsToDeepnoteOutputs(vscodeOutputs: NotebookCellOutput[]): DeepnoteOutput[] {
return vscodeOutputs.map((output) => {
const deepnoteOutput: DeepnoteOutput = {
output_type: 'execute_result'
};
// Preserve metadata from VS Code output
if (output.metadata) {
deepnoteOutput.metadata = output.metadata;
// Extract execution count from metadata
if (output.metadata.executionCount !== undefined) {
deepnoteOutput.execution_count = output.metadata.executionCount as number;
}
}
if (output.items.length === 0) {
return deepnoteOutput;
}
// Check for error output first
const errorItem = output.items.find(item => item.mime === 'application/vnd.code.notebook.error');
if (errorItem) {
deepnoteOutput.output_type = 'error';
try {
const errorData = JSON.parse(new TextDecoder().decode(errorItem.data));
deepnoteOutput.ename = errorData.ename || 'Error';
deepnoteOutput.evalue = errorData.evalue || '';
deepnoteOutput.traceback = errorData.traceback || [];
} catch {
// Fallback if error data is not valid JSON
const errorText = new TextDecoder().decode(errorItem.data);
deepnoteOutput.ename = 'Error';
deepnoteOutput.evalue = errorText;
deepnoteOutput.traceback = [errorText];
}
return deepnoteOutput;
}
// Check for stream outputs (stdout/stderr)
const streamMimes = [
'text/plain',
'application/vnd.code.notebook.stdout',
'application/vnd.code.notebook.stderr'
];
const streamItems = output.items.filter(item => streamMimes.includes(item.mime));
if (streamItems.length > 0) {
deepnoteOutput.output_type = 'stream';
// Combine all stream text
const streamTexts = streamItems.map(item => new TextDecoder().decode(item.data));
deepnoteOutput.text = streamTexts.join('');
// Only set stream name if we can determine it from mime type (don't default to stdout)
const stderrItem = streamItems.find(item => item.mime === 'application/vnd.code.notebook.stderr');
if (stderrItem) {
deepnoteOutput.name = 'stderr';
}
// Don't set name = 'stdout' as default to preserve round-trip compatibility
return deepnoteOutput;
}
// Handle rich/display data outputs
deepnoteOutput.data = {};
let hasDisplayData = false;
for (const item of output.items) {
if (!streamMimes.includes(item.mime) && item.mime !== 'application/vnd.code.notebook.error') {
try {
// Handle different mime types appropriately
if (item.mime.startsWith('image/')) {
// For images, convert to base64 string for Deepnote format
const base64String = btoa(String.fromCharCode(...new Uint8Array(item.data)));
deepnoteOutput.data[item.mime] = `data:${item.mime};base64,${base64String}`;
} else if (item.mime === 'application/json') {
// Handle JSON specifically
const decodedContent = new TextDecoder().decode(item.data);
try {
deepnoteOutput.data[item.mime] = JSON.parse(decodedContent);
} catch {
deepnoteOutput.data[item.mime] = decodedContent;
}
} else if (item.mime.includes('json') || item.mime.startsWith('application/')) {
// Try to parse other application types as JSON, fallback to string
const decodedContent = new TextDecoder().decode(item.data);
try {
deepnoteOutput.data[item.mime] = JSON.parse(decodedContent);
} catch {
deepnoteOutput.data[item.mime] = decodedContent;
}
} else {
// For text-based content, decode as string
const decodedContent = new TextDecoder().decode(item.data);
deepnoteOutput.data[item.mime] = decodedContent;
}
hasDisplayData = true;
} catch (error) {
// Fallback: treat as text if any processing fails
try {
const decodedContent = new TextDecoder().decode(item.data);
deepnoteOutput.data[item.mime] = decodedContent;
hasDisplayData = true;
} catch {
// Skip this item if even text decoding fails
console.warn(`Failed to process output item with mime type: ${item.mime}`, error);
}
}
}
}
if (hasDisplayData) {
// Use display_data for rich outputs without execution count, execute_result for those with
deepnoteOutput.output_type = deepnoteOutput.execution_count ? 'execute_result' : 'display_data';
}
return deepnoteOutput;
});
}
/**
* Creates error output items from Deepnote error output.
* @param output Deepnote error output
* @returns Array of VS Code output items representing the error
*/
private createErrorOutput(output: DeepnoteOutput): NotebookCellOutputItem[] {
// Build comprehensive error message with structured data
const baseMessage = output.text || l10n.t('Error occurred during execution');
// Collect structured error details
const errorDetails: string[] = [];
if (output.ename) {
errorDetails.push(l10n.t('Error Name: {0}', output.ename));
}
if (output.evalue) {
errorDetails.push(l10n.t('Error Value: {0}', output.evalue));
}
// Add any additional structured fields from metadata or direct properties
if (output.error) {
errorDetails.push(l10n.t('Error Details: {0}', JSON.stringify(output.error)));
}
if (output.name && output.name !== output.ename) {
errorDetails.push(l10n.t('Error Type: {0}', output.name));
}
if (output.stack) {
errorDetails.push(l10n.t('Stack Trace: {0}', output.stack));
}
// Include traceback if available
if (output.traceback && Array.isArray(output.traceback) && output.traceback.length > 0) {
errorDetails.push(l10n.t('Traceback:\n{0}', output.traceback.join('\n')));
}
// Combine base message with structured details
const fullErrorMessage = errorDetails.length > 0
? `${baseMessage}\n\n${l10n.t('Error Details:')}\n${errorDetails.join('\n')}`
: baseMessage;
const error = new Error(fullErrorMessage);
// Add structured data as error properties for debugging
if (output.ename) error.name = output.ename;
if (output.evalue) Object.assign(error, { evalue: output.evalue });
if (output.traceback) Object.assign(error, { traceback: output.traceback });
if (output.error) Object.assign(error, { deepnoteError: output.error });
return [NotebookCellOutputItem.error(error)];
}
/**
* Creates output items based on the output type.
* Routes to appropriate handlers for stream, error, or rich output.
* @param output Deepnote output to convert
* @returns Array of VS Code output items
*/
private createOutputItems(output: DeepnoteOutput): NotebookCellOutputItem[] {
if (output.output_type === 'stream') {
return this.createStreamOutput(output);
}
if (output.output_type === 'error') {
return this.createErrorOutput(output);
}
if (output.output_type === 'execute_result' || output.output_type === 'display_data') {
return this.createRichOutput(output);
}
// Fallback for unknown types with text
if (output.text) {
return [NotebookCellOutputItem.text(output.text)];
}
return [];
}
/**
* Creates VS Code output item for a specific MIME type and content.
* Handles proper encoding for different content types (text, images, JSON, etc.).
* @param mimeType MIME type of the content
* @param content Content data to convert
* @returns VS Code output item or null if conversion fails
*/
private createOutputItemForMimeType(mimeType: string, content: unknown): NotebookCellOutputItem | null {
if (mimeType === 'text/plain') {
return NotebookCellOutputItem.text(content as string);
}
if (mimeType === 'text/html') {
return NotebookCellOutputItem.text(content as string, 'text/html');
}
// Handle image/* mime types
if (mimeType.startsWith('image/')) {
try {
let uint8Array: Uint8Array;
if (typeof content === 'string') {
// Assume base64 encoded string, decode it
const base64Data = content.includes(',') ? content.split(',')[1] : content;
const binaryString = atob(base64Data);
uint8Array = new Uint8Array(binaryString.length);
for (let i = 0; i < binaryString.length; i++) {
uint8Array[i] = binaryString.charCodeAt(i);
}
} else if (content instanceof ArrayBuffer) {
uint8Array = new Uint8Array(content);
} else if (content instanceof Uint8Array) {
uint8Array = content;
} else {
return null; // Invalid content type for image
}
return NotebookCellOutputItem.binary(uint8Array, mimeType);
} catch (error) {
// Fallback to text representation if binary handling fails
return NotebookCellOutputItem.text(String(content), mimeType);
}
}
// Handle JSON specifically
if (mimeType === 'application/json') {
try {
let jsonObject: unknown;
if (typeof content === 'string') {
jsonObject = JSON.parse(content);
} else if (typeof content === 'object' && content !== null) {
jsonObject = content;
} else {
return NotebookCellOutputItem.text(String(content), mimeType);
}
// Always use text representation with original mime type to preserve application/json
return NotebookCellOutputItem.text(JSON.stringify(jsonObject, null, 2), mimeType);
} catch (error) {
// Fallback to text representation if JSON parsing fails
return NotebookCellOutputItem.text(String(content), mimeType);
}
}
// Handle other application/* mime types
if (mimeType.startsWith('application/')) {
const jsonContent = typeof content === 'string' ? content : JSON.stringify(content, null, 2);
return NotebookCellOutputItem.text(jsonContent, mimeType);
}
return null;
}
/**
* Creates rich output items from display or execute result data.
* @param output Deepnote output with rich data
* @returns Array of VS Code output items
*/
private createRichOutput(output: DeepnoteOutput): NotebookCellOutputItem[] {
if (!output.data) {
return output.text ? [NotebookCellOutputItem.text(output.text)] : [];
}
const items: NotebookCellOutputItem[] = [];
for (const [mimeType, content] of Object.entries(output.data)) {
const item = this.createOutputItemForMimeType(mimeType, content);
if (item) {
items.push(item);
}
}
return items;
}
/**
* Creates stream output items (stdout/stderr) from Deepnote stream output.
* @param output Deepnote stream output
* @returns Array of VS Code output items
*/
private createStreamOutput(output: DeepnoteOutput): NotebookCellOutputItem[] {
if (!output.text) {
return [];
}
// Route to appropriate stream type based on Deepnote stream name
if (output.name === 'stderr') {
return [NotebookCellOutputItem.stderr(output.text)];
} else {
// Default to stdout for 'stdout' name or any other/missing stream name
return [NotebookCellOutputItem.stdout(output.text)];
}
}
/**
* Generates a random 32-character hexadecimal block ID.
* @returns Random block ID string
*/
private generateBlockId(): string {
const chars = '0123456789abcdef';
let id = '';
for (let i = 0; i < 32; i++) {
id += chars[Math.floor(Math.random() * chars.length)];
}
return id;
}
/**
* Generates a sorting key for a block based on its index.
* Uses alphabetical prefix with numeric suffix (a0, a1, ..., b0, b1, etc.).
* @param index Zero-based index of the block
* @returns Sorting key string
*/
private generateSortingKey(index: number): string {
const alphabet = 'abcdefghijklmnopqrstuvwxyz';
const letterIndex = Math.floor(index / 100);
const letter = letterIndex < alphabet.length ? alphabet[letterIndex] : 'z';
const number = index % 100;
return `${letter}${number}`;
}
}src/notebooks/deepnote/deepnoteNotebookManager.ts import type { DeepnoteProject } from './deepnoteTypes';
/**
* Centralized manager for tracking Deepnote notebook selections and project state.
* Manages per-project and per-URI state including current selections and user preferences.
*/
export class DeepnoteNotebookManager {
/** Maps project IDs to currently selected notebook IDs */
private currentNotebookId = new Map<string, string>();
/** Maps project IDs to original project data for serialization */
private originalProjects = new Map<string, DeepnoteProject>();
/** Maps file URIs to user-selected notebook IDs for persistence */
private selectedNotebookByUri = new Map<string, string>();
/** Set of URIs that should skip the selection prompt on next open */
private skipPromptForUri = new Set<string>();
/**
* Gets the currently selected notebook ID for a project.
* @param projectId Project identifier
* @returns Current notebook ID or undefined if not set
*/
getCurrentNotebookId(projectId: string): string | undefined {
return this.currentNotebookId.get(projectId);
}
/**
* Retrieves the original project data for a given project ID.
* @param projectId Project identifier
* @returns Original project data or undefined if not found
*/
getOriginalProject(projectId: string): DeepnoteProject | undefined {
return this.originalProjects.get(projectId);
}
/**
* Gets the selected notebook ID for a specific file URI.
* @param uri File URI string
* @returns Selected notebook ID or undefined if not set
*/
getSelectedNotebookForUri(uri: string): string | undefined {
return this.selectedNotebookByUri.get(uri);
}
/**
* Associates a notebook ID with a file URI to remember user's notebook selection.
* When a Deepnote file contains multiple notebooks, this mapping persists the user's
* choice so we can automatically open the same notebook on subsequent file opens.
* Also marks the URI to skip the selection prompt on the next immediate open.
*
* @param uri The file URI (or project ID) that identifies the Deepnote file
* @param notebookId The ID of the selected notebook within the file
*/
setSelectedNotebookForUri(uri: string, notebookId: string): void {
this.selectedNotebookByUri.set(uri, notebookId);
this.skipPromptForUri.add(uri);
}
/**
* Checks if prompts should be skipped for a given URI and consumes the skip flag.
* This is used to avoid showing selection prompts immediately after a user makes a choice.
* @param uri File URI string
* @returns True if prompts should be skipped (and resets the flag)
*/
shouldSkipPrompt(uri: string): boolean {
if (this.skipPromptForUri.has(uri)) {
this.skipPromptForUri.delete(uri);
return true;
}
return false;
}
/**
* Stores the original project data and sets the initial current notebook.
* This is used during deserialization to cache project data and track the active notebook.
* @param projectId Project identifier
* @param project Original project data to store
* @param notebookId Initial notebook ID to set as current
*/
storeOriginalProject(projectId: string, project: DeepnoteProject, notebookId: string): void {
this.originalProjects.set(projectId, project);
this.currentNotebookId.set(projectId, notebookId);
}
/**
* Updates the current notebook ID for a project.
* Used when switching notebooks within the same project.
* @param projectId Project identifier
* @param notebookId New current notebook ID
*/
updateCurrentNotebookId(projectId: string, notebookId: string): void {
this.currentNotebookId.set(projectId, notebookId);
}
}src/notebooks/deepnote/deepnoteNotebookSelector.ts import { l10n, type QuickPickItem, window } from 'vscode';
import { toPromise } from '../../platform/common/utils/events';
import type { DeepnoteNotebook } from './deepnoteTypes';
/**
* Extends QuickPickItem to include the associated notebook object.
*/
interface NotebookQuickPickItem extends QuickPickItem {
/** The notebook this quick pick item represents */
notebook: DeepnoteNotebook;
}
/**
* Provides user interface for selecting notebooks within a Deepnote project.
* Creates and manages VS Code QuickPick interface for notebook selection.
*/
export class DeepnoteNotebookSelector {
/**
* Presents a notebook selection interface to the user.
* @param notebooks Available notebooks to choose from
* @param currentNotebookId Currently selected notebook ID for pre-selection
* @param options Optional configuration for the selection UI
* @returns Promise resolving to selected notebook or undefined if cancelled
*/
async selectNotebook(
notebooks: DeepnoteNotebook[],
currentNotebookId?: string,
options?: {
title?: string;
placeHolder?: string;
}
): Promise<DeepnoteNotebook | undefined> {
const items: NotebookQuickPickItem[] = notebooks.map((notebook) => ({
label: notebook.name,
description: this.getDescription(notebook, currentNotebookId),
detail: this.getDetail(notebook),
notebook
}));
// Use createQuickPick for more control over selection
const quickPick = window.createQuickPick<NotebookQuickPickItem>();
quickPick.items = items;
quickPick.placeholder = options?.placeHolder || l10n.t('Select a notebook to open');
quickPick.title = options?.title || l10n.t('Select Notebook');
quickPick.ignoreFocusOut = false;
// Pre-select the current notebook if provided
if (currentNotebookId) {
const activeItem = items.find((item) => item.notebook.id === currentNotebookId);
if (activeItem) {
quickPick.activeItems = [activeItem];
}
}
let accepted = false;
quickPick.show();
await Promise.race([
toPromise(quickPick.onDidAccept).then(() => (accepted = true)),
toPromise(quickPick.onDidHide)
]);
const selectedItem = accepted ? quickPick.selectedItems[0] : undefined;
quickPick.dispose();
return selectedItem?.notebook;
}
/**
* Generates description text for a notebook in the selection list.
* Shows cell count and marks the currently active notebook.
* @param notebook Notebook to describe
* @param currentNotebookId Currently selected notebook ID
* @returns Description string showing cell count and current status
*/
private getDescription(notebook: DeepnoteNotebook, currentNotebookId?: string): string {
const cellCount = notebook.blocks.length;
if (notebook.id === currentNotebookId) {
return l10n.t('{0} cells (current)', cellCount);
}
return l10n.t('{0} cells', cellCount);
}
/**
* Generates detail text for a notebook in the selection list.
* Shows notebook ID and working directory if available.
* @param notebook Notebook to describe
* @returns Detail string showing notebook ID and working directory
*/
private getDetail(notebook: DeepnoteNotebook): string {
if (notebook.workingDirectory) {
return l10n.t('ID: {0} | Working Directory: {1}', notebook.id, notebook.workingDirectory);
}
return l10n.t('ID: {0}', notebook.id);
}
}src/notebooks/deepnote/deepnoteSerializer.ts import { type NotebookData, type NotebookSerializer, type CancellationToken } from 'vscode';
import * as yaml from 'js-yaml';
import { DeepnoteProject, DeepnoteNotebook } from './deepnoteTypes';
import { DeepnoteNotebookManager } from './deepnoteNotebookManager';
import { DeepnoteNotebookSelector } from './deepnoteNotebookSelector';
import { DeepnoteDataConverter } from './deepnoteDataConverter';
export { DeepnoteProject, DeepnoteNotebook, DeepnoteBlock, DeepnoteOutput } from './deepnoteTypes';
/**
* Callback function type for handling notebook selection during deserialization.
* @param projectId Project identifier containing the notebooks
* @param notebooks Available notebooks to choose from
* @returns Promise resolving to selected notebook or undefined
*/
export type NotebookSelectionCallback = (
projectId: string,
notebooks: DeepnoteNotebook[]
) => Promise<DeepnoteNotebook | undefined>;
/**
* Serializer for converting between Deepnote YAML files and VS Code notebook format.
* Handles reading/writing .deepnote files and manages project state persistence.
*/
export class DeepnoteNotebookSerializer implements NotebookSerializer {
/** Manager for tracking notebook selections and project state */
private manager = new DeepnoteNotebookManager();
/** UI component for notebook selection */
private selector = new DeepnoteNotebookSelector();
/** Utility for converting between Deepnote and VS Code formats */
private converter = new DeepnoteDataConverter();
/** Optional callback for custom notebook selection logic */
private notebookSelectionCallback?: NotebookSelectionCallback;
/**
* Gets the notebook manager instance for accessing project state.
* @returns DeepnoteNotebookManager instance
*/
getManager(): DeepnoteNotebookManager {
return this.manager;
}
/**
* Gets the data converter instance for cell/block conversion.
* @returns DeepnoteDataConverter instance
*/
getConverter(): DeepnoteDataConverter {
return this.converter;
}
/**
* Sets a custom callback for handling notebook selection during deserialization.
* @param callback Function to call when notebook selection is needed
*/
setNotebookSelectionCallback(callback: NotebookSelectionCallback) {
this.notebookSelectionCallback = callback;
}
/**
* Deserializes a Deepnote YAML file into VS Code notebook format.
* Parses YAML, selects appropriate notebook, and converts blocks to cells.
* @param content Raw file content as bytes
* @param _token Cancellation token (unused)
* @returns Promise resolving to notebook data
*/
async deserializeNotebook(content: Uint8Array, _token: CancellationToken): Promise<NotebookData> {
try {
const contentString = Buffer.from(content).toString('utf8');
const deepnoteProject = yaml.load(contentString) as DeepnoteProject;
if (!deepnoteProject.project?.notebooks) {
throw new Error('Invalid Deepnote file: no notebooks found');
}
const selectedNotebook = this.notebookSelectionCallback
? await this.notebookSelectionCallback(deepnoteProject.project.id, deepnoteProject.project.notebooks)
: await this.selectNotebookForOpen(deepnoteProject.project.id, deepnoteProject.project.notebooks);
if (!selectedNotebook) {
throw new Error('No notebook selected');
}
const cells = this.converter.convertBlocksToCells(selectedNotebook.blocks);
// Store the original project for later serialization
this.manager.storeOriginalProject(deepnoteProject.project.id, deepnoteProject, selectedNotebook.id);
return {
cells,
metadata: {
deepnoteProjectId: deepnoteProject.project.id,
deepnoteProjectName: deepnoteProject.project.name,
deepnoteNotebookId: selectedNotebook.id,
deepnoteNotebookName: selectedNotebook.name,
deepnoteVersion: deepnoteProject.version
}
};
} catch (error) {
console.error('Error deserializing Deepnote notebook:', error);
throw new Error(
`Failed to parse Deepnote file: ${error instanceof Error ? error.message : 'Unknown error'}`
);
}
}
/**
* Serializes VS Code notebook data back to Deepnote YAML format.
* Converts cells to blocks, updates project data, and generates YAML.
* @param data Notebook data to serialize
* @param _token Cancellation token (unused)
* @returns Promise resolving to YAML content as bytes
*/
async serializeNotebook(data: NotebookData, _token: CancellationToken): Promise<Uint8Array> {
try {
const projectId = data.metadata?.deepnoteProjectId;
if (!projectId) {
throw new Error('Missing Deepnote project ID in notebook metadata');
}
const originalProject = this.manager.getOriginalProject(projectId);
if (!originalProject) {
throw new Error('Original Deepnote project not found. Cannot save changes.');
}
// Get the current notebook ID (may have changed due to switching)
const notebookId = data.metadata?.deepnoteNotebookId || this.manager.getCurrentNotebookId(projectId);
if (!notebookId) {
throw new Error('Cannot determine which notebook to save');
}
// Find the notebook to update
const notebookIndex = originalProject.project.notebooks.findIndex((nb) => nb.id === notebookId);
if (notebookIndex === -1) {
throw new Error(`Notebook with ID ${notebookId} not found in project`);
}
// Create a deep copy of the project to modify
const updatedProject = JSON.parse(JSON.stringify(originalProject)) as DeepnoteProject;
// Convert cells back to blocks
const updatedBlocks = this.converter.convertCellsToBlocks(data.cells);
// Update the notebook's blocks
updatedProject.project.notebooks[notebookIndex].blocks = updatedBlocks;
// Update modification timestamp
updatedProject.metadata.modifiedAt = new Date().toISOString();
// Convert to YAML
const yamlString = yaml.dump(updatedProject, {
indent: 2,
lineWidth: -1,
noRefs: true,
sortKeys: false
});
// Store the updated project for future saves
this.manager.storeOriginalProject(projectId, updatedProject, notebookId);
return new TextEncoder().encode(yamlString);
} catch (error) {
console.error('Error serializing Deepnote notebook:', error);
throw new Error(
`Failed to save Deepnote file: ${error instanceof Error ? error.message : 'Unknown error'}`
);
}
}
/**
* Handles notebook selection logic during file opening.
* Manages automatic selection, stored preferences, and user prompts.
* @param projectId Project identifier containing the notebooks
* @param notebooks Available notebooks to choose from
* @returns Promise resolving to selected notebook
*/
private async selectNotebookForOpen(
projectId: string,
notebooks: DeepnoteNotebook[]
): Promise<DeepnoteNotebook | undefined> {
const fileId = projectId;
const skipPrompt = this.manager.shouldSkipPrompt(fileId);
const storedNotebookId = this.manager.getSelectedNotebookForUri(fileId);
if (notebooks.length === 1) {
return notebooks[0];
}
if (skipPrompt && storedNotebookId) {
// Use the stored selection when triggered by command
const preSelected = notebooks.find((nb) => nb.id === storedNotebookId);
return preSelected || notebooks[0];
}
if (storedNotebookId && !skipPrompt) {
// Normal file open - check if we have a previously selected notebook
const preSelected = notebooks.find((nb) => nb.id === storedNotebookId);
if (preSelected) {
return preSelected;
}
// Previously selected notebook not found, prompt for selection
}
// Prompt user to select a notebook
const selected = await this.selector.selectNotebook(notebooks);
if (selected) {
this.manager.setSelectedNotebookForUri(fileId, selected.id);
return selected;
}
// If user cancelled selection, default to the first notebook
return notebooks[0];
}
}These comprehensive JSDoc docstrings document all the TypeScript classes, interfaces, methods, and their purposes, parameters, and return values. The documentation follows TypeScript conventions and provides clear explanations of the Deepnote integration functionality. ✏️ Learnings added
🧠 Learnings used |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 33
♻️ Duplicate comments (4)
src/notebooks/deepnote/deepnoteDataConverter.ts (4)
1-1: No repo header required (project policy).Per current repo policy for new Deepnote extension files, omit copyright headers.
47-48: Ensure stable, collision-free sortingKey generation.Confirm generateSortingKey(index) returns a zero-padded numeric string (e.g., 00000042). Otherwise order/collisions can occur at scale.
Would mirror prior guidance; update dataConversionUtils if needed.
58-61: executionCount bug: use nullish coalescing.0 is valid; current “||” drops it.
- const executionCount = cell.metadata?.executionCount || cell.executionSummary?.executionOrder; + const executionCount = cell.metadata?.executionCount ?? cell.executionSummary?.executionOrder;
115-123: Avoid duplicating executionCount in metadata.Strip executionCount from metadata once promoted to execution_count.
- if (output.metadata) { - deepnoteOutput.metadata = mergeMetadata(deepnoteOutput.metadata, output.metadata); - - // Extract execution count from metadata - if (output.metadata.executionCount !== undefined) { - deepnoteOutput.execution_count = output.metadata.executionCount as number; - } - } + if (output.metadata) { + const { executionCount, ...rest } = output.metadata as Record<string, unknown>; + deepnoteOutput.metadata = mergeMetadata(deepnoteOutput.metadata, rest); + if (executionCount !== undefined) { + deepnoteOutput.execution_count = executionCount as number; + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
src/notebooks/deepnote/MimeTypeProcessor.ts(1 hunks)src/notebooks/deepnote/OutputTypeDetector.ts(1 hunks)src/notebooks/deepnote/dataConversionUtils.ts(1 hunks)src/notebooks/deepnote/deepnoteDataConverter.ts(1 hunks)src/notebooks/deepnote/outputHandlers/ErrorOutputHandler.ts(1 hunks)src/notebooks/deepnote/outputHandlers/RichOutputHandler.ts(1 hunks)src/notebooks/deepnote/outputHandlers/StreamOutputHandler.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/!(*.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/outputHandlers/StreamOutputHandler.tssrc/notebooks/deepnote/dataConversionUtils.tssrc/notebooks/deepnote/outputHandlers/RichOutputHandler.tssrc/notebooks/deepnote/OutputTypeDetector.tssrc/notebooks/deepnote/outputHandlers/ErrorOutputHandler.tssrc/notebooks/deepnote/MimeTypeProcessor.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
Files:
src/notebooks/deepnote/outputHandlers/StreamOutputHandler.tssrc/notebooks/deepnote/dataConversionUtils.tssrc/notebooks/deepnote/outputHandlers/RichOutputHandler.tssrc/notebooks/deepnote/OutputTypeDetector.tssrc/notebooks/deepnote/outputHandlers/ErrorOutputHandler.tssrc/notebooks/deepnote/MimeTypeProcessor.tssrc/notebooks/deepnote/deepnoteDataConverter.ts
🧠 Learnings (11)
📚 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/outputHandlers/StreamOutputHandler.tssrc/notebooks/deepnote/dataConversionUtils.tssrc/notebooks/deepnote/outputHandlers/RichOutputHandler.tssrc/notebooks/deepnote/outputHandlers/ErrorOutputHandler.tssrc/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 : When adding a new export format, create a new class implementing IExport within the export module
Applied to files:
src/notebooks/deepnote/outputHandlers/StreamOutputHandler.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 : On conversion failures, provide detailed error reporting with suggested fixes
Applied to files:
src/notebooks/deepnote/outputHandlers/RichOutputHandler.tssrc/notebooks/deepnote/outputHandlers/ErrorOutputHandler.tssrc/notebooks/deepnote/deepnoteDataConverter.ts
📚 Learning: 2025-09-10T06:25:39.465Z
Learnt from: Artmann
PR: deepnote/vscode-deepnote#0
File: :0-0
Timestamp: 2025-09-10T06:25:39.465Z
Learning: Generated comprehensive JSDoc-style docstrings for all Deepnote TypeScript classes and interfaces in the VS Code extension, including detailed parameter descriptions, return value explanations, and class/interface purposes following TypeScript documentation conventions.
Applied to files:
src/notebooks/deepnote/outputHandlers/RichOutputHandler.tssrc/notebooks/deepnote/deepnoteDataConverter.ts
📚 Learning: 2025-09-10T06:25:39.465Z
Learnt from: Artmann
PR: deepnote/vscode-deepnote#0
File: :0-0
Timestamp: 2025-09-10T06:25:39.465Z
Learning: Generated comprehensive JSDoc-style docstrings for the Deepnote TypeScript classes and interfaces in the VS Code extension, following TypeScript documentation conventions with parameter descriptions and return value explanations.
Applied to files:
src/notebooks/deepnote/outputHandlers/RichOutputHandler.tssrc/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 : Add the new format to the ExportFormat enum
Applied to files:
src/notebooks/deepnote/OutputTypeDetector.ts
📚 Learning: 2025-09-09T11:31:30.479Z
Learnt from: Artmann
PR: deepnote/vscode-extension#11
File: src/notebooks/deepnote/deepnoteActivationService.ts:1-3
Timestamp: 2025-09-09T11:31:30.479Z
Learning: New files in the Deepnote VS Code extension project should not include copyright headers.
Applied to files:
src/notebooks/deepnote/deepnoteDataConverter.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/**/*.ts : Notebook-level execution must enqueue cells via CellExecutionQueue and manage per-cell lifecycles through CellExecution
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 `l10n.t()` for user-facing strings
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} : Preserve error details while scrubbing PII in messages and telemetry
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/notebooks/deepnote/deepnoteDataConverter.ts
🧬 Code graph analysis (5)
src/notebooks/deepnote/outputHandlers/StreamOutputHandler.ts (2)
src/test/mocks/vsc/extHostedTypes.ts (2)
NotebookCellOutput(71-103)NotebookCellOutputItem(16-69)src/notebooks/deepnote/dataConversionUtils.ts (1)
decodeContent(8-10)
src/notebooks/deepnote/outputHandlers/RichOutputHandler.ts (4)
src/notebooks/deepnote/MimeTypeProcessor.ts (1)
MimeTypeProcessorRegistry(147-169)src/notebooks/deepnote/OutputTypeDetector.ts (1)
OutputTypeDetector(14-51)src/test/mocks/vsc/extHostedTypes.ts (2)
NotebookCellOutput(71-103)NotebookCellOutputItem(16-69)src/notebooks/deepnote/dataConversionUtils.ts (1)
decodeContent(8-10)
src/notebooks/deepnote/OutputTypeDetector.ts (1)
src/test/mocks/vsc/extHostedTypes.ts (1)
NotebookCellOutput(71-103)
src/notebooks/deepnote/outputHandlers/ErrorOutputHandler.ts (2)
src/test/mocks/vsc/extHostedTypes.ts (2)
NotebookCellOutput(71-103)NotebookCellOutputItem(16-69)src/notebooks/deepnote/dataConversionUtils.ts (2)
parseJsonSafely(15-21)decodeContent(8-10)
src/notebooks/deepnote/MimeTypeProcessor.ts (2)
src/test/mocks/vsc/extHostedTypes.ts (1)
NotebookCellOutputItem(16-69)src/notebooks/deepnote/dataConversionUtils.ts (2)
convertBase64ToUint8Array(26-34)parseJsonSafely(15-21)
🪛 GitHub Actions: CI
src/notebooks/deepnote/outputHandlers/StreamOutputHandler.ts
[warning] 1-1: Prettier formatting issue detected. Run 'prettier --write' to fix this file.
src/notebooks/deepnote/dataConversionUtils.ts
[warning] 1-1: Prettier formatting issue detected. Run 'prettier --write' to fix this file.
src/notebooks/deepnote/outputHandlers/RichOutputHandler.ts
[warning] 1-1: Prettier formatting issue detected. Run 'prettier --write' to fix this file.
src/notebooks/deepnote/OutputTypeDetector.ts
[warning] 1-1: Prettier formatting issue detected. Run 'prettier --write' to fix this file.
src/notebooks/deepnote/outputHandlers/ErrorOutputHandler.ts
[warning] 1-1: Prettier formatting issue detected. Run 'prettier --write' to fix this file.
src/notebooks/deepnote/MimeTypeProcessor.ts
[warning] 1-1: Prettier formatting issue detected. Run 'prettier --write' to fix this file.
src/notebooks/deepnote/deepnoteDataConverter.ts
[warning] 1-1: Prettier formatting issue detected. Run 'prettier --write' to fix this file.
🔇 Additional comments (6)
src/notebooks/deepnote/OutputTypeDetector.ts (1)
1-51: Apply formatting fixes
Runnpm run format-fixto auto-apply Prettier fixes and satisfy CI.⛔ Skipped due to learnings
Learnt from: CR PR: deepnote/vscode-extension#0 File: .github/instructions/typescript.instructions.md:0-0 Timestamp: 2025-09-03T13:01:49.096Z Learning: Use `npm run format-fix` to auto-fix formatting issuesLearnt from: CR PR: deepnote/vscode-extension#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-09-03T12:53:28.421Z Learning: Run `npm run format-fix` before committing any changesLearnt from: CR PR: deepnote/vscode-extension#0 File: .github/instructions/typescript.instructions.md:0-0 Timestamp: 2025-09-03T13:01:49.096Z Learning: ALWAYS run `npm run format-fix` before committing changesLearnt from: CR PR: deepnote/vscode-extension#0 File: .github/instructions/typescript.instructions.md:0-0 Timestamp: 2025-09-03T13:01:49.096Z Learning: Use `npm run format` to check code styleLearnt from: CR PR: deepnote/vscode-extension#0 File: .github/instructions/typescript.instructions.md:0-0 Timestamp: 2025-09-03T13:01:49.096Z Learning: Pre-commit: run `npm run format-fix`, `npm run lint`, and `npm run test:unittests` before committingLearnt from: CR PR: deepnote/vscode-extension#0 File: .github/instructions/typescript.instructions.md:0-0 Timestamp: 2025-09-03T13:01:49.096Z Learning: Use `npm run lint` to check for linter issuesLearnt from: CR PR: deepnote/vscode-extension#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-09-03T12:53:28.421Z Learning: Run linting with `npm run lint` to check for linter issuessrc/notebooks/deepnote/MimeTypeProcessor.ts (1)
1-169: Format code with npm script
Runnpm run format-fixto apply Prettier formatting tosrc/notebooks/deepnote/MimeTypeProcessor.ts.src/notebooks/deepnote/outputHandlers/StreamOutputHandler.ts (1)
42-54: LGTM on mapping.stdout/stderr mapping is correct; empty text returns no items.
src/notebooks/deepnote/outputHandlers/RichOutputHandler.ts (1)
67-77: LGTM: VS Code -> items conversion path is clean.Iterating data entries and delegating to the registry is the right approach.
src/notebooks/deepnote/outputHandlers/ErrorOutputHandler.ts (1)
45-64: LGTM: VS Code error output conversion path.Creating a single NotebookCellOutputItem.error from a constructed Error is correct.
src/notebooks/deepnote/deepnoteDataConverter.ts (1)
1-1: Prettier formatting applied to src/notebooks/deepnote/deepnoteDataConverter.ts
Formatting issues resolved; CI formatting check will now pass.
Comment on lines
+1
to
+10
| /** | ||
| * Utility functions for data transformation in Deepnote conversion | ||
| */ | ||
|
|
||
| /** | ||
| * Safely decode content using TextDecoder | ||
| */ | ||
| export function decodeContent(data: Uint8Array): string { | ||
| return new TextDecoder().decode(data); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Docstrings: add param/return tags for utilities.
Minimal JSDoc tags help consumers and tooltips; can generate for all exports on request.
Want me to push a commit adding full JSDoc across this module?
🧰 Tools
🪛 GitHub Actions: CI
[warning] 1-1: Prettier formatting issue detected. Run 'prettier --write' to fix this file.
🤖 Prompt for AI Agents
In src/notebooks/deepnote/dataConversionUtils.ts around lines 1 to 10, the
exported utility lacks JSDoc @param and @returns tags; add a JSDoc block above
the decodeContent export that documents the function purpose, the parameter
(data: Uint8Array) with its description, and the return type (string) with a
brief note about decoding via TextDecoder; keep descriptions short, use standard
@param and @returns tags, and update any other exported utilities in this file
similarly.
| @@ -0,0 +1,91 @@ | |||
| /** | |||
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Formatting and header.
Prettier flagged this file. Also add the repo’s required copyright/license header.
Run: prettier --write src/notebooks/deepnote/dataConversionUtils.ts
🧰 Tools
🪛 GitHub Actions: CI
[warning] 1-1: Prettier formatting issue detected. Run 'prettier --write' to fix this file.
🤖 Prompt for AI Agents
In src/notebooks/deepnote/dataConversionUtils.ts around line 1, the file
header/formatting doesn't match project standards; run Prettier and add the
repository's copyright/license header. Reformat the file using "prettier --write
src/notebooks/deepnote/dataConversionUtils.ts" and insert the project's required
header comment at the top (copy from other files in the repo to ensure exact
wording and year/license), then commit the updated file.
Comment on lines
+26
to
+34
| export function convertBase64ToUint8Array(base64Content: string): Uint8Array { | ||
| const base64Data = base64Content.includes(',') ? base64Content.split(',')[1] : base64Content; | ||
| const binaryString = atob(base64Data); | ||
| const uint8Array = new Uint8Array(binaryString.length); | ||
| for (let i = 0; i < binaryString.length; i++) { | ||
| uint8Array[i] = binaryString.charCodeAt(i); | ||
| } | ||
| return uint8Array; | ||
| } |
Contributor
There was a problem hiding this comment.
Node/web incompatibility: uses atob/btoa which are undefined in Node.
Provide cross-platform fallbacks using Buffer when atob/btoa are unavailable.
export function convertBase64ToUint8Array(base64Content: string): Uint8Array {
const base64Data = base64Content.includes(',') ? base64Content.split(',')[1] : base64Content;
- const binaryString = atob(base64Data);
- const uint8Array = new Uint8Array(binaryString.length);
- for (let i = 0; i < binaryString.length; i++) {
- uint8Array[i] = binaryString.charCodeAt(i);
- }
- return uint8Array;
+ if (typeof globalThis.atob === 'function') {
+ const binaryString = atob(base64Data);
+ const uint8Array = new Uint8Array(binaryString.length);
+ for (let i = 0; i < binaryString.length; i++) {
+ uint8Array[i] = binaryString.charCodeAt(i);
+ }
+ return uint8Array;
+ }
+ // Node fallback
+ // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+ // @ts-ignore Buffer is available in Node runtimes
+ return new Uint8Array(Buffer.from(base64Data, 'base64'));
}
@@
export function convertUint8ArrayToBase64DataUrl(data: Uint8Array, mimeType: string): string {
- const base64String = btoa(String.fromCharCode(...data));
- return `data:${mimeType};base64,${base64String}`;
+ if (typeof globalThis.btoa === 'function') {
+ const base64String = btoa(String.fromCharCode(...data));
+ return `data:${mimeType};base64,${base64String}`;
+ }
+ // Node fallback
+ // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+ // @ts-ignore Buffer is available in Node runtimes
+ const base64 = Buffer.from(data).toString('base64');
+ return `data:${mimeType};base64,${base64}`;
}Also applies to: 39-42
🤖 Prompt for AI Agents
In src/notebooks/deepnote/dataConversionUtils.ts around lines 26-34 (and also
apply to 39-42), the code uses atob/btoa which are not available in Node;
replace those calls with cross-platform fallbacks: detect typeof atob ===
'function' and use it in browsers, otherwise use Buffer (Buffer.from(base64Data,
'base64') to get bytes or Buffer.from(uint8Array).toString('base64') to encode).
For convertBase64ToUint8Array, if atob is unavailable create the Uint8Array
directly from Buffer.from(base64Data, 'base64'); for the inverse conversion
(lines 39-42) use btoa when present, otherwise use
Buffer.from(uint8Array).toString('base64'); keep behavior identical and avoid
changing function signatures.
Comment on lines
+73
to
+80
| export function generateBlockId(): string { | ||
| const chars = '0123456789abcdef'; | ||
| let id = ''; | ||
| for (let i = 0; i < 32; i++) { | ||
| id += chars[Math.floor(Math.random() * chars.length)]; | ||
| } | ||
| return id; | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Prefer cryptographically strong IDs when available.
Use crypto.getRandomValues when present; fall back to Math.random otherwise.
export function generateBlockId(): string {
- const chars = '0123456789abcdef';
- let id = '';
- for (let i = 0; i < 32; i++) {
- id += chars[Math.floor(Math.random() * chars.length)];
- }
- return id;
+ const toHex = (b: number) => b.toString(16).padStart(2, '0');
+ const g = (len: number) => Array.from({ length: len }, () => Math.floor(Math.random() * 256));
+ if (typeof globalThis.crypto?.getRandomValues === 'function') {
+ const buf = new Uint8Array(16);
+ globalThis.crypto.getRandomValues(buf);
+ return Array.from(buf, toHex).join('');
+ }
+ return g(16).map(toHex).join('');
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function generateBlockId(): string { | |
| const chars = '0123456789abcdef'; | |
| let id = ''; | |
| for (let i = 0; i < 32; i++) { | |
| id += chars[Math.floor(Math.random() * chars.length)]; | |
| } | |
| return id; | |
| } | |
| export function generateBlockId(): string { | |
| const toHex = (b: number) => b.toString(16).padStart(2, '0'); | |
| const g = (len: number) => Array.from({ length: len }, () => Math.floor(Math.random() * 256)); | |
| if (typeof globalThis.crypto?.getRandomValues === 'function') { | |
| const buf = new Uint8Array(16); | |
| globalThis.crypto.getRandomValues(buf); | |
| return Array.from(buf, toHex).join(''); | |
| } | |
| return g(16).map(toHex).join(''); | |
| } |
🤖 Prompt for AI Agents
In src/notebooks/deepnote/dataConversionUtils.ts around lines 73 to 80, the
generateBlockId function uses Math.random which is not cryptographically strong;
replace the implementation to use crypto.getRandomValues (or
globalThis.crypto?.getRandomValues) when available to generate 16 random bytes
and convert them to a 32-character hex string, and fall back to the existing
Math.random-based approach only if getRandomValues is not present; ensure no
external libs are required and the fallback preserves current behavior.
Comment on lines
18
to
37
| convertToDeepnote(output: NotebookCellOutput): DeepnoteOutput { | ||
| const streamItems = output.items.filter(item => this.streamMimes.includes(item.mime)); | ||
|
|
||
| // Combine all stream text | ||
| const streamTexts = streamItems.map(item => decodeContent(item.data)); | ||
| const text = streamTexts.join(''); | ||
|
|
||
| const deepnoteOutput: DeepnoteOutput = { | ||
| output_type: 'stream', | ||
| text | ||
| }; | ||
|
|
||
| // Only set stream name if we can determine it from mime type | ||
| const stderrItem = streamItems.find(item => item.mime === 'application/vnd.code.notebook.stderr'); | ||
| if (stderrItem) { | ||
| deepnoteOutput.name = 'stderr'; | ||
| } | ||
|
|
||
| return deepnoteOutput; | ||
| } |
Contributor
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Optional: set explicit stream name and early-exit.
Set name: 'stdout' when not stderr (if Deepnote requires it) and short-circuit when no stream items.
Apply:
- const streamItems = output.items.filter(item => this.streamMimes.includes(item.mime));
+ const streamItems = output.items.filter(item => this.streamMimes.includes(item.mime));
+ if (streamItems.length === 0) {
+ return { output_type: 'stream', text: '' };
+ }
@@
- if (stderrItem) {
- deepnoteOutput.name = 'stderr';
- }
+ deepnoteOutput.name = stderrItem ? 'stderr' : 'stdout';Verify DeepnoteOutput.name optional/required:
🏁 Script executed:
#!/bin/bash
rg -nC2 -i 'interface\s+DeepnoteOutput|type\s+DeepnoteOutput' src | sed -n '1,120p'Length of output: 317
🏁 Script executed:
#!/bin/bash
echo "--- Context for DeepnoteOutput ---"
rg -nC5 'export interface DeepnoteOutput' src/notebooks/deepnote/deepnoteTypes.ts
echo "--- Search for name property ---"
rg -n 'name' src/notebooks/deepnote/deepnoteTypes.ts
echo "--- Full interface DeepnoteOutput block ---"
sed -n '1,200p' src/notebooks/deepnote/deepnoteTypes.ts | sed -n '/export interface DeepnoteOutput/,/}/p'Length of output: 690
Optional: set explicit stream name and early-exit.
Add an early return when streamItems is empty, and default deepnoteOutput.name to 'stdout' when stderr isn’t present—name is optional in DeepnoteOutput.
- const streamItems = output.items.filter(item => this.streamMimes.includes(item.mime));
+ const streamItems = output.items.filter(item => this.streamMimes.includes(item.mime));
+ if (streamItems.length === 0) {
+ return { output_type: 'stream', text: '' };
+ }
@@
- if (stderrItem) {
- deepnoteOutput.name = 'stderr';
- }
+ deepnoteOutput.name = stderrItem ? 'stderr' : 'stdout';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| convertToDeepnote(output: NotebookCellOutput): DeepnoteOutput { | |
| const streamItems = output.items.filter(item => this.streamMimes.includes(item.mime)); | |
| // Combine all stream text | |
| const streamTexts = streamItems.map(item => decodeContent(item.data)); | |
| const text = streamTexts.join(''); | |
| const deepnoteOutput: DeepnoteOutput = { | |
| output_type: 'stream', | |
| text | |
| }; | |
| // Only set stream name if we can determine it from mime type | |
| const stderrItem = streamItems.find(item => item.mime === 'application/vnd.code.notebook.stderr'); | |
| if (stderrItem) { | |
| deepnoteOutput.name = 'stderr'; | |
| } | |
| return deepnoteOutput; | |
| } | |
| convertToDeepnote(output: NotebookCellOutput): DeepnoteOutput { | |
| const streamItems = output.items.filter(item => this.streamMimes.includes(item.mime)); | |
| if (streamItems.length === 0) { | |
| return { output_type: 'stream', text: '' }; | |
| } | |
| // Combine all stream text | |
| const streamTexts = streamItems.map(item => decodeContent(item.data)); | |
| const text = streamTexts.join(''); | |
| const deepnoteOutput: DeepnoteOutput = { | |
| output_type: 'stream', | |
| text | |
| }; | |
| // Only set stream name if we can determine it from mime type | |
| const stderrItem = streamItems.find(item => item.mime === 'application/vnd.code.notebook.stderr'); | |
| deepnoteOutput.name = stderrItem ? 'stderr' : 'stdout'; | |
| return deepnoteOutput; | |
| } |
🤖 Prompt for AI Agents
In src/notebooks/deepnote/outputHandlers/StreamOutputHandler.ts around lines 18
to 37, add an early return when streamItems is empty to avoid producing an empty
stream output, and set deepnoteOutput.name to 'stdout' by default when no stderr
item is found; specifically, after computing streamItems, if streamItems.length
=== 0 return a DeepnoteOutput with output_type 'stream' and text '' (or simply
return null/undefined if consumers expect that), and when building
deepnoteOutput set name = 'stderr' if a stderr item exists otherwise set name =
'stdout'.
| @@ -0,0 +1,51 @@ | |||
| import { NotebookCellOutput } from 'vscode'; | |||
Contributor
There was a problem hiding this comment.
Add required copyright header.
Per guidelines, prepend the Microsoft copyright header.
Apply:
+/*---------------------------------------------------------------------------------------------
+ * Copyright (c) Microsoft Corporation.
+ * Licensed under the MIT License.
+ *--------------------------------------------------------------------------------------------*/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import { NotebookCellOutput } from 'vscode'; | |
| /*--------------------------------------------------------------------------------------------- | |
| * Copyright (c) Microsoft Corporation. | |
| * Licensed under the MIT License. | |
| *--------------------------------------------------------------------------------------------*/ | |
| import { NotebookCellOutput } from 'vscode'; |
🧰 Tools
🪛 GitHub Actions: CI
[warning] 1-1: Prettier formatting issue detected. Run 'prettier --write' to fix this file.
🤖 Prompt for AI Agents
In src/notebooks/deepnote/OutputTypeDetector.ts around line 1, the file is
missing the required Microsoft copyright header; prepend the standard Microsoft
copyright header at the very top of the file (before any imports) per repository
guidelines — for example the single-line or multi-line header used across the
repo (e.g., "Copyright (c) Microsoft Corporation. All rights reserved.") —
ensuring formatting and year/owner fields match the project's existing header
convention.
Comment on lines
21
to
46
| detect(output: NotebookCellOutput): OutputTypeResult { | ||
| if (output.items.length === 0) { | ||
| return { type: 'rich' }; | ||
| } | ||
|
|
||
| // Check for error output first | ||
| const errorItem = output.items.find(item => item.mime === 'application/vnd.code.notebook.error'); | ||
| if (errorItem) { | ||
| return { | ||
| type: 'error', | ||
| errorItem: { mime: errorItem.mime, data: errorItem.data } | ||
| }; | ||
| } | ||
|
|
||
| // Check for stream outputs | ||
| const streamItems = output.items.filter(item => this.streamMimes.includes(item.mime)); | ||
| if (streamItems.length > 0) { | ||
| return { | ||
| type: 'stream', | ||
| streamMimes: streamItems.map(item => item.mime) | ||
| }; | ||
| } | ||
|
|
||
| // Default to rich output | ||
| return { type: 'rich' }; | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Minor: centralize MIME constants.
Avoid duplication across handlers; export/import a shared constants module.
🤖 Prompt for AI Agents
In src/notebooks/deepnote/OutputTypeDetector.ts around lines 21 to 46, the file
hardcodes MIME values (error and stream mimes) instead of using a shared
constants module; create or import a shared constants file (e.g.,
src/notebooks/mimeConstants.ts) exporting the error mime
(APPLICATION_NOTEBOOK_ERROR) and STREAM_MIMES array, replace the inline literal
checks and this.streamMimes usage with the imported constants, update any type
imports if needed, and run tests/compile to ensure no unresolved identifiers
remain.
Artmann
force-pushed
the
open-deepnote-file
branch
from
cd1698e to
3bfb053
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
♻️ Duplicate comments (63)
src/standalone/api/kernels/kernel.unit.test.ts (1)
131-134: Avoid disabling ESLint; drain the async iterator instead.Cleaner intent, no lint suppression.
Apply:
- // eslint-disable-next-line @typescript-eslint/no-unused-vars - for await (const _ of api.executeCode('bogus', token)) { - // - } + await drain(api.executeCode('bogus', token));Add once in this file (near the top, after imports):
async function drain<T>(iter: AsyncIterable<T>) { for await (const _ of iter) { /* no-op */ } }package.nls.json (1)
246-247: Localize the new Select Deepnote Notebook command titleAdd a translatable title for the command so it appears localized in the UI. Use a Deepnote-specific label per your preference for a dedicated category.
"jupyter.languageModelTools.notebook_install_packages.userDescription": "Installs Python packages in the selected Notebook Kernel.", - "deepnote.notebook.displayName": "Deepnote Notebook" + "deepnote.notebook.displayName": "Deepnote Notebook", + "deepnote.command.selectNotebook.title": "Select Deepnote Notebook".github/workflows/ci.yml (3)
39-41: Don’t run postinstall twicenpm ci already runs lifecycle scripts; remove redundant step.
- name: Install dependencies run: npm ci --prefer-offline --no-audit - - - name: Run postinstall - run: npm run postinstallAlso applies to: 64-66
10-12: Remove or actually use NPM_VERSIONIt’s declared but unused. Either drop it or activate via Corepack.
env: NODE_VERSION: 22.x - NPM_VERSION: 10.x
45-46: Use Prettier check mode (non-mutating) in CIAvoids formatting writes on CI.
- - name: Check Prettier formatting - run: npm run format + - name: Check Prettier formatting + run: npx prettier --check .CLAUDE.md (1)
1-29: Fix contradictions with coding guidelines and markdownlint errorsRequire the Microsoft header in TS files, add H1, and satisfy markdownlint.
+# Project Conventions + ## Code Style & Organization -- Order method, fields and properties, first by accessibility and then by alphabetical order. -- Don't add the Microsoft copyright header to new files. +- Order methods, fields and properties first by accessibility and then alphabetically. +- Add the Microsoft copyright header to new TypeScript source files. ## Testing - Unit tests use Mocha/Chai framework with `.unit.test.ts` extension - Test files should be placed alongside the source files they test - Run all tests: `npm test` or `npm run test:unittests` - Run single test file: `npx mocha --config ./build/.mocha.unittests.js.json ./out/path/to/file.unit.test.js` - Tests run against compiled JavaScript files in `out/` directory - Use `assert.deepStrictEqual()` for object comparisons instead of checking individual properties ## Project Structure - VSCode extension for Jupyter notebooks - Uses dependency injection with inversify - Follows separation of concerns pattern - TypeScript codebase that compiles to `out/` directory ## Deepnote Integration - Located in `src/notebooks/deepnote/` - Refactored architecture: - `deepnoteTypes.ts` - Type definitions - `deepnoteNotebookManager.ts` - State management - `deepnoteNotebookSelector.ts` - UI selection logic - `deepnoteDataConverter.ts` - Data transformations - `deepnoteSerializer.ts` - Main serializer (orchestration) - `deepnoteActivationService.ts` - VSCode activation - Whitespace is good for readability, add a blank line after const groups and before return statements - Separate third-party and local file imports +.github/workflows/deps.yml (4)
21-22: Pin checkout to a released major@v5 is not a released tag; use @v4 (or a SHA).
- uses: actions/checkout@v5 + uses: actions/checkout@v4(Apply in both jobs.)
Also applies to: 50-51
59-64: Avoid double-running postinstallSame rationale as CI.
- name: Install dependencies run: npm ci --prefer-offline --no-audit - - - name: Run postinstall - run: npm run postinstall
12-15: Add workflow concurrency to cancel superseded runsPrevents duplicate scheduled/manual runs piling up.
permissions: contents: read actions: read + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true
65-66: Optionally make dependency check non-blocking on scheduleSo weekly reports don’t fail the workflow.
- name: Check dependencies for issues - run: npm run checkDependencies + run: npm run checkDependencies + continue-on-error: ${{ github.event_name == 'schedule' }}src/notebooks/deepnote/OutputTypeDetector.ts (3)
21-46: Centralize MIME constantsExport shared MIME constants to avoid duplication across handlers.
Would you like me to extract a src/notebooks/deepnote/mimeConstants.ts and update imports?
15-19: Do not treat text/plain as a streamPlain text outputs are “rich”, not streams; this misroutes handlers.
- private readonly streamMimes = [ - 'text/plain', - 'application/vnd.code.notebook.stdout', - 'application/vnd.code.notebook.stderr' - ]; + private readonly streamMimes = [ + 'application/vnd.code.notebook.stdout', + 'application/vnd.code.notebook.stderr' + ];
1-1: Add required Microsoft copyright headerPer repo guidelines, prepend the standard header.
+/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. + * Licensed under the MIT License. + *--------------------------------------------------------------------------------------------*/ import { NotebookCellOutput } from 'vscode';src/notebooks/deepnote/dataConversionUtils.ts (4)
26-34: Fix Node/web incompatibility (atob/btoa undefined in Node)Provide cross-platform fallbacks using Buffer.
export function convertBase64ToUint8Array(base64Content: string): Uint8Array { const base64Data = base64Content.includes(',') ? base64Content.split(',')[1] : base64Content; - const binaryString = atob(base64Data); - const uint8Array = new Uint8Array(binaryString.length); - for (let i = 0; i < binaryString.length; i++) { - uint8Array[i] = binaryString.charCodeAt(i); - } - return uint8Array; + if (typeof globalThis.atob === 'function') { + const binaryString = atob(base64Data); + const uint8Array = new Uint8Array(binaryString.length); + for (let i = 0; i < binaryString.length; i++) { + uint8Array[i] = binaryString.charCodeAt(i); + } + return uint8Array; + } + // Node fallback + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + return new Uint8Array(Buffer.from(base64Data, 'base64')); } @@ export function convertUint8ArrayToBase64DataUrl(data: Uint8Array, mimeType: string): string { - const base64String = btoa(String.fromCharCode(...data)); - return `data:${mimeType};base64,${base64String}`; + if (typeof globalThis.btoa === 'function') { + // Avoid spreading large arrays; chunk if needed (omitted for brevity). + const base64String = btoa(String.fromCharCode(...data)); + return `data:${mimeType};base64,${base64String}`; + } + // Node fallback + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + const base64 = Buffer.from(data).toString('base64'); + return `data:${mimeType};base64,${base64}`; }Also applies to: 39-42
73-80: Use crypto.getRandomValues for IDs (with fallback)Stronger randomness; same output shape.
export function generateBlockId(): string { - const chars = '0123456789abcdef'; - let id = ''; - for (let i = 0; i < 32; i++) { - id += chars[Math.floor(Math.random() * chars.length)]; - } - return id; + const toHex = (b: number) => b.toString(16).padStart(2, '0'); + if (typeof globalThis.crypto?.getRandomValues === 'function') { + const buf = new Uint8Array(16); + globalThis.crypto.getRandomValues(buf); + return Array.from(buf, toHex).join(''); + } + const fallback = Array.from({ length: 16 }, () => Math.floor(Math.random() * 256)); + return fallback.map(toHex).join(''); }
5-11: Add concise JSDoc @param/@returns for exported utilitiesImproves tooltips and API docs.
/** * Safely decode content using TextDecoder */ -export function decodeContent(data: Uint8Array): string { +/** + * @param data Bytes to decode (UTF-8). + * @returns Decoded string. + */ +export function decodeContent(data: Uint8Array): string { return new TextDecoder().decode(data); }
1-4: Add required header and run PrettierBring file in line with repo standards.
+/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. + * Licensed under the MIT License. + *--------------------------------------------------------------------------------------------*/ /** * Utility functions for data transformation in Deepnote conversion */package.json (1)
320-325: Localize the command title; keep “Deepnote” category as preferred.Use an l10n token for the title and add it to package.nls.json.
{ "command": "jupyter.selectDeepnoteNotebook", - "title": "Select Notebook", + "title": "%deepnote.command.selectNotebook.title%", "category": "Deepnote", "enablement": "notebookType == 'deepnote'" },src/notebooks/deepnote/deepnoteNotebookSelector.unit.test.ts (2)
35-66: Make assertions locale-agnostic (use l10n.t).-import * as assert from 'assert'; +import * as assert from 'assert'; +import { l10n } from 'vscode'; @@ -assert.strictEqual(description, '1 cells (current)'); +assert.strictEqual(description, l10n.t('{0} cells (current)', 1)); @@ -assert.strictEqual(description, '2 cells'); +assert.strictEqual(description, l10n.t('{0} cells', 2)); @@ -assert.strictEqual(description, '0 cells'); +assert.strictEqual(description, l10n.t('{0} cells', 0)); @@ -assert.strictEqual(description, '2 cells'); +assert.strictEqual(description, l10n.t('{0} cells', 2)); @@ -assert.strictEqual(detail, 'ID: notebook-1 | Working Directory: /home/user'); +assert.strictEqual(detail, l10n.t('ID: {0} | Working Directory: {1}', 'notebook-1', '/home/user')); @@ -assert.strictEqual(detail, 'ID: notebook-2'); +assert.strictEqual(detail, l10n.t('ID: {0}', 'notebook-2')); @@ -assert.strictEqual(detail, 'ID: notebook-1'); +assert.strictEqual(detail, l10n.t('ID: {0}', 'notebook-1'));Also applies to: 69-101
35-66: Avoid testing privates via any-cast. Expose helpers or test through public API.Also applies to: 69-101, 102-117
src/notebooks/deepnote/outputHandlers/StreamOutputHandler.ts (3)
1-1: Add required header and format.+/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. + * Licensed under the MIT License. + *--------------------------------------------------------------------------------------------*/
9-13: Don’t treat text/plain as a stream.Including text/plain misclassifies rich text.
- private readonly streamMimes = [ - 'text/plain', - 'application/vnd.code.notebook.stdout', - 'application/vnd.code.notebook.stderr' - ]; + private readonly streamMimes = [ + 'application/vnd.code.notebook.stdout', + 'application/vnd.code.notebook.stderr' + ];
18-37: Early-exit when no stream items; default name to stdout.- const streamItems = output.items.filter((item) => this.streamMimes.includes(item.mime)); + const streamItems = output.items.filter((item) => this.streamMimes.includes(item.mime)); + if (streamItems.length === 0) { + return { output_type: 'stream', text: '' }; + } @@ - if (stderrItem) { - deepnoteOutput.name = 'stderr'; - } + deepnoteOutput.name = stderrItem ? 'stderr' : 'stdout';src/notebooks/deepnote/deepnoteDataConverter.unit.test.ts (2)
130-153: Cells->blocks happy-path covered. Add output reverse-path tests.+ test('converts stdout cell output back to stream DeepnoteOutput', () => { + const cells: NotebookCellData[] = [{ + kind: NotebookCellKind.Code, + value: 'print("x")', + languageId: 'python', + outputs: [new (require('../test/mocks/vsc/extHostedTypes').NotebookCellOutput)([ + (require('../test/mocks/vsc/extHostedTypes').NotebookCellOutputItem).stdout('x\n') + ])] + }]; + const blocks = converter.convertCellsToBlocks(cells); + assert.strictEqual(blocks[0].outputs?.[0].output_type, 'stream'); + assert.strictEqual(blocks[0].outputs?.[0].text, 'x\n'); + });
425-456: Add broader round-trip with rich outputs (not just streams).+ test('round-trip with rich multi-mime outputs', () => { + const blocks: DeepnoteBlock[] = [{ + id: 'b1', + type: 'code', + content: 'x', + sortingKey: 'a0', + outputs: [{ + output_type: 'execute_result', + execution_count: 7, + data: { + 'text/plain': '42', + 'text/html': '<div>42</div>' + }, + metadata: { note: 'x' } + }] + }]; + const cells = converter.convertBlocksToCells(blocks); + const rt = converter.convertCellsToBlocks(cells); + assert.deepStrictEqual(rt, blocks); + });src/notebooks/deepnote/outputHandlers/ErrorOutputHandler.ts (3)
1-1: Formatting only; header intentionally omitted (per project learning).Run Prettier. Per retrieved learnings for this repo, do not add a copyright header.
prettier --write src/notebooks/deepnote/outputHandlers/ErrorOutputHandler.ts
17-37: Map VS Code error fields and remove redundant try/catch.Support name/message/stack → ename/evalue/traceback, and drop the no-op try/catch around parseJsonSafely.
- try { - const errorData = parseJsonSafely(decodeContent(errorItem.data)); - if (typeof errorData === 'object' && errorData !== null) { - const errorObj = errorData as Record<string, unknown>; - deepnoteOutput.ename = (errorObj.ename as string) || 'Error'; - deepnoteOutput.evalue = (errorObj.evalue as string) || ''; - deepnoteOutput.traceback = (errorObj.traceback as string[]) || []; - } else { - // Fallback if error data is not valid JSON object - const errorText = String(errorData); - deepnoteOutput.ename = 'Error'; - deepnoteOutput.evalue = errorText; - deepnoteOutput.traceback = [errorText]; - } - } catch { - // Final fallback if parsing completely fails - const errorText = decodeContent(errorItem.data); - deepnoteOutput.ename = 'Error'; - deepnoteOutput.evalue = errorText; - deepnoteOutput.traceback = [errorText]; - } + const raw = decodeContent(errorItem.data); + const parsed = parseJsonSafely(raw); + if (parsed && typeof parsed === 'object') { + const o = parsed as Record<string, unknown>; + const ename = (o.ename as string) ?? (o.name as string) ?? 'Error'; + const evalue = (o.evalue as string) ?? (o.message as string) ?? ''; + let traceback: string[] = []; + if (Array.isArray(o.traceback)) { + traceback = o.traceback as string[]; + } else if (typeof o.stack === 'string') { + traceback = (o.stack as string).split('\n'); + } + deepnoteOutput.ename = ename; + deepnoteOutput.evalue = evalue; + deepnoteOutput.traceback = traceback; + if (typeof o.name === 'string') { + deepnoteOutput.name = o.name as string; + } + if (typeof o.stack === 'string') { + deepnoteOutput.stack = o.stack as string; + } + if ('error' in o) { + deepnoteOutput.error = o.error; + } + } else { + const errorText = String(parsed); + deepnoteOutput.ename = 'Error'; + deepnoteOutput.evalue = errorText; + deepnoteOutput.traceback = [errorText]; + }
83-86: Scrub/limit potentially sensitive details in UI.Truncate large payloads before showing in-cell messages.
- if (output.error) { - errorDetails.push(l10n.t('Error Details: {0}', JSON.stringify(output.error))); - } + if (output.error) { + const details = JSON.stringify(output.error); + const safe = details.length > 2000 ? `${details.slice(0, 2000)}…` : details; + errorDetails.push(l10n.t('Error Details: {0}', safe)); + }Also applies to: 101-104
src/notebooks/deepnote/deepnoteNotebookSelector.ts (4)
1-1: No header needed; keep formatting clean.Per retrieved learning, omit copyright headers in new Deepnote files. Ensure Prettier passes.
1-1: Add CancellationToken support to cancel the picker.Wire an optional token in options and hide on cancellation.
-import { l10n, type QuickPickItem, window } from 'vscode'; +import { l10n, type QuickPickItem, window, type CancellationToken } from 'vscode'; @@ options?: { title?: string; placeHolder?: string; + token?: CancellationToken; } @@ - let accepted = false; - quickPick.show(); + let accepted = false; + const cancelSub = options?.token?.onCancellationRequested(() => quickPick.hide()); + quickPick.show(); @@ const selectedItem = accepted ? quickPick.selectedItems[0] : undefined; quickPick.dispose(); + cancelSub?.dispose();Also applies to: 25-29, 52-58, 60-62
38-42: Better QuickPick search UX.Enable searching description/detail.
const quickPick = window.createQuickPick<NotebookQuickPickItem>(); quickPick.items = items; + quickPick.matchOnDescription = true; + quickPick.matchOnDetail = true; quickPick.placeholder = options?.placeHolder || l10n.t('Select a notebook to open');
66-74: Pluralize “cell(s)”.Minor UX polish.
- const cellCount = notebook.blocks.length; - - if (notebook.id === currentNotebookId) { - return l10n.t('{0} cells (current)', cellCount); - } - - return l10n.t('{0} cells', cellCount); + const cellCount = notebook.blocks.length; + const base = cellCount === 1 ? l10n.t('{0} cell', cellCount) : l10n.t('{0} cells', cellCount); + return notebook.id === currentNotebookId ? l10n.t('{0} (current)', base) : base;src/notebooks/deepnote/outputHandlers/RichOutputHandler.ts (4)
1-1: Run Prettier; omit header.Format the file. New Deepnote files should not carry a header (per learning).
prettier --write src/notebooks/deepnote/outputHandlers/RichOutputHandler.ts
7-16: JSDoc: add param/return details.Expand class/method docs as requested.
-/** - * Handles rich/display data outputs conversion between Deepnote and VS Code formats - */ +/** + * Handles rich/display-data outputs conversion between Deepnote and VS Code formats. + * @public + */ @@ - /** - * Convert VS Code rich output to Deepnote format - */ + /** + * Convert a VS Code NotebookCellOutput (rich) into a DeepnoteOutput. + * Skips stream/error items; processes remaining MIME items via the MIME registry. + * @param output VS Code NotebookCellOutput to convert. + * @returns DeepnoteOutput with output_type 'display_data' and populated data map. + */
18-21: Use display_data here; execute_result should be set upstream when execution_count is known.Prevents misleading metadata.
- const deepnoteOutput: DeepnoteOutput = { - output_type: 'execute_result', - data: {} - }; + const deepnoteOutput: DeepnoteOutput = { + output_type: 'display_data', + data: {} + }; @@ - if (hasDisplayData) { - // Use display_data for rich outputs without execution count, execute_result for those with - deepnoteOutput.output_type = deepnoteOutput.execution_count ? 'execute_result' : 'display_data'; - } + // Keep as 'display_data'; 'execute_result' requires an execution_count set upstream.Also applies to: 46-49
29-41: Don’t decode binaries as text; preserve bytes and use MIME-aware fallback.Fixes image/binary corruption and removes console usage.
- const decodedContent = decodeContent(item.data); - deepnoteOutput.data![item.mime] = this.mimeRegistry.processForDeepnote(decodedContent, item.mime); + const processed = this.mimeRegistry.processForDeepnote(item.data, item.mime); + deepnoteOutput.data![item.mime] = processed; hasDisplayData = true; } catch (error) { - // Fallback: treat as text if any processing fails - try { - const decodedContent = decodeContent(item.data); - deepnoteOutput.data![item.mime] = decodedContent; - hasDisplayData = true; - } catch { - // Skip this item if even text decoding fails - console.warn(`Failed to process output item with mime type: ${item.mime}`, error); - } + // Fallbacks + if (item.mime.startsWith('text/')) { + deepnoteOutput.data![item.mime] = decodeContent(item.data); + hasDisplayData = true; + } else { + // Preserve raw bytes for non-text types + deepnoteOutput.data![item.mime] = item.data; + hasDisplayData = true; + } }src/notebooks/deepnote/deepnoteNotebookManager.ts (1)
40-52: Mirror the “key” semantics in setter JSDoc and param.Prevents confusion with callers using projectId.
- /** - * Associates a notebook ID with a file URI to remember user's notebook selection. + /** + * Associates a notebook ID with a key to remember user's notebook selection. + * Recommended key: projectId (to align with serializer and be rename-proof). * When a Deepnote file contains multiple notebooks, this mapping persists the user's * choice so we can automatically open the same notebook on subsequent file opens. - * Also marks the URI to skip the selection prompt on the next immediate open. + * Also marks the key to skip the selection prompt on the next immediate open. * - * @param uri - The file URI (or project ID) that identifies the Deepnote file + * @param key - Identifier key (file URI or projectId) * @param notebookId - The ID of the selected notebook within the file */ - setSelectedNotebookForUri(uri: string, notebookId: string): void { - this.selectedNotebookByUri.set(uri, notebookId); - this.skipPromptForUri.add(uri); + setSelectedNotebookForUri(key: string, notebookId: string): void { + this.selectedNotebookByUri.set(key, notebookId); + this.skipPromptForUri.add(key); }src/notebooks/deepnote/deepnoteTypes.ts (1)
50-62: Narrow output_type to a discriminated union.Improves type-safety and downstream exhaustiveness.
- output_type: string; + output_type: 'stream' | 'display_data' | 'execute_result' | 'error';src/notebooks/deepnote/deepnoteDataConverter.ts (3)
75-79: executionCount 0 bug (falsy check).Use nullish coalescing so 0 survives.
- const executionCount = cell.metadata?.executionCount || cell.executionSummary?.executionOrder; + const executionCount = cell.metadata?.executionCount ?? cell.executionSummary?.executionOrder;
27-30: Don’t mutate inputs; sort numerically-aware.Avoids side effects and lexicographic pitfalls.
- return blocks - .sort((a, b) => a.sortingKey.localeCompare(b.sortingKey)) + return [...blocks] + .sort((a, b) => (a.sortingKey ?? '').localeCompare(b.sortingKey ?? '', undefined, { numeric: true })) .map((block) => this.convertBlockToCell(block));
15-18: Prefer DI for handlers/detector (testability).Inject with defaults.
-export class DeepnoteDataConverter { - private readonly outputDetector = new OutputTypeDetector(); - private readonly streamHandler = new StreamOutputHandler(); - private readonly errorHandler = new ErrorOutputHandler(); - private readonly richHandler = new RichOutputHandler(); +export class DeepnoteDataConverter { + constructor( + private readonly outputDetector: OutputTypeDetector = new OutputTypeDetector(), + private readonly streamHandler: StreamOutputHandler = new StreamOutputHandler(), + private readonly errorHandler: ErrorOutputHandler = new ErrorOutputHandler(), + private readonly richHandler: RichOutputHandler = new RichOutputHandler() + ) {}src/notebooks/deepnote/deepnoteSerializer.ts (6)
62-66: Web-compat decode: use TextDecoder, not Buffer.Buffer breaks in web extensions.
- const contentString = Buffer.from(content).toString('utf8'); + const contentString = new TextDecoder('utf-8').decode(content);
67-70: Typed, localized error for missing notebooks.Replace generic Error with typed + l10n.
- if (!deepnoteProject.project?.notebooks) { - throw new Error('Invalid Deepnote file: no notebooks found'); - } + if (!Array.isArray(deepnoteProject.project?.notebooks)) { + throw new FileFormatError(l10n.t('Invalid Deepnote file: no notebooks found')); + }
95-100: Use ILogger; wrap cause with a typed error.No console.*; preserve the original error.
- } catch (error) { - console.error('Error deserializing Deepnote notebook:', error); - - throw new Error( - `Failed to parse Deepnote file: ${error instanceof Error ? error.message : 'Unknown error'}` - ); + } catch (error) { + this.logger?.error?.('Deepnote: deserialize failed', { error }); + throw new DeserializationError(l10n.t('Failed to parse Deepnote file'), { cause: error }); }
110-132: Typed, localized errors in serialize path prerequisites.Improves UX and diagnostics.
- const projectId = data.metadata?.deepnoteProjectId; - if (!projectId) { - throw new Error('Missing Deepnote project ID in notebook metadata'); - } + const projectId = data.metadata?.deepnoteProjectId; + if (!projectId) { + throw new FileFormatError(l10n.t('Missing Deepnote project ID in notebook metadata')); + } @@ - const originalProject = this.manager.getOriginalProject(projectId); - if (!originalProject) { - throw new Error('Original Deepnote project not found. Cannot save changes.'); - } + const originalProject = this.manager.getOriginalProject(projectId); + if (!originalProject) { + throw new NotFoundError(l10n.t('Original Deepnote project not found. Cannot save changes.')); + } @@ - const notebookId = data.metadata?.deepnoteNotebookId || this.manager.getCurrentNotebookId(projectId); - if (!notebookId) { - throw new Error('Cannot determine which notebook to save'); - } + const notebookId = data.metadata?.deepnoteNotebookId || this.manager.getCurrentNotebookId(projectId); + if (!notebookId) { + throw new FileFormatError(l10n.t('Cannot determine which notebook to save')); + } @@ - if (notebookIndex === -1) { - throw new Error(`Notebook with ID ${notebookId} not found in project`); - } + if (notebookIndex === -1) { + throw new NotFoundError(l10n.t('Notebook with ID {0} not found in project', notebookId)); + }
157-163: Use ILogger and typed error on serialize failure.Mirror deserialize handling.
- } catch (error) { - console.error('Error serializing Deepnote notebook:', error); - throw new Error( - `Failed to save Deepnote file: ${error instanceof Error ? error.message : 'Unknown error'}` - ); + } catch (error) { + this.logger?.error?.('Deepnote: serialize failed', { error }); + throw new SerializationError(l10n.t('Failed to save Deepnote file'), { cause: error }); }
25-45: Inject dependencies instead of hard-coding singletons.Constructor-inject manager/selector/converter (with defaults) to follow “inject interfaces”.
-export class DeepnoteNotebookSerializer implements NotebookSerializer { - private manager = new DeepnoteNotebookManager(); - private selector = new DeepnoteNotebookSelector(); - private converter = new DeepnoteDataConverter(); +export class DeepnoteNotebookSerializer implements NotebookSerializer { + constructor( + private readonly manager: DeepnoteNotebookManager = new DeepnoteNotebookManager(), + private readonly selector: DeepnoteNotebookSelector = new DeepnoteNotebookSelector(), + private readonly converter: DeepnoteDataConverter = new DeepnoteDataConverter(), + private readonly logger?: ILogger + ) {}src/notebooks/deepnote/deepnoteActivationService.ts (6)
49-53: Web-compat decode: use TextDecoder.Avoid Buffer in web extensions.
- const contentString = Buffer.from(rawContent).toString('utf8'); + const contentString = new TextDecoder('utf-8').decode(rawContent);
34-36: Simplify API: drop param; use this.selector.Avoid passing a field back into a method.
- commands.registerCommand(Commands.SelectDeepnoteNotebook, () => this.selectNotebook(this.selector)) + commands.registerCommand(Commands.SelectDeepnoteNotebook, () => this.selectNotebook())- private async selectNotebook(selector: DeepnoteNotebookSelector) { + private async selectNotebook() {- const selectedNotebook = await selector.selectNotebook( + const selectedNotebook = await this.selector.selectNotebook(Also applies to: 66-67, 93-101
126-138: Selection-key mismatch: store under projectId (and URI) to match serializer.Prevents missed preselection/skip flags.
- manager.setSelectedNotebookForUri(fileUri, selectedNotebook.id); + // Store under both keys for robustness (projectId is canonical). + manager.setSelectedNotebookForUri(projectId, selectedNotebook.id); + manager.setSelectedNotebookForUri(fileUri, selectedNotebook.id);
145-151: Log with ILogger; keep user message minimal.Add structured log before toast.
- } catch (error) { - await window.showErrorMessage( + } catch (error) { + this.logger?.error?.('Deepnote: switch notebook failed', { error }); + await window.showErrorMessage( l10n.t( 'Error switching notebook: {0}', error instanceof Error ? error.message : l10n.t('Unknown error') ) );
85-89: Reduce UI noise for single-notebook case.Skip the info toast; just return.
- await window.showInformationMessage(l10n.t('This Deepnote file contains only one notebook.')); - return;
25-37: Prefer DI over new (serializer/selector).Constructor-inject to improve testability and follow guidelines.
- public activate() { - this.serializer = new DeepnoteNotebookSerializer(); - this.selector = new DeepnoteNotebookSelector(); + public activate() { + this.serializer = this.serializer ?? new DeepnoteNotebookSerializer(); + this.selector = this.selector ?? new DeepnoteNotebookSelector();And update constructor to accept optional instances:
- constructor(@inject(IExtensionContext) private extensionContext: IExtensionContext) {} + constructor( + @inject(IExtensionContext) private extensionContext: IExtensionContext, + serializer?: DeepnoteNotebookSerializer, + selector?: DeepnoteNotebookSelector + ) { + this.serializer = serializer!; + this.selector = selector!; + }src/notebooks/deepnote/MimeTypeProcessor.ts (10)
20-22: Fix interface mismatch in TextMimeProcessor.processForDeepnote.Method must accept mimeType to satisfy MimeProcessor.
- processForDeepnote(content: unknown): unknown { + processForDeepnote(content: unknown, _mimeType: string): unknown { return typeof content === 'string' ? content : String(content); }
43-49: Avoid btoa/String.fromCharCode spread; crashes on large arrays and in Node.Use the shared util.
- if (content instanceof Uint8Array) { - const base64String = btoa(String.fromCharCode(...content)); - return `data:${mimeType};base64,${base64String}`; - } + if (content instanceof Uint8Array) { + const base64String = convertUint8ArrayToBase64(content); + return `data:${mimeType};base64,${base64String}`; + }
51-69: Use bytes() factory and plain‑text fallback.Ensures correct VS Code API usage and safe fallback.
- return new NotebookCellOutputItem(uint8Array, mimeType); + return NotebookCellOutputItem.bytes(uint8Array, mimeType); } catch { - return NotebookCellOutputItem.text(String(content), mimeType); + return NotebookCellOutputItem.text(String(content)); }
80-86: Fix interface mismatch in JsonMimeProcessor.processForDeepnote.- processForDeepnote(content: unknown): unknown { + processForDeepnote(content: unknown, _mimeType: string): unknown { if (typeof content === 'string') { return parseJsonSafely(content); } return content; }
114-119: Fix interface mismatch in ApplicationMimeProcessor.processForDeepnote.- processForDeepnote(content: unknown): unknown { + processForDeepnote(content: unknown, _mimeType: string): unknown { if (typeof content === 'string') { return parseJsonSafely(content); } return content; }
121-124: Emit text/plain for application/ textual fallback.*Avoid misleading MIME on text output; param can be unused.
- processForVSCode(content: unknown, mimeType: string): NotebookCellOutputItem | null { + processForVSCode(content: unknown, _mimeType: string): NotebookCellOutputItem | null { const textContent = typeof content === 'string' ? content : JSON.stringify(content, null, 2); - return NotebookCellOutputItem.text(textContent, mimeType); + return NotebookCellOutputItem.text(textContent); }
131-133: Fix Generic.canHandle signature to match interface.- canHandle(): boolean { + canHandle(_mimeType: string): boolean { return true; // Always can handle as fallback }
135-137: Fix Generic.processForDeepnote signature.- processForDeepnote(content: unknown): unknown { + processForDeepnote(content: unknown, _mimeType: string): unknown { return content; }
139-141: Generic fallback should emit text/plain.- processForVSCode(content: unknown, mimeType: string): NotebookCellOutputItem | null { - return NotebookCellOutputItem.text(String(content), mimeType); + processForVSCode(content: unknown, _mimeType: string): NotebookCellOutputItem | null { + return NotebookCellOutputItem.text(String(content)); }
147-155: Constructor injection for processors.Enables testing/override; follows “inject interfaces, not concretes”.
-export class MimeTypeProcessorRegistry { - private readonly processors: MimeProcessor[] = [ - new TextMimeProcessor(), - new ImageMimeProcessor(), - new JsonMimeProcessor(), - new ApplicationMimeProcessor(), - new GenericMimeProcessor() // Must be last as fallback - ]; -} +export class MimeTypeProcessorRegistry { + private readonly processors: MimeProcessor[]; + constructor(processors?: MimeProcessor[]) { + this.processors = + processors ?? + [ + new TextMimeProcessor(), + new ImageMimeProcessor(), + new JsonMimeProcessor(), + new ApplicationMimeProcessor(), + new GenericMimeProcessor() // Must be last as fallback + ]; + } +}
Comment on lines
+85
to
+91
| export function generateSortingKey(index: number): string { | ||
| const alphabet = 'abcdefghijklmnopqrstuvwxyz'; | ||
| const letterIndex = Math.floor(index / 100); | ||
| const letter = letterIndex < alphabet.length ? alphabet[letterIndex] : 'z'; | ||
| const number = index % 100; | ||
| return `${letter}${number}`; | ||
| } |
Contributor
There was a problem hiding this comment.
Pad numeric part to keep lexicographic sort stable
'a10' currently sorts before 'a2'. Zero-pad the number.
export function generateSortingKey(index: number): string {
const alphabet = 'abcdefghijklmnopqrstuvwxyz';
const letterIndex = Math.floor(index / 100);
const letter = letterIndex < alphabet.length ? alphabet[letterIndex] : 'z';
const number = index % 100;
- return `${letter}${number}`;
+ return `${letter}${number.toString().padStart(2, '0')}`;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function generateSortingKey(index: number): string { | |
| const alphabet = 'abcdefghijklmnopqrstuvwxyz'; | |
| const letterIndex = Math.floor(index / 100); | |
| const letter = letterIndex < alphabet.length ? alphabet[letterIndex] : 'z'; | |
| const number = index % 100; | |
| return `${letter}${number}`; | |
| } | |
| export function generateSortingKey(index: number): string { | |
| const alphabet = 'abcdefghijklmnopqrstuvwxyz'; | |
| const letterIndex = Math.floor(index / 100); | |
| const letter = letterIndex < alphabet.length ? alphabet[letterIndex] : 'z'; | |
| const number = index % 100; | |
| return `${letter}${number.toString().padStart(2, '0')}`; | |
| } |
🤖 Prompt for AI Agents
In src/notebooks/deepnote/dataConversionUtils.ts around lines 85 to 91, the
generated sorting key uses an unpadded numeric suffix so values like "a10" sort
before "a2"; change the numeric part to a fixed two-digit zero-padded string
(since number = index % 100) and return `${letter}${paddedNumber}` (e.g., use
number.toString().padStart(2, '0')) while keeping the existing letter selection
logic.
Comment on lines
+39
to
+64
| private async getDeepnoteProject(notebookUri: Uri, projectId?: string): Promise<DeepnoteProject | undefined> { | ||
| // Try cache first if we have a project ID | ||
| if (projectId) { | ||
| const cachedProject = this.serializer.getManager().getOriginalProject(projectId); | ||
| if (cachedProject) { | ||
| return cachedProject; | ||
| } | ||
| } | ||
|
|
||
| // Cache miss or no project ID - read and parse file | ||
| const rawContent = await workspace.fs.readFile(notebookUri); | ||
| const contentString = Buffer.from(rawContent).toString('utf8'); | ||
| const yaml = await import('js-yaml'); | ||
| const deepnoteProject = yaml.load(contentString) as DeepnoteProject; | ||
|
|
||
| // Store in cache if we have a project ID | ||
| if (projectId && deepnoteProject) { | ||
| const manager = this.serializer.getManager(); | ||
| const currentNotebookId = manager.getCurrentNotebookId(projectId); | ||
| if (currentNotebookId) { | ||
| manager.storeOriginalProject(projectId, deepnoteProject, currentNotebookId); | ||
| } | ||
| } | ||
|
|
||
| return deepnoteProject; | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use cache even without currentNotebookId.
Consider caching parsed project regardless; otherwise repeated disk reads on first selection.
Minimal change: remove the currentNotebookId guard and always store.
- if (projectId && deepnoteProject) {
- const manager = this.serializer.getManager();
- const currentNotebookId = manager.getCurrentNotebookId(projectId);
- if (currentNotebookId) {
- manager.storeOriginalProject(projectId, deepnoteProject, currentNotebookId);
- }
- }
+ if (projectId && deepnoteProject) {
+ const manager = this.serializer.getManager();
+ manager.storeOriginalProject(projectId, deepnoteProject, manager.getCurrentNotebookId(projectId) ?? deepnoteProject.project.notebooks[0]?.id ?? '');
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private async getDeepnoteProject(notebookUri: Uri, projectId?: string): Promise<DeepnoteProject | undefined> { | |
| // Try cache first if we have a project ID | |
| if (projectId) { | |
| const cachedProject = this.serializer.getManager().getOriginalProject(projectId); | |
| if (cachedProject) { | |
| return cachedProject; | |
| } | |
| } | |
| // Cache miss or no project ID - read and parse file | |
| const rawContent = await workspace.fs.readFile(notebookUri); | |
| const contentString = Buffer.from(rawContent).toString('utf8'); | |
| const yaml = await import('js-yaml'); | |
| const deepnoteProject = yaml.load(contentString) as DeepnoteProject; | |
| // Store in cache if we have a project ID | |
| if (projectId && deepnoteProject) { | |
| const manager = this.serializer.getManager(); | |
| const currentNotebookId = manager.getCurrentNotebookId(projectId); | |
| if (currentNotebookId) { | |
| manager.storeOriginalProject(projectId, deepnoteProject, currentNotebookId); | |
| } | |
| } | |
| return deepnoteProject; | |
| } | |
| private async getDeepnoteProject(notebookUri: Uri, projectId?: string): Promise<DeepnoteProject | undefined> { | |
| // Try cache first if we have a project ID | |
| if (projectId) { | |
| const cachedProject = this.serializer.getManager().getOriginalProject(projectId); | |
| if (cachedProject) { | |
| return cachedProject; | |
| } | |
| } | |
| // Cache miss or no project ID - read and parse file | |
| const rawContent = await workspace.fs.readFile(notebookUri); | |
| const contentString = Buffer.from(rawContent).toString('utf8'); | |
| const yaml = await import('js-yaml'); | |
| const deepnoteProject = yaml.load(contentString) as DeepnoteProject; | |
| // Store in cache if we have a project ID | |
| if (projectId && deepnoteProject) { | |
| const manager = this.serializer.getManager(); | |
| manager.storeOriginalProject( | |
| projectId, | |
| deepnoteProject, | |
| manager.getCurrentNotebookId(projectId) ?? deepnoteProject.project.notebooks[0]?.id ?? '' | |
| ); | |
| } | |
| return deepnoteProject; | |
| } |
🤖 Prompt for AI Agents
In src/notebooks/deepnote/deepnoteActivationService.ts around lines 39 to 64,
the parsed Deepnote project is only cached when currentNotebookId is truthy
causing repeated disk reads; remove the guard so that whenever projectId and
deepnoteProject exist you always call manager.storeOriginalProject(projectId,
deepnoteProject, currentNotebookId) (passing the possibly undefined
currentNotebookId) to persist the parsed project in the cache after reading the
file.
| @@ -0,0 +1,458 @@ | |||
| import { assert } from 'chai'; | |||
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add standard copyright header.
+/*---------------------------------------------------------------------------------------------
+ * Copyright (c) Microsoft Corporation.
+ * Licensed under the MIT License.
+ *--------------------------------------------------------------------------------------------*/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import { assert } from 'chai'; | |
| /*--------------------------------------------------------------------------------------------- | |
| * Copyright (c) Microsoft Corporation. | |
| * Licensed under the MIT License. | |
| *--------------------------------------------------------------------------------------------*/ | |
| import { assert } from 'chai'; |
🤖 Prompt for AI Agents
In src/notebooks/deepnote/deepnoteDataConverter.unit.test.ts around line 1, the
file is missing the project's standard copyright header; add the exact header
used across the repo (including copyright owner, year range, and license notice)
at the very top of the file before any imports so it matches other files’
formatting and spacing conventions.
Comment on lines
+1
to
+3
| import { NotebookCellOutputItem } from 'vscode'; | ||
| import { parseJsonSafely, convertBase64ToUint8Array } from './dataConversionUtils'; | ||
|
|
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Import cross‑platform base64 encoder util.
You’ll call it below; import it now.
-import { parseJsonSafely, convertBase64ToUint8Array } from './dataConversionUtils';
+import { parseJsonSafely, convertBase64ToUint8Array, convertUint8ArrayToBase64 } from './dataConversionUtils';Add this to dataConversionUtils.ts (Node/Web safe):
export function convertUint8ArrayToBase64(data: Uint8Array): string {
// Node
if (typeof Buffer !== 'undefined' && typeof Buffer.from === 'function') {
return Buffer.from(data).toString('base64');
}
// Web
let binary = '';
const chunk = 0x8000;
for (let i = 0; i < data.length; i += chunk) {
binary += String.fromCharCode.apply(null, Array.from(data.subarray(i, i + chunk)) as any);
}
// eslint-disable-next-line no-undef
return btoa(binary);
}Also make convertBase64ToUint8Array Node/Web-safe:
export function convertBase64ToUint8Array(base64Content: string): Uint8Array {
const base64Data = base64Content.includes(',') ? base64Content.split(',')[1] : base64Content;
// Node
if (typeof Buffer !== 'undefined' && typeof Buffer.from === 'function') {
return new Uint8Array(Buffer.from(base64Data, 'base64'));
}
// Web
// eslint-disable-next-line no-undef
const binaryString = atob(base64Data);
const uint8Array = new Uint8Array(binaryString.length);
for (let i = 0; i < binaryString.length; i++) {
uint8Array[i] = binaryString.charCodeAt(i);
}
return uint8Array;
}🤖 Prompt for AI Agents
In src/notebooks/deepnote/MimeTypeProcessor.ts lines 1 to 3, add an import for
the cross-platform encoder convertUint8ArrayToBase64 from ./dataConversionUtils
(alongside parseJsonSafely and convertBase64ToUint8Array), and update
./dataConversionUtils.ts to export a Node/Web-safe convertUint8ArrayToBase64
implementation and make convertBase64ToUint8Array Node/Web-safe as described:
implement Buffer-based branches when Buffer.from exists and fallback to browser
btoa/atob paths with safe chunking for large arrays and proper base64 payload
slicing when a data: URL is supplied.
Comment on lines
+87
to
+103
| processForVSCode(content: unknown, mimeType: string): NotebookCellOutputItem | null { | ||
| try { | ||
| let jsonObject: unknown; | ||
|
|
||
| if (typeof content === 'string') { | ||
| jsonObject = JSON.parse(content); | ||
| } else if (typeof content === 'object' && content !== null) { | ||
| jsonObject = content; | ||
| } else { | ||
| return NotebookCellOutputItem.text(String(content), mimeType); | ||
| } | ||
|
|
||
| return NotebookCellOutputItem.text(JSON.stringify(jsonObject, null, 2), mimeType); | ||
| } catch { | ||
| return NotebookCellOutputItem.text(String(content), mimeType); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prefer NotebookCellOutputItem.json and plain‑text fallback.
Leverage VS Code’s json factory; don’t stamp application MIME on text.
- } else {
- return NotebookCellOutputItem.text(String(content), mimeType);
- }
-
- return NotebookCellOutputItem.text(JSON.stringify(jsonObject, null, 2), mimeType);
+ } else {
+ return NotebookCellOutputItem.text(String(content));
+ }
+
+ return NotebookCellOutputItem.json(jsonObject);
} catch {
- return NotebookCellOutputItem.text(String(content), mimeType);
+ return NotebookCellOutputItem.text(String(content));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| processForVSCode(content: unknown, mimeType: string): NotebookCellOutputItem | null { | |
| try { | |
| let jsonObject: unknown; | |
| if (typeof content === 'string') { | |
| jsonObject = JSON.parse(content); | |
| } else if (typeof content === 'object' && content !== null) { | |
| jsonObject = content; | |
| } else { | |
| return NotebookCellOutputItem.text(String(content), mimeType); | |
| } | |
| return NotebookCellOutputItem.text(JSON.stringify(jsonObject, null, 2), mimeType); | |
| } catch { | |
| return NotebookCellOutputItem.text(String(content), mimeType); | |
| } | |
| } | |
| processForVSCode(content: unknown, mimeType: string): NotebookCellOutputItem | null { | |
| try { | |
| let jsonObject: unknown; | |
| if (typeof content === 'string') { | |
| jsonObject = JSON.parse(content); | |
| } else if (typeof content === 'object' && content !== null) { | |
| jsonObject = content; | |
| } else { | |
| return NotebookCellOutputItem.text(String(content)); | |
| } | |
| return NotebookCellOutputItem.json(jsonObject); | |
| } catch { | |
| return NotebookCellOutputItem.text(String(content)); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/notebooks/deepnote/MimeTypeProcessor.ts around lines 87 to 103, the code
currently always returns NotebookCellOutputItem.text with the incoming mimeType
even when the content is valid JSON; change it to use
NotebookCellOutputItem.json when you successfully parse or receive an object (so
VS Code treats it as structured JSON), and on non-JSON or parse errors return a
plain-text fallback using NotebookCellOutputItem.text(String(content),
'text/plain'); ensure you still handle string, object, and null/undefined cases
and wrap parsing in try/catch as now but switch the successful branch to
NotebookCellOutputItem.json(jsonObject) and the catch/fallback to text/plain.
Comment on lines
+156
to
+158
| getProcessor(mimeType: string): MimeProcessor { | ||
| return this.processors.find((processor) => processor.canHandle(mimeType)) || new GenericMimeProcessor(); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Reuse fallback instance instead of constructing anew.
- return this.processors.find((processor) => processor.canHandle(mimeType)) || new GenericMimeProcessor();
+ return this.processors.find((p) => p.canHandle(mimeType)) ?? this.processors[this.processors.length - 1];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| getProcessor(mimeType: string): MimeProcessor { | |
| return this.processors.find((processor) => processor.canHandle(mimeType)) || new GenericMimeProcessor(); | |
| } | |
| getProcessor(mimeType: string): MimeProcessor { | |
| return this.processors.find((p) => p.canHandle(mimeType)) ?? this.processors[this.processors.length - 1]; | |
| } |
🤖 Prompt for AI Agents
In src/notebooks/deepnote/MimeTypeProcessor.ts around lines 156 to 158, the
method always constructs a new GenericMimeProcessor as the fallback; instead
create and reuse a single GenericMimeProcessor instance (e.g., a private
readonly genericProcessor = new GenericMimeProcessor() on the class) and change
getProcessor to return this.genericProcessor when no match is found, avoiding
repeated allocations and preserving any state the fallback might hold.
Comment on lines
35
to
42
| // Check for stream outputs | ||
| const streamItems = output.items.filter((item) => this.streamMimes.includes(item.mime)); | ||
| if (streamItems.length > 0) { | ||
| return { | ||
| type: 'stream', | ||
| streamMimes: streamItems.map((item) => item.mime) | ||
| }; | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Minor: use a Set for O(1) membership checks
Small speed/readability win; adjust filter and helper accordingly.
- private readonly streamMimes = [
+ private readonly streamMimes = new Set<string>([
'application/vnd.code.notebook.stdout',
'application/vnd.code.notebook.stderr'
- ];
+ ]);
@@
- const streamItems = output.items.filter((item) => this.streamMimes.includes(item.mime));
+ const streamItems = output.items.filter((item) => this.streamMimes.has(item.mime));
@@
- return this.streamMimes.includes(mimeType);
+ return this.streamMimes.has(mimeType);Also applies to: 48-50
This was referenced Oct 1, 2025
This was referenced Oct 16, 2025
This was referenced Oct 21, 2025
This was referenced Oct 28, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
.deepnotefiles.Summary by CodeRabbit
New Features
Documentation
Tests
Chores