Skip to content

fix: read and write text files as UTF-8 - #192

Open
OsamaAnsar wants to merge 1 commit into
superlinear-ai:mainfrom
OsamaAnsar:fix/utf8-text-io
Open

OsamaAnsar wants to merge 1 commit into
superlinear-ai:mainfrom
OsamaAnsar:fix/utf8-text-io

Conversation

@OsamaAnsar

Copy link
Copy Markdown

Problem

Path.read_text() / Path.write_text() without encoding= use the locale's preferred encoding, which is cp1252 on typical Windows installations (unless Python's UTF-8 mode is on). Several call sites handle UTF-8 data and break there:

Call site File being read/written Effect under cp1252
_markdown.py:211 (.md / .txt) user's UTF-8 document UnicodeDecodeError, or silently wrong text
_markdown.py:225 (fallback when pandoc rejects the format) user's UTF-8 document same
_cli.py:76 / :103 (raglite mcp install) the desktop app's MCP JSON config, which is UTF-8 read crashes, or non-ASCII characters of other servers' entries are decoded as mojibake and then written back
_bench.py:268 (OpenAIVectorStoreEvaluator.insert_documents) temp file whose raw bytes are uploaded to OpenAI UnicodeEncodeError for characters outside cp1252, otherwise non-UTF-8 bytes are uploaded

Reproduction (on main, Windows 11, Python 3.12, PYTHONUTF8=0 python -X utf8=0, locale.getpreferredencoding(False) == 'cp1252')

document_to_markdown on a UTF-8 note.md containing # Note\n\nHe said “hello” — it’s fine.\n (same for .txt, and for a file with an unsupported extension, which goes through the pandoc fallback):

before: UnicodeDecodeError 'charmap' codec can't decode byte 0x9d in position 28: character maps to <undefined>
after:  '# Note\n\nHe said \u201chello\u201d \u2014 it\u2019s fine.\n'

raglite mcp install with an existing UTF-8 config containing another server entry with "/home/José/tool" (and, in a second run, "“x”" in its args), driven through the Typer CliRunner:

before, args contain curly quotes: exit 1, UnicodeDecodeError('charmap', ..., 'character maps to <undefined>')
before, only "José":               exit 0, other server entry preserved: False  ->  {'command': '/home/Jos\xc3\xa9/tool', ...}   # mojibake persisted to the config
after (both):                       exit 0, other server entry preserved: True

With PYTHONUTF8=1 the same inputs work on main, so this only affects non-UTF-8 default encodings.

Fix

Pass encoding="utf-8" at those five call sites. I checked each: the MCP config is a JSON file the desktop app writes as UTF-8, json.dumps in the write path already emits ASCII (ensure_ascii=True) so the write change is only for consistency, and the benchmark temp file is created by this function and uploaded as bytes.

Not changed: _bench.py:70 (TREC run file; contains ids, ranks and scores) and _markdown.py:175 (splitlines on a regex match, unrelated to encoding). pypandoc.convert_file already defaults to UTF-8.

I found no existing issue or PR about this (open PR #190 only touches _embed.py).

Tests

Because the default encoding is already UTF-8 on Linux/macOS, the tests use a small fixture (cp1252_default_text_encoding in tests/conftest.py) that makes Path.read_text / Path.write_text default to cp1252 when no encoding is passed, so they fail on every platform without the fix:

  • tests/test_markdown.py: .md, .txt and the pandoc-rejects-the-format fallback (the latter is skipped if pypandoc is not installed).
  • tests/test_cli.py (new): raglite mcp install against a UTF-8 config with non-ASCII characters. The test installs a stub for the module that mcp install imports to locate the config directory; with the latest fastmcp that I resolved (4.1.1) that module no longer exists, so raglite mcp install currently fails there with ModuleNotFoundError regardless of this PR. I did not touch that.
  • tests/test_bench.py (new): a mocked OpenAI client records the bytes of the files that are uploaded; skipped if the bench dependencies are not installed. I installed only ir_datasets, ir_measures, python-slugify and openai, not the full bench extra (faiss, llama-index).

Results:

  • Fix reverted (only src/ restored to main): the 5 new tests fail (3 UnicodeDecodeError in test_markdown.py, the CLI exits with a UnicodeDecodeError, UnicodeEncodeError in the benchmark test); the existing PDF test passes.
  • With the fix: all pass. I also ran the three files under a real PYTHONUTF8=0 python -X utf8=0 cp1252 locale on Windows: 5 fail without the fix, 6 pass with it.

What I ran

  • pytest src tests with CI=1 (skips slow) and HF_HUB_OFFLINE=1: 51 passed (46 on main, +5 new). The same 9 failures and 30 errors occur on main and on this branch; all 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.
  • Not tested: macOS/Linux, Python 3.10, the lowest-direct resolution, PostgreSQL, a real OpenAI vector store upload, a real desktop app config file (only a synthetic one).

`Path.read_text` and `Path.write_text` without an explicit encoding use
the locale's preferred encoding, which is cp1252 on most Windows
installations (unless Python's UTF-8 mode is enabled). That breaks the
call sites that handle UTF-8 data:

- `document_to_markdown` raised a UnicodeDecodeError when reading a UTF-8
  .md or .txt file (or a file that pandoc cannot convert) containing,
  e.g., curly quotes.
- `raglite mcp install` raised a UnicodeDecodeError on a desktop app MCP
  config containing such characters, or silently decoded other characters
  (e.g., e-acute) into mojibake and wrote that back to the config.
- The OpenAI vector store benchmark wrote documents to temporary files in
  the locale encoding (or raised a UnicodeEncodeError for characters that
  the locale encoding cannot represent) and then uploaded those bytes.

Pass `encoding="utf-8"` at those call sites.

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