refactor(convert): extract shared runnable-file loading & snapshot persistence by tkislan · Pull Request #408 · deepnote/deepnote

tkislan · 2026-06-17T13:31:02Z

Summary

Lifts the runnable-file loader and execution-snapshot persistence helpers that were duplicated between @deepnote/cli and @deepnote/mcp into the shared @deepnote/convert package, and points both consumers at the shared implementations. The two copies had drifted (error wrapping, return shapes, stale-executionCount handling); this unifies them.

Changes

@deepnote/convert (new shared code)
- load-runnable-file.ts — loadRunnableFile, isRunnableExtension, RUNNABLE_EXTENSIONS, LoadRunnableFileError
- snapshot/save-execution-snapshot.ts — saveExecutionSnapshot, mergeOutputsIntoFile, getSnapshotPath
- generateSnapshotFilename now takes an object parameter (output byte-identical)
- convert barrels updated to export the new groups
@deepnote/cli — format-converter.ts and output-persistence.ts become thin wrappers retaining CLI-only concerns (directory rejection, FileResolutionError, debug() logging, IOutput[]-typed public surface)
@deepnote/mcp — execution.ts delegates to the shared helpers; the local copies are removed

Behavior preservation

Behavior is preserved for existing inputs, with two deliberate, documented changes:

mergeOutputsIntoFile now retains a stale executionCount (adopts the MCP merge semantics; pinned by a new test).
Filesystem read errors keep their Node code (ENOENT/EISDIR) so MCP's error envelope is unchanged; only unsupported-extension/parse failures throw the code-less LoadRunnableFileError.

Out of scope (builds on top of this): single-notebook / init-notebook resolution, resolveSnapshotNotebookId, splitByNotebooks / the deepnote split command, filename sanitization.

Verification

pnpm build, pnpm typecheck, pnpm biome:check, and pnpm test (132 files / 2251 tests) all green.
Spot-checked directly against built dist: a missing or directory .deepnote preserves ENOENT / EISDIR through loadRunnableFile, while unsupported-extension and parse failures stay code-less — so MCP's deepnote_run error envelope (incl. code) matches main.

🤖 Generated with Claude Code

Summary by CodeRabbit

Refactor
- Centralized runnable file loading/conversion (Deepnote, Jupyter, Python, Quarto) into shared utilities used by both CLI and other tooling for consistent results.
- Standardized execution snapshot persistence by reusing shared snapshot helpers across tools.
API/Behavior Adjustments
- Execution snapshot “latest” and timestamped filenames remain unchanged; internal snapshot filename generation now uses an options-based pattern.
Tests
- Updated and expanded snapshot-merging coverage to verify execution count retention/overwriting semantics on re-runs.

…rsistence Lift the runnable-file loader and execution-snapshot helpers that were duplicated between @deepnote/cli and @deepnote/mcp into @deepnote/convert, and point both consumers at the shared implementations. The two copies had drifted (error wrapping, return shapes, stale-executionCount handling); this unifies them. - convert: new load-runnable-file.ts (loadRunnableFile, isRunnableExtension, RUNNABLE_EXTENSIONS, LoadRunnableFileError) and snapshot/save-execution-snapshot.ts (saveExecutionSnapshot, mergeOutputsIntoFile, getSnapshotPath) - generateSnapshotFilename now takes an object parameter (output byte-identical) - cli format-converter.ts / output-persistence.ts become thin wrappers that keep CLI-only concerns (directory rejection, FileResolutionError, debug logging, IOutput[]-typed public surface) - mcp execution.ts delegates to the shared helpers; local copies removed Behavior is preserved for existing inputs, with two deliberate, documented changes: - mergeOutputsIntoFile now retains a stale executionCount (MCP semantics), pinned by a new test - filesystem read errors keep their Node code (ENOENT/EISDIR) so MCP's error envelope is unchanged; only unsupported-extension/parse failures throw the code-less LoadRunnableFileError Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

coderabbitai · 2026-06-17T13:31:32Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5da86bec-f58e-4c69-b404-98346ae65f0d

📥 Commits

Reviewing files that changed from the base of the PR and between ea46f89 and 7792f62.

📒 Files selected for processing (3)

packages/cli/src/commands/run.ts
packages/cli/src/utils/format-converter.ts
packages/cli/src/utils/output-persistence.ts

📝 Walkthrough

Walkthrough

Format detection/conversion and snapshot persistence are extracted from CLI and MCP into @deepnote/convert. A new load-runnable-file.ts provides loadRunnableFile with per-extension branches for .deepnote, .ipynb, .qmd, and .py (with Python format auto-detection). A new save-execution-snapshot.ts provides mergeOutputsIntoFile, saveExecutionSnapshot, and getSnapshotPath, preserving pre-existing executionCount when re-runs supply null. generateSnapshotFilename refactored from positional to options-object parameters. CLI and MCP reduced to thin wrappers delegating to shared helpers.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested reviewers

m1so
dinohamzic

🚥 Pre-merge checks | ✅ 6

✅ Passed checks (6 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	Title clearly identifies the main refactoring: extracting shared runnable-file loading and snapshot persistence code into the convert package.
Docstring Coverage	✅ Passed	Docstring coverage is 90.91% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Updates Docs	✅ Passed	Internal refactoring PR extracting duplicated code into shared package; no user-facing features or API changes, so documentation updates not required.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

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

codecov · 2026-06-17T13:33:01Z

Codecov Report

❌ Patch coverage is 85.43689% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.35%. Comparing base (b5c95b4) to head (7792f62).

Files with missing lines	Patch %	Lines
packages/convert/src/load-runnable-file.ts	82.75%	10 Missing ⚠️
packages/cli/src/utils/output-persistence.ts	0.00%	4 Missing ⚠️
packages/cli/src/utils/format-converter.ts	87.50%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #408      +/-   ##
==========================================
+ Coverage   83.94%   84.35%   +0.41%     
==========================================
  Files         151      153       +2     
  Lines        8047     7996      -51     
  Branches     2236     2225      -11     
==========================================
- Hits         6755     6745      -10     
+ Misses       1291     1250      -41     
  Partials        1        1

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/convert/src/load-runnable-file.ts`:
- Around line 73-80: Replace the concatenated template string used in the
LoadRunnableFileError constructor with the dedent function from ts-dedent for
better readability and consistency with project guidelines. Import dedent from
ts-dedent if not already present in the file, then wrap the multiline error
message string with dedent to automatically handle proper indentation and
formatting. Apply this same refactoring to the other similar error message
construction at line 201.

In `@packages/convert/src/snapshot/save-execution-snapshot.ts`:
- Around line 108-110: The timestamp generation in the saveExecutionSnapshot
function is dropping sub-second precision by slicing the ISO string at position
19, which causes file name collisions for executions within the same second.
Modify the slice operation on the timestamp variable to preserve millisecond
precision from the ISO timestamp string after replacing the colon and period
characters, so that multiple snapshots generated in the same second will have
unique filenames.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: d9945608-72d0-4c09-a87a-dcdaf5cfc4ce

📥 Commits

Reviewing files that changed from the base of the PR and between b5c95b4 and 3e2e42b.

📒 Files selected for processing (12)

packages/cli/src/utils/format-converter.ts
packages/cli/src/utils/output-persistence.ts
packages/convert/src/index.ts
packages/convert/src/load-runnable-file.ts
packages/convert/src/snapshot/index.ts
packages/convert/src/snapshot/save-execution-snapshot.test.ts
packages/convert/src/snapshot/save-execution-snapshot.ts
packages/convert/src/snapshot/snapshot-integration.test.ts
packages/convert/src/snapshot/split.test.ts
packages/convert/src/snapshot/split.ts
packages/convert/src/write-deepnote-file.ts
packages/mcp/src/tools/execution.ts

Remove the backward-compat re-exports that were added when runnable-file loading and snapshot persistence moved into @deepnote/convert, and import the moved symbols directly from the package instead. - output-persistence.ts: drop the getSnapshotPath / mergeOutputsIntoFile / ExecutionTiming re-exports (they had no consumers) - format-converter.ts: drop the isRunnableExtension / RUNNABLE_EXTENSIONS / RunnableExtension re-exports - format-converter.test.ts: the only consumer of a removed re-export — now imports isRunnableExtension / RUNNABLE_EXTENSIONS from @deepnote/convert Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Follow-up to dropping the compat re-export shims: - remove the ConvertedFile type alias (a backward-compat rename of LoadedRunnableFile); resolveAndConvertToDeepnote now returns LoadedRunnableFile directly and run.ts imports the type from @deepnote/convert - restore the original BlockExecutionOutput doc comment in output-persistence.ts Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Integrate PR #408 (shared runnable-file loading & snapshot persistence extracted into @deepnote/convert), which was split out of this branch and merged to main. Resolution principle: adopt main's clean extraction as the base, then re-apply this branch's single-notebook feature work on top. Conflict resolutions (12 files): - convert/load-runnable-file.ts: took main's version (preserves fs ENOENT/EISDIR codes on .deepnote reads; this branch had wrapped them into a code-less error). - convert/snapshot/save-execution-snapshot.ts: kept this branch's feature version (notebookId-scoped filenames, init-notebook composed runs, init result paths). Kept the branch's mergeOutputsIntoFile strip semantics (drops stale per-block executionCount + execution timing so a re-run never leaks a prior run's metadata) and rewrote main's pinning test to assert the strip. - convert/snapshot/split.ts + write-deepnote-file.ts: object-param generateSnapshotFilename (from #408) plus this branch's notebookId + sanitizeFilenameComponent additions. - convert barrels (index.ts, snapshot/index.ts): union of #408 exports and the feature exports (resolve-init, snapshot-notebook-id, SaveExecutionSnapshotOptions). - convert test files: object-form generateSnapshotFilename calls. - cli/format-converter.ts: took main's clean thin wrapper (returns LoadedRunnableFile; dropped re-export shims no consumer needs). - cli/output-persistence.ts: main's thin wrapper + this branch's init options and init result paths; dropped the unused mergeOutputsIntoFile/getSnapshotPath shims. - cli/run.ts + mcp/execution.ts: union imports; kept the init composition (resolveAndComposeInit / resolveRunnableWithInit) while preserving main's fs error-code handling contract. Ran pnpm install (main added a `yaml` dep to @deepnote/database-integrations). Verified green: pnpm typecheck, biome:check, build, test (2330 passed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Account for PRs that landed on main after the initial bump: - @deepnote/convert 3.2.3 -> 4.0.0 (patch -> MAJOR): #364 and #408 rename/remove many public exports (convertIpynbFilesToDeepnoteFile -> convertIpynbFileToDeepnoteFile, etc.) and switch the .deepnote format to single-notebook-per-file — breaking changes for a published 3.x package. - @deepnote/mcp 0.3.3 -> 0.4.0 (patch -> minor): #364/#408 change tool behavior (single-notebook conversion output, notebook-scoped snapshots), so it is no longer a dependency-bump-only release. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* chore: bump package versions Release the changes that have accumulated on main since the last tag for each package: - @deepnote/blocks 4.5.1 -> 4.6.0 - @deepnote/cli 0.6.1 -> 0.7.0 - @deepnote/convert 3.2.2 -> 3.2.3 - @deepnote/database-integrations 1.4.3 -> 1.5.0 - @deepnote/mcp 0.3.2 -> 0.3.3 - @deepnote/reactivity 1.2.0 -> 1.2.1 - @deepnote/runtime-core 0.3.0 -> 0.4.0 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: correct convert (major) and mcp (minor) bumps Account for PRs that landed on main after the initial bump: - @deepnote/convert 3.2.3 -> 4.0.0 (patch -> MAJOR): #364 and #408 rename/remove many public exports (convertIpynbFilesToDeepnoteFile -> convertIpynbFileToDeepnoteFile, etc.) and switch the .deepnote format to single-notebook-per-file — breaking changes for a published 3.x package. - @deepnote/mcp 0.3.3 -> 0.4.0 (patch -> minor): #364/#408 change tool behavior (single-notebook conversion output, notebook-scoped snapshots), so it is no longer a dependency-bump-only release. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Merge branch 'main' into tk/dedup-runnable-snapshot-to-convert

3e2e42b

coderabbitai Bot requested changes Jun 17, 2026

View reviewed changes

Comment thread packages/convert/src/load-runnable-file.ts

Comment thread packages/convert/src/snapshot/save-execution-snapshot.ts

coderabbitai Bot previously approved these changes Jun 17, 2026

View reviewed changes

tkislan dismissed coderabbitai[bot]’s stale review via ea46f89 June 17, 2026 16:54

coderabbitai Bot previously approved these changes Jun 17, 2026

View reviewed changes

tkislan dismissed coderabbitai[bot]’s stale review via 7792f62 June 17, 2026 17:35

coderabbitai Bot approved these changes Jun 17, 2026

View reviewed changes

tkislan marked this pull request as ready for review June 18, 2026 08:17

tkislan requested a review from a team as a code owner June 18, 2026 08:17

tkislan requested review from m1so and mfranczel June 18, 2026 08:17

m1so approved these changes Jun 18, 2026

View reviewed changes

tkislan merged commit adb1f6a into main Jun 18, 2026
21 checks passed

tkislan deleted the tk/dedup-runnable-snapshot-to-convert branch June 18, 2026 09:28

coderabbitai Bot mentioned this pull request Jun 24, 2026

feat(deepnote-file): Migration to single notebook deepnote file #364

Merged

tkislan mentioned this pull request Jun 26, 2026

chore: bump package versions #406

Merged

coderabbitai Bot mentioned this pull request Jul 11, 2026

feat(local-runner): @deepnote/local-runner — run a .deepnote with edited inputs, locally or in Deepnote Cloud #419

Merged

Navigation Menu

Uh oh!

refactor(convert): extract shared runnable-file loading & snapshot persistence - #408

refactor(convert): extract shared runnable-file loading & snapshot persistence#408
tkislan merged 4 commits into
mainfrom
tk/dedup-runnable-snapshot-to-convert

tkislan commented Jun 17, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 17, 2026 •

edited

Loading

Walkthrough

Estimated code review effort

Suggested reviewers

Uh oh!

codecov Bot commented Jun 17, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

tkislan commented Jun 17, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Behavior preservation

Verification

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Estimated code review effort

Suggested reviewers

Uh oh!

codecov Bot commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tkislan commented Jun 17, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 17, 2026 •

edited

Loading

codecov Bot commented Jun 17, 2026 •

edited

Loading