fix(parsers): normalize CRLF/CR to LF in text & markdown parsers#59
Merged
Conversation
PR #57 (Step 1.3) merged with a failing windows-latest job: test_parse_plain_text expected 2 paragraphs but saw 1 because the PlainTextParser's blank-line splitter (\n[ \t]*\n+) can't see across the \r between two CRLF-terminated lines, and the test wrote the sample with Path.write_text which emits CRLF on Windows. Root-cause fix in the parsers (not just the test) so any CRLF or bare-CR input — including content fetched from connectors on a Windows host or downloaded from Windows-authored sources — produces the same blocks on every platform. - PlainTextParser._decode now normalises \r\n and bare \r to \n after UTF-8/latin-1 decode, before the paragraph regex runs. - MarkdownParser.parse does the same after decode so markdown-it's line-based Token.map offsets stay in sync with the text we hand back. - Two new tests in tests/parsers/test_text_family.py exercise CRLF + bare-CR inputs via write_bytes so they're platform-independent. - packages/ragctl/tests/test_parse.py switched to write_bytes(b"...\n...") for the three text fixtures — defence in depth against platform newline translation in the integration-style CLI tests. Other parsers (HTML / JSON / YAML / CSV / PDF / DOCX / PPTX / XLSX) use libraries that handle CRLF natively, so no changes there. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR #57 (Step 1.3) merged with a failing windows-latest job. Root cause: the
PlainTextParserparagraph regex\n[ \t]*\n+can't see across the\rbetween two CRLF-terminated lines. The ragctl test was usingPath.write_text, which emits CRLF on Windows, so Windows saw one paragraph instead of two.Fixing the parsers (not just the test) keeps blocks platform-stable for any CRLF / bare-CR input — including content fetched from connectors on a Windows host or files authored on Windows. This is the standing cross-platform requirement (
CLAUDE.md: "every deliverable must work on Windows, macOS, and Linux without WSL").What changed
PlainTextParser._decodenormalises\r\nand bare\rto\nafter UTF-8 / latin-1 decode, before the paragraph regex runs.MarkdownParser.parsedoes the same after decode so markdown-it's line-basedToken.mapoffsets stay in sync with the text we return.Tests
tests/parsers/test_text_family.py— 3 new platform-independent tests viawrite_bytes:test_plain_text_crlf_paragraph_split— CRLF input → 2 paragraphs.test_plain_text_bare_cr_normalized— classic-Mac bare-CR input → 2 paragraphs.test_markdown_crlf_input— CRLF markdown → heading + paragraph + list_item.packages/ragctl/tests/test_parse.py— three text fixtures switched towrite_bytes(b\"...\\n...\")(defence in depth against platform newline translation in CLI-level tests).Verified locally
uv run pytest tests/ packages/ -x -q— full suite passes (no regressions).uv run ruff check/ruff format --check— clean.uv run mypy packages/parsers/— clean.Documentation
No public-API surface change — internal normalisation only, transparent to callers. No
docs/update needed; the existingdocs/reference/parsers.mdanddocs/architecture/parsers.mdcontinue to apply.Test plan
windows-latest(specifically the previously-failingLint & Test (windows-latest)job + theCI passedrollup).ubuntu-22.04andmacos-14.🤖 Generated with Claude Code