fix(ui-egui): #1046 decide on error kinds and flags, not message text

LightcraftApp::run_typed keeps the engine's ErrorKind in a CommandError;
run() is run_typed(..).map_err(|e| e.message), so its callers are
untouched. The face detector offer and the SAM 3 download dialog decide
on ModelMissing, the SAM 3 download toast and the Apple Photos note on
the `cancelled` flag, and a Lightroom inspect/import on its own cancel
flag only, so a failure that mentions "cancelled" is still a failure.

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

crates/ui-egui/src/command_error.rs

@@ -0,0 +1,42 @@
+//! A refused or failed command as the UI handles it (#1046): the message for people, and what kind of
+//! error it is for code that decides on it. Decide on [`CommandError::kind`], never on the message:
+//! messages get reworded and will be translated.
+
+use lightcraft_engine::{EngineError, ErrorKind};
+
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub struct CommandError {
+    pub message: String,
+    pub kind: ErrorKind,
+}
+
+impl std::fmt::Display for CommandError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.write_str(&self.message)
+    }
+}
+
+/// An error the UI raised itself (its own commands, dialogs): [`ErrorKind::Other`].
+impl From<String> for CommandError {
+    fn from(message: String) -> Self {
+        Self { message, kind: ErrorKind::Other }
+    }
+}
+
+impl From<&str> for CommandError {
+    fn from(message: &str) -> Self {
+        message.to_string().into()
+    }
+}
+
+impl From<EngineError> for CommandError {
+    fn from(e: EngineError) -> Self {
+        Self { kind: e.kind(), message: e.to_string() }
+    }
+}
+
+impl From<CommandError> for String {
+    fn from(e: CommandError) -> Self {
+        e.message
+    }
+}

crates/ui-egui/src/export_task.rs

@@ -138,7 +138,7 @@ pub fn apple_photos_note(outcome: &Value) -> String {
         return crate::i18n::tr_format!("not added to Apple Photos: {e}", e = e);
     }
     if let Some(why) = outcome.get("skipped").and_then(Value::as_str) {
-        if why.contains("cancelled") {
+        if outcome.get("cancelled").and_then(Value::as_bool) == Some(true) {
             return crate::i18n::tr("not added to Apple Photos (cancelled)").to_string();
         }
         return crate::i18n::tr_format!("not added to Apple Photos: {e}", e = why);
@@ -342,7 +342,14 @@ mod tests {
             &json!({"error": "LightCraft isn't allowed to control Photos. Allow it in System Settings › Privacy & Security › Automation"}),
         );
         assert!(denied.starts_with("not added to Apple Photos: ") && denied.contains("Automation"), "{denied}");
-        assert_eq!(apple_photos_note(&json!({"skipped": "the export was cancelled"})), "not added to Apple Photos (cancelled)");
+        assert_eq!(apple_photos_note(&json!({"skipped": "the export was cancelled", "cancelled": true})), "not added to Apple Photos (cancelled)");
         assert_eq!(apple_photos_note(&json!({"skipped": "the export wrote no files"})), "not added to Apple Photos: the export wrote no files");
+        // decided on the `cancelled` flag, not the words (#1046): reworded it is still a cancel, and a
+        // reason that only mentions the word is shown as it is
+        assert_eq!(apple_photos_note(&json!({"skipped": "Export abgebrochen", "cancelled": true})), "not added to Apple Photos (cancelled)");
+        assert_eq!(
+            apple_photos_note(&json!({"skipped": "an earlier, cancelled request is still running"})),
+            "not added to Apple Photos: an earlier, cancelled request is still running"
+        );
     }
 }

crates/ui-egui/src/lib.rs

@@ -8,6 +8,7 @@
 
 pub mod album_picker;
 pub mod brush_live;
+mod command_error;
 pub mod control;
 pub mod credits;
 pub mod date_picker;
@@ -64,6 +65,8 @@ mod tests_dialog_fit;
 #[cfg(test)]
 mod tests_dialogs;
 #[cfg(test)]
+mod tests_error_kinds;
+#[cfg(test)]
 mod tests_filmstrip;
 #[cfg(test)]
 mod tests_grading;
@@ -124,9 +127,10 @@ mod tests_zoom_keys;
 
 use std::sync::mpsc::{Receiver, Sender};
 
-use lightcraft_engine::Session;
+use lightcraft_engine::{AiModel, ErrorKind, Session};
 use serde_json::Value;
 
+pub use command_error::CommandError;
 pub use control::{ControlRequest, ControlResponse};
 pub use state::UiState;
 
@@ -487,14 +491,20 @@ impl LightcraftApp {
 
     /// Run a UI or engine command by id. The single entry point for every frontend path.
     pub fn run(&mut self, id: &str, params: Value) -> Result<Value, String> {
+        self.run_typed(id, params).map_err(|e| e.message)
+    }
+
+    /// [`LightcraftApp::run`], keeping what kind of error a refusal is: for code that decides on it
+    /// (offer a model's download, treat a cancel as one) instead of reading the message.
+    pub fn run_typed(&mut self, id: &str, params: Value) -> Result<Value, CommandError> {
         if let Some(result) = model_setup::intercept(self, id, &params) {
-            return result;
+            return result.map_err(CommandError::from);
         }
         if let Some(r) = menus::run_ui_command(self, id, &params) {
             if r.is_ok() {
                 docking::reveal_command(self, id)?;
             }
-            return r;
+            return r.map_err(CommandError::from);
         }
         let params = if id == "crop.aspect"
             && params.get("aspect").and_then(Value::as_str) == Some("toggle")
@@ -505,7 +515,7 @@ impl LightcraftApp {
         } else {
             params
         };
-        let r = self.session.execute(id, &params).map_err(|e| e.to_string());
+        let r = self.session.execute(id, &params).map_err(CommandError::from);
         if r.is_ok() && matches!(id, "crop.aspect" | "crop.rotateAspect") {
             self.ui.crop_default_aspect = if id == "crop.aspect" && params.get("aspect").and_then(Value::as_str) == Some("original") {
                 Some(serde_json::json!("original"))
@@ -553,9 +563,9 @@ impl LightcraftApp {
         match &r {
             Err(e) => {
                 log::warn!("{id}: {e}");
-                self.ui.status = e.clone();
+                self.ui.status = e.message.clone();
                 // no detector yet: offer it (its terms first) instead of only saying so
-                if id == "faces.detect" && e.contains("Settings > Faces") {
+                if id == "faces.detect" && e.kind == ErrorKind::ModelMissing(AiModel::FaceDetector) {
                     panels::faces::offer_detector(self);
                 }
             }
@@ -735,16 +745,11 @@ impl LightcraftApp {
         if self.ui.sam_downloading && !download.running {
             self.ui.sam_downloading = false;
             let open = matches!(self.ui.dialog, Some(state::Dialog::SamModel { .. }));
-            match (&download.error, open) {
-                (_, true) => {}
-                (Some(e), false) if e.contains("cancelled") => {
-                    self.toast(ctx, crate::i18n::tr("SAM 3 download stopped: it resumes where it left off next time."))
-                }
-                (Some(e), false) => self.toast_error(ctx, crate::i18n::tr_format!("The SAM 3 download failed: {e}", e = e)),
-                (None, false) if download.finished => {
-                    self.toast_error(ctx, crate::i18n::tr("The SAM 3 model is installed: Object and Describe masks are ready."))
-                }
-                (None, false) => {}
+            match sam_download_ended(&download) {
+                _ if open => {}
+                Some((text, true)) => self.toast_error(ctx, text),
+                Some((text, false)) => self.toast(ctx, text),
+                None => {}
             }
         }
         if let Some((due, mask)) = self.ui.detail_due {
@@ -763,13 +768,14 @@ impl LightcraftApp {
     }
 
     /// An AI mask error: when the model isn't installed, the dialog that offers to download it
-    /// (`then`: the AI mask to start afterwards); otherwise a toast.
-    pub fn ai_error(&mut self, ctx: &egui::Context, e: impl Into<String>, then: Option<(&str, &str)>) {
+    /// (`then`: the AI mask to start afterwards); otherwise a toast. Decided on the error's kind
+    /// ([`ErrorKind::ModelMissing`]), not its words.
+    pub fn ai_error(&mut self, ctx: &egui::Context, e: impl Into<CommandError>, then: Option<(&str, &str)>) {
         let e = e.into();
-        if lightcraft_engine::segment::Segmenter::AVAILABLE && e.starts_with(lightcraft_engine::segment::NOT_INSTALLED) {
+        if lightcraft_engine::segment::Segmenter::AVAILABLE && e.kind == ErrorKind::ModelMissing(AiModel::Sam3) {
             self.offer_sam_download(then);
         } else {
-            self.toast_error(ctx, e);
+            self.toast_error(ctx, e.message);
         }
     }
 
@@ -1377,6 +1383,19 @@ fn detect_summary(v: &Value) -> String {
     }
 }
 
+/// What to say when a SAM 3 download ended while its dialog was closed: the text, and whether it is
+/// shown as an error toast. A cancel is told by `cancelled`, not by the error's words.
+fn sam_download_ended(download: &lightcraft_engine::segment::DownloadStatus) -> Option<(String, bool)> {
+    match &download.error {
+        Some(_) if download.cancelled => {
+            Some((crate::i18n::tr("SAM 3 download stopped: it resumes where it left off next time.").to_string(), false))
+        }
+        Some(e) => Some((crate::i18n::tr_format!("The SAM 3 download failed: {e}", e = e), true)),
+        None if download.finished => Some((crate::i18n::tr("The SAM 3 model is installed: Object and Describe masks are ready.").to_string(), true)),
+        None => None,
+    }
+}
+
 /// A dropped face model file (`.onnx`).
 pub fn is_model_file(path: &str) -> bool {
     std::path::Path::new(path).extension().is_some_and(|e| e.eq_ignore_ascii_case("onnx"))

crates/ui-egui/src/lightroom_import.rs

@@ -352,17 +352,19 @@ pub fn tick(app: &mut LightcraftApp, ctx: &egui::Context) {
             return;
         }
     };
+    // The task's cancel flag says whether it was cancelled (the worker only stops for it), not an
+    // error's words: a real failure that mentions "cancelled" is still reported as a failure.
     match message {
         Message::Inspected(result) => match result {
             Ok(report) if !task.cancel.load(Ordering::Relaxed) => terminal(app, ctx, &task, report),
             Ok(_) => terminal(app, ctx, &task, json!({"cancelled": true})),
-            Err(e) if task.cancel.load(Ordering::Relaxed) || e.contains("cancelled") => terminal(app, ctx, &task, json!({"cancelled": true})),
+            Err(_) if task.cancel.load(Ordering::Relaxed) => terminal(app, ctx, &task, json!({"cancelled": true})),
             Err(e) => terminal(app, ctx, &task, json!({"error": e})),
         },
         Message::Prepared(result) => {
             let prepared = match result {
                 Ok(p) => p,
-                Err(e) if task.cancel.load(Ordering::Relaxed) || e.contains("cancelled") => {
+                Err(_) if task.cancel.load(Ordering::Relaxed) => {
                     terminal(app, ctx, &task, json!({"cancelled": true}));
                     return;
                 }
@@ -518,6 +520,30 @@ mod tests {
         assert!(h.app.session.activity.list().is_empty());
     }
 
+    /// Whether the task was cancelled is the task's flag, not an error's words (#1046): a cancel
+    /// reads as one whatever the message says, and a failure that mentions "cancelled" is a failure.
+    #[test]
+    fn lightroom_cancel_is_the_flag_not_the_words() {
+        let mut h = demo();
+        let (task, tx, cancel, _, _) = fake_import(&h);
+        h.app.lightroom = Some(task);
+        cancel.store(true, Ordering::Relaxed);
+        tx.send(Message::Prepared(Err("Lightroom-Import abgebrochen".into()))).unwrap();
+        h.step();
+        assert_eq!(h.app.lightroom_last, Some(serde_json::json!({"cancelled": true, "path": "fixture.lrcat"})));
+
+        for message in
+            [Message::Prepared(Err("the drive was ejected: copy cancelled by the system".into())), Message::Inspected(Err("cancelled".into()))]
+        {
+            let (task, tx, _, _, _) = fake_import(&h);
+            h.app.lightroom = Some(task);
+            tx.send(message).unwrap();
+            h.step();
+            let last = h.app.lightroom_last.clone().unwrap_or_default();
+            assert!(last.get("cancelled").is_none() && last["error"].as_str().is_some_and(|e| e.contains("cancelled")), "{last}");
+        }
+    }
+
     #[test]
     fn inspect_report_keeps_empty_catalog_shape() {
         let report = inspect_report(CatalogImport {

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

@@ -1808,7 +1808,7 @@ fn mask_overlay(app: &mut LightcraftApp, ui: &mut egui::Ui, resp: &egui::Respons
         {
             let n = map.norm(q);
             let exclude = ui.input(|i| i.modifiers.alt);
-            match app.run("mask.objectPoint", json!({"x": n.x, "y": n.y, "exclude": exclude})) {
+            match app.run_typed("mask.objectPoint", json!({"x": n.x, "y": n.y, "exclude": exclude})) {
                 Ok(_) => {
                     if let Some(mid) = app.session.active_mask {
                         app.ui.detail_due = Some((ui.input(|i| i.time) + 1.0, mid));

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

@@ -685,8 +685,11 @@ pub(crate) fn start_object(app: &mut LightcraftApp, ctx: &egui::Context, op: &st
     if needs_model(app, "object", op) {
         return;
     }
-    let r =
-        if op == "new" { app.run("mask.add", json!({"kind": "object"})) } else { app.run("mask.addComponent", json!({"op": op, "kind": "object"})) };
+    let r = if op == "new" {
+        app.run_typed("mask.add", json!({"kind": "object"}))
+    } else {
+        app.run_typed("mask.addComponent", json!({"op": op, "kind": "object"}))
+    };
     match r {
         Ok(_) => {
             app.ui.tool = "object".into();
@@ -764,9 +767,9 @@ fn describe_field(app: &mut LightcraftApp, ui: &mut egui::Ui, new_mask: bool) {
     app.ui.describe = Some((op.clone(), text.clone()));
     if submit && !text.trim().is_empty() {
         let r = if op == "new" {
-            app.run("mask.add", json!({"kind": "prompt", "text": text}))
+            app.run_typed("mask.add", json!({"kind": "prompt", "text": text}))
         } else {
-            app.run("mask.addComponent", json!({"op": op, "kind": "prompt", "text": text}))
+            app.run_typed("mask.addComponent", json!({"op": op, "kind": "prompt", "text": text}))
         };
         match r {
             // the mask appears when the model has found it (a detail pass follows)

crates/ui-egui/src/tests_error_kinds.rs

@@ -0,0 +1,70 @@
+//! The UI decides on what kind of error a command returned, not on its words (#1046): messages get
+//! reworded and translated, and a decision that reads them breaks silently when they do.
+
+use lightcraft_engine::segment::{DownloadStatus, NOT_INSTALLED, Segmenter};
+use lightcraft_engine::{AiModel, ErrorKind};
+use serde_json::json;
+
+use crate::headless::Headless;
+use crate::state::Dialog;
+use crate::{CommandError, LightcraftApp, Services};
+
+fn demo() -> Headless {
+    let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), Services { png: None, ..Default::default() });
+    Headless::new(app, [1200.0, 800.0], 1.0)
+}
+
+/// An AI mask error offers the model on `ModelMissing(Sam3)`, whatever the message says, and a
+/// message that merely starts like the engine's is a toast.
+#[test]
+fn ai_mask_errors_offer_the_model_by_kind() {
+    let mut h = demo();
+    let ctx = h.view.ctx.clone();
+    let missing = CommandError { message: "Das SAM-3-Modell ist nicht installiert".into(), kind: ErrorKind::ModelMissing(AiModel::Sam3) };
+    h.app.ai_error(&ctx, missing, Some(("object", "new")));
+    if Segmenter::AVAILABLE {
+        assert_eq!(h.app.ui.dialog, Some(Dialog::SamModel { then: Some(("object".into(), "new".into())), error: None }));
+    } else {
+        assert_eq!(h.app.ui.dialog, None, "a build without AI masks has nothing to offer");
+    }
+    h.app.ui.dialog = None;
+    h.app.ai_error(&ctx, format!("{NOT_INSTALLED}: but this is some other failure"), None);
+    assert_eq!(h.app.ui.dialog, None, "decided on the kind, not the words");
+    // another model missing is not SAM 3's
+    let face = CommandError { message: "x".into(), kind: ErrorKind::ModelMissing(AiModel::FaceDetector) };
+    h.app.ai_error(&ctx, face, None);
+    assert_eq!(h.app.ui.dialog, None);
+}
+
+/// `run_typed` keeps the engine's kind for the code that decides, and `run` the same message.
+#[test]
+fn run_typed_keeps_the_kind_and_run_the_message() {
+    let mut h = demo();
+    let dir = std::env::temp_dir().join(format!("lc-ui-error-kinds-{}", std::process::id()));
+    let _ = std::fs::remove_dir_all(&dir);
+    h.app.session.face_models_dir = Some(dir.join("models"));
+    let typed = h.app.run_typed("faces.detect", json!({})).unwrap_err();
+    assert_eq!(typed.kind, ErrorKind::ModelMissing(AiModel::FaceDetector), "{typed}");
+    h.app.ui.dialog = None;
+    let text = h.app.run("faces.detect", json!({})).unwrap_err();
+    assert_eq!(text, typed.message);
+    let unknown = h.app.run_typed("no.such.command", json!({})).unwrap_err();
+    assert_eq!(unknown.kind, ErrorKind::UnknownCommand, "{unknown}");
+    // an error the UI raised itself is `Other`
+    assert_eq!(CommandError::from("Tick the box to accept the model's terms first".to_string()).kind, ErrorKind::Other);
+    let _ = std::fs::remove_dir_all(&dir);
+}
+
+/// A SAM 3 download that ended while its dialog was closed: a cancel is told by the status's
+/// `cancelled`, not by the error's words.
+#[test]
+fn a_cancelled_sam_download_is_told_by_its_flag() {
+    let ended = |error: &str, cancelled: bool| {
+        crate::sam_download_ended(&DownloadStatus { error: Some(error.into()), cancelled, ..DownloadStatus::default() }).unwrap()
+    };
+    let (text, error) = ended("Download abgebrochen", true);
+    assert!(!error && text.starts_with("SAM 3 download stopped"), "{text}");
+    let (text, error) = ended("could not download model.safetensors: the mirror cancelled the transfer", false);
+    assert!(error && text.starts_with("The SAM 3 download failed: "), "{text}");
+    assert_eq!(crate::sam_download_ended(&DownloadStatus::default()), None);
+}