fix(encoding): pin encoding=utf-8 on dialect.py + config.py text opens

[KeilerHirsch]

What

Pin encoding="utf-8" on the text-mode open() calls in dialect.py and config.py that omitted it. Without an explicit encoding, Python decodes/encodes using the OS locale codepage — on a non-UTF-8 locale (e.g. German Windows = cp1252) this corrupts non-ASCII content in files MemPalace itself writes as UTF-8.

The defect

config.py already writes config.json / people_map.json as UTF-8 (json.dump(..., ensure_ascii=False) with encoding="utf-8"), but the read paths (config.py:377 config load, config.py:553 people_map) opened without an encoding. That asymmetry means a Müller written as UTF-8 is read back as cp1252 → Müller. dialect.py had the same gap across all 11 of its text opens.

Concretely, on a cp1252 process:

# config.json on disk contains valid UTF-8: {"people_map": {"Mueller": "Müller"}}
conf = MempalaceConfig(config_dir=...)
conf._file_config["people_map"]["Mueller"]   # before: 'Müller'  (mojibake)
                                             # after:  'Müller'   (correct)

Changes

• mempalace/dialect.py — encoding="utf-8" on all 11 text-mode open() calls (6 read, 5 write).
• mempalace/config.py — encoding="utf-8" on the 4 text opens that lacked it (the two read paths plus two writes). The four write paths that already pinned UTF-8 are untouched.
• tests/test_encoding_hardening.py — new regression tests that force encoding-less text opens to default to cp1252 (simulating a German Windows process), write real UTF-8 bytes to disk, and assert the umlaut round-trips through Dialect.from_config, the config.json read, and a raw-UTF-8 skip_names value.

Verification

• The 3 new tests are RED before the fix (assert 'Müller' == 'M�ller') and GREEN after — verified by reverting only the two source files and re-running.
• 158/158 existing test_dialect.py + test_config*.py tests still pass (no regression).
• ruff check and ruff format --check both clean on all three files.
• Honest scope note: dialect.py's save_config uses json.dump's default ensure_ascii=True, so its umlauts are ASCII-escaped and codec-agnostic on write — the bug is the read side (and two non-JSON .write() text dumps). This PR does not claim save_config's JSON write was corrupting data.

Out of scope (same defect class, tracked separately)

miner.py (open(config_path) … yaml.safe_load) and room_detector_local.py have the identical encoding-less-open pattern for YAML config files. Not touched here to keep this PR to the two files the finding named; worth a follow-up.

[arnoldwender]

Pinning encoding="utf-8" is the right call, and the cp1252_default_open fixture is a genuinely nice way to make the Windows failure reproducible on any host — that fixture is reusable well beyond this PR.

One gap worth considering before this lands, because it is the mirror image of what the tests cover.

The new tests all exercise UTF-8 bytes on disk, read under a cp1252 default locale — the mojibake direction. They do not cover legacy-codepage bytes already on disk, read under the now-forced UTF-8. That second direction is the migration case, and it changes a degraded read into a hard failure:

>>> issubclass(UnicodeDecodeError, (json.JSONDecodeError, OSError))
False        # UnicodeDecodeError -> UnicodeError -> ValueError

So the existing guards in config.py — except (json.JSONDecodeError, OSError) on both the _config_file read and the people_map read — do not catch it. Reproduced against this branch:

--- before (locale cp1252 read) ---
  OK -> reads without crashing. keys: [Müller, Ökonomie]

--- after (encoding="utf-8" forced) ---
  CRASH: UnicodeDecodeError: 'utf-8' codec can't decode byte 0xfc in position 24: invalid start byte
  caught by (JSONDecodeError, OSError)?  False

Scope is genuinely bounded, and I want to be fair about that: every file MemPalace writes itself goes through json.dump(...) with the default ensure_ascii=True, so it is pure ASCII and decodes under any codec. This can only bite a hand-edited file.

Which is exactly why Dialect.from_config is the sharp edge here. It has no try/except at all, so the traceback reaches the user raw — and dialect.py explicitly instructs them to hand-edit that very file:

print("Edit this file with your own entity mappings, then use --config entities.json")

A German, Russian or Chinese user following that instruction is going to put non-ASCII into an entity map (that is the whole point of an entity map for them), and if their editor saves cp1252/GBK rather than UTF-8, this PR turns a working-if-imperfect read into an unhandled crash. That is the same population the PR is written to protect.

Cheap fix, in the spirit of what is already here:

except (json.JSONDecodeError, UnicodeDecodeError, OSError):

for the two config.py guards, and for from_config a guard that names the cause rather than replacing bytes silently — something like re-raising as ValueError(f"{config_path} is not valid UTF-8 — re-save it as UTF-8"). Silent errors="replace" would be worse than the status quo here, since it would corrupt entity names rather than report the problem.

Happy to send this as a follow-up PR on top of yours if you would rather keep this one tightly scoped to the read/write pinning — your call, and either way this is a clear improvement over what is on develop today.

Separately: #1600 (fix: add explicit UTF-8 encoding to all text file I/O on Windows) is still open and overlaps this file set, so whichever lands second will likely need a rebase.

[KeilerHirsch]

@arnoldwender thanks — reproduced the legacy-codepage migration case exactly and folded the bounded fix into this PR.

What changed:

• both config.py read guards now also catch UnicodeDecodeError and fall back through the existing invalid-config path;
• Dialect.from_config() catches UnicodeDecodeError and raises an actionable ValueError telling the user to re-save the file as UTF-8;
• added 3 regression tests that write real cp1252 bytes for config.json, people_map.json, and a dialect config.

Verification on the review branch:

• new migration tests: 3/3 RED before the fix, then 6/6 focused tests GREEN after;
• test_dialect.py + test_config*.py + encoding-hardening suite: 164 passed;
• Ruff was run/formatted and the final regression suite remained 164/164 GREEN.

Latest head is fec10df. GitHub Actions for that head are currently running. No errors="replace" path was introduced, so entity names are never silently corrupted.

[arnoldwender]

@KeilerHirsch that's exactly the fix I was hoping for, and the fallback matching the existing invalid-config path (rather than inventing a new one) is the right call — thanks for turning it around so fast.

I re-ran your verification locally instead of taking the numbers on trust, because a claim like "3/3 RED before" is only meaningful if the falsification point is the right commit. Your branch gives a clean one: a6b2a73 is the fix, so 867151b is the same tests against the pre-fix production code.

[A] fec10df (PR head)                      -> 6 passed
[B] tests @ fec10df, config.py+dialect.py @ 867151b
    TestLegacyCodepageMigration            -> 3 failed
        ...config_json_legacy_cp1252_is_ignored_instead_of_crashing
        ...people_map_legacy_cp1252_falls_back_instead_of_crashing
        ...dialect_legacy_cp1252_reports_utf8_migration_error

So the three migration tests are genuinely load-bearing, not passes-either-way. Confirmed too: no errors= anywhere in the diff, so no silent-corruption path was traded in for the crash fix. CI is 9/9 green on fec10df now (it was still running when you posted).

Two notes, neither of them a request for changes.

1. The actionable message lands, but wrapped in a traceback. cmd_compress calls Dialect.from_config() unguarded and main() dispatches at cli.py:2283 without a ValueError handler, so a real user hitting this sees ~25 lines of traceback with your message as the last one:

  File ".../dialect.py", line 364, in from_config
    raise ValueError(f"{config_path} is not valid UTF-8 - re-save it as UTF-8") from exc
ValueError: entities.json is not valid UTF-8 - re-save it as UTF-8

I want to be precise about whose problem that is: it is not a regression from this PR. Pre-fix the same command raised a raw UnicodeDecodeError traceback with a strictly worse message, so your change is an improvement at every step. The bare-traceback-on-user-error shape is a pre-existing CLI-wide gap (same family as #1048) and fixing it means touching the dispatch loop, which has no business in an encoding PR. Flagging it only so it's on the record, not to grow your scope.

2. One thing I checked and am clearing, so nobody re-raises it. The new message contains U+2014, and U+2014 is not encodable in cp932 / cp949 / cp437 / cp850 / cp866 — an error message about bad encoding failing to print would have been a memorable bug. It can't happen: main() calls _reconfigure_stdio_utf8_on_windows() before dispatch, which puts stderr on UTF-8 with errors="replace" on Windows, so the message survives regardless of console codepage. Verified, not assumed — recording it here so the next reader doesn't re-derive it.

Nothing blocking from me. This looks ready to land as far as I'm concerned, and the cp1252_default_open fixture is still the part of this PR I'd most like to see reused elsewhere.

[igorls]

Wave 1 for 3.7.0: LGTM. Focused fix with tests (or trivial deploy/manifest change). Merging into develop for the release train.