Add biome config from deepnote/deepnote #36
Conversation
- Add @biomejs/biome@2.2.5 dependency - Add biome.json with exact config from deepnote/deepnote - Apply biome formatting and linting fixes to all source files - Configure .prettierignore to exclude TS/TSX files (formatted by biome) - Add override for TSX files to keep React imports as values - Add .npmrc for GitHub packages authentication - Fix type issues in handler.ts (replace any with proper types)
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
⚙️ Control Options:
|
📝 WalkthroughWalkthroughThis PR integrates Biome as the project's linter and formatter, replacing jlpm with yarn in the pre-commit hook, and standardizes code style throughout. It converts type-only imports where applicable, removes console.error logging paths, adds .npmrc for GitHub Packages authentication, and adjusts block-type conversion logic so more Deepnote blocks map to markdown instead of code. Primarily tooling and stylistic changes with minimal logic alterations. Possibly related PRs
Suggested reviewers
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
Resolved conflicts by: - Accepting new code from main - Applying biome formatting to all conflicted files - Fixed non-null assertion in NotebookPicker.tsx - Updated handler.ts to match new signature (returns unknown) - Updated pre-commit hook to use yarn instead of jlpm
|
React needs to be imported as a value for JSX when tsconfig jsx is set to 'react'
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (15)
.husky/pre-commit(1 hunks).npmrc(1 hunks).prettierignore(1 hunks)biome.json(1 hunks)package.json(1 hunks)src/__tests__/jupyterlab_deepnote.spec.ts(1 hunks)src/components/NotebookPicker.tsx(2 hunks)src/convert-deepnote-block-to-jupyter-cell.ts(1 hunks)src/convert-deepnote-block-type-to-jupyter.ts(1 hunks)src/deepnote-content-provider.ts(3 hunks)src/fallback-data.ts(2 hunks)src/handler.ts(2 hunks)src/index.ts(3 hunks)src/transform-deepnote-yaml-to-notebook-content.ts(1 hunks)src/types.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
PR: deepnote/deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-14T13:43:36.364Z
Learning: Applies to **/*.{ts,tsx} : Follow Biome's lint rules as configured in biome.json
📚 Learning: 2025-09-25T14:43:13.976Z
Learnt from: FilipPyrek
PR: deepnote/deepnote#18531
File: biome.json:58-58
Timestamp: 2025-09-25T14:43:13.976Z
Learning: Biome is a JavaScript/TypeScript linter and formatter that doesn't process .proto files, so Protocol Buffer source files don't need to be explicitly ignored in biome.json configuration.
Applied to files:
biome.json
📚 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 **/*.{ts,tsx} : Follow Biome's lint rules as configured in biome.json
Applied to files:
biome.json
📚 Learning: 2025-09-14T18:25:57.101Z
Learnt from: jamesbhobbs
PR: deepnote/deepnote#18401
File: scripts/check-chromatic-enabled/pr-template.test.ts:1-6
Timestamp: 2025-09-14T18:25:57.101Z
Learning: In the deepnote/deepnote repository, colocated tests use Node's built-in test runner (node:test, node:assert/strict) rather than Jest.
Applied to files:
src/__tests__/jupyterlab_deepnote.spec.ts
📚 Learning: 2025-10-09T11:21:57.494Z
Learnt from: CR
PR: deepnote/vscode-deepnote#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-09T11:21:57.494Z
Learning: Applies to src/notebooks/deepnote/deepnoteNotebookSelector.ts : deepnoteNotebookSelector.ts implements UI selection logic for Deepnote notebooks
Applied to files:
src/components/NotebookPicker.tsx
📚 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/components/NotebookPicker.tsx
📚 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/fallback-data.tssrc/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/deepnoteDataConverter.ts : deepnoteDataConverter.ts performs Deepnote data transformations
Applied to files:
src/convert-deepnote-block-to-jupyter-cell.tssrc/convert-deepnote-block-type-to-jupyter.tssrc/transform-deepnote-yaml-to-notebook-content.tssrc/types.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 : On conversion failures, provide detailed error reporting with suggested fixes
Applied to files:
src/transform-deepnote-yaml-to-notebook-content.ts
📚 Learning: 2025-10-15T09:33:59.409Z
Learnt from: andyjakubowski
PR: deepnote/jupyterlab-deepnote#27
File: src/deepnote-content-provider.ts:8-17
Timestamp: 2025-10-15T09:33:59.409Z
Learning: In the jupyterlab-deepnote project, basic Zod schema validation without `.strict()` mode or datetime refinement is preferred for API response validation in TypeScript files.
Applied to files:
src/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/deepnoteSerializer.ts : deepnoteSerializer.ts is the main serializer orchestrating Deepnote integration
Applied to files:
src/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/** : Deepnote integration code resides under src/notebooks/deepnote/
Applied to files:
.prettierignore
🧬 Code graph analysis (5)
src/components/NotebookPicker.tsx (1)
src/types.ts (1)
deepnoteMetadataSchema(5-14)
src/convert-deepnote-block-to-jupyter-cell.ts (1)
src/convert-deepnote-block-type-to-jupyter.ts (1)
convertDeepnoteBlockTypeToJupyter(1-20)
src/deepnote-content-provider.ts (2)
src/handler.ts (1)
requestAPI(12-49)src/transform-deepnote-yaml-to-notebook-content.ts (1)
transformDeepnoteYamlToNotebookContent(7-53)
src/index.ts (2)
src/deepnote-content-provider.ts (2)
DeepnoteContentProvider(19-54)deepnoteContentProviderName(6-6)src/components/NotebookPicker.tsx (1)
NotebookPicker(9-103)
src/transform-deepnote-yaml-to-notebook-content.ts (4)
src/types.ts (2)
IDeepnoteNotebookContent(29-31)IDeepnoteNotebookMetadata(16-27)src/convert-deepnote-block-to-jupyter-cell.ts (1)
convertDeepnoteBlockToJupyterCell(6-40)src/fallback-data.ts (2)
blankDeepnoteNotebookContent(12-29)blankCodeCell(4-10)src/kernel-metadata-fallback.ts (1)
kernelMetadataFallback(2-20)
🔇 Additional comments (21)
src/convert-deepnote-block-type-to-jupyter.ts (1)
1-20: Mapping change is correct; example.deepnote uses only 'markdown' and 'code' types—both map correctly.The removed block types don't appear in the provided example. Current mapping ('markdown' → 'markdown', 'code' → 'code') aligns with Jupyter semantics. However, no formal test coverage exists; confirm via manual testing with real .deepnote files that no unexpected block types break rendering.
.husky/pre-commit (1)
1-1: LGTM – verify yarn availability.The switch from
jlpmtoyarnis fine. Note that package.json scripts still usejlpmfor builds/lints; ensure dev environments have yarn installed..prettierignore (1)
8-12: LGTM – Biome now handles TS/JS formatting.Excluding TypeScript/JavaScript files from Prettier is correct given Biome adoption.
.npmrc (1)
1-2: LGTM – ensure GITHUB_TOKEN is documented.Standard GitHub Packages auth. Verify that
GITHUB_TOKENis set in all environments (local dev, CI) and document setup steps for contributors.src/transform-deepnote-yaml-to-notebook-content.ts (2)
1-5: LGTM – type-only imports are correct.
7-49: LGTM – formatting changes preserve logic.src/types.ts (2)
1-3: LGTM – type-only import optimization correct.
5-31: LGTM – formatting changes only.src/fallback-data.ts (2)
1-2: LGTM – type-only imports correct.
4-29: LGTM – formatting changes only.src/components/NotebookPicker.tsx (5)
1-7: LGTM – type-only imports and React import correct.
9-23: LGTM – constructor logic preserved.
25-57: LGTM – removed console.error is intentional.Silent failure on invalid metadata aligns with PR objectives (console logging cleanup). Error handling preserved via early return.
59-66: Good defensive guard added.The
if (this.parent)check prevents potential errors if parent is undefined. Nice improvement.
68-102: LGTM – render logic preserved.package.json (1)
74-74: Consider updating Biome to 2.2.6 or verify compatibility.Version 2.2.6 is now available (latest). Version 2.2.5 is secure and not deprecated, with no known vulnerabilities, but is outdated. Evaluate whether updating is appropriate for your use case.
src/deepnote-content-provider.ts (1)
20-27: Non-.deepnote fast‑path looks good.src/__tests__/jupyterlab_deepnote.spec.ts (1)
7-9: Pure formatting. LGTM.src/index.ts (3)
1-4: Type-only imports are correct.All converted imports (JupyterFrontEnd, JupyterFrontEndPlugin, NotebookPanel, ContentsManager) are used exclusively as types in this file.
22-24: Verify silent failure doesn't hide real issues.The early return without logging means the extension will silently fail to register .deepnote support if the registry is unavailable. Confirm this is acceptable behavior and won't mask configuration problems during extension loading.
As noted in the PR objectives, removal of console.error is intentional. Consider testing scenarios where contentProviderRegistry might be missing (older JupyterLab versions, initialization timing issues).
25-58: LGTM! Formatting changes only.The plugin registration logic remains intact. All changes are Biome formatting (trailing commas, spacing).
| "includes": ["*.tsx"], | ||
| "linter": { | ||
| "rules": { | ||
| "style": { | ||
| "useImportType": "off" | ||
| } |
There was a problem hiding this comment.
TSX override glob misses nested files.
"*.tsx" only matches root-level files. Use a recursive glob to apply the rule repo‑wide; otherwise TSX under src/** won’t get the intended useImportType override.
Apply this diff:
- "includes": ["*.tsx"],
+ "includes": ["**/*.tsx"],🤖 Prompt for AI Agents
In biome.json around lines 124 to 129, the TSX override glob currently uses
"*.tsx" which only matches root-level files; update the glob to a recursive
pattern like "**/*.tsx" so the linter override (useImportType: off) applies to
TSX files in nested directories throughout the repo.
| if (Array.isArray(blockOutputs)) { | ||
| blockOutputs.forEach(output => { | ||
| delete output.truncated; | ||
| }); | ||
| delete output.truncated | ||
| }) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Comply with Biome style.useForOf.
forEach will violate the configured rule. Use for‑of.
- if (Array.isArray(blockOutputs)) {
- blockOutputs.forEach(output => {
- delete output.truncated
- })
- }
+ if (Array.isArray(blockOutputs)) {
+ for (const output of blockOutputs) {
+ // Remove Deepnote-only flag
+ // @ts-expect-error property may not exist on Jupyter outputs
+ delete (output as any).truncated
+ }
+ }📝 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.
| if (Array.isArray(blockOutputs)) { | |
| blockOutputs.forEach(output => { | |
| delete output.truncated; | |
| }); | |
| delete output.truncated | |
| }) | |
| } | |
| if (Array.isArray(blockOutputs)) { | |
| for (const output of blockOutputs) { | |
| // Remove Deepnote-only flag | |
| // @ts-expect-error property may not exist on Jupyter outputs | |
| delete (output as any).truncated | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/convert-deepnote-block-to-jupyter-cell.ts around lines 14 to 18, the code
uses blockOutputs.forEach which violates the Biome style.useForOf rule; replace
the forEach with a for-of loop (e.g., for (const output of blockOutputs) {
delete output.truncated }) and keep the existing Array.isArray check and
null/undefined safety so types remain correct and behavior unchanged.
| cell_type: 'markdown', | ||
| metadata: {}, | ||
| source | ||
| }; | ||
| return jupyterCell; | ||
| source, | ||
| } |
There was a problem hiding this comment.
Preserve metadata (incl. cell_id) for markdown cells.
You build jupyterCellMetadata but drop it for markdown. Keep consistency and IDs.
- const jupyterCell: IMarkdownCell = {
+ const jupyterCell: IMarkdownCell = {
cell_type: 'markdown',
- metadata: {},
+ metadata: jupyterCellMetadata,
source,
}🤖 Prompt for AI Agents
In src/convert-deepnote-block-to-jupyter-cell.ts around lines 34 to 37, the
function constructs jupyterCellMetadata but currently drops it when creating
markdown cells (returns metadata: {}); update the markdown branch to set
metadata: jupyterCellMetadata instead of an empty object so markdown cells keep
their full metadata (including cell_id) to match code cell behavior and preserve
IDs.
| throw new Error('Invalid API response shape') | ||
| } |
There was a problem hiding this comment.
Surface Zod error details.
Provide the parse error to aid debugging.
- if (!parsed.success) {
- throw new Error('Invalid API response shape')
+ if (!parsed.success) {
+ throw new Error(`Invalid API response shape: ${parsed.error.message}`)📝 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.
| throw new Error('Invalid API response shape') | |
| } | |
| throw new Error(`Invalid API response shape: ${parsed.error.message}`) | |
| } |
🤖 Prompt for AI Agents
In src/deepnote-content-provider.ts around lines 33-34, the code throws a
generic Error('Invalid API response shape'); change it to surface the Zod parse
error details by catching the ZodError (or the thrown parse error) and
rethrowing or throwing a new Error that includes the parse error information
(e.g. error.message or a formatted/flattened error representation) so callers
and logs receive the specific validation failure details.
| throw new ServerConnection.ResponseError( | ||
| response, | ||
| typeof parsed === 'object' && parsed !== null && 'message' in parsed ? parsed.message : parsed | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
Ensure ResponseError reason is a string.
Passing objects/numbers yields “[object Object]” or non‑helpful output. Stringify fallback.
- throw new ServerConnection.ResponseError(
- response,
- typeof parsed === 'object' && parsed !== null && 'message' in parsed ? parsed.message : parsed
- )
+ throw new ServerConnection.ResponseError(
+ response,
+ typeof parsed === 'object' && parsed !== null && 'message' in parsed && typeof (parsed as any).message === 'string'
+ ? (parsed as any).message
+ : typeof parsed === 'string'
+ ? parsed
+ : JSON.stringify(parsed)
+ )Scan for callers assuming a string return to avoid regressions:
🏁 Script executed:
#!/bin/bash
rg -nP -C2 'requestAPI\s*\(' -g '!**/node_modules/**'Length of output: 736
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Get full context of requestAPI function and error handling
cat src/handler.ts | head -50Length of output: 1317
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Verify TypeScript strict checking and parsed type behavior
cat tsconfig.json 2>/dev/null || echo "tsconfig not found"Length of output: 697
🏁 Script executed:
#!/bin/bash
# Check if parsed.message property access could be string-guarded elsewhere
rg -nP 'parsed\.message' src/ -g '!**/node_modules/**'Length of output: 122
Add type guards to ensure ResponseError receives string message.
The code can pass non-string values (parsed.message could be number/boolean, parsed could be non-string JSON) to ResponseError, resulting in unhelpful "[object Object]" error messages. The suggested fix properly guards the string type and stringifies non-string fallbacks.
throw new ServerConnection.ResponseError(
response,
typeof parsed === 'object' && parsed !== null && 'message' in parsed && typeof (parsed as any).message === 'string'
? (parsed as any).message
: typeof parsed === 'string'
? parsed
: JSON.stringify(parsed)
)📝 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.
| throw new ServerConnection.ResponseError( | |
| response, | |
| typeof parsed === 'object' && parsed !== null && 'message' in parsed ? parsed.message : parsed | |
| ) | |
| } | |
| throw new ServerConnection.ResponseError( | |
| response, | |
| typeof parsed === 'object' && parsed !== null && 'message' in parsed && typeof (parsed as any).message === 'string' | |
| ? (parsed as any).message | |
| : typeof parsed === 'string' | |
| ? parsed | |
| : JSON.stringify(parsed) | |
| ) |
🤖 Prompt for AI Agents
In src/handler.ts around lines 34 to 38, the ResponseError is currently passed
values that may be non-strings which yields unhelpful "[object Object]"
messages; change the logic to ensure a string is always provided by: check if
parsed is an object with a 'message' property that is a string and use that;
else if parsed itself is a string use it; otherwise fallback to
JSON.stringify(parsed) (or String(parsed) if JSON fails), also guard against
null/undefined so the final argument is always a sensible string.
| } catch (_error) {} | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Drop eslint-disable; use empty catch block explicitly.
Keeps linters quiet without a directive.
- // eslint-disable-next-line no-empty
- } catch (_error) {}
+ } catch {
+ // Non‑JSON; fall through to text handling
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/handler.ts around lines 41-42, remove any eslint-disable comment and
replace the current catch (_error) {} with an explicit empty catch block (catch
{}) so the code silently ignores errors without introducing an unused variable
or relying on an eslint-disable directive; ensure no eslint-disable comments
remain for that block.
| } catch (_error) { | ||
| throw new Error('Failed to transform Deepnote YAML to notebook content.') |
There was a problem hiding this comment.
Error context lost – preserve original error.
Removing console.error is fine, but discarding the original error loses stack traces and context. Use Error cause to chain exceptions.
- } catch (_error) {
- throw new Error('Failed to transform Deepnote YAML to notebook content.')
+ } catch (error) {
+ throw new Error('Failed to transform Deepnote YAML to notebook content.', { cause: error })
}📝 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.
| } catch (_error) { | |
| throw new Error('Failed to transform Deepnote YAML to notebook content.') | |
| } catch (error) { | |
| throw new Error('Failed to transform Deepnote YAML to notebook content.', { cause: error }) | |
| } |
🤖 Prompt for AI Agents
In src/transform-deepnote-yaml-to-notebook-content.ts around lines 50 to 51, the
catch block currently swallows the original error and throws a new generic
Error; change it to capture the caught value (e.g., rename _error to error) and
rethrow a new Error that preserves the original error as its cause using the
Error constructor options (throw new Error('Failed to transform Deepnote YAML to
notebook content.', { cause: error })), ensuring the original stack/context is
preserved; if the caught value may not be an Error, normalize it to an Error
before passing as cause.
Add biome config from deepnote/deepnote
Summary
This PR adds Biome linting and formatting configuration to jupyterlab-deepnote, matching the exact setup from the main deepnote/deepnote repository. The changes include:
@biomejs/biome@2.2.5dependency (matching deepnote/deepnote version)biome.jsonconfiguration file with identical rules.prettierignoreto exclude TS/TSX/JS/JSX files (now formatted by Biome only)handler.tsby replacinganytypes with proper types.npmrcfor GitHub packages authenticationReview & Testing Checklist for Human
Manual testing required - I was unable to run the extension in JupyterLab. Please test that:
.deepnotefiles can still be opened and viewedConsole statement removal - Biome removed several
console.logandconsole.errorstatements (seesrc/handler.ts:37,src/deepnote-content-provider.ts:37,src/components/NotebookPicker.tsx:39,src/index.ts:34,src/transform-deepnote-yaml-to-notebook-content.ts:53). Verify this is acceptable or if we need to add them back as structured logging.Type changes in handler.ts - Changed
error as any→error as Erroranddata: any→data: string. Verify error handling still works correctly, especially for non-standard error types.Developer workflow - Prettier now skips TypeScript files. Ensure this doesn't break any existing CI checks or developer tools. Run
yarn lint:checkandyarn testto verify.Biome configuration - The TSX override (
useImportType: "off") is needed for JSX files withjsx: "react"in tsconfig. This is intentional but worth noting as a deviation from the base deepnote config.Notes
Link to Devin run: https://app.devin.ai/sessions/ed2fee7690e04136ae4092f84ec03506
Requested by: James Hobbs (@jamesbhobbs)
Summary by CodeRabbit
Bug Fixes
Refactor