LRC-LIB-SMARTCOLL: smart albums can test other smart albums (loops refused and survived)

An Album rule may now name a smart album: "Keywords contain travel"
and "Album isn't Excluded Photos" (red or rejected) leaves out whatever
Excluded Photos matches, as its rules change. Smart albums used to be
refused for this.

Loops:
- RuleSet::check_for(cat, owner) refuses an Album rule that would make
  the owner include itself, directly or through other smart albums
  (Issue::AlbumLoop). Catalog::album_reaches walks the references with
  a seen-set; RuleSet::albums_tested lists them. album.setRules checks
  for the album being edited, and smart_album_problems reports a loop
  saved anyway.
- Matching never recurses without end: album_contains evaluates a smart
  album's rules through rules::evaluating, a thread-local stack of
  albums being evaluated with a 32-deep cap. An album already on it (a
  saved loop) holds nothing, and the stack is restored by a drop guard
  however evaluation ends. A test builds A → B → C → A and checks that
  every album returns and reports the loop.

The issue "can't test a smart album" is replaced by the loop issue in
all seven locales. The sidebar counts already refresh while any smart
album uses "in the last…", so an album testing one is covered.

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

crates/engine/src/cmd/library.rs

@@ -816,7 +816,7 @@ pub fn specs() -> Vec<CommandSpec> {
                     return Err(bad("album.createSmart", "empty name"));
                 }
                 let rules = match p.get("rules") {
-                    Some(r) => merge_rules(&lightcraft_catalog::Filter::default(), r, "album.createSmart", &s.catalog)?,
+                    Some(r) => merge_rules(&lightcraft_catalog::Filter::default(), r, "album.createSmart", &s.catalog, None)?,
                     None => view_rules(s),
                 };
                 let parent = p.get("parent").and_then(Value::as_u64).map(AlbumId);
@@ -865,7 +865,7 @@ pub fn specs() -> Vec<CommandSpec> {
                     let replace = bool_or(p, "replace", false);
                     let folder = cur.library_folder.clone();
                     let base = if replace { Default::default() } else { cur };
-                    let mut r = merge_rules(&base, p.get("rules").unwrap_or(&Value::Null), "album.setRules", &s.catalog)?;
+                    let mut r = merge_rules(&base, p.get("rules").unwrap_or(&Value::Null), "album.setRules", &s.catalog, Some(id))?;
                     // the rules dialog has no folder field: replacing its rules keeps the folder
                     // unless the call says (`libraryFolder: null`) to drop it
                     if replace && p.get("rules").and_then(|r| r.get("libraryFolder")).is_none() {
@@ -1300,10 +1300,18 @@ fn merge_filter(base: &lightcraft_catalog::Filter, patch: &Value, c: &str) -> Re
     Ok(f)
 }
 
-/// `patch` merged onto `base`, refused when its rule set has problems (`RuleSet::check`).
-fn merge_rules(base: &lightcraft_catalog::Filter, patch: &Value, c: &str, cat: &lightcraft_catalog::Catalog) -> Result<lightcraft_catalog::Filter> {
+/// `patch` merged onto `base`, refused when its rule set has problems (`RuleSet::check_for`;
+/// `owner` is the smart album the rules are for, so testing an album that leads back to it is a
+/// problem too).
+fn merge_rules(
+    base: &lightcraft_catalog::Filter,
+    patch: &Value,
+    c: &str,
+    cat: &lightcraft_catalog::Catalog,
+    owner: Option<AlbumId>,
+) -> Result<lightcraft_catalog::Filter> {
     let f = merge_filter(base, patch, c)?;
-    let problems = f.rule_set.as_ref().map(|r| r.check(cat)).unwrap_or_default();
+    let problems = f.rule_set.as_ref().map(|r| r.check_for(cat, owner)).unwrap_or_default();
     if !problems.is_empty() {
         return Err(bad(c, problems.iter().map(ToString::to_string).collect::<Vec<_>>().join("; ")));
     }

crates/engine/src/tests_library.rs

@@ -571,3 +571,49 @@ fn album_list_reports_problems() {
     s.execute("album.delete", &serde_json::json!({"id": trip})).unwrap();
     assert_eq!(find(&mut s)["problems"], serde_json::json!([format!("rule 1: no album {trip}")]));
 }
+
+/// "Travel" = keywords contain travel, but not in "Excluded Photos" (red or rejected): the counts
+/// follow, and Excluded Photos can't then test Travel back (a loop).
+#[test]
+fn smart_album_excluding_a_smart_album() {
+    let mut s = crate::Session::with_demo();
+    let excluded = s
+        .execute(
+            "album.createSmart",
+            &serde_json::json!({"name": "Excluded Photos", "rules": {"ruleSet": {"match": "any", "rules": [
+                {"field": "label", "op": "is", "value": "red"}, {"field": "flag", "op": "is", "value": "reject"}]}}}),
+        )
+        .unwrap()["id"]
+        .as_u64()
+        .unwrap();
+    let keyword = s.catalog.photos().flat_map(|p| p.meta.keywords.clone()).next().expect("a demo keyword");
+    let travel = s
+        .execute(
+            "album.createSmart",
+            &serde_json::json!({"name": "Travel", "rules": {"ruleSet": {"rules": [
+                {"field": "keywords", "op": "contains", "value": keyword}, {"field": "album", "op": "isNot", "value": excluded}]}}}),
+        )
+        .unwrap()["id"]
+        .as_u64()
+        .unwrap();
+    let count = |s: &crate::Session| s.catalog.album_count(lightcraft_catalog::AlbumId(travel));
+    let want = |s: &crate::Session| {
+        s.catalog
+            .photos()
+            .filter(|p| !p.deleted && p.meta.keywords.iter().any(|k| k.to_lowercase().contains(&keyword.to_lowercase())))
+            .filter(|p| p.label != Some(lightcraft_catalog::ColorLabel::Red) && p.flag != lightcraft_catalog::Flag::Reject)
+            .count()
+    };
+    assert_eq!(count(&s), want(&s));
+    // reject one of Travel's photos: it leaves Travel
+    let before = count(&s);
+    let id = s.catalog.photos().find(|p| s.catalog.album_contains(lightcraft_catalog::AlbumId(travel), p)).map(|p| p.id).expect("a travel photo");
+    s.execute("photo.flag", &serde_json::json!({"ids": [id.0], "flag": "reject"})).unwrap();
+    assert_eq!(count(&s), before - 1);
+    // Excluded Photos testing Travel back would include itself
+    let looped = s.execute(
+        "album.setRules",
+        &serde_json::json!({"id": excluded, "rules": {"ruleSet": {"rules": [{"field": "album", "op": "is", "value": travel}]}}, "replace": true}),
+    );
+    assert!(looped.is_err_and(|e| e.to_string().contains("would make this album include itself")), "a loop is refused");
+}