feat: Add a command for adding chart blocks #137

Artmann · 2025-10-30T12:01:01Z

Summary by CodeRabbit

New Features
- Added "Add Chart Block" command to create chart blocks in Deepnote notebooks. Blocks include a default Vega‑Lite spec for quick visualization and are available from the command palette and Deepnote notebook toolbar.
Updates
- Renamed "Add Big Number Chart Block" to "Add Big Number Block" for clearer labeling.

coderabbitai · 2025-10-30T12:01:30Z

📝 Walkthrough

Walkthrough

This PR adds a new Deepnote command deepnote.addChartBlock with UI metadata and localization, updates command typings and constants, extends variable-name parsing/generation to accept a chart prefix and to derive names from either a chart-specific key or deepnote_variable_name, and implements DeepnoteNotebookCommandListener.addChartBlock() to insert a chart cell with a default Vega‑Lite spec, assign a variable name, place the cell at the computed index, reveal it and enter edit mode. Unit tests for chart insertion and error paths were added.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant VSCode as "VS Code (Palette / Toolbar)"
  participant Listener as "DeepnoteNotebookCommandListener"
  participant VarGen as "getNextDeepnoteVariableName(prefix: chart)"
  participant Notebook as "Notebook API"

  rect rgba(200,240,255,0.4)
    User->>VSCode: Trigger `deepnote.addChartBlock`
    VSCode->>Listener: execute addChartBlock()
  end

  rect rgba(230,255,220,0.4)
    Listener->>VarGen: request next variable name
    VarGen-->>Listener: returns variable name (e.g., chart_1)
    Listener->>Notebook: chainWithPendingUpdates -> insert chart cell (Vega‑Lite JSON, variable, metadata) at index
    Notebook-->>Listener: insertion confirmed
    Listener->>Notebook: revealRange & execute edit command
    Notebook-->>Listener: cell revealed & edit mode active
  end

Possibly related PRs

feat: Add commands to create Deepnote block cells #75: Modifies Deepnote notebook command infrastructure and adds block-creation commands—overlaps command registration, listener logic, and tests.

Suggested reviewers

jamesbhobbs
jankuca
saltenasl

Pre-merge checks

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title "feat: Add a command for adding chart blocks" accurately captures the primary objective of this pull request. The entire changeset is cohesively focused on implementing a single feature: a new command for inserting chart blocks into Deepnote notebooks. The changes span across multiple files (constants, commands, localization, implementation, and tests), all serving this unified purpose. The title is specific and clear—it avoids vague language like "misc updates" and directly identifies what's being added.

📜 Recent 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 6b3b581 and 42f666e.

📒 Files selected for processing (2)

src/notebooks/deepnote/deepnoteNotebookCommandListener.ts (4 hunks)
src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts (2 hunks)

🧰 Additional context used

📓 Path-based instructions (5)

**/!(*.node|*.web).ts

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Place shared cross-platform logic in common .ts files (not suffixed with .node or .web)

Files:

src/notebooks/deepnote/deepnoteNotebookCommandListener.ts
src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts

**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Inject interfaces, not concrete classes
Avoid circular dependencies
Use l10n.t() for user-facing strings
Use typed error classes from src/platform/errors/ when throwing or handling errors
Use the ILogger service instead of console.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 with CancellationToken

**/*.{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.ts
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.ts
src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.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 then property to avoid hanging tests

Files:

src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts

🧠 Learnings (2)

📚 Learning: 2025-10-30T12:09:07.473Z

Learnt from: Artmann
PR: deepnote/vscode-deepnote#137
File: src/notebooks/deepnote/deepnoteNotebookCommandListener.ts:278-305
Timestamp: 2025-10-30T12:09:07.473Z
Learning: In `src/notebooks/deepnote/deepnoteNotebookCommandListener.ts`, chart blocks created by `addChartBlock()` should use the 'df' prefix when calling `getNextDeepnoteVariableName()`, not the 'chart' prefix, even though 'chart' is available as an option in the function signature.

Applied to files:

src/notebooks/deepnote/deepnoteNotebookCommandListener.ts
src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts

📚 Learning: 2025-10-30T12:27:22.797Z

Learnt from: Artmann
PR: deepnote/vscode-deepnote#137
File: src/notebooks/deepnote/deepnoteNotebookCommandListener.ts:290-294
Timestamp: 2025-10-30T12:27:22.797Z
Learning: In the `addChartBlock()` method in `src/notebooks/deepnote/deepnoteNotebookCommandListener.ts`, the `variable: 'df_1'` is intentionally hardcoded, unlike `addSqlBlock()` which generates unique variable names using `getNextDeepnoteVariableName()`.

Applied to files:

src/notebooks/deepnote/deepnoteNotebookCommandListener.ts

🧬 Code graph analysis (2)

src/notebooks/deepnote/deepnoteNotebookCommandListener.ts (3)

src/platform/errors/types.ts (1)

WrappedError (23-57)

src/kernels/execution/notebookUpdater.ts (1)

chainWithPendingUpdates (24-52)

src/test/mocks/vsc/extHostedTypes.ts (3)

NotebookCellData (2546-2572)

NotebookEdit (2467-2530)

NotebookRange (2436-2466)

src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts (2)

src/test/mocks/vsc/extHostedTypes.ts (1)

NotebookRange (2436-2466)

src/platform/errors/types.ts (1)

WrappedError (23-57)

⏰ 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 (3)

src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts (1)

980-1165: Comprehensive test coverage for addChartBlock.

Tests cover insertion scenarios, hardcoded df_1 behavior (intentional per learnings), Vega-Lite spec structure, and error paths. Well done.

src/notebooks/deepnote/deepnoteNotebookCommandListener.ts (2)

80-83: Dual-key support correctly implemented.

Chart blocks use variable, other blocks use deepnote_variable_name. Nullish coalescing handles both cleanly.

270-323: Implementation follows established patterns.

Uses WrappedError, includes blank lines after const groups, follows l10n and async/await conventions. Hardcoded df_1 is intentional per learnings.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codecov · 2025-10-30T12:02:42Z

Codecov Report

❌ Patch coverage is 99.11504% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 72%. Comparing base (87e8346) to head (42f666e).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...ebooks/deepnote/deepnoteNotebookCommandListener.ts	96%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@          Coverage Diff           @@
##            main    #137    +/-   ##
======================================
  Coverage     72%     72%            
======================================
  Files        541     541            
  Lines      41483   41595   +112     
  Branches    5027    5031     +4     
======================================
+ Hits       30211   30322   +111     
  Misses      9593    9593            
- Partials    1679    1680     +1

Files with missing lines	Coverage Δ
...pnote/deepnoteNotebookCommandListener.unit.test.ts	`99% <100%> (+<1%)`	⬆️
...ebooks/deepnote/deepnoteNotebookCommandListener.ts	`85% <96%> (+1%)`	⬆️

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

coderabbitai

Actionable comments posted: 2

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 065b4c0 and a7cb1ca.

📒 Files selected for processing (6)

package.json (2 hunks)
package.nls.json (1 hunks)
src/commands.ts (1 hunks)
src/notebooks/deepnote/deepnoteNotebookCommandListener.ts (3 hunks)
src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts (2 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 .ts files (not suffixed with .node or .web)

Files:

src/platform/common/constants.ts
src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts
src/notebooks/deepnote/deepnoteNotebookCommandListener.ts
src/commands.ts

**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Inject interfaces, not concrete classes
Avoid circular dependencies
Use l10n.t() for user-facing strings
Use typed error classes from src/platform/errors/ when throwing or handling errors
Use the ILogger service instead of console.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 with CancellationToken

**/*.{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/platform/common/constants.ts
src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts
src/notebooks/deepnote/deepnoteNotebookCommandListener.ts
src/commands.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

**/*.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 then property 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.ts
src/notebooks/deepnote/deepnoteNotebookCommandListener.ts

🧬 Code graph analysis (2)

src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts (1)

src/test/mocks/vsc/extHostedTypes.ts (1)

NotebookRange (2436-2466)

src/notebooks/deepnote/deepnoteNotebookCommandListener.ts (3)

src/test/mocks/vsc/extHostedTypes.ts (4)

error (30-37)

NotebookCellData (2546-2572)

NotebookEdit (2467-2530)

NotebookRange (2436-2466)

src/platform/logging/index.ts (1)

logger (35-48)

src/kernels/execution/notebookUpdater.ts (1)

chainWithPendingUpdates (24-52)

🔇 Additional comments (1)

src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts (1)

979-1159: Comprehensive test suite with good coverage.

The test suite thoroughly validates chart block insertion:

Full structure validation including Vega-Lite spec (lines 1009-1025)

Variable name generation considering both variable and deepnote_variable_name keys (lines 1067-1086, 1088-1108)

Position handling and error cases

Minor suggestion: Line 1107's comment "Should consider all df variables" could clarify it uses the highest df_ number (df_5 → df_6).

src/notebooks/deepnote/deepnoteNotebookCommandListener.ts

src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts

coderabbitai

Actionable comments posted: 3

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between a7cb1ca and 3a757ba.

📒 Files selected for processing (2)

src/notebooks/deepnote/deepnoteNotebookCommandListener.ts (3 hunks)
src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (5)

**/!(*.node|*.web).ts

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Place shared cross-platform logic in common .ts files (not suffixed with .node or .web)

Files:

src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts
src/notebooks/deepnote/deepnoteNotebookCommandListener.ts

**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Inject interfaces, not concrete classes
Avoid circular dependencies
Use l10n.t() for user-facing strings
Use typed error classes from src/platform/errors/ when throwing or handling errors
Use the ILogger service instead of console.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 with CancellationToken

**/*.{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.ts
src/notebooks/deepnote/deepnoteNotebookCommandListener.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 then property 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.ts
src/notebooks/deepnote/deepnoteNotebookCommandListener.ts

🧠 Learnings (1)

📚 Learning: 2025-10-30T12:09:07.473Z

Learnt from: Artmann
PR: deepnote/vscode-deepnote#137
File: src/notebooks/deepnote/deepnoteNotebookCommandListener.ts:278-305
Timestamp: 2025-10-30T12:09:07.473Z
Learning: In `src/notebooks/deepnote/deepnoteNotebookCommandListener.ts`, chart blocks created by `addChartBlock()` should use the 'df' prefix when calling `getNextDeepnoteVariableName()`, not the 'chart' prefix, even though 'chart' is available as an option in the function signature.

Applied to files:

src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts
src/notebooks/deepnote/deepnoteNotebookCommandListener.ts

🧬 Code graph analysis (2)

src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts (1)

src/test/mocks/vsc/extHostedTypes.ts (1)

NotebookRange (2436-2466)

src/notebooks/deepnote/deepnoteNotebookCommandListener.ts (2)

src/kernels/execution/notebookUpdater.ts (1)

chainWithPendingUpdates (24-52)

src/test/mocks/vsc/extHostedTypes.ts (3)

NotebookCellData (2546-2572)

NotebookEdit (2467-2530)

NotebookRange (2436-2466)

🔇 Additional comments (2)

src/notebooks/deepnote/deepnoteNotebookCommandListener.ts (2)

78-81: LGTM—correctly handles both chart and non-chart variable names.

The nullish coalescing supports chart blocks' variable key while maintaining backward compatibility with deepnote_variable_name.

153-153: LGTM—command registration follows the established pattern.

src/notebooks/deepnote/deepnoteNotebookCommandListener.ts

src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts

Resolved conflict in deepnoteNotebookCommandListener.unit.test.ts by keeping both imports: - WrappedError from platform/errors/types (for chart block tests) - DATAFRAME_SQL_INTEGRATION_ID from platform/notebooks/deepnote/integrationTypes (from main)

* feat: Add a command for adding chart blocks. * hardcode the variable name * better errrors

feat: Add a command for adding chart blocks.

a7cb1ca

coderabbitai bot requested changes Oct 30, 2025

View reviewed changes

src/notebooks/deepnote/deepnoteNotebookCommandListener.ts Show resolved Hide resolved

src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts Outdated Show resolved Hide resolved

hardcode the variable name

3a757ba

coderabbitai bot requested changes Oct 30, 2025

View reviewed changes

src/notebooks/deepnote/deepnoteNotebookCommandListener.ts Show resolved Hide resolved

src/notebooks/deepnote/deepnoteNotebookCommandListener.ts Show resolved Hide resolved

src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts Show resolved Hide resolved

Artmann added 2 commits October 30, 2025 13:27

better errrors

6b3b581

coderabbitai bot approved these changes Oct 30, 2025

View reviewed changes

Artmann marked this pull request as ready for review October 30, 2025 13:14

Artmann requested a review from a team as a code owner October 30, 2025 13:14

saltenasl approved these changes Oct 30, 2025

View reviewed changes

saltenasl merged commit c1b5526 into main Oct 30, 2025
12 of 13 checks passed

saltenasl deleted the chris/add-add-chart-command branch October 30, 2025 13:24

dinohamzic pushed a commit that referenced this pull request Oct 30, 2025

feat: Add a command for adding chart blocks (#137)

aef5c89

* feat: Add a command for adding chart blocks. * hardcode the variable name * better errrors

coderabbitai bot mentioned this pull request Oct 30, 2025

feat: loading state on startup #142

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: Add a command for adding chart blocks #137

feat: Add a command for adding chart blocks #137

Uh oh!

Artmann commented Oct 30, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Oct 30, 2025 •

edited

Loading

Walkthrough

Sequence Diagram(s)

Possibly related PRs

Suggested reviewers

Uh oh!

codecov bot commented Oct 30, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

feat: Add a command for adding chart blocks #137

feat: Add a command for adding chart blocks #137

Uh oh!

Conversation

Artmann commented Oct 30, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Sequence Diagram(s)

Possibly related PRs

Suggested reviewers

Pre-merge checks

Uh oh!

codecov bot commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Artmann commented Oct 30, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Oct 30, 2025 •

edited

Loading

codecov bot commented Oct 30, 2025 •

edited

Loading