diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f3ff5d7..5e16f692 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,11 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 > Version history splits in two: the `3.x` entries at the bottom are the > original TypeScript/winax implementation. The project is now a Python + -> pywin32 MCP server and restarted its own versioning at `1.0.0`; `1.x` -> entries are the current line. +> pywin32 MCP server and restarted its own versioning at `1.0.0`; the `1.x` +> and `2.x` entries are the current line. ## [Unreleased] +## [2.0.0] - 2026-11-09 + ### Added - **`solidworks-mcp-python selftest`** (#179; `--selftest` works too). @@ -100,6 +102,40 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **The beta tools' faults** (item 7 in the work queue): + - `calculate_mass_properties` reported moments of inertia in kg·m² + under a kg·mm² label; the values are now kg·mm², matching the label. + - `pattern_circular` and `mirror_feature` reported a pattern or mirror + of a cut (a ring of holes) as an error; any change in volume now + counts, and the reply gives `volume_change`. + - `delete_feature` names every feature it removed, dependents included, + and can delete a sketch a feature uses: the features built from it go + with it. SolidWorks refused that delete before. + - `create_extrusion` and `create_revolve` use the sketch `sketch_name` + names (a missing name fails, listing the sketches), and + `create_revolve` turns about the reference axis `axis_entity` names. + - `create_sketch` gives the sketch the `sketch_name` you ask for. SolidWorks + won't rename an open sketch, so `exit_sketch` names it and replies with + the final name; `create_sketch` replies with SolidWorks' name and + `name_on_exit`. + - `create_part` assigns its `material`; `get_material` also reports the + density; `check_interference` honours `include_hidden`; `save_as` + keeps an existing file unless `overwrite` is on; `execute_macro` runs + `repeat_count` times; `sketch_tutorial_simple_hole` points at + `create_cut_extrude`. + - On SolidWorks versions without `IMassProperty`, the products of + inertia Ixz and Iyz were swapped. + - The agent guides' examples no longer send `direction: "cut"` to + `create_extrusion` (use `create_cut_extrude`) or a sketch line as + `create_revolve`'s `axis_entity`, which takes a reference axis. + - `create_part`, `create_assembly` and `create_drawing` say that `name` + is only a label: SolidWorks names a new document itself. + - The 23 tools these fixes cover are now `stable`, each checked by a + live test (`tests/live/test_live_sw_beta_tools.py`). + - `delete_feature` asks the agent to check with the user first, and its + reply carries a `warning` naming everything else it deleted (a + sketch takes the features built from it). + - **`create_extrusion`'s reply says what was built** (#209): the end condition SolidWorks used (`MidPlane` for `both_directions`), and the thin wall. It used to echo `direction: blind`, so an agent concluded @@ -141,6 +177,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **BREAKING: inputs that did nothing are refused** (item 7). A call that + sends one gets an `invalid_input` error naming what to use instead: + - `create_extrusion.direction` (use `end_condition`) and + `create_revolve.direction` (use `both_directions`); + - `create_assembly.components` (use `insert_component`); + - `create_drawing.model_path` and `.sheet_format`, and + `create_technical_drawing`'s `template`, `sheet_size`, `sheet_format`, + `title`, `output_path`, `scale` and `auto_populate_views` (use + `generate_drawing_package`, `save_file` or `set_sheet_scale`); + `create_technical_drawing` takes `projection` instead; + - `save_file.force_save`; `options` on the five `export_*` tools; + - `check_interference.tolerance` and `.check_all_components`; + - all four `calculate_mass_properties` inputs; + - `execute_macro`'s `parameters`, `execution_parameters`, `target_file`, + `execution_mode`, `pause_on_error` and `log_execution`. + `add_drawing_view` refuses an `operation` other than `add`. Moments of + inertia from `calculate_mass_properties` are a million times larger + than before, because they are now in the kg·mm² their label always said. + - **Building the API index needs SolidWorks installed, not running**, and never starts or calls SolidWorks. The lookups build the index themselves, and an index written by an older version is rebuilt automatically. Without diff --git a/docs/agents/prompt-driven-design.md b/docs/agents/prompt-driven-design.md index 2c08e231..f5a66ef7 100644 --- a/docs/agents/prompt-driven-design.md +++ b/docs/agents/prompt-driven-design.md @@ -250,14 +250,14 @@ The plan should include: 2. `add_centerline(...)` — the revolve axis 3. Multiple `add_line` and `add_arc` calls tracing the half-profile 4. `exit_sketch()` -5. `create_revolve(sketch_name="Sketch1", axis_entity="Line1", angle=360.0)` +5. `create_revolve(sketch_name="Sketch1", angle=360.0)` (it turns about the centreline) If the LLM skips `add_centerline` or tries to use a construction line as the axis without naming it, correct before running: ```text Step 3 is missing the centerline. Add: add_centerline(start_x=0, start_y=0, end_x=830, end_y=0) -before the profile lines, and use that as the axis_entity in create_revolve. +before the profile lines; create_revolve turns about it. ``` --- diff --git a/docs/agents/prompting-tutorials.md b/docs/agents/prompting-tutorials.md index d9d9dd9e..0a579f82 100644 --- a/docs/agents/prompting-tutorials.md +++ b/docs/agents/prompting-tutorials.md @@ -150,7 +150,7 @@ Name it "Cylinder". {"tool": "add_centerline","args": {"x1": 0.0, "y1": 0.0, "x2": 80.0, "y2": 0.0}}, {"tool": "add_rectangle", "args": {"x1": 0.0, "y1": 0.0, "x2": 80.0, "y2": 25.0}}, {"tool": "exit_sketch", "args": {}}, - {"tool": "create_revolve","args": {"sketch_name": "Sketch1", "axis_entity": "Line1", "angle": 360.0}}, + {"tool": "create_revolve","args": {"sketch_name": "Sketch1", "angle": 360.0}}, {"tool": "save_as", "args": {"file_path": "C:\\Temp\\Cylinder.sldprt"}} ] ``` @@ -203,7 +203,7 @@ Name it "Baseball Bat Recreation". {"tool": "add_line", "args": {"x1": 17.0, "y1": 22.0, "x2": 680.0, "y2": 35.0}}, {"tool": "add_arc", "args": {"center_x": 680.0, "center_y": 0.0, "start_x": 680.0, "start_y": 35.0, "end_x": 830.0, "end_y": 0.0}}, {"tool": "exit_sketch", "args": {}}, - {"tool": "create_revolve", "args": {"sketch_name": "Sketch1", "axis_entity": "Line1", "angle": 360.0}}, + {"tool": "create_revolve", "args": {"sketch_name": "Sketch1", "angle": 360.0}}, {"tool": "save_as", "args": {"file_path": "C:\\Temp\\Baseball Bat Recreation.sldprt"}} ] ``` @@ -240,12 +240,12 @@ Name "Mounting Plate". {"tool": "add_circle", "args": {"center_x": -32.0, "center_y": 22.0, "radius": 3.0}}, {"tool": "add_circle", "args": {"center_x": 32.0, "center_y": 22.0, "radius": 3.0}}, {"tool": "exit_sketch", "args": {}}, - {"tool": "create_extrusion","args": {"sketch_name": "Sketch2", "depth": 15.0, "direction": "cut"}} + {"tool": "create_cut_extrude","args": {"sketch_name": "Sketch2", "depth": 15.0}} ] ``` !!! note "Counterbore not yet supported directly" - `create_extrusion` with `direction=cut` creates a blind cut. For a true counterbore hole (two-step), run two separate cut extrusions or use `generate_vba_part_modeling`. + `create_cut_extrude` with a `depth` makes a blind cut. For a true counterbore hole (two-step), run two separate cut extrusions or use `generate_vba_part_modeling`. --- @@ -303,7 +303,7 @@ Name "Bolt Circle Disc". {"tool": "add_circle", "args": {"center_x": 0.0, "center_y": 30.0, "radius": 4.0}}, {"tool": "sketch_circular_pattern","args": {"entity": "Arc1", "count": 6, "angle": 60.0, "center_x": 0.0, "center_y": 0.0}}, {"tool": "exit_sketch", "args": {}}, - {"tool": "create_extrusion","args": {"sketch_name": "Sketch2", "depth": 10.0, "direction": "cut"}} + {"tool": "create_cut_extrude","args": {"sketch_name": "Sketch2", "depth": 10.0}} ] ``` diff --git a/docs/agents/reverse-engineering-workflow.md b/docs/agents/reverse-engineering-workflow.md index 492e8d70..6f2c7f91 100644 --- a/docs/agents/reverse-engineering-workflow.md +++ b/docs/agents/reverse-engineering-workflow.md @@ -237,7 +237,7 @@ Likely plan: 5. add_line(start_x=0, start_y=6, end_x=180, end_y=6) # shaft body 6. add_line(start_x=180, start_y=6, end_x=200, end_y=10) # crank end taper 7. exit_sketch() -8. create_revolve(sketch_name="Sketch1", axis_entity="Line1", angle=360.0) +8. create_revolve(sketch_name="Sketch1", angle=360.0) ``` ### Step 3 — Screenshot the assembled view diff --git a/docs/agents/sample-models-guide.md b/docs/agents/sample-models-guide.md index cbb0485e..f2729639 100644 --- a/docs/agents/sample-models-guide.md +++ b/docs/agents/sample-models-guide.md @@ -80,7 +80,7 @@ Create a baseball bat model: - Barrel top: add_arc center at (680,0) from (680,35) to (680,-35) (semicircle) - Return to origin on the axis 5. exit_sketch -6. create_revolve with sketch_name="Sketch1", axis_entity="Line1", angle=360.0 +6. create_revolve with sketch_name="Sketch1", angle=360.0 (about the centreline) ``` !!! tip "Why revolve first?" diff --git a/docs/assets/progress/progress.json b/docs/assets/progress/progress.json index ce1d5a7d..3348beda 100644 --- a/docs/assets/progress/progress.json +++ b/docs/assets/progress/progress.json @@ -2,8 +2,8 @@ "tools": { "total": 156, "by_level": { - "stable": 83, - "beta": 23, + "stable": 106, + "beta": 0, "experimental": 10, "planned": 40 }, @@ -67,8 +67,8 @@ { "name": "add_drawing_view", "family": "drawing", - "level": "beta", - "reason": "only ever adds a view, whatever `operation` says; use `create_drawing_view`.", + "level": "stable", + "reason": "", "summary": "Place one named view of a part or assembly on the active drawing sheet; the same as `create_drawing_view`." }, { @@ -270,8 +270,8 @@ { "name": "calculate_mass_properties", "family": "analysis", - "level": "beta", - "reason": "moments of inertia are labelled kg·mm² but are kg·m²; its input fields are unused.", + "level": "stable", + "reason": "", "summary": "Measure the active part or assembly's mass properties: volume, surface area, mass, centre of mass and moments of inertia." }, { @@ -291,8 +291,8 @@ { "name": "check_interference", "family": "analysis", - "level": "beta", - "reason": "`include_hidden` has no effect.", + "level": "stable", + "reason": "", "summary": "Find where the components of the active assembly overlap (Tools > Interference Detection)." }, { @@ -340,8 +340,8 @@ { "name": "create_assembly", "family": "modeling", - "level": "beta", - "reason": "`components` is ignored; use `insert_component`.", + "level": "stable", + "reason": "", "summary": "Create a new, empty assembly document and make it active, ready for `insert_component`." }, { @@ -368,8 +368,8 @@ { "name": "create_drawing", "family": "modeling", - "level": "beta", - "reason": "`model_path` and `sheet_format` are ignored.", + "level": "stable", + "reason": "", "summary": "Create a new, empty drawing document and make it active." }, { @@ -382,8 +382,8 @@ { "name": "create_extrusion", "family": "modeling", - "level": "beta", - "reason": "`sketch_name` and `direction` are ignored; it extrudes the sketch closed last.", + "level": "stable", + "reason": "", "summary": "Extrude the sketch you just closed into a solid boss, added to the active part." }, { @@ -403,8 +403,8 @@ { "name": "create_part", "family": "modeling", - "level": "beta", - "reason": "`material` is ignored; use `set_material`.", + "level": "stable", + "reason": "", "summary": "Create a new, empty part document and make it active, ready for `create_sketch`." }, { @@ -424,9 +424,9 @@ { "name": "create_revolve", "family": "modeling", - "level": "beta", - "reason": "`sketch_name` and `axis_entity` are ignored; it revolves the sketch closed last around its centreline.", - "summary": "Revolve the sketch you just closed around the centreline drawn in it, adding a solid to the active part." + "level": "stable", + "reason": "", + "summary": "Revolve a closed sketch around an axis, adding a solid to the active part." }, { "name": "create_section_view", @@ -438,8 +438,8 @@ { "name": "create_sketch", "family": "sketching", - "level": "beta", - "reason": "echoes a `sketch_name` you pass instead of SolidWorks' own name.", + "level": "stable", + "reason": "", "summary": "Open a new sketch on a plane of the active part, ready for lines, circles and other sketch entities." }, { @@ -452,8 +452,8 @@ { "name": "create_technical_drawing", "family": "drawing", - "level": "beta", - "reason": "ignores `template`, `sheet_size`, `title` and `output_path`; it only places three views.", + "level": "stable", + "reason": "", "summary": "Place a model's three standard views (front, top and side) on the active drawing sheet in one call." }, { @@ -466,8 +466,8 @@ { "name": "delete_feature", "family": "modeling", - "level": "beta", - "reason": "on SolidWorks its message doesn't name the dependent features it removed.", + "level": "stable", + "reason": "", "summary": "Delete a feature or sketch from the active model, along with the features that depend on it." }, { @@ -480,8 +480,8 @@ { "name": "execute_macro", "family": "macro_recording", - "level": "beta", - "reason": "runs the macro once whatever `repeat_count` says, and only binary .swp files.", + "level": "stable", + "reason": "", "summary": "Run a binary SolidWorks macro (.swp, or legacy .swb) in the running SolidWorks, calling its `main` procedure once." }, { @@ -501,8 +501,8 @@ { "name": "export_dwg", "family": "export", - "level": "beta", - "reason": "`options` is ignored; the file extension sets the format.", + "level": "stable", + "reason": "", "summary": "Export the active document to DWG, like File > Save As with the DWG type." }, { @@ -515,8 +515,8 @@ { "name": "export_iges", "family": "export", - "level": "beta", - "reason": "`options` is ignored; the file extension sets the format.", + "level": "stable", + "reason": "", "summary": "Export the active document to IGES, like File > Save As with the IGES type." }, { @@ -529,22 +529,22 @@ { "name": "export_pdf", "family": "export", - "level": "beta", - "reason": "`options` is ignored; the file extension sets the format.", + "level": "stable", + "reason": "", "summary": "Export the active document to PDF, like File > Save As with the PDF type." }, { "name": "export_step", "family": "export", - "level": "beta", - "reason": "`options` is ignored; the file extension sets the format.", + "level": "stable", + "reason": "", "summary": "Export the active document to STEP, like File > Save As with the STEP type." }, { "name": "export_stl", "family": "export", - "level": "beta", - "reason": "`options` is ignored; the file extension sets the format.", + "level": "stable", + "reason": "", "summary": "Export the active document to STL, like File > Save As with the STL type." }, { @@ -683,8 +683,8 @@ { "name": "get_material", "family": "engineering_data", - "level": "beta", - "reason": "reads the material's name and library, not its properties.", + "level": "stable", + "reason": "", "summary": "Report the material assigned to the active part." }, { @@ -837,8 +837,8 @@ { "name": "mirror_feature", "family": "modeling", - "level": "beta", - "reason": "mirroring a cut is reported as an error.", + "level": "stable", + "reason": "", "summary": "Mirror solid bodies or features of the active part about a plane." }, { @@ -872,8 +872,8 @@ { "name": "pattern_circular", "family": "modeling", - "level": "beta", - "reason": "patterning a cut is reported as an error.", + "level": "stable", + "reason": "", "summary": "Repeat features of the active part around an axis, e.g. a ring of bosses or ribs." }, { @@ -900,8 +900,8 @@ { "name": "save_as", "family": "file_management", - "level": "beta", - "reason": "replaces an existing file whatever `overwrite` says.", + "level": "stable", + "reason": "", "summary": "Save the active document under a new path, as a SolidWorks file or in an exchange format such as STEP or IGES." }, { @@ -921,8 +921,8 @@ { "name": "save_file", "family": "file_management", - "level": "beta", - "reason": "`force_save` is ignored.", + "level": "stable", + "reason": "", "summary": "Save the active document, to its own path or to a new path in its own format." }, { @@ -1033,8 +1033,8 @@ { "name": "sketch_tutorial_simple_hole", "family": "sketching", - "level": "beta", - "reason": "suggests a negative-depth extrusion for the hole instead of `create_cut_extrude`.", + "level": "stable", + "reason": "", "summary": "Demonstrate the sketch steps: draw a 5 mm circle at the origin on the Top plane and close the sketch." }, { @@ -1105,7 +1105,7 @@ "com": { "type_library": "83A33D31-27C5-11CE-BFD4-00400513BB57x0x34x0", "core": { - "covered": 214, + "covered": 215, "total": 2838 }, "api": { @@ -1157,7 +1157,6 @@ "CreateEllipse", "CreateLine", "CreateLinearSketchStepAndRepeat", - "EditDelete", "EditRebuild3", "EditSuppress2", "EditUndo", @@ -1351,6 +1350,7 @@ "FirstFeature", "GetBodies2", "GetBox", + "GetChildren", "GetConfigurationNames", "GetID", "GetModelDoc2", @@ -1370,6 +1370,7 @@ "name": "IFeature", "covered": [ "GetBox", + "GetChildren", "GetErrorCode2", "GetFaces", "GetFirstDisplayDimension", diff --git a/docs/assets/progress/progress.svg b/docs/assets/progress/progress.svg index 73d5a656..719305c0 100644 --- a/docs/assets/progress/progress.svg +++ b/docs/assets/progress/progress.svg @@ -25,28 +25,28 @@ 68% work on SolidWorks today - - + + -stable 83 - -beta 23 +stable 106 + +beta 0 experimental 10 planned 40 Core SolidWorks COM API (12 interfaces) -214 of 2,838 methods · 7.5% +215 of 2,838 methods · 7.6% - - - + + + Whole API: 238 of 5,985 method names (4.0%), counted by a text scan of the adapters. diff --git a/docs/assets/progress/tables.html b/docs/assets/progress/tables.html index 259c7a49..054950f4 100644 --- a/docs/assets/progress/tables.html +++ b/docs/assets/progress/tables.html @@ -10,7 +10,7 @@ add_circlesketchingstableDraw a circle in the open sketch from its centre and radius, in mm. add_datum_tagdrawing_automationstableAdd a datum feature symbol (A, B, C, ...) on an edge of the active drawing. add_dimensiondrawingplanned
no adapter implements it yet.Add one dimension between drawing entities at a position on the sheet. -add_drawing_viewdrawingbeta
only ever adds a view, whatever `operation` says; use `create_drawing_view`.Place one named view of a part or assembly on the active drawing sheet; the same as `create_drawing_view`. +add_drawing_viewdrawingstablePlace one named view of a part or assembly on the active drawing sheet; the same as `create_drawing_view`. add_ellipsesketchingstableDraw an ellipse in the open sketch from its centre and axis lengths, in mm. add_filletmodelingstableRound edges of the active part with a constant-radius fillet. add_gtoldrawing_automationstableAdd a GD&T feature control frame to the active drawing, e.g. position Ø0.2 (M) to A|B|C. @@ -39,46 +39,46 @@ batch_exportexportplanned
no adapter implements it yet.Export every matching SolidWorks file in a folder to one format. batch_file_operationsfile_managementplanned
no adapter implements it yet.Copy, move, rename or delete SolidWorks files on disk in one call. batch_process_filesautomationplanned
no adapter implements it yet.Run one operation (rebuild, save as, export or property update) on every matching file in a folder. -calculate_mass_propertiesanalysisbeta
moments of inertia are labelled kg·mm² but are kg·m²; its input fields are unused.Measure the active part or assembly's mass properties: volume, surface area, mass, centre of mass and moments of inertia. +calculate_mass_propertiesanalysisstableMeasure the active part or assembly's mass properties: volume, surface area, mass, centre of mass and moments of inertia. check_drawing_compliancedrawing_analysisplanned
no adapter implements it yet.Check a drawing's title block and sheet format against a drafting standard (ISO, ANSI or DIN). check_drawing_standardsdrawingplanned
no adapter implements it yet.Check the active drawing against a drafting standard and score it. -check_interferenceanalysisbeta
`include_hidden` has no effect.Find where the components of the active assembly overlap (Tools > Interference Detection). +check_interferenceanalysisstableFind where the components of the active assembly overlap (Tools > Interference Detection). check_sketch_fully_definedsketchingstableCheck whether a sketch is fully defined, under-defined or over-defined. classify_feature_treefile_managementexperimental
not yet checked by a live test on real SolidWorks.Classify the active model's kind (part, sheet metal, assembly, drawing and so on) from its feature tree, and suggest how to work on it. close_modelmodelingstableClose the active document, saving it first only if asked. compare_drawing_versionsdrawing_analysisexperimental
not yet checked by a live test on real SolidWorks.Compare two drawing files on disk: whether they are byte-identical, and how their sizes differ. compare_templatestemplate_managementexperimental
not yet checked by a live test on real SolidWorks.Compare two template files on disk: whether they are byte-identical, and how their sizes differ. convert_file_formatfile_managementplanned
no adapter implements it yet.Convert a SolidWorks file on disk to another format without opening it first. -create_assemblymodelingbeta
`components` is ignored; use `insert_component`.Create a new, empty assembly document and make it active, ready for `insert_component`. +create_assemblymodelingstableCreate a new, empty assembly document and make it active, ready for `insert_component`. create_axismodelingstableAdd a reference axis along the model's x, y or z direction, through the origin. create_cut_extrudemodelingstableCut material from the active part with a closed sketch profile: holes, slots, windows. create_detail_viewdrawingplanned
no adapter implements it yet.Magnify a circular area of a drawing view as a detail view. -create_drawingmodelingbeta
`model_path` and `sheet_format` are ignored.Create a new, empty drawing document and make it active. +create_drawingmodelingstableCreate a new, empty drawing document and make it active. create_drawing_viewdrawingstablePlace one named view of a part or assembly on the active drawing sheet. -create_extrusionmodelingbeta
`sketch_name` and `direction` are ignored; it extrudes the sketch closed last.Extrude the sketch you just closed into a solid boss, added to the active part. +create_extrusionmodelingstableExtrude the sketch you just closed into a solid boss, added to the active part. create_loftmodelingstableLoft a solid through two or more closed profile sketches, in order (Insert > Boss/Base > Loft). create_macro_librarymacro_recordingplanned
no adapter implements it yet.Set up a shared folder of categorised, documented macros. -create_partmodelingbeta
`material` is ignored; use `set_material`.Create a new, empty part document and make it active, ready for `create_sketch`. +create_partmodelingstableCreate a new, empty part document and make it active, ready for `create_sketch`. create_reference_planemodelingstableAdd a reference plane offset from, at an angle to, or on a plane or flat face. create_reference_pointmodelingstableAdd a reference point on an edge (at a distance or percentage along it) or at the centre of a face. -create_revolvemodelingbeta
`sketch_name` and `axis_entity` are ignored; it revolves the sketch closed last around its centreline.Revolve the sketch you just closed around the centreline drawn in it, adding a solid to the active part. +create_revolvemodelingstableRevolve a closed sketch around an axis, adding a solid to the active part. create_section_viewdrawingplanned
no adapter implements it yet.Cut a section view along a line across a drawing view. -create_sketchsketchingbeta
echoes a `sketch_name` you pass instead of SolidWorks' own name.Open a new sketch on a plane of the active part, ready for lines, circles and other sketch entities. +create_sketchsketchingstableOpen a new sketch on a plane of the active part, ready for lines, circles and other sketch entities. create_sweepmodelingstableSweep a closed profile sketch along a path sketch to make a solid (Insert > Boss/Base > Sweep). -create_technical_drawingdrawingbeta
ignores `template`, `sheet_size`, `title` and `output_path`; it only places three views.Place a model's three standard views (front, top and side) on the active drawing sheet in one call. +create_technical_drawingdrawingstablePlace a model's three standard views (front, top and side) on the active drawing sheet in one call. create_templateautomationplanned
no adapter implements it yet.Create a part, assembly or drawing template file (.prtdot, .asmdot, .drwdot) with standard settings. -delete_featuremodelingbeta
on SolidWorks its message doesn't name the dependent features it removed.Delete a feature or sketch from the active model, along with the features that depend on it. +delete_featuremodelingstableDelete a feature or sketch from the active model, along with the features that depend on it. discover_solidworks_docsdocs_discoverystableRebuild the SolidWorks API index on purpose. The lookups don't need it. -execute_macromacro_recordingbeta
runs the macro once whatever `repeat_count` says, and only binary .swp files.Run a binary SolidWorks macro (.swp, or legacy .swb) in the running SolidWorks, calling its `main` procedure once. +execute_macromacro_recordingstableRun a binary SolidWorks macro (.swp, or legacy .swb) in the running SolidWorks, calling its `main` procedure once. execute_workflowautomationplanned
no adapter implements it yet.Run a named sequence of tool steps, in order or in parallel, with an error policy. exit_sketchsketchingstableClose the open sketch so a feature can use it. -export_dwgexportbeta
`options` is ignored; the file extension sets the format.Export the active document to DWG, like File > Save As with the DWG type. +export_dwgexportstableExport the active document to DWG, like File > Save As with the DWG type. export_flat_patternengineering_datastableExport the active sheet-metal part's flat pattern to DXF or DWG, for laser or waterjet cutting. -export_igesexportbeta
`options` is ignored; the file extension sets the format.Export the active document to IGES, like File > Save As with the IGES type. +export_igesexportstableExport the active document to IGES, like File > Save As with the IGES type. export_imageexportstableSave a picture of the active model in a standard view, as PNG or JPG, at a chosen size. -export_pdfexportbeta
`options` is ignored; the file extension sets the format.Export the active document to PDF, like File > Save As with the PDF type. -export_stepexportbeta
`options` is ignored; the file extension sets the format.Export the active document to STEP, like File > Save As with the STEP type. -export_stlexportbeta
`options` is ignored; the file extension sets the format.Export the active document to STL, like File > Save As with the STL type. +export_pdfexportstableExport the active document to PDF, like File > Save As with the PDF type. +export_stepexportstableExport the active document to STEP, like File > Save As with the STEP type. +export_stlexportstableExport the active document to STL, like File > Save As with the STL type. extract_templatetemplate_managementplanned
no adapter implements it yet.Capture a model's settings (custom properties, dimensions, units) as a reusable template file. find_related_api_membersdocs_discoverystableFind SolidWorks API members related to an interface or member name. generate_drawing_packagedrawing_automationstableCreate a complete A3 drawing of a part or assembly in one call, and save it with a PDF. @@ -98,7 +98,7 @@ get_dimensionmodelingexperimental
not yet checked by a live test on real SolidWorks.Read one dimension of the active model by its full name, e.g. "D1@Sketch1". get_file_propertiesfile_managementexperimental
not yet checked by a live test on real SolidWorks.Report the active document's file details: name, path, size, dates, type, configuration and unsaved changes. get_mass_propertiesanalysisstableMeasure the active part or assembly's mass properties: volume, surface area, mass, centre of mass and moments of inertia. -get_materialengineering_databeta
reads the material's name and library, not its properties.Report the material assigned to the active part. +get_materialengineering_datastableReport the material assigned to the active part. get_material_propertiesanalysisplanned
no adapter implements it yet.Report the physical properties of the active part's material: density, modulus, Poisson's ratio, strength. get_model_infofile_managementstableReport the active document's title, path, type, active configuration, unsaved changes, feature count and rebuild status. get_selected_entitiesfile_managementstableDescribe the faces, edges and vertices selected in the SolidWorks window right now. @@ -120,19 +120,19 @@ manage_design_tableautomationplanned
no adapter implements it yet.Create, read or update a design table (an Excel sheet that drives a model's configurations). manage_file_propertiesfile_managementplanned
no adapter implements it yet.Read or change the file-level properties of a SolidWorks file on disk. measure_entitiesengineering_datastableMeasure faces, edges or vertices of the active model with SolidWorks' Measure tool. -mirror_featuremodelingbeta
mirroring a cut is reported as an error.Mirror solid bodies or features of the active part about a plane. +mirror_featuremodelingstableMirror solid bodies or features of the active part about a plane. open_modelmodelingstableOpen a part, assembly or drawing from disk and make it the active document. optimize_macromacro_recordingplanned
no adapter implements it yet.Suggest improvements to a macro's code for speed and reliability. optimize_performanceautomationplanned
no adapter implements it yet.Review SolidWorks' performance settings and apply faster ones. pack_and_go_assemblyfile_managementexperimental
not yet checked by a live test on real SolidWorks.Copy an assembly and every part it references into one folder, with the references updated (SolidWorks Pack and Go). -pattern_circularmodelingbeta
patterning a cut is reported as an error.Repeat features of the active part around an axis, e.g. a ring of bosses or ribs. +pattern_circularmodelingstableRepeat features of the active part around an axis, e.g. a ring of bosses or ribs. rebuild_modelmodelingstableRebuild the active model and report each feature that fails, with the reason and a fix. rename_featuremodelingstableRename a feature in the active model's feature tree. reset_circuit_breakerserver_adminstableLet calls through again to operations the server paused after repeated failures. -save_asfile_managementbeta
replaces an existing file whatever `overwrite` says.Save the active document under a new path, as a SolidWorks file or in an exchange format such as STEP or IGES. +save_asfile_managementstableSave the active document under a new path, as a SolidWorks file or in an exchange format such as STEP or IGES. save_assemblyfile_managementstableSave the active assembly in place, or as a new .sldasm, optionally copying the parts it uses alongside. save_body_as_partfile_managementstableSave one solid body of the active multibody part as a new part file (Insert > Features > Save Bodies). -save_filefile_managementbeta
`force_save` is ignored.Save the active document, to its own path or to a new path in its own format. +save_filefile_managementstableSave the active document, to its own path or to a new path in its own format. save_partfile_managementstableSave the active part in place, or as a new .sldprt with the target path checked first. save_to_template_librarytemplate_managementplanned
no adapter implements it yet.Add a template to a shared, categorised template library. search_solidworks_api_helpdocs_discoverystableSearch SolidWorks API interface and member names by keyword. @@ -148,7 +148,7 @@ sketch_linear_patternsketchingstableRepeat sketch entities in a straight line at a fixed spacing. sketch_mirrorsketchingstableMirror sketch entities about a centreline in the open sketch. sketch_offsetsketchingstableOffset sketch entities by a distance in mm, outward or inward, e.g. for a wall or a clearance. -sketch_tutorial_simple_holesketchingbeta
suggests a negative-depth extrusion for the hole instead of `create_cut_extrude`.Demonstrate the sketch steps: draw a 5 mm circle at the origin on the Top plane and close the sketch. +sketch_tutorial_simple_holesketchingstableDemonstrate the sketch steps: draw a 5 mm circle at the origin on the Top plane and close the sketch. start_macro_recordingmacro_recordingplanned
no adapter implements it yet.Start SolidWorks' macro recorder, to capture what the user does as a macro. stop_macro_recordingmacro_recordingplanned
no adapter implements it yet.Stop the macro recorder and save the recorded macro. suppress_featuremodelingstableSuppress or unsuppress a feature of the active model; suppressed features stay in the tree but are not built. @@ -167,7 +167,7 @@ InterfaceMethods calledOfCalled ISldWorks22330
6.7%ActivateDoc3, CloseAllDocuments, CloseDoc, Frame, GetConfigurationNames, GetCurrentLanguage, GetDocuments, GetExecutablePath, GetExportFileData, GetMassProperties, GetOpenDocumentByName, GetUserPreferenceStringValue, GetUserPreferenceToggle, NewAssembly, NewDocument, NewDrawing2, NewPart, OpenDoc6, RevisionNumber, RunMacro2, SetUserPreferenceIntegerValue, SetUserPreferenceToggle
-IModelDoc266758
8.7%AddDiameterDimension2, AddDimension, AddDimension2, AddRadialDimension2, AddRelation, ClearSelection2, CreateArc, CreateCenterLine, CreateCircleByRadius, CreateCircularSketchStepAndRepeat, CreateEllipse, CreateLine, CreateLinearSketchStepAndRepeat, EditDelete, EditRebuild3, EditSuppress2, EditUndo, EditUndo2, EditUnsuppress2, FeatureByPositionReverse, FeatureChamferType, FeatureCut3, FeatureCut4, FeatureFillet3, FeatureRevolve2, FirstFeature, ForceRebuild3, GetActiveConfiguration, GetActiveSketch2, GetConfigurationNames, GetFeatureCount, GetLineCount, GetMassProperties, GetNext, GetPathName, GetSaveFlag, GetTitle, GetType, GetUserPreferenceStringValue, GetUserPreferenceToggle, GraphicsRedraw2, InsertAxis2, InsertDatumTag2, InsertGtol, InsertNote, InsertProtrusionBlend2, InsertProtrusionSwept4, InsertSketch, Lock, Parameter, Save, Save3, SaveAs, SaveAs2, SaveAs3, SaveBMP, Select, SetSaveFlag, SetUserPreferenceIntegerValue, SetUserPreferenceToggle, ShowNamedView2, SketchAddConstraints, SketchMirror, SketchOffset2, ViewZoomtofit, ViewZoomtofit2
+IModelDoc265758
8.6%AddDiameterDimension2, AddDimension, AddDimension2, AddRadialDimension2, AddRelation, ClearSelection2, CreateArc, CreateCenterLine, CreateCircleByRadius, CreateCircularSketchStepAndRepeat, CreateEllipse, CreateLine, CreateLinearSketchStepAndRepeat, EditRebuild3, EditSuppress2, EditUndo, EditUndo2, EditUnsuppress2, FeatureByPositionReverse, FeatureChamferType, FeatureCut3, FeatureCut4, FeatureFillet3, FeatureRevolve2, FirstFeature, ForceRebuild3, GetActiveConfiguration, GetActiveSketch2, GetConfigurationNames, GetFeatureCount, GetLineCount, GetMassProperties, GetNext, GetPathName, GetSaveFlag, GetTitle, GetType, GetUserPreferenceStringValue, GetUserPreferenceToggle, GraphicsRedraw2, InsertAxis2, InsertDatumTag2, InsertGtol, InsertNote, InsertProtrusionBlend2, InsertProtrusionSwept4, InsertSketch, Lock, Parameter, Save, Save3, SaveAs, SaveAs2, SaveAs3, SaveBMP, Select, SetSaveFlag, SetUserPreferenceIntegerValue, SetUserPreferenceToggle, ShowNamedView2, SketchAddConstraints, SketchMirror, SketchOffset2, ViewZoomtofit, ViewZoomtofit2
IModelDocExtension20337
5.9%AddDimension, CreateMassProperty, CreateMeasure, CustomPropertyManager, DeleteSelection2, GetAnnotations, GetMassProperties, GetPackAndGo, GetUserPreferenceInteger, GetUserPreferenceToggle, GetWhatsWrong, GetWhatsWrongCount, InsertBomTable4, SaveAs, SaveAs2, SaveAs3, SavePackAndGo, SelectByID2, SetUserPreferenceInteger, SetUserPreferenceToggle
IPartDoc12167
7.2%ExportToDWG2, FeatureByName, FeatureExtrusion2, FeatureExtrusion3, FeatureExtrusionThin2, FeatureRevolve2, FirstFeature, GetBodies2, GetMaterialPropertyName2, GetPartBox, InsertMirrorFeature, SetMaterialPropertyName2
IAssemblyDoc11198
5.6%AddComponent4, AddComponent5, AddMate5, CompConfigProperties5, FeatureByName, FixComponent, GetBox, GetComponentByName, GetComponentCount, GetComponents, UnfixComponent
@@ -175,8 +175,8 @@ IFeatureManager16268
6.0%CreateSaveBodyFeature, FeatureCircularPattern5, FeatureCut3, FeatureCut4, FeatureExtrusion2, FeatureExtrusion3, FeatureExtrusionThin2, FeatureFillet3, FeatureRevolve2, GetFeatureCount, InsertFeatureChamfer, InsertMirrorFeature, InsertProtrusionBlend2, InsertProtrusionSwept4, InsertRefPlane, InsertReferencePoint
ISketchManager1283
14.5%CreateArc, CreateCenterLine, CreateCircleByRadius, CreateCircularSketchStepAndRepeat, CreateCornerRectangle, CreateEllipse, CreateLine, CreateLinearSketchStepAndRepeat, CreatePolygon, CreateSpline2, InsertSketch, SketchOffset2
ISelectionMgr461
6.6%CreateSelectData, GetSelectedObject6, GetSelectedObjectCount2, GetSelectedObjectType3
-IComponent217147
11.6%CustomPropertyManager, FeatureByName, FirstFeature, GetBodies2, GetBox, GetConfigurationNames, GetID, GetModelDoc2, GetPathName, GetSuppression2, GetType, IsEnvelope, IsFixed, IsSuppressed, Select, Select2, Select4
-IFeature18115
15.7%GetBox, GetErrorCode2, GetFaces, GetFirstDisplayDimension, GetFirstSubFeature, GetID, GetNameForSelection, GetNextDisplayDimension, GetNextFeature, GetNextSubFeature, GetParents, GetSpecificFeature2, GetTypeName2, IsSuppressed, IsSuppressed2, Parameter, Select, Select2
+IComponent218147
12.2%CustomPropertyManager, FeatureByName, FirstFeature, GetBodies2, GetBox, GetChildren, GetConfigurationNames, GetID, GetModelDoc2, GetPathName, GetSuppression2, GetType, IsEnvelope, IsFixed, IsSuppressed, Select, Select2, Select4
+IFeature19115
16.5%GetBox, GetChildren, GetErrorCode2, GetFaces, GetFirstDisplayDimension, GetFirstSubFeature, GetID, GetNameForSelection, GetNextDisplayDimension, GetNextFeature, GetNextSubFeature, GetParents, GetSpecificFeature2, GetTypeName2, IsSuppressed, IsSuppressed2, Parameter, Select, Select2
ISketch3102
2.9%AutoDimension, GetLineCount, GetSketchSegments
diff --git a/openspec/changes/selftest-and-compatibility-report/.openspec.yaml b/openspec/changes/archive/2026-11-09-selftest-and-compatibility-report/.openspec.yaml similarity index 100% rename from openspec/changes/selftest-and-compatibility-report/.openspec.yaml rename to openspec/changes/archive/2026-11-09-selftest-and-compatibility-report/.openspec.yaml diff --git a/openspec/changes/selftest-and-compatibility-report/design.md b/openspec/changes/archive/2026-11-09-selftest-and-compatibility-report/design.md similarity index 100% rename from openspec/changes/selftest-and-compatibility-report/design.md rename to openspec/changes/archive/2026-11-09-selftest-and-compatibility-report/design.md diff --git a/openspec/changes/selftest-and-compatibility-report/proposal.md b/openspec/changes/archive/2026-11-09-selftest-and-compatibility-report/proposal.md similarity index 100% rename from openspec/changes/selftest-and-compatibility-report/proposal.md rename to openspec/changes/archive/2026-11-09-selftest-and-compatibility-report/proposal.md diff --git a/openspec/changes/selftest-and-compatibility-report/specs/docs/compatibility-reports/spec.md b/openspec/changes/archive/2026-11-09-selftest-and-compatibility-report/specs/docs/compatibility-reports/spec.md similarity index 100% rename from openspec/changes/selftest-and-compatibility-report/specs/docs/compatibility-reports/spec.md rename to openspec/changes/archive/2026-11-09-selftest-and-compatibility-report/specs/docs/compatibility-reports/spec.md diff --git a/openspec/changes/selftest-and-compatibility-report/specs/setup/selftest/spec.md b/openspec/changes/archive/2026-11-09-selftest-and-compatibility-report/specs/setup/selftest/spec.md similarity index 100% rename from openspec/changes/selftest-and-compatibility-report/specs/setup/selftest/spec.md rename to openspec/changes/archive/2026-11-09-selftest-and-compatibility-report/specs/setup/selftest/spec.md diff --git a/openspec/changes/selftest-and-compatibility-report/tasks.md b/openspec/changes/archive/2026-11-09-selftest-and-compatibility-report/tasks.md similarity index 100% rename from openspec/changes/selftest-and-compatibility-report/tasks.md rename to openspec/changes/archive/2026-11-09-selftest-and-compatibility-report/tasks.md diff --git a/openspec/changes/beta-tool-faults/.openspec.yaml b/openspec/changes/beta-tool-faults/.openspec.yaml new file mode 100644 index 00000000..ca7e6273 --- /dev/null +++ b/openspec/changes/beta-tool-faults/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-11-09 diff --git a/openspec/changes/beta-tool-faults/design.md b/openspec/changes/beta-tool-faults/design.md new file mode 100644 index 00000000..d060d58f --- /dev/null +++ b/openspec/changes/beta-tool-faults/design.md @@ -0,0 +1,215 @@ +## Context + +- `support_levels.py` rates 23 tools `beta`, each with a one-line reason. The + same faults sit in the skill's known-faults table + (`skills/solidworks-mcp/references/errors-and-limits.md`) and in the + queue's "Small faults" list. +- Every input model subclasses `CompatInput`, which has `extra="allow"`. Taking + a field off a schema today means a client that still sends it is ignored + silently. That is the same fault this change fixes. +- `create_cut_extrude` already resolves its sketch strictly. It matches + `sketch_name` against the live tree without regard to case, falls back to + `_last_sketch_name` only when that name exists in the tree, never guesses, + and selects the sketch with `SelectByID2` before the feature call + (`adapters/solidworks/features.py`, around line 1336). `create_extrusion` + and `create_revolve` skip all of this and rely on whatever is selected. +- The pattern and mirror adapters measure the volume before and after, which + is right, because SolidWorks returns a Feature even when it built nothing. + But they demand growth, so a cut can never pass. +- The real `delete_feature` returns `features_removed`, a count. The tool reads + `removed_features`, a list that only the mock returns. +- `get_mass_properties` returns moments of inertia in kg·m², because SolidWorks + works in SI. The tool labels them kg·mm². + +## Goals / Non-Goals + +**Goals:** + +- All 23 tools move to `stable`, each called by a live test that proves its + fix. +- No schema field is ignored. Each one is either honoured or refused with a + pointer to the right tool or input. +- The skill's known-faults table loses every row this change fixes. + +**Non-Goals:** + +- Merging duplicate tools. `add_drawing_view` and `create_drawing_view`, and + `get_mass_properties` and `calculate_mass_properties`, stay separate here; + #174 decides which name survives. +- Material properties beyond density. Those are the planned tool + `get_material_properties` (queue item 10). +- `add_mate` with edges or vertices. That is a new capability, not a beta + fault. +- Export options such as STL resolution or PDF sheets. If they're wanted + later, they come back as a typed input per format, not a free-form dict. + +## Decisions + +### 1. Honour or remove: the rule + +An input is **honoured** when SolidWorks supports it in a call or two and no +other tool does the job. It is **removed** when another tool does the job +better, or when honouring it would turn the tool into a different one. For +example, `create_drawing` with `model_path` would become +`generate_drawing_package`. + +| Tool | Input | Decision | How | +| --- | --- | --- | --- | +| `create_extrusion` | `sketch_name` | honour | Shared sketch resolver (decision 3) | +| `create_extrusion` | `direction` | remove | Duplicates `end_condition` | +| `create_revolve` | `sketch_name`, `axis_entity` | honour | Shared resolver; axis selected with mark 4 (decision 4) | +| `create_revolve` | `direction` | remove | Duplicates `both_directions`; reply stops echoing it | +| `create_sketch` | `sketch_name` | honour | Rename after `InsertSketch` (decision 5) | +| `create_part` | `material` | honour | The adapter's `set_material` after the part opens | +| `create_assembly` | `components` | remove | `insert_component` places each part | +| `create_drawing` | `model_path`, `sheet_format` | remove | `create_drawing_view`, `generate_drawing_package` | +| `create_technical_drawing` | `template`, `sheet_size`, `title`, `output_path` | remove | It places views on the active drawing; `generate_drawing_package` makes a whole drawing | +| `check_interference` | `include_hidden` | honour | Pass `ignore_hidden = not include_hidden` | +| `check_interference` | `tolerance`, `check_all_components` | remove | Unused; SolidWorks' check has no tolerance | +| `save_as` | `overwrite` | honour | The tool checks the path before the adapter call | +| `save_file` | `force_save` | remove | SolidWorks saves either way | +| `export_*` (5) | `options` | remove | The extension sets the format | +| `execute_macro` | `repeat_count`, `pause_between_runs` | honour | A loop in the tool (decision 7) | +| `calculate_mass_properties` | its 4 inputs | remove | It always measures the active model | +| `add_drawing_view` | `operation` other than `add` | refuse | It only adds views | + +Any other unused `execute_macro` field found during the work (`execution_mode`, +`pause_on_error`, `log_execution`) gets the same treatment by the same rule. +The decision is recorded in tasks.md. + +### 2. Refusing removed inputs: one helper + +`input_compat.py` gains `REMOVED_INPUTS: dict[tuple[str, str], str]`, mapping +(model class, field) to a pointer sentence, and +`reject_removed_inputs(model)`. `normalize_input` calls the helper after +validation. It raises `InvalidInputError` when any removed field is among the +model's extras (`model.model_extra`). So a removed field leaves the schema +(clients stop seeing it), and a stale client still gets a clear +`invalid_input`. No tool gains a `try/except`; the error boundary formats the +error. + +Not chosen: + +- **Keeping the fields with `deprecated=True`.** The schema would still + advertise them, and agents read schemas. +- **Switching `CompatInput` to `extra="forbid"`.** That would break the + aliases it exists for. + +### 3. One sketch resolver for extrude, revolve and cut + +Lift the resolution block out of the cut-extrude implementation into +`_resolve_profile_sketch(adapter, sketch_name) -> str`. It: + +1. matches `sketch_name` against the tree without regard to case; +2. if `sketch_name` is empty, falls back to `_last_sketch_name`, but only if + that name is in the tree; +3. otherwise raises an error listing the available sketches; +4. selects the result with `SelectByID2(..., "SKETCH", ..., _null_callout(), 0)`. + +The extrude, revolve and cut implementations all call it inside their +`_handle_com_operation` closures. `ExtrusionParameters` and +`RevolveParameters` gain `sketch_name: str | None`. The cut's behaviour and +messages are unchanged, so its existing tests are the regression check. + +### 4. The revolve axis + +When `axis_entity` is given, the resolver selects the profile with mark 0 and +appends the axis with `SelectByID2(axis_entity, "AXIS", ..., append=True, +mark=4)`. Without it, the sketch's centreline is used as today. A name that +isn't an axis in the tree fails before the feature call. `axis_entity` becomes +optional in `CreateRevolveInput`. That is not breaking: it was required, and +every existing caller passes a value, which is now either used or rejected +with a clear reason. A centreline name such as "Line1" can't be selected by +`SelectByID2` reliably. So any value that isn't a reference axis fails with a +hint: draw the axis with `add_centerline` and leave `axis_entity` out. + +### 5. Naming a new sketch + +After `InsertSketch` returns the active sketch, read its feature +(`ISketch` → `IFeature`, the cast the adapter already makes in `exit_sketch`) +and set `Name`. If the setter is refused (the name is taken, or SolidWorks +won't rename a sketch in edit mode), keep the original name and return a +`warning`. The live test settles which happens. If renaming during edit is +refused, the rename moves to `exit_sketch`, and `create_sketch` remembers the +wanted name. The spec allows either route, because both give the name in the +tree. + +**Settled by live batch 1 (2026-11-09):** SolidWorks 2026 accepts the new +`Name` on an open sketch but the tree keeps the old one, so the rename is done +by `exit_sketch`. `create_sketch` replies with SolidWorks' name plus +`name_on_exit`; `exit_sketch` replies with the final name, or a `warning` +when the name is taken. A new `create_sketch` drops a name still waiting on +a sketch left open. The mock refuses to rename an open sketch the same way. + +### 6. Pattern and mirror: any volume change counts + +Replace the growth test with `abs(after - before) > before * tolerance`. +Pattern keeps its 0.01 % tolerance and mirror its 0.1 %. The error text loses +the "did not grow" wording. The reply already carries `volume_before` and +`volume_after`, and gains `volume_change`, which is negative for a cut. + +### 7. Repeating a macro + +The tool loops `repeat_count` times and calls `adapter.execute_macro` each +time. It waits `asyncio.sleep(pause_between_runs)` between runs, so it stays on +the event-loop thread, as the COM rule requires. It stops at the first failure. +The reply gains `runs_completed`. A failure after some successes raises +`ToolOperationError` with the count. `repeat_count` gets `ge=1, le=100`, so a +typo can't hang SolidWorks for hours. + +### 8. Units and the rest + +- **Mass properties:** the tool multiplies the moments and products of inertia + by 1e6 (kg·m² → kg·mm²). The `MassProperties` dataclass stays SI, and + `verify_model` doesn't read inertia. The label says kg·mm², and the + description stops warning about it. +- **`get_material`:** density comes from `IPartDoc.GetMaterialPropertyValues2` + where it's available. Otherwise it comes from the material database entry + the adapter already finds. If neither is readable, it's `null`, never a guess. +- **`delete_feature`:** the adapter lists feature names before and after + (it already counts both) and returns `removed_features` as the names that + went, in tree order. The mock returns the same shape. +- **`save_as`:** `Path(file_path).exists() and not overwrite` raises + `InvalidInputError` before any COM call. + +### 9. Live tests and sign-off + +`tests/live/test_live_sw_beta_tools.py` has one test per tool, each printing +`[evidence]` lines. They run in five batches, each with the user's permission +and at the usual pace: + +1. sketch and feature inputs (extrude, revolve, sketch name, part material) +2. patterns, mirror and delete +3. mass properties, interference and material density +4. files (`save_as` overwrite, `save_file`, the five exports, macro repeats) +5. drawings (`create_drawing`, `create_technical_drawing`, `add_drawing_view`) + and the tutorial + +A batch that fails is fixed and run again before the next one starts. A tool +is promoted only after its batch passes. + +## Risks / Trade-offs + +- [**Clients sending a removed input now get an error.**] The error names the + replacement, and the CHANGELOG lists every removed field. The fields did + nothing before, so no result changes, only the reply. +- [**Renaming a sketch in edit mode may be refused.**] Decision 5's fallback + covers it. The live test runs first in batch 1, so the route is settled + early. +- [**`IgnoreHiddenBodies` may cover hidden bodies, not hidden components.**] + Batch 3 tests a hidden component. If SolidWorks still reports it, the fix + moves to the adapter's components filter, and the requirement doesn't change. +- [**Changing the moments of inertia by 1e6 changes numbers clients may have + saved.**] The old numbers were labelled wrong, so any saved value was already + misread. The CHANGELOG says so. +- [**Shared sketch resolution changes cut-extrude's code path.**] The cut's + unit tests and its live tests run unchanged in batch 1. + +## Migration Plan + +Removed fields are listed in the CHANGELOG under **Changed**, each with its +replacement. No data migration. Rollback is reverting the PR. + +## Open Questions + +None. Decisions 4 and 5 each have a fallback that the live tests settle. diff --git a/openspec/changes/beta-tool-faults/proposal.md b/openspec/changes/beta-tool-faults/proposal.md new file mode 100644 index 00000000..6af6be80 --- /dev/null +++ b/openspec/changes/beta-tool-faults/proposal.md @@ -0,0 +1,115 @@ +## Why + +23 of the 156 tools are rated beta. Each one works but has a known fault: it +ignores an input it advertises, reports a wrong value, or fails on a valid +model. Each fault is written down in the tool's description and in the agent +skill's workaround table, which makes it honest, but an agent still has to read +around it. Some faults give a wrong answer with no warning: the moments of +inertia are off by a factor of a million, and a ring of holes made with +`pattern_circular` is reported as a failure. The maintainer chose +reliability before new features (2026-11-09): fix the beta tools first, then +work up the support levels. + +## What Changes + +The 23 tools fall into four groups. + +**Wrong results (4 tools)** + +- `calculate_mass_properties`: moments of inertia are converted from kg·m² to + kg·mm², so the label is true. Its four unused inputs are removed. +- `pattern_circular` and `mirror_feature`: succeed when the volume changes in + either direction, so patterning or mirroring a cut works. They still fail + when the volume doesn't change. +- `delete_feature`: names the dependent features it removed, found by comparing + the feature tree before and after. + +**Inputs that are now honoured (8 tools)** + +- `create_extrusion`: `sketch_name` picks the sketch, the same way + `create_cut_extrude` already does. +- `create_revolve`: `sketch_name` picks the sketch, and `axis_entity`, when it + names a reference axis, is the axis. +- `create_sketch`: `sketch_name` renames the new sketch. The reply always + carries the real name. +- `create_part`: `material` is applied. +- `check_interference`: `include_hidden` reaches the adapter. +- `save_as`: refuses to replace an existing file unless `overwrite` is on. +- `execute_macro`: runs `repeat_count` times, pausing `pause_between_runs` + between runs. +- `get_material`: also returns the density. + +**Inputs that are removed (10 tools)** + +- `create_extrusion`'s `direction`, which duplicates `end_condition`, and + `create_revolve`'s `direction`, which duplicates `both_directions`. The + revolve reply also stops echoing `direction`. +- `create_assembly`'s `components`. Use `insert_component` instead. +- `create_drawing`'s `model_path` and `sheet_format`. +- `create_technical_drawing`'s `template`, `sheet_size`, `title` and + `output_path`. +- `save_file`'s `force_save`. +- `options` on the five `export_*` tools, and the unused `ExportSTEPInput`, + `ExportIGESInput` and `ExportSTLInput` classes. +- `add_drawing_view` now refuses an `operation` other than `add`. + +**BREAKING:** a call that still sends a removed input gets an `invalid_input` +error that names the tool to use instead. Today the input is silently ignored. + +**Wrong guidance (1 tool)** + +- `sketch_tutorial_simple_hole` tells the agent to use `create_cut_extrude` for + the hole, not a negative-depth extrusion. + +**For every tool** + +- A live SolidWorks test proves the fix. +- The tool moves to `stable` in `support_levels.py`. +- Its row is removed from the skill's known-faults table. +- Its description no longer mentions the fault. +- The progress page is regenerated. + +## Capabilities + +### New Capabilities + +- `tools/input-contract`: a tool either honours each input it advertises or + refuses it. A removed input is refused with a pointer to the right tool. +- `tools/mass-properties`: the units and values `calculate_mass_properties` + reports. +- `tools/feature-patterns`: when `pattern_circular` and `mirror_feature` + succeed, including for cuts. +- `tools/feature-deletion`: what `delete_feature` reports about the dependent + features it removed. +- `tools/document-creation`: what `create_part`, `create_sketch`, + `create_assembly` and `create_drawing` apply. +- `tools/file-saving`: `save_as` and `overwrite`. +- `tools/macro-execution`: `execute_macro` repeats. + +### Modified Capabilities + +- `tools/extrusion`: `sketch_name` picks the sketch, and the same applies to + `create_revolve`. + +`tools/support-levels` is not modified. Its existing rule, that a stable tool +is called by a live test, is what each promotion here has to meet. + +## Impact + +- **Tools:** `modeling.py`, `sketching.py`, `analysis.py`, `drawing.py`, + `export.py`, `file_management.py`, `macro_recording.py`, + `engineering_data.py`, and their input models. +- **Adapters (COM risk):** `adapters/solidworks/features.py` (shared sketch + resolution for extrude and revolve, the revolve axis, the pattern and mirror + checks, delete's before/after names), `sketch.py` (renaming the new sketch), + `documentation.py` (density), and `pywin32_adapter.py` (macro repeats). + All COM work stays inside `_handle_com_operation` on the connect thread. + The mock adapter changes to match. +- **Skill:** `skills/solidworks-mcp/references/errors-and-limits.md` loses 11 + rows. `add_mate`'s edge row stays. +- **Docs:** the support levels in `support_levels.py`, the generated progress + page, and the CHANGELOG. +- **Tests:** unit tests beside each module. A new + `tests/live/test_live_sw_beta_tools.py` holds one live test per tool, + run in batches with the user's permission. +- **Issue:** none filed yet. This is item 7 in `openspec/queue.md`. diff --git a/openspec/changes/beta-tool-faults/specs/tools/document-creation/spec.md b/openspec/changes/beta-tool-faults/specs/tools/document-creation/spec.md new file mode 100644 index 00000000..3b22ca64 --- /dev/null +++ b/openspec/changes/beta-tool-faults/specs/tools/document-creation/spec.md @@ -0,0 +1,36 @@ +## Purpose + +What the document and sketch creation tools apply from their inputs, and the +names they report back. + +## ADDED Requirements + +### Requirement: A new part gets the material asked for + +`create_part` with `material` SHALL assign that material to the new part. When +the material isn't found, the part SHALL still be created, and the reply SHALL +say the material wasn't applied and which library was searched. + +#### Scenario: A known material +- **WHEN** an agent creates a part with `material: "AISI 304"` +- **THEN** reading the part's material afterwards returns AISI 304 + +#### Scenario: An unknown material +- **WHEN** an agent creates a part with `material: "Unobtainium"` +- **THEN** the part is created, and the reply carries a `warning` that the material wasn't found + +### Requirement: A new sketch has the name asked for + +`create_sketch` with `sketch_name` SHALL give the new sketch that name in the +feature tree when `exit_sketch` closes it, because SolidWorks won't rename an +open sketch. Every reply's sketch name SHALL be the name the feature tree shows +at that moment: `create_sketch` gives SolidWorks' name and the wanted one as +`name_on_exit`, and `exit_sketch` gives the final name. + +#### Scenario: A named sketch +- **WHEN** an agent creates a sketch with `sketch_name: "HoleProfile"`, draws in it and closes it with `exit_sketch` +- **THEN** the feature tree shows `HoleProfile`, `exit_sketch` replies with that name, and `create_cut_extrude` with `sketch_name: "HoleProfile"` uses it + +#### Scenario: A name already taken +- **WHEN** the requested name is already in the tree +- **THEN** the sketch keeps SolidWorks' own name, and `exit_sketch` replies with that name and a `warning` saying why diff --git a/openspec/changes/beta-tool-faults/specs/tools/extrusion/spec.md b/openspec/changes/beta-tool-faults/specs/tools/extrusion/spec.md new file mode 100644 index 00000000..b951cc9e --- /dev/null +++ b/openspec/changes/beta-tool-faults/specs/tools/extrusion/spec.md @@ -0,0 +1,27 @@ +## ADDED Requirements + +### Requirement: The named sketch is the one used + +`create_extrusion` and `create_revolve` SHALL use the sketch `sketch_name` +names, found in the feature tree without regard to case. When the name matches +no sketch, the call SHALL fail before anything is built, and the message SHALL +list the sketches that exist. When `sketch_name` is empty, they SHALL use the +sketch the session closed last, as `create_cut_extrude` does. + +#### Scenario: Extruding an earlier sketch +- **WHEN** a part has `Sketch1` (a 20 mm square) and `Sketch2` (a Ø10 circle), both closed, and an agent extrudes `sketch_name: "Sketch1"` 5 mm +- **THEN** the new body is 20 × 20 × 5 mm, 2,000 mm³ + +#### Scenario: A sketch that doesn't exist +- **WHEN** an agent extrudes `sketch_name: "Sketch9"` +- **THEN** the call fails, naming `Sketch1` and `Sketch2`, and the feature tree is unchanged + +### Requirement: The revolve axis can be a reference axis + +`create_revolve` with `axis_entity` naming a reference axis SHALL revolve about +that axis. Without `axis_entity`, it SHALL revolve about the centreline drawn +in the sketch. + +#### Scenario: Revolving about Axis1 +- **WHEN** a sketch holds a 10 × 20 mm rectangle 5 mm from `Axis1`, with no centreline, and an agent revolves it 360° about `axis_entity: "Axis1"` +- **THEN** the body is a tube with volume π·(15² − 5²)·20 = 12,566.4 mm³ within 0.1 % diff --git a/openspec/changes/beta-tool-faults/specs/tools/feature-deletion/spec.md b/openspec/changes/beta-tool-faults/specs/tools/feature-deletion/spec.md new file mode 100644 index 00000000..0d3928ae --- /dev/null +++ b/openspec/changes/beta-tool-faults/specs/tools/feature-deletion/spec.md @@ -0,0 +1,16 @@ +## Purpose + +What `delete_feature` reports, so an agent knows every feature that went, +including the dependent features SolidWorks removed along with the named one. + +## ADDED Requirements + +### Requirement: The reply names every feature removed + +`delete_feature` SHALL return `removed_features`, the names of every feature +that was in the tree before the call and isn't after it. Its message SHALL +name the dependent features too. + +#### Scenario: Deleting a sketch that a feature uses +- **WHEN** an agent deletes `Sketch2`, which `Cut-Extrude1` was built from +- **THEN** `removed_features` lists both `Sketch2` and `Cut-Extrude1`, and the message says the cut went with it diff --git a/openspec/changes/beta-tool-faults/specs/tools/feature-patterns/spec.md b/openspec/changes/beta-tool-faults/specs/tools/feature-patterns/spec.md new file mode 100644 index 00000000..b132d1b2 --- /dev/null +++ b/openspec/changes/beta-tool-faults/specs/tools/feature-patterns/spec.md @@ -0,0 +1,24 @@ +## Purpose + +When `pattern_circular` and `mirror_feature` count as having worked, for +features that add material and features that remove it. + +## ADDED Requirements + +### Requirement: A pattern or mirror succeeds when the volume changes + +`pattern_circular` and `mirror_feature` SHALL succeed when the part's volume +changes, up or down, and SHALL fail when it doesn't change. The reply SHALL +include the volume before and after. + +#### Scenario: A ring of holes +- **WHEN** an agent patterns a Ø6 mm through-hole in a 10 mm plate 6 times around an axis, with every instance on the plate +- **THEN** the call succeeds and the volume falls by 5 × π·3²·10 = 1,413.7 mm³ within 0.1 % + +#### Scenario: A mirrored cut +- **WHEN** an agent mirrors a cut about a plane, and the mirrored cut lands on the body +- **THEN** the call succeeds and the volume falls by the cut's volume + +#### Scenario: Nothing patterned +- **WHEN** a pattern's instances all land off the body, so the volume is unchanged +- **THEN** the call fails and says the pattern produced no geometry diff --git a/openspec/changes/beta-tool-faults/specs/tools/file-saving/spec.md b/openspec/changes/beta-tool-faults/specs/tools/file-saving/spec.md new file mode 100644 index 00000000..6202317a --- /dev/null +++ b/openspec/changes/beta-tool-faults/specs/tools/file-saving/spec.md @@ -0,0 +1,19 @@ +## Purpose + +How `save_as` treats a file that already exists at the target path. + +## ADDED Requirements + +### Requirement: save_as keeps an existing file unless told to overwrite + +`save_as` SHALL refuse to write when a file already exists at `file_path` and +`overwrite` is off, and SHALL leave that file untouched. With `overwrite` on, it +SHALL replace the file. + +#### Scenario: The file exists and overwrite is off +- **WHEN** an agent saves the active part as a path that already holds a file +- **THEN** the reply is an `invalid_input` error naming the path and `overwrite`, and the file's contents and modified time are unchanged + +#### Scenario: Overwrite is on +- **WHEN** the same call has `overwrite: true` +- **THEN** the file is replaced and the call succeeds diff --git a/openspec/changes/beta-tool-faults/specs/tools/input-contract/spec.md b/openspec/changes/beta-tool-faults/specs/tools/input-contract/spec.md new file mode 100644 index 00000000..682303b2 --- /dev/null +++ b/openspec/changes/beta-tool-faults/specs/tools/input-contract/spec.md @@ -0,0 +1,77 @@ +## Purpose + +A tool's input schema is a promise: every field it advertises changes what the +tool does, and an input the tool no longer takes is refused, not ignored. + +## ADDED Requirements + +### Requirement: Advertised inputs are honoured + +Every field in a stable tool's input schema SHALL change what the tool does or +what it reports. A tool SHALL NOT echo back an input it didn't apply. + +#### Scenario: An input the tool applies +- **WHEN** an agent calls `create_part` with `material: "AISI 304"` +- **THEN** the new part's material is AISI 304, and the reply says so + +#### Scenario: No echo of an unapplied input +- **WHEN** a tool's reply names a value the agent passed +- **THEN** that value is what SolidWorks now holds, not only what was asked for + +### Requirement: Removed inputs are refused with a pointer + +When a call sends an input the tool no longer takes, the tool SHALL return +`status: "error"` with `error_type: "invalid_input"` before anything changes in +SolidWorks. The message SHALL name the input and say which tool or input to use +instead. The removed inputs are: + +- `create_extrusion.direction` → `end_condition` +- `create_revolve.direction` → `both_directions` +- `create_assembly.components` → `insert_component` +- `create_drawing.model_path` and `create_drawing.sheet_format` → + `create_drawing_view` or `generate_drawing_package` +- `create_technical_drawing.template`, `.sheet_size`, `.title` and + `.output_path` → `generate_drawing_package` +- `save_file.force_save` → none: the tool always saves +- `check_interference.tolerance` and `.check_all_components` → none: the + check has no tolerance and always covers the whole assembly, or the + `components` given +- `export_step.options`, `export_iges.options`, `export_stl.options`, + `export_pdf.options` and `export_dwg.options` → the file extension +- `calculate_mass_properties.model_path`, `.units`, `.include_hidden` and + `.reference_coordinate_system` → open and activate the model first + +#### Scenario: A removed input is sent +- **WHEN** an agent calls `create_assembly` with `components: ["a.sldprt"]` +- **THEN** the reply is an `invalid_input` error naming `components` and `insert_component`, and no assembly is created + +#### Scenario: An operation the tool doesn't do +- **WHEN** an agent calls `add_drawing_view` with `operation: "remove"` +- **THEN** the reply is an `invalid_input` error saying the tool only adds views, and the drawing is unchanged + +### Requirement: Interference checks follow include_hidden + +`check_interference` SHALL leave hidden components out of the check unless +`include_hidden` is on. + +#### Scenario: A hidden component overlaps +- **WHEN** two components overlap and one is hidden +- **THEN** the check finds no interference by default, and finds one with `include_hidden: true` + +### Requirement: get_material reports the density + +`get_material` SHALL return the active part's material density in kg/m³ beside +its name and library, or `null` when no material is assigned. + +#### Scenario: A steel part +- **WHEN** an agent reads the material of a part assigned AISI 304 +- **THEN** the reply gives the name AISI 304 and the density SolidWorks holds for it, 8,000 kg/m³ + +### Requirement: The hole tutorial teaches the cut tool + +`sketch_tutorial_simple_hole` SHALL tell the agent to make the hole with +`create_cut_extrude`, never with a negative-depth extrusion. + +#### Scenario: Reading the tutorial +- **WHEN** an agent calls `sketch_tutorial_simple_hole` +- **THEN** its steps call `create_cut_extrude` for the hole and none passes a negative `depth` diff --git a/openspec/changes/beta-tool-faults/specs/tools/macro-execution/spec.md b/openspec/changes/beta-tool-faults/specs/tools/macro-execution/spec.md new file mode 100644 index 00000000..421a0d32 --- /dev/null +++ b/openspec/changes/beta-tool-faults/specs/tools/macro-execution/spec.md @@ -0,0 +1,21 @@ +## Purpose + +How many times `execute_macro` runs a macro, and what its reply says about the +runs. + +## ADDED Requirements + +### Requirement: A macro runs repeat_count times + +`execute_macro` SHALL run the macro `repeat_count` times, waiting +`pause_between_runs` seconds between runs. It SHALL stop at the first run that +fails. The reply SHALL give the number of runs that completed, and the message +SHALL state that number, not the number asked for. + +#### Scenario: Three runs +- **WHEN** an agent runs a macro that adds one feature with `repeat_count: 3` +- **THEN** three features are added and the reply says 3 runs completed + +#### Scenario: A run fails part way +- **WHEN** the second of three runs fails +- **THEN** the reply is an error saying 1 run completed and the second failed, and no third run is attempted diff --git a/openspec/changes/beta-tool-faults/specs/tools/mass-properties/spec.md b/openspec/changes/beta-tool-faults/specs/tools/mass-properties/spec.md new file mode 100644 index 00000000..24cc4b23 --- /dev/null +++ b/openspec/changes/beta-tool-faults/specs/tools/mass-properties/spec.md @@ -0,0 +1,16 @@ +## Purpose + +The units and values `calculate_mass_properties` and `get_mass_properties` +report, so every number matches the unit printed beside it. + +## ADDED Requirements + +### Requirement: Moments of inertia in kg·mm² + +`calculate_mass_properties` SHALL report the moments and products of inertia +in kg·mm², labelled `kg·mm²`, matching the millimetres used for the centre of +mass. + +#### Scenario: A steel block +- **WHEN** an agent measures a 100 × 50 × 20 mm block of material with density 7,800 kg/m³ (mass 0.78 kg), about its centre of mass +- **THEN** `Ixx`, `Iyy` and `Izz` match m·(b² + c²)/12 for each axis in kg·mm² within 0.1 %, for example m·(50² + 20²)/12 = 188.5 kg·mm² about the 100 mm axis diff --git a/openspec/changes/beta-tool-faults/tasks.md b/openspec/changes/beta-tool-faults/tasks.md new file mode 100644 index 00000000..ed103f6f --- /dev/null +++ b/openspec/changes/beta-tool-faults/tasks.md @@ -0,0 +1,80 @@ +## 1. Shared pieces + +- [x] 1.1 `input_compat.py`: `REMOVED_INPUTS` and `reject_removed_inputs`, called from `normalize_input`; tests for a removed field (refused with its pointer), a kept alias (accepted), and an unknown extra (still accepted) +- [x] 1.2 `features.py`: lift `_resolve_profile_sketch` out of the cut-extrude code; the cut uses it unchanged (existing cut tests pass untouched) + +## 2. Sketch and feature inputs (live batch 1) + +- [x] 2.1 `create_extrusion`: pass `sketch_name` through `ExtrusionParameters` to the resolver; remove `direction`; mock matches; tests for a named sketch, a missing sketch, and no name +- [x] 2.2 `create_revolve`: `sketch_name` through the resolver, `axis_entity` optional and selected with mark 4 when it names a reference axis; remove `direction` and its echo; tests +- [x] 2.3 `create_sketch`: rename the new sketch to `sketch_name`; on refusal keep the original name and return a `warning`; the reply always gives the tree's name; tests. Live batch 1 showed an open sketch can't be renamed, so `exit_sketch` gives the name (design decision 5, settled) +- [x] 2.4 `create_part`: apply `material` after the part opens; an unknown material gives a `warning`, not an error; tests +- [x] 2.5 Live batch 1 in `tests/live/test_live_sw_beta_tools.py` (extrude an earlier sketch, a missing sketch, revolve about `Axis1`, a named sketch used by a cut, a part with AISI 304); run with the user's permission; settle design decision 5's route + +## 3. Patterns, mirror and delete (live batch 2) + +- [x] 3.1 `pattern_circular` and `mirror_feature`: any volume change beyond the tolerance succeeds; add `volume_change`; mock and tests for growth, a cut, and no change +- [x] 3.2 `delete_feature`: the adapter returns `removed_features` by name from before/after lists; the tool's message names the dependents; mock and tests +- [x] 3.3 Live batch 2 (a ring of 6 holes, a mirrored cut, deleting a sketch with its cut) + +## 4. Measurements (live batch 3) + +- [x] 4.1 `calculate_mass_properties`: inertia ×1e6 to kg·mm²; remove its 4 inputs; tests including the 100 × 50 × 20 mm block's hand values +- [x] 4.2 `check_interference`: `include_hidden` → `ignore_hidden`; remove `tolerance` and `check_all_components`; tests +- [x] 4.3 `get_material`: density in kg/m³ or `null`; mock and tests +- [x] 4.4 Live batch 3 (block inertia, a hidden overlapping component both ways, AISI 304 density) + +## 5. Files and macros (live batch 4) + +- [x] 5.1 `save_as`: refuse an existing path unless `overwrite`; tests that the file is untouched +- [x] 5.2 `save_file`: remove `force_save`; the five `export_*` tools: remove `options`; delete `ExportSTEPInput`, `ExportIGESInput` and `ExportSTLInput`; tests +- [x] 5.3 `execute_macro`: loop `repeat_count` (1-100) with `pause_between_runs`, stop at the first failure, `runs_completed` in the reply; decide each other unused field by design decision 1 and record it here; tests. Decided: `proc_name` (read by the adapter but undocumented) is now a field; `parameters`, `execution_parameters`, `target_file`, `execution_mode`, `pause_on_error` and `log_execution` are removed (nothing read them) +- [x] 5.4 Live batch 4 (`save_as` over an existing file both ways, `save_file`, each export, a macro run 3 times) + +## 6. Drawings and the tutorial (live batch 5) + +- [x] 6.1 `create_assembly`: remove `components`; `create_drawing`: remove `model_path` and `sheet_format`; tests +- [x] 6.2 `create_technical_drawing`: remove `template`, `sheet_size`, `title` and `output_path`; tests +- [x] 6.3 `add_drawing_view`: refuse an `operation` other than `add`; tests +- [x] 6.4 `sketch_tutorial_simple_hole`: its hole step uses `create_cut_extrude`; test that no step passes a negative depth +- [x] 6.5 Live batch 5 (an assembly, a drawing, three standard views, `add_drawing_view` refusing `remove`, the tutorial's steps run as written) + +## 7. Promote and document + +- [x] 7.1 Each tool's description loses its fault sentence and states what the honoured inputs do +- [x] 7.2 `support_levels.py`: the 23 tools move to `stable`; regenerate with `python -m solidworks_mcp.progress.generate` +- [x] 7.3 `errors-and-limits.md`: remove the 11 fixed rows (the `add_mate` edge row stays); the queue's "Small faults" list loses them +- [x] 7.4 CHANGELOG: **Fixed** (the wrong results), **Changed** (every removed input, with its replacement; inertia now in kg·mm²) +- [x] 7.5 Full mock suite at 100% coverage, ruff, `mkdocs build --strict` +- [ ] 7.6 Live-demo sign-off with the user before the PR + +## Notes from the implementation + +- `create_technical_drawing` had three more unread fields than design + decision 1 listed (`sheet_format`, `scale`, `auto_populate_views`); all are + removed by the same rule. `projection`, which the adapter already read but + the schema didn't show, is now a field. +- `calculate_mass_properties` had a branch for an adapter method no adapter + implements; it returned raw numbers and skipped the unit conversion, so it + is deleted. +- Without `sketch_name`, `create_extrusion` still uses the sketch `exit_sketch` + just closed without reselecting it, as before, so a thin wall from an open + chain keeps working; only a named sketch goes through the strict resolver. +- `get_material` reads the density from the material's `.sldmat` library + entry (the `DENS` value, kg/m³), the same files `list_available_materials` + reads. +- Live batch 2: `EditDelete` would not delete a sketch a cut used ("did + not remove"). `delete_feature` now calls `DeleteSelection2` with + `swDelete_Children` (1; `swDelete_Absorbed` is 2, both read from + swconst.tlb). That still returned False with the sketch selected: + SolidWorks won't delete a sketch a feature uses. So when the plain delete + returns False, the features built from it (`IFeature::GetChildren`) are + deleted with `swDelete_Children | swDelete_Absorbed`, which takes the + sketch with them. Deleting a cut still leaves its absorbed sketch. +- Inertia products (batch 3): the path SolidWorks 2026 takes, + `IMassProperty.GetMomentOfInertia(0)`, returns the row-major 3 x 3 tensor, + and the adapter's indices 1, 2 and 5 are Ixy, Ixz and Iyz, as they should + be. The fallback for older versions, `IModelDoc2.GetMassProperties`, ends + Lxy, Lzx, Lyz (API docs) and was mapping index 10 to Iyz and 11 to Ixz; + fixed, with a test of the order. The live block test passes (its products + are zero, so it checks the moments only). diff --git a/openspec/queue.md b/openspec/queue.md index 2fc20177..c0c1c737 100644 --- a/openspec/queue.md +++ b/openspec/queue.md @@ -6,7 +6,7 @@ reviewed, then implemented, signed off live where it touches SolidWorks, and archived after its PR merges. Update this file when an item is done or the order changes. -Last updated 2026-11-07. +Last updated 2026-11-09. | # | Item | Issue | OpenSpec change | State | | --- | --- | --- | --- | --- | @@ -14,19 +14,20 @@ Last updated 2026-11-07. | 1 | Tool descriptions agents can act on: one description standard, MCP hints and family tags, and a check that keeps them | #82 (docstring half) | `tool-descriptions-for-agents` (archived) | Done 2026-11-05 (#202, #203, #204). Left: after the next release, ask Glama for a rescan and record the score in the internal assessment | | 2 | Guidance through MCP and an installable skill: the server's `instructions`, the skill served as `skill://` resources with FastMCP 3.4.9's `SkillProvider`, and the same skill installable into an agent | #175 | `agent-skill-and-instructions` (archived) | Done 2026-11-06 (#205, #211; #147 closed as superseded) | | 3 | The circuit breaker locks a tool out after a success and never reopens; a reset tool | #208 | `circuit-breaker-recovery` (archived) | Done 2026-11-06 (#213) | -| 4 | Support levels for every tool, and COM coverage progress bars at the top of the README with an interactive docs page | #79 (coverage half) | `tool-support-levels-and-progress` (archived) | Done 2026-11-06 (#214). #79's licence-tier half is item 8 | +| 4 | Support levels for every tool, and COM coverage progress bars at the top of the README with an interactive docs page | #79 (coverage half) | `tool-support-levels-and-progress` (archived) | Done 2026-11-06 (#214). #79's licence-tier half is item 13 | | 5 | Tool gaps found building the U-joint: face mates, fixed parts and flexible sub-assemblies, angled planes, angled chamfers, `both_directions`, a stale interference result, thin extrudes | #209 | `ujoint-tool-gaps` (archived) | Done 2026-11-07 (#216), signed off live | -| 6 | `--selftest` command and a compatibility-report issue template | #179 | `selftest-and-compatibility-report` | In progress 2026-11-07 on `feat/selftest` | -| 7 | Skills that improve the agent's results, measured on the U-joint prompt (Opus 5.5 at medium effort: the first skill changed how the agent worked, not what it built) | #207 | not yet written | | -| 8 | README: licence-tier tags | #79 (other half) | not yet written | | -| 9 | Remove the stale `validate_coverage.py`; decide whether mypy gates CI | #157 | not yet written | | -| 10 | Monitor stand demo follow-ups | #206 | not yet written | | -| 11 | Clear out duplicate and overlapping tools: compare each cluster, keep one, settle a verb list, deprecate old names for one release | #174 | not yet written | Bottom of the queue, by request 2026-11-04 | +| 6 | `--selftest` command and a compatibility-report issue template | #179 | `selftest-and-compatibility-report` (archived) | Done 2026-11-09 (#217), signed off live | +| 7 | Beta tools to stable: fix each beta tool's known fault (the small faults listed below), prove it live, and move it up a level | not filed yet | `beta-tool-faults` | Next, by request 2026-11-09: reliability before new features, moving up the support levels one at a time | +| 8 | Experimental tools to stable: a live test for each of the 10, then promote it or record its fault | not filed yet | not yet written | | +| 9 | Monitor stand demo follow-ups | #206 | not yet written | | +| 10 | The 40 planned tools: implement each one or take it out of the tool list (listed below). A registered tool that can't run is a promise the server doesn't keep | not filed yet | not yet written | Check against #174 first, which may retire some as duplicates | +| 11 | Clear out duplicate and overlapping tools: compare each cluster, keep one, settle a verb list, deprecate old names for one release | #174 | not yet written | | | 12 | Optional toolsets or a discovery mode, to cut the tool list's size | #176 | not yet written | After #174, which decides which families are core | -| 13 | Make the "not available yet" tools work, or retire them: each is registered but no adapter implements it, so every call returns `not_supported` (listed below) | not filed yet | not yet written | After #174, which may retire some of them as duplicates | -| 14 | Small faults found while describing the tools (listed below) | not filed yet | not yet written | Fixing one moves its tool up a support level (item 4) | +| 13 | README: licence-tier tags | #79 (other half) | not yet written | | +| 14 | Remove the stale `validate_coverage.py`; decide whether mypy gates CI | #157 | not yet written | | +| 15 | Skills that improve the agent's results, measured on the U-joint prompt | #207 | not yet written | Tabled 2026-11-09: needs research first (what "better" means, a harness to run and score sessions). See `openspec/research/agent-skill-evaluation.md` | -## Tools that can't run yet (item 13) +## Tools that can't run yet (item 10) Each one's description says "Not available yet ... planned for a future release" (the wording is in `docs/agents/tool-description-standard.md`). @@ -57,7 +58,7 @@ Found while rewriting the descriptions (39), plus `tutorial_simple_hole` - **Sketching (1):** `tutorial_simple_hole`: its cut step fails on SolidWorks (it calls the adapter's `create_cut`, which only the mock has). -## Small faults (item 14) +## Small faults (item 7) When one is fixed, remove it from the agent skill's `src/solidworks_mcp/skills/solidworks-mcp/references/errors-and-limits.md` @@ -65,35 +66,14 @@ in the same change, and move its tool up a level in `src/solidworks_mcp/tools/support_levels.py` (beta tools list the fault as their reason), then run `python -m solidworks_mcp.progress.generate`. -- `execute_macro` ignores `repeat_count`: it runs the macro once, but its - message says "executed N times". -- `create_technical_drawing` ignores `template`, `sheet_size`, `title` and - `output_path`; it only places three views on the active drawing. -- `add_drawing_view` takes an `operation` (add, update, remove) but only - ever adds a view; it is the same code as `create_drawing_view`. -- `create_extrusion` ignores `sketch_name` and `direction`, and - `create_revolve` ignores `sketch_name` and `axis_entity`: both use the - sketch `exit_sketch` closed (the revolve its centreline). `create_cut_extrude` - does honour `sketch_name`. -- `create_part` ignores `material`, `create_assembly` ignores `components`, - `create_drawing` ignores `model_path` and `sheet_format`, and - `create_sketch` echoes `sketch_name` back as the sketch's name without - applying it, so the reply can name a sketch that doesn't exist. -- `pattern_circular` and `mirror_feature` fail unless the volume grows, so - patterning or mirroring a cut (a ring of holes) is reported as an error. -- `calculate_mass_properties` labels the moments of inertia kg·mm², but they - are SolidWorks' kg·m² (a factor of a million); its input fields are unused. -- `delete_feature` reads `removed_features`, but the real adapter returns - `features_removed` (a count), so on SolidWorks its message never mentions - the dependent features that went too. -- `sketch_tutorial_simple_hole` suggests a negative-depth extrusion for the - hole instead of `create_cut_extrude`. -- `save_as` ignores `overwrite` (an existing file is replaced) and - `save_file` ignores `force_save`; the `export_*` file tools ignore - `options` (`format_type` is optional since #209: the extension decides); - `ExportSTEPInput`, `ExportIGESInput` and `ExportSTLInput` are unused. -- `check_interference` passes `include_hidden` where the adapter reads - `ignore_hidden`, so it has no effect. +- `create_drawing_view` and `add_drawing_view` place a view wherever + `position` says, even off the sheet, and report success (seen in the beta + live tests, 2026-11-09). They could read the sheet size (`_sheet_bounds` + in `adapters/solidworks/documentation.py`) and refuse or warn. +- `export_stl` logs `Extension.SaveAs2 failed: Type mismatch` on its first + attempt, then writes the file through its fallback (seen on SolidWorks + 2026, 2026-11-09). The export works; the first attempt's arguments need + checking. ## Notes diff --git a/openspec/research/agent-skill-evaluation.md b/openspec/research/agent-skill-evaluation.md new file mode 100644 index 00000000..a3eaff8c --- /dev/null +++ b/openspec/research/agent-skill-evaluation.md @@ -0,0 +1,69 @@ +# Research: measuring whether a skill improves the agent (#207) + +Tabled on 2026-11-09, by the maintainer's decision. The skill work in #207 needs +research before it can become an OpenSpec change. This note records why and +what the research has to answer, so the work can restart from here. + +## Why it's tabled + +The one measurement so far (#207) found that the skill changed *how* the agent +worked, not *what* it built. Running the comparison again with a different +skill won't show more on its own: + +- One U-joint run takes over 20 minutes and about 400k output tokens, and each + arm needs several runs before a difference means anything. +- There's no harness to run the sessions, collect their tool calls and score + the results. Comparing many prompt or skill variants would mean building one, + and that harness is effectively a small framework of its own. +- "Better" isn't defined yet. Without a definition, a run can't pass or fail. + +## Questions the research has to answer + +### 1. What does "better" or "more robust" mean? + +These are candidate measures. They can disagree with one another, so the +research has to pick which matter and how to weigh them: + +- **Result quality:** do the parts match the answer key (volumes, bounding + boxes, mates, motion)? This is the measure #207 says counts most. +- **Fewer tool calls:** less work for the same result. +- **Time and tokens:** wall-clock time and output tokens per finished model. +- **Tool choice:** did the agent pick the right tool the first time, or reach + a result through workarounds (for example, building planes from angle and + perpendicular pairs)? +- **Planning:** did it plan the build before calling tools? Did it replan + after an error rather than repeat it? Did it check its work (`verify_model`) + at sensible points? +- **Failures:** how many calls errored, how many errors repeated, and did any + leave the model in a broken state? +- **Robustness:** how much the result varies across runs of the same arm, and + across models and effort levels. + +### 2. How do we run it? + +- A harness that runs N sessions per arm (with and without the skill, or with + skill A and skill B). It has to record the model, the effort level, every + tool call with its result, the time and the tokens. +- A scorer that grades each session against the answer key in + `tests/live/data/ujoint/` and against the measures chosen in question 1. +- Whether shorter prompts (one part, one sub-assembly) can stand in for the + full U-joint. Short prompts would make many runs affordable; the open + question is whether their results predict the full build. +- Whether tool calls can be optimised offline. One option is to replay + recorded sessions against the mock adapter, so prompt variants are compared + without SolidWorks. The research has to say what that misses: the mock has + no geometry. + +### 3. What could a skill teach that the tools don't? + +From #207: mates that leave motion free, recipes for reference geometry, +sheet-metal and thin-plate features, design-intent checks, and when to stop +and ask about an ambiguous spec. Each is a hypothesis for the harness to +test, not a skill to write yet. + +## Before restarting + +Read the current literature and tooling on evaluating agents and tool use +(agent benchmarks, trajectory scoring, LLM-as-judge for planning quality), and +what others measure when they compare prompts or skills. Then turn questions +1 and 2 into an OpenSpec change for the harness. Write the skills after that. diff --git a/openspec/specs/docs/compatibility-reports/spec.md b/openspec/specs/docs/compatibility-reports/spec.md new file mode 100644 index 00000000..0d3f0851 --- /dev/null +++ b/openspec/specs/docs/compatibility-reports/spec.md @@ -0,0 +1,42 @@ +# docs/compatibility-reports Specification + +## Purpose +Give users a form to send their selftest results, and show everyone which +SolidWorks releases the server has passed the selftest on. +## Requirements +### Requirement: A compatibility issue form + +The repository SHALL offer a "Compatibility report" issue form that asks +for the selftest summary (required), the SolidWorks version and service +pack (required), the Windows version, and the MCP client and model. Its +summary field SHALL accept being filled from the link the selftest prints. +The form SHALL label new issues `compatibility`. + +#### Scenario: The selftest link fills the form + +- **WHEN** the user opens the link the selftest printed +- **THEN** the compatibility form opens with the summary field already holding the selftest's summary + +### Requirement: The form is easy to find + +The README's "Tested on" line and the feedback form SHALL point to the +compatibility form and say how to run the selftest. + +#### Scenario: A user on SolidWorks 2022 reads the README + +- **WHEN** a user reads that the server is tested on SolidWorks 2026 +- **THEN** the same line tells them to run `solidworks-mcp-python selftest` and send the result with the compatibility form + +### Requirement: The releases verified so far are listed + +The docs SHALL have a page listing each SolidWorks release and service +pack the selftest has run on, with the date, each area's result and a link +to the report. It SHALL start with SolidWorks 2026 SP4.0 from the live +test. A release with a failing area SHALL still be listed, with the area +that failed. + +#### Scenario: A report with a failure is added + +- **WHEN** a report from SolidWorks 2024 SP5 shows the drawing area failing +- **THEN** the page lists 2024 SP5 with part, assembly and export PASS, drawing FAIL, and a link to that issue + diff --git a/openspec/specs/setup/selftest/spec.md b/openspec/specs/setup/selftest/spec.md new file mode 100644 index 00000000..744da04e --- /dev/null +++ b/openspec/specs/setup/selftest/spec.md @@ -0,0 +1,100 @@ +# setup/selftest Specification + +## Purpose +`solidworks-mcp-python selftest` checks a user's SolidWorks setup through +the same MCP tools an agent calls, and prints a report they can send us. +## Requirements +### Requirement: One command checks the setup + +`solidworks-mcp-python selftest` SHALL run the selftest, and +`solidworks-mcp-python --selftest` SHALL do the same. `--year` SHALL be +accepted as it is when starting the server. The selftest SHALL drive +SolidWorks through the registered MCP tools, called with plain dict +arguments, so it exercises the same path an MCP client does. + +#### Scenario: The flag spelling works + +- **WHEN** the user runs `solidworks-mcp-python --selftest --year 2026` +- **THEN** the selftest runs, exactly as `solidworks-mcp-python selftest --year 2026` would + +### Requirement: The environment is reported + +The report SHALL give the SolidWorks revision and language, the default +part, assembly and drawing templates with whether each file exists, the +`--year` in use, the operating system, and the Python, pywin32 and +package versions. A value that cannot be read SHALL be reported as +unknown, never guessed, and SHALL NOT stop the selftest. + +#### Scenario: A missing template is reported, not fatal + +- **WHEN** the default drawing template points at a file that does not exist +- **THEN** the environment section shows that path marked missing, and the other areas still run + +### Requirement: Each area is checked against a hand calculation + +The selftest SHALL check four areas in order: part, assembly, drawing and +export. Each check SHALL compare a value read back from SolidWorks with an +expected value worked out by hand, within a stated tolerance: + +- part: volume and bounding box after the extrude, after the hole and + after the fillet; +- assembly: component count, mate count, and no interference; +- drawing: the drawing has exactly one view of the part; +- export: the STEP file exists and is not empty. + +An area whose setup step fails SHALL be FAIL with the tool's error message. +An area that needs a file an earlier area did not save (the assembly, +drawing and export use the saved parts) SHALL be SKIP, naming what it +needed. An area whose files were saved SHALL still run when an earlier +area failed a check, so one wrong value does not hide the other results. + +#### Scenario: A wrong volume fails its area + +- **WHEN** the part's volume after the hole differs from the hand calculation by more than the tolerance +- **THEN** the part area is FAIL, and its line shows the expected value, the measured value and the tolerance + +#### Scenario: A failed part skips the rest + +- **WHEN** the part area fails before the part is saved +- **THEN** the assembly, drawing and export areas are SKIP, each saying it needs the part + +### Requirement: The report can be pasted into an issue + +The selftest SHALL print one line per area (PASS, FAIL or SKIP, with the +checks passed out of the checks run), then a Markdown summary holding the +environment, each area's checks and the tools called. When the summary +fits, it SHALL also print a link that opens the compatibility issue form +with the summary filled in. With `--report ` it SHALL also write the +summary to that file. The report SHALL NOT include file paths under the +user's profile beyond the templates' file names, nor any MCP client +configuration. + +#### Scenario: A long summary still gets to the form + +- **WHEN** the summary is too long for a link +- **THEN** the selftest prints the plain issue-form link and tells the user to paste the summary printed above it + +### Requirement: Exit codes say what happened + +The selftest SHALL exit 0 when every area passes, 1 when any area fails or +is skipped, and 2 when it cannot run at all: not on Windows, SolidWorks not +installed, or SolidWorks not running. Exit 2 SHALL print what to do next. + +#### Scenario: SolidWorks is not running + +- **WHEN** the user runs the selftest and SolidWorks is not running +- **THEN** it does not start SolidWorks, prints "Start SolidWorks, wait for it to finish loading, then run this again", and exits 2 + +### Requirement: The user's documents are left alone + +The selftest SHALL NOT save, close, activate-and-change or rebuild any +document that was open before it started. It SHALL save only into a new +temporary folder it creates, SHALL close only documents it opened, without +saving, and SHALL delete that folder at the end, also when an area fails +or the run is interrupted. + +#### Scenario: A user's part stays open and unchanged + +- **WHEN** the user has an unsaved part open and runs the selftest +- **THEN** after the selftest that part is still open, still unsaved, and still has the same features, and every document the selftest made is closed + diff --git a/pyproject.toml b/pyproject.toml index 4d1469f4..8bc0c9c7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "solidworks-mcp-python" -version = "1.5.0" +version = "2.0.0" description = "Python MCP server for SolidWorks: lets Claude and other AI agents build parts, assemblies and drawings through the SolidWorks COM API." # The README with relative links made absolute for PyPI: see the fancy-pypi-readme hook. dynamic = ["readme"] diff --git a/server.json b/server.json index 2adff7e1..fd7cbcb4 100644 --- a/server.json +++ b/server.json @@ -8,12 +8,12 @@ "source": "github" }, "websiteUrl": "https://andrewbartels1.com/SolidworksMCP-python/", - "version": "1.5.0", + "version": "2.0.0", "packages": [ { "registryType": "pypi", "identifier": "solidworks-mcp-python", - "version": "1.5.0", + "version": "2.0.0", "runtimeHint": "uvx", "transport": { "type": "stdio" diff --git a/src/solidworks_mcp/adapters/base.py b/src/solidworks_mcp/adapters/base.py index debbc89f..e3b0ae0b 100644 --- a/src/solidworks_mcp/adapters/base.py +++ b/src/solidworks_mcp/adapters/base.py @@ -246,6 +246,10 @@ class RevolveParameters(BaseModel): reverse_direction (bool): The reverse direction value. thin_feature (bool): The thin feature value. thin_thickness (float | None): The thin thickness value. + sketch_name (str | None): The sketch to revolve, by its tree name; + ``None`` for the sketch closed last. + axis_entity (str | None): A reference axis to revolve about, by its + tree name; ``None`` for the sketch's own centreline. """ angle: float @@ -254,6 +258,8 @@ class RevolveParameters(BaseModel): thin_feature: bool = False thin_thickness: float | None = None merge_result: bool = True + sketch_name: str | None = None + axis_entity: str | None = None class SweepParameters(BaseModel): diff --git a/src/solidworks_mcp/adapters/mock_adapter.py b/src/solidworks_mcp/adapters/mock_adapter.py index fe5be055..f3749db9 100644 --- a/src/solidworks_mcp/adapters/mock_adapter.py +++ b/src/solidworks_mcp/adapters/mock_adapter.py @@ -1179,6 +1179,11 @@ async def create_extrusion( if isinstance(params, str): params = ExtrusionParameters(depth=depth or 0.0) + sketch = self._mock_named_sketch( + getattr(params, "sketch_name", None), "extrude" + ) + if isinstance(sketch, AdapterResult): + return sketch await asyncio.sleep(self._delays["feature_operation"]) self._operation_count += 1 @@ -1200,6 +1205,7 @@ async def create_extrusion( "thin_feature": params.thin_feature, "thin_thickness": params.thin_thickness, "thin_side": params.thin_side if params.thin_feature else None, + "sketch_name": sketch, }, properties={"created": datetime.now().isoformat(), "mock": True}, ) @@ -1229,6 +1235,19 @@ async def create_revolve( status=AdapterResultStatus.ERROR, error="No active model" ) + sketch = self._mock_named_sketch( + getattr(params, "sketch_name", None), "revolve" + ) + if isinstance(sketch, AdapterResult): + return sketch + axis = getattr(params, "axis_entity", None) + if axis and axis not in self._axes: + return AdapterResult( + status=AdapterResultStatus.ERROR, + error=f"Axis '{axis}' is not a reference axis in the " + "model. Create one with create_axis.", + ) + await asyncio.sleep(self._delays["feature_operation"]) self._operation_count += 1 @@ -1243,6 +1262,8 @@ async def create_revolve( "both_directions": params.both_directions, "thin_feature": params.thin_feature, "thin_thickness": params.thin_thickness, + "sketch_name": sketch, + "axis_entity": axis, }, properties={"created": datetime.now().isoformat(), "mock": True}, ) @@ -1256,6 +1277,66 @@ async def create_revolve( execution_time=self._delays["feature_operation"], ) + def _mock_rename_sketch(self, old: str, new: str) -> AdapterResult[dict[str, Any]]: + """Rename a mock sketch, refusing a name already in use. + + Args: + old: The sketch's current name. + new: The name to give it. + + Returns: + AdapterResult[dict[str, Any]]: As ``rename_feature`` reports it. + The open sketch is refused, as SolidWorks refuses it (it accepts + the new name but the tree keeps the old one). + """ + if self._current_sketch == old: + return AdapterResult( + status=AdapterResultStatus.ERROR, + error=f"Cannot rename '{old}' while it is open for editing", + ) + taken = new in self._sketches or any( + f.name == new for f in self._features.values() + ) + if taken: + return AdapterResult( + status=AdapterResultStatus.ERROR, + error=f"Cannot rename to '{new}': a feature with that name already exists", + ) + self._sketches = { + (new if name == old else name): plane + for name, plane in self._sketches.items() + } + return AdapterResult( + status=AdapterResultStatus.SUCCESS, + data={"renamed": True, "old_name": old, "new_name": new}, + execution_time=self._delays["model_operation"], + ) + + def _mock_named_sketch( + self, name: str | None, purpose: str + ) -> str | None | AdapterResult[SolidWorksFeature]: + """The mock sketch ``name`` matches, without regard to case. + + Args: + name: The sketch asked for, or ``None`` for the last closed. + purpose: How the error names the use, e.g. ``"extrude"``. + + Returns: + The sketch's name, ``None`` when none was asked for, or an error + result listing the sketches when ``name`` matches none. + """ + if not name: + return None + for real in self._sketches: + if real.lower() == name.lower(): + return real + available = ", ".join(self._sketches) or "(none found)" + return AdapterResult( + status=AdapterResultStatus.ERROR, + error=f"Sketch '{name}' not found in the feature tree. Sketches " + f"available to {purpose}: {available}", + ) + async def create_sweep( self, params: SweepParameters ) -> AdapterResult[SolidWorksFeature]: @@ -2998,6 +3079,8 @@ async def rename_feature( status=AdapterResultStatus.ERROR, error="new_name must not be empty" ) + if old_name in self._sketches and new != old_name: + return self._mock_rename_sketch(old_name, new) key = next((k for k, f in self._features.items() if f.name == old_name), None) if key is None: return AdapterResult( @@ -3471,6 +3554,7 @@ async def mirror_feature( "mirror_bodies": bool(mirror_bodies), "volume_before": volume_before, "volume_after": volume_after, + "volume_change": volume_after - volume_before, "volume_ratio": round(volume_after / volume_before, 6), }, execution_time=self._delays["feature_operation"], @@ -3573,6 +3657,7 @@ async def pattern_circular( "equal_spacing": bool(equal_spacing), "volume_before": volume_before, "volume_after": volume_after, + "volume_change": volume_after - volume_before, "volume_ratio": round(volume_after / volume_before, 6), }, execution_time=self._delays["feature_operation"], @@ -3606,6 +3691,13 @@ async def pattern_circular( }, ) _MOCK_MATERIAL_FOLDER = "mock://sldmaterials" + #: Densities in kg/m³, as SolidWorks 2026's library holds them. + _MOCK_DENSITIES: dict[str, float] = { + "AISI 1020": 7900.0, + "AISI 304": 8000.0, + "6061-T6 (SS)": 2700.0, + "ABS": 1020.0, + } def _mock_doc_error( self, allowed: set[str], operation: str @@ -3692,6 +3784,9 @@ async def get_material( data={ "material": assigned["name"] if assigned else None, "database": assigned["database"] if assigned else None, + "density_kg_m3": ( + self._MOCK_DENSITIES.get(assigned["name"]) if assigned else None + ), "configuration": configuration, }, ) diff --git a/src/solidworks_mcp/adapters/solidworks/documentation.py b/src/solidworks_mcp/adapters/solidworks/documentation.py index 5412cd48..dbbe42d6 100644 --- a/src/solidworks_mcp/adapters/solidworks/documentation.py +++ b/src/solidworks_mcp/adapters/solidworks/documentation.py @@ -151,6 +151,65 @@ def _parse_material_library(path: str) -> list[dict[str, str]]: return materials +def _library_files(folders: list[str]) -> list[str]: + """The ``.sldmat`` files in ``folders``, folder by folder, sorted by name. + + Args: + folders: Material library folders; missing ones are skipped. + + Returns: + list[str]: Full paths. + """ + files: list[str] = [] + for folder in folders: + if os.path.isdir(folder): + files.extend( + os.path.join(folder, entry) + for entry in sorted(os.listdir(folder)) + if entry.lower().endswith(".sldmat") + ) + return files + + +def _material_density(files: list[str], name: str, database: str) -> float | None: + """A material's mass density in kg/m³, from its ``.sldmat`` entry. + + The library whose file name matches ``database`` is searched first, then + the others. The value is the ``DENS`` element's ``value``, which the + libraries store in kg/m³. + + Args: + files: ``.sldmat`` paths to search. + name: The material's name, as ``GetMaterialPropertyName2`` gives it. + database: Its library's name, the file name without extension. + + Returns: + float | None: The density, or ``None`` when no entry has one. + """ + wanted = database.lower() + ordered = sorted( + files, + key=lambda f: os.path.splitext(os.path.basename(f))[0].lower() != wanted, + ) + for path in ordered: + try: + root = ET.parse(path).getroot() + except (ET.ParseError, OSError, DefusedXmlException): + continue + for element in root.iter(): + if element.tag.rsplit("}", 1)[-1] != "material": + continue + if element.get("name") != name: + continue + for prop in element.iter(): + if prop.tag.rsplit("}", 1)[-1].upper() == "DENS": + try: + return float(prop.get("value", "")) + except ValueError: + return None + return None + + def _unpack_custom_property(raw: Any) -> tuple[str, str]: """Pull (value, resolved value) out of an ``ICustomPropertyManager::Get6`` result. @@ -509,8 +568,9 @@ async def get_material( configuration: Configuration name; the active one when omitted. Returns: - AdapterResult[dict[str, Any]]: ``material`` (``None`` when unset) - and ``database``. + AdapterResult[dict[str, Any]]: ``material`` (``None`` when unset), + ``database`` and ``density_kg_m3``, read from the material's entry + in its ``.sldmat`` library (``None`` when unset or not found). """ guard = self._require_doc_type({1}, "get_material") if guard is not None: @@ -519,9 +579,18 @@ async def get_material( def _get() -> dict[str, Any]: part = _sw_type_info.flagged(self.currentModel, "IPartDoc") name, database = _read_material(part, configuration) + app = getattr(self, "swApp", None) + density = ( + _material_density( + _library_files(self._material_folders(app)), name, database + ) + if name and app is not None + else None + ) return { "material": name or None, "database": database or None, + "density_kg_m3": density, "configuration": configuration, } @@ -584,6 +653,40 @@ def _set() -> dict[str, Any]: self._handle_com_operation("set_material", _set), ) + def _material_folders(self, app: Any) -> list[str]: + """The material library folders SolidWorks reads, in its order. + + ``swFileLocationsMaterialDatabases`` lists only the folders the user + added: "" on a fresh SW 2025 install, just "Custom Materials" on SW + 2026 (both measured live). SolidWorks always shows its built-in + ``sldmaterials`` library under ``/lang/`` as well, + so that is added. + + Args: + app: The connected ``ISldWorks``. + + Returns: + list[str]: Folder paths, which may not all exist. + """ + configured = str( + self._attempt( + lambda: app.GetUserPreferenceStringValue(_SW_PREF_MATERIAL_DATABASES), + default="", + ) + or "" + ) + exe_dir = str(self._attempt(lambda: app.GetExecutablePath(), default="") or "") + language = str( + self._attempt(lambda: app.GetCurrentLanguage(), default="") or "english" + ) + if exe_dir.lower().endswith(".exe"): + exe_dir = os.path.dirname(exe_dir) + built_in = os.path.join(exe_dir, "lang", language, "sldmaterials") + folder_list = [f.strip() for f in configured.split(";") if f.strip()] + if os.path.normcase(built_in) not in map(os.path.normcase, folder_list): + folder_list.append(built_in) + return folder_list + async def list_available_materials( self, search: str | None = None ) -> AdapterResult[list[dict[str, str]]]: @@ -612,42 +715,11 @@ async def list_available_materials( searched: dict[str, Any] = {} def _list() -> list[dict[str, str]]: - configured = str( - self._attempt( - lambda: app.GetUserPreferenceStringValue( - _SW_PREF_MATERIAL_DATABASES - ), - default="", - ) - or "" - ) - # swFileLocationsMaterialDatabases lists only the folders the user - # added: "" on a fresh SW 2025 install, just "Custom Materials" on - # SW 2026 (both measured live). SolidWorks always shows its - # built-in \lang\\sldmaterials library as - # well, so read that too. - exe_dir = str( - self._attempt(lambda: app.GetExecutablePath(), default="") or "" - ) - language = str( - self._attempt(lambda: app.GetCurrentLanguage(), default="") or "english" - ) - if exe_dir.lower().endswith(".exe"): - exe_dir = os.path.dirname(exe_dir) - built_in = os.path.join(exe_dir, "lang", language, "sldmaterials") - folder_list = [f.strip() for f in configured.split(";") if f.strip()] - if os.path.normcase(built_in) not in map(os.path.normcase, folder_list): - folder_list.append(built_in) + folder_list = self._material_folders(app) folders = ";".join(folder_list) materials: list[dict[str, str]] = [] - for folder in folder_list: - if not os.path.isdir(folder): - continue - for entry in sorted(os.listdir(folder)): - if entry.lower().endswith(".sldmat"): - materials.extend( - _parse_material_library(os.path.join(folder, entry)) - ) + for path in _library_files(folder_list): + materials.extend(_parse_material_library(path)) searched["folders"] = folder_list searched["total"] = len(materials) if not materials: diff --git a/src/solidworks_mcp/adapters/solidworks/features.py b/src/solidworks_mcp/adapters/solidworks/features.py index be553139..278b43ab 100644 --- a/src/solidworks_mcp/adapters/solidworks/features.py +++ b/src/solidworks_mcp/adapters/solidworks/features.py @@ -108,7 +108,34 @@ async def add_chamfer( return _add_chamfer_impl(self, distance, edge_names, angle, flip) async def delete_feature(self, name: str) -> AdapterResult[dict[str, Any]]: - return _delete_feature_impl(self, name) + """Delete a feature and report every feature that went with it. + + The tree is listed before and after with ``list_features``, the read + path that works on SolidWorks, and the names that disappeared become + ``removed_features``: the named feature and its dependents. + + Args: + name: The feature or sketch to delete. + + Returns: + AdapterResult[dict[str, Any]]: The delete's data plus + ``removed_features`` in tree order (just ``name`` when the tree + can't be listed). + """ + before = await self.list_features(include_suppressed=True) + result = _delete_feature_impl(self, name) + if not result.is_success: + return result + after = await self.list_features(include_suppressed=True) + data = dict(result.data or {}) + data["removed_features"] = _removed_names(before, after) or [ + name.split("@", 1)[0] + ] + return AdapterResult( + status=AdapterResultStatus.SUCCESS, + data=data, + execution_time=result.execution_time, + ) async def suppress_feature( self, name: str, suppress: bool = True @@ -191,9 +218,10 @@ async def pattern_circular( returns ``None`` while a selection is active. Note that instances landing off the parent body are silently dropped - by SolidWorks, so the volume rises by less than ``count - 1`` times the - seed. That is correct behaviour, not a failure - only *no* growth at - all means the pattern did nothing. + by SolidWorks, so the volume changes by less than ``count - 1`` times + the seed. That is correct behaviour, not a failure - only *no* change + at all means the pattern did nothing. A pattern of a cut lowers the + volume, a pattern of a boss raises it; both count. Args: features: Names of the features to pattern. @@ -206,8 +234,8 @@ async def pattern_circular( Returns: AdapterResult[dict[str, Any]]: What was patterned and the volume - before and after. ``ERROR`` when nothing could be selected or the - volume did not grow. + before, after and its change. ``ERROR`` when nothing could be + selected or the volume did not change. """ adapter = self._adapter(self) if not adapter.currentModel: @@ -238,7 +266,7 @@ async def pattern_circular( "were created is unknown." ), ) - if volume_after <= volume_before * 1.0001: + if abs(volume_after - volume_before) <= volume_before * 0.0001: return AdapterResult( status=AdapterResultStatus.ERROR, error=( @@ -250,15 +278,7 @@ async def pattern_circular( ), ) - data = dict(result.data or {}) - data["volume_before"] = volume_before - data["volume_after"] = volume_after - data["volume_ratio"] = round(volume_after / volume_before, 6) - return AdapterResult( - status=AdapterResultStatus.SUCCESS, - data=data, - execution_time=result.execution_time, - ) + return _with_volume_change(result, volume_before, volume_after) async def mirror_feature( self, @@ -271,7 +291,8 @@ async def mirror_feature( ``InsertMirrorFeature`` returns a Feature object even when it mirrored nothing, so the model's volume is measured before and after and the - call is reported as failed if the volume did not grow. That check runs + call is reported as failed if the volume did not change (a mirrored + cut lowers it, a mirrored boss or body raises it). That check runs here rather than inside the COM closure because ``get_mass_properties`` is the read path known to work - reading ``CreateMassProperty().Volume`` inline returns ``None`` when a plane is still in the selection set, @@ -293,8 +314,8 @@ async def mirror_feature( Returns: AdapterResult[dict[str, Any]]: What was mirrored, and the volume - before and after. ``ERROR`` when nothing was selected, the call - failed, or the volume did not grow. + before, after and its change. ``ERROR`` when nothing was selected, + the call failed, or the volume did not change. """ adapter = self._adapter(self) if not adapter.currentModel: @@ -325,7 +346,7 @@ async def mirror_feature( "produced is unknown." ), ) - if volume_after <= volume_before * 1.001: + if abs(volume_after - volume_before) <= volume_before * 0.001: return AdapterResult( status=AdapterResultStatus.ERROR, error=( @@ -338,15 +359,34 @@ async def mirror_feature( ), ) - data = dict(result.data or {}) - data["volume_before"] = volume_before - data["volume_after"] = volume_after - data["volume_ratio"] = round(volume_after / volume_before, 6) - return AdapterResult( - status=AdapterResultStatus.SUCCESS, - data=data, - execution_time=result.execution_time, - ) + return _with_volume_change(result, volume_before, volume_after) + + +def _with_volume_change( + result: AdapterResult[dict[str, Any]], before: float, after: float +) -> AdapterResult[dict[str, Any]]: + """A pattern or mirror's result, with the volume it changed. + + Args: + result: The feature call's successful result. + before: The part's volume before, in mm³. + after: The part's volume after, in mm³. + + Returns: + AdapterResult[dict[str, Any]]: ``result``'s data plus + ``volume_before``, ``volume_after``, ``volume_change`` (negative for a + cut) and ``volume_ratio``. + """ + data = dict(result.data or {}) + data["volume_before"] = before + data["volume_after"] = after + data["volume_change"] = after - before + data["volume_ratio"] = round(after / before, 6) + return AdapterResult( + status=AdapterResultStatus.SUCCESS, + data=data, + execution_time=result.execution_time, + ) #: ``end_condition`` spellings accepted by ``create_extrusion``, keyed by the @@ -484,6 +524,14 @@ def _extrusion_operation() -> SolidWorksFeature: f"Unsupported thin_side {normalized.thin_side!r}; use one of: " + ", ".join(_THIN_SIDES) ) + # A named sketch is found and selected first; without a name the + # sketch exit_sketch just closed is still selected, as before. + requested = getattr(params, "sketch_name", None) or None + sketch_used = ( + _resolve_profile_sketch(adapter, requested, purpose="extrude") + if requested + else None + ) feature_manager = adapter.currentModel.FeatureManager t0 = adapter.constants.get("swStartSketchPlane", 0) end_name, t1 = _extrusion_end_condition( @@ -616,6 +664,7 @@ def _extrusion_operation() -> SolidWorksFeature: "thin_side": normalized.thin_side if normalized.thin_feature else None, "both_directions": normalized.both_directions, "end_condition": end_name, + "sketch_name": sketch_used, }, properties={"created": datetime.now().isoformat()}, ) @@ -626,6 +675,45 @@ def _extrusion_operation() -> SolidWorksFeature: ) +def _select_revolve_inputs(adapter: Any, params: RevolveParameters) -> str | None: + """Select a revolve's sketch and axis when they are named. + + With neither named, nothing is selected: the sketch ``exit_sketch`` just + closed and its centreline are used, as before. A named axis needs the + sketch selected first (mark 0), then the axis appended with mark 4, the + mark ``FeatureRevolve2`` reads the axis from. + + Args: + adapter: A connected adapter with a valid ``currentModel``. + params: The revolve's parameters. + + Returns: + str | None: The sketch's tree name, or ``None`` when none was named. + + Raises: + Exception: The sketch or the axis isn't in the model. + """ + sketch_name = getattr(params, "sketch_name", None) + axis = getattr(params, "axis_entity", None) + if not sketch_name and not axis: + return None + sketch = _resolve_profile_sketch(adapter, sketch_name, purpose="revolve") + if axis: + selected = adapter._attempt( + lambda: adapter.currentModel.Extension.SelectByID2( + axis, "AXIS", 0.0, 0.0, 0.0, True, 4, _null_callout(), 0 + ), + default=False, + ) + if not selected: + raise Exception( + f"Axis '{axis}' is not a reference axis in the model. Create one " + "with create_axis, or draw the axis in the sketch with " + "add_centerline and leave axis_entity out." + ) + return sketch + + def _create_revolve_impl( adapter: Any, params: RevolveParameters ) -> AdapterResult[SolidWorksFeature]: @@ -679,6 +767,8 @@ def _revolve_operation() -> SolidWorksFeature: Raises: Exception: If ``FeatureRevolve2`` returns ``None``. """ + sketch_used = _select_revolve_inputs(adapter, params) + # Detect SW major version for FeatureRevolve2 API choice revolve_sw_major = 0 if getattr(adapter, "swApp", None): @@ -770,6 +860,8 @@ def _revolve_operation() -> SolidWorksFeature: "both_directions": params.both_directions, "thin_feature": params.thin_feature, "thin_thickness": params.thin_thickness, + "sketch_name": sketch_used, + "axis_entity": getattr(params, "axis_entity", None), }, properties={"created": datetime.now().isoformat()}, ) @@ -1206,6 +1298,108 @@ def _tangency(value: str | None) -> int: ) +def _resolve_profile_sketch( + adapter: Any, sketch_name: str | None, *, purpose: str +) -> str: + """Find the sketch a feature is built from, and select it. + + Sketch resolution never guesses: ``sketch_name`` is matched against the + live feature tree without regard to case; without it, the sketch the + session closed last is used only if that name is really in the tree. + Anything else raises, listing the sketches that exist. The sketch is then + selected with ``SelectByID2`` so the feature call never relies on + SolidWorks' implicit active-sketch state. Runs inside the caller's + ``_handle_com_operation`` closure. + + Args: + adapter: A connected adapter with a valid ``currentModel``. + sketch_name: The sketch to use, or ``None`` for the last closed. + purpose: How the error messages name the use, e.g. ``"cut from"``. + + Returns: + str: The sketch's name as the tree spells it. + + Raises: + Exception: The sketch isn't in the tree, or can't be selected. + """ + # --- Sketch resolution: explicit > tracked-and-verified > clear + # error. Never a silent guess — see the docstring above. Resolving + # this up front (before any FeatureCut attempt) also lets every + # failure below name the sketch it was actually targeting. + tree_names = _profile_feature_names(adapter) + + def _match_tree_name(name: str) -> str | None: + for real in tree_names: + if real.lower() == name.lower(): + return real + return None + + available = ", ".join(tree_names) if tree_names else "(none found)" + if sketch_name: + target_sketch = _match_tree_name(sketch_name) + if target_sketch is None: + raise Exception( + f"Sketch '{sketch_name}' not found in the " + f"feature tree. Sketches available to {purpose}: {available}" + ) + else: + last_tracked = getattr(adapter, "_last_sketch_name", None) + target_sketch = _match_tree_name(last_tracked) if last_tracked else None + if target_sketch is None: + hint = ( + f" (the last create_sketch call returned '{last_tracked}', " + "which does not match any real sketch in the model — " + "SolidWorks likely didn't hand back a usable sketch " + "object synchronously)" + if last_tracked + else " (no create_sketch/exit_sketch call has run this session)" + ) + raise Exception( + "No sketch_name given, and no reliably-tracked sketch to " + f"{purpose}{hint}. Pass sketch_name explicitly. Sketches " + f"available to {purpose}: {available}" + ) + + # Explicit selection before any FeatureCut attempt. SolidWorks' own + # "implicit" active-sketch state (whatever was last open) is never + # relied upon — that is exactly the race that made this feature + # intermittently cut nothing while reporting no error at all. + adapter._attempt(lambda: adapter.currentModel.ClearSelection2(True), default=None) + # Callout must be an explicit VT_DISPATCH null, not plain None — a + # bare None marshals as VT_NULL and SolidWorks rejects the whole call + # with DISP_E_TYPEMISMATCH (runbook item 11). Using + # _attempt_with_error (not _attempt) here means that error surfaces + # in the failure message below instead of being swallowed into a + # generic "selection failed" guess. + select_result, select_error = adapter._attempt_with_error( + lambda: adapter.currentModel.Extension.SelectByID2( + target_sketch, + "SKETCH", + 0.0, + 0.0, + 0.0, + False, + 0, + _null_callout(), + 0, + ) + ) + sketch_selected = bool(select_result) + if not sketch_selected: + reason = ( + f" COM error: {select_error}" + if select_error is not None + else " SelectByID2 returned False with no COM error — the " + "sketch may already be consumed by another feature, be " + "suppressed, or not be a valid selectable profile." + ) + raise Exception( + f"Failed to select sketch '{target_sketch}' via SelectByID2." + reason + ) + adapter._last_sketch_name = target_sketch + return target_sketch + + def _create_cut_extrude_impl( adapter: Any, params: ExtrusionParameters ) -> AdapterResult[SolidWorksFeature]: @@ -1320,83 +1514,9 @@ def _cut_operation() -> SolidWorksFeature: ) feature_manager = adapter.currentModel.FeatureManager - # --- Sketch resolution: explicit > tracked-and-verified > clear - # error. Never a silent guess — see the docstring above. Resolving - # this up front (before any FeatureCut attempt) also lets every - # failure below name the sketch it was actually targeting. - tree_names = _profile_feature_names(adapter) - - def _match_tree_name(name: str) -> str | None: - for real in tree_names: - if real.lower() == name.lower(): - return real - return None - - available = ", ".join(tree_names) if tree_names else "(none found)" - if normalized.sketch_name: - target_sketch = _match_tree_name(normalized.sketch_name) - if target_sketch is None: - raise Exception( - f"Sketch '{normalized.sketch_name}' not found in the " - f"feature tree. Sketches available to cut from: {available}" - ) - else: - last_tracked = getattr(adapter, "_last_sketch_name", None) - target_sketch = _match_tree_name(last_tracked) if last_tracked else None - if target_sketch is None: - hint = ( - f" (the last create_sketch call returned '{last_tracked}', " - "which does not match any real sketch in the model — " - "SolidWorks likely didn't hand back a usable sketch " - "object synchronously)" - if last_tracked - else " (no create_sketch/exit_sketch call has run this session)" - ) - raise Exception( - "No sketch_name given, and no reliably-tracked sketch to " - f"cut from{hint}. Pass sketch_name explicitly. Sketches " - f"available to cut from: {available}" - ) - - # Explicit selection before any FeatureCut attempt. SolidWorks' own - # "implicit" active-sketch state (whatever was last open) is never - # relied upon — that is exactly the race that made this feature - # intermittently cut nothing while reporting no error at all. - adapter._attempt( - lambda: adapter.currentModel.ClearSelection2(True), default=None + target_sketch = _resolve_profile_sketch( + adapter, normalized.sketch_name, purpose="cut from" ) - # Callout must be an explicit VT_DISPATCH null, not plain None — a - # bare None marshals as VT_NULL and SolidWorks rejects the whole call - # with DISP_E_TYPEMISMATCH (runbook item 11). Using - # _attempt_with_error (not _attempt) here means that error surfaces - # in the failure message below instead of being swallowed into a - # generic "selection failed" guess. - select_result, select_error = adapter._attempt_with_error( - lambda: adapter.currentModel.Extension.SelectByID2( - target_sketch, - "SKETCH", - 0.0, - 0.0, - 0.0, - False, - 0, - _null_callout(), - 0, - ) - ) - sketch_selected = bool(select_result) - if not sketch_selected: - reason = ( - f" COM error: {select_error}" - if select_error is not None - else " SelectByID2 returned False with no COM error — the " - "sketch may already be consumed by another feature, be " - "suppressed, or not be a valid selectable profile." - ) - raise Exception( - f"Failed to select sketch '{target_sketch}' via SelectByID2." + reason - ) - adapter._last_sketch_name = target_sketch end_condition = (normalized.end_condition or "Blind").strip().lower() blind = adapter.constants["swEndCondBlind"] @@ -2322,16 +2442,91 @@ def _is_suppressed(adapter: Any, feature: Any) -> bool | None: return None +def _removed_names( + before: AdapterResult[list[dict[str, Any]]], + after: AdapterResult[list[dict[str, Any]]], +) -> list[str]: + """The feature names listed before a delete and missing after it. + + Args: + before: ``list_features`` before the delete. + after: ``list_features`` after it. + + Returns: + list[str]: The names that went, in tree order; empty when either + listing failed. + """ + if not (before.is_success and after.is_success): + return [] + remaining = {str(f.get("name")) for f in after.data or []} + return [ + str(f.get("name")) + for f in before.data or [] + if f.get("name") and str(f.get("name")) not in remaining + ] + + +#: ``swDeleteSelectionOptions_e`` (read from swconst.tlb, 2026): +#: ``swDelete_Children`` deletes the features built on the selection too, +#: ``swDelete_Absorbed`` a feature's own sketch with it. +_DELETE_CHILDREN = 1 +_DELETE_ABSORBED = 2 + + +def _delete_users_of(adapter: Any, feature: Any) -> tuple[Any, Exception | None]: + """Delete the features built from ``feature``, with their absorbed sketch. + + SolidWorks won't delete a sketch a feature still uses: ``DeleteSelection2`` + returns False even with ``swDelete_Children`` (measured on 2026). Deleting + the features that use it (``IFeature::GetChildren``) with + ``swDelete_Absorbed`` takes the sketch with them. + + Args: + adapter: A connected adapter with a valid ``currentModel``. + feature: The ``IFeature`` that could not be deleted on its own. + + Returns: + tuple[Any, Exception | None]: ``DeleteSelection2``'s return value and + what it raised; ``(False, None)`` when nothing uses ``feature``. + """ + children = adapter._attempt( + lambda: adapter._get_attr_or_call(feature, "GetChildren"), default=None + ) + names = [ + str(name) + for child in children or () + if (name := adapter._attempt(lambda c=child: c.Name, default=None)) + ] + if not names: + return False, None + adapter._attempt(lambda: adapter.currentModel.ClearSelection2(True), default=None) + for child_name in names: + _select_feature_by_name(adapter, child_name, append=True) + return cast( + tuple[Any, Exception | None], + adapter._attempt_with_error( + lambda: adapter.currentModel.Extension.DeleteSelection2( + _DELETE_CHILDREN | _DELETE_ABSORBED + ) + ), + ) + + def _delete_feature_impl(adapter: Any, name: str) -> AdapterResult[dict[str, Any]]: """Delete a named feature or sketch from the active model. - Selects the feature, then removes it with ``IModelDoc2::EditDelete``. - Deleting a parent also removes its children, which is SolidWorks' normal - cascade and the intended "erase this and what depends on it" behaviour - - so the number of features removed is reported, not assumed to be one. + Selects the feature, then removes it with + ``IModelDocExtension::DeleteSelection2(swDelete_Children)``, which takes + the features built on it too ("erase this and what depends on it"), so + the number of features removed is reported, not assumed to be one. + ``IModelDoc2::EditDelete`` left a sketch in place when a cut used it + (measured on SolidWorks 2026). A feature's own sketch is "absorbed", not + a child, so deleting a cut leaves its sketch. A sketch a feature uses + can't be deleted on its own at all, so its users are deleted instead, + taking it with them (:func:`_delete_users_of`). - ``EditDelete`` returns nothing useful, so success is confirmed by the - feature being absent from the tree afterwards. + Success is confirmed by the feature being absent from the tree + afterwards, not by the call's return value. Args: adapter: A connected adapter with a valid ``currentModel``. @@ -2349,23 +2544,34 @@ def _delete_feature_impl(adapter: Any, name: str) -> AdapterResult[dict[str, Any def _delete_operation() -> dict[str, Any]: before = _feature_count(adapter) - if _select_feature_by_name(adapter, name) is None: + feature = _select_feature_by_name(adapter, name) + if feature is None: raise Exception(f"Feature not found: {name}") - adapter._attempt( - lambda: adapter._get_attr_or_call(adapter.currentModel, "EditDelete"), + selected = adapter._attempt( + lambda: adapter.currentModel.SelectionManager.GetSelectedObjectCount2(-1), default=None, ) + deleted, error = adapter._attempt_with_error( + lambda: adapter.currentModel.Extension.DeleteSelection2(_DELETE_CHILDREN) + ) + if not deleted and error is None: + deleted, error = _delete_users_of(adapter, feature) adapter._attempt(lambda: adapter.currentModel.EditRebuild3(), default=None) - # EditDelete reports nothing useful, so confirm by lookup: the - # feature must no longer resolve by name. + # Confirm by lookup: the feature must no longer resolve by name. bare = name.split("@", 1)[0] still_there = adapter._attempt( lambda: adapter.currentModel.FeatureByName(bare), default=None ) if still_there: - raise Exception(f"EditDelete did not remove feature: {name}") + outcome = ( + f"raised {error}" if error is not None else f"returned {deleted!r}" + ) + raise Exception( + f"SolidWorks did not delete feature: {name} (DeleteSelection2 " + f"{outcome} with {selected} item(s) selected)" + ) after = _feature_count(adapter) return { @@ -2543,6 +2749,11 @@ def _rename_operation() -> dict[str, Any]: actual = adapter._attempt( lambda: adapter._get_attr_or_call(refreshed, "Name"), default=None ) + renamed_to = actual if isinstance(actual, str) and actual else new + # A sketch renamed while it is the session's latest keeps being the + # one a feature without a sketch_name uses. + if getattr(adapter, "_last_sketch_name", None) == bare: + adapter._last_sketch_name = renamed_to return { "renamed": True, "old_name": bare, diff --git a/src/solidworks_mcp/adapters/solidworks/io.py b/src/solidworks_mcp/adapters/solidworks/io.py index a19c8cb0..634b56dd 100644 --- a/src/solidworks_mcp/adapters/solidworks/io.py +++ b/src/solidworks_mcp/adapters/solidworks/io.py @@ -2374,14 +2374,17 @@ def _get() -> MassProperties: surface_area = raw[4] * 1e6 mass = raw[5] + # GetMassProperties gives Lxx, Lyy, Lzz, Lxy, Lzx, Lyz at + # 6-11 (API docs); moi is the row-major 3 x 3 tensor that + # IMassProperty.GetMomentOfInertia returns. moi = [0.0] * 9 if len(raw) >= 12: moi[0] = raw[6] moi[4] = raw[7] moi[8] = raw[8] - moi[1] = raw[9] - moi[5] = raw[10] - moi[2] = raw[11] + moi[1] = raw[9] # Lxy + moi[2] = raw[10] # Lzx = Ixz + moi[5] = raw[11] # Lyz return MassProperties( volume=volume, diff --git a/src/solidworks_mcp/skills/__init__.py b/src/solidworks_mcp/skills/__init__.py index cc93e1ce..d42af58f 100644 --- a/src/solidworks_mcp/skills/__init__.py +++ b/src/solidworks_mcp/skills/__init__.py @@ -22,7 +22,7 @@ - Start with `get_model_info` (or `list_open_documents`) to see what is open. - Build in this order: `create_part`, `create_sketch`, draw (`add_rectangle`, \ `add_circle`, ...), `exit_sketch`, then the feature (`create_extrusion`, \ -`create_cut_extrude`, `create_revolve`). Features use the sketch you closed last. +`create_cut_extrude`, `create_revolve`). Features use the sketch `sketch_name` \nnames, or the one you closed last. - Inputs are millimetres and degrees unless a field says metres. - A tool's success does not prove the geometry is right: finish a build with \ `verify_model`, giving the size, volume or counts you expect. diff --git a/src/solidworks_mcp/skills/solidworks-mcp/SKILL.md b/src/solidworks_mcp/skills/solidworks-mcp/SKILL.md index 972657db..00fc4fd0 100644 --- a/src/solidworks_mcp/skills/solidworks-mcp/SKILL.md +++ b/src/solidworks_mcp/skills/solidworks-mcp/SKILL.md @@ -25,17 +25,17 @@ A solid feature always comes from a closed sketch: 1. `create_part` (or `open_model` for an existing file). 2. `create_sketch` on a plane: "Front", "Top", "Right", or a plane from - `create_reference_plane`. Don't pass `sketch_name`; the reply gives - SolidWorks' own name for the sketch. + `create_reference_plane`. Pass `sketch_name` to name it; the name is + given when `exit_sketch` closes it, and that reply has the final name. 3. Draw: `add_rectangle`, `add_circle`, `add_line`, `add_arc`, `add_polygon`, and so on. For a revolve, add the axis with `add_centerline` in this - sketch. + sketch, or make a reference axis with `create_axis`. 4. `exit_sketch`. 5. The feature: `create_extrusion` (add material), `create_cut_extrude` (remove it), `create_revolve`, `create_sweep`, `create_loft`. - `create_extrusion` and `create_revolve` use the sketch you just closed, - whatever name you pass, so call them straight after `exit_sketch`. - `create_cut_extrude` takes `sketch_name`; pass it. + Each takes `sketch_name`; without it, the sketch you closed last is + used. `create_revolve` turns about the sketch's centreline, or about + the reference axis `axis_entity` names. 6. Then edge treatments: `select_face_edges`, then `add_fillet` or `add_chamfer` with empty `edge_names`. 7. Finish with `verify_model` (see "Check the result"). diff --git a/src/solidworks_mcp/skills/solidworks-mcp/references/errors-and-limits.md b/src/solidworks_mcp/skills/solidworks-mcp/references/errors-and-limits.md index 0c4bbd54..6ef6f7cd 100644 --- a/src/solidworks_mcp/skills/solidworks-mcp/references/errors-and-limits.md +++ b/src/solidworks_mcp/skills/solidworks-mcp/references/errors-and-limits.md @@ -10,6 +10,7 @@ and a `hint`; some carry `data` with details. Act on `error_type`: | `no_active_document` | Open or create one (`open_model`, `create_part`), or `activate_document`. | | `no_active_sketch` | `create_sketch` first. | | `wrong_document_type` | The active document is the wrong kind (a drawing tool on a part); switch with `activate_document`. | +| `invalid_input` | An argument is wrong, or is one a tool no longer takes; the message says what to send instead. Fix the call. | | `not_found` | A name or path is wrong. List what exists (`list_features`, `list_components`, `list_drawing_views`, `list_open_documents`) and retry with the exact name. | | `feature_not_built` | SolidWorks accepted the call but built nothing: the sketch isn't closed, the cut misses the body, the depth is wrong. Change the input, don't repeat the call. | | `not_supported` | The tool can't run yet. Use the one its description names. | @@ -27,7 +28,9 @@ it will again. - `undo` steps back one or more operations; its reply says whether the feature tree changed. - `suppress_feature` turns off a feature without deleting it; - `delete_feature` removes it and its dependents. + `delete_feature` removes it and its dependents. Ask the user before + deleting: a sketch takes every feature built from it with it, and the + reply's `warning` names what else went. - `get_model_info` and `list_features` show where the model stands now. ## Tools that can't run yet @@ -44,15 +47,4 @@ mostly batch, template, macro-recording and drawing-analysis tools, plus | Fault | Workaround | | --- | --- | -| `create_extrusion` and `create_revolve` ignore `sketch_name` (and the revolve `axis_entity`): they use the sketch closed last. | Call them straight after `exit_sketch` on the right sketch; put the revolve axis in that sketch with `add_centerline`. | -| `create_sketch` echoes a `sketch_name` you pass instead of SolidWorks' name. | Don't pass `sketch_name`; use the name in the reply. | -| `pattern_circular` and `mirror_feature` report an error unless the volume grows, so a pattern or mirror of a cut fails. | Pattern or mirror the sketch entities (`sketch_circular_pattern`, `sketch_linear_pattern`, `sketch_mirror`), then cut once. | -| `create_part` ignores `material`; `create_assembly` ignores `components`; `create_drawing` ignores `model_path` and `sheet_format`. | `set_material` after the first feature; `insert_component` for each part; `create_drawing_view` or `generate_drawing_package` for views. | -| `create_technical_drawing` ignores its template, sheet size, title and output path. | `generate_drawing_package` for a complete drawing. | -| `delete_feature`'s message may not mention the dependents it also removed. | `list_features` afterwards. | -| `save_as` replaces an existing file whatever `overwrite` says. | `save_part` or `save_assembly`, which refuse an existing file unless `overwrite` is on. | -| The `export_*` file tools take the format from the file extension; `format_type` is optional and `options` is ignored. | Name the file with the right extension. | -| `calculate_mass_properties` labels moments of inertia kg·mm², but they are kg·m². | Read them as kg·m². | -| `check_interference` ignores `include_hidden`. | Show the components you want checked. | -| `execute_macro` runs a macro once, whatever `repeat_count` says, and runs only binary `.swp` files. | Call it again for a repeat. | | `add_mate` can't mate edges or vertices, only planes, axes and faces. | Mate the faces that meet at the edge. | diff --git a/src/solidworks_mcp/skills/solidworks-mcp/references/selection-and-units.md b/src/solidworks_mcp/skills/solidworks-mcp/references/selection-and-units.md index ed9c0189..9c0f005a 100644 --- a/src/solidworks_mcp/skills/solidworks-mcp/references/selection-and-units.md +++ b/src/solidworks_mcp/skills/solidworks-mcp/references/selection-and-units.md @@ -20,7 +20,7 @@ angle dimension. `set_units` changes what the document shows, not what the tools take. Mass is in kg, volume in mm³. `calculate_mass_properties` returns moments -of inertia in kg·m², whatever their `units` label says. +of inertia in kg·mm², about the centre of mass. ## Planes and model axes diff --git a/src/solidworks_mcp/tools/analysis.py b/src/solidworks_mcp/tools/analysis.py index 94b85c42..8fa739a0 100644 --- a/src/solidworks_mcp/tools/analysis.py +++ b/src/solidworks_mcp/tools/analysis.py @@ -25,38 +25,21 @@ # Input schemas using Python 3.14 built-in types -class MassPropertiesInput(CompatInput): - """Input schema for mass properties analysis. +#: kg·m² (SolidWorks' SI) to kg·mm². +_KG_M2_TO_KG_MM2 = 1_000_000.0 - Attributes: - include_hidden (bool): The include hidden value. - model_path (str | None): The model path value. - reference_coordinate_system (str | None): The reference coordinate system value. - units (str): The units value. - """ - model_path: str | None = Field(default=None, description="Path to the model file") - units: str = Field(default="metric", description="Units for mass properties") - include_hidden: bool = Field(default=False, description="Include hidden components") - reference_coordinate_system: str | None = Field( - default=None, description="Reference coordinate system alias" - ) - - def model_post_init(self, __context: Any) -> None: - """Provide model post init support for the mass properties input. +def _kg_mm2(value: float | None) -> float | None: + """A moment or product of inertia from kg·m² to kg·mm².""" + return None if value is None else value * _KG_M2_TO_KG_MM2 - Args: - __context (Any): The context value. - Returns: - None: None. +class MassPropertiesInput(CompatInput): + """Input schema for mass properties analysis. - Raises: - ValueError: If the operation cannot be completed. - """ - valid_units = {"metric", "kg", "g", "lb"} - if self.units not in valid_units: - raise ValueError(f"units must be one of {sorted(valid_units)}") + It has no fields: the tool always measures the active model. Inputs it + used to take are refused (``input_compat.REMOVED_INPUTS``). + """ class InterferenceCheckInput(CompatInput): @@ -64,24 +47,22 @@ class InterferenceCheckInput(CompatInput): Attributes: assembly_path (str | None): The assembly path value. - check_all_components (bool): The check all components value. components (list[str]): The components value. include_hidden (bool): The include hidden value. - tolerance (float): The tolerance value. """ assembly_path: str | None = Field(default=None, description="Assembly path alias") - check_all_components: bool = Field( - default=False, description="Check all components alias" + include_hidden: bool = Field( + default=False, + description=( + "Check hidden components too; by default they are left out, as in " + "SolidWorks' own Interference Detection" + ), ) - include_hidden: bool = Field(default=False, description="Include hidden components") components: list[str] = Field( default_factory=list, description="List of component names to check for interference", ) - tolerance: float = Field( - default=0.001, description="Interference detection tolerance in mm" - ) class GeometryAnalysisInput(BaseModel): @@ -340,25 +321,15 @@ async def calculate_mass_properties( Mass depends on the assigned material (`set_material`). Needs: an open part or assembly. Read-only. It rebuilds first. Returns: `mass_properties` with `volume` (mm³), `surface_area` (mm²), `mass` (kg), `center_of_mass` (mm), - `moments_of_inertia` (in kg·m², whatever its `units` label says) and + `moments_of_inertia` (kg·mm², about the centre of mass) and `principal_axes`. Args: - input_data: Nothing is required; the fields (`model_path`, `units`, `include_hidden`, `reference_coordinate_system`) are not used yet: it always measures the active model. + input_data: Nothing: it always measures the active model. Returns: dict[str, Any]: The mass properties. """ - if hasattr(adapter, "calculate_mass_properties"): - result = await adapter.calculate_mass_properties(input_data.model_dump()) - require_success(result, context="Failed to calculate mass properties") - return { - "status": "success", - "message": "Mass properties calculated successfully", - "data": result.data, - "execution_time": result.execution_time, - } - result = await adapter.get_mass_properties() require_success(result, context="Failed to calculate mass properties") @@ -378,12 +349,10 @@ async def calculate_mass_properties( "units": "mm", }, "moments_of_inertia": { - "Ixx": props.moments_of_inertia["Ixx"], - "Iyy": props.moments_of_inertia["Iyy"], - "Izz": props.moments_of_inertia["Izz"], - "Ixy": props.moments_of_inertia["Ixy"], - "Ixz": props.moments_of_inertia["Ixz"], - "Iyz": props.moments_of_inertia["Iyz"], + **{ + key: _kg_mm2(props.moments_of_inertia[key]) + for key in ("Ixx", "Iyy", "Izz", "Ixy", "Ixz", "Iyz") + }, "units": "kg·mm²", }, "principal_axes": props.principal_axes, @@ -401,11 +370,11 @@ async def get_mass_properties( the values you expect, use `verify_model`. Mass depends on the assigned material (`set_material`). Needs: an open part or assembly. Read-only. It rebuilds first. Returns: `mass_properties` with `volume` (mm³), `surface_area` (mm²), `mass` - (kg), `center_of_mass` (mm), `moments_of_inertia` (in kg·m², whatever its - `units` label says) and `principal_axes`. + (kg), `center_of_mass` (mm), `moments_of_inertia` (kg·mm², about the centre + of mass) and `principal_axes`. Args: - input_data: Optional, and not used yet: it always measures the active model. + input_data: Optional, and empty: it always measures the active model. Returns: dict[str, Any]: The mass properties. @@ -424,8 +393,8 @@ async def check_interference(input_data: InterferenceCheckInput) -> dict[str, An Args: input_data: Nothing is required. `components` limits the report to - interferences involving those components; `tolerance`, - `include_hidden` and `check_all_components` are not used. + interferences involving those components; `include_hidden` also + checks hidden components, which are left out by default. Returns: dict[str, Any]: The interferences found. @@ -438,7 +407,9 @@ async def check_interference(input_data: InterferenceCheckInput) -> dict[str, An "directly (Tools > Interference Detection) to check this " "assembly.", ) - result = await adapter.check_interference(input_data.model_dump()) + options = input_data.model_dump() + options["ignore_hidden"] = not input_data.include_hidden + result = await adapter.check_interference(options) require_success(result, "Interference check failed") return { "status": "success", diff --git a/src/solidworks_mcp/tools/drawing.py b/src/solidworks_mcp/tools/drawing.py index 955f2f88..71d5eb55 100644 --- a/src/solidworks_mcp/tools/drawing.py +++ b/src/solidworks_mcp/tools/drawing.py @@ -4,7 +4,7 @@ technical drawings in SolidWorks. """ -from typing import Any +from typing import Any, Literal from fastmcp import FastMCP from pydantic import BaseModel, Field @@ -12,6 +12,7 @@ from ..adapters.base import SolidWorksAdapter from .input_compat import CompatInput from .tool_boundary import ( + InvalidInputError, UnsupportedOperationError, normalize_input, require_capability, @@ -187,30 +188,19 @@ def model_post_init(self, __context: Any) -> None: class DrawingCreationInput(CompatInput): - """Input schema for creating a new drawing. + """Input schema for placing a model's three standard views. Attributes: - auto_populate_views (bool): The auto populate views value. - model_file (str | None): The model file value. - output_path (str | None): The output path value. - scale (str): The scale value. - sheet_format (str | None): The sheet format value. - sheet_size (str): The sheet size value. - template (str | None): The template value. - title (str): The title value. + model_file (str | None): The model to draw. + projection (str): third_angle or first_angle. """ - template: str | None = Field(default=None, description="Drawing template file path") model_file: str | None = Field( - default=None, description="Model file to create drawing from" + default=None, description="Model file (.sldprt or .sldasm) to draw" ) - sheet_size: str = Field(default="A3", description="Sheet size") - title: str = Field(default="", description="Drawing title") - output_path: str | None = Field(default=None, description="Output drawing path") - sheet_format: str | None = Field(default=None, description="Sheet format") - scale: str = Field(default="1:1", description="Drawing scale") - auto_populate_views: bool = Field( - default=False, description="Automatically populate standard views" + projection: Literal["third_angle", "first_angle"] = Field( + default="third_angle", + description="Projection the views are laid out in", ) @@ -232,7 +222,8 @@ class DrawingViewInput(CompatInput): drawing_path: str | None = Field(default=None, description="Path to drawing file") view_name: str | None = Field(default=None, description="Name of the drawing view") operation: str = Field( - default="add", description="Operation to perform (create, update, delete)" + default="add", + description='Only "add" is supported; any other value is refused', ) model_file: str | None = Field(default=None, description="Model file path") view_type: str | None = Field(default=None, description="Type of view") @@ -630,15 +621,14 @@ async def create_technical_drawing( Use it to start a drawing's views quickly; for one view, use `create_drawing_view`, and to create and save a complete drawing from a model - file, use `generate_drawing_package`. Views are laid out in third-angle - projection. The template, sheet size, title and output path fields are not used - yet. Needs: an open, active drawing and the model file on disk. Changes: adds - three views to the sheet. Returns: `data` with `views` (the names added), - `model_path`, `projection` and the view counts. + file, use `generate_drawing_package`. Views are laid out in `projection` + (third-angle by default). Needs: an open, active drawing and the model file on + disk. Changes: adds three views to the sheet. Returns: `data` with `views` (the + names added), `model_path`, `projection` and the view counts. Args: - input_data: The model to draw. Give `model_file`; the other fields have - defaults and are not used yet. + input_data: The model to draw. Give `model_file`; `projection` is + third_angle unless you pass first_angle. Returns: dict[str, Any]: The views added. @@ -663,8 +653,8 @@ async def add_drawing_view(input_data: DrawingViewInput) -> dict[str, Any]: """Place one named view of a part or assembly on the active drawing sheet; the same as `create_drawing_view`. Prefer `create_drawing_view`; this tool reaches the same code and is kept for - existing callers. It only adds views: `operation` values to update or remove a - view are not acted on. Needs: an open, active drawing and the model file on + existing callers. It only adds views: an `operation` other than "add" is + refused. Needs: an open, active drawing and the model file on disk. Changes: adds a view to the sheet. Returns: `data` with the view's `name`, `orientation`, `position`, `scale` and the view counts. @@ -675,6 +665,11 @@ async def add_drawing_view(input_data: DrawingViewInput) -> dict[str, Any]: Returns: dict[str, Any]: The placed view. """ + if input_data.operation.strip().lower() != "add": + raise InvalidInputError( + f"operation '{input_data.operation}' is not supported: " + "add_drawing_view only adds views, and the drawing was not changed." + ) require_capability( adapter, "add_drawing_view", diff --git a/src/solidworks_mcp/tools/engineering_data.py b/src/solidworks_mcp/tools/engineering_data.py index cbe244b1..0e1819fd 100644 --- a/src/solidworks_mcp/tools/engineering_data.py +++ b/src/solidworks_mcp/tools/engineering_data.py @@ -268,7 +268,9 @@ async def get_material( Use it before `calculate_mass_properties` or `verify_model` with a mass: without a material SolidWorks uses a default density. Needs: an open, active part (any other document is `wrong_document_type`). Read-only. Returns: `data` with - `material` (null when none is assigned) and `database`. + `material` (null when none is assigned), `database` and `density_kg_m3` (from + the material's library entry; null when unknown). For the other properties + (modulus, strength), open the material in SolidWorks. Args: input_data: Nothing is required: `configuration` defaults to the active diff --git a/src/solidworks_mcp/tools/export.py b/src/solidworks_mcp/tools/export.py index 722fdaed..9da6a74b 100644 --- a/src/solidworks_mcp/tools/export.py +++ b/src/solidworks_mcp/tools/export.py @@ -31,7 +31,6 @@ class ExportFileInput(CompatInput): Attributes: file_path (str): The file path value. format_type (str): The format type value. - options (dict[str, Any] | None): The options value. """ file_path: str = Field(description="Full path for the exported file") @@ -42,9 +41,6 @@ class ExportFileInput(CompatInput): "tool's own format" ), ) - options: dict[str, Any] | None = Field( - default=None, description="Format-specific export options" - ) class ExportImageInput(CompatInput): @@ -104,206 +100,6 @@ def model_post_init(self, __context: Any) -> None: self.height = int(height) -class ExportSTEPInput(CompatInput): - """Input schema for STEP export. - - Attributes: - file_path (str | None): The file path value. - model_path (str | None): The model path value. - output_path (str | None): The output path value. - precision (float): The precision value. - step_version (str | None): The step version value. - units (str): The units value. - version (str): The version value. - """ - - file_path: str | None = Field( - default=None, description="Output file path for STEP file" - ) - model_path: str | None = Field(default=None, description="Path to the model file") - output_path: str | None = Field(default=None, description="Alternative output path") - precision: float = Field(default=0.01, description="Export precision") - units: str = Field(default="mm", description="Units for export") - version: str = Field(default="AP214", description="STEP version") - step_version: str | None = Field( - default=None, description="Alternative STEP version" - ) - - def model_post_init(self, __context: Any) -> None: - """Provide model post init support for the export stepinput. - - Args: - __context (Any): The context value. - - Returns: - None: None. - """ - if self.file_path is None: - self.file_path = self.output_path - - -class ExportIGESInput(CompatInput): - """Input schema for IGES export. - - Attributes: - file_path (str | None): The file path value. - iges_version (str | None): The iges version value. - model_path (str | None): The model path value. - output_path (str | None): The output path value. - precision (float): The precision value. - surface_type (str): The surface type value. - units (str): The units value. - """ - - file_path: str | None = Field( - default=None, description="Output file path for IGES file" - ) - model_path: str | None = Field(default=None, description="Path to the model file") - output_path: str | None = Field(default=None, description="Alternative output path") - precision: float = Field(default=0.01, description="Export precision") - units: str = Field(default="mm", description="Units for export") - surface_type: str = Field(default="NURBS", description="Surface type") - iges_version: str | None = Field( - default=None, description="Alternative IGES version" - ) - - def model_post_init(self, __context: Any) -> None: - """Provide model post init support for the export igesinput. - - Args: - __context (Any): The context value. - - Returns: - None: None. - """ - if self.file_path is None: - self.file_path = self.output_path - - -class ExportSTLInput(CompatInput): - """Input schema for STL export. - - Attributes: - binary_format (bool): The binary format value. - file_format (str | None): The file format value. - file_path (str | None): The file path value. - format_type (str): The format type value. - model_path (str | None): The model path value. - output_path (str | None): The output path value. - resolution (str): The resolution value. - units (str): The units value. - """ - - file_path: str | None = Field( - default=None, description="Output file path for STL file" - ) - model_path: str | None = Field(default=None, description="Path to the model file") - output_path: str | None = Field(default=None, description="Alternative output path") - resolution: str = Field( - default="fine", description="Mesh resolution (coarse, fine, custom)" - ) - binary_format: bool = Field(default=True, description="Use binary STL format") - format_type: str = Field(default="Binary", description="STL format type") - file_format: str | None = Field( - default=None, description="Alternative STL format field" - ) - units: str = Field(default="mm", description="Units for export") - - def model_post_init(self, __context: Any) -> None: - """Provide model post init support for the export stlinput. - - Args: - __context (Any): The context value. - - Returns: - None: None. - """ - if self.file_path is None: - self.file_path = self.output_path - - -class ExportPDFInput(CompatInput): - """Input schema for PDF export. - - Attributes: - color_mode (str): The color mode value. - drawing_path (str | None): The drawing path value. - file_path (str | None): The file path value. - include_3d (bool): The include 3d value. - output_path (str | None): The output path value. - quality (str): The quality value. - """ - - file_path: str | None = Field( - default=None, description="Output file path for PDF file" - ) - drawing_path: str | None = Field( - default=None, description="Path to the drawing file" - ) - output_path: str | None = Field(default=None, description="Alternative output path") - quality: str = Field(default="high", description="PDF quality (low, medium, high)") - include_3d: bool = Field(default=False, description="Include 3D PDF content") - color_mode: str = Field(default="Color", description="Color mode") - - def model_post_init(self, __context: Any) -> None: - """Provide model post init support for the export pdfinput. - - Args: - __context (Any): The context value. - - Returns: - None: None. - """ - if self.file_path is None: - self.file_path = self.output_path - - -class ExportDWGInput(CompatInput): - """Input schema for DWG export. - - Attributes: - autocad_version (str | None): The autocad version value. - drawing_path (str | None): The drawing path value. - dwg_version (str): The dwg version value. - file_path (str | None): The file path value. - layer_mapping (bool): The layer mapping value. - output_path (str | None): The output path value. - units (str): The units value. - version (str): The version value. - """ - - file_path: str | None = Field( - default=None, description="Output file path for DWG file" - ) - drawing_path: str | None = Field( - default=None, description="Path to the drawing file" - ) - output_path: str | None = Field(default=None, description="Alternative output path") - version: str = Field( - default="2018", description="AutoCAD version (2018, 2014, 2010, etc.)" - ) - autocad_version: str | None = Field( - default=None, description="AutoCAD version alias" - ) - units: str = Field(default="mm", description="Units for export") - dwg_version: str = Field(default="2018", description="DWG version") - layer_mapping: bool = Field(default=False, description="Enable layer mapping") - - def model_post_init(self, __context: Any) -> None: - """Provide model post init support for the export dwginput. - - Args: - __context (Any): The context value. - - Returns: - None: None. - """ - if self.file_path is None: - self.file_path = self.output_path - if self.autocad_version is not None: - self.version = self.autocad_version - - class BatchExportInput(CompatInput): """Input schema for batch export operations. @@ -357,8 +153,7 @@ def _file_input(input_data: Any, format_type: str) -> Any: """Fill ``format_type`` with the tool's own format when it was left out. The file extension decides the format; the adapter is still told which - format the tool exports. The per-format models (``ExportSTEPInput`` and - the like) pass through unchanged. + format the tool exports. """ if not isinstance(input_data, ExportFileInput) or input_data.format_type: return input_data @@ -406,7 +201,7 @@ async def export_step(input_data: ExportFileInput) -> dict[str, Any]: Args: input_data: The export. `file_path` (ending in .step or .stp) is required; `format_type` is not needed: the file extension decides the - format, and `options` is not used yet. + format. Returns: dict[str, Any]: The file written. @@ -451,7 +246,7 @@ async def export_iges(input_data: ExportFileInput) -> dict[str, Any]: Args: input_data: The export. `file_path` (ending in .igs or .iges) is required; `format_type` is not needed: the file extension decides the - format, and `options` is not used yet. + format. Returns: dict[str, Any]: The file written. @@ -495,7 +290,7 @@ async def export_stl(input_data: ExportFileInput) -> dict[str, Any]: Args: input_data: The export. `file_path` (ending in .stl) is required; `format_type` is not needed: the file extension decides the - format, and `options` is not used yet. + format. Returns: dict[str, Any]: The file written. @@ -540,7 +335,7 @@ async def export_pdf(input_data: ExportFileInput) -> dict[str, Any]: Args: input_data: The export. `file_path` (ending in .pdf) is required; `format_type` is not needed: the file extension decides the - format, and `options` is not used yet. + format. Returns: dict[str, Any]: The file written. @@ -584,7 +379,7 @@ async def export_dwg(input_data: ExportFileInput) -> dict[str, Any]: Args: input_data: The export. `file_path` (ending in .dwg) is required; `format_type` is not needed: the file extension decides the - format, and `options` is not used yet. + format. Returns: dict[str, Any]: The file written. diff --git a/src/solidworks_mcp/tools/file_management.py b/src/solidworks_mcp/tools/file_management.py index e641ca78..f8f6d5e4 100644 --- a/src/solidworks_mcp/tools/file_management.py +++ b/src/solidworks_mcp/tools/file_management.py @@ -35,10 +35,8 @@ class SaveFileInput(CompatInput): Attributes: file_path (str | None): The file path value. - force_save (bool): The force save value. """ - force_save: bool = Field(default=True, description="Force save even if no changes") file_path: str | None = Field( default=None, description="Optional output path. If omitted, saves the current active document.", @@ -584,7 +582,7 @@ async def save_file(input_data: SaveFileInput) -> dict[str, Any]: Args: input_data: The save. Nothing is required: give `file_path` to save to a - new path, or omit it to save in place; `force_save` is not used. + new path, or omit it to save in place. Returns: dict[str, Any]: The save result. @@ -611,18 +609,23 @@ async def save_as(input_data: SaveAsInput) -> dict[str, Any]: For a SolidWorks format (solidworks, sldprt, sldasm, slddrw) it saves a copy like File > Save As; any other `format_type` is passed to the exporter. For one known format with its options, prefer `export_step`, `export_stl` and the other - `export_*` tools. Needs: an open, active document. Changes: writes the file, - replacing one already at `file_path` (`overwrite` is not checked). Returns: - `file_path` and `format`. + `export_*` tools. Needs: an open, active document. Changes: writes the file. A + file already at `file_path` is kept and the call refused, unless `overwrite` + is on. Returns: `file_path` and `format`. Args: input_data: The target. `file_path` is required; `format_type` defaults - to "solidworks", and `overwrite` is not used yet. + to "solidworks", and `overwrite` is off. Returns: dict[str, Any]: The path written. """ input_data = _coerce_input(SaveAsInput, input_data) + if Path(input_data.file_path).exists() and not input_data.overwrite: + raise InvalidInputError( + f"A file already exists at {input_data.file_path}; it was left " + "untouched. Pass overwrite: true to replace it, or choose another path." + ) if hasattr(adapter, "save_file") and input_data.format_type.lower() in { "solidworks", "sldprt", diff --git a/src/solidworks_mcp/tools/input_compat.py b/src/solidworks_mcp/tools/input_compat.py index 11758975..03b29a7f 100644 --- a/src/solidworks_mcp/tools/input_compat.py +++ b/src/solidworks_mcp/tools/input_compat.py @@ -15,3 +15,72 @@ class CompatInput(BaseModel): """ model_config = ConfigDict(extra="allow", populate_by_name=True) + + +_DRAWING_PACKAGE = "use `generate_drawing_package` for a complete drawing" +_FILE_EXTENSION = "the file extension sets the format and its options" +_ACTIVE_MODEL = "it measures the active model; open and activate the one you want" + +#: Inputs a tool used to advertise but never applied, by input model name. +#: Each maps to what to use instead. A call that still sends one is refused +#: (``removed_inputs``) rather than silently ignored. +REMOVED_INPUTS: dict[str, dict[str, str]] = { + "CreateExtrusionInput": {"direction": "use `end_condition`"}, + "CreateRevolveInput": {"direction": "use `both_directions`"}, + "CreateAssemblyInput": { + "components": "call `insert_component` for each part", + }, + "CreateDrawingInput": { + "model_path": "place views with `create_drawing_view`, or " + _DRAWING_PACKAGE, + "sheet_format": _DRAWING_PACKAGE, + }, + "DrawingCreationInput": { + "template": _DRAWING_PACKAGE, + "sheet_size": _DRAWING_PACKAGE, + "sheet_format": _DRAWING_PACKAGE, + "title": _DRAWING_PACKAGE, + "output_path": "save the drawing with `save_file`", + "scale": "set it afterwards with `set_sheet_scale`", + "auto_populate_views": "the tool always places the three standard views", + }, + "SaveFileInput": {"force_save": "the tool always saves"}, + "ExportFileInput": {"options": _FILE_EXTENSION}, + "InterferenceCheckInput": { + "tolerance": "the check has no tolerance", + "check_all_components": "it checks the whole assembly, or `components`", + }, + "MacroPlaybackInput": { + "parameters": "the macro takes no arguments; have it read a file you write", + "execution_parameters": "the macro takes no arguments; have it read a file", + "target_file": "open the document first with `open_model`", + "execution_mode": "the macro runs in the running SolidWorks session", + "pause_on_error": "it stops at the first run that fails", + "log_execution": "the reply reports every run", + }, + "MassPropertiesInput": { + "model_path": _ACTIVE_MODEL, + "units": "volumes are mm³, masses kg and inertia kg·mm²", + "include_hidden": _ACTIVE_MODEL, + "reference_coordinate_system": "it reports about the centre of mass", + }, +} + + +def removed_inputs(model: BaseModel) -> list[str]: + """The removed inputs ``model`` was given, each with what to use instead. + + Args: + model: A validated tool input. + + Returns: + list[str]: One sentence per removed field present, e.g. + "`components` is no longer an input: call `insert_component` for each + part". Empty when none was sent. + """ + removed = REMOVED_INPUTS.get(type(model).__name__, {}) + extras = model.model_extra or {} + return [ + f"`{field}` is no longer an input: {pointer}" + for field, pointer in removed.items() + if field in extras + ] diff --git a/src/solidworks_mcp/tools/macro_recording.py b/src/solidworks_mcp/tools/macro_recording.py index 0cd74895..14bbcb7e 100644 --- a/src/solidworks_mcp/tools/macro_recording.py +++ b/src/solidworks_mcp/tools/macro_recording.py @@ -4,6 +4,7 @@ and workflow optimization. """ +import asyncio from pathlib import Path from typing import Any @@ -13,6 +14,7 @@ from ..adapters.base import SolidWorksAdapter from .input_compat import CompatInput from .tool_boundary import ( + ToolOperationError, UnsupportedOperationError, normalize_input, require_capability, @@ -84,16 +86,11 @@ class MacroPlaybackInput(CompatInput): """Input schema for macro playback. Attributes: - execution_mode (str | None): The execution mode value. - execution_parameters (dict[str, Any] | None): The execution parameters value. - log_execution (bool): The log execution value. macro_file (str | None): The macro file value. macro_path (str | None): The macro path value. - parameters (dict[str, Any]): The parameters value. + proc_name (str): The procedure to call. pause_between_runs (float): The pause between runs value. - pause_on_error (bool): The pause on error value. repeat_count (int): The repeat count value. - target_file (str | None): The target file value. """ macro_path: str | None = Field( @@ -106,19 +103,17 @@ class MacroPlaybackInput(CompatInput): macro_file: str | None = Field( default=None, description="Alternative to macro_path (.swp or .swb)" ) - parameters: dict[str, Any] = Field( - default_factory=dict, description="Macro parameters" + proc_name: str = Field( + default="main", description="The macro procedure to call; main by default" ) - target_file: str | None = Field(default=None, description="Target file") - execution_mode: str | None = Field(default=None, description="Execution mode") - pause_on_error: bool = Field(default=False, description="Pause on error") - log_execution: bool = Field(default=False, description="Log execution") - execution_parameters: dict[str, Any] | None = Field( - default=None, description="Execution parameters" + repeat_count: int = Field( + default=1, + ge=1, + le=100, + description="How many times to run the macro (1-100); it stops at the first failure", ) - repeat_count: int = Field(default=1, description="Number of times to execute") pause_between_runs: float = Field( - default=0.0, description="Pause between executions in seconds" + default=0.0, ge=0.0, description="Seconds to wait between runs" ) def model_post_init(self, __context: Any) -> None: @@ -286,15 +281,16 @@ async def execute_macro(input_data: MacroPlaybackInput) -> dict[str, Any]: Use it to run an existing macro file. VBA source (.bas, .vb, plain text, or what the `generate_vba_*` tools return) is refused as invalid input before SolidWorks sees it. The macro must not wait for the user (no MsgBox or forms), or the call - hangs. `repeat_count` is not used yet: the macro runs once, although the message - counts the repeats. Needs: SolidWorks running and the macro file on disk. - Changes: whatever the macro does; it can't be undone from here. Returns: `data` - with `macro_path`, `module_name`, `proc_name` and `errors`. + hangs. It runs `repeat_count` times, `pause_between_runs` seconds apart, and + stops at the first run that fails. Needs: SolidWorks running and the macro file + on disk. Changes: whatever the macro does; it can't be undone from here. + Returns: `runs_completed` and `data` from the last run, with `macro_path`, + `module_name`, `proc_name` and `errors`. Args: input_data: The macro to run. Give `macro_path` (or `macro_file`); - `target_file`, `execution_mode` and the logging switches have - defaults. + `proc_name` (main), `repeat_count` (1) and `pause_between_runs` (0 s) + have defaults. Returns: dict[str, Any]: The run's result. @@ -307,13 +303,29 @@ async def execute_macro(input_data: MacroPlaybackInput) -> dict[str, Any]: f"{input_data.macro_path} was not run, and no features " "were created.", ) - result = await adapter.execute_macro(input_data.model_dump()) - require_success(result, "Failed to execute macro") + payload = input_data.model_dump() + total = input_data.repeat_count + execution_time = 0.0 + result = None + for run in range(total): + if run and input_data.pause_between_runs: + await asyncio.sleep(input_data.pause_between_runs) + result = await adapter.execute_macro(payload) + if not result.is_success: + raise ToolOperationError( + f"Run {run + 1} of {total} failed after {run} completed: " + f"{result.error or 'the macro reported an error'}", + execution_time=execution_time + (result.execution_time or 0.0), + data=result.data, + ) + execution_time += result.execution_time or 0.0 + runs = "1 run" if total == 1 else f"{total} runs" return { "status": "success", - "message": f"Macro executed {input_data.repeat_count} times successfully", - "data": result.data, - "execution_time": result.execution_time, + "message": f"Macro completed {runs}", + "runs_completed": total, + "data": result.data if result is not None else None, + "execution_time": execution_time, } @mcp.tool() diff --git a/src/solidworks_mcp/tools/modeling.py b/src/solidworks_mcp/tools/modeling.py index 2b738bce..85fdf6cb 100644 --- a/src/solidworks_mcp/tools/modeling.py +++ b/src/solidworks_mcp/tools/modeling.py @@ -154,7 +154,13 @@ class CreatePartInput(CompatInput): units (str | None): The units value. """ - name: str = Field(description="Name for the new part") + name: str = Field( + description=( + "A label for your own reference; SolidWorks names the new part " + "itself (Part1, ...) and the file takes the name of the path " + "you save it to with save_part" + ) + ) template: str | None = Field( default=None, description=( @@ -186,21 +192,24 @@ class CreateExtrusionInput(CompatInput): Attributes: both_directions (bool): The both directions value. depth (float): The depth value. - direction (str): The direction value. draft_angle (float): The draft angle value. end_condition (str): The end condition value. merge_result (bool): The merge result value. reverse (bool | None): The reverse value. reverse_direction (bool): The reverse direction value. - sketch_name (str): The sketch name value. + sketch_name (str | None): The sketch to extrude; None for the last closed. thin_feature (bool): The thin feature value. thin_thickness (float | None): The thin thickness value. thin_side (str): The side a thin wall grows on. """ - sketch_name: str = Field(description="Sketch name to extrude") + sketch_name: str | None = Field( + default=None, + description=( + 'Sketch to use, by its name in the feature tree (e.g. "Sketch2"); omit to use the sketch `exit_sketch` closed last' + ), + ) depth: float = Field(description="Extrusion depth in millimeters") - direction: str = Field(default="blind", description="Extrusion direction") reverse: bool | None = Field(default=None, description="Reverse direction alias") draft_angle: float = Field(default=0.0, description="Draft angle in degrees") reverse_direction: bool = Field( @@ -243,12 +252,10 @@ def model_post_init(self, __context: Any) -> None: None: None. Raises: - ValueError: Sketch_name is required. + ValueError: A non-positive depth, or a thin wall without thickness. """ if self.depth <= 0: raise ValueError("depth must be positive") - if not self.sketch_name.strip(): - raise ValueError("sketch_name is required") if self.reverse is not None: self.reverse_direction = self.reverse if self.thin_feature and not (self.thin_thickness and self.thin_thickness > 0): @@ -260,20 +267,30 @@ class CreateRevolveInput(CompatInput): Attributes: angle (float): The angle value. - axis_entity (str): The axis entity value. + axis_entity (str | None): A reference axis to revolve about. both_directions (bool): The both directions value. - direction (str): The direction value. merge_result (bool): The merge result value. reverse_direction (bool): The reverse direction value. - sketch_name (str): The sketch name value. + sketch_name (str | None): The sketch to revolve; None for the last closed. thin_feature (bool): The thin feature value. thin_thickness (float | None): The thin thickness value. """ - sketch_name: str = Field(description="Sketch name to revolve") - axis_entity: str = Field(description="Axis entity for the revolve") + sketch_name: str | None = Field( + default=None, + description=( + 'Sketch to use, by its name in the feature tree (e.g. "Sketch2"); omit to use the sketch `exit_sketch` closed last' + ), + ) + axis_entity: str | None = Field( + default=None, + description=( + "A reference axis to revolve about, by its name in the feature tree " + '(e.g. "Axis1" from `create_axis`); omit to use the centreline ' + "drawn in the sketch" + ), + ) angle: float = Field(description="Revolve angle in degrees") - direction: str = Field(default="one_direction", description="Revolve direction") reverse_direction: bool = Field( default=False, description="Reverse revolve direction" ) @@ -579,19 +596,23 @@ class CreateAssemblyInput(CompatInput): """Input schema for creating a new assembly. Attributes: - components (list[str]): The components value. name (str): The name value. template (str | None): The template value. """ - name: str = Field(description="Name for the new assembly") + name: str = Field( + description=( + "A label for your own reference; SolidWorks names the new assembly " + "itself (Assem1, ...) and the file takes the name of the path " + "you save it to with save_assembly" + ) + ) template: str | None = Field( default=None, description=( "Assembly template (.asmdot) path; omit to use the SolidWorks default" ), ) - components: list[str] = Field(default_factory=list, description="Component list") class InsertComponentInput(CompatInput): @@ -1057,21 +1078,23 @@ class CreateDrawingInput(CompatInput): """Input schema for creating a new drawing. Attributes: - model_path (str | None): The model path value. name (str): The name value. - sheet_format (str | None): The sheet format value. template (str | None): The template value. """ - name: str = Field(description="Name for the new drawing") + name: str = Field( + description=( + "A label for your own reference; SolidWorks names the new drawing " + "itself (Draw1, ...) and the file takes the name of the path " + "you save it to with save_file" + ) + ) template: str | None = Field( default=None, description=( "Drawing template (.drwdot) path; omit to use the SolidWorks default" ), ) - model_path: str | None = Field(default=None, description="Source model path") - sheet_format: str | None = Field(default=None, description="Sheet format template") async def register_modeling_tools( @@ -1144,15 +1167,16 @@ async def create_part(input_data: CreatePartInput) -> dict[str, Any]: """Create a new, empty part document and make it active, ready for `create_sketch`. Uses `template` (a .prtdot) or the default part template, then sets `units` if - given. The part is not saved until `save_part`. Needs: SolidWorks running. - Changes: opens a new part document. Returns: `part` with `name`, `units` and - `template`. `material` is not applied; use `set_material` after the first - feature. + given, and assigns `material` if given. The part is not saved until + `save_part`. Needs: SolidWorks running. Changes: opens a new part document. + Returns: `part` with `name`, `units`, `template` and `material` (the one + assigned, or null), plus a `warning` when the material couldn't be assigned; + the part is created either way. Args: - input_data: The part. `name` is required (used in the reply; SolidWorks - names the document); `template` and `units` (mm, cm, m, in, ft) have - defaults, and `material` is not used. + input_data: The part. `name` is required but only a label: the reply + gives SolidWorks' own name (Part1, ...); `template`, `units` (mm, cm, m, in, ft) and + `material` (a name from `list_available_materials`) are optional. Returns: dict[str, Any]: The new part. @@ -1166,17 +1190,33 @@ async def create_part(input_data: CreatePartInput) -> dict[str, Any]: model = result.data part_name = _result_value(model, "name", default=input_data.name) units = _result_value(model, "units", default=input_data.units or "mm") - return { + material = None + warning = None + if input_data.material: + assigned = await adapter.set_material(input_data.material) + if assigned.is_success: + material = input_data.material + else: + warning = ( + f"The part was created without a material: " + f"'{input_data.material}' could not be assigned " + f"({assigned.error}). Use list_available_materials, then " + "set_material." + ) + reply: dict[str, Any] = { "status": "success", "message": f"Created new part: {part_name}", "part": { "name": part_name, "units": units, - "material": input_data.material, + "material": material, "template": input_data.template, }, "execution_time": result.execution_time, } + if warning: + reply["warning"] = warning + return reply @mcp.tool() async def create_assembly(input_data: CreateAssemblyInput) -> dict[str, Any]: @@ -1184,12 +1224,13 @@ async def create_assembly(input_data: CreateAssemblyInput) -> dict[str, Any]: Uses `template` (an .asmdot) or the default assembly template. It is not saved until `save_assembly`. Needs: SolidWorks running. Changes: opens a new assembly - document. Returns: `assembly` with `name` and `template`. `components` are not - inserted; call `insert_component` for each. + document. Returns: `assembly` with `name` and `template`. Then call + `insert_component` for each part. Args: - input_data: The assembly. `name` is required; `template` has a default, - and `components` is not used. + input_data: The assembly. `name` is required but only a label: the + reply gives SolidWorks' own name (Assem1, ...); `template` has a + default. Returns: dict[str, Any]: The new assembly. @@ -1205,7 +1246,6 @@ async def create_assembly(input_data: CreateAssemblyInput) -> dict[str, Any]: "message": f"Created new assembly: {assembly_name}", "assembly": { "name": assembly_name, - "components": input_data.components, "template": input_data.template, }, "execution_time": result.execution_time, @@ -1221,8 +1261,9 @@ async def create_drawing(input_data: CreateDrawingInput) -> dict[str, Any]: new drawing document. Returns: `drawing` with `name` and `template`. Args: - input_data: The drawing. `name` is required; `template` has a default, - and `model_path` and `sheet_format` are not used. + input_data: The drawing. `name` is required but only a label: the + reply gives SolidWorks' own name (Draw1, ...); `template` has a + default. Returns: dict[str, Any]: The new drawing. @@ -1233,16 +1274,11 @@ async def create_drawing(input_data: CreateDrawingInput) -> dict[str, Any]: require_success(result, context="Failed to create drawing") model = result.data drawing_name = _result_value(model, "name", default=input_data.name) - sheet_format = _result_value( - model, "sheet_format", default=input_data.sheet_format - ) return { "status": "success", "message": f"Created new drawing: {drawing_name}", "drawing": { "name": drawing_name, - "model_path": input_data.model_path, - "sheet_format": sheet_format, "template": input_data.template, }, "execution_time": result.execution_time, @@ -1278,18 +1314,20 @@ async def create_extrusion(input_data: CreateExtrusionInput) -> dict[str, Any]: """Extrude the sketch you just closed into a solid boss, added to the active part. Use it after `exit_sketch`; to remove material, use `create_cut_extrude`. It - extrudes the sketch `exit_sketch` closed, whatever `sketch_name` says. End + extrudes the sketch `sketch_name` names, or the one `exit_sketch` closed last; + a name that isn't in the feature tree fails before anything is built. End conditions: Blind (default), MidPlane, ThroughAll, UpToNext; `both_directions` means MidPlane. With `thin_feature` and `thin_thickness` it makes a wall from the sketch's outline, and an open chain works (a bent plate from its centreline). - Needs: an open part and a profile just exited (closed, unless thin). Changes: - adds a Boss-Extrude feature. Returns: `extrusion` (`name`, `depth`, the - `end_condition` SolidWorks used, `both_directions`, and `thin` when thin) and + Needs: an open part and a closed profile (open is fine when thin). Changes: + adds a Boss-Extrude feature. Returns: `extrusion` (`name`, `sketch` when one + was named, `depth`, the `end_condition` SolidWorks used, `both_directions`, + and `thin` when thin) and `measured_change` (volume, bounding box and solid-body count before and after; left out when the model can't be read). Args: - input_data: The extrusion. `sketch_name` and `depth` (mm) are required; + input_data: The extrusion. `depth` (mm) is required; `sketch_name`, `end_condition`, `both_directions`, `reverse_direction`, `draft_angle`, `thin_feature`/`thin_thickness` and `merge_result` have defaults. @@ -1311,6 +1349,7 @@ async def create_extrusion(input_data: CreateExtrusionInput) -> dict[str, Any]: merge_result=input_data.merge_result, feature_scope=False, auto_select=True, + sketch_name=input_data.sketch_name, ) before = await _read_summary(adapter) @@ -1323,7 +1362,7 @@ async def create_extrusion(input_data: CreateExtrusionInput) -> dict[str, Any]: applied = _result_value(feature, "parameters", default=None) or {} extrusion: dict[str, Any] = { "name": name, - "sketch": input_data.sketch_name, + "sketch": applied.get("sketch_name"), "depth": input_data.depth, "end_condition": applied.get("end_condition") or ("MidPlane" if input_data.both_directions else input_data.end_condition), @@ -1344,18 +1383,21 @@ async def create_extrusion(input_data: CreateExtrusionInput) -> dict[str, Any]: @mcp.tool() async def create_revolve(input_data: CreateRevolveInput) -> dict[str, Any]: - """Revolve the sketch you just closed around the centreline drawn in it, adding a solid to the active part. - - Use it for turned shapes (shafts, knobs). Draw the axis with `add_centerline` in - the same sketch; `sketch_name` and `axis_entity` are not used: the sketch - `exit_sketch` closed and its centreline are. Needs: an open part and a closed - profile with a centreline. Changes: adds a Revolve feature. Returns: `revolve` - (`name`, `angle`) and `measured_change` (volume, bounding box and solid-body - count before and after; left out when the model can't be read). + """Revolve a closed sketch around an axis, adding a solid to the active part. + + Use it for turned shapes (shafts, knobs). It revolves the sketch `sketch_name` + names, or the one `exit_sketch` closed last, about the reference axis + `axis_entity` names (from `create_axis`), or about the centreline drawn in the + sketch with `add_centerline`. A sketch or axis that isn't in the model fails + before anything is built. Needs: an open part and a closed profile with a + centreline or a reference axis. Changes: adds a Revolve feature. Returns: + `revolve` (`name`, `sketch` and `axis_entity` when named, `angle`) and + `measured_change` (volume, bounding box and solid-body count before and + after; left out when the model can't be read). Args: - input_data: The revolve. `sketch_name`, `axis_entity` and `angle` - (degrees, 360 for a full turn) are required; `both_directions`, + input_data: The revolve. `angle` (degrees, 360 for a full turn) is + required; `sketch_name`, `axis_entity`, `both_directions`, `reverse_direction`, `thin_feature` and `merge_result` have defaults. Returns: @@ -1370,6 +1412,8 @@ async def create_revolve(input_data: CreateRevolveInput) -> dict[str, Any]: thin_feature=input_data.thin_feature, thin_thickness=input_data.thin_thickness, merge_result=input_data.merge_result, + sketch_name=input_data.sketch_name, + axis_entity=input_data.axis_entity, ) before = await _read_summary(adapter) @@ -1378,17 +1422,16 @@ async def create_revolve(input_data: CreateRevolveInput) -> dict[str, Any]: require_success(result, context="Failed to create revolve") after = await _read_summary(adapter) if before is not None else None feature = result.data + name = _result_value(feature, "feature_name", "name", default="Revolve") + applied = _result_value(feature, "parameters", default=None) or {} reply = { "status": "success", - "message": f"Created revolve: {_result_value(feature, 'feature_name', 'name', default='Revolve')}", + "message": f"Created revolve: {name}", "revolve": { - "name": _result_value( - feature, "feature_name", "name", default="Revolve" - ), - "sketch": input_data.sketch_name, - "axis_entity": input_data.axis_entity, + "name": name, + "sketch": applied.get("sketch_name"), + "axis_entity": applied.get("axis_entity"), "angle": input_data.angle, - "direction": input_data.direction, }, "execution_time": result.execution_time, } @@ -1858,10 +1901,15 @@ async def list_components() -> dict[str, Any]: async def delete_feature(input_data: DeleteFeatureInput) -> dict[str, Any]: """Delete a feature or sketch from the active model, along with the features that depend on it. - Use `suppress_feature` instead when you might want it back. Find names with - `list_features`. Needs: an open part or assembly. Changes: removes the feature - and its dependents (`undo` can restore them). Returns: `data` with `deleted`, - `features_before`, `features_after` and `features_removed`. + Ask the user before you call it: deleting a sketch also deletes every feature + built from it (a cut, a boss), and deleting a feature deletes everything that + depends on it. Use `suppress_feature` instead when you might want it back. Find + names and dependents with `list_features`. Needs: an open part or assembly. + Changes: removes the feature and its dependents (`undo` can restore them). + Returns: `data` with `deleted`, `removed_features` (every feature that went, + dependents included), `features_before` and `features_after`; the message names + the dependents, and a `warning` says so whenever more than the named feature + went: tell the user. Args: input_data: The feature. `name` (e.g. "Fillet1") is required. @@ -1874,19 +1922,26 @@ async def delete_feature(input_data: DeleteFeatureInput) -> dict[str, Any]: require_success(result, context="Failed to delete feature") data = result.data if isinstance(result.data, dict) else {} removed = data.get("removed_features") or [input_data.name] - return { + dependents = [n for n in removed if n != input_data.name.split("@", 1)[0]] + reply: dict[str, Any] = { "status": "success", "message": ( f"Deleted {input_data.name}" + ( - f" and {len(removed) - 1} dependent feature(s)" - if len(removed) > 1 + f" and its dependents: {', '.join(dependents)}" + if dependents else "" ) ), "data": data, "execution_time": result.execution_time, } + if dependents: + reply["warning"] = ( + f"Deleting {input_data.name} also deleted {', '.join(dependents)}. " + "Tell the user; `undo` restores them." + ) + return reply @mcp.tool() async def suppress_feature(input_data: SuppressFeatureInput) -> dict[str, Any]: @@ -2213,10 +2268,12 @@ async def mirror_feature(input_data: MirrorFeatureInput) -> dict[str, Any]: """Mirror solid bodies or features of the active part about a plane. Use it for symmetric parts: model one half, then mirror. Body mirroring (the - default) is what works for lofts and sweeps. The volume must grow, so mirroring - a cut is reported as an error today. Needs: an open part with the named bodies - or features. Changes: adds a Mirror feature. Returns: `mirror` with `name`, - `mirrored`, `mirror_plane`, `merge` and `mirror_bodies`. + default) is what works for lofts and sweeps; to mirror a cut, name the cut and + set `mirror_bodies` off. It fails when the volume doesn't change, which means + nothing was mirrored. Needs: an open part with the named bodies or features. + Changes: adds a Mirror feature. Returns: `mirror` with `name`, `mirrored`, + `mirror_plane`, `merge`, `mirror_bodies`, and `volume_before`, + `volume_after` and `volume_change` (mm³, negative for a cut). Args: input_data: The mirror. `features` (at least one name) and @@ -2247,12 +2304,13 @@ async def mirror_feature(input_data: MirrorFeatureInput) -> dict[str, Any]: async def pattern_circular(input_data: PatternCircularInput) -> dict[str, Any]: """Repeat features of the active part around an axis, e.g. a ring of bosses or ribs. - Create the axis first with `create_axis`. The volume must grow, so patterning a - cut (a ring of holes) is reported as an error today; draw the holes in one - sketch with `sketch_circular_pattern` and cut once instead. Needs: an open part - with the named features and axis. Changes: adds a circular pattern feature. - Returns: `pattern` with `name`, `patterned`, `axis`, `count`, `angle` and - `equal_spacing`. + Create the axis first with `create_axis`. Bosses and cuts both work (a ring of + holes is a patterned cut). It fails when the volume doesn't change, which + means no instance landed on the part. Needs: an open part with the named + features and axis. Changes: adds a circular pattern feature. Returns: + `pattern` with `name`, `patterned`, `axis`, `count`, `angle`, + `equal_spacing`, and `volume_before`, `volume_after` and `volume_change` + (mm³, negative for cuts). Args: input_data: The pattern. `features` (at least one), `axis` and `count` diff --git a/src/solidworks_mcp/tools/sketching.py b/src/solidworks_mcp/tools/sketching.py index 340c899c..7b8a8e0e 100644 --- a/src/solidworks_mcp/tools/sketching.py +++ b/src/solidworks_mcp/tools/sketching.py @@ -34,7 +34,14 @@ class CreateSketchInput(CompatInput): plane: str = Field( description="Sketch plane name (e.g., 'Top', 'Front', 'Right', 'XY', 'XZ', 'YZ')" ) - sketch_name: str | None = Field(default=None, description="Sketch name alias") + sketch_name: str | None = Field( + default=None, + description=( + "Name to give the sketch in the feature tree, applied when " + "exit_sketch closes it; omit to keep SolidWorks' own (Sketch1, " + "Sketch2, ...)" + ), + ) def model_post_init(self, __context: Any) -> None: """Provide model post init support for the create sketch input. @@ -336,6 +343,11 @@ async def register_sketching_tools( """ mcp = with_tool_boundary(mcp) tool_count = 0 + # SolidWorks won't rename a sketch while it is open for editing (it + # accepts the name, but the tree keeps the old one), so the name + # create_sketch was asked for is given when exit_sketch closes it: + # SolidWorks' name -> the name asked for. + names_on_exit: dict[str, str] = {} @mcp.tool() async def create_sketch(input_data: CreateSketchInput) -> dict[str, Any]: @@ -345,12 +357,13 @@ async def create_sketch(input_data: CreateSketchInput) -> dict[str, Any]: `create_extrusion` or `create_cut_extrude`. Planes: Front, Top, Right (or XY, XZ, YZ), or a plane made by `create_reference_plane`. Coordinates in the sketch are mm from the origin. Needs: an open part. Changes: adds a sketch and opens it - for editing. Returns: `sketch` with `name` and `plane`; `name` echoes - `sketch_name` when you pass one, as SolidWorks names the sketch itself. + for editing; give `sketch_name` and `exit_sketch` names it that when it closes + it (SolidWorks won't rename an open sketch). Returns: `sketch` with `name` (the + name in the feature tree now) and `plane`, plus `name_on_exit` when you gave + `sketch_name`. Args: - input_data: The sketch. `plane` is required; `sketch_name` is not - applied to SolidWorks. + input_data: The sketch. `plane` is required; `sketch_name` names it. Returns: dict[str, Any]: The new sketch. @@ -359,12 +372,12 @@ async def create_sketch(input_data: CreateSketchInput) -> dict[str, Any]: result = await adapter.create_sketch(input_data.plane) require_success(result, context="Failed to create sketch") - sketch_name = input_data.sketch_name or ( + sketch_name = ( result.data.get("sketch_name") if isinstance(result.data, dict) else result.data ) - return { + reply: dict[str, Any] = { "status": "success", "message": f"Created sketch '{sketch_name}' on {input_data.plane} plane", "sketch": { @@ -373,6 +386,13 @@ async def create_sketch(input_data: CreateSketchInput) -> dict[str, Any]: }, "execution_time": result.execution_time, } + names_on_exit.clear() # a sketch left open is abandoned with its name + wanted = (input_data.sketch_name or "").strip() + if wanted and wanted != sketch_name: + names_on_exit[str(sketch_name)] = wanted + reply["sketch"]["name_on_exit"] = wanted + reply["message"] += f"; exit_sketch names it '{wanted}'" + return reply @mcp.tool() async def add_line(input_data: AddLineInput) -> dict[str, Any]: @@ -542,17 +562,33 @@ async def exit_sketch() -> dict[str, Any]: Call it after drawing and before `create_extrusion`, `create_revolve` or `create_cut_extrude`; those use the sketch closed last. Needs: an open sketch. - Changes: leaves sketch editing; the sketch stays in the feature tree. Returns: a - message only. + Changes: leaves sketch editing; the sketch stays in the feature tree, renamed + to the `sketch_name` `create_sketch` was given. Returns: a message, plus, after + a `sketch_name`, `sketch` with `name` (the name in the feature tree, which later + tools take as `sketch_name`) and a `warning` when the name couldn't be given + (another feature has it). """ result = await adapter.exit_sketch() require_success(result, context="Failed to exit sketch") - return { + reply: dict[str, Any] = { "status": "success", "message": "Exited sketch editing mode", "execution_time": result.execution_time, } + if names_on_exit: + current, wanted = names_on_exit.popitem() + renamed = await adapter.rename_feature(current, wanted) + name = current + if renamed.is_success: + name = (renamed.data or {}).get("new_name") or wanted + else: + reply["warning"] = ( + f"The sketch keeps SolidWorks' name '{current}': it could " + f"not be named '{wanted}' ({renamed.error})." + ) + reply["sketch"] = {"name": name} + return reply @mcp.tool() async def check_sketch_fully_defined( @@ -1023,17 +1059,21 @@ async def sketch_tutorial_simple_hole() -> dict[str, Any]: """Demonstrate the sketch steps: draw a 5 mm circle at the origin on the Top plane and close the sketch. It is a fixed example; for a real hole, use `create_sketch`, `add_circle`, - `exit_sketch` and `create_cut_extrude`. Its `next_step` text suggests a - negative-depth extrusion: use `create_cut_extrude` instead. Needs: an open part. - Changes: adds a sketch with one circle. Returns: `steps_completed`, `next_step` - and `suggested_extrusion`. + `exit_sketch` and `create_cut_extrude`. Needs: an open part. Changes: adds a + sketch with one circle. Returns: `steps_completed`, `next_step` (cut the hole + with `create_cut_extrude`) and `suggested_cut`, that call's input. """ steps_completed = [] # Step 1: Create sketch sketch_result = await adapter.create_sketch("Top") require_success(sketch_result, context="Failed to create sketch") - steps_completed.append(f"Created sketch: {sketch_result.data}") + sketch_name = ( + sketch_result.data.get("sketch_name") + if isinstance(sketch_result.data, dict) + else sketch_result.data + ) + steps_completed.append(f"Created sketch: {sketch_name}") # Step 2: Add circle circle_result = await adapter.add_circle(0, 0, 2.5) @@ -1049,10 +1089,14 @@ async def sketch_tutorial_simple_hole() -> dict[str, Any]: "status": "success", "message": "Tutorial completed: Simple hole sketch created", "steps_completed": steps_completed, - "next_step": "Use create_extrusion with negative depth to create the hole", - "suggested_extrusion": { - "depth": -10, # Negative for cut - "reverse_direction": False, + "next_step": ( + "Cut the hole through the part with create_cut_extrude, using " + "suggested_cut as its input" + ), + "suggested_cut": { + "sketch_name": sketch_name, + "depth": 10.0, + "end_condition": "ThroughAll", }, } diff --git a/src/solidworks_mcp/tools/support_levels.py b/src/solidworks_mcp/tools/support_levels.py index 1958585a..1cbaeb5a 100644 --- a/src/solidworks_mcp/tools/support_levels.py +++ b/src/solidworks_mcp/tools/support_levels.py @@ -78,20 +78,11 @@ def warning(self) -> str | None: _PLANNED = Support(SupportLevel.PLANNED, "no adapter implements it yet.") -def _beta(reason: str) -> Support: - return Support(SupportLevel.BETA, reason) - - -_EXPORT_FORMAT = _beta("`options` is ignored; the file extension sets the format.") - SUPPORT_LEVELS: dict[str, Support] = { # analysis "analyze_geometry": _PLANNED, - "calculate_mass_properties": _beta( - "moments of inertia are labelled kg·mm² but are kg·m²; its input fields " - "are unused." - ), - "check_interference": _beta("`include_hidden` has no effect."), + "calculate_mass_properties": _STABLE, + "check_interference": _STABLE, "get_mass_properties": _STABLE, "get_material_properties": _PLANNED, "verify_model": _STABLE, @@ -113,9 +104,7 @@ def _beta(reason: str) -> Support: # drawing "add_annotation": _PLANNED, "add_dimension": _PLANNED, - "add_drawing_view": _beta( - "only ever adds a view, whatever `operation` says; use `create_drawing_view`." - ), + "add_drawing_view": _STABLE, "add_note": _STABLE, "auto_center_marks": _STABLE, "auto_dimension_view": _PLANNED, @@ -123,10 +112,7 @@ def _beta(reason: str) -> Support: "create_detail_view": _PLANNED, "create_drawing_view": _STABLE, "create_section_view": _PLANNED, - "create_technical_drawing": _beta( - "ignores `template`, `sheet_size`, `title` and `output_path`; it only " - "places three views." - ), + "create_technical_drawing": _STABLE, "list_drawing_views": _STABLE, "update_sheet_format": _PLANNED, "update_title_block": _PLANNED, @@ -150,7 +136,7 @@ def _beta(reason: str) -> Support: # engineering_data "export_flat_pattern": _STABLE, "get_custom_properties": _STABLE, - "get_material": _beta("reads the material's name and library, not its properties."), + "get_material": _STABLE, "list_available_materials": _STABLE, "list_model_dimensions": _STABLE, "measure_entities": _STABLE, @@ -158,12 +144,12 @@ def _beta(reason: str) -> Support: "set_material": _STABLE, # export "batch_export": _PLANNED, - "export_dwg": _EXPORT_FORMAT, - "export_iges": _EXPORT_FORMAT, + "export_dwg": _STABLE, + "export_iges": _STABLE, "export_image": _STABLE, - "export_pdf": _EXPORT_FORMAT, - "export_step": _EXPORT_FORMAT, - "export_stl": _EXPORT_FORMAT, + "export_pdf": _STABLE, + "export_step": _STABLE, + "export_stl": _STABLE, # file_management "activate_document": _STABLE, "batch_file_operations": _PLANNED, @@ -179,18 +165,16 @@ def _beta(reason: str) -> Support: "load_part": _UNCHECKED, "manage_file_properties": _PLANNED, "pack_and_go_assembly": _UNCHECKED, - "save_as": _beta("replaces an existing file whatever `overwrite` says."), + "save_as": _STABLE, "save_assembly": _STABLE, "save_body_as_part": _STABLE, - "save_file": _beta("`force_save` is ignored."), + "save_file": _STABLE, "save_part": _STABLE, # macro_recording "analyze_macro": _PLANNED, "batch_execute_macros": _PLANNED, "create_macro_library": _PLANNED, - "execute_macro": _beta( - "runs the macro once whatever `repeat_count` says, and only binary .swp files." - ), + "execute_macro": _STABLE, "optimize_macro": _PLANNED, "start_macro_recording": _PLANNED, "stop_macro_recording": _PLANNED, @@ -199,31 +183,24 @@ def _beta(reason: str) -> Support: "add_fillet": _STABLE, "add_mate": _STABLE, "close_model": _STABLE, - "create_assembly": _beta("`components` is ignored; use `insert_component`."), + "create_assembly": _STABLE, "create_axis": _STABLE, "create_cut_extrude": _STABLE, - "create_drawing": _beta("`model_path` and `sheet_format` are ignored."), - "create_extrusion": _beta( - "`sketch_name` and `direction` are ignored; it extrudes the sketch closed last." - ), + "create_drawing": _STABLE, + "create_extrusion": _STABLE, "create_loft": _STABLE, - "create_part": _beta("`material` is ignored; use `set_material`."), + "create_part": _STABLE, "create_reference_plane": _STABLE, "create_reference_point": _STABLE, - "create_revolve": _beta( - "`sketch_name` and `axis_entity` are ignored; it revolves the sketch " - "closed last around its centreline." - ), + "create_revolve": _STABLE, "create_sweep": _STABLE, - "delete_feature": _beta( - "on SolidWorks its message doesn't name the dependent features it removed." - ), + "delete_feature": _STABLE, "get_dimension": _UNCHECKED, "insert_component": _STABLE, "list_components": _STABLE, - "mirror_feature": _beta("mirroring a cut is reported as an error."), + "mirror_feature": _STABLE, "open_model": _STABLE, - "pattern_circular": _beta("patterning a cut is reported as an error."), + "pattern_circular": _STABLE, "rebuild_model": _STABLE, "rename_feature": _STABLE, "select_face_edges": _STABLE, @@ -247,18 +224,13 @@ def _beta(reason: str) -> Support: "add_sketch_dimension": _STABLE, "add_spline": _STABLE, "check_sketch_fully_defined": _STABLE, - "create_sketch": _beta( - "echoes a `sketch_name` you pass instead of SolidWorks' own name." - ), + "create_sketch": _STABLE, "exit_sketch": _STABLE, "sketch_circular_pattern": _STABLE, "sketch_linear_pattern": _STABLE, "sketch_mirror": _STABLE, "sketch_offset": _STABLE, - "sketch_tutorial_simple_hole": _beta( - "suggests a negative-depth extrusion for the hole instead of " - "`create_cut_extrude`." - ), + "sketch_tutorial_simple_hole": _STABLE, "tutorial_simple_hole": _PLANNED, # its cut step fails on real SolidWorks # template_management "apply_template": _PLANNED, diff --git a/src/solidworks_mcp/tools/tool_boundary.py b/src/solidworks_mcp/tools/tool_boundary.py index ddda5ad7..709c8757 100644 --- a/src/solidworks_mcp/tools/tool_boundary.py +++ b/src/solidworks_mcp/tools/tool_boundary.py @@ -39,6 +39,7 @@ Classification, ErrorClassifier, ) +from .input_compat import removed_inputs from .support_levels import SUPPORT_LEVELS, Support from .tool_hints import TOOL_HINTS, ToolHints @@ -230,7 +231,9 @@ def tool_boundary( A dict passed where the tool expects its input model is validated as that model first, so a script can call any tool function with a dict, as - an MCP client effectively does. + an MCP client effectively does. An input the tool no longer takes + (``input_compat.REMOVED_INPUTS``) is refused as invalid input before the + tool runs. Returns: The wrapped tool. @@ -246,6 +249,10 @@ def with_models( for name, model in models.items(): if isinstance(bound.arguments.get(name), dict): bound.arguments[name] = normalize_input(bound.arguments[name], model) + value = bound.arguments.get(name) + removed = removed_inputs(value) if isinstance(value, BaseModel) else [] + if removed: + raise InvalidInputError("; ".join(removed)) return bound @functools.wraps(fn) diff --git a/src/solidworks_mcp/version.py b/src/solidworks_mcp/version.py index 0d24e024..59cfe524 100644 --- a/src/solidworks_mcp/version.py +++ b/src/solidworks_mcp/version.py @@ -1,6 +1,6 @@ """Version information for SolidWorks MCP Server.""" -__version__ = "1.5.0" +__version__ = "2.0.0" __author__ = "Andrew Bartels" __email__ = "andrewbartels1@gmail.com" __description__ = "SolidWorks MCP Server with FastMCP and PydanticAI" diff --git a/tests/conftest.py b/tests/conftest.py index d1a623f8..09d7092c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -396,7 +396,6 @@ def create_extrusion_data() -> dict[str, Any]: return { "sketch_name": "test_sketch", "depth": 10.0, - "direction": "blind", "reverse": False, } diff --git a/tests/integration/test_all_endpoints_harness.py b/tests/integration/test_all_endpoints_harness.py index a6c041cf..4eb429b7 100644 --- a/tests/integration/test_all_endpoints_harness.py +++ b/tests/integration/test_all_endpoints_harness.py @@ -782,7 +782,7 @@ async def test_c03_sketch_and_extrude( assert result["status"] == "success" save_file = await _find_tool(real_server, "save_file") - result = await save_file({"force_save": True}) + result = await save_file({}) assert result["status"] == "success" @pytest.mark.asyncio diff --git a/tests/integration/test_integration.py b/tests/integration/test_integration.py index 08a1dab7..a11c93bf 100644 --- a/tests/integration/test_integration.py +++ b/tests/integration/test_integration.py @@ -134,7 +134,7 @@ async def test_simple_part_creation_workflow(self, mock_config): assert create_extrusion_tool is not None extrusion_result = await create_extrusion_tool( - CreateExtrusionInput(sketch_name="Sketch1", depth=10.0, direction="blind") + CreateExtrusionInput(sketch_name="Sketch1", depth=10.0) ) workflow_results.append(("create_extrusion", extrusion_result)) assert extrusion_result["status"] == "success" diff --git a/tests/integration/test_real_solidworks_integration.py b/tests/integration/test_real_solidworks_integration.py index fe67b35e..45be894e 100644 --- a/tests/integration/test_real_solidworks_integration.py +++ b/tests/integration/test_real_solidworks_integration.py @@ -221,7 +221,7 @@ async def test_real_part_create_save_open_close( open_result = await open_model(OpenModelInput(file_path=str(part_path))) assert open_result["status"] == "success", open_result - save_result = await save_file(SaveFileInput(force_save=True)) + save_result = await save_file(SaveFileInput()) assert save_result["status"] == "success", save_result final_close = await close_model(CloseModelInput(save=True)) @@ -296,7 +296,7 @@ async def test_real_cross_category_minimal_smoke( reopen_result = await open_model(OpenModelInput(file_path=str(smoke_part_path))) assert reopen_result["status"] == "success", reopen_result - save_result = await save_file(SaveFileInput(force_save=True)) + save_result = await save_file(SaveFileInput()) assert save_result["status"] == "success", save_result close_result = await close_model(CloseModelInput(save=True)) diff --git a/tests/live/evidence.py b/tests/live/evidence.py index c14c929c..012fea01 100644 --- a/tests/live/evidence.py +++ b/tests/live/evidence.py @@ -113,23 +113,40 @@ def starting(label: str, call: str | None = None) -> None: def _print(label: str, payload: Any, call: str | None) -> None: - """Print the proof in colour; plain text when ``rich`` is missing.""" + """Print the proof in colour; plain text when ``rich`` is missing. + + A console on a legacy code page (cp1252) can't encode ``π`` or ``²``; + the line is then printed plain with those characters replaced, so a + printed proof never fails the test it belongs to. + """ try: from rich.console import Console from rich.markup import escape from rich.pretty import Pretty from rich.text import Text except ImportError: - call_text = f" {call} ->" if call else "" - print(f"\n[evidence] {label}:{call_text} {payload}", flush=True) + _print_plain(label, payload, call) return console = Console(force_terminal=True, highlight=True, width=_CAPTION_WIDTH_CHARS) - console.print() - console.print(f"[bold green]\\[evidence][/] [bold]{escape(label)}[/]") - if call: - console.print(Text("call ", style="bold cyan"), call, sep="") - console.print(Text("result", style="bold cyan")) - console.print(Pretty(payload)) + try: + console.print() + console.print(f"[bold green]\\[evidence][/] [bold]{escape(label)}[/]") + if call: + console.print(Text("call ", style="bold cyan"), call, sep="") + console.print(Text("result", style="bold cyan")) + console.print(Pretty(payload)) + except UnicodeEncodeError: + _print_plain(label, payload, call) + + +def _print_plain(label: str, payload: Any, call: str | None) -> None: + """Print the proof uncoloured, replacing what the console can't encode.""" + import sys + + call_text = f"\ncall {call}\nresult" if call else "" + text = f"\n[evidence] {label}{call_text}\n{pprint.pformat(payload, sort_dicts=False)}\n" + encoding = getattr(sys.stdout, "encoding", None) or "utf-8" + print(text.encode(encoding, "replace").decode(encoding), flush=True) def _test_name() -> str: diff --git a/tests/live/test_live_sw_beta_tools.py b/tests/live/test_live_sw_beta_tools.py new file mode 100644 index 00000000..0b1c21d5 --- /dev/null +++ b/tests/live/test_live_sw_beta_tools.py @@ -0,0 +1,590 @@ +"""The beta tools' fixes (``beta-tool-faults``), each checked on real SolidWorks. + +Every test calls the tools as an agent would, through the MCP tool functions +with plain dicts, and prints its proof as ``[evidence]`` lines. They run in +five batches, one test per process, each with the maintainer's permission: + +1. sketch and feature inputs: ``create_extrusion`` and ``create_revolve`` + with ``sketch_name`` and ``axis_entity``, ``create_sketch`` naming its + sketch, ``create_part`` with a material; +2. ``pattern_circular`` and ``mirror_feature`` on cuts, ``delete_feature`` + naming the dependents it removed; +3. ``calculate_mass_properties`` in kg·mm², ``check_interference`` and + hidden components, ``get_material``'s density; +4. ``save_as`` and ``overwrite``, ``save_file``, the five ``export_*`` tools, + ``execute_macro`` repeating; +5. ``create_assembly``, ``create_drawing``, ``create_technical_drawing``, + ``add_drawing_view`` and ``sketch_tutorial_simple_hole``. + +Run one with:: + + powershell -ExecutionPolicy Bypass -File tests\\scripts\\live_demo.ps1 ` + -Test tests/live/test_live_sw_beta_tools.py::test_extrude_an_earlier_sketch_by_name + +Collected only with ``SOLIDWORKS_MCP_RUN_REAL_INTEGRATION=1`` on Windows (see +``tests/live/conftest.py``). +""" + +from __future__ import annotations + +import math +from pathlib import Path +from typing import Any + +import pytest + +from tests.live.evidence import evidence +from tests.live.test_live_sw_ujoint_tool_gaps import _plate_and_pin, _plate_then_pin + +pytestmark = pytest.mark.asyncio + + +async def _tools(adapter: Any) -> dict[str, Any]: + """Every tool family these tests need, registered on ``adapter``; name -> fn.""" + from fastmcp import FastMCP + + from solidworks_mcp.tools.analysis import register_analysis_tools + from solidworks_mcp.tools.drawing import register_drawing_tools + from solidworks_mcp.tools.engineering_data import register_engineering_data_tools + from solidworks_mcp.tools.export import register_export_tools + from solidworks_mcp.tools.file_management import register_file_management_tools + from solidworks_mcp.tools.macro_recording import register_macro_recording_tools + from solidworks_mcp.tools.modeling import register_modeling_tools + from solidworks_mcp.tools.sketching import register_sketching_tools + + mcp = FastMCP("beta-tools-live") + for register in ( + register_modeling_tools, + register_sketching_tools, + register_analysis_tools, + register_drawing_tools, + register_engineering_data_tools, + register_export_tools, + register_file_management_tools, + register_macro_recording_tools, + ): + await register(mcp, adapter, {}) + return {t.name: t.fn for t in await mcp.list_tools()} + + +async def _volume(adapter: Any) -> float: + result = await adapter.get_mass_properties() + assert result.is_success, result.error + return float(result.data.volume) + + +async def _feature_names(adapter: Any) -> list[str]: + result = await adapter.list_features(include_suppressed=True) + assert result.is_success, result.error + return [str(f.get("name")) for f in result.data] + + +async def _sketch( + tools: dict[str, Any], plane: str, draw: dict[str, Any], name: str | None = None +) -> str: + """A closed sketch with one rectangle or circle; its tree name. + + A ``name`` is given when ``exit_sketch`` closes the sketch, so that reply + holds the final name. + """ + args: dict[str, Any] = {"plane": plane} + if name: + args["sketch_name"] = name + made = await tools["create_sketch"](args) + assert made["status"] == "success", made + shape = "add_circle" if "radius" in draw else "add_rectangle" + assert (await tools[shape](draw))["status"] == "success" + closed = await tools["exit_sketch"]() + assert closed["status"] == "success", closed + return str(closed.get("sketch", made["sketch"])["name"]) + + +async def _plate(tools: dict[str, Any], adapter: Any) -> None: + """A 100 x 100 x 10 mm plate on the Top Plane, centred on the origin.""" + assert (await tools["create_part"]({"name": "plate"}))["status"] == "success" + await _sketch(tools, "Top", {"x1": -50, "y1": -50, "x2": 50, "y2": 50}) + boss = await tools["create_extrusion"]({"depth": 10.0}) + assert boss["status"] == "success", boss + + +async def _hole(tools: dict[str, Any], x: float, radius: float = 3.0) -> str: + """A through hole at (x, 0) on the Top Plane; the cut's name.""" + sketch = await _sketch( + tools, "Top", {"center_x": x, "center_y": 0, "radius": radius} + ) + cut = await tools["create_cut_extrude"]( + {"sketch_name": sketch, "depth": 10.0, "end_condition": "ThroughAll"} + ) + assert cut["status"] == "success", cut + return str(cut["cut_extrude"]["name"]) + + +# ---- batch 1: sketch and feature inputs ---------------------------------------- + + +async def test_extrude_an_earlier_sketch_by_name(scratch) -> None: + """Two closed sketches; the named earlier one is extruded, a missing name fails.""" + tools = await _tools(scratch.adapter) + assert (await tools["create_part"]({"name": "two_sketches"}))["status"] == "success" + square = await _sketch(tools, "Front", {"x1": 0, "y1": 0, "x2": 20, "y2": 20}) + await _sketch(tools, "Top", {"center_x": 60, "center_y": 0, "radius": 5}) + + missing = await tools["create_extrusion"]({"sketch_name": "Sketch9", "depth": 5.0}) + used = await tools["create_extrusion"]({"sketch_name": square, "depth": 5.0}) + volume = await _volume(scratch.adapter) + evidence( + "extrude the earlier 20 mm square by name (expect 2,000 mm³)", + { + "missing": missing.get("message"), + "sketch_used": used.get("extrusion", {}).get("sketch"), + "volume_mm3": round(volume, 2), + }, + call=f'create_extrusion({{"sketch_name": "{square}", "depth": 5}})', + ) + assert missing["status"] == "error" and square in missing["message"] + assert used["status"] == "success", used + assert used["extrusion"]["sketch"] == square + assert volume == pytest.approx(2000.0, rel=1e-4) + + +async def test_revolve_about_a_named_axis(scratch) -> None: + """A 10 x 20 mm rectangle 5 mm off Axis1, no centreline: a tube.""" + adapter = scratch.adapter + tools = await _tools(adapter) + assert (await tools["create_part"]({"name": "tube"}))["status"] == "success" + profile = await _sketch(tools, "Front", {"x1": 5, "y1": 0, "x2": 15, "y2": 20}) + axis = (await adapter.create_axis("y")).data["name"] + + revolve = await tools["create_revolve"]( + {"sketch_name": profile, "axis_entity": axis, "angle": 360} + ) + volume = await _volume(adapter) + expected = math.pi * (15**2 - 5**2) * 20 + evidence( + f"revolve about {axis} (expect π·(15² − 5²)·20 = {expected:,.1f} mm³)", + {"revolve": revolve.get("revolve"), "volume_mm3": round(volume, 1)}, + call=f'create_revolve({{"sketch_name": "{profile}", "axis_entity": "{axis}", "angle": 360}})', + ) + assert revolve["status"] == "success", revolve + assert revolve["revolve"]["axis_entity"] == axis + assert volume == pytest.approx(expected, rel=1e-3) + + +async def test_a_named_sketch_is_used_by_a_cut(scratch) -> None: + """exit_sketch gives the name create_sketch was asked for; a cut finds it.""" + tools = await _tools(scratch.adapter) + await _plate(tools, scratch.adapter) + before = await _volume(scratch.adapter) + named = await _sketch( + tools, "Top", {"center_x": 0, "center_y": 0, "radius": 5}, name="HoleProfile" + ) + # The same name again: the second sketch keeps SolidWorks' name. + await tools["create_sketch"]({"plane": "Top", "sketch_name": "HoleProfile"}) + circle = {"center_x": 30, "center_y": 0, "radius": 2} + assert (await tools["add_circle"](circle))["status"] == "success" + taken = await tools["exit_sketch"]() + + cut = await tools["create_cut_extrude"]( + {"sketch_name": "HoleProfile", "depth": 10.0, "end_condition": "ThroughAll"} + ) + removed = before - await _volume(scratch.adapter) + evidence( + "a sketch named HoleProfile, then cut by that name (expect π·5²·10 = 785.4 mm³)", + { + "named": named, + "second_request": [taken["sketch"]["name"], taken.get("warning")], + "removed_mm3": round(removed, 1), + "tree": await _feature_names(scratch.adapter), + }, + call='create_sketch({"plane": "Top", "sketch_name": "HoleProfile"})', + ) + assert named == "HoleProfile" + assert taken["sketch"]["name"] != "HoleProfile" + assert "could not be named" in taken["warning"] + assert cut["status"] == "success", cut + assert removed == pytest.approx(math.pi * 25 * 10, rel=1e-3) + + +async def test_a_new_part_gets_its_material(scratch) -> None: + tools = await _tools(scratch.adapter) + + made = await tools["create_part"]({"name": "steel", "material": "AISI 304"}) + unknown = await tools["create_part"]({"name": "odd", "material": "Unobtainium"}) + evidence( + "create_part with a material, and with one that doesn't exist", + { + "aisi_304": made["part"]["material"], + "unknown": [unknown["part"]["material"], unknown.get("warning")], + }, + call='create_part({"name": "steel", "material": "AISI 304"})', + ) + assert made["part"]["material"] == "AISI 304" + assert unknown["status"] == "success" + assert unknown["part"]["material"] is None + assert "could not be assigned" in unknown["warning"] + + +# ---- batch 2: patterns, mirror and delete --------------------------------------- + + +async def test_a_ring_of_six_holes(scratch) -> None: + """A Ø6 hole patterned 6 times round the y axis: 5 more holes cut.""" + adapter = scratch.adapter + tools = await _tools(adapter) + await _plate(tools, adapter) + cut = await _hole(tools, 30.0) + axis = (await adapter.create_axis("y")).data["name"] + + ring = await tools["pattern_circular"]( + {"features": [cut], "axis": axis, "count": 6} + ) + expected = -5 * math.pi * 3**2 * 10 + evidence( + f"a ring of 6 Ø6 holes (expect a change of {expected:,.1f} mm³)", + { + "message": ring.get("message"), + "volume_change": ring.get("pattern", {}).get("volume_change"), + }, + call=f'pattern_circular({{"features": ["{cut}"], "axis": "{axis}", "count": 6}})', + ) + assert ring["status"] == "success", ring + assert ring["pattern"]["volume_change"] == pytest.approx(expected, rel=1e-3) + + +async def test_a_mirrored_cut(scratch) -> None: + """A hole at x = 30 mirrored about the Right Plane: a second hole at x = -30.""" + tools = await _tools(scratch.adapter) + await _plate(tools, scratch.adapter) + cut = await _hole(tools, 30.0) + + mirrored = await tools["mirror_feature"]( + {"features": [cut], "mirror_plane": "Right Plane", "mirror_bodies": False} + ) + expected = -math.pi * 3**2 * 10 + evidence( + f"a mirrored Ø6 hole (expect a change of {expected:,.1f} mm³)", + { + "message": mirrored.get("message"), + "volume_change": mirrored.get("mirror", {}).get("volume_change"), + }, + call=f'mirror_feature({{"features": ["{cut}"], "mirror_plane": "Right Plane", "mirror_bodies": false}})', + ) + assert mirrored["status"] == "success", mirrored + assert mirrored["mirror"]["volume_change"] == pytest.approx(expected, rel=1e-3) + + +async def test_deleting_a_sketch_names_the_cut_that_went_with_it(scratch) -> None: + tools = await _tools(scratch.adapter) + await _plate(tools, scratch.adapter) + sketch = await _sketch(tools, "Top", {"center_x": 0, "center_y": 0, "radius": 5}) + cut = await tools["create_cut_extrude"]( + {"sketch_name": sketch, "depth": 10.0, "end_condition": "ThroughAll"} + ) + assert cut["status"] == "success", cut + + deleted = await tools["delete_feature"]({"name": sketch}) + evidence( + f"delete {sketch}, which the cut was built from", + { + "message": deleted.get("message"), + "removed": deleted.get("data", {}).get("removed_features"), + }, + call=f'delete_feature({{"name": "{sketch}"}})', + ) + assert deleted["status"] == "success", deleted + removed = deleted["data"]["removed_features"] + assert sketch in removed and len(removed) >= 2 + assert "dependents" in deleted["message"] + + +# ---- batch 3: measurements ------------------------------------------------------ + + +async def test_block_inertia_in_kg_mm2(scratch) -> None: + """A 100 x 20 x 50 mm block: the moments match m(b² + c²)/12 per axis.""" + tools = await _tools(scratch.adapter) + made = await tools["create_part"]({"name": "block", "material": "AISI 1020"}) + assert made["part"]["material"] == "AISI 1020", made + await _sketch(tools, "Top", {"x1": -50, "y1": -25, "x2": 50, "y2": 25}) + assert (await tools["create_extrusion"]({"depth": 20.0}))["status"] == "success" + + props = (await tools["calculate_mass_properties"]({}))["mass_properties"] + mass = props["mass"]["value"] + inertia = props["moments_of_inertia"] + expected = sorted( + mass * (b * b + c * c) / 12 for b, c in ((20, 50), (100, 50), (100, 20)) + ) + measured = sorted(inertia[k] for k in ("Ixx", "Iyy", "Izz")) + evidence( + "block inertia about its centre (expect m(b²+c²)/12 in kg·mm²)", + { + "mass_kg": round(mass, 4), + "measured": [round(v, 2) for v in measured], + "expected": [round(v, 2) for v in expected], + "products": [inertia[k] for k in ("Ixy", "Ixz", "Iyz")], + "units": inertia["units"], + }, + call="calculate_mass_properties({})", + ) + assert inertia["units"] == "kg·mm²" + assert measured == pytest.approx(expected, rel=1e-3) + assert all(abs(inertia[k]) < 1e-6 * expected[-1] for k in ("Ixy", "Ixz", "Iyz")) + + +async def test_a_hidden_component_is_left_out_unless_asked(scratch, tmp_path) -> None: + """The pin overlaps the plate; hidden, it counts only with include_hidden.""" + from solidworks_mcp.adapters import sw_type_info + + adapter = scratch.adapter + tools = await _tools(adapter) + plate, pin = await _plate_and_pin(scratch, tools, tmp_path, "hidden") + assert (await adapter.create_assembly()).is_success + assert (await adapter.insert_component(plate, 0.0, 0.0, 0.0)).is_success + assert (await adapter.insert_component(pin, 0.0, 0.0, 0.0)).is_success + scratch.written_files.append(str(tmp_path / "hidden.SLDASM")) + _plate_name, pin_name = await _plate_then_pin(adapter) + assembly = sw_type_info.flagged(adapter.currentModel, "IAssemblyDoc") + for component in assembly.GetComponents(True): + c = sw_type_info.flagged(component, "IComponent2") + if str(c.Name2) == pin_name: + c.Visible = 0 # swComponentHidden + + default = await tools["check_interference"]({}) + hidden_too = await tools["check_interference"]({"include_hidden": True}) + evidence( + "the overlapping pin hidden: checked by default, then with include_hidden", + { + "default": default["data"]["interference_count"], + "include_hidden": hidden_too["data"]["interference_count"], + }, + call='check_interference({"include_hidden": true})', + ) + assert default["data"]["interference_count"] == 0 + assert hidden_too["data"]["interference_count"] >= 1 + + +async def test_get_material_reports_the_density(scratch) -> None: + tools = await _tools(scratch.adapter) + assert (await tools["create_part"]({"name": "steel", "material": "AISI 304"}))[ + "status" + ] == "success" + await _sketch(tools, "Top", {"x1": -10, "y1": -10, "x2": 10, "y2": 10}) + assert (await tools["create_extrusion"]({"depth": 10.0}))["status"] == "success" + + read = await tools["get_material"]({}) + evidence( + "get_material on an AISI 304 part (expect 8,000 kg/m³)", + read["data"], + call="get_material({})", + ) + assert read["data"]["material"] == "AISI 304" + assert read["data"]["density_kg_m3"] == pytest.approx(8000.0) + + +# ---- batch 4: files and macros -------------------------------------------------- + + +async def _saved_block(scratch: Any, tools: dict[str, Any], path: Path) -> None: + assert (await tools["create_part"]({"name": path.stem}))["status"] == "success" + await _sketch(tools, "Top", {"x1": -10, "y1": -10, "x2": 10, "y2": 10}) + assert (await tools["create_extrusion"]({"depth": 10.0}))["status"] == "success" + saved = await tools["save_file"]({"file_path": str(path)}) + scratch.written_files.append(str(path)) + assert saved["status"] == "success", saved + + +async def test_save_as_keeps_an_existing_file(scratch, tmp_path) -> None: + tools = await _tools(scratch.adapter) + first = tmp_path / "keep.SLDPRT" + await _saved_block(scratch, tools, first) + copy = tmp_path / "copy.SLDPRT" + scratch.written_files.append(str(copy)) + assert (await tools["save_as"]({"file_path": str(copy)}))["status"] == "success" + stamp = (copy.stat().st_size, copy.stat().st_mtime_ns) + + kept = await tools["save_as"]({"file_path": str(copy)}) + untouched = (copy.stat().st_size, copy.stat().st_mtime_ns) == stamp + replaced = await tools["save_as"]({"file_path": str(copy), "overwrite": True}) + force = await tools["save_file"]({"force_save": True}) + evidence( + "save_as onto an existing file, without and with overwrite", + { + "without": kept.get("message"), + "file_untouched": untouched, + "with": replaced.get("status"), + "force_save": force.get("message"), + }, + call=f'save_as({{"file_path": "{copy.name}"}})', + ) + assert kept["error_type"] == "invalid_input" + assert stamp[0] > 0 and kept["status"] == "error" and untouched + assert replaced["status"] == "success", replaced + assert force["error_type"] == "invalid_input" + + +async def test_every_export_writes_its_file(scratch, tmp_path) -> None: + """STEP, IGES and STL from the part; PDF and DWG from its drawing.""" + tools = await _tools(scratch.adapter) + part = tmp_path / "export.SLDPRT" + await _saved_block(scratch, tools, part) + sizes: dict[str, int] = {} + # Each tool named in full, so the support-level check finds it. + for tool, ext in ( + ("export_step", "step"), + ("export_iges", "igs"), + ("export_stl", "stl"), + ): + target = tmp_path / f"export.{ext}" + scratch.written_files.append(str(target)) + done = await tools[tool]({"file_path": str(target)}) + assert done["status"] == "success", done + sizes[ext] = target.stat().st_size + assert (await tools["create_drawing"]({"name": "export_drawing"}))[ + "status" + ] == "success" + views = await tools["create_technical_drawing"]({"model_file": str(part)}) + assert views["status"] == "success", views + for tool, ext in (("export_pdf", "pdf"), ("export_dwg", "dwg")): + target = tmp_path / f"export.{ext}" + scratch.written_files.append(str(target)) + done = await tools[tool]({"file_path": str(target)}) + assert done["status"] == "success", done + sizes[ext] = target.stat().st_size + refused = await tools["export_step"]( + {"file_path": str(tmp_path / "x.step"), "options": {"a": 1}} + ) + evidence( + "every export's file size in bytes", + {**sizes, "options": refused.get("message")}, + call="export_step/iges/stl/pdf/dwg", + ) + assert all(size > 0 for size in sizes.values()) + assert refused["error_type"] == "invalid_input" + + +_MACRO = """Dim swApp As Object +Dim Part As Object +Sub main() + Set swApp = Application.SldWorks + Set Part = swApp.ActiveDoc + Part.ClearSelection2 True + Part.Extension.SelectByID2 "Front Plane", "PLANE", 0, 0, 0, False, 0, Nothing, 0 + Part.SketchManager.InsertSketch True + Part.SketchManager.CreatePoint 0, 0, 0 + Part.SketchManager.InsertSketch True +End Sub +""" + + +async def test_a_macro_runs_three_times(scratch, tmp_path) -> None: + """Each run adds one sketch, so three runs add three.""" + tools = await _tools(scratch.adapter) + assert (await tools["create_part"]({"name": "macro"}))["status"] == "success" + macro = tmp_path / "add_sketch.swb" + macro.write_text(_MACRO, encoding="ascii") + before = await _feature_names(scratch.adapter) + + ran = await tools["execute_macro"]( + {"macro_path": str(macro), "repeat_count": 3, "pause_between_runs": 1} + ) + added = [n for n in await _feature_names(scratch.adapter) if n not in before] + evidence( + "a macro that adds one sketch, run 3 times", + { + "message": ran.get("message"), + "runs_completed": ran.get("runs_completed"), + "added": added, + }, + call=f'execute_macro({{"macro_path": "{macro.name}", "repeat_count": 3}})', + ) + assert ran["status"] == "success", ran + assert ran["runs_completed"] == 3 + assert len(added) == 3 + + +# ---- batch 5: drawings and the tutorial ----------------------------------------- + + +async def test_new_documents_refuse_removed_inputs(scratch) -> None: + tools = await _tools(scratch.adapter) + + assembly = await tools["create_assembly"]({"name": "asm"}) + refused_asm = await tools["create_assembly"]( + {"name": "asm2", "components": ["a.sldprt"]} + ) + drawing = await tools["create_drawing"]({"name": "drw"}) + refused_drw = await tools["create_drawing"]( + {"name": "drw2", "model_path": "a.sldprt"} + ) + evidence( + "create_assembly and create_drawing, then each with a removed input", + { + "assembly": assembly.get("assembly"), + "components": refused_asm.get("message"), + "drawing": drawing.get("drawing"), + "model_path": refused_drw.get("message"), + }, + call='create_assembly({"name": "asm", "components": ["a.sldprt"]})', + ) + assert assembly["status"] == "success" and drawing["status"] == "success" + assert refused_asm["error_type"] == "invalid_input" + assert refused_drw["error_type"] == "invalid_input" + + +async def test_three_views_and_one_more(scratch, tmp_path) -> None: + """Standard views in first-angle projection, an isometric view, and a refused remove.""" + tools = await _tools(scratch.adapter) + part = tmp_path / "views.SLDPRT" + await _saved_block(scratch, tools, part) + assert (await tools["create_drawing"]({"name": "views"}))["status"] == "success" + + standard = await tools["create_technical_drawing"]( + {"model_file": str(part), "projection": "first_angle"} + ) + iso = await tools["add_drawing_view"]( + # The sheet's free lower right, clear of the three standard views. + {"model_file": str(part), "view_type": "isometric", "position": [230, 100]} + ) + remove = await tools["add_drawing_view"]( + {"operation": "remove", "view_name": "Drawing View1"} + ) + listed = await tools["list_drawing_views"]() + evidence( + "three standard views (first angle), one isometric view, a refused remove", + { + "standard": standard.get("data", {}).get("views"), + "projection": standard.get("data", {}).get("projection"), + "iso": iso.get("status"), + "remove": remove.get("message"), + "count": listed.get("count"), + }, + call='create_technical_drawing({"model_file": "views.SLDPRT", "projection": "first_angle"})', + ) + assert standard["status"] == "success", standard + assert len(standard["data"]["views"]) == 3 + assert iso["status"] == "success", iso + assert remove["error_type"] == "invalid_input" + assert listed["count"] == 4 + + +async def test_the_hole_tutorial_runs_as_written(scratch) -> None: + """Its sketch, then its suggested cut, makes a Ø5 hole in a plate.""" + tools = await _tools(scratch.adapter) + await _plate(tools, scratch.adapter) + before = await _volume(scratch.adapter) + + tutorial = await tools["sketch_tutorial_simple_hole"]() + cut = await tools["create_cut_extrude"](tutorial["suggested_cut"]) + removed = before - await _volume(scratch.adapter) + expected = math.pi * 2.5**2 * 10 + evidence( + f"the tutorial's sketch and suggested cut (expect π·2.5²·10 = {expected:.1f} mm³)", + { + "next_step": tutorial["next_step"], + "suggested_cut": tutorial["suggested_cut"], + "removed_mm3": round(removed, 1), + }, + call="sketch_tutorial_simple_hole(), then create_cut_extrude(suggested_cut)", + ) + assert cut["status"] == "success", cut + assert removed == pytest.approx(expected, rel=1e-3) diff --git a/tests/solidworks_mcp/adapters/solidworks/test_documentation.py b/tests/solidworks_mcp/adapters/solidworks/test_documentation.py index 55bcb66a..6d81c684 100644 --- a/tests/solidworks_mcp/adapters/solidworks/test_documentation.py +++ b/tests/solidworks_mcp/adapters/solidworks/test_documentation.py @@ -453,6 +453,7 @@ async def test_get_material_reads_the_assignment(raw, material, database) -> Non assert result.data == { "material": material, "database": database, + "density_kg_m3": None, # no SolidWorks app to find the library through "configuration": "Default", } @@ -775,3 +776,57 @@ def test_unpack_custom_property_shapes(self) -> None: ) assert _unpack_custom_property(("P-001", "P-001", False)) == ("P-001", "P-001") assert _unpack_custom_property(None) == ("", "") + + +_DENSITIES = """ + + + + + + + + + +""" + + +def test_material_density_reads_the_dens_value(tmp_path) -> None: + """Libraries are UTF-16 XML; DENS holds kg/m³.""" + other = tmp_path / "custom.sldmat" + other.write_text(_DENSITIES.replace("8000", "1"), encoding="utf-16") + library = tmp_path / "solidworks materials.sldmat" + library.write_text(_DENSITIES, encoding="utf-16") + broken = tmp_path / "broken.sldmat" + broken.write_text(" None: + folder = _library_folder(tmp_path) + + files = doc._library_files([str(tmp_path / "missing"), str(folder)]) + + assert files == [str(folder / "solidworks materials.sldmat")] + + +@pytest.mark.asyncio +async def test_get_material_reports_the_density_from_the_library(tmp_path) -> None: + folder = tmp_path / "lib" + folder.mkdir() + (folder / "solidworks materials.sldmat").write_text(_DENSITIES, encoding="utf-16") + harness = _Harness( + _part(("AISI 304", "SOLIDWORKS Materials", "byref")), _app(str(folder)) + ) + + result = await harness.get_material() + + assert result.data["material"] == "AISI 304" + assert result.data["density_kg_m3"] == 8000.0 diff --git a/tests/solidworks_mcp/adapters/solidworks/test_features.py b/tests/solidworks_mcp/adapters/solidworks/test_features.py index eb8e8647..93b2d75e 100644 --- a/tests/solidworks_mcp/adapters/solidworks/test_features.py +++ b/tests/solidworks_mcp/adapters/solidworks/test_features.py @@ -1526,10 +1526,50 @@ def test_errors_when_edit_delete_did_not_remove_feature(self, monkeypatch) -> No feature = SimpleNamespace(Select2=lambda append, mark: True) adapter.currentModel.FeatureByName.return_value = feature + adapter.currentModel.Extension.DeleteSelection2.return_value = False + adapter.currentModel.SelectionManager.GetSelectedObjectCount2.return_value = 1 + result = _delete_feature_impl(adapter, "Boss-Extrude1") assert result.is_error - assert "EditDelete did not remove feature" in (result.error or "") + assert "SolidWorks did not delete feature" in (result.error or "") + assert "returned False with 1 item(s) selected" in (result.error or "") + + def test_a_used_sketch_goes_with_the_features_built_from_it( + self, monkeypatch + ) -> None: + """SolidWorks refuses the sketch alone; its cut goes, absorbing it.""" + adapter = _build_adapter(monkeypatch) + adapter.currentModel = MagicMock() + cut = SimpleNamespace(Name="Cut-Extrude1") + nameless = SimpleNamespace(Name=None) + sketch = _selectable_feature(GetChildren=lambda: (cut, nameless)) + adapter.currentModel.FeatureByName.side_effect = [ + sketch, + _selectable_feature(), # the cut, selected to delete + None, # the sketch is gone afterwards + ] + delete = adapter.currentModel.Extension.DeleteSelection2 + delete.side_effect = [False, True] + + result = _delete_feature_impl(adapter, "Sketch2") + + assert result.is_success, result.error + assert [c.args for c in delete.call_args_list] == [(1,), (3,)] + adapter.currentModel.FeatureByName.assert_any_call("Cut-Extrude1") + + def test_a_raising_delete_says_what_it_raised(self, monkeypatch) -> None: + adapter = _build_adapter(monkeypatch) + adapter.currentModel = MagicMock() + feature = SimpleNamespace(Select2=lambda append, mark: True) + adapter.currentModel.FeatureByName.return_value = feature + adapter.currentModel.Extension.DeleteSelection2.side_effect = RuntimeError( + "Member not found" + ) + + result = _delete_feature_impl(adapter, "Sketch2") + + assert "raised Member not found" in (result.error or "") class TestSuppressFeatureImplMoreErrors: @@ -2297,7 +2337,6 @@ async def test_mock_create_axis_plane_pairs_match_the_com_layer() -> None: @pytest.mark.parametrize( ("method", "impl", "args"), [ - ("delete_feature", "_delete_feature_impl", ("Fillet1",)), ("suppress_feature", "_suppress_feature_impl", ("Fillet1", False)), ("rename_feature", "_rename_feature_impl", ("Fillet1", "EdgeRound")), ("undo", "_undo_impl", (2,)), @@ -2389,10 +2428,39 @@ async def test_success_reports_volume_before_and_after( "name": "Feature9", "volume_before": 100.0, "volume_after": 250.0, + "volume_change": 150.0, "volume_ratio": 2.5, } assert result.execution_time == 0.5 + @pytest.mark.asyncio + @pytest.mark.parametrize( + ("method", "impl", "args"), + [ + ("pattern_circular", "_pattern_circular_impl", (["Cut1"], "Axis1", 6)), + ("mirror_feature", "_mirror_feature_impl", (["Cut1"], "Right Plane")), + ], + ) + async def test_a_cut_lowers_the_volume_and_succeeds( + self, monkeypatch, method, impl, args + ) -> None: + """A ring of holes or a mirrored cut removes material: that counts.""" + adapter = _build_adapter(monkeypatch) + adapter.currentModel = MagicMock() + adapter.get_mass_properties = self._volumes(1000.0, 900.0) + monkeypatch.setattr( + features, + impl, + lambda *_a: AdapterResult( + status=AdapterResultStatus.SUCCESS, data={"name": "Feature9"} + ), + ) + + result = await getattr(adapter, method)(*args) + + assert result.is_success + assert result.data["volume_change"] == -100.0 + def test_sweep_prefers_the_last_created_sketch_as_profile() -> None: """A helix base circle precedes the profile; the tracked sketch wins.""" @@ -2543,6 +2611,8 @@ def test_delete_reports_how_many_features_went(self, monkeypatch) -> None: result = _delete_feature_impl(adapter, "Boss-Extrude1@Part1") + # Children too: EditDelete left a sketch a cut used in place. + adapter.currentModel.Extension.DeleteSelection2.assert_called_once_with(1) assert result.data == { "deleted": "Boss-Extrude1@Part1", "features_before": 7, @@ -3427,3 +3497,127 @@ async def test_a_failed_fillet_is_returned_unchanged(self, monkeypatch) -> None: assert await adapter.add_fillet(1.0, ["0,0,0"]) is failed adapter.get_mass_properties.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_delete_feature_names_every_feature_it_removed(monkeypatch) -> None: + """The tree is listed before and after; the names that went are reported.""" + adapter = _build_adapter(monkeypatch) + listed = [ + [{"name": "Sketch1"}, {"name": "Sketch2"}, {"name": "Cut-Extrude1"}], + [{"name": "Sketch1"}], + ] + adapter.list_features = AsyncMock( + side_effect=[ + AdapterResult(status=AdapterResultStatus.SUCCESS, data=d) for d in listed + ] + ) + monkeypatch.setattr( + features, + "_delete_feature_impl", + lambda *_a: AdapterResult( + status=AdapterResultStatus.SUCCESS, + data={"deleted": "Sketch2", "features_removed": 2}, + execution_time=0.2, + ), + ) + + result = await adapter.delete_feature("Sketch2") + + assert result.data["removed_features"] == ["Sketch2", "Cut-Extrude1"] + assert result.execution_time == 0.2 + + +@pytest.mark.asyncio +async def test_delete_feature_falls_back_to_the_name_without_a_listing( + monkeypatch, +) -> None: + adapter = _build_adapter(monkeypatch) + adapter.list_features = AsyncMock( + return_value=AdapterResult(status=AdapterResultStatus.ERROR, error="no tree") + ) + monkeypatch.setattr( + features, + "_delete_feature_impl", + lambda *_a: AdapterResult(status=AdapterResultStatus.SUCCESS, data=None), + ) + + result = await adapter.delete_feature("Fillet1@Part1") + + assert result.data == {"removed_features": ["Fillet1"]} + + +@pytest.mark.asyncio +async def test_a_failed_delete_is_returned_unchanged(monkeypatch) -> None: + adapter = _build_adapter(monkeypatch) + adapter.list_features = AsyncMock( + return_value=AdapterResult(status=AdapterResultStatus.SUCCESS, data=[]) + ) + failed = AdapterResult(status=AdapterResultStatus.ERROR, error="Feature not found") + monkeypatch.setattr(features, "_delete_feature_impl", lambda *_a: failed) + + assert await adapter.delete_feature("Nope") is failed + adapter.list_features.assert_awaited_once() + + +def test_renaming_the_latest_sketch_keeps_it_tracked() -> None: + """A later feature without sketch_name still finds the renamed sketch.""" + sketch = _Feature("Sketch3") + adapter = _rename_adapter({"Sketch3": sketch, "Profile": sketch}) + adapter.currentModel.FeatureByName = lambda name: ( + sketch if name in {"Sketch3", sketch.Name} else None + ) + adapter._last_sketch_name = "Sketch3" + + result = features._rename_feature_impl(adapter, "Sketch3", "Profile") + + assert result.data["new_name"] == "Profile" + assert adapter._last_sketch_name == "Profile" + + +# ---- revolve sketch and axis ------------------------------------------------ + + +def _revolve_adapter(axis_selects: bool) -> _FakeFeatureAdapter: + adapter = _FakeFeatureAdapter() + picks: list[tuple] = [] + + def select(name, kind, *_rest): + picks.append((name, kind, _rest[3], _rest[4])) # append, mark + return kind != "AXIS" or axis_selects + + adapter.currentModel = SimpleNamespace( + Extension=SimpleNamespace(SelectByID2=select) + ) + adapter.picks = picks # type: ignore[attr-defined] + return adapter + + +def test_select_revolve_inputs_does_nothing_without_names() -> None: + adapter = _revolve_adapter(axis_selects=True) + params = SimpleNamespace(sketch_name=None, axis_entity=None) + + assert features._select_revolve_inputs(adapter, params) is None + assert adapter.picks == [] + + +def test_select_revolve_inputs_appends_the_axis_with_mark_4(monkeypatch) -> None: + adapter = _revolve_adapter(axis_selects=True) + monkeypatch.setattr( + features, "_resolve_profile_sketch", lambda _a, name, purpose: name or "Sketch1" + ) + params = SimpleNamespace(sketch_name="Sketch2", axis_entity="Axis1") + + assert features._select_revolve_inputs(adapter, params) == "Sketch2" + assert adapter.picks == [("Axis1", "AXIS", True, 4)] + + +def test_select_revolve_inputs_names_an_axis_that_is_not_there(monkeypatch) -> None: + adapter = _revolve_adapter(axis_selects=False) + monkeypatch.setattr( + features, "_resolve_profile_sketch", lambda _a, name, purpose: "Sketch1" + ) + params = SimpleNamespace(sketch_name=None, axis_entity="Line1") + + with pytest.raises(Exception, match="Axis 'Line1' is not a reference axis"): + features._select_revolve_inputs(adapter, params) diff --git a/tests/solidworks_mcp/adapters/solidworks/test_io.py b/tests/solidworks_mcp/adapters/solidworks/test_io.py index c0ec2cb9..f3f7a676 100644 --- a/tests/solidworks_mcp/adapters/solidworks/test_io.py +++ b/tests/solidworks_mcp/adapters/solidworks/test_io.py @@ -78,6 +78,28 @@ async def test_get_mass_properties_uses_callable_gmp() -> None: assert result.data.mass == 0.3 +@pytest.mark.asyncio +async def test_get_mass_properties_reads_products_in_the_documented_order() -> None: + """GetMassProperties ends Lxy, Lzx, Lyz: index 10 is Ixz, 11 is Iyz.""" + raw = [0.0, 0.0, 0.0, 1.0, 2.0, 3.0, 11.0, 22.0, 33.0, 12.0, 13.0, 23.0] + current_model = SimpleNamespace( + ForceRebuild3=lambda _flag: None, + Extension=SimpleNamespace(CreateMassProperty=lambda: None), + GetMassProperties=raw, + ) + + result = await _IOHarness(current_model).get_mass_properties() + + assert result.data.moments_of_inertia == { + "Ixx": 11.0, + "Iyy": 22.0, + "Izz": 33.0, + "Ixy": 12.0, + "Ixz": 13.0, + "Iyz": 23.0, + } + + @pytest.mark.asyncio async def test_get_mass_properties_missing_gmp_fails() -> None: """Missing GetMassProperties should surface a failure.""" diff --git a/tests/solidworks_mcp/adapters/test_mock_adapter.py b/tests/solidworks_mcp/adapters/test_mock_adapter.py index 927522fe..ec2598df 100644 --- a/tests/solidworks_mcp/adapters/test_mock_adapter.py +++ b/tests/solidworks_mcp/adapters/test_mock_adapter.py @@ -1299,6 +1299,25 @@ async def test_mock_rename_onto_an_existing_name_is_an_error() -> None: assert "already exists" in (result.error or "") +@pytest.mark.asyncio +async def test_mock_rename_of_the_open_sketch_is_refused() -> None: + """SolidWorks keeps an open sketch's name; once closed it can be renamed.""" + adapter = MockSolidWorksAdapter({}) + await adapter.connect() + await adapter.create_part() + await adapter.create_sketch("Front") + name = next(iter(adapter._sketches)) + + refused = await adapter.rename_feature(name, "Profile") + await adapter.exit_sketch() + renamed = await adapter.rename_feature(name, "Profile") + + assert not refused.is_success + assert "open for editing" in (refused.error or "") + assert renamed.is_success + assert list(adapter._sketches) == ["Profile"] + + @pytest.mark.asyncio async def test_mock_rename_to_same_name_is_a_noop_success() -> None: """Renaming a feature to its current name changes nothing but does not fail.""" @@ -2070,3 +2089,19 @@ def test_mock_orientations_match_the_com_layer() -> None: assert set(_NAMED_VIEWS) == set(_MOCK_NAMED_VIEWS), ( "orientation lists have drifted between io.py and mock_adapter.py" ) + + +@pytest.mark.asyncio +async def test_mock_revolve_names_a_sketch_that_is_not_there() -> None: + from solidworks_mcp.adapters.base import RevolveParameters + + adapter = MockSolidWorksAdapter({}) + await adapter.connect() + await adapter.create_part() + + result = await adapter.create_revolve( + RevolveParameters(angle=360.0, sketch_name="Sketch9") + ) + + assert not result.is_success + assert "Sketch9" in result.error and "(none found)" in result.error diff --git a/tests/solidworks_mcp/progress/test_generate.py b/tests/solidworks_mcp/progress/test_generate.py index ff81696b..599a4f53 100644 --- a/tests/solidworks_mcp/progress/test_generate.py +++ b/tests/solidworks_mcp/progress/test_generate.py @@ -52,7 +52,7 @@ async def test_every_registered_tool_gets_a_row() -> None: assert [r.name for r in rows] == sorted(SUPPORT_LEVELS) extrude = next(r for r in rows if r.name == "create_extrusion") assert extrude.family == "modeling" - assert extrude.level is SupportLevel.BETA + assert extrude.level is SupportLevel.STABLE assert extrude.summary.startswith("Extrude the sketch you just closed") assert "\n" not in extrude.summary diff --git a/tests/solidworks_mcp/tools/test_analysis.py b/tests/solidworks_mcp/tools/test_analysis.py index 53430299..45caf279 100644 --- a/tests/solidworks_mcp/tools/test_analysis.py +++ b/tests/solidworks_mcp/tools/test_analysis.py @@ -41,24 +41,30 @@ async def test_calculate_mass_properties_success( """Test successful mass properties calculation.""" await register_analysis_tools(mcp_server, mock_adapter, mock_config) - mock_adapter.calculate_mass_properties = AsyncMock( + # SolidWorks reports inertia in kg·m²; the tool reports kg·mm². + mock_adapter.get_mass_properties = AsyncMock( return_value=Mock( is_success=True, - data={ - "mass": 0.125, - "volume": 1.5e-5, - "center_of_mass": [0.0, 0.0, 10.0], - "moments_of_inertia": {"Ixx": 1.2e-6, "Iyy": 1.2e-6, "Izz": 2.4e-6}, - }, + data=SimpleNamespace( + volume=15000.0, + surface_area=4000.0, + mass=0.125, + center_of_mass=[0.0, 0.0, 10.0], + moments_of_inertia={ + "Ixx": 1.2e-6, + "Iyy": 1.2e-6, + "Izz": 2.4e-6, + "Ixy": 0.0, + "Ixz": 0.0, + "Iyz": -3.0e-7, + }, + principal_axes=None, + ), execution_time=0.3, ) ) - input_data = MassPropertiesInput( - model_path="test_part.sldprt", - units="kg", - reference_coordinate_system="default", - ) + input_data = MassPropertiesInput() # Find and call the tool tool_func = None @@ -71,9 +77,15 @@ async def test_calculate_mass_properties_success( result = await tool_func(input_data=input_data) assert result["status"] == "success" - assert result["data"]["mass"] == 0.125 - assert result["data"]["volume"] == 1.5e-5 - mock_adapter.calculate_mass_properties.assert_called_once() + props = result["mass_properties"] + assert props["mass"]["value"] == 0.125 + assert props["volume"]["value"] == 15000.0 + inertia = props["moments_of_inertia"] + assert inertia["units"] == "kg·mm²" + assert inertia["Ixx"] == pytest.approx(1.2) + assert inertia["Izz"] == pytest.approx(2.4) + assert inertia["Iyz"] == pytest.approx(-0.3) + mock_adapter.get_mass_properties.assert_called_once() @pytest.mark.asyncio async def test_check_interference_success( @@ -96,9 +108,7 @@ async def test_check_interference_success( ) input_data = InterferenceCheckInput( - assembly_path="test_assembly.sldasm", - check_all_components=True, - include_hidden=False, + assembly_path="test_assembly.sldasm", include_hidden=False ) tool_func = None @@ -114,6 +124,18 @@ async def test_check_interference_success( assert result["data"]["interference_count"] == 1 assert "Component1" in result["data"]["interfering_bodies"] mock_adapter.check_interference.assert_called_once() + # Hidden components are left out unless include_hidden is on. + assert ( + mock_adapter.check_interference.call_args.args[0]["ignore_hidden"] is True + ) + await tool_func(input_data={"include_hidden": True}) + assert ( + mock_adapter.check_interference.call_args.args[0]["ignore_hidden"] is False + ) + + refused = await tool_func(input_data={"tolerance": 0.01}) + assert refused["error_type"] == "invalid_input" + assert "`tolerance`" in refused["message"] @pytest.mark.asyncio async def test_mass_properties_adapter_error( @@ -122,13 +144,13 @@ async def test_mass_properties_adapter_error( """Test handling of adapter errors in mass properties.""" await register_analysis_tools(mcp_server, mock_adapter, mock_config) - mock_adapter.calculate_mass_properties = AsyncMock( + mock_adapter.get_mass_properties = AsyncMock( return_value=Mock( is_success=False, error="Model file not found", execution_time=0.1 ) ) - input_data = MassPropertiesInput(model_path="missing_part.sldprt", units="kg") + input_data = MassPropertiesInput() tool_func = None for tool in await mcp_server.list_tools(): @@ -166,30 +188,17 @@ async def test_interference_check_adapter_error( assert "Assembly failed to load" in result["message"] @pytest.mark.unit - def test_mass_properties_input_validation(self): - """Test input validation for mass properties.""" - # Valid input - valid_input = MassPropertiesInput(model_path="test.sldprt", units="kg") - assert valid_input.model_path == "test.sldprt" - assert valid_input.units == "kg" - - # Invalid units should be handled by Pydantic validation - with pytest.raises(ValueError): # Pydantic validation error - MassPropertiesInput(model_path="test.sldprt", units="invalid_unit") + def test_mass_properties_input_has_no_fields(self): + """It always measures the active model, so it takes nothing.""" + assert MassPropertiesInput.model_fields == {} @pytest.mark.unit def test_interference_check_input_validation(self): """Test input validation for interference check.""" # Valid input - valid_input = InterferenceCheckInput( - assembly_path="test.sldasm", check_all_components=True - ) + valid_input = InterferenceCheckInput(assembly_path="test.sldasm") assert valid_input.assembly_path == "test.sldasm" - assert valid_input.check_all_components is True - - # Test optional parameters - minimal_input = InterferenceCheckInput(assembly_path="test.sldasm") - assert minimal_input.check_all_components is False # Default value + assert valid_input.include_hidden is False # hidden components left out @pytest.mark.asyncio async def test_mass_properties_fallback_and_alias_inputs( @@ -198,10 +207,6 @@ async def test_mass_properties_fallback_and_alias_inputs( """Cover fallback mass-property path plus alias normalization branches.""" await register_analysis_tools(mcp_server, mock_adapter, mock_config) - # Remove calculate_mass_properties so the tool uses adapter.get_mass_properties(). - if hasattr(mock_adapter, "calculate_mass_properties"): - delattr(mock_adapter, "calculate_mass_properties") - mock_adapter.get_mass_properties = AsyncMock( return_value=Mock( is_success=True, @@ -245,7 +250,10 @@ async def test_mass_properties_fallback_and_alias_inputs( alias_dict = await alias_tool( input_data={"units": "lb", "include_hidden": True} ) - assert alias_dict["status"] == "success" + assert alias_dict["status"] == "error" + assert alias_dict["error_type"] == "invalid_input" + assert "`units`" in alias_dict["message"] + assert "`include_hidden`" in alias_dict["message"] alias_model = await alias_tool(input_data=MassPropertiesInput()) assert alias_model["status"] == "success" diff --git a/tests/solidworks_mcp/tools/test_drawing.py b/tests/solidworks_mcp/tools/test_drawing.py index 04ee002b..c89d2836 100644 --- a/tests/solidworks_mcp/tools/test_drawing.py +++ b/tests/solidworks_mcp/tools/test_drawing.py @@ -54,14 +54,7 @@ async def test_create_technical_drawing_success( ) ) - input_data = DrawingCreationInput( - model_file="test_part.sldprt", - template="A_size_template.drwdot", - output_path="drawings/part_drawing.slddrw", - sheet_format="A (ANSI) Landscape", - scale="1:1", - auto_populate_views=True, - ) + input_data = DrawingCreationInput(model_file="test_part.sldprt") tool_func = None for tool in await mcp_server.list_tools(): @@ -261,9 +254,7 @@ async def test_drawing_creation_error_handling( ) ) - input_data = DrawingCreationInput( - model_file="test_part.sldprt", template="missing_template.drwdot" - ) + input_data = DrawingCreationInput(model_file="test_part.sldprt") tool_func = None for tool in await mcp_server.list_tools(): @@ -386,21 +377,53 @@ def test_auto_center_marks_input_rejects_blank_view(self): def test_drawing_creation_input_validation(self): """Test input validation for drawing creation.""" # Valid input - valid_input = DrawingCreationInput( - model_file="test.sldprt", template="template.drwdot" - ) + valid_input = DrawingCreationInput(model_file="test.sldprt") assert valid_input.model_file == "test.sldprt" - assert valid_input.template == "template.drwdot" + assert valid_input.projection == "third_angle" + + first = DrawingCreationInput(model_file="test.sldprt", projection="first_angle") + assert first.projection == "first_angle" + with pytest.raises(ValueError): + DrawingCreationInput(model_file="test.sldprt", projection="isometric") + + @pytest.mark.asyncio + async def test_technical_drawing_refuses_removed_inputs( + self, mcp_server, mock_adapter, mock_config + ): + """A removed input is refused with a pointer, before the adapter runs.""" + await register_drawing_tools(mcp_server, mock_adapter, mock_config) + mock_adapter.create_technical_drawing = AsyncMock() + tool = next( + t.fn + for t in await mcp_server.list_tools() + if t.name == "create_technical_drawing" + ) + + result = await tool( + input_data={"model_file": "p.sldprt", "template": "a.drwdot", "title": "T"} + ) + + assert result["error_type"] == "invalid_input" + assert "`template`" in result["message"] and "`title`" in result["message"] + assert "generate_drawing_package" in result["message"] + mock_adapter.create_technical_drawing.assert_not_awaited() - # Test optional parameters - full_input = DrawingCreationInput( - model_file="test.sldprt", - template="template.drwdot", - scale="1:2", - auto_populate_views=True, + @pytest.mark.asyncio + async def test_add_drawing_view_refuses_other_operations( + self, mcp_server, mock_adapter, mock_config + ): + """It only adds views; update or remove is refused, not ignored.""" + await register_drawing_tools(mcp_server, mock_adapter, mock_config) + mock_adapter.add_drawing_view = AsyncMock() + tool = next( + t.fn for t in await mcp_server.list_tools() if t.name == "add_drawing_view" ) - assert full_input.scale == "1:2" - assert full_input.auto_populate_views is True + + result = await tool(input_data={"operation": "remove", "view_name": "Front"}) + + assert result["error_type"] == "invalid_input" + assert "only adds views" in result["message"] + mock_adapter.add_drawing_view.assert_not_awaited() @pytest.mark.unit def test_dimension_input_validation(self): @@ -538,11 +561,7 @@ async def test_create_technical_drawing_reports_the_real_outcome( type(mock_adapter), "create_technical_drawing", raising=False ) - input_data = DrawingCreationInput( - model_file="part.sldprt", - template="template.drwdot", - auto_populate_views=False, - ) + input_data = DrawingCreationInput(model_file="part.sldprt") tool_func = next( ( t.fn @@ -568,9 +587,7 @@ async def test_create_technical_drawing_adapter_success( execution_time=1.0, ) ) - input_data = DrawingCreationInput( - model_file="part.sldprt", auto_populate_views=True - ) + input_data = DrawingCreationInput(model_file="part.sldprt") tool_func = next( ( t.fn @@ -1082,9 +1099,7 @@ async def test_drawing_tools_refuse_without_adapter_support(mcp_server, mock_con assert ( await (await _tool(mcp_server, "create_technical_drawing"))( - input_data=DrawingCreationInput( - output_path="o.slddrw", auto_populate_views=True - ) + input_data=DrawingCreationInput() ) )["status"] == "error" assert ( diff --git a/tests/solidworks_mcp/tools/test_drawing_analysis.py b/tests/solidworks_mcp/tools/test_drawing_analysis.py index 00c726f0..23e0b240 100644 --- a/tests/solidworks_mcp/tools/test_drawing_analysis.py +++ b/tests/solidworks_mcp/tools/test_drawing_analysis.py @@ -1030,7 +1030,7 @@ async def test_drawing_and_analysis_adapter_passthrough_success( assert ( await (await _tool(mcp_server, "create_technical_drawing"))( - input_data=DrawingCreationInput(output_path="a.slddrw") + input_data=DrawingCreationInput() ) )["status"] == "success" assert ( diff --git a/tests/solidworks_mcp/tools/test_export.py b/tests/solidworks_mcp/tools/test_export.py index 1bdebab6..055469a2 100644 --- a/tests/solidworks_mcp/tools/test_export.py +++ b/tests/solidworks_mcp/tools/test_export.py @@ -8,13 +8,8 @@ from solidworks_mcp.tools.export import ( BatchExportInput, - ExportDWGInput, ExportFileInput, - ExportIGESInput, ExportImageInput, - ExportPDFInput, - ExportSTEPInput, - ExportSTLInput, register_export_tools, ) @@ -47,12 +42,7 @@ async def test_export_step_success(self, mcp_server, mock_adapter, mock_config): ) ) - input_data = ExportSTEPInput( - model_path="test_part.sldprt", - output_path="exports/test_part.step", - step_version="AP214", - units="mm", - ) + input_data = ExportFileInput(file_path="exports/test_part.step") tool_func = None for tool in await mcp_server.list_tools(): @@ -87,12 +77,7 @@ async def test_export_iges_success(self, mcp_server, mock_adapter, mock_config): ) ) - input_data = ExportIGESInput( - model_path="test_part.sldprt", - output_path="exports/test_part.iges", - iges_version="2014", - surface_type="NURBS", - ) + input_data = ExportFileInput(file_path="exports/test_part.iges") tool_func = None for tool in await mcp_server.list_tools(): @@ -127,13 +112,7 @@ async def test_export_stl_success(self, mcp_server, mock_adapter, mock_config): ) ) - input_data = ExportSTLInput( - model_path="test_part.sldprt", - output_path="exports/test_part.stl", - resolution="High", - file_format="Binary", - units="mm", - ) + input_data = ExportFileInput(file_path="exports/test_part.stl") tool_func = None for tool in await mcp_server.list_tools(): @@ -168,12 +147,7 @@ async def test_export_pdf_success(self, mcp_server, mock_adapter, mock_config): ) ) - input_data = ExportPDFInput( - drawing_path="test_drawing.slddrw", - output_path="exports/test_drawing.pdf", - quality="High", - color_mode="Color", - ) + input_data = ExportFileInput(file_path="exports/test_drawing.pdf") tool_func = None for tool in await mcp_server.list_tools(): @@ -208,12 +182,7 @@ async def test_export_dwg_success(self, mcp_server, mock_adapter, mock_config): ) ) - input_data = ExportDWGInput( - drawing_path="test_drawing.slddrw", - output_path="exports/test_drawing.dwg", - autocad_version="2020", - layer_mapping=True, - ) + input_data = ExportFileInput(file_path="exports/test_drawing.dwg") tool_func = None for tool in await mcp_server.list_tools(): @@ -352,9 +321,7 @@ async def test_export_error_handling(self, mcp_server, mock_adapter, mock_config ) ) - input_data = ExportSTEPInput( - model_path="test_part.sldprt", output_path="/invalid_path/test_part.step" - ) + input_data = ExportFileInput(file_path="/invalid_path/test_part.step") tool_func = None for tool in await mcp_server.list_tools(): @@ -366,24 +333,6 @@ async def test_export_error_handling(self, mcp_server, mock_adapter, mock_config assert result["status"] == "error" assert "Output directory does not exist" in result["message"] - @pytest.mark.unit - def test_export_step_input_validation(self): - """Test input validation for STEP export.""" - # Valid input - valid_input = ExportSTEPInput(model_path="test.sldprt", output_path="test.step") - assert valid_input.model_path == "test.sldprt" - assert valid_input.output_path == "test.step" - - # Test optional parameters - full_input = ExportSTEPInput( - model_path="test.sldprt", - output_path="test.step", - step_version="AP214", - units="mm", - ) - assert full_input.step_version == "AP214" - assert full_input.units == "mm" - @pytest.mark.unit def test_batch_export_input_validation(self): """Test input validation for batch export.""" @@ -441,17 +390,11 @@ async def test_remaining_export_error_paths( by_name = {t.name: t.fn for t in await mcp_server.list_tools()} r1 = await by_name["export_iges"]( - input_data=ExportIGESInput(model_path="a.sldprt", output_path="a.iges") - ) - r2 = await by_name["export_stl"]( - input_data=ExportSTLInput(model_path="a.sldprt", output_path="a.stl") - ) - r3 = await by_name["export_pdf"]( - input_data=ExportPDFInput(model_path="a.slddrw", output_path="a.pdf") - ) - r4 = await by_name["export_dwg"]( - input_data=ExportDWGInput(model_path="a.slddrw", output_path="a.dwg") + input_data=ExportFileInput(file_path="a.iges") ) + r2 = await by_name["export_stl"](input_data=ExportFileInput(file_path="a.stl")) + r3 = await by_name["export_pdf"](input_data=ExportFileInput(file_path="a.pdf")) + r4 = await by_name["export_dwg"](input_data=ExportFileInput(file_path="a.dwg")) r5 = await by_name["export_image"]( input_data=ExportImageInput(model_path="a.sldprt", output_path="a.png") ) @@ -492,18 +435,10 @@ async def test_remaining_export_exception_paths( by_name = {t.name: t.fn for t in await mcp_server.list_tools()} results = [ - await by_name["export_iges"]( - ExportIGESInput(model_path="a.sldprt", output_path="a.iges") - ), - await by_name["export_stl"]( - ExportSTLInput(model_path="a.sldprt", output_path="a.stl") - ), - await by_name["export_pdf"]( - ExportPDFInput(model_path="a.slddrw", output_path="a.pdf") - ), - await by_name["export_dwg"]( - ExportDWGInput(model_path="a.slddrw", output_path="a.dwg") - ), + await by_name["export_iges"](ExportFileInput(file_path="a.iges")), + await by_name["export_stl"](ExportFileInput(file_path="a.stl")), + await by_name["export_pdf"](ExportFileInput(file_path="a.pdf")), + await by_name["export_dwg"](ExportFileInput(file_path="a.dwg")), await by_name["export_image"]( ExportImageInput(model_path="a.sldprt", output_path="a.png") ), @@ -531,9 +466,7 @@ async def test_export_step_exception_path( tool = next( t.fn for t in await mcp_server.list_tools() if t.name == "export_step" ) - result = await tool( - ExportSTEPInput(model_path="test_part.sldprt", output_path="out.step") - ) + result = await tool(ExportFileInput(file_path="out.step")) assert result["status"] == "error" assert result["error_type"] == "internal" diff --git a/tests/solidworks_mcp/tools/test_file_management.py b/tests/solidworks_mcp/tools/test_file_management.py index 3014c472..5bf51273 100644 --- a/tests/solidworks_mcp/tools/test_file_management.py +++ b/tests/solidworks_mcp/tools/test_file_management.py @@ -450,7 +450,7 @@ async def test_save_file_exception_path( break assert save_tool is not None - result = await save_tool(input_data={"force_save": True}) + result = await save_tool(input_data={}) assert result["status"] == "error" assert result["error_type"] == "internal" @@ -477,7 +477,7 @@ async def test_save_file_success_with_adapter_path( break assert save_tool is not None - result = await save_tool(input_data={"force_save": True}) + result = await save_tool(input_data={}) assert result["status"] == "success" assert result["data"]["saved"] is True @@ -495,7 +495,7 @@ async def test_save_file_fallback_without_adapter_method( save_tool = tool.fn break - fallback_result = await save_tool(input_data={"force_save": False}) + fallback_result = await save_tool(input_data={}) assert fallback_result["status"] == "error" assert "does not support save_file" in fallback_result["message"] @@ -516,7 +516,7 @@ async def test_save_file_adapter_error_path( save_tool = tool.fn break - result = await save_tool(input_data={"force_save": True}) + result = await save_tool(input_data={}) assert result["status"] == "error" assert "write failed" in result["message"] @@ -2278,3 +2278,38 @@ def resolve(self, strict=False): ) assert str(target_dir / "bracket_1.sldprt") in result["copied_files"] + + +@pytest.mark.asyncio +async def test_save_as_keeps_an_existing_file_unless_overwrite( + mcp_server, mock_adapter, mock_config, tmp_path: Path +) -> None: + save_as = await _tool(mcp_server, mock_adapter, mock_config, "save_as") + mock_adapter.save_file = AsyncMock( + return_value=Mock(is_success=True, data=None, execution_time=0.1) + ) + target = tmp_path / "part.sldprt" + target.write_bytes(b"original") + + kept = await save_as(input_data={"file_path": str(target)}) + replaced = await save_as(input_data={"file_path": str(target), "overwrite": True}) + + assert kept["error_type"] == "invalid_input" + assert "left untouched" in kept["message"] and "overwrite" in kept["message"] + assert target.read_bytes() == b"original" + assert replaced["status"] == "success" + mock_adapter.save_file.assert_awaited_once_with(str(target)) + + +@pytest.mark.asyncio +async def test_save_file_refuses_force_save( + mcp_server, mock_adapter, mock_config +) -> None: + save_file = await _tool(mcp_server, mock_adapter, mock_config, "save_file") + mock_adapter.save_file = AsyncMock() + + result = await save_file(input_data={"force_save": True}) + + assert result["error_type"] == "invalid_input" + assert "the tool always saves" in result["message"] + mock_adapter.save_file.assert_not_awaited() diff --git a/tests/solidworks_mcp/tools/test_input_compat.py b/tests/solidworks_mcp/tools/test_input_compat.py new file mode 100644 index 00000000..ea3f6efd --- /dev/null +++ b/tests/solidworks_mcp/tools/test_input_compat.py @@ -0,0 +1,47 @@ +"""Tests for the shared input base and the removed-inputs table.""" + +from __future__ import annotations + +from solidworks_mcp.tools.input_compat import ( + REMOVED_INPUTS, + CompatInput, + removed_inputs, +) + + +class _Thing(CompatInput): + name: str + + +def test_removed_inputs_name_each_field_sent_with_its_pointer(monkeypatch) -> None: + monkeypatch.setitem(REMOVED_INPUTS, "_Thing", {"old": "use `new`", "gone": "x"}) + + sent = _Thing(name="a", old=1, other=2) + + assert removed_inputs(sent) == ["`old` is no longer an input: use `new`"] + assert removed_inputs(_Thing(name="a")) == [] + + +def test_a_model_without_removed_inputs_is_never_refused() -> None: + class _Free(CompatInput): + pass + + assert removed_inputs(_Free(anything=1)) == [] + + +def test_every_removed_field_is_really_gone_from_its_model() -> None: + """A field still declared would never reach model_extra, so never be refused.""" + from importlib import import_module + from pkgutil import iter_modules + + import solidworks_mcp.tools as tools_pkg + + models: dict[str, type] = {} + for info in iter_modules(tools_pkg.__path__): + module = import_module(f"solidworks_mcp.tools.{info.name}") + for value in vars(module).values(): + if isinstance(value, type) and issubclass(value, CompatInput): + models[value.__name__] = value + for model_name, fields in REMOVED_INPUTS.items(): + assert model_name in models, model_name + assert not set(fields) & set(models[model_name].model_fields), model_name diff --git a/tests/solidworks_mcp/tools/test_macro_recording.py b/tests/solidworks_mcp/tools/test_macro_recording.py index 76794feb..e888395a 100644 --- a/tests/solidworks_mcp/tools/test_macro_recording.py +++ b/tests/solidworks_mcp/tools/test_macro_recording.py @@ -162,14 +162,7 @@ async def test_execute_macro_success(self, mcp_server, mock_adapter, mock_config ) ) - input_data = MacroPlaybackInput( - macro_file="batch_process.swp", - target_file="input_part.sldprt", - execution_mode="Normal", - pause_on_error=False, - log_execution=True, - execution_parameters={"batch_size": 10}, - ) + input_data = MacroPlaybackInput(macro_file="batch_process.swp") tool_func = None for tool in await mcp_server.list_tools(): @@ -184,6 +177,8 @@ async def test_execute_macro_success(self, mcp_server, mock_adapter, mock_config assert result["data"]["execution_status"] == "Completed" assert result["data"]["operations_performed"] == 25 assert result["data"]["errors_encountered"] == 0 + assert result["runs_completed"] == 1 + assert result["message"] == "Macro completed 1 run" mock_adapter.execute_macro.assert_called_once() @pytest.mark.asyncio @@ -613,7 +608,9 @@ async def test_execute_macro_adapter_error_path( await register_macro_recording_tools(mcp_server, mock_adapter, mock_config) mock_adapter.execute_macro = AsyncMock( - return_value=Mock(is_success=False, error="macro syntax error") + return_value=Mock( + is_success=False, error="macro syntax error", execution_time=0.1 + ) ) execute_tool = None @@ -628,6 +625,7 @@ async def test_execute_macro_adapter_error_path( ) assert result["status"] == "error" assert "macro syntax error" in result["message"] + assert "Run 1 of 1 failed after 0 completed" in result["message"] @pytest.mark.asyncio async def test_execute_macro_fallback_refuses_without_running_anything( @@ -991,3 +989,69 @@ async def test_execute_macro_rejects_vb_before_reaching_the_adapter( assert result["error_type"] == "invalid_input" assert ".swp" in result["message"] mock_adapter.execute_macro.assert_not_called() + + +@pytest.mark.asyncio +async def test_execute_macro_repeats_and_pauses(mcp_server, mock_adapter, monkeypatch): + """repeat_count runs the macro that many times, pausing between runs.""" + await register_macro_recording_tools(mcp_server, mock_adapter, {}) + mock_adapter.execute_macro = AsyncMock( + return_value=Mock(is_success=True, data={"errors": 0}, execution_time=0.5) + ) + pauses: list[float] = [] + + async def _sleep(seconds: float) -> None: + pauses.append(seconds) + + monkeypatch.setattr("solidworks_mcp.tools.macro_recording.asyncio.sleep", _sleep) + tool = next( + t.fn for t in await mcp_server.list_tools() if t.name == "execute_macro" + ) + + result = await tool( + input_data={"macro_path": "m.swp", "repeat_count": 3, "pause_between_runs": 2} + ) + + assert result["runs_completed"] == 3 + assert result["message"] == "Macro completed 3 runs" + assert result["execution_time"] == 1.5 + assert mock_adapter.execute_macro.await_count == 3 + assert pauses == [2, 2] + + +@pytest.mark.asyncio +async def test_execute_macro_stops_at_the_first_failed_run(mcp_server, mock_adapter): + """The second of three runs fails: no third run, and the count is reported.""" + await register_macro_recording_tools(mcp_server, mock_adapter, {}) + ok = Mock(is_success=True, data={}, execution_time=0.5) + bad = Mock(is_success=False, error="boom", data=None, execution_time=0.25) + mock_adapter.execute_macro = AsyncMock(side_effect=[ok, bad, ok]) + tool = next( + t.fn for t in await mcp_server.list_tools() if t.name == "execute_macro" + ) + + result = await tool(input_data={"macro_path": "m.swp", "repeat_count": 3}) + + assert result["status"] == "error" + assert "Run 2 of 3 failed after 1 completed: boom" in result["message"] + assert result["execution_time"] == 0.75 + assert mock_adapter.execute_macro.await_count == 2 + + +@pytest.mark.asyncio +async def test_execute_macro_refuses_removed_inputs_and_bad_counts( + mcp_server, mock_adapter +): + await register_macro_recording_tools(mcp_server, mock_adapter, {}) + mock_adapter.execute_macro = AsyncMock() + tool = next( + t.fn for t in await mcp_server.list_tools() if t.name == "execute_macro" + ) + + removed = await tool(input_data={"macro_path": "m.swp", "target_file": "a.sldprt"}) + too_many = await tool(input_data={"macro_path": "m.swp", "repeat_count": 101}) + + assert removed["error_type"] == "invalid_input" + assert "open_model" in removed["message"] + assert too_many["error_type"] == "invalid_input" + mock_adapter.execute_macro.assert_not_awaited() diff --git a/tests/solidworks_mcp/tools/test_modeling.py b/tests/solidworks_mcp/tools/test_modeling.py index cfc7e3cf..28feed8d 100644 --- a/tests/solidworks_mcp/tools/test_modeling.py +++ b/tests/solidworks_mcp/tools/test_modeling.py @@ -140,6 +140,9 @@ async def test_create_part_success(self, mcp_server, mock_adapter, mock_config): execution_time=1.2, ) ) + mock_adapter.set_material = AsyncMock( + return_value=Mock(is_success=True, data={"material": "Steel"}) + ) tool_func = None for tool in await mcp_server.list_tools(): @@ -156,6 +159,8 @@ async def test_create_part_success(self, mcp_server, mock_adapter, mock_config): assert result["part"]["name"] == "new_part" assert result["part"]["units"] == "mm" assert result["part"]["material"] == "Steel" + assert "could not be assigned" not in result.get("warning", "") + mock_adapter.set_material.assert_awaited_once_with("Steel") # The template must reach the adapter, not just be echoed back. mock_adapter.create_part.assert_awaited_once_with( "new_part", "mm", "standard_part" @@ -181,19 +186,28 @@ async def test_create_assembly_with_components( break input_data = CreateAssemblyInput( - name="test_assembly", - template="standard_assembly", - components=["part1.sldprt", "part2.sldprt"], + name="test_assembly", template="standard_assembly" ) result = await tool_func(input_data) assert result["status"] == "success" - assert result["assembly"]["name"] == "test_assembly" - assert len(result["assembly"]["components"]) == 2 + assert result["assembly"] == { + "name": "test_assembly", + "template": "standard_assembly", + } mock_adapter.create_assembly.assert_awaited_once_with( "test_assembly", "standard_assembly" ) + refused = await tool_func( + {"name": "x", "components": ["part1.sldprt", "part2.sldprt"]} + ) + assert refused["status"] == "error" + assert refused["error_type"] == "invalid_input" + assert "`components`" in refused["message"] + assert "insert_component" in refused["message"] + assert mock_adapter.create_assembly.await_count == 1 + @pytest.mark.asyncio async def test_create_drawing_from_model( self, mcp_server, mock_adapter, mock_config @@ -216,21 +230,26 @@ async def test_create_drawing_from_model( break input_data = CreateDrawingInput( - name="test_drawing", - model_path="test_part.sldprt", - sheet_format="A3", - template="standard_drawing", + name="test_drawing", template="standard_drawing" ) result = await tool_func(input_data) assert result["status"] == "success" - assert result["drawing"]["name"] == "test_drawing" - assert result["drawing"]["model_path"] == "test_part.sldprt" - assert result["drawing"]["sheet_format"] == "A3" + assert result["drawing"] == { + "name": "test_drawing", + "template": "standard_drawing", + } mock_adapter.create_drawing.assert_awaited_once_with( "test_drawing", "standard_drawing" ) + refused = await tool_func( + {"name": "x", "model_path": "part.sldprt", "sheet_format": "A3"} + ) + assert refused["error_type"] == "invalid_input" + assert "`model_path`" in refused["message"] + assert "`sheet_format`" in refused["message"] + @pytest.mark.asyncio async def test_create_extrusion_blind(self, mcp_server, mock_adapter, mock_config): """Test blind extrusion creation.""" @@ -251,12 +270,14 @@ async def test_create_extrusion_blind(self, mcp_server, mock_adapter, mock_confi break input_data = CreateExtrusionInput( - sketch_name="Sketch1", depth=25.0, direction="blind", reverse=False + sketch_name="Sketch1", depth=25.0, reverse=False ) result = await tool_func(input_data) assert result["status"] == "success" - assert result["extrusion"]["sketch"] == "Sketch1" + # The adapter is told which sketch; the reply names the one it used. + assert mock_adapter.create_extrusion.call_args.args[0].sketch_name == "Sketch1" + assert result["extrusion"]["sketch"] is None assert result["extrusion"]["depth"] == 25.0 assert result["extrusion"]["end_condition"] == "Blind" assert "direction" not in result["extrusion"] @@ -279,16 +300,19 @@ async def test_create_revolve_full(self, mcp_server, mock_adapter, mock_config): break input_data = CreateRevolveInput( - sketch_name="Sketch1", - axis_entity="centerline", - angle=360.0, - direction="one_direction", + sketch_name="Sketch1", axis_entity="Axis1", angle=360.0 ) result = await tool_func(input_data) assert result["status"] == "success" assert result["revolve"]["angle"] == 360.0 - assert result["revolve"]["direction"] == "one_direction" + assert "direction" not in result["revolve"] + params = mock_adapter.create_revolve.call_args.args[0] + assert (params.sketch_name, params.axis_entity) == ("Sketch1", "Axis1") + + refused = await tool_func({"angle": 360.0, "direction": "one_direction"}) + assert refused["error_type"] == "invalid_input" + assert "both_directions" in refused["message"] @pytest.mark.asyncio async def test_create_sweep_success(self, mcp_server, mock_adapter, mock_config): @@ -1038,6 +1062,35 @@ async def test_delete_feature_success(self, mcp_server, mock_adapter, mock_confi assert name in result["message"] assert result["data"]["deleted"] == name assert "execution_time" in result + assert "warning" not in result # nothing else went + + @pytest.mark.asyncio + async def test_deleting_a_sketch_warns_that_its_cut_went_too( + self, mcp_server, mock_adapter, mock_config + ): + """More than the named feature went: the reply warns, naming them.""" + await register_modeling_tools(mcp_server, mock_adapter, mock_config) + mock_adapter.delete_feature = AsyncMock( + return_value=Mock( + is_success=True, + data={ + "deleted": "Sketch2", + "removed_features": ["Cut-Extrude1", "Sketch2"], + }, + execution_time=0.1, + ) + ) + tool_func = next( + t.fn for t in await mcp_server.list_tools() if t.name == "delete_feature" + ) + + result = await tool_func(DeleteFeatureInput(name="Sketch2")) + + assert result["message"] == "Deleted Sketch2 and its dependents: Cut-Extrude1" + assert result["warning"] == ( + "Deleting Sketch2 also deleted Cut-Extrude1. Tell the user; `undo` " + "restores them." + ) @pytest.mark.asyncio async def test_delete_feature_error_when_adapter_lacks_capability( @@ -1892,12 +1945,11 @@ def test_create_extrusion_input_depth_zero_raises(self): with pytest.raises(ValueError, match="depth must be positive"): CreateExtrusionInput(sketch_name="Sketch1", depth=-1.0) - def test_create_extrusion_input_empty_sketch_name_raises(self): - """Line 136 — empty sketch_name raises ValueError.""" + def test_create_extrusion_input_sketch_name_is_optional(self): + """No sketch_name means the sketch exit_sketch closed last.""" from solidworks_mcp.tools.modeling import CreateExtrusionInput - with pytest.raises(ValueError, match="sketch_name is required"): - CreateExtrusionInput(sketch_name=" ", depth=5.0) + assert CreateExtrusionInput(depth=5.0).sketch_name is None @pytest.mark.asyncio async def test_close_model_exception(self): @@ -2076,9 +2128,7 @@ async def test_extrusion_reply_says_mid_plane_for_both_directions( t.fn for t in await mcp_server.list_tools() if t.name == "create_extrusion" ) - result = await tool( - {"sketch_name": "Sketch1", "depth": 38.1, "both_directions": True} - ) + result = await tool({"depth": 38.1, "both_directions": True}) assert result["extrusion"]["end_condition"] == "MidPlane" assert result["extrusion"]["both_directions"] is True @@ -2095,7 +2145,6 @@ async def test_a_thin_extrusion_reports_its_wall(mcp_server, mock_adapter) -> No result = await tool( { - "sketch_name": "Sketch1", "depth": 38.1, "thin_feature": True, "thin_thickness": 6.35, @@ -2260,3 +2309,70 @@ async def test_the_mock_refuses_an_empty_state_change(mock_adapter) -> None: result = await mock_adapter.set_component_state("crank-1") assert result.is_error + + +@pytest.mark.asyncio +async def test_create_part_with_an_unknown_material_still_creates_the_part( + mcp_server, mock_adapter +) -> None: + await register_modeling_tools(mcp_server, mock_adapter, {}) + tool = next(t.fn for t in await mcp_server.list_tools() if t.name == "create_part") + + result = await tool({"name": "p", "material": "Unobtainium"}) + + assert result["status"] == "success" + assert result["part"]["material"] is None + assert "'Unobtainium' could not be assigned" in result["warning"] + assert (await mock_adapter.get_material()).data["material"] is None + + +@pytest.mark.asyncio +async def test_create_part_assigns_a_known_material(mcp_server, mock_adapter) -> None: + await register_modeling_tools(mcp_server, mock_adapter, {}) + tool = next(t.fn for t in await mcp_server.list_tools() if t.name == "create_part") + + result = await tool({"name": "p", "material": "AISI 304"}) + + assert result["part"]["material"] == "AISI 304" + material = (await mock_adapter.get_material()).data + assert (material["material"], material["density_kg_m3"]) == ("AISI 304", 8000.0) + + +@pytest.mark.asyncio +async def test_extrusion_names_the_sketch_it_used(mcp_server, mock_adapter) -> None: + await mock_adapter.create_part() + await mock_adapter.create_sketch("Front") + await mock_adapter.exit_sketch() + await register_modeling_tools(mcp_server, mock_adapter, {}) + tool = next( + t.fn for t in await mcp_server.list_tools() if t.name == "create_extrusion" + ) + + used = await tool({"sketch_name": "sketch1", "depth": 5.0}) + missing = await tool({"sketch_name": "Sketch9", "depth": 5.0}) + + assert used["extrusion"]["sketch"] == "Sketch1" + assert missing["status"] == "error" + assert "Sketch9" in missing["message"] and "Sketch1" in missing["message"] + + +@pytest.mark.asyncio +async def test_revolve_about_a_named_axis(mcp_server, mock_adapter) -> None: + await mock_adapter.create_part() + await mock_adapter.create_sketch("Front") + await mock_adapter.exit_sketch() + axis = (await mock_adapter.create_axis("y")).data["name"] + await register_modeling_tools(mcp_server, mock_adapter, {}) + tool = next( + t.fn for t in await mcp_server.list_tools() if t.name == "create_revolve" + ) + + used = await tool({"sketch_name": "Sketch1", "axis_entity": axis, "angle": 360}) + missing = await tool({"axis_entity": "Axis9", "angle": 360}) + + assert (used["revolve"]["sketch"], used["revolve"]["axis_entity"]) == ( + "Sketch1", + axis, + ) + assert missing["status"] == "error" + assert "create_axis" in missing["message"] diff --git a/tests/solidworks_mcp/tools/test_modeling_measured_change.py b/tests/solidworks_mcp/tools/test_modeling_measured_change.py index bcde5759..7889d69e 100644 --- a/tests/solidworks_mcp/tools/test_modeling_measured_change.py +++ b/tests/solidworks_mcp/tools/test_modeling_measured_change.py @@ -96,14 +96,7 @@ class _BareAdapter: _CALLS["create_extrusion"] ) - # create_extrusion is beta, so the boundary adds its level warning. - assert set(result) == { - "status", - "message", - "extrusion", - "execution_time", - "warning", - } + assert set(result) == {"status", "message", "extrusion", "execution_time"} @pytest.mark.asyncio diff --git a/tests/solidworks_mcp/tools/test_sketching.py b/tests/solidworks_mcp/tools/test_sketching.py index 8c24ba14..e707cd17 100644 --- a/tests/solidworks_mcp/tools/test_sketching.py +++ b/tests/solidworks_mcp/tools/test_sketching.py @@ -63,11 +63,15 @@ async def test_create_sketch_on_plane(self, mcp_server, mock_adapter, mock_confi assert tool_func is not None + mock_adapter.rename_feature = AsyncMock() input_data = CreateSketchInput(plane="Front Plane", sketch_name="TestSketch") result = await tool_func(input_data) assert result["status"] == "success" - assert result["sketch"]["name"] == "TestSketch" + # SolidWorks won't rename an open sketch: exit_sketch names it. + assert result["sketch"]["name"] == "Sketch1" + assert result["sketch"]["name_on_exit"] == "TestSketch" + mock_adapter.rename_feature.assert_not_awaited() assert result["sketch"]["plane"] == "Front Plane" assert result["execution_time"] == 0.3 @@ -898,6 +902,10 @@ async def create_sketch(self, _plane): """Test helper for create sketch.""" return _ok({"sketch_name": "Sketch1"}) + async def rename_feature(self, _old, new): + """Test helper for rename feature.""" + return _ok({"new_name": new}) + async def add_sketch_line(self, *_args): """Test helper for add sketch line.""" return _ok({"entity_id": "Line1"}) @@ -1359,3 +1367,65 @@ async def test_add_sketch_constraint_accepts_plain_arguments(): assert result["status"] == "success", result assert result["constraint"]["type"] == "equal" + + +@pytest.mark.asyncio +async def test_exit_sketch_gives_the_name_create_sketch_was_asked_for( + mcp_server, mock_adapter, mock_config +): + """Named on close; a taken name keeps the tree's name and says why.""" + await register_sketching_tools(mcp_server, mock_adapter, mock_config) + await mock_adapter.create_part() + create = await _tool(mcp_server, "create_sketch") + exit_ = await _tool(mcp_server, "exit_sketch") + + await create(input_data={"plane": "Front", "sketch_name": "Profile"}) + first = await exit_() + await create(input_data={"plane": "Top", "sketch_name": "Profile"}) + second = await exit_() + await create(input_data={"plane": "Right"}) + unnamed = await exit_() + + assert first["sketch"] == {"name": "Profile"} + assert "warning" not in first + assert second["sketch"] == {"name": "Sketch2"} + assert "could not be named 'Profile'" in second["warning"] + assert "sketch" not in unnamed + assert list(mock_adapter._sketches) == ["Profile", "Sketch2", "Sketch3"] + + +@pytest.mark.asyncio +async def test_a_sketch_left_open_does_not_pass_its_name_on( + mcp_server, mock_adapter, mock_config +): + """A new sketch drops the name an abandoned open sketch was waiting for.""" + await register_sketching_tools(mcp_server, mock_adapter, mock_config) + await mock_adapter.create_part() + create = await _tool(mcp_server, "create_sketch") + + await create(input_data={"plane": "Front", "sketch_name": "Profile"}) + await mock_adapter.exit_sketch() # closed outside the tool: no rename + await create(input_data={"plane": "Top"}) + closed = await (await _tool(mcp_server, "exit_sketch"))() + + assert "sketch" not in closed + assert list(mock_adapter._sketches) == ["Sketch1", "Sketch2"] + + +@pytest.mark.asyncio +async def test_the_hole_tutorial_points_at_the_cut_tool( + mcp_server, mock_adapter, mock_config +): + """Its next step is create_cut_extrude, never a negative-depth extrusion.""" + await register_sketching_tools(mcp_server, mock_adapter, mock_config) + await mock_adapter.create_part() + tool = await _tool(mcp_server, "sketch_tutorial_simple_hole") + + result = await tool() + + assert "create_cut_extrude" in result["next_step"] + assert "suggested_extrusion" not in result + cut = result["suggested_cut"] + assert cut["sketch_name"] == "Sketch1" + assert cut["end_condition"] == "ThroughAll" + assert cut["depth"] > 0 diff --git a/tests/solidworks_mcp/tools/test_tool_boundary.py b/tests/solidworks_mcp/tools/test_tool_boundary.py index bf7db27c..c67089d0 100644 --- a/tests/solidworks_mcp/tools/test_tool_boundary.py +++ b/tests/solidworks_mcp/tools/test_tool_boundary.py @@ -11,6 +11,7 @@ from solidworks_mcp.adapters.base import AdapterResult, AdapterResultStatus from solidworks_mcp.tools.error_classifier import RPC_S_SERVER_UNAVAILABLE +from solidworks_mcp.tools.modeling import CreateAssemblyInput from solidworks_mcp.tools.support_levels import Support, SupportLevel from solidworks_mcp.tools.tool_boundary import ( InvalidInputError, @@ -543,3 +544,26 @@ async def body(input_data: NoSuchModel) -> dict[str, Any]: # noqa: F821 return {} assert model_parameters(body) == {} + + +@pytest.mark.asyncio +async def test_a_removed_input_is_refused_before_the_tool_runs() -> None: + """Model or dict, a removed field is invalid input and the body never runs.""" + ran: list[Any] = [] + + async def body(input_data: CreateAssemblyInput) -> dict[str, Any]: + ran.append(input_data) + return {"status": "success"} + + wrapped = tool_boundary(body) + from_dict = await wrapped({"name": "a", "components": ["p.sldprt"]}) + from_model = await wrapped( + input_data=CreateAssemblyInput(name="a", components=["p.sldprt"]) + ) + kept = await wrapped({"name": "a", "colour": "red"}) # unknown extras pass + + for result in (from_dict, from_model): + assert result["error_type"] == "invalid_input" + assert "`components` is no longer an input" in result["message"] + assert kept == {"status": "success"} + assert len(ran) == 1 diff --git a/tests/solidworks_mcp/tools/test_tool_hints.py b/tests/solidworks_mcp/tools/test_tool_hints.py index 783b9554..625918c1 100644 --- a/tests/solidworks_mcp/tools/test_tool_hints.py +++ b/tests/solidworks_mcp/tools/test_tool_hints.py @@ -41,7 +41,7 @@ async def test_spot_checks(mock_adapter) -> None: assert tools["list_features"].annotations.readOnlyHint is True assert tools["delete_feature"].annotations.destructiveHint is True assert tools["create_extrusion"].annotations.readOnlyHint is False - assert tools["create_extrusion"].tags == {"modeling", "level:beta"} + assert tools["create_extrusion"].tags == {"modeling", "level:stable"} assert tools["save_file"].annotations.destructiveHint is True