aleleba/gridcraft
1
0
Fork 0
mirror of https://github.com/storytold/gridcraft.git synced 2026-11-02 16:30:50 -06:00
Code Issues Packages Projects Releases Wiki Activity

Engine: a deleted reference drops a Python cell's kept result; spills read past the scan budget stay values

Browse Source 
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.
This commit is contained in:
prxnvt
2026-11-02 02:27:28 -05:00
parent ad6aca08bf
commit e8f2becf4a
3 changed files with 76 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/engine/src/cmd/edit/paste_special.rs
+15 -5
View File
@@ -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);
}
}
}
}
crates/engine/src/tests/python_files.rs
+47
View File
@@ -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!("<c r=\"{at}\" cm=\"1\"><f t=\"array\" ref=\"{at}:{below}\">SEQUENCE(2)</f><v>1</v></c>")
})
.collect();
let rows: String = (2..=5_000u32).map(|r| format!("<row r=\"{r}\"><c r=\"A{r}\"><v>{r}</v></c></row>")).collect();
let data = format!(
"<sheetData><row r=\"1\"><c r=\"A1\" cm=\"1\"><f t=\"array\" ref=\"A1:A5000\">_xlfn._xlws.PY(0,0)</f><v>1</v></c>{arrays}</row>{rows}</sheetData>"
);
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;
crates/xlsx/src/sheet_read.rs
+14
View File
@@ -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<CellRef> = 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<Sheet, IoE
if st.dropped > 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;