From ad6aca08bf225dd3bf2df45776b4c8ea29ba7a58 Mon Sep 17 00:00:00 2001 From: prxnvt <53838599+prxnvt@users.noreply.github.com> Date: Mon, 2 Nov 2026 02:02:07 -0500 Subject: [PATCH] XLSX: a Python spill is recorded at its anchor, and one too large keeps its values A Python cell's spill range was recorded only when the file held a record for one of its other cells, so a result whose other cells are blank, which a file may leave out, opened with no spill and no rebuilt result. The range is now recorded when the anchor is read, if a result can fill it. A Python range larger than a result may be (more than 1,000,000 cells, but within what the reader tracks) emptied the cells it covered, although it is no spill and file-format-parity.md says such cells keep the values the file shows. They are now kept as plain values; a formula's spilled values are still dropped, since recalculation fills them again. Tests: a_python_spill_whose_blank_cells_have_no_records_is_still_a_spill and a_python_spill_range_larger_than_a_result_keeps_its_values_as_plain_values; both fail without the fix. --- crates/engine/src/tests/python_files.rs | 36 +++++++++++++++++++++++++ crates/xlsx/src/sheet_read.rs | 29 +++++++++++++++++--- 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/tests/python_files.rs b/crates/engine/src/tests/python_files.rs index e04d6a7da..f2b68feb2 100644 --- a/crates/engine/src/tests/python_files.rs +++ b/crates/engine/src/tests/python_files.rs @@ -317,6 +317,42 @@ fn a_python_cell_without_its_code_keeps_its_whole_cached_result_as_values() { } } +#[test] +fn a_python_spill_whose_blank_cells_have_no_records_is_still_a_spill() { + // A1's result spills over A1:A3, and the file holds no records for the blank A2 and A3. + let data = "_xlfn._xlws.PY(0,0)1"; + let mut s = s(); + open(&mut s, "sparse.xlsx", &excel_shaped(data, Some(&["[1, None, None]"]))); + let sh = s.doc().unwrap().wb.active().unwrap(); + let a1 = CellRef::new(0, 0); + assert_eq!(sh.spill_ranges.get(&a1), Some(&RangeRef::parse("A1:A3").unwrap()), "the range is recorded at its anchor"); + assert_eq!(sh.spill_anchor(CellRef::parse("A3").unwrap()), Some(a1)); + let kept = sh.cell(a1).and_then(|c| c.py.as_ref()).and_then(|p| p.output.as_ref()).map(|o| o.value.clone()); + assert_eq!(kept, Some(array(3, 1, vec![1.0.into(), Value::Empty, Value::Empty])), "the result is rebuilt over the whole range"); + // A formula entered in the spill takes the area, as with any spill. + s.execute("cell.set", json!({"cell": "A3", "input": "7"})).unwrap(); + assert_eq!(v(&s, "A1"), Value::Error(CellError::Spill)); +} + +#[test] +fn a_python_spill_range_larger_than_a_result_keeps_its_values_as_plain_values() { + // C1:D600000 is 1,200,000 cells, more than a result may have but within what the reader + // tracks; its cached values are kept as the file shows them, and nothing spills. + let (mut s, _) = build(); + let bytes = save(&mut s, "xlsx"); + let crafted = with_sheet_xml(&bytes, |xml| xml.replace("ref=\"C1:D2\"", "ref=\"C1:D600000\"")); + assert_ne!(crafted, bytes); + open(&mut s, "crafted.xlsx", &crafted); + let sh = s.doc().unwrap().wb.active().unwrap(); + assert!(!sh.spill_ranges.contains_key(&CellRef::new(0, 2)), "{:?}", sh.spill_ranges); + assert_eq!( + (v(&s, "C1"), v(&s, "D1"), v(&s, "C2"), v(&s, "D2")), + (Value::Number(1.0), Value::text("two"), Value::Number(3.0), Value::text("four")) + ); + let d2 = sh.cell(CellRef::parse("D2").unwrap()).expect("D2 is stored"); + assert!(d2.py.is_none() && d2.formula.is_none(), "a plain value"); +} + #[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 0dae14344..487dd5b26 100644 --- a/crates/xlsx/src/sheet_read.rs +++ b/crates/xlsx/src/sheet_read.rs @@ -93,6 +93,20 @@ fn forget_dynamic(st: &mut RowState, pos: CellRef) { } } +/// Records the spill range of the Python cell read at `pos`, its anchor, when the file declares +/// one that a result can fill: the spill then exists even when the file holds no records for its +/// blank cells. Charged like [`dynamic_range`]'s scans. +fn record_python_spill(st: &mut RowState, sheet: &mut Sheet, pos: CellRef) { + if st.dynamic_checks > MAX_DYNAMIC_CHECKS { + return; + } + st.dynamic_checks = st.dynamic_checks.saturating_add(st.dynamic.len() as u64); + let Some(r) = st.dynamic.iter().rev().find(|r| r.start == pos).copied() else { return }; + if r.count() > 1 && r.count() <= gridcraft_model::python::MAX_RESULT_CELLS as u64 { + sheet.spill_ranges.insert(pos, r); + } +} + 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. @@ -557,18 +571,25 @@ fn read_cell(cx: &mut Ctx<'_>, sheet: &mut Sheet, st: &mut RowState, c: &El, pos (None, None) } }; + if py.is_some() { + record_python_spill(st, sheet, pos); + } let spilled_from = if formula.is_none() && py.is_none() { dynamic_range(st, pos) } else { None }; + let python_anchor = spilled_from.is_some_and(|r| sheet.cells.get(r.start).is_some_and(|a| a.py.is_some())); let value = match spilled_from { - // A Python cell's spilled values are kept: they are its result until it runs again. A - // range larger than a result may be is a crafted file's: its values are dropped, like a - // formula's spilled values, and no spill is recorded. - Some(r) if r.count() <= gridcraft_model::python::MAX_RESULT_CELLS as u64 && sheet.cells.get(r.start).is_some_and(|a| a.py.is_some()) => { + // A Python cell's spilled values are kept: they are its result until it runs again. + Some(r) if python_anchor && r.count() <= gridcraft_model::python::MAX_RESULT_CELLS as u64 => { if !value.is_empty() { sheet.spill.insert(pos, value); } sheet.spill_ranges.insert(r.start, r); Value::Empty } + // A Python cell's range larger than a result may be is a crafted file's: it is no spill, + // and its cells keep the values the file shows as plain values, as when the cell keeps + // only its own value. + Some(_) if python_anchor => value, + // A formula's spilled values are dropped: recalculation fills them again. Some(_) => Value::Empty, None => value, };