feat: add cspell configuration and CI spell-check #34
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
⚙️ Control Options:
|
📝 WalkthroughWalkthroughThis PR substantially restructures the repository by removing Deepnote-specific features and renderers while introducing spell-check infrastructure. Key changes include: removing Deepnote converters, serialization, integration management, and the dataframe/Vega renderers; simplifying cell execution to eliminate Deepnote block transformation; removing PostCSS and Tailwind build integration; updating Husky hooks to relax git constraints; and expanding .gitignore. The build system and kernel execution paths are simplified, with output metadata handling streamlined to remove per-cell identifiers. Sequence DiagramsequenceDiagram
participant Editor as VS Code Editor
participant Kernel as Kernel Executor
participant Output as Output Handler
participant Cell as Cell Metadata
Note over Editor,Cell: BEFORE: Deepnote conversion path
Editor->>Kernel: Cell code + metadata
Kernel->>Output: Execute & produce output
Output->>Output: Extract cellId, metadata
Output->>Cell: Attach indexed metadata
Cell->>Editor: Display with metadata
Note over Editor,Cell: AFTER: Simplified direct path
Editor->>Kernel: Cell code (direct)
Kernel->>Output: Execute & produce output
Output->>Output: Transform output (data only)
Output->>Cell: Minimal metadata
Cell->>Editor: Display simplified
Possibly related PRs
Suggested reviewers
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🧰 Additional context used📓 Path-based instructions (3)**/!(*.node|*.web).ts📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
src/platform/**/*.ts📄 CodeRabbit inference engine (.github/instructions/platform.instructions.md)
Files:
🧠 Learnings (1)📚 Learning: 2025-10-17T12:29:11.866ZApplied to files:
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/ci.yml(1 hunks)cspell.json(1 hunks)package.json(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
package.json
[error] 1-1: npm ci failed: package-lock.json is out of sync. Please run 'npm install' to update the lockfile before continuing. The log shows multiple missing/invalid entries (e.g., Missing: cspell@9.2.1, @cspell/...; Invalid: lock file's flatted@3.2.9 vs 3.3.3, core-util-is@1.0.2 vs 1.0.3, import-fresh@3.3.0 vs 3.3.1).
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
cspell.json(1 hunks)
⏰ 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: Spell Check
…y from license check Merged latest main changes which added: - CODEOWNERS file - Qlty configuration - Audit jobs (audit-prod, audit-all) - Code coverage improvements Preserved spell-check job and added @cspell/dict-en-common-misspellings@2.1.6 to license excludePackages list to fix license check failures.
|
…rrors - Added 1773 unique words from CI spell-check failures - Includes technical terms: Pylance, ipywidgets, nteract, plotly, etc. - Excluded widget sample files and test data with base64 content - Local verification: 0 issues found in 1090 files
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
cspell.json (1)
13-15: Recursive globs needed.These patterns only hit root-level artifacts; minified bundles/maps under subdirectories still get scanned.
Apply:
- "*.min.js", - "*.min.css", - "*.map" + "**/*.min.js", + "**/*.min.css", + "**/*.map"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
cspell.json(1 hunks)
⏰ 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: Lint & Format
- Remove massive ~1,800 word list from cspell.json - Add 'files' array to check only src/notebooks/deepnote/, src/kernels/deepnote/, and deepnoteServerNotFoundError.ts - Update package.json spell-check scripts to use cspell without file globs (respects files config) - Add minimal 28 technical terms found in Deepnote code (venv, ipykernel, numpy, etc.) - No longer checks forked Microsoft/vscode-jupyter code Result: 36 files checked (down from 1,108), 0 spelling issues (down from 15,903)
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
package.json (1)
2168-2168: Regenerate package-lock.json.cspell was added; ensure package-lock.json is updated so
npm cipasses.Run and commit:
#!/bin/bash set -euo pipefail npm install --package-lock-only git status --porcelain | rg 'package-lock.json' -n || truecspell.json (1)
18-20: Use recursive ignore globs for minified/maps.Replace single-star patterns so artifacts in subfolders are ignored.
- "*.min.js", - "*.min.css", - "*.map" + "**/*.min.js", + "**/*.min.css", + "**/*.map"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
cspell.json(1 hunks)package.json(2 hunks)
⏰ 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 (1)
cspell.json (1)
23-52: Remove generic misspellings from cspell.json. Drop “dont”, “DONT” and “venv’s”; whitelist only project-specific terms and manually verify remaining entries aren’t unused.
Fix Lint & Format CI failure by applying Prettier formatting to cspell.json (4-space indentation)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
cspell.json (1)
18-20: Make ignore globs recursive.Current patterns only match files at repo root; use recursive to ignore artifacts anywhere.
- "*.min.js", - "*.min.css", - "*.map" + "**/*.min.js", + "**/*.min.css", + "**/*.map"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
cspell.json(1 hunks)
🔇 Additional comments (1)
cspell.json (1)
3-7: No drift in spell-check globs: bothspell-checkandspell-check:fixinvoke cspell without overriding paths, so cspell.json’sfilesfilter applies consistently.
| @@ -0,0 +1,54 @@ | |||
| { | |||
| "version": "0.2", | |||
| "files": [ | |||
There was a problem hiding this comment.
Pls check if we should add to here when reviewing
There was a problem hiding this comment.
I would add src/webviews/** and package.json
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: jamesbhobbs
PR: deepnote/deepnote#1
File: package.json:24-28
Timestamp: 2025-09-30T13:33:12.669Z
Learning: In the deepnote repository, do not suggest adding spell-check to lint-staged pre-commit hooks due to performance concerns. Spell-check runs in CI instead.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build & Test
- GitHub Check: copilot-setup-steps
| uses: actions/checkout@v5 | ||
|
|
||
| - name: Setup Node.js | ||
| uses: actions/setup-node@v5 | ||
| with: | ||
| cache: 'npm' | ||
| node-version: ${{ env.NODE_VERSION }} | ||
| registry-url: 'https://npm.pkg.github.com' | ||
| scope: '@deepnote' | ||
|
|
||
| - name: Install dependencies | ||
| run: npm ci --prefer-offline --no-audit |
There was a problem hiding this comment.
Re-pin GitHub Actions to SHAs.
Every other job pins actions/checkout and actions/setup-node to vetted SHAs; this job drops back to floating @v5, undermining the supply-chain hardening we already enforce. Please use the same commit SHAs as the rest of the workflow.
- - name: Checkout
- uses: actions/checkout@v5
+ - name: Checkout
+ uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5
@@
- - name: Setup Node.js
- uses: actions/setup-node@v5
+ - name: Setup Node.js
+ uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
.github/workflows/ci.yml lines 175-186: the job uses floating action tags
actions/checkout@v5 and actions/setup-node@v5 which weakens the repository's
supply-chain hardening; replace both with the exact commit SHAs used elsewhere
in this workflow (or the vetted SHAs used in other jobs) so they are pinned to
specific commits, i.e. locate the SHA entries for actions/checkout and
actions/setup-node in the other jobs and substitute @v5 with the corresponding
@<full-commit-sha> for each action.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yml(1 hunks)
⏰ 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
| uses: actions/checkout@v5 | ||
|
|
||
| - name: Setup Node.js | ||
| uses: actions/setup-node@v5 | ||
| with: | ||
| cache: 'npm' | ||
| node-version: ${{ env.NODE_VERSION }} | ||
| registry-url: 'https://npm.pkg.github.com' | ||
| scope: '@deepnote' | ||
|
|
||
| - name: Install dependencies | ||
| run: npm ci --prefer-offline --no-audit | ||
| env: | ||
| NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }} |
There was a problem hiding this comment.
Pin actions to commit SHAs.
Other jobs lock actions/checkout and actions/setup-node to specific SHAs; this job floats on the v5 tag, reopening a supply-chain risk. Please pin both actions to the same commits used elsewhere in the workflow.
- - name: Checkout
- uses: actions/checkout@v5
+ - name: Checkout
+ uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5
@@
- - name: Setup Node.js
- uses: actions/setup-node@v5
+ - name: Setup Node.js
+ uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
.github/workflows/ci.yml lines 175-188: the workflow uses floating tags for
actions/checkout@v5 and actions/setup-node@v5; replace both with the exact
commit SHAs used by the other jobs in this workflow (match the same commit SHAs
already pinned elsewhere) to eliminate the supply-chain risk — update the two
"uses:" lines to reference the exact SHA commits, run a quick lint/test of the
workflow, and commit the change.
- Resolves lockfile sync issue reported by user - Brings in new features: dataframe renderer, vega charts, SQL integration - Updates dependencies: @deepnote/blocks v1.2.0, react-vega, tailwindcss, etc.
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (23)
src/notebooks/deepnote/deepnoteSerializer.ts (2)
84-89: Use typed error classes and ILogger service.Guidelines require:
- ILogger service instead of
console.error- Typed error classes from
src/platform/errors/As per coding guidelines.
151-155: Use typed error classes and ILogger service.Guidelines require:
- ILogger service instead of
console.error- Typed error classes from
src/platform/errors/As per coding guidelines.
src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts (1)
80-90: Replace hardcoded strings with l10n.t() calls.User-facing strings must use localization per coding guidelines. These prompts appear in modal dialogs.
As per coding guidelines.
- const yes = 'Yes'; - const no = 'No'; - const dontAskAgain = "Don't Ask Again"; + const yes = l10n.t('Yes'); + const no = l10n.t('No'); + const dontAskAgain = l10n.t("Don't Ask Again"); const response = await window.showWarningMessage( - `A requirements.txt file already exists in this workspace. Do you want to override it with requirements from your Deepnote project?`, + l10n.t('A requirements.txt file already exists in this workspace. Do you want to override it with requirements from your Deepnote project?'), { modal: true },build/esbuild/build.ts (6)
183-186: Bug: externals mutated globally across builds.Pushing into common/web externals mutates shared arrays, causing cross-build leakage.
- const external = target === 'web' ? webExternals : commonExternals; - if (source.toLowerCase().endsWith('extension.node.ts')) { - external.push(...desktopExternals); - } + const externalBase = target === 'web' ? webExternals : commonExternals; + const external = [...externalBase]; + if (source.toLowerCase().endsWith('extension.node.ts')) { + external.push(...desktopExternals); + }
56-56: webExternals duplicates commonExternals.Simplify and avoid dupes.
-const webExternals = commonExternals.concat('os').concat(commonExternals); +const webExternals = [...commonExternals, 'os'];
18-34: Duplicate dependency entry ("fontkit").Listed twice; keep one.
const deskTopNodeModulesToExternalize = [ 'pdfkit/js/pdfkit.standalone', 'crypto-js', - 'fontkit', 'png-js', 'zeromq', // Copy, do not bundle 'zeromqold', // Copy, do not bundle @@ - // Its loaded by ivonv-lite, & since that is lazy loaded + // Its loaded by iconv-lite, & since that is lazy loaded // there's no need to include into the main bundle. - 'fontkit', + 'fontkit',
16-31: Fix typos in comments.Minor polish.
-// In desktop, we will bundle/copye each of these separately into the node_modules folder. +// In desktop, we will bundle/copy each of these separately into the node_modules folder. @@ -// Its loaded by ivonv-lite, & since that is lazy loaded +// Its loaded by iconv-lite, & since that is lazy loaded
432-446: Prefer resolving by package id for robustness.require.resolve on an absolute dir can be brittle; resolve the package entry directly.
- const fullPath = require.resolve(source); + const fullPath = require.resolve('vscode-jsonrpc');
497-498: Remove unused variable.started is never used.
-const started = Date.now();src/notebooks/deepnote/deepnoteExplorerView.ts (2)
92-93: Localize all user-facing strings.Wrap showError/Info/Warning messages with l10n.t and use placeholders.
- await window.showErrorMessage(`Failed to open notebook: ${errorMessage}`); + await window.showErrorMessage(l10n.t('Failed to open notebook: {0}', errorMessage)); - await window.showInformationMessage('No active Deepnote notebook found.'); + await window.showInformationMessage(l10n.t('No active Deepnote notebook found.')); - await window.showWarningMessage('Cannot reveal notebook: missing metadata.'); + await window.showWarningMessage(l10n.t('Cannot reveal notebook: missing metadata.')); - await window.showInformationMessage( - `Active notebook: ${notebookMetadata?.deepnoteNotebookName || 'Untitled'} in project ${ - notebookMetadata?.deepnoteProjectName || 'Untitled' - }` - ); + await window.showInformationMessage( + l10n.t('Active notebook: {0} in project {1}', + notebookMetadata?.deepnoteNotebookName || l10n.t('Untitled'), + notebookMetadata?.deepnoteProjectName || l10n.t('Untitled')) +);Also applies to: 113-116, 123-141, 145-149
62-89: Add ILogger injection and replace all console calls; redact IDs and localize error messages.File requires:
- Import ILogger from
../../platform/logging/types- Inject via
@inject(ILogger) private readonly logger: ILogger- Replace 4 console.log calls in
openNotebook()(lines 61, 75, 79, 83) withthis.logger.debug(), redacting IDs via.slice(0,8)- Replace console.error in
revealActiveNotebook()(line 142) withthis.logger.error()- Wrap error message at line 88 with
l10n.t()src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts (4)
78-85: Test description mismatches implementation.Code adds a query to URI; it’s not a “base file URI without fragments.” Rename the test to reflect query usage.
- test('should use base file URI without fragments', async () => { + test('should use file URI with query to disambiguate notebooks', async () => {
93-101: Use enum values, not strings, for item types.Import DeepnoteTreeItemType and use its members; string literals can bypass branches and weaken tests.
-import type { DeepnoteTreeItemContext } from './deepnoteTreeItem'; +import type { DeepnoteTreeItemContext } from './deepnoteTreeItem'; +import { DeepnoteTreeItemType } from './deepnoteTreeItem'; -const mockTreeItem = { type: 'notebook', context: { filePath: '/test/path' } } as any; +const mockTreeItem = { type: DeepnoteTreeItemType.Notebook, context: { filePath: '/test/path' } } as any; -const mockTreeItem = { type: 'ProjectFile', context: { filePath: '/test/path/project.deepnote' } } as any; +const mockTreeItem = { type: DeepnoteTreeItemType.ProjectFile, context: { filePath: '/test/path/project.deepnote' } } as any;If enum member for “notebook” differs, adjust accordingly.
Also applies to: 108-116
167-177: Remove stale serializer reference.explorerView has no serializer; this assertion misleads and can mask issues.
- const hasSerializer = (explorerView as any).serializer !== undefined; - assert.isTrue(hasTreeDataProvider || hasSerializer, 'Components are being initialized'); + assert.isTrue(hasTreeDataProvider, 'Components are being initialized');
34-46: Stub thewindow.createTreeView()dependency instead of catching errors.The
activate()method depends on VS Code'swindowAPI. Rather than wrapping in try-catch, mock this dependency at the module level or inject it via the test setup.Example approach:
- Use
sinon.stub(vscode.window, 'createTreeView')in test setup- Or provide a test mock that implements the required vscode module exports
- Verify the method called the API with correct arguments rather than verifying no error was thrown
Lines 34–46 should assert specific behavior, not just catch exceptions.
src/notebooks/deepnote/pocket.unit.test.ts (1)
182-189: Add a test for markup default type.Ensure createBlockFromPocket defaults to 'markdown' for markup cells with no pocket.
+ test('uses markdown type for markup cells when no pocket exists', () => { + const cell = new NotebookCellData(NotebookCellKind.Markup, '# Title', 'markdown'); + const block = createBlockFromPocket(cell, 0); + assert.strictEqual(block.type, 'markdown'); + });src/notebooks/deepnote/deepnoteDataConverter.unit.test.ts (2)
150-172: Add ordering test by sortingKey.JSDoc promises sorting; assert that output order follows sortingKey when input is unsorted.
+ test('orders cells by sortingKey when converting blocks', () => { + const blocks: DeepnoteBlock[] = [ + { id: 'b', type: 'code', content: '2', sortingKey: 'a1', blockGroup: 'g' }, + { id: 'a', type: 'code', content: '1', sortingKey: 'a0', blockGroup: 'g' } + ] as any; + const cells = converter.convertBlocksToCells(blocks); + assert.strictEqual(cells[0].value, '1'); + assert.strictEqual(cells[1].value, '2'); + });
263-299: Add image output tests to catch base64 encode/decode issues.Covers PNG/JPEG paths and prevents regressions in binary handling.
src/notebooks/deepnote/pocket.ts (1)
18-40: Merging existing pockets.addPocketToCellMetadata overwrites any pre-existing __deepnotePocket. If callers may set it earlier, consider shallow-merging instead.
- cell.metadata = { - ...src, - __deepnotePocket: pocket - }; + cell.metadata = { + ...src, + __deepnotePocket: { ...(src.__deepnotePocket as object | undefined), ...pocket } + };src/notebooks/deepnote/deepnoteDataConverter.ts (4)
90-99: Replace atob/btoa; these aren’t available in Node/VS Code EH and are slow for large data.Use Buffer for base64 encode/decode; avoid spread on large arrays.
- private base64ToUint8Array(base64: string): Uint8Array { - const binaryString = atob(base64); - const bytes = new Uint8Array(binaryString.length); - for (let i = 0; i < binaryString.length; i++) { - bytes[i] = binaryString.charCodeAt(i); - } - return bytes; - } + private base64ToUint8Array(base64: string): Uint8Array { + return Uint8Array.from(Buffer.from(base64, 'base64')); + } ... - data['image/png'] = btoa(String.fromCharCode(...new Uint8Array(item.data))); + data['image/png'] = Buffer.from(item.data).toString('base64'); ... - data['image/jpeg'] = btoa(String.fromCharCode(...new Uint8Array(item.data))); + data['image/jpeg'] = Buffer.from(item.data).toString('base64');Verification:
#!/bin/bash rg -nP '\b(atob|btoa)\s*\('Also applies to: 173-178
24-31: Either sort blocks or update JSDoc.JSDoc claims sorting by sortingKey; implement it or fix the comment.
- convertBlocksToCells(blocks: DeepnoteBlock[]): NotebookCellData[] { - return blocks.map((block) => { + convertBlocksToCells(blocks: DeepnoteBlock[]): NotebookCellData[] { + const sorted = blocks.slice().sort((a, b) => (a.sortingKey || '').localeCompare(b.sortingKey || '')); + return sorted.map((block) => {
111-121: Align fallback cell metadata with pocket mechanism.Use the same pocket fields for consistency; optional but reduces special cases.
- private createFallbackCell(block: DeepnoteBlock): NotebookCellData { - const cell = new NotebookCellData(NotebookCellKind.Markup, block.content || '', 'markdown'); - cell.metadata = { - deepnoteBlockId: block.id, - deepnoteBlockType: block.type, - deepnoteSortingKey: block.sortingKey, - deepnoteMetadata: block.metadata - }; - return cell; - } + private createFallbackCell(block: DeepnoteBlock): NotebookCellData { + const cell = new NotebookCellData(NotebookCellKind.Markup, block.content || '', 'markdown'); + cell.metadata = { + id: block.id, + type: block.type, + sortingKey: block.sortingKey, + ...(block.metadata ?? {}) + }; + addPocketToCellMetadata(cell); + return cell; + }
205-321: Minor: reuse encoders/decoders to reduce churn.Cache TextEncoder/TextDecoder or use NotebookCellOutputItem helpers consistently. Non-blocking.
♻️ Duplicate comments (2)
package.json (1)
2058-2060: Prior review comments remain unresolved.Past comments flagged:
- Critical: Missing
package-lock.jsonupdate blocks CI- Major: Spell-check scripts lack cache/determinism
- Major (resolved by maintainer): License exclusion rationale
Address the lockfile issue before merging.
.github/workflows/ci.yml (1)
175-178: Pin actions to commit SHAs.Past comments already flagged: floating
@v5tags weaken supply-chain security. Use the same pinned SHAs as other jobs (e.g.,actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (71)
.github/workflows/ci.yml(1 hunks).gitignore(1 hunks).husky/post-checkout(0 hunks).husky/pre-commit(1 hunks).husky/pre-push(0 hunks)build/esbuild/build.ts(2 hunks)package.json(6 hunks)package.nls.json(0 hunks)postcss.config.js(0 hunks)src/kernels/deepnote/deepnoteToolkitInstaller.node.ts(2 hunks)src/kernels/execution/cellExecution.ts(0 hunks)src/kernels/execution/cellExecutionCreator.ts(3 hunks)src/kernels/execution/cellExecutionCreator.unit.test.ts(0 hunks)src/kernels/execution/cellExecutionMessageHandler.ts(3 hunks)src/kernels/execution/helpers.ts(9 hunks)src/messageTypes.ts(0 hunks)src/notebooks/deepnote/blocks.md(0 hunks)src/notebooks/deepnote/converters/sqlBlockConverter.ts(0 hunks)src/notebooks/deepnote/converters/sqlBlockConverter.unit.test.ts(0 hunks)src/notebooks/deepnote/converters/visualizationBlockConverter.ts(0 hunks)src/notebooks/deepnote/converters/visualizationBlockConverter.unit.test.ts(0 hunks)src/notebooks/deepnote/deepnoteActivationService.ts(1 hunks)src/notebooks/deepnote/deepnoteActivationService.unit.test.ts(3 hunks)src/notebooks/deepnote/deepnoteDataConverter.ts(10 hunks)src/notebooks/deepnote/deepnoteDataConverter.unit.test.ts(3 hunks)src/notebooks/deepnote/deepnoteExplorerView.ts(1 hunks)src/notebooks/deepnote/deepnoteExplorerView.unit.test.ts(1 hunks)src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts(3 hunks)src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts(16 hunks)src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts(5 hunks)src/notebooks/deepnote/deepnoteSerializer.ts(5 hunks)src/notebooks/deepnote/integrations/integrationDetector.ts(0 hunks)src/notebooks/deepnote/integrations/integrationManager.ts(0 hunks)src/notebooks/deepnote/integrations/integrationStorage.ts(0 hunks)src/notebooks/deepnote/integrations/integrationTypes.ts(0 hunks)src/notebooks/deepnote/integrations/integrationUtils.ts(0 hunks)src/notebooks/deepnote/integrations/integrationWebview.ts(0 hunks)src/notebooks/deepnote/integrations/types.ts(0 hunks)src/notebooks/deepnote/pocket.ts(3 hunks)src/notebooks/deepnote/pocket.unit.test.ts(7 hunks)src/notebooks/deepnote/serialization.md(1 hunks)src/notebooks/serviceRegistry.node.ts(0 hunks)src/notebooks/serviceRegistry.web.ts(0 hunks)src/notebooks/types.ts(1 hunks)src/platform/common/constants.ts(0 hunks)src/platform/common/utils/localize.ts(0 hunks)src/platform/webviews/webviewHost.ts(1 hunks)src/standalone/intellisense/resolveCompletionItem.unit.test.ts(1 hunks)src/test/vscode-mock.ts(0 hunks)src/webviews/extension-side/dataframe/dataframeController.ts(0 hunks)src/webviews/extension-side/dataframe/dataframeController.unit.test.ts(0 hunks)src/webviews/extension-side/serviceRegistry.node.ts(0 hunks)src/webviews/extension-side/serviceRegistry.web.ts(0 hunks)src/webviews/webview-side/dataframe-renderer/DataframeRenderer.tsx(0 hunks)src/webviews/webview-side/dataframe-renderer/index.ts(0 hunks)src/webviews/webview-side/dataframe-renderer/tailwind.css(0 hunks)src/webviews/webview-side/integrations/BigQueryForm.tsx(0 hunks)src/webviews/webview-side/integrations/ConfigurationForm.tsx(0 hunks)src/webviews/webview-side/integrations/IntegrationItem.tsx(0 hunks)src/webviews/webview-side/integrations/IntegrationList.tsx(0 hunks)src/webviews/webview-side/integrations/IntegrationPanel.tsx(0 hunks)src/webviews/webview-side/integrations/PostgresForm.tsx(0 hunks)src/webviews/webview-side/integrations/index.html(0 hunks)src/webviews/webview-side/integrations/index.tsx(0 hunks)src/webviews/webview-side/integrations/integrations.css(0 hunks)src/webviews/webview-side/integrations/types.ts(0 hunks)src/webviews/webview-side/vega-renderer/VegaRenderer.tsx(0 hunks)src/webviews/webview-side/vega-renderer/colors.ts(0 hunks)src/webviews/webview-side/vega-renderer/index.ts(0 hunks)src/webviews/webview-side/vega-renderer/number-formats.ts(0 hunks)tailwind.config.js(0 hunks)
💤 Files with no reviewable changes (46)
- src/platform/common/constants.ts
- package.nls.json
- src/notebooks/deepnote/blocks.md
- src/notebooks/deepnote/integrations/integrationTypes.ts
- src/webviews/webview-side/integrations/BigQueryForm.tsx
- src/notebooks/serviceRegistry.node.ts
- src/notebooks/deepnote/converters/sqlBlockConverter.ts
- src/webviews/webview-side/vega-renderer/VegaRenderer.tsx
- src/notebooks/deepnote/integrations/integrationManager.ts
- src/webviews/webview-side/integrations/ConfigurationForm.tsx
- src/notebooks/deepnote/integrations/integrationDetector.ts
- src/test/vscode-mock.ts
- src/notebooks/deepnote/integrations/integrationStorage.ts
- src/notebooks/deepnote/integrations/types.ts
- src/webviews/extension-side/serviceRegistry.node.ts
- src/webviews/webview-side/integrations/IntegrationItem.tsx
- src/messageTypes.ts
- src/kernels/execution/cellExecutionCreator.unit.test.ts
- src/webviews/webview-side/integrations/index.html
- .husky/pre-push
- src/notebooks/deepnote/converters/visualizationBlockConverter.ts
- src/webviews/webview-side/integrations/IntegrationList.tsx
- src/webviews/webview-side/integrations/integrations.css
- src/webviews/webview-side/vega-renderer/colors.ts
- src/webviews/webview-side/vega-renderer/number-formats.ts
- src/webviews/webview-side/integrations/types.ts
- src/notebooks/deepnote/integrations/integrationWebview.ts
- src/webviews/extension-side/dataframe/dataframeController.unit.test.ts
- tailwind.config.js
- src/notebooks/serviceRegistry.web.ts
- src/platform/common/utils/localize.ts
- src/webviews/extension-side/serviceRegistry.web.ts
- src/webviews/webview-side/integrations/PostgresForm.tsx
- src/webviews/webview-side/dataframe-renderer/DataframeRenderer.tsx
- .husky/post-checkout
- src/webviews/webview-side/integrations/index.tsx
- src/webviews/extension-side/dataframe/dataframeController.ts
- src/notebooks/deepnote/converters/visualizationBlockConverter.unit.test.ts
- postcss.config.js
- src/webviews/webview-side/integrations/IntegrationPanel.tsx
- src/webviews/webview-side/vega-renderer/index.ts
- src/webviews/webview-side/dataframe-renderer/tailwind.css
- src/kernels/execution/cellExecution.ts
- src/notebooks/deepnote/integrations/integrationUtils.ts
- src/notebooks/deepnote/converters/sqlBlockConverter.unit.test.ts
- src/webviews/webview-side/dataframe-renderer/index.ts
🧰 Additional context used
📓 Path-based instructions (12)
**/!(*.node|*.web).ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place shared cross-platform logic in common
.tsfiles (not suffixed with.nodeor.web)
Files:
src/platform/webviews/webviewHost.tssrc/standalone/intellisense/resolveCompletionItem.unit.test.tssrc/notebooks/deepnote/deepnoteSerializer.tssrc/notebooks/deepnote/deepnoteExplorerView.unit.test.tssrc/notebooks/deepnote/pocket.tssrc/notebooks/deepnote/deepnoteActivationService.unit.test.tsbuild/esbuild/build.tssrc/notebooks/deepnote/deepnoteExplorerView.tssrc/notebooks/types.tssrc/notebooks/deepnote/deepnoteDataConverter.unit.test.tssrc/notebooks/deepnote/deepnoteActivationService.tssrc/notebooks/deepnote/deepnoteDataConverter.tssrc/kernels/execution/helpers.tssrc/kernels/execution/cellExecutionCreator.tssrc/kernels/execution/cellExecutionMessageHandler.tssrc/notebooks/deepnote/pocket.unit.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Inject interfaces, not concrete classes
Avoid circular dependencies
Usel10n.t()for user-facing strings
Use typed error classes fromsrc/platform/errors/when throwing or handling errors
Use theILoggerservice instead ofconsole.log
Preserve error details while scrubbing PII in messages and telemetry
Include the Microsoft copyright header in source files
Prefer async/await and handle cancellation withCancellationToken
**/*.{ts,tsx}: Order class members (methods, fields, properties) first by accessibility (public/protected/private) and then alphabetically
Do not add the Microsoft copyright header to new files
Use Uri.joinPath() to construct file paths instead of manual string concatenation
Add a blank line after groups of const declarations and before return statements for readability
Separate third-party imports from local file imports
Files:
src/platform/webviews/webviewHost.tssrc/standalone/intellisense/resolveCompletionItem.unit.test.tssrc/notebooks/deepnote/deepnoteSerializer.tssrc/notebooks/deepnote/deepnoteKernelAutoSelector.node.tssrc/notebooks/deepnote/deepnoteExplorerView.unit.test.tssrc/notebooks/deepnote/pocket.tssrc/notebooks/deepnote/deepnoteActivationService.unit.test.tsbuild/esbuild/build.tssrc/notebooks/deepnote/deepnoteExplorerView.tssrc/notebooks/types.tssrc/notebooks/deepnote/deepnoteDataConverter.unit.test.tssrc/kernels/deepnote/deepnoteToolkitInstaller.node.tssrc/notebooks/deepnote/deepnoteActivationService.tssrc/notebooks/deepnote/deepnoteDataConverter.tssrc/kernels/execution/helpers.tssrc/kernels/execution/cellExecutionCreator.tssrc/notebooks/deepnote/deepnoteRequirementsHelper.node.tssrc/kernels/execution/cellExecutionMessageHandler.tssrc/notebooks/deepnote/pocket.unit.test.tssrc/notebooks/deepnote/deepnoteInitNotebookRunner.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/webviews/webviewHost.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/standalone/intellisense/resolveCompletionItem.unit.test.tssrc/notebooks/deepnote/deepnoteExplorerView.unit.test.tssrc/notebooks/deepnote/deepnoteActivationService.unit.test.tssrc/notebooks/deepnote/deepnoteDataConverter.unit.test.tssrc/notebooks/deepnote/pocket.unit.test.ts
**/*.{test,spec}.ts
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
In unit tests, when a mock is returned from a promise, ensure the mocked instance has an undefined
thenproperty to avoid hanging tests
Files:
src/standalone/intellisense/resolveCompletionItem.unit.test.tssrc/notebooks/deepnote/deepnoteExplorerView.unit.test.tssrc/notebooks/deepnote/deepnoteActivationService.unit.test.tssrc/notebooks/deepnote/deepnoteDataConverter.unit.test.tssrc/notebooks/deepnote/pocket.unit.test.ts
src/notebooks/deepnote/**
📄 CodeRabbit inference engine (CLAUDE.md)
Deepnote integration code resides under src/notebooks/deepnote/
Files:
src/notebooks/deepnote/deepnoteSerializer.tssrc/notebooks/deepnote/deepnoteKernelAutoSelector.node.tssrc/notebooks/deepnote/deepnoteExplorerView.unit.test.tssrc/notebooks/deepnote/serialization.mdsrc/notebooks/deepnote/pocket.tssrc/notebooks/deepnote/deepnoteActivationService.unit.test.tssrc/notebooks/deepnote/deepnoteExplorerView.tssrc/notebooks/deepnote/deepnoteDataConverter.unit.test.tssrc/notebooks/deepnote/deepnoteActivationService.tssrc/notebooks/deepnote/deepnoteDataConverter.tssrc/notebooks/deepnote/deepnoteRequirementsHelper.node.tssrc/notebooks/deepnote/pocket.unit.test.tssrc/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts
src/notebooks/deepnote/deepnoteSerializer.ts
📄 CodeRabbit inference engine (CLAUDE.md)
deepnoteSerializer.ts is the main serializer orchestrating Deepnote integration
Files:
src/notebooks/deepnote/deepnoteSerializer.ts
**/*.node.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
*.node.tsfor Desktop-specific implementations that require full file system access and Python environments
Files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.tssrc/kernels/deepnote/deepnoteToolkitInstaller.node.tssrc/notebooks/deepnote/deepnoteRequirementsHelper.node.tssrc/notebooks/deepnote/deepnoteInitNotebookRunner.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/deepnoteToolkitInstaller.node.tssrc/kernels/execution/helpers.tssrc/kernels/execution/cellExecutionCreator.tssrc/kernels/execution/cellExecutionMessageHandler.ts
src/notebooks/deepnote/deepnoteActivationService.ts
📄 CodeRabbit inference engine (CLAUDE.md)
deepnoteActivationService.ts manages VS Code activation for Deepnote
Files:
src/notebooks/deepnote/deepnoteActivationService.ts
src/notebooks/deepnote/deepnoteDataConverter.ts
📄 CodeRabbit inference engine (CLAUDE.md)
deepnoteDataConverter.ts performs Deepnote data transformations
Files:
src/notebooks/deepnote/deepnoteDataConverter.ts
src/kernels/execution/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)
Notebook-level execution must enqueue cells via CellExecutionQueue and manage per-cell lifecycles through CellExecution
Files:
src/kernels/execution/helpers.tssrc/kernels/execution/cellExecutionCreator.tssrc/kernels/execution/cellExecutionMessageHandler.ts
🧠 Learnings (18)
📓 Common learnings
Learnt from: jamesbhobbs
PR: deepnote/deepnote#1
File: package.json:24-28
Timestamp: 2025-09-30T13:33:12.669Z
Learning: In the deepnote repository, do not suggest adding spell-check to lint-staged pre-commit hooks due to performance concerns. Spell-check runs in CI instead.
📚 Learning: 2025-10-09T11:21:57.494Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-09T11:21:57.494Z
Learning: Applies to src/notebooks/deepnote/deepnoteSerializer.ts : deepnoteSerializer.ts is the main serializer orchestrating Deepnote integration
Applied to files:
src/notebooks/deepnote/deepnoteSerializer.tssrc/notebooks/deepnote/serialization.mdsrc/notebooks/deepnote/deepnoteActivationService.ts
📚 Learning: 2025-10-09T11:21:57.494Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-09T11:21:57.494Z
Learning: Applies to src/notebooks/deepnote/deepnoteNotebookManager.ts : deepnoteNotebookManager.ts handles Deepnote notebook state management
Applied to files:
src/notebooks/deepnote/deepnoteSerializer.tssrc/notebooks/deepnote/deepnoteExplorerView.tssrc/notebooks/types.tssrc/notebooks/deepnote/deepnoteActivationService.tssrc/notebooks/deepnote/deepnoteInitNotebookRunner.node.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/deepnoteActivationService.ts : deepnoteActivationService.ts manages VS Code activation for Deepnote
Applied to files:
src/notebooks/deepnote/deepnoteSerializer.tssrc/notebooks/deepnote/deepnoteKernelAutoSelector.node.tssrc/notebooks/deepnote/deepnoteActivationService.unit.test.tssrc/kernels/deepnote/deepnoteToolkitInstaller.node.tssrc/notebooks/deepnote/deepnoteActivationService.tssrc/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts
📚 Learning: 2025-10-09T11:21:57.494Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-09T11:21:57.494Z
Learning: Applies to src/notebooks/deepnote/deepnoteDataConverter.ts : deepnoteDataConverter.ts performs Deepnote data transformations
Applied to files:
src/notebooks/deepnote/deepnoteSerializer.tssrc/notebooks/deepnote/deepnoteDataConverter.ts
📚 Learning: 2025-09-03T12:54:35.368Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/interactiveWindow.instructions.md:0-0
Timestamp: 2025-09-03T12:54:35.368Z
Learning: Applies to src/interactive-window/{interactiveWindow.ts,interactiveWindowController.ts} : Implement robust kernel error handling: automatic restart, clear error display in system info cells, graceful degradation when kernels are unavailable, and user notifications for critical failures
Applied to files:
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts
📚 Learning: 2025-10-14T13:43:36.364Z
Learnt from: CR
PR: deepnote/deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-14T13:43:36.364Z
Learning: Applies to packages/*/src/**/*.test.{ts,tsx} : Import { describe, expect, it } from 'vitest' in tests
Applied to files:
src/notebooks/deepnote/deepnoteExplorerView.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/notebooks/deepnote/pocket.tssrc/notebooks/types.ts
📚 Learning: 2025-09-12T07:58:04.493Z
Learnt from: Artmann
PR: deepnote/vscode-deepnote#12
File: src/notebooks/deepnote/deepnoteActivationService.unit.test.ts:31-45
Timestamp: 2025-09-12T07:58:04.493Z
Learning: In src/notebooks/deepnote/deepnoteActivationService.ts, the activate() method is synchronous with signature `public activate()` and returns void. It should be called without await in tests.
Applied to files:
src/notebooks/deepnote/deepnoteActivationService.unit.test.tssrc/notebooks/deepnote/deepnoteActivationService.ts
📚 Learning: 2025-09-12T07:58:04.493Z
Learnt from: Artmann
PR: deepnote/vscode-deepnote#12
File: src/notebooks/deepnote/deepnoteActivationService.unit.test.ts:31-45
Timestamp: 2025-09-12T07:58:04.493Z
Learning: In src/notebooks/deepnote/deepnoteActivationService.ts, the activate() method is synchronous and returns void, not a Promise. It should be called without await in tests.
Applied to files:
src/notebooks/deepnote/deepnoteActivationService.unit.test.tssrc/notebooks/deepnote/deepnoteActivationService.ts
📚 Learning: 2025-09-03T13:01:49.096Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-09-03T13:01:49.096Z
Learning: Use `npm run format` to check code style
Applied to files:
.husky/pre-commit
📚 Learning: 2025-09-03T12:53:28.421Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-03T12:53:28.421Z
Learning: Run `npm run format-fix` before committing any changes
Applied to files:
.husky/pre-commit
📚 Learning: 2025-09-03T13:01:49.096Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-09-03T13:01:49.096Z
Learning: ALWAYS run `npm run format-fix` before committing changes
Applied to files:
.husky/pre-commit
📚 Learning: 2025-10-07T19:51:40.201Z
Learnt from: m1so
PR: deepnote/vscode-deepnote#25
File: src/kernels/deepnote/deepnoteServerStarter.node.ts:150-154
Timestamp: 2025-10-07T19:51:40.201Z
Learning: The deepnote-toolkit server is a separate Python package maintained in a different repository from the VS Code extension (vscode-deepnote). The VS Code extension launches the toolkit as a subprocess and communicates via environment variables and HTTP. The toolkit implementation cannot be verified from the extension codebase.
Applied to files:
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
📚 Learning: 2025-09-03T12:59:14.489Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/notebooks.instructions.md:0-0
Timestamp: 2025-09-03T12:59:14.489Z
Learning: Applies to src/notebooks/export/**/*.ts : Update FileConverter to handle the new export format
Applied to files:
src/notebooks/deepnote/deepnoteDataConverter.ts
📚 Learning: 2025-09-03T12:58:18.091Z
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel.instructions.md:0-0
Timestamp: 2025-09-03T12:58:18.091Z
Learning: Applies to src/kernels/execution/CellExecutionMessageHandler.ts : CellExecutionMessageHandler must process kernel messages (iopub, execute_reply) and update VS Code cell outputs
Applied to files:
src/kernels/execution/cellExecutionMessageHandler.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: Use dependency injection with inversify
Applied to files:
src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts
📚 Learning: 2025-09-30T13:33:12.669Z
Learnt from: jamesbhobbs
PR: deepnote/deepnote#1
File: package.json:24-28
Timestamp: 2025-09-30T13:33:12.669Z
Learning: In the deepnote repository, do not suggest adding spell-check to lint-staged pre-commit hooks due to performance concerns. Spell-check runs in CI instead.
Applied to files:
package.json
🧬 Code graph analysis (8)
src/standalone/intellisense/resolveCompletionItem.unit.test.ts (1)
src/kernels/execution/helpers.ts (1)
cellOutputToVSCCellOutput(129-163)
src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts (1)
src/platform/logging/index.ts (1)
logger(35-48)
src/notebooks/deepnote/pocket.ts (2)
src/notebooks/deepnote/deepnoteTypes.ts (1)
DeepnoteOutput(23-51)src/notebooks/deepnote/dataConversionUtils.ts (1)
generateBlockId(8-15)
src/notebooks/deepnote/deepnoteDataConverter.ts (4)
src/notebooks/deepnote/converters/markdownBlockConverter.ts (1)
MarkdownBlockConverter(6-24)src/notebooks/deepnote/pocket.ts (2)
addPocketToCellMetadata(18-40)createBlockFromPocket(46-78)src/test/mocks/vsc/extHostedTypes.ts (3)
NotebookCellData(2523-2549)NotebookCellOutput(71-103)NotebookCellOutputItem(16-69)src/notebooks/deepnote/deepnoteTypes.ts (2)
DeepnoteBlock(6-6)DeepnoteOutput(23-51)
src/kernels/execution/helpers.ts (1)
src/test/mocks/vsc/extHostedTypes.ts (2)
NotebookCellOutput(71-103)NotebookCellOutputItem(16-69)
src/kernels/execution/cellExecutionCreator.ts (3)
src/platform/logging/index.ts (1)
logger(35-48)src/platform/common/utils/misc.ts (1)
noop(11-11)src/notebooks/controllers/notebookIPyWidgetCoordinator.ts (1)
controller(27-32)
src/kernels/execution/cellExecutionMessageHandler.ts (2)
src/kernels/execution/helpers.ts (1)
cellOutputToVSCCellOutput(129-163)src/platform/common/utils.ts (2)
concatMultilineString(211-225)formatStreamText(275-278)
src/notebooks/deepnote/pocket.unit.test.ts (1)
src/notebooks/deepnote/pocket.ts (1)
extractPocketFromCellMetadata(42-44)
🔇 Additional comments (21)
src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts (1)
237-237: Verify progress calculation change is intentional.Calculation changed from
(i + 1) / lengthtoi / length. Now shows 0% for first block and never reaches 100% during execution (max is(n-1)/n %). Confirm this matches intended progress semantics.src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts (1)
15-15: Verify removal of localization against coding guidelines.The l10n import was removed and all user-facing strings in this file are now hard-coded English. This conflicts with the coding guideline "Use
l10n.t()for user-facing strings."Confirm this is intentional as part of the broader localization removal described in the PR.
As per coding guidelines.
src/notebooks/deepnote/deepnoteActivationService.ts (1)
18-21: LGTM.Constructor correctly simplified to remove integration manager dependency, aligning with broader integration infrastructure removal.
src/notebooks/deepnote/deepnoteActivationService.unit.test.ts (3)
18-18: LGTM.Test correctly updated for two-argument constructor.
78-79: LGTM.Service instances correctly instantiated with updated constructor signature.
104-105: LGTM.Constructor calls properly updated.
src/platform/webviews/webviewHost.ts (1)
261-261: No issues found—removed localization keys verified absent from codebase.The search across all TypeScript and JavaScript files returned zero matches for the seven removed keys. Webview code contains no references to the deprecated localization strings.
package.json (1)
1834-1834: LGTM.Condition change ensures Deepnote explorer only shows when folders are open.
.github/workflows/ci.yml (1)
169-192: Spell-check job structure is sound.Job mirrors the existing CI pattern (checkout → setup → install → run script) and correctly keeps spell-check out of pre-commit hooks per learnings.
Based on learnings
.gitignore (1)
1-74: LGTM.Comprehensive reorganization covers build outputs, dependencies, test artifacts, logs, and temporary files. Negations for
!yarn.lock,!build/, and!src/test/pythonEnvironments/**/*.*are preserved correctly.src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (1)
21-22: Well-handled concurrent installation prevention.The
pendingInstallationsmap correctly prevents multiple simultaneous installs to the same venv, with proper double-check and cleanup logic.Also applies to: 70-113
src/standalone/intellisense/resolveCompletionItem.unit.test.ts (1)
549-549: Direct function reference is fine.Single-arg function; extra map args are ignored. No behavior change.
src/kernels/execution/cellExecutionMessageHandler.ts (2)
1149-1153: Update‑display path aligned.Translator usage matches simplified signature; good.
637-637: API shift verified: all 19 call sites use single-arg signature.Every invocation of
cellOutputToVSCCellOutput()passes exactly one argument. No multi-arg calls remain. The refactor is consistent across production, tests, and helpers.src/kernels/execution/helpers.ts (5)
118-129: Mapper typed to single‑arg translators: good.Cleaner public surface; matches call‑site refactors.
129-161: cellOutputToVSCCellOutput refactor: good.Fallback to display_data is acceptable; logs warn.
165-188: Metadata builder is sound.Deep‑clones output.metadata and carries transient/executionCount.
203-236: Display data translation OK.Single‑arg path; preserves metadata and item ordering.
545-556: Error translation preserves original error.Attaches originalError and uses unified metadata. Good.
src/kernels/execution/cellExecutionCreator.ts (2)
65-76: Review comment is satisfied; all real executions properly set clearOutputOnStartWithTime.Call sites correctly differentiate: kernel executions (vscodeNotebookController.ts:529, cellExecution.ts:126-129 for non-resumed) pass
true; error-handling and temporary executions (helpers.ts:771, interactiveWindow.ts:259) correctly default tofalse. No issues found.
126-147: Review comment is incorrect.VS Code's NotebookCellExecution API allows calling end() before start(). The execution gracefully transitions from Pending → Idle without rejection. The current code at line 135 is correct and requires no guard. The suggested guard based on "VS Code may reject end() pre-start" is unnecessary.
Likely an incorrect or invalid review comment.
| if npm list prettier | grep -q prettier@ ; then | ||
| echo husky - prettier is installed | ||
| else | ||
| echo husky - prettier is not installed, skipping format check | ||
| exit 0 | ||
| fi | ||
|
|
||
| # run prettier on all .ts files that are in the commit | ||
| changed=$(git diff --diff-filter=ACM --cached --name-only | grep .ts$ | tr '\n' ' ') | ||
|
|
||
| if [ -z "$changed" ]; then | ||
| echo husky - No modified files to check | ||
| exit 0 | ||
| fi | ||
|
|
||
| npx prettier $changed --check |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Pre-commit hook checks but doesn't fix formatting.
Learnings indicate "ALWAYS run npm run format-fix before committing changes," but this hook runs prettier --check (line 17), which fails the commit without auto-fixing. Either switch to prettier --write or add guidance that developers must manually run format-fix.
Based on learnings
🏁 Script executed:
#!/bin/bash
# Check actual .husky/pre-commit file
echo "=== .husky/pre-commit ==="
cat .husky/pre-commit 2>/dev/null || echo "File not found"
# Check npm scripts for format-fix and related commands
echo -e "\n=== package.json scripts (format-related) ==="
jq '.scripts | with_entries(select(.key | contains("format") or contains("prettier")))' package.json 2>/dev/null || echo "package.json not found or jq failed"
# List .husky directory contents
echo -e "\n=== .husky directory ==="
ls -la .husky/ 2>/dev/null || echo ".husky directory not found"Length of output: 1030
Pre-commit hook enforces but doesn't fix formatting.
The hook runs prettier --check without --write, failing commits with formatting issues instead of auto-fixing them. Learnings indicate developers should always run format-fix before committing. Either switch to prettier --write or add npm run format-fix as a prior step in this hook.
🤖 Prompt for AI Agents
In .husky/pre-commit around lines 2 to 17, the hook currently runs Prettier with
--check which rejects commits instead of auto-fixing formatting; change the hook
to run Prettier in write mode or invoke the repository's format-fix script
before checking. Update the sequence to first run "npm run format-fix" (or "npx
prettier --write" on the staged .ts files), then re-stage any changed files and
finally run "npx prettier --check" to verify formatting so commits get
auto-corrected rather than failing.
| fi | ||
|
|
||
| # run prettier on all .ts files that are in the commit | ||
| changed=$(git diff --diff-filter=ACM --cached --name-only | grep .ts$ | tr '\n' ' ') |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Strengthen the file pattern.
grep .ts$ matches .ts anywhere in the path (e.g., foo.ts.backup). Use grep '\.ts$' or grep -E '\.ts$' for an anchored extension match.
-changed=$(git diff --diff-filter=ACM --cached --name-only | grep .ts$ | tr '\n' ' ')
+changed=$(git diff --diff-filter=ACM --cached --name-only | grep '\.ts$' | tr '\n' ' ')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| changed=$(git diff --diff-filter=ACM --cached --name-only | grep .ts$ | tr '\n' ' ') | |
| changed=$(git diff --diff-filter=ACM --cached --name-only | grep '\.ts$' | tr '\n' ' ') |
🤖 Prompt for AI Agents
In .husky/pre-commit around line 10, the grep pattern grep .ts$ matches anywhere
in filenames (e.g., foo.ts.backup); replace it with an anchored, escaped
extension match such as grep '\.ts$' or grep -E '\.ts$' so only files ending
with .ts are selected (ensure the backslash is quoted so the shell passes the
pattern correctly).
| plugins.push(style()); | ||
| plugins.push(lessLoader()); | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Align CSS minify with dev/prod.
style() always minifies; degrade DX in dev. Pass minify based on isDevbuild.
- plugins.push(style());
+ plugins.push(style({ minify: !isDevbuild }));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| plugins.push(style()); | |
| plugins.push(lessLoader()); | |
| plugins.push(style({ minify: !isDevbuild })); | |
| plugins.push(lessLoader()); | |
🤖 Prompt for AI Agents
In build/esbuild/build.ts around lines 161 to 163, the call
plugins.push(style()) always enables CSS minification which hurts DX in
development; update the call to pass a minify option tied to the build mode
(e.g., plugins.push(style({ minify: !isDevbuild }))) so minification is disabled
when isDevbuild is true and enabled for production builds; leave lessLoader()
unchanged.
| import { CancellationToken, Uri, workspace } from 'vscode'; | ||
| import { PythonEnvironment } from '../../platform/pythonEnvironments/info'; | ||
| import { IDeepnoteToolkitInstaller, DEEPNOTE_TOOLKIT_WHEEL_URL, DEEPNOTE_TOOLKIT_VERSION } from './types'; | ||
| import { IDeepnoteToolkitInstaller, DEEPNOTE_TOOLKIT_WHEEL_URL } from './types'; |
There was a problem hiding this comment.
🧩 Analysis chain
Toolkit version upgrades won't invalidate existing venvs.
Removing version from the venv path means ensureInstalled (line 84) will reuse old venvs even after DEEPNOTE_TOOLKIT_WHEEL_URL points to a new version. Users won't get toolkit upgrades without manual cleanup.
If version-independent venvs are intentional, add explicit version checking to isToolkitInstalled or document the manual cleanup requirement. Otherwise, restore version in the venv path or add a version file check.
Verify whether toolkit version tracking is handled elsewhere:
Also applies to: 32-35
🏁 Script executed:
#!/bin/bash
# Check if version tracking exists in related files
rg -n "DEEPNOTE_TOOLKIT_VERSION" --type=ts
# Check types file for version constant
rg -nA5 "DEEPNOTE_TOOLKIT_WHEEL_URL" src/kernels/deepnote/types.tsLength of output: 937
---
🏁 Script executed:
```shell
#!/bin/bash
# Get the actual implementation of key methods in deepnoteToolkitInstaller.node.ts
rg -nA10 "private.*getVenvPath\|protected.*getVenvPath\|public.*getVenvPath" src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
echo "---"
# Check isToolkitInstalled implementation
rg -nA15 "isToolkitInstalled" src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
echo "---"
# Check ensureInstalled to see version logic
rg -nA20 "ensureInstalled" src/kernels/deepnote/deepnoteToolkitInstaller.node.ts | head -30
Length of output: 289
---
🏁 Script executed:
```shell
#!/bin/bash
# Find all .node.ts files in deepnote directory
fd "\.node\.ts$" src/kernels/deepnote/
echo "---"
# List all files in deepnote directory
fd "." src/kernels/deepnote/ --max-depth 1 --type f
echo "---"
# Search for any file with "deepnote" and "Installer" in name
fd "Installer" src/kernels/deepnote/
Length of output: 334
🏁 Script executed:
#!/bin/bash
# Find all files in deepnote directory
find src/kernels/deepnote -maxdepth 1 -type f -name "*.ts"
echo "---"
# Check directory structure
ls -la src/kernels/deepnote/ | head -20Length of output: 763
🏁 Script executed:
#!/bin/bash
# Get line 7 and surrounding context (imports)
echo "=== IMPORTS (lines 1-15) ==="
head -15 src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
echo -e "\n=== getVenvPath method (lines 32-45) ==="
sed -n '32,45p' src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
echo -e "\n=== ensureInstalled method (lines 82-100) ==="
sed -n '82,100p' src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
echo -e "\n=== isToolkitInstalled method (lines 260-280) ==="
sed -n '260,280p' src/kernels/deepnote/deepnoteToolkitInstaller.node.tsLength of output: 3462
🏁 Script executed:
#!/bin/bash
# Get full getVenvHash implementation
sed -n '276,295p' src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
echo -e "\n=== Check if DEEPNOTE_TOOLKIT_WHEEL_URL is used anywhere else in file ==="
grep -n "DEEPNOTE_TOOLKIT_WHEEL_URL" src/kernels/deepnote/deepnoteToolkitInstaller.node.tsLength of output: 1212
Toolkit version upgrades won't invalidate existing venvs.
Removing DEEPNOTE_TOOLKIT_VERSION from imports eliminates version tracking. The venv hash (line 276) depends only on the file path—not the toolkit version or DEEPNOTE_TOOLKIT_WHEEL_URL. When the URL updates to a new version, ensureInstalled (line 84) reuses the existing venv because isToolkitInstalled (line 262) only checks module presence, not version. Users remain on stale toolkit versions without manual cleanup.
Fix: Include the toolkit URL/version in the venv hash, or add version validation to isToolkitInstalled.
| const output = cellOutputToVSCCellOutput({ | ||
| output_type: 'stream', | ||
| name: msg.content.name, | ||
| text: msg.content.text | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Normalize stream text across all branches.
Apply CR/backspace fixes consistently to avoid mismatched rendering.
@@
- const output = cellOutputToVSCCellOutput({
+ const output = cellOutputToVSCCellOutput({
output_type: 'stream',
name: msg.content.name,
- text: msg.content.text
+ text: formatStreamText(concatMultilineString(msg.content.text))
});
@@
- const output = cellOutputToVSCCellOutput({
+ const output = cellOutputToVSCCellOutput({
output_type: 'stream',
name: msg.content.name,
- text
+ text: formatStreamText(text)
});
@@
- const output = cellOutputToVSCCellOutput({
+ const output = cellOutputToVSCCellOutput({
output_type: 'stream',
name: msg.content.name,
text
});Also applies to: 1016-1020, 1027-1031
🤖 Prompt for AI Agents
In src/kernels/execution/cellExecutionMessageHandler.ts around lines 1006-1010
(and also apply to 1016-1020 and 1027-1031), the stream branch creates the VS
Code cell output using msg.content.text directly which results in inconsistent
handling of carriage returns/backspaces across branches; normalize the stream
text in all branches by running the same CR/backspace sanitization function (the
one used elsewhere in this file) on msg.content.text before passing it to
cellOutputToVSCCellOutput so every branch produces the same cleaned string and
rendering behavior.
| const cells = this.converter.convertBlocksToCells(selectedNotebook.blocks); | ||
|
|
||
| logger.debug(`DeepnoteSerializer: Converted ${cells.length} cells from notebook blocks`); | ||
| console.log(`Converted ${cells.length} cells from notebook blocks.`); |
There was a problem hiding this comment.
Violates coding guidelines: use ILogger service.
Coding guidelines require ILogger service instead of console.log.
As per coding guidelines.
🤖 Prompt for AI Agents
In src/notebooks/deepnote/deepnoteSerializer.ts around line 67, replace the
console.log call with the project's ILogger usage: remove console.log and call
the ILogger instance (e.g., logger.info(...)) with the same message; if there is
no local logger variable, import or accept the ILogger service used across the
codebase (or obtain it via the existing DI/getLogger helper) and use
logger.info(`Converted ${cells.length} cells from notebook blocks.`); ensure no
other console.* calls remain in this file.
| import type { NotebookCellData } from 'vscode'; | ||
|
|
||
| import type { DeepnoteBlock } from './deepnoteTypes'; | ||
| import type { DeepnoteBlock, DeepnoteOutput } from './deepnoteTypes'; | ||
| import { generateBlockId, generateSortingKey } from './dataConversionUtils'; |
There was a problem hiding this comment.
Default block type should reflect cell.kind.
Currently defaults to 'code' even for markup cells without a pocket; leads to incorrect conversions.
-import type { NotebookCellData } from 'vscode';
+import type { NotebookCellData } from 'vscode';
+import { NotebookCellKind } from 'vscode';
...
- type: pocket?.type || 'code'
+ type: pocket?.type || (cell.kind === NotebookCellKind.Markup ? 'markdown' : 'code')Would you like a follow-up PR to add the corresponding unit test (see suggested test in pocket.unit.test.ts)?
Also applies to: 64-67
🤖 Prompt for AI Agents
In src/notebooks/deepnote/pocket.ts around lines 1-4 and also lines 64-67, the
function currently defaults block.type to 'code' even for markup cells without a
pocket; change the default to reflect the NotebookCellData.kind so markup cells
get 'markup' and code cells get 'code'. Update the logic to inspect cell.kind
(e.g., NotebookCellKind.Markup vs code) and set block.type = 'markup' for markup
cells and 'code' otherwise, and mirror the same fix for the code paths at lines
64-67 so both creation sites use the cell.kind-aware default.
| **Key functions:** | ||
| - `deepnoteDataConverter.ts::convertBlocksToCells(blocks)` - Main entry point | ||
| - `pocket.ts::addPocketToCellMetadata(cell)` - Creates the pocket | ||
| - `deepnoteDataConverter.ts::transformOutputsForVsCode(outputs)` - Converts outputs | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Doc aligns with implementation; add one caution.
Note that storing outputs in the pocket can be large; prefer keeping only round‑trip‑critical fields if size becomes an issue.
Also applies to: 79-83
🤖 Prompt for AI Agents
In src/notebooks/deepnote/serialization.md around lines 52-56 (and similarly
79-83), the docs mention storing outputs in the pocket but lack a caution about
size; update the documentation to warn that output payloads can be large and
recommend storing only round-trip-critical fields (e.g., output type, minimal
metadata, hashes or references) or truncating/omitting bulky data (full
binary/blobs, large text) when size is a concern, and add a brief example
sentence describing this trade-off and a pointer to where to list the specific
fields to keep.
| export interface IDeepnoteNotebookManager { | ||
| getCurrentNotebookId(projectId: string): string | undefined; | ||
| getOriginalProject(projectId: string): DeepnoteProject | undefined; | ||
| getOriginalProject(projectId: string): unknown | undefined; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify callers handle unknown return type.
Changed from DeepnoteProject | undefined to unknown | undefined. Confirm all call sites of getOriginalProject handle the opaque type correctly.
🏁 Script executed:
#!/bin/bash
# Find all usages of getOriginalProject
rg -n -C5 'getOriginalProject\(' --type=ts -g '!*.unit.test.ts'Length of output: 148
🏁 Script executed:
#!/bin/bash
# Check if file exists and search for getOriginalProject more broadly
echo "=== Checking file existence ==="
fd -t f "types.ts" | grep notebooks
echo -e "\n=== Search for getOriginalProject without type filter ==="
rg -n 'getOriginalProject' --glob '*.ts' --glob '*.tsx' -C3
echo -e "\n=== Search in entire repo without filters ==="
rg -n 'getOriginalProject' -C3Length of output: 15724
Interface-implementation type mismatch requires resolution.
The interface signature changed to return unknown | undefined, but the implementation in src/notebooks/deepnote/deepnoteNotebookManager.ts:31 still returns DeepnoteProject | undefined. All three call sites compensate with explicit as DeepnoteProject | undefined casts, so functionality works—but this violates the type contract and requires casting workarounds.
Either update the interface to match the implementation (DeepnoteProject | undefined) or align the implementation with the interface (unknown | undefined).
🤖 Prompt for AI Agents
In src/notebooks/types.ts around line 30 the getOriginalProject signature
returns unknown | undefined which mismatches the implementation in
src/notebooks/deepnote/deepnoteNotebookManager.ts (returns DeepnoteProject |
undefined) and forces callers to cast; update the interface to return
DeepnoteProject | undefined to match the implementation and remove the need for
casts, or alternatively change the deepnoteNotebookManager method to return
unknown | undefined and update its callers to handle/validate the unknown;
preferred fix: change the types.ts signature to DeepnoteProject | undefined so
it aligns with the manager implementation and remove the explicit casts at call
sites.
| getTheSelectedNotebookForAProject(projectId: string): string | undefined; | ||
| selectNotebookForProject(projectId: string, notebookId: string): void; | ||
| storeOriginalProject(projectId: string, project: DeepnoteProject, notebookId: string): void; | ||
| storeOriginalProject(projectId: string, project: unknown, notebookId: string): void; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify callers can pass unknown type.
Changed from project: DeepnoteProject to project: unknown. Ensure call sites are updated or their types are compatible.
🏁 Script executed:
#!/bin/bash
# Find all usages of storeOriginalProject
rg -n -C5 'storeOriginalProject\(' --type=ts -g '!*.unit.test.ts'Length of output: 148
🏁 Script executed:
#!/bin/bash
# Broader search: find all TypeScript files first to verify structure
fd -e ts -e tsx | head -20Length of output: 667
🏁 Script executed:
#!/bin/bash
# Search for storeOriginalProject without restrictive filters
rg 'storeOriginalProject' --type ts -nLength of output: 2626
🏁 Script executed:
#!/bin/bash
# Check the actual interface definition in the file mentioned
cat src/notebooks/types.ts | head -50Length of output: 1802
Implementation parameter type does not match interface declaration.
The interface in src/notebooks/types.ts:33 declares project: unknown, but the implementation in src/notebooks/deepnote/deepnoteNotebookManager.ts:63 still has project: DeepnoteProject. Update the implementation's parameter type to unknown to match the interface signature.
🤖 Prompt for AI Agents
In src/notebooks/types.ts around line 33 and
src/notebooks/deepnote/deepnoteNotebookManager.ts at line 63, the interface
declares storeOriginalProject(projectId: string, project: unknown, notebookId:
string): void but the implementation still types project as DeepnoteProject;
change the implementation signature to use project: unknown to match the
interface, update any internal usages to safely narrow or validate the unknown
(with type guards or casts) before accessing properties, and run TypeScript
build to ensure no other signature mismatches remain.
- Restore integration-related files and imports from main - Accept main's new integration panel localization strings - Restore SqlCellStatusBarProvider registration - Restore missing integrationDetector.ts file
|
Closing this PR in favor of #87 which has a clean spell-check implementation without merge conflict artifacts. The new PR:
All spell-check related CI checks are passing in #87. |
Add CSpell Configuration for Deepnote-Specific Code Only
This PR implements spell-checking using CSpell, but restricts it to only Deepnote-specific code rather than the entire forked Microsoft/vscode-jupyter codebase.
Changes
Spell-check scope restriction:
src/notebooks/deepnote/,src/kernels/deepnote/, anddeepnoteServerNotFoundError.tsConfiguration files:
cspell.jsonwith minimal 28-word technical dictionaryspell-checkandspell-check:fixcspell@9.2.1as devDependencyLicense handling:
@cspell/dict-en-common-misspellings@2.1.6from license checker (has "Custom" license)🔍 Critical Review Points
Testing
Link to Devin run: https://app.devin.ai/sessions/f834d045e38442e88bff0556346e3a2b
Requested by: @jamesbhobbs
Summary by CodeRabbit
New Features
Bug Fixes & Improvements
Removed Features
Chores