diff --git a/crates/engine/src/cmd/features.rs b/crates/engine/src/cmd/features.rs index 8bd74271..04c5bb22 100644 --- a/crates/engine/src/cmd/features.rs +++ b/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 { 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 { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 2a374503..9e0b42d3 100644 --- a/crates/engine/src/tests.rs +++ b/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] diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs index dced2e33..d1a3a86d 100644 --- a/crates/mcp/src/tests.rs +++ b/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] diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index 7536f18f..3a98e2dd 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -63,7 +63,7 @@ pub fn tool_definitions() -> Vec { 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 { "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) -> 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) -> 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) -> 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. diff --git a/docs/mcp.md b/docs/mcp.md index b3152415..716dc6df 100644 --- a/docs/mcp.md +++ b/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. |