feat: MCP STEP import, forward tool-script dump, and solid compare - #61
Conversation
jackControls
left a comment
There was a problem hiding this comment.
The forward script is not portable after a snapshot attach. records_in_script at mcp-server/src/main.rs:2617 excludes cad_detach but still records successful cad_attach and cad_refresh calls. cad_script can therefore begin with an ephemeral session UUID and depend on mutable files outside the script; replaying it on another machine/process can fail immediately or load a different snapshot, contrary to the advertised successful mutating tool sequence.
Please exclude cad_attach/cad_refresh as session-control reads, or convert attach into a portable cad_load_project_model entry containing the loaded model and define when the trace resets. Add a regression that attaches and refreshes a snapshot, performs a modeling operation, and verifies the dumped script contains only replayable modeling state. Requesting changes; I did not merge.
Expose the existing import_step body feature over MCP, record successful mutating tool calls as cad_script, and summarize scene bbox/mesh counts so a rebuilt history can be checked against an imported reference solid. Import stays a dumb body; this does not reverse-engineer feature history from STEP.
Exclude cad_attach/cad_refresh from forward script recording so dumped scripts contain replayable modeling ops only, not ephemeral session UUIDs. Document that contract in mcp-harness, add an attach+refresh+mutate regression, and finish solid_move_copy / solid_edit_move_copy ToolSpecs with disclosure tags and a unit test.
a27bb23 to
f7363c3
Compare
|
Addressed the CHANGES_REQUESTED review:
|
jackControls
left a comment
There was a problem hiding this comment.
The UUID leak is fixed, but the dumped script is still not replayable after attach/refresh.
cad_attach and cad_refresh replace self.manager from model.json (load_snapshot_model), while records_in_script now omits both operations and tool_trace is neither reset nor seeded with a portable baseline. In the new regression, the emitted script therefore contains solid_mirror with a body ID that only exists in the omitted snapshot. Replaying those calls on a fresh CadServer fails because the script never creates that body. The test only asserts that attach/refresh/UUID are absent; it never replays the returned calls.
Please make snapshot loads establish a replayable trace baseline—e.g. clear/seed the trace with cad_load_project_model carrying the loaded model_json, with a defined refresh policy—and add a test that executes every emitted call against a fresh server and compares the result. Requesting changes on the current head; I did not merge.
Clear/seed tool_trace with cad_load_project_model + loaded model_json when snapshot loads so dumped scripts replay on a fresh CadServer. Refresh replaces that baseline the same way. Regression executes every emitted call and compares solid metrics.
|
Addressed the latest CHANGES_REQUESTED:
|
Stack STEP import + solid move/copy ToolSpecs (jackControls#61) onto the fixed assembly submit-map tip (jackControls#63 @ 19800b9) so cad_submit accepts solid_import_step, solid_edit_import_step, solid_move_copy, and solid_edit_move_copy while attached. cad_script and cad_compare_solids stay read-safe control helpers (not inbox mutates). Upgrade cad_script to jackControls#61 @ 52266b2 behavior: attach/refresh seeds a cad_load_project_model baseline, and the e2e test actually replays the dumped script on a fresh CadServer (detach after attach so jackControls#60 session_read_only still holds). Bump MODELING_TOOL_COUNT and keep ToolSpec↔map sync.
jackControls
left a comment
There was a problem hiding this comment.
Re-reviewed at 52266b2. Attach/refresh now seed replay with a portable snapshot, session-only calls remain excluded, and the regression test replays the complete dump on a fresh server. I also ran the full MCP server suite locally: 34/34 tests passed. Approved.
jackControls
left a comment
There was a problem hiding this comment.
Re-reviewed at 52266b2. Attach and refresh now seed replay with a portable cad_load_project_model snapshot, session-only calls remain excluded, and the regression test replays the complete dump on a fresh server. I also ran the full MCP server suite locally: 34 of 34 tests passed. Approved.
Advance session engine_revision in run_ui_mutation under the publisher lock across the live engine call so inbox OCC cannot race a later JS note. Dead-letter conflicting and malformed inbox heads to inbox/failed/ so the queue cannot wedge; next sequence can apply. Sync jackControls#61 mutates (move_copy/import_step) into the shared map after rebase onto main.
Summary
solid_import_step/solid_edit_import_stepover the existingimport_stepbody feature (dumb reference solid + stored STEP bytes). This does not reverse-engineer sketch/extrude history from B-rep.cad_scriptto dump this process’s successful mutatingtools/callsequence as{ "calls": [ { "name", "arguments" } ] }(forward record; skips inspect/export, failed calls, andcad_scriptitself).cad_compare_solidsto summarizesolid_scenebody count plus per-body bbox / vertex / triangle counts so a rebuilt history can be checked against an imported reference.Test plan
cargo test --manifest-path mcp-server/Cargo.toml --bin nbcad-mcp(29 passed, includingmcp_import_step_records_forward_script)cad_new_projector a fresh process,solid_import_step, confirm one body,cad_scriptcontainssolid_import_step,cad_compare_solidsreports mesh counts