Merge PR #2423: PERF-SMART: test a smart album's rules cheapest first

docs/whats-new.md: kept both entries under the same date.

crates/catalog/src/lib.rs

@@ -294,6 +294,10 @@ pub struct Catalog {
     /// Lookups derived from the above (never saved), see [`index`].
     #[serde(skip)]
     index: index::Indexes,
+    /// Each smart album's rules ready to run (cheapest rule first), worked out when first asked and
+    /// forgotten when an op adds or removes an album or changes rules.
+    #[serde(skip)]
+    plans: PlansMemo,
 }
 
 /// What of a photo an op changed: enough to tell which tallies of the library an edit leaves as they were (a
@@ -332,6 +336,32 @@ impl Aspects {
 /// The most photos remembered as touched; older changes are then answered with "anything may have changed".
 const MAX_TOUCHED: usize = 8192;
 
+/// [`Catalog::plans`] as last worked out: derived from the albums, like [`index::Indexes`], so no
+/// part of what a catalog is; a copy starts without it.
+#[derive(Default)]
+struct PlansMemo {
+    plans: std::sync::OnceLock<std::collections::HashMap<AlbumId, query::Prepared>>,
+    guard: std::sync::OnceLock<bool>,
+}
+
+impl Clone for PlansMemo {
+    fn clone(&self) -> Self {
+        Self::default()
+    }
+}
+
+impl PartialEq for PlansMemo {
+    fn eq(&self, _: &PlansMemo) -> bool {
+        true
+    }
+}
+
+impl std::fmt::Debug for PlansMemo {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.write_str("PlansMemo")
+    }
+}
+
 /// A short memory of what the latest ops changed, so that what is derived from the catalog (the grid's list) can
 /// follow a small edit without being worked out again from every photo.
 #[derive(Clone, Debug, Default)]
@@ -442,12 +472,61 @@ impl Catalog {
     /// no smart album).
     pub fn album_contains(&self, id: AlbumId, p: &Photo) -> bool {
         match self.albums.get(&id) {
-            Some(Album { smart: Some(rules), .. }) => self.smart_album_has(id, rules, &rules.prepare_one(), p),
+            Some(Album { smart: Some(rules), .. }) => match self.plans().get(&id) {
+                Some(plan) => self.smart_album_has(id, rules, plan, p),
+                None => self.smart_album_has(id, rules, &rules.prepare_one(self), p),
+            },
             Some(a) => self.album_lists(a.id, p.id),
             None => false,
         }
     }
 
+    /// Every smart album's [`Filter::prepare_one`], worked out once for the albums as they are
+    /// (so a photo asked about does not prepare the album's rules again).
+    fn plans(&self) -> &std::collections::HashMap<AlbumId, query::Prepared> {
+        self.plans.plans.get_or_init(|| self.albums.values().filter_map(|a| a.smart.as_ref().map(|rules| (a.id, rules.prepare_one(self)))).collect())
+    }
+
+    /// Whether rules must be tested as written, not cheapest first ([`Filter::prepare`]): some smart
+    /// album tests itself (a saved or damaged catalog; the app refuses to make one), or a chain of
+    /// smart albums testing smart albums is as long as [`rules::MAX_ALBUM_DEPTH`], where the
+    /// innermost hold nothing. There an answer can depend on which album was asked first (the
+    /// answers memo keeps what was worked out under the cut), so reordering could change who
+    /// matches. Worked out once for the albums as they are.
+    pub(crate) fn keeps_written_order(&self) -> bool {
+        *self.plans.guard.get_or_init(|| {
+            self.albums.values().any(|a| a.smart.is_some() && self.album_reaches(a.id, a.id)) || self.longest_chain() >= rules::MAX_ALBUM_DEPTH
+        })
+    }
+
+    /// The most smart albums one smart album leads through, itself included, by the smart albums
+    /// its rules and album filter test. Only asked of albums that lead back to none.
+    fn longest_chain(&self) -> usize {
+        let tests = |id: &AlbumId| -> Vec<AlbumId> {
+            let tested = self.albums.get(id).and_then(|a| a.smart.as_deref()).map(Filter::albums_tested).unwrap_or_default();
+            tested.into_iter().filter(|t| self.albums.get(t).is_some_and(|a| a.smart.is_some())).collect()
+        };
+        let mut chain: std::collections::HashMap<AlbumId, usize> = std::collections::HashMap::new();
+        for root in self.albums.values().filter(|a| a.smart.is_some()).map(|a| a.id) {
+            let mut stack = vec![root];
+            while let Some(a) = stack.last().copied() {
+                if chain.contains_key(&a) {
+                    stack.pop();
+                    continue;
+                }
+                let kids = tests(&a);
+                let pending: Vec<AlbumId> = kids.iter().copied().filter(|k| !chain.contains_key(k)).collect();
+                if pending.is_empty() {
+                    chain.insert(a, 1 + kids.iter().filter_map(|k| chain.get(k)).copied().max().unwrap_or(0));
+                    stack.pop();
+                } else {
+                    stack.extend(pending);
+                }
+            }
+        }
+        chain.values().copied().max().unwrap_or(0)
+    }
+
     /// Whether smart album `id` with `rules` (as [`Filter::prepare`]d) holds `p`.
     fn smart_album_has(&self, id: AlbumId, rules: &Filter, prepared: &query::Prepared, p: &Photo) -> bool {
         // guarded: a smart album testing smart albums can't loop or recurse without end
@@ -502,7 +581,7 @@ impl Catalog {
     pub fn album_photos(&self, id: AlbumId) -> Vec<PhotoId> {
         match self.albums.get(&id) {
             Some(Album { smart: Some(rules), .. }) => {
-                let prepared = rules.prepare();
+                let prepared = rules.prepare(self);
                 self.photos.values().filter(|p| self.smart_album_has(id, rules, &prepared, p)).map(|p| p.id).collect()
             }
             Some(a) => a.photos.clone(),
@@ -515,7 +594,7 @@ impl Catalog {
         match self.albums.get(&id) {
             // counted in place: no id list is built just for its length
             Some(Album { smart: Some(rules), .. }) => {
-                let prepared = rules.prepare();
+                let prepared = rules.prepare(self);
                 self.photos.values().filter(|p| self.smart_album_has(id, rules, &prepared, p)).count()
             }
             Some(a) => a.photos.iter().filter(|p| self.photos.get(p).is_some_and(|p| !p.deleted)).count(),
@@ -779,6 +858,7 @@ impl Catalog {
                 Op::SetHistoryCursor { id, cursor: std::mem::replace(&mut p.history_cursor, cursor) }
             }
             Op::AddAlbum { album } => {
+                self.plans = PlansMemo::default();
                 if self.albums.contains_key(&album.id) {
                     return Err(CatalogError::Invalid(format!("album {:?} exists", album.id)));
                 }
@@ -800,6 +880,7 @@ impl Catalog {
                 Op::RemoveAlbum { id }
             }
             Op::RemoveAlbum { id } => {
+                self.plans = PlansMemo::default();
                 if self.albums.values().any(|a| a.parent == Some(id)) {
                     return Err(CatalogError::Invalid("folder is not empty".into()));
                 }
@@ -860,6 +941,7 @@ impl Catalog {
                 Op::SetAlbumCover { id, cover: std::mem::replace(&mut a.cover, cover) }
             }
             Op::SetAlbumRules { id, rules } => {
+                self.plans = PlansMemo::default();
                 self.validate_rules(&rules)?;
                 let a = self.album_mut(id)?;
                 let Some(old) = a.smart.as_mut() else {
@@ -1149,6 +1231,8 @@ impl Catalog {
         if !changed.is_empty() {
             // stacks and albums changed outside `apply`: the lookups are built again when asked
             self.index = index::Indexes::default();
+            // (and what was worked out from the albums, the guard of [`Self::keeps_written_order`] too)
+            self.plans = PlansMemo::default();
         }
         changed
     }
@@ -1209,6 +1293,8 @@ mod tests_lock;
 #[cfg(test)]
 mod tests_repair;
 #[cfg(test)]
+mod tests_rule_order;
+#[cfg(test)]
 mod tests_scale;
 #[cfg(test)]
 mod tests_tally;

crates/catalog/src/query.rs

@@ -143,6 +143,9 @@ fn shuffle_rank(seed: u64, id: PhotoId) -> u64 {
 pub(crate) struct Prepared {
     root: Option<String>,
     only: Option<std::collections::HashSet<PhotoId>>,
+    /// [`Filter::rule_set`] with its rules in the order to test them ([`crate::RuleSet::by_cost`]);
+    /// `None` tests them as written.
+    rules: Option<crate::RuleSet>,
 }
 
 /// The tokens of a search text: split at whitespace, except inside double quotes, so
@@ -329,18 +332,35 @@ impl Filter {
     }
 
     pub fn matches(&self, p: &Photo, cat: &Catalog) -> bool {
-        self.matches_in(p, cat, &self.prepare_one())
+        // one photo: its rules are tested as written (ordering them would cost more than it saves)
+        self.matches_in(p, cat, &Prepared { root: self.library_root(), only: None, rules: None })
     }
 
-    /// [`Filter::prepare`] for testing one photo (the `only` ids are looked through, not hashed).
-    pub(crate) fn prepare_one(&self) -> Prepared {
-        Prepared { root: self.library_root(), only: None }
+    /// [`Filter::prepare`] for testing one photo at a time, with what each needs worked out once
+    /// (the `only` ids are looked through, not hashed).
+    pub(crate) fn prepare_one(&self, cat: &Catalog) -> Prepared {
+        Prepared { root: self.library_root(), only: None, rules: self.ordered_rules(cat) }
+    }
+
+    /// The rule set to test, cheapest rule first: once per prepared filter, not per photo. `None`
+    /// (as written) while any smart album tests itself or a chain is too deep ([`Catalog::keeps_written_order`]), where the
+    /// order could change which photos match.
+    fn ordered_rules(&self, cat: &Catalog) -> Option<crate::RuleSet> {
+        if cat.keeps_written_order() {
+            return None;
+        }
+        self.rule_set.as_ref().map(|rs| rs.by_cost(cat))
     }
 
     /// What [`Filter::matches_in`] needs worked out once for a pass over many photos rather than
-    /// once per photo: the library folder's identity and the `only` ids as a set.
-    pub(crate) fn prepare(&self) -> Prepared {
-        Prepared { root: self.library_root(), only: (!self.only.is_empty()).then(|| self.only.iter().copied().collect()) }
+    /// once per photo: the library folder's identity, the `only` ids as a set and the rules in the
+    /// order to test them.
+    pub(crate) fn prepare(&self, cat: &Catalog) -> Prepared {
+        Prepared {
+            root: self.library_root(),
+            only: (!self.only.is_empty()).then(|| self.only.iter().copied().collect()),
+            rules: self.ordered_rules(cat),
+        }
     }
 
     /// [`Filter::library_folder`] as a folder identity ([`folder_key`]); `None` without a choice.
@@ -384,7 +404,7 @@ impl Filter {
             return false;
         }
         if let Some(rs) = &self.rule_set
-            && !rs.matches(p, cat)
+            && !prepared.rules.as_ref().unwrap_or(rs).matches(p, cat)
         {
             return false;
         }
@@ -484,9 +504,9 @@ impl Catalog {
     /// album's rules, the filter bar), as long as they are still where the view looks (`within`: in the library,
     /// the album, the folder; not deleted).
     pub fn query_keeping(&self, filter: &Filter, sort: &Sort, keep: &std::collections::HashSet<PhotoId>, within: &Filter) -> Vec<PhotoId> {
-        let prepared = filter.prepare();
+        let prepared = filter.prepare(self);
         // `within` only decides for the few kept photos: worked out per photo, not hashed
-        let within_prepared = within.prepare_one();
+        let within_prepared = within.prepare_one(self);
         let mut v: Vec<&Photo> = self
             .photos()
             .map(|p| p.as_ref())
@@ -504,7 +524,7 @@ impl Catalog {
     pub fn follow(&self, was: &[PhotoId], touched: &[PhotoId], filter: &Filter, sort: &Sort, within: Option<&Filter>) -> Vec<PhotoId> {
         let listed: std::collections::HashSet<PhotoId> = was.iter().copied().filter(|id| touched.binary_search(id).is_ok()).collect();
         let mut ids: Vec<PhotoId> = was.iter().copied().filter(|id| !listed.contains(id)).collect();
-        let (prepared, within_prepared) = (filter.prepare_one(), within.map(Filter::prepare_one));
+        let (prepared, within_prepared) = (filter.prepare_one(self), within.map(|w| w.prepare_one(self)));
         for id in touched {
             let Some(p) = self.photo(*id) else { continue };
             let kept = within.zip(within_prepared.as_ref()).is_some_and(|(w, wp)| listed.contains(id) && w.matches_in(p, self, wp));
@@ -519,7 +539,7 @@ impl Catalog {
 
     /// How many photos match `filter`: [`Catalog::query`]'s length without its sorting.
     pub fn count(&self, filter: &Filter) -> usize {
-        let prepared = filter.prepare();
+        let prepared = filter.prepare(self);
         self.photos().filter(|p| filter.matches_in(p, self, &prepared)).count()
     }
 
@@ -576,7 +596,7 @@ impl Catalog {
     pub fn people_in(&self, filter: &Filter) -> Vec<Person> {
         let filter = Filter { person: None, ..filter.clone() };
         let mut m: std::collections::HashMap<String, (Person, f64)> = Default::default();
-        let prepared = filter.prepare();
+        let prepared = filter.prepare(self);
         for p in self.photos().filter(|p| filter.matches_in(p, self, &prepared)) {
             let mut seen: Vec<String> = Vec::new();
             for r in p.meta.regions.iter().filter(|r| r.kind == lightcraft_meta::RegionKind::Face) {

crates/catalog/src/rules.rs

@@ -488,6 +488,12 @@ fn pass_answer(id: crate::AlbumId, photo: crate::PhotoId) -> Option<bool> {
     })
 }
 
+#[cfg(test)]
+thread_local! {
+    /// Tests: test rules in the order they were written, as before [`RuleSet::by_cost`].
+    pub(crate) static WRITTEN_ORDER: std::cell::Cell<bool> = const { std::cell::Cell::new(false) };
+}
+
 #[cfg(test)]
 thread_local! {
     /// How many times a smart album's rules were worked out on this thread (tests count them).
@@ -1048,6 +1054,48 @@ impl Rule {
     }
 }
 
+/// How much work one rule is, in tiers: [`RuleSet::by_cost`] tests the cheaper first.
+const COST_SCALAR: u8 = 0;
+const COST_SMALL: u8 = 1;
+const COST_TEXT: u8 = 2;
+const COST_ALBUM: u8 = 3;
+
+impl Rule {
+    /// A rule's tier: 0 numbers, choices and yes/no questions about one photo's own record (and
+    /// anything that is settled at once: an unknown field or a missing album); 1 dates, keyword and
+    /// people lists, short single fields, an index lookup and a plain album's list (a folder holds no photos here: an empty lookup); 2 text built
+    /// from several fields and file paths (long strings, matched for each photo); 3 a smart album,
+    /// which runs a whole rule set for the photo. A group costs as much as its dearest rule.
+    fn cost(&self, cat: &Catalog) -> u8 {
+        match self {
+            Rule::Group { group } => group.rules.iter().map(|r| r.cost(cat)).max().unwrap_or(COST_SCALAR),
+            Rule::Field { field, value, .. } => match field.as_str() {
+                "album" => album_rule_id(value).map_or(COST_SCALAR, |id| match cat.albums.get(&id) {
+                    None => COST_SCALAR,
+                    Some(a) if a.smart.is_some() => COST_ALBUM,
+                    Some(_) => COST_SMALL,
+                }),
+                "text" | "filePath" | "location" => COST_TEXT,
+                "keywords" | "person" | "personCount" | "stacked" | "copyName" | "fileName" | "extension" | "format" | "title" | "caption"
+                | "altText" | "city" | "state" | "country" | "camera" | "lens" | "creator" | "copyright" | "captureDate" | "importDate"
+                | "editDate" | "shutterSpeed" | "aspect" => COST_SMALL,
+                _ => COST_SCALAR,
+            },
+        }
+    }
+
+    /// This rule with its nested groups ordered by cost, and its tier.
+    fn by_cost(&self, cat: &Catalog) -> (u8, Rule) {
+        match self {
+            Rule::Group { group } => {
+                let group = group.by_cost(cat);
+                (group.rules.iter().map(|r| r.cost(cat)).max().unwrap_or(COST_SCALAR), Rule::Group { group })
+            }
+            field => (field.cost(cat), field.clone()),
+        }
+    }
+}
+
 impl RuleSet {
     pub fn matches(&self, p: &Photo, cat: &Catalog) -> bool {
         if self.rules.is_empty() {
@@ -1060,6 +1108,22 @@ impl RuleSet {
         }
     }
 
+    /// This rule set with its rules (and those of nested groups) in the order to test them: the
+    /// cheapest first ([`Rule::cost`]), the user's order among equals. Rules are pure (they read
+    /// the photo and the catalog; a smart album's answer is the same however it is reached as long
+    /// as no smart album tests itself and no chain is cut by the depth limit, which callers check first: [`Catalog::keeps_written_order`]),
+    /// so for all, any and none the photos that match are the same in any order; only the work
+    /// differs. Worked out once for a pass, never per photo. The saved rules are untouched.
+    pub(crate) fn by_cost(&self, cat: &Catalog) -> RuleSet {
+        #[cfg(test)]
+        if WRITTEN_ORDER.get() {
+            return self.clone();
+        }
+        let mut rules: Vec<(u8, Rule)> = self.rules.iter().map(|r| r.by_cost(cat)).collect();
+        rules.sort_by_key(|(cost, _)| *cost);
+        RuleSet { mode: self.mode, rules: rules.into_iter().map(|(_, r)| r).collect() }
+    }
+
     /// Whether matches depend on the clock ("in the last…" rules, nested groups included): the
     /// same photos can enter or leave the set without any catalog change.
     pub fn depends_on_now(&self) -> bool {

crates/catalog/src/tests_repair.rs

@@ -218,3 +218,27 @@ fn an_album_filed_under_another_id_gets_its_own() {
     assert_eq!((a.id, a.parent), (inner, Some(folder)));
     assert_eq!(c.album_children(Some(folder)).iter().map(|a| a.id).collect::<Vec<_>>(), vec![inner]);
 }
+
+/// What was worked out from the albums (whether rules may be tested cheapest first) is worked out
+/// again when a repair changes the albums.
+#[test]
+fn a_repair_forgets_what_was_worked_out_from_the_albums() {
+    let (mut c, _) = sound();
+    let smart = c.alloc_album_id();
+    c.apply(Op::AddAlbum { album: Album { smart: Some(Box::default()), ..album(smart.0, None, false, vec![]) } }).unwrap();
+    assert!(!c.keeps_written_order(), "worked out, and remembered");
+    // damaged outside `apply`: the smart album tests itself, and an album is filed under another id
+    let rules: Filter =
+        serde_json::from_value(serde_json::json!({"ruleSet": {"match": "all", "rules": [{"field": "album", "op": "is", "value": smart.0}]}}))
+            .unwrap();
+    if let Some(a) = c.albums.get_mut(&smart) {
+        a.smart = Some(Box::new(rules));
+    }
+    let inner = c.albums().find(|a| !a.folder && a.smart.is_none()).unwrap().id;
+    if let Some(mut a) = c.albums.remove(&inner) {
+        a.id = AlbumId(900);
+        c.albums.insert(inner, a);
+    }
+    assert!(!c.repair().is_empty());
+    assert!(c.keeps_written_order(), "the loop the repair left is seen");
+}

crates/catalog/src/tests_rule_order.rs

@@ -0,0 +1,365 @@
+//! A rule set is tested cheapest rule first ([`RuleSet::by_cost`]), once per pass; what matches
+//! is what testing the rules in the order they were written gives.
+//!
+//! Scenarios:
+//! - Given random libraries (Unicode and Windows paths, nested groups, album rules that test
+//!   smart albums and missing albums), then every album, query and count gives what the written
+//!   order gives, for all, any and none.
+//! - Given a library where smart albums test themselves (the app refuses to make one; a saved or
+//!   damaged catalog can hold it, and there the answer can depend on which album is asked first),
+//!   then the rules are tested as written, so the answers are the written order's, as before.
+//! - Given `all[album is <smart album>, <a rule no photo meets>]`, then the smart album is never
+//!   asked about a photo: the cheap rule ends it first (written first, it ran for every photo).
+//! - Given rules in any order, then the saved rules and the order the editor shows are untouched.
+
+use serde_json::json;
+
+use crate::rules::{EVALUATED, WRITTEN_ORDER};
+use crate::*;
+
+/// A small generator: the same sequence every run.
+struct Rng(u64);
+impl Rng {
+    fn next(&mut self, n: u64) -> u64 {
+        self.0 = self.0.wrapping_mul(6364136223846793005).wrapping_add(1442695040888963407);
+        (self.0 >> 33) % n
+    }
+    fn pick<'a, T>(&mut self, of: &'a [T]) -> &'a T {
+        &of[self.next(of.len() as u64) as usize]
+    }
+}
+
+const PATHS: &[&str] = &[
+    "/Volumes/share/photos/Asie/Japon/Été/IMG_0001.jpg",
+    "/Volumes/share/photos/ASIE/japon/IMG_0002.JPG",
+    "C:\\Users\\me\\Pictures\\Ünïcode\\Tokyo\\IMG_0003.jpg",
+    "/Volumes/share/photos/Straße/İstanbul/IMG_0004.cr2",
+    "/Volumes/share/photos/東京/渋谷/IMG_0005.jpg",
+    "/Volumes/share/photos/Asie/Chine/clip.mp4",
+    "relative/dir/IMG_0007.jpg",
+];
+
+const WORDS: &[&str] =
+    &["asie", "Japon", "été", "ÜNÏCODE", "tokyo", "straße", "i̇stanbul", "東京", "photos\\asie", "photos/Asie/", " ", "", "img_000"];
+
+fn library(rng: &mut Rng) -> (Catalog, Vec<PhotoId>) {
+    let mut c = Catalog::new();
+    let ids: Vec<PhotoId> = (0..24)
+        .map(|i| {
+            let id = c.alloc_photo_id();
+            let path = if i == 5 { None } else { Some(rng.pick(PATHS).to_string()) };
+            let source = path.map_or(Source::Demo { scene: 1 }, |path| Source::File { path });
+            let name = format!("IMG_{i:04}.{}", ["jpg", "cr2", "mp4"][rng.next(3) as usize]);
+            let mut p = Photo::new(id, source, &name, "JPEG", 6000, 4000, "2026-09-30T10:00:00");
+            p.rating = rng.next(6) as u8;
+            p.flag = [Flag::None, Flag::Pick, Flag::Reject][rng.next(3) as usize];
+            p.label = [None, Some(ColorLabel::Red), Some(ColorLabel::Blue)][rng.next(3) as usize];
+            p.meta.keywords = (0..rng.next(4)).map(|k| format!("kw{}", k + rng.next(2))).collect();
+            p.captured = Some(format!("20{}-0{}-1{}T10:00:00", 10 + rng.next(16), 1 + rng.next(9), rng.next(9)));
+            c.apply(Op::AddPhoto { photo: Box::new(p) }).unwrap();
+            id
+        })
+        .collect();
+    (c, ids)
+}
+
+fn leaf(rng: &mut Rng, albums: &[AlbumId]) -> serde_json::Value {
+    let field = |f: &str, op: &str, v: serde_json::Value| json!({"field": f, "op": op, "value": v});
+    match rng.next(14) {
+        0 => field("rating", ["gte", "lte", "is", "gt", "lt"][rng.next(5) as usize], json!(rng.next(6))),
+        1 => field("flag", ["is", "isNot"][rng.next(2) as usize], json!(["pick", "reject", "none"][rng.next(3) as usize])),
+        2 => field("label", ["is", "isNot"][rng.next(2) as usize], json!(["red", "blue", "none"][rng.next(3) as usize])),
+        3 => field("kind", ["is", "isNot"][rng.next(2) as usize], json!(["image", "raw", "video"][rng.next(3) as usize])),
+        4 => field("keywordCount", ["lte", "gte"][rng.next(2) as usize], json!(rng.next(4))),
+        5 => field("keywords", ["contains", "notContains", "isEmpty"][rng.next(3) as usize], json!(format!("kw{}", rng.next(3)))),
+        6 => field("captureDate", ["before", "after"][rng.next(2) as usize], json!(format!("20{}", 10 + rng.next(16)))),
+        7 | 8 => field("filePath", ["contains", "notContains", "startsWith", "is", "isEmpty"][rng.next(5) as usize], json!(*rng.pick(WORDS))),
+        9 => field("fileName", "contains", json!(["img_000", ".cr2", "mp4"][rng.next(3) as usize])),
+        10 => field("text", ["contains", "notContains"][rng.next(2) as usize], json!(*rng.pick(WORDS))),
+        // an album that may not exist (99) or may be on a loop
+        _ => {
+            let album = if rng.next(8) == 0 { AlbumId(99) } else { *rng.pick(albums) };
+            field("album", ["is", "isNot"][rng.next(2) as usize], json!(album.0))
+        }
+    }
+}
+
+fn rule_set(rng: &mut Rng, albums: &[AlbumId], depth: u32) -> serde_json::Value {
+    let rules: Vec<serde_json::Value> = (0..rng.next(5))
+        .map(|_| if depth < 2 && rng.next(4) == 0 { json!({"group": rule_set(rng, albums, depth + 1)}) } else { leaf(rng, albums) })
+        .collect();
+    let mode = ["all", "any", "none"][rng.next(3) as usize];
+    json!({"match": mode, "rules": rules})
+}
+
+fn filter(rules: serde_json::Value) -> Filter {
+    Filter { rule_set: Some(serde_json::from_value(rules).unwrap()), ..Default::default() }
+}
+
+/// What a catalog answers for every album, photo and a few queries.
+fn answers(c: &Catalog, albums: &[AlbumId], photos: &[PhotoId], queries: &[Filter]) -> String {
+    let mut out = String::new();
+    for a in albums {
+        let held: Vec<bool> = photos.iter().map(|p| c.album_contains(*a, c.photo(*p).unwrap())).collect();
+        out += &format!("{a:?} {held:?} {:?} {}\n", c.album_photos(*a), c.album_count(*a));
+    }
+    for p in photos {
+        out += &format!("{p:?} {:?}\n", c.albums_of(*p));
+    }
+    for q in queries {
+        out += &format!("{:?} {}\n", c.query(q, &Sort::default()), c.count(q));
+    }
+    out
+}
+
+#[test]
+fn the_cheapest_first_order_matches_what_the_written_order_matches() {
+    let chained = same_answers_in_random_libraries(Rng(11), false);
+    assert!(chained > 20, "the generator makes smart albums that test smart albums: {chained}");
+}
+
+#[test]
+fn a_library_with_loops_answers_as_written_and_does_not_hang() {
+    let looping = same_answers_in_random_libraries(Rng(5), true);
+    assert!(looping > 20, "the generator makes loops: {looping}");
+}
+
+/// Albums of random libraries and queries answer the same cheapest first and as written; returns
+/// how many smart albums tested a smart album (`loops`: one that leads back to itself).
+fn same_answers_in_random_libraries(mut rng: Rng, loops: bool) -> usize {
+    let mut chained = 0;
+    for round in 0..150 {
+        let (mut c, photos) = library(&mut rng);
+        let plain = c.alloc_album_id();
+        c.apply(Op::AddAlbum { album: Album { photos: photos.iter().copied().step_by(3).collect(), ..Album::new(plain, "Plain") } }).unwrap();
+        let folder = c.alloc_album_id();
+        c.apply(Op::AddAlbum { album: Album { folder: true, ..Album::new(folder, "F") } }).unwrap();
+        let smarts: Vec<AlbumId> = (0..6)
+            .map(|i| {
+                let id = c.alloc_album_id();
+                let parent = (i % 3 == 0).then_some(folder);
+                c.apply(Op::AddAlbum { album: Album { parent, smart: Some(Box::default()), ..Album::new(id, format!("S{i}")) } }).unwrap();
+                id
+            })
+            .collect();
+        let albums: Vec<AlbumId> = [plain, folder].into_iter().chain(smarts.iter().copied()).collect();
+        for (i, s) in smarts.iter().enumerate() {
+            // (without loops a smart album tests only the albums made before it)
+            let testable = if loops { &albums[..] } else { &albums[..2 + i] };
+            let _ = c.apply(Op::SetAlbumRules { id: *s, rules: Box::new(filter(rule_set(&mut rng, testable, 0))) });
+        }
+        let queries: Vec<Filter> = (0..4).map(|_| filter(rule_set(&mut rng, &albums, 0))).collect();
+        // (the copy starts without what the catalog worked out, so each is worked out under its own order)
+        let written = c.clone();
+        let ordered_answers = answers(&c, &albums, &photos, &queries);
+        WRITTEN_ORDER.set(true);
+        let written_answers = answers(&written, &albums, &photos, &queries);
+        WRITTEN_ORDER.set(false);
+        assert_eq!(ordered_answers, written_answers, "round {round}: {}", c.to_snapshot());
+        let reaching = |s: &AlbumId| {
+            smarts.iter().any(|t| if loops { c.album_reaches(*t, *s) && c.album_reaches(*s, *t) } else { t != s && c.album_reaches(*s, *t) })
+        };
+        chained += smarts.iter().filter(|s| reaching(s)).count();
+        assert_eq!(c.keeps_written_order(), smarts.iter().any(|s| c.album_reaches(*s, *s)));
+        assert!(loops || !c.keeps_written_order(), "round {round}: {}", c.to_snapshot());
+    }
+    chained
+}
+
+#[test]
+fn a_cheap_rule_that_fails_ends_the_test_before_a_smart_album_runs() {
+    let mut c = Catalog::new();
+    let photos: Vec<PhotoId> = (0..20)
+        .map(|i| {
+            let id = c.alloc_photo_id();
+            let p = Photo::new(id, Source::File { path: format!("/a/b/{i}.jpg") }, &format!("{i}.jpg"), "JPEG", 6000, 4000, "2026-09-30T10:00:00");
+            c.apply(Op::AddPhoto { photo: Box::new(p) }).unwrap();
+            id
+        })
+        .collect();
+    let inner = c.alloc_album_id();
+    c.apply(Op::AddAlbum {
+        album: Album {
+            smart: Some(Box::new(filter(json!({"rules": [{"field": "filePath", "op": "contains", "value": "/a/"}]})))),
+            ..Album::new(inner, "Inner")
+        },
+    })
+    .unwrap();
+    let rules = |first: serde_json::Value, second: serde_json::Value| json!({"match": "all", "rules": [first, second]});
+    let album = json!({"field": "album", "op": "is", "value": inner.0});
+    // no photo is rated 6 stars (a rating is at most 5)
+    let never = json!({"field": "rating", "op": "gte", "value": 6});
+    let outer = c.alloc_album_id();
+    c.apply(Op::AddAlbum { album: Album { smart: Some(Box::new(filter(rules(album.clone(), never.clone())))), ..Album::new(outer, "Outer") } })
+        .unwrap();
+    EVALUATED.set(0);
+    assert_eq!(c.album_count(outer), 0);
+    // (the outer album's own rules count as one run for each photo; its inner album must run for none)
+    assert_eq!(EVALUATED.get(), photos.len(), "only the outer album ran, once per photo");
+    // the same photos whichever way round the rules are written
+    let swapped = c.alloc_album_id();
+    c.apply(Op::AddAlbum { album: Album { smart: Some(Box::new(filter(rules(never, album)))), ..Album::new(swapped, "Swapped") } }).unwrap();
+    assert_eq!((c.album_count(swapped), c.album_photos(swapped)), (0, vec![]));
+}
+
+#[test]
+fn rules_are_ordered_by_cost_without_touching_the_saved_ones() {
+    let mut c = Catalog::new();
+    let plain = c.alloc_album_id();
+    c.apply(Op::AddAlbum { album: Album::new(plain, "Plain") }).unwrap();
+    let smart = c.alloc_album_id();
+    c.apply(Op::AddAlbum { album: Album { smart: Some(Box::default()), ..Album::new(smart, "Smart") } }).unwrap();
+    let written: RuleSet = serde_json::from_value(json!({"match": "all", "rules": [
+        {"field": "album", "op": "is", "value": smart.0},
+        {"field": "filePath", "op": "contains", "value": "x"},
+        {"group": {"match": "any", "rules": [
+            {"field": "album", "op": "isNot", "value": smart.0},
+            {"field": "rating", "op": "gte", "value": 3}]}},
+        {"field": "keywords", "op": "contains", "value": "k"},
+        {"field": "album", "op": "is", "value": plain.0},
+        {"field": "rating", "op": "gte", "value": 3},
+        {"field": "album", "op": "is", "value": 99},
+        {"field": "flag", "op": "is", "value": "pick"}]}))
+    .unwrap();
+    let before = written.clone();
+    let ordered = written.by_cost(&c);
+    assert_eq!(written, before, "the saved rules stay as the user wrote them");
+    let names: Vec<String> = ordered
+        .rules
+        .iter()
+        .map(|r| match r {
+            Rule::Group { .. } => "group".to_string(),
+            Rule::Field { field, op, .. } => format!("{field} {op}"),
+        })
+        .collect();
+    // scalars (a missing album is false at once), then lists and a plain album, then text, then
+    // smart albums and the group that holds one; equals keep the written order
+    assert_eq!(names, ["rating gte", "album is", "flag is", "keywords contains", "album is", "filePath contains", "album is", "group"]);
+    let inner: Vec<&str> = match ordered.rules.get(7) {
+        Some(Rule::Group { group }) => group.rules.iter().map(|r| if let Rule::Field { field, .. } = r { field.as_str() } else { "group" }).collect(),
+        _ => Vec::new(),
+    };
+    assert_eq!(inner, ["rating", "album"], "nested groups are ordered too");
+}
+
+/// B = all[album is C], C = all[album isNot B], A = all[any[album is C, flag is pick], album is B]:
+/// asked in the order the rules are written, a picked photo is not in A; asked cheapest first
+/// (the flag, then the albums) it was. With a loop the order stays as written.
+#[test]
+fn smart_albums_that_test_each_other_keep_the_written_order() {
+    let mut c = Catalog::new();
+    let mut p = Photo::new(c.alloc_photo_id(), Source::File { path: "/a/1.jpg".into() }, "1.jpg", "JPEG", 6000, 4000, "2026-09-30T10:00:00");
+    p.flag = Flag::Pick;
+    let photo = p.id;
+    c.apply(Op::AddPhoto { photo: Box::new(p) }).unwrap();
+    let ids: Vec<AlbumId> = ["A", "B", "C"]
+        .iter()
+        .map(|name| {
+            let id = c.alloc_album_id();
+            c.apply(Op::AddAlbum { album: Album { smart: Some(Box::default()), ..Album::new(id, *name) } }).unwrap();
+            id
+        })
+        .collect();
+    let (a, b, cc) = (ids[0], ids[1], ids[2]);
+    let album = |op: &str, id: AlbumId| json!({"field": "album", "op": op, "value": id.0});
+    let set = |c: &mut Catalog, id: AlbumId, rules: serde_json::Value| c.apply(Op::SetAlbumRules { id, rules: Box::new(filter(rules)) }).unwrap();
+    set(&mut c, b, json!({"match": "all", "rules": [album("is", cc)]}));
+    set(&mut c, cc, json!({"match": "all", "rules": [album("isNot", b)]}));
+    set(
+        &mut c,
+        a,
+        json!({"match": "all", "rules": [
+            {"group": {"match": "any", "rules": [album("is", cc), {"field": "flag", "op": "is", "value": "pick"}]}},
+            album("is", b)]}),
+    );
+    let written = c.clone();
+    let ask = |c: &Catalog| {
+        ids.iter().map(|id| (c.album_contains(*id, c.photo(photo).unwrap()), c.album_photos(*id), c.album_count(*id))).collect::<Vec<_>>()
+    };
+    WRITTEN_ORDER.set(true);
+    let as_written = ask(&written);
+    WRITTEN_ORDER.set(false);
+    assert!(c.keeps_written_order());
+    assert_eq!(ask(&c), as_written);
+}
+
+/// A = all[any[album is D1, flag is pick], album is W], D1 tests D2 … D30, D30 tests W, W tests Z,
+/// Z = rating ≥ 0: asked from A, the chain is cut by the depth limit, and asked cheapest first (the
+/// flag, then the albums) the cut falls elsewhere. Chains that long keep the written order.
+#[test]
+fn a_chain_as_deep_as_the_limit_keeps_the_written_order() {
+    let mut c = Catalog::new();
+    let mut p = Photo::new(c.alloc_photo_id(), Source::File { path: "/a/1.jpg".into() }, "1.jpg", "JPEG", 6000, 4000, "2026-09-30T10:00:00");
+    p.flag = Flag::Pick;
+    p.rating = 0; // (Z, rating at least 0, holds for it)
+    let photo = p.id;
+    c.apply(Op::AddPhoto { photo: Box::new(p) }).unwrap();
+    let new_smart = |c: &mut Catalog, name: &str| {
+        let id = c.alloc_album_id();
+        c.apply(Op::AddAlbum { album: Album { smart: Some(Box::default()), ..Album::new(id, name) } }).unwrap();
+        id
+    };
+    let a = new_smart(&mut c, "A");
+    let chain: Vec<AlbumId> = (1..=30).map(|i| new_smart(&mut c, &format!("D{i}"))).collect();
+    let (w, z) = (new_smart(&mut c, "W"), new_smart(&mut c, "Z"));
+    let album = |id: AlbumId| json!({"field": "album", "op": "is", "value": id.0});
+    let set = |c: &mut Catalog, id: AlbumId, rules: serde_json::Value| c.apply(Op::SetAlbumRules { id, rules: Box::new(filter(rules)) }).unwrap();
+    set(
+        &mut c,
+        a,
+        json!({"match": "all", "rules": [{"group": {"match": "any", "rules": [album(chain[0]), {"field": "flag", "op": "is", "value": "pick"}]}}, album(w)]}),
+    );
+    for pair in chain.windows(2) {
+        set(&mut c, pair[0], json!({"match": "all", "rules": [album(pair[1])]}));
+    }
+    set(&mut c, chain[29], json!({"match": "all", "rules": [album(w)]}));
+    set(&mut c, w, json!({"match": "all", "rules": [album(z)]}));
+    set(&mut c, z, json!({"match": "all", "rules": [{"field": "rating", "op": "gte", "value": 0}]}));
+    let written = c.clone();
+    let ask = |c: &Catalog| [a, w, z, chain[0]].map(|id| (c.album_contains(id, c.photo(photo).unwrap()), c.album_photos(id), c.album_count(id)));
+    WRITTEN_ORDER.set(true);
+    let as_written = ask(&written);
+    // (the expected answers, written down: the same evaluator gives both sides below, so a
+    // regression that lets the 33rd album in would otherwise change both alike)
+    assert_eq!(as_written.each_ref().map(|answer| answer.0), [false, true, true, true], "the depth limit cuts the chain only while A is asked");
+    WRITTEN_ORDER.set(false);
+    assert!(c.keeps_written_order());
+    assert_eq!(ask(&c), as_written);
+}
+
+/// What was worked out from the albums (whether rules may be tested cheapest first) is worked out
+/// again after an album is added or removed: 31 smart albums in a chain are fine, a 32nd on top of
+/// it reaches the depth limit (the written order from then on), and taking it away lifts that.
+#[test]
+fn adding_or_removing_an_album_forgets_whether_the_rules_may_be_reordered() {
+    let mut c = Catalog::new();
+    let mut p = Photo::new(c.alloc_photo_id(), Source::File { path: "/a/1.jpg".into() }, "1.jpg", "JPEG", 6000, 4000, "2026-09-30T10:00:00");
+    p.flag = Flag::Pick;
+    let photo = p.id;
+    c.apply(Op::AddPhoto { photo: Box::new(p) }).unwrap();
+    let album = |id: AlbumId| json!({"field": "album", "op": "is", "value": id.0});
+    let smart = |c: &mut Catalog, name: &str, rules: serde_json::Value| {
+        let id = c.alloc_album_id();
+        c.apply(Op::AddAlbum { album: Album { smart: Some(Box::new(filter(rules))), ..Album::new(id, name) } }).unwrap();
+        id
+    };
+    // S1 tests S2 … S30 tests S31, which tests nothing: 31 albums
+    let mut chain = vec![smart(&mut c, "S31", json!({"match": "all", "rules": []}))];
+    for i in (1..=30).rev() {
+        let next = chain[chain.len() - 1];
+        chain.push(smart(&mut c, &format!("S{i}"), json!({"match": "all", "rules": [album(next)]})));
+    }
+    let bottom = chain[0];
+    let top = chain[30];
+    let ask = |c: &Catalog| [top, bottom].map(|id| (c.album_contains(id, c.photo(photo).unwrap()), c.album_photos(id), c.album_count(id)));
+    assert!(!c.keeps_written_order(), "31 albums in a chain are under the limit (worked out, and remembered)");
+    let before = ask(&c);
+    let head = smart(&mut c, "S0", json!({"match": "all", "rules": [album(top)]}));
+    assert!(c.keeps_written_order(), "a 32nd album on top of the chain reaches the depth limit");
+    assert_eq!(c.clone().keeps_written_order(), c.keeps_written_order(), "as a catalog that worked it out afresh");
+    assert_eq!(ask(&c), ask(&c.clone()));
+    c.apply(Op::RemoveAlbum { id: head }).unwrap();
+    assert!(!c.keeps_written_order(), "without it the chain is under the limit again");
+    assert_eq!(c.clone().keeps_written_order(), c.keeps_written_order());
+    assert_eq!(ask(&c), before);
+}

docs/whats-new.md

@@ -174,6 +174,14 @@
   it also shows on a white one) and the other selected photos a lighter one (1.5 pt), in colours each theme picks to
   stand out from its grid. The filmstrip rings its photos the same way, and so do Compare, Reference and Survey (where
   the ring is the only sign of the active photo) and the People view's chosen person.
+### Smart albums open and count faster on large libraries
+- A smart album that tests another smart album and also a file path or other text (for example "in Rejects, and the
+  path contains …") now checks the quick rules first and the slow ones last, so most photos are ruled out before the
+  other album is asked about them. The photos in the album and their order are the same, and the rules still show in
+  the order you wrote them. (A library whose smart albums test each other in a circle, which LightCraft doesn't let
+  you make, or test each other 32 deep, keeps testing the rules as written.) On a 312,000-photo test library, opening
+  51 such albums in turn went from about 19 s to about 5.8 s (about 377 ms to 113 ms each; a busy machine,
+  so rough numbers).
 
 ### Sharper resampling in HDR and panorama merges
 - Merged photos (HDR and panorama) are resampled with a sharper filter (Lanczos-3 instead of bilinear). Moving a