From bf456c9ff8465b0c9b65ea921b19316b49d73dc2 Mon Sep 17 00:00:00 2001 From: prxnvt <53838599+prxnvt@users.noreply.github.com> Date: Wed, 21 Oct 2026 15:25:34 -0400 Subject: [PATCH] Engine: a macro's footprint dry runs keep to the run's timeLimit footprint always ran its dry run with the default 60 second limit, so macro.run with timeLimit 5 could run for over two minutes (a dry run, then another after a shift) and fail with "ran longer than 60 seconds". footprint now takes the run's clock and limit. make_way starts the run's clock before the dry runs, so the checks and the run share one limit, and macro.check takes timeLimit too. --- crates/engine/src/cmd/macros.rs | 2 +- crates/engine/src/macros/run.rs | 25 ++++++++++++++++--------- crates/engine/src/tests/macros.rs | 11 +++++++++++ docs/macros.md | 8 +++++--- 4 files changed, 33 insertions(+), 13 deletions(-) diff --git a/crates/engine/src/cmd/macros.rs b/crates/engine/src/cmd/macros.rs index 2aba053b0..53856e720 100644 --- a/crates/engine/src/cmd/macros.rs +++ b/crates/engine/src/cmd/macros.rs @@ -21,7 +21,7 @@ pub fn specs() -> Vec { "Check Macro", [], None, - "{name, inputs?} validates the inputs and dry-runs the macro on the workbook (then puts everything back) → {ok, errors?, needsApproval?, inputs, output?, conflicts, footprint}", + "{name, inputs?, timeLimit?: seconds} validates the inputs and dry-runs the macro on the workbook (then puts everything back) → {ok, errors?, needsApproval?, inputs, output?, conflicts, footprint}", has_doc, check ), diff --git a/crates/engine/src/macros/run.rs b/crates/engine/src/macros/run.rs index 1f4f39f74..57a70562e 100644 --- a/crates/engine/src/macros/run.rs +++ b/crates/engine/src/macros/run.rs @@ -346,13 +346,13 @@ fn holds(v: &Value, wanted: gridcraft_model::ColType) -> &'static str { /// A stopwatch for the time limit. `Instant::now` traps on wasm, so there it measures nothing. #[derive(Clone, Copy, Debug)] -struct Stopwatch { +pub struct Stopwatch { #[cfg(not(target_arch = "wasm32"))] start: std::time::Instant, } impl Stopwatch { - fn start() -> Stopwatch { + pub fn start() -> Stopwatch { #[cfg(not(target_arch = "wasm32"))] #[allow(clippy::disallowed_methods)] // never on wasm (see above) let w = Stopwatch { start: std::time::Instant::now() }; @@ -907,8 +907,8 @@ impl Saved { } /// Runs the macro in dry-run mode on its workbook, puts everything back, and says what it would -/// change. -pub fn footprint(s: &mut Session, p: &Prepared) -> Result { +/// change. The dry run stops once `time_limit_ms` has passed on `clock`. +pub fn footprint(s: &mut Session, p: &Prepared, clock: Stopwatch, time_limit_ms: f64) -> Result { let i = s.docs.iter().position(|d| d.uid == p.doc).ok_or(EngineError::NoDocument)?; s.set_active(i); let saved = Saved::take(s, p.doc); @@ -918,6 +918,7 @@ pub fn footprint(s: &mut Session, p: &Prepared) -> Result { d.touched = Some(Vec::new()); } let mut m = Machine::new(p, true); + (m.clock, m.time_limit_ms) = (clock, time_limit_ms); let outcome = m.run_to_end(s); let (after, touched) = match s.docs.iter_mut().find(|d| d.uid == p.doc) { Some(d) => (d.wb.clone(), d.touched.take().unwrap_or_default()), @@ -1107,14 +1108,15 @@ pub fn begin(s: &mut Session, name: &str, p: &Json) -> Result { approve(s, &m, doc, &asks, p.get("approve").and_then(Json::as_bool).unwrap_or(false))?; let prepared = prepare(s, &m, p.get("inputs"))?; let rule = conflict_rule(p, &m)?; - let time_limit_ms = p.get("timeLimit").and_then(Json::as_f64).map(|t| (t * 1000.0).clamp(1.0, 600_000.0)).unwrap_or(DEFAULT_TIME_LIMIT_MS); + let time_limit_ms = time_limit(p); let inputs = prepared.inputs_json(&s.doc()?.wb); Ok(Begun { mac: m, prepared, rule, time_limit_ms, inputs }) } /// The footprint check and the output rule, inside the run's undo group: an error when the macro /// would fail or write over data it may not, after shifting the data if the rule says so. Returns -/// the machine for the run and whether it shifted. +/// the machine for the run and whether it shifted. The dry runs count towards the run's time +/// limit. /// /// A partial footprint proves nothing about the steps it didn't reach, so then the machine /// checks each step as it runs and stops the run at one that writes over data. @@ -1125,7 +1127,7 @@ pub fn make_way(s: &mut Session, b: &Begun) -> Result<(Machine, bool)> { if rule == OnConflict::Overwrite { return Ok((machine, false)); } - let mut fp = footprint(s, &b.prepared)?; + let mut fp = footprint(s, &b.prepared, machine.clock, b.time_limit_ms)?; if let Some(e) = fp.error.take() { return Err(e.into_engine(&m.name)); } @@ -1144,7 +1146,7 @@ pub fn make_way(s: &mut Session, b: &Begun) -> Result<(Machine, bool)> { } OnConflict::ShiftDown | OnConflict::ShiftRight => { make_room(s, &fp, rule == OnConflict::ShiftDown)?; - fp = footprint(s, &b.prepared)?; + fp = footprint(s, &b.prepared, machine.clock, b.time_limit_ms)?; if let Some(e) = fp.error.take() { return Err(e.into_engine(&m.name)); } @@ -1191,6 +1193,11 @@ pub fn run(s: &mut Session, name: &str, p: &Json) -> Result { }) } +/// The run's `timeLimit` (seconds) in milliseconds, or the default. +fn time_limit(p: &Json) -> f64 { + p.get("timeLimit").and_then(Json::as_f64).map(|t| (t * 1000.0).clamp(1.0, 600_000.0)).unwrap_or(DEFAULT_TIME_LIMIT_MS) +} + /// The conflict rule for this run: `onConflict`, or `overwrite: true` (the answer to the /// question), else the macro's own. fn conflict_rule(p: &Json, m: &Macro) -> Result { @@ -1240,7 +1247,7 @@ pub fn check(s: &mut Session, name: &str, p: &Json) -> Result { if !asks.is_empty() && !approved { return Ok(out); } - let fp = footprint(s, &prepared)?; + let fp = footprint(s, &prepared, Stopwatch::start(), time_limit(p))?; out["ok"] = json!(fp.error.is_none()); out["conflicts"] = json!(fp.conflict_count()); if let Some(e) = &fp.error { diff --git a/crates/engine/src/tests/macros.rs b/crates/engine/src/tests/macros.rs index 5f483e31d..2855f11e6 100644 --- a/crates/engine/src/tests/macros.rs +++ b/crates/engine/src/tests/macros.rs @@ -356,6 +356,17 @@ fn macros_are_bounded() { assert!(err.contains("ran more than 100000 steps"), "{err}"); let err = s.execute("macro.run", json!({"name": "Spin", "timeLimit": 0.001})).unwrap_err().to_string(); assert!(err.contains("ran longer than"), "{err}"); + // The footprint's dry run keeps to the time limit too, and so does the check's. + put( + &mut s, + json!({"name": "Spin", "output": {"onConflict": "fail"}, "steps": [{"type": "repeat", "over": "A1:A400", "body": [ + {"type": "repeat", "over": "A1:A400", "body": [{"type": "if", "cell": "A1", "op": "empty"}]} + ]}]}), + ); + let err = s.execute("macro.run", json!({"name": "Spin", "timeLimit": 0.001})).unwrap_err().to_string(); + assert!(err.contains("ran longer than"), "{err}"); + let c = s.execute("macro.check", json!({"name": "Spin", "timeLimit": 0.001})).unwrap(); + assert!(c["errors"][0].as_str().unwrap().contains("ran longer than"), "{c}"); } #[test] diff --git a/docs/macros.md b/docs/macros.md index 2b17c6a50..1e1b873fb 100644 --- a/docs/macros.md +++ b/docs/macros.md @@ -42,7 +42,9 @@ no macro debugger, recorder or management commands in this version, and no macro `nonEmpty` and `defaultRef`. A run binds each input to the range it is given, its default, or (for the first input) the selection, and refuses with messages that name the cells: "Input column 2 (Amount) must hold numbers; C4 holds text." -- **Limits**: 1,000 macros a workbook, 10,000 steps a macro, and Repeat and If nested 4 deep. +- **Limits**: 1,000 macros a workbook, 10,000 steps a macro, Repeat and If nested 4 deep, + 100,000 steps executed per run, and 60 seconds a run, footprint checks included (`timeLimit` + to change; the web build has no clock, so there only the step limit applies). ## Tokens @@ -94,8 +96,8 @@ called Output, even when a token or input has that name. step: `Step 3.1 (cell.set): …` (paths are 1-based; `2.1` is inside a Repeat, `4.then.1` inside an If). -`macro.check {name, inputs?}` reports the bound inputs, the output anchor, any approval needed and -the footprint, without changing anything. +`macro.check {name, inputs?, timeLimit?}` reports the bound inputs, the output anchor, any +approval needed and the footprint, without changing anything. ## Storage