From dadffdb33f2b9fc212cf4b55996c9a38815405e5 Mon Sep 17 00:00:00 2001
From: prxnvt <53838599+prxnvt@users.noreply.github.com>
Date: Sun, 25 Oct 2026 03:21:15 -0400
Subject: [PATCH] Engine: macro.check shifts and checks again, as the run does,
 before it says ok

Under `shiftDown` or `shiftRight` with conflicts, `macro.check` looked only for the refusals made
before a shift, so it said `ok` for a run the second footprint stops (an anchor that moves with
the shifted data). It now shifts the data inside an undo group, takes the footprint again, rolls
both back and puts the session back, and reports what still stands in the way.

- Test: the `Named` case and a shift that makes room in `shifting_makes_room_and_is_checked_again`.
---
 crates/engine/src/macros/run.rs   | 26 +++++++++++++++++++++++++-
 crates/engine/src/tests/macros.rs | 13 +++++++++++++
 docs/macros.md                    |  3 ++-
 3 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/crates/engine/src/macros/run.rs b/crates/engine/src/macros/run.rs
index bff4e0b90..6a6d3f5c8 100644
--- a/crates/engine/src/macros/run.rs
+++ b/crates/engine/src/macros/run.rs
@@ -1488,6 +1488,27 @@ fn conflict_rule(p: &Json, m: &Macro) -> Result<OnConflict> {
     Ok(rule)
 }
 
+/// For `macro.check`, as the run would: shifts the data, checks the footprint again, and puts
+/// everything back. What still stands in the way, or `None`.
+fn shifted_conflicts(s: &mut Session, fp: &Footprint, p: &Prepared, budget: &Machine, down: bool) -> Result<Option<String>> {
+    let saved = Saved::take(s, p.doc);
+    let token = s.group_begin("Check Macro")?;
+    let mut spent = budget.clone();
+    (spent.executed, spent.param_bytes) = (fp.executed, fp.param_bytes);
+    let again = make_room(s, fp, p, down).and_then(|()| footprint(s, p, &spent));
+    let rolled = s.group_rollback(token);
+    saved.restore(s, p.doc);
+    rolled?;
+    Ok(match again {
+        Err(e) => Some(e.to_string()),
+        Ok(again) => match (&again.error, again.conflict_count()) {
+            (Some(e), _) => Some(e.describe()),
+            (None, 0) => None,
+            (None, _) => Some(format!("Shifting the data doesn't make room ({}).", again.describe_conflicts())),
+        },
+    })
+}
+
 /// `macro.check`: what running the macro would do, without changing anything.
 pub fn check(s: &mut Session, name: &str, p: &Json) -> Result<Json> {
     let d = s.doc()?;
@@ -1549,7 +1570,10 @@ pub fn check(s: &mut Session, name: &str, p: &Json) -> Result<Json> {
             OnConflict::Fail => problems.push(format!("It would write over cells that hold data ({what}).")),
             OnConflict::Overwrite => {}
             OnConflict::ShiftDown | OnConflict::ShiftRight => {
-                if let Some(why) = shift_refusal(&s.doc()?.wb, &fp, &prepared, rule == OnConflict::ShiftDown) {
+                let down = rule == OnConflict::ShiftDown;
+                if let Some(why) = shift_refusal(&s.doc()?.wb, &fp, &prepared, down) {
+                    problems.push(why);
+                } else if let Some(why) = shifted_conflicts(s, &fp, &prepared, &budget, down)? {
                     problems.push(why);
                 }
             }
diff --git a/crates/engine/src/tests/macros.rs b/crates/engine/src/tests/macros.rs
index 57307ee8c..50fbcfc14 100644
--- a/crates/engine/src/tests/macros.rs
+++ b/crates/engine/src/tests/macros.rs
@@ -304,6 +304,19 @@ fn shifting_makes_room_and_is_checked_again() {
     let err = s.execute("macro.run", json!({"name": "Named"})).unwrap_err().to_string();
     assert!(err.contains("shifting the data didn't make room (E3 on Sheet1)"), "{err}");
     assert!(Arc::ptr_eq(&s.doc().unwrap().wb, &start));
+    // The check shifts and checks again as the run does, and puts everything back.
+    let journal = s.journal.len();
+    let c = s.execute("macro.check", json!({"name": "Named"})).unwrap();
+    assert_eq!((&c["ok"], &c["errors"]), (&json!(false), &json!(["Shifting the data doesn't make room (E3 on Sheet1)."])), "{c}");
+    assert!(Arc::ptr_eq(&s.doc().unwrap().wb, &start));
+    assert_eq!(s.journal.len(), journal);
+    // Where the shift does make room, the check says the run would go ahead.
+    let mut s = super::s();
+    data(&mut s);
+    set(&mut s, "E2", "old 1");
+    put(&mut s, totals());
+    let c = s.execute("macro.check", json!({"name": "Totals", "inputs": "B2:C5", "onConflict": "shiftDown"})).unwrap();
+    assert_eq!((&c["ok"], c.get("errors")), (&json!(true), None), "{c}");
 }
 
 #[test]
diff --git a/docs/macros.md b/docs/macros.md
index fe169fb3b..4286fb487 100644
--- a/docs/macros.md
+++ b/docs/macros.md
@@ -143,7 +143,8 @@ refused rather than read as the header.
 output anchor, any approval needed and the footprint, and puts back everything the dry run
 changed. `ok` says whether `macro.run` with the same parameters would go ahead without a question;
 when it wouldn't, `errors` says why: the run would fail at a step, a Stop step rolls it back, it
-would write over data under `ask` or `fail`, shifting can't make room, or the run would go past
+would write over data under `ask` or `fail`, shifting can't make room (the check shifts and
+checks again, then puts everything back), or the run would go past
 the step or parameter limit counting the dry runs before it. The random numbers RAND gives move
 on, as after any recalculation.