-
Notifications
You must be signed in to change notification settings - Fork 6
feat: Add commands to add Text blocks to notebooks #286
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 PR adds text-block insertion to the Deepnote VS Code extension. It introduces five new commands (picker plus paragraph and three heading-specific), localization entries, command constants, and command-to-argument mappings. DeepnoteNotebookCommandListener now shows a QuickPick for text-block types and implements handlers to insert a markdown text cell with starter content and pocket metadata, reveal the cell, and enter edit mode. Sequence Diagram(s)sequenceDiagram
actor User
participant CommandPalette as Command Palette
participant Picker as QuickPick UI
participant Listener as DeepnoteNotebookCommandListener
participant Editor as NotebookEditor
participant Cell as Notebook Cell
User->>CommandPalette: Invoke deepnote.addTextBlock
CommandPalette->>Listener: addTextBlockThroughPicker()
Listener->>Picker: Show options (Paragraph, H1, H2, H3)
Picker->>User: Display options
User->>Picker: Select option
Picker->>Listener: Return selection
Listener->>Listener: addTextBlock(editor, textBlockType)
Listener->>Editor: Insert markdown cell
Editor->>Cell: Create cell with starter content
Listener->>Cell: Attach metadata (__deepnotePocket.type)
Listener->>Editor: Reveal/select cell
Listener->>Editor: Enter edit mode
sequenceDiagram
actor User
participant CommandPalette as Command Palette
participant Listener as DeepnoteNotebookCommandListener
participant Editor as NotebookEditor
participant Cell as Notebook Cell
User->>CommandPalette: Invoke deepnote.addTextBlockHeading1
CommandPalette->>Listener: addTextBlockCommandHandler(textBlockType: 'text-cell-h1')
Listener->>Listener: Resolve active editor
Listener->>Listener: addTextBlock(editor, 'text-cell-h1')
Listener->>Editor: Insert markdown cell
Editor->>Cell: Create cell with H1 starter content
Listener->>Cell: Attach metadata
Listener->>Editor: Reveal/select cell
Listener->>Editor: Enter edit mode
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #286 +/- ##
===========================
===========================
🚀 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: 3
🤖 Fix all issues with AI agents
In @src/notebooks/deepnote/deepnoteNotebookCommandListener.ts:
- Around line 447-486: Replace the plain Error thrown in addTextBlock when
notebookUpdaterUtils.chainWithPendingUpdates fails with the typed WrappedError
used elsewhere (e.g., addChartBlock); specifically, change the throw new
Error(l10n.t('Failed to insert text block')) to throw new
WrappedError(l10n.t('Failed to insert text block')) (or use the same
WrappedError factory/constructor pattern used in addChartBlock) so the failure
case is consistent with existing error handling and typing.
- Around line 402-427: The QuickPick description and placeholder are mismatched
for the text-block picker: in addTextBlockThroughPicker(), change the items
creation (where TEXT_BLOCK_TYPE_LABELS and items are used) so the description
uses the human-readable label (TEXT_BLOCK_TYPE_LABELS[textBlockType]) instead of
the raw textBlockType ID, and update the window.showQuickPick options to use a
placeholder that references text blocks (e.g., "Select a text block type")
rather than "environment" so the picker copy matches its purpose.
In @src/platform/common/constants.ts:
- Around line 239-243: The three heading constants use camelCase while the rest
are PascalCase; rename addTextBlockHeading1, addTextBlockHeading2, and
addTextBlockHeading3 to AddTextBlockHeading1, AddTextBlockHeading2, and
AddTextBlockHeading3 respectively in the constants file and update any
references/usages throughout the codebase to the new symbol names; keep the
underlying string values (e.g., 'deepnote.addTextBlockHeading1') unchanged so
command IDs remain stable.
📜 Review details
Configuration used: Organization 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 selected for processing (5)
package.jsonpackage.nls.jsonsrc/commands.tssrc/notebooks/deepnote/deepnoteNotebookCommandListener.tssrc/platform/common/constants.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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/deepnoteNotebookCommandListener.tssrc/commands.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/deepnoteNotebookCommandListener.tssrc/commands.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/deepnoteNotebookCommandListener.tssrc/commands.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/deepnoteNotebookCommandListener.ts
🧬 Code graph analysis (1)
src/notebooks/deepnote/deepnoteNotebookCommandListener.ts (3)
src/kernels/execution/notebookUpdater.ts (1)
notebookUpdaterUtils(65-68)src/test/mocks/vsc/extHostedTypes.ts (3)
NotebookCellData(2547-2573)NotebookEdit(2468-2531)NotebookRange(2437-2467)src/platform/deepnote/pocket.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build & Test
🔇 Additional comments (5)
package.nls.json (1)
269-273: Text block titles are consistent and well-scopedThe new localization keys and titles match the command IDs and existing naming for other block types; no issues here.
package.json (2)
233-262: Command contributions for text blocks look correctCommand IDs, l10n titles, category, and icon wiring for the five new text-block commands are consistent with existing Deepnote block commands and with the constants in
src/platform/common/constants.ts.
1059-1063: Toolbar wiring for generic “Add Text Block” is correctThe notebook toolbar menu entry correctly targets
deepnote.addTextBlock, uses a sensible group ordering, and scopes itself tonotebookType == 'deepnote'; behavior should align with the new picker-based flow.src/commands.ts (1)
200-204: Type mapping for new text-block commands is correctAll five new text-block commands are added to
ICommandNameArgumentTypeMappingwith empty argument tuples, matching how they’re invoked; this keeps command usage strongly typed.src/notebooks/deepnote/deepnoteNotebookCommandListener.ts (1)
191-213: Command registrations are wired correctlyThe new registrations for
AddTextBlock,addTextBlockHeading1/2/3, andAddTextBlockParagraphcorrectly route to the picker and specific handlers, and they don’t depend on external arguments, so they’ll work both from the toolbar and command palette.
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: 1
🤖 Fix all issues with AI agents
In @src/notebooks/deepnote/deepnoteNotebookCommandListener.ts:
- Around line 402-438: The Quick Pick placeholder in addTextBlockThroughPicker
is incorrect ("Select an environment for this notebook"); update the placeholder
passed to window.showQuickPick to reflect text block selection (e.g., use
l10n.t('Select a text block to add') or similar localized string) so it matches
the TEXT_BLOCK_TYPES/ TEXT_BLOCK_TYPE_LABELS context and intent of the method.
📜 Review details
Configuration used: Organization 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 selected for processing (3)
src/commands.tssrc/notebooks/deepnote/deepnoteNotebookCommandListener.tssrc/platform/common/constants.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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/commands.tssrc/platform/common/constants.tssrc/notebooks/deepnote/deepnoteNotebookCommandListener.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/commands.tssrc/platform/common/constants.tssrc/notebooks/deepnote/deepnoteNotebookCommandListener.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/commands.tssrc/platform/common/constants.tssrc/notebooks/deepnote/deepnoteNotebookCommandListener.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
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/deepnoteNotebookCommandListener.ts
🧬 Code graph analysis (1)
src/notebooks/deepnote/deepnoteNotebookCommandListener.ts (3)
src/platform/logging/index.ts (1)
logger(35-48)src/test/mocks/vsc/extHostedTypes.ts (3)
NotebookCellData(2547-2573)NotebookEdit(2468-2531)NotebookRange(2437-2467)src/platform/deepnote/pocket.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build & Test
🔇 Additional comments (7)
src/platform/common/constants.ts (1)
239-243: LGTM!Command constants follow established naming conventions and are properly grouped with similar commands.
src/commands.ts (1)
200-204: LGTM!Command-to-argument mappings correctly specify empty argument tuples, consistent with other block insertion commands in this interface.
src/notebooks/deepnote/deepnoteNotebookCommandListener.ts (5)
11-13: LGTM!Import additions are appropriate for the new text block functionality.
Also applies to: 38-38
51-53: LGTM!Constant uses
as constfor proper type narrowing, and the type derivation follows TypeScript best practices.
191-213: LGTM!Command registrations follow established patterns and properly use the disposable registry for cleanup.
440-447: LGTM!Clean delegation pattern with proper error handling.
449-488: LGTM!Implementation follows established patterns for block insertion, uses typed errors, proper localization, and correct VS Code notebook APIs.
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: 3
🤖 Fix all issues with AI agents
In @src/notebooks/deepnote/deepnoteNotebookCommandListener.ts:
- Around line 191-213: The command handler methods are exposed publicly but are
internal-only; change the visibility of addTextBlockThroughPicker,
addTextBlockCommandHandler, and addTextBlock from public to private in the
DeepnoteNotebookCommandListener class so they are not part of the class API
surface. Ensure the command registrations that call these methods (where
Commands.AddTextBlock*, etc. are registered) remain inside the class so private
access still works, and update any external callers to stop using these methods
(or add a new public facade if external access is required).
- Around line 402-447: The two places that throw plain Error when there is no
active notebook editor — addTextBlockThroughPicker and
addTextBlockCommandHandler — should throw the same typed WrappedError used
elsewhere in this file; replace `throw new Error(l10n.t('No active notebook
editor found'))` with creating and throwing a WrappedError with the same
localized message (matching the pattern used at the earlier occurrence of
WrappedError in this file) so callers can distinguish and handle this specific
error type.
- Around line 402-438: The QuickPick is showing internal ids because description
is built using textBlockType; in addTextBlockThroughPicker() replace the
description assignment so it uses the localized label
(TEXT_BLOCK_TYPE_LABELS[textBlockType]) and, if you still want the internal id
searchable, set detail: textBlockType (or vice‑versa) when returning each item;
keep label as the localized label and ensure logger/info and addTextBlock call
continue to use selected.textBlockType.
📜 Review details
Configuration used: Organization 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 selected for processing (1)
src/notebooks/deepnote/deepnoteNotebookCommandListener.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{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/notebooks/deepnote/deepnoteNotebookCommandListener.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/notebooks/deepnote/deepnoteNotebookCommandListener.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/notebooks/deepnote/deepnoteNotebookCommandListener.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/deepnoteNotebookCommandListener.ts
⏰ 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 (2)
src/notebooks/deepnote/deepnoteNotebookCommandListener.ts (2)
11-14: Nice:TEXT_BLOCK_TYPES+TextBlockTypekeeps the command args honest.
No concerns with these additions.Also applies to: 38-38, 51-54
449-488: The text-cell-* types are established and valid Pocket types. They're defined inTEXT_BLOCK_TYPES, properly handled bytextBlockConverter, and have comprehensive test coverage. The code is correct.Likely an incorrect or invalid review 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.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @src/notebooks/deepnote/deepnoteNotebookCommandListener.ts:
- Around line 402-488: Summary: Public methods are out of alphabetical order;
move them to comply with the rule "order by accessibility then alphabetically."
Fix: relocate the three public methods addTextBlock, addTextBlockCommandHandler,
and addTextBlockThroughPicker so they appear with the other public methods and
are alphabetized (e.g., after addInputBlock should be addTextBlock,
addTextBlockCommandHandler, addTextBlockThroughPicker). Keep their signatures,
implementations, and any referenced symbols
(notebookUpdaterUtils.chainWithPendingUpdates, TEXT_BLOCK_TYPE_* constants,
logger, commands.executeCommand) unchanged while only reordering their
declarations.
- Line 412: Replace the generic throw new Error(...) uses with the project's
typed error class (use WrappedError as used elsewhere in this file) so error
handling is consistent; change the throw new Error(l10n.t('No active notebook
editor found')) to throw a WrappedError(...) including the localized message and
any available context/inner error, and make the same replacement for the other
occurrences flagged (the throws at the other places referenced around lines 443
and 480) so all three use WrappedError instead of Error.
- Around line 425-429: The QuickPick call to window.showQuickPick should accept
and respect a CancellationToken: update the containing function to accept a
CancellationToken parameter (commonly named token or cancellationToken), pass
that token as the third argument to window.showQuickPick(items, { ... }, token),
and add an early return if token?.isCancellationRequested before/after awaiting
the call (in addition to the existing undefined check on selected). This ensures
the selected variable handling honors explicit cancellation tokens.
📜 Review details
Configuration used: Organization 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 selected for processing (1)
src/notebooks/deepnote/deepnoteNotebookCommandListener.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{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/notebooks/deepnote/deepnoteNotebookCommandListener.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/notebooks/deepnote/deepnoteNotebookCommandListener.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/notebooks/deepnote/deepnoteNotebookCommandListener.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/deepnoteNotebookCommandListener.ts
⏰ 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
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: 4
🤖 Fix all issues with AI agents
In @src/notebooks/deepnote/deepnoteNotebookCommandListener.ts:
- Around line 402-488: New public methods addTextBlockThroughPicker,
addTextBlockCommandHandler, and addTextBlock are out of alphabetical order;
reorder them among other public methods so methods are alphabetized within the
same accessibility level (e.g., move addTextBlock to come after addSqlBlock and
place addTextBlockCommandHandler and addTextBlockThroughPicker in their proper
alphabetical positions relative to other public methods) while preserving method
implementations and any references/calls.
- Around line 440-447: Replace the generic Error thrown in
addTextBlockCommandHandler with the typed WrappedError: when
window.activeNotebookEditor is falsy, throw new WrappedError(l10n.t('No active
notebook editor found')) (or wrap any additional context if needed); keep the
existing l10n.t message and ensure WrappedError (imported at top) is used
instead of Error in the addTextBlockCommandHandler method.
- Around line 449-488: In addTextBlock, replace the generic throw new
Error(l10n.t('Failed to insert text block')) with throwing the typed
WrappedError (already imported) so the failure is wrapped consistently; update
the line to throw new WrappedError(l10n.t('Failed to insert text block')) (or
include any available context/cause if WrappedError supports a second argument)
so the error thrown from notebookUpdaterUtils.chainWithPendingUpdates is a
WrappedError instead of Error.
- Around line 402-438: In addTextBlockThroughPicker replace the generic throw
new Error(...) with a typed WrappedError: change throw new Error(l10n.t('No
active notebook editor found')) to throw new WrappedError(l10n.t('No active
notebook editor found')) (WrappedError is already imported), ensuring the typed
error class is used for better error handling and consistency.
📜 Review details
Configuration used: Organization 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 selected for processing (1)
src/notebooks/deepnote/deepnoteNotebookCommandListener.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{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/notebooks/deepnote/deepnoteNotebookCommandListener.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/notebooks/deepnote/deepnoteNotebookCommandListener.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/notebooks/deepnote/deepnoteNotebookCommandListener.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/deepnoteNotebookCommandListener.ts
🧬 Code graph analysis (1)
src/notebooks/deepnote/deepnoteNotebookCommandListener.ts (4)
src/platform/logging/index.ts (1)
logger(35-48)src/kernels/execution/notebookUpdater.ts (1)
notebookUpdaterUtils(65-68)src/test/mocks/vsc/extHostedTypes.ts (3)
NotebookCellData(2547-2573)NotebookEdit(2468-2531)NotebookRange(2437-2467)src/platform/deepnote/pocket.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build & Test
🔇 Additional comments (5)
src/notebooks/deepnote/deepnoteNotebookCommandListener.ts (5)
11-13: LGTM!New imports support the text block picker and command functionality correctly.
38-38: LGTM!Pocket import needed for type-safe metadata annotation in text block creation.
51-53: LGTM!Type-safe constants with proper
as constassertion and derived union type.
471-475: Verify metadata pattern.Text blocks only set
__deepnotePocketmetadata, whileaddSqlBlock(line 239-245) andaddInputBlock(line 372-378) spread metadata to the root level as well. Chart blocks follow the same pattern as text blocks. Confirm this difference is intentional based on block type requirements.
191-213: All command constants are properly defined insrc/platform/common/constants.ts(lines 239–243).
Artmann
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.
🚀

Fixes #277
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.