Implement DatabaseBlockMarkdownAdapter for markdown conversion #14218
Conversation
📝 WalkthroughWalkthroughA new Markdown adapter for database blocks is introduced. The adapter converts DatabaseBlockModel instances into MarkdownTableNode by instantiating a datasource, constructing table headers from property names, iterating through rows to collect cell values, and returning a formatted markdown table. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
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 |
![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
Fix all issues with AI Agents 🤖
In @blocksuite/affine/shared/src/adapters/markdown/database-block-adapter.ts:
- Around line 21-25: The code reads dataSource.properties$.value into
propertyIds without guarding against null/undefined, which can throw if the
observable isn't initialized; update the logic around propertyIds (from
dataSource.properties$.value) to first check for nullish (e.g., if
(!propertyIds) return or skip) before calling propertyIds.forEach, and ensure
the header population loop still uses dataSource.propertyNameGet(propId) to push
propertyName || propId into header only when propertyIds is present.
- Around line 28-29: The code reads dataSource.rows$.value without a
null/undefined guard which can throw; update the access to safely handle missing
rows by checking dataSource.rows$ and dataSource.rows$.value (similar to
properties$.value) before iterating—e.g., use a safe fallback like const rowIds
= dataSource.rows$?.value ?? [] or early-return if !dataSource.rows$ ||
!dataSource.rows$.value, then proceed with rowIds.forEach(...).
🧹 Nitpick comments (2)
blocksuite/affine/shared/src/adapters/markdown/database-block-adapter.ts (2)
30-38: Consider improving cell value conversion for complex types.Using
String(cellValue ?? '')will produce"[object Object]"for objects and may not format dates or arrays optimally. While the PR notes acknowledge that rich text, dates, and relations need future work, consider adding a helper function to handle common cases more gracefully.💡 Example approach
function formatCellValue(value: unknown): string { if (value == null) return ''; if (Array.isArray(value)) return value.join(', '); if (value instanceof Date) return value.toISOString(); if (typeof value === 'object') return JSON.stringify(value); return String(value); }Then use:
- rowContent.push(String(cellValue ?? '')); + rowContent.push(formatCellValue(cellValue));
40-46: Consider handling empty tables explicitly.If a database block has no properties or rows, this will return a table with empty header/rows arrays. Verify whether the markdown renderer handles empty tables gracefully, or consider returning a different node type (e.g., a text node indicating "Empty table").
📜 Review details
Configuration used: defaults
Review profile: CHILL
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 (1)
blocksuite/affine/shared/src/adapters/markdown/database-block-adapter.ts
🔇 Additional comments (4)
blocksuite/affine/shared/src/adapters/markdown/database-block-adapter.ts (4)
6-9: Verify whether the database flavour constant is already exported from the model.The comment suggests this flavour is "typically defined in a corresponding model file." Check if
DatabaseBlockModel(from@blocksuite/affine-model) already exports aFLAVOURor similar constant. If it does, import and reuse it instead of redefining it locally to avoid duplication and maintenance issues.
1-1: No action needed.The import path
../../../blocks/database/src/data-sourceis correct. DatabaseBlockDataSource is properly exported fromblocksuite/affine/blocks/database/src/data-source.tsand the relative path resolves correctly from the adapter file location.
14-15: No action required.DatabaseBlockDataSourcedoes not implement any disposal methods and the usage pattern only performs synchronous reads (accessing.valueon signals without creating subscriptions). The instance is a local variable that's used once and discarded naturally when theconvertfunction returns.Likely an incorrect or invalid review comment.
13-47: Error handling is not needed—the code is already defensive.The adapter uses proper fallbacks (
propertyName || propIdandString(cellValue ?? '')) to handle missing or invalid data. The underlyingDatabaseBlockDataSourcemethods (cellValueGet,propertyNameGet) are designed defensively, returningundefinedor empty strings rather than throwing errors. This approach aligns with the codebase patterns shown in test usage, whereDatabaseBlockDataSourceis instantiated without try-catch blocks.Likely an incorrect or invalid review comment.
| const propertyIds = dataSource.properties$.value; | ||
| propertyIds.forEach(propId => { | ||
| const propertyName = dataSource.propertyNameGet(propId); | ||
| header.push(propertyName || propId); // Use property name, fall back to ID if name is empty. | ||
| }); |
There was a problem hiding this comment.
Add null/undefined check for properties$.value.
Accessing dataSource.properties$.value without checking for null/undefined can cause a runtime error if the observable hasn't been initialized or returns a nullish value.
🔎 Proposed fix
- const propertyIds = dataSource.properties$.value;
+ const propertyIds = dataSource.properties$.value ?? [];
propertyIds.forEach(propId => {📝 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 propertyIds = dataSource.properties$.value; | |
| propertyIds.forEach(propId => { | |
| const propertyName = dataSource.propertyNameGet(propId); | |
| header.push(propertyName || propId); // Use property name, fall back to ID if name is empty. | |
| }); | |
| const propertyIds = dataSource.properties$.value ?? []; | |
| propertyIds.forEach(propId => { | |
| const propertyName = dataSource.propertyNameGet(propId); | |
| header.push(propertyName || propId); // Use property name, fall back to ID if name is empty. | |
| }); |
🤖 Prompt for AI Agents
In @blocksuite/affine/shared/src/adapters/markdown/database-block-adapter.ts
around lines 21-25, The code reads dataSource.properties$.value into propertyIds
without guarding against null/undefined, which can throw if the observable isn't
initialized; update the logic around propertyIds (from
dataSource.properties$.value) to first check for nullish (e.g., if
(!propertyIds) return or skip) before calling propertyIds.forEach, and ensure
the header population loop still uses dataSource.propertyNameGet(propId) to push
propertyName || propId into header only when propertyIds is present.
| const rowIds = dataSource.rows$.value; | ||
| rowIds.forEach(rowId => { |
There was a problem hiding this comment.
Add null/undefined check for rows$.value.
Similar to properties$.value, accessing dataSource.rows$.value without a null check can cause runtime errors.
🔎 Proposed fix
- const rowIds = dataSource.rows$.value;
+ const rowIds = dataSource.rows$.value ?? [];
rowIds.forEach(rowId => {📝 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 rowIds = dataSource.rows$.value; | |
| rowIds.forEach(rowId => { | |
| const rowIds = dataSource.rows$.value ?? []; | |
| rowIds.forEach(rowId => { |
🤖 Prompt for AI Agents
In @blocksuite/affine/shared/src/adapters/markdown/database-block-adapter.ts
around lines 28-29, The code reads dataSource.rows$.value without a
null/undefined guard which can throw; update the access to safely handle missing
rows by checking dataSource.rows$ and dataSource.rows$.value (similar to
properties$.value) before iterating—e.g., use a safe fallback like const rowIds
= dataSource.rows$?.value ?? [] or early-return if !dataSource.rows$ ||
!dataSource.rows$.value, then proceed with rowIds.forEach(...).
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## canary #14218 +/- ##
==========================================
- Coverage 56.51% 56.50% -0.01%
==========================================
Files 2775 2775
Lines 142437 142437
Branches 21697 21695 -2
==========================================
- Hits 80493 80479 -14
- Misses 60160 60174 +14
Partials 1784 1784
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
Adds a DatabaseBlockMarkdownAdapter to serialize affine:database blocks into markdown tables. Previously, pages containing only table views were treated as empty by the AI. This adapter exposes table headers and rows, enabling AI summarization and project tracking.
### Notes:
Currently handles basic cell types; rich text, dates, and relations may need special handling in future PRs.
Kanban grouping, filters, and sorts are not yet supported.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.