Skip to content

fix: stop emitting null block metadata defaults that @deepnote/blocks rejects on save - #407

Open
sLightlyDev wants to merge 4 commits into
mainfrom
fix/input-select-default-select-type
Open

fix: stop emitting null block metadata defaults that @deepnote/blocks rejects on save#407
sLightlyDev wants to merge 4 commits into
mainfrom
fix/input-select-default-select-type

Conversation

@sLightlyDev

@sLightlyDev sLightlyDev commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Problem

The local Zod schemas in deepnoteSchemas.ts produce the metadata for newly-created blocks. That metadata is handed straight back to @deepnote/blocks on save, and its block schema rejects an explicit null on optional fields. Two fields defaulted to null, so the blocks they belong to could not be saved at all:

Block Field Default before
Input Select deepnote_variable_select_type null
Input File deepnote_allowed_file_extensions null

Verified against serializeDeepnoteFile from @deepnote/blocks@4.6.0:

REJECTED input-select  -> deepnote_variable_select_type: Expected 'from-options' | 'from-variable', received null
REJECTED input-file    -> deepnote_allowed_file_extensions: Expected string, received null

deepnote_variable_default_value also defaults to null in several schemas, but the package explicitly coerces null → undefined for that one field, so it is fine.

Fix

  • deepnote_variable_select_type defaults to 'from-options'.
  • deepnote_allowed_file_extensions defaults to undefined rather than null, matching the package's optional-string field. Both consumers (deepnoteInputBlockCellStatusBarProvider.ts:556,933) already read it as string | undefined behind a falsy guard, so behavior is unchanged.

Tests

Rather than testing the two instances, this covers the class. getInputBlockMetadata now has one test per entry in INPUT_BLOCK_TYPES asserting that its default metadata survives serializeDeepnoteFile. Any future drift between the local schemas and the package fails at that boundary instead of at a user's save.

The block under test is built with createBlockFromPocket, so the test runs the same cell-to-block path used on save rather than a hand-assembled approximation.

Both new failures were confirmed against the pre-fix schema, each for the expected reason:

1) getInputBlockMetadata default input-select metadata is accepted by the @deepnote/blocks serializer
   expected [Function] to not throw but '{"expected": "'from-options'|'from-variable'", received: null}' was thrown
2) getInputBlockMetadata default input-file metadata is accepted by the @deepnote/blocks serializer
   expected [Function] to not throw but '{"code": "invalid_type", received: null}' was thrown

Full unit suite: 2634 passing, 0 failing. Typecheck, lint and compile-tsc clean.

expectedMetadataKeys for input-file drops deepnote_allowed_file_extensions — that assertion was pinning the unsaveable output.

The round-trip/escaping test from the first revision was removed: it hardcoded select_type, so it passed with and without the fix, and the escaping behavior it described is already covered by the InputSelectBlockConverter suite.

Follow-up (not in this PR)

The root cause is that deepnoteSchemas.ts re-declares block metadata that @deepnote/blocks also declares, and the two drift. The package does not currently export its per-block schemas (only the deepnoteBlockSchema union), and it pins zod 3.25.76 while this repo is on zod 4.x — so the schemas cannot simply be imported and reused today.

Worth noting separately: getInputBlockMetadata's return type is not correlated with its blockType argument, so it returns a union of all nine metadata shapes. Correlating them would let callers construct a DeepnoteBlock without a cast.

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of missing Deepnote variable selection and allowed file extension settings.
    • Ensured omitted file extension metadata is represented consistently.
  • Tests

    • Expanded coverage to verify all supported input block types serialize correctly with default metadata.

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 958b6288-8576-4cf8-b14e-aec91a5e8487

📥 Commits

Reviewing files that changed from the base of the PR and between 64d9656 and 8df4516.

📒 Files selected for processing (2)
  • src/notebooks/deepnote/deepnoteNotebookCommandListener.unit.test.ts
  • src/notebooks/deepnote/deepnoteSchemas.ts
💤 Files with no reviewable changes (1)
  • src/notebooks/deepnote/deepnoteSchemas.ts

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The schema restores the deepnote_variable_select_type fallback to from-options and maps nullish deepnote_allowed_file_extensions values to undefined. Tests add Deepnote serialization helpers and parameterized coverage for every input block type. Input-file metadata expectations now omit deepnote_allowed_file_extensions.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 8df45

The change replaces rejected null metadata defaults with serializer-compatible values, preventing save failures for affected input blocks. No actionable merge-blocking risk remains after normal checks and review.

Possibly related PRs

Suggested reviewers: tkislan, mfranczel, dinohamzic

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Updates Docs ❓ Inconclusive The PR diff contains only schema and test changes; no documentation files changed, and the public/private Deepnote repositories are unavailable here. Please verify or update the deepnote/deepnote OSS documentation and the deepnote-internal landing-page roadmap.
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the fix: preventing null block metadata defaults that Deepnote rejects during save.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 8, 2026
New Input Select blocks were created with deepnote_variable_select_type
set to null. The @deepnote/blocks serializer validates this field against
a strict 'from-options' | 'from-variable' enum and rejects null, which made
freshly-added Input Select blocks impossible to save.

Default the field to 'from-options' so new blocks serialize cleanly, and add
regression tests covering the default value and idempotent round-tripping of
the cell value (guarding against repeated JSON escaping).
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 8, 2026
tkislan and others added 2 commits August 19, 2026 08:04
deepnote_allowed_file_extensions had the same defect as
deepnote_variable_select_type: the local schema defaulted it to null, which
the @deepnote/blocks block schema rejects, so freshly-added Input File blocks
could not be saved either. Default it to undefined instead, matching the
package's optional-string field. Both consumers already read it as
`string | undefined` behind a falsy guard.

Cover the class rather than the two instances: getInputBlockMetadata now has a
test per entry in INPUT_BLOCK_TYPES asserting the default metadata survives
serializeDeepnoteFile, so any future drift between the local schemas and the
package fails at the boundary instead of at a user's save.

Drop the round-trip test added alongside the select_type fix — it hardcodes
select_type, so it passes with or without that fix, and the escaping behavior
it describes is already covered by the InputSelectBlockConverter suite. The
remaining select_type assertion moves into that suite.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017kn67m6zUgugyCve39giNT
@tkislan tkislan changed the title fix: default input-select select_type to 'from-options' fix: stop emitting null block metadata defaults that @deepnote/blocks rejects on save Aug 19, 2026
@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0%. Comparing base (aff145f) to head (8df4516).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@     Coverage Diff     @@
##   main   #407   +/-   ##
===========================
===========================
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Aug 19, 2026
The select_type default assertion duplicated the getInputBlockMetadata
boundary test, which already fails on that exact default; removing it returns
inputConverters.unit.test.ts to its original state.

Build the block under test with createBlockFromPocket instead of hand-assembling
one. The cast was papering over a real gap — getInputBlockMetadata's return type
is not correlated with its blockType argument, so TypeScript cannot pick a branch
of the DeepnoteBlock union. Going through the production cell-to-block path types
cleanly and exercises what actually runs on save.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017kn67m6zUgugyCve39giNT
@tkislan
tkislan marked this pull request as ready for review August 19, 2026 12:21
@tkislan
tkislan requested a review from a team as a code owner August 19, 2026 12:21
@tkislan
tkislan requested review from dinohamzic, mfranczel and voyti and removed request for dinohamzic August 19, 2026 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants