Skip to content

Conversation

@jankuca
Copy link
Member

@jankuca jankuca commented Oct 23, 2025

Also pre-fills name based on the project integration list

Summary by CodeRabbit

  • New Features

    • Shows integration type labels (Postgres, BigQuery) alongside names across lists, items and forms.
    • Integration forms accept an optional integration name and prefill it when available.
    • Saving or deleting integrations updates the project’s integration list and refreshes the UI.
    • SQL cell picker now lists project-scoped integrations with a clearer "Configure" option.
  • Refactor

    • Switched to a project-level integrations model for discovery, status and UI state.
  • Tests

    • Added and updated unit tests for project-level integration updates and edge cases.

@codecov
Copy link

codecov bot commented Oct 23, 2025

Codecov Report

❌ Patch coverage is 98.30508% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 72%. Comparing base (5c3d0f1) to head (5a91e78).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/notebooks/deepnote/sqlCellStatusBarProvider.ts 88% 0 Missing and 3 partials ⚠️
Additional details and impacted files
@@          Coverage Diff           @@
##            main    #109    +/-   ##
======================================
  Coverage     72%     72%            
======================================
  Files        536     536            
  Lines      40681   40829   +148     
  Branches    4982    4990     +8     
======================================
+ Hits       29442   29588   +146     
  Misses      9578    9578            
- Partials    1661    1663     +2     
Files with missing lines Coverage Δ
src/messageTypes.ts 100% <ø> (ø)
src/notebooks/deepnote/deepnoteNotebookManager.ts 92% <100%> (+5%) ⬆️
...ooks/deepnote/deepnoteNotebookManager.unit.test.ts 100% <100%> (ø)
src/notebooks/deepnote/integrations/types.ts 100% <ø> (ø)
...oks/deepnote/sqlCellStatusBarProvider.unit.test.ts 99% <100%> (+<1%) ⬆️
src/notebooks/types.ts 100% <ø> (ø)
src/platform/common/utils/localize.ts 87% <100%> (+<1%) ⬆️
...rc/platform/notebooks/deepnote/integrationTypes.ts 100% <100%> (ø)
src/notebooks/deepnote/sqlCellStatusBarProvider.ts 78% <88%> (+1%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 23, 2025

📝 Walkthrough

Walkthrough

This PR shifts Deepnote integrations to a project-scoped model and threads projectId through the integration UI and tooling. detectIntegrations now reads from project.project.integrations and returns IntegrationWithStatus entries including optional integrationName and integrationType. The integration webview show signature changed to show(projectId, ...); the webview calls IDeepnoteNotebookManager.updateProjectIntegrations after save/delete to persist project.integrations. New mappings convert between Deepnote type strings and internal IntegrationType. Localization keys for PostgreSQL and BigQuery were added and included in webview payloads.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Webview
    participant Provider as IntegrationWebviewProvider
    participant NMgr as DeepnoteNotebookManager
    participant Storage

    rect rgb(220,240,220)
    Note over User,Webview: User saves or deletes an integration
    User->>Webview: submit save/delete
    Webview->>Provider: postMessage(save/delete)
    Provider->>Provider: updateProjectIntegrationsList()
    Provider->>NMgr: updateProjectIntegrations(projectId, integrations)
    NMgr->>Storage: persist project.integrations
    Storage-->>NMgr: ack
    Provider->>Webview: postMessage(updated integrations payload)
    end

    rect rgb(240,230,210)
    Note over Provider,NMgr: UI discovery / listing
    Provider->>NMgr: getOriginalProject(projectId)
    NMgr-->>Provider: project (with project.integrations[])
    Provider->>Provider: map -> IntegrationWithStatus (config/status/name/type)
    Provider->>Webview: send integrations payload (includes localized labels)
    end
Loading

Possibly related PRs

Suggested reviewers

  • andyjakubowski
  • saltenasl
  • Artmann

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: Use integration listing from project, not from blocks" directly reflects the primary architectural change in this changeset. The raw summary confirms that integrationDetector.ts replaced block-scanning logic with project-integrations-based approach, and supporting infrastructure (updateProjectIntegrations method, webview updates, etc.) enables storing and retrieving integrations at the project level rather than from blocks. The title is specific, concise, and clearly communicates the main change without vagueness or unnecessary details.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c3eb62a and 4126aaf.

📒 Files selected for processing (2)
  • src/notebooks/deepnote/sqlCellStatusBarProvider.ts (5 hunks)
  • src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts (16 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/sqlCellStatusBarProvider.unit.test.ts
  • src/notebooks/deepnote/sqlCellStatusBarProvider.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/sqlCellStatusBarProvider.unit.test.ts
  • src/notebooks/deepnote/sqlCellStatusBarProvider.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/sqlCellStatusBarProvider.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/sqlCellStatusBarProvider.unit.test.ts
src/notebooks/deepnote/**

📄 CodeRabbit inference engine (CLAUDE.md)

Deepnote integration code resides under src/notebooks/deepnote/

Files:

  • src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts
  • src/notebooks/deepnote/sqlCellStatusBarProvider.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/deepnoteActivationService.ts : deepnoteActivationService.ts manages VS Code activation for Deepnote

Applied to files:

  • src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts
🧬 Code graph analysis (1)
src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts (2)
src/platform/notebooks/deepnote/types.ts (2)
  • IIntegrationStorage (6-6)
  • IIntegrationStorage (7-42)
src/notebooks/types.ts (2)
  • IDeepnoteNotebookManager (37-37)
  • IDeepnoteNotebookManager (38-58)
⏰ 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). (3)
  • GitHub Check: Build & Package Extension
  • GitHub Check: Build & Test
  • GitHub Check: Lint & Format
🔇 Additional comments (8)
src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts (4)

21-21: LGTM: Clean dependency injection.

The IDeepnoteNotebookManager integration is correctly added and consistently used throughout the test file.

Also applies to: 27-27, 33-34


161-235: Excellent test coverage for project integration scenarios.

These two new tests properly verify:

  • "Unknown integration (configure)" when not in project list
  • Integration name from project with "(configure)" suffix when config missing but integration exists

Both scenarios are critical for the new project-based integration flow.


301-312: Consistent test suite updates.

All test suites correctly instantiate the provider with the three-parameter constructor and mock notebookManager.

Also applies to: 356-366, 416-428, 543-555


577-815: Comprehensive coverage of switchIntegration with project-based data.

The updated and new tests thoroughly exercise:

  • Standard integration switching with project data
  • Missing project ID and missing project error paths (lines 751-770)
  • Filtering of DATAFRAME_SQL_INTEGRATION_ID from project integrations while retaining DuckDB as separate entry (lines 772-815)

The filtering test is particularly valuable—it explicitly verifies the implementation skips DATAFRAME_SQL_INTEGRATION_ID from the project list but still presents DuckDB in the picker.

src/notebooks/deepnote/sqlCellStatusBarProvider.ts (4)

25-31: LGTM: Proper dependency injection.

The IDeepnoteNotebookManager dependency is correctly injected and the imports include all necessary types for project-based integration handling.

Also applies to: 51-52


205-217: Well-structured display name fallback logic.

The three-tier fallback correctly handles:

  1. Configured integrations (use config.name)
  2. Integrations in project but not configured (use projectIntegration.name with "(configure)" suffix)
  3. Unknown integrations (fallback to "Unknown integration (configure)")

Optional chaining ensures safety when project or integrations list is unavailable.


307-319: Appropriate error handling for missing project context.

The validation ensures both projectId and project exist before proceeding, with clear error messages for users.


324-360: Correct integration list building with proper filtering.

The implementation:

  • Retrieves integrations from project.project.integrations with empty array fallback
  • Skips DATAFRAME_SQL_INTEGRATION_ID from project integrations (lines 346-348) since DuckDB is added separately as a special case (lines 363-369)
  • Maps integration types using DEEPNOTE_TO_INTEGRATION_TYPE with safe fallback to raw type string (lines 350-351)
  • Properly identifies unknown integrations (line 330)

This matches the test expectations and handles all edge cases.


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

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8597009 and 9e86485.

📒 Files selected for processing (18)
  • src/messageTypes.ts (1 hunks)
  • src/notebooks/deepnote/deepnoteNotebookManager.ts (1 hunks)
  • src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts (1 hunks)
  • src/notebooks/deepnote/integrations/integrationDetector.ts (3 hunks)
  • src/notebooks/deepnote/integrations/integrationManager.ts (1 hunks)
  • src/notebooks/deepnote/integrations/integrationWebview.ts (8 hunks)
  • src/notebooks/deepnote/integrations/types.ts (1 hunks)
  • src/notebooks/deepnote/sqlCellStatusBarProvider.ts (4 hunks)
  • src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts (13 hunks)
  • src/notebooks/types.ts (1 hunks)
  • src/platform/common/utils/localize.ts (1 hunks)
  • src/platform/notebooks/deepnote/integrationTypes.ts (2 hunks)
  • src/webviews/webview-side/integrations/BigQueryForm.tsx (1 hunks)
  • src/webviews/webview-side/integrations/ConfigurationForm.tsx (3 hunks)
  • src/webviews/webview-side/integrations/IntegrationItem.tsx (2 hunks)
  • src/webviews/webview-side/integrations/IntegrationPanel.tsx (4 hunks)
  • src/webviews/webview-side/integrations/PostgresForm.tsx (2 hunks)
  • src/webviews/webview-side/integrations/types.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/!(*.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/deepnoteNotebookManager.ts
  • src/platform/common/utils/localize.ts
  • src/messageTypes.ts
  • src/webviews/webview-side/integrations/types.ts
  • src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts
  • src/notebooks/types.ts
  • src/notebooks/deepnote/integrations/integrationDetector.ts
  • src/notebooks/deepnote/integrations/types.ts
  • src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts
  • src/platform/notebooks/deepnote/integrationTypes.ts
  • src/notebooks/deepnote/integrations/integrationManager.ts
  • src/notebooks/deepnote/sqlCellStatusBarProvider.ts
  • src/notebooks/deepnote/integrations/integrationWebview.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/deepnoteNotebookManager.ts
  • src/platform/common/utils/localize.ts
  • src/messageTypes.ts
  • src/webviews/webview-side/integrations/types.ts
  • src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts
  • src/webviews/webview-side/integrations/IntegrationItem.tsx
  • src/notebooks/types.ts
  • src/notebooks/deepnote/integrations/integrationDetector.ts
  • src/notebooks/deepnote/integrations/types.ts
  • src/webviews/webview-side/integrations/BigQueryForm.tsx
  • src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts
  • src/webviews/webview-side/integrations/PostgresForm.tsx
  • src/platform/notebooks/deepnote/integrationTypes.ts
  • src/webviews/webview-side/integrations/IntegrationPanel.tsx
  • src/notebooks/deepnote/integrations/integrationManager.ts
  • src/notebooks/deepnote/sqlCellStatusBarProvider.ts
  • src/webviews/webview-side/integrations/ConfigurationForm.tsx
  • src/notebooks/deepnote/integrations/integrationWebview.ts
src/notebooks/deepnote/**

📄 CodeRabbit inference engine (CLAUDE.md)

Deepnote integration code resides under src/notebooks/deepnote/

Files:

  • src/notebooks/deepnote/deepnoteNotebookManager.ts
  • src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts
  • src/notebooks/deepnote/integrations/integrationDetector.ts
  • src/notebooks/deepnote/integrations/types.ts
  • src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts
  • src/notebooks/deepnote/integrations/integrationManager.ts
  • src/notebooks/deepnote/sqlCellStatusBarProvider.ts
  • src/notebooks/deepnote/integrations/integrationWebview.ts
src/notebooks/deepnote/deepnoteNotebookManager.ts

📄 CodeRabbit inference engine (CLAUDE.md)

deepnoteNotebookManager.ts handles Deepnote notebook state management

Files:

  • src/notebooks/deepnote/deepnoteNotebookManager.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
  • src/platform/notebooks/deepnote/integrationTypes.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/deepnoteNotebookManager.unit.test.ts
  • src/notebooks/deepnote/sqlCellStatusBarProvider.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/deepnoteNotebookManager.unit.test.ts
  • src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts
🧠 Learnings (3)
📚 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/sqlCellStatusBarProvider.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/deepnoteTypes.ts : deepnoteTypes.ts contains Deepnote-related type definitions

Applied to files:

  • src/platform/notebooks/deepnote/integrationTypes.ts
  • src/notebooks/deepnote/integrations/integrationWebview.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/sqlCellStatusBarProvider.ts
  • src/notebooks/deepnote/integrations/integrationWebview.ts
🧬 Code graph analysis (12)
src/notebooks/deepnote/deepnoteNotebookManager.ts (2)
src/notebooks/deepnote/deepnoteSerializer.ts (1)
  • DeepnoteProject (10-10)
src/notebooks/deepnote/deepnoteTypes.ts (1)
  • DeepnoteProject (12-12)
src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts (2)
src/notebooks/deepnote/deepnoteSerializer.ts (1)
  • DeepnoteProject (10-10)
src/notebooks/deepnote/deepnoteTypes.ts (1)
  • DeepnoteProject (12-12)
src/webviews/webview-side/integrations/IntegrationItem.tsx (2)
src/webviews/webview-side/integrations/types.ts (2)
  • IntegrationWithStatus (29-35)
  • IntegrationType (1-1)
src/webviews/webview-side/react-common/locReactSide.ts (1)
  • getLocString (11-17)
src/notebooks/deepnote/integrations/integrationDetector.ts (1)
src/platform/notebooks/deepnote/integrationTypes.ts (3)
  • IntegrationWithStatus (87-99)
  • DATAFRAME_SQL_INTEGRATION_ID (5-5)
  • mapDeepnoteIntegrationType (23-25)
src/notebooks/deepnote/integrations/types.ts (2)
src/platform/notebooks/deepnote/integrationTypes.ts (1)
  • IntegrationWithStatus (87-99)
src/webviews/webview-side/integrations/types.ts (1)
  • IntegrationWithStatus (29-35)
src/webviews/webview-side/integrations/BigQueryForm.tsx (2)
src/platform/notebooks/deepnote/integrationTypes.ts (1)
  • BigQueryIntegrationConfig (64-68)
src/webviews/webview-side/integrations/types.ts (1)
  • BigQueryIntegrationConfig (21-25)
src/webviews/webview-side/integrations/PostgresForm.tsx (2)
src/platform/notebooks/deepnote/integrationTypes.ts (1)
  • PostgresIntegrationConfig (51-59)
src/webviews/webview-side/integrations/types.ts (1)
  • PostgresIntegrationConfig (11-19)
src/platform/notebooks/deepnote/integrationTypes.ts (1)
src/webviews/webview-side/integrations/types.ts (1)
  • IntegrationType (1-1)
src/webviews/webview-side/integrations/IntegrationPanel.tsx (1)
src/webviews/webview-side/integrations/types.ts (1)
  • IntegrationType (1-1)
src/notebooks/deepnote/sqlCellStatusBarProvider.ts (2)
src/notebooks/types.ts (2)
  • IDeepnoteNotebookManager (28-28)
  • IDeepnoteNotebookManager (29-39)
src/platform/notebooks/deepnote/integrationTypes.ts (2)
  • DATAFRAME_SQL_INTEGRATION_ID (5-5)
  • mapDeepnoteIntegrationType (23-25)
src/webviews/webview-side/integrations/ConfigurationForm.tsx (4)
src/platform/notebooks/deepnote/integrationTypes.ts (1)
  • IntegrationConfig (73-73)
src/webviews/webview-side/integrations/types.ts (2)
  • IntegrationConfig (27-27)
  • IntegrationType (1-1)
src/webviews/webview-side/integrations/PostgresForm.tsx (1)
  • PostgresForm (13-183)
src/webviews/webview-side/integrations/BigQueryForm.tsx (1)
  • BigQueryForm (13-156)
src/notebooks/deepnote/integrations/integrationWebview.ts (4)
src/platform/notebooks/deepnote/types.ts (2)
  • IIntegrationStorage (6-6)
  • IIntegrationStorage (7-42)
src/notebooks/types.ts (2)
  • IDeepnoteNotebookManager (28-28)
  • IDeepnoteNotebookManager (29-39)
src/platform/notebooks/deepnote/integrationTypes.ts (2)
  • IntegrationWithStatus (87-99)
  • mapToDeepnoteIntegrationType (35-37)
src/webviews/webview-side/integrations/types.ts (1)
  • IntegrationWithStatus (29-35)
⏰ 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). (3)
  • GitHub Check: Build & Test
  • GitHub Check: Build & Package Extension
  • GitHub Check: Lint & Format
🔇 Additional comments (46)
src/platform/common/utils/localize.ts (1)

834-836: LGTM!

Localization constants follow existing patterns and provide clear integration type labels.

src/messageTypes.ts (1)

182-184: LGTM!

Type additions align with new localization constants.

src/webviews/webview-side/integrations/BigQueryForm.tsx (1)

8-8: LGTM!

Integration name pre-fill follows clean fallback pattern and correctly handles form reset on prop changes.

Also applies to: 13-19, 20-20, 25-38

src/webviews/webview-side/integrations/IntegrationItem.tsx (2)

11-20: Good type label mapping.

The default case returns the raw type value. This provides a reasonable fallback if new integration types are added before UI labels are defined.


32-39: LGTM!

Display name composition uses sensible fallback chain and cleanly integrates type labels.

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

186-261: LGTM!

Test suite is comprehensive, covering update scenarios, edge cases, and property preservation. Correctly uses assert.deepStrictEqual() per coding guidelines.

src/webviews/webview-side/integrations/types.ts (1)

33-34: LGTM!

Optional fields support project-level integration metadata propagation without breaking existing code.

Also applies to: 52-53

src/notebooks/deepnote/integrations/types.ts (1)

23-31: LGTM!

API signature cleanly extended to accept projectId. Documentation is clear.

src/webviews/webview-side/integrations/PostgresForm.tsx (3)

8-8: LGTM!

Optional integrationName prop added to support pre-filling from project-level integration metadata.


13-20: LGTM!

Form initialization correctly prioritizes existingConfig.name, then integrationName, then empty string.


28-47: LGTM!

Reset logic and useEffect dependencies correctly handle integrationName updates.

src/notebooks/deepnote/integrations/integrationManager.ts (1)

165-165: LGTM!

Call site updated to match new show() signature with projectId.

src/notebooks/deepnote/integrations/integrationDetector.ts (2)

5-10: LGTM!

Imports updated to support type mapping and filtering.


27-74: No changes needed. Unmapped integration types are handled correctly.

The code properly handles undefined integrationType values:

  • The field is correctly marked optional (integrationType?: IntegrationType) in IntegrationWithStatus
  • All downstream consumers check for undefined before use: IntegrationItem uses fallback patterns, ConfigurationForm checks truthiness, and IntegrationWebview logs warnings when type is missing
  • Fallback strategies exist everywhere (config type, original Deepnote string, null handling)
src/webviews/webview-side/integrations/IntegrationPanel.tsx (4)

6-6: LGTM!

IntegrationType import added to support type metadata.


17-20: LGTM!

State tracking for integration metadata correctly initialized.


58-59: LGTM!

Integration metadata captured from showForm message.


153-154: LGTM!

Integration metadata properly forwarded to ConfigurationForm.

src/platform/notebooks/deepnote/integrationTypes.ts (2)

15-25: LGTM!

Deepnote-to-internal type mapping correctly handles known types and returns undefined for unknown.


91-98: LGTM!

Optional fields for pre-filling integration forms are well-documented.

src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts (5)

21-21: LGTM!

IDeepnoteNotebookManager imported to support project-level integration access.


27-34: LGTM!

Mock notebookManager correctly injected into provider.


530-557: LGTM!

Test updated to use getOriginalProject for integration source.


705-724: LGTM!

Good coverage for missing project ID error case.


726-769: LGTM!

Excellent test coverage for DATAFRAME_SQL_INTEGRATION_ID filtering behavior.

src/webviews/webview-side/integrations/ConfigurationForm.tsx (4)

5-5: LGTM!

IntegrationType imported for type detection.


10-11: LGTM!

Props interface extended to support project-level integration metadata.


24-44: LGTM!

Type detection correctly prioritizes integrationType prop over ID-based inference. Good use of selectedIntegrationType to avoid shadowing.


62-78: LGTM!

integrationName properly forwarded to both PostgresForm and BigQueryForm.

src/notebooks/deepnote/sqlCellStatusBarProvider.ts (7)

25-30: LGTM!

Imports properly organized with third-party separated from local modules.


48-52: LGTM!

Dependency injection follows guidelines—injecting interface, not concrete class.


294-306: LGTM!

Proper null-checking with early returns and localized error messages.


349-372: LGTM!

DuckDB item and configure option properly added with appropriate conditionals.


374-419: LGTM!

Selection handling, metadata updates, and error handling all correct.


421-430: All IntegrationType values are handled.

The enum contains only Postgres and BigQuery. Both have localized labels in the method.


311-311: Add defensive check for project.project before accessing integrations.

Line 311 accesses project.project.integrations but only guards against falsy project. If project.project is undefined, this throws. Add a check: project?.project?.integrations || [] or verify that DeepnoteFile always has a non-null project property (currently unverifiable—the type is from external package @deepnote/blocks).

src/notebooks/deepnote/integrations/integrationWebview.ts (10)

8-8: LGTM!

New imports properly added for project integration management.

Also applies to: 13-14


28-28: LGTM!

Field properly typed as optional.


30-34: LGTM!

Interface injection follows guidelines.


42-48: LGTM!

Method signature updated to track projectId, properly stored before use.


131-132: LGTM!

Type label localization strings properly added.


177-178: LGTM!

Integration name and type properly included in webview payload.


228-229: LGTM!

Form message enhanced with name and type for better UX.


249-250: LGTM!

Project integrations list updated after save, properly awaited.


284-285: LGTM!

Project integrations list updated after delete, consistent with save flow.


304-344: LGTM!

Project sync logic properly handles missing types and unmappable values with warnings. Logging uses ILogger service as required.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9e86485 and ecb0a82.

📒 Files selected for processing (5)
  • cspell.json (2 hunks)
  • src/notebooks/deepnote/integrations/integrationDetector.ts (3 hunks)
  • src/notebooks/deepnote/integrations/integrationWebview.ts (8 hunks)
  • src/notebooks/deepnote/sqlCellStatusBarProvider.ts (4 hunks)
  • src/platform/notebooks/deepnote/integrationTypes.ts (2 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 .ts files (not suffixed with .node or .web)

Files:

  • src/platform/notebooks/deepnote/integrationTypes.ts
  • src/notebooks/deepnote/integrations/integrationDetector.ts
  • src/notebooks/deepnote/sqlCellStatusBarProvider.ts
  • src/notebooks/deepnote/integrations/integrationWebview.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/notebooks/deepnote/integrationTypes.ts
  • src/notebooks/deepnote/integrations/integrationDetector.ts
  • src/notebooks/deepnote/sqlCellStatusBarProvider.ts
  • src/notebooks/deepnote/integrations/integrationWebview.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/notebooks/deepnote/integrationTypes.ts
src/notebooks/deepnote/**

📄 CodeRabbit inference engine (CLAUDE.md)

Deepnote integration code resides under src/notebooks/deepnote/

Files:

  • src/notebooks/deepnote/integrations/integrationDetector.ts
  • src/notebooks/deepnote/sqlCellStatusBarProvider.ts
  • src/notebooks/deepnote/integrations/integrationWebview.ts
🧠 Learnings (2)
📚 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/deepnoteTypes.ts : deepnoteTypes.ts contains Deepnote-related type definitions

Applied to files:

  • src/platform/notebooks/deepnote/integrationTypes.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/integrations/integrationWebview.ts
🧬 Code graph analysis (4)
src/platform/notebooks/deepnote/integrationTypes.ts (1)
src/webviews/webview-side/integrations/types.ts (1)
  • IntegrationType (1-1)
src/notebooks/deepnote/integrations/integrationDetector.ts (2)
src/platform/notebooks/deepnote/integrationTypes.ts (3)
  • IntegrationWithStatus (79-91)
  • DATAFRAME_SQL_INTEGRATION_ID (5-5)
  • DEEPNOTE_TO_INTEGRATION_TYPE (18-21)
src/webviews/webview-side/integrations/types.ts (2)
  • IntegrationWithStatus (29-35)
  • IntegrationStatus (3-3)
src/notebooks/deepnote/sqlCellStatusBarProvider.ts (3)
src/platform/notebooks/deepnote/types.ts (2)
  • IIntegrationStorage (6-6)
  • IIntegrationStorage (7-42)
src/notebooks/types.ts (2)
  • IDeepnoteNotebookManager (28-28)
  • IDeepnoteNotebookManager (29-39)
src/platform/notebooks/deepnote/integrationTypes.ts (2)
  • DATAFRAME_SQL_INTEGRATION_ID (5-5)
  • DEEPNOTE_TO_INTEGRATION_TYPE (18-21)
src/notebooks/deepnote/integrations/integrationWebview.ts (4)
src/platform/notebooks/deepnote/types.ts (2)
  • IIntegrationStorage (6-6)
  • IIntegrationStorage (7-42)
src/notebooks/types.ts (2)
  • IDeepnoteNotebookManager (28-28)
  • IDeepnoteNotebookManager (29-39)
src/platform/notebooks/deepnote/integrationTypes.ts (2)
  • IntegrationWithStatus (79-91)
  • INTEGRATION_TYPE_TO_DEEPNOTE (26-29)
src/webviews/webview-side/integrations/types.ts (1)
  • IntegrationWithStatus (29-35)
🪛 GitHub Check: TypeCheck
src/notebooks/deepnote/sqlCellStatusBarProvider.ts

[failure] 29-29:
'mapDeepnoteIntegrationType' is declared but its value is never read.


[failure] 29-29:
'"../../platform/notebooks/deepnote/integrationTypes"' has no exported member named 'mapDeepnoteIntegrationType'. Did you mean 'DEEPNOTE_TO_INTEGRATION_TYPE'?

⏰ 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 & Package Extension
  • GitHub Check: Lint & Format
🔇 Additional comments (16)
cspell.json (1)

32-32: LGTM!

Additions appropriately alphabetized and support the new DuckDB and PostgreSQL integrations.

Also applies to: 46-46

src/platform/notebooks/deepnote/integrationTypes.ts (2)

15-21: LGTM - Mapping supports Deepnote type translation.

The mapping correctly translates Deepnote integration type strings to internal enum values. Callers should handle potentially undefined lookups for unknown types.


83-90: Well-documented optional fields for prefilling.

The integrationName and integrationType fields are appropriately optional and clearly documented for their prefilling purpose.

src/notebooks/deepnote/integrations/integrationDetector.ts (1)

5-10: Imports correctly updated for project-level integration logic.

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

52-52: Correct DI injection of notebook manager.


295-307: Proper project retrieval with error handling.


309-348: Project-level integration list correctly integrated.

The quick pick now uses project.project.integrations with proper type mapping and fallback handling.

src/notebooks/deepnote/integrations/integrationWebview.ts (9)

8-8: Correct import of notebook manager interface.


11-11: Correct import of type mapping.


28-28: Appropriate projectId field for tracking.


30-34: Correct DI injection of notebook manager.


42-48: Project ID correctly captured for integration updates.

The show method now accepts and stores projectId, enabling project-level integration tracking.


131-132: Type labels added for UI display.


174-180: Integration metadata correctly propagated to webview.


225-231: Form prefill data correctly included.


304-344: Project integration list update correctly handles type mapping.

The method properly maps internal types to Deepnote types with warnings for unmapped types, then updates the notebook manager.

@jankuca jankuca force-pushed the jk/feat/integration-list-from-project branch from ecb0a82 to addaa7d Compare October 23, 2025 11:14
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

♻️ Duplicate comments (2)
src/notebooks/deepnote/deepnoteNotebookManager.ts (1)

86-103: Consider safer cloning and logging.

Current issues:

  • JSON serialization (line 93) drops non-serializable properties
  • Missing project returns false silently without logging

Recommendations:

  1. Use structuredClone() or a safe deep-clone utility
  2. Add logger.warn when project not found for debugging

Apply this diff:

     updateProjectIntegrations(projectId: string, integrations: ProjectIntegration[]): boolean {
         const project = this.originalProjects.get(projectId);
 
         if (!project) {
+            logger.warn(`updateProjectIntegrations: Project ${projectId} not found`);
             return false;
         }
 
-        const updatedProject = JSON.parse(JSON.stringify(project)) as DeepnoteProject;
+        const updatedProject = structuredClone(project) as DeepnoteProject;
         updatedProject.project.integrations = integrations;
src/notebooks/deepnote/integrations/integrationDetector.ts (1)

45-70: Handle unknown integration types.

Line 59 maps the type but doesn't validate. Unknown types produce undefined integrationType, silently added to results.

Apply this diff:

         for (const projectIntegration of projectIntegrations) {
             const integrationId = projectIntegration.id;
 
             // Skip the internal DuckDB integration
             if (integrationId === DATAFRAME_SQL_INTEGRATION_ID) {
                 continue;
             }
 
+            // Map the Deepnote integration type to our IntegrationType
+            const integrationType = DEEPNOTE_TO_INTEGRATION_TYPE[projectIntegration.type];
+            if (!integrationType) {
+                logger.warn(
+                    `IntegrationDetector: Unknown integration type '${projectIntegration.type}' for ${integrationId}, skipping`
+                );
+                continue;
+            }
+
             logger.debug(`IntegrationDetector: Found integration: ${integrationId} (${projectIntegration.type})`);
 
             // Check if the integration is configured
             const config = await this.integrationStorage.getIntegrationConfig(integrationId);
 
-            // Map the Deepnote integration type to our IntegrationType
-            const integrationType = DEEPNOTE_TO_INTEGRATION_TYPE[projectIntegration.type];
-
             const status: IntegrationWithStatus = {
                 config: config || null,
                 status: config ? IntegrationStatus.Connected : IntegrationStatus.Disconnected,
                 // Include integration metadata from project for prefilling when config is null
                 integrationName: projectIntegration.name,
-                integrationType: integrationType
+                integrationType
             };
 
             integrations.set(integrationId, status);
         }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ecb0a82 and addaa7d.

📒 Files selected for processing (7)
  • src/notebooks/deepnote/deepnoteNotebookManager.ts (2 hunks)
  • src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts (1 hunks)
  • src/notebooks/deepnote/integrations/integrationDetector.ts (3 hunks)
  • src/notebooks/deepnote/integrations/integrationWebview.ts (8 hunks)
  • src/notebooks/deepnote/sqlCellStatusBarProvider.ts (4 hunks)
  • src/notebooks/types.ts (2 hunks)
  • src/platform/notebooks/deepnote/integrationTypes.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/!(*.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/notebooks/deepnote/integrationTypes.ts
  • src/notebooks/types.ts
  • src/notebooks/deepnote/deepnoteNotebookManager.ts
  • src/notebooks/deepnote/sqlCellStatusBarProvider.ts
  • src/notebooks/deepnote/integrations/integrationDetector.ts
  • src/notebooks/deepnote/integrations/integrationWebview.ts
  • src/notebooks/deepnote/deepnoteNotebookManager.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/platform/notebooks/deepnote/integrationTypes.ts
  • src/notebooks/types.ts
  • src/notebooks/deepnote/deepnoteNotebookManager.ts
  • src/notebooks/deepnote/sqlCellStatusBarProvider.ts
  • src/notebooks/deepnote/integrations/integrationDetector.ts
  • src/notebooks/deepnote/integrations/integrationWebview.ts
  • src/notebooks/deepnote/deepnoteNotebookManager.unit.test.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/notebooks/deepnote/integrationTypes.ts
src/notebooks/deepnote/**

📄 CodeRabbit inference engine (CLAUDE.md)

Deepnote integration code resides under src/notebooks/deepnote/

Files:

  • src/notebooks/deepnote/deepnoteNotebookManager.ts
  • src/notebooks/deepnote/sqlCellStatusBarProvider.ts
  • src/notebooks/deepnote/integrations/integrationDetector.ts
  • src/notebooks/deepnote/integrations/integrationWebview.ts
  • src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts
src/notebooks/deepnote/deepnoteNotebookManager.ts

📄 CodeRabbit inference engine (CLAUDE.md)

deepnoteNotebookManager.ts handles Deepnote notebook state management

Files:

  • src/notebooks/deepnote/deepnoteNotebookManager.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/deepnoteNotebookManager.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/deepnoteNotebookManager.unit.test.ts
🧠 Learnings (2)
📚 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/deepnoteTypes.ts : deepnoteTypes.ts contains Deepnote-related type definitions

Applied to files:

  • src/platform/notebooks/deepnote/integrationTypes.ts
  • src/notebooks/types.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/types.ts
  • src/notebooks/deepnote/deepnoteNotebookManager.ts
  • src/notebooks/deepnote/sqlCellStatusBarProvider.ts
🧬 Code graph analysis (5)
src/platform/notebooks/deepnote/integrationTypes.ts (1)
src/webviews/webview-side/integrations/types.ts (1)
  • IntegrationType (1-1)
src/notebooks/deepnote/deepnoteNotebookManager.ts (3)
src/notebooks/types.ts (1)
  • ProjectIntegration (31-35)
src/notebooks/deepnote/deepnoteSerializer.ts (1)
  • DeepnoteProject (10-10)
src/notebooks/deepnote/deepnoteTypes.ts (1)
  • DeepnoteProject (12-12)
src/notebooks/deepnote/sqlCellStatusBarProvider.ts (3)
src/platform/notebooks/deepnote/types.ts (2)
  • IIntegrationStorage (6-6)
  • IIntegrationStorage (7-42)
src/notebooks/types.ts (2)
  • IDeepnoteNotebookManager (37-37)
  • IDeepnoteNotebookManager (38-58)
src/platform/notebooks/deepnote/integrationTypes.ts (1)
  • DEEPNOTE_TO_INTEGRATION_TYPE (18-21)
src/notebooks/deepnote/integrations/integrationWebview.ts (3)
src/notebooks/types.ts (3)
  • IDeepnoteNotebookManager (37-37)
  • IDeepnoteNotebookManager (38-58)
  • ProjectIntegration (31-35)
src/platform/notebooks/deepnote/integrationTypes.ts (2)
  • IntegrationWithStatus (79-91)
  • INTEGRATION_TYPE_TO_DEEPNOTE (26-29)
src/webviews/webview-side/integrations/types.ts (1)
  • IntegrationWithStatus (29-35)
src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts (2)
src/notebooks/deepnote/deepnoteSerializer.ts (1)
  • DeepnoteProject (10-10)
src/notebooks/deepnote/deepnoteTypes.ts (1)
  • DeepnoteProject (12-12)
🔇 Additional comments (11)
src/notebooks/types.ts (2)

28-35: LGTM: Clean interface definition.

The ProjectIntegration interface is well-structured with clear field types.


46-54: LGTM: Proper error signaling.

The boolean return value and comprehensive JSDoc address the previous review feedback effectively. Callers can now handle missing projects explicitly.

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

186-272: LGTM: Comprehensive test coverage.

The test suite covers all critical paths: successful updates, replacements, empty arrays, unknown projects, and property preservation. Boolean assertions use strictEqual as per guidelines.

src/platform/notebooks/deepnote/integrationTypes.ts (2)

23-29: LGTM: Exhaustive mapping.

The mapped type { [type in IntegrationType]: string } ensures all enum members are covered at compile time.


83-90: LGTM: Well-documented optional fields.

The JSDoc clearly explains when these fields are used.

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

25-29: LGTM: Clean imports.

The imports are correct. Previous issue with mapDeepnoteIntegrationType has been resolved.


294-347: LGTM: Robust project-based integration handling.

The implementation properly:

  • Validates project existence
  • Uses project.project.integrations as source
  • Handles unknown types with fallback to raw type string
  • Skips internal DuckDB from project list
src/notebooks/deepnote/integrations/integrationWebview.ts (4)

28-28: LGTM: Property addition.

The projectId property properly tracks the current project context.


42-48: LGTM: Updated signature with projectId.

The method now accepts projectId as first parameter and stores it for later use. Signature aligns with the updated flow.


131-132: LGTM: Localization additions.

Type labels for PostgreSQL and BigQuery properly added to localization payload.


304-353: LGTM: Comprehensive project integration sync.

The method properly:

  • Guards against missing projectId
  • Filters out integrations without types
  • Maps types via INTEGRATION_TYPE_TO_DEEPNOTE
  • Uses type guard for null filtering
  • Handles and logs update failures
  • Shows user-facing error when project not found

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/notebooks/deepnote/sqlCellStatusBarProvider.ts (1)

1-30: Group third-party imports together.

Blank line on line 20 should be removed (vscode and inversify are both third-party), then add blank line after inversify before local imports.

Apply this diff:

 } from 'vscode';
 import { inject, injectable } from 'inversify';
-
 
 import { IExtensionSyncActivationService } from '../../platform/activation/types';

As per coding guidelines

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between addaa7d and ccb2196.

📒 Files selected for processing (3)
  • src/notebooks/deepnote/integrations/integrationDetector.ts (3 hunks)
  • src/notebooks/deepnote/sqlCellStatusBarProvider.ts (4 hunks)
  • src/platform/notebooks/deepnote/integrationTypes.ts (2 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 .ts files (not suffixed with .node or .web)

Files:

  • src/platform/notebooks/deepnote/integrationTypes.ts
  • src/notebooks/deepnote/integrations/integrationDetector.ts
  • src/notebooks/deepnote/sqlCellStatusBarProvider.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/notebooks/deepnote/integrationTypes.ts
  • src/notebooks/deepnote/integrations/integrationDetector.ts
  • src/notebooks/deepnote/sqlCellStatusBarProvider.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/notebooks/deepnote/integrationTypes.ts
src/notebooks/deepnote/**

📄 CodeRabbit inference engine (CLAUDE.md)

Deepnote integration code resides under src/notebooks/deepnote/

Files:

  • src/notebooks/deepnote/integrations/integrationDetector.ts
  • src/notebooks/deepnote/sqlCellStatusBarProvider.ts
🧠 Learnings (2)
📚 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/deepnoteTypes.ts : deepnoteTypes.ts contains Deepnote-related type definitions

Applied to files:

  • src/platform/notebooks/deepnote/integrationTypes.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/sqlCellStatusBarProvider.ts
🧬 Code graph analysis (3)
src/platform/notebooks/deepnote/integrationTypes.ts (1)
src/webviews/webview-side/integrations/types.ts (1)
  • IntegrationType (1-1)
src/notebooks/deepnote/integrations/integrationDetector.ts (2)
src/platform/notebooks/deepnote/integrationTypes.ts (3)
  • IntegrationWithStatus (81-93)
  • DATAFRAME_SQL_INTEGRATION_ID (5-5)
  • DEEPNOTE_TO_INTEGRATION_TYPE (28-31)
src/webviews/webview-side/integrations/types.ts (2)
  • IntegrationWithStatus (29-35)
  • IntegrationStatus (3-3)
src/notebooks/deepnote/sqlCellStatusBarProvider.ts (3)
src/platform/notebooks/deepnote/types.ts (2)
  • IIntegrationStorage (6-6)
  • IIntegrationStorage (7-42)
src/notebooks/types.ts (2)
  • IDeepnoteNotebookManager (37-37)
  • IDeepnoteNotebookManager (38-58)
src/platform/notebooks/deepnote/integrationTypes.ts (2)
  • DATAFRAME_SQL_INTEGRATION_ID (5-5)
  • DEEPNOTE_TO_INTEGRATION_TYPE (28-31)
🔇 Additional comments (5)
src/platform/notebooks/deepnote/integrationTypes.ts (2)

15-31: Bidirectional mappings implemented correctly.

The use of satisfies { [type in IntegrationType]: string } ensures exhaustiveness, and RawIntegrationType provides type-safe reverse lookup. Well done.


85-92: Extension of IntegrationWithStatus is clear and well-documented.

Optional fields with explicit purpose for prefilling when config is absent.

src/notebooks/deepnote/integrations/integrationDetector.ts (1)

27-84: Project-level integration detection implemented correctly.

Unknown types handled with explicit warning. Type assertion necessary for external data. Logic is defensive and clear.

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

48-52: Dependency injection for notebookManager added correctly.

Proper use of @Inject decorator and IDeepnoteNotebookManager interface.


337-339: Type mapping and label resolution implemented correctly.

Type assertion necessary for external data. Fallback to raw type if mapping not found is defensive.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (2)
src/notebooks/deepnote/deepnoteNotebookManager.ts (2)

93-93: Replace JSON serialization with structuredClone.

JSON.parse(JSON.stringify()) drops non-serializable properties (functions, undefined, symbols). Use structuredClone for safer deep cloning.

Apply this diff:

-    const updatedProject = JSON.parse(JSON.stringify(project)) as DeepnoteProject;
+    const updatedProject = structuredClone(project) as DeepnoteProject;

89-91: Add logging for missing project.

Returning false handles the error, but logging helps debugging when projects aren't found.

     if (!project) {
+        // Consider: this.logger.warn(`Project not found: ${projectId}`);
         return false;
     }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ccb2196 and cf472f0.

📒 Files selected for processing (3)
  • src/notebooks/deepnote/deepnoteNotebookManager.ts (2 hunks)
  • src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts (1 hunks)
  • src/notebooks/types.ts (2 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/notebooks/types.ts
  • src/notebooks/deepnote/deepnoteNotebookManager.ts
  • src/notebooks/deepnote/deepnoteNotebookManager.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/types.ts
  • src/notebooks/deepnote/deepnoteNotebookManager.ts
  • src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts
src/notebooks/deepnote/**

📄 CodeRabbit inference engine (CLAUDE.md)

Deepnote integration code resides under src/notebooks/deepnote/

Files:

  • src/notebooks/deepnote/deepnoteNotebookManager.ts
  • src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts
src/notebooks/deepnote/deepnoteNotebookManager.ts

📄 CodeRabbit inference engine (CLAUDE.md)

deepnoteNotebookManager.ts handles Deepnote notebook state management

Files:

  • src/notebooks/deepnote/deepnoteNotebookManager.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/deepnoteNotebookManager.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/deepnoteNotebookManager.unit.test.ts
🧠 Learnings (2)
📚 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/deepnoteTypes.ts : deepnoteTypes.ts contains Deepnote-related type definitions

Applied to files:

  • src/notebooks/types.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/types.ts
  • src/notebooks/deepnote/deepnoteNotebookManager.ts
🧬 Code graph analysis (2)
src/notebooks/deepnote/deepnoteNotebookManager.ts (2)
src/notebooks/types.ts (1)
  • ProjectIntegration (31-35)
src/platform/deepnote/deepnoteTypes.ts (1)
  • DeepnoteProject (12-12)
src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts (2)
src/kernels/execution/cellExecution.ts (1)
  • result (69-71)
src/platform/deepnote/deepnoteTypes.ts (1)
  • DeepnoteProject (12-12)
⏰ 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: copilot-setup-steps
🔇 Additional comments (2)
src/notebooks/types.ts (2)

28-35: LGTM!

The interface is well-defined with clear fields and documentation.


46-54: LGTM!

The method signature properly returns a boolean for error handling and the JSDoc clearly documents the behavior when the project doesn't exist.

coderabbitai[bot]
coderabbitai bot previously approved these changes Oct 23, 2025
@jankuca jankuca marked this pull request as ready for review October 23, 2025 12:07
@jankuca jankuca requested a review from Artmann as a code owner October 23, 2025 12:07
@jankuca jankuca enabled auto-merge (squash) October 23, 2025 12:07
Copy link
Member

@saltenasl saltenasl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! working well

@jankuca jankuca merged commit e2fa9b5 into main Oct 24, 2025
13 checks passed
@jankuca jankuca deleted the jk/feat/integration-list-from-project branch October 24, 2025 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants