Random sort: review fixes (null seed, 2^53 seed bound, real tests), README

- library.shuffle treats a null seed as none given
- seeds are limited to 0..2^53-1 so JSON-double clients read them back exactly
- album test now fails if manual order overrides Random; persistence test goes through
  close/reopen of the library instead of a serde round trip
- README lists random among the sort options

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

README.md

@@ -200,7 +200,7 @@ A library that stays out of your way: **All Photos**, **Recently Added**, **Pick
 **Folders**, and **Recently Deleted**. Rate with <kbd>0</kbd>–<kbd>5</kbd>, flag with <kbd>P</kbd> / <kbd>X</kbd> /
 <kbd>U</kbd>, colour-label with <kbd>6</kbd>–<kbd>9</kbd>. Search understands fields:
 `rating:>3 flag:pick iso:>800 camera:x2 date:2026-04 keyword:mountains`. Every view sorts by capture date,
-import date, edit date, name, rating or size. The justified **Photo Grid** and **Square Grid** views are virtualized,
+import date, edit date, name, rating, size or at random (a stable shuffle; View → Sort → Reshuffle for a new one). The justified **Photo Grid** and **Square Grid** views are virtualized,
 so they stay smooth whether you have forty photos or forty thousand.
 
 <table>

crates/engine/src/cmd/library.rs

@@ -224,11 +224,14 @@ fn select_by(s: &mut Session, p: &Value) -> Result<Value> {
     Ok(json!({"selected": n}))
 }
 
-/// `seed` as a non-negative integer, or `default` when absent; anything else is an error.
+/// Seeds stay exactly representable as a JSON double, so web and agent clients read back what they wrote.
+const MAX_SEED: u64 = 1 << 53;
+
+/// `seed` as an integer in `0..2^53`, or `default` when absent or null; anything else is an error.
 fn seed_param(p: &Value, cmd: &str, default: u64) -> Result<u64> {
     match p.get("seed") {
         None | Some(Value::Null) => Ok(default),
-        Some(v) => v.as_u64().ok_or_else(|| bad(cmd, "seed must be a non-negative integer")),
+        Some(v) => v.as_u64().filter(|n| *n < MAX_SEED).ok_or_else(|| bad(cmd, "seed must be an integer from 0 to 2^53 - 1")),
     }
 }
 
@@ -237,7 +240,7 @@ fn seed_param(p: &Value, cmd: &str, default: u64) -> Result<u64> {
 /// clients that read numbers as doubles (web, MCP agents) get the same seed back.
 fn next_seed(prev: u64, now: &str) -> u64 {
     let t = now.bytes().fold(0u64, |h, b| lightcraft_catalog::mix64(h ^ u64::from(b)));
-    (lightcraft_catalog::mix64(prev.wrapping_add(1)) ^ t) & ((1 << 53) - 1)
+    (lightcraft_catalog::mix64(prev.wrapping_add(1)) ^ t) & (MAX_SEED - 1)
 }
 
 pub fn specs() -> Vec<CommandSpec> {
@@ -318,11 +321,11 @@ pub fn specs() -> Vec<CommandSpec> {
             "Reshuffle",
             ["View", "Sort"],
             None,
-            "{seed?: u64} — sort at random; without `seed` a new shuffle each time",
+            "{seed?: 0..2^53-1} — sort at random; without `seed` a new shuffle each time",
             always,
             |s, p| {
                 let seed = match seed_param(p, "library.shuffle", s.sort.seed)? {
-                    given if p.get("seed").is_some() => given,
+                    given if p.get("seed").is_some_and(|v| !v.is_null()) => given,
                     // derived from the previous seed and the session clock: reproducible under a test
                     // clock, and no RNG needed
                     prev => next_seed(prev, &(s.clock)()),

crates/engine/src/tests_libops.rs

@@ -494,8 +494,8 @@ fn random_sort_is_stable_until_reshuffled() {
     assert_eq!(s.sort.key, SortKey::Random);
     assert_ne!(seed1, 11);
     assert_ne!(s.visible_cloned(), a);
-    s.execute("library.shuffle", &json!({})).unwrap();
-    assert_ne!(s.sort.seed, seed1, "every reshuffle is new");
+    s.execute("library.shuffle", &json!({"seed": null})).unwrap();
+    assert_ne!(s.sort.seed, seed1, "every reshuffle is new, and a null seed means none given");
     assert!(s.sort.seed < 1 << 53, "seeds survive a round trip through a JSON double");
     // an explicit seed is reproducible
     s.execute("library.shuffle", &json!({"seed": 11})).unwrap();
@@ -509,14 +509,18 @@ fn random_sort_rejects_bad_params() {
     assert!(s.execute("library.sort", &json!({"seed": "x"})).is_err());
     assert!(s.execute("library.sort", &json!({"seed": -1})).is_err());
     assert!(s.execute("library.shuffle", &json!({"seed": 1.5})).is_err());
-    assert_eq!(s.sort, before, "a rejected command leaves the sort alone");
+    assert!(s.execute("library.sort", &json!({"seed": 1u64 << 53})).is_err(), "beyond what a JSON double holds exactly");
+    assert!(s.execute("library.sort", &json!({"seed": (1u64 << 53) - 1})).is_ok());
+    assert_eq!(s.sort.key, before.key, "a rejected command leaves the sort alone");
 }
 
 #[test]
-fn random_sort_applies_to_albums_and_folders_too() {
+fn random_sort_applies_to_albums_too() {
     let mut s = Session::with_demo();
     s.execute("library.sort", &json!({"key": "random", "seed": 5})).unwrap();
     let all = s.visible_cloned();
+    // manual order shows newest-first (descending), i.e. the reverse of this insertion order, so it
+    // differs from the shuffle unless Random really overrides it
     let ids: Vec<u64> = all.iter().take(12).map(|i| i.0).collect();
     let album = s.execute("album.create", &json!({"name": "Shuffled"})).unwrap()["id"].as_u64().unwrap();
     s.execute("album.addPhotos", &json!({"id": album, "ids": ids})).unwrap();
@@ -526,12 +530,3 @@ fn random_sort_applies_to_albums_and_folders_too() {
     let expect: Vec<_> = all.iter().filter(|i| in_album.contains(i)).copied().collect();
     assert_eq!(in_album, expect, "the album shows its photos in the shuffle's order, not manual order");
 }
-
-/// The shuffle comes back after a restart (the sort is part of the saved view state).
-#[test]
-fn random_sort_survives_view_state_round_trip() {
-    use lightcraft_catalog::{Sort, SortKey};
-    let sort = Sort { key: SortKey::Random, seed: u64::MAX, ..Default::default() };
-    let back: Sort = serde_json::from_str(&serde_json::to_string(&sort).unwrap()).unwrap();
-    assert_eq!(back, sort);
-}

crates/engine/src/tests_organize.rs

@@ -242,3 +242,17 @@ fn filter_chips_clear_one_filter_and_total_ignores_the_filter() {
     }
     assert_eq!(s.filter, Default::default());
 }
+
+/// A shuffle is part of the saved view: the same seed (and so the same grid) is back on the next launch.
+#[test]
+fn random_sort_persists_with_the_view() {
+    let dir = temp_dir("random-view");
+    let mut s = open(&dir);
+    s.execute("library.sort", &json!({"key": "random", "seed": 123_456_789})).unwrap();
+    s.close_library().unwrap();
+    drop(s);
+    let s2 = open(&dir);
+    assert_eq!(s2.sort.key, lightcraft_catalog::SortKey::Random);
+    assert_eq!(s2.sort.seed, 123_456_789);
+    let _ = std::fs::remove_dir_all(&dir);
+}