fix(sleep): read and write state.json as UTF-8

[PerryLink (PerryLink)]

skillopt_sleep/state.py reads and writes state.json with no explicit encoding, while save() deliberately passes ensure_ascii=False. The file therefore lands in the platform's locale charset, and two things follow -- the second is the serious one.

1. On a locale that cannot represent the text, save() raises. On a GBK machine:

UnicodeEncodeError: 'gbk' codec can't encode character '\u2705' in position ...

2. Even when it does not raise, the bytes are not UTF-8, and load() swallows the failure. load() wraps its decode in a bare except Exception: pass, so any reader whose locale codec is UTF-8 -- a container, CI, WSL, PYTHONUTF8=1, or simply another machine -- gets a decode error, discards it, and returns a fresh state:

locale.getencoding() = cp936
bytes on disk decode as UTF-8 : NO -> 'utf-8' codec can't decode byte 0xd6 in position 67
bytes on disk decode as GBK   : yes
as UTF-8  -> UnicodeDecodeError; load() swallows it via `except Exception: pass`
             -> a fresh state, i.e. every field silently reset

Night counter, harvest cursors, history and the cross-night memory all disappear without a warning. The file is readable only by the machine that wrote it.

This is the defect #124 fixed in skillopt/config.py -- "read YAML config files as UTF-8", merged 2026-07-12, one line:

-    with open(abs_path) as f:
+    with open(abs_path, encoding="utf-8") as f:

state.py was simply missed by that sweep. The change here is the same two lines:

-                with open(path) as f:
+                with open(path, encoding="utf-8") as f:
...
-        with open(tmp, "w") as f:
+        with open(tmp, "w", encoding="utf-8") as f:

Tests

Two tests in tests/test_sleep_state.py, and the second exists because the first is not sufficient:

• test_state_file_round_trips_non_ascii_as_utf8 -- saves a CJK lesson and a CJK project path, asserts the bytes on disk decode as UTF-8, and asserts the round-trip preserves both.
• test_state_io_does_not_fall_back_to_the_locale_codec -- repeats the round-trip in a subprocess under -X warn_default_encoding -W error::EncodingWarning.

The behavioural test cannot fail on a UTF-8 CI runner, because there the locale codec is UTF-8. On its own it would ship a regression test that never goes red. The warning-to-error guard makes CPython's own EncodingWarning a failure on every platform, which is what actually pins the fix.

Before (source fix reverted, tests kept):

FAILED tests/test_sleep_state.py::test_state_file_round_trips_non_ascii_as_utf8
FAILED tests/test_sleep_state.py::test_state_io_does_not_fall_back_to_the_locale_codec
EncodingWarning: 'encoding' argument not specified
2 failed, 1 passed

After:

3 passed

[Yifan Yang (Yif-Yang)]

Explicit UTF-8 for newly written state is useful, but the October 6 review of 129d249b4ab8 found an upgrade data-loss regression that must be addressed before merge.

A state file successfully produced and read by the existing writer under GBK or cp1252 becomes unreadable by the new UTF-8-only loader on that same simulated locale. The existing broad exception handler then returns fresh state, and the next save overwrites the original memory/history/cursors. The baseline producer was used to verify the legacy fixture bytes, not merely a hand-written corrupt file.

The positive state slice passed 13 tests. The independent upgrade slice had 4 failures and 1 UTF-8 control pass; the original-main compatibility controls passed. These are Linux simulations of the target module's implicit codec, not native Windows results. No user data was accessed.

Please preserve UTF-8 for new writes while adding an explicit legacy migration/compatibility path, or at minimum fail with an actionable diagnostic and prevent overwriting an undecodable existing state. Add a load-then-save upgrade regression that preserves prior fields, including unknown fields. This is a data migration, not a safe two-line change to merge first. Related #304 has the analogous configuration-upgrade concern.

[PerryLink (PerryLink)]

Fixed in 9dc00d4, and you were right that this is a data migration rather than a two-line change.

What the previous revision did. I reproduced the loss directly, with a state file written by the old locale-codec writer:

loader on a GBK state file   -> night = 0     (the whole state discarded)
after load + save            -> night = 1, slow_memory = '', history = [],
                                unknown field gone

The file on disk was replaced by a fresh one. except Exception: pass is what made it silent — a reset state is indistinguishable from a first run.

The change. load() now reads bytes and decodes them as UTF-8 first; on UnicodeDecodeError it falls back to the locale codec, which is the one the old writer used on the machine that produced the file, and returns that data. The next save() rewrites it as UTF-8, so the migration completes on the first write instead of needing a separate tool.

loader on the same GBK file  -> night = 42, slow_memory and history intact,
                                unknown field preserved
after load + save            -> night = 43, every field carried over, file now UTF-8

A file that decodes under neither is no longer replaced. load() raises StateFileError, whose message names the path and says it was left untouched. I took the louder option rather than "return fresh state but block save()", because continuing would still run a night against a reset counter and empty harvest cursors — the failure belongs before the work, not after it. That is a deliberate behaviour change to load(), so it is worth a look: it now raises where it used to return a default.

Tests. test_legacy_locale_encoded_state_is_migrated_not_discarded (load then save, asserting every field survives including an unknown one, and that the bytes end up UTF-8) and test_unreadable_state_is_reported_and_left_untouched (raises, and the bytes are identical afterwards). Both fail against the previous revision — I ran them there to confirm — and neither can pass by accident on a UTF-8 runner, since the locale codec is injected and the fixture is asserted to be genuinely non-UTF-8.

tests/test_sleep_state.py: 5 passed. The 4 failures in tests/test_model_change_warning.py are a missing optional llm_client import and are present before this change as well.

Thanks for testing this with the real producer rather than a hand-written corrupt file — a hand-written fixture would have missed the case entirely.

[PerryLink (PerryLink)]

@microsoft-github-policy-service agree