fix: stop emitting null block metadata defaults that @deepnote/blocks rejects on save - #407
fix: stop emitting null block metadata defaults that @deepnote/blocks rejects on save#407sLightlyDev wants to merge 4 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe schema restores the Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to 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: 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (5 passed)
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. Comment |
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).
3a007f5 to
ea0346f
Compare
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
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #407 +/- ##
===========================
===========================
🚀 New features to boost your workflow:
|
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
Problem
The local Zod schemas in
deepnoteSchemas.tsproduce the metadata for newly-created blocks. That metadata is handed straight back to@deepnote/blockson save, and its block schema rejects an explicitnullon optional fields. Two fields defaulted tonull, so the blocks they belong to could not be saved at all:deepnote_variable_select_typenulldeepnote_allowed_file_extensionsnullVerified against
serializeDeepnoteFilefrom@deepnote/blocks@4.6.0:deepnote_variable_default_valuealso defaults tonullin several schemas, but the package explicitly coercesnull → undefinedfor that one field, so it is fine.Fix
deepnote_variable_select_typedefaults to'from-options'.deepnote_allowed_file_extensionsdefaults toundefinedrather thannull, matching the package's optional-string field. Both consumers (deepnoteInputBlockCellStatusBarProvider.ts:556,933) already read it asstring | undefinedbehind a falsy guard, so behavior is unchanged.Tests
Rather than testing the two instances, this covers the class.
getInputBlockMetadatanow has one test per entry inINPUT_BLOCK_TYPESasserting that its default metadata survivesserializeDeepnoteFile. 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:
Full unit suite: 2634 passing, 0 failing. Typecheck, lint and
compile-tscclean.expectedMetadataKeysforinput-filedropsdeepnote_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 theInputSelectBlockConvertersuite.Follow-up (not in this PR)
The root cause is that
deepnoteSchemas.tsre-declares block metadata that@deepnote/blocksalso declares, and the two drift. The package does not currently export its per-block schemas (only thedeepnoteBlockSchemaunion), 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 itsblockTypeargument, so it returns a union of all nine metadata shapes. Correlating them would let callers construct aDeepnoteBlockwithout a cast.Summary by CodeRabbit
Bug Fixes
Tests