diff --git a/AGENTS.md b/AGENTS.md index fdb7c49..963fc6d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -26,7 +26,7 @@ People trust GridCraft with their numbers; a crash loses their work. **This outr - **Input-derived numbers are hostile** (files, formulas, commands, MCP/control params): `get()` instead of indexing, checked/saturating arithmetic, cap input-sized allocations (huge ranges, `REPT`, `SEQUENCE`, whole-column references). - **Bound recursion** (formula nesting, dependency chains, names referring to names). - **Last-resort guard:** `Session::execute` catches escaped panics and restores the workbook. It's a safety net, not a licence. -- **Prove it:** every crash fix lands with a regression test (`tests::every_command_survives_empty_params` fuzzes every command). +- **Prove it:** every crash fix lands with a regression test (`tests::every_command_survives_empty_params` fuzzes every command; `crates/engine/tests/command_sequence_fuzz.rs` fuzzes random command sequences against undo/redo, XLSX round-trip and full-recalc invariants; soak it with `GRIDCRAFT_FUZZ_SEEDS`/`GRIDCRAFT_FUZZ_STEPS`, replay with `GRIDCRAFT_FUZZ_SEED`). - Crates carry `#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]`. ## Non-negotiables diff --git a/CLAUDE.md b/CLAUDE.md index 4dc333b..280fe8b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -26,7 +26,7 @@ People trust GridCraft with their numbers; a crash loses their work. **This outr - **Input-derived numbers are hostile** (files, formulas, commands, MCP/control params): `get()` instead of indexing, checked/saturating arithmetic, cap input-sized allocations (huge ranges, `REPT`, `SEQUENCE`, whole-column references). - **Bound recursion** (formula nesting, dependency chains, names referring to names). - **Last-resort guard:** `Session::execute` catches escaped panics and restores the workbook. It's a safety net, not a licence. -- **Prove it:** every crash fix lands with a regression test (`tests::every_command_survives_empty_params` fuzzes every command). +- **Prove it:** every crash fix lands with a regression test (`tests::every_command_survives_empty_params` fuzzes every command; `crates/engine/tests/command_sequence_fuzz.rs` fuzzes random command sequences against undo/redo, XLSX round-trip and full-recalc invariants; soak it with `GRIDCRAFT_FUZZ_SEEDS`/`GRIDCRAFT_FUZZ_STEPS`, replay with `GRIDCRAFT_FUZZ_SEED`). - Crates carry `#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]`. ## Non-negotiables diff --git a/ROADMAP.md b/ROADMAP.md index 34fcf12..0748614 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -132,6 +132,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | What landed | |---|---| +| 2026-10-11 | Command-sequence fuzzing: random command sequences checked against undo/redo, XLSX round-trip and full recalculation; fixes for the table, legacy-array, spill, AutoSum, Watch Window and sheet-copy bugs it found | | 2026-10-11 | Text boxes keep their formatting: run size, bold, italic, typeface, text colour and outline, alignment, vertical anchor and "no outline" (#377) | | 2026-10-11 | Desktop printing opens the print PDF in the system viewer; Save As offers CSV, web page and PDF; macOS File › Properties opens File › Info (#370, #343) | | 2026-10-11 | Windows: DirectX 12 shaders compile with FXC, so a missing or stray `dxcompiler.dll` no longer stops the app at start (#339, #354) | diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 91a81b3..8eccb10 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -210,10 +210,17 @@ fn cell_set(s: &mut Session, p: &Json) -> Result { { return Err(EngineError::Other(m)); } + let active_sheet = s.doc()?.wb.active_sheet; edit(s, |cx| { if array { - // Legacy Ctrl+Shift+Enter array formula over the selection. - let range = cx.sel.current(); + // Legacy Ctrl+Shift+Enter array formula over the selection, when the selection holds + // the cell; a cell elsewhere (or on another sheet) gets a one-cell array. + let selected = cx.sel.current(); + let range = if sheet == active_sheet && selected.contains(at) { selected } else { RangeRef::cell(at) }; + // A table header can't hold an array's value, and a table resizes under it. + if !range.is_single() && cx.wb.sheet(sheet).is_some_and(|sh| sh.tables.iter().any(|t| t.range.intersects(&range))) { + return Err(EngineError::Other("Multi-cell array formulas aren't allowed in tables.".into())); + } let mut cell = input_to_cell(&input, sheet, range.start, &mut cx.wb)?.unwrap_or_default(); if let Some(f) = cell.formula.as_mut() { std::sync::Arc::make_mut(f).array = Some(range); @@ -578,6 +585,7 @@ fn undo(s: &mut Session, p: &Json) -> Result { let steps = u32_param(p, "steps").unwrap_or(1).max(1); let d = s.doc_mut()?; let mut label = String::new(); + let now = d.wb.clone(); for _ in 0..steps { let Some(e) = d.undo.pop() else { break }; let cur = crate::HistoryEntry { label: e.label.clone(), wb: d.wb.clone(), selection: d.selection.clone() }; @@ -586,6 +594,7 @@ fn undo(s: &mut Session, p: &Json) -> Result { d.wb = e.wb; d.selection = e.selection; } + super::extra::keep_watch_list(&now, &mut d.wb); d.calc.rebuild(&d.wb); d.revision += 1; Ok(json!({"undone": label})) @@ -595,6 +604,7 @@ fn redo(s: &mut Session, p: &Json) -> Result { let steps = u32_param(p, "steps").unwrap_or(1).max(1); let d = s.doc_mut()?; let mut label = String::new(); + let now = d.wb.clone(); for _ in 0..steps { let Some(e) = d.redo.pop() else { break }; let cur = crate::HistoryEntry { label: e.label.clone(), wb: d.wb.clone(), selection: d.selection.clone() }; @@ -603,6 +613,7 @@ fn redo(s: &mut Session, p: &Json) -> Result { d.wb = e.wb; d.selection = e.selection; } + super::extra::keep_watch_list(&now, &mut d.wb); d.calc.rebuild(&d.wb); d.revision += 1; Ok(json!({"redone": label})) @@ -972,7 +983,9 @@ fn paste_special(s: &mut Session, p: &Json) -> Result { e }; let mut moved = Formula::from_expr(e); - moved.array = f.array; + // The array range moves with its formula. + let (dr, dc) = (dest.row as i64 - sc.row as i64, dest.col as i64 - sc.col as i64); + moved.array = f.array.and_then(|a| Some(RangeRef::new(a.start.offset(dr, dc)?, a.end.offset(dr, dc)?))); return Some(std::sync::Arc::new(moved)); } let moved = @@ -1139,6 +1152,10 @@ fn paste_special(s: &mut Session, p: &Json) -> Result { /// Applies a reference adjustment to every cell formula and defined name in the workbook. pub(crate) fn rewrite_all_formulas(wb: &mut gridcraft_model::Workbook, target: &str, e: &gridcraft_formula::adjust::Edit) { + use gridcraft_formula::adjust::Edit; + // Rows and columns inserted or deleted on the target sheet move its legacy array ranges too + // (a cut moves its arrays itself). + let moves_arrays = matches!(e, Edit::Insert { .. } | Edit::Delete { .. } | Edit::InsertCells { .. } | Edit::DeleteCells { .. }); for si in 0..wb.sheets.len() { let host = wb.sheets.get(si).map(|s| s.name.clone()).unwrap_or_default(); let keys: Vec<(CellRef, std::sync::Arc)> = @@ -1146,16 +1163,21 @@ pub(crate) fn rewrite_all_formulas(wb: &mut gridcraft_model::Workbook, target: & if keys.is_empty() { continue; } + let on_target = moves_arrays && host.eq_ignore_ascii_case(target); let Some(sheet) = wb.sheet_mut(si) else { continue }; for (c, f) in keys { let Some(expr) = f.expr() else { continue }; let new = gridcraft_formula::adjust::adjust(expr, &host, target, e); let text = gridcraft_formula::print(&new); - if text != f.text + let array = match f.array { + Some(a) if on_target => super::sheet::map_range(a, e), + a => a, + }; + if (text != f.text || array != f.array) && let Some(cell) = sheet.cells.get_mut(c) { let mut nf = Formula::from_expr(new); - nf.array = f.array; + nf.array = array; cell.formula = Some(std::sync::Arc::new(nf)); } } @@ -1554,6 +1576,22 @@ mod tests { assert_eq!(bar.as_deref(), Some("{=B2:B3*2}")); } + #[test] + fn an_array_entered_outside_the_selection_covers_only_its_cell() { + // Found by the command-sequence fuzzer: the array took the selection's range even with + // `cell` elsewhere (or on another sheet), so its formula sat outside its own range and the + // selected cells were wiped. + let mut s = Session::new(); + s.new_workbook(); + s.execute("cell.set", json!({"cell": "K20", "input": "keep"})).unwrap(); + s.execute("selection.set", json!({"range": "K20:K22"})).unwrap(); + s.execute("cell.set", json!({"cell": "H5", "input": "=1+1", "array": true})).unwrap(); + let sh = s.doc().unwrap().wb.active().unwrap(); + let f = sh.cell(CellRef::parse("H5").unwrap()).and_then(|c| c.formula.clone()).unwrap(); + assert_eq!(f.array, Some(RangeRef::parse("H5").unwrap())); + assert_eq!(sh.value(CellRef::parse("K20").unwrap()), Value::text("keep")); + } + #[test] fn wildcards() { assert!(wildcard("hello", "h*o")); diff --git a/crates/engine/src/cmd/extra.rs b/crates/engine/src/cmd/extra.rs index 9f2565a..5543ce9 100644 --- a/crates/engine/src/cmd/extra.rs +++ b/crates/engine/src/cmd/extra.rs @@ -651,6 +651,19 @@ fn stash_set(wb: &mut Workbook, key: &str, v: &Json) { wb.names.push(DefinedName { name: key.into(), scope: None, formula, comment: String::new(), hidden: true }); } +/// The Watch Window list isn't an edit (Excel doesn't undo it): undo and redo keep the current +/// list when they swap in another snapshot of the workbook. +pub(crate) fn keep_watch_list(from: &Workbook, to: &mut std::sync::Arc) { + let find = |wb: &Workbook| wb.names.iter().find(|n| n.name == WATCH_NAME).cloned(); + let keep = find(from); + if keep == find(to) { + return; + } + let wb = std::sync::Arc::make_mut(to); + wb.names.retain(|n| n.name != WATCH_NAME); + wb.names.extend(keep); +} + fn custom_views(s: &mut Session, p: &Json) -> Result { let d = s.doc()?; let mut views = stash_get(&d.wb, VIEWS_NAME); diff --git a/crates/engine/src/cmd/format.rs b/crates/engine/src/cmd/format.rs index be09ba2..fb2bae5 100644 --- a/crates/engine/src/cmd/format.rs +++ b/crates/engine/src/cmd/format.rs @@ -561,8 +561,11 @@ fn merge(s: &mut Session, p: &Json, how: &str) -> Result { if how == "center" && s.doc()?.wb.sheet(sheet).is_some_and(|sh| ranges.iter().all(|r| sh.merges.contains(r))) { "unmerge" } else { how }; edit(s, |cx| { let mut touched = Vec::new(); + // Areas whose merging changes: a merged cell blocks a dynamic array's spill. + let mut areas: Vec = ranges.clone(); for r in &ranges { let sh = cx.sheet_mut(sheet)?; + areas.extend(sh.merges.iter().filter(|m| m.intersects(r))); sh.merges.retain(|m| !m.intersects(r)); match how { "unmerge" => {} @@ -593,6 +596,17 @@ fn merge(s: &mut Session, p: &Json, how: &str) -> Result { } } } + // Spills over the areas, and blocked ones that could reach them, are evaluated again. + let sh = cx.sheet_mut(sheet)?; + for a in &areas { + touched.extend(sh.spill_ranges.iter().filter(|(_, r)| r.intersects(a)).map(|(anchor, _)| (sheet, *anchor))); + let reach = RangeRef::new(CellRef::new(0, 0), a.end); + let blocked = sh + .cells + .iter_range(reach) + .filter(|(_, x)| x.formula.is_some() && x.value == gridcraft_core::Value::Error(gridcraft_core::CellError::Spill)); + touched.extend(blocked.map(|(c, _)| (sheet, c))); + } cx.changed.extend(touched); Ok(()) })?; diff --git a/crates/engine/src/cmd/formulas.rs b/crates/engine/src/cmd/formulas.rs index df48a2c..ddf2f24 100644 --- a/crates/engine/src/cmd/formulas.rs +++ b/crates/engine/src/cmd/formulas.rs @@ -78,18 +78,28 @@ fn auto_sum(s: &mut Session, p: &Json) -> Result { }; if !sel.is_single() { // A block: totals below each column (and to the right when a blank column is included). - let num_end = sel.end; - let below = CellRef::new(num_end.row + 1, 0); - let mut writes = Vec::new(); - let last_row_blank = (sel.start.col..=sel.end.col).all(|c| sh.value(CellRef::new(sel.end.row, c)).is_empty()); - let (data_end_row, out_row) = if last_row_blank { (sel.end.row.saturating_sub(1), sel.end.row) } else { (sel.end.row, below.row) }; - for c in sel.start.col..=sel.end.col { - let r = RangeRef::new(CellRef::new(sel.start.row, c), CellRef::new(data_end_row, c)); - writes.push((CellRef::new(out_row, c), format!("={func}({})", r.a1()))); - } - for (c, f) in writes { - s.execute("cell.set", json!({"cell": c.a1(), "input": f}))?; + // Columns past the sheet's data get no total (a whole-row selection doesn't write 16,384 + // formulas), and all totals are one edit (one undo step). + let Some(last_col) = sh.cells.used_range().map(|u| u.end.col.min(sel.end.col)) else { return ok() }; + let last_row_blank = (sel.start.col..=last_col).all(|c| sh.value(CellRef::new(sel.end.row, c)).is_empty()); + let (data_end_row, out_row) = if last_row_blank { (sel.end.row.saturating_sub(1), sel.end.row) } else { (sel.end.row, sel.end.row + 1) }; + if out_row >= gridcraft_core::MAX_ROWS { + return Err(EngineError::Other("There's no room below the selection for the totals.".into())); } + let writes: Vec<(CellRef, String)> = (sel.start.col..=last_col) + .map(|c| { + (CellRef::new(out_row, c), format!("={func}({})", RangeRef::new(CellRef::new(sel.start.row, c), CellRef::new(data_end_row, c)).a1())) + }) + .collect(); + let sheet = d.wb.active_sheet; + edit(s, |cx| { + for (c, f) in &writes { + let cell = super::edit::input_to_cell(f, sheet, *c, &mut cx.wb)?.unwrap_or_default(); + cx.sheet_mut(sheet)?.set_cell(*c, cell); + cx.touch(sheet, *c); + } + Ok(()) + })?; return ok(); } // Single cell: guess the range above, else to the left. diff --git a/crates/engine/src/cmd/insert.rs b/crates/engine/src/cmd/insert.rs index 16859c3..0197709 100644 --- a/crates/engine/src/cmd/insert.rs +++ b/crates/engine/src/cmd/insert.rs @@ -1,7 +1,7 @@ //! Insert tab: tables, charts, sparklines, pictures, shapes, hyperlinks, comments and notes, //! conditional formatting (Home › Styles) and symbols. -use gridcraft_core::{CellRef, RangeRef}; +use gridcraft_core::{CellRef, MAX_ROWS, RangeRef}; use gridcraft_model::*; use serde_json::{Value as Json, json}; @@ -224,6 +224,12 @@ fn insert_table(s: &mut Session, p: &Json) -> Result { if d.wb.sheet(sheet).is_some_and(|sh| sh.tables.iter().any(|t| t.range.intersects(&r))) { return Err(EngineError::Other("A table can't overlap another table.".into())); } + let arrays = |sh: &gridcraft_model::Sheet| { + sh.cells.iter().any(|(_, x)| x.formula.as_ref().and_then(|f| f.array).is_some_and(|a| !a.is_single() && a.intersects(&r))) + }; + if d.wb.sheet(sheet).is_some_and(arrays) { + return Err(EngineError::Other("Multi-cell array formulas aren't allowed in tables.".into())); + } if r.count() > 50_000_000 { return Err(bad("insert.table", "range too large")); } @@ -234,9 +240,19 @@ fn insert_table(s: &mut Session, p: &Json) -> Result { let name = str_param(p, "name") .map(str::to_string) .unwrap_or_else(|| (1..).map(|n| format!("Table{n}")).find(|n| d.wb.table(n).is_none()).unwrap_or_else(|| "Table".into())); + if name.is_empty() || name.contains(' ') || name.chars().next().is_some_and(|c| c.is_ascii_digit()) || CellRef::parse(&name).is_some() { + return Err(EngineError::Other("The name that you entered is not valid.".into())); + } if d.wb.table(&name).is_some() || d.wb.names.iter().any(|n| n.name.eq_ignore_ascii_case(&name)) { return Err(EngineError::Other("The name entered already exists. Enter a unique name.".into())); } + // Without a header row, the data moves down a row to make room for one: never a table with it. + let moves_a_table = |sh: &gridcraft_model::Sheet| { + sh.tables.iter().any(|t| t.range.end.row >= r.start.row && t.range.start.col <= r.end.col && t.range.end.col >= r.start.col) + }; + if !header && d.wb.sheet(sheet).is_some_and(moves_a_table) { + return Err(EngineError::Other("This operation isn't allowed: it would shift cells in a table on your worksheet.".into())); + } let style = str_param(p, "style").unwrap_or("TableStyleMedium2").to_string(); let id = d.wb.next_object_id(); edit(s, |cx| { @@ -245,6 +261,11 @@ fn insert_table(s: &mut Session, p: &Json) -> Result { r } else { let sh = cx.sheet_mut(sheet)?; + // The header row pushes the data down one row: never off the end of the sheet. + let last = RangeRef::new(CellRef::new(MAX_ROWS - 1, r.start.col), CellRef::new(MAX_ROWS - 1, r.end.col)); + if sh.cells.iter_range(last).any(|(_, x)| x.formula.is_some() || !x.value.is_empty()) { + return Err(EngineError::Other("To prevent possible loss of data, cells can't be pushed off the end of the worksheet.".into())); + } sh.cells.shift_rows_in_cols(r.start.col, r.end.col, r.start.row, 1); sh.cell_pictures = std::mem::take(&mut sh.cell_pictures) .into_iter() @@ -257,7 +278,7 @@ fn insert_table(s: &mut Session, p: &Json) -> Result { sh.set_value(CellRef::new(r.start.row, c), gridcraft_core::Value::text(format!("Column{}", i + 1))); } cx.structural = true; - RangeRef::new(r.start, CellRef::new(r.end.row + 1, r.end.col)) + RangeRef::new(r.start, CellRef::new((r.end.row + 1).min(MAX_ROWS - 1), r.end.col)) }; let sh = cx.sheet_mut(sheet)?; let mut names: Vec = Vec::new(); diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index 7a380c7..12ef907 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -302,6 +302,9 @@ pub(crate) fn commit(d: &mut DocState, f: impl FnOnce(&mut Ctx) -> Result) if !protection_checked { check_protection(&d.wb, &wb, &d.unlocked_ranges)?; } + if !structural { + check_arrays(&d.wb, &wb)?; + } if changed.len() <= 200_000 { fit_rows.extend(changed.iter().map(|(s, c)| (*s, c.row))); } @@ -372,6 +375,36 @@ pub(crate) fn locked_cell_error(sh: &gridcraft_model::Sheet, c: CellRef, unlocke }) } +/// Refuses an edit that put something into a cell of a multi-cell legacy array other than its +/// formula cell, while the array stays (Excel: "You can't change part of an array."). The array +/// would show its value there while the file kept the new content. Cells shifted by row and +/// column edits aren't edits of their content, so structural commits aren't checked. +fn check_arrays(old: &Workbook, new: &Workbook) -> Result<()> { + for sh in &old.sheets { + if sh.spill.is_empty() { + continue; + } + let Some(after) = new.sheet_index(&sh.name).and_then(|i| new.sheets.get(i)) else { continue }; + if std::sync::Arc::ptr_eq(sh, after) { + continue; + } + for c in sh.cells.diff(&after.cells) { + let filled = after.cell(c).is_some_and(|x| x.formula.is_some() || !x.value.is_empty()); + if !filled || !sh.spill.contains_key(&c) || same_content(sh.cell(c), after.cell(c)) { + continue; + } + let in_array = sh.spill_ranges.keys().any(|&anchor| { + anchor != c + && after.cell(anchor).and_then(|x| x.formula.as_ref()).and_then(|f| f.array).is_some_and(|a| a.start == anchor && a.contains(c)) + }); + if in_array { + return Err(EngineError::Other("You can't change part of an array.".into())); + } + } + } + Ok(()) +} + /// Two cells hold the same value or formula (formats aside). fn same_content(a: Option<&gridcraft_model::Cell>, b: Option<&gridcraft_model::Cell>) -> bool { let empty = |c: Option<&gridcraft_model::Cell>| c.is_none_or(|c| c.formula.is_none() && c.value.is_empty()); diff --git a/crates/engine/src/cmd/sheet.rs b/crates/engine/src/cmd/sheet.rs index 864dd11..f05b77d 100644 --- a/crates/engine/src/cmd/sheet.rs +++ b/crates/engine/src/cmd/sheet.rs @@ -167,27 +167,8 @@ fn shift_sheet_features(sh: &mut Sheet, axis: Axis, at: u32, count: u32, insert: sh.validations.retain(|dv| !dv.ranges.is_empty()); for t in sh.tables.iter_mut() { if let Some(r) = map_range(t.range) { - // Columns inserted inside a table become new table columns. - if axis == Axis::Cols && insert && at > t.range.start.col && at <= t.range.end.col { - let idx = (at - t.range.start.col) as usize; - for k in 0..count { - let name = format!("Column{}", t.columns.len() + 1); - t.columns.insert( - (idx + k as usize).min(t.columns.len()), - gridcraft_model::TableColumn { name, totals: Default::default(), totals_label: None, formula: None }, - ); - } - } - if axis == Axis::Cols && !insert { - let lo = at.max(t.range.start.col); - let hi = (at + count).min(t.range.end.col + 1); - if hi > lo { - let a = (lo - t.range.start.col) as usize; - let b = (hi - t.range.start.col) as usize; - if b <= t.columns.len() { - t.columns.drain(a..b); - } - } + if axis == Axis::Cols { + shift_table_columns(t, at, count, insert); } t.range = r; } else { @@ -444,6 +425,9 @@ fn insert_cells(s: &mut Session, p: &Json) -> Result { } let e = Edit::InsertCells { axis: if shift == "right" { Axis::Cols } else { Axis::Rows }, range: r }; let sheet = target_sheet(s, p)?; + if let Some(sh) = s.doc()?.wb.sheet(sheet) { + check_table_shift(sh, &e)?; + } let right = shift == "right"; let out = edit(s, |cx| { let sh = cx.sheet_mut(sheet)?; @@ -472,6 +456,9 @@ fn delete_cells(s: &mut Session, p: &Json) -> Result { } let e = Edit::DeleteCells { axis: if shift == "left" { Axis::Cols } else { Axis::Rows }, range: r }; let sheet = target_sheet(s, p)?; + if let Some(sh) = s.doc()?.wb.sheet(sheet) { + check_table_shift(sh, &e)?; + } let left = shift == "left"; let out = edit(s, |cx| { let name = cx.wb.sheet(sheet).map(|s| s.name.clone()).unwrap_or_default(); @@ -798,8 +785,16 @@ fn move_sheet(s: &mut Session, p: &Json) -> Result { if copy { let mut sh = (*src).clone(); let base = sh.name.clone(); - let name = (2..).map(|k| format!("{base} ({k})")).find(|nm| cx.wb.sheet_index(nm).is_none()).unwrap_or(base.clone()); - sh.name = name.chars().take(31).collect(); + // `Name (2)`: the name is shortened so the suffix fits in Excel's 31 characters. + let name = (2..) + .map(|k| { + let suffix = format!(" ({k})"); + let keep = 31usize.saturating_sub(suffix.chars().count()); + format!("{}{suffix}", base.chars().take(keep).collect::().trim_end()) + }) + .find(|nm| cx.wb.sheet_index(nm).is_none()) + .unwrap_or(base.clone()); + sh.name = name; sh.tables.clear(); // table names must be unique sh.preserved = None; // kept file content (form controls, OLE objects…) stays with the original let at = to.min(n); @@ -910,7 +905,7 @@ fn step_sheet(s: &mut Session, dir: i64) -> Result { } /// Where a range of the edited sheet goes after a structural edit (`None` = deleted). -fn map_range(r: RangeRef, e: &Edit) -> Option { +pub(crate) fn map_range(r: RangeRef, e: &Edit) -> Option { let expr = gridcraft_formula::Expr::Ref(gridcraft_formula::Reference { sheet: gridcraft_formula::SheetSel::Current, kind: gridcraft_formula::RefKind::Range( @@ -942,9 +937,16 @@ fn shift_cell_features(sh: &mut Sheet, e: &Edit) { } sh.validations.retain(|dv| !dv.ranges.is_empty()); for t in sh.tables.iter_mut() { + // Cells inserted or deleted across a table's full height add or remove table columns. + if let Edit::InsertCells { axis: Axis::Cols, range } | Edit::DeleteCells { axis: Axis::Cols, range } = *e + && range.start.row <= t.range.start.row + && range.end.row >= t.range.end.row + { + shift_table_columns(t, range.start.col, range.width(), matches!(e, Edit::InsertCells { .. })); + } t.range = map_range(t.range, e).unwrap_or_default(); } - sh.tables.retain(|t| t.range != RangeRef::default()); + sh.tables.retain(|t| t.range != RangeRef::default() && !t.columns.is_empty()); if let Some(af) = sh.autofilter.as_mut() { match map_range(af.range, e) { Some(r) => af.range = r, @@ -964,3 +966,64 @@ fn shift_cell_features(sh: &mut Sheet, e: &Edit) { sh.print.print_area = map_range(pa, e); } } + +/// Columns inserted inside a table become new table columns (with names no other column has); +/// deleted ones leave it. +fn shift_table_columns(t: &mut gridcraft_model::Table, at: u32, count: u32, insert: bool) { + if insert { + if at > t.range.start.col && at <= t.range.end.col { + let idx = (at - t.range.start.col) as usize; + let mut n = t.columns.len(); + for k in 0..count as usize { + let name = loop { + n += 1; + let name = format!("Column{n}"); + if !t.columns.iter().any(|c| c.name.eq_ignore_ascii_case(&name)) { + break name; + } + }; + let col = gridcraft_model::TableColumn { name, totals: Default::default(), totals_label: None, formula: None }; + t.columns.insert((idx + k).min(t.columns.len()), col); + } + } + return; + } + let lo = at.max(t.range.start.col); + let hi = at.saturating_add(count).min(t.range.end.col + 1); + if hi > lo { + let a = (lo - t.range.start.col) as usize; + let b = (hi - t.range.start.col) as usize; + if b <= t.columns.len() { + t.columns.drain(a..b); + } + } +} + +/// Like Excel, Insert/Delete Cells never shifts part of a table: cells shifted up or down must +/// hold whole table columns, cells shifted left or right whole table rows, and deleting cells +/// never takes a table's header row without the whole table. +pub(crate) fn check_table_shift(sh: &Sheet, e: &Edit) -> Result<()> { + let (axis, r, delete) = match *e { + Edit::InsertCells { axis, range } => (axis, range, false), + Edit::DeleteCells { axis, range } => (axis, range, true), + _ => return Ok(()), + }; + for t in &sh.tables { + let tr = t.range; + let ok = match axis { + Axis::Rows => { + let moves = tr.end.row >= r.start.row && tr.start.col <= r.end.col && tr.end.col >= r.start.col; + let header_alone = delete && t.header_row && r.start.row <= tr.start.row && r.end.row >= tr.start.row && r.end.row < tr.end.row; + !moves || (r.start.col <= tr.start.col && r.end.col >= tr.end.col && !header_alone) + } + Axis::Cols => { + let moves = tr.end.col >= r.start.col && tr.start.row <= r.end.row && tr.end.row >= r.start.row; + !moves || (r.start.row <= tr.start.row && r.end.row >= tr.end.row) + } + }; + if !ok { + return Err(EngineError::Other("This operation isn't allowed: it would shift cells in a table on your worksheet.".into())); + } + } + Ok(()) +} diff --git a/crates/engine/src/tables.rs b/crates/engine/src/tables.rs index 21bc3be..9182cd4 100644 --- a/crates/engine/src/tables.rs +++ b/crates/engine/src/tables.rs @@ -176,8 +176,15 @@ pub(crate) fn sync_headers(cx: &mut crate::cmd::Ctx) { let Some(t) = sh.tables.get(ti) else { continue }; let col = (c.col - t.range.start.col) as usize; let Some(old) = t.columns.get(col).map(|x| x.name.clone()) else { continue }; - let text = crate::display::cell_text(&cx.wb, sh, c); - let is_text = sh.value(c).is_text(); + // A text header names its column as it is: a number format only changes how it looks (an + // accounting format pads text with spaces), and the file writes the name into the cell. + let value = sh.value(c); + let text = match value.as_text() { + Some(t) => t.to_string(), + None => crate::display::cell_text(&cx.wb, sh, c), + }; + // Headers hold constant text: a formula (or a spilled value) there becomes its text. + let is_text = value.is_text() && sh.cell(c).is_some_and(|x| x.formula.is_none()); if text == old && is_text { continue; } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index c9f4d0a..94481c3 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1032,6 +1032,202 @@ fn slicer_filters_table() { assert!(s.execute("slicer.toggle", json!({"value": "Nowhere"})).is_err()); } +#[test] +fn table_header_formatted_as_accounting_keeps_its_text_as_the_column_name() { + // Found by the command-sequence fuzzer: the accounting format pads text (` 172.4 `), and that + // padded text became the column name, which the XLSX writer then put in the header cell. + let mut s = s(); + s.execute("range.setValues", json!({"range": "A1", "values": [[172.4, "Qty"], [1, 2]]})).unwrap(); + s.execute("home.accounting", json!({"range": "A1:B2"})).unwrap(); + s.execute("insert.table", json!({"range": "A1:B2", "header": true})).unwrap(); + let d = s.doc().unwrap(); + let t = &d.wb.sheet(0).unwrap().tables[0]; + let names: Vec<_> = t.columns.iter().map(|c| c.name.as_str()).collect(); + assert_eq!(names, ["172.4", "Qty"]); + assert_eq!(v(&s, "A1"), Value::text("172.4")); + reopen(&mut s); + assert_eq!(v(&s, "A1"), Value::text("172.4")); +} + +#[test] +fn a_formula_moved_into_a_table_header_becomes_its_text() { + // Found by the command-sequence fuzzer: deleting a header row moved a formula into the + // header, which kept it while the XLSX writer wrote the column name there instead. + let mut s = s(); + s.execute("range.setValues", json!({"range": "A1", "values": [["Item", "Qty"], ["x", "=1+1"], ["y", 5]]})).unwrap(); + s.execute("insert.table", json!({"range": "A1:B3", "header": true})).unwrap(); + s.execute("home.deleteRows", json!({"rows": "1:1"})).unwrap(); + let d = s.doc().unwrap(); + let sh = d.wb.sheet(0).unwrap(); + assert_eq!(sh.cell(CellRef::new(0, 1)).and_then(|x| x.formula.clone()), None); + assert_eq!(v(&s, "B1"), Value::text("2")); + let names: Vec<_> = sh.tables[0].columns.iter().map(|c| c.name.as_str()).collect(); + assert_eq!(names, ["x", "2"]); +} + +#[test] +fn a_whole_column_table_without_headers_stays_on_the_sheet() { + // Found by the command-sequence fuzzer: the inserted header row pushed the table's end past + // the last row, so the table had an invalid range and was dropped from the XLSX file. + let mut s = s(); + s.execute("insert.table", json!({"range": "A:C", "header": false})).unwrap(); + let d = s.doc().unwrap(); + assert_eq!(d.wb.sheet(0).unwrap().tables[0].range.a1(), "A:C"); + s.execute("cell.set", json!({"cell": "E1", "input": "=ROWS(Table1)"})).unwrap(); + assert_eq!(v(&s, "E1"), Value::Number(1_048_575.0)); + reopen(&mut s); + assert_eq!(v(&s, "E1"), Value::Number(1_048_575.0)); + s.execute("cell.set", json!({"cell": "F1048576", "input": "x"})).unwrap(); + let e = s.execute("insert.table", json!({"range": "F:F", "header": false})).unwrap_err(); + assert!(e.to_string().contains("pushed off the end"), "{e}"); +} + +#[test] +fn autosum_over_whole_rows_is_one_undo_step_within_the_data() { + // Found by the command-sequence fuzzer: AutoSum over rows `3:5` ran one `cell.set` per column + // (16,384 undo steps, so undo never got back to the start). + let mut s = s(); + s.execute("range.setValues", json!({"range": "A3", "values": [[1, 2], [3, 4], [5, 6]]})).unwrap(); + s.execute("formulas.autoSum", json!({"range": "3:5"})).unwrap(); + assert_eq!(v(&s, "A6"), Value::Number(9.0)); + assert_eq!(v(&s, "B6"), Value::Number(12.0)); + let d = s.doc().unwrap(); + assert_eq!(d.undo.len(), 2); + assert_eq!(d.wb.active().unwrap().cells.used_range().unwrap().a1(), "A3:B6"); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(v(&s, "A6"), Value::Empty); + assert_eq!(v(&s, "B6"), Value::Empty); +} + +#[test] +fn insert_and_delete_cells_never_shift_part_of_a_table() { + // Found by the command-sequence fuzzer: Delete Cells moved part of a table's cells (so two + // tables overlapped) or narrowed a table without dropping its columns, and a table without + // headers pushed the cells of another table down from under it. + let mut s = s(); + s.execute("range.setValues", json!({"range": "D8", "values": [["a", "b", "c"], [1, 2, 3], [4, 5, 6]]})).unwrap(); + s.execute("insert.table", json!({"range": "D8:F10", "header": true})).unwrap(); + for (cmd, range, shift) in [ + ("home.deleteCells", "E9:G9", "left"), + ("home.deleteCells", "E9", "up"), + ("home.insertCells", "C9:D9", "down"), + ("home.deleteCells", "D8:F8", "up"), + ("home.insertCells", "A9:B9", "right"), + ] { + let e = s.execute(cmd, json!({"range": range, "shift": shift})).unwrap_err(); + assert!(e.to_string().contains("shift cells in a table"), "{cmd} {range} {shift}: {e}"); + } + // Whole table columns: the table loses (or gains) columns with them. + s.execute("home.deleteCells", json!({"range": "E7:E12", "shift": "left"})).unwrap(); + let names = |s: &Session| -> Vec { s.doc().unwrap().wb.sheet(0).unwrap().tables[0].columns.iter().map(|c| c.name.clone()).collect() }; + assert_eq!(names(&s), ["a", "c"]); + assert_eq!(s.doc().unwrap().wb.sheet(0).unwrap().tables[0].range.a1(), "D8:E10"); + s.execute("home.insertCells", json!({"range": "E8:E10", "shift": "right"})).unwrap(); + assert_eq!(names(&s), ["a", "Column2", "c"]); + assert_eq!(v(&s, "E8"), Value::text("Column2")); + let e = s.execute("insert.table", json!({"range": "H1:H2", "name": "T1"})).unwrap_err(); + assert!(e.to_string().contains("not valid"), "a cell reference isn't a table name: {e}"); + let e = s.execute("insert.table", json!({"range": "D5:F6", "header": false})).unwrap_err(); + assert!(e.to_string().contains("shift cells in a table"), "{e}"); +} + +#[test] +fn undo_and_redo_keep_the_watch_window_list() { + // Found by the command-sequence fuzzer: the list lives in the workbook but isn't an edit, so + // undoing an earlier edit swapped in a snapshot without it. + let mut s = s(); + s.execute("cell.set", json!({"cell": "A1", "input": "1"})).unwrap(); + s.execute("formulas.watchWindow", json!({"add": "Sheet1!A1"})).unwrap(); + let watched = |s: &mut Session| -> Vec { + let rows = s.execute("formulas.watchWindow", json!({})).unwrap(); + rows.as_array().unwrap().iter().map(|r| r["cell"].as_str().unwrap().to_string()).collect() + }; + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(watched(&mut s), ["Sheet1!A1"], "kept after undo"); + s.execute("edit.redo", json!({})).unwrap(); + assert_eq!(watched(&mut s), ["Sheet1!A1"], "kept after redo"); +} + +#[test] +fn merging_over_a_spill_blocks_it_and_unmerging_unblocks_it() { + // Found by the command-sequence fuzzer: merges changed no cell the spill depended on, so the + // spill went on as if the merged cells were free (a full recalc gave #SPILL!). + let mut s = s(); + s.execute("cell.set", json!({"cell": "A1", "input": "=SEQUENCE(3)"})).unwrap(); + s.execute("home.mergeCenter", json!({"range": "A2:B3"})).unwrap(); + assert_eq!(v(&s, "A1"), Value::Error(gridcraft_core::CellError::Spill)); + s.execute("home.mergeCenter", json!({"range": "A2:B3"})).unwrap(); + assert_eq!(v(&s, "A1"), Value::Number(1.0)); + assert_eq!(v(&s, "A3"), Value::Number(3.0)); +} + +#[test] +fn multi_cell_array_formulas_and_tables_dont_overlap() { + // Found by the command-sequence fuzzer: an array over a table header lost its value to the + // header name written as text, then showed #SPILL! (and reloaded from XLSX differently). + let mut s = s(); + s.execute("range.setValues", json!({"range": "B2", "values": [["Item", "Qty"], ["x", 2]]})).unwrap(); + s.execute("insert.table", json!({"range": "B2:C3", "header": true})).unwrap(); + s.execute("selection.set", json!({"range": "B1:B2"})).unwrap(); + let e = s.execute("cell.set", json!({"input": "=1", "array": true})).unwrap_err(); + assert!(e.to_string().contains("aren't allowed in tables"), "{e}"); + assert_eq!(v(&s, "B2"), Value::text("Item")); + s.execute("selection.set", json!({"range": "E1:E2"})).unwrap(); + s.execute("cell.set", json!({"input": "=1", "array": true})).unwrap(); + let e = s.execute("insert.table", json!({"range": "E2:F3"})).unwrap_err(); + assert!(e.to_string().contains("aren't allowed in tables"), "{e}"); + // One-cell arrays are fine. + s.execute("selection.set", json!({"range": "C3"})).unwrap(); + s.execute("cell.set", json!({"input": "=5", "array": true})).unwrap(); + assert_eq!(v(&s, "C3"), Value::Number(5.0)); +} + +#[test] +fn copies_of_a_long_named_sheet_get_unique_names() { + // Found by the command-sequence fuzzer: `name (2)` was cut to 31 characters, so the second + // copy of a 30-character name took the first copy's name. + let mut s = s(); + s.execute("sheet.rename", json!({"sheet": 0, "name": "Long name of a sheet 31 chars!"})).unwrap(); + s.execute("sheet.move", json!({"sheet": 0, "to": 1, "copy": true})).unwrap(); + s.execute("sheet.move", json!({"sheet": 0, "to": 2, "copy": true})).unwrap(); + let d = s.doc().unwrap(); + let names: Vec<&str> = d.wb.sheets.iter().map(|x| x.name.as_str()).collect(); + assert_eq!(names, ["Long name of a sheet 31 chars!", "Long name of a sheet 31 cha (2)", "Long name of a sheet 31 cha (3)"]); +} + +#[test] +fn row_and_column_inserts_move_legacy_array_ranges() { + // Found by the command-sequence fuzzer: the formula cell moved but its array range stayed, + // so the array covered the wrong cells (and read back from XLSX as #SPILL!). + let mut s = s(); + s.execute("cell.set", json!({"cell": "A1", "input": "7"})).unwrap(); + s.execute("selection.set", json!({"range": "C3:D3"})).unwrap(); + s.execute("cell.set", json!({"input": "=$A$1*2", "array": true})).unwrap(); + s.execute("home.insertColumns", json!({"cols": "B:B"})).unwrap(); + s.execute("home.insertRows", json!({"rows": "1:1"})).unwrap(); + let d = s.doc().unwrap(); + let f = d.wb.sheet(0).unwrap().cell(CellRef::new(3, 3)).and_then(|x| x.formula.clone()).unwrap(); + assert_eq!(f.array, Some(gridcraft_core::RangeRef::parse("D4:E4").unwrap())); + assert_eq!(v(&s, "E4"), Value::Number(14.0)); + reopen(&mut s); + assert_eq!(v(&s, "E4"), Value::Number(14.0)); +} + +#[test] +fn part_of_a_legacy_array_cant_be_changed() { + // Found by the command-sequence fuzzer: a value typed into an array cell showed in the + // workbook, but the file kept the array there (Excel refuses the edit). + let mut s = s(); + s.execute("selection.set", json!({"range": "A1:A3"})).unwrap(); + s.execute("cell.set", json!({"input": "=5", "array": true})).unwrap(); + let e = s.execute("range.setValues", json!({"range": "A2", "values": [[13]]})).unwrap_err(); + assert!(e.to_string().contains("part of an array"), "{e}"); + assert_eq!(v(&s, "A2"), Value::Number(5.0)); + // Replacing the whole array is fine. + s.execute("range.setValues", json!({"range": "A1", "values": [[1], [2], [3]]})).unwrap(); + assert_eq!(v(&s, "A2"), Value::Number(2.0)); +} + #[test] fn create_names_from_selection() { let names = |s: &Session| -> Vec<(String, String)> { diff --git a/crates/engine/tests/command_sequence_fuzz.rs b/crates/engine/tests/command_sequence_fuzz.rs new file mode 100644 index 0000000..f8cb603 --- /dev/null +++ b/crates/engine/tests/command_sequence_fuzz.rs @@ -0,0 +1,1090 @@ +//! Command-sequence fuzzing: reproducible random sequences of commands on the sample workbooks, +//! with invariants checked as they run. Per-command tests (`every_command_survives_empty_params`) +//! miss the bugs that need a sequence — insert rows under a spill, undo, sort, paste into a table — +//! and those are the ones that corrupt a workbook. +//! +//! Each seed picks a sample (sales, budget, grades) and runs `GRIDCRAFT_FUZZ_STEPS` commands. Most +//! come from a curated list of state-changing commands with hand-shaped params; the rest are drawn +//! from every registered command, with params generated from its `params` doc string. +//! +//! Invariants: +//! - no command panics (`Session::execute` reports a caught panic as [`EngineError::Internal`]), +//! and saving, reloading or recalculating never panics either; +//! - a non-undoable command never changes cell content (it would be lost to undo); +//! - the structure stays valid: active sheet in range, sheet and table names valid and unique, +//! merges and tables on the sheet and not overlapping, legacy arrays anchored at their +//! top-left cell, the selection on the sheet; +//! - a full recalculation from scratch gives the values the incremental recalculation left +//! (catches stale dependency graphs); +//! - every few steps and at the end, an XLSX save and reload (with the recalculation opening does) +//! gives the same sheets, formulas and values; +//! - at the end, undoing everything gives back the starting workbook and redoing everything the +//! final one (view state aside), and both keep the Watch Window list, which isn't an edit. +//! +//! Knobs (environment variables): +//! - `GRIDCRAFT_FUZZ_SEEDS` (default 6) and `GRIDCRAFT_FUZZ_STEPS` (default 40): the budget. Soak with +//! `GRIDCRAFT_FUZZ_SEEDS=200 GRIDCRAFT_FUZZ_STEPS=200 cargo test --release -p gridcraft-engine --test command_sequence_fuzz`. +//! - `GRIDCRAFT_FUZZ_SEED=17`: replay that one seed (the failure message prints the command line). +//! - `GRIDCRAFT_FUZZ_SHRINK=1`: on failure, shrink the sequence to a minimal list of commands that +//! still fails the same way, printed as `execute` calls ready for a regression test. +//! - `GRIDCRAFT_FUZZ_KEEP_GOING=1`: report every failing seed instead of stopping at the first. + +use std::collections::BTreeSet; +use std::sync::Arc; + +use gridcraft_core::{CellRef, MAX_COLS, MAX_ROWS, RangeRef, Value, col_to_letters}; +use gridcraft_engine::calc::Calc; +use gridcraft_engine::model::{CalcMode, Formula, Sheet, Workbook}; +use gridcraft_engine::{EngineError, Session, command_specs, find_command, io}; +use serde_json::{Value as Json, json}; + +const SAMPLES: [&str; 3] = ["sales", "budget", "grades"]; +/// Episodes end (undo/redo checks, then a fresh history) before the engine's 100-step undo limit +/// starts dropping the oldest entries. +const EPISODE_UNDO_DEPTH: usize = 90; +/// Save and reload every this many steps (and at the end of every episode). +const ROUNDTRIP_EVERY: usize = 10; +/// The hidden defined name `formulas.watchWindow` keeps its list in. +const WATCH_NAME: &str = "_sc_watch"; + +/// Commands the fuzzer never runs, and why. +const EXCLUDED: &[(&str, &str)] = &[ + ("file.new", "opens another document"), + ("file.open", "opens another document"), + ("file.close", "closes the document under test"), + ("file.save", "writes files"), + ("file.saveAs", "writes files"), + ("file.exportCsv", "writes files"), + ("file.exportHtml", "writes files"), + ("file.exportPdf", "renders a PDF: slow, and may write files"), + ("file.print", "renders a PDF: slow"), + ("window.activate", "switches documents"), + ("view.newWindow", "opens another document"), + ("insert.picture", "reads files"), + ("pageLayout.background", "reads files"), + ("formulas.calculationOptions", "manual and iterative calculation differ from a full recalc by design"), +]; + +// ---------------------------------------------------------------- deterministic randomness + +/// SplitMix64: tiny, deterministic and good enough to pick commands. +struct Rng(u64); + +impl Rng { + fn next(&mut self) -> u64 { + self.0 = self.0.wrapping_add(0x9E37_79B9_7F4A_7C15); + let mut z = self.0; + z = (z ^ (z >> 30)).wrapping_mul(0xBF58_476D_1CE4_E5B9); + z = (z ^ (z >> 27)).wrapping_mul(0x94D0_49BB_1331_11EB); + z ^ (z >> 31) + } + fn below(&mut self, n: usize) -> usize { + if n == 0 { 0 } else { (self.next() % n as u64) as usize } + } + /// Inclusive range. + fn int(&mut self, lo: i64, hi: i64) -> i64 { + lo + (self.next() % ((hi - lo + 1).max(1) as u64)) as i64 + } + fn chance(&mut self, percent: u64) -> bool { + self.next() % 100 < percent + } + fn pick<'a, T>(&mut self, xs: &'a [T]) -> &'a T { + &xs[self.below(xs.len())] + } +} + +// ---------------------------------------------------------------- harness + +type Step = (String, Json); + +#[derive(Debug)] +struct Failure { + /// Index of the step that broke an invariant (`steps.len()` for the end-of-run checks). + step: usize, + /// Short category, used to keep shrinking on the same bug. + kind: &'static str, + what: String, +} + +fn env_num(name: &str, default: u64) -> u64 { + std::env::var(name).ok().and_then(|v| v.trim().parse().ok()).unwrap_or(default) +} + +#[test] +fn command_sequences_keep_the_workbook_consistent() { + let seeds = env_num("GRIDCRAFT_FUZZ_SEEDS", 6); + let steps = env_num("GRIDCRAFT_FUZZ_STEPS", 40) as usize; + let keep_going = std::env::var_os("GRIDCRAFT_FUZZ_KEEP_GOING").is_some(); + let list: Vec = match std::env::var("GRIDCRAFT_FUZZ_SEED").ok().and_then(|v| v.trim().parse().ok()) { + Some(one) => vec![one], + None => (0..seeds).collect(), + }; + let mut reports = Vec::new(); + for seed in list { + let (sample, sequence, failure) = run_seed(seed, steps); + if let Some(f) = failure { + reports.push(report(seed, steps, sample, &sequence, &f)); + if !keep_going { + break; + } + } + } + assert!(reports.is_empty(), "{} failing seed(s)\n\n{}", reports.len(), reports.join("\n\n")); +} + +fn report(seed: u64, steps: usize, sample: &str, sequence: &[Step], f: &Failure) -> String { + let at = match sequence.get(f.step) { + Some((id, p)) => format!("step {} of {}: `{id}` {p}", f.step, sequence.len()), + None => format!("end-of-run checks after {} steps", sequence.len()), + }; + let mut out = format!( + "command-sequence fuzz failure: seed {seed} (sample `{sample}`), {at}\n invariant [{}]: {}\n replay: GRIDCRAFT_FUZZ_SEED={seed} GRIDCRAFT_FUZZ_STEPS={steps} cargo test -p gridcraft-engine --test command_sequence_fuzz\n", + f.kind, f.what + ); + let end = (f.step + 1).min(sequence.len()); + let from = end.saturating_sub(30); + out.push_str(&format!(" steps {from}..{end}:\n")); + for (i, (id, p)) in sequence.iter().enumerate().take(end).skip(from) { + out.push_str(&format!(" {i:4} {id} {p}\n")); + } + if std::env::var_os("GRIDCRAFT_FUZZ_SHRINK").is_some() { + let min = shrink(sample, &sequence[..end], f.kind); + out.push_str(&format!(" minimal sequence ({} steps) from file.new {{\"sample\": \"{sample}\"}}:\n", min.len())); + for (id, p) in &min { + out.push_str(&format!(" s.execute(\"{id}\", json!({p})).ok();\n")); + } + if let Some(f) = replay(sample, &min) { + out.push_str(&format!(" which fails with [{}]: {}\n", f.kind, f.what)); + } + } else { + out.push_str(" (set GRIDCRAFT_FUZZ_SHRINK=1 for a minimal sequence)\n"); + } + out +} + +/// Generates and runs one seed's sequence; returns the sample, the steps run and the first failure. +fn run_seed(seed: u64, steps: usize) -> (&'static str, Vec, Option) { + let mut rng = Rng(seed.wrapping_mul(0xA24B_AED4_963E_E407) ^ 0x5EED); + let sample = SAMPLES[(seed % SAMPLES.len() as u64) as usize]; + let mut h = Harness::new(sample); + let mut sequence = Vec::new(); + for i in 0..steps { + let (id, params) = generate(&mut rng, &h.s); + sequence.push((id.clone(), params.clone())); + if let Err(f) = h.step(i, &id, ¶ms) { + return (sample, sequence, Some(f)); + } + } + let failure = h.finish(steps).err(); + (sample, sequence, failure) +} + +/// Runs a recorded sequence with the same checks. +fn replay(sample: &str, steps: &[Step]) -> Option { + let mut h = Harness::new(sample); + for (i, (id, p)) in steps.iter().enumerate() { + if let Err(f) = h.step(i, id, p) { + return Some(f); + } + } + h.finish(steps.len()).err() +} + +/// Greedy delta debugging: drop chunks of steps while the run still fails the same way. +fn shrink(sample: &str, steps: &[Step], kind: &str) -> Vec { + let mut cur = steps.to_vec(); + let mut chunk = cur.len().div_ceil(2).max(1); + loop { + let mut i = 0; + let mut removed = false; + while i < cur.len() { + let mut trial = cur.clone(); + trial.drain(i..(i + chunk).min(trial.len())); + if replay(sample, &trial).is_some_and(|f| f.kind == kind) { + cur = trial; + removed = true; + } else { + i += chunk; + } + } + if chunk == 1 && !removed { + return cur; + } + if !removed { + chunk = chunk.div_ceil(2); + } + } +} + +struct Harness { + s: Session, + /// The workbook at the start of the current episode (what undoing everything gives back). + base: Arc, +} + +fn fail(step: usize, kind: &'static str, what: impl Into) -> Failure { + Failure { step, kind, what: what.into() } +} + +/// Runs `f`, turning a panic into a failure (for checks that run outside `Session::execute`). +fn guarded(step: usize, kind: &'static str, f: impl FnOnce() -> Result) -> Result { + match std::panic::catch_unwind(std::panic::AssertUnwindSafe(f)) { + Ok(Ok(v)) => Ok(v), + Ok(Err(e)) => Err(fail(step, kind, e)), + Err(p) => { + let msg = p.downcast_ref::<&str>().map(|s| s.to_string()).or_else(|| p.downcast_ref::().cloned()).unwrap_or_default(); + Err(fail(step, kind, format!("panicked: {msg}"))) + } + } +} + +impl Harness { + fn new(sample: &str) -> Harness { + let mut s = Session::new(); + s.execute("file.new", json!({"sample": sample})).expect("sample opens"); + let base = s.doc().expect("document").wb.clone(); + Harness { s, base } + } + + fn wb(&self) -> Arc { + self.s.doc().expect("document").wb.clone() + } + + fn step(&mut self, i: usize, id: &str, params: &Json) -> Result<(), Failure> { + let before = self.wb(); + let r = self.s.execute(id, params.clone()); + self.s.take_ui_requests(); + if let Err(EngineError::Internal(_, msg)) = &r { + return Err(fail(i, "panic", format!("`{id}` panicked (caught by Session::execute): {msg}"))); + } + if self.s.documents().len() != 1 { + return Err(fail(i, "documents", format!("`{id}` changed the number of open documents; exclude it in EXCLUDED"))); + } + let after = self.wb(); + let spec = find_command(id).expect("generated ids are registered"); + if !spec.undoable + && !matches!(id, "edit.undo" | "edit.redo") + && let Some(diff) = content_diff(&before, &after) + { + return Err(fail(i, "not undoable", format!("non-undoable `{id}` changed cell content: {diff}"))); + } + self.check_state(i)?; + if (i + 1).is_multiple_of(ROUNDTRIP_EVERY) { + guarded(i, "xlsx roundtrip", || roundtrip_diff(&after))?; + } + if self.s.doc().expect("document").undo.len() >= EPISODE_UNDO_DEPTH { + self.end_episode(i)?; + } + Ok(()) + } + + /// Structure and recalculation invariants on the current state. + fn check_state(&self, i: usize) -> Result<(), Failure> { + let d = self.s.doc().expect("document"); + if let Some(e) = structure_problem(&d.wb, &d.selection) { + return Err(fail(i, "structure", e)); + } + let wb = d.wb.clone(); + guarded(i, "recalc", || recalc_diff(&wb)) + } + + /// Undo everything, compare with the start, redo everything, compare with the end, then save + /// and reload; the next episode starts from here with an empty history. + fn end_episode(&mut self, i: usize) -> Result<(), Failure> { + let end = self.wb(); + let mut n = 0; + while !self.s.doc().expect("document").undo.is_empty() { + if let Err(EngineError::Internal(_, msg)) = self.s.execute("edit.undo", json!({})) { + return Err(fail(i, "panic", format!("`edit.undo` panicked while undoing everything: {msg}"))); + } + n += 1; + if n > 1_000 { + return Err(fail(i, "undo", "undo never empties its history")); + } + } + let undone = self.wb(); + if let Some(diff) = workbook_diff(&self.base, &undone) { + return Err(fail(i, "undo", format!("undoing all {n} steps doesn't give back the starting workbook: {diff}"))); + } + if watch_list(&end) != watch_list(&undone) { + return Err(fail(i, "undo", format!("undoing all {n} steps changed the Watch Window list (not an edit)"))); + } + self.check_state(i)?; + // Only the steps undone here: an `edit.undo {steps}` in the sequence may have left more. + for _ in 0..n { + if let Err(EngineError::Internal(_, msg)) = self.s.execute("edit.redo", json!({})) { + return Err(fail(i, "panic", format!("`edit.redo` panicked while redoing everything: {msg}"))); + } + } + if let Some(diff) = workbook_diff(&end, &self.wb()) { + return Err(fail(i, "redo", format!("redoing all {n} steps doesn't give back the final workbook: {diff}"))); + } + self.check_state(i)?; + guarded(i, "xlsx roundtrip", || roundtrip_diff(&end))?; + let d = self.s.active_mut().expect("document"); + d.undo.clear(); + d.redo.clear(); + self.base = d.wb.clone(); + Ok(()) + } + + fn finish(&mut self, steps: usize) -> Result<(), Failure> { + self.end_episode(steps) + } +} + +// ---------------------------------------------------------------- invariants + +fn has_content(c: &gridcraft_engine::model::Cell) -> bool { + c.formula.is_some() || !c.value.is_empty() +} + +/// Cells whose formula or constant differs (computed values of formulas aside). +fn content_diff(a: &Workbook, b: &Workbook) -> Option { + let names = |wb: &Workbook| wb.sheets.iter().map(|s| s.name.clone()).collect::>(); + if names(a) != names(b) { + return Some(format!("sheets {:?} became {:?}", names(a), names(b))); + } + for (x, y) in a.sheets.iter().zip(&b.sheets) { + if Arc::ptr_eq(x, y) { + continue; + } + for c in x.cells.diff(&y.cells) { + let content = |s: &Sheet| { + s.cell(c).filter(|c| has_content(c)).map(|c| match &c.formula { + Some(f) => format!("={}", f.text), + None => format!("{:?}", c.value), + }) + }; + if content(x) != content(y) { + return Some(format!("{}!{c}: {:?} became {:?}", x.name, content(x), content(y))); + } + } + } + None +} + +fn overlaps(a: &RangeRef, b: &RangeRef) -> bool { + a.intersection(b).is_some() +} + +fn structure_problem(wb: &Workbook, sel: &gridcraft_engine::Selection) -> Option { + if wb.sheets.is_empty() { + return Some("no sheets left".into()); + } + if wb.active_sheet >= wb.sheets.len() { + return Some(format!("active sheet {} of {}", wb.active_sheet, wb.sheets.len())); + } + let mut tables = BTreeSet::new(); + for (i, sh) in wb.sheets.iter().enumerate() { + if let Err(e) = wb.check_sheet_name(&sh.name, Some(i)) { + return Some(format!("sheet name `{}`: {e}", sh.name)); + } + let on_sheet = |r: &RangeRef| r.start.is_valid() && r.end.is_valid() && r.start.row <= r.end.row && r.start.col <= r.end.col; + if let Some(r) = sh.merges.iter().chain(sh.tables.iter().map(|t| &t.range)).find(|r| !on_sheet(r)) { + return Some(format!("{}: merge or table range {r:?} isn't on the sheet", sh.name)); + } + // A legacy array formula lives in the top-left cell of its range. + for (c, cell) in sh.cells.iter() { + if let Some(a) = cell.formula.as_ref().and_then(|f| f.array) + && (a.start != c || !on_sheet(&a)) + { + return Some(format!("{}!{c}: array formula covers {a}", sh.name)); + } + } + for (k, m) in sh.merges.iter().enumerate() { + if let Some(o) = sh.merges.iter().skip(k + 1).find(|o| overlaps(m, o)) { + return Some(format!("{}: merges {m} and {o} overlap", sh.name)); + } + } + for (k, t) in sh.tables.iter().enumerate() { + if !tables.insert(t.name.to_ascii_lowercase()) { + return Some(format!("two tables named `{}`", t.name)); + } + if t.columns.len() as u32 != t.range.end.col - t.range.start.col + 1 { + return Some(format!("table `{}` at {} has {} columns", t.name, t.range, t.columns.len())); + } + if let Some(o) = sh.tables.iter().skip(k + 1).find(|o| overlaps(&t.range, &o.range)) { + return Some(format!("{}: tables `{}` ({}) and `{}` ({}) overlap", sh.name, t.name, t.range, o.name, o.range)); + } + } + } + if sel.ranges.is_empty() || !sel.active.is_valid() || sel.ranges.iter().any(|r| !r.start.is_valid() || !r.end.is_valid()) { + return Some(format!("selection {sel:?}")); + } + None +} + +fn same_value(a: &Value, b: &Value) -> bool { + match (a, b) { + (Value::Number(x), Value::Number(y)) => x == y || (x.is_nan() && y.is_nan()), + _ => a == b, + } +} + +/// The first cell whose displayed value (own value or spilled) differs between two workbooks. +fn value_diff(a: &Workbook, b: &Workbook, what: &str) -> Option { + for (x, y) in a.sheets.iter().zip(&b.sheets) { + let mut keys: BTreeSet = BTreeSet::new(); + for s in [x, y] { + keys.extend(s.cells.iter().filter(|(_, c)| has_content(c)).map(|(c, _)| c)); + keys.extend(s.spill.keys().copied()); + } + for c in keys { + let (va, vb) = (x.value(c), y.value(c)); + if !same_value(&va, &vb) { + let f = x.cell(c).and_then(|c| c.formula.as_ref()).map(|f| format!(" (formula ={})", f.text)).unwrap_or_default(); + return Some(format!("{}!{c}{f}: {va:?} in memory, {vb:?} after {what}", x.name)); + } + } + } + None +} + +/// A full recalculation from scratch must agree with the incrementally maintained values. +fn recalc_diff(wb: &Workbook) -> Result<(), String> { + if wb.calc.mode != CalcMode::Automatic || wb.calc.iterative { + return Ok(()); + } + let mut fresh = wb.clone(); + Calc::new().recalc_all(&mut fresh); + value_diff(wb, &fresh, "a full recalc").map_or(Ok(()), Err) +} + +fn normalized_formula(f: &Formula) -> String { + let text = Formula::new(&f.text).text; + if f.array.is_some() { format!("{{={text}}}") } else { format!("={text}") } +} + +/// Save to XLSX, reload (recalculating like opening does) and compare sheets, formulas and values. +fn roundtrip_diff(wb: &Workbook) -> Result<(), String> { + let bytes = io::save_bytes(wb, "fuzz.xlsx").map_err(|e| format!("save failed: {e}"))?; + let (mut back, _) = io::open_bytes("fuzz.xlsx", &bytes).map_err(|e| format!("reload failed: {e}"))?; + Calc::new().recalc_all(&mut back); + let names = |wb: &Workbook| wb.sheets.iter().map(|s| s.name.clone()).collect::>(); + if names(wb) != names(&back) { + return Err(format!("sheets {:?} reload as {:?}", names(wb), names(&back))); + } + for (x, y) in wb.sheets.iter().zip(&back.sheets) { + let formulas = |s: &Sheet| s.cells.iter().filter_map(|(c, cell)| Some((c, normalized_formula(cell.formula.as_ref()?)))).collect::>(); + let (fx, fy) = (formulas(x), formulas(y)); + if fx != fy { + let first = fx.iter().zip(&fy).find(|(a, b)| a != b).map(|(a, b)| format!("{a:?} vs {b:?}")); + let first = first.unwrap_or_else(|| format!("{} formulas vs {}", fx.len(), fy.len())); + return Err(format!("{}: formulas differ after reload: {first}", x.name)); + } + } + if wb.calc.mode == CalcMode::Automatic && !wb.calc.iterative { + value_diff(wb, &back, "an XLSX save and reload").map_or(Ok(()), Err)?; + } + Ok(()) +} + +/// The Watch Window list, which is workbook state but not an edit (undo keeps it). +fn watch_list(wb: &Workbook) -> Option<&str> { + wb.names.iter().find(|n| n.name == WATCH_NAME).map(|n| n.formula.as_str()) +} + +/// The workbook with view state (active sheet, scroll, active cell) and the Watch Window list +/// reset: undo restores view state as it was, but switching sheets isn't an undoable edit. +fn canonical(wb: &Workbook) -> Workbook { + let mut wb = wb.clone(); + wb.active_sheet = 0; + wb.names.retain(|n| n.name != WATCH_NAME); + for sh in wb.sheets.iter_mut() { + let sh = Arc::make_mut(sh); + sh.view_active = CellRef::default(); + sh.view_top_left = CellRef::default(); + } + wb +} + +fn workbook_diff(a: &Workbook, b: &Workbook) -> Option { + let (a, b) = (canonical(a), canonical(b)); + if a == b { + return None; + } + if let Some(d) = content_diff(&a, &b) { + return Some(d); + } + for (x, y) in a.sheets.iter().zip(&b.sheets) { + if x != y { + if let Some(c) = x.cells.diff(&y.cells).first() { + return Some(format!("{}!{c}: {:?} vs {:?}", x.name, x.cell(*c), y.cell(*c))); + } + let field = [ + ("merges", x.merges != y.merges), + ("tables", x.tables != y.tables), + ("charts", x.charts != y.charts), + ("rows", x.rows != y.rows), + ("cols", x.cols != y.cols), + ("autofilter", x.autofilter != y.autofilter), + ("cond_formats", x.cond_formats != y.cond_formats), + ("validations", x.validations != y.validations), + ("spill", x.spill != y.spill || x.spill_ranges != y.spill_ranges), + ] + .into_iter() + .find(|(_, d)| *d) + .map_or("other sheet properties", |(n, _)| n); + return Some(format!("{}: {field} differ", x.name)); + } + } + Some("workbook-level properties (names, styles, settings) differ".into()) +} + +// ---------------------------------------------------------------- generation + +fn letter(c: u32) -> String { + col_to_letters(c) +} + +/// A row/column index: mostly near the data, sometimes further out, rarely at the sheet's edge. +fn coord(rng: &mut Rng, near: i64, far: i64, max: u32) -> u32 { + match rng.below(20) { + 0 => max - 1 - rng.int(0, 2) as u32, + 1 | 2 => rng.int(near, far) as u32, + _ => rng.int(0, near) as u32, + } +} + +fn cell(rng: &mut Rng) -> CellRef { + CellRef::new(coord(rng, 24, 120, MAX_ROWS), coord(rng, 11, 40, MAX_COLS)) +} + +/// A cell near the data (formula references stay small). +fn near_cell(rng: &mut Rng) -> CellRef { + CellRef::new(rng.int(0, 22) as u32, rng.int(0, 10) as u32) +} + +fn near_range(rng: &mut Rng) -> RangeRef { + let a = near_cell(rng); + let b = CellRef::new(a.row + rng.int(0, 7) as u32, a.col + rng.int(0, 4) as u32); + RangeRef::new(a, b) +} + +fn range(rng: &mut Rng) -> String { + match rng.below(20) { + 0 | 1 => { + let c = rng.int(0, 10) as u32; + format!("{}:{}", letter(c), letter(c + rng.int(0, 2) as u32)) + } + 2 | 3 => { + let r = rng.int(1, 25); + format!("{r}:{}", r + rng.int(0, 2)) + } + 4 => cell(rng).a1(), + 5 => { + let a = cell(rng); + RangeRef::new(a, a.offset_clamped(rng.int(0, 3), rng.int(0, 3))).a1() + } + _ => near_range(rng).a1(), + } +} + +fn rows_spec(rng: &mut Rng) -> String { + let r = coord(rng, 24, 120, MAX_ROWS) as i64 + 1; + format!("{r}:{}", (r + rng.int(0, 2)).min(MAX_ROWS as i64)) +} + +fn cols_spec(rng: &mut Rng) -> String { + let c = coord(rng, 11, 40, MAX_COLS); + format!("{}:{}", letter(c), letter((c + rng.int(0, 1) as u32).min(MAX_COLS - 1))) +} + +fn quoted(name: &str) -> String { + format!("'{}'", name.replace('\'', "''")) +} + +fn wb_of(s: &Session) -> &Workbook { + &s.doc().expect("document").wb +} + +fn spill_anchors(wb: &Workbook) -> Vec<(usize, CellRef)> { + wb.sheets.iter().enumerate().flat_map(|(i, sh)| sh.spill_ranges.keys().map(move |c| (i, *c))).collect() +} + +fn tables(wb: &Workbook) -> Vec<(usize, &gridcraft_engine::model::Table)> { + wb.sheets.iter().enumerate().flat_map(|(i, sh)| sh.tables.iter().map(move |t| (i, t))).collect() +} + +fn other_sheet(rng: &mut Rng, wb: &Workbook) -> String { + quoted(&wb.sheets[rng.below(wb.sheets.len())].name) +} + +fn formula(rng: &mut Rng, wb: &Workbook) -> String { + let c = near_cell(rng).a1(); + let c2 = near_cell(rng).a1(); + let r = near_range(rng).a1(); + let n = rng.int(1, 6); + let col = letter(rng.int(0, 9) as u32); + let tabs = tables(wb); + match rng.below(30) { + 0 => format!("={c}+{n}"), + 1 => format!("=SUM({r})"), + 2 => format!("={c}*{c2}"), + 3 => format!("={}!{c}*2", other_sheet(rng, wb)), + 4 => format!("=SEQUENCE({n},{})", rng.int(1, 3)), + 5 | 6 if !tabs.is_empty() => { + let (_, t) = rng.pick(&tabs); + let column = t.columns.get(rng.below(t.columns.len())).map(|c| c.name.clone()).unwrap_or_default(); + match rng.below(4) { + 0 => format!("=SUM({}[{column}])", t.name), + 1 => format!("=ROWS({})", t.name), + 2 => format!("={}[@{column}]", t.name), + _ => format!("=COUNTA({}[#All])", t.name), + } + } + 7 => format!("=INDIRECT(\"{c}\")"), + 8 => format!("=OFFSET({c},1,1)"), + 9 => format!("=IF({c}>{n},\"big\",{c})"), + 10 => { + let anchors = spill_anchors(wb); + match anchors.get(rng.below(anchors.len().max(1))) { + Some((si, a)) => format!("=SUM({}!{a}#)", quoted(&wb.sheets[*si].name)), + None => format!("={c}#"), + } + } + 11 => format!("=SORT({r})"), + 12 => format!("=UNIQUE({r})"), + 13 => format!("=TRANSPOSE({r})"), + 14 => format!("=COUNTIF({col}:{col},\">0\")"), + 15 => format!("=SUM({}:{}!{c})", other_sheet(rng, wb), other_sheet(rng, wb)), + 16 => format!("=LET(x,{c},x*2)"), + 17 => format!("=XLOOKUP({c},{col}1:{col}20,{col}1:{col}20)"), + 18 => format!("=FILTER({col}1:{col}20,{col}1:{col}20>{n})"), + 19 => match wb.names.get(rng.below(wb.names.len().max(1))) { + Some(nm) => format!("=SUM({})", nm.name), + None => format!("={c}"), + }, + 20 => "=ROW()+COLUMN()".into(), + 21 => format!("={c}&\"-\"&{c2}"), + 22 => format!("=AVERAGE({r})*{n}"), + 23 => format!("=MAX({r})-MIN({r})"), + 24 => rng.pick(&["=1/0", "=NA()", "=#REF!+1", "=NOSUCHFN(1)", "=SUM(", "=\"text\""]).to_string(), + 25 => format!("={r}"), + 26 => format!("=INDEX({r},{n},1)"), + 27 => format!("=VLOOKUP({c},{r},2,FALSE)"), + 28 => format!("=SEQUENCE({n})*{c}"), + _ => format!("=TEXT({c},\"0.00\")"), + } +} + +fn literal(rng: &mut Rng) -> String { + match rng.below(14) { + 0..=3 => rng.int(-50, 500).to_string(), + 4 => format!("{}.{}", rng.int(-9, 99), rng.int(0, 99)), + 5 => rng.pick(&["North", "South", "East", "West", "abc", "Total", "x y"]).to_string(), + 6 => rng.pick(&["TRUE", "FALSE"]).to_string(), + 7 => format!("{}%", rng.int(0, 100)), + 8 => format!("2026-{:02}-{:02}", rng.int(1, 12), rng.int(1, 28)), + 9 => String::new(), + 10 => "'007".into(), + 11 => "$1,234.50".into(), + 12 => "#N/A".into(), + _ => "1e3".into(), + } +} + +fn input(rng: &mut Rng, wb: &Workbook) -> String { + if rng.chance(55) { formula(rng, wb) } else { literal(rng) } +} + +fn json_input(rng: &mut Rng, wb: &Workbook) -> Json { + match rng.below(10) { + 0..=1 => json!(rng.int(-20, 200)), + 2 => Json::Null, + 3 => json!(rng.chance(50)), + _ => json!(input(rng, wb)), + } +} + +fn values_2d(rng: &mut Rng, wb: &Workbook) -> Json { + let (h, w) = (rng.int(1, 4), rng.int(1, 3)); + Json::Array((0..h).map(|_| Json::Array((0..w).map(|_| json_input(rng, wb)).collect())).collect()) +} + +fn sheet_param(rng: &mut Rng, wb: &Workbook) -> Json { + match rng.below(10) { + 0 => json!(wb.sheets.len() + rng.below(2)), + 1..=4 => json!(wb.sheets[rng.below(wb.sheets.len())].name), + _ => json!(rng.below(wb.sheets.len())), + } +} + +fn new_name(rng: &mut Rng) -> String { + rng.pick(&["Data", "Q4 2026", "Summary", "Sheet9", "x", "Sales", "Long name of a sheet 31 chars!", "it's", "Grades"]).to_string() +} + +fn color(rng: &mut Rng) -> String { + rng.pick(&["#FF0000", "#00B050", "#FFFF00", "#1F4E79", "none"]).to_string() +} + +fn table_param(rng: &mut Rng, wb: &Workbook) -> Json { + let tabs = tables(wb); + match tabs.get(rng.below(tabs.len() + 1)) { + Some((_, t)) => json!(t.name), + None => json!("NoSuchTable"), + } +} + +fn chart_ids(wb: &Workbook) -> Vec { + wb.sheets.iter().flat_map(|s| s.charts.iter().map(|c| c.id)).collect() +} + +/// Curated state-changing commands and their weights. +const CURATED: &[(&str, usize)] = &[ + ("cell.set", 22), + ("range.setValues", 5), + ("range.fill", 2), + ("selection.set", 8), + ("edit.undo", 6), + ("edit.redo", 3), + ("edit.copy", 3), + ("edit.cut", 2), + ("edit.paste", 4), + ("edit.pasteSpecial", 2), + ("edit.clearContents", 2), + ("edit.clearAll", 1), + ("edit.fillDown", 2), + ("edit.fillRight", 1), + ("edit.autoFill", 2), + ("edit.fillSeries", 1), + ("home.bold", 1), + ("home.fillColor", 1), + ("home.numberFormat", 1), + ("home.borders", 1), + ("home.mergeCenter", 2), + ("home.mergeCells", 1), + ("home.unmergeCells", 1), + ("home.insertRows", 4), + ("home.insertColumns", 3), + ("home.deleteRows", 4), + ("home.deleteColumns", 3), + ("home.insertCells", 2), + ("home.deleteCells", 2), + ("home.hideRows", 1), + ("home.unhideRows", 1), + ("home.insertSheet", 2), + ("home.deleteSheet", 2), + ("sheet.rename", 2), + ("sheet.move", 2), + ("sheet.activate", 3), + ("insert.table", 3), + ("table.totalRow", 1), + ("table.resize", 1), + ("table.convertToRange", 1), + ("insert.chart", 2), + ("chart.delete", 1), + ("data.sortAscending", 2), + ("data.sort", 2), + ("data.filter", 2), + ("data.filterBy", 2), + ("data.clearFilter", 1), + ("data.removeDuplicates", 1), + ("formulas.defineName", 2), + ("formulas.deleteName", 1), + ("edit.replace", 1), +]; + +fn generate(rng: &mut Rng, s: &Session) -> Step { + let wb = wb_of(s); + if rng.chance(82) { + let total: usize = CURATED.iter().map(|(_, w)| w).sum(); + let mut roll = rng.below(total); + for (id, w) in CURATED { + if roll < *w { + return (id.to_string(), curated_params(rng, wb, id)); + } + roll -= w; + } + } + let specs: Vec<_> = command_specs().iter().filter(|c| !EXCLUDED.iter().any(|(x, _)| *x == c.id)).collect(); + let spec = specs[rng.below(specs.len())]; + (spec.id.to_string(), doc_params(rng, wb, spec.params)) +} + +fn curated_params(rng: &mut Rng, wb: &Workbook, id: &str) -> Json { + let maybe_range = |rng: &mut Rng| if rng.chance(70) { json!({"range": range(rng)}) } else { json!({}) }; + match id { + "cell.set" => { + let mut p = json!({"cell": cell(rng).a1(), "input": input(rng, wb)}); + if rng.chance(10) { + p["sheet"] = sheet_param(rng, wb); + } + if rng.chance(4) { + p["array"] = json!(true); + } + p + } + "range.setValues" => json!({"range": near_cell(rng).a1(), "values": values_2d(rng, wb)}), + "range.fill" => json!({"range": near_range(rng).a1(), "input": input(rng, wb)}), + "selection.set" => { + let mut p = json!({"range": range(rng)}); + if rng.chance(15) { + p["range"] = json!(format!("{},{}", near_range(rng).a1(), near_cell(rng).a1())); + } + p + } + "edit.undo" | "edit.redo" => json!({"steps": rng.int(1, 3)}), + "edit.copy" | "edit.cut" => maybe_range(rng), + "edit.paste" => { + if rng.chance(15) { + json!({"at": near_cell(rng).a1(), "text": "1\t2\nx\t=A1"}) + } else { + json!({"at": cell(rng).a1()}) + } + } + "edit.pasteSpecial" => json!({ + "what": rng.pick(&["all", "formulas", "values", "formats", "allExceptBorders", "columnWidths", "valuesAndNumberFormats"]), + "operation": rng.pick(&["none", "add", "multiply"]), + "skipBlanks": rng.chance(30), + "transpose": rng.chance(30), + }), + "edit.autoFill" => { + let src = near_range(rng); + let tgt = RangeRef::new(src.start, CellRef::new(src.end.row + rng.int(0, 6) as u32, src.end.col + rng.int(0, 1) as u32)); + json!({"source": src.a1(), "target": tgt.a1(), "mode": rng.pick(&["series", "copy", "alternate", "formats", "values"])}) + } + "edit.fillSeries" => json!({"range": near_range(rng).a1(), "type": rng.pick(&["linear", "growth", "autofill"]), "step": rng.int(-2, 3)}), + "home.bold" => json!({"range": range(rng)}), + "home.fillColor" => json!({"range": range(rng), "color": color(rng)}), + "home.numberFormat" => json!({"range": range(rng), "format": rng.pick(&["Currency", "Percentage", "Short Date", "Text", "General"])}), + "home.borders" => json!({"range": range(rng), "preset": rng.pick(&["all", "outside", "none", "thickBottom"])}), + "home.insertRows" | "home.deleteRows" | "home.hideRows" | "home.unhideRows" => { + let mut p = json!({"rows": rows_spec(rng)}); + if id == "home.insertRows" && rng.chance(20) { + p["count"] = json!(rng.int(1, 4)); + } + p + } + "home.insertColumns" | "home.deleteColumns" => json!({"cols": cols_spec(rng)}), + "home.insertCells" => json!({"range": near_range(rng).a1(), "shift": rng.pick(&["right", "down", "row", "column"])}), + "home.deleteCells" => json!({"range": near_range(rng).a1(), "shift": rng.pick(&["left", "up", "row", "column"])}), + "home.insertSheet" => { + let mut p = json!({}); + if rng.chance(40) { + p["name"] = json!(new_name(rng)); + } + if rng.chance(40) { + p["before"] = json!(rng.below(wb.sheets.len() + 1)); + } + p + } + "home.deleteSheet" | "sheet.activate" => json!({"sheet": sheet_param(rng, wb)}), + "sheet.rename" => json!({"sheet": sheet_param(rng, wb), "name": new_name(rng)}), + "sheet.move" => json!({"sheet": sheet_param(rng, wb), "to": rng.below(wb.sheets.len() + 1), "copy": rng.chance(35)}), + "insert.table" => { + let mut p = json!({"range": near_range(rng).a1(), "header": rng.chance(80)}); + if rng.chance(40) { + p["name"] = json!(rng.pick(&["T1", "Sales", "Data", "Grades"])); + } + p + } + "table.totalRow" => json!({"table": table_param(rng, wb), "on": rng.chance(50)}), + "table.resize" => json!({"table": table_param(rng, wb), "range": near_range(rng).a1()}), + "table.convertToRange" => json!({"table": table_param(rng, wb)}), + "insert.chart" => { + json!({"range": near_range(rng).a1(), "type": rng.pick(&["column", "line", "pie", "scatter", "bar"]), "at": cell(rng).a1()}) + } + "chart.delete" => { + let ids = chart_ids(wb); + match ids.get(rng.below(ids.len() + 1)) { + Some(id) => json!({"chart": id}), + None => json!({}), + } + } + "data.sortAscending" => { + let mut p = maybe_range(rng); + if rng.chance(50) { + p["column"] = json!(letter(rng.int(0, 9) as u32)); + } + p + } + "data.sort" => json!({ + "range": near_range(rng).a1(), + "header": rng.chance(50), + "keys": [{"column": letter(rng.int(0, 9) as u32), "order": rng.pick(&["asc", "desc"])}], + "orientation": if rng.chance(15) { "columns" } else { "rows" }, + }), + "data.filter" => maybe_range(rng), + "data.filterBy" => { + let column = letter(rng.int(0, 9) as u32); + match rng.below(4) { + 0 => json!({"column": column, "values": [literal(rng), literal(rng)]}), + 1 => json!({"column": column, "custom": {"op": ">", "value": rng.int(0, 60000).to_string()}}), + 2 => json!({"column": column, "top": {"count": rng.int(1, 5)}}), + _ => json!({"column": column, "clear": true}), + } + } + "data.removeDuplicates" => json!({"range": near_range(rng).a1(), "header": rng.chance(50)}), + "formulas.defineName" => { + let sheet = other_sheet(rng, wb); + let r = near_range(rng); + let abs = format!("${}${}:${}${}", letter(r.start.col), r.start.row + 1, letter(r.end.col), r.end.row + 1); + json!({"name": rng.pick(&["Rate", "Totals", "my_list", "Q"]), "refersTo": format!("={sheet}!{abs}")}) + } + "formulas.deleteName" => match wb.names.get(rng.below(wb.names.len() + 1)) { + Some(n) => json!({"name": n.name}), + None => json!({"name": "Nope"}), + }, + "edit.replace" => json!({"what": rng.pick(&["North", "1", "A", "Sales"]), "with": rng.pick(&["South", "2", "B", ""]), "all": true}), + _ => maybe_range(rng), + } +} + +/// Top-level keys of a params doc (`{range?, shift: right|down, count?}`): (key, optional, hint). +fn doc_keys(doc: &str) -> Vec<(String, bool, String)> { + let Some(start) = doc.find('{') else { return vec![] }; + let mut parts = Vec::new(); + let (mut depth, mut in_str, mut cur) = (0i32, false, String::new()); + for ch in doc[start + 1..].chars() { + match ch { + '"' => { + in_str = !in_str; + cur.push(ch); + } + _ if in_str => cur.push(ch), + '{' | '[' | '(' => { + depth += 1; + cur.push(ch); + } + '}' | ']' | ')' if depth == 0 => break, + '}' | ']' | ')' => { + depth -= 1; + cur.push(ch); + } + ',' if depth == 0 => parts.push(std::mem::take(&mut cur)), + _ => cur.push(ch), + } + } + parts.push(cur); + let mut out = Vec::new(); + for part in parts { + let part = part.trim(); + let key: String = part.chars().take_while(|c| c.is_ascii_alphanumeric() || *c == '_').collect(); + if key.is_empty() { + continue; + } + let rest = &part[key.len()..]; + let optional = rest.starts_with('?'); + let hint = rest.trim_start_matches('?').trim_start(); + let hint = hint.strip_prefix(':').unwrap_or(hint).trim().to_string(); + out.push((key, optional, hint)); + } + out +} + +/// Bare-word alternatives in a hint (`asc|desc`, `"Good"|"Bad"`), if it is an enumeration. +fn alternatives(hint: &str) -> Option> { + if !hint.contains('|') { + return None; + } + let alts: Vec = hint + .split('|') + .map(|a| a.split(" (").next().unwrap_or(a).trim().trim_matches('"').trim_end_matches('…').to_string()) + .filter(|a| !a.is_empty() && a.chars().all(|c| c.is_ascii_alphanumeric() || " .".contains(c))) + .collect(); + (!alts.is_empty()).then_some(alts) +} + +/// Params from a command's params doc: values chosen by key name, then by the hint's shape. +fn doc_params(rng: &mut Rng, wb: &Workbook, doc: &str) -> Json { + let mut p = serde_json::Map::new(); + for (key, optional, hint) in doc_keys(doc) { + if optional && rng.chance(40) { + continue; + } + if let Some(v) = doc_value(rng, wb, &key, &hint) { + p.insert(key, v); + } + } + Json::Object(p) +} + +fn doc_value(rng: &mut Rng, wb: &Workbook, key: &str, hint: &str) -> Option { + let lower = key.to_ascii_lowercase(); + let v = match lower.as_str() { + "path" | "base64" | "password" | "html" => return None, + "range" | "source" | "target" | "destination" | "criteria" | "copyto" | "printarea" | "location" | "resultcells" | "changing" + | "rowinput" | "columninput" | "add" | "remove" | "apply" => json!(range(rng)), + "refersto" => json!(format!("={}!$A$1:$B$4", other_sheet(rng, wb))), + "cell" | "at" | "active" | "fromcell" | "set" | "reference" => json!(cell(rng).a1()), + "rows" | "titlerows" if !hint.starts_with('[') => json!(rows_spec(rng)), + "cols" | "titlecols" if !hint.starts_with('[') => json!(cols_spec(rng)), + "sheet" => sheet_param(rng, wb), + "input" | "formula" | "formula1" | "formula2" | "value" | "value2" | "with" => json!(input(rng, wb)), + "table" => table_param(rng, wb), + "chart" | "id" => { + let ids = chart_ids(wb); + json!(ids.get(rng.below(ids.len() + 1)).copied().unwrap_or(rng.int(0, 5) as u32)) + } + "ids" => json!(chart_ids(wb)), + "color" => json!(color(rng)), + "values" if hint.starts_with("[[") => values_2d(rng, wb), + "keys" => json!([{"column": letter(rng.int(0, 9) as u32), "order": rng.pick(&["asc", "desc"])}]), + "column" | "groupby" => json!(letter(rng.int(0, 9) as u32)), + "columns" | "sources" if !hint.contains('|') => json!([letter(rng.int(0, 9) as u32)]), + "points" => json!([[10, 10], [40, 25], [70, 5]]), + "commands" => json!([{"command": "cell.set", "params": {"cell": near_cell(rng).a1(), "input": input(rng, wb)}}]), + "char" => json!("€"), + "name" | "oldname" if alternatives(hint).is_none() => json!(new_name(rng)), + "text" | "title" | "comment" | "author" | "word" | "header" | "footer" | "what" if alternatives(hint).is_none() => { + json!(rng.pick(&["North", "note", "&CPage &P", "x"])) + } + _ => return Some(hint_value(rng, wb, hint)), + }; + Some(v) +} + +fn hint_value(rng: &mut Rng, wb: &Workbook, hint: &str) -> Json { + if let Some(alts) = alternatives(hint) { + return json!(rng.pick(&alts)); + } + if hint.contains("bool") || hint == "true" || hint == "false" { + return json!(rng.chance(50)); + } + if let Some((a, b)) = hint.split_once("..") + && let (Ok(a), Ok(b)) = (a.trim().parse::(), b.trim().split(' ').next().unwrap_or("").parse::()) + { + return json!(rng.int(a, b)); + } + if let Ok(n) = hint.split(' ').next().unwrap_or("").parse::() { + return json!((n * rng.int(0, 3) as f64).round()); + } + if hint.starts_with('[') { + let heads: Vec = tables(wb).iter().flat_map(|(_, t)| t.columns.iter().map(|c| c.name.clone())).collect(); + return match heads.get(rng.below(heads.len() + 1)) { + Some(h) => json!([h]), + None => json!([]), + }; + } + if let Some(ex) = hint.strip_prefix('"').and_then(|h| h.split('"').next()) + && rng.chance(60) + { + return json!(ex); + } + match rng.below(4) { + 0 => json!(rng.chance(50)), + 1 => json!(rng.int(0, 5)), + 2 => json!(near_range(rng).a1()), + _ => json!(literal(rng)), + } +} + +#[test] +fn params_docs_parse_into_keys() { + let keys = doc_keys("{range?, shift: right|down|row|column, input: \"text, number or =formula\"}"); + let names: Vec<_> = keys.iter().map(|(k, o, _)| (k.as_str(), *o)).collect(); + assert_eq!(names, [("range", true), ("shift", false), ("input", false)]); + assert_eq!(alternatives(&keys[1].2), Some(vec!["right".into(), "down".into(), "row".into(), "column".into()])); + // Every registered command's doc parses, and the fuzzer's exclusions name real commands. + for spec in command_specs() { + let _ = doc_keys(spec.params); + } + for (id, _) in EXCLUDED { + assert!(find_command(id).is_some(), "EXCLUDED names unknown command `{id}`"); + } + for (id, _) in CURATED { + assert!(find_command(id).is_some(), "CURATED names unknown command `{id}`"); + } +} diff --git a/crates/xlsx/src/fmla.rs b/crates/xlsx/src/fmla.rs index c483414..be24762 100644 --- a/crates/xlsx/src/fmla.rs +++ b/crates/xlsx/src/fmla.rs @@ -76,7 +76,9 @@ fn expr_to_file(e: Expr) -> String { Some(n) => Expr::Call(n, args), None => Expr::Call(name, args), }, - // Files have no `#` operator: Excel writes `A1#` as `_xlfn.ANCHORARRAY(A1)`. + // Files have no `#` operator: Excel writes `A1#` as `_xlfn.ANCHORARRAY(A1)`, and a spill + // reference that became `#REF!` (its cell was deleted) as plain `#REF!`. + Expr::Unary(gridcraft_formula::UnOp::Spill, r) if matches!(*r, Expr::Error(_)) => *r, Expr::Unary(gridcraft_formula::UnOp::Spill, r) => Expr::Call("_xlfn.ANCHORARRAY".into(), vec![*r]), // Nor an `@` operator: Excel writes `@x` as `_xlfn.SINGLE(x)`. Expr::Unary(gridcraft_formula::UnOp::At, x) => Expr::Call("_xlfn.SINGLE".into(), vec![*x]), @@ -115,6 +117,11 @@ mod tests { assert_eq!(to_file(&Formula::new("SUM(Sheet1!$D$1#)+B2#")), "SUM(_xlfn.ANCHORARRAY(Sheet1!$D$1))+_xlfn.ANCHORARRAY(B2)"); assert_eq!(text_to_file("=Sheet1!$D$1#"), "_xlfn.ANCHORARRAY(Sheet1!$D$1)"); assert_eq!(from_file("SUM(_xlfn.ANCHORARRAY(Sheet1!$D$1))+_xlfn.ANCHORARRAY(B2)"), "SUM(Sheet1!$D$1#)+B2#"); + // Found by the command-sequence fuzzer: deleting the anchor of `H5#` left `=#REF!` in + // the workbook but `ANCHORARRAY(#REF!)` in the file, which read back differently. + let deleted = Formula::from_expr(Expr::Unary(gridcraft_formula::UnOp::Spill, Box::new(Expr::Error(gridcraft_core::CellError::Ref)))); + assert_eq!(deleted.text, "#REF!"); + assert_eq!(to_file(&deleted), "#REF!"); } #[test] diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 8a00f5a..d6dbf7e 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -1,6 +1,6 @@ # Target-app parity: GridCraft vs Microsoft Excel -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-11 · **Change:** minor (community merges: worksheet functions 513 / 523, catalog 274 / 290) · **Target:** Microsoft Excel (Microsoft 365) +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-11 · **Change:** minor (stability: command-sequence fuzzing against undo/redo, XLSX round-trip and full-recalc invariants) · **Target:** Microsoft Excel (Microsoft 365) This is the authoritative parity assessment. [`ROADMAP.md`](../ROADMAP.md) summarizes it, [`gaps.md`](gaps.md) lists every shortfall, and the area checklists hold the detail: @@ -102,7 +102,7 @@ performance 8, stability 8, localization 4, hardware 2, platforms 2, ecosystem 2 | File formats | 15 | ~50% | 80–120 | XLSX read/write is broad but unproven on a real corpus and in Excel; XLSB and ODS are values-only import; no XLS, XML Spreadsheet 2003, SYLK, DIF, PRN; VBA dropped; 100 MB XLSX fails (#175). [file-format-parity.md](file-format-parity.md) | | UI/UX fidelity | 12 | ~55% | 50–80 | Excel-style ribbon, galleries, dialogs, formula bar, status bar; 59 commands with shortcuts vs Excel's 200+; key tips on part of the Home tab only; short context menu; single window. [ui-parity.md](ui-parity.md) | | Performance | 8 | ~35% | 35–55 | [Performance](#performance) | -| Stability | 8 | ~60% | 15–25 | Never-crash rules enforced by lint, every command fuzzed with hostile params, last-resort guard in `Session::execute`. Field reports: Windows Intel GPU crash (#120), error on close (#117), freeze opening from Finder (#166), screen flashing (#76) | +| Stability | 8 | ~60% | 15–25 | Never-crash rules enforced by lint, every command fuzzed with hostile params, random command sequences fuzzed against undo/redo, XLSX round-trip and full-recalc invariants, last-resort guard in `Session::execute`. Field reports: Windows Intel GPU crash (#120), error on close (#117), freeze opening from Finder (#166), screen flashing (#76) | | Localization | 4 | ~10% | 100–165 | Ribbon-only catalog in five languages (ja, zh, ko, pt-BR, ru; ~12% of strings each), CJK/Thai/Hebrew font fallback, no shaping or RTL. [localization-parity.md](localization-parity.md) | | Hardware | 2 | ~40% | 25–40 | GPU UI via wgpu; single-threaded calc; no pen pressure; one window. [hardware-parity.md](hardware-parity.md) | | Platforms | 2 | ~80% | 30–60 | [Platforms](#platforms) | @@ -332,6 +332,7 @@ The inventory carried over from the 2026-10-07 ROADMAP.md, updated for what land | Date | Change | Summary | |---|---|---| +| 2026-10-11 | minor | Stability: command-sequence fuzz harness (`crates/engine/tests/command_sequence_fuzz.rs`) checks random command sequences against undo/redo, XLSX round-trip, full-recalc and structure invariants; the sequence bugs it found are fixed (no score change) | | 2026-10-11 | minor | Worksheet functions 505 / 523 (96.6%; 496 working): ODDFPRICE, ODDFYIELD, ODDLPRICE, ODDLYIELD | | 2026-10-11 | minor | Worksheet functions 503 / 523 (96.2%; 494 working): PERCENTOF, EUROCONVERT | | 2026-10-11 | minor | Worksheet functions 502 / 523 (96.0%; 493 working): BAHTTEXT |