lgtm

pipeline/transform/postcode_boundaries/test_postcode_boundaries.py

@@ -4,6 +4,7 @@ Each test targets a specific bug or edge case identified during code review.
 """
 
 import json
+import math
 
 import numpy as np
 import polars as pl
@@ -21,6 +22,8 @@ from .inspire import build_inspire_index
 from .oa_boundaries import parse_gpkg_geometry
 from .greenspace import subtract_greenspace
 from .output import (
+    _approx_area_m2,
+    _eliminate_small_detached_parts,
     _fill_holes,
     merge_fragments,
     to_wgs84_geojson,
@@ -1830,3 +1833,231 @@ class TestFragmentsCache:
         cache.write_text("c")
         # arcgis is optional/absent — it cannot have invalidated the cache.
         assert fragments_cache_is_fresh(cache, [tmp_path / "absent.parquet"]) is True
+
+
+# ---------------------------------------------------------------------------
+# De-fragmentation: small detached parts are absorbed into their best neighbour
+# ---------------------------------------------------------------------------
+
+
+# WGS84 helper: build a box from METRE offsets around a fixed England anchor, so
+# the eliminate pass (which works in WGS84 degrees and measures area with a
+# latitude-scaled approximation) sees realistic coordinates and areas.
+_ANCHOR_LON, _ANCHOR_LAT = -0.1, 51.5
+_M_PER_DEG_LAT = 111_320.0
+_M_PER_DEG_LON = 111_320.0 * math.cos(math.radians(_ANCHOR_LAT))
+
+
+def _mbox(x0, y0, x1, y1):
+    return box(
+        _ANCHOR_LON + x0 / _M_PER_DEG_LON,
+        _ANCHOR_LAT + y0 / _M_PER_DEG_LAT,
+        _ANCHOR_LON + x1 / _M_PER_DEG_LON,
+        _ANCHOR_LAT + y1 / _M_PER_DEG_LAT,
+    )
+
+
+def _as_dict(items):
+    return dict(items)
+
+
+class TestEliminateSmallDetachedParts:
+    """A small detached part should be absorbed into the postcode it borders most,
+    de-fragmenting the unit while conserving coverage and never dropping a
+    postcode. Genuine large splits must survive."""
+
+    def test_small_island_absorbed_by_surrounding_neighbour(self):
+        # A: a big main blob (left) plus a small 30x30m island sitting INSIDE B's
+        # territory; B fills the middle/right with a hole where the island is.
+        main_a = _mbox(0, 0, 100, 200)         # 20000 m²
+        island = _mbox(150, 90, 180, 120)      # 900 m², surrounded by B
+        a = MultiPolygon([main_a, island])
+        b = _mbox(100, 0, 300, 200).difference(island)  # hole around the island
+        before = unary_union([a, b]).area
+
+        out = _as_dict(_eliminate_small_detached_parts([("A", a), ("B", b)]))
+
+        assert "A" in out and "B" in out, "no postcode may be dropped"
+        assert out["A"].geom_type == "Polygon", "A's island should be absorbed away"
+        # The island area moves to B (the surrounding postcode); coverage is kept.
+        assert out["B"].contains(island.representative_point())
+        after = unary_union([out["A"], out["B"]]).area
+        assert after == pytest.approx(before, rel=1e-6), "coverage must be conserved"
+
+    def test_genuine_large_split_is_kept(self):
+        # Two substantial parts (both 40000 m²) far apart — a real bisection, not
+        # scatter. Neither is below the absolute/fraction thresholds, so both stay.
+        a = MultiPolygon([_mbox(0, 0, 200, 200), _mbox(400, 0, 600, 200)])
+        out = _as_dict(_eliminate_small_detached_parts([("A", a)]))
+        assert out["A"].geom_type == "MultiPolygon"
+        assert len(out["A"].geoms) == 2
+
+    def test_midsize_minor_part_kept_when_above_abs_threshold(self):
+        # A big main plus a 10000 m² detached part: above the 3000 m² absolute
+        # threshold, so it is a genuine secondary piece and must NOT be eliminated,
+        # even though it is a small fraction of the postcode.
+        a = MultiPolygon([_mbox(0, 0, 300, 300), _mbox(1000, 0, 1100, 100)])
+        out = _as_dict(_eliminate_small_detached_parts([("A", a)]))
+        assert out["A"].geom_type == "MultiPolygon"
+
+    def test_tiny_neighbourless_sliver_is_dropped(self):
+        # A main blob plus a 10x10m (100 m²) sliver far from anything: below the
+        # drop threshold with no neighbour to absorb it -> dropped as snap debris.
+        a = MultiPolygon([_mbox(0, 0, 200, 200), _mbox(1000, 1000, 1010, 1010)])
+        out = _as_dict(_eliminate_small_detached_parts([("A", a)]))
+        assert "A" in out, "the postcode itself must survive"
+        assert out["A"].geom_type == "Polygon"
+
+    def test_largest_part_always_retained_no_postcode_dropped(self):
+        # Even a postcode that is ENTIRELY a tiny sliver keeps its (largest) part —
+        # active postcodes must never be dropped (validate_outputs is zero-tolerance).
+        tiny = _mbox(500, 500, 505, 505)  # 25 m², the whole postcode
+        big = _mbox(0, 0, 300, 300)
+        out = _as_dict(
+            _eliminate_small_detached_parts([("TINY", tiny), ("BIG", big)])
+        )
+        assert "TINY" in out and not out["TINY"].is_empty
+        assert "BIG" in out
+
+    def test_no_overlaps_introduced(self):
+        # Absorbing parts must keep the output a partition (no double-coverage).
+        main_a = _mbox(0, 0, 100, 200)
+        island = _mbox(150, 90, 180, 120)
+        a = MultiPolygon([main_a, island])
+        b = _mbox(100, 0, 300, 200).difference(island)
+        out = _as_dict(_eliminate_small_detached_parts([("A", a), ("B", b)]))
+        overlap = out["A"].intersection(out["B"]).area
+        assert overlap < (1.0 / (_M_PER_DEG_LAT * _M_PER_DEG_LON)), "no overlap"
+
+    def test_empty_postcode_not_dropped_when_reassignment_fires(self):
+        # Regression: an empty-geometry postcode must survive even when a
+        # reassignment fires elsewhere in the same pass (the rebuild used to drop
+        # any key absent from `recs`). No active postcode may ever be dropped.
+        main_a = _mbox(0, 0, 100, 200)
+        island = _mbox(150, 90, 180, 120)  # B absorbs this -> reassignment fires
+        a = MultiPolygon([main_a, island])
+        b = _mbox(100, 0, 300, 200).difference(island)
+        empty = Polygon()  # degenerate input that contributes no parts
+        out = _as_dict(
+            _eliminate_small_detached_parts([("A", a), ("B", b), ("EMPTY", empty)])
+        )
+        assert "EMPTY" in out, "an empty-geometry postcode must not be dropped"
+        assert "A" in out and "B" in out
+
+    def test_tiebreak_is_deterministic_smaller_postcode_wins(self):
+        # An island bordered by TWO postcodes with EQUAL shared edges must go to
+        # the lexicographically smaller postcode, independent of STRtree order.
+        a_main = _mbox(0, 0, 100, 100)            # A's main, far from the island
+        island = _mbox(300, 50, 320, 70)          # 400 m², A's detached part
+        a = MultiPolygon([a_main, island])
+        bbb = _mbox(250, 0, 300, 120)             # borders island's left edge
+        ccc = _mbox(320, 0, 370, 120)             # borders island's right edge (equal)
+        out = _as_dict(
+            _eliminate_small_detached_parts([("A", a), ("BBB", bbb), ("CCC", ccc)])
+        )
+        pt = island.representative_point()
+        assert out["BBB"].contains(pt), "tie must go to the smaller postcode string"
+        assert not out["CCC"].contains(pt)
+        assert out["A"].geom_type == "Polygon"
+
+    def test_gapped_sliver_within_nearest_radius_is_reassigned_not_dropped(self):
+        # A 300 m² sliver 1.2 m from its only neighbour N — beyond the border probe
+        # but inside the nearest-fallback radius. Before the gather buffer was
+        # widened to the accept radius, the old (smaller) gather buffer never
+        # returned N, so the sliver fell through to the drop branch. Now it is
+        # reassigned to N, conserving coverage.
+        x_main = _mbox(0, 0, 100, 100)            # far from the sliver
+        x_sliver = _mbox(278.8, 50, 298.8, 65)    # 300 m², right edge at x=298.8
+        x = MultiPolygon([x_main, x_sliver])
+        n = _mbox(300, 0, 400, 200)               # left edge at x=300 -> 1.2 m gap
+        before = unary_union([x, n]).area
+        out = _as_dict(_eliminate_small_detached_parts([("X", x), ("N", n)]))
+        assert out["X"].geom_type == "Polygon", "sliver should leave X"
+        assert out["N"].contains(x_sliver.representative_point()), "absorbed into N"
+        after = unary_union([out["X"], out["N"]]).area
+        assert after == pytest.approx(before, rel=1e-6), "coverage conserved, not dropped"
+
+    def test_approx_area_matches_real_metric_area(self):
+        # The latitude-scaled area approximation should be within a few percent of
+        # the true projected area for a building-scale polygon.
+        import pyproj
+        from shapely.ops import transform as transform_geometry
+
+        poly = _mbox(0, 0, 50, 60)  # ~3000 m²
+        to_bng = pyproj.Transformer.from_crs(
+            "EPSG:4326", "EPSG:27700", always_xy=True
+        )
+        true_m2 = transform_geometry(to_bng.transform, poly).area
+        assert _approx_area_m2(poly) == pytest.approx(true_m2, rel=0.02)
+
+
+# ---------------------------------------------------------------------------
+# Greenspace subtraction is connectivity-preserving
+# ---------------------------------------------------------------------------
+
+
+class TestGreenspaceReconnect:
+    """Greenspace that CROSSES a postcode must not shatter it: a narrow strip is
+    bridged back (postcode stays one piece), a wide barrier is left subtracted
+    (postcode genuinely splits)."""
+
+    def test_narrow_strip_reconnects_postcode(self):
+        from shapely.strtree import STRtree
+
+        postcode = box(0, 0, 200, 100)
+        strip = box(95, 0, 105, 100)  # 10 m wide green strip crossing the postcode
+        # Sanity: a plain difference WOULD split it into two parts.
+        assert postcode.difference(strip).geom_type == "MultiPolygon"
+
+        result = subtract_greenspace(postcode, STRtree([strip]), [strip])
+        assert result.geom_type == "Polygon", "narrow strip should be bridged"
+        assert result.is_valid
+
+    def test_wide_barrier_keeps_postcode_split(self):
+        from shapely.strtree import STRtree
+
+        postcode = box(0, 0, 200, 100)
+        barrier = box(60, 0, 130, 100)  # 70 m wide — beyond 2x the bridge width
+        result = subtract_greenspace(postcode, STRtree([barrier]), [barrier])
+        assert result.geom_type == "MultiPolygon", "wide barrier should stay split"
+        assert len(result.geoms) == 2
+
+    def test_edge_greenspace_still_trimmed(self):
+        # Greenspace on the edge (not crossing) is trimmed as before; reconnect is
+        # a no-op because the result stays a single piece.
+        from shapely.strtree import STRtree
+
+        postcode = box(0, 0, 100, 100)  # 10000 m²
+        park = box(60, 0, 100, 100)  # 4000 m² on the right edge
+        result = subtract_greenspace(postcode, STRtree([park]), [park])
+        assert result.geom_type == "Polygon"
+        assert result.area == pytest.approx(6000, rel=0.01)
+
+    def test_result_is_always_polygonal(self):
+        # Regression: _reconnect_split must never return a GeometryCollection
+        # (line/point debris) — downstream to_wgs84_geojson_multi would truncate a
+        # GC to a single part, silently dropping substantial pieces. Sweep many
+        # strip widths/offsets (incl. coincident-edge-prone integer geometries).
+        from shapely.strtree import STRtree
+
+        postcode = box(0, 0, 200, 120)
+        for w in (2, 5, 8, 10, 25, 49, 50, 51, 60, 90):
+            for x0 in (40, 70, 95, 100, 130):
+                strip = box(x0, -10, x0 + w, 130)
+                result = subtract_greenspace(postcode, STRtree([strip]), [strip])
+                assert result.geom_type in ("Polygon", "MultiPolygon"), (
+                    f"w={w} x0={x0} produced {result.geom_type}"
+                )
+                assert result.is_valid and not result.is_empty
+
+    def test_wide_barrier_preserves_all_substantial_parts(self):
+        # The motivating case for the GC fix: a wide barrier genuinely splits the
+        # postcode; ALL substantial parts must survive (not be truncated to one).
+        from shapely.strtree import STRtree
+
+        postcode = box(0, 0, 300, 100)
+        barrier = box(120, 0, 200, 100)  # 80 m wide -> beyond 2x bridge
+        result = subtract_greenspace(postcode, STRtree([barrier]), [barrier])
+        assert result.geom_type == "MultiPolygon"
+        areas = sorted(p.area for p in result.geoms)
+        assert areas == pytest.approx([10000, 12000], rel=0.01)  # both banks kept