Pin UTF-8 encoding when reading/writing adapter config JSON

[dltsum]

Problem

PeftConfig.from_json_file reads adapter_config.json using the locale's preferred encoding:

with open(path_json_file) as file:
    json_object = json.load(file)

On Windows with a non-UTF-8 locale (e.g. cp936/GBK), loading an adapter whose config contains raw non-ASCII JSON crashes:

UnicodeDecodeError: 'gbk' codec can't decode byte 0xaf in position 59: illegal multibyte sequence

A realistic source of such configs: a user (or a third-party tool) writes base_model_name_or_path pointing at a local model directory with CJK characters, saved as UTF-8 without ASCII escaping.

Reproducer (Windows 11, cp936, Python 3.14)

import json
open('adapter_config.json', 'w', encoding='utf-8').write(
    json.dumps({'peft_type': 'LORA', 'base_model_name_or_path': '模型/路径', 'r': 8}, ensure_ascii=False)
)
from peft.config import PeftConfig
PeftConfig.from_json_file('adapter_config.json')  # UnicodeDecodeError before; parses fine after

Fix

Pin encoding="utf-8" on the read, and on the corresponding write in save_pretrained for symmetry (the write was already ASCII-safe through json.dumps defaults; this only makes the encoding explicit).

Verification

• Before: reproducer above raises UnicodeDecodeError; after: parses correctly.
• pytest tests/test_config.py → 1282 passed, 12 skipped.

🤖 Generated with Claude Code

[BenjaminBossan]

Thanks for opening this PR. In the future, please open an issue first so that we can discuss the change beforehand.

As to the problem, I don't have a Windows machine to test this. However, we have Windows on CI, so could you please add a unit test for this behavior? It should fail without the fix and pass with the fix. Guard the test on the platform being Windows.

Also, as mentioned on #3772, please add that change here.

[dltsum]

Thanks for the review! Done both:

1. Folded the shadow-backbone config.json read (from Pin UTF-8 encoding when reading the shadow model's config.json #3772) into this PR — closing that one. A repo-wide scan found no other user-facing config reads missing the encoding pin.
2. Added a Windows-gated regression test (test_from_json_file_utf8_non_ascii): it writes an adapter config with literal UTF-8 non-ASCII values and round-trips it through from_json_file. Verified locally on a cp936 machine: fails without the fix (UnicodeDecodeError on read), passes with it.

And noted re: opening an issue first next time — will do.

[sergereview]

🤗 Serge says:

Verdict: The fix is correct, minimal, and safe — pinning encoding="utf-8" on both the read and write of adapter_config.json is right, and the shadow/model.py change is consistent with it. But the test only exercises the fix on Windows, and the PR does not meet the repo's agent-contribution/approval requirements.

Correctness

• Backwards-compatible: save_pretrained has always written via json.dumps with default ensure_ascii=True, so every config PEFT wrote is pure ASCII and decodes identically under UTF-8; non-UTF-8 legacy configs could only come from third-party tools, which is exactly the case this fixes. Verified these are the only open()/json.load text-decode sites in src/peft (the rest are safe_open binary readers).

Tests

• test_from_json_file_utf8_non_ascii is gated on sys.platform.startswith("win"), so it never runs on the Ubuntu CI job. On windows-latest the ANSI code page is cp1252 (not cp936), so it catches the bug there only incidentally and on one matrix leg; the same crash occurs on Linux under a C/POSIX locale, so the guard is unnecessarily narrow — see inline suggestion.
• Only the read path is covered; a roundtrip through save_pretrained with a non-ASCII base_model_name_or_path would also lock in the write-side pin.

serge v0.1.0 · model: zai-org/GLM-5.3-Flash · 14 LLM turns · 15 tool calls · 110.9s · 126641 in / 6875 out tokens

[BenjaminBossan]

@dltsum Thanks for the update. I checked if the new test would fail in the Windows CI without the fix and it indeed does: https://github.com/huggingface/peft/actions/runs/35717602918/job/106713214870?pr=3785

I also asked the Serge bot to review the PR (using GLM 5.3 Flash). As mentioned, I don't have a Windows machine to verify this, but please check if the feedback makes sense.

For the testing, there should also be a test specific to ShadowPEFT, right?

[dltsum]

Thanks for confirming the test goes red on Windows CI without the fix, and for the Serge review — both points made sense and are now addressed in a4cdcd7:

1. Windows gate removed: the regression test now runs on all platforms, since the failure mode is locale-dependent, not OS-dependent (LC_ALL=C on Linux reproduces it too).
2. Roundtrip added: save_pretrained → from_json_file roundtrip with a non-ASCII value now locks in both directions. One note: json.dumps defaults to ensure_ascii=True, so save_pretrained output is ASCII-only and the write-side pin is defensive (it only matters if the serialization ever switches to ensure_ascii=False); the roundtrip passes either way today, but it will catch that change if it ever happens.
3. ShadowPEFT-specific test added: test_explicit_shadow_model_utf8_config writes a real shadow checkpoint whose config.json contains a literal CJK _name_or_path (UTF-8, ensure_ascii=False) and loads it via get_peft_model — this exercises the folded-in fix in shadow/model.py end to end.

Both tests pass locally on a real Windows machine (cp936 locale).

[dltsum]

@BenjaminBossan Thanks for verifying the test on the Windows CI! The Serge bot feedback makes sense — addressed both points in a4cdcd7:

1. Dropped the Windows-only skipif: the regression shows on any non-UTF-8 locale (e.g. LC_ALL=C on Linux), so the test now runs everywhere. Also removed the now-unused sys import.
2. Added the save_pretrained roundtrip assertion to lock in the write-side pin as well.
3. Added a ShadowPEFT-specific test (test_explicit_shadow_model_utf8_config in tests/test_shadow.py): saves a tiny Llama shadow checkpoint, rewrites its config.json with a literal non-ASCII _name_or_path (UTF-8, ensure_ascii=False), and loads it via ShadowConfig(shadow_model=<path>) — exercising the pinned read in _load_shadow_backbone.

Verified red/green locally on a cp936 Windows machine: without the pins both tests fail with UnicodeDecodeError: 'gbk' codec; with the pins both pass.

[BenjaminBossan]

Thanks for the updates. I still have some comments, please check.

[BenjaminBossan]

Suggested change

	# ensure_ascii=False). Not Windows-gated: the regression shows on any non-UTF-8 locale.
	# ensure_ascii=False).

This extra comment doesn't make sense outside the context of the review of this PR. Only add comments that make sense on their own.

[dltsum]

Done in b326332 — replaced with a self-contained comment.

[BenjaminBossan]

Is it not possible to call locale.setlocale?

[dltsum]

Good question — I tried that first, but setlocale can't reliably produce a legacy-encoding locale inside an already-running Python process:

• Windows: locale.setlocale(locale.LC_ALL, "C") doesn't change what open() uses — locale.getpreferredencoding(False) still returns the ANSI code page (e.g. cp1252/cp936) afterwards, because that value comes from the OS code page, not the C locale. Verified locally on Windows 11 / Python 3.14: before and after setlocale(LC_ALL, "C") the preferred encoding is unchanged.
• POSIX: with LC_ALL=C, CPython's UTF-8 mode / PEP 538 coercion usually makes the locale encoding report UTF-8 anyway, so the test would silently stop exercising the legacy-decode path on Linux CI.

The goal of the monkeypatch is just to make the default-encoding path deterministic on every platform (fail without the fix, pass with it), independent of the host locale. Happy to switch if you know a cleaner way to force a non-UTF-8 default encoding mid-process.

[BenjaminBossan]

Hmm, that's too bad. I think this all makes the testing more complicated than it's worth it. I'd say, let's go back to the status from before you where you only checked Windows, skipping the test on other platforms.

[dltsum]

Agreed — done in f44c15e. Back to the Windows-gated version: skip on non-Windows, plain write-then-read with literal non-ASCII, and the save_pretrained roundtrip from the last round kept. Ran it on a real Windows machine (cp936 locale): passes with the pin, and this is the scenario that fails on Windows CI without it.

[BenjaminBossan]

Thanks for the updates. I still have some comments.

To verify that the tests fail on Windows without the fix, I started a CI run with those tests (#3970). It looks like the ShadowPEFT passes even without the fix, could you please check why?

Please also run make style before pushing your changes.

[BenjaminBossan]

This should also be skipped on Platforms other than Windows.

[dltsum]

Done in 432f61c — now skipped unless platform.system() == "Windows", same as the test_config.py one.

[BenjaminBossan]

Comment needs updating regarding Linux. Also, please refer to the other test and explain why we need this specific one for ShadowPEFT.

[dltsum]

Done in 432f61c — comment rewritten: dropped the Linux reference, and it now points to test_from_json_file_utf8_non_ascii in tests/test_config.py, explaining that the shadow backbone config loads through a separate code path (_load_shadow_backbone) so it needs its own regression. Verified on a real Windows machine (cp936): passes.

[BenjaminBossan]

@dltsum There is a merge conflict, could you please check?

[dltsum]

@BenjaminBossan Rebased onto current main and resolved the conflict — upstream switched to _sort_keys(output_dict), so the UTF-8 pin is now applied on that call. The utf8 test still passes locally.