feat(@deepnote/convert): export detectFormat and NotebookFormat #210
Conversation
📝 WalkthroughWalkthroughThis pull request adds a new format detection module exporting 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🔇 Additional comments (4)
✏️ Tip: You can disable this entire section by setting Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #210 +/- ##
==========================================
+ Coverage 91.52% 91.57% +0.04%
==========================================
Files 40 41 +1
Lines 2029 2041 +12
Branches 623 630 +7
==========================================
+ Hits 1857 1869 +12
Misses 172 172 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/convert/src/cli.ts (1)
143-159: Potential DRY opportunity.This logic mirrors
detectFormat. Could delegate to it and catch its errors for custom messaging. Optional since current approach is clear.
🤖 Fix all issues with AI agents
In `@packages/convert/src/format-detection.test.ts`:
- Around line 112-147: Add a test case to the existing "Python file detection"
suite that verifies precedence when both markers exist: call
detectFormat('file.py', content) where content includes both a Marimo marker
(e.g., import marimo or `@app.cell`) and a percent marker (# %%), and assert the
result is 'marimo' to document that detectFormat prefers Marimo over percent;
add the test named like "prefers Marimo when both formats are present" alongside
the other .py tests.
In `@packages/convert/src/format-detection.ts`:
- Line 26: The variable named ext is misleading because it contains the full
lowercase filename rather than an extension; rename ext to a clearer identifier
like lowerFilename (or lowercasedFilename) wherever it is declared and used in
format-detection.ts (the const ext = filename.toLowerCase() line and all
subsequent references) and ensure any logic that actually extracts the extension
uses a separate, correctly named variable (e.g., ext =
path.extname(lowerFilename) or similar).
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
packages/blocks/package.jsonpackages/convert/src/cli.tspackages/convert/src/format-detection.test.tspackages/convert/src/format-detection.tspackages/convert/src/index.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Place test files next to source files with.test.tsor.test.tsxextension
Use strict type checking in TypeScript files
Prefer type safety over convenience in TypeScript
Useconstfor immutable values
Avoidanytypes - use proper type definitions
Follow Biome's rules (configured inbiome.json)
Use literal keys instead of bracket notation when possible
Prefer single quotes for strings (except when avoiding escapes)
Keep code clean and readable following Biome standards
Files:
packages/convert/src/index.tspackages/convert/src/cli.tspackages/convert/src/format-detection.tspackages/convert/src/format-detection.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Use Vitest as the testing framework
Follow existing test patterns in the codebase (seepackages/blocks/src/blocks/*.test.ts)
Test edge cases, error handling, and special characters
Files:
packages/convert/src/format-detection.test.ts
🧬 Code graph analysis (1)
packages/convert/src/format-detection.test.ts (2)
packages/convert/src/format-detection.ts (3)
isMarimoContent(4-12)isPercentContent(15-19)detectFormat(25-42)packages/convert/src/index.ts (1)
detectFormat(43-43)
🔇 Additional comments (5)
packages/blocks/package.json (1)
3-3: Unrelated version bump?This bumps
@deepnote/blocksbut the PR changes are in@deepnote/convert. Verify this is intentional.packages/convert/src/format-detection.ts (1)
1-42: Clean module design.Well-structured with clear separation of concerns. The regex heuristics for string exclusion are documented and reasonable for the use case.
packages/convert/src/format-detection.test.ts (1)
1-156: Solid test coverage.Clear structure, good edge cases for string literals. Tests align with implementation behavior.
packages/convert/src/cli.ts (1)
16-16: Good extraction.Centralizes format detection logic. Direct import from
./format-detectionavoids potential circular dependency through index.packages/convert/src/index.ts (1)
40-43: Appropriate public API.Exports only
detectFormatandNotebookFormat. If consumers needisMarimoContent/isPercentContentdirectly, consider exporting them too.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
1 participant
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.