From e8f2becf4a2009bde6428754d2c16de9053096ea Mon Sep 17 00:00:00 2001
From: prxnvt <53838599+prxnvt@users.noreply.github.com>
Date: Mon, 2 Nov 2026 02:26:24 -0500
Subject: [PATCH] Engine: a deleted reference drops a Python cell's kept
result; spills read past the scan budget stay values
A structural edit that deleted the data an xl() reference names rewrote the
reference to #REF! but kept the cell's cached result, so the cell went on
showing a result for data that is gone. rewrite_python_refs now makes such a
cell new code (#BLOCKED!, no kept result), and the recalculation removes its
spill. A rename or a move, after which the reference names the same data,
still keeps the result.
When the reader's range-scan budget ran out part way through a Python spill,
the rest of its cells were read as plain cells, and the result rebuilt on open
was then blocked by them (#SPILL!). On such a sheet every Python spill now
keeps its values as plain values, with no live spill.
Tests: deleting_the_data_python_code_reads_drops_its_kept_result and
python_spills_read_past_the_scan_budget_keep_their_values_as_plain_values;
both fail without the fix.
---
crates/engine/src/cmd/edit/paste_special.rs | 20 ++++++---
crates/engine/src/tests/python_files.rs | 47 +++++++++++++++++++++
crates/xlsx/src/sheet_read.rs | 14 ++++++
3 files changed, 76 insertions(+), 5 deletions(-)
diff --git a/crates/engine/src/cmd/edit/paste_special.rs b/crates/engine/src/cmd/edit/paste_special.rs
index ff1adfc29..b886dfc19 100644
--- a/crates/engine/src/cmd/edit/paste_special.rs
+++ b/crates/engine/src/cmd/edit/paste_special.rs
@@ -397,7 +397,10 @@ pub(crate) fn rewrite_all_formulas(wb: &mut gridcraft_model::Workbook, target: &
/// Rewrites the `xl("…")` literals in every Python cell with `f`, which maps a reference held on
/// sheet `host` to its new form. Every reference-moving edit (insert and delete of lines or cells,
/// cut-move, sheet rename and delete) runs it, as it adjusts formulas. A literal is only replaced
-/// when the reference it names changed, and text that doesn't parse is kept. A rewrite that would
+/// when the reference it names changed, and text that doesn't parse is kept. A cell's cached
+/// result is kept while its references still name the same data (a rename, a move); a reference
+/// the edit deleted (`#REF!`) leaves the result stale, so the cell is new code (`#BLOCKED!`) and
+/// the recalculation removes its spill. A rewrite that would
/// make a cell's code longer than one cell holds (a longer sheet name, `Z1` becoming `AA1`) refuses
/// the whole edit, which then changes nothing: see [`check_python_fits`].
pub(crate) fn rewrite_python_refs(
@@ -407,29 +410,36 @@ pub(crate) fn rewrite_python_refs(
for si in 0..wb.sheets.len() {
let Some(sh) = wb.sheets.get(si) else { continue };
let host = sh.name.clone();
- let mut new_code: Vec<(CellRef, String)> = Vec::new();
+ let mut new_code: Vec<(CellRef, String, bool)> = Vec::new();
for (c, cell) in sh.cells.iter() {
let Some(py) = &cell.py else { continue };
+ let mut lost = false;
let changed = gridcraft_model::python::rewrite_xl(&py.code, |reference| {
let expr = gridcraft_formula::parse(reference).ok()?;
let before = gridcraft_formula::print(&expr);
let after = gridcraft_formula::print(&f(&host, expr));
+ lost |= after.contains("#REF!") && !before.contains("#REF!");
(after != before).then_some(after)
});
if let Some(code) = changed {
check_python_fits(&gridcraft_model::PyCode { code: code.clone(), object: py.object, ..Default::default() }, &host, c)?;
- new_code.push((c, code));
+ new_code.push((c, code, lost));
}
}
if new_code.is_empty() {
continue;
}
let Some(sh) = wb.sheet_mut(si) else { continue };
- for (c, code) in new_code {
+ for (c, code, lost) in new_code {
if let Some(cell) = sh.cells.get_mut(c)
&& let Some(py) = cell.py.as_mut()
{
- std::sync::Arc::make_mut(py).code = code;
+ let py = std::sync::Arc::make_mut(py);
+ py.code = code;
+ if lost {
+ py.output = None;
+ cell.value = Value::Error(gridcraft_core::CellError::Blocked);
+ }
}
}
}
diff --git a/crates/engine/src/tests/python_files.rs b/crates/engine/src/tests/python_files.rs
index f2b68feb2..a88369d0e 100644
--- a/crates/engine/src/tests/python_files.rs
+++ b/crates/engine/src/tests/python_files.rs
@@ -353,6 +353,53 @@ fn a_python_spill_range_larger_than_a_result_keeps_its_values_as_plain_values()
assert!(d2.py.is_none() && d2.formula.is_none(), "a plain value");
}
+#[test]
+fn python_spills_read_past_the_scan_budget_keep_their_values_as_plain_values() {
+ // A1's result spills over A1:A5000. Four thousand ordinary dynamic arrays on row 1 make each
+ // later plain cell's range lookup cost 4,001 checks, so the reader's scan budget runs out
+ // part way down column A, and the rest of the spill is read as plain cells.
+ let arrays: String = (1..=4_000u32)
+ .map(|c| {
+ let at = CellRef::new(0, c).a1();
+ let below = CellRef::new(1, c).a1();
+ format!("SEQUENCE(2)1")
+ })
+ .collect();
+ let rows: String = (2..=5_000u32).map(|r| format!("{r}
")).collect();
+ let data = format!(
+ "_xlfn._xlws.PY(0,0)1{arrays}
{rows}"
+ );
+ let mut s = s();
+ open(&mut s, "budget.xlsx", &excel_shaped(&data, Some(&["list(range(1, 5001))"])));
+ let sh = s.doc().unwrap().wb.active().unwrap();
+ assert!(!sh.spill_ranges.contains_key(&CellRef::new(0, 0)), "no live spill");
+ assert!(code(&s, "A1").is_some_and(|p| p.output.is_none()), "no result is rebuilt");
+ assert_eq!((v(&s, "A2"), v(&s, "A5000")), (Value::Number(2.0), Value::Number(5000.0)), "the values stay");
+ s.execute("formulas.calculateNow", json!({})).unwrap();
+ assert_eq!(v(&s, "A1"), Value::Number(1.0), "the cell keeps its value, not #SPILL!");
+}
+
+#[test]
+fn deleting_the_data_python_code_reads_drops_its_kept_result() {
+ let mut s = s();
+ s.execute("cell.set", json!({"cell": "A1", "input": "1"})).unwrap();
+ s.execute("cell.set", json!({"cell": "A2", "input": "2"})).unwrap();
+ py(&mut s, "C5", "xl(\"A1:A2\") * 2");
+ cache(&mut s, "C5", array(2, 1, vec![2.0.into(), 4.0.into()]), "list");
+ s.execute("formulas.calculateNow", json!({})).unwrap();
+ assert_eq!(v(&s, "C6"), Value::Number(4.0));
+ // A move keeps the result: the reference still names the same data.
+ s.execute("home.insertRows", json!({"rows": "1"})).unwrap();
+ assert!(code(&s, "C6").is_some_and(|p| p.output.is_some() && !p.code.contains("A1:A2")), "{:?}", code(&s, "C6"));
+ assert_eq!(v(&s, "C7"), Value::Number(4.0));
+ // Deleting the rows it reads leaves `#REF!`: the kept result no longer belongs to the code.
+ s.execute("home.deleteRows", json!({"rows": "2:3"})).unwrap();
+ let c4 = code(&s, "C4").expect("the Python cell moved up to C4");
+ assert!(c4.code.contains("#REF!") && c4.output.is_none(), "{c4:?}");
+ assert_eq!(v(&s, "C4"), Value::Error(CellError::Blocked));
+ assert_eq!(v(&s, "C5"), Value::Empty, "its spill is gone");
+}
+
#[test]
fn a_script_is_measured_after_its_escapes_are_decoded() {
let max = gridcraft_model::python::MAX_FORMULA_CHARS;
diff --git a/crates/xlsx/src/sheet_read.rs b/crates/xlsx/src/sheet_read.rs
index 487dd5b26..73c98386a 100644
--- a/crates/xlsx/src/sheet_read.rs
+++ b/crates/xlsx/src/sheet_read.rs
@@ -107,6 +107,19 @@ fn record_python_spill(st: &mut RowState, sheet: &mut Sheet, pos: CellRef) {
}
}
+/// Past the scan budget, the cells of a Python spill read afterwards were kept as plain cells, not
+/// as its spilled values, and a result rebuilt over them would block itself. Every Python spill of
+/// such a sheet keeps its values as plain values instead, with no live spill.
+fn settle_python_spills_past_budget(st: &RowState, sheet: &mut Sheet) {
+ if st.dynamic_checks <= MAX_DYNAMIC_CHECKS {
+ return;
+ }
+ let anchors: Vec = sheet.spill_ranges.keys().copied().filter(|a| sheet.cells.get(*a).is_some_and(|c| c.py.is_some())).collect();
+ for anchor in anchors {
+ sheet.settle_spill(anchor);
+ }
+}
+
const MAX_HYPERLINK_CELLS: u64 = 10_000;
/// Target and tooltip bytes kept for hyperlinks of one sheet, over all cells: the same URL is
/// copied for every cell of its range, so length × cells must stay bounded whatever the file.
@@ -163,6 +176,7 @@ pub fn read_sheet(cx: &mut Ctx<'_>, part: &str, name: &str) -> Result 0 {
cx.warn(format!("sheet \"{name}\": {} cells or rows outside the 1,048,576 × 16,384 grid were dropped", st.dropped));
}
+ settle_python_spills_past_budget(&st, &mut sheet);
drop(bytes);
let mut fit_to_page = false;