Merge pull request #2595 from vangel76/mcp-insert-table-2242

MCP: insert_table tool for filled tables (#2242)

ROADMAP.md

@@ -450,6 +450,7 @@ Imported Word 97–2003 `.doc`/`.dot` files now ask for a supported DOCX target
 | 2026-10-18 | Clearing a font query with no matches keeps Recently Used Fonts visible at the top of a scrollable list. The initial scroll position no longer skips its heading; measured keyboard scrolling is unchanged (#2068). |
 | 2026-10-19 | Centred numbered headings after a leading page break keep their number before the text (#2964). The label uses the alignment of the line it is drawn on; heading text positions and page count stay the same. |
 | 2026-10-17 | Edit Recipient List finds records (#1576): a Find in list box hides rows with no cell containing the text (any case) and counts the matches, and "Show only records that will be merged" hides the rows the filter leaves out; both only change the view. Rows are striped, and edits and removes go to the right record however the grid is narrowed |
+| 2026-10-16 | MCP `insert_table` tool (#2242): a filled table in one call from `values`, with `header` (repeating first row) and `style`; `insert.spreadsheet` takes the same `values`/`header`/`style` and rejects ragged, oversized or non-array input |
 | 2026-10-17 | AccessKit focus guard (#2726): every tree update egui sends carries its focused node, so a frame that drops it falls back to the root instead of panicking in `accesskit_consumer` and closing the window on Windows |
 | 2026-10-17 | Equation arrays centre a row that has no `&` among rows that do, as Word does, so case equations like Word's "First Property of Radicals" no longer push the aligned row right (#2617). |
 | 2026-10-17 | Formatting revisions: a table, row, cell or section property change made while tracking now records as `w:tblPrChange`/`w:trPrChange`/`w:tcPrChange`/`w:sectPrChange` (structure stays out), so Reject All restores and Accept All keeps it; test tracked_table_changes_are_kept_as_formatting_changes |

crates/engine/src/cmd/tools/actions.rs

@@ -341,3 +341,42 @@ pub(super) fn customize_ribbon(s: &mut Session, v: &Value) -> CmdResult {
     }
     Ok(Value::Object(out))
 }
+
+/// The rows of an `insert.spreadsheet` call: `values` (a rectangular array of cells) or `csv`.
+pub(super) fn spreadsheet_rows(v: &Value) -> Result<Vec<Vec<String>>, CmdError> {
+    // The CSV fallback is for an absent `values`; an explicit null is a malformed argument.
+    let Some(values) = v.get("values") else {
+        return Ok(crate::cmd::mailings::parse_csv(p::str(v, "csv").unwrap_or("A,B,C\n1,2,3\n4,5,6")));
+    };
+    let list = values.as_array().ok_or_else(|| CmdError::Params("`values` must be an array of rows".into()))?;
+    if list.is_empty() || list.len() > 1000 {
+        return Err(CmdError::Params("`values` needs 1 to 1000 rows".into()));
+    }
+    let mut rows: Vec<Vec<String>> = Vec::with_capacity(list.len());
+    for (r, row) in list.iter().enumerate() {
+        let cells = row.as_array().ok_or_else(|| CmdError::Params(format!("`values` row {} is not an array", r + 1)))?;
+        if cells.is_empty() || cells.len() > 63 {
+            return Err(CmdError::Params(format!("`values` row {} needs 1 to 63 cells", r + 1)));
+        }
+        if rows.first().is_some_and(|f| f.len() != cells.len()) {
+            return Err(CmdError::Params(format!(
+                "`values` row {} has {} cells, row 1 has {}",
+                r + 1,
+                cells.len(),
+                rows.first().map_or(0, Vec::len)
+            )));
+        }
+        let mut out = Vec::with_capacity(cells.len());
+        for cell in cells {
+            out.push(match cell {
+                Value::String(x) => x.clone(),
+                Value::Number(x) => x.to_string(),
+                Value::Bool(x) => x.to_string(),
+                Value::Null => String::new(),
+                _ => return Err(CmdError::Params(format!("`values` row {} holds a cell that is not text, a number or a boolean", r + 1))),
+            });
+        }
+        rows.push(out);
+    }
+    Ok(rows)
+}

crates/engine/src/cmd/tools/specs.rs

@@ -1,5 +1,6 @@
 use super::actions::{
     customize_keyboard, customize_ribbon, hf_nav, language, macros, preferred_language, quick_parts, search_commands, select_similar,
+    spreadsheet_rows,
 };
 use super::autocorrect::autocorrect_options;
 use super::compare::{COMPARE_PARAMS, compare};
@@ -213,10 +214,10 @@ pub fn specs() -> Vec<CommandSpec> {
         })
         .params(r#"{"path": string}"#),
         CommandSpec::new("insert.spreadsheet", "Spreadsheet Table", "Insert › Tables", |s, v| {
-            let csv = p::str(v, "csv").unwrap_or("A,B,C\n1,2,3\n4,5,6");
-            let rows = crate::cmd::mailings::parse_csv(csv);
+            let rows = spreadsheet_rows(v)?;
             let cols = rows.iter().map(Vec::len).max().unwrap_or(1).clamp(1, 63);
-            s.run("insert.table", &json!({"rows": rows.len().max(1), "cols": cols, "style": "GridTable1Light"}))?;
+            let style = p::str(v, "style").unwrap_or("GridTable1Light");
+            s.run("insert.table", &json!({"rows": rows.len().max(1), "cols": cols, "style": style}))?;
             let Some((tp, _, _)) = s.sel.focus.path.cell() else { return sel_result(s) };
             let t = s.doc.table_mut(s.sel.focus.story, &tp)?;
             for (r, row) in rows.iter().enumerate() {
@@ -226,9 +227,14 @@ pub fn specs() -> Vec<CommandSpec> {
                     }
                 }
             }
+            if p::bool(v, "header").unwrap_or(false)
+                && let Some(first) = t.rows.first_mut()
+            {
+                first.props.header = true;
+            }
             sel_result(s)
         })
-        .params(r#"{"csv"?: string}"#),
+        .params(r#"{"csv"?: string, "values"?: [[cell, …], …] (rows of equal length, at most 1000 rows × 63 columns; wins over csv), "header"?: bool (first row repeats on each page), "style"?: table style name (default GridTable1Light)}"#),
         CommandSpec::new("layout.linkToPrevious", "Link to Previous", "Header & Footer › Navigation", |s, v| {
             let header = p::bool(v, "footer").map(|f| !f).unwrap_or(true);
             let block = s.sel.focus.path.0.first().copied().unwrap_or(0) as usize;

crates/engine/src/cmd/tools/tests.rs

@@ -255,6 +255,59 @@ fn proofing_language_options() {
     assert!(s.run("review.language", &json!({"lang": "<script>"})).is_err());
 }
 
+/// #2242: `insert.spreadsheet` fills a table from `values`, with a repeating header and a style.
+#[test]
+fn spreadsheet_takes_values_header_and_style() {
+    let mut s = Session::new(Document::from_text(""));
+    s.run("insert.spreadsheet", &json!({"values": [["Month", "Cases"], ["Jan", 12], ["Feb", null]], "header": true, "style": "TableGrid"})).unwrap();
+    let Some(Block::Table(t)) = s.doc.body.iter().map(|b| &**b).find(|b| matches!(b, Block::Table(_))) else { panic!("no table") };
+    assert_eq!((t.rows.len(), t.rows[0].cells.len()), (3, 2));
+    assert!(t.rows[0].props.header && !t.rows[1].props.header);
+    assert_eq!(t.rows[1].cells[1].blocks[0].as_para().unwrap().text, "12");
+    assert_eq!(t.props.style.as_deref(), Some("TableGrid"));
+}
+
+/// An explicit `values: null` is a parameter error, not the legacy CSV sample table: the document
+/// and the undo history stay as they were. Only an absent `values` falls back to `csv`.
+#[test]
+fn spreadsheet_rejects_an_explicit_null_values_without_changing_anything() {
+    let mut s = Session::new(Document::from_text("keep"));
+    let depth = s.undo_depth();
+    for args in [json!({"values": null}), json!({"values": null, "csv": "a,b\n1,2"})] {
+        assert!(s.run("insert.spreadsheet", &args).is_err(), "{args}");
+        assert_eq!(s.doc.plain_text(StoryRef::Body), "keep");
+        assert!(!s.doc.body.iter().any(|b| matches!(&**b, Block::Table(_))));
+        assert_eq!(s.undo_depth(), depth, "{args}: no undo step");
+    }
+    s.run("insert.spreadsheet", &json!({"csv": "a,b\n1,2"})).unwrap();
+    let Some(Block::Table(t)) = s.doc.body.iter().map(|b| &**b).find(|b| matches!(b, Block::Table(_))) else { panic!("no table") };
+    assert_eq!((t.rows.len(), t.rows[0].cells.len()), (2, 2), "an absent `values` still takes the csv");
+}
+
+/// #2242: malformed `values` is an error, never a panic, and leaves the document alone.
+#[test]
+fn spreadsheet_rejects_hostile_values() {
+    let wide: Vec<Value> = (0..64).map(|i| json!(i)).collect();
+    let many: Vec<Value> = (0..1001).map(|_| json!(["x"])).collect();
+    for bad in [
+        json!("x"),
+        json!(5),
+        json!([]),
+        json!(["a"]),
+        json!([["a", "b"], ["c"]]),
+        json!([wide]),
+        json!(many),
+        json!([[{"a": 1}]]),
+        json!([[[1]]]),
+        json!([[]]),
+    ] {
+        let mut s = Session::new(Document::from_text("keep"));
+        assert!(s.run("insert.spreadsheet", &json!({"values": bad})).is_err(), "{bad}");
+        assert_eq!(s.doc.plain_text(StoryRef::Body), "keep");
+        assert!(!s.doc.body.iter().any(|b| matches!(&**b, Block::Table(_))));
+    }
+}
+
 /// The inspector's removals delete straight from the document, so they aren't tracked: on a
 /// document that tracks changes the text would go with no revision to show for it, and the
 /// revision trail would be broken behind the user's back. The removal is refused instead,

crates/mcp/src/tests.rs

@@ -356,6 +356,70 @@ fn tools_address_an_open_document_by_id() {
     assert_eq!(tool(&mut s, "get_text", json!({"doc": first}))["text"].as_str().unwrap().trim(), "First more again");
 }
 
+/// #2242: `insert_table` fills a table in one call; the header row and style survive a DOCX round trip.
+#[test]
+fn insert_table_fills_a_table_and_survives_docx() {
+    let mut s = Server::new(Box::new(Headless::default()));
+    tool(&mut s, "new_document", json!({}));
+    tool(&mut s, "insert_table", json!({"values": [["Month", "Cases"], ["January", 12], ["February", 7]], "header": true, "style": "TableGrid"}));
+    let path = std::env::temp_dir().join(format!("wordcraft-mcp-table-{}.docx", std::process::id()));
+    let path_str = path.to_str().expect("utf-8 path").to_string();
+    tool(&mut s, "save_document", json!({"path": path_str}));
+    let mut s2 = Server::new(Box::new(Headless::default()));
+    tool(&mut s2, "open_document", json!({"path": path_str}));
+    let doc = tool(&mut s2, "inspect_document", json!({}));
+    let table = doc["blocks"].as_array().unwrap().iter().find(|b| b["type"] == "table").expect("a table");
+    let text = table.to_string();
+    for cell in ["Month", "Cases", "January", "12", "February", "7"] {
+        assert!(text.contains(cell), "{cell} missing in {text}");
+    }
+    assert!(text.contains("TableGrid"), "{text}");
+    // The header flag is not in `inspect_document`; read it from the reopened document.
+    let mut h = Headless::default();
+    h.session.run("file.open", &json!({"path": path_str})).expect("reopened");
+    let _ = std::fs::remove_file(&path);
+    let table = h.session.doc.body.iter().find_map(|b| match &**b {
+        wordcraft_doc::Block::Table(t) => Some(t.clone()),
+        _ => None,
+    });
+    let table = table.expect("a table");
+    assert!(table.rows[0].props.header && !table.rows[1].props.header);
+}
+
+/// #2242: malformed `insert_table` arguments are tool errors, not panics.
+#[test]
+fn insert_table_rejects_hostile_values() {
+    let mut s = Server::new(Box::new(Headless::default()));
+    let wide: Vec<Value> = (0..64).map(|i| json!(i)).collect();
+    for args in [
+        json!({}),
+        json!({"values": "x"}),
+        json!({"values": [["a", "b"], ["c"]]}),
+        json!({"values": [wide]}),
+        json!({"values": [[{"a": 1}]]}),
+        json!({"values": []}),
+    ] {
+        let r = call(&mut s, 12, "tools/call", json!({"name": "insert_table", "arguments": args}));
+        assert_eq!(r["result"]["isError"], true, "{args}: {r}");
+    }
+}
+
+/// An explicit `values: null` is a tool error and changes nothing: no table, and the undo history
+/// still ends with the earlier edit (one Undo takes that back, not a phantom table insertion).
+#[test]
+fn insert_table_rejects_null_values_without_changing_the_document() {
+    let mut s = Server::new(Box::new(Headless::default()));
+    tool(&mut s, "new_document", json!({}));
+    tool(&mut s, "type_text", json!({"text": "keep"}));
+    let r = call(&mut s, 13, "tools/call", json!({"name": "insert_table", "arguments": {"values": null}}));
+    assert_eq!(r["result"]["isError"], true, "{r}");
+    assert_eq!(paragraphs(&mut s).len(), 1, "no table or paragraph added");
+    assert_eq!(tool(&mut s, "get_text", json!({}))["text"].as_str().unwrap(), "keep");
+    assert!(!tool(&mut s, "inspect_document", json!({}))["blocks"].to_string().contains("table"));
+    tool(&mut s, "execute", json!({"command": "edit.undo"}));
+    assert_eq!(tool(&mut s, "get_text", json!({}))["text"].as_str().unwrap(), "", "the undo step is the typing, not an insert");
+}
+
 /// #3676: a page number that cannot fit an index is rejected, not silently rendered as
 /// page 1 (the old cast truncated 2^32 to 0 on 32-bit and wasm).
 #[test]

crates/mcp/src/tools.rs

@@ -75,10 +75,24 @@ pub fn tool_definitions() -> Value {
         tool(
             "execute",
             "Run a command",
-            "Run one command by id with params, e.g. {command:\"format.bold\"} or {command:\"insert.table\", params:{rows:3, cols:4}}. Commands work on the active document, or on the open document `doc` names. file.new and file.open open another document beside the open ones and make it active; view.switchWindows {id} makes another one active and file.closeDocument {id, force?} closes one.",
+            "Run one command by id with params, e.g. {command:\"format.bold\"} or {command:\"insert.table\", params:{rows:3, cols:4}}. To insert a table that is already filled, use `insert_table`. Commands work on the active document, or on the open document `doc` names. file.new and file.open open another document beside the open ones and make it active; view.switchWindows {id} makes another one active and file.closeDocument {id, force?} closes one.",
             with_doc(obj(json!({"command": s("Command id"), "params": {"type": "object", "description": "Command parameters"}}), &["command"])),
             false
         ),
+        tool(
+            "insert_table",
+            "Insert filled table",
+            "Insert a table at the caret, filled from `values` in one call (an array of rows, each an array of cells; all rows the same length, up to 1000 rows and 63 columns). Set `header` to make the first row a repeating header row.",
+            with_doc(obj(
+                json!({
+                    "values": {"type": "array", "description": "Rows of cells (text, numbers or booleans)", "items": {"type": "array", "items": {"type": ["string", "number", "boolean", "null"]}}},
+                    "header": {"type": "boolean", "description": "First row is a header row that repeats on each page"},
+                    "style": s("Table style name (default GridTable1Light)")
+                }),
+                &["values"]
+            )),
+            false
+        ),
         tool(
             "batch",
             "Run several commands",
@@ -238,6 +252,18 @@ pub fn call_tool(b: &mut dyn Backend, name: &str, a: &Value) -> ToolResult {
             Some(c) => wrap(exec(b, c, a.get("params").cloned().unwrap_or(json!({})), doc)),
             None => ToolResult::error("missing `command`"),
         },
+        "insert_table" => match a.get("values") {
+            Some(values) => {
+                let mut params = json!({"values": values});
+                for k in ["header", "style"] {
+                    if let Some(x) = a.get(k) {
+                        params[k] = x.clone();
+                    }
+                }
+                wrap(exec(b, "insert.spreadsheet", params, doc))
+            }
+            None => ToolResult::error("missing `values`: an array of rows, each an array of cells"),
+        },
         "batch" => {
             let keep = a.get("keepGoing").and_then(Value::as_bool).unwrap_or(false);
             let mut out = Vec::new();

docs/mcp.md

@@ -149,7 +149,8 @@ test source in File › Options › Language and on the bar's Install.
 | Tool | What it does |
 |---|---|
 | `list_commands` | every command (filter with `query`) — over 270 of them |
-| `execute` | run one command: `{command: "insert.table", params: {rows: 3, cols: 4}}` |
+| `execute` | run one command: `{command: "insert.table", params: {rows: 3, cols: 4}}` (an empty table; for a filled one use `insert_table`) |
+| `insert_table` | insert a filled table in one call: `{values: [["Month", "Cases"], ["January", 12]], header: true, style?}` (same as `insert.spreadsheet`) |
 | `batch` | run several commands in order |
 | `new_document` | blank, sample, letter, resume, report; opens beside the open documents |
 | `open_document` / `save_document` | docx, doc (open only), odt, rtf, html, md, tex, txt, json; save also pdf, png |
@@ -172,7 +173,7 @@ document beside the ones already open and make it the active one; their reply ca
 active again (`alreadyOpen`). `list_documents` lists the open documents with their ids.
 
 Tools work on the active document. `execute`, `batch`, `type_text`, `select_text`, `get_text`,
-`inspect_document`, `render_page` and `save_document` also take `doc`, the id of another open
+`insert_table`, `inspect_document`, `render_page` and `save_document` also take `doc`, the id of another open
 document: the tool works on that document and the active one stays active.
 
 ```json