fixing number-maths issue & some tests #155
Conversation
WalkthroughThe updates introduce enhanced Markdown rendering capabilities, especially for lists and math blocks, through new grouping logic and improved formatting in the renderer plugins. Several new feature and integration tests are added to validate math rendering, spacing, and nested list behaviors. Test infrastructure is expanded with new scripts, documentation, and a consolidated test runner. Minor script and import adjustments are also present. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MDXRenderer
participant BlockTransformer
User->>MDXRenderer: renderBlocksAsMarkdown(blocks, metadata)
loop For each group of blocks
MDXRenderer->>BlockTransformer: processBlock(block, metadata)
alt List group (bulleted/numbered)
BlockTransformer->>BlockTransformer: groupListItems(...)
BlockTransformer->>BlockTransformer: render grouped list recursively
else Non-list block
BlockTransformer->>BlockTransformer: render block as Markdown
end
end
MDXRenderer-->>User: Markdown string
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/plugins/renderer/mdx/index.tsOops! Something went wrong! :( ESLint: 9.30.1 Error: The 'jiti' library is required for loading TypeScript configuration files. Make sure to install it. src/core/renderer/index.tsOops! Something went wrong! :( ESLint: 9.30.1 Error: The 'jiti' library is required for loading TypeScript configuration files. Make sure to install it. test/features/test-math-rendering.jsOops! Something went wrong! :( ESLint: 9.30.1 Error: The 'jiti' library is required for loading TypeScript configuration files. Make sure to install it.
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (2)
test/integration/test-math-issue-case.js (1)
1-1: Verify build directory dependency.Same issue as the previous test file - this test depends on the build directory being present.
test/features/test-math-rendering.js (1)
1-1: Verify build directory dependency.Same build directory dependency issue as other test files.
🧹 Nitpick comments (5)
test/README.md (1)
7-19: Well-documented test structure with minor formatting improvement.The documentation clearly explains the test organization and provides helpful usage instructions.
Consider specifying the language for the fenced code block to improve rendering:
-``` +```text test/ ├── unit/ # Unit tests (Jest)src/plugins/renderer/mdx/index.ts (1)
57-117: Well-implemented list grouping method with opportunities for improvement.The
renderBlocksAsMarkdownmethod correctly groups consecutive list items and handles the rendering logic properly. The approach of processing blocks sequentially and maintaining list context is sound.Consider these improvements for better maintainability:
- public async renderBlocksAsMarkdown( - blocks: any[], - metadata: any = {}, - ): Promise<string> { + public async renderBlocksAsMarkdown( + blocks: NotionBlock[], + metadata: ContextMetadata = {}, + ): Promise<string> {Also, consider extracting the list grouping logic into a helper method to reduce duplication:
private async processListGroup( blocks: NotionBlock[], startIndex: number, listType: 'numbered_list_item' | 'bulleted_list_item', metadata: ContextMetadata ): Promise<{ result: string; nextIndex: number }> { // Common grouping logic here }test/integration/test-math-issue-case.js (1)
164-181: Simplify complex test data generation.The dynamic generation of nested list items using spread operator and map makes the test data hard to read and maintain.
Consider extracting the nested list generation into a helper function:
+function createNestedListItems(items, parentId) { + return items.map((content, idx) => ({ + object: 'block', + id: `${parentId}-${String.fromCharCode(97 + idx)}`, + type: 'numbered_list_item', + numbered_list_item: { + rich_text: [ + { type: 'text', text: { content: `(${String.fromCharCode(97 + idx)}) ${content}` }, plain_text: '', annotations: {}, href: null } + ], color: 'default' + }, parent: { type: 'block_id', block_id: parentId }, children: [] + })); +} - ...['a', 'b', 'c', 'd', 'e', 'f'].map((letter, idx) => ({ - object: 'block', - id: `item-10-${letter}`, - type: 'numbered_list_item', - numbered_list_item: { - rich_text: [ - { type: 'text', text: { content: `(${String.fromCharCode(97 + idx)}) ` }, plain_text: '', annotations: {}, href: null }, - { type: 'text', text: { content: [ - 'Assume $a\' = \\frac{a}{d}$ and $b\' = \\frac{b}{d}$ are not coprime and $d = \\gcd(a, b)$', - 'there is $e > 1$ such that $e = \\gcd(a\', b\')$', - 'Therefore $\\frac{a\'}{e} = \\frac{a}{ed}$ is an integer, same goes with $\\frac{b\'}{e} = \\frac{b}{ed}$', - '$ed > d$ since $e > 1$', - 'Because there is an integer larger than $d$ that divides $a$ and $b$, $d \\neq \\gcd(a, b)$', - 'Therefore (a) is wrong, $a\'$ and $b\'$ are coprime $\\square$' - ][idx] }, plain_text: '', annotations: {}, href: null } - ], color: 'default' - }, parent: { type: 'block_id', block_id: 'item-10' }, children: [] - })) + ...createNestedListItems([ + 'Assume $a\' = \\frac{a}{d}$ and $b\' = \\frac{b}{d}$ are not coprime and $d = \\gcd(a, b)$', + 'there is $e > 1$ such that $e = \\gcd(a\', b\')$', + 'Therefore $\\frac{a\'}{e} = \\frac{a}{ed}$ is an integer, same goes with $\\frac{b\'}{e} = \\frac{b}{ed}$', + '$ed > d$ since $e > 1$', + 'Because there is an integer larger than $d$ that divides $a$ and $b$, $d \\neq \\gcd(a, b)$', + 'Therefore (a) is wrong, $a\'$ and $b\'$ are coprime $\\square$' + ], 'item-10')test/run-all-tests.js (2)
20-22: Make test file paths more maintainable.Hardcoded test file paths make the script brittle when test files are added or moved.
-const integrationTests = [ - 'test/integration/test-math-issue-case.js' -]; +const integrationTests = require('fs').readdirSync('test/integration/') + .filter(file => file.endsWith('.js')) + .map(file => `test/integration/${file}`); -const featureTests = [ - 'test/features/test-math-rendering.js', - 'test/features/test-nested-numbered-lists.js', - 'test/features/test-mdx-spacing.js' -]; +const featureTests = require('fs').readdirSync('test/features/') + .filter(file => file.endsWith('.js')) + .map(file => `test/features/${file}`);Also applies to: 37-41
11-11: Consider consistent stdio handling.The script uses different stdio options for different test types, which may lead to inconsistent output handling.
Consider using consistent stdio handling or document why different approaches are needed:
- execSync('npx jest test/unit/ --silent', { stdio: 'inherit' }); + execSync('npx jest test/unit/ --silent', { stdio: 'inherit' }); - execSync(`node ${test}`, { stdio: 'pipe' }); + execSync(`node ${test}`, { stdio: 'inherit' });Also applies to: 27-27, 46-46
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
test/unit/__snapshots__/test-jsx-renderer.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (11)
package.json(1 hunks)src/core/renderer/index.ts(1 hunks)src/plugins/renderer/mdx/index.ts(1 hunks)src/plugins/renderer/mdx/transformers/blocks.ts(3 hunks)test/README.md(1 hunks)test/features/test-math-rendering.js(1 hunks)test/features/test-mdx-spacing.js(1 hunks)test/features/test-nested-numbered-lists.js(1 hunks)test/integration/test-math-issue-case.js(1 hunks)test/run-all-tests.js(1 hunks)test/unit/test-jsx-renderer.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
test/features/test-mdx-spacing.js (1)
src/plugins/renderer/mdx/index.ts (1)
MDXRenderer(32-118)
test/run-all-tests.js (5)
test/features/test-math-rendering.js (1)
require(1-1)test/features/test-mdx-spacing.js (1)
require(1-1)test/features/test-nested-numbered-lists.js (1)
require(1-1)test/integration/test-math-issue-case.js (1)
require(1-1)src/core/renderer/index.ts (1)
process(257-290)
🪛 markdownlint-cli2 (0.17.2)
test/README.md
7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (11)
test/unit/test-jsx-renderer.ts (1)
1-2: LGTM: Import path correction aligns with test directory restructuring.The updated import paths correctly reflect the new test directory organization described in the PR objectives.
src/core/renderer/index.ts (1)
322-329: Excellent bug fix: Pass equation expression to transformer instead of plain text.This change correctly ensures that equation annotation transformers receive the actual mathematical expression (
item.equation.expression) rather than the plain text representation. This is crucial for proper math rendering in formats like LaTeX or MathML.test/features/test-mdx-spacing.js (1)
93-131: Comprehensive spacing test with good analysis coverage.The test effectively validates MDX spacing output across multiple block types and provides detailed analysis of spacing patterns. The use of pattern matching to identify spacing issues is particularly valuable for debugging.
test/features/test-nested-numbered-lists.js (1)
4-110: LGTM: Comprehensive test data structure.The test data structure is well-formed and comprehensive, covering nested numbered lists with proper parent-child relationships and all required metadata fields.
test/features/test-math-rendering.js (1)
4-105: LGTM: Comprehensive math test cases.The test data covers both block equations and inline math within paragraphs, including complex LaTeX expressions like integrals and fractions.
test/run-all-tests.js (1)
1-54: LGTM: Well-structured test runner.The test runner provides good organization with clear output, appropriate error handling, and fail-fast behavior.
package.json (1)
10-22: LGTM: Improved script organization.The script updates provide better developer experience with granular test commands, cleaner build process, and additional linting/formatting options.
src/plugins/renderer/mdx/transformers/blocks.ts (4)
137-198: Well-structured list grouping implementation with proper nesting support.The refactored bulleted list transformer correctly groups consecutive list items and handles nested lists properly. The logic for marking items as rendered prevents duplication, and the recursive handling of nested bullet and numbered lists maintains proper hierarchy.
203-267: Robust numbered list implementation with correct sequential numbering.The numbered list transformer properly handles grouping and maintains correct sequential numbering (
idx + 1) for each item in the group. The nested list handling mirrors the bulleted list logic consistently.
516-516: Improved math rendering format aligns with standard LaTeX conventions.The change from code blocks to
$$delimiters is correct for block-level math expressions and follows standard LaTeX/MathJax conventions.
709-761: Fallback transformers provide necessary compatibility for different contexts.The fallback transformers are well-documented and provide simpler per-block processing when block tree context is unavailable. The comment clearly explains the purpose and future improvement direction.
| @@ -0,0 +1,129 @@ | |||
| const { MDXRenderer } = require('../../build/plugins/renderer'); | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify build directory dependency.
The test imports from ../../build/plugins/renderer, which assumes the project has been built. Ensure this dependency is documented and that the build process runs before tests.
🏁 Script executed:
#!/bin/bash
# Check if build directory exists and contains the expected files
if [ ! -d "build/plugins/renderer" ]; then
echo "Build directory missing - tests will fail"
exit 1
fi
# Check if the expected export exists
if [ ! -f "build/plugins/renderer/index.js" ]; then
echo "Expected renderer export missing"
exit 1
fi
echo "Build directory structure verified"Length of output: 141
Ensure renderer import is available before running tests
The test at test/features/test-nested-numbered-lists.js currently does:
const { MDXRenderer } = require('../../build/plugins/renderer');but the build/plugins/renderer directory isn’t present in the repo, so the test will always fail. Please update one of the following:
- Add a pre-test build step to generate build/plugins/renderer/index.js (e.g. run
npm run buildbeforenpm test). - Or import directly from the source file so tests don’t depend on built artifacts:
- const { MDXRenderer } = require('../../build/plugins/renderer'); + const { MDXRenderer } = require('../../plugins/renderer');
🤖 Prompt for AI Agents
In test/features/test-nested-numbered-lists.js at line 1, the import of
MDXRenderer from '../../build/plugins/renderer' fails because the build
directory is missing. To fix this, either add a pre-test build step that runs
the build script to generate the required files before tests run, or change the
import to point directly to the source file location of the renderer so tests do
not depend on built artifacts.
| async function testNestedNumberedLists() { | ||
| const renderer = new MDXRenderer(); | ||
| // Use the new grouping-aware renderer method | ||
| const mdx = await renderer.renderBlocksAsMarkdown(testBlocks); | ||
|
|
||
| console.log('--- Nested Numbered Lists MDX Output ---'); | ||
| console.log(mdx); | ||
| console.log('--- Expected Output ---'); | ||
| console.log('1. First item'); | ||
| console.log(' 1. Nested item 1'); | ||
| console.log(' 2. Nested item 2'); | ||
| console.log(''); | ||
| console.log('2. Second item'); | ||
| console.log(''); | ||
| console.log('------------------------------'); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add proper test assertions instead of console output.
The test currently only outputs results to console without actual assertions. Consider adding proper test assertions to verify the rendered output matches expectations.
async function testNestedNumberedLists() {
const renderer = new MDXRenderer();
- // Use the new grouping-aware renderer method
const mdx = await renderer.renderBlocksAsMarkdown(testBlocks);
- console.log('--- Nested Numbered Lists MDX Output ---');
- console.log(mdx);
- console.log('--- Expected Output ---');
- console.log('1. First item');
- console.log(' 1. Nested item 1');
- console.log(' 2. Nested item 2');
- console.log('');
- console.log('2. Second item');
- console.log('');
- console.log('------------------------------');
+ const expectedOutput = `1. First item
+ 1. Nested item 1
+ 2. Nested item 2
+
+2. Second item
+
+`;
+
+ if (mdx.trim() !== expectedOutput.trim()) {
+ console.error('Test failed! Expected:', expectedOutput);
+ console.error('Got:', mdx);
+ process.exit(1);
+ }
+
+ console.log('✅ Nested numbered lists test passed!');
}📝 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.
| async function testNestedNumberedLists() { | |
| const renderer = new MDXRenderer(); | |
| // Use the new grouping-aware renderer method | |
| const mdx = await renderer.renderBlocksAsMarkdown(testBlocks); | |
| console.log('--- Nested Numbered Lists MDX Output ---'); | |
| console.log(mdx); | |
| console.log('--- Expected Output ---'); | |
| console.log('1. First item'); | |
| console.log(' 1. Nested item 1'); | |
| console.log(' 2. Nested item 2'); | |
| console.log(''); | |
| console.log('2. Second item'); | |
| console.log(''); | |
| console.log('------------------------------'); | |
| } | |
| async function testNestedNumberedLists() { | |
| const renderer = new MDXRenderer(); | |
| const mdx = await renderer.renderBlocksAsMarkdown(testBlocks); | |
| const expectedOutput = `1. First item | |
| 1. Nested item 1 | |
| 2. Nested item 2 | |
| 2. Second item | |
| `; | |
| if (mdx.trim() !== expectedOutput.trim()) { | |
| console.error('Test failed! Expected:', expectedOutput); | |
| console.error('Got:', mdx); | |
| process.exit(1); | |
| } | |
| console.log('✅ Nested numbered lists test passed!'); | |
| } |
🤖 Prompt for AI Agents
In test/features/test-nested-numbered-lists.js around lines 112 to 127, replace
the console.log statements with proper test assertions that compare the rendered
markdown output to the expected string. Use an assertion library like assert or
expect to verify that the mdx variable matches the expected nested numbered list
format exactly, ensuring the test validates correctness instead of just printing
output.
| async function testMathIssueCase() { | ||
| const renderer = new MDXRenderer(); | ||
| const mdx = await renderer.renderBlocksAsMarkdown(testBlocks); | ||
|
|
||
| console.log('--- Math Issue Case MDX Output ---'); | ||
| console.log(mdx); | ||
| console.log('------------------------------'); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add proper assertions for integration test.
Similar to the feature test, this integration test only outputs to console without verifying correctness.
async function testMathIssueCase() {
const renderer = new MDXRenderer();
const mdx = await renderer.renderBlocksAsMarkdown(testBlocks);
- console.log('--- Math Issue Case MDX Output ---');
- console.log(mdx);
- console.log('------------------------------');
+ // Verify the output contains expected mathematical expressions
+ const expectedElements = [
+ '## **PROOF**',
+ '\\gcd(a, b) = am + bn',
+ 'c = ax + by',
+ 'infinitely many'
+ ];
+
+ for (const element of expectedElements) {
+ if (!mdx.includes(element)) {
+ console.error(`Test failed! Missing expected element: ${element}`);
+ process.exit(1);
+ }
+ }
+
+ console.log('✅ Math issue case test passed!');
}📝 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.
| async function testMathIssueCase() { | |
| const renderer = new MDXRenderer(); | |
| const mdx = await renderer.renderBlocksAsMarkdown(testBlocks); | |
| console.log('--- Math Issue Case MDX Output ---'); | |
| console.log(mdx); | |
| console.log('------------------------------'); | |
| } | |
| async function testMathIssueCase() { | |
| const renderer = new MDXRenderer(); | |
| const mdx = await renderer.renderBlocksAsMarkdown(testBlocks); | |
| // Verify the output contains expected mathematical expressions | |
| const expectedElements = [ | |
| '## **PROOF**', | |
| '\\gcd(a, b) = am + bn', | |
| 'c = ax + by', | |
| 'infinitely many' | |
| ]; | |
| for (const element of expectedElements) { | |
| if (!mdx.includes(element)) { | |
| console.error(`Test failed! Missing expected element: ${element}`); | |
| process.exit(1); | |
| } | |
| } | |
| console.log('✅ Math issue case test passed!'); | |
| } |
🤖 Prompt for AI Agents
In test/integration/test-math-issue-case.js around lines 326 to 333, the test
currently only logs output without verifying correctness. Add proper assertions
to check that the rendered markdown (mdx) matches expected results. Define
expected output or key properties and use assertion methods to validate the
output instead of just printing it.
| object: 'block', id: 'item-8', type: 'numbered_list_item', numbered_list_item: { | ||
| rich_text: [ | ||
| { type: 'text', text: { content: 'Therefore we can write ' }, plain_text: '', annotations: {}, href: null }, | ||
| { type: 'equation', equation: { expression: 'ax + by = ax' + "'" + ' + by' + "'" }, plain_text: '', annotations: {}, href: null } |
There was a problem hiding this comment.
Fix fragile string concatenation in equation expression.
The string concatenation with quotes in the equation expression is fragile and could break LaTeX parsing.
- { type: 'equation', equation: { expression: 'ax + by = ax' + "'" + ' + by' + "'" }, plain_text: '', annotations: {}, href: null }
+ { type: 'equation', equation: { expression: 'ax + by = ax\' + by\'' }, plain_text: '', annotations: {}, href: null }📝 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.
| { type: 'equation', equation: { expression: 'ax + by = ax' + "'" + ' + by' + "'" }, plain_text: '', annotations: {}, href: null } | |
| { type: 'equation', equation: { expression: 'ax + by = ax\' + by\'' }, plain_text: '', annotations: {}, href: null } |
🤖 Prompt for AI Agents
In test/integration/test-math-issue-case.js at line 143, the equation expression
uses fragile string concatenation with quotes that may break LaTeX parsing.
Replace the concatenation with a single properly escaped string literal or
template literal that correctly represents the equation expression without
splitting or manually adding quotes, ensuring the LaTeX syntax remains intact
and parsable.
| console.log('```math'); | ||
| console.log('E = mc^2'); | ||
| console.log('```'); | ||
| console.log(''); | ||
| console.log('```math'); | ||
| console.log('\\frac{-b \\pm \\sqrt{b^2-4ac}}{2a}'); | ||
| console.log('```'); |
There was a problem hiding this comment.
Fix inconsistent expected output format.
The expected output shows math blocks using ```math syntax, but the AI summary indicates the actual implementation uses $$ delimiters for math blocks.
- console.log('```math');
- console.log('E = mc^2');
- console.log('```');
- console.log('');
- console.log('```math');
- console.log('\\frac{-b \\pm \\sqrt{b^2-4ac}}{2a}');
- console.log('```');
+ console.log('$$');
+ console.log('E = mc^2');
+ console.log('$$');
+ console.log('');
+ console.log('$$');
+ console.log('\\frac{-b \\pm \\sqrt{b^2-4ac}}{2a}');
+ console.log('$$');🤖 Prompt for AI Agents
In test/features/test-math-rendering.js around lines 114 to 120, the expected
output uses ```math fenced code blocks for math expressions, but the actual
implementation uses $$ delimiters. Update the console.log statements to replace
the ```math and ``` lines with $$ to match the actual output format, ensuring
consistent math block delimiters.
| async function testMathRendering() { | ||
| const renderer = new MDXRenderer(); | ||
| const mdx = await renderer.renderBlocksAsMarkdown(testBlocks); | ||
|
|
||
| console.log('--- Math Rendering MDX Output ---'); | ||
| console.log(mdx); | ||
| console.log('--- Expected Output ---'); | ||
| console.log('```math'); | ||
| console.log('E = mc^2'); | ||
| console.log('```'); | ||
| console.log(''); | ||
| console.log('```math'); | ||
| console.log('\\frac{-b \\pm \\sqrt{b^2-4ac}}{2a}'); | ||
| console.log('```'); | ||
| console.log(''); | ||
| console.log('This is a paragraph with inline math: $x^2 + y^2 = z^2$'); | ||
| console.log(''); | ||
| console.log('Complex inline math: $\\int_{-\\infty}^{\\infty} e^{-x^2} dx = \\sqrt{\\pi}$'); | ||
| console.log(''); | ||
| console.log('------------------------------'); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add proper test assertions for math rendering.
Replace console output with actual assertions to verify math rendering works correctly.
async function testMathRendering() {
const renderer = new MDXRenderer();
const mdx = await renderer.renderBlocksAsMarkdown(testBlocks);
- console.log('--- Math Rendering MDX Output ---');
- console.log(mdx);
- console.log('--- Expected Output ---');
- console.log('$$');
- console.log('E = mc^2');
- console.log('$$');
- console.log('');
- console.log('$$');
- console.log('\\frac{-b \\pm \\sqrt{b^2-4ac}}{2a}');
- console.log('$$');
- console.log('');
- console.log('This is a paragraph with inline math: $x^2 + y^2 = z^2$');
- console.log('');
- console.log('Complex inline math: $\\int_{-\\infty}^{\\infty} e^{-x^2} dx = \\sqrt{\\pi}$');
- console.log('');
- console.log('------------------------------');
+ // Verify block math equations
+ if (!mdx.includes('$$\nE = mc^2\n$$')) {
+ console.error('Test failed! Block math equation not rendered correctly');
+ process.exit(1);
+ }
+
+ // Verify inline math equations
+ if (!mdx.includes('$x^2 + y^2 = z^2$')) {
+ console.error('Test failed! Inline math equation not rendered correctly');
+ process.exit(1);
+ }
+
+ console.log('✅ Math rendering test passed!');
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In test/features/test-math-rendering.js around lines 107 to 127, replace the
console.log statements with proper test assertions that compare the rendered MDX
output against the expected math rendering strings. Use assertion methods to
verify that block math and inline math are rendered correctly, ensuring the test
automatically validates the output instead of just printing it.
| function groupListItems(block, siblings, type) { | ||
| // Find all consecutive list items of the same type at the same parent | ||
| const startIdx = siblings.findIndex((b) => b.id === block.id); | ||
| const group = []; | ||
| for ( | ||
| let i = startIdx; | ||
| i < siblings.length && | ||
| siblings[i].type === type && | ||
| siblings[i].parent?.id === block.parent?.id; | ||
| i++ | ||
| ) { | ||
| group.push(siblings[i]); | ||
| } | ||
| return group; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add defensive programming for robustness.
Consider adding null checks and error handling to prevent runtime errors when siblings is undefined or when block.id is not found.
function groupListItems(block, siblings, type) {
+ if (!siblings || !Array.isArray(siblings)) {
+ return [block];
+ }
+
// Find all consecutive list items of the same type at the same parent
const startIdx = siblings.findIndex((b) => b.id === block.id);
+ if (startIdx === -1) {
+ return [block];
+ }
+
const group = [];
for (
let i = startIdx;
i < siblings.length &&
siblings[i].type === type &&
siblings[i].parent?.id === block.parent?.id;
i++
) {
group.push(siblings[i]);
}
return group;
}📝 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.
| function groupListItems(block, siblings, type) { | |
| // Find all consecutive list items of the same type at the same parent | |
| const startIdx = siblings.findIndex((b) => b.id === block.id); | |
| const group = []; | |
| for ( | |
| let i = startIdx; | |
| i < siblings.length && | |
| siblings[i].type === type && | |
| siblings[i].parent?.id === block.parent?.id; | |
| i++ | |
| ) { | |
| group.push(siblings[i]); | |
| } | |
| return group; | |
| } | |
| function groupListItems(block, siblings, type) { | |
| if (!siblings || !Array.isArray(siblings)) { | |
| return [block]; | |
| } | |
| // Find all consecutive list items of the same type at the same parent | |
| const startIdx = siblings.findIndex((b) => b.id === block.id); | |
| if (startIdx === -1) { | |
| return [block]; | |
| } | |
| const group = []; | |
| for ( | |
| let i = startIdx; | |
| i < siblings.length && | |
| siblings[i].type === type && | |
| siblings[i].parent?.id === block.parent?.id; | |
| i++ | |
| ) { | |
| group.push(siblings[i]); | |
| } | |
| return group; | |
| } |
🤖 Prompt for AI Agents
In src/plugins/renderer/mdx/transformers/blocks.ts around lines 763 to 777, the
function groupListItems assumes siblings is defined and block.id exists in
siblings, which can cause runtime errors. Add defensive checks to ensure
siblings is an array before using findIndex, and verify that startIdx is not -1
before proceeding with the loop. If these conditions are not met, return an
empty array early to prevent errors.
💡 Verification agent
🧩 Analysis chain
Verify the groupListItems function handles edge cases correctly.
The grouping logic looks sound, but please ensure it handles edge cases properly, particularly when siblings array might be undefined or when block IDs are not found.
🏁 Script executed:
#!/bin/bash
# Description: Search for usage of groupListItems to understand the calling context and verify siblings parameter handling
# Search for calls to groupListItems function
rg -A 5 -B 5 "groupListItems\(" --type typescript
# Search for metadata.siblings usage to understand how it's populated
rg -A 3 -B 3 "metadata\.siblings" --type typescriptLength of output: 175
#!/bin/bash
# Search for calls to groupListItems in .ts and .tsx files
rg -n -A5 -B5 'groupListItems\(' -g '*.ts' -g '*.tsx'
# Search for metadata.siblings usage in .ts and .tsx files
rg -n -A3 -B3 'metadata\.siblings' -g '*.ts' -g '*.tsx'🏁 Script executed:
#!/bin/bash
# Search for usage of groupListItems in .ts and .tsx files
rg -n -A5 -B5 'groupListItems\(' -g '*.ts' -g '*.tsx'
# Search for metadata.siblings usage in .ts and .tsx files
rg -n -A3 -B3 'metadata\.siblings' -g '*.ts' -g '*.tsx'Length of output: 4202
Handle undefined siblings and missing block IDs in groupListItems
The current implementation will throw if siblings is undefined or if block.id isn’t found (startIdx === -1). Add guards at the top of groupListItems to return an empty array in those cases.
Locations to update:
src/plugins/renderer/mdx/transformers/blocks.tslines 763–777
Suggested diff:
function groupListItems(block, siblings, type) {
- // Find all consecutive list items of the same type at the same parent
- const startIdx = siblings.findIndex((b) => b.id === block.id);
+ // Bail out if siblings is not an array or block id isn’t in siblings
+ if (!Array.isArray(siblings)) return [];
+ const startIdx = siblings.findIndex((b) => b.id === block.id);
+ if (startIdx < 0) return [];
const group = [];
for (
let i = startIdx;
i < siblings.length &&
siblings[i].type === type &&
siblings[i].parent?.id === block.parent?.id;
i++
) {
group.push(siblings[i]);
}
return group;
}This ensures we never call .findIndex or index into siblings when it’s unset or when block.id isn’t present.
📝 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.
| function groupListItems(block, siblings, type) { | |
| // Find all consecutive list items of the same type at the same parent | |
| const startIdx = siblings.findIndex((b) => b.id === block.id); | |
| const group = []; | |
| for ( | |
| let i = startIdx; | |
| i < siblings.length && | |
| siblings[i].type === type && | |
| siblings[i].parent?.id === block.parent?.id; | |
| i++ | |
| ) { | |
| group.push(siblings[i]); | |
| } | |
| return group; | |
| } | |
| function groupListItems(block, siblings, type) { | |
| // Bail out if siblings is not an array or block id isn’t in siblings | |
| if (!Array.isArray(siblings)) return []; | |
| const startIdx = siblings.findIndex((b) => b.id === block.id); | |
| if (startIdx < 0) return []; | |
| const group = []; | |
| for ( | |
| let i = startIdx; | |
| i < siblings.length && | |
| siblings[i].type === type && | |
| siblings[i].parent?.id === block.parent?.id; | |
| i++ | |
| ) { | |
| group.push(siblings[i]); | |
| } | |
| return group; | |
| } |
🤖 Prompt for AI Agents
In src/plugins/renderer/mdx/transformers/blocks.ts around lines 763 to 777, the
function groupListItems does not handle cases where siblings is undefined or
block.id is not found, causing potential runtime errors. Add a guard at the
start of the function to check if siblings is falsy or if findIndex returns -1
for block.id, and return an empty array immediately in those cases to prevent
errors when accessing siblings or indexing.
Summary
This PR:
What’s Changed
Test Runner Script (test/run-all-tests.js):
- [as previously described]
Math Rendering Fixes:
- Improved handling of both block and inline math expressions in MDX output.
- Ensured correct rendering of complex equations (e.g., \gcd(a, b), fractions, integrals).
- Added/updated feature and integration tests to cover math edge cases.
Numbered List/Nested List Fixes:
- Fixed logic for rendering nested numbered lists, ensuring correct indentation and numbering.
- Added/updated feature tests for deeply nested and sequential lists.
Test File Organization:
- [as previously described]
Bug Fixes:
- [as previously described]
Math and Numbered List Issues: Fixed bugs where math blocks or nested lists were not rendered or numbered correctly.
Here are some screenshots:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores