LRC-LIB-SMARTCOLL: rule editor marks bad rules, blocks OK and picks albums from a list

The dialog showed only the first problem, in place of the match count,
without saying which rule it was about. OK stayed clickable, and an
edit renamed the album before its rules were refused.

- Each rule with a problem gets the message right under its row, in the
  caution colour; inside groups too, and on an empty group's frame. The
  rules_editor::Env carries the problems from RuleSet::check, and each
  row finds its own by path. Markers register `ruleProblem:<salt>`.
- The summary reads "Fix the marked rules to save this album." and OK is
  disabled while any problem remains.
- confirm_dialog refuses problems before anything changes (not even the
  name), and the control channel's ui.dialog.confirm keeps the editor
  open on that error, as the button does.
- Album is its own field kind: is / isn't only (it was a number field
  that offered ≥ and between). It picks its album from a list of the
  plain albums (`ruleAlbum:` / `ruleAlbumItem:<id>:`) and starts with
  none chosen ("choose an album"). album.ruleFields reports kind "album".
- Found while testing: the number editor's spinner clamped a stored
  value outside its range as soon as it was drawn, so "Rating is 9"
  quietly became "Rating is 5". It now uses
  clamp_existing_to_range(false): the range limits dragging, and a
  stored value stays as it is, marked by the check.

New labels (Choose an album…, No albums yet, Fix the marked rules…) are
translated in all seven locales.

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

crates/catalog/src/rules.rs

@@ -48,6 +48,8 @@ pub enum Kind {
     Keywords,
     Number,
     Date,
+    /// A plain album, by id: in it or not.
+    Album,
     /// One of a fixed set: (id, label). Rules store the id; people see the label.
     Choice(&'static [(&'static str, &'static str)]),
     Bool,
@@ -65,7 +67,7 @@ pub const FIELDS: &[(&str, &str, Kind)] = &[
     ),
     ("text", "Any Searchable Text", Kind::Text),
     // Source
-    ("album", "Album", Kind::Number),
+    ("album", "Album", Kind::Album),
     ("virtualCopy", "Virtual Copy", Kind::Bool),
     ("copyName", "Copy Name", Kind::Text),
     ("stacked", "In a Stack", Kind::Bool),
@@ -173,11 +175,12 @@ fn field_problem(field: &str, op: &str, value: &Value, cat: &Catalog) -> Option<
     }
     match kind {
         Kind::Bool => bool_value(value).is_none().then(|| format!("`{field}` is yes or no, not {value}")),
+        Kind::Album => album_problem(value, cat),
         Kind::Number => {
             let each = match value {
-                Value::Array(a) if op == "between" && a.len() == 2 => a.iter().find_map(|v| number_problem(field, v, cat)),
+                Value::Array(a) if op == "between" && a.len() == 2 => a.iter().find_map(|v| number_problem(field, v)),
                 _ if op == "between" => Some(format!("`{field}` between needs two values, not {value}")),
-                v => number_problem(field, v, cat),
+                v => number_problem(field, v),
             };
             // a rating rule no rating could meet ("is 9", "≥ 7"); "< 6" is fine, if broad
             each.or_else(|| {
@@ -199,21 +202,23 @@ fn field_problem(field: &str, op: &str, value: &Value, cat: &Catalog) -> Option<
     }
 }
 
-fn number_problem(field: &str, v: &Value, cat: &Catalog) -> Option<String> {
+fn number_problem(field: &str, v: &Value) -> Option<String> {
     if field == "shutterSpeed" {
         return shutter_value(v).is_null().then(|| format!("`shutterSpeed` needs a time like 1/250 or 2, not {v}"));
     }
-    let Some(n) = number(v).filter(|n| n.is_finite()) else { return Some(format!("`{field}` needs a number, not {v}")) };
-    match field {
-        "album" => {
-            let album = (n >= 0.0 && n.fract() == 0.0).then(|| cat.album(crate::AlbumId(n as u64))).flatten();
-            match album {
-                None => Some(format!("no album {v}")),
-                Some(a) if a.is_smart() => Some(format!("album {v} is a smart album; only plain albums can be tested")),
-                Some(_) => None,
-            }
-        }
-        _ => None,
+    number(v).filter(|n| n.is_finite()).is_none().then(|| format!("`{field}` needs a number, not {v}"))
+}
+
+/// An Album rule's value: an album that exists and holds photos itself (not a smart album).
+fn album_problem(v: &Value, cat: &Catalog) -> Option<String> {
+    if v.is_null() {
+        return Some("choose an album".into());
+    }
+    let id = number(v).filter(|n| n.is_finite() && *n >= 0.0 && n.fract() == 0.0);
+    match id.and_then(|n| cat.album(crate::AlbumId(n as u64))) {
+        None => Some(format!("no album {v}")),
+        Some(a) if a.is_smart() => Some(format!("album {v} is a smart album; only plain albums can be tested")),
+        Some(_) => None,
     }
 }
 
@@ -342,7 +347,7 @@ pub fn ops_for(kind: Kind) -> &'static [(&'static str, &'static str)] {
             ("notInLast", "isn't in the last"),
             ("isEmpty", "is unknown"),
         ],
-        Kind::Choice(_) => &[("is", "is"), ("isNot", "isn't")],
+        Kind::Choice(_) | Kind::Album => &[("is", "is"), ("isNot", "isn't")],
         Kind::Bool => &[("is", "is")],
     }
 }
@@ -1235,6 +1240,16 @@ mod tests {
         set_now(None);
     }
 
+    /// An album is in it or not: Album offers "is" and "isn't", never "≥" or "between".
+    #[test]
+    fn album_rules_are_is_or_isnt() {
+        assert_eq!(field_kind("album"), Some(Kind::Album));
+        let ops: Vec<&str> = ops_for(Kind::Album).iter().map(|o| o.0).collect();
+        assert_eq!(ops, ["is", "isNot"]);
+        let r = rs(json!({"rules": [{"field": "album", "op": "gte", "value": 1}]}));
+        assert_eq!(r.check(&Catalog::new()).first().map(|p| p.message.as_str()), Some("`album` has no operator `gte`"));
+    }
+
     /// The field menu shows every rule field once: at the top level or in exactly one group,
     /// and [`FIELDS`] lists them in menu order.
     #[test]

crates/engine/src/cmd/library.rs

@@ -823,7 +823,7 @@ pub fn specs() -> Vec<CommandSpec> {
                 Ok(json!({"id": id.0, "count": s.catalog.album_count(id)}))
             }
         ),
-        cmd!(query "album.ruleFields", "Smart Album Rule Fields", [], None, "{} → [{field, label, kind, group (field-menu submenu, null at the top level), ops: [{op, label}], choices? (ids), choiceLabels? ({id: label})}] for ruleSet rules, in menu order", always, |_, _| {
+        cmd!(query "album.ruleFields", "Smart Album Rule Fields", [], None, "{} → [{field, label, kind (text|keywords|number|date|choice|bool|album: an album id), group (field-menu submenu, null at the top level), ops: [{op, label}], choices? (ids), choiceLabels? ({id: label})}] for ruleSet rules, in menu order", always, |_, _| {
             use lightcraft_catalog::rules::{FIELDS, Kind, field_group, ops_for};
             Ok(json!(FIELDS
                 .iter()
@@ -832,6 +832,7 @@ pub fn specs() -> Vec<CommandSpec> {
                         Kind::Text => "text",
                         Kind::Keywords => "keywords",
                         Kind::Number => "number",
+                        Kind::Album => "album",
                         Kind::Date => "date",
                         Kind::Choice(_) => "choice",
                         Kind::Bool => "bool",

crates/engine/src/tests_library.rs

@@ -260,6 +260,8 @@ fn smart_album_rule_sets() {
     let flag = fields.as_array().unwrap().iter().find(|f| f["field"] == "copyrightStatus").unwrap();
     assert_eq!(flag["choices"], serde_json::json!(["copyrighted", "publicDomain", "unknown"]));
     assert_eq!(flag["choiceLabels"]["publicDomain"], "Public Domain");
+    let album = fields.as_array().unwrap().iter().find(|f| f["field"] == "album").unwrap();
+    assert_eq!((album["kind"].as_str(), album["ops"].as_array().map(Vec::len)), (Some("album"), Some(2)), "an album id, is / isn't");
     let bad = s.execute(
         "album.createSmart",
         &serde_json::json!({"name": "Bad", "rules": {"ruleSet": {"rules": [{"field": "rating", "op": "contains", "value": 1}]}}}),

crates/ui-egui/locales/de.json

@@ -1896,5 +1896,8 @@
   "Portrait (tall)": "Hochformat",
   "Square": "Quadratisch",
   "In Color": "Farbig",
-  "No Label": "Ohne Markierung"
+  "No Label": "Ohne Markierung",
+  "Choose an album…": "Album wählen…",
+  "No albums yet": "Noch keine Alben",
+  "Fix the marked rules to save this album.": "Korrigiere die markierten Regeln, um dieses Album zu speichern."
 }

crates/ui-egui/locales/es.json

@@ -1741,5 +1741,8 @@
   "Portrait (tall)": "Vertical (retrato)",
   "Square": "Cuadrada",
   "In Color": "En color",
-  "No Label": "Sin etiqueta"
+  "No Label": "Sin etiqueta",
+  "Choose an album…": "Elige un álbum…",
+  "No albums yet": "Aún no hay álbumes",
+  "Fix the marked rules to save this album.": "Corrige las reglas marcadas para guardar este álbum."
 }

crates/ui-egui/locales/ja.json

@@ -1745,5 +1745,8 @@
   "smart album value|Yes": "あり",
   "smart album value|No": "なし",
   "In a Stack": "スタック",
-  "Cropped": "切り抜き"
+  "Cropped": "切り抜き",
+  "Choose an album…": "アルバムを選択…",
+  "No albums yet": "アルバムはまだありません",
+  "Fix the marked rules to save this album.": "このアルバムを保存するには、印の付いた条件を修正してください。"
 }

crates/ui-egui/locales/pt-br.json

@@ -1741,5 +1741,8 @@
   "Portrait (tall)": "Retrato (vertical)",
   "Square": "Quadrada",
   "In Color": "Colorida",
-  "No Label": "Sem rótulo"
+  "No Label": "Sem rótulo",
+  "Choose an album…": "Escolha um álbum…",
+  "No albums yet": "Ainda não há álbuns",
+  "Fix the marked rules to save this album.": "Corrija as regras marcadas para salvar este álbum."
 }

crates/ui-egui/locales/ru.json

@@ -1413,5 +1413,8 @@
   "Portrait (tall)": "Книжная",
   "Square": "Квадратная",
   "In Color": "Цветное",
-  "No Label": "Без метки"
+  "No Label": "Без метки",
+  "Choose an album…": "Выберите альбом…",
+  "No albums yet": "Альбомов пока нет",
+  "Fix the marked rules to save this album.": "Исправьте отмеченные условия, чтобы сохранить альбом."
 }

crates/ui-egui/locales/zh-hans.json

@@ -1741,5 +1741,8 @@
   "Portrait (tall)": "纵向",
   "Square": "正方形",
   "In Color": "彩色",
-  "No Label": "无标签"
+  "No Label": "无标签",
+  "Choose an album…": "选择相册…",
+  "No albums yet": "还没有相册",
+  "Fix the marked rules to save this album.": "请修正标记的规则以保存此相册。"
 }

crates/ui-egui/locales/zh-hant.json

@@ -1741,5 +1741,8 @@
   "Portrait (tall)": "直向",
   "Square": "正方形",
   "In Color": "彩色",
-  "No Label": "無標籤"
+  "No Label": "無標籤",
+  "Choose an album…": "選擇相簿…",
+  "No albums yet": "還沒有相簿",
+  "Fix the marked rules to save this album.": "請修正標記的規則以儲存此相簿。"
 }

crates/ui-egui/src/control.rs

@@ -310,8 +310,12 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context, req: &ControlRequest
         "ui.dialog.confirm" => match app.ui.dialog.take() {
             Some(d) => {
                 let r = crate::panels::dialogs::confirm_dialog(app, &d);
-                // the import review stays open on an error, as with its button
-                if (r.is_err() && matches!(d, crate::state::Dialog::Import { .. } | crate::state::Dialog::SamModel { .. }))
+                // the import review and the rule editor stay open on an error, as with their buttons
+                if (r.is_err()
+                    && matches!(
+                        d,
+                        crate::state::Dialog::Import { .. } | crate::state::Dialog::SamModel { .. } | crate::state::Dialog::SmartRules { .. }
+                    ))
                     || (r.is_ok() && crate::panels::dialogs::keeps_open(app, &d))
                 {
                     app.ui.dialog = Some(d);

crates/ui-egui/src/panels/dialogs.rs

@@ -242,22 +242,28 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) {
                     let r = ui.add(egui::TextEdit::singleline(name).hint_text(crate::i18n::tr("Name")).desired_width(f32::INFINITY));
                     crate::widgets::register(ui.ctx(), "field:smartName", r.rect);
                     ui.add_space(6.0);
+                    // checked before drawing, so each row marks its own problem
+                    let env = crate::panels::rules_editor::Env {
+                        problems: rules.check(&app.session.catalog),
+                        albums: app.session.catalog.albums().filter(|a| !a.is_smart() && !a.folder).map(|a| (a.id.0, a.name.clone())).collect(),
+                    };
                     egui::ScrollArea::vertical().max_height(360.0).auto_shrink([false, true]).show(ui, |ui| {
-                        crate::panels::rules_editor::edit(ui, rules, "rules", 0);
+                        crate::panels::rules_editor::edit(ui, rules, "rules", 0, &[], &env);
                     });
-                    let problems: Vec<String> = rules.check(&app.session.catalog).iter().map(ToString::to_string).collect();
+                    let problems = rules.check(&app.session.catalog);
                     // the folder an album made from a folder view carries is not in the editor, but it counts
                     let folder = id.and_then(|id| app.session.catalog.album(lightcraft_catalog::AlbumId(id))).and_then(|a| a.smart.as_deref().and_then(|f| f.library_folder.clone()));
                     let f = lightcraft_catalog::Filter { rule_set: Some(rules.clone()), library_folder: folder, ..Default::default() };
                     let n = if problems.is_empty() { app.session.catalog.query(&f, &Default::default()).len() } else { 0 };
                     ui.add_space(4.0);
-                    ui.label(
-                        egui::RichText::new(match problems.first() {
-                            Some(p) => p.clone(),
-                            None => crate::i18n::tr_format!("{n} photo{} match · updates automatically as photos change", if n == 1 { "" } else { "s" }, n = n),
-                        })
-                        .color(t.text_dim),
-                    );
+                    if problems.is_empty() {
+                        ui.label(
+                            egui::RichText::new(crate::i18n::tr_format!("{n} photo{} match · updates automatically as photos change", if n == 1 { "" } else { "s" }, n = n))
+                                .color(t.text_dim),
+                        );
+                    } else {
+                        ui.label(egui::RichText::new(crate::i18n::tr("Fix the marked rules to save this album.")).color(t.caution));
+                    }
                 }
                 Dialog::NewSmartAlbum { name, .. } => {
                     let r = ui.add(egui::TextEdit::singleline(name).hint_text(crate::i18n::tr("Name")).desired_width(f32::INFINITY));
@@ -833,7 +839,10 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) {
                     _ => "OK",
                 };
                 // installing waits for the licence to be accepted
-                let can_confirm = informational || !matches!(&dlg, Dialog::DenoiseModel { accepted: false, .. } | Dialog::FaceModel { accepted: false, .. });
+                let can_confirm = informational
+                    || !matches!(&dlg, Dialog::DenoiseModel { accepted: false, .. } | Dialog::FaceModel { accepted: false, .. })
+                        // smart-album rules that can't mean anything wait to be fixed
+                        && !matches!(&dlg, Dialog::SmartRules { rules, .. } if !rules.check(&app.session.catalog).is_empty());
                 let r = (!ok.is_empty()).then(|| ui.add_enabled(can_confirm, egui::Button::new(crate::i18n::tr(ok))));
                 if let Some(r) = &r {
                     crate::widgets::register(ui.ctx(), "button:dialogOk", r.rect);
@@ -1075,6 +1084,10 @@ pub fn confirm_dialog(app: &mut LightcraftApp, dlg: &Dialog) -> Result<serde_jso
             app.run("photo.analyze", p)
         }
         Dialog::SmartRules { id, name, rules, parent } => {
+            // refused before anything changes (not even the name)
+            if let Some(problem) = rules.check(&app.session.catalog).first() {
+                return Err(problem.to_string());
+            }
             let name = if name.trim().is_empty() { "Smart Album".to_string() } else { name.trim().to_string() };
             match id {
                 Some(id) => {

crates/ui-egui/src/panels/rules_editor.rs

@@ -2,7 +2,7 @@
 //! value), with nested groups. Field and operator lists come from `lightcraft_catalog::rules`.
 
 use egui::RichText;
-use lightcraft_catalog::rules::{FIELD_GROUPS, Kind, TOP_LEVEL_FIELDS, bool_value, field_kind, field_label, ops_for};
+use lightcraft_catalog::rules::{FIELD_GROUPS, Kind, Problem, TOP_LEVEL_FIELDS, bool_value, field_kind, field_label, ops_for};
 use lightcraft_catalog::{Match, Rule, RuleSet};
 use serde_json::{Value, json};
 
@@ -27,6 +27,8 @@ pub fn default_value(field: &str, op: &str) -> Value {
         (Some(Kind::Number), _) => json!(if field == "rating" { 3 } else { 0 }),
         (Some(Kind::Choice(c)), _) => json!(c.first().map_or("", |c| c.0)),
         (Some(Kind::Bool), _) => json!(true),
+        // picked from the list
+        (Some(Kind::Album), _) => Value::Null,
         _ => json!(""),
     }
 }
@@ -77,18 +79,49 @@ fn number_value(ui: &mut egui::Ui, v: &mut Value, field: &str) {
         "longEdge" | "shortEdge" => (0.0, 100_000.0, 10.0),
         _ => (0.0, 100_000.0, 0.5),
     };
-    let whole = matches!(field, "rating" | "iso" | "album" | "keywordCount" | "personCount" | "longEdge" | "shortEdge");
-    let dv = egui::DragValue::new(&mut n).range(lo..=hi).speed(speed);
+    let whole = matches!(field, "rating" | "iso" | "keywordCount" | "personCount" | "longEdge" | "shortEdge");
+    // the range limits dragging; a stored value outside it (a rating of 9 from an agent) is shown
+    // as it is, marked by the check, not quietly clamped to something that matches
+    let dv = egui::DragValue::new(&mut n).range(lo..=hi).clamp_existing_to_range(false).speed(speed);
     let dv = if whole { dv.fixed_decimals(0) } else { dv.max_decimals(2) };
     if ui.add(dv).changed() {
         *v = if whole { json!(n.round() as i64) } else { json!(n) };
     }
 }
 
+/// What the editor shows besides the rules: the problems `RuleSet::check` found in them (each
+/// row marks its own) and the albums an Album rule can test.
+#[derive(Default)]
+pub struct Env {
+    pub problems: Vec<Problem>,
+    /// The plain albums: (id, name), in the sidebar's order.
+    pub albums: Vec<(u64, String)>,
+}
+
+/// An Album rule's value: one of the plain albums, picked by name.
+fn album_value(ui: &mut egui::Ui, v: &mut Value, salt: &str, env: &Env) {
+    let cur = v.as_u64();
+    let text = env.albums.iter().find(|a| Some(a.0) == cur).map_or_else(|| crate::i18n::tr("Choose an album…").to_string(), |a| a.1.clone());
+    let r = egui::ComboBox::from_id_salt(format!("{salt}-album")).width(160.0).selected_text(text).show_ui(ui, |ui| {
+        for (id, name) in &env.albums {
+            let r = ui.selectable_label(cur == Some(*id), name);
+            register(ui.ctx(), format!("ruleAlbumItem:{id}:{salt}"), r.rect);
+            if r.clicked() {
+                *v = json!(id);
+            }
+        }
+        if env.albums.is_empty() {
+            ui.label(crate::i18n::tr("No albums yet"));
+        }
+    });
+    register(ui.ctx(), format!("ruleAlbum:{salt}"), r.response.rect);
+}
+
 /// The value editor for one rule.
-fn value_editor(ui: &mut egui::Ui, field: &str, op: &str, v: &mut Value, salt: &str) {
+fn value_editor(ui: &mut egui::Ui, field: &str, op: &str, v: &mut Value, salt: &str, env: &Env) {
     match (field_kind(field), op) {
         (_, "isEmpty" | "isNotEmpty") => {}
+        (Some(Kind::Album), _) => album_value(ui, v, salt, env),
         (Some(Kind::Date), "inLast" | "notInLast") => {
             if !v.is_object() {
                 *v = default_value(field, op);
@@ -209,8 +242,18 @@ fn field_menu(ui: &mut egui::Ui, salt: &str, field: &mut String, op: &mut String
     *value = default_value(field, op);
 }
 
-/// Edit `rs` in place; `salt` keeps widget ids apart between groups.
-pub fn edit(ui: &mut egui::Ui, rs: &mut RuleSet, salt: &str, depth: usize) {
+/// A rule's problem, shown under it in the caution colour (`ruleProblem:<salt>` for tests and
+/// agents).
+fn problem_note(ui: &mut egui::Ui, problem: Option<&Problem>, salt: &str) {
+    let Some(p) = problem else { return };
+    let t = Tokens::get(ui.ctx());
+    let r = ui.label(RichText::new(format!("⚠ {}", p.message)).color(t.caution).small());
+    register(ui.ctx(), format!("ruleProblem:{salt}"), r.rect);
+}
+
+/// Edit `rs` in place; `salt` keeps widget ids apart between groups, `path` is where `rs` sits
+/// (empty at the top) so each row finds its own problem in `env`.
+pub fn edit(ui: &mut egui::Ui, rs: &mut RuleSet, salt: &str, depth: usize, path: &[usize], env: &Env) {
     let t = Tokens::get(ui.ctx());
     ui.horizontal(|ui| {
         ui.label(RichText::new(crate::i18n::tr("Match")).color(t.text_label));
@@ -221,6 +264,8 @@ pub fn edit(ui: &mut egui::Ui, rs: &mut RuleSet, salt: &str, depth: usize) {
     let mut insert: Option<(usize, Rule)> = None;
     for (i, rule) in rs.rules.iter_mut().enumerate() {
         let rsalt = format!("{salt}-{i}");
+        let here: Vec<usize> = path.iter().copied().chain([i]).collect();
+        let problem = env.problems.iter().find(|p| p.path == here);
         match rule {
             Rule::Group { group } => {
                 egui::Frame::group(ui.style()).show(ui, |ui| {
@@ -228,8 +273,9 @@ pub fn edit(ui: &mut egui::Ui, rs: &mut RuleSet, salt: &str, depth: usize) {
                         if ui.small_button("−").on_hover_text(crate::i18n::tr("Remove this group")).clicked() {
                             remove = Some(i);
                         }
-                        ui.vertical(|ui| edit(ui, group, &rsalt, depth + 1));
+                        ui.vertical(|ui| edit(ui, group, &rsalt, depth + 1, &here, env));
                     });
+                    problem_note(ui, problem, &rsalt);
                 });
             }
             Rule::Field { field, op, value } => {
@@ -248,7 +294,7 @@ pub fn edit(ui: &mut egui::Ui, rs: &mut RuleSet, salt: &str, depth: usize) {
                             }
                         }
                     });
-                    value_editor(ui, field, op, value, &rsalt);
+                    value_editor(ui, field, op, value, &rsalt, env);
                     ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| {
                         let add = ui.small_button("+").on_hover_text(crate::i18n::tr("Add a rule (⌥-click: a group)"));
                         register(ui.ctx(), format!("button:ruleAdd-{rsalt}"), add.rect);
@@ -266,6 +312,7 @@ pub fn edit(ui: &mut egui::Ui, rs: &mut RuleSet, salt: &str, depth: usize) {
                         }
                     });
                 });
+                problem_note(ui, problem, &rsalt);
             }
         }
     }
@@ -320,6 +367,7 @@ mod tests {
                     Kind::Number if *field == "shutterSpeed" => v.as_str().and_then(lightcraft_catalog::parse_shutter_seconds).is_some(),
                     Kind::Number => v.is_number(),
                     Kind::Bool => v.is_boolean(),
+                    Kind::Album => v.is_null(),
                     Kind::Text | Kind::Keywords | Kind::Date => v.is_string(),
                 };
                 match *op {

crates/ui-egui/src/tests_masking.rs

@@ -340,6 +340,73 @@ fn smart_album_editor_keeps_an_unreadable_yes_no_value() {
     assert!(h.app.session.catalog.albums().all(|a| a.name != "Odd"), "OK refuses it: {r}");
 }
 
+/// The rule editor marks each rule that can't mean anything, inside groups too, and keeps OK
+/// disabled until they are fixed; the control channel's confirm is refused the same way.
+#[test]
+fn smart_album_editor_marks_bad_rules_and_blocks_ok() {
+    let mut h = detail("panel.edit");
+    exec(&mut h, "dialog.smartAlbum", json!({"name": "Checked"}));
+    let set = |h: &mut Headless, rules: serde_json::Value| {
+        let Some(crate::state::Dialog::SmartRules { rules: r, .. }) = &mut h.app.ui.dialog else { panic!("no rule editor") };
+        *r = serde_json::from_value(json!({"rules": rules})).unwrap();
+        h.settle(SETTLE);
+    };
+    let has = |h: &Headless, id: &str| h.app.widgets.iter().any(|(w, _)| w == id);
+    set(
+        &mut h,
+        json!([
+            {"field": "rating", "op": "gte", "value": 3},
+            {"field": "captureDate", "op": "is", "value": "banana"},
+            {"group": {"match": "any", "rules": [{"field": "iso", "op": "is", "value": 100}, {"field": "rating", "op": "is", "value": 9}]}}
+        ]),
+    );
+    assert!(!has(&h, "ruleProblem:rules-0"), "a good rule isn't marked");
+    assert!(has(&h, "ruleProblem:rules-1"), "the bad date is marked");
+    assert!(has(&h, "ruleProblem:rules-2-1") && !has(&h, "ruleProblem:rules-2-0"), "inside a group, the bad rule itself");
+    let Some(crate::state::Dialog::SmartRules { rules, .. }) = &h.app.ui.dialog else { panic!("no rule editor") };
+    assert!(serde_json::to_string(rules).unwrap().contains(r#""value":9"#), "drawing doesn't clamp the rating to 5");
+    let r = h.request("ui.clickWidget", json!({"id": "button:dialogOk"}), T);
+    assert_eq!(r["ok"], true, "{r}");
+    h.settle(SETTLE);
+    assert!(h.app.session.catalog.albums().all(|a| a.name != "Checked"), "OK is disabled");
+    let r = h.request("ui.dialog.confirm", json!({}), T);
+    assert!(h.app.session.catalog.albums().all(|a| a.name != "Checked"), "confirm refuses too: {r}");
+    assert!(h.app.ui.dialog.is_some(), "and the dialog stays open with the rules");
+    // fixed, the marks go and OK saves
+    set(&mut h, json!([{"field": "rating", "op": "gte", "value": 3}, {"field": "captureDate", "op": "is", "value": "2026"}]));
+    assert!(!h.app.widgets.iter().any(|(w, _)| w.starts_with("ruleProblem:")));
+    let r = h.request("ui.clickWidget", json!({"id": "button:dialogOk"}), T);
+    assert_eq!(r["ok"], true, "{r}");
+    h.settle(SETTLE);
+    assert!(h.app.session.catalog.albums().any(|a| a.name == "Checked"));
+}
+
+/// An Album rule picks its album from a list of the plain albums, not by typing an id.
+#[test]
+fn smart_album_editor_picks_an_album_from_a_list() {
+    let mut h = detail("panel.edit");
+    let trip = exec(&mut h, "album.create", json!({"name": "Trip", "addSelected": false}))["id"].as_u64().unwrap();
+    let smart = exec(&mut h, "album.createSmart", json!({"name": "Fives", "rules": {"rating": 5}}))["id"].as_u64().unwrap();
+    exec(&mut h, "dialog.smartAlbum", json!({"name": "In Trip"}));
+    let Some(crate::state::Dialog::SmartRules { rules, .. }) = &mut h.app.ui.dialog else { panic!("no rule editor") };
+    rules.rules[0] = serde_json::from_value(json!({"field": "album", "op": "is", "value": 0})).unwrap();
+    h.settle(SETTLE);
+    assert!(h.app.widgets.iter().any(|(w, _)| w == "ruleProblem:rules-0"), "no album chosen yet");
+    let r = h.request("ui.clickWidget", json!({"id": "ruleAlbum:rules-0"}), T);
+    assert_eq!(r["ok"], true, "{r}");
+    h.settle(SETTLE);
+    let has = |h: &Headless, id: String| h.app.widgets.iter().any(|(w, _)| *w == id);
+    assert!(has(&h, format!("ruleAlbumItem:{trip}:rules-0")), "plain albums are offered");
+    assert!(!has(&h, format!("ruleAlbumItem:{smart}:rules-0")), "smart albums can't be tested");
+    let r = h.request("ui.clickWidget", json!({"id": format!("ruleAlbumItem:{trip}:rules-0")}), T);
+    assert_eq!(r["ok"], true, "{r}");
+    h.settle(SETTLE);
+    let Some(crate::state::Dialog::SmartRules { rules, .. }) = &h.app.ui.dialog else { panic!("no rule editor") };
+    let Rule::Field { value, .. } = &rules.rules[0] else { panic!("not a field rule") };
+    assert_eq!(value, &json!(trip));
+    assert!(!h.app.widgets.iter().any(|(w, _)| w == "ruleProblem:rules-0"));
+}
+
 #[test]
 fn g_toggles_grids_and_shift_g_starts_guided_upright() {
     let mut h = detail("panel.edit");