feat(blocks): add indent support for bullet text blocks in Markdown#347
Conversation
📝 WalkthroughWalkthroughAdded support for indentation levels in bullet text blocks. A new Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #347 +/- ##
=======================================
Coverage 82.70% 82.70%
=======================================
Files 126 126
Lines 7614 7616 +2
Branches 2053 2118 +65
=======================================
+ Hits 6297 6299 +2
Misses 1316 1316
Partials 1 1 ☔ 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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/blocks/src/blocks/text-blocks.ts`:
- Around line 113-115: The bullet emitter uses an indent prefix (`const indent =
' '.repeat(metadata?.indent_level ?? 0)`) and returns `${indent}-
${escapeMarkdown(content)}`, but the normalization step only strips `- ` at
column 0 and will leave `- ` for indented bullets; update the normalization code
to remove a leading bullet with any amount of whitespace (e.g. strip
`/^\s*-\s+/`) so that indented bullets emitted by the
BulletTextBlockMetadata/indent logic are normalized correctly, and ensure
escapeMarkdown(content) behavior is preserved when matching and removing the
bullet prefix.
- Around line 113-115: The code unsafely casts block.metadata to
BulletTextBlockMetadata and calls 'indent_level' with .repeat(), which will
throw for negative or non-integer values; instead narrow/validate metadata
without an unchecked cast (avoid 'as'), check that metadata?.indent_level is a
finite non-negative integer (e.g., Number.isFinite and Number.isInteger) and
clamp or default to 0 (e.g., let level = Math.max(0, Math.floor(parsedValue ||
0))), then compute indent = ' '.repeat(level) and return `${indent}-
${escapeMarkdown(content)}`; update the code paths around the metadata variable
(the metadata binding in this snippet) and any helper that reads indent_level to
enforce this validation.
🪄 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: 8833b10f-538d-4f4d-ab77-11fda8d9f81d
📒 Files selected for processing (1)
packages/blocks/src/blocks/text-blocks.ts
| const metadata = block.metadata as BulletTextBlockMetadata | ||
| const indent = ' '.repeat(metadata?.indent_level ?? 0) | ||
| return `${indent}- ${escapeMarkdown(content)}` |
There was a problem hiding this comment.
Keep bullet markdown round-trip consistent with indentation.
This change emits indented bullets, but Line 153 only strips bullets at column 0. Indented bullet text may retain - when normalized.
Suggested fix (outside this hunk)
if (block.type === 'text-cell-bullet') {
- return content.replace(/^-+\s+/, '').trim()
+ return content.replace(/^\s*-\s+/, '').trim()
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/blocks/src/blocks/text-blocks.ts` around lines 113 - 115, The bullet
emitter uses an indent prefix (`const indent = '
'.repeat(metadata?.indent_level ?? 0)`) and returns `${indent}-
${escapeMarkdown(content)}`, but the normalization step only strips `- ` at
column 0 and will leave `- ` for indented bullets; update the normalization code
to remove a leading bullet with any amount of whitespace (e.g. strip
`/^\s*-\s+/`) so that indented bullets emitted by the
BulletTextBlockMetadata/indent logic are normalized correctly, and ensure
escapeMarkdown(content) behavior is preserved when matching and removing the
bullet prefix.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find schema/type definitions relevant to bullet metadata.
fd 'deepnote-file-schema.*' packages
# Inspect typing/usage of TextCellBulletBlock metadata and indent_level.
rg -n -C3 --type=ts '\bTextCellBulletBlock\b|\bindent_level\b|\bmetadata\b' packagesRepository: deepnote/deepnote
Length of output: 50374
🏁 Script executed:
rg -n 'BulletTextBlockMetadata' packages --type=ts -A 5 -B 2Repository: deepnote/deepnote
Length of output: 1283
🏁 Script executed:
fd 'deepnote-file-schema' packages --exec head -100 {}Repository: deepnote/deepnote
Length of output: 3899
🏁 Script executed:
cat -n packages/blocks/src/blocks/text-blocks.ts | sed -n '105,120p'Repository: deepnote/deepnote
Length of output: 687
🏁 Script executed:
rg -n 'text-cell-bullet|text.cell.bullet' packages --type=ts -A 3 -B 1Repository: deepnote/deepnote
Length of output: 3642
🏁 Script executed:
rg -n 'indent_level' packages --type=ts -B 2 -A 2Repository: deepnote/deepnote
Length of output: 836
Guard indent_level before .repeat() and avoid unchecked cast.
indent_level is typed as number with no constraints ensuring it's non-negative. A negative value will cause .repeat() to throw RangeError at runtime. Remove the unsafe cast and validate the value:
Suggested fix
- const metadata = block.metadata as BulletTextBlockMetadata
- const indent = ' '.repeat(metadata?.indent_level ?? 0)
+ const metadata = block.metadata
+ const rawIndentLevel = metadata?.indent_level
+ const indentLevel =
+ Number.isInteger(rawIndentLevel) && rawIndentLevel >= 0 ? rawIndentLevel : 0
+ const indent = ' '.repeat(indentLevel)
return `${indent}- ${escapeMarkdown(content)}`Per coding guidelines: "Use strict type checking in TypeScript files" and "Prefer type safety over convenience in TypeScript".
📝 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.
| const metadata = block.metadata as BulletTextBlockMetadata | |
| const indent = ' '.repeat(metadata?.indent_level ?? 0) | |
| return `${indent}- ${escapeMarkdown(content)}` | |
| const metadata = block.metadata | |
| const rawIndentLevel = metadata?.indent_level | |
| const indentLevel = | |
| Number.isInteger(rawIndentLevel) && rawIndentLevel >= 0 ? rawIndentLevel : 0 | |
| const indent = ' '.repeat(indentLevel) | |
| return `${indent}- ${escapeMarkdown(content)}` |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/blocks/src/blocks/text-blocks.ts` around lines 113 - 115, The code
unsafely casts block.metadata to BulletTextBlockMetadata and calls
'indent_level' with .repeat(), which will throw for negative or non-integer
values; instead narrow/validate metadata without an unchecked cast (avoid 'as'),
check that metadata?.indent_level is a finite non-negative integer (e.g.,
Number.isFinite and Number.isInteger) and clamp or default to 0 (e.g., let level
= Math.max(0, Math.floor(parsedValue || 0))), then compute indent = '
'.repeat(level) and return `${indent}- ${escapeMarkdown(content)}`; update the
code paths around the metadata variable (the metadata binding in this snippet)
and any helper that reads indent_level to enforce this validation.
2 participants
Summary by CodeRabbit