feat: compact MCP batch replies; feature replies name changed and removed bodies (#530) (#599)

MCP batch takes verbose: false for compact replies: feature steps leave out the full body list and sketch steps the ids they repeat outside created. Feature replies now also list changed_bodies and removed_bodies next to created_bodies, so a compact reply still shows every effect of a step. Un-ignores the #530 regression test. Fixes #530.

Co-authored-by: jonaswre <[email protected]>

crates/engine/src/cmd/features.rs

@@ -365,6 +365,8 @@ pub(super) fn add_feature(s: &mut Session, p: &Value, kind: FeatureKind) -> Resu
 /// `add_feature`; `world` is false when `kind` is already in the active component's frame.
 fn add_feature_in(s: &mut Session, p: &Value, kind: FeatureKind, world: bool) -> Result<Value> {
     let name = str_(p, "name");
+    // The bodies before the feature, to say which ones it changed or removed.
+    let before = s.model.state();
     // Commands speak world coordinates; the feature is authored in the active component.
     let mut kind = kind;
     if world {
@@ -474,8 +476,15 @@ fn add_feature_in(s: &mut Session, p: &Value, kind: FeatureKind, world: bool) ->
     let bodies: Vec<&str> = st.bodies.iter().map(|b| b.name.as_str()).collect();
     // The bodies this feature made, apart from the ones it changed or that were there (#374).
     let created: Vec<&str> = st.bodies.iter().filter(|b| b.feature == id).map(|b| b.name.as_str()).collect();
+    // Bodies that were there before and now have other geometry, and bodies that are gone.
+    let old: std::collections::HashMap<&str, &solidcraft_doc::kernel::Body> = before.bodies.iter().map(|b| (b.name.as_str(), &b.body)).collect();
+    let now: std::collections::HashSet<&str> = st.bodies.iter().map(|b| b.name.as_str()).collect();
+    let changed: Vec<&str> =
+        st.bodies.iter().filter(|b| b.feature != id && old.get(b.name.as_str()).is_some_and(|o| !o.same(&b.body))).map(|b| b.name.as_str()).collect();
+    let removed: Vec<&str> = before.bodies.iter().map(|b| b.name.as_str()).filter(|n| !now.contains(n)).collect();
     let fname = s.doc.feature(id).map(|f| f.name.clone()).unwrap_or_default();
-    Ok(json!({"feature": id, "name": fname, "bodies": bodies, "created_bodies": created, "ms": r.map(|r| r.ms)}))
+    Ok(json!({"feature": id, "name": fname, "bodies": bodies, "created_bodies": created, "changed_bodies": changed,
+        "removed_bodies": removed, "ms": r.map(|r| r.ms)}))
 }
 
 fn extrude(s: &mut Session, p: &Value) -> Result<Value> {

crates/engine/src/tests.rs

@@ -2951,6 +2951,25 @@ fn feature_replies_list_only_the_bodies_they_created() {
     assert_eq!(r["bodies"], json!(["Base"]), "{r}");
 }
 
+/// A feature reply also says which existing bodies it changed and which are gone, so a client
+/// that skips the full body list (MCP `batch` with `verbose: false`, #530) still sees every effect.
+#[test]
+fn feature_replies_list_the_bodies_they_changed_and_removed() {
+    let mut s = Session::default();
+    run(&mut s, "solid.box", json!({"length": 10, "width": 10, "height": 10, "body_name": "A"}));
+    run(&mut s, "solid.box", json!({"length": 10, "width": 10, "height": 10, "corner": [20, 0, 0], "operation": "new", "body_name": "B"}));
+    run(&mut s, "solid.box", json!({"length": 10, "width": 10, "height": 10, "corner": [40, 0, 0], "operation": "new", "body_name": "C"}));
+    // A pocket in A changes A only.
+    let r = run(&mut s, "solid.box", json!({"length": 4, "width": 4, "height": 10, "corner": [3, 3, 5], "operation": "cut", "targets": ["A"]}));
+    assert_eq!((&r["created_bodies"], &r["changed_bodies"], &r["removed_bodies"]), (&json!([]), &json!(["A"]), &json!([])), "{r}");
+    // Joining C into B changes B and removes C.
+    let r = run(&mut s, "solid.combine", json!({"target": "B", "tools": ["C"], "operation": "join"}));
+    assert_eq!((&r["changed_bodies"], &r["removed_bodies"]), (&json!(["B"]), &json!(["C"])), "{r}");
+    // A new body changes nothing else.
+    let r = run(&mut s, "solid.box", json!({"length": 1, "width": 1, "height": 1, "corner": [0, 50, 0], "operation": "new", "body_name": "D"}));
+    assert_eq!((&r["created_bodies"], &r["changed_bodies"], &r["removed_bodies"]), (&json!(["D"]), &json!([]), &json!([])), "{r}");
+}
+
 /// Drafting a cylinder's side face 5° from the base plane makes a cone frustum (#547): radius 10
 /// at the base, 10 − 20 · tan 5° at the top.
 #[test]

crates/mcp/src/tests.rs

@@ -459,7 +459,6 @@ fn screenshot_renders_a_saved_named_view() {
 /// is mostly the same body list once per step. A compact batch reply names only what each step
 /// added or changed.
 #[test]
-#[ignore = "fails until #530 is fixed"]
 fn compact_batch_reply_does_not_repeat_the_body_list() {
     let mut s = server();
     let cube =
@@ -480,6 +479,31 @@ fn compact_batch_reply_does_not_repeat_the_body_list() {
     }
 }
 
+/// A compact batch reply keeps what each step did: the body a cut changed, and a sketch
+/// command's new ids once (under `created`); the default reply is unchanged.
+#[test]
+fn compact_batch_reply_keeps_changed_bodies_and_created_ids() {
+    let mut s = server();
+    let steps = json!([
+        {"command": "solid.box", "params": {"length": 10, "width": 10, "height": 10, "body_name": "Base"}},
+        {"command": "solid.box", "params": {"length": 4, "width": 4, "height": 10, "corner": [3, 3, 5], "operation": "cut", "targets": ["Base"]}},
+        {"command": "sketch.create", "params": {"plane": "XY"}},
+        {"command": "sketch.rectangle.two_point", "params": {"p0": [20, 0], "p1": [30, 10]}},
+    ]);
+    let full = tool(&mut s, "batch", json!({"commands": steps}));
+    assert_eq!(full["results"][1]["bodies"], json!(["Base"]), "{full}");
+    assert_eq!(full["results"][3]["curves"], full["results"][3]["created"]["curves"], "{full}");
+    tool(&mut s, "new_design", json!({}));
+    let r = tool(&mut s, "batch", json!({"commands": steps, "verbose": false}));
+    let cut = &r["results"][1];
+    assert!(cut.get("bodies").is_none(), "{r}");
+    assert_eq!(cut["changed_bodies"], json!(["Base"]), "{r}");
+    let rect = &r["results"][3];
+    assert!(rect.get("curves").is_none() && rect.get("constraints").is_none(), "{r}");
+    assert_eq!(rect["created"]["curves"].as_array().map(Vec::len), Some(4), "{r}");
+    assert!(rect["sides"].is_object(), "results that are not repeated stay: {r}");
+}
+
 /// #531: a product model keeps an optional variant hidden in its default presentation. Hiding
 /// it must survive saving and reopening the design (suppressing it would change the design).
 #[test]

crates/mcp/src/tools.rs

@@ -63,7 +63,7 @@ pub fn tool_definitions() -> Vec<Value> {
              created: {points, curves, constraints, dimensions}: the ids of what they added, stable and not reused \
              after a delete (undo rewinds the counters with the design), usable right away \
              in sketch.dimension, sketch.constraint.* and sketch.delete. Feature commands return the feature's id \
-             and name, every body (bodies) and the bodies the feature created (created_bodies).",
+             and name, every body (bodies) and the bodies the feature created, changed and removed (created_bodies, changed_bodies, removed_bodies).",
             obj(
                 json!({
                     "command": string("Command id (see list_commands)"),
@@ -89,6 +89,7 @@ pub fn tool_definitions() -> Vec<Value> {
                         "items": {"type": "object", "properties": {"command": {"type": "string"}, "params": {"type": "object"}}, "required": ["command"]},
                     },
                     "rollback": {"type": "boolean", "description": "Undo completed steps when one fails (default true)"},
+                    "verbose": {"type": "boolean", "description": "false: a compact reply. Feature steps leave out the full body list (created_bodies, changed_bodies and removed_bodies stay), sketch steps the ids they repeat outside `created` (default true)"},
                 }),
                 &["commands"],
             ),
@@ -449,6 +450,7 @@ fn batch(b: &mut dyn Backend, a: &Map<String, Value>) -> Value {
         return text_result(&json!(format!("batch too long ({} steps, at most {MAX_BATCH})", steps.len())), true);
     }
     let rollback = a.get("rollback").and_then(Value::as_bool).unwrap_or(true);
+    let verbose = a.get("verbose").and_then(Value::as_bool).unwrap_or(true);
     let start = if rollback { Undo::begin(b) } else { Undo::None };
     let mut results = Vec::with_capacity(steps.len());
     for (i, step) in steps.iter().enumerate() {
@@ -458,7 +460,7 @@ fn batch(b: &mut dyn Backend, a: &Map<String, Value>) -> Value {
             None => Err("missing `command`".to_string()),
         };
         match r {
-            Ok(v) => results.push(v),
+            Ok(v) => results.push(if verbose { v } else { compact(v) }),
             Err(e) => {
                 let undone = start.rollback(b, i);
                 return text_result(
@@ -479,6 +481,24 @@ fn batch(b: &mut dyn Backend, a: &Map<String, Value>) -> Value {
     text_result(&json!({"ok": true, "completed": results.len(), "results": results}), false)
 }
 
+/// A step's result without what the design already had (`batch` with `verbose: false`): a
+/// feature's full body list (its created, changed and removed bodies stay), and the sketch ids
+/// a sketch command repeats outside `created`.
+fn compact(mut v: Value) -> Value {
+    let Some(o) = v.as_object_mut() else { return v };
+    if o.contains_key("created_bodies") {
+        o.remove("bodies");
+    }
+    if let Some(created) = o.get("created").cloned() {
+        for k in ["points", "curves", "constraints", "dimensions"] {
+            if o.get(k).is_some_and(|x| created.get(k) == Some(x)) {
+                o.remove(k);
+            }
+        }
+    }
+    v
+}
+
 static SHOT: AtomicU64 = AtomicU64::new(0);
 
 /// `screenshot`: render or capture to a file, return it as an MCP image content block.

docs/mcp.md

@@ -50,8 +50,8 @@ Claude Desktop (`claude_desktop_config.json`), bridged to a running app:
 | Tool | Arguments | What it does |
 |---|---|---|
 | `list_commands` | `filter?` | The command catalog: id, label, tab/panel, shortcut, parameter docs, enabled now. `filter` matches id, label, tab and panel. |
-| `execute` | `command`, `params?` | Run any command with JSON parameters (`list_commands` shows them). Failures leave the design unchanged. A feature command replies with the feature (`feature`, `name`), every body (`bodies`) and the bodies it created (`created_bodies`). |
-| `batch` | `commands: [{command, params}]`, `rollback?` (default true) | Run steps in order, stop at the first error and undo the completed steps. The reply names the failing step (`failed.index`, `failed.command`, `failed.error`) and every result before it. |
+| `execute` | `command`, `params?` | Run any command with JSON parameters (`list_commands` shows them). Failures leave the design unchanged. A feature command replies with the feature (`feature`, `name`), every body (`bodies`) and the bodies it created, changed and removed (`created_bodies`, `changed_bodies`, `removed_bodies`). |
+| `batch` | `commands: [{command, params}]`, `rollback?` (default true), `verbose?` (default true) | Run steps in order, stop at the first error and undo the completed steps. The reply names the failing step (`failed.index`, `failed.command`, `failed.error`) and every result before it. `verbose: false` keeps the reply small in large designs: feature steps leave out the full body list (`created_bodies`, `changed_bodies`, `removed_bodies` stay) and sketch steps the ids they repeat outside `created`. |
 | `inspect_design` | `measure?` (default true), `sketches?` (default true) | Parameters, timeline (type, suppressed, rolled back, error, warning, timing), bodies (volume, area, centre of mass, bbox, face/edge/vertex counts, visible), sketches (curves, points, constraints, profiles, DOF, solve status, visible), selection, undo depth. Hide helper sketches, bodies or construction planes with `execute` `browser.visibility` (`{items, visible}` or `{folder, visible}`). |
 | `measure` | `body?` | Volume (mm³), area (mm²), centre of mass, bbox and topology counts per body, with totals. |
 | `body_topology` | `body` | Edges (index, midpoint, length, ends) and faces (index, area, centroid, normal). An edge midpoint is the `[x,y,z]` that picks that edge for fillet and chamfer. |