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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd
This commit is contained in:
Umut CAN
2026-09-22 19:48:00 +03:00
co-authored by Claude Opus 5
parent ac6c4075ba
commit 457c53644f
2 changed files with 183 additions and 21 deletions
+65 -18
View File
@@ -202,6 +202,23 @@ def _anchor(feature, part: RevolvedPart) -> tuple[float, float, int]:
return (x, left, -1) if left >= right else (x, right, +1) 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]: def _shoulder(feature, part: RevolvedPart) -> tuple[float, float, int]:
"""(x, radius of the SMALLER cylinder, direction into it) for a relief groove.""" """(x, radius of the SMALLER cylinder, direction into it) for a relief groove."""
kind, index = _parse_at(feature.at, part) kind, index = _parse_at(feature.at, part)
@@ -342,15 +359,22 @@ class Chamfer(_Base):
angle = _number(self.angle, "angle") angle = _number(self.angle, "angle")
if not 0.0 < angle < 90.0: if not 0.0 < angle < 90.0:
raise ValueError(f"angle: a chamfer angle is between 0 and 90 degrees, got {angle}.") 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() axial, radial = self._legs()
if radial >= radius: if radial >= radius:
raise ValueError( raise ValueError(
f"size {self.size} at {self.at!r} cuts {radial:.3f} mm off a {radius:.3f} mm " 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." "radius: the chamfer is larger than the material it sits on."
) )
if axial > part_length(part): index = _anchor_segment(self, part)
raise ValueError(f"size {self.size} is longer than the part ({part_length(part)} mm).") 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 del x
def corner_edit(self, part) -> dict: def corner_edit(self, part) -> dict:
@@ -549,10 +573,14 @@ class Keyway(_Base):
half = width / 2.0 half = width / 2.0
top = math.sqrt(max(radius * radius - half * half, 0.0)) top = math.sqrt(max(radius * radius - half * half, 0.0))
floor = radius - depth floor = radius - depth
start = math.degrees(math.atan2(top, half)) % 360.0 right = math.degrees(math.atan2(top, half)) % 360.0
end = 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 ( 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, top), (-half, floor), "visible"),
Line((-half, floor), (half, floor), "visible"), Line((-half, floor), (half, floor), "visible"),
Line((half, floor), (half, top), "visible"), Line((half, floor), (half, top), "visible"),
@@ -1417,12 +1445,24 @@ class GearTeeth(_Base):
# "front" : the outline's own XY # "front" : the outline's own XY
# "side" : u = part Y, v = z in [0, thickness] (looking along -X) # "side" : u = part Y, v = z in [0, thickness] (looking along -X)
# "top" : u = part X, v = z in [0, thickness] (looking along -Y) # "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 @_register
@dataclass(frozen=True, kw_only=True) @dataclass(frozen=True, kw_only=True)
class Hole(_Base): 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" KIND: ClassVar[str] = "hole"
x: float x: float
@@ -1484,19 +1524,21 @@ class Hole(_Base):
return tuple(prims) return tuple(prims)
if view in ("side", "top"): if view in ("side", "top"):
u = y if view == "side" else x u = y if view == "side" else x
face = part.thickness
depth = part.thickness if self.depth is None else float(self.depth) depth = part.thickness if self.depth is None else float(self.depth)
floor = face - depth
prims = [ prims = [
Line((u - half, 0.0), (u - half, depth), "hidden"), Line((u - half, face), (u - half, floor), "hidden"),
Line((u + half, 0.0), (u + half, depth), "hidden"), Line((u + half, face), (u + half, floor), "hidden"),
] ]
if self.depth is not None: 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: if self.cbore_d is not None:
cb = float(self.cbore_d) / 2.0 cb = float(self.cbore_d) / 2.0
cd = float(self.cbore_depth) cf = face - float(self.cbore_depth)
prims.append(Line((u - cb, 0.0), (u - cb, cd), "hidden")) prims.append(Line((u - cb, face), (u - cb, cf), "hidden"))
prims.append(Line((u + cb, 0.0), (u + cb, cd), "hidden")) prims.append(Line((u + cb, face), (u + cb, cf), "hidden"))
prims.append(Line((u - cb, cd), (u + cb, cd), "hidden")) prims.append(Line((u - cb, cf), (u + cb, cf), "hidden"))
return tuple(prims) return tuple(prims)
return () return ()
@@ -1647,7 +1689,11 @@ class Slot(_Base):
@_register @_register
@dataclass(frozen=True, kw_only=True) @dataclass(frozen=True, kw_only=True)
class Pocket(_Base): 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" KIND: ClassVar[str] = "pocket"
outline: tuple[tuple[float, float], ...] outline: tuple[tuple[float, float], ...]
@@ -1685,11 +1731,12 @@ class Pocket(_Base):
if view in ("side", "top"): if view in ("side", "top"):
box = self.removal_box(part) box = self.removal_box(part)
u0, u1 = (box[1], box[3]) if view == "side" else (box[0], box[2]) 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 ( return (
Line((u0, part.thickness), (u0, floor), "hidden"), Line((u0, face), (u0, floor), "hidden"),
Line((u0, floor), (u1, floor), "hidden"), Line((u0, floor), (u1, floor), "hidden"),
Line((u1, floor), (u1, part.thickness), "hidden"), Line((u1, floor), (u1, face), "hidden"),
) )
return () return ()
+118 -3
View File
@@ -41,7 +41,7 @@ from engineering.mech.features import (
mirror_about_x, mirror_about_x,
) )
from engineering.mech.part import PrismaticPart, RevolvedPart, Segment, build_part 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"} 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)) 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(): 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) feature = Fillet(id="f1", at="step:0", radius=2.0)
part = shaft(feature) 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(): 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) feature = Keyway(id="kw", segment=1, length=25.0)
part = shaft(feature) part = shaft(feature)
prims = feature.prims(part, "side", CTX) 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].p1[0] == pytest.approx(-width / 2.0)
assert lines[1].p2[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(): def test_a_keyway_longer_than_its_segment_is_refused_by_name():
feature = Keyway(id="kw", segment=1, length=50.0) 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 assert sum(1 for p in front if isinstance(p, Line) and p.role == "center") == 2
side = feature.prims(part, "side", CTX) side = feature.prims(part, "side", CTX)
assert {p.role for p in side} == {"hidden"} 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) 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(): def test_a_hole_outside_the_outline_is_refused_by_name():