feat(mcp): host-neutral assembly component tools - #62
Conversation
jackControls
left a comment
There was a problem hiding this comment.
The update ToolSpec schemas permit destructive partial replacement.
assembly_update_component requires only id and name, but deserialization defaults omitted body_ids to [], the local coordinate system to identity, and promoted to false. AssemblyModel::update_component then replaces the whole definition. A valid rename-only call can therefore silently remove every body from the component and reset its coordinate system.
Likewise, assembly_update_occurrence requires only id, name, and component_id; omitted parent, pose, visibility, and grounded values deserialize to defaults before the whole occurrence is replaced. A rename-only call can reparent/reposition/show/unground an occurrence.
Please either require every replacement field in both schemas or introduce patch DTOs that preserve omitted values, with rename-only regression tests. Requesting changes; I did not merge.
|
Addressed CHANGES_REQUESTED with patch DTOs (Option B): |
jackControls
left a comment
There was a problem hiding this comment.
Re-reviewed at b6d6543. The patch DTO semantics, omitted-field preservation, and explicit-null parent clearing now look correct; the targeted assembly and MCP preservation tests pass locally. The remaining blocker is the failed required MCP check: reports formatting diffs in and . Please run and push the result.
jackControls
left a comment
There was a problem hiding this comment.
Re-reviewed at b6d6543. The patch DTO semantics, omitted-field preservation, and explicit-null parent clearing now look correct; the targeted assembly and MCP preservation tests pass locally. The remaining blocker is the failed required MCP check: cargo fmt --all -- --check reports formatting diffs in crates/assembly/src/lib.rs and crates/sketch/src/lib.rs. Please run cargo fmt --all and push the result.
|
fmt only |
Add Direct MCP ToolSpecs for assembly_document, assembly_solution, create/update component and occurrence, set pose, and set grounded so agents can build real assemblies without hand-editing model.json. Joints deferred. Register a dedicated assembly focus pack and cover the flow with a headless roundtrip test.
Introduce ComponentDefinitionPatchDto / ComponentOccurrencePatchDto with optional fields (id required). Host update_component / update_occurrence merge omitted values instead of full-replacing with serde defaults. Rename-only MCP + assembly regression tests cover the wipe bug Jack flagged.
f41edb7 to
695b410
Compare
Stack assembly ToolSpecs (jackControls#62) onto the UI-owned apply / mcp-mutate branch (jackControls#60) so cad_submit accepts assembly_create_component, assembly_update_component, assembly_create_occurrence, assembly_update_occurrence, assembly_set_occurrence_pose, and assembly_set_occurrence_grounded while attached. assembly_document and assembly_solution stay read-safe inspect tools. Add Assembly focus pack and classifier/roundtrip coverage.
|
Rebased onto
|
jackControls
left a comment
There was a problem hiding this comment.
Re-reviewed at 695b410. The patch DTOs now preserve omitted component and occurrence fields, explicit null correctly clears an occurrence parent, validation remains transactional, and formatting is clean. Local verification passed: 32 assembly tests and all 37 MCP-server tests. Approved.
Rebase jackControls#63 onto jackControls#60 tip f241739. Mutate map entries and ToolSpecs already landed with jackControls#60/jackControls#62; keep unique classifier/lookup coverage so cad_submit accepts the six assembly modeling mutates while assembly_document and assembly_solution stay read-safe inspect tools.
Rebase jackControls#63 onto jackControls#60 tip f241739. Mutate map entries and ToolSpecs already landed with jackControls#60/jackControls#62; keep unique classifier/lookup coverage so cad_submit accepts the six assembly modeling mutates while assembly_document and assembly_solution stay read-safe inspect tools.
Rebase jackControls#63 onto jackControls#60 tip f241739. Mutate map entries and ToolSpecs already landed with jackControls#60/jackControls#62; keep unique classifier/lookup coverage so cad_submit accepts the six assembly modeling mutates while assembly_document and assembly_solution stay read-safe inspect tools.
Assembly joints were deferred on jackControls#62. Host already has stable CreateJointRequestDto / UpdateJointRequestDto; this slice adds matching MCP tools plus a headless two-body revolute create/update/query golden.
Rebase jackControls#63 onto jackControls#60 tip f241739. Mutate map entries and ToolSpecs already landed with jackControls#60/jackControls#62; keep unique classifier/lookup coverage so cad_submit accepts the six assembly modeling mutates while assembly_document and assembly_solution stay read-safe inspect tools.
Assembly joints were deferred on jackControls#62. Host already has stable CreateJointRequestDto / UpdateJointRequestDto; this slice adds matching MCP tools plus a headless two-body revolute create/update/query golden.
Summary
host::handle), so agents can build real assemblies (NIST FTC / print-kit style) without hand-editingmodel.json.assembly_document,assembly_solution,assembly_create_component,assembly_update_component,assembly_create_occurrence,assembly_update_occurrence,assembly_set_occurrence_pose,assembly_set_occurrence_grounded.assemblyfocus pack in disclosure (ribbon-aligned); schemas matchCreateComponentRequestDto/ occurrence / pose / grounded DTOs.assembly_create_joint/ delete) deferred to a follow-up when schemas stay small.Test plan
cargo testinmcp-server(32 passed), includingassembly_component_occurrence_grounded_roundtrip(new project → extrude box → create component absorb → set pose/grounded →assembly_documentshows component; headless, no attach)