From 482c3e94817e94d168b736d69b98b529a51f9698 Mon Sep 17 00:00:00 2001 From: quantmind-br Date: Tue, 13 Oct 2026 09:38:41 -0300 Subject: [PATCH] Engine: formulas.evaluate refuses what cell entry refuses, saying why `formulas.evaluate` (behind `gridcraft-cli eval` and MCP's `evaluate_formula`) turned any formula that didn't parse into #NAME?, so `=SUM(1;2)` looked like an unknown function. It now parses the way `cell.set` does (missing closing parentheses are added) and returns the same errors: "There's a problem with this formula: expected `,` or `)`, found Semicolon", or the wrong-argument-count message. An unknown function still evaluates to #NAME?, as in Excel. --- ROADMAP.md | 1 + crates/engine/src/cmd/edit.rs | 30 +++++++++++++++++------------- crates/engine/src/cmd/formulas.rs | 9 +++++++-- crates/engine/src/tests.rs | 18 ++++++++++++++++++ crates/mcp/src/tools.rs | 3 ++- docs/cli.md | 2 +- docs/mcp.md | 2 +- 7 files changed, 47 insertions(+), 18 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 070552d07..555b7171d 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-13 | `formulas.evaluate` (`gridcraft-cli eval`, MCP `evaluate_formula`) refuses what entering the formula in a cell refuses, with the reason (`=SUM(1;2)`: "expected `,` or `)`"), instead of `#NAME?` for any formula that doesn't parse | | 2026-10-12 | Rows auto-fit oversized fonts: opening a file and whole-row/column formatting grow non-custom rows to their tallest font (#833) | | 2026-10-12 | Number formats: parsing a format code is linear in its length (long runs of `[`, `,` or `/` no longer take quadratic time), and an unclosed `[` is a literal when splitting sections as well as when tokenizing (#810) | | 2026-10-12 | Number formats: a fraction denominator with 65,536 zeros no longer crashes; format codes are capped at 255 characters like Excel (longer codes display as General, `TEXT` gives `#VALUE!`) (#807) | diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 82dbe0be7..5570bf3fa 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -138,18 +138,7 @@ pub(crate) fn input_to_cell(input: &str, sheet: usize, at: CellRef, wb: &mut gri && input.chars().nth(1).is_some_and(|c| c.is_ascii_alphabetic() || c == '(')); if is_formula { let body = input.strip_prefix('=').unwrap_or(input); - // Excel closes missing parentheses for you. - let mut text = body.to_string(); - let mut parsed = gridcraft_formula::parse(&text); - for _ in 0..8 { - if parsed.is_ok() { - break; - } - text.push(')'); - parsed = gridcraft_formula::parse(&text); - } - let expr = parsed.map_err(|e| EngineError::Other(format!("There's a problem with this formula: {e}")))?; - let expr = wb.name_call_case(expr); + let expr = wb.name_call_case(parse_typed_formula(body)?); check_arg_counts(&expr)?; let mut cell = Cell::formula(Formula::from_expr(expr)); cell.style = style; @@ -191,11 +180,26 @@ pub(crate) fn input_to_cell(input: &str, sheet: usize, at: CellRef, wb: &mut gri Ok(Some(Cell { value: parsed.value, formula: None, style })) } +/// Parses a formula as it is typed (without the `=`). Excel closes missing parentheses for you, +/// and refuses a formula that still doesn't parse. +pub(crate) fn parse_typed_formula(body: &str) -> Result { + let mut text = body.to_string(); + let mut parsed = gridcraft_formula::parse(&text); + for _ in 0..8 { + if parsed.is_ok() { + break; + } + text.push(')'); + parsed = gridcraft_formula::parse(&text); + } + parsed.map_err(|e| EngineError::Other(format!("There's a problem with this formula: {e}"))) +} + /// Like Excel, a formula that calls a built-in function with too few or too many arguments is /// refused when it's entered, rather than stored to show `#VALUE!`. Functions the registry doesn't /// hold (special forms like LET, LAMBDA, OFFSET, INDIRECT, SUBTOTAL), defined LAMBDA names and /// unknown functions are skipped. -fn check_arg_counts(expr: &gridcraft_formula::Expr) -> Result<()> { +pub(crate) fn check_arg_counts(expr: &gridcraft_formula::Expr) -> Result<()> { let mut bad = None; expr.walk(&mut |e| { if bad.is_none() diff --git a/crates/engine/src/cmd/formulas.rs b/crates/engine/src/cmd/formulas.rs index 40df72df0..5f1be0162 100644 --- a/crates/engine/src/cmd/formulas.rs +++ b/crates/engine/src/cmd/formulas.rs @@ -53,7 +53,7 @@ pub fn specs() -> Vec { cmd!("formulas.showFormulas", "Show Formulas", ["Formulas", "Formula Auditing"], Some("Ctrl+`"), "{on?}", has_doc, show_formulas), cmd!(query "formulas.errorChecking", "Error Checking", ["Formulas", "Formula Auditing"], None, "{} → cells with errors", has_doc, error_checking), cmd!(query "formulas.evaluateFormula", "Evaluate Formula", ["Formulas", "Formula Auditing"], None, "{cell?, formula?} → steps", has_doc, evaluate_formula), - cmd!(query "formulas.evaluate", "Evaluate", [], None, "{formula: \"=SUM(A1:A3)\", cell?} → value without changing the sheet", has_doc, evaluate), + cmd!(query "formulas.evaluate", "Evaluate", [], None, "{formula: \"=SUM(A1:A3)\", cell?} → value without changing the sheet; a formula that couldn't be entered in a cell is an error", has_doc, evaluate), cmd!(noundo "formulas.calculateNow", "Calculate Now", ["Formulas", "Calculation"], Some("F9"), "{}", has_doc, calc_now), cmd!(noundo "formulas.calculateSheet", "Calculate Sheet", ["Formulas", "Calculation"], Some("Shift+F9"), "{}", has_doc, calc_now), cmd!( @@ -540,11 +540,16 @@ fn evaluate_formula(s: &mut Session, p: &Json) -> Result { Ok(json!({"formula": format!("={text}"), "steps": steps, "result": crate::cmd::inspect::value_json(&result)})) } +/// Evaluates what typing `formula` into a cell would show. A formula `cell.set` would refuse (it +/// doesn't parse, or calls a function with the wrong number of arguments) is an error that says +/// why, not `#NAME?`; an unknown function name still evaluates to `#NAME?`, as in Excel. fn evaluate(s: &mut Session, p: &Json) -> Result { let d = s.doc()?; let f = str_param(p, "formula").ok_or_else(|| bad("formulas.evaluate", "missing `formula`"))?; let at = cell_param(s, p, "cell")?.unwrap_or(d.selection.active); - let v = gridcraft_calc::evaluate(&d.wb, d.wb.active_sheet, at, f); + let expr = crate::cmd::edit::parse_typed_formula(f.strip_prefix('=').unwrap_or(f))?; + crate::cmd::edit::check_arg_counts(&expr)?; + let v = gridcraft_calc::recalc::evaluate_expr(&d.wb, d.wb.active_sheet, at, &expr); Ok(crate::cmd::inspect::value_json(&v)) } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index ae499a189..1de25d2f3 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -241,6 +241,24 @@ fn formulas_with_the_wrong_argument_count_are_refused_on_entry() { assert_eq!(v(&s, "C1"), Value::text("=DATEDIF(1,2)")); } +#[test] +fn evaluate_refuses_what_entry_refuses_and_says_why() { + // `formulas.evaluate` (the CLI's `eval`, MCP's evaluate_formula) gave #NAME? for any formula + // that didn't parse, which hid the cause: `=SUM(1;2)`, with the `;` a French or German Excel + // uses, looked like an unknown function. + let mut s = s(); + let eval = |s: &mut Session, f: &str| s.execute("formulas.evaluate", json!({"formula": f})); + let e = eval(&mut s, "=SUM(1;2)").unwrap_err().to_string(); + assert!(e.contains("There's a problem with this formula") && e.contains("expected `,`"), "{e}"); + let e = eval(&mut s, "=DATEDIF(1,2)").unwrap_err().to_string(); + assert!(e.contains("too few arguments"), "{e}"); + // What entry accepts evaluates: an unknown function is #NAME?, as in Excel, missing closing + // parentheses are added, and the `=` is optional. + assert_eq!(eval(&mut s, "=SOMME(1,2)").unwrap(), json!({"error": "#NAME?"})); + assert_eq!(eval(&mut s, "=SUM(1,2").unwrap(), json!(3.0)); + assert_eq!(eval(&mut s, "1/4").unwrap(), json!(0.25)); +} + #[test] fn undo_redo() { let mut s = s(); diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index e6322f016..a6879b3ff 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -196,7 +196,8 @@ pub fn tool_definitions() -> Vec { "evaluate_formula", "Evaluate formula", "Evaluate a formula against the active sheet WITHOUT changing any cell, e.g. \"=SUM(B2:B10)/COUNT(B2:B10)\" or \ - \"=VLOOKUP(\\\"x\\\",A:B,2,FALSE)\". `cell` sets the formula's position for relative references and ROW()/COLUMN().", + \"=VLOOKUP(\\\"x\\\",A:B,2,FALSE)\". `cell` sets the formula's position for relative references and ROW()/COLUMN(). \ + Use English function names and `,` between arguments; a formula that couldn't be entered in a cell is an error saying why.", obj( json!({"formula": string("Formula text, with or without the leading ="), "cell": string("Evaluate as if in this cell (default: active cell)")}), &["formula"], diff --git a/docs/cli.md b/docs/cli.md index ec9578d60..ad56f0d56 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -24,7 +24,7 @@ cargo install --path apps/gridcraft-cli # or: cargo run -p gridcraft-cli -- |---|---| | `info [--json]` | `gridcraft-cli info book.xlsx` — sheets, used ranges, cell counts, tables, charts, names. | | `convert [--sheet NAME]` | `gridcraft-cli convert book.xlsx data.csv --sheet Sales` — output format from the extension: `.xlsx`, `.csv`, `.tsv`, `.json`, `.html`. | -| `eval [--in FILE] [--sheet S] [--cell A1] [--json]` | `gridcraft-cli eval '=PMT(5%/12,360,-300000)'`; `gridcraft-cli eval '=SUM(B2:B9)' --in book.xlsx` | +| `eval [--in FILE] [--sheet S] [--cell A1] [--json]` | `gridcraft-cli eval '=PMT(5%/12,360,-300000)'`; `gridcraft-cli eval '=SUM(B2:B9)' --in book.xlsx` — formulas in English with `,` between arguments. A formula that couldn't be entered in a cell (it doesn't parse, or a function gets the wrong number of arguments) is an error saying why; an unknown function is `#NAME?`, as in Excel. | | `cat [--range R] [--sheet S] [--formulas] [--csv]` | `gridcraft-cli cat book.xlsx --range A1:F20` — aligned table of displayed values. | | `check ... [--json] [--tolerance 1e-9] [--max-report N]` | `gridcraft-cli check corpus/` — calc oracle and XLSX round-trip fidelity, see below. | | `run …` | see below | diff --git a/docs/mcp.md b/docs/mcp.md index 3e6e7bc11..1877cf228 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -45,7 +45,7 @@ with a file open. | `read_range` | `range?`, `sheet?`, `formulas?`, `formatted?` | Rows of values (default: the used range). | | `write_range` | `range`, `values` (2-D) | `range.setValues`; strings starting with `=` are formulas. | | `set_cell` / `get_cell` | `cell`, `input` / `cell` | Enter one cell / read value, formula, display text, style. | -| `evaluate_formula` | `formula`, `cell?` | Evaluates without changing the sheet. | +| `evaluate_formula` | `formula`, `cell?` | Evaluates without changing the sheet. A formula that couldn't be entered in a cell (it doesn't parse, or a function gets the wrong number of arguments) is an error saying why. | | `select` | `range`, `active?` | Sets the selection. | | `format_range` | `range`, `bold?`, `italic?`, `underline?`, `fontName?`, `fontSize?`, `fontColor?`, `fillColor?`, `numberFormat?`, `horizontalAlign?`, `wrapText?`, `borders?` | Composes the `home.*` formatting commands. | | `insert_chart` | `range`, `type`, `title?`, `subtype?`, `at?` | `insert.chart`. |