feat: Better UX for input blocks #115
Conversation
📝 WalkthroughWalkthroughAdds a centralized formatter and language mapper for input-block cell content, and updates Deepnote conversion and cell-creation flows to emit plaintext or type-specific content and languages. Introduces DeepnoteInputBlockEditProtection to revert edits to readonly input blocks and batch-fix language IDs. Reworks the input-block status-bar provider to return multiple items, register type-specific commands, expose onDidChangeCellStatusBarItems, and open a Select Input Settings webview. Implements a React Select Input Settings webview and provider that persist settings via WorkspaceEdit. Wires the webview provider and edit protection into activation and adds an esbuild build step for the new webview bundle. Sequence Diagram(s)sequenceDiagram
participant User
participant StatusBar as DeepnoteInputBlockCellStatusBarProvider
participant Webview as SelectInputSettingsWebviewProvider
participant Formatter as formatInputBlockCellContent
participant VSCode
participant Notebook
Note over StatusBar: User clicks status-bar control
User->>StatusBar: command "selectInputSettings" (cell)
StatusBar->>Webview: show(cell)
Webview->>VSCode: create webview panel & post 'init' (settings, loc)
VSCode->>User: render webview UI
User->>Webview: postMessage 'save' (settings)
Webview->>Notebook: apply WorkspaceEdit (update cell metadata/content)
Notebook-->>Webview: edit result
Webview-->>StatusBar: resolve → status change event
StatusBar->>VSCode: provideCellStatusBarItems refreshed
sequenceDiagram
participant User
participant VSCode
participant EditProt as DeepnoteInputBlockEditProtection
participant Formatter as formatInputBlockCellContent
participant Notebook
User->>VSCode: edits notebook cell(s)
VSCode->>EditProt: onDidChangeNotebookDocument(event)
alt Edited readonly input block
EditProt->>Formatter: formatInputBlockCellContent(blockType, metadata)
Formatter-->>EditProt: expectedContent
EditProt->>Notebook: apply WorkspaceEdit to revert cell content
end
alt Language mismatches detected
EditProt->>EditProt: collect mismatches across notebook
EditProt->>Notebook: apply batched WorkspaceEdit to fix language IDs
end
Possibly related PRs
Suggested reviewers
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 26
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (14)
build/esbuild/build.ts(1 hunks)src/notebooks/deepnote/converters/inputConverters.ts(12 hunks)src/notebooks/deepnote/converters/inputConverters.unit.test.ts(16 hunks)src/notebooks/deepnote/deepnoteActivationService.ts(3 hunks)src/notebooks/deepnote/deepnoteInputBlockCellStatusBarProvider.ts(5 hunks)src/notebooks/deepnote/deepnoteInputBlockCellStatusBarProvider.unit.test.ts(2 hunks)src/notebooks/deepnote/deepnoteInputBlockEditProtection.ts(1 hunks)src/notebooks/deepnote/inputBlockContentFormatter.ts(1 hunks)src/notebooks/deepnote/selectInputSettingsWebview.ts(1 hunks)src/webviews/webview-side/selectInputSettings/SelectInputSettingsPanel.tsx(1 hunks)src/webviews/webview-side/selectInputSettings/index.html(1 hunks)src/webviews/webview-side/selectInputSettings/index.tsx(1 hunks)src/webviews/webview-side/selectInputSettings/selectInputSettings.css(1 hunks)src/webviews/webview-side/selectInputSettings/types.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/!(*.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/selectInputSettingsWebview.tsbuild/esbuild/build.tssrc/webviews/webview-side/selectInputSettings/types.tssrc/notebooks/deepnote/inputBlockContentFormatter.tssrc/notebooks/deepnote/deepnoteInputBlockEditProtection.tssrc/notebooks/deepnote/deepnoteInputBlockCellStatusBarProvider.unit.test.tssrc/notebooks/deepnote/converters/inputConverters.unit.test.tssrc/notebooks/deepnote/converters/inputConverters.tssrc/notebooks/deepnote/deepnoteInputBlockCellStatusBarProvider.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Inject interfaces, not concrete classes
Avoid circular dependencies
Usel10n.t()for user-facing strings
Use typed error classes fromsrc/platform/errors/when throwing or handling errors
Use theILoggerservice instead ofconsole.log
Preserve error details while scrubbing PII in messages and telemetry
Include the Microsoft copyright header in source files
Prefer async/await and handle cancellation withCancellationToken
**/*.{ts,tsx}: Order class members (methods, fields, properties) first by accessibility (public/protected/private) and then alphabetically
Do not add the Microsoft copyright header to new files
Use Uri.joinPath() to construct file paths instead of manual string concatenation
Add a blank line after groups of const declarations and before return statements for readability
Separate third-party imports from local file imports
Files:
src/notebooks/deepnote/deepnoteActivationService.tssrc/notebooks/deepnote/selectInputSettingsWebview.tsbuild/esbuild/build.tssrc/webviews/webview-side/selectInputSettings/types.tssrc/notebooks/deepnote/inputBlockContentFormatter.tssrc/notebooks/deepnote/deepnoteInputBlockEditProtection.tssrc/notebooks/deepnote/deepnoteInputBlockCellStatusBarProvider.unit.test.tssrc/webviews/webview-side/selectInputSettings/SelectInputSettingsPanel.tsxsrc/notebooks/deepnote/converters/inputConverters.unit.test.tssrc/webviews/webview-side/selectInputSettings/index.tsxsrc/notebooks/deepnote/converters/inputConverters.tssrc/notebooks/deepnote/deepnoteInputBlockCellStatusBarProvider.ts
src/notebooks/deepnote/**
📄 CodeRabbit inference engine (CLAUDE.md)
Deepnote integration code resides under src/notebooks/deepnote/
Files:
src/notebooks/deepnote/deepnoteActivationService.tssrc/notebooks/deepnote/selectInputSettingsWebview.tssrc/notebooks/deepnote/inputBlockContentFormatter.tssrc/notebooks/deepnote/deepnoteInputBlockEditProtection.tssrc/notebooks/deepnote/deepnoteInputBlockCellStatusBarProvider.unit.test.tssrc/notebooks/deepnote/converters/inputConverters.unit.test.tssrc/notebooks/deepnote/converters/inputConverters.tssrc/notebooks/deepnote/deepnoteInputBlockCellStatusBarProvider.ts
src/notebooks/deepnote/deepnoteActivationService.ts
📄 CodeRabbit inference engine (CLAUDE.md)
deepnoteActivationService.ts manages VS Code activation for Deepnote
Files:
src/notebooks/deepnote/deepnoteActivationService.ts
**/*.unit.test.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place unit tests in files matching
*.unit.test.ts
**/*.unit.test.ts: Unit tests must use Mocha/Chai and have the .unit.test.ts extension
Place unit test files alongside the source files they test
Use assert.deepStrictEqual() for object comparisons in tests instead of checking individual properties
Files:
src/notebooks/deepnote/deepnoteInputBlockCellStatusBarProvider.unit.test.tssrc/notebooks/deepnote/converters/inputConverters.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/deepnoteInputBlockCellStatusBarProvider.unit.test.tssrc/notebooks/deepnote/converters/inputConverters.unit.test.ts
🧠 Learnings (7)
📚 Learning: 2025-10-09T11:21:57.494Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-09T11:21:57.494Z
Learning: Applies to src/notebooks/deepnote/deepnoteActivationService.ts : deepnoteActivationService.ts manages VS Code activation for Deepnote
Applied to files:
src/notebooks/deepnote/deepnoteActivationService.ts
📚 Learning: 2025-10-09T11:21:57.494Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-09T11:21:57.494Z
Learning: Applies to src/notebooks/deepnote/deepnoteSerializer.ts : deepnoteSerializer.ts is the main serializer orchestrating Deepnote integration
Applied to files:
src/notebooks/deepnote/deepnoteActivationService.ts
📚 Learning: 2025-10-09T11:21:57.494Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-09T11:21:57.494Z
Learning: Applies to src/notebooks/deepnote/deepnoteNotebookManager.ts : deepnoteNotebookManager.ts handles Deepnote notebook state management
Applied to files:
src/notebooks/deepnote/deepnoteActivationService.ts
📚 Learning: 2025-10-09T11:21:57.494Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-09T11:21:57.494Z
Learning: Applies to src/notebooks/deepnote/deepnoteNotebookSelector.ts : deepnoteNotebookSelector.ts implements UI selection logic for Deepnote notebooks
Applied to files:
src/notebooks/deepnote/selectInputSettingsWebview.ts
📚 Learning: 2025-09-03T12:59:14.489Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/notebooks.instructions.md:0-0
Timestamp: 2025-09-03T12:59:14.489Z
Learning: Applies to src/notebooks/export/**/*.ts : Update FileConverter to handle the new export format
Applied to files:
src/notebooks/deepnote/converters/inputConverters.unit.test.ts
📚 Learning: 2025-10-09T11:21:57.494Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-09T11:21:57.494Z
Learning: Applies to src/notebooks/deepnote/deepnoteDataConverter.ts : deepnoteDataConverter.ts performs Deepnote data transformations
Applied to files:
src/notebooks/deepnote/converters/inputConverters.unit.test.tssrc/notebooks/deepnote/converters/inputConverters.ts
📚 Learning: 2025-10-20T15:03:43.208Z
Learnt from: jankuca
PR: deepnote/vscode-deepnote#86
File: src/notebooks/deepnote/sqlCellStatusBarProvider.ts:98-126
Timestamp: 2025-10-20T15:03:43.208Z
Learning: In VS Code's notebook API, NotebookCellStatusBarItem is an interface, not a class with a constructor. Status bar items should be created as plain object literals that conform to the NotebookCellStatusBarItem interface. Do not suggest using `new NotebookCellStatusBarItem(...)` in TypeScript files working with VS Code notebooks.
Applied to files:
src/notebooks/deepnote/deepnoteInputBlockCellStatusBarProvider.ts
🧬 Code graph analysis (7)
src/notebooks/deepnote/deepnoteActivationService.ts (1)
src/notebooks/deepnote/deepnoteInputBlockEditProtection.ts (1)
DeepnoteInputBlockEditProtection(19-175)
src/notebooks/deepnote/selectInputSettingsWebview.ts (2)
src/notebooks/deepnote/deepnoteInputBlockCellStatusBarProvider.ts (1)
injectable(32-1125)src/test/mocks/vsc/extHostedTypes.ts (3)
WorkspaceEdit(649-833)NotebookEdit(2444-2507)NotebookRange(2413-2443)
src/notebooks/deepnote/deepnoteInputBlockEditProtection.ts (2)
src/notebooks/deepnote/inputBlockContentFormatter.ts (1)
formatInputBlockCellContent(12-91)src/test/mocks/vsc/extHostedTypes.ts (2)
NotebookEdit(2444-2507)NotebookRange(2413-2443)
src/webviews/webview-side/selectInputSettings/SelectInputSettingsPanel.tsx (4)
src/webviews/webview-side/react-common/postOffice.ts (1)
IVsCodeApi(9-16)src/webviews/webview-side/selectInputSettings/types.ts (2)
SelectInputSettings(4-10)WebviewMessage(12-16)src/notebooks/deepnote/selectInputSettingsWebview.ts (1)
handleMessage(122-143)src/webviews/webview-side/react-common/locReactSide.ts (2)
storeLocStrings(19-21)getLocString(11-17)
src/notebooks/deepnote/converters/inputConverters.unit.test.ts (2)
src/test/mocks/vsc/extHostedTypes.ts (1)
NotebookCellData(2523-2549)src/notebooks/deepnote/converters/constants.ts (1)
DEEPNOTE_VSCODE_RAW_CONTENT_KEY(1-1)
src/notebooks/deepnote/converters/inputConverters.ts (2)
src/notebooks/deepnote/converters/constants.ts (1)
DEEPNOTE_VSCODE_RAW_CONTENT_KEY(1-1)src/notebooks/deepnote/inputBlockContentFormatter.ts (1)
formatInputBlockCellContent(12-91)
src/notebooks/deepnote/deepnoteInputBlockCellStatusBarProvider.ts (3)
src/test/mocks/vsc/index.ts (1)
EventEmitter(65-89)src/test/mocks/vsc/extHostedTypes.ts (2)
WorkspaceEdit(649-833)NotebookEdit(2444-2507)src/notebooks/deepnote/inputBlockContentFormatter.ts (1)
formatInputBlockCellContent(12-91)
🪛 Biome (2.1.2)
src/webviews/webview-side/selectInputSettings/SelectInputSettingsPanel.tsx
[error] 136-139: Static Elements should not be interactive.
To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.
(lint/a11y/noStaticElementInteractions)
[error] 136-139: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
[error] 198-201: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
[error] 198-201: Static Elements should not be interactive.
To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.
(lint/a11y/noStaticElementInteractions)
[error] 158-164: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 184-189: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 233-233: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 236-236: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
src/notebooks/deepnote/deepnoteInputBlockCellStatusBarProvider.ts
[error] 522-522: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 523-523: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 524-524: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 531-531: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 538-538: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 545-545: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 546-546: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🪛 GitHub Actions: CI
src/notebooks/deepnote/deepnoteInputBlockEditProtection.ts
[error] 136-136: Unexpected any. Specify a different type @typescript-eslint/no-explicit-any
src/webviews/webview-side/selectInputSettings/SelectInputSettingsPanel.tsx
[error] 101-101: TS2345: Argument of type '"selectInputSettingsTitle"' is not assignable to parameter of type 'keyof LocalizedMessages'.
🪛 GitHub Check: Lint & Format
src/notebooks/deepnote/deepnoteInputBlockEditProtection.ts
[failure] 136-136:
Unexpected any. Specify a different type
🪛 GitHub Check: TypeCheck
src/webviews/webview-side/selectInputSettings/SelectInputSettingsPanel.tsx
[failure] 213-213:
Argument of type '"fromVariableDescription"' is not assignable to parameter of type 'keyof LocalizedMessages'.
[failure] 210-210:
Argument of type '"fromVariable"' is not assignable to parameter of type 'keyof LocalizedMessages'.
[failure] 190-190:
Argument of type '"addButton"' is not assignable to parameter of type 'keyof LocalizedMessages'.
[failure] 181-181:
Argument of type '"addOptionPlaceholder"' is not assignable to parameter of type 'keyof LocalizedMessages'.
[failure] 150-150:
Argument of type '"fromOptionsDescription"' is not assignable to parameter of type 'keyof LocalizedMessages'.
[failure] 148-148:
Argument of type '"fromOptions"' is not assignable to parameter of type 'keyof LocalizedMessages'.
[failure] 133-133:
Argument of type '"valueSourceTitle"' is not assignable to parameter of type 'keyof LocalizedMessages'.
[failure] 120-120:
Argument of type '"allowEmptyValue"' is not assignable to parameter of type 'keyof LocalizedMessages'.
[failure] 106-106:
Argument of type '"allowMultipleValues"' is not assignable to parameter of type 'keyof LocalizedMessages'.
[failure] 101-101:
Argument of type '"selectInputSettingsTitle"' is not assignable to parameter of type 'keyof LocalizedMessages'.
🔇 Additional comments (8)
src/notebooks/deepnote/deepnoteActivationService.ts (1)
23-24: Edit protection wiring looks good.Instantiation and disposal via context.subscriptions are correct.
Also applies to: 40-44
build/esbuild/build.ts (1)
382-387: No action needed—review comment is based on a false premise.The CSS import in
selectInputSettings/index.tsxis handled correctly by the style plugin: it injects CSS at runtime into the JS bundle viainjectStyle(). No provider currently linksselectInputSettings.css, and no separate CSS file is emitted—which is the intended behavior. The build entry is correct.Likely an incorrect or invalid review comment.
src/notebooks/deepnote/converters/inputConverters.unit.test.ts (3)
43-46: LGTM: plaintext for input-text is correct.Expectation aligns with new formatter-driven model.
203-206: LGTM: textarea now emits Python variable stub.'# input_2' pattern is consistent and clear.
943-946: LGTM: button renders Python comment.Matches UX: buttons are non-executable in VS Code.
src/notebooks/deepnote/deepnoteInputBlockCellStatusBarProvider.unit.test.ts (1)
62-63: Avoid magic number for alignment.Assert against the enum for clarity.
-import { NotebookCell, NotebookCellKind, NotebookDocument } from 'vscode'; +import { NotebookCell, NotebookCellKind, NotebookDocument, NotebookCellStatusBarAlignment } from 'vscode'; @@ - expect(items?.[0].alignment).to.equal(1); // Left + expect(items?.[0].alignment).to.equal(NotebookCellStatusBarAlignment.Left);⛔ Skipped due to learnings
Learnt from: jankuca PR: deepnote/vscode-deepnote#86 File: src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts:51-64 Timestamp: 2025-10-20T15:08:08.268Z Learning: In the vscode-deepnote test files (*.unit.test.ts), `NotebookCellStatusBarAlignment` cannot be imported from 'vscode' and is not available in the test environment. Use the numeric value (1 for Left alignment) directly in assertions instead.Learnt from: jankuca PR: deepnote/vscode-deepnote#86 File: src/notebooks/deepnote/sqlCellStatusBarProvider.ts:118-128 Timestamp: 2025-10-21T15:14:08.102Z Learning: In the vscode-deepnote project, `NotebookCellStatusBarAlignment` cannot be imported from 'vscode'. Always use numeric values with inline comments (e.g., `alignment: 1, // NotebookCellStatusBarAlignment.Left`) instead of attempting to import and use the enum constant.src/notebooks/deepnote/converters/inputConverters.ts (1)
87-91: Verify 'plaintext' as a code-cell languageUsing NotebookCellKind.Code with language 'plaintext' may not be recognized by the Deepnote controller. Prefer 'python' (or Markup cell) for consistency. As per coding guidelines.
-const cell = new NotebookCellData(NotebookCellKind.Code, cellValue, 'plaintext'); +const cell = new NotebookCellData(NotebookCellKind.Code, cellValue, 'python');Same for InputTextareaBlockConverter.
src/notebooks/deepnote/deepnoteInputBlockCellStatusBarProvider.ts (1)
4-20: Use NotebookCellStatusBarAlignment enum, not magic numberReplace alignment: 1 with NotebookCellStatusBarAlignment.Left for clarity and safety.
import { Disposable, EventEmitter, NotebookCell, NotebookCellStatusBarItem, + NotebookCellStatusBarAlignment, NotebookCellStatusBarItemProvider, ... - items.push({ + items.push({ text: statusText, - alignment: 1, // NotebookCellStatusBarAlignment.Left + alignment: NotebookCellStatusBarAlignment.Left, priority: 100, tooltip: tooltipLines.join('\n') });Apply to all items in this file.
Also applies to: 248-255
⛔ Skipped due to learnings
Learnt from: jankuca PR: deepnote/vscode-deepnote#86 File: src/notebooks/deepnote/sqlCellStatusBarProvider.ts:118-128 Timestamp: 2025-10-21T15:14:08.102Z Learning: In the vscode-deepnote project, `NotebookCellStatusBarAlignment` cannot be imported from 'vscode'. Always use numeric values with inline comments (e.g., `alignment: 1, // NotebookCellStatusBarAlignment.Left`) instead of attempting to import and use the enum constant.Learnt from: jankuca PR: deepnote/vscode-deepnote#86 File: src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts:51-64 Timestamp: 2025-10-20T15:08:08.268Z Learning: In the vscode-deepnote test files (*.unit.test.ts), `NotebookCellStatusBarAlignment` cannot be imported from 'vscode' and is not available in the test environment. Use the numeric value (1 for Left alignment) directly in assertions instead.Learnt from: jankuca PR: deepnote/vscode-deepnote#86 File: src/notebooks/deepnote/sqlCellStatusBarProvider.ts:98-126 Timestamp: 2025-10-20T15:03:43.208Z Learning: In VS Code's notebook API, NotebookCellStatusBarItem is an interface, not a class with a constructor. Status bar items should be created as plain object literals that conform to the NotebookCellStatusBarItem interface. Do not suggest using `new NotebookCellStatusBarItem(...)` in TypeScript files working with VS Code notebooks.
src/notebooks/deepnote/deepnoteInputBlockCellStatusBarProvider.ts
Outdated
Show resolved
Hide resolved
src/webviews/webview-side/selectInputSettings/SelectInputSettingsPanel.tsx
Show resolved
Hide resolved
src/webviews/webview-side/selectInputSettings/SelectInputSettingsPanel.tsx
Show resolved
Hide resolved
src/webviews/webview-side/selectInputSettings/SelectInputSettingsPanel.tsx
Show resolved
Hide resolved
src/webviews/webview-side/selectInputSettings/SelectInputSettingsPanel.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/notebooks/deepnote/selectInputSettingsWebview.ts (3)
54-64: Localize the panel title.Use
l10n.t()for the webview title per i18n guidelines.+import { l10n } from 'vscode'; + this.currentPanel = window.createWebviewPanel( 'deepnoteSelectInputSettings', - 'Select Input Settings', + l10n.t('Select Input Settings'), column || ViewColumn.One,
175-207: Use updateCellMetadata instead of replaceCells.Replacing the cell drops outputs and attachments. Update metadata directly to preserve cell state.
const edit = new WorkspaceEdit(); const metadata = { ...(this.currentCell.metadata as Record<string, unknown>) }; metadata.deepnote_allow_multiple_values = settings.allowMultipleValues; metadata.deepnote_allow_empty_values = settings.allowEmptyValue; metadata.deepnote_variable_select_type = settings.selectType; metadata.deepnote_variable_custom_options = settings.options; metadata.deepnote_variable_selected_variable = settings.selectedVariable; if (settings.selectType === 'from-options') { metadata.deepnote_variable_options = settings.options; } -// Replace the cell with updated metadata -const cellData = { - kind: this.currentCell.kind, - languageId: this.currentCell.document.languageId, - value: this.currentCell.document.getText(), - metadata -}; - edit.set(this.currentCell.notebook.uri, [ - NotebookEdit.replaceCells(new NotebookRange(this.currentCell.index, this.currentCell.index + 1), [cellData]) + NotebookEdit.updateCellMetadata(this.currentCell.index, metadata) ]);
228-265: Fix ESM loading, remove non-existent CSS, localize title.
- Bundle is ESM; add
type="module"to script tag- CSS is injected via bundle; remove external stylesheet link
- Localize the HTML title
+import { l10n } from 'vscode'; + private getWebviewContent(): string { // ... - const styleUri = webview.asWebviewUri( - Uri.joinPath( - this.extensionContext.extensionUri, - 'dist', - 'webviews', - 'webview-side', - 'selectInputSettings', - 'selectInputSettings.css' - ) - ); const codiconUri = webview.asWebviewUri( Uri.joinPath( this.extensionContext.extensionUri, 'dist', 'webviews', 'webview-side', 'react-common', 'codicon', 'codicon.css' ) ); + const title = l10n.t('Select Input Settings'); return `<!DOCTYPE html> <html lang="en"> <head> <meta charset="UTF-8"> <meta name="viewport" content="width=device-width, initial-scale=1.0"> <meta http-equiv="Content-Security-Policy" content="default-src 'none'; style-src ${webview.cspSource}; script-src 'nonce-${nonce}'; font-src ${webview.cspSource};"> <link rel="stylesheet" href="${codiconUri}"> - <link rel="stylesheet" href="${styleUri}"> - <title>Select Input Settings</title> + <title>${title}</title> </head> <body> <div id="root"></div> - <script nonce="${nonce}" src="${scriptUri}"></script> + <script nonce="${nonce}" type="module" src="${scriptUri}"></script> </body> </html>`; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
src/messageTypes.ts(1 hunks)src/notebooks/deepnote/selectInputSettingsWebview.ts(1 hunks)src/platform/common/utils/localize.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/!(*.node|*.web).ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place shared cross-platform logic in common
.tsfiles (not suffixed with.nodeor.web)
Files:
src/notebooks/deepnote/selectInputSettingsWebview.tssrc/platform/common/utils/localize.tssrc/messageTypes.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Inject interfaces, not concrete classes
Avoid circular dependencies
Usel10n.t()for user-facing strings
Use typed error classes fromsrc/platform/errors/when throwing or handling errors
Use theILoggerservice instead ofconsole.log
Preserve error details while scrubbing PII in messages and telemetry
Include the Microsoft copyright header in source files
Prefer async/await and handle cancellation withCancellationToken
**/*.{ts,tsx}: Order class members (methods, fields, properties) first by accessibility (public/protected/private) and then alphabetically
Do not add the Microsoft copyright header to new files
Use Uri.joinPath() to construct file paths instead of manual string concatenation
Add a blank line after groups of const declarations and before return statements for readability
Separate third-party imports from local file imports
Files:
src/notebooks/deepnote/selectInputSettingsWebview.tssrc/platform/common/utils/localize.tssrc/messageTypes.ts
src/notebooks/deepnote/**
📄 CodeRabbit inference engine (CLAUDE.md)
Deepnote integration code resides under src/notebooks/deepnote/
Files:
src/notebooks/deepnote/selectInputSettingsWebview.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/utils/localize.ts
🧠 Learnings (1)
📚 Learning: 2025-10-09T11:21:57.494Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-09T11:21:57.494Z
Learning: Applies to src/notebooks/deepnote/deepnoteNotebookSelector.ts : deepnoteNotebookSelector.ts implements UI selection logic for Deepnote notebooks
Applied to files:
src/notebooks/deepnote/selectInputSettingsWebview.ts
🧬 Code graph analysis (1)
src/platform/common/utils/localize.ts (1)
src/webviews/webview-side/selectInputSettings/types.ts (1)
SelectInputSettings(4-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). (2)
- GitHub Check: Build & Test
- GitHub Check: Build & Package Extension
🔇 Additional comments (2)
src/platform/common/utils/localize.ts (1)
862-877: LGTM!The new SelectInputSettings namespace follows the established pattern and properly localizes all UI strings.
src/messageTypes.ts (1)
207-220: LGTM!The new localization keys properly extend LocalizedMessages to support the select input settings UI.
|
Crazy stuff 😍 |
There was a problem hiding this comment.
- Add commands don't work as they're supposed to, e.g. "Deepnote: Add Input Date Range Block" adds: (screenshot below)
- from your example file, the following input types dont "run" (and therefore are unavailable as variables in subsequent blocks):
- text input
- text area input
- input slider
- input file
jankuca
force-pushed
the
jk/feat/input-block-ux
branch
from
9809d53 to
e5bdfa7
Compare
|
@saltenasl Much thanks for testing. The commands were added after the core of this logic, updated them, they should now work correctly. The execution now works for all the block types, I confirmed the variables are being populated. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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 selected for processing (3)
src/notebooks/deepnote/deepnoteNotebookCommandListener.ts(2 hunks)src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts(3 hunks)src/platform/common/constants.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/!(*.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/deepnoteNotebookCommandListener.unit.test.tssrc/notebooks/deepnote/deepnoteNotebookCommandListener.tssrc/platform/common/constants.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Inject interfaces, not concrete classes
Avoid circular dependencies
Usel10n.t()for user-facing strings
Use typed error classes fromsrc/platform/errors/when throwing or handling errors
Use theILoggerservice instead ofconsole.log
Preserve error details while scrubbing PII in messages and telemetry
Include the Microsoft copyright header in source files
Prefer async/await and handle cancellation withCancellationToken
**/*.{ts,tsx}: Order class members (methods, fields, properties) first by accessibility (public/protected/private) and then alphabetically
Do not add the Microsoft copyright header to new files
Use Uri.joinPath() to construct file paths instead of manual string concatenation
Add a blank line after groups of const declarations and before return statements for readability
Separate third-party imports from local file imports
Files:
src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.tssrc/notebooks/deepnote/deepnoteNotebookCommandListener.tssrc/platform/common/constants.ts
**/*.unit.test.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place unit tests in files matching
*.unit.test.ts
**/*.unit.test.ts: Unit tests must use Mocha/Chai and have the .unit.test.ts extension
Place unit test files alongside the source files they test
Use assert.deepStrictEqual() for object comparisons in tests instead of checking individual properties
Files:
src/notebooks/deepnote/deepnoteNotebookCommandListener.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/deepnoteNotebookCommandListener.unit.test.ts
src/notebooks/deepnote/**
📄 CodeRabbit inference engine (CLAUDE.md)
Deepnote integration code resides under src/notebooks/deepnote/
Files:
src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.tssrc/notebooks/deepnote/deepnoteNotebookCommandListener.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
🧬 Code graph analysis (2)
src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts (1)
src/notebooks/deepnote/inputBlockContentFormatter.ts (2)
getInputBlockLanguage(10-23)formatInputBlockCellContent(31-110)
src/notebooks/deepnote/deepnoteNotebookCommandListener.ts (3)
src/kernels/execution/notebookUpdater.ts (1)
chainWithPendingUpdates(24-52)src/notebooks/deepnote/inputBlockContentFormatter.ts (2)
formatInputBlockCellContent(31-110)getInputBlockLanguage(10-23)src/test/mocks/vsc/extHostedTypes.ts (1)
NotebookCellData(2546-2572)
🔇 Additional comments (6)
src/platform/common/constants.ts (1)
129-129: LGTM!Plaintext addition aligns with input-block language support. Comment is clear.
src/notebooks/deepnote/deepnoteNotebookCommandListener.ts (2)
21-21: LGTM!New formatter utilities properly imported for centralized input-block formatting.
351-356: LGTM!Dynamic content formatting and language selection properly implemented. The fallback to
'python'on line 354 appears unreachable sincegetInputBlockLanguagereturns values for allInputBlockTypecases, but it's good defensive coding.src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts (3)
21-21: LGTM!Formatter utilities imported to verify dynamic content and language behavior.
612-618: LGTM!Test assertions correctly updated to verify dynamic language selection and formatted content generation. Passing
newCell.metadatatoformatInputBlockCellContentis correct since top-level keys are accessible after the spread.
634-642: LGTM!Top-level metadata verification added to match the implementation's metadata structure. Complements existing
__deepnotePocketassertions.
|
@coderabbitai resolve |
✅ Actions performedComments resolved and changes approved. |
inputs.mov
This notebook can be used for testing:
Input-types.deepnote.zip
Summary by CodeRabbit
New Features
Tests
Chores