From 259b6e67e88324780815a75912dc4159be5b472a Mon Sep 17 00:00:00 2001
From: prxnvt <53838599+prxnvt@users.noreply.github.com>
Date: Fri, 23 Oct 2026 21:45:54 -0400
Subject: [PATCH] Model: Repeat and If nest four deep, as the limit says

The depth check refused any step list at the fifth level, so a fourth Repeat
or If could not even hold an empty body: three levels nested, not the four
MAX_MACRO_DEPTH and the docs state. The check now counts the Repeat or If
steps themselves, in the model's limits and in the parser's pass over the
JSON. The test asks for four and refuses a fifth.
---
 crates/engine/src/macros/mod.rs   |  8 +++++---
 crates/engine/src/macros/tests.rs | 19 ++++++++++++++-----
 crates/model/src/macros.rs        |  7 ++++---
 3 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/crates/engine/src/macros/mod.rs b/crates/engine/src/macros/mod.rs
index 38016049d..2aa45e12c 100644
--- a/crates/engine/src/macros/mod.rs
+++ b/crates/engine/src/macros/mod.rs
@@ -53,9 +53,6 @@ fn check_size(m: &Macro) -> Result<(), String> {
 /// before the steps are deserialized.
 fn normalize_steps(steps: &mut Json, level: usize) -> Result<(), String> {
     let Some(list) = steps.as_array_mut() else { return Err("`steps` must be a list.".into()) };
-    if level > gridcraft_model::MAX_MACRO_DEPTH {
-        return Err(format!("Repeat and If steps can nest at most {} deep.", gridcraft_model::MAX_MACRO_DEPTH));
-    }
     for st in list {
         let Some(o) = st.as_object_mut() else { return Err("Every step must be an object.".into()) };
         if !o.contains_key("type")
@@ -64,6 +61,11 @@ fn normalize_steps(steps: &mut Json, level: usize) -> Result<(), String> {
             o.insert("type".into(), json!("command"));
             o.insert("id".into(), id);
         }
+        // A step with a body is a Repeat or an If: one at the top is nested 1 deep.
+        let nested = ["body", "then", "else"].iter().any(|key| o.contains_key(*key));
+        if nested && level > gridcraft_model::MAX_MACRO_DEPTH {
+            return Err(format!("Repeat and If steps can nest at most {} deep.", gridcraft_model::MAX_MACRO_DEPTH));
+        }
         for key in ["body", "then", "else"] {
             if let Some(body) = o.get_mut(key) {
                 normalize_steps(body, level + 1)?;
diff --git a/crates/engine/src/macros/tests.rs b/crates/engine/src/macros/tests.rs
index 7b498f0b8..5fbc6d38c 100644
--- a/crates/engine/src/macros/tests.rs
+++ b/crates/engine/src/macros/tests.rs
@@ -55,16 +55,25 @@ fn macros_over_the_limits_are_refused() {
     let flat = |n: usize| json!({"name": "Big", "steps": vec![json!({"command": "cell.set"}); n]});
     assert!(parse_macro(&flat(MAX_MACRO_STEPS)).is_ok());
     assert!(parse_macro(&flat(MAX_MACRO_STEPS + 1)).unwrap_err().contains("10000 steps"));
-    // Nested Repeat steps: 4 levels are fine, 5 are not.
-    let nest = |levels: usize| {
+    // Nested Repeat steps: 4 Repeats one inside the other are fine (the innermost holds a
+    // command, or nothing), a 5th is not.
+    let nest = |repeats: usize| {
         let mut step = json!({"command": "cell.set"});
-        for _ in 1..levels {
+        for _ in 0..repeats {
             step = json!({"type": "repeat", "over": "Input", "body": [step]});
         }
         json!({"name": "Deep", "steps": [step]})
     };
-    assert!(parse_macro(&nest(4)).is_ok());
-    assert!(parse_macro(&nest(5)).unwrap_err().contains("4 deep"));
+    assert!(parse_macro(&nest(MAX_MACRO_DEPTH)).is_ok());
+    assert_eq!(parse_macro(&nest(MAX_MACRO_DEPTH)).unwrap().step_count(), MAX_MACRO_DEPTH + 1);
+    assert!(parse_macro(&nest(MAX_MACRO_DEPTH + 1)).unwrap_err().contains("4 deep"));
+    let mut empty = json!({"type": "repeat", "over": "Input", "body": []});
+    for _ in 1..MAX_MACRO_DEPTH {
+        empty = json!({"type": "if", "cell": "A1", "op": "eq", "value": 1, "then": [empty], "else": []});
+    }
+    let deep = parse_macro(&json!({"name": "Deep", "steps": [empty]})).unwrap();
+    assert_eq!(deep.step_count(), MAX_MACRO_DEPTH);
+    assert!(deep.check_limits().is_ok(), "the model's check agrees with the parser");
     // Far deeper than serde would allow is refused before it is read.
     assert!(parse_macro(&nest(200)).is_err());
     for bad in [
diff --git a/crates/model/src/macros.rs b/crates/model/src/macros.rs
index 8dc657f90..03cb03cac 100644
--- a/crates/model/src/macros.rs
+++ b/crates/model/src/macros.rs
@@ -81,10 +81,8 @@ impl Macro {
             return Err(format!("A macro description can be at most {MAX_MACRO_TEXT} characters long."));
         }
         // Count without recursing past the depth limit (a hostile file can nest deeper).
+        // A Repeat or If at the top is nested 1 deep; one inside it, 2 deep.
         fn walk(steps: &[Step], level: usize, count: &mut usize) -> Result<(), String> {
-            if level > MAX_MACRO_DEPTH {
-                return Err(format!("Repeat and If steps can nest at most {MAX_MACRO_DEPTH} deep."));
-            }
             for s in steps {
                 *count = count.saturating_add(1);
                 if *count > MAX_MACRO_STEPS {
@@ -93,6 +91,9 @@ impl Macro {
                 if s.note.chars().count() > MAX_MACRO_TEXT {
                     return Err(format!("A step note can be at most {MAX_MACRO_TEXT} characters long."));
                 }
+                if s.kind.children().next().is_some() && level > MAX_MACRO_DEPTH {
+                    return Err(format!("Repeat and If steps can nest at most {MAX_MACRO_DEPTH} deep."));
+                }
                 for body in s.kind.children() {
                     walk(body, level + 1, count)?;
                 }