Skip to content

Conversation

@m1so
Copy link
Member

@m1so m1so commented Oct 31, 2025

Summary by CodeRabbit

  • Chores

    • Updated Deepnote toolkit to version 1.0.0rc2.
    • Modified toolkit installation to use PyPI registry for package resolution.
  • Refactor

    • Updated error messages to report package version information instead of installation source URL.

@linear
Copy link

linear bot commented Oct 31, 2025

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 31, 2025

📝 Walkthrough

Walkthrough

This PR migrates the Deepnote toolkit installation from a URL-based wheel specification to a PyPI version-based approach. The DEEPNOTE_TOOLKIT_WHEEL_URL constant is removed from types.ts, and the toolkit version bumps from 0.2.30.post30 to 1.0.0rc2. Both installer files now use pip's version specifier syntax (== operator) instead of wheel URLs. The DeepnoteToolkitInstallError error class updates to track packageVersion instead of packageUrl in its diagnostics.

Possibly related PRs

Suggested reviewers

  • jamesbhobbs
  • andyjakubowski
  • 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 "chore: Upgrade Toolkit to v1" directly and clearly reflects the primary objective of this PR, which is upgrading the Deepnote toolkit from version 0.2.30.post30 to 1.0.0rc2. All changes across four files support this central purpose: removing wheel URL-based installation, switching to PyPI version-based installation, updating the version constant, and adjusting error handling accordingly. The title is concise, uses conventional commit formatting, and provides sufficient clarity for scanning PR history.

📜 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 5e8dd8d and 45c643e.

📒 Files selected for processing (4)
  • src/kernels/deepnote/deepnoteSharedToolkitInstaller.node.ts (2 hunks)
  • src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (5 hunks)
  • src/kernels/deepnote/types.ts (1 hunks)
  • src/platform/errors/deepnoteKernelErrors.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/kernels/deepnote/types.ts
  • src/platform/errors/deepnoteKernelErrors.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/kernels/deepnote/types.ts
  • src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
  • src/platform/errors/deepnoteKernelErrors.ts
  • src/kernels/deepnote/deepnoteSharedToolkitInstaller.node.ts
src/kernels/**/*.ts

📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)

src/kernels/**/*.ts: Use event-driven updates (EventEmitter) for state changes
Monitor and dispose pending promises to prevent leaks during teardown
Use WeakRef/weak references for notebook/object backreferences to avoid memory leaks
Respect CancellationToken in all async operations

Files:

  • src/kernels/deepnote/types.ts
  • src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
  • src/kernels/deepnote/deepnoteSharedToolkitInstaller.node.ts
**/*.node.ts

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

Use *.node.ts for Desktop-specific implementations that require full file system access and Python environments

Files:

  • src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
  • src/kernels/deepnote/deepnoteSharedToolkitInstaller.node.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/errors/deepnoteKernelErrors.ts
🧬 Code graph analysis (2)
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (2)
src/platform/logging/index.ts (1)
  • logger (35-48)
src/kernels/deepnote/types.ts (1)
  • DEEPNOTE_TOOLKIT_VERSION (159-159)
src/kernels/deepnote/deepnoteSharedToolkitInstaller.node.ts (1)
src/kernels/deepnote/types.ts (1)
  • DEEPNOTE_TOOLKIT_VERSION (159-159)
🔇 Additional comments (9)
src/kernels/deepnote/deepnoteSharedToolkitInstaller.node.ts (2)

13-13: LGTM: Import correctly updated.

Wheel URL removed, version-based installation only.


223-223: LGTM: PyPI version specifier correctly applied.

Installs from PyPI instead of wheel URL.

src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (4)

7-7: LGTM: Import aligned with PyPI approach.


192-192: LGTM: Log message clarifies PyPI source.


202-202: LGTM: Version specifier correctly applied.


263-263: LGTM: Error calls updated for version parameter.

Consistent with DeepnoteToolkitInstallError signature change.

Also applies to: 281-281

src/platform/errors/deepnoteKernelErrors.ts (2)

102-102: LGTM: Parameter renamed for version-based installation.

Breaking change but all call sites updated.


115-115: LGTM: Technical details updated.

src/kernels/deepnote/types.ts (1)

159-159: Confirm RC version is appropriate for this use case.

deepnote-toolkit version 1.0.0rc2 exists on PyPI, but it's a pre-release. Given the major version bump (0.x → 1.x), verify this RC is stable enough for your needs or consider using a stable release if available.


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

@codecov
Copy link

codecov bot commented Oct 31, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72%. Comparing base (5e8dd8d) to head (45c643e).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@          Coverage Diff          @@
##            main    #146   +/-   ##
=====================================
  Coverage     72%     72%           
=====================================
  Files        545     545           
  Lines      41777   41777           
  Branches    5047    5047           
=====================================
  Hits       30431   30431           
  Misses      9665    9665           
  Partials    1681    1681           
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@m1so m1so marked this pull request as ready for review October 31, 2025 10:12
@m1so m1so requested a review from a team as a code owner October 31, 2025 10:12
@m1so m1so merged commit 9f0d02b into main Oct 31, 2025
13 checks passed
@m1so m1so deleted the michalbaumgartner/blu-5090-update-vscode-extension-to-use-toolkit-v1 branch October 31, 2025 10:16
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