Skip to content

Conversation

@ksylvan
Copy link
Owner

@ksylvan ksylvan commented Jun 1, 2025

Fix section extraction bug in extractSection method

Summary

Fixed a critical bug in the extractSection method where the md-tree extract command was incorrectly extracting additional sections beyond the requested section. The issue was caused by using visit() function for tree traversal, which doesn't work correctly with array indices needed for section slicing. Replaced with direct for loops over tree.children to ensure accurate index tracking and proper section boundary detection.

Files Changed

  • markdown-parser.js - Fixed the extractSection method to use direct array iteration instead of visit() function calls. This ensures correct index tracking when finding headings and determining section boundaries.

Code Changes

markdown-parser.js

Before (problematic code):

// Used visit() which traverses all nodes including nested ones
visit(tree, "heading", (node, index) => {
  // index here doesn't match tree.children indices
  if (node.type === "heading") {
    // Logic for finding headings
  }
});

visit(tree, (node, index) => {
  // Logic for finding section boundaries
});

After (fixed code):

// Direct iteration over tree.children for accurate indices
for (let i = 0; i < tree.children.length; i++) {
  const node = tree.children[i];
  if (node.type === "heading") {
    // Logic for finding headings with correct index i
  }
}

// Direct iteration for finding section boundaries
for (let i = startIndex + 1; i < tree.children.length; i++) {
  const node = tree.children[i];
  if (node.type === "heading" && node.depth <= targetDepth) {
    endIndex = i;
    break;
  }
}

Reason for Changes

This change fixes a bug where the md-tree extract command was extracting more content than intended. For example, running md-tree extract bmad-prd.md '6. Epic Overview' would incorrectly return sections 6, 7, and 8 instead of just section 6.

The root cause was that the visit() function traverses ALL nodes in the AST tree (including nested nodes within paragraphs, lists, etc.), but the section extraction logic needed to work with direct children indices to properly slice the tree.children array.

Impact of Changes

  • ✅ Bug Resolution: Section extraction now works correctly, extracting only the requested section
  • ✅ Improved Reliability: More predictable behavior when extracting sections from markdown documents
  • ✅ Better User Experience: Users can now reliably extract specific sections without getting unwanted additional content
  • ✅ No Breaking Changes: All existing functionality remains intact
  • ✅ Performance: Slightly improved performance by avoiding unnecessary tree traversal

Test Plan

The changes were thoroughly tested using multiple approaches:

  1. Reproduction Testing: Verified the original bug was fixed by testing the problematic command:

    node bin/md-tree.js extract tmp/bmad-prd.md '6. Epic Overview'
    • Before: Extracted sections 6, 7, and 8
    • After: Correctly extracts only section 6
  2. Edge Case Testing: Tested extraction of different sections including:

    • First section
    • Middle sections
    • Last section
    • Non-existent sections
  3. Regression Testing: Ran the full test suite to ensure no existing functionality was broken:

    npm test  # All 18 tests pass
    node test/test-cli.js  # All 6 CLI tests pass
  4. Multiple File Testing: Verified the fix works with different markdown files and heading structures

Additional Notes

  • The fix maintains backward compatibility and doesn't change the public API
  • Debug statements were added during development and subsequently removed for clean production code
  • The solution is more robust as it works directly with array indices rather than relying on tree traversal callbacks
  • All existing tests continue to pass, demonstrating that the fix doesn't introduce regressions
  • The change follows the existing code style and conventions used throughout the project

## CHANGES

- Replace all double quotes with single quotes
- Convert visit() callbacks to for loops in getSection()
- Optimize heading search with direct array iteration
- Remove unnecessary visit() call for section end detection
- Maintain same functionality with cleaner code style
- Update package version from 1.4.1 to 1.4.2
@ksylvan ksylvan merged commit ee99b1f into main Jun 1, 2025
4 checks passed
@ksylvan ksylvan deleted the 0531-bmad-extract-section-fix branch June 1, 2025 01:28
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