Skip to content

fix: honor encoding= in partition_md - #4488

Open
linhongyu510 wants to merge 1 commit into
Unstructured-IO:mainfrom
linhongyu510:fix/partition-md-honor-encoding
Open

linhongyu510 wants to merge 1 commit into
Unstructured-IO:mainfrom
linhongyu510:fix/partition-md-honor-encoding

Conversation

@linhongyu510

@linhongyu510 linhongyu510 commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Problem

partition_md() reads the source through read_txt_file() without passing an encoding, and has no encoding parameter of its own:

if filename is not None:
    _, text = read_txt_file(filename=filename)
elif file is not None:
    _, text = read_txt_file(file=file)

so a caller's codec lands in **kwargs, which the docstring documents as forwarded to partition_html — and is silently dropped for the read that actually needs it.

Charset detection covers the easy cases, which is why this stays invisible for a long time. I checked the whole family on main (3376cc96) against a cp1252 document:

partitioner has encoding= cp1252 result
partition_text yes Café naïve sección
partition_md no Café naďve sección

UTF-8 and BOM-bearing UTF-16 decode correctly with or without the argument, so they do not expose the gap — a legacy single-byte codepage does. The same bytes are recoverable through partition_text() because it accepts encoding; through partition_md() there is no way to express it and the mojibake is final.

Fix

Accept encoding and forward it to both read_txt_file() call sites.

The parameter is added after the existing ones, so current positional calls keep binding exactly as before. Omitting it preserves the detecting behavior unchanged.

Tests

  • test_partition_md_honors_an_explicit_encoding_for_a_filename
  • test_partition_md_honors_an_explicit_encoding_for_a_file_like_object
  • test_partition_md_still_detects_the_encoding_when_none_is_given — guards the unchanged default

The tests use cp1252 deliberately: a UTF-16 payload would pass even if the argument were ignored, so it could not tell the fix from the bug.

Verification

  • 42 pass in test_md.py (39 pre-existing + 3 new).
  • Rollback counter-proof: dropping only the two encoding= pass-throughs while keeping the parameter and the tests fails exactly the 2 "honors encoding" tests, with the "still detects" test still passing — the tests separate the new capability from the preserved default.
  • No new breakage: across test_md.py, test_text.py, test_json.py and test_ndjson.py the failure set is identical before and after; passing count goes 219 → 222, exactly the 3 added tests. (3 pre-existing failures are environmental and unrelated to this change.)
  • ruff check and ruff format --check clean on both changed files.
  • Checked the 124 open PRs for overlap with partition/md.py: none touches it. Compatibility Issue with Chinese Text in Document Parsing #3530 and Compatibility Issue with Chinese Text in Document Parsing #3267 modify test_md.py and partition/text.py but change sentence-splitting for Chinese text, not encoding — and both have been idle since 2024.

Separate from #4483, which covers partition_json / partition_ndjson; this branch is cut from current main and the two do not overlap.

Review in cubic

`partition_md()` reads the source with `read_txt_file()` but never passes an
encoding, and has no `encoding` parameter of its own -- so a caller's codec is
swallowed by `**kwargs`, which the docstring documents as forwarded to
`partition_html`.

Charset detection covers the easy cases, so this is invisible for a UTF-8 or
BOM-bearing UTF-16 document. A legacy single-byte codepage is where it bites:

    partition_md(filename="cp1252.md")   ->  "Café naďve sección"
    partition_text(filename="cp1252.txt", encoding="cp1252")
                                         ->  "Café naïve sección"

The same bytes are recoverable through `partition_text()` because it takes
`encoding`; through `partition_md()` there is no way to express it and the
mojibake is final.

Accept `encoding` and forward it to both `read_txt_file()` call sites. The
parameter is added after the existing ones so current positional calls keep
binding as before, and omitting it preserves the detecting behavior exactly.
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