-
Notifications
You must be signed in to change notification settings - Fork 4
feat: Export projects and notebooks. #234
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughThis pull request adds export functionality to the Deepnote VSCode extension. Two new commands ( Sequence Diagram(s)mermaid Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #234 +/- ##
===========================
===========================
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
build/mocha-esm-loader.js (1)
72-79: Mock for@deepnote/convertis reasonable but must track real APIThe loader intercept and
deepnote-convertmock look coherent and match how tests useconvertDeepnoteToJupyterNotebooks(filename + nbformat 4 JSON with Deepnote metadata). Two minor points:
- This reimplements the converter’s behavior; if
@deepnote/convertchanges its output shape, tests may still pass while runtime code breaks. Consider adding a brief comment noting that this mock must stay aligned with the library’s contract, or deriving its types from the real package to reduce drift.- The mock attaches
outputseven to markdown cells. That’s harmless for current tests, but if you later add stricter nbformat validation, you may want to mirror the real converter’s behavior more closely.Also applies to: 310-344
package.json (1)
1583-1615: Explorer context menu grouping for project/notebook actions looks consistentRe‑grouping:
addNotebookToProjectunder1_add@1,renameProjectunder2_manage@1withexportProjectat2_manage@2,- notebook
rename/duplicateunder1_edit@1/2withexportNotebookunder2_export@1,creates a clear “add / manage / export / delete” ordering in the Deepnote explorer. This wiring matches the new export commands and should produce a sensible context menu.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
build/mocha-esm-loader.js(2 hunks)package.json(4 hunks)package.nls.json(1 hunks)src/notebooks/deepnote/deepnoteExplorerView.ts(3 hunks)src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts(1 hunks)src/platform/common/constants.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Usel10n.t()for all user-facing strings in TypeScript files
Use typed error classes fromsrc/platform/errors/instead of generic errors
UseILoggerservice instead of console.log for logging
Preserve error details in error messages while scrubbing personally identifiable information (PII)
Prefer async/await over promise chains
Handle cancellation with CancellationTokenOrder method, fields and properties first by accessibility (public/private/protected) and then by alphabetical order
Files:
src/platform/common/constants.tssrc/notebooks/deepnote/deepnoteExplorerView.unit.test.tssrc/notebooks/deepnote/deepnoteExplorerView.ts
src/platform/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/platform.instructions.md)
src/platform/**/*.ts: Use Inversify-based dependency injection with IServiceManager and IServiceContainer for managing service relationships and dependencies
Define service interfaces using Symbol-based exports (e.g.,export const IFileSystem = Symbol('IFileSystem')) paired with corresponding TypeScript interfaces
Register types using the patternserviceManager.addSingleton<IService>(IService, ServiceImplementation)in registerTypes functions
Use @Injectable() and @Inject() decorators from Inversify for class dependency injection configuration
Implement unified interfaces for cross-platform services (IFileSystem, IPlatformService, etc.) with graceful degradation for web environment limitations
Use structured logging with context via logger.info, logger.error, etc. rather than console.log
Provide platform capability checks and feature detection before using advanced platform-specific features
Register separate service implementations for desktop and web environments, allowing the DI container to resolve the correct platform implementation
Files:
src/platform/common/constants.ts
**/*.ts
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.ts: ALWAYS check the 'Core - Build' task output for TypeScript compilation errors before running any script or declaring work complete
ALWAYS runnpm run format-fixbefore committing changes to ensure proper code formatting
FIX all TypeScript compilation errors before moving forward with development
Usenpm run format-fixto auto-fix TypeScript formatting issues before committing
Usenpm run lintto check for linter issues in TypeScript files and attempt to fix before committing
Files:
src/platform/common/constants.tssrc/notebooks/deepnote/deepnoteExplorerView.unit.test.tssrc/notebooks/deepnote/deepnoteExplorerView.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{ts,tsx}: Don't add the Microsoft copyright header to new files
UseUri.joinPath()for constructing file paths instead of string concatenation with/to ensure platform-correct path separators
Follow established patterns when importing new packages, using helper imports rather than direct imports (e.g., useimport { generateUuid } from '../platform/common/uuid'instead of importing uuid directly)
Add blank lines after const groups and before return statements for readability
Separate third-party and local file imports
Files:
src/platform/common/constants.tssrc/notebooks/deepnote/deepnoteExplorerView.unit.test.tssrc/notebooks/deepnote/deepnoteExplorerView.ts
**/*.unit.test.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Create unit tests in
*.unit.test.tsfiles
**/*.unit.test.ts: Unit tests use Mocha/Chai framework with.unit.test.tsextension
Test files should be placed alongside the source files they test
Useassert.deepStrictEqual()for object comparisons instead of checking individual properties
Files:
src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Create integration tests in
*.test.tsfiles (not*.unit.test.ts)
**/*.test.ts: ALWAYS check the 'Unittest - Build' task output for TypeScript compilation errors before running any script or declaring work complete
When a mock is returned from a promise in unit tests, ensure the mocked instance has an undefinedthenproperty to avoid hanging tests
Usenpm run test:unittestsfor TypeScript unit tests before committing changes
Files:
src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts
src/notebooks/deepnote/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Deepnote integration located in
src/notebooks/deepnote/with 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)
Files:
src/notebooks/deepnote/deepnoteExplorerView.unit.test.tssrc/notebooks/deepnote/deepnoteExplorerView.ts
🧬 Code graph analysis (1)
src/notebooks/deepnote/deepnoteExplorerView.ts (2)
src/notebooks/deepnote/deepnoteTreeItem.ts (1)
DeepnoteTreeItem(25-126)src/notebooks/deepnote/deepnoteProjectUtils.ts (1)
readDeepnoteProjectFile(5-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build & Test
🔇 Additional comments (7)
src/platform/common/constants.ts (1)
250-251: New Deepnote export command IDs are consistent
ExportProjectandExportNotebookmatch the new manifest command IDs and follow existing naming patterns; no issues spotted.package.nls.json (1)
276-277: Localization entries line up with new commandsThe added titles for project/notebook export use the expected key naming and match the new commands; looks good.
src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts (1)
2074-2317: Export‑notebook tests cover the key branches cleanlyThe
exportNotebooksuite mirrors the project export tests well: early returns on user cancel, invalid Deepnote file handling, single‑notebook filtering bynotebookId, not‑found behavior, and write failures are all exercised. The mocking pattern (VS Code APIs +workspace.fs) is consistent with the rest of the file; no issues from a test‑logic standpoint.package.json (1)
282-293: Export commands are correctly declared in the manifest
deepnote.exportProjectanddeepnote.exportNotebookare added with localized titles, category, and icons, and match the identifiers inconstants.tsandpackage.nls.json. Nothing problematic here.src/notebooks/deepnote/deepnoteExplorerView.ts (3)
5-5: Import addition looks good.Follows existing import pattern from
@deepnote/convert.
401-412: Command registrations follow the established pattern.
1032-1035: Error handling follows project conventions.Uses
l10n.t()and extracts error message safely.Also applies to: 1092-1095
saltenasl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀 🚀 🚀
Export projects and/or notebooks as Jupyter notebooks.
Fixes #230
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.