fix(engine): an unknown layer id or name lists the document's layers, and a name matches regardless of case as a fallback (#5058) (#5133)

A command that fails on a layer id or name the document doesn't have now says which layers there are, top-down as id and name (at most 20, names cut at 64 characters), so an agent can recover from the error alone: ids are not positions, and a freshly opened PNG's Background is layer 2. NoLayer becomes UnknownLayer { id, known } at both command entry points, keeping the 'no such layer' prefix. A layer name that matches no layer exactly matches regardless of case; an exact match always wins.

Fixes #5058.

crates/engine/src/commands.rs

@@ -295,7 +295,8 @@ fn set_clipped(s: &mut Session, p: &Value, clip: bool) -> Result<Value> {
 
 /// The layer a command's `"layer"` param names: an id, or a name (the topmost layer of that
 /// name, as Photoshop's "select layer by name" action step finds it, so a loaded action replays
-/// in another document). `None` when the param is absent.
+/// in another document; a name that matches no layer exactly matches regardless of case, so
+/// `"background"` finds the Background). `None` when the param is absent.
 pub(crate) fn explicit_layer(s: &Session, p: &Value) -> Result<Option<LayerId>> {
     match p.get("layer") {
         Some(Value::Number(n)) => {
@@ -303,19 +304,41 @@ pub(crate) fn explicit_layer(s: &Session, p: &Value) -> Result<Option<LayerId>>
         }
         Some(Value::String(name)) => {
             let d = s.active().ok_or(EngineError::NoDocument)?;
-            d.doc
-                .walk()
-                .into_iter()
-                .rev()
+            let layers = d.doc.walk();
+            let mut top_down = layers.iter().rev();
+            top_down
+                .clone()
                 .find(|(_, _, l)| l.name == *name)
+                .or_else(|| top_down.find(|(_, _, l)| l.name.to_lowercase() == name.to_lowercase()))
                 .map(|(_, _, l)| Some(l.id))
-                .ok_or_else(|| EngineError::Other(format!("no layer named \"{name}\"")))
+                .ok_or_else(|| EngineError::Other(format!("no layer named \"{name}\"; {}", layer_listing(&d.doc))))
         }
         Some(Value::Null) | None => Ok(None),
         Some(other) => Err(EngineError::Other(format!("`layer` must be an id or a name, not {other}"))),
     }
 }
 
+/// The layers of `doc` as a caller can name them, for an error about a layer that isn't there:
+/// `the document's layers are 2 "Background", 3 "Layer 1"` (top-down, at most 20).
+pub(crate) fn layer_listing(doc: &Document) -> String {
+    const MAX: usize = 20;
+    let layers = doc.walk();
+    if layers.is_empty() {
+        return "the document has no layers".into();
+    }
+    // Names are unbounded input: each one is cut at NAME characters so the error stays short.
+    const NAME: usize = 64;
+    let name = |n: &str| match n.char_indices().nth(NAME) {
+        Some((cut, _)) => format!("{}…", &n[..cut]),
+        None => n.to_string(),
+    };
+    let mut list: Vec<String> = layers.iter().rev().take(MAX).map(|(_, _, l)| format!("{} {:?}", l.id.0, name(&l.name))).collect();
+    if layers.len() > MAX {
+        list.push(format!("… {} more (see `document.inspect`)", layers.len() - MAX));
+    }
+    format!("the document's layers are {} (a `layer` param takes an id or a name)", list.join(", "))
+}
+
 /// The layer a command targets: [`explicit_layer`], else the active layer.
 pub(crate) fn layer_param(s: &Session, p: &Value) -> Result<LayerId> {
     match explicit_layer(s, p)? {

crates/engine/src/jobs.rs

@@ -543,7 +543,7 @@ impl Session {
                 job.ctx.cancel();
                 Err(e)
             }
-            (Err(e), None) => Err(e),
+            (Err(e), None) => Err(self.explain_missing_layer(e)),
             (Ok(_), Some(mut job)) => {
                 job.command = command.to_string();
                 job.params = params;
@@ -740,7 +740,7 @@ impl Session {
                 job.ctx.cancel();
                 Err(e)
             }
-            (Err(e), None) => Err(e),
+            (Err(e), None) => Err(self.explain_missing_layer(e)),
             (Ok(_), Some(mut job)) => {
                 job.command = id.to_string();
                 job.params = params;
@@ -756,6 +756,18 @@ impl Session {
         }
     }
 
+    /// A command that failed on a layer id the document doesn't have says which ids it does have
+    /// (#5058): ids are not positions (an opened PNG's Background is not layer 1), and an agent
+    /// should not need `document.inspect` to recover.
+    fn explain_missing_layer(&self, e: EngineError) -> EngineError {
+        match (e, self.active()) {
+            (EngineError::NoLayer(id), Some(st)) if st.doc.layer(id).is_none() => {
+                EngineError::UnknownLayer { id, known: crate::commands::layer_listing(&st.doc) }
+            }
+            (e, _) => e,
+        }
+    }
+
     /// Bookkeeping after a command (or a job's apply) succeeded.
     ///
     /// `nested_from` is the journal length when the command started. A replayable command stands

crates/engine/src/lib.rs

@@ -156,6 +156,10 @@ pub enum EngineError {
     NoDocument,
     #[error("no such layer {0:?}")]
     NoLayer(LayerId),
+    /// [`Self::NoLayer`] as a command reports it: with the layers the document does have, as
+    /// ids are not positions (#5058).
+    #[error("no such layer {}; {known}", id.0)]
+    UnknownLayer { id: LayerId, known: String },
     #[error("{0}")]
     Other(String),
     /// A background job was cancelled (see [`jobs`]); nothing changed.

crates/engine/src/type_caret_cmds.rs

@@ -482,7 +482,7 @@ mod tests {
         bad(&mut s, "type.navigate", json!({"layer": id, "index": 9, "move": "end"}));
         bad(&mut s, "type.navigate", json!({"layer": id, "index": 0, "move": "lineNext", "x": "no"}));
         bad(&mut s, "type.navigate", json!({"layer": pixel, "index": 0, "move": "start"}));
-        assert!(matches!(s.execute("type.caret", json!({"layer": u64::MAX, "index": 0})), Err(EngineError::NoLayer(_))));
+        assert!(matches!(s.execute("type.caret", json!({"layer": u64::MAX, "index": 0})), Err(EngineError::UnknownLayer { .. })));
         // A point on the named layer but outside its text still returns a caret, marked outside.
         let far = s.execute("type.hitTest", json!({"layer": id, "x": 390, "y": 290})).unwrap();
         assert_eq!(far["inside"], false);

crates/engine/tests/missing_layer_error.rs

@@ -0,0 +1,68 @@
+//! A `"layer"` param that names no layer says which layers there are (#5058): ids are not
+//! positions (a freshly opened PNG's Background is layer 2), and an agent should be able to
+//! recover from the error alone. A name matches regardless of case as a fallback.
+
+use photocraft_engine::{EngineError, Session};
+use serde_json::json;
+
+fn session() -> (Session, u64) {
+    let mut s = Session::new();
+    s.execute("file.new", json!({"width": 8, "height": 6})).unwrap();
+    let bg = s.active().unwrap().active_layer.unwrap().0;
+    (s, bg)
+}
+
+fn unused_id(s: &Session) -> u64 {
+    let doc = &s.active().unwrap().doc;
+    (1..).find(|id| doc.layer(photocraft_doc::LayerId(*id)).is_none()).unwrap()
+}
+
+#[test]
+fn an_unknown_layer_id_lists_the_documents_layers() {
+    let (mut s, bg) = session();
+    let layer = s.execute("layer.new.layer", json!({})).unwrap()["layer"].as_u64().unwrap();
+    let missing = unused_id(&s);
+    let err = s.execute("layer.setProps", json!({"layer": missing, "visible": false})).unwrap_err();
+    assert!(matches!(err, EngineError::UnknownLayer { id, .. } if id.0 == missing), "{err}");
+    let err = err.to_string();
+    assert!(err.contains(&format!("no such layer {missing};")), "{err}");
+    assert!(err.contains(&format!("{layer} \"Layer 1\", {bg} \"Background\"")), "top-down ids and names: {err}");
+}
+
+#[test]
+fn an_unknown_layer_name_lists_the_documents_layers() {
+    let (mut s, bg) = session();
+    let err = s.execute("layer.setProps", json!({"layer": "Nope", "visible": false})).unwrap_err().to_string();
+    assert!(err.contains("no layer named \"Nope\""), "{err}");
+    assert!(err.contains(&format!("{bg} \"Background\"")), "{err}");
+}
+
+#[test]
+fn a_layer_name_matches_regardless_of_case() {
+    let (mut s, bg) = session();
+    s.execute("layer.new.layer", json!({})).unwrap();
+    s.execute("layer.setProps", json!({"layer": "background", "name": "Base"})).unwrap();
+    assert_eq!(s.active().unwrap().doc.layer(photocraft_doc::LayerId(bg)).unwrap().name, "Base");
+}
+
+#[test]
+fn an_exact_name_wins_over_a_case_insensitive_one() {
+    let (mut s, _) = session();
+    let lower = s.execute("layer.new.layer", json!({"name": "shadow"})).unwrap()["layer"].as_u64().unwrap();
+    let upper = s.execute("layer.new.layer", json!({"name": "Shadow"})).unwrap()["layer"].as_u64().unwrap();
+    s.execute("layer.setProps", json!({"layer": "shadow", "visible": false})).unwrap();
+    let doc = &s.active().unwrap().doc;
+    assert!(!doc.layer(photocraft_doc::LayerId(lower)).unwrap().visible);
+    assert!(doc.layer(photocraft_doc::LayerId(upper)).unwrap().visible);
+}
+
+#[test]
+fn a_long_layer_name_is_shortened_in_the_listing() {
+    let (mut s, _) = session();
+    let long = "é".repeat(500);
+    let layer = s.execute("layer.new.layer", json!({"name": long})).unwrap()["layer"].as_u64().unwrap();
+    let err = s.execute("layer.setProps", json!({"layer": "Nope", "visible": false})).unwrap_err().to_string();
+    let shown = format!("{layer} \"{}…\"", "é".repeat(64));
+    assert!(err.contains(&shown), "the name is cut at 64 characters: {err}");
+    assert!(err.chars().count() < 300, "{err}");
+}