Repository navigation
Python: write and read chat history files as UTF-8 - #14745
Hyacinth-of-Security (lxcxjxhx) wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Existing tests would still pass if explicit UTF-8 encodings were removed from ONNX initialization and chat-history persistence, so targeted regression coverage is needed.
2 open issues
What changed in this PR
This PR makes file encoding explicit in the Python package, improving chat-history portability and ensuring ONNX model configuration files are read as UTF-8.
Changes:
- Use UTF-8 when writing and reading chat-history files.
- Use UTF-8 when reading ONNX
genai_config.jsonfiles.
| File | Description |
|---|---|
python/semantic_kernel/contents/chat_history.py |
Specifies UTF-8 for history file reads and writes. |
python/semantic_kernel/connectors/ai/onnx/services/onnx_gen_ai_completion_base.py |
Specifies UTF-8 for model configuration reads. |
Review effort: Balanced
| try: | ||
| json_gen_ai_config = os.path.join(ai_model_path + "/genai_config.json") | ||
| with open(json_gen_ai_config) as file: | ||
| with open(json_gen_ai_config, encoding="utf-8") as file: |
There was a problem hiding this comment.
Added in 3fb8d30: test_onnx_config_is_read_as_utf8 in python/tests/unit/connectors/ai/onnx/services/test_onnx_chat_completion.py.
It filters open.call_args_list down to the genai_config.json call and asserts call.kwargs.get('encoding') == 'utf-8' on every one of them, so removing the explicit encoding fails the assertion rather than passing silently.
| """ | ||
| json_str = self.serialize() | ||
| with open(file_path, "w") as local_file: | ||
| with open(file_path, "w", encoding="utf-8") as local_file: |
There was a problem hiding this comment.
Added in 3fb8d30: test_chat_history_file_round_trip_keeps_non_ascii in python/tests/unit/contents/test_chat_history.py.
It stores héllo — 世界 and then asserts on bytes: 'héllo — 世界'.encode() in file_path.read_bytes(). Reading through read_text() would round-trip under any single-byte codec and prove nothing; the byte comparison is what fails when the encoding='utf-8' on the write is removed on a host whose default is not UTF-8.
Both the chat history file round trip and the ONNX `genai_config.json` read relied on the host's default encoding in their tests, so removing the explicit `encoding="utf-8"` left them green. Assert the encoding each open requests, and add a non-ASCII round trip for the chat history. Verified by reverting the source encodings: the two new assertions fail, and both suites pass with them (`113 passed`).
|
Both points were right, and the fix is in Chat history (
The honest split between them: the round-trip test passes on a UTF-8-default host even without the source change — I checked by reverting the encodings — so it documents the user-visible outcome rather than pinning the fix. The spy test is the one that fails when the ONNX ( Replayed the regression, with the source encodings removed: With the encodings restored: |

What
Three text-mode
open()calls in shipped code rely on the platform default encoding. On Windows that is the ANSI code page (GBK on a zh-CN machine, cp1252 on en-US), so:ChatHistory.store_chat_history_to_file()writes a file that is not UTF-8: a history containinghéllo — 世界lands on disk ash\xa8\xa6llo \xa1\xaa \xca\xc0\xbd\xe7— GBK bytes for the same text. That file cannot be read on Linux/macOS or inside a container, whereopen()defaults to UTF-8:UnicodeDecodeError: 'utf-8' codec can't decode byte 0xa8, and any character outside the ANSI code page raisesUnicodeEncodeErrorwhen writing.ChatHistory.load_chat_history_from_file()is the matching half, so the pair only round-trips on the machine that wrote it.OnnxGenAICompletionBase._load_genai_settingsreadsgenai_config.jsonfrom a model directory without an encoding — those configs come from the model package and are UTF-8.Change
Three lines, no behaviour change on POSIX (UTF-8 is already the default there).
Verification
Windows 11 / CPython 3.13,
semantic_kernelinstalled editable frompython/:python/tests/unit/contents/test_chat_history.py: 76 passed before and after (the suite does not pin an encoding, so it does not notice the difference — the fix is about interoperability, not about a failing test).Scope
Only
python/semantic_kernel/. A scan of the tree finds 32 more text-mode opens without an encoding, but they are in tests, samples and scripts; I kept this PR to the three in the shipped package so it stays reviewable in one pass. Happy to send the test/sample sweep separately if you want it.