From 0a2ed67e94d06f7e855571c15db27b8533ad3ccf Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Fri, 26 Jun 2026 12:24:52 +0000 Subject: [PATCH] =?UTF-8?q?Harden=20Dwelling-Roof=20Cap=20on=20real=20data?= =?UTF-8?q?:=20positional=20segments,=20ground-floor=20basis=20?= =?UTF-8?q?=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three corrections found by re-running property 742003 end-to-end: - roofSegmentStats are POSITIONAL — real responses omit the segmentIndex field the fixture happened to carry; key the centre/area lookup by array position. - Base the cap on ground_floor_area (the footprint the roof covers), not the greatest per-storey area; roof_area is the fallback. - Clamp the basis by total_floor_area: predicted EPCs borrow the structural template's geometry (742003: a 118.62 m² MAIN ground floor) decoupled from the predicted 55 m² (ADR-0029), so without the clamp the cap reads the template's larger footprint. Result: 742003 plan A/92.4 (16 kWp) -> C/74.4 (6.4 kWp). 29 solar tests + orchestration threading + products green. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../generators/solar_recommendation.py | 26 +++++++++++-- domain/modelling/solar_potential.py | 14 ++++--- .../modelling/test_solar_config_selection.py | 38 +++++++++++++++++++ .../domain/modelling/test_solar_potential.py | 4 +- 4 files changed, 71 insertions(+), 11 deletions(-) diff --git a/domain/modelling/generators/solar_recommendation.py b/domain/modelling/generators/solar_recommendation.py index 5a855920..f5bf6e10 100644 --- a/domain/modelling/generators/solar_recommendation.py +++ b/domain/modelling/generators/solar_recommendation.py @@ -24,7 +24,7 @@ from datatypes.epc.domain.epc_property_data import ( ) from datatypes.epc.domain.epc_property_data import BuildingPartIdentifier from datatypes.epc.domain.field_mappings import PROPERTY_TYPE_LOOKUP -from domain.building_geometry import roof_area +from domain.building_geometry import ground_floor_area, roof_area from domain.geospatial.planning_restrictions import PlanningRestrictions from domain.modelling.products import Products, SolarCostInputs from domain.modelling.measure_type import MeasureType @@ -338,11 +338,29 @@ def recommend_solar( def _dwelling_roof_area_m2(epc: EpcPropertyData) -> Optional[float]: """The dwelling's own roof plan area for the Dwelling-Roof Cap (ADR-0038), or None when the EPC has no MAIN building part to measure (the cap then - falls back to Google's `maxArrayPanelsCount` cap).""" + falls back to Google's `maxArrayPanelsCount` cap). + + The roof covers the **ground-floor footprint**, so the basis is + `ground_floor_area` (not the greatest per-storey area); `roof_area` is the + fallback when no ground (floor 0) dimension is lodged. + + Clamped by total floor area: a footprint can never exceed the whole + dwelling's floor area (= footprint × storeys), so the clamp is a no-op on a + consistent lodged EPC. It matters for a **predicted** EPC, whose building-part + geometry is the structural template's — decoupled from the predicted floor + area (ADR-0029) — so without it the cap would read the template neighbour's + (larger) footprint rather than the predicted dwelling's size.""" try: - return roof_area(epc, BuildingPartIdentifier.MAIN) + footprint: float = ground_floor_area(epc, BuildingPartIdentifier.MAIN) except StopIteration: - return None + try: + footprint = roof_area(epc, BuildingPartIdentifier.MAIN) + except StopIteration: + return None + total: Optional[float] = epc.total_floor_area_m2 + if total is not None and total > 0.0: + return min(footprint, total) + return footprint def _solar_eligible( diff --git a/domain/modelling/solar_potential.py b/domain/modelling/solar_potential.py index cf9c13ad..2881c189 100644 --- a/domain/modelling/solar_potential.py +++ b/domain/modelling/solar_potential.py @@ -124,12 +124,16 @@ class SolarPotential: or "panelCapacityWatts" not in solar_potential ): return None - # Per-segment centre + area live on the top-level `roofSegmentStats`, - # keyed by `segmentIndex`; the per-config `roofSegmentSummaries` carry - # only the panel/orientation fields. Build the lookup once. + # Per-segment centre + area live on the top-level `roofSegmentStats`; + # the per-config `roofSegmentSummaries` carry only the panel/orientation + # fields. `roofSegmentSummaries[].segmentIndex` refers to the POSITION in + # `roofSegmentStats` (the entries are positional — Google omits an + # explicit `segmentIndex` field on them), so key the lookup by position. stats_by_index: dict[int, Mapping[str, Any]] = { - int(stats["segmentIndex"]): stats - for stats in solar_potential.get("roofSegmentStats", []) + index: stats + for index, stats in enumerate( + solar_potential.get("roofSegmentStats", []) + ) } def _segment(summary: Mapping[str, Any]) -> SolarRoofSegment: diff --git a/tests/domain/modelling/test_solar_config_selection.py b/tests/domain/modelling/test_solar_config_selection.py index dca22f69..da08fd1a 100644 --- a/tests/domain/modelling/test_solar_config_selection.py +++ b/tests/domain/modelling/test_solar_config_selection.py @@ -11,7 +11,14 @@ import json from pathlib import Path from typing import Any +from datatypes.epc.domain.epc_property_data import ( + BuildingPartIdentifier, + EpcPropertyData, + SapBuildingPart, + SapFloorDimension, +) from domain.modelling.generators.solar_recommendation import ( + _dwelling_roof_area_m2, select_conservative_configs, ) from domain.modelling.solar_potential import ( @@ -20,6 +27,37 @@ from domain.modelling.solar_potential import ( SolarRoofSegment, ) + +def _epc_with_roof(per_storey_areas: tuple[float, ...], total_floor_area: float) -> EpcPropertyData: + epc: EpcPropertyData = object.__new__(EpcPropertyData) + epc.total_floor_area_m2 = total_floor_area + part: SapBuildingPart = object.__new__(SapBuildingPart) + part.identifier = BuildingPartIdentifier.MAIN + dims: list[SapFloorDimension] = [] + for floor, area in enumerate(per_storey_areas): + fd: SapFloorDimension = object.__new__(SapFloorDimension) + fd.floor = floor # 0 = ground + fd.total_floor_area_m2 = area + dims.append(fd) + part.sap_floor_dimensions = dims + epc.sap_building_parts = [part] + return epc + + +def test_dwelling_roof_basis_is_ground_floor_clamped_by_total_floor_area() -> None: + # ADR-0038/0029: the basis is the ground-floor footprint, clamped by total + # floor area. A predicted EPC's building-part geometry is the structural + # template's (here a 118 m² ground floor), decoupled from the predicted + # floor area (55 m²); a footprint can't exceed total floor area, so the cap + # basis clamps to 55 — not the borrowed template's 118. + predicted = _epc_with_roof(per_storey_areas=(118.0,), total_floor_area=55.0) + assert _dwelling_roof_area_m2(predicted) == 55.0 + + # A consistent 2-storey house — ground 55, upper 50, total 105 — uses the + # GROUND floor (55), not the greatest per-storey area, and the clamp is inert. + lodged = _epc_with_roof(per_storey_areas=(55.0, 50.0), total_floor_area=105.0) + assert _dwelling_roof_area_m2(lodged) == 55.0 + _FIXTURE: Path = ( Path(__file__).resolve().parent / "fixtures" diff --git a/tests/domain/modelling/test_solar_potential.py b/tests/domain/modelling/test_solar_potential.py index 3c92914d..098b886e 100644 --- a/tests/domain/modelling/test_solar_potential.py +++ b/tests/domain/modelling/test_solar_potential.py @@ -122,9 +122,9 @@ def test_projection_enriches_segments_with_centre_and_area() -> None: # carry its centre + area — sourced from the top-level roofSegmentStats, # keyed by segmentIndex (the per-config roofSegmentSummaries omit them). insights = _insights() + # roofSegmentStats are positional — segmentIndex refers to the array index. stats_by_index = { - int(s["segmentIndex"]): s - for s in insights["solarPotential"]["roofSegmentStats"] + i: s for i, s in enumerate(insights["solarPotential"]["roofSegmentStats"]) } potential = SolarPotential.from_building_insights(insights)