Skip to content

fix: split Markdown lines like markdown-it does - #191

Open
OsamaAnsar wants to merge 1 commit into
superlinear-ai:mainfrom
OsamaAnsar:fix/markdown-it-line-index
Open

OsamaAnsar wants to merge 1 commit into
superlinear-ai:mainfrom
OsamaAnsar:fix/markdown-it-line-index

Conversation

@OsamaAnsar

Copy link
Copy Markdown

Problem

markdown_sentence_boundaries (_split_sentences.py) and markdown_chunklet_boundaries (_split_chunklets.py) turn markdown-it's token.map line numbers into character offsets by splitting the document with doc.splitlines(keepends=True).

markdown-it only treats LF, CRLF and CR as line endings (it normalises CRLF/CR to LF and nothing else), but str.splitlines also splits on \x0b, \x0c, \x1c-\x1e, \x85, \u2028 and \u2029. If a document contains any of these characters (for example a form feed in text extracted from a PDF), the line index built from splitlines has more entries than markdown-it's, so every token.map line after that point is converted to the wrong character offset. Heading spans and paragraph/heading boundary probabilities then land on the wrong text.

Reproduction (on main, markdown-it-py 4.2.0, Python 3.12, Windows)

Heading span marked by markdown_sentence_boundaries for "Intro paragraph one." + sep + "Intro paragraph two continues here.\n\n# Real Heading\n\nBody text after the heading.\n":

sep         before (main)        after (this PR)
LF          '# Real Heading'     '# Real Heading'
CRLF        '# Real Heading'     '# Real Heading'
CR          '# Real Heading'     '# Real Heading'
\x0b        '#'                  '# Real Heading'
\x0c        '#'                  '# Real Heading'
\u2028      '#'                  '# Real Heading'
\x85        '#'                  '# Real Heading'

markdown_chunklet_boundaries(["intro\x0c", "para2\n\n", "# Real Heading\n\n", "body"]):

before (main): [0.0, 1.0, 0.0, 0.0]   # heading boundary shifted onto the paragraph, paragraph boundary lost
after:         [0.5, 0.0, 1.0, 0.0]   # identical to the same input with the form feed replaced by a space

Fix

Split lines with re.findall(r"[^\r\n]*(?:\r\n|\r|\n)|[^\r\n]+", doc), which recognises exactly the line endings that markdown-it does. For text that only contains LF, CRLF and CR this returns the same list as doc.splitlines(keepends=True) (checked against 200,000 random strings over a, b, space, \n, \r, \r\n, #, and a non-ASCII letter, plus the empty string), so normal documents are unaffected.

_markdown.py:175 also calls splitlines, but on a regex match of ATX heading lines and not on anything tied to token.map, so I left it alone.

I found nothing in the open or closed issues/PRs about this specific cause. #81 touched the same function (heading boundary probabilities) for an out-of-bounds problem; this is a separate misalignment.

Tests

New parametrized tests in tests/test_split_sentences.py and tests/test_split_chunklets.py cover LF, CRLF, CR and each of the other separators listed above, without needing the sentence splitter model.

  • With the fix reverted (only src/ restored to main): 16 of the 22 new cases fail (all the exotic separators); the LF/CRLF/CR cases pass, as expected.
  • With the fix: all 22 pass.

What I ran

  • pytest tests/test_split_chunklets.py tests/test_split_sentences.py tests/test_markdown.py tests/test_import.py tests/test_split_chunks.py: 50 passed (Python 3.12.10, Windows 11).
  • pytest src tests with CI=1 (skips slow) and HF_HUB_OFFLINE=1: 68 passed (46 on main, +22 new). The same 9 failures and 30 errors occur on main and on this branch; all of them are tests that need to download the Qwen3/bge-m3 llama.cpp models (OfflineModeIsEnabled). 97 tests were skipped (PostgreSQL / OpenAI key not available).
  • pre-commit run on the changed files (ruff check, ruff format, mypy, commitizen, the other hooks): passed. pre-commit run --all-files fails on main and on this branch alike with ICN001 in src/raglite/_database.py under ruff 0.17.1 (the version I had installed); I did not touch that.
  • Python 3.10 and the lowest-direct resolution were not tested, nor was PostgreSQL.

markdown_sentence_boundaries and markdown_chunklet_boundaries convert the
line numbers in markdown-it's `token.map` into character offsets by
splitting the document into lines with `str.splitlines`. That method also
splits on the vertical tab, form feed, file/group/record separators, NEL,
U+2028 and U+2029, whereas markdown-it only treats LF, CRLF and CR as line
endings. A document containing any of those characters (e.g., a form feed
in text extracted from a PDF) therefore got shifted line offsets, so
heading spans and paragraph boundary probabilities landed on the wrong
sentences.

Split lines with a regex that only recognises LF, CRLF and CR instead.
For documents that only contain those line endings the result is
identical to `str.splitlines(keepends=True)`.

This branch has not been deployed

No deployments
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.

1 participant