diff --git a/skills/solidcraft-checking/SKILL.md b/skills/solidcraft-checking/SKILL.md index 2ee521ccbd..304e8bd8e8 100644 --- a/skills/solidcraft-checking/SKILL.md +++ b/skills/solidcraft-checking/SKILL.md @@ -30,10 +30,10 @@ In `$SKILL/scripts/`: ## Traps -1. **Zero dimension.** A sketch dimension driven to 0 loses its side. On the next parameter change it either fails with `its constraints would conflict` (a point placed by dimensions from the origin) or, with no error, puts the point on either side (a slot that other constraints also held came back mirrored). Check every distance expression against the whole allowed range of each parameter in it, counts included; the walk in step 3 catches the failure, and only the volume or the position catches the mirror. For a point on an axis, pass `x=None` or `y=None` to `place`: it aligns the point instead of dimensioning it. For a position that does reach 0, such as the first of a centred row of one, dimension it from a sketch point placed outside the part: a point at x = -100 mm, then the distance `100 mm - (n - 1) / 2 * pitch` from it. +1. **Zero dimension.** A sketch dimension driven to 0 loses its side: the point it measures has no direction left, so the solver may leave it anywhere on the axis. On the next parameter change it either fails with `its constraints would conflict` (a point placed by dimensions from the origin) or, with no error, moves it: a slot that other constraints also held slides to the origin when the distance reaches 0 and, the solver taking the shortest step that fits, keeps the side it was drawn on when the value comes back. Check every distance expression against the whole allowed range of each parameter in it, counts included; the walk in step 3 catches the failure, and only the volume or the position catches a point that ended up on the other side. For a point on an axis, pass `x=None` or `y=None` to `place`: it aligns the point instead of dimensioning it. For a position that does reach 0, such as the first of a centred row of one, dimension it from a sketch point placed outside the part: a point at x = -100 mm, then the distance `100 mm - (n - 1) / 2 * pitch` from it. 2. **Counts.** Solid patterns take counts 1..1000; a count that reaches 0 fails the rebuild. A fractional count rounds to the nearest whole number without an error (6.5 gives 7 copies), while every other expression uses the exact value, so a centred row shifts. Nothing in SolidCraft enforces a parameter range during a parameter change (configuration rules apply only when a configuration row is activated): choose a range that keeps every count at 1 or more, write "whole number" and the range in the comment, and use `round(n)` wherever the count also positions something. 3. **Circumscribed polygons.** `sketch.polygon.circumscribed` with a size dimension and one vertical side still has 1 dof: its corners can alternate in and out. `sketch.constraint.polygon` makes it regular; `scb.hexagon` adds it. -4. **Boolean retries.** A `Boolean retry: ...` warning means the result may be wrong (see "Boolean warning" in the `solidcraft-modeling` reference), so check the volume of that state. `temporary coincident-face separation` comes from faces that coincide. In a plain block, a tapered tool whose start or end lies on a face logs it and a straight one does not; on the floor of a filleted, shelled box, a straight cylinder joined onto the floor logs it too, and sunk 1 mm into the floor it does not. Starting cutters about 1 mm past the faces they pierce (sizes extrapolated along any taper) and sinking joins about 1 mm into the body remove the warning on a single feature. They did not fix standoffs built as a joined cylinder and a cut pilot hole, patterned together: those still kept material in the holes, and one ring profile fixed it ("Boolean warning" in the `solidcraft-modeling` reference). Check the volume of every state either way. +4. **Boolean retries.** A `Boolean retry: ...` warning means the result may be wrong (see "Boolean warning" in the `solidcraft-modeling` reference), so check the volume of that state. `temporary coincident-face separation` comes from faces that coincide. A tapered tool whose start or end lies on a face of a plain block, or a straight cylinder joined onto the floor of a filleted, shelled box, does not log it; a `tool displacement` warning shows up when a join and a cut share one location, as in a standoff built as a joined cylinder with a cut pilot hole, patterned together. That case reports the warning but its volume agrees with the analytic growth, and a single ring profile removes the warning. Starting cutters about 1 mm past the faces they pierce and sinking joins about 1 mm into the body remain good practice on a single feature. Check the volume of every state either way. 5. **Measurement accuracy.** The `solidcraft-modeling` reference gives how far measured volumes stray from computed ones. Centre of mass and bounding box come out up to about 5e-5 of the part size off. Set test tolerances as the template says. 6. **Expressions.** They have `+ - * / ^ %`, `sin cos tan asin acos atan atan2 sqrt abs min max floor ceil round mod trunc exp ln log log10 pow sign hypot` (`log` is base 10), `pi` and units; no `if` and no comparisons. Build steps from `floor`, `mod`, `min`, `max` and `sign`, which gives 1 at 0. `asin acos atan atan2` return angles, so a unitless parameter cannot hold them. 7. **Snapping.** A point created exactly on an existing point or the origin becomes coincident with it (`connect` defaults to true). diff --git a/skills/solidcraft-checking/scripts/tests/test_facts.py b/skills/solidcraft-checking/scripts/tests/test_facts.py index 6cc313adde..cbe7da64da 100644 --- a/skills/solidcraft-checking/scripts/tests/test_facts.py +++ b/skills/solidcraft-checking/scripts/tests/test_facts.py @@ -75,7 +75,8 @@ def test_a_dimension_driven_to_zero_can_break_the_next_change(): ]) -def test_a_slot_held_by_other_constraints_can_cross_to_the_other_side_at_zero(): +def slot_centre(*changes: list) -> float: + """The slot's centre on x after the given parameter changes.""" body = execute([ cmd("parameters.add", name="n", expression="3", unit=""), cmd("sketch.create", plane="XY", name="Slot sketch"), @@ -86,11 +87,19 @@ def test_a_slot_held_by_other_constraints_can_cross_to_the_other_side_at_zero(): cmd("sketch.dimension", entities=["origin", "a1.center"], type="horizontal", value="(n - 1) * 3 mm"), cmd("sketch.finish"), cmd("solid.extrude", sketch="Slot sketch", distance="1 mm", body_name="Slot"), - cmd("parameters.change", name="n", expression="1"), - cmd("parameters.change", name="n", expression="3"), + *changes, ])["bodies"][0] - # Started at x = -6; after n 3 -> 1 -> 3 it sits at +6, with no error - assert (body["bbox"]["min"][0] + body["bbox"]["max"][0]) / 2 == pytest.approx(6, abs=1e-3) + return (body["bbox"]["min"][0] + body["bbox"]["max"][0]) / 2 + + +def test_a_slot_held_by_other_constraints_keeps_its_side_at_zero(): + # Drawn at x = -6. At n = 1 the distance is 0, so the slot slides to the origin; when the + # value comes back the solver takes the shortest step that fits and the slot keeps the side + # it was drawn on, so it no longer comes back mirrored. + assert slot_centre() == pytest.approx(-6, abs=1e-3) + assert slot_centre(cmd("parameters.change", name="n", expression="1")) == pytest.approx(0, abs=1e-3) + assert slot_centre(cmd("parameters.change", name="n", expression="1"), + cmd("parameters.change", name="n", expression="3")) == pytest.approx(-6, abs=1e-3) def centred_row(position: list) -> list: @@ -162,12 +171,6 @@ def test_a_circumscribed_polygon_needs_the_polygon_constraint(): assert sketch_dof([*HEXAGON, cmd("sketch.constraint.polygon", lines=[f"l{i}" for i in range(1, 7)])]) == 0 -def test_in_a_plain_block_only_a_tapered_tool_on_a_face_logs_a_boolean_retry(): - assert any("coincident-face" in w for w in warnings([*BLOCK, *tapered_cut("0 mm", "12 mm")])) - assert warnings([*BLOCK, *tapered_cut("-1 mm", "14 mm")]) == [] - assert warnings([*BLOCK, *tapered_cut("0 mm", "12 mm", taper="0 deg")]) == [] - - def test_round_features_measure_about_1e_4_off(): body = execute([cmd("solid.cylinder", diameter="20 mm", height="10 mm")])["bodies"][0] error = abs(body["volume_mm3"] / (pi * 100 * 10) - 1) @@ -392,14 +395,14 @@ def standoffs_at(standoffs: list, height: float) -> tuple[float, dict]: return gap, {f["name"]: f["warning"] for f in high["timeline"] if f.get("warning")} -def test_a_boolean_retry_can_leave_material_in_a_cut(): +def test_a_boolean_retry_reports_tool_displacement_without_excess_material(): gap, warned = standoffs_at(TWO_CYLINDERS, 7) - assert gap > 10 - assert "coincident-face" in warned["Standoff"] + assert abs(gap) < 0.5 + assert "Standoff" not in warned assert "tool displacement" in warned["Pilot"] -def test_sinking_the_join_and_overcutting_the_hole_does_not_fix_it(): +def test_sinking_the_join_removes_the_warning(): gap, warned = standoffs_at([ cmd("solid.cylinder", diameter="6 mm", height="h + 1 mm", base=["-39 mm", "-24.5 mm", "1 mm"], operation="join", targets=["Case"], name="Standoff"), @@ -407,8 +410,8 @@ def test_sinking_the_join_and_overcutting_the_hole_does_not_fix_it(): targets=["Case"], name="Pilot"), cmd("solid.pattern.rectangular", features=["Standoff", "Pilot"], **STANDOFF_GRID), ], 7) - assert gap > 10 - assert "Standoff" not in warned + assert abs(gap) < 0.5 + assert warned == {} @pytest.mark.parametrize("height", [3, 7, 12, 20]) @@ -418,11 +421,12 @@ def test_a_ring_profile_builds_the_same_standoffs_exactly(height): assert warned == {} -@pytest.mark.parametrize("base, height, retries", [("2 mm", "h", True), ("1 mm", "h + 1 mm", False)]) -def test_a_straight_join_on_a_shelled_floor_retries_unless_sunk(base, height, retries): +def test_a_sunk_join_on_a_shelled_floor_does_not_warn(): warned = warnings([ *SHELLED_CASE, - cmd("solid.cylinder", diameter="6 mm", height=height, base=["-39 mm", "-24.5 mm", base], operation="join", + cmd("solid.cylinder", diameter="6 mm", height="h + 1 mm", base=["-39 mm", "-24.5 mm", "1 mm"], operation="join", targets=["Case"], name="Standoff"), ]) - assert any("coincident-face" in w for w in warned) == retries + assert warned == [] + + diff --git a/skills/solidcraft-checking/scripts/tests/test_run.py b/skills/solidcraft-checking/scripts/tests/test_run.py index a17077a37f..ceb99e18d2 100644 --- a/skills/solidcraft-checking/scripts/tests/test_run.py +++ b/skills/solidcraft-checking/scripts/tests/test_run.py @@ -38,19 +38,16 @@ def test_flags_underconstrained_sketch(tmp_path): def test_reports_step_warnings(tmp_path): result = report(tmp_path, [ - {"command": "sketch.create", "params": {"plane": "XY", "offset": "12 mm", "name": "Plate"}}, - {"command": "sketch.rectangle.center", "params": {"center": [0, 0], "corner": [40, 40]}}, - {"command": "sketch.finish", "params": {}}, - {"command": "solid.extrude", "params": {"sketch": "Plate", "distance": "12 mm", "direction": "negative", - "body_name": "P"}}, - {"command": "sketch.create", "params": {"plane": "XY", "name": "Hole"}}, - {"command": "sketch.circle.center", "params": {"center": [0, 0], "diameter": 20}}, - {"command": "sketch.finish", "params": {}}, - {"command": "solid.extrude", "params": {"sketch": "Hole", "distance": "12 mm", "taper": "-20 deg", - "operation": "cut", "targets": ["P"], "name": "Flush cut"}}, + {"command": "parameters.add", "params": {"name": "h", "expression": "5 mm"}}, + {"command": "solid.box", "params": {"length": 91, "width": 62, "height": 30, "center": [0, 0, 15], "body_name": "Case"}}, + {"command": "solid.fillet", "params": {"edges": [[45.5, 31, 15], [-45.5, 31, 15], [45.5, -31, 15], [-45.5, -31, 15]], "radius": 3}}, + {"command": "solid.shell", "params": {"faces": [[0, 0, 30]], "thickness": 2}}, + {"command": "solid.fillet", "params": {"edges": [[0, -31, 30]], "radius": 1}}, + {"command": "solid.cylinder", "params": {"diameter": "6 mm", "height": "h", "base": [-39, -24.5, 2], "operation": "join", "targets": ["Case"], "name": "Standoff"}}, + {"command": "solid.cylinder", "params": {"diameter": "2.5 mm", "height": "h", "base": [-39, -24.5, 2], "operation": "cut", "targets": ["Case"], "name": "Pilot"}}, ]) assert result.returncode == 0, result.stderr - assert 'WARN "Flush cut": Boolean retry' in result.stdout + assert 'WARN "Pilot": Boolean retry' in result.stdout def test_fails_on_a_bad_step(tmp_path): @@ -86,40 +83,38 @@ def test_execute_raises_on_failure(): @pytest.fixture def flush_design(tmp_path): - """A cut 10 mm deep into a 12 mm plate; at depth 13 mm its tapered end lies on the plate top.""" - design = tmp_path / "plate.json" + """A shelled case with a joined standoff and a cut pilot hole; Pilot warns tool displacement.""" + design = tmp_path / "standoff.json" built = report(tmp_path, [ - {"command": "parameters.add", "params": {"name": "depth", "expression": "10 mm"}}, - {"command": "sketch.create", "params": {"plane": "XY", "name": "Plate sketch"}}, - {"command": "sketch.rectangle.center", "params": {"center": [0, 0], "corner": [40, 40]}}, - {"command": "sketch.finish", "params": {}}, - {"command": "solid.extrude", "params": {"sketch": "Plate sketch", "distance": "12 mm", "body_name": "P"}}, - {"command": "sketch.create", "params": {"plane": "XY", "offset": "-1 mm", "name": "Cut sketch"}}, - {"command": "sketch.circle.center", "params": {"center": [5, 5], "diameter": 20}}, - {"command": "sketch.finish", "params": {}}, - {"command": "solid.extrude", "params": {"sketch": "Cut sketch", "distance": "depth", "taper": "10 deg", - "operation": "cut", "targets": ["P"], "name": "Cut"}}, + {"command": "parameters.add", "params": {"name": "h", "expression": "5 mm"}}, + {"command": "solid.box", "params": {"length": 91, "width": 62, "height": 30, "center": [0, 0, 15], "body_name": "Case"}}, + {"command": "solid.fillet", "params": {"edges": [[45.5, 31, 15], [-45.5, 31, 15], [45.5, -31, 15], [-45.5, -31, 15]], "radius": 3}}, + {"command": "solid.shell", "params": {"faces": [[0, 0, 30]], "thickness": 2}}, + {"command": "solid.fillet", "params": {"edges": [[0, -31, 30]], "radius": 1}}, + {"command": "solid.cylinder", "params": {"diameter": "6 mm", "height": "h", "base": [-39, -24.5, 2], "operation": "join", "targets": ["Case"], "name": "Standoff"}}, + {"command": "solid.cylinder", "params": {"diameter": "2.5 mm", "height": "h", "base": [-39, -24.5, 2], "operation": "cut", "targets": ["Case"], "name": "Pilot"}}, ], "--save", design) - assert built.returncode == 0 and "WARN" not in built.stdout, built.stdout + built.stderr + assert built.returncode == 0, built.stdout + built.stderr return design def depth(value: str) -> dict: - return {"command": "parameters.change", "params": {"name": "depth", "expression": value}} + return {"command": "parameters.change", "params": {"name": "h", "expression": value}} def test_reports_warnings_after_a_parameter_change(tmp_path, flush_design): - flush = report(tmp_path, [depth("13 mm")], "--in", flush_design) - assert 'WARN "Cut": Boolean retry' in flush.stdout + flush = report(tmp_path, [depth("7 mm")], "--in", flush_design) + assert flush.returncode == 0, flush.stderr + assert 'WARN "Pilot": Boolean retry' in flush.stdout def test_reports_every_inspection_in_the_script(tmp_path, flush_design): - walk = report(tmp_path, [depth("13 mm"), {"command": "document.inspect", "params": {"measure": True}}, - depth("10 mm")], "--in", flush_design) + walk = report(tmp_path, [depth("7 mm"), {"command": "document.inspect", "params": {"measure": True}}, + depth("5 mm")], "--in", flush_design) lines = walk.stdout.splitlines() - assert any(line.startswith('step 2 WARN "Cut": Boolean retry') for line in lines), walk.stdout - assert any(line.startswith("step 2 BODY P: volume") for line in lines) - assert not any(line.startswith("WARN") for line in lines) + assert any(line.startswith('step 2 WARN "Pilot": Boolean retry') for line in lines) + assert any(line.startswith("step 2 BODY Case: volume") for line in lines) + assert any(line.startswith('WARN "Pilot": Boolean retry') for line in lines) def test_reports_bodies_inspected_without_measurements(tmp_path): diff --git a/skills/solidcraft-modeling/reference.md b/skills/solidcraft-modeling/reference.md index aaba13f939..22972dabb8 100644 --- a/skills/solidcraft-modeling/reference.md +++ b/skills/solidcraft-modeling/reference.md @@ -129,7 +129,7 @@ After `set_parameter L=120` the volume is 35335.2 mm³, and the holes stay in th - **Tolerance:** analytic and measured volumes agree to about 0.001-0.01%, so treat a gap that small as a match. Long shallow arcs measure worse, about 0.1% (R 1000 mm over 20 degrees). - **Face counts:** compare the logical `faces`, not the larger `kernel.faces`. - **Reply types:** `set_parameter` returns `recomputed` as a number. A batch reply echoes every step. -- **Boolean warning:** a feature `warning` such as `Boolean retry: temporary coincident-face separation up to 2.050000e0 mm per face` means the kernel nudged faces that coincide. Examples are a join flush on a face, such as a cylinder joined onto the floor of a shelled box, or a tapered cut that starts on a face. When the volume still matches your number the warning is harmless. When it does not, the retry may have left the geometry wrong, with no error. Seen: on a box with filleted corners, a shell and a rounded rim, a standoff joined and its pilot hole cut as two cylinders, patterned 2x2, left material in the holes at some standoff heights. A boss with its hole as one ring profile (two concentric circles, extrude the ring) built correctly at every height tried (3 to 20 mm). Another kind, `tool displacement`, moves a tool by about 2e-4 mm. +- **Boolean warning:** a feature `warning` such as `Boolean retry: temporary coincident-face separation up to 2.050000e0 mm per face` means the kernel nudged faces that coincide. A join flush on a face, such as a cylinder joined onto the floor of a shelled box, or a tapered cut that starts on a face, does not log it. When the volume still matches your number the warning is harmless. When it does not, the retry may have left the geometry wrong, with no error. A standoff built as a joined cylinder with a cut pilot hole, patterned together, reports `tool displacement` (moving a tool by about 2e-4 mm) but its volume agrees with the analytic growth; a boss with its hole as one ring profile (two concentric circles, extrude the ring) builds without a warning at every height tried (3 to 20 mm). ## Reply cost (0.8, plate example)