From 198fe482a1d76d9349be88d4e748c3cbca0fe308 Mon Sep 17 00:00:00 2001 From: prxnvt <53838599+prxnvt@users.noreply.github.com> Date: Sun, 25 Oct 2026 03:19:59 -0400 Subject: [PATCH 1/2] Engine: the run's check of each step finds the inputs on their sheet after a sheet moved - The check for writes over data compared the inputs' sheet with the sheet's index when the run started, while the inputs follow their cells: after a step inserted a sheet in front, a step writing into its own input on the moved sheet was stopped as writing over data. It now uses the sheet's index now. Test: the input case in `inputs_and_the_output_follow_their_cells_when_a_step_moves_them`. - Docs: the page says `macro.run` checks the macro, its inputs and parameters and the policy before it runs, and that a formula whose references a move rewrites counts as changed too. --- crates/engine/src/macros/run.rs | 3 ++- crates/engine/src/tests/macros.rs | 13 ++++++++++++- docs/macros.md | 10 ++++++---- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/macros/run.rs b/crates/engine/src/macros/run.rs index 3d293e609..917deed74 100644 --- a/crates/engine/src/macros/run.rs +++ b/crates/engine/src/macros/run.rs @@ -508,7 +508,8 @@ impl Guard { if Arc::ptr_eq(ps, sa) { continue; } - let in_input = |c: CellRef| inputs.iter().any(|(_, p, _)| p.sheet == si && p.range.contains(c)); + // The inputs have followed their cells, so they are on the sheet's index now. + let in_input = |c: CellRef| inputs.iter().any(|(_, p, _)| p.sheet == ai && p.range.contains(c)); let mut hit: Vec = ps .cells .diff(&sa.cells) diff --git a/crates/engine/src/tests/macros.rs b/crates/engine/src/tests/macros.rs index 0df51122c..a4871f2f2 100644 --- a/crates/engine/src/tests/macros.rs +++ b/crates/engine/src/tests/macros.rs @@ -191,10 +191,21 @@ fn inputs_and_the_output_follow_their_cells_when_a_step_moves_them() { {"command": "home.insertSheet", "params": {"name": "Summary", "before": 0}}, {"command": "cell.set", "params": {"cell": "Input", "input": "x"}} ]); - put(&mut s, json!({"name": "Front", "inputs": [{"name": "Input", "kind": "cell"}], "output": {"onConflict": "overwrite"}, "steps": steps})); + put( + &mut s, + json!({"name": "Front", "inputs": [{"name": "Input", "kind": "cell"}], "output": {"onConflict": "overwrite"}, "steps": steps.clone()}), + ); s.execute("macro.run", json!({"name": "Front", "inputs": "Sheet1!B2"})).unwrap(); assert_eq!((on(&s, "Sheet1", "B2"), on(&s, "Summary", "B2")), ("x".into(), "".into())); + // Data in the input: the step writes into its own input, which isn't data written over, on + // the input's sheet wherever it moved. + let mut s = super::s(); + set(&mut s, "B2", "held"); + put(&mut s, json!({"name": "Front", "inputs": [{"name": "Input", "kind": "cell"}], "output": {"onConflict": "fail"}, "steps": steps})); + s.execute("macro.run", json!({"name": "Front", "inputs": "Sheet1!B2"})).unwrap(); + assert_eq!(on(&s, "Sheet1", "B2"), "x"); + // Rows inserted above: the input and the output move down with their cells. let mut s = super::s(); set(&mut s, "B5", "in"); diff --git a/docs/macros.md b/docs/macros.md index c0a09890f..d4f1cc43e 100644 --- a/docs/macros.md +++ b/docs/macros.md @@ -9,9 +9,10 @@ error (a file it already wrote stays), stops before it writes over data it may n touch files without permission. Agents run macros with `macro.run` like any other command. -This page describes the macro format, its tokens, and running macros. The engine has no check of a -macro before it runs, no macro debugger, recorder or management commands in this version, and no -macro interface. +This page describes the macro format, its tokens, and running macros. `macro.run` checks the +macro, its inputs, its parameters and the policy before it runs, and each step as it runs; this +version has no command that checks a macro without running it, no macro debugger, recorder or +management commands, and no macro interface. ## A macro @@ -115,7 +116,8 @@ refused rather than read as the header. step that writes over cells outside the inputs that held data when the run started, or deletes sheets or objects that were there, stops the run, which rolls the workbook back (a file already written stays). Moving such cells, as inserting or deleting rows or cells above them does, - counts as writing over them: the check compares cells by place. The rule is `onConflict` (or + counts as writing over them, and so does a formula whose references such a move rewrites: the + check compares cells by place, so it may ask about data that only moved, never the other way. The rule is `onConflict` (or the macro's `output.onConflict`): `ask` (the call fails with `confirm: …`; `overwrite: true` answers it), `fail` or `overwrite`. Shifting the data to make room (`shiftDown`, `shiftRight`) needs to know where the macro writes before it runs, which this version doesn't 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 2/2] 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 { 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> { + 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 { let d = s.doc()?; @@ -1549,7 +1570,10 @@ pub fn check(s: &mut Session, name: &str, p: &Json) -> Result { 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.