Merge pull request #1329 from Hestia-Homes/feature/modelling-e2e-bugs

Fix: synthesise SapFloorDimension for SAP-16.0 flats with missing floor dimensions to unblock roof recommendations
This commit is contained in:
Daniel Roth 2026-06-25 16:12:46 +01:00 committed by GitHub
commit c89ab7692d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 147 additions and 13 deletions

View file

@ -799,7 +799,15 @@ class EpcPropertyDataMapper:
# D1: derive heat-loss perimeter + party-wall length from the
# measured wall areas (full SAP lodges neither directly).
sap_building_parts=[
_sap_17_1_building_part(bp, i)
_sap_17_1_building_part(
bp,
i,
total_floor_area=float(schema.total_floor_area),
is_single_part=len(schema.sap_building_parts) == 1,
is_flat=schema.sap_flat_details is not None,
address_line_1=schema.address_line_1,
postcode=schema.postcode,
)
for i, bp in enumerate(schema.sap_building_parts)
],
# D6: full-SAP heating — translate the differing field names onto
@ -2905,7 +2913,13 @@ def _with_recorded_performance(
def _sap_17_1_building_part(
bp: SapBuildingPart_SAP_17_1, index: int
bp: SapBuildingPart_SAP_17_1,
index: int,
total_floor_area: float,
is_single_part: bool,
is_flat: bool,
address_line_1: str,
postcode: str,
) -> SapBuildingPart:
"""D1: build one `SapBuildingPart`, deriving the heat-loss perimeter and
party-wall length the engine needs from the measured wall areas full SAP
@ -2915,6 +2929,12 @@ def _sap_17_1_building_part(
length, 5 internal discarded; any other code fails loud. Exposed/party
areas divide by Σ storey-heights so the per-storey perimeter, summed as
Σ(perimeter × height) by the engine, reconstructs the measured area exactly.
When `bp.sap_floor_dimensions` is absent (SAP-16.0 flat certs) and the cert
is confirmed single-part and flat-type, synthesises one SapFloorDimension
from the cert-level `total_floor_area`. Only safe for single-storey
single-part flats; multi-part or multi-storey cases are left empty so
downstream failures stay diagnosable.
"""
exposed_area = 0.0
party_area = 0.0
@ -2932,17 +2952,46 @@ def _sap_17_1_building_part(
perimeter_per_storey = exposed_area / total_height
party_length_per_storey = party_area / total_height
floor_dimensions = [
SapFloorDimension(
room_height_m=fd.storey_height,
total_floor_area_m2=fd.total_floor_area,
heat_loss_perimeter_m=perimeter_per_storey,
party_wall_length_m=party_length_per_storey,
floor=fd.storey,
floor_construction=fd.floor_type,
)
for fd in bp.sap_floor_dimensions
]
# Guard: synthesise only when floor dims are absent AND we can confirm
# single-storey single-part. `is_flat` (sap_flat_details present) is the
# current signal — flats are always single-storey within the dwelling.
# Bungalows are equally single-storey but carry no equivalent structural
# field; `dwelling_type` string matching is the only handle, and has not
# been confirmed necessary (no bungalows with empty floor dims were found
# in the failing batch). An alternative worth considering: drop `is_flat`
# entirely and trigger on `not bp.sap_floor_dimensions and is_single_part`
# alone — if the cert lodged no dims at all, there is no per-storey data to
# misattribute regardless of dwelling type. Requires corpus evidence that
# multi-storey single-part certs never omit floor dims before widening.
if not bp.sap_floor_dimensions and is_single_part and is_flat:
floor_dimensions = [
SapFloorDimension(
room_height_m=1.0,
total_floor_area_m2=total_floor_area,
heat_loss_perimeter_m=perimeter_per_storey,
party_wall_length_m=party_length_per_storey,
floor=0,
)
]
else:
if not bp.sap_floor_dimensions:
raise ValueError(
f"{address_line_1!r}, {postcode!r}, "
f"building_part {bp.building_part_number!r}: "
f"sap_floor_dimensions is empty and cannot be synthesised "
f"(is_single_part={is_single_part}, is_flat={is_flat})"
)
floor_dimensions = [
SapFloorDimension(
room_height_m=fd.storey_height,
total_floor_area_m2=fd.total_floor_area,
heat_loss_perimeter_m=perimeter_per_storey,
party_wall_length_m=party_length_per_storey,
floor=fd.storey,
floor_construction=fd.floor_type,
)
for fd in bp.sap_floor_dimensions
]
if index == 0:
identifier = BuildingPartIdentifier.MAIN

View file

@ -751,3 +751,81 @@ class TestFullSapSchema16xRouting:
# Assert
assert not any("broken schema_type" in r.message for r in caplog.records)
class TestFullSapSchema16xNoFloorDimensions:
"""SAP-Schema-16.0 certs with assessment_type=SAP that lodge no
sap_floor_dimensions inside sap_building_parts (uprn 10090783001,
cert lodged 88). The API only carries total_floor_area at the top level;
the building part has sap_walls but no per-storey dimension data.
This is the root cause of the `max() arg is an empty sequence` crash in
building_geometry.roof_area() documented in
scripts/handover_max_empty_sequence_design_question.md.
"""
@pytest.fixture
def epc(self) -> EpcPropertyData:
return EpcPropertyDataMapper.from_api_response(
load("sap_16_0_full_no_floor_dims.json")
)
def test_maps_successfully(self, epc: EpcPropertyData) -> None:
assert isinstance(epc, EpcPropertyData)
def test_uprn_and_total_floor_area(self, epc: EpcPropertyData) -> None:
assert epc.uprn == 10090783001
assert epc.total_floor_area_m2 == 72.0
def test_building_part_floor_dimensions_synthesised(
self, epc: EpcPropertyData
) -> None:
# The API lodges no sap_floor_dimensions for this cert family, but the
# mapper synthesises one from the cert-level total_floor_area so that
# roof_area() and gross_heat_loss_wall_area() do not crash downstream.
main_part = epc.sap_building_parts[0]
assert len(main_part.sap_floor_dimensions) == 1
def test_total_floor_area_available_at_cert_level(
self, epc: EpcPropertyData
) -> None:
# total_floor_area_m2 IS present at the EpcPropertyData level (72 m²).
# This is the candidate fallback value for roof_area() — accurate for
# single-storey single-part certs, an overestimate for multi-storey.
assert epc.total_floor_area_m2 == 72.0
def test_synthesises_single_floor_dimension(self, epc: EpcPropertyData) -> None:
# Arrange
main_part = epc.sap_building_parts[0]
# Act / Assert
assert len(main_part.sap_floor_dimensions) == 1
def test_synthesised_floor_dimension_total_area(self, epc: EpcPropertyData) -> None:
# Arrange
fd = epc.sap_building_parts[0].sap_floor_dimensions[0]
# Act / Assert
assert fd.total_floor_area_m2 == 72.0
def test_synthesised_floor_dimension_heat_loss_perimeter(
self, epc: EpcPropertyData
) -> None:
# Arrange: both sap_walls are wall_type=2 (exposed): 43.2 + 3.88 = 47.08 m².
# total_height falls back to 1.0 (no storey heights lodged), so perimeter = 47.08 m.
fd = epc.sap_building_parts[0].sap_floor_dimensions[0]
# Act / Assert
assert fd.heat_loss_perimeter_m == pytest.approx(47.08)
def test_raises_when_floor_dims_absent_and_synthesis_not_possible(
self,
) -> None:
# Arrange: strip sap_flat_details so is_flat=False — synthesis guard
# cannot fire, empty floor dims must raise rather than silently produce [].
data = load("sap_16_0_full_no_floor_dims.json")
data.pop("sap_flat_details", None)
# Act / Assert
with pytest.raises(ValueError, match="Wardalls Grove.*SE14 5FB.*sap_floor_dimensions"):
EpcPropertyDataMapper.from_api_response(data)

View file

@ -171,6 +171,11 @@ class EnergyElement:
environmental_efficiency_rating: int
@dataclass
class SapFlatDetails:
level: Optional[int] = None
@dataclass
class SapSchema17_1:
uprn: int
@ -199,3 +204,5 @@ class SapSchema17_1:
# Some 17.0 full-SAP certs omit the top-level flag and lodge it only under
# sap_heating; the mapper falls back to sap_heating.has_hot_water_cylinder.
has_hot_water_cylinder: Optional[str] = None
# Present for flat-type dwellings; absence means not a flat.
sap_flat_details: Optional[SapFlatDetails] = None