diff --git a/src/draftwright/annotations/holes.py b/src/draftwright/annotations/holes.py index e15f559c..1249bf34 100644 --- a/src/draftwright/annotations/holes.py +++ b/src/draftwright/annotations/holes.py @@ -77,7 +77,6 @@ _diameter_column_left, _diameter_row_below, callout_from_spec, - hole_callout_spec, ) from draftwright.annotations.hole_leader_candidates import ( FrontHoleLeaderCandidateAdapter, @@ -116,6 +115,37 @@ _AXIS_ALIGN_COS = 0.9996 +def _editable_hole_callout_batches( + feature, model, views, *, include_source_pmi, manufacturing_tags=None +): + """Read one edit verb through the same member-aware batches as automatic ink.""" + from draftwright.model.callout import hole_callout_batches + + group = next( + ( + candidate + for candidate in plan_dimensions(model, planned_views=views) + if candidate.feature is feature + ), + None, + ) + batches = ( + hole_callout_batches( + (group,), + include_source_pmi=include_source_pmi, + manufacturing_tags=manufacturing_tags, + ) + if group is not None + else () + ) + if len(batches) > 1: + raise ValueError( + "callout(): this pattern needs separate member callouts; " + "one editable callout would lose member requirements" + ) + return group, batches + + def add_feature_callout( dwg, feature, @@ -129,9 +159,10 @@ def add_feature_callout( """Add a hole/pattern ø-depth **leader callout** for *feature* — the #414 add verb, the callout-mechanism half of the editable surface (symmetric with :meth:`Drawing.drop`). - Funnels into the same :func:`hole_callout_spec` / :func:`callout_from_spec` the - auto-pass uses, so the callout text (ø, ``n×``, through/depth, cbore, pattern suffix) - is identical. Placement is a single reasonable leader beside the feature's end-on view + Funnels into the same batching and :func:`callout_from_spec` the auto-pass uses, + so the callout text (ø, ``n×``, through/depth, cbore, pattern suffix) is identical. + A pattern needing separate member callouts raises instead of losing requirements. + Placement is a single reasonable leader beside the feature's end-on view — not the auto-pass's whole-set priority solve (byte-identity is not a goal, #400 Ph2): a lone added callout goes into free strip space and leans on :meth:`Drawing.repair` / the coverage lint for the rest. The leader is tagged with *feature* so :meth:`drop` / @@ -147,22 +178,14 @@ def add_feature_callout( "callout(): feature is not from this drawing's model — " "pass one from dwg.model().features" ) - group = next( - ( - g - for g in plan_dimensions(model, planned_views=tuple(dwg.views)) - if g.feature is feature - ), - None, - ) - spec = ( - hole_callout_spec( - group, - include_source_pmi=not ctx.document_member or cast(Analysis, a).pmi_mode == "annotate", - ) - if group is not None - else None + group, batches = _editable_hole_callout_batches( + feature, + model, + tuple(dwg.views), + include_source_pmi=not ctx.document_member or cast(Analysis, a).pmi_mode == "annotate", + manufacturing_tags=getattr(ctx, "manufacturing_tags", None), ) + spec = batches[0].spec if batches else None if spec is None: if any(o.feature is feature and o.authored for o in compile_dimensions(model).diagnostics): # "Exposes none" would be a false claim: the feature exposes a bore ⌀ and was @@ -181,11 +204,10 @@ def add_feature_callout( f"{type(feature).__name__} exposes none — use dimension() for a linear param" ) draft = dwg.draft - members = feature.members or (feature.frame.origin,) - # count comes from the spec (== feat.count) — the same source the auto-pass's - # bare path uses — not re-derived from len(members). + members = batches[0].locations + # The batch, not the feature, owns the count represented by this callout. callout = callout_from_spec(spec, draft, spec["count"]) - assert callout is not None # spec is non-None here, so callout_from_spec returns one + assert callout is not None view = view or (group.view if group is not None else _END_ON[feature.frame.axis]) if view not in (*_END_ON.values(), "rear") or (view == "rear" and feature.frame.axis != "y"): raise ValueError(f"callout(): view {view!r} is not a hole-callout view for this axis") diff --git a/src/draftwright/build_policy.py b/src/draftwright/build_policy.py index 54552f17..2197ab3d 100644 --- a/src/draftwright/build_policy.py +++ b/src/draftwright/build_policy.py @@ -432,15 +432,34 @@ def _record(winner, attempts): # the default preserved A. That is the opposite of "preserve every supported requirement # or reject the candidate" (#1130). # - # The rule is therefore one-sided, and deliberately so: the alternative may not introduce - # any blocker the preferred result did not already have. It is free to preserve MORE, and - # it does not have to beat the historical arrangement on volume — that arrangement - # was the comparison floor before this local choice existed, so an alternative - # earns its place by costing nothing, not by costing less. + # The comparison is one-sided for ordinary losses: the alternative may not introduce + # a different blocker merely to reduce the number lost. The exception below applies + # only when both drawings are incomplete and the alternative preserves a strict + # superset of source-authored requirements without losing another source requirement + # or introducing a warning/error-level inferred loss. introduced = collections.Counter(map(_blocker_identity, blockers)) - collections.Counter( map(_blocker_identity, preferred_blockers) ) - if introduced: + # Neither arrangement can be called complete when both have blockers. In that + # case, a candidate that preserves strictly more source-authored requirements + # may still win despite losing informational inferred measurements. Keep the losses visible + # on the chosen drawing; a different source loss never trades for this gain. + candidate_source = collections.Counter( + _blocker_identity(blocker) for blocker in blockers if blocker.get("source_ids") + ) + preferred_source = collections.Counter( + _blocker_identity(blocker) for blocker in preferred_blockers if blocker.get("source_ids") + ) + preserves_more_source = ( + not candidate_source - preferred_source + and bool(preferred_source - candidate_source) + and all( + blocker.get("severity") == "info" + for blocker in blockers + if _blocker_identity(blocker) in introduced + ) + ) + if introduced and not preserves_more_source: return _record( preferred, [ diff --git a/src/draftwright/drawing.py b/src/draftwright/drawing.py index 5cbd5592..dc4a8190 100644 --- a/src/draftwright/drawing.py +++ b/src/draftwright/drawing.py @@ -1621,6 +1621,8 @@ def callout(self, feature, *, view=None, name=None) -> str: whole-set solve (byte-identity is not a goal, #400 Ph2) — :meth:`repair` tidies the rest. A step/boss diameter that finds no room returns ``""`` (a warning-level drop, like the auto-pass), rather than raising, so a reconstruction script never aborts. + A hole pattern needing separate member callouts raises; automatic annotation + emits them with their own requirement provenance. """ kind = getattr(feature, "kind", None) if (kind in _MACHINED_CALLOUT_KINDS or kind in ("pocket_pattern", "slot_pattern")) and ( @@ -1641,6 +1643,21 @@ def callout(self, feature, *, view=None, name=None) -> str: # live call returns "" with an `authored_omission` build issue, and the deferred # intent drains through the same migrated renderers to the same nothing. if self._defer_intents: # #426: record, don't place — finalize() drains it + if ( + kind in ("hole", "pattern") + and self._part_model is not None + and (not self._document_member or self._analysis is not None) + and any(owner is feature for owner in self._part_model.features) + ): + from draftwright.annotations.holes import _editable_hole_callout_batches + + _editable_hole_callout_batches( + feature, + self._part_model, + tuple(self.views), + include_source_pmi=not self._document_member + or getattr(self._analysis, "pmi_mode", None) == "annotate", + ) self._intents.append(Intent("callout", feature, {"view": view, "name": name})) return "" from draftwright.annotations.holes import add_feature_callout, add_feature_diameter diff --git a/src/draftwright/linting/pmi_coverage.py b/src/draftwright/linting/pmi_coverage.py index bc76a3cc..cb7e466b 100644 --- a/src/draftwright/linting/pmi_coverage.py +++ b/src/draftwright/linting/pmi_coverage.py @@ -149,6 +149,8 @@ def _registry_names_for_decoration(registry, key: tuple) -> list: kind = str(key[1]) if len(key) > 1 else "" role = str(key[2]) if len(key) > 2 else "" parameter = f"{role}.{kind}" if role else "" + if parameter and len(key) > 3: + parameter += f".{key[3]}" return [ name for name in registry.names() @@ -203,6 +205,10 @@ def _decorated_source_features(decorations, *, features=()) -> list[tuple[tuple, out.append((key, source_ids)) for feature in features: owner = feature + for index, requirement in enumerate(getattr(feature, "member_size_requirements", ())): + source_ids = _source_ids(requirement) + if source_ids: + out.append(((owner, "diameter", "bore", f"member_{index}"), source_ids)) target = getattr(feature, "member", feature) thread = getattr(target, "thread", None) source_ids = _source_ids(thread) diff --git a/src/draftwright/model/callout.py b/src/draftwright/model/callout.py index 524f0c54..8292e096 100644 --- a/src/draftwright/model/callout.py +++ b/src/draftwright/model/callout.py @@ -46,6 +46,60 @@ class HoleCalloutBatch: spec: dict +def _merge_member_pattern_batches(batches: list[HoleCalloutBatch]) -> list[HoleCalloutBatch]: + """Use one callout for members with identical complete printed requirements.""" + excluded = { + "count", + "measurements", + "source_measurements", + "geometry_measurements", + "geometry_qualifiers", + "source_ids", + "source_features", + "owner_counts", + } + grouped: dict[tuple, list[HoleCalloutBatch]] = {} + for batch in batches: + key = ( + id(batch.groups[0].feature), + any( + measurement.parameter.startswith("bore.diameter.member_") + for _source_id, measurement in batch.spec["source_measurements"] + ), + tuple((name, value) for name, value in batch.spec.items() if name not in excluded), + ) + grouped.setdefault(key, []).append(batch) + result = [] + for siblings in grouped.values(): + first = siblings[0] + feature = first.groups[0].feature + locations = tuple(location for sibling in siblings for location in sibling.locations) + spec = dict(first.spec) + spec["count"] = len(locations) if len(locations) > 1 else None + for field in ( + "measurements", + "source_measurements", + "geometry_measurements", + "source_ids", + ): + spec[field] = tuple( + dict.fromkeys(item for sibling in siblings for item in sibling.spec[field]) + ) + qualifiers = tuple( + dict.fromkeys( + item for sibling in siblings for item in sibling.spec["geometry_qualifiers"] + ) + ) + spec["geometry_qualifiers"] = ( + (*qualifiers, "grouping.count") if len(locations) > 1 else qualifiers + ) + spec["owner_counts"] = ((feature, len(locations)),) + result.append( + HoleCalloutBatch(tuple(sibling.groups[0] for sibling in siblings), locations, spec) + ) + return result + + def bore_callout_value(spec: dict, tolerance_suffix=lambda _value: "") -> str: """Format the bore value after the callout's leading diameter symbol.""" if limits := spec.get("diameter_limits"): @@ -77,10 +131,48 @@ def hole_callout_batches( member sites. Pattern furniture and profiled supports remain independent. A batch never combines overlapping member sites or incomplete count inventories. """ + member_batches: list[HoleCalloutBatch] = [] buckets: dict[tuple, list[list]] = {} ordered: list[list] = [] for group in groups: feature = group.feature + if isinstance(feature, PatternFeature) and feature.member_size_requirements: + complete = tuple(feature.members or (group.anchor,)) + locations = ( + complete if member_locations is None else member_locations.get(id(feature), ()) + ) + for index, point in enumerate(complete): + if point not in locations: + continue + member_id = f"bore.diameter.member_{index}" + member_group = replace( + group, + units=tuple( + unit + for unit in group.units + if unit.id == member_id + or not str(unit.id).startswith("bore.diameter.member_") + ), + ) + spec = hole_callout_spec( + member_group, + include_source_pmi=include_source_pmi, + manufacturing_tags=manufacturing_tags, + ) + if spec is None: + continue + spec["count"] = None + spec["pattern_suffix"] = None + spec["geometry_qualifiers"] = tuple( + qualifier + for qualifier in spec["geometry_qualifiers"] + if qualifier != "grouping.count" + ) + spec["suffix"] = hole_callout_suffix(spec) + spec["source_features"] = (feature,) + spec["owner_counts"] = ((feature, 1),) + member_batches.append(HoleCalloutBatch((member_group,), (point,), spec)) + continue spec = hole_callout_spec( group, include_source_pmi=include_source_pmi, @@ -186,7 +278,9 @@ def hole_callout_batches( spec, ) ) - return _qualify_coincident_axial_patterns(result) + return _qualify_coincident_axial_patterns( + [*_merge_member_pattern_batches(member_batches), *result] + ) def _qualify_coincident_axial_patterns( diff --git a/src/draftwright/model/declare.py b/src/draftwright/model/declare.py index bea9e6c5..883fd3c0 100644 --- a/src/draftwright/model/declare.py +++ b/src/draftwright/model/declare.py @@ -1882,6 +1882,7 @@ def pattern( rows=None, cols=None, angle=None, + member_size_requirements=(), ) -> PatternFeature: """A hole pattern = ``count`` × a *member* hole (build one with :func:`hole`). The arrangement (``bolt_circle`` / ``linear`` / ``grid``) and its defining dims (``bcd`` / @@ -1967,6 +1968,7 @@ def pattern( rows=rows, cols=cols, angle=angle, + member_size_requirements=tuple(member_size_requirements), ) diff --git a/src/draftwright/model/ir_foundation.py b/src/draftwright/model/ir_foundation.py index 0365355c..ab4524f4 100644 --- a/src/draftwright/model/ir_foundation.py +++ b/src/draftwright/model/ir_foundation.py @@ -6,7 +6,7 @@ from __future__ import annotations -from dataclasses import dataclass +from dataclasses import dataclass, replace from math import atan2, cos, hypot, isfinite, pi, radians, sin from typing import TYPE_CHECKING, ClassVar, Literal, Protocol, runtime_checkable @@ -845,8 +845,8 @@ class PatternFeature: ``count`` × a `member` hole arranged by the pattern. It composes the member `HoleFeature` (so the member's bore + counterbore/spotface/depth all come along — a counterbored bolt circle keeps its counterbore) and adds the - pattern-defining dims (BCD / pitch / grid pitches). The member holes are NOT - emitted individually (the engine's grouped ``n× ø`` callout).""" + pattern-defining dims (BCD / pitch / grid pitches). Uniform members share a + grouped callout; member-specific source requirements retain their own scope.""" #: The compiled stem this feature's position is minted under — see #: :attr:`HoleFeature.LOCATION_STEM` for why it is declared here (#966). @@ -864,10 +864,51 @@ class PatternFeature: rows: int | None = None cols: int | None = None angle: float | None = None # grid lattice rotation (degrees) + # Source size requirements addressed to individual physical members. Empty means the + # established uniform group callout; a populated tuple has one entry per member. + member_size_requirements: tuple[ToleranceDecoration | NominalRequirement | None, ...] = () kind: ClassVar[str] = "pattern" + def __post_init__(self) -> None: + if self.member_size_requirements and len(self.member_size_requirements) != self.count: + raise ValueError("member_size_requirements must have one entry per pattern member") + if self.member_size_requirements and self.pattern not in ("grid", "linear"): + raise ValueError("member size requirements need a grid or linear pattern") + if self.member_size_requirements and len(self.members) != self.count: + raise ValueError("member size requirements need every physical member location") + if any( + requirement is not None + and not isinstance(requirement, ToleranceDecoration | NominalRequirement) + for requirement in self.member_size_requirements + ): + raise ValueError("member size requirements must be typed source requirements") + if any( + isinstance(requirement, NominalRequirement) + and not requirement.agrees_with(self.member.diameter) + for requirement in self.member_size_requirements + ): + raise ValueError("member nominal requirement must agree with bore diameter") + def parameters(self) -> list[DimParameter]: ps = list(self.member.parameters()) # bore (+ counterbore / spotface / depth) + if self.member_size_requirements: + bore = ps.pop(0) + ps[:0] = [ + replace( + bore, + discriminator=f"member_{index}", + tolerance=( + requirement.value if isinstance(requirement, ToleranceDecoration) else None + ), + source_ids=requirement.source_ids if requirement is not None else (), + limit_bounds=( + requirement.limit_bounds + if isinstance(requirement, ToleranceDecoration) + else None + ), + ) + for index, requirement in enumerate(self.member_size_requirements) + ] if self.bcd is not None: ps.append(DimParameter("diameter", "bolt_circle", self.bcd)) if self.pitch is not None: diff --git a/src/draftwright/model/pmi_lowering.py b/src/draftwright/model/pmi_lowering.py index 62210d16..7d8920c7 100644 --- a/src/draftwright/model/pmi_lowering.py +++ b/src/draftwright/model/pmi_lowering.py @@ -125,7 +125,11 @@ def _hole_tolerance_proposal( "canonical hole/pattern features" ) owner_index, owner, member_indices = matches[0] - if isinstance(owner, PatternFeature) and len(member_indices) != len(_members(owner)): + if ( + isinstance(owner, PatternFeature) + and owner.pattern not in ("grid", "linear") + and len(member_indices) != len(_members(owner)) + ): return ( "unsupported hole correlation: AP242 requirement covers only part of a " "canonical hole pattern" @@ -138,15 +142,98 @@ def _hole_tolerance_proposal( return owner_index, member_indices, value +def _lower_pattern_member_sizes( + feature: PatternFeature, + member_requirements: dict[int, list[int]], + proposals: dict[int, tuple[int, tuple[int, ...], ToleranceValue]], + dimensions: dict[int, AuthoredDimension], + decorations: dict, + feature_remap: FeatureRemap | None, +) -> PatternFeature: + points = _members(feature) + groups = tuple(tuple(member_requirements.get(index, ())) for index in range(len(points))) + if len(set(groups)) == 1 and not feature.member_size_requirements: + dim_indices = sorted(set(groups[0])) + decorations[(feature, "diameter", "bore")] = ToleranceDecoration( + value=proposals[dim_indices[0]][2], + source="ap242_pmi", + source_ids=tuple( + dict.fromkeys( + source_id + for dim_index in dim_indices + for source_id in _source_ids(dimensions[dim_index]) + ) + ), + limit_bounds=_limit_bounds(dimensions[dim_indices[0]]), + ) + return feature + requirements = list(feature.member_size_requirements or (None,) * len(points)) + for member_index, member_dims in enumerate(groups): + if not member_dims: + continue + first = member_dims[0] + requirements[member_index] = ToleranceDecoration( + value=proposals[first][2], + source="ap242_pmi", + source_ids=tuple( + dict.fromkeys( + source_id + for dim_index in member_dims + for source_id in _source_ids(dimensions[dim_index]) + ) + ), + limit_bounds=_limit_bounds(dimensions[first]), + ) + replacement = replace(feature, member_size_requirements=tuple(requirements)) + for key, decoration in tuple(decorations.items()): + if key[0] is feature: + del decorations[key] + decorations[(replacement, *key[1:])] = decoration + if feature_remap is not None: + feature_remap(feature, (replacement,), (tuple(range(len(points))),)) + return replacement + + +def _is_internal_toleranced_diameter(feature: AuthoredDimension) -> bool: + """Select AP242 bore limits without sending external cylinders to the hole join.""" + return ( + feature.dimension_kind == "diameter" + and feature.source == "ap242_pmi" + and ( + not feature.cylindrical_refs + or all(reference.sense == "internal" for reference in feature.cylindrical_refs) + ) + and any( + value is not None + for value in ( + feature.lower_tol, + feature.upper_tol, + feature.lower_bound, + feature.upper_bound, + ) + ) + ) + + +def _hole_ownership_conflict( + owner: HoleFeature | PatternFeature, member_indices: tuple[int, ...], decorations: dict +) -> str | None: + if (owner, "diameter", "bore") in decorations or (owner, "diameter") in decorations: + return "ambiguous hole tolerance ownership: bore already has a tolerance" + if isinstance(owner, PatternFeature) and owner.member_size_requirements: + if any(owner.member_size_requirements[index] is not None for index in member_indices): + return "ambiguous hole tolerance ownership: member already has a source requirement" + return None + + def lower_ap242_hole_tolerances( model: PartModel, *, feature_remap: FeatureRemap | None = None ) -> PartModel: """Consume confidently correlated AP242 hole-tolerance dimensions exactly once. - A count-group is split only where member requirements differ. A real pattern stays a - pattern and therefore accepts only a requirement whose referenced geometry covers every - member. Those rules preserve machining-spec identity instead of applying one member's - tolerance to its untoleranced siblings or destroying pattern membership to make a match. + A count-group is split only where member requirements differ. A grid or linear pattern + retains its arrangement while individual source sizes remain scoped to exact members. + Other pattern kinds still require one source requirement to cover the whole group. """ targets: dict[int, HoleFeature | PatternFeature] = { index: feature @@ -156,18 +243,7 @@ def lower_ap242_hole_tolerances( dimensions = { index: feature for index, feature in enumerate(model.features) - if isinstance(feature, AuthoredDimension) - and feature.dimension_kind == "diameter" - and feature.source == "ap242_pmi" - and any( - value is not None - for value in ( - feature.lower_tol, - feature.upper_tol, - feature.lower_bound, - feature.upper_bound, - ) - ) + if isinstance(feature, AuthoredDimension) and _is_internal_toleranced_diameter(feature) } if not dimensions: return model @@ -189,11 +265,8 @@ def lower_ap242_hole_tolerances( # the same parameter have two sources and violate ADR 4 (was 0011)'s single-owner decoration map. for dim_index, (owner_index, _member_indices, _value) in tuple(proposals.items()): owner = targets[owner_index] - if (owner, "diameter", "bore") in model.decorations or ( - owner, - "diameter", - ) in model.decorations: - blocked[dim_index] = "ambiguous hole tolerance ownership: bore already has a tolerance" + if conflict := _hole_ownership_conflict(owner, _member_indices, model.decorations): + blocked[dim_index] = conflict del proposals[dim_index] # A member cannot carry two different imported requirements. Equal repeats are one @@ -204,7 +277,7 @@ def lower_ap242_hole_tolerances( by_member[(owner_index, member_index)].append(dim_index) for dim_indices in by_member.values(): active = [index for index in dim_indices if index in proposals] - values = {proposals[index][2] for index in active} + values = {(proposals[index][2], _limit_bounds(dimensions[index])) for index in active} if len(values) > 1: for dim_index in active: blocked[dim_index] = ( @@ -237,22 +310,16 @@ def lower_ap242_hole_tolerances( continue if isinstance(feature, PatternFeature): - dim_indices = sorted({i for indices in member_requirements.values() for i in indices}) - pattern_value = proposals[dim_indices[0]][2] - ids = tuple( - dict.fromkeys( - source_id - for dim_index in dim_indices - for source_id in _source_ids(dimensions[dim_index]) + rebuilt.append( + _lower_pattern_member_sizes( + feature, + member_requirements, + proposals, + dimensions, + decorations, + feature_remap, ) ) - rebuilt.append(feature) - decorations[(feature, "diameter", "bore")] = ToleranceDecoration( - value=pattern_value, - source="ap242_pmi", - source_ids=ids, - limit_bounds=_limit_bounds(dimensions[dim_indices[0]]), - ) continue assert isinstance(feature, HoleFeature) @@ -509,7 +576,257 @@ def _nominal_owner_matches( return len(covered) == len(members) -def lower_ap242_nominal_diameters(model: PartModel) -> PartModel: +def _pattern_nominal_members( + dimension: AuthoredDimension, pattern: PatternFeature, bbox +) -> tuple[int, ...]: + """Match every source cylinder to exactly one physical pattern member.""" + if pattern.pattern not in ("grid", "linear") or not pattern.members: + return () + covered: set[int] = set() + for reference in dimension.cylindrical_refs: + matches = [ + index + for index, member in enumerate(pattern.members) + if _internal_member_matches(reference, pattern.member, member, bbox) + ] + if len(matches) != 1: + return () + covered.add(matches[0]) + return tuple(sorted(covered)) + + +def _lower_pattern_nominal_diameters( + model: PartModel, *, feature_remap: FeatureRemap | None +) -> PartModel: + """Give each exact pattern member its source nominal without losing its tolerance.""" + features = list(model.features) + decorations = dict(model.decorations) + consumed: set[int] = set() + blocked: dict[int, str] = {} + for dim_index, dimension in enumerate(model.features): + if not ( + isinstance(dimension, AuthoredDimension) + and dimension.dimension_kind == "diameter" + and dimension.source == "ap242_pmi" + and dimension.cylindrical_refs + and not dimension.lowering_blockers + and not dimension.rendering_blockers + and all( + value is None + for value in ( + dimension.lower_tol, + dimension.upper_tol, + dimension.lower_bound, + dimension.upper_bound, + ) + ) + ): + continue + matches = [ + (index, feature, member_indices) + for index, feature in enumerate(features) + if isinstance(feature, PatternFeature) + and (member_indices := _pattern_nominal_members(dimension, feature, model.bbox)) + and (len(member_indices) < feature.count or feature.member_size_requirements) + ] + if not matches: + continue + other_matches = [ + feature + for feature in features + if isinstance(feature, StepFeature | BossFeature | HoleFeature | RotationalFeature) + and _nominal_owner_matches(dimension, feature, model.bbox) + ] + if len(matches) != 1 or other_matches: + blocked[dim_index] = ( + "ambiguous diameter ownership: source cylinders match multiple features" + ) + continue + owner_index, owner, member_indices = matches[0] + nominal = NominalRequirement(dimension.value, "ap242_pmi", _source_ids(dimension)) + if not nominal.agrees_with(owner.member.diameter): + blocked[dim_index] = ( + f"source nominal {dimension.value!r} disagrees with canonical " + f"bore.diameter={owner.member.diameter!r}" + ) + continue + if (owner, "diameter", "bore") in decorations or (owner, "diameter") in decorations: + blocked[dim_index] = ( + "ambiguous diameter ownership: pattern bore has an authored aspect" + ) + continue + requirements = list(owner.member_size_requirements or (None,) * owner.count) + if any( + requirement is not None and requirement.source != "ap242_pmi" + for index in member_indices + if (requirement := requirements[index]) is not None + ): + blocked[dim_index] = "ambiguous diameter ownership: member has an authored aspect" + continue + for index in member_indices: + requirement = requirements[index] + requirements[index] = ( + nominal + if requirement is None + else replace( + requirement, + source_ids=tuple( + dict.fromkeys((*requirement.source_ids, *nominal.source_ids)) + ), + ) + ) + replacement = replace(owner, member_size_requirements=tuple(requirements)) + for key, value in tuple(decorations.items()): + if key[0] is owner: + del decorations[key] + decorations[(replacement, *key[1:])] = value + if feature_remap is not None: + feature_remap(owner, (replacement,), (tuple(range(owner.count)),)) + features[owner_index] = replacement + consumed.add(dim_index) + if not consumed and not blocked: + return model + return replace( + model, + features=[ + _block(feature, blocked[index]) + if index in blocked and isinstance(feature, AuthoredDimension) + else feature + for index, feature in enumerate(features) + if index not in consumed + ], + decorations=decorations, + ) + + +def lower_ap242_external_diameter_tolerances(model: PartModel) -> PartModel: + """Attach source limits to an exactly matched external cylinder measurement.""" + owners = tuple( + feature + for feature in model.features + if isinstance(feature, StepFeature | BossFeature | RotationalFeature) + ) + candidates = { + index: feature + for index, feature in enumerate(model.features) + if isinstance(feature, AuthoredDimension) + and feature.source == "ap242_pmi" + and feature.dimension_kind == "diameter" + and feature.cylindrical_refs + and all(reference.sense == "external" for reference in feature.cylindrical_refs) + and any( + value is not None + for value in ( + feature.lower_tol, + feature.upper_tol, + feature.lower_bound, + feature.upper_bound, + ) + ) + } + if not candidates: + return model + proposals: dict[ + int, tuple[StepFeature | BossFeature | RotationalFeature, ToleranceDecoration] + ] = {} + blocked: dict[int, str] = {} + for index, dimension in candidates.items(): + if dimension.lowering_blockers or dimension.rendering_blockers: + continue + matches = [ + owner + for owner in owners + if _nominal_owner_matches(dimension, owner, model.bbox) + and _same_number(dimension.value, _external_diameter(owner)) + ] + if any(isinstance(owner, StepFeature) for owner in matches): + matches = [owner for owner in matches if isinstance(owner, StepFeature)] + elif any(isinstance(owner, RotationalFeature) for owner in matches): + matches = [owner for owner in matches if isinstance(owner, RotationalFeature)] + if len(matches) != 1: + blocked[index] = ( + "unmatched external diameter ownership: no exact canonical cylinder" + if not matches + else f"ambiguous external diameter ownership: {len(matches)} canonical cylinders" + ) + continue + try: + tolerance = _requirement(dimension) + except ValueError as exc: + blocked[index] = f"unsupported external diameter tolerance: {exc}" + continue + assert tolerance is not None + proposals[index] = ( + matches[0], + ToleranceDecoration( + value=tolerance, + source="ap242_pmi", + source_ids=_source_ids(dimension), + limit_bounds=_limit_bounds(dimension), + ), + ) + by_owner: dict[StepFeature | BossFeature | RotationalFeature, list[int]] = defaultdict(list) + for index, (owner, _requirement_value) in proposals.items(): + by_owner[owner].append(index) + decorations = dict(model.decorations) + consumed: set[int] = set() + for owner, indices in by_owner.items(): + parameter = _external_diameter_parameter(owner) + key = (owner, "diameter", parameter.role) + values = {proposals[index][1].value for index in indices} + bounds = {proposals[index][1].limit_bounds for index in indices} + if ( + len(values) != 1 + or len(bounds) != 1 + or key in decorations + or ( + owner, + "diameter", + ) + in decorations + ): + for index in indices: + blocked[index] = "ambiguous external diameter tolerance ownership" + continue + first = proposals[indices[0]][1] + decorations[key] = replace( + first, + source_ids=tuple( + dict.fromkeys( + source for index in indices for source in proposals[index][1].source_ids + ) + ), + ) + consumed.update(indices) + return replace( + model, + features=[ + _block(feature, blocked[index]) + if index in blocked and isinstance(feature, AuthoredDimension) + else feature + for index, feature in enumerate(model.features) + if index not in consumed + ], + decorations=decorations, + ) + + +def _external_diameter(feature: StepFeature | BossFeature | RotationalFeature) -> float: + return float(feature.od if isinstance(feature, RotationalFeature) else feature.diameter) + + +def _external_diameter_parameter(feature: StepFeature | BossFeature | RotationalFeature): + return next( + parameter + for parameter in feature.parameters() + if parameter.kind == "diameter" + and parameter.role == ("od" if isinstance(feature, RotationalFeature) else feature.kind) + ) + + +def lower_ap242_nominal_diameters( + model: PartModel, *, feature_remap: FeatureRemap | None = None +) -> PartModel: """Give untoleranced Size_Diameter PMI to an existing canonical diameter owner. Correlation uses the referenced cylinder's topology axis, line, finite span, polarity, @@ -517,6 +834,7 @@ def lower_ap242_nominal_diameters(model: PartModel) -> PartModel: that cannot prove one owner remains an :class:`AuthoredDimension`; its typed cylinder is still sufficient for the shared standalone placement path (#1296). """ + model = _lower_pattern_nominal_diameters(model, feature_remap=feature_remap) targets: list[ tuple[ int, @@ -1806,7 +2124,10 @@ def lower_ap242_dimensions( """Run every geometry-correlated AP242 lowering at the IR waist.""" dimensions = lower_ap242_nominal_step_lengths( lower_ap242_nominal_diameters( - lower_ap242_hole_tolerances(model, feature_remap=feature_remap) + lower_ap242_external_diameter_tolerances( + lower_ap242_hole_tolerances(model, feature_remap=feature_remap) + ), + feature_remap=feature_remap, ) ) manufacturing = lower_ap242_manufacturing_requirements(dimensions, feature_remap=feature_remap) diff --git a/src/draftwright/sheet_emit.py b/src/draftwright/sheet_emit.py index 739b966d..dde4da19 100644 --- a/src/draftwright/sheet_emit.py +++ b/src/draftwright/sheet_emit.py @@ -1514,6 +1514,15 @@ def _declaration_metadata(model, source_feature_ids, source_detected, declaratio return declaration_metadata +def _pattern_requirement_imports(model) -> set[str]: + return { + type(requirement).__name__ + for feature in model.features + for requirement in getattr(feature, "member_size_requirements", ()) + if isinstance(requirement, ToleranceDecoration | NominalRequirement) + } + + def _model_constructor_imports(model): """Find constructor names potentially used by the emitted feature declarations.""" # Every constructor a member template can name has to be listed here. The pattern verbs @@ -1526,6 +1535,7 @@ def _model_constructor_imports(model): model_imports.add("AngularReference") if any(f.kind in ("hole", "pattern") for f in model.features): model_imports.add("hole") + model_imports.update(_pattern_requirement_imports(model)) if any( f.kind == "pattern" and getattr(f.member, "profile", None) == "double_d" for f in model.features diff --git a/src/draftwright/sheet_feature_lines.py b/src/draftwright/sheet_feature_lines.py index a79195ed..913aa938 100644 --- a/src/draftwright/sheet_feature_lines.py +++ b/src/draftwright/sheet_feature_lines.py @@ -700,6 +700,12 @@ def _stock_feature_line( raise AssertionError(f"unexpected stock feature kind: {k}") +def _pattern_member_size_arguments(feature) -> tuple[str, ...]: + if not feature.member_size_requirements: + return () + return (f"member_size_requirements={feature.member_size_requirements!r}",) + + def _machined_feature_line(f, *, exact_parameter: str | None) -> str: """Emit machined features and their repeated arrangements.""" k = f.kind @@ -822,6 +828,7 @@ def _machined_feature_line(f, *, exact_parameter: str | None) -> str: parts.append(f"angle={_n(f.angle)}") if f.members: parts.append("members=[" + ", ".join(_pt(p) for p in f.members) + "]") + parts.extend(_pattern_member_size_arguments(f)) return ( f"sheet.pattern({_member_hole_str(f.member, exact_parameter=exact_parameter)}, " + ", ".join(parts) diff --git a/tests/_tier_manifest.py b/tests/_tier_manifest.py index 9050c010..937e4587 100644 --- a/tests/_tier_manifest.py +++ b/tests/_tier_manifest.py @@ -280,6 +280,7 @@ class ContractGroup: ( "test_issue_1298_manufacturing_requirements.py", "test_manufacturing_pmi_lowering.py", + "test_part_model.py", ), ), "slot_rendering": ContractGroup( @@ -781,6 +782,7 @@ class ContractGroup: ), "src/draftwright/model/manufacturing_schedule.py": ("test_manufacturing_schedule.py",), "src/draftwright/model/pmi_lowering.py": ( + "test_issue_1296_cylindrical_diameter_pmi.py", "test_manufacturing_pmi_lowering.py", "test_nominal_step_pmi.py", ), diff --git a/tests/test_arrangement_gate.py b/tests/test_arrangement_gate.py index b6a2059b..7526fa59 100644 --- a/tests/test_arrangement_gate.py +++ b/tests/test_arrangement_gate.py @@ -329,6 +329,24 @@ def test_an_alternative_that_loses_something_else_is_rejected(self): == "preferred" ) + def test_incomplete_alternative_preserves_more_source_pmi(self): + shared = {**_blocker("pmi_dropped", "datum"), "source_ids": ("datum:A",)} + source_loss = { + **_blocker("pmi_dropped", "position"), + "source_ids": ("geometric_tolerance:position",), + } + inferred = [ + {**_blocker("off_axis_location_dropped", "hole_x_left"), "severity": "info"}, + {**_blocker("off_axis_location_dropped", "hole_x_right"), "severity": "info"}, + ] + + assert self._decide([shared, *inferred], [shared, source_loss]) == "alternative" + assert self._decide([shared, source_loss], [shared, inferred[0]]) == "preferred" + assert ( + self._decide([shared, {**inferred[0], "severity": "error"}], [shared, source_loss]) + == "preferred" + ) + def test_an_alternative_losing_the_same_thing_is_not_penalised(self): # The converse, so the rule is not simply "always reject": a blocker the default # produces too is not the alternative's fault, and it keeps its smaller sheet. diff --git a/tests/test_issue_1116_ap242_hole_tolerance_lowering.py b/tests/test_issue_1116_ap242_hole_tolerance_lowering.py index 3cd9a10f..45916f50 100644 --- a/tests/test_issue_1116_ap242_hole_tolerance_lowering.py +++ b/tests/test_issue_1116_ap242_hole_tolerance_lowering.py @@ -9,7 +9,7 @@ from build123d import Box, import_step from draftwright import Sheet -from draftwright.builder import detect_part_model +from draftwright.builder import build_drawing, detect_part_model from draftwright.model.ir import ( AuthoredDimension, Frame, @@ -235,7 +235,10 @@ def test_pattern_wide_requirement_preserves_pattern_identity_and_membership(): ) -def test_partial_pattern_and_ambiguous_matches_fall_back_with_explicit_reasons(): +def test_partial_pattern_keeps_member_scope_and_ambiguous_matches_fall_back(): + from draftwright.model.callout import hole_callout_batches + from draftwright.model.planner import plan_dimensions + members = ((-10.0, 0.0, 0.0), (10.0, 0.0, 0.0)) pattern = PatternFeature( frame=Frame((0.0, 0.0, 0.0), "z"), @@ -252,23 +255,27 @@ def test_partial_pattern_and_ambiguous_matches_fall_back_with_explicit_reasons() bbox=(-15.1, -5.1, -0.1, -4.9, 5.1, 8.1), ) partial_model = lower_ap242_hole_tolerances(_model(pattern, partial)) - fallback = next(f for f in partial_model.features if f.kind == "authored_dimension") - assert fallback.lowering_blockers == ( - "unsupported hole correlation: AP242 requirement covers only part of a canonical hole pattern", + (owned_pattern,) = partial_model.features + assert owned_pattern.member_size_requirements == ( + ToleranceDecoration(0.1, "ap242_pmi", ("dimension:test",)), + None, ) + batches = hole_callout_batches(plan_dimensions(partial_model)) + assert [(batch.locations, batch.spec["source_ids"]) for batch in batches] == [ + ((members[0],), ("dimension:test",)), + ((members[1],), ()), + ] source = emit_sheet_script(partial_model, "part", "fallback", title="P", number="N") - assert "lowering_blockers=('unsupported hole correlation:" in source + assert source.count("dimension:test") == 1 namespace = {"part": Box(40, 40, 10)} exec( # noqa: S102 compile(source[: source.index("drawing = sheet.build()")], "", "exec"), namespace, ) restored = next( - feature - for feature in namespace["sheet"].model().features - if feature.kind == "authored_dimension" + feature for feature in namespace["sheet"].model().features if feature.kind == "pattern" ) - assert restored.lowering_blockers == fallback.lowering_blockers + assert restored.member_size_requirements == owned_pattern.member_size_requirements duplicate = replace(_hole(), frame=Frame((0.0, 0.0, 0.0), "z")) ambiguous_model = lower_ap242_hole_tolerances( @@ -277,6 +284,53 @@ def test_partial_pattern_and_ambiguous_matches_fall_back_with_explicit_reasons() fallback = next(f for f in ambiguous_model.features if f.kind == "authored_dimension") assert fallback.lowering_blockers[0].startswith("ambiguous hole correlation:") + existing = replace( + pattern, + member_size_requirements=( + ToleranceDecoration(0.2, "declared", ("source:declared",)), + None, + ), + ) + conflict = lower_ap242_hole_tolerances(_model(existing, partial)) + assert conflict.features[0] == existing + assert conflict.features[1].lowering_blockers == ( + "ambiguous hole tolerance ownership: member already has a source requirement", + ) + + +def test_editable_callout_refuses_a_mixed_member_tolerance_pattern(): + from draftwright.model.callout import hole_callout_batches + from draftwright.model.planner import plan_dimensions + + members = ((-10.0, 0.0, 0.0), (10.0, 0.0, 0.0)) + pattern = PatternFeature( + frame=Frame((0.0, 0.0, 0.0), "z"), + pattern="linear", + count=2, + member=_hole(at=members[0], diameter=4.0), + members=members, + pitch=20.0, + direction=(1.0, 0.0, 0.0), + member_size_requirements=( + ToleranceDecoration(0.1, "ap242_pmi", ("member:0",)), + ToleranceDecoration(0.2, "ap242_pmi", ("member:1",)), + ), + ) + drawing = build_drawing(Box(40, 40, 10), model=_model(pattern), auto_dims=False) + owner = drawing.model().features[0] + batches = hole_callout_batches(plan_dimensions(drawing.model())) + assert [(batch.spec["count"], batch.spec["tolerance"]) for batch in batches] == [ + (None, 0.1), + (None, 0.2), + ] + + with pytest.raises(ValueError, match="separate member callouts"): + drawing.callout(owner) + with pytest.raises(ValueError, match="separate member callouts"): + with drawing.deferred(): + drawing.callout(owner) + assert not [name for name in drawing.annotations() if name.startswith("hc_")] + def test_unmatched_requirement_without_any_hole_owner_keeps_an_explicit_reason(): lowered = lower_ap242_hole_tolerances(_model(_dimension(lower_tol=0.1, upper_tol=0.1))) @@ -427,15 +481,20 @@ def test_ctc01_consumes_all_hole_tolerances_once_and_emits_provenance(): assert [(feature.dimension_kind, feature.source_id) for feature in authored] == [ ("angular", "dimension:0:1:4:17"), - *(("diameter", f"dimension:0:1:4:{index}") for index in (21, 22, 25, 26, 29)), ] - assert all( - feature.lowering_blockers - == ( - "unsupported hole correlation: AP242 requirement covers only part of a canonical hole pattern", - ) - for feature in authored[1:] - ) + patterns = { + feature.member.diameter: feature + for feature in model.features + if isinstance(feature, PatternFeature) + } + assert [ + requirement.source_ids if requirement is not None else () + for requirement in patterns[35.0].member_size_requirements + ] == [(f"dimension:0:1:4:{index}",) for index in (21, 22, 25, 26)] + assert [ + requirement.source_ids if requirement is not None else () + for requirement in patterns[25.0].member_size_requirements + ] == [(), (), ("dimension:0:1:4:29",), ()] tolerance_source_ids = { "dimension:0:1:4:21", "dimension:0:1:4:22", @@ -449,12 +508,16 @@ def test_ctc01_consumes_all_hole_tolerances_once_and_emits_provenance(): source_id for requirement in requirements for source_id in requirement.source_ids } assert decorated_source_ids == {"dimension:0:1:4:23", "dimension:0:1:4:24"} - assert ( - decorated_source_ids | {feature.source_id for feature in authored[1:]} - == tolerance_source_ids - ) + member_source_ids = { + source_id + for pattern in patterns.values() + for requirement in pattern.member_size_requirements + if requirement is not None + for source_id in requirement.source_ids + } + assert decorated_source_ids | member_source_ids == tolerance_source_ids source = emit_sheet_script(model, "part", "ctc01", title="CTC01", number="N") - assert source.count("sheet.measured_dimension(") == 6 + assert source.count("sheet.measured_dimension(") == 1 expected_source_ids = tolerance_source_ids | {"dimension:0:1:4:17"} assert source.count("source='ap242_pmi'") == len(expected_source_ids) assert all(source.count(repr(source_id)) == 1 for source_id in expected_source_ids) diff --git a/tests/test_issue_1296_cylindrical_diameter_pmi.py b/tests/test_issue_1296_cylindrical_diameter_pmi.py index 1180d3c9..02341170 100644 --- a/tests/test_issue_1296_cylindrical_diameter_pmi.py +++ b/tests/test_issue_1296_cylindrical_diameter_pmi.py @@ -215,6 +215,191 @@ def test_finite_span_and_axis_line_disambiguate_equal_nominal_steps(): ) +def test_toleranced_external_diameter_joins_the_recognized_step_once_issue_2172(): + step = StepFeature( + frame=Frame((-1.6, 0.0, 0.0), "x"), + length=3.2, + diameter=4.0, + span=((-3.2, 0.0, 0.0), (0.0, 0.0, 0.0)), + ) + source = replace( + _dimension(_cylinder()), + label="ø4 +0.2/-0.1", + lower_tol=0.1, + upper_tol=0.2, + ) + model = PartModel(Box(20, 10, 10).bounding_box(), "x", [step, source]) + lowered = lower_ap242_dimensions(model) + + assert lowered.features == [step], "the source and geometry still make one measurement" + assert lowered.decorations[(step, "diameter", "step")] == ToleranceDecoration( + (0.1, 0.2), "ap242_pmi", ("dimension:test",) + ) + group = next(group for group in plan_dimensions(lowered) if group.feature is step) + diameter = next(pd.param for pd in group.dims if pd.param.parameter_id == "step.diameter") + assert diameter.tolerance == (0.1, 0.2) + assert diameter.source_ids == ("dimension:test",) + + +def test_external_diameter_refuses_conflicting_source_tolerances_issue_2172(): + step = StepFeature(Frame((-1.6, 0.0, 0.0), "x"), 3.2, 4.0, ((-3.2, 0.0, 0.0), (0.0, 0.0, 0.0))) + source = _dimension(_cylinder()) + first = replace(source, source_id="dimension:first", lower_tol=0.1, upper_tol=0.2) + second = replace(source, source_id="dimension:second", lower_tol=0.1, upper_tol=0.3) + assert first.upper_tol != second.upper_tol + assert first.cylindrical_refs == second.cylindrical_refs + lowered = lower_ap242_dimensions( + PartModel(Box(20, 10, 10).bounding_box(), "x", [step, first, second]) + ) + + assert (step, "diameter", "step") not in lowered.decorations + remaining = [feature for feature in lowered.features if isinstance(feature, AuthoredDimension)] + assert {feature.source_id for feature in remaining} == { + "dimension:first", + "dimension:second", + } + assert all( + feature.lowering_blockers == ("ambiguous external diameter tolerance ownership",) + for feature in remaining + ) + + +def test_external_diameter_refuses_invalid_limits_without_consuming_source_issue_2172(): + step = StepFeature(Frame((-1.6, 0.0, 0.0), "x"), 3.2, 4.0, ((-3.2, 0.0, 0.0), (0.0, 0.0, 0.0))) + source = replace(_dimension(_cylinder()), lower_bound=5.0, upper_bound=6.0) + assert source.lower_bound > source.value + lowered = lower_ap242_dimensions( + PartModel(Box(20, 10, 10).bounding_box(), "x", [step, source]) + ) + + assert lowered.features[0] is step + assert isinstance(lowered.features[1], AuthoredDimension) + assert lowered.features[1].source_id == source.source_id + assert lowered.features[1].lowering_blockers == ( + "unsupported external diameter tolerance: negative deviation magnitude", + ) + assert (step, "diameter", "step") not in lowered.decorations + + +def test_nominal_pattern_member_joins_once_and_keeps_other_member_generic_issue_2172(): + from draftwright.model.callout import hole_callout_batches + + members = ((0.0, -5.0, 0.0), (0.0, 5.0, 0.0)) + hole = HoleFeature(Frame(members[0], "x"), 4.0, 20.0, True) + pattern = PatternFeature( + frame=Frame((0.0, 0.0, 0.0), "x"), + pattern="linear", + count=2, + member=hole, + members=members, + pitch=10.0, + direction=(0.0, 1.0, 0.0), + ) + first = _cylinder(interval=(-10.0, 10.0), axis_origin=members[0], sense="internal") + source = _dimension(first, "dimension:first-member") + lowered = lower_ap242_nominal_diameters( + PartModel(Box(20, 20, 20).bounding_box(), "x", [pattern, source]) + ) + + (owned,) = lowered.features + assert isinstance(owned, PatternFeature) + assert owned.member_size_requirements == ( + NominalRequirement(4.0, "ap242_pmi", ("dimension:first-member",)), + None, + ) + batches = hole_callout_batches(plan_dimensions(lowered)) + assert [(batch.locations, batch.spec["source_ids"]) for batch in batches] == [ + ((members[0],), ("dimension:first-member",)), + ((members[1],), ()), + ] + + source_script = emit_sheet_script(lowered, "part", "nominal_members", title="P", number="N") + namespace = {"part": Box(20, 20, 20)} + exec( # noqa: S102 + compile( + source_script[: source_script.index("drawing = sheet.build()")], "", "exec" + ), + namespace, + ) + restored = next( + feature + for feature in namespace["sheet"].model().features + if isinstance(feature, PatternFeature) + ) + assert restored.member_size_requirements == owned.member_size_requirements + + +def test_nominal_pattern_member_refuses_ambiguous_or_authored_ownership_issue_2172(): + members = ((0.0, -5.0, 0.0), (0.0, 5.0, 0.0)) + hole = HoleFeature(Frame(members[0], "x"), 4.0, 20.0, True) + pattern = PatternFeature( + frame=Frame((0.0, 0.0, 0.0), "x"), + pattern="linear", + count=2, + member=hole, + members=members, + pitch=10.0, + direction=(0.0, 1.0, 0.0), + ) + source = _dimension( + _cylinder(interval=(-10.0, 10.0), axis_origin=members[0], sense="internal"), + "dimension:first-member", + ) + assert source.cylindrical_refs[0].sense == "internal" + bbox = Box(20, 20, 20).bounding_box() + + overlapping_owner = replace(pattern) + assert overlapping_owner is not pattern and overlapping_owner == pattern + ambiguous = lower_ap242_nominal_diameters( + PartModel(bbox, "x", [pattern, overlapping_owner, source]) + ) + assert [feature.member_size_requirements for feature in ambiguous.features[:2]] == [(), ()] + assert ambiguous.features[2].lowering_blockers == ( + "ambiguous diameter ownership: source cylinders match multiple features", + ) + + authored = lower_ap242_nominal_diameters( + PartModel(bbox, "x", [pattern, source], decorations={(pattern, "diameter", "bore"): 0.2}) + ) + assert authored.features[0] is pattern + assert authored.features[1].lowering_blockers == ( + "ambiguous diameter ownership: pattern bore has an authored aspect", + ) + assert authored.decorations[(pattern, "diameter", "bore")] == 0.2 + + +def test_group_nominal_coowns_members_after_one_member_gains_tolerance_issue_2172(): + members = ((0.0, -5.0, 0.0), (0.0, 5.0, 0.0)) + hole = HoleFeature(Frame(members[0], "x"), 4.0, 20.0, True) + pattern = PatternFeature( + frame=Frame((0.0, 0.0, 0.0), "x"), + pattern="linear", + count=2, + member=hole, + members=members, + pitch=10.0, + direction=(0.0, 1.0, 0.0), + member_size_requirements=( + ToleranceDecoration(0.1, "ap242_pmi", ("dimension:tolerance",)), + None, + ), + ) + references = tuple( + _cylinder(interval=(-10.0, 10.0), axis_origin=member, sense="internal") + for member in members + ) + source = replace(_dimension(references[0], "dimension:group"), cylindrical_refs=references) + lowered = lower_ap242_nominal_diameters( + PartModel(Box(20, 20, 20).bounding_box(), "x", [pattern, source]) + ) + + (owned,) = lowered.features + assert owned.member_size_requirements == ( + ToleranceDecoration(0.1, "ap242_pmi", ("dimension:tolerance", "dimension:group")), + NominalRequirement(4.0, "ap242_pmi", ("dimension:group",)), + ) + + def test_nominal_hole_ownership_coexists_with_bore_tolerance_and_round_trips(): hole = HoleFeature(Frame((0.0, 0.0, 0.0), "x"), 4.0, 10.0, True) nominal = _dimension( diff --git a/tests/test_pareto_loop_canary_issue_1753.py b/tests/test_pareto_loop_canary_issue_1753.py index e1656c95..049401e2 100644 --- a/tests/test_pareto_loop_canary_issue_1753.py +++ b/tests/test_pareto_loop_canary_issue_1753.py @@ -27,9 +27,12 @@ def _fixed_requirements() -> tuple[ExpectedRequirement, ...]: rows: list[tuple[str, str]] = [] # The physical four-hole lattices are Ø25 at X±160/Y±45 and Ø35 at - # X±325/Y±175. Quiddity #791 gives each proved rectangle one grid owner. + # X±325/Y±175. Quiddity #791 gives each proved rectangle one grid owner; + # #2172 keeps its four source-sized members distinct within that owner. for declaration in (1, 2): - rows.append((f"declaration:{declaration}", "bore.diameter")) + rows.extend( + (f"declaration:{declaration}", f"bore.diameter.member_{member}") for member in range(4) + ) rows.extend( (f"declaration:{declaration}", parameter) for parameter in ( @@ -74,7 +77,7 @@ def _fixed_requirements() -> tuple[ExpectedRequirement, ...]: rows.extend((f"declaration:{declaration}", "chamfer.length") for declaration in range(10, 13)) rows.extend((f"declaration:{declaration}", "fillet.radius") for declaration in range(13, 20)) rows.extend((f"declaration:{declaration}", "blend.radius") for declaration in range(20, 48)) - assert len(rows) == 69 + assert len(rows) == 75 return tuple(ExpectedRequirement(*row) for row in rows) @@ -167,4 +170,4 @@ def test_ctc01_resolved_gdt_finding_is_not_offered_to_the_pareto_loop( assert { (row["declaration_id"], row["parameter_id"]) for row in baseline["measurements"]["entries"] } == expected - assert len(baseline["measurements"]["entries"]) == 69 + assert len(baseline["measurements"]["entries"]) == 75 diff --git a/tests/test_part_model.py b/tests/test_part_model.py index 830c44c1..eab13722 100644 --- a/tests/test_part_model.py +++ b/tests/test_part_model.py @@ -10,8 +10,9 @@ import inspect import pickle -from dataclasses import dataclass +from dataclasses import dataclass, replace +import pytest from build123d import Box, Cylinder, Pos from draftwright.model import ( @@ -20,15 +21,54 @@ DimParameter, Frame, HoleFeature, + NominalRequirement, PartModel, PatternFeature, StepFeature, + ToleranceDecoration, build_part_model, display, plan_dimensions, ) +@pytest.mark.parametrize( + ("changes", "message"), + ( + ({"member_size_requirements": (ToleranceDecoration(0.1, "ap242_pmi"),)}, "one entry"), + ({"pattern": "bolt_circle"}, "grid or linear"), + ({"members": ()}, "every physical member"), + ({"member_size_requirements": ("not a requirement", None)}, "typed source"), + ( + { + "member_size_requirements": ( + NominalRequirement(99.0, "ap242_pmi", ("dimension:bogus",)), + None, + ) + }, + "must agree with bore diameter", + ), + ), +) +def test_pattern_member_sizes_reject_incomplete_or_unaddressable_declarations_issue_2172( + changes, message +): + members = ((-10.0, 0.0, 0.0), (10.0, 0.0, 0.0)) + pattern = PatternFeature( + frame=Frame((0.0, 0.0, 0.0), "z"), + pattern="linear", + count=2, + member=HoleFeature(Frame(members[0], "z"), 4.0, 8.0, True), + members=members, + pitch=20.0, + direction=(1.0, 0.0, 0.0), + member_size_requirements=(ToleranceDecoration(0.1, "ap242_pmi"), None), + ) + assert len(pattern.members) == len(pattern.member_size_requirements) == pattern.count + with pytest.raises(ValueError, match=message): + replace(pattern, **changes) + + def test_dimension_intent_exports_keep_ir_pickle_and_source_paths(): from draftwright.model import ir from draftwright.model.dimension_intent import _linear_projection_view diff --git a/tests/test_pmi.py b/tests/test_pmi.py index fcaab2a9..10b8d2de 100644 --- a/tests/test_pmi.py +++ b/tests/test_pmi.py @@ -1867,6 +1867,15 @@ def _single_source_dimension_drawing(**opts): class TestBuildDrawingPmi: + def test_ctc01_reconciled_pmi_keeps_automatic_a3_sheet(self, ctc01_annotated): + drawing = ctc01_annotated + assert (drawing.page_w, drawing.page_h, drawing.scale) == (420.0, 297.0, 0.2) + assert drawing.arrangement_decision["chosen"] == "staggered-side" + assert all( + "geometric_tolerance:0:1:4:5" not in blocker["source_ids"] + for blocker in drawing.scale_decision.get("blockers", ()) + ) + def test_explicit_pmi_off_reports_one_ignored_inventory_without_render_failures( self, tmp_path ): @@ -1972,7 +1981,7 @@ def test_pmi_report_extracts_but_does_not_annotate(self, tmp_path): } def test_pmi_annotate_adds_dims(self, ctc01_annotated): - """Singleton PMI uses bore callouts; patterned members retain distinct source marks.""" + """Source sizes ride the exact recognized member instead of a second dimension.""" from draftwright.model.ir import AuthoredDimension, PatternFeature, ToleranceDecoration requirements = [ @@ -1984,7 +1993,7 @@ def test_pmi_annotate_adds_dims(self, ctc01_annotated): "dimension:0:1:4:23", "dimension:0:1:4:24", } - standalone_ids = { + member_ids = { "dimension:0:1:4:21", "dimension:0:1:4:22", "dimension:0:1:4:25", @@ -1994,53 +2003,41 @@ def test_pmi_annotate_adds_dims(self, ctc01_annotated): standalone = [ feature for feature in ctc01_annotated.model().features - if isinstance(feature, AuthoredDimension) and feature.source_id in standalone_ids + if isinstance(feature, AuthoredDimension) and feature.source_id in member_ids ] - assert {feature.source_id for feature in standalone} == standalone_ids + assert standalone == [] patterns = [ feature for feature in ctc01_annotated.model().features if isinstance(feature, PatternFeature) ] assert len(patterns) == 2 - for dimension in standalone: - assert dimension.ref_bbox is not None - assert ( - sum( - pattern.member.diameter == dimension.value - and sum( - all( - dimension.ref_bbox[index] - 1e-6 - <= member[index] - <= dimension.ref_bbox[index + 3] + 1e-6 - for index in range(3) - ) - for member in pattern.members - ) - == 1 - for pattern in patterns - ) - == 1 - ) - assert all( - feature.lowering_blockers - == ( - "unsupported hole correlation: AP242 requirement covers only part of a canonical hole pattern", - ) - for feature in standalone - ) + assert { + source_id + for pattern in patterns + for requirement in pattern.member_size_requirements + if requirement is not None + for source_id in requirement.source_ids + } == member_ids assert all(ctc01_annotated.registry.names_for_feature(owner) for owner, _ in requirements) - assert all( - len(ctc01_annotated.registry.names_for_feature(feature)) == 1 for feature in standalone - ) + annotations = dict(ctc01_annotated.iter_annotations()) + for source_id in member_ids: + claims = [ + annotations[name] + for pattern in patterns + for name in ctc01_annotated.registry.names_for_feature(pattern) + if source_id + in { + source + for source, _measurement in getattr( + annotations[name], "source_measurements", () + ) + } + ] + assert len(claims) == 1 pmi_names = {name for name in ctc01_annotated.annotations() if name.startswith("pmi_")} - assert len(pmi_names) == 6 - assert any(name.startswith("pmi_angle_") for name in pmi_names) - assert { - name - for feature in standalone - for name in ctc01_annotated.registry.names_for_feature(feature) - } == {name for name in pmi_names if name.startswith("pmi_d_")} + assert len(pmi_names) == 1 + assert next(iter(pmi_names)).startswith("pmi_angle_") def test_pmi_annotate_places_each_source_surface_label_once(self, ctc01_annotated): from draftwright.model.ir import Note @@ -2068,35 +2065,33 @@ def test_pmi_annotate_places_each_source_surface_label_once(self, ctc01_annotate def test_pmi_annotate_keeps_equal_symmetric_nist_range_requirements_distinct( self, ctc01_annotated ): - from draftwright.model.ir import AuthoredDimension, PatternFeature, ToleranceDecoration + from draftwright.model.ir import PatternFeature source_ids = {"dimension:0:1:4:25", "dimension:0:1:4:26"} - requirements = [ + pattern = next( feature for feature in ctc01_annotated.model().features - if isinstance(feature, AuthoredDimension) and feature.source_id in source_ids - ] + if isinstance(feature, PatternFeature) and feature.member.diameter == 35 + ) annotations = dict(ctc01_annotated.iter_annotations()) - assert len(requirements) == 2 - assert {requirement.source_id for requirement in requirements} == source_ids - assert all(requirement.value == 35.0 for requirement in requirements) - assert all( - (requirement.lower_bound, requirement.upper_bound) == (34.8, 35.2) - for requirement in requirements - ) - assert not [ - value - for (owner, *_tail), value in ctc01_annotated.model().decorations.items() - if isinstance(owner, PatternFeature) and isinstance(value, ToleranceDecoration) + requirements = pattern.member_size_requirements[2:] + assert [requirement.source_ids for requirement in requirements] == [ + ("dimension:0:1:4:25",), + ("dimension:0:1:4:26",), ] - assert all( - any( - annotations[name].label == "ø34.8 - ø35.2" - for name in ctc01_annotated.registry.names_for_feature(requirement) - ) - for requirement in requirements - ) + assert all(requirement.limit_bounds == (34.8, 35.2) for requirement in requirements) + callouts = [ + annotations[name] + for name in ctc01_annotated.registry.names_for_feature(pattern) + if source_ids + == { + source + for source, _measurement in getattr(annotations[name], "source_measurements", ()) + } + ] + assert len(callouts) == 1 + assert callouts[0].label.startswith("2× ⌀35 ±0.2 THRU") def test_pmi_callouts_distinguish_source_dimensions_from_geometry_qualifiers( self, ctc01_annotated @@ -2242,14 +2237,8 @@ def test_pmi_annotate_accounts_for_each_typed_dimension_at_the_render_seam( } dropped = all_dropped & {feature.source_id for feature in authored} - # `dimension:0:1:4:17` is an ANGULAR dimension (label '60 ±0.5'). It used to render - # through the linear path, producing an annotation whose label states an angle and - # whose geometry states a length — the #1177 defect, present in a real NIST AP242 - # fixture. Its extracted planar supports now route it through the angular renderer, - # preserving the authored tolerance label. Two singleton diameter records are - # consumed as canonical bore decorations. Five member-specific diameter records - # retain their typed standalone render path because their canonical owners are - # four-member patterns with different source requirements per member. + # The angular source stays typed. All seven diameter sources now ride canonical + # bores: two singleton decorations and five exact pattern-member requirements. refused = { source_id for issue in ctc01_annotated.registry.issues @@ -2259,8 +2248,8 @@ def test_pmi_annotate_accounts_for_each_typed_dimension_at_the_render_seam( angular = { feature.source_id for feature in authored if feature.dimension_kind == "angular" } - assert len(authored) == 6 - assert rendered == angular | {f"dimension:0:1:4:{index}" for index in (21, 22, 25, 26, 29)} + assert len(authored) == 1 + assert rendered == angular assert len(dropped) == 0 assert refused == set() assert rendered.isdisjoint(dropped) and rendered.isdisjoint(refused) @@ -2279,8 +2268,7 @@ def test_pmi_annotate_accounts_for_each_typed_dimension_at_the_render_seam( }, "extracted": 31, "lowered": 27, - # Two diameter sources ride canonical bore owners, five member-specific - # diameter sources render as typed standalone PMI, the angular record renders + # Seven diameter sources ride canonical bores, the angular record renders # from its planar supports, and four raw location records remain unlowered. # Crowded GD&T and datum candidates may be dropped, but the source census must # account for them explicitly rather than report them as rendered. @@ -2419,9 +2407,12 @@ class TestDeclaredModelPmi: reproduces on the declared path. (The emitted Sheet-script round-trip is a separate gap — import_step strips AP242 PMI.)""" - def test_declared_model_annotate_matches_auto(self, tmp_path, ctc01_annotated): - # This checks source identity, not rendering parity. A fixed permissive - # sheet avoids a second automatic page search. + def test_declared_model_annotate_matches_auto(self, tmp_path): + from draftwright.builder import detect_part_model + + # This checks source identity, not rendering parity. Compare the + # automatic IR before layout so the test builds only the declared sheet. + automatic_model = detect_part_model(str(CTC01), pmi="annotate") declared = build_drawing( str(CTC01), out=str(tmp_path / "d"), @@ -2433,10 +2424,10 @@ def test_declared_model_annotate_matches_auto(self, tmp_path, ctc01_annotated): scale_policy="permissive", ) - def source_ids(drawing): + def source_ids(model): ids = { source_id - for feature in drawing.model().features + for feature in model.features for source_id in ( tuple(getattr(feature, "source_ids", ())) or ( @@ -2448,15 +2439,21 @@ def source_ids(drawing): } ids.update( source_id - for value in drawing.model().decorations.values() + for value in model.decorations.values() for source_id in getattr(value, "source_ids", ()) ) + ids.update( + source_id + for feature in model.features + for requirement in getattr(feature, "member_size_requirements", ()) + for source_id in getattr(requirement, "source_ids", ()) + ) return ids # With geometry features available the automatic path correlates hole requirements; # an empty declared model cannot, so it keeps them materialised. Both still account for # every extracted source identity — #472's no-loss invariant. - assert source_ids(ctc01_annotated) == source_ids(declared) + assert source_ids(automatic_model) == source_ids(declared.model()) def test_declared_model_pmi_off_stays_clean(self, tmp_path): # the synthesis is gated on pmi_mode == 'annotate' — a declared build without PMI stays 0 @@ -2485,6 +2482,9 @@ def test_synthetic_pmi_lowering_preserves_existing_declaration_identity(self, tm title="P", model=declared, pmi="annotate", + page="A2", + scale=0.2, + scale_policy="permissive", ) lowered_identities = drawing.model().declaration_identities diff --git a/tests/test_slot_completeness.py b/tests/test_slot_completeness.py index 7b7ca964..2bcb6612 100644 --- a/tests/test_slot_completeness.py +++ b/tests/test_slot_completeness.py @@ -14,8 +14,7 @@ from draftwright.model.compiled import compile_dimensions -def test_ctc_left_slot_position_survives_grid_pitch_carve(monkeypatch): - from draftwright.annotations import _slots +def test_ctc_left_slot_position_survives_grid_pitch_carve(): from draftwright.annotations._placement_occupancy import annotation_ink_clear source = Path(__file__).parent / "fixtures/nist_ctc_01_asme1_ap242.stp" @@ -52,11 +51,25 @@ def test_ctc_left_slot_position_survives_grid_pitch_carve(monkeypatch): ) assert not any(issue.code == "slot_dim_dropped" for issue in drawing.lint()) - # Removing the exact-ink retry exposes the original strip-capacity failure. - monkeypatch.setattr(_slots, "_slot_position_ink_candidates", lambda *_args, **_kwargs: ()) - without_retry = build_drawing(source, **options) - assert without_retry.get_annotation("m_slot0_pos") is None - assert any(issue.code == "slot_dim_dropped" for issue in without_retry.lint()) + +def test_unplaced_slot_position_keeps_a_bounded_ink_retry_issue_2172(): + from draftwright._core import Strip + from draftwright.annotations._placement_occupancy import strip_free_span + from draftwright.annotations._slots import _slot_position_retries + + strip = Strip(anchor=40.0, outer_limit=70.0, gap=2.0, spacing=4.0) + tier = 5.0 + lo, hi, _inner = strip_free_span(strip) + assert hi - lo - tier >= max(strip.spacing, 1.0) + + retry = _slot_position_retries( + SimpleNamespace(kind="slot"), "pos", strip, lambda position: position, tier + ) + assert retry is not None + candidates = tuple(retry(None)) + assert candidates + assert all(lo <= position <= hi - tier for position in candidates) + assert tuple(retry(object())) == () def _off_centre_slot(): diff --git a/tests/test_tier_manifest.py b/tests/test_tier_manifest.py index 979bf778..8e3e68e3 100644 --- a/tests/test_tier_manifest.py +++ b/tests/test_tier_manifest.py @@ -459,6 +459,12 @@ def test_oriented_slot_geometry_change_runs_its_semantics_contract(): def test_ir_foundation_change_runs_manufacturing_requirement_contract(): selected = pr_modules(_TESTS, ["src/draftwright/model/ir_foundation.py"]) assert "test_issue_1298_manufacturing_requirements.py" in selected + assert "test_part_model.py" in selected + + +def test_pmi_lowering_change_runs_source_diameter_ownership_contract(): + selected = pr_modules(_TESTS, ["src/draftwright/model/pmi_lowering.py"]) + assert "test_issue_1296_cylindrical_diameter_pmi.py" in selected def test_slot_renderer_change_runs_its_slot_and_pocket_behavior_contracts():