From bd73b3a854b1b2dd59948cf030f0ed7d3b836b07 Mon Sep 17 00:00:00 2001 From: Umut CAN Date: Tue, 22 Sep 2026 23:04:25 +0300 Subject: [PATCH] fix(task-6): a reachable ISO 286 route, a tolerance schema that refuses, a reference dimension MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review findings, all root causes in `engineering/mech/dimension.py`. 1. The ISO 286 fit path was unreachable from the module's own pipeline. `resolve_tolerance` resolved `intent.fit` correctly, but nothing produced a DimIntent carrying one: no feature and no segment has a fit, and `dimension_intents` constructed every intent without `fit=`. Only a hand-built DimIntent - which is what the tests used - reached the code, so the spec's `mech_iso286_on_dimension` and the `mech_assembly` benchmark's "ISO 286 fits on the bearing seats" had no route. `dimension_intents(..., fits={key: code})` now stamps the code onto the intent its key names, and the part's own diameters became addressable one at a time through the new `segment_key(i, bore=...)` -> "segment[1]", "segment[1].bore"; a feature's stay addressable by id. A key that names nothing is refused with the names that exist, a malformed code is refused where it is attached rather than at draw time, a fit on a reference dimension or on one that already carries a tol is refused, and a fit on a hole whose diameter moved into the hole table (which has no tolerance column) is refused by name instead of vanishing. Task 7's planned `dimension_part(..., fits=None)` passes straight through - the argument is additive and keyword-only, so every call written against the pinned signature keeps working. The fit also now reaches the *text*: `intent_text` expands the `<>` placeholder and appends the code, so `text_rects` measures the string that is actually drawn and a hole that already names itself ("⌀6") keeps its own text and gains the code. 2. An explicit tolerance was silently discarded when `tol` omitted "mode". `mode = tol.get("mode", "none")` and `build_dim_override(0.05, 0.05, "none", None)` returns `({}, None)` without raising, so a ±0.05 resolved, echoed its own numbers back in the payload, and drew a dimension with no DIMTOL/DIMTP/DIMTM at all. `DimIntent.tol` now has a schema - {"mode", "upper", "lower"} and nothing else - validated by `_explicit_tol`: an unknown key, a missing "mode", a non-dict, and deviations under a mode that writes none are each refused by name. The mutual-exclusion guard reads `tol is not None` rather than truthiness, so `tol={}` is refused too. 3. The ISO 129-1 "no measurement twice" test was vacuous and the property was false. The old test built the key `_dedupe` uniquifies as the last step of `dimension_intents`, so it could not fail; meanwhile `style="chain"` emitted 20, 40, 15 *and* 75 as four plain DIMENSIONs. `redundant_linear` now finds the span that shorter ones tile end to end on the same axis and level, and `dimension_intents` marks it as an auxiliary (reference) dimension - the new `REFERENCE_FORMAT` "(<>)", which renders "(75)". The plan's own test still holds (the 75 is present); it is no longer a fourth plain dimension. The error is one-sided on purpose: a tiling that is found is real, a tiling the greedy walk misses leaves the dimension plain, which is the direction the repository's critique rule wants. Baseline and ordinate parenthesise nothing, and two horizontal chains at different heights are not each other's pieces. Tests: the vacuous gate is replaced by four that fail against the pre-fix code - `redundant_linear` on the shaft's plain intents returns (3,) there and () here; the parametrised "a tolerance is either refused or reaches the dimension variables" gate resolves the mode-less shape to an empty DIMVAR override there; and `fits=` is a TypeError there. `_dedupe` is now exercised through a duplicate fed in via the view's own dims, which the collector does not generate itself. 46 tests in tests/test_mech_dimension.py (was 23). Live AutoCAD: no finding was measured live and no new ActiveX call was written - this module is pure Python with no backend, no async and no DXF. A DIMTOL probe against the running seat was attempted twice and `GetActiveObject("AutoCAD.Application")` blocked past the ~90 s contention window both times, so the live engine was unavailable; no scratch document was created (the probe never got the application object) and none was left open. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd --- engineering/mech/__init__.py | 10 ++ engineering/mech/dimension.py | 314 +++++++++++++++++++++++++++++++--- tests/test_mech_dimension.py | 250 ++++++++++++++++++++++++++- 3 files changed, 546 insertions(+), 28 deletions(-) diff --git a/engineering/mech/__init__.py b/engineering/mech/__init__.py index b58a1d7..da0eaa4 100644 --- a/engineering/mech/__init__.py +++ b/engineering/mech/__init__.py @@ -8,13 +8,18 @@ from __future__ import annotations from .dimension import ( DIM_STYLES, + REFERENCE_FORMAT, TEXT_WIDTH_FACTOR, + TOL_KEYS, dimension_intents, hole_table_rows, intent_text, + is_reference, layout_dimensions, measured_value, + redundant_linear, resolve_tolerance, + segment_key, text_rects, ) from .features import ( @@ -131,6 +136,7 @@ __all__ = [ "Prim", "PrismaticPart", "Pt", + "REFERENCE_FORMAT", "ROLES", "ROLE_LAYER", "RetainingGroove", @@ -141,6 +147,7 @@ __all__ = [ "Segment", "Slot", "TEXT_WIDTH_FACTOR", + "TOL_KEYS", "Text", "Thread", "Undercut", @@ -157,6 +164,7 @@ __all__ = [ "hole_table_rows", "inner_radius_at", "intent_text", + "is_reference", "layer_for", "layout_dimensions", "layout_origin", @@ -172,12 +180,14 @@ __all__ = [ "point_in_outline", "points_of", "radial_profile", + "redundant_linear", "resolve_tolerance", "rotate", "scale", "section_profile_prims", "section_view", "segment_bounds", + "segment_key", "silhouette_prims", "text_rects", "translate", diff --git a/engineering/mech/dimension.py b/engineering/mech/dimension.py index 41e29e0..4e532b1 100644 --- a/engineering/mech/dimension.py +++ b/engineering/mech/dimension.py @@ -22,13 +22,34 @@ The test asserts pairwise disjointness with the same helper, which makes "no overlapping dimension text" measured rather than claimed - and it is the same property the repository's `dim_overlap` critique focus checks on the finished drawing. + +Three rules the module enforces rather than assumes: + +*ISO 286 fits reach the pipeline.* :func:`dimension_intents` takes +``fits={key: code}`` and stamps the code onto the intent it names, so a bearing +seat gets its k6 without anyone hand-building a ``DimIntent``. The part's own +diameters are addressable by segment (``"segment[1]"``, ``"segment[1].bore"``); +a feature's are addressable by its id. An unknown key is refused by name - a fit +that lands nowhere is a manufacturing requirement lost in silence. + +*An explicit tolerance is validated as a whole.* ``DimIntent.tol`` is +``{"mode", "upper", "lower"}`` and nothing else. ``mode`` is required, because +``build_dim_override`` reads a missing mode as ``"none"`` and returns an empty +override: a +-0.05 with no ``mode`` used to resolve, report its own numbers back +and then draw a dimension carrying no tolerance at all. + +*ISO 129-1: each measurement appears once.* A closed chain plus the overall +states the overall twice. :func:`redundant_linear` finds the span that the +others tile end to end, and :func:`dimension_intents` marks it as an auxiliary +(reference) dimension - ``(<>)``, drawn in parentheses - rather than emitting a +fourth plain DIMENSION that contradicts nothing and adds nothing. """ from __future__ import annotations import math -from engineering.fits import fit_lookup +from engineering.fits import fit_lookup, parse_fit_code from engineering.mech.features import Hole from engineering.mech.part import ( PrismaticPart, @@ -38,7 +59,7 @@ from engineering.mech.part import ( segment_bounds, ) from engineering.mech.primitives import DimIntent -from engineering.tolerances import build_dim_override +from engineering.tolerances import TOL_MODES, build_dim_override _EPS = 1e-9 @@ -52,6 +73,18 @@ TEXT_WIDTH_FACTOR = 0.7 #: Padding added around a measured label, in text heights. TEXT_PADDING = 0.6 +#: How an auxiliary (reference) dimension is written - ISO 129-1 puts a +#: measurement that is already given elsewhere in parentheses. ``<>`` is the +#: DIMENSION text placeholder for the measured value, on both engines. +REFERENCE_FORMAT = "(<>)" + +#: The only keys ``DimIntent.tol`` accepts. +TOL_KEYS: frozenset[str] = frozenset({"mode", "upper", "lower"}) + +#: Tolerance modes that write no deviation, so carrying upper/lower with them +#: would silently discard the numbers. +_NO_DEVIATION_MODES: frozenset[str] = frozenset({"none", "basic"}) + def measured_value(intent: DimIntent) -> float: """What the dimension measures, in drawing units.""" @@ -66,10 +99,20 @@ def _format(value: float) -> str: return f"{round(value, 3):g}" -def intent_text(intent: DimIntent) -> str: - """The text a dimension will carry - the override, or the measurement.""" - if intent.text_override: - return intent.text_override +def _with_fit(text: str, fit: str | None) -> str: + """Append an ISO 286 code to a dimension's text, once. + + A feature that already names itself (a hole's ``⌀6``) keeps its own text and + gains the code; without this the deviations would be written and the code + the inspector reads would not be on the sheet. + """ + code = (fit or "").strip() + if not code or code in text: + return text + return f"{text} {code}" + + +def _measured_text(intent: DimIntent) -> str: if intent.kind == "diameter": return f"⌀{_format(measured_value(intent))}" if intent.kind == "radius": @@ -77,6 +120,28 @@ def intent_text(intent: DimIntent) -> str: return _format(measured_value(intent)) +def intent_text(intent: DimIntent) -> str: + """The text a dimension will carry, as the engine will render it. + + ``<>`` is the DIMENSION placeholder for the measured value, so an override + of ``"(<>)"`` reads ``"(75)"`` and a resolved fit's ``"<> H7"`` reads + ``"⌀40 H7"``. Expanding it here is what makes :func:`text_rects` measure the + string that is actually drawn - a label measured as ``"<> H7"`` and drawn as + ``"⌀40 H7"`` is a layout computed against the wrong width. + """ + measured = _measured_text(intent) + base = intent.text_override.replace("<>", measured) if intent.text_override else measured + # The fit code is appended to the text by `resolve_tolerance`; the rectangle + # has to reserve room for it before the layout runs. + return _with_fit(base, intent.fit) + + +def is_reference(intent: DimIntent) -> bool: + """True for an auxiliary (reference) dimension - ISO 129-1 parentheses.""" + override = intent.text_override or "" + return override.startswith("(") and override.endswith(")") + + def _text_size(intent: DimIntent, height: float) -> tuple[float, float]: """``(width, height)`` of the rectangle this intent's label occupies.""" label = intent_text(intent) @@ -104,6 +169,44 @@ def text_rects(intents, *, height: float = 3.5): # -- tolerances -------------------------------------------------------------- +def _explicit_tol(intent: DimIntent) -> tuple[str, float | None, float | None]: + """Validate ``intent.tol`` as a whole and return ``(mode, upper, lower)``. + + ``build_dim_override`` alone is not enough: it defaults a missing mode to + ``"none"`` and answers with an empty override, so ``{"upper": 0.05, + "lower": 0.05}`` used to resolve, echo its own numbers back and draw a + dimension with no DIMTOL/DIMTP/DIMTM at all. A tolerance that disappears + between the spec and the sheet is the silent wrong value this repository + refuses, so the schema is checked here, by name. + """ + who = intent.feature or intent.kind + tol = intent.tol + if not isinstance(tol, dict): + raise ValueError( + f"{who}.tol: expected a dict with keys {sorted(TOL_KEYS)}, got {type(tol).__name__}." + ) + unknown = set(tol) - TOL_KEYS + if unknown: + raise ValueError( + f"{who}.tol: unknown key(s) {sorted(unknown)}; a tolerance takes {sorted(TOL_KEYS)}." + ) + if "mode" not in tol: + raise ValueError( + f"{who}.tol: needs an explicit 'mode' (one of {list(TOL_MODES)}). Without it the " + "deviations are carried in the payload and no tolerance reaches the drawing." + ) + mode = str(tol["mode"]).strip().lower() + upper = tol.get("upper") + lower = tol.get("lower") + if mode in _NO_DEVIATION_MODES and (upper is not None or lower is not None): + raise ValueError( + f"{who}.tol: tol_mode={mode!r} writes no deviation, but upper={upper!r} / " + f"lower={lower!r} were given. Use 'symmetric', 'deviation' or 'limit', or drop " + "the values." + ) + return mode, upper, lower + + def resolve_tolerance(intent: DimIntent, nominal: float | None = None) -> dict: """Turn an intent's ``fit`` / ``tol`` into the backend dimension call's arguments. @@ -111,7 +214,7 @@ def resolve_tolerance(intent: DimIntent, nominal: float | None = None) -> dict: ``build_dim_override`` reads ``tol_lower`` as the *magnitude* of the minus deviation, so a resolved fit passes ``-deviation.lower_mm``. """ - has_tol = bool(intent.tol) + has_tol = intent.tol is not None if intent.fit and has_tol: raise ValueError( f"{intent.feature or intent.kind}: pass either fit= or an explicit " @@ -129,13 +232,10 @@ def resolve_tolerance(intent: DimIntent, nominal: float | None = None) -> dict: "tol_upper": deviation.upper_mm, "tol_lower": -deviation.lower_mm, "tol_mode": "deviation", - "text_override": intent.text_override or f"<> {deviation.code}", + "text_override": _with_fit(intent.text_override or "<>", deviation.code), } if has_tol: - tol = dict(intent.tol) - mode = str(tol.get("mode", "none")) - upper = tol.get("upper") - lower = tol.get("lower") + mode, upper, lower = _explicit_tol(intent) build_dim_override(upper, lower, mode, intent.text_override) # validates, or raises return { "tol_upper": upper, @@ -172,6 +272,129 @@ def _dedupe(intents) -> tuple[DimIntent, ...]: return tuple(out) +def _linear_span(intent: DimIntent) -> tuple[str, float, float, float] | None: + """``(axis, level, lo, hi)`` of an axis-parallel linear dimension, else None. + + ``level`` is the coordinate the dimension does *not* run along, so two + horizontal dimensions at different heights are never compared with each + other. A skewed linear intent has no span: it is not part of a chain. + """ + if intent.kind != "linear": + return None + (ax, ay), (bx, by) = intent.p1, intent.p2 + if abs(bx - ax) >= abs(by - ay): + if abs(by - ay) > _EPS: + return None + return ("x", round(float(ay), 6), round(min(ax, bx), 6), round(max(ax, bx), 6)) + if abs(bx - ax) > _EPS: + return None + return ("y", round(float(ax), 6), round(min(ay, by), 6), round(max(ay, by), 6)) + + +def redundant_linear(intents) -> tuple[int, ...]: + """Indices of the linear intents that shorter ones already tile end to end. + + ISO 129-1: a measurement appears once. A closed chain (20 + 40 + 15) that + also carries the overall (75) states the overall twice, and the second + statement has to be an auxiliary (reference) dimension or be left off. + + The error is one-sided on purpose. A tiling that is found is a real one - + the pieces butt, in order, from ``lo`` to ``hi`` on the same axis and level + - so a dimension is never called redundant when it is not. A tiling that + the greedy walk misses simply leaves the dimension plain, which is the way + round the repository's critique rule wants: a check that misses a bad + drawing costs a review, a check that calls a good drawing bad gets + switched off. + """ + groups: dict[tuple[str, float], list[tuple[float, float, int]]] = {} + for index, intent in enumerate(intents): + span = _linear_span(intent) + if span is None: + continue + axis, level, lo, hi = span + groups.setdefault((axis, level), []).append((lo, hi, index)) + + out: list[int] = [] + for group in groups.values(): + for lo, hi, index in group: + pieces = sorted( + (a, b) + for a, b, other in group + if other != index + and a >= lo - 1e-6 + and b <= hi + 1e-6 + and (b - a) < (hi - lo) - 1e-6 + ) + cursor = lo + for a, b in pieces: + if abs(a - cursor) <= 1e-6: + cursor = b + if cursor > lo + 1e-6 and abs(cursor - hi) <= 1e-6: + out.append(index) + return tuple(sorted(out)) + + +def _mark_references(intents: list[DimIntent]) -> list[DimIntent]: + """Put the redundant measurements in parentheses, ISO 129-1.""" + for index in redundant_linear(intents): + intent = intents[index] + if intent.text_override is None: + intents[index] = intent._replace(text_override=REFERENCE_FORMAT) + return intents + + +def _apply_fits(intents: list[DimIntent], fits, *, table_ids: frozenset[str]) -> list[DimIntent]: + """Stamp each ``{key: ISO 286 code}`` onto the intent its key names. + + Without this the ``fit`` branch of :func:`resolve_tolerance` is reachable + only from a hand-built ``DimIntent``: no feature and no segment carries a + fit, so the ISO 286 path would be real in the table and absent from every + drawing the module actually produces. + """ + if not fits: + return intents + if not isinstance(fits, dict): + raise ValueError( + f"fits: expected a dict of {{key: ISO 286 code}}, got {type(fits).__name__}." + ) + + available = {intent.feature for intent in intents if intent.feature} + by_key: dict[str, list[int]] = {} + for index, intent in enumerate(intents): + if intent.feature: + by_key.setdefault(intent.feature, []).append(index) + + for key, code in fits.items(): + name = str(key) + if name not in available: + if name in table_ids: + raise ValueError( + f"fits[{name!r}]: that hole's diameter moved into the hole table, which has " + "no tolerance column. Raise hole_table_threshold, or dimension the hole " + "directly." + ) + raise ValueError( + f"fits[{name!r}]: nothing in this view is dimensioned under that name. " + f"Available: {sorted(available)}." + ) + parse_fit_code(str(code)) # refuses a typo here, not at draw time + for index in by_key[name]: + intent = intents[index] + if is_reference(intent): + raise ValueError( + f"fits[{name!r}]: {intent_text(intent)} is an auxiliary (reference) " + "dimension - ISO 129-1 gives it no tolerance. Put the fit on the " + "dimension that states the size." + ) + if intent.tol is not None: + raise ValueError( + f"fits[{name!r}]: that dimension already carries an explicit tol. Pass " + "either fit= or an explicit tol, not both." + ) + intents[index] = intent._replace(fit=str(code)) + return intents + + def _revolved_axial(part: RevolvedPart, style: str) -> list[DimIntent]: stations = [0.0] + [x1 for _, x1 in segment_bounds(part)] intents: list[DimIntent] = [] @@ -189,18 +412,39 @@ def _revolved_axial(part: RevolvedPart, style: str) -> list[DimIntent]: return intents +def segment_key(index: int, *, bore: bool = False) -> str: + """The ``fits`` key that names one segment's outer diameter, or its bore. + + The part's own diameters have to be addressable one at a time: "ISO 286 + fits on the bearing seats" means a code on *segment 1*, not on every + dimension the part emits. + """ + return f"segment[{int(index)}].bore" if bore else f"segment[{int(index)}]" + + def _revolved_diameters(part: RevolvedPart) -> list[DimIntent]: intents: list[DimIntent] = [] - for (x0, x1), segment in zip(segment_bounds(part), part.segments, strict=True): + bounds = zip(segment_bounds(part), part.segments, strict=True) + for index, ((x0, x1), segment) in enumerate(bounds): mid = (x0 + x1) / 2.0 radius = segment.d_outer / 2.0 intents.append( - DimIntent(kind="diameter", p1=(mid, radius), p2=(mid, -radius), feature="part") + DimIntent( + kind="diameter", + p1=(mid, radius), + p2=(mid, -radius), + feature=segment_key(index), + ) ) if segment.d_inner > _EPS: bore = segment.d_inner / 2.0 intents.append( - DimIntent(kind="diameter", p1=(mid, bore), p2=(mid, -bore), feature="part") + DimIntent( + kind="diameter", + p1=(mid, bore), + p2=(mid, -bore), + feature=segment_key(index, bore=True), + ) ) return intents @@ -226,8 +470,21 @@ def dimension_intents( *, style: str = "chain", hole_table_threshold: int = 8, + fits: dict | None = None, ) -> tuple[DimIntent, ...]: - """Every dimension this view wants: the features' own, plus the part's sizes.""" + """Every dimension this view wants: the features' own, plus the part's sizes. + + ``fits`` maps a dimension's name to an ISO 286 code - a feature id, or one + of the part's own diameters by :func:`segment_key` (``"segment[1]"``, + ``"segment[1].bore"``). The code is stamped onto the matching intent, so + :func:`resolve_tolerance` resolves it and :func:`layout_dimensions` reserves + room for the widened text. A key that names nothing is refused with the + names that exist. + + ISO 129-1 is applied last: a measurement the other dimensions already tile + end to end is marked as an auxiliary (reference) dimension, in parentheses, + instead of being drawn a second time as a plain DIMENSION. + """ name = str(style or "").strip().lower() if name not in DIM_STYLES: raise ValueError(f"style: expected one of {DIM_STYLES}, got {style!r}.") @@ -240,15 +497,26 @@ def dimension_intents( intents.extend(_revolved_axial(part, name)) intents.extend(_revolved_diameters(part)) elif kind == "side": - near = part.segments[-1] + last = len(part.segments) - 1 + near = part.segments[last] radius = near.d_outer / 2.0 intents.append( - DimIntent(kind="diameter", p1=(-radius, 0.0), p2=(radius, 0.0), feature="part") + DimIntent( + kind="diameter", + p1=(-radius, 0.0), + p2=(radius, 0.0), + feature=segment_key(last), + ) ) if near.d_inner > _EPS: bore = near.d_inner / 2.0 intents.append( - DimIntent(kind="diameter", p1=(-bore, 0.0), p2=(bore, 0.0), feature="part") + DimIntent( + kind="diameter", + p1=(-bore, 0.0), + p2=(bore, 0.0), + feature=segment_key(last, bore=True), + ) ) elif isinstance(part, PrismaticPart): x0, y0, x1, y1 = outline_bbox(part) @@ -270,11 +538,13 @@ def dimension_intents( groups = _hole_groups(part) total_holes = sum(len(holes) for holes in groups.values()) + table_ids: frozenset[str] = frozenset() if total_holes > int(hole_table_threshold): - hole_ids = {hole.id for holes in groups.values() for hole in holes} - intents = [i for i in intents if i.feature not in hole_ids] + table_ids = frozenset(hole.id for holes in groups.values() for hole in holes) + intents = [i for i in intents if i.feature not in table_ids] - return _dedupe(intents) + collected = _mark_references(list(_dedupe(intents))) + return tuple(_apply_fits(collected, fits, table_ids=table_ids)) def hole_table_rows(part, view, *, hole_table_threshold: int = 8) -> tuple[dict, ...]: diff --git a/tests/test_mech_dimension.py b/tests/test_mech_dimension.py index 298508e..23ee80d 100644 --- a/tests/test_mech_dimension.py +++ b/tests/test_mech_dimension.py @@ -7,6 +7,7 @@ overlapping dimension text" is a measured property rather than a claim. from __future__ import annotations +import dataclasses import itertools import pytest @@ -16,15 +17,19 @@ from engineering.mech.dimension import ( dimension_intents, hole_table_rows, intent_text, + is_reference, layout_dimensions, measured_value, + redundant_linear, resolve_tolerance, + segment_key, text_rects, ) from engineering.mech.features import Hole, Keyway from engineering.mech.part import PrismaticPart, RevolvedPart, Segment from engineering.mech.primitives import DimIntent from engineering.mech.views import build_view +from engineering.tolerances import build_dim_override def shaft(*features) -> RevolvedPart: @@ -121,6 +126,175 @@ def test_a_fit_with_no_measurable_nominal_is_refused_rather_than_guessed(): resolve_tolerance(intent) +@pytest.mark.parametrize( + "tol", + [ + {"mode": "symmetric", "upper": 0.05}, + {"mode": "deviation", "upper": 0.05, "lower": 0.02}, + {"mode": "limit", "upper": 30.05, "lower": 29.98}, + {"upper": 0.05, "lower": 0.05}, # the shape whose tolerance used to vanish + {"mode": "none", "upper": 0.05}, + {}, + ], +) +def test_a_tolerance_is_either_refused_or_reaches_the_dimension_variables(tol): + """Accepted and then written as nothing is the one answer that is not allowed. + + `build_dim_override` defaults a missing mode to "none" and answers with an + empty override, so a tol with no mode used to resolve, echo its own numbers + back and draw a dimension carrying no DIMTOL/DIMTP/DIMTM at all. + """ + intent = DimIntent(kind="linear", p1=(0.0, 0.0), p2=(30.0, 0.0), tol=tol) + try: + resolved = resolve_tolerance(intent) + except ValueError: + return # refused up front, which is the other legal answer + override, _ = build_dim_override( + resolved["tol_upper"], + resolved["tol_lower"], + resolved["tol_mode"], + resolved["text_override"], + ) + assert override, f"{tol} resolved to {resolved} and wrote no dimension variable" + + +def test_a_tolerance_with_no_mode_is_refused_rather_than_silently_discarded(): + intent = DimIntent( + kind="linear", p1=(0.0, 0.0), p2=(30.0, 0.0), tol={"upper": 0.05, "lower": 0.05} + ) + with pytest.raises(ValueError, match="needs an explicit 'mode'"): + resolve_tolerance(intent) + + +def test_an_empty_tol_is_refused_rather_than_read_as_no_tolerance(): + intent = DimIntent(kind="linear", p1=(0.0, 0.0), p2=(30.0, 0.0), tol={}) + with pytest.raises(ValueError, match="mode"): + resolve_tolerance(intent) + + +def test_a_tol_key_outside_the_schema_is_refused_by_name(): + intent = DimIntent( + kind="linear", + p1=(0.0, 0.0), + p2=(30.0, 0.0), + tol={"mode": "symmetric", "upper": 0.05, "plus": 0.05}, + ) + with pytest.raises(ValueError, match="plus"): + resolve_tolerance(intent) + + +def test_deviations_under_a_mode_that_writes_none_are_refused(): + intent = DimIntent( + kind="linear", p1=(0.0, 0.0), p2=(30.0, 0.0), tol={"mode": "none", "upper": 0.05} + ) + with pytest.raises(ValueError, match="writes no deviation"): + resolve_tolerance(intent) + + +def test_a_tol_that_is_not_a_dict_is_refused(): + intent = DimIntent(kind="linear", p1=(0.0, 0.0), p2=(30.0, 0.0), tol=0.05) + with pytest.raises(ValueError, match="expected a dict"): + resolve_tolerance(intent) + + +# -- the ISO 286 route from the part to the dimension ------------------------ + + +def bush() -> RevolvedPart: + """A 60 mm bore housing: one segment, ⌀60 outside, ⌀40 bore.""" + return RevolvedPart(name="bush", segments=(Segment(60.0, 60.0, 40.0),)) + + +def test_a_fit_reaches_a_dimension_through_the_modules_own_pipeline(): + """`mech_iso286_on_dimension`, end to end: a ⌀40 H7 bore carries +0.025 / 0. + + `resolve_tolerance` always resolved a fit; nothing produced an intent that + carried one, so the headline claim was reachable only from a hand-built + DimIntent - which is what the tests used. + """ + part = bush() + view = build_view(part, "front") + intents = dimension_intents(part, view, fits={segment_key(0, bore=True): "H7"}) + fitted = [i for i in intents if i.fit] + assert len(fitted) == 1 + assert measured_value(fitted[0]) == pytest.approx(40.0) + resolved = resolve_tolerance(fitted[0]) + assert resolved["tol_upper"] == pytest.approx(0.025) + assert resolved["tol_lower"] == pytest.approx(0.0) + assert resolved["tol_mode"] == "deviation" + assert resolved["text_override"] == "<> H7" + # the fit has to survive the layout, or the number is right in the table + # and absent from the sheet + placed = layout_dimensions(view, intents) + assert [i.fit for i in placed if i.fit] == ["H7"] + + +def test_a_bearing_seat_takes_its_fit_by_segment(): + part = shaft() + view = build_view(part, "front") + intents = dimension_intents(part, view, fits={segment_key(1): "k6"}) + seat = [i for i in intents if i.fit] + assert [i.feature for i in seat] == ["segment[1]"] + assert measured_value(seat[0]) == pytest.approx(30.0) + assert intent_text(seat[0]) == "⌀30 k6" + + +def test_a_fit_on_a_feature_id_lands_on_that_features_dimension(): + part = plate(*grid(3)) + view = build_view(part, "front") + intents = dimension_intents(part, view, fits={"h1": "H7"}) + assert [i.feature for i in intents if i.fit] == ["h1"] + # a hole already names itself; the code is appended, never swallowed + hole = next(i for i in intents if i.fit) + assert intent_text(hole) == "⌀6 H7" + assert resolve_tolerance(hole)["text_override"] == "⌀6 H7" + + +def test_a_fit_widens_the_measured_label_so_the_layout_reserves_room(): + part = shaft() + view = build_view(part, "front") + plain = next(i for i in dimension_intents(part, view) if i.feature == segment_key(1)) + fitted = next( + i + for i in dimension_intents(part, view, fits={segment_key(1): "k6"}) + if i.feature == segment_key(1) + ) + + def width(intent): + rect = text_rects(layout_dimensions(view, [intent]))[0] + return rect[2] - rect[0] + + assert width(fitted) > width(plain) + + +def test_a_fit_key_that_names_nothing_is_refused_with_the_names_that_exist(): + part = shaft() + view = build_view(part, "front") + with pytest.raises(ValueError, match=r"segment\[0\]"): + dimension_intents(part, view, fits={"bearing_seat": "k6"}) + + +def test_a_malformed_fit_code_is_refused_where_it_is_attached(): + part = shaft() + view = build_view(part, "front") + with pytest.raises(ValueError, match="fit code"): + dimension_intents(part, view, fits={segment_key(1): "H"}) + + +def test_a_fit_on_a_reference_dimension_is_refused(): + part = shaft() + view = build_view(part, "front") + with pytest.raises(ValueError, match="reference"): + dimension_intents(part, view, fits={"part": "H7"}) + + +def test_a_fit_on_a_hole_that_moved_into_the_table_is_refused_by_name(): + part = plate(*grid(10)) + view = build_view(part, "front") + with pytest.raises(ValueError, match="hole table"): + dimension_intents(part, view, fits={"h1": "H7"}, hole_table_threshold=8) + + # -- collecting the intents -------------------------------------------------- @@ -156,15 +330,79 @@ def test_a_feature_intent_survives_collection_and_keeps_its_feature_id(): assert any(i.feature == "kw" for i in intents) -def test_iso129_the_same_measurement_is_not_dimensioned_twice(): +def test_a_duplicate_intent_is_collapsed_rather_than_drawn_twice(): + """`_dedupe` against an intent the collector did not generate itself. + + Asserting uniqueness over what `dimension_intents` returns proves nothing - + it is the key `_dedupe` uniquifies. Feeding the duplicate in through the + view's own dims is what exercises the gate. + """ part = shaft() view = build_view(part, "front") - intents = dimension_intents(part, view) - keys = [ - (i.kind, round(i.p1[0], 6), round(i.p1[1], 6), round(i.p2[0], 6), round(i.p2[1], 6)) - for i in intents + once = dimension_intents(part, view) + repeated = next(i for i in once if i.kind == "diameter") + doubled = dataclasses.replace(view, dims=view.dims + (repeated._replace(text_at=None),)) + assert len(dimension_intents(part, doubled)) == len(once) + + +def test_iso129_a_closed_chain_marks_the_overall_as_a_reference_dimension(): + """20 + 40 + 15 and 75 state the same distance twice; ISO 129-1 parenthesises one. + + The plan's own test asserts the 75 is present, which a bare fourth plain + DIMENSION satisfies. What matters is that it is drawn as an auxiliary + (reference) dimension. + """ + part = shaft() + view = build_view(part, "front") + linear = [i for i in dimension_intents(part, view, style="chain") if i.kind == "linear"] + assert sorted(round(measured_value(i), 6) for i in linear) == [15.0, 20.0, 40.0, 75.0] + overall = [i for i in linear if measured_value(i) == 75.0] + assert len(overall) == 1 + assert is_reference(overall[0]) + assert intent_text(overall[0]) == "(75)" + assert [i for i in linear if is_reference(i)] == overall + + +def test_iso129_no_plain_dimension_restates_a_measurement_the_others_give(): + """The real property, on every style and both part kinds.""" + parts = (shaft(Keyway(id="kw", segment=1, length=20.0)), plate(*grid(4))) + for style in DIM_STYLES: + for part in parts: + view = build_view(part, "front") + intents = dimension_intents(part, view, style=style) + plain = [i for i in intents if not is_reference(i)] + assert redundant_linear(plain) == (), f"{part.name} / {style}" + + +def test_the_redundancy_check_fires_instead_of_restating_its_own_post_condition(): + """A gate that cannot fail is not a gate: hand it the un-marked chain.""" + chain = [ + DimIntent(kind="linear", p1=(0.0, 0.0), p2=(20.0, 0.0)), + DimIntent(kind="linear", p1=(20.0, 0.0), p2=(60.0, 0.0)), + DimIntent(kind="linear", p1=(60.0, 0.0), p2=(75.0, 0.0)), + DimIntent(kind="linear", p1=(0.0, 0.0), p2=(75.0, 0.0)), ] - assert len(keys) == len(set(keys)) + assert redundant_linear(chain) == (3,) + baseline = [ + DimIntent(kind="linear", p1=(0.0, 0.0), p2=(20.0, 0.0)), + DimIntent(kind="linear", p1=(0.0, 0.0), p2=(60.0, 0.0)), + DimIntent(kind="linear", p1=(0.0, 0.0), p2=(75.0, 0.0)), + ] + assert redundant_linear(baseline) == () + # two horizontal chains at different heights measure different things + stacked = [ + DimIntent(kind="linear", p1=(0.0, 0.0), p2=(20.0, 0.0)), + DimIntent(kind="linear", p1=(20.0, 5.0), p2=(75.0, 5.0)), + DimIntent(kind="linear", p1=(0.0, 0.0), p2=(75.0, 0.0)), + ] + assert redundant_linear(stacked) == () + + +def test_baseline_states_every_station_once_so_nothing_is_parenthesised(): + part = shaft() + view = build_view(part, "front") + intents = dimension_intents(part, view, style="baseline") + assert not any(is_reference(i) for i in intents) def test_a_prismatic_front_view_gets_its_two_overall_sizes():