Skip to content

fix(storage): write local JSON in stable numeric key order - #66

Open
alycda wants to merge 7 commits into
taskbook-sh:masterfrom
alycda:fix/stable-storage-order
Open

alycda wants to merge 7 commits into
taskbook-sh:masterfrom
alycda:fix/stable-storage-order

Conversation

@alycda

@alycda alycda commented Oct 23, 2026

Copy link
Copy Markdown

Fixes #65.

LocalStorage now writes storage.json and archive.json in ascending numeric key order, so the same data always produces the same bytes and a change to one item changes only that item's lines.

Change

A small SortedItems wrapper sorts the entries when they're serialized. Numeric keys come first in numeric order. Any non-numeric keys come after them in lexical order. Distinct keys with the same value, such as "1" and "01", are ordered lexically, so the output never depends on HashMap iteration order.

The on-disk schema, the StorageBackend trait and the in-memory types are unchanged. For IDs taskbook generates, the order matches what the Node.js version wrote. The remote backend is not touched.

Considered and left out

  • Changing StorageBackend to a BTreeMap. This would make order deterministic everywhere, but it touches taskbook.rs, the TUI, MCP and the remote backend. A BTreeMap<u64, _> would also reject non-numeric keys that load today.
  • Indentation. The Rust port indents with 2 spaces and the Node.js version used 4. Users who switch between the two still get one whole-file diff after each switch. Matching Node would rewrite every existing Rust user's file once, so I've left the decision to you.
  • fsync. The temp-file-and-rename write does not fsync the file or the directory before the rename. That was already the case before this change. It's a separate durability question.

Testing

  • cargo fmt --check and cargo clippy --workspace --all-targets -- -D warnings are clean. cargo test --workspace passes.
  • There are 4 new unit tests in storage::local. The order and same-bytes tests fail when the sort is removed. The tie test failed before its fix.
  • End to end, with 12 tasks under git:
Action Before After
--check 3 33 lines added, 33 removed 1 line (isComplete)
--check 3 twice (no net change) 30 lines added, 30 removed no diff
--delete 2 not measured only that item's 14 lines removed

The "before" numbers come from the 8-task reproduction in #65.

LocalStorage serialized a HashMap directly, so every write emitted keys
in a per-process random order. Any change, or none, rewrote the whole
file, which makes storage.json and archive.json unusable under git or jj.

Sort entries by numeric key on write (non-numeric keys last, lexically),
matching the order the Node.js version's JSON.stringify produced. The
on-disk schema and the in-memory types are unchanged.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01DsL2SpgcXHswgy5n42oi7Z
u64::from_str accepts "01" and "+1", so distinct keys could compare
Equal and keep HashMap iteration order, which made the output
nondeterministic again for hand-edited files. Break such ties lexically.

Review finding (code-review on 748e42e).

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01DsL2SpgcXHswgy5n42oi7Z
JSON.stringify sorts only canonical array-index keys and keeps other
keys in insertion order, so the match holds for taskbook's own IDs,
not for arbitrary keys like "01".

Review finding (code-review on 748e42e).

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01DsL2SpgcXHswgy5n42oi7Z
Each run left two tb-local-test-<uuid>/ trees in the system temp dir.
Clean up at the end of each test, as config.rs's tests do.

Review finding (code-review on 748e42e).

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01DsL2SpgcXHswgy5n42oi7Z
group_by_board and group_by_date push items while iterating a HashMap,
so the CLI board view, --find, --list, --timeline and --archive list a
group's items in a different order on every run. Node.js taskbook
listed them in ID order (Object.keys). These tests fail on this commit
by design; the next commit makes them pass.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01DsL2SpgcXHswgy5n42oi7Z
group_by_board and group_by_date now walk items in ascending key order,
using the same compare_keys that orders the storage file, so CLI
listings no longer follow HashMap iteration order. Makes the tests from
the previous commit pass.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01DsL2SpgcXHswgy5n42oi7Z
Sort with item.id() instead of exporting the storage key comparator.
The CLI, TUI and MCP now share one notion of ID order, and compare_keys
stays private to the storage file format.

Review finding (code-review on 973898a...cf0db59).

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01DsL2SpgcXHswgy5n42oi7Z
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Local storage writes keys in random order, so every write rewrites the whole file

2 participants