feat(user_dict): durable save + recovering open (LXUW hardening)#290
Merged
Conversation
Harden the user dictionary (LXUW) with the same durability/recovery
discipline as LXUD, using the shared `crate::persist` primitives — the
minimal, format-invariant hardening from design §9 (no WAL: a few-dozen
entries with explicit `save`, so the WAL machinery earns nothing here).
- **save**: `persist::write_atomic` (tmp → F_FULLFSYNC → rename → best-effort
dir fsync). Fixes the unsynced write and the `with_extension("tmp")` stray
`user_dict.tmp` — the two bugs LXUD had already fixed in its copy.
- **from_bytes**: bounded reader (`persist::bincode_reader_v1`, allow-trailing
for this no-CRC / v1-class format) so a corrupt length prefix can't trigger
a giant allocation.
- **open_recovering**: corruption (bad magic / version / bincode / length) is
quarantined (`.corrupt-<ts>`, bytes preserved, newest 3 kept) and an empty
dictionary is returned — closing the path where a corrupt file re-threw
every startup and registered words were lost forever. `Err` is reserved for
environmental read failures (EACCES); quarantine-rename failure still yields
an empty usable dict. The strict, side-effect-free `open` stays for the CLI.
- **FFI/Swift**: `LexUserDictionary::open` no longer throws on corruption;
`open_report()` surfaces `data_loss_suspected` (computed Rust-side).
EngineContainer records it on the success path (not `catch`) as a new
`.userDictionaryDataLoss` case, distinct from `.userDictionary` (env failure
→ dict unavailable) — mirroring the `.history` / `.historyDataLoss` split.
This supersedes design §10's "reuse `.userDictionary`" note (settled in
AGENTS.md).
Format unchanged (LXUW magic + version 1 + bincode) — no migration. SPEC.md
gains the symmetric user_dict recovery/offline-tool lines. Version-bump policy
(§7) documented on the VERSION const.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Harden the user dictionary (LXUW) with the same durability/recovery discipline
as the learning history (LXUD), using the shared
crate::persistprimitivesextracted in #289. This is the minimal, format-invariant hardening from
design §9 — deliberately no WAL/seq/migration (a few-dozen-entry store with
explicit
save, where that machinery earns nothing).persist::write_atomic(tmp → F_FULLFSYNC → rename → best-effortdir fsync). Fixes the unsynced write and the
with_extension("tmp")strayuser_dict.tmp— the two bugs LXUD had already fixed in its own copy.persist::bincode_reader_v1(allow-trailing, matching theoriginal plain
bincode::deserialize, for this no-CRC/v1-class format) with anallocation cap so a corrupt length prefix can't trigger a giant allocation.
quarantined (
.corrupt-<ts>, bytes preserved, newest 3 kept) and an emptydictionary is returned. Closes the path where a corrupt file re-threw every
startup and registered words were lost forever.
Erris reserved forenvironmental read failures (EACCES); quarantine-rename failure still yields an
empty usable dict (loss still surfaced via the report). The strict,
side-effect-free
openstays for the CLI (an audit tool must never rename alive user's file — enforced by construction, pinned by a test).
LexUserDictionary::openno longer throws on corruption;open_report()surfacesdata_loss_suspected(computed Rust-side).EngineContainerrecords it on the success path (notcatch, where itwould never fire) as a new
.userDictionaryDataLosscase, distinct from.userDictionary(env failure → dict unavailable) — mirroring the merged.history/.historyDataLosssplit. This supersedes design §10's older"reuse
.userDictionary" note (recorded as settled inAGENTS.md).Scope / compatibility
to_bytesisuntouched, output byte-identical, no migration owed.
crate::persistextraction); this is PR3b of that 2-PRsplit. Diff here is the user_dict hardening only.
version-bump policy (§7) is documented on the
VERSIONconst.Verification (local pre-push gate)
cargo fmt --all --check,clippy --workspace --all-features -D warningscargo test --workspace --all-features— 594 tests, 0 failed (7 newuser_dict tests: quarantine, rotation, strict-open-no-side-effect,
no-stray-tmp, oversized-length cap, clean/nonexistent)
mise run build(Swift bindings regenerated) +mise run test-swift—139 Swift tests, 0 failed
cargo deny/vet/machete/simplify(4 agents) +/code-review high(correctness +conventions) +
/lexime-review(Axis 3 structural / 4 UniFFI / 5 context) —all 0 findings
Test plan
.corrupt-*, bytes preserved) + empty usable dict + report flags lossopen(CLI) still errors on corruption without renaming.tmp; oversized length prefix rejected🤖 Generated with Claude Code