From 457c53644f950902e280b8108239ca3b667314bd Mon Sep 17 00:00:00 2001 From: Umut CAN Date: Tue, 22 Sep 2026 19:48:00 +0300 Subject: [PATCH] fix(task-3): keyway end-view arc, the machined face, and the chamfer's segment bound Three defects in the feature catalogue, all faithfully transcribed from the plan text and all wrong in shipped code. 1. Keyway.prims(view="side") emitted the arc the keyway REMOVES. `Arc` is CCW start -> end, and the emitted (right wall -> left wall) is the ~31 degree cap over the slot opening, not the ~329 degrees of shaft that survives it. Measured live on AutoCAD 2026 (Documents.Add scratch doc, closed in a finally): ModelSpace.AddArc(center, radius, start_rad, end_rad) -- angles in RADIANS, CCW start -> end -- gives ArcLength 8.098 mm for the emitted 74.534 -> 105.466 deg and 86.150 mm for the reversed 105.466 -> 74.534 deg, against a 94.248 mm full circle at r = 15; StartPoint/EndPoint of the emitted arc are exactly the notch walls at x = +/-4. Swapped, so the arc now runs left wall -> right wall the long way round, and the side view's bbox goes from (-4, 11, 4, 15) to (-15, -15, 15, 14.457). 2. Hole and Pocket cut the plate from OPPOSITE faces in the same view frame: Hole ran z = 0 -> depth, Pocket z = thickness -> thickness - depth, and neither takes a face parameter. A plate carrying both showed the pocket milled from one side and the blind hole drilled from the other, silently -- omission() returns None for both, so nothing reported it. The frame comment now names the machined face (v = thickness, the face the front view looks at), and Hole drills from it: the blind floor, the counterbore step and the through bore all hang down from there, as Pocket already did. 3. Chamfer.validate bounded the axial leg against the whole part, never against the segment the chamfer sits on. A shallow lead-in passed validation and was drawn straight through a shoulder at a radius that segment does not have (size 30 at 10 deg on a 15 mm end segment: radial 5.29 < 10 so the radial guard was happy, yet `before` landed at x = 45, two segments away). New helper _anchor_segment names the segment a corner actually cuts into -- at a step, the adjacent one carrying the larger radius -- and the leg is bounded by its length, refused by name with the step it would have crossed. Tests: the keyway test asserted only primitive counts and two x coordinates, so it passed with the arc reversed; it now pins the sweep, the endpoints and the bbox. The hole test exercised only a through hole, which is symmetric; added a blind-hole-and-pocket test that pins both to the same face, and a counterbore test. Added three chamfer tests (two refusals, one that must still be accepted). Each fix was mutation-checked: reverting it fails its test. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd --- engineering/mech/features.py | 83 ++++++++++++++++++------ tests/test_mech_features.py | 121 ++++++++++++++++++++++++++++++++++- 2 files changed, 183 insertions(+), 21 deletions(-) diff --git a/engineering/mech/features.py b/engineering/mech/features.py index 34cc21d..56e5e55 100644 --- a/engineering/mech/features.py +++ b/engineering/mech/features.py @@ -202,6 +202,23 @@ def _anchor(feature, part: RevolvedPart) -> tuple[float, float, int]: return (x, left, -1) if left >= right else (x, right, +1) +def _anchor_segment(feature, part: RevolvedPart) -> int: + """Index of the segment a chamfer at ``feature.at`` actually cuts into. + + ``_anchor`` gives the corner and the direction into the material; the leg + runs that way, so at a step it belongs to whichever of the two adjacent + segments carries the larger radius. + """ + kind, index = _parse_at(feature.at, part) + if kind == "start": + return 0 + if kind == "end": + return len(part.segments) - 1 + left = _radius_right_end(part, index) + right = _radius_left_end(part, index + 1) + return index if left >= right else index + 1 + + def _shoulder(feature, part: RevolvedPart) -> tuple[float, float, int]: """(x, radius of the SMALLER cylinder, direction into it) for a relief groove.""" kind, index = _parse_at(feature.at, part) @@ -342,15 +359,22 @@ class Chamfer(_Base): angle = _number(self.angle, "angle") if not 0.0 < angle < 90.0: raise ValueError(f"angle: a chamfer angle is between 0 and 90 degrees, got {angle}.") - x, radius, _ = _anchor(self, part) + x, radius, direction = _anchor(self, part) axial, radial = self._legs() if radial >= radius: raise ValueError( f"size {self.size} at {self.at!r} cuts {radial:.3f} mm off a {radius:.3f} mm " "radius: the chamfer is larger than the material it sits on." ) - if axial > part_length(part): - raise ValueError(f"size {self.size} is longer than the part ({part_length(part)} mm).") + index = _anchor_segment(self, part) + segment_length = part.segments[index].length + if axial > segment_length + _EPS: + far = x + direction * segment_length + raise ValueError( + f"size {self.size} at {self.at!r} runs {axial:g} mm along the axis, past " + f"segment {index}'s {segment_length:g} mm: the chamfer would cross the step " + f"at x={far:g} and be anchored at a radius that segment does not have." + ) del x def corner_edit(self, part) -> dict: @@ -549,10 +573,14 @@ class Keyway(_Base): half = width / 2.0 top = math.sqrt(max(radius * radius - half * half, 0.0)) floor = radius - depth - start = math.degrees(math.atan2(top, half)) % 360.0 - end = math.degrees(math.atan2(top, -half)) % 360.0 + right = math.degrees(math.atan2(top, half)) % 360.0 + left = math.degrees(math.atan2(top, -half)) % 360.0 + # `Arc` is CCW from start to end, so the shaft that SURVIVES the slot + # runs from the left notch wall the long way round to the right one. + # Emitting (right -> left) would draw the ~31 degree cap over the + # opening instead of the ~329 degrees of material. return ( - Arc((0.0, 0.0), radius, start, end, "visible"), + Arc((0.0, 0.0), radius, left, right, "visible"), Line((-half, top), (-half, floor), "visible"), Line((-half, floor), (half, floor), "visible"), Line((half, floor), (half, top), "visible"), @@ -1417,12 +1445,24 @@ class GearTeeth(_Base): # "front" : the outline's own XY # "side" : u = part Y, v = z in [0, thickness] (looking along -X) # "top" : u = part X, v = z in [0, thickness] (looking along -Y) +# +# THE MACHINED FACE IS v = thickness. The front view looks at the outline from +# +Z, so the face it shows -- the face a feature placed by its (x, y) is cut +# from -- is the top of the side and top views, and every blind feature hangs +# down from there toward v = 0. Hole, Pocket and anything added later must +# agree on this: a drawing whose pocket is milled from one face and whose blind +# hole is drilled from the other is silently wrong, and no validator can tell. @_register @dataclass(frozen=True, kw_only=True) class Hole(_Base): - """A hole in a prismatic part, optionally counterbored, countersunk or tapped.""" + """A hole in a prismatic part, optionally counterbored, countersunk or tapped. + + Drilled from the machined face (``v = thickness``, the one the front view + looks at), so a blind hole hangs down from there and a counterbore steps + down from the same face -- the convention ``Pocket`` follows too. + """ KIND: ClassVar[str] = "hole" x: float @@ -1484,19 +1524,21 @@ class Hole(_Base): return tuple(prims) if view in ("side", "top"): u = y if view == "side" else x + face = part.thickness depth = part.thickness if self.depth is None else float(self.depth) + floor = face - depth prims = [ - Line((u - half, 0.0), (u - half, depth), "hidden"), - Line((u + half, 0.0), (u + half, depth), "hidden"), + Line((u - half, face), (u - half, floor), "hidden"), + Line((u + half, face), (u + half, floor), "hidden"), ] if self.depth is not None: - prims.append(Line((u - half, depth), (u + half, depth), "hidden")) + prims.append(Line((u - half, floor), (u + half, floor), "hidden")) if self.cbore_d is not None: cb = float(self.cbore_d) / 2.0 - cd = float(self.cbore_depth) - prims.append(Line((u - cb, 0.0), (u - cb, cd), "hidden")) - prims.append(Line((u + cb, 0.0), (u + cb, cd), "hidden")) - prims.append(Line((u - cb, cd), (u + cb, cd), "hidden")) + cf = face - float(self.cbore_depth) + prims.append(Line((u - cb, face), (u - cb, cf), "hidden")) + prims.append(Line((u + cb, face), (u + cb, cf), "hidden")) + prims.append(Line((u - cb, cf), (u + cb, cf), "hidden")) return tuple(prims) return () @@ -1647,7 +1689,11 @@ class Slot(_Base): @_register @dataclass(frozen=True, kw_only=True) class Pocket(_Base): - """A milled pocket. ``no_section_hatch`` marks a rib or web (ISO 128-3).""" + """A milled pocket. ``no_section_hatch`` marks a rib or web (ISO 128-3). + + Milled from the machined face (``v = thickness``), the same face ``Hole`` + drills from. + """ KIND: ClassVar[str] = "pocket" outline: tuple[tuple[float, float], ...] @@ -1685,11 +1731,12 @@ class Pocket(_Base): if view in ("side", "top"): box = self.removal_box(part) u0, u1 = (box[1], box[3]) if view == "side" else (box[0], box[2]) - floor = part.thickness - float(self.depth) + face = part.thickness + floor = face - float(self.depth) return ( - Line((u0, part.thickness), (u0, floor), "hidden"), + Line((u0, face), (u0, floor), "hidden"), Line((u0, floor), (u1, floor), "hidden"), - Line((u1, floor), (u1, part.thickness), "hidden"), + Line((u1, floor), (u1, face), "hidden"), ) return () diff --git a/tests/test_mech_features.py b/tests/test_mech_features.py index 7a00e73..b529d3f 100644 --- a/tests/test_mech_features.py +++ b/tests/test_mech_features.py @@ -41,7 +41,7 @@ from engineering.mech.features import ( mirror_about_x, ) from engineering.mech.part import PrismaticPart, RevolvedPart, Segment, build_part -from engineering.mech.primitives import Arc, Circle, Line, Poly, Text +from engineering.mech.primitives import Arc, Circle, Line, Poly, Text, bbox CTX: dict = {"side": "right", "scale": 1.0, "projection": "first"} @@ -167,6 +167,43 @@ def test_a_chamfer_bigger_than_the_step_is_refused_by_name(): feature.validate(shaft(feature)) +def test_a_chamfer_whose_axial_leg_outruns_its_segment_is_refused_by_name(): + """The radial guard only bites near 45 deg; a shallow lead-in slips past it. + + The end segment of `shaft()` is 15 mm long at r = 10. A 30 mm chamfer at + 10 deg takes only 5.29 mm off the radius, so `radial >= radius` is happy, + and the old part-length bound (75 mm) was happy too - yet `before` landed + at x = 45, two segments away, anchored at a radius that segment does not + have. + """ + feature = Chamfer(id="c1", at="end", size=30.0, angle=10.0) + with pytest.raises(ValueError) as excinfo: + feature.validate(shaft(feature)) + message = str(excinfo.value) + assert "30" in message and "15" in message and "segment 2" in message + + +def test_a_shallow_chamfer_that_fits_inside_its_segment_is_still_accepted(): + """The new bound refuses only what leaves the segment; 14 < 15 mm stays.""" + feature = Chamfer(id="c1", at="end", size=14.0, angle=10.0) + part = shaft(feature) + feature.validate(part) + edit = feature.corner_edit(part) + assert edit["before"][0] == pytest.approx(61.0) + assert edit["before"][0] >= 60.0 # inside segment 2, which spans x 60..75 + + +def test_a_chamfer_at_a_step_is_bounded_by_the_segment_it_cuts_into(): + """`shaft()` steps UP at x = 20, so the chamfer cuts into segment 1 (40 mm). + + A 45 mm leg outruns that segment even though the part is 75 mm long. + """ + feature = Chamfer(id="c1", at="step:0", size=45.0, angle=5.0) + with pytest.raises(ValueError) as excinfo: + feature.validate(shaft(feature)) + assert "segment 1" in str(excinfo.value) and "40" in str(excinfo.value) + + def test_a_fillet_at_a_step_up_emits_a_tangent_arc_in_the_lower_right_quadrant(): feature = Fillet(id="f1", at="step:0", radius=2.0) part = shaft(feature) @@ -216,6 +253,16 @@ def test_a_keyway_takes_its_width_and_depth_from_din_6885(): def test_a_keyway_end_view_breaks_the_circle_and_draws_the_notch(): + """The arc is the shaft that SURVIVES the slot, not the cap over its mouth. + + `Arc` is CCW start -> end, so the sweep has to be the long way round: it + leaves the left notch wall, goes over the bottom of the shaft and arrives + at the right one. Emitting the two angles the other way round draws a short + chord-like cap straight across the keyway opening - geometrically a legal + arc, which is why only the sweep and the endpoints catch it. Measured on + AutoCAD 2026: the 30.93 deg arc is 8.098 mm long, the 329.07 deg one is + 86.150 mm, and the full r=15 circle is 94.248 mm. + """ feature = Keyway(id="kw", segment=1, length=25.0) part = shaft(feature) prims = feature.prims(part, "side", CTX) @@ -226,6 +273,22 @@ def test_a_keyway_end_view_breaks_the_circle_and_draws_the_notch(): assert lines[1].p1[0] == pytest.approx(-width / 2.0) assert lines[1].p2[0] == pytest.approx(width / 2.0) + arc = arcs[0] + radius = 15.0 + half = width / 2.0 + top = math.sqrt(radius * radius - half * half) + # start at the LEFT wall, end at the RIGHT one: everything but the opening. + assert arc.radius == pytest.approx(radius) + assert arc.start_deg == pytest.approx(math.degrees(math.atan2(top, -half))) + assert arc.end_deg == pytest.approx(math.degrees(math.atan2(top, half))) + sweep = (arc.end_deg - arc.start_deg) % 360.0 + assert sweep == pytest.approx(329.068, abs=1e-3) + assert math.radians(sweep) * radius == pytest.approx(86.150, abs=1e-3) + # and the end view therefore spans the whole shaft, not just the notch: + # full width and full depth, topped by the notch walls because the slot has + # taken the crown of the circle away. + assert bbox(prims) == pytest.approx((-radius, -radius, radius, top)) + def test_a_keyway_longer_than_its_segment_is_refused_by_name(): feature = Keyway(id="kw", segment=1, length=50.0) @@ -468,9 +531,61 @@ def test_a_hole_draws_a_circle_in_true_shape_and_hidden_lines_in_the_side_view() assert sum(1 for p in front if isinstance(p, Line) and p.role == "center") == 2 side = feature.prims(part, "side", CTX) assert {p.role for p in side} == {"hidden"} - assert side[0].p1 == (20.0, 0.0) and side[0].p2 == (20.0, 8.0) + # a through hole runs the whole thickness, drawn from the machined face down + assert side[0].p1 == (20.0, 8.0) and side[0].p2 == (20.0, 0.0) top = feature.prims(part, "top", CTX) - assert top[0].p1 == (15.0, 0.0) + assert top[0].p1 == (15.0, 8.0) + + +def test_a_blind_hole_and_a_pocket_are_cut_from_the_same_face(): + """Both enter at v = thickness - the face the front view looks at. + + Nothing downstream can catch a disagreement here: two blind features cut + from opposite faces of the same plate are each a legal drawing on their + own, `omission()` returns None for both, and the validator sees only + hidden lines inside the material. So it has to be pinned at the source. + """ + hole = Hole(id="h1", x=20.0, y=25.0, diameter=6.0, depth=5.0) + pocket = Pocket( + id="p1", + outline=((40.0, 10.0), (70.0, 10.0), (70.0, 40.0), (40.0, 40.0)), + depth=5.0, + ) + part = plate(hole, pocket) + hole.validate(part) + pocket.validate(part) + for view in ("side", "top"): + entry = {round(p.p1[1], 9) for p in hole.prims(part, view, CTX)} | { + round(p.p2[1], 9) for p in hole.prims(part, view, CTX) + } + pocket_entry = {round(p.p1[1], 9) for p in pocket.prims(part, view, CTX)} | { + round(p.p2[1], 9) for p in pocket.prims(part, view, CTX) + } + # the same two v values: the face at 8.0 and a floor 5 mm below it + assert entry == {8.0, 3.0} + assert pocket_entry == {8.0, 3.0} + (left, right, floor) = hole.prims(part, "side", CTX) + assert left.p1 == (22.0, 8.0) and left.p2 == (22.0, 3.0) + assert right.p1 == (28.0, 8.0) and right.p2 == (28.0, 3.0) + assert floor.p1 == (22.0, 3.0) and floor.p2 == (28.0, 3.0) + + +def test_a_counterbore_is_stepped_down_from_the_machined_face_too(): + feature = Hole(id="h1", x=20.0, y=25.0, diameter=6.0, cbore_d=12.0, cbore_depth=3.0) + part = plate(feature) + feature.validate(part) + # the side view runs u = part Y, so the hole sits at u = 25 and the + # counterbore walls at 25 +/- 6. + side = feature.prims(part, "side", CTX) + cbore = [p for p in side if {19.0, 31.0} & {p.p1[0], p.p2[0]}] + assert len(cbore) == 3 + assert {round(p.p1[1], 9) for p in cbore} | {round(p.p2[1], 9) for p in cbore} == {8.0, 5.0} + # and the bore itself still runs the full thickness from that same face + bore = [p for p in side if {22.0, 28.0} & {p.p1[0], p.p2[0]}] + assert [(p.p1, p.p2) for p in bore] == [ + ((22.0, 8.0), (22.0, 0.0)), + ((28.0, 8.0), (28.0, 0.0)), + ] def test_a_hole_outside_the_outline_is_refused_by_name():