diff --git a/.claude/hooks/fluent_paths.py b/.claude/hooks/fluent_paths.py index 6d4d43d..30dc238 100755 --- a/.claude/hooks/fluent_paths.py +++ b/.claude/hooks/fluent_paths.py @@ -24,14 +24,17 @@ def force_utf8_io() -> None: - """Make stdout/stderr UTF-8 so emoji/CJK output doesn't crash on Windows. + """Make stdin/stdout/stderr UTF-8 so emoji/CJK doesn't crash on Windows. Windows consoles default to a legacy code page (cp1252/gbk); printing the - emoji in the hook summaries raises UnicodeEncodeError there. No-op on - platforms whose streams are already UTF-8 or predate ``reconfigure``. - Call once at the top of any hook that prints. + emoji in the hook summaries raises UnicodeEncodeError there. Under an + ASCII/C locale (e.g. Git Bash) stdin is decoded with surrogateescape, so + CJK bytes in a piped payload survive as lone surrogates and blow up only + later, at re-encode time. No-op on platforms whose streams are already + UTF-8 or predate ``reconfigure``. Call once at the top of any hook that + prints or reads a payload. """ - for stream in (sys.stdout, sys.stderr): + for stream in (sys.stdin, sys.stdout, sys.stderr): try: stream.reconfigure(encoding="utf-8") except (AttributeError, ValueError): diff --git a/.claude/hooks/update-db.py b/.claude/hooks/update-db.py index 6743c83..c5d79f0 100755 --- a/.claude/hooks/update-db.py +++ b/.claude/hooks/update-db.py @@ -35,14 +35,31 @@ def load_json(path: Path) -> dict: return json.load(f) -def save_json(path: Path, data: dict): - tmp_path = path.with_suffix('.json.tmp') - with open(tmp_path, 'w', encoding='utf-8') as f: - json.dump(data, f, indent=2, ensure_ascii=False) - f.write('\n') - f.flush() - os.fsync(f.fileno()) - os.replace(str(tmp_path), str(path)) # atomic + overwrites (os.rename fails on Windows if dest exists) +def save_all(files: dict, data: dict): + """Two-phase commit across every DB: stage each one to a .tmp file first, + then swap them all in. A serialization/encoding error during staging + aborts before a single real database is replaced, so exit 2 never leaves + the six files mutually inconsistent (e.g. profile updated but session-log + not).""" + staged = [] + try: + for key, path in files.items(): + tmp_path = path.with_suffix('.json.tmp') + with open(tmp_path, 'w', encoding='utf-8') as f: + json.dump(data[key], f, indent=2, ensure_ascii=False) + f.write('\n') + f.flush() + os.fsync(f.fileno()) + staged.append((tmp_path, path)) + except Exception: + for path in files.values(): + try: + path.with_suffix('.json.tmp').unlink(missing_ok=True) + except OSError: + pass + raise + for tmp_path, path in staged: + os.replace(str(tmp_path), str(path)) # atomic + overwrites (os.rename fails on Windows if dest exists) def parse_date(s: str) -> datetime: @@ -136,11 +153,19 @@ def normalize_milestones(session: dict) -> list: return normalized -def backup_all(tag: str): +def backup_all(tag: str) -> Path: + # Never reuse an existing dir: re-running the same session_id (e.g. a + # retry after a failure) would otherwise overwrite the only backup taken + # before anything went wrong. backup_path = BACKUP_DIR / tag - backup_path.mkdir(parents=True, exist_ok=True) + n = 1 + while backup_path.exists(): + n += 1 + backup_path = BACKUP_DIR / f"{tag}-{n}" + backup_path.mkdir(parents=True) for f in DATA_DIR.glob("*.json"): shutil.copy2(f, backup_path / f.name) + return backup_path # --- SM-2 Algorithm --- @@ -583,8 +608,7 @@ def main(): backup_all(f"pre-update-{session['session_id']}") try: - for k, p in files.items(): - save_json(p, data[k]) + save_all(files, data) except Exception as e: print(f"[Fluent] Error saving databases: {e}", file=sys.stderr) sys.exit(2) diff --git a/CHANGELOG.md b/CHANGELOG.md index 118cf58..0ad01c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,27 @@ All notable changes to Fluent will be documented in this file. +## [Unreleased] + +### Fixed + +- `update-db.py` no longer corrupts learner databases when the session payload + contains non-ASCII text (CJK, Arabic, …) on Windows/Git Bash. `force_utf8_io()` + now reconfigures stdin as well — under an ASCII/C locale the payload was + decoded with surrogateescape and crashed at save time, *after* some databases + had already been written (double-counting stats on retry). +- All six databases are now written with a two-phase commit (stage every file + to `.tmp`, then swap all in), so a serialization or encoding error exits `2` + without touching any database — the documented "no files were modified" + guarantee now actually holds. +- Pre-update backups are no longer overwritten when the same `session_id` is + retried; a numbered sibling directory (`pre-update--2`, `-3`, …) is + created instead, preserving the earliest (pre-corruption) backup. +- Test suite: file reads now pass `encoding="utf-8"` so non-Latin milestone + tests pass on Windows (default cp932/cp1252 locales); added regression tests + for CJK payloads, ASCII-locale runs, backup preservation, and save-failure + atomicity. + ## [0.3.0] — 2026-06-15 ### Added diff --git a/docs/DB_SCRIPTS.md b/docs/DB_SCRIPTS.md index aaee3fb..daf03fe 100644 --- a/docs/DB_SCRIPTS.md +++ b/docs/DB_SCRIPTS.md @@ -108,9 +108,13 @@ Everything else is optional; omitted fields do not update. ### Side effects - Backs up `data/*.json` to `.backups/pre-update-/` *before* - writing. -- Writes each JSON file via a `.tmp` + `fsync` + `rename` pattern so a crash - mid-write cannot leave a half-written file. + writing. If that directory already exists (e.g. a retry of the same + session_id), a numbered sibling (`pre-update--2`, `-3`, …) is + created instead — an earlier backup is never overwritten. +- Writes all six databases with a two-phase commit: every file is staged to a + `.tmp` (with `fsync`) first, then all are swapped in via `rename`. A + serialization or encoding error therefore aborts before any real database + is touched — the six files never end up mutually inconsistent. - Rebuilds `spaced-repetition.review_queue` from scratch each run — any manual edits there will be overwritten. diff --git a/tests/test_update_db.py b/tests/test_update_db.py index 2a1b5f0..f766a32 100644 --- a/tests/test_update_db.py +++ b/tests/test_update_db.py @@ -167,7 +167,7 @@ def test_happy_path(self): self.assertEqual(proc.returncode, 0, msg=f"stdout={proc.stdout!r} stderr={proc.stderr!r}") - with open(self.tmp / "data" / "session-log.json") as f: + with open(self.tmp / "data" / "session-log.json", encoding="utf-8") as f: log = json.load(f) latest = log["sessions"][-1] self.assertEqual(latest["session_id"], "session-002") @@ -180,7 +180,7 @@ def test_happy_path(self): self.assertIn("achievements_earned", latest) self.assertEqual(latest["streak_day"], 3) # was 2, yesterday -> +1 - with open(self.tmp / "data" / "learner-profile.json") as f: + with open(self.tmp / "data" / "learner-profile.json", encoding="utf-8") as f: profile = json.load(f) self.assertEqual(profile["current_streak_days"], 3) conf = profile["skills"]["vocabulary"]["confidence"] @@ -188,7 +188,7 @@ def test_happy_path(self): self.assertGreaterEqual(conf, 0) self.assertLessEqual(conf, 100) - with open(self.tmp / "data" / "spaced-repetition.json") as f: + with open(self.tmp / "data" / "spaced-repetition.json", encoding="utf-8") as f: sr = json.load(f) dag = sr["items"]["vocab_dag"] # Schema preserved @@ -208,7 +208,7 @@ def test_happy_path(self): "total_reviews", "priority"): self.assertIn(k, huis, f"new item missing {k}") - with open(self.tmp / "data" / "mistakes-db.json") as f: + with open(self.tmp / "data" / "mistakes-db.json", encoding="utf-8") as f: mistakes = json.load(f) self.assertIn("verb_spreek", mistakes["error_patterns"]) pat = mistakes["error_patterns"]["verb_spreek"] @@ -232,7 +232,7 @@ def test_same_day_does_not_bump_streak(self): payload["date"] = "2026-04-23" proc = self._run(payload) self.assertEqual(proc.returncode, 0, msg=proc.stderr) - with open(self.tmp / "data" / "learner-profile.json") as f: + with open(self.tmp / "data" / "learner-profile.json", encoding="utf-8") as f: profile = json.load(f) self.assertEqual(profile["current_streak_days"], 2) @@ -246,7 +246,7 @@ def _payload_with(self, session_id, milestones, date="2026-04-24"): return payload def _load(self, name): - with open(self.tmp / "data" / name) as f: + with open(self.tmp / "data" / name, encoding="utf-8") as f: return json.load(f) def test_milestone_string_form(self): @@ -349,6 +349,78 @@ def test_milestone_non_latin_distinct_nonempty_ids(self): for i in ids: self.assertFalse(i.endswith("_"), f"bare trailing underscore: {i}") + def test_cjk_payload_roundtrips_utf8(self): + # Regression: under an ASCII/C locale (Git Bash on Windows), stdin was + # decoded with surrogateescape and CJK payloads crashed at save time, + # after some databases were already written. + payload = dict(SESSION_PAYLOAD) + payload["session_id"] = "session-300" + payload["errors"] = [dict(SESSION_PAYLOAD["errors"][0], + your_answer="achieve = 実績", + correct_answer="achieve = 達成する（動詞）")] + proc = self._run(payload) + self.assertEqual(proc.returncode, 0, + msg=f"stdout={proc.stdout!r} stderr={proc.stderr!r}") + with open(self.tmp / "data" / "mistakes-db.json", encoding="utf-8") as f: + mistakes = json.load(f) + example = mistakes["error_patterns"]["verb_spreek"]["examples"][-1] + self.assertEqual(example["incorrect"], "achieve = 実績") + self.assertEqual(example["correct"], "achieve = 達成する（動詞）") + + def test_cjk_payload_survives_ascii_locale(self): + # Same as above but forcing the worst-case locale explicitly. + payload = dict(SESSION_PAYLOAD) + payload["session_id"] = "session-301" + payload["session_notes"] = "過去形と冠詞のリベンジ成功" + env = dict(os.environ, LC_ALL="C", LANG="C") + env.pop("PYTHONUTF8", None) + env.pop("PYTHONIOENCODING", None) + proc = subprocess.run( + ["python3", str(SCRIPT)], + input=json.dumps(payload).encode(), + cwd=str(self.tmp), + capture_output=True, + env=env, + ) + self.assertEqual(proc.returncode, 0, + msg=f"stdout={proc.stdout!r} stderr={proc.stderr!r}") + with open(self.tmp / "data" / "session-log.json", encoding="utf-8") as f: + log = json.load(f) + self.assertEqual(log["sessions"][-1]["notes"], "過去形と冠詞のリベンジ成功") + + def test_rerun_same_session_id_preserves_first_backup(self): + proc = self._run(SESSION_PAYLOAD) + self.assertEqual(proc.returncode, 0, msg=proc.stderr) + first = self.tmp / "data" / ".backups" / "pre-update-session-002" + marker = json.loads((first / "learner-profile.json").read_text()) + + proc = self._run(SESSION_PAYLOAD) + self.assertEqual(proc.returncode, 0, msg=proc.stderr) + second = self.tmp / "data" / ".backups" / "pre-update-session-002-2" + self.assertTrue(second.exists(), "retry should get a numbered backup dir") + # First backup still holds the original pre-update state. + preserved = json.loads((first / "learner-profile.json").read_text()) + self.assertEqual(preserved, marker) + + def test_save_failure_leaves_all_databases_unchanged(self): + # Force a staging failure by making one destination's .tmp path a + # directory; the two-phase commit must abort before replacing any DB. + before = {name: (self.tmp / "data" / name).read_text() + for name in ("learner-profile.json", "progress-db.json", + "mistakes-db.json", "mastery-db.json", + "spaced-repetition.json", "session-log.json")} + blocker = self.tmp / "data" / "session-log.json.tmp" + blocker.mkdir() + try: + proc = self._run(SESSION_PAYLOAD) + self.assertEqual(proc.returncode, 2, msg=proc.stderr) + after = {name: (self.tmp / "data" / name).read_text() + for name in before} + self.assertEqual(after, before, + "a failed save must not modify any database") + finally: + blocker.rmdir() + def test_milestones_empty_and_omitted_are_noops(self): for n, payload in enumerate([ self._payload_with("session-107", []),