diff --git a/CHANGELOG.md b/CHANGELOG.md index e1c9512..648baf5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,89 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). As of `1. public API, the request and result schemas, the numeric and units policy, the validation rules and the compatibility policy are frozen: breaking any of them costs a major version. +## [1.1.0] + +An additive release on the 1.0.0 freeze. Route-aware unloading becomes a property of the +container rather than of the whole solve, and the result contract grows the reserved, +typed shape a future optimality gap needs. Every schema change is additive: a request that +omits the new field gets the answer it got before, and both frozen public API surfaces are +unchanged. + +The four engines' candidate hot paths were also profiled and rewritten. That work changes +no result — all 399 corpus fixtures are byte-identical — and it is in this release because +two of its findings were correctness fixes rather than speed. + +### Added + +- **Doors on a container: `container.access_directions`.** Naming which walls an item can + be pulled through, so that nothing due at a later stop stands between an earlier item and + a door. Previously this could only be stated once for a whole solve; a container now + states its own doors and falls back to that setting when it states none. An empty list + leaves the rule inert — it is not read as a sealed container, and it is deliberately not + read as all six walls, which would switch a real constraint on for callers who never set + the field. Implemented in all four engines, and refused by none. +- **A reserved shape for reporting an optimality gap.** The result schema now declares the + names, types and ceilings a gap will use, together with the rule that an attained bound + carries no gap at all rather than a zero. No engine emits any of them yet. The existing + extension point on that object stays open, so a producer accepted by the 1.0 schema is + still accepted by 1.1. + +### Changed + +- **The solvers do less repeated work.** Byte-identical results everywhere, with the + largest gains where a scene has many contacts: on a 120-item contact-heavy request the + Rust engine goes from 10.1 s to 1.4 s and the JavaScript fallback from 1.70 s to 0.76 s; + on a 200-item adversarial free-space scene JavaScript goes from 3.25 s to 0.58 s. Some + scenes are unchanged and one is marginally slower. **This is not a speed-leadership + claim** — measured against other libraries on identical hardware, that claim is false on + latency, and no Packvium surface makes it. +- **Every published package manifest now names its author, licence, repository, issue + tracker and homepage.** Absent fields are read as an anonymous package. +- **`@packvium/engine` no longer declares `@packvium/native` as an optional dependency.** + That package is not published, so the entry named something npm could not fetch. Nothing + a caller can observe changes: the install already succeeded and answered from the + JavaScript engine, and `index.js` still loads `@packvium/native` by literal specifier, so + installing it yourself alongside the engine still selects the compiled backend. + +### Fixed + +- **The Rust engine accepted placements the validator refused.** Support-ratio comparison + used a floating-point epsilon wide enough to admit an area a whole square tick short of + the requirement. It is now the same exact integer rule the other three engines use. +- **The JavaScript fallback accepted door names no other engine would.** An unknown + direction was refused only on requests that took the general solving path; a request + simple enough to be answered by the compact-grid shortcut was answered instead of + refused. +- **The JavaScript fallback broke identifier ties by host locale.** That is neither stable + across machines nor equal to the code-point order the other engines use, so two hosts + could order the same items differently. Every tie-break now uses the shared code-point + order. +- **A container's doors could be answered from another container's cached corridor.** Two + containers of the same size with different doors are two different questions; the cache + key did not separate them, which could have silently accepted a placement that walls an + item in. +- **The PHP package could not be loaded on PHP 7.3 or 7.4.** The package advertises + `php: >=7.3` and carries a second, downgraded source tree for runtimes below 8.2. In + `1.0.0` that tree contained one line of PHP 8.1 syntax, so the whole of it failed to + parse on both older runtimes. `1.1.0` is the first version whose legacy tree loads. If + you are on PHP 8.2 or newer you were never affected — the canonical tree is what your + runtime selects. + +- **The engine package no longer loads its native backend through a computed specifier.** + Every module the package can load can now be resolved by reading the source. Behaviour + is identical: an absent, unbuilt or incompatible native addon still means the pure + JavaScript engine answers instead. + +### Not claimed + +- **A reported optimality gap.** The names are reserved and typed; nothing emits them. +- **`container.pallet_overhang_limit`.** Reserved in the request schema and refused by all + four engines. +- **Identical placements across engines.** Unchanged from 1.0.0: different engines may + return different, equally valid arrangements, and that is measured and budgeted. +- **Optimal packings for arbitrary requests.** 3D packing remains NP-hard. +- **Fastest engine.** Still false on latency and still claimed nowhere. + ## [1.0.0] The stable core release. It freezes the contract that already exists rather than adding a diff --git a/README.md b/README.md index 5f6f80d..d5e6b06 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ dependencies**, exact integer geometry. Full documentation, the constraint reference and benchmarks live at [packvium.com](https://packvium.com). -> **Version 1.0.0 — the public API is frozen.** Field names, status codes and the +> **Version 1.1.0 — the public API is frozen.** Field names, status codes and the > objective vector do not change without a major version, so any `1.x` is a safe upgrade > from any earlier `1.x`. > Read [docs/GUARANTEES.md](https://github.com/toxakara/packvium-python/blob/main/docs/GUARANTEES.md) before relying on a result. diff --git a/docs/GUARANTEES.md b/docs/GUARANTEES.md index 3d2f188..34bed67 100644 --- a/docs/GUARANTEES.md +++ b/docs/GUARANTEES.md @@ -48,7 +48,7 @@ silently — if you need them, they belong in your own layer above this library. ## Status of this release -Version `1.0.0` freezes the public API. Field names, status codes, the objective +Version `1.1.0` freezes the public API. Field names, status codes, the objective vector, the numeric policy and the validation rules do not change without a major version, so any `1.x` is a safe upgrade from any earlier `1.x`. A caret or tilde constraint on `1.0` is enough; an exact pin is no longer required. diff --git a/docs/PUBLIC-API.md b/docs/PUBLIC-API.md index f403f3f..9996b03 100644 --- a/docs/PUBLIC-API.md +++ b/docs/PUBLIC-API.md @@ -90,7 +90,13 @@ move backwards. This is a testing seam, not a serialized request field. - three independent result facts: - `feasibility.code`: `feasible`, `infeasible`, or `unknown`; - `termination.code`: `complete`, `time_limit`, `effort_limit`, or `error`; - - `optimality.code`: `proven_optimal`, `proven_infeasible`, `best_found`, or `not_proven`; + - `optimality.code`: `proven_optimal`, `proven_infeasible`, `best_found`, or `not_proven`. + The 1.1.0 freeze reserved and typed `gap_key`, `absolute_gap` and `relative_gap` + alongside it. The pre-1.1.0 extension point remains open: closing the object would + make a previously valid producer fail a 1.1.0 validator, which is a breaking change + and therefore not legal in this minor release. No engine emits the reserved fields + yet, so `code` is still the only key any Packvium result carries. See + OPTIMALITY-CERTIFICATES.md; - legacy `status`: `optimal`, `feasible`, `best_found`, `time_limit`, `infeasible`, or `invalid_result`; - packed containers with exact coordinates, dimensions, rotations, and a `centre_of_mass_offset_ppm` — the weighted centre of mass's exact-integer Chebyshev diff --git a/docs/UNITS-AND-NUMERICS.md b/docs/UNITS-AND-NUMERICS.md index 9b1aa73..1badad2 100644 --- a/docs/UNITS-AND-NUMERICS.md +++ b/docs/UNITS-AND-NUMERICS.md @@ -22,6 +22,8 @@ Conversion supports floor, ceiling and ties-to-even nearest rounding. Applicatio Feasibility checks are integer-only, including the support ratio: the requested fraction is converted once to parts per million and compared as `supported_area >= floor(base_area x ratio_ppm / 10^6)`, never as a float division against an epsilon. The independent validator rechecks boundaries and intersections using exact integers. +The Rust solver and validator were the exception until 2026-09-02: both compared `area / base_area + 1e-12 < ratio` in `f64`. On a real-sized base (a square centimetre is `2.56 x 10^10` square ticks) an area one square tick short of the requirement sits well inside that epsilon, so Rust admitted a placement the shared validator refused. Both now go through `support_area_sufficient`, the rule above; the `f64` ratio survives only as the reported `support_ratio` on the placement record. + Ordering keys are exact too. Two volumes that differ by one cubic tick must not collapse onto the same value, or two implementations of one algorithm can order the same items differently. ## PHP integer limits diff --git a/examples/serialization.py b/examples/serialization.py index 8f74fdb..9db0828 100644 --- a/examples/serialization.py +++ b/examples/serialization.py @@ -39,6 +39,14 @@ "profile": "balanced", "seed": 42, "top_k": 2, + # An example must not change answer merely because the machine is busy. `top_k` + # asks the portfolio for runners-up, and how many it finds is bounded by the + # *wall clock* unless a budget says otherwise -- so without this line two runs on + # a loaded host can print a different number of alternatives, which is + # exactly. gave every conformance fixture an explicit budget for this + # reason; the examples were not part of that sweep. The value is a safety fuse, + # not a target: nothing here comes close to it. + "time_limit_ms": 60_000, }, "items": [ { @@ -131,14 +139,14 @@ # And a field this engine has named as not-yet-implemented is refused explicitly, so a # request written for a newer engine fails loudly instead of being half-honoured. The list -# below is the engine's own constant, and it is empty: implemented `convex_hull` +# below is the engine's own constant, and it is empty: implemented `convex_hull` # and `compressible`, the last reserved names left on it, so this engine now serves every # field and every `shape_type` value the schema defines. print("fields this engine refuses by name:", {scope: fields for scope, fields in UNSUPPORTED_FIELDS.items() if fields} or "none") # Caught up is the right state and a poor demonstration, so the guard takes its lists as -# parameters -- the same hook its own tests use. Passing the value retired shows +# parameters -- the same hook its own tests use. Passing the value retired shows # the refusal a caller still gets from an engine that is behind, and shows it naming the # *value* rather than the field: `rigid_cuboid` is the default and is implemented, so a # caller who spells the default out must be served, not refused. diff --git a/examples/shapes.py b/examples/shapes.py index 15504e7..83ca716 100644 --- a/examples/shapes.py +++ b/examples/shapes.py @@ -19,6 +19,10 @@ request runs unchanged against the Python, PHP, Rust and JavaScript engines. """ +# `list | None` in a signature is PEP 604, which needs Python 3.10 at runtime. This +# package supports 3.9, so the annotation is deferred rather than evaluated. +from __future__ import annotations + from packvium import pack_from_dict diff --git a/pyproject.toml b/pyproject.toml index 60a60ef..0d01aa7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "packvium" -version = "1.0.0" +version = "1.1.0" description = "Deterministic, extensible 3D cartonization and rectangular bin-packing library" readme = "README.md" requires-python = ">=3.9" diff --git a/src/packvium/bounds.py b/src/packvium/bounds.py index b0196a2..2d51b4f 100644 --- a/src/packvium/bounds.py +++ b/src/packvium/bounds.py @@ -3,8 +3,8 @@ The mathematics is fixed by [docs/OPTIMALITY-CERTIFICATES.md](../../../docs/OPTIMALITY-CERTIFICATES.md) and `scripts/optimality_bounds.py` is the independent oracle. This module is written from the document and never imports the oracle, so the property tests compare two -implementations rather than one implementation with itself -- the discipline set -for irregular items and restated here. +implementations rather than one implementation with itself -- the discipline set +for irregular items and restated here. What a bound is for. Every solver in this project is a heuristic: it returns an arrangement and has no notion of what it did not try. A bound is the other half of that @@ -22,7 +22,7 @@ are *dropped* rather than scaled -- a bound that is sometimes wrong is not a bound. `convex_hull` and `compressible` have exactly the same property and the document does not -say so, because was written before the irregular shapes existed. A hull occupies +say so, because was written before the irregular shapes existed. A hull occupies its hull rather than its bounding box, and a compressible item gives up height under load; summing nominal box volumes over-states what a solution must carry, which over-states the container count. diff --git a/src/packvium/constraints.py b/src/packvium/constraints.py index 6d66a37..2a37ed0 100644 --- a/src/packvium/constraints.py +++ b/src/packvium/constraints.py @@ -81,6 +81,44 @@ class PlacementConstraint(Protocol): def evaluate(self, context: ConstraintContext) -> ConstraintResult: ... +def active_constraints(constraints: Sequence[PlacementConstraint], container: Container, item: ItemInstance, + stack_sensitive: bool, route_sensitive: bool) -> list[PlacementConstraint]: + """The constraints that could reject *some* candidate of `item` in `container`. + + A candidate search evaluates the whole chain once per feasible position, and most rules + are opt-in: they begin by reading a flag on the item, the container or the search state + and allowing when it is off. Those flags are fixed for the duration of one search, so + the answer is read once here instead of once per position. A rule that is dropped would + have allowed every position, so the first rejecting rule -- and everything counted or + traced on the way to it -- is unchanged. Exact type checks are intentional: custom + constraints and subclasses with overridden behaviour are always evaluated, and an + unrelated extension method cannot accidentally opt into this internal optimisation. + """ + active: list[PlacementConstraint] = [] + for constraint in constraints: + constraint_type = type(constraint) + inert = ( + (constraint_type is FloorConstraint and not item.item.must_be_on_floor) + or (constraint_type is ContainerEligibilityConstraint + and (not item.item.eligible_container_tags + or bool(item.item.eligible_container_tags & container.tags))) + or (constraint_type is CompatibilityConstraint + and not item.item.tags and not item.item.incompatible_tags) + or (constraint_type is TagCountConstraint + and (not container.tag_limits + or not (item.item.tags & container.tag_limits.keys()))) + or (constraint_type is TopLoadConstraint and not stack_sensitive) + or (constraint_type is RouteOrderConstraint and not route_sensitive) + or (constraint_type is StopAccessibilityConstraint + and (not route_sensitive + or not (container.access_directions or constraint._default_directions))) + or (constraint_type is AxleLoadConstraint and container.axles is None) + ) + if not inert: + active.append(constraint) + return active + + class ProvenRejection(Protocol): """A constraint that can rule an item out of *every* offered container without searching for a placement first. @@ -293,7 +331,7 @@ def with_unit(self, unit: LoadUnit, cell_hint: int = 1) -> "LoadSupportGraph": """This graph plus one more unit, appended at the next index. The search evaluates many candidates against one unchanged set of placements, and - rebuilding the whole support graph for each of them was the cost exists to + rebuilding the whole support graph for each of them was the cost exists to remove. Adding a box cannot change contact between two boxes already placed, so the face graph only needs its two planes queried -- see `ContactGraph.with_box`. @@ -635,7 +673,7 @@ class TopLoadConstraint: only the box directly underneath, so a tower of light items cannot crush its base. The support graph over the *placed* boxes is the same for every candidate evaluated - against one search state, and rebuilding it per candidate was the cost + against one search state, and rebuilding it per candidate was the cost removes. One base per placement tuple is kept here and each candidate is appended to it. The cache is deliberately a single entry compared by identity: search evaluates a run of candidates against one state before moving on, so a one-entry cache captures @@ -760,12 +798,20 @@ class StopAccessibilityConstraint: neither implies the other; docs/STOP-ACCESSIBILITY.md derives the rule and the post-validator's whole-scene replay remains the sufficient check. - Opt-in twice over, and both are load-bearing. It is inert unless the caller supplies - exit directions, because the request schema has no field for them: assuming all six - walls open would enforce a rule that is true of no real vehicle and nearly vacuous - besides, since a box is almost always free through *some* face. And it is inert unless - two distinct stops are in play, which is what keeps a caller who never populates - `stop_index` paying nothing. + Opt-in twice over, and both are load-bearing. It is inert unless doors are stated, + because assuming all six walls open would enforce a rule that is true of no real + vehicle and nearly vacuous besides, since a box is almost always free through *some* + face. And it is inert unless two distinct stops are in play, which is what keeps a + caller who never populates `stop_index` paying nothing. + + **The doors come from the container, and fall back to the configuration.** + `container.access_directions` is a request field, and it has to be a per-container one: + two doors on one trailer and none on another is the case that makes the rule worth + having, and a solve opening several container types would otherwise have to pick one + answer for all of them. The constructor argument stays as the default so that the + library callers who drove this through `PackingConfig(access_directions=...)` before + the field existed keep working unchanged -- a container that states its own doors + overrides it, a container that states none inherits it. The blocker set is `{q : s(q) > s(p)}` -- strictly later. Items due at the *same* stop are excluded because the order within a stop is free: whichever is in the way comes off @@ -773,7 +819,8 @@ class StopAccessibilityConstraint: which is an ordinary load. """ - __slots__ = ("_directions", "_placements", "_container", "_clear", "_stops") + __slots__ = ("_default_directions", "_placements", "_container", "_directions", + "_clear", "_stops") def __init__(self, directions: Sequence[str] = ()) -> None: for direction in directions: @@ -781,13 +828,15 @@ def __init__(self, directions: Sequence[str] = ()) -> None: raise InvalidDirectionError(direction) # Deduplicated in the canonical order rather than as given: two callers passing the # same doors in different orders must search identically. - self._directions = tuple(d for d in ALL_DIRECTIONS if d in set(directions)) + self._default_directions = tuple(d for d in ALL_DIRECTIONS if d in set(directions)) self._placements: tuple[Placement, ...] | None = None self._container: Dimensions | None = None + self._directions: tuple[str, ...] = () self._clear: tuple[frozenset[str], ...] = () self._stops: tuple[float, ...] = () - def _base_for(self, placements: tuple[Placement, ...], container: Dimensions): + def _base_for(self, placements: tuple[Placement, ...], container: Dimensions, + directions: tuple[str, ...]): """Per placed box, the doors still open to it against the already-placed boxes. Cached by tuple identity for the same reason `TopLoadConstraint` does it: the @@ -797,7 +846,11 @@ def _base_for(self, placements: tuple[Placement, ...], container: Dimensions): # Keyed on the container as well as the placements, because a corridor runs to a # *wall*: the same boxes have different exits in a longer container, and reusing # the answer across two would silently accept a placement that walls an item in. - if self._placements is placements and self._container == container: + # And on the doors, since made them a property of the container rather than + # of the solve: two containers of the same size with different doors have different + # answers for the same boxes, and nothing else in the key separates them. + if (self._placements is placements and self._container == container + and self._directions == directions): return self._clear, self._stops stops = tuple(_stop_of(p) for p in placements) boxes = [p.envelope_box for p in placements] @@ -805,10 +858,10 @@ def _base_for(self, placements: tuple[Placement, ...], container: Dimensions): for index, box in enumerate(boxes): if stops[index] == RIDES_THE_WHOLE_ROUTE: # Never unloaded, so it needs no door of its own -- it only ever blocks. - clear.append(frozenset(self._directions)) + clear.append(frozenset(directions)) continue open_doors = frozenset( - direction for direction in self._directions + direction for direction in directions if not any(other != index and stops[other] > stops[index] and sweep_intersects(swept_volume(box, container, direction), boxes[other]) for other in range(len(boxes))) @@ -816,18 +869,23 @@ def _base_for(self, placements: tuple[Placement, ...], container: Dimensions): clear.append(open_doors) self._placements = placements self._container = container + self._directions = directions self._clear = tuple(clear) self._stops = stops return self._clear, self._stops def evaluate(self, context: ConstraintContext) -> ConstraintResult: - if not self._directions: return ConstraintResult.allow() if not context.route_sensitive: return ConstraintResult.allow() + # The container's own doors win; the configured tuple is what a container that + # states none inherits. `or` rather than a None check because both sides are + # already canonical tuples and "no doors" is the same answer either way. + directions = context.container.access_directions or self._default_directions + if not directions: return ConstraintResult.allow() candidate_stop = context.item.item.stop_index if candidate_stop is None: candidate_stop = RIDES_THE_WHOLE_ROUTE inner = context.container.inner_dimensions - clear, stops = self._base_for(context.placements, inner) + clear, stops = self._base_for(context.placements, inner, directions) # One distinct stop means nothing can be due before anything else, so no corridor # can be blocked by a later item. Checked over the candidate too, or the first @@ -858,7 +916,7 @@ def evaluate(self, context: ConstraintContext) -> ConstraintResult: not any(stops[other] > candidate_stop and sweep_intersects(swept_volume(candidate, inner, direction), boxes[other]) for other in range(len(boxes))) - for direction in self._directions + for direction in directions ): return ConstraintResult.reject( "stop_accessibility_violation", diff --git a/src/packvium/geometry.py b/src/packvium/geometry.py index 65629e7..e4b3904 100644 --- a/src/packvium/geometry.py +++ b/src/packvium/geometry.py @@ -2,6 +2,7 @@ from ._compat import dataclass from enum import Enum +from functools import lru_cache from itertools import product from typing import Iterable @@ -25,6 +26,16 @@ def upright(cls) -> tuple["Rotation", ...]: return (cls.LWH, cls.WLH) +# Which declared side lands on each axis under a rotation. A module-level table rather +# than a dict literal inside `rotated`: that method runs once per rotation per candidate +# search, and rebuilding six tuples there was measurable. +_ROTATION_AXES: dict[Rotation, tuple[int, int, int]] = { + Rotation.LWH: (0, 1, 2), Rotation.LHW: (0, 2, 1), + Rotation.WLH: (1, 0, 2), Rotation.WHL: (1, 2, 0), + Rotation.HLW: (2, 0, 1), Rotation.HWL: (2, 1, 0), +} + + class ShapeType(str, Enum): """How much of an item's declared box the item actually occupies. @@ -84,24 +95,12 @@ def max_edge(self) -> int: return max(self.length.ticks, self.width.ticks, self.height.ticks) def rotated(self, rotation: Rotation) -> "Dimensions": - l, w, h = self.length, self.width, self.height - values = { - Rotation.LWH: (l, w, h), Rotation.LHW: (l, h, w), - Rotation.WLH: (w, l, h), Rotation.WHL: (w, h, l), - Rotation.HLW: (h, l, w), Rotation.HWL: (h, w, l), - }[rotation] - return Dimensions(*values) + sides = (self.length, self.width, self.height) + first, second, third = _ROTATION_AXES[rotation] + return Dimensions(sides[first], sides[second], sides[third]) def unique_rotations(self, allowed: Iterable[Rotation]) -> tuple[tuple[Rotation, "Dimensions"], ...]: - seen: set[tuple[int, int, int]] = set() - result = [] - for rotation in allowed: - dims = self.rotated(rotation) - key = (dims.length.ticks, dims.width.ticks, dims.height.ticks) - if key not in seen: - seen.add(key) - result.append((rotation, dims)) - return tuple(result) + return _unique_rotations(self, tuple(allowed)) def fits_inside(self, other: "Dimensions") -> bool: return self.length.ticks <= other.length.ticks and self.width.ticks <= other.width.ticks and self.height.ticks <= other.height.ticks @@ -114,6 +113,21 @@ def to_dict(self, unit: str = "mm") -> dict: return {"length": self.length.to_dict(unit), "width": self.width.to_dict(unit), "height": self.height.to_dict(unit)} +@lru_cache(maxsize=4096) +def _unique_rotations(dimensions: Dimensions, allowed: tuple[Rotation, ...]) -> tuple[tuple[Rotation, Dimensions], ...]: + """Memoised on the (immutable) box and rotation list: every candidate search of an + item asks this again, and the answer is the same tuple of the same frozen values.""" + seen: set[tuple[int, int, int]] = set() + result = [] + for rotation in allowed: + dims = dimensions.rotated(rotation) + key = (dims.length.ticks, dims.width.ticks, dims.height.ticks) + if key not in seen: + seen.add(key) + result.append((rotation, dims)) + return tuple(result) + + def dimensional_weight(dimensions: Dimensions, divisor: int, length_unit: str = "in", weight_unit: str = "lb") -> Weight: """The carrier-style volumetric weight of a box: (L * W * H in `length_unit`) / divisor. diff --git a/src/packvium/models.py b/src/packvium/models.py index 46f7100..d5054cc 100644 --- a/src/packvium/models.py +++ b/src/packvium/models.py @@ -7,7 +7,8 @@ from .centre_of_mass import centre_of_mass_offset_ppm from . import compression, hull -from .geometry import AxisAlignedBox, Dimensions, Point, Rotation, ShapeType +from .geometry import (ALL_DIRECTIONS, AxisAlignedBox, Dimensions, InvalidDirectionError, + Point, Rotation, ShapeType) from .lattice_summary import LatticeSummary from .nesting import used_volume as nesting_used_volume from .units import Length, Weight @@ -45,7 +46,7 @@ class Axle: max_load: Weight | None = None -#: The largest `stop_index` every engine can carry identically (/KI defect found +#: The largest `stop_index` every engine can carry identically ( /KI defect found #: under ). Route order is decided by comparing stop indices, and JavaScript holds #: numbers as doubles: `JSON.parse` already collapses 2**53 + 1 to 2**53 before any #: constraint sees it, so two consecutive stops above this bound become one number there @@ -313,6 +314,14 @@ class Container: max_stack_density: Weight | None = None # (front, rear), front nearer the container's own x origin. axles: tuple[Axle, Axle] | None = None + # Which walls this container can be unloaded through. Empty means the + # horizontal half of route order is not enforced for it -- not that it is sealed. + # A container with no stated doors is the pre- default, and defaulting to all + # six instead would enforce a rule true of no real vehicle: a box is almost always + # free through *some* face, so six doors is nearly the same as none, but it is a + # *different* nearly-nothing and it would change answers for every caller who never + # set the field. + access_directions: tuple[str, ...] = () def __post_init__(self) -> None: if not self.id: raise ValueError("container id is required") @@ -326,6 +335,14 @@ def __post_init__(self) -> None: if front.position.ticks < 0 or rear.position.ticks > self.inner_dimensions.length.ticks: raise ValueError("axle positions must lie within the container's length") if any(limit < 1 for limit in self.tag_limits.values()): raise ValueError("tag_limits must be at least 1") + # Deduplicated into the canonical order rather than kept as given: two callers + # naming the same doors in a different order must search identically, and this is + # the one place every construction path passes through. + for direction in self.access_directions: + if direction not in ALL_DIRECTIONS: + raise InvalidDirectionError(direction) + object.__setattr__(self, "access_directions", + tuple(d for d in ALL_DIRECTIONS if d in set(self.access_directions))) if self.outer_dimensions and not self.inner_dimensions.fits_inside(self.outer_dimensions): raise ValueError("outer dimensions cannot be smaller than inner dimensions") boundary = AxisAlignedBox(Point(0, 0, 0), self.inner_dimensions) for obstacle in self.obstacles: diff --git a/src/packvium/serialization.py b/src/packvium/serialization.py index 2c53192..3779e03 100644 --- a/src/packvium/serialization.py +++ b/src/packvium/serialization.py @@ -109,6 +109,7 @@ def _container(raw: dict, unit: str) -> Container: tag_limits={str(k): int(v) for k, v in raw.get("tag_limits", {}).items()}, metadata=raw.get("metadata", {}), max_stack_density=None if raw.get("max_stack_density") is None else Weight.parse(raw["max_stack_density"]), axles=_axles(raw.get("axles"), unit), + access_directions=tuple(str(d) for d in raw.get("access_directions", ())), ) @@ -137,7 +138,12 @@ class UnsupportedFeatureError(ValueError): # in , when Python gained both the solver behaviour and the independent validation # the staged rollout requires. PHP, Rust and the JavaScript fallback still carry them. "item": (), - "container": (), + # `pallet_overhang_limit` was reserved in the schema by at the 1.1.0 contract + # freeze and is refused everywhere until an engine implements it from a request: a field + # a caller can set and the solver ignores is worse than a refusal. + # `access_directions` left this list in , which wired the reserved field through + # to `StopAccessibilityConstraint` in all four engines at once. + "container": ("pallet_overhang_limit",), } #: `item.shape_type` values this engine does not implement. diff --git a/src/packvium/solvers.py b/src/packvium/solvers.py index 0f3e123..b03e4d7 100644 --- a/src/packvium/solvers.py +++ b/src/packvium/solvers.py @@ -18,8 +18,8 @@ ContainerEligibilityConstraint, FloorConstraint, LoadUnit, PlacementConstraint, RIDES_THE_WHOLE_ROUTE, RouteOrderConstraint, StopAccessibilityConstraint, SupportConstraint, - TagCountConstraint, TopLoadConstraint, direct_support_view, load_units, - top_loads, usable_volume) + TagCountConstraint, TopLoadConstraint, active_constraints, direct_support_view, + load_units, top_loads, usable_volume) from . import bounds, hull from .geometry import (AxisAlignedBox, Dimensions, Point, Rotation, ShapeType, dimensional_weight) @@ -131,7 +131,13 @@ def with_effort(self, effort_budget: "EffortBudget | None", stats: "SearchStats" return Deadline(0, limit_ns=self.remaining_ns, clock=self._clock, effort_budget=effort_budget, stats=stats) def check(self) -> None: - if self.expired: raise TimeLimitReached("packing time limit reached") + # Polled once per candidate point; the same test as `expired`, without the three + # chained property calls that made it measurable there. + budget = self._effort_budget + if budget is not None and self._stats is not None and budget.exceeded(self._stats): + raise TimeLimitReached("packing time limit reached") + if self.limit_ns - (self._clock() - self.started) <= 0: + raise TimeLimitReached("packing time limit reached") @property def uses_real_clock(self) -> bool: @@ -305,13 +311,17 @@ def add(self, placement: Placement) -> None: # available to the next item. Keeping those points alive is what lets the exact # collision test below actually decide something; pruning them first would mean the # engine could describe an interlocking pack it could never propose. - retired = ({key for key, point in self.points.items() if box.contains_point(point)} - if shape is None else set()) - for key in retired: - del self.points[key] - if retired: - self.ordered_points = [point for point in self.ordered_points - if (point.x, point.y, point.z) not in retired] + if shape is None: + x1, y1, z1, x2, y2, z2 = bound + retired = [key for key in self.points + if x1 <= key[0] < x2 and y1 <= key[1] < y2 and z1 <= key[2] < z2] + for key in retired: + del self.points[key] + # `ordered_points` holds exactly the points of `points`, so the same test + # retires the same set there without rebuilding a key per point. + if retired: + self.ordered_points = [point for point in self.ordered_points + if not (x1 <= point.x < x2 and y1 <= point.y < y2 and z1 <= point.z < z2)] self._absorb(self._exposed_points(box)) def add_direct(self, placement: Placement) -> None: @@ -342,13 +352,18 @@ def add_direct(self, placement: Placement) -> None: def _absorb(self, points: Iterable[Point]) -> None: dims = self.container.inner_dimensions length, width, height = dims.length.ticks, dims.width.ticks, dims.height.ticks + # A solid containing a point is registered in that point's cell, so only that + # bucket has to be checked rather than every bound in the container. + index, bounds = self.index, self.bounds + cell_x, cell_y, cell_z, cells = index.cell_x, index.cell_y, index.cell_z, index.cells for point in points: x, y, z = point.x, point.y, point.z if x >= length or y >= width or z >= height: continue key = (x, y, z) if key in self.points: continue + bucket = cells.get((x // cell_x, y // cell_y, z // cell_z), ()) if any(bx1 <= x < bx2 and by1 <= y < by2 and bz1 <= z < bz2 - for bx1, by1, bz1, bx2, by2, bz2 in self.bounds): continue + for bx1, by1, bz1, bx2, by2, bz2 in map(bounds.__getitem__, bucket)): continue self.points[key] = point point_key = (z, y, x) low, high = 0, len(self.ordered_points) @@ -375,14 +390,40 @@ def _exposed_points(self, box: AxisAlignedBox) -> list[Point]: ] return [*corners, *projections] + # Each projection is the highest face at or below a ceiling among the solids whose + # footprint covers the point on the other two axes. Such a solid ends inside the + # ceiling, so it is registered in one of the cells of the ray below it: walking that + # ray visits every solid that can contribute, and a maximum is indifferent to seeing + # one twice. def _surface_z(self, x: int, y: int, ceiling: int) -> int: - return max((b[5] for b in self.bounds if b[5] <= ceiling and b[0] <= x < b[3] and b[1] <= y < b[4]), default=0) + index, bounds = self.index, self.bounds + ix, iy, cells = x // index.cell_x, y // index.cell_y, index.cells + best = 0 + for iz in range(-(-ceiling // index.cell_z)): + for position in cells.get((ix, iy, iz), ()): + b = bounds[position] + if best < b[5] <= ceiling and b[0] <= x < b[3] and b[1] <= y < b[4]: best = b[5] + return best def _surface_y(self, x: int, z: int, ceiling: int) -> int: - return max((b[4] for b in self.bounds if b[4] <= ceiling and b[0] <= x < b[3] and b[2] <= z < b[5]), default=0) + index, bounds = self.index, self.bounds + ix, iz, cells = x // index.cell_x, z // index.cell_z, index.cells + best = 0 + for iy in range(-(-ceiling // index.cell_y)): + for position in cells.get((ix, iy, iz), ()): + b = bounds[position] + if best < b[4] <= ceiling and b[0] <= x < b[3] and b[2] <= z < b[5]: best = b[4] + return best def _surface_x(self, y: int, z: int, ceiling: int) -> int: - return max((b[3] for b in self.bounds if b[3] <= ceiling and b[1] <= y < b[4] and b[2] <= z < b[5]), default=0) + index, bounds = self.index, self.bounds + iy, iz, cells = y // index.cell_y, z // index.cell_z, index.cells + best = 0 + for ix in range(-(-ceiling // index.cell_x)): + for position in cells.get((ix, iy, iz), ()): + b = bounds[position] + if best < b[3] <= ceiling and b[1] <= y < b[4] and b[2] <= z < b[5]: best = b[3] + return best @dataclass(frozen=True, slots=True) @@ -510,9 +551,19 @@ def find_candidates(state: ContainerState, item: ItemInstance, config: PackingCo bounds = state.bounds hull_shapes = state.hull_shapes index = state.index + nesting = item.item.nesting_height is not None + placement_offset = len(bounds) - len(placed) + reserve_check = container.void_fill_reserve_ratio > 0 and reserve_needs_candidate + usable = usable_volume(container) if reserve_check else 0 + # Everything that decides whether a rule can fire is fixed for this call, so the chain + # is pruned once here rather than answered "allow" once per position. The support rule + # stays in regardless: `support_checks` counts every time the chain reaches it. + active = [(constraint, isinstance(constraint, SupportConstraint)) + for constraint in active_constraints(constraints, container, item, stack_sensitive, route_sensitive)] + tracing = trace.active() if points is None: if item.item.nesting_height is None: - ordered = list(state.ordered_points[:config.max_candidate_points]) + ordered = state.ordered_points[:config.max_candidate_points] else: merged = {(point.x, point.y, point.z): point for point in (*state.ordered_points, *_nesting_points(state, item))} @@ -531,14 +582,13 @@ def find_candidates(state: ContainerState, item: ItemInstance, config: PackingCo stats.placements_attempted += 1 x2, y2, z2 = x1 + dx, y1 + dy, z1 + dz if x2 > limit_x or y2 > limit_y or z2 > limit_z: - if trace.active(): + if tracing: trace.emit({"type": "filter", "item_id": item.id, "point": {"x": x1, "y": y1, "z": z1}, "rotation": rotation.value, "reason": "boundary"}) continue tentative = None - if item.item.nesting_height is not None: + if nesting: position = Point(x1 + clearance, y1 + clearance, z1 + clearance) tentative = Placement(item, position, rotation, physical, point, envelope) - placement_offset = len(bounds) - len(placed) blocked = False for candidate_index in index.query(x1, y1, z1, x2, y2, z2): bx1, by1, bz1, bx2, by2, bz2 = bounds[candidate_index] @@ -559,17 +609,17 @@ def find_candidates(state: ContainerState, item: ItemInstance, config: PackingCo blocked = True break if blocked: - if trace.active(): + if tracing: trace.emit({"type": "filter", "item_id": item.id, "point": {"x": x1, "y": y1, "z": z1}, "rotation": rotation.value, "reason": "collision"}) continue context = ConstraintContext(container, placed, item, point, rotation, physical, envelope, stack_sensitive, route_sensitive) rejected = False - for constraint in constraints: - if isinstance(constraint, SupportConstraint): + for constraint, counts_support in active: + if counts_support: stats.support_checks += 1 result = constraint.evaluate(context) if not result.allowed: - if trace.active(): + if tracing: trace.emit({"type": "placement_rejection", "item_id": item.id, "point": {"x": x1, "y": y1, "z": z1}, "rotation": rotation.value, "constraint": type(constraint).__name__, "code": result.code}) rejected = True break @@ -577,7 +627,7 @@ def find_candidates(state: ContainerState, item: ItemInstance, config: PackingCo position = (tentative.position if tentative is not None else Point(x1 + clearance, y1 + clearance, z1 + clearance)) candidate = Candidate(point, position, rotation, physical, envelope, _candidate_score(state, point, envelope)) - if container.void_fill_reserve_ratio > 0 and reserve_needs_candidate: + if reserve_check: reserve_placement = tentative or Placement( item, position, rotation, physical, point, envelope ) @@ -590,17 +640,17 @@ def find_candidates(state: ContainerState, item: ItemInstance, config: PackingCo upper_bound = state.used_volume_ticks + occupied_volume(reserve_placement) projected_volume = ( upper_bound - if upper_bound <= usable_volume(container) + if upper_bound <= usable else _used_volume_with_current_loads((*placed, reserve_placement)) ) else: projected_volume = state.used_volume_ticks + used_volume_delta( placed, reserve_placement ) - if projected_volume > usable_volume(container): + if projected_volume > usable: continue stats.candidates_evaluated += 1 - if trace.active(): + if tracing: trace.emit({"type": "score", "item_id": item.id, "point": {"x": x1, "y": y1, "z": z1}, "rotation": rotation.value, "score": list(candidate.score)}) if max_candidates == 1: if best is None or candidate.score < best.score: best = candidate diff --git a/src/packvium/spatial_index.py b/src/packvium/spatial_index.py index 4e7e432..a397e81 100644 --- a/src/packvium/spatial_index.py +++ b/src/packvium/spatial_index.py @@ -19,7 +19,7 @@ from __future__ import annotations -from typing import Iterable, Iterator +from typing import Iterable, Sequence Bound = tuple[int, int, int, int, int, int] @@ -38,25 +38,37 @@ class SpatialIndex: of placements spreads them across many buckets instead of one. """ - __slots__ = ("cell_x", "cell_y", "cell_z", "cells") + __slots__ = ("cell_x", "cell_y", "cell_z", "cells", "_answers") def __init__(self, length_ticks: int, width_ticks: int, height_ticks: int, cells_per_axis: int = 8): self.cell_x = max(1, _ceil_div(max(1, length_ticks), cells_per_axis)) self.cell_y = max(1, _ceil_div(max(1, width_ticks), cells_per_axis)) self.cell_z = max(1, _ceil_div(max(1, height_ticks), cells_per_axis)) - self.cells: dict[tuple[int, int, int], list[int]] = {} + # Cell contents are tuples, never lists: a bucket is replaced on insert rather than + # appended to, so a fork only has to copy the dict, and `query` can hand a bucket + # straight back without exposing anything a caller could mutate. + self.cells: dict[tuple[int, int, int], tuple[int, ...]] = {} + # Query answers by cell range, valid for the current contents only. A candidate + # scan asks about far more boxes than there are distinct cell ranges -- the grid + # is coarse by design -- and the answer for a range is a pure function of the + # contents, so it is computed once per range per generation of the index. + self._answers: dict[tuple[int, int, int, int, int, int], Sequence[int]] = {} def copy(self) -> "SpatialIndex": """A structural fork: independent of the original from this point on. Search branches copy a `ContainerState` and then diverge, each adding its own - placements -- sharing the cell lists would let one branch's insert corrupt + placements -- sharing the buckets would let one branch's insert corrupt another's index, the same reason `ContainerState.copy()` already copies its own - `bounds` list rather than aliasing it. + `bounds` list rather than aliasing it. Buckets are immutable, so a shallow copy + of the dict is a complete fork. """ clone = SpatialIndex.__new__(SpatialIndex) clone.cell_x, clone.cell_y, clone.cell_z = self.cell_x, self.cell_y, self.cell_z - clone.cells = {key: list(indices) for key, indices in self.cells.items()} + clone.cells = dict(self.cells) + # Same contents, same answers. Safe to share: an insert on either side replaces + # the dict on that side rather than mutating it, so the other keeps a valid one. + clone._answers = self._answers return clone def _cell_range(self, x1: int, y1: int, z1: int, x2: int, y2: int, z2: int): @@ -68,22 +80,51 @@ def _cell_range(self, x1: int, y1: int, z1: int, x2: int, y2: int, z2: int): def add(self, index: int, bound: Bound) -> None: x1, y1, z1, x2, y2, z2 = bound ix1, ix2, iy1, iy2, iz1, iz2 = self._cell_range(x1, y1, z1, x2, y2, z2) + cells = self.cells for ix in range(ix1, ix2): for iy in range(iy1, iy2): for iz in range(iz1, iz2): - self.cells.setdefault((ix, iy, iz), []).append(index) + key = (ix, iy, iz) + cells[key] = cells.get(key, ()) + (index,) + self._answers = {} - def query(self, x1: int, y1: int, z1: int, x2: int, y2: int, z2: int) -> Iterator[int]: - """Bound indices sharing at least one cell with the given box, each at most once.""" - ix1, ix2, iy1, iy2, iz1, iz2 = self._cell_range(x1, y1, z1, x2, y2, z2) - seen: set[int] = set() + def query(self, x1: int, y1: int, z1: int, x2: int, y2: int, z2: int) -> Sequence[int]: + """Bound indices sharing at least one cell with the given box, each at most once. + + Cells are visited in `(x, y, z)` order and an index keeps its first position, so + the sequence is a deterministic function of the index contents. Returned as a + sequence rather than a generator: this is the innermost call of the candidate + scan, and a bucket can be handed back as-is when the box touches only one. + """ + cell_x, cell_y, cell_z = self.cell_x, self.cell_y, self.cell_z + ix1, ix2 = x1 // cell_x, -(-max(x2, x1 + 1) // cell_x) + iy1, iy2 = y1 // cell_y, -(-max(y2, y1 + 1) // cell_y) + iz1, iz2 = z1 // cell_z, -(-max(z2, z1 + 1) // cell_z) + cell_range = (ix1, ix2, iy1, iy2, iz1, iz2) + answers = self._answers + answer = answers.get(cell_range) + if answer is not None: return answer + answers[cell_range] = answer = self._collect(ix1, ix2, iy1, iy2, iz1, iz2) + return answer + + def _collect(self, ix1: int, ix2: int, iy1: int, iy2: int, iz1: int, iz2: int) -> Sequence[int]: + cells = self.cells + hits: list[tuple[int, ...]] = [] for ix in range(ix1, ix2): for iy in range(iy1, iy2): for iz in range(iz1, iz2): - for index in self.cells.get((ix, iy, iz), ()): - if index not in seen: - seen.add(index) - yield index + bucket = cells.get((ix, iy, iz)) + if bucket: hits.append(bucket) + if not hits: return () + if len(hits) == 1: return hits[0] + seen: set[int] = set() + found: list[int] = [] + for bucket in hits: + for index in bucket: + if index not in seen: + seen.add(index) + found.append(index) + return found def build(bounds: Iterable[Bound], length_ticks: int, width_ticks: int, height_ticks: int) -> SpatialIndex: diff --git a/tests/test_access_directions_contract.py b/tests/test_access_directions_contract.py new file mode 100644 index 0000000..4dec4e7 --- /dev/null +++ b/tests/test_access_directions_contract.py @@ -0,0 +1,126 @@ +"""`container.access_directions` at its boundaries. + +The field was reserved at the 1.1.0 freeze and implemented in all four engines in one +change. What the wave shipped with it was a *validation and canonicalisation* path per +engine, and coverage showed that path untested in every one of them: the happy request +was exercised by conformance, the refusals by nothing. + +That is the failure mode worth guarding against here rather than the packing rule, which +`test_constraints.py` and the shared corpus already hold. A field whose canonicalisation +silently stops working does not raise -- it makes two callers who named the same doors in +a different order search differently, which is a determinism break that no fixture +notices, because every fixture names its doors one way. +""" + +from __future__ import annotations + +import pytest + +from packvium.geometry import ALL_DIRECTIONS, Dimensions, InvalidDirectionError +from packvium.models import Container +from packvium.serialization import pack_from_dict +from packvium.units import Length + +MM = 16_000 + + +def crate(**kwargs) -> Container: + side = Dimensions(Length(100 * MM), Length(100 * MM), Length(100 * MM)) + return Container(id="crate", inner_dimensions=side, **kwargs) + + +# ------------------------------------------------------------------ canonicalisation + +def test_doors_are_deduplicated_into_the_canonical_order(): + """Two callers naming the same doors differently must search identically. + + This is the assertion the corpus cannot make: every fixture states its doors once, in + one order, so a canonicalisation that quietly stopped working would leave all 399 + green while making the engine order-sensitive. + """ + assert crate(access_directions=("+z", "-x", "+z", "-x")).access_directions == ("-x", "+z") + assert crate(access_directions=("-x", "+z")).access_directions == ("-x", "+z") + + +def test_every_legal_direction_survives_canonicalisation(): + """All six, in the declared order, so a typo in ALL_DIRECTIONS cannot silently drop + one -- a dropped door is a refusal the caller never asked for.""" + assert crate(access_directions=tuple(reversed(ALL_DIRECTIONS))).access_directions \ + == tuple(ALL_DIRECTIONS) + + +def test_a_container_states_no_doors_by_default(): + """The pre- default, and it is *inert* rather than permissive: six walls and + none are both nearly-vacuous, but they are different nearly-vacuous, and defaulting to + six would switch a real constraint on for every caller who never set the field.""" + assert crate().access_directions == () + + +# --------------------------------------------------------------------------- refusals + +@pytest.mark.parametrize("direction", ["north", "x", "+X", "+w", "", "±x", "-x "]) +def test_an_unknown_direction_is_refused_rather_than_dropped(direction): + """Refused, not filtered out. Silently discarding an unrecognised door would leave a + container with fewer exits than the caller believes it has, and the packing would be + legal for a vehicle that does not exist.""" + with pytest.raises(InvalidDirectionError) as refusal: + crate(access_directions=(direction,)) + assert repr(direction) in str(refusal.value) + + +def test_one_bad_direction_refuses_the_whole_list(): + """A partially-honoured list is the worst outcome available: it validates and means + something the caller did not write.""" + with pytest.raises(InvalidDirectionError): + crate(access_directions=("-x", "sideways", "+z")) + + +# ------------------------------------------------------- the same rules through a request + +def _request(doors): + container = {"id": "van", + "inner_dimensions": {"length": "200", "width": "100", "height": "100"}} + if doors is not None: + container["access_directions"] = doors + return {"units": {"length": "mm"}, + "items": [{"id": "cube", "quantity": 1, + "dimensions": {"length": "100", "width": "100", "height": "100"}}], + "containers": [container]} + + +def test_a_request_reaches_the_same_validation_as_the_constructor(): + """The decoder is a fourth way to name the doors, and it must not be a way around the + canonicalisation. `docs/STOP-ACCESSIBILITY.md` records that a rule no request can + switch on is untested along the path it will be switched on through; this is that + path.""" + with pytest.raises(InvalidDirectionError): + pack_from_dict(_request(["upwards"])) + + +def _without_wall_clock(result: dict) -> dict: + """Everything the contract promises to reproduce. + + `algorithm.duration_ms` is wall clock and is the one field a determinism assertion + must not read -- comparing whole documents passes or fails on how busy the machine + is, which is a test that reports the host rather than the engine. + """ + trimmed = {key: value for key, value in result.items() if key != "algorithm"} + trimmed["algorithm"] = {key: value for key, value in result["algorithm"].items() + if key != "duration_ms"} + return trimmed + + +def test_a_request_naming_doors_in_either_order_gives_one_answer(): + """Determinism across two spellings of one intent, end to end rather than on the + domain object alone.""" + first = pack_from_dict(_request(["-x", "+z"])) + second = pack_from_dict(_request(["+z", "-x"])) + assert _without_wall_clock(first) == _without_wall_clock(second) + + +def test_an_empty_door_list_is_accepted_and_inert(): + """`[]` is a caller saying "no doors stated", not a malformed request. It has to + behave exactly like the absent field, or the two spellings of the default diverge.""" + stated = pack_from_dict(_request([])) + absent = pack_from_dict(_request(None)) + assert _without_wall_clock(stated) == _without_wall_clock(absent) diff --git a/tests/test_constraints.py b/tests/test_constraints.py index 82a3819..8a12e25 100644 --- a/tests/test_constraints.py +++ b/tests/test_constraints.py @@ -1379,30 +1379,75 @@ def test_the_corridor_base_is_reused_for_a_second_candidate_on_the_same_state(): """The cache is why a candidate costs `O(m * |D|)` rather than `O(m^2 * |D|)`: search asks a run of candidates against one immutable state, so one entry covers the run. - It is keyed on the placements *and* the container. The container half is a guard rather - than something a legal scene can demonstrate -- a placement always lies inside its own - container, so widening a wall only lengthens a sweep into empty space. What it protects - against is one placement tuple being asked about two different containers, which a - multi-container solve can do; the key makes the second question rebuild instead of - inheriting the first answer. + It is keyed on the placements, the container *and* the doors. The container half is a + guard rather than something a legal scene can demonstrate -- a placement always lies + inside its own container, so widening a wall only lengthens a sweep into empty space. + What it protects against is one placement tuple being asked about two different + containers, which a multi-container solve can do; the key makes the second question + rebuild instead of inheriting the first answer. + + The doors half is not a guard at all since made them a property of the + container: two containers of the same size with different doors give *different* + answers for the same boxes, and nothing else in the key separates them. """ constraint = constraints.StopAccessibilityConstraint(DOOR_AT_MINUS_X) early, early_x = _wide("early", 60, 40, stop=0) late, late_x = _wide("late", 0, 60, stop=1) placements = (placed(early, x=early_x),) scene = context(late, x=late_x, placements=placements) + doors = tuple(DOOR_AT_MINUS_X) first = constraint.evaluate(scene) - built = constraint._base_for(placements, BOX.inner_dimensions) + built = constraint._base_for(placements, BOX.inner_dimensions, doors) # Asking again on the same state must take the cached path and answer identically. - assert constraint._base_for(placements, BOX.inner_dimensions) == built + assert constraint._base_for(placements, BOX.inner_dimensions, doors) == built assert constraint.evaluate(scene).allowed == first.allowed longer = Container.create("longer", Dimensions.mm(300, 100, 100)) - constraint._base_for(placements, longer.inner_dimensions) + constraint._base_for(placements, longer.inner_dimensions, doors) assert constraint._container == longer.inner_dimensions, ( "a different container must rebuild the base rather than inherit it") + # The same boxes and the same walls, through the other door: a different answer, and + # the entry above must not be handed back for it. + through_plus_x = constraint._base_for(placements, BOX.inner_dimensions, ("+x",)) + assert constraint._directions == ("+x",) + assert through_plus_x != built, ( + "the same placements behind two different doors are two different questions") + + +def test_a_container_states_its_own_doors_and_a_silent_one_inherits_the_default(): + """ . The field is per container because two doors on one trailer and none on + another is the case that makes the rule worth having; the constructor argument stays as + the default so the library callers who predate the field keep working.""" + early, early_x = _wide("early", 60, 40, stop=0) + late, late_x = _wide("late", 0, 60, stop=1) + placements = (placed(early, x=early_x),) + + sealed = Container.create("sealed", BOX.inner_dimensions) + through_minus_x = Container.create("through-minus-x", BOX.inner_dimensions, + access_directions=("-x",)) + through_plus_x = Container.create("through-plus-x", BOX.inner_dimensions, + access_directions=("+x",)) + + def verdict(constraint, container): + return constraint.evaluate( + context(late, x=late_x, placements=placements, container=container)).allowed + + # The stop-1 item fills the stop-0 item's only corridor to `-x`, and does not touch its + # corridor to `+x`. One container refuses it, the other does not, and nothing about the + # boxes changed. + stated = constraints.StopAccessibilityConstraint() + assert not verdict(stated, through_minus_x) + assert verdict(stated, through_plus_x) + assert verdict(stated, sealed), "no doors anywhere leaves the rule inert" + + # A container that states none inherits what the caller configured. + configured = constraints.StopAccessibilityConstraint(DOOR_AT_MINUS_X) + assert not verdict(configured, sealed) + # And a container that states its own overrides it, rather than adding to it. + assert verdict(configured, through_plus_x) + def test_permanent_cargo_that_blocks_nobody_is_allowed(): """The `rides the whole route` short-circuit after the placed-item loop. An item with no @@ -1568,7 +1613,7 @@ def test_a_single_supporter_covering_over_half_the_base_always_contains_the_cent refuses, and at 0.3 and 0.45 it refuses more (benchmarks/results/support-predicates.json). This is the boundary, not a bug -- but it means the hull is built per candidate for a - verdict that a cheaper comparison already fixed, which is the finding records. + verdict that a cheaper comparison already fixed, which is the finding records. """ rng = random.Random(6000 + seed) length, width = rng.randint(20, 60), rng.randint(20, 60) diff --git a/tests/test_decoder_refusals.py b/tests/test_decoder_refusals.py new file mode 100644 index 0000000..6b3bced --- /dev/null +++ b/tests/test_decoder_refusals.py @@ -0,0 +1,185 @@ +"""The refusal paths in the request decoder and the domain validators. + +Coverage put `serialization.py` at 90.24% and `models.py` at 96.83%, and every uncovered +line in both was a `raise`. That is the worst shape a coverage gap can take here: a +validator nothing exercises does not fail loudly when it breaks, it starts *accepting* +what it was written to refuse, and the engine then answers confidently about a request +that never made sense. + +`catalog_versions_used` is the sharpest case. It is provenance -- which immutable catalog +version produced the request -- so a malformed entry that slips through does not corrupt a +packing, it corrupts the record of what the packing was computed from, and that is +discovered much later than a wrong box. +""" + +from __future__ import annotations + +import pytest + +from packvium.models import MAX_EXACT_STOP_INDEX, Item +from packvium.geometry import Dimensions +from packvium.serialization import (UnsupportedFeatureError, pack_from_dict, + reject_unsupported) +from packvium.units import Length + +MM = 16_000 + + +def cube(**kwargs) -> Item: + side = Dimensions(Length(10 * MM), Length(10 * MM), Length(10 * MM)) + return Item(id="cube", dimensions=side, **kwargs) + + +# ------------------------------------------------------------------- item validation + +def test_a_stack_limit_below_one_is_refused(): + """Zero would mean "may not be stacked", which `stackable=False` already says, and a + negative one means nothing at all. Two spellings of one rule is how they drift.""" + assert cube(max_stacked_items=1).max_stacked_items == 1 + with pytest.raises(ValueError, match="max_stacked_items must be at least 1"): + cube(max_stacked_items=0) + + +@pytest.mark.parametrize("stop", [-1, MAX_EXACT_STOP_INDEX + 1]) +def test_a_stop_index_outside_the_exact_range_is_refused(stop): + """The ceiling is 2**53 - 1 and not an arbitrary limit: past it a float cannot tell two + consecutive stops apart, and the route rule uses `inf` as an ordering sentinel beside + real stops. Merging two stops into one would silently excuse a blocker.""" + with pytest.raises(ValueError): + cube(stop_index=stop) + + +def test_the_two_ends_of_the_exact_stop_range_are_accepted(): + """The boundary itself is legal; only past it is not.""" + assert cube(stop_index=0).stop_index == 0 + assert cube(stop_index=MAX_EXACT_STOP_INDEX).stop_index == MAX_EXACT_STOP_INDEX + + +@pytest.mark.parametrize("height", [10 * MM, 11 * MM]) +def test_a_nesting_height_outside_its_own_item_is_refused(height): + """Nesting height is how far an item sinks into the one below. Equal to its own height + means it vanishes; more means it occupies negative space.""" + with pytest.raises(ValueError, match="nesting_height"): + cube(nesting_height=Length(height)) + + +def test_a_negative_nesting_height_is_refused_by_the_unit_not_the_item(): + """Worth pinning rather than folding into the case above: the refusal comes from + `Length`, one layer below, so the item validator never sees it. If `Length` ever grew + permissive, `Item` would silently inherit that -- and this test is what would fail.""" + with pytest.raises(ValueError, match="length cannot be negative"): + Length(-1) + + +def test_an_unknown_ground_contact_rule_is_refused(): + with pytest.raises(ValueError, match="ground_contact_rule must be one of"): + cube(ground_contact_rule="hovering") + + +# ------------------------------------------------------- catalog provenance refusals + +def _with_catalog(catalog): + return {"units": {"length": "mm"}, + "items": [{"id": "cube", "quantity": 1, + "dimensions": {"length": "10", "width": "10", "height": "10"}}], + "containers": [{"id": "crate", + "inner_dimensions": {"length": "100", "width": "100", + "height": "100"}}], + "catalog_versions_used": catalog} + + +GOOD = {"catalog_id": "boxes", "version": 3, "effective_at": 0, "resolved_at": 0} + + +def test_a_well_formed_catalog_reference_is_carried_through(): + """The accepting case first: a refusal test that never sees an acceptance proves only + that the field is rejected, which is equally true of a decoder that refuses everything.""" + result = pack_from_dict(_with_catalog([GOOD])) + assert result["catalog_versions_used"] == [GOOD] + + +def test_catalog_versions_used_must_be_an_array(): + with pytest.raises(ValueError, match="must be an array"): + pack_from_dict(_with_catalog({"catalog_id": "boxes"})) + + +@pytest.mark.parametrize("reference", [ + {"catalog_id": "boxes"}, + {**GOOD, "extra": 1}, + "boxes", +]) +def test_a_reference_with_the_wrong_key_set_is_refused(reference): + """Exactly the four keys, not "at least". An extra key is a claim the format does not + define, and accepting it would make two engines disagree about what was recorded.""" + with pytest.raises(ValueError, match="must contain exactly"): + pack_from_dict(_with_catalog([reference])) + + +@pytest.mark.parametrize("catalog_id", ["", 7, None]) +def test_an_empty_or_non_string_catalog_id_is_refused(catalog_id): + with pytest.raises(ValueError, match="catalog_id must be non-empty"): + pack_from_dict(_with_catalog([{**GOOD, "catalog_id": catalog_id}])) + + +def test_a_duplicate_catalog_id_is_refused_as_ambiguous(): + """Two versions of one catalog in one request do not say which was used. Silently + keeping the last would make provenance a function of ordering.""" + with pytest.raises(ValueError, match="ambiguous duplicate"): + pack_from_dict(_with_catalog([GOOD, {**GOOD, "version": 4}])) + + +@pytest.mark.parametrize("field,value", [ + ("version", 0), ("version", -1), ("effective_at", -1), ("resolved_at", -1), + ("version", True), ("version", 1.0), ("effective_at", "0"), +]) +def test_out_of_range_or_mistyped_catalog_numbers_are_refused(field, value): + """`True` is the one worth spelling out: it is an `int` in Python and `True >= 1`, so + a bare range check would accept `version: true` and record a provenance nobody wrote.""" + with pytest.raises(ValueError, match=field): + pack_from_dict(_with_catalog([{**GOOD, field: value}])) + + +# --------------------------------------------------------------- the rest of the decoder + +def test_a_rate_table_is_decoded_rather_than_ignored(): + """`_rate_table`'s constructing arm was uncovered: the unit suite only ever reached the + `None` branch, so the shape of a decoded card rested on conformance alone.""" + request = {"units": {"length": "mm"}, + "items": [{"id": "cube", "quantity": 1, "weight": {"value": "1", "unit": "kg"}, + "dimensions": {"length": "10", "width": "10", "height": "10"}}], + "containers": [{"id": "crate", + "inner_dimensions": {"length": "100", "width": "100", + "height": "100"}, + "rate_table": {"weight_brackets_g": [1000, 5000], + "prices_minor": [500, 900]}}], + "configuration": {"objective": "lowest_landed_cost", + "dimensional_weight_divisor": 139}} + assert pack_from_dict(request)["status"] + + +def test_a_malformed_entry_does_not_derail_the_unsupported_field_scan(): + """The scan walks `items` and `containers` looking for reserved names, and a non-object + entry is the schema's problem rather than the guard's. + + Asserted on the guard directly, with an injected list, for the reason the guard takes + its lists as parameters at all: against the shipped lists this would prove only that + nothing was rejected, which is equally true of a guard that does nothing. The + injected list names a field the malformed entry cannot carry, so reaching the end + without raising is the whole assertion -- a guard that indexed into the string would + raise from inside itself and mask the schema error the caller should see. + """ + reject_unsupported( + {"items": ["not-an-object", 7, None], "containers": ["neither"]}, + {"item": ("hull_vertices",), "container": ("pallet_overhang_limit",)}, + ) + + +def test_the_scan_still_finds_a_reserved_field_beside_a_malformed_entry(): + """Skipping the junk must not skip the sibling: a request mixing one bad entry with a + real reserved field is still refused, and named.""" + with pytest.raises(UnsupportedFeatureError, match="container.pallet_overhang_limit"): + reject_unsupported( + {"items": ["not-an-object"], + "containers": ["junk", {"id": "c", "pallet_overhang_limit": {}}]}, + {"item": (), "container": ("pallet_overhang_limit",)}, + ) diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 0ecb6fb..8a630b2 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -75,6 +75,20 @@ def test_a_constraint_that_refuses_everything_leaves_the_order_unpacked(): assert result.containers == () +def test_an_extension_method_named_inert_for_does_not_opt_out_of_the_chain(): + """Activity pruning is an internal built-in specification, not a duck-typed public + extension point whose name an unrelated constraint can accidentally collide with.""" + class MisleadingConstraint(RejectEverything): + def inert_for(self, *_args): + return True + + result = packer(placement_constraints=(MisleadingConstraint(),)).pack( + [item("a", 10, 10, 10)], [container("b", 100, 100, 100)]) + + assert not result.complete + assert result.containers == () + + def test_a_custom_constraint_is_applied_at_every_candidate_point(): items = [item("a", 40, 40, 40, quantity=8)] containers = [container("c", 100, 100, 100, quantity=4)] diff --git a/tests/test_irregular_items.py b/tests/test_irregular_items.py index adfc940..b3821cd 100644 --- a/tests/test_irregular_items.py +++ b/tests/test_irregular_items.py @@ -337,7 +337,7 @@ def test_a_face_carrying_a_non_corner_vertex_is_wound_past_it(): leave every other vertex on one side, it takes the farthest, which skips an edge-interior point instead of turning the face into a degenerate spur. Getting it wrong does not raise -- it produces a surface that fails to close and a volume that is quietly too small, which - is how the defect fixed in survived a review cycle. + is how the defect fixed in survived a review cycle. A pyramid on a square base, with a redundant vertex at the midpoint of one base edge. The base is five coplanar points and only four of them are corners; the volume is a third of diff --git a/tests/test_optimality_bounds.py b/tests/test_optimality_bounds.py index ae16a16..4fe1c5d 100644 --- a/tests/test_optimality_bounds.py +++ b/tests/test_optimality_bounds.py @@ -258,7 +258,7 @@ def test_a_score_below_its_bound_is_refused_and_an_attained_one_reports_no_gap() def test_a_nesting_request_drops_the_volume_argument_end_to_end(): - """The branch found unsound, exercised through `compute` rather than the helper. + """The branch found unsound, exercised through `compute` rather than the helper. Five 10mm cubes cannot fit one 10mm container by volume, and the bound says four are stranded. Declare a nesting height on the same items and the volume argument is dropped diff --git a/tests/test_unsupported_fields.py b/tests/test_unsupported_fields.py index e1d164a..76eb804 100644 --- a/tests/test_unsupported_fields.py +++ b/tests/test_unsupported_fields.py @@ -61,6 +61,16 @@ def test_a_request_that_touches_nothing_listed_is_accepted() -> None: reject_unsupported(REQUEST, {"request": ("other",), "item": ("unrelated",)}) +def public_name(scope: str, name: str) -> str: + """The name an engine's `unsupported_feature` diagnostic gives a refused field.""" + return name if scope == "request" else f"{scope}.{name}" + + +def field_of(rejection_name: str) -> str: + """A value-keyed template such as `item.shape_type={value}` names the field before `=`.""" + return rejection_name.split("=", 1)[0] + + def test_the_unsupported_lists_match_what_the_field_matrix_records() -> None: """Every refusal this engine makes is recorded in the matrix, and the reverse. @@ -68,31 +78,43 @@ def test_the_unsupported_lists_match_what_the_field_matrix_records() -> None: they were -- and stopped being the same thing the moment one was populated. What the coupling is actually for is that the corpus *asserts* each rejection instead of merely tolerating it, so read the matrix and compare both directions. + + An engine refuses a field by name; the matrix is keyed on the schema's leaves, so one + refused field is several rows. The matrix's own `rejection_name` -- the name the + conformance harness demands in the diagnostic -- is what ties the rows to the field, + so the comparison is made on that and never inferred from the spelling of a path. """ - matrix = json.loads( - (ROOT / "conformance/public-field-matrix.json").read_text() - ) - rejected_by_matrix = { - path + # A cross-language artifact kept one level above this package; a published copy does + # not carry it, and the guard itself is exercised by the cases above. + shared = ROOT / "conformance/public-field-matrix.json" + if not shared.is_file(): + pytest.skip("the shared public field matrix is not part of this package") + matrix = json.loads(shared.read_text()) + rows = { + path: (row.get("rejection_name"), matrix["support_sets"][row["support"]]["python"]) for path, row in matrix["fields"].items() - if matrix["support_sets"][row["support"]]["python"] - == "rejected:unsupported_feature" } - declared = {f"{scope}s.*.{name}" if scope in ("item", "container") else name + rejected_by_matrix = { + field_of(name) for name, support in rows.values() + if support == "rejected:unsupported_feature" + } + declared = {public_name(scope, name) for scope, names in UNSUPPORTED_FIELDS.items() for name in names} - # A value-keyed refusal is one matrix row for the field itself. `hull_vertices` is an - # array of points, so the schema's leaves -- and therefore its rows -- are the three - # coordinates, not the array. - declared |= {"items.*.shape_type"} if UNSUPPORTED_SHAPE_TYPES else set() - if "items.*.hull_vertices" in declared: - declared.discard("items.*.hull_vertices") - declared |= {f"items.*.hull_vertices.*.{axis}" for axis in "xyz"} + declared |= {"item.shape_type"} if UNSUPPORTED_SHAPE_TYPES else set() assert declared == rejected_by_matrix, ( "the engine and the matrix disagree about what Python refuses; " f"engine only: {sorted(declared - rejected_by_matrix)}, " f"matrix only: {sorted(rejected_by_matrix - declared)}" ) + # A field refused by name is refused on every one of its leaves: a row that names a + # refused field while recording this engine as implementing it is a matrix error. + half_recorded = sorted( + path for path, (name, support) in rows.items() + if name is not None and field_of(name) in declared + and support != "rejected:unsupported_feature" + ) + assert half_recorded == [], f"rows recorded as implemented for a field Python refuses: {half_recorded}" def test_the_default_shape_type_is_served_rather_than_refused() -> None: